On 17-01-15 09:36 AM, Paul Blakey wrote: > > > On 08/01/2017 19:12, Jiri Pirko wrote: >> Mon, Jan 02, 2017 at 03:59:49PM CET, j...@mojatatu.com wrote: >>> >>> We have been using a cookie as well for actions (which we have been >>> using but have been too lazy to submit so far). I am going to port >>> it over to the newer kernels and post it. >> >> Hard to deal with something we can't look at :) >> >> >>> In our case that is intended to be opaque to the kernel i.e kernel >>> never inteprets it; in that case it is similar to the kernel >>> FIB protocol field. >> >> In case of this patch, kernel also never interprets it. What makes you >> think otherwise. Bot kernel, it is always a binary blob. >> >> >>> >>> In your case - could this cookie have been a class/flowid >>> (a 32 bit)? >>> And would it not make more sense for it the cookie to be >>> generic to all classifiers? i.e why is it specific to flower? >> >> Correct, makes sense to have it generic for all cls and perhaps also >> acts. >> >> > > Hi all, > I've started implementing a general cookie for all classifiers, > I added the cookie on tcf_proto struct but then I realized that it won't work > as > I want. What I want is cookie per internal element (those that are identified > by > handles), which we can have many per one tcf_proto: > > tc filter add dev <DEV> parent ffff: prio 1 cookie 0x123 basic action drop > tc filter add dev <DEV> parent ffff: prio 1 cookie 0x456 "port6" basic action > drop > tc filter add dev <DEV> parent ffff: prio 1 cookie 0x777 basic action drop > > So there is three options to do that: > 1) Implement it in each classifier, using some generic function. (kinda like > stats, where classifiler_dump() calls tcf_exts_dump_stats()) > 2) Have a global hash table for cookies [prio,handle]->[cookie] > 3) Have it on flower only for now :) > > With the first one we will have to change each classifier (or leave it > unsupported as default). > Second is less code to change (instead of changing each classifier), but might > be slower. I'm afraid how it will affect dumping several filters. > Third is this patch. > > Thanks, > Paul.
Avoid (2) it creates way more problems than its worth like is it locked/not locked, how is it synced, collisions, etc. Seems way overkill. I like the generic functionality of (1) but unless we see this pop up in different filters I wouldn't require it for now. If you just do it in flower (option 3) when its added to another classifier we can generalize it. As a middle ground creating nice helper routines as needed goes a long way. .John
