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.
