netdev - Re: [PATCH net-next] net: dqs: introduce NETIF_F_NO

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: <ZmaeNfHr-D5jTsq9@nanopsycho.orion>
Date: Mon, 10 Jun 2024 08:33:25 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	davem@...emloft.net, dsahern@...nel.org, mst@...hat.com,
	jasowang@...hat.com, xuanzhuo@...ux.alibaba.com,
	eperezma@...hat.com, leitao@...ian.org, netdev@...r.kernel.org,
	Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next] net: dqs: introduce NETIF_F_NO_BQL device
 feature

Sun, Jun 09, 2024 at 03:17:32PM CEST, kerneljasonxing@...il.com wrote:
>From: Jason Xing <kernelxing@...cent.com>
>
>Since commit 74293ea1c4db6 ("net: sysfs: Do not create sysfs for non
>BQL device") limits the non-BQL driver not creating byte_queue_limits
>directory, I found there is one exception, namely, virtio-net driver,
>which should also be limited in netdev_uses_bql().
>
>I decided to introduce a NO_BQL bit in device feature because
>1) it can help us limit virtio-net driver for now.
>2) if we found another non-BQL driver, we can take it into account.
>3) we can replace all the driver meeting those two statements in
>netdev_uses_bql() in future.
>
>For now, I would like to make the first step to use this new bit for dqs
>use instead of replacing/applying all the non-BQL drivers.
>
>After this patch, 1) there is no byte_queue_limits directory in virtio-net
>driver. 2) running ethtool -k eth1 shows "no-bql: on [fixed]".

Wait, you introduce this flag only for the sake of virtio_net driver,
don't you. Since there is currently an attempt to implement bql in
virtio_net, wouldn't it make this flag obsolete? Can't you wait?


>
>Signed-off-by: Jason Xing <kernelxing@...cent.com>
>---
> drivers/net/virtio_net.c        | 2 +-
> include/linux/netdev_features.h | 3 ++-
> net/core/net-sysfs.c            | 2 +-
> net/ethtool/common.c            | 1 +
> 4 files changed, 5 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>index 61a57d134544..619908fed14b 100644
>--- a/drivers/net/virtio_net.c
>+++ b/drivers/net/virtio_net.c
>@@ -5634,7 +5634,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> 			   IFF_TX_SKB_NO_LINEAR;
> 	dev->netdev_ops = &virtnet_netdev;
> 	dev->stat_ops = &virtnet_stat_ops;
>-	dev->features = NETIF_F_HIGHDMA;
>+	dev->features = NETIF_F_HIGHDMA | NETIF_F_NO_BQL;
> 
> 	dev->ethtool_ops = &virtnet_ethtool_ops;
> 	SET_NETDEV_DEV(dev, &vdev->dev);
>diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>index 7c2d77d75a88..9bc603bb4227 100644
>--- a/include/linux/netdev_features.h
>+++ b/include/linux/netdev_features.h
>@@ -14,7 +14,6 @@ typedef u64 netdev_features_t;
> enum {
> 	NETIF_F_SG_BIT,			/* Scatter/gather IO. */
> 	NETIF_F_IP_CSUM_BIT,		/* Can checksum TCP/UDP over IPv4. */
>-	__UNUSED_NETIF_F_1,
> 	NETIF_F_HW_CSUM_BIT,		/* Can checksum all the packets. */
> 	NETIF_F_IPV6_CSUM_BIT,		/* Can checksum TCP/UDP over IPV6 */
> 	NETIF_F_HIGHDMA_BIT,		/* Can DMA to high memory. */
>@@ -91,6 +90,7 @@ enum {
> 	NETIF_F_HW_HSR_FWD_BIT,		/* Offload HSR forwarding */
> 	NETIF_F_HW_HSR_DUP_BIT,		/* Offload HSR duplication */
> 
>+	NETIF_F_NO_BQL_BIT,		/* non-BQL driver */
> 	/*
> 	 * Add your fresh new feature above and remember to update
> 	 * netdev_features_strings[] in net/ethtool/common.c and maybe
>@@ -168,6 +168,7 @@ enum {
> #define NETIF_F_HW_HSR_TAG_RM	__NETIF_F(HW_HSR_TAG_RM)
> #define NETIF_F_HW_HSR_FWD	__NETIF_F(HW_HSR_FWD)
> #define NETIF_F_HW_HSR_DUP	__NETIF_F(HW_HSR_DUP)
>+#define NETIF_F_NO_BQL		__NETIF_F(NO_BQL)
> 
> /* Finds the next feature with the highest number of the range of start-1 till 0.
>  */
>diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
>index 4c27a360c294..ff397a76f1fe 100644
>--- a/net/core/net-sysfs.c
>+++ b/net/core/net-sysfs.c
>@@ -1764,7 +1764,7 @@ static const struct kobj_type netdev_queue_ktype = {
> 
> static bool netdev_uses_bql(const struct net_device *dev)
> {
>-	if (dev->features & NETIF_F_LLTX ||
>+	if (dev->features & (NETIF_F_LLTX | NETIF_F_NO_BQL) ||
> 	    dev->priv_flags & IFF_NO_QUEUE)
> 		return false;
> 
>diff --git a/net/ethtool/common.c b/net/ethtool/common.c
>index 6b2a360dcdf0..efa7ac4158ce 100644
>--- a/net/ethtool/common.c
>+++ b/net/ethtool/common.c
>@@ -74,6 +74,7 @@ const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
> 	[NETIF_F_HW_HSR_TAG_RM_BIT] =	 "hsr-tag-rm-offload",
> 	[NETIF_F_HW_HSR_FWD_BIT] =	 "hsr-fwd-offload",
> 	[NETIF_F_HW_HSR_DUP_BIT] =	 "hsr-dup-offload",
>+	[NETIF_F_NO_BQL_BIT] =		 "no-bql",
> };
> 
> const char
>-- 
>2.37.3
>
>