Subject: Re: [iproute2 PATCH v2 1/1] tc simple action update and breakage

From: Daniel Borkmann <[email-masked]>
Date: Tue, 24 May 2016 00:39:19 +0200

On 05/08/2016 05:02 PM, Jamal Hadi Salim wrote:
> From: Jamal Hadi Salim <[email-masked]>
>
> Brings it closer to more serious actions (adding branching
> and allowing for late binding)
>
> Unfortunately this breaks old syntax of the simple action.
> But because simple is a pedagogical example unlikely to be used
> in production environments (i.e its role is to serve as an example
> on how to write actions), then this is ok.
>
> New syntax for simple has new keyword "sdata". Example usage is:
>
> sudo tc actions add action simple sdata "foobar" index 1
> or
> tc filter add dev $DEV parent ffff: protocol ip prio 1 u32\
> match ip dst 17.0.0.1/32 flowid 1:10 action simple sdata "foobar"
>
> Signed-off-by: Jamal Hadi Salim <[email-masked]>
[...]

>   static void usage(void)
> @@ -99,56 +100,85 @@ parse_simple(struct action_util *a, int *argc_p, char ***argv_p, int tca_id,
>   	struct tc_defact sel = {};
>   	int argc = *argc_p;
>   	char **argv = *argv_p;
> -	int ok = 0;
> +	int ok = 0, maybe_bind = 0;
>   	struct rtattr *tail;
>   	char *simpdata = NULL;
>
[...]

>   	if (argc) {
>   		if (matches(*argv, "index") == 0) {
>   			NEXT_ARG();
>   			if (get_u32(&sel.index, *argv, 10)) {
> -				fprintf(stderr, "simple: Illegal \"index\"\n");
> +				fprintf(stderr, "simple: Illegal \"index\"\n",
> +					*argv);
>   				return -1;
>   			}

Btw, this causes:

m_simple.c: In function ‘parse_simple’:
m_simple.c:154:6: warning: too many arguments for format [-Wformat-extra-args]
       *argv);
       ^
m_simple.c:103:14: warning: unused variable ‘maybe_bind’ [-Wunused-variable]
   int ok = 0, maybe_bind = 0;
               ^