Re: [PATCH bpf-next v2 3/6] net: remove duplicate reuseport_lookup functions

On Tue, Jun 13, 2023 at 7:57 PM Kuniyuki Iwashima kuniyu@amazon.com wrote:

The assignment to result below is buggy. Let's say SO_REUSEPROT group have TCP_CLOSE and TCP_ESTABLISHED sockets.

1. Find TCP_CLOSE sk and do SO_REUSEPORT lookup
2. result is not NULL, but the group has TCP_ESTABLISHED sk
3. result = result
4. Find TCP_ESTABLISHED sk, which has a higher score
5. result = result (TCP_CLOSE) <-- should be sk.

Same for v6 function.

Thanks for your explanation, I think I get it now. I misunderstood that you were worried about returning TCP_ESTABLISHED instead of TCP_CLOSE, but it's exactly the other way around.

I have a follow up question regarding the existing code:

result = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum); /* Fall back to scoring if group has connections */ if (result && !reuseport_has_conns(sk)) return result;

result = result ? : sk; badness = score;

Assuming that result != NULL but reuseport_has_conns() == true, we use the reuseport socket as the result, but assign the score of sk to badness. Shouldn't we use the score of the reuseport socket?

Thanks Lorenz