Re: [PATCH iproute2-next v2] net:sched: add action inheritdsfield to skbedit

[Stephen Hemminger]

On Thu, 12 Jul 2018 12:09:26 -0400
Qiaobin Fu  wrote:

> + if (*flags & SKBEDIT_F_INHERITDSFIELD)
> + print_string(PRINT_ANY, "inheritdsfield", " %s",
> +  "inheritdsfield");

Flags should be represented in JSON output as a null value (or boolean).
print_null(PRINT_ANY, "inheritdsfield", " %s", 
"inheritdsfield");
This will generate:
"inheritdsfield" : null,
Instead of:

"inheritdsfield" : "inheritdsfield",

Re: [PATCH iproute2-next v2] net:sched: add action inheritdsfield to skbedit

[Marcelo Ricardo Leitner]

On Thu, Jul 12, 2018 at 12:09:26PM -0400, Qiaobin Fu wrote:
> @@ -156,6 +162,9 @@ parse_skbedit(struct action_util *a, int *argc_p, char 
> ***argv_p, int tca_id,
>   if (flags & SKBEDIT_F_PTYPE)
>   addattr_l(n, MAX_MSG, TCA_SKBEDIT_PTYPE,
> , sizeof(ptype));
> + if (pure_flags != 0)
> + addattr_l(n, MAX_MSG, TCA_SKBEDIT_FLAGS,
> + _flags, sizeof(pure_flags));

It is missing 2 spaces  ^--- here, to make the indentation right. (as
in the block above)

  Marcelo

[PATCH iproute2-next v2] net:sched: add action inheritdsfield to skbedit

[Qiaobin Fu]

The new action inheritdsfield copies the field DS of
IPv4 and IPv6 packets into skb->priority. This enables
later classification of packets based on the DS field.

v2:
* Align the output syntax with the input syntax

* Fix the style issues

Original idea by Jamal Hadi Salim 

Signed-off-by: Qiaobin Fu 
Reviewed-by: Michel Machado 
Reviewed-by: Cong Wang 
---

Note that the motivation for this patch is found in the following discussion:
https://www.spinics.net/lists/netdev/msg501061.html
---
 tc/m_skbedit.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tc/m_skbedit.c b/tc/m_skbedit.c
index 7391fc7f..59b2affc 100644
--- a/tc/m_skbedit.c
+++ b/tc/m_skbedit.c
@@ -30,16 +30,18 @@
 
 static void explain(void)
 {
-   fprintf(stderr, "Usage: ... skbedit <[QM] [PM] [MM] [PT]>\n"
+   fprintf(stderr, "Usage: ... skbedit <[QM] [PM] [MM] [PT] [IF]>\n"
"QM = queue_mapping QUEUE_MAPPING\n"
"PM = priority PRIORITY\n"
"MM = mark MARK\n"
"PT = ptype PACKETYPE\n"
+   "IF = inheritdsfield\n"
"PACKETYPE = is one of:\n"
"  host, otherhost, broadcast, multicast\n"
"QUEUE_MAPPING = device transmit queue to use\n"
"PRIORITY = classID to assign to priority field\n"
-   "MARK = firewall mark to set\n");
+   "MARK = firewall mark to set\n"
+   "note: inheritdsfield maps DS field to skb->priority\n");
 }
 
 static void
@@ -60,6 +62,7 @@ parse_skbedit(struct action_util *a, int *argc_p, char 
***argv_p, int tca_id,
unsigned int tmp;
__u16 queue_mapping, ptype;
__u32 flags = 0, priority, mark;
+   __u64 pure_flags = 0;
struct tc_skbedit sel = { 0 };
 
if (matches(*argv, "skbedit") != 0)
@@ -111,6 +114,9 @@ parse_skbedit(struct action_util *a, int *argc_p, char 
***argv_p, int tca_id,
}
flags |= SKBEDIT_F_PTYPE;
ok++;
+   } else if (matches(*argv, "inheritdsfield") == 0) {
+   pure_flags |= SKBEDIT_F_INHERITDSFIELD;
+   ok++;
} else if (matches(*argv, "help") == 0) {
usage();
} else {
@@ -156,6 +162,9 @@ parse_skbedit(struct action_util *a, int *argc_p, char 
***argv_p, int tca_id,
if (flags & SKBEDIT_F_PTYPE)
addattr_l(n, MAX_MSG, TCA_SKBEDIT_PTYPE,
  , sizeof(ptype));
+   if (pure_flags != 0)
+   addattr_l(n, MAX_MSG, TCA_SKBEDIT_FLAGS,
+   _flags, sizeof(pure_flags));
addattr_nest_end(n, tail);
 
*argc_p = argc;
@@ -214,6 +223,13 @@ static int print_skbedit(struct action_util *au, FILE *f, 
struct rtattr *arg)
else
print_uint(PRINT_ANY, "ptype", " ptype %u", ptype);
}
+   if (tb[TCA_SKBEDIT_FLAGS] != NULL) {
+   __u64 *flags = RTA_DATA(tb[TCA_SKBEDIT_FLAGS]);
+
+   if (*flags & SKBEDIT_F_INHERITDSFIELD)
+   print_string(PRINT_ANY, "inheritdsfield", " %s",
+"inheritdsfield");
+   }
 
print_action_control(f, " ", p->action, "");
 
-- 
2.17.1