Re: bgpd: deny redefinition of default RIBs

2018-11-06 Thread Claudio Jeker
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.h4 Nov 2018 14:34:00 -   1.352
> +++ bgpd.h4 Nov 2018 17:49:38 -
> @@ -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_DEFAULT0x0010
>  #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 -   1.362
> +++ parse.y   4 Nov 2018 17:49:38 -
> @@ -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 -   1.445
> +++ rde.c 4 Nov 2018 17:49:38 -
> @@ -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



Re: bgpd: deny redefinition of default RIBs

2018-11-04 Thread Denis Fondras
On Sun, Nov 04, 2018 at 08:05:01PM +0100, Sebastian Benoit wrote:
> Denis Fondras(de...@openbsd.org) 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 -   1.352
> > +++ bgpd.h  4 Nov 2018 17:49:38 -
> > @@ -1057,6 +1057,7 @@ extern struct rib_names ribnames;
> >  #define F_RIB_NOEVALUATE   0x0002
> >  #define F_RIB_NOFIB0x0004
> >  #define F_RIB_NOFIBSYNC0x0008
> > +#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 -   1.362
> > +++ parse.y 4 Nov 2018 17:49:38 -
> > @@ -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 -   1.445
> > +++ rde.c   4 Nov 2018 17:49:38 -
> > @@ -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)
> > 
> 



Re: bgpd: deny redefinition of default RIBs

2018-11-04 Thread Sebastian Benoit
Denis Fondras(de...@openbsd.org) 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.h4 Nov 2018 14:34:00 -   1.352
> +++ bgpd.h4 Nov 2018 17:49:38 -
> @@ -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_DEFAULT0x0010
>  #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 -   1.362
> +++ parse.y   4 Nov 2018 17:49:38 -
> @@ -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 -   1.445
> +++ rde.c 4 Nov 2018 17:49:38 -
> @@ -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)
> 



bgpd: deny redefinition of default RIBs

2018-11-04 Thread Denis Fondras
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 -   1.352
+++ bgpd.h  4 Nov 2018 17:49:38 -
@@ -1057,6 +1057,7 @@ extern struct rib_names ribnames;
 #define F_RIB_NOEVALUATE   0x0002
 #define F_RIB_NOFIB0x0004
 #define F_RIB_NOFIBSYNC0x0008
+#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 -   1.362
+++ parse.y 4 Nov 2018 17:49:38 -
@@ -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 -   1.445
+++ rde.c   4 Nov 2018 17:49:38 -
@@ -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)