Skip to content

[VRF]: submit vrf feature #943

Merged
prsunny merged 46 commits intosonic-net:masterfrom
tylerlinp:master
Nov 6, 2019
Merged

[VRF]: submit vrf feature #943
prsunny merged 46 commits intosonic-net:masterfrom
tylerlinp:master

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

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
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
Copy link
Contributor Author

retest this please

2 similar comments
@prsunny
Copy link
Collaborator

prsunny commented Nov 1, 2019

retest this please

@prsunny
Copy link
Collaborator

prsunny commented Nov 1, 2019

retest this please

@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
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
Janetxxx pushed a commit to Janetxxx/sonic-swss that referenced this pull request Nov 10, 2025
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.