-
Notifications
You must be signed in to change notification settings - Fork 667
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
[CLI] MACsec support #1727
base: master
Are you sure you want to change the base?
[CLI] MACsec support #1727
Conversation
This pull request introduces 1 alert when merging 766aaf9 into 4422911 - view on LGTM.com new alerts:
|
Please fix the lgtm alert |
This pull request introduces 1 alert when merging 42eab9a8aa36d6dfaa8c430fe18fa08d324aec82 into ca728b8 - view on LGTM.com new alerts:
|
This pull request introduces 4 alerts when merging c5ed372 into ca728b8 - view on LGTM.com new alerts:
|
This pull request introduces 4 alerts when merging 193ef4c53f31982953f3b4a911871bf6f8ab1910 into 095bf54 - view on LGTM.com new alerts:
|
def display_connections(self, intf_name): | ||
""" | ||
Get MACsec info from wpa_cli. | ||
""" | ||
macsec_cmd = 'docker exec -it macsec wpa_cli -g /var/run/' + intf_name + ' IFNAME=' + intf_name + ' macsec' | ||
click.echo("Interface: {} ".format(intf_name)) | ||
p = subprocess.Popen(macsec_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) | ||
(output, err) = p.communicate() | ||
rc = p.wait() | ||
if rc == 0: | ||
click.echo(output) | ||
else: | ||
click.echo("Error retrieving MACsec connections: {} ".format(err)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to fetch the connections from APP DB instead of wpa_cli. Because in the future, maybe we will use multiple wpa_supplicant processes to one port for Primary/Fallback key feature. Or we can use the static SAK directly in APP DB for debugging.
So, I think it's better to believe the data in APP DB for more flexibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is here for contribution to the community in its current state. Only bug fixes will be made to this PR at this point. Any major functionality changes will not be made in the context of this PR. If a different implementation of the "show macsec connections" command is preferred, feel free to use the changes in this PR as a basis for another PR and I will close this one.
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
What I did
How I did it
How to verify it
System integration test of port add/del and repeated port add/del/add testing on JNPR and VS MACsec platforms.
System integration test of profile add/del on JNPR and VS MACsec platforms.
System integration test of "show macsec connections" on JNPR and VS MACsec platforms.
System integration test of "show macsec statistics" on JNPR and VS MACsec platforms.
Previous command output (if the output of a command-line utility has changed)
N/A
New command output (if the output of a command-line utility has changed)
N/A