On Thu, 16 Oct 2025, Paolo Abeni wrote:
On 10/13/25 7:03 PM, chia-yu.chang@nokia-bell-labs.com wrote:
From: Ilpo Järvinen ij@kernel.org
As AccECN may keep CWR bit asserted due to different interpretation of the bit, flushing with GRO because of CWR may effectively disable GRO until AccECN counter field changes such that CWR-bit becomes 0.
There is no harm done from not immediately forwarding the CWR'ed segment with RFC3168 ECN.
I guess this change could introduce additional latency for RFC3168 notification, which sounds not good.
@Eric: WDYT?
I'm not Eric but I want to add I foresaw somebody making this argument and thus wanted to not hide this change into some other patch so it can be properly discussed and rejected if so preferred, either way it's not a correctness issue.
I agree it's possible for some delay be added but the question is why would that matter? "CWR" tells sender did already reduce its sending rate which is where congestion control aims to. So the reaction to congestion is already done when GRO sees CWR (some might have a misconception that delivering CWR causes sender to reduce sending rate but that's not the case). With RFC 3168 ECN, CWR only tells the receiving end to stop sending ECE. Why does it matter if that information arrives a bit later?
If there are other segments, they normally don't have CWR with RFC 3168 ECN which normally set CWR once per RTT. A non-CWR'ed segment results in flush after an inter-packet delay due to flags difference. That delay is nothing compared to GRO aggregating non-CWR segments en masse which is in n times the inter-packet delay (simplification, ignores burstiness, etc.).
If there are no other segments, the receiver won't be sending any ECEs either, so the extra delay does not seem that impactful.
Some might argue that with this "special delivery" for CWR the segment could trigger an ACK "sooner", but GRO shouldn't hold the segment forever either (though I don't recall the details anymore). But if we make that argument (which is no longer ECN signalling related at all, BTW), why use GRO at all as it add delay for other segments too delaying other ACKs, why is this CWR'ed segment so special that it in particular must elicit ACK ASAP? It's hard to justify that distinction/CWR speciality, unless one has that misconception CWR must arrive ASAP to expedite congestion reaction which is based on misunderstanding how RFC 3168 ECN works.
Thus, what I wrote to the changelog about the delay not being harmful seems well justified.
On the flip side adding too much AccECN logic to GRO (i.e. to allow aggregation only for AccECN enabled flows) looks overkill.
The usual aggregation works on header bits remaining identical which just happens to also suit AccECN better here. The RFC 3168 CWR trickery is what is an expection to the rule, and as explained above, it does not seem even that useful.
This CWR special delivery rule, on the other hand, is clearly harmful for aggregating AccECN segments which may have long row of CWR flagged segments if ACE field remains unchanging. None of them can be aggregated by GRO if this particular change is not accepted. Not an end of the world but if we weight the pros and cons, it seems to clearly favor not keeping this special delivery rule.