Re: [PATCH v2] net-next: Fix IP_UNICAST_IF option behavior for connected sockets

On 8/29/22 5:18 AM, Richard Gobert wrote:

The IP_UNICAST_IF socket option is used to set the outgoing interface for outbound packets.

The IP_UNICAST_IF socket option was added as it was needed by the Wine project, since no other existing option (SO_BINDTODEVICE socket option, IP_PKTINFO socket option or the bind function) provided the needed characteristics needed by the IP_UNICAST_IF socket option. [1] The IP_UNICAST_IF socket option works well for unconnected sockets, that is, the interface specified by the IP_UNICAST_IF socket option is taken into consideration in the route lookup process when a packet is being sent. However, for connected sockets, the outbound interface is chosen when connecting the socket, and in the route lookup process which is done when a packet is being sent, the interface specified by the IP_UNICAST_IF socket option is being ignored.

This inconsistent behavior was reported and discussed in an issue opened on systemd's GitHub project [2]. Also, a bug report was submitted in the kernel's bugzilla [3].

To understand the problem in more detail, we can look at what happens for UDP packets over IPv4 (The same analysis was done separately in the referenced systemd issue). When a UDP packet is sent the udp_sendmsg function gets called and the following happens:

1. The oif member of the struct ipcm_cookie ipc (which stores the

output interface of the packet) is initialized by the ipcm_init_sk function to inet->sk.sk_bound_dev_if (the device set by the SO_BINDTODEVICE socket option).

2. If the IP_PKTINFO socket option was set, the oif member gets

overridden by the call to the ip_cmsg_send function.

3. If no output interface was selected yet, the interface specified

by the IP_UNICAST_IF socket option is used.

4. If the socket is connected and no destination address is

specified in the send function, the struct ipcm_cookie ipc is not taken into consideration and the cached route, that was calculated in the connect function is being used.

Thus, for a connected socket, the IP_UNICAST_IF sockopt isn't taken into consideration.

This patch corrects the behavior of the IP_UNICAST_IF socket option for connect()ed sockets by taking into consideration the IP_UNICAST_IF sockopt when connecting the socket.

In order to avoid reconnecting the socket, this option is still ignored when applied on an already connected socket until connect() is called again by the Richard Gobert.

Change the __ip4_datagram_connect function, which is called during socket connection, to take into consideration the interface set by the IP_UNICAST_IF socket option, in a similar way to what is done in the udp_sendmsg function.

[1] https://lore.kernel.org/netdev/1328685717.4736.4.camel@edumazet-laptop/T/ [2] https://github.com/systemd/systemd/issues/11935#issuecomment-618691018 [3] https://bugzilla.kernel.org/show_bug.cgi?id=210255

Signed-off-by: Richard Gobert richardbgobert@gmail.com

v1 -> v2: Added self-tests and targeted to net-next.

net/ipv4/datagram.c | 2 ++ tools/testing/selftests/net/fcnal-test.sh | 30 +++++++++++++++++++++++ tools/testing/selftests/net/nettest.c | 16 ++++++++++-- 3 files changed, 46 insertions(+), 2 deletions(-)

Reviewed-by: David Ahern dsahern@kernel.org

diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.sh index 03b586760164..31c3b6ebd388 100755 --- a/tools/testing/selftests/net/fcnal-test.sh +++ b/tools/testing/selftests/net/fcnal-test.sh @@ -1466,6 +1466,13 @@ ipv4_udp_novrf() run_cmd nettest -D -r ${a} -d ${NSA_DEV} -S -0 ${NSA_IP} log_test_addr ${a} $? 0 "Client, device bind via IP_UNICAST_IF"

• log_start
  

• run_cmd_nsb nettest -D -s &
  

• sleep 1
  

• run_cmd nettest -D -r ${a} -d ${NSA_DEV} -S -0 ${NSA_IP} -U
  

• log_test_addr ${a} $? 0 "Client, device bind via IP_UNICAST_IF, with connect()"
  

• log_start show_hint "Should fail 'Connection refused'" run_cmd nettest -D -r ${a}

@@ -1525,6 +1532,13 @@ ipv4_udp_novrf() run_cmd nettest -D -d ${NSA_DEV} -S -r ${a} log_test_addr ${a} $? 0 "Global server, device client via IP_UNICAST_IF, local connection"

• log_start
• run_cmd nettest -s -D &
• sleep 1
• run_cmd nettest -D -d ${NSA_DEV} -S -r ${a} -U
• log_test_addr ${a} $? 0 "Global server, device client via IP_UNICAST_IF, local connection, with connect()"
• # IPv4 with device bind has really weird behavior - it overrides the # fib lookup, generates an rtable and tries to send the packet. This # causes failures for local traffic at different places

@@ -1550,6 +1564,15 @@ ipv4_udp_novrf() sleep 1 run_cmd nettest -D -r ${a} -d ${NSA_DEV} -S log_test_addr ${a} $? 1 "Global server, device client via IP_UNICAST_IF, local connection"

• log_start
  

• show_hint "Should fail since addresses on loopback are out of device scope"
  

• run_cmd nettest -D -s &
  

• sleep 1
  

• run_cmd nettest -D -r ${a} -d ${NSA_DEV} -S -U
  

• log_test_addr ${a} $? 1 "Global server, device client via IP_UNICAST_IF, local connection, with connect()"
  

• done

a=${NSA_IP} @@ -3157,6 +3180,13 @@ ipv6_udp_novrf() sleep 1 run_cmd nettest -6 -D -r ${a} -d ${NSA_DEV} -S log_test_addr ${a} $? 1 "Global server, device client via IP_UNICAST_IF, local connection"

• log_start
  

• show_hint "Should fail 'No route to host' since addresses on loopback are out of device scope"
  

• run_cmd nettest -6 -D -s &
  

• sleep 1
  

• run_cmd nettest -6 -D -r ${a} -d ${NSA_DEV} -S -U
  

• log_test_addr ${a} $? 1 "Global server, device client via IP_UNICAST_IF, local connection, with connect()"
  

  done

a=${NSA_IP6}

All of the tests are 'novrf' but this should work with VRF too. The device is in a VRF, so binding it to should. Please add these tests again for the VRF cases. It can be a followup patch.