Re: [PKT_SCHED]: Allow using nfmark as key in U32 classifier.

[Thomas Graf <tgraf@suug.ch>]

* jamal <1104268498.1090.254.camel@jzny.localdomain> 2004-12-28 16:14
> Whatever you had before is fine for action/policer - with intent to kill
> policer eventually.

I left it in for now but I see no reason why to do so actually. Old
iproute2 binaries should do just fine with the action backward
compatibility code?

> What i objected to is the indev and any other thing that has to do with
> classification helping - thats not where it should fit.
> Take u32 for example: The fit for match extensions is really at the key
> level not at a layer above.
> We need a sel2 which has new keys (which is easy because thats
> transported in a TLV).

Take a look at http://people.suug.ch/~tgr/patches/queue/03_tcf_exts_u32.diff

The extensions are on the same level as the selector. The patchset still
has errors in the patches for route and tcindex since it's non-trivial
to adapt them to allow changing parameter on-the-fly. The rest is tested
and works perfectly fine. I can create a subset or we can just take the
first few patches for now and do the development on u32/fw and port it
later.

> Why not reuse what already exists in terms of classifier/filter return
> codes? They are pretty sufficient and cover all the cases.

I do reuse them. TC_ACT_* from include/linux/pkt_cls.h

> Hrm, so someone writting the one page extension now has to fill in all
> these functions?

No, that's just how the classifier accesses the extensions API.

> [ematch api]

Exactly, this would be API visible to the matches.

> If what you describe above is internal - accessible via classifier then
> fine (other than tcf_exts_match) - lathough it looks excessive.

The validate/change split is needed to implement consistent changes
in classifiers. The current way causes corruption in classifer data
whenever an action configuration fails.

> I dont see these things calling actions. They are interleaved between
> matches. At completion of matches/filtering then you call the action
> code.

Right, tcf_exts_match calls the generic matches and at the very end
the action.

> Whats wrong with extended TLVs you mentioned earlier?
> 
> match u32 ..
> ematch indev ...
> match u32 ...
> ematch meta tcindex ..
> 
> the ematches are essentially TLVs on their own and are owned by
> the classifier. The classifier doesnt know whats in them. It just
> calls generic code to execute them.

They should go into TCA_XXX_EXTS as embeded TLVs. The problem is
not how to do it but rather how far to go. Do we want userspace
to be able to delete a single generic match? Do we want to only
allow replacing all matches? We will hit the limit of skbs at
some point if we keep on encapsulating. ;->

> I think you are only refering to one ematch kind above --> for metadata.

Correct. This would be the generic match for metadata.

> What i talked about is arbitrary (example i could put a quick hack to
> grep strings without writting a full classifier). Essentially what you
> have fits just fine - you may need two ids; one for IDing as meta match
> and other as tcindex etc. The second one can be hidden.

I don't get this.