lkml.org 
[lkml]   [2009]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patches in this message
    /
    Date
    From
    SubjectRe: [PATCH] bonding: allow bond in mode balance-alb to work properly in bridge
    On Sat, 14 Mar 2009 10:49:11 +0100
    Jiri Pirko <jpirko@redhat.com> wrote:

    > Sat, Mar 14, 2009 at 06:39:32AM CET, shemminger@linux-foundation.org wrote:
    > >On Fri, 13 Mar 2009 19:33:04 +0100
    > >Jiri Pirko <jpirko@redhat.com> wrote:
    > >
    > >> Hi all.
    > >>
    > >> This is only a draft of patch to consult. I'm aware that it should be divided
    > >> into multiple patches. I want to know opinion from you folks.
    > >>
    > >> The problem is described in following bugzilla:
    > >> https://bugzilla.redhat.com/show_bug.cgi?id=487763
    > >>
    > >> Basically here's what's going on. In every mode, bonding interface uses the same
    > >> mac address for all enslaved devices. Except for mode balance-alb. When you put
    > >> this kind of bond device into a bridge it will only add one of mac adresses into
    > >> a hash list of mac addresses, say X. This mac address is marked as local. But
    > >> this bonding interface also has mac address Y. Now then packet arrives with
    > >> destination address Y, this address is not marked as local and the packed looks
    > >> like it needs to be forwarded. This packet is then lost which is wrong.
    > >>
    > >> Notice that interfaces can be added and removed from bond while it is in bridge.
    > >> Therefore I introduce another function pointer in struct net_device_ops -
    > >> ndo_check_mac_address. This function when it's implemented should check passed
    > >> mac address against the one set in device. I'm using this in bonding driver when
    > >> the bond is in mode balance-alb to walk thru all slaves and checking if any of
    > >> them equals passed address.
    > >>
    > >> Then in bridge function br_handle_frame_finish() I'm using ndo_check_mac_address
    > >> to recognize the destination mac address as local.
    > >>
    > >> Please look at this and tell me what you think about it.
    > >>
    > >> Thanks
    > >>
    > >> Jirka
    > >>
    > >
    > >A better and more general way to do this have the dev_set_mac_address
    > >function check the return of the notifier and unwind. Then any protocol
    > >can easily prevent address from changing.
    >
    > Can you please describe this thougth a bit more? I can't understand it now...
    >
    > Thanks
    >
    > Jirka

    Something like this:

    --- a/net/core/dev.c 2009-03-15 15:55:02.098126056 -0700
    +++ b/net/core/dev.c 2009-03-15 16:02:43.999251305 -0700
    @@ -3830,6 +3830,7 @@ int dev_set_mac_address(struct net_devic
    {
    const struct net_device_ops *ops = dev->netdev_ops;
    int err;
    + char save_addr[MAX_ADDR_LEN];

    if (!ops->ndo_set_mac_address)
    return -EOPNOTSUPP;
    @@ -3837,9 +3838,17 @@ int dev_set_mac_address(struct net_devic
    return -EINVAL;
    if (!netif_device_present(dev))
    return -ENODEV;
    +
    + memcpy(save_addr, dev->dev_addr, dev->addr_len);
    err = ops->ndo_set_mac_address(dev, sa);
    - if (!err)
    - call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
    + if (err)
    + return err;
    +
    + err = call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
    + if (err) {
    + memcpy(sa->sa_data, save_addr, dev->addr_len);
    + ops->ndo_set_mac_address(dev, sa);
    + }
    return err;
    }

    And something like this:

    --- a/drivers/net/bonding/bond_main.c 2009-03-15 16:03:53.909000973 -0700
    +++ b/drivers/net/bonding/bond_main.c 2009-03-15 16:11:43.227127031 -0700
    @@ -3534,6 +3534,7 @@ static int bond_slave_netdev_event(unsig
    {
    struct net_device *bond_dev = slave_dev->master;
    struct bonding *bond = netdev_priv(bond_dev);
    + int err;

    switch (event) {
    case NETDEV_UNREGISTER:
    @@ -3570,6 +3571,15 @@ static int bond_slave_netdev_event(unsig
    * servitude.
    */
    break;
    + case NETDEV_CHANGEADDR:
    + if (bond->params.mode == BOND_MODE_ALB)
    + err = bond_alb_check_mac_address(bond);
    + else if (compare_ether_addr(bond_dev->dev_addr, addr) != 0)
    + err = -EINVAL;
    +
    + if (err)
    + return notifier_from_errno(err);
    + break;
    case NETDEV_CHANGENAME:
    /*
    * TODO: handle changing the primary's name




    \
     
     \ /
      Last update: 2009-03-16 00:17    [W:0.026 / U:98.760 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site