From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 552ECA034F for ; Mon, 6 Dec 2021 10:54:29 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2F56D40040; Mon, 6 Dec 2021 10:54:29 +0100 (CET) Received: from smtp-relay-internal-0.canonical.com (smtp-relay-internal-0.canonical.com [185.125.188.122]) by mails.dpdk.org (Postfix) with ESMTP id D54AF40040 for ; Mon, 6 Dec 2021 10:54:27 +0100 (CET) Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id A4AA73F19E for ; Mon, 6 Dec 2021 09:54:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1638784467; bh=qfwbac3TulJ4g1mNvg7u8gRG2azarX8FZYAOFbr65wg=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=q9yOziIo0WfKKDsQik4UvxK1N+dtfi8Ua7izqvNBz3uCkQXB5zikUkDPY0tlukCUn M6B30vmp/ptN9q+BOQ3MFq8JJlrYW7B8ICak17rElqWox6Q3ry1h0Xi9KQc8AbiRgd m0OF1Zic9hlAzllwKnITNKIzFfgPAfzGuIUK6LXciDKho7+ACqfH/c5Qq2RU/puiTb OxE9m8yrWrVK6fM4OtMOLF4V6U54BxYu42emeXMzOEYqHGUo5lR0EF5HJyLzC+tFZ2 djS8MntXLHmmHMPv//kOrkq2nVmmlYWEnouY7GUYPuRqid5d0TaUP8u8QWwshopVs8 TTo5iUmdkctlg== Received: by mail-qv1-f71.google.com with SMTP id fn12-20020ad45d6c000000b003bd9c921c0eso11152456qvb.21 for ; Mon, 06 Dec 2021 01:54:27 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=qfwbac3TulJ4g1mNvg7u8gRG2azarX8FZYAOFbr65wg=; b=zpXgkwu+s2UO+WI/bUKF7KKUSx+f+1FJdgi0pjNCbp9PjAchOjukzVdJNBPK6AhGUu /nNm22PmjcoHEzlKyPPUNb88/c3arfDRt7Ov5HcNqaXxNa1wRVZk/cjwjx+x5uRM9RUP sjiLiQUxiSNOrNdtnb0I7OSLOS8rFMbrVT4lgURNtFJ3xZXdAbYL94VOhQuK7Xwqxbhb G05oEy1cvs557XOgKBwN0wuN78GNmvyHyuv0BATcpegqWootFnLSbePZltbv23HEcky3 uEW+5scwWMgHQucvuwDsBrmDkN81oQrHr1RieYhPhmXR4VEzfM8qE5tSo7mf/5z0AnUU 5r8Q== X-Gm-Message-State: AOAM530ly/K0cGLSGPwfgxqLf/cAd1x6U77UTye5VfRVgD/vapgyCqNm T+nA+jY2SN21R36kYklaHlCNtxxG5OxIGAiqqIV+l1HoNllUAIn7bbRk+Y9HkmzlaaJtYLZOD8K Z0GLChz2rI8F2QqGtLjXgUvACtAIfA/IOKcE1ytYH X-Received: by 2002:a05:6214:21ed:: with SMTP id p13mr35513673qvj.111.1638784466697; Mon, 06 Dec 2021 01:54:26 -0800 (PST) X-Google-Smtp-Source: ABdhPJxDI3gqDA77tswNzQu60iOiDlzqgSyZgrgGgQydlFzP0ZZ6dlWaqsxtkuXvtJ5JrTObI8T2NCl/65MG5aDSvPU= X-Received: by 2002:a05:6214:21ed:: with SMTP id p13mr35513641qvj.111.1638784466469; Mon, 06 Dec 2021 01:54:26 -0800 (PST) MIME-Version: 1.0 References: <20211202160314.26638-1-getelson@nvidia.com> In-Reply-To: <20211202160314.26638-1-getelson@nvidia.com> From: Christian Ehrhardt Date: Mon, 6 Dec 2021 10:54:00 +0100 Message-ID: Subject: Re: [PATCH 19.11 4/6] net/mlx5: fix GRE protocol type translation To: Gregory Etelson Cc: stable@dpdk.org, Viacheslav Ovsiienko , Matan Azrad , Shahaf Shuler , Ori Kam , Yongseok Koh Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On Thu, Dec 2, 2021 at 5:04 PM Gregory Etelson 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 > Acked-by: Viacheslav Ovsiienko > --- > 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, voi= d *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 =3D {0,}; > const struct rte_flow_item_gre *gre_m =3D item->mask; > const struct rte_flow_item_gre *gre_v =3D item->spec; > - void *headers_m; > - void *headers_v; > + void *headers_m =3D MLX5_ADDR_OF(fte_match_param, matcher, outer_= headers); > + void *headers_v =3D MLX5_ADDR_OF(fte_match_param, key, outer_head= ers); > void *misc_m =3D MLX5_ADDR_OF(fte_match_param, matcher, misc_para= meters); > void *misc_v =3D MLX5_ADDR_OF(fte_match_param, key, misc_paramete= rs); > struct { > @@ -5903,26 +5904,17 @@ flow_dv_translate_item_gre(void *matcher, void *k= ey, > uint16_t value; > }; > } gre_crks_rsvd0_ver_m, gre_crks_rsvd0_ver_v; > + uint16_t protocol_m, protocol_v; > > - if (inner) { > - headers_m =3D MLX5_ADDR_OF(fte_match_param, matcher, > - inner_headers); > - headers_v =3D MLX5_ADDR_OF(fte_match_param, key, inner_he= aders); > - } else { > - headers_m =3D MLX5_ADDR_OF(fte_match_param, matcher, > - outer_headers); > - headers_v =3D MLX5_ADDR_OF(fte_match_param, key, outer_he= aders); > - } > 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_G= RE); > - if (!gre_v) > - return; > - if (!gre_m) > - gre_m =3D &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 =3D &empty_gre; > + gre_m =3D &empty_gre; > + } else { > + if (!gre_m) > + gre_m =3D &rte_flow_item_gre_mask; > + } > gre_crks_rsvd0_ver_m.value =3D rte_be_to_cpu_16(gre_m->c_rsvd0_ve= r); > gre_crks_rsvd0_ver_v.value =3D rte_be_to_cpu_16(gre_v->c_rsvd0_ve= r); > MLX5_SET(fte_match_set_misc, misc_m, gre_c_present, > @@ -5940,6 +5932,16 @@ flow_dv_translate_item_gre(void *matcher, void *ke= y, > 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 =3D rte_be_to_cpu_16(gre_m->protocol); > + protocol_v =3D rte_be_to_cpu_16(gre_v->protocol); > + if (!protocol_m) { > + /* Force next protocol to prevent matchers duplication */ > + protocol_m =3D 0xFFFF; > + protocol_v =3D 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 *k= ey, > * 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 =3D item->mask; > const struct rte_flow_item_nvgre *nvgre_v =3D item->spec; > @@ -5984,7 +5986,7 @@ flow_dv_translate_item_nvgre(void *matcher, void *k= ey, > .mask =3D &gre_mask, > .last =3D 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_valu= e, > - items, tunnel); > matcher.priority =3D flow->rss.level >=3D 2 ? > MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_= MAP_L4; > last_item =3D MLX5_FLOW_LAYER_GRE; > + tunnel_item =3D 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 =3D MLX5_FLOW_LAYER_GRE_KEY; > break; > case RTE_FLOW_ITEM_TYPE_NVGRE: > - flow_dv_translate_item_nvgre(match_mask, match_va= lue, > - items, tunnel); > matcher.priority =3D flow->rss.level >=3D 2 ? > MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_= MAP_L4; > last_item =3D MLX5_FLOW_LAYER_GRE; > + tunnel_item =3D items; > break; > case RTE_FLOW_ITEM_TYPE_VXLAN: > flow_dv_translate_item_vxlan(match_mask, match_va= lue, > @@ -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 =3D=3D RTE_FLOW_ITEM_TYPE_GRE) > + flow_dv_translate_item_gre(match_mask, match_valu= e, > + tunnel_item, item_flag= s); > + else if (tunnel_item->type =3D=3D RTE_FLOW_ITEM_TYPE_NVGR= E) > + flow_dv_translate_item_nvgre(match_mask, match_va= lue, > + tunnel_item, item_fl= ags); > + 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 =E2=80=98__flow_dv_translat= e=E2=80=99: ../drivers/net/mlx5/mlx5_flow_dv.c:7912:4: warning: implicit declaration of function =E2=80=98MLX5_ASSERT=E2=80=99; did you mean =E2=80= =98MLX5_SET=E2=80=99? [-Wimplicit-function-declaration] 7912 | MLX5_ASSERT(false); | ^~~~~~~~~~~ | MLX5_SET ../drivers/net/mlx5/mlx5_flow_dv.c:7912:4: warning: nested extern declaration of =E2=80=98MLX5_ASSERT=E2=80=99 [-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 > --=20 Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd