It has been reported that preemptible traffic doesn't completely behave
as expected. Namely, large packets should be able to be squeezed
(through fragmentation) through taprio time slots smaller than the
transmission time of the full frame. That does not happen due to logic
in the driver (for oversize frame dropping with taprio) that was not
updated in order for this use case to work.
I am not sure whether it qualifies as "net" material, because some
structural changes are involved, and it is a "never worked" scenario.
OTOH, this is a complaint coming from users for a v6.4 kernel.
It's up to maintainers to decide whether this series can be considered;
I've submitted it as non-RFC in the optimistic case that it will be :)
Demo script illustrating the issue below.
#!/bin/bash
add_taprio()
{
local ifname=$1
echo "Creating root taprio"
tc qdisc replace dev $ifname handle 8001: parent root stab overhead 24 taprio \
num_tc 8 \
map 0 1 2 3 4 5 6 7 \
queues 1@0 1@1 1@2 1@3 1@4 1@5 1@6 1@7 \
base-time 0 \
sched-entry S 01 1216 \
sched-entry S fe 12368 \
fp P E E E E E E E \
flags 0x2
}
remove_taprio()
{
local ifname=$1
echo "Removing taprio"
tc qdisc del dev $ifname root
}
ip netns add ns0
ip link set eno0 netns ns0 && ip -n ns0 link set eno0 up && ip -n ns0 addr add 192.168.100.1/24 dev eno0
ip addr add 192.168.100.2/24 dev swp0 && ip link set swp0 up
ip netns exec ns0 ethtool --set-mm eno0 pmac-enabled on verify-enabled off tx-enabled on
ethtool --set-mm swp0 pmac-enabled on verify-enabled off tx-enabled on
add_taprio swp0
ping 192.168.100.1 -s 1000 -c 5 # sent through TC0
ethtool -I --show-mm swp0 | grep MACMergeFragCountTx # should increase
ip addr flush swp0 && ip link set swp0 down
remove_taprio swp0
ethtool --set-mm swp0 pmac-enabled off verify-enabled off tx-enabled off
ip netns exec ns0 ethtool --set-mm eno0 pmac-enabled off verify-enabled off tx-enabled off
ip netns del ns0
Vladimir Oltean (3):
net: mscc: ocelot: extend ocelot->fwd_domain_lock to cover
ocelot->tas_lock
net: dsa: felix: make vsc9959_tas_guard_bands_update() visible to
ocelot->ops
net: mscc: ocelot: fix oversize frame dropping for preemptible TCs
drivers/net/dsa/ocelot/felix.c | 9 ++--
drivers/net/dsa/ocelot/felix.h | 1 -
drivers/net/dsa/ocelot/felix_vsc9959.c | 59 +++++++++++++++++---------
drivers/net/ethernet/mscc/ocelot.c | 1 -
drivers/net/ethernet/mscc/ocelot_mm.c | 14 +++---
include/soc/mscc/ocelot.h | 9 ++--
6 files changed, 56 insertions(+), 37 deletions(-)
--
2.34.1
In a future change we will need to make
ocelot_port_update_active_preemptible_tcs() call
vsc9959_tas_guard_bands_update(), but that is currently not possible,
since the ocelot switch lib does not have access to functions private to
the DSA wrapper.
Move the pointer to vsc9959_tas_guard_bands_update() from felix->info
(which is private to the DSA driver) to ocelot->ops (which is also
visible to the ocelot switch lib).
Signed-off-by: Vladimir Oltean <[email protected]>
---
drivers/net/dsa/ocelot/felix.c | 5 ++---
drivers/net/dsa/ocelot/felix.h | 1 -
drivers/net/dsa/ocelot/felix_vsc9959.c | 2 +-
include/soc/mscc/ocelot.h | 1 +
4 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 0c1207613aa4..dee43caee19e 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1786,14 +1786,13 @@ static int felix_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
{
struct ocelot *ocelot = ds->priv;
struct ocelot_port *ocelot_port = ocelot->ports[port];
- struct felix *felix = ocelot_to_felix(ocelot);
ocelot_port_set_maxlen(ocelot, port, new_mtu);
mutex_lock(&ocelot->fwd_domain_lock);
- if (ocelot_port->taprio && felix->info->tas_guard_bands_update)
- felix->info->tas_guard_bands_update(ocelot, port);
+ if (ocelot_port->taprio && ocelot->ops->tas_guard_bands_update)
+ ocelot->ops->tas_guard_bands_update(ocelot, port);
mutex_unlock(&ocelot->fwd_domain_lock);
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index 96008c046da5..1d4befe7cfe8 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -57,7 +57,6 @@ struct felix_info {
void (*mdio_bus_free)(struct ocelot *ocelot);
int (*port_setup_tc)(struct dsa_switch *ds, int port,
enum tc_setup_type type, void *type_data);
- void (*tas_guard_bands_update)(struct ocelot *ocelot, int port);
void (*port_sched_speed_set)(struct ocelot *ocelot, int port,
u32 speed);
void (*phylink_mac_config)(struct ocelot *ocelot, int port,
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 56b8bcac9690..d7caadd13f83 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -2599,6 +2599,7 @@ static const struct ocelot_ops vsc9959_ops = {
.cut_through_fwd = vsc9959_cut_through_fwd,
.tas_clock_adjust = vsc9959_tas_clock_adjust,
.update_stats = vsc9959_update_stats,
+ .tas_guard_bands_update = vsc9959_tas_guard_bands_update,
};
static const struct felix_info felix_info_vsc9959 = {
@@ -2624,7 +2625,6 @@ static const struct felix_info felix_info_vsc9959 = {
.port_modes = vsc9959_port_modes,
.port_setup_tc = vsc9959_port_setup_tc,
.port_sched_speed_set = vsc9959_sched_speed_set,
- .tas_guard_bands_update = vsc9959_tas_guard_bands_update,
};
/* The INTB interrupt is shared between for PTP TX timestamp availability
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index eb5f8914a66c..a8c2817335b9 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -663,6 +663,7 @@ struct ocelot_ops {
struct flow_stats *stats);
void (*cut_through_fwd)(struct ocelot *ocelot);
void (*tas_clock_adjust)(struct ocelot *ocelot);
+ void (*tas_guard_bands_update)(struct ocelot *ocelot, int port);
void (*update_stats)(struct ocelot *ocelot);
};
--
2.34.1
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
On Wed, 5 Jul 2023 13:44:19 +0300 you wrote:
> It has been reported that preemptible traffic doesn't completely behave
> as expected. Namely, large packets should be able to be squeezed
> (through fragmentation) through taprio time slots smaller than the
> transmission time of the full frame. That does not happen due to logic
> in the driver (for oversize frame dropping with taprio) that was not
> updated in order for this use case to work.
>
> [...]
Here is the summary with links:
- [net,1/3] net: mscc: ocelot: extend ocelot->fwd_domain_lock to cover ocelot->tas_lock
https://git.kernel.org/netdev/net/c/009d30f1a777
- [net,2/3] net: dsa: felix: make vsc9959_tas_guard_bands_update() visible to ocelot->ops
https://git.kernel.org/netdev/net/c/c60819149b63
- [net,3/3] net: mscc: ocelot: fix oversize frame dropping for preemptible TCs
https://git.kernel.org/netdev/net/c/c6efb4ae387c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html