bgpd: deny redefinition of default RIBs

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

bgpd: deny redefinition of default RIBs

Denis Fondras-3
Redefining a default RIB is not desirable.

Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.352
diff -u -p -r1.352 bgpd.h
--- bgpd.h 4 Nov 2018 14:34:00 -0000 1.352
+++ bgpd.h 4 Nov 2018 17:49:38 -0000
@@ -1057,6 +1057,7 @@ extern struct rib_names ribnames;
 #define F_RIB_NOEVALUATE 0x0002
 #define F_RIB_NOFIB 0x0004
 #define F_RIB_NOFIBSYNC 0x0008
+#define F_RIB_DEFAULT 0x0010
 #define F_RIB_HASNOFIB (F_RIB_NOFIB | F_RIB_NOEVALUATE)
 
 /* 4-byte magic AS number */
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.362
diff -u -p -r1.362 parse.y
--- parse.y 1 Nov 2018 00:18:44 -0000 1.362
+++ parse.y 4 Nov 2018 17:49:38 -0000
@@ -3319,10 +3319,10 @@ parse_config(char *filename, struct bgpd
  netconf = &conf->networks;
 
  add_rib("Adj-RIB-In", conf->default_tableid,
-    F_RIB_NOFIB | F_RIB_NOEVALUATE);
+    F_RIB_NOFIB | F_RIB_NOEVALUATE | F_RIB_DEFAULT);
  add_rib("Adj-RIB-Out", conf->default_tableid,
-    F_RIB_NOFIB | F_RIB_NOEVALUATE);
- add_rib("Loc-RIB", conf->default_tableid, F_RIB_LOCAL);
+    F_RIB_NOFIB | F_RIB_NOEVALUATE | F_RIB_DEFAULT);
+ add_rib("Loc-RIB", conf->default_tableid, F_RIB_LOCAL | F_RIB_DEFAULT);
 
  if ((file = pushfile(filename, 1)) == NULL) {
  free(conf);
@@ -3876,6 +3876,11 @@ add_rib(char *name, u_int rtableid, u_in
  return (-1);
  }
  }
+ if (rr->flags & F_RIB_DEFAULT) {
+ yyerror("redefinition of %s not permitted", rr->name);
+ return (-1);
+ }
+
  if (strlcpy(rr->name, name, sizeof(rr->name)) >= sizeof(rr->name)) {
  yyerror("rib name \"%s\" too long: max %zu",
    name, sizeof(rr->name) - 1);
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.445
diff -u -p -r1.445 rde.c
--- rde.c 4 Nov 2018 12:34:54 -0000 1.445
+++ rde.c 4 Nov 2018 17:49:38 -0000
@@ -217,8 +217,10 @@ rde_main(int debug, int verbose)
  peer_init(peerhashsize);
 
  /* make sure the default RIBs are setup */
- rib_new("Adj-RIB-In", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE);
- rib_new("Adj-RIB-Out", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE);
+ rib_new("Adj-RIB-In", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE |
+    F_RIB_DEFAULT);
+ rib_new("Adj-RIB-Out", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE |
+    F_RIB_DEFAULT);
 
  out_rules = calloc(1, sizeof(struct filter_head));
  if (out_rules == NULL)

Reply | Threaded
Open this post in threaded view
|

Re: bgpd: deny redefinition of default RIBs

Sebastian Benoit-3
Denis Fondras([hidden email]) on 2018.11.04 18:51:39 +0100:
> Redefining a default RIB is not desirable.

ok benno@

the rde.c bit isnt needed i guess, but it doesnt hurt either.

> Index: bgpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.352
> diff -u -p -r1.352 bgpd.h
> --- bgpd.h 4 Nov 2018 14:34:00 -0000 1.352
> +++ bgpd.h 4 Nov 2018 17:49:38 -0000
> @@ -1057,6 +1057,7 @@ extern struct rib_names ribnames;
>  #define F_RIB_NOEVALUATE 0x0002
>  #define F_RIB_NOFIB 0x0004
>  #define F_RIB_NOFIBSYNC 0x0008
> +#define F_RIB_DEFAULT 0x0010
>  #define F_RIB_HASNOFIB (F_RIB_NOFIB | F_RIB_NOEVALUATE)
>  
>  /* 4-byte magic AS number */
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.362
> diff -u -p -r1.362 parse.y
> --- parse.y 1 Nov 2018 00:18:44 -0000 1.362
> +++ parse.y 4 Nov 2018 17:49:38 -0000
> @@ -3319,10 +3319,10 @@ parse_config(char *filename, struct bgpd
>   netconf = &conf->networks;
>  
>   add_rib("Adj-RIB-In", conf->default_tableid,
> -    F_RIB_NOFIB | F_RIB_NOEVALUATE);
> +    F_RIB_NOFIB | F_RIB_NOEVALUATE | F_RIB_DEFAULT);
>   add_rib("Adj-RIB-Out", conf->default_tableid,
> -    F_RIB_NOFIB | F_RIB_NOEVALUATE);
> - add_rib("Loc-RIB", conf->default_tableid, F_RIB_LOCAL);
> +    F_RIB_NOFIB | F_RIB_NOEVALUATE | F_RIB_DEFAULT);
> + add_rib("Loc-RIB", conf->default_tableid, F_RIB_LOCAL | F_RIB_DEFAULT);
>  
>   if ((file = pushfile(filename, 1)) == NULL) {
>   free(conf);
> @@ -3876,6 +3876,11 @@ add_rib(char *name, u_int rtableid, u_in
>   return (-1);
>   }
>   }
> + if (rr->flags & F_RIB_DEFAULT) {
> + yyerror("redefinition of %s not permitted", rr->name);
> + return (-1);
> + }
> +
>   if (strlcpy(rr->name, name, sizeof(rr->name)) >= sizeof(rr->name)) {
>   yyerror("rib name \"%s\" too long: max %zu",
>     name, sizeof(rr->name) - 1);
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.445
> diff -u -p -r1.445 rde.c
> --- rde.c 4 Nov 2018 12:34:54 -0000 1.445
> +++ rde.c 4 Nov 2018 17:49:38 -0000
> @@ -217,8 +217,10 @@ rde_main(int debug, int verbose)
>   peer_init(peerhashsize);
>  
>   /* make sure the default RIBs are setup */
> - rib_new("Adj-RIB-In", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE);
> - rib_new("Adj-RIB-Out", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE);
> + rib_new("Adj-RIB-In", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE |
> +    F_RIB_DEFAULT);
> + rib_new("Adj-RIB-Out", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE |
> +    F_RIB_DEFAULT);
>  
>   out_rules = calloc(1, sizeof(struct filter_head));
>   if (out_rules == NULL)
>

Reply | Threaded
Open this post in threaded view
|

Re: bgpd: deny redefinition of default RIBs

Denis Fondras-3
On Sun, Nov 04, 2018 at 08:05:01PM +0100, Sebastian Benoit wrote:
> Denis Fondras([hidden email]) on 2018.11.04 18:51:39 +0100:
> > Redefining a default RIB is not desirable.
>
> ok benno@
>
> the rde.c bit isnt needed i guess, but it doesnt hurt either.
>

Yes, it was added for the sake of consistency.

> > Index: bgpd.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> > retrieving revision 1.352
> > diff -u -p -r1.352 bgpd.h
> > --- bgpd.h 4 Nov 2018 14:34:00 -0000 1.352
> > +++ bgpd.h 4 Nov 2018 17:49:38 -0000
> > @@ -1057,6 +1057,7 @@ extern struct rib_names ribnames;
> >  #define F_RIB_NOEVALUATE 0x0002
> >  #define F_RIB_NOFIB 0x0004
> >  #define F_RIB_NOFIBSYNC 0x0008
> > +#define F_RIB_DEFAULT 0x0010
> >  #define F_RIB_HASNOFIB (F_RIB_NOFIB | F_RIB_NOEVALUATE)
> >  
> >  /* 4-byte magic AS number */
> > Index: parse.y
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> > retrieving revision 1.362
> > diff -u -p -r1.362 parse.y
> > --- parse.y 1 Nov 2018 00:18:44 -0000 1.362
> > +++ parse.y 4 Nov 2018 17:49:38 -0000
> > @@ -3319,10 +3319,10 @@ parse_config(char *filename, struct bgpd
> >   netconf = &conf->networks;
> >  
> >   add_rib("Adj-RIB-In", conf->default_tableid,
> > -    F_RIB_NOFIB | F_RIB_NOEVALUATE);
> > +    F_RIB_NOFIB | F_RIB_NOEVALUATE | F_RIB_DEFAULT);
> >   add_rib("Adj-RIB-Out", conf->default_tableid,
> > -    F_RIB_NOFIB | F_RIB_NOEVALUATE);
> > - add_rib("Loc-RIB", conf->default_tableid, F_RIB_LOCAL);
> > +    F_RIB_NOFIB | F_RIB_NOEVALUATE | F_RIB_DEFAULT);
> > + add_rib("Loc-RIB", conf->default_tableid, F_RIB_LOCAL | F_RIB_DEFAULT);
> >  
> >   if ((file = pushfile(filename, 1)) == NULL) {
> >   free(conf);
> > @@ -3876,6 +3876,11 @@ add_rib(char *name, u_int rtableid, u_in
> >   return (-1);
> >   }
> >   }
> > + if (rr->flags & F_RIB_DEFAULT) {
> > + yyerror("redefinition of %s not permitted", rr->name);
> > + return (-1);
> > + }
> > +
> >   if (strlcpy(rr->name, name, sizeof(rr->name)) >= sizeof(rr->name)) {
> >   yyerror("rib name \"%s\" too long: max %zu",
> >     name, sizeof(rr->name) - 1);
> > Index: rde.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> > retrieving revision 1.445
> > diff -u -p -r1.445 rde.c
> > --- rde.c 4 Nov 2018 12:34:54 -0000 1.445
> > +++ rde.c 4 Nov 2018 17:49:38 -0000
> > @@ -217,8 +217,10 @@ rde_main(int debug, int verbose)
> >   peer_init(peerhashsize);
> >  
> >   /* make sure the default RIBs are setup */
> > - rib_new("Adj-RIB-In", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE);
> > - rib_new("Adj-RIB-Out", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE);
> > + rib_new("Adj-RIB-In", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE |
> > +    F_RIB_DEFAULT);
> > + rib_new("Adj-RIB-Out", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE |
> > +    F_RIB_DEFAULT);
> >  
> >   out_rules = calloc(1, sizeof(struct filter_head));
> >   if (out_rules == NULL)
> >
>

Reply | Threaded
Open this post in threaded view
|

Re: bgpd: deny redefinition of default RIBs

Claudio Jeker
In reply to this post by Denis Fondras-3
On Sun, Nov 04, 2018 at 06:51:39PM +0100, Denis Fondras wrote:

> Redefining a default RIB is not desirable.
>
> Index: bgpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.352
> diff -u -p -r1.352 bgpd.h
> --- bgpd.h 4 Nov 2018 14:34:00 -0000 1.352
> +++ bgpd.h 4 Nov 2018 17:49:38 -0000
> @@ -1057,6 +1057,7 @@ extern struct rib_names ribnames;
>  #define F_RIB_NOEVALUATE 0x0002
>  #define F_RIB_NOFIB 0x0004
>  #define F_RIB_NOFIBSYNC 0x0008
> +#define F_RIB_DEFAULT 0x0010
>  #define F_RIB_HASNOFIB (F_RIB_NOFIB | F_RIB_NOEVALUATE)
>  
>  /* 4-byte magic AS number */
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.362
> diff -u -p -r1.362 parse.y
> --- parse.y 1 Nov 2018 00:18:44 -0000 1.362
> +++ parse.y 4 Nov 2018 17:49:38 -0000
> @@ -3319,10 +3319,10 @@ parse_config(char *filename, struct bgpd
>   netconf = &conf->networks;
>  
>   add_rib("Adj-RIB-In", conf->default_tableid,
> -    F_RIB_NOFIB | F_RIB_NOEVALUATE);
> +    F_RIB_NOFIB | F_RIB_NOEVALUATE | F_RIB_DEFAULT);
>   add_rib("Adj-RIB-Out", conf->default_tableid,
> -    F_RIB_NOFIB | F_RIB_NOEVALUATE);
> - add_rib("Loc-RIB", conf->default_tableid, F_RIB_LOCAL);
> +    F_RIB_NOFIB | F_RIB_NOEVALUATE | F_RIB_DEFAULT);
> + add_rib("Loc-RIB", conf->default_tableid, F_RIB_LOCAL | F_RIB_DEFAULT);
>  
>   if ((file = pushfile(filename, 1)) == NULL) {
>   free(conf);
> @@ -3876,6 +3876,11 @@ add_rib(char *name, u_int rtableid, u_in
>   return (-1);
>   }
>   }
> + if (rr->flags & F_RIB_DEFAULT) {
> + yyerror("redefinition of %s not permitted", rr->name);
> + return (-1);
> + }
> +
>   if (strlcpy(rr->name, name, sizeof(rr->name)) >= sizeof(rr->name)) {
>   yyerror("rib name \"%s\" too long: max %zu",
>     name, sizeof(rr->name) - 1);
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.445
> diff -u -p -r1.445 rde.c
> --- rde.c 4 Nov 2018 12:34:54 -0000 1.445
> +++ rde.c 4 Nov 2018 17:49:38 -0000
> @@ -217,8 +217,10 @@ rde_main(int debug, int verbose)
>   peer_init(peerhashsize);
>  
>   /* make sure the default RIBs are setup */
> - rib_new("Adj-RIB-In", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE);
> - rib_new("Adj-RIB-Out", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE);
> + rib_new("Adj-RIB-In", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE |
> +    F_RIB_DEFAULT);
> + rib_new("Adj-RIB-Out", 0, F_RIB_NOFIB | F_RIB_NOEVALUATE |
> +    F_RIB_DEFAULT);
>  
>   out_rules = calloc(1, sizeof(struct filter_head));
>   if (out_rules == NULL)
>

Unsure if we need an extra flag, shouldn't it be enough to not allow
re-adding of a RIB in the conf? e.g. having 'rde rib foo' twice in the
config with different options could also be considered a
miss-configuration. Also it may be necessary to skip the default ribs in
printconf.c else bgpd -nv output will not work as input (like in the
regress tests).

--
:wq Claudio