Thread (10 messages) 10 messages, 2 authors, 2026-02-25

Re: [PATCH net-next 2/3] net: dsa: yt921x: Add port police support

From: Andrew Lunn <andrew@lunn.ch>
Date: 2026-02-25 14:17:58
Also in: lkml

+static void u32a_update_u64(u32 *arr, u64 mask, u64 val)
+{
+	arr[0] &= ~((u32)mask);
+	arr[1] &= ~((u32)(mask >> 32));
+	arr[0] |= (u32)val;
+	arr[1] |= (u32)(val >> 32);
+}
The name of this function is not helping me understand what it
actually does.
+/* v * 2^e */
+#define ldexpi(v, e) ({ \
+	__auto_type _v = (v); \
+	__auto_type _e = (e); \
+	_e >= 0 ? _v << _e : _v >> -_e; \
+})
+
+/* slot (ns) * rate (/s) / 10^9 (ns/s) = 2^C * token * 4^unit */
+#define rate2token(rate, unit, slot_ns, C) \
+	((u32)(ldexpi((slot_ns) * (rate), \
+		      -(2 * (unit) + (C) + YT921X_TOKEN_RATE_C)) / 1000000000))
+#define token2rate(token, unit, slot_ns, C) \
+	(ldexpi(1000000000 * (u64)(token), \
+		2 * (unit) + (C) + YT921X_TOKEN_RATE_C) / (slot_ns))
+
+/* burst = 2^C * token * 4^unit */
+#define burst2token(burst, unit, C) ((u32)ldexpi((burst), -(2 * (unit) + (C))))
+#define token2burst(token, unit, C) ldexpi((u64)(token), 2 * (unit) + (C))
Please avoid macros like this. Write functions. The compiler will
inline them if that makes sense. But you gain type checking, etc.
+static struct yt921x_meter
+yt921x_meter_tfm(struct yt921x_priv *priv, int port, unsigned int slot_ns,
+		 u64 rate, u64 burst, unsigned int flags,
+		 u32 cir_max, u32 cbs_max, int unit_max)
+{
+	const int C = flags & YT921X_METER_PKT_MODE ? YT921X_TOKEN_PKT_C :
+		      YT921X_TOKEN_BYTE_C;
+	struct device *dev = to_device(priv);
+	struct yt921x_meter meter;
+	bool distorted;
+	u64 burst_est;
+	u64 burst_max;
+	u64 rate_max;
+
+	meter.unit = unit_max;
+	rate_max = token2rate(cir_max, meter.unit, slot_ns, C);
+	burst_max = token2burst(cbs_max, meter.unit, C);
+
+	/* Clamp rate and burst */
+	if (rate > rate_max || burst > burst_max) {
+		dev_warn(dev,
+			 "rate %llu, burst %llu too high, max is %llu, %llu, clamping...\n",
+			 rate, burst, rate_max, burst_max);
I'm not sure clamping is the correct thing to do here. -ERANGE would
make it clearer that what the user requests is not possible.
+
+		if (rate > rate_max)
+			rate = rate_max;
+		if (burst > burst_max)
+			burst = burst_max;
+
+		burst_est = slot_ns * rate / 1000000000;
+	} else {
+		u64 burst_sug;
+
+		burst_est = slot_ns * rate / 1000000000;
+		burst_sug = burst_est;
+		if (flags & YT921X_METER_PKT_MODE)
+			burst_sug++;
+		else
+			burst_sug += ETH_HLEN + yt921x_mtu_fetch(priv, port) +
+				     ETH_FCS_LEN;
+		if (burst_sug > burst)
+			dev_warn(dev,
+				 "Consider burst at least %llu to match rate %llu\n",
+				 burst_sug, rate);
+
+		/* Select unit */
+		for (; meter.unit > 0; meter.unit--) {
+			if (rate > (rate_max >> 2) || burst > (burst_max >> 2))
+				break;
+			rate_max >>= 2;
+			burst_max >>= 2;
+		}
+	}
+
+	/* Calculate information rate and bucket size */
+	distorted = false;
+	meter.cir = rate2token(rate, meter.unit, slot_ns, C);
+	if (!meter.cir) {
+		distorted = true;
+		meter.cir = 1;
+	} else if (meter.cir > cir_max) {
+		WARN_ON(1);
+		meter.cir = cir_max;
What does this WARN_ON() indicate? If this fires, you want somebody
who is debugging it to be able to understand it.
+	}
+	meter.cbs = burst2token(burst, meter.unit, C);
+	if (!meter.cbs) {
+		distorted = true;
+		meter.cbs = 1;
+	} else if (meter.cbs > cbs_max) {
+		WARN_ON(1);
+		meter.cbs = cbs_max;
+	}
+	if (distorted)
+		dev_warn(dev,
+			 "Have to increase rate %llu, burst %llu to %llu, %llu\n",
+			 rate, burst,
+			 token2rate(meter.cir, meter.unit, slot_ns, C),
+			 token2burst(meter.cbs, meter.unit, C));
I see a difference between clamping, and the granularity the hardware
can do. Clamping should be an error. However, rounding to what the
hardware can actually do is expected, so i would not spam the logs
with it.
+static int
+yt921x_dsa_port_policer_add(struct dsa_switch *ds, int port,
+			    const struct flow_action_police *policer)
+{
+	struct yt921x_priv *priv = to_yt921x_priv(ds);
+	struct device *dev = to_device(priv);
+	u32 ctrls[YT921X_METER_CTRL_S];
+	struct yt921x_meter meter;
+	bool pkt_mode;
+	u64 burst;
+	u64 rate;
+	u32 ctrl;
+	int res;
+
+	/* mtu defaults to unlimited but we got 2040 here, don't know why */
+	if (policer->exceed.act_id != FLOW_ACTION_DROP ||
+	    policer->notexceed.act_id != FLOW_ACTION_ACCEPT ||
+	    policer->peakrate_bytes_ps || policer->avrate ||
+	    policer->overhead) {
+		dev_err(dev, "Unsupported options specified\n");
dev_dbg()

You might also want to extend .port_policer_add() to pass the extack
dsa_user_add_cls_matchall_police() has, so you can give a user
friendly error message.

	Andrew
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help