linux-kernel - Re: [RFC net-next v3 5/9] net: napi: Add napi

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: <66e4b22743592_19ec3c294db@willemb.c.googlers.com.notmuch>
Date: Fri, 13 Sep 2024 17:44:07 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Joe Damato <jdamato@...tly.com>, 
 netdev@...r.kernel.org
Cc: mkarsten@...terloo.ca, 
 kuba@...nel.org, 
 skhawaja@...gle.com, 
 sdf@...ichev.me, 
 bjorn@...osinc.com, 
 amritha.nambiar@...el.com, 
 sridhar.samudrala@...el.com, 
 "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, 
 Paolo Abeni <pabeni@...hat.com>, 
 Jonathan Corbet <corbet@....net>, 
 Jiri Pirko <jiri@...nulli.us>, 
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>, 
 Lorenzo Bianconi <lorenzo@...nel.org>, 
 David Ahern <dsahern@...nel.org>, 
 Johannes Berg <johannes.berg@...el.com>, 
 "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>, 
 open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC net-next v3 5/9] net: napi: Add napi_config

Joe Damato wrote:
> Several comments on different things below for this patch that I just noticed.
> 
> On Thu, Sep 12, 2024 at 10:07:13AM +0000, Joe Damato wrote:
> > Add a persistent NAPI config area for NAPI configuration to the core.
> > Drivers opt-in to setting the storage for a NAPI by passing an index
> > when calling netif_napi_add_storage.
> > 
> > napi_config is allocated in alloc_netdev_mqs, freed in free_netdev
> > (after the NAPIs are deleted), and set to 0 when napi_enable is called.
> 
> Forgot to re-read all the commit messages. I will do that for rfcv4
> and make sure they are all correct; this message is not correct.
>  
> > Drivers which implement call netif_napi_add_storage will have persistent
> > NAPI IDs.
> > 
> > Signed-off-by: Joe Damato <jdamato@...tly.com>

> > @@ -11062,6 +11110,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
> >  		return NULL;
> >  	}
> >  
> > +	WARN_ON_ONCE(txqs != rxqs);
> 
> This warning triggers for me on boot every time with mlx5 NICs.
> 
> The code in mlx5 seems to get the rxq and txq maximums in:
>   drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>     mlx5e_create_netdev
> 
>   which does:
> 
>     txqs = mlx5e_get_max_num_txqs(mdev, profile);
>     rxqs = mlx5e_get_max_num_rxqs(mdev, profile);
> 
>     netdev = alloc_etherdev_mqs(sizeof(struct mlx5e_priv), txqs, rxqs);
> 
> In my case for my device, txqs: 760, rxqs: 63.
> 
> I would guess that this warning will trigger everytime for mlx5 NICs
> and would be quite annoying.
> 
> We may just want to replace the allocation logic to allocate
> txqs+rxqs, remove the WARN_ON_ONCE, and be OK with some wasted
> space?

I was about to say that txqs == rxqs is not necessary.

The number of napi config structs you want depends on whether the
driver configures separate IRQs for Tx and Rx or not.

Allocating the max of the two is perhaps sufficient for now.