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

[Joe Damato <jdamato@fastly.com>]

On Tue, Nov 12, 2024 at 10:17:40AM +0100, Paolo Abeni wrote:
> On 10/11/24 20:45, Joe Damato wrote:
> > +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();
> > +
> > +	napi = napi_by_id(napi_id);
> 
> AFAICS the above causes a RCU splat in the selftests:
> 
> https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/856342/61-busy-poll-test-sh/stderr
> 
> because napi_by_id() only checks for the RCU lock.
> 
> Could you please have a look?

Thanks for letting me know.

I rebuilt my kernel with CONFIG_PROVE_RCU_LIST and a couple other
debugging options and I was able to reproduce the splat you
mentioned.

I took a look and it looks like there might be two things:
  - netdev_nl_napi_set_doit needs to call rcu_read_lock /
    rcu_read_unlock, which would be a Fixes on the commit in the
    series just merged, and
  - netdev_nl_napi_get_doit also has the same issue and should be
    fixed in a separate commit with its own fixes tag.

If that sounds right to you, I'll propose a short series of 2
patches, 1 to fix each.

Let me know if that sounds OK?

In the meantime, I'm rebuilding a kernel now to ensure my proposed
fix fixes the splat.