Re: [PATCH net-next v4 1/2] net sched actions: Add support for user cookies

On 01/17/2017 05:50 PM, Jamal Hadi Salim wrote:

On 17-01-17 09:16 AM, Daniel Borkmann wrote:

quoted

On 01/17/2017 12:11 PM, Jamal Hadi Salim wrote:

quoted

From: Jamal Hadi Salim <jhs@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.

quoted

  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.

Thanks,
Daniel