-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add VLAN Stacking HLD #915
base: master
Are you sure you want to change the base?
Conversation
This document describes the design details of the VLAN stacking feature. VLAN stacking is a feature designed for service providers who carry traffic of multiple customers across their networks and are required to maintain the VLAN and Layer 2 protocol configurations of each customer without impacting the traffic of other customers. VLAN stacking include QinQ feature and VLAN Translation feature. | ||
|
||
IEEE 802.1Q tunneling (QinQ tunneling) uses a single Service Provider VLAN (SPVLAN) for customers who have multiple VLANs. Customer VLAN IDs are preserved and traffic from different customers is segregated within the service provider’s network even when they use the same customer-specific VLAN IDs. QinQ tunneling expands VLAN space by using a VLAN-in-VLAN hierarchy, preserving the customer’s original tagged packets, and adding SPVLAN tags to each frame (also called double tagging). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is MAC-learning targeted in this feature ? If yes, are MACs learned in context of C-VLAN or S-VLAN. If we are learning in S-VLAN context then MACs have to unique across C-VLANs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, MAC-learning works in this feature, when the feature is configured, the MAC will learn in S-VLAN. Will write new comments on this HLD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sunesh Is there any other comment there, ifnot, please help to approve to resolve this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pttch may be later in the doc u can clarify n:1 map for C-VLAN to S-VLAN requires macs to be unique across all C-VLANs since learning is happening only on S-VLAN.
* Direction - Traffic direction. | ||
* Action - The rewrite action for the frames, support the following actions: | ||
* push | ||
* Apply for all tagged frames, single or double tagged frames, at ingress direction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Native VLAN supported on ingress port. We should explicitly mention behavior for native VLAN case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VLAN stacking will only effect the tagged packet, so un-tagged packet will be transmitted by native VLAN. Will write new comments on this HLD.
* Action - The rewrite action for the frames, support the following actions: | ||
* push | ||
* Apply for all tagged frames, single or double tagged frames, at ingress direction. | ||
* Add an outer VLAN with TPID=8100, COS=0, VLAN=<specified-S-VLAN> (range 1 ~ 4094). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason TPID=8100 was picked in place of IEEE defined 88a8. We should add details about CoS behavior. Is this always a fixed value or plan to support copy from inner etc..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Currently we use the TPID=8100 as many other product do. This value can changed by predefine value.
- Will write the cos behavior in this HLD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume we'll have an option to configure TPID value as 0x8100 (802.1Q) or 0x88a8 (802.1ad) or any non-standard value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, it do not have option for this, it's also can add this to the HLD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cos behavior
The TPID and COS behavior is already mentioned in "Requirements" section (TPID and COS are fixed to 0x8100 and 0, and they are not configurable by now). These parameters are possible be configured but not plan to support for now. Shall we place this function requirement into the future work section?
Add it will to the future work, below will add to the commit.
Future Work
The values of TPID and COS in the adding outer VLAN are configurable. For example, TPID may be configured as 0x88a8 which is defined in IEEE or other values. For the COS value, it may be able to configure to extract from the COS value of the original VLAN.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks!
} | ||
|
||
leaf action { | ||
type string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an optional field? if yes, what's default action?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the action is mandatory. Will to add mandatory attribute in here
|
||
leaf s_vlanid { | ||
type leafref { | ||
path /vlan:sonic-vlan/vlan:VLAN/vlan:VLAN_LIST/vlan:vlanid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how this s_vlanid is being used for various actions in different stages, please add description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provided only if action is push or swap. Will to add comment in here.
|
||
# Overview | ||
|
||
This document describes the design details of the VLAN stacking feature. VLAN stacking is a feature designed for service providers who carry traffic of multiple customers across their networks and are required to maintain the VLAN and Layer 2 protocol configurations of each customer without impacting the traffic of other customers. VLAN stacking include QinQ feature and VLAN Translation feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this HLD cover all-to-one mapping (tunneling)? How to configure many-to-one (selective Q-in-Q)? Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It support. The below example show how to configure many-to-one as matching C-VLAN 10, 20 to S-VLAN 300.
{"VLAN_STACKING": {"Ethernet52|INGRESS|10": {"action": "PUSH","s_vlanid": "30"},"Ethernet52|INGRESS|20": {"action": "PUSH","s_vlanid": "30"},"Ethernet52|INGRESS|40": {"action": "PUSH","s_vlanid": "30"},"Ethernet52|EGRESS|30": {"action": "POP"}}}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it mean that each and every C-VLANs must be defined explicitly? Possibly hundreds of them... How about all-to-one mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it need to define the C-VLANs. Regarding to all-to-one. It's hardware dependent. If the entry has enough vlan ids, it can do the configuration. For the future work, it can change new schema to support range format for VLAN ids.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akokhan Is there any other comment there, ifnot, please help to approve to resolve this
Regarding to SAI, my colleague alredy has a PR and got approve, waiting for Merge now. |
QinQ tunneling uses double tagging to preserve the customer’s VLAN tags on traffic crossing the service provider’s network. However, if any switch in the path crossing the service provider’s network does not support this feature, then the switches directly connected to that device can be configured to swap the customer’s VLAN ID with the service provider’s VLAN ID for upstream traffic, or the service provider’s VLAN ID with the customer’s VLAN ID for downstream traffic. | ||
|
||
## VLAN Stacking Deployment Use Cases | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @pttch
Here are our comments from Dell team (@skg-net Senthil Kumar Ganesan, @avinothcgl Vinoth Kumar Arumugam, and myself)
Changes in the config-DB:
-
Enhance PORT & PORTCHANNEL table to include the fields
a) customer edge port (true/false)
b) SVID field (valid only if a) is set)
c) SVID override priority (valid only if a) is set) -> (if this field is not configured, the default behavior is to
inherit priority from C-tag)
-> Upon getting any C-tag on this port/port-channel, SVID should be pushed on the ingress stage and the same should be popped on the egress stage. -
VLAN_STACKING -> Keys: a) Port/Portchannel b) SVID
fields: cvid leaf-list
-> This table config would be used when there is a need for explicit CVID to SVID and vice-versa mapping -
VLAN_XLATE -> This table will be used to translate from one VLAN to another, there can be a field to indicate whether the translation is expected on the inner or outer VLAN tag and another field to set the priority after translation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@venkatmahalingam Thanks for your suggestion, Could you share the mib files to me ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I sent it to your email.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@venkatmahalingam
After internal discuss, we have the questions,
- a) customer edge port (true/false) --> does this only use in VLAN_Stacking, because VLAN_XLATE is in another table.
- b) SVID field (valid only if a) is set) --> what's use for ? It shall cover in VLAN_STACKING table.
- c) SVID override priority (valid only if a) is set) --> what's use for ? It shall cover in VLAN_STACKING table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@venkatmahalingam After internal discuss, we have the questions,
- a) customer edge port (true/false) --> does this only use in VLAN_Stacking, because VLAN_XLATE is in another table.
- b) SVID field (valid only if a) is set) --> what's use for ? It shall cover in VLAN_STACKING table.
- c) SVID override priority (valid only if a) is set) --> what's use for ? It shall cover in VLAN_STACKING table.
VLAN stacking can be done in two ways,
-
Upon getting traffic with any CVID on the port/port-channel (customer edge port), push SVID in the ingress stage, and on the egress stage, SVID should be popped, if we don't want to inherit CVID prioirty, SVID priority should be configured explicitly.
-
VLAN_STACKING is an alternative method to do the SVID mapping for a given [cvid list, port/port-channel]
VLAN_STACKING -> Keys: 1) Port/Portchannel 2) SVID
fields: cvid leaf-list
If you have any further questions, we can discuss in the meeting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please go ahead and update the HLD for review again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@venkatmahalingam
Regarding to the VLAN_Transition we will use the following table.
VLAN_TRANSLATION:
key: port + svid
cvid: int
If you feel well for this, we will also re-define the db.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@venkatmahalingam Hi, I'm update the DB schema, please have a look for this. If this is ok, please also help to approve this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pttch I don't see the below config option in the HLD, this will satisfy the requirement for adding a s-tag for traffic with any c-tag coming on the port/port-channel. Please let us know if you have any plan to take care of this as well in this HLD.
Enhance PORT & PORTCHANNEL table to include the fields
a) customer edge port (true/false)
b) SVID field (valid only if a) is set)
c) SVID override priority (valid only if a) is set) -> (if this field is not configured, the default behavior is to
inherit priority from C-tag)
-> Upon getting any C-tag on this port/port-channel, SVID should be pushed on the ingress stage and the same should be popped on the egress stage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@venkatmahalingam
a) customer edge port (true/false)
b) SVID field (valid only if a) is set)
--> It can use vlan range setting for all c-vlan, this example show all c-vlan will push s-vlan 100 in Ethernet1 and strips S-VLAN 100 in egress side.
"VLAN_STACKING": {
"Ethernet1|100": {
"c_vlanids": ["1..4095"],
"s_vlan_priority": "0"
}
}
c) SVID override priority (valid only if a) is set) -> (if this field is not configured, the default behavior is to
inherit priority from C-tag)
--> Yes, the sai do not implement this, we will do this in the future.
; field = value | ||
c_vlanids = vlan_id-or-range[,vlan_id-or-range]* ; list of VLAN IDs | ||
s_vlan_priority = %x30-37 ; a number between 0 and 7 | ||
vlan_id = %x31-39 ; 1-9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vlan_id is not a field, please align accordingly.
A new table `VLAN_STACKING` is added in APP DB. | ||
|
||
``` | ||
VLAN_STACKING_TABLE:{{interface-name}}:{{s_vlanid}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APP DB should use "|" key separator.
admin@root:~# show vlan-stacking | ||
Interface s_vlanid c_vlanids s_vlan_priority | ||
----------- -------- ----------- ---------------- | ||
Ethernet0 200 100 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we expect to inherit the C-VLAN priority to S-VLAN as well, how do we show it in the output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@venkatmahalingam The value is must the behavior is override, you can read in cli spec
Usage: config vlan-stacking add [OPTIONS] <interface_name> <s_vlanid> <c_vlanids> <s_vlan_priority>
|
||
import sonic-port { | ||
prefix port; | ||
revision-date 2019-07-01; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove revision-date from YANG.
leaf s_vlan_priority { | ||
type uint8 { | ||
range 0..7; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default behaviour?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@venkatmahalingam The default behavior is override.
|
||
ext:key-regex-configdb-to-yang "^([a-zA-Z0-9_-]+)|([a-zA-Z0-9_-]+)|([0-9]+)$"; | ||
|
||
ext:key-regex-yang-to-configdb "<interface_name>|<s_vlanid>"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think we need this, please check in the existing SONiC YANG models.
path /port:sonic-port/port:PORT/port:PORT_LIST/port:port_name; | ||
} | ||
type leafref { | ||
path /lag:sonic-portchannel/lag:PORTCHANNEL/lag:PORTCHANNEL_LIST/lag:portchannel_name; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add description for all fields.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@venkatmahalingam Will update format issue in next commit.
All feedback from the community review has been incorporated by EdgeCore. Thanks, |
|
||
This document describes the design details of the VLAN stacking feature. VLAN stacking is a feature designed for service providers who carry traffic of multiple customers across their networks and are required to maintain the VLAN and Layer 2 protocol configurations of each customer without impacting the traffic of other customers. VLAN stacking include QinQ feature and VLAN Translation feature. | ||
|
||
IEEE 802.1Q tunneling (QinQ tunneling) uses a single Service Provider VLAN (SPVLAN) for customers who have multiple VLANs. Customer VLAN IDs are preserved and traffic from different customers is segregated within the service provider’s network even when they use the same customer-specific VLAN IDs. QinQ tunneling expands VLAN space by using a VLAN-in-VLAN hierarchy, preserving the customer’s original tagged packets, and adding SPVLAN tags to each frame (also called double tagging). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we planning to make any changes in the kernel for VLAN stacking/translation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, no, is there any concern for change the kernel code ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, we create an entry in the kernel for things we program in the HW e.g VLAN, in this case, we assume that we don't need VLAN stacking support for CPU originated traffic, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@venkatmahalingam Yes, VLAN stacking do not need for CPU traffic.
@pttch would you please add the code PRs by referring to EVPN VxLAN update for platforms using P2MP tunnel based L2 forwarding by dgsudharsan · Pull Request #806 · Azure/SONiC (github.com) as an example |
} | ||
|
||
leaf c_vlanid { | ||
type leafref { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is expected out of leafref here? Are you referring to the vlan id syntax or the existence of c_vlanid at /vlan:sonic-vlan/vlan:VLAN/vlan:VLAN_LIST/vlan:vlanid; ?
c_vlanid will not be known to the system.. and leafref should point to a key.. im not sure, if vlanid is a key here.
A new table `VLAN_TRANSLATION` is added in Config DB. | ||
|
||
``` | ||
VLAN_TRANSLATION|{{interface_name}}|{{s_vlanid}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we include Direction as the key in both VLAN_TRANSLATION & VALN_STACKING tables ? SAI proposal has the support for this via the attribute SAI_VLAN_STACK_ATTR_STAGE.
Some of the vendors support the partial translation rules, where the rule applies for Ingress direction only or for Egress direction only.
Regarding PR tracking for sonic-sairedis it looks like sonic-net/sonic-sairedis#1076 has been closed in favor of sonic-net/sonic-sairedis#1103 |
SONiC: Add VLAN Stacking HLD
Signed-off-by: pttch@edge-core.com