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.

Sign up for GitHub

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

Jump to bottom

[VRF]: submit vrf feature #943

Merged
merged 46 commits into from
Nov 6, 2019
Merged

[VRF]: submit vrf feature #943

merged 46 commits into from
Nov 6, 2019

Conversation

tylerlinp
Copy link
Contributor

What I did
Submit VRF implementation according to vrf HLD #392

Why I did it
Following sonic 201908 roadmap

How I verified it
Pass vrf vs tests case and ansible test cases

Details if related

Tyler Li and others added 17 commits April 10, 2018 02:52
FdbOrch getPort failed because fdb_entry have no switch_id
f448a22
[crmorch]: neighbor used counter increased twice
7f81135
@tylerlinp
Merge pull request #1 from Azure/master
91f1f00 
merge azure sonic-swss master
@leoli-nps
[Portsorch]: Add lag-name and oid map to counter table COUNTERS_PORT_…
5c2af28 
…NAME_MAP
@leoli-nps
[Portsorch]:Add operation to remove lag-name and oid map from counter…
53f2663 
… table COUNTERS_PORT_NAME_MAP
@tylerlinp
Merge pull request #2 from Azure/master
fa95f1e 
merge azure sonic-swss master
@leoli-nps
[Pfc-wd]: Add pfc-wd support for nephos
03c0472
@leoli-nps
Merge branch 'master' into master
e456159
@leoli-nps
Update Makefile.am
88e3f4f 
Delete the extra line left in the conflict resolution
@leoli-nps
[Pfc-wd]: Remove pfc-wd code for nephos from this branch
91fdc69
@tylerlinp
Merge pull request #3 from Azure/master
7bfc853 
Merge Azure/sonic-swss
Support VRF
e772a12
revert portsorch to asure
213bca9
Loopback support VRF
771d5f1
@tylerlinp
Merge pull request #4 from Azure/master
90e8000 
Merge Azure/sonic-swss
VS test for VRF
e47cbc4
VRF: filter routes pointed to vrf interfaces
2c64599
@tylerlinp tylerlinp mentioned this pull request Jun 21, 2019
Merged
lguohan
lguohan requested changes Jun 25, 2019
View reviewed changes
Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

this PR is too huge, can you break down into multiple small patches? there is no possibilities that this huge PR can be reviewed properly.

@tylerlinp tylerlinp mentioned this pull request Jun 25, 2019
Closed
@shine4chen shine4chen mentioned this pull request Jun 27, 2019
Closed
@tylerlinp
Copy link
Contributor Author

this PR is too huge, can you break down into multiple small patches? there is no possibilities that this huge PR can be reviewed properly.

I had try to split but found it has heavy dependence. Huge codes have to rewite for any PR splited, furthermore it is difficult to test then. If truely rewrite to several PRs, it would be a big trouble to merge them. For example the PR 878, it is just the content of "move subnet route from intfsorch to routeorch", which is the version based origin master. But after composing other parts, it had undergone enormous changes. So it is meaningless to review PR 878 or something similar.
For esaier review, I suggest to recommit to a single PR, spliting to several commits. Maybe split based process, vrfmgrd/intfmgrd/nbrmgrd/neighsyncd/fpmsyncd/ORCHAGENT/TESTS. How do you think about?

@prsunny
Copy link
Collaborator

prsunny commented Jul 3, 2019

@tylerlinp , we have planned to split based on subcomponents. We won't be able to review/merge this big change. Can you check on this list:

a. One PR to change Nexthop/Neighbor Keys to include interface name. (This is generic irrespective of VRF)
b. One PR to move subnet routes to routeorch and cleanup interface orch (This is generic irrespective of VRF)
c. One PR to handle loopback interface changes
d. One PR for the rest of VRF changes.

@tylerlinp tylerlinp mentioned this pull request Jul 17, 2019
Merged
@shine4chen
Copy link
Contributor

retest this please

4 similar comments
@prsunny
Copy link
Collaborator

prsunny commented Oct 31, 2019

retest this please

@tylerlinp
Copy link
Contributor Author

retest this please

@prsunny
Copy link
Collaborator

prsunny commented Nov 1, 2019

retest this please

@prsunny
Copy link
Collaborator

prsunny commented Nov 1, 2019

retest this please

@prsunny prsunny mentioned this pull request Nov 1, 2019
Merged
@shine4chen
Copy link
Contributor

@prsunny We notice that recently test_FdbAddedAfterMemberCreated testcases almost failed in all PR vs. We will help to dig it next week. We speculate that it may have something to do with vs test topology or environment.

@prsunny
Copy link
Collaborator

prsunny commented Nov 3, 2019

@shine4chen , thanks. That would be helpful.

@tylerlinp
Copy link
Contributor Author

retest this please

@tylerlinp tylerlinp force-pushed the master branch 2 times, most recently from ef6f72e to 559e304 Compare November 4, 2019 09:53
@prsunny
Copy link
Collaborator

prsunny commented Nov 4, 2019

Hi Tyler, I see that the FDB test is still failing, may be it needs a sleep before checking? Also please don't change the test in this PR. It has to be another PR which can be merged and then retest this.

@tylerlinp
Copy link
Contributor Author

@prsunny The new failure is result from a bug that is_fdb_entry_exists didn't match input key, so when fdb table is not empty then get true. I will revert this test change later and raise a new PR.

@tylerlinp tylerlinp force-pushed the master branch 2 times, most recently from 6740f34 to 70c3222 Compare November 5, 2019 12:53
@tylerlinp
Copy link
Contributor Author

@prsunny PR #1118 fix the fdb test failure. This PR will revert to commit 7987477 and retest.

@prsunny
Copy link
Collaborator

prsunny commented Nov 5, 2019

@shine4chen , thanks. Can you please resolve confict

@tylerlinp
Copy link
Contributor Author

retest this please

@prsunny prsunny merged commit 85ff17d into sonic-net:master Nov 6, 2019
@prsunny prsunny mentioned this pull request Nov 12, 2019
Merged
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
[crm] Use swsssdk API instead of redis-cli for getting keys from redi…
1565ba2 
…s DB (sonic-net#943)
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
@kcudnik
[unittest] Update meta tests (sonic-net#943)
26b2237
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karthikarum karthikarum karthikarum left review comments

@shine4chen shine4chen shine4chen left review comments

@preetham-singh preetham-singh preetham-singh approved these changes

@lguohan lguohan lguohan requested changes

@prsunny prsunny prsunny approved these changes

@venkatmahalingam venkatmahalingam venkatmahalingam approved these changes

Assignees
No one assigned
Labels
Enhancement ➕ Included in 201911 Branch request_for_201911_branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.