> From: Robin Jarry [mailto:rja...@redhat.com]
>
> Morten Brørup, Jul 19, 2024 at 11:12:
> > > Vladimir Medvedkin, Jul 18, 2024 at 23:25:
> > > > I think alignment should be 1 since in FIB6 users usually don't
> > > > copy IPv6 address and just provide a pointer to the memory inside
> > > > the packet.
> >
> > How can they do that? The bulk lookup function takes an array of IPv6
> > addresses, not an array of pointers to IPv6 addresses.
> >
> > What you are suggesting only works with single lookup, not bulk
> > lookup.
>
> Indeed for bulk lookup, you need to copy addresses on the stack.
>
> > > Yes, my intention was exactly that, being able to map that structure
> > > directly in packets without copying them on the stack.
> >
> > This would require changing the bulk lookup API to take an array of
> > pointers instead of an array of IPv6 addresses.
> >
> > It would be acceptable to introduce a new single address lookup
> > function, taking a pointer to an unaligned (or 2 byte aligned) IPv6
> > address for the single lookup use cases mentioned above.
>
> That would require two different IPv6 structures. I would prefer it we
> could avoid that. Or the unaligned lookup API needs to take a simple
> `const uint8_t *` parameter.
>
> > I'm not worried about the IPv6 FIB functions. This proposal introduces
> > a generic IPv6 address type for *all of DPDK*, so you need to consider
> > *all* aspects, not just one library!
> >
> > There may be current or future CPUs, where alignment makes
> > a performance difference. Do all architectures support unaligned 128
> > bit access at 100 % similar performance to aligned 128 bit access?
> > I think not!
> > E.g. on X86 architecture, load/store across a cache boundary has
> > a performance impact. If the type is explicitly unaligned, an instance
> > on the stack (i.e. a local variable holding an IPv6 address) might
> > cross a cache boundary, whereas an 128 bit aligned instance on the
> > stack is guaranteed not to cross a cache boundary.
> >
> > The generic IPv4 address type is natively aligned (i.e. 4 byte). When
> > accessing an IPv4 address in an IPv4 header following an Ethernet
> > header, it is not 4 byte aligned, so this is an *exception* from the
> > general case, and must be treated as such. You don't want to make the
> > general type unaligned (and thus inefficient) everywhere it is being
> > used, only because a few use cases require the unaligned form.
>
> I think the main difference is that you almost never pass IPv4 addresses
> as reference but always as values. So alignment does not matter.
When passing an IPv4 address as a value, alignment does matter; it must be 4
byte aligned.
On a CPU with 128 bit registers, I would probably also pass an IPv6 address as
a value. With such a CPU, the parameter type should be uint128_t or
rte_be128_t, depending on byte order.
>
> > The same principle must apply to the IPv6 address type. Let's make the
> > generic type natively aligned (16 byte). And you might also offer an
> > explicitly unaligned type for the exception use cases requiring
> > unaligned access.
>
> The main issue with this is that you would not be able to use that type
> in the IPv6 header structure to map it to mbuf data. That leaves us with
> two options:
>
> 1) Keep a single unaligned IPv6 type and hope for the best with
> performance. It will not be different from the current state of
> things where every IPv6 is a uint8_t pointer.
>
> 2) Have two IPv6 types, one 16 bytes aligned, and another one 1 byte
> aligned. The main issue with that second approach is that users may
> get confused about which one to use and when.
>
> I would prefer to keep it simple at first and go with option 1). We can
> always revisit that later and introduce an aligned IPv6 type for certain
> use cases.
>
> What do you think?
There's a 3rd option:
Have an IPv6 type that is simply an array of 16 bytes with no explicitly
specified alignment:
struct rte_ipv6_addr {
unsigned char addr_bytes[RTE_IPV6_ADDR_LEN];
};
Or:
typedef struct rte_ipv6_addr {
unsigned char addr_bytes[RTE_IPV6_ADDR_LEN];
} rte_ipv6_addr_t;
If used as is, it will be unaligned.
And if alignment offers improved performance for some use cases, explicit
alignment attributes can be added to the type in those use cases.
Not using an uint128_t type (or a union of other types than unsigned char) will
also avoid byte order issues.
I guess Stephen was right to begin with. :-)
