Re: [dpdk-dev] [PATCH] net/mlx5: txq_inline_min not set for ConnectX-5 adapters

[Slava Ovsiienko <viacheslavo@mellanox.com>]

> -----Original Message-----
> From: David Christensen <drc@linux.vnet.ibm.com>
> Sent: Friday, August 2, 2019 21:18
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] net/mlx5: txq_inline_min not set for ConnectX-5
> adapters
> 
> >>> Yes, thank you for the patch, acked.
> >>>
> >>> The "txq_inline_min" value was not set for ConnectX-5 to 0 as default.
> >>> At the TX queue setup time the "txq_inline_min" is checked against
> >>> MLX5_ARG_UNSET and default value 0 is set:
> >>>
> >>> txq_set_params()
> >>> ...
> >>> inlen_mode = (config->txq_inline_min == MLX5_ARG_UNSET) ?
> >>>                        0 : (unsigned int)config->txq_inline_min;
> >>>
> >>> So, there should be no negative backwards. Did you observe any?
> >>
> >> The gdb session below shows how the value is unchanged after passing
> >> through mlx5_set_min_inline on my system without the change.
> >
> > Yes, mlx5_set_min_inline() does not set txq_inline_min by default.
> > So, your patch is OK.
> >
> > I mean the actual inline_len is set to 0 before actual usage in
> > txq_set_params() routine, so not setting config->txq_inline_min to
> > default zero should not cause negative backwards.
> 
> So that implies the assert to verify that txq_inline_min >= 0 in
> mlx5_set_txlimit_params() is unnecessary.  Should the patch change and just
> remove the assert?
I think no need to change the patch. If  I understand correctly this assert helped to
catch missing line in mlx5_set_min_inline() which your patch fixes.
If so - assert works well and should be kept, IMHO.

Also, please, delegate your patch to Raslan (http://patches.dpdk.org/project/dpdk/list/).

Thanks, Slava

> 
> I think the patch is the more logical solution, setting the value at the source
> similar to what's done for all other adapters.
> 
> Dave