From: Jakub Kicinski <kuba@kernel.org>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Paolo Abeni <pabeni@redhat.com>,
davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
Linux Kernel Functional Testing <lkft@linaro.org>,
jhs@mojatatu.com, jiri@resnulli.us
Subject: Re: [PATCH net] net: sched: gred: prevent races when adding offloads to stats
Date: Thu, 19 Jan 2023 13:16:28 -0800 [thread overview]
Message-ID: <20230119131628.52cb23f9@kernel.org> (raw)
In-Reply-To: <Y8ml6VQ2L+YQqGmB@pop-os.localdomain>
On Thu, 19 Jan 2023 12:19:53 -0800 Cong Wang wrote:
> > > I'm unsure I read you comment correctly. Please note that the
> > > referenced message includes several splats. The first one - arguably
> > > the most relevant - points to the lack of locking in the gred control
> > > path.
> >
> > Yup, I'm not really sure if we're fixing the right splat for the bug.
> > But I am fairly confident we should be holding a lock while writing
> > bstats from the dump path, enqueue/dequeue may run concurrently.
>
> Explain htb_dump_class_stats()? :) I see two _bstats_update() calls but
> I don't see any tree lock there.
I don't know the HTB offload, if the datapath writes to the same
cl->bstats then it's buggy in the same way. But maybe it doesn't -
the HTB offload is local (as in offloads normal host egress) while
the RED offloads are for switches so the data flows both thru the
"representor" netdevs and therefore the SW instance and the offload
on the HW forwarding path.
next prev parent reply other threads:[~2023-01-19 21:22 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-13 4:41 [PATCH net] net: sched: gred: prevent races when adding offloads to stats Jakub Kicinski
2023-01-15 2:20 ` Cong Wang
2023-01-17 9:00 ` Paolo Abeni
2023-01-17 19:10 ` Jakub Kicinski
2023-01-19 20:19 ` Cong Wang
2023-01-19 21:16 ` Jakub Kicinski [this message]
2023-01-19 20:17 ` Cong Wang
2023-01-19 5:10 ` patchwork-bot+netdevbpf
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=20230119131628.52cb23f9@kernel.org \
--to=kuba@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=lkft@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=xiyou.wangcong@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).