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 A28D941DFD; Tue, 7 Mar 2023 08:47:39 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3C9FA40E03; Tue, 7 Mar 2023 08:47:39 +0100 (CET) Received: from mail-ua1-f47.google.com (mail-ua1-f47.google.com [209.85.222.47]) by mails.dpdk.org (Postfix) with ESMTP id B48774067E for ; Tue, 7 Mar 2023 08:47:37 +0100 (CET) Received: by mail-ua1-f47.google.com with SMTP id m5so8233709uae.11 for ; Mon, 06 Mar 2023 23:47:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1678175257; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=775jqEzhDqHCbxxynOpLntCudYBlgahxTBOQ5PwPBLw=; b=lqQbR7l7yMw3ee/VROJb4leHBCFDLnTdmD16ok2BpiBIf0Rh5+3A8Kgfql5WWAAJwz lv00cnC6MaOpYBT+VwUvD4BwREvfEb1eHhHKs0hNCVslZ/nbwoiStslliaXe20fAovwY cyDCy9NtF8xSt0OieXMz/1t6L0M5bgzXcxNxm0ZgqeSORlwUEF4a28Umfxd5/hPrLLkt f4D+OVrRHbQvtTplWPKFBY5q5YriE4V49wzcH23EUk3E6YblRAkhgsEQKH1luSrqedF4 /q888djRalh8ECoAKvpNjH7Q753PNC8MEPKr0BFNEH6XVcSkAo6JpRJJ56KOEYP1OUDg pDwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678175257; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=775jqEzhDqHCbxxynOpLntCudYBlgahxTBOQ5PwPBLw=; b=s6iN1j2lNEY8jy/1Z0/t1cDL8VwofhXcxIazYqHaetYnYEaoTZSWsjyMsy6ZL39I4q asCvi6q/bKQ9TnKNggUY/lYXdX3aaQ6F1oWAWKwfzvZ1K14jyWZy6Edhs9SKnGapajX8 KrKpOPi8Oodh1Oju6yIfg4D3xAg26iCFUHwHnO4NJBQywm1ex4CapRPP8D6T0juFuuJc 6ca8LkwtNql5/Hb1vstRX3s7GLteJ6/ZhuQCQbWUfRb332ShX0lNsNVf3MNJVP7bpqQy 9D4l0ymZw3Lnae2KsUU67zFiqjyFeLi3iPNp5k4y8fnRkkIenbJELWy9pX0/XKn/DU9E QcvA== X-Gm-Message-State: AO0yUKUO49EgEWnyXtuKu8yMXSYlr5wVe5mqypHNsEqCmS7aFm5Mt8nC SqR0wFSTLVqioVvpqkxTBfRlNQ3F8L1YENm2EmLu/mO85IY= X-Google-Smtp-Source: AK7set9EQGSZ5/tBe73ySsanrBSgYOdcx/x9nB1EuLjK34VIoBjlRL7wAcHxK0p9XyUJuawVGf9BbhE9Ga0XhFR0EQw= X-Received: by 2002:ab0:4a12:0:b0:68b:b624:7b86 with SMTP id q18-20020ab04a12000000b0068bb6247b86mr9025882uae.2.1678175256819; Mon, 06 Mar 2023 23:47:36 -0800 (PST) MIME-Version: 1.0 References: <20230306120853.849297-1-hpothula@marvell.com> In-Reply-To: <20230306120853.849297-1-hpothula@marvell.com> From: Jerin Jacob Date: Tue, 7 Mar 2023 13:17:10 +0530 Message-ID: Subject: Re: [PATCH v1 1/1] drivers: remove implementation of Rx metadata negotiation To: Hanumanth Pothula Cc: Nithin Dabilpuram , Kiran Kumar K , Sunil Kumar Kori , Satha Rao , dev@dpdk.org, jerinj@marvell.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 On Mon, Mar 6, 2023 at 5:39=E2=80=AFPM Hanumanth Pothula wrote: > > Presently, Rx metadata is sent to PMD by default, leading > to a performance drop as processing for the same in Rx path > takes extra cycles. > > Hence, removing driver implementation of Rx metadata negotiation > and falling back to old implementation where mark actions are > tracked as part of the flow rule. > > Signed-off-by: Hanumanth Pothula > --- > drivers/common/cnxk/roc_npc.c | 19 +++++++++++++++++++ > drivers/common/cnxk/roc_npc.h | 3 +++ > drivers/common/cnxk/roc_npc_priv.h | 1 + > drivers/common/cnxk/version.map | 2 ++ > drivers/net/cnxk/cn10k_ethdev.c | 26 -------------------------- > drivers/net/cnxk/cn10k_flow.c | 19 +++++++++++++++++++ > drivers/net/cnxk/cn9k_ethdev.c | 25 ------------------------- > drivers/net/cnxk/cn9k_flow.c | 20 ++++++++++++++++++++ > drivers/net/cnxk/cnxk_ethdev.h | 1 - > 9 files changed, 64 insertions(+), 52 deletions(-) > > diff --git a/drivers/common/cnxk/roc_npc.c b/drivers/common/cnxk/roc_npc.= c > index a795114326..47536c8ce8 100644 > --- a/drivers/common/cnxk/roc_npc.c > +++ b/drivers/common/cnxk/roc_npc.c > @@ -5,6 +5,23 @@ > #include "roc_api.h" > #include "roc_priv.h" > > +int > +roc_npc_mark_actions_get(struct roc_npc *roc_npc) > +{ > + struct npc *npc =3D roc_npc_to_npc_priv(roc_npc); > + > + return npc->mark_actions; > +} > + > +int > +roc_npc_mark_actions_sub_return(struct roc_npc *roc_npc, uint32_t count) > +{ > + struct npc *npc =3D roc_npc_to_npc_priv(roc_npc); > + > + npc->mark_actions -=3D count; > + return npc->mark_actions; > +} > + > int > roc_npc_vtag_actions_get(struct roc_npc *roc_npc) > { > @@ -488,12 +505,14 @@ npc_parse_actions(struct roc_npc *roc_npc, const st= ruct roc_npc_attr *attr, > } > mark =3D act_mark->id + 1; > req_act |=3D ROC_NPC_ACTION_TYPE_MARK; > + npc->mark_actions +=3D 1; > flow->match_id =3D mark; > break; > > case ROC_NPC_ACTION_TYPE_FLAG: > mark =3D NPC_FLOW_FLAG_VAL; > req_act |=3D ROC_NPC_ACTION_TYPE_FLAG; > + npc->mark_actions +=3D 1; > break; > > case ROC_NPC_ACTION_TYPE_COUNT: > diff --git a/drivers/common/cnxk/roc_npc.h b/drivers/common/cnxk/roc_npc.= h > index 5e07e26a91..61d0628f5f 100644 > --- a/drivers/common/cnxk/roc_npc.h > +++ b/drivers/common/cnxk/roc_npc.h > @@ -397,6 +397,9 @@ int __roc_api roc_npc_mcam_free_all_resources(struct = roc_npc *roc_npc); > void __roc_api roc_npc_flow_dump(FILE *file, struct roc_npc *roc_npc); > void __roc_api roc_npc_flow_mcam_dump(FILE *file, struct roc_npc *roc_np= c, > struct roc_npc_flow *mcam); > +int __roc_api roc_npc_mark_actions_get(struct roc_npc *roc_npc); > +int __roc_api roc_npc_mark_actions_sub_return(struct roc_npc *roc_npc, > + uint32_t count); > int __roc_api roc_npc_vtag_actions_get(struct roc_npc *roc_npc); > int __roc_api roc_npc_vtag_actions_sub_return(struct roc_npc *roc_npc, > uint32_t count); > diff --git a/drivers/common/cnxk/roc_npc_priv.h b/drivers/common/cnxk/roc= _npc_priv.h > index 08d763eeb4..714dcb09c9 100644 > --- a/drivers/common/cnxk/roc_npc_priv.h > +++ b/drivers/common/cnxk/roc_npc_priv.h > @@ -393,6 +393,7 @@ struct npc { > uint16_t flow_prealloc_size; /* Pre allocated mcam siz= e */ > uint16_t flow_max_priority; /* Max priority for flow = */ > uint16_t switch_header_type; /* Supported switch header type */ > + uint32_t mark_actions; > uint32_t vtag_strip_actions; /* vtag insert/strip actions */ > uint16_t pf_func; /* pf_func of device */ > npc_dxcfg_t prx_dxcfg; /* intf, lid, lt, extract */ > diff --git a/drivers/common/cnxk/version.map b/drivers/common/cnxk/versio= n.map > index 5d2b75fb5a..3eff3870d1 100644 > --- a/drivers/common/cnxk/version.map > +++ b/drivers/common/cnxk/version.map > @@ -344,6 +344,8 @@ INTERNAL { > roc_npc_flow_parse; > roc_npc_get_low_priority_mcam; > roc_npc_init; > + roc_npc_mark_actions_get; > + roc_npc_mark_actions_sub_return; > roc_npc_vtag_actions_get; > roc_npc_vtag_actions_sub_return; > roc_npc_mcam_alloc_entries; > diff --git a/drivers/net/cnxk/cn10k_ethdev.c b/drivers/net/cnxk/cn10k_eth= dev.c > index b84fed6d90..512b9c2597 100644 > --- a/drivers/net/cnxk/cn10k_ethdev.c > +++ b/drivers/net/cnxk/cn10k_ethdev.c > @@ -39,9 +39,6 @@ nix_rx_offload_flags(struct rte_eth_dev *eth_dev) > if (dev->rx_offloads & RTE_ETH_RX_OFFLOAD_SECURITY) > flags |=3D NIX_RX_OFFLOAD_SECURITY_F; > > - if (dev->rx_mark_update) > - flags |=3D NIX_RX_OFFLOAD_MARK_UPDATE_F; > - > return flags; > } > > @@ -562,27 +559,6 @@ cn10k_nix_dev_start(struct rte_eth_dev *eth_dev) > return 0; > } > > -static int > -cn10k_nix_rx_metadata_negotiate(struct rte_eth_dev *eth_dev, uint64_t *f= eatures) > -{ > - struct cnxk_eth_dev *dev =3D cnxk_eth_pmd_priv(eth_dev); > - > - *features &=3D > - (RTE_ETH_RX_METADATA_USER_FLAG | RTE_ETH_RX_METADATA_USER= _MARK); > - > - if (*features) { > - dev->rx_offload_flags |=3D NIX_RX_OFFLOAD_MARK_UPDATE_F; > - dev->rx_mark_update =3D true; > - } else { > - dev->rx_offload_flags &=3D ~NIX_RX_OFFLOAD_MARK_UPDATE_F; > - dev->rx_mark_update =3D false; > - } > - > - cn10k_eth_set_rx_function(eth_dev); > - > - return 0; > -} > - > static int > cn10k_nix_reassembly_capability_get(struct rte_eth_dev *eth_dev, > struct rte_eth_ip_reassembly_params *reassembly_capa) > @@ -770,8 +746,6 @@ nix_eth_dev_ops_override(void) > cnxk_eth_dev_ops.dev_ptypes_set =3D cn10k_nix_ptypes_set; > cnxk_eth_dev_ops.timesync_enable =3D cn10k_nix_timesync_enable; > cnxk_eth_dev_ops.timesync_disable =3D cn10k_nix_timesync_disable; > - cnxk_eth_dev_ops.rx_metadata_negotiate =3D > - cn10k_nix_rx_metadata_negotiate; > cnxk_eth_dev_ops.timesync_read_tx_timestamp =3D > cn10k_nix_timesync_read_tx_timestamp; > cnxk_eth_dev_ops.ip_reassembly_capability_get =3D > diff --git a/drivers/net/cnxk/cn10k_flow.c b/drivers/net/cnxk/cn10k_flow.= c > index 2ce9e1de74..3a7b09d794 100644 > --- a/drivers/net/cnxk/cn10k_flow.c > +++ b/drivers/net/cnxk/cn10k_flow.c > @@ -135,6 +135,7 @@ cn10k_flow_create(struct rte_eth_dev *eth_dev, const = struct rte_flow_attr *attr, > struct roc_npc_flow *flow; > int vtag_actions =3D 0; > uint32_t req_act =3D 0; > + int mark_actions =3D 0; Is explict 0 assigmenet needed? Please check in other place too? > int i, rc; > > for (i =3D 0; actions[i].type !=3D RTE_FLOW_ACTION_TYPE_END; i++)= { > @@ -197,6 +198,12 @@ cn10k_flow_create(struct rte_eth_dev *eth_dev, const= struct rte_flow_attr *attr, > cn10k_mtr_connect(eth_dev, mtr->mtr_id); > } > > + mark_actions =3D roc_npc_mark_actions_get(npc); > + if (mark_actions) { > + dev->rx_offload_flags |=3D NIX_RX_OFFLOAD_MARK_UPDATE_F; > + cn10k_eth_set_rx_function(eth_dev); > + } > + > vtag_actions =3D roc_npc_vtag_actions_get(npc); > > if (vtag_actions) { > @@ -231,9 +238,21 @@ cn10k_flow_destroy(struct rte_eth_dev *eth_dev, stru= ct rte_flow *rte_flow, > struct cnxk_eth_dev *dev =3D cnxk_eth_pmd_priv(eth_dev); > struct roc_npc *npc =3D &dev->npc; > int vtag_actions =3D 0; > + int mark_actions =3D 0; > + uint16_t match_id =3D 0; Same as above. > uint32_t mtr_id; > int rc; > > + match_id =3D (flow->npc_action >> NPC_RX_ACT_MATCH_OFFSET) & > + NPC_RX_ACT_MATCH_MASK; > + if (match_id) { > + mark_actions =3D roc_npc_mark_actions_sub_return(npc, 1); > + if (mark_actions =3D=3D 0) { > + dev->rx_offload_flags &=3D ~NIX_RX_OFFLOAD_MARK_U= PDATE_F; > + cn10k_eth_set_rx_function(eth_dev); > + } > + } > + > vtag_actions =3D roc_npc_vtag_actions_get(npc); > if (vtag_actions) { > if (flow->nix_intf =3D=3D ROC_NPC_INTF_RX) { > diff --git a/drivers/net/cnxk/cn9k_ethdev.c b/drivers/net/cnxk/cn9k_ethde= v.c > index e5c3074d91..e55a2aa133 100644 > --- a/drivers/net/cnxk/cn9k_ethdev.c > +++ b/drivers/net/cnxk/cn9k_ethdev.c > @@ -39,9 +39,6 @@ nix_rx_offload_flags(struct rte_eth_dev *eth_dev) > if (dev->rx_offloads & RTE_ETH_RX_OFFLOAD_SECURITY) > flags |=3D NIX_RX_OFFLOAD_SECURITY_F; > > - if (dev->rx_mark_update) > - flags |=3D NIX_RX_OFFLOAD_MARK_UPDATE_F; > - > return flags; > } > > @@ -541,27 +538,6 @@ cn9k_nix_timesync_read_tx_timestamp(struct rte_eth_d= ev *eth_dev, > return 0; > } > > -static int > -cn9k_nix_rx_metadata_negotiate(struct rte_eth_dev *eth_dev, uint64_t *fe= atures) > -{ > - struct cnxk_eth_dev *dev =3D cnxk_eth_pmd_priv(eth_dev); > - > - *features &=3D > - (RTE_ETH_RX_METADATA_USER_FLAG | RTE_ETH_RX_METADATA_USER= _MARK); > - > - if (*features) { > - dev->rx_offload_flags |=3D NIX_RX_OFFLOAD_MARK_UPDATE_F; > - dev->rx_mark_update =3D true; > - } else { > - dev->rx_offload_flags &=3D ~NIX_RX_OFFLOAD_MARK_UPDATE_F; > - dev->rx_mark_update =3D false; > - } > - > - cn9k_eth_set_rx_function(eth_dev); > - > - return 0; > -} > - > static int > cn9k_nix_tm_mark_vlan_dei(struct rte_eth_dev *eth_dev, int mark_green, > int mark_yellow, int mark_red, > @@ -695,7 +671,6 @@ nix_eth_dev_ops_override(void) > cnxk_eth_dev_ops.timesync_enable =3D cn9k_nix_timesync_enable; > cnxk_eth_dev_ops.timesync_disable =3D cn9k_nix_timesync_disable; > cnxk_eth_dev_ops.mtr_ops_get =3D NULL; > - cnxk_eth_dev_ops.rx_metadata_negotiate =3D cn9k_nix_rx_metadata_n= egotiate; > cnxk_eth_dev_ops.timesync_read_tx_timestamp =3D > cn9k_nix_timesync_read_tx_timestamp; > } > diff --git a/drivers/net/cnxk/cn9k_flow.c b/drivers/net/cnxk/cn9k_flow.c > index a418af185d..b9ce1112a7 100644 > --- a/drivers/net/cnxk/cn9k_flow.c > +++ b/drivers/net/cnxk/cn9k_flow.c > @@ -16,11 +16,19 @@ cn9k_flow_create(struct rte_eth_dev *eth_dev, const s= truct rte_flow_attr *attr, > struct roc_npc *npc =3D &dev->npc; > struct roc_npc_flow *flow; > int vtag_actions =3D 0; > + int mark_actions =3D 0; Same as above. > > flow =3D cnxk_flow_create(eth_dev, attr, pattern, actions, error)= ; > if (!flow) > return NULL; > > + mark_actions =3D roc_npc_mark_actions_get(npc); > + > + if (mark_actions) { > + dev->rx_offload_flags |=3D NIX_RX_OFFLOAD_MARK_UPDATE_F; > + cn9k_eth_set_rx_function(eth_dev); > + } > + > vtag_actions =3D roc_npc_vtag_actions_get(npc); > > if (vtag_actions) { > @@ -39,6 +47,18 @@ cn9k_flow_destroy(struct rte_eth_dev *eth_dev, struct = rte_flow *rte_flow, > struct cnxk_eth_dev *dev =3D cnxk_eth_pmd_priv(eth_dev); > struct roc_npc *npc =3D &dev->npc; > int vtag_actions =3D 0; > + uint16_t match_id =3D 0; > + int mark_actions =3D 0; > + > + match_id =3D (flow->npc_action >> NPC_RX_ACT_MATCH_OFFSET) & > + NPC_RX_ACT_MATCH_MASK; > + if (match_id) { > + mark_actions =3D roc_npc_mark_actions_sub_return(npc, 1); > + if (mark_actions =3D=3D 0) { > + dev->rx_offload_flags &=3D ~NIX_RX_OFFLOAD_MARK_U= PDATE_F; > + cn9k_eth_set_rx_function(eth_dev); > + } > + } > > vtag_actions =3D roc_npc_vtag_actions_get(npc); > if (vtag_actions) { > diff --git a/drivers/net/cnxk/cnxk_ethdev.h b/drivers/net/cnxk/cnxk_ethde= v.h > index f0eab4244c..59a06292d8 100644 > --- a/drivers/net/cnxk/cnxk_ethdev.h > +++ b/drivers/net/cnxk/cnxk_ethdev.h > @@ -315,7 +315,6 @@ struct cnxk_eth_dev { > bool tx_compl_ena; > bool tx_mark; > bool ptp_en; > - bool rx_mark_update; /* Enable/Disable mark update to mbuf */ > > /* Pointer back to rte */ > struct rte_eth_dev *eth_dev; > -- > 2.25.1 >