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

Antonio Quartulli
2024-05-13T09:31:45Z

On 13/05/2024 11:19, Sabrina Dubroca wrote:
> 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?

Ok, will start with this and see how much work is to add the err
counters right away.

Thanks for the hint!

--
Antonio Quartulli
OpenVPN Inc.