[PATCH] Babel: Allow v6 next hop on interfaces with v4 addresses

[Toke Høiland-Jørgensen]

Chris Webb <chris at arachsys.com> writes:

> By default, the Babel protocol will advertise an IPv6 extended next hop
> address on interfaces which don't have an IPv4 address. This is controlled
> by the 'extended next hop' option.
>
> Add support for 'extended next hop force' which advertises an extended
> next hop even when IPv4 addresses are present. (They may be temporary,
> for debugging or otherwise unsuitable for including in an
> advertisement.)

No objection to the feature, but one nit on the implementation, see
below:

> ---
>  doc/bird.sgml        |  9 ++++++---
>  proto/babel/babel.c  | 10 +++++++---
>  proto/babel/babel.h  |  2 +-
>  proto/babel/config.Y |  3 ++-
>  4 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/doc/bird.sgml b/doc/bird.sgml
> index 0d1e6f49..cb5fa572 100644
> --- a/doc/bird.sgml
> +++ b/doc/bird.sgml
> @@ -2266,7 +2266,7 @@ protocol babel [<name>] {
>  		check link <switch>;
>  		next hop ipv4 <address>;
>  		next hop ipv6 <address>;
> -		extended next hop <switch>;
> +		extended next hop <switch>|force;
>  		rtt cost <number>;
>  		rtt min <time>;
>  		rtt max <time>;
> @@ -2377,10 +2377,13 @@ protocol babel [<name>] {
>        source for Babel packets will be used. In normal operation, it should not
>        be necessary to set this option.
>  
> -      <tag><label id="babel-extended-next-hop">extended next hop <m/switch/</tag>
> +      <tag><label id="babel-extended-next-hop">extended next hop
> +      <m/switch/|force</tag>
>        If enabled, BIRD will accept and emit IPv4 routes with an IPv6 next
>        hop when IPv4 addresses are absent from the interface as described in
> -      <rfc id="9229">. Default: yes.
> +      <rfc id="9229">. Use <cf/extended next hop force/ to emit IPv4 routes
> +      with an IPv6 next hop even when IPv4 addresses are present on the
> +      interface. Default: yes.
>  
>        <tag><label id="babel-rtt-cost">rtt cost <m/number/</tag>
>        The RTT-based cost that will be applied to all routes from each neighbour
> diff --git a/proto/babel/babel.c b/proto/babel/babel.c
> index 4187d258..1541edc1 100644
> --- a/proto/babel/babel.c
> +++ b/proto/babel/babel.c
> @@ -1836,8 +1836,12 @@ babel_iface_update_addr4(struct babel_iface *ifa)
>  {
>    struct babel_proto *p = ifa->proto;
>  
> -  ip_addr addr4 = ifa->iface->addr4 ? ifa->iface->addr4->ip : IPA_NONE;
> -  ifa->next_hop_ip4 = ipa_nonzero(ifa->cf->next_hop_ip4) ? ifa->cf->next_hop_ip4 : addr4;
> +  if (ipa_nonzero(ifa->cf->next_hop_ip4))
> +    ifa->next_hop_ip4 = ifa->cf->next_hop_ip4;
> +  else if (ifa->cf->ext_next_hop < 2 && ifa->iface->addr4)
> +    ifa->next_hop_ip4 = ifa->iface->addr4->ip;
> +  else
> +    ifa->next_hop_ip4 = IPA_NONE;
>  
>    if (ipa_zero(ifa->next_hop_ip4) && p->ip4_channel && !ifa->cf->ext_next_hop)
>      log(L_WARN "%s: Missing IPv4 next hop address for %s", p->p.name, ifa->ifname);
> @@ -1912,7 +1916,7 @@ babel_add_iface(struct babel_proto *p, struct iface *new, struct babel_iface_con
>  
>    add_tail(&p->interfaces, NODE ifa);
>  
> -  ip_addr addr4 = new->addr4 ? new->addr4->ip : IPA_NONE;
> +  ip_addr addr4 = ic->ext_next_hop < 2 && new->addr4 ? new->addr4->ip : IPA_NONE;
>    ifa->next_hop_ip4 = ipa_nonzero(ic->next_hop_ip4) ? ic->next_hop_ip4 : addr4;
>    ifa->next_hop_ip6 = ipa_nonzero(ic->next_hop_ip6) ? ic->next_hop_ip6 : ifa->addr;
>  
> diff --git a/proto/babel/babel.h b/proto/babel/babel.h
> index edde4cab..f832a699 100644
> --- a/proto/babel/babel.h
> +++ b/proto/babel/babel.h
> @@ -163,7 +163,7 @@ struct babel_iface_config {
>  
>    ip_addr next_hop_ip4;
>    ip_addr next_hop_ip6;
> -  u8 ext_next_hop;			/* Enable IPv4 via IPv6 */
> +  u8 ext_next_hop;			/* Enable IPv4 via IPv6, set >= 2 to force */
>  
>    u8 auth_type;				/* Authentication type (BABEL_AUTH_*) */
>    u8 auth_permissive;			/* Don't drop packets failing auth check */
> diff --git a/proto/babel/config.Y b/proto/babel/config.Y
> index d412a54b..734c184b 100644
> --- a/proto/babel/config.Y
> +++ b/proto/babel/config.Y
> @@ -24,7 +24,7 @@ CF_DECLS
>  
>  CF_KEYWORDS(BABEL, INTERFACE, METRIC, RXCOST, HELLO, UPDATE, INTERVAL, PORT,
>  	TYPE, WIRED, WIRELESS, RX, TX, BUFFER, PRIORITY, LENGTH, CHECK, LINK,
> -	NEXT, HOP, IPV4, IPV6, BABEL_METRIC, SHOW, INTERFACES, NEIGHBORS,
> +	NEXT, HOP, FORCE, IPV4, IPV6, BABEL_METRIC, SHOW, INTERFACES, NEIGHBORS,
>  	ENTRIES, RANDOMIZE, ROUTER, ID, AUTHENTICATION, NONE, MAC, PERMISSIVE,
>  	EXTENDED, TUNNEL, RTT, MIN, MAX, DECAY, SEND, TIMESTAMPS, COST, DELAY)
>  
> @@ -159,6 +159,7 @@ babel_iface_item:
>   | NEXT HOP IPV4 ipa { BABEL_IFACE->next_hop_ip4 = $4; if (!ipa_is_ip4($4)) cf_error("Must be an IPv4 address"); }
>   | NEXT HOP IPV6 ipa { BABEL_IFACE->next_hop_ip6 = $4; if (!ipa_is_ip6($4)) cf_error("Must be an IPv6 address"); }
>   | EXTENDED NEXT HOP bool { BABEL_IFACE->ext_next_hop = $4; }
> + | EXTENDED NEXT HOP FORCE { BABEL_IFACE->ext_next_hop = 2; }

I think this should be turned into a proper enum (EXT_NEXT_HOP_OFF = 0,
EXT_NEXT_HOP_ON = 1, EXT_NEXT_HOP_FORCE = 2 or something like that),
instead of hard-coding the '2' everywhere.

-Toke