Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets

From: Cyrill Gorcunov
Date: Wed Sep 28 2016 - 07:27:22 EST


On Wed, Sep 28, 2016 at 07:06:26AM -0400, Jamal Hadi Salim wrote:
> >
> > This structure is uapi, so anyone has complete rights to reference
> > @pad in the userspace programs. Sure it would be more clear to remove
> > the @pad completely, but if we choose so I think it's better to do
> > on top instead and then if someone complain we can easily revert
> > the single trivial commit instead of this big patch.
>
> I am conflicted.
> A field labelled "pad" does not appear to be valid as "UAPI". It is
> a cosmetic indicator. If you did sizeof() with or without it being
> present the value doesnt change.

I think you miss the point what I'm trying to say: currently end-user
may have reference to this member (for any reason) and his program
will compile and run. If we change the name the compilation procedure
fails and this will break API. Yes, referrning @pad is bad idea for
userspace code, and yes (!) better to simply rename it but lets do
that later, on top, so that if we break something in userspace
we could easily revert the oneline change.

> BTW: There is at least one major structure in inet diag has a hole
> today and doesnt have a padding indicator.
>
> > If protocol goes over u8 then complete inet_diag_req_v2 structure will
> > have to be reworked becaue @sdiag_protocol is u8 as well. IOW, once
> > someone liftup IPPROTO_MAX > 255, he will notice the problem immediately
> > because diag for such module simply stop working properly.
> >
>
> ok.
>
> cheers,
> jamal
>

Cyrill