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

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 7 Sep 2026 15:47:07 +0000 Sverdlin, Alexander wrote:
> 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.

Sound pretty typical for a NIC so far..

> 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.

Global pause means no drops for all rings, PFC/DCB can be more
selective with certain queues and traffic classes being "lossless".

> 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?

The problem is fairly generic. Ideally you would find something
matching in PFC/DCB APIs or even devlink shared-buffer config. 
Failing that we can add something to netlink APIs at queue level. 
But since you seem to have a strong association between the rings 
and traffic classes AFAIU, the PFC/devlink direction would probably 
be better than ring level.