diff options
author | Vladimir Oltean <olteanv@gmail.com> | 2020-01-04 02:37:09 +0200 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2020-01-05 15:13:12 -0800 |
commit | 0a51826c6e05c5b6cc423b376b81c311e9e485b0 (patch) | |
tree | da44bcc48f616410d04f10c11cce8fbf16a5bf58 /drivers/net/dsa/sja1105 | |
parent | 8bd17dc6da3d74980fd86912e18388ebf6a0083b (diff) | |
download | linux-0a51826c6e05c5b6cc423b376b81c311e9e485b0.tar.bz2 |
net: dsa: sja1105: Always send through management routes in slot 0
I finally found out how the 4 management route slots are supposed to
be used, but.. it's not worth it.
The description from the comment I've just deleted in this commit is
still true: when more than 1 management slot is active at the same time,
the switch will match frames incoming [from the CPU port] on the lowest
numbered management slot that matches the frame's DMAC.
My issue was that one was not supposed to statically assign each port a
slot. Yes, there are 4 slots and also 4 non-CPU ports, but that is a
mere coincidence.
Instead, the switch can be used like this: every management frame gets a
slot at the right of the most recently assigned slot:
Send mgmt frame 1 through S0: S0 x x x
Send mgmt frame 2 through S1: S0 S1 x x
Send mgmt frame 3 through S2: S0 S1 S2 x
Send mgmt frame 4 through S3: S0 S1 S2 S3
The difference compared to the old usage is that the transmission of
frames 1-4 doesn't need to wait until the completion of the management
route. It is safe to use a slot to the right of the most recently used
one, because by protocol nobody will program a slot to your left and
"steal" your route towards the correct egress port.
So there is a potential throughput benefit here.
But mgmt frame 5 has no more free slot to use, so it has to wait until
_all_ of S0, S1, S2, S3 are full, in order to use S0 again.
And that's actually exactly the problem: I was looking for something
that would bring more predictable transmission latency, but this is
exactly the opposite: 3 out of 4 frames would be transmitted quicker,
but the 4th would draw the short straw and have a worse worst-case
latency than before.
Useless.
Things are made even worse by PTP TX timestamping, which is something I
won't go deeply into here. Suffice to say that the fact there is a
driver-level lock on the SPI bus offsets any potential throughput gains
that parallelism might bring.
So there's no going back to the multi-slot scheme, remove the
"mgmt_slot" variable from sja1105_port and the dummy static assignment
made at probe time.
While passing by, also remove the assignment to casc_port altogether.
Don't pretend that we support cascaded setups.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net/dsa/sja1105')
-rw-r--r-- | drivers/net/dsa/sja1105/sja1105_main.c | 26 |
1 files changed, 1 insertions, 25 deletions
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c index 1da5ac111499..79dd965227bc 100644 --- a/drivers/net/dsa/sja1105/sja1105_main.c +++ b/drivers/net/dsa/sja1105/sja1105_main.c @@ -426,14 +426,6 @@ static int sja1105_init_general_params(struct sja1105_private *priv) .tpid2 = ETH_P_SJA1105, }; struct sja1105_table *table; - int i, k = 0; - - for (i = 0; i < SJA1105_NUM_PORTS; i++) { - if (dsa_is_dsa_port(priv->ds, i)) - default_general_params.casc_port = i; - else if (dsa_is_user_port(priv->ds, i)) - priv->ports[i].mgmt_slot = k++; - } table = &priv->static_config.tables[BLK_IDX_GENERAL_PARAMS]; @@ -1827,30 +1819,14 @@ static netdev_tx_t sja1105_port_deferred_xmit(struct dsa_switch *ds, int port, struct sk_buff *skb) { struct sja1105_private *priv = ds->priv; - struct sja1105_port *sp = &priv->ports[port]; - int slot = sp->mgmt_slot; struct sk_buff *clone; - /* The tragic fact about the switch having 4x2 slots for installing - * management routes is that all of them except one are actually - * useless. - * If 2 slots are simultaneously configured for two BPDUs sent to the - * same (multicast) DMAC but on different egress ports, the switch - * would confuse them and redirect first frame it receives on the CPU - * port towards the port configured on the numerically first slot - * (therefore wrong port), then second received frame on second slot - * (also wrong port). - * So for all practical purposes, there needs to be a lock that - * prevents that from happening. The slot used here is utterly useless - * (could have simply been 0 just as fine), but we are doing it - * nonetheless, in case a smarter idea ever comes up in the future. - */ mutex_lock(&priv->mgmt_lock); /* The clone, if there, was made by dsa_skb_tx_timestamp */ clone = DSA_SKB_CB(skb)->clone; - sja1105_mgmt_xmit(ds, port, slot, skb, !!clone); + sja1105_mgmt_xmit(ds, port, 0, skb, !!clone); if (!clone) goto out; |