[PATCH iproute2 net-next V3] tc: flower: Refactor matching flags to be more user friendly

2017-01-19 Thread Paul Blakey
Instead of "magic numbers" we can now specify each flag
by name. Prefix of "no"  (e.g nofrag) unsets the flag,
otherwise it wil be set.

Example:
# add a flower filter that will drop fragmented packets
tc filter add dev ens4f0 protocol ip parent : \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags frag \
action drop

# add a flower filter that will drop non-fragmented packets
tc filter add dev ens4f0 protocol ip parent : \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags nofrag \
action drop

Fixes: 22a8f019891c ('tc: flower: support matching flags')
Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---

Hi,
Added a framework to add new flags more easily, such 
as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.

Thanks,
 Paul.


Changelog:

v3:
Changed prefix to "no" instead of "no_".

v2:
Changed delimiter to "/" to avoid shell pipe errors.


 man/man8/tc-flower.8 |  12 +-
 tc/f_flower.c| 117 ---
 2 files changed, 102 insertions(+), 27 deletions(-)

diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 2dd2c5e..77da10b 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -46,7 +46,9 @@ flower \- flow based traffic control filter
 .BR enc_dst_ip " | " enc_src_ip " } { "
 .IR ipv4_address " | " ipv6_address " } | "
 .B enc_dst_port
-.IR port_number
+.IR port_number " | "
+.BR ip_flags
+.IR IP_FLAGS
 .SH DESCRIPTION
 The
 .B flower
@@ -183,13 +185,19 @@ prefix length. If the prefix is missing, \fBtc\fR assumes 
a full-length
 host match.  Dst port
 .I NUMBER
 is a 16 bit UDP dst port.
+.TP
+.BI ip_flags " IP_FLAGS"
+.I IP_FLAGS
+may be either
+.BR frag " or " no_frag
+to match on fragmented packets or not respectively.
 .SH NOTES
 As stated above where applicable, matches of a certain layer implicitly depend
 on the matches of the next lower layer. Precisely, layer one and two matches
 (\fBindev\fR,  \fBdst_mac\fR and \fBsrc_mac\fR)
 have no dependency, layer three matches
 (\fBip_proto\fR, \fBdst_ip\fR, \fBsrc_ip\fR, \fBarp_tip\fR, \fBarp_sip\fR,
-\fBarp_op\fR, \fBarp_tha\fR and \fBarp_sha\fR)
+\fBarp_op\fR, \fBarp_tha\fR, \fBarp_sha\fR and \fBip_flags\fR)
 depend on the
 .B protocol
 option of tc filter, layer four port matches
diff --git a/tc/f_flower.c b/tc/f_flower.c
index d301db3..3dffe2b 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -24,6 +24,10 @@
 #include "tc_util.h"
 #include "rt_names.h"
 
+enum flower_matching_flags {
+   FLOWER_IP_FLAGS,
+};
+
 enum flower_endpoint {
FLOWER_ENDPOINT_SRC,
FLOWER_ENDPOINT_DST
@@ -63,7 +67,7 @@ static void explain(void)
"   enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] 
|\n"
"   enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] 
|\n"
"   enc_key_id [ KEY-ID ] |\n"
-   "   matching_flags MATCHING-FLAGS | \n"
+   "   ip_flags IP-FLAGS | \n"
"   enc_dst_port [ port_number ] }\n"
"   FILTERID := X:Y:Z\n"
"   MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS 
}\n"
@@ -136,28 +140,56 @@ static int flower_parse_vlan_eth_type(char *str, __be16 
eth_type, int type,
return 0;
 }
 
-static int flower_parse_matching_flags(char *str, int type, int mask_type,
-  struct nlmsghdr *n)
-{
-   __u32 mtf, mtf_mask;
-   char *c;
+struct flag_to_string {
+   int flag;
+   enum flower_matching_flags type;
+   char *string;
+};
 
-   c = strchr(str, '/');
-   if (c)
-   *c = '\0';
+static struct flag_to_string flags_str[] = {
+   { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
+};
 
-   if (get_u32(, str, 0))
-   return -1;
+static int flower_parse_matching_flags(char *str,
+  enum flower_matching_flags type,
+  __u32 *mtf, __u32 *mtf_mask)
+{
+   char *token;
+   bool no;
+   bool found;
+   int i;
 
-   if (c) {
-   if (get_u32(_mask, ++c, 0))
+   token = strtok(str, "/");
+
+   while (token) {
+   if (!strncmp(token, "no", 2)) {
+   no = true;
+   token = strchr(token, '_') + 1;
+   } else
+   no = false;
+
+   found = false;
+   for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+   if (type != flags_str[i].type)
+   continue;
+
+   if (!strcmp(token, 

Re: [PATCH iproute2 net-next V3] tc: flower: Refactor matching flags to be more user friendly

2017-01-19 Thread Paul Blakey



On 19/01/2017 16:14, Paul Blakey wrote:

Instead of "magic numbers" we can now specify each flag
by name. Prefix of "no"  (e.g nofrag) unsets the flag,
otherwise it wil be set.

Example:
# add a flower filter that will drop fragmented packets
tc filter add dev ens4f0 protocol ip parent : \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags frag \
action drop

# add a flower filter that will drop non-fragmented packets
tc filter add dev ens4f0 protocol ip parent : \
flower \
src_mac e4:1d:2d:fd:8b:01 \
dst_mac e4:1d:2d:fd:8b:02 \
indev ens4f0 \
ip_flags nofrag \
action drop

Fixes: 22a8f019891c ('tc: flower: support matching flags')
Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---

Hi,
Added a framework to add new flags more easily, such
as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.

Thanks,
 Paul.


Changelog:

v3:
Changed prefix to "no" instead of "no_".

v2:
Changed delimiter to "/" to avoid shell pipe errors.


 man/man8/tc-flower.8 |  12 +-
 tc/f_flower.c| 117 ---
 2 files changed, 102 insertions(+), 27 deletions(-)

diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 2dd2c5e..77da10b 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -46,7 +46,9 @@ flower \- flow based traffic control filter
 .BR enc_dst_ip " | " enc_src_ip " } { "
 .IR ipv4_address " | " ipv6_address " } | "
 .B enc_dst_port
-.IR port_number
+.IR port_number " | "
+.BR ip_flags
+.IR IP_FLAGS
 .SH DESCRIPTION
 The
 .B flower
@@ -183,13 +185,19 @@ prefix length. If the prefix is missing, \fBtc\fR assumes 
a full-length
 host match.  Dst port
 .I NUMBER
 is a 16 bit UDP dst port.
+.TP
+.BI ip_flags " IP_FLAGS"
+.I IP_FLAGS
+may be either
+.BR frag " or " no_frag
+to match on fragmented packets or not respectively.
 .SH NOTES
 As stated above where applicable, matches of a certain layer implicitly depend
 on the matches of the next lower layer. Precisely, layer one and two matches
 (\fBindev\fR,  \fBdst_mac\fR and \fBsrc_mac\fR)
 have no dependency, layer three matches
 (\fBip_proto\fR, \fBdst_ip\fR, \fBsrc_ip\fR, \fBarp_tip\fR, \fBarp_sip\fR,
-\fBarp_op\fR, \fBarp_tha\fR and \fBarp_sha\fR)
+\fBarp_op\fR, \fBarp_tha\fR, \fBarp_sha\fR and \fBip_flags\fR)
 depend on the
 .B protocol
 option of tc filter, layer four port matches
diff --git a/tc/f_flower.c b/tc/f_flower.c
index d301db3..3dffe2b 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -24,6 +24,10 @@
 #include "tc_util.h"
 #include "rt_names.h"

+enum flower_matching_flags {
+   FLOWER_IP_FLAGS,
+};
+
 enum flower_endpoint {
FLOWER_ENDPOINT_SRC,
FLOWER_ENDPOINT_DST
@@ -63,7 +67,7 @@ static void explain(void)
"   enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] 
|\n"
"   enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] 
|\n"
"   enc_key_id [ KEY-ID ] |\n"
-   "   matching_flags MATCHING-FLAGS | \n"
+   "   ip_flags IP-FLAGS | \n"
"   enc_dst_port [ port_number ] }\n"
"   FILTERID := X:Y:Z\n"
"   MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS 
}\n"
@@ -136,28 +140,56 @@ static int flower_parse_vlan_eth_type(char *str, __be16 
eth_type, int type,
return 0;
 }

-static int flower_parse_matching_flags(char *str, int type, int mask_type,
-  struct nlmsghdr *n)
-{
-   __u32 mtf, mtf_mask;
-   char *c;
+struct flag_to_string {
+   int flag;
+   enum flower_matching_flags type;
+   char *string;
+};

-   c = strchr(str, '/');
-   if (c)
-   *c = '\0';
+static struct flag_to_string flags_str[] = {
+   { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
+};

-   if (get_u32(, str, 0))
-   return -1;
+static int flower_parse_matching_flags(char *str,
+  enum flower_matching_flags type,
+  __u32 *mtf, __u32 *mtf_mask)
+{
+   char *token;
+   bool no;
+   bool found;
+   int i;

-   if (c) {
-   if (get_u32(_mask, ++c, 0))
+   token = strtok(str, "/");
+
+   while (token) {
+   if (!strncmp(token, "no", 2)) {
+   no = true;
+   token = strchr(token, '_') + 1;
+   } else
+   no = false;
+
+   found = false;
+   for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
+   if (type != flags_str[i].type)
+   continue;
+
+