Re: [PATCH] net/nfp: set the appropriate initialized value of flbufsz

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 10/15/2022 8:38 AM, Nole Zhang wrote:
>   > On 10/10/2022 7:48 AM, Chaoyong He wrote:
>>> From: Peng Zhang <peng.zhang@corigine.com>
>>>
>>> When the testpmd app start-up with parameter max-pkt-len, it will set
>> MTU.
>>> But the initialized value of flubfsz is inappropriate, if the value of
>>> flbufsz is smaller than the valude of max-pkt-len, the testpmd app
>>> will start fail.
>>>
>>
>> What is the failure in the testpmd?
> 
> The log is as follows:
> [root@volstruis ~]# dpdk-testpmd --main-lcore 10 -l 10,11,12 -n 4 -a 0000:81:00.0 --socket-mem 2048,2048 --proc-type auto -- --portmask 0x3 --nb-cores 2 --rxq 1 --txq 1 --rxd 1024 --txd 1024 --port-topology loop --forward-mode macswap  --max-pkt-len 9216 --mbuf-size 9600  --rss-udp --burst=32
> EAL: Detected CPU lcores: 40
> EAL: Detected NUMA nodes: 2
> EAL: Auto-detected process type: PRIMARY
> EAL: Detected static linkage of DPDK
> EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
> EAL: Selected IOVA mode 'VA'
> EAL: VFIO support initialized
> EAL: Using IOMMU type 1 (Type 1)
> EAL: Probe PCI driver: net_nfp_pf (19ee:4000) device: 0000:81:00.0 (socket 1) NFP HWINFO header: 48490200
> TELEMETRY: No legacy callbacks, legacy socket not created Set macswap packet forwarding mode
> testpmd: create a new mbuf pool <mb_pool_1>: n=163456, size=9600, socket=1
> testpmd: preferred mempool ops selected: ring_mp_mc Configuring Port 0 (socket 1)
> Port0 dev_configure = -34
> Fail to configure port 0
> EAL: Error - exiting with code: 1
>    Cause: Start ports failed
> 
> First  in the `nfp_net_configure()`, we will judge the value of MTU and hw->flbufsz, If MTU > hw->flbufsz, it will have the error.
> 
> And the `--max-pkt-len` is setting the MTU in the initialize process, the initialized  value of  hw->flbufsz is just 1500 at first.
> 
> So if we set the `max-pkt-len`  bigger than the initialized value of flbufsz, It will lead the error.
> 
> Hence we set the new value of hw->flbufsz, it can large the range max-pkt-len in the initialized process.
> 
> 
>>
>> This patch is fixing something but it is not clear what is fixed, the concern is it
>> may be changing driver to make something pass in test application (testpmd).
>>
>> What is 'flubfsz', is it Hw configured frame buffer size?
> 
> 
> It is configured in the `nfp_net_rx_queue_setup()`{`hw->flbufsz = rxq->mbuf_size`}.
> If the rxq->mbuf_size < MTU, the MTU can't work.
> 

It looks like `hw->flbufsz` holds the Rx buffer size, as you highlighted 
above.

And you don't want to accept frames bigger than buffer size, since it 
seems driver doesn't support `RTE_ETH_RX_OFFLOAD_SCATTER`, all looks OK.


According above logic, I agree "hw->flbufsz = RTE_ETHER_MTU;" is wrong, 
but equally `hw->flbufsz = hw->max_mtu;` seems wrong.

In above command line, it is safe because "mbuf-size=9600" and 
"max-pkt-len=9216", buffer size is bigger than packet size.

You should able to set `hw->flbufsz` to current buffer size, instead of 
a hardcoded value.

In `nfp_net_init()`, most probably you don't know the buffer size yet, 
can't you skip setting this value here and set it in 
`nfp_net_rx_queue_setup()` when you know the buffer size?


>>
>>
>>> Fixes: 5c305e218f15 ("net/nfp: fix initialization")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
>>> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>>> ---
>>>    drivers/net/nfp/nfp_ethdev.c    | 2 +-
>>>    drivers/net/nfp/nfp_ethdev_vf.c | 2 +-
>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/nfp/nfp_ethdev.c
>>> b/drivers/net/nfp/nfp_ethdev.c index 5cdd34e588..b95e623f1f 100644
>>> --- a/drivers/net/nfp/nfp_ethdev.c
>>> +++ b/drivers/net/nfp/nfp_ethdev.c
>>> @@ -517,7 +517,7 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
>>>        hw->cap = nn_cfg_readl(hw, NFP_NET_CFG_CAP);
>>>        hw->max_mtu = nn_cfg_readl(hw, NFP_NET_CFG_MAX_MTU);
>>>        hw->mtu = RTE_ETHER_MTU;
>>> -     hw->flbufsz = RTE_ETHER_MTU;
>>> +     hw->flbufsz = hw->max_mtu;
>>>
>>>        /* VLAN insertion is incompatible with LSOv2 */
>>>        if (hw->cap & NFP_NET_CFG_CTRL_LSO2) diff --git
>>> a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
>>> index d304d78d34..47acb4c60e 100644
>>> --- a/drivers/net/nfp/nfp_ethdev_vf.c
>>> +++ b/drivers/net/nfp/nfp_ethdev_vf.c
>>> @@ -396,7 +396,7 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
>>>        hw->cap = nn_cfg_readl(hw, NFP_NET_CFG_CAP);
>>>        hw->max_mtu = nn_cfg_readl(hw, NFP_NET_CFG_MAX_MTU);
>>>        hw->mtu = RTE_ETHER_MTU;
>>> -     hw->flbufsz = RTE_ETHER_MTU;
>>> +     hw->flbufsz = hw->max_mtu;
>>>
>>>        /* VLAN insertion is incompatible with LSOv2 */
>>>        if (hw->cap & NFP_NET_CFG_CTRL_LSO2)
>