Hi Ferruh From: Yigit, Ferruh <ferruh.yi...@linux.intel.com> > 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? >
Yes, we need this massage at least. Will work on it. Thanks.