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

[Nole Zhang <peng.zhang@corigine.com>]

> 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.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.
> > 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.
> >>
> >> 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()`.

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