Message ID | 20230802105227.3691713-1-rkannoth@marvell.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9f41:0:b0:3e4:2afc:c1 with SMTP id v1csp412556vqx; Wed, 2 Aug 2023 05:18:00 -0700 (PDT) X-Google-Smtp-Source: APBJJlEuoQqjxXqt70lnPxCB2ilGL9QBvgHjwVV43f2paVcIOSpjOiVbLD31YnVfJRgitj/0ibex X-Received: by 2002:aa7:d7d1:0:b0:522:5980:ae08 with SMTP id e17-20020aa7d7d1000000b005225980ae08mr5464286eds.18.1690978679949; Wed, 02 Aug 2023 05:17:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690978679; cv=none; d=google.com; s=arc-20160816; b=qo5+QPkee1XegIcOTpn8P3X5hxfo5OF+kqLgNsicLeMXJyCK3e9wkBZ/zYgw4FspOC eLqIXe0MxjjNxGcrPut6swsqsRzKt/Dldmb4p65QncxNNg/DRUl2cO8GzJ+iGlBWPAZD cKKQVZDHFqzXrcxUmTyZkdIQKte/9WHRx/K2rtHt3oHgRrFnP4lhb4CK0HYOEujsGmZt lgVFkgzWAZEmIW1ZbLRot0xFvln8CYJqTp0fIm6Axr+53cq7G2254Ju56vq6yJTqU7Zb BBcfyntLHfge6NdBAkJh7XP8K2poryXh7NMh1qhD1fSah987Z5Ws7uAOwxEAw4c9PND2 NPjg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=t4uuEf0Rdqki78Zrbxaj+3q3dQ9rrzkccpAd/gVC1/E=; fh=WdkSJffPD9/eg/O7ngQyBkrqA6jfgFcp5Tr2BNWPkxs=; b=ewwgFa1OlJiVmZr8OGQ1/KMM8hYZJRx+/Lk+/6E8Y1FfhT2LzqxeTLjzVWbJ1+59pH 1jwvNqatG0l1fHOCu/S086GHPd0HIgd/33FOztQgMBOCkxtcyzLiCkIqIvr3A/QXLb4S tetWSkV/qrOkXavgywH1rzxAEMfajPqOZcyUUd/cUmm4HKSQkTU8R2aPzJ8YA6k7UL/9 vqp6CiOnsKGekZqBw9DH8pO19QQaDfyYXk+/jSfOCfXuEm9N1cCwUrvud462anO5Auxq xTi4rJ/vebhoGlzmSSYH7BWvudNCFE1luBT8jF4795DTH6+vwNaZwE1q+99m5voP+D0z h+xg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@marvell.com header.s=pfpt0220 header.b="Fr/9hpKa"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=REJECT dis=NONE) header.from=marvell.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id b23-20020aa7c6d7000000b00522d85a7617si1148093eds.244.2023.08.02.05.17.36; Wed, 02 Aug 2023 05:17:59 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@marvell.com header.s=pfpt0220 header.b="Fr/9hpKa"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=REJECT dis=NONE) header.from=marvell.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233722AbjHBKwr (ORCPT <rfc822;maxi.paulin@gmail.com> + 99 others); Wed, 2 Aug 2023 06:52:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36210 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230245AbjHBKwn (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 2 Aug 2023 06:52:43 -0400 Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 95E1B1BD; Wed, 2 Aug 2023 03:52:42 -0700 (PDT) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3729FC7u015426; Wed, 2 Aug 2023 03:52:36 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : mime-version : content-transfer-encoding : content-type; s=pfpt0220; bh=t4uuEf0Rdqki78Zrbxaj+3q3dQ9rrzkccpAd/gVC1/E=; b=Fr/9hpKaFI1U/rUIl+AzcHOmXQqmJhxgfVHUPly5Ta9NwsNTlWnMBLx73SI7XFG5J5kK cfJ+j719qMI4D4WM26lw4JjqDQst9OeAOpd3/ddFOtHJ7KXuCmZWJY1fnLaCW5U4Qk4s hIKlvtHqjus4oXo1wiv0qfm5clgTxW1cYpl1zjKUCqJKf8XyGGPzKUoR9WrX2fqk8PEa EWmg6ZwQdDDs+pmiFGs3HyPsoigN2QPxxPVa54diTV1QhDcB9xlqXXL4uJ0pAoH5ve3j KdZGWv1wTRAkubqLJRWHqnxCPC9lUcb8pQdj3LiW+L7r8+FBekEUsjXzz6ZUilMHG27u 0A== Received: from dc5-exch01.marvell.com ([199.233.59.181]) by mx0b-0016f401.pphosted.com (PPS) with ESMTPS id 3s529kc918-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Wed, 02 Aug 2023 03:52:35 -0700 Received: from DC5-EXCH01.marvell.com (10.69.176.38) by DC5-EXCH01.marvell.com (10.69.176.38) with Microsoft SMTP Server (TLS) id 15.0.1497.48; Wed, 2 Aug 2023 03:52:33 -0700 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH01.marvell.com (10.69.176.38) with Microsoft SMTP Server id 15.0.1497.48 via Frontend Transport; Wed, 2 Aug 2023 03:52:33 -0700 Received: from marvell-OptiPlex-7090.marvell.com (unknown [10.28.36.165]) by maili.marvell.com (Postfix) with ESMTP id 3D42F3F7086; Wed, 2 Aug 2023 03:52:29 -0700 (PDT) From: Ratheesh Kannoth <rkannoth@marvell.com> To: <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org> CC: <sgoutham@marvell.com>, <gakula@marvell.com>, <sbhatta@marvell.com>, <hkelam@marvell.com>, <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>, Ratheesh Kannoth <rkannoth@marvell.com> Subject: [PATCH net] octeontx2-pf: Set maximum queue size to 16K Date: Wed, 2 Aug 2023 16:22:27 +0530 Message-ID: <20230802105227.3691713-1-rkannoth@marvell.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Proofpoint-GUID: Du8bMgPQRCRyS_7yw1zxAB20XxjgtS7q X-Proofpoint-ORIG-GUID: Du8bMgPQRCRyS_7yw1zxAB20XxjgtS7q X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-08-02_06,2023-08-01_01,2023-05-22_02 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED 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-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773119660351144543 X-GMAIL-MSGID: 1773119660351144543 |
Series |
[net] octeontx2-pf: Set maximum queue size to 16K
|
|
Commit Message
Ratheesh Kannoth
Aug. 2, 2023, 10:52 a.m. UTC
page_pool_init() return error on requesting ring size > 32K.
PF uses page pool for rx. octeon-tx2 Supported queue size
are 16, 64, 256, 1K, 2K, 4K, 16K, 64K. If user try to
configure larger ring size for rx, return error.
Fixes: b2e3406a38f0 ("octeontx2-pf: Add support for page pool")
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
From: Ratheesh Kannoth <rkannoth@marvell.com> Date: Wed, 2 Aug 2023 16:22:27 +0530 > page_pool_init() return error on requesting ring size > 32K. > PF uses page pool for rx. octeon-tx2 Supported queue size > are 16, 64, 256, 1K, 2K, 4K, 16K, 64K. If user try to > configure larger ring size for rx, return error. > > Fixes: b2e3406a38f0 ("octeontx2-pf: Add support for page pool") > Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com> > --- > drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c > index c47d91da32dc..978e371008d6 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c > @@ -378,7 +378,7 @@ static void otx2_get_ringparam(struct net_device *netdev, > struct otx2_nic *pfvf = netdev_priv(netdev); > struct otx2_qset *qs = &pfvf->qset; > > - ring->rx_max_pending = Q_COUNT(Q_SIZE_MAX); > + ring->rx_max_pending = 16384; /* Page pool support on RX */ This is very hardcodish. Why not limit the Page Pool size when creating instead? It's perfectly fine to have a queue with 64k descriptors and a Page Pool with only ("only" :D) 16k elements. Page Pool size affects only the size of the embedded ptr_ring, which is used for indirect (locking) recycling. I would even recommend to not go past 2k for PP sizes, it makes no sense and only consumes memory. > ring->rx_pending = qs->rqe_cnt ? qs->rqe_cnt : Q_COUNT(Q_SIZE_256); > ring->tx_max_pending = Q_COUNT(Q_SIZE_MAX); > ring->tx_pending = qs->sqe_cnt ? qs->sqe_cnt : Q_COUNT(Q_SIZE_4K); Thanks, Olek
On Wed, 2 Aug 2023 18:11:35 +0200 Alexander Lobakin wrote: > > - ring->rx_max_pending = Q_COUNT(Q_SIZE_MAX); > > + ring->rx_max_pending = 16384; /* Page pool support on RX */ > > This is very hardcodish. Why not limit the Page Pool size when creating > instead? It's perfectly fine to have a queue with 64k descriptors and a > Page Pool with only ("only" :D) 16k elements. > Page Pool size affects only the size of the embedded ptr_ring, which is > used for indirect (locking) recycling. I would even recommend to not go > past 2k for PP sizes, it makes no sense and only consumes memory. Should we make the page pool cap the size at 32k then, instead of having drivers do the gymnastics? I don't see what else the driver can do, other than cap :S
> From: Alexander Lobakin <aleksander.lobakin@intel.com> > Sent: Wednesday, August 2, 2023 9:42 PM > To: Ratheesh Kannoth <rkannoth@marvell.com> > Subject: [EXT] Re: [PATCH net] octeontx2-pf: Set maximum queue size to 16K > +ring->rx_max_pending = 16384; /* Page pool support on RX */ > > This is very hardcodish. Why not limit the Page Pool size when creating > instead? It's perfectly fine to have a queue with 64k descriptors and a Page > Pool with only ("only" :D) 16k elements. > Page Pool size affects only the size of the embedded ptr_ring, which is used > for indirect (locking) recycling. I would even recommend to not go past 2k for > PP sizes, it makes no sense and only consumes memory. These recycling will impact on performance, right ? else, why didn't page pool made this size as constant.
From: Ratheesh Kannoth <rkannoth@marvell.com> Date: Thu, 3 Aug 2023 02:08:18 +0000 >> From: Alexander Lobakin <aleksander.lobakin@intel.com> >> Sent: Wednesday, August 2, 2023 9:42 PM >> To: Ratheesh Kannoth <rkannoth@marvell.com> >> Subject: [EXT] Re: [PATCH net] octeontx2-pf: Set maximum queue size to 16K > >> +ring->rx_max_pending = 16384; /* Page pool support on RX */ >> >> This is very hardcodish. Why not limit the Page Pool size when creating >> instead? It's perfectly fine to have a queue with 64k descriptors and a Page >> Pool with only ("only" :D) 16k elements. >> Page Pool size affects only the size of the embedded ptr_ring, which is used >> for indirect (locking) recycling. I would even recommend to not go past 2k for >> PP sizes, it makes no sense and only consumes memory. > > These recycling will impact on performance, right ? else, why didn't page pool made this size as constant. Page Pool doesn't need huge ptr_ring sizes to successfully recycle pages. Especially given that the recent PP optimizations made locking recycling happen much more rarely. If you prove with some performance numbers that creating page_pools with the ptr_ring size of 2k when the rings have 32k descriptors really hurt the throughput comparing to 16k PP + 32k rings, I'll change my mind. Re "size as constant" -- because lots of NICs don't need more than 256 or 512 descriptors and it would be only a waste to create page_pools with huge ptr_rings for them. Queue sizes bigger than 1024 (ok, maybe 2048) is the moment when the linear scale stops working. That's why I believe that going out of [64, 2048] for page_pools doesn't make much sense. Thanks, Olek
> From: Alexander Lobakin <aleksander.lobakin@intel.com> > Sent: Thursday, August 3, 2023 8:37 PM > To: Ratheesh Kannoth <rkannoth@marvell.com> > Subject: [EXT] Re: [PATCH net] octeontx2-pf: Set maximum queue size to 16K > > These recycling will impact on performance, right ? else, why didn't page > pool made this size as constant. > > Page Pool doesn't need huge ptr_ring sizes to successfully recycle pages. > Especially given that the recent PP optimizations made locking recycling > happen much more rarely. Got it. Thanks. > Re "size as constant" -- because lots of NICs don't need more than 256 or 512 > descriptors and it would be only a waste to create page_pools with huge > ptr_rings for them. Queue sizes bigger than 1024 (ok, maybe > 2048) is the moment when the linear scale stops working. That's why I > believe that going out of [64, 2048] for page_pools doesn't make much > sense. So, will clamp to 2048 in page_pool_init() ? But it looks odd to me, as User requests > 2048, but will never be aware that it is clamped to 2048. Better do this clamping in Driver and print a warning message ? -Ratheesh
From: Ratheesh Kannoth <rkannoth@marvell.com> Date: Fri, 4 Aug 2023 02:25:55 +0000 >> From: Alexander Lobakin <aleksander.lobakin@intel.com> >> Sent: Thursday, August 3, 2023 8:37 PM >> To: Ratheesh Kannoth <rkannoth@marvell.com> >> Subject: [EXT] Re: [PATCH net] octeontx2-pf: Set maximum queue size to 16K > > >>> These recycling will impact on performance, right ? else, why didn't page >> pool made this size as constant. >> >> Page Pool doesn't need huge ptr_ring sizes to successfully recycle pages. >> Especially given that the recent PP optimizations made locking recycling >> happen much more rarely. > Got it. Thanks. > >> Re "size as constant" -- because lots of NICs don't need more than 256 or 512 >> descriptors and it would be only a waste to create page_pools with huge >> ptr_rings for them. Queue sizes bigger than 1024 (ok, maybe >> 2048) is the moment when the linear scale stops working. That's why I >> believe that going out of [64, 2048] for page_pools doesn't make much >> sense. > So, will clamp to 2048 in page_pool_init() ? But it looks odd to me, as > User requests > 2048, but will never be aware that it is clamped to 2048. Why should he be aware of that? :D But seriously, I can't just say: "hey, I promise you that your driver will work best when PP size is clamped to 2048, just blindly follow", it's more of a preference right now. Because... > Better do this clamping in Driver and print a warning message ? ...because you just need to test your driver with different PP sizes and decide yourself which upper cap to set. If it works the same when queues are 16k and PPs are 2k versus 16k + 16k -- fine, you can stop on that. If 16k + 16k or 16 + 8 or whatever works better -- stop on that. No hard reqs. Just don't cap maximum queue length due to PP sanity check, it doesn't make sense. > > -Ratheesh Thanks, Olek
On Fri, 4 Aug 2023 16:43:51 +0200 Alexander Lobakin wrote: > > So, will clamp to 2048 in page_pool_init() ? But it looks odd to me, as > > User requests > 2048, but will never be aware that it is clamped to 2048. > > Why should he be aware of that? :D > But seriously, I can't just say: "hey, I promise you that your driver > will work best when PP size is clamped to 2048, just blindly follow", > it's more of a preference right now. Because... > > > Better do this clamping in Driver and print a warning message ? > > ...because you just need to test your driver with different PP sizes and > decide yourself which upper cap to set. If it works the same when queues > are 16k and PPs are 2k versus 16k + 16k -- fine, you can stop on that. > If 16k + 16k or 16 + 8 or whatever works better -- stop on that. No hard > reqs. > > Just don't cap maximum queue length due to PP sanity check, it doesn't > make sense. IDK if I agree with you here :S Tuning this in the driver relies on the assumption that the HW / driver is the thing that matters. I'd think that the workload, platform (CPU) and config (e.g. is IOMMU enabled?) will matter at least as much. While driver developers will end up tuning to whatever servers they have, random single config and most likely.. iperf. IMO it's much better to re-purpose "pool_size" and treat it as the ring size, because that's what most drivers end up putting there. Defer tuning of the effective ring size to the core and user input (via the "it will be added any minute now" netlink API for configuring page pools)... So capping the recycle ring to 32k instead of returning the error seems like an okay solution for now.
> From: Jakub Kicinski <kuba@kernel.org> > Sent: Saturday, August 5, 2023 2:05 AM > To: Alexander Lobakin <aleksander.lobakin@intel.com> > Subject: Re: [EXT] Re: [PATCH net] octeontx2-pf: Set maximum queue size to > 16K > > IDK if I agree with you here :S Tuning this in the driver relies on the > assumption that the HW / driver is the thing that matters. > I'd think that the workload, platform (CPU) and config (e.g. is IOMMU > enabled?) will matter at least as much. While driver developers will end up > tuning to whatever servers they have, random single config and most likely.. > iperf. > > IMO it's much better to re-purpose "pool_size" and treat it as the ring size, > because that's what most drivers end up putting there. > Defer tuning of the effective ring size to the core and user input (via the "it > will be added any minute now" netlink API for configuring page pools)... > > So capping the recycle ring to 32k instead of returning the error seems like an > okay solution for now. Either of the solutions looks Okay to me. Let me push a patch with Jacub's proposal for now. -Ratheesh.
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c index c47d91da32dc..978e371008d6 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c @@ -378,7 +378,7 @@ static void otx2_get_ringparam(struct net_device *netdev, struct otx2_nic *pfvf = netdev_priv(netdev); struct otx2_qset *qs = &pfvf->qset; - ring->rx_max_pending = Q_COUNT(Q_SIZE_MAX); + ring->rx_max_pending = 16384; /* Page pool support on RX */ ring->rx_pending = qs->rqe_cnt ? qs->rqe_cnt : Q_COUNT(Q_SIZE_256); ring->tx_max_pending = Q_COUNT(Q_SIZE_MAX); ring->tx_pending = qs->sqe_cnt ? qs->sqe_cnt : Q_COUNT(Q_SIZE_4K);