All of lore.kernel.org
 help / color / mirror / Atom feed
* [iproute2-next PATCH v3 1/2] tc: flower: Classify packets based port ranges
@ 2018-11-16  0:55 Amritha Nambiar
  2018-11-16  0:55 ` [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option Amritha Nambiar
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Amritha Nambiar @ 2018-11-16  0:55 UTC (permalink / raw)
  To: stephen, netdev, dsahern
  Cc: jakub.kicinski, amritha.nambiar, sridhar.samudrala, jhs,
	xiyou.wangcong, jiri

Added support for filtering based on port ranges.
UAPI changes have been accepted into net-next.

Example:
1. Match on a port range:
-------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
  prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
  action drop

$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
  eth_type ipv4
  ip_proto tcp
  dst_port range 20-30
  skip_hw
  not_in_hw
        action order 1: gact action drop
         random type none pass val 0
         index 1 ref 1 bind 1 installed 85 sec used 3 sec
        Action statistics:
        Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

2. Match on IP address and port range:
--------------------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
  prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
  skip_hw action drop

$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0 handle 0x2
  eth_type ipv4
  ip_proto tcp
  dst_ip 192.168.1.1
  dst_port range 100-200
  skip_hw
  not_in_hw
        action order 1: gact action drop
         random type none pass val 0
         index 2 ref 1 bind 1 installed 58 sec used 2 sec
        Action statistics:
        Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
        backlog 0b 0p requeues 0

v3:
Modified flower_port_range_attr_type calls.

v2:
Addressed Jiri's comment to sync output format with input

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
---
 include/uapi/linux/pkt_cls.h |    7 ++
 tc/f_flower.c                |  143 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 140 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 401d0c1..95d0db2 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -485,6 +485,11 @@ enum {
 
 	TCA_FLOWER_IN_HW_COUNT,
 
+	TCA_FLOWER_KEY_PORT_SRC_MIN,	/* be16 */
+	TCA_FLOWER_KEY_PORT_SRC_MAX,	/* be16 */
+	TCA_FLOWER_KEY_PORT_DST_MIN,	/* be16 */
+	TCA_FLOWER_KEY_PORT_DST_MAX,	/* be16 */
+
 	__TCA_FLOWER_MAX,
 };
 
@@ -518,6 +523,8 @@ enum {
 	TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
 };
 
+#define TCA_FLOWER_MASK_FLAGS_RANGE	(1 << 0) /* Range-based match */
+
 /* Match-all classifier */
 
 enum {
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 65fca04..9bddf7b 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -494,6 +494,68 @@ static int flower_parse_port(char *str, __u8 ip_proto,
 	return 0;
 }
 
+static int flower_port_range_attr_type(__u8 ip_proto, enum flower_endpoint type,
+				       __be16 *min_port_type,
+				       __be16 *max_port_type)
+{
+	if (ip_proto == IPPROTO_TCP || ip_proto == IPPROTO_UDP ||
+	    ip_proto == IPPROTO_SCTP) {
+		if (type == FLOWER_ENDPOINT_SRC) {
+			*min_port_type = TCA_FLOWER_KEY_PORT_SRC_MIN;
+			*max_port_type = TCA_FLOWER_KEY_PORT_SRC_MAX;
+		} else {
+			*min_port_type = TCA_FLOWER_KEY_PORT_DST_MIN;
+			*max_port_type = TCA_FLOWER_KEY_PORT_DST_MAX;
+		}
+	} else {
+		return -1;
+	}
+
+	return 0;
+}
+
+static int flower_parse_port_range(__be16 *min, __be16 *max, __u8 ip_proto,
+				   enum flower_endpoint endpoint,
+				   struct nlmsghdr *n)
+{
+	__be16 min_port_type, max_port_type;
+
+	if (flower_port_range_attr_type(ip_proto, endpoint, &min_port_type,
+					&max_port_type))
+		return -1;
+
+	addattr16(n, MAX_MSG, min_port_type, *min);
+	addattr16(n, MAX_MSG, max_port_type, *max);
+
+	return 0;
+}
+
+static int get_range(__be16 *min, __be16 *max, char *argv)
+{
+	char *r;
+
+	r = strchr(argv, '-');
+	if (r) {
+		*r = '\0';
+		if (get_be16(min, argv, 10)) {
+			fprintf(stderr, "invalid min range\n");
+			return -1;
+		}
+		if (get_be16(max, r + 1, 10)) {
+			fprintf(stderr, "invalid max range\n");
+			return -1;
+		}
+		if (htons(*max) <= htons(*min)) {
+			fprintf(stderr, "max value should be greater than min value\n");
+			return -1;
+		}
+	} else {
+		fprintf(stderr, "Illegal range format\n");
+		return -1;
+	}
+	return 0;
+}
+
 #define TCP_FLAGS_MAX_MASK 0xfff
 
 static int flower_parse_tcp_flags(char *str, int flags_type, int mask_type,
@@ -1061,20 +1123,54 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 				return -1;
 			}
 		} else if (matches(*argv, "dst_port") == 0) {
+			__be16 min, max;
+
 			NEXT_ARG();
-			ret = flower_parse_port(*argv, ip_proto,
-						FLOWER_ENDPOINT_DST, n);
-			if (ret < 0) {
-				fprintf(stderr, "Illegal \"dst_port\"\n");
-				return -1;
+			if (matches(*argv, "range") == 0) {
+				NEXT_ARG();
+				ret = get_range(&min, &max, *argv);
+				if (ret < 0)
+					return -1;
+				ret = flower_parse_port_range(&min, &max,
+							      ip_proto,
+							      FLOWER_ENDPOINT_DST,
+							      n);
+				if (ret < 0) {
+					fprintf(stderr, "Illegal \"dst_port range\"\n");
+					return -1;
+				}
+			} else {
+				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) {
+			__be16 min, max;
+
 			NEXT_ARG();
-			ret = flower_parse_port(*argv, ip_proto,
-						FLOWER_ENDPOINT_SRC, n);
-			if (ret < 0) {
-				fprintf(stderr, "Illegal \"src_port\"\n");
-				return -1;
+			if (matches(*argv, "range") == 0) {
+				NEXT_ARG();
+				ret = get_range(&min, &max, *argv);
+				if (ret < 0)
+					return -1;
+				ret = flower_parse_port_range(&min, &max,
+							      ip_proto,
+							      FLOWER_ENDPOINT_SRC,
+							      n);
+				if (ret < 0) {
+					fprintf(stderr, "Illegal \"src_port range\"\n");
+					return -1;
+				}
+			} else {
+				ret = flower_parse_port(*argv, ip_proto,
+							FLOWER_ENDPOINT_SRC, n);
+				if (ret < 0) {
+					fprintf(stderr, "Illegal \"src_port\"\n");
+					return -1;
+				}
 			}
 		} else if (matches(*argv, "tcp_flags") == 0) {
 			NEXT_ARG();
@@ -1490,6 +1586,22 @@ static void flower_print_port(char *name, struct rtattr *attr)
 	print_hu(PRINT_ANY, name, namefrm, rta_getattr_be16(attr));
 }
 
+static void flower_print_port_range(char *name, struct rtattr *min_attr,
+				    struct rtattr *max_attr)
+{
+	SPRINT_BUF(namefrm);
+	SPRINT_BUF(out);
+	size_t done;
+
+	if (!min_attr || !max_attr)
+		return;
+
+	done = sprintf(out, "%u", rta_getattr_be16(min_attr));
+	sprintf(out + done, "-%u", rta_getattr_be16(max_attr));
+	sprintf(namefrm, "\n  %s %%s", name);
+	print_string(PRINT_ANY, name, namefrm, out);
+}
+
 static void flower_print_tcp_flags(const char *name, struct rtattr *flags_attr,
 				   struct rtattr *mask_attr)
 {
@@ -1678,6 +1790,7 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
 			    struct rtattr *opt, __u32 handle)
 {
 	struct rtattr *tb[TCA_FLOWER_MAX + 1];
+	__be16 min_port_type, max_port_type;
 	int nl_type, nl_mask_type;
 	__be16 eth_type = 0;
 	__u8 ip_proto = 0xff;
@@ -1796,6 +1909,16 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
 	if (nl_type >= 0)
 		flower_print_port("src_port", tb[nl_type]);
 
+	if (!flower_port_range_attr_type(ip_proto, FLOWER_ENDPOINT_DST,
+					 &min_port_type, &max_port_type))
+		flower_print_port_range("dst_port range",
+					tb[min_port_type], tb[max_port_type]);
+
+	if (!flower_port_range_attr_type(ip_proto, FLOWER_ENDPOINT_SRC,
+					 &min_port_type, &max_port_type))
+		flower_print_port_range("src_port range",
+					tb[min_port_type], tb[max_port_type]);
+
 	flower_print_tcp_flags("tcp_flags", tb[TCA_FLOWER_KEY_TCP_FLAGS],
 			       tb[TCA_FLOWER_KEY_TCP_FLAGS_MASK]);
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
  2018-11-16  0:55 [iproute2-next PATCH v3 1/2] tc: flower: Classify packets based port ranges Amritha Nambiar
@ 2018-11-16  0:55 ` Amritha Nambiar
  2018-11-20 22:56   ` David Ahern
  2018-11-18 19:30 ` [iproute2-next PATCH v3 1/2] tc: flower: Classify packets based port ranges Jiri Pirko
  2018-11-20 23:00 ` David Ahern
  2 siblings, 1 reply; 11+ messages in thread
From: Amritha Nambiar @ 2018-11-16  0:55 UTC (permalink / raw)
  To: stephen, netdev, dsahern
  Cc: jakub.kicinski, amritha.nambiar, sridhar.samudrala, jhs,
	xiyou.wangcong, jiri

Add details explaining filtering based on port ranges.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
---
 man/man8/tc-flower.8 |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 8be8882..768bfa1 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -56,8 +56,10 @@ flower \- flow based traffic control filter
 .IR MASKED_IP_TTL " | { "
 .BR dst_ip " | " src_ip " } "
 .IR PREFIX " | { "
-.BR dst_port " | " src_port " } "
-.IR port_number " } | "
+.BR dst_port " | " src_port " } { "
+.IR port_number " | "
+.B range
+.IR min_port_number-max_port_number " } | "
 .B tcp_flags
 .IR MASKED_TCP_FLAGS " | "
 .B type
@@ -227,6 +229,12 @@ 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 range " MIN_VALUE-MAX_VALUE"
+Match on a range of 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 tcp_flags " MASKED_TCP_FLAGS"
 Match on TCP flags represented as 12bit bitfield in in hexadecimal format.
 A mask may be optionally provided to limit the bits which are matched. A mask

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [iproute2-next PATCH v3 1/2] tc: flower: Classify packets based port ranges
  2018-11-16  0:55 [iproute2-next PATCH v3 1/2] tc: flower: Classify packets based port ranges Amritha Nambiar
  2018-11-16  0:55 ` [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option Amritha Nambiar
@ 2018-11-18 19:30 ` Jiri Pirko
  2018-11-20 23:00 ` David Ahern
  2 siblings, 0 replies; 11+ messages in thread
From: Jiri Pirko @ 2018-11-18 19:30 UTC (permalink / raw)
  To: Amritha Nambiar
  Cc: stephen, netdev, dsahern, jakub.kicinski, sridhar.samudrala, jhs,
	xiyou.wangcong

Fri, Nov 16, 2018 at 01:55:13AM CET, amritha.nambiar@intel.com wrote:
>Added support for filtering based on port ranges.
>UAPI changes have been accepted into net-next.
>
>Example:
>1. Match on a port range:
>-------------------------
>$ tc filter add dev enp4s0 protocol ip parent ffff:\
>  prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
>  action drop
>
>$ tc -s filter show dev enp4s0 parent ffff:
>filter protocol ip pref 1 flower chain 0
>filter protocol ip pref 1 flower chain 0 handle 0x1
>  eth_type ipv4
>  ip_proto tcp
>  dst_port range 20-30
>  skip_hw
>  not_in_hw
>        action order 1: gact action drop
>         random type none pass val 0
>         index 1 ref 1 bind 1 installed 85 sec used 3 sec
>        Action statistics:
>        Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
>        backlog 0b 0p requeues 0
>
>2. Match on IP address and port range:
>--------------------------------------
>$ tc filter add dev enp4s0 protocol ip parent ffff:\
>  prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
>  skip_hw action drop
>
>$ tc -s filter show dev enp4s0 parent ffff:
>filter protocol ip pref 1 flower chain 0 handle 0x2
>  eth_type ipv4
>  ip_proto tcp
>  dst_ip 192.168.1.1
>  dst_port range 100-200
>  skip_hw
>  not_in_hw
>        action order 1: gact action drop
>         random type none pass val 0
>         index 2 ref 1 bind 1 installed 58 sec used 2 sec
>        Action statistics:
>        Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
>        backlog 0b 0p requeues 0
>
>v3:
>Modified flower_port_range_attr_type calls.
>
>v2:
>Addressed Jiri's comment to sync output format with input
>
>Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>

Looks ok. But why do you have man changes in a separate patch ?
I think it should be in this one. Anyway

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
  2018-11-16  0:55 ` [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option Amritha Nambiar
@ 2018-11-20 22:56   ` David Ahern
  2018-11-21  4:44     ` Nambiar, Amritha
  0 siblings, 1 reply; 11+ messages in thread
From: David Ahern @ 2018-11-20 22:56 UTC (permalink / raw)
  To: Amritha Nambiar, stephen, netdev
  Cc: jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong, jiri

On 11/15/18 5:55 PM, Amritha Nambiar wrote:
> Add details explaining filtering based on port ranges.
> 
> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
> ---
>  man/man8/tc-flower.8 |   12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
> index 8be8882..768bfa1 100644
> --- a/man/man8/tc-flower.8
> +++ b/man/man8/tc-flower.8
> @@ -56,8 +56,10 @@ flower \- flow based traffic control filter
>  .IR MASKED_IP_TTL " | { "
>  .BR dst_ip " | " src_ip " } "
>  .IR PREFIX " | { "
> -.BR dst_port " | " src_port " } "
> -.IR port_number " } | "
> +.BR dst_port " | " src_port " } { "
> +.IR port_number " | "
> +.B range
> +.IR min_port_number-max_port_number " } | "
>  .B tcp_flags
>  .IR MASKED_TCP_FLAGS " | "
>  .B type
> @@ -227,6 +229,12 @@ 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 range " MIN_VALUE-MAX_VALUE"
> +Match on a range of 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 tcp_flags " MASKED_TCP_FLAGS"
>  Match on TCP flags represented as 12bit bitfield in in hexadecimal format.
>  A mask may be optionally provided to limit the bits which are matched. A mask
> 

This prints as:

dst_port NUMBER
src_port NUMBER
      Match  on  layer  4  protocol source or destination port number.
      Only available for ip_proto values udp, tcp and sctp which  have
      to be specified in beforehand.

range MIN_VALUE-MAX_VALUE
      Match  on a range of layer 4 protocol source or destination port
      number. Only available for ip_proto values  udp,  tcp  and  sctp
      which have to be specified in beforehand.

###

That makes it look like range is a standalone option - independent of
dst_port/src_port.

It seems to me the dst_port / src_port should be updated to:

dst_port {NUMBER | range MIN_VALUE-MAX_VALUE}

with the description updated for both options and indented under
dst_port / src_port

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [iproute2-next PATCH v3 1/2] tc: flower: Classify packets based port ranges
  2018-11-16  0:55 [iproute2-next PATCH v3 1/2] tc: flower: Classify packets based port ranges Amritha Nambiar
  2018-11-16  0:55 ` [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option Amritha Nambiar
  2018-11-18 19:30 ` [iproute2-next PATCH v3 1/2] tc: flower: Classify packets based port ranges Jiri Pirko
@ 2018-11-20 23:00 ` David Ahern
  2 siblings, 0 replies; 11+ messages in thread
From: David Ahern @ 2018-11-20 23:00 UTC (permalink / raw)
  To: Amritha Nambiar, stephen, netdev
  Cc: jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong, jiri

On 11/15/18 5:55 PM, Amritha Nambiar wrote:
> Added support for filtering based on port ranges.
> UAPI changes have been accepted into net-next.
> 
> Example:
> 1. Match on a port range:
> -------------------------
> $ tc filter add dev enp4s0 protocol ip parent ffff:\
>   prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
>   action drop
> 
> $ tc -s filter show dev enp4s0 parent ffff:
> filter protocol ip pref 1 flower chain 0
> filter protocol ip pref 1 flower chain 0 handle 0x1
>   eth_type ipv4
>   ip_proto tcp
>   dst_port range 20-30
>   skip_hw
>   not_in_hw
>         action order 1: gact action drop
>          random type none pass val 0
>          index 1 ref 1 bind 1 installed 85 sec used 3 sec
>         Action statistics:
>         Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
>         backlog 0b 0p requeues 0
> 
> 2. Match on IP address and port range:
> --------------------------------------
> $ tc filter add dev enp4s0 protocol ip parent ffff:\
>   prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
>   skip_hw action drop
> 
> $ tc -s filter show dev enp4s0 parent ffff:
> filter protocol ip pref 1 flower chain 0 handle 0x2
>   eth_type ipv4
>   ip_proto tcp
>   dst_ip 192.168.1.1
>   dst_port range 100-200
>   skip_hw
>   not_in_hw
>         action order 1: gact action drop
>          random type none pass val 0
>          index 2 ref 1 bind 1 installed 58 sec used 2 sec
>         Action statistics:
>         Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
>         backlog 0b 0p requeues 0
> 
> v3:
> Modified flower_port_range_attr_type calls.
> 
> v2:
> Addressed Jiri's comment to sync output format with input
> 
> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
> ---
>  include/uapi/linux/pkt_cls.h |    7 ++
>  tc/f_flower.c                |  143 +++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 140 insertions(+), 10 deletions(-)
> 

applied to iproute2-next. Thanks

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
  2018-11-20 22:56   ` David Ahern
@ 2018-11-21  4:44     ` Nambiar, Amritha
  2018-11-21  4:46       ` David Ahern
  0 siblings, 1 reply; 11+ messages in thread
From: Nambiar, Amritha @ 2018-11-21  4:44 UTC (permalink / raw)
  To: David Ahern, stephen, netdev
  Cc: jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong, jiri

On 11/20/2018 2:56 PM, David Ahern wrote:
> On 11/15/18 5:55 PM, Amritha Nambiar wrote:
>> Add details explaining filtering based on port ranges.
>>
>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>> ---
>>  man/man8/tc-flower.8 |   12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
>> index 8be8882..768bfa1 100644
>> --- a/man/man8/tc-flower.8
>> +++ b/man/man8/tc-flower.8
>> @@ -56,8 +56,10 @@ flower \- flow based traffic control filter
>>  .IR MASKED_IP_TTL " | { "
>>  .BR dst_ip " | " src_ip " } "
>>  .IR PREFIX " | { "
>> -.BR dst_port " | " src_port " } "
>> -.IR port_number " } | "
>> +.BR dst_port " | " src_port " } { "
>> +.IR port_number " | "
>> +.B range
>> +.IR min_port_number-max_port_number " } | "
>>  .B tcp_flags
>>  .IR MASKED_TCP_FLAGS " | "
>>  .B type
>> @@ -227,6 +229,12 @@ 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 range " MIN_VALUE-MAX_VALUE"
>> +Match on a range of 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 tcp_flags " MASKED_TCP_FLAGS"
>>  Match on TCP flags represented as 12bit bitfield in in hexadecimal format.
>>  A mask may be optionally provided to limit the bits which are matched. A mask
>>
> 
> This prints as:
> 
> dst_port NUMBER
> src_port NUMBER
>       Match  on  layer  4  protocol source or destination port number.
>       Only available for ip_proto values udp, tcp and sctp which  have
>       to be specified in beforehand.
> 
> range MIN_VALUE-MAX_VALUE
>       Match  on a range of layer 4 protocol source or destination port
>       number. Only available for ip_proto values  udp,  tcp  and  sctp
>       which have to be specified in beforehand.
> 
> ###
> 
> That makes it look like range is a standalone option - independent of
> dst_port/src_port.
> 
> It seems to me the dst_port / src_port should be updated to:
> 
> dst_port {NUMBER | range MIN_VALUE-MAX_VALUE}
> 
> with the description updated for both options and indented under
> dst_port / src_port
> 

Okay, will do.

- Amritha

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
  2018-11-21  4:44     ` Nambiar, Amritha
@ 2018-11-21  4:46       ` David Ahern
  2018-11-21  4:59         ` Nambiar, Amritha
  0 siblings, 1 reply; 11+ messages in thread
From: David Ahern @ 2018-11-21  4:46 UTC (permalink / raw)
  To: Nambiar, Amritha, stephen, netdev
  Cc: jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong, jiri

On 11/20/18 9:44 PM, Nambiar, Amritha wrote:
> On 11/20/2018 2:56 PM, David Ahern wrote:
>> On 11/15/18 5:55 PM, Amritha Nambiar wrote:
>>> Add details explaining filtering based on port ranges.
>>>
>>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>>> ---
>>>  man/man8/tc-flower.8 |   12 ++++++++++--
>>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
>>> index 8be8882..768bfa1 100644
>>> --- a/man/man8/tc-flower.8
>>> +++ b/man/man8/tc-flower.8
>>> @@ -56,8 +56,10 @@ flower \- flow based traffic control filter
>>>  .IR MASKED_IP_TTL " | { "
>>>  .BR dst_ip " | " src_ip " } "
>>>  .IR PREFIX " | { "
>>> -.BR dst_port " | " src_port " } "
>>> -.IR port_number " } | "
>>> +.BR dst_port " | " src_port " } { "
>>> +.IR port_number " | "
>>> +.B range
>>> +.IR min_port_number-max_port_number " } | "
>>>  .B tcp_flags
>>>  .IR MASKED_TCP_FLAGS " | "
>>>  .B type
>>> @@ -227,6 +229,12 @@ 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 range " MIN_VALUE-MAX_VALUE"
>>> +Match on a range of 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 tcp_flags " MASKED_TCP_FLAGS"
>>>  Match on TCP flags represented as 12bit bitfield in in hexadecimal format.
>>>  A mask may be optionally provided to limit the bits which are matched. A mask
>>>
>>
>> This prints as:
>>
>> dst_port NUMBER
>> src_port NUMBER
>>       Match  on  layer  4  protocol source or destination port number.
>>       Only available for ip_proto values udp, tcp and sctp which  have
>>       to be specified in beforehand.
>>
>> range MIN_VALUE-MAX_VALUE
>>       Match  on a range of layer 4 protocol source or destination port
>>       number. Only available for ip_proto values  udp,  tcp  and  sctp
>>       which have to be specified in beforehand.
>>
>> ###
>>
>> That makes it look like range is a standalone option - independent of
>> dst_port/src_port.
>>
>> It seems to me the dst_port / src_port should be updated to:
>>
>> dst_port {NUMBER | range MIN_VALUE-MAX_VALUE}
>>
>> with the description updated for both options and indented under
>> dst_port / src_port
>>
> 
> Okay, will do.
> 

Thinking about this perhaps the 'range' keyword can just be dropped. We
do not use it in other places -- e.g., ip rule.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
  2018-11-21  4:46       ` David Ahern
@ 2018-11-21  4:59         ` Nambiar, Amritha
  2018-11-21  4:59           ` David Ahern
  0 siblings, 1 reply; 11+ messages in thread
From: Nambiar, Amritha @ 2018-11-21  4:59 UTC (permalink / raw)
  To: David Ahern, stephen, netdev
  Cc: jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong, jiri

On 11/20/2018 8:46 PM, David Ahern wrote:
> On 11/20/18 9:44 PM, Nambiar, Amritha wrote:
>> On 11/20/2018 2:56 PM, David Ahern wrote:
>>> On 11/15/18 5:55 PM, Amritha Nambiar wrote:
>>>> Add details explaining filtering based on port ranges.
>>>>
>>>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>>>> ---
>>>>  man/man8/tc-flower.8 |   12 ++++++++++--
>>>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
>>>> index 8be8882..768bfa1 100644
>>>> --- a/man/man8/tc-flower.8
>>>> +++ b/man/man8/tc-flower.8
>>>> @@ -56,8 +56,10 @@ flower \- flow based traffic control filter
>>>>  .IR MASKED_IP_TTL " | { "
>>>>  .BR dst_ip " | " src_ip " } "
>>>>  .IR PREFIX " | { "
>>>> -.BR dst_port " | " src_port " } "
>>>> -.IR port_number " } | "
>>>> +.BR dst_port " | " src_port " } { "
>>>> +.IR port_number " | "
>>>> +.B range
>>>> +.IR min_port_number-max_port_number " } | "
>>>>  .B tcp_flags
>>>>  .IR MASKED_TCP_FLAGS " | "
>>>>  .B type
>>>> @@ -227,6 +229,12 @@ 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 range " MIN_VALUE-MAX_VALUE"
>>>> +Match on a range of 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 tcp_flags " MASKED_TCP_FLAGS"
>>>>  Match on TCP flags represented as 12bit bitfield in in hexadecimal format.
>>>>  A mask may be optionally provided to limit the bits which are matched. A mask
>>>>
>>>
>>> This prints as:
>>>
>>> dst_port NUMBER
>>> src_port NUMBER
>>>       Match  on  layer  4  protocol source or destination port number.
>>>       Only available for ip_proto values udp, tcp and sctp which  have
>>>       to be specified in beforehand.
>>>
>>> range MIN_VALUE-MAX_VALUE
>>>       Match  on a range of layer 4 protocol source or destination port
>>>       number. Only available for ip_proto values  udp,  tcp  and  sctp
>>>       which have to be specified in beforehand.
>>>
>>> ###
>>>
>>> That makes it look like range is a standalone option - independent of
>>> dst_port/src_port.
>>>
>>> It seems to me the dst_port / src_port should be updated to:
>>>
>>> dst_port {NUMBER | range MIN_VALUE-MAX_VALUE}
>>>
>>> with the description updated for both options and indented under
>>> dst_port / src_port
>>>
>>
>> Okay, will do.
>>
> 
> Thinking about this perhaps the 'range' keyword can just be dropped. We
> do not use it in other places -- e.g., ip rule.
> 

Oops, submitted the v2 patch for man changes too soon, without seeing
this. So, in this case, should I re-submit the iproute2-flower patch
that was accepted removing the 'range' keyword?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
  2018-11-21  4:59         ` Nambiar, Amritha
@ 2018-11-21  4:59           ` David Ahern
  2018-11-21  5:16             ` Nambiar, Amritha
  2018-11-21  8:50             ` Jiri Pirko
  0 siblings, 2 replies; 11+ messages in thread
From: David Ahern @ 2018-11-21  4:59 UTC (permalink / raw)
  To: Nambiar, Amritha, stephen, netdev
  Cc: jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong, jiri

On 11/20/18 9:59 PM, Nambiar, Amritha wrote:
> Oops, submitted the v2 patch for man changes too soon, without seeing
> this. So, in this case, should I re-submit the iproute2-flower patch
> that was accepted removing the 'range' keyword?

I think so. Consistency across commands is a good thing.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
  2018-11-21  4:59           ` David Ahern
@ 2018-11-21  5:16             ` Nambiar, Amritha
  2018-11-21  8:50             ` Jiri Pirko
  1 sibling, 0 replies; 11+ messages in thread
From: Nambiar, Amritha @ 2018-11-21  5:16 UTC (permalink / raw)
  To: David Ahern, stephen, netdev
  Cc: jakub.kicinski, sridhar.samudrala, jhs, xiyou.wangcong, jiri

On 11/20/2018 8:59 PM, David Ahern wrote:
> On 11/20/18 9:59 PM, Nambiar, Amritha wrote:
>> Oops, submitted the v2 patch for man changes too soon, without seeing
>> this. So, in this case, should I re-submit the iproute2-flower patch
>> that was accepted removing the 'range' keyword?
> 
> I think so. Consistency across commands is a good thing.
> 

Okay, will do. I'll also combine the 'man patch' into 'flower patch' and
make a single patch as Jiri recommended.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option
  2018-11-21  4:59           ` David Ahern
  2018-11-21  5:16             ` Nambiar, Amritha
@ 2018-11-21  8:50             ` Jiri Pirko
  1 sibling, 0 replies; 11+ messages in thread
From: Jiri Pirko @ 2018-11-21  8:50 UTC (permalink / raw)
  To: David Ahern
  Cc: Nambiar, Amritha, stephen, netdev, jakub.kicinski,
	sridhar.samudrala, jhs, xiyou.wangcong

Wed, Nov 21, 2018 at 05:59:45AM CET, dsahern@gmail.com wrote:
>On 11/20/18 9:59 PM, Nambiar, Amritha wrote:
>> Oops, submitted the v2 patch for man changes too soon, without seeing
>> this. So, in this case, should I re-submit the iproute2-flower patch
>> that was accepted removing the 'range' keyword?
>
>I think so. Consistency across commands is a good thing.

+1

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-11-21 19:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-16  0:55 [iproute2-next PATCH v3 1/2] tc: flower: Classify packets based port ranges Amritha Nambiar
2018-11-16  0:55 ` [iproute2-next PATCH v3 2/2] man: tc-flower: Add explanation for range option Amritha Nambiar
2018-11-20 22:56   ` David Ahern
2018-11-21  4:44     ` Nambiar, Amritha
2018-11-21  4:46       ` David Ahern
2018-11-21  4:59         ` Nambiar, Amritha
2018-11-21  4:59           ` David Ahern
2018-11-21  5:16             ` Nambiar, Amritha
2018-11-21  8:50             ` Jiri Pirko
2018-11-18 19:30 ` [iproute2-next PATCH v3 1/2] tc: flower: Classify packets based port ranges Jiri Pirko
2018-11-20 23:00 ` David Ahern

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.