Re: [dpdk-stable] [dpdk-dev] [PATCH 1/2] app/testpmd: fix scatter offload configuration

[Matan Azrad <matan@mellanox.com>]

Hi Ferruh

From: Ferruh Yigit
> On 7/30/2019 7:34 PM, Matan Azrad wrote:
> >
> >
> > From: Ferruh Yigit
> >> On 7/30/2019 4:56 PM, Matan Azrad wrote:
> >>> Hi Ferruh
> >>>
> >>>  From: Ferruh Yigit
> >>>> Sent: Tuesday, July 30, 2019 6:22 PM
> >>>> To: Matan Azrad <matan@mellanox.com>; Wenzhuo Lu
> >>>> <wenzhuo.lu@intel.com>; Jingjing Wu <jingjing.wu@intel.com>
> >>>> Cc: dev@dpdk.org; stable@dpdk.org
> >>>> Subject: Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix scatter
> >>>> offload configuration
> >>>>
> >>>> On 7/30/2019 2:17 PM, Matan Azrad wrote:
> >>>>> Hi Ferruh
> >>>>>
> >>>>> From: Ferruh Yigit
> >>>>>> Sent: Tuesday, July 30, 2019 4:09 PM
> >>>>>> To: Matan Azrad <matan@mellanox.com>; Wenzhuo Lu
> >>>>>> <wenzhuo.lu@intel.com>; Jingjing Wu <jingjing.wu@intel.com>
> >>>>>> Cc: dev@dpdk.org; stable@dpdk.org
> >>>>>> Subject: Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix scatter
> >>>>>> offload configuration
> >>>>>>
> >>>>>> On 7/29/2019 1:36 PM, Matan Azrad wrote:
> >>>>>>> When the mbuf data size cannot contain the maximum Rx packet
> >>>>>>> length with the mbuf headroom, a packet should be scattered in
> >>>>>>> more than one
> >>>>>> mbuf.
> >>>>>>>
> >>>>>>> The application did not configure scatter offload in the above case.
> >>>>>>>
> >>>>>>> Enable the Rx scatter offload in the above case.
> >>>>>>>
> >>>>>>> Fixes: 33f9630fc23d ("app/testpmd: create mbuf based on max
> >>>>>>> supported
> >>>>>>> segments")
> >>>>>>> Cc: stable@dpdk.org
> >>>>>>>
> >>>>>>> Signed-off-by: Matan Azrad <matan@mellanox.com>
> >>>>>>
> >>>>>> Deferring the patchset to next release, they were late anyway and
> >>>>>> not actually fixing a defect, safer to defer than getting them in rc3.
> >>>>>
> >>>>> Yes this patch came late for RC3 but it is a fix.
> >>>>>
> >>>>> What are you concerns here?
> >>>>> Why don't you think defect found?
> >>>>
> >>>> First patch changes the behavior, when mbuf size is larger than
> >>>> configured size and user didn't provided the scatter offload,
> >>>> should test application automatically enable it?
> >>>
> >>> No, only when the mbuf size is smaller than the max_rx_pkt_len with
> >> headroom.
> >>> If scatter is not enabled in the above case, how can the PMD provide
> >>> a
> >> packet with max_rx_pkt_len size?
> >>>
> >
> > Answer here?
> 
> Is it because drivers also "automatically" enable scattered Rx based on other
> values?

Scatter is a defined RX offload.
Like other offloads I think it always should be explicitly set by the user if he wants it, and vice versa.
If the user doesn't configure it, the PMD should not scatter packets because the user doesn't expect multi-mbuf packets in datapath and maybe even doesn't handle it.

So, I think the above case is a user conflict in Rx configuration and like other conflicts it should cause an error.
In MLX5 PMD, an error will be returned from Rx setup.