Looks like this replaces #42897 (looks like there was a bad rebase on that one?)

I see there's a typo in the commit message (peformance instead of performance); if you're updating, perhaps you could fix that one as well (in case we need to search in git history).

I'm also curious: why move the values inline here instead of retaining the loadBalancerConfig package-level variable?

I think that was my suggestion in f014fa2#r718371177, where I initially thought both were setting the same options. The variable was only used in a single place, so I suggested to either move them closer to where they're used, or just to inline them (to reduce the cognitive overload of finding "where" they're used, and the reverse ("what options are in loadBalancerConfig and ingressConfig ?)

That said, the earlier version did set two different options in two places; see f014fa2, so I'm a bit confused now why this PR is different from the earlier iteration; I think this PR is missing some of the changes from f014fa2 / #42897

Yep -- the rebase did clobber the old PR when trying to sign off one of the commits.

The change to libnetwork/controller.go also got missed when I pulled it over. Added now. @thaJeztah is exactly right about the inlining -- it was a suggestion from the previous PR

@thaJeztah @samuelkarp @corhere can I get a re-review?

CI failure looks unrelated here. Can someone kick it?

(kicked CI)

@corhere is #43146 (comment) ok with you, or do you still prefer to have it addressed?

@thaJeztah I would prefer to have it addressed, but I wouldn't block merging on it It's been addressed; merge away!

Oh; I was wrong; looks like it is a known flaky (but on specific architectures); #41561

For posterity; failure was;

=== RUN   TestNetworkLocalhostTCPNat
    nat_test.go:55: assertion failed: hi yall (msg string) !=  (string)
--- FAIL: TestNetworkLocalhostTCPNat (0.99s)

anyway, let's see if CI goes green and get this in

Bringing this one in; CI is green now 👍

I found another issue. The sysctl settings are not applied after the 1st time starting docker daemon since OS boot. Then after systemctl restart docker, the settings are applied.

<right after booting OS>
docker@ub2204-1:~$ sudo nsenter --net=/var/run/docker/netns/lb_wgyv3nagr sysctl net.ipv4.vs.expire_nodest_conn
net.ipv4.vs.expire_nodest_conn = 0
docker@ub2204-1:~$ sudo sysctl net.ipv4.vs.expire_nodest_conn
net.ipv4.vs.expire_nodest_conn = 0

<restart docker daemon>
docker@ub2204-1:~$ sudo systemctl restart docker

docker@ub2204-1:~$ sudo nsenter --net=/var/run/docker/netns/lb_wgyv3nagr sysctl net.ipv4.vs.expire_nodest_conn
net.ipv4.vs.expire_nodest_conn = 1
docker@ub2204-1:~$ sudo sysctl net.ipv4.vs.expire_nodest_conn
net.ipv4.vs.expire_nodest_conn = 1

(Above testing was run with the dockerd binary including this PR.)

This means the settings would get lost after the user reboots the system and they probably would not notice that.

@xinfengliu nice find! Looks like there's some other code-path that should also apply it.

Could you perhaps open a ticket for that? Just to be sure it's not forgotten (easy to miss comments on merged pull requests); contributions welcome as well of course!

@thaJeztah

After further checking, I find it is because ip_vs module is not loaded by default in my system, docker loads ip_vs on-demand in addServiceBinding, but docker tries to set the ipvs sysctl parameters for the first time in NewSandbox, the ip_vs module has not been loaded yet.

May 22 13:22:53 ub2204-1 dockerd[1053]: time="2022-05-22T13:22:53.350537245Z" level=error msg="error reading the kernel parameter net.ipv4.vs.conn_reuse_mode" error="open /proc/sys/net/ipv4/vs/conn_reuse_mode: no such file or directory"
May 22 13:22:53 ub2204-1 dockerd[1053]: time="2022-05-22T13:22:53.350561161Z" level=error msg="error reading the kernel parameter net.ipv4.vs.expire_nodest_conn" error="open /proc/sys/net/ipv4/vs/expire_nodest_conn: no such file or directory"
May 22 13:22:53 ub2204-1 dockerd[1053]: time="2022-05-22T13:22:53.350567369Z" level=error msg="error reading the kernel parameter net.ipv4.vs.expire_quiescent_template" error="open /proc/sys/net/ipv4/vs/expire_quiescent_template: no such file or directory"
May 22 13:22:53 ub2204-1 dockerd[1053]: time="2022-05-22T13:22:53.350577244Z" level=error msg="error reading the kernel parameter net.ipv4.vs.conn_reuse_mode" error="open /proc/sys/net/ipv4/vs/conn_reuse_mode: no such file or directory"

Not sure if we should create an moby issue for it (addressing it in moby code). I guess we could use sync.Once in addLBBackend to set those sysctl parameters.

Even if we did use sync.Once, there's a basic issue of idempotency. It should not be the case that "the first time I start dockerd, it behaves differently".

Other modules are loaded (nf_conntrack[_netlink]) elsewhere as part of bringup to avoid this, and the same thing would make sense here. I'll find a good point to inject it.