Add simple tests for SO_REUSEPORT_LB sockets.

[markj]

Summary

These tests create a LB group on the loopback address, connect to the
group address a number of times, and verify that the connections were
roughly balanced among the listening sockets in the group.

This provides some basic regression testing and makes it marginally
easier to add additional SO_REUSEPORT_LB tests in the future.

[asomers]

tests/sys/netinet/reuseport_lb.c

107 ↗

(On Diff #47886)

What's the meaning of 8?

109 ↗

(On Diff #47886)

waitpid doesn't need to be in the loop.

123 ↗

(On Diff #47886)

Are the write and read really necessary?

175 ↗

(On Diff #47886)

Does the use of port 0 force the test to run as root?

184 ↗

(On Diff #47886)

Any reason not to use ATF_REQUIRE_EQ_MSG?

262 ↗

(On Diff #47886)

magic numbers should be #defined.

[markj]

tests/sys/netinet/reuseport_lb.c

107 ↗

(On Diff #47886)

Because connections are balanced among sockets in the group, we expect each listening socket to accept nconns / sdcnt connections. 8 is just an arbitrary factor to allow for some error. I will add a comment.

109 ↗

(On Diff #47886)

What if the child exits prematurely after creating a few connections?

123 ↗

(On Diff #47886)

I guess not. The child is using a blocking socket, so connect() will block until the parent accept()s. Will remove.

175 ↗

(On Diff #47886)

No, it's the wildcard port. (Is there a constant for this? I can't find one.)

184 ↗

(On Diff #47886)

No, but I find it less clear.

[markj]

• Address some review feedback.

[asomers]

In fact, you don't even need the second process for this test. It's possible to do this entirely with one single-threaded process. See the "accept_success" test in tests/sys/audit/network.c for an example. Basically it goes like this:

for (i=0; i<sdcnt; i++)
    listen(server_fd[i]);
while (true) {
    client_fd = socket();
    client_fd.connect(server_addr);  //blocking, but returns immediately
    for ((i=0; i<sdcnt; i++) {
        if (accept(server_fd[i]) == 0) {    // non-blocking
            acceptcnt[i]++;
            break;
        }
    }
}

What do you think of the single-threaded approach?

[markj]

In D17110#364786, @asomers wrote:

What do you think of the single-threaded approach?

It's much better. :) I'll give it a try.

[markj]

• Simplify according to asomers' suggestion.

[asomers]

All good here.

[markj]

• Simplify further: we don't need non-blocking connect().
• Fix comment, fix loop which verifies that acceptcnt is balanced.

[asomers]

tests/sys/netinet/reuseport_lb.c

92 ↗

(On Diff #47903)

I think you should assert here that i < nsds. Otherwise, it's possible that some connections don't get accepted on any sockets and you wouldn't notice.

[markj]

• Assert i < nsds after the accept() loop.

[asomers]

Do the tests still pass for you? After your latest change, I get the "connection wasn't accepted" error every time.

[markj]

In D17110#364949, @asomers wrote:

Do the tests still pass for you? After your latest change, I get the "connection wasn't accepted" error every time.

Yes, I just noticed that. (I rebuilt and ran the tests on two different machines by accident.) It seems like the polling loop is needed after all, which is somewhat surprising to me given that we're using a blocking connect().

[markj]

In D17110#364950, @markj wrote:

In D17110#364949, @asomers wrote:

Do the tests still pass for you? After your latest change, I get the "connection wasn't accepted" error every time.

Yes, I just noticed that. (I rebuilt and ran the tests on two different machines by accident.) It seems like the polling loop is needed after all, which is somewhat surprising to me given that we're using a blocking connect().

I guess this is expected behaviour for TCP. connect(2) returns once the SYN|ACK reply is received, but the child socket isn't complete until the final ACK is received, so there's a window that we have to account for.

[markj]

• Re-add the polling loop.

[asomers]

In D17110#364972, @markj wrote:

In D17110#364950, @markj wrote:

In D17110#364949, @asomers wrote:

Do the tests still pass for you? After your latest change, I get the "connection wasn't accepted" error every time.

Yes, I just noticed that. (I rebuilt and ran the tests on two different machines by accident.) It seems like the polling loop is needed after all, which is somewhat surprising to me given that we're using a blocking connect().

I guess this is expected behaviour for TCP. connect(2) returns once the SYN|ACK reply is received, but the child socket isn't complete until the final ACK is received, so there's a window that we have to account for.

That makes sense. The listening sockets need to be non-blocking because you don't know which one will receive the next connection. But by making them nonblocking you're also changing the behavior of the connecting sockets. You could probably eliminate the polling loop by making the listening sockets blocking and using select(2) to identify which one got the connection. But I think that would be more trouble than it's worth.

[markj]

In D17110#364983, @asomers wrote:

You could probably eliminate the polling loop by making the listening sockets blocking and using select(2) to identify which one got the connection. But I think that would be more trouble than it's worth.

Right, you can select() for new connections by adding the listening sockets to the read set. But as this is all over the loopback, I didn't bother with that.

Thanks for the review.