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.

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

Validations checks while adding a member to PortChannel and removing a member from a Portchannel #1328

Merged
merged 8 commits into from
Jan 12, 2021
117 changes: 116 additions & 1 deletion config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@
CFG_LOOPBACK_ID_MAX_VAL = 999
CFG_LOOPBACK_NO="<0-999>"

CFG_PORTCHANNEL_PREFIX = "PortChannel"
CFG_PORTCHANNEL_PREFIX_LEN = 11
CFG_PORTCHANNEL_NAME_TOTAL_LEN_MAX = 15
CFG_PORTCHANNEL_MAX_VAL = 9999
CFG_PORTCHANNEL_NO="<0-9999>"

PORT_MTU = "mtu"
PORT_SPEED = "speed"

asic_type = None

Expand Down Expand Up @@ -390,6 +398,45 @@ def is_interface_bind_to_vrf(config_db, interface_name):
return True
return False

def is_portchannel_name_valid(portchannel_name):
"""Port channel name validation
"""

# Return True if Portchannel name is PortChannelXXXX (XXXX can be 0-9999)
if portchannel_name[:CFG_PORTCHANNEL_PREFIX_LEN] != CFG_PORTCHANNEL_PREFIX :
return False
if (portchannel_name[CFG_PORTCHANNEL_PREFIX_LEN:].isdigit() is False or
int(portchannel_name[CFG_PORTCHANNEL_PREFIX_LEN:]) > CFG_PORTCHANNEL_MAX_VAL) :
return False
if len(portchannel_name) > CFG_PORTCHANNEL_NAME_TOTAL_LEN_MAX:
return False
return True

def is_portchannel_present_in_db(db, portchannel_name):
"""Check if Portchannel is present in Config DB
"""

# Return True if Portchannel name exists in the CONFIG_DB
portchannel_list = db.get_table(CFG_PORTCHANNEL_PREFIX)
if portchannel_list is None:
return False
if portchannel_name in portchannel_list:
return True
return False

def is_port_member_of_this_portchannel(db, port_name, portchannel_name):
"""Check if a port is member of given portchannel
"""
portchannel_list = db.get_table(CFG_PORTCHANNEL_PREFIX)
if portchannel_list is None:
return False

for k,v in db.get_table('PORTCHANNEL_MEMBER'):
if (k == portchannel_name) and (v == port_name):
return True

return False

# Return the namespace where an interface belongs
# The port name input could be in default mode or in alias mode.
def get_port_namespace(port):
Expand Down Expand Up @@ -1330,9 +1377,64 @@ def add_portchannel_member(ctx, portchannel_name, port_name):
ctx.fail("{} is configured as mirror destination port".format(port_name))

# Check if the member interface given by user is valid in the namespace.
if interface_name_is_valid(db, port_name) is False:
if port_name.startswith("Ethernet") is False or interface_name_is_valid(db, port_name) is False:
ctx.fail("Interface name is invalid. Please enter a valid interface name!!")

# Dont proceed if the port channel name is not valid
if is_portchannel_name_valid(portchannel_name) is False:
ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'"
.format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO))

# Dont proceed if the port channel does not exist
if is_portchannel_present_in_db(db, portchannel_name) is False:
ctx.fail("{} is not present.".format(portchannel_name))

# Dont allow a port to be member of port channel if it is configured with an IP address
for k,v in db.get_table('INTERFACE').iteritems():
if type(k) == tuple:
continue
if k == port_name:
ctx.fail(" {} has ip address {} configured".format(port_name, v))
return

# 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:
ctx.fail("%s Interface configured as VLAN_MEMBER under vlan : %s" %(port_name,str(k)))
return

# Dont allow a port to be member of port channel if it is already member of a port channel
for k,v in db.get_table('PORTCHANNEL_MEMBER'):
if v == port_name:
ctx.fail("{} Interface is already member of {} ".format(v,k))

# Dont allow a port to be member of port channel if its speed does not match with existing members
for k,v in db.get_table('PORTCHANNEL_MEMBER'):
if k == portchannel_name:
member_port_entry = db.get_entry('PORT', v)
port_entry = db.get_entry('PORT', port_name)

if member_port_entry is not None and port_entry is not None:
member_port_speed = member_port_entry.get(PORT_SPEED)

port_speed = port_entry.get(PORT_SPEED)
if member_port_speed != port_speed:
ctx.fail("Port speed of {} is different than the other members of the portchannel {}"
.format(port_name, portchannel_name))

# Dont allow a port to be member of port channel if its MTU does not match with portchannel
portchannel_entry = db.get_entry('PORTCHANNEL', portchannel_name)
if portchannel_entry and portchannel_entry.get(PORT_MTU) is not None :
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also reject configuring speed/mtu on an interface if its already a port-channel member? Else the check is only in one flow, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added similar check while configuring the mtu for an interface.

Copy link

@yintao1995 yintao1995 Feb 1, 2021

Choose a reason for hiding this comment

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

May I ask a question, has it cover this kind of situation below:
if i configure a portchannel using json file, but it doesn't contain mtu. e.g:
{
"PORTCHANNEL":{
"PortChannel1234": {}
}
}
so teammgrd will consider it as default mtu 9100.
then i want to join a port with mtu 2000 into this portchannel with config cmd, the cmd will accept even their mtu are different because portchannel's mtu attrbute would not appear in config_db in this situation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yintao1995 Right, this kind of situation is not covered.

port_entry = db.get_entry('PORT', port_name)

if port_entry and port_entry.get(PORT_MTU) is not None:
port_mtu = port_entry.get(PORT_MTU)

portchannel_mtu = portchannel_entry.get(PORT_MTU)
if portchannel_mtu != port_mtu:
ctx.fail("Port MTU of {} is different than the {} MTU size"
.format(port_name, portchannel_name))

db.set_entry('PORTCHANNEL_MEMBER', (portchannel_name, port_name),
{'NULL': 'NULL'})

Expand All @@ -1342,12 +1444,25 @@ def add_portchannel_member(ctx, portchannel_name, port_name):
@click.pass_context
def del_portchannel_member(ctx, portchannel_name, port_name):
"""Remove member from portchannel"""
# Dont proceed if the port channel name is not valid
if is_portchannel_name_valid(portchannel_name) is False:
ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'"
.format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO))

db = ctx.obj['db']

# Check if the member interface given by user is valid in the namespace.
if interface_name_is_valid(db, port_name) is False:
ctx.fail("Interface name is invalid. Please enter a valid interface name!!")

# Dont proceed if the port channel does not exist
if is_portchannel_present_in_db(db, portchannel_name) is False:
ctx.fail("{} is not present.".format(portchannel_name))

# Dont proceed if the the port is not an existing member of the port channel
if not is_port_member_of_this_portchannel(db, port_name, portchannel_name):
ctx.fail("{} is not a member of portchannel {}".format(port_name, portchannel_name))

db.set_entry('PORTCHANNEL_MEMBER', (portchannel_name, port_name), None)
db.set_entry('PORTCHANNEL_MEMBER', portchannel_name + '|' + port_name, None)

Expand Down