Re: [net-next v5 6/9] netdev-genl: Support setting per-NAPI config values
From: Joe Damato
Date: Fri Oct 11 2024 - 13:51:13 EST
On Fri, Oct 11, 2024 at 07:19:47PM +0200, Eric Dumazet wrote:
> On Thu, Oct 10, 2024 at 6:40 PM Joe Damato <jdamato@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx/
> > > > >
> > > > > 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.
Thank you, Eric.
I am happy to help with the future changes if needed. Feel free to
reach out if you'd like me to assist in any way as I know you have a
tremendous amount of work on your plate.
I will submit the v6 shortly, after I've rebased and retested.