Skip to content

Routed subinterface enhancements #1821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 118 additions & 1 deletion config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from portconfig import get_child_ports
from socket import AF_INET, AF_INET6
from sonic_py_common import device_info, multi_asic
from sonic_py_common.interface import get_interface_table_name, get_port_table_name
from sonic_py_common.interface import get_interface_table_name, get_port_table_name, get_intf_longname
from utilities_common import util_base
from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector
from utilities_common.db import Db
Expand Down Expand Up @@ -5971,6 +5971,123 @@ def smoothing_interval(interval, rates_type):
helper = util_base.UtilHelper()
helper.load_and_register_plugins(plugins, config)

#
# 'subinterface' group ('config subinterface ...')
#
@config.group()
@click.pass_context
@click.option('-s', '--redis-unix-socket-path', help='unix socket path for redis connection')
def subinterface(ctx, redis_unix_socket_path):
"""subinterface-related configuration tasks"""
kwargs = {}
if redis_unix_socket_path:
kwargs['unix_socket_path'] = redis_unix_socket_path
config_db = ConfigDBConnector(**kwargs)
config_db.connect(wait_for_init=False)
ctx.obj = {'db': config_db}

def subintf_vlan_check(config_db, parent_intf, vlan):
subintf_db = config_db.get_table('VLAN_SUB_INTERFACE')
subintf_names = [k for k in subintf_db if type(k) != tuple]
for subintf in subintf_names:
sub_intf_sep_idx = subintf.find(VLAN_SUB_INTERFACE_SEPARATOR)
if sub_intf_sep_idx == -1:
continue
if parent_intf == subintf[:sub_intf_sep_idx]:
Copy link
Contributor

@venkatmahalingam venkatmahalingam Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please put these new Click commands in this PR description for easy reference? also, hope you are planning to update the Click config guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if 'vlan' in subintf_db[subintf]:
if str(vlan) == subintf_db[subintf]['vlan']:
return True
else:
vlan_id = subintf[sub_intf_sep_idx + 1:]
if str(vlan) == vlan_id:
return True
return False

@subinterface.command('add')
@click.argument('subinterface_name', metavar='<subinterface_name>', required=True)
@click.argument('vid', metavar='<vid>', required=False, type=click.IntRange(1,4094))
@click.pass_context
def add_subinterface(ctx, subinterface_name, vid):
sub_intf_sep_idx = subinterface_name.find(VLAN_SUB_INTERFACE_SEPARATOR)
if sub_intf_sep_idx == -1:
ctx.fail("{} is invalid vlan subinterface".format(subinterface_name))

interface_alias = subinterface_name[:sub_intf_sep_idx]
if interface_alias is None:
ctx.fail("{} invalid subinterface".format(interface_alias))

if interface_alias.startswith("Po") is True:
intf_table_name = CFG_PORTCHANNEL_PREFIX
elif interface_alias.startswith("Eth") is True:
intf_table_name = 'PORT'

config_db = ctx.obj['db']
port_dict = config_db.get_table(intf_table_name)
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():
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):
ctx.fail("{} is configured as a member of portchannel. Cannot configure subinterface"
.format(interface_alias))

# 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):
ctx.fail("{} is configured as a member of vlan. Cannot configure subinterface"
.format(interface_alias))

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:
ctx.fail("{} already exists".format(subinterface_name))

subintf_dict = {}
if vid is not None:
subintf_dict.update({"vlan" : vid})

if subintf_vlan_check(config_db, get_intf_longname(interface_alias), vid) is True:
ctx.fail("Vlan {} encap already configured on other subinterface on {}".format(vid, interface_alias))

subintf_dict.update({"admin_status" : "up"})
config_db.set_entry('VLAN_SUB_INTERFACE', subinterface_name, subintf_dict)

@subinterface.command('del')
@click.argument('subinterface_name', metavar='<subinterface_name>', required=True)
@click.pass_context
def del_subinterface(ctx, subinterface_name):
sub_intf_sep_idx = subinterface_name.find(VLAN_SUB_INTERFACE_SEPARATOR)
if sub_intf_sep_idx == -1:
ctx.fail("{} is invalid vlan subinterface".format(subinterface_name))

config_db = ctx.obj['db']
#subinterface_name = subintf_get_shortname(subinterface_name)
if interface_name_is_valid(config_db, subinterface_name) is False:
ctx.fail("{} is invalid ".format(subinterface_name))

subintf_config_db = config_db.get_table('VLAN_SUB_INTERFACE')
sub_intfs = [k for k,v in subintf_config_db.items() if type(k) != tuple]
if subinterface_name not in sub_intfs:
ctx.fail("{} does not exists".format(subinterface_name))

ips = {}
ips = [ k[1] for k in config_db.get_table('VLAN_SUB_INTERFACE') if type(k) == tuple and k[0] == subinterface_name ]
for ip in ips:
try:
ipaddress.ip_network(ip, strict=False)
config_db.set_entry('VLAN_SUB_INTERFACE', (subinterface_name, ip), None)
except ValueError:
ctx.fail("Invalid ip {} found on interface {}".format(ip, subinterface_name))

subintf_config_db = config_db.get_table('INTERFACE')
ips = [ k[1] for k in subintf_config_db if type(k) == tuple and k[0] == subinterface_name ]
for ip in ips:
config_db.set_entry('INTERFACE', (subinterface_name, ip), None)

config_db.set_entry('VLAN_SUB_INTERFACE', subinterface_name, None)

if __name__ == '__main__':
config()
58 changes: 58 additions & 0 deletions doc/Command-Reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@
* [Startup Configuration](#startup-configuration)
* [Running Configuration](#running-configuration)
* [Static routing](#static-routing)
* [Subinterfaces](#subinterfaces)
* [Subinterfaces Show Commands](#subinterfaces-show-commands)
* [Subinterfaces Config Commands](#subinterfaces-config-commands)
* [Syslog](#syslog)
* [Syslog config commands](#syslog-config-commands)
* [System State](#system-state)
Expand Down Expand Up @@ -391,6 +394,7 @@ This command displays the full list of show commands available in the software;
runningconfiguration Show current running configuration...
services Show all daemon services
startupconfiguration Show startup configuration information
subinterfaces Show details of the sub port interfaces
system-memory Show memory information
tacacs Show TACACS+ configuration
techsupport Gather information for troubleshooting
Expand Down Expand Up @@ -8128,6 +8132,60 @@ This sub-section explains of command is used to show current routes.

Go Back To [Beginning of the document](#) or [Beginning of this section](#static-routing)

## Subinterfaces

### Subinterfaces Show Commands

**show subinterfaces status**

This command displays all the subinterfaces that are configured on the device and its current status.

- Usage:
```
show subinterfaces status
```

- Example:
```
admin@sonic:~$ show subinterfaces status
Sub port interface Speed MTU Vlan Admin Type
------------------ ------- ----- ------ ------- -------------------
Eth64.10 100G 9100 100 up dot1q-encapsulation
Ethernet0.100 100G 9100 100 up dot1q-encapsulation
```

### Subinterfaces Config Commands

This sub-section explains how to configure subinterfaces.

**config subinterface**

- Usage:
```
config subinterface (add | del) <subinterface_name> [vlan <1-4094>]
```

- Example (Create the subinterfces with name "Ethernet0.100"):
```
admin@sonic:~$ sudo config subinterface add Ethernet0.100
```

- Example (Create the subinterfces with name "Eth64.100"):
```
admin@sonic:~$ sudo config subinterface add Eth64.100 100
```

- Example (Delete the subinterfces with name "Ethernet0.100"):
```
admin@sonic:~$ sudo config subinterface del Ethernet0.100
```

- Example (Delete the subinterfces with name "Eth64.100"):
```
admin@sonic:~$ sudo config subinterface del Eth64.100 100
```

Go Back To [Beginning of the document](#) or [Beginning of this section](#static-routing)

## Syslog

Expand Down
5 changes: 3 additions & 2 deletions scripts/intfutil
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ from tabulate import tabulate
from utilities_common import constants
from utilities_common import multi_asic as multi_asic_util
from utilities_common.intf_filter import parse_interface_in_filter
from sonic_py_common.interface import get_intf_longname

# mock the redis for unit test purposes #
try:
Expand Down Expand Up @@ -313,12 +314,12 @@ def appl_db_portchannel_status_get(appl_db, config_db, po_name, status_type, por
def appl_db_sub_intf_status_get(appl_db, config_db, front_panel_ports_list, portchannel_speed_dict, sub_intf_name, status_type):
sub_intf_sep_idx = sub_intf_name.find(VLAN_SUB_INTERFACE_SEPARATOR)
if sub_intf_sep_idx != -1:
parent_port_name = sub_intf_name[:sub_intf_sep_idx]
vlan_id = sub_intf_name[sub_intf_sep_idx + 1:]
parent_port_name = get_intf_longname(sub_intf_name[:sub_intf_sep_idx])

full_intf_table_name = "INTF_TABLE" + ":" + sub_intf_name

if status_type == "vlan":
vlan_id = appl_db.get(appl_db.APPL_DB, full_intf_table_name, status_type)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where can we find DB migrator change to populate vlan_id field from sub-interface name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per updated design, intfmgrd supports both short name and long name format subinterfaces.
In long name format, vlan_id is derived from subinterface index by intfmgrd.
In short name format, vlan_id is mandatory parameter to be configured by user(as part of click cli).
With this design we do not need db_migrator changes.

return vlan_id

if status_type == "admin_status":
Expand Down
15 changes: 15 additions & 0 deletions tests/intfutil_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,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"
" Ethernet0.10 25G 9100 10 up 802.1q-encapsulation"
)
self.assertEqual(result.output.strip(), expected_output)
Expand Down Expand Up @@ -218,13 +219,27 @@ def test_single_subintf_status(self):
print(output, file=sys.stderr)
self.assertEqual(output.strip(), expected_output)

expected_output = (
"Sub port interface Speed MTU Vlan Admin Type\n"
"-------------------- ------- ----- ------ ------- --------------------\n"
" Eth32.10 40G 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)
print(output, file=sys.stderr)
self.assertEqual(output.strip(), expected_output)

# Test '--verbose' status of single sub interface
def test_single_subintf_status_verbose(self):
result = self.runner.invoke(show.cli.commands["subinterfaces"].commands["status"], ["Ethernet0.10", "--verbose"])
print(result.output, file=sys.stderr)
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"])
print(result.output, file=sys.stderr)
expected_output = "Command: intfutil -c status -i Eth32.10"
self.assertEqual(result.output.split('\n')[0], expected_output)

# Test status of single sub interface in alias naming mode
def test_single_subintf_status_alias_mode(self):
Expand Down
7 changes: 6 additions & 1 deletion tests/mock_tables/appl_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,12 @@
"speed": "100000"
},
"INTF_TABLE:Ethernet0.10": {
"admin_status": "up"
"admin_status": "up",
"vlan": "10"
},
"INTF_TABLE:Eth32.10": {
"admin_status": "up",
"vlan": "100"
},
"_GEARBOX_TABLE:phy:1": {
"name": "sesto-1",
Expand Down
4 changes: 4 additions & 0 deletions tests/mock_tables/config_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,10 @@
"VLAN_SUB_INTERFACE|Ethernet0.10": {
"admin_status": "up"
},
"VLAN_SUB_INTERFACE|Eth32.10": {
"admin_status": "up",
"vlan": "100"
},
"ACL_RULE|NULL_ROUTE_V4|DEFAULT_RULE": {
"PACKET_ACTION": "DROP",
"PRIORITY": "1"
Expand Down