[yang]: modify Vlan Member to include PortChannel along with Port #7407
Conversation
Signed-off-by: Arthi Sivanantham <[email protected]>
Signed-off-by: Arthi Sivanantham <[email protected]>
| } | ||
| } | ||
|
|
||
| leaf ifname { |
There was a problem hiding this comment.
please do not change the name here.
There was a problem hiding this comment.
In community call regarding naming, we have decided to name it as port for physical ethernet port and ifname for mix of ports cases. And just “name” for interfaces defined in sonic-port.yang, sonic-portchannel.yang, sonic-vlan.yang. Also since this is a key attribute, changing attribute-name will not cause any backward compatibility impact. Based on these, we have updated here
There was a problem hiding this comment.
ifname for mix of ports cases.?, ..., ifname is for interface name not for mix of ports.
We can stick to port:
as https://github.com/Azure/sonic-buildimage/blob/29763cd095e8bb63c0756a84b375ef5d445f4648/src/sonic-yang-models/yang-models/sonic-acl.yang#L258
There was a problem hiding this comment.
i do not understand this. vlan_member is only for one member port, why do we have a list of port now?
There was a problem hiding this comment.
@lguohan : Same VLAN can have multiple ports as members. Also same VLAN can have ethernet port & port-channel as members here. The comment mix-of-ports is mentioned in the perspective of different types of interfaces (Ethernet, Port-channel). Hope this clarifies the context of discussion.
@praveen-li : I remember, while we were discussing about consistent naming for interface names for this PR (#7049) in call, we said port is generally in the context specific to "Physical ethernet interfaces" and ifname (since it's interface-name) can be widely used to refer multiple different types of interfaces like Ethernet & Port-channel together + other types of interfaces like VLAN, loopback in other applications context like BGP where it will enable on all types of interfaces. In those understanding, here we have renamed it as ifname to include Etherent & Port-channel together.
|
@praveen-li to review |
lguohan
changed the title
Signed-off-by: Arthi Sivanantham <[email protected]>
|
@praveen-li : @ArthiSivanantham has addressed few comments with changes & we have responded all other comments. Can you please check & approve the PR, if no further comments? Thanks. |
ArthiSivanantham
force-pushed
the
sonic_intf
branch
from
3fe8c86
to
7c6e7df
Compare
Signed-off-by: Arthi Sivanantham <[email protected]>
ArthiSivanantham
force-pushed
the
sonic_intf
branch
from
7c6e7df
to
f1206a4
Compare
|
@praveen-li - All comments are addressed. Can you please review and approve the PR ? |
|
@lguohan - Can you please check this generic build failure ?
Looks like pip got updated and made a backward incompatible change is breaking pyang. |
|
@ArthiSivanantham : I see a recent conversation in sonic-community email thread about this recent compilation errors with sonic-acl.yang & sonic-device_metadata.yang. Looks like it's fixed in latest master. Can you try to sync the latest master to your branch and see whether builds succeeds? That community email conversation about this:
|
|
@praveen-li - All review comments are addressed. Can you please review and approve this PR ? |
…nic-net#7407) To include PortChannel as Vlan Member (in addition to the already existing physical port) Signed-off-by: Arthi Sivanantham <[email protected]>
…nic-net#7407) To include PortChannel as Vlan Member (in addition to the already existing physical port) Signed-off-by: Arthi Sivanantham <[email protected]>
Signed-off-by: Arthi Sivanantham [email protected]
Why I did it
To include PortChannel as Vlan Member (in addition to the already existing physical port)
How I did it
Modified the type of vlan member port to a union of PortChannel name and Port name.
How to verify it
sonic_yang_models package build
sonic_yang_mgmt package build
Which release branch to backport (provide reason below if selected)
Description for the changelog
Modify Vlan Member to include PortChannel along with Port