Date:	Wed, 07 Dec 2011 11:59:49 -0800
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Arvid Brodin <arvid.brodin@...a.com>
cc:	Stephen Hemminger <shemminger@...tta.com>, netdev@...r.kernel.org
Subject: Re: bridge: HSR support

Arvid Brodin <arvid.brodin@...a.com> wrote:

>Stephen Hemminger wrote:
>> On Wed, 7 Dec 2011 00:23:21 +0100
>> Arvid Brodin <arvid.brodin@...a.com> wrote:
>> 
>>> Stephen Hemminger wrote:
>>>> On Fri, 28 Oct 2011 17:34:18 +0200
>>>> Arvid Brodin <arvid.brodin@...a.com> wrote:
>>>>
>>>>> Ok, so after a lot of reading and looking through code I have this idea of a
>>>>> standalone solution:
>>>>>
>>>>> 1) Add ioctls to create (and remove) "hsr" netdevs which encapsulates two
>>>>>    physical Ethernet interfaces each (somewhat like the bridge code does, but
>>>>>    with precisely 2 interfaces slaved).
>>>> Please use the newer netlink interface and the master attribute for this
>>>> rather than inventing yet another ioctl.
>>>
>>> Is the rtnl interface documented anywhere (the usage of the different IFLA_
>>> flags etc.)? Specifically: how do I use the IFLA_MASTER flag (what's the
>>> meaning of the 32-bit data it wants, and how is it used by the kernel)? I 
>>> haven't been very successful figuring this out by looking at the kernel code.
>> 
>> 
>> Look at bridging or bonding.
>
>Believe me, I have! :) Turns out IFLA_MASTER is actually handled in net/core/rtnetlink.c.
>Although the message is sent to the slave device, the functions called are the
>master device's ndo_{add,del}_slave(). Looking at the bridging and bonding
>implementations br_add_slave() and bond_enslave() and the functions they call,
>it all boils down mostly to 
>
>* netdev_set_master() which assigns slave_dev->master = master_dev.
>* bonding also set IFF_SLAVE on the slave.
>* netdev_rx_handler_register(slave_dev, ). "This handler will then be called
>  from __netif_receive_skb."
>
>So far so good, but:
>
>* I don't know the meaning of the IFF_SLAVE flag. It's referenced all over the place
>  (core, vlan, bonding, ipv6, eql). Do I need to/want to set this?

	Only if you actually need to for some reason.  There are a few
tests that make actual use of IFF_SLAVE, e.g., IPv6 won't run addrconf
on an interface with IFF_SLAVE set (which prevents bonding slaves from
having an IPv6 address distinct from the master).  Netpoll also treats
interfaces with IFF_SLAVE in a special way.  Bonding uses it internally
for various tests.

>* I don't know the effects of setting dev->master. Do I need/want this?

	Maybe.  One effect of netdev_set_master is that a reference is
acquired on the master, on behalf of the slave, so the master cannot
simply vanish until the slave releases that reference.  This predates
the notifier facility, and careful use of notifiers (handling
NETDEV_UNREGISTER) can get around the need for dev->master, but, e.g.,
vlan still acquires a reference to the real_dev without using
dev->master.

	It used to be that dev->master was used in netif_receive_skb for
packet handling purposes (for bonding, mostly; bridge and I think
macvlan had separate hooks).  That special sauce is now done by the
rx_handler, so there's really no requirement to use dev->master if you
have no need.

>* I don't want to forward all ingress frames on the slave devices to my master
>  device; I only want the ones with protocol 0x88FB to be forwarded (other
>  frames should be received by the slaves as normal). I think I already have this
>  covered by registering a protocol handler (using dev_add_pack(packet_type)).
>  So perhaps calling netdev_rx_handler_register() is not necessary in my case?

	You may want to use the rx_handler, and have it set skb->dev
appropriately for the frames that should forward to the master, and
leave skb->dev alone for the ones that should stick with the slave.
Both of those need the appropriate return from the rx_handler, which is
documented in netdevice.h.

	I'm not sure that you need a dev_add_pack at all; bonding
doesn't use one anymore, since everything it needs can now be done via
rx_handler.  The dev_add_pack approach may work, but rx_handler is
probably more efficient.

>* As far as I can see, neither bridging nor bonding is handled by the ip program
>  (iproute2 suite)? I.e. no examples of binding more than one interface to a
>  virtual interface when it comes to which messages to send, etc. VLAN uses
>  IFLA_IFNAME (name of the vlan link), IFLA_LINK (physical link behind the vlan
>  link), and some IFLA_VLAN-specific messages.

	In current versions of iproute, something like "ip link set
device eth0 master bond0" would add a slave to a bond.  You are correct,
though, that ip does not permit changing the bonding options, and I
don't believe it will create new master devices, either, so the bonding
support is limited.

	-J

>  What I want to do is to atomically (from a user space perspective) create the
>  HSR bonding, i.e.:
>
>  	# ip link add name hsr0 type hsr slave1 slave2
>
>  I have written a hsr "plugin" to iproute2 that accepts these parameters, I'm
>  just not sure how to tell the kernel about them. Perhaps then I should define
>  my own IFLA_HSR_UNSPEC, IFLA_HSR_SLAVE1, IFLA_HSR_SLAVE2 messages?
>
>>> Also, how do I best tell the kernel which my slave devices are when creating
>>> the hsr device? Should I create my own IFLA_HSR_UNSPEC, etc, or can I use some
>>> of the generic flags?
>> 
>> Look at macvlan, vlan, or bridging. There this is done by processing a newlink
>> message.
>
>macvlan and vlan both use IFLA_LINK to tell the kernel about their single
>underlying "real" device. None of these use more than one underlying device.
>Bridging does not implement newlink at all (uses ioctls, I think).
>
>
>>> Oh, and the kernel (struct rtnl_link_ops).newlink method has two (struct
>>> nlattr *[]) params: tb and data. What are their roles?
>>>
>
>Heh, I just realised that the difference is that "tb" contains generic (IFLA_)
>message data and "data" contains specific (e.g. IFLA_VLAN_) message data. 

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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists