* Re: [dpdk-stable] [PATCH] net/mlx5: fix matcher metadata register c0 field setup
2019-12-20 7:48 [dpdk-stable] [PATCH] net/mlx5: fix matcher metadata register c0 field setup Viacheslav Ovsiienko
@ 2020-01-06 14:50 ` Matan Azrad
2020-01-07 11:31 ` Raslan Darawsheh
2020-01-08 14:47 ` Ferruh Yigit
2020-01-17 11:01 ` [dpdk-stable] [PATCH v2] net/mlx5: fix shared metadata matcher " Viacheslav Ovsiienko
2 siblings, 1 reply; 7+ messages in thread
From: Matan Azrad @ 2020-01-06 14:50 UTC (permalink / raw)
To: Slava Ovsiienko, dev; +Cc: Raslan Darawsheh, Ori Kam, stable
From: Viacheslav Ovsiienko
> The metadata register c0 field in the matcher might be split into two
> independent fields - the source vport index and META item value. These
> fields have no permanent assigned bits, the configuration is queried from the
> kernel drivers.
>
> MLX5_SET configures the specified 32-bit field as whole entity.
> For metadata register c0 we should take into account the provided mask in
> order to configure the specified subfield bits only.
>
> Fixes: acfcd5c52f94 ("net/mlx5: update meta register matcher set")
> Cc: stable@dpdk.org
>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
> drivers/net/mlx5/mlx5_flow_dv.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index 4c16281..893db3e 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -5742,6 +5742,7 @@ struct field_modify_info modify_tcp[] = {
> MLX5_ADDR_OF(fte_match_param, matcher,
> misc_parameters_2);
> void *misc2_v =
> MLX5_ADDR_OF(fte_match_param, key,
> misc_parameters_2);
> + uint32_t temp;
>
> data &= mask;
> switch (reg_type) {
> @@ -5754,8 +5755,18 @@ struct field_modify_info modify_tcp[] = {
> MLX5_SET(fte_match_set_misc2, misc2_v, metadata_reg_b,
> data);
> break;
> case REG_C_0:
> - MLX5_SET(fte_match_set_misc2, misc2_m,
> metadata_reg_c_0, mask);
> - MLX5_SET(fte_match_set_misc2, misc2_v,
> metadata_reg_c_0, data);
> + /*
> + * The metadata register C0 field might be divided into
> + * source vport index and META item value, we should set
> + * this field accorfing to specified mask, not as whole one.
> + */
Typo - accorfing => according.
> + temp = MLX5_GET(fte_match_set_misc2, misc2_m,
> metadata_reg_c_0);
> + temp |= mask;
> + MLX5_SET(fte_match_set_misc2, misc2_m,
> metadata_reg_c_0, temp);
> + temp = MLX5_GET(fte_match_set_misc2, misc2_v,
> metadata_reg_c_0);
> + temp &= ~mask;
> + temp |= data;
> + MLX5_SET(fte_match_set_misc2, misc2_v,
> metadata_reg_c_0, temp);
> break;
> case REG_C_1:
> MLX5_SET(fte_match_set_misc2, misc2_m,
> metadata_reg_c_1, mask);
Raslan, please fix the typo in integration.
Acked-by: Matan Azrad <matan@mellanox.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-stable] [PATCH] net/mlx5: fix matcher metadata register c0 field setup
2020-01-06 14:50 ` Matan Azrad
@ 2020-01-07 11:31 ` Raslan Darawsheh
0 siblings, 0 replies; 7+ messages in thread
From: Raslan Darawsheh @ 2020-01-07 11:31 UTC (permalink / raw)
To: Matan Azrad, Slava Ovsiienko, dev; +Cc: Ori Kam, stable
Hi,
> -----Original Message-----
> From: Matan Azrad <matan@mellanox.com>
> Sent: Monday, January 6, 2020 4:50 PM
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
> Cc: Raslan Darawsheh <rasland@mellanox.com>; Ori Kam
> <orika@mellanox.com>; stable@dpdk.org
> Subject: RE: [PATCH] net/mlx5: fix matcher metadata register c0 field setup
>
>
>
> From: Viacheslav Ovsiienko
> > The metadata register c0 field in the matcher might be split into two
> > independent fields - the source vport index and META item value. These
> > fields have no permanent assigned bits, the configuration is queried
> > from the kernel drivers.
> >
> > MLX5_SET configures the specified 32-bit field as whole entity.
> > For metadata register c0 we should take into account the provided mask
> > in order to configure the specified subfield bits only.
> >
> > Fixes: acfcd5c52f94 ("net/mlx5: update meta register matcher set")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > ---
> > drivers/net/mlx5/mlx5_flow_dv.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> > b/drivers/net/mlx5/mlx5_flow_dv.c index 4c16281..893db3e 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -5742,6 +5742,7 @@ struct field_modify_info modify_tcp[] = {
> > MLX5_ADDR_OF(fte_match_param, matcher,
> misc_parameters_2);
> > void *misc2_v =
> > MLX5_ADDR_OF(fte_match_param, key,
> > misc_parameters_2);
> > + uint32_t temp;
> >
> > data &= mask;
> > switch (reg_type) {
> > @@ -5754,8 +5755,18 @@ struct field_modify_info modify_tcp[] = {
> > MLX5_SET(fte_match_set_misc2, misc2_v, metadata_reg_b,
> data);
> > break;
> > case REG_C_0:
> > - MLX5_SET(fte_match_set_misc2, misc2_m,
> > metadata_reg_c_0, mask);
> > - MLX5_SET(fte_match_set_misc2, misc2_v,
> > metadata_reg_c_0, data);
> > + /*
> > + * The metadata register C0 field might be divided into
> > + * source vport index and META item value, we should set
> > + * this field accorfing to specified mask, not as whole one.
> > + */
>
> Typo - accorfing => according.
>
> > + temp = MLX5_GET(fte_match_set_misc2, misc2_m,
> > metadata_reg_c_0);
> > + temp |= mask;
> > + MLX5_SET(fte_match_set_misc2, misc2_m,
> > metadata_reg_c_0, temp);
> > + temp = MLX5_GET(fte_match_set_misc2, misc2_v,
> > metadata_reg_c_0);
> > + temp &= ~mask;
> > + temp |= data;
> > + MLX5_SET(fte_match_set_misc2, misc2_v,
> > metadata_reg_c_0, temp);
> > break;
> > case REG_C_1:
> > MLX5_SET(fte_match_set_misc2, misc2_m,
> metadata_reg_c_1, mask);
>
> Raslan, please fix the typo in integration.
> Acked-by: Matan Azrad <matan@mellanox.com>
>
Fixed typo issue and applied patch to next-net-mlx.
Kindest regards,
Raslan Darawsheh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-stable] [PATCH] net/mlx5: fix matcher metadata register c0 field setup
2019-12-20 7:48 [dpdk-stable] [PATCH] net/mlx5: fix matcher metadata register c0 field setup Viacheslav Ovsiienko
2020-01-06 14:50 ` Matan Azrad
@ 2020-01-08 14:47 ` Ferruh Yigit
2020-01-17 15:04 ` Slava Ovsiienko
2020-01-17 11:01 ` [dpdk-stable] [PATCH v2] net/mlx5: fix shared metadata matcher " Viacheslav Ovsiienko
2 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2020-01-08 14:47 UTC (permalink / raw)
To: Viacheslav Ovsiienko, dev; +Cc: matan, rasland, orika, stable
On 12/20/2019 7:48 AM, Viacheslav Ovsiienko wrote:
> The metadata register c0 field in the matcher might be split
> into two independent fields - the source vport index and META
> item value. These fields have no permanent assigned bits, the
> configuration is queried from the kernel drivers.
>
> MLX5_SET configures the specified 32-bit field as whole entity.
> For metadata register c0 we should take into account the provided
> mask in order to configure the specified subfield bits only.
Hi Slava,
Is there a more human friendly name for the field instead of "C0"?
I don't know what "matcher metadata register" is, what is the impact of the fix?
Which functionality was broken now fixed? Does it make sense to reflect it in
the patch title / commit log?
Same comment for the related patches:
net/mlx5: fix register c0 usage for metadata entities
net/mlx5: fix metadata item endianness conversion
>
> Fixes: acfcd5c52f94 ("net/mlx5: update meta register matcher set")
> Cc: stable@dpdk.org
>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
> drivers/net/mlx5/mlx5_flow_dv.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index 4c16281..893db3e 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -5742,6 +5742,7 @@ struct field_modify_info modify_tcp[] = {
> MLX5_ADDR_OF(fte_match_param, matcher, misc_parameters_2);
> void *misc2_v =
> MLX5_ADDR_OF(fte_match_param, key, misc_parameters_2);
> + uint32_t temp;
>
> data &= mask;
> switch (reg_type) {
> @@ -5754,8 +5755,18 @@ struct field_modify_info modify_tcp[] = {
> MLX5_SET(fte_match_set_misc2, misc2_v, metadata_reg_b, data);
> break;
> case REG_C_0:
> - MLX5_SET(fte_match_set_misc2, misc2_m, metadata_reg_c_0, mask);
> - MLX5_SET(fte_match_set_misc2, misc2_v, metadata_reg_c_0, data);
> + /*
> + * The metadata register C0 field might be divided into
> + * source vport index and META item value, we should set
> + * this field accorfing to specified mask, not as whole one.
> + */
> + temp = MLX5_GET(fte_match_set_misc2, misc2_m, metadata_reg_c_0);
> + temp |= mask;
> + MLX5_SET(fte_match_set_misc2, misc2_m, metadata_reg_c_0, temp);
> + temp = MLX5_GET(fte_match_set_misc2, misc2_v, metadata_reg_c_0);
> + temp &= ~mask;
> + temp |= data;
> + MLX5_SET(fte_match_set_misc2, misc2_v, metadata_reg_c_0, temp);
> break;
> case REG_C_1:
> MLX5_SET(fte_match_set_misc2, misc2_m, metadata_reg_c_1, mask);
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-stable] [PATCH] net/mlx5: fix matcher metadata register c0 field setup
2020-01-08 14:47 ` Ferruh Yigit
@ 2020-01-17 15:04 ` Slava Ovsiienko
0 siblings, 0 replies; 7+ messages in thread
From: Slava Ovsiienko @ 2020-01-17 15:04 UTC (permalink / raw)
To: Ferruh Yigit, dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam, stable
Raslan, Ferruh
In these patches (merged in mlx5 tree) just the commit messages are updated:
http://patchwork.dpdk.org/patch/64831/
http://patchwork.dpdk.org/patch/64832/
http://patchwork.dpdk.org/patch/64852/
With best regards, Slava
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, January 8, 2020 16:47
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Ori Kam <orika@mellanox.com>; stable@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH] net/mlx5: fix matcher metadata register c0
> field setup
>
> On 12/20/2019 7:48 AM, Viacheslav Ovsiienko wrote:
> > The metadata register c0 field in the matcher might be split into two
> > independent fields - the source vport index and META item value. These
> > fields have no permanent assigned bits, the configuration is queried
> > from the kernel drivers.
> >
> > MLX5_SET configures the specified 32-bit field as whole entity.
> > For metadata register c0 we should take into account the provided mask
> > in order to configure the specified subfield bits only.
>
> Hi Slava,
>
> Is there a more human friendly name for the field instead of "C0"?
>
> I don't know what "matcher metadata register" is, what is the impact of the
> fix?
> Which functionality was broken now fixed? Does it make sense to reflect it in
> the patch title / commit log?
>
> Same comment for the related patches:
> net/mlx5: fix register c0 usage for metadata entities
> net/mlx5: fix metadata item endianness conversion
^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-stable] [PATCH v2] net/mlx5: fix shared metadata matcher field setup
2019-12-20 7:48 [dpdk-stable] [PATCH] net/mlx5: fix matcher metadata register c0 field setup Viacheslav Ovsiienko
2020-01-06 14:50 ` Matan Azrad
2020-01-08 14:47 ` Ferruh Yigit
@ 2020-01-17 11:01 ` Viacheslav Ovsiienko
2020-01-17 17:18 ` Ferruh Yigit
2 siblings, 1 reply; 7+ messages in thread
From: Viacheslav Ovsiienko @ 2020-01-17 11:01 UTC (permalink / raw)
To: dev; +Cc: matan, rasland, ferruh.yigit, stable
Matcher is flow table related structure providing the flow pattern
to be translated directly in hardware controlling data. Some fields
in this structure might be split (by software) between multiple items.
For example, the metadata register c0 field in the matcher might be
split into two independent subfields - the source vport index and
META item value. These subfields have no permanent assigned masks,
the actual configuration is queried from the kernel drivers in
runtime. To handle source vport value (the port of e-Switch which
is origin of the packet) the kernel might use the dedicated vport
field in the matcher or the part of register c0 field, depending
on configuration.
To setup the matcher structure fields the macro MLX5_SET is used.
MLX5_SET configures the specified 32-bit field as whole entity.
For metadata register c0 we should take into account the provided
mask in order to configure the specified subfield bits only,
otherwise setting vport overrides the META values and vice versa.
Fixes: acfcd5c52f94 ("net/mlx5: update meta register matcher set")
Cc: stable@dpdk.org
Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
v1: - http://patches.dpdk.org/patch/64068/
v2: - update commit message
- update headline
"net/mlx5: fix matcher metadata register c0 field setup"
drivers/net/mlx5/mlx5_flow_dv.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 5bd1b1c..2b70788 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5748,6 +5748,7 @@ struct field_modify_info modify_tcp[] = {
MLX5_ADDR_OF(fte_match_param, matcher, misc_parameters_2);
void *misc2_v =
MLX5_ADDR_OF(fte_match_param, key, misc_parameters_2);
+ uint32_t temp;
data &= mask;
switch (reg_type) {
@@ -5760,8 +5761,18 @@ struct field_modify_info modify_tcp[] = {
MLX5_SET(fte_match_set_misc2, misc2_v, metadata_reg_b, data);
break;
case REG_C_0:
- MLX5_SET(fte_match_set_misc2, misc2_m, metadata_reg_c_0, mask);
- MLX5_SET(fte_match_set_misc2, misc2_v, metadata_reg_c_0, data);
+ /*
+ * The metadata register C0 field might be divided into
+ * source vport index and META item value, we should set
+ * this field according to specified mask, not as whole one.
+ */
+ temp = MLX5_GET(fte_match_set_misc2, misc2_m, metadata_reg_c_0);
+ temp |= mask;
+ MLX5_SET(fte_match_set_misc2, misc2_m, metadata_reg_c_0, temp);
+ temp = MLX5_GET(fte_match_set_misc2, misc2_v, metadata_reg_c_0);
+ temp &= ~mask;
+ temp |= data;
+ MLX5_SET(fte_match_set_misc2, misc2_v, metadata_reg_c_0, temp);
break;
case REG_C_1:
MLX5_SET(fte_match_set_misc2, misc2_m, metadata_reg_c_1, mask);
--
1.8.3.1
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [dpdk-stable] [PATCH v2] net/mlx5: fix shared metadata matcher field setup
2020-01-17 11:01 ` [dpdk-stable] [PATCH v2] net/mlx5: fix shared metadata matcher " Viacheslav Ovsiienko
@ 2020-01-17 17:18 ` Ferruh Yigit
0 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2020-01-17 17:18 UTC (permalink / raw)
To: Viacheslav Ovsiienko, dev; +Cc: matan, rasland, stable
On 1/17/2020 11:01 AM, Viacheslav Ovsiienko wrote:
> Matcher is flow table related structure providing the flow pattern
> to be translated directly in hardware controlling data. Some fields
> in this structure might be split (by software) between multiple items.
>
> For example, the metadata register c0 field in the matcher might be
> split into two independent subfields - the source vport index and
> META item value. These subfields have no permanent assigned masks,
> the actual configuration is queried from the kernel drivers in
> runtime. To handle source vport value (the port of e-Switch which
> is origin of the packet) the kernel might use the dedicated vport
> field in the matcher or the part of register c0 field, depending
> on configuration.
>
> To setup the matcher structure fields the macro MLX5_SET is used.
> MLX5_SET configures the specified 32-bit field as whole entity.
> For metadata register c0 we should take into account the provided
> mask in order to configure the specified subfield bits only,
> otherwise setting vport overrides the META values and vice versa.
>
> Fixes: acfcd5c52f94 ("net/mlx5: update meta register matcher set")
> Cc: stable@dpdk.org
>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Acked-by: Matan Azrad <matan@mellanox.com>
> ---
> v1: - http://patches.dpdk.org/patch/64068/
> v2: - update commit message
> - update headline
> "net/mlx5: fix matcher metadata register c0 field setup"
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread