From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
To: Gregory Etelson <getelson@nvidia.com>
Cc: stable@dpdk.org, Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
Matan Azrad <matan@nvidia.com>,
Shahaf Shuler <shahafs@nvidia.com>, Ori Kam <orika@mellanox.com>,
Yongseok Koh <yskoh@mellanox.com>
Subject: Re: [PATCH 19.11 4/6] net/mlx5: fix GRE protocol type translation
Date: Mon, 6 Dec 2021 10:54:00 +0100 [thread overview]
Message-ID: <CAATJJ0++cajKhW50jpXOuybCmLn_vV0PGnYq-hcgOgQ=JcxvtA@mail.gmail.com> (raw)
In-Reply-To: <20211202160314.26638-1-getelson@nvidia.com>
On Thu, Dec 2, 2021 at 5:04 PM Gregory Etelson <getelson@nvidia.com> wrote:
>
> [ upstream commit be26e81bfc1cb681d3ee0764aa6733000869984a ]
>
> When application creates several flows to match on GRE tunnel
> without explicitly specifying GRE protocol type value in
> flow rules, PMD will translate that to zero mask.
> RDMA-CORE cannot distinguish between different inner flow types and
> produces identical matchers for each zero mask.
>
> The patch extracts inner header type from flow rule and forces it
> in GRE protocol type, if application did not specify
> any without explicitly specifying GRE protocol type value in
> flow rules, protocol type value.
>
> Fixes: fc2c498ccb94 ("net/mlx5: add Direct Verbs translate items")
> Cc: stable@dpdk.org
>
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> ---
> drivers/net/mlx5/mlx5_flow_dv.c | 70 +++++++++++++++++++--------------
> 1 file changed, 40 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
> index 8dec8d9ff5..edbdba3e23 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -5875,18 +5875,19 @@ flow_dv_translate_item_gre_key(void *matcher, void *key,
> * Flow matcher value.
> * @param[in] item
> * Flow pattern to translate.
> - * @param[in] inner
> - * Item is inner pattern.
> + * @param[in] pattern_flags
> + * Accumulated pattern flags.
> */
> static void
> flow_dv_translate_item_gre(void *matcher, void *key,
> const struct rte_flow_item *item,
> - int inner)
> + uint64_t pattern_flags)
> {
> + static const struct rte_flow_item_gre empty_gre = {0,};
> const struct rte_flow_item_gre *gre_m = item->mask;
> const struct rte_flow_item_gre *gre_v = item->spec;
> - void *headers_m;
> - void *headers_v;
> + void *headers_m = MLX5_ADDR_OF(fte_match_param, matcher, outer_headers);
> + void *headers_v = MLX5_ADDR_OF(fte_match_param, key, outer_headers);
> void *misc_m = MLX5_ADDR_OF(fte_match_param, matcher, misc_parameters);
> void *misc_v = MLX5_ADDR_OF(fte_match_param, key, misc_parameters);
> struct {
> @@ -5903,26 +5904,17 @@ flow_dv_translate_item_gre(void *matcher, void *key,
> uint16_t value;
> };
> } gre_crks_rsvd0_ver_m, gre_crks_rsvd0_ver_v;
> + uint16_t protocol_m, protocol_v;
>
> - if (inner) {
> - headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> - inner_headers);
> - headers_v = MLX5_ADDR_OF(fte_match_param, key, inner_headers);
> - } else {
> - headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
> - outer_headers);
> - headers_v = MLX5_ADDR_OF(fte_match_param, key, outer_headers);
> - }
> MLX5_SET(fte_match_set_lyr_2_4, headers_m, ip_protocol, 0xff);
> MLX5_SET(fte_match_set_lyr_2_4, headers_v, ip_protocol, IPPROTO_GRE);
> - if (!gre_v)
> - return;
> - if (!gre_m)
> - gre_m = &rte_flow_item_gre_mask;
> - MLX5_SET(fte_match_set_misc, misc_m, gre_protocol,
> - rte_be_to_cpu_16(gre_m->protocol));
> - MLX5_SET(fte_match_set_misc, misc_v, gre_protocol,
> - rte_be_to_cpu_16(gre_v->protocol & gre_m->protocol));
> + if (!gre_v) {
> + gre_v = &empty_gre;
> + gre_m = &empty_gre;
> + } else {
> + if (!gre_m)
> + gre_m = &rte_flow_item_gre_mask;
> + }
> gre_crks_rsvd0_ver_m.value = rte_be_to_cpu_16(gre_m->c_rsvd0_ver);
> gre_crks_rsvd0_ver_v.value = rte_be_to_cpu_16(gre_v->c_rsvd0_ver);
> MLX5_SET(fte_match_set_misc, misc_m, gre_c_present,
> @@ -5940,6 +5932,16 @@ flow_dv_translate_item_gre(void *matcher, void *key,
> MLX5_SET(fte_match_set_misc, misc_v, gre_s_present,
> gre_crks_rsvd0_ver_v.s_present &
> gre_crks_rsvd0_ver_m.s_present);
> + protocol_m = rte_be_to_cpu_16(gre_m->protocol);
> + protocol_v = rte_be_to_cpu_16(gre_v->protocol);
> + if (!protocol_m) {
> + /* Force next protocol to prevent matchers duplication */
> + protocol_m = 0xFFFF;
> + protocol_v = mlx5_translate_tunnel_etypes(pattern_flags);
> + }
> + MLX5_SET(fte_match_set_misc, misc_m, gre_protocol, protocol_m);
> + MLX5_SET(fte_match_set_misc, misc_v, gre_protocol,
> + protocol_m & protocol_v);
> }
>
> /**
> @@ -5951,13 +5953,13 @@ flow_dv_translate_item_gre(void *matcher, void *key,
> * Flow matcher value.
> * @param[in] item
> * Flow pattern to translate.
> - * @param[in] inner
> - * Item is inner pattern.
> + * @param[in] pattern_flags
> + * Accumulated pattern flags.
> */
> static void
> flow_dv_translate_item_nvgre(void *matcher, void *key,
> const struct rte_flow_item *item,
> - int inner)
> + unsigned long pattern_flags)
> {
> const struct rte_flow_item_nvgre *nvgre_m = item->mask;
> const struct rte_flow_item_nvgre *nvgre_v = item->spec;
> @@ -5984,7 +5986,7 @@ flow_dv_translate_item_nvgre(void *matcher, void *key,
> .mask = &gre_mask,
> .last = NULL,
> };
> - flow_dv_translate_item_gre(matcher, key, &gre_item, inner);
> + flow_dv_translate_item_gre(matcher, key, &gre_item, pattern_flags);
> if (!nvgre_v)
> return;
> if (!nvgre_m)
> @@ -7788,11 +7790,10 @@ __flow_dv_translate(struct rte_eth_dev *dev,
> MLX5_FLOW_LAYER_OUTER_L4_UDP;
> break;
> case RTE_FLOW_ITEM_TYPE_GRE:
> - flow_dv_translate_item_gre(match_mask, match_value,
> - items, tunnel);
> matcher.priority = flow->rss.level >= 2 ?
> MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_MAP_L4;
> last_item = MLX5_FLOW_LAYER_GRE;
> + tunnel_item = items;
> break;
> case RTE_FLOW_ITEM_TYPE_GRE_KEY:
> flow_dv_translate_item_gre_key(match_mask,
> @@ -7800,11 +7801,10 @@ __flow_dv_translate(struct rte_eth_dev *dev,
> last_item = MLX5_FLOW_LAYER_GRE_KEY;
> break;
> case RTE_FLOW_ITEM_TYPE_NVGRE:
> - flow_dv_translate_item_nvgre(match_mask, match_value,
> - items, tunnel);
> matcher.priority = flow->rss.level >= 2 ?
> MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_MAP_L4;
> last_item = MLX5_FLOW_LAYER_GRE;
> + tunnel_item = items;
> break;
> case RTE_FLOW_ITEM_TYPE_VXLAN:
> flow_dv_translate_item_vxlan(match_mask, match_value,
> @@ -7892,6 +7892,16 @@ __flow_dv_translate(struct rte_eth_dev *dev,
> else if (item_flags & MLX5_FLOW_LAYER_GENEVE)
> flow_dv_translate_item_geneve(match_mask, match_value,
> tunnel_item, item_flags);
> + else if (item_flags & MLX5_FLOW_LAYER_GRE) {
> + if (tunnel_item->type == RTE_FLOW_ITEM_TYPE_GRE)
> + flow_dv_translate_item_gre(match_mask, match_value,
> + tunnel_item, item_flags);
> + else if (tunnel_item->type == RTE_FLOW_ITEM_TYPE_NVGRE)
> + flow_dv_translate_item_nvgre(match_mask, match_value,
> + tunnel_item, item_flags);
> + else
> + MLX5_ASSERT(false);
Hi,
build tests have shown that this breaks on 19.11 like:
[1479/2115] Compiling C object
drivers/libtmp_rte_pmd_mlx5.a.p/net_mlx5_mlx5_flow_dv.c.o
../drivers/net/mlx5/mlx5_flow_dv.c: In function ‘__flow_dv_translate’:
../drivers/net/mlx5/mlx5_flow_dv.c:7912:4: warning: implicit
declaration of function ‘MLX5_ASSERT’; did you mean ‘MLX5_SET’?
[-Wimplicit-function-declaration]
7912 | MLX5_ASSERT(false);
| ^~~~~~~~~~~
| MLX5_SET
../drivers/net/mlx5/mlx5_flow_dv.c:7912:4: warning: nested extern
declaration of ‘MLX5_ASSERT’ [-Wnested-externs]
[1538/2115] Linking target drivers/librte_pmd_mlx5.so.20.0
FAILED: drivers/librte_pmd_mlx5.so.20.0
For now - I'll remove the patch from the WIP 19.11 branch, please
consider submitting a backport adapted to work on
https://github.com/cpaelzer/dpdk-stable-queue/tree/19.11
> + }
> assert(!flow_dv_check_valid_spec(matcher.mask.buf,
> dev_flow->dv.value.buf));
> /*
> --
> 2.34.0
>
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
next prev parent reply other threads:[~2021-12-06 9:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-02 16:03 Gregory Etelson
2021-12-06 9:54 ` Christian Ehrhardt [this message]
2021-12-06 9:57 ` Christian Ehrhardt
2021-12-06 10:10 ` Gregory Etelson
2021-12-06 10:38 ` [PATCH 19.11 v2 1/2] " Gregory Etelson
2021-12-06 10:39 ` [PATCH 19.11 v2 2/2] net/mlx5: fix GRE flow item matching Gregory Etelson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAATJJ0++cajKhW50jpXOuybCmLn_vV0PGnYq-hcgOgQ=JcxvtA@mail.gmail.com' \
--to=christian.ehrhardt@canonical.com \
--cc=getelson@nvidia.com \
--cc=matan@nvidia.com \
--cc=orika@mellanox.com \
--cc=shahafs@nvidia.com \
--cc=stable@dpdk.org \
--cc=viacheslavo@nvidia.com \
--cc=yskoh@mellanox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).