[dpdk-dev] [PATCH] pipeline: relax table match field requirements

[dpdk-dev] [PATCH] pipeline: relax table match field requirements

[Cristian Dumitrescu]

The match fields for a given table have to be part of the same header
or the metadata structure. This commit removes the requirement that
the list of match fields must observe the order of fields within their
structure. For example, the h.ipv4.dst_addr field can now be listed
before the h.ipv4.src_addr field in a table match field list, even
though within the IPv4 header the dst_addr field is present after the
src_addr field.

Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 lib/librte_pipeline/rte_swx_ctl.c      |  43 ++++--
 lib/librte_pipeline/rte_swx_pipeline.c | 188 +++++++++++++++++++------
 2 files changed, 173 insertions(+), 58 deletions(-)

diff --git a/lib/librte_pipeline/rte_swx_ctl.c b/lib/librte_pipeline/rte_swx_ctl.c
index 4d1ff9ead..5d04e750f 100644
--- a/lib/librte_pipeline/rte_swx_ctl.c
+++ b/lib/librte_pipeline/rte_swx_ctl.c
@@ -38,6 +38,13 @@ struct action {
 struct table {
 	struct rte_swx_ctl_table_info info;
 	struct rte_swx_ctl_table_match_field_info *mf;
+
+	/* Match field with the smallest offset. */
+	struct rte_swx_ctl_table_match_field_info *mf_first;
+
+	/* Match field with the biggest offset. */
+	struct rte_swx_ctl_table_match_field_info *mf_last;
+
 	struct rte_swx_ctl_table_action_info *actions;
 	struct rte_swx_table_ops ops;
 	struct rte_swx_table_params params;
@@ -144,29 +151,39 @@ static int
 table_params_get(struct rte_swx_ctl_pipeline *ctl, uint32_t table_id)
 {
 	struct table *table = &ctl->tables[table_id];
+	struct rte_swx_ctl_table_match_field_info *first = NULL, *last = NULL;
 	uint8_t *key_mask = NULL;
 	enum rte_swx_table_match_type match_type = RTE_SWX_TABLE_MATCH_WILDCARD;
 	uint32_t key_size = 0, key_offset = 0, action_data_size = 0, i;
 
 	if (table->info.n_match_fields) {
-		struct rte_swx_ctl_table_match_field_info *first, *last;
-		uint32_t i;
+		uint32_t n_match_fields_em = 0, i;
 
+		/* Find first (smallest offset) and last (biggest offset) match fields. */
 		first = &table->mf[0];
-		last = &table->mf[table->info.n_match_fields - 1];
+		last = &table->mf[0];
+
+		for (i = 1; i < table->info.n_match_fields; i++) {
+			struct rte_swx_ctl_table_match_field_info *f = &table->mf[i];
+
+			if (f->offset < first->offset)
+				first = f;
+
+			if (f->offset > last->offset)
+				last = f;
+		}
 
 		/* match_type. */
 		for (i = 0; i < table->info.n_match_fields; i++) {
-			struct rte_swx_ctl_table_match_field_info *f;
+			struct rte_swx_ctl_table_match_field_info *f = &table->mf[i];
 
-			f = &table->mf[i];
-			if (f->match_type != RTE_SWX_TABLE_MATCH_EXACT)
-				break;
+			if (f->match_type == RTE_SWX_TABLE_MATCH_EXACT)
+				n_match_fields_em++;
 		}
 
-		if (i == table->info.n_match_fields)
+		if (n_match_fields_em == table->info.n_match_fields)
 			match_type = RTE_SWX_TABLE_MATCH_EXACT;
-		else if ((i == table->info.n_match_fields - 1) &&
+		else if ((n_match_fields_em == table->info.n_match_fields - 1) &&
 			 (last->match_type == RTE_SWX_TABLE_MATCH_LPM))
 			match_type = RTE_SWX_TABLE_MATCH_LPM;
 
@@ -181,11 +198,10 @@ table_params_get(struct rte_swx_ctl_pipeline *ctl, uint32_t table_id)
 		CHECK(key_mask, ENOMEM);
 
 		for (i = 0; i < table->info.n_match_fields; i++) {
-			struct rte_swx_ctl_table_match_field_info *f;
+			struct rte_swx_ctl_table_match_field_info *f = &table->mf[i];
 			uint32_t start;
 			size_t size;
 
-			f = &table->mf[i];
 			start = (f->offset - first->offset) / 8;
 			size = f->n_bits / 8;
 
@@ -210,6 +226,9 @@ table_params_get(struct rte_swx_ctl_pipeline *ctl, uint32_t table_id)
 	table->params.action_data_size = action_data_size;
 	table->params.n_keys_max = table->info.size;
 
+	table->mf_first = first;
+	table->mf_last = last;
+
 	return 0;
 }
 
@@ -1627,7 +1646,7 @@ rte_swx_ctl_pipeline_table_entry_read(struct rte_swx_ctl_pipeline *ctl,
 		struct rte_swx_ctl_table_match_field_info *mf = &table->mf[i];
 		char *mf_val = tokens[1 + i], *mf_mask = NULL;
 		uint64_t val, mask = UINT64_MAX;
-		uint32_t offset = (mf->offset - table->mf[0].offset) / 8;
+		uint32_t offset = (mf->offset - table->mf_first->offset) / 8;
 
 		/*
 		 * Mask.
diff --git a/lib/librte_pipeline/rte_swx_pipeline.c b/lib/librte_pipeline/rte_swx_pipeline.c
index d9f47b07e..aeb445755 100644
--- a/lib/librte_pipeline/rte_swx_pipeline.c
+++ b/lib/librte_pipeline/rte_swx_pipeline.c
@@ -767,7 +767,6 @@ struct table {
 	/* Match. */
 	struct match_field *fields;
 	uint32_t n_fields;
-	int is_header; /* Only valid when n_fields > 0. */
 	struct header *header; /* Only valid when n_fields > 0. */
 
 	/* Action. */
@@ -9099,24 +9098,131 @@ rte_swx_pipeline_table_type_register(struct rte_swx_pipeline *p,
 
 static enum rte_swx_table_match_type
 table_match_type_resolve(struct rte_swx_match_field_params *fields,
-			 uint32_t n_fields)
+			 uint32_t n_fields,
+			 uint32_t max_offset_field_id)
 {
-	uint32_t i;
+	uint32_t n_fields_em = 0, i;
 
 	for (i = 0; i < n_fields; i++)
-		if (fields[i].match_type != RTE_SWX_TABLE_MATCH_EXACT)
-			break;
+		if (fields[i].match_type == RTE_SWX_TABLE_MATCH_EXACT)
+			n_fields_em++;
 
-	if (i == n_fields)
+	if (n_fields_em == n_fields)
 		return RTE_SWX_TABLE_MATCH_EXACT;
 
-	if ((i == n_fields - 1) &&
-	    (fields[i].match_type == RTE_SWX_TABLE_MATCH_LPM))
+	if ((n_fields_em == n_fields - 1) &&
+	    (fields[max_offset_field_id].match_type == RTE_SWX_TABLE_MATCH_LPM))
 		return RTE_SWX_TABLE_MATCH_LPM;
 
 	return RTE_SWX_TABLE_MATCH_WILDCARD;
 }
 
+static int
+table_match_fields_check(struct rte_swx_pipeline *p,
+			 struct rte_swx_pipeline_table_params *params,
+			 struct header **header,
+			 uint32_t *min_offset_field_id,
+			 uint32_t *max_offset_field_id)
+{
+	struct header *h0 = NULL;
+	struct field *hf, *mf;
+	uint32_t *offset = NULL, min_offset, max_offset, min_offset_pos, max_offset_pos, i;
+	int status = 0;
+
+	/* Return if no match fields. */
+	if (!params->n_fields) {
+		if (params->fields) {
+			status = -EINVAL;
+			goto end;
+		}
+
+		return 0;
+	}
+
+	/* Memory allocation. */
+	offset = calloc(params->n_fields, sizeof(uint32_t));
+	if (!offset) {
+		status = -ENOMEM;
+		goto end;
+	}
+
+	/* Check that all the match fields belong to either the same header or
+	 * to the meta-data.
+	 */
+	hf = header_field_parse(p, params->fields[0].name, &h0);
+	mf = metadata_field_parse(p, params->fields[0].name);
+	if (!hf && !mf) {
+		status = -EINVAL;
+		goto end;
+	}
+
+	offset[0] = h0 ? hf->offset : mf->offset;
+
+	for (i = 1; i < params->n_fields; i++)
+		if (h0) {
+			struct header *h;
+
+			hf = header_field_parse(p, params->fields[i].name, &h);
+			if (!hf || (h->id != h0->id)) {
+				status = -EINVAL;
+				goto end;
+			}
+
+			offset[i] = hf->offset;
+		} else {
+			mf = metadata_field_parse(p, params->fields[i].name);
+			if (!mf) {
+				status = -EINVAL;
+				goto end;
+			}
+
+			offset[i] = mf->offset;
+		}
+
+	/* Check that there are no duplicated match fields. */
+	for (i = 0; i < params->n_fields; i++) {
+		uint32_t j;
+
+		for (j = 0; j < i; j++)
+			if (offset[j] == offset[i]) {
+				status = -EINVAL;
+				goto end;
+			}
+	}
+
+	/* Find the min and max offset fields. */
+	min_offset = offset[0];
+	max_offset = offset[0];
+	min_offset_pos = 0;
+	max_offset_pos = 0;
+
+	for (i = 1; i < params->n_fields; i++) {
+		if (offset[i] < min_offset) {
+			min_offset = offset[i];
+			min_offset_pos = i;
+		}
+
+		if (offset[i] > max_offset) {
+			max_offset = offset[i];
+			max_offset_pos = i;
+		}
+	}
+
+	/* Return. */
+	if (header)
+		*header = h0;
+
+	if (min_offset_field_id)
+		*min_offset_field_id = min_offset_pos;
+
+	if (max_offset_field_id)
+		*max_offset_field_id = max_offset_pos;
+
+end:
+	free(offset);
+	return status;
+}
+
 int
 rte_swx_pipeline_table_config(struct rte_swx_pipeline *p,
 			      const char *name,
@@ -9129,8 +9235,8 @@ rte_swx_pipeline_table_config(struct rte_swx_pipeline *p,
 	struct table *t;
 	struct action *default_action;
 	struct header *header = NULL;
-	int is_header = 0;
-	uint32_t offset_prev = 0, action_data_size_max = 0, i;
+	uint32_t action_data_size_max = 0, min_offset_field_id = 0, max_offset_field_id = 0, i;
+	int status = 0;
 
 	CHECK(p, EINVAL);
 
@@ -9140,35 +9246,13 @@ rte_swx_pipeline_table_config(struct rte_swx_pipeline *p,
 	CHECK(params, EINVAL);
 
 	/* Match checks. */
-	CHECK(!params->n_fields || params->fields, EINVAL);
-	for (i = 0; i < params->n_fields; i++) {
-		struct rte_swx_match_field_params *field = &params->fields[i];
-		struct header *h;
-		struct field *hf, *mf;
-		uint32_t offset;
-
-		CHECK_NAME(field->name, EINVAL);
-
-		hf = header_field_parse(p, field->name, &h);
-		mf = metadata_field_parse(p, field->name);
-		CHECK(hf || mf, EINVAL);
-
-		offset = hf ? hf->offset : mf->offset;
-
-		if (i == 0) {
-			is_header = hf ? 1 : 0;
-			header = hf ? h : NULL;
-			offset_prev = offset;
-
-			continue;
-		}
-
-		CHECK((is_header && hf && (h->id == header->id)) ||
-		      (!is_header && mf), EINVAL);
-
-		CHECK(offset > offset_prev, EINVAL);
-		offset_prev = offset;
-	}
+	status = table_match_fields_check(p,
+					  params,
+					  &header,
+					  &min_offset_field_id,
+					  &max_offset_field_id);
+	if (status)
+		return status;
 
 	/* Action checks. */
 	CHECK(params->n_actions, EINVAL);
@@ -9206,7 +9290,8 @@ rte_swx_pipeline_table_config(struct rte_swx_pipeline *p,
 		enum rte_swx_table_match_type match_type;
 
 		match_type = table_match_type_resolve(params->fields,
-						      params->n_fields);
+						      params->n_fields,
+						      max_offset_field_id);
 		type = table_type_resolve(p,
 					  recommended_table_type_name,
 					  match_type);
@@ -9253,12 +9338,11 @@ rte_swx_pipeline_table_config(struct rte_swx_pipeline *p,
 		struct match_field *f = &t->fields[i];
 
 		f->match_type = field->match_type;
-		f->field = is_header ?
+		f->field = header ?
 			header_field_parse(p, field->name, NULL) :
 			metadata_field_parse(p, field->name);
 	}
 	t->n_fields = params->n_fields;
-	t->is_header = is_header;
 	t->header = header;
 
 	for (i = 0; i < params->n_actions; i++)
@@ -9295,9 +9379,21 @@ table_params_get(struct table *table)
 	if (!params)
 		return NULL;
 
-	/* Key offset and size. */
+	/* Find first (smallest offset) and last (biggest offset) match fields. */
 	first = table->fields[0].field;
-	last = table->fields[table->n_fields - 1].field;
+	last = table->fields[0].field;
+
+	for (i = 0; i < table->n_fields; i++) {
+		struct field *f = table->fields[i].field;
+
+		if (f->offset < first->offset)
+			first = f;
+
+		if (f->offset > last->offset)
+			last = f;
+	}
+
+	/* Key offset and size. */
 	key_offset = first->offset / 8;
 	key_size = (last->offset + last->n_bits - first->offset) / 8;
 
@@ -9466,7 +9562,7 @@ table_build(struct rte_swx_pipeline *p)
 				}
 
 				/* r->key. */
-				r->key = table->is_header ?
+				r->key = table->header ?
 					&t->structs[table->header->struct_id] :
 					&t->structs[p->metadata_struct_id];
 			} else {
@@ -10143,7 +10239,7 @@ rte_swx_ctl_table_match_field_info_get(struct rte_swx_pipeline *p,
 
 	f = &t->fields[match_field_id];
 	match_field->match_type = f->match_type;
-	match_field->is_header = t->is_header;
+	match_field->is_header = t->header ? 1 : 0;
 	match_field->n_bits = f->field->n_bits;
 	match_field->offset = f->field->offset;
 
-- 
2.17.1

Re: [dpdk-dev] [PATCH] pipeline: relax table match field requirements

[Thomas Monjalon]

15/04/2021 23:15, Cristian Dumitrescu:
> The match fields for a given table have to be part of the same header
> or the metadata structure. This commit removes the requirement that
> the list of match fields must observe the order of fields within their
> structure. For example, the h.ipv4.dst_addr field can now be listed
> before the h.ipv4.src_addr field in a table match field list, even
> though within the IPv4 header the dst_addr field is present after the
> src_addr field.
> 
> Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Applied, thanks