netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.h.duyck@intel.com>
To: Ben Hutchings <bhutchings@solarflare.com>
Cc: Dimitris Michailidis <dm@chelsio.com>,
	Santwona Behera <santwona.behera@sun.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [ethtool PATCH 2/2] Add RX packet classification interface
Date: Mon, 07 Mar 2011 10:43:51 -0800	[thread overview]
Message-ID: <4D752767.9060205@intel.com> (raw)
In-Reply-To: <1299522481.2522.24.camel@bwh-desktop>

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.@location 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?

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?

Thanks,

Alex


  reply	other threads:[~2011-03-07 18:43 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-11  1:18 [ethtool PATCH 0/2] Add support for RX network flow classifier rules Alexander Duyck
2011-02-11  1:18 ` [ethtool PATCH 1/2] Add macro for displaying [value N] formatting to manpage Alexander Duyck
2011-02-21 14:45   ` Ben Hutchings
2011-02-11  1:18 ` [ethtool PATCH 2/2] Add RX packet classification interface Alexander Duyck
2011-02-21 15:40   ` Ben Hutchings
2011-02-22 20:52     ` Alexander Duyck
2011-03-01  0:35       ` Ben Hutchings
2011-03-04 19:09         ` Alexander Duyck
2011-03-07 15:57           ` Ben Hutchings
2011-03-07 17:04             ` Alexander Duyck
2011-03-07 17:57               ` Ben Hutchings
2011-03-07 18:22                 ` Dimitris Michailidis
2011-03-07 18:28                   ` Ben Hutchings
2011-03-07 18:43                     ` Alexander Duyck [this message]
2011-03-07 18:57                       ` Dimitris Michailidis
2011-03-07 19:00                       ` Ben Hutchings
2011-03-07 19:11                         ` Dimitris Michailidis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D752767.9060205@intel.com \
    --to=alexander.h.duyck@intel.com \
    --cc=bhutchings@solarflare.com \
    --cc=dm@chelsio.com \
    --cc=netdev@vger.kernel.org \
    --cc=santwona.behera@sun.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).