On 3/4/2022 10:20 AM, Vamsi Krishna Attunuru wrote:


-----Original Message-----
From: Ferruh Yigit <ferruh.yi...@intel.com>
Sent: Thursday, March 3, 2022 10:52 PM
To: Vamsi Krishna Attunuru <vattun...@marvell.com>; dev@dpdk.org
Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Nithin Kumar
Dabilpuram <ndabilpu...@marvell.com>; yux.ji...@intel.com;
sta...@dpdk.org; Wei Ling <weix.l...@intel.com>; Srikanth Yalavarthi
<syalavar...@marvell.com>
Subject: [EXT] Re: [PATCH v2 1/1] common/cnxk: fix static assertion failure

External Email

----------------------------------------------------------------------
On 3/2/2022 1:46 PM, Vamsi Attunuru wrote:
Use dynamically allocated memory for storing soft expiry ring base
addresses which fixes the static assertion failure, as the size of
dynamic allocation depends on RTE_MAX_ETHPORTS which varies based on
the build config.


Hi Vamsi,

"fix static assertion failure" is not enough descriptive.
assertions already added to verify assumptions, and in this case it seems it
failed, but what was actually wrong?

Is it that allocated memory size for ring wrong? (this is what I got from
commit log but I am not sure)

Can you please describe what actually was wrong and fixed now?

Hi Ferruh,

Earlier sa_soft_exp_ring struct member was an array of pointers and it's size 
is linked to num RTE_MAX_ETHPORTS,
and the whole struct size is confined and protected by size assertion.  It 
resulted in build failure with -Dmax_ethports=1024
option and assertion caught that failure. V2 fixes the issues by allocating the 
required memory dynamically instead
  of using array of pointers.


just to double check if I got it right,

The failing assertion is:
PLT_STATIC_ASSERT(sizeof(struct nix_inl_dev) <= ROC_NIX_INL_MEM_SZ);

Technically you can calculate the 'ROC_NIX_INL_MEM_SZ' as a function
of 'RTE_MAX_ETHPORTS' and that would work (although need to calculate
size for proper cache alignment).

But instead you prefer to convert array to function pointer to fix
the struct size and make it independent from the configured
'RTE_MAX_ETHPORTS' config.


And I assume current magic number for the 'ROC_NIX_INL_MEM_SZ' is
calculated based on max memory requirement of the cn9k & cn10k:
#define ROC_NIX_INL_MEM_SZ (1280)

If so it can be better to describe 'ROC_NIX_INL_MEM_SZ' as what
it is calculated from, like following but it is up to you:
max(sizeof(x), sizeof(y)) ...

Bugzilla ID: 940
Fixes: d26185716d3f ("net/cnxk: support outbound soft expiry
notification") Cc:sta...@dpdk.org

Reported-by: Wei Ling<weix.l...@intel.com>
Reported-by: Yu Jiang<yux.ji...@intel.com>
Signed-off-by: Vamsi Attunuru<vattun...@marvell.com>
Signed-off-by: Srikanth Yalavarthi<syalavar...@marvell.com>
---
V2: Add bugzilla & reportee details, remove unused changes.
---


Reply via email to