Re: Annoying gcc / rdma / networking warnings

[Jason Gunthorpe]

On Sat, May 11, 2019 at 12:52:06PM -0400, Linus Torvalds wrote:
> Jason and Davem,
>  with gcc-9, I'm now seeing a number of annoying warnings from the
> rdma layer. I think it depends on the exact gcc version, because I'm
> seeing them on my laptop but didn't see them on my desktop, probably
> due to updating at different times.

I can see them too on a latest FC30 compiler.. It is pretty amazing
FC30 shipped this month with a compiler that was just released 10 days
ago. :)

Also a lot of of 'taking address of a packed member' warnings in RDMA
code for structs that probably should be aligned(4) not packed. I'll
have to look at those more carefully this week and see what can be
done.

> So if you look at the types like gcc does, then the rdma layer really
> is passing a pointer to a 16-byte sockaddr, and then filling it with
> (much bigger) sockaddr_ip6 data.

It looks like gcc is assuming that since we started with the _sockaddr
union member that the memory is actually bounded to that specific
member. This doesn't seem unreasonable and matches a lot of uses of
unions. However I wonder how that sort of analysis is going to work in
the kernel, considering our container_of idiom breaks it in the same
way, but maybe that is special case'd..

So if we tell gcc the sockaddr memory is actually the whole union then
it becomes happy, see below.

> So David, arguably the kernel "struct sockaddr" is simply wrong, if it
> can't contain a "struct sockaddr_in6". No? Is extending it a huge
> problem for other users that don't need it (mainly stack, I assume..)?

We have sockaddr_storage for this, and arguably this code is just
over-optimizing to save a few bytes of stack vs sockaddr_storage..
 
> Also equally arguably, the rdma code could just use a "struct
> sockaddr_in6 for this use and avoid the gcc issue, couldn't it? 

I think the specific sockaddr types should only ever be used if we
*know* the sa_family is that type. If the sa_family is not known then
it should be sockaddr or sockaddr_storage. Otherwise things get very
confusing.

When using sockaddr_storage code always has the cast to sockaddr
anyhow, as it is not a union, so this jaunty cast is not out of place
in sockets code.

Below silences the warning, and the warning continues to work in other
cases, ie if I remove _sockaddr_in6 from the union I do get compile
warnings about out of bound references.

Jason