Mon, Jan 16, 2017 at 08:54:18AM CET, pa...@mellanox.com wrote: > > >On 15/01/2017 21:08, John Fastabend wrote: >> 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.
+1 >> >> 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 >> > >Hi all, >So is everyone ok with leaving the commit as is, only adding user data >to flower for now? I think we should do it in a generic way, for every classifier, right away. Same as Jamal is doing for actions. I think that first we should get Jamal's patch merged and then do the same for classifiers.
