Re: [bonding] Add basic support for dynamic configuration of bond interfaces

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Amir Noam]

On Sunday 11 January 2004 03:34 am, Jeff Garzik wrote:
> Amir Noam wrote:
> > The following patch sets provide basic support for future bonding
> > operations (specifically for dynamic configuration of bonding
> > interfaces).
> >
> > This is done by adding two new bonding ioctls: one for deviceless
> > commands (an ioctl hook) and one for device oriented commands.
> > Like ethtool, the first u32 value in the data structure will
> > indicate the exact sub-command to be executed.
> >
> > The sets are against the latest netdev-2.4 and
> > net-drivers-2.5-exp trees.
>
> I don't disagree with the overall goal of these patches, but I
> think we might need to pause a bit, and consider how best to
> configure, add, and remove bonding interfaces, if we are coming up
> with a new interface.
>
>
> For configuration tasks that occur outside the scope of a single
> bonding interface (i.e. a single struct net_device), you need a
> separate entity from a socket ioctl.  It's not ideal at all to
> configure N objects using a special ioctl ... when opening one of
> said objects :)

That's not exactly what the new ioctls do.

The new SIOCBONDING ioctl handles commands aimed for the bonding 
module (and not to any specific bonding interface). This is done via 
an ioctl hook that captures these commands at the socket level 
(before dev_ioctl()) and passes it to the bonding code. Such commands 
do not require opening a bonding interface at all, just a valid 
socket to send the ioctl.

Also, such commands do not configure N other objects (like bonding 
interfaces) at all. They are used explicitly for "deviceless" 
commands, like dynamically adding and removing a bonding interface 
from the system.

> I would suggest a simple character device (misc_register), and let
> the userland application configure settings unrelated to a single
> object.

I was trying to follow the example of several other network modules 
(e.g. vlan, dlci, bridge) that use a socket level ioctl hook to 
handle such deviceless commands.

Do you think that a char device is preferrable, given that other 
similar modules use a socket ioctl hook?

> For a configuring state related to a _single_ bonding interface
> (i.e. a single net_device), socket ioctls are OK.

For such configuration commands (aimed at a specific bonding 
interface) I've added the SIOCBONDDEVICE ioctl, which is designed to 
be extendable to more commands in the future.

-- 
Amir

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[jamal]

On Sun, 2004-01-11 at 09:28, Amir Noam wrote:

> 
> > I would suggest a simple character device (misc_register), and let
> > the userland application configure settings unrelated to a single
> > object.
> 
> I was trying to follow the example of several other network modules 
> (e.g. vlan, dlci, bridge) that use a socket level ioctl hook to 
> handle such deviceless commands.
> 
> Do you think that a char device is preferrable, given that other 
> similar modules use a socket ioctl hook?

I think a netlink interface would be the better alternative.
The L2C netlink socket protocol would be appropriate for bonding
at least for 2.6.x. Bridging, VLAN should also move to this.

cheers,
jamal

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Jeff Garzik]

Amir Noam wrote:
> The new SIOCBONDING ioctl handles commands aimed for the bonding 
> module (and not to any specific bonding interface). This is done via 
> an ioctl hook that captures these commands at the socket level 
> (before dev_ioctl()) and passes it to the bonding code. Such commands 
> do not require opening a bonding interface at all, just a valid 
> socket to send the ioctl.

Right.  And this last requirement is spurious.  The operation is 
essentially "open a socket unrelated to X, to perform actions on X".

bonding (and it sounds like VLAN too) needs a specific, well-known 
control point, for controlling module-level data such as the creation 
and deletion of interfaces.

One such well-known control point would be a /dev node.  Jamal's 
suggestion is also an excellent one:  let userspace use netlink to 
communicate with a well known "address" inside the kernel, which would 
work as the central (and thus bonding-module-wide) bonding control 
interface.


> I was trying to follow the example of several other network modules 
> (e.g. vlan, dlci, bridge) that use a socket level ioctl hook to 
> handle such deviceless commands.
> 
> Do you think that a char device is preferrable, given that other 
> similar modules use a socket ioctl hook?

If vlan/dlci/bridge has similar requirements to described above ("open 
socket unrelated to X, to perform actions on X") then they need to be 
fixed up too.

Opening a random socket to use an ioctl(2) to produce some magic 
behavior is just ugly, and we need to avoid that.  Maybe bgreear would 
be open to updating VLAN's interface to netlink at the same time, for 
example?  That would allow us (us==netdev) to understand all angles and 
implications of a good interface design.  VLAN AFAICS has a similar 
requirement:  need an interface _unrelated_ to specific VLAN interfaces, 
to control VLAN interfaces.  And netlink certainly works as a control 
protocol :)

	Jeff

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Ben Greear]

Jeff Garzik wrote:
> Amir Noam wrote:
> 
>> The new SIOCBONDING ioctl handles commands aimed for the bonding 
>> module (and not to any specific bonding interface). This is done via 
>> an ioctl hook that captures these commands at the socket level (before 
>> dev_ioctl()) and passes it to the bonding code. Such commands do not 
>> require opening a bonding interface at all, just a valid socket to 
>> send the ioctl.
> 
> 
> Right.  And this last requirement is spurious.  The operation is 
> essentially "open a socket unrelated to X, to perform actions on X".
> 
> bonding (and it sounds like VLAN too) needs a specific, well-known 
> control point, for controlling module-level data such as the creation 
> and deletion of interfaces.
> 
> One such well-known control point would be a /dev node.  Jamal's 
> suggestion is also an excellent one:  let userspace use netlink to 
> communicate with a well known "address" inside the kernel, which would 
> work as the central (and thus bonding-module-wide) bonding control 
> interface.
> 
> 
>> I was trying to follow the example of several other network modules 
>> (e.g. vlan, dlci, bridge) that use a socket level ioctl hook to handle 
>> such deviceless commands.
>>
>> Do you think that a char device is preferrable, given that other 
>> similar modules use a socket ioctl hook?
> 
> 
> If vlan/dlci/bridge has similar requirements to described above ("open 
> socket unrelated to X, to perform actions on X") then they need to be 
> fixed up too.
> 
> Opening a random socket to use an ioctl(2) to produce some magic 
> behavior is just ugly, and we need to avoid that.  Maybe bgreear would 
> be open to updating VLAN's interface to netlink at the same time, for 

Actually, I am not a fan of netlink for normal configuration, and would
prefer to keep the VLAN control through IOCTL calls.  This is mainly
because I have not had an easy time of figuring out a simple way to
use netlink, while ioctls are very easy to use.

If someone wants to update vlan and vconfig to use netlink, then
maybe we could add that API as well, but definately we should not
remove the IOCTL calls untill at least 2.7.

I would also be open to moving the VLAN ioctls over into the
ethtool ioctl space, but that just exchanges one magic ioctl for
another...

Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Jeff Garzik]

Ben Greear wrote:
> I would also be open to moving the VLAN ioctls over into the
> ethtool ioctl space, but that just exchanges one magic ioctl for
> another...


The key question is what is the best interface for userland to configure 
in-kernel information -that is unrelated to a specific interface-. 
ethtool ioctl space doesn't apply, because that's a per-interface API.

Opening a socket and just ioctl'ing away isn't terribly scalable in the 
long run, either.  Consider all the applications that could legitimately 
claim they need a SIOCxxx ioctl assignment, just for their little slice 
of the networking world.  Further, consider that all an ioctl is is a 
message sent to the kernel driver, perhaps sending and/or receiving some 
data.  :)

ioctls are a pain for 32/64-bit emulation layers too.  It seems much 
easier to define a netlink protocol family of some sort and communicate 
that way.

I'll poke around and see what I can come up with.

	Jeff

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[jamal]

On Sun, 2004-01-11 at 16:59, Jeff Garzik wrote:
> ioctls are a pain for 32/64-bit emulation layers too.  It seems much 
> easier to define a netlink protocol family of some sort and communicate 
> that way.
> 
> I'll poke around and see what I can come up with.

Theres a L2C netlink protocol (for Layer2 Config) which is already in
place being reviewed by DaveM in whatever spurious time he has.
Current user is MPLS but definete candidate for VLAN, and the move of
bridging STP control to user space. It is true that netlink may not be
intuitive to use for someone who hasnt spent time eyeballing it(mostly
because of lack of programming docs really). We are going to try and
improve things by making it easy to use from both kernel and user space
POV and provide sample code which people can cutnpaste in the classical
linux-way(tm). 

cheers,
jamal

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Ben Greear]

Jeff Garzik wrote:
> Ben Greear wrote:
> 
>> I would also be open to moving the VLAN ioctls over into the
>> ethtool ioctl space, but that just exchanges one magic ioctl for
>> another...
> 
> 
> 
> The key question is what is the best interface for userland to configure 
> in-kernel information -that is unrelated to a specific interface-. 
> ethtool ioctl space doesn't apply, because that's a per-interface API.

Actually, VLANs map very well to a per-interface API, since VLANs are
interfaces and reside on other interfaces.

> Opening a socket and just ioctl'ing away isn't terribly scalable in the 
> long run, either.  Consider all the applications that could legitimately 
> claim they need a SIOCxxx ioctl assignment, just for their little slice 
> of the networking world.  Further, consider that all an ioctl is is a 

My assumption is that adding a new ethtool message (or vlan ioctl message
to the existing VLAN ioctl call) does not cause new emulation problems.
Is that true?

> I'll poke around and see what I can come up with.

I'm interested in seeing the result.  I've never found a simple
example of something using the netlink api, and if it can be done,
then I'll probably convert.

Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Jason Lunz]

jgarzik@pobox.com said:
> The key question is what is the best interface for userland to configure 
> in-kernel information -that is unrelated to a specific interface-. 
> ethtool ioctl space doesn't apply, because that's a per-interface API.

ethtool is just as bad as brctl or any of the others. From (userland)
ethtool.c:

	static int doit(void)
	{
		struct ifreq ifr;
		int fd;

		/* Setup our control structures. */
		memset(&ifr, 0, sizeof(ifr));
		strcpy(ifr.ifr_name, devname);
		/* Open control socket. */
		fd = socket(AF_INET, SOCK_DGRAM, 0);
		if (fd < 0) {
			perror("Cannot get control socket");
			return 70;
		}

		/* now do ioctl() on fd, having nothing to do with
		 * AF_INET nor SOCK_DGRAM */

calling a spade a spade, and all that. I don't see how that's any better
than brctl. The per-interface only comes into it when you copy a dev
name into a struct ifreq, but that doesn't associate the fd with the
interface in any way. You could go ahead and issue another ioctl on the
same fd for a different interface. 

Jason

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Jeff Garzik]

Jason Lunz wrote:
> jgarzik@pobox.com said:
> 
>>The key question is what is the best interface for userland to configure 
>>in-kernel information -that is unrelated to a specific interface-. 
>>ethtool ioctl space doesn't apply, because that's a per-interface API.
> 
> 
> ethtool is just as bad as brctl or any of the others. From (userland)
> ethtool.c:

ethtool contains only per-interface operations.

One should not stuff extra-interface operations onto an ioctl, IMO.


> calling a spade a spade, and all that. I don't see how that's any better
> than brctl. The per-interface only comes into it when you copy a dev
> name into a struct ifreq, but that doesn't associate the fd with the
> interface in any way. You could go ahead and issue another ioctl on the
> same fd for a different interface. 

I make it no secret that I dislike ioctls in general, and would like to 
move away from dependence on ioctl (and procfs) magic...  Ideally one 
should be able to maintain and coordinate the ABI of one's own driver, 
without always going through a central authority for some magic number. 
  Linux is (should be) more dynamic than that.  It's a hotplug, 
decentralized world :)

	Jeff

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Andi Kleen]

On Sun, 11 Jan 2004 16:59:10 -0500
Jeff Garzik <jgarzik@pobox.com> wrote:


> ioctls are a pain for 32/64-bit emulation layers too.  It seems much 
> easier to define a netlink protocol family of some sort and communicate 
> that way.

Actually that's not true. netlink is far worse for emulation layers when you
break the protocol. e.g. the current ipsec/pf_key protocol is not compatible
on x86-64 and it is near impossible to fix it without major surgery.
With ioctls it would be far easier to fixbecause the infrastructure for emulation
is already there.

-Andi

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[jamal]

Andi,
Can you point to specifics that break 64 bit emulation so we can avoid
them?
I have to admit the pf_key interaction is relatively unorthrodox but
looked fine.
What are the things you will perform surgery on?

cheers,
jamal

On Mon, 2004-01-12 at 07:38, Andi Kleen wrote:
> On Sun, 11 Jan 2004 16:59:10 -0500
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
> 
> > ioctls are a pain for 32/64-bit emulation layers too.  It seems much 
> > easier to define a netlink protocol family of some sort and communicate 
> > that way.
> 
> Actually that's not true. netlink is far worse for emulation layers when you
> break the protocol. e.g. the current ipsec/pf_key protocol is not compatible
> on x86-64 and it is near impossible to fix it without major surgery.
> With ioctls it would be far easier to fixbecause the infrastructure for emulation
> is already there.
> 
> -Andi
> 

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Andi Kleen]

On 12 Jan 2004 08:51:02 -0500
jamal <hadi@cyberus.ca> wrote:

> Andi,
> Can you point to specifics that break 64 bit emulation so we can avoid
> them?
> I have to admit the pf_key interaction is relatively unorthrodox but
> looked fine.

Ok, here are the full rules: 

Standards:

long/void * are disallowed
use [su]{16,32,64} instead

Non obvious problems specific to IA64 and x86-64 (they don't happen on sparc64/ppc64,
that is why they were often not catched earlier):

When you use [su]64: 

make sure the data element is explicitely padded to be on a 8 byte boundary
in the structure. The reason is that alignof(u64) on i386 is 4 but 8 on ia64/x86-64
and the structure layout could change.

also when you use 64bit types make sure the complete structure is padded to 8 bytes.
The x86-64 ABI pads any data structure to the alignof() of its biggest member
to get good alignment for arrays. This is what broke PF_KEY - it is missing this
padding and the structure layout ends up differently with nested structures.

> What are the things you will perform surgery on?

I'm not sure I will because it is too ugly. Or at least I haven't found a nice elegant
way for it yet. Currently you just cannot use ipsec from 32bit executables, you have to use 
64bit. 

-Andi
 

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[jamal]

On Mon, 2004-01-12 at 10:04, Andi Kleen wrote:
[..]
> When you use [su]64: 
> 
> make sure the data element is explicitely padded to be on a 8 byte boundary
> in the structure. The reason is that alignof(u64) on i386 is 4 but 8 on ia64/x86-64
> and the structure layout could change.
> 
>
> also when you use 64bit types make sure the complete structure is padded to 8 bytes.
> The x86-64 ABI pads any data structure to the alignof() of its biggest member
> to get good alignment for arrays. This is what broke PF_KEY - it is missing this
> padding and the structure layout ends up differently with nested structures.
> 

The behavior of some 32 bit archs alignof()e.g ppc is similar in
aligning to its biggest member. 
We have some major problems with include/linux/pkt_sched.h::tc_stats for
example; i just have to keep hacking tc on ppc everytime so it doesnt
stop processing and bitch about truncated stats; [this happens when
doing a sizeof() of on-the-wire data passed from the kernel being
compared against sizeof(tc_stats)]. I recall there are a few other
similar structures. It didnt seem like there was a clean solution when i
last looked at this. It seems we may need surgery on a few places like
this and may require a few #ifdefs. Any thoughts on this particular one?

cheers,
jamal

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Andi Kleen]

On Tue, Jan 13, 2004 at 07:28:50AM -0500, jamal wrote:
> The behavior of some 32 bit archs alignof()e.g ppc is similar in
> aligning to its biggest member. 
> We have some major problems with include/linux/pkt_sched.h::tc_stats for
> example; i just have to keep hacking tc on ppc everytime so it doesnt
> stop processing and bitch about truncated stats; [this happens when
> doing a sizeof() of on-the-wire data passed from the kernel being
> compared against sizeof(tc_stats)]. I recall there are a few other
> similar structures. It didnt seem like there was a clean solution when i
> last looked at this. It seems we may need surgery on a few places like
> this and may require a few #ifdefs. Any thoughts on this particular one?

I don't think it can be fixed with #ifdefs. For existing structures
we cannot change the layout because they are already enshrined in the ABI.
You have to translate them in the emulation layer somehow or just give up.
And for new ones just add all necessary padding explicitely or best avoid
__u64 (which causes most problems)

In fact we should have some watchdog enforcing these rules for all new
kernel submissions </dreaming>. In fact DaveM did that for some time,
but he unfortunately only enforced the rules needed for sparc64 which
are a subset of those needed by other ports.

-Andi

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Amir Noam]

On Sunday 11 January 2004 11:59 pm, Jeff Garzik wrote:

> ioctls are a pain for 32/64-bit emulation layers too.  It seems
> much easier to define a netlink protocol family of some sort and
> communicate that way.

It seems that a lot of different suggestions were made so far about 
the best way to pass messages unrelated to a specific interface to 
the kernel (char device, netlink, sysfs), all with their own 
advantages and disadvantages.

Until the preferred method is decided on for 2.6, is there a real 
objection to using a generic socket ioctl for bonding in the *2.4* 
kernel? (again, given that several other modules already use such a 
scheme, and won't change that behavior in 2.4)

It would be nice to have the support for dynamically adding/removing 
bonding interfaces in 2.4, and 2.4.25 is about to be the last 2.4 
kernel that accepts new features.

-- 
Amir

[bonding] Add basic support for dynamic configuration of bond interfaces

[Amir Noam]

The following patch sets provide basic support for future bonding 
operations (specifically for dynamic configuration of bonding 
interfaces).

This is done by adding two new bonding ioctls: one for deviceless 
commands (an ioctl hook) and one for device oriented commands. Like 
ethtool, the first u32 value in the data structure will indicate the 
exact sub-command to be executed.

The sets are against the latest netdev-2.4 and net-drivers-2.5-exp 
trees.

-- 
Amir

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Jeff Garzik]

Amir Noam wrote:
> The following patch sets provide basic support for future bonding 
> operations (specifically for dynamic configuration of bonding 
> interfaces).
> 
> This is done by adding two new bonding ioctls: one for deviceless 
> commands (an ioctl hook) and one for device oriented commands. Like 
> ethtool, the first u32 value in the data structure will indicate the 
> exact sub-command to be executed.
> 
> The sets are against the latest netdev-2.4 and net-drivers-2.5-exp 
> trees.


I don't disagree with the overall goal of these patches, but I think we 
might need to pause a bit, and consider how best to configure, add, and 
remove bonding interfaces, if we are coming up with a new interface.

For configuration tasks that occur outside the scope of a single bonding 
interface (i.e. a single struct net_device), you need a separate entity 
from a socket ioctl.  It's not ideal at all to configure N objects using 
a special ioctl ... when opening one of said objects :)

I would suggest a simple character device (misc_register), and let the 
userland application configure settings unrelated to a single object.

For a configuring state related to a _single_ bonding interface (i.e. a 
single net_device), socket ioctls are OK.

	Jeff

Re: [bonding] Add basic support for dynamic configuration of bond interfaces

[Ben Greear]

Jeff Garzik wrote:
> Amir Noam wrote:
> 
>> The following patch sets provide basic support for future bonding 
>> operations (specifically for dynamic configuration of bonding 
>> interfaces).
>>
>> This is done by adding two new bonding ioctls: one for deviceless 
>> commands (an ioctl hook) and one for device oriented commands. Like 
>> ethtool, the first u32 value in the data structure will indicate the 
>> exact sub-command to be executed.
>>
>> The sets are against the latest netdev-2.4 and net-drivers-2.5-exp trees.
> 
> 
> 
> I don't disagree with the overall goal of these patches, but I think we 
> might need to pause a bit, and consider how best to configure, add, and 
> remove bonding interfaces, if we are coming up with a new interface.
> 
> For configuration tasks that occur outside the scope of a single bonding 
> interface (i.e. a single struct net_device), you need a separate entity 
> from a socket ioctl.  It's not ideal at all to configure N objects using 
> a special ioctl ... when opening one of said objects :)

Why is it not ideal?  Are ioctl calls significantly less expensive
than accessing a character device?  It does not seem easier to program
on either side, and wouldn't users have to do a mknod or something like
that to get their char device to appear?

I would not look forward to dealing with framing issues in the kernel
either, and for 'reading' information, it seems there could be some tricky
code dealing with queueing up the message for user-space and then *hoping*
that they did a 'read' before you had to clean up the message and/or
over-write it.

> I would suggest a simple character device (misc_register), and let the 
> userland application configure settings unrelated to a single object.
> 
> For a configuring state related to a _single_ bonding interface (i.e. a 
> single net_device), socket ioctls are OK.

 From a user-space coding perspective, it would be easier to just have
one socket to do all the ioctl calls.  I really don't see even an aesthetic
reason to artificially restrict an socket ioctl to only function on a single
thing.

Please don't tell me you also want to change ethtool to not use
multiplexed ioctl calls as it does now?  (I like the ethtool interface
very much and in fact, if I were starting over, I'd implement VLAN
ioctls as an ethtool command...)


Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com