From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Florian Westphal <fw@strlen.de>
Cc: Jakub Kicinski <kuba@kernel.org>,
Shigeru Yoshida <syoshida@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
Phil Sutter <phil@nwl.cc>,
syzbot+5a66db916cdde0dbcc1c@syzkaller.appspotmail.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
netfilter-devel@vger.kernel.org, coreteam@netfilter.org
Subject: Re: [PATCH net] net: flow_offload: protect driver_block_list in flow_block_cb_setup_simple()
Date: Tue, 17 Feb 2026 12:42:13 +0100 [thread overview]
Message-ID: <aZRUFQGKzEdcjNHG@chamomile> (raw)
In-Reply-To: <aZHE4r18hkxdITD-@strlen.de>
[-- Attachment #1: Type: text/plain, Size: 1752 bytes --]
On Sun, Feb 15, 2026 at 02:06:42PM +0100, Florian Westphal wrote:
> Jakub Kicinski <kuba@kernel.org> wrote:
> > On Fri, 13 Feb 2026 12:30:58 +0100 Florian Westphal wrote:
> > > > > Looking at the *upper layer*, I don't think it expected drivers to use
> > > > > a single global list for this bit something that is scoped to the
> > > > > net_device.
> > > >
> > > > Maybe subjective but the fix seems a little off to me.
> > > > Isn't flow_block_cb_setup_simple() just a "simple" implementation
> > > > for reuse in drivers locking in there doesn't really guarantee much?
> > >
> > > Not sure what you mean. I see the same pattern as netdevsim in all
> > > drivers using this API.
> >
> > Grep for flow_block_cb_add(). Not all drivers use
>
> static int
> mtk_eth_setup_tc_block(struct net_device *dev, struct flow_block_offload *f)
> {
> struct mtk_mac *mac = netdev_priv(dev);
> struct mtk_eth *eth = mac->hw;
> static LIST_HEAD(block_cb_list);
> ~~~~~~
> I have a question.
>
> [..]
> f->driver_block_list = &block_cb_list;
>
> Now I have many questions!
>
> How is this supposed to work?
Last time I met people, I asked how is hw offload actually working
with netns (6 years ago?), someone told me: "maybe there is a driver
that supports it...". I have never seen one, but I am very much
outdates on how this has evolved TBH, I might be wrong.
I don't think any driver really supports netns + hardware offload, so
I suggest to restrict it, see attached patch.
It would be better to add a helper function such as int net_setup_tc()
for the myriad of ->ndo_setup_tc() calls in the code, then move this
check in it to consolidate, but I think you want something you can
pass to -stable at this stage?
[-- Attachment #2: restrict-hw-offload-to-init_netns.patch --]
[-- Type: text/x-diff, Size: 3332 bytes --]
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 99ac747b7906..a97838c1c56d 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -692,6 +692,9 @@ struct tc_cls_u32_offload {
static inline bool tc_can_offload(const struct net_device *dev)
{
+ if (!net_eq(dev_net(dev), &init_net))
+ return false;
+
return dev->features & NETIF_F_HW_TC;
}
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index b1966b68c48a..c6d426052765 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -1173,6 +1173,9 @@ static int nf_flow_table_offload_cmd(struct flow_block_offload *bo,
{
int err;
+ if (!net_eq(dev_net(dev), &init_net))
+ return -EOPNOTSUPP;
+
nf_flow_table_block_offload_init(bo, dev_net(dev), cmd, flowtable,
extack);
down_write(&flowtable->flow_block_lock);
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index fd30e205de84..048fa5f356d9 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -233,6 +233,9 @@ bool nft_chain_offload_support(const struct nft_base_chain *basechain)
ops->hooknum != NF_NETDEV_INGRESS)
return false;
+ if (!net_eq(dev_net(dev), &init_net))
+ return false;
+
dev = ops->dev;
if (!dev->netdev_ops->ndo_setup_tc &&
!flow_indr_dev_exists())
diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
index 8c9a0400c862..6daa9f702a59 100644
--- a/net/sched/sch_cbs.c
+++ b/net/sched/sch_cbs.c
@@ -281,7 +281,7 @@ static int cbs_enable_offload(struct net_device *dev, struct cbs_sched_data *q,
struct tc_cbs_qopt_offload cbs = { };
int err;
- if (!ops->ndo_setup_tc) {
+ if (!tc_can_offload(dev) || !ops->ndo_setup_tc) {
NL_SET_ERR_MSG(extack, "Specified device does not support cbs offload");
return -EOPNOTSUPP;
}
diff --git a/net/sched/sch_etf.c b/net/sched/sch_etf.c
index c74d778c32a1..7801e009e025 100644
--- a/net/sched/sch_etf.c
+++ b/net/sched/sch_etf.c
@@ -323,7 +323,7 @@ static int etf_enable_offload(struct net_device *dev, struct etf_sched_data *q,
struct tc_etf_qopt_offload etf = { };
int err;
- if (!ops->ndo_setup_tc) {
+ if (!tc_can_offload(dev) || !ops->ndo_setup_tc) {
NL_SET_ERR_MSG(extack, "Specified device does not support ETF offload");
return -EOPNOTSUPP;
}
diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c
index f3e5ef9a9592..d853f3b60121 100644
--- a/net/sched/sch_mqprio.c
+++ b/net/sched/sch_mqprio.c
@@ -410,7 +410,7 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt,
* the queue mapping then run ndo_setup_tc otherwise use the
* supplied and verified mapping
*/
- if (qopt->hw) {
+ if (qopt->hw && tc_can_offload(dev)) {
err = mqprio_enable_offload(sch, qopt, extack);
if (err)
return err;
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 300d577b3286..7451d74af91f 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1522,7 +1522,7 @@ static int taprio_enable_offload(struct net_device *dev,
struct tc_taprio_caps caps;
int tc, err = 0;
- if (!ops->ndo_setup_tc) {
+ if (!tc_can_offload(dev) || !ops->ndo_setup_tc) {
NL_SET_ERR_MSG(extack,
"Device does not support taprio offload");
return -EOPNOTSUPP;
next prev parent reply other threads:[~2026-02-17 11:42 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-08 11:00 [PATCH net] net: flow_offload: protect driver_block_list in flow_block_cb_setup_simple() Shigeru Yoshida
2026-02-11 12:06 ` Florian Westphal
2026-02-13 2:34 ` Jakub Kicinski
2026-02-13 11:30 ` Florian Westphal
2026-02-13 16:17 ` Jakub Kicinski
2026-02-15 13:06 ` Florian Westphal
2026-02-17 11:42 ` Pablo Neira Ayuso [this message]
2026-02-17 22:05 ` 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=aZRUFQGKzEdcjNHG@chamomile \
--to=pablo@netfilter.org \
--cc=coreteam@netfilter.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=fw@strlen.de \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=phil@nwl.cc \
--cc=syoshida@redhat.com \
--cc=syzbot+5a66db916cdde0dbcc1c@syzkaller.appspotmail.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.