public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2] net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE
@ 2026-01-31  4:58 David Yang
  2026-02-02 10:52 ` Vladimir Oltean
  0 siblings, 1 reply; 4+ messages in thread
From: David Yang @ 2026-01-31  4:58 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	linux-kernel

New options were introduced to FLOW_ACTION_POLICE after struct
dsa_mall_policer_tc_entry was added. The following commands will succeed
on DSA ports:

  tc qdisc add dev lan1 handle ffff: ingress
  tc filter add dev lan1 ingress matchall skip_sw action police \
    pkts_rate 80000 pkts_burst 100 mtu 1000 conform-exceed ok

resulting
  1. burst_pkt, rate_pkt_ps, etc. being ignored;
  2. burst and rate_bytes_per_sec set to 0 without any error.

Among new options, some may be useful to hardware offloading, such as
packet rate mode. Instead of making decisions for drivers, extend struct
dsa_mall_policer_tc_entry to all options of FLOW_ACTION_POLICE.

Drivers should reject unsupported combinations in their
.port_policer_add() implementations.

We are also aware that .port_policer_add() have been implemented in some
DSA drivers - they have already been affected by the above issue (since
we didn't conduct any checks on our side). However, it's up to them to
decide if options should be checked (which may cause regressions with
previously successful configurations), or they are satisfied with the
current behavior - if not, they should submit their own patches to fix
it.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
v1: https://lore.kernel.org/r/20260126061340.757543-1-mmyangfl@gmail.com
  - fix DSA core only
 include/net/dsa.h | 11 +++++++++++
 net/dsa/user.c    | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 6b2b5ed64ea4..4c177b168ec8 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -21,6 +21,7 @@
 #include <linux/phylink.h>
 #include <net/devlink.h>
 #include <net/switchdev.h>
+#include <net/flow_offload.h>
 
 struct dsa_8021q_context;
 struct tc_action;
@@ -220,6 +221,16 @@ struct dsa_mall_mirror_tc_entry {
 struct dsa_mall_policer_tc_entry {
 	u32 burst;
 	u64 rate_bytes_per_sec;
+	u64 peakrate_bytes_ps;
+	u32 avrate;
+	u16 overhead;
+	u64 burst_pkt;
+	u64 rate_pkt_ps;
+	u32 mtu;
+	struct {
+		enum flow_action_id act_id;
+		u32 extval;
+	} exceed, notexceed;
 };
 
 /* TC matchall entry */
diff --git a/net/dsa/user.c b/net/dsa/user.c
index f59d66f0975d..2a209b83c701 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1497,8 +1497,19 @@ dsa_user_add_cls_matchall_police(struct net_device *dev,
 	mall_tc_entry->cookie = cls->cookie;
 	mall_tc_entry->type = DSA_PORT_MALL_POLICER;
 	policer = &mall_tc_entry->policer;
+	/* until they export the type of act->police in flow_offload.h ... */
 	policer->rate_bytes_per_sec = act->police.rate_bytes_ps;
 	policer->burst = act->police.burst;
+	policer->peakrate_bytes_ps = act->police.peakrate_bytes_ps;
+	policer->avrate = act->police.avrate;
+	policer->overhead = act->police.overhead;
+	policer->burst_pkt = act->police.burst_pkt;
+	policer->rate_pkt_ps = act->police.rate_pkt_ps;
+	policer->mtu = act->police.mtu;
+	policer->exceed.act_id = act->police.exceed.act_id;
+	policer->exceed.extval = act->police.exceed.extval;
+	policer->notexceed.act_id = act->police.notexceed.act_id;
+	policer->notexceed.extval = act->police.notexceed.extval;
 
 	err = ds->ops->port_policer_add(ds, dp->index, policer);
 	if (err) {
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next v2] net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE
  2026-01-31  4:58 [PATCH net-next v2] net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE David Yang
@ 2026-02-02 10:52 ` Vladimir Oltean
  2026-02-03  1:40   ` David Yang
  0 siblings, 1 reply; 4+ messages in thread
From: Vladimir Oltean @ 2026-02-02 10:52 UTC (permalink / raw)
  To: David Yang
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 676 bytes --]

On Sat, Jan 31, 2026 at 12:58:16PM +0800, David Yang wrote:
> diff --git a/net/dsa/user.c b/net/dsa/user.c
> index f59d66f0975d..2a209b83c701 100644
> --- a/net/dsa/user.c
> +++ b/net/dsa/user.c
> @@ -1497,8 +1497,19 @@ dsa_user_add_cls_matchall_police(struct net_device *dev,
>  	mall_tc_entry->cookie = cls->cookie;
>  	mall_tc_entry->type = DSA_PORT_MALL_POLICER;
>  	policer = &mall_tc_entry->policer;
> +	/* until they export the type of act->police in flow_offload.h ... */

There is no "us" and "them", it's a single git tree. Just submit a patch
to whomever ./scripts/get_maintainer.pl says to.

Have you tried something like the patch attached (just compile-tested)?

[-- Attachment #2: 0001-net-dsa-eliminate-local-type-for-tc-policers.patch --]
[-- Type: text/x-diff, Size: 5536 bytes --]

From 7decde3308a690061ec8d874f06b285bde27e4fd Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Mon, 2 Feb 2026 12:45:09 +0200
Subject: [PATCH] net: dsa: eliminate local type for tc policers

David Yang is saying that struct flow_action_entry in
include/net/flow_offload.h has gained new fields and DSA's struct
dsa_mall_policer_tc_entry, derived from that, isn't keeping up.
This structure is passed to drivers and they are completely oblivious to
the values of fields they don't see.

This has happened before, and almost always the solution was to make the
DSA layer thinner and use the upstream data structures. Here, the reason
why we didn't do that is because struct flow_action_entry :: police is
an anonymous structure.

That is easily enough fixable, just name those fields "struct
flow_action_police" and reference them from DSA.

Make the according transformations to the two users (sja1105 and felix):
"rate_bytes_per_sec" -> "rate_bytes_ps".

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c         |  4 ++--
 drivers/net/dsa/sja1105/sja1105_main.c |  4 ++--
 include/net/dsa.h                      | 10 ++--------
 include/net/flow_offload.h             |  2 +-
 net/dsa/user.c                         |  5 ++---
 5 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 9e5ede932b42..5d34eb82e639 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -2003,11 +2003,11 @@ static int felix_cls_flower_stats(struct dsa_switch *ds, int port,
 }
 
 static int felix_port_policer_add(struct dsa_switch *ds, int port,
-				  struct dsa_mall_policer_tc_entry *policer)
+				  const struct flow_action_police *policer)
 {
 	struct ocelot *ocelot = ds->priv;
 	struct ocelot_policer pol = {
-		.rate = div_u64(policer->rate_bytes_per_sec, 1000) * 8,
+		.rate = div_u64(policer->rate_bytes_ps, 1000) * 8,
 		.burst = policer->burst,
 	};
 
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 2a4a0fe20dae..71a817c07a90 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2841,7 +2841,7 @@ static void sja1105_mirror_del(struct dsa_switch *ds, int port,
 }
 
 static int sja1105_port_policer_add(struct dsa_switch *ds, int port,
-				    struct dsa_mall_policer_tc_entry *policer)
+				    const struct flow_action_police *policer)
 {
 	struct sja1105_l2_policing_entry *policing;
 	struct sja1105_private *priv = ds->priv;
@@ -2852,7 +2852,7 @@ static int sja1105_port_policer_add(struct dsa_switch *ds, int port,
 	 * the value of RATE bytes divided by 64, up to a maximum of SMAX
 	 * bytes.
 	 */
-	policing[port].rate = div_u64(512 * policer->rate_bytes_per_sec,
+	policing[port].rate = div_u64(512 * policer->rate_bytes_ps,
 				      1000000);
 	policing[port].smax = policer->burst;
 
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 1e33242b6d94..6c17446f3dcc 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -218,12 +218,6 @@ struct dsa_mall_mirror_tc_entry {
 	bool ingress;
 };
 
-/* TC port policer entry */
-struct dsa_mall_policer_tc_entry {
-	u32 burst;
-	u64 rate_bytes_per_sec;
-};
-
 /* TC matchall entry */
 struct dsa_mall_tc_entry {
 	struct list_head list;
@@ -231,7 +225,7 @@ struct dsa_mall_tc_entry {
 	enum dsa_port_mall_action_type type;
 	union {
 		struct dsa_mall_mirror_tc_entry mirror;
-		struct dsa_mall_policer_tc_entry policer;
+		struct flow_action_police policer;
 	};
 };
 
@@ -1112,7 +1106,7 @@ struct dsa_switch_ops {
 	void	(*port_mirror_del)(struct dsa_switch *ds, int port,
 				   struct dsa_mall_mirror_tc_entry *mirror);
 	int	(*port_policer_add)(struct dsa_switch *ds, int port,
-				    struct dsa_mall_policer_tc_entry *policer);
+				    const struct flow_action_police *policer);
 	void	(*port_policer_del)(struct dsa_switch *ds, int port);
 	int	(*port_setup_tc)(struct dsa_switch *ds, int port,
 				 enum tc_setup_type type, void *type_data);
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 596ab9791e4d..a2b1b752958a 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -275,7 +275,7 @@ struct flow_action_entry {
 			u32			trunc_size;
 			bool			truncate;
 		} sample;
-		struct {				/* FLOW_ACTION_POLICE */
+		struct flow_action_police {		/* FLOW_ACTION_POLICE */
 			u32			burst;
 			u64			rate_bytes_ps;
 			u64			peakrate_bytes_ps;
diff --git a/net/dsa/user.c b/net/dsa/user.c
index f59d66f0975d..df25a8891c01 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1459,8 +1459,8 @@ dsa_user_add_cls_matchall_police(struct net_device *dev,
 	struct netlink_ext_ack *extack = cls->common.extack;
 	struct dsa_port *dp = dsa_user_to_port(dev);
 	struct dsa_user_priv *p = netdev_priv(dev);
-	struct dsa_mall_policer_tc_entry *policer;
 	struct dsa_mall_tc_entry *mall_tc_entry;
+	struct flow_action_police *policer;
 	struct dsa_switch *ds = dp->ds;
 	struct flow_action_entry *act;
 	int err;
@@ -1497,8 +1497,7 @@ dsa_user_add_cls_matchall_police(struct net_device *dev,
 	mall_tc_entry->cookie = cls->cookie;
 	mall_tc_entry->type = DSA_PORT_MALL_POLICER;
 	policer = &mall_tc_entry->policer;
-	policer->rate_bytes_per_sec = act->police.rate_bytes_ps;
-	policer->burst = act->police.burst;
+	memcpy(policer, &act->police, sizeof(*policer));
 
 	err = ds->ops->port_policer_add(ds, dp->index, policer);
 	if (err) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next v2] net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE
  2026-02-02 10:52 ` Vladimir Oltean
@ 2026-02-03  1:40   ` David Yang
  2026-02-03  9:18     ` Vladimir Oltean
  0 siblings, 1 reply; 4+ messages in thread
From: David Yang @ 2026-02-03  1:40 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel

On Mon, Feb 2, 2026 at 6:52 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Sat, Jan 31, 2026 at 12:58:16PM +0800, David Yang wrote:
> > diff --git a/net/dsa/user.c b/net/dsa/user.c
> > index f59d66f0975d..2a209b83c701 100644
> > --- a/net/dsa/user.c
> > +++ b/net/dsa/user.c
> > @@ -1497,8 +1497,19 @@ dsa_user_add_cls_matchall_police(struct net_device *dev,
> >       mall_tc_entry->cookie = cls->cookie;
> >       mall_tc_entry->type = DSA_PORT_MALL_POLICER;
> >       policer = &mall_tc_entry->policer;
> > +     /* until they export the type of act->police in flow_offload.h ... */
>
> There is no "us" and "them", it's a single git tree. Just submit a patch
> to whomever ./scripts/get_maintainer.pl says to.
>
> Have you tried something like the patch attached (just compile-tested)?

Yes, but I was afraid it would be kind of inconsistent not exposing
other struct flow_action_*. Nicer and better approach, though.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next v2] net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE
  2026-02-03  1:40   ` David Yang
@ 2026-02-03  9:18     ` Vladimir Oltean
  0 siblings, 0 replies; 4+ messages in thread
From: Vladimir Oltean @ 2026-02-03  9:18 UTC (permalink / raw)
  To: David Yang
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel

On Tue, Feb 03, 2026 at 09:40:02AM +0800, David Yang wrote:
> On Mon, Feb 2, 2026 at 6:52 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > On Sat, Jan 31, 2026 at 12:58:16PM +0800, David Yang wrote:
> > > diff --git a/net/dsa/user.c b/net/dsa/user.c
> > > index f59d66f0975d..2a209b83c701 100644
> > > --- a/net/dsa/user.c
> > > +++ b/net/dsa/user.c
> > > @@ -1497,8 +1497,19 @@ dsa_user_add_cls_matchall_police(struct net_device *dev,
> > >       mall_tc_entry->cookie = cls->cookie;
> > >       mall_tc_entry->type = DSA_PORT_MALL_POLICER;
> > >       policer = &mall_tc_entry->policer;
> > > +     /* until they export the type of act->police in flow_offload.h ... */
> >
> > There is no "us" and "them", it's a single git tree. Just submit a patch
> > to whomever ./scripts/get_maintainer.pl says to.
> >
> > Have you tried something like the patch attached (just compile-tested)?
> 
> Yes, but I was afraid it would be kind of inconsistent not exposing
> other struct flow_action_*. Nicer and better approach, though.

Since it doesn't affect existing users of struct flow_action_entry,
I don't think it's that big a deal. You can take this patch and submit a
v3 with it if it helps.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-02-03  9:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-31  4:58 [PATCH net-next v2] net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE David Yang
2026-02-02 10:52 ` Vladimir Oltean
2026-02-03  1:40   ` David Yang
2026-02-03  9:18     ` Vladimir Oltean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox