From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 38625A0555; Wed, 19 Feb 2020 16:30:39 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 180EB1BF87; Wed, 19 Feb 2020 16:30:39 +0100 (CET) Received: from EUR01-HE1-obe.outbound.protection.outlook.com (mail-eopbgr130044.outbound.protection.outlook.com [40.107.13.44]) by dpdk.org (Postfix) with ESMTP id 24A1DB62; Wed, 19 Feb 2020 16:30:37 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JHEn+ouEw9bcad0yXAQs++vlJiBko16d7PnldTyVQ91EV2j5JMQWpwXkcefqDaKMz2nqOwk+9j/XQATKrqfEWrv9dvJJCcCkeS90YQsmZZO5AC2GDAISs1tvlpzMPS9HV8GqSL1i1BGL5rxES70gYRXkcsMwBJ3tsDFN9atxhz5RXDZCUm8rM7dzuvRISmIWO0WfY+/X5qQYm7ag7B4dHi13/s2wCptelfakUrFVfl4hfNF9q/FlEix/pqccVaQgOO4v1gBIyF0vKMuJ6Z6Sp4ZXe3S9U/iPZV/DjS3qtIz8W19mNgnc6W0EgkOu3fsypxF8Q6VblCABFRllYL/I1Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=tkcPqIG9sHl71XVJxe3t/vHN7utlSA4aliEYtmvdvmE=; b=jMo0VW8Kw2lOkzBFBaC4BEeJ+I6v/8qmuCD7WMLd0qbDH7LiUByaE5/7lqrYTMExYqXQ78IRQ3of6DzAHOAChrldvXJ75DWCA5tFRqVjj9mMAeGLHRVC4N8fPpZWIGqpHT7Lo1UBqEmkQ0F8iLxcBbb82lToaSfxHQ5LmAWp/4NEnS75QGWQBKZjq7fL9yWquynWC6ygE8vcXYUqd+lCop+Kn1p/Y+fA6eRdwuOMOlsnO4Tp4g8QdQvYNHnzLICK1/vJtK7Bsb8zYD8EjgTFLEEup0jtHiym2fD+PqmJwKX/XFUgQkXRZ8tdWbNr9BkT7ysaFlzZI4vE0hOwcNn2rQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=mellanox.com; dmarc=pass action=none header.from=mellanox.com; dkim=pass header.d=mellanox.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=tkcPqIG9sHl71XVJxe3t/vHN7utlSA4aliEYtmvdvmE=; b=RqVOoNJ4np5liHb3cGzlkEJ4tyxNO3TEjX2WvuFXgXhHJHHcYeStIx0FZi4R3CwhcTbXbl1p+ygmHvOGFU1OCZC9xYgp2EW43y1DFXvcfjJWVYMyVaeCmV+F4fTdVWCXBDiPtMaGqrYl0UxbDekwY0nfmaQeAzfz9iQFd2XuvuI= Received: from AM0PR0502MB4019.eurprd05.prod.outlook.com (52.133.39.139) by AM0PR0502MB3716.eurprd05.prod.outlook.com (52.133.46.161) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2729.29; Wed, 19 Feb 2020 15:30:35 +0000 Received: from AM0PR0502MB4019.eurprd05.prod.outlook.com ([fe80::e495:9258:db09:1de8]) by AM0PR0502MB4019.eurprd05.prod.outlook.com ([fe80::e495:9258:db09:1de8%7]) with mapi id 15.20.2729.032; Wed, 19 Feb 2020 15:30:35 +0000 From: Matan Azrad To: Suanming Mou , Slava Ovsiienko CC: "dev@dpdk.org" , Raslan Darawsheh , "stable@dpdk.org" Thread-Topic: [PATCH 2/2] net/mlx5: fix lack of match information in meter Thread-Index: AQHV5zE9njMpyq20kk21ykT1bnLiTKgipAsA Date: Wed, 19 Feb 2020 15:30:35 +0000 Message-ID: References: <1582122664-54731-1-git-send-email-suanmingm@mellanox.com> <1582122664-54731-3-git-send-email-suanmingm@mellanox.com> In-Reply-To: <1582122664-54731-3-git-send-email-suanmingm@mellanox.com> Accept-Language: en-US, he-IL Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=matan@mellanox.com; x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 01891a6b-9ddb-4b48-8257-08d7b550aa17 x-ms-traffictypediagnostic: AM0PR0502MB3716:|AM0PR0502MB3716: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtFwd x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:4714; x-forefront-prvs: 0318501FAE x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(4636009)(346002)(39860400002)(396003)(366004)(376002)(136003)(189003)(199004)(30864003)(110136005)(54906003)(86362001)(2906002)(478600001)(55016002)(7696005)(316002)(186003)(52536014)(66946007)(66446008)(66556008)(64756008)(66476007)(6506007)(26005)(5660300002)(450100002)(8936002)(8676002)(81166006)(81156014)(4326008)(76116006)(9686003)(33656002)(6636002)(71200400001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM0PR0502MB3716; H:AM0PR0502MB4019.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 74XwcL7EKUn5xO6axBSAy3R/eKPuJXTLDOy9gTy2zBkRuX6sgObYdyIWCA6kJ6NnnhUTgSM6P1LwNZXB4BOzVuPvNqz+2WY/QhAUIv+XMbTRhCECnyUdkJDT/Df/MepWmmtJuizCI6GUbkTDavUWNks6I28zfjTfnoAqJbDFE32jPGqU1LlQ26pHrKDFQ/QlIp/hCMJY+nFhaMZkCsU/I3CMWldDMxSCX9TFFcq3jW4GxR5PrVQ0xcn/TkECbpNoHe2Dr8BTdh693WajKgFIPyRhrLxgJfGupU8iDn6SPNCavwRbEvA5dsvAgyySjntudqI8auUoTNpI7KSVXYFjTnJLOlBCmbZjyUyPYQ0692+2p8k5uTJxt9aVuK+UIN0HtmYdDfDysc7QrAPHvJUKQazr4J0KtEHdcH+BopxItMzS0MXnDMHna6CxWuPRx5n2 x-ms-exchange-antispam-messagedata: 3K95j2GUMUxp2cEZ0f6PBimdA0XvQIOJVUZtThtMvNMqcLf/a+36sjrWz55ZbWRhVGNFzSbFyQfKg3piYHemXwCy0oe/gPaSJxNiOEm6+o+ZRZJeBtb+Kc1RNnFCYfStL87JkJcalrWz6HaICWt+hg== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 01891a6b-9ddb-4b48-8257-08d7b550aa17 X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Feb 2020 15:30:35.0893 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 4zmRyn7FI5cIyCZVIDqe3nVsvGNajg1gQNYEzbTKgnXeoCVq6V2ng2enl0Kwo3aCnA8KQ1oh12UvIH75GcnMpg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR0502MB3716 Subject: Re: [dpdk-dev] [PATCH 2/2] net/mlx5: fix lack of match information in meter X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Mou One comment inline. From: Suanming Mou > As meter flows are split into three subflows each, the prefix subflow wit= h > meter action color the packet, the meter subflow filters out the colored > packets, the suffix subflow applies all the remaining actions to the pass= ed > packets. >=20 > Currently, all the user defined items are matched in the prefix flow. > Flow id tag match item is the only item added to the meter suffix subflow= . > Some of the remaining actions to be applied in the suffix subflow require > more information in the match item, or the suffix subflow will not be cre= ated > successfully. >=20 > Actions require the L3/L4 type in the match items as below: > RTE_FLOW_ACTION_TYPE_SET_TP_SRC > RTE_FLOW_ACTION_TYPE_SET_TP_DST > RTE_FLOW_ACTION_TYPE_DEC_TTL > RTE_FLOW_ACTION_TYPE_SET_TTL > RTE_FLOW_ACTION_TYPE_RSS > RTE_FLOW_ACTION_TYPE_QUEUE >=20 > Inherit the match item flags from meter prefix subflow to make actions in > suffix subflow get sufficient information. >=20 > Fixes: 9ea9b049a960 ("net/mlx5: split meter flow") > Cc: stable@dpdk.org >=20 > Signed-off-by: Suanming Mou > --- > drivers/net/mlx5/mlx5_flow.c | 22 +++++++++++---- > drivers/net/mlx5/mlx5_flow_dv.c | 62 > +++++++++++++++++++++++++++++++---------- > 2 files changed, 65 insertions(+), 19 deletions(-) >=20 > diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c > index 2f57759..f533f78 100644 > --- a/drivers/net/mlx5/mlx5_flow.c > +++ b/drivers/net/mlx5/mlx5_flow.c > @@ -3440,8 +3440,18 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > dev_flow->external =3D external; > /* Subflow object was created, we must include one in the list. */ > LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next); > - if (prefix_flow) > + if (prefix_flow) { > + /* > + * Some suffix subflow need the item flags and decap action > + * flag to know if actions in suffix flow should be applied > + * to new outer layer or not. > + * Action flag will be overwritten in tanslate after using. > + * No need to copy the hash_fields, as it will be generated > + * by RSS action with the item layer flags. > + */ > dev_flow->layers =3D prefix_flow->layers; > + dev_flow->actions =3D prefix_flow->actions; I don't think it is good to move the actions too. You just need to remove the outer layers here if you see decap, no? Now we have only one needed param, it is prefix layers only. please change all the commits to move to prefix_layers instead prefix_flow. in case of decap: Rss need the inner layers only and modify_actions need the inner layers onl= y. so you can just zero the outer layers. please add comment on it in translate - somethink like "layers can be initi= ated by the user according to the prefix sub-flow in case of meter and meta= data splitters". > + } > if (sub_flow) > *sub_flow =3D dev_flow; > return flow_drv_translate(dev, dev_flow, attr, items, actions, error); > @@ -3737,6 +3747,8 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > * Pointer to Ethernet device. > * @param[in] flow > * Parent flow structure pointer. > + * @param[in] prefix_flow > + * Prefix flow structure pointer. > * @param[in] attr > * Flow rule attributes. > * @param[in] items > @@ -3753,6 +3765,7 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, static int > flow_create_split_metadata(struct rte_eth_dev *dev, > struct rte_flow *flow, > + struct mlx5_flow *prefix_flow, > const struct rte_flow_attr *attr, > const struct rte_flow_item items[], > const struct rte_flow_action actions[], @@ -3773,7 > +3786,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, > int32_t priority, > if (!config->dv_flow_en || > config->dv_xmeta_en =3D=3D MLX5_XMETA_MODE_LEGACY || > !mlx5_flow_ext_mreg_supported(dev)) > - return flow_create_split_inner(dev, flow, NULL, NULL, > + return flow_create_split_inner(dev, flow, NULL, prefix_flow, > attr, items, actions, external, > error); > actions_n =3D flow_parse_qrss_action(actions, &qrss); @@ -3856,7 > +3869,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, > int32_t priority, > goto exit; > } > /* Add the unmodified original or prefix subflow. */ > - ret =3D flow_create_split_inner(dev, flow, &dev_flow, NULL, attr, > + ret =3D flow_create_split_inner(dev, flow, &dev_flow, prefix_flow, > attr, > items, ext_actions ? ext_actions : > actions, external, error); > if (ret < 0) > @@ -3892,7 +3905,6 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > .type =3D RTE_FLOW_ACTION_TYPE_END, > }, > }; > - struct mlx5_flow *prefix_flow; >=20 > /* > * Configure the tag item only if there is no meter subflow. > @@ -4052,7 +4064,7 @@ uint32_t mlx5_flow_adjust_priority(struct > rte_eth_dev *dev, int32_t priority, > MLX5_FLOW_TABLE_LEVEL_SUFFIX; > } > /* Add the prefix subflow. */ > - ret =3D flow_create_split_metadata(dev, flow, &sfx_attr, > + ret =3D flow_create_split_metadata(dev, flow, dev_flow, &sfx_attr, > sfx_items ? sfx_items : items, > sfx_actions ? sfx_actions : actions, > external, error); > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c > b/drivers/net/mlx5/mlx5_flow_dv.c index c958fca..359a037 100644 > --- a/drivers/net/mlx5/mlx5_flow_dv.c > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > @@ -85,16 +85,44 @@ > * Pointer to item specification. > * @param[out] attr > * Pointer to flow attributes structure. > + * @param[in] dev_flow > + * Pointer to the sub flow. > * @param[in] tunnel_decap > * Whether action is after tunnel decapsulation. > */ > static void > flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr > *attr, > - bool tunnel_decap) > + struct mlx5_flow *dev_flow, bool tunnel_decap) > { > + if (dev_flow->layers) { > + if (dev_flow->actions & MLX5_FLOW_ACTION_DECAP) { > + if (dev_flow->layers & > MLX5_FLOW_LAYER_INNER_L3_IPV4) > + attr->ipv4 =3D 1; > + else if (dev_flow->layers & > + MLX5_FLOW_LAYER_INNER_L3_IPV6) > + attr->ipv6 =3D 1; > + if (dev_flow->layers & > MLX5_FLOW_LAYER_INNER_L4_TCP) > + attr->tcp =3D 1; > + else if (dev_flow->layers & > + MLX5_FLOW_LAYER_INNER_L4_UDP) > + attr->udp =3D 1; > + } else { > + if (dev_flow->layers & > MLX5_FLOW_LAYER_OUTER_L3_IPV4) > + attr->ipv4 =3D 1; > + else if (dev_flow->layers & > + MLX5_FLOW_LAYER_OUTER_L3_IPV6) > + attr->ipv6 =3D 1; > + if (dev_flow->layers & > MLX5_FLOW_LAYER_OUTER_L4_TCP) > + attr->tcp =3D 1; > + else if (dev_flow->layers & > + MLX5_FLOW_LAYER_OUTER_L4_UDP) > + attr->udp =3D 1; > + } > + attr->valid =3D 1; > + return; > + } > for (; item->type !=3D RTE_FLOW_ITEM_TYPE_END; item++) { > uint8_t next_protocol =3D 0xff; > - > switch (item->type) { > case RTE_FLOW_ITEM_TYPE_GRE: > case RTE_FLOW_ITEM_TYPE_NVGRE: > @@ -640,6 +668,8 @@ struct field_modify_info modify_tcp[] =3D { > * Pointer to rte_flow_item objects list. > * @param[in] attr > * Pointer to flow attributes structure. > + * @param[in] dev_flow > + * Pointer to the sub flow. > * @param[in] tunnel_decap > * Whether action is after tunnel decapsulation. > * @param[out] error > @@ -653,8 +683,8 @@ struct field_modify_info modify_tcp[] =3D { > (struct mlx5_flow_dv_modify_hdr_resource > *resource, > const struct rte_flow_action *action, > const struct rte_flow_item *items, > - union flow_dv_attr *attr, bool tunnel_decap, > - struct rte_flow_error *error) > + union flow_dv_attr *attr, struct mlx5_flow > *dev_flow, > + bool tunnel_decap, struct rte_flow_error *error) > { > const struct rte_flow_action_set_tp *conf =3D > (const struct rte_flow_action_set_tp *)(action->conf); @@ - > 666,7 +696,7 @@ struct field_modify_info modify_tcp[] =3D { > struct field_modify_info *field; >=20 > if (!attr->valid) > - flow_dv_attr_init(items, attr, tunnel_decap); > + flow_dv_attr_init(items, attr, dev_flow, tunnel_decap); > if (attr->udp) { > memset(&udp, 0, sizeof(udp)); > memset(&udp_mask, 0, sizeof(udp_mask)); @@ -716,6 > +746,8 @@ struct field_modify_info modify_tcp[] =3D { > * Pointer to rte_flow_item objects list. > * @param[in] attr > * Pointer to flow attributes structure. > + * @param[in] dev_flow > + * Pointer to the sub flow. > * @param[in] tunnel_decap > * Whether action is after tunnel decapsulation. > * @param[out] error > @@ -729,8 +761,8 @@ struct field_modify_info modify_tcp[] =3D { > (struct mlx5_flow_dv_modify_hdr_resource > *resource, > const struct rte_flow_action *action, > const struct rte_flow_item *items, > - union flow_dv_attr *attr, bool tunnel_decap, > - struct rte_flow_error *error) > + union flow_dv_attr *attr, struct mlx5_flow > *dev_flow, > + bool tunnel_decap, struct rte_flow_error *error) > { > const struct rte_flow_action_set_ttl *conf =3D > (const struct rte_flow_action_set_ttl *)(action->conf); @@ - > 742,7 +774,7 @@ struct field_modify_info modify_tcp[] =3D { > struct field_modify_info *field; >=20 > if (!attr->valid) > - flow_dv_attr_init(items, attr, tunnel_decap); > + flow_dv_attr_init(items, attr, dev_flow, tunnel_decap); > if (attr->ipv4) { > memset(&ipv4, 0, sizeof(ipv4)); > memset(&ipv4_mask, 0, sizeof(ipv4_mask)); @@ -778,6 > +810,8 @@ struct field_modify_info modify_tcp[] =3D { > * Pointer to rte_flow_item objects list. > * @param[in] attr > * Pointer to flow attributes structure. > + * @param[in] dev_flow > + * Pointer to the sub flow. > * @param[in] tunnel_decap > * Whether action is after tunnel decapsulation. > * @param[out] error > @@ -790,8 +824,8 @@ struct field_modify_info modify_tcp[] =3D { > flow_dv_convert_action_modify_dec_ttl > (struct mlx5_flow_dv_modify_hdr_resource > *resource, > const struct rte_flow_item *items, > - union flow_dv_attr *attr, bool tunnel_decap, > - struct rte_flow_error *error) > + union flow_dv_attr *attr, struct mlx5_flow > *dev_flow, > + bool tunnel_decap, struct rte_flow_error *error) > { > struct rte_flow_item item; > struct rte_flow_item_ipv4 ipv4; > @@ -801,7 +835,7 @@ struct field_modify_info modify_tcp[] =3D { > struct field_modify_info *field; >=20 > if (!attr->valid) > - flow_dv_attr_init(items, attr, tunnel_decap); > + flow_dv_attr_init(items, attr, dev_flow, tunnel_decap); > if (attr->ipv4) { > memset(&ipv4, 0, sizeof(ipv4)); > memset(&ipv4_mask, 0, sizeof(ipv4_mask)); @@ -7505,7 > +7539,7 @@ struct field_modify_info modify_tcp[] =3D { > case RTE_FLOW_ACTION_TYPE_SET_TP_DST: > if (flow_dv_convert_action_modify_tp > (mhdr_res, actions, items, > - &flow_attr, !!(action_flags & > + &flow_attr, dev_flow, !!(action_flags > & > MLX5_FLOW_ACTION_DECAP), > error)) > return -rte_errno; > action_flags |=3D actions->type =3D=3D > @@ -7515,7 +7549,7 @@ struct field_modify_info modify_tcp[] =3D { > break; > case RTE_FLOW_ACTION_TYPE_DEC_TTL: > if (flow_dv_convert_action_modify_dec_ttl > - (mhdr_res, items, &flow_attr, > + (mhdr_res, items, &flow_attr, > dev_flow, > !!(action_flags & > MLX5_FLOW_ACTION_DECAP), > error)) > return -rte_errno; > @@ -7524,7 +7558,7 @@ struct field_modify_info modify_tcp[] =3D { > case RTE_FLOW_ACTION_TYPE_SET_TTL: > if (flow_dv_convert_action_modify_ttl > (mhdr_res, actions, items, > &flow_attr, > - !!(action_flags & > + dev_flow, !!(action_flags & > MLX5_FLOW_ACTION_DECAP), > error)) > return -rte_errno; > action_flags |=3D MLX5_FLOW_ACTION_SET_TTL; > -- > 1.8.3.1