From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Hemant Agrawal <hemant.agrawal@nxp.com>, dev@dpdk.org
Cc: shreyansh.jain@nxp.com, Sunil Kumar Kori <sunil.kori@nxp.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/2] net/dpaa: fix the ethdev offload checks
Date: Tue, 24 Apr 2018 17:43:06 +0100 [thread overview]
Message-ID: <ac02791d-06cd-8915-0ba1-82a3c063eab6@intel.com> (raw)
In-Reply-To: <1524582401-14696-1-git-send-email-hemant.agrawal@nxp.com>
On 4/24/2018 4:06 PM, Hemant Agrawal wrote:
> From: Sunil Kumar Kori <sunil.kori@nxp.com>
>
> Fixes: 16e2c27f4fc7 ("net/dpaa: support new ethdev offload APIs")
>
> Signed-off-by: Sunil Kumar Kori <sunil.kori@nxp.com>
> ---
> drivers/net/dpaa/dpaa_ethdev.c | 89 +++++++++++++++++++++++++++---------------
> 1 file changed, 57 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
> index b2740b4..32d36f2 100644
> --- a/drivers/net/dpaa/dpaa_ethdev.c
> +++ b/drivers/net/dpaa/dpaa_ethdev.c
> @@ -45,6 +45,33 @@
> #include <fsl_bman.h>
> #include <fsl_fman.h>
>
> +/* Supported Rx offloads */
> +static uint64_t dev_rx_offloads_sup =
> + DEV_RX_OFFLOAD_JUMBO_FRAME;
> +
> +/* Rx offloads which cannot be disabled */
> +static uint64_t dev_rx_offloads_nodis =
> + DEV_RX_OFFLOAD_IPV4_CKSUM |
> + DEV_RX_OFFLOAD_UDP_CKSUM |
> + DEV_RX_OFFLOAD_TCP_CKSUM |
> + DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
> + DEV_RX_OFFLOAD_CRC_STRIP |
> + DEV_RX_OFFLOAD_SCATTER;
> +
> +/* Supported Tx offloads */
> +static uint64_t dev_tx_offloads_sup;
> +
> +/* Tx offloads which cannot be disabled */
> +static uint64_t dev_tx_offloads_nodis =
> + DEV_TX_OFFLOAD_IPV4_CKSUM |
> + DEV_TX_OFFLOAD_UDP_CKSUM |
> + DEV_TX_OFFLOAD_TCP_CKSUM |
> + DEV_TX_OFFLOAD_SCTP_CKSUM |
> + DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM |
> + DEV_TX_OFFLOAD_MULTI_SEGS |
> + DEV_TX_OFFLOAD_MT_LOCKFREE |
> + DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> +
> /* Keep track of whether QMAN and BMAN have been globally initialized */
> static int is_global_init;
> /* At present we only allow up to 4 push mode queues - as each of this queue
> @@ -143,35 +170,41 @@ dpaa_eth_dev_configure(struct rte_eth_dev *dev)
> {
> struct dpaa_if *dpaa_intf = dev->data->dev_private;
> struct rte_eth_conf *eth_conf = &dev->data->dev_conf;
> - struct rte_eth_dev_info dev_info;
> uint64_t rx_offloads = eth_conf->rxmode.offloads;
> uint64_t tx_offloads = eth_conf->txmode.offloads;
>
> PMD_INIT_FUNC_TRACE();
>
> - dpaa_eth_dev_info(dev, &dev_info);
> - if (((~(dev_info.rx_offload_capa) & rx_offloads) != 0)) {
> - DPAA_PMD_ERR("Some Rx offloads are not supported "
> - "requested 0x%" PRIx64 " supported 0x%" PRIx64,
> - rx_offloads, dev_info.rx_offload_capa);
> - return -ENOTSUP;
> + /* Rx offloads validation */
> + if (dev_rx_offloads_nodis & ~rx_offloads) {
> + DPAA_PMD_WARN(
> + "Rx offloads non configurable - requested 0x%" PRIx64
> + " ignored 0x%" PRIx64,
> + rx_offloads, dev_rx_offloads_nodis);
> }
> -
> - if (((~(dev_info.tx_offload_capa) & tx_offloads) != 0)) {
> - DPAA_PMD_ERR("Some Tx offloads are not supported "
> - "requested 0x%" PRIx64 " supported 0x%" PRIx64,
> - tx_offloads, dev_info.tx_offload_capa);
> + if (~(dev_rx_offloads_sup | dev_rx_offloads_nodis) & rx_offloads) {
> + DPAA_PMD_ERR(
> + "Rx offloads non supported - requested 0x%" PRIx64
> + " supported 0x%" PRIx64,
> + rx_offloads,
> + dev_rx_offloads_sup | dev_rx_offloads_nodis);
> return -ENOTSUP;
> }
Hi Hemant,
Overall this looks good to me, thanks.
Only I would like to ask if you prefer to replace nodis and not_supported checks.
Because with current order, if an offlaod requested that both has not supported
offload and not enable all nodis offloads, this will print both logs and return
error. Since it will return error, do you really need "non configurable" log?
If you replace checks, if any not supported offload requested it will only print
log for it and return error without checking/caring nodis offloads.
It is up to you, please let me know if you want to go with existing set.
Thanks,
ferruh
next prev parent reply other threads:[~2018-04-24 16:43 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-09 10:26 [dpdk-dev] [PATCH 0/2] Support for new Ethdev offload APIs Sunil Kumar Kori
2018-04-09 10:26 ` [dpdk-dev] [PATCH 1/2] net/dpaa: Changes to support ethdev " Sunil Kumar Kori
2018-04-09 13:19 ` Sunil Kumar Kori
2018-04-09 13:19 ` [dpdk-dev] [PATCH 2/2] net/dpaa2: " Sunil Kumar Kori
2018-04-10 16:47 ` Ferruh Yigit
2018-04-10 16:40 ` [dpdk-dev] [PATCH 1/2] net/dpaa: " Ferruh Yigit
2018-04-09 10:26 ` [dpdk-dev] [PATCH 2/2] net/dpaa2: " Sunil Kumar Kori
2018-04-11 11:05 ` [dpdk-dev] [PATCH v2 0/2] Support for new Ethdev " Sunil Kumar Kori
2018-04-11 11:05 ` [dpdk-dev] [PATCH v2 1/2] net/dpaa: Changes to support ethdev " Sunil Kumar Kori
2018-04-11 11:05 ` [dpdk-dev] [PATCH v2 2/2] net/dpaa2: " Sunil Kumar Kori
2018-04-12 18:17 ` [dpdk-dev] [PATCH v2 0/2] Support for new Ethdev " Ferruh Yigit
2018-04-24 15:06 ` [dpdk-dev] [PATCH v3 1/2] net/dpaa: fix the ethdev offload checks Hemant Agrawal
2018-04-24 15:06 ` [dpdk-dev] [PATCH v3 2/2] net/dpaa2: " Hemant Agrawal
2018-04-24 16:43 ` Ferruh Yigit [this message]
2018-04-24 17:23 ` [dpdk-dev] [PATCH v3 1/2] net/dpaa: " Hemant Agrawal
2018-04-24 17:16 ` [dpdk-dev] [PATCH v4 " Hemant Agrawal
2018-04-24 17:16 ` [dpdk-dev] [PATCH v4 2/2] net/dpaa2: " Hemant Agrawal
2018-04-24 18:04 ` [dpdk-dev] [PATCH v4 1/2] net/dpaa: " Ferruh Yigit
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=ac02791d-06cd-8915-0ba1-82a3c063eab6@intel.com \
--to=ferruh.yigit@intel.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
--cc=shreyansh.jain@nxp.com \
--cc=sunil.kori@nxp.com \
/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).