Re: Fixing MBUF_FAST_FREE TX offload requirements?

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Oct 29, 2025 at 03:57:40PM +0100, Thomas Monjalon wrote:
> 29/10/2025 13:23, Morten Brørup:
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > On Wed, Oct 29, 2025 at 12:16:37PM +0300, Andrew Rybchenko wrote:
> > > > On 9/18/25 5:12 PM, Konstantin Ananyev wrote:
> > > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > > > > On Thu, Sep 18, 2025 at 10:50:11AM +0200, Morten Brørup wrote:
> > > > > > > > Dear NIC driver maintainers (CC: DPDK Tech Board),
> > > > > > > >
> > > > > > > > The DPDK Tech Board has discussed that patch [1] (included in
> > > DPDK
> > > > > > > 25.07) extended the documented requirements to the
> > > > > > > RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE offload.
> > > > > > > > These changes put additional limitations on applications' use
> > > of the
> > > > > > > MBUF_FAST_FREE TX offload, and made MBUF_FAST_FREE mutually
> > > exclusive
> > > > > > > with MULTI_SEGS (which is typically used for jumbo frame
> > > support).
> > > > > > > > The Tech Board discussed that these changes do not reflect
> > > the
> > > > > > > intention of the MBUF_FAST_FREE TX offload, and wants to fix
> > > it.
> > > > > > > > Mainly, MBUF_FAST_FREE and MULTI_SEGS should not be mutually
> > > > > > > exclusive.
> > > > > > > >
> > > > > > > > The original RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE requirements
> > > were:
> > > > > > > > When set, application must guarantee that
> > > > > > > > 1) per-queue all mbufs come from the same mempool, and
> > > > > > > > 2) mbufs have refcnt = 1.
> > > > > > > >
> > > > > > > > The patch added the following requirements to the
> > > MBUF_FAST_FREE
> > > > > > > offload, reflecting rte_pktmbuf_prefree_seg() postconditions:
> > > > > > > > 3) mbufs are direct,
> > > > > > > > 4) mbufs have next = NULL and nb_segs = 1.
> > > > > > > >
> > > > > > > > Now, the key question is:
> > > > > > > > Can we roll back to the original two requirements?
> > > > > > > > Or do the drivers also depend on the third and/or fourth
> > > > > > > requirements?
> > > > > > > >
> > > > > > > > <advertisement>
> > > > > > > > Drivers freeing mbufs directly to a mempool should use the
> > > new
> > > > > > > rte_mbuf_raw_free_bulk() instead of rte_mempool_put_bulk(), so
> > > the
> > > > > > > preconditions for freeing mbufs directly into a mempool are
> > > validated
> > > > > > > in mbuf debug mode (with RTE_LIBRTE_MBUF_DEBUG enabled).
> > > > > > > > Similarly, rte_mbuf_raw_alloc_bulk() should be used instead
> > > of
> > > > > > > rte_mempool_get_bulk().
> > > > > > > > </advertisement>
> > > > > > > >
> > > > > > > > PS: The feature documentation [2] still reflects the original
> > > > > > > requirements.
> > > > > > > >
> > > > > > > > [1]:
> > > > > > >
> > > > > >
> > > https://github.com/DPDK/dpdk/commit/55624173bacb2becaa67793b7139188487
> > > > > > 6
> > > > > > > 673c1
> > > > > > > > [2]:
> > > > > > >
> > > https://elixir.bootlin.com/dpdk/v25.07/source/doc/guides/nics/features.
> > > > > > > rst#L125
> > > > > > > >
> > > > > > > >
> > > > > > > > Venlig hilsen / Kind regards,
> > > > > > > > -Morten Brørup
> > > > > > > >
> > > > > > > I'm a little torn on this question, because I can see benefits
> > > for both
> > > > > > > approaches. Firstly, it would be nice if we made FAST_FREE as
> > > > > > > accessible
> > > > > > > for driver use as it was originally, with minimal requirements.
> > > > > > > However, on
> > > > > > > looking at the code, I believe that many drivers actually took
> > > it to
> > > > > > > mean
> > > > > > > that scattered packets couldn't occur in that case either, so
> > > the use
> > > > > > > was
> > > > > > > incorrect.
> > > > > >
> > > > > > I primarily look at Intel drivers, and that's how I read the
> > > driver code too.
> > > > > >
> > > > > > > Similarly, and secondly, if we do have the extra
> > > > > > > requirements
> > > > > > > for FAST_FREE, it does mean that any use of it can be very,
> > > very
> > > > > > > minimal
> > > > > > > and efficient, since we don't need to check anything before
> > > freeing the
> > > > > > > buffers.
> > > > > > >
> > > > > > > Given where we are now, I think keeping the more restrictive
> > > definition
> > > > > > > of
> > > > > > > FAST_FREE is the way to go - keeping it exclusive with
> > > MULTI_SEGS -
> > > > > > > because
> > > > > > > it means that we are less likely to have bugs. If we look to
> > > change it
> > > > > > > back, I think we'd have to check all drivers to ensure they are
> > > using
> > > > > > > the
> > > > > > > flag safely.
> > > > > >
> > > > > > However, those driver bugs are not new.
> > > > > > If we haven't received bug reports from users affected by them,
> > > maybe we can
> > > > > > disregard them (in this discussion about pros and cons).
> > > > > > I prefer we register them as driver bugs, instead of changing the
> > > API to
> > > > > > accommodate bugs in the drivers.
> > > > > >
> > > > > >  From an application perspective, here's an idea for
> > > consideration:
> > > > > > Assuming that indirect mbufs are uncommon, we keep requirement
> > > #3.
> > > > > > To allow MULTI_SEGS (jumbo frames) with FAST_FREE, we get rid of
> > > requirement
> > > > > > #4.
> > > > >
> > > > > Do we really need to enable FAST_FREE for jumbo-frames?
> > > > > Jumbo-frames usually means much smaller PPS number and actual RX/TX
> > > overhead
> > > > > becomes really tiny.
> > > >
> > > > +1
> > > > > > Since the driver knows that refcnt == 1, the driver can set next
> > > = NULL and
> > > > > > nb_segs = 1 at any time, either when writing the TX descriptor
> > > (when it reads the
> > > > > > mbuf anyway), or when freeing the mbuf.
> > > > > > Regarding performance, this means that the driver's TX code path
> > > has to write to
> > > > > > the mbufs (i.e. adding the performance cost of memory store
> > > operations) when
> > > > > > segmented - but that is a universal requirement when freeing
> > > segmented mbufs
> > > > > > to the mempool.
> > > > >
> > > > > It might work, but I think it will become way too complicated.
> > > > > Again I don't know who is going to inspect/fix all the drivers.
> > > > > Just not allowing FAST_FREE for mulsti-seg seems like a much more
> > > simpler and safer approach.
> > > > > > For even more optimized driver performance, as Bruce mentions...
> > > > > > If a port is configured for FAST_FREE and not MULTI_SEGS, the
> > > driver can use a
> > > > > > super lean transmit function.
> > > > > > Since the driver's transmit function pointer is per port (not per
> > > queue), this would
> > > > > > require the driver to provide the MULTI_SEGS capability only per
> > > port, and not
> > > > > > per queue. (Or we would have to add a NOT_MULTI_SEGS offload
> > > flag, to ensure
> > > > > > that no queue is configured for MULTI_SEGS.)
> > > >
> > > >
> > > > FAST_FREE is not a real Tx offload, since there is no promise from
> > > > driver to do something (like other Tx offloads, e.g. checksumming or
> > > > TSO). Is it a promise to ignore refcount or take a look at memory
> > > pool
> > > > of some packets only? I guess no. If so, basically any driver may
> > > > advertise it and simply ignore if the offload is requested, but
> > > > driver can do nothing with these limitations on input data.
> > > >
> > > > It is a performance hint in fact and promise from application to
> > > > follow specified limitations on Tx mbufs.
> > > >
> > > > So, if application specifies both FAST_FREE and MULTI_SEG, but driver
> > > > code can't FAST_FREE with MULTI_SEG, it should just ignore FAST_FREE.
> > > > That's it. The performance hint is simply useless in this case.
> > > > There is no point to make FAST_FREE and MULTI_SEG mutual exclusive.
> > > > If some drivers can really support both - great. If no, just ignore
> > > > FAST_FREE and support MULTI_SEG.
> > > >
> > > > "mbufs are direct" must be FAST_FREE requirement. Since otherwise
> > > > freeing is not simple. I guess is was simply lost in the original
> > > > definition of FAST_FREE.
> > 
> > Agree about the "mbufs are direct" statement being lost in the original definition.
> > It can be extended to include mbufs using "pinned external buffer with refcnt==1", because freeing those is just as simple as freeing "direct" mbufs.
> > 
> > > >
> > > That's a good point and expanation of things. Perhaps we are better to
> > > deprecate FAST_FREE and replace it with a couple of explicit hints that
> > > better explain what they are?
> > > 
> > > - RTE_ETH_TX_HINT_DIRECT_MBUFS
> > 
> > In the FAST_FREE case, this hint would be TX_HINT_MBUF_DIRECT_OR_SINGLE_OWNER_PINNED_EXTBUF.
> > 
> > > - RTE_ETH_TX_HINT_SINGLE_MEMPOOL
> > 
> > Prefer TX_HINT_SINGLE_MEMPOOL -> TX_HINT_SAME_MEMPOOL, so we can add a globally scoped TX_HINT_SINGLE_MEMPOOL later.
> > 
> > Also, RTE_ETH_TX_HINT_NON_SEGMENTED can be added later.
> > 
> > I strongly agree with the finer granularity for the hints; the optimization of freeing to one mempool instead of a variety of mempools is orthogonal to the optimization of not having to consider indirect mbufs.
> > And the drivers are free to only optimize if multiple hints are present; so there is no downside to using a finer granularity for hints.
> 
> Yes we can have finer granularity.
> 
> 
> > Although we are reusing "offload" fields for hints, there's no need for drivers to announce capability for such hints, including FAST_FREE; since the drivers can freely ignore any hints, hint capability doesn't contain any information about the driver's ability to do anything useful with the hints.
> 
> Capability does not need to be announced,
> but it would be useful to have debug logs when an optimization is enabled.
> I'm not sure how we can enforce such logs in drivers.
> 
> 
> > Regarding naming, we should use "promise" instead of "hint",
> > to emphasize that the "hint" is not allowed to be violated.
> 
> I'm not sure why but I'm not confortable with the word "promise".
> To me, a "hint" is already something strong.
> 

Agree. Also, promise is too long a word. Hint is shorter.