| Message ID | 20220227202757.519015-3-jakub@cloudflare.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Fixes for sock_fields selftests | expand |
On Sun, Feb 27, 2022 at 09:27:56PM +0100, Jakub Sitnicki wrote: > cgroup_skb/egress programs which sock_fields test installs process packets > flying in both directions, from the client to the server, and in reverse > direction. > > Recently added dst_port check relies on the fact that destination > port (remote peer port) of the socket which sends the packet is known ahead > of time. This holds true only for the client socket, which connects to the > known server port. > > Filter out any traffic that is not bound to be egressing from the client > socket in the test program for reading the dst_port. > > Fixes: 8f50f16ff39d ("selftests/bpf: Extend verifier and bpf_sock tests for dst_port loads") > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > --- > .../testing/selftests/bpf/progs/test_sock_fields.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/bpf/progs/test_sock_fields.c b/tools/testing/selftests/bpf/progs/test_sock_fields.c > index 3e2e3ee51cc9..186fed1deaab 100644 > --- a/tools/testing/selftests/bpf/progs/test_sock_fields.c > +++ b/tools/testing/selftests/bpf/progs/test_sock_fields.c > @@ -42,6 +42,11 @@ struct { > __type(value, struct bpf_spinlock_cnt); > } sk_pkt_out_cnt10 SEC(".maps"); > > +enum { > + TCP_SYN_SENT = 2, > + TCP_LISTEN = 10, Thanks for the clean up. A nit. directly use BPF_TCP_SYN_SENT and BPF_TCP_LISTEN. Others lgtm. Acked-by: Martin KaFai Lau <kafai@fb.com>
On Mon, Feb 28, 2022 at 10:25 PM -08, Martin KaFai Lau wrote: > On Sun, Feb 27, 2022 at 09:27:56PM +0100, Jakub Sitnicki wrote: >> cgroup_skb/egress programs which sock_fields test installs process packets >> flying in both directions, from the client to the server, and in reverse >> direction. >> >> Recently added dst_port check relies on the fact that destination >> port (remote peer port) of the socket which sends the packet is known ahead >> of time. This holds true only for the client socket, which connects to the >> known server port. >> >> Filter out any traffic that is not bound to be egressing from the client >> socket in the test program for reading the dst_port. >> >> Fixes: 8f50f16ff39d ("selftests/bpf: Extend verifier and bpf_sock tests for dst_port loads") >> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> >> --- >> .../testing/selftests/bpf/progs/test_sock_fields.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/tools/testing/selftests/bpf/progs/test_sock_fields.c b/tools/testing/selftests/bpf/progs/test_sock_fields.c >> index 3e2e3ee51cc9..186fed1deaab 100644 >> --- a/tools/testing/selftests/bpf/progs/test_sock_fields.c >> +++ b/tools/testing/selftests/bpf/progs/test_sock_fields.c >> @@ -42,6 +42,11 @@ struct { >> __type(value, struct bpf_spinlock_cnt); >> } sk_pkt_out_cnt10 SEC(".maps"); >> >> +enum { >> + TCP_SYN_SENT = 2, >> + TCP_LISTEN = 10, > Thanks for the clean up. > > A nit. directly use BPF_TCP_SYN_SENT and BPF_TCP_LISTEN. Thanks. Completely forgot about those.
diff --git a/tools/testing/selftests/bpf/progs/test_sock_fields.c b/tools/testing/selftests/bpf/progs/test_sock_fields.c index 3e2e3ee51cc9..186fed1deaab 100644 --- a/tools/testing/selftests/bpf/progs/test_sock_fields.c +++ b/tools/testing/selftests/bpf/progs/test_sock_fields.c @@ -42,6 +42,11 @@ struct { __type(value, struct bpf_spinlock_cnt); } sk_pkt_out_cnt10 SEC(".maps"); +enum { + TCP_SYN_SENT = 2, + TCP_LISTEN = 10, +}; + struct bpf_tcp_sock listen_tp = {}; struct sockaddr_in6 srv_sa6 = {}; struct bpf_tcp_sock cli_tp = {}; @@ -138,7 +143,7 @@ int egress_read_sock_fields(struct __sk_buff *skb) * TCP_LISTEN (10) socket will be copied at the ingress side. */ if (sk->family != AF_INET6 || !is_loopback6(sk->src_ip6) || - sk->state == 10) + sk->state == TCP_LISTEN) return CG_OK; if (sk->src_port == bpf_ntohs(srv_sa6.sin6_port)) { @@ -233,7 +238,7 @@ int ingress_read_sock_fields(struct __sk_buff *skb) return CG_OK; /* Only interested in TCP_LISTEN */ - if (sk->state != 10) + if (sk->state != TCP_LISTEN) return CG_OK; /* It must be a fullsock for cgroup_skb/ingress prog */ @@ -281,6 +286,10 @@ int read_sk_dst_port(struct __sk_buff *skb) if (!sk) RET_LOG(); + /* Ignore everything but the SYN from the client socket */ + if (sk->state != TCP_SYN_SENT) + return CG_OK; + if (!sk_dst_port__load_word(sk)) RET_LOG(); if (!sk_dst_port__load_half(sk))
cgroup_skb/egress programs which sock_fields test installs process packets flying in both directions, from the client to the server, and in reverse direction. Recently added dst_port check relies on the fact that destination port (remote peer port) of the socket which sends the packet is known ahead of time. This holds true only for the client socket, which connects to the known server port. Filter out any traffic that is not bound to be egressing from the client socket in the test program for reading the dst_port. Fixes: 8f50f16ff39d ("selftests/bpf: Extend verifier and bpf_sock tests for dst_port loads") Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> --- .../testing/selftests/bpf/progs/test_sock_fields.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)