diff mbox series

[bpf-next,v6,04/11] bpf: Support passing args to sock_ops bpf function

Message ID 20180120014548.2941040-5-brakmo@fb.com
State Changes Requested, archived
Delegated to: BPF Maintainers
Headers show
Series bpf: More sock_ops callbacks | expand

Commit Message

Lawrence Brakmo Jan. 20, 2018, 1:45 a.m. UTC 
Adds support for passing up to 4 arguments to sock_ops bpf functions. It
reusues the reply union, so the bpf_sock_ops structures are not
increased in size.

Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
 include/linux/filter.h   |  1 +
 include/net/tcp.h        | 64 ++++++++++++++++++++++++++++++++++++++++++++----
 include/uapi/linux/bpf.h |  5 ++--
 net/ipv4/tcp.c           |  2 +-
 net/ipv4/tcp_nv.c        |  2 +-
 net/ipv4/tcp_output.c    |  2 +-
 6 files changed, 66 insertions(+), 10 deletions(-)

Comments

Daniel Borkmann Jan. 24, 2018, 1:11 a.m. UTC | #1 
On 01/20/2018 02:45 AM, Lawrence Brakmo wrote:
> Adds support for passing up to 4 arguments to sock_ops bpf functions. It
> reusues the reply union, so the bpf_sock_ops structures are not
> increased in size.
> 
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> ---
>  include/linux/filter.h   |  1 +
>  include/net/tcp.h        | 64 ++++++++++++++++++++++++++++++++++++++++++++----
>  include/uapi/linux/bpf.h |  5 ++--
>  net/ipv4/tcp.c           |  2 +-
>  net/ipv4/tcp_nv.c        |  2 +-
>  net/ipv4/tcp_output.c    |  2 +-
>  6 files changed, 66 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index daa5a67..20384c4 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -1003,6 +1003,7 @@ struct bpf_sock_ops_kern {
>  	struct	sock *sk;
>  	u32	op;
>  	union {
> +		u32 args[4];
>  		u32 reply;
>  		u32 replylong[4];
>  	};
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 108d16a..8e9111f 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2005,7 +2005,7 @@ void tcp_cleanup_ulp(struct sock *sk);
>   * program loaded).
>   */
>  #ifdef CONFIG_BPF
> -static inline int tcp_call_bpf(struct sock *sk, int op)
> +static inline int tcp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
>  {
>  	struct bpf_sock_ops_kern sock_ops;
>  	int ret;
> @@ -2018,6 +2018,8 @@ static inline int tcp_call_bpf(struct sock *sk, int op)
>  
>  	sock_ops.sk = sk;
>  	sock_ops.op = op;
> +	if (nargs > 0)
> +		memcpy(sock_ops.args, args, nargs*sizeof(u32));

Small nit given respin: nargs * sizeof(*args)

>  	ret = BPF_CGROUP_RUN_PROG_SOCK_OPS(&sock_ops);
>  	if (ret == 0)
> @@ -2026,18 +2028,70 @@ static inline int tcp_call_bpf(struct sock *sk, int op)
>  		ret = -1;
>  	return ret;
>  }
> +
> +static inline int tcp_call_bpf_1arg(struct sock *sk, int op, u32 arg)
> +{
> +	return tcp_call_bpf(sk, op, 1, &arg);
> +}
> +
> +static inline int tcp_call_bpf_2arg(struct sock *sk, int op, u32 arg1, u32 arg2)
> +{
> +	u32 args[2] = {arg1, arg2};
> +
> +	return tcp_call_bpf(sk, op, 2, args);
> +}
> +
> +static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,
> +				    u32 arg3)
> +{
> +	u32 args[3] = {arg1, arg2, arg3};
> +
> +	return tcp_call_bpf(sk, op, 3, args);
> +}
> +
> +static inline int tcp_call_bpf_4arg(struct sock *sk, int op, u32 arg1, u32 arg2,
> +				    u32 arg3, u32 arg4)
> +{
> +	u32 args[4] = {arg1, arg2, arg3, arg4};
> +
> +	return tcp_call_bpf(sk, op, 4, args);
> +}
> +
>  #else
> -static inline int tcp_call_bpf(struct sock *sk, int op)
> +static inline int tcp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
>  {
>  	return -EPERM;
>  }
> +
> +static inline int tcp_call_bpf_1arg(struct sock *sk, int op, u32 arg)
> +{
> +	return -EPERM;
> +}
> +
> +static inline int tcp_call_bpf_2arg(struct sock *sk, int op, u32 arg1, u32 arg2)
> +{
> +	return -EPERM;
> +}
> +
> +static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,
> +	u32 arg3)

indent: arg3

> +{
> +	return -EPERM;
> +}
> +
> +static inline int tcp_call_bpf_4arg(struct sock *sk, int op, u32 arg1, u32 arg2,
> +				    u32 arg3, u32 arg4)
> +{
> +	return -EPERM;
> +}
> +
>  #endif

tcp_call_bpf_1arg() and tcp_call_bpf_4arg() unused for the time being?
Lawrence Brakmo Jan. 24, 2018, 1:30 a.m. UTC | #2 
On 1/23/18, 5:11 PM, "Daniel Borkmann" <daniel@iogearbox.net> wrote:

    On 01/20/2018 02:45 AM, Lawrence Brakmo wrote:
    > Adds support for passing up to 4 arguments to sock_ops bpf functions. It

    > reusues the reply union, so the bpf_sock_ops structures are not

    > increased in size.

    > 

    > Signed-off-by: Lawrence Brakmo <brakmo@fb.com>

    > ---

    >  include/linux/filter.h   |  1 +

    >  include/net/tcp.h        | 64 ++++++++++++++++++++++++++++++++++++++++++++----

    >  include/uapi/linux/bpf.h |  5 ++--

    >  net/ipv4/tcp.c           |  2 +-

    >  net/ipv4/tcp_nv.c        |  2 +-

    >  net/ipv4/tcp_output.c    |  2 +-

    >  6 files changed, 66 insertions(+), 10 deletions(-)

    > 

    > diff --git a/include/linux/filter.h b/include/linux/filter.h

    > index daa5a67..20384c4 100644

    > --- a/include/linux/filter.h

    > +++ b/include/linux/filter.h

    > @@ -1003,6 +1003,7 @@ struct bpf_sock_ops_kern {

    >  	struct	sock *sk;

    >  	u32	op;

    >  	union {

    > +		u32 args[4];

    >  		u32 reply;

    >  		u32 replylong[4];

    >  	};

    > diff --git a/include/net/tcp.h b/include/net/tcp.h

    > index 108d16a..8e9111f 100644

    > --- a/include/net/tcp.h

    > +++ b/include/net/tcp.h

    > @@ -2005,7 +2005,7 @@ void tcp_cleanup_ulp(struct sock *sk);

    >   * program loaded).

    >   */

    >  #ifdef CONFIG_BPF

    > -static inline int tcp_call_bpf(struct sock *sk, int op)

    > +static inline int tcp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)

    >  {

    >  	struct bpf_sock_ops_kern sock_ops;

    >  	int ret;

    > @@ -2018,6 +2018,8 @@ static inline int tcp_call_bpf(struct sock *sk, int op)

    >  

    >  	sock_ops.sk = sk;

    >  	sock_ops.op = op;

    > +	if (nargs > 0)

    > +		memcpy(sock_ops.args, args, nargs*sizeof(u32));

    
    Small nit given respin: nargs * sizeof(*args)

Thanks, will fix.
    
    >  	ret = BPF_CGROUP_RUN_PROG_SOCK_OPS(&sock_ops);

    >  	if (ret == 0)

    > @@ -2026,18 +2028,70 @@ static inline int tcp_call_bpf(struct sock *sk, int op)

    >  		ret = -1;

    >  	return ret;

    >  }

    > +

    > +static inline int tcp_call_bpf_1arg(struct sock *sk, int op, u32 arg)

    > +{

    > +	return tcp_call_bpf(sk, op, 1, &arg);

    > +}

    > +

    > +static inline int tcp_call_bpf_2arg(struct sock *sk, int op, u32 arg1, u32 arg2)

    > +{

    > +	u32 args[2] = {arg1, arg2};

    > +

    > +	return tcp_call_bpf(sk, op, 2, args);

    > +}

    > +

    > +static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,

    > +				    u32 arg3)

    > +{

    > +	u32 args[3] = {arg1, arg2, arg3};

    > +

    > +	return tcp_call_bpf(sk, op, 3, args);

    > +}

    > +

    > +static inline int tcp_call_bpf_4arg(struct sock *sk, int op, u32 arg1, u32 arg2,

    > +				    u32 arg3, u32 arg4)

    > +{

    > +	u32 args[4] = {arg1, arg2, arg3, arg4};

    > +

    > +	return tcp_call_bpf(sk, op, 4, args);

    > +}

    > +

    >  #else

    > -static inline int tcp_call_bpf(struct sock *sk, int op)

    > +static inline int tcp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)

    >  {

    >  	return -EPERM;

    >  }

    > +

    > +static inline int tcp_call_bpf_1arg(struct sock *sk, int op, u32 arg)

    > +{

    > +	return -EPERM;

    > +}

    > +

    > +static inline int tcp_call_bpf_2arg(struct sock *sk, int op, u32 arg1, u32 arg2)

    > +{

    > +	return -EPERM;

    > +}

    > +

    > +static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,

    > +	u32 arg3)

    
    indent: arg3

OK
    
    > +{

    > +	return -EPERM;

    > +}

    > +

    > +static inline int tcp_call_bpf_4arg(struct sock *sk, int op, u32 arg1, u32 arg2,

    > +				    u32 arg3, u32 arg4)

    > +{

    > +	return -EPERM;

    > +}

    > +

    >  #endif

    
    tcp_call_bpf_1arg() and tcp_call_bpf_4arg() unused for the time being?

Yes, I just thought I should add them for completeness. Should I remove them until
they are actually used?
Daniel Borkmann Jan. 24, 2018, 1:34 a.m. UTC | #3 
On 01/24/2018 02:30 AM, Lawrence Brakmo wrote:
> On 1/23/18, 5:11 PM, "Daniel Borkmann" <daniel@iogearbox.net> wrote:
[...]
>     > +{
>     > +	return -EPERM;
>     > +}
>     > +
>     > +static inline int tcp_call_bpf_4arg(struct sock *sk, int op, u32 arg1, u32 arg2,
>     > +				    u32 arg3, u32 arg4)
>     > +{
>     > +	return -EPERM;
>     > +}
>     > +
>     >  #endif
>     
>     tcp_call_bpf_1arg() and tcp_call_bpf_4arg() unused for the time being?
> 
> Yes, I just thought I should add them for completeness. Should I remove them until
> they are actually used?

Yeah, I think that would be preferred way.

Thanks again,
Daniel
diff mbox series

Patch

diff --git a/include/linux/filter.h b/include/linux/filter.h
index daa5a67..20384c4 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1003,6 +1003,7 @@  struct bpf_sock_ops_kern {
 	struct	sock *sk;
 	u32	op;
 	union {
+		u32 args[4];
 		u32 reply;
 		u32 replylong[4];
 	};
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 108d16a..8e9111f 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2005,7 +2005,7 @@  void tcp_cleanup_ulp(struct sock *sk);
  * program loaded).
  */
 #ifdef CONFIG_BPF
-static inline int tcp_call_bpf(struct sock *sk, int op)
+static inline int tcp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
 {
 	struct bpf_sock_ops_kern sock_ops;
 	int ret;
@@ -2018,6 +2018,8 @@  static inline int tcp_call_bpf(struct sock *sk, int op)
 
 	sock_ops.sk = sk;
 	sock_ops.op = op;
+	if (nargs > 0)
+		memcpy(sock_ops.args, args, nargs*sizeof(u32));
 
 	ret = BPF_CGROUP_RUN_PROG_SOCK_OPS(&sock_ops);
 	if (ret == 0)
@@ -2026,18 +2028,70 @@  static inline int tcp_call_bpf(struct sock *sk, int op)
 		ret = -1;
 	return ret;
 }
+
+static inline int tcp_call_bpf_1arg(struct sock *sk, int op, u32 arg)
+{
+	return tcp_call_bpf(sk, op, 1, &arg);
+}
+
+static inline int tcp_call_bpf_2arg(struct sock *sk, int op, u32 arg1, u32 arg2)
+{
+	u32 args[2] = {arg1, arg2};
+
+	return tcp_call_bpf(sk, op, 2, args);
+}
+
+static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,
+				    u32 arg3)
+{
+	u32 args[3] = {arg1, arg2, arg3};
+
+	return tcp_call_bpf(sk, op, 3, args);
+}
+
+static inline int tcp_call_bpf_4arg(struct sock *sk, int op, u32 arg1, u32 arg2,
+				    u32 arg3, u32 arg4)
+{
+	u32 args[4] = {arg1, arg2, arg3, arg4};
+
+	return tcp_call_bpf(sk, op, 4, args);
+}
+
 #else
-static inline int tcp_call_bpf(struct sock *sk, int op)
+static inline int tcp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
 {
 	return -EPERM;
 }
+
+static inline int tcp_call_bpf_1arg(struct sock *sk, int op, u32 arg)
+{
+	return -EPERM;
+}
+
+static inline int tcp_call_bpf_2arg(struct sock *sk, int op, u32 arg1, u32 arg2)
+{
+	return -EPERM;
+}
+
+static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,
+	u32 arg3)
+{
+	return -EPERM;
+}
+
+static inline int tcp_call_bpf_4arg(struct sock *sk, int op, u32 arg1, u32 arg2,
+				    u32 arg3, u32 arg4)
+{
+	return -EPERM;
+}
+
 #endif
 
 static inline u32 tcp_timeout_init(struct sock *sk)
 {
 	int timeout;
 
-	timeout = tcp_call_bpf(sk, BPF_SOCK_OPS_TIMEOUT_INIT);
+	timeout = tcp_call_bpf(sk, BPF_SOCK_OPS_TIMEOUT_INIT, 0, NULL);
 
 	if (timeout <= 0)
 		timeout = TCP_TIMEOUT_INIT;
@@ -2048,7 +2102,7 @@  static inline u32 tcp_rwnd_init_bpf(struct sock *sk)
 {
 	int rwnd;
 
-	rwnd = tcp_call_bpf(sk, BPF_SOCK_OPS_RWND_INIT);
+	rwnd = tcp_call_bpf(sk, BPF_SOCK_OPS_RWND_INIT, 0, NULL);
 
 	if (rwnd < 0)
 		rwnd = 0;
@@ -2057,7 +2111,7 @@  static inline u32 tcp_rwnd_init_bpf(struct sock *sk)
 
 static inline bool tcp_bpf_ca_needs_ecn(struct sock *sk)
 {
-	return (tcp_call_bpf(sk, BPF_SOCK_OPS_NEEDS_ECN) == 1);
+	return (tcp_call_bpf(sk, BPF_SOCK_OPS_NEEDS_ECN, 0, NULL) == 1);
 }
 
 #if IS_ENABLED(CONFIG_SMC)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 406c19d..8d5874c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -952,8 +952,9 @@  struct bpf_map_info {
 struct bpf_sock_ops {
 	__u32 op;
 	union {
-		__u32 reply;
-		__u32 replylong[4];
+		__u32 args[4];		/* Optionally passed to bpf program */
+		__u32 reply;		/* Returned by bpf program	    */
+		__u32 replylong[4];	/* Optionally returned by bpf prog  */
 	};
 	__u32 family;
 	__u32 remote_ip4;	/* Stored in network byte order */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index d7cf861..88b6244 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -463,7 +463,7 @@  void tcp_init_transfer(struct sock *sk, int bpf_op)
 	tcp_mtup_init(sk);
 	icsk->icsk_af_ops->rebuild_header(sk);
 	tcp_init_metrics(sk);
-	tcp_call_bpf(sk, bpf_op);
+	tcp_call_bpf(sk, bpf_op, 0, NULL);
 	tcp_init_congestion_control(sk);
 	tcp_init_buffer_space(sk);
 }
diff --git a/net/ipv4/tcp_nv.c b/net/ipv4/tcp_nv.c
index 0b5a05b..ddbce73 100644
--- a/net/ipv4/tcp_nv.c
+++ b/net/ipv4/tcp_nv.c
@@ -146,7 +146,7 @@  static void tcpnv_init(struct sock *sk)
 	 * within a datacenter, where we have reasonable estimates of
 	 * RTTs
 	 */
-	base_rtt = tcp_call_bpf(sk, BPF_SOCK_OPS_BASE_RTT);
+	base_rtt = tcp_call_bpf(sk, BPF_SOCK_OPS_BASE_RTT, 0, NULL);
 	if (base_rtt > 0) {
 		ca->nv_base_rtt = base_rtt;
 		ca->nv_lower_bound_rtt = (base_rtt * 205) >> 8; /* 80% */
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 95461f0..d12f7f7 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3469,7 +3469,7 @@  int tcp_connect(struct sock *sk)
 	struct sk_buff *buff;
 	int err;
 
-	tcp_call_bpf(sk, BPF_SOCK_OPS_TCP_CONNECT_CB);
+	tcp_call_bpf(sk, BPF_SOCK_OPS_TCP_CONNECT_CB, 0, NULL);
 
 	if (inet_csk(sk)->icsk_af_ops->rebuild_header(sk))
 		return -EHOSTUNREACH; /* Routing failure or similar. */