Re: [PATCH net-next] bonding: lacp_port_id setting for 802.3ad

[Jay Vosburgh <fubar@us.ibm.com>]

Krisztian Ivancso <github-ivan@ivancso.net> wrote:

>On 08/13/2013 11:39 AM, Ding Tianhong wrote:
>> On 2013/8/13 17:20, Krisztian Ivancso wrote:
>>> On 08/13/2013 03:07 AM, Ding Tianhong wrote:
>>>> On 2013/8/12 19:19, Krisztian Ivancso wrote:
>>>>> >From 472fffa5a8f170daed9e4cc677af8e2560b86be2 Mon Sep 17 00:00:00 2001
>>>>> From: Krisztian Ivancso <github-ivan@ivancso.net>
>>>>> Date: Sun, 11 Aug 2013 20:30:44 +0200
>>>>> Subject: [PATCH net-next] bonding: lacp_port_id setting for 802.3ad ports
>> 
>> ok, for example: the bonding has four slave, slave1 and slave2 aggregation to 1 group,
>> and slave3 and slave4 aggregtion to 2 group, how you distinguish the 1 and 2 group by initialize id.
>
>this is not possible, because all slave have to be a member of the same
>aggregation group.

	Just on the above point, bonding can group slaves into multiple
aggregators, but only one aggregator will be active at any given time.

	To answer the question, the four slaves would each be given
unique port IDs that do not conflict.

>i think we misunderstood each other.
>
>here is a new example:
>- switch1 is a switch with a configured lag with two members ports
>  (member1 and member2)
>- two linux (linux1 and linux2) box with a configured bonding device
>  (bond0) with the same MAC set in both box and one
>  slave on each
>- lacp_port_id is set to 10 in linux1 and 20 in linux2
>
>you can attach the slave from both linux boxes to the same
>lag on switch1. (slave from linux1 to port member1 and
>slave from linux2 to port member2 on switch1)
>
>port id must be unique within a system.
>bonding implementation set a unique system id for every bonding device
>which is derived from MAC of one of the slave interfaces.
>
>if we use the current bonding implementation second linux box can't be
>a member on switch1 because port id is 1 in both linux bonding device.
>
>if we can set different starting port id for bonding in different boxes
>the second box can be a member also.

	I understand what you're trying to do here (permit multiple
instances of bonding on different systems to connect to a single
aggregator on a switch), and I don't really have a problem with it in
general.

	I do have some comments:

	First, altering the lacp_port_id (via sysfs) should only be
permitted when there are no slaves in the bond, otherwise the
/proc/net/bonding/bond0 output for the first port id will not match the
actual first port id value assigned to the slaves.  As a practical
matter, altering lacp_port_id while slaves are present in the bond has
no effect until all slaves are released and the first new slave is
added, so this is not reducing functionality.

	Second, the lacp_port_id is global across all bonds created
within the loaded module, and so multiple bonds will all use the same
starting value.  Setting the lacp_port_id via sysfs has no effect, as it
alters a per-bond value, bond->params.lacp_port_id, that is never
actually used to set the port ID of a first slave in bond_enslave.

	The global default value should only be used to initialize the
per-bond value when a bond is created, and that per-bond value should be
used when setting the port id in bond_enslave().  The per-bond value is
already displayed in /proc/net/bonding/bond0, and is the value modified
by the sysfs functions

	Third, consider adding the port ID to the 803.2ad section in
bond_info_show_slave.

	Lastly, I think this should be tested against systems other than
Cisco to insure that it really interoperates with, for example,
Juniper's methodology for spanning an aggregator across physical
chassis.  I'm not sure why it wouldn't, but once new functionality
becomes part of the kernel, changing it in non-backwards compatible ways
is difficult.

	-J

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