Re: [PATCH net-next v3 12/24] ovpn: store tunnel and transport statistics

[Sabrina Dubroca <sd@queasysnail.net>]

2024-05-13, 09:25:29 +0200, Antonio Quartulli wrote:
> On 12/05/2024 10:47, Sabrina Dubroca wrote:
> > 2024-05-06, 03:16:25 +0200, Antonio Quartulli wrote:
> > > Byte/packet counters for in-tunnel and transport streams
> > > are now initialized and updated as needed.
> > > 
> > > To be exported via netlink.
> > > 
> > > Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
> > > ---
> > >   drivers/net/ovpn/Makefile |  1 +
> > >   drivers/net/ovpn/io.c     | 10 ++++++++
> > >   drivers/net/ovpn/peer.c   |  3 +++
> > >   drivers/net/ovpn/peer.h   | 13 +++++++---
> > >   drivers/net/ovpn/stats.c  | 21 ++++++++++++++++
> > >   drivers/net/ovpn/stats.h  | 52 +++++++++++++++++++++++++++++++++++++++
> > 
> > What I'm seeing in this patch are "success" counters. I don't see any
> > stats for dropped packets that would help the user figure out why
> > their VPN isn't working, or why their CPU is burning up decrypting
> > packets that don't show up on the host, etc. You can guess there are
> > issues by subtracting the link and vpn stats, but that's very limited.
> 
> This stats are just the bare minimum to make our current userspace happy :-)
> 
> But we can always extend the stats reporting later on, no?
> 
> > 
> > For example:
> >   - counter for packets dropped during the udp encap/decap
> >   - counter for failed encrypt/decrypt (especially failed decrypt)
> >   - counter for replay protection failures
> >   - counter for malformed packets
> > 
> > Maybe not a separate counter for each of the prints you added in the
> > rx/tx code, but at least enough of them to start figuring out what's
> > going on without enabling all the prints and parsing dmesg.
> 
> Definitely a good suggestion! I'd just postpone it for later, unless you
> think it's a blocker.

I'm not sure. It's not strictly necessary to make the driver work, but
from a user/admin's point of view, I think counters would be really
useful.

Maybe at least increment the rx_dropped/rx_errors/etc counters from
rtnl_link_stats on the netdevice?


> indeed, darn. Juggling this many patches has been quite tedious
>
>
> 
> Yeah..I guess I'll write a small script to catch all these things..it's easy
> to lose them across the whole patchset.
> 
> Thanks for spotting them! I will make sure they all go away

Thanks. I know it's painful :(

-- 
Sabrina