diff mbox series

[v6,1/2] net: fix IPv4 change announce

Message ID 20211013171354.27817-2-getelson@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net: introduce IPv4 ihl and version fields |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Gregory Etelson Oct. 13, 2021, 5:13 p.m. UTC 
  IPv4 header encodes fragment information into 16 bits field.
3 bits hold flags and remaining 13 bits are for fragment offset.
13 bits bit-field cannot be defined both for big and little endian
systems.

The patch removes IPv4 fragments union announce.

Fixes: f7383e7c7ec1 ("net: announce changes in IPv4 header access")

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
---
 doc/guides/rel_notes/deprecation.rst | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Thomas Monjalon Oct. 14, 2021, 8:37 a.m. UTC | #1 
13/10/2021 19:13, Gregory Etelson:
> IPv4 header encodes fragment information into 16 bits field.
> 3 bits hold flags and remaining 13 bits are for fragment offset.
> 13 bits bit-field cannot be defined both for big and little endian
> systems.
> 
> The patch removes IPv4 fragments union announce.
> 
> Fixes: f7383e7c7ec1 ("net: announce changes in IPv4 header access")
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>

OK to drop this announce.
There is no implementation anyway,
it will be back in one year if there is a solution.

Acked-by: Thomas Monjalon <thomas@monjalon.net>
Ferruh Yigit Oct. 14, 2021, 11:03 a.m. UTC | #2 
On 10/14/2021 9:37 AM, Thomas Monjalon wrote:
> 13/10/2021 19:13, Gregory Etelson:
>> IPv4 header encodes fragment information into 16 bits field.
>> 3 bits hold flags and remaining 13 bits are for fragment offset.
>> 13 bits bit-field cannot be defined both for big and little endian
>> systems.
>>
>> The patch removes IPv4 fragments union announce.
>>
>> Fixes: f7383e7c7ec1 ("net: announce changes in IPv4 header access")
>>
>> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> 
> OK to drop this announce.
> There is no implementation anyway,
> it will be back in one year if there is a solution.
> 

If there is an option to have it back, why not keep it in the deprecation
notice, this ensures we won't forgot it.

> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 
>
Gregory Etelson Oct. 14, 2021, 12:21 p.m. UTC | #3 
Hello Ferruh,

> On 10/14/2021 9:37 AM, Thomas Monjalon
> wrote:
> > 13/10/2021 19:13, Gregory Etelson:
> >> IPv4 header encodes fragment information
> into 16 bits field.
> >> 3 bits hold flags and remaining 13 bits are for
> fragment offset.
> >> 13 bits bit-field cannot be defined both for big
> and little endian
> >> systems.
> >>
> >> The patch removes IPv4 fragments union
> announce.
> >>
> >> Fixes: f7383e7c7ec1 ("net: announce changes
> in IPv4 header access")
> >>
> >> Signed-off-by: Gregory Etelson
> <getelson@nvidia.com>
> >
> > OK to drop this announce.
> > There is no implementation anyway,
> > it will be back in one year if there is a solution.
> >
> 
> If there is an option to have it back, why not
> keep it in the deprecation
> notice, this ensures we won't forgot it.
>

It doesn't look that 3/13 bits split can be implemented as

#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
	/* LE bit-fields */
# elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
	/* BE bit-fields */
#endif
 
The minimal bits ratio for that option is 4/12.

> > Acked-by: Thomas Monjalon
> <thomas@monjalon.net>
> >
> >
Ferruh Yigit Oct. 14, 2021, 12:32 p.m. UTC | #4 
On 10/14/2021 1:21 PM, Gregory Etelson wrote:
> Hello Ferruh,
> 
>> On 10/14/2021 9:37 AM, Thomas Monjalon
>> wrote:
>>> 13/10/2021 19:13, Gregory Etelson:
>>>> IPv4 header encodes fragment information
>> into 16 bits field.
>>>> 3 bits hold flags and remaining 13 bits are for
>> fragment offset.
>>>> 13 bits bit-field cannot be defined both for big
>> and little endian
>>>> systems.
>>>>
>>>> The patch removes IPv4 fragments union
>> announce.
>>>>
>>>> Fixes: f7383e7c7ec1 ("net: announce changes
>> in IPv4 header access")
>>>>
>>>> Signed-off-by: Gregory Etelson
>> <getelson@nvidia.com>
>>>
>>> OK to drop this announce.
>>> There is no implementation anyway,
>>> it will be back in one year if there is a solution.
>>>
>>
>> If there is an option to have it back, why not
>> keep it in the deprecation
>> notice, this ensures we won't forgot it.
>>
> 
> It doesn't look that 3/13 bits split can be implemented as
> 
> #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> 	/* LE bit-fields */
> # elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> 	/* BE bit-fields */
> #endif
>   
> The minimal bits ratio for that option is 4/12.
> 

I got your point, no objection from me there.

So we want to completely abandon the change, and I expect people who acked
the deprecation notice at first place would like to ack dismissing it.

>>> Acked-by: Thomas Monjalon
>> <thomas@monjalon.net>
>>>
>>>
>
Akhil Goyal Oct. 14, 2021, 12:53 p.m. UTC | #5 
> 13/10/2021 19:13, Gregory Etelson:
> > IPv4 header encodes fragment information into 16 bits field.
> > 3 bits hold flags and remaining 13 bits are for fragment offset.
> > 13 bits bit-field cannot be defined both for big and little endian
> > systems.
> >
> > The patch removes IPv4 fragments union announce.
> >
> > Fixes: f7383e7c7ec1 ("net: announce changes in IPv4 header access")
> >
> > Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> 
> OK to drop this announce.
> There is no implementation anyway,
> it will be back in one year if there is a solution.
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Akhil Goyal <gakhil@marvell.com>
Ori Kam Oct. 14, 2021, 1:34 p.m. UTC | #6 
Hi

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> Sent: Thursday, October 14, 2021 3:33 PM
> Subject: Re: [dpdk-dev] [PATCH v6 1/2] net: fix IPv4 change announce
> 
> On 10/14/2021 1:21 PM, Gregory Etelson wrote:
> > Hello Ferruh,
> >
> >> On 10/14/2021 9:37 AM, Thomas Monjalon
> >> wrote:
> >>> 13/10/2021 19:13, Gregory Etelson:
> >>>> IPv4 header encodes fragment information
> >> into 16 bits field.
> >>>> 3 bits hold flags and remaining 13 bits are for
> >> fragment offset.
> >>>> 13 bits bit-field cannot be defined both for big
> >> and little endian
> >>>> systems.
> >>>>
> >>>> The patch removes IPv4 fragments union
> >> announce.
> >>>>
> >>>> Fixes: f7383e7c7ec1 ("net: announce changes
> >> in IPv4 header access")
> >>>>
> >>>> Signed-off-by: Gregory Etelson
> >> <getelson@nvidia.com>
> >>>
> >>> OK to drop this announce.
> >>> There is no implementation anyway,
> >>> it will be back in one year if there is a solution.
> >>>
> >>
> >> If there is an option to have it back, why not keep it in the
> >> deprecation notice, this ensures we won't forgot it.
> >>
> >
> > It doesn't look that 3/13 bits split can be implemented as
> >
> > #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> > 	/* LE bit-fields */
> > # elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> > 	/* BE bit-fields */
> > #endif
> >
> > The minimal bits ratio for that option is 4/12.
> >
> 
> I got your point, no objection from me there.
> 
> So we want to completely abandon the change, and I expect people who acked the deprecation
> notice at first place would like to ack dismissing it.
> 
> >>> Acked-by: Thomas Monjalon
> >> <thomas@monjalon.net>
> >>>
> >>>
> >
Acked-by: Ori Kam <orika@nvidia.com>
Best,
Ori
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 309f1056cf..841653fe30 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -158,11 +158,8 @@  Deprecation Notices
   consistent with existing outer header checksum status flag naming, which
   should help in reducing confusion about its usage.
 
-* net: The structure ``rte_ipv4_hdr`` will have two unions.
-  The first union is for existing ``version_ihl`` byte
-  and new bitfield for version and IHL.
-  The second union is for existing ``fragment_offset``
-  and new bitfield for fragment flags and offset.
+* net: The structure ``rte_ipv4_hdr`` will have a union for
+  existing ``version_ihl`` byte and new bitfield for ``version`` and ``ihl``.
 
 * vhost: ``rte_vdpa_register_device``, ``rte_vdpa_unregister_device``,
   ``rte_vhost_host_notifier_ctrl`` and ``rte_vdpa_relay_vring_used`` vDPA