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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 13 May 2024 11:19:12 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Antonio Quartulli <antonio@...nvpn.net>
Cc: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
	Sergey Ryazanov <ryazanov.s.a@...il.com>,
	Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
	Andrew Lunn <andrew@...n.ch>, Esben Haabendal <esben@...nix.com>
Subject: Re: [PATCH net-next v3 12/24] ovpn: store tunnel and transport
 statistics

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@...nvpn.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