On Fri, Nov 16, 2018 at 08:56:52PM +0100, Reyk Floeter wrote:
> > the following diff allows rad(8) to watch interface groups. This
> > allows to automatically add/remove interfaces in a given group.
> >
> > For example, I put "interface tap" into rad.conf and it automatically
> > serves my VM interfaces. You could also configure a custom group in
> > vm.conf and rad.conf. I'm working on IPv6 for vmd that needs it.
Nice idea/work, I like it.
> > For reasons that I don't remember, I always missed this feature in
> > rtadvd(8)[RIP]. It was amazingly simple to add it to rad(8) as it
> > already reinitializes itself on interface changes.
> >
> > This diff includes the previous ENXIO fix to prevent it from crashing
> > when a cloner interface such as tap is destroyed.
> >
>
> Updated diff after committing the ENXIO fix.
>
> Two explanations:
>
> - merge_ra_interfaces() calls merge_ra_interface() for each configured
> interface (explicit config) and for each member that is found in an
> interface group. It is basically just some code shuffling.
>
> - ra_iface->name is split into ra_iface->name and ra_iface->conf as
> "name" indicates and actual (found) interface name ("tap0") and "conf"
> is used to lookup the config where it was derived from ("tap" or
> "tap0", depending on the configuration).
This begs the question about priority in rad.conf(5). It's always been
possible to define duplicate `interface' blocks although it didn't make
any sense.
Now it does since you could define a default config for the entire group
and add further interface specific blocks.
As it currently is, the first matching block wins, so vether0 would
always get RDNS ::11 whether a more specific block exists or not:
interface vether { dns { nameserver ::11 } }
interface vether0 { dns { nameserver ::22 } }
Maybe we should clarify behaviour in rad.conf(5) regardless of your
changes?
> void
> +merge_ra_interface(struct ra_iface_conf *ra_iface_conf, char *name)
> +{
> + struct ra_iface *ra_iface;
> + uint32_t if_index;
> +
> + ra_iface = find_ra_iface_by_name(name);
> + if (ra_iface == NULL) {
> + log_debug("new interface %s", name);
> + if ((if_index = if_nametoindex(name)) == 0)
> + return;
> + log_debug("adding interface %s", name);
> + if ((ra_iface = calloc(1, sizeof(*ra_iface))) == NULL)
> + fatal("%s", __func__);
> +
> + strlcpy(ra_iface->name, name, sizeof(ra_iface->name));
> + strlcpy(ra_iface->conf, ra_iface_conf->name,
> + sizeof(ra_iface->conf));
> +
> + ra_iface->if_index = if_index;
> + SIMPLEQ_INIT(&ra_iface->prefixes);
> + TAILQ_INSERT_TAIL(&ra_interfaces, ra_iface, entry);
> + join_all_routers_mcast_group(ra_iface);
> + } else {
> + log_debug("keeping interface %s", name);
> + ra_iface->removed = 0;
> + }
> +}
How about flipping the check to reduce indentation?
if (ra_iface) {
log_debug("keeping interface %s", name);
...
return;
}
log_debug("new interface %s", name);
...
