netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] bpf: add helper for getting xfrm states
@ 2018-04-17  4:48 Eyal Birger
  2018-04-17  4:48 ` [PATCH bpf-next 1/2] " Eyal Birger
  2018-04-17  4:48 ` [PATCH bpf-next 2/2] samples/bpf: extend test_tunnel_bpf.sh with xfrm state test Eyal Birger
  0 siblings, 2 replies; 5+ messages in thread
From: Eyal Birger @ 2018-04-17  4:48 UTC (permalink / raw)
  To: netdev; +Cc: shmulik, ast, daniel, fw, steffen.klassert, Eyal Birger

This patchset adds support for fetching XFRM state information from
an eBPF program called from TC.

The first patch introduces a helper for fetching an XFRM state from the
skb's secpath. The XFRM state is modeled using a new virtual struct which
contains the SPI, peer address, and reqid values of the state; This struct
can be extended in the future to provide additional state information.

The second patch adds a test example in test_tunnel_bpf.sh. The sample
validates the correct extraction of state information by the eBPF program.

---


Eyal Birger (2):
  bpf: add helper for getting xfrm states
  samples/bpf: extend test_tunnel_bpf.sh with xfrm state test

 include/uapi/linux/bpf.h                  | 25 ++++++++++-
 net/core/filter.c                         | 46 ++++++++++++++++++++
 samples/bpf/tcbpf2_kern.c                 | 15 +++++++
 samples/bpf/test_tunnel_bpf.sh            | 71 +++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h            | 25 ++++++++++-
 tools/testing/selftests/bpf/bpf_helpers.h |  3 ++
 6 files changed, 183 insertions(+), 2 deletions(-)

-- 
2.7.4

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

* [PATCH bpf-next 1/2] bpf: add helper for getting xfrm states
  2018-04-17  4:48 [PATCH bpf-next 0/2] bpf: add helper for getting xfrm states Eyal Birger
@ 2018-04-17  4:48 ` Eyal Birger
  2018-04-18 20:59   ` Daniel Borkmann
  2018-04-17  4:48 ` [PATCH bpf-next 2/2] samples/bpf: extend test_tunnel_bpf.sh with xfrm state test Eyal Birger
  1 sibling, 1 reply; 5+ messages in thread
From: Eyal Birger @ 2018-04-17  4:48 UTC (permalink / raw)
  To: netdev; +Cc: shmulik, ast, daniel, fw, steffen.klassert, Eyal Birger

This commit introduces a helper which allows fetching xfrm state
parameters by eBPF programs attached to TC.

Prototype:
bpf_skb_get_xfrm_state(skb, index, xfrm_state, size, flags)

skb: pointer to skb
index: the index in the skb xfrm_state secpath array
xfrm_state: pointer to 'struct bpf_xfrm_state'
size: size of 'struct bpf_xfrm_state'
flags: reserved for future extensions

The helper returns 0 on success. Non zero if no xfrm state at the index
is found - or non exists at all.

struct bpf_xfrm_state currently includes the SPI, peer IPv4/IPv6
address and the reqid; it can be further extended by adding elements to
its end - indicating the populated fields by the 'size' argument -
keeping backwards compatibility.

Typical usage:

struct bpf_xfrm_state x = {};
bpf_skb_get_xfrm_state(skb, 0, &x, sizeof(x), 0);
...

Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
---
 include/uapi/linux/bpf.h | 25 ++++++++++++++++++++++++-
 net/core/filter.c        | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c5ec897..132e172 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -755,6 +755,15 @@ union bpf_attr {
  *     @addr: pointer to struct sockaddr to bind socket to
  *     @addr_len: length of sockaddr structure
  *     Return: 0 on success or negative error code
+ *
+ * int bpf_skb_get_xfrm_state(skb, index, xfrm_state, size, flags)
+ *     retrieve XFRM state
+ *     @skb: pointer to skb
+ *     @index: index of the xfrm state in the secpath
+ *     @key: pointer to 'struct bpf_xfrm_state'
+ *     @size: size of 'struct bpf_xfrm_state'
+ *     @flags: room for future extensions
+ *     Return: 0 on success or negative error
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -821,7 +830,8 @@ union bpf_attr {
 	FN(msg_apply_bytes),		\
 	FN(msg_cork_bytes),		\
 	FN(msg_pull_data),		\
-	FN(bind),
+	FN(bind),			\
+	FN(skb_get_xfrm_state),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
@@ -927,6 +937,19 @@ struct bpf_tunnel_key {
 	__u32 tunnel_label;
 };
 
+/* user accessible mirror of in-kernel xfrm_state.
+ * new fields can only be added to the end of this structure
+ */
+struct bpf_xfrm_state {
+	__u32 reqid;
+	__u32 spi;
+	__u16 family;
+	union {
+		__u32 remote_ipv4;
+		__u32 remote_ipv6[4];
+	};
+};
+
 /* Generic BPF return codes which all BPF program types may support.
  * The values are binary compatible with their TC_ACT_* counter-part to
  * provide backwards compatibility with existing SCHED_CLS and SCHED_ACT
diff --git a/net/core/filter.c b/net/core/filter.c
index d31aff9..c06600a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -57,6 +57,7 @@
 #include <net/sock_reuseport.h>
 #include <net/busy_poll.h>
 #include <net/tcp.h>
+#include <net/xfrm.h>
 #include <linux/bpf_trace.h>
 
 /**
@@ -3703,6 +3704,49 @@ static const struct bpf_func_proto bpf_bind_proto = {
 	.arg3_type	= ARG_CONST_SIZE,
 };
 
+BPF_CALL_5(bpf_skb_get_xfrm_state, struct sk_buff *, skb, u32, index,
+	   struct bpf_xfrm_state *, to, u32, size, u64, flags)
+{
+#ifdef CONFIG_XFRM
+	const struct sec_path *sp = skb_sec_path(skb);
+	const struct xfrm_state *x;
+
+	if (!sp || index >= sp->len)
+		goto err_clear;
+
+	x = sp->xvec[index];
+
+	if (unlikely(size != sizeof(struct bpf_xfrm_state)))
+		goto err_clear;
+
+	to->reqid = x->props.reqid;
+	to->spi = be32_to_cpu(x->id.spi);
+	to->family = x->props.family;
+	if (to->family == AF_INET6) {
+		memcpy(to->remote_ipv6, x->props.saddr.a6,
+		       sizeof(to->remote_ipv6));
+	} else {
+		to->remote_ipv4 = be32_to_cpu(x->props.saddr.a4);
+	}
+
+	return 0;
+err_clear:
+#endif
+	memset(to, 0, size);
+	return -EINVAL;
+}
+
+static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = {
+	.func		= bpf_skb_get_xfrm_state,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_ANYTHING,
+	.arg3_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg4_type	= ARG_CONST_SIZE,
+	.arg5_type	= ARG_ANYTHING,
+};
+
 static const struct bpf_func_proto *
 bpf_base_func_proto(enum bpf_func_id func_id)
 {
@@ -3844,6 +3888,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_socket_cookie_proto;
 	case BPF_FUNC_get_socket_uid:
 		return &bpf_get_socket_uid_proto;
+	case BPF_FUNC_skb_get_xfrm_state:
+		return &bpf_skb_get_xfrm_state_proto;
 	default:
 		return bpf_base_func_proto(func_id);
 	}
-- 
2.7.4

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

* [PATCH bpf-next 2/2] samples/bpf: extend test_tunnel_bpf.sh with xfrm state test
  2018-04-17  4:48 [PATCH bpf-next 0/2] bpf: add helper for getting xfrm states Eyal Birger
  2018-04-17  4:48 ` [PATCH bpf-next 1/2] " Eyal Birger
@ 2018-04-17  4:48 ` Eyal Birger
  1 sibling, 0 replies; 5+ messages in thread
From: Eyal Birger @ 2018-04-17  4:48 UTC (permalink / raw)
  To: netdev; +Cc: shmulik, ast, daniel, fw, steffen.klassert, Eyal Birger

Add a test for fetching xfrm state parameters from a tc program running
on ingress.

Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
---
 samples/bpf/tcbpf2_kern.c                 | 15 +++++++
 samples/bpf/test_tunnel_bpf.sh            | 71 +++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h            | 25 ++++++++++-
 tools/testing/selftests/bpf/bpf_helpers.h |  3 ++
 4 files changed, 113 insertions(+), 1 deletion(-)

diff --git a/samples/bpf/tcbpf2_kern.c b/samples/bpf/tcbpf2_kern.c
index 9a8db7bd..3303803 100644
--- a/samples/bpf/tcbpf2_kern.c
+++ b/samples/bpf/tcbpf2_kern.c
@@ -593,4 +593,19 @@ int _ip6ip6_get_tunnel(struct __sk_buff *skb)
 	return TC_ACT_OK;
 }
 
+SEC("xfrm_get_state")
+int _xfrm_get_state(struct __sk_buff *skb)
+{
+	struct bpf_xfrm_state x;
+	char fmt[] = "reqid %d spi 0x%x remote ip 0x%x\n";
+	int ret;
+
+	ret = bpf_skb_get_xfrm_state(skb, 0, &x, sizeof(x), 0);
+	if (ret < 0)
+		return TC_ACT_OK;
+
+	bpf_trace_printk(fmt, sizeof(fmt), x.reqid, x.spi, x.remote_ipv4);
+	return TC_ACT_OK;
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/test_tunnel_bpf.sh b/samples/bpf/test_tunnel_bpf.sh
index c265863..9c534dc 100755
--- a/samples/bpf/test_tunnel_bpf.sh
+++ b/samples/bpf/test_tunnel_bpf.sh
@@ -155,6 +155,57 @@ function add_ipip_tunnel {
 	ip addr add dev $DEV 10.1.1.200/24
 }
 
+function setup_xfrm_tunnel {
+	auth=0x$(printf '1%.0s' {1..40})
+	enc=0x$(printf '2%.0s' {1..32})
+	spi_in_to_out=0x1
+	spi_out_to_in=0x2
+	# in namespace
+	# in -> out
+	ip netns exec at_ns0 \
+		ip xfrm state add src 172.16.1.100 dst 172.16.1.200 proto esp \
+			spi $spi_in_to_out reqid 1 mode tunnel \
+			auth-trunc 'hmac(sha1)' $auth 96 enc 'cbc(aes)' $enc
+	ip netns exec at_ns0 \
+		ip xfrm policy add src 10.1.1.100/32 dst 10.1.1.200/32 dir out \
+		tmpl src 172.16.1.100 dst 172.16.1.200 proto esp reqid 1 \
+		mode tunnel
+	# out -> in
+	ip netns exec at_ns0 \
+		ip xfrm state add src 172.16.1.200 dst 172.16.1.100 proto esp \
+			spi $spi_out_to_in reqid 2 mode tunnel \
+			auth-trunc 'hmac(sha1)' $auth 96 enc 'cbc(aes)' $enc
+	ip netns exec at_ns0 \
+		ip xfrm policy add src 10.1.1.200/32 dst 10.1.1.100/32 dir in \
+		tmpl src 172.16.1.200 dst 172.16.1.100 proto esp reqid 2 \
+		mode tunnel
+	# address & route
+	ip netns exec at_ns0 \
+		ip addr add dev veth0 10.1.1.100/32
+	ip netns exec at_ns0 \
+		ip route add 10.1.1.200 dev veth0 via 172.16.1.200 \
+			src 10.1.1.100
+
+	# out of namespace
+	# in -> out
+	ip xfrm state add src 172.16.1.100 dst 172.16.1.200 proto esp \
+		spi $spi_in_to_out reqid 1 mode tunnel \
+		auth-trunc 'hmac(sha1)' $auth 96  enc 'cbc(aes)' $enc
+	ip xfrm policy add src 10.1.1.100/32 dst 10.1.1.200/32 dir in \
+		tmpl src 172.16.1.100 dst 172.16.1.200 proto esp reqid 1 \
+		mode tunnel
+	# out -> in
+	ip xfrm state add src 172.16.1.200 dst 172.16.1.100 proto esp \
+		spi $spi_out_to_in reqid 2 mode tunnel \
+		auth-trunc 'hmac(sha1)' $auth 96  enc 'cbc(aes)' $enc
+	ip xfrm policy add src 10.1.1.200/32 dst 10.1.1.100/32 dir out \
+		tmpl src 172.16.1.200 dst 172.16.1.100 proto esp reqid 2 \
+		mode tunnel
+	# address & route
+	ip addr add dev veth1 10.1.1.200/32
+	ip route add 10.1.1.100 dev veth1 via 172.16.1.100 src 10.1.1.200
+}
+
 function attach_bpf {
 	DEV=$1
 	SET_TUNNEL=$2
@@ -278,6 +329,22 @@ function test_ipip {
 	cleanup
 }
 
+function test_xfrm_tunnel {
+	config_device
+        tcpdump -nei veth1 ip &
+	output=$(mktemp)
+	cat /sys/kernel/debug/tracing/trace_pipe | tee $output &
+        setup_xfrm_tunnel
+	tc qdisc add dev veth1 clsact
+	tc filter add dev veth1 proto ip ingress bpf da obj tcbpf2_kern.o \
+		sec xfrm_get_state
+	ip netns exec at_ns0 ping -c 1 10.1.1.200
+	grep "reqid 1" $output
+	grep "spi 0x1" $output
+	grep "remote ip 0xac100164" $output
+	cleanup
+}
+
 function cleanup {
 	set +ex
 	pkill iperf
@@ -291,6 +358,8 @@ function cleanup {
 	ip link del geneve11
 	ip link del erspan11
 	ip link del ip6erspan11
+	ip x s flush
+	ip x p flush
 	pkill tcpdump
 	pkill cat
 	set -ex
@@ -316,4 +385,6 @@ echo "Testing GENEVE tunnel..."
 test_geneve
 echo "Testing IPIP tunnel..."
 test_ipip
+echo "Testing IPSec tunnel..."
+test_xfrm_tunnel
 echo "*** PASS ***"
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 9d07465..233ae6e 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -755,6 +755,15 @@ union bpf_attr {
  *     @addr: pointer to struct sockaddr to bind socket to
  *     @addr_len: length of sockaddr structure
  *     Return: 0 on success or negative error code
+ *
+ * int bpf_skb_get_xfrm_state(skb, index, xfrm_state, size, flags)
+ *     retrieve XFRM state
+ *     @skb: pointer to skb
+ *     @index: index of the xfrm state in the secpath
+ *     @key: pointer to 'struct bpf_xfrm_state'
+ *     @size: size of 'struct bpf_xfrm_state'
+ *     @flags: room for future extensions
+ *     Return: 0 on success or negative error
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -821,7 +830,8 @@ union bpf_attr {
 	FN(msg_apply_bytes),		\
 	FN(msg_cork_bytes),		\
 	FN(msg_pull_data),		\
-	FN(bind),
+	FN(bind),			\
+	FN(skb_get_xfrm_state),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
@@ -926,6 +936,19 @@ struct bpf_tunnel_key {
 	__u32 tunnel_label;
 };
 
+/* user accessible mirror of in-kernel xfrm_state.
+ * new fields can only be added to the end of this structure
+ */
+struct bpf_xfrm_state {
+	__u32 reqid;
+	__u32 spi;
+	__u16 family;
+	union {
+		__u32 remote_ipv4;
+		__u32 remote_ipv6[4];
+	};
+};
+
 /* Generic BPF return codes which all BPF program types may support.
  * The values are binary compatible with their TC_ACT_* counter-part to
  * provide backwards compatibility with existing SCHED_CLS and SCHED_ACT
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index d8223d9..bf46b58 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -96,6 +96,9 @@ static int (*bpf_msg_pull_data)(void *ctx, int start, int end, int flags) =
 	(void *) BPF_FUNC_msg_pull_data;
 static int (*bpf_bind)(void *ctx, void *addr, int addr_len) =
 	(void *) BPF_FUNC_bind;
+static int (*bpf_skb_get_xfrm_state)(void *ctx, int index, void *state,
+				     int size, int flags) =
+	(void *) BPF_FUNC_skb_get_xfrm_state;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
-- 
2.7.4

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

* Re: [PATCH bpf-next 1/2] bpf: add helper for getting xfrm states
  2018-04-17  4:48 ` [PATCH bpf-next 1/2] " Eyal Birger
@ 2018-04-18 20:59   ` Daniel Borkmann
  2018-04-18 22:02     ` Eyal Birger
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2018-04-18 20:59 UTC (permalink / raw)
  To: Eyal Birger, netdev; +Cc: shmulik, ast, fw, steffen.klassert

On 04/17/2018 06:48 AM, Eyal Birger wrote:
> This commit introduces a helper which allows fetching xfrm state
> parameters by eBPF programs attached to TC.
> 
> Prototype:
> bpf_skb_get_xfrm_state(skb, index, xfrm_state, size, flags)
> 
> skb: pointer to skb
> index: the index in the skb xfrm_state secpath array
> xfrm_state: pointer to 'struct bpf_xfrm_state'
> size: size of 'struct bpf_xfrm_state'
> flags: reserved for future extensions
> 
> The helper returns 0 on success. Non zero if no xfrm state at the index
> is found - or non exists at all.
> 
> struct bpf_xfrm_state currently includes the SPI, peer IPv4/IPv6
> address and the reqid; it can be further extended by adding elements to
> its end - indicating the populated fields by the 'size' argument -
> keeping backwards compatibility.
> 
> Typical usage:
> 
> struct bpf_xfrm_state x = {};
> bpf_skb_get_xfrm_state(skb, 0, &x, sizeof(x), 0);
> ...
> 
> Signed-off-by: Eyal Birger <eyal.birger@gmail.com>

Patch looks good to me, two comments below:

> ---
>  include/uapi/linux/bpf.h | 25 ++++++++++++++++++++++++-
>  net/core/filter.c        | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index c5ec897..132e172 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -755,6 +755,15 @@ union bpf_attr {
>   *     @addr: pointer to struct sockaddr to bind socket to
>   *     @addr_len: length of sockaddr structure
>   *     Return: 0 on success or negative error code
> + *
> + * int bpf_skb_get_xfrm_state(skb, index, xfrm_state, size, flags)
> + *     retrieve XFRM state
> + *     @skb: pointer to skb
> + *     @index: index of the xfrm state in the secpath
> + *     @key: pointer to 'struct bpf_xfrm_state'
> + *     @size: size of 'struct bpf_xfrm_state'
> + *     @flags: room for future extensions
> + *     Return: 0 on success or negative error
>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -821,7 +830,8 @@ union bpf_attr {
>  	FN(msg_apply_bytes),		\
>  	FN(msg_cork_bytes),		\
>  	FN(msg_pull_data),		\
> -	FN(bind),
> +	FN(bind),			\
> +	FN(skb_get_xfrm_state),
>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> @@ -927,6 +937,19 @@ struct bpf_tunnel_key {
>  	__u32 tunnel_label;
>  };
>  
> +/* user accessible mirror of in-kernel xfrm_state.
> + * new fields can only be added to the end of this structure
> + */
> +struct bpf_xfrm_state {
> +	__u32 reqid;
> +	__u32 spi;
> +	__u16 family;
> +	union {
> +		__u32 remote_ipv4;
> +		__u32 remote_ipv6[4];
> +	};
> +};
> +
>  /* Generic BPF return codes which all BPF program types may support.
>   * The values are binary compatible with their TC_ACT_* counter-part to
>   * provide backwards compatibility with existing SCHED_CLS and SCHED_ACT
> diff --git a/net/core/filter.c b/net/core/filter.c
> index d31aff9..c06600a 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -57,6 +57,7 @@
>  #include <net/sock_reuseport.h>
>  #include <net/busy_poll.h>
>  #include <net/tcp.h>
> +#include <net/xfrm.h>
>  #include <linux/bpf_trace.h>
>  
>  /**
> @@ -3703,6 +3704,49 @@ static const struct bpf_func_proto bpf_bind_proto = {
>  	.arg3_type	= ARG_CONST_SIZE,
>  };
>  
> +BPF_CALL_5(bpf_skb_get_xfrm_state, struct sk_buff *, skb, u32, index,
> +	   struct bpf_xfrm_state *, to, u32, size, u64, flags)
> +{
> +#ifdef CONFIG_XFRM
> +	const struct sec_path *sp = skb_sec_path(skb);
> +	const struct xfrm_state *x;
> +
> +	if (!sp || index >= sp->len)

This should be something like: if (!sp || unlikely(index >= sp->len || flags))
Such that we unconditionally bail out on any flags currently, since this is
reserved for future use and anything non-zero would be invalid and rejected
until we start extending it.

> +		goto err_clear;
> +
> +	x = sp->xvec[index];
> +
> +	if (unlikely(size != sizeof(struct bpf_xfrm_state)))
> +		goto err_clear;
> +
> +	to->reqid = x->props.reqid;
> +	to->spi = be32_to_cpu(x->id.spi);
> +	to->family = x->props.family;
> +	if (to->family == AF_INET6) {
> +		memcpy(to->remote_ipv6, x->props.saddr.a6,
> +		       sizeof(to->remote_ipv6));
> +	} else {
> +		to->remote_ipv4 = be32_to_cpu(x->props.saddr.a4);
> +	}
> +
> +	return 0;
> +err_clear:
> +#endif
> +	memset(to, 0, size);
> +	return -EINVAL;
> +}
> +
> +static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = {
> +	.func		= bpf_skb_get_xfrm_state,
> +	.gpl_only	= false,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_PTR_TO_CTX,
> +	.arg2_type	= ARG_ANYTHING,
> +	.arg3_type	= ARG_PTR_TO_UNINIT_MEM,
> +	.arg4_type	= ARG_CONST_SIZE,
> +	.arg5_type	= ARG_ANYTHING,
> +};
> +
>  static const struct bpf_func_proto *
>  bpf_base_func_proto(enum bpf_func_id func_id)
>  {
> @@ -3844,6 +3888,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &bpf_get_socket_cookie_proto;
>  	case BPF_FUNC_get_socket_uid:
>  		return &bpf_get_socket_uid_proto;
> +	case BPF_FUNC_skb_get_xfrm_state:
> +		return &bpf_skb_get_xfrm_state_proto;

Potentially, on kernels with !CONFIG_XFRM, you might want to let the program
bail out at program verification phase already? Thus it would become ...

#ifdef CONFIG_XFRM
	case BPF_FUNC_skb_get_xfrm_state:
		return &bpf_skb_get_xfrm_state_proto;
#endif

... where you'd also wrap the helper + state_proto in CONFIG_XFRM.

>  	default:
>  		return bpf_base_func_proto(func_id);
>  	}
> 

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

* Re: [PATCH bpf-next 1/2] bpf: add helper for getting xfrm states
  2018-04-18 20:59   ` Daniel Borkmann
@ 2018-04-18 22:02     ` Eyal Birger
  0 siblings, 0 replies; 5+ messages in thread
From: Eyal Birger @ 2018-04-18 22:02 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: netdev

On Wed, 18 Apr 2018 22:59:27 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:

> On 04/17/2018 06:48 AM, Eyal Birger wrote:
> > This commit introduces a helper which allows fetching xfrm state
> > parameters by eBPF programs attached to TC.
> > 
> > Prototype:
> > bpf_skb_get_xfrm_state(skb, index, xfrm_state, size, flags)
> > 
> > skb: pointer to skb
> > index: the index in the skb xfrm_state secpath array
> > xfrm_state: pointer to 'struct bpf_xfrm_state'
> > size: size of 'struct bpf_xfrm_state'
> > flags: reserved for future extensions
> > 
> > The helper returns 0 on success. Non zero if no xfrm state at the
> > index is found - or non exists at all.
> > 
> > struct bpf_xfrm_state currently includes the SPI, peer IPv4/IPv6
> > address and the reqid; it can be further extended by adding
> > elements to its end - indicating the populated fields by the 'size'
> > argument - keeping backwards compatibility.
> > 
> > Typical usage:
> > 
> > struct bpf_xfrm_state x = {};
> > bpf_skb_get_xfrm_state(skb, 0, &x, sizeof(x), 0);
> > ...
> > 
> > Signed-off-by: Eyal Birger <eyal.birger@gmail.com>  
> 
> Patch looks good to me, two comments below:

Thanks! I incorporated your suggestions in v2.
Eyal.

> 
> > ---
> >  include/uapi/linux/bpf.h | 25 ++++++++++++++++++++++++-
> >  net/core/filter.c        | 46
> > ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70
> > insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index c5ec897..132e172 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -755,6 +755,15 @@ union bpf_attr {
> >   *     @addr: pointer to struct sockaddr to bind socket to
> >   *     @addr_len: length of sockaddr structure
> >   *     Return: 0 on success or negative error code
> > + *
> > + * int bpf_skb_get_xfrm_state(skb, index, xfrm_state, size, flags)
> > + *     retrieve XFRM state
> > + *     @skb: pointer to skb
> > + *     @index: index of the xfrm state in the secpath
> > + *     @key: pointer to 'struct bpf_xfrm_state'
> > + *     @size: size of 'struct bpf_xfrm_state'
> > + *     @flags: room for future extensions
> > + *     Return: 0 on success or negative error
> >   */
> >  #define __BPF_FUNC_MAPPER(FN)		\
> >  	FN(unspec),			\
> > @@ -821,7 +830,8 @@ union bpf_attr {
> >  	FN(msg_apply_bytes),		\
> >  	FN(msg_cork_bytes),		\
> >  	FN(msg_pull_data),		\
> > -	FN(bind),
> > +	FN(bind),			\
> > +	FN(skb_get_xfrm_state),
> >  
> >  /* integer value in 'imm' field of BPF_CALL instruction selects
> > which helper
> >   * function eBPF program intends to call
> > @@ -927,6 +937,19 @@ struct bpf_tunnel_key {
> >  	__u32 tunnel_label;
> >  };
> >  
> > +/* user accessible mirror of in-kernel xfrm_state.
> > + * new fields can only be added to the end of this structure
> > + */
> > +struct bpf_xfrm_state {
> > +	__u32 reqid;
> > +	__u32 spi;
> > +	__u16 family;
> > +	union {
> > +		__u32 remote_ipv4;
> > +		__u32 remote_ipv6[4];
> > +	};
> > +};
> > +
> >  /* Generic BPF return codes which all BPF program types may
> > support.
> >   * The values are binary compatible with their TC_ACT_*
> > counter-part to
> >   * provide backwards compatibility with existing SCHED_CLS and
> > SCHED_ACT diff --git a/net/core/filter.c b/net/core/filter.c
> > index d31aff9..c06600a 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -57,6 +57,7 @@
> >  #include <net/sock_reuseport.h>
> >  #include <net/busy_poll.h>
> >  #include <net/tcp.h>
> > +#include <net/xfrm.h>
> >  #include <linux/bpf_trace.h>
> >  
> >  /**
> > @@ -3703,6 +3704,49 @@ static const struct bpf_func_proto
> > bpf_bind_proto = { .arg3_type	= ARG_CONST_SIZE,
> >  };
> >  
> > +BPF_CALL_5(bpf_skb_get_xfrm_state, struct sk_buff *, skb, u32,
> > index,
> > +	   struct bpf_xfrm_state *, to, u32, size, u64, flags)
> > +{
> > +#ifdef CONFIG_XFRM
> > +	const struct sec_path *sp = skb_sec_path(skb);
> > +	const struct xfrm_state *x;
> > +
> > +	if (!sp || index >= sp->len)  
> 
> This should be something like: if (!sp || unlikely(index >= sp->len
> || flags)) Such that we unconditionally bail out on any flags
> currently, since this is reserved for future use and anything
> non-zero would be invalid and rejected until we start extending it.
> 
> > +		goto err_clear;
> > +
> > +	x = sp->xvec[index];
> > +
> > +	if (unlikely(size != sizeof(struct bpf_xfrm_state)))
> > +		goto err_clear;
> > +
> > +	to->reqid = x->props.reqid;
> > +	to->spi = be32_to_cpu(x->id.spi);
> > +	to->family = x->props.family;
> > +	if (to->family == AF_INET6) {
> > +		memcpy(to->remote_ipv6, x->props.saddr.a6,
> > +		       sizeof(to->remote_ipv6));
> > +	} else {
> > +		to->remote_ipv4 = be32_to_cpu(x->props.saddr.a4);
> > +	}
> > +
> > +	return 0;
> > +err_clear:
> > +#endif
> > +	memset(to, 0, size);
> > +	return -EINVAL;
> > +}
> > +
> > +static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = {
> > +	.func		= bpf_skb_get_xfrm_state,
> > +	.gpl_only	= false,
> > +	.ret_type	= RET_INTEGER,
> > +	.arg1_type	= ARG_PTR_TO_CTX,
> > +	.arg2_type	= ARG_ANYTHING,
> > +	.arg3_type	= ARG_PTR_TO_UNINIT_MEM,
> > +	.arg4_type	= ARG_CONST_SIZE,
> > +	.arg5_type	= ARG_ANYTHING,
> > +};
> > +
> >  static const struct bpf_func_proto *
> >  bpf_base_func_proto(enum bpf_func_id func_id)
> >  {
> > @@ -3844,6 +3888,8 @@ tc_cls_act_func_proto(enum bpf_func_id
> > func_id, const struct bpf_prog *prog) return
> > &bpf_get_socket_cookie_proto; case BPF_FUNC_get_socket_uid:
> >  		return &bpf_get_socket_uid_proto;
> > +	case BPF_FUNC_skb_get_xfrm_state:
> > +		return &bpf_skb_get_xfrm_state_proto;  
> 
> Potentially, on kernels with !CONFIG_XFRM, you might want to let the
> program bail out at program verification phase already? Thus it would
> become ...
> 
> #ifdef CONFIG_XFRM
> 	case BPF_FUNC_skb_get_xfrm_state:
> 		return &bpf_skb_get_xfrm_state_proto;
> #endif
> 
> ... where you'd also wrap the helper + state_proto in CONFIG_XFRM.
> 
> >  	default:
> >  		return bpf_base_func_proto(func_id);
> >  	}
> >   
> 

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

end of thread, other threads:[~2018-04-18 22:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-17  4:48 [PATCH bpf-next 0/2] bpf: add helper for getting xfrm states Eyal Birger
2018-04-17  4:48 ` [PATCH bpf-next 1/2] " Eyal Birger
2018-04-18 20:59   ` Daniel Borkmann
2018-04-18 22:02     ` Eyal Birger
2018-04-17  4:48 ` [PATCH bpf-next 2/2] samples/bpf: extend test_tunnel_bpf.sh with xfrm state test Eyal Birger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).