DPDK patches and discussions
 help / color / mirror / Atom feed
From: fengchengwen <fengchengwen@huawei.com>
To: "Morten Brørup" <mb@smartsharesystems.com>,
	dev@dpdk.org, "Aman Singh" <aman.deep.singh@intel.com>,
	"Thomas Monjalon" <thomas@monjalon.net>,
	"Andrew Rybchenko" <andrew.rybchenko@oktetlabs.ru>,
	"Ivan Malov" <ivan.malov@arknetworks.am>,
	"Stephen Hemminger" <stephen@networkplumber.org>
Cc: Konstantin Ananyev <konstantin.ananyev@huawei.com>,
	Bruce Richardson <bruce.richardson@intel.com>
Subject: Re: [PATCH v3 3/3] ethdev: Reject conflicting TX offloads configuration
Date: Tue, 5 Aug 2025 10:10:15 +0800	[thread overview]
Message-ID: <b3da43b5-dbf1-428d-ad13-0c787d322654@huawei.com> (raw)
In-Reply-To: <20250803194218.683318-3-mb@smartsharesystems.com>

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.

> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@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

> +			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?
2\ Debug level is easy to ignore, how about warning?

> +
>  	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));


  reply	other threads:[~2025-08-05  2:10 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-31  9:07 [PATCH] " Morten Brørup
2025-07-31  9:24 ` Bruce Richardson
2025-07-31 10:07   ` Andrew Rybchenko
2025-07-31  9:27 ` Ivan Malov
2025-07-31  9:34   ` Morten Brørup
2025-07-31 11:32 ` Konstantin Ananyev
2025-07-31 12:56 ` [PATCH v2 1/3] testpmd: Do not enable mbuf fast release TX offload by default Morten Brørup
2025-07-31 12:56   ` [PATCH v2 2/3] ethdev: Improve descriptions of RX and TX offloads Morten Brørup
2025-07-31 12:56   ` [PATCH v2 3/3] ethdev: Reject conflicting TX offloads configuration Morten Brørup
2025-08-01  1:09     ` zhoumin
2025-08-02 20:53 ` [PATCH] " Stephen Hemminger
2025-08-02 21:33   ` Ivan Malov
2025-08-03 10:51     ` Morten Brørup
2025-08-03 15:56       ` Stephen Hemminger
2025-08-03 19:42 ` [PATCH v3 1/3] testpmd: Do not enable mbuf fast release TX offload by default Morten Brørup
2025-08-03 19:42   ` [PATCH v3 2/3] ethdev: Improve descriptions of RX and TX offloads Morten Brørup
2025-08-03 19:42   ` [PATCH v3 3/3] ethdev: Reject conflicting TX offloads configuration Morten Brørup
2025-08-05  2:10     ` fengchengwen [this message]
2025-08-05  0:58   ` [PATCH v3 1/3] testpmd: Do not enable mbuf fast release TX offload by default fengchengwen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b3da43b5-dbf1-428d-ad13-0c787d322654@huawei.com \
    --to=fengchengwen@huawei.com \
    --cc=aman.deep.singh@intel.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=ivan.malov@arknetworks.am \
    --cc=konstantin.ananyev@huawei.com \
    --cc=mb@smartsharesystems.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).