From mboxrd@z Thu Jan  1 00:00:00 1970
From: Petr Machata <petrm@mellanox.com>
Subject: Re: [PATCH net-next v2 1/6] net: bridge: Publish bridge accessor functions
Date: Fri, 27 Apr 2018 19:36:45 +0300
Message-ID: <wihy3h8txlu.fsf@dev-r-vrt-156.mtr.labs.mlnx>
References: <20180427151111.22099-1-idosch@mellanox.com>
        <20180427151111.22099-2-idosch@mellanox.com>
        <20180427092829.1673ed74@xeon-e3>
Mime-Version: 1.0
Content-Type: text/plain
Cc: Ido Schimmel <idosch@mellanox.com>, netdev@vger.kernel.org,
        bridge@lists.linux-foundation.org, davem@davemloft.net,
        jiri@mellanox.com, nikolay@cumulusnetworks.com, mlxsw@mellanox.com
To: Stephen Hemminger <stephen@networkplumber.org>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-he1eur01on0068.outbound.protection.outlook.com ([104.47.0.68]:2176
        "EHLO EUR01-HE1-obe.outbound.protection.outlook.com"
        rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
        id S1758696AbeD0QhL (ORCPT <rfc822;netdev@vger.kernel.org>);
        Fri, 27 Apr 2018 12:37:11 -0400
In-Reply-To: <20180427092829.1673ed74@xeon-e3> (Stephen Hemminger's message of
        "Fri, 27 Apr 2018 09:28:29 -0700")
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Stephen Hemminger <stephen@networkplumber.org> writes:

> On Fri, 27 Apr 2018 18:11:06 +0300
> Ido Schimmel <idosch@mellanox.com> wrote:
>
>> +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))
>> +		vg = br_vlan_group(netdev_priv(dev));
>> +	else
>> +		return -EINVAL;
>> +
>> +	*p_pvid = br_get_pvid(vg);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(br_vlan_pvid_rtnl);
>
> Unless there is a RCU version, no need for _rtnl suffix.

All right, I wanted to be explicit, but if you prefer to drop the _rtnl,
no problem.

> Minor style issue, why not make error then go on and return -1 on error.

Basically to make it more straightforward to extend when people want to
add support for the other device kind (bridge / port).

>
>
> int br_vlan_pvid(const struct net_device *dev)
> {
> 	const struct net_bridge_vlan_group *vg;
>
> 	ASSERT_RTNL();
>
> 	if (!netif_is_bridge_master(dev))
> 		return -1;
>
> 	vg = br_vlan_group(netdev_priv(dev));
> 	return br_get_pvid(vg);
> }