From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yongseok Koh Subject: Re: [PATCH 4/4] net/mlx5: add DV decap L2 and L3 operations Date: Wed, 3 Oct 2018 07:07:25 +0000 Message-ID: <20181003070716.GA21743@mtidpdk.mti.labs.mlnx> References: <1538059845-35896-1-git-send-email-dekelp@mellanox.com> <1538059845-35896-5-git-send-email-dekelp@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , Shahaf Shuler , Ori Kam To: Dekel Peled Return-path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0047.outbound.protection.outlook.com [104.47.1.47]) by dpdk.org (Postfix) with ESMTP id 40E6D2C36 for ; Wed, 3 Oct 2018 09:07:27 +0200 (CEST) In-Reply-To: <1538059845-35896-5-git-send-email-dekelp@mellanox.com> Content-Language: en-US Content-ID: <1ECEF28AF058494E9330E8165E47AD88@eurprd05.prod.outlook.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Sep 27, 2018 at 05:50:45PM +0300, Dekel Peled wrote: > This patch adds support for Direct Verbs decap operations, L2 and L3. >=20 Almost same code as encap, so I also have the same comments. :-) One more. In the title, we can't use vendor specific acronyms. net/mlx5: add DV decap L2 and L3 operations DV -> Direct Verbs How about something like this? net/mlx5: add L2/L3 encap to Direct Verbs flow > Signed-off-by: Dekel Peled > --- > drivers/net/mlx5/mlx5_flow_dv.c | 258 ++++++++++++++++++++++++++++++++++= ++++-- > 1 file changed, 248 insertions(+), 10 deletions(-) >=20 > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow= _dv.c > index 50925ac..ca44f13 100644 > --- a/drivers/net/mlx5/mlx5_flow_dv.c > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > @@ -126,12 +126,20 @@ > NULL, > "can't have 2 encap actions in same" > " flow"); > + if (action_flags & (MLX5_ACTION_TUNNEL_DECAP | > + MLX5_ACTION_TUNNEL_DECAP_L3)) > + return rte_flow_error_set(error, > + ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ACTION, > + NULL, > + "can't encap and decap in same" > + " flow"); > if (attr->ingress) > return rte_flow_error_set(error, ENOTSUP, > - RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > - NULL, > - "encap action not supported for " > - "ingress"); > + RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > + NULL, > + "encap action not supported for " > + "ingress"); > return 0; > } > =20 > @@ -178,12 +186,130 @@ > NULL, > "can't have 2 encap actions in same" > " flow"); > + if (action_flags & (MLX5_ACTION_TUNNEL_DECAP | > + MLX5_ACTION_TUNNEL_DECAP_L3)) > + return rte_flow_error_set(error, > + ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ACTION, > + NULL, > + "can't encap and decap in same" > + " flow"); > if (attr->ingress) > return rte_flow_error_set(error, ENOTSUP, > - RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > - NULL, > - "encap action not supported for " > - "ingress"); > + RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > + NULL, > + "encap action not supported for " > + "ingress"); > + return 0; > +} > + > +/** > + * Validate the tunnel decap action > + * > + * @param[in] action_flags > + * holds the actions detected until now. > + * @param[in] attr > + * Pointer to flow attributes > + * @param[out] error > + * Pointer to error structure. > + * > + * @return > + * 0 on success, a negative errno value otherwise and rte_ernno is set= . > + */ > +static int > +mlx5_flow_validate_action_tunnel_decap(uint64_t action_flags, > + const struct rte_flow_attr *attr, > + struct rte_flow_error *error) > +{ > + if (action_flags & MLX5_ACTION_DROP) > + return rte_flow_error_set(error, > + ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ACTION, > + NULL, > + "can't drop and decap in same flow"); > + if (action_flags & (MLX5_ACTION_TUNNEL_DECAP | > + MLX5_ACTION_TUNNEL_DECAP_L3)) > + return rte_flow_error_set(error, > + ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ACTION, > + NULL, > + "can't have 2 decap actions in same" > + " flow"); > + if (action_flags & (MLX5_ACTION_TUNNEL_ENCAP | > + MLX5_ACTION_TUNNEL_ENCAP_L3)) > + return rte_flow_error_set(error, > + ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ACTION, > + NULL, > + "can't encap and decap in same" > + " flow"); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > + NULL, > + "decap action not supported for " > + "egress"); > + return 0; > +} > + > +/** > + * Validate the tunnel decap L3 action > + * > + * @param[in] action_flags > + * holds the actions detected until now. > + * @param[in] action > + * Pointer to the decap action. > + * @param[in] attr > + * Pointer to flow attributes > + * @param[out] error > + * Pointer to error structure. > + * > + * @return > + * 0 on success, a negative errno value otherwise and rte_ernno is set= . > + */ > +static int > +mlx5_flow_validate_action_tunnel_decap_l3(uint64_t action_flags, > + const struct rte_flow_action *action, > + const struct rte_flow_attr *attr, > + struct rte_flow_error *error) > +{ > + const struct rte_flow_action_tunnel_decap_l3 *tunnel_decap_l3 =3D > + action->conf; > + > + if (!tunnel_decap_l3 || !(tunnel_decap_l3->buf)) > + > + return rte_flow_error_set(error, EINVAL, > + RTE_FLOW_ERROR_TYPE_ACTION, > + action, > + "configuration cannot be null"); > + if (action_flags & MLX5_ACTION_DROP) > + return rte_flow_error_set(error, > + ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ACTION, > + NULL, > + "can't drop and decap in same flow"); > + if (action_flags & (MLX5_ACTION_TUNNEL_DECAP | > + MLX5_ACTION_TUNNEL_DECAP_L3)) > + return rte_flow_error_set(error, > + ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ACTION, > + NULL, > + "can't have 2 decap actions in same" > + " flow"); > + if (action_flags & (MLX5_ACTION_TUNNEL_ENCAP | > + MLX5_ACTION_TUNNEL_ENCAP_L3)) > + return rte_flow_error_set(error, > + ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ACTION, > + NULL, > + "can't encap and decap in same" > + " flow"); > + if (attr->egress) > + return rte_flow_error_set(error, ENOTSUP, > + RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, > + NULL, > + "decap action not supported for " > + "egress"); > return 0; > } > =20 > @@ -257,6 +383,72 @@ > } > =20 > /** > + * Convert decap action to DV specification. > + * > + * @param[in] action > + * Pointer to action structure. > + * @param[in] ctx > + * Verbs context. > + * @param[out] error > + * Pointer to the error structure. > + * > + * @return > + * Pointer to action on success, NULL otherwise and rte_errno is set. > + */ > +static struct ibv_flow_action * > +mlx5_flow_dv_create_decap(const struct rte_flow_action *action __rte_unu= sed, > + struct ibv_context *ctx, > + struct rte_flow_error *error) > +{ > + struct ibv_flow_action *decap_verb =3D NULL; > + > + decap_verb =3D mlx5_glue->dv_create_flow_action_packet_reformat(ctx, > + 0, NULL, > + MLX5DV_FLOW_ACTION_PACKET_REFORMAT_TYPE_L2_TUNNEL_TO_L2, > + MLX5DV_FLOW_TABLE_TYPE_NIC_RX); > + if (!decap_verb) > + rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION, > + NULL, "cannot create decap action"); > + return decap_verb; > +} > + > +/** > + * Convert decap L3 action to DV specification. > + * > + * @param[in] action > + * Pointer to action structure. > + * @param[in] ctx > + * Verbs context. > + * @param[out] error > + * Pointer to the error structure. > + * > + * @return > + * Pointer to action on success, NULL otherwise and rte_errno is set. > + */ > +static struct ibv_flow_action * > +mlx5_flow_dv_create_decap_l3(const struct rte_flow_action *action, > + struct ibv_context *ctx, > + struct rte_flow_error *error) > +{ > + struct ibv_flow_action *decap_l3_verb =3D NULL; > + const struct rte_flow_action_tunnel_decap_l3 *decap_l3_data; > + > + decap_l3_data =3D > + (const struct rte_flow_action_tunnel_decap_l3 *)action->conf; > + decap_l3_verb =3D mlx5_glue->dv_create_flow_action_packet_reformat(ctx, > + decap_l3_data->size, > + decap_l3_data->size ? decap_l3_data->buf : > + NULL, > + MLX5DV_FLOW_ACTION_PACKET_REFORMAT_TYPE_L3_TUNNEL_TO_L2, > + MLX5DV_FLOW_TABLE_TYPE_NIC_RX); > + if (!decap_l3_verb) > + rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION, > + NULL, "cannot create decap L3 action"); > + > + return decap_l3_verb; > +} > + > +/** > * Verify the @p attributes will be correctly understood by the NIC and = store > * them in the @p flow if everything is correct. > * > @@ -529,6 +721,25 @@ > return ret; > action_flags |=3D MLX5_ACTION_TUNNEL_ENCAP_L3; > break; > + case RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP: > + ret =3D mlx5_flow_validate_action_tunnel_decap > + (action_flags, > + attr, > + error); > + if (ret < 0) > + return ret; > + action_flags |=3D MLX5_ACTION_TUNNEL_DECAP; > + break; > + case RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP_L3: > + ret =3D mlx5_flow_validate_action_tunnel_decap_l3 > + (action_flags, > + actions, > + attr, > + error); > + if (ret < 0) > + return ret; > + action_flags |=3D MLX5_ACTION_TUNNEL_DECAP_L3; > + break; > default: > return rte_flow_error_set(error, ENOTSUP, > RTE_FLOW_ERROR_TYPE_ACTION, > @@ -1320,6 +1531,30 @@ > flow->actions |=3D MLX5_ACTION_TUNNEL_ENCAP_L3; > actions_n++; > break; > + case RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP: > + dev_flow->dv.actions[actions_n].type =3D > + MLX5DV_FLOW_ACTION_IBV_FLOW_ACTION; > + dev_flow->dv.actions[actions_n].action =3D > + mlx5_flow_dv_create_decap(action, ctx, error); > + if (!(dev_flow->dv.actions[actions_n].action)) > + return -rte_errno; > + dev_flow->dv.decap_verb =3D > + dev_flow->dv.actions[actions_n].action; > + flow->actions |=3D MLX5_ACTION_TUNNEL_DECAP; > + actions_n++; > + break; > + case RTE_FLOW_ACTION_TYPE_TUNNEL_DECAP_L3: > + dev_flow->dv.actions[actions_n].type =3D > + MLX5DV_FLOW_ACTION_IBV_FLOW_ACTION; > + dev_flow->dv.actions[actions_n].action =3D > + mlx5_flow_dv_create_decap_l3(action, ctx, error); > + if (!(dev_flow->dv.actions[actions_n].action)) > + return -rte_errno; > + dev_flow->dv.decap_verb =3D > + dev_flow->dv.actions[actions_n].action; > + flow->actions |=3D MLX5_ACTION_TUNNEL_DECAP_L3; > + actions_n++; > + break; > default: > break; > } > @@ -1679,13 +1914,16 @@ > LIST_REMOVE(dev_flow, next); > if (dev_flow->dv.matcher) > flow_dv_matcher_release(dev, dev_flow->dv.matcher); > - Looks like it is a mistake on rebasing. > if (dev_flow->dv.encap_verb) { > claim_zero(mlx5_glue->destroy_flow_action > (dev_flow->dv.encap_verb)); > dev_flow->dv.encap_verb =3D NULL; > } > - > + if (dev_flow->dv.decap_verb) { > + claim_zero(mlx5_glue->destroy_flow_action > + (dev_flow->dv.decap_verb)); > + dev_flow->dv.decap_verb =3D NULL; > + } > rte_free(dev_flow); > } > } > --=20 > 1.8.3.1 >=20