Re: [ethtool PATCH 4/4] v5 Add RX packet classification interface

On Tue, May 3, 2011 at 6:35 PM, Dimitris Michailidis [off-list ref] wrote:

On 05/03/2011 05:29 PM, Alexander Duyck wrote:

quoted

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