Blueprint (discuss on issue) mef_eline VLAN range EVPL

[viniarck]

Blueprint mef_eline VLAN range EVPL

This is for extending mef_eline to support VLAN range when creating an EVPL.

The API should be extended, of_core, flow_manager should support OF mask match on vlan.

This blueprint should identify the use case, how translations will work, NApps dependencies involved, what needs to be changed, and what's already supported.

I'll assign this one to @ajoaoff since he's started an initial assessment.

[viniarck]

Similar to kytos-ng/mef_eline#18. Since we won't have a blueprint, but will only discuss on issue, the discussion can take place on this issue or on the linked one, and then we can close one as duplicated.

[ajoaoff]

of_core seems to support masks on all match fields https://github.com/kytos-ng/of_core/blob/master/v0x04/match_fields.py#L30-L62

[ajoaoff]

I have tested flow_manager with VLAN masks and the only error I found was the pydantic model that has dl_vlan: Optional[int]
When I changed the field to str, flow_manager was able to install the flow.
I removed it manually to test the consistency and it also worked, the flow was reinstalled.

[viniarck]

I have tested flow_manager with VLAN masks and the only error I found was the pydantic model that has dl_vlan: Optional[int] When I changed the field to str, flow_manager was able to install the flow. I removed it manually to test the consistency and it also worked, the flow was reinstalled.

@ajoaoff nice, and thanks for anticipating that PR. If you could also break down the dependencies or how the feature will behave (consolidating some of the information that you also shared and researched some time ago on Slack and clarified requirements with Jeronimo), much appreciated. cc'ing also @Alopalao @italovalcy @gretelliz since they're working on related mef VLAN untagged/any and or sdntrace_cp.

[ajoaoff]

Regarding sdntrace_cp, it is used by mef_eline to evaluate if the path is available. This is done by using a sample "packet" matching the UNI and performing a match in each switch. As it is a packet, we can't have a VLAN range in it, just a single VLAN. What do we want here? A single vlan within the range is enough for us to assume the circuit is up? Or we want to test every possible vlan?

[viniarck]

Regarding sdntrace_cp, it is used by mef_eline to evaluate if the path is available. This is done by using a sample "packet" matching the UNI and performing a match in each switch. As it is a packet, we can't have a VLAN range in it, just a single VLAN. What do we want here? A single vlan within the range is enough for us to assume the circuit is up? Or we want to test every possible vlan?

I have an impression that a single VLAN should suffice, otherwise we'd be ignoring at the application level that a concept of a matched range is supported by the switches. In terms request payload, probably sdntrace_cp and sdntrace should still be supporting only a single dl_vlan: str, but then sdntrace_cp being able to match accordingly. cc'ing @gretelliz

@italovalcy @jab1982 would you be OK with a single vlan in the payload of traces? based on how you guys will use in production do you see any issues with this approach?

[viniarck]

Also, as a range of VLANs, are we only considering a contiguous [low, high] range or more flexibility is needed?

[italovalcy]

Regarding sdntrace_cp, it is used by mef_eline to evaluate if the path is available. This is done by using a sample "packet" matching the UNI and performing a match in each switch. As it is a packet, we can't have a VLAN range in it, just a single VLAN. What do we want here? A single vlan within the range is enough for us to assume the circuit is up? Or we want to test every possible vlan?

Hey @ajoaoff, good question. Thank you for bringing that up. Just one VLAN is enough, in my opinion. However, I suggest using a "random VLAN inside the expected range". That way we simplify the tests but we also probabilistically cover other possibilities though out the time. Wdyt?

[italovalcy]

Also, as a range of VLANs, are we only considering a contiguous [low, high] range or more flexibility is needed?

I would suggest more flexibility here, maybe something similar to what was defined for SDX data model (L2VPN):

"vlan_range": [(1,100), (300,305), (1000,1500)]

If that turns out to be much more complicated, we can leverage the new VLAN string type to do something like:

 {"tag": {"value": "1-100,200-300,400", "tag_type": 1}

[italovalcy]

Some points of attention I see:

• UI should be updated to support the VLAN range
• VLAN range should keep compatibility with the failover path whenever applicable
• We have to add some end-to-end tests for this feature
• We should check the flow_stats Zabbix integration wrapper for compatibility when accounting for the traffic of an EVC - to make sure all flows will be considered

[italovalcy]

Regarding sdntrace_cp, it is used by mef_eline to evaluate if the path is available. This is done by using a sample "packet" matching the UNI and performing a match in each switch. As it is a packet, we can't have a VLAN range in it, just a single VLAN. What do we want here? A single vlan within the range is enough for us to assume the circuit is up? Or we want to test every possible vlan?

Hey @ajoaoff, good question. Thank you for bringing that up. Just one VLAN is enough, in my opinion. However, I suggest using a "random VLAN inside the expected range". That way we simplify the tests but we also probabilistically cover other possibilities though out the time. Wdyt?

UPDATE: Thinking about this I realized that maybe one trace per VLAN/MASK would fit better than one trace for the whole EVC. Explanation: since the goal of the trace is to validate consistency between mef_eline and flow_manager, we have to make sure all necessary flows for an EVC are there. For a EVPL with a single VLAN ID, we have two flows, and we run two traces: one from UNI_A and another one from UNI_Z; which checks the two flows. In the scenario of VLAN Range, we will probably have more than two flows. For example, suppose you request the following range:

1779-1799

Depending on the strategy to calculate the necessary flows, you may end up requiring more than two flows to implement that. Indeed, the following calculation is one of the possibilities (using Noviflow's syntax):

# plans 1779 to 1799 (using vlan range: 1779/12 + 1780/10 + 1784/9 + 1792/9, where /x means x bits set from the 12 available)
set config flow tableid 0 command add priority 200 matchfields in_port vlan_vid valuesmasks 3 1779 instruction apply_actions action output port 20 ofpff persistent
set config flow tableid 0 command add priority 200 matchfields in_port vlan_vid valuesmasks 20 1779 instruction apply_actions action output port 3 ofpff persistent
set config flow tableid 0 command add priority 200 matchfields in_port vlan_vid valuesmasks 3 1780-4092 instruction apply_actions action output port 20 ofpff persistent
set config flow tableid 0 command add priority 200 matchfields in_port vlan_vid valuesmasks 20 1780-4092 instruction apply_actions action output port 3 ofpff persistent
set config flow tableid 0 command add priority 200 matchfields in_port vlan_vid valuesmasks 3 1784-4088 instruction apply_actions action output port 20 ofpff persistent
set config flow tableid 0 command add priority 200 matchfields in_port vlan_vid valuesmasks 20 1784-4088 instruction apply_actions action output port 3 ofpff persistent
set config flow tableid 0 command add priority 200 matchfields in_port vlan_vid valuesmasks 3 1792-4088 instruction apply_actions action output port 20 ofpff persistent
set config flow tableid 0 command add priority 200 matchfields in_port vlan_vid valuesmasks 20 1792-4088 instruction apply_actions action output port 3 ofpff persistent

Thus, having one trace for each pair of flows (ingress-egress) in each direction makes more sense, in my opinion.

[viniarck]

Also, as a range of VLANs, are we only considering a contiguous [low, high] range or more flexibility is needed?

I would suggest more flexibility here, maybe something similar to what was defined for SDX data model (L2VPN):

"vlan_range": [(1,100), (300,305), (1000,1500)]

If that turns out to be much more complicated, we can leverage the new VLAN string type to do something like:

 {"tag": {"value": "1-100,200-300,400", "tag_type": 1}

Great suggestion, @italovalcy. In the API spec, I think the second suggested option {"tag": {"value": "1-100,200-300,400", "tag_type": 1} is probably easier to maintain, and since this can be also validated as a regex, which has also been shown to work correctly with OpenAPI validations that would be very aligned with what we currently have. Also since parsing this validated str as a unique list isn't that difficult then I believe it would fit well.

Internally, though, @ajoaoff, I'll recommend to try to potentially subclass UNI as UNIRange or something similar. But then the subclass introducing a self.values: List[TAG] and acting as container and overwrite the methods accordingly. That way everywhere whare a UNI object is being used, if a UNIRange is being used, then the caller can differentiate and use it for validation and getting the values. Each Interface object when using, releasing or checking if a tag a TAG object has been protected with a lock, so we'll also need to ensure that when checking if an entire vlan range is available that the same Interface._tag_lock is also used to avoid race conditions when checking if a range is available.

[viniarck]

Thus, having one trace for each pair of flows (ingress-egress) in each direction makes more sense, in my opinion.

Agreed, this sounds better and easier to maintain too. With sdntrace_cp, currently with bulk traces it's always sending a pair of traces (uni_a -> uni_z and uni_z -> uni_a), so it's a matter of sending additional traces and gather the results accordingly. Also, when checking for the success of a trace, when a uni has a range, then any trace result should not not overwrite a false result that for the circuit, adapting that part is relatively simple. Good thing that with bulk trace it's only a single round trip. Btw, thanks for providing the example on NoviFlow.

[viniarck]

@italovalcy @ajoaoff, should UNI vlan range always be the same size on both ends? for an EVPL we currently support asymmetric VLANs UNI since it always knows the expected C-VLAN on the other UNI. I think we need to confirm both if the length needs to be the same and also if they can be asymmetrical (different values) or if symmetrical is sufficient.

[italovalcy]

@italovalcy @ajoaoff, should UNI vlan range always be the same size on both ends? for an EVPL we currently support asymmetric VLANs UNI since it always knows the expected C-VLAN on the other UNI. I think we need to confirm both if the length needs to be the same and also if they can be asymmetrical (different values) or if symmetrical is sufficient.

Hi @viniarck, good question, and great observation! IMHO, we should not support different ranges on the UNIs. If we use a VLAN range, both UNIs should have the same range. Even though we may have use cases for having different ranges, that would be very difficult to implement using OpenFlow rules. Thus, to make it simple and fully compatible with the current data plane implementation, my suggestion is that both UNIs have the same range. Then, when matching ingress traffic in a UNI, we just push the service tag VLAN and output to the next hop port (for intra-switch, we don't even need to push service tag VLAN) - instead of also applying the set_field VLAN as we currently have.

[viniarck]

IMHO, we should not support different ranges on the UNIs.

Sounds good, @italovalcy. Thanks for your input and confirming the use case.

@ajoaoff looks like you have all the requirements in place, feel free to start breaking down the tasks on GitHub in the epic_mef_eline_vlan_range epic. If you need anything else to confirm/refine the scope let us know, thanks.

[viniarck]

Also, as a range of VLANs, are we only considering a contiguous [low, high] range or more flexibility is needed?

I would suggest more flexibility here, maybe something similar to what was defined for SDX data model (L2VPN):

"vlan_range": [(1,100), (300,305), (1000,1500)]

If that turns out to be much more complicated, we can leverage the new VLAN string type to do something like:

 {"tag": {"value": "1-100,200-300,400", "tag_type": 1}

Great suggestion, @italovalcy. In the API spec, I think the second suggested option {"tag": {"value": "1-100,200-300,400", "tag_type": 1} is probably easier to maintain, and since this can be also validated as a regex, which has also been shown to work correctly with OpenAPI validations that would be very aligned with what we currently have. Also since parsing this validated str as a unique list isn't that difficult then I believe it would fit well.

Internally, though, @ajoaoff, I'll recommend to try to potentially subclass UNI as UNIRange or something similar. But then the subclass introducing a self.values: List[TAG] and acting as container and overwrite the methods accordingly. That way everywhere whare a UNI object is being used, if a UNIRange is being used, then the caller can differentiate and use it for validation and getting the values. Each Interface object when using, releasing or checking if a tag a TAG object has been protected with a lock, so we'll also need to ensure that when checking if an entire vlan range is available that the same Interface._tag_lock is also used to avoid race conditions when checking if a range is available.

Additional comments:

• Currently UNIs aren't consuming or releasing vlans from available_tags, it might also be worth trying to address mef_eline isn't allocating/releasing available_tags for UNIs mef_eline#324, to make it easier to check if a range is still available or not. We'll still need to shares_uni(other) to detect overlapping port-based circuits though, but at least the implementation won't be much more complicated.
• Regarding the potential UNIRange, since we don't have a reentrant lock with interface._tag_lock we'll likely need to add additional methods on Interface to use the same lock but taking an iterable as argument like def are_tags_available(self, tags: list[TAG]) -> bool, def use_tags(self, tags: list[TAG]) -> bool, and def make_tags_available(self, tags: list[TAG]) -> bool

[Alopalao]

Closing this issue, this discussion contains more details about updated features. The epic is in progress and soon to be finnished.