On Fri 20/07/2018 07:57, Florian Obser wrote:
> 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.
New diff, which addresses all your comments except the "no mtu" bit in
the parser as I do not understand what you mean.
diff --git frontend.c frontend.c
index b06fa43038c..0509d2b3efe 100644
--- frontend.c
+++ frontend.c
@@ -887,6 +887,7 @@ void
build_packet(struct ra_iface *ra_iface)
{
struct nd_router_advert *ra;
+ struct nd_opt_mtu *ndopt_mtu;
struct nd_opt_prefix_info *ndopt_pi;
struct ra_iface_conf *ra_iface_conf;
struct ra_options_conf *ra_options_conf;
@@ -904,6 +905,8 @@ build_packet(struct ra_iface *ra_iface)
ra_options_conf = &ra_iface_conf->ra_options;
len = sizeof(*ra);
+ if (ra_options_conf->mtu > 0)
+ len += sizeof(*ndopt_mtu);
len += sizeof(*ndopt_pi) * ra_iface->prefix_count;
if (ra_iface_conf->rdnss_count > 0)
len += sizeof(*ndopt_rdnss) + ra_iface_conf->rdnss_count *
@@ -940,6 +943,15 @@ build_packet(struct ra_iface *ra_iface)
ra->nd_ra_retransmit = htonl(ra_options_conf->retrans_timer);
p += sizeof(*ra);
+ if (ra_options_conf->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_options_conf->mtu);
+ p += sizeof(*ndopt_mtu);
+ }
+
SIMPLEQ_FOREACH(ra_prefix_conf, &ra_iface->prefixes, entry) {
ndopt_pi = (struct nd_opt_prefix_info *)p;
memset(ndopt_pi, 0, sizeof(*ndopt_pi));
diff --git parse.y parse.y
index 582d165c7d6..6eb6990f97b 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
@@ -209,6 +209,9 @@ ra_opt_block : DEFAULT ROUTER yesno {
| RETRANS TIMER NUMBER {
ra_options->retrans_timer = $3;
}
+ | MTU NUMBER {
+ ra_options->mtu = $2;
+ }
;
optnl : '\n' optnl /* zero or more newlines */
@@ -423,6 +426,7 @@ lookup(char *s)
{"lifetime", LIFETIME},
{"limit", LIMIT},
{"managed", MANAGED},
+ {"mtu", MTU},
{"no", NO},
{"on-link", ONLINK},
{"other", OTHER},
diff --git printconf.c printconf.c
index 1fc8cd4ca3f..b7864dbae6b 100644
--- printconf.c
+++ printconf.c
@@ -54,6 +54,7 @@ print_ra_options(const char *indent, const struct
ra_options_conf *ra_options)
printf("%srouter lifetime %d\n", indent, ra_options->router_lifetime);
printf("%sreachable time %u\n", indent, ra_options->reachable_time);
printf("%sretrans timer %u\n", indent, ra_options->retrans_timer);
+ printf("%smtu %u\n", indent, ra_options->mtu);
}
void
diff --git rad.c rad.c
index 4652e1e0fb7..9f4be1b2190 100644
--- rad.c
+++ rad.c
@@ -716,6 +716,7 @@ config_new_empty(void)
xconf->ra_options.router_lifetime = 1800;
xconf->ra_options.reachable_time = 0;
xconf->ra_options.retrans_timer = 0;
+ xconf->ra_options.mtu = 0;
return (xconf);
}
diff --git rad.conf.5 rad.conf.5
index 6ca6b3ef956..486bec24769 100644
--- rad.conf.5
+++ rad.conf.5
@@ -87,6 +87,11 @@ The default is 1800 seconds.
.\" XXX
.\" .It Ic retrans timer Ar number
.\" XXX
+.It Ic mtu Ar bytes
+The MTU option is used in Router Advertisement messages to ensure that all
+nodes on a link use the same MTU value in those cases where the link MTU
+is not well known.
+The default is 0, meaning unspecified by this router.
.El
.Sh INTERFACES
A list of interfaces to send advertisments on:
diff --git rad.h rad.h
index db80d7c772d..f9d26d66c9c 100644
--- rad.h
+++ rad.h
@@ -76,7 +76,7 @@ enum imsg_type {
IMSG_SOCKET_IPC
};
-/* RFC 4861 Section 4.2 */
+/* RFC 4861 Sections 4.2 and 4.6.4 */
struct ra_options_conf {
int dfr; /* is default router? */
int cur_hl; /* current hop limit */
@@ -85,6 +85,7 @@ struct ra_options_conf {
int router_lifetime; /* default router lifetime */
uint32_t reachable_time;
uint32_t retrans_timer;
+ uint32_t mtu;
};
/* RFC 4861 Section 4.6.2 */
