> From: fengchengwen [mailto:fengcheng...@huawei.com] > Sent: Tuesday, 5 August 2025 04.10 > > On 8/4/2025 3:42 AM, Morten Brørup wrote: > > When an ethdev port is configured for fast mbuf release, the driver can > > use a TX burst function relying on the fast mbuf release preconditions. > > Thus, also configuring this port or a queue on the port for transmitting > > segmented packets is prohibited. > > Checks for these conflicting configurations have been added to the ethdev > > library, so the drivers don't have to implement them. > > I think the new comment part of ombuf-fast-free and multi-seg in '[PATCH v3 > 2/3] ethdev: Improve descriptions of RX and TX offloads' > could drop or within this commit.
Although the comments are new, I think the behavior described by the new comments is not really new, it was just undocumented. So, patch 2/3 properly documents it, and patch 3/3 enforces it in the ethdev layer, so drivers don't have to. > > > > > Signed-off-by: Morten Brørup <m...@smartsharesystems.com> > > Acked-by: Bruce Richardson <bruce.richard...@intel.com> > > Acked-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru> > > Acked-by: Konstantin Ananyev <konstantin.anan...@huawei.com> > > --- > > v3: > > * Shorten the added log messages. (Stephen Hemminger, Ivan Malov) > > --- > > lib/ethdev/rte_ethdev.c | 37 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > > > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c > > index dd7c00bc94..00afda873f 100644 > > --- a/lib/ethdev/rte_ethdev.c > > +++ b/lib/ethdev/rte_ethdev.c > > @@ -1531,6 +1531,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t > nb_rx_q, uint16_t nb_tx_q, > > goto rollback; > > } > > > > + /* MBUF_FAST_FREE preconditions conflict with MULTI_SEGS support. */ > > + if ((dev_conf->txmode.offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) && > > + (dev_conf->txmode.offloads & RTE_ETH_TX_OFFLOAD_MULTI_SEGS)) { > > + RTE_ETHDEV_LOG_LINE(ERR, > > + "id=%d offload clash, %s vs %s", > > Suggest keep the same trace fmt. e.g.: Ethdev port_id=%u > The same below Initial versions of the series did that, but other reviewers requested a shorter log format. > > > + port_id, > > + > rte_eth_dev_tx_offload_name(RTE_ETH_TX_OFFLOAD_MULTI_SEGS), > > + > rte_eth_dev_tx_offload_name(RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE)); > > + ret = -EINVAL; > > + goto rollback; > > + } > > + > > dev->data->dev_conf.rx_adv_conf.rss_conf.rss_hf = > > rte_eth_rss_hf_refine(dev_conf->rx_adv_conf.rss_conf.rss_hf); > > > > @@ -2709,6 +2721,31 @@ rte_eth_tx_queue_setup(uint16_t port_id, uint16_t > tx_queue_id, > > return -EINVAL; > > } > > > > + /* > > + * If the driver uses a Tx function with MBUF_FAST_FREE preconditions, > > + * per-queue MULTI_SEGS support is not possible. > > + */ > > + if ((dev->data->dev_conf.txmode.offloads & > RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) && > > + (local_conf.offloads & RTE_ETH_TX_OFFLOAD_MULTI_SEGS)) { > > + RTE_ETHDEV_LOG_LINE(ERR, > > + "id=%d txq=%d offload clash, per-queue %s vs per-port > > %s", > > + port_id, tx_queue_id, > > + > rte_eth_dev_tx_offload_name(RTE_ETH_TX_OFFLOAD_MULTI_SEGS), > > + > rte_eth_dev_tx_offload_name(RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE)); > > + return -EINVAL; > > + } > > + /* > > + * If the driver uses a Tx function with MULTI_SEGS support, > > + * runtime support for per-queue MBUF_FAST_FREE optimization depends on > the driver. > > + */ > > + if ((dev->data->dev_conf.txmode.offloads & > RTE_ETH_TX_OFFLOAD_MULTI_SEGS) && > > + (local_conf.offloads & > > RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE)) > > + RTE_ETHDEV_LOG_LINE(DEBUG, > > + "id=%d txq=%d potential offload clash, per-queue %s vs > per-port %s: PMD to decide", > > + port_id, tx_queue_id, > > + > rte_eth_dev_tx_offload_name(RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE), > > + > rte_eth_dev_tx_offload_name(RTE_ETH_TX_OFFLOAD_MULTI_SEGS)); > > 1\ The global port configuration will take effect on all queues. > If it don't support both MBUF_FAST_FREE and MULTI_SEG, why don't ban it > here? There is a slight difference... If a port is configured for MBUF_FAST_FREE, it may install a TX function (port-wide, i.e. on all queues) relying on the mbuf-fast-free requirements, which prohibit segmented packets. So combining per-port MBUF_FAST_FREE with per-port or per-queue MULTI_SEGS is impossible. However, if a port is configured for MULTI_SEGS, it may install a TX function (port-wide) with support for per-queue MBUF_FAST_FREE, by checking the queue structure at runtime. > 2\ Debug level is easy to ignore, how about warning? If a driver's installed MULTI_SEGS TX function supports runtime checking the queue for MBUF_FAST_FREE, then there should be no warning/notice level log message, as it is considered "normal" behavior. However, if a driver doesn't support it, the driver must return -EINVAL for such a queue configuration request. And if the driver doesn't log any message with that, then this debug level log message could help the user debug the issue. I was in doubt if I should add this debug check or not, but I decided that it might be useful for debugging driver issues related to such a configuration. > > > + > > rte_ethdev_trace_txq_setup(port_id, tx_queue_id, nb_tx_desc, tx_conf); > > return eth_err(port_id, dev->dev_ops->tx_queue_setup(dev, > > tx_queue_id, nb_tx_desc, socket_id, &local_conf));