Re: [PATCH v2 net-next] net: dsa: felix: enable cut-through forwarding between ports by default

On Thu, 25 Nov 2021 12:16:52 +0200 Vladimir Oltean wrote:

quoted

quoted

+			if (min_speed > other_ocelot_port->speed)
+				min_speed = other_ocelot_port->speed;  

break; ?  

Break where and why?
Breaking in the "if" block means "stop at the first @other_port in
@port's forwarding domain which has a lower speed than @port". But that
isn't necessarily the minimum...
And breaking below the "if" block means stopping at the first
@other_port in @port's forwarding domain, which doesn't make sense.
This is the simple calculation of the minimum value of an array, no
special sauce here.

A single slower port is enough to disable cut through, this is can be
read as a proof of nonexistence rather than min calculation. But really
just a nit pick, don't think any bot will bother us about it.

quoted

quoted

 	/* Core: Enable port for frame transfer */
 	ocelot_fields_write(ocelot, port,
 			    QSYS_SWITCH_PORT_MODE_PORT_ENA, 1);  

Does this enable forwarding? Is there a window here with forwarding
enabled and old cut-thru masks if we don't clear cut-thru when port
goes down?  

Correct, I should be updating the cut-through masks before this, thanks.

quoted

quoted

+	if (ocelot->ops->cut_through_fwd)
+		ocelot->ops->cut_through_fwd(ocelot);
 }
 EXPORT_SYMBOL_GPL(ocelot_phylink_mac_link_up);
 

@@ -1637,6 +1647,9 @@ void ocelot_apply_bridge_fwd_mask(struct ocelot *ocelot)
 
 		ocelot_write_rix(ocelot, mask, ANA_PGID_PGID, PGID_SRC + port);
 	}  

Obviously shooting from the hip here, but I was expecting the cut-thru
update to be before the bridge reconfig if port is joining, and after
if port is leaving. Do you know what I'm getting at?  

Yes, I know what you're getting at. But it's a bit complicated to do,
given the layering constraints and that cut-through forwarding is an
optional feature which isn't present on all devices, so I am trying to
keep its footprint minimal on the ocelot library.

What I can do is I can disable cut-through forwarding for ports that are
standalone (not in a bridge). I don't have a use case for that anyway:
the store-and-forward latency is indistinguishable from network stack
latency. This will guarantee that when a port joins a bridge, it has
cut-through forwarding disabled. So there are no issues if it happens to
join a bridge and its link speed is higher than anybody else: there will
be no packet underruns.

Hm, to make sure I understand - fixing standalone ports doesn't
necessary address the issue of a slow standalone port joining, right?