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 44B15A034F for ; Mon, 6 Dec 2021 10:58:08 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3503840040; Mon, 6 Dec 2021 10:58:08 +0100 (CET) Received: from smtp-relay-internal-1.canonical.com (smtp-relay-internal-1.canonical.com [185.125.188.123]) by mails.dpdk.org (Postfix) with ESMTP id C030140040 for ; Mon, 6 Dec 2021 10:58:07 +0100 (CET) Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) (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-1.canonical.com (Postfix) with ESMTPS id 949303F1BD for ; Mon, 6 Dec 2021 09:58:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1638784687; bh=cknCQ7qgab06AbP4BNJX4fQM0xHWChlmT5vRjKV0xfw=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=kcZMuCbCVKt62znQKG73IlNVIKVd2kNUrjKnT6E1WwwmV/RRJJZm/SWKhPdCEPZZR KyR/g3xZsy6z2VXAvl6DeLmx+WJlq16pkojdGf/Ogyb5P0LkyMf3zLAVAJbr00m/yf AoPc7F7yVToirH6wkbGQs+ZgRnChwk0QVu/GYj5PRzXi16x15fem+oPBOZ13bQ/Nh3 eK/vrxJjG9mZut2OHjeWo3Q2wPSbIofsHOMeAtnKTNhtqQsUIOzyD9z6eswDfrYV2M LrMRWrLzBTwgVrI9+zcvcQRRfLm5XsJZ/Qtgk5T8JS5v7xsXJfhGtBJtghZewjLSRJ AyYU/ry7vmOFA== Received: by mail-qt1-f200.google.com with SMTP id a26-20020ac8001a000000b002b6596897dcso2306786qtg.19 for ; Mon, 06 Dec 2021 01:58:07 -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=cknCQ7qgab06AbP4BNJX4fQM0xHWChlmT5vRjKV0xfw=; b=MT1sMH65etiCo5+wi9A6aMnvsx4cemNPWb6iGuGIDjCBdzkrRDLPZ9JdQBDo0Cq4FU Y2fI+/omWnDWmymt0W5hUB2okx5XGAKJF2eUZvubyy0wH0zb8cVAJMSK0mAIRMelM7Kj 9X9aDo4B0T22rK08iRCc5KyBNABSgUKFh8+P52AORQGIg+W9hTsiwq++Z6xuQzQQWb+R yml2lKi9rkryl816isBiih/MZKA/6IlZaW+p+UFuC0ixAmOWqsP2ESSTcAV5oxmbulLV JI1LdNwvJd6c7pgIhloKVOBMztV6l0/Nd51o++wF06w3LNAUYFzuMHCEFIa5y5oFnEs2 i2tg== X-Gm-Message-State: AOAM533XrQmcniKGAtzH6Okgy1zJ4hw5xk/yqFgBlm4+alzTh0T7OEH8 K+RRh1+lY8Fuhb9TLqT8oVlJ3GTdQmDtzm+WpklIpruYBE/tH0sdnD29qKpFdMJpZ27zpkUxYzk CTS3zLf6/wo6vZydmevsMYDARzj4wZYSsGS8dIClm X-Received: by 2002:a05:620a:172c:: with SMTP id az44mr32438992qkb.93.1638784686467; Mon, 06 Dec 2021 01:58:06 -0800 (PST) X-Google-Smtp-Source: ABdhPJzz35UF7d6scYg1Pzev2hYHHk0zjvpJQZiqf+pEjyA4pRh6Tg147TwQryoY++5fP84JjQoNv2XjZsKj8wAb3jI= X-Received: by 2002:a05:620a:172c:: with SMTP id az44mr32438966qkb.93.1638784686193; Mon, 06 Dec 2021 01:58:06 -0800 (PST) MIME-Version: 1.0 References: <20211202160314.26638-1-getelson@nvidia.com> In-Reply-To: From: Christian Ehrhardt Date: Mon, 6 Dec 2021 10:57:40 +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 Mon, Dec 6, 2021 at 10:54 AM Christian Ehrhardt wrote: > > On Thu, Dec 2, 2021 at 5:04 PM Gregory Etelson wrot= e: > > > > [ 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_fl= ow_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, v= oid *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, oute= r_headers); > > + void *headers_v =3D MLX5_ADDR_OF(fte_match_param, key, outer_he= aders); > > void *misc_m =3D MLX5_ADDR_OF(fte_match_param, matcher, misc_pa= rameters); > > void *misc_v =3D MLX5_ADDR_OF(fte_match_param, key, misc_parame= ters); > > 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 =3D MLX5_ADDR_OF(fte_match_param, matcher, > > - inner_headers); > > - headers_v =3D MLX5_ADDR_OF(fte_match_param, key, inner_= headers); > > - } else { > > - headers_m =3D MLX5_ADDR_OF(fte_match_param, matcher, > > - outer_headers); > > - headers_v =3D 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 =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_= ver); > > gre_crks_rsvd0_ver_v.value =3D 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 =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_fla= gs); > > + } > > + 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 =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 = *key, > > .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_fla= gs); > > if (!nvgre_v) > > return; > > if (!nvgre_m) > > @@ -7788,11 +7790,10 @@ __flow_dv_translate(struct rte_eth_dev *dev, > > MLX5_FLOW_LAYER_OUTER_L4_U= DP; > > break; > > case RTE_FLOW_ITEM_TYPE_GRE: > > - flow_dv_translate_item_gre(match_mask, match_va= lue, > > - items, tunnel); > > matcher.priority =3D flow->rss.level >=3D 2 ? > > MLX5_PRIORITY_MAP_L2 : MLX5_PRIORIT= Y_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_= value, > > - items, tunnel); > > matcher.priority =3D flow->rss.level >=3D 2 ? > > MLX5_PRIORITY_MAP_L2 : MLX5_PRIORIT= Y_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_= 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 =3D=3D RTE_FLOW_ITEM_TYPE_GRE) > > + flow_dv_translate_item_gre(match_mask, match_va= lue, > > + tunnel_item, item_fl= ags); > > + else if (tunnel_item->type =3D=3D RTE_FLOW_ITEM_TYPE_NV= GRE) > > + 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 =E2=80=98__flow_dv_transl= ate=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 And dropping that makes the subsequent "net/mlx5: fix GRE flow item matching" fail to apply well, so I had to drop that as well for now. Let me know if you consider re-submitting updated backports of those two. > > + } > > 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 -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd