Re: [PATCH net-next v2] net: fec: support RX flushing via an ethtool private flag

[Sverdlin, Alexander]

Hi Jakub,

On Fri, 2026-09-04 at 18:20 -0700, Jakub Kicinski wrote:
> On Tue,  1 Sep 2026 09:47:29 +0200 A. Sverdlin wrote:
> > The FEC controller can flush the RX FIFO of an individual receive queue
> > through the RX_FLUSHn bits of the FEC_QOS_SCHEME (ENET_QOS) register.
> > 
> > RX flushing prevents frames in the RX FIFO from being blocked. Blocking
> > can occur when the frame at the head of the RX FIFO cannot be forwarded
> > because the ring it is associated with cannot accept it, i.e. when the
> > ring's RxBD[EMPTY] is not set or ENET_RDARn is not set. When RX flushing
> > is enabled for a ring, such a blocking frame is flushed (discarded)
> > instead of stalling the FIFO and holding up the other rings.
> > 
> > Expose this per-queue capability as a runtime ethtool private flag
> > ("rx-flush-qN").
> > 
> > Due to erratum ERR050395 (observed e.g. on i.MX8QXP), enabling RX flushing
> > on more than one queue at a time can lock up the receive path, so only a
> > single queue may have flushing enabled.
> > 
> > According to i.MX8QXP Reference Manual, "when both class 1 and class 2 are
> > disabled, RX flushing for these rings must also be disabled". Both RX and
> > TX QoS classes are enabled via DMA_CLASS_EN bit, currently tied to
> > num_tx_queues in the driver. Currently there are no constraints on
> > num_tx_queues == num_rx_queues, neither in DT nor in the code, even though
> > having different values doesn't make much sense even without RX flushing.
> > RX flushing code just takes min() of the both values for now.
> 
> For the errata - how is the user supposed to pick a queue?
> The documentation talks a lot about device specific terms instead of
> using generic easily understood terms. IOW it reads like slop.

Fair point on the wording, let me re-phrase:

The FEC has a single shared RX FIFO that feeds multiple DMA rings via
hardware classification. The FIFO is drained strictly in order, so if the
frame at its head is classified to a ring that currently cannot accept it
(the host has not replenished that ring's descriptors), that one frame
head-of-line-blocks the entire FIFO and starves all other rings,
including higher-priority ones. RX flush lets the MAC drop such a blocking
frame instead of stalling the whole port.

So this is not a generic "queue full -> drop its own packets" situation.
On a controller with independent per-queue FIFOs a backed-up ring only
drops its own traffic; there is no cross-queue impact. On FEC the shared
FIFO means a low-priority ring can stall a high-priority one, which is the
specific problem this knob addresses.

In practice a user would enable the flushing on the lower-priority
ring(s) — the ones whose backlog you are willing to discard to keep the
FIFO head clear for high-priority traffic. The queue->priority association
is hardcoded in the driver: VLAN PCP 0–1 (and unclassified) -> ring 0,
PCP 2–4 -> ring 1, PCP 5–7 -> ring 2.

There are actually two constraints stacked here, and I should have made
both explicit in the commit message and documentation:

- Why flush is needed at all: a low-priority ring can head-of-line-block
a high-priority ring through the shared FIFO.
- Erratum ERR050395: only one RX_FLUSH bit may be set at a time, so only
one ring can be protected this way. With three rings, the remaining
ring must be guaranteed not to stall by other means (limiting link rate,
for instance).

So because of the Errata we cannot set RX_FLUSH in queues 1 and 2, but
if PCP 5-7 bandwith is already limited in HW, we might want to set RX_FLUSH
not in ring 2, but in ring 1.

> In general AFAIU the flushing should be enabled in tandem with pause
> frames right now. It'd be great if we can stick to that, or failing
> that add a generic config knob rather than private flags.
> There's nothing private about the drop decision when host ring is empty

I don't think tying it to pause works on FEC, because pause-frame
generation is port-global. There is a single flow-control enable and a
single pause-quanta setting, driven by the shared RX FIFO thresholds.
There is no per-ring pause.

I fully agree there is nothing private about "drop when the host ring is
backed up". If we want a generic uAPI, the natural shape would be a
per-queue attribute (drop-on-backpressure / flush), not a pause-linked
one, since the decision is inherently per-ring.

Does the above justify the ethtool private flag in your opinion, with the
fix being a much better commit message and documentation or do you have
a new generic uAPI proposal in mind?

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com