[subinterface]Added additional checks in portchannel and subinterface…

… commands (sonic-net#2345)

*Added additional checks in subinterface and portchannel commands so they don't conflict. Without the checks, a subinterface could be created on a portchannel member and vice versa which will lead to SAI failure followed by orchagent crash.

config/main.py

@@ -2113,6 +2113,14 @@ def add_portchannel_member(ctx, portchannel_name, port_name):
  ctx.fail(" {} has ip address configured".format(port_name))
  return
 
+ for key in db.get_keys('VLAN_SUB_INTERFACE'):
+ if type(key) == tuple:
+ continue
+ intf = key.split(VLAN_SUB_INTERFACE_SEPARATOR)[0]
+ parent_intf = get_intf_longname(intf)
+ if parent_intf == port_name:
+ ctx.fail(" {} has subinterfaces configured".format(port_name))
+
  # Dont allow a port to be member of port channel if it is configured as a VLAN member
  for k,v in db.get_table('VLAN_MEMBER'):
  if v == port_name:
@@ -6762,23 +6770,24 @@ def add_subinterface(ctx, subinterface_name, vid):
 
  config_db = ctx.obj['db']
  port_dict = config_db.get_table(intf_table_name)
+ parent_intf = get_intf_longname(interface_alias)
  if interface_alias is not None:
  if not port_dict:
  ctx.fail("{} parent interface not found. {} table none".format(interface_alias, intf_table_name))
- if get_intf_longname(interface_alias) not in port_dict.keys():
+ if parent_intf not in port_dict.keys():
  ctx.fail("{} parent interface not found".format(subinterface_name))
 
  # Validate if parent is portchannel member
  portchannel_member_table = config_db.get_table('PORTCHANNEL_MEMBER')
- if interface_is_in_portchannel(portchannel_member_table, interface_alias):
+ if interface_is_in_portchannel(portchannel_member_table, parent_intf):
  ctx.fail("{} is configured as a member of portchannel. Cannot configure subinterface"
- .format(interface_alias))
+ .format(parent_intf))
 
  # Validate if parent is vlan member
  vlan_member_table = config_db.get_table('VLAN_MEMBER')
- if interface_is_in_vlan(vlan_member_table, interface_alias):
+ if interface_is_in_vlan(vlan_member_table, parent_intf):
  ctx.fail("{} is configured as a member of vlan. Cannot configure subinterface"
- .format(interface_alias))
+ .format(parent_intf))
 
  sub_intfs = [k for k,v in config_db.get_table('VLAN_SUB_INTERFACE').items() if type(k) != tuple]
  if subinterface_name in sub_intfs:

tests/intfutil_test.py

@@ -209,7 +209,7 @@ def test_subintf_status(self):
  expected_output = (
  "Sub port interface Speed MTU Vlan Admin Type\n"
  "-------------------- ------- ----- ------ ------- --------------------\n"
- " Eth32.10 40G 9100 100 up 802.1q-encapsulation\n"
+ " Eth36.10 10M 9100 100 up 802.1q-encapsulation\n"
  " Ethernet0.10 25G 9100 10 up 802.1q-encapsulation\n"
  " Po0001.10 40G 9100 100 up 802.1q-encapsulation"
  )
@@ -248,10 +248,10 @@ def test_single_subintf_status(self):
  expected_output = (
  "Sub port interface Speed MTU Vlan Admin Type\n"
  "-------------------- ------- ----- ------ ------- --------------------\n"
- " Eth32.10 40G 9100 100 up 802.1q-encapsulation"
+ " Eth36.10 10M 9100 100 up 802.1q-encapsulation"
  )
- # Test 'intfutil status Eth32.10'
- output = subprocess.check_output('intfutil -c status -i Eth32.10', stderr=subprocess.STDOUT, shell=True, text=True)
+ # Test 'intfutil status Eth36.10'
+ output = subprocess.check_output('intfutil -c status -i Eth36.10', stderr=subprocess.STDOUT, shell=True, text=True)
  print(output, file=sys.stderr)
  self.assertEqual(output.strip(), expected_output)
 
@@ -272,9 +272,9 @@ def test_single_subintf_status_verbose(self):
  expected_output = "Command: intfutil -c status -i Ethernet0.10"
  self.assertEqual(result.output.split('\n')[0], expected_output)
 
- result = self.runner.invoke(show.cli.commands["subinterfaces"].commands["status"], ["Eth32.10", "--verbose"])
+ result = self.runner.invoke(show.cli.commands["subinterfaces"].commands["status"], ["Eth36.10", "--verbose"])
  print(result.output, file=sys.stderr)
- expected_output = "Command: intfutil -c status -i Eth32.10"
+ expected_output = "Command: intfutil -c status -i Eth36.10"
  self.assertEqual(result.output.split('\n')[0], expected_output)
 
  result = self.runner.invoke(show.cli.commands["subinterfaces"].commands["status"], ["Po0001.10", "--verbose"])

tests/ip_config_test.py

@@ -54,11 +54,11 @@ def test_add_del_interface_valid_ipv4(self):
  assert result.exit_code == 0
  assert ('Ethernet0.10', '10.11.10.1/24') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
 
- # config int ip add Eth32.10 32.11.10.1/24
- result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Eth32.10", "32.11.10.1/24"], obj=obj)
+ # config int ip add Eth36.10 32.11.10.1/24
+ result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Eth36.10", "32.11.10.1/24"], obj=obj)
  print(result.exit_code, result.output)
  assert result.exit_code == 0
- assert ('Eth32.10', '32.11.10.1/24') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+ assert ('Eth36.10', '32.11.10.1/24') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
 
  # config int ip remove Ethernet64 10.10.10.1/24
  result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet64", "10.10.10.1/24"], obj=obj)
@@ -72,11 +72,11 @@ def test_add_del_interface_valid_ipv4(self):
  assert result.exit_code != 0
  assert ('Ethernet0.10', '10.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
 
- # config int ip remove Eth32.10 32.11.10.1/24
- result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth32.10", "32.11.10.1/24"], obj=obj)
+ # config int ip remove Eth36.10 32.11.10.1/24
+ result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth36.10", "32.11.10.1/24"], obj=obj)
  print(result.exit_code, result.output)
  assert result.exit_code != 0
- assert ('Eth32.10', '32.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+ assert ('Eth36.10', '32.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
 
  def test_add_interface_invalid_ipv4(self):
  db = Db()
@@ -129,10 +129,10 @@ def test_add_del_interface_valid_ipv6(self):
  assert result.exit_code == 0
  assert ('Ethernet0.10', '1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
 
- result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Eth32.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
+ result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Eth36.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
  print(result.exit_code, result.output)
  assert result.exit_code == 0
- assert ('Eth32.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+ assert ('Eth36.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
 
  # config int ip remove Ethernet72 2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34
  result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
@@ -145,10 +145,10 @@ def test_add_del_interface_valid_ipv6(self):
  assert result.exit_code != 0
  assert ('Ethernet0.10', '1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
 
- result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth32.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
+ result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth36.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
  print(result.exit_code, result.output)
  assert result.exit_code != 0
- assert ('Eth32.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+ assert ('Eth36.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
 
  def test_del_interface_case_sensitive_ipv6(self):
  db = Db()

tests/loopback_action_test.py

@@ -7,7 +7,7 @@
 show_ip_interfaces_loopback_action_output="""\
 Interface Action
 --------------- --------
-Eth32.10 drop
+Eth36.10 drop
 Ethernet0 forward
 PortChannel0001 drop
 Vlan3000 forward

tests/mock_tables/appl_db.json

@@ -188,7 +188,7 @@
  "admin_status": "up",
  "vlan": "10"
  },
- "INTF_TABLE:Eth32.10": {
+ "INTF_TABLE:Eth36.10": {
  "admin_status": "up",
  "vrf_name": "Vrf1",
  "vlan": "100"
@@ -202,15 +202,15 @@
  "family": "IPv4",
  "scope": "global"
  },
- "INTF_TABLE:Eth32.10|32.10.11.12/24": {
+ "INTF_TABLE:Eth36.10|32.10.11.12/24": {
  "family": "IPv4",
  "scope": "global"
  },
  "INTF_TABLE:Po0001.10|10.10.11.12/24": {
  "family": "IPv4",
  "scope": "global"
  },
- "INTF_TABLE:Eth32.10|3210::12/126": {
+ "INTF_TABLE:Eth36.10|3210::12/126": {
  "family": "IPv6",
  "scope": "global"
  },

tests/mock_tables/config_db.json

@@ -376,16 +376,16 @@
  "VLAN_SUB_INTERFACE|Ethernet0.10|10.11.12.13/24": {
  "NULL" : "NULL"
  },
- "VLAN_SUB_INTERFACE|Eth32.10": {
+ "VLAN_SUB_INTERFACE|Eth36.10": {
  "admin_status": "up",
  "loopback_action": "drop",
  "vrf_name": "Vrf1",
  "vlan": "100"
  },
- "VLAN_SUB_INTERFACE|Eth32.10|32.10.11.12/24": {
+ "VLAN_SUB_INTERFACE|Eth36.10|32.10.11.12/24": {
  "NULL" : "NULL"
  },
- "VLAN_SUB_INTERFACE|Eth32.10|3210::12/126": {
+ "VLAN_SUB_INTERFACE|Eth36.10|3210::12/126": {
  "NULL" : "NULL"
  },
  "VLAN_SUB_INTERFACE|Po0001.10": {

tests/portchannel_test.py

@@ -147,6 +147,19 @@ def test_add_portchannel_member_which_has_ipaddress(self):
  assert result.exit_code != 0
  assert "Error: Ethernet0 has ip address configured" in result.output
 
+ def test_add_portchannel_member_which_has_subintf(self):
+ runner = CliRunner()
+ db = Db()
+ obj = {'db':db.cfgdb}
+
+ # add a portchannel member with port which has ip-address
+ result = runner.invoke(config.config.commands["portchannel"].commands["member"].commands["add"], ["PortChannel1001", "Ethernet36"], obj=obj)
+ print(result.exit_code)
+ print(result.output)
+ assert result.exit_code != 0
+ print(result.output)
+ assert "Error: Ethernet36 has subinterfaces configured" in result.output
+
  def test_add_portchannel_member_which_is_member_of_vlan(self):
  runner = CliRunner()
  db = Db()

tests/show_vrf_test.py

@@ -29,7 +29,7 @@ def test_vrf_show(self):
 Vrf101 Ethernet0.10
 Vrf102 PortChannel0002
  Vlan40
- Eth32.10
+ Eth36.10
 Vrf103 Ethernet4
  Loopback0
  Po0002.101
@@ -53,7 +53,7 @@ def test_vrf_bind_unbind(self):
 Vrf101 Ethernet0.10
 Vrf102 PortChannel0002
  Vlan40
- Eth32.10
+ Eth36.10
 Vrf103 Ethernet4
  Loopback0
  Po0002.101
@@ -86,10 +86,10 @@ def test_vrf_bind_unbind(self):
  assert result.exit_code == 0
  assert 'PortChannel002' not in db.cfgdb.get_table('PORTCHANNEL_INTERFACE')
 
- result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Eth32.10"], obj=obj)
+ result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Eth36.10"], obj=obj)
  print(result.exit_code, result.output)
  assert result.exit_code == 0
- assert ('vrf_name', 'Vrf102') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Eth32.10']
+ assert ('vrf_name', 'Vrf102') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Eth36.10']
 
  result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Ethernet0.10"], obj=obj)
  print(result.exit_code, result.output)
@@ -114,7 +114,7 @@ def test_vrf_bind_unbind(self):
 Vrf101 Ethernet0.10
 Vrf102 PortChannel0002
  Vlan40
- Eth32.10
+ Eth36.10
 Vrf103 Ethernet4
  Loopback0
  Po0002.101

tests/static_routes_test.py

@@ -403,16 +403,16 @@ def test_static_route_nexthop_subinterface(self):
  print(result.exit_code, result.output)
  assert not ('2.2.3.5/32') in db.cfgdb.get_table('STATIC_ROUTE')
 
- # config route add prefix 2.2.3.5/32 nexthop dev Eth32.10
+ # config route add prefix 2.2.3.5/32 nexthop dev Eth36.10
  result = runner.invoke(config.config.commands["route"].commands["add"], \
- ["prefix", "2.2.3.5/32", "nexthop", "dev", "Eth32.10"], obj=obj)
+ ["prefix", "2.2.3.5/32", "nexthop", "dev", "Eth36.10"], obj=obj)
  print(result.exit_code, result.output)
  assert ('2.2.3.5/32') in db.cfgdb.get_table('STATIC_ROUTE')
- assert db.cfgdb.get_entry('STATIC_ROUTE', '2.2.3.5/32') == {'nexthop': '', 'blackhole': 'false', 'distance': '0', 'ifname': 'Eth32.10', 'nexthop-vrf': ''}
+ assert db.cfgdb.get_entry('STATIC_ROUTE', '2.2.3.5/32') == {'nexthop': '', 'blackhole': 'false', 'distance': '0', 'ifname': 'Eth36.10', 'nexthop-vrf': ''}
 
- # config route del prefix 2.2.3.5/32 nexthop dev Eth32.10
+ # config route del prefix 2.2.3.5/32 nexthop dev Eth36.10
  result = runner.invoke(config.config.commands["route"].commands["del"], \
- ["prefix", "2.2.3.5/32", "nexthop", "dev", "Eth32.10"], obj=obj)
+ ["prefix", "2.2.3.5/32", "nexthop", "dev", "Eth36.10"], obj=obj)
  print(result.exit_code, result.output)
  assert not ('2.2.3.5/32') in db.cfgdb.get_table('STATIC_ROUTE')
 

tests/subintf_test.py

@@ -7,6 +7,12 @@
 import show.main as show
 from utilities_common.db import Db
 
+SUB_INTF_ON_LAG_MEMBER_ERR="""\
+Usage: add [OPTIONS] <subinterface_name> <vid>
+Try "add --help" for help.
+
+Error: Ethernet32 is configured as a member of portchannel. Cannot configure subinterface
+"""
 
 class TestSubinterface(object):
  @classmethod
@@ -141,6 +147,21 @@ def test_invalid_subintf_creation(self):
  print(result.exit_code, result.output)
  assert result.exit_code != 0
 
+ def test_subintf_creation_on_lag_member(self):
+ runner = CliRunner()
+ db = Db()
+ obj = {'db':db.cfgdb}
+
+ result = runner.invoke(config.config.commands["subinterface"].commands["add"], ["Ethernet32.10"], obj=obj)
+ print(result.exit_code, result.output)
+ assert result.exit_code != 0
+ assert(result.output == SUB_INTF_ON_LAG_MEMBER_ERR)
+
+ result = runner.invoke(config.config.commands["subinterface"].commands["add"], ["Eth32.20"], obj=obj)
+ print(result.exit_code, result.output)
+ assert result.exit_code != 0
+ assert(result.output == SUB_INTF_ON_LAG_MEMBER_ERR)
+
  def test_subintf_vrf_bind_unbind(self):
  runner = CliRunner()
  db = Db()

tests/vrf_input/config_db.json

@@ -3,7 +3,7 @@
  "vrf_name": "Vrf101",
  "admin_status": "up"
  },
- "VLAN_SUB_INTERFACE|Eth32.10": {
+ "VLAN_SUB_INTERFACE|Eth36.10": {
  "vrf_name": "Vrf102",
  "admin_status": "up",
  "vlan": "100"