netdev - Re: [PATCH net-next] bridge: mrp: Implement LC mode for MRP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201123140519.3bb3db16@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 23 Nov 2020 14:05:19 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Nikolay Aleksandrov <nikolay@...dia.com>
Cc:     Horatiu Vultur <horatiu.vultur@...rochip.com>, <roopa@...dia.com>,
        <davem@...emloft.net>, <linux-kernel@...r.kernel.org>,
        <bridge@...ts.linux-foundation.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] bridge: mrp: Implement LC mode for MRP

On Mon, 23 Nov 2020 16:25:53 +0200 Nikolay Aleksandrov wrote:
> >>> @@ -156,4 +157,10 @@ struct br_mrp_in_link_hdr {
> >>>       __be16 interval;
> >>>  };
> >>>
> >>> +struct br_mrp_in_link_status_hdr {
> >>> +     __u8 sa[ETH_ALEN];
> >>> +     __be16 port_role;
> >>> +     __be16 id;
> >>> +};
> >>> +  
> >>
> >> I didn't see this struct used anywhere, am I missing anything?  
> > 
> > Yes, you are right, the struct is not used any. But I put it there as I
> > put the other frame types for MRP.
> >   
> 
> I see, we don't usually add unused code. The patch is fine as-is and since
> this is already the case for other MRP parts I'm not strictly against it, so:
> 
> Acked-by: Nikolay Aleksandrov <nikolay@...dia.com>
> 
> If Jakub decides to adhere to that rule you can keep my acked-by and just remove
> the struct for v2.

Yes, good catch, let's drop it, we don't want to make too much of 
a precedent for using kernel uAPI headers as a place to provide
protocol-related structs if the kernel doesn't need them.

The existing structs are only present in net-next as well, so if you
don't mind Horatiu it'd be good to follow up and remove the unused ones
and move the ones (if any) which are only used by the kernel but not by
the user space <-> kernel API communication out of include/uapi.