On 28/05/18 11:42 AM, Marcelo Ricardo Leitner wrote:
On Mon, May 28, 2018 at 05:40:18AM +0000, Fu, Qiaobin wrote:
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.

Original idea by Jamal Hadi Salim <j...@mojatatu.com>

Signed-off-by: Qiaobin Fu <qiaob...@bu.edu>
Reviewed-by: Michel Machado <mic...@digirati.com.br>
---

Note that the motivation for this patch is found in the following discussion:
https://www.spinics.net/lists/netdev/msg501061.html
---

diff --git a/include/uapi/linux/tc_act/tc_skbedit.h 
b/include/uapi/linux/tc_act/tc_skbedit.h
index fbcfe27..432ad2f 100644
--- a/include/uapi/linux/tc_act/tc_skbedit.h
+++ b/include/uapi/linux/tc_act/tc_skbedit.h
@@ -30,9 +30,11 @@
  #define SKBEDIT_F_MARK                        0x4
  #define SKBEDIT_F_PTYPE                       0x8
  #define SKBEDIT_F_MASK                        0x10
+#define SKBEDIT_F_INHERITDSFIELD       0x20
struct tc_skbedit {
        tc_gen;
+       __u64 flags;

I don't think this is doable. It looks like it was prepared for such
change, but it breaks UAPI as it causes tc without the respective
patch to not be able to talk to skbedit anymore:

With this patch:
[root@f28 ~]# tc action add action skbedit priority 1
RTNETLINK answers: Numerical result out of range
We have an error talking to the kernel
[root@f28 ~]#

While without this patch:
[root@f28 ~]# tc action add action skbedit priority 1
[root@f28 ~]#

And the new flags field doesnt even seem to be in use.
Qiaobin, you dont seem to need this.
Also you have added a workaround for default_priority
which I dont think is needed either (sounds like you
want to make sure that SKBEDIT_F_PRIORITY and new
feature are mutually exclusive - which seems unnecessary.
My suggestion is to remove:
+       } else {
+default_priority:
+               if (d->flags & SKBEDIT_F_PRIORITY)
+                       skb->priority = d->priority;
+       }

and leave alone:
-       if (d->flags & SKBEDIT_F_PRIORITY)
-               skb->priority = d->priority;

If someone wants to set both flags, then let them.

cheers,
jamal

Reply via email to