Multi asic platform changes for interface, portchannel commands #878

judyjoseph · 2020-04-13T22:42:46Z

- What I did
Changes to the following config commands and scripts for Multi-ASIC devices.

config interface
Added a new argument to specify the namespace. [ -n namespace ]
In Multi-ASIC devices the namespace argument is mandatory
Ignored for Single ASIC platforms.

config portchannel
Added a new argument to specify the namespace. [ -n namespace ]
In Multi-ASIC devices it is mandatory for (add/del) of portchannel and member interface
Ignored for Single ASIC platforms.

- How I did it

The changes done here are based on the namespace support added in SonicDBConfig/ConfigDBConnector/SonicV2Connector classes in the PR sonic-net/sonic-py-swsssdk#63

To connect to DB in a particular namespace pass the parameter namespace=namespaceName
To connect to the DB in the local context ( where ever we are ) don't pass any namespace input, it would be defaulted to '' (empty string ). This is the default option, so that the single ASIC scenario will work as is.

The following are the changes introduced in this PR

(a) added a new parameter of "config_db" to the interface helper API's. This approach is better, as now we have to do this once config_db = ConfigDBConnector() in the CLI handler and the config_db instance is passed down as argument to the API.

(b) config interface, config port channel -- added the namespace option. This option is made mandatory for multi-asic platforms.

- How to verify it
Verified on a multi-ASIC devices, by running the Config commands.
Verified on single ASIC devices so that the commands works ok.

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

In Multi-ASIC

admin@str-n -acs-1:~$ sudo config interface
Usage: config interface [OPTIONS] COMMAND [ARGS]...

  Interface-related configuration tasks

Options:
  -n, --namespace [asic0|asic1|asic2|asic3|asic4|asic5]
                                  Namespace name  [required]
  -?, -h, --help                  Show this message and exit.

Commands:
  breakout     Set interface breakout mode
  fec          Set interface fec
  ip           Add or remove IP address
  mtu          Set interface mtu
  pfc          Set PFC configuration.
  shutdown     Shut down interface
  speed        Set interface speed
  startup      Start up interface
  transceiver  SFP transceiver configuration
  vrf          Bind or unbind VRF


admin@str-n -acs-1:~$ sudo config interface -n asic0 fec Ethernet0 none
admin@str-n -acs-1:~$ sudo ip netns exec asic0 show interface status 
      Interface            Lanes    Speed    MTU    FEC          Alias             Vlan    Oper    Admin    Type    Asym PFC
---------------  ---------------  -------  -----  -----  -------------  ---------------  ------  -------  ------  ----------
      Ethernet0      33,34,35,36      40G   9100   none    Ethernet1/1  PortChannel0002      up       up     N/A         off

admin@str-n -acs-1:~$ sudo config interface -n asic0 ip add Ethernet8 1.1.1.1 2.2.2.2
admin@str-n -acs-1:~$ sudo ip netns exec asic0 show ip interface
Interface        Master    IPv4 address/mask    Admin/Oper    BGP Neighbor    Neighbor IP
---------------  --------  -------------------  ------------  --------------  -------------
Ethernet8                  1.1.1.1/32           down/down     N/A             N/A

admin@str-n -acs-1:~$ sudo config interface -n asic0 ip remove Ethernet8 1.1.1.1
admin@str-n -acs-1:~$ sudo ip netns exec asic0 show ip interface
Interface        Master    IPv4 address/mask    Admin/Oper    BGP Neighbor    Neighbor IP
---------------  --------  -------------------  ------------  --------------  -------------
Loopback0                  8.0.0.0/32           up/up         N/A             N/A

admin@str-n -acs-1:~$ sudo config interface -n asic0 mtu Ethernet0 8600
admin@str-n -acs-1:~$ sudo ip netns exec asic0 show interface status 
      Interface            Lanes    Speed    MTU    FEC          Alias             Vlan    Oper    Admin    Type    Asym PFC
---------------  ---------------  -------  -----  -----  -------------  ---------------  ------  -------  ------  ----------
      Ethernet0      33,34,35,36      40G   8600   none    Ethernet1/1  PortChannel0002      up       up     N/A         off


admin@str-n -acs-1:~$ sudo config interface -n asic0 pfc asymmetric Ethernet8  on
admin@str-n -acs-1:~$ sudo config interface -n asic0 pfc asymmetric Ethernet8  off


admin@str-n -acs-1:~$ sudo config interface -n asic0 shutdown Ethernet0
admin@str-n -acs-1:~$ sudo ip netns exec asic0 show interface status
      Interface            Lanes    Speed    MTU    FEC          Alias             Vlan    Oper    Admin    Type    Asym PFC
---------------  ---------------  -------  -----  -----  -------------  ---------------  ------  -------  ------  ----------
      Ethernet0      33,34,35,36      40G   9100   none    Ethernet1/1  PortChannel0002    down     down     N/A         off


admin@str-n -acs-1:~$ sudo config interface -n asic0 startup Ethernet0
admin@str-n -acs-1:~$ sudo ip netns exec asic0 show interface status
      Interface            Lanes    Speed    MTU    FEC          Alias             Vlan    Oper    Admin    Type    Asym PFC
---------------  ---------------  -------  -----  -----  -------------  ---------------  ------  -------  ------  ----------
      Ethernet0      33,34,35,36      40G   9100   none    Ethernet1/1  PortChannel0002      up       up     N/A         off

admin@str-n -acs-1:~$ sudo config interface -n asic0 shutdown Ethernet-BP384
admin@str-n -acs-1:~$ sudo ip netns exec asic5 show interface status
      Interface            Lanes    Speed    MTU    FEC           Alias             Vlan    Oper    Admin    Type    Asym PFC
---------------  ---------------  -------  -----  -----  --------------  ---------------  ------  -------  ------  ----------
 Ethernet-BP384      29,30,31,32      40G   9100    N/A  Ethernet-BP384  PortChannel4013      up       up     N/A         off

admin@str-n -acs-1:~$ sudo config interface -n asic0 startup Ethernet-BP384
admin@str-n -acs-1:~$ sudo ip netns exec asic5 show interface status
      Interface            Lanes    Speed    MTU    FEC           Alias             Vlan    Oper    Admin    Type    Asym PFC
---------------  ---------------  -------  -----  -----  --------------  ---------------  ------  -------  ------  ----------
 Ethernet-BP384      29,30,31,32      40G   9100    N/A  Ethernet-BP384  PortChannel4013      up       up     N/A         off

admin@str-n -acs-1:~$ sudo config interface -n asic0 speed Ethernet0 40000
admin@str-n -acs-1:~$ sudo ip netns exec asic5 show interface status
      Interface            Lanes    Speed    MTU    FEC           Alias             Vlan    Oper    Admin    Type    Asym PFC
---------------  ---------------  -------  -----  -----  --------------  ---------------  ------  -------  ------  ----------
 Ethernet-BP384      29,30,31,32      40G   9100    N/A  Ethernet-BP384  PortChannel4013      up       up     N/A         off

admin@str-n -acs-1:~$ sudo config portchannel -n asic0 add PortChannel0022
admin@str-n -acs-1:~$ sudo config portchannel -n asic0 member add PortChannel0022 Etehrnet4
Usage: config portchannel member add [OPTIONS] <portchannel_name> <port_name>

Error: Interface name is invalid. Please enter a valid interface name!!
admin@str-n -acs-1:~$ sudo config portchannel -n asic0 member add PortChannel0022 Ethernet4
admin@str-n -acs-1:~$ sudo config portchannel -n asic0 member add PortChannel0022 Ethernet64
Usage: config portchannel member add [OPTIONS] <portchannel_name> <port_name>

Error: Interface name is invalid. Please enter a valid interface name!!
admin@str-n -acs-1:~$ sudo config portchannel -n asic0 member add PortChannel0022 Ethernet8
admin@str-n -acs-1:~$ sudo ip netns exec asic0 show interface status
      Interface            Lanes    Speed    MTU    FEC          Alias             Vlan    Oper    Admin    Type    Asym PFC
---------------  ---------------  -------  -----  -----  -------------  ---------------  ------  -------  ------  ----------
      Ethernet0      33,34,35,36      40G   9100   none    Ethernet1/1  PortChannel0002      up       up     N/A         off
      Ethernet4      29,30,31,32      40G   9100    N/A    Ethernet1/2  PortChannel0022      up       up     N/A         off
      Ethernet8      41,42,43,44      40G   9100    N/A    Ethernet1/3  PortChannel0022    down     down     N/A         off

admin@str-n -acs-1:~$ sudo config portchannel -n asic0 member del PortChannel0022 Ethernet4
admin@str-n -acs-1:~$ sudo config portchannel -n asic0 member del PortChannel0022 Ethernet8
admin@str-n -acs-1:~$ sudo ip netns exec asic0 show interface status
      Interface            Lanes    Speed    MTU    FEC          Alias             Vlan    Oper    Admin    Type    Asym PFC
---------------  ---------------  -------  -----  -----  -------------  ---------------  ------  -------  ------  ----------
      Ethernet0      33,34,35,36      40G   9100   none    Ethernet1/1  PortChannel0002      up       up     N/A         off
      Ethernet4      29,30,31,32      40G   9100    N/A    Ethernet1/2  PortChannel0002      up       up     N/A         off
      Ethernet8      41,42,43,44      40G   9100    N/A    Ethernet1/3           routed    down     down     N/A         off
admin@str-n -acs-1:~$ sudo config portchannel -n asic0 del PortChannel0022

In Single ASIC

**admin@:/$ sudo config interface**
Usage: config interface [OPTIONS] COMMAND [ARGS]...

  Interface-related configuration tasks

Options:
  -n, --namespace []  Namespace name

Note : Even though it is present as option, the user can ignore it. Planning to hide it in the command line once we have Click 7.0 in all branches which support "hidden" keyword.

config/main.py

lgtm-com · 2020-05-12T07:25:01Z

This pull request introduces 7 alerts when merging 2187176daf02ef3456b8194bafe0bf64ea11185c into 8bfa3b7 - view on LGTM.com

new alerts:

5 for Wrong number of arguments in a call
2 for Unnecessary pass

config/main.py

jleveque · 2020-05-12T17:32:23Z

Also fix LGTM alerts

judyjoseph · 2020-05-12T18:41:11Z

Also fix LGTM alerts

Fixed already, thanks

jleveque · 2020-05-12T19:23:11Z

Fixed already, thanks

Strange ... The LGTM check didn't re-run, and now it's missing from the list of checks.

jleveque · 2020-05-12T19:38:00Z

Looks good to me. Wait for other reviewers.

jleveque

Looks good to me. Wait for other reviewers.

lguohan · 2020-05-29T06:06:03Z

config/main.py

+        else:
+            namespace = ns
+    config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace)
+    config_db.connect()


remove code duplication.

1822, 1862, ...

Will remove code duplication, will make it functions and invoke those, thanks.

Defined new function get_interface_configDB_connector() and used it to remove the code duplicated.

lguohan · 2020-05-29T06:06:34Z

config/main.py

+        if intf_ns is None:
+            ctx.fail("member interface {} is invalid".format(port_name))
+        elif intf_ns != ctx.obj['namespace']:
+            ctx.fail("member interface {} doesn't exist in namespace {}".format(port_name, ctx.obj['namespace']))


code duplication as above

same as above

Updated the code.

lguohan · 2020-05-29T06:07:20Z

config/main.py

@@ -931,11 +964,21 @@ def hostname(new_hostname):
 # 'portchannel' group ('config portchannel ...')
 #
 @config.group(cls=AbbreviationGroup)
+@click.option('-n', '--namespace', help='Namespace name', default=None)


None->DEFAULT_NAMESPACE

lguohan · 2020-05-29T06:09:03Z

config/main.py

@@ -167,11 +167,42 @@ def validate_namespace(namespace):
    else:
        return False

-def interface_alias_to_name(interface_alias):
+# Return the namespace where an interface belongs
+def get_intf_namespace(port):


get_port_namespace

lguohan · 2020-05-29T06:10:55Z

config/main.py

+    if port.startswith("PortChannel") or port.startswith("Vlan") or port.startswith("Loopback"):
+        return None
+
+    if port.startswith("Ethernet"):


get the port table, if it is in the port table then return the namespace, otherwise return non.

Added a new API get_port_table_name() to get the table to be checked for the existence of the interface

lguohan · 2020-05-29T06:11:07Z

config/main.py

+        return DEFAULT_NAMESPACE
+
+    # If it is PortChannel or Vlan interface or Loopback, user needs to input the namespace.
+    if port.startswith("PortChannel") or port.startswith("Vlan") or port.startswith("Loopback"):


no need to hardcode all these logics.

The idea of checking for interfaces like PortChannel, SVI interfaces here and return back "None" explicitly so that the user will have to mandatorily pass the parameter -n namespace for such interfaces.

For physical interfaces ( those starting with "Ethernet") idea was to make the -namespace parameter optional.

If the user don't provide it, the config handler will use the namespace obtained from this get_port_namespace() API.

If the user provides the namespace parameter, the config handler will make sure it is same as what we find from this get_port_namespace() API.

For the physical interfaces behavior could be similar to single NPU devices now .. where user need not pass the namespace as a mandatory parameter while configuring.

Added a new API get_port_table_name() to get the table to be checked for the existence of the interface. Removed the earlier checks.

lguohan · 2020-05-29T06:19:19Z

config/main.py

@@ -973,6 +1016,14 @@ def portchannel_member(ctx):
 def add_portchannel_member(ctx, portchannel_name, port_name):
    """Add member to port channel"""
    db = ctx.obj['db']
+    if sonic_device_util.is_multi_npu():


this is logic is not straightforward.

why not just connect to the db in ns, and check if port_name is in the port_table, it will work for both single asic and multi-asic.

The logic here is to make sure that the member interface which is getting added into the port channel is in fact present in the same namespace the user inputted with the -n namespace argument.
I have introduced this check only for the multi-asic platforms where -n namespace parameter is mandatory with these port-channel commands.

Okay but I understand the point from your comment to avoid the multi-npu check, will take care of it - thanks.

I updated to use the existing function interface_name_is_valid(), as in this case we already have the details of which config db to connect to.

lguohan · 2020-05-29T06:20:51Z

config/main.py

    """VLAN-related configuration tasks"""
    kwargs = {}
    if redis_unix_socket_path:
        kwargs['unix_socket_path'] = redis_unix_socket_path
-    config_db = ConfigDBConnector(**kwargs)
+
+    # If multi ASIC platform, check if the namespace entered by user is valid


duplicate code. make it as a single check function.

lguohan · 2020-05-29T06:22:05Z

config/main.py

+        if intf_ns is None:
+            ctx.fail("member interface {} is invalid".format(interface_name))
+        elif intf_ns != ctx.obj['namespace']:
+            ctx.fail("member interface {} doesn't exist in namespace {}".format(interface_name, ctx.obj['namespace']))


this check logic is special, just connect to ctx.obj['namespace'] and check if port is there or not, make it work for single asic too.

I updated to use the existing function interface_name_is_valid(), as in this case we already have the details of which config db to connect to.

lguohan · 2020-05-29T06:22:19Z

config/main.py

@@ -1361,12 +1439,20 @@ def add_vlan_member(ctx, vid, interface_name, untagged):
 def del_vlan_member(ctx, vid, interface_name):
    """Delete VLAN member"""
    log_info("'vlan member del {} {}' executing...".format(vid, interface_name))
+    if sonic_device_util.is_multi_npu():


same as above.

judyjoseph · 2020-08-14T19:15:05Z

Updates based on multi_asic and other API's in to sonic_py_common package. The build is failing now as "multi_asic" and "interface" utilities are still not updated in sonic_py_common package.

lgtm-com · 2020-08-14T19:21:56Z

This pull request introduces 1 alert when merging 2d2e7c9 into a15b6bf - view on LGTM.com

new alerts:

1 for Unused import

…e rebasing.

judyjoseph · 2020-08-19T19:17:43Z

retest this please

judyjoseph · 2020-08-21T18:10:18Z

retest this please

Verified on both single-asic and multi-asic platforms.

judyjoseph · 2020-08-25T16:30:57Z

@jleveque have updated the TODO's for the functions which we can move to py-common. Also updated the PR description with tests with the CLI commands in multi-asic device and syntax for reference. Please take a look.

jleveque

LGTM. Please wait for other reviews.

…mmands ( sonic-net#878) into 201911 branch

…ceiver (#1087) * Merging the Multi asic platform changes for interface, portchannel commands ( #878) into 201911 branch * Merging the Multi asic platform changes show transceiver command (PR #1081) into 201911 branch * Updated needed in 201911 * Correcting the changes done to sfputil/main.py * Remove unused import

* 0323d5e noaOrMlnx Fix flex counters logic of converting poll interval to seconds from MS (sonic-net#878) Signed-off-by: Volodymyr Samotiy <[email protected]>

judyjoseph requested review from prsunny, pavel-shirshov and lguohan April 14, 2020 16:35

judyjoseph added Request for 201911 Branch Multi-ASIC 🔢 labels Apr 30, 2020

judyjoseph force-pushed the intf_cmds branch from be47642 to 2a1e347 Compare May 12, 2020 01:06

judyjoseph requested a review from jleveque May 12, 2020 01:47

jleveque suggested changes May 12, 2020

View reviewed changes

config/main.py Outdated Show resolved Hide resolved

config/main.py Outdated Show resolved Hide resolved

config/main.py Outdated Show resolved Hide resolved

jleveque suggested changes May 12, 2020

View reviewed changes

config/main.py Outdated Show resolved Hide resolved

config/main.py Outdated Show resolved Hide resolved

jleveque previously approved these changes May 12, 2020

View reviewed changes

judyjoseph dismissed jleveque’s stale review via e3f9328 May 19, 2020 15:21

judyjoseph force-pushed the intf_cmds branch from a17a5d4 to e3f9328 Compare May 19, 2020 15:21

judyjoseph requested a review from jleveque May 22, 2020 17:13

jleveque previously approved these changes May 22, 2020

View reviewed changes

lguohan reviewed May 29, 2020

View reviewed changes

judyjoseph dismissed jleveque’s stale review via 33dad12 June 9, 2020 16:24

Vlan command moved to a different file vlan.py, and some updates whil…

07b24e3

…e rebasing.

lguohan previously approved these changes Aug 21, 2020

View reviewed changes

Fixes needed to get in sync with the common library changes.

e956e38

Verified on both single-asic and multi-asic platforms.

judyjoseph dismissed lguohan’s stale review via e956e38 August 24, 2020 22:37

This comment has been minimized.

Sign in to view

Fix the lgtm warning

4f068fa

judyjoseph changed the title ~~Multi asic platform changes for interface, vlan, portchannel commands~~ Multi asic platform changes for interface, portchannel commands Aug 24, 2020

judyjoseph requested a review from jleveque August 25, 2020 01:22

Update TODO comments for API's which we plan to move to sonic-py-common

0113996

jleveque previously approved these changes Aug 25, 2020

View reviewed changes

Merge branch 'master' into intf_cmds

549e4f8

judyjoseph dismissed jleveque’s stale review via 549e4f8 August 26, 2020 01:45

Removing adding line while merge

78162bf

jleveque approved these changes Aug 26, 2020

View reviewed changes

judyjoseph merged commit e18252e into sonic-net:master Aug 26, 2020

judyjoseph mentioned this pull request Sep 1, 2020

[201911] Multi asic platform config interface portchannel, show transceiver #1087

Merged

stephenxs mentioned this pull request Sep 4, 2020

[submodule] Advance submodule head for sonic-utilities sonic-net/sonic-buildimage#5318

Closed

3 tasks

judyjoseph added a commit to judyjoseph/sonic-utilities that referenced this pull request Sep 14, 2020

Merging the Multi asic platform changes for interface, portchannel co…

6a57b5e

…mmands ( sonic-net#878) into 201911 branch

abdosi added the Included in 201911 Branch label Sep 19, 2020

Multi asic platform changes for interface, portchannel commands #878

Multi asic platform changes for interface, portchannel commands #878

Conversation

judyjoseph commented Apr 13, 2020 • edited Loading

In Multi-ASIC

In Single ASIC

lgtm-com bot commented May 12, 2020

jleveque commented May 12, 2020

judyjoseph commented May 12, 2020

jleveque commented May 12, 2020

jleveque commented May 12, 2020

jleveque left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

judyjoseph Jun 9, 2020 • 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

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

judyjoseph Jun 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

judyjoseph May 29, 2020 • 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

judyjoseph commented Aug 14, 2020 • edited Loading

lgtm-com bot commented Aug 14, 2020

judyjoseph commented Aug 19, 2020

judyjoseph commented Aug 21, 2020

This comment has been minimized.

judyjoseph commented Aug 25, 2020

jleveque left a comment

Choose a reason for hiding this comment

judyjoseph commented Apr 13, 2020 •

edited

Loading

judyjoseph Jun 9, 2020 •

edited

Loading

judyjoseph Jun 9, 2020 •

edited

Loading

judyjoseph May 29, 2020 •

edited

Loading

judyjoseph commented Aug 14, 2020 •

edited

Loading