* [PATCH bpf] selftests/bpf: fix v4_to_v6 in sk_lookup @ 2020-08-07 22:38 Stanislav Fomichev 2020-08-08 18:46 ` Jakub Sitnicki 2020-08-11 13:43 ` Daniel Borkmann 0 siblings, 2 replies; 5+ messages in thread From: Stanislav Fomichev @ 2020-08-07 22:38 UTC (permalink / raw) To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev I'm getting some garbage in bytes 8 and 9 when doing conversion from sockaddr_in to sockaddr_in6 (leftover from AF_INET?). Let's explicitly clear the higher bytes. Signed-off-by: Stanislav Fomichev <sdf@google.com> --- tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c index c571584c00f5..9ff0412e1fd3 100644 --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c @@ -309,6 +309,7 @@ static void v4_to_v6(struct sockaddr_storage *ss) v6->sin6_addr.s6_addr[10] = 0xff; v6->sin6_addr.s6_addr[11] = 0xff; memcpy(&v6->sin6_addr.s6_addr[12], &v4.sin_addr.s_addr, 4); + memset(&v6->sin6_addr.s6_addr[0], 0, 10); } static int udp_recv_send(int server_fd) -- 2.28.0.236.gb10cc79966-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] selftests/bpf: fix v4_to_v6 in sk_lookup 2020-08-07 22:38 [PATCH bpf] selftests/bpf: fix v4_to_v6 in sk_lookup Stanislav Fomichev @ 2020-08-08 18:46 ` Jakub Sitnicki 2020-08-10 16:14 ` Stanislav Fomichev 2020-08-11 13:43 ` Daniel Borkmann 1 sibling, 1 reply; 5+ messages in thread From: Jakub Sitnicki @ 2020-08-08 18:46 UTC (permalink / raw) To: Stanislav Fomichev; +Cc: netdev, bpf, davem, ast, daniel On Sat, Aug 08, 2020 at 12:38 AM CEST, Stanislav Fomichev wrote: > I'm getting some garbage in bytes 8 and 9 when doing conversion > from sockaddr_in to sockaddr_in6 (leftover from AF_INET?). > Let's explicitly clear the higher bytes. > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > --- > tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c > index c571584c00f5..9ff0412e1fd3 100644 > --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c > +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c > @@ -309,6 +309,7 @@ static void v4_to_v6(struct sockaddr_storage *ss) > v6->sin6_addr.s6_addr[10] = 0xff; > v6->sin6_addr.s6_addr[11] = 0xff; > memcpy(&v6->sin6_addr.s6_addr[12], &v4.sin_addr.s_addr, 4); > + memset(&v6->sin6_addr.s6_addr[0], 0, 10); > } > > static int udp_recv_send(int server_fd) That was badly written. Sorry about that. And thanks for the fix. I'd even zero out the whole thing: memset(v6, 0, sizeof(*v6)); ... because right now IPv4 address is left as sin6_flowinfo. I can follow up with that change, unless you'd like to roll a v2. Fixes: 0ab5539f8584 ("selftests/bpf: Tests for BPF_SK_LOOKUP attach point") Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] selftests/bpf: fix v4_to_v6 in sk_lookup 2020-08-08 18:46 ` Jakub Sitnicki @ 2020-08-10 16:14 ` Stanislav Fomichev 2020-08-10 20:30 ` Jakub Sitnicki 0 siblings, 1 reply; 5+ messages in thread From: Stanislav Fomichev @ 2020-08-10 16:14 UTC (permalink / raw) To: Jakub Sitnicki Cc: Netdev, bpf, David Miller, Alexei Starovoitov, Daniel Borkmann On Sat, Aug 8, 2020 at 11:46 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: > > On Sat, Aug 08, 2020 at 12:38 AM CEST, Stanislav Fomichev wrote: > > I'm getting some garbage in bytes 8 and 9 when doing conversion > > from sockaddr_in to sockaddr_in6 (leftover from AF_INET?). > > Let's explicitly clear the higher bytes. > > > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > --- > > tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c > > index c571584c00f5..9ff0412e1fd3 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c > > +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c > > @@ -309,6 +309,7 @@ static void v4_to_v6(struct sockaddr_storage *ss) > > v6->sin6_addr.s6_addr[10] = 0xff; > > v6->sin6_addr.s6_addr[11] = 0xff; > > memcpy(&v6->sin6_addr.s6_addr[12], &v4.sin_addr.s_addr, 4); > > + memset(&v6->sin6_addr.s6_addr[0], 0, 10); > > } > > > > static int udp_recv_send(int server_fd) > > That was badly written. Sorry about that. And thanks for the fix. > > I'd even zero out the whole thing: > > memset(v6, 0, sizeof(*v6)); > > ... because right now IPv4 address is left as sin6_flowinfo. I can > follow up with that change, unless you'd like to roll a v2. Up to you, but I'm not sure zeroing out the whole v6 portion is the best way forward. IMO, it's a bit confusing when reading the code. It will work, but only because v4 and v6 address portions don't really overlap :-/ I was thinking about adding new, on the stack sin6, fully initializing it and then doing memcpy into ss. But I decided that adding memset is probably good enough :-) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] selftests/bpf: fix v4_to_v6 in sk_lookup 2020-08-10 16:14 ` Stanislav Fomichev @ 2020-08-10 20:30 ` Jakub Sitnicki 0 siblings, 0 replies; 5+ messages in thread From: Jakub Sitnicki @ 2020-08-10 20:30 UTC (permalink / raw) To: Stanislav Fomichev Cc: Netdev, bpf, David Miller, Alexei Starovoitov, Daniel Borkmann On Mon, Aug 10, 2020 at 06:14 PM CEST, Stanislav Fomichev wrote: > On Sat, Aug 8, 2020 at 11:46 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: >> >> On Sat, Aug 08, 2020 at 12:38 AM CEST, Stanislav Fomichev wrote: >> > I'm getting some garbage in bytes 8 and 9 when doing conversion >> > from sockaddr_in to sockaddr_in6 (leftover from AF_INET?). >> > Let's explicitly clear the higher bytes. >> > >> > Signed-off-by: Stanislav Fomichev <sdf@google.com> >> > --- >> > tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c >> > index c571584c00f5..9ff0412e1fd3 100644 >> > --- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c >> > +++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c >> > @@ -309,6 +309,7 @@ static void v4_to_v6(struct sockaddr_storage *ss) >> > v6->sin6_addr.s6_addr[10] = 0xff; >> > v6->sin6_addr.s6_addr[11] = 0xff; >> > memcpy(&v6->sin6_addr.s6_addr[12], &v4.sin_addr.s_addr, 4); >> > + memset(&v6->sin6_addr.s6_addr[0], 0, 10); >> > } >> > >> > static int udp_recv_send(int server_fd) >> >> That was badly written. Sorry about that. And thanks for the fix. >> >> I'd even zero out the whole thing: >> >> memset(v6, 0, sizeof(*v6)); >> >> ... because right now IPv4 address is left as sin6_flowinfo. I can >> follow up with that change, unless you'd like to roll a v2. > Up to you, but I'm not sure zeroing out the whole v6 portion is the > best way forward. > IMO, it's a bit confusing when reading the code. > It will work, but only because v4 and v6 address portions don't really > overlap :-/ It's not that hacky :-) We copy sockaddr_in bits before overwriting ss: struct sockaddr_in v4 = *(struct sockaddr_in *)ss; It could be easier to read, perhaps by copying just the fields we need: struct sockaddr_in *v4 = (struct sockaddr_in *)ss; uint32_t addr = v4->sin_addr.saddr; in_port_t port = v4->sin_port; > I was thinking about adding new, on the stack sin6, fully initializing > it and then doing memcpy into ss. > But I decided that adding memset is probably good enough :-) Makes sense. Either way sounds good to me. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] selftests/bpf: fix v4_to_v6 in sk_lookup 2020-08-07 22:38 [PATCH bpf] selftests/bpf: fix v4_to_v6 in sk_lookup Stanislav Fomichev 2020-08-08 18:46 ` Jakub Sitnicki @ 2020-08-11 13:43 ` Daniel Borkmann 1 sibling, 0 replies; 5+ messages in thread From: Daniel Borkmann @ 2020-08-11 13:43 UTC (permalink / raw) To: Stanislav Fomichev, netdev, bpf; +Cc: davem, ast On 8/8/20 12:38 AM, Stanislav Fomichev wrote: > I'm getting some garbage in bytes 8 and 9 when doing conversion > from sockaddr_in to sockaddr_in6 (leftover from AF_INET?). > Let's explicitly clear the higher bytes. > > Signed-off-by: Stanislav Fomichev <sdf@google.com> Applied, thanks! ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-11 13:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-07 22:38 [PATCH bpf] selftests/bpf: fix v4_to_v6 in sk_lookup Stanislav Fomichev 2020-08-08 18:46 ` Jakub Sitnicki 2020-08-10 16:14 ` Stanislav Fomichev 2020-08-10 20:30 ` Jakub Sitnicki 2020-08-11 13:43 ` Daniel Borkmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).