[Ecn-sane] [PATCH net-next] tcp: avoid negotitating ECN for BBR

[Sebastian Moeller]

Hi Pete,


> On Nov 28, 2023, at 09:43, Pete Heist via Ecn-sane <ecn-sane at lists.bufferbloat.net> wrote:
> 
> It doesn't look like the patch to set TCP_CONG_DONT_USE_ECN in BBRv1
> was ever applied, but it also doesn't look like v1 was ever modified to
> react to CE, whereas BBRv3 appears to:
> 
> https://github.com/google/bbr/blob/v3/README.md#enabling-ecn-support
> 
> I'll post something about ecn_low separately.

	The way I read this is that BBR still does ignore rfc3168 ECN, but allows to use DCTCP style ECN on a route by route base... this at least seems to acknowledge that currently no randomly selected path is likely to support DCTCP style ECN... this seems far more cautious than I had feared. I still think that BBR did itself a disservice to ignore rfc3168 style ECN, but that is water down the bridge...

Regards
	Sebastian



> 
> Pete
> 
> On Mon, 2023-11-27 at 19:12 -0500, Dave Taht via Ecn-sane wrote:
>> Was this ever resolved?
>> 
>> ---------- Forwarded message ---------
>> From: David Miller <davem at davemloft.net>
>> Date: Fri, Jan 19, 2018 at 2:33 PM
>> Subject: Re: [PATCH net-next] tcp: avoid negotitating ECN for BBR
>> To: <ycheng at google.com>
>> Cc: <netdev at vger.kernel.org>, <edumazet at google.com>,
>> <ysseung at google.com>, <ncardwell at google.com>
>> 
>> 
>> From: Yuchung Cheng <ycheng at google.com>
>> Date: Tue, 16 Jan 2018 17:57:26 -0800
>> 
>>> This patch keeps BBR from negotiating ECN if sysctl ECN is
>>> set. Prior to this patch, BBR negotiates ECN if enabled, sends
>>> CWR upon receiving ECE ACKs but does not react to them. This can
>>> cause confusion from the protocol perspective. Therefore this
>>> patch prevents the connection from negotiating ECN if BBR is the
>>> congestion control during the handshake.
>>> 
>>> Note that after the handshake, the user can still switch to a
>>> different congestion control that supports or even requires ECN
>>> (e.g. DCTCP).  In that case, the connection can not re-negotiate
>>> ECN and has to go with the ECN-free mode in that congestion
>>> control.
>>> 
>>> There are other cases BBR would still respond to ECE ACKs with CWR
>>> but does not react to it like the behavior before this patch.
>>> First,
>>> when the user switches to BBR congestion control but the connection
>>> has already negotiated ECN before. Second, the system has
>>> configured
>>> the ip route and/or uses eBPF to enable ECN on the connection that
>>> uses BBR congestion control.
>>> 
>>> Signed-off-by: Yuchung Cheng <ycheng at google.com>
>>> Signed-off-by: Neal Cardwell <ncardwell at google.com>
>>> Acked-by: Yousuk Seung <ysseung at google.com>
>>> Acked-by: Eric Dumazet <edumazet at google.com>
>> 
>> Well, this is a bit disappointing.  I'm having trouble justifying
>> applying this.
>> 
>> Why doesn't BBR react to ECN notifications?  Is it because BBR's
>> idea of congestion differs from the one ECN is likely indicating?
>> 
>> This is really unfortunate, because if BBR does become quite
>> prominent
>> (that's what you want right? :-) then what little success there has
>> been deploying working ECN will be for almost nothing, and there
>> will be little incentive for further ECN deployment.
>> 
>> And the weird behavior you list in your last paragraph, about how if
>> the user switches to BBR then ECN will be active, is just a red flag
>> that shows perhaps this is a bad idea overall.
>> 
>> ECN behavior should not be so tightly bound to the congestion control
>> algorithm like this, it's a connection property independant of
>> congestion control algorithm.
>> 
>> I'm not applying this for now, sorry.  Maybe if you significantly
>> enhance the commit message and try to do something sane with the
>> algorithm switching case it is work a respin.
>> 
>> Thanks.
>> 
>> 
> 
> _______________________________________________
> Ecn-sane mailing list
> Ecn-sane at lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/ecn-sane