Tue, Jan 17, 2017 at 05:57:58PM CET, dan...@iogearbox.net wrote:
>On 01/17/2017 05:50 PM, Jamal Hadi Salim wrote:
>> On 17-01-17 09:16 AM, Daniel Borkmann wrote:
>> > On 01/17/2017 12:11 PM, Jamal Hadi Salim wrote:
>> > > From: Jamal Hadi Salim <j...@mojatatu.com>
>> > >
>> > > Introduce optional 128-bit action cookie.
>> > > Like all other cookie schemes in the networking world (eg in protocols
>> > > like http or existing kernel fib protocol field, etc) the idea is to save
>> > > user state that when retrieved serves as a correlator. The kernel
>> > > _should not_ intepret it. The user can store whatever they wish in the
>> > > 128 bits.
>> > [...]
>> >
>> > Since it looks like you need a v5 anyway, few comments below.
>> >
>> > > include/net/act_api.h | 1 +
>> > > include/net/pkt_cls.h | 8 ++++++++
>> > > include/uapi/linux/pkt_cls.h | 3 +++
>> > > net/sched/act_api.c | 25 +++++++++++++++++++++++++
>> > > 4 files changed, 37 insertions(+)
>> > >
>> > > diff --git a/include/net/act_api.h b/include/net/act_api.h
>> > > index 1d71644..0692458 100644
>> > > --- a/include/net/act_api.h
>> > > +++ b/include/net/act_api.h
>> > > @@ -41,6 +41,7 @@ struct tc_action {
>> > > struct rcu_head tcfa_rcu;
>> > > struct gnet_stats_basic_cpu __percpu *cpu_bstats;
>> > > struct gnet_stats_queue __percpu *cpu_qstats;
>> > > + struct tc_cookie *act_ck;
>> >
>> > Since we know anyway that this is part of struct tc_action, can't
>> > you just give this some real/readable name like ...
>> >
>> > struct tc_cookie cookie;
>>
>> Grep-ability.
>> I was worried about when the classifier adds its cookie it
>> would need to use something like cls_cookie etc.
>
>Given this cookie is just used for correlation in user space anyway
>and not processed any further by the kernel, I think we can well
>handle these very few spots, so would be better if the code is more
>maintainable instead.
I agree with Daniel. His naming change suggestions make sense.
In fact, Jamal, now I know why there are names like this all over TC :)
