netinet6: In tcp_lro_rx_ipv6(), check for IPv6 flow ID.

[nc]

Summary

netinet6: In tcp_lro_rx_ipv6(), check for IPv6 flow ID.

Submitted by: Neel Chauhan <neel AT neelc DOT org>

[donner]

In RFC 6437 there is no rule, that flow labels are immutable for a given TCP session. It only notes in section 6.1, that changing the flow label within a TCP stream might be suspicious.
IPv6 flow labels are designed as QoS/routing indicators (like the DCSP field), which (in theory) might vary during the life time of the TCP session.

Given, that LRO handling is on the recipient end of the session -- there are no routing, forwarding or QoS behind this point -- the flow label is irrelevant by definition.
In my eyes, throwing away packets which successfully reached the destination for potential QoS/routing issues, is unexpected.

If I'm wrong, the change can be passed.
If I'm right, please change the comment itself to reflect the reasoning.

[rstone]

In D25085#560681, @lutz_donnerhacke.de wrote:

In my eyes, throwing away packets which successfully reached the destination for potential QoS/routing issues, is unexpected.

We don't throw away the packet. We just refuse to merge it with the packet in the LRO buffer. This causes us to send both packets up the stack separately, so that the flow label is properly preserved.

[nc]

Thanks for approving this.

Could one of you please commit this?

[bz]

sys/netinet/tcp_lro.c

385–386

Aehm veto. The mbuf packet header flowid is the 4-tuple and has nothing to do with the IPv6 flow label, or am I wrong?

[nc]

Good catch.

Would this updated patch work?

[bz]

sys/netinet/tcp_lro.c

385

ip6 is derived from m in tcp_lro_rx2() so you are comparing the identical memory here.

I think what needs to be done is to store the ip6_flow or see if we have the entire ip6 header stored in lc already. The code has changed quite a bit since I wrote these comments. lc->leip->ip6 might have that but we'd need to make sure it actually does and then comapre to ip6->ip6_flow or if the former is NULL (I'd assume) pass?

[nc]

Thanks for the clarification.

Is this good?

[bz]

sys/netinet/tcp_lro.c

385–386

I assume le is != NULL here given on return of this function we unconditionally write to it; the question is probably more le->le_ip6 != NULL.

At this point someone who knows the current code should then have a look over this again as I honestly have no idea about the code flow anymore. @rrs is probably the best candidate; @gallatin or @np are probably also people who might be able to review/test this.