Subject: Re: [PKT_SCHED]: Allow using nfmark as key in U32 classifier.
From: jamal <hadi@cyberus.ca>
Reply-To: hadi@cyberus.ca
To: Thomas Graf <tgraf@suug.ch>
Cc: "David S. Miller" <davem@davemloft.net>, netdev@oss.sgi.com
In-Reply-To: <20041231110836.GD32419@postel.suug.ch>
References: <20041228231916.GG32419@postel.suug.ch>
	 <1104277165.1100.291.camel@jzny.localdomain>
	 <20041229000928.GH32419@postel.suug.ch>
	 <1104282811.1090.314.camel@jzny.localdomain>
	 <20041229124842.GI32419@postel.suug.ch>
	 <1104330054.1089.329.camel@jzny.localdomain>
	 <20041229150140.GJ32419@postel.suug.ch>
	 <1104335620.1025.22.camel@jzny.localdomain>
	 <20041230174313.GB32419@postel.suug.ch>
	 <1104469111.1049.219.camel@jzny.localdomain>
	 <20041231110836.GD32419@postel.suug.ch>
Content-Type: text/plain
Organization: jamalopolous
Message-Id: <1104505142.1048.262.camel@jzny.localdomain>
Mime-Version: 1.0
Date: 31 Dec 2004 09:59:02 -0500
Content-Transfer-Encoding: 7bit
Sender: netdev-bounce@oss.sgi.com
Errors-to: netdev-bounce@oss.sgi.com
Precedence: bulk

On Fri, 2004-12-31 at 06:08, Thomas Graf wrote:
> * jamal <1104469111.1049.219.camel@jzny.localdomain> 2004-12-30 23:58

> > perhaps we need a few bits in the selector to identify
> > the owner who installed the rule to begin with. Then it would be safe to
> > interpret the meaning of the ID (by the same app). Did you say there
> > were some unused bits in the selector?
> 
> Right, but why not do this in userspace by having a global map
> somewhere in a file? A u32 config could have been modified by
> multiple pids and it would get really messy to store a pid for
> every possible changeable item.

The "PID" is per app - same as it is for rtm where the different known
routing apps like zebra are given an ID and certain IDs are left blank
for general use. So if you run multiple tcs they will all have the same
ID. The way the id is used in the rtm historically has been for allowing
multiple route protocols to install route entries (this way for example 
OSPF does/nt announce RIP routes etc). In our case this would mean:
"this rule was installed by tc - only it knows what the opaque value
10 means". In that case tc would be responsible to decode 10 which would
mean to it "this is an ip match therefore use ip_print() to pretty
print" and 10 is global to tc only and in that case stored in some
header file belonging to tc. 

> > I think all you need really is to say "this match starts at IP" i.e such
> > a definition is global.
> > handles per rule already exist - and you can actually specify them when
> > installing a rule. Are those insufficient?
> 
> Those are absolutely sufficient. I was thinking of giving a match a
> 16bit ID which can be used for both, identifying and mapping, i.e:
> 
> __u8 kind; /* match type, for lookup in matchers table */

255 possible matchers max?

> __u8 flags; /* Invert Flag + Relations */
> __u16 handle; /* must be unique per selector, may be autogenerated */

Ok this is the one used to store the opaque IDs - unique per app
so may be the same across multiple selectors.
Probably steal a few bits from here and use them in nkind.

> I want to have those matches be as small as possible, so no nested
> TLVs but rather this u32 + matcher specific data form a TLV together.
> 
> A selector consists of a TLV array of such matches. The first TLV,
> type=1 becomes a header with the possibility to transfer classifier
> specific options (such as hash table configuration for u32).

Maybe i misunderstood you. You are going to have:

SEL2
  |
  |
  +--match1
  |    |
  |    + -- extended match1
  .    .
  .    .
  .    .
  .    +---- extnded matchn
  |
  +--- matchn


Why does the first one have to be speacial?

One of the mistakes in u32 is the tight packing of the matches. ie the
match1-n above are packet together. If they were put in TLVs probably
only new thing that will be needed is MATCH2 TLV.
No harm in transporting an extra 32 bits for a TLV - its not like you
are going to have a million matches and need to save space.
So i would suggest everything under a TLV (SEL2->MATCH(ES)->EMATCH(ES))

> > Why not make the always-true to be an extended match? actually a u32
> > match of 0 0 is always true. Those hashes are quiet tricky/flexible;
> > i would rather we clone u32 and call it something else then speacilize
> > it.
> 
> Agreed, I don't want to change u32 but I want to introduce ematches
> in u32 as well so we can benefit from the hashing but for those who
> don't need hashing u32 is already bloat so we can do a simple
> always-true classifier which does nothing more than evaluating the
> ematches. I want to have the u32 match be a ematch as well so the
> always-true classifier would become a u32 alternative but without
> the hashing overhead.

Sounds fine to me. so a u32 match 00 emathes here ..

> It requires the following parameters:
>  - start offset
>  - end offset
>  - pattern
>  - prefix table
> 
> and then will simply start at `start` and scans until `end` looking
> for pattern with the help of the prefix table. Again, I'm not sure what
> you mean by state ;->

A virus would span several packets. So the state/knowldge of whether
something is a virus is spread across several packets. That knowledge
typically needs to be accumulated before making a call. Is this thing
capable of remembering?

cheers,
jamal