Re: IPv6 APIs rework

["Robin Jarry" <rjarry@redhat.com>]

Hi Morten, Stephen,

Morten Brørup, Jul 21, 2024 at 18:12:
> If the IPv6 address type you tested with was a struct containing 
> a union of different types (other than an array of 16 bytes), then 
> those sub-types made your IPv6 address type non-byte aligned, and 
> caused padding when used in other structures.
>
> Please try again with the simple array type:
> struct rte_ipv6_addr { unsigned char addr_bytes[16]; };
>
> This should not cause any padding when used in other structures, 
> except if used with alignas().

Indeed removing the sub-types in the union removes the need for strict 
alignment and packing.

Too bad, I found these intermediate integers made the code a bit nicer 
but I can understand that it brings a lot of trouble down the line.

NB: I tried uint8_t vs unsigned char, it makes no difference with 
implicit casting to (uint16_t *) or (uint32_t *). Explicit casting is 
required anyway.

> If you are introducing an official IPv6 address type into DPDK, its 
> scope it not just the FIB6 API.
>
> Both Stephen and I can see that - in a broader perspective - the 
> packed and unaligned constraints are unacceptable for performance.
>
> It might not be a problem for the current FIB6 implementation, but it 
> *will* be a problem in many other places, if converted to using the 
> new IPv6 address type.
>
> PS:
> I do consider adding a dedicated IPv6 address type to DPDK an 
> improvement over the current convention of using an uint8_t[16] array.
> But we need to agree on the type, which must work optimally for 
> a broad spectrum of use cases. Otherwise, the new type is not an 
> improvement, but a deterioration of DPDK.

OK, I understand the stakes. I will comply and propose a simple struct 
without any packing nor explicit alignment.

    struct rte_ipv6_addr {
        union {
            unsigned char a[RTE_IPV6_ADDR_SIZE];
        };
    };

I have left the door open in order to ease adding sub-types in the 
future. Indeed, lpm6/fib6 tests rely on literal definitions of IPv6 
addresses and union types need an extra set of curly braces for literal 
definitions. If you think we will never need to add sub-types, I can get 
rid of this. It makes no difference at runtime.

About the timing: when should I send a patch to announce IPv6 API 
breakage for 24.11?

Thanks for taking the time.
Cheers.