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

[PATCH 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

Changes since RFC:
* Update names of enums (Jiri)
* Use enums
* Drop RFC designation


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           | 125 ++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 136 insertions(+), 19 deletions(-)

-- 
2.7.0.rc3.207.g0ac5344

[PATCH 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 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>
---
v2
* Rename enums FLOWER_ENDPOINT_SRC/DST
---
 tc/f_flower.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/tc/f_flower.c b/tc/f_flower.c
index 615e8f27bed2..22d95305acb4 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_ENDPOINT_SRC,
+	FLOWER_ENDPOINT_DST
+};
+
 static void explain(void)
 {
 	fprintf(stderr,
@@ -160,29 +165,33 @@ 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_ENDPOINT_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_ENDPOINT_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_ENDPOINT_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 +349,16 @@ 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_ENDPOINT_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_ENDPOINT_SRC, n);
 			if (ret < 0) {
 				fprintf(stderr, "Illegal \"src_port\"\n");
 				return -1;
-- 
2.7.0.rc3.207.g0ac5344

[PATCH 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>
---
v2
* Rename enums FLOWER_ICMP_FIELD_TYPE/CODE
* Use above enums (oversight in v1)
---
 man/man8/tc-flower.8 | 20 ++++++++---
 tc/f_flower.c        | 98 ++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 107 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 22d95305acb4..6cac344683e6 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -28,6 +28,11 @@ enum flower_endpoint {
 	FLOWER_ENDPOINT_DST
 };
 
+enum flower_icmp_field {
+	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,41 @@ 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_FIELD_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_FIELD_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,
+			     enum flower_icmp_field field, struct nlmsghdr *n)
+{
+	int ret;
+	int type;
+	uint8_t value;
+
+	type = flower_icmp_attr_type(eth_type, ip_proto, field);
+	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)
@@ -363,6 +416,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,
+						FLOWER_ICMP_FIELD_TYPE, 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,
+						FLOWER_ICMP_FIELD_CODE, 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);
@@ -476,6 +545,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;
@@ -524,6 +597,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)
 {
@@ -592,6 +671,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 iproute2/net-next 3/3] tc: flower: support matching on ICMP type and code

[Simon Horman]

On Fri, Dec 02, 2016 at 07:24:39PM +0100, Simon Horman 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>

This series no longer applies to net-next.

...

> @@ -524,6 +597,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)));

ntohs() should be removed from the above as its argument is a u8.

...