[PATCH bpf-next 0/4] bpf: Support socket lookup in CGROUP_SOCK_ADDR progs

[PATCH bpf-next 0/4] bpf: Support socket lookup in CGROUP_SOCK_ADDR progs

[Andrey Ignatov]

This patch set makes bpf_sk_lookup_tcp, bpf_sk_lookup_udp and
bpf_sk_release helpers available in programs of type
BPF_PROG_TYPE_CGROUP_SOCK_ADDR.

Patch 1 is a fix for bpf_sk_lookup_udp that was already sent to netdev
separately for bpf (stable) tree. Here it's prerequisite for patch 4.

Patch 2 is refactoring to prepare for patch 3. Similar refactoring was done
as part of "bpf: Extend the sk_lookup() helper to XDP hookpoint." patch
published on netdev earlier but not merged yet. This patch set can reuse
the work done for xdp if it's merged. This patch doesn't make any logic
changes and simply moves code around.

Patch 3 is the main patch in the set, it makes the helpers available for
BPF_PROG_TYPE_CGROUP_SOCK_ADDR and provides more details about use-case.

Patch 4 adds selftest for new functionality.


Andrey Ignatov (4):
  bpf: Fix IPv6 dport byte order in bpf_sk_lookup_udp
  bpf: Split bpf_sk_lookup
  bpf: Support socket lookup in CGROUP_SOCK_ADDR progs
  selftest/bpf: Use bpf_sk_lookup_{tcp,udp} in test_sock_addr

 net/core/filter.c                           | 96 +++++++++++++++++----
 tools/testing/selftests/bpf/connect4_prog.c | 43 +++++++--
 tools/testing/selftests/bpf/connect6_prog.c | 56 +++++++++---
 3 files changed, 156 insertions(+), 39 deletions(-)

-- 
2.17.1

[PATCH bpf-next 1/4] bpf: Fix IPv6 dport byte order in bpf_sk_lookup_udp

[Andrey Ignatov]

Lookup functions in sk_lookup have different expectations about byte
order of provided arguments.

Specifically __inet_lookup, __udp4_lib_lookup and __udp6_lib_lookup
expect dport to be in network byte order and do ntohs(dport) internally.

At the same time __inet6_lookup expects dport to be in host byte order
and correspondingly name the argument hnum.

sk_lookup works correctly with __inet_lookup, __udp4_lib_lookup and
__inet6_lookup with regard to dport. But in __udp6_lib_lookup case it
uses host instead of expected network byte order. It makes result
returned by bpf_sk_lookup_udp for IPv6 incorrect.

The patch fixes byte order of dport passed to __udp6_lib_lookup.

Originally sk_lookup properly handled UDPv6, but not TCPv6. 5ef0ae84f02a
fixes TCPv6 but breaks UDPv6.

Fixes: 5ef0ae84f02a ("bpf: Fix IPv6 dport byte-order in bpf_sk_lookup")
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
 net/core/filter.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index e521c5ebc7d1..9a1327eb25fa 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4852,18 +4852,17 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
 	} else {
 		struct in6_addr *src6 = (struct in6_addr *)&tuple->ipv6.saddr;
 		struct in6_addr *dst6 = (struct in6_addr *)&tuple->ipv6.daddr;
-		u16 hnum = ntohs(tuple->ipv6.dport);
 		int sdif = inet6_sdif(skb);
 
 		if (proto == IPPROTO_TCP)
 			sk = __inet6_lookup(net, &tcp_hashinfo, skb, 0,
 					    src6, tuple->ipv6.sport,
-					    dst6, hnum,
+					    dst6, ntohs(tuple->ipv6.dport),
 					    dif, sdif, &refcounted);
 		else if (likely(ipv6_bpf_stub))
 			sk = ipv6_bpf_stub->udp6_lib_lookup(net,
 							    src6, tuple->ipv6.sport,
-							    dst6, hnum,
+							    dst6, tuple->ipv6.dport,
 							    dif, sdif,
 							    &udp_table, skb);
 #endif
-- 
2.17.1

[PATCH bpf-next 2/4] bpf: Split bpf_sk_lookup

[Andrey Ignatov]

Split bpf_sk_lookup to separate core functionality, that can be reused
to make socket lookup available to more program types, from
functionality specific to program types that have access to skb.

Core functionality is placed to __bpf_sk_lookup. And bpf_sk_lookup only
gets caller netns and ifindex from skb and passes it to __bpf_sk_lookup.

Program types that don't have access to skb can just pass NULL to
__bpf_sk_lookup that will be handled correctly by both inet{,6}_sdif and
lookup functions.

This is refactoring that simply moves blocks around and does NOT change
existing logic.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 9a1327eb25fa..dc0f86a707b7 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4825,14 +4825,10 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
 
 #ifdef CONFIG_INET
 static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
-			      struct sk_buff *skb, u8 family, u8 proto)
+			      struct sk_buff *skb, u8 family, u8 proto, int dif)
 {
 	bool refcounted = false;
 	struct sock *sk = NULL;
-	int dif = 0;
-
-	if (skb->dev)
-		dif = skb->dev->ifindex;
 
 	if (family == AF_INET) {
 		__be32 src4 = tuple->ipv4.saddr;
@@ -4875,16 +4871,16 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
 	return sk;
 }
 
-/* bpf_sk_lookup performs the core lookup for different types of sockets,
+/* __bpf_sk_lookup performs the core lookup for different types of sockets,
  * taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
  * Returns the socket as an 'unsigned long' to simplify the casting in the
  * callers to satisfy BPF_CALL declarations.
  */
 static unsigned long
-bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
-	      u8 proto, u64 netns_id, u64 flags)
+__bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
+		u8 proto, u64 netns_id, struct net *caller_net, int ifindex,
+		u64 flags)
 {
-	struct net *caller_net;
 	struct sock *sk = NULL;
 	u8 family = AF_UNSPEC;
 	struct net *net;
@@ -4893,19 +4889,15 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
 	if (unlikely(family == AF_UNSPEC || netns_id > U32_MAX || flags))
 		goto out;
 
-	if (skb->dev)
-		caller_net = dev_net(skb->dev);
-	else
-		caller_net = sock_net(skb->sk);
 	if (netns_id) {
 		net = get_net_ns_by_id(caller_net, netns_id);
 		if (unlikely(!net))
 			goto out;
-		sk = sk_lookup(net, tuple, skb, family, proto);
+		sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
 		put_net(net);
 	} else {
 		net = caller_net;
-		sk = sk_lookup(net, tuple, skb, family, proto);
+		sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
 	}
 
 	if (sk)
@@ -4914,6 +4906,22 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
 	return (unsigned long) sk;
 }
 
+static unsigned long
+bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
+	      u8 proto, u64 netns_id, u64 flags)
+{
+	struct net *caller_net = sock_net(skb->sk);
+	int ifindex = 0;
+
+	if (skb->dev) {
+		caller_net = dev_net(skb->dev);
+		ifindex = skb->dev->ifindex;
+	}
+
+	return __bpf_sk_lookup(skb, tuple, len, proto, netns_id, caller_net,
+			       ifindex, flags);
+}
+
 BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
 	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
 {
-- 
2.17.1

[PATCH bpf-next 3/4] bpf: Support socket lookup in CGROUP_SOCK_ADDR progs

[Andrey Ignatov]

Make bpf_sk_lookup_tcp, bpf_sk_lookup_udp and bpf_sk_release helpers
available in programs of type BPF_PROG_TYPE_CGROUP_SOCK_ADDR.

Such programs operate on sockets and have access to socket and struct
sockaddr passed by user to system calls such as sys_bind, sys_connect,
sys_sendmsg.

It's useful to be able to lookup other sockets from these programs.
E.g. sys_connect may lookup IP:port endpoint and if there is a server
socket bound to that endpoint ("server" can be defined by saddr & sport
being zero), redirect client connection to it by rewriting IP:port in
sockaddr passed to sys_connect.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index dc0f86a707b7..2e8575a34a1e 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4971,6 +4971,51 @@ static const struct bpf_func_proto bpf_sk_release_proto = {
 	.ret_type	= RET_INTEGER,
 	.arg1_type	= ARG_PTR_TO_SOCKET,
 };
+
+static unsigned long
+bpf_sock_addr_sk_lookup(struct sock *sk, struct bpf_sock_tuple *tuple, u32 len,
+			u8 proto, u64 netns_id, u64 flags)
+{
+	return __bpf_sk_lookup(NULL, tuple, len, proto, netns_id, sock_net(sk),
+			       0, flags);
+}
+
+BPF_CALL_5(bpf_sock_addr_sk_lookup_tcp, struct bpf_sock_addr_kern *, ctx,
+	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
+{
+	return bpf_sock_addr_sk_lookup(ctx->sk, tuple, len, IPPROTO_TCP,
+				       netns_id, flags);
+}
+
+static const struct bpf_func_proto bpf_sock_addr_sk_lookup_tcp_proto = {
+	.func		= bpf_sock_addr_sk_lookup_tcp,
+	.gpl_only	= false,
+	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_CONST_SIZE,
+	.arg4_type	= ARG_ANYTHING,
+	.arg5_type	= ARG_ANYTHING,
+};
+
+BPF_CALL_5(bpf_sock_addr_sk_lookup_udp, struct bpf_sock_addr_kern *, ctx,
+	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
+{
+	return bpf_sock_addr_sk_lookup(ctx->sk, tuple, len, IPPROTO_UDP,
+				       netns_id, flags);
+}
+
+static const struct bpf_func_proto bpf_sock_addr_sk_lookup_udp_proto = {
+	.func		= bpf_sock_addr_sk_lookup_udp,
+	.gpl_only	= false,
+	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_PTR_TO_MEM,
+	.arg3_type	= ARG_CONST_SIZE,
+	.arg4_type	= ARG_ANYTHING,
+	.arg5_type	= ARG_ANYTHING,
+};
+
 #endif /* CONFIG_INET */
 
 bool bpf_helper_changes_pkt_data(void *func)
@@ -5077,6 +5122,14 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_socket_cookie_sock_addr_proto;
 	case BPF_FUNC_get_local_storage:
 		return &bpf_get_local_storage_proto;
+#ifdef CONFIG_INET
+	case BPF_FUNC_sk_lookup_tcp:
+		return &bpf_sock_addr_sk_lookup_tcp_proto;
+	case BPF_FUNC_sk_lookup_udp:
+		return &bpf_sock_addr_sk_lookup_udp_proto;
+	case BPF_FUNC_sk_release:
+		return &bpf_sk_release_proto;
+#endif /* CONFIG_INET */
 	default:
 		return bpf_base_func_proto(func_id);
 	}
-- 
2.17.1

[PATCH bpf-next 4/4] selftest/bpf: Use bpf_sk_lookup_{tcp,udp} in test_sock_addr

[Andrey Ignatov]

Use bpf_sk_lookup_tcp, bpf_sk_lookup_udp and bpf_sk_release helpers from
test_sock_addr programs to make sure they're available and can lookup
and release socket properly for IPv4/IPv4, TCP/UDP.

Reading from a few fields of returned struct bpf_sock is also tested.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 tools/testing/selftests/bpf/connect4_prog.c | 43 ++++++++++++----
 tools/testing/selftests/bpf/connect6_prog.c | 56 ++++++++++++++++-----
 2 files changed, 78 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/bpf/connect4_prog.c b/tools/testing/selftests/bpf/connect4_prog.c
index 5a88a681d2ab..b8395f3c43e9 100644
--- a/tools/testing/selftests/bpf/connect4_prog.c
+++ b/tools/testing/selftests/bpf/connect4_prog.c
@@ -21,23 +21,48 @@ int _version SEC("version") = 1;
 SEC("cgroup/connect4")
 int connect_v4_prog(struct bpf_sock_addr *ctx)
 {
+	struct bpf_sock_tuple tuple = {};
 	struct sockaddr_in sa;
+	struct bpf_sock *sk;
+
+	/* Verify that new destination is available. */
+	memset(&tuple.ipv4.saddr, 0, sizeof(tuple.ipv4.saddr));
+	memset(&tuple.ipv4.sport, 0, sizeof(tuple.ipv4.sport));
+
+	tuple.ipv4.daddr = bpf_htonl(DST_REWRITE_IP4);
+	tuple.ipv4.dport = bpf_htons(DST_REWRITE_PORT4);
+
+	if (ctx->type != SOCK_STREAM && ctx->type != SOCK_DGRAM)
+		return 0;
+	else if (ctx->type == SOCK_STREAM)
+		sk = bpf_sk_lookup_tcp(ctx, &tuple, sizeof(tuple.ipv4), 0, 0);
+	else
+		sk = bpf_sk_lookup_udp(ctx, &tuple, sizeof(tuple.ipv4), 0, 0);
+
+	if (!sk)
+		return 0;
+
+	if (sk->src_ip4 != tuple.ipv4.daddr ||
+	    sk->src_port != DST_REWRITE_PORT4) {
+		bpf_sk_release(sk);
+		return 0;
+	}
+
+	bpf_sk_release(sk);
 
 	/* Rewrite destination. */
 	ctx->user_ip4 = bpf_htonl(DST_REWRITE_IP4);
 	ctx->user_port = bpf_htons(DST_REWRITE_PORT4);
 
-	if (ctx->type == SOCK_DGRAM || ctx->type == SOCK_STREAM) {
-		///* Rewrite source. */
-		memset(&sa, 0, sizeof(sa));
+	/* Rewrite source. */
+	memset(&sa, 0, sizeof(sa));
 
-		sa.sin_family = AF_INET;
-		sa.sin_port = bpf_htons(0);
-		sa.sin_addr.s_addr = bpf_htonl(SRC_REWRITE_IP4);
+	sa.sin_family = AF_INET;
+	sa.sin_port = bpf_htons(0);
+	sa.sin_addr.s_addr = bpf_htonl(SRC_REWRITE_IP4);
 
-		if (bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa)) != 0)
-			return 0;
-	}
+	if (bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa)) != 0)
+		return 0;
 
 	return 1;
 }
diff --git a/tools/testing/selftests/bpf/connect6_prog.c b/tools/testing/selftests/bpf/connect6_prog.c
index 8ea3f7d12dee..25f5dc7b7aa0 100644
--- a/tools/testing/selftests/bpf/connect6_prog.c
+++ b/tools/testing/selftests/bpf/connect6_prog.c
@@ -29,7 +29,41 @@ int _version SEC("version") = 1;
 SEC("cgroup/connect6")
 int connect_v6_prog(struct bpf_sock_addr *ctx)
 {
+	struct bpf_sock_tuple tuple = {};
 	struct sockaddr_in6 sa;
+	struct bpf_sock *sk;
+
+	/* Verify that new destination is available. */
+	memset(&tuple.ipv6.saddr, 0, sizeof(tuple.ipv6.saddr));
+	memset(&tuple.ipv6.sport, 0, sizeof(tuple.ipv6.sport));
+
+	tuple.ipv6.daddr[0] = bpf_htonl(DST_REWRITE_IP6_0);
+	tuple.ipv6.daddr[1] = bpf_htonl(DST_REWRITE_IP6_1);
+	tuple.ipv6.daddr[2] = bpf_htonl(DST_REWRITE_IP6_2);
+	tuple.ipv6.daddr[3] = bpf_htonl(DST_REWRITE_IP6_3);
+
+	tuple.ipv6.dport = bpf_htons(DST_REWRITE_PORT6);
+
+	if (ctx->type != SOCK_STREAM && ctx->type != SOCK_DGRAM)
+		return 0;
+	else if (ctx->type == SOCK_STREAM)
+		sk = bpf_sk_lookup_tcp(ctx, &tuple, sizeof(tuple.ipv6), 0, 0);
+	else
+		sk = bpf_sk_lookup_udp(ctx, &tuple, sizeof(tuple.ipv6), 0, 0);
+
+	if (!sk)
+		return 0;
+
+	if (sk->src_ip6[0] != tuple.ipv6.daddr[0] ||
+	    sk->src_ip6[1] != tuple.ipv6.daddr[1] ||
+	    sk->src_ip6[2] != tuple.ipv6.daddr[2] ||
+	    sk->src_ip6[3] != tuple.ipv6.daddr[3] ||
+	    sk->src_port != DST_REWRITE_PORT6) {
+		bpf_sk_release(sk);
+		return 0;
+	}
+
+	bpf_sk_release(sk);
 
 	/* Rewrite destination. */
 	ctx->user_ip6[0] = bpf_htonl(DST_REWRITE_IP6_0);
@@ -39,21 +73,19 @@ int connect_v6_prog(struct bpf_sock_addr *ctx)
 
 	ctx->user_port = bpf_htons(DST_REWRITE_PORT6);
 
-	if (ctx->type == SOCK_DGRAM || ctx->type == SOCK_STREAM) {
-		/* Rewrite source. */
-		memset(&sa, 0, sizeof(sa));
+	/* Rewrite source. */
+	memset(&sa, 0, sizeof(sa));
 
-		sa.sin6_family = AF_INET6;
-		sa.sin6_port = bpf_htons(0);
+	sa.sin6_family = AF_INET6;
+	sa.sin6_port = bpf_htons(0);
 
-		sa.sin6_addr.s6_addr32[0] = bpf_htonl(SRC_REWRITE_IP6_0);
-		sa.sin6_addr.s6_addr32[1] = bpf_htonl(SRC_REWRITE_IP6_1);
-		sa.sin6_addr.s6_addr32[2] = bpf_htonl(SRC_REWRITE_IP6_2);
-		sa.sin6_addr.s6_addr32[3] = bpf_htonl(SRC_REWRITE_IP6_3);
+	sa.sin6_addr.s6_addr32[0] = bpf_htonl(SRC_REWRITE_IP6_0);
+	sa.sin6_addr.s6_addr32[1] = bpf_htonl(SRC_REWRITE_IP6_1);
+	sa.sin6_addr.s6_addr32[2] = bpf_htonl(SRC_REWRITE_IP6_2);
+	sa.sin6_addr.s6_addr32[3] = bpf_htonl(SRC_REWRITE_IP6_3);
 
-		if (bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa)) != 0)
-			return 0;
-	}
+	if (bpf_bind(ctx, (struct sockaddr *)&sa, sizeof(sa)) != 0)
+		return 0;
 
 	return 1;
 }
-- 
2.17.1

Re: [PATCH bpf-next 2/4] bpf: Split bpf_sk_lookup

[Martin Lau]

On Thu, Nov 08, 2018 at 08:54:23AM -0800, Andrey Ignatov wrote:
> Split bpf_sk_lookup to separate core functionality, that can be reused
> to make socket lookup available to more program types, from
> functionality specific to program types that have access to skb.
> 
> Core functionality is placed to __bpf_sk_lookup. And bpf_sk_lookup only
> gets caller netns and ifindex from skb and passes it to __bpf_sk_lookup.
> 
> Program types that don't have access to skb can just pass NULL to
> __bpf_sk_lookup that will be handled correctly by both inet{,6}_sdif and
> lookup functions.
> 
> This is refactoring that simply moves blocks around and does NOT change
> existing logic.
> 
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  net/core/filter.c | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9a1327eb25fa..dc0f86a707b7 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4825,14 +4825,10 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
>  
>  #ifdef CONFIG_INET
>  static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> -			      struct sk_buff *skb, u8 family, u8 proto)
> +			      struct sk_buff *skb, u8 family, u8 proto, int dif)
>  {
>  	bool refcounted = false;
>  	struct sock *sk = NULL;
> -	int dif = 0;
> -
> -	if (skb->dev)
> -		dif = skb->dev->ifindex;
>  
>  	if (family == AF_INET) {
>  		__be32 src4 = tuple->ipv4.saddr;
> @@ -4875,16 +4871,16 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
>  	return sk;
>  }
>  
> -/* bpf_sk_lookup performs the core lookup for different types of sockets,
> +/* __bpf_sk_lookup performs the core lookup for different types of sockets,
>   * taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
>   * Returns the socket as an 'unsigned long' to simplify the casting in the
>   * callers to satisfy BPF_CALL declarations.
>   */
>  static unsigned long
> -bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> -	      u8 proto, u64 netns_id, u64 flags)
> +__bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> +		u8 proto, u64 netns_id, struct net *caller_net, int ifindex,
> +		u64 flags)
That looks a bit different from the one landed to bpf-next.
You may need to respin the set.

>  {
> -	struct net *caller_net;
>  	struct sock *sk = NULL;
>  	u8 family = AF_UNSPEC;
>  	struct net *net;
> @@ -4893,19 +4889,15 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
>  	if (unlikely(family == AF_UNSPEC || netns_id > U32_MAX || flags))
>  		goto out;
>  
> -	if (skb->dev)
> -		caller_net = dev_net(skb->dev);
> -	else
> -		caller_net = sock_net(skb->sk);
>  	if (netns_id) {
>  		net = get_net_ns_by_id(caller_net, netns_id);
>  		if (unlikely(!net))
>  			goto out;
> -		sk = sk_lookup(net, tuple, skb, family, proto);
> +		sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
>  		put_net(net);
>  	} else {
>  		net = caller_net;
> -		sk = sk_lookup(net, tuple, skb, family, proto);
> +		sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
>  	}
>  
>  	if (sk)
> @@ -4914,6 +4906,22 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
>  	return (unsigned long) sk;
>  }
>  
> +static unsigned long
> +bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> +	      u8 proto, u64 netns_id, u64 flags)
> +{
> +	struct net *caller_net = sock_net(skb->sk);
> +	int ifindex = 0;
> +
> +	if (skb->dev) {
> +		caller_net = dev_net(skb->dev);
> +		ifindex = skb->dev->ifindex;
> +	}
> +
> +	return __bpf_sk_lookup(skb, tuple, len, proto, netns_id, caller_net,
> +			       ifindex, flags);
> +}
> +
>  BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
>  	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
>  {
> -- 
> 2.17.1
> 

Re: [PATCH bpf-next 3/4] bpf: Support socket lookup in CGROUP_SOCK_ADDR progs

[Martin Lau]

On Thu, Nov 08, 2018 at 08:54:24AM -0800, Andrey Ignatov wrote:
> Make bpf_sk_lookup_tcp, bpf_sk_lookup_udp and bpf_sk_release helpers
> available in programs of type BPF_PROG_TYPE_CGROUP_SOCK_ADDR.
> 
> Such programs operate on sockets and have access to socket and struct
> sockaddr passed by user to system calls such as sys_bind, sys_connect,
> sys_sendmsg.
> 
> It's useful to be able to lookup other sockets from these programs.
> E.g. sys_connect may lookup IP:port endpoint and if there is a server
> socket bound to that endpoint ("server" can be defined by saddr & sport
> being zero), redirect client connection to it by rewriting IP:port in
> sockaddr passed to sys_connect.
> 
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  net/core/filter.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/net/core/filter.c b/net/core/filter.c
> index dc0f86a707b7..2e8575a34a1e 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4971,6 +4971,51 @@ static const struct bpf_func_proto bpf_sk_release_proto = {
>  	.ret_type	= RET_INTEGER,
>  	.arg1_type	= ARG_PTR_TO_SOCKET,
>  };
> +
> +static unsigned long
> +bpf_sock_addr_sk_lookup(struct sock *sk, struct bpf_sock_tuple *tuple, u32 len,
> +			u8 proto, u64 netns_id, u64 flags)
Nit. This func looks unnecessary. as good as directly calling __bpf_sk_lookup().

Others LGTM.

> +{
> +	return __bpf_sk_lookup(NULL, tuple, len, proto, netns_id, sock_net(sk),
> +			       0, flags);
> +}
> +
> +BPF_CALL_5(bpf_sock_addr_sk_lookup_tcp, struct bpf_sock_addr_kern *, ctx,
> +	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> +{
> +	return bpf_sock_addr_sk_lookup(ctx->sk, tuple, len, IPPROTO_TCP,
> +				       netns_id, flags);
> +}
> +
> +static const struct bpf_func_proto bpf_sock_addr_sk_lookup_tcp_proto = {
> +	.func		= bpf_sock_addr_sk_lookup_tcp,
> +	.gpl_only	= false,
> +	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
> +	.arg1_type	= ARG_PTR_TO_CTX,
> +	.arg2_type	= ARG_PTR_TO_MEM,
> +	.arg3_type	= ARG_CONST_SIZE,
> +	.arg4_type	= ARG_ANYTHING,
> +	.arg5_type	= ARG_ANYTHING,
> +};
> +
> +BPF_CALL_5(bpf_sock_addr_sk_lookup_udp, struct bpf_sock_addr_kern *, ctx,
> +	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> +{
> +	return bpf_sock_addr_sk_lookup(ctx->sk, tuple, len, IPPROTO_UDP,
> +				       netns_id, flags);
> +}
> +
> +static const struct bpf_func_proto bpf_sock_addr_sk_lookup_udp_proto = {
> +	.func		= bpf_sock_addr_sk_lookup_udp,
> +	.gpl_only	= false,
> +	.ret_type	= RET_PTR_TO_SOCKET_OR_NULL,
> +	.arg1_type	= ARG_PTR_TO_CTX,
> +	.arg2_type	= ARG_PTR_TO_MEM,
> +	.arg3_type	= ARG_CONST_SIZE,
> +	.arg4_type	= ARG_ANYTHING,
> +	.arg5_type	= ARG_ANYTHING,
> +};
> +
>  #endif /* CONFIG_INET */
>  
>  bool bpf_helper_changes_pkt_data(void *func)
> @@ -5077,6 +5122,14 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &bpf_get_socket_cookie_sock_addr_proto;
>  	case BPF_FUNC_get_local_storage:
>  		return &bpf_get_local_storage_proto;
> +#ifdef CONFIG_INET
> +	case BPF_FUNC_sk_lookup_tcp:
> +		return &bpf_sock_addr_sk_lookup_tcp_proto;
> +	case BPF_FUNC_sk_lookup_udp:
> +		return &bpf_sock_addr_sk_lookup_udp_proto;
> +	case BPF_FUNC_sk_release:
> +		return &bpf_sk_release_proto;
> +#endif /* CONFIG_INET */
>  	default:
>  		return bpf_base_func_proto(func_id);
>  	}
> -- 
> 2.17.1
> 

Re: [PATCH bpf-next 4/4] selftest/bpf: Use bpf_sk_lookup_{tcp,udp} in test_sock_addr

[Martin Lau]

On Thu, Nov 08, 2018 at 08:54:25AM -0800, Andrey Ignatov wrote:
> Use bpf_sk_lookup_tcp, bpf_sk_lookup_udp and bpf_sk_release helpers from
> test_sock_addr programs to make sure they're available and can lookup
> and release socket properly for IPv4/IPv4, TCP/UDP.
> 
> Reading from a few fields of returned struct bpf_sock is also tested.
> 
Acked-by: Martin KaFai Lau <kafai@fb.com>

Re: [PATCH bpf-next 2/4] bpf: Split bpf_sk_lookup

[Andrey Ignatov]

Martin Lau <kafai@fb.com> [Fri, 2018-11-09 09:19 -0800]:
> On Thu, Nov 08, 2018 at 08:54:23AM -0800, Andrey Ignatov wrote:
> > Split bpf_sk_lookup to separate core functionality, that can be reused
> > to make socket lookup available to more program types, from
> > functionality specific to program types that have access to skb.
> > 
> > Core functionality is placed to __bpf_sk_lookup. And bpf_sk_lookup only
> > gets caller netns and ifindex from skb and passes it to __bpf_sk_lookup.
> > 
> > Program types that don't have access to skb can just pass NULL to
> > __bpf_sk_lookup that will be handled correctly by both inet{,6}_sdif and
> > lookup functions.
> > 
> > This is refactoring that simply moves blocks around and does NOT change
> > existing logic.
> > 
> > Signed-off-by: Andrey Ignatov <rdna@fb.com>
> > Acked-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> >  net/core/filter.c | 38 +++++++++++++++++++++++---------------
> >  1 file changed, 23 insertions(+), 15 deletions(-)
> > 
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 9a1327eb25fa..dc0f86a707b7 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -4825,14 +4825,10 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
> >  
> >  #ifdef CONFIG_INET
> >  static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> > -			      struct sk_buff *skb, u8 family, u8 proto)
> > +			      struct sk_buff *skb, u8 family, u8 proto, int dif)
> >  {
> >  	bool refcounted = false;
> >  	struct sock *sk = NULL;
> > -	int dif = 0;
> > -
> > -	if (skb->dev)
> > -		dif = skb->dev->ifindex;
> >  
> >  	if (family == AF_INET) {
> >  		__be32 src4 = tuple->ipv4.saddr;
> > @@ -4875,16 +4871,16 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> >  	return sk;
> >  }
> >  
> > -/* bpf_sk_lookup performs the core lookup for different types of sockets,
> > +/* __bpf_sk_lookup performs the core lookup for different types of sockets,
> >   * taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
> >   * Returns the socket as an 'unsigned long' to simplify the casting in the
> >   * callers to satisfy BPF_CALL declarations.
> >   */
> >  static unsigned long
> > -bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> > -	      u8 proto, u64 netns_id, u64 flags)
> > +__bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> > +		u8 proto, u64 netns_id, struct net *caller_net, int ifindex,
> > +		u64 flags)
> That looks a bit different from the one landed to bpf-next.
> You may need to respin the set.

Since Nitin's version is landed now, I'll rebase on top of it and this
patch just won't be needed (initially I did it to unblock myself).

I'll also address the nit in patch 3 and send v2 with both changes.

Thanks Martin!

> >  {
> > -	struct net *caller_net;
> >  	struct sock *sk = NULL;
> >  	u8 family = AF_UNSPEC;
> >  	struct net *net;
> > @@ -4893,19 +4889,15 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> >  	if (unlikely(family == AF_UNSPEC || netns_id > U32_MAX || flags))
> >  		goto out;
> >  
> > -	if (skb->dev)
> > -		caller_net = dev_net(skb->dev);
> > -	else
> > -		caller_net = sock_net(skb->sk);
> >  	if (netns_id) {
> >  		net = get_net_ns_by_id(caller_net, netns_id);
> >  		if (unlikely(!net))
> >  			goto out;
> > -		sk = sk_lookup(net, tuple, skb, family, proto);
> > +		sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
> >  		put_net(net);
> >  	} else {
> >  		net = caller_net;
> > -		sk = sk_lookup(net, tuple, skb, family, proto);
> > +		sk = sk_lookup(net, tuple, skb, family, proto, ifindex);
> >  	}
> >  
> >  	if (sk)
> > @@ -4914,6 +4906,22 @@ bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> >  	return (unsigned long) sk;
> >  }
> >  
> > +static unsigned long
> > +bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> > +	      u8 proto, u64 netns_id, u64 flags)
> > +{
> > +	struct net *caller_net = sock_net(skb->sk);
> > +	int ifindex = 0;
> > +
> > +	if (skb->dev) {
> > +		caller_net = dev_net(skb->dev);
> > +		ifindex = skb->dev->ifindex;
> > +	}
> > +
> > +	return __bpf_sk_lookup(skb, tuple, len, proto, netns_id, caller_net,
> > +			       ifindex, flags);
> > +}
> > +
> >  BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
> >  	   struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> >  {
> > -- 
> > 2.17.1
> > 

-- 
Andrey Ignatov