From d71e89fb3c1ba83479a5cc83455d27f5869e3051 Mon Sep 17 00:00:00 2001
From: Sudharsan Dhamal Gopalarathnam <dgsudharsan@users.noreply.github.com>
Date: Tue, 25 May 2021 09:07:32 -0700
Subject: [PATCH] FEC none config through minigraph (#7667)

When FECDisabled is set to true in minigraph.py, push 'fec' 'none' explicitly to config_db. When 'fec' is defined in port_config.ini do not override it with 'rs' for 100G
---
 src/sonic-config-engine/minigraph.py         |  6 ++++--
 src/sonic-config-engine/tests/test_cfggen.py | 10 +++++-----
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py
index 3bbf4095a32f..66974d45e41b 100644
--- a/src/sonic-config-engine/minigraph.py
+++ b/src/sonic-config-engine/minigraph.py
@@ -1410,9 +1410,11 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
     for port_name, port in list(ports.items()):
         # get port alias from port_config.ini
         alias = port.get('alias', port_name)
-        # generate default 100G FEC
+        # generate default 100G FEC only if FECDisabled is not true and 'fec' is not defined in port_config.ini
         # Note: FECDisabled only be effective on 100G port right now
-        if port.get('speed') == '100000' and linkmetas.get(alias, {}).get('FECDisabled', '').lower() != 'true':
+        if linkmetas.get(alias, {}).get('FECDisabled', '').lower() == 'true':
+            port['fec'] = 'none'
+        elif not port.get('fec') and port.get('speed') == '100000':
             port['fec'] = 'rs'
 
         # If AutoNegotiation is available in the minigraph, we override any value we may have received from port_config.ini
diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py
index 69da794ca5a0..4d0595270071 100644
--- a/src/sonic-config-engine/tests/test_cfggen.py
+++ b/src/sonic-config-engine/tests/test_cfggen.py
@@ -342,7 +342,7 @@ def test_minigraph_port_fec_disabled(self):
         output = self.run_script(argument)
         self.assertEqual(
             utils.to_dict(output.strip()),
-            utils.to_dict("{'lanes': '25,26,27,28', 'description': 'Servers0:eth0', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/4', 'admin_status': 'up', 'speed': '100000', 'autoneg': 'on'}")
+            utils.to_dict("{'lanes': '25,26,27,28', 'description': 'Servers0:eth0', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/4', 'admin_status': 'up', 'speed': '100000', 'autoneg': 'on', 'fec': 'none'}")
         )
 
     def test_minigraph_port_autonegotiation(self):
@@ -353,8 +353,8 @@ def test_minigraph_port_autonegotiation(self):
             utils.to_dict(output.strip()),
             utils.to_dict(
                 "{'Ethernet0': {'alias': 'fortyGigE0/0', 'pfc_asym': 'off', 'lanes': '29,30,31,32', 'description': 'fortyGigE0/0', 'mtu': '9100'}, "
-                "'Ethernet4': {'lanes': '25,26,27,28', 'description': 'Servers0:eth0', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/4', 'admin_status': 'up', 'autoneg': 'on', 'speed': '100000'}, "
-                "'Ethernet8': {'lanes': '37,38,39,40', 'description': 'fortyGigE0/8', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/8', 'admin_status': 'up', 'autoneg': 'off'}, "
+                "'Ethernet4': {'lanes': '25,26,27,28', 'description': 'Servers0:eth0', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/4', 'admin_status': 'up', 'autoneg': 'on', 'speed': '100000', 'fec': 'none'}, "
+                "'Ethernet8': {'lanes': '37,38,39,40', 'description': 'fortyGigE0/8', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/8', 'admin_status': 'up', 'autoneg': 'off', 'fec': 'none'}, "
                 "'Ethernet12': {'lanes': '33,34,35,36', 'description': 'fortyGigE0/12', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/12', 'admin_status': 'up'}, "
                 "'Ethernet16': {'lanes': '41,42,43,44', 'description': 'fortyGigE0/16', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/16', 'admin_status': 'up'}, "
                 "'Ethernet20': {'lanes': '45,46,47,48', 'description': 'fortyGigE0/20', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/20', 'admin_status': 'up'}, "
@@ -394,8 +394,8 @@ def test_minigraph_port_autonegotiation(self):
             utils.to_dict(output.strip()),
             utils.to_dict(
                 "{'Ethernet0': {'lanes': '29,30,31,32', 'description': 'fortyGigE0/0', 'mtu': '9100', 'alias': 'fortyGigE0/0', 'pfc_asym': 'off', 'autoneg': 'off'}, "
-                "'Ethernet4': {'lanes': '25,26,27,28', 'description': 'Servers0:eth0', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/4', 'admin_status': 'up', 'autoneg': 'on', 'speed': '100000'}, "
-                "'Ethernet8': {'lanes': '37,38,39,40', 'description': 'fortyGigE0/8', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/8', 'admin_status': 'up', 'autoneg': 'off'}, "
+                "'Ethernet4': {'lanes': '25,26,27,28', 'description': 'Servers0:eth0', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/4', 'admin_status': 'up', 'autoneg': 'on', 'speed': '100000', 'fec': 'none'}, "
+                "'Ethernet8': {'lanes': '37,38,39,40', 'description': 'fortyGigE0/8', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/8', 'admin_status': 'up', 'autoneg': 'off', 'fec': 'none'}, "
                 "'Ethernet12': {'lanes': '33,34,35,36', 'description': 'fortyGigE0/12', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/12', 'admin_status': 'up', 'autoneg': 'off'}, "
                 "'Ethernet16': {'lanes': '41,42,43,44', 'description': 'fortyGigE0/16', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/16', 'admin_status': 'up', 'autoneg': 'off'}, "
                 "'Ethernet20': {'lanes': '45,46,47,48', 'description': 'fortyGigE0/20', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/20', 'admin_status': 'up', 'autoneg': 'off'}, "