mod_cc(9) per-algorithm per-connection {get|set}sockopt support

[lstewart]

Summary

A modification to TCP sockopt handling and the mod_cc(9) KPI which allows an application to get/set algorithm specific parameters on a per connection basis.

Proposed commit log message:

---

Add a new TCP socket option "TCP_CCALGOOPT" and associated infrastructure to allow applications to query or set congestion control algorithm specific parameters on a per socket basis using {get|set}sockopt():

• Extend the mod_cc(9) KPI with a new ctl_output() hook which provides the means for modules to expose parameters to applications if they so choose.

• Install algorithm specific header files in <netinet/cc/> in the system include directory so that applications can refer to #defines of socket option names available for a given algorithm.

• Implement support for the TCP_CCALGOOPT socket option by introducing a new struct "cc_sockopt" which acts as an opaque payload for the TCP level sockopt handling code in tcp_usrreq.c to then pass through to the module for handling via the module's ctl_output() hook.

• Update relevant documentation.

Reviewed by: XXX
Sponsored by: Netflix, Inc.
MFC after: 1 week (needs tweaking to not break ABI)

---

[glebius]

I find tautology in "socket option TCP_CCALGOOPT". All other socket options do not have abbreviation "OPT" in their names. I'd suggest to name it TCP_CCALGO or somehow other way.

[lstewart]

In D711#6, @glebius wrote:

I find tautology in "socket option TCP_CCALGOOPT". All other socket options do not have abbreviation "OPT" in their names. I'd suggest to name it TCP_CCALGO or somehow other way.

Yes because they are options of the TCP layer itself, whereas TCP_CCALGOOPT is in fact an option to set an option within a "sublayer" below TCP i.e. we're settling a CC algorithm option, not the TCP cc algo, which is why I appended OPT to the name. Actual CC algo option names you assign to sopt_name in struct cc_scoopt do not have OPT appended to their names because like regular sockopts, are an option of the layer they're being interpreted by - the CC module in this case.

The mechanism is a bit like the film Inception, but it works :)

[gnn]

Looks good to me. Commit.

[lstewart]

Last call - I'll commit the patch if I don't hear any objections in the next 48 hours.

[glebius]

Lawrence, did you notice my inline comments that I left on initial review?

share/man/man4/tcp.4

143 ↗

(On Diff #1325)

I'd suggest to add mod_cc(4) to SEE ALSO section of tcp(4).

sys/netinet/tcp_usrreq.c

1487 ↗

(On Diff #1325)

What if userland doesn't put \0 at end of string? I'd suggest explicit:

ccsopt->cc_name[TCP_CA_NAME_MAX - 1] = '\0';

After that do we need strlen() at all? Why don't do strcmp() only?

P.S. Same comment for lower chunk of diff.

[glebius]

Oh, the comments got pushed only after I added the new comment. All the weeks they were visible only to me. Stupid fabricator.

[bz]

Only very minor comments; nothing to stop you; glebius caught the problematic cases.

share/man/man4/mod_cc.4

33 ↗

(On Diff #1325)

Don't forget to update on the commit day

share/man/man4/tcp.4

38 ↗

(On Diff #1325)

Again, please remember to update before committing.

share/man/man9/mod_cc.9

35 ↗

(On Diff #1325)

And another date to update before committing.

sys/netinet/tcp_usrreq.c

1475 ↗

(On Diff #1325)

This is generally style we don't do; If we do we make it a proper block usually and declare the variable at the beginning. Wouldn't worry though.

[lstewart]

Thanks for the feedback guys. I'll update the patch shortly. Not sure why your comments never came through Gleb, but I definitely didn't see them until your follow up a few days ago.

[hiren]

Eagerly waiting for this to get in with suggested changes!

cheers,
Hiren