All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Vlad Yasevich <vyasevic@redhat.com>
Cc: netdev@vger.kernel.org, bridge@lists.linux-foundation.org
Subject: Re: [Bridge] [PATCH net-next 1/2] bridge: Add flag to control mac learning.
Date: Mon, 29 Apr 2013 19:11:23 +0300	[thread overview]
Message-ID: <20130429161123.GB10663@redhat.com> (raw)
In-Reply-To: <1367251102-25193-2-git-send-email-vyasevic@redhat.com>

On Mon, Apr 29, 2013 at 11:58:21AM -0400, Vlad Yasevich wrote:
> Allow user to control whether mac learning is enabled on the port.
> By default, mac learning is enabled.  Disabling mac learning will
> cause new dynamic FDB entries to not be created for a particular port.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>  include/uapi/linux/if_link.h |    1 +
>  net/bridge/br_fdb.c          |    4 ++++
>  net/bridge/br_if.c           |    2 +-
>  net/bridge/br_netlink.c      |    6 +++++-
>  net/bridge/br_private.h      |    1 +
>  net/bridge/br_sysfs_if.c     |    2 ++
>  6 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index e316354..80fad7f 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -221,6 +221,7 @@ enum {
>  	IFLA_BRPORT_GUARD,	/* bpdu guard              */
>  	IFLA_BRPORT_PROTECT,	/* root port protection    */
>  	IFLA_BRPORT_FAST_LEAVE,	/* multicast fast leave    */
> +	IFLA_BRPORT_LEARNING,	/* mac learning */
>  	__IFLA_BRPORT_MAX
>  };
>  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index c581f12..9c4fe65 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -477,10 +477,14 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>  	} else {
>  		spin_lock(&br->hash_lock);
>  		if (likely(!fdb_find(head, addr, vid))) {
> +			if (!(source->flags & BR_LEARNING))
> +				goto unlock;
> +
>  			fdb = fdb_create(head, source, addr, vid);
>  			if (fdb)
>  				fdb_notify(br, fdb, RTM_NEWNEIGH);
>  		}
> +unlock:
>  		/* else  we lose race and someone else inserts
>  		 * it first, don't bother updating
>  		 */

Wait a second, this will affect adding fdbs using RTM_NEWNEIGH, won't it?
And let's not bother with lookups if learning is off.
So I imagine we need something like the below instead
(in addition to the sysfs/netlink part of the patch which
look fine):

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index bab338e..30eaa6b 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -448,8 +448,9 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
 	return ret;
 }
 
-void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
-		   const unsigned char *addr, u16 vid)
+static
+void __br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
+		     const unsigned char *addr, u16 vid)
 {
 	struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
 	struct net_bridge_fdb_entry *fdb;
@@ -490,6 +491,14 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 	}
 }
 
+void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
+		     const unsigned char *addr, u16 vid)
+{
+	if (!(source->flags & BR_LEARNING))
+		return;
+	__br_fdb_update(br, source, addr, vid);
+}
+
 static int fdb_to_nud(const struct net_bridge_fdb_entry *fdb)
 {
 	if (fdb->is_local)
@@ -655,7 +664,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge_port *p,
 
 	if (ndm->ndm_flags & NTF_USE) {
 		rcu_read_lock();
-		br_fdb_update(p->br, p, addr, vid);
+		__br_fdb_update(p->br, p, addr, vid);
 		rcu_read_unlock();
 	} else {
 		spin_lock_bh(&p->br->hash_lock);
 


WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Vlad Yasevich <vyasevic@redhat.com>
Cc: netdev@vger.kernel.org, bridge@lists.linux-foundation.org
Subject: Re: [PATCH net-next 1/2] bridge: Add flag to control mac learning.
Date: Mon, 29 Apr 2013 19:11:23 +0300	[thread overview]
Message-ID: <20130429161123.GB10663@redhat.com> (raw)
In-Reply-To: <1367251102-25193-2-git-send-email-vyasevic@redhat.com>

On Mon, Apr 29, 2013 at 11:58:21AM -0400, Vlad Yasevich wrote:
> Allow user to control whether mac learning is enabled on the port.
> By default, mac learning is enabled.  Disabling mac learning will
> cause new dynamic FDB entries to not be created for a particular port.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>  include/uapi/linux/if_link.h |    1 +
>  net/bridge/br_fdb.c          |    4 ++++
>  net/bridge/br_if.c           |    2 +-
>  net/bridge/br_netlink.c      |    6 +++++-
>  net/bridge/br_private.h      |    1 +
>  net/bridge/br_sysfs_if.c     |    2 ++
>  6 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index e316354..80fad7f 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -221,6 +221,7 @@ enum {
>  	IFLA_BRPORT_GUARD,	/* bpdu guard              */
>  	IFLA_BRPORT_PROTECT,	/* root port protection    */
>  	IFLA_BRPORT_FAST_LEAVE,	/* multicast fast leave    */
> +	IFLA_BRPORT_LEARNING,	/* mac learning */
>  	__IFLA_BRPORT_MAX
>  };
>  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index c581f12..9c4fe65 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -477,10 +477,14 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>  	} else {
>  		spin_lock(&br->hash_lock);
>  		if (likely(!fdb_find(head, addr, vid))) {
> +			if (!(source->flags & BR_LEARNING))
> +				goto unlock;
> +
>  			fdb = fdb_create(head, source, addr, vid);
>  			if (fdb)
>  				fdb_notify(br, fdb, RTM_NEWNEIGH);
>  		}
> +unlock:
>  		/* else  we lose race and someone else inserts
>  		 * it first, don't bother updating
>  		 */

Wait a second, this will affect adding fdbs using RTM_NEWNEIGH, won't it?
And let's not bother with lookups if learning is off.
So I imagine we need something like the below instead
(in addition to the sysfs/netlink part of the patch which
look fine):

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index bab338e..30eaa6b 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -448,8 +448,9 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
 	return ret;
 }
 
-void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
-		   const unsigned char *addr, u16 vid)
+static
+void __br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
+		     const unsigned char *addr, u16 vid)
 {
 	struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
 	struct net_bridge_fdb_entry *fdb;
@@ -490,6 +491,14 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
 	}
 }
 
+void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
+		     const unsigned char *addr, u16 vid)
+{
+	if (!(source->flags & BR_LEARNING))
+		return;
+	__br_fdb_update(br, source, addr, vid);
+}
+
 static int fdb_to_nud(const struct net_bridge_fdb_entry *fdb)
 {
 	if (fdb->is_local)
@@ -655,7 +664,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge_port *p,
 
 	if (ndm->ndm_flags & NTF_USE) {
 		rcu_read_lock();
-		br_fdb_update(p->br, p, addr, vid);
+		__br_fdb_update(p->br, p, addr, vid);
 		rcu_read_unlock();
 	} else {
 		spin_lock_bh(&p->br->hash_lock);
 

  reply	other threads:[~2013-04-29 16:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-29 15:58 [Bridge] [PATCH net-next 0/2] Add some new flags to bridges Vlad Yasevich
2013-04-29 15:58 ` Vlad Yasevich
2013-04-29 15:58 ` [Bridge] [PATCH net-next 1/2] bridge: Add flag to control mac learning Vlad Yasevich
2013-04-29 15:58   ` Vlad Yasevich
2013-04-29 16:11   ` Michael S. Tsirkin [this message]
2013-04-29 16:11     ` Michael S. Tsirkin
2013-04-29 16:33     ` [Bridge] " Vlad Yasevich
2013-04-29 16:33       ` Vlad Yasevich
2013-04-29 15:58 ` [Bridge] [PATCH net-next 2/2] bridge: Add a flag to control unicast packet flood Vlad Yasevich
2013-04-29 15:58   ` Vlad Yasevich
2013-04-29 16:12   ` [Bridge] " Michael S. Tsirkin
2013-04-29 16:12     ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130429161123.GB10663@redhat.com \
    --to=mst@redhat.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=netdev@vger.kernel.org \
    --cc=vyasevic@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.