netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* the future of ethtool
@ 2010-11-15 19:41 Jeff Garzik
  2010-11-15 20:18 ` Ben Hutchings
  0 siblings, 1 reply; 14+ messages in thread
From: Jeff Garzik @ 2010-11-15 19:41 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: NetDev, David Miller

Thanks for accepting ethtool maintainership.

There are two key unresolved issues with ethtool that are worth noting 
to the next maintainer.  Both of these come from years of user and 
customer complaints.

1) ethtool command line interface.

For 1,001 minor reasons of user taste and expectation, people tend to 
complain about the command line interface.  Due to script usage it is 
set in stone, and has been since before my tenure.  But users 
continually request something more flexible, often, in particular, 
wanting to set multiple settings in one execution, or wanting to apply 
the same setting to multiple interface in one execution.

Obviously one can script this, but, it is probably the #1 user request.

My thought was to create "nictool", a new tool with more flexible 
command line interface, using the same old ethtool ioctls currently in 
use today.  ('nictool' also solves a minor naming complaint from 
wireless and other people, who use ethtool on non-ethernet network 
interfaces)


2) multiple settings and the ethtool kernel interface

Another common complaint is related to multiple settings, and associated 
hardware NIC resets.

Many ethtool driver implementations look like this:

	ethtool_op_do_something()
		stop RX/TX
		apply settings
		perform full NIC reset, consuming much time
		start RX/TX

The problem arises when the user wishes to change multiple hardware 
attributes at the same time.  A user wishing to change 4 attributes 
could wind up with 4 ethtool(1) invocations, with 4 accompanying 
hardware NIC resets.  Time consuming, inefficient, and unnecessary.


Obviously the world has not ended without these changes, but these items 
do cause continued complaints from users, and we're here to be 
responsive to users presumably ;-)

	Jeff




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 19:41 the future of ethtool Jeff Garzik
@ 2010-11-15 20:18 ` Ben Hutchings
  2010-11-15 20:44   ` Stephen Hemminger
  2010-11-15 21:03   ` Jeff Garzik
  0 siblings, 2 replies; 14+ messages in thread
From: Ben Hutchings @ 2010-11-15 20:18 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: NetDev, David Miller

On Mon, 2010-11-15 at 14:41 -0500, Jeff Garzik wrote:
> Thanks for accepting ethtool maintainership.
> 
> There are two key unresolved issues with ethtool that are worth noting 
> to the next maintainer.  Both of these come from years of user and 
> customer complaints.
> 
> 1) ethtool command line interface.
> 
> For 1,001 minor reasons of user taste and expectation, people tend to 
> complain about the command line interface.  Due to script usage it is 
> set in stone, and has been since before my tenure.  But users 
> continually request something more flexible, often, in particular, 
> wanting to set multiple settings in one execution, or wanting to apply 
> the same setting to multiple interface in one execution.
> 
> Obviously one can script this, but, it is probably the #1 user request.

Thinking further along those lines, it would be useful to have ethtool
API bindings for Perl/Python/whatever, though those belong outside of
the current ethtool package.  I tried doing that for use in my own
scripts and it looks reasonably practical, though I'm not volunteering
to maintain such bindings.

> My thought was to create "nictool", a new tool with more flexible 
> command line interface, using the same old ethtool ioctls currently in 
> use today.  ('nictool' also solves a minor naming complaint from 
> wireless and other people, who use ethtool on non-ethernet network 
> interfaces)

I agree, some of the ethtool operations are very Ethernet-specific but
enough of them are applicable to other media that this makes sense.

I've recently been looking at FreeBSD where the sort of configuration we
do through ethtool is invoked through ifconfig, but then ifconfig is
effectively deprecated on Linux...

> 2) multiple settings and the ethtool kernel interface
> 
> Another common complaint is related to multiple settings, and associated 
> hardware NIC resets.
> 
> Many ethtool driver implementations look like this:
> 
> 	ethtool_op_do_something()
> 		stop RX/TX
> 		apply settings
> 		perform full NIC reset, consuming much time
> 		start RX/TX
> 
> The problem arises when the user wishes to change multiple hardware 
> attributes at the same time.  A user wishing to change 4 attributes 
> could wind up with 4 ethtool(1) invocations, with 4 accompanying 
> hardware NIC resets.  Time consuming, inefficient, and unnecessary.

Right.  In fact the begin() and complete() operations look like they
were meant to support this sort of optimisation.  Is that the case?

Ben.

> Obviously the world has not ended without these changes, but these items 
> do cause continued complaints from users, and we're here to be 
> responsive to users presumably ;-)


-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 20:18 ` Ben Hutchings
@ 2010-11-15 20:44   ` Stephen Hemminger
  2010-11-15 21:14     ` Ben Hutchings
  2010-11-15 21:03   ` Jeff Garzik
  1 sibling, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2010-11-15 20:44 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Jeff Garzik, NetDev, David Miller

On Mon, 15 Nov 2010 20:18:46 +0000
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Mon, 2010-11-15 at 14:41 -0500, Jeff Garzik wrote:
> > Thanks for accepting ethtool maintainership.
> > 
> > There are two key unresolved issues with ethtool that are worth noting 
> > to the next maintainer.  Both of these come from years of user and 
> > customer complaints.
> > 
> > 1) ethtool command line interface.
> > 
> > For 1,001 minor reasons of user taste and expectation, people tend to 
> > complain about the command line interface.  Due to script usage it is 
> > set in stone, and has been since before my tenure.  But users 
> > continually request something more flexible, often, in particular, 
> > wanting to set multiple settings in one execution, or wanting to apply 
> > the same setting to multiple interface in one execution.
> > 
> > Obviously one can script this, but, it is probably the #1 user request.
> 
> Thinking further along those lines, it would be useful to have ethtool
> API bindings for Perl/Python/whatever, though those belong outside of
> the current ethtool package.  I tried doing that for use in my own
> scripts and it looks reasonably practical, though I'm not volunteering
> to maintain such bindings.
> 
> > My thought was to create "nictool", a new tool with more flexible 
> > command line interface, using the same old ethtool ioctls currently in 
> > use today.  ('nictool' also solves a minor naming complaint from 
> > wireless and other people, who use ethtool on non-ethernet network 
> > interfaces)
> 
> I agree, some of the ethtool operations are very Ethernet-specific but
> enough of them are applicable to other media that this makes sense.
> 
> I've recently been looking at FreeBSD where the sort of configuration we
> do through ethtool is invoked through ifconfig, but then ifconfig is
> effectively deprecated on Linux...
> 
> > 2) multiple settings and the ethtool kernel interface
> > 
> > Another common complaint is related to multiple settings, and associated 
> > hardware NIC resets.
> > 
> > Many ethtool driver implementations look like this:
> > 
> > 	ethtool_op_do_something()
> > 		stop RX/TX
> > 		apply settings
> > 		perform full NIC reset, consuming much time
> > 		start RX/TX
> > 
> > The problem arises when the user wishes to change multiple hardware 
> > attributes at the same time.  A user wishing to change 4 attributes 
> > could wind up with 4 ethtool(1) invocations, with 4 accompanying 
> > hardware NIC resets.  Time consuming, inefficient, and unnecessary.
> 
> Right.  In fact the begin() and complete() operations look like they
> were meant to support this sort of optimisation.  Is that the case?
> 
> Ben.
> 
> > Obviously the world has not ended without these changes, but these items 
> > do cause continued complaints from users, and we're here to be 
> > responsive to users presumably ;-)
> 
> 

My views are simple:

Ethtool needs to be an extension of existing netlink API for interfaces.
  - handles multiple values per transaction
  - extensible

Someone has to write good libraries to access netlink from Perl/Python/C++.
The best so far is libmnl.

-- 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 20:18 ` Ben Hutchings
  2010-11-15 20:44   ` Stephen Hemminger
@ 2010-11-15 21:03   ` Jeff Garzik
  1 sibling, 0 replies; 14+ messages in thread
From: Jeff Garzik @ 2010-11-15 21:03 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: NetDev, David Miller

On 11/15/2010 03:18 PM, Ben Hutchings wrote:
> On Mon, 2010-11-15 at 14:41 -0500, Jeff Garzik wrote:
>> Thanks for accepting ethtool maintainership.
>>
>> There are two key unresolved issues with ethtool that are worth noting
>> to the next maintainer.  Both of these come from years of user and
>> customer complaints.
>>
>> 1) ethtool command line interface.
>>
>> For 1,001 minor reasons of user taste and expectation, people tend to
>> complain about the command line interface.  Due to script usage it is
>> set in stone, and has been since before my tenure.  But users
>> continually request something more flexible, often, in particular,
>> wanting to set multiple settings in one execution, or wanting to apply
>> the same setting to multiple interface in one execution.
>>
>> Obviously one can script this, but, it is probably the #1 user request.
>
> Thinking further along those lines, it would be useful to have ethtool
> API bindings for Perl/Python/whatever, though those belong outside of
> the current ethtool package.  I tried doing that for use in my own
> scripts and it looks reasonably practical, though I'm not volunteering
> to maintain such bindings.

I agree.  FWIW, python-ethtool exists out there in the wild.

There is 
git://git.kernel.org/pub/scm/linux/kernel/git/acme/python-ethtool.git
but I'm pretty sure acme handed off maintainership to someone else.  If 
you have the energy, integrating that into the official ethtool package 
would probably be welcomed by all parties.


>> My thought was to create "nictool", a new tool with more flexible
>> command line interface, using the same old ethtool ioctls currently in
>> use today.  ('nictool' also solves a minor naming complaint from
>> wireless and other people, who use ethtool on non-ethernet network
>> interfaces)
>
> I agree, some of the ethtool operations are very Ethernet-specific but
> enough of them are applicable to other media that this makes sense.
>
> I've recently been looking at FreeBSD where the sort of configuration we
> do through ethtool is invoked through ifconfig, but then ifconfig is
> effectively deprecated on Linux...

FreeBSD is back in the stone ages, when it comes to this sort of thing, IMO.

Though, I do understand users' desire for a single tool that configures 
both generic network interface parameters and hardware-specific, 
vendor-specific interface parameters.


>> 2) multiple settings and the ethtool kernel interface
>>
>> Another common complaint is related to multiple settings, and associated
>> hardware NIC resets.
>>
>> Many ethtool driver implementations look like this:
>>
>> 	ethtool_op_do_something()
>> 		stop RX/TX
>> 		apply settings
>> 		perform full NIC reset, consuming much time
>> 		start RX/TX
>>
>> The problem arises when the user wishes to change multiple hardware
>> attributes at the same time.  A user wishing to change 4 attributes
>> could wind up with 4 ethtool(1) invocations, with 4 accompanying
>> hardware NIC resets.  Time consuming, inefficient, and unnecessary.
>
> Right.  In fact the begin() and complete() operations look like they
> were meant to support this sort of optimisation.  Is that the case?

Somewhat...

->begin() and ->complete() were initially for reducing internal code 
duplication.  You would put stop-RXTX in begin(), and 
nic-reset/start-RXTX in complete().

Fundamentally, the ioctl interface itself is still fine-grained.  You 
don't have ETHTOOL_BEGIN and ETHTOOL_COMPLETE ioctls.  And even if you 
did, such operations would be fundamentally race-prone.

AFAICS, the only race-free way to implement would be to have userspace 
issue a single ioctl that contains a list of operations to perform. eg.

A new ETHTOOL_MULTI_OP ioctl would contain
	op SSET + payload
	op SSG + payload
	op SGSO + payload

and the kernel would handle that internally by calling
	->begin()
	->set_settings()
	->set_sg()
	->set_gso()
	->complete()

So the kernel API is largely already there, while the userspace ABI 
constrains us mightily.

	Jeff




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 20:44   ` Stephen Hemminger
@ 2010-11-15 21:14     ` Ben Hutchings
  2010-11-15 21:14       ` Stephen Hemminger
  0 siblings, 1 reply; 14+ messages in thread
From: Ben Hutchings @ 2010-11-15 21:14 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jeff Garzik, NetDev, David Miller

On Mon, 2010-11-15 at 12:44 -0800, Stephen Hemminger wrote:
[...]
> My views are simple:
> 
> Ethtool needs to be an extension of existing netlink API for interfaces.
>   - handles multiple values per transaction
>   - extensible
[...]

Are you suggesting to send and receive the existing ethtool command and
result structures (with some wrapping) through netlink?  Or some larger
change to the API?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 21:14     ` Ben Hutchings
@ 2010-11-15 21:14       ` Stephen Hemminger
  2010-11-15 21:52         ` Ben Hutchings
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2010-11-15 21:14 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Jeff Garzik, NetDev, David Miller

On Mon, 15 Nov 2010 21:14:02 +0000
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Mon, 2010-11-15 at 12:44 -0800, Stephen Hemminger wrote:
> [...]
> > My views are simple:
> > 
> > Ethtool needs to be an extension of existing netlink API for interfaces.
> >   - handles multiple values per transaction
> >   - extensible
> [...]
> 
> Are you suggesting to send and receive the existing ethtool command and
> result structures (with some wrapping) through netlink?  Or some larger
> change to the API?

The existing ioctl base API should be kept as legacy and something
better developed.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 21:14       ` Stephen Hemminger
@ 2010-11-15 21:52         ` Ben Hutchings
  2010-11-15 22:49           ` Jeff Garzik
  0 siblings, 1 reply; 14+ messages in thread
From: Ben Hutchings @ 2010-11-15 21:52 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jeff Garzik, NetDev, David Miller

On Mon, 2010-11-15 at 13:14 -0800, Stephen Hemminger wrote:
> On Mon, 15 Nov 2010 21:14:02 +0000
> Ben Hutchings <bhutchings@solarflare.com> wrote:
> 
> > On Mon, 2010-11-15 at 12:44 -0800, Stephen Hemminger wrote:
> > [...]
> > > My views are simple:
> > > 
> > > Ethtool needs to be an extension of existing netlink API for interfaces.
> > >   - handles multiple values per transaction
> > >   - extensible
> > [...]
> > 
> > Are you suggesting to send and receive the existing ethtool command and
> > result structures (with some wrapping) through netlink?  Or some larger
> > change to the API?
> 
> The existing ioctl base API should be kept as legacy and something
> better developed.

So you're saying: expose all new operations through netlink (and only
netlink) while keeping the old operations exposed only through ioctl?
That's hardly an improvement as it means ethtool or any other
configuration utility will have to support both APIs indefinitely.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 21:52         ` Ben Hutchings
@ 2010-11-15 22:49           ` Jeff Garzik
  2010-11-15 23:33             ` Thomas Graf
  0 siblings, 1 reply; 14+ messages in thread
From: Jeff Garzik @ 2010-11-15 22:49 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Stephen Hemminger, NetDev, David Miller

On 11/15/2010 04:52 PM, Ben Hutchings wrote:
> On Mon, 2010-11-15 at 13:14 -0800, Stephen Hemminger wrote:
>> On Mon, 15 Nov 2010 21:14:02 +0000
>> Ben Hutchings<bhutchings@solarflare.com>  wrote:
>>
>>> On Mon, 2010-11-15 at 12:44 -0800, Stephen Hemminger wrote:
>>> [...]
>>>> My views are simple:
>>>>
>>>> Ethtool needs to be an extension of existing netlink API for interfaces.
>>>>    - handles multiple values per transaction
>>>>    - extensible
>>> [...]
>>>
>>> Are you suggesting to send and receive the existing ethtool command and
>>> result structures (with some wrapping) through netlink?  Or some larger
>>> change to the API?
>>
>> The existing ioctl base API should be kept as legacy and something
>> better developed.
>
> So you're saying: expose all new operations through netlink (and only
> netlink) while keeping the old operations exposed only through ioctl?
> That's hardly an improvement as it means ethtool or any other
> configuration utility will have to support both APIs indefinitely.

s/only//   I don't think Stephen is suggesting sending _some_ ops 
through netlink and others through old-ioctl.  That's just silly.  Any 
new netlink interface should transit all existing ETHTOOL_xxx 
commands/structures.

But presumably, one would have the ability to send multiple ETHTOOL_xxx 
bundled together into a single netlink transaction, facilitating the 
kernel's calling of struct ethtool_ops'
	->begin()
	... first operation specified by userspace via netlink ...
	... second operation specified by userspace via netlink ...
	... etc.
	->end()

The underlying struct ethtool_ops remains unchanged; you're only 
changing the transit method.

Thus, the ethtool userspace utility would switch entirely to netlink, 
while the ioctl processing code remains for binary compatibility.

Or... ethtool userspace utility could remain unchanged, and a new 
'nictool' utility provides the same features but with (a) a new CLI and 
(b) exclusively uses netlink rather than ioctl.

	Jeff



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 22:49           ` Jeff Garzik
@ 2010-11-15 23:33             ` Thomas Graf
  2010-11-16  0:07               ` Jeff Garzik
                                 ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Thomas Graf @ 2010-11-15 23:33 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Ben Hutchings, Stephen Hemminger, NetDev, David Miller

On Mon, Nov 15, 2010 at 05:49:33PM -0500, Jeff Garzik wrote:
> s/only//   I don't think Stephen is suggesting sending _some_ ops
> through netlink and others through old-ioctl.  That's just silly.
> Any new netlink interface should transit all existing ETHTOOL_xxx
> commands/structures.
> 
> But presumably, one would have the ability to send multiple
> ETHTOOL_xxx bundled together into a single netlink transaction,
> facilitating the kernel's calling of struct ethtool_ops'
> 	->begin()
> 	... first operation specified by userspace via netlink ...
> 	... second operation specified by userspace via netlink ...
> 	... etc.
> 	->end()
> 
> The underlying struct ethtool_ops remains unchanged; you're only
> changing the transit method.
> 
> Thus, the ethtool userspace utility would switch entirely to
> netlink, while the ioctl processing code remains for binary
> compatibility.
> 
> Or... ethtool userspace utility could remain unchanged, and a new
> 'nictool' utility provides the same features but with (a) a new CLI
> and (b) exclusively uses netlink rather than ioctl.

I actually have code for this including userspace. I never submitted
it because I wasn't confident it is the way to go since it literally
duplicates all ethtool code in the kernel.

There is one major problem with bundling multiple requests though. If
one change request fails but other changes have been committed already
we can't really undo them without causing lots of races. We have to
leave the device in a somewhat inconsistent state. It's even difficult
to tell what has been comitted and what hasn't. It also makes error
reporting more difficult as a -ERANGE error code could apply to any
of the values to be changed.

I tried to solve this by splitting the validate/change operation and
thus be able to validate all requests before starting to commit
them. This would mean changing all drivers though which I wasn't
willing to do.

I can clean up what I have and submit it so we have something to
start with.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 23:33             ` Thomas Graf
@ 2010-11-16  0:07               ` Jeff Garzik
  2010-11-16  0:10               ` Ben Hutchings
  2010-11-16  2:02               ` David Miller
  2 siblings, 0 replies; 14+ messages in thread
From: Jeff Garzik @ 2010-11-16  0:07 UTC (permalink / raw)
  To: Ben Hutchings, Stephen Hemminger, NetDev, David Miller

On 11/15/2010 06:33 PM, Thomas Graf wrote:
> On Mon, Nov 15, 2010 at 05:49:33PM -0500, Jeff Garzik wrote:
>> s/only//   I don't think Stephen is suggesting sending _some_ ops
>> through netlink and others through old-ioctl.  That's just silly.
>> Any new netlink interface should transit all existing ETHTOOL_xxx
>> commands/structures.
>>
>> But presumably, one would have the ability to send multiple
>> ETHTOOL_xxx bundled together into a single netlink transaction,
>> facilitating the kernel's calling of struct ethtool_ops'
>> 	->begin()
>> 	... first operation specified by userspace via netlink ...
>> 	... second operation specified by userspace via netlink ...
>> 	... etc.
>> 	->end()
>>
>> The underlying struct ethtool_ops remains unchanged; you're only
>> changing the transit method.
>>
>> Thus, the ethtool userspace utility would switch entirely to
>> netlink, while the ioctl processing code remains for binary
>> compatibility.
>>
>> Or... ethtool userspace utility could remain unchanged, and a new
>> 'nictool' utility provides the same features but with (a) a new CLI
>> and (b) exclusively uses netlink rather than ioctl.
>
> I actually have code for this including userspace. I never submitted
> it because I wasn't confident it is the way to go since it literally
> duplicates all ethtool code in the kernel.
>
> There is one major problem with bundling multiple requests though. If
> one change request fails but other changes have been committed already
> we can't really undo them without causing lots of races. We have to
> leave the device in a somewhat inconsistent state. It's even difficult
> to tell what has been comitted and what hasn't. It also makes error
> reporting more difficult as a -ERANGE error code could apply to any
> of the values to be changed.

Well, what are the range of possibilities for the _hardware_, given the 
current struct ethtool_ops software interface?

We can either reset+restart RXTX after such events, or not.

That's a binary decision, one easily be passed in from userspace before 
any ethtool ops are executed.

Further down the road, if one wanted to travel that far, we could save 
the hardware state at the beginning, and restore hardware state if 
anything fails.  Depends on peoples' motivation over this rare issue.

We already save/restore hardware state for suspend/resume, so this does 
not seem overly onerous.

	Jeff



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 23:33             ` Thomas Graf
  2010-11-16  0:07               ` Jeff Garzik
@ 2010-11-16  0:10               ` Ben Hutchings
  2010-11-16  6:25                 ` Thomas Graf
  2010-11-16  2:02               ` David Miller
  2 siblings, 1 reply; 14+ messages in thread
From: Ben Hutchings @ 2010-11-16  0:10 UTC (permalink / raw)
  To: Thomas Graf; +Cc: Jeff Garzik, Stephen Hemminger, NetDev, David Miller

On Mon, 2010-11-15 at 18:33 -0500, Thomas Graf wrote:
> On Mon, Nov 15, 2010 at 05:49:33PM -0500, Jeff Garzik wrote:
> > s/only//   I don't think Stephen is suggesting sending _some_ ops
> > through netlink and others through old-ioctl.  That's just silly.
> > Any new netlink interface should transit all existing ETHTOOL_xxx
> > commands/structures.
> > 
> > But presumably, one would have the ability to send multiple
> > ETHTOOL_xxx bundled together into a single netlink transaction,
> > facilitating the kernel's calling of struct ethtool_ops'
> > 	->begin()
> > 	... first operation specified by userspace via netlink ...
> > 	... second operation specified by userspace via netlink ...
> > 	... etc.
> > 	->end()
> > 
> > The underlying struct ethtool_ops remains unchanged; you're only
> > changing the transit method.
> > 
> > Thus, the ethtool userspace utility would switch entirely to
> > netlink, while the ioctl processing code remains for binary
> > compatibility.
> > 
> > Or... ethtool userspace utility could remain unchanged, and a new
> > 'nictool' utility provides the same features but with (a) a new CLI
> > and (b) exclusively uses netlink rather than ioctl.
> 
> I actually have code for this including userspace. I never submitted
> it because I wasn't confident it is the way to go since it literally
> duplicates all ethtool code in the kernel.
> 
> There is one major problem with bundling multiple requests though. If
> one change request fails but other changes have been committed already
> we can't really undo them without causing lots of races. We have to
> leave the device in a somewhat inconsistent state. It's even difficult
> to tell what has been comitted and what hasn't. It also makes error
> reporting more difficult as a -ERANGE error code could apply to any
> of the values to be changed.
[...]

I think it's hopeless to make this truly transactional.  Unless the
ethtool core maintains all the settings in one giant structure and
passes them over to the driver to check and apply then there is no way
driver authors are going to get it right in general.  And if the ethtool
core does that then, as you say, error reporting is going to be
terrible.  There will be even more need to go look in the kernel log to
see the driver's explanation of why the settings are invalid which was
too long to fit in this margin^Wreturn code.

I would expect to treat each operation in a multiple-set as conditional
on the success of all previous operations.  ethtool or other utilities
should then take care to put operations in a sensible order (e.g. enable
TX checksum before TSO, if those remain separate operations).  Error
reporting in the core is then as simple as reporting how many operations
were successful plus the error code for the one that failed.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-15 23:33             ` Thomas Graf
  2010-11-16  0:07               ` Jeff Garzik
  2010-11-16  0:10               ` Ben Hutchings
@ 2010-11-16  2:02               ` David Miller
  2010-11-16  6:17                 ` Thomas Graf
  2 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2010-11-16  2:02 UTC (permalink / raw)
  To: tgraf; +Cc: jeff, bhutchings, shemminger, netdev

From: Thomas Graf <tgraf@infradead.org>
Date: Mon, 15 Nov 2010 18:33:35 -0500

> I tried to solve this by splitting the validate/change operation and
> thus be able to validate all requests before starting to commit
> them. This would mean changing all drivers though which I wasn't
> willing to do.

It isn't sufficient.  You can still get into unwindable failures.

Earlier operations can consume fixed resources like TCAM filter
slots or rx/tx queues, making a subsequent operation in the
sequence fail.

A validate/commit scheme cannot detect this effectively.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-16  2:02               ` David Miller
@ 2010-11-16  6:17                 ` Thomas Graf
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Graf @ 2010-11-16  6:17 UTC (permalink / raw)
  To: David Miller; +Cc: jeff, bhutchings, shemminger, netdev

On Mon, Nov 15, 2010 at 06:02:25PM -0800, David Miller wrote:
> It isn't sufficient.  You can still get into unwindable failures.
> 
> Earlier operations can consume fixed resources like TCAM filter
> slots or rx/tx queues, making a subsequent operation in the
> sequence fail.
> 
> A validate/commit scheme cannot detect this effectively.

I agree, there are many more scenarios where it would not work
reliably. It would have ensured that all provided values are
within range boundries and that all requested operations are in
fact supported. Since I have disregarded the idea, I does not
matter much anymore.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: the future of ethtool
  2010-11-16  0:10               ` Ben Hutchings
@ 2010-11-16  6:25                 ` Thomas Graf
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Graf @ 2010-11-16  6:25 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Jeff Garzik, Stephen Hemminger, NetDev, David Miller

On Tue, Nov 16, 2010 at 12:10:30AM +0000, Ben Hutchings wrote:
> I would expect to treat each operation in a multiple-set as conditional
> on the success of all previous operations.  ethtool or other utilities
> should then take care to put operations in a sensible order (e.g. enable
> TX checksum before TSO, if those remain separate operations).  Error
> reporting in the core is then as simple as reporting how many operations
> were successful plus the error code for the one that failed.

This is already not that trivial with current netlink limitations.
We are pretty much limited to a single int when returning error
states. One more reason to rethink current netlink error semantics.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2010-11-16  6:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-15 19:41 the future of ethtool Jeff Garzik
2010-11-15 20:18 ` Ben Hutchings
2010-11-15 20:44   ` Stephen Hemminger
2010-11-15 21:14     ` Ben Hutchings
2010-11-15 21:14       ` Stephen Hemminger
2010-11-15 21:52         ` Ben Hutchings
2010-11-15 22:49           ` Jeff Garzik
2010-11-15 23:33             ` Thomas Graf
2010-11-16  0:07               ` Jeff Garzik
2010-11-16  0:10               ` Ben Hutchings
2010-11-16  6:25                 ` Thomas Graf
2010-11-16  2:02               ` David Miller
2010-11-16  6:17                 ` Thomas Graf
2010-11-15 21:03   ` Jeff Garzik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).