[PATCH v2 net-next 0/4] Prevent permanently closed tc-taprio gates from blocking a Felix DSA switch port

Vladimir Oltean posted 4 patches 3 years, 9 months ago
drivers/net/dsa/ocelot/felix.c         |   9 +
drivers/net/dsa/ocelot/felix.h         |   1 +
drivers/net/dsa/ocelot/felix_vsc9959.c | 242 ++++++++++++++++++++++---
include/linux/time64.h                 |   3 +
include/soc/mscc/ocelot.h              |   5 +-
net/sched/sch_taprio.c                 |   5 +-
6 files changed, 238 insertions(+), 27 deletions(-)
[PATCH v2 net-next 0/4] Prevent permanently closed tc-taprio gates from blocking a Felix DSA switch port
Posted by Vladimir Oltean 3 years, 9 months ago
v1->v2:
- define PSEC_PER_NSEC in include/linux/time64.h rather than
  include/vdso/time64.h
- add missing #include <linux/time.h> to users of PSEC_PER_NSEC
- move the PSEC_PER_NSEC consolidation to the last patch

Richie Pearn reports that if we install a tc-taprio schedule on a Felix
switch port, and that schedule has at least one gate that never opens
(for example TC0 below):

tc qdisc add dev swp1 root 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 fe 1000000 flags 0x2

then packets classified to the permanently closed traffic class will not
be dequeued by the egress port. They will just remain in the queue
system, to consume resources. Frame aging does not trigger either,
because in order for that to happen, the packets need to be eligible for
egress scheduling in the first place, which they aren't. If that port is
allowed to consume the entire shared buffer of the switch (as we
configure things by default using devlink-sb), then eventually, by
sending enough packets, the entire switch will hang.

If we think enough about the problem, we realize that this is only a
special case of a more general issue, and can also be reproduced with
gates that aren't permanently closed, but are not large enough to send
an entire frame. In that sense, a permanently closed gate is simply a
case where all frames are oversized.

The ENETC has logic to reject transmitted packets that would overrun the
time window - see commit 285e8dedb4bd ("net: enetc: count the tc-taprio
window drops").

The Felix switch has no such thing on a per-packet basis, but it has a
register replicated per {egress port, TC} which essentially limits the
max MTU. A packet which exceeds the per-port-TC MTU is immediately
discarded and therefore will not hang the port anymore (albeit, sadly,
this only bumps a generic drop hardware counter and we cannot really
infer the reason such as to offer a dedicated counter for these events).

This patch set calculates the max MTU per {port, TC} when the tc-taprio
config, or link speed, or port-global MTU values change. This solves the
larger "gate too small for packet" problem, but also the original issue
with the gate permanently closed that was reported by Richie.

Q: Bug fix patch sent to net-next?
A: Yeah, after Xiaoliang started sending bug fixes to net-next himself
   (see https://patchwork.kernel.org/project/netdevbpf/patch/20220617032423.13852-1-xiaoliang.yang_1@nxp.com/)
   there is absolutely no gain in targeting "net" here - I am modifying
   the same areas of code, that have already diverged from "net", 5.19
   and earlier. So this is why I am also taking the opportunity to
   introduce cleanup patches, to leave things as clean as possible after
   the rework. I'd be interested if there is a better approach to this.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>

Vladimir Oltean (4):
  net: dsa: felix: keep reference on entire tc-taprio config
  net: dsa: felix: keep QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF) out of rmw
  net: dsa: felix: drop oversized frames with tc-taprio instead of
    hanging the port
  time64.h: consolidate uses of PSEC_PER_NSEC

 drivers/net/dsa/ocelot/felix.c         |   9 +
 drivers/net/dsa/ocelot/felix.h         |   1 +
 drivers/net/dsa/ocelot/felix_vsc9959.c | 242 ++++++++++++++++++++++---
 include/linux/time64.h                 |   3 +
 include/soc/mscc/ocelot.h              |   5 +-
 net/sched/sch_taprio.c                 |   5 +-
 6 files changed, 238 insertions(+), 27 deletions(-)

-- 
2.25.1
Re: [PATCH v2 net-next 0/4] Prevent permanently closed tc-taprio gates from blocking a Felix DSA switch port
Posted by patchwork-bot+netdevbpf@kernel.org 3 years, 9 months ago
Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 28 Jun 2022 17:52:34 +0300 you wrote:
> v1->v2:
> - define PSEC_PER_NSEC in include/linux/time64.h rather than
>   include/vdso/time64.h
> - add missing #include <linux/time.h> to users of PSEC_PER_NSEC
> - move the PSEC_PER_NSEC consolidation to the last patch
> 
> Richie Pearn reports that if we install a tc-taprio schedule on a Felix
> switch port, and that schedule has at least one gate that never opens
> (for example TC0 below):
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/4] net: dsa: felix: keep reference on entire tc-taprio config
    https://git.kernel.org/netdev/net-next/c/1c9017e44af2
  - [v2,net-next,2/4] net: dsa: felix: keep QSYS_TAG_CONFIG_INIT_GATE_STATE(0xFF) out of rmw
    https://git.kernel.org/netdev/net-next/c/d68a373bfbf4
  - [v2,net-next,3/4] net: dsa: felix: drop oversized frames with tc-taprio instead of hanging the port
    https://git.kernel.org/netdev/net-next/c/55a515b1f5a9
  - [v2,net-next,4/4] time64.h: consolidate uses of PSEC_PER_NSEC
    https://git.kernel.org/netdev/net-next/c/837ced3a1a5d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html