[PATCH v2] net: ethernet: mtk_ppe: Change PPE entries number to 16K

[PATCH v2] net: ethernet: mtk_ppe: Change PPE entries number to 16K

[Shengyu Qu]

MT7981,7986 and 7988 all supports 32768 PPE entries, and MT7621/MT7620
supports 16384 PPE entries, but only set to 8192 entries in driver. So
incrase max entries to 16384 instead.

Cc: stable@vger.kernel.org
Signed-off-by: Elad Yifee <eladwf@gmail.com>
Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE")
---
Changes since V1:
 - Reduced max entries from 32768 to 16384 to keep compatible with MT7620/21 devices.
 - Add fixes tag
---
 drivers/net/ethernet/mediatek/mtk_ppe.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.h b/drivers/net/ethernet/mediatek/mtk_ppe.h
index 691806bca372..223f709e2704 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.h
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.h
@@ -8,7 +8,7 @@
 #include <linux/bitfield.h>
 #include <linux/rhashtable.h>
 
-#define MTK_PPE_ENTRIES_SHIFT		3
+#define MTK_PPE_ENTRIES_SHIFT		4
 #define MTK_PPE_ENTRIES			(1024 << MTK_PPE_ENTRIES_SHIFT)
 #define MTK_PPE_HASH_MASK		(MTK_PPE_ENTRIES - 1)
 #define MTK_PPE_WAIT_TIMEOUT_US		1000000
-- 
2.39.2

Re: [PATCH v2] net: ethernet: mtk_ppe: Change PPE entries number to 16K

[Paolo Abeni]

On Tue, 2024-06-25 at 19:16 +0800, Shengyu Qu wrote:
> MT7981,7986 and 7988 all supports 32768 PPE entries, and MT7621/MT7620
> supports 16384 PPE entries, but only set to 8192 entries in driver. So
> incrase max entries to 16384 instead.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Elad Yifee <eladwf@gmail.com>
> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
> Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE")
> ---
> Changes since V1:
>  - Reduced max entries from 32768 to 16384 to keep compatible with MT7620/21 devices.
>  - Add fixes tag

@Sean, @Mark, @Lorenzo or @Felix, can any of you actually test this?

Thanks!

Paolo

Re: [PATCH v2] net: ethernet: mtk_ppe: Change PPE entries number to 16K

[Jakub Kicinski]

On Tue, 25 Jun 2024 19:16:54 +0800 Shengyu Qu wrote:
> MT7981,7986 and 7988 all supports 32768 PPE entries, and MT7621/MT7620
> supports 16384 PPE entries, but only set to 8192 entries in driver. So
> incrase max entries to 16384 instead.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Elad Yifee <eladwf@gmail.com>
> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
> Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE")

nit: Fixes tag usually goes before the sign-offs

This doesn't strike me as a bug fix, tho. What's the user-visible
impact? We can't utilize HW to its full abilities?
-- 
pw-bot: cr

Re: [PATCH v2] net: ethernet: mtk_ppe: Change PPE entries number to 16K

[Shengyu Qu]

[-- Attachment #1.1.1: Type: text/plain, Size: 896 bytes --]

Hi,

BT download and PCDN would create tons of connections, and might be
easily to reach the 8192 limit, one of my friend sees 50000+ links when
hosting PCDN.

Best regards,
Shengyu

在 2024/7/3 2:02, Jakub Kicinski 写道:
> On Tue, 25 Jun 2024 19:16:54 +0800 Shengyu Qu wrote:
>> MT7981,7986 and 7988 all supports 32768 PPE entries, and MT7621/MT7620
>> supports 16384 PPE entries, but only set to 8192 entries in driver. So
>> incrase max entries to 16384 instead.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Elad Yifee <eladwf@gmail.com>
>> Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
>> Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE")
> 
> nit: Fixes tag usually goes before the sign-offs
> 
> This doesn't strike me as a bug fix, tho. What's the user-visible
> impact? We can't utilize HW to its full abilities?

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 6977 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2] net: ethernet: mtk_ppe: Change PPE entries number to 16K

[Jakub Kicinski]

On Thu, 4 Jul 2024 01:38:50 +0800 Shengyu Qu wrote:
> BT download and PCDN would create tons of connections, and might be
> easily to reach the 8192 limit, one of my friend sees 50000+ links when
> hosting PCDN.

I don't know what PCDN is, but what we care about in Linux is whether
the change under Fixes introduced a regression. Optimizations, and 
improvements no matter how trivial in terms of code are not fixes.
So did ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for
initializing the PPE") make things worse. And if you're saying there
are 50k "links" in real world why is 16k a major win? it's 1/3rd of
the total.

Re: [PATCH v2] net: ethernet: mtk_ppe: Change PPE entries number to 16K

[Shengyu Qu]

Hi,

PCDN means P2P CDN[1]; This modification already exists in a heavily
modified openwrt fork here[2] for over 2 years so it should be working
with to regression. Although a higher limit would be better for PCDN use
case, but only newer devices like MT7986 supports 32768 max entries.
Setting to 16384 would keep old devices working.

[1] https://www.w3.org/wiki/Networks/P2P_CDN
[2] https://github.com/coolsnowwolf/lede/blob/2ef8b6a6142798b5e58501fe12ffd10b0961947f/target/linux/ramips/files/drivers/net/ethernet/mtk/mtk_hnat/hnat.h#L604

在 2024/7/4 7:48, Jakub Kicinski 写道:
> On Thu, 4 Jul 2024 01:38:50 +0800 Shengyu Qu wrote:
>> BT download and PCDN would create tons of connections, and might be
>> easily to reach the 8192 limit, one of my friend sees 50000+ links when
>> hosting PCDN.
> 
> I don't know what PCDN is, but what we care about in Linux is whether
> the change under Fixes introduced a regression. Optimizations, and 
> improvements no matter how trivial in terms of code are not fixes.
> So did ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for
> initializing the PPE") make things worse. And if you're saying there
> are 50k "links" in real world why is 16k a major win? it's 1/3rd of
> the total.

Re: [PATCH v2] net: ethernet: mtk_ppe: Change PPE entries number to 16K

[Jakub Kicinski]

On Thu, 4 Jul 2024 19:06:29 +0800 Shengyu Qu wrote:
> PCDN means P2P CDN[1]; This modification already exists in a heavily
> modified openwrt fork here[2] for over 2 years so it should be working
> with to regression. Although a higher limit would be better for PCDN use
> case, but only newer devices like MT7986 supports 32768 max entries.
> Setting to 16384 would keep old devices working.
> 
> [1] https://www.w3.org/wiki/Networks/P2P_CDN
> [2] https://github.com/coolsnowwolf/lede/blob/2ef8b6a6142798b5e58501fe12ffd10b0961947f/target/linux/ramips/files/drivers/net/ethernet/mtk/mtk_hnat/hnat.h#L604

Makes sense but not a fix, please resend without CC stable and without
the fixes tag.