tcp: enable LRD by default and move sysctl from tcp.do_lrd tp tcp.sack.lrd

[rscheff]

Summary

Lost Retransmission Detection was added as a feature in May 2021, but disabled by default.

It is time to enable this feature to reduce the completion time / SRT by avoiding RTOs when retransmissions get lost too by default.

While here, moving the sysctl control to the tcp.sack branch, as LRD only works in conjunction with SACK loss recovery - and update the tcp.4 man page accordingly.

MFC after: 10 weeks

Test Plan

repeatedly loose the same segment - the base stack should (if not running out of cwnd first) retry multiple times to send the repeatedly lost packet again.

[tuexen]

Question: can the moving of the sysctl variable be MFCed? Enabling the feature can, of course.

[zlei]

In D42845#977215, @tuexen wrote:

Question: can the moving of the sysctl variable be MFCed? Enabling the feature can, of course.

It is user visible change. I think it can NOT be MFCed unless it is treated as debugging purpose only knob.

I'm OK with "enable LRD by default" .

[rscheff]

In D42845#977248, @zlei wrote:

In D42845#977215, @tuexen wrote:

Question: can the moving of the sysctl variable be MFCed? Enabling the feature can, of course.

It is user visible change. I think it can NOT be MFCed unless it is treated as debugging purpose only knob.

I'm OK with "enable LRD by default" .

While looking at the original commit, I also had a special sockopt added; I don't think that anyone ever used this really, for true A/B testing (whcih was the reason to have it in the first place).

Moving the lrd sysctl to become part of the sack subtree makes IMHO sense (and I should really have done it ffrom the beginnig), as LRD only works with SACK (there exists DupACK counting mechansims to approximate this, but that would be more fragile under some network pathologies).

I could leave out the MFC for the sysctl part (or better: split this into a MFC enable by default, and a non-MFC for all other changes) - but really would like to know if keeping the sockopt makes sense - few other singular mechanisms entertain their own sockopt to be programmatically changed on a session-by-session basis....

[tuexen]

In D42845#977275, @rscheff wrote:

In D42845#977248, @zlei wrote:

In D42845#977215, @tuexen wrote:

Question: can the moving of the sysctl variable be MFCed? Enabling the feature can, of course.

It is user visible change. I think it can NOT be MFCed unless it is treated as debugging purpose only knob.

I'm OK with "enable LRD by default" .

While looking at the original commit, I also had a special sockopt added; I don't think that anyone ever used this really, for true A/B testing (whcih was the reason to have it in the first place).

Does this feature apply only to the base stack or also to the RACK/BBR stack?

Moving the lrd sysctl to become part of the sack subtree makes IMHO sense (and I should really have done it ffrom the beginnig), as LRD only works with SACK (there exists DupACK counting mechansims to approximate this, but that would be more fragile under some network pathologies).

I could leave out the MFC for the sysctl part (or better: split this into a MFC enable by default, and a non-MFC for all other changes) - but really would like to know if keeping the sockopt makes sense - few other singular mechanisms entertain their own sockopt to be programmatically changed on a session-by-session basis....

I guess it would be good to split the patch up in two: one changing the default, which can be MFCed, one being the move of the sysctl node, which can't be MFCed.

[rscheff]

• tcp: enable lost retransmission detection by default

[rscheff]

In D42845#977279, @tuexen wrote:

Does this feature apply only to the base stack or also to the RACK/BBR stack?

Only the base stack; RACK deals with lost retransmissions as an emergent property of the RACK mechanism and per-segment state itself.

I guess it would be good to split the patch up in two: one changing the default, which can be MFCed, one being the move of the sysctl node, which can't be MFCed.

Yes. Adjusting this Diff accordingly.

[zlei]

Looks good to me.

[tuexen]

In D42845#977283, @rscheff wrote:

In D42845#977279, @tuexen wrote:

Does this feature apply only to the base stack or also to the RACK/BBR stack?

Only the base stack; RACK deals with lost retransmissions as an emergent property of the RACK mechanism and per-segment state itself.

At least Netflix will then not need the socket option support for A/B testing.

I guess it would be good to split the patch up in two: one changing the default, which can be MFCed, one being the move of the sysctl node, which can't be MFCed.

Yes. Adjusting this Diff accordingly.

Perfect. Approved.