DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Tom Barbette <barbette@kth.se>, dev@dpdk.org
Cc: thomas@monjalon.net, arybchenko@solarflare.com,
	Shahaf Shuler <shahafs@mellanox.com>,
	Yongseok Koh <yskoh@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH] reta_query: Doc requirements on reta_conf
Date: Thu, 4 Apr 2019 18:00:14 +0100	[thread overview]
Message-ID: <82bb0190-a905-bebc-747b-cb42141ca2ea@intel.com> (raw)
In-Reply-To: <1554281333-119866-1-git-send-email-barbette@kth.se>

On 4/3/2019 9:48 AM, Tom Barbette wrote:
> As the librte function checks that bits of up to reta_size in reta_conf
> are set, the arg passed must have the mask set to 1. I spent quite some
> time before finding the issue here, so I thought updating the doc may
> help other people.

In 'reta_conf' there are 64 table entries and have a 64 bits 'mask', for each
entry requested matching mask bit should be set.

'rte_eth_dev_rss_reta_query()' returns error if none of the bits in 'mask' set,
this makes sense because it means user is calling this function without
requesting any work at all.

So the requirement is not to set 'mask' to 1 but set bits per requested table entry.

> 
> The problem raise when the size of the table is not on a (byte?)
> boundary, eg when using a power-of-2 amount of queues with mlx5, the
> table size becomes the number of queue. Then a usual (bzero)
> initialization raise EINVAL, because of this bits problem.
> 
> I'd say the requirement should go, one expects this functions to overwrite
> whatever is passed as table arguments. Who would want to query the table
> except a few entries?> Maybe (idk) mlx5 is actually the only device to support non-byte sized RSS
> table, and mlx5 actually ignore the mask in reta_query...

You are right, mlx5 implementation is wrong!

It should take account the mask and set only values in the reta that requested.
Although returning all table will work from user point of view since values user
looking for will be valid in the table. But it is wrong from API implementation
point of view.

> ---
>  lib/librte_ethdev/rte_ethdev.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index e254da8..ace425f 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -2941,7 +2941,7 @@ int rte_eth_dev_rss_reta_update(uint16_t port_id,
>   * @param port_id
>   *   The port identifier of the Ethernet device.
>   * @param reta_conf
> - *   RETA to query.
> + *   RETA to query. The mask bits must be set according to reta_size.

Overall no objection to clarify mask bits should be set requirement, but not
sure if it should be documented related to reta_size.

what about something like following, please feel free to update/correct as needed:
'For each requested reta entry, corresponding bit in mask must be set.'

>   * @param reta_size
>   *   Redirection table size. The table size can be queried by
>   *   rte_eth_dev_info_get().
> 

  parent reply	other threads:[~2019-04-04 17:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-03  8:48 Tom Barbette
2019-04-03  8:48 ` Tom Barbette
2019-04-04 17:00 ` Ferruh Yigit [this message]
2019-04-04 17:00   ` 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=82bb0190-a905-bebc-747b-cb42141ca2ea@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=arybchenko@solarflare.com \
    --cc=barbette@kth.se \
    --cc=dev@dpdk.org \
    --cc=shahafs@mellanox.com \
    --cc=thomas@monjalon.net \
    --cc=yskoh@mellanox.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).