Date:	Tue, 3 May 2011 20:10:14 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Dimitris Michailidis <dm@...lsio.com>
Cc:	Alexander Duyck <alexander.h.duyck@...el.com>,
	Ben Hutchings <bhutchings@...arflare.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [ethtool PATCH 4/4] v5 Add RX packet classification interface

On Tue, May 3, 2011 at 6:35 PM, Dimitris Michailidis <dm@...lsio.com> wrote:
> On 05/03/2011 05:29 PM, Alexander Duyck wrote:
>> The thing to keep in mind is that the index doesn't have to be a hardware
>> index.  In ixgbe we have a field in the hardware which is meant to just be a
>> unique software index and that is what I am using as the location field for
>> our filters.  All the location information in the rules really provides is a
>> logical way of tracking rules.  It doesn't necessarily have to represent the
>> physical location of the rule in hardware.
>
> I appreciate the intent but there are couple problems.
> a) ethtool.h documents location as
>
>  * @location: Index of filter in hardware table
>
> i.e., physical location.  But we could change that.

I will probably want to change that.  The fact is as I recall even niu
was using a hash in addition to the location index.  As such it isn't
really the true location in the hardware since there is a second piece
to determining the actual location in hardware.

> b) for TCAMs physical location is important and if ethtool offers to provide
> only a logical index and massages the original user input to do so where
> will a driver get the physical location it ultimately needs?  For a device
> with physical indices a multiple of 4 the logical index ethtool picks will
> very frequently be illegal as physical location.  E.g., if location 1 is
> available ethtool will keep selecting it and the driver will need to deal
> with these requests without the benefit of knowing what the user really
> asked for.
>
> Another problem with ethtool selecting locations is it assumes it's the sole
> allocator (I think there's a comment in the code pointing this out).  But
> this isn't a valid assumption, e.g., HW RFS comes to mind as another
> allocator.

The other thing to keep in mind is that this doesn't preclude the
option of adding an ethtool command at some point in the future for
handling the determination of filter location.  The fact is all that
would need to be done is to add an extra ioctl call to determine the
location based on the filter and if that returns op not supported we
fall back to the current rule manager.

The way I have things setup now provides a good foundation in
user-space for managing the rules.  I fully admit it doesn't fit all
solutions, and I welcome follow-on patches to add extra functionality,
but at this time I really would prefer avoiding adding extra
functionality for yet to be implemented features in device drivers.
The ntuple display functionality provides a good example of why I
would prefer to avoid it.  Everything looked like it should have
worked when get_rx_ntuple was implemented in the device drivers, but
as soon as I implemented a get_rx_ntuple for ixgbe I quickly
discovered it didn't work and as such I am now stuck moving everything
over to network flow classifier for ixgbe.

Thanks,

Alex
--
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