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

add lacp_rate to portchannel #2036

Merged
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
14 changes: 10 additions & 4 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1950,8 +1950,11 @@ def portchannel(db, ctx, namespace):
@click.argument('portchannel_name', metavar='<portchannel_name>', required=True)
@click.option('--min-links', default=1, type=click.IntRange(1,1024))
@click.option('--fallback', default='false')
@click.option('--fast-rate', default='false',
type=click.Choice(['true', 'false'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

@globaltrouble IMHO, better to use flags or click.BOOL

Copy link

Choose a reason for hiding this comment

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

In the current implementation, the syntax is the same for fallback and for fast-rate. Booth requires an argument. If change type to bool or flags then I think it should be done for both options together is separate PR

case_sensitive=False))
@click.pass_context
def add_portchannel(ctx, portchannel_name, min_links, fallback):
def add_portchannel(ctx, portchannel_name, min_links, fallback, fast_rate):
Copy link
Collaborator

Choose a reason for hiding this comment

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

@globaltrouble please add a UTs to cover this case

"""Add port channel"""
if is_portchannel_name_valid(portchannel_name) != True:
ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'"
Expand All @@ -1962,9 +1965,12 @@ def add_portchannel(ctx, portchannel_name, min_links, fallback):
if is_portchannel_present_in_db(db, portchannel_name):
ctx.fail("{} already exists!".format(portchannel_name))

fvs = {'admin_status': 'up',
'mtu': '9100',
'lacp_key': 'auto'}
fvs = {
'admin_status': 'up',
'mtu': '9100',
'lacp_key': 'auto',
'fast_rate': fast_rate.lower(),
}
if min_links != 0:
fvs['min_links'] = str(min_links)
if fallback != 'false':
Expand Down
5 changes: 3 additions & 2 deletions doc/Command-Reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -6925,12 +6925,13 @@ When any port is already member of any other portchannel and if user tries to ad
Command takes two optional arguements given below.
1) min-links - minimum number of links required to bring up the portchannel
2) fallback - true/false. LACP fallback feature can be enabled / disabled. When it is set to true, only one member port will be selected as active per portchannel during fallback mode. Refer https://github.com/Azure/SONiC/blob/master/doc/lag/LACP%20Fallback%20Feature%20for%20SONiC_v0.5.md for more details about fallback feature.
3) fast-rate - true/false, default is false (slow). Option specifying the rate in which we'll ask our link partner to transmit LACPDU packets in 802.3ad mode. slow - request partner to transmit LACPDUs every 30 seconds, fast - request partner to transmit LACPDUs every 1 second. In slow mode 60-90 seconds needed to detect linkdown, in fast mode only 2-3 seconds.

A port channel can be deleted only if it does not have any members or the members are already deleted. When a user tries to delete a port channel and the port channel still has one or more members that exist, the deletion of port channel is blocked.

- Usage:
```
config portchannel (add | del) <portchannel_name> [--min-links <num_min_links>] [--fallback (true | false)]
config portchannel (add | del) <portchannel_name> [--min-links <num_min_links>] [--fallback (true | false) [--fast-rate (true | false)]
```

- Example (Create the portchannel with name "PortChannel0011"):
Expand Down Expand Up @@ -11020,4 +11021,4 @@ ZTP will be restarted. You may lose switch data and connectivity, continue? [y/N
Running command: ztp run -y
```

Go Back To [Beginning of the document](#SONiC-COMMAND-LINE-INTERFACE-GUIDE) or [Beginning of this section](#ztp-configuration-and-show-commands)
Go Back To [Beginning of the document](#SONiC-COMMAND-LINE-INTERFACE-GUIDE) or [Beginning of this section](#ztp-configuration-and-show-commands)
28 changes: 27 additions & 1 deletion tests/portchannel_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import pytest
import traceback

from click.testing import CliRunner
Expand Down Expand Up @@ -60,7 +61,32 @@ def test_delete_non_existing_portchannel(self):
print(result.output)
assert result.exit_code != 0
assert "Error: PortChannel0005 is not present." in result.output


@pytest.mark.parametrize("fast_rate", ["False", "True", "false", "true"])
def test_add_portchannel_with_fast_rate(self, fast_rate):
runner = CliRunner()
db = Db()
obj = {'db':db.cfgdb}

# add a portchannel with fats rate
result = runner.invoke(config.config.commands["portchannel"].commands["add"], ["PortChannel0005", "--fast-rate", fast_rate], obj=obj)
print(result.exit_code)
print(result.output)
assert result.exit_code == 0

@pytest.mark.parametrize("fast_rate", ["Fls", "tru"])
def test_add_portchannel_with_invalid_fast_rate(self, fast_rate):
runner = CliRunner()
db = Db()
obj = {'db':db.cfgdb}

# add a portchannel with invalid fats rate
result = runner.invoke(config.config.commands["portchannel"].commands["add"], ["PortChannel0005", "--fast-rate", fast_rate], obj=obj)
print(result.exit_code)
print(result.output)
assert result.exit_code != 0
assert 'Invalid value for "--fast-rate"' in result.output

def test_add_portchannel_member_with_invalid_name(self):
runner = CliRunner()
db = Db()
Expand Down