On 10/18/2022 10:50 AM, Nole Zhang wrote:


-----Original Message-----
From: Ferruh Yigit <ferruh.yi...@amd.com>
Sent: 2022年10月18日 17:36
To: Nole Zhang <peng.zh...@corigine.com>; Chaoyong He
<chaoyong...@corigine.com>; dev@dpdk.org
Cc: oss-drivers <oss-driv...@corigine.com>; Niklas Soderlund
<niklas.soderl...@corigine.com>; sta...@dpdk.org
Subject: Re: [PATCH] net/nfp: set the appropriate initialized value of flbufsz

On 10/18/2022 9:51 AM, Nole Zhang wrote:
On 10/18/2022 2:41 AM, Nole Zhang wrote:

On 10/15/2022 8:38 AM, Nole Zhang wrote:
     > On 10/10/2022 7:48 AM, Chaoyong He wrote:
From: Peng Zhang <peng.zh...@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.
Yes, if I need set the hardcoded value, I should set the max max-pkt-len.

I think it should be 'mbuf-size', since 'hw->flbufsz' is the buffer size.
'max-pkt-len' is the max Rx frame accepted by NIC, for some devices
'max- pkt-len' can be bigger than buffer size but it will work fine
because of segmented Rx, but it seems this config doesn't work for nfp.

As you said, maybe the ` NFP_FRAME_SIZE_MAX ` is right, for our nic,
the max_rx_pktlen is NFP_FRAME_SIZE_MAX.

If 'NFP_FRAME_SIZE_MAX' is your HW limit, agree to set it if it will be a
hardcoded value, please see below.


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?


But If I just depends on the `nfp_net_rx_queue_setup()`, in the
`nfp_net_init()`, it will Call the  `nfp_net_configure()`, it will
lead the testpmd start failed, so I add the hardcoded value in the
initialize
process. Or  I can remove the judge about  `hw->flbufsz` in the
`nfp_net_init()`.


Instead of setting 'hw->flbufsz' in 'nfp_net_init()' and check it in
'nfp_net_configure()', why not set 'hw->flbufsz' in 'nfp_net_configure()'?

Because in 'nfp_net_configure()', 'mtu' is a configuration parameter.
It can be possible to convert 'mtu' to frame size and set it. If
there is a HW limitation on buffer size, it can fail in 'nfp_net_configure()'
when that limit hit.

You mean in the 'nfp_net_configure()', set the value of 'hw->flbufsz',
Like `if (rxmode->mtu > hw->flbufsz) {
                hw->flbufsz = rxmode->mtu;
        }`
Because the 'nfp_net_configure()' also isn't only called once.
If it will be set the values every times, so I first just set the
Initial value in the `nfp_net_inital()`.



But during init, 'nfp_net_init()', you don't know the buffer size, that is why
you are just setting a hardcoded value, which can be wrong, as you are
observing now.

Also thinking twice setting "hw->flbufsz = rxmode->mtu" is not correct, since
it is not frame size either.


What about:
- in 'nfp_net_configure()' change check as:
if (rxmode->mtu > NFP_FRAME_SIZE_MAX)
        return error

And you may want to save 'mtu' in driver or in device via
nn_cfg_writel(hw, NFP_NET_CFG_MTU, mtu);


- in start() add check for buffer size:
if (mtu > hw->flbufsz)
        return error

- in init() you can either remove setting 'hw->flbufsz' or set it to max
(NFP_FRAME_SIZE_MAX) as you suggested.
If you remove setting it in init, you can have logic in configure() as
`
bufsize = hw->flbufsz ? hw->flbufsz : NFP_FRAME_SIZE_MAX;
if (rxmode->mtu > bufsize)
        return error
`
Yes, thanks for your advice.

I will add the
  - in start() add check for buffer size:
`if (mtu > hw->flbufsz)
        return error`
-in the init() keep the value
`
hw->flbufsz = NFP_FRAME_SIZE_MAX;
`
-in the conigdure() use this check
`
if (rxmode->mtu > NFP_FRAME_SIZE_MAX)
        return error
`

If you will initialize 'hw->flbufsz' as 'NFP_FRAME_SIZE_MAX' in the init(), you can keep the check in the configure() intact, up to you.

I will send the v2 after the testing.


ack, thanks.

Thanks for your review and help again.


Thanks for your advice.



Fixes: 5c305e218f15 ("net/nfp: fix initialization")
Cc: sta...@dpdk.org
...
          /* VLAN insertion is incompatible with LSOv2 */
          if (hw->cap & NFP_NET_CFG_CTRL_LSO2)





Reply via email to