[PATCH net-next 0/3] net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE

[PATCH net-next 0/3] net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE

[David Yang]

Fix tc command which should not succeed:

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

David Yang (3):
  net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE
  net: dsa: add dsa_mall_policer_tc_entry_type() helper
  net: dsa: ocelot: check policer entry

 drivers/net/dsa/ocelot/felix.c |  4 ++++
 include/net/dsa.h              | 25 +++++++++++++++++++++++++
 net/dsa/dsa.c                  | 33 +++++++++++++++++++++++++++++++++
 net/dsa/user.c                 |  7 +++++--
 4 files changed, 67 insertions(+), 2 deletions(-)

-- 
2.51.0

[PATCH net-next 1/3] net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE

[David Yang]

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.

Instead of making decisions for drivers, extend struct
dsa_mall_policer_tc_entry to all options for FLOW_ACTION_POLICE in favor
of full functionalities, such as packet rate mode.

Drivers must reject unsupported combinations.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 include/net/dsa.h | 11 +++++++++++
 net/dsa/user.c    |  7 +++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

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..4c9cff629d5c 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1497,8 +1497,11 @@ 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;
+	/* so sad, flow_offload.h did not export the type of act->police, and
+	 * it's a nightmare to copy it field by field
+	 */
+	static_assert(sizeof(act->police) == sizeof(*policer));
+	memcpy(policer, &act->police, sizeof(*policer));
 
 	err = ds->ops->port_policer_add(ds, dp->index, policer);
 	if (err) {
-- 
2.51.0

Re: [PATCH net-next 1/3] net: dsa: sync dsa_mall_policer_tc_entry with FLOW_ACTION_POLICE

[Jakub Kicinski]

On Mon, 26 Jan 2026 14:13:29 +0800 David Yang wrote:
> -	policer->rate_bytes_per_sec = act->police.rate_bytes_ps;
> -	policer->burst = act->police.burst;
> +	/* so sad, flow_offload.h did not export the type of act->police, and
> +	 * it's a nightmare to copy it field by field
> +	 */
> +	static_assert(sizeof(act->police) == sizeof(*policer));
> +	memcpy(policer, &act->police, sizeof(*policer));

This is of course an unacceptable hack. Nobody will realize that
there's a layout dependency here, not to mention struct randomization.
If someone adds fields presumably they will have to update
dsa_mall_policer_tc_entry_type() anyway, so we aren't saving much with this hack

[PATCH net-next 2/3] net: dsa: add dsa_mall_policer_tc_entry_type() helper

[David Yang]

It has shown FLOW_ACTION_POLICE is not stable, and it's very likely that
options were added at drivers' request.

However, that also means .port_policer_add() would have to check the
parameters themselves. Giving that struct dsa_mall_policer_tc_entry is
not stable, adjusting the interface between the framework and drivers
would be a pain.

Introduce a helper to maximize forward compatibility. Drivers may use it
to recognize the pattern of tc entry reliably, and ensure all
newly-added options are set to 0 / their default values and do not break
the semantics.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 include/net/dsa.h | 14 ++++++++++++++
 net/dsa/dsa.c     | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 4c177b168ec8..957439846fe5 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1410,4 +1410,18 @@ netdev_tx_t dsa_enqueue_skb(struct sk_buff *skb, struct net_device *dev);
 void dsa_port_phylink_mac_change(struct dsa_switch *ds, int port, bool up);
 bool dsa_supports_eee(struct dsa_switch *ds, int port);
 
+enum dsa_mall_policer_tc_type {
+	/* Pattern recognized, no surprising fields exist */
+	DSA_MALL_POLICER_TC_KNOWN = BIT(0),
+	/* Unset: .burst and .rate_bytes_per_sec valid
+	 * Set: .burst_pkt and .rate_pkt_ps valid
+	 */
+	DSA_MALL_POLICER_TC_PKT_MODE = BIT(1),
+	/* .mtu valid */
+	DSA_MALL_POLICER_TC_MTU = BIT(2),
+};
+
+unsigned long
+dsa_mall_policer_tc_entry_type(struct dsa_mall_policer_tc_entry *entry);
+
 #endif
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 35ce3941fae3..c8102cea5ab3 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -1834,6 +1834,39 @@ int dsa_port_simple_hsr_leave(struct dsa_switch *ds, int port,
 }
 EXPORT_SYMBOL_GPL(dsa_port_simple_hsr_leave);
 
+/* dsa_mall_policer_tc_entry_type - map tc_entry to some "known" types
+ * @entry: the tc entry
+ *
+ * A helper to check dsa_mall_policer_tc_entry against some known patterns,
+ * without having to know the exact struct layout.
+ *
+ * Returns: ORs of enum dsa_mall_policer_tc_type with DSA_MALL_POLICER_TC_KNOWN
+ * set if recognized, otherwise 0
+ */
+unsigned long
+dsa_mall_policer_tc_entry_type(struct dsa_mall_policer_tc_entry *entry)
+{
+	bool byte_mode = (entry->burst || entry->rate_bytes_per_sec);
+	bool pkt_mode = (entry->burst_pkt || entry->rate_pkt_ps);
+	unsigned long flags = DSA_MALL_POLICER_TC_KNOWN;
+
+	if (byte_mode == pkt_mode)
+		return 0;
+	if (entry->peakrate_bytes_ps || entry->avrate || entry->overhead)
+		return 0;
+	if (entry->exceed.act_id != FLOW_ACTION_DROP ||
+	    entry->notexceed.act_id != FLOW_ACTION_ACCEPT)
+		return 0;
+
+	if (pkt_mode)
+		flags |= DSA_MALL_POLICER_TC_PKT_MODE;
+	if (entry->mtu)
+		flags |= DSA_MALL_POLICER_TC_MTU;
+
+	return flags;
+}
+EXPORT_SYMBOL_GPL(dsa_mall_policer_tc_entry_type);
+
 static const struct dsa_stubs __dsa_stubs = {
 	.conduit_hwtstamp_validate = __dsa_conduit_hwtstamp_validate,
 };
-- 
2.51.0

[PATCH net-next 3/3] net: dsa: ocelot: check policer entry

[David Yang]

rate_bytes_per_sec might be 0, check for it.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/ocelot/felix.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 9e5ede932b42..fb2d02ff0fe7 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -2011,6 +2011,10 @@ static int felix_port_policer_add(struct dsa_switch *ds, int port,
 		.burst = policer->burst,
 	};
 
+	if (dsa_mall_policer_tc_entry_type(policer) !=
+	    DSA_MALL_POLICER_TC_KNOWN)
+		return -EOPNOTSUPP;
+
 	return ocelot_port_policer_add(ocelot, port, &pol);
 }
 
-- 
2.51.0

Re: [PATCH net-next 3/3] net: dsa: ocelot: check policer entry

[Paolo Abeni]

On 1/26/26 7:13 AM, David Yang wrote:
> rate_bytes_per_sec might be 0, check for it.
> 
> Signed-off-by: David Yang <mmyangfl@gmail.com>
> ---
>  drivers/net/dsa/ocelot/felix.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index 9e5ede932b42..fb2d02ff0fe7 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -2011,6 +2011,10 @@ static int felix_port_policer_add(struct dsa_switch *ds, int port,
>  		.burst = policer->burst,
>  	};
>  
> +	if (dsa_mall_policer_tc_entry_type(policer) !=
> +	    DSA_MALL_POLICER_TC_KNOWN)
> +		return -EOPNOTSUPP;

I'm not sure this strict mode is what we want or even we can apply it
now: it could cause functional regression with setup previously
completing successfully in presence of 'new' params and working to some
degree and now failing to start.

Generally speaking some/most offloads are free to ignore
tunable/parameter not available in the H/W.

I think a way to opt-in the new behavior could help.

/P