All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: Support sk lookup in netns with id 0
@ 2018-11-26 21:27 Joe Stringer
  2018-11-26 22:08 ` David Ahern
  0 siblings, 1 reply; 6+ messages in thread
From: Joe Stringer @ 2018-11-26 21:27 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, dsahern, nicolas.dichtel

David Ahern and Nicolas Dichtel report that the handling of the netns id
0 is incorrect for the BPF socket lookup helpers: rather than finding
the netns with id 0, it is resolving to the current netns. This renders
the netns_id 0 inaccessible.

To fix this, adjust the API for the netns to treat all u32 values with
the highest bit set (BPF_F_SK_CURRENT_NS) as a lookup in the current
netns, while any values with a lower value (including zero) would result
in a lookup for a socket in the netns corresponding to that id. As
before, if the netns with that ID does not exist, no socket will be
found.

Signed-off-by: Joe Stringer <joe@wand.net.nz>
---
 include/uapi/linux/bpf.h                      | 29 +++++++++-------
 net/core/filter.c                             | 16 ++++-----
 tools/include/uapi/linux/bpf.h                | 33 ++++++++++++-------
 .../selftests/bpf/test_sk_lookup_kern.c       | 18 +++++-----
 4 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 852dc17ab47a..543945d520b9 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2187,12 +2187,13 @@ union bpf_attr {
  *		**sizeof**\ (*tuple*\ **->ipv6**)
  *			Look for an IPv6 socket.
  *
- *		If the *netns* is zero, then the socket lookup table in the
- *		netns associated with the *ctx* will be used. For the TC hooks,
- *		this in the netns of the device in the skb. For socket hooks,
- *		this in the netns of the socket. If *netns* is non-zero, then
- *		it specifies the ID of the netns relative to the netns
- *		associated with the *ctx*.
+ *		If the *netns* is **BPF_F_SK_CURRENT_NS** or greater, then the
+ *		socket lookup table in the netns associated with the *ctx* will
+ *		will be used. For the TC hooks, this is the netns of the device
+ *		in the skb. For socket hooks, this is the netns of the socket.
+ *		If *netns* is less than **BPF_F_SK_CURRENT_NS**, then it
+ *		specifies the ID of the netns relative to the netns associated
+ *		with the *ctx*.
  *
  *		All values for *flags* are reserved for future usage, and must
  *		be left at zero.
@@ -2219,12 +2220,13 @@ union bpf_attr {
  *		**sizeof**\ (*tuple*\ **->ipv6**)
  *			Look for an IPv6 socket.
  *
- *		If the *netns* is zero, then the socket lookup table in the
- *		netns associated with the *ctx* will be used. For the TC hooks,
- *		this in the netns of the device in the skb. For socket hooks,
- *		this in the netns of the socket. If *netns* is non-zero, then
- *		it specifies the ID of the netns relative to the netns
- *		associated with the *ctx*.
+ *		If the *netns* is **BPF_F_SK_CURRENT_NS** or greater, then the
+ *		socket lookup table in the netns associated with the *ctx* will
+ *		will be used. For the TC hooks, this is the netns of the device
+ *		in the skb. For socket hooks, this is the netns of the socket.
+ *		If *netns* is less than **BPF_F_SK_CURRENT_NS**, then it
+ *		specifies the ID of the netns relative to the netns associated
+ *		with the *ctx*.
  *
  *		All values for *flags* are reserved for future usage, and must
  *		be left at zero.
@@ -2405,6 +2407,9 @@ enum bpf_func_id {
 /* BPF_FUNC_perf_event_output for sk_buff input context. */
 #define BPF_F_CTXLEN_MASK		(0xfffffULL << 32)
 
+/* BPF_FUNC_sk_lookup_tcp and BPF_FUNC_sk_lookup_udp flags. */
+#define BPF_F_SK_CURRENT_NS		0x80000000 /* For netns field */
+
 /* Mode for BPF_FUNC_skb_adjust_room helper. */
 enum bpf_adj_room_mode {
 	BPF_ADJ_ROOM_NET,
diff --git a/net/core/filter.c b/net/core/filter.c
index 9a1327eb25fa..8c8a7ad3f5e6 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4882,7 +4882,7 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
  */
 static unsigned long
 bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
-	      u8 proto, u64 netns_id, u64 flags)
+	      u8 proto, u32 netns_id, u64 flags)
 {
 	struct net *caller_net;
 	struct sock *sk = NULL;
@@ -4890,22 +4890,22 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
 	struct net *net;
 
 	family = len == sizeof(tuple->ipv4) ? AF_INET : AF_INET6;
-	if (unlikely(family == AF_UNSPEC || netns_id > U32_MAX || flags))
+	if (unlikely(family == AF_UNSPEC || flags))
 		goto out;
 
 	if (skb->dev)
 		caller_net = dev_net(skb->dev);
 	else
 		caller_net = sock_net(skb->sk);
-	if (netns_id) {
+	if (netns_id & BPF_F_SK_CURRENT_NS) {
+		net = caller_net;
+		sk = sk_lookup(net, tuple, skb, family, proto);
+	} else {
 		net = get_net_ns_by_id(caller_net, netns_id);
 		if (unlikely(!net))
 			goto out;
 		sk = sk_lookup(net, tuple, skb, family, proto);
 		put_net(net);
-	} else {
-		net = caller_net;
-		sk = sk_lookup(net, tuple, skb, family, proto);
 	}
 
 	if (sk)
@@ -4915,7 +4915,7 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
 }
 
 BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
-	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
+	   struct bpf_sock_tuple *, tuple, u32, len, u32, netns_id, u64, flags)
 {
 	return bpf_sk_lookup(skb, tuple, len, IPPROTO_TCP, netns_id, flags);
 }
@@ -4933,7 +4933,7 @@ static const struct bpf_func_proto bpf_sk_lookup_tcp_proto = {
 };
 
 BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
-	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
+	   struct bpf_sock_tuple *, tuple, u32, len, u32, netns_id, u64, flags)
 {
 	return bpf_sk_lookup(skb, tuple, len, IPPROTO_UDP, netns_id, flags);
 }
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 852dc17ab47a..73649732513d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2187,12 +2187,13 @@ union bpf_attr {
  *		**sizeof**\ (*tuple*\ **->ipv6**)
  *			Look for an IPv6 socket.
  *
- *		If the *netns* is zero, then the socket lookup table in the
- *		netns associated with the *ctx* will be used. For the TC hooks,
- *		this in the netns of the device in the skb. For socket hooks,
- *		this in the netns of the socket. If *netns* is non-zero, then
- *		it specifies the ID of the netns relative to the netns
- *		associated with the *ctx*.
+ *		If the *netns* is **BPF_F_SK_CURRENT_NS** or greater, then the
+ *		socket lookup table in the netns associated with the *ctx* will
+ *		will be used. For the TC hooks, this is the netns of the device
+ *		in the skb. For socket hooks, this is the netns of the socket.
+ *		If *netns* is less than **BPF_F_SK_CURRENT_NS**, then it
+ *		specifies the ID of the netns relative to the netns associated
+ *		with the *ctx*.
  *
  *		All values for *flags* are reserved for future usage, and must
  *		be left at zero.
@@ -2201,6 +2202,8 @@ union bpf_attr {
  *		**CONFIG_NET** configuration option.
  *	Return
  *		Pointer to *struct bpf_sock*, or NULL in case of failure.
+ *		For sockets with reuseport option, *struct bpf_sock*
+ *		return is from reuse->socks[] using hash of the packet.
  *
  * struct bpf_sock *bpf_sk_lookup_udp(void *ctx, struct bpf_sock_tuple *tuple, u32 tuple_size, u32 netns, u64 flags)
  *	Description
@@ -2219,12 +2222,13 @@ union bpf_attr {
  *		**sizeof**\ (*tuple*\ **->ipv6**)
  *			Look for an IPv6 socket.
  *
- *		If the *netns* is zero, then the socket lookup table in the
- *		netns associated with the *ctx* will be used. For the TC hooks,
- *		this in the netns of the device in the skb. For socket hooks,
- *		this in the netns of the socket. If *netns* is non-zero, then
- *		it specifies the ID of the netns relative to the netns
- *		associated with the *ctx*.
+ *		If the *netns* is **BPF_F_SK_CURRENT_NS** or greater, then the
+ *		socket lookup table in the netns associated with the *ctx* will
+ *		will be used. For the TC hooks, this is the netns of the device
+ *		in the skb. For socket hooks, this is the netns of the socket.
+ *		If *netns* is less than **BPF_F_SK_CURRENT_NS**, then it
+ *		specifies the ID of the netns relative to the netns associated
+ *		with the *ctx*.
  *
  *		All values for *flags* are reserved for future usage, and must
  *		be left at zero.
@@ -2233,6 +2237,8 @@ union bpf_attr {
  *		**CONFIG_NET** configuration option.
  *	Return
  *		Pointer to *struct bpf_sock*, or NULL in case of failure.
+ *		For sockets with reuseport option, *struct bpf_sock*
+ *		return is from reuse->socks[] using hash of the packet.
  *
  * int bpf_sk_release(struct bpf_sock *sk)
  *	Description
@@ -2405,6 +2411,9 @@ enum bpf_func_id {
 /* BPF_FUNC_perf_event_output for sk_buff input context. */
 #define BPF_F_CTXLEN_MASK		(0xfffffULL << 32)
 
+/* BPF_FUNC_sk_lookup_tcp and BPF_FUNC_sk_lookup_udp flags. */
+#define BPF_F_SK_CURRENT_NS		0x80000000 /* For netns field */
+
 /* Mode for BPF_FUNC_skb_adjust_room helper. */
 enum bpf_adj_room_mode {
 	BPF_ADJ_ROOM_NET,
diff --git a/tools/testing/selftests/bpf/test_sk_lookup_kern.c b/tools/testing/selftests/bpf/test_sk_lookup_kern.c
index b745bdc08c2b..673073d1a9eb 100644
--- a/tools/testing/selftests/bpf/test_sk_lookup_kern.c
+++ b/tools/testing/selftests/bpf/test_sk_lookup_kern.c
@@ -72,7 +72,7 @@ int bpf_sk_lookup_test0(struct __sk_buff *skb)
 		return TC_ACT_SHOT;
 
 	tuple_len = ipv4 ? sizeof(tuple->ipv4) : sizeof(tuple->ipv6);
-	sk = bpf_sk_lookup_tcp(skb, tuple, tuple_len, 0, 0);
+	sk = bpf_sk_lookup_tcp(skb, tuple, tuple_len, BPF_F_SK_CURRENT_NS, 0);
 	if (sk)
 		bpf_sk_release(sk);
 	return sk ? TC_ACT_OK : TC_ACT_UNSPEC;
@@ -84,7 +84,7 @@ int bpf_sk_lookup_test1(struct __sk_buff *skb)
 	struct bpf_sock_tuple tuple = {};
 	struct bpf_sock *sk;
 
-	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), BPF_F_SK_CURRENT_NS, 0);
 	if (sk)
 		bpf_sk_release(sk);
 	return 0;
@@ -97,7 +97,7 @@ int bpf_sk_lookup_uaf(struct __sk_buff *skb)
 	struct bpf_sock *sk;
 	__u32 family = 0;
 
-	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), BPF_F_SK_CURRENT_NS, 0);
 	if (sk) {
 		bpf_sk_release(sk);
 		family = sk->family;
@@ -112,7 +112,7 @@ int bpf_sk_lookup_modptr(struct __sk_buff *skb)
 	struct bpf_sock *sk;
 	__u32 family;
 
-	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), BPF_F_SK_CURRENT_NS, 0);
 	if (sk) {
 		sk += 1;
 		bpf_sk_release(sk);
@@ -127,7 +127,7 @@ int bpf_sk_lookup_modptr_or_null(struct __sk_buff *skb)
 	struct bpf_sock *sk;
 	__u32 family;
 
-	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), BPF_F_SK_CURRENT_NS, 0);
 	sk += 1;
 	if (sk)
 		bpf_sk_release(sk);
@@ -139,7 +139,7 @@ int bpf_sk_lookup_test2(struct __sk_buff *skb)
 {
 	struct bpf_sock_tuple tuple = {};
 
-	bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), BPF_F_SK_CURRENT_NS, 0);
 	return 0;
 }
 
@@ -149,7 +149,7 @@ int bpf_sk_lookup_test3(struct __sk_buff *skb)
 	struct bpf_sock_tuple tuple = {};
 	struct bpf_sock *sk;
 
-	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), BPF_F_SK_CURRENT_NS, 0);
 	bpf_sk_release(sk);
 	bpf_sk_release(sk);
 	return 0;
@@ -161,7 +161,7 @@ int bpf_sk_lookup_test4(struct __sk_buff *skb)
 	struct bpf_sock_tuple tuple = {};
 	struct bpf_sock *sk;
 
-	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	sk = bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), BPF_F_SK_CURRENT_NS, 0);
 	bpf_sk_release(sk);
 	return 0;
 }
@@ -169,7 +169,7 @@ int bpf_sk_lookup_test4(struct __sk_buff *skb)
 void lookup_no_release(struct __sk_buff *skb)
 {
 	struct bpf_sock_tuple tuple = {};
-	bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), 0, 0);
+	bpf_sk_lookup_tcp(skb, &tuple, sizeof(tuple), BPF_F_SK_CURRENT_NS, 0);
 }
 
 SEC("fail_no_release_subcall")
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf] bpf: Support sk lookup in netns with id 0
  2018-11-26 21:27 [PATCH bpf] bpf: Support sk lookup in netns with id 0 Joe Stringer
@ 2018-11-26 22:08 ` David Ahern
  2018-11-27 14:49   ` Nicolas Dichtel
  0 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2018-11-26 22:08 UTC (permalink / raw)
  To: Joe Stringer, daniel, ast; +Cc: netdev, nicolas.dichtel

On 11/26/18 2:27 PM, Joe Stringer wrote:
> @@ -2405,6 +2407,9 @@ enum bpf_func_id {
>  /* BPF_FUNC_perf_event_output for sk_buff input context. */
>  #define BPF_F_CTXLEN_MASK		(0xfffffULL << 32)
>  
> +/* BPF_FUNC_sk_lookup_tcp and BPF_FUNC_sk_lookup_udp flags. */
> +#define BPF_F_SK_CURRENT_NS		0x80000000 /* For netns field */
> +

I went down the nsid road because it will be needed for other use cases
(e.g., device lookups), and we should have a general API for network
namespaces. Given that, I think the _SK should be dropped from the name.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf] bpf: Support sk lookup in netns with id 0
  2018-11-26 22:08 ` David Ahern
@ 2018-11-27 14:49   ` Nicolas Dichtel
  2018-11-27 18:01     ` Joe Stringer
  0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Dichtel @ 2018-11-27 14:49 UTC (permalink / raw)
  To: David Ahern, Joe Stringer, daniel, ast; +Cc: netdev

Le 26/11/2018 à 23:08, David Ahern a écrit :
> On 11/26/18 2:27 PM, Joe Stringer wrote:
>> @@ -2405,6 +2407,9 @@ enum bpf_func_id {
>>  /* BPF_FUNC_perf_event_output for sk_buff input context. */
>>  #define BPF_F_CTXLEN_MASK		(0xfffffULL << 32)
>>  
>> +/* BPF_FUNC_sk_lookup_tcp and BPF_FUNC_sk_lookup_udp flags. */
>> +#define BPF_F_SK_CURRENT_NS		0x80000000 /* For netns field */
>> +
> 
> I went down the nsid road because it will be needed for other use cases
> (e.g., device lookups), and we should have a general API for network
> namespaces. Given that, I think the _SK should be dropped from the name.
> 
Would it not be possible to have a s32 instead of an u32 for the coming APIs?
It would be better to match the current netlink and kernel APIs.


Regards,
Nicolas

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf] bpf: Support sk lookup in netns with id 0
  2018-11-27 14:49   ` Nicolas Dichtel
@ 2018-11-27 18:01     ` Joe Stringer
  2018-11-27 21:11       ` Alexei Starovoitov
  0 siblings, 1 reply; 6+ messages in thread
From: Joe Stringer @ 2018-11-27 18:01 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: David Ahern, Joe Stringer, daniel, ast, netdev

On Tue, 27 Nov 2018 at 06:49, Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>
> Le 26/11/2018 à 23:08, David Ahern a écrit :
> > On 11/26/18 2:27 PM, Joe Stringer wrote:
> >> @@ -2405,6 +2407,9 @@ enum bpf_func_id {
> >>  /* BPF_FUNC_perf_event_output for sk_buff input context. */
> >>  #define BPF_F_CTXLEN_MASK           (0xfffffULL << 32)
> >>
> >> +/* BPF_FUNC_sk_lookup_tcp and BPF_FUNC_sk_lookup_udp flags. */
> >> +#define BPF_F_SK_CURRENT_NS         0x80000000 /* For netns field */
> >> +
> >
> > I went down the nsid road because it will be needed for other use cases
> > (e.g., device lookups), and we should have a general API for network
> > namespaces. Given that, I think the _SK should be dropped from the name.

Fair point, I'll drop _SK from the name

> >
> Would it not be possible to have a s32 instead of an u32 for the coming APIs?
> It would be better to match the current netlink and kernel APIs.

Sure, I'll look into this.

I had earlier considered whether it's worth attempting to leave the
upper 32 bits of this parameter open for potential future expansion,
but at this point I'm not taking that into consideration. If anyone
has preferences or thoughts on that I'd be interested to hear them.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf] bpf: Support sk lookup in netns with id 0
  2018-11-27 18:01     ` Joe Stringer
@ 2018-11-27 21:11       ` Alexei Starovoitov
  2018-11-28 19:24         ` Joe Stringer
  0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2018-11-27 21:11 UTC (permalink / raw)
  To: Joe Stringer; +Cc: Nicolas Dichtel, David Ahern, daniel, ast, netdev

On Tue, Nov 27, 2018 at 10:01:40AM -0800, Joe Stringer wrote:
> On Tue, 27 Nov 2018 at 06:49, Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> >
> > Le 26/11/2018 à 23:08, David Ahern a écrit :
> > > On 11/26/18 2:27 PM, Joe Stringer wrote:
> > >> @@ -2405,6 +2407,9 @@ enum bpf_func_id {
> > >>  /* BPF_FUNC_perf_event_output for sk_buff input context. */
> > >>  #define BPF_F_CTXLEN_MASK           (0xfffffULL << 32)
> > >>
> > >> +/* BPF_FUNC_sk_lookup_tcp and BPF_FUNC_sk_lookup_udp flags. */
> > >> +#define BPF_F_SK_CURRENT_NS         0x80000000 /* For netns field */
> > >> +
> > >
> > > I went down the nsid road because it will be needed for other use cases
> > > (e.g., device lookups), and we should have a general API for network
> > > namespaces. Given that, I think the _SK should be dropped from the name.
> 
> Fair point, I'll drop _SK from the name
> 
> > >
> > Would it not be possible to have a s32 instead of an u32 for the coming APIs?
> > It would be better to match the current netlink and kernel APIs.
> 
> Sure, I'll look into this.
> 
> I had earlier considered whether it's worth attempting to leave the
> upper 32 bits of this parameter open for potential future expansion,
> but at this point I'm not taking that into consideration. If anyone
> has preferences or thoughts on that I'd be interested to hear them.

Can we keep u64 as an argument type and do
if ((s32)netns_id < 0) {
  net = caller_net;
} else {
  if (netns_id > S32_MAX)
    goto err;
  net = get_net_ns_by_id(caller_net, netns_id);
}

No need for extra macro in such case and passing -1 would match the rest of the kernel.
Upper 32-bit would still be open for future expansion.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf] bpf: Support sk lookup in netns with id 0
  2018-11-27 21:11       ` Alexei Starovoitov
@ 2018-11-28 19:24         ` Joe Stringer
  0 siblings, 0 replies; 6+ messages in thread
From: Joe Stringer @ 2018-11-28 19:24 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Joe Stringer, Nicolas Dichtel, David Ahern, daniel, ast, netdev

On Tue, 27 Nov 2018 at 13:12, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Nov 27, 2018 at 10:01:40AM -0800, Joe Stringer wrote:
> > On Tue, 27 Nov 2018 at 06:49, Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> > >
> > > Le 26/11/2018 ą 23:08, David Ahern a écrit :
> > > > On 11/26/18 2:27 PM, Joe Stringer wrote:
> > > >> @@ -2405,6 +2407,9 @@ enum bpf_func_id {
> > > >>  /* BPF_FUNC_perf_event_output for sk_buff input context. */
> > > >>  #define BPF_F_CTXLEN_MASK           (0xfffffULL << 32)
> > > >>
> > > >> +/* BPF_FUNC_sk_lookup_tcp and BPF_FUNC_sk_lookup_udp flags. */
> > > >> +#define BPF_F_SK_CURRENT_NS         0x80000000 /* For netns field */
> > > >> +
> > > >
> > > > I went down the nsid road because it will be needed for other use cases
> > > > (e.g., device lookups), and we should have a general API for network
> > > > namespaces. Given that, I think the _SK should be dropped from the name.
> >
> > Fair point, I'll drop _SK from the name
> >
> > > >
> > > Would it not be possible to have a s32 instead of an u32 for the coming APIs?
> > > It would be better to match the current netlink and kernel APIs.
> >
> > Sure, I'll look into this.
> >
> > I had earlier considered whether it's worth attempting to leave the
> > upper 32 bits of this parameter open for potential future expansion,
> > but at this point I'm not taking that into consideration. If anyone
> > has preferences or thoughts on that I'd be interested to hear them.
>
> Can we keep u64 as an argument type and do
> if ((s32)netns_id < 0) {
>   net = caller_net;
> } else {
>   if (netns_id > S32_MAX)
>     goto err;
>   net = get_net_ns_by_id(caller_net, netns_id);
> }
>
> No need for extra macro in such case and passing -1 would match the rest of the kernel.
> Upper 32-bit would still be open for future expansion.

Sounds good.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-29  6:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-26 21:27 [PATCH bpf] bpf: Support sk lookup in netns with id 0 Joe Stringer
2018-11-26 22:08 ` David Ahern
2018-11-27 14:49   ` Nicolas Dichtel
2018-11-27 18:01     ` Joe Stringer
2018-11-27 21:11       ` Alexei Starovoitov
2018-11-28 19:24         ` Joe Stringer

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.