openconfig / public

Repository for publishing OpenConfig models, documentation, and other material for the community.
Apache License 2.0
905 stars 656 forks source link

Clarify what "match-set-options=ALL" means when "community-member" is a list of regexp #875

Closed xchu-arista closed 4 months ago

xchu-arista commented 1 year ago

In routing-policy/defined-sets/bgp-defined-sets we have

pyang -p third_party/ietf -p release release/models/*/*.yang -f tree --tree-path /routing-policy/defined-sets/bgp-defined-sets
module: openconfig-routing-policy
  +--rw routing-policy
     +--rw defined-sets
        +--rw oc-bgp-pol:bgp-defined-sets
           +--rw oc-bgp-pol:community-sets
           |  +--rw oc-bgp-pol:community-set* [community-set-name]
           |     +--rw oc-bgp-pol:community-set-name    -> ../config/community-set-name
           |     +--rw oc-bgp-pol:config
           |     |  +--rw oc-bgp-pol:community-set-name    string
           |     |  +--rw oc-bgp-pol:community-member*     union
           |     |  +--rw oc-bgp-pol:match-set-options?    oc-pol-types:match-set-options-type
           |     +--ro oc-bgp-pol:state
           |        +--ro oc-bgp-pol:community-set-name    string
           |        +--ro oc-bgp-pol:community-member*     union
           |        +--ro oc-bgp-pol:match-set-options?    oc-pol-types:match-set-options-type
           +--rw oc-bgp-pol:ext-community-sets
           |  +--rw oc-bgp-pol:ext-community-set* [ext-community-set-name]
           |     +--rw oc-bgp-pol:ext-community-set-name    -> ../config/ext-community-set-name
           |     +--rw oc-bgp-pol:config
           |     |  +--rw oc-bgp-pol:ext-community-set-name?   string
           |     |  +--rw oc-bgp-pol:ext-community-member*     union
           |     |  +--rw oc-bgp-pol:match-set-options?        oc-pol-types:match-set-options-type
           |     +--ro oc-bgp-pol:state
           |        +--ro oc-bgp-pol:ext-community-set-name?   string
           |        +--ro oc-bgp-pol:ext-community-member*     union
           |        +--ro oc-bgp-pol:match-set-options?        oc-pol-types:match-set-options-type

Specifically, match-set-options=ALL means "match is true if given value matches all members of the defined set" (see https://github.com/openconfig/public/blob/master/release/models/policy/openconfig-policy-types.yang#L102-L121).

This is straightforward when leaf-list community-member is just a list of community values. For all members of the defined set to match the given value, we just need leaf-list community-member to be a subset of the BGP community path.

However, it's unclear what match-set-options=ALL means when leaf-list community-member is a list of regular expressions (see https://github.com/openconfig/public/blob/master/release/models/bgp/openconfig-bgp-policy.yang#L443-L472).

To me this doesn't seem to make much sense semantically. If this is indeed something meaningful we can configure, then a clarification in the description section can be really helpful.

Tagging @dplore because we chatted about this offline.

rolandphung commented 1 year ago

@dplore for vis

dplore commented 1 year ago

Agreed, when using regex, it only seems useful to use match ANY. Will bring up in OC Operator call on May 23

dplore commented 1 year ago

This proposal was reviewed without objection. I have raised #891 to address it. Please provide comments there.

github-actions[bot] commented 6 months ago

This issue is stale because it has been open 180 days with no activity. If you wish to keep this issue active, please remove the stale label or add a comment, otherwise will be closed in 14 days.