Re: [PATCH SET][bonding] cleanup

Re: [PATCH SET][bonding] cleanup

[Shmulik Hen]

On Thursday 25 September 2003 07:22 pm, Jay Vosburgh wrote:
> >patch 7 - eliminate the multicast_mode module param. settings are
> > now done only according to mode.
>
>         This goes a bit beyond straight cleanup; could you explain
> the rationale for this change?  Also, unless I'm missing something,
> the patch does not appear to update bonding.txt to reflect the fact
> that the module parameter is no more.
>
>

That question rings a bell :)
I had this discussion with Deniel Laurent on 8/30 and it sounded like 
that wouldn't be a problem after all.

[from that thread]
> Do you know that this breaks upward compatibility ?
[snip]
> But if everyone is OK ... (I'd be since I currently only use
> multicast_mode = 1 ;-).

The rationale is that the propagation code already made this module 
param obsolete, so the next step was to remove it entirely since it 
had no effect anyway.

According to the propagation RFC we sent on 2/6, multicast list, 
allmulti flag and promisc flag are all controlled the same way, and 
that is according to the USES_PRIMARY macro. When the bond uses the 
current slave as a primary interface, it, and only it, is supposed to 
have the bond's properties, while in aggregation modes all slaves 
have the same settings. There is no point in settings other slaves, 
that are not supposed to be receiving in the first place, to have 
loose filtering. Otherwise the stack will be flooded by duplicate 
packets. The situation is bad enough now since bonding has no 
solution for broadcast packets, but that's for another thread.

-- 
| Shmulik Hen   Advanced Network Services  |
| Israel Design Center, Jerusalem          |
| LAN Access Division, Platform Networking |
| Intel Communications Group, Intel corp.  |

Re: [PATCH SET][bonding] cleanup

[Jay Vosburgh]

>patch 7 - eliminate the multicast_mode module param. settings are now 
>          done only according to mode.

	This goes a bit beyond straight cleanup; could you explain the
rationale for this change?  Also, unless I'm missing something, the
patch does not appear to update bonding.txt to reflect the fact that
the module parameter is no more.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

[PATCH SET][bonding] cleanup

[Shmulik Hen]

Hi,

Now that all the 2.4<->2.6 synchronizing stuff is done, we are moving 
forward with the cleanup plan. This set is similar to the previous 
set I sent on 8/27, but it is based on the latest version that was 
accepted into the kernel with the seq_file changes, a few bug fixes 
and a bit more cleanup stuff. This set is very comprehensive and 
touches almost all the code. The set is broken down to many patches 
for better tracking. It was already tested by me for functionality 
and is undergoing a more thorough set of testing by our QA group for 
any corner case bugs. A set that cleans up the 802.3ad code will 
follow shortly.

This patch applies on 2.4.23-pre5. It would also apply on 2.6.0 after 
Amir's patch 2/10 from the "[bonding 2.6] propagating master's 
settings to slaves" set is accepted by Jeff and applied on 2.6.

patch set can be downloaded from:
http://osdn.dl.sourceforge.net/sourceforge/bonding/bonding-cleanup-2.4.23-pre5.tar.bz2

This will update the following files:

        Documentation/networking/bonding.txt
        Documentation/networking/ifenslave.c
        drivers/net/bonding/bond_3ad.c
        drivers/net/bonding/bond_alb.c
        drivers/net/bonding/bond_alb.h
        drivers/net/bonding/bonding.h
        drivers/net/bonding/bond_main.c
        include/linux/if_bonding.h

Description:
patch 1 - ifenslave lite - No more IP settings to slaves, unified 
          printing format, code re-org and broken to more functions.
patch 2 - convert all debug prints to use the dprintk macro and 
          consolidate format of all prints (e.g. "bonding: Error: 
          ...").
patch 3 - death of typedef. eliminate bonding_t/slave_t types and 
          consolidate casting.
patch 4 - remove dead code, old compatibility stuff and redundant 
          checks.
patch 5 - consolidate timers initialization, error checking and 
          re-queuing.
patch 6 - convert too long if-else to a switch-case. Fix all locations 
          that handles bond->primary.
patch 7 - eliminate the multicast_mode module param. settings are now 
          done only according to mode.
patch 8 - slave list iteration - bond is no longer part of the list, 
          added cyclic list iteration macros.
patch 9 - consolidate function declarations:
          o all functions begin with bond_
          o return value, function name and all params are on the same 
            line.
          o change some function names to be more descriptive.
patch 10 - consolidate names of function params and variables (e.g. 
           bond_dev instead of dev/master/master_dev).
patch 11 - change names/types for some of the members in struct 
           bonding. change position of members.
patch 12 - consolidate return values of functions.
patch 13 - put curly braces around all if, else, for, while, switch 
           statements. change conditions to short format.
           e.g. (ptr == NULL) ==> (!ptr)
patch 14 - consolidate error handling in all xmit functions.
patch 15 - chomp all trailing white space.
patch 16 - remove duplicate empty lines. add empty lines where needed 
           to improve readability.
patch 17 - fix indentations.
patch 18 - code re-organization in bond_main.c according to context
           (e.g. module initialization, bond initialization, device 
           entry points, monitoring, etc.). some last minute minor 
           changes and fixes.

-- 
| Shmulik Hen   Advanced Network Services  |
| Israel Design Center, Jerusalem          |
| LAN Access Division, Platform Networking |
| Intel Communications Group, Intel corp.  |