On 16-02-18 04:14 AM, Jamal Hadi Salim wrote: > On 16-02-17 06:07 PM, John Fastabend wrote: >> [...] >> > >> Actually thinking about this a bit more I wrote this thinking >> that there existed some hardware that actually cared if it was >> a new rule or an existing rule. For me it doesn't matter I do >> the same thing in the new/replace cases I just write into the >> slot on the hardware table and if it happens to have something >> in it well its overwritten e.g. "replaced". This works because >> the cls_u32 layer protects us from doing something unexpected. >> > > You are describing create-or-update which is a reasonable default > BUT: counting on the user to specify the htid+bktid+nodeid > for every filter and knowing what that means is prone to mistakes > when for example (using your big hammer approach right now) they > dont specify the handle and the kernel creates one for them. > > IMO, it would be better at this early stage to enforce the correct > behavior for future generations. > To follow the netlink semantics which a lot of people are already > trained to think in. > > Current netlink behavior is supposed to be: > > 1) NEW ==> "Create". > Ambigous - could mean a)"create if it doesnt exist" or b) "fail if it > exists otherwise create" > Unfortunately different parts of the kernel often assume some > default from either #a or #b. >
But this is already handled by the core cls_api.c code. We never get to u32_change if the flags are not correct. Look at the block right above the op call into the classifiers change() code in cls_api.c. Starting at line 287. > 2) NEW|REPLACE flag ==> "Create if it doesnt exist and replace > if it exists" > > 3)NEW|EXCLUSIVE ==> "Create if it doesnt exist and fail if it > exists" > > 4)NEW|APPEND ==> "just fscking create; i dont care if it exists". > > IOW, just add the flag field which is intepreted from whatever > the user explicitly asks for. And reject say what the hardware > doesnt support. I think I agree with what your saying but I'm fairly sure this is working as you describe. > I have worked with tcams where we support #3. It is a bit inefficient > because you have to check if a rule exists first. And i have worked > in cases where #1 is assumed to mean #2 and at times #4. It is better > user experience to be explicit. Agreed, and I think it is :) > > cheers, > jamal
