Bug 442124 - bonding: incorrect backport creates possible incorrect interface flags
Summary: bonding: incorrect backport creates possible incorrect interface flags
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: kernel
Version: 4.7
Hardware: All
OS: Linux
urgent
urgent
Target Milestone: rc
: ---
Assignee: Andy Gospodarek
QA Contact: Martin Jenner
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-04-11 21:04 UTC by Andy Gospodarek
Modified: 2014-06-29 23:00 UTC (History)
2 users (show)

Fixed In Version: RHSA-2008-0665
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-07-24 19:28:57 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2008:0665 0 normal SHIPPED_LIVE Moderate: Updated kernel packages for Red Hat Enterprise Linux 4.7 2008-07-24 16:41:06 UTC

Description Andy Gospodarek 2008-04-11 21:04:58 UTC
This commit to the RHEL4 git tree contains contains an error:

commit ea821e7617146fa966f4e61a8ce3a010fafad098
Author: Brad Peters <bpeters>
Date:   Wed Mar 12 17:00:54 2008 -0400

    [NET] bonding: add MAC based failover support to bonding driver

This hunk is incorrect:

@@ -1874,17 +1890,20 @@ static int bond_enslave(struct net_device *bond_dev,
struct net_device *slave_de
                 * and then set it to the master's address
                 */
                memcpy(new_slave->perm_hwaddr, slave_dev->dev_addr, ETH_ALEN);
-
-               /* set slave to master's mac address
-                * The application already set the master's
-                * mac address to that of the first slave
-                */
-               memcpy(addr.sa_data, bond_dev->dev_addr, bond_dev->addr_len);
-               addr.sa_family = slave_dev->type;
-               res = slave_dev->set_mac_address(slave_dev, &addr);
-               if (res) {
-                       dprintk("Error %d calling set_mac_address\n", res);
-                       goto err_unset_master;
+               if (!bond->params.fail_over_mac) {
+                       /*
+                       * Set slave to master's mac address.  The
+                       * application already set the master's mac
+                       * address to that of the first slave
+                       */
+                       memcpy(addr.sa_data, bond_dev->dev_addr,
+                               bond_dev->addr_len);
+                       addr.sa_family = slave_dev->type;
+                       res = slave_dev->set_mac_address(slave_dev, &addr);
+                       if (res) {
+                               dprintk("Error %d calling set_mac_address\n", res);
+                               goto err_free;
+                       }
                }

                /* open the slave since the application closed it */

This patch corrects the problem.

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c883da8..b5592f4 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1903,7 +1903,7 @@ static int bond_enslave(struct net_device *bond_dev,
struct net_device *slave_de
                        res = slave_dev->set_mac_address(slave_dev, &addr);
                        if (res) {
                                dprintk("Error %d calling set_mac_address\n", res);
-                               goto err_free;
+                               goto err_unset_master;
                        }
                }

As the code stands right now if a call to slave_dev->set_mac_address(slave_dev,
&addr) fails the IFF_SLAVE flag will continue to be set on an interface.  This
will prevent the device from being enslaved again and would require deletion of
that device before it could be used in a bond.

The impact is probably minimal, but it should be fixed since this code was
introduced in this update cycle.

Comment 1 Andy Gospodarek 2008-04-11 21:08:29 UTC
Correctly formatted patch available here:

http://sharks.rdu.redhat.com/?p=gospo/rhel4-gtest.git;a=commitdiff;h=289b80ca09a5d3ba0ce31b467828ab486f45b731

Hopefully I'll add this to my test kernels next week.

Comment 2 RHEL Program Management 2008-04-11 21:09:30 UTC
This request was evaluated by Red Hat Product Management for inclusion in a Red
Hat Enterprise Linux maintenance release.  Product Management has requested
further review of this request by Red Hat Engineering, for potential
inclusion in a Red Hat Enterprise Linux Update release for currently deployed
products.  This request is not yet committed for inclusion in an Update
release.

Comment 3 Andy Gospodarek 2008-04-14 13:33:20 UTC
My test kernels have been updated to include a patch for this bugzilla.

http://people.redhat.com/agospoda/#rhel4

Please test them and report back your results.

Comment 6 Vivek Goyal 2008-06-12 18:31:20 UTC
Committed in 73.EL . RPMS are available at http://people.redhat.com/vgoyal/rhel4/

Comment 9 errata-xmlrpc 2008-07-24 19:28:57 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2008-0665.html


Note You need to log in before you can comment on or make changes to this bug.