| Message ID | TY3P286MB2611AD036755E0BC411847FC98D52@TY3P286MB2611.JPNP286.PROD.OUTLOOK.COM (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] net: ethernet: mtk_ppe: Change PPE entries number to 16K | expand |
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
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?
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?
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.
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.
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.
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