Fix the 'show acl table' format issue #155

zhenggen-xu · 2017-11-27T21:05:22Z

Old output:
Command: acl-loader show table
Name Type Ports Description

dataacl L3 , dataacl
,
,
,
1
1
1
6
7
8
8
9
E
E
E
E
E
e
e
e
e
e
...

New output:
$ show acl table
Command: acl-loader show table
Name Type Ports Description

dataacl L3 Ethernet8 dataacl
Ethernet9
Ethernet16
Ethernet17
Ethernet18

jleveque · 2017-11-27T21:14:08Z

acl_loader/main.py

@@ -384,7 +385,7 @@ def show_table(self, table_name):
            if not val["ports"]:
                data.append([key, val["type"], "", val["policy_desc"]])
            else:
-                ports = sorted(val["ports"], )
+                ports = natsorted(val["ports"].split(","),)


Why the .split(",")? val["ports"] is a list, and does not have a .split() attribute.

Also, you can remove the trailing comma.

I believe the final working line should simply be ports = natsorted(val["ports"])

Thanks for reviewing, the "val["ports"]" is not list but string if the key of the ACL Table uses "ports", It is working with key being "ports@". I am not exactly sure which one we should use, I was using "ports" which was why it didn't work. Any insight of which should be used?

acl json examples seem to suggest "ports"

So you're claiming that the format of the "ports" value can vary between a list and a comma-separated string? I have only seen a list as the value of "ports". I believe a list is the expected data type.

@oleksandrivantsiv: Can you please confirm/add insight here?

This is the format in config_db.json:
"ACL_TABLE": {
"dataacl": {
"type": "L3",
"policy_desc": "dataacl",
"ports": "Ethernet8,Ethernet9,Ethernet16,Ethernet17,Ethernet18"
},
Which give me string for val["ports"]. but If I use "ports@" instead in the config_db.json, I got list. But the document and examples suggest the schema should be "ports". How did you load the ACL table? Can you check the ACL_TABLE in redis-DB to see the keys/values?

If you look at this example, it would not work with acl-loader script:
https://github.com/Azure/SONiC/wiki/ACL-High-Level-Design#appendix-b-sample-input-json-file

@taoyl-ms Can we make it clear which schema we should use:
{ "ports": "Ethernet8,Ethernet9" } or
{ "ports": ["Ethernet8", "Ethernet9"] } ?

I am fine with either way, but we need to be clear from the document and examples.

I believe we want the ports value defined as a list, e.g., { "ports": ["Ethernet8", "Ethernet9"] }.

@taoyl-ms: Can you confirm? If you agree it should be a list, can you please update any incorrect documentation?

Yes, list is the correct schema to use. I'll edit the incorrect document mentioned by @zhenggen-xu.

Thanks, @taoyl-ms!

@zhenggen-xu: As we have now determined your issue was due to data formatting and not an actual issue with acl_loader, I suggest you close this PR. However, I believe the change to natural sorting the results in the command line output is a nice enhancement. Could you please open a new, clean PR with this change and a more appropriate title?

sure, will do that.

oleksandrivantsiv · 2017-11-29T08:00:16Z

acl_loader/main.py

@@ -384,7 +385,7 @@ def show_table(self, table_name):
            if not val["ports"]:
                data.append([key, val["type"], "", val["policy_desc"]])
            else:
-                ports = sorted(val["ports"], )
+                ports = natsorted(val["ports"].split(","),)


acl-loader doesn't operate with config_db.json directly. It uses API defined in swsssdk python package (ConfigDBConnector module). Data that ConfigDBConnector class returns can be verified through python interpreter:

In [1]: from swsssdk import ConfigDBConnector In [2]: configdb = ConfigDBConnector() In [3]: configdb.connect() In [4]: configdb.get_table("ACL_TABLE") Out[4]: {'DATAACL': {'policy_desc': 'DATAACL', 'ports': ['Ethernet0', 'Ethernet4', 'Ethernet8', 'Ethernet12', 'Ethernet16', 'Ethernet20', 'Ethernet24', 'Ethernet28', 'Ethernet32', 'Ethernet36', 'Ethernet40', 'Ethernet44', 'Ethernet48', 'Ethernet52', 'Ethernet56', 'Ethernet60', 'Ethernet64', 'Ethernet68', 'Ethernet72', 'Ethernet76', 'Ethernet80', 'Ethernet84', 'Ethernet88', 'Ethernet92', 'Ethernet96', 'Ethernet100', 'Ethernet104', 'Ethernet108', 'Ethernet112', 'Ethernet116', 'Ethernet120', 'Ethernet124'], 'type': 'L3'}, 'EVERFLOW': {'policy_desc': 'EVERFLOW', 'ports': ['Ethernet8', 'Ethernet0', 'Ethernet4', 'Ethernet108', 'Ethernet100', 'Ethernet104', 'Ethernet68', 'Ethernet96', 'Ethernet124', 'Ethernet92', 'Ethernet120', 'Ethernet52', 'Ethernet56', 'Ethernet76', 'Ethernet72', 'Ethernet64', 'Ethernet32', 'Ethernet16', 'Ethernet36', 'Ethernet12', 'Ethernet88', 'Ethernet116', 'Ethernet80', 'Ethernet112', 'Ethernet84', 'Ethernet48', 'Ethernet44', 'Ethernet40', 'Ethernet28', 'Ethernet60', 'Ethernet20', 'Ethernet24'], 'type': 'MIRROR'}}

acl-loader show table command output for the same data:

root@arc-switch1026:/home/admin# acl-loader show table Name Type Ports Description -------- ------ ----------- ------------- EVERFLOW MIRROR Ethernet0 EVERFLOW Ethernet100 Ethernet104 Ethernet108 Ethernet112 Ethernet116 Ethernet12 Ethernet120 Ethernet124 Ethernet16 Ethernet20 Ethernet24 Ethernet28 Ethernet32 Ethernet36 Ethernet4 Ethernet40 Ethernet44 Ethernet48 Ethernet52 Ethernet56 Ethernet60 Ethernet64 Ethernet68 Ethernet72 Ethernet76 Ethernet8 Ethernet80 Ethernet84 Ethernet88 Ethernet92 Ethernet96 DATAACL L3 Ethernet0 DATAACL Ethernet100 Ethernet104 Ethernet108 Ethernet112 Ethernet116 Ethernet12 Ethernet120 Ethernet124 Ethernet16 Ethernet20 Ethernet24 Ethernet28 Ethernet32 Ethernet36 Ethernet4 Ethernet40 Ethernet44 Ethernet48 Ethernet52 Ethernet56 Ethernet60 Ethernet64 Ethernet68 Ethernet72 Ethernet76 Ethernet8 Ethernet80 Ethernet84 Ethernet88 Ethernet92 Ethernet96

On the latest immage command works as expected:

root@arc-switch1026:/home/admin# show version SONiC Software Version: SONiC.HEAD.159-b7c2ffa Distribution: Debian 8.9 Kernel: 3.16.0-4-amd64 Build commit: b7c2ffa Build date: Tue Nov 28 07:45:43 UTC 2017 Built by: johnar@jenkins-worker-4 Docker images: REPOSITORY TAG IMAGE ID SIZE docker-orchagent-mlnx HEAD.159-b7c2ffa f28cb41712a9 258.2 MB docker-orchagent-mlnx latest f28cb41712a9 258.2 MB docker-syncd-mlnx-rpc HEAD.159-b7c2ffa e0a60aed904f 567.6 MB docker-syncd-mlnx-rpc latest e0a60aed904f 567.6 MB docker-lldp-sv2 HEAD.159-b7c2ffa f5e98d128cf5 255.4 MB docker-lldp-sv2 latest f5e98d128cf5 255.4 MB docker-dhcp-relay HEAD.159-b7c2ffa 59898b2dfb45 251.9 MB docker-dhcp-relay latest 59898b2dfb45 251.9 MB docker-database HEAD.159-b7c2ffa 80daddb46b39 250.6 MB docker-database latest 80daddb46b39 250.6 MB docker-snmp-sv2 HEAD.159-b7c2ffa d99f64816f01 290.2 MB docker-snmp-sv2 latest d99f64816f01 290.2 MB docker-teamd HEAD.159-b7c2ffa a4a0c47214c3 255.3 MB docker-teamd latest a4a0c47214c3 255.3 MB docker-router-advertiser HEAD.159-b7c2ffa cf0a57ec182d 248.2 MB docker-router-advertiser latest cf0a57ec182d 248.2 MB docker-platform-monitor HEAD.159-b7c2ffa 2abfa1cc3e20 269.9 MB docker-platform-monitor latest 2abfa1cc3e20 269.9 MB docker-fpm-quagga HEAD.159-b7c2ffa a9fcd823c52e 261.8 MB docker-fpm-quagga latest a9fcd823c52e 261.8 MB root@arc-switch1026:/home/admin#

Update transceiver info DB key names (sonic-net#146) [Multiasic]: Provide namespace support for ipNetToMediaPhysAddress (sonic-net#129) [LLDP]: Modify OID index of LLDPRemTableUpdater MIB (sonic-net#155) [Namespace]: Simplify sync_d functions to use higher order (sonic-net#154) [Namespace]: Fix interface counters in RFC 1213 (sonic-net#145) Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

61acd3a2e4a457f3bc706cbfaf3162b947763864 (HEAD -> 201911, origin/201911) [xcvrd] Change in xcvrd ports cache creation, now ports are being fetched from config DB (#5892) (sonic-net#155) Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

…net#155) This PR fixes eeprom_tlvinfo.py to make it compatible with both Python2 & Python3. The current implementation causes runtime issue as follows: ``` Dec 14 12:04:34.908841 sonic WARNING pmon#syseepromd[51]: Failed to load platform-specific eeprom from sonic_platform package due to UnicodeDecodeError('ascii', '*\x02\x00\x8c', 3, 4, 'ordinal not in range(128)') Dec 14 12:04:34.910093 sonic NOTICE swss#orchagent: :- doPortTask: Set port Ethernet56 admin status to up Dec 14 12:04:34.923442 sonic ERR pmon#syseepromd[51]: Failed to load platform-specific eeprom implementation: UnicodeDecodeError('ascii', '*\x02\x00\x8c', 3, 4, 'ordinal not in range(128)') Dec 14 12:04:34.925628 sonic NOTICE swss#orchagent: :- doPortTask: Set port Ethernet60 MTU to 9100 Dec 14 12:04:34.939277 sonic NOTICE swss#orchagent: :- doPortTask: Set port Ethernet60 fec to rs Dec 14 12:04:34.943819 sonic NOTICE swss#orchagent: :- doPortTask: Set port Ethernet60 admin status to up ``` This cause `syseepromd` crash: ``` Dec 14 12:04:43.194489 sonic INFO pmon#supervisord 2020-12-14 12:04:34,651 INFO spawned: 'syseepromd' with pid 51 Dec 14 12:04:43.194489 sonic INFO pmon#supervisord 2020-12-14 12:04:34,955 INFO exited: syseepromd (exit status 5; not expected) ``` Signed-off-by: Andriy Kokhan <akokhan@barefootnetworks.com>

Fix the 'show acl table' format issue

5b0a859

jleveque suggested changes Nov 27, 2017

View reviewed changes

jleveque added the Enhancement label Nov 27, 2017

oleksandrivantsiv suggested changes Nov 29, 2017

View reviewed changes

zhenggen-xu closed this Dec 1, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the 'show acl table' format issue #155

Fix the 'show acl table' format issue #155

zhenggen-xu commented Nov 27, 2017 •

edited

Loading

jleveque Nov 27, 2017

zhenggen-xu Nov 28, 2017

zhenggen-xu Nov 28, 2017

jleveque Nov 28, 2017

zhenggen-xu Nov 28, 2017

zhenggen-xu Nov 29, 2017

jleveque Nov 30, 2017

taoyl-ms Nov 30, 2017

jleveque Nov 30, 2017 •

edited

Loading

zhenggen-xu Dec 1, 2017

oleksandrivantsiv Nov 29, 2017

Fix the 'show acl table' format issue #155

Fix the 'show acl table' format issue #155

Conversation

zhenggen-xu commented Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jleveque Nov 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhenggen-xu commented Nov 27, 2017 •

edited

Loading

jleveque Nov 30, 2017 •

edited

Loading