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

[teammgr]: Waiting MACsec ready before doLagMemberTask #2286

Merged
merged 6 commits into from
Jul 6, 2022

Conversation

Pterosaur
Copy link
Contributor

@Pterosaur Pterosaur commented May 23, 2022

Signed-off-by: Ze Gan ganze718@gmail.com, Judy Joseph jujoseph@microsoft.com

What I did
If a member of portchannel has macsec profile attached in config, enable MACsec on the port before it's been added as a member of portchannel.

Why I did it
Due to some hardware limitation, cannot enable MACsec on a member of portchannel.
So we enable the macsec on interface first and then add it as part of portchannel.

Note: This is a work around which will be removed when h/w supports it future releases.

The approach taken in this PR is

In the teamdMgr when an interface is added as part of the LAG, we wait for the macsecPort creation done in SAI and Ingress SA creation complete (if macsec is enabled on the interface)
The above takes care of config reload, reboot scenario's where we cannot guarantee the sequence of macsec attach to interface, add interface as part of portchannel.

If we do a manual removal of port from portchannel, or remove macsec config from the interface, Please follow this steps

First remove the portchannel member out of portchannel
Remove the macsec profile attached to interface.

How I verified it

Verified with config reload, reboot with the macsec profile attached to portchannel member interfaces.
Verified case when SAK rekey is enabled on macsec on portchannel members
Verified case when member interface link flaps

Details if related

Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur marked this pull request as ready for review May 24, 2022 11:55
cfgmgr/teammgr.cpp Outdated Show resolved Hide resolved
cfgmgr/teammgr.cpp Outdated Show resolved Hide resolved
Signed-off-by: Ze Gan <ganze718@gmail.com>
Earlier we were checking for Macsec Port creation.
Just check for macsec attached to interface is enough.

Can later plan to introduce the yang model validation for macsec leaf in PORT if needed.
@judyjoseph
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

cfgmgr/teammgr.cpp Outdated Show resolved Hide resolved
@judyjoseph judyjoseph requested a review from prsunny June 23, 2022 22:00
@Pterosaur
Copy link
Contributor Author

Pterosaur commented Jul 1, 2022

image
image

The coverage tool has a false negative to this PR. Because I truly got the "missing(declared by coverage)" line from syslog. I think The most functions have been covered by our test case.

@prsunny Could you please help to merge this PR?

@Pterosaur
Copy link
Contributor Author

image
This is another evidence that I find in the log from log/test_inband_intf_mgmt_vrf/log/syslog (https://dev.azure.com/mssonic/build/_build/results?buildId=113863&view=artifacts&pathAsName=false&type=publishedArtifacts)

Copy link
Contributor

@judyjoseph judyjoseph left a comment

Choose a reason for hiding this comment

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

Validated this logic is applied only on ports with macsec configured.. @prsunny to comment - the coverage details added by Ze looks not in sync with logs

@Pterosaur Pterosaur merged commit d621542 into sonic-net:master Jul 6, 2022
@prsunny
Copy link
Collaborator

prsunny commented Jul 6, 2022

Validated this logic is applied only on ports with macsec configured.. @prsunny to comment - the coverage details added by Ze looks not in sync with logs

Agree with Ze and Judy that this is a false negative. @theasianpianist for viz

dprital added a commit to dprital/sonic-buildimage that referenced this pull request Jul 7, 2022
Update sonic-swss submodule pointer to include the following:
* Port configuration incremental update support ([sonic-net#2305](sonic-net/sonic-swss#2305))
* [VS Test] Skip failing subport tests ([sonic-net#2370](sonic-net/sonic-swss#2370))
* [teammgr]: Waiting MACsec ready before doLagMemberTask ([sonic-net#2286](sonic-net/sonic-swss#2286))
* [vnetorch] [vxlanorch] fix a set of memory usage issues ([sonic-net#2352](sonic-net/sonic-swss#2352))
* [tests] [asan] add graceful stop flag ([sonic-net#2347](sonic-net/sonic-swss#2347))
* [asan] suppress the static variable leaks ([sonic-net#2354](sonic-net/sonic-swss#2354))
* Add support for IP interface loopback action ([sonic-net#2307](sonic-net/sonic-swss#2307))
* [orchagent]: srv6orch support for uSID ([sonic-net#2335](sonic-net/sonic-swss#2335))

Signed-off-by: dprital <drorp@nvidia.com>
liat-grozovik pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Jul 14, 2022
Update sonic-swss submodule pointer to include the following:
* Port configuration incremental update support ([#2305](sonic-net/sonic-swss#2305))
* [VS Test] Skip failing subport tests ([#2370](sonic-net/sonic-swss#2370))
* [teammgr]: Waiting MACsec ready before doLagMemberTask ([#2286](sonic-net/sonic-swss#2286))
* [vnetorch] [vxlanorch] fix a set of memory usage issues ([#2352](sonic-net/sonic-swss#2352))
* [tests] [asan] add graceful stop flag ([#2347](sonic-net/sonic-swss#2347))
* [asan] suppress the static variable leaks ([#2354](sonic-net/sonic-swss#2354))
* Add support for IP interface loopback action ([#2307](sonic-net/sonic-swss#2307))
* [orchagent]: srv6orch support for uSID ([#2335](sonic-net/sonic-swss#2335))

Signed-off-by: dprital <drorp@nvidia.com>
yxieca pushed a commit that referenced this pull request Jul 17, 2022
Signed-off-by: Ze Gan ganze718@gmail.com, Judy Joseph jujoseph@microsoft.com

What I did
If a member of portchannel has macsec profile attached in config, enable MACsec on the port before it's been added as a member of portchannel.

Why I did it
Due to some hardware limitation, cannot enable MACsec on a member of portchannel.
So we enable the macsec on interface first and then add it as part of portchannel.

Note: This is a work around which will be removed when h/w supports it future releases.

The approach taken in this PR is

In the teamdMgr when an interface is added as part of the LAG, we wait for the macsecPort creation done in SAI and Ingress SA creation complete (if macsec is enabled on the interface)
The above takes care of config reload, reboot scenario's where we cannot guarantee the sequence of macsec attach to interface, add interface as part of portchannel.

If we do a manual removal of port from portchannel, or remove macsec config from the interface, Please follow this steps

First remove the portchannel member out of portchannel
Remove the macsec profile attached to interface.

How I verified it

Verified with config reload, reboot with the macsec profile attached to portchannel member interfaces.
Verified case when SAK rekey is enabled on macsec on portchannel members
Verified case when member interface link flaps
preetham-singh pushed a commit to preetham-singh/sonic-swss that referenced this pull request Aug 6, 2022
Signed-off-by: Ze Gan ganze718@gmail.com, Judy Joseph jujoseph@microsoft.com

What I did
If a member of portchannel has macsec profile attached in config, enable MACsec on the port before it's been added as a member of portchannel.

Why I did it
Due to some hardware limitation, cannot enable MACsec on a member of portchannel.
So we enable the macsec on interface first and then add it as part of portchannel.

Note: This is a work around which will be removed when h/w supports it future releases.

The approach taken in this PR is

In the teamdMgr when an interface is added as part of the LAG, we wait for the macsecPort creation done in SAI and Ingress SA creation complete (if macsec is enabled on the interface)
The above takes care of config reload, reboot scenario's where we cannot guarantee the sequence of macsec attach to interface, add interface as part of portchannel.

If we do a manual removal of port from portchannel, or remove macsec config from the interface, Please follow this steps

First remove the portchannel member out of portchannel
Remove the macsec profile attached to interface.

How I verified it

Verified with config reload, reboot with the macsec profile attached to portchannel member interfaces.
Verified case when SAK rekey is enabled on macsec on portchannel members
Verified case when member interface link flaps
abdosi added a commit to sonic-net/sonic-mgmt that referenced this pull request Nov 15, 2023
…#10732)

What I did:
macsec workaround for portchannel as define here sonic-net/sonic-swss#2286 applies only to dnx platforms.

This workaround causes failure of macsec bgp protocol testcase on platforms that does not need this workaround.

How I verify:

Test case passes after this change.
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Nov 20, 2023
…sonic-net#10732)

What I did:
macsec workaround for portchannel as define here sonic-net/sonic-swss#2286 applies only to dnx platforms.

This workaround causes failure of macsec bgp protocol testcase on platforms that does not need this workaround.

How I verify:

Test case passes after this change.
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Nov 20, 2023
…sonic-net#10732)

What I did:
macsec workaround for portchannel as define here sonic-net/sonic-swss#2286 applies only to dnx platforms.

This workaround causes failure of macsec bgp protocol testcase on platforms that does not need this workaround.

How I verify:

Test case passes after this change.
mssonicbld pushed a commit to sonic-net/sonic-mgmt that referenced this pull request Nov 20, 2023
…#10732)

What I did:
macsec workaround for portchannel as define here sonic-net/sonic-swss#2286 applies only to dnx platforms.

This workaround causes failure of macsec bgp protocol testcase on platforms that does not need this workaround.

How I verify:

Test case passes after this change.
mssonicbld pushed a commit to sonic-net/sonic-mgmt that referenced this pull request Nov 20, 2023
…#10732)

What I did:
macsec workaround for portchannel as define here sonic-net/sonic-swss#2286 applies only to dnx platforms.

This workaround causes failure of macsec bgp protocol testcase on platforms that does not need this workaround.

How I verify:

Test case passes after this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants