public icon indicating copy to clipboard operation
public copied to clipboard

Clarify use match-set-options=ALL with regex

Open dplore opened this issue 1 year ago • 10 comments

  • (M) openconfig-policy-types.yang

match-set-options=ALL is not supported when used with oc-bgp-types:bgp-community-regexp-type

Resolves #875

Scope of Change

There is ambiguity in the OC model as to how matches are intended to be performed. Sorry for the long explanation below, but this hopefully clearly explains it considering OC policy match vs. what NOS are doing today.

I believe the logic issue can be explained this way:

Match behavior for match-set-options-type=ANY

There is no issue with the ANY behavior. I present this as a case which is well understood as background for the motivation for this PR.

match-set-options-type=ANY The device should compare each item in a set of values with a list of match-set criteria. If one or more values match one or more of the elements in the match-set, then the match operation is true. In other words, a logical "OR" operation is used both for comparing a value with each match-set and across all the values.

For example, a prefix contains two communities: 5413:1, 29636:2 and the match-set contains 5413:[01] and 5555:[12] match-set-options-type=ANY will return a match because 5413:1 matches regex 5413:[01].

Match behavior for match-set-options-type=ALL

match-set-options-type=ALL requires a logical "AND" operation to be performed. But it's ambiguous to me exactly how OC expects this to be performed. There are two options I can see.

  1. The device should compare each item in a list of values with the list of match criteria. ALL of the match criteria must be true for a given value in the list for a match to be true.
  2. The device should compare each item in a list of values with ALL the items in the list of match criteria. All of the match criteria must match at least one, but not all values in the value list.
  3. The device should compare each item in the list of values with the match criteria. ALL of the values in the list must match at least one match criteria.

In option 1, multiple regexes aren't logical because they must have some common/overlapping criteria in order for one value to match multiple regexes. In this case it is more simple just have just one regex with the desired match criteria.
This is the motivation for this PR. However, it is not clear if OC requires this match behavior.

In option 2, using multiple regexes seems logical.

In option 3, using multiple regexes seems logical.

Cisco IOS XR supports behavior like option 2, but only for community ranges, not for regex.

JunOS's policy match implementation for regex specifies that ANY / OR is implemented when regex is in place. A match-set-options ALL is not supported when using regex.

Arista EOS ip extcommunity-list regexp allows defining a list of regular expressions. In a comment below, Arista indicates they support option 2 behavior.

Nokia SR Linux supports policy match using a community set with multiple regex entries but it is not clear what the match behavior is from the documention. Below @hellt describes that currently ALL is not implemented, but if they did implement it, it would follow option 3.

dplore avatar Jun 10 '23 05:06 dplore

Major YANG version changes in commit a1d1282adca014a6c57ae118979dd982e110fd25:

OpenConfigBot avatar Jun 10 '23 05:06 OpenConfigBot

Compatibility Report for commit a1d1282adca014a6c57ae118979dd982e110fd25: ⛔ yanglint@SO 1.10.17

OpenConfigBot avatar Jun 10 '23 05:06 OpenConfigBot

Can you explain why this is not logical?

i.e., if I were to write two regular expressions like 5413:[01] and 29636:[12] - could I not expect that communities that are associated with the route match both? i.e., there are multiple communities possible per route.

Do we rather mean that this is not commonly supported?

robshakir avatar Jun 28 '23 14:06 robshakir

Moved to 1st comment/main description for the PR

dplore avatar Jun 29 '23 01:06 dplore

@dplore in SR Linux case (unrelated to OC) the policy framework works as follows:

  1. A route R has communities C1 C2 C3
  2. community-set has a list of members [M1 M2], where M1 and M2 are regexp expressions
  3. SRL will match the route if any of C1, C2, or C3 matches any of M1 or M2.
  4. if/when we would support ‘ALL’ it would apply to the Cx list - i.e a route matches if C1 is matched by M1 OR M2, AND C2 is matched by M1 OR M2, and C3 is matched by M1 OR M2.

hellt avatar Jun 29 '23 17:06 hellt

There seem to be two discrete sets of functionality there -- thanks for highlighting the alternate @hellt.

If we say that ALL means that "all regexps within the set must be matched", then I think @dplore's analysis covers that case well. Alternatively, with @hellt's envisaged behaviour we're saying ALL means "all members of the community list must match one of the regexps specified".

I don't recall having seen an implementation that would do the "all members must match" option, so I'm OK with not allowing the "ALL" case here, which seems to be consistent with the other places that we say "ALL is not supported" like neighbour sets/prefix sets.

This all being said -- I note that community-set and ext-community-set seem to be the only place where we define this within the actual set, rather than the match. All the other matches that are in routing policy have the match-set-options specified outside of them. I can't remember the history here -- was this a change that was made specifically for these two types? I believe that there was some discussion of something like this previously.

robshakir avatar Jul 07 '23 00:07 robshakir

I updated my list of options to include the algorithm that @hellt specifies. and moved my options explanation to the first comment in this PR.

I'm awaiting feedback from @earies and @rolandphung before making more changes.

dplore avatar Jul 08 '23 01:07 dplore

@dplore Arista EOS ip community-list and ip extcommunity-list support match behaviors outlined in both the match-set-options=ANY case and match-set-options=ALL option 2 case.

When used with regexp type communities however, match-set-options=ALL is not supported.

xchu-arista avatar Jul 13 '23 14:07 xchu-arista

I can offer different view point here: Observe that community-set has leaf-list[]. Observe that match-set-type is attribute of community-set. Not a method. Hence ALL|ANY|INVERT match-set-type applys to elements of list, regardless if element is exact-value or regexp. Evaluation therefore is: check if any of path's community matches liaf-list[i] element. This produce array[] of true/false, one per each leaf-list element. Then apply match-set-type against this array[].

  • ALL then AND(array[]) (case 2)
  • ANY then OR(array[])
  • INVERT thaen !(OR(array[])).

Please note that communities are transitive attributes, hence in Internet context, one AS may add attribute to route and expect to be carried over internet. Thence logic that requires that ALL communities of path are matching is impractical. We can't know what community will appear next. But we may ask that some communities C1 & C2 and C3 exist simultanously as attributes of routes.


Above logic will work well for regexp.

rszarecki avatar Aug 02 '23 17:08 rszarecki

@hellt and @rszarecki thank you both for your interpretations. They logical explanations of the match algorithm. There is a difference in the criteria.

@cfyo and I will provide some example configuration we are using operationally today to guide which options are required. We can then use this to clarify descriptions and/or change the model.

dplore avatar Aug 22 '23 17:08 dplore