+++ This bug was initially created as a clone of Bug #467954 +++

Description of problem:
The syntax change from bug #288151 causes an upgrade of initscripts from 8.45.17 to 8.45.19 to break bonding when using arp_ip_target

Version-Release number of selected component (if applicable):
8.45.17
8.45.19

How reproducible:


Steps to Reproduce:
1. Setup a functional bonding configuration using arp_ip_target with initscripts-8.45.17.EL-1
2. Upgrade initscripts to initscripts-8.45.19.EL-1
  
Actual results:
Entry in /sys/class/net/bond0/bonding/arp_ip_target is now wrong.

Expected results:
Entry in /sys/class/net/bond0/bonding/arp_ip_target should have remained the same.

Additional info:
When upgrading from RHEL 5.1 with initscripts-8.45.17 to RHEL5.2 and initscripts-8.45.19 , the configuration of a bonded network interface needs to be re-written if arp_ip_target is being set in /etc/sysconfig/network-scripts/ifcfg-bond0

Also, the documentation included in kernel-doc-2.6.18-92.el5.noarch.rpm under /usr/share/doc/kernel-doc-2.6.18/Documentation/networking/bonding.txt states that using the +<arp_ip_target> syntax is correct.

"
For later versions of initscripts, such as that found with Fedora
7 and Red Hat Enterprise Linux version 5 (or later), it is possible, and,
indeed, preferable, to specify the bonding options in the ifcfg-bond0
file, e.g. a line of the format:

BONDING_OPTS="mode=active-backup arp_interval=60 arp_ip_target=+192.168.1.254"

        will configure the bond with the specified options.  The options
specified in BONDING_OPTS are identical to the bonding module parameters
except for the arp_ip_target field.  Each target should be included as a
separate option and should be preceded by a '+' to indicate it should be
added to the list of queried targets, e.g.,

        arp_ip_target=+192.168.1.1 arp_ip_target=+192.168.1.2

        is the proper syntax to specify multiple targets.
"

--- Additional comment from darcy.sherwood on 2008-10-21 18:26:26 EDT ---

I've written a patch that will handle both the syntax with and without the +

I tested the change, and I was able to specify arp_ip_target both with and without the plus, as well as in lists of formats "arp_ip_target=192.168.1.1,192.168.1.2" and "arp_ip_target=+192.168.1.1 arp_ip_target=+192.168.1.2"

diff ifup-eth ifup-eth-fix
129c129
<         if [ "${key}" = "arp_ip_target" ]; then
---
>         if [ "${key}" = "arp_ip_target" -a "${value:0:1}" != "+" ]; then

--- Additional comment from notting on 2008-10-21 19:48:33 EDT ---

Andy - did the wires get crossed on the way to the upstream docs somewhere?

I don't have a big issue with this, but it may not get in for a while due to scheduling.

--- Additional comment from notting on 2008-10-21 19:49:13 EDT ---

Well, considering working configs broke in 5.2, marking as a regression.

--- Additional comment from pm-rhel on 2008-10-21 19:49:58 EDT ---

This bugzilla has Keywords: Regression.  

Since no regressions are allowed between releases, 
it is also being proposed as a blocker for this release.  

Please resolve ASAP.

--- Additional comment from agospoda on 2008-10-22 11:48:31 EDT ---

I think the documentation just didn't get updated.  The intent was that we could specify:

arp_ip_target=10.0.0.1,10.0.0.2,10.0.0.3

going foward instead of

arp_ip_target=+10.0.0.1 arp_ip_target=+10.0.0.2 arp_ip_target=+10.0.0.3

in the BONDING_OPTS field in ifcfg-bondX.

I'll test now to make sure that works.

--- Additional comment from agospoda on 2008-10-22 12:45:51 EDT ---

Ok, so it appears the bug we wanted to fix (getting rid of the '+') is working as expected.  It seems we need to clarify the documentation in the kernel source tree so it is correct.  That should not be too tough to manage -- I'll get cracking on it.

--- Additional comment from casmith on 2008-10-22 15:10:05 EDT ---

I think that the issue is that the previous method, using the + signs, worked in 5.0 and 5.1 and are now broken in 5.2. So a user who got bonding working in 5.0 or 5.1 and updated their server to 5.2 would break their bonding configuration. It's my understanding that configuration files should not break between point releases the OS.

It appears that the patch that Darcy supplied will allow either format to work, which will fix the config file regression as well as allowing for the new format to work, the best of both worlds.

--- Additional comment from agospoda on 2008-10-22 15:32:01 EDT ---

Created an attachment (id=321205)
bonding-doc-fix.patch

I agree, Calvin, it would be nice to support both.

Unfortunately the fact that the '+' was needed was a bug itself.  I did not originally intend that we use it when I did the original initscripts patch for RHEL5.

When we discovered we needed the '+' in the configuration file, we went out and fixed it for 5.2 so the module options matched what was needed in ifcfg-bond0.

It's hard for me to call bugs that we fix 'regressions,' but I guess when customers rely on them that's how they feel.

Attached is the patch to the kernel documentation that should reflect our original intent.