Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
From:   Geethasowjanya Akula <gakula@marvell.com>
To:     Simon Horman <horms@kernel.org>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "edumazet@google.com" <edumazet@google.com>,
        Sunil Kovvuri Goutham <sgoutham@marvell.com>,
        Linu Cherian <lcherian@marvell.com>,
        Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@marvell.com>,
        Hariprasad Kelam <hkelam@marvell.com>
Subject: RE: [EXT] Re: [net v3 PATCH 1/5] octeontx2-af: Adjust Tx credits when
 MCS external bypass is disabled
Thread-Topic: [EXT] Re: [net v3 PATCH 1/5] octeontx2-af: Adjust Tx credits
 when MCS external bypass is disabled
Thread-Index: AQHaI2MCmzc2c26P406SbfgryAxPK7CXyFGAgADVSgA=
Date:   Mon, 4 Dec 2023 05:26:18 +0000
Message-ID: <DM6PR18MB2602EC1560A801724F77095ACD86A@DM6PR18MB2602.namprd18.prod.outlook.com>
References: <20231130075818.18401-1-gakula@marvell.com>
 <20231130075818.18401-2-gakula@marvell.com>
 <20231203164121.GK50400@kernel.org>
In-Reply-To: <20231203164121.GK50400@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?W3NYvuLtS4VZOnmtFa36MZ08z4ar74XFUTTJpYLtVBbVkMGSIhLN/tbFE35N?=
 =?us-ascii?Q?c4b+8WrIaBZVz1aAlEnO3m37GwV2CYNnAH2ud5nd72xkhtXL9BqJtYOukc5+?=
 =?us-ascii?Q?qkDrwov9ku/L9mm0gb+s1VRHezIMn19ayv9fCb/gsAGGGyS8sHQzZ6erxGZC?=
 =?us-ascii?Q?mkadUXzygvVG55IIr7YlNWdFqNYPNwXiBRDaod9vNQPGbFbk+vLf5DwLX3y0?=
 =?us-ascii?Q?H4lpO3/mB+1UfpfWXVx0I3PNfY5lIw4rAZcWIYOYHompwal9eCZ6+U8P5ul8?=
 =?us-ascii?Q?ETTp071/2XCyIc7sGgJF1NGnAnUyR8YpFYGdAa4/gFg0wFoB8TYadHGb2sT5?=
 =?us-ascii?Q?mADe9m2Oj4o7UYlphkrKIPppvCiytQAJylXWcFzOeIOcy8Ekiu+YPsLGhqM/?=
 =?us-ascii?Q?NbZZlGCB0R/0arHOTSyennOES8H2YqEtRFMU05gMzab9wA6Pe0sNCHX4Pxfd?=
 =?us-ascii?Q?z4iqky0EQCD8uIU19DbI/nW6rMb2w0TjplMTXMlGQMq3dVu1LDI7adu6sXR6?=
 =?us-ascii?Q?sBbQ7WO3CHvuOygYycng3YJrRsKDRNFk6H6NHeytN/Y6T7MMzMf84K+8q1Gk?=
 =?us-ascii?Q?br2wtq5X3u9xMuONwupBNXpzJn42vPZupZj0xaW2hNU+1cS38Rn+jCKqn6LP?=
 =?us-ascii?Q?LWev/3+5FZKtd8RUPnu7vjT2GFw74yg+ByvG8/VZ2rktoP7SbRKBr9vuBXKy?=
 =?us-ascii?Q?NqG7tXuqe0NRigfE41YWRHBHNzn9gaQPm0iMEzLH+f17WLHcVUqqzHc8zrve?=
 =?us-ascii?Q?5xdtYTKBs1quiLzqICXtMEvpHkhtMYTTph60gv7sK2Nr2Rua0PLyiYcjefLD?=
 =?us-ascii?Q?EwLjVB/1b2xGdJrUpLmyNneGblvbJA639apJ6/GVJu9q68KCiU8PHuMqyyli?=
 =?us-ascii?Q?IHx/Fy2z2983ge3AdcBs0HBU1EdIur63Au69SmwVK+EbU7Hx4oGwmQ7iPq1j?=
 =?us-ascii?Q?PNE1EcpBzoYRzU5SJMsiTEtOvgWF4tcPbhbhAJc81m32PJMtkKXt0ASDHKVb?=
 =?us-ascii?Q?XMWTRU7w/FJI88LP/3Ul2u0YQBNUOsb2Tb6Zz9RbbzMoy+wOVesv/TS76OWs?=
 =?us-ascii?Q?6JobbLUKNxGf1NB1FMu+vGXIyDtAWqgA5DYnYAFjjMCYZDKrt8fgDRvdgo3B?=
 =?us-ascii?Q?pEu/AtEXLqrdFmjJD1orekyKClI7uEmH2FO/9DicWwaoDj6eXwKHbBDj8vfv?=
 =?us-ascii?Q?Ew6X7yi1QXYwQAJ/EF3KS9z7AORFJWNIaSZ/2rdEbKxdN5DLLZV4s9aqdVPD?=
 =?us-ascii?Q?ZDa6gwQAJYwceJcUay5w+7j6P2K/ZJyDyb/k2ab85dkopBXlnhFm77ddguiU?=
 =?us-ascii?Q?up34CaUB2qlBTyQvIGMiAc149BLfigKdFrpQykwmt+fHUqSucb4K7qHC07Cs?=
 =?us-ascii?Q?qrh3MbVH4xhPyvxOSylKYJk39H9PNoMHyg4g581cE17fnUcq2vKFkYHG9YUl?=
 =?us-ascii?Q?F3RagUjTh83P93tJHyyQ1LfzQ8hBYZ502bcVPI7UKkiFfRXRsXxp7Ez+Z/j7?=
 =?us-ascii?Q?Q6QE/thhOFh61MHM7jKVtYlOXAD4BpQK9VoXVF/CedPvDHre/If1SybC65H7?=
 =?us-ascii?Q?q0iJSgC2Fb86TE5JElo=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR18MB2602.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2179ed82-1718-4e3e-e764-08dbf4898af0
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Dec 2023 05:26:18.1710
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 6QXIJspp9+SJHICLpxLQBWdMS2iwhM00gow1Md81xx4ZNeini+h47JV909S0y1yguCH4AuzhJn/juF0k3QsYHw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR18MB4302
X-Proofpoint-GUID: b-Gw0IPaL7QYjFr6ZHnlk7dADoDsG1Ev
X-Proofpoint-ORIG-GUID: b-Gw0IPaL7QYjFr6ZHnlk7dADoDsG1Ev
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-04_03,2023-11-30_01,2023-05-22_02
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Sun, 03 Dec 2023 21:26:55 -0800 (PST)


> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Sunday, December 3, 2023 10:11 PM
> To: Geethasowjanya Akula <gakula@marvell.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kuba@kernel.org=
;
> davem@davemloft.net; pabeni@redhat.com; edumazet@google.com; Sunil
> Kovvuri Goutham <sgoutham@marvell.com>; Linu Cherian
> <lcherian@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Subbaraya Sundeep Bhatta <sbhatta@marvell.com>; Hariprasad Kelam
> <hkelam@marvell.com>
> Subject: [EXT] Re: [net v3 PATCH 1/5] octeontx2-af: Adjust Tx credits whe=
n
> MCS external bypass is disabled
>=20
> External Email
>=20
> ----------------------------------------------------------------------
> On Thu, Nov 30, 2023 at 01:28:14PM +0530, Geetha sowjanya wrote:
> > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >
> > When MCS external bypass is disabled, MCS returns additional
> > 2 credits(32B) for every packet Tx'ed on LMAC. To account for these
> > extra credits, NIX_AF_TX_LINKX_NORM_CREDIT.CC_MCS_CNT
> > needs to be configured as otherwise NIX Tx credits would overflow and
> > will never be returned to idle state credit count causing issues with
> > credit control and MTU change.
> >
> > This patch fixes the same by configuring CC_MCS_CNT at probe time for
> > MCS enabled SoC's
> >
> > Fixes: bd69476e86fc ("octeontx2-af: cn10k: mcs: Install a default TCAM
> > for normal traffic")
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > Signed-off-by: Geetha sowjanya <gakula@marvell.com>
> > Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
> > Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
>=20
> Hi Geetha and Nithin,
>=20
> some minor feedback from my side.
>=20
> > ---
> >  drivers/net/ethernet/marvell/octeontx2/af/mcs.c     | 12 ++++++++++++
> >  drivers/net/ethernet/marvell/octeontx2/af/mcs.h     |  2 ++
> >  drivers/net/ethernet/marvell/octeontx2/af/rvu.h     |  1 +
> >  drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c |  8 ++++++++
> >  4 files changed, 23 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mcs.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/mcs.c
> > index c43f19dfbd74..d6effbe46208 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/mcs.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/mcs.c
> > @@ -1219,6 +1219,17 @@ struct mcs *mcs_get_pdata(int mcs_id)
> >  	return NULL;
> >  }
> >
> > +bool is_mcs_bypass(int mcs_id)
> > +{
> > +	struct mcs *mcs_dev;
> > +
> > +	list_for_each_entry(mcs_dev, &mcs_list, mcs_list) {
> > +		if (mcs_dev->mcs_id =3D=3D mcs_id)
> > +			return mcs_dev->bypass;
> > +	}
> > +	return true;
> > +}
> > +
> >  void mcs_set_port_cfg(struct mcs *mcs, struct mcs_port_cfg_set_req
> > *req)  {
> >  	u64 val =3D 0;
> > @@ -1447,6 +1458,7 @@ static void mcs_set_external_bypass(struct mcs
> *mcs, u8 bypass)
> >  	else
> >  		val &=3D ~BIT_ULL(6);
> >  	mcs_reg_write(mcs, MCSX_MIL_GLOBAL, val);
> > +	mcs->bypass =3D bypass;
>=20
> I think that bool would be a more appropriate type than u8 for:
>=20
> * The bypass parameter of mcs_set_external_bypass()
> * The bypass field of struct mcs
>=20
> >  }
> >
> >  static void mcs_global_cfg(struct mcs *mcs) diff --git
> > a/drivers/net/ethernet/marvell/octeontx2/af/mcs.h
> > b/drivers/net/ethernet/marvell/octeontx2/af/mcs.h
> > index 0f89dcb76465..ccd43c3f3460 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/mcs.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/mcs.h
> > @@ -149,6 +149,7 @@ struct mcs {
> >  	u16			num_vec;
> >  	void			*rvu;
> >  	u16			*tx_sa_active;
> > +	u8                      bypass;
> >  };
> >
> >  struct mcs_ops {
> > @@ -206,6 +207,7 @@ void mcs_get_custom_tag_cfg(struct mcs *mcs,
> > struct mcs_custom_tag_cfg_get_req *  int mcs_alloc_ctrlpktrule(struct
> > rsrc_bmap *rsrc, u16 *pf_map, u16 offset, u16 pcifunc);  int
> > mcs_free_ctrlpktrule(struct mcs *mcs, struct
> > mcs_free_ctrl_pkt_rule_req *req);  int mcs_ctrlpktrule_write(struct
> > mcs *mcs, struct mcs_ctrl_pkt_rule_write_req *req);
> > +bool is_mcs_bypass(int mcs_id);
> >
> >  /* CN10K-B APIs */
> >  void cn10kb_mcs_set_hw_capabilities(struct mcs *mcs); diff --git
> > a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
> > b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
> > index c4d999ef5ab4..9887edccadf7 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
> > @@ -345,6 +345,7 @@ struct nix_hw {
> >  	struct nix_txvlan txvlan;
> >  	struct nix_ipolicer *ipolicer;
> >  	u64    *tx_credits;
> > +	u64 cc_mcs_cnt;
> >  };
> >
> >  /* RVU block's capabilities or functionality, diff --git
> > a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > index c112c71ff576..daafce5fef46 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > @@ -12,6 +12,7 @@
> >  #include "rvu_reg.h"
> >  #include "rvu.h"
> >  #include "npc.h"
> > +#include "mcs.h"
> >  #include "cgx.h"
> >  #include "lmac_common.h"
> >  #include "rvu_npc_hash.h"
> > @@ -4389,6 +4390,12 @@ static void nix_link_config(struct rvu *rvu, int
> blkaddr,
> >  			    SDP_HW_MAX_FRS << 16 | NIC_HW_MIN_FRS);
> >  	}
> >
> > +	/* Get MCS external bypass status for CN10K-B */
> > +	if (mcs_get_blkcnt() =3D=3D 1) {
> > +		/* Adjust for 2 credits when external bypass is disabled */
> > +		nix_hw->cc_mcs_cnt =3D is_mcs_bypass(0) ? 0 : 2;
>=20
> Perhaps it doesn't matter, but to me it seems a bit excessive to use a 64=
-bit
> field to store such small values.
>=20
> > +	}
> > +
> >  	/* Set credits for Tx links assuming max packet length allowed.
> >  	 * This will be reconfigured based on MTU set for PF/VF.
> >  	 */
> > @@ -4412,6 +4419,7 @@ static void nix_link_config(struct rvu *rvu, int
> blkaddr,
> >  			tx_credits =3D (lmac_fifo_len - lmac_max_frs) / 16;
> >  			/* Enable credits and set credit pkt count to max
> allowed */
> >  			cfg =3D  (tx_credits << 12) | (0x1FF << 2) | BIT_ULL(1);
> > +			cfg |=3D (nix_hw->cc_mcs_cnt << 32);
>=20
> I do see that cc_mcs_cnt needs to be 64-bit here to avoid truncation.
> But overall I think this function could benefit from the use of FIELD_PRE=
P(),
> which I think would side-step that problem.
Thanks for the feedback. Will submit next version with the suggested change=
s.
>=20
> >
> >  			link =3D iter + slink;
> >  			nix_hw->tx_credits[link] =3D tx_credits;
> > --
> > 2.25.1
> >