diff mbox

[net-next] bridge: allow adding of fdb entries pointing to the bridge device

Message ID 1444144600-26541-1-git-send-email-roopa@cumulusnetworks.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Roopa Prabhu Oct. 6, 2015, 3:16 p.m. UTC 
From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch enables adding of fdb entries pointing to the bridge device.
This can be used to propagate mac address of vlan interfaces
configured on top of the vlan filtering bridge.

Before:
$bridge fdb add 44:38:39:00:27:9f dev bridge
RTNETLINK answers: Invalid argument

After:
$bridge fdb add 44:38:39:00:27:9f dev bridge

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 net/bridge/br_fdb.c | 106 ++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 83 insertions(+), 23 deletions(-)

Comments

kernel test robot Oct. 6, 2015, 3:29 p.m. UTC | #1 
Hi Roopa,

[auto build test WARNING on next-20151006 -- if it's inappropriate base, please ignore]

config: sh-titan_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sh 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   net/bridge/br_fdb.c: In function 'br_fdb_add':
>> net/bridge/br_fdb.c:784:13: warning: 'p' may be used uninitialized in this function [-Wuninitialized]
   net/bridge/br_fdb.c:850:26: note: 'p' was declared here
   net/bridge/br_fdb.c: In function 'br_fdb_delete':
   net/bridge/br_fdb.c:952:11: warning: 'p' may be used uninitialized in this function [-Wuninitialized]
   net/bridge/br_fdb.c:977:26: note: 'p' was declared here

vim +/p +784 net/bridge/br_fdb.c

eb8d7baa Wilson Kok        2015-05-25  768  
2ba071ec Vlad Yasevich     2013-02-13  769  	fdb = fdb_find(head, addr, vid);
64af1bac stephen hemminger 2011-09-30  770  	if (fdb == NULL) {
64af1bac stephen hemminger 2011-09-30  771  		if (!(flags & NLM_F_CREATE))
64af1bac stephen hemminger 2011-09-30  772  			return -ENOENT;
36fd2b63 stephen hemminger 2011-04-04  773  
2ba071ec Vlad Yasevich     2013-02-13  774  		fdb = fdb_create(head, source, addr, vid);
36fd2b63 stephen hemminger 2011-04-04  775  		if (!fdb)
36fd2b63 stephen hemminger 2011-04-04  776  			return -ENOMEM;
b0a397fb roopa             2013-04-22  777  
b0a397fb roopa             2013-04-22  778  		modified = true;
64af1bac stephen hemminger 2011-09-30  779  	} else {
64af1bac stephen hemminger 2011-09-30  780  		if (flags & NLM_F_EXCL)
64af1bac stephen hemminger 2011-09-30  781  			return -EEXIST;
b0a397fb roopa             2013-04-22  782  
b0a397fb roopa             2013-04-22  783  		if (fdb->dst != source) {
b0a397fb roopa             2013-04-22 @784  			fdb->dst = source;
b0a397fb roopa             2013-04-22  785  			modified = true;
b0a397fb roopa             2013-04-22  786  		}
64af1bac stephen hemminger 2011-09-30  787  	}
36fd2b63 stephen hemminger 2011-04-04  788  
0e29720e Roopa Prabhu      2015-10-06  789  	if (fdb_to_nud(br, fdb) != state) {
145beee8 Vlad Yasevich     2014-05-16  790  		if (state & NUD_PERMANENT) {
145beee8 Vlad Yasevich     2014-05-16  791  			fdb->is_local = 1;
145beee8 Vlad Yasevich     2014-05-16  792  			if (!fdb->is_static) {

:::::: The code at line 784 was first introduced by commit
:::::: b0a397fb352e65e3b6501dca9662617a18862ef1 bridge: Add fdb dst check during fdb update

:::::: TO: roopa <roopa@cumulusnetworks.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Roopa Prabhu Oct. 6, 2015, 3:55 p.m. UTC | #2 
On 10/6/15, 8:29 AM, kbuild test robot wrote:
> Hi Roopa,
>
> [auto build test WARNING on next-20151006 -- if it's inappropriate base, please ignore]
>
> config: sh-titan_defconfig (attached as .config)
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=sh 
>
> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
> http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
>
> All warnings (new ones prefixed by >>):
>
>    net/bridge/br_fdb.c: In function 'br_fdb_add':
>>> net/bridge/br_fdb.c:784:13: warning: 'p' may be used uninitialized in this function [-Wuninitialized]
>    net/bridge/br_fdb.c:850:26: note: 'p' was declared here
>    net/bridge/br_fdb.c: In function 'br_fdb_delete':
>    net/bridge/br_fdb.c:952:11: warning: 'p' may be used uninitialized in this function [-Wuninitialized]
>    net/bridge/br_fdb.c:977:26: note: 'p' was declared here
>
> vim +/p +784 net/bridge/br_fdb.c
>
> eb8d7baa Wilson Kok        2015-05-25  768  
> 2ba071ec Vlad Yasevich     2013-02-13  769  	fdb = fdb_find(head, addr, vid);
> 64af1bac stephen hemminger 2011-09-30  770  	if (fdb == NULL) {
> 64af1bac stephen hemminger 2011-09-30  771  		if (!(flags & NLM_F_CREATE))
> 64af1bac stephen hemminger 2011-09-30  772  			return -ENOENT;
> 36fd2b63 stephen hemminger 2011-04-04  773  
> 2ba071ec Vlad Yasevich     2013-02-13  774  		fdb = fdb_create(head, source, addr, vid);
> 36fd2b63 stephen hemminger 2011-04-04  775  		if (!fdb)
> 36fd2b63 stephen hemminger 2011-04-04  776  			return -ENOMEM;
> b0a397fb roopa             2013-04-22  777  
> b0a397fb roopa             2013-04-22  778  		modified = true;
> 64af1bac stephen hemminger 2011-09-30  779  	} else {
> 64af1bac stephen hemminger 2011-09-30  780  		if (flags & NLM_F_EXCL)
> 64af1bac stephen hemminger 2011-09-30  781  			return -EEXIST;
> b0a397fb roopa             2013-04-22  782  
> b0a397fb roopa             2013-04-22  783  		if (fdb->dst != source) {
> b0a397fb roopa             2013-04-22 @784  			fdb->dst = source;
> b0a397fb roopa             2013-04-22  785  			modified = true;
> b0a397fb roopa             2013-04-22  786  		}
> 64af1bac stephen hemminger 2011-09-30  787  	}
> 36fd2b63 stephen hemminger 2011-04-04  788  
> 0e29720e Roopa Prabhu      2015-10-06  789  	if (fdb_to_nud(br, fdb) != state) {
> 145beee8 Vlad Yasevich     2014-05-16  790  		if (state & NUD_PERMANENT) {
> 145beee8 Vlad Yasevich     2014-05-16  791  			fdb->is_local = 1;
> 145beee8 Vlad Yasevich     2014-05-16  792  			if (!fdb->is_static) {
>
>
It is not used when it is not initialized. But I will resubmit the patch by initializing it to NULL (thanks nikolay for the suggestion).

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 7f7d551..5d0f6f9 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -608,13 +608,14 @@  void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 	}
 }
 
-static int fdb_to_nud(const struct net_bridge_fdb_entry *fdb)
+static int fdb_to_nud(const struct net_bridge *br,
+		      const struct net_bridge_fdb_entry *fdb)
 {
 	if (fdb->is_local)
 		return NUD_PERMANENT;
 	else if (fdb->is_static)
 		return NUD_NOARP;
-	else if (has_expired(fdb->dst->br, fdb))
+	else if (has_expired(br, fdb))
 		return NUD_STALE;
 	else
 		return NUD_REACHABLE;
@@ -640,7 +641,7 @@  static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
 	ndm->ndm_flags	 = fdb->added_by_external_learn ? NTF_EXT_LEARNED : 0;
 	ndm->ndm_type	 = 0;
 	ndm->ndm_ifindex = fdb->dst ? fdb->dst->dev->ifindex : br->dev->ifindex;
-	ndm->ndm_state   = fdb_to_nud(fdb);
+	ndm->ndm_state   = fdb_to_nud(br, fdb);
 
 	if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->addr))
 		goto nla_put_failure;
@@ -785,7 +786,7 @@  static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
 		}
 	}
 
-	if (fdb_to_nud(fdb) != state) {
+	if (fdb_to_nud(br, fdb) != state) {
 		if (state & NUD_PERMANENT) {
 			fdb->is_local = 1;
 			if (!fdb->is_static) {
@@ -848,6 +849,7 @@  int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 	struct net_bridge_vlan_group *vg;
 	struct net_bridge_port *p;
 	struct net_bridge_vlan *v;
+	struct net_bridge *br = NULL;
 	int err = 0;
 
 	if (!(ndm->ndm_state & (NUD_PERMANENT|NUD_NOARP|NUD_REACHABLE))) {
@@ -860,14 +862,19 @@  int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 		return -EINVAL;
 	}
 
-	p = br_port_get_rtnl(dev);
-	if (p == NULL) {
-		pr_info("bridge: RTM_NEWNEIGH %s not a bridge port\n",
-			dev->name);
-		return -EINVAL;
+	if (dev->priv_flags & IFF_EBRIDGE) {
+		br = netdev_priv(dev);
+		vg = br_vlan_group(br);
+	} else {
+		p = br_port_get_rtnl(dev);
+		if (!p) {
+			pr_info("bridge: RTM_NEWNEIGH %s not a bridge port\n",
+				dev->name);
+			return -EINVAL;
+		}
+		vg = nbp_vlan_group(p);
 	}
 
-	vg = nbp_vlan_group(p);
 	if (vid) {
 		v = br_vlan_find(vg, vid);
 		if (!v) {
@@ -877,9 +884,15 @@  int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 		}
 
 		/* VID was specified, so use it. */
-		err = __br_fdb_add(ndm, p, addr, nlh_flags, vid);
+		if (dev->priv_flags & IFF_EBRIDGE)
+			err = br_fdb_insert(br, NULL, addr, vid);
+		else
+			err = __br_fdb_add(ndm, p, addr, nlh_flags, vid);
 	} else {
-		err = __br_fdb_add(ndm, p, addr, nlh_flags, 0);
+		if (dev->priv_flags & IFF_EBRIDGE)
+			err = br_fdb_insert(br, NULL, addr, 0);
+		else
+			err = __br_fdb_add(ndm, p, addr, nlh_flags, 0);
 		if (err || !vg || !vg->num_vlans)
 			goto out;
 
@@ -888,7 +901,11 @@  int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 		 * vlan on this port.
 		 */
 		list_for_each_entry(v, &vg->vlan_list, vlist) {
-			err = __br_fdb_add(ndm, p, addr, nlh_flags, v->vid);
+			if (dev->priv_flags & IFF_EBRIDGE)
+				err = br_fdb_insert(br, NULL, addr, v->vid);
+			else
+				err = __br_fdb_add(ndm, p, addr, nlh_flags,
+						   v->vid);
 			if (err)
 				goto out;
 		}
@@ -898,6 +915,32 @@  out:
 	return err;
 }
 
+static int fdb_delete_by_addr(struct net_bridge *br, const u8 *addr,
+			      u16 vid)
+{
+	struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
+	struct net_bridge_fdb_entry *fdb;
+
+	fdb = fdb_find(head, addr, vid);
+	if (!fdb)
+		return -ENOENT;
+
+	fdb_delete(br, fdb);
+	return 0;
+}
+
+static int __br_fdb_delete_by_addr(struct net_bridge *br,
+				   const unsigned char *addr, u16 vid)
+{
+	int err;
+
+	spin_lock_bh(&br->hash_lock);
+	err = fdb_delete_by_addr(br, addr, vid);
+	spin_unlock_bh(&br->hash_lock);
+
+	return err;
+}
+
 static int fdb_delete_by_addr_and_port(struct net_bridge_port *p,
 				       const u8 *addr, u16 vlan)
 {
@@ -933,16 +976,22 @@  int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
 	struct net_bridge_vlan_group *vg;
 	struct net_bridge_port *p;
 	struct net_bridge_vlan *v;
+	struct net_bridge *br;
 	int err;
 
-	p = br_port_get_rtnl(dev);
-	if (p == NULL) {
-		pr_info("bridge: RTM_DELNEIGH %s not a bridge port\n",
-			dev->name);
-		return -EINVAL;
+	if (dev->priv_flags & IFF_EBRIDGE) {
+		br = netdev_priv(dev);
+		vg = br_vlan_group(br);
+	} else {
+		p = br_port_get_rtnl(dev);
+		if (!p) {
+			pr_info("bridge: RTM_DELNEIGH %s not a bridge port\n",
+				dev->name);
+			return -EINVAL;
+		}
+		vg = nbp_vlan_group(p);
 	}
 
-	vg = nbp_vlan_group(p);
 	if (vid) {
 		v = br_vlan_find(vg, vid);
 		if (!v) {
@@ -951,15 +1000,26 @@  int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
 			return -EINVAL;
 		}
 
-		err = __br_fdb_delete(p, addr, vid);
+		if (dev->priv_flags & IFF_EBRIDGE)
+			err = __br_fdb_delete_by_addr(br, addr, vid);
+		else
+			err = __br_fdb_delete(p, addr, vid);
 	} else {
 		err = -ENOENT;
-		err &= __br_fdb_delete(p, addr, 0);
+		if (dev->priv_flags & IFF_EBRIDGE)
+			err = __br_fdb_delete_by_addr(br, addr, 0);
+		else
+			err &= __br_fdb_delete(p, addr, 0);
+
 		if (!vg || !vg->num_vlans)
 			goto out;
 
-		list_for_each_entry(v, &vg->vlan_list, vlist)
-			err &= __br_fdb_delete(p, addr, v->vid);
+		list_for_each_entry(v, &vg->vlan_list, vlist) {
+			if (dev->priv_flags & IFF_EBRIDGE)
+				err = __br_fdb_delete_by_addr(br, addr, v->vid);
+			else
+				err &= __br_fdb_delete(p, addr, v->vid);
+		}
 	}
 out:
 	return err;