ethtool: missing implementation of n_priv_flags

ethtool: missing implementation of n_priv_flags

[Brandeburg, Jesse]

Was just looking at implementing the driver private flags to add a new feature to 
a driver.

It appears that nothing in the core ethtool.c ever accesses or prints n_priv_flags, 
even if a driver assigns it (which none in the kernel currently do)

Is this just an oversight?

Re: ethtool: missing implementation of n_priv_flags

[Ben Hutchings]

Brandeburg, Jesse wrote:
> Was just looking at implementing the driver private flags to add a new feature to 
> a driver.
> 
> It appears that nothing in the core ethtool.c ever accesses or prints n_priv_flags, 
> even if a driver assigns it (which none in the kernel currently do)
[...]

You mean net/core/ethtool.c?  Or the ethtool utility?

If you set it in your get_drvinfo() operation then it will be copied out.
As for the ethtool utility, I've implemented some generic flag parsing and
printing functionality so it shouldn't be too hard to use that for private
flags.

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.

Re: ethtool: missing implementation of n_priv_flags

[Brandeburg, Jesse]

On Thu, 28 Oct 2010, Ben Hutchings wrote:
> Brandeburg, Jesse wrote:
> > Was just looking at implementing the driver private flags to add a new feature to 
> > a driver.
> > 
> > It appears that nothing in the core ethtool.c ever accesses or prints n_priv_flags, 
> > even if a driver assigns it (which none in the kernel currently do)
> [...]
> 
> You mean net/core/ethtool.c?  Or the ethtool utility?

The ethtool utility (the app that would print the 1-1 match between bits 
and strings)
 
> If you set it in your get_drvinfo() operation then it will be copied out.
> As for the ethtool utility, I've implemented some generic flag parsing and
> printing functionality so it shouldn't be too hard to use that for private
> flags.

Right, I see the code where it gets copied into drvinfo in the kernel but 
nothing ever prints it on the userspace side.

We'll take a shot at an implementation in the ethtool proper and post it 
(hopefully soon).  I imagine it will just be printed when one runs the 
command 
# ethtool ethX

and the set side will probably be implemented as part of -s

# ethtool -s ethX pflag [0-0xFFFFFFFF]

Re: ethtool: missing implementation of n_priv_flags

[Jeff Garzik]

On Thu, Oct 28, 2010 at 01:34:34PM -0700, Brandeburg, Jesse wrote:
> Was just looking at implementing the driver private flags to add a new feature to 
> a driver.
> 
> It appears that nothing in the core ethtool.c ever accesses or prints n_priv_flags, 
> even if a driver assigns it (which none in the kernel currently do)
> 
> Is this just an oversight?

It was waiting for the first user, basically...  I created an example
kernel patch for one Intel driver, but you guys didn't seem interested
in it at the time.  Glad to see it's getting some attention.

	Jeff

Re: ethtool: missing implementation of n_priv_flags

[Ben Hutchings]

Brandeburg, Jesse wrote:
[...]
> We'll take a shot at an implementation in the ethtool proper and post it 
> (hopefully soon).  I imagine it will just be printed when one runs the 
> command 
> # ethtool ethX
> 
> and the set side will probably be implemented as part of -s
> 
> # ethtool -s ethX pflag [0-0xFFFFFFFF]

This is crap.  Use ETHTOOL_GSTRINGS with string_set = ETH_SS_PRIV_FLAGS
to get the flag names, then convert that array into an array of struct
cmdline_info and parse the flags by name.

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.

Re: ethtool: missing implementation of n_priv_flags

[Jeff Garzik]

On Thu, Oct 28, 2010 at 10:54:18PM +0100, Ben Hutchings wrote:
> Brandeburg, Jesse wrote:
> [...]
> > We'll take a shot at an implementation in the ethtool proper and post it 
> > (hopefully soon).  I imagine it will just be printed when one runs the 
> > command 
> > # ethtool ethX
> > 
> > and the set side will probably be implemented as part of -s
> > 
> > # ethtool -s ethX pflag [0-0xFFFFFFFF]
> 
> This is crap.  Use ETHTOOL_GSTRINGS with string_set = ETH_SS_PRIV_FLAGS
> to get the flag names, then convert that array into an array of struct
> cmdline_info and parse the flags by name.

Indeed.  It was intended to be a flexible interface where a driver can
easily pass arbitrary text-named flags to the user for setting/clearing.

If e1000e has a special Intel-specific feature that makes the NIC
process packets more rapidly, you could select the string "go_faster" in
the ethtool private flags interface.  The ethtool utility reads the
strings, which determine the flags exported for that network interface.

	Jeff