Chris Webb <ch...@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
