From nobody Thu Dec 25 10:31:50 2025 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 08C9213AFB; Thu, 18 Jan 2024 12:55:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.187 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705582533; cv=none; b=dK47J+gNDnG+6GcQI8L+gmBqcAoilDqQVKf7lK6NRnUPBcHuqJG+sndLIVZYLe3xT/sKUjAdepY89OjWrRV2dBb8unVs59Pe3oK3MmWZptPHm2hht1ECrKZ31JRzy9+HHx3H+SE4WitWv+VFnxM3JmcDBF2UYUl8knEPUNNP2Hk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705582533; c=relaxed/simple; bh=JMIsJtQBX6kLyKtnYaMFEzQtsIApPm45z17ASci53Wo=; h=Received:Received:Received:Received:From:To:CC:Subject: Thread-Topic:Thread-Index:Date:Message-ID:References:In-Reply-To: Accept-Language:Content-Language:X-MS-Has-Attach: X-MS-TNEF-Correlator:x-originating-ip:Content-Type: Content-Transfer-Encoding:MIME-Version; b=J2zHAmwzgzk+xUM6lYtNu4lsb0ZP37UAglJpazSImv8Zd/h5I+ehllMVks+BC6Erlleu794hHnLd/QrxjAfF/17C3yfiysSfMinWT6Dk+kK3qwmAoEtBNAGYhry3bJEzZPkN9p69m7WldwFVuOoxTcg5upCVDrqQoVCdIHfTWGI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=45.249.212.187 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.19.162.254]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4TG2kw08YhzsVws; Thu, 18 Jan 2024 20:54:32 +0800 (CST) Received: from kwepemd200003.china.huawei.com (unknown [7.221.188.150]) by mail.maildlp.com (Postfix) with ESMTPS id 231D318005B; Thu, 18 Jan 2024 20:55:27 +0800 (CST) Received: from dggpemm500008.china.huawei.com (7.185.36.136) by kwepemd200003.china.huawei.com (7.221.188.150) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.2.1258.28; Thu, 18 Jan 2024 20:55:26 +0800 Received: from dggpemm500008.china.huawei.com ([7.185.36.136]) by dggpemm500008.china.huawei.com ([7.185.36.136]) with mapi id 15.01.2507.035; Thu, 18 Jan 2024 20:55:26 +0800 From: wangyunjian To: Willem de Bruijn , "jasowang@redhat.com" , "kuba@kernel.org" , "davem@davemloft.net" CC: "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , xudingke Subject: RE: [PATCH net v3] tun: add missing rx stats accounting in tun_xdp_act Thread-Topic: [PATCH net v3] tun: add missing rx stats accounting in tun_xdp_act Thread-Index: AQHaSTdaoh49U62FKkOqzRq1lENoTbDdnv0AgAHaJdA= Date: Thu, 18 Jan 2024 12:55:26 +0000 Message-ID: References: <1705490503-28844-1-git-send-email-wangyunjian@huawei.com> <65a7f560a4643_6ba59294a7@willemb.c.googlers.com.notmuch> In-Reply-To: <65a7f560a4643_6ba59294a7@willemb.c.googlers.com.notmuch> Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 > -----Original Message----- > From: Willem de Bruijn [mailto:willemdebruijn.kernel@gmail.com] > Sent: Wednesday, January 17, 2024 11:42 PM > To: wangyunjian ; > willemdebruijn.kernel@gmail.com; jasowang@redhat.com; kuba@kernel.org; > davem@davemloft.net > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; xudingke > ; wangyunjian > Subject: Re: [PATCH net v3] tun: add missing rx stats accounting in tun_x= dp_act >=20 > Yunjian Wang wrote: > > The TUN can be used as vhost-net backend, and it is necessary to count > > the packets transmitted from TUN to vhost-net/virtio-net. However, > > there are some places in the receive path that were not taken into > > account when using XDP. The commit 8ae1aff0b331 ("tuntap: split out > > XDP logic") only includes dropped counter for XDP_DROP, XDP_ABORTED, > > and invalid XDP actions. It would be beneficial to also include new > > accounting for successfully received bytes using > > dev_sw_netstats_rx_add and introduce new dropped counter for XDP errors > on XDP_TX and XDP_REDIRECT. >=20 > From the description it is clear that these are two separate changes wrap= ped > into one patch. I should have flagged this previously. Do I need to split these two modifications into 2 patches? 1. only fix dropped counter 2. add new accounting for successfully received bytes Or: Only fix dropped counter? >=20 > Ack on returning the error counter that was previously present and matches > the Fixes tag. >=20 > For the second change, I had to check a few other XDP capable drivers to = verify > that it is indeed common to count such packets. >=20 > > Fixes: 8ae1aff0b331 ("tuntap: split out XDP logic") > > Signed-off-by: Yunjian Wang > > --- > > v3: update commit log and code > > v2: add Fixes tag > > --- > > drivers/net/tun.c | 14 +++++++++----- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c index > > afa5497f7c35..0704a17e74e1 100644 > > --- a/drivers/net/tun.c > > +++ b/drivers/net/tun.c > > @@ -1625,18 +1625,15 @@ static struct sk_buff *__tun_build_skb(struct > > tun_file *tfile, static int tun_xdp_act(struct tun_struct *tun, struct > bpf_prog *xdp_prog, > > struct xdp_buff *xdp, u32 act) { > > - int err; > > + unsigned int datasize =3D xdp->data_end - xdp->data; > > + int err =3D 0; > > > > switch (act) { > > case XDP_REDIRECT: > > err =3D xdp_do_redirect(tun->dev, xdp, xdp_prog); > > - if (err) > > - return err; > > break; > > case XDP_TX: > > err =3D tun_xdp_tx(tun->dev, xdp); > > - if (err < 0) > > - return err; > > break; > > case XDP_PASS: > > break; > > @@ -1651,6 +1648,13 @@ static int tun_xdp_act(struct tun_struct *tun, > struct bpf_prog *xdp_prog, > > break; > > } > > > > + if (err < 0) { > > + act =3D err; > > + dev_core_stats_rx_dropped_inc(tun->dev); > > + } else if (act =3D=3D XDP_REDIRECT || act =3D=3D XDP_TX) { > > + dev_sw_netstats_rx_add(tun->dev, datasize); > > + } > > + >=20 > Let's avoid adding yet another branch and just do these operations in the= case > statements, like XDP_DROP. Fix it like this? --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1625,18 +1625,25 @@ static struct sk_buff *__tun_build_skb(struct tun_f= ile *tfile, static int tun_xdp_act(struct tun_struct *tun, struct bpf_prog *xdp_prog, struct xdp_buff *xdp, u32 act) { + unsigned int datasize =3D xdp->data_end - xdp->data; int err; =20 switch (act) { case XDP_REDIRECT: err =3D xdp_do_redirect(tun->dev, xdp, xdp_prog); - if (err) + if (err) { + dev_core_stats_rx_dropped_inc(tun->dev); return err; + } + dev_sw_netstats_rx_add(tun->dev, datasize); break; case XDP_TX: err =3D tun_xdp_tx(tun->dev, xdp); - if (err < 0) + if (err < 0) { + dev_core_stats_rx_dropped_inc(tun->dev); return err; + } + dev_sw_netstats_rx_add(tun->dev, datasize); break; case XDP_PASS: >=20 > > return act; > > } > > > > -- > > 2.41.0 > > >=20 >=20