From: Joe Damato <jdamato@fastly.com>
To: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
netdev@vger.kernel.org, mkarsten@uwaterloo.ca,
skhawaja@google.com, sdf@fomichev.me, bjorn@rivosinc.com,
amritha.nambiar@intel.com, sridhar.samudrala@intel.com,
willemdebruijn.kernel@gmail.com,
Donald Hunter <donald.hunter@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Paolo Abeni <pabeni@redhat.com>,
Jesper Dangaard Brouer <hawk@kernel.org>,
Mina Almasry <almasrymina@google.com>,
Xuan Zhuo <xuanzhuo@linux.alibaba.com>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [net-next v5 6/9] netdev-genl: Support setting per-NAPI config values
Date: Fri, 11 Oct 2024 10:50:59 -0700 [thread overview]
Message-ID: <ZwllgzfOrK86q15M@LQ3V64L9R2> (raw)
In-Reply-To: <CANn89iL65LPmJbiHVt10JvKXSVMhk-SsTN5xdaZ7MjgXXT4f9w@mail.gmail.com>
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@fastly.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@kernel.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.
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.
next prev parent reply other threads:[~2024-10-11 17:51 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-09 0:54 [net-next v5 0/9] Add support for per-NAPI config via netlink Joe Damato
2024-10-09 0:54 ` [net-next v5 1/9] net: napi: Make napi_defer_hard_irqs per-NAPI Joe Damato
2024-10-10 4:10 ` Eric Dumazet
2024-10-09 0:54 ` [net-next v5 2/9] netdev-genl: Dump napi_defer_hard_irqs Joe Damato
2024-10-10 4:11 ` Eric Dumazet
2024-10-09 0:54 ` [net-next v5 3/9] net: napi: Make gro_flush_timeout per-NAPI Joe Damato
2024-10-10 4:13 ` Eric Dumazet
2024-10-09 0:54 ` [net-next v5 4/9] netdev-genl: Dump gro_flush_timeout Joe Damato
2024-10-10 3:14 ` Jakub Kicinski
2024-10-10 4:34 ` Joe Damato
2024-10-10 4:45 ` Eric Dumazet
2024-10-10 4:59 ` Joe Damato
2024-10-09 0:54 ` [net-next v5 5/9] net: napi: Add napi_config Joe Damato
2024-10-10 4:20 ` Eric Dumazet
2024-10-10 15:59 ` Joe Damato
2024-10-10 16:08 ` Jakub Kicinski
2024-10-09 0:55 ` [net-next v5 6/9] netdev-genl: Support setting per-NAPI config values Joe Damato
2024-10-10 4:24 ` Eric Dumazet
2024-10-10 15:19 ` Jakub Kicinski
2024-10-10 15:30 ` Eric Dumazet
2024-10-10 16:40 ` Joe Damato
2024-10-11 17:19 ` Eric Dumazet
2024-10-11 17:50 ` Joe Damato [this message]
2024-10-09 0:55 ` [net-next v5 7/9] bnxt: Add support for persistent NAPI config Joe Damato
2024-10-10 4:25 ` Eric Dumazet
2024-10-09 0:55 ` [net-next v5 8/9] mlx5: " Joe Damato
2024-10-10 4:26 ` Eric Dumazet
2024-10-09 0:55 ` [net-next v5 9/9] mlx4: Add support for persistent NAPI config to RX CQs Joe Damato
2024-10-10 4:28 ` Eric Dumazet
2024-10-10 16:07 ` Joe Damato
2024-10-10 3:17 ` [net-next v5 0/9] Add support for per-NAPI config via netlink Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZwllgzfOrK86q15M@LQ3V64L9R2 \
--to=jdamato@fastly.com \
--cc=almasrymina@google.com \
--cc=amritha.nambiar@intel.com \
--cc=bjorn@rivosinc.com \
--cc=davem@davemloft.net \
--cc=donald.hunter@gmail.com \
--cc=edumazet@google.com \
--cc=hawk@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mkarsten@uwaterloo.ca \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sdf@fomichev.me \
--cc=skhawaja@google.com \
--cc=sridhar.samudrala@intel.com \
--cc=willemdebruijn.kernel@gmail.com \
--cc=xuanzhuo@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.