[PATCH] Add statistics to the OSPF interface object

[Kenth Eriksson]

On Tue, 2019-09-10 at 19:05 +0200, Ondrej Zajicek wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> On Thu, Sep 05, 2019 at 04:28:17PM +0200, Kenth Eriksson wrote:
> > For now only OSPF authentication drops...
> 
> Hi
> 
> Please check commit 60dda81b825725716c0f5ba65256e9a4c3f45948 (not part of
> master branch):
> 

This looks good, really comprehensive set of OSPF interface statistics!
I see great value in this and would like to see it in master! I have
tested your commit SHA on 2.0.4 (resolved some conflicts).

> https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.labs.nic.cz%2Flabs%2Fbird%2Fcommit%2F60dda81b825725716c0f5ba65256e9a4c3f45948&data=02%7C01%7Ckenth.eriksson%40infinera.com%7C5c17f6e6555c4a250dd608d736111b34%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C1%7C637037319426511149&sdata=PGLJMK4tEiNLaAc5cqcWbo01r4zBxYbCigdDjZ3WJmU%3D&reserved=0
> 
> That has comprehensive set of interface statistics. I would like to include
> it to master, but did not yet decided about presentation of statistics
> (there is just crude ospf_iface_stats() as a part of 'show ospf
> interface' command).

Maybe it is more sensible to add this to a new CLI command, e.g. 'show
ospf statistics'? Then you don't have the problem with cluttering down
the OSPF interface view. We also need a way to reset the counters. 

> 
> There is also 8972a08fabfb6bf074480927d79f7b2869f52697 for BGP statistics
> that is just directly in 'show protocols all'.
> 
> I would like to get some feedback and suggestions about how to integrate
> statistics presentation to other commands to avoid cluttering up output
> of other commands with statistics. Perhaps common option 'stats' that
> can be used to modify other commands (like 'show ospf interface')
> indicating that statistics should be displayed?
> 
> 
> > Signed-off-by: Kenth Eriksson <kenth.eriksson at infinera.com>
> > ---
> >  proto/ospf/iface.c  | 1 +
> >  proto/ospf/ospf.h   | 6 ++++++
> >  proto/ospf/packet.c | 4 ++++
> >  3 files changed, 11 insertions(+)
> > 
> > diff --git a/proto/ospf/iface.c b/proto/ospf/iface.c
> > index 6e7e498f..988c0b70 100644
> > --- a/proto/ospf/iface.c
> > +++ b/proto/ospf/iface.c
> > @@ -1404,4 +1404,5 @@ ospf_iface_info(struct ospf_iface *ifa)
> >      cli_msg(-1015, "\tBackup designated router (ID): %R", ifa->bdrid);
> >      cli_msg(-1015, "\tBackup designated router (IP): %I", ifa->bdrip);
> >    }
> > +  cli_msg(-1015, "\tAuthentication drops: %u", ifa->stats.drop_auth);
> >  }
> > diff --git a/proto/ospf/ospf.h b/proto/ospf/ospf.h
> > index 8318ee95..e13e4bb2 100644
> > --- a/proto/ospf/ospf.h
> > +++ b/proto/ospf/ospf.h
> > @@ -273,6 +273,11 @@ struct ospf_area
> >    struct fib rtr;            /* Routing tables for routers */
> >  };
> > 
> > +struct ospf_iface_stats
> > +{
> > +  u32 drop_auth; /* Drops due to authentication fail */
> > +};
> > +
> >  struct ospf_iface
> >  {
> >    node n;
> > @@ -316,6 +321,7 @@ struct ospf_iface
> >    u8 instance_id;            /* Used to differentiate between more OSPF
> >                                  instances on one interface */
> >    u8 autype;                 /* Authentication type (OSPF_AUTH_*) */
> > +  struct ospf_iface_stats stats;/* Interface statistics */
> >    u8 type;                   /* OSPF view of type (OSPF_IT_*) */
> >    u8 strictnbma;             /* Can I talk with unknown neighbors? */
> >    u8 stub;                   /* Inactive interface */
> > diff --git a/proto/ospf/packet.c b/proto/ospf/packet.c
> > index 599f3094..7c6c328e 100644
> > --- a/proto/ospf/packet.c
> > +++ b/proto/ospf/packet.c
> > @@ -214,6 +214,7 @@ ospf_pkt_checkauth2(struct ospf_neighbor *n, struct ospf_iface *ifa, struct ospf
> >      if (n && (rcv_csn < n->csn))
> >        // DROP("lower sequence number", rcv_csn);
> >      {
> > +      ifa->stats.drop_auth++;
> >        /* We want to report both new and old CSN */
> >        LOG_PKT_AUTH("Authentication failed for nbr %R on %s - "
> >                  "lower sequence number (rcv %u, old %u)",
> > @@ -245,6 +246,7 @@ ospf_pkt_checkauth2(struct ospf_neighbor *n, struct ospf_iface *ifa, struct ospf
> >    }
> > 
> >  drop:
> > +  ifa->stats.drop_auth++;
> >    LOG_PKT_AUTH("Authentication failed for nbr %R on %s - %s (%u)",
> >              (n ? n->rid : ntohl(pkt->routerid)), ifa->ifname, err_dsc, err_val);
> > 
> > @@ -327,6 +329,7 @@ ospf_pkt_checkauth3(struct ospf_neighbor *n, struct ospf_iface *ifa, struct ospf
> >    u64 rcv_csn = get_u64(&auth->csn);
> >    if (n && (rcv_csn <= n->csn64[pt]))
> >    {
> > +    ifa->stats.drop_auth++;
> >      /* We want to report both new and old CSN */
> >      LOG_PKT_AUTH("Authentication failed for nbr %R on %s - "
> >                "lower sequence number (rcv %u, old %u)",
> > @@ -358,6 +361,7 @@ ospf_pkt_checkauth3(struct ospf_neighbor *n, struct ospf_iface *ifa, struct ospf
> >    return 1;
> > 
> >  drop:
> > +  ifa->stats.drop_auth++;
> >    LOG_PKT_AUTH("Authentication failed for nbr %R on %s - %s (%u)",
> >              (n ? n->rid : ntohl(pkt->routerid)), ifa->ifname, err_dsc, err_val);
> > 
> > --
> > 2.21.0
> 
> --
> Elen sila lumenn' omentielvo
> 
> Ondrej 'Santiago' Zajicek (email: santiago at crfreenet.org)
> OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
> "To err is human -- to blame it on a computer is even more so."