[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7b5d610b-0fd6-d466-cd6d-bb2725397cdd@gmail.com>
Date: Tue, 22 Jun 2021 09:39:31 -0600
From: David Ahern <dsahern@...il.com>
To: Asbjørn Sloth Tønnesen <asbjorn@...jorn.st>,
netdev@...r.kernel.org, Jamal Hadi Salim <jhs@...atatu.com>,
Cong Wang <xiyou.wangcong@...il.com>
Cc: Stephen Hemminger <stephen@...workplumber.org>,
Jiri Pirko <jiri@...dia.com>
Subject: Re: [PATCH iproute2 v3 2/2] tc: pedit: add decrement operation
[ looks fine to me; adding tc folks to make sure they see it ]
On 6/18/21 10:06 AM, Asbjørn Sloth Tønnesen wrote:
> Implement a decrement operation for ttl and hoplimit.
>
> Since this is just syntactic sugar, it goes that:
>
> tc filter add ... action pedit ex munge ip ttl dec ...
> tc filter add ... action pedit ex munge ip6 hoplimit dec ...
>
> is just a more readable version of this:
>
> tc filter add ... action pedit ex munge ip ttl add 0xff ...
> tc filter add ... action pedit ex munge ip6 hoplimit add 0xff ...
>
> This feature was suggested by some pseudo tc examples in Mellanox's
> documentation[1], but wasn't present in neither their mlnx-iproute2
> nor iproute2.
>
> Tested with skip_sw on Mellanox ConnectX-6 Dx.
>
> [1] https://docs.mellanox.com/pages/viewpage.action?pageId=47033989
>
> v3:
> - Use dedicated flags argument in parse_cmd() (David Ahern)
> - Minor rewording of the man page
>
> v2:
> - Fix whitespace issue (Stephen Hemminger)
> - Add to usage info in explain()
>
> Signed-off-by: Asbjørn Sloth Tønnesen <asbjorn@...jorn.st>
> ---
> man/man8/tc-pedit.8 | 8 +++++++-
> tc/m_pedit.c | 25 +++++++++++++++++++------
> tc/m_pedit.h | 4 ++++
> tc/p_ip.c | 2 +-
> tc/p_ip6.c | 2 +-
> 5 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8
> index 376ad4a8..15159ddd 100644
> --- a/man/man8/tc-pedit.8
> +++ b/man/man8/tc-pedit.8
> @@ -77,6 +77,7 @@ pedit - generic packet editor action
> .IR VAL " | "
> .BR add
> .IR VAL " | "
> +.BR decrement " | "
> .BR preserve " } [ " retain
> .IR RVAL " ]"
>
> @@ -96,7 +97,7 @@ chosen automatically based on the header field size.
> .B ex
> Use extended pedit.
> .I EXTENDED_LAYERED_OP
> -and the add
> +and the add/decrement
> .I CMD_SPEC
> are allowed only in this mode.
> .TP
> @@ -288,6 +289,11 @@ is defined by the size of the addressed header field in
> .IR EXTENDED_LAYERED_OP .
> This operation is supported only for extended layered op.
> .TP
> +.BI decrement
> +Decrease the addressed data by one.
> +This operation is supported only for
> +.BR ip " " ttl " and " ip6 " " hoplimit "."
> +.TP
> .B preserve
> Keep the addressed data as is.
> .TP
> diff --git a/tc/m_pedit.c b/tc/m_pedit.c
> index b745c379..54949e43 100644
> --- a/tc/m_pedit.c
> +++ b/tc/m_pedit.c
> @@ -41,7 +41,7 @@ static void explain(void)
> "\t\tATC:= at <atval> offmask <maskval> shift <shiftval>\n"
> "\t\tNOTE: offval is byte offset, must be multiple of 4\n"
> "\t\tNOTE: maskval is a 32 bit hex number\n \t\tNOTE: shiftval is a shift value\n"
> - "\t\tCMD:= clear | invert | set <setval>| add <addval> | retain\n"
> + "\t\tCMD:= clear | invert | set <setval> | add <addval> | decrement | retain\n"
> "\t<LAYERED>:= ip <ipdata> | ip6 <ip6data>\n"
> " \t\t| udp <udpdata> | tcp <tcpdata> | icmp <icmpdata>\n"
> "\tCONTROL:= reclassify | pipe | drop | continue | pass |\n"
> @@ -360,15 +360,24 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
> if (matches(*argv, "add") == 0)
> tkey->cmd = TCA_PEDIT_KEY_EX_CMD_ADD;
>
> - if (!sel->extended && tkey->cmd) {
> - fprintf(stderr,
> - "Non extended mode. only 'set' command is supported\n");
> - return -1;
> - }
> + if (!sel->extended && tkey->cmd)
> + goto non_ext_only_set_cmd;
>
> NEXT_ARG();
> if (parse_val(&argc, &argv, val, type))
> return -1;
> + } else if (matches(*argv, "decrement") == 0) {
> + if ((flags & PEDIT_ALLOW_DEC) == 0) {
> + fprintf(stderr,
> + "decrement command is not supported for this field\n");
> + return -1;
> + }
> +
> + if (!sel->extended)
> + goto non_ext_only_set_cmd;
> +
> + tkey->cmd = TCA_PEDIT_KEY_EX_CMD_ADD;
> + *v = retain; /* decrement by overflow */
> } else if (matches(*argv, "preserve") == 0) {
> retain = 0;
> } else {
> @@ -431,6 +440,10 @@ done:
> *argv_p = argv;
> return res;
>
> +non_ext_only_set_cmd:
> + fprintf(stderr,
> + "Non extended mode. only 'set' command is supported\n");
> + return -1;
> }
>
> static int parse_offset(int *argc_p, char ***argv_p, struct m_pedit_sel *sel,
> diff --git a/tc/m_pedit.h b/tc/m_pedit.h
> index 7398f66d..549bcf86 100644
> --- a/tc/m_pedit.h
> +++ b/tc/m_pedit.h
> @@ -39,6 +39,10 @@
>
> #define PEDITKINDSIZ 16
>
> +enum m_pedit_flags {
> + PEDIT_ALLOW_DEC = 1<<0,
> +};
> +
> struct m_pedit_key {
> __u32 mask; /* AND */
> __u32 val; /*XOR */
> diff --git a/tc/p_ip.c b/tc/p_ip.c
> index 2d1643d0..8eed9e8d 100644
> --- a/tc/p_ip.c
> +++ b/tc/p_ip.c
> @@ -68,7 +68,7 @@ parse_ip(int *argc_p, char ***argv_p,
> if (strcmp(*argv, "ttl") == 0) {
> NEXT_ARG();
> tkey->off = 8;
> - res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, 0);
> + res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, PEDIT_ALLOW_DEC);
> goto done;
> }
> if (strcmp(*argv, "protocol") == 0) {
> diff --git a/tc/p_ip6.c b/tc/p_ip6.c
> index f9d5d3b0..f855c59e 100644
> --- a/tc/p_ip6.c
> +++ b/tc/p_ip6.c
> @@ -71,7 +71,7 @@ parse_ip6(int *argc_p, char ***argv_p,
> if (strcmp(*argv, "hoplimit") == 0) {
> NEXT_ARG();
> tkey->off = 7;
> - res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, 0);
> + res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, PEDIT_ALLOW_DEC);
> goto done;
> }
> if (strcmp(*argv, "traffic_class") == 0) {
>
Powered by blists - more mailing lists