[RFC,net-next,v1,1/2] net: add name field to napi struct

Commit Message

Yannick Vignon May 6, 2021, 5:20 p.m. UTC

From: Yannick Vignon <[e-mail]>

An interesting possibility offered by the new thread NAPI code is to
fine-tune the affinities and priorities of different NAPI instances. In a
real-time networking context, this makes it possible to ensure packets
received in a high-priority queue are always processed, and with low
latency.

However, the way the NAPI threads are named does not really expose which
one is responsible for a given queue. Assigning a more explicit name to
NAPI instances can make that determination much easier.

Signed-off-by: Yannick Vignon <[e-mail]>
---
 include/linux/netdevice.h | 42 +++++++++++++++++++++++++++++++++++++--
 net/core/dev.c            | 20 +++++++++++++++----
 2 files changed, 56 insertions(+), 6 deletions(-)

Comments

Eric Dumazet May 6, 2021, 5:35 p.m. UTC | #1

On Thu, May 6, 2021 at 7:20 PM Yannick Vignon
<[e-mail]> wrote:
>
> From: Yannick Vignon <[e-mail]>
>
> An interesting possibility offered by the new thread NAPI code is to
> fine-tune the affinities and priorities of different NAPI instances. In a
> real-time networking context, this makes it possible to ensure packets
> received in a high-priority queue are always processed, and with low
> latency.
>
> However, the way the NAPI threads are named does not really expose which
> one is responsible for a given queue. Assigning a more explicit name to
> NAPI instances can make that determination much easier.
>
> Signed-off-by: Yannick Vignon <[e-mail]>
> -

Having to change drivers seems a lot of work

How about exposing thread id (and napi_id eventually) in
/sys/class/net/eth0/queues/*/kthread_pid  ?

Yannick Vignon May 11, 2021, 4:44 p.m. UTC | #2

On 5/6/2021 7:35 PM, Eric Dumazet wrote:
> On Thu, May 6, 2021 at 7:20 PM Yannick Vignon
> <[e-mail]> wrote:
>>
>> From: Yannick Vignon <[e-mail]>
>>
>> An interesting possibility offered by the new thread NAPI code is to
>> fine-tune the affinities and priorities of different NAPI instances. In a
>> real-time networking context, this makes it possible to ensure packets
>> received in a high-priority queue are always processed, and with low
>> latency.
>>
>> However, the way the NAPI threads are named does not really expose which
>> one is responsible for a given queue. Assigning a more explicit name to
>> NAPI instances can make that determination much easier.
>>
>> Signed-off-by: Yannick Vignon <[e-mail]>
>> -
> 
> Having to change drivers seems a lot of work
> 
> How about exposing thread id (and napi_id eventually) in
> /sys/class/net/eth0/queues/*/kthread_pid  ?
> 

This seemed like a good idea, but after looking into how to actually 
implement it, I can't find a way to "map" rx queues to napi instances. 
Am I missing something?

In the end, I'm afraid that the NAPI<->RX queue mapping is only known 
within the drivers, so we'll have no choice but to modify them
to extract that information somehow.

Patch

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5cbc950b34df..01fa9d342383 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -318,6 +318,8 @@  struct gro_list {
  */
 #define GRO_HASH_BUCKETS	8
 
+
+#define NAPINAMSIZ		8
 /*
  * Structure for NAPI scheduling similar to tasklet but with weighting
  */
@@ -348,6 +350,7 @@  struct napi_struct {
 	struct hlist_node	napi_hash_node;
 	unsigned int		napi_id;
 	struct task_struct	*thread;
+	char			name[NAPINAMSIZ];
 };
 
 enum {
@@ -2445,6 +2448,21 @@  static inline void *netdev_priv(const struct net_device *dev)
  */
 #define NAPI_POLL_WEIGHT 64
 
+/**
+ *	netif_napi_add_named - initialize a NAPI context
+ *	@dev:  network device
+ *	@napi: NAPI context
+ *	@poll: polling function
+ *	@weight: default weight
+ *	@name: napi instance name
+ *
+ * netif_napi_add_named() must be used to initialize a NAPI context prior to calling
+ * *any* of the other NAPI-related functions.
+ */
+void netif_napi_add_named(struct net_device *dev, struct napi_struct *napi,
+		    int (*poll)(struct napi_struct *, int), int weight,
+		    const char *name);
+
 /**
  *	netif_napi_add - initialize a NAPI context
  *	@dev:  network device
@@ -2458,6 +2476,27 @@  static inline void *netdev_priv(const struct net_device *dev)
 void netif_napi_add(struct net_device *dev, struct napi_struct *napi,
 		    int (*poll)(struct napi_struct *, int), int weight);
 
+/**
+ *	netif_tx_napi_add_named - initialize a NAPI context
+ *	@dev:  network device
+ *	@napi: NAPI context
+ *	@poll: polling function
+ *	@weight: default weight
+ *	@name: napi instance name
+ *
+ * This variant of netif_napi_add_named() should be used from drivers using NAPI
+ * to exclusively poll a TX queue.
+ * This will avoid we add it into napi_hash[], thus polluting this hash table.
+ */
+static inline void netif_tx_napi_add_named(struct net_device *dev,
+				     struct napi_struct *napi,
+				     int (*poll)(struct napi_struct *, int),
+				     int weight, const char *name)
+{
+	set_bit(NAPI_STATE_NO_BUSY_POLL, &napi->state);
+	netif_napi_add_named(dev, napi, poll, weight, name);
+}
+
 /**
  *	netif_tx_napi_add - initialize a NAPI context
  *	@dev:  network device
@@ -2474,8 +2513,7 @@  static inline void netif_tx_napi_add(struct net_device *dev,
 				     int (*poll)(struct napi_struct *, int),
 				     int weight)
 {
-	set_bit(NAPI_STATE_NO_BUSY_POLL, &napi->state);
-	netif_napi_add(dev, napi, poll, weight);
+	netif_tx_napi_add_named(dev, napi, poll, weight, NULL);
 }
 
 /**
diff --git a/net/core/dev.c b/net/core/dev.c
index 222b1d322c96..bc70f545cc5a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1563,8 +1563,8 @@  static int napi_kthread_create(struct napi_struct *n)
 	 * TASK_INTERRUPTIBLE mode to avoid the blocked task
 	 * warning and work with loadavg.
 	 */
-	n->thread = kthread_run(napi_threaded_poll, n, "napi/%s-%d",
-				n->dev->name, n->napi_id);
+	n->thread = kthread_run(napi_threaded_poll, n, "napi/%s-%s",
+				n->dev->name, n->name);
 	if (IS_ERR(n->thread)) {
 		err = PTR_ERR(n->thread);
 		pr_err("kthread_run failed with err %d\n", err);
@@ -6844,8 +6844,9 @@  int dev_set_threaded(struct net_device *dev, bool threaded)
 }
 EXPORT_SYMBOL(dev_set_threaded);
 
-void netif_napi_add(struct net_device *dev, struct napi_struct *napi,
-		    int (*poll)(struct napi_struct *, int), int weight)
+void netif_napi_add_named(struct net_device *dev, struct napi_struct *napi,
+			  int (*poll)(struct napi_struct *, int), int weight,
+			  const char *name)
 {
 	if (WARN_ON(test_and_set_bit(NAPI_STATE_LISTED, &napi->state)))
 		return;
@@ -6871,6 +6872,10 @@  void netif_napi_add(struct net_device *dev, struct napi_struct *napi,
 	set_bit(NAPI_STATE_NPSVC, &napi->state);
 	list_add_rcu(&napi->dev_list, &dev->napi_list);
 	napi_hash_add(napi);
+	if (name)
+		strncpy(napi->name, name, NAPINAMSIZ);
+	else
+		snprintf(napi->name, NAPINAMSIZ, "%d", napi->napi_id);
 	/* Create kthread for this napi if dev->threaded is set.
 	 * Clear dev->threaded if kthread creation failed so that
 	 * threaded mode will not be enabled in napi_enable().
@@ -6878,6 +6883,13 @@  void netif_napi_add(struct net_device *dev, struct napi_struct *napi,
 	if (dev->threaded && napi_kthread_create(napi))
 		dev->threaded = 0;
 }
+EXPORT_SYMBOL(netif_napi_add_named);
+
+void netif_napi_add(struct net_device *dev, struct napi_struct *napi,
+		    int (*poll)(struct napi_struct *, int), int weight)
+{
+	netif_napi_add_named(dev, napi, poll, weight, NULL);
+}
 EXPORT_SYMBOL(netif_napi_add);
 
 void napi_disable(struct napi_struct *n)