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 8B657A034F; Wed, 31 Mar 2021 12:50:27 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1A3F4140E6C; Wed, 31 Mar 2021 12:50:27 +0200 (CEST) Received: from mail-io1-f47.google.com (mail-io1-f47.google.com [209.85.166.47]) by mails.dpdk.org (Postfix) with ESMTP id 55109406A3 for ; Wed, 31 Mar 2021 12:50:25 +0200 (CEST) Received: by mail-io1-f47.google.com with SMTP id k25so19633281iob.6 for ; Wed, 31 Mar 2021 03:50:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=6HBpnheEVEAX2AnquecX+D3miqtxQE/BO/QaGhynYso=; b=C6Ew0dpSIE3LJ7u41NBZcBwkLr1QdTvyxh2tFKt2dxnafZMg2Zqja+EjZbmmpDfcBa XdhTRg3tt7mK/17xmB1+/fOicEYkSQXYhP3I4J51NX4t/8NLMbYKoAeXP2hFnXAXfZ1X eBYwESjDh+VuBRdwvq/nWRG+2y30oW5OSVJIp0hCJ/ZSzvCDiuOeRhUPgxWStM7MHCqL KqOpNrL+QJarJnpI/MJXleTr3EF2WDES7pdyjYLs7Ksc8kgodbE0tOVB3IODu7MW7SK/ tdqg3HRAQxhnEPDJC/NFfLLK9xrAnyJ6mOQB8sFfT5oDY58LKXZ4GpPg7hu6ngvhYV0u WnAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=6HBpnheEVEAX2AnquecX+D3miqtxQE/BO/QaGhynYso=; b=duJDDLoQUKtegVTOT3g3YYsu3i/qu5mtAGDqB9uBvKXtP9BMrauzOsDqcUn6Tq4UEm YsjAZ0TTJD8XZ5orXCyNa5JzEhSXAuHVgDwxKoLBjZFXjQP/yLLLr1BkYY4A0SPuJkfA xgh9F+KP9dXTQ9S1JXoWjUkHO05je3/1OHC6An82FntbsgY9C3XJIv3f+/EkurHcX+Zz dBRA20BNWrlDrqiIp1h+/bOEHvoJ/JZdCCXuyHDVJFKdwwM9SEAVbOcZOIRpwZdXhSO3 UiIOyVYlha9jVjHfIQzsLFc66EAD8jiWRy/zgDlFtrM1dNv/CIRtZE1j+hjs+pGF0+uP GEGA== X-Gm-Message-State: AOAM531N+6HjFzlLm1c7SlP9FgaaNTcmoj7cxg9WWtBa1F5v7EZJ1bhu 4d3hhAdJKMvuv22P419cgWPgaP9BBMVxDRZ00rU= X-Google-Smtp-Source: ABdhPJxkAoVaHITZM7BJ0KA06x//psg1lLfo8tdEYhiBpD2mYR18gE3aXZ1gDgpFKXStkVUVtQ+B8XyOswA1ZUKa6/Q= X-Received: by 2002:a02:7f0e:: with SMTP id r14mr2376574jac.112.1617187823169; Wed, 31 Mar 2021 03:50:23 -0700 (PDT) MIME-Version: 1.0 References: <20210318085815.804896-1-lizh@nvidia.com> In-Reply-To: From: Jerin Jacob Date: Wed, 31 Mar 2021 16:20:06 +0530 Message-ID: To: Matan Azrad Cc: Li Zhang , Dekel Peled , Ori Kam , Slava Ovsiienko , Shahaf Shuler , Liron Himi , Jasvinder Singh , NBU-Contact-Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Cristian Dumitrescu , dpdk-dev , Raslan Darawsheh , Roni Bar Yanai Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH 1/2] [RFC]: ethdev: add pre-defined meter policy API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" On Tue, Mar 30, 2021 at 2:01 AM Matan Azrad wrote: > > Hi Jerin > > Thanks for the review. > PSB > > From: Jerin Jacob > > On Thu, Mar 18, 2021 at 2:28 PM Li Zhang wrote: > > > > > > Currently, the flow meter policy does not support multiple actions per > > > color; also the allowed action types per color are very limited. > > > In addition, the policy cannot be pre-defined. > > > > > > Due to the growing in flow actions offload abilities there is a > > > potential for the user to use variety of actions per color differently. > > > This new meter policy API comes to allow this potential in the most > > > ethdev common way using rte_flow action definition. > > > A list of rte_flow actions will be provided by the user per color in > > > order to create a meter policy. > > > In addition, the API forces to pre-define the policy before the meters > > > creation in order to allow sharing of single policy with multiple > > > meters efficiently. > > > > > > meter_policy_id is added into struct rte_mtr_params. > > > So that it can get the policy during the meters creation. > > > > > > Policy id 0 is default policy. Action per color as below: > > > green - no action, yellow - no action, red - drop > > > > > > Allow coloring the packet using a new rte_flow_action_color as could > > > be done by the old policy API, > > > > > > The next API function were added: > > > - rte_mtr_meter_policy_add > > > - rte_mtr_meter_policy_delete > > > - rte_mtr_meter_policy_update > > > - rte_mtr_meter_policy_validate > > > The next struct was changed: > > > - rte_mtr_params > > > - rte_mtr_capabilities > > > The next API was deleted: > > > - rte_mtr_policer_actions_update > > > > > > Signed-off-by: Li Zhang > > > --- > > > lib/librte_ethdev/rte_flow.h | 18 ++++ > > > lib/librte_ethdev/rte_mtr.c | 55 ++++++++-- > > > lib/librte_ethdev/rte_mtr.h | 166 ++++++++++++++++++++--------- > > > lib/librte_ethdev/rte_mtr_driver.h | 45 ++++++-- > > > 4 files changed, 210 insertions(+), 74 deletions(-) > > > > > > diff --git a/lib/librte_ethdev/rte_flow.h > > > b/lib/librte_ethdev/rte_flow.h index 669e677e91..5f38aa7fa4 100644 > > > --- a/lib/librte_ethdev/rte_flow.h > > > +++ b/lib/librte_ethdev/rte_flow.h > > > @@ -31,6 +31,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > > > > #ifdef __cplusplus > > > extern "C" { > > > @@ -2236,6 +2237,13 @@ enum rte_flow_action_type { > > > * See struct rte_flow_action_modify_field. > > > */ > > > RTE_FLOW_ACTION_TYPE_MODIFY_FIELD, > > > + > > > + /** > > > + * Color the packet to reflect the meter color result. > > > + * > > > + * See struct rte_flow_action_color. > > > + */ > > > + RTE_FLOW_ACTION_TYPE_COlOR, > > > > Based on my understanding of this API, > > 1) Application creates the policy > > 2) Attachs the policy ID to meter object in params If so, Why we need this new > > action? > > Yes, > In the new policy API the meter actions will be defined by rte_flow actions. > The old policy API used rte_mtr actions: color green\color yellow\color red\drop. > > This new rte_flow COLOR action comes to replace the old policy API "color" actions which set the color field in mbuf. OK. Looks good to me. I think, it is better to change to RTE_FLOW_ACTION_TYPE_METER_COLOR to denote it as meter-specific action. > > > > > }; > > > > > > /** > > > @@ -2828,6 +2836,16 @@ struct rte_flow_action_set_dscp { > > > */ > > > struct rte_flow_shared_action; > > > > > > +/** > > > + * Meter policy add > > > + * > > > + * Create a new meter policy. The new policy > > > + * is used to create single or multiple MTR objects. > > > + * > > > + * @param[in] port_id > > > + * The port identifier of the Ethernet device. > > > + * @param[in] policy_id > > > + * Policy identifier for the new meter policy. > > > + * @param[in] actions > > > + * Associated actions per color. > > > + * list NULL is legal and means no special action. > > > + * (list terminated by the END action). > > > + * @param[out] error > > > + * Error details. Filled in only on error, when not NULL. > > > + * @return > > > + * 0 on success, non-zero error code otherwise. > > > + */ > > > +__rte_experimental > > > +int > > > +rte_mtr_meter_policy_add(uint16_t port_id, > > > > > > _create() may be better here instead of _add() as you have used _delete() > > Yes! OK > > > > + uint32_t policy_id, > > > + const struct rte_flow_action *actions[RTE_COLORS], > > > > > > 1) Does this mean that MLX HW can support any rte_flow actions like, if packet > > color is green do RTE_FLOW_ACTION_TYPE_SECURITY etc. > > Theoretically yes, we can support most of the actions. > For the first stage we are going to support next: queue\RSS\port id\mark\tag\jump. > > For example, user can select different queue\port id per color. OK. > > > 2) Is there any real-world use case other than using normal action like pass or > > drop as it is used in conjunction with meter object? > > Yes, I wrote above. > > > 3) Marvell HW has the following policy actions > > a) PASS > > b) DROP > > c) RED (Random early discard) > > > > Both (a) and (c) are not in enumated as rte_flow_actions. > > (a) is in, just don't specify any action. > > Can you explain what is "Random early discard"? https://en.wikipedia.org/wiki/Random_early_detection > How did you specify it in the old\current meter policy API? It can not. We are planning to add support for the meter in the new cnxk driver. Planning change the meter API based on the driver patch. > > > Note, that after the policy actions the device should continue to do the rest of the actions in the flow (after meter) if no termination action in the policy color. > > > > > Should we take rte_flow_action or create meter-specific policy actions? > > This patch removes the meter-specific policy actions. > You need to use rte_flow action. > > By the way, can you help to adjust Marvell driver to the change? The current driver(octeontx2) is not using meter. >