netdev - Re: [PATCH net-next v2 1/6] net: bridge: Publish bridge accessor functions

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: <wihd0ykvdh1.fsf@dev-r-vrt-156.mtr.labs.mlnx>
Date:   Fri, 27 Apr 2018 19:08:42 +0300
From:   Petr Machata <petrm@...lanox.com>
To:     Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Cc:     Ido Schimmel <idosch@...lanox.com>, netdev@...r.kernel.org,
        bridge@...ts.linux-foundation.org, davem@...emloft.net,
        jiri@...lanox.com, stephen@...workplumber.org, mlxsw@...lanox.com
Subject: Re: [PATCH net-next v2 1/6] net: bridge: Publish bridge accessor functions

Nikolay Aleksandrov <nikolay@...ulusnetworks.com> writes:

> On 27/04/18 18:11, Ido Schimmel wrote:
>> From: Petr Machata <petrm@...lanox.com>
>> 
>> Add a couple new functions to allow querying FDB and vlan settings of a
>> bridge.
>> 
>> Signed-off-by: Petr Machata <petrm@...lanox.com>
>> Signed-off-by: Ido Schimmel <idosch@...lanox.com>
>> ---
>>  include/linux/if_bridge.h | 28 ++++++++++++++++++++++++++++
>>  net/bridge/br_fdb.c       | 22 ++++++++++++++++++++++
>>  net/bridge/br_private.h   | 11 +++++++++++
>>  net/bridge/br_vlan.c      | 39 +++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 100 insertions(+)
>> 
>
> Thanks! This looks good to me although the new exported helpers could've
> taken both bridge or port and return the result. Usually when adding a
> port-only functions we name them with nbp_ prefix instead of br_.
>
> Anyway this can be done later since the API is internal,

The idea is that the API would accept both ports and bridges, but
currently there's no user for the parts that I didn't code up--it would
be a dead code. It's super simple to extend these if/when there are
clients, e.g.:

modified   net/bridge/br_vlan.c
@@ -1155,7 +1155,10 @@ int br_vlan_pvid_rtnl(const struct net_device *dev, u16 *p_pvid)
 	struct net_bridge_vlan_group *vg;
 
 	ASSERT_RTNL();
-	if (netif_is_bridge_master(dev))
+	p = br_port_get_check_rtnl(dev);
+	if (p)
+		vg = nbp_vlan_group(p);
+	else if (netif_is_bridge_master(dev))
 		vg = br_vlan_group(netdev_priv(dev));
 	else
 		return -EINVAL;

I can post a follow-up with the renames if you prefer that.

Thanks,
Petr