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

[macsec]: Override dp_poll of ptf by MACsec's #5490

Merged
merged 22 commits into from
May 30, 2022

Conversation

Pterosaur
Copy link
Contributor

@Pterosaur Pterosaur commented Apr 8, 2022

Description of PR

Summary:
Override dp_poll by MACsec dp_poll so that these dataplane testcases can be tested under MACsec environment

Wait for PR: sonic-net/sonic-buildimage#10507

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911
  • 202012

Approach

What is the motivation for this PR?

Existing dataplane tests cannot be tested under MACsec environment due to the traffic under MACsec link is encrypted.

How did you do it?

I write a macsec_dp_poll to override the original ptf dp_poll, and for each MACsec packets, the macsec_dp_poll will decrypt them and pass the clear packets to the upper layer.

How did you verify/test it?

Check Azp and try FIB test under MACsec environment.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

* Add check neighbors health for SONiC neighbor devices

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Add MACsec test

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Skip to move bp port to docker if neighbor devices are sonic

Signed-off-by: Ze Gan <ganze718@gmail.com>

* AddPolish code

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Add data plane server to neighbor

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Polish dataplane test

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Add ptf service in nSONiC eighbor devices

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Add MACsec pkt checking in neighbor devices

Signed-off-by: Ze Gan <ganze718@gmail.com>

* polish ansible task name

Signed-off-by: Ze Gan <ganze718@gmail.com>

* [MACsec test]Add test neighbor to neighbor

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Fix test

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Fix warning

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Add macsec to azp

Signed-off-by: Ze Gan <ganze718@gmail.com>

* test

Signed-off-by: Ze Gan <ganze718@gmail.com>

* fixtest

Signed-off-by: Ze Gan <ganze718@gmail.com>

* fix test

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Different priority to peers

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Add enable_macsec_feature before setup

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Polish test

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Add xpn and send_sci

Signed-off-by: Ze Gan <ganze718@gmail.com>

* remove unused import

Signed-off-by: Ze Gan <ganze718@gmail.com>

* increase timeout in sonictest-sonic-t0

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Modify timeout

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Remove useless function

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Polish code

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Polish code

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Add fault handling test

Signed-off-by: Ze Gan <ganze718@gmail.com>

* decrease cpu count of neighbor devices

Signed-off-by: Ze Gan <ganze718@gmail.com>

* Wait until mka session recovering from flap

Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur changed the title Macsec verify hook [macsec]: Override dp_poll by MACsec dp_poll Apr 8, 2022
@Pterosaur Pterosaur changed the title [macsec]: Override dp_poll by MACsec dp_poll [macsec]: Override dp_poll of ptf by MACsec's Apr 8, 2022
lguohan pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Apr 9, 2022
Why I did it
Existing dataplane tests cannot be tested under MACsec environment due to the traffic under MACsec link is encrypted. So, I will override the dp_poll of ptf to MACsec dp_poll to decrypt the MACsec packets on injected ports (PR: sonic-net/sonic-mgmt#5490). MACsec decryption library depends on scapy 2.4.5.

How I did it
Upgrade scapy library to 2.4.5 by pip.

How to verify it
Check the scapy version in docker-ptf by

python -c "import scapy; print(scapy.__version__)"
2.4.5

Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch 2 times, most recently from 2c0ffa1 to e0431d1 Compare April 11, 2022 07:42
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch from e0431d1 to 4dc25c4 Compare April 11, 2022 08:14
@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch from 79c6a0f to 73486ce Compare May 12, 2022 15:15
@lgtm-com
Copy link

lgtm-com bot commented May 12, 2022

This pull request introduces 1 alert when merging 27eff8253c1da43bd9300210e5d11bf2dbe7a1b4 into 1a1f6e5 - view on LGTM.com

new alerts:

  • 1 for Syntax error

Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch from 27eff82 to 1f37a22 Compare May 12, 2022 23:47
@lgtm-com
Copy link

lgtm-com bot commented May 13, 2022

This pull request introduces 1 alert and fixes 1 when merging 9ff4ac87960337726f784a85419ebae2e056a5ed into ead6a03 - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 1 for Unused import

@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch 2 times, most recently from 413d968 to db98ffe Compare May 14, 2022 05:45
@lgtm-com
Copy link

lgtm-com bot commented May 14, 2022

This pull request introduces 1 alert and fixes 1 when merging db98ffe72a79ee4fe1b73a22b16f063f3cf25784 into 7d3e46e - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 1 for Unused import

Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch from db98ffe to f82a69f Compare May 16, 2022 08:22
@lgtm-com
Copy link

lgtm-com bot commented May 16, 2022

This pull request introduces 1 alert and fixes 1 when merging 7351e3f into 57e5b17 - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 1 for Unused import

@Pterosaur Pterosaur marked this pull request as ready for review May 20, 2022 04:01
@Pterosaur Pterosaur requested a review from a team as a code owner May 20, 2022 04:01
@Pterosaur Pterosaur requested a review from jimmyzhai May 20, 2022 04:02
@@ -58,22 +60,22 @@ def __init__(self, file_path):
for line in f.readlines():
if pattern.match(line): continue
entry = line.split(' ', 1)
prefix = ip_network(unicode(entry[0]))
prefix = ip_network(unicode(entry[0]) if sys.version_info[0] == 2 else entry[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there another better way to making unicode string for python2 and 3 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to Xin's suggestion, we can leverage the library six for portable text string at here.

@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch 2 times, most recently from 0c2a547 to 060adbe Compare May 24, 2022 10:06
@Pterosaur Pterosaur requested a review from jimmyzhai May 24, 2022 12:16
@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch from 060adbe to 7ac68b9 Compare May 24, 2022 12:48
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch from 7ac68b9 to 774d323 Compare May 24, 2022 13:05
Pterosaur added 2 commits May 24, 2022 13:13
This reverts commit 774d323.
Signed-off-by: Ze Gan <ganze718@gmail.com>
@lgtm-com
Copy link

lgtm-com bot commented May 25, 2022

This pull request introduces 1 alert when merging cdc633452677865384b04c03c6822f01ddf34671 into c1ecb7e - view on LGTM.com

new alerts:

  • 1 for Syntax error

Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch 2 times, most recently from 1527417 to 5eadeaf Compare May 27, 2022 00:58
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch from 5eadeaf to b292cb7 Compare May 27, 2022 04:21
if ptf.dataplane.match_exp_pkt(exp_pkt, pkt):
return ret
else:
continue
Copy link
Contributor

@jimmyzhai jimmyzhai May 27, 2022

Choose a reason for hiding this comment

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

Potential loop forever? Then using while timeout > 0 in line 44?

MACSEC_INFOS = pickle.load(f)
if MACSEC_INFOS:
__origin_dp_poll = ptf.testutils.dp_poll
ptf.testutils.dp_poll = __macsec_dp_poll
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to use macsec_helper.macsec_dp_poll ? Then not to define __macsec_dp_poll.

Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur force-pushed the macsec_verify_hook branch from 2beca00 to ed14786 Compare May 27, 2022 08:23
Signed-off-by: Ze Gan <ganze718@gmail.com>
@abdosi
Copy link
Contributor

abdosi commented Jul 26, 2022

@Pterosaur looks like this change is not backward compatible. It forces user to upgrade PTF docker for running fib test. Can we make it if we want to test fib under macsec environment then only trigger this change.

@Pterosaur
Copy link
Contributor Author

@Pterosaur looks like this change is not backward compatible. It forces user to upgrade PTF docker for running fib test. Can we make it if we want to test fib under macsec environment then only trigger this change.

I will take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants