linux-kernel - Re: [net-next v5 6/9] netdev-genl: Support setting per-NAPI config values

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: <CANn89iL65LPmJbiHVt10JvKXSVMhk-SsTN5xdaZ7MjgXXT4f9w@mail.gmail.com>
Date: Fri, 11 Oct 2024 19:19:47 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Joe Damato <jdamato@...tly.com>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org, mkarsten@...terloo.ca, 
	skhawaja@...gle.com, sdf@...ichev.me, bjorn@...osinc.com, 
	amritha.nambiar@...el.com, sridhar.samudrala@...el.com, 
	willemdebruijn.kernel@...il.com, Donald Hunter <donald.hunter@...il.com>, 
	"David S. Miller" <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, 
	Jesper Dangaard Brouer <hawk@...nel.org>, Mina Almasry <almasrymina@...gle.com>, 
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [net-next v5 6/9] netdev-genl: Support setting per-NAPI config values

On Thu, Oct 10, 2024 at 6:40 PM Joe Damato <jdamato@...tly.com> wrote:
>
> On Thu, Oct 10, 2024 at 05:30:26PM +0200, Eric Dumazet wrote:
> > On Thu, Oct 10, 2024 at 5:19 PM Jakub Kicinski <kuba@...nel.org> wrote:
> > >
> > > On Thu, 10 Oct 2024 06:24:54 +0200 Eric Dumazet wrote:
> > > > > +static const struct netlink_range_validation netdev_a_napi_defer_hard_irqs_range = {
> > > > > +       .max    = 2147483647ULL,
> > > >
> > > > Would (u64)INT_MAX  work ?
> > >
> > > I sent a codegen change for this. The codegen is a bit of a mess.
> > >
> > > > > +int netdev_nl_napi_set_doit(struct sk_buff *skb, struct genl_info *info)
> > > > > +{
> > > > > +       struct napi_struct *napi;
> > > > > +       unsigned int napi_id;
> > > > > +       int err;
> > > > > +
> > > > > +       if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_NAPI_ID))
> > > > > +               return -EINVAL;
> > > > > +
> > > > > +       napi_id = nla_get_u32(info->attrs[NETDEV_A_NAPI_ID]);
> > > > > +
> > > > > +       rtnl_lock();
> > > >
> > > > Hmm.... please see my patch there :
> > > >
> > > >  https://patchwork.kernel.org/project/netdevbpf/patch/20241009232728.107604-2-edumazet@google.com/
> > > >
> > > > Lets not add another rtnl_lock() :/
> > >
> > > It's not as easy since NAPIs can come and go at driver's whim.
> > > I'm quietly hoping we can convert all netdev-nl NAPI accesses
> > > to use the netdev->lock protection I strong-armed Paolo into
> > > adding in his shaper series. But perhaps we can do that after
> > > this series? NAPI GET already takes RTNL lock.
> >
> >
> > napi_by_id() is protected by rcu and its own spinlock ( napi_hash_lock )
> > I do not see why rtnl is needed.
> > This will also be a big issue with per netns-RTNL anyway.
>
> I deeply appreciate and respect both of your thoughts on this; I
> will hold off on sending a v6 until a decision is made on this
> particular issue.
>

I do not want to block your series.

Whatever is needed later, I can handle.