Add support for TCP_CONGESTION socketopt

[BobAnkh]

• tcp_congestion uses raw syscall since diff os have diff policy on modifying len
• set_tcp_congestion uses raw syscall since exact str len must be passed in

Closes: #370

Co-authored-by: Campbell He kp.campbell.he@duskmoon314.com

[BobAnkh]

Currently have no idea why FreeBSD throws an error on the code debug_assert_eq!(len as usize, size_of::<T>());(src/sys/unix.rs:980:9) since let mut len = size_of::<T>() as libc::socklen_t;.

Woking on this now. Any suggestion?

[BobAnkh]

I have fixed the error thrown by FreeBSD. This is because that FreeBSD will modify the len field to the exact string length while Linux not.

Now this pr is ready for review.

[BobAnkh]

Code has been polished again, ready for another review!

[Thomasdezeeuw]

This is UB. Only &buf[0..len] bytes are initialised by the kernel.

[Thomasdezeeuw]

This is not resolved. You can't call payload.assume_init, it's not fully initialised. I think you can simply remove this line and we're good.

[BobAnkh]

Ok so now I use directly [u8; TCP_CA_NAME_MAX](initialized as [0; TCP_CA_NAME_MAX]) instead of MaybeUninit

[Thomasdezeeuw]

Not what I asked for, but fine for now.

Allow me to explain the UB in the old code one more time. You create an array of size TCP_CA_NAME_MAX, accessing any of those by would be UB. Then you call the kernel to fill the bytes, but it doesn't fill all of them just n bytes (return by the system call). With the old code you called payload.assume_init(), which requires the entire array all (TCP_CA_NAME_MAX bytes) to be initialised, but that's not always the case as it's only up to n bytes.

So the only change needed to the old code was to remove the payload.assume_init() line. Because the next line creating a slice of only the initialised bytes and the line after that cast it to a slice of bytes (MaybeUninit<u8> -> u8).

[BobAnkh]

Some modification:

• New tcp ca test is added
• tcp_congestion return the untruncated u8 buf[..len]

[BobAnkh]

Execute the command sysctl net.inet.tcp.cc.available on FreeBSD CI, which shows that it only supports newreno. This might be because other cc modules are not loaded.

So now I only test newreno on FreeBSD. Any suggestion?

[BobAnkh]

I have a suggestion that can we use OsString instead as the crate nix does? Accepting a &[u8] and especially returning the Vec with trailing \0s seems not so user-friendly I'm afraid. Up to you to confirm which interface you prefer.

Also, newreno is the only ca we can test on CI's FreeBSD.

So any ideas on the two points? Or is there anything else I should implement?

[Thomasdezeeuw]

So now I only test newreno on FreeBSD. Any suggestion?

That's fine, let's just set newreno again.

I have a suggestion that can we use OsString instead as the crate nix does? Accepting a &[u8] and especially returning the Vec with trailing \0s seems not so user-friendly I'm afraid. Up to you to confirm which interface you prefer.

No, we're already using Vec<u8>/&[u8] other places. Let's stick with a consistent API.

[BobAnkh]

Get your point. Then I've implemented all requested.

[Thomasdezeeuw]

Still has UB.

[Thomasdezeeuw]

This is not resolved. You can't call payload.assume_init, it's not fully initialised. I think you can simply remove this line and we're good.

[Thomasdezeeuw]

Not what I asked for, but fine for now.

Allow me to explain the UB in the old code one more time. You create an array of size TCP_CA_NAME_MAX, accessing any of those by would be UB. Then you call the kernel to fill the bytes, but it doesn't fill all of them just n bytes (return by the system call). With the old code you called payload.assume_init(), which requires the entire array all (TCP_CA_NAME_MAX bytes) to be initialised, but that's not always the case as it's only up to n bytes.

So the only change needed to the old code was to remove the payload.assume_init() line. Because the next line creating a slice of only the initialised bytes and the line after that cast it to a slice of bytes (MaybeUninit<u8> -> u8).

[Thomasdezeeuw]

Thanks @BobAnkh