This is a great idea, thanks for the implementation! <inline> for some comments.
On Mon, Jul 27, 2015 at 1:56 AM, Michael Rossberg < [email protected]> wrote: > Hi, > > when considering small networks that have extreme requirements on > availability and thus convergence delay, the timers given in the OSPF > RFC seem a little “conservative”, i.e., the delay between accepted LSAs > and the rate at which LSAs are sent. Cisco introduced two commands > 'timers throttle lsa all’ and 'timers lsa arrival’, which allow operators > to tune these parameters. > > I have been writing a patch to also support 'timers lsa arrival’ fully and > ‘timers throttle lsa all’ (without the throttling part) also in quagga. Do > you > see any chance to get this upstream? If so, what needs to be still done? > Warnings that the RFC compliance is gone? > Best regards > > Michael > > ------ > > diff --git a/lib/libospf.h b/lib/libospf.h > index 2796209..e8db5c1 100644 > --- a/lib/libospf.h > +++ b/lib/libospf.h > @@ -39,8 +39,8 @@ > #else > #define OSPF_LS_REFRESH_TIME 1800 > #endif > -#define OSPF_MIN_LS_INTERVAL 5 > -#define OSPF_MIN_LS_ARRIVAL 1 > +#define OSPF_MIN_LS_INTERVAL 5000 /* msec */ > +#define OSPF_MIN_LS_ARRIVAL 1000 /* msec */ > #define OSPF_LSA_INITIAL_AGE 0 /* useful for > debug */ > #define OSPF_LSA_MAXAGE 3600 > #define OSPF_CHECK_AGE 300 > diff --git a/ospfd/ospf_flood.c b/ospfd/ospf_flood.c > index 0e42ff5..df19adf 100644 > --- a/ospfd/ospf_flood.c > +++ b/ospfd/ospf_flood.c > @@ -266,7 +266,7 @@ ospf_flood (struct ospf *ospf, struct ospf_neighbor > *nbr, > ; /* Accept this LSA for quick LSDB resynchronization. */ > } > else if (tv_cmp (tv_sub (recent_relative_time (), current->tv_recv), > - int2tv (OSPF_MIN_LS_ARRIVAL)) < 0) > + msec2tv (ospf->min_ls_arrival)) < 0) > { > if (IS_DEBUG_OSPF_EVENT) > zlog_debug ("LSA[Flooding]: LSA is received recently."); > diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c > index f032601..e62a4e7 100644 > --- a/ospfd/ospf_lsa.c > +++ b/ospfd/ospf_lsa.c > @@ -108,6 +108,17 @@ int2tv (int a) > } > > struct timeval > +msec2tv (int a) > +{ > + struct timeval ret; > + > + ret.tv_sec = 0; > + ret.tv_usec = a * 1000; > + > + return tv_adjust (ret); > +} > I'm not a very big fan of returning local static information in msec2tv. I would prefer that you pass in the struct timeval * that you want filled in. Otherwise you are going to get some very weird behavior when someone down the line missappropriates the msec2tv and calls it twice and expects the output to stay consistent. Additionally if someone ever adds another thread it will misbehave as well. > + > +struct timeval > tv_add (struct timeval a, struct timeval b) > { > struct timeval ret; > @@ -145,9 +156,9 @@ ospf_lsa_refresh_delay (struct ospf_lsa *lsa) > quagga_gettime (QUAGGA_CLK_MONOTONIC, &now); > delta = tv_sub (now, lsa->tv_orig); > > - if (tv_cmp (delta, int2tv (OSPF_MIN_LS_INTERVAL)) < 0) > + if (tv_cmp (delta, msec2tv (OSPF_MIN_LS_INTERVAL)) < 0) > { > - delay = tv_ceil (tv_sub (int2tv (OSPF_MIN_LS_INTERVAL), delta)); > + delay = tv_ceil (tv_sub (msec2tv (OSPF_MIN_LS_INTERVAL), delta)); > > if (IS_DEBUG_OSPF (lsa, LSA_GENERATE)) > zlog_debug ("LSA[Type%d:%s]: Refresh timer delay %d seconds", > diff --git a/ospfd/ospf_lsa.h b/ospfd/ospf_lsa.h > index c71877d..dd3b91a 100644 > --- a/ospfd/ospf_lsa.h > +++ b/ospfd/ospf_lsa.h > @@ -237,6 +237,7 @@ extern struct timeval tv_adjust (struct timeval); > extern int tv_ceil (struct timeval); > extern int tv_floor (struct timeval); > extern struct timeval int2tv (int); > +extern struct timeval msec2tv (int); > extern struct timeval tv_add (struct timeval, struct timeval); > extern struct timeval tv_sub (struct timeval, struct timeval); > extern int tv_cmp (struct timeval, struct timeval); > diff --git a/ospfd/ospf_opaque.c b/ospfd/ospf_opaque.c > index f584fc7..d8f7e04 100644 > --- a/ospfd/ospf_opaque.c > +++ b/ospfd/ospf_opaque.c > @@ -1334,7 +1334,7 @@ ospf_opaque_lsa_originate_schedule (struct > ospf_interface *oi, int *delay0) > zlog_debug ("Schedule Type-9 Opaque-LSA origination in %d sec > later.", delay); > oi->t_opaque_lsa_self = > thread_add_timer (master, ospf_opaque_type9_lsa_originate, oi, > delay); > - delay += OSPF_MIN_LS_INTERVAL; > + delay += top->min_ls_interval / 1000; > } > > if (! list_isempty (ospf_opaque_type10_funclist) > @@ -1351,7 +1351,7 @@ ospf_opaque_lsa_originate_schedule (struct > ospf_interface *oi, int *delay0) > area->t_opaque_lsa_self = > thread_add_timer (master, ospf_opaque_type10_lsa_originate, > area, delay); > - delay += OSPF_MIN_LS_INTERVAL; > + delay += top->min_ls_interval / 1000; > } > > if (! list_isempty (ospf_opaque_type11_funclist) > @@ -1368,7 +1368,7 @@ ospf_opaque_lsa_originate_schedule (struct > ospf_interface *oi, int *delay0) > top->t_opaque_lsa_self = > thread_add_timer (master, ospf_opaque_type11_lsa_originate, > top, delay); > - delay += OSPF_MIN_LS_INTERVAL; > + delay += top->min_ls_interval / 1000; > delay is an integer, do you really mean to throw away the sub second accuracty here? If I am reading the code right, I am able to setup a min_ls_interval of <0-5000>. If I pass in 1500, I want 1.5 seconds, we are going to get 1 second. > } > > /* > @@ -1794,7 +1794,7 @@ ospf_opaque_lsa_reoriginate_schedule (void > *lsa_type_dependent, > * it is highly possible that these conditions might not be satisfied > * at the time of re-origination function is to be called. > */ > - delay = OSPF_MIN_LS_INTERVAL; /* XXX */ > + delay = top->min_ls_interval / 1000; /* XXX */ > > As above. thanks! donald
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
