David Gwynne(da...@gwynne.id.au) on 2019.04.29 19:36:51 +1000:
> 
> 
> > On 29 Apr 2019, at 4:59 pm, Remi Locherer <remi.loche...@relo.ch> wrote:
> > 
> > Hi David
> > 
> > On Mon, Apr 29, 2019 at 11:53:27AM +1000, David Gwynne wrote:
> >> it's always bothered me that i config areas on a crisco using a number,
> >> but then have to think hard to convert that number to an address for use
> >> in openbsd. eg, i was given area 700 in one place, which is 0.0.2.188
> >> as an address. super annoying.
> >> 
> >> so this changes the ospfd parser so it accepts both a number or address.
> >> i also changed it so it prints the number by default, which may be
> >> contentious. the manpage is slightly tweaked too.
> >> 
> >> thoughts?
> > 
> > I like it to be able to use a number instead of an address!
> > 
> > It worked fine in my short test I performed.
> > 
> > The output with the comment looks a bit strange to me.
> 
> Are you sure it doesn't look... awesome?

I like it!

> 
> > typhoon ..sbin/ospfd$ doas obj/ospfd -nv 
> > 
> > router-id 0.0.0.7
> > fib-update yes
> > fib-priority 32
> > rfc1583compat no
> > spf-delay msec 1000
> > spf-holdtime msec 5000
> > 
> > area 7 { # 0.0.0.7
> >         ^^^^^^^^^
> >        interface pair7:10.77.77.1 {
> >                metric 10
> >                retransmit-interval 5
> >                router-dead-time 40
> > 
> > 
> > I'd prefer if we settle for one output format and then use only that. The
> > number format is more common but that would be a change for the users. I'm
> > fine with either format for outputs.
> 
> I lean toward the number too. I don't think it would hurt to change it so
> only one is output, so long input works either way.

We need a way to show both: to make migration easier, and to avoid the same
problem you encountered when entering the area in some other equipment.

I dont care if thats in the config printer or in ospfctl output though.

> 
> > There is also "ospfctl show database area 0.0.0.0" and ospf6d. ;-)
> 
> Are you offering to help with the implementation of those?
> 
> dlg
> 
> > 
> > Regards,
> > Remi
> > 
> > 
> >> 
> >> with this diff, i can do the following and things keep
> >> working:
> >> 
> >> --- /etc/ospfd.conf        Mon Apr 29 11:29:56 2019
> >> +++ /etc/ospfd.conf.new    Mon Apr 29 11:39:45 2019
> >> @@ -7,5 +7,5 @@
> >> redistribute rtlabel "backup" set metric 65535
> >> 
> >> -area 0.0.2.188 {
> >> +area 700 {
> >>    router-dead-time minimal
> >>    fast-hello-interval msec 300
> >> 
> >> Index: ospfd.conf.5
> >> ===================================================================
> >> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
> >> retrieving revision 1.55
> >> diff -u -p -r1.55 ospfd.conf.5
> >> --- ospfd.conf.5   28 Dec 2018 19:25:10 -0000      1.55
> >> +++ ospfd.conf.5   29 Apr 2019 01:45:40 -0000
> >> @@ -68,7 +68,7 @@ Macros are not expanded inside quotes.
> >> For example:
> >> .Bd -literal -offset indent
> >> hi="5"
> >> -area 0.0.0.0 {
> >> +area 0 {
> >>    interface em0 {
> >>            hello-interval $hi
> >>    }
> >> @@ -257,10 +257,10 @@ Areas are used for grouping interfaces.
> >> All interface-specific parameters can
> >> be configured per area, overruling the global settings.
> >> .Bl -tag -width Ds
> >> -.It Ic area Ar address
> >> +.It Ic area Ar id Ns | Ns Ar address
> >> Specify an area section, grouping one or more interfaces.
> >> .Bd -literal -offset indent
> >> -area 0.0.0.0 {
> >> +area 0 {
> >>    interface em0
> >>    interface em1 {
> >>            metric 10
> >> Index: parse.y
> >> ===================================================================
> >> RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
> >> retrieving revision 1.95
> >> diff -u -p -r1.95 parse.y
> >> --- parse.y        13 Feb 2019 22:57:08 -0000      1.95
> >> +++ parse.y        29 Apr 2019 01:45:40 -0000
> >> @@ -120,6 +120,7 @@ typedef struct {
> >>            int64_t          number;
> >>            char            *string;
> >>            struct redistribute *redist;
> >> +          struct in_addr   id;
> >>    } v;
> >>    int lineno;
> >> } YYSTYPE;
> >> @@ -145,6 +146,7 @@ typedef struct {
> >> %type      <v.number>      deadtime
> >> %type      <v.string>      string dependon
> >> %type      <v.redist>      redistribute
> >> +%type     <v.id>          areaid
> >> 
> >> %%
> >> 
> >> @@ -588,15 +590,8 @@ comma         : ','
> >>            | /*empty*/
> >>            ;
> >> 
> >> -area              : AREA STRING {
> >> -                  struct in_addr  id;
> >> -                  if (inet_aton($2, &id) == 0) {
> >> -                          yyerror("error parsing area");
> >> -                          free($2);
> >> -                          YYERROR;
> >> -                  }
> >> -                  free($2);
> >> -                  area = conf_get_area(id);
> >> +area              : AREA areaid {
> >> +                  area = conf_get_area($2);
> >> 
> >>                    memcpy(&areadefs, defs, sizeof(areadefs));
> >>                    md_list_copy(&areadefs.md_list, &defs->md_list);
> >> @@ -610,6 +605,23 @@ area          : AREA STRING {
> >> 
> >> demotecount        : NUMBER        { $$ = $1; }
> >>            | /*empty*/     { $$ = 1; }
> >> +          ;
> >> +
> >> +areaid            : NUMBER {
> >> +                  if ($1 < 0 || $1 > 0xffffffff) {
> >> +                          yyerror("invalid area id");
> >> +                          YYERROR;
> >> +                  }
> >> +                  $$.s_addr = htonl($1);
> >> +          }
> >> +          | STRING {
> >> +                  if (inet_aton($1, &$$) == 0) {
> >> +                          yyerror("error parsing area");
> >> +                          free($1);
> >> +                          YYERROR;
> >> +                  }
> >> +                  free($1);
> >> +          }
> >>            ;
> >> 
> >> areaopts_l : areaopts_l areaoptsl nl
> >> Index: printconf.c
> >> ===================================================================
> >> RCS file: /cvs/src/usr.sbin/ospfd/printconf.c,v
> >> retrieving revision 1.20
> >> diff -u -p -r1.20 printconf.c
> >> --- printconf.c    28 Dec 2018 19:25:10 -0000      1.20
> >> +++ printconf.c    29 Apr 2019 01:45:40 -0000
> >> @@ -181,7 +181,8 @@ print_config(struct ospfd_conf *conf)
> >>    printf("\n");
> >> 
> >>    LIST_FOREACH(area, &conf->area_list, entry) {
> >> -          printf("area %s {\n", inet_ntoa(area->id));
> >> +          printf("area %u { # %s\n", ntohl(area->id.s_addr),
> >> +              inet_ntoa(area->id));
> >>            if (area->stub) {
> >>                    printf("\tstub");
> >>                    if (SIMPLEQ_EMPTY(&area->redist_list))
> >> 
> 

Reply via email to