From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 1E3F9A0524;
Tue, 13 Apr 2021 14:28:17 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id D34EB160F04;
Tue, 13 Apr 2021 14:28:16 +0200 (CEST)
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
by mails.dpdk.org (Postfix) with ESMTP id 414DB160EFC
for <dev@dpdk.org>; Tue, 13 Apr 2021 14:28:15 +0200 (CEST)
IronPort-SDR: HzLAbh8scH96LeOvb8pK0EXo6+ql8Dj53inRxjFkNPtPbLA7YG3DmCqk2nmEspiYFyqsfyfCH2
faX0djVIwJtA==
X-IronPort-AV: E=McAfee;i="6200,9189,9952"; a="194514142"
X-IronPort-AV: E=Sophos;i="5.82,219,1613462400"; d="scan'208";a="194514142"
Received: from fmsmga008.fm.intel.com ([10.253.24.58])
by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
13 Apr 2021 05:28:13 -0700
IronPort-SDR: vo7hkBBd52BFiXIRxa3Z8pedRsW/t7Tdhc7JPzXWzNx9PsJIPhvRfiV8uracowVDkTzSk58jLK
hP8EgHDAoZ4A==
X-IronPort-AV: E=Sophos;i="5.82,219,1613462400"; d="scan'208";a="417840202"
Received: from vmedvedk-mobl.ger.corp.intel.com (HELO [10.252.18.25])
([10.252.18.25])
by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
13 Apr 2021 05:28:10 -0700
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: "Chilikin, Andrey" <andrey.chilikin@intel.com>,
"Kinsella, Ray" <ray.kinsella@intel.com>,
"Wang, Yipeng1" <yipeng1.wang@intel.com>,
"Gobriel, Sameh" <sameh.gobriel@intel.com>,
"Richardson, Bruce" <bruce.richardson@intel.com>
References: <1615919077-77774-1-git-send-email-vladimir.medvedkin@intel.com>
<1617738643-258635-3-git-send-email-vladimir.medvedkin@intel.com>
<DM6PR11MB4491B58CD26988E83EC504C69A759@DM6PR11MB4491.namprd11.prod.outlook.com>
<3fb31ad3-cb84-05a4-fb0e-46e33e008f57@intel.com>
<DM6PR11MB4491CE573779470E934BE7C49A709@DM6PR11MB4491.namprd11.prod.outlook.com>
From: "Medvedkin, Vladimir" <vladimir.medvedkin@intel.com>
Message-ID: <8cb12932-aba3-66f2-4560-f7baffe950ea@intel.com>
Date: Tue, 13 Apr 2021 15:28:08 +0300
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
Thunderbird/78.9.0
MIME-Version: 1.0
In-Reply-To: <DM6PR11MB4491CE573779470E934BE7C49A709@DM6PR11MB4491.namprd11.prod.outlook.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH v2 2/3] hash: add predictable RSS
implementation
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Hi Konstantin,
On 12/04/2021 12:47, Ananyev, Konstantin wrote:
>>
>> <snip>
>>
>>>> +#defineRETA_SZ_MIN2U
>>>> +#defineRETA_SZ_MAX16U
>>>
>>> Should these RETA_SZ defines be in public header?
>>> So user can know what are allowed values?
>>>
>>
>> I don't think this is necessary, because the user chooses it not
>> arbitrary, but depending on the NIC.
>
> Sure thing, but it would be goo for the user to know what are the SW
> Limitations on it without digging through .c.
>
I see, I'll move it to .h
>>
>>>> +#define RETA_SZ_IN_RANGE(reta_sz)((reta_sz >= RETA_SZ_MIN) && \
>>
>> <snip>
>>
>>>> +uint32_t i;
>>>
>>> Empty line is missing.
>>>
>>
>> Thanks
>>
>>>> +if ((name == NULL) || (key_len == 0) || !RETA_SZ_IN_RANGE(reta_sz)) {
>>>> +rte_errno = EINVAL;
>>>> +return NULL;
>>>> +}
>>
>> <snip>
>>
>>>> +static inline void
>>>> +set_bit(uint8_t *ptr, uint32_t bit, uint32_t pos)
>>>> +{
>>>> +uint32_t byte_idx = pos >> 3;
>>>
>>> Just as a nit to be consistent with the line below:
>>> pos / CHAR_BIT;
>>>
>>
>> Fixed
>>
>>>> +uint32_t bit_idx = (CHAR_BIT - 1) - (pos & (CHAR_BIT - 1));
>>>> +uint8_t tmp;
>>
>> <snip>
>>
>>>> +ent = rte_zmalloc(NULL, sizeof(struct rte_thash_subtuple_helper) +
>>>> +sizeof(uint32_t) * (1 << ctx->reta_sz_log), 0);
>>>
>>> Helper can be used by data-path code (via rte_thash_get_compliment()) right?
>>> Then might be better to align it at cache-line.
>>>
>>
>> Agree, I'll fix it
>>
>>>> +if (ent == NULL)
>>>> +return -ENOMEM;
>>
>> <snip>
>>
>>>> uint32_t
>>>> -rte_thash_get_compliment(struct rte_thash_subtuple_helper *h __rte_unused,
>>>> -uint32_t hash __rte_unused, uint32_t desired_hash __rte_unused)
>>>> +rte_thash_get_compliment(struct rte_thash_subtuple_helper *h,
>>>> +uint32_t hash, uint32_t desired_hash)
>>>> {
>>>> -return 0;
>>>> +return h->compl_table[(hash ^ desired_hash) & h->lsb_msk];
>>>> }
>>>
>>> Would it make sense to add another-one for multi values:
>>> rte_thash_get_compliment(uint32_t hash, const uint32_t desired_hashes[], uint32_t adj_hash[], uint32_t num);
>>> So user can get adjustment values for multiple queues at once?
>>>
>>
>> At the moment I can't find scenarios why do we need to have a bulk
>> version for this function
>
> My thought was about case when number of configured
> HW queues is less than reta_size.
> Let say reta_size==4, but user configured only 3 queues and reta={0,1,2,0}.
> In that case for queue 0, both 0 and 3 values would suit.
>
Ah, yes, I remember out discussion about this. I'll think about this and
add next releases.
I should be kind of
int
rte_thash_get_compliment_reta(struct rte_thash_subtuple_helper *helper,
uint32_t hash, const uint16_t queue_id, struct rte_eth_rss_reta_entry64
*reta, int reta_sz);
where reta is completed by rte_eth_dev_rss_reta_query()
--
Regards,
Vladimir