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

[Daniel Borkmann]

On 02/06/2017 03:17 AM, Chenbo Feng wrote:

From: Chenbo Feng <fe...@google.com>

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 <chenbofeng.ker...@gmail.com>

[...]

diff --git a/net/core/filter.c b/net/core/filter.c
index 0b753cb..632fb91 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -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;
        }

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.

diff --git a/net/core/sock_diag.c b/net/core/sock_diag.c
index 6b10573..acd2a6c 100644
--- a/net/core/sock_diag.c
+++ b/net/core/sock_diag.c
@@ -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);