On Fri, Jul 19, 2024 at 12:02:38PM +0200, Robin Jarry wrote: > 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. > > > 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? >
+1 for option 1 - keep minimally aligned type. Having two types would be confusing. /Bruce
