[PATCH/RFC iproute2/net-next 0/3] tc: flower: Support matching on ICMP

[PATCH/RFC iproute2/net-next 0/3] tc: flower: Support matching on ICMP

[Simon Horman]

Add support for matching on ICMP type and code to flower. This is modeled
on existing support for matching on L4 ports.

The second patch provided a minor cleanup which is in keeping with
they style used in the last patch.

This is marked as an RFC to match the same designation given to the
corresponding kernel patches.

Based on iproute2/net-next with the following applied:
* [[PATCH iproute2/net-next v2] 0/4] tc: flower: SCTP and other port fixes

Simon Horman (3):
  tc: flower: update headers for TCA_FLOWER_KEY_ICMP*
  tc: flower: introduce enum flower_endpoint
  tc: flower: support matching on ICMP type and code

 include/linux/pkt_cls.h |  10 ++++
 man/man8/tc-flower.8    |  20 ++++++--
 tc/f_flower.c           | 118 ++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 129 insertions(+), 19 deletions(-)

-- 
2.7.0.rc3.207.g0ac5344

[PATCH/RFC iproute2/net-next 1/3] tc: flower: update headers for TCA_FLOWER_KEY_ICMP*

[Simon Horman]

These are proposed changes for net-next.

Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
 include/linux/pkt_cls.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index a3d8a4f17d8e..fa435ea8ad21 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -403,6 +403,16 @@ enum {
 	TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK,	/* be16 */
 	TCA_FLOWER_KEY_ENC_UDP_DST_PORT,	/* be16 */
 	TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK,	/* be16 */
+
+	TCA_FLOWER_KEY_ICMPV4_CODE,     /* u8 */
+	TCA_FLOWER_KEY_ICMPV4_CODE_MASK,/* u8 */
+	TCA_FLOWER_KEY_ICMPV4_TYPE,     /* u8 */
+	TCA_FLOWER_KEY_ICMPV4_TYPE_MASK,/* u8 */
+	TCA_FLOWER_KEY_ICMPV6_CODE,     /* u8 */
+	TCA_FLOWER_KEY_ICMPV6_CODE_MASK,/* u8 */
+	TCA_FLOWER_KEY_ICMPV6_TYPE,     /* u8 */
+	TCA_FLOWER_KEY_ICMPV6_TYPE_MASK,/* u8 */
+
 	__TCA_FLOWER_MAX,
 };
 
-- 
2.7.0.rc3.207.g0ac5344

[PATCH/RFC iproute2/net-next 2/3] tc: flower: introduce enum flower_endpoint

[Simon Horman]

Introduce enum flower_endpoint and use it instead of a bool
as the type for paramatising source and destination.

This is intended to improve read-ability and provide some type
checking of endpoint parameters.

Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
 tc/f_flower.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index 615e8f27bed2..42253067b43d 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -23,6 +23,11 @@
 #include "tc_util.h"
 #include "rt_names.h"
 
+enum flower_endpoint {
+	flower_src,
+	flower_dst
+};
+
 static void explain(void)
 {
 	fprintf(stderr,
@@ -160,29 +165,30 @@ static int flower_parse_ip_addr(char *str, __be16 eth_type,
 	return 0;
 }
 
-static int flower_port_attr_type(__u8 ip_proto, bool is_src)
+static int flower_port_attr_type(__u8 ip_proto, enum flower_endpoint endpoint)
 {
 	if (ip_proto == IPPROTO_TCP)
-		return is_src ? TCA_FLOWER_KEY_TCP_SRC :
+		return endpoint == flower_src ? TCA_FLOWER_KEY_TCP_SRC :
 			TCA_FLOWER_KEY_TCP_DST;
 	else if (ip_proto == IPPROTO_UDP)
-		return is_src ? TCA_FLOWER_KEY_UDP_SRC :
+		return endpoint == flower_src ? TCA_FLOWER_KEY_UDP_SRC :
 			TCA_FLOWER_KEY_UDP_DST;
 	else if (ip_proto == IPPROTO_SCTP)
-		return is_src ? TCA_FLOWER_KEY_SCTP_SRC :
+		return endpoint == flower_src ? TCA_FLOWER_KEY_SCTP_SRC :
 			TCA_FLOWER_KEY_SCTP_DST;
 	else
 		return -1;
 }
 
-static int flower_parse_port(char *str, __u8 ip_proto, bool is_src,
+static int flower_parse_port(char *str, __u8 ip_proto,
+			     enum flower_endpoint endpoint,
 			     struct nlmsghdr *n)
 {
 	int ret;
 	int type;
 	__be16 port;
 
-	type = flower_port_attr_type(ip_proto, is_src);
+	type = flower_port_attr_type(ip_proto, endpoint);
 	if (type < 0)
 		return -1;
 
@@ -340,14 +346,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 			}
 		} else if (matches(*argv, "dst_port") == 0) {
 			NEXT_ARG();
-			ret = flower_parse_port(*argv, ip_proto, false, n);
+			ret = flower_parse_port(*argv, ip_proto, flower_dst, n);
 			if (ret < 0) {
 				fprintf(stderr, "Illegal \"dst_port\"\n");
 				return -1;
 			}
 		} else if (matches(*argv, "src_port") == 0) {
 			NEXT_ARG();
-			ret = flower_parse_port(*argv, ip_proto, true, n);
+			ret = flower_parse_port(*argv, ip_proto, flower_src, n);
 			if (ret < 0) {
 				fprintf(stderr, "Illegal \"src_port\"\n");
 				return -1;
-- 
2.7.0.rc3.207.g0ac5344

[PATCH/RFC iproute2/net-next 3/3] tc: flower: support matching on ICMP type and code

[Simon Horman]

Support matching on ICMP type and code.

Example usage:

tc qdisc add dev eth0 ingress

tc filter add dev eth0 protocol ip parent ffff: flower \
	indev eth0 ip_proto icmp type 8 code 0 action drop

tc filter add dev eth0 protocol ipv6 parent ffff: flower \
	indev eth0 ip_proto icmpv6 type 128 code 0 action drop

Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
 man/man8/tc-flower.8 | 20 ++++++++---
 tc/f_flower.c        | 96 ++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 105 insertions(+), 11 deletions(-)

diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index a401293fed50..c01ace6249dd 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -29,7 +29,7 @@ flower \- flow based traffic control filter
 .IR PRIORITY " | "
 .BR vlan_eth_type " { " ipv4 " | " ipv6 " | "
 .IR ETH_TYPE " } | "
-.BR ip_proto " { " tcp " | " udp " | " sctp " | "
+.BR ip_proto " { " tcp " | " udp " | " sctp " | " icmp " | " icmpv6 " | "
 .IR IP_PROTO " } | { "
 .BR dst_ip " | " src_ip " } { "
 .IR ipv4_address " | " ipv6_address " } | { "
@@ -94,7 +94,7 @@ or an unsigned 16bit value in hexadecimal format.
 Match on layer four protocol.
 .I IP_PROTO
 may be
-.BR tcp ", " udp ", " sctp
+.BR tcp ", " udp ", " sctp ", " icmp ", " icmpv6
 or an unsigned 8bit value in hexadecimal format.
 .TP
 .BI dst_ip " ADDRESS"
@@ -112,6 +112,13 @@ option of tc filter.
 Match on layer 4 protocol source or destination port number. Only available for
 .BR ip_proto " values " udp ", " tcp  " and " sctp
 which have to be specified in beforehand.
+.TP
+.BI type " NUMBER"
+.TQ
+.BI code " NUMBER"
+Match on ICMP type or code. Only available for
+.BR ip_proto " values " icmp  " and " icmpv6
+which have to be specified in beforehand.
 .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
@@ -120,13 +127,16 @@ have no dependency, layer three matches
 (\fBip_proto\fR, \fBdst_ip\fR and \fBsrc_ip\fR)
 depend on the
 .B protocol
-option of tc filter
-and finally layer four matches
+option of tc filter, layer four port matches
 (\fBdst_port\fR and \fBsrc_port\fR)
 depend on
 .B ip_proto
 being set to
-.BR tcp ", " udp " or " sctp.
+.BR tcp ", " udp " or " sctp,
+and finally ICMP matches (\fBcode\fR and \fBtype\fR) depend on
+.B ip_proto
+being set to
+.BR icmp " or " icmpv6.
 .P
 There can be only used one mask per one prio. If user needs to specify different
 mask, he has to use different prio.
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 42253067b43d..59f6f1ea26e6 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -28,6 +28,11 @@ enum flower_endpoint {
 	flower_dst
 };
 
+enum flower_icmp_field {
+	flower_icmp_type,
+	flower_icmp_code
+};
+
 static void explain(void)
 {
 	fprintf(stderr,
@@ -42,11 +47,13 @@ static void explain(void)
 		"                       vlan_ethtype [ ipv4 | ipv6 | ETH-TYPE ] |\n"
 		"                       dst_mac MAC-ADDR |\n"
 		"                       src_mac MAC-ADDR |\n"
-		"                       ip_proto [tcp | udp | sctp | IP-PROTO ] |\n"
+		"                       ip_proto [tcp | udp | sctp | icmp | icmpv6 | IP-PROTO ] |\n"
 		"                       dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
 		"                       src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
 		"                       dst_port PORT-NUMBER |\n"
-		"                       src_port PORT-NUMBER }\n"
+		"                       src_port PORT-NUMBER |\n"
+		"                       type ICMP-TYPE |\n"
+		"                       code ICMP-CODE }\n"
 		"       FILTERID := X:Y:Z\n"
 		"       ACTION-SPEC := ... look at individual actions\n"
 		"\n"
@@ -95,16 +102,23 @@ static int flower_parse_ip_proto(char *str, __be16 eth_type, int type,
 	int ret;
 	__u8 ip_proto;
 
-	if (eth_type != htons(ETH_P_IP) && eth_type != htons(ETH_P_IPV6)) {
-		fprintf(stderr, "Illegal \"eth_type\" for ip proto\n");
-		return -1;
-	}
+	if (eth_type != htons(ETH_P_IP) && eth_type != htons(ETH_P_IPV6))
+		goto err;
+
 	if (matches(str, "tcp") == 0) {
 		ip_proto = IPPROTO_TCP;
 	} else if (matches(str, "udp") == 0) {
 		ip_proto = IPPROTO_UDP;
 	} else if (matches(str, "sctp") == 0) {
 		ip_proto = IPPROTO_SCTP;
+	} else if (matches(str, "icmp") == 0) {
+		if (eth_type != htons(ETH_P_IP))
+			goto err;
+		ip_proto = IPPROTO_ICMP;
+	} else if (matches(str, "icmpv6") == 0) {
+		if (eth_type != htons(ETH_P_IPV6))
+			goto err;
+		ip_proto = IPPROTO_ICMPV6;
 	} else {
 		ret = get_u8(&ip_proto, str, 16);
 		if (ret)
@@ -113,6 +127,10 @@ static int flower_parse_ip_proto(char *str, __be16 eth_type, int type,
 	addattr8(n, MAX_MSG, type, ip_proto);
 	*p_ip_proto = ip_proto;
 	return 0;
+
+err:
+	fprintf(stderr, "Illegal \"eth_type\" for ip proto\n");
+	return -1;
 }
 
 static int flower_parse_ip_addr(char *str, __be16 eth_type,
@@ -165,6 +183,39 @@ static int flower_parse_ip_addr(char *str, __be16 eth_type,
 	return 0;
 }
 
+static int flower_icmp_attr_type(__be16 eth_type, __u8 ip_proto,
+				 enum flower_icmp_field field)
+{
+	if (eth_type == htons(ETH_P_IP) && ip_proto == IPPROTO_ICMP)
+		return field == flower_icmp_code ? TCA_FLOWER_KEY_ICMPV4_CODE :
+			TCA_FLOWER_KEY_ICMPV4_TYPE;
+	else if (eth_type == htons(ETH_P_IPV6) &&ip_proto == IPPROTO_ICMPV6)
+		return field == flower_icmp_code ? TCA_FLOWER_KEY_ICMPV6_CODE :
+			TCA_FLOWER_KEY_ICMPV6_TYPE;
+
+	return -1;
+}
+
+static int flower_parse_icmp(char *str, __u16 eth_type, __u8 ip_proto,
+			     bool is_code, struct nlmsghdr *n)
+{
+	int ret;
+	int type;
+	uint8_t value;
+
+	type = flower_icmp_attr_type(eth_type, ip_proto, is_code);
+	if (type < 0)
+		return -1;
+
+	ret = get_u8(&value, str, 10);
+	if (ret)
+		return -1;
+
+	addattr8(n, MAX_MSG, type, value);
+
+	return 0;
+}
+
 static int flower_port_attr_type(__u8 ip_proto, enum flower_endpoint endpoint)
 {
 	if (ip_proto == IPPROTO_TCP)
@@ -358,6 +409,22 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 				fprintf(stderr, "Illegal \"src_port\"\n");
 				return -1;
 			}
+		} else if (matches(*argv, "type") == 0) {
+			NEXT_ARG();
+			ret = flower_parse_icmp(*argv, eth_type, ip_proto,
+						false, n);
+			if (ret < 0) {
+				fprintf(stderr, "Illegal \"icmp type\"\n");
+				return -1;
+			}
+		} else if (matches(*argv, "code") == 0) {
+			NEXT_ARG();
+			ret = flower_parse_icmp(*argv, eth_type, ip_proto,
+						true, n);
+			if (ret < 0) {
+				fprintf(stderr, "Illegal \"icmp code\"\n");
+				return -1;
+			}
 		} else if (matches(*argv, "action") == 0) {
 			NEXT_ARG();
 			ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
@@ -471,6 +538,10 @@ static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
 		fprintf(f, "udp");
 	else if (ip_proto == IPPROTO_SCTP)
 		fprintf(f, "sctp");
+	else if (ip_proto == IPPROTO_ICMP)
+		fprintf(f, "icmp");
+	else if (ip_proto == IPPROTO_ICMPV6)
+		fprintf(f, "icmpv6");
 	else
 		fprintf(f, "%02x", ip_proto);
 	*p_ip_proto = ip_proto;
@@ -519,6 +590,12 @@ static void flower_print_port(FILE *f, char *name, struct rtattr *attr)
 		fprintf(f, "\n  %s %d", name, ntohs(rta_getattr_u16(attr)));
 }
 
+static void flower_print_icmp(FILE *f, char *name, struct rtattr *attr)
+{
+	if (attr)
+		fprintf(f, "\n  %s %d", name, ntohs(rta_getattr_u8(attr)));
+}
+
 static int flower_print_opt(struct filter_util *qu, FILE *f,
 			    struct rtattr *opt, __u32 handle)
 {
@@ -587,6 +664,13 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
 	if (nl_type >= 0)
 		flower_print_port(f, "src_port", tb[nl_type]);
 
+	nl_type = flower_icmp_attr_type(eth_type, ip_proto, false);
+	if (nl_type >= 0)
+		flower_print_icmp(f, "icmp_type", tb[nl_type]);
+	nl_type = flower_icmp_attr_type(eth_type, ip_proto, true);
+	if (nl_type >= 0)
+		flower_print_icmp(f, "icmp_code", tb[nl_type]);
+
 	if (tb[TCA_FLOWER_FLAGS]) {
 		__u32 flags = rta_getattr_u32(tb[TCA_FLOWER_FLAGS]);
 
-- 
2.7.0.rc3.207.g0ac5344

Re: [PATCH/RFC iproute2/net-next 2/3] tc: flower: introduce enum flower_endpoint

[Jiri Pirko]

Fri, Dec 02, 2016 at 10:59:44AM CET, simon.horman@netronome.com wrote:
>Introduce enum flower_endpoint and use it instead of a bool
>as the type for paramatising source and destination.
>
>This is intended to improve read-ability and provide some type
>checking of endpoint parameters.
>
>Signed-off-by: Simon Horman <simon.horman@netronome.com>
>---
> tc/f_flower.c | 22 ++++++++++++++--------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
>diff --git a/tc/f_flower.c b/tc/f_flower.c
>index 615e8f27bed2..42253067b43d 100644
>--- a/tc/f_flower.c
>+++ b/tc/f_flower.c
>@@ -23,6 +23,11 @@
> #include "tc_util.h"
> #include "rt_names.h"
> 
>+enum flower_endpoint {
>+	flower_src,
>+	flower_dst

	FLOWER_ENDPOINT_SRC,
	FLOWER_ENDPOINT_DST,

>+};
>+
> static void explain(void)
> {
> 	fprintf(stderr,
>@@ -160,29 +165,30 @@ static int flower_parse_ip_addr(char *str, __be16 eth_type,
> 	return 0;
> }
> 
>-static int flower_port_attr_type(__u8 ip_proto, bool is_src)
>+static int flower_port_attr_type(__u8 ip_proto, enum flower_endpoint endpoint)
> {
> 	if (ip_proto == IPPROTO_TCP)
>-		return is_src ? TCA_FLOWER_KEY_TCP_SRC :
>+		return endpoint == flower_src ? TCA_FLOWER_KEY_TCP_SRC :
> 			TCA_FLOWER_KEY_TCP_DST;
> 	else if (ip_proto == IPPROTO_UDP)
>-		return is_src ? TCA_FLOWER_KEY_UDP_SRC :
>+		return endpoint == flower_src ? TCA_FLOWER_KEY_UDP_SRC :
> 			TCA_FLOWER_KEY_UDP_DST;
> 	else if (ip_proto == IPPROTO_SCTP)
>-		return is_src ? TCA_FLOWER_KEY_SCTP_SRC :
>+		return endpoint == flower_src ? TCA_FLOWER_KEY_SCTP_SRC :
> 			TCA_FLOWER_KEY_SCTP_DST;
> 	else
> 		return -1;
> }
> 
>-static int flower_parse_port(char *str, __u8 ip_proto, bool is_src,
>+static int flower_parse_port(char *str, __u8 ip_proto,
>+			     enum flower_endpoint endpoint,
> 			     struct nlmsghdr *n)
> {
> 	int ret;
> 	int type;
> 	__be16 port;
> 
>-	type = flower_port_attr_type(ip_proto, is_src);
>+	type = flower_port_attr_type(ip_proto, endpoint);
> 	if (type < 0)
> 		return -1;
> 
>@@ -340,14 +346,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> 			}
> 		} else if (matches(*argv, "dst_port") == 0) {
> 			NEXT_ARG();
>-			ret = flower_parse_port(*argv, ip_proto, false, n);
>+			ret = flower_parse_port(*argv, ip_proto, flower_dst, n);
> 			if (ret < 0) {
> 				fprintf(stderr, "Illegal \"dst_port\"\n");
> 				return -1;
> 			}
> 		} else if (matches(*argv, "src_port") == 0) {
> 			NEXT_ARG();
>-			ret = flower_parse_port(*argv, ip_proto, true, n);
>+			ret = flower_parse_port(*argv, ip_proto, flower_src, n);
> 			if (ret < 0) {
> 				fprintf(stderr, "Illegal \"src_port\"\n");
> 				return -1;
>-- 
>2.7.0.rc3.207.g0ac5344
>

Re: [PATCH/RFC iproute2/net-next 3/3] tc: flower: support matching on ICMP type and code

[Jiri Pirko]

Fri, Dec 02, 2016 at 10:59:45AM CET, simon.horman@netronome.com wrote:
>Support matching on ICMP type and code.
>
>Example usage:
>
>tc qdisc add dev eth0 ingress
>
>tc filter add dev eth0 protocol ip parent ffff: flower \
>	indev eth0 ip_proto icmp type 8 code 0 action drop
>
>tc filter add dev eth0 protocol ipv6 parent ffff: flower \
>	indev eth0 ip_proto icmpv6 type 128 code 0 action drop
>
>Signed-off-by: Simon Horman <simon.horman@netronome.com>
>---
> man/man8/tc-flower.8 | 20 ++++++++---
> tc/f_flower.c        | 96 ++++++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 105 insertions(+), 11 deletions(-)
>
>diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
>index a401293fed50..c01ace6249dd 100644
>--- a/man/man8/tc-flower.8
>+++ b/man/man8/tc-flower.8
>@@ -29,7 +29,7 @@ flower \- flow based traffic control filter
> .IR PRIORITY " | "
> .BR vlan_eth_type " { " ipv4 " | " ipv6 " | "
> .IR ETH_TYPE " } | "
>-.BR ip_proto " { " tcp " | " udp " | " sctp " | "
>+.BR ip_proto " { " tcp " | " udp " | " sctp " | " icmp " | " icmpv6 " | "
> .IR IP_PROTO " } | { "
> .BR dst_ip " | " src_ip " } { "
> .IR ipv4_address " | " ipv6_address " } | { "
>@@ -94,7 +94,7 @@ or an unsigned 16bit value in hexadecimal format.
> Match on layer four protocol.
> .I IP_PROTO
> may be
>-.BR tcp ", " udp ", " sctp
>+.BR tcp ", " udp ", " sctp ", " icmp ", " icmpv6
> or an unsigned 8bit value in hexadecimal format.
> .TP
> .BI dst_ip " ADDRESS"
>@@ -112,6 +112,13 @@ option of tc filter.
> Match on layer 4 protocol source or destination port number. Only available for
> .BR ip_proto " values " udp ", " tcp  " and " sctp
> which have to be specified in beforehand.
>+.TP
>+.BI type " NUMBER"
>+.TQ
>+.BI code " NUMBER"
>+Match on ICMP type or code. Only available for
>+.BR ip_proto " values " icmp  " and " icmpv6
>+which have to be specified in beforehand.
> .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
>@@ -120,13 +127,16 @@ have no dependency, layer three matches
> (\fBip_proto\fR, \fBdst_ip\fR and \fBsrc_ip\fR)
> depend on the
> .B protocol
>-option of tc filter
>-and finally layer four matches
>+option of tc filter, layer four port matches
> (\fBdst_port\fR and \fBsrc_port\fR)
> depend on
> .B ip_proto
> being set to
>-.BR tcp ", " udp " or " sctp.
>+.BR tcp ", " udp " or " sctp,
>+and finally ICMP matches (\fBcode\fR and \fBtype\fR) depend on
>+.B ip_proto
>+being set to
>+.BR icmp " or " icmpv6.
> .P
> There can be only used one mask per one prio. If user needs to specify different
> mask, he has to use different prio.
>diff --git a/tc/f_flower.c b/tc/f_flower.c
>index 42253067b43d..59f6f1ea26e6 100644
>--- a/tc/f_flower.c
>+++ b/tc/f_flower.c
>@@ -28,6 +28,11 @@ enum flower_endpoint {
> 	flower_dst
> };
> 
>+enum flower_icmp_field {
>+	flower_icmp_type,
>+	flower_icmp_code

	FLOWER_ICMP_FIELD_TYPE,
	FLOWER_ICMP_FIELD_CODE,

>+};
>+
> static void explain(void)
> {
> 	fprintf(stderr,
>@@ -42,11 +47,13 @@ static void explain(void)
> 		"                       vlan_ethtype [ ipv4 | ipv6 | ETH-TYPE ] |\n"
> 		"                       dst_mac MAC-ADDR |\n"
> 		"                       src_mac MAC-ADDR |\n"
>-		"                       ip_proto [tcp | udp | sctp | IP-PROTO ] |\n"
>+		"                       ip_proto [tcp | udp | sctp | icmp | icmpv6 | IP-PROTO ] |\n"
> 		"                       dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
> 		"                       src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
> 		"                       dst_port PORT-NUMBER |\n"
>-		"                       src_port PORT-NUMBER }\n"
>+		"                       src_port PORT-NUMBER |\n"
>+		"                       type ICMP-TYPE |\n"
>+		"                       code ICMP-CODE }\n"
> 		"       FILTERID := X:Y:Z\n"
> 		"       ACTION-SPEC := ... look at individual actions\n"
> 		"\n"
>@@ -95,16 +102,23 @@ static int flower_parse_ip_proto(char *str, __be16 eth_type, int type,
> 	int ret;
> 	__u8 ip_proto;
> 
>-	if (eth_type != htons(ETH_P_IP) && eth_type != htons(ETH_P_IPV6)) {
>-		fprintf(stderr, "Illegal \"eth_type\" for ip proto\n");
>-		return -1;
>-	}
>+	if (eth_type != htons(ETH_P_IP) && eth_type != htons(ETH_P_IPV6))
>+		goto err;
>+
> 	if (matches(str, "tcp") == 0) {
> 		ip_proto = IPPROTO_TCP;
> 	} else if (matches(str, "udp") == 0) {
> 		ip_proto = IPPROTO_UDP;
> 	} else if (matches(str, "sctp") == 0) {
> 		ip_proto = IPPROTO_SCTP;
>+	} else if (matches(str, "icmp") == 0) {
>+		if (eth_type != htons(ETH_P_IP))
>+			goto err;
>+		ip_proto = IPPROTO_ICMP;
>+	} else if (matches(str, "icmpv6") == 0) {
>+		if (eth_type != htons(ETH_P_IPV6))
>+			goto err;
>+		ip_proto = IPPROTO_ICMPV6;
> 	} else {
> 		ret = get_u8(&ip_proto, str, 16);
> 		if (ret)
>@@ -113,6 +127,10 @@ static int flower_parse_ip_proto(char *str, __be16 eth_type, int type,
> 	addattr8(n, MAX_MSG, type, ip_proto);
> 	*p_ip_proto = ip_proto;
> 	return 0;
>+
>+err:
>+	fprintf(stderr, "Illegal \"eth_type\" for ip proto\n");
>+	return -1;
> }
> 
> static int flower_parse_ip_addr(char *str, __be16 eth_type,
>@@ -165,6 +183,39 @@ static int flower_parse_ip_addr(char *str, __be16 eth_type,
> 	return 0;
> }
> 
>+static int flower_icmp_attr_type(__be16 eth_type, __u8 ip_proto,
>+				 enum flower_icmp_field field)
>+{
>+	if (eth_type == htons(ETH_P_IP) && ip_proto == IPPROTO_ICMP)
>+		return field == flower_icmp_code ? TCA_FLOWER_KEY_ICMPV4_CODE :
>+			TCA_FLOWER_KEY_ICMPV4_TYPE;
>+	else if (eth_type == htons(ETH_P_IPV6) &&ip_proto == IPPROTO_ICMPV6)
>+		return field == flower_icmp_code ? TCA_FLOWER_KEY_ICMPV6_CODE :
>+			TCA_FLOWER_KEY_ICMPV6_TYPE;
>+
>+	return -1;
>+}
>+
>+static int flower_parse_icmp(char *str, __u16 eth_type, __u8 ip_proto,
>+			     bool is_code, struct nlmsghdr *n)
>+{
>+	int ret;
>+	int type;
>+	uint8_t value;
>+
>+	type = flower_icmp_attr_type(eth_type, ip_proto, is_code);
>+	if (type < 0)
>+		return -1;
>+
>+	ret = get_u8(&value, str, 10);
>+	if (ret)
>+		return -1;
>+
>+	addattr8(n, MAX_MSG, type, value);
>+
>+	return 0;
>+}
>+
> static int flower_port_attr_type(__u8 ip_proto, enum flower_endpoint endpoint)
> {
> 	if (ip_proto == IPPROTO_TCP)
>@@ -358,6 +409,22 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> 				fprintf(stderr, "Illegal \"src_port\"\n");
> 				return -1;
> 			}
>+		} else if (matches(*argv, "type") == 0) {
>+			NEXT_ARG();
>+			ret = flower_parse_icmp(*argv, eth_type, ip_proto,
>+						false, n);
>+			if (ret < 0) {
>+				fprintf(stderr, "Illegal \"icmp type\"\n");
>+				return -1;
>+			}
>+		} else if (matches(*argv, "code") == 0) {
>+			NEXT_ARG();
>+			ret = flower_parse_icmp(*argv, eth_type, ip_proto,
>+						true, n);
>+			if (ret < 0) {
>+				fprintf(stderr, "Illegal \"icmp code\"\n");
>+				return -1;
>+			}
> 		} else if (matches(*argv, "action") == 0) {
> 			NEXT_ARG();
> 			ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
>@@ -471,6 +538,10 @@ static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
> 		fprintf(f, "udp");
> 	else if (ip_proto == IPPROTO_SCTP)
> 		fprintf(f, "sctp");
>+	else if (ip_proto == IPPROTO_ICMP)
>+		fprintf(f, "icmp");
>+	else if (ip_proto == IPPROTO_ICMPV6)
>+		fprintf(f, "icmpv6");
> 	else
> 		fprintf(f, "%02x", ip_proto);
> 	*p_ip_proto = ip_proto;
>@@ -519,6 +590,12 @@ static void flower_print_port(FILE *f, char *name, struct rtattr *attr)
> 		fprintf(f, "\n  %s %d", name, ntohs(rta_getattr_u16(attr)));
> }
> 
>+static void flower_print_icmp(FILE *f, char *name, struct rtattr *attr)
>+{
>+	if (attr)
>+		fprintf(f, "\n  %s %d", name, ntohs(rta_getattr_u8(attr)));
>+}
>+
> static int flower_print_opt(struct filter_util *qu, FILE *f,
> 			    struct rtattr *opt, __u32 handle)
> {
>@@ -587,6 +664,13 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
> 	if (nl_type >= 0)
> 		flower_print_port(f, "src_port", tb[nl_type]);
> 
>+	nl_type = flower_icmp_attr_type(eth_type, ip_proto, false);
>+	if (nl_type >= 0)
>+		flower_print_icmp(f, "icmp_type", tb[nl_type]);
>+	nl_type = flower_icmp_attr_type(eth_type, ip_proto, true);
>+	if (nl_type >= 0)
>+		flower_print_icmp(f, "icmp_code", tb[nl_type]);
>+
> 	if (tb[TCA_FLOWER_FLAGS]) {
> 		__u32 flags = rta_getattr_u32(tb[TCA_FLOWER_FLAGS]);
> 
>-- 
>2.7.0.rc3.207.g0ac5344
>

Re: [PATCH/RFC iproute2/net-next 0/3] tc: flower: Support matching on ICMP

[Jiri Pirko]

Fri, Dec 02, 2016 at 10:59:42AM CET, simon.horman@netronome.com wrote:
>Add support for matching on ICMP type and code to flower. This is modeled
>on existing support for matching on L4 ports.
>
>The second patch provided a minor cleanup which is in keeping with
>they style used in the last patch.
>
>This is marked as an RFC to match the same designation given to the
>corresponding kernel patches.

Looks nice, I only have those 2 enum nitpicks.

Thanks.

>
>Based on iproute2/net-next with the following applied:
>* [[PATCH iproute2/net-next v2] 0/4] tc: flower: SCTP and other port fixes
>
>Simon Horman (3):
>  tc: flower: update headers for TCA_FLOWER_KEY_ICMP*
>  tc: flower: introduce enum flower_endpoint
>  tc: flower: support matching on ICMP type and code
>
> include/linux/pkt_cls.h |  10 ++++
> man/man8/tc-flower.8    |  20 ++++++--
> tc/f_flower.c           | 118 ++++++++++++++++++++++++++++++++++++++++++------
> 3 files changed, 129 insertions(+), 19 deletions(-)
>
>-- 
>2.7.0.rc3.207.g0ac5344
>

Re: [PATCH/RFC iproute2/net-next 0/3] tc: flower: Support matching on ICMP

[Simon Horman]

On Fri, Dec 02, 2016 at 06:10:20PM +0100, Jiri Pirko wrote:
> Fri, Dec 02, 2016 at 10:59:42AM CET, simon.horman@netronome.com wrote:
> >Add support for matching on ICMP type and code to flower. This is modeled
> >on existing support for matching on L4 ports.
> >
> >The second patch provided a minor cleanup which is in keeping with
> >they style used in the last patch.
> >
> >This is marked as an RFC to match the same designation given to the
> >corresponding kernel patches.
> 
> Looks nice, I only have those 2 enum nitpicks.

Thanks, I'll fix those and repost.

Re: [PATCH/RFC iproute2/net-next 1/3] tc: flower: update headers for TCA_FLOWER_KEY_ICMP*

[Stephen Hemminger]

On Fri,  2 Dec 2016 10:59:43 +0100
Simon Horman <simon.horman@netronome.com> wrote:

> These are proposed changes for net-next.
> 
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
> ---
>  include/linux/pkt_cls.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
> index a3d8a4f17d8e..fa435ea8ad21 100644
> --- a/include/linux/pkt_cls.h
> +++ b/include/linux/pkt_cls.h
> @@ -403,6 +403,16 @@ enum {
>  	TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK,	/* be16 */
>  	TCA_FLOWER_KEY_ENC_UDP_DST_PORT,	/* be16 */
>  	TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK,	/* be16 */
> +
> +	TCA_FLOWER_KEY_ICMPV4_CODE,     /* u8 */
> +	TCA_FLOWER_KEY_ICMPV4_CODE_MASK,/* u8 */
> +	TCA_FLOWER_KEY_ICMPV4_TYPE,     /* u8 */
> +	TCA_FLOWER_KEY_ICMPV4_TYPE_MASK,/* u8 */
> +	TCA_FLOWER_KEY_ICMPV6_CODE,     /* u8 */
> +	TCA_FLOWER_KEY_ICMPV6_CODE_MASK,/* u8 */
> +	TCA_FLOWER_KEY_ICMPV6_TYPE,     /* u8 */
> +	TCA_FLOWER_KEY_ICMPV6_TYPE_MASK,/* u8 */
> +
>  	__TCA_FLOWER_MAX,
>  };
>  

I picked this up by updating from kernel headers.