[PATCH v2 0/2] Improved improvements to RAdv
Hello, I'm sending a second version of my patch, after discussing the first version in person. I'll use this space to elaborate on the thoughts I have about my changes that I omitted from the first version. Again the commits are available on <https://gitea.ledoian.cz/LEdoian/bird> on branch `ledo-patch-v2`. The first commit is still the same, I'm sending it again just for completeness. For the second commit, I tried to keep the changes minimal, and we agreed to forbid declaring the `default prefix` options after any prefix is already configured, to keep the declarative feel of the config. I still feel somewhat uncertain about the particular wording of `default prefix {}`: I tried to avoid using the word `default` in a place where a prefix could be in order to not mistake this for the ::/0 prefix which behaves differently, but now it's quite similar to `default preference` and `default lifetime`, which are a different kinds of default. Also I'm not sure if I'm sticking to conventions and idiosyncrasies of BIRD, e.g. the defaults are now defined in `config.Y`. I was also thinking about whether it would make sense to control prefix options with attributes and/or filters and while I see some utility in that (mostly slow discontinuation of prefixes), I'm not sure that's what BIRD's task should be. There are also use cases that are possible only with configurations that feel unclean (when a prefix is not static, the only way is to match it against something very general like ::/0 or 2000::/3), but I'm not sure if it causes any problems that actually occur (e.g. it is not at the moment possible to only enable autoconfiguration when the dynamic prefix's length is in some pre-defined range, but does anyone actually need that?) As before, I'd appreciate guidance/tips about what would be a good way to write test for my contribution, as all evidence of it working has only been anecdotal so far. Best, LEdoian LEdoian (2): Docs: Add note about when RAdv keeps prefix RAdv: Make default prefix options configurable doc/bird.sgml | 15 ++++++++++++++- proto/radv/config.Y | 27 ++++++++++++++++++++++----- proto/radv/radv.c | 14 ++++++-------- proto/radv/radv.h | 1 + 4 files changed, 43 insertions(+), 14 deletions(-) -- 2.55.0
This was technically implied at the `prefix linger time` option, but felt quite non-obvious. --- doc/bird.sgml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/bird.sgml b/doc/bird.sgml index 660ea4c60..a79c842cc 100644 --- a/doc/bird.sgml +++ b/doc/bird.sgml @@ -6322,7 +6322,8 @@ custom option type 38 value hex:0e:10:20:01:0d:b8:00:0a:00:0b:00:00:00:00; advertised. This is useful for making exceptions from a default policy of advertising all prefixes. Note that for withdrawing an already advertised prefix it is more useful to advertise it with zero valid - lifetime. Default: no + lifetime (when reconfiguring, this happens automatically until + <ref id="radv-prefix-linger-time" name="prefix linger time"> times out). Default: no <tag><label id="radv-prefix-onlink">onlink <m/switch/</tag> This option specifies whether hosts may use the advertised prefix for -- 2.55.0
Before this, it was necessary to include all the differences from the hardcoded defaults for each prefix, as there was no inheritance. That could be an issue for deployments that don't use the BIRD's defaults (e.g. not using SLAAC or using different lifetimes). The new option `default prefix` allows overriding BIRD's default in the instance's config. This also made it possible to only have BIRD's defaults in a single place – the copy in `radv.c` is no longer useful. --- doc/bird.sgml | 12 ++++++++++++ proto/radv/config.Y | 27 ++++++++++++++++++++++----- proto/radv/radv.c | 14 ++++++-------- proto/radv/radv.h | 1 + 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/doc/bird.sgml b/doc/bird.sgml index a79c842cc..2a7b39154 100644 --- a/doc/bird.sgml +++ b/doc/bird.sgml @@ -6137,6 +6137,11 @@ definitions, prefix definitions and DNS definitions: prefix definition is matching if the network prefix is a subnet of the prefix in prefix definition. + <tag><label id="radv-default-prefix">default prefix { <m/options/ }</tag> + This allows to alter the default prefix options for this instance of RAdv + and has to be specified above any prefix options (and at most once in the + instance). + <tag><label id="radv-rdnss">rdnss { <m/options/ }</tag> RDNSS definitions allow to specify a list of advertised recursive DNS servers together with their options. As options are seldom necessary, @@ -6426,6 +6431,11 @@ protocol radv { propagate routes yes; # Propagate the routes from the radv_routes table ipv6 { table radv_routes; export all; }; + default prefix { + preferred lifetime 3600; # use shorter lifetimes for all networks + valid lifetime 14400; + }; + interface "eth2" { max ra interval 5; # Fast failover with more routers managed yes; # Using DHCPv6 on eth2 @@ -6442,6 +6452,8 @@ protocol radv { prefix 2001:0DB8:2000::/48 { autonomous off; # Do not autoconfigure + preferred lifetime 14400; # use conventional lifetimes + valid lifetime 86400; }; rdnss 2001:0DB8:1234::10; # Short form of RDNSS diff --git a/proto/radv/config.Y b/proto/radv/config.Y index ee1e273c1..6c928a39d 100644 --- a/proto/radv/config.Y +++ b/proto/radv/config.Y @@ -24,6 +24,7 @@ static struct radv_rdnss_config this_radv_rdnss; static struct radv_dnssl_config this_radv_dnssl; static list radv_dns_list; /* Used by radv_rdnss and radv_dnssl */ static u8 radv_mult_val; /* Used by radv_mult for second return value */ +static bool radv_default_forbidden; static inline void radv_add_to_custom_list(list *l, int type, const struct adata *payload) @@ -63,6 +64,14 @@ radv_proto_start: proto_start RADV init_list(&RADV_CFG->rdnss_list); init_list(&RADV_CFG->dnssl_list); init_list(&RADV_CFG->custom_list); + + RADV_CFG->default_prefix_config = cfg_allocz(sizeof(struct radv_prefix_config)); + RADV_CFG->default_prefix_config->onlink = 1; + RADV_CFG->default_prefix_config->autonomous = 1; + RADV_CFG->default_prefix_config->valid_lifetime = DEFAULT_VALID_LIFETIME; + RADV_CFG->default_prefix_config->preferred_lifetime = DEFAULT_PREFERRED_LIFETIME; + + radv_default_forbidden = false; }; radv_proto_item: @@ -70,6 +79,7 @@ radv_proto_item: | proto_channel | nbrs_channel | INTERFACE radv_iface + | DEFAULT PREFIX radv_default_prefix | PREFIX radv_prefix { add_tail(&RADV_CFG->pref_list, NODE this_radv_prefix); } | RDNSS { init_list(&radv_dns_list); } radv_rdnss { add_tail_list(&RADV_CFG->rdnss_list, &radv_dns_list); } | DNSSL { init_list(&radv_dns_list); } radv_dnssl { add_tail_list(&RADV_CFG->dnssl_list, &radv_dns_list); } @@ -208,13 +218,10 @@ CF_CLI(SHOW RADV INTERFACES, optproto opttext, [<name>] [\"<interfaces>\"], [[Sh radv_prefix_start: net_ip6 { + radv_default_forbidden = true; this_radv_prefix = cfg_allocz(sizeof(struct radv_prefix_config)); + memcpy(RADV_PREFIX, RADV_CFG->default_prefix_config, sizeof(struct radv_prefix_config)); RADV_PREFIX->prefix = *(net_addr_ip6 *) &($1); - - RADV_PREFIX->onlink = 1; - RADV_PREFIX->autonomous = 1; - RADV_PREFIX->valid_lifetime = DEFAULT_VALID_LIFETIME; - RADV_PREFIX->preferred_lifetime = DEFAULT_PREFERRED_LIFETIME; }; radv_prefix_item: @@ -254,6 +261,16 @@ radv_prefix_opt_list: radv_prefix: radv_prefix_start radv_prefix_opt_list radv_prefix_finish; +radv_default_prefix_start: +{ + if (radv_default_forbidden) + cf_error("Re-defining default options for prefixes or declaring them after prefix blocks is not supported"); + radv_default_forbidden = true; + this_radv_prefix = RADV_CFG->default_prefix_config; +}; + +radv_default_prefix: + radv_default_prefix_start radv_prefix_opt_list radv_prefix_finish; radv_rdnss_node: ipa diff --git a/proto/radv/radv.c b/proto/radv/radv.c index 499aeaea5..cd6cfd218 100644 --- a/proto/radv/radv.c +++ b/proto/radv/radv.c @@ -88,13 +88,6 @@ radv_nbr_timer(timer *tm) radv_prune_neighbors(p); } -static struct radv_prefix_config default_prefix = { - .onlink = 1, - .autonomous = 1, - .valid_lifetime = DEFAULT_VALID_LIFETIME, - .preferred_lifetime = DEFAULT_PREFERRED_LIFETIME -}; - static struct radv_prefix_config dead_prefix = { }; @@ -114,7 +107,12 @@ radv_prefix_match(struct radv_iface *ifa, net_addr_ip6 *px) if (net_in_net_ip6(px, &pc->prefix)) return pc; - return &default_prefix; + struct radv_prefix_config *ret = cf->default_prefix_config; + if (!ret) { + log(L_BUG "RAdv: default config for prefixes was not allocated"); + return &dead_prefix; + } + return ret; } /* diff --git a/proto/radv/radv.h b/proto/radv/radv.h index 69b5f9f86..a0c46220d 100644 --- a/proto/radv/radv.h +++ b/proto/radv/radv.h @@ -56,6 +56,7 @@ struct radv_config net_addr trigger; /* Prefix of a trigger route, if defined */ u8 propagate_routes; /* Do we propagate more specific routes (RFC 4191)? */ u32 max_linger_time; /* Maximum of interface route_linger_time */ + struct radv_prefix_config *default_prefix_config; /* base config for prefixes */ }; struct radv_iface_config -- 2.55.0
participants (1)
-
LEdoian