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.eriks...@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: santi...@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."
