Creation of sysfs entries is expensive, mainly for workloads that
constantly creates netdev and netns often.
Do not create BQL sysfs entries for devices that don't need,
basically those that do not have a real queue, i.e, devices that has
NETIF_F_LLTX and IFF_NO_QUEUE, such as `lo` interface.
This will remove the /sys/class/net/eth0/queues/tx-X/byte_queue_limits/
directory for these devices.
In the example below, eth0 has the `byte_queue_limits` directory but not
`lo`.
# ls /sys/class/net/lo/queues/tx-0/
traffic_class tx_maxrate tx_timeout xps_cpus xps_rxqs
# ls /sys/class/net/eth0/queues/tx-0/byte_queue_limits/
hold_time inflight limit limit_max limit_min
Suggested-by: Eric Dumazet <[email protected]>
Signed-off-by: Breno Leitao <[email protected]>
---
net/core/net-sysfs.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index a09d507c5b03..c79bc11a0347 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1417,6 +1417,15 @@ static ssize_t bql_show_inflight(struct netdev_queue *queue,
return sysfs_emit(buf, "%u\n", dql->num_queued - dql->num_completed);
}
+static bool netdev_uses_bql(struct net_device *dev)
+{
+ if (dev->features & NETIF_F_LLTX ||
+ dev->priv_flags & IFF_NO_QUEUE)
+ return false;
+
+ return true;
+}
+
static struct netdev_queue_attribute bql_inflight_attribute __ro_after_init =
__ATTR(inflight, 0444, bql_show_inflight, NULL);
@@ -1709,9 +1718,11 @@ static int netdev_queue_add_kobject(struct net_device *dev, int index)
goto err;
#ifdef CONFIG_BQL
- error = sysfs_create_group(kobj, &dql_group);
- if (error)
- goto err;
+ if (netdev_uses_bql(dev)) {
+ error = sysfs_create_group(kobj, &dql_group);
+ if (error)
+ goto err;
+ }
#endif
kobject_uevent(kobj, KOBJ_ADD);
@@ -1734,7 +1745,8 @@ static int tx_queue_change_owner(struct net_device *ndev, int index,
return error;
#ifdef CONFIG_BQL
- error = sysfs_group_change_owner(kobj, &dql_group, kuid, kgid);
+ if (netdev_uses_bql(ndev))
+ error = sysfs_group_change_owner(kobj, &dql_group, kuid, kgid);
#endif
return error;
}
@@ -1768,7 +1780,8 @@ netdev_queue_update_kobjects(struct net_device *dev, int old_num, int new_num)
if (!refcount_read(&dev_net(dev)->ns.count))
queue->kobj.uevent_suppress = 1;
#ifdef CONFIG_BQL
- sysfs_remove_group(&queue->kobj, &dql_group);
+ if (netdev_uses_bql(dev))
+ sysfs_remove_group(&queue->kobj, &dql_group);
#endif
kobject_put(&queue->kobj);
}
--
2.39.3
On Thu, Feb 15, 2024 at 12:28 PM Breno Leitao <[email protected]> wrote:
>
> Creation of sysfs entries is expensive, mainly for workloads that
> constantly creates netdev and netns often.
>
> Do not create BQL sysfs entries for devices that don't need,
> basically those that do not have a real queue, i.e, devices that has
> NETIF_F_LLTX and IFF_NO_QUEUE, such as `lo` interface.
>
> This will remove the /sys/class/net/eth0/queues/tx-X/byte_queue_limits/
> directory for these devices.
>
> In the example below, eth0 has the `byte_queue_limits` directory but not
> `lo`.
>
> # ls /sys/class/net/lo/queues/tx-0/
> traffic_class tx_maxrate tx_timeout xps_cpus xps_rxqs
>
> # ls /sys/class/net/eth0/queues/tx-0/byte_queue_limits/
> hold_time inflight limit limit_max limit_min
>
> Suggested-by: Eric Dumazet <[email protected]>
> Signed-off-by: Breno Leitao <[email protected]>
> ---
> net/core/net-sysfs.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index a09d507c5b03..c79bc11a0347 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -1417,6 +1417,15 @@ static ssize_t bql_show_inflight(struct netdev_queue *queue,
> return sysfs_emit(buf, "%u\n", dql->num_queued - dql->num_completed);
> }
>
> +static bool netdev_uses_bql(struct net_device *dev)
const struct net_device *dev
> +{
> + if (dev->features & NETIF_F_LLTX ||
> + dev->priv_flags & IFF_NO_QUEUE)
> + return false;
> +
> + return true;
> +}
> +
>
Thanks.
On Thu, Feb 15, 2024 at 01:47:47PM +0100, Eric Dumazet wrote:
> On Thu, Feb 15, 2024 at 12:28 PM Breno Leitao <[email protected]> wrote:
> >
> > Creation of sysfs entries is expensive, mainly for workloads that
> > constantly creates netdev and netns often.
> >
> > Do not create BQL sysfs entries for devices that don't need,
> > basically those that do not have a real queue, i.e, devices that has
> > NETIF_F_LLTX and IFF_NO_QUEUE, such as `lo` interface.
> >
> > This will remove the /sys/class/net/eth0/queues/tx-X/byte_queue_limits/
> > directory for these devices.
> >
> > In the example below, eth0 has the `byte_queue_limits` directory but not
> > `lo`.
> >
> > # ls /sys/class/net/lo/queues/tx-0/
> > traffic_class tx_maxrate tx_timeout xps_cpus xps_rxqs
> >
> > # ls /sys/class/net/eth0/queues/tx-0/byte_queue_limits/
> > hold_time inflight limit limit_max limit_min
> >
> > Suggested-by: Eric Dumazet <[email protected]>
> > Signed-off-by: Breno Leitao <[email protected]>
> > ---
> > net/core/net-sysfs.c | 23 ++++++++++++++++++-----
> > 1 file changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> > index a09d507c5b03..c79bc11a0347 100644
> > --- a/net/core/net-sysfs.c
> > +++ b/net/core/net-sysfs.c
> > @@ -1417,6 +1417,15 @@ static ssize_t bql_show_inflight(struct netdev_queue *queue,
> > return sysfs_emit(buf, "%u\n", dql->num_queued - dql->num_completed);
> > }
> >
> > +static bool netdev_uses_bql(struct net_device *dev)
>
> const struct net_device *dev
Thanks. I will update and incorporate it in a v2 (that I am planning to
send tomorrow).
On Thu, 15 Feb 2024 03:27:27 -0800 Breno Leitao wrote:
> Creation of sysfs entries is expensive, mainly for workloads that
> constantly creates netdev and netns often.
>
> Do not create BQL sysfs entries for devices that don't need,
> basically those that do not have a real queue, i.e, devices that has
> NETIF_F_LLTX and IFF_NO_QUEUE, such as `lo` interface.
>
> This will remove the /sys/class/net/eth0/queues/tx-X/byte_queue_limits/
> directory for these devices.
>
> In the example below, eth0 has the `byte_queue_limits` directory but not
> `lo`.
>
> # ls /sys/class/net/lo/queues/tx-0/
> traffic_class tx_maxrate tx_timeout xps_cpus xps_rxqs
>
> # ls /sys/class/net/eth0/queues/tx-0/byte_queue_limits/
> hold_time inflight limit limit_max limit_min
I'm tempted to also get rid of the #ifdefs while at it.
> +static bool netdev_uses_bql(struct net_device *dev)
> +{
> + if (dev->features & NETIF_F_LLTX ||
> + dev->priv_flags & IFF_NO_QUEUE)
> + return false;
> +
> + return true;
make this
return IS_ENABLED(CONFIG_BQL);
And throw in something like:
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index a09d507c5b03..119075dff0ee 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1454,6 +1454,9 @@ static const struct attribute_group dql_group = {
.name = "byte_queue_limits",
.attrs = dql_attrs,
};
+#else
+/* Fake declaration, all the code using it should be dead */
+extern const struct attribute_group dql_group;
#endif /* CONFIG_BQL */
#ifdef CONFIG_XPS
You should then be able to remove the #ifdef CONFIG_BQL around the
uses of netdev_uses_bql(), compiler will realize it always returns
false and eliminate the code making use of &dql_group.