[dpdk-dev,12/22] app/testpmd: add rte_flow item spec handler

Message ID 188971FCDA171749BED5DA74ABF3E6F03B6625B9@shsmsx102.ccr.corp.intel.com (mailing list archive)
State Not Applicable, archived
Headers

Checks

Context Check Description
ci/Intel compilation success Compilation OK

Commit Message

Pei, Yulong Dec. 16, 2016, 3:01 a.m. UTC
  Hi Adrien,

I try to setup the following rule, but it seems that after set 'spec'  param, can not set 'mask' param,  is it an issue here or am I wrong to use it ?

testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00
 dst [TOKEN]: destination MAC
 src [TOKEN]: source MAC
 type [TOKEN]: EtherType
 / [TOKEN]: specify next pattern item


Best Regards
Yulong Pei

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
Sent: Thursday, November 17, 2016 12:24 AM
To: dev@dpdk.org
Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Olivier Matz <olivier.matz@6wind.com>
Subject: [dpdk-dev] [PATCH 12/22] app/testpmd: add rte_flow item spec handler

Add parser code to fully set individual fields of pattern item specification structures, using the following operators:

- fix: sets field and applies full bit-mask for perfect matching.
- spec: sets field without modifying its bit-mask.
- last: sets upper value of the spec => last range.
- mask: sets bit-mask affecting both spec and last from arbitrary value.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 app/test-pmd/cmdline_flow.c | 110 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

 		.next = NEXT(next_item),
 		.call = parse_vc,
 	},
+	[ITEM_PARAM_FIX] = {
+		.name = "fix",
+		.help = "match value perfectly (with full bit-mask)",
+		.call = parse_vc_spec,
+	},
+	[ITEM_PARAM_SPEC] = {
+		.name = "spec",
+		.help = "match value according to configured bit-mask",
+		.call = parse_vc_spec,
+	},
+	[ITEM_PARAM_LAST] = {
+		.name = "last",
+		.help = "specify upper bound to establish a range",
+		.call = parse_vc_spec,
+	},
+	[ITEM_PARAM_MASK] = {
+		.name = "mask",
+		.help = "specify bit-mask with relevant bits set to one",
+		.call = parse_vc_spec,
+	},
 	[ITEM_NEXT] = {
 		.name = "/",
 		.help = "specify next pattern item",
@@ -605,6 +640,7 @@ parse_init(struct context *ctx, const struct token *token,
 	memset((uint8_t *)out + sizeof(*out), 0x22, size - sizeof(*out));
 	ctx->objdata = 0;
 	ctx->object = out;
+	ctx->objmask = NULL;
 	return len;
 }
 
@@ -632,11 +668,13 @@ parse_vc(struct context *ctx, const struct token *token,
 		out->command = ctx->curr;
 		ctx->objdata = 0;
 		ctx->object = out;
+		ctx->objmask = NULL;
 		out->args.vc.data = (uint8_t *)out + size;
 		return len;
 	}
 	ctx->objdata = 0;
 	ctx->object = &out->args.vc.attr;
+	ctx->objmask = NULL;
 	switch (ctx->curr) {
 	case GROUP:
 	case PRIORITY:
@@ -652,6 +690,7 @@ parse_vc(struct context *ctx, const struct token *token,
 			(void *)RTE_ALIGN_CEIL((uintptr_t)(out + 1),
 					       sizeof(double));
 		ctx->object = out->args.vc.pattern;
+		ctx->objmask = NULL;
 		return len;
 	case ACTIONS:
 		out->args.vc.actions =
@@ -660,6 +699,7 @@ parse_vc(struct context *ctx, const struct token *token,
 						out->args.vc.pattern_n),
 					       sizeof(double));
 		ctx->object = out->args.vc.actions;
+		ctx->objmask = NULL;
 		return len;
 	default:
 		if (!token->priv)
@@ -682,6 +722,7 @@ parse_vc(struct context *ctx, const struct token *token,
 		};
 		++out->args.vc.pattern_n;
 		ctx->object = item;
+		ctx->objmask = NULL;
 	} else {
 		const struct parse_action_priv *priv = token->priv;
 		struct rte_flow_action *action =
@@ -698,6 +739,7 @@ parse_vc(struct context *ctx, const struct token *token,
 		};
 		++out->args.vc.actions_n;
 		ctx->object = action;
+		ctx->objmask = NULL;
 	}
 	memset(data, 0, data_size);
 	out->args.vc.data = data;
@@ -705,6 +747,60 @@ parse_vc(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse pattern item parameter type. */ static int 
+parse_vc_spec(struct context *ctx, const struct token *token,
+	      const char *str, unsigned int len,
+	      void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+	struct rte_flow_item *item;
+	uint32_t data_size;
+	int index;
+	int objmask = 0;
+
+	(void)size;
+	/* Token name must match. */
+	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+		return -1;
+	/* Parse parameter types. */
+	switch (ctx->curr) {
+	case ITEM_PARAM_FIX:
+		index = 0;
+		objmask = 1;
+		break;
+	case ITEM_PARAM_SPEC:
+		index = 0;
+		break;
+	case ITEM_PARAM_LAST:
+		index = 1;
+		break;
+	case ITEM_PARAM_MASK:
+		index = 2;
+		break;
+	default:
+		return -1;
+	}
+	/* Nothing else to do if there is no buffer. */
+	if (!out)
+		return len;
+	if (!out->args.vc.pattern_n)
+		return -1;
+	item = &out->args.vc.pattern[out->args.vc.pattern_n - 1];
+	data_size = ctx->objdata / 3; /* spec, last, mask */
+	/* Point to selected object. */
+	ctx->object = out->args.vc.data + (data_size * index);
+	if (objmask) {
+		ctx->objmask = out->args.vc.data + (data_size * 2); /* mask */
+		item->mask = ctx->objmask;
+	} else
+		ctx->objmask = NULL;
+	/* Update relevant item pointer. */
+	*((const void **[]){ &item->spec, &item->last, &item->mask })[index] =
+		ctx->object;
+	return len;
+}
+
 /** Parse tokens for destroy command. */  static int  parse_destroy(struct context *ctx, const struct token *token, @@ -727,6 +823,7 @@ parse_destroy(struct context *ctx, const struct token *token,
 		out->command = ctx->curr;
 		ctx->objdata = 0;
 		ctx->object = out;
+		ctx->objmask = NULL;
 		out->args.destroy.rule =
 			(void *)RTE_ALIGN_CEIL((uintptr_t)(out + 1),
 					       sizeof(double));
@@ -737,6 +834,7 @@ parse_destroy(struct context *ctx, const struct token *token,
 		return -1;
 	ctx->objdata = 0;
 	ctx->object = out->args.destroy.rule + out->args.destroy.rule_n++;
+	ctx->objmask = NULL;
 	return len;
 }
 
@@ -762,6 +860,7 @@ parse_flush(struct context *ctx, const struct token *token,
 		out->command = ctx->curr;
 		ctx->objdata = 0;
 		ctx->object = out;
+		ctx->objmask = NULL;
 	}
 	return len;
 }
@@ -788,6 +887,7 @@ parse_query(struct context *ctx, const struct token *token,
 		out->command = ctx->curr;
 		ctx->objdata = 0;
 		ctx->object = out;
+		ctx->objmask = NULL;
 	}
 	return len;
 }
@@ -849,6 +949,7 @@ parse_list(struct context *ctx, const struct token *token,
 		out->command = ctx->curr;
 		ctx->objdata = 0;
 		ctx->object = out;
+		ctx->objmask = NULL;
 		out->args.list.group =
 			(void *)RTE_ALIGN_CEIL((uintptr_t)(out + 1),
 					       sizeof(double));
@@ -859,6 +960,7 @@ parse_list(struct context *ctx, const struct token *token,
 		return -1;
 	ctx->objdata = 0;
 	ctx->object = out->args.list.group + out->args.list.group_n++;
+	ctx->objmask = NULL;
 	return len;
 }
 
@@ -891,6 +993,7 @@ parse_int(struct context *ctx, const struct token *token,
 		return len;
 	buf = (uint8_t *)ctx->object + arg->offset;
 	size = arg->size;
+objmask:
 	switch (size) {
 	case sizeof(uint8_t):
 		*(uint8_t *)buf = u;
@@ -907,6 +1010,11 @@ parse_int(struct context *ctx, const struct token *token,
 	default:
 		goto error;
 	}
+	if (ctx->objmask && buf != (uint8_t *)ctx->objmask + arg->offset) {
+		u = -1;
+		buf = (uint8_t *)ctx->objmask + arg->offset;
+		goto objmask;
+	}
 	return len;
 error:
 	push_args(ctx, arg);
@@ -927,6 +1035,7 @@ parse_port(struct context *ctx, const struct token *token,
 	else {
 		ctx->objdata = 0;
 		ctx->object = out;
+		ctx->objmask = NULL;
 		size = sizeof(*out);
 	}
 	ret = parse_int(ctx, token, str, len, out, size); @@ -1033,6 +1142,7 @@ cmd_flow_context_init(struct context *ctx)
 	ctx->port = 0;
 	ctx->objdata = 0;
 	ctx->object = NULL;
+	ctx->objmask = NULL;
 }
 
 /** Parse a token (cmdline API). */
--
2.1.4
  

Comments

Adrien Mazarguil Dec. 16, 2016, 9:17 a.m. UTC | #1
Hi Yulong,

On Fri, Dec 16, 2016 at 03:01:15AM +0000, Pei, Yulong wrote:
> Hi Adrien,
> 
> I try to setup the following rule, but it seems that after set 'spec'  param, can not set 'mask' param,  is it an issue here or am I wrong to use it ?
> 
> testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00
>  dst [TOKEN]: destination MAC
>  src [TOKEN]: source MAC
>  type [TOKEN]: EtherType
>  / [TOKEN]: specify next pattern item

You need to re-specify dst with "mask" instead of "spec". You can specify it
as many times you like to update each structure in turn, e.g.:

 testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst mask 00:00:00:00:ff:ff

If you want to specify both spec and mask at once assuming you want it full,
these commands yield the same result:

 testpmd> flow create 0 ingress pattern eth dst fix 00:00:00:00:09:00
 testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst mask ff:ff:ff:ff:ff:ff
 testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst prefix 48

You are even allowed to change your mind:

 testpmd> flow create 0 ingress pattern eth dst fix 00:00:2a:2a:2a:2a dst fix 00:00:00:00:09:00

All these will be properly documented in the v2 patchset. Note, this version
will replace the "fix" keyword with "is" ("fix" made no sense according to
feedback).
  
Xing, Beilei Dec. 16, 2016, 12:22 p.m. UTC | #2
Thanks Adrien.

I have two questions:
1.  when I set " / vlan tci fix 10" with testpmd, I find the mask of tci is 0xFFFF.
     Actually tci includes PRI, CFI, and Vlan_id which holds 12 bits, so is it possible
     to set the mask to 0xFFF? 
     Our driver will check the mask only covers vlan_id instead of the whole tci.

2. When we test destroy function, we find the pointer provided to PMD is NULL
    instead of the pointer PMD returned to RTE during creating flow. Could you
    please have double check? Thanks.

Best Regards
Beilei

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Friday, December 16, 2016 5:18 PM
> To: Pei, Yulong <yulong.pei@intel.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas.monjalon@6wind.com>; De
> Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Olivier Matz
> <olivier.matz@6wind.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 12/22] app/testpmd: add rte_flow item spec
> handler
> 
> Hi Yulong,
> 
> On Fri, Dec 16, 2016 at 03:01:15AM +0000, Pei, Yulong wrote:
> > Hi Adrien,
> >
> > I try to setup the following rule, but it seems that after set 'spec'  param,
> can not set 'mask' param,  is it an issue here or am I wrong to use it ?
> >
> > testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00
> >  dst [TOKEN]: destination MAC
> >  src [TOKEN]: source MAC
> >  type [TOKEN]: EtherType
> >  / [TOKEN]: specify next pattern item
> 
> You need to re-specify dst with "mask" instead of "spec". You can specify it
> as many times you like to update each structure in turn, e.g.:
> 
>  testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst
> mask 00:00:00:00:ff:ff
> 
> If you want to specify both spec and mask at once assuming you want it full,
> these commands yield the same result:
> 
>  testpmd> flow create 0 ingress pattern eth dst fix 00:00:00:00:09:00  testpmd>
> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst mask
> ff:ff:ff:ff:ff:ff  testpmd> flow create 0 ingress pattern eth dst spec
> 00:00:00:00:09:00 dst prefix 48
> 
> You are even allowed to change your mind:
> 
>  testpmd> flow create 0 ingress pattern eth dst fix 00:00:2a:2a:2a:2a dst fix
> 00:00:00:00:09:00
> 
> All these will be properly documented in the v2 patchset. Note, this version
> will replace the "fix" keyword with "is" ("fix" made no sense according to
> feedback).
> 
> --
> Adrien Mazarguil
> 6WIND
  
Adrien Mazarguil Dec. 16, 2016, 3:25 p.m. UTC | #3
Hi Beilei,

On Fri, Dec 16, 2016 at 12:22:52PM +0000, Xing, Beilei wrote:
> Thanks Adrien.
> 
> I have two questions:
> 1.  when I set " / vlan tci fix 10" with testpmd, I find the mask of tci is 0xFFFF.
>      Actually tci includes PRI, CFI, and Vlan_id which holds 12 bits, so is it possible
>      to set the mask to 0xFFF? 
>      Our driver will check the mask only covers vlan_id instead of the whole tci.

Right, I'll work on a method to do that. TCI remains 16 bit either way so
the current approach remains accurate, although not convenient because a
10 bit mask must be specified manually.

This change won't be included in v2 though.

> 2. When we test destroy function, we find the pointer provided to PMD is NULL
>     instead of the pointer PMD returned to RTE during creating flow. Could you
>     please have double check? Thanks.

There is indeed a bug [1]. It is fixed in v2.

Thanks.

 [1] http://dpdk.org/ml/archives/dev/2016-November/050435.html
  

Patch

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c index e70e8e2..790b4b8 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -89,6 +89,10 @@  enum index {
 
 	/* Validate/create pattern. */
 	PATTERN,
+	ITEM_PARAM_FIX,
+	ITEM_PARAM_SPEC,
+	ITEM_PARAM_LAST,
+	ITEM_PARAM_MASK,
 	ITEM_NEXT,
 	ITEM_END,
 	ITEM_VOID,
@@ -121,6 +125,7 @@  struct context {
 	uint16_t port; /**< Current port ID (for completions). */
 	uint32_t objdata; /**< Object-specific data. */
 	void *object; /**< Address of current object for relative offsets. */
+	void *objmask; /**< Object a full mask must be written to. */
 };
 
 /** Token argument. */
@@ -267,6 +272,14 @@  static const enum index next_list_attr[] = {
 	0,
 };
 
+static const enum index item_param[] = {
+	ITEM_PARAM_FIX,
+	ITEM_PARAM_SPEC,
+	ITEM_PARAM_LAST,
+	ITEM_PARAM_MASK,
+	0,
+};
+
 static const enum index next_item[] = {
 	ITEM_END,
 	ITEM_VOID,
@@ -287,6 +300,8 @@  static int parse_init(struct context *, const struct token *,  static int parse_vc(struct context *, const struct token *,
 		    const char *, unsigned int,
 		    void *, unsigned int);
+static int parse_vc_spec(struct context *, const struct token *,
+			 const char *, unsigned int, void *, unsigned int);
 static int parse_destroy(struct context *, const struct token *,
 			 const char *, unsigned int,
 			 void *, unsigned int);
@@ -492,6 +507,26 @@  static const struct token token_list[] = {