ospf(6)d.conf: define interface parameters per area or globally

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

ospf(6)d.conf: define interface parameters per area or globally

Remi Locherer
Hi,

interface-specific parameters can be defined globally or per area.
But they are applied to the interfaces only if the interfaces are
declared afterwards.

Or is the GLOBAL CONFIURATION section the better place for this?
I opted for the AREA section because I consider it unlikely a user adds
global parameters at the end of the config file. But who knows. ;-)

Remi

Index: ospfd/ospfd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
retrieving revision 1.58
diff -u -p -r1.58 ospfd.conf.5
--- ospfd/ospfd.conf.5 19 Nov 2019 09:55:55 -0000 1.58
+++ ospfd/ospfd.conf.5 4 Jan 2020 21:48:00 -0000
@@ -256,11 +256,13 @@ is set to a value other than 1 or if the
 Areas are used for grouping interfaces.
 All interface-specific parameters can
 be configured per area, overruling the global settings.
+These interface-specific parameters need to be defined before the interfaces.
 .Bl -tag -width Ds
 .It Ic area Ar id | address
 Specify an area section, grouping one or more interfaces.
 .Bd -literal -offset indent
 area 0.0.0.0 {
+ hello-interval 3
  interface em0
  interface em1 {
  metric 10
Index: ospf6d/ospf6d.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.conf.5,v
retrieving revision 1.20
diff -u -p -r1.20 ospf6d.conf.5
--- ospf6d/ospf6d.conf.5 26 Dec 2019 10:24:18 -0000 1.20
+++ ospf6d/ospf6d.conf.5 4 Jan 2020 21:48:30 -0000
@@ -236,11 +236,13 @@ is set to a value different to 1 or if t
 Areas are used for grouping interfaces.
 All interface-specific parameters can
 be configured per area, overruling the global settings.
+These interface-specific parameters need to be defined before the interfaces.
 .Bl -tag -width Ds
 .It Ic area Ar address Ns | Ns Ar id
 Specify an area section, grouping one or more interfaces.
 .Bd -literal -offset indent
 area 0.0.0.0 {
+ hello-interval 3
  interface em0
  interface em1 {
  metric 10

Reply | Threaded
Open this post in threaded view
|

Re: ospf(6)d.conf: define interface parameters per area or globally

Denis Fondras
On Sat, Jan 04, 2020 at 11:11:36PM +0100, Remi Locherer wrote:
> Hi,
>
> interface-specific parameters can be defined globally or per area.
> But they are applied to the interfaces only if the interfaces are
> declared afterwards.
>

I have a diff to allow parameters after interface or area definition.
Not sure if we want to do that though.

> Or is the GLOBAL CONFIURATION section the better place for this?
> I opted for the AREA section because I consider it unlikely a user adds
> global parameters at the end of the config file. But who knows. ;-)
>

In the MACRO section I would change the last sentence too (or even remove it as
it is close to the GLOBAL first paragraph).

Anyway OK denis@

> Remi
>
> Index: ospfd/ospfd.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
> retrieving revision 1.58
> diff -u -p -r1.58 ospfd.conf.5
> --- ospfd/ospfd.conf.5 19 Nov 2019 09:55:55 -0000 1.58
> +++ ospfd/ospfd.conf.5 4 Jan 2020 21:48:00 -0000
> @@ -256,11 +256,13 @@ is set to a value other than 1 or if the
>  Areas are used for grouping interfaces.
>  All interface-specific parameters can
>  be configured per area, overruling the global settings.
> +These interface-specific parameters need to be defined before the interfaces.
>  .Bl -tag -width Ds
>  .It Ic area Ar id | address
>  Specify an area section, grouping one or more interfaces.
>  .Bd -literal -offset indent
>  area 0.0.0.0 {
> + hello-interval 3
>   interface em0
>   interface em1 {
>   metric 10
> Index: ospf6d/ospf6d.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.conf.5,v
> retrieving revision 1.20
> diff -u -p -r1.20 ospf6d.conf.5
> --- ospf6d/ospf6d.conf.5 26 Dec 2019 10:24:18 -0000 1.20
> +++ ospf6d/ospf6d.conf.5 4 Jan 2020 21:48:30 -0000
> @@ -236,11 +236,13 @@ is set to a value different to 1 or if t
>  Areas are used for grouping interfaces.
>  All interface-specific parameters can
>  be configured per area, overruling the global settings.
> +These interface-specific parameters need to be defined before the interfaces.
>  .Bl -tag -width Ds
>  .It Ic area Ar address Ns | Ns Ar id
>  Specify an area section, grouping one or more interfaces.
>  .Bd -literal -offset indent
>  area 0.0.0.0 {
> + hello-interval 3
>   interface em0
>   interface em1 {
>   metric 10
>

Reply | Threaded
Open this post in threaded view
|

Re: ospf(6)d.conf: define interface parameters per area or globally

Remi Locherer
On Sat, Jan 04, 2020 at 11:34:45PM +0100, Denis Fondras wrote:

> On Sat, Jan 04, 2020 at 11:11:36PM +0100, Remi Locherer wrote:
> > Hi,
> >
> > interface-specific parameters can be defined globally or per area.
> > But they are applied to the interfaces only if the interfaces are
> > declared afterwards.
> >
>
> I have a diff to allow parameters after interface or area definition.
> Not sure if we want to do that though.

I would appreciate that! ;-)

> > Or is the GLOBAL CONFIURATION section the better place for this?
> > I opted for the AREA section because I consider it unlikely a user adds
> > global parameters at the end of the config file. But who knows. ;-)
> >
>
> In the MACRO section I would change the last sentence too (or even remove it as
> it is close to the GLOBAL first paragraph).

True, it does not add a lot of value. But I don't have a strong opinion.

>
> Anyway OK denis@
>
> > Remi
> >
> > Index: ospfd/ospfd.conf.5
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
> > retrieving revision 1.58
> > diff -u -p -r1.58 ospfd.conf.5
> > --- ospfd/ospfd.conf.5 19 Nov 2019 09:55:55 -0000 1.58
> > +++ ospfd/ospfd.conf.5 4 Jan 2020 21:48:00 -0000
> > @@ -256,11 +256,13 @@ is set to a value other than 1 or if the
> >  Areas are used for grouping interfaces.
> >  All interface-specific parameters can
> >  be configured per area, overruling the global settings.
> > +These interface-specific parameters need to be defined before the interfaces.
> >  .Bl -tag -width Ds
> >  .It Ic area Ar id | address
> >  Specify an area section, grouping one or more interfaces.
> >  .Bd -literal -offset indent
> >  area 0.0.0.0 {
> > + hello-interval 3
> >   interface em0
> >   interface em1 {
> >   metric 10
> > Index: ospf6d/ospf6d.conf.5
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.conf.5,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 ospf6d.conf.5
> > --- ospf6d/ospf6d.conf.5 26 Dec 2019 10:24:18 -0000 1.20
> > +++ ospf6d/ospf6d.conf.5 4 Jan 2020 21:48:30 -0000
> > @@ -236,11 +236,13 @@ is set to a value different to 1 or if t
> >  Areas are used for grouping interfaces.
> >  All interface-specific parameters can
> >  be configured per area, overruling the global settings.
> > +These interface-specific parameters need to be defined before the interfaces.
> >  .Bl -tag -width Ds
> >  .It Ic area Ar address Ns | Ns Ar id
> >  Specify an area section, grouping one or more interfaces.
> >  .Bd -literal -offset indent
> >  area 0.0.0.0 {
> > + hello-interval 3
> >   interface em0
> >   interface em1 {
> >   metric 10
> >
>

Reply | Threaded
Open this post in threaded view
|

Re: ospf(6)d.conf: define interface parameters per area or globally

Denis Fondras
On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > I have a diff to allow parameters after interface or area definition.
> > Not sure if we want to do that though.
>
> I would appreciate that! ;-)
>

The ospfd diff needs some more work. Crypt authentication handling is not
perfect.

Index: ospf6d/ospf6d.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v
retrieving revision 1.44
diff -u -p -r1.44 ospf6d.h
--- ospf6d/ospf6d.h 3 Jan 2020 17:45:02 -0000 1.44
+++ ospf6d/ospf6d.h 8 Jan 2020 12:11:20 -0000
@@ -328,7 +328,7 @@ struct iface {
  enum iface_type type;
  u_int8_t if_type;
  u_int8_t linkstate;
- u_int8_t priority;
+ int16_t priority;
  u_int8_t p2p;
  u_int8_t cflags;
 #define F_IFACE_PASSIVE 0x01
Index: ospf6d/parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/parse.y,v
retrieving revision 1.48
diff -u -p -r1.48 parse.y
--- ospf6d/parse.y 26 Dec 2019 10:24:18 -0000 1.48
+++ ospf6d/parse.y 8 Jan 2020 12:11:20 -0000
@@ -101,7 +101,7 @@ struct config_defaults {
  u_int16_t hello_interval;
  u_int16_t rxmt_interval;
  u_int16_t metric;
- u_int8_t priority;
+ int16_t priority;
 };
 
 struct config_defaults globaldefs;
@@ -111,6 +111,7 @@ struct config_defaults *defs;
 
 struct area *conf_get_area(struct in_addr);
 int conf_check_rdomain(u_int);
+void iface_settings(struct iface *, struct config_defaults *);
 
 typedef struct {
  union {
@@ -465,9 +466,14 @@ comma : ','
 area : AREA areaid {
  area = conf_get_area($2);
 
- memcpy(&areadefs, defs, sizeof(areadefs));
+ memset(&areadefs, 0, sizeof(areadefs));
+ areadefs.priority = -1;
  defs = &areadefs;
  } '{' optnl areaopts_l '}' {
+ struct iface *i;
+ LIST_FOREACH(i, &area->iface_list, entry) {
+ iface_settings(i, &areadefs);
+ }
  area = NULL;
  defs = &globaldefs;
  }
@@ -540,15 +546,12 @@ interface : INTERFACE STRING {
  iface->area = area;
  LIST_INSERT_HEAD(&area->iface_list, iface, entry);
 
- memcpy(&ifacedefs, defs, sizeof(ifacedefs));
+ memset(&ifacedefs, 0, sizeof(ifacedefs));
+ ifacedefs.priority = -1;
  defs = &ifacedefs;
  } interface_block {
- iface->dead_interval = defs->dead_interval;
- iface->transmit_delay = defs->transmit_delay;
- iface->hello_interval = defs->hello_interval;
- iface->rxmt_interval = defs->rxmt_interval;
- iface->metric = defs->metric;
- iface->priority = defs->priority;
+ iface->priority = -1;
+ iface_settings(iface, defs);
  iface->cflags |= F_IFACE_CONFIGURED;
  iface = NULL;
  /* interface is always part of an area */
@@ -1018,6 +1021,8 @@ popfile(void)
 struct ospfd_conf *
 parse_config(char *filename, int opts)
 {
+ struct area *a;
+ struct iface *i;
  struct sym *sym, *next;
 
  if ((conf = calloc(1, sizeof(struct ospfd_conf))) == NULL)
@@ -1068,6 +1073,10 @@ parse_config(char *filename, int opts)
  }
  }
 
+ LIST_FOREACH(a, &conf->area_list, entry)
+ LIST_FOREACH(i, &a->iface_list, entry)
+ iface_settings(i, defs);
+
  /* check that all interfaces belong to the configured rdomain */
  errors += conf_check_rdomain(conf->rdomain);
 
@@ -1319,4 +1328,21 @@ prefix(const char *s, struct in6_addr *a
  }
  *plen = 128;
  return (host(s, addr));
+}
+
+void
+iface_settings(struct iface *i, struct config_defaults *cfg)
+{
+ if (i->dead_interval == 0)
+ i->dead_interval = cfg->dead_interval;
+ if (i->transmit_delay == 0)
+ i->transmit_delay = cfg->transmit_delay;
+ if (i->hello_interval == 0)
+ i->hello_interval = cfg->hello_interval;
+ if (i->rxmt_interval == 0)
+ i->rxmt_interval = cfg->rxmt_interval;
+ if (i->metric == 0)
+ i->metric = cfg->metric;
+ if (i->priority == -1)
+ i->priority = cfg->priority;
 }
Index: ospfd/logmsg.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/logmsg.c,v
retrieving revision 1.1
diff -u -p -r1.1 logmsg.c
--- ospfd/logmsg.c 2 Sep 2016 14:04:25 -0000 1.1
+++ ospfd/logmsg.c 8 Jan 2020 12:11:20 -0000
@@ -101,15 +101,13 @@ const char *
 if_auth_name(enum auth_type type)
 {
  switch (type) {
- case AUTH_NONE:
- return ("none");
  case AUTH_SIMPLE:
  return ("simple");
  case AUTH_CRYPT:
  return ("crypt");
+ default:
+ return ("none");
  }
- /* NOTREACHED */
- return ("unknown");
 }
 
 const char *
Index: ospfd/ospf.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospf.h,v
retrieving revision 1.23
diff -u -p -r1.23 ospf.h
--- ospfd/ospf.h 17 Jan 2013 09:14:15 -0000 1.23
+++ ospfd/ospf.h 8 Jan 2020 12:11:20 -0000
@@ -102,10 +102,6 @@
 #define PACKET_TYPE_LS_ACK 5
 
 /* OSPF auth types */
-#define AUTH_TYPE_NONE 0
-#define AUTH_TYPE_SIMPLE 1
-#define AUTH_TYPE_CRYPT 2
-
 #define MIN_AUTHTYPE 0
 #define MAX_AUTHTYPE 2
 
Index: ospfd/ospfd.h
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
retrieving revision 1.105
diff -u -p -r1.105 ospfd.h
--- ospfd/ospfd.h 19 Nov 2019 09:55:55 -0000 1.105
+++ ospfd/ospfd.h 8 Jan 2020 12:11:20 -0000
@@ -275,6 +275,7 @@ enum nbr_action {
 
 /* auth types */
 enum auth_type {
+ AUTH_UNSET = -1,
  AUTH_NONE,
  AUTH_SIMPLE,
  AUTH_CRYPT
@@ -360,9 +361,9 @@ struct iface {
  enum iface_type type;
  enum auth_type auth_type;
  u_int8_t if_type;
- u_int8_t auth_keyid;
+ int16_t auth_keyid;
  u_int8_t linkstate;
- u_int8_t priority;
+ int16_t priority;
  u_int8_t passive;
  u_int8_t p2p;
 };
Index: ospfd/parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
retrieving revision 1.99
diff -u -p -r1.99 parse.y
--- ospfd/parse.y 19 Nov 2019 09:55:55 -0000 1.99
+++ ospfd/parse.y 8 Jan 2020 12:11:20 -0000
@@ -101,8 +101,8 @@ struct config_defaults {
  u_int16_t rxmt_interval;
  u_int16_t metric;
  enum auth_type auth_type;
- u_int8_t auth_keyid;
- u_int8_t priority;
+ int16_t auth_keyid;
+ int16_t priority;
 };
 
 struct config_defaults globaldefs;
@@ -113,6 +113,7 @@ struct config_defaults *defs;
 struct area *conf_get_area(struct in_addr);
 struct iface *conf_get_if(struct kif *, struct kif_addr *);
 int conf_check_rdomain(unsigned int);
+void iface_settings(struct iface *, struct config_defaults *);
 
 typedef struct {
  union {
@@ -592,10 +593,17 @@ comma : ','
 area : AREA areaid {
  area = conf_get_area($2);
 
- memcpy(&areadefs, defs, sizeof(areadefs));
- md_list_copy(&areadefs.md_list, &defs->md_list);
+ memset(&areadefs, 0, sizeof(areadefs));
+ areadefs.priority = -1;
+ areadefs.auth_keyid = -1;
+ areadefs.auth_type = AUTH_UNSET;
+ TAILQ_INIT(&areadefs.md_list);
  defs = &areadefs;
  } '{' optnl areaopts_l '}' {
+ struct iface *i;
+ LIST_FOREACH(i, &area->iface_list, entry) {
+ iface_settings(i, &areadefs);
+ }
  area = NULL;
  md_list_clr(&defs->md_list);
  defs = &globaldefs;
@@ -707,25 +715,17 @@ interface : INTERFACE STRING {
  iface->area = area;
  LIST_INSERT_HEAD(&area->iface_list, iface, entry);
 
- memcpy(&ifacedefs, defs, sizeof(ifacedefs));
- md_list_copy(&ifacedefs.md_list, &defs->md_list);
+ memset(&ifacedefs, 0, sizeof(ifacedefs));
+ ifacedefs.priority = -1;
+ ifacedefs.auth_keyid = -1;
+ ifacedefs.auth_type = AUTH_UNSET;
+ TAILQ_INIT(&ifacedefs.md_list);
  defs = &ifacedefs;
  } interface_block {
- iface->dead_interval = defs->dead_interval;
- iface->fast_hello_interval = defs->fast_hello_interval;
- iface->transmit_delay = defs->transmit_delay;
- if (iface->dead_interval == FAST_RTR_DEAD_TIME)
- iface->hello_interval = 0;
- else
- iface->hello_interval = defs->hello_interval;
- iface->rxmt_interval = defs->rxmt_interval;
- iface->metric = defs->metric;
- iface->priority = defs->priority;
- iface->auth_type = defs->auth_type;
- iface->auth_keyid = defs->auth_keyid;
- memcpy(iface->auth_key, defs->auth_key,
-    sizeof(iface->auth_key));
- md_list_copy(&iface->auth_md_list, &defs->md_list);
+ iface->priority = -1;
+ iface->auth_keyid = -1;
+ iface->auth_type = AUTH_UNSET;
+ iface_settings(iface, defs);
  md_list_clr(&defs->md_list);
  iface = NULL;
  /* interface is always part of an area */
@@ -1207,6 +1207,8 @@ popfile(void)
 struct ospfd_conf *
 parse_config(char *filename, int opts)
 {
+ struct area *a;
+ struct iface *i;
  struct sym *sym, *next;
 
  if ((conf = calloc(1, sizeof(struct ospfd_conf))) == NULL)
@@ -1259,6 +1261,10 @@ parse_config(char *filename, int opts)
  }
  }
 
+ LIST_FOREACH(a, &conf->area_list, entry)
+ LIST_FOREACH(i, &a->iface_list, entry)
+ iface_settings(i, defs);
+
  /* free global config defaults */
  md_list_clr(&globaldefs.md_list);
 
@@ -1317,7 +1323,7 @@ int
 cmdline_symset(char *s)
 {
  char *sym, *val;
- int ret;
+ int ret;
 
  if ((val = strrchr(s, '=')) == NULL)
  return (-1);
@@ -1501,4 +1507,33 @@ host(const char *s, struct in_addr *addr
  mask->s_addr = prefixlen2mask(bits);
 
  return (1);
+}
+
+void
+iface_settings(struct iface *i, struct config_defaults *cfg)
+{
+ if (i->dead_interval == 0)
+ i->dead_interval = cfg->dead_interval;
+ if (i->dead_interval == FAST_RTR_DEAD_TIME)
+ i->hello_interval = 0;
+ else if (i->hello_interval == 0)
+ i->hello_interval = cfg->hello_interval;
+ if (i->transmit_delay == 0)
+ i->transmit_delay = cfg->transmit_delay;
+ if (i->fast_hello_interval == 0)
+ i->fast_hello_interval = cfg->fast_hello_interval;
+ if (i->rxmt_interval == 0)
+ i->rxmt_interval = cfg->rxmt_interval;
+ if (i->metric == 0)
+ i->metric = cfg->metric;
+ if (i->priority == -1)
+ i->priority = cfg->priority;
+ if (i->auth_keyid == -1)
+ i->auth_keyid = cfg->auth_keyid;
+ if (i->auth_type == AUTH_UNSET)
+ i->auth_type = cfg->auth_type;
+ if (strlen(i->auth_key) == 0)
+ memcpy(i->auth_key, cfg->auth_key, sizeof(i->auth_key));
+ if (TAILQ_EMPTY(&i->auth_md_list))
+ md_list_copy(&i->auth_md_list, &cfg->md_list);
 }
Index: ospfd/printconf.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
retrieving revision 1.21
diff -u -p -r1.21 printconf.c
--- ospfd/printconf.c 19 Nov 2019 09:55:55 -0000 1.21
+++ ospfd/printconf.c 8 Jan 2020 12:11:20 -0000
@@ -154,12 +154,12 @@ print_iface(struct iface *iface)
 
  printf("\t\tauth-type %s\n", if_auth_name(iface->auth_type));
  switch (iface->auth_type) {
- case AUTH_TYPE_NONE:
+ case AUTH_NONE:
  break;
- case AUTH_TYPE_SIMPLE:
+ case AUTH_SIMPLE:
  printf("\t\tauth-key XXXXXX\n");
  break;
- case AUTH_TYPE_CRYPT:
+ case AUTH_CRYPT:
  printf("\t\tauth-md-keyid %d\n", iface->auth_keyid);
  TAILQ_FOREACH(md, &iface->auth_md_list, entry)
  printf("\t\tauth-md %d XXXXXX\n", md->keyid);

Reply | Threaded
Open this post in threaded view
|

Re: ospf(6)d.conf: define interface parameters per area or globally

Remi Locherer
On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > I have a diff to allow parameters after interface or area definition.
> > > Not sure if we want to do that though.
> >
> > I would appreciate that! ;-)
> >
>
> The ospfd diff needs some more work. Crypt authentication handling is not
> perfect.

This works fine for me and the diff reads good. I tested ospfd and ospf6d.
Also the crypt options for ospfd worked fine.

ok remi@

>
> Index: ospf6d/ospf6d.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v
> retrieving revision 1.44
> diff -u -p -r1.44 ospf6d.h
> --- ospf6d/ospf6d.h 3 Jan 2020 17:45:02 -0000 1.44
> +++ ospf6d/ospf6d.h 8 Jan 2020 12:11:20 -0000
> @@ -328,7 +328,7 @@ struct iface {
>   enum iface_type type;
>   u_int8_t if_type;
>   u_int8_t linkstate;
> - u_int8_t priority;
> + int16_t priority;
>   u_int8_t p2p;
>   u_int8_t cflags;
>  #define F_IFACE_PASSIVE 0x01
> Index: ospf6d/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/parse.y,v
> retrieving revision 1.48
> diff -u -p -r1.48 parse.y
> --- ospf6d/parse.y 26 Dec 2019 10:24:18 -0000 1.48
> +++ ospf6d/parse.y 8 Jan 2020 12:11:20 -0000
> @@ -101,7 +101,7 @@ struct config_defaults {
>   u_int16_t hello_interval;
>   u_int16_t rxmt_interval;
>   u_int16_t metric;
> - u_int8_t priority;
> + int16_t priority;
>  };
>  
>  struct config_defaults globaldefs;
> @@ -111,6 +111,7 @@ struct config_defaults *defs;
>  
>  struct area *conf_get_area(struct in_addr);
>  int conf_check_rdomain(u_int);
> +void iface_settings(struct iface *, struct config_defaults *);
>  
>  typedef struct {
>   union {
> @@ -465,9 +466,14 @@ comma : ','
>  area : AREA areaid {
>   area = conf_get_area($2);
>  
> - memcpy(&areadefs, defs, sizeof(areadefs));
> + memset(&areadefs, 0, sizeof(areadefs));
> + areadefs.priority = -1;
>   defs = &areadefs;
>   } '{' optnl areaopts_l '}' {
> + struct iface *i;
> + LIST_FOREACH(i, &area->iface_list, entry) {
> + iface_settings(i, &areadefs);
> + }
>   area = NULL;
>   defs = &globaldefs;
>   }
> @@ -540,15 +546,12 @@ interface : INTERFACE STRING {
>   iface->area = area;
>   LIST_INSERT_HEAD(&area->iface_list, iface, entry);
>  
> - memcpy(&ifacedefs, defs, sizeof(ifacedefs));
> + memset(&ifacedefs, 0, sizeof(ifacedefs));
> + ifacedefs.priority = -1;
>   defs = &ifacedefs;
>   } interface_block {
> - iface->dead_interval = defs->dead_interval;
> - iface->transmit_delay = defs->transmit_delay;
> - iface->hello_interval = defs->hello_interval;
> - iface->rxmt_interval = defs->rxmt_interval;
> - iface->metric = defs->metric;
> - iface->priority = defs->priority;
> + iface->priority = -1;
> + iface_settings(iface, defs);
>   iface->cflags |= F_IFACE_CONFIGURED;
>   iface = NULL;
>   /* interface is always part of an area */
> @@ -1018,6 +1021,8 @@ popfile(void)
>  struct ospfd_conf *
>  parse_config(char *filename, int opts)
>  {
> + struct area *a;
> + struct iface *i;
>   struct sym *sym, *next;
>  
>   if ((conf = calloc(1, sizeof(struct ospfd_conf))) == NULL)
> @@ -1068,6 +1073,10 @@ parse_config(char *filename, int opts)
>   }
>   }
>  
> + LIST_FOREACH(a, &conf->area_list, entry)
> + LIST_FOREACH(i, &a->iface_list, entry)
> + iface_settings(i, defs);
> +
>   /* check that all interfaces belong to the configured rdomain */
>   errors += conf_check_rdomain(conf->rdomain);
>  
> @@ -1319,4 +1328,21 @@ prefix(const char *s, struct in6_addr *a
>   }
>   *plen = 128;
>   return (host(s, addr));
> +}
> +
> +void
> +iface_settings(struct iface *i, struct config_defaults *cfg)
> +{
> + if (i->dead_interval == 0)
> + i->dead_interval = cfg->dead_interval;
> + if (i->transmit_delay == 0)
> + i->transmit_delay = cfg->transmit_delay;
> + if (i->hello_interval == 0)
> + i->hello_interval = cfg->hello_interval;
> + if (i->rxmt_interval == 0)
> + i->rxmt_interval = cfg->rxmt_interval;
> + if (i->metric == 0)
> + i->metric = cfg->metric;
> + if (i->priority == -1)
> + i->priority = cfg->priority;
>  }
> Index: ospfd/logmsg.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/logmsg.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 logmsg.c
> --- ospfd/logmsg.c 2 Sep 2016 14:04:25 -0000 1.1
> +++ ospfd/logmsg.c 8 Jan 2020 12:11:20 -0000
> @@ -101,15 +101,13 @@ const char *
>  if_auth_name(enum auth_type type)
>  {
>   switch (type) {
> - case AUTH_NONE:
> - return ("none");
>   case AUTH_SIMPLE:
>   return ("simple");
>   case AUTH_CRYPT:
>   return ("crypt");
> + default:
> + return ("none");
>   }
> - /* NOTREACHED */
> - return ("unknown");
>  }
>  
>  const char *
> Index: ospfd/ospf.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospf.h,v
> retrieving revision 1.23
> diff -u -p -r1.23 ospf.h
> --- ospfd/ospf.h 17 Jan 2013 09:14:15 -0000 1.23
> +++ ospfd/ospf.h 8 Jan 2020 12:11:20 -0000
> @@ -102,10 +102,6 @@
>  #define PACKET_TYPE_LS_ACK 5
>  
>  /* OSPF auth types */
> -#define AUTH_TYPE_NONE 0
> -#define AUTH_TYPE_SIMPLE 1
> -#define AUTH_TYPE_CRYPT 2
> -
>  #define MIN_AUTHTYPE 0
>  #define MAX_AUTHTYPE 2
>  
> Index: ospfd/ospfd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
> retrieving revision 1.105
> diff -u -p -r1.105 ospfd.h
> --- ospfd/ospfd.h 19 Nov 2019 09:55:55 -0000 1.105
> +++ ospfd/ospfd.h 8 Jan 2020 12:11:20 -0000
> @@ -275,6 +275,7 @@ enum nbr_action {
>  
>  /* auth types */
>  enum auth_type {
> + AUTH_UNSET = -1,
>   AUTH_NONE,
>   AUTH_SIMPLE,
>   AUTH_CRYPT
> @@ -360,9 +361,9 @@ struct iface {
>   enum iface_type type;
>   enum auth_type auth_type;
>   u_int8_t if_type;
> - u_int8_t auth_keyid;
> + int16_t auth_keyid;
>   u_int8_t linkstate;
> - u_int8_t priority;
> + int16_t priority;
>   u_int8_t passive;
>   u_int8_t p2p;
>  };
> Index: ospfd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
> retrieving revision 1.99
> diff -u -p -r1.99 parse.y
> --- ospfd/parse.y 19 Nov 2019 09:55:55 -0000 1.99
> +++ ospfd/parse.y 8 Jan 2020 12:11:20 -0000
> @@ -101,8 +101,8 @@ struct config_defaults {
>   u_int16_t rxmt_interval;
>   u_int16_t metric;
>   enum auth_type auth_type;
> - u_int8_t auth_keyid;
> - u_int8_t priority;
> + int16_t auth_keyid;
> + int16_t priority;
>  };
>  
>  struct config_defaults globaldefs;
> @@ -113,6 +113,7 @@ struct config_defaults *defs;
>  struct area *conf_get_area(struct in_addr);
>  struct iface *conf_get_if(struct kif *, struct kif_addr *);
>  int conf_check_rdomain(unsigned int);
> +void iface_settings(struct iface *, struct config_defaults *);
>  
>  typedef struct {
>   union {
> @@ -592,10 +593,17 @@ comma : ','
>  area : AREA areaid {
>   area = conf_get_area($2);
>  
> - memcpy(&areadefs, defs, sizeof(areadefs));
> - md_list_copy(&areadefs.md_list, &defs->md_list);
> + memset(&areadefs, 0, sizeof(areadefs));
> + areadefs.priority = -1;
> + areadefs.auth_keyid = -1;
> + areadefs.auth_type = AUTH_UNSET;
> + TAILQ_INIT(&areadefs.md_list);
>   defs = &areadefs;
>   } '{' optnl areaopts_l '}' {
> + struct iface *i;
> + LIST_FOREACH(i, &area->iface_list, entry) {
> + iface_settings(i, &areadefs);
> + }
>   area = NULL;
>   md_list_clr(&defs->md_list);
>   defs = &globaldefs;
> @@ -707,25 +715,17 @@ interface : INTERFACE STRING {
>   iface->area = area;
>   LIST_INSERT_HEAD(&area->iface_list, iface, entry);
>  
> - memcpy(&ifacedefs, defs, sizeof(ifacedefs));
> - md_list_copy(&ifacedefs.md_list, &defs->md_list);
> + memset(&ifacedefs, 0, sizeof(ifacedefs));
> + ifacedefs.priority = -1;
> + ifacedefs.auth_keyid = -1;
> + ifacedefs.auth_type = AUTH_UNSET;
> + TAILQ_INIT(&ifacedefs.md_list);
>   defs = &ifacedefs;
>   } interface_block {
> - iface->dead_interval = defs->dead_interval;
> - iface->fast_hello_interval = defs->fast_hello_interval;
> - iface->transmit_delay = defs->transmit_delay;
> - if (iface->dead_interval == FAST_RTR_DEAD_TIME)
> - iface->hello_interval = 0;
> - else
> - iface->hello_interval = defs->hello_interval;
> - iface->rxmt_interval = defs->rxmt_interval;
> - iface->metric = defs->metric;
> - iface->priority = defs->priority;
> - iface->auth_type = defs->auth_type;
> - iface->auth_keyid = defs->auth_keyid;
> - memcpy(iface->auth_key, defs->auth_key,
> -    sizeof(iface->auth_key));
> - md_list_copy(&iface->auth_md_list, &defs->md_list);
> + iface->priority = -1;
> + iface->auth_keyid = -1;
> + iface->auth_type = AUTH_UNSET;
> + iface_settings(iface, defs);
>   md_list_clr(&defs->md_list);
>   iface = NULL;
>   /* interface is always part of an area */
> @@ -1207,6 +1207,8 @@ popfile(void)
>  struct ospfd_conf *
>  parse_config(char *filename, int opts)
>  {
> + struct area *a;
> + struct iface *i;
>   struct sym *sym, *next;
>  
>   if ((conf = calloc(1, sizeof(struct ospfd_conf))) == NULL)
> @@ -1259,6 +1261,10 @@ parse_config(char *filename, int opts)
>   }
>   }
>  
> + LIST_FOREACH(a, &conf->area_list, entry)
> + LIST_FOREACH(i, &a->iface_list, entry)
> + iface_settings(i, defs);
> +
>   /* free global config defaults */
>   md_list_clr(&globaldefs.md_list);
>  
> @@ -1317,7 +1323,7 @@ int
>  cmdline_symset(char *s)
>  {
>   char *sym, *val;
> - int ret;
> + int ret;
>  
>   if ((val = strrchr(s, '=')) == NULL)
>   return (-1);
> @@ -1501,4 +1507,33 @@ host(const char *s, struct in_addr *addr
>   mask->s_addr = prefixlen2mask(bits);
>  
>   return (1);
> +}
> +
> +void
> +iface_settings(struct iface *i, struct config_defaults *cfg)
> +{
> + if (i->dead_interval == 0)
> + i->dead_interval = cfg->dead_interval;
> + if (i->dead_interval == FAST_RTR_DEAD_TIME)
> + i->hello_interval = 0;
> + else if (i->hello_interval == 0)
> + i->hello_interval = cfg->hello_interval;
> + if (i->transmit_delay == 0)
> + i->transmit_delay = cfg->transmit_delay;
> + if (i->fast_hello_interval == 0)
> + i->fast_hello_interval = cfg->fast_hello_interval;
> + if (i->rxmt_interval == 0)
> + i->rxmt_interval = cfg->rxmt_interval;
> + if (i->metric == 0)
> + i->metric = cfg->metric;
> + if (i->priority == -1)
> + i->priority = cfg->priority;
> + if (i->auth_keyid == -1)
> + i->auth_keyid = cfg->auth_keyid;
> + if (i->auth_type == AUTH_UNSET)
> + i->auth_type = cfg->auth_type;
> + if (strlen(i->auth_key) == 0)
> + memcpy(i->auth_key, cfg->auth_key, sizeof(i->auth_key));
> + if (TAILQ_EMPTY(&i->auth_md_list))
> + md_list_copy(&i->auth_md_list, &cfg->md_list);
>  }
> Index: ospfd/printconf.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 printconf.c
> --- ospfd/printconf.c 19 Nov 2019 09:55:55 -0000 1.21
> +++ ospfd/printconf.c 8 Jan 2020 12:11:20 -0000
> @@ -154,12 +154,12 @@ print_iface(struct iface *iface)
>  
>   printf("\t\tauth-type %s\n", if_auth_name(iface->auth_type));
>   switch (iface->auth_type) {
> - case AUTH_TYPE_NONE:
> + case AUTH_NONE:
>   break;
> - case AUTH_TYPE_SIMPLE:
> + case AUTH_SIMPLE:
>   printf("\t\tauth-key XXXXXX\n");
>   break;
> - case AUTH_TYPE_CRYPT:
> + case AUTH_CRYPT:
>   printf("\t\tauth-md-keyid %d\n", iface->auth_keyid);
>   TAILQ_FOREACH(md, &iface->auth_md_list, entry)
>   printf("\t\tauth-md %d XXXXXX\n", md->keyid);
>

Reply | Threaded
Open this post in threaded view
|

Re: ospf(6)d.conf: define interface parameters per area or globally

Claudio Jeker
On Sun, Jan 12, 2020 at 03:46:15PM +0100, Remi Locherer wrote:

> On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> > On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > > I have a diff to allow parameters after interface or area definition.
> > > > Not sure if we want to do that though.
> > >
> > > I would appreciate that! ;-)
> > >
> >
> > The ospfd diff needs some more work. Crypt authentication handling is not
> > perfect.
>
> This works fine for me and the diff reads good. I tested ospfd and ospf6d.
> Also the crypt options for ospfd worked fine.
>
> ok remi@

Currently all daemons behave the same way and inherit at the moment of
creation. Having this behave different between daemons is confusing.
At least ospfd and bgpd should behave the same. Not saying that the
current behaviour is great.
I think in the case of ospfd the way auth-md is handled by this diff is
not comparable with the behaviour of the other settings.

area 0.0.0.0 {
        hello-interval 10
        auth-md 1 foo

        interface em0

        hello-interval 20
        auth-md 1 bar
        auth-md 2 foofoo

        interface em1 {
                auth-md 3 barbar
        }

        hello-interval 30
        auth-md 1 bay
        auth-md 2 foobar
}

What values for hello-interval and auth-md should be set on em0 and em1?
 

> >
> > Index: ospf6d/ospf6d.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v
> > retrieving revision 1.44
> > diff -u -p -r1.44 ospf6d.h
> > --- ospf6d/ospf6d.h 3 Jan 2020 17:45:02 -0000 1.44
> > +++ ospf6d/ospf6d.h 8 Jan 2020 12:11:20 -0000
> > @@ -328,7 +328,7 @@ struct iface {
> >   enum iface_type type;
> >   u_int8_t if_type;
> >   u_int8_t linkstate;
> > - u_int8_t priority;
> > + int16_t priority;
> >   u_int8_t p2p;
> >   u_int8_t cflags;
> >  #define F_IFACE_PASSIVE 0x01
> > Index: ospf6d/parse.y
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospf6d/parse.y,v
> > retrieving revision 1.48
> > diff -u -p -r1.48 parse.y
> > --- ospf6d/parse.y 26 Dec 2019 10:24:18 -0000 1.48
> > +++ ospf6d/parse.y 8 Jan 2020 12:11:20 -0000
> > @@ -101,7 +101,7 @@ struct config_defaults {
> >   u_int16_t hello_interval;
> >   u_int16_t rxmt_interval;
> >   u_int16_t metric;
> > - u_int8_t priority;
> > + int16_t priority;
> >  };
> >  
> >  struct config_defaults globaldefs;
> > @@ -111,6 +111,7 @@ struct config_defaults *defs;
> >  
> >  struct area *conf_get_area(struct in_addr);
> >  int conf_check_rdomain(u_int);
> > +void iface_settings(struct iface *, struct config_defaults *);
> >  
> >  typedef struct {
> >   union {
> > @@ -465,9 +466,14 @@ comma : ','
> >  area : AREA areaid {
> >   area = conf_get_area($2);
> >  
> > - memcpy(&areadefs, defs, sizeof(areadefs));
> > + memset(&areadefs, 0, sizeof(areadefs));
> > + areadefs.priority = -1;
> >   defs = &areadefs;
> >   } '{' optnl areaopts_l '}' {
> > + struct iface *i;
> > + LIST_FOREACH(i, &area->iface_list, entry) {
> > + iface_settings(i, &areadefs);
> > + }
> >   area = NULL;
> >   defs = &globaldefs;
> >   }
> > @@ -540,15 +546,12 @@ interface : INTERFACE STRING {
> >   iface->area = area;
> >   LIST_INSERT_HEAD(&area->iface_list, iface, entry);
> >  
> > - memcpy(&ifacedefs, defs, sizeof(ifacedefs));
> > + memset(&ifacedefs, 0, sizeof(ifacedefs));
> > + ifacedefs.priority = -1;
> >   defs = &ifacedefs;
> >   } interface_block {
> > - iface->dead_interval = defs->dead_interval;
> > - iface->transmit_delay = defs->transmit_delay;
> > - iface->hello_interval = defs->hello_interval;
> > - iface->rxmt_interval = defs->rxmt_interval;
> > - iface->metric = defs->metric;
> > - iface->priority = defs->priority;
> > + iface->priority = -1;
> > + iface_settings(iface, defs);
> >   iface->cflags |= F_IFACE_CONFIGURED;
> >   iface = NULL;
> >   /* interface is always part of an area */
> > @@ -1018,6 +1021,8 @@ popfile(void)
> >  struct ospfd_conf *
> >  parse_config(char *filename, int opts)
> >  {
> > + struct area *a;
> > + struct iface *i;
> >   struct sym *sym, *next;
> >  
> >   if ((conf = calloc(1, sizeof(struct ospfd_conf))) == NULL)
> > @@ -1068,6 +1073,10 @@ parse_config(char *filename, int opts)
> >   }
> >   }
> >  
> > + LIST_FOREACH(a, &conf->area_list, entry)
> > + LIST_FOREACH(i, &a->iface_list, entry)
> > + iface_settings(i, defs);
> > +
> >   /* check that all interfaces belong to the configured rdomain */
> >   errors += conf_check_rdomain(conf->rdomain);
> >  
> > @@ -1319,4 +1328,21 @@ prefix(const char *s, struct in6_addr *a
> >   }
> >   *plen = 128;
> >   return (host(s, addr));
> > +}
> > +
> > +void
> > +iface_settings(struct iface *i, struct config_defaults *cfg)
> > +{
> > + if (i->dead_interval == 0)
> > + i->dead_interval = cfg->dead_interval;
> > + if (i->transmit_delay == 0)
> > + i->transmit_delay = cfg->transmit_delay;
> > + if (i->hello_interval == 0)
> > + i->hello_interval = cfg->hello_interval;
> > + if (i->rxmt_interval == 0)
> > + i->rxmt_interval = cfg->rxmt_interval;
> > + if (i->metric == 0)
> > + i->metric = cfg->metric;
> > + if (i->priority == -1)
> > + i->priority = cfg->priority;
> >  }
> > Index: ospfd/logmsg.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospfd/logmsg.c,v
> > retrieving revision 1.1
> > diff -u -p -r1.1 logmsg.c
> > --- ospfd/logmsg.c 2 Sep 2016 14:04:25 -0000 1.1
> > +++ ospfd/logmsg.c 8 Jan 2020 12:11:20 -0000
> > @@ -101,15 +101,13 @@ const char *
> >  if_auth_name(enum auth_type type)
> >  {
> >   switch (type) {
> > - case AUTH_NONE:
> > - return ("none");
> >   case AUTH_SIMPLE:
> >   return ("simple");
> >   case AUTH_CRYPT:
> >   return ("crypt");
> > + default:
> > + return ("none");
> >   }
> > - /* NOTREACHED */
> > - return ("unknown");
> >  }
> >  
> >  const char *
> > Index: ospfd/ospf.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospfd/ospf.h,v
> > retrieving revision 1.23
> > diff -u -p -r1.23 ospf.h
> > --- ospfd/ospf.h 17 Jan 2013 09:14:15 -0000 1.23
> > +++ ospfd/ospf.h 8 Jan 2020 12:11:20 -0000
> > @@ -102,10 +102,6 @@
> >  #define PACKET_TYPE_LS_ACK 5
> >  
> >  /* OSPF auth types */
> > -#define AUTH_TYPE_NONE 0
> > -#define AUTH_TYPE_SIMPLE 1
> > -#define AUTH_TYPE_CRYPT 2
> > -
> >  #define MIN_AUTHTYPE 0
> >  #define MAX_AUTHTYPE 2
> >  
> > Index: ospfd/ospfd.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
> > retrieving revision 1.105
> > diff -u -p -r1.105 ospfd.h
> > --- ospfd/ospfd.h 19 Nov 2019 09:55:55 -0000 1.105
> > +++ ospfd/ospfd.h 8 Jan 2020 12:11:20 -0000
> > @@ -275,6 +275,7 @@ enum nbr_action {
> >  
> >  /* auth types */
> >  enum auth_type {
> > + AUTH_UNSET = -1,
> >   AUTH_NONE,
> >   AUTH_SIMPLE,
> >   AUTH_CRYPT
> > @@ -360,9 +361,9 @@ struct iface {
> >   enum iface_type type;
> >   enum auth_type auth_type;
> >   u_int8_t if_type;
> > - u_int8_t auth_keyid;
> > + int16_t auth_keyid;
> >   u_int8_t linkstate;
> > - u_int8_t priority;
> > + int16_t priority;
> >   u_int8_t passive;
> >   u_int8_t p2p;
> >  };
> > Index: ospfd/parse.y
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
> > retrieving revision 1.99
> > diff -u -p -r1.99 parse.y
> > --- ospfd/parse.y 19 Nov 2019 09:55:55 -0000 1.99
> > +++ ospfd/parse.y 8 Jan 2020 12:11:20 -0000
> > @@ -101,8 +101,8 @@ struct config_defaults {
> >   u_int16_t rxmt_interval;
> >   u_int16_t metric;
> >   enum auth_type auth_type;
> > - u_int8_t auth_keyid;
> > - u_int8_t priority;
> > + int16_t auth_keyid;
> > + int16_t priority;
> >  };
> >  
> >  struct config_defaults globaldefs;
> > @@ -113,6 +113,7 @@ struct config_defaults *defs;
> >  struct area *conf_get_area(struct in_addr);
> >  struct iface *conf_get_if(struct kif *, struct kif_addr *);
> >  int conf_check_rdomain(unsigned int);
> > +void iface_settings(struct iface *, struct config_defaults *);
> >  
> >  typedef struct {
> >   union {
> > @@ -592,10 +593,17 @@ comma : ','
> >  area : AREA areaid {
> >   area = conf_get_area($2);
> >  
> > - memcpy(&areadefs, defs, sizeof(areadefs));
> > - md_list_copy(&areadefs.md_list, &defs->md_list);
> > + memset(&areadefs, 0, sizeof(areadefs));
> > + areadefs.priority = -1;
> > + areadefs.auth_keyid = -1;
> > + areadefs.auth_type = AUTH_UNSET;
> > + TAILQ_INIT(&areadefs.md_list);
> >   defs = &areadefs;
> >   } '{' optnl areaopts_l '}' {
> > + struct iface *i;
> > + LIST_FOREACH(i, &area->iface_list, entry) {
> > + iface_settings(i, &areadefs);
> > + }
> >   area = NULL;
> >   md_list_clr(&defs->md_list);
> >   defs = &globaldefs;
> > @@ -707,25 +715,17 @@ interface : INTERFACE STRING {
> >   iface->area = area;
> >   LIST_INSERT_HEAD(&area->iface_list, iface, entry);
> >  
> > - memcpy(&ifacedefs, defs, sizeof(ifacedefs));
> > - md_list_copy(&ifacedefs.md_list, &defs->md_list);
> > + memset(&ifacedefs, 0, sizeof(ifacedefs));
> > + ifacedefs.priority = -1;
> > + ifacedefs.auth_keyid = -1;
> > + ifacedefs.auth_type = AUTH_UNSET;
> > + TAILQ_INIT(&ifacedefs.md_list);
> >   defs = &ifacedefs;
> >   } interface_block {
> > - iface->dead_interval = defs->dead_interval;
> > - iface->fast_hello_interval = defs->fast_hello_interval;
> > - iface->transmit_delay = defs->transmit_delay;
> > - if (iface->dead_interval == FAST_RTR_DEAD_TIME)
> > - iface->hello_interval = 0;
> > - else
> > - iface->hello_interval = defs->hello_interval;
> > - iface->rxmt_interval = defs->rxmt_interval;
> > - iface->metric = defs->metric;
> > - iface->priority = defs->priority;
> > - iface->auth_type = defs->auth_type;
> > - iface->auth_keyid = defs->auth_keyid;
> > - memcpy(iface->auth_key, defs->auth_key,
> > -    sizeof(iface->auth_key));
> > - md_list_copy(&iface->auth_md_list, &defs->md_list);
> > + iface->priority = -1;
> > + iface->auth_keyid = -1;
> > + iface->auth_type = AUTH_UNSET;
> > + iface_settings(iface, defs);
> >   md_list_clr(&defs->md_list);
> >   iface = NULL;
> >   /* interface is always part of an area */
> > @@ -1207,6 +1207,8 @@ popfile(void)
> >  struct ospfd_conf *
> >  parse_config(char *filename, int opts)
> >  {
> > + struct area *a;
> > + struct iface *i;
> >   struct sym *sym, *next;
> >  
> >   if ((conf = calloc(1, sizeof(struct ospfd_conf))) == NULL)
> > @@ -1259,6 +1261,10 @@ parse_config(char *filename, int opts)
> >   }
> >   }
> >  
> > + LIST_FOREACH(a, &conf->area_list, entry)
> > + LIST_FOREACH(i, &a->iface_list, entry)
> > + iface_settings(i, defs);
> > +
> >   /* free global config defaults */
> >   md_list_clr(&globaldefs.md_list);
> >  
> > @@ -1317,7 +1323,7 @@ int
> >  cmdline_symset(char *s)
> >  {
> >   char *sym, *val;
> > - int ret;
> > + int ret;
> >  
> >   if ((val = strrchr(s, '=')) == NULL)
> >   return (-1);
> > @@ -1501,4 +1507,33 @@ host(const char *s, struct in_addr *addr
> >   mask->s_addr = prefixlen2mask(bits);
> >  
> >   return (1);
> > +}
> > +
> > +void
> > +iface_settings(struct iface *i, struct config_defaults *cfg)
> > +{
> > + if (i->dead_interval == 0)
> > + i->dead_interval = cfg->dead_interval;
> > + if (i->dead_interval == FAST_RTR_DEAD_TIME)
> > + i->hello_interval = 0;
> > + else if (i->hello_interval == 0)
> > + i->hello_interval = cfg->hello_interval;
> > + if (i->transmit_delay == 0)
> > + i->transmit_delay = cfg->transmit_delay;
> > + if (i->fast_hello_interval == 0)
> > + i->fast_hello_interval = cfg->fast_hello_interval;
> > + if (i->rxmt_interval == 0)
> > + i->rxmt_interval = cfg->rxmt_interval;
> > + if (i->metric == 0)
> > + i->metric = cfg->metric;
> > + if (i->priority == -1)
> > + i->priority = cfg->priority;
> > + if (i->auth_keyid == -1)
> > + i->auth_keyid = cfg->auth_keyid;
> > + if (i->auth_type == AUTH_UNSET)
> > + i->auth_type = cfg->auth_type;
> > + if (strlen(i->auth_key) == 0)
> > + memcpy(i->auth_key, cfg->auth_key, sizeof(i->auth_key));
> > + if (TAILQ_EMPTY(&i->auth_md_list))
> > + md_list_copy(&i->auth_md_list, &cfg->md_list);
> >  }
> > Index: ospfd/printconf.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
> > retrieving revision 1.21
> > diff -u -p -r1.21 printconf.c
> > --- ospfd/printconf.c 19 Nov 2019 09:55:55 -0000 1.21
> > +++ ospfd/printconf.c 8 Jan 2020 12:11:20 -0000
> > @@ -154,12 +154,12 @@ print_iface(struct iface *iface)
> >  
> >   printf("\t\tauth-type %s\n", if_auth_name(iface->auth_type));
> >   switch (iface->auth_type) {
> > - case AUTH_TYPE_NONE:
> > + case AUTH_NONE:
> >   break;
> > - case AUTH_TYPE_SIMPLE:
> > + case AUTH_SIMPLE:
> >   printf("\t\tauth-key XXXXXX\n");
> >   break;
> > - case AUTH_TYPE_CRYPT:
> > + case AUTH_CRYPT:
> >   printf("\t\tauth-md-keyid %d\n", iface->auth_keyid);
> >   TAILQ_FOREACH(md, &iface->auth_md_list, entry)
> >   printf("\t\tauth-md %d XXXXXX\n", md->keyid);
> >
>

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: ospf(6)d.conf: define interface parameters per area or globally

Remi Locherer
On Sun, Jan 12, 2020 at 04:18:26PM +0100, Claudio Jeker wrote:

> On Sun, Jan 12, 2020 at 03:46:15PM +0100, Remi Locherer wrote:
> > On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> > > On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > > > I have a diff to allow parameters after interface or area definition.
> > > > > Not sure if we want to do that though.
> > > >
> > > > I would appreciate that! ;-)
> > > >
> > >
> > > The ospfd diff needs some more work. Crypt authentication handling is not
> > > perfect.
> >
> > This works fine for me and the diff reads good. I tested ospfd and ospf6d.
> > Also the crypt options for ospfd worked fine.
> >
> > ok remi@
>
> Currently all daemons behave the same way and inherit at the moment of
> creation. Having this behave different between daemons is confusing.
> At least ospfd and bgpd should behave the same. Not saying that the
> current behaviour is great.
> I think in the case of ospfd the way auth-md is handled by this diff is
> not comparable with the behaviour of the other settings.

I agree. But that should not stop us improving one program before the
other ones.

>
> area 0.0.0.0 {
> hello-interval 10
> auth-md 1 foo
>
> interface em0
>
> hello-interval 20
> auth-md 1 bar
> auth-md 2 foofoo
>
> interface em1 {
> auth-md 3 barbar
> }
>
> hello-interval 30
> auth-md 1 bay
> auth-md 2 foobar
> }
>
> What values for hello-interval and auth-md should be set on em0 and em1?
>  

To me it looks natural if the latest value per level is used. With your
example that would be:

em0:
- auth-md 1 "bay"
- auth-md 2 "foobar"
- hello-interval 30

em1:
- auth-md 1 "bay"
- auth-md 2 "foobar"
- auth-md 3 "barbar"
- hello-interval 30

In my testing this is the result of the diff from Denis. (I modified
printconf.c to print the keys to see the results).

Another option would be to make it an error specifying the same option
more than once at the same level.

While looking closer I noticed, that the default value for auth-md-keyid
is set to 0 while the manual says it is 1. But that is not a change
introduced by this diff.

> > >
> > > Index: ospf6d/ospf6d.h
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v
> > > retrieving revision 1.44
> > > diff -u -p -r1.44 ospf6d.h
> > > --- ospf6d/ospf6d.h 3 Jan 2020 17:45:02 -0000 1.44
> > > +++ ospf6d/ospf6d.h 8 Jan 2020 12:11:20 -0000
> > > @@ -328,7 +328,7 @@ struct iface {
> > >   enum iface_type type;
> > >   u_int8_t if_type;
> > >   u_int8_t linkstate;
> > > - u_int8_t priority;
> > > + int16_t priority;
> > >   u_int8_t p2p;
> > >   u_int8_t cflags;
> > >  #define F_IFACE_PASSIVE 0x01
> > > Index: ospf6d/parse.y
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/ospf6d/parse.y,v
> > > retrieving revision 1.48
> > > diff -u -p -r1.48 parse.y
> > > --- ospf6d/parse.y 26 Dec 2019 10:24:18 -0000 1.48
> > > +++ ospf6d/parse.y 8 Jan 2020 12:11:20 -0000
> > > @@ -101,7 +101,7 @@ struct config_defaults {
> > >   u_int16_t hello_interval;
> > >   u_int16_t rxmt_interval;
> > >   u_int16_t metric;
> > > - u_int8_t priority;
> > > + int16_t priority;
> > >  };
> > >  
> > >  struct config_defaults globaldefs;
> > > @@ -111,6 +111,7 @@ struct config_defaults *defs;
> > >  
> > >  struct area *conf_get_area(struct in_addr);
> > >  int conf_check_rdomain(u_int);
> > > +void iface_settings(struct iface *, struct config_defaults *);
> > >  
> > >  typedef struct {
> > >   union {
> > > @@ -465,9 +466,14 @@ comma : ','
> > >  area : AREA areaid {
> > >   area = conf_get_area($2);
> > >  
> > > - memcpy(&areadefs, defs, sizeof(areadefs));
> > > + memset(&areadefs, 0, sizeof(areadefs));
> > > + areadefs.priority = -1;
> > >   defs = &areadefs;
> > >   } '{' optnl areaopts_l '}' {
> > > + struct iface *i;
> > > + LIST_FOREACH(i, &area->iface_list, entry) {
> > > + iface_settings(i, &areadefs);
> > > + }
> > >   area = NULL;
> > >   defs = &globaldefs;
> > >   }
> > > @@ -540,15 +546,12 @@ interface : INTERFACE STRING {
> > >   iface->area = area;
> > >   LIST_INSERT_HEAD(&area->iface_list, iface, entry);
> > >  
> > > - memcpy(&ifacedefs, defs, sizeof(ifacedefs));
> > > + memset(&ifacedefs, 0, sizeof(ifacedefs));
> > > + ifacedefs.priority = -1;
> > >   defs = &ifacedefs;
> > >   } interface_block {
> > > - iface->dead_interval = defs->dead_interval;
> > > - iface->transmit_delay = defs->transmit_delay;
> > > - iface->hello_interval = defs->hello_interval;
> > > - iface->rxmt_interval = defs->rxmt_interval;
> > > - iface->metric = defs->metric;
> > > - iface->priority = defs->priority;
> > > + iface->priority = -1;
> > > + iface_settings(iface, defs);
> > >   iface->cflags |= F_IFACE_CONFIGURED;
> > >   iface = NULL;
> > >   /* interface is always part of an area */
> > > @@ -1018,6 +1021,8 @@ popfile(void)
> > >  struct ospfd_conf *
> > >  parse_config(char *filename, int opts)
> > >  {
> > > + struct area *a;
> > > + struct iface *i;
> > >   struct sym *sym, *next;
> > >  
> > >   if ((conf = calloc(1, sizeof(struct ospfd_conf))) == NULL)
> > > @@ -1068,6 +1073,10 @@ parse_config(char *filename, int opts)
> > >   }
> > >   }
> > >  
> > > + LIST_FOREACH(a, &conf->area_list, entry)
> > > + LIST_FOREACH(i, &a->iface_list, entry)
> > > + iface_settings(i, defs);
> > > +
> > >   /* check that all interfaces belong to the configured rdomain */
> > >   errors += conf_check_rdomain(conf->rdomain);
> > >  
> > > @@ -1319,4 +1328,21 @@ prefix(const char *s, struct in6_addr *a
> > >   }
> > >   *plen = 128;
> > >   return (host(s, addr));
> > > +}
> > > +
> > > +void
> > > +iface_settings(struct iface *i, struct config_defaults *cfg)
> > > +{
> > > + if (i->dead_interval == 0)
> > > + i->dead_interval = cfg->dead_interval;
> > > + if (i->transmit_delay == 0)
> > > + i->transmit_delay = cfg->transmit_delay;
> > > + if (i->hello_interval == 0)
> > > + i->hello_interval = cfg->hello_interval;
> > > + if (i->rxmt_interval == 0)
> > > + i->rxmt_interval = cfg->rxmt_interval;
> > > + if (i->metric == 0)
> > > + i->metric = cfg->metric;
> > > + if (i->priority == -1)
> > > + i->priority = cfg->priority;
> > >  }
> > > Index: ospfd/logmsg.c
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/ospfd/logmsg.c,v
> > > retrieving revision 1.1
> > > diff -u -p -r1.1 logmsg.c
> > > --- ospfd/logmsg.c 2 Sep 2016 14:04:25 -0000 1.1
> > > +++ ospfd/logmsg.c 8 Jan 2020 12:11:20 -0000
> > > @@ -101,15 +101,13 @@ const char *
> > >  if_auth_name(enum auth_type type)
> > >  {
> > >   switch (type) {
> > > - case AUTH_NONE:
> > > - return ("none");
> > >   case AUTH_SIMPLE:
> > >   return ("simple");
> > >   case AUTH_CRYPT:
> > >   return ("crypt");
> > > + default:
> > > + return ("none");
> > >   }
> > > - /* NOTREACHED */
> > > - return ("unknown");
> > >  }
> > >  
> > >  const char *
> > > Index: ospfd/ospf.h
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/ospfd/ospf.h,v
> > > retrieving revision 1.23
> > > diff -u -p -r1.23 ospf.h
> > > --- ospfd/ospf.h 17 Jan 2013 09:14:15 -0000 1.23
> > > +++ ospfd/ospf.h 8 Jan 2020 12:11:20 -0000
> > > @@ -102,10 +102,6 @@
> > >  #define PACKET_TYPE_LS_ACK 5
> > >  
> > >  /* OSPF auth types */
> > > -#define AUTH_TYPE_NONE 0
> > > -#define AUTH_TYPE_SIMPLE 1
> > > -#define AUTH_TYPE_CRYPT 2
> > > -
> > >  #define MIN_AUTHTYPE 0
> > >  #define MAX_AUTHTYPE 2
> > >  
> > > Index: ospfd/ospfd.h
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
> > > retrieving revision 1.105
> > > diff -u -p -r1.105 ospfd.h
> > > --- ospfd/ospfd.h 19 Nov 2019 09:55:55 -0000 1.105
> > > +++ ospfd/ospfd.h 8 Jan 2020 12:11:20 -0000
> > > @@ -275,6 +275,7 @@ enum nbr_action {
> > >  
> > >  /* auth types */
> > >  enum auth_type {
> > > + AUTH_UNSET = -1,
> > >   AUTH_NONE,
> > >   AUTH_SIMPLE,
> > >   AUTH_CRYPT
> > > @@ -360,9 +361,9 @@ struct iface {
> > >   enum iface_type type;
> > >   enum auth_type auth_type;
> > >   u_int8_t if_type;
> > > - u_int8_t auth_keyid;
> > > + int16_t auth_keyid;
> > >   u_int8_t linkstate;
> > > - u_int8_t priority;
> > > + int16_t priority;
> > >   u_int8_t passive;
> > >   u_int8_t p2p;
> > >  };
> > > Index: ospfd/parse.y
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
> > > retrieving revision 1.99
> > > diff -u -p -r1.99 parse.y
> > > --- ospfd/parse.y 19 Nov 2019 09:55:55 -0000 1.99
> > > +++ ospfd/parse.y 8 Jan 2020 12:11:20 -0000
> > > @@ -101,8 +101,8 @@ struct config_defaults {
> > >   u_int16_t rxmt_interval;
> > >   u_int16_t metric;
> > >   enum auth_type auth_type;
> > > - u_int8_t auth_keyid;
> > > - u_int8_t priority;
> > > + int16_t auth_keyid;
> > > + int16_t priority;
> > >  };
> > >  
> > >  struct config_defaults globaldefs;
> > > @@ -113,6 +113,7 @@ struct config_defaults *defs;
> > >  struct area *conf_get_area(struct in_addr);
> > >  struct iface *conf_get_if(struct kif *, struct kif_addr *);
> > >  int conf_check_rdomain(unsigned int);
> > > +void iface_settings(struct iface *, struct config_defaults *);
> > >  
> > >  typedef struct {
> > >   union {
> > > @@ -592,10 +593,17 @@ comma : ','
> > >  area : AREA areaid {
> > >   area = conf_get_area($2);
> > >  
> > > - memcpy(&areadefs, defs, sizeof(areadefs));
> > > - md_list_copy(&areadefs.md_list, &defs->md_list);
> > > + memset(&areadefs, 0, sizeof(areadefs));
> > > + areadefs.priority = -1;
> > > + areadefs.auth_keyid = -1;
> > > + areadefs.auth_type = AUTH_UNSET;
> > > + TAILQ_INIT(&areadefs.md_list);
> > >   defs = &areadefs;
> > >   } '{' optnl areaopts_l '}' {
> > > + struct iface *i;
> > > + LIST_FOREACH(i, &area->iface_list, entry) {
> > > + iface_settings(i, &areadefs);
> > > + }
> > >   area = NULL;
> > >   md_list_clr(&defs->md_list);
> > >   defs = &globaldefs;
> > > @@ -707,25 +715,17 @@ interface : INTERFACE STRING {
> > >   iface->area = area;
> > >   LIST_INSERT_HEAD(&area->iface_list, iface, entry);
> > >  
> > > - memcpy(&ifacedefs, defs, sizeof(ifacedefs));
> > > - md_list_copy(&ifacedefs.md_list, &defs->md_list);
> > > + memset(&ifacedefs, 0, sizeof(ifacedefs));
> > > + ifacedefs.priority = -1;
> > > + ifacedefs.auth_keyid = -1;
> > > + ifacedefs.auth_type = AUTH_UNSET;
> > > + TAILQ_INIT(&ifacedefs.md_list);
> > >   defs = &ifacedefs;
> > >   } interface_block {
> > > - iface->dead_interval = defs->dead_interval;
> > > - iface->fast_hello_interval = defs->fast_hello_interval;
> > > - iface->transmit_delay = defs->transmit_delay;
> > > - if (iface->dead_interval == FAST_RTR_DEAD_TIME)
> > > - iface->hello_interval = 0;
> > > - else
> > > - iface->hello_interval = defs->hello_interval;
> > > - iface->rxmt_interval = defs->rxmt_interval;
> > > - iface->metric = defs->metric;
> > > - iface->priority = defs->priority;
> > > - iface->auth_type = defs->auth_type;
> > > - iface->auth_keyid = defs->auth_keyid;
> > > - memcpy(iface->auth_key, defs->auth_key,
> > > -    sizeof(iface->auth_key));
> > > - md_list_copy(&iface->auth_md_list, &defs->md_list);
> > > + iface->priority = -1;
> > > + iface->auth_keyid = -1;
> > > + iface->auth_type = AUTH_UNSET;
> > > + iface_settings(iface, defs);
> > >   md_list_clr(&defs->md_list);
> > >   iface = NULL;
> > >   /* interface is always part of an area */
> > > @@ -1207,6 +1207,8 @@ popfile(void)
> > >  struct ospfd_conf *
> > >  parse_config(char *filename, int opts)
> > >  {
> > > + struct area *a;
> > > + struct iface *i;
> > >   struct sym *sym, *next;
> > >  
> > >   if ((conf = calloc(1, sizeof(struct ospfd_conf))) == NULL)
> > > @@ -1259,6 +1261,10 @@ parse_config(char *filename, int opts)
> > >   }
> > >   }
> > >  
> > > + LIST_FOREACH(a, &conf->area_list, entry)
> > > + LIST_FOREACH(i, &a->iface_list, entry)
> > > + iface_settings(i, defs);
> > > +
> > >   /* free global config defaults */
> > >   md_list_clr(&globaldefs.md_list);
> > >  
> > > @@ -1317,7 +1323,7 @@ int
> > >  cmdline_symset(char *s)
> > >  {
> > >   char *sym, *val;
> > > - int ret;
> > > + int ret;
> > >  
> > >   if ((val = strrchr(s, '=')) == NULL)
> > >   return (-1);
> > > @@ -1501,4 +1507,33 @@ host(const char *s, struct in_addr *addr
> > >   mask->s_addr = prefixlen2mask(bits);
> > >  
> > >   return (1);
> > > +}
> > > +
> > > +void
> > > +iface_settings(struct iface *i, struct config_defaults *cfg)
> > > +{
> > > + if (i->dead_interval == 0)
> > > + i->dead_interval = cfg->dead_interval;
> > > + if (i->dead_interval == FAST_RTR_DEAD_TIME)
> > > + i->hello_interval = 0;
> > > + else if (i->hello_interval == 0)
> > > + i->hello_interval = cfg->hello_interval;
> > > + if (i->transmit_delay == 0)
> > > + i->transmit_delay = cfg->transmit_delay;
> > > + if (i->fast_hello_interval == 0)
> > > + i->fast_hello_interval = cfg->fast_hello_interval;
> > > + if (i->rxmt_interval == 0)
> > > + i->rxmt_interval = cfg->rxmt_interval;
> > > + if (i->metric == 0)
> > > + i->metric = cfg->metric;
> > > + if (i->priority == -1)
> > > + i->priority = cfg->priority;
> > > + if (i->auth_keyid == -1)
> > > + i->auth_keyid = cfg->auth_keyid;
> > > + if (i->auth_type == AUTH_UNSET)
> > > + i->auth_type = cfg->auth_type;
> > > + if (strlen(i->auth_key) == 0)
> > > + memcpy(i->auth_key, cfg->auth_key, sizeof(i->auth_key));
> > > + if (TAILQ_EMPTY(&i->auth_md_list))
> > > + md_list_copy(&i->auth_md_list, &cfg->md_list);
> > >  }
> > > Index: ospfd/printconf.c
> > > ===================================================================
> > > RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
> > > retrieving revision 1.21
> > > diff -u -p -r1.21 printconf.c
> > > --- ospfd/printconf.c 19 Nov 2019 09:55:55 -0000 1.21
> > > +++ ospfd/printconf.c 8 Jan 2020 12:11:20 -0000
> > > @@ -154,12 +154,12 @@ print_iface(struct iface *iface)
> > >  
> > >   printf("\t\tauth-type %s\n", if_auth_name(iface->auth_type));
> > >   switch (iface->auth_type) {
> > > - case AUTH_TYPE_NONE:
> > > + case AUTH_NONE:
> > >   break;
> > > - case AUTH_TYPE_SIMPLE:
> > > + case AUTH_SIMPLE:
> > >   printf("\t\tauth-key XXXXXX\n");
> > >   break;
> > > - case AUTH_TYPE_CRYPT:
> > > + case AUTH_CRYPT:
> > >   printf("\t\tauth-md-keyid %d\n", iface->auth_keyid);
> > >   TAILQ_FOREACH(md, &iface->auth_md_list, entry)
> > >   printf("\t\tauth-md %d XXXXXX\n", md->keyid);
> > >
> >
>
> --
> :wq Claudio
>

Reply | Threaded
Open this post in threaded view
|

Re: ospf(6)d.conf: define interface parameters per area or globally

Theo de Raadt-2
Remi Locherer <[hidden email]> wrote:

> On Sun, Jan 12, 2020 at 04:18:26PM +0100, Claudio Jeker wrote:
> > On Sun, Jan 12, 2020 at 03:46:15PM +0100, Remi Locherer wrote:
> > > On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> > > > On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > > > > I have a diff to allow parameters after interface or area definition.
> > > > > > Not sure if we want to do that though.
> > > > >
> > > > > I would appreciate that! ;-)
> > > > >
> > > >
> > > > The ospfd diff needs some more work. Crypt authentication handling is not
> > > > perfect.
> > >
> > > This works fine for me and the diff reads good. I tested ospfd and ospf6d.
> > > Also the crypt options for ospfd worked fine.
> > >
> > > ok remi@
> >
> > Currently all daemons behave the same way and inherit at the moment of
> > creation. Having this behave different between daemons is confusing.
> > At least ospfd and bgpd should behave the same. Not saying that the
> > current behaviour is great.
> > I think in the case of ospfd the way auth-md is handled by this diff is
> > not comparable with the behaviour of the other settings.
>
> I agree. But that should not stop us improving one program before the
> other ones.
>
> >
> > area 0.0.0.0 {
> > hello-interval 10
> > auth-md 1 foo
> >
> > interface em0
> >
> > hello-interval 20
> > auth-md 1 bar
> > auth-md 2 foofoo
> >
> > interface em1 {
> > auth-md 3 barbar
> > }
> >
> > hello-interval 30
> > auth-md 1 bay
> > auth-md 2 foobar
> > }
> >
> > What values for hello-interval and auth-md should be set on em0 and em1?
> >  
>
> To me it looks natural if the latest value per level is used. With your
> example that would be:
>
> em0:
> - auth-md 1 "bay"
> - auth-md 2 "foobar"
> - hello-interval 30
>
> em1:
> - auth-md 1 "bay"
> - auth-md 2 "foobar"
> - auth-md 3 "barbar"
> - hello-interval 30
>
> In my testing this is the result of the diff from Denis. (I modified
> printconf.c to print the keys to see the results).

I think that is very dangerous.  In some other daemons it could be
disastrous.

> Another option would be to make it an error specifying the same option
> more than once at the same level.

I think that will be the easier solution.

The approach of "collect all the root info first, then apply to the
children aftwards" will be difficult to apply to all our
domain-specific-grammer-daemons.

Reply | Threaded
Open this post in threaded view
|

Re: ospf(6)d.conf: define interface parameters per area or globally

Sebastian Benoit-3
Theo de Raadt([hidden email]) on 2020.01.12 12:03:40 -0700:

> Remi Locherer <[hidden email]> wrote:
>
> > On Sun, Jan 12, 2020 at 04:18:26PM +0100, Claudio Jeker wrote:
> > > On Sun, Jan 12, 2020 at 03:46:15PM +0100, Remi Locherer wrote:
> > > > On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> > > > > On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > > > > > I have a diff to allow parameters after interface or area definition.
> > > > > > > Not sure if we want to do that though.
> > > > > >
> > > > > > I would appreciate that! ;-)
> > > > > >
> > > > >
> > > > > The ospfd diff needs some more work. Crypt authentication handling is not
> > > > > perfect.
> > > >
> > > > This works fine for me and the diff reads good. I tested ospfd and ospf6d.
> > > > Also the crypt options for ospfd worked fine.
> > > >
> > > > ok remi@
> > >
> > > Currently all daemons behave the same way and inherit at the moment of
> > > creation. Having this behave different between daemons is confusing.
> > > At least ospfd and bgpd should behave the same. Not saying that the
> > > current behaviour is great.
> > > I think in the case of ospfd the way auth-md is handled by this diff is
> > > not comparable with the behaviour of the other settings.
> >
> > I agree. But that should not stop us improving one program before the
> > other ones.
> >
> > >
> > > area 0.0.0.0 {
> > > hello-interval 10
> > > auth-md 1 foo
> > >
> > > interface em0
> > >
> > > hello-interval 20
> > > auth-md 1 bar
> > > auth-md 2 foofoo
> > >
> > > interface em1 {
> > > auth-md 3 barbar
> > > }
> > >
> > > hello-interval 30
> > > auth-md 1 bay
> > > auth-md 2 foobar
> > > }
> > >
> > > What values for hello-interval and auth-md should be set on em0 and em1?
> > >  
> >
> > To me it looks natural if the latest value per level is used. With your
> > example that would be:
> >
> > em0:
> > - auth-md 1 "bay"
> > - auth-md 2 "foobar"
> > - hello-interval 30
> >
> > em1:
> > - auth-md 1 "bay"
> > - auth-md 2 "foobar"
> > - auth-md 3 "barbar"
> > - hello-interval 30
> >
> > In my testing this is the result of the diff from Denis. (I modified
> > printconf.c to print the keys to see the results).
>
> I think that is very dangerous.  In some other daemons it could be
> disastrous.
>
>
> > Another option would be to make it an error specifying the same option
> > more than once at the same level.
>
> I think that will be the easier solution.

I too think that should be an error.
Why should you specify it twice?
 
> The approach of "collect all the root info first, then apply to the
> children aftwards" will be difficult to apply to all our
> domain-specific-grammer-daemons.

We should keep it the same in the routing daemons. I think the rest is
different here and there already.

/Benno