All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 1/2] bpf: allow to retrieve sol_socket opts from sock_addr progs
@ 2021-01-11 16:17 Daniel Borkmann
  2021-01-11 16:17 ` [PATCH bpf-next 2/2] bpf: extend bind v4/v6 selftests for mark/prio/bindtoifindex Daniel Borkmann
  2021-01-11 20:03 ` [PATCH bpf-next 1/2] bpf: allow to retrieve sol_socket opts from sock_addr progs Yonghong Song
  0 siblings, 2 replies; 5+ messages in thread
From: Daniel Borkmann @ 2021-01-11 16:17 UTC (permalink / raw
  To: ast; +Cc: yhs, bpf, netdev, Daniel Borkmann

The _bpf_setsockopt() is able to set some of the SOL_SOCKET level options,
however, _bpf_getsockopt() has little support to actually retrieve them.
This small patch adds few misc options such as SO_MARK, SO_PRIORITY and
SO_BINDTOIFINDEX. For the latter getter and setter are added. The mark and
priority in particular allow to retrieve the options from BPF cgroup hooks
to then implement custom behavior / settings on the syscall hooks compared
to other sockets that stick to the defaults, for example.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 net/core/filter.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 255aeee72402..9ab94e90d660 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4770,6 +4770,10 @@ static int _bpf_setsockopt(struct sock *sk, int level, int optname,
 				ifindex = dev->ifindex;
 				dev_put(dev);
 			}
+			fallthrough;
+		case SO_BINDTOIFINDEX:
+			if (optname == SO_BINDTOIFINDEX)
+				ifindex = val;
 			ret = sock_bindtoindex(sk, ifindex, false);
 			break;
 		case SO_KEEPALIVE:
@@ -4932,8 +4936,25 @@ static int _bpf_getsockopt(struct sock *sk, int level, int optname,
 
 	sock_owned_by_me(sk);
 
+	if (level == SOL_SOCKET) {
+		if (optlen != sizeof(int))
+			goto err_clear;
+
+		switch (optname) {
+		case SO_MARK:
+			*((int *)optval) = sk->sk_mark;
+			break;
+		case SO_PRIORITY:
+			*((int *)optval) = sk->sk_priority;
+			break;
+		case SO_BINDTOIFINDEX:
+			*((int *)optval) = sk->sk_bound_dev_if;
+			break;
+		default:
+			goto err_clear;
+		}
 #ifdef CONFIG_INET
-	if (level == SOL_TCP && sk->sk_prot->getsockopt == tcp_getsockopt) {
+	} else if (level == SOL_TCP && sk->sk_prot->getsockopt == tcp_getsockopt) {
 		struct inet_connection_sock *icsk;
 		struct tcp_sock *tp;
 
@@ -4986,12 +5007,12 @@ static int _bpf_getsockopt(struct sock *sk, int level, int optname,
 		default:
 			goto err_clear;
 		}
+#endif
 #endif
 	} else {
 		goto err_clear;
 	}
 	return 0;
-#endif
 err_clear:
 	memset(optval, 0, optlen);
 	return -EINVAL;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 2/2] bpf: extend bind v4/v6 selftests for mark/prio/bindtoifindex
  2021-01-11 16:17 [PATCH bpf-next 1/2] bpf: allow to retrieve sol_socket opts from sock_addr progs Daniel Borkmann
@ 2021-01-11 16:17 ` Daniel Borkmann
  2021-01-11 20:15   ` Yonghong Song
  2021-01-11 20:03 ` [PATCH bpf-next 1/2] bpf: allow to retrieve sol_socket opts from sock_addr progs Yonghong Song
  1 sibling, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2021-01-11 16:17 UTC (permalink / raw
  To: ast; +Cc: yhs, bpf, netdev, Daniel Borkmann

Extend existing cgroup bind4/bind6 tests to add coverage for setting and
retrieving SO_MARK, SO_PRIORITY and SO_BINDTOIFINDEX at the bind hook.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 .../testing/selftests/bpf/progs/bind4_prog.c  | 41 +++++++++++++++++--
 .../testing/selftests/bpf/progs/bind6_prog.c  | 41 +++++++++++++++++--
 2 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
index c6520f21f5f5..4479ac27b1d3 100644
--- a/tools/testing/selftests/bpf/progs/bind4_prog.c
+++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
@@ -29,18 +29,47 @@ static __inline int bind_to_device(struct bpf_sock_addr *ctx)
 	char veth2[IFNAMSIZ] = "test_sock_addr2";
 	char missing[IFNAMSIZ] = "nonexistent_dev";
 	char del_bind[IFNAMSIZ] = "";
+	int veth1_idx, veth2_idx;
 
 	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
-				&veth1, sizeof(veth1)))
+			   &veth1, sizeof(veth1)))
+		return 1;
+	if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
+			   &veth1_idx, sizeof(veth1_idx)) || !veth1_idx)
 		return 1;
 	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
-				&veth2, sizeof(veth2)))
+			   &veth2, sizeof(veth2)))
+		return 1;
+	if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
+			   &veth2_idx, sizeof(veth2_idx)) || !veth2_idx ||
+	    veth1_idx == veth2_idx)
 		return 1;
 	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
-				&missing, sizeof(missing)) != -ENODEV)
+			   &missing, sizeof(missing)) != -ENODEV)
+		return 1;
+	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
+			   &veth1_idx, sizeof(veth1_idx)))
 		return 1;
 	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
-				&del_bind, sizeof(del_bind)))
+			   &del_bind, sizeof(del_bind)))
+		return 1;
+
+	return 0;
+}
+
+static __inline int misc_opts(struct bpf_sock_addr *ctx, int opt)
+{
+	int old, tmp, new = 0xeb9f;
+
+	if (bpf_getsockopt(ctx, SOL_SOCKET, opt, &old, sizeof(old)) ||
+	    old == new)
+		return 1;
+	if (bpf_setsockopt(ctx, SOL_SOCKET, opt, &new, sizeof(new)))
+		return 1;
+	if (bpf_getsockopt(ctx, SOL_SOCKET, opt, &tmp, sizeof(tmp)) ||
+	    tmp != new)
+		return 1;
+	if (bpf_setsockopt(ctx, SOL_SOCKET, opt, &old, sizeof(old)))
 		return 1;
 
 	return 0;
@@ -93,6 +122,10 @@ int bind_v4_prog(struct bpf_sock_addr *ctx)
 	if (bind_to_device(ctx))
 		return 0;
 
+	/* Test for misc socket options. */
+	if (misc_opts(ctx, SO_MARK) || misc_opts(ctx, SO_PRIORITY))
+		return 0;
+
 	ctx->user_ip4 = bpf_htonl(SERV4_REWRITE_IP);
 	ctx->user_port = bpf_htons(SERV4_REWRITE_PORT);
 
diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c
index 4358e44dcf47..1b4142fcdd4b 100644
--- a/tools/testing/selftests/bpf/progs/bind6_prog.c
+++ b/tools/testing/selftests/bpf/progs/bind6_prog.c
@@ -35,18 +35,47 @@ static __inline int bind_to_device(struct bpf_sock_addr *ctx)
 	char veth2[IFNAMSIZ] = "test_sock_addr2";
 	char missing[IFNAMSIZ] = "nonexistent_dev";
 	char del_bind[IFNAMSIZ] = "";
+	int veth1_idx, veth2_idx;
 
 	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
-				&veth1, sizeof(veth1)))
+			   &veth1, sizeof(veth1)))
+		return 1;
+	if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
+			   &veth1_idx, sizeof(veth1_idx)) || !veth1_idx)
 		return 1;
 	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
-				&veth2, sizeof(veth2)))
+			   &veth2, sizeof(veth2)))
+		return 1;
+	if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
+			   &veth2_idx, sizeof(veth2_idx)) || !veth2_idx ||
+	    veth1_idx == veth2_idx)
 		return 1;
 	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
-				&missing, sizeof(missing)) != -ENODEV)
+			   &missing, sizeof(missing)) != -ENODEV)
+		return 1;
+	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
+			   &veth1_idx, sizeof(veth1_idx)))
 		return 1;
 	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
-				&del_bind, sizeof(del_bind)))
+			   &del_bind, sizeof(del_bind)))
+		return 1;
+
+	return 0;
+}
+
+static __inline int misc_opts(struct bpf_sock_addr *ctx, int opt)
+{
+	int old, tmp, new = 0xeb9f;
+
+	if (bpf_getsockopt(ctx, SOL_SOCKET, opt, &old, sizeof(old)) ||
+	    old == new)
+		return 1;
+	if (bpf_setsockopt(ctx, SOL_SOCKET, opt, &new, sizeof(new)))
+		return 1;
+	if (bpf_getsockopt(ctx, SOL_SOCKET, opt, &tmp, sizeof(tmp)) ||
+	    tmp != new)
+		return 1;
+	if (bpf_setsockopt(ctx, SOL_SOCKET, opt, &old, sizeof(old)))
 		return 1;
 
 	return 0;
@@ -107,6 +136,10 @@ int bind_v6_prog(struct bpf_sock_addr *ctx)
 	if (bind_to_device(ctx))
 		return 0;
 
+	/* Test for misc socket options. */
+	if (misc_opts(ctx, SO_MARK) || misc_opts(ctx, SO_PRIORITY))
+		return 0;
+
 	ctx->user_ip6[0] = bpf_htonl(SERV6_REWRITE_IP_0);
 	ctx->user_ip6[1] = bpf_htonl(SERV6_REWRITE_IP_1);
 	ctx->user_ip6[2] = bpf_htonl(SERV6_REWRITE_IP_2);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: allow to retrieve sol_socket opts from sock_addr progs
  2021-01-11 16:17 [PATCH bpf-next 1/2] bpf: allow to retrieve sol_socket opts from sock_addr progs Daniel Borkmann
  2021-01-11 16:17 ` [PATCH bpf-next 2/2] bpf: extend bind v4/v6 selftests for mark/prio/bindtoifindex Daniel Borkmann
@ 2021-01-11 20:03 ` Yonghong Song
  1 sibling, 0 replies; 5+ messages in thread
From: Yonghong Song @ 2021-01-11 20:03 UTC (permalink / raw
  To: Daniel Borkmann, ast; +Cc: bpf, netdev



On 1/11/21 8:17 AM, Daniel Borkmann wrote:
> The _bpf_setsockopt() is able to set some of the SOL_SOCKET level options,
> however, _bpf_getsockopt() has little support to actually retrieve them.
> This small patch adds few misc options such as SO_MARK, SO_PRIORITY and
> SO_BINDTOIFINDEX. For the latter getter and setter are added. The mark and
> priority in particular allow to retrieve the options from BPF cgroup hooks
> to then implement custom behavior / settings on the syscall hooks compared
> to other sockets that stick to the defaults, for example.
> 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

Acked-by: Yonghong Song <yhs@fb.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf: extend bind v4/v6 selftests for mark/prio/bindtoifindex
  2021-01-11 16:17 ` [PATCH bpf-next 2/2] bpf: extend bind v4/v6 selftests for mark/prio/bindtoifindex Daniel Borkmann
@ 2021-01-11 20:15   ` Yonghong Song
  2021-01-11 21:01     ` Daniel Borkmann
  0 siblings, 1 reply; 5+ messages in thread
From: Yonghong Song @ 2021-01-11 20:15 UTC (permalink / raw
  To: Daniel Borkmann, ast; +Cc: bpf, netdev



On 1/11/21 8:17 AM, Daniel Borkmann wrote:
> Extend existing cgroup bind4/bind6 tests to add coverage for setting and
> retrieving SO_MARK, SO_PRIORITY and SO_BINDTOIFINDEX at the bind hook.
> 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

Ack with a minor comments below.

Acked-by: Yonghong Song <yhs@fb.com>

> ---
>   .../testing/selftests/bpf/progs/bind4_prog.c  | 41 +++++++++++++++++--
>   .../testing/selftests/bpf/progs/bind6_prog.c  | 41 +++++++++++++++++--
>   2 files changed, 74 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
> index c6520f21f5f5..4479ac27b1d3 100644
> --- a/tools/testing/selftests/bpf/progs/bind4_prog.c
> +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
> @@ -29,18 +29,47 @@ static __inline int bind_to_device(struct bpf_sock_addr *ctx)
>   	char veth2[IFNAMSIZ] = "test_sock_addr2";
>   	char missing[IFNAMSIZ] = "nonexistent_dev";
>   	char del_bind[IFNAMSIZ] = "";
> +	int veth1_idx, veth2_idx;
>   
>   	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
> -				&veth1, sizeof(veth1)))
> +			   &veth1, sizeof(veth1)))
> +		return 1;
> +	if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
> +			   &veth1_idx, sizeof(veth1_idx)) || !veth1_idx)
>   		return 1;
>   	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
> -				&veth2, sizeof(veth2)))
> +			   &veth2, sizeof(veth2)))
> +		return 1;
> +	if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
> +			   &veth2_idx, sizeof(veth2_idx)) || !veth2_idx ||
> +	    veth1_idx == veth2_idx)
>   		return 1;
>   	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
> -				&missing, sizeof(missing)) != -ENODEV)
> +			   &missing, sizeof(missing)) != -ENODEV)
> +		return 1;
> +	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
> +			   &veth1_idx, sizeof(veth1_idx)))
>   		return 1;
>   	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
> -				&del_bind, sizeof(del_bind)))
> +			   &del_bind, sizeof(del_bind)))
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static __inline int misc_opts(struct bpf_sock_addr *ctx, int opt)
> +{
> +	int old, tmp, new = 0xeb9f;
> +
> +	if (bpf_getsockopt(ctx, SOL_SOCKET, opt, &old, sizeof(old)) ||
> +	    old == new)
> +		return 1;

Here, we assume old never equals to new. it would be good to add
a comment to explicitly state this is true. Maybe in the future
somebody will try to add more misc_opts which might have conflict
here.

Alternatively, you could pass in "new" values
from user space with global variables for each option,
but that may be an overkill.

> +	if (bpf_setsockopt(ctx, SOL_SOCKET, opt, &new, sizeof(new)))
> +		return 1;
> +	if (bpf_getsockopt(ctx, SOL_SOCKET, opt, &tmp, sizeof(tmp)) ||
> +	    tmp != new)
> +		return 1;
> +	if (bpf_setsockopt(ctx, SOL_SOCKET, opt, &old, sizeof(old)))
>   		return 1;
>   
>   	return 0;
> @@ -93,6 +122,10 @@ int bind_v4_prog(struct bpf_sock_addr *ctx)
>   	if (bind_to_device(ctx))
>   		return 0;
>   
> +	/* Test for misc socket options. */
> +	if (misc_opts(ctx, SO_MARK) || misc_opts(ctx, SO_PRIORITY))
> +		return 0;
> +
>   	ctx->user_ip4 = bpf_htonl(SERV4_REWRITE_IP);
>   	ctx->user_port = bpf_htons(SERV4_REWRITE_PORT);
>   
> diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c
> index 4358e44dcf47..1b4142fcdd4b 100644
> --- a/tools/testing/selftests/bpf/progs/bind6_prog.c
> +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c
> @@ -35,18 +35,47 @@ static __inline int bind_to_device(struct bpf_sock_addr *ctx)
>   	char veth2[IFNAMSIZ] = "test_sock_addr2";
>   	char missing[IFNAMSIZ] = "nonexistent_dev";
>   	char del_bind[IFNAMSIZ] = "";
> +	int veth1_idx, veth2_idx;
>   
>   	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
> -				&veth1, sizeof(veth1)))
> +			   &veth1, sizeof(veth1)))
> +		return 1;
> +	if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
> +			   &veth1_idx, sizeof(veth1_idx)) || !veth1_idx)
>   		return 1;
>   	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
> -				&veth2, sizeof(veth2)))
> +			   &veth2, sizeof(veth2)))
> +		return 1;
> +	if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
> +			   &veth2_idx, sizeof(veth2_idx)) || !veth2_idx ||
> +	    veth1_idx == veth2_idx)
>   		return 1;
>   	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
> -				&missing, sizeof(missing)) != -ENODEV)
> +			   &missing, sizeof(missing)) != -ENODEV)
> +		return 1;
> +	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
> +			   &veth1_idx, sizeof(veth1_idx)))
>   		return 1;
>   	if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
> -				&del_bind, sizeof(del_bind)))
> +			   &del_bind, sizeof(del_bind)))
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static __inline int misc_opts(struct bpf_sock_addr *ctx, int opt)
> +{
> +	int old, tmp, new = 0xeb9f;
> +
> +	if (bpf_getsockopt(ctx, SOL_SOCKET, opt, &old, sizeof(old)) ||
> +	    old == new)
> +		return 1;
> +	if (bpf_setsockopt(ctx, SOL_SOCKET, opt, &new, sizeof(new)))
> +		return 1;
> +	if (bpf_getsockopt(ctx, SOL_SOCKET, opt, &tmp, sizeof(tmp)) ||
> +	    tmp != new)
> +		return 1;
> +	if (bpf_setsockopt(ctx, SOL_SOCKET, opt, &old, sizeof(old)))
>   		return 1;
>   
>   	return 0;
> @@ -107,6 +136,10 @@ int bind_v6_prog(struct bpf_sock_addr *ctx)
>   	if (bind_to_device(ctx))
>   		return 0;
>   
> +	/* Test for misc socket options. */
> +	if (misc_opts(ctx, SO_MARK) || misc_opts(ctx, SO_PRIORITY))
> +		return 0;
> +
>   	ctx->user_ip6[0] = bpf_htonl(SERV6_REWRITE_IP_0);
>   	ctx->user_ip6[1] = bpf_htonl(SERV6_REWRITE_IP_1);
>   	ctx->user_ip6[2] = bpf_htonl(SERV6_REWRITE_IP_2);
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf: extend bind v4/v6 selftests for mark/prio/bindtoifindex
  2021-01-11 20:15   ` Yonghong Song
@ 2021-01-11 21:01     ` Daniel Borkmann
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2021-01-11 21:01 UTC (permalink / raw
  To: Yonghong Song, ast; +Cc: bpf, netdev

On 1/11/21 9:15 PM, Yonghong Song wrote:
> On 1/11/21 8:17 AM, Daniel Borkmann wrote:
>> Extend existing cgroup bind4/bind6 tests to add coverage for setting and
>> retrieving SO_MARK, SO_PRIORITY and SO_BINDTOIFINDEX at the bind hook.
>>
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> 
> Ack with a minor comments below.
> 
> Acked-by: Yonghong Song <yhs@fb.com>
> 
>> ---
>>   .../testing/selftests/bpf/progs/bind4_prog.c  | 41 +++++++++++++++++--
>>   .../testing/selftests/bpf/progs/bind6_prog.c  | 41 +++++++++++++++++--
>>   2 files changed, 74 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
>> index c6520f21f5f5..4479ac27b1d3 100644
>> --- a/tools/testing/selftests/bpf/progs/bind4_prog.c
>> +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
>> @@ -29,18 +29,47 @@ static __inline int bind_to_device(struct bpf_sock_addr *ctx)
>>       char veth2[IFNAMSIZ] = "test_sock_addr2";
>>       char missing[IFNAMSIZ] = "nonexistent_dev";
>>       char del_bind[IFNAMSIZ] = "";
>> +    int veth1_idx, veth2_idx;
>>       if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
>> -                &veth1, sizeof(veth1)))
>> +               &veth1, sizeof(veth1)))
>> +        return 1;
>> +    if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
>> +               &veth1_idx, sizeof(veth1_idx)) || !veth1_idx)
>>           return 1;
>>       if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
>> -                &veth2, sizeof(veth2)))
>> +               &veth2, sizeof(veth2)))
>> +        return 1;
>> +    if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
>> +               &veth2_idx, sizeof(veth2_idx)) || !veth2_idx ||
>> +        veth1_idx == veth2_idx)
>>           return 1;
>>       if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
>> -                &missing, sizeof(missing)) != -ENODEV)
>> +               &missing, sizeof(missing)) != -ENODEV)
>> +        return 1;
>> +    if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX,
>> +               &veth1_idx, sizeof(veth1_idx)))
>>           return 1;
>>       if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE,
>> -                &del_bind, sizeof(del_bind)))
>> +               &del_bind, sizeof(del_bind)))
>> +        return 1;
>> +
>> +    return 0;
>> +}
>> +
>> +static __inline int misc_opts(struct bpf_sock_addr *ctx, int opt)
>> +{
>> +    int old, tmp, new = 0xeb9f;
>> +
>> +    if (bpf_getsockopt(ctx, SOL_SOCKET, opt, &old, sizeof(old)) ||
>> +        old == new)
>> +        return 1;
> 
> Here, we assume old never equals to new. it would be good to add
> a comment to explicitly state this is true. Maybe in the future
> somebody will try to add more misc_opts which might have conflict
> here.

I thought it's obvious, but yes I can add a comment.

> Alternatively, you could pass in "new" values
> from user space with global variables for each option,
> but that may be an overkill.

Agree, that's overkill.

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-01-11 21:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-11 16:17 [PATCH bpf-next 1/2] bpf: allow to retrieve sol_socket opts from sock_addr progs Daniel Borkmann
2021-01-11 16:17 ` [PATCH bpf-next 2/2] bpf: extend bind v4/v6 selftests for mark/prio/bindtoifindex Daniel Borkmann
2021-01-11 20:15   ` Yonghong Song
2021-01-11 21:01     ` Daniel Borkmann
2021-01-11 20:03 ` [PATCH bpf-next 1/2] bpf: allow to retrieve sol_socket opts from sock_addr progs Yonghong Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.