Skip to contentSkip to conversation

Two Helper function about socket information

PATCH Superseded Chenbo Feng 5 participants 9 messages Feb 6, 2017

Subsystems (5)
Networking Network drivers BPF Sockets +1 more subsystems

v2 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Chenbo Feng

author
#0 lore raw

v2 cover Two Helper function about socket information

From: Chenbo Feng <[email protected]>

Introduce two eBpf helper function to get the socket cookie and socket uid for each packet. The helper function is useful when the *sk field inside sk_buff is not empty.

Chenbo Feng (2):
Add a helper function to get socket cookie in eBPF Add a eBPF helper function to retrieve socket uid

 include/linux/sock_diag.h |  1 +
 include/uapi/linux/bpf.h  | 16 +++++++++++++++-
 net/core/filter.c         | 32 ++++++++++++++++++++++++++++++++
 net/core/sock_diag.c      |  2 +-
 4 files changed, 49 insertions(+), 2 deletions(-)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH v2 1/2 Add a helper function to get socket cookie in eBPF

From: Chenbo Feng <[email protected]>

Retrieve the socket cookie generated by sock_gen_cookie() from a sk_buff with a known socket. Generates a new cookie if one was not yet set.If the socket pointer inside sk_buff is NULL, 0 is returned. The helper function coud be useful in monitoring per socket networking traffic statistics and provide a unique socket identifier per namespace.

Signed-off-by Chenbo Feng <[email protected]>
---
 include/linux/sock_diag.h |  1 +
 include/uapi/linux/bpf.h  |  9 ++++++++-
 net/core/filter.c         | 15 +++++++++++++++
 net/core/sock_diag.c      |  2 +-
 4 files changed, 25 insertions(+), 2 deletions(-)
include/linux/sock_diag.h +1 -0
index a0596ca0..a2f8109 100644
@@ -24,6 +24,7 @@ void sock_diag_unregister(const struct sock_diag_handler *h);
 void sock_diag_register_inet_compat(int (*fn)(struct sk_buff *skb, struct nlmsghdr *nlh));
 void sock_diag_unregister_inet_compat(int (*fn)(struct sk_buff *skb, struct nlmsghdr *nlh));
 
+u64 sock_gen_cookie(struct sock *sk);
 int sock_diag_check_cookie(struct sock *sk, const __u32 *cookie);
 void sock_diag_save_cookie(struct sock *sk, __u32 *cookie);
include/uapi/linux/bpf.h +8 -1
index e07fd5a..6923d21 100644
@@ -449,6 +449,12 @@ union bpf_attr {
  *     Return:
  *       > 0 length of the string including the trailing NUL on success
  *       < 0 error
+ *
+ * u64 bpf_bpf_get_socket_cookie(skb)
+ *     Get the cookie for the socket stored inside sk_buff.
+ *     @skb: pointer to skb
+ *     Return: 8 Bytes non-decreasing number on success or 0 if the socket
+ *     field is missing inside sk_buff
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\

@@ -496,7 +502,8 @@ union bpf_attr {
 	FN(get_numa_node_id),		\
 	FN(skb_change_head),		\
 	FN(xdp_adjust_head),		\
-	FN(probe_read_str),
+	FN(probe_read_str),		\
+	FN(get_socket_cookie),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
net/core/filter.c +15 -0
index 0b753cb..632fb91 100644
@@ -26,6 +26,7 @@
 #include <linux/mm.h>
 #include <linux/fcntl.h>
 #include <linux/socket.h>
+#include <linux/sock_diag.h>
 #include <linux/in.h>
 #include <linux/inet.h>
 #include <linux/netdevice.h>

@@ -2599,6 +2600,18 @@ static const struct bpf_func_proto bpf_xdp_event_output_proto = {
 	.arg5_type	= ARG_CONST_SIZE,
 };
 
+BPF_CALL_1(bpf_get_socket_cookie, struct sk_buff *, skb)
+{
+	return skb->sk ? sock_gen_cookie(skb->sk) : 0;
+}
+
+static const struct bpf_func_proto bpf_get_socket_cookie_proto = {
+	.func           = bpf_get_socket_cookie,
+	.gpl_only       = false,
+	.ret_type       = RET_INTEGER,
+	.arg1_type      = ARG_PTR_TO_CTX,
+};
+
 static const struct bpf_func_proto *
 bpf_base_func_proto(enum bpf_func_id func_id)
 {

@@ -2622,6 +2635,8 @@ bpf_base_func_proto(enum bpf_func_id func_id)
 	case BPF_FUNC_trace_printk:
 		if (capable(CAP_SYS_ADMIN))
 			return bpf_get_trace_printk_proto();
+	case BPF_FUNC_get_socket_cookie:
+		return &bpf_get_socket_cookie_proto;
 	default:
 		return NULL;
 	}
net/core/sock_diag.c +1 -1
index 6b10573..acd2a6c 100644
@@ -19,7 +19,7 @@ static int (*inet_rcv_compat)(struct sk_buff *skb, struct nlmsghdr *nlh);
 static DEFINE_MUTEX(sock_diag_table_mutex);
 static struct workqueue_struct *broadcast_wq;
 
-static u64 sock_gen_cookie(struct sock *sk)
+u64 sock_gen_cookie(struct sock *sk)
 {
 	while (1) {
 		u64 res = atomic64_read(&sk->sk_cookie);
Signature · 1 line
2.7.4
Acked-by on v2 1/2
#2 lore raw

Re: [PATCH net-next v2 1/2] Add a helper function to get socket cookie in eBPF

Quoted 10 lines · Chenbo Feng

From: Chenbo Feng <[email protected]>

Retrieve the socket cookie generated by sock_gen_cookie() from a sk_buff with a known socket. Generates a new cookie if one was not yet set.If the socket pointer inside sk_buff is NULL, 0 is returned. The helper function coud be useful in monitoring per socket networking traffic statistics and provide a unique socket identifier per namespace.

Signed-off-by Chenbo Feng <[email protected]>

---

Acked-by Eric Dumazet <[email protected]>
#3 lore raw

Re: [PATCH net-next v2 1/2] Add a helper function to get socket cookie in eBPF

In reply to Chenbo Feng

net/core/filter.c static const struct bpf_func_proto bpf_xdp_event_output_proto = { · line 2607

Quoted 5 lines · Chenbo Feng
net/core/filter.c +5 -0
+BPF_CALL_1(bpf_get_socket_cookie, struct sk_buff *, skb)
+{
+       return skb->sk ? sock_gen_cookie(skb->sk) : 0;
+}
+

Does this code need to increment the socket refcount, or call ACCESS_ONCE to get skb->sk? The socket filter codepath should be safe, but if this function is called in xt_ebpf, could it race with something that sets skb->sk to null?

#4 lore raw

Re: [PATCH net-next v2 1/2] Add a helper function to get socket cookie in eBPF

Quoted 12 lines · Lorenzo Colitti

net/core/filter.c static const struct bpf_func_proto bpf_xdp_event_output_proto = { · line 2607

Quoted 5 lines · Chenbo Feng
net/core/filter.c +5 -0
+BPF_CALL_1(bpf_get_socket_cookie, struct sk_buff *, skb)
+{
+       return skb->sk ? sock_gen_cookie(skb->sk) : 0;
+}
+

Does this code need to increment the socket refcount, or call ACCESS_ONCE to get skb->sk? The socket filter codepath should be safe, but if this function is called in xt_ebpf, could it race with something that sets skb->sk to null?

I do not see how this could possibly happen.

READ_ONCE() would not prevent the 'old' sk from disappearing anyway.

maintainer
#5 lore raw

Re: [PATCH net-next v2 1/2] Add a helper function to get socket cookie in eBPF

In reply to Chenbo Feng
Quoted 9 lines · Chenbo Feng

From: Chenbo Feng <[email protected]>

Retrieve the socket cookie generated by sock_gen_cookie() from a sk_buff with a known socket. Generates a new cookie if one was not yet set.If the socket pointer inside sk_buff is NULL, 0 is returned. The helper function coud be useful in monitoring per socket networking traffic statistics and provide a unique socket identifier per namespace.

Signed-off-by Chenbo Feng <[email protected]>

[...]

Quoted 32 lines
net/core/filter.c +13 -0
index 0b753cb..632fb91 100644
@@ -26,6 +26,7 @@
  #include <linux/mm.h>
  #include <linux/fcntl.h>
  #include <linux/socket.h>
+#include <linux/sock_diag.h>
  #include <linux/in.h>
  #include <linux/inet.h>
  #include <linux/netdevice.h>

@@ -2599,6 +2600,18 @@ static const struct bpf_func_proto bpf_xdp_event_output_proto = {
  	.arg5_type	= ARG_CONST_SIZE,
  };
 
+BPF_CALL_1(bpf_get_socket_cookie, struct sk_buff *, skb)
+{
+	return skb->sk ? sock_gen_cookie(skb->sk) : 0;
+}
+
+static const struct bpf_func_proto bpf_get_socket_cookie_proto = {
+	.func           = bpf_get_socket_cookie,
+	.gpl_only       = false,
+	.ret_type       = RET_INTEGER,
+	.arg1_type      = ARG_PTR_TO_CTX,
+};
+
  static const struct bpf_func_proto *
  bpf_base_func_proto(enum bpf_func_id func_id)
  {

net/core/filter.c bpf_base_func_proto(enum bpf_func_id func_id) · line 2642

Quoted 8 lines
net/core/filter.c +2 -0
@@ -2622,6 +2635,8 @@ bpf_base_func_proto(enum bpf_func_id func_id)
  	case BPF_FUNC_trace_printk:
  		if (capable(CAP_SYS_ADMIN))
  			return bpf_get_trace_printk_proto();
+	case BPF_FUNC_get_socket_cookie:
+		return &bpf_get_socket_cookie_proto;
  	default:
  		return NULL;
  	}

This still has one issue that would need to be addressed, otherwise looks good and ready to me.

Issue is that it cannot be added to bpf_base_func_proto(), because that is also used by cg_sock_ops, which has struct sock as input instead of struct sk_buff.

I suggest, we initially add this to both, sk_filter_func_proto() and tc_cls_act_func_proto() and, if needed, we could follow-up with other prog types at some later point in time. Otherwise, looks fine.

Quoted 14 lines
net/core/sock_diag.c +1 -1
index 6b10573..acd2a6c 100644
@@ -19,7 +19,7 @@ static int (*inet_rcv_compat)(struct sk_buff *skb, struct nlmsghdr *nlh);
  static DEFINE_MUTEX(sock_diag_table_mutex);
  static struct workqueue_struct *broadcast_wq;
 
-static u64 sock_gen_cookie(struct sock *sk)
+u64 sock_gen_cookie(struct sock *sk)
  {
  	while (1) {
  		u64 res = atomic64_read(&sk->sk_cookie);
author
#6 lore raw

PATCH v2 2/2 Add a eBPF helper function to retrieve socket uid

In reply to Chenbo Feng

From: Chenbo Feng <[email protected]>

Returns the owner uid of the socket inside a sk_buff. This is useful to perform per-UID accounting of network traffic or per-UID packet filtering.

Signed-off-by Chenbo Feng <[email protected]>
---
 include/uapi/linux/bpf.h |  9 ++++++++-
 net/core/filter.c        | 17 +++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)
include/uapi/linux/bpf.h +8 -1
index 6923d21..4854027 100644
@@ -455,6 +455,12 @@ union bpf_attr {
  *     @skb: pointer to skb
  *     Return: 8 Bytes non-decreasing number on success or 0 if the socket
  *     field is missing inside sk_buff
+ *
+ * u32 bpf_get_socket_uid(skb)
+ *     Get the owner uid of the socket stored inside sk_buff.
+ *     @skb: pointer to skb
+ *     Return: uid of the socket owner on success or 0 if the socket pointer
+ *     inside sk_buff is NULL
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\

@@ -503,7 +509,8 @@ union bpf_attr {
 	FN(skb_change_head),		\
 	FN(xdp_adjust_head),		\
 	FN(probe_read_str),		\
-	FN(get_socket_cookie),
+	FN(get_socket_cookie),		\
+	FN(get_socket_uid),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
net/core/filter.c +17 -0
index 632fb91..523ed08 100644
@@ -2612,6 +2612,21 @@ static const struct bpf_func_proto bpf_get_socket_cookie_proto = {
 	.arg1_type      = ARG_PTR_TO_CTX,
 };
 
+BPF_CALL_1(bpf_get_socket_uid, struct sk_buff *, skb)
+{
+	struct sock *sk = skb->sk;
+	kuid_t kuid = sock_net_uid(net, sk && sk_fullsock(sk) ?
+				   sk : NULL);
+	return (u32)kuid.val;
+}
+
+static const struct bpf_func_proto bpf_get_socket_uid_proto = {
+	.func           = bpf_get_socket_uid,
+	.gpl_only       = false,
+	.ret_type       = RET_INTEGER,
+	.arg1_type      = ARG_PTR_TO_CTX,
+};
+
 static const struct bpf_func_proto *
 bpf_base_func_proto(enum bpf_func_id func_id)
 {

@@ -2637,6 +2652,8 @@ bpf_base_func_proto(enum bpf_func_id func_id)
 			return bpf_get_trace_printk_proto();
 	case BPF_FUNC_get_socket_cookie:
 		return &bpf_get_socket_cookie_proto;
+	case BPF_FUNC_get_socket_uid:
+		return &bpf_get_socket_uid_proto;
 	default:
 		return NULL;
 	}
Signature · 1 line
2.7.4
build bot
#7 lore raw

Re: [PATCH net-next v2 2/2] Add a eBPF helper function to retrieve socket uid

url: https://github.com/0day-ci/linux/commits/Chenbo-Feng/Two-Helper-function-about-socket-information/20170206-102835 config: x86_64-randconfig-x014-201706 (attached as .config) compiler: gcc-6…

Hi Chenbo,

[auto build test ERROR on net-next/master]

url: https://github.com/0day-ci/linux/commits/Chenbo-Feng/Two-Helper-function-about-socket-information/20170206-102835
config: x86_64-randconfig-x014-201706 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

net/core/filter.c: In function '____bpf_get_socket_uid':

>> net/core/filter.c:2618:29: error: 'net' undeclared (first use in this function)
     kuid_t kuid = sock_net_uid(net, sk && sk_fullsock(sk) ?
                                ^~~

net/core/filter.c:2618:29: note: each undeclared identifier is reported only once for each function it appears in

vim +/net +2618 net/core/filter.c

2612 .arg1_type = ARG_PTR_TO_CTX,
2613 };
2614
2615 BPF_CALL_1(bpf_get_socket_uid, struct sk_buff *, skb)
2616 {
2617 struct sock *sk = skb->sk;

> 2618		kuid_t kuid = sock_net_uid(net, sk && sk_fullsock(sk) ?

2619 sk : NULL);
2620 return (u32)kuid.val;
2621 }

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

#8 lore raw

Re: [PATCH net-next v2 2/2] Add a eBPF helper function to retrieve socket uid

In reply to Chenbo Feng
Quoted 8 lines · Chenbo Feng

From: Chenbo Feng <[email protected]>

Returns the owner uid of the socket inside a sk_buff. This is useful to perform per-UID accounting of network traffic or per-UID packet filtering.

Signed-off-by Chenbo Feng <[email protected]>

---

net/core/filter.c static const struct bpf_func_proto bpf_get_socket_cookie_proto = { · line 2622

Quoted 8 lines
net/core/filter.c +8 -0
+BPF_CALL_1(bpf_get_socket_uid, struct sk_buff *, skb)
+{
+	struct sock *sk = skb->sk;
+	kuid_t kuid = sock_net_uid(net, sk && sk_fullsock(sk) ?
+				   sk : NULL);
+	return (u32)kuid.val;
+}
+

Have you considered to use sk_to_full_sk() ?

struct sock *sk = sk_to_full_sk(skb->sk);
kuid_t kuid = sock_net_uid(net, sk);