pierky / arouteserver

A tool to automatically build (and test) feature-rich configurations for BGP route servers.
https://arouteserver.readthedocs.org/
GNU General Public License v3.0
284 stars 46 forks source link

Refactor ext-communities for OpenBGPd #98

Closed stkonst closed 2 years ago

stkonst commented 2 years ago

Hi @pierky

It seems that extended communities in OpenBGPd do not support macros. For example, the following snippet: `

Transform NO_EXPORT into $INTCOMM_NO_EXPORT

match from group clients community NO_EXPORT set { ext-community $INTCOMM_NO_EXPORT community delete NO_EXPORT }

Transform NO_ADVERTISE into $INTCOMM_NO_ADVERTISE

match from group clients community NO_ADVERTISE set { ext-community $INTCOMM_NO_ADVERTISE community delete NO_ADVERTISE } `

It raises the following syntax errors on OpengBGPd (version 7.2):

root@bgpdvm:~# /usr/local/sbin/bgpd -v -f openbgpd_config.conf INTCOMM_PREF_OK_ROA = "65535:1" INTCOMM_ROUTE_OK_WL = "65535:2" INTCOMM_PREF_OK_ARINDB = "65535:3" INTCOMM_PREF_OK_REGISTROBRDB = "65535:12" INTCOMM_ORIGIN_OK = "65535:4" INTCOMM_ORIGIN_KO = "65535:5" INTCOMM_PREFIX_OK = "65535:6" INTCOMM_PREFIX_KO = "65535:7" INTCOMM_IRR_REJECT = "65535:8" INTCOMM_RPKI_UNKNOWN = "65535:9" INTCOMM_RPKI_INVALID = "65535:10" INTCOMM_RPKI_VALID = "65535:11" INTCOMM_NO_EXPORT = "65535:65281" INTCOMM_NO_ADVERTISE = "65535:65282" openbgpd_config.conf:420: syntax error <----- openbgpd_config.conf:426: syntax error <-----

When disabling the ext-communities, the errors are gone. Upon checking the following documentation of OpenBGPd: https://www.freebsd.org/cgi/man.cgi?query=bgpd.conf&apropos=0&sektion=5&manpath=FreeBSD+7-current&format=html

Manual page says that it accepts the following params: ext-community [delete] subtype as-number:local ext-community [delete] subtype IP:local ext-community [delete] subtype numvalue ext-community [delete] ovs (valid | not-found | invalid)

The attribute "name" is not listed (which explains the syntax error), however it is listed in communities and large-communities: community [delete] name .... large-community [delete] name

I tried to disable globally the ext-communities but it seems that there is no such knob in the general.yml

If the above holds true, then I have 2 recommendations to propose:

1) Introduce a knob to disable ext-communities completely. Let the script do not generate any config related to the ext-communities 2) When ext-communities are activated, use their real value in the config instead of the macro reference.

Thank's in advance.

pierky commented 2 years ago

Hi @stkonst, would you mind sharibg the general.yml file and clients.yml file that you used when you got the error? You can send them pruvately to me if you prefer: pierky (@) pierky (dot) com.

I'd like to reproduce the issue. It's very weird, because the config is tested succesfully under many scenarios. Thanks.

stkonst commented 2 years ago

Files sent to mail address. Thank's for the response and looking forward for the feedback.

pierky commented 2 years ago

Thanks, I’m on it, it will take bit of time, it’s a corner case triggered by the config of prepending communities.

pierky commented 2 years ago

Hello @stkonst,

I was able to reproduce the issue using the config files you shared, and I think it should be fixed now. The CI/CD pipeline is running, and if everything goes well the pre-release v1.13.1-alpha2 should be pushed to the test instance of PyPi in about one hour. As for the other issues that you recently reported, it'd be great if you could try it out: instructions on how to install it from the test PyPi instance can be found in the docs here.

The issue was triggered by a bug in the way prepending standard communities were processed for the outbound policy towards 32bit ASN clients.

From the commit message:

A general.yml configuration on which prepend communities were
configured with only 'std' commmunities...

    prepend_once_to_any:
      std: "65501:65501"

... led to a broken OpenBGPD configuration for the outbound
policy towards 32bit ASNs:

    # prepend_once_to_any; remove further "prepend" communities from matching routes
    match to 192.0.2.1 community 65501:65501 set prepend-neighbor 1
    match to 192.0.2.1 community 65501:65501 set {  }

The error was on the line of configuration where all the prepend
communities are removed from routes that already matched a prepend
community. The resulting 'set {}' statement on that line was empty
because the 32bit ASN for which the outbound policy was generated
was preventing the 'std' community from being matched, even though
it shouldn't matter for 'std' communities not having a 'peer_as'
part.

I don't think the issue was due to what you mentioned in the text here on GitHub. OpenBGPD macros seem well supported in the processing of all the types of communities. Indeed, using the same input files you shared, I was able to build and validate the configuration that was generated after the fix (tested with OpenBGPD 7.2 portable edition).

Please let me know if the issue persists even after the fix I've implemented here.

Thanks a lot.

pierky commented 2 years ago

Update: 1.31.1-alpha2 is out on the test instance of PyPi.

pierky commented 2 years ago

I'm planning to release a further improvement to this fix in 1.14.0, where I'm going to change the way prepending communities are removed when the first one matches.

pierky commented 2 years ago

v1.13.1 is out with this fix/improvement.