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

Fine Grained ECMP test #1788

Merged
merged 8 commits into from
Oct 22, 2020
Merged

Fine Grained ECMP test #1788

merged 8 commits into from
Oct 22, 2020

Conversation

anish-n
Copy link
Contributor

@anish-n anish-n commented Jun 18, 2020

##Description of PR
This PR contains the changes for Fine Grained ECMP test.
It is based on the following specs:
https://github.com/Azure/SONiC/blob/master/doc/ecmp/fine_grained_next_hop_hld.md
https://github.com/Azure/SONiC/pull/623/files

Summary:

Type of change

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

Approach

This test creates Fine Grained ECMP objects on the DUT, and validates data plane for fine grained ecmp via PTF tests.

What is the motivation for this PR?

https://github.com/Azure/SONiC/blob/master/doc/ecmp/fine_grained_next_hop_hld.md

How did you do it?

Initially create a route with x next-hops, send 1000 flows, change the set of next-hops for all the possible fine grained ecmp scenarios and validate that the 1000 flows adjust as expected in the scenario

How did you verify/test it?

By running the PTF test

Any platform specific information?

Applicable to all platforms which support fine grained ecmp

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

T0

Documentation

https://github.com/Azure/SONiC/blob/master/doc/ecmp/fine_grained_next_hop_hld.md

@lgtm-com
Copy link

lgtm-com bot commented Jun 18, 2020

This pull request introduces 11 alerts when merging 3214e86 into ca24f81 - view on LGTM.com

new alerts:

  • 8 for Unused import
  • 2 for Variable defined multiple times
  • 1 for 'import *' may pollute namespace

@neethajohn neethajohn requested a review from abdosi June 19, 2020 17:26
@lgtm-com
Copy link

lgtm-com bot commented Jun 20, 2020

This pull request introduces 11 alerts when merging 87039c7 into 9cd2589 - view on LGTM.com

new alerts:

  • 8 for Unused import
  • 2 for Variable defined multiple times
  • 1 for 'import *' may pollute namespace

@abdosi
Copy link
Contributor

abdosi commented Oct 16, 2020

@anish-n minigraph based test cases will be added later

@anish-n
Copy link
Contributor Author

anish-n commented Oct 16, 2020

@anish-n minigraph based test cases will be added later

Yes, will raise a separate PR to include minigraph based test case, this is because the test case for minigraph is expected to have a good deal of minigraph-related distinct changes and can be made into a separate PR. Also the minigraph changes are still open in PR form, so this would raise a dependency, so good to merge this now if possible, and have a 2nd PR to deal with the minigraph side.

tests/ecmp/test_fgnhg.py Outdated Show resolved Hide resolved
tests/ecmp/test_fgnhg.py Outdated Show resolved Hide resolved
tests/ecmp/test_fgnhg.py Outdated Show resolved Hide resolved
tests/ecmp/test_fgnhg.py Outdated Show resolved Hide resolved
@abdosi
Copy link
Contributor

abdosi commented Oct 21, 2020

@nazariig Can you approve this ?

@abdosi abdosi merged commit 0f26395 into sonic-net:master Oct 22, 2020
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.

4 participants