netdev - Re: [PATCH net-next 1/5] net: bridge: provide shim definition for br_vlan

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: <96d38f44-61f0-d021-109c-f24f74251cd7@nvidia.com>
Date:   Thu, 28 Oct 2021 11:45:34 +0300
From:   Nikolay Aleksandrov <nikolay@...dia.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Roopa Prabhu <roopa@...dia.com>, Jiri Pirko <jiri@...dia.com>,
        Ido Schimmel <idosch@...dia.com>
Subject: Re: [PATCH net-next 1/5] net: bridge: provide shim definition for
 br_vlan_flags

On 27/10/2021 22:54, Vladimir Oltean wrote:
> On Wed, Oct 27, 2021 at 10:50:47PM +0300, Nikolay Aleksandrov wrote:
>> On 27/10/2021 22:45, Vladimir Oltean wrote:
>>> On Wed, Oct 27, 2021 at 10:28:12PM +0300, Nikolay Aleksandrov wrote:
>>>> On 27/10/2021 19:21, Vladimir Oltean wrote:
>>>>> br_vlan_replay() needs this, and we're preparing to move it to
>>>>> br_switchdev.c, which will be compiled regardless of whether or not
>>>>> CONFIG_BRIDGE_VLAN_FILTERING is enabled.
>>>>>
>>>>> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
>>>>> ---
>>>>>  net/bridge/br_private.h | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>>> index 3c9327628060..cc31c3fe1e02 100644
>>>>> --- a/net/bridge/br_private.h
>>>>> +++ b/net/bridge/br_private.h
>>>>> @@ -1708,6 +1708,11 @@ static inline bool br_vlan_can_enter_range(const struct net_bridge_vlan *v_curr,
>>>>>  	return true;
>>>>>  }
>>>>>  
>>>>> +static inline u16 br_vlan_flags(const struct net_bridge_vlan *v, u16 pvid)
>>>>> +{
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>>  static inline int br_vlan_replay(struct net_device *br_dev,
>>>>>  				 struct net_device *dev, const void *ctx,
>>>>>  				 bool adding, struct notifier_block *nb,
>>>>>
>>>>
>>>> hm, shouldn't the vlan replay be a shim if bridge vlans are not defined?
>>>> I.e. shouldn't this rather be turned into br_vlan_replay's shim?
>>>>
>>>> TBH, I haven't looked into the details just wonder why we would compile all that vlan
>>>> code if bridge vlan filtering is not enabled.
>>>
>>> The main reason is that I would like to avoid #ifdef if possible. If you
>>> have a strong opinion otherwise I can follow suit.
>>>
>>
>> Well, I see that we add ifdefs for IGMP, so I don't see a reason why not
>> to ifdef out the vlan replay in the same way too.
>>
>> I don't have a strong preference either way, end result is the same.
> 
> Since the caller and the callee are in the same C file, shimming out is
> not as clean as providing a static inline function definition with an
> empty body, and if I could avoid doing what I did for
> 
> br_mdb_replay()
> {
> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> 	<some other variables>;
> 	int err;
> 
> 	err = <body>;
> 
> 	if (err)
> 		return err;
> #endif
> 
> 	return 0;
> }
> 
> I'd do it. For br_vlan_replay() I could avoid it, so I left it at that.
> 

that's ok,
Acked-by: Nikolay Aleksandrov <nikolay@...dia.com>