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@xxxxxxxxxxxxxx> 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@xxxxxxxxxx> >> > --- >> > 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.
