mirror of
https://github.com/torvalds/linux.git
synced 2024-11-18 01:51:53 +00:00
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>
This commit is contained in:
parent
8bd17dc6da
commit
0a51826c6e
@ -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;
|
||||
|
@ -56,7 +56,6 @@ struct sja1105_port {
|
||||
struct sja1105_tagger_data *data;
|
||||
struct dsa_port *dp;
|
||||
bool hwts_tx_en;
|
||||
int mgmt_slot;
|
||||
};
|
||||
|
||||
#endif /* _NET_DSA_SJA1105_H */
|
||||
|
Loading…
Reference in New Issue
Block a user