On 7/31/2019 7:11 AM, Matan Azrad wrote: > 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 <ma...@mellanox.com>; Wenzhuo Lu >>>>>> <wenzhuo...@intel.com>; Jingjing Wu <jingjing...@intel.com> >>>>>> Cc: dev@dpdk.org; sta...@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 <ma...@mellanox.com>; Wenzhuo Lu >>>>>>>> <wenzhuo...@intel.com>; Jingjing Wu <jingjing...@intel.com> >>>>>>>> Cc: dev@dpdk.org; sta...@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: sta...@dpdk.org >>>>>>>>> >>>>>>>>> Signed-off-by: Matan Azrad <ma...@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.
+1 So what about having the log message but not implicitly update the offload config? > > 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. > >