Skip to content
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.

Sign up for GitHub

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

[config] Add support of PortChannels to portconfig #1375

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

[config] Add support of PortChannels to portconfig #1375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
46485f9
[config] Add support of PortChannels to portconfig
Jan 21, 2021
ca3c2c7
[portconfig] Refactor portconfig script
Jan 21, 2021
4a35666
[config][portconfig][unit tests] Add mocking of redis to portconfig. …
Feb 19, 2021
1f44432
[config][unit tests] Add unit tests for subcommand 'config intf mtu'
Feb 19, 2021
cfb23f6
[portconfig] Refactor portconfig script after rebase
NGorb-jabil Oct 21, 2021
6d8387c
[portconfig] Delete unused import
KonstiantynHalushka Nov 9, 2021
ed55f2c
Merge branch 'master' into fix_portconfig_portchannel
KonstiantynHalushka Nov 9, 2021
e063eca
Revert "[portconfig] Delete unused import"
Nov 9, 2021
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
18 changes: 11 additions & 7 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3638,14 +3638,18 @@ def mtu(ctx, interface_name, interface_mtu, verbose):
"""Set interface mtu"""
# Get the config_db connector
config_db = ctx.obj['config_db']
if clicommon.get_interface_naming_mode() == "alias":
interface_name = interface_alias_to_name(config_db, interface_name)
if interface_name is None:
ctx.fail("'interface_name' is None!")

portchannel_member_table = config_db.get_table('PORTCHANNEL_MEMBER')
if interface_is_in_portchannel(portchannel_member_table, interface_name):
ctx.fail("'interface_name' is in portchannel!")
if not interface_name.startswith("PortChannel"):
# The checks are only for Ethernet interfaces and their aliases
if clicommon.get_interface_naming_mode() == "alias":
name_from_alias = interface_alias_to_name(config_db, interface_name)
if interface_name is None or name_from_alias == interface_name:
Copy link
Contributor

Choose a reason for hiding this comment

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

With this added check you are requiring that once the device is configured with naming_mode alias it must use the alias name and not the SONiC interface name and it will cause this to fail? Can we preserve the original design/check and not treat the case where the look up resulted to the same name as a failure scenario?

ctx.fail(f"Failed to convert alias {interface_name} to interface name. Result: {str(interface_name)}")
interface_name = name_from_alias

portchannel_member_table = config_db.get_table('PORTCHANNEL_MEMBER')
if interface_is_in_portchannel(portchannel_member_table, interface_name):
ctx.fail(f"{interface_name} is in portchannel!")

if ctx.obj['namespace'] is DEFAULT_NAMESPACE:
command = "portconfig -p {} -m {}".format(interface_name, interface_mtu)
Expand Down
73 changes: 41 additions & 32 deletions scripts/portconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env python3
#!/usr/bin/python3

"""
portconfig is the utility to show and change ECN configuration
Expand All @@ -25,6 +25,7 @@ optional arguments:
import os
import sys
import argparse
from sonic_py_common import interface

# mock the redis for unit test purposes #
try:
Expand All @@ -34,6 +35,9 @@ try:
sys.path.insert(0, modules_path)
sys.path.insert(0, test_path)
import mock_tables.dbconnector
if os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] == "multi_asic":
import mock_tables.mock_multi_asic
mock_tables.dbconnector.load_namespace_config()
except KeyError:
pass

Expand Down Expand Up @@ -67,7 +71,7 @@ VALID_INTERFACE_TYPE_SET = set(['CR','CR2','CR4','SR','SR2','SR4',

class portconfig(object):
"""
Process aclstat
Process portconfig
"""
def __init__(self, verbose, port, namespace):
self.verbose = verbose
Expand Down Expand Up @@ -100,13 +104,13 @@ class portconfig(object):
else:
raise Exception("Invalid port %s" % (port))

def list_params(self, port):
def list_params(self, table_name, port):
# chack whether table for this port exists
port_tables = self.db.get_table(PORT_TABLE_NAME)
port_tables = self.db.get_table(table_name)
if port in port_tables:
print(port_tables[port])

def set_speed(self, port, speed):
def set_speed(self, table_name, port, speed):
if self.verbose:
print("Setting speed %s on port %s" % (speed, port))
supported_speeds_str = self.get_supported_speeds(port)
Expand All @@ -115,25 +119,25 @@ class portconfig(object):
print('Invalid speed specified: {}'.format(speed))
print('Valid speeds:{}'.format(supported_speeds_str))
exit(1)
self.db.mod_entry(PORT_TABLE_NAME, port, {PORT_SPEED_CONFIG_FIELD_NAME: speed})
self.db.mod_entry(table_name, port, {PORT_SPEED_CONFIG_FIELD_NAME: speed})

def set_fec(self, port, fec):
def set_fec(self, table_name, port, fec):
if self.verbose:
print("Setting fec %s on port %s" % (fec, port))
self.db.mod_entry(PORT_TABLE_NAME, port, {PORT_FEC_CONFIG_FIELD_NAME: fec})
self.db.mod_entry(table_name, port, {PORT_FEC_CONFIG_FIELD_NAME: fec})

def set_mtu(self, port, mtu):
def set_mtu(self, table_name, port, mtu):
if self.verbose:
print("Setting mtu %s on port %s" % (mtu, port))
self.db.mod_entry(PORT_TABLE_NAME, port, {PORT_MTU_CONFIG_FIELD_NAME: mtu})
self.db.mod_entry(table_name, port, {PORT_MTU_CONFIG_FIELD_NAME: mtu})

def set_autoneg(self, port, mode):
def set_autoneg(self, table_name, port, mode):
if self.verbose:
print("Setting autoneg %s on port %s" % (mode, port))
mode = 'on' if mode == 'enabled' else 'off'
self.db.mod_entry(PORT_TABLE_NAME, port, {PORT_AUTONEG_CONFIG_FIELD_NAME: mode})
self.db.mod_entry(table_name, port, {PORT_AUTONEG_CONFIG_FIELD_NAME: mode})

def set_adv_speeds(self, port, adv_speeds):
def set_adv_speeds(self, table_name, port, adv_speeds):
if self.verbose:
print("Setting adv_speeds %s on port %s" % (adv_speeds, port))

Expand All @@ -152,18 +156,18 @@ class portconfig(object):
print('Valid speeds:{}'.format(supported_speeds_str))
exit(1)

self.db.mod_entry(PORT_TABLE_NAME, port, {PORT_ADV_SPEEDS_CONFIG_FIELD_NAME: adv_speeds})
self.db.mod_entry(table_name, port, {PORT_ADV_SPEEDS_CONFIG_FIELD_NAME: adv_speeds})

def set_interface_type(self, port, interface_type):
def set_interface_type(self, table_name, port, interface_type):
if self.verbose:
print("Setting interface_type %s on port %s" % (interface_type, port))
if interface_type not in VALID_INTERFACE_TYPE_SET:
print("Invalid interface type specified: {}".format(interface_type))
print("Valid interface types:{}".format(','.join(VALID_INTERFACE_TYPE_SET)))
exit(1)
self.db.mod_entry(PORT_TABLE_NAME, port, {PORT_INTERFACE_TYPE_CONFIG_FIELD_NAME: interface_type})
self.db.mod_entry(table_name, port, {PORT_INTERFACE_TYPE_CONFIG_FIELD_NAME: interface_type})

def set_adv_interface_types(self, port, adv_interface_types):
def set_adv_interface_types(self, table_name, port, adv_interface_types):
if self.verbose:
print("Setting adv_interface_types %s on port %s" % (adv_interface_types, port))

Expand All @@ -178,7 +182,8 @@ class portconfig(object):
print("Invalid interface type specified: {}".format(','.join(invalid_interface_types)))
print("Valid interface types:{}".format(','.join(VALID_INTERFACE_TYPE_SET)))
exit(1)
self.db.mod_entry(PORT_TABLE_NAME, port, {PORT_ADV_INTERFACE_TYPES_CONFIG_FIELD_NAME: adv_interface_types})

self.db.mod_entry(table_name, port, {PORT_ADV_INTERFACE_TYPES_CONFIG_FIELD_NAME: adv_interface_types})

def get_supported_speeds(self, port):
if not self.namespace:
Expand All @@ -188,7 +193,7 @@ class portconfig(object):
state_db.connect(state_db.STATE_DB)
return state_db.get(state_db.STATE_DB, '{}|{}'.format(PORT_STATE_TABLE_NAME, port), PORT_STATE_SUPPORTED_SPEEDS)

def set_tpid(self, port, tpid):
def set_tpid(self, table_name, port, tpid):
if self.verbose:
print("Setting tpid %s on port %s" % (tpid, port))

Expand Down Expand Up @@ -223,7 +228,7 @@ class portconfig(object):
raise Exception("%s is already member of %s. Set TPID NOT allowed." % (port, self.parent))
else:
if tpid_port_capable:
self.db.mod_entry(PORT_TABLE_NAME, port, {TPID_CONFIG_FIELD_NAME: tpid})
self.db.mod_entry(table_name, port, {TPID_CONFIG_FIELD_NAME: tpid})
else:
raise Exception("HW is not capable to support Port TPID config.")
else:
Expand All @@ -236,15 +241,15 @@ def main():
parser = argparse.ArgumentParser(description='Set SONiC port parameters',
formatter_class=argparse.RawTextHelpFormatter)
parser.add_argument('-p', '--port', type=str, help='port name (e.g. Ethernet0)', required=True, default=None)
parser.add_argument('-l', '--list', action='store_true', help='list port parametars', default=False)
parser.add_argument('-l', '--list', action='store_true', help='list port parameters', default=False)
parser.add_argument('-s', '--speed', type=int, help='port speed value in Mbit', default=None)
parser.add_argument('-f', '--fec', type=str, help='port fec mode value in (none, rs, fc)', default=None)
parser.add_argument('-m', '--mtu', type=int, help='port mtu value in bytes', default=None)
parser.add_argument('-tp', '--tpid', type=str, help='port TPID value in hex (e.g. 0x8100)', default=None)
parser.add_argument('-v', '--version', action='version', version='%(prog)s 1.0')
parser.add_argument('-vv', '--verbose', action='store_true', help='Verbose output', default=False)
parser.add_argument('-n', '--namespace', metavar='namespace details', type = str, required = False,
help = 'The asic namespace whose DB instance we need to connect', default=None)
parser.add_argument('-n', '--namespace', metavar='namespace details', type=str, required=False,
help='The asic namespace whose DB instance we need to connect', default=None)
parser.add_argument('-an', '--autoneg', type = str, required = False,
help = 'port auto negotiation mode', default=None)
parser.add_argument('-S', '--adv-speeds', type = str, required = False,
Expand All @@ -258,26 +263,30 @@ def main():
# Load database config files
load_db_config()
try:
port_table_name = interface.get_port_table_name(args.port)
Copy link
Contributor

Choose a reason for hiding this comment

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

have you tested this with device set to naming_mode as "alias" and if you passed the alias name instead would this still work? I think we should move the naming mode check to here instead of handled at set_mtu only so that it should be handling the alias case in case naming_mode is set to alias?

if not port_table_name:
raise Exception("Invalid port type specified")

port = portconfig(args.verbose, args.port, args.namespace)
if args.list:
port.list_params(args.port)
port.list_params(port_table_name, args.port)
elif args.speed or args.fec or args.mtu or args.autoneg or args.adv_speeds or args.interface_type or args.adv_interface_types or args.tpid:
if args.speed:
port.set_speed(args.port, args.speed)
port.set_speed(port_table_name, args.port, args.speed)
if args.fec:
port.set_fec(args.port, args.fec)
port.set_fec(port_table_name, args.port, args.fec)
if args.mtu:
port.set_mtu(args.port, args.mtu)
port.set_mtu(port_table_name, args.port, args.mtu)
if args.autoneg:
port.set_autoneg(args.port, args.autoneg)
port.set_autoneg(port_table_name, args.port, args.autoneg)
if args.adv_speeds:
port.set_adv_speeds(args.port, args.adv_speeds)
port.set_adv_speeds(port_table_name, args.port, args.adv_speeds)
if args.interface_type:
port.set_interface_type(args.port, args.interface_type)
port.set_interface_type(port_table_name, args.port, args.interface_type)
if args.adv_interface_types:
port.set_adv_interface_types(args.port, args.adv_interface_types)
port.set_adv_interface_types(port_table_name, args.port, args.adv_interface_types)
if args.tpid:
port.set_tpid(args.port, args.tpid)
port.set_tpid(port_table_name, args.port, args.tpid)
else:
parser.print_help()
sys.exit(1)
Expand Down
Loading