Re: [PATCH net-next] Extending bpf_setsockopt with SO_BINDTODEVICE sockopt

[sdf]

On 05/21, Ferenc Fejes wrote:

This option makes possible to programatically bind sockets to netdevices.
With the help of this option sockets of VRF unaware applications
could be distributed between multiple VRFs with eBPF sock_ops program.
This let the applications benefit from the multiple possible routes.

Signed-off-by: Ferenc Fejes <fe...@inf.elte.hu>
---
  net/core/filter.c | 39 ++++++++++++++++++++++++++++++++++++++-
  1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 822d662f97ef..25dac75bfc5d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c

@@ -4248,6 +4248,9 @@ static const struct bpf_func_proto bpf_get_socket_uid_proto = {

  static int _bpf_setsockopt(struct sock *sk, int level, int optname,
                           char *optval, int optlen, u32 flags)
  {
+       char devname[IFNAMSIZ];
+       struct net *net;
+       int ifindex;
        int ret = 0;
        int val;

@@ -4257,7 +4260,7 @@ static int _bpf_setsockopt(struct sock *sk, int level, int optname,

        sock_owned_by_me(sk);

        if (level == SOL_SOCKET) {
-               if (optlen != sizeof(int))
+               if (optlen != sizeof(int) && optname != SO_BINDTODEVICE)
                        return -EINVAL;
                val = *((int *)optval);

@@ -4298,6 +4301,40 @@ static int _bpf_setsockopt(struct sock *sk, int level, int optname,

                                sk_dst_reset(sk);
                        }
                        break;
+               case SO_BINDTODEVICE:
+                       ret = -ENOPROTOOPT;
+#ifdef CONFIG_NETDEVICES

Any specific reason you're not reusing sock_setbindtodevice or at least
sock_setbindtodevice_locked here? I think, historically, we've
reimplemented some of the sockopts because they were 'easy' (i.e.
were just setting a flag in the socket), this one looks more involved.

I'd suggest, add an optional 'lock_sk' argument to sock_setbindtodevice,
call it with 'true' from real setsockopt, and call it with 'false'
here.

And, as Andrii pointed out, it would be nice to have a selftest
that exercises this new option.