netdev - Re: [PATCH bpf v2 4/8] veth: Use tstats per-CPU traffic counters

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]

Message-ID: <3436cf40-cfb2-029e-9a97-4bcf00b24d1f@blackwall.org>
Date: Mon, 13 Nov 2023 10:53:04 +0200
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Daniel Borkmann <daniel@...earbox.net>, martin.lau@...nel.org
Cc: kuba@...nel.org, sdf@...gle.com, netdev@...r.kernel.org,
 bpf@...r.kernel.org, Peilin Ye <peilin.ye@...edance.com>
Subject: Re: [PATCH bpf v2 4/8] veth: Use tstats per-CPU traffic counters

On 11/12/23 22:30, Daniel Borkmann wrote:
> From: Peilin Ye <peilin.ye@...edance.com>
> 
> Currently veth devices use the lstats per-CPU traffic counters, which only
> cover TX traffic. veth_get_stats64() actually populates RX stats of a veth
> device from its peer's TX counters, based on the assumption that a veth
> device can _only_ receive packets from its peer, which is no longer true:
> 
> For example, recent CNIs (like Cilium) can use the bpf_redirect_peer() BPF
> helper to redirect traffic from NIC's tc ingress to veth's tc ingress (in
> a different netns), skipping veth's peer device. Unfortunately, this kind
> of traffic isn't currently accounted for in veth's RX stats.
> 
> In preparation for the fix, use tstats (instead of lstats) to maintain
> both RX and TX counters for each veth device. We'll use RX counters for
> bpf_redirect_peer() traffic, and keep using TX counters for the usual
> "peer-to-peer" traffic. In veth_get_stats64(), calculate RX stats by
> _adding_ RX count to peer's TX count, in order to cover both kinds of
> traffic.
> 
> veth_stats_rx() might need a name change (perhaps to "veth_stats_xdp()")
> for less confusion, but let's leave it to another patch to keep the fix
> minimal.
> 
> Signed-off-by: Peilin Ye <peilin.ye@...edance.com>
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> ---
>   drivers/net/veth.c | 30 +++++++++++-------------------
>   1 file changed, 11 insertions(+), 19 deletions(-)
> 

Reviewed-by: Nikolay Aleksandrov <razor@...ckwall.org>