Re: [PATCH] net: prefer socket bound to interface when not in VRF

[Benjamin Poirier <bpoirier@nvidia.com>]

On 2022-06-12 21:52 -0600, David Ahern wrote:
[...]
> > 
> > Hi Mike,
> > 
> > I was looking at this commit, 8d6c414cd2fb ("net: prefer socket bound to
> > interface when not in VRF"), and I get the feeling that it is only
> > partially effective. It works with UDP connected sockets but it doesn't
> > work for TCP and UDP unconnected sockets.
> > 
> > The compute_score() functions are a bit misleading. Because of the
> > reuseport shortcut in their callers (inet_lhash2_lookup() and the like),
> > the first socket with score > 0 may be chosen, not necessarily the
> > socket with highest score. In order to prefer certain sockets, I think
> > an approach like commit d894ba18d4e4 ("soreuseport: fix ordering for
> > mixed v4/v6 sockets") would be needed. What do you think?
> > 
> > Extra info:
> > 1) fcnal-test.sh results
> > 
> > I tried to reproduce the fcnal-test.sh test results quoted above but in
> > my case the test cases already pass at 8d6c414cd2fb^ and 9e9fb7655ed5.
> > Moreover I believe those test cases don't have multiple listening
> > sockets. So that just added to my confusion.
> > 
> > Running 9e9fb7655ed5,
> > root@vsid:/src/linux/tools/testing/selftests/net# ./fcnal-test.sh -t use_cases
> 
> use_cases group is a catchall for bug reports. You want run all of the
> TCP and UDP cases to cover socket permutations and I know some of those
> cover dual listeners (though I can't remember ATM if that is only the
> MD5 tests).

I was talking specifically about the two test cases quoted in Mike's
original posting, in case that wasn't clear.

> If not, you can add them fairly easily and illustrate your
> point.

What about reuseport_bindtodevice.c from my previous message? Running it
on the current net/master (619c010a6539) shows:

root@vsid:~# ./reuseport_bindtodevice
IPv4 TCP ... fail
IPv4 UDP unconnected ... fail
IPv4 UDP connected ... pass
IPv6 TCP ... fail
IPv6 UDP unconnected ... fail
IPv6 UDP connected ... pass
FAIL

> 
> As for compute_score, it does weight device binds a bit higher. TCP:
> 
> score =  sk->sk_bound_dev_if ? 2 : 1;
> 
> UDP:
> if (sk->sk_bound_dev_if)
>         score += 4;

I think there was a misunderstanding. A higher score does not lead to a
higher preference for a socket in many cases. That's the pitfall I tried
to describe earlier.