DPDK patches and discussions
 help / color / mirror / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download: 
* [dpdk-dev] can't build memnic pmd
@ 2015-04-15 16:49 13% Sowmini Varadhan
  0 siblings, 0 replies; 38+ results
From: Sowmini Varadhan @ 2015-04-15 16:49 UTC (permalink / raw)
  To: dev

I am trying to build memnic-1.3/pmd on a qemu-kvm running
4.0.0-rc7+ with gcc 4.8.2, and it fails with a few errors, the first of
which is
/path/to/dpdk-2.0.0/build/include/rte_memcpy.h:625:2: error: implicit
declaration of function ?_mm_alignr_epi8?
[-Werror=implicit-function-declaration]
  MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);

I see that rte_vect.h only includes tmmintrin.h for __GNUC_MINOR__ < 4,
and I probably need to do other things to enable SSSE3 or its equivalent.
Question is- what are they?

More generally, what are the steps to getting the l2fwd-ivshmem examples
from dpdk to work between 2 kvms? I was not able to find a simple README
for this.

Thanks in advance,

--Sowmini

^ permalink raw reply	[relevance 13%]

* [dpdk-dev] running l2fwd-ivshmem
@ 2015-04-28 23:45 13% Sowmini Varadhan
  0 siblings, 0 replies; 38+ results
From: Sowmini Varadhan @ 2015-04-28 23:45 UTC (permalink / raw)
  To: dev

hello,
has anyone out there successfully run the l2fwd-ivshmem
example successfully between 2 kvms, and got some notes
or instructions to share?

I see I'm not the first one to struggle with this:
  http://dpdk.org/ml/archives/dev/2014-June/003556.html

but memnic-pmd doesnt build with the latest kernels either,
   http://www.litchfie.dpdk.org/ml/archives/dev/2015-April/016500.html

Examples accompanied by READMEs would be a good idea :-)

Any help is appreciated, including redirects to other groups where
I may find the answers to the questions above.

Thanks
Sowmini

^ permalink raw reply	[relevance 13%]

* [dpdk-dev] [PATCH RFC 0/2] TCP flow classification using 4-tuple and flags
@ 2020-01-12 23:08 21% Sowmini Varadhan
  2020-01-12 23:08 19% ` [dpdk-dev] [PATCH RFC 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
  2020-01-12 23:08 18% ` [dpdk-dev] [PATCH RFC 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
  0 siblings, 2 replies; 38+ results
From: Sowmini Varadhan @ 2020-01-12 23:08 UTC (permalink / raw)
  To: sowmini05, dev

An interesting class of problems is  TCP flow tracking 
and classification based on TCP state, which  requires 
the ability to classify TCP flows on more packet properties 
than just the 4-tuple

This patch-set investigates the set of changes needed in the 
examples/flow_classify.c needed to achieve  one instance of
this class of problems by adding hooks to filter/classify 
on both the 4-tuple and tcp flags.

Patch 1 extends examples/flow_classify.c to allow constraints
on tcp flags. Patch 2 extends the ACL handling in  
librte_flow_classify to include keys on the properties in 
addition to the tcp 4-tuple.

Note that one particular part of this patch-set where feedback
is requested is in allocate_acl_ipv4_tcp_5tuple_rule():
we need to add a key for the 8 bit flags, but the multibit
trie lookup moves in steps of 4 bytes, so it took some hackery
to figure out what byte-ordering was expected, and there were
no documentation/examples to provide guidelines. Comments/suggestions
would be particularly helpful.

Sowmini Varadhan (2):
  Hooks to allow the setting of filters on tcp flags
  Allow the flow_classify example to add an ACL table for tcp.

 examples/flow_classify/flow_classify.c        | 113 ++++++++++++++++--
 examples/flow_classify/ipv4_rules_file.txt    |  22 ++--
 lib/librte_flow_classify/rte_flow_classify.c  |  84 +++++++++++++
 lib/librte_flow_classify/rte_flow_classify.h  |  19 +++
 .../rte_flow_classify_parse.c                 |   8 +-
 5 files changed, 221 insertions(+), 25 deletions(-)

-- 
2.20.1


^ permalink raw reply	[relevance 21%]

* [dpdk-dev] [PATCH RFC 1/2] Hooks to allow the setting of filters on tcp flags
  2020-01-12 23:08 21% [dpdk-dev] [PATCH RFC 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
@ 2020-01-12 23:08 19% ` Sowmini Varadhan
  2020-01-12 23:08 18% ` [dpdk-dev] [PATCH RFC 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
  1 sibling, 0 replies; 38+ results
From: Sowmini Varadhan @ 2020-01-12 23:08 UTC (permalink / raw)
  To: sowmini05, dev

The rte_eth_ntuple_filter allows tcp_flags which can check for things like
        #define RTE_TCP_CWR_FLAG 0x80 /**< Congestion Window Reduced */
        #define RTE_TCP_ECE_FLAG 0x40 /**< ECN-Echo */
        #define RTE_TCP_URG_FLAG 0x20 /**< Urgent Pointer field significant */
        #define RTE_TCP_ACK_FLAG 0x10 /**< Acknowledgment field significant */
        #define RTE_TCP_PSH_FLAG 0x08 /**< Push Function */
        #define RTE_TCP_RST_FLAG 0x04 /**< Reset the connection */
        #define RTE_TCP_SYN_FLAG 0x02 /**< Synchronize sequence numbers */
        #define RTE_TCP_FIN_FLAG 0x01 /**< No more data from sender */
but there are no existing examples that demonstrate how to use this feature.

This patch extends the exisiting classification support to allow
an optional flags in the input file. The flags string can be any
concatenation of characters from {C, E, U, A, P, R, S, F} and
"*" indicates "dont care". These flags are set in the ntuple_filter and
are used to construct the tcp_spec and tcp_mask sent to the driver

The rte_acl_field_def is updated to use the (u8) tcp flag as lookup key.
Note that, as per
  https://doc.dpdk.org/guides/prog_guide/packet_classif_access_ctrl.html
this field MUST be allocated fo4 4 bytes, thus it has sizeof(uint32_t).

However, also note the XXX in this commit: additional updates are
needed to the rte_flow_classify_table_entry_add() so that it does
not ignore any key fields other than the 5-tuple.

Signed-off-by: Sowmini Varadhan <sowmini05@gmail.com>
---
 examples/flow_classify/flow_classify.c     | 88 ++++++++++++++++++++--
 examples/flow_classify/ipv4_rules_file.txt | 22 +++---
 2 files changed, 91 insertions(+), 19 deletions(-)

diff --git a/examples/flow_classify/flow_classify.c b/examples/flow_classify/flow_classify.c
index 1c12bbb2f..e74a53be7 100644
--- a/examples/flow_classify/flow_classify.c
+++ b/examples/flow_classify/flow_classify.c
@@ -51,6 +51,7 @@ enum {
 	CB_FLD_DST_PORT_MASK,
 	CB_FLD_PROTO,
 	CB_FLD_PRIORITY,
+	CB_FLD_TCP_FLAGS,
 	CB_FLD_NUM,
 };
 
@@ -81,6 +82,7 @@ enum {
 	DST_FIELD_IPV4,
 	SRCP_FIELD_IPV4,
 	DSTP_FIELD_IPV4,
+	TCP_FLAGS_FIELD,
 	NUM_FIELDS_IPV4
 };
 
@@ -88,7 +90,8 @@ enum {
 	PROTO_INPUT_IPV4,
 	SRC_INPUT_IPV4,
 	DST_INPUT_IPV4,
-	SRCP_DESTP_INPUT_IPV4
+	SRCP_DESTP_INPUT_IPV4,
+	TCP_FLAGS_INDEX,
 };
 
 static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
@@ -145,6 +148,17 @@ static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
 			sizeof(struct rte_ipv4_hdr) +
 			offsetof(struct rte_tcp_hdr, dst_port),
 	},
+	/* next field must be 4 bytes, even though flags is only 1 byte */
+	{
+		/* rte_flags */
+		.type = RTE_ACL_FIELD_TYPE_BITMASK,
+		.size = sizeof(uint32_t),
+		.field_index = TCP_FLAGS_FIELD,
+		.input_index = TCP_FLAGS_INDEX,
+		.offset = sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr) +
+			offsetof(struct rte_tcp_hdr, tcp_flags),
+	},
 };
 
 /* flow classify data */
@@ -272,12 +286,14 @@ lcore_main(struct flow_classifier *cls_app)
 	int ret;
 	int i = 0;
 
-	ret = rte_flow_classify_table_entry_delete(cls_app->cls,
-			rules[7]);
-	if (ret)
-		printf("table_entry_delete failed [7] %d\n\n", ret);
-	else
-		printf("table_entry_delete succeeded [7]\n\n");
+	if (rules[7]) {
+		ret = rte_flow_classify_table_entry_delete(cls_app->cls,
+				rules[7]);
+		if (ret)
+			printf("table_entry_delete failed [7] %d\n\n", ret);
+		else
+			printf("table_entry_delete succeeded [7]\n\n");
+	}
 
 	/*
 	 * Check that the port is on the same NUMA node as the polling thread
@@ -395,6 +411,53 @@ parse_ipv4_net(char *in, uint32_t *addr, uint32_t *mask_len)
 	return 0;
 }
 
+static int 
+get_tcp_flags(char *in, struct rte_eth_ntuple_filter *ntuple_filter)
+{
+	int len = strlen(in);
+	int i;
+	uint8_t flags = 0;
+
+	if (strcmp(in, "*") == 0) {
+		ntuple_filter->tcp_flags = 0;
+		return 0;
+	}
+
+	for (i = 0; i < len; i++) {
+		switch (in[i]) {
+		case 'S': 
+			flags |= RTE_TCP_SYN_FLAG;
+			break;
+		case 'F':
+			flags |= RTE_TCP_FIN_FLAG;
+			break;
+		case 'R':
+			flags |= RTE_TCP_RST_FLAG;
+			break;
+		case 'P':
+			flags |= RTE_TCP_PSH_FLAG;
+			break;
+		case 'A':
+			flags |= RTE_TCP_ACK_FLAG;
+			break;
+		case 'U':
+			flags |= RTE_TCP_URG_FLAG;
+			break;
+		case 'E':
+			flags |= RTE_TCP_ECE_FLAG;
+			break;
+		case 'C':
+			flags |= RTE_TCP_CWR_FLAG;
+			break;
+		default:
+			fprintf(stderr, "unknown flag %c\n", in[i]);
+			return -1;
+		}
+	}
+	ntuple_filter->tcp_flags = flags;
+	return 0;
+}
+
 static int
 parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter *ntuple_filter)
 {
@@ -424,7 +487,7 @@ parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter *ntuple_filter)
 			&ntuple_filter->dst_ip,
 			&ntuple_filter->dst_ip_mask);
 	if (ret != 0) {
-		flow_classify_log("failed to read source address/mask: %s\n",
+		flow_classify_log("failed to read ssourceource address/mask: %s\n",
 			in[CB_FLD_DST_ADDR]);
 		return ret;
 	}
@@ -466,6 +529,8 @@ parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter *ntuple_filter)
 	ntuple_filter->priority = (uint16_t)temp;
 	if (ntuple_filter->priority > FLOW_CLASSIFY_MAX_PRIORITY)
 		ret = -EINVAL;
+	if (get_tcp_flags(in[CB_FLD_TCP_FLAGS], ntuple_filter))
+		return -EINVAL;
 
 	return ret;
 }
@@ -582,10 +647,13 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		memset(&tcp_spec, 0, sizeof(tcp_spec));
 		tcp_spec.hdr.src_port = ntuple_filter->src_port;
 		tcp_spec.hdr.dst_port = ntuple_filter->dst_port;
+		tcp_spec.hdr.tcp_flags = ntuple_filter->tcp_flags;
 
 		memset(&tcp_mask, 0, sizeof(tcp_mask));
 		tcp_mask.hdr.src_port = ntuple_filter->src_port_mask;
 		tcp_mask.hdr.dst_port = ntuple_filter->dst_port_mask;
+		if (tcp_spec.hdr.tcp_flags != 0)
+			tcp_mask.hdr.tcp_flags = 0xff;
 
 		tcp_item.type = RTE_FLOW_ITEM_TYPE_TCP;
 		tcp_item.spec = &tcp_spec;
@@ -640,6 +708,10 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		return ret;
 	}
 
+	/* XXX but this only adds table_type of  RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
+	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule() so the tcp_flags is
+	 * ignored! 
+	 */
 	rule = rte_flow_classify_table_entry_add(
 			cls_app->cls, &attr, pattern_ipv4_5tuple,
 			actions, &key_found, &error);
diff --git a/examples/flow_classify/ipv4_rules_file.txt b/examples/flow_classify/ipv4_rules_file.txt
index dfa0631fc..415573732 100644
--- a/examples/flow_classify/ipv4_rules_file.txt
+++ b/examples/flow_classify/ipv4_rules_file.txt
@@ -1,14 +1,14 @@
 #file format:
-#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask proto/mask priority
+#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask proto/mask priority tcp_flags
 #
-2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0
-9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1
-9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2
-9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3
-6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4
-6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5
-6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6
-6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7
-6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8
+2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0 *
+9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1 *
+9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2 *
+9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3 *
+6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4 *
+6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5 *
+6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6 *
+6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7 *
+6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8 *
 #error rules
-#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9
\ No newline at end of file
+#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9 *
-- 
2.20.1


^ permalink raw reply	[relevance 19%]

* [dpdk-dev] [PATCH RFC 2/2] Allow the flow_classify example to add an ACL table for tcp.
  2020-01-12 23:08 21% [dpdk-dev] [PATCH RFC 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
  2020-01-12 23:08 19% ` [dpdk-dev] [PATCH RFC 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
@ 2020-01-12 23:08 18% ` Sowmini Varadhan
  1 sibling, 0 replies; 38+ results
From: Sowmini Varadhan @ 2020-01-12 23:08 UTC (permalink / raw)
  To: sowmini05, dev

The struct rte_flow_classifier can have upto RTE_FLOW_CLASSIFY_TABLE_MAX (32)
classifier tables, but the existing flow_classify examples only adds a single
table for the L4 5-tuple.

When dealing with tcp flows, we frequently want to add add ACLs and filters
to filter based on the state of the TCP connection, e.g., by looking at
the tcp flags field.

So we enhance flow_classify to add an additional acl table for
tcp 5-typles. If the input-file-parser detects a filter for a tcp flow with
a non-wildcard argument for tcp_flags, the IP4_TCP_5TUPLE table is used
by flow_classify.

Signed-off-by: Sowmini Varadhan <sowmini05@gmail.com>
---
 examples/flow_classify/flow_classify.c        | 33 ++++++--
 lib/librte_flow_classify/rte_flow_classify.c  | 84 +++++++++++++++++++
 lib/librte_flow_classify/rte_flow_classify.h  | 19 +++++
 .../rte_flow_classify_parse.c                 |  8 +-
 4 files changed, 134 insertions(+), 10 deletions(-)

diff --git a/examples/flow_classify/flow_classify.c b/examples/flow_classify/flow_classify.c
index e74a53be7..54ae65d46 100644
--- a/examples/flow_classify/flow_classify.c
+++ b/examples/flow_classify/flow_classify.c
@@ -708,10 +708,6 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		return ret;
 	}
 
-	/* XXX but this only adds table_type of  RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
-	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule() so the tcp_flags is
-	 * ignored! 
-	 */
 	rule = rte_flow_classify_table_entry_add(
 			cls_app->cls, &attr, pattern_ipv4_5tuple,
 			actions, &key_found, &error);
@@ -838,7 +834,8 @@ main(int argc, char *argv[])
 	int ret;
 	int socket_id;
 	struct rte_table_acl_params table_acl_params;
-	struct rte_flow_classify_table_params cls_table_params;
+	struct rte_table_acl_params table_acl_tcp_params;
+	struct rte_flow_classify_table_params cls_table_params[2];
 	struct flow_classifier *cls_app;
 	struct rte_flow_classifier_params cls_params;
 	uint32_t size;
@@ -901,16 +898,34 @@ main(int argc, char *argv[])
 	memcpy(table_acl_params.field_format, ipv4_defs, sizeof(ipv4_defs));
 
 	/* initialise table create params */
-	cls_table_params.ops = &rte_table_acl_ops;
-	cls_table_params.arg_create = &table_acl_params;
-	cls_table_params.type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+	cls_table_params[0].ops = &rte_table_acl_ops;
+	cls_table_params[0].arg_create = &table_acl_params;
+	cls_table_params[0].type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+
+	/* initialise ACL table params */
+	table_acl_tcp_params.name = "table_acl_ipv4_tcp_5tuple";
+	table_acl_tcp_params.n_rules = FLOW_CLASSIFY_MAX_RULE_NUM;
+	table_acl_tcp_params.n_rule_fields = RTE_DIM(ipv4_defs);
+	memcpy(table_acl_tcp_params.field_format, ipv4_defs, sizeof(ipv4_defs));
+
+	/* initialise table create params */
+	cls_table_params[1].ops = &rte_table_acl_ops;
+	cls_table_params[1].arg_create = &table_acl_tcp_params;
+	cls_table_params[1].type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
 
-	ret = rte_flow_classify_table_create(cls_app->cls, &cls_table_params);
+	ret = rte_flow_classify_table_create(cls_app->cls, &cls_table_params[0]);
 	if (ret) {
 		rte_flow_classifier_free(cls_app->cls);
 		rte_free(cls_app);
 		rte_exit(EXIT_FAILURE, "Failed to create classifier table\n");
 	}
+	ret = rte_flow_classify_table_create(cls_app->cls, &cls_table_params[1]);
+	if (ret) {
+		rte_flow_classifier_free(cls_app->cls);
+		rte_free(cls_app);
+		rte_exit(EXIT_FAILURE, "Failed to create classifier table\n");
+	}
+
 
 	/* read file of IPv4 5 tuple rules and initialize parameters
 	 * for rte_flow_classify_validate and rte_flow_classify_table_entry_add
diff --git a/lib/librte_flow_classify/rte_flow_classify.c b/lib/librte_flow_classify/rte_flow_classify.c
index 5ff585803..4ec36a397 100644
--- a/lib/librte_flow_classify/rte_flow_classify.c
+++ b/lib/librte_flow_classify/rte_flow_classify.c
@@ -62,6 +62,7 @@ enum {
 	DST_FIELD_IPV4,
 	SRCP_FIELD_IPV4,
 	DSTP_FIELD_IPV4,
+	TCP_FLAGS_FIELD,
 	NUM_FIELDS_IPV4
 };
 
@@ -74,6 +75,7 @@ struct classify_rules {
 	enum rte_flow_classify_rule_type type;
 	union {
 		struct rte_flow_classify_ipv4_5tuple ipv4_5tuple;
+		struct rte_flow_classify_ipv4_tcp_5tuple ipv4_tcp_5tuple;
 	} u;
 };
 
@@ -482,6 +484,81 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls)
 	return rule;
 }
 
+static struct rte_flow_classify_rule *
+allocate_acl_ipv4_tcp_5tuple_rule(struct rte_flow_classifier *cls)
+{
+	struct rte_flow_classify_rule *rule;
+	int log_level;
+
+	rule = malloc(sizeof(struct rte_flow_classify_rule));
+	if (!rule)
+		return rule;
+
+	memset(rule, 0, sizeof(struct rte_flow_classify_rule));
+	rule->id = unique_id++;
+	rule->rules.type = RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE;
+
+	/* key add values */
+	rule->u.key.key_add.priority = cls->ntuple_filter.priority;
+	rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].mask_range.u8 =
+			cls->ntuple_filter.proto_mask;
+	rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].value.u8 =
+			cls->ntuple_filter.proto;
+	rule->rules.u.ipv4_tcp_5tuple.proto = cls->ntuple_filter.proto;
+	rule->rules.u.ipv4_tcp_5tuple.proto_mask = cls->ntuple_filter.proto_mask;
+
+	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].mask_range.u32 =
+			cls->ntuple_filter.src_ip_mask;
+	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].value.u32 =
+			cls->ntuple_filter.src_ip;
+	rule->rules.u.ipv4_tcp_5tuple.src_ip_mask = cls->ntuple_filter.src_ip_mask;
+	rule->rules.u.ipv4_tcp_5tuple.src_ip = cls->ntuple_filter.src_ip;
+
+	rule->u.key.key_add.field_value[DST_FIELD_IPV4].mask_range.u32 =
+			cls->ntuple_filter.dst_ip_mask;
+	rule->u.key.key_add.field_value[DST_FIELD_IPV4].value.u32 =
+			cls->ntuple_filter.dst_ip;
+	rule->rules.u.ipv4_tcp_5tuple.dst_ip_mask = cls->ntuple_filter.dst_ip_mask;
+	rule->rules.u.ipv4_tcp_5tuple.dst_ip = cls->ntuple_filter.dst_ip;
+
+	rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].mask_range.u16 =
+			cls->ntuple_filter.src_port_mask;
+	rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].value.u16 =
+			cls->ntuple_filter.src_port;
+	rule->rules.u.ipv4_tcp_5tuple.src_port_mask =
+			cls->ntuple_filter.src_port_mask;
+	rule->rules.u.ipv4_tcp_5tuple.src_port = cls->ntuple_filter.src_port;
+
+	rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].mask_range.u16 =
+			cls->ntuple_filter.dst_port_mask;
+	rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].value.u16 =
+			cls->ntuple_filter.dst_port;
+	rule->rules.u.ipv4_tcp_5tuple.dst_port_mask =
+			cls->ntuple_filter.dst_port_mask;
+	rule->rules.u.ipv4_tcp_5tuple.dst_port = cls->ntuple_filter.dst_port;
+
+	rule->u.key.key_add.field_value[TCP_FLAGS_FIELD].mask_range.u32 =
+			rte_be_to_cpu_32(0xff);
+	rule->u.key.key_add.field_value[TCP_FLAGS_FIELD].value.u32 =
+			rte_be_to_cpu_32(cls->ntuple_filter.tcp_flags);
+	rule->rules.u.ipv4_tcp_5tuple.tcp_flags = cls->ntuple_filter.tcp_flags;
+
+	log_level = rte_log_get_level(librte_flow_classify_logtype);
+
+	if (log_level == RTE_LOG_DEBUG)
+		print_acl_ipv4_key_add(&rule->u.key.key_add);
+
+	/* key delete values */
+	memcpy(&rule->u.key.key_del.field_value[PROTO_FIELD_IPV4],
+	       &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4],
+	       NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field));
+
+	if (log_level == RTE_LOG_DEBUG)
+		print_acl_ipv4_key_delete(&rule->u.key.key_del);
+
+	return rule;
+}
+
 struct rte_flow_classify_rule *
 rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls,
 		const struct rte_flow_attr *attr,
@@ -519,6 +596,13 @@ rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls,
 		rule->tbl_type = table_type;
 		cls->table_mask |= table_type;
 		break;
+	case RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE:
+		rule = allocate_acl_ipv4_tcp_5tuple_rule(cls);
+		if (!rule)
+			return NULL;
+		rule->tbl_type = table_type;
+		cls->table_mask |= table_type;
+		break;
 	default:
 		return NULL;
 	}
diff --git a/lib/librte_flow_classify/rte_flow_classify.h b/lib/librte_flow_classify/rte_flow_classify.h
index 74d1ecaf5..921277f90 100644
--- a/lib/librte_flow_classify/rte_flow_classify.h
+++ b/lib/librte_flow_classify/rte_flow_classify.h
@@ -78,6 +78,8 @@ enum rte_flow_classify_rule_type {
 	RTE_FLOW_CLASSIFY_RULE_TYPE_NONE,
 	/** IPv4 5tuple type */
 	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_5TUPLE,
+	/** IPv4 TCP 5tuple type */
+	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE,
 };
 
 /** Flow classify table type */
@@ -90,6 +92,8 @@ enum rte_flow_classify_table_type {
 	RTE_FLOW_CLASSIFY_TABLE_ACL_VLAN_IP4_5TUPLE = 1 << 2,
 	/** ACL QinQ IP4 5TUPLE */
 	RTE_FLOW_CLASSIFY_TABLE_ACL_QINQ_IP4_5TUPLE = 1 << 3,
+	/** ACL IP4 5TUPLE with tcp_flags */
+	RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE = 1 << 4,
 
 };
 
@@ -129,6 +133,21 @@ struct rte_flow_classify_ipv4_5tuple {
 	uint8_t proto_mask;      /**< Mask of L4 protocol. */
 };
 
+/** IPv4 5-tuple data with tcp flags*/
+struct rte_flow_classify_ipv4_tcp_5tuple {
+	uint32_t dst_ip;         /**< Destination IP address in big endian. */
+	uint32_t dst_ip_mask;    /**< Mask of destination IP address. */
+	uint32_t src_ip;         /**< Source IP address in big endian. */
+	uint32_t src_ip_mask;    /**< Mask of destination IP address. */
+	uint16_t dst_port;       /**< Destination port in big endian. */
+	uint16_t dst_port_mask;  /**< Mask of destination port. */
+	uint16_t src_port;       /**< Source Port in big endian. */
+	uint16_t src_port_mask;  /**< Mask of source port. */
+	uint8_t proto;           /**< L4 protocol. */
+	uint8_t proto_mask;      /**< Mask of L4 protocol. */
+	uint8_t tcp_flags;       /**< Tcp only */
+};
+
 /**
  * Flow stats
  *
diff --git a/lib/librte_flow_classify/rte_flow_classify_parse.c b/lib/librte_flow_classify/rte_flow_classify_parse.c
index 465330291..fe4ee05b6 100644
--- a/lib/librte_flow_classify/rte_flow_classify_parse.c
+++ b/lib/librte_flow_classify/rte_flow_classify_parse.c
@@ -216,6 +216,7 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 	const struct rte_flow_action_count *count;
 	const struct rte_flow_action_mark *mark_spec;
 	uint32_t index;
+	bool have_tcp = false;
 
 	/* parse pattern */
 	index = 0;
@@ -375,6 +376,8 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		filter->dst_port  = tcp_spec->hdr.dst_port;
 		filter->src_port  = tcp_spec->hdr.src_port;
 		filter->tcp_flags = tcp_spec->hdr.tcp_flags;
+		if (filter->tcp_flags != 0)
+			have_tcp = true;
 	} else if (item->type == RTE_FLOW_ITEM_TYPE_UDP) {
 		udp_mask = item->mask;
 
@@ -434,7 +437,10 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		return -EINVAL;
 	}
 
-	table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+	if (have_tcp)
+		table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
+	else
+		table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
 
 	/* parse attr */
 	/* must be input direction */
-- 
2.20.1


^ permalink raw reply	[relevance 18%]

* [dpdk-dev] [PATCH RFC V2 0/2] TCP flow classification using 4-tuple and flags
@ 2020-01-13 18:05 21% Sowmini Varadhan
  2020-01-13 18:05 20% ` [dpdk-dev] [PATCH RFC V2 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
                   ` (2 more replies)
  0 siblings, 3 replies; 38+ results
From: Sowmini Varadhan @ 2020-01-13 18:05 UTC (permalink / raw)
  To: sowmini05, dev

V2 updates: checkpatch fixes, revert accidently spelling error
introduced in V1;

The problem space of TCP flow tracking and classification
based on TCP state requires the ability to classify TCP
flows on more packet properties than just the 4-tuple,
e.g., TCP flags. This patch-set investigates the set of
changes needed in the examples/flow_classify.c needed to
achieve this.

Patch 1 extends examples/flow_classify.c to allow constraints
on tcp flags. Patch 2 extends the ACL handling in  
librte_flow_classify to include keys on the properties in 
addition to the tcp 4-tuple.

Note that one particular part of this patch-set where feedback
is requested is in allocate_acl_ipv4_tcp_5tuple_rule():
we need to add a key for the 8 bit flags, but the multibit
trie lookup moves in steps of 4 bytes, so it took some hackery
to figure out what byte-ordering was expected, and there were
no documentation/examples to provide guidelines. Comments/suggestions
would be particularly helpful.

Sowmini Varadhan (2):
  Hooks to allow the setting of filters on tcp flags
  Allow the flow_classify example to add an ACL table for tcp.

 examples/flow_classify/flow_classify.c        | 121 +++++++++++++++---
 examples/flow_classify/ipv4_rules_file.txt    |  22 ++--
 lib/librte_flow_classify/rte_flow_classify.c  |  87 +++++++++++++
 lib/librte_flow_classify/rte_flow_classify.h  |  19 +++
 .../rte_flow_classify_parse.c                 |   8 +-
 5 files changed, 230 insertions(+), 27 deletions(-)

-- 
2.20.1


^ permalink raw reply	[relevance 21%]

* [dpdk-dev] [PATCH RFC V2 1/2] Hooks to allow the setting of filters on tcp flags
  2020-01-13 18:05 21% [dpdk-dev] [PATCH RFC V2 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
@ 2020-01-13 18:05 20% ` Sowmini Varadhan
  2020-01-13 18:05 18% ` [dpdk-dev] [PATCH RFC V2 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
  2023-06-12 21:14  6% ` [dpdk-dev] [PATCH RFC V2 0/2] TCP flow classification using 4-tuple and flags Stephen Hemminger
  2 siblings, 0 replies; 38+ results
From: Sowmini Varadhan @ 2020-01-13 18:05 UTC (permalink / raw)
  To: sowmini05, dev


The rte_eth_ntuple_filter allows tcp_flags which can check for things
like
    #define RTE_TCP_CWR_FLAG 0x80 /**< Congestion Window Reduced */
    #define RTE_TCP_ECE_FLAG 0x40 /**< ECN-Echo */
    #define RTE_TCP_URG_FLAG 0x20 /**< Urgent Pointer field significant */
    #define RTE_TCP_ACK_FLAG 0x10 /**< Acknowledgment field significant */
    #define RTE_TCP_PSH_FLAG 0x08 /**< Push Function */
    #define RTE_TCP_RST_FLAG 0x04 /**< Reset the connection */
    #define RTE_TCP_SYN_FLAG 0x02 /**< Synchronize sequence numbers */
    #define RTE_TCP_FIN_FLAG 0x01 /**< No more data from sender */
but there are no existing examples that demonstrate how to use this
feature.

This patch extends the exisiting classification support to allow
an optional flags in the input file. The flags string can be any
concatenation of characters from {C, E, U, A, P, R, S, F} and
"*" indicates "dont care". These flags are set in the ntuple_filter and
are used to construct the tcp_spec and tcp_mask sent to the driver

The rte_acl_field_def is updated to use the (u8) tcp flag as lookup key.
Note that, as per
  https://doc.dpdk.org/guides/prog_guide/packet_classif_access_ctrl.html
this field MUST be allocated fo4 4 bytes, thus it has sizeof(uint32_t).

However, also note the XXX in this commit: additional updates are
needed to the rte_flow_classify_table_entry_add() so that it does
not ignore any key fields other than the 5-tuple.

Signed-off-by: Sowmini Varadhan <sowmini05@gmail.com>
---
V2 : checkpatch fixes; revert accidental spelling mistake introduced in V1.

 examples/flow_classify/flow_classify.c     | 87 ++++++++++++++++++++--
 examples/flow_classify/ipv4_rules_file.txt | 22 +++---
 2 files changed, 91 insertions(+), 18 deletions(-)

diff --git a/examples/flow_classify/flow_classify.c b/examples/flow_classify/flow_classify.c
index 1c12bbb2f..e8a41741f 100644
--- a/examples/flow_classify/flow_classify.c
+++ b/examples/flow_classify/flow_classify.c
@@ -51,6 +51,7 @@ enum {
 	CB_FLD_DST_PORT_MASK,
 	CB_FLD_PROTO,
 	CB_FLD_PRIORITY,
+	CB_FLD_TCP_FLAGS,
 	CB_FLD_NUM,
 };
 
@@ -81,6 +82,7 @@ enum {
 	DST_FIELD_IPV4,
 	SRCP_FIELD_IPV4,
 	DSTP_FIELD_IPV4,
+	TCP_FLAGS_FIELD,
 	NUM_FIELDS_IPV4
 };
 
@@ -88,7 +90,8 @@ enum {
 	PROTO_INPUT_IPV4,
 	SRC_INPUT_IPV4,
 	DST_INPUT_IPV4,
-	SRCP_DESTP_INPUT_IPV4
+	SRCP_DESTP_INPUT_IPV4,
+	TCP_FLAGS_INDEX,
 };
 
 static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
@@ -145,6 +148,17 @@ static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
 			sizeof(struct rte_ipv4_hdr) +
 			offsetof(struct rte_tcp_hdr, dst_port),
 	},
+	/* next field must be 4 bytes, even though flags is only 1 byte */
+	{
+		/* rte_flags */
+		.type = RTE_ACL_FIELD_TYPE_BITMASK,
+		.size = sizeof(uint32_t),
+		.field_index = TCP_FLAGS_FIELD,
+		.input_index = TCP_FLAGS_INDEX,
+		.offset = sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr) +
+			offsetof(struct rte_tcp_hdr, tcp_flags),
+	},
 };
 
 /* flow classify data */
@@ -272,12 +286,14 @@ lcore_main(struct flow_classifier *cls_app)
 	int ret;
 	int i = 0;
 
-	ret = rte_flow_classify_table_entry_delete(cls_app->cls,
-			rules[7]);
-	if (ret)
-		printf("table_entry_delete failed [7] %d\n\n", ret);
-	else
-		printf("table_entry_delete succeeded [7]\n\n");
+	if (rules[7]) {
+		ret = rte_flow_classify_table_entry_delete(cls_app->cls,
+				rules[7]);
+		if (ret)
+			printf("table_entry_delete failed [7] %d\n\n", ret);
+		else
+			printf("table_entry_delete succeeded [7]\n\n");
+	}
 
 	/*
 	 * Check that the port is on the same NUMA node as the polling thread
@@ -395,6 +411,53 @@ parse_ipv4_net(char *in, uint32_t *addr, uint32_t *mask_len)
 	return 0;
 }
 
+static int
+get_tcp_flags(char *in, struct rte_eth_ntuple_filter *ntuple_filter)
+{
+	int len = strlen(in);
+	int i;
+	uint8_t flags = 0;
+
+	if (strcmp(in, "*") == 0) {
+		ntuple_filter->tcp_flags = 0;
+		return 0;
+	}
+
+	for (i = 0; i < len; i++) {
+		switch (in[i]) {
+		case 'S':
+			flags |= RTE_TCP_SYN_FLAG;
+			break;
+		case 'F':
+			flags |= RTE_TCP_FIN_FLAG;
+			break;
+		case 'R':
+			flags |= RTE_TCP_RST_FLAG;
+			break;
+		case 'P':
+			flags |= RTE_TCP_PSH_FLAG;
+			break;
+		case 'A':
+			flags |= RTE_TCP_ACK_FLAG;
+			break;
+		case 'U':
+			flags |= RTE_TCP_URG_FLAG;
+			break;
+		case 'E':
+			flags |= RTE_TCP_ECE_FLAG;
+			break;
+		case 'C':
+			flags |= RTE_TCP_CWR_FLAG;
+			break;
+		default:
+			fprintf(stderr, "unknown flag %c\n", in[i]);
+			return -1;
+		}
+	}
+	ntuple_filter->tcp_flags = flags;
+	return 0;
+}
+
 static int
 parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter *ntuple_filter)
 {
@@ -466,6 +529,8 @@ parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter *ntuple_filter)
 	ntuple_filter->priority = (uint16_t)temp;
 	if (ntuple_filter->priority > FLOW_CLASSIFY_MAX_PRIORITY)
 		ret = -EINVAL;
+	if (get_tcp_flags(in[CB_FLD_TCP_FLAGS], ntuple_filter))
+		return -EINVAL;
 
 	return ret;
 }
@@ -582,10 +647,13 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		memset(&tcp_spec, 0, sizeof(tcp_spec));
 		tcp_spec.hdr.src_port = ntuple_filter->src_port;
 		tcp_spec.hdr.dst_port = ntuple_filter->dst_port;
+		tcp_spec.hdr.tcp_flags = ntuple_filter->tcp_flags;
 
 		memset(&tcp_mask, 0, sizeof(tcp_mask));
 		tcp_mask.hdr.src_port = ntuple_filter->src_port_mask;
 		tcp_mask.hdr.dst_port = ntuple_filter->dst_port_mask;
+		if (tcp_spec.hdr.tcp_flags != 0)
+			tcp_mask.hdr.tcp_flags = 0xff;
 
 		tcp_item.type = RTE_FLOW_ITEM_TYPE_TCP;
 		tcp_item.spec = &tcp_spec;
@@ -640,6 +708,11 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		return ret;
 	}
 
+	/* XXX but this only adds table_type of
+	 * RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
+	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule()
+	 * so the tcp_flags is ignored!
+	 */
 	rule = rte_flow_classify_table_entry_add(
 			cls_app->cls, &attr, pattern_ipv4_5tuple,
 			actions, &key_found, &error);
diff --git a/examples/flow_classify/ipv4_rules_file.txt b/examples/flow_classify/ipv4_rules_file.txt
index dfa0631fc..415573732 100644
--- a/examples/flow_classify/ipv4_rules_file.txt
+++ b/examples/flow_classify/ipv4_rules_file.txt
@@ -1,14 +1,14 @@
 #file format:
-#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask proto/mask priority
+#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask proto/mask priority tcp_flags
 #
-2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0
-9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1
-9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2
-9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3
-6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4
-6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5
-6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6
-6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7
-6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8
+2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0 *
+9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1 *
+9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2 *
+9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3 *
+6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4 *
+6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5 *
+6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6 *
+6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7 *
+6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8 *
 #error rules
-#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9
\ No newline at end of file
+#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9 *
-- 
2.20.1


^ permalink raw reply	[relevance 20%]

* [dpdk-dev] [PATCH RFC V2 2/2] Allow the flow_classify example to add an ACL table for tcp.
  2020-01-13 18:05 21% [dpdk-dev] [PATCH RFC V2 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
  2020-01-13 18:05 20% ` [dpdk-dev] [PATCH RFC V2 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
@ 2020-01-13 18:05 18% ` Sowmini Varadhan
  2021-04-28 15:22  6%   ` Bernard Iremonger
  2023-06-12 21:14  6% ` [dpdk-dev] [PATCH RFC V2 0/2] TCP flow classification using 4-tuple and flags Stephen Hemminger
  2 siblings, 1 reply; 38+ results
From: Sowmini Varadhan @ 2020-01-13 18:05 UTC (permalink / raw)
  To: sowmini05, dev


The struct rte_flow_classifier can have upto RTE_FLOW_CLASSIFY_TABLE_MAX
(32) classifier tables, but the existing flow_classify examples only adds
a single table for the L4 5-tuple.

When dealing with tcp flows, we frequently want to add add ACLs and filters
to filter based on the state of the TCP connection, e.g., by looking at
the tcp flags field.

So we enhance flow_classify to add an additional acl table for
tcp 5-typles. If the input-file-parser detects a filter for a tcp flow with
a non-wildcard argument for tcp_flags, the IP4_TCP_5TUPLE table is used
by flow_classify.

Signed-off-by: Sowmini Varadhan <sowmini05@gmail.com>
---
V2 updates: checkpatch fixes

 examples/flow_classify/flow_classify.c        | 44 +++++++---
 lib/librte_flow_classify/rte_flow_classify.c  | 87 +++++++++++++++++++
 lib/librte_flow_classify/rte_flow_classify.h  | 19 ++++
 .../rte_flow_classify_parse.c                 |  8 +-
 4 files changed, 144 insertions(+), 14 deletions(-)

diff --git a/examples/flow_classify/flow_classify.c b/examples/flow_classify/flow_classify.c
index e8a41741f..a0d138eb5 100644
--- a/examples/flow_classify/flow_classify.c
+++ b/examples/flow_classify/flow_classify.c
@@ -708,11 +708,6 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		return ret;
 	}
 
-	/* XXX but this only adds table_type of
-	 * RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
-	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule()
-	 * so the tcp_flags is ignored!
-	 */
 	rule = rte_flow_classify_table_entry_add(
 			cls_app->cls, &attr, pattern_ipv4_5tuple,
 			actions, &key_found, &error);
@@ -839,7 +834,8 @@ main(int argc, char *argv[])
 	int ret;
 	int socket_id;
 	struct rte_table_acl_params table_acl_params;
-	struct rte_flow_classify_table_params cls_table_params;
+	struct rte_table_acl_params table_acl_tcp_params;
+	struct rte_flow_classify_table_params cls_table_params[2];
 	struct flow_classifier *cls_app;
 	struct rte_flow_classifier_params cls_params;
 	uint32_t size;
@@ -902,20 +898,42 @@ main(int argc, char *argv[])
 	memcpy(table_acl_params.field_format, ipv4_defs, sizeof(ipv4_defs));
 
 	/* initialise table create params */
-	cls_table_params.ops = &rte_table_acl_ops;
-	cls_table_params.arg_create = &table_acl_params;
-	cls_table_params.type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+	cls_table_params[0].ops = &rte_table_acl_ops;
+	cls_table_params[0].arg_create = &table_acl_params;
+	cls_table_params[0].type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+
+	/* initialise ACL table params */
+	table_acl_tcp_params.name = "table_acl_ipv4_tcp_5tuple";
+	table_acl_tcp_params.n_rules = FLOW_CLASSIFY_MAX_RULE_NUM;
+	table_acl_tcp_params.n_rule_fields = RTE_DIM(ipv4_defs);
+	memcpy(table_acl_tcp_params.field_format, ipv4_defs, sizeof(ipv4_defs));
+
+	/* initialise table create params */
+	cls_table_params[1].ops = &rte_table_acl_ops;
+	cls_table_params[1].arg_create = &table_acl_tcp_params;
+	cls_table_params[1].type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
 
-	ret = rte_flow_classify_table_create(cls_app->cls, &cls_table_params);
+	ret = rte_flow_classify_table_create(cls_app->cls,
+					     &cls_table_params[0]);
 	if (ret) {
 		rte_flow_classifier_free(cls_app->cls);
 		rte_free(cls_app);
-		rte_exit(EXIT_FAILURE, "Failed to create classifier table\n");
+		rte_exit(EXIT_FAILURE,
+			 "Failed to create classifier table\n");
 	}
+	ret = rte_flow_classify_table_create(cls_app->cls,
+					     &cls_table_params[1]);
+	if (ret) {
+		rte_flow_classifier_free(cls_app->cls);
+		rte_free(cls_app);
+		rte_exit(EXIT_FAILURE,
+			 "Failed to create classifier table\n");
+	}
+
 
 	/* read file of IPv4 5 tuple rules and initialize parameters
-	 * for rte_flow_classify_validate and rte_flow_classify_table_entry_add
-	 * API's.
+	 * for rte_flow_classify_validate and
+	 * rte_flow_classify_table_entry_add  API's.
 	 */
 	if (add_rules(parm_config.rule_ipv4_name, cls_app)) {
 		rte_flow_classifier_free(cls_app->cls);
diff --git a/lib/librte_flow_classify/rte_flow_classify.c b/lib/librte_flow_classify/rte_flow_classify.c
index 5ff585803..cda1a6129 100644
--- a/lib/librte_flow_classify/rte_flow_classify.c
+++ b/lib/librte_flow_classify/rte_flow_classify.c
@@ -62,6 +62,7 @@ enum {
 	DST_FIELD_IPV4,
 	SRCP_FIELD_IPV4,
 	DSTP_FIELD_IPV4,
+	TCP_FLAGS_FIELD,
 	NUM_FIELDS_IPV4
 };
 
@@ -74,6 +75,7 @@ struct classify_rules {
 	enum rte_flow_classify_rule_type type;
 	union {
 		struct rte_flow_classify_ipv4_5tuple ipv4_5tuple;
+		struct rte_flow_classify_ipv4_tcp_5tuple ipv4_tcp_5tuple;
 	} u;
 };
 
@@ -482,6 +484,84 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls)
 	return rule;
 }
 
+static struct rte_flow_classify_rule *
+allocate_acl_ipv4_tcp_5tuple_rule(struct rte_flow_classifier *cls)
+{
+	struct rte_flow_classify_rule *rule;
+	int log_level;
+
+	rule = malloc(sizeof(struct rte_flow_classify_rule));
+	if (!rule)
+		return rule;
+
+	memset(rule, 0, sizeof(struct rte_flow_classify_rule));
+	rule->id = unique_id++;
+	rule->rules.type = RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE;
+
+	/* key add values */
+	rule->u.key.key_add.priority = cls->ntuple_filter.priority;
+	rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].mask_range.u8 =
+			cls->ntuple_filter.proto_mask;
+	rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].value.u8 =
+			cls->ntuple_filter.proto;
+	rule->rules.u.ipv4_tcp_5tuple.proto = cls->ntuple_filter.proto;
+	rule->rules.u.ipv4_tcp_5tuple.proto_mask =
+			cls->ntuple_filter.proto_mask;
+
+	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].mask_range.u32 =
+			cls->ntuple_filter.src_ip_mask;
+	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].value.u32 =
+			cls->ntuple_filter.src_ip;
+	rule->rules.u.ipv4_tcp_5tuple.src_ip_mask =
+			cls->ntuple_filter.src_ip_mask;
+	rule->rules.u.ipv4_tcp_5tuple.src_ip = cls->ntuple_filter.src_ip;
+
+	rule->u.key.key_add.field_value[DST_FIELD_IPV4].mask_range.u32 =
+			cls->ntuple_filter.dst_ip_mask;
+	rule->u.key.key_add.field_value[DST_FIELD_IPV4].value.u32 =
+			cls->ntuple_filter.dst_ip;
+	rule->rules.u.ipv4_tcp_5tuple.dst_ip_mask =
+			cls->ntuple_filter.dst_ip_mask;
+	rule->rules.u.ipv4_tcp_5tuple.dst_ip = cls->ntuple_filter.dst_ip;
+
+	rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].mask_range.u16 =
+			cls->ntuple_filter.src_port_mask;
+	rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].value.u16 =
+			cls->ntuple_filter.src_port;
+	rule->rules.u.ipv4_tcp_5tuple.src_port_mask =
+			cls->ntuple_filter.src_port_mask;
+	rule->rules.u.ipv4_tcp_5tuple.src_port = cls->ntuple_filter.src_port;
+
+	rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].mask_range.u16 =
+			cls->ntuple_filter.dst_port_mask;
+	rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].value.u16 =
+			cls->ntuple_filter.dst_port;
+	rule->rules.u.ipv4_tcp_5tuple.dst_port_mask =
+			cls->ntuple_filter.dst_port_mask;
+	rule->rules.u.ipv4_tcp_5tuple.dst_port = cls->ntuple_filter.dst_port;
+
+	rule->u.key.key_add.field_value[TCP_FLAGS_FIELD].mask_range.u32 =
+			rte_be_to_cpu_32(0xff);
+	rule->u.key.key_add.field_value[TCP_FLAGS_FIELD].value.u32 =
+			rte_be_to_cpu_32(cls->ntuple_filter.tcp_flags);
+	rule->rules.u.ipv4_tcp_5tuple.tcp_flags = cls->ntuple_filter.tcp_flags;
+
+	log_level = rte_log_get_level(librte_flow_classify_logtype);
+
+	if (log_level == RTE_LOG_DEBUG)
+		print_acl_ipv4_key_add(&rule->u.key.key_add);
+
+	/* key delete values */
+	memcpy(&rule->u.key.key_del.field_value[PROTO_FIELD_IPV4],
+	       &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4],
+	       NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field));
+
+	if (log_level == RTE_LOG_DEBUG)
+		print_acl_ipv4_key_delete(&rule->u.key.key_del);
+
+	return rule;
+}
+
 struct rte_flow_classify_rule *
 rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls,
 		const struct rte_flow_attr *attr,
@@ -519,6 +599,13 @@ rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls,
 		rule->tbl_type = table_type;
 		cls->table_mask |= table_type;
 		break;
+	case RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE:
+		rule = allocate_acl_ipv4_tcp_5tuple_rule(cls);
+		if (!rule)
+			return NULL;
+		rule->tbl_type = table_type;
+		cls->table_mask |= table_type;
+		break;
 	default:
 		return NULL;
 	}
diff --git a/lib/librte_flow_classify/rte_flow_classify.h b/lib/librte_flow_classify/rte_flow_classify.h
index 74d1ecaf5..921277f90 100644
--- a/lib/librte_flow_classify/rte_flow_classify.h
+++ b/lib/librte_flow_classify/rte_flow_classify.h
@@ -78,6 +78,8 @@ enum rte_flow_classify_rule_type {
 	RTE_FLOW_CLASSIFY_RULE_TYPE_NONE,
 	/** IPv4 5tuple type */
 	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_5TUPLE,
+	/** IPv4 TCP 5tuple type */
+	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE,
 };
 
 /** Flow classify table type */
@@ -90,6 +92,8 @@ enum rte_flow_classify_table_type {
 	RTE_FLOW_CLASSIFY_TABLE_ACL_VLAN_IP4_5TUPLE = 1 << 2,
 	/** ACL QinQ IP4 5TUPLE */
 	RTE_FLOW_CLASSIFY_TABLE_ACL_QINQ_IP4_5TUPLE = 1 << 3,
+	/** ACL IP4 5TUPLE with tcp_flags */
+	RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE = 1 << 4,
 
 };
 
@@ -129,6 +133,21 @@ struct rte_flow_classify_ipv4_5tuple {
 	uint8_t proto_mask;      /**< Mask of L4 protocol. */
 };
 
+/** IPv4 5-tuple data with tcp flags*/
+struct rte_flow_classify_ipv4_tcp_5tuple {
+	uint32_t dst_ip;         /**< Destination IP address in big endian. */
+	uint32_t dst_ip_mask;    /**< Mask of destination IP address. */
+	uint32_t src_ip;         /**< Source IP address in big endian. */
+	uint32_t src_ip_mask;    /**< Mask of destination IP address. */
+	uint16_t dst_port;       /**< Destination port in big endian. */
+	uint16_t dst_port_mask;  /**< Mask of destination port. */
+	uint16_t src_port;       /**< Source Port in big endian. */
+	uint16_t src_port_mask;  /**< Mask of source port. */
+	uint8_t proto;           /**< L4 protocol. */
+	uint8_t proto_mask;      /**< Mask of L4 protocol. */
+	uint8_t tcp_flags;       /**< Tcp only */
+};
+
 /**
  * Flow stats
  *
diff --git a/lib/librte_flow_classify/rte_flow_classify_parse.c b/lib/librte_flow_classify/rte_flow_classify_parse.c
index 465330291..fe4ee05b6 100644
--- a/lib/librte_flow_classify/rte_flow_classify_parse.c
+++ b/lib/librte_flow_classify/rte_flow_classify_parse.c
@@ -216,6 +216,7 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 	const struct rte_flow_action_count *count;
 	const struct rte_flow_action_mark *mark_spec;
 	uint32_t index;
+	bool have_tcp = false;
 
 	/* parse pattern */
 	index = 0;
@@ -375,6 +376,8 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		filter->dst_port  = tcp_spec->hdr.dst_port;
 		filter->src_port  = tcp_spec->hdr.src_port;
 		filter->tcp_flags = tcp_spec->hdr.tcp_flags;
+		if (filter->tcp_flags != 0)
+			have_tcp = true;
 	} else if (item->type == RTE_FLOW_ITEM_TYPE_UDP) {
 		udp_mask = item->mask;
 
@@ -434,7 +437,10 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		return -EINVAL;
 	}
 
-	table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+	if (have_tcp)
+		table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
+	else
+		table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
 
 	/* parse attr */
 	/* must be input direction */
-- 
2.20.1


^ permalink raw reply	[relevance 18%]

* [dpdk-dev] patches not showing up on list?
@ 2020-01-13 18:48 13% Sowmini Varadhan
  2020-01-13 23:18  8% ` Thomas Monjalon
  0 siblings, 1 reply; 38+ results
From: Sowmini Varadhan @ 2020-01-13 18:48 UTC (permalink / raw)
  To: dev; +Cc: thomas, david.marchand, sovaradh


I just sent a couple of (RFC) patches to dev@dpdk.org and they 
are showing up in the patch queue, but not on the list, is there
something wrong with my mail headers? How can I debug this?

Patches I am referring to are:

https://patchwork.dpdk.org/patch/64574/
https://patchwork.dpdk.org/patch/64575/

--Sowmini



^ permalink raw reply	[relevance 13%]

* Re: [dpdk-dev] patches not showing up on list?
  2020-01-13 18:48 13% [dpdk-dev] patches not showing up on list? Sowmini Varadhan
@ 2020-01-13 23:18  8% ` Thomas Monjalon
  0 siblings, 0 replies; 38+ results
From: Thomas Monjalon @ 2020-01-13 23:18 UTC (permalink / raw)
  To: Sowmini Varadhan; +Cc: dev, david.marchand, sovaradh

13/01/2020 19:48, Sowmini Varadhan:
> 
> I just sent a couple of (RFC) patches to dev@dpdk.org and they 
> are showing up in the patch queue, but not on the list, is there
> something wrong with my mail headers? How can I debug this?

They are showing up on the list:
	http://inbox.dpdk.org/dev/?q=Sowmini




^ permalink raw reply	[relevance 8%]

* [dpdk-dev] [dpdk-announce] [HELP REQUIRED] call for reviews
@ 2020-06-26 16:43  5% Thomas Monjalon
  0 siblings, 0 replies; 38+ results
From: Thomas Monjalon @ 2020-06-26 16:43 UTC (permalink / raw)
  To: announce

Hi all,

Our next milestone is approaching, 20.08-rc1 should be in 12 days :
	http://core.dpdk.org/roadmap/#dates
and the roadmap is very long:
	http://core.dpdk.org/roadmap/#2008

We all want to have our code reviewed and accepted.
In order to have a small hope of reaching our goals,
we need a large participation in reviews.

PLEASE REVIEW some patches from the list below.

If you need a breath between two reviews,
please enjoy a satifying video from https://twitter.com/engineers_feed
Now that you are ready, here is the list to cherry-pick from:

- introduce global debug flag
  http://inbox.dpdk.org/dev/20200422214555.11837-1-l.wojciechow@partner.samsung.com/

- one more step in makefiles deprecation
  http://inbox.dpdk.org/dev/20200625214338.1838457-1-thomas@monjalon.net/

- Experimental/internal libraries cleanup
  http://inbox.dpdk.org/dev/20200626081638.29890-1-david.marchand@redhat.com/

- rename blacklist/whitelist to block/allow
  http://inbox.dpdk.org/dev/20200613000055.7909-1-stephen@networkplumber.org/

- Change references to master/slave
  http://inbox.dpdk.org/dev/20200605225811.26342-1-stephen@networkplumber.org/

- plugin loading
  http://inbox.dpdk.org/dev/20200622143337.562637-1-bruce.richardson@intel.com/

- rte init framework
  http://inbox.dpdk.org/dev/1586787714-13890-1-git-send-email-xiangxia.m.yue@gmail.com/

- Register non-EAL threads as lcore
  http://inbox.dpdk.org/dev/20200626144736.11011-1-david.marchand@redhat.com/

- rte_log registration usage improvement
  http://inbox.dpdk.org/dev/20200617063047.1555518-1-jerinj@marvell.com/

- EAL resources cleanup
  http://inbox.dpdk.org/dev/20200428235827.15383-1-stephen@networkplumber.org/

- proc-info cleanup
  http://inbox.dpdk.org/dev/20200506195758.27057-1-stephen@networkplumber.org/

- PRNG compat
  http://inbox.dpdk.org/dev/20200422234255.7066-1-dg@adax.com/

- malloc debug
  http://inbox.dpdk.org/dev/1587110623-405-1-git-send-email-xuemingl@mellanox.com/#t

- ring: make ring implementation non-inlined
  http://inbox.dpdk.org/dev/20200320164138.8510-1-konstantin.ananyev@intel.com/

- ring: add scatter gather and serial dequeue APIs 
  http://inbox.dpdk.org/dev/20200224203931.21256-1-honnappa.nagarahalli@arm.com/

- mempool ring + HTS
  http://inbox.dpdk.org/dev/20200521132027.28219-1-konstantin.ananyev@intel.com/

- Use of WFE in ring and spinlock
  http://inbox.dpdk.org/dev/20200426083909.897-1-gavin.hu@arm.com/

- x86 only IO API
  http://inbox.dpdk.org/dev/1592568380-27120-1-git-send-email-radu.nicolau@intel.com/

- eal: use c11 atomics for interrupt status
  http://inbox.dpdk.org/dev/1591871065-12461-2-git-send-email-phil.yang@arm.com/

- mbuf: use c11 atomics for refcnt operations
  http://inbox.dpdk.org/dev/1591871178-12542-1-git-send-email-phil.yang@arm.com/

- mbuf: accurate packet Tx scheduling
  http://inbox.dpdk.org/dev/1591771085-24959-1-git-send-email-viacheslavo@mellanox.com/

- mbuf: add Tx offloads for packet marking
  http://inbox.dpdk.org/dev/20200417072254.11455-1-nithind1988@gmail.com/

- add new kv hash table
  http://inbox.dpdk.org/dev/cover.1588967562.git.vladimir.medvedkin@intel.com/

- TCP flow classification using 4-tuple and flags
  http://inbox.dpdk.org/dev/cover.1578936382.git.sowmini.varadhan@microsoft.com/

- fib + AVX512
  http://inbox.dpdk.org/dev/cover.1589890262.git.vladimir.medvedkin@intel.com/

- RCU integration in LPM
  http://inbox.dpdk.org/dev/20200608051658.144417-1-ruifeng.wang@arm.com/

- compile librte_net for windows
  http://inbox.dpdk.org/dev/20200610120040.17968-1-fady@mellanox.com/

- Windows bus/pci support
  http://inbox.dpdk.org/dev/20200624082847.21344-1-talshn@mellanox.com/

- cmdline: support Windows
  http://inbox.dpdk.org/dev/20200620210511.13134-1-dmitry.kozliuk@gmail.com/

- ethdev: verify reserved HW ring
  http://inbox.dpdk.org/dev/20200624093520.2142722-1-ferruh.yigit@intel.com/

- ethdev: allow unknown link speed
  http://inbox.dpdk.org/dev/20200615090158.18912-1-i.dyukov@samsung.com/

- Power-optimized RX for Ethernet devices
  http://inbox.dpdk.org/dev/cover.1590598121.git.anatoly.burakov@intel.com/

- ethdev: add rte_device to port_id function 
  http://inbox.dpdk.org/dev/a62bdf6dad57737ffc169e1d2d6717efdec9436d.1587142035.git.grive@u256.net/

- add flow action map
  http://inbox.dpdk.org/dev/1591194009-4086-1-git-send-email-bernard.iremonger@intel.com/

- add flow action context API
  http://inbox.dpdk.org/dev/20200620133231.12355-1-andrey.vesnovaty@gmail.com/

- support the flow-based traffic sampling
  http://inbox.dpdk.org/dev/1593102379-400132-1-git-send-email-jiaweiw@mellanox.com/

- ethdev: tunnel offload model 
  http://inbox.dpdk.org/dev/20200625160348.26220-1-getelson@mellanox.com/

- Introduce flow perf application
  http://inbox.dpdk.org/dev/20200604133502.28491-1-wisamm@mellanox.com/

- Introduce IF proxy library
  http://inbox.dpdk.org/dev/20200622092128.3560-1-aostruszka@marvell.com/

- add support for DOCSIS protocol
  http://inbox.dpdk.org/dev/20200623101423.9215-1-david.coyle@intel.com/

- RegEx
  http://inbox.dpdk.org/dev/1588844756-10086-1-git-send-email-orika@mellanox.com/
  http://inbox.dpdk.org/dev/1591219752-46544-1-git-send-email-orika@mellanox.com/




^ permalink raw reply	[relevance 5%]

* [dpdk-dev] [PATCH RFC V2 2/2] Allow the flow_classify example to add an ACL table for tcp.
  2020-01-13 18:05 18% ` [dpdk-dev] [PATCH RFC V2 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
@ 2021-04-28 15:22  6%   ` Bernard Iremonger
  0 siblings, 0 replies; 38+ results
From: Bernard Iremonger @ 2021-04-28 15:22 UTC (permalink / raw)
  To: sowmini05; +Cc: dev

Hi Sowmini,

Could you rebase this patchset to the latest DPDK-21.05-rc1  code.

Regards,

Bernard


^ permalink raw reply	[relevance 6%]

* [dpdk-dev] [PATCH 0/2] TCP flow classification using 4-tuple and flags
@ 2021-08-12 20:17 13% Sowmini Varadhan
  2021-08-12 20:17 11% ` [dpdk-dev] [PATCH 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
  2021-08-12 20:17 10% ` [dpdk-dev] [PATCH 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
  0 siblings, 2 replies; 38+ results
From: Sowmini Varadhan @ 2021-08-12 20:17 UTC (permalink / raw)
  To: sowmini05, bernard.iremonger, dev, sovaradh; +Cc: thomas

The problem space of TCP flow tracking and classification
based on TCP state requires the ability to classify TCP
flows on more packet properties than just the 4-tuple,
e.g., TCP flags. This patch-set provides the set of
changes needed in the examples/flow_classify.c needed to
achieve this.

Patch 1 extends examples/flow_classify.c to allow constraints
on tcp flags. Patch 2 extends the ACL handling in  
librte_flow_classify to include keys on the properties in 
addition to the tcp 4-tuple.

Sowmini Varadhan (2):
  Hooks to allow the setting of filters on tcp flags
  Allow the flow_classify example to add an ACL table for tcp.

 examples/flow_classify/flow_classify.c      | 118 +++++++++++++++++---
 examples/flow_classify/ipv4_rules_file.txt  |  22 ++--
 lib/flow_classify/rte_flow_classify.c       |  87 +++++++++++++++
 lib/flow_classify/rte_flow_classify.h       |  19 ++++
 lib/flow_classify/rte_flow_classify_parse.c |   8 +-
 5 files changed, 228 insertions(+), 26 deletions(-)

-- 
2.17.1


^ permalink raw reply	[relevance 13%]

* [dpdk-dev] [PATCH 2/2] Allow the flow_classify example to add an ACL table for tcp.
  2021-08-12 20:17 13% [dpdk-dev] [PATCH 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
  2021-08-12 20:17 11% ` [dpdk-dev] [PATCH 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
@ 2021-08-12 20:17 10% ` Sowmini Varadhan
  2021-08-17 12:08  6%   ` Iremonger, Bernard
  1 sibling, 1 reply; 38+ results
From: Sowmini Varadhan @ 2021-08-12 20:17 UTC (permalink / raw)
  To: sowmini05, bernard.iremonger, dev, sowmin05, sovaradh; +Cc: thomas

The struct rte_flow_classifier can have upto RTE_FLOW_CLASSIFY_TABLE_MAX
(32) classifier tables, but the existing flow_classify examples only adds
a single table for the L4 5-tuple.

When dealing with tcp flows, we frequently want to add add ACLs and filters
to filter based on the state of the TCP connection, e.g., by looking at
the tcp flags field.

So we enhance flow_classify to add an additional acl table for
tcp 5-typles. If the input-file-parser detects a filter for a tcp flow with
a non-wildcard argument for tcp_flags, the IP4_TCP_5TUPLE table is used
by flow_classify.

Signed-off-by: Sowmini Varadhan <sovaradh@linux.microsoft.com>
---
 examples/flow_classify/flow_classify.c      | 41 +++++++---
 lib/flow_classify/rte_flow_classify.c       | 87 +++++++++++++++++++++
 lib/flow_classify/rte_flow_classify.h       | 19 +++++
 lib/flow_classify/rte_flow_classify_parse.c |  8 +-
 4 files changed, 142 insertions(+), 13 deletions(-)

diff --git a/examples/flow_classify/flow_classify.c b/examples/flow_classify/flow_classify.c
index 772b15adf2..b42d0df5c3 100644
--- a/examples/flow_classify/flow_classify.c
+++ b/examples/flow_classify/flow_classify.c
@@ -723,11 +723,6 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		return ret;
 	}
 
-	/* XXX but this only adds table_type of
-	 * RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
-	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule()
-	 * so the tcp_flags is ignored!
-	 */
 	rule = rte_flow_classify_table_entry_add(
 			cls_app->cls, &attr, pattern_ipv4_5tuple,
 			actions, &key_found, &error);
@@ -856,7 +851,8 @@ main(int argc, char *argv[])
 	int ret;
 	int socket_id;
 	struct rte_table_acl_params table_acl_params;
-	struct rte_flow_classify_table_params cls_table_params;
+	struct rte_table_acl_params table_acl_tcp_params;
+	struct rte_flow_classify_table_params cls_table_params[2];
 	struct flow_classifier *cls_app;
 	struct rte_flow_classifier_params cls_params;
 	uint32_t size;
@@ -923,21 +919,42 @@ main(int argc, char *argv[])
 	memcpy(table_acl_params.field_format, ipv4_defs, sizeof(ipv4_defs));
 
 	/* initialise table create params */
-	cls_table_params.ops = &rte_table_acl_ops;
-	cls_table_params.arg_create = &table_acl_params;
-	cls_table_params.type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+	cls_table_params[0].ops = &rte_table_acl_ops;
+	cls_table_params[0].arg_create = &table_acl_params;
+	cls_table_params[0].type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+
+	/* initialise ACL table params */
+	table_acl_tcp_params.name = "table_acl_ipv4_tcp_5tuple";
+	table_acl_tcp_params.n_rules = FLOW_CLASSIFY_MAX_RULE_NUM;
+	table_acl_tcp_params.n_rule_fields = RTE_DIM(ipv4_defs);
+	memcpy(table_acl_tcp_params.field_format, ipv4_defs, sizeof(ipv4_defs));
+
+	/* initialise table create params */
+	cls_table_params[1].ops = &rte_table_acl_ops;
+	cls_table_params[1].arg_create = &table_acl_tcp_params;
+	cls_table_params[1].type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
 
-	ret = rte_flow_classify_table_create(cls_app->cls, &cls_table_params);
+	ret = rte_flow_classify_table_create(cls_app->cls,
+					     &cls_table_params[0]);
 	if (ret) {
 		rte_flow_classifier_free(cls_app->cls);
 		rte_free(cls_app);
 		rte_exit(EXIT_FAILURE, "Failed to create classifier table\n");
 	}
+	ret = rte_flow_classify_table_create(cls_app->cls,
+					     &cls_table_params[1]);
+	if (ret) {
+		rte_flow_classifier_free(cls_app->cls);
+		rte_free(cls_app);
+		rte_exit(EXIT_FAILURE,
+			 "Failed to create classifier table\n");
+	}
+
 	/* >8 End of initialization of table create params. */
 
 	/* read file of IPv4 5 tuple rules and initialize parameters
-	 * for rte_flow_classify_validate and rte_flow_classify_table_entry_add
-	 * API's.
+	 * for rte_flow_classify_validate and
+	 * rte_flow_classify_table_entry_add  API's.
 	 */
 
 	/* Read file of IPv4 tuple rules. 8< */
diff --git a/lib/flow_classify/rte_flow_classify.c b/lib/flow_classify/rte_flow_classify.c
index d3ba2ed227..e960c3b140 100644
--- a/lib/flow_classify/rte_flow_classify.c
+++ b/lib/flow_classify/rte_flow_classify.c
@@ -60,6 +60,7 @@ enum {
 	DST_FIELD_IPV4,
 	SRCP_FIELD_IPV4,
 	DSTP_FIELD_IPV4,
+	TCP_FLAGS_FIELD,
 	NUM_FIELDS_IPV4
 };
 
@@ -72,6 +73,7 @@ struct classify_rules {
 	enum rte_flow_classify_rule_type type;
 	union {
 		struct rte_flow_classify_ipv4_5tuple ipv4_5tuple;
+		struct rte_flow_classify_ipv4_tcp_5tuple ipv4_tcp_5tuple;
 	} u;
 };
 
@@ -477,6 +479,84 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls)
 	return rule;
 }
 
+static struct rte_flow_classify_rule *
+allocate_acl_ipv4_tcp_5tuple_rule(struct rte_flow_classifier *cls)
+{
+	struct rte_flow_classify_rule *rule;
+	int log_level;
+
+	rule = malloc(sizeof(struct rte_flow_classify_rule));
+	if (!rule)
+		return rule;
+
+	memset(rule, 0, sizeof(struct rte_flow_classify_rule));
+	rule->id = unique_id++;
+	rule->rules.type = RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE;
+
+	/* key add values */
+	rule->u.key.key_add.priority = cls->ntuple_filter.priority;
+	rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].mask_range.u8 =
+			cls->ntuple_filter.proto_mask;
+	rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].value.u8 =
+			cls->ntuple_filter.proto;
+	rule->rules.u.ipv4_tcp_5tuple.proto = cls->ntuple_filter.proto;
+	rule->rules.u.ipv4_tcp_5tuple.proto_mask =
+			cls->ntuple_filter.proto_mask;
+
+	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].mask_range.u32 =
+			cls->ntuple_filter.src_ip_mask;
+	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].value.u32 =
+			cls->ntuple_filter.src_ip;
+	rule->rules.u.ipv4_tcp_5tuple.src_ip_mask =
+			cls->ntuple_filter.src_ip_mask;
+	rule->rules.u.ipv4_tcp_5tuple.src_ip = cls->ntuple_filter.src_ip;
+
+	rule->u.key.key_add.field_value[DST_FIELD_IPV4].mask_range.u32 =
+			cls->ntuple_filter.dst_ip_mask;
+	rule->u.key.key_add.field_value[DST_FIELD_IPV4].value.u32 =
+			cls->ntuple_filter.dst_ip;
+	rule->rules.u.ipv4_tcp_5tuple.dst_ip_mask =
+			cls->ntuple_filter.dst_ip_mask;
+	rule->rules.u.ipv4_tcp_5tuple.dst_ip = cls->ntuple_filter.dst_ip;
+
+	rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].mask_range.u16 =
+			cls->ntuple_filter.src_port_mask;
+	rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].value.u16 =
+			cls->ntuple_filter.src_port;
+	rule->rules.u.ipv4_tcp_5tuple.src_port_mask =
+			cls->ntuple_filter.src_port_mask;
+	rule->rules.u.ipv4_tcp_5tuple.src_port = cls->ntuple_filter.src_port;
+
+	rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].mask_range.u16 =
+			cls->ntuple_filter.dst_port_mask;
+	rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].value.u16 =
+			cls->ntuple_filter.dst_port;
+	rule->rules.u.ipv4_tcp_5tuple.dst_port_mask =
+			cls->ntuple_filter.dst_port_mask;
+	rule->rules.u.ipv4_tcp_5tuple.dst_port = cls->ntuple_filter.dst_port;
+
+	rule->u.key.key_add.field_value[TCP_FLAGS_FIELD].mask_range.u32 =
+			rte_be_to_cpu_32(0xff);
+	rule->u.key.key_add.field_value[TCP_FLAGS_FIELD].value.u32 =
+			rte_be_to_cpu_32(cls->ntuple_filter.tcp_flags);
+	rule->rules.u.ipv4_tcp_5tuple.tcp_flags = cls->ntuple_filter.tcp_flags;
+
+	log_level = rte_log_get_level(librte_flow_classify_logtype);
+
+	if (log_level == RTE_LOG_DEBUG)
+		print_acl_ipv4_key_add(&rule->u.key.key_add);
+
+	/* key delete values */
+	memcpy(&rule->u.key.key_del.field_value[PROTO_FIELD_IPV4],
+	       &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4],
+	       NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field));
+
+	if (log_level == RTE_LOG_DEBUG)
+		print_acl_ipv4_key_delete(&rule->u.key.key_del);
+
+	return rule;
+}
+
 struct rte_flow_classify_rule *
 rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls,
 		const struct rte_flow_attr *attr,
@@ -514,6 +594,13 @@ rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls,
 		rule->tbl_type = table_type;
 		cls->table_mask |= table_type;
 		break;
+	case RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE:
+		rule = allocate_acl_ipv4_tcp_5tuple_rule(cls);
+		if (!rule)
+			return NULL;
+		rule->tbl_type = table_type;
+		cls->table_mask |= table_type;
+		break;
 	default:
 		return NULL;
 	}
diff --git a/lib/flow_classify/rte_flow_classify.h b/lib/flow_classify/rte_flow_classify.h
index 82ea92b6a6..65585d9f92 100644
--- a/lib/flow_classify/rte_flow_classify.h
+++ b/lib/flow_classify/rte_flow_classify.h
@@ -80,6 +80,8 @@ enum rte_flow_classify_rule_type {
 	RTE_FLOW_CLASSIFY_RULE_TYPE_NONE,
 	/** IPv4 5tuple type */
 	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_5TUPLE,
+	/** IPv4 TCP 5tuple type */
+	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE,
 };
 
 /** Flow classify table type */
@@ -92,6 +94,8 @@ enum rte_flow_classify_table_type {
 	RTE_FLOW_CLASSIFY_TABLE_ACL_VLAN_IP4_5TUPLE = 1 << 2,
 	/** ACL QinQ IP4 5TUPLE */
 	RTE_FLOW_CLASSIFY_TABLE_ACL_QINQ_IP4_5TUPLE = 1 << 3,
+	/** ACL IP4 5TUPLE with tcp_flags */
+	RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE = 1 << 4,
 
 };
 
@@ -131,6 +135,21 @@ struct rte_flow_classify_ipv4_5tuple {
 	uint8_t proto_mask;      /**< Mask of L4 protocol. */
 };
 
+/** IPv4 5-tuple data with tcp flags*/
+struct rte_flow_classify_ipv4_tcp_5tuple {
+	uint32_t dst_ip;         /**< Destination IP address in big endian. */
+	uint32_t dst_ip_mask;    /**< Mask of destination IP address. */
+	uint32_t src_ip;         /**< Source IP address in big endian. */
+	uint32_t src_ip_mask;    /**< Mask of destination IP address. */
+	uint16_t dst_port;       /**< Destination port in big endian. */
+	uint16_t dst_port_mask;  /**< Mask of destination port. */
+	uint16_t src_port;       /**< Source Port in big endian. */
+	uint16_t src_port_mask;  /**< Mask of source port. */
+	uint8_t proto;           /**< L4 protocol. */
+	uint8_t proto_mask;      /**< Mask of L4 protocol. */
+	uint8_t tcp_flags;       /**< Tcp only */
+};
+
 /**
  * Flow stats
  *
diff --git a/lib/flow_classify/rte_flow_classify_parse.c b/lib/flow_classify/rte_flow_classify_parse.c
index 465330291f..fe4ee05b6f 100644
--- a/lib/flow_classify/rte_flow_classify_parse.c
+++ b/lib/flow_classify/rte_flow_classify_parse.c
@@ -216,6 +216,7 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 	const struct rte_flow_action_count *count;
 	const struct rte_flow_action_mark *mark_spec;
 	uint32_t index;
+	bool have_tcp = false;
 
 	/* parse pattern */
 	index = 0;
@@ -375,6 +376,8 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		filter->dst_port  = tcp_spec->hdr.dst_port;
 		filter->src_port  = tcp_spec->hdr.src_port;
 		filter->tcp_flags = tcp_spec->hdr.tcp_flags;
+		if (filter->tcp_flags != 0)
+			have_tcp = true;
 	} else if (item->type == RTE_FLOW_ITEM_TYPE_UDP) {
 		udp_mask = item->mask;
 
@@ -434,7 +437,10 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		return -EINVAL;
 	}
 
-	table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+	if (have_tcp)
+		table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
+	else
+		table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
 
 	/* parse attr */
 	/* must be input direction */
-- 
2.17.1


^ permalink raw reply	[relevance 10%]

* [dpdk-dev] [PATCH 1/2] Hooks to allow the setting of filters on tcp flags
  2021-08-12 20:17 13% [dpdk-dev] [PATCH 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
@ 2021-08-12 20:17 11% ` Sowmini Varadhan
  2021-08-17 11:40  6%   ` Iremonger, Bernard
  2021-08-12 20:17 10% ` [dpdk-dev] [PATCH 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
  1 sibling, 1 reply; 38+ results
From: Sowmini Varadhan @ 2021-08-12 20:17 UTC (permalink / raw)
  To: sowmini05, bernard.iremonger, dev, sovaradh; +Cc: thomas

The rte_eth_ntuple_filter allows tcp_flags which can check for things
like
    #define RTE_TCP_CWR_FLAG 0x80 /**< Congestion Window Reduced */
    #define RTE_TCP_ECE_FLAG 0x40 /**< ECN-Echo */
    #define RTE_TCP_URG_FLAG 0x20 /**< Urgent Pointer field significant */
    #define RTE_TCP_ACK_FLAG 0x10 /**< Acknowledgment field significant */
    #define RTE_TCP_PSH_FLAG 0x08 /**< Push Function */
    #define RTE_TCP_RST_FLAG 0x04 /**< Reset the connection */
    #define RTE_TCP_SYN_FLAG 0x02 /**< Synchronize sequence numbers */
    #define RTE_TCP_FIN_FLAG 0x01 /**< No more data from sender */
but there are no existing examples that demonstrate how to use this
feature.

This patch extends the exisiting classification support to allow
an optional flags in the input file. The flags string can be any
concatenation of characters from {C, E, U, A, P, R, S, F} and
"*" indicates "dont care". These flags are set in the ntuple_filter and
are used to construct the tcp_spec and tcp_mask sent to the driver

The rte_acl_field_def is updated to use the (u8) tcp flag as lookup key.
Note that, as per
  https://doc.dpdk.org/guides/prog_guide/packet_classif_access_ctrl.html
this field MUST be allocated fo4 4 bytes, thus it has sizeof(uint32_t).

However, also note the XXX in this commit: additional updates are
needed to the rte_flow_classify_table_entry_add() so that it does
not ignore any key fields other than the 5-tuple.

Signed-off-by: Sowmini Varadhan <sovaradh@linux.microsoft.com>
---
 examples/flow_classify/flow_classify.c     | 87 ++++++++++++++++++++--
 examples/flow_classify/ipv4_rules_file.txt | 22 +++---
 2 files changed, 91 insertions(+), 18 deletions(-)

diff --git a/examples/flow_classify/flow_classify.c b/examples/flow_classify/flow_classify.c
index db71f5aa04..772b15adf2 100644
--- a/examples/flow_classify/flow_classify.c
+++ b/examples/flow_classify/flow_classify.c
@@ -51,6 +51,7 @@ enum {
 	CB_FLD_DST_PORT_MASK,
 	CB_FLD_PROTO,
 	CB_FLD_PRIORITY,
+	CB_FLD_TCP_FLAGS,
 	CB_FLD_NUM,
 };
 
@@ -86,6 +87,7 @@ enum {
 	DST_FIELD_IPV4,
 	SRCP_FIELD_IPV4,
 	DSTP_FIELD_IPV4,
+	TCP_FLAGS_FIELD,
 	NUM_FIELDS_IPV4
 };
 
@@ -93,7 +95,8 @@ enum {
 	PROTO_INPUT_IPV4,
 	SRC_INPUT_IPV4,
 	DST_INPUT_IPV4,
-	SRCP_DESTP_INPUT_IPV4
+	SRCP_DESTP_INPUT_IPV4,
+	TCP_FLAGS_INDEX,
 };
 
 static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
@@ -150,6 +153,17 @@ static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
 			sizeof(struct rte_ipv4_hdr) +
 			offsetof(struct rte_tcp_hdr, dst_port),
 	},
+	/* next field must be 4 bytes, even though flags is only 1 byte */
+	{
+		/* rte_flags */
+		.type = RTE_ACL_FIELD_TYPE_BITMASK,
+		.size = sizeof(uint32_t),
+		.field_index = TCP_FLAGS_FIELD,
+		.input_index = TCP_FLAGS_INDEX,
+		.offset = sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr) +
+			offsetof(struct rte_tcp_hdr, tcp_flags),
+	},
 };
 /* >8 End of creation of ACL table. */
 
@@ -285,12 +299,14 @@ lcore_main(struct flow_classifier *cls_app)
 	int ret;
 	int i = 0;
 
-	ret = rte_flow_classify_table_entry_delete(cls_app->cls,
-			rules[7]);
-	if (ret)
-		printf("table_entry_delete failed [7] %d\n\n", ret);
-	else
-		printf("table_entry_delete succeeded [7]\n\n");
+	if (rules[7]) {
+		ret = rte_flow_classify_table_entry_delete(cls_app->cls,
+				rules[7]);
+		if (ret)
+			printf("table_entry_delete failed [7] %d\n\n", ret);
+		else
+			printf("table_entry_delete succeeded [7]\n\n");
+	}
 
 	/*
 	 * Check that the port is on the same NUMA node as the polling thread
@@ -410,6 +426,53 @@ parse_ipv4_net(char *in, uint32_t *addr, uint32_t *mask_len)
 	return 0;
 }
 
+static int
+get_tcp_flags(char *in, struct rte_eth_ntuple_filter *ntuple_filter)
+{
+	int len = strlen(in);
+	int i;
+	uint8_t flags = 0;
+
+	if (strcmp(in, "*") == 0) {
+		ntuple_filter->tcp_flags = 0;
+		return 0;
+	}
+
+	for (i = 0; i < len; i++) {
+		switch (in[i]) {
+		case 'S':
+			flags |= RTE_TCP_SYN_FLAG;
+			break;
+		case 'F':
+			flags |= RTE_TCP_FIN_FLAG;
+			break;
+		case 'R':
+			flags |= RTE_TCP_RST_FLAG;
+			break;
+		case 'P':
+			flags |= RTE_TCP_PSH_FLAG;
+			break;
+		case 'A':
+			flags |= RTE_TCP_ACK_FLAG;
+			break;
+		case 'U':
+			flags |= RTE_TCP_URG_FLAG;
+			break;
+		case 'E':
+			flags |= RTE_TCP_ECE_FLAG;
+			break;
+		case 'C':
+			flags |= RTE_TCP_CWR_FLAG;
+			break;
+		default:
+			fprintf(stderr, "unknown flag %c\n", in[i]);
+			return -1;
+		}
+	}
+	ntuple_filter->tcp_flags = flags;
+	return 0;
+}
+
 static int
 parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter *ntuple_filter)
 {
@@ -481,6 +544,8 @@ parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter *ntuple_filter)
 	ntuple_filter->priority = (uint16_t)temp;
 	if (ntuple_filter->priority > FLOW_CLASSIFY_MAX_PRIORITY)
 		ret = -EINVAL;
+	if (get_tcp_flags(in[CB_FLD_TCP_FLAGS], ntuple_filter))
+		return -EINVAL;
 
 	return ret;
 }
@@ -597,10 +662,13 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		memset(&tcp_spec, 0, sizeof(tcp_spec));
 		tcp_spec.hdr.src_port = ntuple_filter->src_port;
 		tcp_spec.hdr.dst_port = ntuple_filter->dst_port;
+		tcp_spec.hdr.tcp_flags = ntuple_filter->tcp_flags;
 
 		memset(&tcp_mask, 0, sizeof(tcp_mask));
 		tcp_mask.hdr.src_port = ntuple_filter->src_port_mask;
 		tcp_mask.hdr.dst_port = ntuple_filter->dst_port_mask;
+		if (tcp_spec.hdr.tcp_flags != 0)
+			tcp_mask.hdr.tcp_flags = 0xff;
 
 		tcp_item.type = RTE_FLOW_ITEM_TYPE_TCP;
 		tcp_item.spec = &tcp_spec;
@@ -655,6 +723,11 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		return ret;
 	}
 
+	/* XXX but this only adds table_type of
+	 * RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
+	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule()
+	 * so the tcp_flags is ignored!
+	 */
 	rule = rte_flow_classify_table_entry_add(
 			cls_app->cls, &attr, pattern_ipv4_5tuple,
 			actions, &key_found, &error);
diff --git a/examples/flow_classify/ipv4_rules_file.txt b/examples/flow_classify/ipv4_rules_file.txt
index dfa0631fcc..415573732a 100644
--- a/examples/flow_classify/ipv4_rules_file.txt
+++ b/examples/flow_classify/ipv4_rules_file.txt
@@ -1,14 +1,14 @@
 #file format:
-#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask proto/mask priority
+#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask proto/mask priority tcp_flags
 #
-2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0
-9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1
-9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2
-9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3
-6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4
-6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5
-6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6
-6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7
-6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8
+2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0 *
+9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1 *
+9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2 *
+9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3 *
+6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4 *
+6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5 *
+6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6 *
+6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7 *
+6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8 *
 #error rules
-#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9
\ No newline at end of file
+#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9 *
-- 
2.17.1


^ permalink raw reply	[relevance 11%]

* Re: [dpdk-dev] [PATCH 1/2] Hooks to allow the setting of filters on tcp flags
  2021-08-12 20:17 11% ` [dpdk-dev] [PATCH 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
@ 2021-08-17 11:40  6%   ` Iremonger, Bernard
  0 siblings, 0 replies; 38+ results
From: Iremonger, Bernard @ 2021-08-17 11:40 UTC (permalink / raw)
  To: Sowmini Varadhan, sowmini05, dev; +Cc: thomas

Hi Sowmini,

> -----Original Message-----
> From: Sowmini Varadhan <sovaradh@linux.microsoft.com>
> Sent: Thursday, August 12, 2021 9:18 PM
> To: sowmini05@gmail.com; Iremonger, Bernard
> <bernard.iremonger@intel.com>; dev@dpdk.org;
> sovaradh@linux.microsoft.com
> Cc: thomas@monjalon.net
> Subject: [PATCH 1/2] Hooks to allow the setting of filters on tcp flags

~/dpdk_21_08/devtools# ./check-git-log.sh -1
Wrong headline format:
        Hooks to allow the setting of filters on tcp flags

The subject line should be prefixed with examples/flow_classify:
examples/flow_classify: Hooks to allow the setting of filters on tcp flags
> 
> The rte_eth_ntuple_filter allows tcp_flags which can check for things like
>     #define RTE_TCP_CWR_FLAG 0x80 /**< Congestion Window Reduced */
>     #define RTE_TCP_ECE_FLAG 0x40 /**< ECN-Echo */
>     #define RTE_TCP_URG_FLAG 0x20 /**< Urgent Pointer field significant */
>     #define RTE_TCP_ACK_FLAG 0x10 /**< Acknowledgment field significant
> */
>     #define RTE_TCP_PSH_FLAG 0x08 /**< Push Function */
>     #define RTE_TCP_RST_FLAG 0x04 /**< Reset the connection */
>     #define RTE_TCP_SYN_FLAG 0x02 /**< Synchronize sequence numbers */
>     #define RTE_TCP_FIN_FLAG 0x01 /**< No more data from sender */ but
> there are no existing examples that demonstrate how to use this feature.
> 
> This patch extends the exisiting classification support to allow an optional

Typo: " exisiting"  should be "existing"

> flags in the input file. The flags string can be any concatenation of characters
> from {C, E, U, A, P, R, S, F} and "*" indicates "dont care". These flags are set in
> the ntuple_filter and are used to construct the tcp_spec and tcp_mask sent
> to the driver
> 
> The rte_acl_field_def is updated to use the (u8) tcp flag as lookup key.
> Note that, as per
>   https://doc.dpdk.org/guides/prog_guide/packet_classif_access_ctrl.html
> this field MUST be allocated fo4 4 bytes, thus it has sizeof(uint32_t).

Typo:  "fo4" should be "for"
 
> 
> However, also note the XXX in this commit: additional updates are needed to
> the rte_flow_classify_table_entry_add() so that it does not ignore any key
> fields other than the 5-tuple.
> 
> Signed-off-by: Sowmini Varadhan <sovaradh@linux.microsoft.com>
> ---
>  examples/flow_classify/flow_classify.c     | 87 ++++++++++++++++++++--
>  examples/flow_classify/ipv4_rules_file.txt | 22 +++---
>  2 files changed, 91 insertions(+), 18 deletions(-)
> 
> diff --git a/examples/flow_classify/flow_classify.c
> b/examples/flow_classify/flow_classify.c
> index db71f5aa04..772b15adf2 100644
> --- a/examples/flow_classify/flow_classify.c
> +++ b/examples/flow_classify/flow_classify.c
> @@ -51,6 +51,7 @@ enum {
>  	CB_FLD_DST_PORT_MASK,
>  	CB_FLD_PROTO,
>  	CB_FLD_PRIORITY,
> +	CB_FLD_TCP_FLAGS,
>  	CB_FLD_NUM,
>  };
> 
> @@ -86,6 +87,7 @@ enum {
>  	DST_FIELD_IPV4,
>  	SRCP_FIELD_IPV4,
>  	DSTP_FIELD_IPV4,
> +	TCP_FLAGS_FIELD,
>  	NUM_FIELDS_IPV4
>  };
> 
> @@ -93,7 +95,8 @@ enum {
>  	PROTO_INPUT_IPV4,
>  	SRC_INPUT_IPV4,
>  	DST_INPUT_IPV4,
> -	SRCP_DESTP_INPUT_IPV4
> +	SRCP_DESTP_INPUT_IPV4,
> +	TCP_FLAGS_INDEX,
>  };
> 
>  static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = { @@ -150,6
> +153,17 @@ static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
>  			sizeof(struct rte_ipv4_hdr) +
>  			offsetof(struct rte_tcp_hdr, dst_port),
>  	},
> +	/* next field must be 4 bytes, even though flags is only 1 byte */
> +	{
> +		/* rte_flags */
> +		.type = RTE_ACL_FIELD_TYPE_BITMASK,
> +		.size = sizeof(uint32_t),
> +		.field_index = TCP_FLAGS_FIELD,
> +		.input_index = TCP_FLAGS_INDEX,
> +		.offset = sizeof(struct rte_ether_hdr) +
> +			sizeof(struct rte_ipv4_hdr) +
> +			offsetof(struct rte_tcp_hdr, tcp_flags),
> +	},
>  };
>  /* >8 End of creation of ACL table. */
> 
> @@ -285,12 +299,14 @@ lcore_main(struct flow_classifier *cls_app)
>  	int ret;
>  	int i = 0;
> 
> -	ret = rte_flow_classify_table_entry_delete(cls_app->cls,
> -			rules[7]);
> -	if (ret)
> -		printf("table_entry_delete failed [7] %d\n\n", ret);
> -	else
> -		printf("table_entry_delete succeeded [7]\n\n");
> +	if (rules[7]) {
> +		ret = rte_flow_classify_table_entry_delete(cls_app->cls,
> +				rules[7]);
> +		if (ret)
> +			printf("table_entry_delete failed [7] %d\n\n", ret);
> +		else
> +			printf("table_entry_delete succeeded [7]\n\n");
> +	}
> 
>  	/*
>  	 * Check that the port is on the same NUMA node as the polling
> thread @@ -410,6 +426,53 @@ parse_ipv4_net(char *in, uint32_t *addr,
> uint32_t *mask_len)
>  	return 0;
>  }
> 
> +static int
> +get_tcp_flags(char *in, struct rte_eth_ntuple_filter *ntuple_filter) {
> +	int len = strlen(in);
> +	int i;
> +	uint8_t flags = 0;
> +
> +	if (strcmp(in, "*") == 0) {
> +		ntuple_filter->tcp_flags = 0;
> +		return 0;
> +	}
> +
> +	for (i = 0; i < len; i++) {
> +		switch (in[i]) {
> +		case 'S':
> +			flags |= RTE_TCP_SYN_FLAG;
> +			break;
> +		case 'F':
> +			flags |= RTE_TCP_FIN_FLAG;
> +			break;
> +		case 'R':
> +			flags |= RTE_TCP_RST_FLAG;
> +			break;
> +		case 'P':
> +			flags |= RTE_TCP_PSH_FLAG;
> +			break;
> +		case 'A':
> +			flags |= RTE_TCP_ACK_FLAG;
> +			break;
> +		case 'U':
> +			flags |= RTE_TCP_URG_FLAG;
> +			break;
> +		case 'E':
> +			flags |= RTE_TCP_ECE_FLAG;
> +			break;
> +		case 'C':
> +			flags |= RTE_TCP_CWR_FLAG;
> +			break;
> +		default:
> +			fprintf(stderr, "unknown flag %c\n", in[i]);
> +			return -1;

Probably better to return -EINVAL  similar to other functions.

> +		}
> +	}
> +	ntuple_filter->tcp_flags = flags;
> +	return 0;
> +}
> +
>  static int
>  parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter
> *ntuple_filter)  { @@ -481,6 +544,8 @@ parse_ipv4_5tuple_rule(char *str,
> struct rte_eth_ntuple_filter *ntuple_filter)
>  	ntuple_filter->priority = (uint16_t)temp;
>  	if (ntuple_filter->priority > FLOW_CLASSIFY_MAX_PRIORITY)
>  		ret = -EINVAL;
> +	if (get_tcp_flags(in[CB_FLD_TCP_FLAGS], ntuple_filter))
> +		return -EINVAL;
> 
>  	return ret;
>  }
> @@ -597,10 +662,13 @@ add_classify_rule(struct rte_eth_ntuple_filter
> *ntuple_filter,
>  		memset(&tcp_spec, 0, sizeof(tcp_spec));
>  		tcp_spec.hdr.src_port = ntuple_filter->src_port;
>  		tcp_spec.hdr.dst_port = ntuple_filter->dst_port;
> +		tcp_spec.hdr.tcp_flags = ntuple_filter->tcp_flags;
> 
>  		memset(&tcp_mask, 0, sizeof(tcp_mask));
>  		tcp_mask.hdr.src_port = ntuple_filter->src_port_mask;
>  		tcp_mask.hdr.dst_port = ntuple_filter->dst_port_mask;
> +		if (tcp_spec.hdr.tcp_flags != 0)
> +			tcp_mask.hdr.tcp_flags = 0xff;
> 
>  		tcp_item.type = RTE_FLOW_ITEM_TYPE_TCP;
>  		tcp_item.spec = &tcp_spec;
> @@ -655,6 +723,11 @@ add_classify_rule(struct rte_eth_ntuple_filter
> *ntuple_filter,
>  		return ret;
>  	}
> 
> +	/* XXX but this only adds table_type of
> +	 * RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
> +	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule()
> +	 * so the tcp_flags is ignored!
> +	 */
>  	rule = rte_flow_classify_table_entry_add(
>  			cls_app->cls, &attr, pattern_ipv4_5tuple,
>  			actions, &key_found, &error);
> diff --git a/examples/flow_classify/ipv4_rules_file.txt
> b/examples/flow_classify/ipv4_rules_file.txt
> index dfa0631fcc..415573732a 100644
> --- a/examples/flow_classify/ipv4_rules_file.txt
> +++ b/examples/flow_classify/ipv4_rules_file.txt
> @@ -1,14 +1,14 @@
>  #file format:
> -#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask
> proto/mask priority
> +#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask
> +proto/mask priority tcp_flags
>  #
> -2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0
> -9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1
> -9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2
> -9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3
> -6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4
> -6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5
> -6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6
> -6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7
> -6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8
> +2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0 *
> +9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1 *
> +9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2 *
> +9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3 *
> +6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4 *
> +6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5 *
> +6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6 *
> +6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7 *
> +6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8 *
>  #error rules
> -#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9 \ No newline at end
> of file
> +#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9 *
> --
> 2.17.1


^ permalink raw reply	[relevance 6%]

* Re: [dpdk-dev] [PATCH 2/2] Allow the flow_classify example to add an ACL table for tcp.
  2021-08-12 20:17 10% ` [dpdk-dev] [PATCH 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
@ 2021-08-17 12:08  6%   ` Iremonger, Bernard
  0 siblings, 0 replies; 38+ results
From: Iremonger, Bernard @ 2021-08-17 12:08 UTC (permalink / raw)
  To: Sowmini Varadhan, sowmini05, dev, sowmin05; +Cc: thomas

Hi Sowmini,

> -----Original Message-----
> From: Sowmini Varadhan <sovaradh@linux.microsoft.com>
> Sent: Thursday, August 12, 2021 9:18 PM
> To: sowmini05@gmail.com; Iremonger, Bernard
> <bernard.iremonger@intel.com>; dev@dpdk.org; sowmin05@gmail.com;
> sovaradh@linux.microsoft.com
> Cc: thomas@monjalon.net
> Subject: [PATCH 2/2] Allow the flow_classify example to add an ACL table for
> tcp.

./check-git-log.sh -1
Wrong headline format:
        Allow the flow_classify example to add an ACL table for tcp.

The subject line should be prefixed with examples/flow_classify:
flow_classify: Allow the flow_classify example to add an ACL table for tcp

> 
> The struct rte_flow_classifier can have upto
> RTE_FLOW_CLASSIFY_TABLE_MAX
> (32) classifier tables, but the existing flow_classify examples only adds a
> single table for the L4 5-tuple.
> 
> When dealing with tcp flows, we frequently want to add add ACLs and filters
> to filter based on the state of the TCP connection, e.g., by looking at the tcp
> flags field.
> 
> So we enhance flow_classify to add an additional acl table for tcp 5-typles. If
> the input-file-parser detects a filter for a tcp flow with a non-wildcard
> argument for tcp_flags, the IP4_TCP_5TUPLE table is used by flow_classify.
> 
> Signed-off-by: Sowmini Varadhan <sovaradh@linux.microsoft.com>
> ---
>  examples/flow_classify/flow_classify.c      | 41 +++++++---
>  lib/flow_classify/rte_flow_classify.c       | 87 +++++++++++++++++++++
>  lib/flow_classify/rte_flow_classify.h       | 19 +++++
>  lib/flow_classify/rte_flow_classify_parse.c |  8 +-
>  4 files changed, 142 insertions(+), 13 deletions(-)
> 
> diff --git a/examples/flow_classify/flow_classify.c
> b/examples/flow_classify/flow_classify.c
> index 772b15adf2..b42d0df5c3 100644
> --- a/examples/flow_classify/flow_classify.c
> +++ b/examples/flow_classify/flow_classify.c
> @@ -723,11 +723,6 @@ add_classify_rule(struct rte_eth_ntuple_filter
> *ntuple_filter,
>  		return ret;
>  	}
> 
> -	/* XXX but this only adds table_type of
> -	 * RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
> -	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule()
> -	 * so the tcp_flags is ignored!
> -	 */
>  	rule = rte_flow_classify_table_entry_add(
>  			cls_app->cls, &attr, pattern_ipv4_5tuple,
>  			actions, &key_found, &error);
> @@ -856,7 +851,8 @@ main(int argc, char *argv[])
>  	int ret;
>  	int socket_id;
>  	struct rte_table_acl_params table_acl_params;
> -	struct rte_flow_classify_table_params cls_table_params;
> +	struct rte_table_acl_params table_acl_tcp_params;
> +	struct rte_flow_classify_table_params cls_table_params[2];
>  	struct flow_classifier *cls_app;
>  	struct rte_flow_classifier_params cls_params;
>  	uint32_t size;
> @@ -923,21 +919,42 @@ main(int argc, char *argv[])
>  	memcpy(table_acl_params.field_format, ipv4_defs,
> sizeof(ipv4_defs));
> 
>  	/* initialise table create params */
> -	cls_table_params.ops = &rte_table_acl_ops;
> -	cls_table_params.arg_create = &table_acl_params;
> -	cls_table_params.type =
> RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
> +	cls_table_params[0].ops = &rte_table_acl_ops;
> +	cls_table_params[0].arg_create = &table_acl_params;
> +	cls_table_params[0].type =
> RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
> +
> +	/* initialise ACL table params */
> +	table_acl_tcp_params.name = "table_acl_ipv4_tcp_5tuple";
> +	table_acl_tcp_params.n_rules = FLOW_CLASSIFY_MAX_RULE_NUM;
> +	table_acl_tcp_params.n_rule_fields = RTE_DIM(ipv4_defs);
> +	memcpy(table_acl_tcp_params.field_format, ipv4_defs,
> +sizeof(ipv4_defs));
> +
> +	/* initialise table create params */
> +	cls_table_params[1].ops = &rte_table_acl_ops;
> +	cls_table_params[1].arg_create = &table_acl_tcp_params;
> +	cls_table_params[1].type =
> RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
> 
> -	ret = rte_flow_classify_table_create(cls_app->cls,
> &cls_table_params);
> +	ret = rte_flow_classify_table_create(cls_app->cls,
> +					     &cls_table_params[0]);
>  	if (ret) {
>  		rte_flow_classifier_free(cls_app->cls);
>  		rte_free(cls_app);
>  		rte_exit(EXIT_FAILURE, "Failed to create classifier table\n");
>  	}
> +	ret = rte_flow_classify_table_create(cls_app->cls,
> +					     &cls_table_params[1]);
> +	if (ret) {
> +		rte_flow_classifier_free(cls_app->cls);
> +		rte_free(cls_app);
> +		rte_exit(EXIT_FAILURE,
> +			 "Failed to create classifier table\n");
> +	}
> +
>  	/* >8 End of initialization of table create params. */
> 
>  	/* read file of IPv4 5 tuple rules and initialize parameters
> -	 * for rte_flow_classify_validate and
> rte_flow_classify_table_entry_add
> -	 * API's.
> +	 * for rte_flow_classify_validate and
> +	 * rte_flow_classify_table_entry_add  API's.
>  	 */
> 
>  	/* Read file of IPv4 tuple rules. 8< */ diff --git
> a/lib/flow_classify/rte_flow_classify.c b/lib/flow_classify/rte_flow_classify.c
> index d3ba2ed227..e960c3b140 100644
> --- a/lib/flow_classify/rte_flow_classify.c
> +++ b/lib/flow_classify/rte_flow_classify.c
> @@ -60,6 +60,7 @@ enum {
>  	DST_FIELD_IPV4,
>  	SRCP_FIELD_IPV4,
>  	DSTP_FIELD_IPV4,
> +	TCP_FLAGS_FIELD,
>  	NUM_FIELDS_IPV4
>  };
> 
> @@ -72,6 +73,7 @@ struct classify_rules {
>  	enum rte_flow_classify_rule_type type;
>  	union {
>  		struct rte_flow_classify_ipv4_5tuple ipv4_5tuple;
> +		struct rte_flow_classify_ipv4_tcp_5tuple ipv4_tcp_5tuple;
>  	} u;
>  };
> 
> @@ -477,6 +479,84 @@ allocate_acl_ipv4_5tuple_rule(struct
> rte_flow_classifier *cls)
>  	return rule;
>  }
> 
> +static struct rte_flow_classify_rule *
> +allocate_acl_ipv4_tcp_5tuple_rule(struct rte_flow_classifier *cls) {
> +	struct rte_flow_classify_rule *rule;
> +	int log_level;
> +
> +	rule = malloc(sizeof(struct rte_flow_classify_rule));
> +	if (!rule)
> +		return rule;
> +
> +	memset(rule, 0, sizeof(struct rte_flow_classify_rule));
> +	rule->id = unique_id++;
> +	rule->rules.type =
> RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE;
> +
> +	/* key add values */
> +	rule->u.key.key_add.priority = cls->ntuple_filter.priority;
> +	rule-
> >u.key.key_add.field_value[PROTO_FIELD_IPV4].mask_range.u8 =
> +			cls->ntuple_filter.proto_mask;
> +	rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].value.u8 =
> +			cls->ntuple_filter.proto;
> +	rule->rules.u.ipv4_tcp_5tuple.proto = cls->ntuple_filter.proto;
> +	rule->rules.u.ipv4_tcp_5tuple.proto_mask =
> +			cls->ntuple_filter.proto_mask;
> +
> +	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].mask_range.u32
> =
> +			cls->ntuple_filter.src_ip_mask;
> +	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].value.u32 =
> +			cls->ntuple_filter.src_ip;
> +	rule->rules.u.ipv4_tcp_5tuple.src_ip_mask =
> +			cls->ntuple_filter.src_ip_mask;
> +	rule->rules.u.ipv4_tcp_5tuple.src_ip = cls->ntuple_filter.src_ip;
> +
> +	rule->u.key.key_add.field_value[DST_FIELD_IPV4].mask_range.u32
> =
> +			cls->ntuple_filter.dst_ip_mask;
> +	rule->u.key.key_add.field_value[DST_FIELD_IPV4].value.u32 =
> +			cls->ntuple_filter.dst_ip;
> +	rule->rules.u.ipv4_tcp_5tuple.dst_ip_mask =
> +			cls->ntuple_filter.dst_ip_mask;
> +	rule->rules.u.ipv4_tcp_5tuple.dst_ip = cls->ntuple_filter.dst_ip;
> +
> +	rule-
> >u.key.key_add.field_value[SRCP_FIELD_IPV4].mask_range.u16 =
> +			cls->ntuple_filter.src_port_mask;
> +	rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].value.u16 =
> +			cls->ntuple_filter.src_port;
> +	rule->rules.u.ipv4_tcp_5tuple.src_port_mask =
> +			cls->ntuple_filter.src_port_mask;
> +	rule->rules.u.ipv4_tcp_5tuple.src_port = cls->ntuple_filter.src_port;
> +
> +	rule-
> >u.key.key_add.field_value[DSTP_FIELD_IPV4].mask_range.u16 =
> +			cls->ntuple_filter.dst_port_mask;
> +	rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].value.u16 =
> +			cls->ntuple_filter.dst_port;
> +	rule->rules.u.ipv4_tcp_5tuple.dst_port_mask =
> +			cls->ntuple_filter.dst_port_mask;
> +	rule->rules.u.ipv4_tcp_5tuple.dst_port = cls->ntuple_filter.dst_port;
> +
> +	rule-
> >u.key.key_add.field_value[TCP_FLAGS_FIELD].mask_range.u32 =
> +			rte_be_to_cpu_32(0xff);
> +	rule->u.key.key_add.field_value[TCP_FLAGS_FIELD].value.u32 =
> +			rte_be_to_cpu_32(cls->ntuple_filter.tcp_flags);
> +	rule->rules.u.ipv4_tcp_5tuple.tcp_flags =
> +cls->ntuple_filter.tcp_flags;
> +
> +	log_level = rte_log_get_level(librte_flow_classify_logtype);
> +
> +	if (log_level == RTE_LOG_DEBUG)
> +		print_acl_ipv4_key_add(&rule->u.key.key_add);
> +
> +	/* key delete values */
> +	memcpy(&rule->u.key.key_del.field_value[PROTO_FIELD_IPV4],
> +	       &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4],
> +	       NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field));
> +
> +	if (log_level == RTE_LOG_DEBUG)
> +		print_acl_ipv4_key_delete(&rule->u.key.key_del);
> +
> +	return rule;
> +}
> +
>  struct rte_flow_classify_rule *
>  rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls,
>  		const struct rte_flow_attr *attr,
> @@ -514,6 +594,13 @@ rte_flow_classify_table_entry_add(struct
> rte_flow_classifier *cls,
>  		rule->tbl_type = table_type;
>  		cls->table_mask |= table_type;
>  		break;
> +	case RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE:
> +		rule = allocate_acl_ipv4_tcp_5tuple_rule(cls);
> +		if (!rule)
> +			return NULL;
> +		rule->tbl_type = table_type;
> +		cls->table_mask |= table_type;
> +		break;
>  	default:
>  		return NULL;
>  	}
> diff --git a/lib/flow_classify/rte_flow_classify.h
> b/lib/flow_classify/rte_flow_classify.h
> index 82ea92b6a6..65585d9f92 100644
> --- a/lib/flow_classify/rte_flow_classify.h
> +++ b/lib/flow_classify/rte_flow_classify.h
> @@ -80,6 +80,8 @@ enum rte_flow_classify_rule_type {
>  	RTE_FLOW_CLASSIFY_RULE_TYPE_NONE,
>  	/** IPv4 5tuple type */
>  	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_5TUPLE,
> +	/** IPv4 TCP 5tuple type */
> +	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE,
>  };
> 
>  /** Flow classify table type */
> @@ -92,6 +94,8 @@ enum rte_flow_classify_table_type {
>  	RTE_FLOW_CLASSIFY_TABLE_ACL_VLAN_IP4_5TUPLE = 1 << 2,
>  	/** ACL QinQ IP4 5TUPLE */
>  	RTE_FLOW_CLASSIFY_TABLE_ACL_QINQ_IP4_5TUPLE = 1 << 3,
> +	/** ACL IP4 5TUPLE with tcp_flags */
> +	RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE = 1 << 4,
> 
>  };
> 
> @@ -131,6 +135,21 @@ struct rte_flow_classify_ipv4_5tuple {
>  	uint8_t proto_mask;      /**< Mask of L4 protocol. */
>  };
> 
> +/** IPv4 5-tuple data with tcp flags*/
> +struct rte_flow_classify_ipv4_tcp_5tuple {
> +	uint32_t dst_ip;         /**< Destination IP address in big endian. */
> +	uint32_t dst_ip_mask;    /**< Mask of destination IP address. */
> +	uint32_t src_ip;         /**< Source IP address in big endian. */
> +	uint32_t src_ip_mask;    /**< Mask of destination IP address. */
> +	uint16_t dst_port;       /**< Destination port in big endian. */
> +	uint16_t dst_port_mask;  /**< Mask of destination port. */
> +	uint16_t src_port;       /**< Source Port in big endian. */
> +	uint16_t src_port_mask;  /**< Mask of source port. */
> +	uint8_t proto;           /**< L4 protocol. */
> +	uint8_t proto_mask;      /**< Mask of L4 protocol. */
> +	uint8_t tcp_flags;       /**< Tcp only */
> +};
> +
>  /**
>   * Flow stats
>   *
> diff --git a/lib/flow_classify/rte_flow_classify_parse.c
> b/lib/flow_classify/rte_flow_classify_parse.c
> index 465330291f..fe4ee05b6f 100644
> --- a/lib/flow_classify/rte_flow_classify_parse.c
> +++ b/lib/flow_classify/rte_flow_classify_parse.c
> @@ -216,6 +216,7 @@ classify_parse_ntuple_filter(const struct
> rte_flow_attr *attr,
>  	const struct rte_flow_action_count *count;
>  	const struct rte_flow_action_mark *mark_spec;
>  	uint32_t index;
> +	bool have_tcp = false;
> 
>  	/* parse pattern */
>  	index = 0;
> @@ -375,6 +376,8 @@ classify_parse_ntuple_filter(const struct
> rte_flow_attr *attr,
>  		filter->dst_port  = tcp_spec->hdr.dst_port;
>  		filter->src_port  = tcp_spec->hdr.src_port;
>  		filter->tcp_flags = tcp_spec->hdr.tcp_flags;
> +		if (filter->tcp_flags != 0)
> +			have_tcp = true;
>  	} else if (item->type == RTE_FLOW_ITEM_TYPE_UDP) {
>  		udp_mask = item->mask;
> 
> @@ -434,7 +437,10 @@ classify_parse_ntuple_filter(const struct
> rte_flow_attr *attr,
>  		return -EINVAL;
>  	}
> 
> -	table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
> +	if (have_tcp)
> +		table_type =
> RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
> +	else
> +		table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
> 
>  	/* parse attr */
>  	/* must be input direction */
> --
> 2.17.1

This patch for some reason has "..patch" instead of ".patch"
Could you use ".patch" in the v2 .

Regards,

Bernard.


^ permalink raw reply	[relevance 6%]

* [dpdk-dev] [PATCH v2 0/2] TCP flow classification using 4-tuple and flags
@ 2021-08-18 15:01 13% Sowmini Varadhan
  2021-08-18 15:01 11% ` [dpdk-dev] [PATCH v2 1/2] examples/flow_classify: hooks for filters on tcp flags Sowmini Varadhan
  2021-08-18 15:01 10% ` [dpdk-dev] [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp Sowmini Varadhan
  0 siblings, 2 replies; 38+ results
From: Sowmini Varadhan @ 2021-08-18 15:01 UTC (permalink / raw)
  To: sowmini05, bernard.iremonger, dev, sovaradh; +Cc: thomas

V2 updates: checkpatch typo fixes, bernard.iremonger review fixes

The problem space of TCP flow tracking and classification
based on TCP state requires the ability to classify TCP
flows on more packet properties than just the 4-tuple,
e.g., TCP flags. This patch-set provides the set of
changes needed in the examples/flow_classify.c needed to
achieve this.

Patch 1 extends examples/flow_classify.c to allow constraints
on tcp flags. Patch 2 extends the ACL handling in
librte_flow_classify to include keys on the properties in
addition to the tcp 4-tuple.

Sowmini Varadhan (2):
  examples/flow_classify: hooks for filters on tcp flags
  examples/flow_classify: add an ACL table for tcp

 examples/flow_classify/flow_classify.c      | 118 +++++++++++++++++---
 examples/flow_classify/ipv4_rules_file.txt  |  22 ++--
 lib/flow_classify/rte_flow_classify.c       |  87 +++++++++++++++
 lib/flow_classify/rte_flow_classify.h       |  19 ++++
 lib/flow_classify/rte_flow_classify_parse.c |   8 +-
 5 files changed, 228 insertions(+), 26 deletions(-)

-- 
2.17.1


^ permalink raw reply	[relevance 13%]

* [dpdk-dev] [PATCH v2 1/2] examples/flow_classify: hooks for filters on tcp flags
  2021-08-18 15:01 13% [dpdk-dev] [PATCH v2 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
@ 2021-08-18 15:01 11% ` Sowmini Varadhan
  2021-08-19 16:08  6%   ` Iremonger, Bernard
  2021-08-18 15:01 10% ` [dpdk-dev] [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp Sowmini Varadhan
  1 sibling, 1 reply; 38+ results
From: Sowmini Varadhan @ 2021-08-18 15:01 UTC (permalink / raw)
  To: sowmini05, bernard.iremonger, dev, sovaradh; +Cc: thomas

v2 fixes:  typo fixes, get_tcp_flags returns -EINVAL

The rte_eth_ntuple_filter allows tcp_flags which can check for things
like
    #define RTE_TCP_CWR_FLAG 0x80 /**< Congestion Window Reduced */
    #define RTE_TCP_ECE_FLAG 0x40 /**< ECN-Echo */
    #define RTE_TCP_URG_FLAG 0x20 /**< Urgent Pointer field significant */
    #define RTE_TCP_ACK_FLAG 0x10 /**< Acknowledgment field significant */
    #define RTE_TCP_PSH_FLAG 0x08 /**< Push Function */
    #define RTE_TCP_RST_FLAG 0x04 /**< Reset the connection */
    #define RTE_TCP_SYN_FLAG 0x02 /**< Synchronize sequence numbers */
    #define RTE_TCP_FIN_FLAG 0x01 /**< No more data from sender */
but there are no existing examples that demonstrate how to use this
feature.

This patch extends the existing classification support to allow
an optional flags in the input file. The flags string can be any
concatenation of characters from {C, E, U, A, P, R, S, F} and
"*" indicates "don't care". These flags are set in the ntuple_filter and
are used to construct the tcp_spec and tcp_mask sent to the driver

The rte_acl_field_def is updated to use the (u8) tcp flag as lookup key.
Note that, as per
  https://doc.dpdk.org/guides/prog_guide/packet_classif_access_ctrl.html
this field MUST be allocated for 4 bytes, thus it has sizeof(uint32_t).

However, also note the XXX in this commit: additional updates are
needed to the rte_flow_classify_table_entry_add() so that it does
not ignore any key fields other than the 5-tuple.

Signed-off-by: Sowmini Varadhan <sovaradh@linux.microsoft.com>
---
 examples/flow_classify/flow_classify.c     | 87 ++++++++++++++++++++--
 examples/flow_classify/ipv4_rules_file.txt | 22 +++---
 2 files changed, 91 insertions(+), 18 deletions(-)

diff --git a/examples/flow_classify/flow_classify.c b/examples/flow_classify/flow_classify.c
index db71f5aa04..ff4c8bc2f5 100644
--- a/examples/flow_classify/flow_classify.c
+++ b/examples/flow_classify/flow_classify.c
@@ -51,6 +51,7 @@ enum {
 	CB_FLD_DST_PORT_MASK,
 	CB_FLD_PROTO,
 	CB_FLD_PRIORITY,
+	CB_FLD_TCP_FLAGS,
 	CB_FLD_NUM,
 };
 
@@ -86,6 +87,7 @@ enum {
 	DST_FIELD_IPV4,
 	SRCP_FIELD_IPV4,
 	DSTP_FIELD_IPV4,
+	TCP_FLAGS_FIELD,
 	NUM_FIELDS_IPV4
 };
 
@@ -93,7 +95,8 @@ enum {
 	PROTO_INPUT_IPV4,
 	SRC_INPUT_IPV4,
 	DST_INPUT_IPV4,
-	SRCP_DESTP_INPUT_IPV4
+	SRCP_DESTP_INPUT_IPV4,
+	TCP_FLAGS_INDEX,
 };
 
 static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
@@ -150,6 +153,17 @@ static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
 			sizeof(struct rte_ipv4_hdr) +
 			offsetof(struct rte_tcp_hdr, dst_port),
 	},
+	/* next field must be 4 bytes, even though flags is only 1 byte */
+	{
+		/* rte_flags */
+		.type = RTE_ACL_FIELD_TYPE_BITMASK,
+		.size = sizeof(uint32_t),
+		.field_index = TCP_FLAGS_FIELD,
+		.input_index = TCP_FLAGS_INDEX,
+		.offset = sizeof(struct rte_ether_hdr) +
+			sizeof(struct rte_ipv4_hdr) +
+			offsetof(struct rte_tcp_hdr, tcp_flags),
+	},
 };
 /* >8 End of creation of ACL table. */
 
@@ -285,12 +299,14 @@ lcore_main(struct flow_classifier *cls_app)
 	int ret;
 	int i = 0;
 
-	ret = rte_flow_classify_table_entry_delete(cls_app->cls,
-			rules[7]);
-	if (ret)
-		printf("table_entry_delete failed [7] %d\n\n", ret);
-	else
-		printf("table_entry_delete succeeded [7]\n\n");
+	if (rules[7]) {
+		ret = rte_flow_classify_table_entry_delete(cls_app->cls,
+				rules[7]);
+		if (ret)
+			printf("table_entry_delete failed [7] %d\n\n", ret);
+		else
+			printf("table_entry_delete succeeded [7]\n\n");
+	}
 
 	/*
 	 * Check that the port is on the same NUMA node as the polling thread
@@ -410,6 +426,53 @@ parse_ipv4_net(char *in, uint32_t *addr, uint32_t *mask_len)
 	return 0;
 }
 
+static int
+get_tcp_flags(char *in, struct rte_eth_ntuple_filter *ntuple_filter)
+{
+	int len = strlen(in);
+	int i;
+	uint8_t flags = 0;
+
+	if (strcmp(in, "*") == 0) {
+		ntuple_filter->tcp_flags = 0;
+		return 0;
+	}
+
+	for (i = 0; i < len; i++) {
+		switch (in[i]) {
+		case 'S':
+			flags |= RTE_TCP_SYN_FLAG;
+			break;
+		case 'F':
+			flags |= RTE_TCP_FIN_FLAG;
+			break;
+		case 'R':
+			flags |= RTE_TCP_RST_FLAG;
+			break;
+		case 'P':
+			flags |= RTE_TCP_PSH_FLAG;
+			break;
+		case 'A':
+			flags |= RTE_TCP_ACK_FLAG;
+			break;
+		case 'U':
+			flags |= RTE_TCP_URG_FLAG;
+			break;
+		case 'E':
+			flags |= RTE_TCP_ECE_FLAG;
+			break;
+		case 'C':
+			flags |= RTE_TCP_CWR_FLAG;
+			break;
+		default:
+			fprintf(stderr, "unknown flag %c\n", in[i]);
+			return -EINVAL;
+		}
+	}
+	ntuple_filter->tcp_flags = flags;
+	return 0;
+}
+
 static int
 parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter *ntuple_filter)
 {
@@ -481,6 +544,8 @@ parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter *ntuple_filter)
 	ntuple_filter->priority = (uint16_t)temp;
 	if (ntuple_filter->priority > FLOW_CLASSIFY_MAX_PRIORITY)
 		ret = -EINVAL;
+	if (get_tcp_flags(in[CB_FLD_TCP_FLAGS], ntuple_filter))
+		return -EINVAL;
 
 	return ret;
 }
@@ -597,10 +662,13 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		memset(&tcp_spec, 0, sizeof(tcp_spec));
 		tcp_spec.hdr.src_port = ntuple_filter->src_port;
 		tcp_spec.hdr.dst_port = ntuple_filter->dst_port;
+		tcp_spec.hdr.tcp_flags = ntuple_filter->tcp_flags;
 
 		memset(&tcp_mask, 0, sizeof(tcp_mask));
 		tcp_mask.hdr.src_port = ntuple_filter->src_port_mask;
 		tcp_mask.hdr.dst_port = ntuple_filter->dst_port_mask;
+		if (tcp_spec.hdr.tcp_flags != 0)
+			tcp_mask.hdr.tcp_flags = 0xff;
 
 		tcp_item.type = RTE_FLOW_ITEM_TYPE_TCP;
 		tcp_item.spec = &tcp_spec;
@@ -655,6 +723,11 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		return ret;
 	}
 
+	/* XXX but this only adds table_type of
+	 * RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
+	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule()
+	 * so the tcp_flags is ignored!
+	 */
 	rule = rte_flow_classify_table_entry_add(
 			cls_app->cls, &attr, pattern_ipv4_5tuple,
 			actions, &key_found, &error);
diff --git a/examples/flow_classify/ipv4_rules_file.txt b/examples/flow_classify/ipv4_rules_file.txt
index dfa0631fcc..415573732a 100644
--- a/examples/flow_classify/ipv4_rules_file.txt
+++ b/examples/flow_classify/ipv4_rules_file.txt
@@ -1,14 +1,14 @@
 #file format:
-#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask proto/mask priority
+#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask proto/mask priority tcp_flags
 #
-2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0
-9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1
-9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2
-9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3
-6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4
-6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5
-6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6
-6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7
-6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8
+2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0 *
+9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1 *
+9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2 *
+9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3 *
+6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4 *
+6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5 *
+6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6 *
+6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7 *
+6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8 *
 #error rules
-#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9
\ No newline at end of file
+#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9 *
-- 
2.17.1


^ permalink raw reply	[relevance 11%]

* [dpdk-dev] [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp
  2021-08-18 15:01 13% [dpdk-dev] [PATCH v2 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
  2021-08-18 15:01 11% ` [dpdk-dev] [PATCH v2 1/2] examples/flow_classify: hooks for filters on tcp flags Sowmini Varadhan
@ 2021-08-18 15:01 10% ` Sowmini Varadhan
  2021-08-19 15:06  6%   ` Iremonger, Bernard
  1 sibling, 1 reply; 38+ results
From: Sowmini Varadhan @ 2021-08-18 15:01 UTC (permalink / raw)
  To: sowmini05, bernard.iremonger, dev, sovaradh; +Cc: thomas

v2 updates: typo fixes for checkpatch,  bernard.iremonger comments

The struct rte_flow_classifier can have up to RTE_FLOW_CLASSIFY_TABLE_MAX
(32) classifier tables, but the existing flow_classify examples only adds
a single table for the L4 5-tuple.

When dealing with tcp flows, we frequently want to add ACLs and filters
to filter based on the state of the TCP connection, e.g., by looking at
the tcp flags field.

So we enhance flow_classify to add an additional acl table for
tcp 5-tuples. If the input-file-parser detects a filter for a tcp flow with
a non-wildcard argument for tcp_flags, the IP4_TCP_5TUPLE table is used
by flow_classify.

Signed-off-by: Sowmini Varadhan <sovaradh@linux.microsoft.com>
---
 examples/flow_classify/flow_classify.c      | 41 +++++++---
 lib/flow_classify/rte_flow_classify.c       | 87 +++++++++++++++++++++
 lib/flow_classify/rte_flow_classify.h       | 19 +++++
 lib/flow_classify/rte_flow_classify_parse.c |  8 +-
 4 files changed, 142 insertions(+), 13 deletions(-)

diff --git a/examples/flow_classify/flow_classify.c b/examples/flow_classify/flow_classify.c
index ff4c8bc2f5..8f6aacae03 100644
--- a/examples/flow_classify/flow_classify.c
+++ b/examples/flow_classify/flow_classify.c
@@ -723,11 +723,6 @@ add_classify_rule(struct rte_eth_ntuple_filter *ntuple_filter,
 		return ret;
 	}
 
-	/* XXX but this only adds table_type of
-	 * RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
-	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule()
-	 * so the tcp_flags is ignored!
-	 */
 	rule = rte_flow_classify_table_entry_add(
 			cls_app->cls, &attr, pattern_ipv4_5tuple,
 			actions, &key_found, &error);
@@ -856,7 +851,8 @@ main(int argc, char *argv[])
 	int ret;
 	int socket_id;
 	struct rte_table_acl_params table_acl_params;
-	struct rte_flow_classify_table_params cls_table_params;
+	struct rte_table_acl_params table_acl_tcp_params;
+	struct rte_flow_classify_table_params cls_table_params[2];
 	struct flow_classifier *cls_app;
 	struct rte_flow_classifier_params cls_params;
 	uint32_t size;
@@ -923,21 +919,42 @@ main(int argc, char *argv[])
 	memcpy(table_acl_params.field_format, ipv4_defs, sizeof(ipv4_defs));
 
 	/* initialise table create params */
-	cls_table_params.ops = &rte_table_acl_ops;
-	cls_table_params.arg_create = &table_acl_params;
-	cls_table_params.type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+	cls_table_params[0].ops = &rte_table_acl_ops;
+	cls_table_params[0].arg_create = &table_acl_params;
+	cls_table_params[0].type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+
+	/* initialise ACL table params */
+	table_acl_tcp_params.name = "table_acl_ipv4_tcp_5tuple";
+	table_acl_tcp_params.n_rules = FLOW_CLASSIFY_MAX_RULE_NUM;
+	table_acl_tcp_params.n_rule_fields = RTE_DIM(ipv4_defs);
+	memcpy(table_acl_tcp_params.field_format, ipv4_defs, sizeof(ipv4_defs));
+
+	/* initialise table create params */
+	cls_table_params[1].ops = &rte_table_acl_ops;
+	cls_table_params[1].arg_create = &table_acl_tcp_params;
+	cls_table_params[1].type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
 
-	ret = rte_flow_classify_table_create(cls_app->cls, &cls_table_params);
+	ret = rte_flow_classify_table_create(cls_app->cls,
+					     &cls_table_params[0]);
 	if (ret) {
 		rte_flow_classifier_free(cls_app->cls);
 		rte_free(cls_app);
 		rte_exit(EXIT_FAILURE, "Failed to create classifier table\n");
 	}
+	ret = rte_flow_classify_table_create(cls_app->cls,
+					     &cls_table_params[1]);
+	if (ret) {
+		rte_flow_classifier_free(cls_app->cls);
+		rte_free(cls_app);
+		rte_exit(EXIT_FAILURE,
+			 "Failed to create classifier table\n");
+	}
+
 	/* >8 End of initialization of table create params. */
 
 	/* read file of IPv4 5 tuple rules and initialize parameters
-	 * for rte_flow_classify_validate and rte_flow_classify_table_entry_add
-	 * API's.
+	 * for rte_flow_classify_validate and
+	 * rte_flow_classify_table_entry_add  API's.
 	 */
 
 	/* Read file of IPv4 tuple rules. 8< */
diff --git a/lib/flow_classify/rte_flow_classify.c b/lib/flow_classify/rte_flow_classify.c
index d3ba2ed227..e960c3b140 100644
--- a/lib/flow_classify/rte_flow_classify.c
+++ b/lib/flow_classify/rte_flow_classify.c
@@ -60,6 +60,7 @@ enum {
 	DST_FIELD_IPV4,
 	SRCP_FIELD_IPV4,
 	DSTP_FIELD_IPV4,
+	TCP_FLAGS_FIELD,
 	NUM_FIELDS_IPV4
 };
 
@@ -72,6 +73,7 @@ struct classify_rules {
 	enum rte_flow_classify_rule_type type;
 	union {
 		struct rte_flow_classify_ipv4_5tuple ipv4_5tuple;
+		struct rte_flow_classify_ipv4_tcp_5tuple ipv4_tcp_5tuple;
 	} u;
 };
 
@@ -477,6 +479,84 @@ allocate_acl_ipv4_5tuple_rule(struct rte_flow_classifier *cls)
 	return rule;
 }
 
+static struct rte_flow_classify_rule *
+allocate_acl_ipv4_tcp_5tuple_rule(struct rte_flow_classifier *cls)
+{
+	struct rte_flow_classify_rule *rule;
+	int log_level;
+
+	rule = malloc(sizeof(struct rte_flow_classify_rule));
+	if (!rule)
+		return rule;
+
+	memset(rule, 0, sizeof(struct rte_flow_classify_rule));
+	rule->id = unique_id++;
+	rule->rules.type = RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE;
+
+	/* key add values */
+	rule->u.key.key_add.priority = cls->ntuple_filter.priority;
+	rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].mask_range.u8 =
+			cls->ntuple_filter.proto_mask;
+	rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].value.u8 =
+			cls->ntuple_filter.proto;
+	rule->rules.u.ipv4_tcp_5tuple.proto = cls->ntuple_filter.proto;
+	rule->rules.u.ipv4_tcp_5tuple.proto_mask =
+			cls->ntuple_filter.proto_mask;
+
+	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].mask_range.u32 =
+			cls->ntuple_filter.src_ip_mask;
+	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].value.u32 =
+			cls->ntuple_filter.src_ip;
+	rule->rules.u.ipv4_tcp_5tuple.src_ip_mask =
+			cls->ntuple_filter.src_ip_mask;
+	rule->rules.u.ipv4_tcp_5tuple.src_ip = cls->ntuple_filter.src_ip;
+
+	rule->u.key.key_add.field_value[DST_FIELD_IPV4].mask_range.u32 =
+			cls->ntuple_filter.dst_ip_mask;
+	rule->u.key.key_add.field_value[DST_FIELD_IPV4].value.u32 =
+			cls->ntuple_filter.dst_ip;
+	rule->rules.u.ipv4_tcp_5tuple.dst_ip_mask =
+			cls->ntuple_filter.dst_ip_mask;
+	rule->rules.u.ipv4_tcp_5tuple.dst_ip = cls->ntuple_filter.dst_ip;
+
+	rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].mask_range.u16 =
+			cls->ntuple_filter.src_port_mask;
+	rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].value.u16 =
+			cls->ntuple_filter.src_port;
+	rule->rules.u.ipv4_tcp_5tuple.src_port_mask =
+			cls->ntuple_filter.src_port_mask;
+	rule->rules.u.ipv4_tcp_5tuple.src_port = cls->ntuple_filter.src_port;
+
+	rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].mask_range.u16 =
+			cls->ntuple_filter.dst_port_mask;
+	rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].value.u16 =
+			cls->ntuple_filter.dst_port;
+	rule->rules.u.ipv4_tcp_5tuple.dst_port_mask =
+			cls->ntuple_filter.dst_port_mask;
+	rule->rules.u.ipv4_tcp_5tuple.dst_port = cls->ntuple_filter.dst_port;
+
+	rule->u.key.key_add.field_value[TCP_FLAGS_FIELD].mask_range.u32 =
+			rte_be_to_cpu_32(0xff);
+	rule->u.key.key_add.field_value[TCP_FLAGS_FIELD].value.u32 =
+			rte_be_to_cpu_32(cls->ntuple_filter.tcp_flags);
+	rule->rules.u.ipv4_tcp_5tuple.tcp_flags = cls->ntuple_filter.tcp_flags;
+
+	log_level = rte_log_get_level(librte_flow_classify_logtype);
+
+	if (log_level == RTE_LOG_DEBUG)
+		print_acl_ipv4_key_add(&rule->u.key.key_add);
+
+	/* key delete values */
+	memcpy(&rule->u.key.key_del.field_value[PROTO_FIELD_IPV4],
+	       &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4],
+	       NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field));
+
+	if (log_level == RTE_LOG_DEBUG)
+		print_acl_ipv4_key_delete(&rule->u.key.key_del);
+
+	return rule;
+}
+
 struct rte_flow_classify_rule *
 rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls,
 		const struct rte_flow_attr *attr,
@@ -514,6 +594,13 @@ rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls,
 		rule->tbl_type = table_type;
 		cls->table_mask |= table_type;
 		break;
+	case RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE:
+		rule = allocate_acl_ipv4_tcp_5tuple_rule(cls);
+		if (!rule)
+			return NULL;
+		rule->tbl_type = table_type;
+		cls->table_mask |= table_type;
+		break;
 	default:
 		return NULL;
 	}
diff --git a/lib/flow_classify/rte_flow_classify.h b/lib/flow_classify/rte_flow_classify.h
index 82ea92b6a6..65585d9f92 100644
--- a/lib/flow_classify/rte_flow_classify.h
+++ b/lib/flow_classify/rte_flow_classify.h
@@ -80,6 +80,8 @@ enum rte_flow_classify_rule_type {
 	RTE_FLOW_CLASSIFY_RULE_TYPE_NONE,
 	/** IPv4 5tuple type */
 	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_5TUPLE,
+	/** IPv4 TCP 5tuple type */
+	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE,
 };
 
 /** Flow classify table type */
@@ -92,6 +94,8 @@ enum rte_flow_classify_table_type {
 	RTE_FLOW_CLASSIFY_TABLE_ACL_VLAN_IP4_5TUPLE = 1 << 2,
 	/** ACL QinQ IP4 5TUPLE */
 	RTE_FLOW_CLASSIFY_TABLE_ACL_QINQ_IP4_5TUPLE = 1 << 3,
+	/** ACL IP4 5TUPLE with tcp_flags */
+	RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE = 1 << 4,
 
 };
 
@@ -131,6 +135,21 @@ struct rte_flow_classify_ipv4_5tuple {
 	uint8_t proto_mask;      /**< Mask of L4 protocol. */
 };
 
+/** IPv4 5-tuple data with tcp flags*/
+struct rte_flow_classify_ipv4_tcp_5tuple {
+	uint32_t dst_ip;         /**< Destination IP address in big endian. */
+	uint32_t dst_ip_mask;    /**< Mask of destination IP address. */
+	uint32_t src_ip;         /**< Source IP address in big endian. */
+	uint32_t src_ip_mask;    /**< Mask of destination IP address. */
+	uint16_t dst_port;       /**< Destination port in big endian. */
+	uint16_t dst_port_mask;  /**< Mask of destination port. */
+	uint16_t src_port;       /**< Source Port in big endian. */
+	uint16_t src_port_mask;  /**< Mask of source port. */
+	uint8_t proto;           /**< L4 protocol. */
+	uint8_t proto_mask;      /**< Mask of L4 protocol. */
+	uint8_t tcp_flags;       /**< Tcp only */
+};
+
 /**
  * Flow stats
  *
diff --git a/lib/flow_classify/rte_flow_classify_parse.c b/lib/flow_classify/rte_flow_classify_parse.c
index 465330291f..fe4ee05b6f 100644
--- a/lib/flow_classify/rte_flow_classify_parse.c
+++ b/lib/flow_classify/rte_flow_classify_parse.c
@@ -216,6 +216,7 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 	const struct rte_flow_action_count *count;
 	const struct rte_flow_action_mark *mark_spec;
 	uint32_t index;
+	bool have_tcp = false;
 
 	/* parse pattern */
 	index = 0;
@@ -375,6 +376,8 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		filter->dst_port  = tcp_spec->hdr.dst_port;
 		filter->src_port  = tcp_spec->hdr.src_port;
 		filter->tcp_flags = tcp_spec->hdr.tcp_flags;
+		if (filter->tcp_flags != 0)
+			have_tcp = true;
 	} else if (item->type == RTE_FLOW_ITEM_TYPE_UDP) {
 		udp_mask = item->mask;
 
@@ -434,7 +437,10 @@ classify_parse_ntuple_filter(const struct rte_flow_attr *attr,
 		return -EINVAL;
 	}
 
-	table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
+	if (have_tcp)
+		table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
+	else
+		table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
 
 	/* parse attr */
 	/* must be input direction */
-- 
2.17.1


^ permalink raw reply	[relevance 10%]

* Re: [dpdk-dev] [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp
  2021-08-18 15:01 10% ` [dpdk-dev] [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp Sowmini Varadhan
@ 2021-08-19 15:06  6%   ` Iremonger, Bernard
  2021-08-19 15:20 13%     ` Sowmini Varadhan
  0 siblings, 1 reply; 38+ results
From: Iremonger, Bernard @ 2021-08-19 15:06 UTC (permalink / raw)
  To: Sowmini Varadhan, sowmini05, dev; +Cc: thomas

Hi Sowmini,

> -----Original Message-----
> From: Sowmini Varadhan <sovaradh@linux.microsoft.com>
> Sent: Wednesday, August 18, 2021 4:01 PM
> To: sowmini05@gmail.com; Iremonger, Bernard
> <bernard.iremonger@intel.com>; dev@dpdk.org;
> sovaradh@linux.microsoft.com
> Cc: thomas@monjalon.net
> Subject: [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp
> 
> v2 updates: typo fixes for checkpatch,  bernard.iremonger comments

The above line should not be added to the commit message.
~/dpdk/devtools/check-git-log.sh -1
Wrong tag:
        v2 fixes:  typo fixes, get_tcp_flags returns -EINVAL

The check-git-log.sh  script is in your dpdk checkout, best to run this script before sending patches

> 
> The struct rte_flow_classifier can have up to
> RTE_FLOW_CLASSIFY_TABLE_MAX
> (32) classifier tables, but the existing flow_classify examples only adds a
> single table for the L4 5-tuple.
> 
> When dealing with tcp flows, we frequently want to add ACLs and filters to
> filter based on the state of the TCP connection, e.g., by looking at the tcp
> flags field.
> 
> So we enhance flow_classify to add an additional acl table for tcp 5-tuples. If
> the input-file-parser detects a filter for a tcp flow with a non-wildcard
> argument for tcp_flags, the IP4_TCP_5TUPLE table is used by flow_classify.
> 
> Signed-off-by: Sowmini Varadhan <sovaradh@linux.microsoft.com>
> ---

If you want comment on the patch updates (optional), it can be added here after the --- line

>  examples/flow_classify/flow_classify.c      | 41 +++++++---
>  lib/flow_classify/rte_flow_classify.c       | 87 +++++++++++++++++++++
>  lib/flow_classify/rte_flow_classify.h       | 19 +++++
>  lib/flow_classify/rte_flow_classify_parse.c |  8 +-
>  4 files changed, 142 insertions(+), 13 deletions(-)

This patch contains changes to the flow_classify example and the flow_classify  library.
It needs to be split in two, a patch for the flow_classify example and a patch for the flow classify library.

> 
> diff --git a/examples/flow_classify/flow_classify.c
> b/examples/flow_classify/flow_classify.c
> index ff4c8bc2f5..8f6aacae03 100644
> --- a/examples/flow_classify/flow_classify.c
> +++ b/examples/flow_classify/flow_classify.c
> @@ -723,11 +723,6 @@ add_classify_rule(struct rte_eth_ntuple_filter
> *ntuple_filter,
>  		return ret;
>  	}
> 
> -	/* XXX but this only adds table_type of
> -	 * RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
> -	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule()
> -	 * so the tcp_flags is ignored!
> -	 */
>  	rule = rte_flow_classify_table_entry_add(
>  			cls_app->cls, &attr, pattern_ipv4_5tuple,
>  			actions, &key_found, &error);
> @@ -856,7 +851,8 @@ main(int argc, char *argv[])
>  	int ret;
>  	int socket_id;
>  	struct rte_table_acl_params table_acl_params;
> -	struct rte_flow_classify_table_params cls_table_params;
> +	struct rte_table_acl_params table_acl_tcp_params;
> +	struct rte_flow_classify_table_params cls_table_params[2];
>  	struct flow_classifier *cls_app;
>  	struct rte_flow_classifier_params cls_params;
>  	uint32_t size;
> @@ -923,21 +919,42 @@ main(int argc, char *argv[])
>  	memcpy(table_acl_params.field_format, ipv4_defs,
> sizeof(ipv4_defs));
> 
>  	/* initialise table create params */
> -	cls_table_params.ops = &rte_table_acl_ops;
> -	cls_table_params.arg_create = &table_acl_params;
> -	cls_table_params.type =
> RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
> +	cls_table_params[0].ops = &rte_table_acl_ops;
> +	cls_table_params[0].arg_create = &table_acl_params;
> +	cls_table_params[0].type =
> RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
> +
> +	/* initialise ACL table params */
> +	table_acl_tcp_params.name = "table_acl_ipv4_tcp_5tuple";
> +	table_acl_tcp_params.n_rules = FLOW_CLASSIFY_MAX_RULE_NUM;
> +	table_acl_tcp_params.n_rule_fields = RTE_DIM(ipv4_defs);
> +	memcpy(table_acl_tcp_params.field_format, ipv4_defs,
> +sizeof(ipv4_defs));
> +
> +	/* initialise table create params */
> +	cls_table_params[1].ops = &rte_table_acl_ops;
> +	cls_table_params[1].arg_create = &table_acl_tcp_params;
> +	cls_table_params[1].type =
> RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
> 
> -	ret = rte_flow_classify_table_create(cls_app->cls,
> &cls_table_params);
> +	ret = rte_flow_classify_table_create(cls_app->cls,
> +					     &cls_table_params[0]);
>  	if (ret) {
>  		rte_flow_classifier_free(cls_app->cls);
>  		rte_free(cls_app);
>  		rte_exit(EXIT_FAILURE, "Failed to create classifier table\n");
>  	}
> +	ret = rte_flow_classify_table_create(cls_app->cls,
> +					     &cls_table_params[1]);
> +	if (ret) {
> +		rte_flow_classifier_free(cls_app->cls);
> +		rte_free(cls_app);
> +		rte_exit(EXIT_FAILURE,
> +			 "Failed to create classifier table\n");
> +	}
> +
>  	/* >8 End of initialization of table create params. */
> 
>  	/* read file of IPv4 5 tuple rules and initialize parameters
> -	 * for rte_flow_classify_validate and
> rte_flow_classify_table_entry_add
> -	 * API's.
> +	 * for rte_flow_classify_validate and
> +	 * rte_flow_classify_table_entry_add  API's.
>  	 */
> 
>  	/* Read file of IPv4 tuple rules. 8< */ diff --git
> a/lib/flow_classify/rte_flow_classify.c b/lib/flow_classify/rte_flow_classify.c
> index d3ba2ed227..e960c3b140 100644
> --- a/lib/flow_classify/rte_flow_classify.c
> +++ b/lib/flow_classify/rte_flow_classify.c
> @@ -60,6 +60,7 @@ enum {
>  	DST_FIELD_IPV4,
>  	SRCP_FIELD_IPV4,
>  	DSTP_FIELD_IPV4,
> +	TCP_FLAGS_FIELD,

I think it would be better to rename TCP_FLAGS_FIELD to FLAGS_FIELD_TCP in line with the other values.

>  	NUM_FIELDS_IPV4
>  };
> 
> @@ -72,6 +73,7 @@ struct classify_rules {
>  	enum rte_flow_classify_rule_type type;
>  	union {
>  		struct rte_flow_classify_ipv4_5tuple ipv4_5tuple;
> +		struct rte_flow_classify_ipv4_tcp_5tuple ipv4_tcp_5tuple;
>  	} u;
>  };
> 
> @@ -477,6 +479,84 @@ allocate_acl_ipv4_5tuple_rule(struct
> rte_flow_classifier *cls)
>  	return rule;
>  }
> 
> +static struct rte_flow_classify_rule *
> +allocate_acl_ipv4_tcp_5tuple_rule(struct rte_flow_classifier *cls) {
> +	struct rte_flow_classify_rule *rule;
> +	int log_level;
> +
> +	rule = malloc(sizeof(struct rte_flow_classify_rule));
> +	if (!rule)
> +		return rule;
> +
> +	memset(rule, 0, sizeof(struct rte_flow_classify_rule));
> +	rule->id = unique_id++;
> +	rule->rules.type =
> RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE;
> +
> +	/* key add values */
> +	rule->u.key.key_add.priority = cls->ntuple_filter.priority;
> +	rule-
> >u.key.key_add.field_value[PROTO_FIELD_IPV4].mask_range.u8 =
> +			cls->ntuple_filter.proto_mask;
> +	rule->u.key.key_add.field_value[PROTO_FIELD_IPV4].value.u8 =
> +			cls->ntuple_filter.proto;
> +	rule->rules.u.ipv4_tcp_5tuple.proto = cls->ntuple_filter.proto;
> +	rule->rules.u.ipv4_tcp_5tuple.proto_mask =
> +			cls->ntuple_filter.proto_mask;
> +
> +	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].mask_range.u32
> =
> +			cls->ntuple_filter.src_ip_mask;
> +	rule->u.key.key_add.field_value[SRC_FIELD_IPV4].value.u32 =
> +			cls->ntuple_filter.src_ip;
> +	rule->rules.u.ipv4_tcp_5tuple.src_ip_mask =
> +			cls->ntuple_filter.src_ip_mask;
> +	rule->rules.u.ipv4_tcp_5tuple.src_ip = cls->ntuple_filter.src_ip;
> +
> +	rule->u.key.key_add.field_value[DST_FIELD_IPV4].mask_range.u32
> =
> +			cls->ntuple_filter.dst_ip_mask;
> +	rule->u.key.key_add.field_value[DST_FIELD_IPV4].value.u32 =
> +			cls->ntuple_filter.dst_ip;
> +	rule->rules.u.ipv4_tcp_5tuple.dst_ip_mask =
> +			cls->ntuple_filter.dst_ip_mask;
> +	rule->rules.u.ipv4_tcp_5tuple.dst_ip = cls->ntuple_filter.dst_ip;
> +
> +	rule-
> >u.key.key_add.field_value[SRCP_FIELD_IPV4].mask_range.u16 =
> +			cls->ntuple_filter.src_port_mask;
> +	rule->u.key.key_add.field_value[SRCP_FIELD_IPV4].value.u16 =
> +			cls->ntuple_filter.src_port;
> +	rule->rules.u.ipv4_tcp_5tuple.src_port_mask =
> +			cls->ntuple_filter.src_port_mask;
> +	rule->rules.u.ipv4_tcp_5tuple.src_port = cls->ntuple_filter.src_port;
> +
> +	rule-
> >u.key.key_add.field_value[DSTP_FIELD_IPV4].mask_range.u16 =
> +			cls->ntuple_filter.dst_port_mask;
> +	rule->u.key.key_add.field_value[DSTP_FIELD_IPV4].value.u16 =
> +			cls->ntuple_filter.dst_port;
> +	rule->rules.u.ipv4_tcp_5tuple.dst_port_mask =
> +			cls->ntuple_filter.dst_port_mask;
> +	rule->rules.u.ipv4_tcp_5tuple.dst_port = cls->ntuple_filter.dst_port;
> +
> +	rule-
> >u.key.key_add.field_value[TCP_FLAGS_FIELD].mask_range.u32 =
> +			rte_be_to_cpu_32(0xff);
> +	rule->u.key.key_add.field_value[TCP_FLAGS_FIELD].value.u32 =
> +			rte_be_to_cpu_32(cls->ntuple_filter.tcp_flags);
> +	rule->rules.u.ipv4_tcp_5tuple.tcp_flags =
> +cls->ntuple_filter.tcp_flags;
> +
> +	log_level = rte_log_get_level(librte_flow_classify_logtype);
> +
> +	if (log_level == RTE_LOG_DEBUG)
> +		print_acl_ipv4_key_add(&rule->u.key.key_add);
> +
> +	/* key delete values */
> +	memcpy(&rule->u.key.key_del.field_value[PROTO_FIELD_IPV4],
> +	       &rule->u.key.key_add.field_value[PROTO_FIELD_IPV4],
> +	       NUM_FIELDS_IPV4 * sizeof(struct rte_acl_field));
> +
> +	if (log_level == RTE_LOG_DEBUG)
> +		print_acl_ipv4_key_delete(&rule->u.key.key_del);
> +
> +	return rule;
> +}
> +
>  struct rte_flow_classify_rule *
>  rte_flow_classify_table_entry_add(struct rte_flow_classifier *cls,
>  		const struct rte_flow_attr *attr,
> @@ -514,6 +594,13 @@ rte_flow_classify_table_entry_add(struct
> rte_flow_classifier *cls,
>  		rule->tbl_type = table_type;
>  		cls->table_mask |= table_type;
>  		break;
> +	case RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE:
> +		rule = allocate_acl_ipv4_tcp_5tuple_rule(cls);
> +		if (!rule)
> +			return NULL;
> +		rule->tbl_type = table_type;
> +		cls->table_mask |= table_type;
> +		break;
>  	default:
>  		return NULL;
>  	}
> diff --git a/lib/flow_classify/rte_flow_classify.h
> b/lib/flow_classify/rte_flow_classify.h
> index 82ea92b6a6..65585d9f92 100644
> --- a/lib/flow_classify/rte_flow_classify.h
> +++ b/lib/flow_classify/rte_flow_classify.h
> @@ -80,6 +80,8 @@ enum rte_flow_classify_rule_type {
>  	RTE_FLOW_CLASSIFY_RULE_TYPE_NONE,
>  	/** IPv4 5tuple type */
>  	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_5TUPLE,
> +	/** IPv4 TCP 5tuple type */
> +	RTE_FLOW_CLASSIFY_RULE_TYPE_IPV4_TCP_5TUPLE,
>  };
> 
>  /** Flow classify table type */
> @@ -92,6 +94,8 @@ enum rte_flow_classify_table_type {
>  	RTE_FLOW_CLASSIFY_TABLE_ACL_VLAN_IP4_5TUPLE = 1 << 2,
>  	/** ACL QinQ IP4 5TUPLE */
>  	RTE_FLOW_CLASSIFY_TABLE_ACL_QINQ_IP4_5TUPLE = 1 << 3,
> +	/** ACL IP4 5TUPLE with tcp_flags */
> +	RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE = 1 << 4,
> 
>  };
> 
> @@ -131,6 +135,21 @@ struct rte_flow_classify_ipv4_5tuple {
>  	uint8_t proto_mask;      /**< Mask of L4 protocol. */
>  };
> 
> +/** IPv4 5-tuple data with tcp flags*/
> +struct rte_flow_classify_ipv4_tcp_5tuple {
> +	uint32_t dst_ip;         /**< Destination IP address in big endian. */
> +	uint32_t dst_ip_mask;    /**< Mask of destination IP address. */
> +	uint32_t src_ip;         /**< Source IP address in big endian. */
> +	uint32_t src_ip_mask;    /**< Mask of destination IP address. */
> +	uint16_t dst_port;       /**< Destination port in big endian. */
> +	uint16_t dst_port_mask;  /**< Mask of destination port. */
> +	uint16_t src_port;       /**< Source Port in big endian. */
> +	uint16_t src_port_mask;  /**< Mask of source port. */
> +	uint8_t proto;           /**< L4 protocol. */
> +	uint8_t proto_mask;      /**< Mask of L4 protocol. */
> +	uint8_t tcp_flags;       /**< Tcp only */
> +};
> +
>  /**
>   * Flow stats
>   *
> diff --git a/lib/flow_classify/rte_flow_classify_parse.c
> b/lib/flow_classify/rte_flow_classify_parse.c
> index 465330291f..fe4ee05b6f 100644
> --- a/lib/flow_classify/rte_flow_classify_parse.c
> +++ b/lib/flow_classify/rte_flow_classify_parse.c
> @@ -216,6 +216,7 @@ classify_parse_ntuple_filter(const struct
> rte_flow_attr *attr,
>  	const struct rte_flow_action_count *count;
>  	const struct rte_flow_action_mark *mark_spec;
>  	uint32_t index;
> +	bool have_tcp = false;
> 
>  	/* parse pattern */
>  	index = 0;
> @@ -375,6 +376,8 @@ classify_parse_ntuple_filter(const struct
> rte_flow_attr *attr,
>  		filter->dst_port  = tcp_spec->hdr.dst_port;
>  		filter->src_port  = tcp_spec->hdr.src_port;
>  		filter->tcp_flags = tcp_spec->hdr.tcp_flags;
> +		if (filter->tcp_flags != 0)
> +			have_tcp = true;
>  	} else if (item->type == RTE_FLOW_ITEM_TYPE_UDP) {
>  		udp_mask = item->mask;
> 
> @@ -434,7 +437,10 @@ classify_parse_ntuple_filter(const struct
> rte_flow_attr *attr,
>  		return -EINVAL;
>  	}
> 
> -	table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
> +	if (have_tcp)
> +		table_type =
> RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_TCP_5TUPLE;
> +	else
> +		table_type = RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE;
> 
>  	/* parse attr */
>  	/* must be input direction */
> --
> 2.17.1

Regards,

Bernard.

^ permalink raw reply	[relevance 6%]

* Re: [dpdk-dev] [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp
  2021-08-19 15:06  6%   ` Iremonger, Bernard
@ 2021-08-19 15:20 13%     ` Sowmini Varadhan
  2021-08-19 16:21  6%       ` Iremonger, Bernard
  0 siblings, 1 reply; 38+ results
From: Sowmini Varadhan @ 2021-08-19 15:20 UTC (permalink / raw)
  To: Iremonger, Bernard; +Cc: sowmini05, dev, thomas

On (08/19/21 15:06), Iremonger, Bernard wrote:
> > v2 updates: typo fixes for checkpatch,  bernard.iremonger comments
> 
> The above line should not be added to the commit message.
> ~/dpdk/devtools/check-git-log.sh -1
> Wrong tag:
>         v2 fixes:  typo fixes, get_tcp_flags returns -EINVAL
> 
> The check-git-log.sh  script is in your dpdk checkout, best to run this script before sending patches

apologies, I was not aware of this, I will fix and send out v3. 
I did run check-git-log.sh but before editing the vX updates.

thanks for your patience.

--Sowmini

^ permalink raw reply	[relevance 13%]

* Re: [dpdk-dev] [PATCH v2 1/2] examples/flow_classify: hooks for filters on tcp flags
  2021-08-18 15:01 11% ` [dpdk-dev] [PATCH v2 1/2] examples/flow_classify: hooks for filters on tcp flags Sowmini Varadhan
@ 2021-08-19 16:08  6%   ` Iremonger, Bernard
  0 siblings, 0 replies; 38+ results
From: Iremonger, Bernard @ 2021-08-19 16:08 UTC (permalink / raw)
  To: Sowmini Varadhan, sowmini05, dev; +Cc: thomas

HI Sowmini,

> -----Original Message-----
> From: Sowmini Varadhan <sovaradh@linux.microsoft.com>
> Sent: Wednesday, August 18, 2021 4:01 PM
> To: sowmini05@gmail.com; Iremonger, Bernard
> <bernard.iremonger@intel.com>; dev@dpdk.org;
> sovaradh@linux.microsoft.com
> Cc: thomas@monjalon.net
> Subject: [PATCH v2 1/2] examples/flow_classify: hooks for filters on tcp flags
> 
> v2 fixes:  typo fixes, get_tcp_flags returns -EINVAL

The above line should not be added to the commit message.
~/dpdk/devtools/check-git-log.sh -1
Wrong tag:
        v2 fixes:  typo fixes, get_tcp_flags returns -EINVAL

The check-git-log.sh  script is in your dpdk checkout, best to run this script before sending patches

> 
> The rte_eth_ntuple_filter allows tcp_flags which can check for things like
>     #define RTE_TCP_CWR_FLAG 0x80 /**< Congestion Window Reduced */
>     #define RTE_TCP_ECE_FLAG 0x40 /**< ECN-Echo */
>     #define RTE_TCP_URG_FLAG 0x20 /**< Urgent Pointer field significant */
>     #define RTE_TCP_ACK_FLAG 0x10 /**< Acknowledgment field significant
> */
>     #define RTE_TCP_PSH_FLAG 0x08 /**< Push Function */
>     #define RTE_TCP_RST_FLAG 0x04 /**< Reset the connection */
>     #define RTE_TCP_SYN_FLAG 0x02 /**< Synchronize sequence numbers */
>     #define RTE_TCP_FIN_FLAG 0x01 /**< No more data from sender */ but
> there are no existing examples that demonstrate how to use this feature.
> 
> This patch extends the existing classification support to allow an optional
> flags in the input file. The flags string can be any concatenation of characters
> from {C, E, U, A, P, R, S, F} and "*" indicates "don't care". These flags are set
> in the ntuple_filter and are used to construct the tcp_spec and tcp_mask
> sent to the driver
> 
> The rte_acl_field_def is updated to use the (u8) tcp flag as lookup key.
> Note that, as per
>   https://doc.dpdk.org/guides/prog_guide/packet_classif_access_ctrl.html
> this field MUST be allocated for 4 bytes, thus it has sizeof(uint32_t).
> 
> However, also note the XXX in this commit: additional updates are needed to
> the rte_flow_classify_table_entry_add() so that it does not ignore any key
> fields other than the 5-tuple.
> 
> Signed-off-by: Sowmini Varadhan <sovaradh@linux.microsoft.com>
> ---

If you want comment on the patch updates (optional), it can be added here after the --- line

>  examples/flow_classify/flow_classify.c     | 87 ++++++++++++++++++++--
>  examples/flow_classify/ipv4_rules_file.txt | 22 +++---
>  2 files changed, 91 insertions(+), 18 deletions(-)
> 
> diff --git a/examples/flow_classify/flow_classify.c
> b/examples/flow_classify/flow_classify.c
> index db71f5aa04..ff4c8bc2f5 100644
> --- a/examples/flow_classify/flow_classify.c
> +++ b/examples/flow_classify/flow_classify.c
> @@ -51,6 +51,7 @@ enum {
>  	CB_FLD_DST_PORT_MASK,
>  	CB_FLD_PROTO,
>  	CB_FLD_PRIORITY,
> +	CB_FLD_TCP_FLAGS,
>  	CB_FLD_NUM,
>  };
> 
> @@ -86,6 +87,7 @@ enum {
>  	DST_FIELD_IPV4,
>  	SRCP_FIELD_IPV4,
>  	DSTP_FIELD_IPV4,
> +	TCP_FLAGS_FIELD,

I think it would be better to rename TCP_FLAGS_FIELD to TCP_FLAGS_FIELD_IPV4  inline with the other values.

>  	NUM_FIELDS_IPV4
>  };
> 
> @@ -93,7 +95,8 @@ enum {
>  	PROTO_INPUT_IPV4,
>  	SRC_INPUT_IPV4,
>  	DST_INPUT_IPV4,
> -	SRCP_DESTP_INPUT_IPV4
> +	SRCP_DESTP_INPUT_IPV4,
> +	TCP_FLAGS_INDEX,

I think it would be better to rename  TCP_FLAGS_INDEX to TCP_FLAGS_INPUT_IPV4 inline with the other values.

>  };
> 
>  static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = { @@ -150,6
> +153,17 @@ static struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
>  			sizeof(struct rte_ipv4_hdr) +
>  			offsetof(struct rte_tcp_hdr, dst_port),
>  	},
> +	/* next field must be 4 bytes, even though flags is only 1 byte */
> +	{
> +		/* rte_flags */
> +		.type = RTE_ACL_FIELD_TYPE_BITMASK,
> +		.size = sizeof(uint32_t),
> +		.field_index = TCP_FLAGS_FIELD,
> +		.input_index = TCP_FLAGS_INDEX,
> +		.offset = sizeof(struct rte_ether_hdr) +
> +			sizeof(struct rte_ipv4_hdr) +
> +			offsetof(struct rte_tcp_hdr, tcp_flags),
> +	},
>  };
>  /* >8 End of creation of ACL table. */
> 
> @@ -285,12 +299,14 @@ lcore_main(struct flow_classifier *cls_app)
>  	int ret;
>  	int i = 0;
> 
> -	ret = rte_flow_classify_table_entry_delete(cls_app->cls,
> -			rules[7]);
> -	if (ret)
> -		printf("table_entry_delete failed [7] %d\n\n", ret);
> -	else
> -		printf("table_entry_delete succeeded [7]\n\n");
> +	if (rules[7]) {
> +		ret = rte_flow_classify_table_entry_delete(cls_app->cls,
> +				rules[7]);
> +		if (ret)
> +			printf("table_entry_delete failed [7] %d\n\n", ret);
> +		else
> +			printf("table_entry_delete succeeded [7]\n\n");
> +	}
> 
>  	/*
>  	 * Check that the port is on the same NUMA node as the polling
> thread @@ -410,6 +426,53 @@ parse_ipv4_net(char *in, uint32_t *addr,
> uint32_t *mask_len)
>  	return 0;
>  }
> 
> +static int
> +get_tcp_flags(char *in, struct rte_eth_ntuple_filter *ntuple_filter) {
> +	int len = strlen(in);
> +	int i;
> +	uint8_t flags = 0;
> +
> +	if (strcmp(in, "*") == 0) {
> +		ntuple_filter->tcp_flags = 0;
> +		return 0;
> +	}
> +
> +	for (i = 0; i < len; i++) {
> +		switch (in[i]) {
> +		case 'S':
> +			flags |= RTE_TCP_SYN_FLAG;
> +			break;
> +		case 'F':
> +			flags |= RTE_TCP_FIN_FLAG;
> +			break;
> +		case 'R':
> +			flags |= RTE_TCP_RST_FLAG;
> +			break;
> +		case 'P':
> +			flags |= RTE_TCP_PSH_FLAG;
> +			break;
> +		case 'A':
> +			flags |= RTE_TCP_ACK_FLAG;
> +			break;
> +		case 'U':
> +			flags |= RTE_TCP_URG_FLAG;
> +			break;
> +		case 'E':
> +			flags |= RTE_TCP_ECE_FLAG;
> +			break;
> +		case 'C':
> +			flags |= RTE_TCP_CWR_FLAG;
> +			break;
> +		default:
> +			fprintf(stderr, "unknown flag %c\n", in[i]);
> +			return -EINVAL;
> +		}
> +	}
> +	ntuple_filter->tcp_flags = flags;
> +	return 0;
> +}
> +
>  static int
>  parse_ipv4_5tuple_rule(char *str, struct rte_eth_ntuple_filter
> *ntuple_filter)  { @@ -481,6 +544,8 @@ parse_ipv4_5tuple_rule(char *str,
> struct rte_eth_ntuple_filter *ntuple_filter)
>  	ntuple_filter->priority = (uint16_t)temp;
>  	if (ntuple_filter->priority > FLOW_CLASSIFY_MAX_PRIORITY)
>  		ret = -EINVAL;
> +	if (get_tcp_flags(in[CB_FLD_TCP_FLAGS], ntuple_filter))
> +		return -EINVAL;
> 
>  	return ret;
>  }
> @@ -597,10 +662,13 @@ add_classify_rule(struct rte_eth_ntuple_filter
> *ntuple_filter,
>  		memset(&tcp_spec, 0, sizeof(tcp_spec));
>  		tcp_spec.hdr.src_port = ntuple_filter->src_port;
>  		tcp_spec.hdr.dst_port = ntuple_filter->dst_port;
> +		tcp_spec.hdr.tcp_flags = ntuple_filter->tcp_flags;
> 
>  		memset(&tcp_mask, 0, sizeof(tcp_mask));
>  		tcp_mask.hdr.src_port = ntuple_filter->src_port_mask;
>  		tcp_mask.hdr.dst_port = ntuple_filter->dst_port_mask;
> +		if (tcp_spec.hdr.tcp_flags != 0)
> +			tcp_mask.hdr.tcp_flags = 0xff;
> 
>  		tcp_item.type = RTE_FLOW_ITEM_TYPE_TCP;
>  		tcp_item.spec = &tcp_spec;
> @@ -655,6 +723,11 @@ add_classify_rule(struct rte_eth_ntuple_filter
> *ntuple_filter,
>  		return ret;
>  	}
> 
> +	/* XXX but this only adds table_type of
> +	 * RTE_FLOW_CLASSIFY_TABLE_ACL_IP4_5TUPLE
> +	 * i.e., it only ever does allocate_acl_ipv4_5tuple_rule()
> +	 * so the tcp_flags is ignored!
> +	 */
>  	rule = rte_flow_classify_table_entry_add(
>  			cls_app->cls, &attr, pattern_ipv4_5tuple,
>  			actions, &key_found, &error);
> diff --git a/examples/flow_classify/ipv4_rules_file.txt
> b/examples/flow_classify/ipv4_rules_file.txt
> index dfa0631fcc..415573732a 100644
> --- a/examples/flow_classify/ipv4_rules_file.txt
> +++ b/examples/flow_classify/ipv4_rules_file.txt
> @@ -1,14 +1,14 @@
>  #file format:
> -#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask
> proto/mask priority
> +#src_ip/masklen dst_ip/masklen src_port : mask dst_port : mask
> +proto/mask priority tcp_flags
>  #
> -2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0
> -9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1
> -9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2
> -9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3
> -6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4
> -6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5
> -6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6
> -6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7
> -6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8
> +2.2.2.3/24 2.2.2.7/24 32 : 0xffff 33 : 0xffff 17/0xff 0 *
> +9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 17/0xff 1 *
> +9.9.9.3/24 9.9.9.7/24 32 : 0xffff 33 : 0xffff 6/0xff 2 *
> +9.9.8.3/24 9.9.8.7/24 32 : 0xffff 33 : 0xffff 6/0xff 3 *
> +6.7.8.9/24 2.3.4.5/24 32 : 0x0000 33 : 0x0000 132/0xff 4 *
> +6.7.8.9/32 192.168.0.36/32 10 : 0xffff 11 : 0xffff 6/0xfe 5 *
> +6.7.8.9/24 192.168.0.36/24 10 : 0xffff 11 : 0xffff 6/0xfe 6 *
> +6.7.8.9/16 192.168.0.36/16 10 : 0xffff 11 : 0xffff 6/0xfe 7 *
> +6.7.8.9/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 8 *
>  #error rules
> -#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9 \ No newline at end
> of file
> +#9.8.7.6/8 192.168.0.36/8 10 : 0xffff 11 : 0xffff 6/0xfe 9 *
> --
> 2.17.1

Regards,

Bernard.

^ permalink raw reply	[relevance 6%]

* Re: [dpdk-dev] [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp
  2021-08-19 15:20 13%     ` Sowmini Varadhan
@ 2021-08-19 16:21  6%       ` Iremonger, Bernard
  2021-08-19 19:34 15%         ` Sowmini Varadhan
  0 siblings, 1 reply; 38+ results
From: Iremonger, Bernard @ 2021-08-19 16:21 UTC (permalink / raw)
  To: Sowmini Varadhan; +Cc: sowmini05, dev, thomas

Hi Sowmini,

Looking closer at this patchset, I am not sure that a second ACL table is needed.
The existing ACL table handles UDP, TCP and SCP, however it is not processing the TCP flags.
I think it just needs to be modified to process the TCP flags.
Could you take another look to see if the above proposed solution will work for you.

Regards,

Bernard. 

> -----Original Message-----
> From: Sowmini Varadhan <sovaradh@linux.microsoft.com>
> Sent: Thursday, August 19, 2021 4:21 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>
> Cc: sowmini05@gmail.com; dev@dpdk.org; thomas@monjalon.net
> Subject: Re: [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp
> 
> On (08/19/21 15:06), Iremonger, Bernard wrote:
> > > v2 updates: typo fixes for checkpatch,  bernard.iremonger comments
> >
> > The above line should not be added to the commit message.
> > ~/dpdk/devtools/check-git-log.sh -1
> > Wrong tag:
> >         v2 fixes:  typo fixes, get_tcp_flags returns -EINVAL
> >
> > The check-git-log.sh  script is in your dpdk checkout, best to run this script
> before sending patches
> 
> apologies, I was not aware of this, I will fix and send out v3.
> I did run check-git-log.sh but before editing the vX updates.
> 
> thanks for your patience.
> 
> --Sowmini

^ permalink raw reply	[relevance 6%]

* Re: [dpdk-dev] [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp
  2021-08-19 16:21  6%       ` Iremonger, Bernard
@ 2021-08-19 19:34 15%         ` Sowmini Varadhan
  2023-07-03 23:38  6%           ` Stephen Hemminger
  0 siblings, 1 reply; 38+ results
From: Sowmini Varadhan @ 2021-08-19 19:34 UTC (permalink / raw)
  To: Iremonger, Bernard; +Cc: sowmini05, dev, thomas

On (08/19/21 16:21), Iremonger, Bernard wrote:
> 
> Looking closer at this patchset, I am not sure that a second ACL table is needed.
> The existing ACL table handles UDP, TCP and SCP, however it is not processing the TCP flags.
> I think it just needs to be modified to process the TCP flags.
> Could you take another look to see if the above proposed solution will work for you.

I'm not sure it would. As I pointed out in the original rfc  at
https://inbox.dpdk.org/dev/cover.1578936382.git.sowmini.varadhan@microsoft.com/

we need to add a key for the 8 bit flags, but the multibit
trie lookup moves in steps of 4 bytes.

However, it has admittedly been a while since I tinkereed with this,
and I can give it a shot and get back.

Thanks
--Sowmini


^ permalink raw reply	[relevance 15%]

* Re: [dpdk-dev] [PATCH RFC V2 0/2] TCP flow classification using 4-tuple and flags
  2020-01-13 18:05 21% [dpdk-dev] [PATCH RFC V2 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
  2020-01-13 18:05 20% ` [dpdk-dev] [PATCH RFC V2 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
  2020-01-13 18:05 18% ` [dpdk-dev] [PATCH RFC V2 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
@ 2023-06-12 21:14  6% ` Stephen Hemminger
  2 siblings, 0 replies; 38+ results
From: Stephen Hemminger @ 2023-06-12 21:14 UTC (permalink / raw)
  To: Sowmini Varadhan; +Cc: dev

On Mon, 13 Jan 2020 18:05:28 +0000
Sowmini Varadhan <sowmini05@gmail.com> wrote:

> V2 updates: checkpatch fixes, revert accidently spelling error
> introduced in V1;
> 
> The problem space of TCP flow tracking and classification
> based on TCP state requires the ability to classify TCP
> flows on more packet properties than just the 4-tuple,
> e.g., TCP flags. This patch-set investigates the set of
> changes needed in the examples/flow_classify.c needed to
> achieve this.
> 
> Patch 1 extends examples/flow_classify.c to allow constraints
> on tcp flags. Patch 2 extends the ACL handling in  
> librte_flow_classify to include keys on the properties in 
> addition to the tcp 4-tuple.
> 
> Note that one particular part of this patch-set where feedback
> is requested is in allocate_acl_ipv4_tcp_5tuple_rule():
> we need to add a key for the 8 bit flags, but the multibit
> trie lookup moves in steps of 4 bytes, so it took some hackery
> to figure out what byte-ordering was expected, and there were
> no documentation/examples to provide guidelines. Comments/suggestions
> would be particularly helpful.
> 
> Sowmini Varadhan (2):
>   Hooks to allow the setting of filters on tcp flags
>   Allow the flow_classify example to add an ACL table for tcp.
> 
>  examples/flow_classify/flow_classify.c        | 121 +++++++++++++++---
>  examples/flow_classify/ipv4_rules_file.txt    |  22 ++--
>  lib/librte_flow_classify/rte_flow_classify.c  |  87 +++++++++++++
>  lib/librte_flow_classify/rte_flow_classify.h  |  19 +++
>  .../rte_flow_classify_parse.c                 |   8 +-
>  5 files changed, 230 insertions(+), 27 deletions(-)
> 

Is anyone still interested in this patch?
It would need work for DPDK 23.08 or later code base.
For now, marking it as "Changes Requested"

^ permalink raw reply	[relevance 6%]

* Re: [dpdk-dev] [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp
  2021-08-19 19:34 15%         ` Sowmini Varadhan
@ 2023-07-03 23:38  6%           ` Stephen Hemminger
  0 siblings, 0 replies; 38+ results
From: Stephen Hemminger @ 2023-07-03 23:38 UTC (permalink / raw)
  To: Sowmini Varadhan; +Cc: Iremonger, Bernard, sowmini05, dev, thomas

On Thu, 19 Aug 2021 12:34:46 -0700
Sowmini Varadhan <sovaradh@linux.microsoft.com> wrote:

> On (08/19/21 16:21), Iremonger, Bernard wrote:
> > 
> > Looking closer at this patchset, I am not sure that a second ACL table is needed.
> > The existing ACL table handles UDP, TCP and SCP, however it is not processing the TCP flags.
> > I think it just needs to be modified to process the TCP flags.
> > Could you take another look to see if the above proposed solution will work for you.  
> 
> I'm not sure it would. As I pointed out in the original rfc  at
> https://inbox.dpdk.org/dev/cover.1578936382.git.sowmini.varadhan@microsoft.com/
> 
> we need to add a key for the 8 bit flags, but the multibit
> trie lookup moves in steps of 4 bytes.
> 
> However, it has admittedly been a while since I tinkereed with this,
> and I can give it a shot and get back.
> 
> Thanks
> --Sowmini
> 

Doing a cleanup of patch backlog prior to next LTS release.
The patch never got updated based on comments.
Please resubmit if you still want to pursue it.

^ permalink raw reply	[relevance 6%]

* Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
  @ 2023-11-02 18:37  0% ` Stephen Hemminger
  2023-11-07 17:33  0%   ` rahul gupta
  2023-11-08  4:38  0%   ` Rahul Gupta
  0 siblings, 2 replies; 38+ results
From: Stephen Hemminger @ 2023-11-02 18:37 UTC (permalink / raw)
  To: Rahul Gupta
  Cc: dev, thomas, sovaradh, okaya, sujithsankar, sowmini.varadhan,
	rahulrgupta27, Rahul Gupta

On Thu,  2 Nov 2023 11:19:24 -0700
Rahul Gupta <rahulgupt@linux.microsoft.com> wrote:

> From: Rahul Gupta <rahulgupt@linux.microsoft.com>
> To: dev@dpdk.org,  thomas@monjalon.net
> Cc: sovaradh@linux.microsoft.com, okaya@kernel.org, sujithsankar@microsoft.com,  sowmini.varadhan@microsoft.com, rahulrgupta27@gmail.com,  Rahul Gupta <rahulgupt@microsoft.com>,  Rahul Gupta <rahulgupt@linux.microsoft.com>
> Subject: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> Date: Thu,  2 Nov 2023 11:19:24 -0700
> X-Mailer: git-send-email 1.8.3.1
> 
> From: Rahul Gupta <rahulgupt@microsoft.com>
> 
> Initialization often requires rte_eal_init + rte_pktmbuf_pool_create
> which can consume a total time of 500-600 ms:
> a) For many devices FLR may take a significant chunk of time
>    (200-250 ms in our use-case), this FLR is triggered during device
>    probe in rte_eal_init().
> b) rte_pktmbuf_pool_create() can consume upto 300-350 ms for
> applications that require huge memory.
> 
> This cost is incurred on each restart (which happens in our use-case
> during binary updates for servicing).
> This patch provides an optimization using pthreads that appplications
> can use and which can save 200-230ms.
> 
> In this patch, rte_eal_init() is refactored into two parts-
> a) 1st part is dependent code ie- it’s a perquisite of the FLR and
>    mempool creation. So this code needs to be executed before any
>    pthreads. Its named as rte_eal_init_setup()
> b) 2nd part of code is independent code ie- it can execute in parallel
>    to mempool creation in a pthread. Its named as rte_probe_and_ioctl().
> 
> Existing applications require no changes unless they wish to leverage
> the optimization.
> 
> If the application wants to use pthread functionality, it should call-
> a) rte_eal_init_setup() then create two or more pthreads-
> b) in one pthread call- rte_probe_and_ioctl(),
> c) second pthread call- rte_pktmbuf_pool_create()
> d) (optional) Other pthreads for  any other independent function.
> 
> Signed-off-by: Rahul Gupta <rahulgupt@linux.microsoft.com>

These probably marked internal rather than part of API/ABI.

^ permalink raw reply	[relevance 0%]

* Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
  2023-11-02 18:37  0% ` Stephen Hemminger
  2023-11-07 17:33  0%   ` rahul gupta
@ 2023-11-08  4:38  0%   ` Rahul Gupta
  1 sibling, 0 replies; 38+ results
From: Rahul Gupta @ 2023-11-08  4:38 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, thomas, sovaradh, okaya, sujithsankar, sowmini.varadhan,
	rahulrgupta27, Rahul Gupta, Rahul Gupta

On (11/02/23 11:37), Stephen Hemminger wrote:
> Date: Thu, 2 Nov 2023 11:37:59 -0700
> From: Stephen Hemminger <stephen@networkplumber.org>
> To: Rahul Gupta <rahulgupt@linux.microsoft.com>
> Cc: dev@dpdk.org, thomas@monjalon.net, sovaradh@linux.microsoft.com,
>  okaya@kernel.org, sujithsankar@microsoft.com,
>  sowmini.varadhan@microsoft.com, rahulrgupta27@gmail.com, Rahul Gupta
>  <rahulgupt@microsoft.com>
> Subject: Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-redhat-linux-gnu)
> 
> On Thu,  2 Nov 2023 11:19:24 -0700
> Rahul Gupta <rahulgupt@linux.microsoft.com> wrote:
> 
> > From: Rahul Gupta <rahulgupt@linux.microsoft.com>
> > To: dev@dpdk.org,  thomas@monjalon.net
> > Cc: sovaradh@linux.microsoft.com, okaya@kernel.org, sujithsankar@microsoft.com,  sowmini.varadhan@microsoft.com, rahulrgupta27@gmail.com,  Rahul Gupta <rahulgupt@microsoft.com>,  Rahul Gupta <rahulgupt@linux.microsoft.com>
> > Subject: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> > Date: Thu,  2 Nov 2023 11:19:24 -0700
> > X-Mailer: git-send-email 1.8.3.1
> > 
> > From: Rahul Gupta <rahulgupt@microsoft.com>
> > 
> > Initialization often requires rte_eal_init + rte_pktmbuf_pool_create
> > which can consume a total time of 500-600 ms:
> > a) For many devices FLR may take a significant chunk of time
> >    (200-250 ms in our use-case), this FLR is triggered during device
> >    probe in rte_eal_init().
> > b) rte_pktmbuf_pool_create() can consume upto 300-350 ms for
> > applications that require huge memory.
> > 
> > This cost is incurred on each restart (which happens in our use-case
> > during binary updates for servicing).
> > This patch provides an optimization using pthreads that appplications
> > can use and which can save 200-230ms.
> > 
> > In this patch, rte_eal_init() is refactored into two parts-
> > a) 1st part is dependent code ie- it’s a perquisite of the FLR and
> >    mempool creation. So this code needs to be executed before any
> >    pthreads. Its named as rte_eal_init_setup()
> > b) 2nd part of code is independent code ie- it can execute in parallel
> >    to mempool creation in a pthread. Its named as rte_probe_and_ioctl().
> > 
> > Existing applications require no changes unless they wish to leverage
> > the optimization.
> > 
> > If the application wants to use pthread functionality, it should call-
> > a) rte_eal_init_setup() then create two or more pthreads-
> > b) in one pthread call- rte_probe_and_ioctl(),
> > c) second pthread call- rte_pktmbuf_pool_create()
> > d) (optional) Other pthreads for  any other independent function.
> > 
> > Signed-off-by: Rahul Gupta <rahulgupt@linux.microsoft.com>
> 
> These probably marked internal rather than part of API/ABI.

Hi Stephen,

Thanks for your review.
If I make it __rte_internal then, testpmd or our application can't use it.
So instead I am planning to make it __rte_experimental.

Regards,
Rahul.

^ permalink raw reply	[relevance 0%]

* Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
  2023-11-02 18:37  0% ` Stephen Hemminger
@ 2023-11-07 17:33  0%   ` rahul gupta
  2023-11-08 13:53  0%     ` Dmitry Kozlyuk
  2023-11-08  4:38  0%   ` Rahul Gupta
  1 sibling, 1 reply; 38+ results
From: rahul gupta @ 2023-11-07 17:33 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Rahul Gupta, dev, thomas, sovaradh, okaya, sujithsankar,
	sowmini.varadhan, Rahul Gupta

[-- Attachment #1: Type: text/plain, Size: 2558 bytes --]

Hi Stephen,

Thanks for your review.
If I make it __rte_internal then, testpmd or our application can't use it.
So instead I am planning to make it __rte_experimental.

Regards,
Rahul.

On Fri, 3 Nov 2023 at 00:08, Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Thu,  2 Nov 2023 11:19:24 -0700
> Rahul Gupta <rahulgupt@linux.microsoft.com> wrote:
>
> > From: Rahul Gupta <rahulgupt@linux.microsoft.com>
> > To: dev@dpdk.org,  thomas@monjalon.net
> > Cc: sovaradh@linux.microsoft.com, okaya@kernel.org,
> sujithsankar@microsoft.com,  sowmini.varadhan@microsoft.com,
> rahulrgupta27@gmail.com,  Rahul Gupta <rahulgupt@microsoft.com>,  Rahul
> Gupta <rahulgupt@linux.microsoft.com>
> > Subject: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> > Date: Thu,  2 Nov 2023 11:19:24 -0700
> > X-Mailer: git-send-email 1.8.3.1
> >
> > From: Rahul Gupta <rahulgupt@microsoft.com>
> >
> > Initialization often requires rte_eal_init + rte_pktmbuf_pool_create
> > which can consume a total time of 500-600 ms:
> > a) For many devices FLR may take a significant chunk of time
> >    (200-250 ms in our use-case), this FLR is triggered during device
> >    probe in rte_eal_init().
> > b) rte_pktmbuf_pool_create() can consume upto 300-350 ms for
> > applications that require huge memory.
> >
> > This cost is incurred on each restart (which happens in our use-case
> > during binary updates for servicing).
> > This patch provides an optimization using pthreads that appplications
> > can use and which can save 200-230ms.
> >
> > In this patch, rte_eal_init() is refactored into two parts-
> > a) 1st part is dependent code ie- it’s a perquisite of the FLR and
> >    mempool creation. So this code needs to be executed before any
> >    pthreads. Its named as rte_eal_init_setup()
> > b) 2nd part of code is independent code ie- it can execute in parallel
> >    to mempool creation in a pthread. Its named as rte_probe_and_ioctl().
> >
> > Existing applications require no changes unless they wish to leverage
> > the optimization.
> >
> > If the application wants to use pthread functionality, it should call-
> > a) rte_eal_init_setup() then create two or more pthreads-
> > b) in one pthread call- rte_probe_and_ioctl(),
> > c) second pthread call- rte_pktmbuf_pool_create()
> > d) (optional) Other pthreads for  any other independent function.
> >
> > Signed-off-by: Rahul Gupta <rahulgupt@linux.microsoft.com>
>
> These probably marked internal rather than part of API/ABI.
>

[-- Attachment #2: Type: text/html, Size: 4000 bytes --]

^ permalink raw reply	[relevance 0%]

* Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
  2023-11-07 17:33  0%   ` rahul gupta
@ 2023-11-08 13:53  0%     ` Dmitry Kozlyuk
  2023-11-08 15:40  0%       ` Thomas Monjalon
  0 siblings, 1 reply; 38+ results
From: Dmitry Kozlyuk @ 2023-11-08 13:53 UTC (permalink / raw)
  To: rahul gupta
  Cc: Stephen Hemminger, Rahul Gupta, dev, thomas, sovaradh, okaya,
	sujithsankar, sowmini.varadhan, Rahul Gupta

2023-11-07 23:03 (UTC+0530), rahul gupta:
> > > From: Rahul Gupta <rahulgupt@linux.microsoft.com>
> > > To: dev@dpdk.org,  thomas@monjalon.net
> > > Cc: sovaradh@linux.microsoft.com, okaya@kernel.org,  
> > sujithsankar@microsoft.com,  sowmini.varadhan@microsoft.com,
> > rahulrgupta27@gmail.com,  Rahul Gupta <rahulgupt@microsoft.com>,  Rahul
> > Gupta <rahulgupt@linux.microsoft.com>  
> > > Subject: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> > > Date: Thu,  2 Nov 2023 11:19:24 -0700
> > > X-Mailer: git-send-email 1.8.3.1
> > >
> > > From: Rahul Gupta <rahulgupt@microsoft.com>
> > >
> > > Initialization often requires rte_eal_init + rte_pktmbuf_pool_create
> > > which can consume a total time of 500-600 ms:
> > > a) For many devices FLR may take a significant chunk of time
> > >    (200-250 ms in our use-case), this FLR is triggered during device
> > >    probe in rte_eal_init().
> > > b) rte_pktmbuf_pool_create() can consume upto 300-350 ms for
> > > applications that require huge memory.
> > >
> > > This cost is incurred on each restart (which happens in our use-case
> > > during binary updates for servicing).
> > > This patch provides an optimization using pthreads that appplications
> > > can use and which can save 200-230ms.
> > >
> > > In this patch, rte_eal_init() is refactored into two parts-
> > > a) 1st part is dependent code ie- it’s a perquisite of the FLR and
> > >    mempool creation. So this code needs to be executed before any
> > >    pthreads. Its named as rte_eal_init_setup()
> > > b) 2nd part of code is independent code ie- it can execute in parallel
> > >    to mempool creation in a pthread. Its named as rte_probe_and_ioctl().
> > >
> > > Existing applications require no changes unless they wish to leverage
> > > the optimization.
> > >
> > > If the application wants to use pthread functionality, it should call-
> > > a) rte_eal_init_setup() then create two or more pthreads-
> > > b) in one pthread call- rte_probe_and_ioctl(),
> > > c) second pthread call- rte_pktmbuf_pool_create()
> > > d) (optional) Other pthreads for  any other independent function.
> > >
> > > Signed-off-by: Rahul Gupta <rahulgupt@linux.microsoft.com>  

I doubt that the new API is required.
It is already possible to block all devices from automatic probing
with EAL options and then probe explicitly in any threads desired.
At the same time, this RFC shows a valuable optimization pattern,
so maybe it is worth having in DPDK as an example.
There are DPDK use cases when probing is completely unnecessary.
Exposing the initialization process stages makes it harder to refactor
and requires precise documentation of when and what is initialized
(for example, in this RFC rte_eal_init_setup()
does not make service core API usable yet).

P. S. You may be also interested in using `--huge-unlink=never`
to speed rte_pktmbuf_pool_create() during restarts:

https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#id3

^ permalink raw reply	[relevance 0%]

* Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
  2023-11-08 13:53  0%     ` Dmitry Kozlyuk
@ 2023-11-08 15:40  0%       ` Thomas Monjalon
  2023-11-09 17:26  0%         ` Rahul Gupta
  0 siblings, 1 reply; 38+ results
From: Thomas Monjalon @ 2023-11-08 15:40 UTC (permalink / raw)
  To: rahul gupta, Dmitry Kozlyuk
  Cc: Stephen Hemminger, Rahul Gupta, dev, sovaradh, okaya,
	sujithsankar, sowmini.varadhan, Rahul Gupta

08/11/2023 14:53, Dmitry Kozlyuk:
> 2023-11-07 23:03 (UTC+0530), rahul gupta:
> > > > From: Rahul Gupta <rahulgupt@linux.microsoft.com>
> > > > To: dev@dpdk.org,  thomas@monjalon.net
> > > > Cc: sovaradh@linux.microsoft.com, okaya@kernel.org,  
> > > sujithsankar@microsoft.com,  sowmini.varadhan@microsoft.com,
> > > rahulrgupta27@gmail.com,  Rahul Gupta <rahulgupt@microsoft.com>,  Rahul
> > > Gupta <rahulgupt@linux.microsoft.com>  
> > > > Subject: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> > > > Date: Thu,  2 Nov 2023 11:19:24 -0700
> > > > X-Mailer: git-send-email 1.8.3.1
> > > >
> > > > From: Rahul Gupta <rahulgupt@microsoft.com>
> > > >
> > > > Initialization often requires rte_eal_init + rte_pktmbuf_pool_create
> > > > which can consume a total time of 500-600 ms:
> > > > a) For many devices FLR may take a significant chunk of time
> > > >    (200-250 ms in our use-case), this FLR is triggered during device
> > > >    probe in rte_eal_init().
> > > > b) rte_pktmbuf_pool_create() can consume upto 300-350 ms for
> > > > applications that require huge memory.
> > > >
> > > > This cost is incurred on each restart (which happens in our use-case
> > > > during binary updates for servicing).
> > > > This patch provides an optimization using pthreads that appplications
> > > > can use and which can save 200-230ms.
> > > >
> > > > In this patch, rte_eal_init() is refactored into two parts-
> > > > a) 1st part is dependent code ie- it’s a perquisite of the FLR and
> > > >    mempool creation. So this code needs to be executed before any
> > > >    pthreads. Its named as rte_eal_init_setup()
> > > > b) 2nd part of code is independent code ie- it can execute in parallel
> > > >    to mempool creation in a pthread. Its named as rte_probe_and_ioctl().
> > > >
> > > > Existing applications require no changes unless they wish to leverage
> > > > the optimization.
> > > >
> > > > If the application wants to use pthread functionality, it should call-
> > > > a) rte_eal_init_setup() then create two or more pthreads-
> > > > b) in one pthread call- rte_probe_and_ioctl(),
> > > > c) second pthread call- rte_pktmbuf_pool_create()
> > > > d) (optional) Other pthreads for  any other independent function.
> > > >
> > > > Signed-off-by: Rahul Gupta <rahulgupt@linux.microsoft.com>  
> 
> I doubt that the new API is required.
> It is already possible to block all devices from automatic probing
> with EAL options and then probe explicitly in any threads desired.
> At the same time, this RFC shows a valuable optimization pattern,
> so maybe it is worth having in DPDK as an example.
> There are DPDK use cases when probing is completely unnecessary.

It seems here we want to do the device probing,
but start it in parallel of other tasks.

> Exposing the initialization process stages makes it harder to refactor
> and requires precise documentation of when and what is initialized
> (for example, in this RFC rte_eal_init_setup()
> does not make service core API usable yet).

Yes the init order is sensitive, that's why we have a big init function.
But in general I would agree to try splitting it with necessary warnings
and explanations.

> P. S. You may be also interested in using `--huge-unlink=never`
> to speed rte_pktmbuf_pool_create() during restarts:
> 
> https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#id3

Yes good tip :)




^ permalink raw reply	[relevance 0%]

* Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
  2023-11-08 15:40  0%       ` Thomas Monjalon
@ 2023-11-09 17:26  0%         ` Rahul Gupta
  2023-11-09 17:32  0%           ` Bruce Richardson
  0 siblings, 1 reply; 38+ results
From: Rahul Gupta @ 2023-11-09 17:26 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: rahul gupta, Dmitry Kozlyuk, Stephen Hemminger, dev, sovaradh,
	okaya, sujithsankar, sowmini.varadhan

On (11/08/23 16:40), Thomas Monjalon wrote:
> Date: Wed, 08 Nov 2023 16:40:07 +0100
> From: Thomas Monjalon <thomas@monjalon.net>
> To: rahul gupta <rahulrgupta27@gmail.com>, Dmitry Kozlyuk
>  <dmitry.kozliuk@gmail.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>, Rahul Gupta
>  <rahulgupt@linux.microsoft.com>, dev@dpdk.org,
>  sovaradh@linux.microsoft.com, okaya@kernel.org,
>  sujithsankar@microsoft.com, sowmini.varadhan@microsoft.com, Rahul Gupta
>  <rahulgupt@microsoft.com>
> Subject: Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> 
> 08/11/2023 14:53, Dmitry Kozlyuk:
> > 2023-11-07 23:03 (UTC+0530), rahul gupta:
> > > > > From: Rahul Gupta <rahulgupt@linux.microsoft.com>
> > > > > To: dev@dpdk.org,  thomas@monjalon.net
> > > > > Cc: sovaradh@linux.microsoft.com, okaya@kernel.org,  
> > > > sujithsankar@microsoft.com,  sowmini.varadhan@microsoft.com,
> > > > rahulrgupta27@gmail.com,  Rahul Gupta <rahulgupt@microsoft.com>,  Rahul
> > > > Gupta <rahulgupt@linux.microsoft.com>  
> > > > > Subject: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> > > > > Date: Thu,  2 Nov 2023 11:19:24 -0700
> > > > > X-Mailer: git-send-email 1.8.3.1
> > > > >
> > > > > From: Rahul Gupta <rahulgupt@microsoft.com>
> > > > >
> > > > > Initialization often requires rte_eal_init + rte_pktmbuf_pool_create
> > > > > which can consume a total time of 500-600 ms:
> > > > > a) For many devices FLR may take a significant chunk of time
> > > > >    (200-250 ms in our use-case), this FLR is triggered during device
> > > > >    probe in rte_eal_init().
> > > > > b) rte_pktmbuf_pool_create() can consume upto 300-350 ms for
> > > > > applications that require huge memory.
> > > > >
> > > > > This cost is incurred on each restart (which happens in our use-case
> > > > > during binary updates for servicing).
> > > > > This patch provides an optimization using pthreads that appplications
> > > > > can use and which can save 200-230ms.
> > > > >
> > > > > In this patch, rte_eal_init() is refactored into two parts-
> > > > > a) 1st part is dependent code ie- it’s a perquisite of the FLR and
> > > > >    mempool creation. So this code needs to be executed before any
> > > > >    pthreads. Its named as rte_eal_init_setup()
> > > > > b) 2nd part of code is independent code ie- it can execute in parallel
> > > > >    to mempool creation in a pthread. Its named as rte_probe_and_ioctl().
> > > > >
> > > > > Existing applications require no changes unless they wish to leverage
> > > > > the optimization.
> > > > >
> > > > > If the application wants to use pthread functionality, it should call-
> > > > > a) rte_eal_init_setup() then create two or more pthreads-
> > > > > b) in one pthread call- rte_probe_and_ioctl(),
> > > > > c) second pthread call- rte_pktmbuf_pool_create()
> > > > > d) (optional) Other pthreads for  any other independent function.
> > > > >
> > > > > Signed-off-by: Rahul Gupta <rahulgupt@linux.microsoft.com>  
> > 
> > I doubt that the new API is required.
> > It is already possible to block all devices from automatic probing
> > with EAL options and then probe explicitly in any threads desired.
> > At the same time, this RFC shows a valuable optimization pattern,
> > so maybe it is worth having in DPDK as an example.
> > There are DPDK use cases when probing is completely unnecessary.
> 
> It seems here we want to do the device probing,
> but start it in parallel of other tasks.
> 
> > Exposing the initialization process stages makes it harder to refactor
> > and requires precise documentation of when and what is initialized
> > (for example, in this RFC rte_eal_init_setup()
> > does not make service core API usable yet).
> 
> Yes the init order is sensitive, that's why we have a big init function.
> But in general I would agree to try splitting it with necessary warnings
> and explanations.
> 
> > P. S. You may be also interested in using `--huge-unlink=never`
> > to speed rte_pktmbuf_pool_create() during restarts:
> > 
> > https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#id3
> 
> Yes good tip :)
> 
> 
Thank you for the comments. I will send a patch shortly.
eal_init_async(); //Internally forks a thread to do FLR.
/* Application can do other stuff, including mempool_create, possibly in
   multiple threads. If threads are forked, then application has to do any
   needed thread-joins */
eal_init_async_done(); //To sync with FLR thread.

^ permalink raw reply	[relevance 0%]

* Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
  2023-11-09 17:26  0%         ` Rahul Gupta
@ 2023-11-09 17:32  0%           ` Bruce Richardson
  2023-11-10 17:25  0%             ` Rahul Gupta
  0 siblings, 1 reply; 38+ results
From: Bruce Richardson @ 2023-11-09 17:32 UTC (permalink / raw)
  To: Rahul Gupta
  Cc: Thomas Monjalon, rahul gupta, Dmitry Kozlyuk, Stephen Hemminger,
	dev, sovaradh, okaya, sujithsankar, sowmini.varadhan

On Thu, Nov 09, 2023 at 09:26:27AM -0800, Rahul Gupta wrote:
> On (11/08/23 16:40), Thomas Monjalon wrote:
> > Date: Wed, 08 Nov 2023 16:40:07 +0100
> > From: Thomas Monjalon <thomas@monjalon.net>
> > To: rahul gupta <rahulrgupta27@gmail.com>, Dmitry Kozlyuk
> >  <dmitry.kozliuk@gmail.com>
> > Cc: Stephen Hemminger <stephen@networkplumber.org>, Rahul Gupta
> >  <rahulgupt@linux.microsoft.com>, dev@dpdk.org,
> >  sovaradh@linux.microsoft.com, okaya@kernel.org,
> >  sujithsankar@microsoft.com, sowmini.varadhan@microsoft.com, Rahul Gupta
> >  <rahulgupt@microsoft.com>
> > Subject: Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> > 
> > 08/11/2023 14:53, Dmitry Kozlyuk:
> > > 2023-11-07 23:03 (UTC+0530), rahul gupta:
> > > > > > From: Rahul Gupta <rahulgupt@linux.microsoft.com>
> > > > > > To: dev@dpdk.org,  thomas@monjalon.net
> > > > > > Cc: sovaradh@linux.microsoft.com, okaya@kernel.org,  
> > > > > sujithsankar@microsoft.com,  sowmini.varadhan@microsoft.com,
> > > > > rahulrgupta27@gmail.com,  Rahul Gupta <rahulgupt@microsoft.com>,  Rahul
> > > > > Gupta <rahulgupt@linux.microsoft.com>  
> > > > > > Subject: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> > > > > > Date: Thu,  2 Nov 2023 11:19:24 -0700
> > > > > > X-Mailer: git-send-email 1.8.3.1
> > > > > >
> > > > > > From: Rahul Gupta <rahulgupt@microsoft.com>
> > > > > >
> > > > > > Initialization often requires rte_eal_init + rte_pktmbuf_pool_create
> > > > > > which can consume a total time of 500-600 ms:
> > > > > > a) For many devices FLR may take a significant chunk of time
> > > > > >    (200-250 ms in our use-case), this FLR is triggered during device
> > > > > >    probe in rte_eal_init().
> > > > > > b) rte_pktmbuf_pool_create() can consume upto 300-350 ms for
> > > > > > applications that require huge memory.
> > > > > >
> > > > > > This cost is incurred on each restart (which happens in our use-case
> > > > > > during binary updates for servicing).
> > > > > > This patch provides an optimization using pthreads that appplications
> > > > > > can use and which can save 200-230ms.
> > > > > >
> > > > > > In this patch, rte_eal_init() is refactored into two parts-
> > > > > > a) 1st part is dependent code ie- it’s a perquisite of the FLR and
> > > > > >    mempool creation. So this code needs to be executed before any
> > > > > >    pthreads. Its named as rte_eal_init_setup()
> > > > > > b) 2nd part of code is independent code ie- it can execute in parallel
> > > > > >    to mempool creation in a pthread. Its named as rte_probe_and_ioctl().
> > > > > >
> > > > > > Existing applications require no changes unless they wish to leverage
> > > > > > the optimization.
> > > > > >
> > > > > > If the application wants to use pthread functionality, it should call-
> > > > > > a) rte_eal_init_setup() then create two or more pthreads-
> > > > > > b) in one pthread call- rte_probe_and_ioctl(),
> > > > > > c) second pthread call- rte_pktmbuf_pool_create()
> > > > > > d) (optional) Other pthreads for  any other independent function.
> > > > > >
> > > > > > Signed-off-by: Rahul Gupta <rahulgupt@linux.microsoft.com>  
> > > 
> > > I doubt that the new API is required.
> > > It is already possible to block all devices from automatic probing
> > > with EAL options and then probe explicitly in any threads desired.
> > > At the same time, this RFC shows a valuable optimization pattern,
> > > so maybe it is worth having in DPDK as an example.
> > > There are DPDK use cases when probing is completely unnecessary.
> > 
> > It seems here we want to do the device probing,
> > but start it in parallel of other tasks.
> > 
> > > Exposing the initialization process stages makes it harder to refactor
> > > and requires precise documentation of when and what is initialized
> > > (for example, in this RFC rte_eal_init_setup()
> > > does not make service core API usable yet).
> > 
> > Yes the init order is sensitive, that's why we have a big init function.
> > But in general I would agree to try splitting it with necessary warnings
> > and explanations.
> > 
> > > P. S. You may be also interested in using `--huge-unlink=never`
> > > to speed rte_pktmbuf_pool_create() during restarts:
> > > 
> > > https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#id3
> > 
> > Yes good tip :)
> > 
> > 
> Thank you for the comments. I will send a patch shortly.
> eal_init_async(); //Internally forks a thread to do FLR.
> /* Application can do other stuff, including mempool_create, possibly in
>    multiple threads. If threads are forked, then application has to do any
>    needed thread-joins */
> eal_init_async_done(); //To sync with FLR thread.

Just to note, the documentation on rte_eal_init_async() needs to call out
very explicitly what DPDK APIs, if any, can be called before the call to
async_done().

/Bruce

^ permalink raw reply	[relevance 0%]

* Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
  2023-11-09 17:32  0%           ` Bruce Richardson
@ 2023-11-10 17:25  0%             ` Rahul Gupta
  0 siblings, 0 replies; 38+ results
From: Rahul Gupta @ 2023-11-10 17:25 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Thomas Monjalon, rahul gupta, Dmitry Kozlyuk, Stephen Hemminger,
	dev, sovaradh, okaya, sujithsankar, sowmini.varadhan

On (11/09/23 17:32), Bruce Richardson wrote:
> Date: Thu, 9 Nov 2023 17:32:31 +0000
> From: Bruce Richardson <bruce.richardson@intel.com>
> To: Rahul Gupta <rahulgupt@linux.microsoft.com>
> CC: Thomas Monjalon <thomas@monjalon.net>, rahul gupta
>  <rahulrgupta27@gmail.com>, Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>,
>  Stephen Hemminger <stephen@networkplumber.org>, dev@dpdk.org,
>  sovaradh@linux.microsoft.com, okaya@kernel.org,
>  sujithsankar@microsoft.com, sowmini.varadhan@microsoft.com
> Subject: Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> 
> On Thu, Nov 09, 2023 at 09:26:27AM -0800, Rahul Gupta wrote:
> > On (11/08/23 16:40), Thomas Monjalon wrote:
> > > Date: Wed, 08 Nov 2023 16:40:07 +0100
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > To: rahul gupta <rahulrgupta27@gmail.com>, Dmitry Kozlyuk
> > >  <dmitry.kozliuk@gmail.com>
> > > Cc: Stephen Hemminger <stephen@networkplumber.org>, Rahul Gupta
> > >  <rahulgupt@linux.microsoft.com>, dev@dpdk.org,
> > >  sovaradh@linux.microsoft.com, okaya@kernel.org,
> > >  sujithsankar@microsoft.com, sowmini.varadhan@microsoft.com, Rahul Gupta
> > >  <rahulgupt@microsoft.com>
> > > Subject: Re: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> > > 
> > > 08/11/2023 14:53, Dmitry Kozlyuk:
> > > > 2023-11-07 23:03 (UTC+0530), rahul gupta:
> > > > > > > From: Rahul Gupta <rahulgupt@linux.microsoft.com>
> > > > > > > To: dev@dpdk.org,  thomas@monjalon.net
> > > > > > > Cc: sovaradh@linux.microsoft.com, okaya@kernel.org,  
> > > > > > sujithsankar@microsoft.com,  sowmini.varadhan@microsoft.com,
> > > > > > rahulrgupta27@gmail.com,  Rahul Gupta <rahulgupt@microsoft.com>,  Rahul
> > > > > > Gupta <rahulgupt@linux.microsoft.com>  
> > > > > > > Subject: [RFC] eal: RFC to refactor rte_eal_init into sub-functions
> > > > > > > Date: Thu,  2 Nov 2023 11:19:24 -0700
> > > > > > > X-Mailer: git-send-email 1.8.3.1
> > > > > > >
> > > > > > > From: Rahul Gupta <rahulgupt@microsoft.com>
> > > > > > >
> > > > > > > Initialization often requires rte_eal_init + rte_pktmbuf_pool_create
> > > > > > > which can consume a total time of 500-600 ms:
> > > > > > > a) For many devices FLR may take a significant chunk of time
> > > > > > >    (200-250 ms in our use-case), this FLR is triggered during device
> > > > > > >    probe in rte_eal_init().
> > > > > > > b) rte_pktmbuf_pool_create() can consume upto 300-350 ms for
> > > > > > > applications that require huge memory.
> > > > > > >
> > > > > > > This cost is incurred on each restart (which happens in our use-case
> > > > > > > during binary updates for servicing).
> > > > > > > This patch provides an optimization using pthreads that appplications
> > > > > > > can use and which can save 200-230ms.
> > > > > > >
> > > > > > > In this patch, rte_eal_init() is refactored into two parts-
> > > > > > > a) 1st part is dependent code ie- it’s a perquisite of the FLR and
> > > > > > >    mempool creation. So this code needs to be executed before any
> > > > > > >    pthreads. Its named as rte_eal_init_setup()
> > > > > > > b) 2nd part of code is independent code ie- it can execute in parallel
> > > > > > >    to mempool creation in a pthread. Its named as rte_probe_and_ioctl().
> > > > > > >
> > > > > > > Existing applications require no changes unless they wish to leverage
> > > > > > > the optimization.
> > > > > > >
> > > > > > > If the application wants to use pthread functionality, it should call-
> > > > > > > a) rte_eal_init_setup() then create two or more pthreads-
> > > > > > > b) in one pthread call- rte_probe_and_ioctl(),
> > > > > > > c) second pthread call- rte_pktmbuf_pool_create()
> > > > > > > d) (optional) Other pthreads for  any other independent function.
> > > > > > >
> > > > > > > Signed-off-by: Rahul Gupta <rahulgupt@linux.microsoft.com>  
> > > > 
> > > > I doubt that the new API is required.
> > > > It is already possible to block all devices from automatic probing
> > > > with EAL options and then probe explicitly in any threads desired.
> > > > At the same time, this RFC shows a valuable optimization pattern,
> > > > so maybe it is worth having in DPDK as an example.
> > > > There are DPDK use cases when probing is completely unnecessary.
> > > 
> > > It seems here we want to do the device probing,
> > > but start it in parallel of other tasks.
> > > 
> > > > Exposing the initialization process stages makes it harder to refactor
> > > > and requires precise documentation of when and what is initialized
> > > > (for example, in this RFC rte_eal_init_setup()
> > > > does not make service core API usable yet).
> > > 
> > > Yes the init order is sensitive, that's why we have a big init function.
> > > But in general I would agree to try splitting it with necessary warnings
> > > and explanations.
> > > 
> > > > P. S. You may be also interested in using `--huge-unlink=never`
> > > > to speed rte_pktmbuf_pool_create() during restarts:
> > > > 
> > > > https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#id3
> > > 
> > > Yes good tip :)
> > > 
> > > 
> > Thank you for the comments. I will send a patch shortly.
> > eal_init_async(); //Internally forks a thread to do FLR.
> > /* Application can do other stuff, including mempool_create, possibly in
> >    multiple threads. If threads are forked, then application has to do any
> >    needed thread-joins */
> > eal_init_async_done(); //To sync with FLR thread.
> 
> Just to note, the documentation on rte_eal_init_async() needs to call out
> very explicitly what DPDK APIs, if any, can be called before the call to
> async_done().
> 
> /Bruce

Yes, I will document it in commit log and near code.

Regards,
Rahul.

^ permalink raw reply	[relevance 0%]

* Re: [dpdk-dev] [PATCH v4] eal: refactor rte_eal_init into sub-functions
  @ 2024-01-29  5:35  0%   ` Rahul Gupta
    0 siblings, 1 reply; 38+ results
From: Rahul Gupta @ 2024-01-29  5:35 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, bruce.richardson, dmitry.kozliuk, stephen, sovaradh,
	okaya, sujithsankar, sowmini.varadhan, krathinavel,
	rahulrgupta27

On (01/24/24 16:53), David Marchand wrote:
> Date: Wed, 24 Jan 2024 16:53:33 +0100
> From: David Marchand <david.marchand@redhat.com>
> To: Rahul Gupta <rahulgupt@linux.microsoft.com>
> Cc: dev@dpdk.org, thomas@monjalon.net, bruce.richardson@intel.com,
>  dmitry.kozliuk@gmail.com, stephen@networkplumber.org,
>  sovaradh@linux.microsoft.com, okaya@kernel.org,
>  sujithsankar@microsoft.com, sowmini.varadhan@microsoft.com,
>  krathinavel@microsoft.com, rahulrgupta27@gmail.com, Rahul Gupta
>  <rahulgupt@microsoft.com>
> Subject: Re: [dpdk-dev] [PATCH v4] eal: refactor rte_eal_init into
>  sub-functions
> 
> On Wed, Jan 24, 2024 at 2:45 PM Rahul Gupta
> <rahulgupt@linux.microsoft.com> wrote:
> >
> > From: Rahul Gupta <rahulgupt@microsoft.com>
> >
> > In continuation to the following email, I am sending this patch.
> > (https://inbox.dpdk.org/dev/20231110172523.GA17466@microsoft.com/)
> >
> > Initialization requires rte_eal_init + rte_pktmbuf_pool_create which
> > can consume a total time of 500-600 ms:
> > a) For many devices FLR may take a significant chunk of time
> >    (200-250 ms in our use-case), this FLR is triggered during device
> >    probe in rte_eal_init().
> > b) rte_pktmbuf_pool_create() can consume up to 300-350 ms for
> > applications that require huge memory.
> >
> > This cost is incurred on each restart (which happens in our use-case
> > during binary updates for servicing).
> > This patch provides an optimization using pthreads that applications
> > can use and which can save 200-230ms.
> >
> > In this patch, rte_eal_init() is refactored into two parts-
> > a) 1st part is dependent code ie- it’s a perquisite of the FLR and
> >    mempool creation. So this code needs to be executed before any
> >    pthreads. Its named as rte_eal_init_setup()
> > b) 2nd part of code is independent code ie- it can execute in parallel
> >    to mempool creation in a pthread. Its named as rte_eal_init_async_setup().
> >
> > In existing applications no changes are required unless they wish to leverage
> > the optimization.
> >
> > If the application wants to leverage this optimization, then it needs to call
> > rte_eal_init_async() (instead of call rte_eal_init()), then it can create a
> > thread using rte_eal_remote_launch() to schedule a task it would like todo in
> > parallel rte_eal_init_async_setup(), this task can be a mbuf pool creation
> > using- rte_pktmbuf_pool_create()
> >
> > After this, if next operations require completion of above task, then
> > user can use rte_eal_init_wait_async_setup_complete(),
> > or if user wants to just check status of that thread, then use-
> > rte_eal_init_async_setup_done()
> 
> Looking at what this patch does.. I am under the impression all you
> really need is rte_eal_init without initial probing.
> Such behavior can probably be achieved with a allowlist set to a non
> existing device (like for example "-a 0000:00:00.0"), then later, use
> device hotplug.
The patch will be useful to all the adapters irrespective of their
host plug support.
> 
> Some quick note on this patch.
> 
> - don't expose symbols externally if they are only for internal use in
> the same library,
done in next patch.
> - current version is 24.03, not 24.01 (wrt comment in version.map),
done
> - other OSes are not handled by this patch, please do the work for
> FreeBSD and Windows,
I can send patch to support non-linux OS, but due to lack of setup,
I will need help to test same.

Also, I am planning to do the porting at the end (ie after incorporating
all review comments, in order to prevent duplication of efforts).

> - as a followup of the previous point, please check if we can share
> code between OSes and, if so, move the shared code to lib/eal/common,
The code for rte_eal_init() is different for all three distros, (even if
I consider just the 1st part of rte_eal_init() ie rte_eal_init_setup()).
So its difficult to move to common dir. We will have todo it separately
for all OS.
> - did you test this series with multiprocess?
Yes it works fine, I have tested it with simple_mp app.
> - why should telemetry and other parts of the current rte_eal_init()
> be left in the second stage of this initialisation pipeline?
Actually motivation of this patch was todo most of the work in parallel
ie in second stage, so not only probe/FLR but telemetry and any other work
which can be executed in parallel are done here. (pls refer to the link
in the commit message for more details)
> 
> 
> -- 
> David Marchand


Thanks for the reviewing, my comments are inline.

Thanks,
Rahul.

^ permalink raw reply	[relevance 0%]

* Re: [dpdk-dev] [PATCH v4] eal: refactor rte_eal_init into sub-functions
  @ 2024-02-03 12:57  0%         ` Rahul Gupta
  2024-02-08 17:05  0%           ` Rahul Gupta
  0 siblings, 1 reply; 38+ results
From: Rahul Gupta @ 2024-02-03 12:57 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: David Marchand, dev, bruce.richardson, dmitry.kozliuk, stephen,
	sovaradh, okaya, sujithsankar, sowmini.varadhan, krathinavel,
	rahulrgupta27

On (02/02/24 11:21), Thomas Monjalon wrote:
> Date: Fri, 02 Feb 2024 11:21:59 +0100
> From: Thomas Monjalon <thomas@monjalon.net>
> To: Rahul Gupta <rahulgupt@linux.microsoft.com>
> Cc: David Marchand <david.marchand@redhat.com>, dev@dpdk.org,
>  bruce.richardson@intel.com, dmitry.kozliuk@gmail.com,
>  stephen@networkplumber.org, sovaradh@linux.microsoft.com,
>  okaya@kernel.org, sujithsankar@microsoft.com,
>  sowmini.varadhan@microsoft.com, krathinavel@microsoft.com,
>  rahulrgupta27@gmail.com
> Subject: Re: [dpdk-dev] [PATCH v4] eal: refactor rte_eal_init into
>  sub-functions
> 
> 29/01/2024 08:55, David Marchand:
> > On Mon, Jan 29, 2024 at 6:35 AM Rahul Gupta
> > <rahulgupt@linux.microsoft.com> wrote:
> > > > Looking at what this patch does.. I am under the impression all you
> > > > really need is rte_eal_init without initial probing.
> > > > Such behavior can probably be achieved with a allowlist set to a non
> > > > existing device (like for example "-a 0000:00:00.0"), then later, use
> > > > device hotplug.
> > > The patch will be useful to all the adapters irrespective of their
> > > host plug support.
> > 
> > I did not say hotplug support is needed.
> > If what I described already works, this patch adds nothing.
> 
> I agree with David.
> Disabling initial probing should provide what you want.
> Did you test his proposal?
> 
> 
Yes, I was about to reply after testing same, will be done with testing in few days.
But I think the bootup time saved by my patch and hot plug patch will be almost same,
because apart from FLR (probe()) the extra work done by my patch (i.e. telemetry,
rte_service_init() in parallel to mbuf pool creation) are consuming very less bootup time.
So in future if more things are added to 2nd part of eal_init (i.e. rte_eal_init_async_setup()),
then the bootup time will be less if we use my patch.
I think we can defer this patch till then.

Thanks,
Rahul.

^ permalink raw reply	[relevance 0%]

* Re: [dpdk-dev] [PATCH v4] eal: refactor rte_eal_init into sub-functions
  2024-02-03 12:57  0%         ` Rahul Gupta
@ 2024-02-08 17:05  0%           ` Rahul Gupta
  0 siblings, 0 replies; 38+ results
From: Rahul Gupta @ 2024-02-08 17:05 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: David Marchand, dev, bruce.richardson, dmitry.kozliuk, stephen,
	sovaradh, okaya, sujithsankar, sowmini.varadhan, krathinavel,
	rahulrgupta27

On (02/03/24 04:57), Rahul Gupta wrote:
> Date: Sat, 3 Feb 2024 04:57:49 -0800
> From: Rahul Gupta <rahulgupt@linux.microsoft.com>
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: David Marchand <david.marchand@redhat.com>, dev@dpdk.org,
>  bruce.richardson@intel.com, dmitry.kozliuk@gmail.com,
>  stephen@networkplumber.org, sovaradh@linux.microsoft.com,
>  okaya@kernel.org, sujithsankar@microsoft.com,
>  sowmini.varadhan@microsoft.com, krathinavel@microsoft.com,
>  rahulrgupta27@gmail.com
> Subject: Re: [dpdk-dev] [PATCH v4] eal: refactor rte_eal_init into
>  sub-functions
> User-Agent: Mutt/1.5.21 (2010-09-15)
> 
> On (02/02/24 11:21), Thomas Monjalon wrote:
> > Date: Fri, 02 Feb 2024 11:21:59 +0100
> > From: Thomas Monjalon <thomas@monjalon.net>
> > To: Rahul Gupta <rahulgupt@linux.microsoft.com>
> > Cc: David Marchand <david.marchand@redhat.com>, dev@dpdk.org,
> >  bruce.richardson@intel.com, dmitry.kozliuk@gmail.com,
> >  stephen@networkplumber.org, sovaradh@linux.microsoft.com,
> >  okaya@kernel.org, sujithsankar@microsoft.com,
> >  sowmini.varadhan@microsoft.com, krathinavel@microsoft.com,
> >  rahulrgupta27@gmail.com
> > Subject: Re: [dpdk-dev] [PATCH v4] eal: refactor rte_eal_init into
> >  sub-functions
> > 
> > 29/01/2024 08:55, David Marchand:
> > > On Mon, Jan 29, 2024 at 6:35 AM Rahul Gupta
> > > <rahulgupt@linux.microsoft.com> wrote:
> > > > > Looking at what this patch does.. I am under the impression all you
> > > > > really need is rte_eal_init without initial probing.
> > > > > Such behavior can probably be achieved with a allowlist set to a non
> > > > > existing device (like for example "-a 0000:00:00.0"), then later, use
> > > > > device hotplug.
> > > > The patch will be useful to all the adapters irrespective of their
> > > > host plug support.
> > > 
> > > I did not say hotplug support is needed.
> > > If what I described already works, this patch adds nothing.
> > 
> > I agree with David.
> > Disabling initial probing should provide what you want.
> > Did you test his proposal?
> > 
> > 
> Yes, I was about to reply after testing same, will be done with testing in few days.
> But I think the bootup time saved by my patch and hot plug patch will be almost same,
> because apart from FLR (probe()) the extra work done by my patch (i.e. telemetry,
> rte_service_init() in parallel to mbuf pool creation) are consuming very less bootup time.
> So in future if more things are added to 2nd part of eal_init (i.e. rte_eal_init_async_setup()),
> then the bootup time will be less if we use my patch.
> I think we can defer this patch till then.
> 
> Thanks,
> Rahul.

Initial tests looks ok wrt application bootup time saving by using the hot plug APIs
and so we may not need the patch for rte_eal_init().

Thanks for revewing patch and suggestions.
Thanks,
Rahul.

^ permalink raw reply	[relevance 0%]

Results 1-38 of 38 | reverse | sort options + mbox downloads above
-- links below jump to the message on this page --
2015-04-15 16:49 13% [dpdk-dev] can't build memnic pmd Sowmini Varadhan
2015-04-28 23:45 13% [dpdk-dev] running l2fwd-ivshmem Sowmini Varadhan
2020-01-12 23:08 21% [dpdk-dev] [PATCH RFC 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
2020-01-12 23:08 19% ` [dpdk-dev] [PATCH RFC 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
2020-01-12 23:08 18% ` [dpdk-dev] [PATCH RFC 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
2020-01-13 18:05 21% [dpdk-dev] [PATCH RFC V2 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
2020-01-13 18:05 20% ` [dpdk-dev] [PATCH RFC V2 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
2020-01-13 18:05 18% ` [dpdk-dev] [PATCH RFC V2 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
2021-04-28 15:22  6%   ` Bernard Iremonger
2023-06-12 21:14  6% ` [dpdk-dev] [PATCH RFC V2 0/2] TCP flow classification using 4-tuple and flags Stephen Hemminger
2020-01-13 18:48 13% [dpdk-dev] patches not showing up on list? Sowmini Varadhan
2020-01-13 23:18  8% ` Thomas Monjalon
2020-06-26 16:43  5% [dpdk-dev] [dpdk-announce] [HELP REQUIRED] call for reviews Thomas Monjalon
2021-08-12 20:17 13% [dpdk-dev] [PATCH 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
2021-08-12 20:17 11% ` [dpdk-dev] [PATCH 1/2] Hooks to allow the setting of filters on tcp flags Sowmini Varadhan
2021-08-17 11:40  6%   ` Iremonger, Bernard
2021-08-12 20:17 10% ` [dpdk-dev] [PATCH 2/2] Allow the flow_classify example to add an ACL table for tcp Sowmini Varadhan
2021-08-17 12:08  6%   ` Iremonger, Bernard
2021-08-18 15:01 13% [dpdk-dev] [PATCH v2 0/2] TCP flow classification using 4-tuple and flags Sowmini Varadhan
2021-08-18 15:01 11% ` [dpdk-dev] [PATCH v2 1/2] examples/flow_classify: hooks for filters on tcp flags Sowmini Varadhan
2021-08-19 16:08  6%   ` Iremonger, Bernard
2021-08-18 15:01 10% ` [dpdk-dev] [PATCH v2 2/2] examples/flow_classify: add an ACL table for tcp Sowmini Varadhan
2021-08-19 15:06  6%   ` Iremonger, Bernard
2021-08-19 15:20 13%     ` Sowmini Varadhan
2021-08-19 16:21  6%       ` Iremonger, Bernard
2021-08-19 19:34 15%         ` Sowmini Varadhan
2023-07-03 23:38  6%           ` Stephen Hemminger
2023-11-02 18:19     [RFC] eal: RFC to refactor rte_eal_init into sub-functions Rahul Gupta
2023-11-02 18:37  0% ` Stephen Hemminger
2023-11-07 17:33  0%   ` rahul gupta
2023-11-08 13:53  0%     ` Dmitry Kozlyuk
2023-11-08 15:40  0%       ` Thomas Monjalon
2023-11-09 17:26  0%         ` Rahul Gupta
2023-11-09 17:32  0%           ` Bruce Richardson
2023-11-10 17:25  0%             ` Rahul Gupta
2023-11-08  4:38  0%   ` Rahul Gupta
2024-01-24 13:45     [dpdk-dev] [PATCH v4] eal: " Rahul Gupta
2024-01-24 15:53     ` David Marchand
2024-01-29  5:35  0%   ` Rahul Gupta
2024-01-29  7:55         ` David Marchand
2024-02-02 10:21           ` Thomas Monjalon
2024-02-03 12:57  0%         ` Rahul Gupta
2024-02-08 17:05  0%           ` Rahul Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).