> -----Original Message----- > From: Ferruh Yigit <ferruh.yi...@intel.com> > Sent: Thursday, January 9, 2020 12:50 > To: Slava Ovsiienko <viachesl...@mellanox.com>; dev@dpdk.org > Cc: Matan Azrad <ma...@mellanox.com>; Raslan Darawsheh > <rasl...@mellanox.com>; Ori Kam <or...@mellanox.com>; sta...@dpdk.org; > Thomas Monjalon <tho...@monjalon.net> > Subject: Re: [dpdk-stable] [PATCH] net/mlx5: fix ConnectX-4LX Tx burst > routines set > > On 1/9/2020 9:03 AM, Slava Ovsiienko wrote: > >> -----Original Message----- > >> From: Ferruh Yigit <ferruh.yi...@intel.com> > >> Sent: Wednesday, January 8, 2020 17:55 > >> To: Slava Ovsiienko <viachesl...@mellanox.com>; dev@dpdk.org > >> Cc: Matan Azrad <ma...@mellanox.com>; Raslan Darawsheh > >> <rasl...@mellanox.com>; Ori Kam <or...@mellanox.com>; > >> sta...@dpdk.org; Thomas Monjalon <tho...@monjalon.net> > >> Subject: Re: [dpdk-stable] [PATCH] net/mlx5: fix ConnectX-4LX Tx > >> burst routines set > >> > >> On 1/8/2020 3:50 PM, Slava Ovsiienko wrote: > >>> Hi, Ferruh > >>> > >>>> -----Original Message----- > >>>> From: Ferruh Yigit <ferruh.yi...@intel.com> > >>>> Sent: Wednesday, January 8, 2020 16:55 > >>>> To: Slava Ovsiienko <viachesl...@mellanox.com>; dev@dpdk.org > >>>> Cc: Matan Azrad <ma...@mellanox.com>; Raslan Darawsheh > >>>> <rasl...@mellanox.com>; Ori Kam <or...@mellanox.com>; > >>>> sta...@dpdk.org; Thomas Monjalon <tho...@monjalon.net> > >>>> Subject: Re: [dpdk-stable] [PATCH] net/mlx5: fix ConnectX-4LX Tx > >>>> burst routines set > >>>> > >>>> On 1/8/2020 2:53 PM, Ferruh Yigit wrote: > >>>>> On 12/20/2019 10:48 AM, Viacheslav Ovsiienko wrote: > >>>>>> The tx_burst routine supporting multi-segment packets with legacy > >>>>>> MPW and without inline was missed, and there was no valid > >>>>>> selection for these options, patch adds the missing routine. > >>>>>> > >>>>>> Fixes: 82e75f8323bf ("net/mlx5: fix legacy multi-packet Tx > >>>>>> descriptors") > >>>>>> Cc: sta...@dpdk.org > >>>>>> > >>>>>> Signed-off-by: Viacheslav Ovsiienko <viachesl...@mellanox.com> > >> > >> <...> > >> > >>>>>> @@ -5297,6 +5305,7 @@ enum mlx5_txcmp_code { > >>>>>> DRV_LOG(DEBUG, "port %u has no selected Tx > function" > >>>>>> " for requested offloads %04X", > >>>>>> dev->data->port_id, olx); > >>>>>> + assert(false); > >> > >> <...> > >> > >>> > >>>> > >>>>> > >>>>> I think we should avoid PMDs calling the assert unconditionally, > >>>>> specially in a code that debug level log is printed. > >>>>> > >>>>>> return NULL; > >>>>>> } > >>>>>> DRV_LOG(DEBUG, "port %u has selected Tx function" > >>> > >>> Yes, I agree. We just do not have the check for the result returned > >>> by mlx5_select_tx_function(). I think we should check against NULL > >>> and report an error. "assert" is a temporary solution till this > >>> upgrade (in debug mode we have a lot of messages and break on assert > >>> helps to locate the problem quickly, reporting error will do the same). > >>> > >> > >> Can it be possible to drop the patch from mlx tree and prepare a new > >> version without 'assert'? > > The selection routine error handling is rather generic and is not merely > related to ConnectX-4LX. > > I propose to prepare the dedicated patch, what do you think? > > > > My concern is with the assert, the error handling can be another patch, but > can we have this change without an assert?
Yes, please: http://patches.dpdk.org/patch/64340/ With best regards, Slava PS. Removing this assert urges me to add error handling ASAP 😊