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

Fix tagged VlanInterface if attached to multiple vlan as untagged member #8927

Merged
merged 6 commits into from
Apr 19, 2022
Merged

Fix tagged VlanInterface if attached to multiple vlan as untagged member #8927

merged 6 commits into from
Apr 19, 2022

Conversation

qiluo-msft
Copy link
Collaborator

@qiluo-msft qiluo-msft commented Oct 8, 2021 •
edited
Loading

Why I did it

Fix several bugs:

  1. If one vlan member belongs to multiple vlans, and if any of the vlans is "Tagged" type, we respect the tagged type
  2. If one vlan member belongs to multiple vlans, and all of the vlans have no "Tagged" type, we override it to be a tagged member
  3. make sure vlantype_name is assigned correctly in each iteration

How I did it

How to verify it

  1. Test the command line to parse a minigraph and make sure the output does not change.
./sonic-cfggen -m minigraph.mlnx20.xml

The minigraph is for HwSKU Mellanox-SN2700-D40C8S8.

  1. Test on a DUT with HwSKU Mellanox-SN2700-D40C8S8
sudo config load_minigraph
show vlan brief

Checked the "Port Tagging" column in the output.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

@qiluo-msft
[minigraph] Fix tagged VlanInterface if attached to multiple vlan as …
6366538 
…untagged member

Signed-off-by: Qi Luo <qiluo-msft@users.noreply.github.com>
@qiluo-msft
Fix bug: uninitialized vlantype_name
4deb792
ganglyu
ganglyu previously approved these changes Oct 8, 2021
View reviewed changes
anish-n
anish-n previously approved these changes Oct 9, 2021
View reviewed changes
kktheballer
kktheballer previously approved these changes Oct 9, 2021
View reviewed changes
Copy link
Contributor

@kktheballer kktheballer left a comment

Choose a reason for hiding this comment

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

Tested AzD scenario using this mg on latest public master image

@qiluo-msft
Copy link
Collaborator Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft
Copy link
Collaborator Author

Waiting @kktheballer to update unit test, and will combine into this PR.

@qiluo-msft qiluo-msft marked this pull request as draft October 18, 2021 11:23
@qiluo-msft qiluo-msft dismissed stale reviews from kktheballer, anish-n, and ganglyu via 211776c April 10, 2022 21:44
@qiluo-msft qiluo-msft marked this pull request as ready for review April 11, 2022 06:18
@qiluo-msft qiluo-msft mentioned this pull request Apr 16, 2022
Merged
kktheballer
kktheballer previously approved these changes Apr 18, 2022
View reviewed changes
Copy link
Contributor

@kktheballer kktheballer left a comment

Choose a reason for hiding this comment

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

Validated 202012 version for AzD scenario

@qiluo-msft qiluo-msft merged commit 936d93c into sonic-net:master Apr 19, 2022
@qiluo-msft qiluo-msft deleted the qiluo/tagged branch April 19, 2022 22:47
qiluo-msft added a commit that referenced this pull request May 9, 2022
@qiluo-msft
[202012] Fix tagged VlanInterface if attached to multiple vlan as unt…
be5eb80 
…agged member (#10589)

Backport #8927 to 202012 branch
liushilongbuaa pushed a commit to liushilongbuaa/sonic-buildimage that referenced this pull request Jun 20, 2022
@yxieca
Merged PR 6004325: Manual merge github master to internal
19c4542 
Related work items: #49, #58, #107, sonic-net#247, sonic-net#249, sonic-net#277, sonic-net#593, sonic-net#597, sonic-net#1035, sonic-net#2130, sonic-net#2150, sonic-net#2165, sonic-net#2169, sonic-net#2178, sonic-net#2179, sonic-net#2187, sonic-net#2188, sonic-net#2191, sonic-net#2195, sonic-net#2197, sonic-net#2198, sonic-net#2200, sonic-net#2202, sonic-net#2206, sonic-net#2209, sonic-net#2211, sonic-net#2216, sonic-net#7909, sonic-net#8927, sonic-net#9681, sonic-net#9733, sonic-net#9746, sonic-net#9850, sonic-net#9967, sonic-net#10104, sonic-net#10152, sonic-net#10168, sonic-net#10228, sonic-net#10266, sonic-net#10288, sonic-net#10294, sonic-net#10313, sonic-net#10394, sonic-net#10403, sonic-net#10404, sonic-net#10421, sonic-net#10431, sonic-net#10437, sonic-net#10445, sonic-net#10457, sonic-net#10458, sonic-net#10465, sonic-net#10467, sonic-net#10469, sonic-net#10470, sonic-net#10474, sonic-net#10477, sonic-net#10478, sonic-net#10482, sonic-net#10485, sonic-net#10488, sonic-net#10489, sonic-net#10492, sonic-net#10494, sonic-net#10498, sonic-net#10501, sonic-net#10509, sonic-net#10512, sonic-net#10514, sonic-net#10516, sonic-net#10517, sonic-net#10523, sonic-net#10525, sonic-net#10531, sonic-net#10532, sonic-net#10538, sonic-net#10555, sonic-net#10557, sonic-net#10559, sonic-net#10561, sonic-net#10565, sonic-net#10572, sonic-net#10574, sonic-net#10576, sonic-net#10578, sonic-net#10581, sonic-net#10585, sonic-net#10587, sonic-net#10599, sonic-net#10607, sonic-net#10611, sonic-net#10616, sonic-net#10618, sonic-net#10619, sonic-net#10623, sonic-net#10624, sonic-net#10633, sonic-net#10646, sonic-net#10655, sonic-net#10660, sonic-net#10664, sonic-net#10680, sonic-net#10683
@qiluo-msft
Copy link
Collaborator Author

@prsunny Do you want this fix on 201911?

@qiluo-msft qiluo-msft mentioned this pull request Jan 28, 2023
Merged
8 tasks
qiluo-msft added a commit that referenced this pull request Jan 30, 2023
@qiluo-msft
[201911] Fix tagged VlanInterface if attached to multiple vlan as unt…
9ce9ba4 
…agged member (#13534)

Backport #8927
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neethajohn neethajohn neethajohn approved these changes

@anish-n anish-n anish-n approved these changes

@ganglyu ganglyu ganglyu left review comments

@kktheballer kktheballer kktheballer left review comments

@lguohan lguohan Awaiting requested review from lguohan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@qiluo-msft @kktheballer @anish-n @neethajohn @ganglyu