[PATCH v2 bpf-next 0/2] Add support for bpf_setsockopt and

[PATCH v2 bpf-next 0/2] Add support for bpf_setsockopt and

[Prankur gupta]

v2: Added details about the test in commit log

This patch contains support to set and get socket options from
setsockopt
bpf program.
This enables us to set multiple socket option when the user changes a
particular socket option.
Example use case, when the user sets the IPV6_TCLASS socket option we
would also like to change the tcp-cc for that socket. We don't have any
use case for calling bpf_setsockopt from supposedly read-only
sys_getsockopt, so it is made available to BPF_CGROUP_SETSOCKOPT only.

Prankur gupta (2):
  bpf: Add support for {set|get} socket options from setsockopt BPF
  selftests/bpf: Add test for {set|get} socket option from setsockopt
    BPF program

 kernel/bpf/cgroup.c                           |  8 +++
 tools/testing/selftests/bpf/bpf_tcp_helpers.h | 18 +++++
 .../bpf/prog_tests/sockopt_qos_to_cc.c        | 70 +++++++++++++++++++
 .../selftests/bpf/progs/sockopt_qos_to_cc.c   | 39 +++++++++++
 4 files changed, 135 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
 create mode 100644 tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c

-- 
2.30.2

[PATCH bpf-next 1/2] bpf: Add support for {set|get} socket options from setsockopt BPF

[Prankur gupta]

Add logic to call bpf_setsockopt and bpf_getsockopt from
setsockopt BPF programs.
Example use case, when the user sets the IPV6_TCLASS socket option
we would also like to change the tcp-cc for that socket.
We don't have any use case for calling bpf_setsockopt from
supposedly read-only sys_getsockopti, so it is made available to
BPF_CGROUP_SETSOCKOPT only.

Signed-off-by: Prankur gupta <prankgup@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 kernel/bpf/cgroup.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 9f35928bab0a..8e9d99e2ade4 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1873,6 +1873,14 @@ cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_sk_storage_get_proto;
 	case BPF_FUNC_sk_storage_delete:
 		return &bpf_sk_storage_delete_proto;
+	case BPF_FUNC_setsockopt:
+		if (prog->expected_attach_type == BPF_CGROUP_SETSOCKOPT)
+			return &bpf_sk_setsockopt_proto;
+		return NULL;
+	case BPF_FUNC_getsockopt:
+		if (prog->expected_attach_type == BPF_CGROUP_SETSOCKOPT)
+			return &bpf_sk_getsockopt_proto;
+		return NULL;
 #endif
 #ifdef CONFIG_INET
 	case BPF_FUNC_tcp_sock:
-- 
2.30.2

[PATCH bpf-next 2/2] selftests/bpf: Add test for {set|get} socket option from setsockopt BPF program

[Prankur gupta]

Adding selftests for new added functionality to call bpf_setsockopt and
bpf_getsockopt from setsockopt BPF programs

Test Details:
1. BPF Program
   Checks for changes in IPV6_TCLASS(SOL_IPV6) via setsockopt
   If the cca for the socket is not cubic do nothing
   If the newly set value for IPV6_TCLASS is 45 (0x2d) (as per our usecase)
   then change the cc from cubic to reno

2. User Space Program
   Creates an AF_INET6 socket and set the cca for that to be "cubic"
   Attach the program and set the IPV6_TCLASS to 0x2d using setsockopt
   Verify the cca for the socket changed to reno

Signed-off-by: Prankur gupta <prankgup@fb.com>
---
 tools/testing/selftests/bpf/bpf_tcp_helpers.h | 18 +++++
 .../bpf/prog_tests/sockopt_qos_to_cc.c        | 70 +++++++++++++++++++
 .../selftests/bpf/progs/sockopt_qos_to_cc.c   | 39 +++++++++++
 3 files changed, 127 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
 create mode 100644 tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c

diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index 029589c008c9..c9f9bdad60c7 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -12,6 +12,10 @@
 SEC("struct_ops/"#name) \
 BPF_PROG(name, args)
 
+#ifndef SOL_TCP
+#define SOL_TCP 6
+#endif
+
 #define tcp_jiffies32 ((__u32)bpf_jiffies64())
 
 struct sock_common {
@@ -203,6 +207,20 @@ static __always_inline bool tcp_is_cwnd_limited(const struct sock *sk)
 	return !!BPF_CORE_READ_BITFIELD(tp, is_cwnd_limited);
 }
 
+static __always_inline bool tcp_cc_eq(const char *a, const char *b)
+{
+	int i;
+
+	for (i = 0; i < TCP_CA_NAME_MAX; i++) {
+		if (a[i] != b[i])
+			return false;
+		if (!a[i])
+			break;
+	}
+
+	return true;
+}
+
 extern __u32 tcp_slow_start(struct tcp_sock *tp, __u32 acked) __ksym;
 extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c b/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
new file mode 100644
index 000000000000..6b53b3cb8dad
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Facebook */
+#include <test_progs.h>
+#include <netinet/tcp.h>
+#include "sockopt_qos_to_cc.skel.h"
+
+static void run_setsockopt_test(int cg_fd, int sock_fd)
+{
+	socklen_t optlen;
+	char cc[16]; /* TCP_CA_NAME_MAX */
+	int buf;
+	int err = -1;
+
+	buf = 0x2D;
+	err = setsockopt(sock_fd, SOL_IPV6, IPV6_TCLASS, &buf, sizeof(buf));
+	if (!ASSERT_OK(err, "setsockopt(sock_fd, IPV6_TCLASS)"))
+		return;
+
+	/* Verify the setsockopt cc change */
+	optlen = sizeof(cc);
+	err = getsockopt(sock_fd, SOL_TCP, TCP_CONGESTION, cc, &optlen);
+	if (!ASSERT_OK(err, "getsockopt(sock_fd, TCP_CONGESTION)"))
+		return;
+
+	if (!ASSERT_STREQ(cc, "reno", "getsockopt(sock_fd, TCP_CONGESTION)"))
+		return;
+}
+
+void test_sockopt_qos_to_cc(void)
+{
+	struct sockopt_qos_to_cc *skel;
+	char cc_cubic[16] = "cubic"; /* TCP_CA_NAME_MAX */
+	int cg_fd = -1;
+	int sock_fd = -1;
+	int err;
+
+	cg_fd = test__join_cgroup("/sockopt_qos_to_cc");
+	if (!ASSERT_GE(cg_fd, 0, "cg-join(sockopt_qos_to_cc)"))
+		return;
+
+	skel = sockopt_qos_to_cc__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel"))
+		goto done;
+
+	sock_fd = socket(AF_INET6, SOCK_STREAM, 0);
+	if (!ASSERT_GE(sock_fd, 0, "v6 socket open"))
+		goto done;
+
+	err = setsockopt(sock_fd, SOL_TCP, TCP_CONGESTION, &cc_cubic,
+			 sizeof(cc_cubic));
+	if (!ASSERT_OK(err, "setsockopt(sock_fd, TCP_CONGESTION)"))
+		goto done;
+
+	skel->links.sockopt_qos_to_cc =
+		bpf_program__attach_cgroup(skel->progs.sockopt_qos_to_cc,
+					   cg_fd);
+	if (!ASSERT_OK_PTR(skel->links.sockopt_qos_to_cc,
+			   "prog_attach(sockopt_qos_to_cc)"))
+		goto done;
+
+	run_setsockopt_test(cg_fd, sock_fd);
+
+done:
+	if (sock_fd != -1)
+		close(sock_fd);
+	if (cg_fd != -1)
+		close(cg_fd);
+	/* destroy can take null and error pointer */
+	sockopt_qos_to_cc__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c b/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
new file mode 100644
index 000000000000..1bce83b6e3a7
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Facebook */
+#include <string.h>
+#include <linux/tcp.h>
+#include <netinet/in.h>
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_tcp_helpers.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("cgroup/setsockopt")
+int sockopt_qos_to_cc(struct bpf_sockopt *ctx)
+{
+	void *optval_end = ctx->optval_end;
+	int *optval = ctx->optval;
+	char buf[TCP_CA_NAME_MAX];
+	char cc_reno[TCP_CA_NAME_MAX] = "reno";
+	char cc_cubic[TCP_CA_NAME_MAX] = "cubic";
+
+	if (ctx->level != SOL_IPV6 || ctx->optname != IPV6_TCLASS)
+		return 1;
+
+	if (optval + 1 > optval_end)
+		return 0; /* EPERM, bounds check */
+
+	if (bpf_getsockopt(ctx->sk, SOL_TCP, TCP_CONGESTION, &buf, sizeof(buf)))
+		return 0;
+
+	if (!tcp_cc_eq(buf, cc_cubic))
+		return 0;
+
+	if (*optval == 0x2d) {
+		if (bpf_setsockopt(ctx->sk, SOL_TCP, TCP_CONGESTION, &cc_reno,
+				sizeof(cc_reno)))
+			return 0;
+	}
+	return 1;
+}
-- 
2.30.2

Re: [PATCH bpf-next 2/2] selftests/bpf: Add test for {set|get} socket option from setsockopt BPF program

[Song Liu]

On Tue, Aug 17, 2021 at 3:43 PM Prankur gupta <prankgup@fb.com> wrote:
>
> Adding selftests for new added functionality to call bpf_setsockopt and
> bpf_getsockopt from setsockopt BPF programs
>
> Test Details:
> 1. BPF Program
>    Checks for changes in IPV6_TCLASS(SOL_IPV6) via setsockopt
>    If the cca for the socket is not cubic do nothing
>    If the newly set value for IPV6_TCLASS is 45 (0x2d) (as per our usecase)
>    then change the cc from cubic to reno
>
> 2. User Space Program
>    Creates an AF_INET6 socket and set the cca for that to be "cubic"
>    Attach the program and set the IPV6_TCLASS to 0x2d using setsockopt
>    Verify the cca for the socket changed to reno
>
> Signed-off-by: Prankur gupta <prankgup@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>

Re: [PATCH v2 bpf-next 0/2] Add support for bpf_setsockopt and

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (refs/heads/master):

On Tue, 17 Aug 2021 15:42:19 -0700 you wrote:
> v2: Added details about the test in commit log
> 
> This patch contains support to set and get socket options from
> setsockopt
> bpf program.
> This enables us to set multiple socket option when the user changes a
> particular socket option.
> Example use case, when the user sets the IPV6_TCLASS socket option we
> would also like to change the tcp-cc for that socket. We don't have any
> use case for calling bpf_setsockopt from supposedly read-only
> sys_getsockopt, so it is made available to BPF_CGROUP_SETSOCKOPT only.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] bpf: Add support for {set|get} socket options from setsockopt BPF
    https://git.kernel.org/bpf/bpf-next/c/2c531639deb5
  - [bpf-next,2/2] selftests/bpf: Add test for {set|get} socket option from setsockopt BPF program
    https://git.kernel.org/bpf/bpf-next/c/f2a6ee924d26

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf-next 2/2] selftests/bpf: Add test for {set|get} socket option from setsockopt BPF program

[Song Liu]

On Mon, Aug 16, 2021 at 5:20 PM Song Liu <song@kernel.org> wrote:
>
> On Mon, Aug 16, 2021 at 4:18 PM Prankur gupta <prankgup@fb.com> wrote:
> >
> > Adding selftests for new added functionality to call bpf_setsockopt and
> > bpf_getsockopt from setsockopt BPF programs
> >
> > Signed-off-by: Prankur gupta <prankgup@fb.com>
> > ---
> >  tools/testing/selftests/bpf/bpf_tcp_helpers.h | 18 +++++
> >  .../bpf/prog_tests/sockopt_qos_to_cc.c        | 70 +++++++++++++++++++
> >  .../selftests/bpf/progs/sockopt_qos_to_cc.c   | 39 +++++++++++
> >  3 files changed, 127 insertions(+)
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> > index 029589c008c9..c9f9bdad60c7 100644
> > --- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> > +++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> > @@ -12,6 +12,10 @@
> >  SEC("struct_ops/"#name) \
> >  BPF_PROG(name, args)
> >
> > +#ifndef SOL_TCP
> > +#define SOL_TCP 6
> > +#endif
> > +
> >  #define tcp_jiffies32 ((__u32)bpf_jiffies64())
> >
> >  struct sock_common {
> > @@ -203,6 +207,20 @@ static __always_inline bool tcp_is_cwnd_limited(const struct sock *sk)
> >         return !!BPF_CORE_READ_BITFIELD(tp, is_cwnd_limited);
> >  }
> >
> > +static __always_inline bool tcp_cc_eq(const char *a, const char *b)
> > +{
> > +       int i;
> > +
> > +       for (i = 0; i < TCP_CA_NAME_MAX; i++) {
> > +               if (a[i] != b[i])
> > +                       return false;
> > +               if (!a[i])
> > +                       break;
> > +       }
> > +
> > +       return true;
> > +}
>
> IIUC, this is copied from bpf_iter_setsockopt.c. I think we should be
> more careful with it
> as this is a header. With current code,
>
>    tcp_cc_eq("ABC", "ABCD") = true;
>    tcp_cc_eq("ABCD", "ABC") = false;
>
> Is this expected?

Ooops, I misread the code. Please ignore the comment above.

>
> > +
> >  extern __u32 tcp_slow_start(struct tcp_sock *tp, __u32 acked) __ksym;
> >  extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym;
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c b/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
> > new file mode 100644
> > index 000000000000..6b53b3cb8dad
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2021 Facebook */
> > +#include <test_progs.h>
> > +#include <netinet/tcp.h>
> > +#include "sockopt_qos_to_cc.skel.h"
> > +
> > +static void run_setsockopt_test(int cg_fd, int sock_fd)
> > +{
> > +       socklen_t optlen;
> > +       char cc[16]; /* TCP_CA_NAME_MAX */
> > +       int buf;
> > +       int err = -1;
> nit: this err = -1 is not needed.
>
> > +
> > +       buf = 0x2D;
>
> Please explain the test case in the commit log. Otherwise, it is not
> easy to follow
> the test case. For example, why we use 0x2d ('-'?) here?
>
> > +       err = setsockopt(sock_fd, SOL_IPV6, IPV6_TCLASS, &buf, sizeof(buf));
> > +       if (!ASSERT_OK(err, "setsockopt(sock_fd, IPV6_TCLASS)"))
> > +               return;
> > +
> > +       /* Verify the setsockopt cc change */
> > +       optlen = sizeof(cc);
> > +       err = getsockopt(sock_fd, SOL_TCP, TCP_CONGESTION, cc, &optlen);
> > +       if (!ASSERT_OK(err, "getsockopt(sock_fd, TCP_CONGESTION)"))
> > +               return;
> > +
> > +       if (!ASSERT_STREQ(cc, "reno", "getsockopt(sock_fd, TCP_CONGESTION)"))
> > +               return;
> > +}
> > +
> > +void test_sockopt_qos_to_cc(void)
> > +{
> > +       struct sockopt_qos_to_cc *skel;
> > +       char cc_cubic[16] = "cubic"; /* TCP_CA_NAME_MAX */
> > +       int cg_fd = -1;
> > +       int sock_fd = -1;
> > +       int err;
> > +
> > +       cg_fd = test__join_cgroup("/sockopt_qos_to_cc");
> > +       if (!ASSERT_GE(cg_fd, 0, "cg-join(sockopt_qos_to_cc)"))
> > +               return;
> > +
> > +       skel = sockopt_qos_to_cc__open_and_load();
> > +       if (!ASSERT_OK_PTR(skel, "skel"))
> > +               goto done;
> > +
> > +       sock_fd = socket(AF_INET6, SOCK_STREAM, 0);
> > +       if (!ASSERT_GE(sock_fd, 0, "v6 socket open"))
> > +               goto done;
> > +
> > +       err = setsockopt(sock_fd, SOL_TCP, TCP_CONGESTION, &cc_cubic,
> > +                        sizeof(cc_cubic));
> > +       if (!ASSERT_OK(err, "setsockopt(sock_fd, TCP_CONGESTION)"))
> > +               goto done;
> > +
> > +       skel->links.sockopt_qos_to_cc =
> > +               bpf_program__attach_cgroup(skel->progs.sockopt_qos_to_cc,
> > +                                          cg_fd);
> > +       if (!ASSERT_OK_PTR(skel->links.sockopt_qos_to_cc,
> > +                          "prog_attach(sockopt_qos_to_cc)"))
> > +               goto done;
> > +
> > +       run_setsockopt_test(cg_fd, sock_fd);
> > +
> > +done:
> > +       if (sock_fd != -1)
> > +               close(sock_fd);
> > +       if (cg_fd != -1)
> > +               close(cg_fd);
> > +       /* destroy can take null and error pointer */
> > +       sockopt_qos_to_cc__destroy(skel);
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c b/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
> > new file mode 100644
> > index 000000000000..1bce83b6e3a7
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
> > @@ -0,0 +1,39 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2021 Facebook */
> > +#include <string.h>
> > +#include <linux/tcp.h>
> > +#include <netinet/in.h>
> > +#include <linux/bpf.h>
> > +#include <bpf/bpf_helpers.h>
> > +#include "bpf_tcp_helpers.h"
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +SEC("cgroup/setsockopt")
> > +int sockopt_qos_to_cc(struct bpf_sockopt *ctx)
> > +{
> > +       void *optval_end = ctx->optval_end;
> > +       int *optval = ctx->optval;
> > +       char buf[TCP_CA_NAME_MAX];
> > +       char cc_reno[TCP_CA_NAME_MAX] = "reno";
> > +       char cc_cubic[TCP_CA_NAME_MAX] = "cubic";
> > +
> > +       if (ctx->level != SOL_IPV6 || ctx->optname != IPV6_TCLASS)
> > +               return 1;
> > +
> > +       if (optval + 1 > optval_end)
> > +               return 0; /* EPERM, bounds check */
> > +
> > +       if (bpf_getsockopt(ctx->sk, SOL_TCP, TCP_CONGESTION, &buf, sizeof(buf)))
> > +               return 0;
> > +
> > +       if (!tcp_cc_eq(buf, cc_cubic))
> > +               return 0;
> > +
> > +       if (*optval == 0x2d) {
> > +               if (bpf_setsockopt(ctx->sk, SOL_TCP, TCP_CONGESTION, &cc_reno,
> > +                               sizeof(cc_reno)))
> > +                       return 0;
> > +       }
> > +       return 1;
> > +}
> > --
> > 2.30.2
> >

Re: [PATCH bpf-next 2/2] selftests/bpf: Add test for {set|get} socket option from setsockopt BPF program

[Song Liu]

On Mon, Aug 16, 2021 at 4:18 PM Prankur gupta <prankgup@fb.com> wrote:
>
> Adding selftests for new added functionality to call bpf_setsockopt and
> bpf_getsockopt from setsockopt BPF programs
>
> Signed-off-by: Prankur gupta <prankgup@fb.com>
> ---
>  tools/testing/selftests/bpf/bpf_tcp_helpers.h | 18 +++++
>  .../bpf/prog_tests/sockopt_qos_to_cc.c        | 70 +++++++++++++++++++
>  .../selftests/bpf/progs/sockopt_qos_to_cc.c   | 39 +++++++++++
>  3 files changed, 127 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
>  create mode 100644 tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
>
> diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> index 029589c008c9..c9f9bdad60c7 100644
> --- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
> @@ -12,6 +12,10 @@
>  SEC("struct_ops/"#name) \
>  BPF_PROG(name, args)
>
> +#ifndef SOL_TCP
> +#define SOL_TCP 6
> +#endif
> +
>  #define tcp_jiffies32 ((__u32)bpf_jiffies64())
>
>  struct sock_common {
> @@ -203,6 +207,20 @@ static __always_inline bool tcp_is_cwnd_limited(const struct sock *sk)
>         return !!BPF_CORE_READ_BITFIELD(tp, is_cwnd_limited);
>  }
>
> +static __always_inline bool tcp_cc_eq(const char *a, const char *b)
> +{
> +       int i;
> +
> +       for (i = 0; i < TCP_CA_NAME_MAX; i++) {
> +               if (a[i] != b[i])
> +                       return false;
> +               if (!a[i])
> +                       break;
> +       }
> +
> +       return true;
> +}

IIUC, this is copied from bpf_iter_setsockopt.c. I think we should be
more careful with it
as this is a header. With current code,

   tcp_cc_eq("ABC", "ABCD") = true;
   tcp_cc_eq("ABCD", "ABC") = false;

Is this expected?


> +
>  extern __u32 tcp_slow_start(struct tcp_sock *tp, __u32 acked) __ksym;
>  extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym;
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c b/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
> new file mode 100644
> index 000000000000..6b53b3cb8dad
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2021 Facebook */
> +#include <test_progs.h>
> +#include <netinet/tcp.h>
> +#include "sockopt_qos_to_cc.skel.h"
> +
> +static void run_setsockopt_test(int cg_fd, int sock_fd)
> +{
> +       socklen_t optlen;
> +       char cc[16]; /* TCP_CA_NAME_MAX */
> +       int buf;
> +       int err = -1;
nit: this err = -1 is not needed.

> +
> +       buf = 0x2D;

Please explain the test case in the commit log. Otherwise, it is not
easy to follow
the test case. For example, why we use 0x2d ('-'?) here?

> +       err = setsockopt(sock_fd, SOL_IPV6, IPV6_TCLASS, &buf, sizeof(buf));
> +       if (!ASSERT_OK(err, "setsockopt(sock_fd, IPV6_TCLASS)"))
> +               return;
> +
> +       /* Verify the setsockopt cc change */
> +       optlen = sizeof(cc);
> +       err = getsockopt(sock_fd, SOL_TCP, TCP_CONGESTION, cc, &optlen);
> +       if (!ASSERT_OK(err, "getsockopt(sock_fd, TCP_CONGESTION)"))
> +               return;
> +
> +       if (!ASSERT_STREQ(cc, "reno", "getsockopt(sock_fd, TCP_CONGESTION)"))
> +               return;
> +}
> +
> +void test_sockopt_qos_to_cc(void)
> +{
> +       struct sockopt_qos_to_cc *skel;
> +       char cc_cubic[16] = "cubic"; /* TCP_CA_NAME_MAX */
> +       int cg_fd = -1;
> +       int sock_fd = -1;
> +       int err;
> +
> +       cg_fd = test__join_cgroup("/sockopt_qos_to_cc");
> +       if (!ASSERT_GE(cg_fd, 0, "cg-join(sockopt_qos_to_cc)"))
> +               return;
> +
> +       skel = sockopt_qos_to_cc__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "skel"))
> +               goto done;
> +
> +       sock_fd = socket(AF_INET6, SOCK_STREAM, 0);
> +       if (!ASSERT_GE(sock_fd, 0, "v6 socket open"))
> +               goto done;
> +
> +       err = setsockopt(sock_fd, SOL_TCP, TCP_CONGESTION, &cc_cubic,
> +                        sizeof(cc_cubic));
> +       if (!ASSERT_OK(err, "setsockopt(sock_fd, TCP_CONGESTION)"))
> +               goto done;
> +
> +       skel->links.sockopt_qos_to_cc =
> +               bpf_program__attach_cgroup(skel->progs.sockopt_qos_to_cc,
> +                                          cg_fd);
> +       if (!ASSERT_OK_PTR(skel->links.sockopt_qos_to_cc,
> +                          "prog_attach(sockopt_qos_to_cc)"))
> +               goto done;
> +
> +       run_setsockopt_test(cg_fd, sock_fd);
> +
> +done:
> +       if (sock_fd != -1)
> +               close(sock_fd);
> +       if (cg_fd != -1)
> +               close(cg_fd);
> +       /* destroy can take null and error pointer */
> +       sockopt_qos_to_cc__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c b/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
> new file mode 100644
> index 000000000000..1bce83b6e3a7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2021 Facebook */
> +#include <string.h>
> +#include <linux/tcp.h>
> +#include <netinet/in.h>
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_tcp_helpers.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +SEC("cgroup/setsockopt")
> +int sockopt_qos_to_cc(struct bpf_sockopt *ctx)
> +{
> +       void *optval_end = ctx->optval_end;
> +       int *optval = ctx->optval;
> +       char buf[TCP_CA_NAME_MAX];
> +       char cc_reno[TCP_CA_NAME_MAX] = "reno";
> +       char cc_cubic[TCP_CA_NAME_MAX] = "cubic";
> +
> +       if (ctx->level != SOL_IPV6 || ctx->optname != IPV6_TCLASS)
> +               return 1;
> +
> +       if (optval + 1 > optval_end)
> +               return 0; /* EPERM, bounds check */
> +
> +       if (bpf_getsockopt(ctx->sk, SOL_TCP, TCP_CONGESTION, &buf, sizeof(buf)))
> +               return 0;
> +
> +       if (!tcp_cc_eq(buf, cc_cubic))
> +               return 0;
> +
> +       if (*optval == 0x2d) {
> +               if (bpf_setsockopt(ctx->sk, SOL_TCP, TCP_CONGESTION, &cc_reno,
> +                               sizeof(cc_reno)))
> +                       return 0;
> +       }
> +       return 1;
> +}
> --
> 2.30.2
>

[PATCH bpf-next 2/2] selftests/bpf: Add test for {set|get} socket option from setsockopt BPF program

[Prankur gupta]

Adding selftests for new added functionality to call bpf_setsockopt and
bpf_getsockopt from setsockopt BPF programs

Signed-off-by: Prankur gupta <prankgup@fb.com>
---
 tools/testing/selftests/bpf/bpf_tcp_helpers.h | 18 +++++
 .../bpf/prog_tests/sockopt_qos_to_cc.c        | 70 +++++++++++++++++++
 .../selftests/bpf/progs/sockopt_qos_to_cc.c   | 39 +++++++++++
 3 files changed, 127 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
 create mode 100644 tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c

diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
index 029589c008c9..c9f9bdad60c7 100644
--- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h
@@ -12,6 +12,10 @@
 SEC("struct_ops/"#name) \
 BPF_PROG(name, args)
 
+#ifndef SOL_TCP
+#define SOL_TCP 6
+#endif
+
 #define tcp_jiffies32 ((__u32)bpf_jiffies64())
 
 struct sock_common {
@@ -203,6 +207,20 @@ static __always_inline bool tcp_is_cwnd_limited(const struct sock *sk)
 	return !!BPF_CORE_READ_BITFIELD(tp, is_cwnd_limited);
 }
 
+static __always_inline bool tcp_cc_eq(const char *a, const char *b)
+{
+	int i;
+
+	for (i = 0; i < TCP_CA_NAME_MAX; i++) {
+		if (a[i] != b[i])
+			return false;
+		if (!a[i])
+			break;
+	}
+
+	return true;
+}
+
 extern __u32 tcp_slow_start(struct tcp_sock *tp, __u32 acked) __ksym;
 extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c b/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
new file mode 100644
index 000000000000..6b53b3cb8dad
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt_qos_to_cc.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Facebook */
+#include <test_progs.h>
+#include <netinet/tcp.h>
+#include "sockopt_qos_to_cc.skel.h"
+
+static void run_setsockopt_test(int cg_fd, int sock_fd)
+{
+	socklen_t optlen;
+	char cc[16]; /* TCP_CA_NAME_MAX */
+	int buf;
+	int err = -1;
+
+	buf = 0x2D;
+	err = setsockopt(sock_fd, SOL_IPV6, IPV6_TCLASS, &buf, sizeof(buf));
+	if (!ASSERT_OK(err, "setsockopt(sock_fd, IPV6_TCLASS)"))
+		return;
+
+	/* Verify the setsockopt cc change */
+	optlen = sizeof(cc);
+	err = getsockopt(sock_fd, SOL_TCP, TCP_CONGESTION, cc, &optlen);
+	if (!ASSERT_OK(err, "getsockopt(sock_fd, TCP_CONGESTION)"))
+		return;
+
+	if (!ASSERT_STREQ(cc, "reno", "getsockopt(sock_fd, TCP_CONGESTION)"))
+		return;
+}
+
+void test_sockopt_qos_to_cc(void)
+{
+	struct sockopt_qos_to_cc *skel;
+	char cc_cubic[16] = "cubic"; /* TCP_CA_NAME_MAX */
+	int cg_fd = -1;
+	int sock_fd = -1;
+	int err;
+
+	cg_fd = test__join_cgroup("/sockopt_qos_to_cc");
+	if (!ASSERT_GE(cg_fd, 0, "cg-join(sockopt_qos_to_cc)"))
+		return;
+
+	skel = sockopt_qos_to_cc__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel"))
+		goto done;
+
+	sock_fd = socket(AF_INET6, SOCK_STREAM, 0);
+	if (!ASSERT_GE(sock_fd, 0, "v6 socket open"))
+		goto done;
+
+	err = setsockopt(sock_fd, SOL_TCP, TCP_CONGESTION, &cc_cubic,
+			 sizeof(cc_cubic));
+	if (!ASSERT_OK(err, "setsockopt(sock_fd, TCP_CONGESTION)"))
+		goto done;
+
+	skel->links.sockopt_qos_to_cc =
+		bpf_program__attach_cgroup(skel->progs.sockopt_qos_to_cc,
+					   cg_fd);
+	if (!ASSERT_OK_PTR(skel->links.sockopt_qos_to_cc,
+			   "prog_attach(sockopt_qos_to_cc)"))
+		goto done;
+
+	run_setsockopt_test(cg_fd, sock_fd);
+
+done:
+	if (sock_fd != -1)
+		close(sock_fd);
+	if (cg_fd != -1)
+		close(cg_fd);
+	/* destroy can take null and error pointer */
+	sockopt_qos_to_cc__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c b/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
new file mode 100644
index 000000000000..1bce83b6e3a7
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/sockopt_qos_to_cc.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Facebook */
+#include <string.h>
+#include <linux/tcp.h>
+#include <netinet/in.h>
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_tcp_helpers.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("cgroup/setsockopt")
+int sockopt_qos_to_cc(struct bpf_sockopt *ctx)
+{
+	void *optval_end = ctx->optval_end;
+	int *optval = ctx->optval;
+	char buf[TCP_CA_NAME_MAX];
+	char cc_reno[TCP_CA_NAME_MAX] = "reno";
+	char cc_cubic[TCP_CA_NAME_MAX] = "cubic";
+
+	if (ctx->level != SOL_IPV6 || ctx->optname != IPV6_TCLASS)
+		return 1;
+
+	if (optval + 1 > optval_end)
+		return 0; /* EPERM, bounds check */
+
+	if (bpf_getsockopt(ctx->sk, SOL_TCP, TCP_CONGESTION, &buf, sizeof(buf)))
+		return 0;
+
+	if (!tcp_cc_eq(buf, cc_cubic))
+		return 0;
+
+	if (*optval == 0x2d) {
+		if (bpf_setsockopt(ctx->sk, SOL_TCP, TCP_CONGESTION, &cc_reno,
+				sizeof(cc_reno)))
+			return 0;
+	}
+	return 1;
+}
-- 
2.30.2