Skip to content
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

Bgp ecommlist count #18159

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pguibert6WIND
Copy link
Member

Add extensions in route-map for handling extended community-list in match operations:

  • add the exact-match and the any options
  • add the extcommunity-limit count

@pguibert6WIND pguibert6WIND marked this pull request as draft February 14, 2025 09:55
@pguibert6WIND pguibert6WIND marked this pull request as ready for review February 14, 2025 14:40
bgpd/bgp_clist.c Outdated
uint32_t i;

for (i = 0; i < ecom->size; i++) {
ptr = ecom->val + (i * ECOMMUNITY_SIZE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should handle IPv6-specific extended communities also. Thus ecom->unit_size should be used instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@@ -1168,8 +1168,10 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter)
bool unk_ecom = false;
memset(encbuf, 0x00, sizeof(encbuf));

if (number != -1 && (uint32_t)number != i)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean (number)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some more information, and renamed it to index.


path = (struct bgp_path_info *)object;

piextcomm = bgp_attr_get_ecommunity(path->attr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include also IPv6-address specific extended communities...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

doc/user/bgp.rst Outdated
@@ -2907,7 +2907,13 @@ Extended Community Lists
Note that all extended community lists shares a single name space, so it's
not necessary to specify their type when creating or destroying them.

.. clicmd:: show bgp extcommunity-list [NAME detail]
.. clicmd:: match community-limit (0-65535)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this duplicate? We already have this documented.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you re right. simplifying it.

@pguibert6WIND pguibert6WIND marked this pull request as draft February 15, 2025 06:48
@pguibert6WIND pguibert6WIND force-pushed the bgp_ecommlist_count branch 2 times, most recently from 4e2da9d to 336f122 Compare February 21, 2025 11:25
@pguibert6WIND pguibert6WIND marked this pull request as ready for review February 21, 2025 11:26
@pguibert6WIND pguibert6WIND force-pushed the bgp_ecommlist_count branch 2 times, most recently from 7200c52 to a46d82e Compare February 24, 2025 07:12
doc/user/bgp.rst Outdated
@@ -2918,7 +2918,20 @@ Extended Community Lists
BGP Extended Communities in Route Map
"""""""""""""""""""""""""""""""""""""

.. clicmd:: match extcommunity WORD
.. clicmd:: match extcommunity WORD [exact-match]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any also?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

.. clicmd:: match extcommunity WORD
.. clicmd:: match extcommunity WORD [exact-match]

This command perform match to BGP updates using extended community list WORD.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

performs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

doc/user/bgp.rst Outdated

This command perform match to BGP updates using extended community list WORD.
When the one of BGP extended communities value match to the one of the extended
communities value in community list, it is match. When `exact-match` keyword is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To highlight, we should use double ticks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Add a mechanism in route-map to filter out route-map which have a list
of extended communities greater than the given number.

Signed-off-by: Philippe Guibert <[email protected]>
It should be possible to configure the community-limit count to 0.

Fixes: f19b866 ("bgpd: add 'match community-count' command to restrict comm count")
Signed-off-by: Philippe Guibert <[email protected]>
The exact-match and the any options are missing for the extended
communities. Add missing options that are present on the match
operations for communities and large-communities.

> route-map rmap permit 1
>  match extcommunity 1
> exit
> !
> route-map rmap permit 2
>  match extcommunity 2 any
> exit
> !
> route-map rmap permit 3
>  match extcommunity 3 exact-match
> exit

Signed-off-by: Philippe Guibert <[email protected]>
Add a test suite that checks that it is possible to filter out
BGP updates based on the extcommunity-list match operation of the
route-map. Check also the extcommunity-limit option.

Signed-off-by: Philippe Guibert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants