Switchport Modes Port & Port Channel Config-Engine Support by ridahanif96 · Pull Request #17632 · sonic-net/sonic-buildimage

ridahanif96 · 2023-12-28T12:49:54Z

Why I did it

This PR is created in reference to add Switchport Modes Port & Port Channel config-engine changes

How I did it

I have added changes in sonic-config-engine related files.
Added support of mode attribute "routed or trunk" in minigraph.py. Default value will be routed and vlan membership for port and portchannel will have trunk

How to verify it

Port & Portchannel with vlan membership will have mode attribute with value trunk else it will be routed by default.

Description for the changelog

This PR is added to provide support for Switchport Modes Port & Port Channel Config-Engine changes

Link to config_db schema for YANG module changes

This PR is linked with PR which has YANG model support

ganglyu · 2024-01-22T00:18:43Z

I think we need to merge #13580 at first, and make sure PR checker can pass.

ridahanif96 · 2024-01-22T03:54:50Z

I think we need to merge #13580 at first, and make sure PR checker can pass.

Agree, waiting for PR#13580 to get merged so we can pass failures

qiluo-msft · 2024-01-26T19:19:57Z

Could you merge the latest master? #Closed

ridahanif96 · 2024-01-26T19:23:14Z

Could you merge the latest master?

Let's me rerun this latest push to pass these failures. Thanks

ridahanif96 · 2024-01-27T13:42:22Z

@ganglyu can you please review this PR. All failures has been passed.

ganglyu · 2024-01-28T00:55:51Z

src/sonic-config-engine/minigraph.py

                    del pc_members[pc_mbr_del_key]

-    # set default port channel MTU as 9100 and admin status up and default TPID 0x8100
+    # set default port channel MTU as 9100 and admin status up and default TPID 0x8100 


space at the end of line

ganglyu · 2024-01-28T00:56:53Z

src/sonic-config-engine/tests/sample_output/platform_output.json

+      "mode": "routed"
  }
-}
+}


Please fix end of file.

ganglyu · 2024-01-28T00:57:44Z

src/sonic-config-engine/tests/t0-sample-autoneg-port-config.ini

-Ethernet124     101,102,103,104   fortyGigE0/124    on
+# name          lanes             alias             autoneg      mode
+Ethernet0       29,30,31,32       fortyGigE0/0      off          routed
+Ethernet4       25,26,27,28       fortyGigE0/4      off          trunk 


space at end of line

ganglyu · 2024-01-28T00:57:51Z

src/sonic-config-engine/tests/t0-sample-autoneg-port-config.ini

+Ethernet0       29,30,31,32       fortyGigE0/0      off          routed
+Ethernet4       25,26,27,28       fortyGigE0/4      off          trunk 
+Ethernet8       37,38,39,40       fortyGigE0/8      off          trunk
+Ethernet12      33,34,35,36       fortyGigE0/12     off          trunk 


space at end of line

ganglyu · 2024-01-28T00:57:58Z

src/sonic-config-engine/tests/t0-sample-autoneg-port-config.ini

+Ethernet12      33,34,35,36       fortyGigE0/12     off          trunk 
+Ethernet16      41,42,43,44       fortyGigE0/16     off          trunk
+Ethernet20      45,46,47,48       fortyGigE0/20     off          trunk
+Ethernet24      5,6,7,8           fortyGigE0/24     off          trunk 


space at end of line

ganglyu · 2024-01-28T00:58:13Z

src/sonic-config-engine/tests/t0-sample-port-config.ini

+Ethernet28      1,2,3,4           fortyGigE0/28         trunk
+Ethernet32      9,10,11,12        fortyGigE0/32         trunk
+Ethernet36      13,14,15,16       fortyGigE0/36         trunk
+Ethernet40      21,22,23,24       fortyGigE0/40         trunk 


space at end of line

ganglyu · 2024-01-28T00:58:26Z

src/sonic-config-engine/tests/t0-sample-port-config.ini

+Ethernet52      53,54,55,56       fortyGigE0/52         trunk
+Ethernet56      61,62,63,64       fortyGigE0/56         trunk
+Ethernet60      57,58,59,60       fortyGigE0/60         trunk
+Ethernet64      65,66,67,68       fortyGigE0/64         trunk 


space at end of line

ganglyu · 2024-01-28T00:58:36Z

src/sonic-config-engine/tests/t0-sample-port-config.ini

+Ethernet60      57,58,59,60       fortyGigE0/60         trunk
+Ethernet64      65,66,67,68       fortyGigE0/64         trunk 
+Ethernet68      69,70,71,72       fortyGigE0/68         trunk
+Ethernet72      77,78,79,80       fortyGigE0/72         trunk 


space at end of line

ganglyu · 2024-01-28T00:59:12Z

src/sonic-config-engine/tests/t0-sample-port-config.ini

+Ethernet92      113,114,115,116   fortyGigE0/92         trunk
+Ethernet96      121,122,123,124   fortyGigE0/96         trunk
+Ethernet100     125,126,127,128   fortyGigE0/100        trunk
+Ethernet104     85,86,87,88       fortyGigE0/104        trunk 


space at end of line

ganglyu · 2024-01-28T01:00:13Z

src/sonic-config-engine/tests/test_cfggen.py

        )

    def test_minigraph_neighbor_interfaces(self):
        argument = ['-m', self.sample_graph_simple_case, '-p', self.port_config, '-v', "PORT"]
        output = self.run_script(argument)
-
+        self.maxDiff = None


why do you need this change?

Removed this change

ganglyu · 2024-01-28T01:00:26Z

src/sonic-config-engine/tests/test_cfggen.py

            )
        )

    def test_minigraph_neighbor_interfaces_config_db(self):
        # test to check if PORT table is retrieved from config_db
        argument = ['-m', self.sample_graph_simple_case, '-p', self.port_config, '-v', "PORT"]
        output = self.run_script(argument)
-
+        self.maxDiff = None


why do you need this change?

Removed this change

ganglyu · 2024-01-28T01:00:42Z

src/sonic-config-engine/tests/test_cfggen.py

            )
        )

    def test_minigraph_extra_ethernet_interfaces(self, **kwargs):
        graph_file = kwargs.get('graph_file', self.sample_graph_simple)
        argument = ['-m', graph_file, '-p', self.port_config, '-v', "PORT"]
        output = self.run_script(argument)
+        self.maxDiff = None


why do you need this change?

Removed this change

ganglyu · 2024-01-28T01:01:00Z

src/sonic-config-engine/tests/test_cfggen.py

@@ -970,6 +969,7 @@ def test_minigraph_voq_inband_port(self):
        argument = ["-j", self.macsec_profile, "-m", self.sample_graph_voq, "-p", self.voq_port_config, "--var-json", "PORT"]
        output = self.run_script(argument)
        output_dict = utils.to_dict(output.strip())
+        self.maxDiff = None


same question as above

Removed this change

ganglyu · 2024-01-28T01:01:17Z

src/sonic-config-engine/tests/test_cfggen.py

@@ -1084,52 +1085,51 @@ def test_minigraph_packet_chassis_400g_zr_port_config(self):
    def test_minigraph_400g_to_100G_speed(self):
        argument = ["-j", self.macsec_profile, "-m", self.voq_sample_masic_graph, "-p", self.voq_port_config_400g, "-n",  "asic0", "-v", "PORT"]
        output = self.run_script(argument)
+        self.maxDiff = None


same question as above

Removed this change

ganglyu · 2024-01-28T01:01:31Z

src/sonic-config-engine/tests/test_cfggen.py

    def test_minigraph_cisco_400g_to_100G_speed(self):
        argument = ["-m", self.sample_cisco_100_graph, "-p", self.sample_cisco_port_config_400g, "-v", "PORT"]
        self.assertTrue(self.yang.validate(argument))
        output = self.run_script(argument)
+        self.maxDiff = None


same question as above

Removed this change

ganglyu · 2024-01-28T01:01:44Z

src/sonic-config-engine/tests/test_cfggen.py

    def test_minigraph_cisco_400G_to_400G_speed(self):
        argument = ["-m", self.sample_cisco_400_graph, "-p", self.sample_cisco_port_config_400g, "-v", "PORT"]
        output = self.run_script(argument)
+        self.maxDiff = None


same question as above

Removed this change

ganglyu · 2024-01-28T01:01:58Z

src/sonic-config-engine/tests/test_cfggen.py

@@ -1138,10 +1138,11 @@ def test_minigraph_cisco_400g_to_100G_speed_no_lane_change(self):
        argument = ["-m", self.sample_cisco_8111_graph, "-p", self.sample_cisco_8111_port_config, "-v", "PORT"]
        self.assertTrue(self.yang.validate(argument))
        output = self.run_script(argument)
+        self.maxDiff = None


same question as above

Removed this change

ganglyu · 2024-01-28T01:02:22Z

src/sonic-config-engine/tests/test_multinpu_cfggen.py

        output = self.run_script(argument)
        self.assertEqual(
            utils.liststr_to_dict(output.strip()),
+


No need to add this line

ganglyu · 2024-01-29T04:55:27Z

src/sonic-config-engine/tests/test_cfggen.py

        )

    def test_minigraph_neighbor_interfaces(self):
        argument = ['-m', self.sample_graph_simple_case, '-p', self.port_config, '-v', "PORT"]
        output = self.run_script(argument)
-
+        


Please remove space

ganglyu · 2024-01-29T04:55:41Z

src/sonic-config-engine/tests/test_cfggen.py

-                "'Ethernet116': {'alias': 'fortyGigE0/116', 'pfc_asym': 'off', 'lanes': '93,94,95,96', 'description': 'fortyGigE0/116', 'mtu': '9100', 'tpid': '0x8100', 'fec': 'rs', 'speed': '100000'}, "
-                "'Ethernet120': {'alias': 'fortyGigE0/120', 'pfc_asym': 'off', 'lanes': '97,98,99,100', 'description': 'fortyGigE0/120', 'mtu': '9100', 'tpid': '0x8100', 'fec': 'rs', 'speed': '100000'}, "
-                "'Ethernet124': {'alias': 'fortyGigE0/124', 'pfc_asym': 'off', 'lanes': '101,102,103,104', 'description': 'fortyGigE0/124', 'mtu': '9100', 'tpid': '0x8100', 'fec': 'rs', 'speed': '100000'}}"
+            utils.to_dict( 


space at the end of line

ganglyu · 2024-01-29T04:56:58Z

src/sonic-config-engine/tests/test_multinpu_cfggen.py

-                {'PortChannel0002': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
-                 'PortChannel4001': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'},
-                 'PortChannel4002': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'lacp_key': 'auto'}})
+                {'PortChannel0002': {'admin_status': 'up', 'min_links': '2', 'mtu': '9100', 'tpid': '0x8100', 'mode': 'routed', 'lacp_key': 'auto' },


Space after 'auto'

ridahanif96 · 2024-01-29T09:34:23Z

@venkatmahalingam can you please help review this PR, thanks

ridahanif96 · 2024-02-14T14:38:57Z

@qiluo-msft can you pls help merge this PR. Thanks

ridahanif96 · 2024-02-16T12:58:30Z

@qiluo-msft Can you please help merge this PR! Thanks

ridahanif96 · 2024-03-13T13:46:47Z

@qiluo-msft @zhangyanzhao can you please help to merge this PR, thanks.

ridahanif96 · 2024-03-21T17:41:11Z

@qiluo-msft can you also please help review this one, meanwhile i am fixing with other default disruptive change.

qiluo-msft · 2024-03-21T20:24:10Z

src/sonic-config-engine/minigraph.py

    for port in ports.values():
        port['mtu'] = '9100'
        port['tpid'] = '0x8100'
+    # mode check for vlan membership in PORT table default mode is routed and for vlan membership it is trunk


mode check for vlan membership in PORT table default mode is routed and for vlan membership it is trunk

Since you can implement a conditional default value in relevant consuming component, no need to add explicit attribute during load_minigraph. We try to prevent any minigraph.py behavior change now. #Closed

Here in minigraph .py, we implemented conditional default value in relevant consuming component.
Do we need to remove all this conditional change as well from minigraph.py

qiluo-msft

Block any minigraph.py behavior change. Feel free to reach me if any questions.

ridahanif96 · 2024-03-22T08:40:42Z

Block any minigraph.py behavior change. Feel free to reach me if any questions.

We have added conditional change in minigraph.py and relevant changes in other files, do we need to also remove this conditional change in minigraph.py

qiluo-msft · 2024-03-27T20:13:56Z

src/sonic-config-engine/minigraph.py

        port['mtu'] = '9100'
        port['tpid'] = '0x8100'
-
    # asymmetric PFC is disabled by default


you do not need remove empty line.

Unblocked. However I see PR checkers fail. Is this PR still needed?

qiluo-msft · 2024-05-14T06:45:04Z

@ridahanif96 will double check and close this PR.

ridahanif96 · 2024-05-14T07:46:29Z

@ridahanif96 will double check and close this PR.

This PR is not required any more as we will not be using sonic-config-Engine changes for Switchport Modes Port HLD. Therefore i am closing this PR.

ridahanif96 added 7 commits December 28, 2023 08:07

Fix for Elastic test

ada3e18

Switchport config-engine changes

b60bbbc

Fix for platform output

e3408f8

Fix for failures

78c97cb

Fixutres

61038fa

Fix for failure

6698bcc

Fix for Failures

57ec25f

ridahanif96 mentioned this pull request Dec 30, 2023

Switchport Modes Port & Port Channel Yang Model Configurations #13580

Merged

sabakram mentioned this pull request Jan 11, 2024

Fix for Switch Port Modes and VLAN CLI Enhancement sonic-net/sonic-utilities#3108

Merged

zhangyanzhao requested review from ganglyu and venkatmahalingam January 22, 2024 00:14

Rerun pipelines

d728e19

ganglyu reviewed Jan 28, 2024

View reviewed changes

Fixing spaces and other issues

32f8696

ganglyu reviewed Jan 29, 2024

View reviewed changes

Unnessary spaces removed

e3f034f

ganglyu approved these changes Jan 29, 2024

View reviewed changes

venkatmahalingam approved these changes Feb 14, 2024

View reviewed changes

zhangyanzhao added the YANG YANG model related changes label Mar 19, 2024

qiluo-msft reviewed Mar 21, 2024

View reviewed changes

qiluo-msft previously requested changes Mar 21, 2024

View reviewed changes

Fix for Vlan membership check in mingraph related files

872e12d

qiluo-msft reviewed Mar 27, 2024

View reviewed changes

ridahanif96 closed this May 14, 2024

Conversation

ridahanif96 commented Dec 28, 2023

Why I did it

How I did it

How to verify it

Description for the changelog

Link to config_db schema for YANG module changes

Uh oh!

ganglyu commented Jan 22, 2024

Uh oh!

ridahanif96 commented Jan 22, 2024

Uh oh!

qiluo-msft commented Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ridahanif96 commented Jan 26, 2024

Uh oh!

ridahanif96 commented Jan 27, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

qiluo-msft commented Jan 26, 2024 •

edited

Loading