mbox series

[v6,0/2] net: introduce IPv4 ihl and version fields

Message ID 20211013171354.27817-1-getelson@nvidia.com (mailing list archive)
Headers
Series net: introduce IPv4 ihl and version fields |

Message

Gregory Etelson Oct. 13, 2021, 5:13 p.m. UTC
  Gregory Etelson (2):
  net: fix IPv4 change announce
  net: introduce IPv4 ihl and version fields

 app/test/test_flow_classify.c          |  8 ++++----
 doc/guides/rel_notes/deprecation.rst   |  6 ------
 doc/guides/rel_notes/release_21_11.rst |  3 +++
 lib/net/rte_ip.h                       | 16 +++++++++++++++-
 4 files changed, 22 insertions(+), 11 deletions(-)
  

Comments

Ferruh Yigit Oct. 14, 2021, 8:21 a.m. UTC | #1
On 10/13/2021 6:13 PM, Gregory Etelson wrote:
> Gregory Etelson (2):
>    net: fix IPv4 change announce
>    net: introduce IPv4 ihl and version fields
> 

Hi Gregory,

Can you please change the order of the first and second patch?

This way I can get the first one, since it is already acked, before -rc1,
and continue reviews for second one, it will be OK since it is a
doc patch.

Thanks,
ferruh
  
Thomas Monjalon Oct. 14, 2021, 8:30 a.m. UTC | #2
14/10/2021 10:21, Ferruh Yigit:
> On 10/13/2021 6:13 PM, Gregory Etelson wrote:
> > Gregory Etelson (2):
> >    net: fix IPv4 change announce
> >    net: introduce IPv4 ihl and version fields
> > 
> 
> Hi Gregory,
> 
> Can you please change the order of the first and second patch?
> 
> This way I can get the first one, since it is already acked, before -rc1,
> and continue reviews for second one, it will be OK since it is a
> doc patch.

It makes more sense in this order I think.
The first patch is just dropping a deprecation note, I can ack.
  
Gregory Etelson Oct. 14, 2021, 9:29 a.m. UTC | #3
Hello Ferruh,

> On 10/13/2021 6:13 PM, Gregory Etelson wrote:
> > Gregory Etelson (2):
> >    net: fix IPv4 change announce
> >    net: introduce IPv4 ihl and version fields
> >
> 
> Hi Gregory,
> 
> Can you please change the order of the first and
> second patch?
> 

In the existing order, the code patch reflects announced changes.

> This way I can get the first one, since it is already
> acked, before -rc1,
> and continue reviews for second one, it will be
> OK since it is a
> doc patch.
> 
> Thanks,
> ferruh
  
Ferruh Yigit Oct. 14, 2021, 11:01 a.m. UTC | #4
On 10/14/2021 9:30 AM, Thomas Monjalon wrote:
> 14/10/2021 10:21, Ferruh Yigit:
>> On 10/13/2021 6:13 PM, Gregory Etelson wrote:
>>> Gregory Etelson (2):
>>>     net: fix IPv4 change announce
>>>     net: introduce IPv4 ihl and version fields
>>>
>>
>> Hi Gregory,
>>
>> Can you please change the order of the first and second patch?
>>
>> This way I can get the first one, since it is already acked, before -rc1,
>> and continue reviews for second one, it will be OK since it is a
>> doc patch.
> 
> It makes more sense in this order I think.
> The first patch is just dropping a deprecation note, I can ack.
> 

It will be same I think, first patch can have implementation and remove
the implemented part of the deprecation notice,
remaining deprecation notice part can be removed with or without its
implementation later.

Anyway, this is for operational needs, if the first patch gets enough
ack/review timely, not update is required and I can get both patches.
  
Ferruh Yigit Oct. 14, 2021, 11:10 a.m. UTC | #5
On 10/14/2021 10:29 AM, Gregory Etelson wrote:
> Hello Ferruh,
> 
>> On 10/13/2021 6:13 PM, Gregory Etelson wrote:
>>> Gregory Etelson (2):
>>>     net: fix IPv4 change announce
>>>     net: introduce IPv4 ihl and version fields
>>>
>>
>> Hi Gregory,
>>
>> Can you please change the order of the first and
>> second patch?
>>
> 
> In the existing order, the code patch reflects announced changes.
> 

Overall you are not implementing the announced changes fully, but partially.

Question is how to manage announced but not implemented part. I am for
separating discussion of that part from the code change that already acked.

>> This way I can get the first one, since it is already
>> acked, before -rc1,
>> and continue reviews for second one, it will be
>> OK since it is a
>> doc patch.
>>
>> Thanks,
>> ferruh