On Wed, Jun 06, 2018 at 09:01:49AM +0200, Claudio Jeker wrote:
> On Wed, Jun 06, 2018 at 08:06:30AM +0200, Remi Locherer wrote:
> > Hi,
> > 
> > RfC 5340 says that for intra area prefix LSAs metric should be set to 0
> > in case of point-to-multipoint  or loopback interfaces. Otherwise metric
> > should be set to the value of the interfaces output cost.
> > 
> > ospf6d currently sends intra area prefix LSAs *always* with metric 0.
> > 
> > Below diff fixes this.
> > 
> > OK?
> > 
> > Remi
> > 
> > 
> > Index: rde.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ospf6d/rde.c,v
> > retrieving revision 1.72
> > diff -u -p -r1.72 rde.c
> > --- rde.c   12 Aug 2017 16:27:50 -0000      1.72
> > +++ rde.c   5 Jun 2018 20:45:03 -0000
> > @@ -1301,7 +1301,6 @@ append_prefix_lsa(struct lsa **lsa, u_in
> >     /* Append prefix to LSA. */
> >     copy = (struct lsa_prefix *)(new_lsa + *len);
> >     memcpy(copy, prefix, lsa_prefix_len);
> > -   copy->metric = 0;
> >  
> >     *lsa = (struct lsa *)new_lsa;
> >     *len = new_len;
> > @@ -1514,6 +1513,7 @@ orig_intra_lsa_rtr(struct area *area, st
> >                     if (iface->type == IF_TYPE_POINTOMULTIPOINT ||
> >                         iface->state & IF_STA_LOOPBACK) {
> >                             lsa_prefix->prefixlen = 128;
> > +                           lsa_prefix->metric = 0;
> 
> This is not strictly needed because there is a bzero done before.

I thought its easier to spot that metric is 0 on purpose.
Same in the updated diff below in prefix_tree_add (calloc()).

What is more common in such a case?

> 
> >                     } else {
> >                             lsa_prefix->prefixlen = ia->prefixlen;
> >                             lsa_prefix->metric = htons(iface->metric);
> > 
> 
> What about the append_prefix_lsa() in orig_intra_lsa_net()?
> Currently the metric is set to 0 there but now it wont.
> What should the behaviour be there?

Metric should be set to 0.

> I guess it would be possible to change prefix_tree_add() to set metric to
> 0 there. Also I think prefix_tree_add() is leaking memory.
> The if (!(IN6_IS_ADDR_LINKLOCAL(&addr)) && ... should have an
> else free(new); block to make sure we don't leak new if it is not added to
> the prefix_tree.


Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/rde.c,v
retrieving revision 1.72
diff -u -p -r1.72 rde.c
--- rde.c       12 Aug 2017 16:27:50 -0000      1.72
+++ rde.c       6 Jun 2018 16:29:46 -0000
@@ -1301,7 +1301,6 @@ append_prefix_lsa(struct lsa **lsa, u_in
        /* Append prefix to LSA. */
        copy = (struct lsa_prefix *)(new_lsa + *len);
        memcpy(copy, prefix, lsa_prefix_len);
-       copy->metric = 0;
 
        *lsa = (struct lsa *)new_lsa;
        *len = new_len;
@@ -1354,6 +1353,8 @@ prefix_tree_add(struct prefix_tree *tree
                memcpy(&addr, new->prefix + 1,
                    LSA_PREFIXSIZE(new->prefix->prefixlen));
 
+               new->prefix->metric = 0;
+
                if (!(IN6_IS_ADDR_LINKLOCAL(&addr)) &&
                    (new->prefix->options & OSPF_PREFIX_NU) == 0 &&
                    (new->prefix->options & OSPF_PREFIX_LA) == 0) {
@@ -1362,7 +1363,8 @@ prefix_tree_add(struct prefix_tree *tree
                                old->prefix->options |= new->prefix->options;
                                free(new);
                        }
-               }
+               } else
+                       free(new);
 
                cur_prefix = cur_prefix + len;
        }
@@ -1514,6 +1516,7 @@ orig_intra_lsa_rtr(struct area *area, st
                        if (iface->type == IF_TYPE_POINTOMULTIPOINT ||
                            iface->state & IF_STA_LOOPBACK) {
                                lsa_prefix->prefixlen = 128;
+                               lsa_prefix->metric = 0;
                        } else {
                                lsa_prefix->prefixlen = ia->prefixlen;
                                lsa_prefix->metric = htons(iface->metric);

Reply via email to