* [PATCH iproute2 1/1] actions: Add support for user cookies
@ 2017-03-21 20:35 Jamal Hadi Salim
2017-03-21 21:22 ` Stephen Hemminger
0 siblings, 1 reply; 10+ messages in thread
From: Jamal Hadi Salim @ 2017-03-21 20:35 UTC (permalink / raw)
To: stephen; +Cc: netdev, Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
Make use of 128b user cookies
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.
Sample exercise(showing variable length use of cookie)
.. create an accept action with cookie a1b2c3d4
sudo $TC actions add action ok index 1 cookie a1b2c3d4
.. dump all gact actions..
sudo $TC -s actions ls action gact
action order 0: gact action pass
random type none pass val 0
index 1 ref 1 bind 0 installed 5 sec used 5 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
cookie a1b2c3d4
.. bind the accept action to a filter..
sudo $TC filter add dev lo parent ffff: protocol ip prio 1 \
u32 match ip dst 127.0.0.1/32 flowid 1:1 action gact index 1
... send some traffic..
$ ping 127.0.0.1 -c 3
PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.020 ms
64 bytes from 127.0.0.1: icmp_seq=2 ttl=64 time=0.027 ms
64 bytes from 127.0.0.1: icmp_seq=3 ttl=64 time=0.038 ms
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
tc/m_action.c | 44 ++++++++++++++++++++++++++++++++++++++------
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/tc/m_action.c b/tc/m_action.c
index 05ef07e..9d5857c 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -150,18 +150,19 @@ new_cmd(char **argv)
}
-int
-parse_action(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n)
+int parse_action(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n)
{
int argc = *argc_p;
char **argv = *argv_p;
struct rtattr *tail, *tail2;
char k[16];
+ int act_ck_len = 0;
int ok = 0;
int eap = 0; /* expect action parameters */
int ret = 0;
int prio = 0;
+ unsigned char act_ck[TC_COOKIE_MAX_SIZE];
if (argc <= 0)
return -1;
@@ -215,16 +216,39 @@ done0:
addattr_l(n, MAX_MSG, ++prio, NULL, 0);
addattr_l(n, MAX_MSG, TCA_ACT_KIND, k, strlen(k) + 1);
- ret = a->parse_aopt(a, &argc, &argv, TCA_ACT_OPTIONS, n);
+ ret = a->parse_aopt(a, &argc, &argv, TCA_ACT_OPTIONS,
+ n);
if (ret < 0) {
fprintf(stderr, "bad action parsing\n");
goto bad_val;
}
+
+ if (*argv && strcmp(*argv, "cookie") == 0) {
+ int slen;
+
+ NEXT_ARG();
+ slen = strlen(*argv);
+ if (slen > (TC_COOKIE_MAX_SIZE*2))
+ invarg("cookie cannot exceed %d\n",
+ *argv);
+
+ if (hex2mem(*argv, act_ck, slen/2) < 0)
+ invarg("cookie must be a hex string\n",
+ *argv);
+
+ act_ck_len = slen;
+ argc--;
+ argv++;
+ }
+
+ if (act_ck_len)
+ addattr_l(n, MAX_MSG, TCA_ACT_COOKIE,
+ (const void *)&act_ck, act_ck_len);
+
tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
ok++;
}
-
}
if (eap > 0) {
@@ -245,8 +269,7 @@ bad_val:
return -1;
}
-static int
-tc_print_one_action(FILE *f, struct rtattr *arg)
+static int tc_print_one_action(FILE *f, struct rtattr *arg)
{
struct rtattr *tb[TCA_ACT_MAX + 1];
@@ -274,8 +297,17 @@ tc_print_one_action(FILE *f, struct rtattr *arg)
return err;
if (show_stats && tb[TCA_ACT_STATS]) {
+
fprintf(f, "\tAction statistics:\n");
print_tcstats2_attr(f, tb[TCA_ACT_STATS], "\t", NULL);
+ if (tb[TCA_ACT_COOKIE]) {
+ int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
+ char b1[strsz+1];
+
+ fprintf(f, "\n\tcookie len %d %s ", strsz,
+ hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
+ strsz, b1, sizeof(b1)));
+ }
fprintf(f, "\n");
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH iproute2 1/1] actions: Add support for user cookies
2017-03-21 20:35 [PATCH iproute2 1/1] actions: Add support for user cookies Jamal Hadi Salim
@ 2017-03-21 21:22 ` Stephen Hemminger
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2017-03-21 21:22 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: netdev
Minor style issues.
> + if (*argv && strcmp(*argv, "cookie") == 0) {
> + int slen;
slen is strlen() and that returns size_t not int.
> +
> + NEXT_ARG();
> + slen = strlen(*argv);
> + if (slen > (TC_COOKIE_MAX_SIZE*2))
No extra (), and space around *
> + invarg("cookie cannot exceed %d\n",
> + *argv);
> +
> + if (hex2mem(*argv, act_ck, slen/2) < 0)
Space around / operator
> + invarg("cookie must be a hex string\n",
> + *argv);
> +
> + act_ck_len = slen;
> + argc--;
> + argv++;
> + }
> +
> + if (act_ck_len)
> + addattr_l(n, MAX_MSG, TCA_ACT_COOKIE,
> + (const void *)&act_ck, act_ck_len);
Cast to void * is not necessary.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH iproute2 1/1] actions: Add support for user cookies
@ 2017-04-22 12:36 Jamal Hadi Salim
2017-04-23 16:11 ` Stephen Hemminger
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Jamal Hadi Salim @ 2017-04-22 12:36 UTC (permalink / raw)
To: stephen; +Cc: netdev, Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
Make use of 128b user cookies
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.
Sample exercise(showing variable length use of cookie)
.. create an accept action with cookie a1b2c3d4
sudo $TC actions add action ok index 1 cookie a1b2c3d4
.. dump all gact actions..
sudo $TC -s actions ls action gact
action order 0: gact action pass
random type none pass val 0
index 1 ref 1 bind 0 installed 5 sec used 5 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
cookie a1b2c3d4
.. bind the accept action to a filter..
sudo $TC filter add dev lo parent ffff: protocol ip prio 1 \
u32 match ip dst 127.0.0.1/32 flowid 1:1 action gact index 1
... send some traffic..
$ ping 127.0.0.1 -c 3
PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.020 ms
64 bytes from 127.0.0.1: icmp_seq=2 ttl=64 time=0.027 ms
64 bytes from 127.0.0.1: icmp_seq=3 ttl=64 time=0.038 ms
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
tc/m_action.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 43 insertions(+), 6 deletions(-)
diff --git a/tc/m_action.c b/tc/m_action.c
index 05ef07e..6ebe85e 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -150,18 +150,19 @@ new_cmd(char **argv)
}
-int
-parse_action(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n)
+int parse_action(int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n)
{
int argc = *argc_p;
char **argv = *argv_p;
struct rtattr *tail, *tail2;
char k[16];
+ int act_ck_len = 0;
int ok = 0;
int eap = 0; /* expect action parameters */
int ret = 0;
int prio = 0;
+ unsigned char act_ck[TC_COOKIE_MAX_SIZE];
if (argc <= 0)
return -1;
@@ -215,16 +216,44 @@ done0:
addattr_l(n, MAX_MSG, ++prio, NULL, 0);
addattr_l(n, MAX_MSG, TCA_ACT_KIND, k, strlen(k) + 1);
- ret = a->parse_aopt(a, &argc, &argv, TCA_ACT_OPTIONS, n);
+ ret = a->parse_aopt(a, &argc, &argv, TCA_ACT_OPTIONS,
+ n);
if (ret < 0) {
fprintf(stderr, "bad action parsing\n");
goto bad_val;
}
+
+ if (*argv && strcmp(*argv, "cookie") == 0) {
+ size_t slen;
+
+ NEXT_ARG();
+ slen = strlen(*argv);
+ if (slen > TC_COOKIE_MAX_SIZE * 2) {
+ char cookie_err_m[128];
+
+ snprintf(cookie_err_m, 128,
+ "%zd Max allowed size %d",
+ slen, TC_COOKIE_MAX_SIZE*2);
+ invarg(cookie_err_m, *argv);
+ }
+
+ if (hex2mem(*argv, act_ck, slen / 2) < 0)
+ invarg("cookie must be a hex string\n",
+ *argv);
+
+ act_ck_len = slen;
+ argc--;
+ argv++;
+ }
+
+ if (act_ck_len)
+ addattr_l(n, MAX_MSG, TCA_ACT_COOKIE,
+ &act_ck, act_ck_len);
+
tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
ok++;
}
-
}
if (eap > 0) {
@@ -245,8 +274,7 @@ bad_val:
return -1;
}
-static int
-tc_print_one_action(FILE *f, struct rtattr *arg)
+static int tc_print_one_action(FILE *f, struct rtattr *arg)
{
struct rtattr *tb[TCA_ACT_MAX + 1];
@@ -274,8 +302,17 @@ tc_print_one_action(FILE *f, struct rtattr *arg)
return err;
if (show_stats && tb[TCA_ACT_STATS]) {
+
fprintf(f, "\tAction statistics:\n");
print_tcstats2_attr(f, tb[TCA_ACT_STATS], "\t", NULL);
+ if (tb[TCA_ACT_COOKIE]) {
+ int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
+ char b1[strsz+1];
+
+ fprintf(f, "\n\tcookie len %d %s ", strsz,
+ hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
+ strsz, b1, sizeof(b1)));
+ }
fprintf(f, "\n");
}
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH iproute2 1/1] actions: Add support for user cookies
2017-04-22 12:36 Jamal Hadi Salim
@ 2017-04-23 16:11 ` Stephen Hemminger
2017-04-23 17:18 ` Jamal Hadi Salim
2017-11-23 16:36 ` Jiri Pirko
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2017-04-23 16:11 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: netdev
On Sat, 22 Apr 2017 08:36:23 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
>
> Make use of 128b user cookies
>
> 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.
>
> Sample exercise(showing variable length use of cookie)
>
> .. create an accept action with cookie a1b2c3d4
> sudo $TC actions add action ok index 1 cookie a1b2c3d4
>
> .. dump all gact actions..
> sudo $TC -s actions ls action gact
>
> action order 0: gact action pass
> random type none pass val 0
> index 1 ref 1 bind 0 installed 5 sec used 5 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> cookie a1b2c3d4
>
> .. bind the accept action to a filter..
> sudo $TC filter add dev lo parent ffff: protocol ip prio 1 \
> u32 match ip dst 127.0.0.1/32 flowid 1:1 action gact index 1
>
> ... send some traffic..
> $ ping 127.0.0.1 -c 3
> PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
> 64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.020 ms
> 64 bytes from 127.0.0.1: icmp_seq=2 ttl=64 time=0.027 ms
> 64 bytes from 127.0.0.1: icmp_seq=3 ttl=64 time=0.038 ms
>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Applied. Please update man page as well.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH iproute2 1/1] actions: Add support for user cookies
2017-04-22 12:36 Jamal Hadi Salim
2017-04-23 16:11 ` Stephen Hemminger
@ 2017-11-23 16:36 ` Jiri Pirko
2017-11-23 17:32 ` Jamal Hadi Salim
2017-11-23 21:09 ` Stephen Hemminger
2017-11-23 21:12 ` Stephen Hemminger
3 siblings, 1 reply; 10+ messages in thread
From: Jiri Pirko @ 2017-11-23 16:36 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: stephen, netdev
Sat, Apr 22, 2017 at 02:36:23PM CEST, jhs@mojatatu.com wrote:
[...]
>@@ -274,8 +302,17 @@ tc_print_one_action(FILE *f, struct rtattr *arg)
> return err;
>
> if (show_stats && tb[TCA_ACT_STATS]) {
>+
> fprintf(f, "\tAction statistics:\n");
> print_tcstats2_attr(f, tb[TCA_ACT_STATS], "\t", NULL);
>+ if (tb[TCA_ACT_COOKIE]) {
>+ int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
>+ char b1[strsz+1];
>+
>+ fprintf(f, "\n\tcookie len %d %s ", strsz,
>+ hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
>+ strsz, b1, sizeof(b1)));
>+ }
Jamal, is there any particular reason that you print cookie only in case
you show stats? What is the relation between cookie and stats?
> fprintf(f, "\n");
> }
>
>--
>1.9.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH iproute2 1/1] actions: Add support for user cookies
2017-11-23 16:36 ` Jiri Pirko
@ 2017-11-23 17:32 ` Jamal Hadi Salim
2017-11-23 17:55 ` Jiri Pirko
0 siblings, 1 reply; 10+ messages in thread
From: Jamal Hadi Salim @ 2017-11-23 17:32 UTC (permalink / raw)
To: Jiri Pirko; +Cc: stephen, netdev
On 17-11-23 11:36 AM, Jiri Pirko wrote:
> Sat, Apr 22, 2017 at 02:36:23PM CEST, jhs@mojatatu.com wrote:
>
> [...]
>
[..]
>
> Jamal, is there any particular reason that you print cookie only in case
> you show stats? What is the relation between cookie and stats?
-s has been thus far used to imply "verbose" on. Other than that
no good reason.
cheers,
jamal
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH iproute2 1/1] actions: Add support for user cookies
2017-11-23 17:32 ` Jamal Hadi Salim
@ 2017-11-23 17:55 ` Jiri Pirko
0 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2017-11-23 17:55 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: stephen, netdev
Thu, Nov 23, 2017 at 06:32:38PM CET, jhs@mojatatu.com wrote:
>On 17-11-23 11:36 AM, Jiri Pirko wrote:
>> Sat, Apr 22, 2017 at 02:36:23PM CEST, jhs@mojatatu.com wrote:
>>
>> [...]
>>
>
>[..]
>>
>> Jamal, is there any particular reason that you print cookie only in case
>> you show stats? What is the relation between cookie and stats?
>
>-s has been thus far used to imply "verbose" on. Other than that
:O Afaik, "-d" is used for that in iproute2 suite.
>no good reason.
Ok, I will move it out.
>
>cheers,
>jamal
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH iproute2 1/1] actions: Add support for user cookies
2017-04-22 12:36 Jamal Hadi Salim
2017-04-23 16:11 ` Stephen Hemminger
2017-11-23 16:36 ` Jiri Pirko
@ 2017-11-23 21:09 ` Stephen Hemminger
2017-11-23 21:12 ` Stephen Hemminger
3 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2017-11-23 21:09 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: netdev
On Sat, 22 Apr 2017 08:36:23 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> +
> + if (*argv && strcmp(*argv, "cookie") == 0) {
> + size_t slen;
> +
> + NEXT_ARG();
> + slen = strlen(*argv);
> + if (slen > TC_COOKIE_MAX_SIZE * 2) {
> + char cookie_err_m[128];
> +
> + snprintf(cookie_err_m, 128,
> + "%zd Max allowed size %d",
> + slen, TC_COOKIE_MAX_SIZE*2);
> + invarg(cookie_err_m, *argv);
Don't bother going to effort of building a string for invarg(). Instead either
just print invalid cookie or use fprintf.
Also, what if kernel limits change?
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH iproute2 1/1] actions: Add support for user cookies
2017-04-22 12:36 Jamal Hadi Salim
` (2 preceding siblings ...)
2017-11-23 21:09 ` Stephen Hemminger
@ 2017-11-23 21:12 ` Stephen Hemminger
3 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2017-11-23 21:12 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: netdev
On Sat, 22 Apr 2017 08:36:23 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> +
> fprintf(f, "\tAction statistics:\n");
> print_tcstats2_attr(f, tb[TCA_ACT_STATS], "\t", NULL);
> + if (tb[TCA_ACT_COOKIE]) {
> + int strsz = RTA_PAYLOAD(tb[TCA_ACT_COOKIE]);
> + char b1[strsz+1];
> +
> + fprintf(f, "\n\tcookie len %d %s ", strsz,
> + hexstring_n2a(RTA_DATA(tb[TCA_ACT_COOKIE]),
> + strsz, b1, sizeof(b1)))
I don't think you need to print len here. Already know that by the output of
hexstring. Since payload length is in bytes and hex takes two characters per byte
then the buffer needs to be bigger (like TCPCOOKIE_MAX_SIZE * 2 + 1)
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-11-23 21:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-21 20:35 [PATCH iproute2 1/1] actions: Add support for user cookies Jamal Hadi Salim
2017-03-21 21:22 ` Stephen Hemminger
-- strict thread matches above, loose matches on Subject: below --
2017-04-22 12:36 Jamal Hadi Salim
2017-04-23 16:11 ` Stephen Hemminger
2017-04-23 17:18 ` Jamal Hadi Salim
2017-11-23 16:36 ` Jiri Pirko
2017-11-23 17:32 ` Jamal Hadi Salim
2017-11-23 17:55 ` Jiri Pirko
2017-11-23 21:09 ` Stephen Hemminger
2017-11-23 21:12 ` Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).