| Message ID | 20130429161123.GB10663@redhat.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
On 04/29/2013 12:11 PM, Michael S. Tsirkin wrote: > 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? Hm.. Yes, I missed this. But this has got me thinking of whether it is correct to allow 'creation' of an entry if NTF_USE is set. Consider, that the current behavior is rather broken in that if someone sets NLM_F_CREATE|NLM_F_EXCL and NTF_USE, we will update the FDB (which is technically wrong since we ignore the exclusivity). Also, if someone doesn't specify NLM_F_CREATE, we create anyway. I think that may be I'll fix the above inconsistencies first. -vlad > 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); > > -- 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 --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);