DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/2] ACL fix 8B field
@ 2022-04-26 17:44 Konstantin Ananyev
  2022-04-26 17:44 ` [PATCH 1/2] acl: fix rules with 8 bytes field size are broken Konstantin Ananyev
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Konstantin Ananyev @ 2022-04-26 17:44 UTC (permalink / raw)
  To: dev

Fix problem with 8B fields and extend test-acl test coverage.

Konstantin Ananyev (2):
  acl: fix rules with 8 bytes field size are broken
  app/acl: support different formats for IPv6 address

 app/test-acl/main.c | 355 ++++++++++++++++++++++++++++++++++----------
 lib/acl/acl_bld.c   |  14 +-
 2 files changed, 286 insertions(+), 83 deletions(-)

-- 
2.34.1


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

* [PATCH 1/2] acl: fix rules with 8 bytes field size are broken
  2022-04-26 17:44 [PATCH 0/2] ACL fix 8B field Konstantin Ananyev
@ 2022-04-26 17:44 ` Konstantin Ananyev
  2022-04-26 17:44 ` [PATCH 2/2] app/acl: support different formats for IPv6 address Konstantin Ananyev
  2022-05-15 20:03 ` [PATCH v2 0/2] ACL fix 8B field Konstantin Ananyev
  2 siblings, 0 replies; 8+ messages in thread
From: Konstantin Ananyev @ 2022-04-26 17:44 UTC (permalink / raw)
  To: dev; +Cc: stable, Ido Goshen

In theory ACL library allows fields with 8B long.
Though in practice they usually not used, not tested,
and as was revealed by Ido, this functionality is not working properly.
There are few places inside ACL build code-path that need to be addressed.

Bugzilla ID: 673
Fixes: dc276b5780c2 ("acl: new library")
Cc: stable@dpdk.org

Reported-by: Ido Goshen <ido@cgstowernetworks.com>
Signed-off-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
---
 lib/acl/acl_bld.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/acl/acl_bld.c b/lib/acl/acl_bld.c
index 7ea30f4186..2816632803 100644
--- a/lib/acl/acl_bld.c
+++ b/lib/acl/acl_bld.c
@@ -12,6 +12,9 @@
 /* number of pointers per alloc */
 #define ACL_PTR_ALLOC	32
 
+/* account for situation when all fields are 8B long */
+#define ACL_MAX_INDEXES	(2 * RTE_ACL_MAX_FIELDS)
+
 /* macros for dividing rule sets heuristics */
 #define NODE_MAX	0x4000
 #define NODE_MIN	0x800
@@ -80,7 +83,7 @@ struct acl_build_context {
 	struct tb_mem_pool        pool;
 	struct rte_acl_trie       tries[RTE_ACL_MAX_TRIES];
 	struct rte_acl_bld_trie   bld_tries[RTE_ACL_MAX_TRIES];
-	uint32_t            data_indexes[RTE_ACL_MAX_TRIES][RTE_ACL_MAX_FIELDS];
+	uint32_t            data_indexes[RTE_ACL_MAX_TRIES][ACL_MAX_INDEXES];
 
 	/* memory free lists for nodes and blocks used for node ptrs */
 	struct acl_mem_block      blocks[MEM_BLOCK_NUM];
@@ -988,7 +991,7 @@ build_trie(struct acl_build_context *context, struct rte_acl_build_rule *head,
 				 */
 				uint64_t mask;
 				mask = RTE_ACL_MASKLEN_TO_BITMASK(
-					fld->mask_range.u32,
+					fld->mask_range.u64,
 					rule->config->defs[n].size);
 
 				/* gen a mini-trie for this field */
@@ -1301,6 +1304,9 @@ acl_build_index(const struct rte_acl_config *config, uint32_t *data_index)
 		if (last_header != config->defs[n].input_index) {
 			last_header = config->defs[n].input_index;
 			data_index[m++] = config->defs[n].offset;
+			if (config->defs[n].size > sizeof(uint32_t))
+				data_index[m++] = config->defs[n].offset +
+					sizeof(uint32_t);
 		}
 	}
 
@@ -1487,7 +1493,7 @@ acl_set_data_indexes(struct rte_acl_ctx *ctx)
 		memcpy(ctx->data_indexes + ofs, ctx->trie[i].data_index,
 			n * sizeof(ctx->data_indexes[0]));
 		ctx->trie[i].data_index = ctx->data_indexes + ofs;
-		ofs += RTE_ACL_MAX_FIELDS;
+		ofs += ACL_MAX_INDEXES;
 	}
 }
 
@@ -1643,7 +1649,7 @@ rte_acl_build(struct rte_acl_ctx *ctx, const struct rte_acl_config *cfg)
 			/* allocate and fill run-time  structures. */
 			rc = rte_acl_gen(ctx, bcx.tries, bcx.bld_tries,
 				bcx.num_tries, bcx.cfg.num_categories,
-				RTE_ACL_MAX_FIELDS * RTE_DIM(bcx.tries) *
+				ACL_MAX_INDEXES * RTE_DIM(bcx.tries) *
 				sizeof(ctx->data_indexes[0]), max_size);
 			if (rc == 0) {
 				/* set data indexes. */
-- 
2.34.1


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

* [PATCH 2/2] app/acl: support different formats for IPv6 address
  2022-04-26 17:44 [PATCH 0/2] ACL fix 8B field Konstantin Ananyev
  2022-04-26 17:44 ` [PATCH 1/2] acl: fix rules with 8 bytes field size are broken Konstantin Ananyev
@ 2022-04-26 17:44 ` Konstantin Ananyev
  2022-05-15 20:03 ` [PATCH v2 0/2] ACL fix 8B field Konstantin Ananyev
  2 siblings, 0 replies; 8+ messages in thread
From: Konstantin Ananyev @ 2022-04-26 17:44 UTC (permalink / raw)
  To: dev

    Within ACL rule IPv6 address can be represented in different ways:
    either as 4x4B fields, or as 2x8B fields.
    Till now, only first format was supported.
    Extend test-acl to support both formats, mainly for testing and
    demonstrating purposes.
    To control desired behavior '--ipv6' command-line option is extend
    to accept an optional argument:
    To be more precise:
     '--ipv6'    - use 4x4B fields format (default behavior)
     '--ipv6=4B' - use 4x4B fields format (default behavior)
     '--ipv6=8B' - use 2x8B fields format
app/acl: use posix functions for network address parsing

Also replaced home brewed IPv4/IPv6 address parsing with inet_pton() calls.

Signed-off-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
---
 app/test-acl/main.c | 355 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 276 insertions(+), 79 deletions(-)

diff --git a/app/test-acl/main.c b/app/test-acl/main.c
index 06e3847ab9..41ce83db08 100644
--- a/app/test-acl/main.c
+++ b/app/test-acl/main.c
@@ -57,6 +57,12 @@ enum {
 	DUMP_MAX
 };
 
+enum {
+	IPV6_FRMT_NONE,
+	IPV6_FRMT_U32,
+	IPV6_FRMT_U64,
+};
+
 struct acl_alg {
 	const char *name;
 	enum rte_acl_classify_alg alg;
@@ -123,7 +129,7 @@ static struct {
 		.name = "default",
 		.alg = RTE_ACL_CLASSIFY_DEFAULT,
 	},
-	.ipv6 = 0
+	.ipv6 = IPV6_FRMT_NONE,
 };
 
 static struct rte_acl_param prm = {
@@ -210,6 +216,7 @@ struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
 #define	IPV6_ADDR_LEN	16
 #define	IPV6_ADDR_U16	(IPV6_ADDR_LEN / sizeof(uint16_t))
 #define	IPV6_ADDR_U32	(IPV6_ADDR_LEN / sizeof(uint32_t))
+#define	IPV6_ADDR_U64	(IPV6_ADDR_LEN / sizeof(uint64_t))
 
 struct ipv6_5tuple {
 	uint8_t  proto;
@@ -219,6 +226,7 @@ struct ipv6_5tuple {
 	uint16_t port_dst;
 };
 
+/* treat IPV6 address as uint32_t[4] (default mode) */
 enum {
 	PROTO_FIELD_IPV6,
 	SRC1_FIELD_IPV6,
@@ -234,6 +242,27 @@ enum {
 	NUM_FIELDS_IPV6
 };
 
+/* treat IPV6 address as uint64_t[2] (default mode) */
+enum {
+	PROTO_FIELD_IPV6_U64,
+	SRC1_FIELD_IPV6_U64,
+	SRC2_FIELD_IPV6_U64,
+	DST1_FIELD_IPV6_U64,
+	DST2_FIELD_IPV6_U64,
+	SRCP_FIELD_IPV6_U64,
+	DSTP_FIELD_IPV6_U64,
+	NUM_FIELDS_IPV6_U64
+};
+
+enum {
+	PROTO_INDEX_IPV6_U64 = PROTO_FIELD_IPV6_U64,
+	SRC1_INDEX_IPV6_U64 = SRC1_FIELD_IPV6_U64,
+	SRC2_INDEX_IPV6_U64 = SRC2_FIELD_IPV6_U64 + 1,
+	DST1_INDEX_IPV6_U64 = DST1_FIELD_IPV6_U64 + 2,
+	DST2_INDEX_IPV6_U64 = DST2_FIELD_IPV6_U64 + 3,
+	PRT_INDEX_IPV6_U64 = SRCP_FIELD_IPV6 + 4,
+};
+
 struct rte_acl_field_def ipv6_defs[NUM_FIELDS_IPV6] = {
 	{
 		.type = RTE_ACL_FIELD_TYPE_BITMASK,
@@ -314,6 +343,57 @@ struct rte_acl_field_def ipv6_defs[NUM_FIELDS_IPV6] = {
 	},
 };
 
+struct rte_acl_field_def ipv6_u64_defs[NUM_FIELDS_IPV6_U64] = {
+	{
+		.type = RTE_ACL_FIELD_TYPE_BITMASK,
+		.size = sizeof(uint8_t),
+		.field_index = PROTO_FIELD_IPV6_U64,
+		.input_index = PROTO_FIELD_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, proto),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_MASK,
+		.size = sizeof(uint64_t),
+		.field_index = SRC1_FIELD_IPV6_U64,
+		.input_index = SRC1_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, ip_src[0]),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_MASK,
+		.size = sizeof(uint64_t),
+		.field_index = SRC2_FIELD_IPV6_U64,
+		.input_index = SRC2_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, ip_src[2]),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_MASK,
+		.size = sizeof(uint64_t),
+		.field_index = DST1_FIELD_IPV6_U64,
+		.input_index = DST1_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, ip_dst[0]),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_MASK,
+		.size = sizeof(uint64_t),
+		.field_index = DST2_FIELD_IPV6_U64,
+		.input_index = DST2_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, ip_dst[2]),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_RANGE,
+		.size = sizeof(uint16_t),
+		.field_index = SRCP_FIELD_IPV6_U64,
+		.input_index = PRT_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, port_src),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_RANGE,
+		.size = sizeof(uint16_t),
+		.field_index = DSTP_FIELD_IPV6_U64,
+		.input_index = PRT_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, port_dst),
+	},
+};
 
 enum {
 	CB_FLD_SRC_ADDR,
@@ -385,49 +465,11 @@ parse_cb_ipv4_trace(char *str, struct ipv4_5tuple *v)
 	return 0;
 }
 
-/*
- * Parse IPv6 address, expects the following format:
- * XXXX:XXXX:XXXX:XXXX:XXXX:XXXX:XXXX:XXXX (where X is a hexadecimal digit).
- */
-static int
-parse_ipv6_addr(const char *in, const char **end, uint32_t v[IPV6_ADDR_U32],
-	char dlm)
-{
-	uint32_t addr[IPV6_ADDR_U16];
-
-	GET_CB_FIELD(in, addr[0], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[1], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[2], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[3], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[4], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[5], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[6], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[7], 16, UINT16_MAX, dlm);
-
-	*end = in;
-
-	v[0] = (addr[0] << 16) + addr[1];
-	v[1] = (addr[2] << 16) + addr[3];
-	v[2] = (addr[4] << 16) + addr[5];
-	v[3] = (addr[6] << 16) + addr[7];
-
-	return 0;
-}
-
 static int
 parse_cb_ipv6_addr_trace(const char *in, uint32_t v[IPV6_ADDR_U32])
 {
-	int32_t rc;
-	const char *end;
-
-	rc = parse_ipv6_addr(in, &end, v, 0);
-	if (rc != 0)
-		return rc;
-
-	v[0] = rte_cpu_to_be_32(v[0]);
-	v[1] = rte_cpu_to_be_32(v[1]);
-	v[2] = rte_cpu_to_be_32(v[2]);
-	v[3] = rte_cpu_to_be_32(v[3]);
+	if (inet_pton(AF_INET6, in, v) != 1)
+		return -EINVAL;
 
 	return 0;
 }
@@ -548,20 +590,33 @@ tracef_init(void)
 }
 
 static int
-parse_ipv6_net(const char *in, struct rte_acl_field field[4])
+parse_ipv6_u32_net(char *in, struct rte_acl_field field[IPV6_ADDR_U32])
 {
-	int32_t rc;
-	const char *mp;
-	uint32_t i, m, v[4];
+	char *sa, *sm, *sv;
+	uint32_t i, m, v[IPV6_ADDR_U32];
+
+	const char *dlm = "/";
 	const uint32_t nbu32 = sizeof(uint32_t) * CHAR_BIT;
 
 	/* get address. */
-	rc = parse_ipv6_addr(in, &mp, v, '/');
-	if (rc != 0)
-		return rc;
+	sv = NULL;
+	sa = strtok_r(in, dlm, &sv);
+	if (sa == NULL)
+		return -EINVAL;
+	sm = strtok_r(NULL, dlm, &sv);
+	if (sm == NULL)
+		return -EINVAL;
+
+	if (inet_pton(AF_INET6, sa, v) != 1)
+		return -EINVAL;
+
+	v[0] = rte_be_to_cpu_32(v[0]);
+	v[1] = rte_be_to_cpu_32(v[1]);
+	v[2] = rte_be_to_cpu_32(v[2]);
+	v[3] = rte_be_to_cpu_32(v[3]);
 
 	/* get mask. */
-	GET_CB_FIELD(mp, m, 0, CHAR_BIT * sizeof(v), 0);
+	GET_CB_FIELD(sm, m, 0, CHAR_BIT * sizeof(v), 0);
 
 	/* put all together. */
 	for (i = 0; i != RTE_DIM(v); i++) {
@@ -569,7 +624,7 @@ parse_ipv6_net(const char *in, struct rte_acl_field field[4])
 			field[i].mask_range.u32 = nbu32;
 		else
 			field[i].mask_range.u32 = m > (i * nbu32) ?
-				m - (i * 32) : 0;
+				m - (i * nbu32) : 0;
 
 		field[i].value.u32 = v[i];
 	}
@@ -577,14 +632,88 @@ parse_ipv6_net(const char *in, struct rte_acl_field field[4])
 	return 0;
 }
 
+static int
+parse_ipv6_u64_net(char *in, struct rte_acl_field field[IPV6_ADDR_U64])
+{
+	char *sa, *sm, *sv;
+	uint32_t i, m;
+	uint64_t v[IPV6_ADDR_U64];
+
+	const char *dlm = "/";
+	const uint32_t nbu64 = sizeof(uint64_t) * CHAR_BIT;
+
+	/* get address. */
+	sv = NULL;
+	sa = strtok_r(in, dlm, &sv);
+	if (sa == NULL)
+		return -EINVAL;
+	sm = strtok_r(NULL, dlm, &sv);
+	if (sm == NULL)
+		return -EINVAL;
+
+	if (inet_pton(AF_INET6, sa, v) != 1)
+		return -EINVAL;
+
+	v[0] = rte_be_to_cpu_64(v[0]);
+	v[1] = rte_be_to_cpu_64(v[1]);
+
+	/* get mask. */
+	GET_CB_FIELD(sm, m, 0, CHAR_BIT * sizeof(v), 0);
+
+	/* put all together. */
+	for (i = 0; i != RTE_DIM(v); i++) {
+		if (m >= (i + 1) * nbu64)
+			field[i].mask_range.u32 = nbu64;
+		else
+			field[i].mask_range.u32 = m > (i * nbu64) ?
+				m - (i * nbu64) : 0;
+
+		field[i].value.u64 = v[i];
+	}
+
+	return 0;
+}
 
 static int
-parse_cb_ipv6_rule(char *str, struct acl_rule *v)
+parse_cb_ipv6_rule(char *str, struct acl_rule *v, int frmt)
 {
 	int i, rc;
+	uint32_t fidx;
+	const uint32_t *field_map;
 	char *s, *sp, *in[CB_FLD_NUM];
+	int (*parse_ipv6_net)(char *s, struct rte_acl_field f[]);
+
 	static const char *dlm = " \t\n";
 
+	static const uint32_t field_map_u32[CB_FLD_NUM] = {
+		[CB_FLD_SRC_ADDR] = SRC1_FIELD_IPV6,
+		[CB_FLD_DST_ADDR] = DST1_FIELD_IPV6,
+		[CB_FLD_SRC_PORT_LOW] = SRCP_FIELD_IPV6,
+		[CB_FLD_SRC_PORT_HIGH] = SRCP_FIELD_IPV6,
+		[CB_FLD_DST_PORT_LOW] = DSTP_FIELD_IPV6,
+		[CB_FLD_DST_PORT_HIGH] = DSTP_FIELD_IPV6,
+		[CB_FLD_PROTO] = PROTO_FIELD_IPV6,
+	};
+
+	static const uint32_t field_map_u64[CB_FLD_NUM] = {
+		[CB_FLD_SRC_ADDR] = SRC1_FIELD_IPV6_U64,
+		[CB_FLD_DST_ADDR] = DST1_FIELD_IPV6_U64,
+		[CB_FLD_SRC_PORT_LOW] = SRCP_FIELD_IPV6_U64,
+		[CB_FLD_SRC_PORT_HIGH] = SRCP_FIELD_IPV6_U64,
+		[CB_FLD_DST_PORT_LOW] = DSTP_FIELD_IPV6_U64,
+		[CB_FLD_DST_PORT_HIGH] = DSTP_FIELD_IPV6_U64,
+		[CB_FLD_PROTO] = PROTO_FIELD_IPV6_U64,
+	};
+
+	if (frmt == IPV6_FRMT_U32) {
+		field_map = field_map_u32;
+		parse_ipv6_net = parse_ipv6_u32_net;
+	} else if (frmt == IPV6_FRMT_U64) {
+		field_map = field_map_u64;
+		parse_ipv6_net = parse_ipv6_u64_net;
+	} else
+		return -ENOTSUP;
+
 	/*
 	 * Skip leading '@'
 	 */
@@ -600,28 +729,30 @@ parse_cb_ipv6_rule(char *str, struct acl_rule *v)
 		s = NULL;
 	}
 
-	rc = parse_ipv6_net(in[CB_FLD_SRC_ADDR], v->field + SRC1_FIELD_IPV6);
+	fidx = CB_FLD_SRC_ADDR;
+	rc = parse_ipv6_net(in[fidx], v->field + field_map[fidx]);
 	if (rc != 0) {
 		RTE_LOG(ERR, TESTACL,
-			"failed to read source address/mask: %s\n",
-			in[CB_FLD_SRC_ADDR]);
+			"failed to read source address/mask: %s\n", in[fidx]);
 		return rc;
 	}
 
-	rc = parse_ipv6_net(in[CB_FLD_DST_ADDR], v->field + DST1_FIELD_IPV6);
+	fidx = CB_FLD_DST_ADDR;
+	rc = parse_ipv6_net(in[fidx], v->field + field_map[fidx]);
 	if (rc != 0) {
 		RTE_LOG(ERR, TESTACL,
 			"failed to read destination address/mask: %s\n",
-			in[CB_FLD_DST_ADDR]);
+			in[fidx]);
 		return rc;
 	}
 
 	/* source port. */
-	GET_CB_FIELD(in[CB_FLD_SRC_PORT_LOW],
-		v->field[SRCP_FIELD_IPV6].value.u16,
+	fidx = CB_FLD_SRC_PORT_LOW;
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].value.u16,
 		0, UINT16_MAX, 0);
-	GET_CB_FIELD(in[CB_FLD_SRC_PORT_HIGH],
-		v->field[SRCP_FIELD_IPV6].mask_range.u16,
+
+	fidx = CB_FLD_SRC_PORT_HIGH;
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].mask_range.u16,
 		0, UINT16_MAX, 0);
 
 	if (strncmp(in[CB_FLD_SRC_PORT_DLM], cb_port_delim,
@@ -629,37 +760,61 @@ parse_cb_ipv6_rule(char *str, struct acl_rule *v)
 		return -EINVAL;
 
 	/* destination port. */
-	GET_CB_FIELD(in[CB_FLD_DST_PORT_LOW],
-		v->field[DSTP_FIELD_IPV6].value.u16,
+	fidx = CB_FLD_DST_PORT_LOW;
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].value.u16,
 		0, UINT16_MAX, 0);
-	GET_CB_FIELD(in[CB_FLD_DST_PORT_HIGH],
-		v->field[DSTP_FIELD_IPV6].mask_range.u16,
+
+	fidx = CB_FLD_DST_PORT_HIGH;
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].mask_range.u16,
 		0, UINT16_MAX, 0);
 
 	if (strncmp(in[CB_FLD_DST_PORT_DLM], cb_port_delim,
 			sizeof(cb_port_delim)) != 0)
 		return -EINVAL;
 
-	GET_CB_FIELD(in[CB_FLD_PROTO], v->field[PROTO_FIELD_IPV6].value.u8,
+	fidx = CB_FLD_PROTO;
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].value.u8,
 		0, UINT8_MAX, '/');
-	GET_CB_FIELD(in[CB_FLD_PROTO], v->field[PROTO_FIELD_IPV6].mask_range.u8,
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].mask_range.u8,
 		0, UINT8_MAX, 0);
 
 	return 0;
 }
 
 static int
-parse_ipv4_net(const char *in, uint32_t *addr, uint32_t *mask_len)
+parse_cb_ipv6_u32_rule(char *str, struct acl_rule *v)
 {
-	uint8_t a, b, c, d, m;
+	return parse_cb_ipv6_rule(str, v, IPV6_FRMT_U32);
+}
 
-	GET_CB_FIELD(in, a, 0, UINT8_MAX, '.');
-	GET_CB_FIELD(in, b, 0, UINT8_MAX, '.');
-	GET_CB_FIELD(in, c, 0, UINT8_MAX, '.');
-	GET_CB_FIELD(in, d, 0, UINT8_MAX, '/');
-	GET_CB_FIELD(in, m, 0, sizeof(uint32_t) * CHAR_BIT, 0);
+static int
+parse_cb_ipv6_u64_rule(char *str, struct acl_rule *v)
+{
+	return parse_cb_ipv6_rule(str, v, IPV6_FRMT_U64);
+}
+
+static int
+parse_ipv4_net(char *in, uint32_t *addr, uint32_t *mask_len)
+{
+	char *sa, *sm, *sv;
+	uint32_t m, v;
 
-	addr[0] = RTE_IPV4(a, b, c, d);
+	const char *dlm = "/";
+
+	sv = NULL;
+	sa = strtok_r(in, dlm, &sv);
+	if (sa == NULL)
+		return -EINVAL;
+	sm = strtok_r(NULL, dlm, &sv);
+	if (sm == NULL)
+		return -EINVAL;
+
+	if (inet_pton(AF_INET, sa, &v) != 1)
+		return -EINVAL;
+
+	addr[0] = rte_be_to_cpu_32(v);
+
+	GET_CB_FIELD(sm, m, 0, sizeof(uint32_t) * CHAR_BIT, 0);
 	mask_len[0] = m;
 
 	return 0;
@@ -757,8 +912,14 @@ add_cb_rules(FILE *f, struct rte_acl_ctx *ctx)
 	struct acl_rule v;
 	parse_5tuple parser;
 
+	static const parse_5tuple parser_func[] = {
+		[IPV6_FRMT_NONE] = parse_cb_ipv4_rule,
+		[IPV6_FRMT_U32] = parse_cb_ipv6_u32_rule,
+		[IPV6_FRMT_U64] = parse_cb_ipv6_u64_rule,
+	};
+
 	memset(&v, 0, sizeof(v));
-	parser = (config.ipv6 != 0) ? parse_cb_ipv6_rule : parse_cb_ipv4_rule;
+	parser = parser_func[config.ipv6];
 
 	k = 0;
 	for (i = 1; fgets(line, sizeof(line), f) != NULL; i++) {
@@ -804,9 +965,12 @@ acx_init(void)
 	memset(&cfg, 0, sizeof(cfg));
 
 	/* setup ACL build config. */
-	if (config.ipv6) {
+	if (config.ipv6 == IPV6_FRMT_U32) {
 		cfg.num_fields = RTE_DIM(ipv6_defs);
 		memcpy(&cfg.defs, ipv6_defs, sizeof(ipv6_defs));
+	} else if (config.ipv6 == IPV6_FRMT_U64) {
+		cfg.num_fields = RTE_DIM(ipv6_u64_defs);
+		memcpy(&cfg.defs, ipv6_u64_defs, sizeof(ipv6_u64_defs));
 	} else {
 		cfg.num_fields = RTE_DIM(ipv4_defs);
 		memcpy(&cfg.defs, ipv4_defs, sizeof(ipv4_defs));
@@ -958,6 +1122,37 @@ get_alg_opt(const char *opt, const char *name)
 		opt, name);
 }
 
+static void
+get_ipv6_opt(const char *opt, const char *name)
+{
+	uint32_t i;
+
+	static const struct {
+		const char *name;
+		uint32_t val;
+	} ipv6_opt[] = {
+		{
+			.name = "4B",
+			.val = IPV6_FRMT_U32,
+		},
+		{
+			.name = "8B",
+			.val = IPV6_FRMT_U64,
+		},
+	};
+
+	for (i = 0; i != RTE_DIM(ipv6_opt); i++) {
+		if (strcmp(opt, ipv6_opt[i].name) == 0) {
+			config.ipv6 = ipv6_opt[i].val;
+			return;
+		}
+	}
+
+	rte_exit(-EINVAL, "invalid value: \"%s\" for option: %s\n",
+		opt, name);
+}
+
+
 static void
 print_usage(const char *prgname)
 {
@@ -999,7 +1194,7 @@ print_usage(const char *prgname)
 		"[--" OPT_ITER_NUM "=<number of iterations to perform>]\n"
 		"[--" OPT_VERBOSE "=<verbose level>]\n"
 		"[--" OPT_SEARCH_ALG "=%s]\n"
-		"[--" OPT_IPV6 "=<IPv6 rules and trace files>]\n",
+		"[--" OPT_IPV6 "(=4B | 8B) <IPv6 rules and trace files>]\n",
 		prgname, RTE_ACL_RESULTS_MULTIPLIER,
 		(uint32_t)RTE_ACL_MAX_CATEGORIES,
 		buf);
@@ -1050,7 +1245,7 @@ get_input_opts(int argc, char **argv)
 		{OPT_ITER_NUM, 1, 0, 0},
 		{OPT_VERBOSE, 1, 0, 0},
 		{OPT_SEARCH_ALG, 1, 0, 0},
-		{OPT_IPV6, 0, 0, 0},
+		{OPT_IPV6, 2, 0, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -1099,7 +1294,9 @@ get_input_opts(int argc, char **argv)
 				OPT_SEARCH_ALG) == 0) {
 			get_alg_opt(optarg, lgopts[opt_idx].name);
 		} else if (strcmp(lgopts[opt_idx].name, OPT_IPV6) == 0) {
-			config.ipv6 = 1;
+			config.ipv6 = IPV6_FRMT_U32;
+			if (optarg != NULL)
+				get_ipv6_opt(optarg, lgopts[opt_idx].name);
 		}
 	}
 	config.trace_sz = config.ipv6 ? sizeof(struct ipv6_5tuple) :
-- 
2.34.1


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

* [PATCH v2 0/2] ACL fix 8B field
  2022-04-26 17:44 [PATCH 0/2] ACL fix 8B field Konstantin Ananyev
  2022-04-26 17:44 ` [PATCH 1/2] acl: fix rules with 8 bytes field size are broken Konstantin Ananyev
  2022-04-26 17:44 ` [PATCH 2/2] app/acl: support different formats for IPv6 address Konstantin Ananyev
@ 2022-05-15 20:03 ` Konstantin Ananyev
  2022-05-15 20:03   ` [PATCH v2 1/2] acl: fix rules with 8 bytes field size are broken Konstantin Ananyev
                     ` (2 more replies)
  2 siblings, 3 replies; 8+ messages in thread
From: Konstantin Ananyev @ 2022-05-15 20:03 UTC (permalink / raw)
  To: dev; +Cc: ido

Fix problem with 8B fields and extend test-acl test coverage.

v2:
 - extend app/test-acl/test-acl.sh to support both formats for IPv6
   addresses.

Konstantin Ananyev (2):
  acl: fix rules with 8 bytes field size are broken
  app/acl: support different formats for IPv6 address

 app/test-acl/main.c      | 355 ++++++++++++++++++++++++++++++---------
 app/test-acl/test-acl.sh |   4 +-
 lib/acl/acl_bld.c        |  14 +-
 3 files changed, 289 insertions(+), 84 deletions(-)

-- 
2.34.1


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

* [PATCH v2 1/2] acl: fix rules with 8 bytes field size are broken
  2022-05-15 20:03 ` [PATCH v2 0/2] ACL fix 8B field Konstantin Ananyev
@ 2022-05-15 20:03   ` Konstantin Ananyev
  2022-05-16  6:23     ` Ido Goshen
  2022-05-15 20:03   ` [PATCH v2 2/2] app/acl: support different formats for IPv6 address Konstantin Ananyev
  2022-05-30 21:33   ` [PATCH v2 0/2] ACL fix 8B field Thomas Monjalon
  2 siblings, 1 reply; 8+ messages in thread
From: Konstantin Ananyev @ 2022-05-15 20:03 UTC (permalink / raw)
  To: dev; +Cc: ido, stable

In theory ACL library allows fields with 8B long.
Though in practice they usually not used, not tested,
and as was revealed by Ido, this functionality is not working properly.
There are few places inside ACL build code-path that need to be addressed.

Bugzilla ID: 673
Fixes: dc276b5780c2 ("acl: new library")
Cc: stable@dpdk.org

Reported-by: Ido Goshen <ido@cgstowernetworks.com>
Signed-off-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
---
 lib/acl/acl_bld.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/acl/acl_bld.c b/lib/acl/acl_bld.c
index 7ea30f4186..2816632803 100644
--- a/lib/acl/acl_bld.c
+++ b/lib/acl/acl_bld.c
@@ -12,6 +12,9 @@
 /* number of pointers per alloc */
 #define ACL_PTR_ALLOC	32
 
+/* account for situation when all fields are 8B long */
+#define ACL_MAX_INDEXES	(2 * RTE_ACL_MAX_FIELDS)
+
 /* macros for dividing rule sets heuristics */
 #define NODE_MAX	0x4000
 #define NODE_MIN	0x800
@@ -80,7 +83,7 @@ struct acl_build_context {
 	struct tb_mem_pool        pool;
 	struct rte_acl_trie       tries[RTE_ACL_MAX_TRIES];
 	struct rte_acl_bld_trie   bld_tries[RTE_ACL_MAX_TRIES];
-	uint32_t            data_indexes[RTE_ACL_MAX_TRIES][RTE_ACL_MAX_FIELDS];
+	uint32_t            data_indexes[RTE_ACL_MAX_TRIES][ACL_MAX_INDEXES];
 
 	/* memory free lists for nodes and blocks used for node ptrs */
 	struct acl_mem_block      blocks[MEM_BLOCK_NUM];
@@ -988,7 +991,7 @@ build_trie(struct acl_build_context *context, struct rte_acl_build_rule *head,
 				 */
 				uint64_t mask;
 				mask = RTE_ACL_MASKLEN_TO_BITMASK(
-					fld->mask_range.u32,
+					fld->mask_range.u64,
 					rule->config->defs[n].size);
 
 				/* gen a mini-trie for this field */
@@ -1301,6 +1304,9 @@ acl_build_index(const struct rte_acl_config *config, uint32_t *data_index)
 		if (last_header != config->defs[n].input_index) {
 			last_header = config->defs[n].input_index;
 			data_index[m++] = config->defs[n].offset;
+			if (config->defs[n].size > sizeof(uint32_t))
+				data_index[m++] = config->defs[n].offset +
+					sizeof(uint32_t);
 		}
 	}
 
@@ -1487,7 +1493,7 @@ acl_set_data_indexes(struct rte_acl_ctx *ctx)
 		memcpy(ctx->data_indexes + ofs, ctx->trie[i].data_index,
 			n * sizeof(ctx->data_indexes[0]));
 		ctx->trie[i].data_index = ctx->data_indexes + ofs;
-		ofs += RTE_ACL_MAX_FIELDS;
+		ofs += ACL_MAX_INDEXES;
 	}
 }
 
@@ -1643,7 +1649,7 @@ rte_acl_build(struct rte_acl_ctx *ctx, const struct rte_acl_config *cfg)
 			/* allocate and fill run-time  structures. */
 			rc = rte_acl_gen(ctx, bcx.tries, bcx.bld_tries,
 				bcx.num_tries, bcx.cfg.num_categories,
-				RTE_ACL_MAX_FIELDS * RTE_DIM(bcx.tries) *
+				ACL_MAX_INDEXES * RTE_DIM(bcx.tries) *
 				sizeof(ctx->data_indexes[0]), max_size);
 			if (rc == 0) {
 				/* set data indexes. */
-- 
2.34.1


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

* [PATCH v2 2/2] app/acl: support different formats for IPv6 address
  2022-05-15 20:03 ` [PATCH v2 0/2] ACL fix 8B field Konstantin Ananyev
  2022-05-15 20:03   ` [PATCH v2 1/2] acl: fix rules with 8 bytes field size are broken Konstantin Ananyev
@ 2022-05-15 20:03   ` Konstantin Ananyev
  2022-05-30 21:33   ` [PATCH v2 0/2] ACL fix 8B field Thomas Monjalon
  2 siblings, 0 replies; 8+ messages in thread
From: Konstantin Ananyev @ 2022-05-15 20:03 UTC (permalink / raw)
  To: dev; +Cc: ido

Within ACL rule IPv6 address can be represented in different ways:
either as 4x4B fields, or as 2x8B fields.
Till now, only first format was supported.
Extend test-acl to support both formats, mainly for testing and
demonstrating purposes.
To control desired behavior '--ipv6' command-line option is extend
to accept an optional argument:
To be more precise:
'--ipv6'    - use 4x4B fields format (default behavior)
'--ipv6=4B' - use 4x4B fields format (default behavior)
'--ipv6=8B' - use 2x8B fields format

Also replaced home brewed IPv4/IPv6 address parsing with inet_pton() calls.

Signed-off-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
---
 app/test-acl/main.c      | 355 ++++++++++++++++++++++++++++++---------
 app/test-acl/test-acl.sh |   4 +-
 2 files changed, 279 insertions(+), 80 deletions(-)

diff --git a/app/test-acl/main.c b/app/test-acl/main.c
index 06e3847ab9..41ce83db08 100644
--- a/app/test-acl/main.c
+++ b/app/test-acl/main.c
@@ -57,6 +57,12 @@ enum {
 	DUMP_MAX
 };
 
+enum {
+	IPV6_FRMT_NONE,
+	IPV6_FRMT_U32,
+	IPV6_FRMT_U64,
+};
+
 struct acl_alg {
 	const char *name;
 	enum rte_acl_classify_alg alg;
@@ -123,7 +129,7 @@ static struct {
 		.name = "default",
 		.alg = RTE_ACL_CLASSIFY_DEFAULT,
 	},
-	.ipv6 = 0
+	.ipv6 = IPV6_FRMT_NONE,
 };
 
 static struct rte_acl_param prm = {
@@ -210,6 +216,7 @@ struct rte_acl_field_def ipv4_defs[NUM_FIELDS_IPV4] = {
 #define	IPV6_ADDR_LEN	16
 #define	IPV6_ADDR_U16	(IPV6_ADDR_LEN / sizeof(uint16_t))
 #define	IPV6_ADDR_U32	(IPV6_ADDR_LEN / sizeof(uint32_t))
+#define	IPV6_ADDR_U64	(IPV6_ADDR_LEN / sizeof(uint64_t))
 
 struct ipv6_5tuple {
 	uint8_t  proto;
@@ -219,6 +226,7 @@ struct ipv6_5tuple {
 	uint16_t port_dst;
 };
 
+/* treat IPV6 address as uint32_t[4] (default mode) */
 enum {
 	PROTO_FIELD_IPV6,
 	SRC1_FIELD_IPV6,
@@ -234,6 +242,27 @@ enum {
 	NUM_FIELDS_IPV6
 };
 
+/* treat IPV6 address as uint64_t[2] (default mode) */
+enum {
+	PROTO_FIELD_IPV6_U64,
+	SRC1_FIELD_IPV6_U64,
+	SRC2_FIELD_IPV6_U64,
+	DST1_FIELD_IPV6_U64,
+	DST2_FIELD_IPV6_U64,
+	SRCP_FIELD_IPV6_U64,
+	DSTP_FIELD_IPV6_U64,
+	NUM_FIELDS_IPV6_U64
+};
+
+enum {
+	PROTO_INDEX_IPV6_U64 = PROTO_FIELD_IPV6_U64,
+	SRC1_INDEX_IPV6_U64 = SRC1_FIELD_IPV6_U64,
+	SRC2_INDEX_IPV6_U64 = SRC2_FIELD_IPV6_U64 + 1,
+	DST1_INDEX_IPV6_U64 = DST1_FIELD_IPV6_U64 + 2,
+	DST2_INDEX_IPV6_U64 = DST2_FIELD_IPV6_U64 + 3,
+	PRT_INDEX_IPV6_U64 = SRCP_FIELD_IPV6 + 4,
+};
+
 struct rte_acl_field_def ipv6_defs[NUM_FIELDS_IPV6] = {
 	{
 		.type = RTE_ACL_FIELD_TYPE_BITMASK,
@@ -314,6 +343,57 @@ struct rte_acl_field_def ipv6_defs[NUM_FIELDS_IPV6] = {
 	},
 };
 
+struct rte_acl_field_def ipv6_u64_defs[NUM_FIELDS_IPV6_U64] = {
+	{
+		.type = RTE_ACL_FIELD_TYPE_BITMASK,
+		.size = sizeof(uint8_t),
+		.field_index = PROTO_FIELD_IPV6_U64,
+		.input_index = PROTO_FIELD_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, proto),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_MASK,
+		.size = sizeof(uint64_t),
+		.field_index = SRC1_FIELD_IPV6_U64,
+		.input_index = SRC1_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, ip_src[0]),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_MASK,
+		.size = sizeof(uint64_t),
+		.field_index = SRC2_FIELD_IPV6_U64,
+		.input_index = SRC2_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, ip_src[2]),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_MASK,
+		.size = sizeof(uint64_t),
+		.field_index = DST1_FIELD_IPV6_U64,
+		.input_index = DST1_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, ip_dst[0]),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_MASK,
+		.size = sizeof(uint64_t),
+		.field_index = DST2_FIELD_IPV6_U64,
+		.input_index = DST2_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, ip_dst[2]),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_RANGE,
+		.size = sizeof(uint16_t),
+		.field_index = SRCP_FIELD_IPV6_U64,
+		.input_index = PRT_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, port_src),
+	},
+	{
+		.type = RTE_ACL_FIELD_TYPE_RANGE,
+		.size = sizeof(uint16_t),
+		.field_index = DSTP_FIELD_IPV6_U64,
+		.input_index = PRT_INDEX_IPV6_U64,
+		.offset = offsetof(struct ipv6_5tuple, port_dst),
+	},
+};
 
 enum {
 	CB_FLD_SRC_ADDR,
@@ -385,49 +465,11 @@ parse_cb_ipv4_trace(char *str, struct ipv4_5tuple *v)
 	return 0;
 }
 
-/*
- * Parse IPv6 address, expects the following format:
- * XXXX:XXXX:XXXX:XXXX:XXXX:XXXX:XXXX:XXXX (where X is a hexadecimal digit).
- */
-static int
-parse_ipv6_addr(const char *in, const char **end, uint32_t v[IPV6_ADDR_U32],
-	char dlm)
-{
-	uint32_t addr[IPV6_ADDR_U16];
-
-	GET_CB_FIELD(in, addr[0], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[1], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[2], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[3], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[4], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[5], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[6], 16, UINT16_MAX, ':');
-	GET_CB_FIELD(in, addr[7], 16, UINT16_MAX, dlm);
-
-	*end = in;
-
-	v[0] = (addr[0] << 16) + addr[1];
-	v[1] = (addr[2] << 16) + addr[3];
-	v[2] = (addr[4] << 16) + addr[5];
-	v[3] = (addr[6] << 16) + addr[7];
-
-	return 0;
-}
-
 static int
 parse_cb_ipv6_addr_trace(const char *in, uint32_t v[IPV6_ADDR_U32])
 {
-	int32_t rc;
-	const char *end;
-
-	rc = parse_ipv6_addr(in, &end, v, 0);
-	if (rc != 0)
-		return rc;
-
-	v[0] = rte_cpu_to_be_32(v[0]);
-	v[1] = rte_cpu_to_be_32(v[1]);
-	v[2] = rte_cpu_to_be_32(v[2]);
-	v[3] = rte_cpu_to_be_32(v[3]);
+	if (inet_pton(AF_INET6, in, v) != 1)
+		return -EINVAL;
 
 	return 0;
 }
@@ -548,20 +590,33 @@ tracef_init(void)
 }
 
 static int
-parse_ipv6_net(const char *in, struct rte_acl_field field[4])
+parse_ipv6_u32_net(char *in, struct rte_acl_field field[IPV6_ADDR_U32])
 {
-	int32_t rc;
-	const char *mp;
-	uint32_t i, m, v[4];
+	char *sa, *sm, *sv;
+	uint32_t i, m, v[IPV6_ADDR_U32];
+
+	const char *dlm = "/";
 	const uint32_t nbu32 = sizeof(uint32_t) * CHAR_BIT;
 
 	/* get address. */
-	rc = parse_ipv6_addr(in, &mp, v, '/');
-	if (rc != 0)
-		return rc;
+	sv = NULL;
+	sa = strtok_r(in, dlm, &sv);
+	if (sa == NULL)
+		return -EINVAL;
+	sm = strtok_r(NULL, dlm, &sv);
+	if (sm == NULL)
+		return -EINVAL;
+
+	if (inet_pton(AF_INET6, sa, v) != 1)
+		return -EINVAL;
+
+	v[0] = rte_be_to_cpu_32(v[0]);
+	v[1] = rte_be_to_cpu_32(v[1]);
+	v[2] = rte_be_to_cpu_32(v[2]);
+	v[3] = rte_be_to_cpu_32(v[3]);
 
 	/* get mask. */
-	GET_CB_FIELD(mp, m, 0, CHAR_BIT * sizeof(v), 0);
+	GET_CB_FIELD(sm, m, 0, CHAR_BIT * sizeof(v), 0);
 
 	/* put all together. */
 	for (i = 0; i != RTE_DIM(v); i++) {
@@ -569,7 +624,7 @@ parse_ipv6_net(const char *in, struct rte_acl_field field[4])
 			field[i].mask_range.u32 = nbu32;
 		else
 			field[i].mask_range.u32 = m > (i * nbu32) ?
-				m - (i * 32) : 0;
+				m - (i * nbu32) : 0;
 
 		field[i].value.u32 = v[i];
 	}
@@ -577,14 +632,88 @@ parse_ipv6_net(const char *in, struct rte_acl_field field[4])
 	return 0;
 }
 
+static int
+parse_ipv6_u64_net(char *in, struct rte_acl_field field[IPV6_ADDR_U64])
+{
+	char *sa, *sm, *sv;
+	uint32_t i, m;
+	uint64_t v[IPV6_ADDR_U64];
+
+	const char *dlm = "/";
+	const uint32_t nbu64 = sizeof(uint64_t) * CHAR_BIT;
+
+	/* get address. */
+	sv = NULL;
+	sa = strtok_r(in, dlm, &sv);
+	if (sa == NULL)
+		return -EINVAL;
+	sm = strtok_r(NULL, dlm, &sv);
+	if (sm == NULL)
+		return -EINVAL;
+
+	if (inet_pton(AF_INET6, sa, v) != 1)
+		return -EINVAL;
+
+	v[0] = rte_be_to_cpu_64(v[0]);
+	v[1] = rte_be_to_cpu_64(v[1]);
+
+	/* get mask. */
+	GET_CB_FIELD(sm, m, 0, CHAR_BIT * sizeof(v), 0);
+
+	/* put all together. */
+	for (i = 0; i != RTE_DIM(v); i++) {
+		if (m >= (i + 1) * nbu64)
+			field[i].mask_range.u32 = nbu64;
+		else
+			field[i].mask_range.u32 = m > (i * nbu64) ?
+				m - (i * nbu64) : 0;
+
+		field[i].value.u64 = v[i];
+	}
+
+	return 0;
+}
 
 static int
-parse_cb_ipv6_rule(char *str, struct acl_rule *v)
+parse_cb_ipv6_rule(char *str, struct acl_rule *v, int frmt)
 {
 	int i, rc;
+	uint32_t fidx;
+	const uint32_t *field_map;
 	char *s, *sp, *in[CB_FLD_NUM];
+	int (*parse_ipv6_net)(char *s, struct rte_acl_field f[]);
+
 	static const char *dlm = " \t\n";
 
+	static const uint32_t field_map_u32[CB_FLD_NUM] = {
+		[CB_FLD_SRC_ADDR] = SRC1_FIELD_IPV6,
+		[CB_FLD_DST_ADDR] = DST1_FIELD_IPV6,
+		[CB_FLD_SRC_PORT_LOW] = SRCP_FIELD_IPV6,
+		[CB_FLD_SRC_PORT_HIGH] = SRCP_FIELD_IPV6,
+		[CB_FLD_DST_PORT_LOW] = DSTP_FIELD_IPV6,
+		[CB_FLD_DST_PORT_HIGH] = DSTP_FIELD_IPV6,
+		[CB_FLD_PROTO] = PROTO_FIELD_IPV6,
+	};
+
+	static const uint32_t field_map_u64[CB_FLD_NUM] = {
+		[CB_FLD_SRC_ADDR] = SRC1_FIELD_IPV6_U64,
+		[CB_FLD_DST_ADDR] = DST1_FIELD_IPV6_U64,
+		[CB_FLD_SRC_PORT_LOW] = SRCP_FIELD_IPV6_U64,
+		[CB_FLD_SRC_PORT_HIGH] = SRCP_FIELD_IPV6_U64,
+		[CB_FLD_DST_PORT_LOW] = DSTP_FIELD_IPV6_U64,
+		[CB_FLD_DST_PORT_HIGH] = DSTP_FIELD_IPV6_U64,
+		[CB_FLD_PROTO] = PROTO_FIELD_IPV6_U64,
+	};
+
+	if (frmt == IPV6_FRMT_U32) {
+		field_map = field_map_u32;
+		parse_ipv6_net = parse_ipv6_u32_net;
+	} else if (frmt == IPV6_FRMT_U64) {
+		field_map = field_map_u64;
+		parse_ipv6_net = parse_ipv6_u64_net;
+	} else
+		return -ENOTSUP;
+
 	/*
 	 * Skip leading '@'
 	 */
@@ -600,28 +729,30 @@ parse_cb_ipv6_rule(char *str, struct acl_rule *v)
 		s = NULL;
 	}
 
-	rc = parse_ipv6_net(in[CB_FLD_SRC_ADDR], v->field + SRC1_FIELD_IPV6);
+	fidx = CB_FLD_SRC_ADDR;
+	rc = parse_ipv6_net(in[fidx], v->field + field_map[fidx]);
 	if (rc != 0) {
 		RTE_LOG(ERR, TESTACL,
-			"failed to read source address/mask: %s\n",
-			in[CB_FLD_SRC_ADDR]);
+			"failed to read source address/mask: %s\n", in[fidx]);
 		return rc;
 	}
 
-	rc = parse_ipv6_net(in[CB_FLD_DST_ADDR], v->field + DST1_FIELD_IPV6);
+	fidx = CB_FLD_DST_ADDR;
+	rc = parse_ipv6_net(in[fidx], v->field + field_map[fidx]);
 	if (rc != 0) {
 		RTE_LOG(ERR, TESTACL,
 			"failed to read destination address/mask: %s\n",
-			in[CB_FLD_DST_ADDR]);
+			in[fidx]);
 		return rc;
 	}
 
 	/* source port. */
-	GET_CB_FIELD(in[CB_FLD_SRC_PORT_LOW],
-		v->field[SRCP_FIELD_IPV6].value.u16,
+	fidx = CB_FLD_SRC_PORT_LOW;
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].value.u16,
 		0, UINT16_MAX, 0);
-	GET_CB_FIELD(in[CB_FLD_SRC_PORT_HIGH],
-		v->field[SRCP_FIELD_IPV6].mask_range.u16,
+
+	fidx = CB_FLD_SRC_PORT_HIGH;
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].mask_range.u16,
 		0, UINT16_MAX, 0);
 
 	if (strncmp(in[CB_FLD_SRC_PORT_DLM], cb_port_delim,
@@ -629,37 +760,61 @@ parse_cb_ipv6_rule(char *str, struct acl_rule *v)
 		return -EINVAL;
 
 	/* destination port. */
-	GET_CB_FIELD(in[CB_FLD_DST_PORT_LOW],
-		v->field[DSTP_FIELD_IPV6].value.u16,
+	fidx = CB_FLD_DST_PORT_LOW;
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].value.u16,
 		0, UINT16_MAX, 0);
-	GET_CB_FIELD(in[CB_FLD_DST_PORT_HIGH],
-		v->field[DSTP_FIELD_IPV6].mask_range.u16,
+
+	fidx = CB_FLD_DST_PORT_HIGH;
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].mask_range.u16,
 		0, UINT16_MAX, 0);
 
 	if (strncmp(in[CB_FLD_DST_PORT_DLM], cb_port_delim,
 			sizeof(cb_port_delim)) != 0)
 		return -EINVAL;
 
-	GET_CB_FIELD(in[CB_FLD_PROTO], v->field[PROTO_FIELD_IPV6].value.u8,
+	fidx = CB_FLD_PROTO;
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].value.u8,
 		0, UINT8_MAX, '/');
-	GET_CB_FIELD(in[CB_FLD_PROTO], v->field[PROTO_FIELD_IPV6].mask_range.u8,
+	GET_CB_FIELD(in[fidx], v->field[field_map[fidx]].mask_range.u8,
 		0, UINT8_MAX, 0);
 
 	return 0;
 }
 
 static int
-parse_ipv4_net(const char *in, uint32_t *addr, uint32_t *mask_len)
+parse_cb_ipv6_u32_rule(char *str, struct acl_rule *v)
 {
-	uint8_t a, b, c, d, m;
+	return parse_cb_ipv6_rule(str, v, IPV6_FRMT_U32);
+}
 
-	GET_CB_FIELD(in, a, 0, UINT8_MAX, '.');
-	GET_CB_FIELD(in, b, 0, UINT8_MAX, '.');
-	GET_CB_FIELD(in, c, 0, UINT8_MAX, '.');
-	GET_CB_FIELD(in, d, 0, UINT8_MAX, '/');
-	GET_CB_FIELD(in, m, 0, sizeof(uint32_t) * CHAR_BIT, 0);
+static int
+parse_cb_ipv6_u64_rule(char *str, struct acl_rule *v)
+{
+	return parse_cb_ipv6_rule(str, v, IPV6_FRMT_U64);
+}
+
+static int
+parse_ipv4_net(char *in, uint32_t *addr, uint32_t *mask_len)
+{
+	char *sa, *sm, *sv;
+	uint32_t m, v;
 
-	addr[0] = RTE_IPV4(a, b, c, d);
+	const char *dlm = "/";
+
+	sv = NULL;
+	sa = strtok_r(in, dlm, &sv);
+	if (sa == NULL)
+		return -EINVAL;
+	sm = strtok_r(NULL, dlm, &sv);
+	if (sm == NULL)
+		return -EINVAL;
+
+	if (inet_pton(AF_INET, sa, &v) != 1)
+		return -EINVAL;
+
+	addr[0] = rte_be_to_cpu_32(v);
+
+	GET_CB_FIELD(sm, m, 0, sizeof(uint32_t) * CHAR_BIT, 0);
 	mask_len[0] = m;
 
 	return 0;
@@ -757,8 +912,14 @@ add_cb_rules(FILE *f, struct rte_acl_ctx *ctx)
 	struct acl_rule v;
 	parse_5tuple parser;
 
+	static const parse_5tuple parser_func[] = {
+		[IPV6_FRMT_NONE] = parse_cb_ipv4_rule,
+		[IPV6_FRMT_U32] = parse_cb_ipv6_u32_rule,
+		[IPV6_FRMT_U64] = parse_cb_ipv6_u64_rule,
+	};
+
 	memset(&v, 0, sizeof(v));
-	parser = (config.ipv6 != 0) ? parse_cb_ipv6_rule : parse_cb_ipv4_rule;
+	parser = parser_func[config.ipv6];
 
 	k = 0;
 	for (i = 1; fgets(line, sizeof(line), f) != NULL; i++) {
@@ -804,9 +965,12 @@ acx_init(void)
 	memset(&cfg, 0, sizeof(cfg));
 
 	/* setup ACL build config. */
-	if (config.ipv6) {
+	if (config.ipv6 == IPV6_FRMT_U32) {
 		cfg.num_fields = RTE_DIM(ipv6_defs);
 		memcpy(&cfg.defs, ipv6_defs, sizeof(ipv6_defs));
+	} else if (config.ipv6 == IPV6_FRMT_U64) {
+		cfg.num_fields = RTE_DIM(ipv6_u64_defs);
+		memcpy(&cfg.defs, ipv6_u64_defs, sizeof(ipv6_u64_defs));
 	} else {
 		cfg.num_fields = RTE_DIM(ipv4_defs);
 		memcpy(&cfg.defs, ipv4_defs, sizeof(ipv4_defs));
@@ -958,6 +1122,37 @@ get_alg_opt(const char *opt, const char *name)
 		opt, name);
 }
 
+static void
+get_ipv6_opt(const char *opt, const char *name)
+{
+	uint32_t i;
+
+	static const struct {
+		const char *name;
+		uint32_t val;
+	} ipv6_opt[] = {
+		{
+			.name = "4B",
+			.val = IPV6_FRMT_U32,
+		},
+		{
+			.name = "8B",
+			.val = IPV6_FRMT_U64,
+		},
+	};
+
+	for (i = 0; i != RTE_DIM(ipv6_opt); i++) {
+		if (strcmp(opt, ipv6_opt[i].name) == 0) {
+			config.ipv6 = ipv6_opt[i].val;
+			return;
+		}
+	}
+
+	rte_exit(-EINVAL, "invalid value: \"%s\" for option: %s\n",
+		opt, name);
+}
+
+
 static void
 print_usage(const char *prgname)
 {
@@ -999,7 +1194,7 @@ print_usage(const char *prgname)
 		"[--" OPT_ITER_NUM "=<number of iterations to perform>]\n"
 		"[--" OPT_VERBOSE "=<verbose level>]\n"
 		"[--" OPT_SEARCH_ALG "=%s]\n"
-		"[--" OPT_IPV6 "=<IPv6 rules and trace files>]\n",
+		"[--" OPT_IPV6 "(=4B | 8B) <IPv6 rules and trace files>]\n",
 		prgname, RTE_ACL_RESULTS_MULTIPLIER,
 		(uint32_t)RTE_ACL_MAX_CATEGORIES,
 		buf);
@@ -1050,7 +1245,7 @@ get_input_opts(int argc, char **argv)
 		{OPT_ITER_NUM, 1, 0, 0},
 		{OPT_VERBOSE, 1, 0, 0},
 		{OPT_SEARCH_ALG, 1, 0, 0},
-		{OPT_IPV6, 0, 0, 0},
+		{OPT_IPV6, 2, 0, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -1099,7 +1294,9 @@ get_input_opts(int argc, char **argv)
 				OPT_SEARCH_ALG) == 0) {
 			get_alg_opt(optarg, lgopts[opt_idx].name);
 		} else if (strcmp(lgopts[opt_idx].name, OPT_IPV6) == 0) {
-			config.ipv6 = 1;
+			config.ipv6 = IPV6_FRMT_U32;
+			if (optarg != NULL)
+				get_ipv6_opt(optarg, lgopts[opt_idx].name);
 		}
 	}
 	config.trace_sz = config.ipv6 ? sizeof(struct ipv6_5tuple) :
diff --git a/app/test-acl/test-acl.sh b/app/test-acl/test-acl.sh
index c7bdc24113..30814f3fe2 100644
--- a/app/test-acl/test-acl.sh
+++ b/app/test-acl/test-acl.sh
@@ -86,7 +86,9 @@ for i in ${V4F}; do
 done
 
 for i in ${V6F}; do
-	XPRM='--ipv6'
+	XPRM='--ipv6=4B'
+	run_test $i
+	XPRM='--ipv6=8B'
 	run_test $i
 	unset XPRM
 done
-- 
2.34.1


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

* RE: [PATCH v2 1/2] acl: fix rules with 8 bytes field size are broken
  2022-05-15 20:03   ` [PATCH v2 1/2] acl: fix rules with 8 bytes field size are broken Konstantin Ananyev
@ 2022-05-16  6:23     ` Ido Goshen
  0 siblings, 0 replies; 8+ messages in thread
From: Ido Goshen @ 2022-05-16  6:23 UTC (permalink / raw)
  To: Konstantin Ananyev, dev; +Cc: stable

Tested-by: Ido Goshen <ido@cgstowernetworks.com>

> -----Original Message-----
> From: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> Sent: Sunday, 15 May 2022 23:03
> To: dev@dpdk.org
> Cc: Ido Goshen <Ido@cgstowernetworks.com>; stable@dpdk.org
> Subject: [PATCH v2 1/2] acl: fix rules with 8 bytes field size are broken
> 
> In theory ACL library allows fields with 8B long.
> Though in practice they usually not used, not tested, and as was revealed by Ido,
> this functionality is not working properly.
> There are few places inside ACL build code-path that need to be addressed.
> 
> Bugzilla ID: 673
> Fixes: dc276b5780c2 ("acl: new library")
> Cc: stable@dpdk.org
> 
> Reported-by: Ido Goshen <ido@cgstowernetworks.com>
> Signed-off-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> ---
>  lib/acl/acl_bld.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/acl/acl_bld.c b/lib/acl/acl_bld.c index 7ea30f4186..2816632803
> 100644
> --- a/lib/acl/acl_bld.c
> +++ b/lib/acl/acl_bld.c
> @@ -12,6 +12,9 @@
>  /* number of pointers per alloc */
>  #define ACL_PTR_ALLOC	32
> 
> +/* account for situation when all fields are 8B long */
> +#define ACL_MAX_INDEXES	(2 * RTE_ACL_MAX_FIELDS)
> +
>  /* macros for dividing rule sets heuristics */
>  #define NODE_MAX	0x4000
>  #define NODE_MIN	0x800
> @@ -80,7 +83,7 @@ struct acl_build_context {
>  	struct tb_mem_pool        pool;
>  	struct rte_acl_trie       tries[RTE_ACL_MAX_TRIES];
>  	struct rte_acl_bld_trie   bld_tries[RTE_ACL_MAX_TRIES];
> -	uint32_t
> data_indexes[RTE_ACL_MAX_TRIES][RTE_ACL_MAX_FIELDS];
> +	uint32_t
> data_indexes[RTE_ACL_MAX_TRIES][ACL_MAX_INDEXES];
> 
>  	/* memory free lists for nodes and blocks used for node ptrs */
>  	struct acl_mem_block      blocks[MEM_BLOCK_NUM];
> @@ -988,7 +991,7 @@ build_trie(struct acl_build_context *context, struct
> rte_acl_build_rule *head,
>  				 */
>  				uint64_t mask;
>  				mask = RTE_ACL_MASKLEN_TO_BITMASK(
> -					fld->mask_range.u32,
> +					fld->mask_range.u64,
>  					rule->config->defs[n].size);
> 
>  				/* gen a mini-trie for this field */ @@ -1301,6
> +1304,9 @@ acl_build_index(const struct rte_acl_config *config, uint32_t
> *data_index)
>  		if (last_header != config->defs[n].input_index) {
>  			last_header = config->defs[n].input_index;
>  			data_index[m++] = config->defs[n].offset;
> +			if (config->defs[n].size > sizeof(uint32_t))
> +				data_index[m++] = config->defs[n].offset +
> +					sizeof(uint32_t);
>  		}
>  	}
> 
> @@ -1487,7 +1493,7 @@ acl_set_data_indexes(struct rte_acl_ctx *ctx)
>  		memcpy(ctx->data_indexes + ofs, ctx->trie[i].data_index,
>  			n * sizeof(ctx->data_indexes[0]));
>  		ctx->trie[i].data_index = ctx->data_indexes + ofs;
> -		ofs += RTE_ACL_MAX_FIELDS;
> +		ofs += ACL_MAX_INDEXES;
>  	}
>  }
> 
> @@ -1643,7 +1649,7 @@ rte_acl_build(struct rte_acl_ctx *ctx, const struct
> rte_acl_config *cfg)
>  			/* allocate and fill run-time  structures. */
>  			rc = rte_acl_gen(ctx, bcx.tries, bcx.bld_tries,
>  				bcx.num_tries, bcx.cfg.num_categories,
> -				RTE_ACL_MAX_FIELDS * RTE_DIM(bcx.tries) *
> +				ACL_MAX_INDEXES * RTE_DIM(bcx.tries) *
>  				sizeof(ctx->data_indexes[0]), max_size);
>  			if (rc == 0) {
>  				/* set data indexes. */
> --
> 2.34.1


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

* Re: [PATCH v2 0/2] ACL fix 8B field
  2022-05-15 20:03 ` [PATCH v2 0/2] ACL fix 8B field Konstantin Ananyev
  2022-05-15 20:03   ` [PATCH v2 1/2] acl: fix rules with 8 bytes field size are broken Konstantin Ananyev
  2022-05-15 20:03   ` [PATCH v2 2/2] app/acl: support different formats for IPv6 address Konstantin Ananyev
@ 2022-05-30 21:33   ` Thomas Monjalon
  2 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2022-05-30 21:33 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, ido

> Konstantin Ananyev (2):
>   acl: fix rules with 8 bytes field size are broken
>   app/acl: support different formats for IPv6 address

Applied, thanks.




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

end of thread, other threads:[~2022-05-30 21:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-26 17:44 [PATCH 0/2] ACL fix 8B field Konstantin Ananyev
2022-04-26 17:44 ` [PATCH 1/2] acl: fix rules with 8 bytes field size are broken Konstantin Ananyev
2022-04-26 17:44 ` [PATCH 2/2] app/acl: support different formats for IPv6 address Konstantin Ananyev
2022-05-15 20:03 ` [PATCH v2 0/2] ACL fix 8B field Konstantin Ananyev
2022-05-15 20:03   ` [PATCH v2 1/2] acl: fix rules with 8 bytes field size are broken Konstantin Ananyev
2022-05-16  6:23     ` Ido Goshen
2022-05-15 20:03   ` [PATCH v2 2/2] app/acl: support different formats for IPv6 address Konstantin Ananyev
2022-05-30 21:33   ` [PATCH v2 0/2] ACL fix 8B field Thomas Monjalon

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).