Thank you for the code and review! I've synthesized the existing patch
and review into something that successfully advertises router
preferences in local testing (verified w/ rdisc6). This patch does not
implement the route information option specified in RFC 4191 section
2.3.

diff --git a/usr.sbin/rad/frontend.c b/usr.sbin/rad/frontend.c
index 8178b058629..4031da6b99d 100644
--- a/usr.sbin/rad/frontend.c
+++ b/usr.sbin/rad/frontend.c
@@ -411,7 +411,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
            ra_prefix_conf))
               fatalx("%s: IMSG_RECONF_RA_PREFIX wrong "
                "length: %lu", __func__,
-                IMSG_DATA_SIZE(imsg));
+                IMSG_DATA_SIZE(imsg));
           if ((ra_prefix_conf = malloc(sizeof(struct
            ra_prefix_conf))) == NULL)
               fatal(NULL);
@@ -1023,6 +1023,18 @@ build_packet(struct ra_iface *ra_iface)
       ra->nd_ra_router_lifetime =
        htons(ra_options_conf->router_lifetime);
   }
+
+   /* add router preference flags */
+   if (ra_options_conf->preference == ND_RA_FLAG_RTPREF_RSV) {
+       fatalx("Invalid router preference found during RA packet
construction.");
+   }
+
+   if (ra_options_conf->router_lifetime == 0) {
+       log_debug("Router lifetime set to zero; ignoring router
preference per https://tools.ietf.org/html/rfc4191#section-2.2";);
+   } else {
+       ra->nd_ra_flags_reserved |= ra_options_conf->preference;
+   }
+
   ra->nd_ra_reachable = htonl(ra_options_conf->reachable_time);
   ra->nd_ra_retransmit = htonl(ra_options_conf->retrans_timer);
   p += sizeof(*ra);
diff --git a/usr.sbin/rad/parse.y b/usr.sbin/rad/parse.y
index 004e5e22f92..74480148246 100644
--- a/usr.sbin/rad/parse.y
+++ b/usr.sbin/rad/parse.y
@@ -32,6 +32,7 @@
#include <net/if.h>
#include <arpa/inet.h>
+#include <netinet/icmp6.h>
#include <ctype.h>
#include <err.h>
@@ -117,10 +118,12 @@ typedef struct {
%token CONFIGURATION OTHER LIFETIME REACHABLE TIME RETRANS TIMER
%token AUTO PREFIX VALID PREFERRED LIFETIME ONLINK AUTONOMOUS
%token ADDRESS_CONFIGURATION DNS NAMESERVER SEARCH MTU
+%token PREFERENCE LOW MEDIUM HIGH
%token <v.string>  STRING
%token <v.number>  NUMBER
%type  <v.number>  yesno
+%type  <v.number>  preference
%type  <v.string>  string
%%
@@ -166,6 +169,11 @@ yesno      : YES   { $$ = 1; }
       | NO    { $$ = 0; }
       ;
+preference : LOW   { $$ = ND_RA_FLAG_RTPREF_LOW; }
+       | MEDIUM { $$ = ND_RA_FLAG_RTPREF_MEDIUM; }
+       | HIGH { $$ = ND_RA_FLAG_RTPREF_HIGH; }
+       ;
+
varset     : STRING '=' string     {
           char *s = $1;
           if (cmd_opts & OPT_VERBOSE)
@@ -213,6 +221,9 @@ ra_opt_block    : DEFAULT ROUTER yesno {
       | MTU NUMBER {
           ra_options->mtu = $2;
       }
+       | PREFERENCE preference {
+           ra_options->preference = $2;
+       }
       | DNS dns_block
       ;
@@ -426,16 +437,20 @@ lookup(char *s)
       {"default",     DEFAULT},
       {"dns",         DNS},
       {"hop",         HOP},
+       {"high",        HIGH},
       {"include",     INCLUDE},
       {"interface",       RA_IFACE},
       {"lifetime",        LIFETIME},
       {"limit",       LIMIT},
+       {"low",         LOW},
       {"managed",     MANAGED},
+       {"medium",      MEDIUM},
       {"mtu",         MTU},
       {"nameserver",      NAMESERVER},
       {"no",          NO},
       {"on-link",     ONLINK},
       {"other",       OTHER},
+       {"preference",      PREFERENCE},
       {"preferred",       PREFERRED},
       {"prefix",      PREFIX},
       {"reachable",       REACHABLE},
diff --git a/usr.sbin/rad/printconf.c b/usr.sbin/rad/printconf.c
index d42890da518..c2173d2142f 100644
--- a/usr.sbin/rad/printconf.c
+++ b/usr.sbin/rad/printconf.c
@@ -26,6 +26,7 @@
#include <net/if.h>
#include <arpa/inet.h>
+#include <netinet/icmp6.h>
#include <event.h>
#include <imsg.h>
@@ -34,6 +35,7 @@
#include "rad.h"
const char*    yesno(int);
+const char*    preference(int);
void       print_ra_options(const char*, const struct ra_options_conf*);
void       print_prefix_options(const char*, const struct ra_prefix_conf*);
@@ -42,6 +44,20 @@ yesno(int flag)
{
   return flag ? "yes" : "no";
}
+const char*
+preference(int p)
+{
+   switch (p) {
+       case ND_RA_FLAG_RTPREF_LOW:
+           return "low";
+       case ND_RA_FLAG_RTPREF_MEDIUM:
+           return "medium";
+       case ND_RA_FLAG_RTPREF_HIGH:
+           return "high";
+       default:
+           return "invalid";
+   }
+}
void
print_ra_options(const char *indent, const struct ra_options_conf *ra_options)
@@ -60,6 +76,9 @@ print_ra_options(const char *indent, const struct
ra_options_conf *ra_options)
   printf("%sretrans timer %u\n", indent, ra_options->retrans_timer);
   if (ra_options->mtu > 0)
       printf("%smtu %u\n", indent, ra_options->mtu);
+   if (ra_options->preference != ND_RA_FLAG_RTPREF_RSV)
+       printf("%spreference %s\n", indent,
+        preference(ra_options->preference));
   if (!SIMPLEQ_EMPTY(&ra_options->ra_rdnss_list) ||
    !SIMPLEQ_EMPTY(&ra_options->ra_dnssl_list)) {
diff --git a/usr.sbin/rad/rad.c b/usr.sbin/rad/rad.c
index 93675167b6b..cb0593f11ab 100644
--- a/usr.sbin/rad/rad.c
+++ b/usr.sbin/rad/rad.c
@@ -433,7 +433,7 @@ main_dispatch_frontend(int fd, short event, void *bula)
       case IMSG_CTL_LOG_VERBOSE:
           if (IMSG_DATA_SIZE(imsg) != sizeof(verbose))
               fatalx("%s: IMSG_CTL_LOG_VERBOSE wrong length: "
-                "%lu", __func__, IMSG_DATA_SIZE(imsg));
+                "%lu", __func__, IMSG_DATA_SIZE(imsg));
           memcpy(&verbose, imsg.data, sizeof(verbose));
           log_setverbose(verbose);
           break;
@@ -754,6 +754,7 @@ config_new_empty(void)
   xconf->ra_options.cur_hl = 0;
   xconf->ra_options.m_flag = 0;
   xconf->ra_options.o_flag = 0;
+   xconf->ra_options.preference = ND_RA_FLAG_RTPREF_MEDIUM;
   xconf->ra_options.router_lifetime = 1800;
   xconf->ra_options.reachable_time = 0;
   xconf->ra_options.retrans_timer = 0;
diff --git a/usr.sbin/rad/rad.conf.5 b/usr.sbin/rad/rad.conf.5
index f651a715d1a..b822f3d195d 100644
--- a/usr.sbin/rad/rad.conf.5
+++ b/usr.sbin/rad/rad.conf.5
@@ -107,6 +107,8 @@ The default is 1800 seconds.
.\" XXX
.\" .It Ic retrans timer Ar number
.\" XXX
+.It Ic preference Pq Ic low Ns | Ns Ic medium Ns | Ns Ic high
+Communicate router preference to clients. The default is medium.
.El
.Sh INTERFACES
A list of interfaces or interface groups to send advertisments on:
diff --git a/usr.sbin/rad/rad.h b/usr.sbin/rad/rad.h
index 2bbf7c8ed5c..cfaa5e88638 100644
--- a/usr.sbin/rad/rad.h
+++ b/usr.sbin/rad/rad.h
@@ -92,6 +92,7 @@ struct ra_options_conf {
   int     cur_hl;         /* current hop limit */
   int     m_flag;         /* managed address conf flag */
   int     o_flag;         /* other conf flag */
+   int     preference;     /* router preference (see RFC 4191 2.2) */
   int     router_lifetime;    /* default router lifetime */
   uint32_t    reachable_time;
   uint32_t    retrans_timer;


On Wed, Aug 7, 2019 at 2:04 AM Florian Obser <flor...@openbsd.org> wrote:
>
> On Tue, Aug 06, 2019 at 11:17:04PM +0200, Sebastian Benoit wrote:
> > Caleb(enlightened.des...@gmail.com) on 2019.08.06 08:05:48 -0700:
> > > How do I publish default router preferences as defined in RFC 4191
> > > (https://tools.ietf.org/html/rfc4191) using rad in OpenBSD 6.5?
> > > I've read the friendly rad.conf man page
> > > (https://man.openbsd.org/rad.conf.5) and scanned the source
> > > (https://github.com/openbsd/src/tree/master/usr.sbin/rad) with no
> > > success.
> >
> > You can't, because it was not implemented.
> >
> > That is, until now.
> >
> > I wrote a patch, which you can test if you like. It's completly untested
> > though.
> >
>
> needs more yak shaving
>
> >
> > diff --git usr.sbin/rad/frontend.c usr.sbin/rad/frontend.c
> > index 8178b058629..75723797fcf 100644
> > --- usr.sbin/rad/frontend.c
> > +++ usr.sbin/rad/frontend.c
> > @@ -1016,6 +1016,8 @@ build_packet(struct ra_iface *ra_iface)
> >               ra->nd_ra_flags_reserved |= ND_RA_FLAG_MANAGED;
> >       if (ra_options_conf->o_flag)
> >               ra->nd_ra_flags_reserved |= ND_RA_FLAG_OTHER;
> > +     ra->nd_ra_flags_reserved |=
> > +         ra_options_conf->preference;
> >       if (ra_iface->removed)
> >               /* tell clients that we are no longer a default router */
> >               ra->nd_ra_router_lifetime = 0;
> > @@ -1048,6 +1050,8 @@ build_packet(struct ra_iface *ra_iface)
> >               if (ra_prefix_conf->aflag)
> >                       ndopt_pi->nd_opt_pi_flags_reserved |=
> >                           ND_OPT_PI_FLAG_AUTO;
> > +             ndopt_pi->nd_opt_pi_flags_reserved |=
> > +                         ra_prefix_conf->preference;
>
> This is a prefix information option (type 3) not a route information option 
> (type 24).
> Option 3 does not have a preference.
>
> >               ndopt_pi->nd_opt_pi_valid_time = 
> > htonl(ra_prefix_conf->vltime);
> >               ndopt_pi->nd_opt_pi_preferred_time =
> >                   htonl(ra_prefix_conf->pltime);
> > diff --git usr.sbin/rad/parse.y usr.sbin/rad/parse.y
> > index 004e5e22f92..b004ab37356 100644
> > --- usr.sbin/rad/parse.y
> > +++ usr.sbin/rad/parse.y
> > @@ -106,6 +106,7 @@ typedef struct {
> >       union {
> >               int64_t          number;
> >               char            *string;
> > +             short            pref;
>
> eek, just treat it as a number?
>
> >       } v;
> >       int lineno;
> >  } YYSTYPE;
> > @@ -117,11 +118,13 @@ typedef struct {
> >  %token       CONFIGURATION OTHER LIFETIME REACHABLE TIME RETRANS TIMER
> >  %token       AUTO PREFIX VALID PREFERRED LIFETIME ONLINK AUTONOMOUS
> >  %token       ADDRESS_CONFIGURATION DNS NAMESERVER SEARCH MTU
> > +%token       PREFERENCE LOW MEDIUM HIGH
> >
> >  %token       <v.string>      STRING
> >  %token       <v.number>      NUMBER
> >  %type        <v.number>      yesno
> >  %type        <v.string>      string
> > +%type        <v.pref>        preftype
> >
> >  %%
> >
> > @@ -213,6 +216,9 @@ ra_opt_block      : DEFAULT ROUTER yesno {
> >               | MTU NUMBER {
> >                       ra_options->mtu = $2;
> >               }
> > +             | PREFERENCE preftype {
> > +                     ra_options->preference = $2;
> > +             }
> >               | DNS dns_block
> >               ;
> >
> > @@ -298,6 +304,19 @@ ra_prefixoptsl   : VALID LIFETIME NUMBER {
> >               | AUTONOMOUS ADDRESS_CONFIGURATION yesno {
> >                       ra_prefix_conf->aflag = $3;
> >               }
> > +             | PREFERENCE preftype {
> > +                     ra_prefix_conf->preference = $2;
> > +             }
> > +             ;
>
> see above, we are announcing prefix information, not route information
>
> > +preftype     : LOW {
> > +                     $$ = RA_PREFIXOPT_PREF_LOW;
> > +             }
> > +             | MEDIUM {
> > +                     $$ = RA_PREFIXOPT_PREF_MEDIUM;
> > +             }
> > +             | HIGH {
> > +                     $$ = RA_PREFIXOPT_PREF_HIGH;
> > +             }
>
> please use the defines from icmp6.h:
>
> #define ND_RA_FLAG_RTPREF_HIGH  0x08    /* 00001000 */
> #define ND_RA_FLAG_RTPREF_MEDIUM        0x00    /* 00000000 */
> #define ND_RA_FLAG_RTPREF_LOW   0x18    /* 00011000 */
> #define ND_RA_FLAG_RTPREF_RSV   0x10    /* 00010000 */
>
>
> >               ;
> >  dns_block    : '{' optnl dnsopts_l '}'
> >               | '{' optnl '}'
> > @@ -425,17 +444,21 @@ lookup(char *s)
> >               {"configuration",       CONFIGURATION},
> >               {"default",             DEFAULT},
> >               {"dns",                 DNS},
> > +             {"high",                HIGH},
> >               {"hop",                 HOP},
> >               {"include",             INCLUDE},
> >               {"interface",           RA_IFACE},
> >               {"lifetime",            LIFETIME},
> >               {"limit",               LIMIT},
> > +             {"low",                 LOW},
> >               {"managed",             MANAGED},
> > +             {"medium",              MEDIUM},
> >               {"mtu",                 MTU},
> >               {"nameserver",          NAMESERVER},
> >               {"no",                  NO},
> >               {"on-link",             ONLINK},
> >               {"other",               OTHER},
> > +             {"preference",          PREFERENCE},
> >               {"preferred",           PREFERRED},
> >               {"prefix",              PREFIX},
> >               {"reachable",           REACHABLE},
> > diff --git usr.sbin/rad/printconf.c usr.sbin/rad/printconf.c
> > index d42890da518..e063daaa19f 100644
> > --- usr.sbin/rad/printconf.c
> > +++ usr.sbin/rad/printconf.c
> > @@ -34,6 +34,7 @@
> >  #include "rad.h"
> >
> >  const char*  yesno(int);
> > +const char*  preference(short);
>
> make this an in            ^
>
> >  void         print_ra_options(const char*, const struct ra_options_conf*);
> >  void         print_prefix_options(const char*, const struct 
> > ra_prefix_conf*);
> >
> > @@ -43,6 +44,21 @@ yesno(int flag)
> >       return flag ? "yes" : "no";
> >  }
> >
> > +const char*
> > +preference(short p)
>               ^ and here
> > +{
> > +     switch (p) {
> > +             case RA_PREFIXOPT_PREF_LOW:
> > +                     return "low";
> > +             case RA_PREFIXOPT_PREF_MEDIUM:
> > +                     return "medium";
> > +             case RA_PREFIXOPT_PREF_HIGH:
> > +                     return "high";
> > +             default:
> > +                     return "invalid";
>
> use the defines from icmp6.h
>
> > +     }
> > +}
> > +
> >  void
> >  print_ra_options(const char *indent, const struct ra_options_conf 
> > *ra_options)
> >  {
> > @@ -60,6 +76,9 @@ print_ra_options(const char *indent, const struct 
> > ra_options_conf *ra_options)
> >       printf("%sretrans timer %u\n", indent, ra_options->retrans_timer);
> >       if (ra_options->mtu > 0)
> >               printf("%smtu %u\n", indent, ra_options->mtu);
> > +     if (ra_options->preference > 0)
>
> this does not work, if you set the default in the config file (medium)
> it will never get printed since it's 0, but maybe that's your
> intention? I'd store ND_RA_FLAG_RTPREF_RSV as default and map it to
> medium when building the packet. Then you can match on it here.
>
>
> > +             printf("%spreference %s\n", indent,
> > +                 preference(ra_options->preference));
> >
> >       if (!SIMPLEQ_EMPTY(&ra_options->ra_rdnss_list) ||
> >           !SIMPLEQ_EMPTY(&ra_options->ra_dnssl_list)) {
> > @@ -95,6 +114,8 @@ print_prefix_options(const char *indent, const struct 
> > ra_prefix_conf
> >       printf("%son-link %s\n", indent, yesno(ra_prefix_conf->lflag));
> >       printf("%sautonomous address-configuration %s\n", indent,
> >           yesno(ra_prefix_conf->aflag));
> > +     printf("%spreference %s\n", indent,
> > +         preference(ra_prefix_conf->preference));
>
> Prefix Information, not Route Information
>
> >  }
> >
> >  void
> > diff --git usr.sbin/rad/rad.conf.5 usr.sbin/rad/rad.conf.5
> > index f651a715d1a..888a8f79b76 100644
> > --- usr.sbin/rad/rad.conf.5
> > +++ usr.sbin/rad/rad.conf.5
> > @@ -107,6 +107,11 @@ The default is 1800 seconds.
> >  .\" XXX
> >  .\" .It Ic retrans timer Ar number
> >  .\" XXX
> > +.It Ic preference Pq Ic low Ns | Ns Ic medium Ns | Ns Ic high
> > +Specify the router preference that is communicated to hosts through
> > +router advertisements.
> > +It can be used to communicate a prefered default router to IPv6 hosts.
> > +The default is medium.
> >  .El
> >  .Sh INTERFACES
> >  A list of interfaces or interface groups to send advertisments on:
> > @@ -147,6 +152,9 @@ The default is 604800.
> >  The valid lifetime (vltime) in seconds for addresses generated from this
> >  prefix.
> >  The default is 2592000.
> > +.It Ic preference Pq Ic low Ns | Ns Ic medium Ns | Ns Ic high
> > +The preference of the prefix is low, medium or high.
> > +The default is medium.
>
> Prefix Information vs. Route Informatiuon
>
> >  .El
> >  .Sh FILES
> >  .Bl -tag -width "/etc/rad.conf" -compact
> > diff --git usr.sbin/rad/rad.h usr.sbin/rad/rad.h
> > index 2bbf7c8ed5c..9508783da2e 100644
> > --- usr.sbin/rad/rad.h
> > +++ usr.sbin/rad/rad.h
> > @@ -93,6 +93,7 @@ struct ra_options_conf {
> >       int             m_flag;                 /* managed address conf flag 
> > */
> >       int             o_flag;                 /* other conf flag */
> >       int             router_lifetime;        /* default router lifetime */
> > +     short           preference;             /* rfc4191 def. router pref. 
> > */
>
>         ^ make it an int
>
> >       uint32_t        reachable_time;
> >       uint32_t        retrans_timer;
> >       uint32_t        mtu;
> > @@ -112,8 +113,17 @@ struct ra_prefix_conf {
> >       uint32_t                         pltime;        /* prefered lifetime 
> > */
> >       int                              lflag;         /* on-link flag*/
> >       int                              aflag;         /* autonom. addr flag 
> > */
> > +     short                            preference;    /* preference rfc4191 
> > */
>
>
> Prefix Information vs. Route Information
>
> >  };
> >
> > +/*
> > +  RFC4191 preference values, in the middle of the 8bit reserved field.
> > +  01 High, 00 Medium (default),  11 Low, 10 Reserved - MUST NOT be sent
> > +*/
> > +#define RA_PREFIXOPT_PREF_LOW                0x18
> > +#define RA_PREFIXOPT_PREF_MEDIUM     0x00
> > +#define RA_PREFIXOPT_PREF_HIGH               0x08
> > +
>
> see above, use the defines from icmp6.h
>
> >  struct ra_iface_conf {
> >       SIMPLEQ_ENTRY(ra_iface_conf)             entry;
> >       struct ra_options_conf                   ra_options;
>
> --
> I'm not entirely sure you are real.

Reply via email to