Re: [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 10/10/2018 11:57 AM, Ferruh Yigit wrote:
> On 10/10/2018 6:57 AM, Shahaf Shuler wrote:
>> Tuesday, October 9, 2018 6:49 PM, Ferruh Yigit:
>>> Subject: Re: [dpdk-dev] [PATCH 0/7] net/mlx5: fixes for the new flow engine
>>>
>>> On 10/9/2018 9:58 AM, Shahaf Shuler wrote:
>>>> Monday, October 8, 2018 9:02 PM, Yongseok Koh:
>>>>> Subject: [PATCH 0/7] net/mlx5: fixes for the new flow engine
>>
>> [...]
>>
>>>>> djWRGvBzaqpfUVsn8%3D&reserved=0
>>>>>
>>>>> Yongseok Koh (7):
>>>>>   net/mlx5: fix wrong flow action macro usage
>>>>>   net/mlx5: use standard IP protocol numbers
>>>>>   net/mlx5: rename flow macros
>>>>>   net/mlx5: fix validation of VLAN ID in flow spec
>>>>>   net/mlx5: fix flow validation for no fate action
>>>>>   net/mlx5: add missing VLAN action constraints
>>>>>   net/mlx5: fix errno values for flow engine
>>>>>
>>>>>  drivers/net/mlx5/mlx5_flow.c       | 117 +++++++++++++++++++-----------
>>> ----
>>>>> ---
>>>>>  drivers/net/mlx5/mlx5_flow.h       |  54 ++++++++---------
>>>>>  drivers/net/mlx5/mlx5_flow_dv.c    |  30 +++++-----
>>>>>  drivers/net/mlx5/mlx5_flow_tcf.c   |  78 +++++++++++++++++++------
>>>>>  drivers/net/mlx5/mlx5_flow_verbs.c |  59 ++++++++++---------
>>>>>  5 files changed, 193 insertions(+), 145 deletions(-)
>>>>
>>>> Series applied to next-net-mlx, thanks.
>>>> Need to add the missing VLAN limitation of the "pop always to non-uplink"
>>> on a separate commit, don't want to stall the entire series.
>>>
>>> Hi Shahaf,
>>>
>>> These are fixing mlx5 patches which are merged very recently, that is why I
>>> tried to squash these to original commit. This is both for more clean git
>>> history and to have correct Fixes information in commit logs.
>>
>> I am not sure I agree here. There was a feature which was accepted and later on it had some bug fixes. 
>> I think it is better to separate between the two, because:
>> 1. I don't think it spams that much the git history
>> 2. if the small fix raised a different bug it will be easier to bisect and track it rather than trying to check what is wrong on the big feature patch
> 
> I would agree but for this case original feature is too fresh. I think it is
> opportunity to merge it into original feature. Eventually final code will be
> same, if there is a bug it will be in both ways, just to improve the history.
> 
> And I didn't understand why it is better to "fix the fix" instead of merge the
> fix to the original feature and "fix the feature" if the fix is wrong.
> 
> Also this "fix the fix" chains may make reading/understanding original code
> harder in the future.
> 
>>
>>>
>>> I can able to squash: 1/7, 2/7 & 4/7
>>>
>>> But 4 are still remaining, main reason is they fixes more than one commit so
>>> not easy to squash into one.
>>>
>>> I won't merge remaining ones for now, can you please rebase them on top of
>>> next-net, and try to arrange in a way to squash into next-net, if not able to
>>> make we can get them as it is.
>>
>> If it is not critical for you, I suggest we take them as is. It will require some work to re-arrange them + test again.
>> Let me know. 
> 
> No it is not critical, but again patchset doesn't look like too big, so if it is
> not too much effort I prefer squashing them. And as a final result code should
> be exact same, so testing effort shouldn't change but re-arrange takes effort, I
> already did a few of them for you...

All squashed except from 6/7 & "7/7 partially", I can confirm final code has not
changed but please double check latest next-net head.