Re: [PATCH v2 net-next] net: dsa: felix: enable cut-through forwarding between ports by default
From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-11-25 02:40:29
On Tue, 23 Nov 2021 15:21:16 +0200 Vladimir Oltean wrote:
+static void vsc9959_cut_through_fwd(struct ocelot *ocelot)
+{
+ struct felix *felix = ocelot_to_felix(ocelot);
+ struct dsa_switch *ds = felix->ds;
+ int port, other_port;
+
+ for (port = 0; port < ocelot->num_phys_ports; port++) {
+ struct ocelot_port *ocelot_port = ocelot->ports[port];
+ unsigned long mask;
+ int min_speed;
+ u32 val = 0;
+
+ if (ocelot_port->speed <= 0)
+ continue;Would it not be safer to disable cut-thru for ports which are down? goto set;
+ min_speed = ocelot_port->speed;
+ if (port == ocelot->npi) {
+ /* Ocelot switches forward from the NPI port towards
+ * any port, regardless of it being in the NPI port's
+ * forwarding domain or not.
+ */
+ mask = dsa_user_ports(ds);
+ } else {
+ mask = ocelot_read_rix(ocelot, ANA_PGID_PGID,
+ PGID_SRC + port);
+ /* Ocelot switches forward to the NPI port despite it
+ * not being in the source ports' forwarding domain.
+ */
+ if (ocelot->npi >= 0)
+ mask |= BIT(ocelot->npi);
+ }
+
+ for_each_set_bit(other_port, &mask, ocelot->num_phys_ports) {
+ struct ocelot_port *other_ocelot_port;
+
+ other_ocelot_port = ocelot->ports[other_port];
+ if (other_ocelot_port->speed <= 0)
+ continue;
+
+ if (min_speed > other_ocelot_port->speed)
+ min_speed = other_ocelot_port->speed;break; ?
+ } + + /* Enable cut-through forwarding for all traffic classes. */ + if (ocelot_port->speed == min_speed)
Any particular reason this is not <= ?
+ val = GENMASK(7, 0);
set: ?
+ ocelot_write_rix(ocelot, val, ANA_CUT_THRU_CFG, port); + } +}
quoted hunk ↗ jump to hunk
static const struct felix_info felix_info_vsc9959 = {diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index 95920668feb0..30c790f401be 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c
quoted hunk ↗ jump to hunk
@@ -697,6 +702,8 @@ void ocelot_phylink_mac_link_up(struct ocelot *ocelot, int port, int mac_speed, mode = 0; u32 mac_fc_cfg; + ocelot_port->speed = speed; + /* The MAC might be integrated in systems where the MAC speed is fixed * and it's the PCS who is performing the rate adaptation, so we have * to write "1000Mbps" into the LINK_SPEED field of DEV_CLOCK_CFG@@ -772,6 +779,9 @@ void ocelot_phylink_mac_link_up(struct ocelot *ocelot, int port, /* 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?
quoted hunk ↗ jump to hunk
+ 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?
+ if (ocelot->ops->cut_through_fwd) + ocelot->ops->cut_through_fwd(ocelot); } EXPORT_SYMBOL(ocelot_apply_bridge_fwd_mask);