lkml.org 
[lkml]   [2009]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/4] net: bridge: use device address list instead of dev_addr
    On Mon, 13 Apr 2009 10:44:08 +0200
    Jiri Pirko <jpirko@redhat.com> wrote:

    > This patch changes the handling of mac addresses of bridge port devices. Now
    > it uses previously introduced list of device addresses. It allows the bridge to
    > know more then one local mac address per port which is mandatory for the right
    > work in some cases.
    >
    > Signed-off-by: Jiri Pirko <jpirko@redhat.com>
    > ---
    > net/bridge/br_fdb.c | 120 +++++++++++++++++++++++++++++++++--------------
    > net/bridge/br_if.c | 2 +-
    > net/bridge/br_notify.c | 2 +-
    > net/bridge/br_private.h | 4 +-
    > 4 files changed, 89 insertions(+), 39 deletions(-)
    >
    > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
    > index a48f5ef..6efc556 100644
    > --- a/net/bridge/br_fdb.c
    > +++ b/net/bridge/br_fdb.c
    > @@ -77,10 +77,45 @@ static inline void fdb_delete(struct net_bridge_fdb_entry *f)
    > br_fdb_put(f);
    > }
    >
    > -void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr)
    > +/*
    > + * Finds out if passed address is one of the addresses assigned to the device.
    > + * Returns 1 on positive result
    > + */
    > +static inline int is_dev_addr(struct net_device *dev, unsigned char *addr)

    Why not a general version in net_device.h or etherdevice.h?

    static inline bool is_etherdev_addr(const struct net_device *dev, const unsigned char addr[ETH_ALEN])

    > +{
    > + struct hw_addr *ha;
    > + int ret = 1;
    > +
    > + netif_dev_addr_lock_bh(dev);
    > + for_each_dev_addr(dev, ha) {
    User RCU

    > + ret = compare_ether_addr(addr, ha->addr);
    > + if (!ret)
    > + break;
    > + }
    > + netif_dev_addr_unlock_bh(dev);
    > + return !ret ? 1 : 0;
    > +}
    > +
    > +static int another_port_has_addr(const struct net_bridge_port *p,
    > + struct net_bridge_fdb_entry *f)
    > +{
    > + struct net_bridge *br = p->br;
    > + struct net_bridge_port *op;
    > +
    > + list_for_each_entry(op, &br->port_list, list) {
    > + if (op != p && is_dev_addr(op->dev, f->addr.addr)) {
    > + f->dst = op;
    > + return 1;
    > + }
    > + }
    > + return 0;
    > +}

    Forwarding database is hot path, people sometimes run lots of devices
    on single bridge, doesn't this scale worse?

    > +void br_fdb_changeaddr(struct net_bridge_port *p, struct net_device *dev)
    > {
    > struct net_bridge *br = p->br;
    > int i;
    > + struct hw_addr *ha;
    >
    > spin_lock_bh(&br->hash_lock);
    >
    > @@ -92,26 +127,23 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr)
    >
    > f = hlist_entry(h, struct net_bridge_fdb_entry, hlist);
    > if (f->dst == p && f->is_local) {
    > - /* maybe another port has same hw addr? */
    > - struct net_bridge_port *op;
    > - list_for_each_entry(op, &br->port_list, list) {
    > - if (op != p &&
    > - !compare_ether_addr(op->dev->dev_addr,
    > - f->addr.addr)) {
    > - f->dst = op;
    > - goto insert;
    > - }
    > - }
    > -
    > - /* delete old one */
    > - fdb_delete(f);
    > - goto insert;
    > + /*
    > + * maybe another port has same hw addr?,
    > + * if not then delete it
    > + */
    > + if (!another_port_has_addr(p, f))
    > + fdb_delete(f);
    > }
    > }
    > }
    > - insert:
    > - /* insert new address, may fail if invalid address or dup. */
    > - fdb_insert(br, p, newaddr);
    > +
    > + /* insert device addresses, may fail if invalid address. */
    > +
    > + netif_dev_addr_lock_bh(dev);
    > + for_each_dev_addr(dev, ha) {
    > + fdb_insert(br, p, ha->addr);
    > + }
    > + netif_dev_addr_unlock_bh(dev);
    >

    You added another layer of locking on the already hot bridge
    fast path.


    \
     
     \ /
      Last update: 2009-04-13 16:57    [W:0.029 / U:7.792 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site