Bug 483711 - [RHEL5] [PATCH] Cleanup arp_ip_target with ifdown-eth network script for avoiding write error message
[RHEL5] [PATCH] Cleanup arp_ip_target with ifdown-eth network script for avoi...
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: initscripts (Show other bugs)
5.3
All Linux
low Severity high
: rc
: ---
Assigned To: initscripts Maintenance Team
BaseOS QE
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-03 03:53 EST by Masaki MAENO
Modified: 2014-09-08 07:27 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-09-02 07:14:41 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
ifdown-eth script patch (738 bytes, patch)
2009-02-03 03:53 EST, Masaki MAENO
no flags Details | Diff
ifdown-up script patch (check all current 'arp_ip_target' entries before regist a new entry) (958 bytes, patch)
2009-02-03 20:24 EST, Masaki MAENO
no flags Details | Diff

  None (edit)
Description Masaki MAENO 2009-02-03 03:53:58 EST
Created attachment 330717 [details]
ifdown-eth script patch

Description of problem:

If the entry in /sys/class/net/bondX/bonding/arp_ip_target
and the entry of 'arp_ip_target' in /etc/sysconfig/network-scripts/ifcfg-bondX 
are the same, 'echo: write error' is output.

> Bringing up interface bond0:  /etc/sysconfig/network-scripts/ifup-eth: \
> line 133: echo: write error: Invalid argument

But, there is no problem in 'ifup bond0' operation.
Therefore, I am unpleasant.

Please take this patch by all means so that it is easy.

Steps to Reproduce:
0. set 'arp_ip_target' in /etc/sysconfig/network-scripts/ifcfg-bondX
1. ifup bondX (regist 'arp_ip_target')
2. ifdown bondX
3. ifup bondX (output 'echo: write error' because of the entry overlaps)

Actual results:
errormsg is output.

Expected results:
No errormsg is output.
Comment 1 Masaki MAENO 2009-02-03 03:56:52 EST
The patch deletes all of 'arp_ip_target' in /sys/class/net/bondX/bonding/arp_ip_target at doing 'ifdown bondX'
Comment 2 Bill Nottingham 2009-02-03 12:35:40 EST
I'm not sure that's the right answer - we don't deconfigure other bonding parameters of the interface on ifdown.
Comment 3 Andy Gospodarek 2009-02-03 13:17:03 EST
Bill, you are correct that we don't deconfigure other bonding parameters, but the initscripts presume that all parameters will be unset when they begin configuration.

It would be wise to consider removing all entries for arp_ip_target either when the interface goes down (with ifdown) or when it comes up (with ifup) before the parameters are set.  It might even be wise to zero out all parameters when the interface is just brought up to be sure no residual information remains from the previous configuration.
Comment 4 Bill Nottingham 2009-02-03 14:07:49 EST
'Zero out'... given how bonding is configured, there's no way to guess what needs zeroed out to what.

For arp_ip_target, is there a '-*' or similar syntax supported?
Comment 5 Andy Gospodarek 2009-02-03 14:45:18 EST
So I didn't elaborate on what I meant by 'zero out.'  I liked the patch in the Description that simply did this:

       # cleanup arp_ip_target
       if [ -d "/sys/class/net/${DEVICE}" ]; then
           arp_ip_target_path=/sys/class/net/${DEVICE}/bonding/arp_ip_target
           arp_ip_targets=`cat ${arp_ip_target_path}`
           for arg in ${arp_ip_targets}; do
              echo "-${arg}" > ${arp_ip_target_path}
           done
       fi

The sysfs files for 'arp_ip_target' and 'slaves' both use +[string] and -[string] syntax for adding and removing entries from their lists.  I might be a good idea to clear both of those with a patch much like the one above when it is first detected that a bonding device is coming up.

Maybe something like this untested patch:

diff --git a/sysconfig/network-scripts/ifup-eth b/sysconfig/network-scripts/ifup-eth
index 33df60f..2a9fe7d 100755
--- a/sysconfig/network-scripts/ifup-eth
+++ b/sysconfig/network-scripts/ifup-eth
@@ -122,6 +122,17 @@ if [ "$ISALIAS" = no ] && is_bonding_device ${DEVICE} ; then

     /sbin/ip link set dev ${DEVICE} down

+    # cleanup old settings if there are any
+    for arg in "slaves arp_ip_target" ; do
+       if [ -d "/sys/class/net/${DEVICE}" ]; then
+           sysfs_path=/sys/class/net/${DEVICE}/bonding/${arg}
+           targets=`cat ${sysfs_path}`
+           for entry in ${targets}; do
+              echo "-${entry}" > ${sysfs_path}
+           done
+       fi
+    done
+
     # add the bits to setup driver parameters here
     for arg in $BONDING_OPTS ; do
         key=${arg%%=*};
Comment 6 Bill Nottingham 2009-02-03 15:59:16 EST
Hm, this means that running 'ifup' twice in a row will bounce the device and all the slave links. Is that what we want?
Comment 7 Masaki MAENO 2009-02-03 19:22:40 EST
I am good only to erase all current entries of 'arp_ip_target'
by "echo -<ip address> > /sys/class/net/bondX/bonding/arp_ip_target" that kernel supports,
when bondX is doing down.

I think that I need not maintain 'arp_ip_target' configuration when bondX is down.
Why should kernel maintain 'arp_ip_target' configuration?


If the end-user sees this message, he becomes perplexed.
Please fix the problem.
> Bringing up interface bond0:  /etc/sysconfig/network-scripts/ifup-eth: \
> line 133: echo: write error: Invalid argument
Comment 8 Masaki MAENO 2009-02-03 19:43:18 EST
I confirmed that there is a success case and is a failure case
by operation of 'echo "-<ip address>" > /sys/class/net/bondX/bonding/arp_ip_target'

If it is so, I think that 'ifup-eth' script should check it so as not to register doubly
before it regist 'arp_ip_target'.
What do you think?
Comment 9 Masaki MAENO 2009-02-03 20:24:00 EST
Created attachment 330805 [details]
ifdown-up script patch (check all current 'arp_ip_target' entries before regist a new entry)

I changed the patch from deleting all 'arp_ip_target' entries in 'ifdown-eth'
to checking all current entries before regist a new entry.
Comment 10 Masaki MAENO 2009-02-03 20:25:24 EST
typo: ifdown-up script patch ----> correct: ifup-eth script patch
Comment 11 Bill Nottingham 2009-03-17 12:51:31 EDT
Upstream commit is at http://git.fedorahosted.org/git/?p=initscripts.git;a=commitdiff;h=a0895aba2529c34387cf9abdc658f5f43c39d32c - it's probably better to do it in the down path.
Comment 13 Harald Hoyer 2009-05-05 08:52:34 EDT
Please test the erratum candidate:
http://people.redhat.com/harald/downloads/initscripts/initscripts-8.45.26.1.el5/
Comment 17 errata-xmlrpc 2009-09-02 07:14:41 EDT
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/RHBA-2009-1344.html

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