Message-ID: <4D752A8C.9090007@chelsio.com>
Date:	Mon, 07 Mar 2011 10:57:16 -0800
From:	Dimitris Michailidis <dm@...lsio.com>
To:	Alexander Duyck <alexander.h.duyck@...el.com>
CC:	Ben Hutchings <bhutchings@...arflare.com>,
	Santwona Behera <santwona.behera@....com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [ethtool PATCH 2/2] Add RX packet classification interface

Alexander Duyck wrote:
> On 3/7/2011 10:28 AM, Ben Hutchings wrote:
>> On Mon, 2011-03-07 at 10:22 -0800, Dimitris Michailidis wrote:
>>> Ben Hutchings wrote:
>>>> On Mon, 2011-03-07 at 09:04 -0800, Alexander Duyck wrote:
>>>>> The only time where location really matters is if you are 
>>>>> attempting to
>>>>> overwrite an existing rule and I am not sure how that would be handled
>>>>> in ntuple anyway since right now adding additional rules via ntuple 
>>>>> for
>>>>> ixgbe just results in duplicate rules being defined.
>>>>
>>>> As I understand it, the location also determines the *priority* for the
>>>> rule.
>>>
>>> This is true, at least for TCAMs.  But it's relevant only when multiple
>>> filters would match a packet.  People often use non-overlapping 
>>> filters, for
>>> these adding the filter at any available slot is OK.
>>
>> Right.  But ethtool would have to determine that the filter was non-
>> overlapping, before ignoring the location.  Also it cannot allow
>> deletion by location if it ever ignores the location on insertion.  We
>> should make the location optional at both the command-line and API
>> level, but never ignore it.
>>
> 
> I wasn't implying that we ignore it for rules inserted via the nfc 
> interface.  Only for those inserted via the ntuple interface.
> 
> My reasoning for that was because it had occurred to me that what my 
> patch series had done is allow for ntuples to be displayed via the 
> get_rx_nfc interface.  As such you would end up with a location being 
> implied when displaying the rules since it would give you a list of n 
> entities.
> 
> If you attempted to restore the rules you would probably end up with the 
> location information for filters 0..(n-1), and that could be dropped 
> since it would just be extra information.
> 
>>>> Which is why I wrote that "@fs.@...ation specifies the index to
>>>> use and must not be ignored."
>>>>
>>>> To support hardware where the filter table is hash-based rather than a
>>>> TCAM, we would need some kind of flag or special value of location that
>>>> means 'wherever'.
>>>
>>> I'd find the 'wherever' option useful for TCAMs too.  Maybe even have 
>>> a few
>>> of those, like 'first available', 'any', and 'last available'.  The 
>>> last one
>>> is quite useful for catch-all rules without requiring one to know the 
>>> TCAM size.
>>
>> Agreed.
>>
>> Ben.
> 
> The first and last options make a lot of sense to me.  The one I'm not 
> sure about would be the "any" option.  It seems like it would be 
> redundant with the "first available" option or is there something I'm 
> missing?

Not really.  I offered it because we were talking about 'wherever' options. 
  I too find 'first' and 'last' to be most useful.

> Also the code I have currently for the user space is just starting at 0 
> and filling in the rules on a first available basis for location not 
> specified.  Is this going to work for most cases or should I look at 
> changing it to something like a "last available" approach for the nfc 
> based filters?

Oh, I hadn't noticed that user space is trying to allocate the index if 
unspecified.  In my case, some filters need to have special indices, eg 
multiple of 4.  User space wouldn't know this.  I think user space needs to 
let the driver allocate indices if the user didn't select one.  I'd default 
to first available.
--
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