Implement IPv6

[june-fish]

No description provided.

[sidd-lotlikar]

I would not recommend using bitfields for version, traffic class, and flow label, as bitfields cannot guarantee layout or endianness across compilers and architectures. The compiler may reorder the version and traffic_class in ways that do not match the network-byte order required by IPv6.

I would pack version, traffic class, and flow label into one u32 field.

u32 ver_tc_fl;        // version(4) | tc(8) | flow(20)

Then, we can extract the values manually using bitwise operations. This also makes struct padding much better.

[remexre]

I think using bitfields in general is good and fine; the portability argument is kinda ehhhhh:

• Bitfield layout rules are part of the ABI, so compilers can't easily change the layout without breaking lots of applications; see this article for a bit more on that.
• We only support GCC right now (we'll probably support Clang in the near future, but there's C23 features they're missing), and GNU documents its rules.
• Because they're part of the ABI, the platform actually documents them; for RISC-V, that's section 2.1 of the psABI.
  Those rules are the same rules GCC implements.

That said, this particular code is busted; I think you want something like:

struct ipv6_header {
  // If you want these to be packed together, they need to be the same type.
  u32 version : 4;
  u32 traffic_class : 8;
  u32 flow_label : 20;

  u16 payload_length;
  u8 next_header;
  u8 hop_limit;

  // We don't have u128 in the kernel; in principle you could use _BitInt(128),
  // but 1) I'm trying to keep us away from _BitInt, because there's tricky
  // questions about what parts of the spec get implemented by the compiler vs
  // the libc (i.e., us) that the LLVM and GCC people aren't in agreement about
  // yet, and 2) alignment concerns, see below.
  u8 src_address[16];
  u8 dst_address[16];
};

// Have one of these whenever it matters.
static_assert(sizeof(struct ipv6_header) == 40);
// Oops, 4 is maybe not what we want. See below.
static_assert(alignof(struct ipv6_header) == 4);

The tricky thing here is if we use u32 (either the bitfield type as I wrote or the "bitfield implemented by hand" version), we end up with 4-byte alignment for this struct.
Except... an Ethernet frame's payload (e.g. the IPv6 packet) is either 14 or 18 bytes after the start of the field, i.e. only 2-byte aligned.

It seems gross to split up the u32 into two u16s and then have to stitch them back together if we're doing bitfields.

I suppose it depends how we're actually doing field access:

• If we memcpy the header bytes into an aligned local buffer on the stack and access them, GCC is smart enough to say "actually, I'll just do an unaligned access / do the bitfield math myself."
  It's even smart enough to choose which one of those two it does based on how performant unaligned accesses are on the underlying hardware.
  (e.g. on our hardware, an unaligned access that doesn't cross cache-line boundaries is zero-cost, whereas on a Sifive P550, it's hundreds of times more expensive).

  This lets us keep the same structure definition and avoid UB, but people have to not cast u8 * to struct ipv6_header *.

• If we have macros wrapping accesses, they can just read the right bytes out as u16s or what-not and reassemble them prior to doing bitwise math.

  Folks will need to know to use these macros, but the fields not lining up with what people expect would hopefully cause them to check what the deal is and find docs in the headers.

• Ethernet device drivers on Linux make the guarantee that the IP header is 16-byte aligned.
  We could make the same guarantee.

  This is an extra gotcha when writing those drivers, but there'll probably be fewer of them than there are places that access Ethernet frames' payloads.

  (LWN makes it sound like some DMA controllers also had problems with this 40 years ago, but ours doesn't, so I'm inclined to ignore that.)

...sorry for the essay.

[sidd-lotlikar]

struct ipv6_header {
    u8 version_tc_fl[4]; 
    u16 payload_length;
    u8  next_header;
    u8  hop_limit;
    u8 src_address[16];
    u8 dst_address[16];
};

So, wouldn't this work for 2-byte alignment? The memory management is more convenient. The tradeoff is that we have to do more manual work, and it will be slightly slower than what you wrote. I think memcpy would be safe with this struct. I am new to C, so I may make mistakes with the code I wrote (I hope you get the general idea), but what do you think about this idea?

version_tc_fl[0]  // version + upper 4 bits is traffic class
version_tc_fl[1]  // traffic class + upper 4 bits is flow label
version_tc_fl[2]  // flow label
version_tc_fl[3]  // flow label

To extract fields (to get version, traffic class, and flow label):

u8 version = version_tc_fl[0] >> 4;
u8 traffic_class = ((version_tc_fl[0] & 0x0F) << 4) | (version_tc_fl[1] >> 4);
u32 flow_label = ((version_tc_fl[1] & 0x0F) << 16) | (version_tc_fl[2] << 8) | version_tc_fl[3];

To write values back:

version_tc_fl[0] = (version << 4) | (traffic_class >> 4);
version_tc_fl[1] = ((traffic_class & 0x0F) << 4) | ((flow_label >> 16) & 0x0F);
version_tc_fl[2] = (flow_label >> 8) & 0xFF;
version_tc_fl[3] = flow_label & 0xFF;

[remexre]

Yeah, those expressions are doing the reassembling that'd be in the macros / static inline functions / whatever in option 2.

The downside is just that they're less readable than just being able to do e.g. hdr->flow_label.

[jastintime]

rebased off trunk

[jastintime]

ok rebased off actual current trunk fr this time

[jastintime]

ready for review, ideally #81 should be merged first.

[remexre]

look into making this a bitfield(trash)

[jastintime]

depends on #73 being merged, should be good now to look over.

changing it back to bitfields and getting rid of 9088ec1#diff-6073ec55fe8e2da8a08b9d5d0ac4f54470e13b64d8a357b95a319c1f9c1e6188R15. would be a good idea.

78ed7d9 will also be dropped before merging.