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.
... and then ...
};
#define tcf_head common.tcfa_head
#define tcf_index common.tcfa_index
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index f0a0514..e0bc7e8 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -515,4 +515,12 @@ struct tc_cls_bpf_offload {
u32 gen_flags;
};
+
+/* This structure holds cookie structure that is passed from user
+ * to the kernel for actions and classifiers
+ */
+struct tc_cookie {
+ unsigned char ck[TC_COOKIE_MAX_SIZE];
+ unsigned char ck_len;
... embed and make this ...
struct tc_cookie {
u8 *data;
u32 len;
};
as act->act_ck->ck_len is rather funky, compare that to act->cookie->len.
+};
#endif
[...]
@@ -575,6 +583,23 @@ struct tc_action *tcf_action_init_1(struct net
*net, struct nlattr *nla,
if (err < 0)
goto err_mod;
+ if (tb[TCA_ACT_COOKIE]) {
+ if (nla_len(tb[TCA_ACT_COOKIE]) > TC_COOKIE_MAX_SIZE) {
+ err = -EINVAL;
+ goto err_mod;
+ }
+
+ a->act_ck = kzalloc(sizeof(*a->act_ck), GFP_KERNEL);
+ if (unlikely(!a->act_ck)) {
+ err = -ENOMEM;
+ goto err_mod;
+ }
+
+ memcpy(a->act_ck->ck, nla_data(tb[TCA_ACT_COOKIE]),
+ nla_len(tb[TCA_ACT_COOKIE]));
+ a->act_ck->ck_len = nla_len(tb[TCA_ACT_COOKIE]);
Then you can also simplify all this and use nla_memdup() here for
act->cookie->data.
I can do that if you feel strongly. I am dropping the first patch and
hope Dave just applies this first patch.
cheers,
jamal