On Thu, Jul 19, 2018 at 09:47:06PM +0200, Björn Ketelaars wrote:
> On Wed 18/07/2018 08:54, Florian Obser wrote:
> > During g2k18 I commited rad(8).
> >
> > The latest amd64 and i386 snapshots should contain it with enough
> > features to replace rtadvd(8). If you are using rtadvd(8) I'd
> > appreciate if you could switch to rad(8) and report back if any
> > features are missing.
>
> Florian,
>
> Thank you for rad(8). I like the configuration format, and it seems to
> work for me as replacement for rtadvd(8). For reference purposes:
>
> $ cat /etc/rtadvd.conf
> vlan1003:\
> :mtu#1480:\
> :rdnss="2001:470:7ce4:1::1":\
> :dnssl="guest":
>
> $ cat /etc/rad.conf
> interface vlan1003 {
> dns {
> resolver 2001:470:7ce4:1::1
> search guest
> }
> }
>
> There is one thing that I'm missing, and that is the MTU option in RA
> messages (RFC 4861 Section 4.6.4). I've baked up a diff which works for
> me. My current rad.conf looks like:
>
> interface vlan1003 {
> dns {
> resolver 2001:470:7ce4:1::1
> search guest
> }
>
> mtu 1480
> }
>
> Output "tcpdump -vvvv -ttt -i vlan1003 icmp6 and 'ip6[40] = 134'":
>
> Jul 19 21:26:10.009348 fe80::a562:1f47:a75c:a5fc > ff02::1: icmp6: router
> advertisement(chlim=0, pref=medium, router_ltime=1800, reachable_time=0,
> retrans_time=0)(prefix info: LA valid_ltime=2592000, preferred_ltime=604800,
> prefix=2001:470:7ce4:1::/64)(mtu: mtu=1480)[ndp opt] (len 88, hlim 255)
>
> I'm not sure this is an option for rad(8) that is useful for anyone else
> but me. What do you think? Also, what do you think of the diff below?
I like it, I think we should have it. Out of curiosity, which clients
pay attention to the MTU option? I know that slaacd does not. If you
are sufficiently bored you might want to implement it there, too! ;)
(hint: you will need to add to the wroute pledge in the kernel)
Diff reads mostly ok but needs a few tweaks, see inline:
>
>
> diff --git engine.c engine.c
> index 5d186439dd8..4ade634a689 100644
> --- engine.c
> +++ engine.c
> @@ -355,6 +355,9 @@ engine_dispatch_main(int fd, short event, void *bula)
> SIMPLEQ_INSERT_TAIL(&ra_iface_conf->ra_prefix_list,
> ra_prefix_conf, entry);
> break;
> + case IMSG_RECONF_RA_MTU:
> + ra_iface_conf->ra_mtu = *((uint32_t *)imsg.data);
> + break;
no need for the extra IMSG, ra_mtu is not a substructure in
ra_iface_conf, it will be copied with the rest of the ra_iface_conf in
IMSG_RECONF_RA_IFACE
> case IMSG_RECONF_RA_RDNS_LIFETIME:
> ra_iface_conf->rdns_lifetime = *((uint32_t *)imsg.data);
> break;
> diff --git frontend.c frontend.c
> index b06fa43038c..06c4cc5cb2b 100644
> --- frontend.c
> +++ frontend.c
> @@ -397,6 +397,9 @@ frontend_dispatch_main(int fd, short event, void *bula)
> SIMPLEQ_INSERT_TAIL(&ra_iface_conf->ra_prefix_list,
> ra_prefix_conf, entry);
> break;
> + case IMSG_RECONF_RA_MTU:
> + ra_iface_conf->ra_mtu = *((uint32_t *)imsg.data);
> + break;
same here
> case IMSG_RECONF_RA_RDNS_LIFETIME:
> ra_iface_conf->rdns_lifetime = *((uint32_t *)imsg.data);
> break;
> @@ -888,6 +891,7 @@ build_packet(struct ra_iface *ra_iface)
> {
> struct nd_router_advert *ra;
> struct nd_opt_prefix_info *ndopt_pi;
> + struct nd_opt_mtu *ndopt_mtu;
> struct ra_iface_conf *ra_iface_conf;
> struct ra_options_conf *ra_options_conf;
> struct ra_prefix_conf *ra_prefix_conf;
> @@ -960,6 +964,15 @@ build_packet(struct ra_iface *ra_iface)
> p += sizeof(*ndopt_pi);
> }
>
> + if (ra_iface_conf->ra_mtu > 0) {
> + ndopt_mtu = (struct nd_opt_mtu *)p;
> + ndopt_mtu->nd_opt_mtu_type = ND_OPT_MTU;
> + ndopt_mtu->nd_opt_mtu_len = 1;
> + ndopt_mtu->nd_opt_mtu_reserved = 0;
> + ndopt_mtu->nd_opt_mtu_mtu = htonl(ra_iface_conf->ra_mtu);
> + p += sizeof(*ndopt_mtu);
> + }
> +
you need to account for the length of the mtu option, see the beginning
of build_packet(), something like
if (ra_iface_conf->ra_mtu > 0)
len += sizeof(*ndopt_mtu);
> if (ra_iface_conf->rdnss_count > 0) {
> ndopt_rdnss = (struct nd_opt_rdnss *)p;
> ndopt_rdnss->nd_opt_rdnss_type = ND_OPT_RDNSS;
> diff --git parse.y parse.y
> index 582d165c7d6..9696ac58213 100644
> --- parse.y
> +++ parse.y
> @@ -115,7 +115,7 @@ typedef struct {
> %token DEFAULT ROUTER HOP LIMIT MANAGED ADDRESS
> %token CONFIGURATION OTHER LIFETIME REACHABLE TIME RETRANS TIMER
> %token AUTO PREFIX VALID PREFERRED LIFETIME ONLINK AUTONOMOUS
> -%token ADDRESS_CONFIGURATION DNS RESOLVER SEARCH
> +%token ADDRESS_CONFIGURATION DNS RESOLVER SEARCH MTU
>
> %token <v.string> STRING
> %token <v.number> NUMBER
> @@ -265,6 +265,9 @@ ra_ifaceoptsl : NO AUTO PREFIX {
> } ra_prefix_block {
> ra_prefix_conf = NULL;
> }
> + | MTU NUMBER {
> + ra_iface_conf->ra_mtu = $2;
> + }
> | DNS dns_block
> | ra_opt_block
> ;
> @@ -423,6 +426,7 @@ lookup(char *s)
> {"lifetime", LIFETIME},
> {"limit", LIMIT},
> {"managed", MANAGED},
> + {"mtu", MTU},
> {"no", NO},
> {"on-link", ONLINK},
> {"other", OTHER},
> @@ -958,6 +962,8 @@ conf_get_ra_iface(char *name)
> /* Inherit attributes set in global section. */
> iface->ra_options = conf->ra_options;
>
> + iface->ra_mtu = DEFAULT_MTU;
> +
no need, 0 is the default since it's calloc'ed
> iface->rdns_lifetime = DEFAULT_RDNS_LIFETIME;
>
> SIMPLEQ_INIT(&iface->ra_prefix_list);
> diff --git printconf.c printconf.c
> index 1fc8cd4ca3f..0285bdb75be 100644
> --- printconf.c
> +++ printconf.c
> @@ -99,6 +99,9 @@ print_config(struct rad_conf *conf)
> printf("\t}\n");
> }
>
> + if (iface->ra_mtu > 0)
> + printf("\tmtu %u\n", iface->ra_mtu);
> +
> if (!SIMPLEQ_EMPTY(&iface->ra_rdnss_list) ||
> !SIMPLEQ_EMPTY(&iface->ra_dnssl_list)) {
> printf("\tdns {\n");
> diff --git rad.c rad.c
> index 4652e1e0fb7..cdaf529d806 100644
> --- rad.c
> +++ rad.c
> @@ -622,6 +622,9 @@ main_imsg_send_config(struct rad_conf *xconf)
> ra_prefix_conf, sizeof(*ra_prefix_conf)) == -1)
> return (-1);
> }
> + if (main_sendboth(IMSG_RECONF_RA_MTU, &ra_iface_conf->ra_mtu,
> + sizeof(ra_iface_conf->ra_mtu)) == -1)
> + return (-1);
no need
> if (main_sendboth(IMSG_RECONF_RA_RDNS_LIFETIME,
> &ra_iface_conf->rdns_lifetime,
> sizeof(ra_iface_conf->rdns_lifetime)) == -1)
> diff --git rad.conf.5 rad.conf.5
> index 6ca6b3ef956..6165e74070d 100644
> --- rad.conf.5
> +++ rad.conf.5
> @@ -126,6 +126,14 @@ prefix.
> The default is 2592000.
> .El
> .Pp
> +A maximum transmission unit (MTU) field can be advertised by configuring it
> +in an interface block:
> +.Bl -tag -width Ds
> +.It Ic mtu Ar bytes
> +MTU in bytes. If 0 is specified the option will not be included.
> +The default is 0.
> +.El
> +.Pp
> Recursive resolvers are configured inside an interface block:
> .Bd -unfilled -offset indent
> .Ic dns Brq dns options
> diff --git rad.h rad.h
> index db80d7c772d..8d919303723 100644
> --- rad.h
> +++ rad.h
> @@ -26,6 +26,7 @@
> #define OPT_VERBOSE2 0x00000002
> #define OPT_NOACTION 0x00000004
>
> +#define DEFAULT_MTU 0
no need
>
> #define MAX_RTR_ADV_INTERVAL 600
> #define MIN_RTR_ADV_INTERVAL 200
> @@ -59,6 +60,7 @@ enum imsg_type {
> IMSG_RECONF_RA_IFACE,
> IMSG_RECONF_RA_AUTOPREFIX,
> IMSG_RECONF_RA_PREFIX,
> + IMSG_RECONF_RA_MTU,
> IMSG_RECONF_RA_RDNS_LIFETIME,
> IMSG_RECONF_RA_RDNSS,
> IMSG_RECONF_RA_DNSSL,
> @@ -114,6 +116,7 @@ struct ra_iface_conf {
> struct ra_prefix_conf *autoprefix;
> SIMPLEQ_HEAD(ra_prefix_conf_head,
> ra_prefix_conf) ra_prefix_list;
> + uint32_t ra_mtu;
> uint32_t rdns_lifetime;
> SIMPLEQ_HEAD(, ra_rdnss_conf) ra_rdnss_list;
> int rdnss_count;
After reading the diff a 2nd time I think I'd prefer to have the mtu
in ra_options, as a potential global setting. After all, if you need
to set the mtu on one interface you probably have to set it on all.
We then also need a ``no mtu'' in the parser, but that's easy enough.
--
I'm not entirely sure you are real.
