lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <93ae82e6-44f4-4f2f-b3b6-71240f84500c@sima.ai>
Date: Mon, 26 May 2025 09:17:05 -0700
From: Nikunj Kela <nikunj.kela@...a.ai>
To: Yanteng Si <si.yanteng@...ux.dev>, andrew+netdev@...n.ch,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, mcoquelin.stm32@...il.com, alexandre.torgue@...s.st.com
Cc: rmk+kernel@...linux.org.uk, 0x1207@...il.com, netdev@...r.kernel.org,
 linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: stmmac: set multicast filter to zero if feature is
 unsupported


On 5/25/25 7:09 PM, Yanteng Si wrote:
> 在 5/24/25 6:19 AM, Nikunj Kela 写道:
>> Hash based multicast filtering is an optional feature. Currently,
>> driver overrides the value of multicast_filter_bins based on the hash
>> table size. If the feature is not supported, hash table size reads 0
>> however the value of multicast_filter_bins remains set to default
>> HASH_TABLE_SIZE which is incorrect. Let's override it to 0 if the
>> feature is unsupported.
>>
>> Signed-off-by: Nikunj Kela <nikunj.kela@...a.ai>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 085c09039af4..ccea9f811a05 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -7241,6 +7241,9 @@ static int stmmac_hw_init(struct stmmac_priv 
>> *priv)
>>                       (BIT(priv->dma_cap.hash_tb_sz) << 5);
>>               priv->hw->mcast_bits_log2 =
>> ilog2(priv->hw->multicast_filter_bins);
>> +        } else {
>> +            priv->hw->multicast_filter_bins = 0;
>> +            priv->hw->mcast_bits_log2 = 0;
>>           }
>
> I didn't read the code carefully, just did a simple search:
>
> ❯ grep -rn multicast_filter_bins drivers/net/
> drivers/net/ethernet/stmicro/stmmac/common.h:611:    unsigned int 
> multicast_filter_bins;
> drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c:26: 
> plat_dat->multicast_filter_bins = 0;
> ***
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:512: 
> plat->multicast_filter_bins = HASH_TABLE_SIZE;
> ***
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:536: 
> &plat->multicast_filter_bins);
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:541: 
> plat->multicast_filter_bins = dwmac1000_validate_mcast_bins(
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:542: &pdev->dev, 
> plat->multicast_filter_bins);
> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c:523: 
> mac->multicast_filter_bins = priv->plat->multicast_filter_bins;
> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c:527:    if 
> (mac->multicast_filter_bins)
> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c:528: 
> mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:516: 
> (netdev_mc_count(dev) > hw->multicast_filter_bins)) {
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:1527: 
> mac->multicast_filter_bins = priv->plat->multicast_filter_bins;
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:1531:    if 
> (mac->multicast_filter_bins)
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:1532: 
> mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:1568: 
> mac->multicast_filter_bins = priv->plat->multicast_filter_bins;
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:1572:    if 
> (mac->multicast_filter_bins)
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c:1573: 
> mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
> drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:543:    if 
> (netdev_mc_count(priv->dev) >= priv->hw->multicast_filter_bins)
> drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:633:    if 
> (netdev_mc_count(priv->dev) >= priv->hw->multicast_filter_bins)
> drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c:679:    if 
> (netdev_mc_count(priv->dev) >= priv->hw->multicast_filter_bins)
> drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:31: 
> plat->multicast_filter_bins = HASH_TABLE_SIZE;
> drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c:84: 
> plat->multicast_filter_bins = HASH_TABLE_SIZE;
> drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c:98: 
> plat->multicast_filter_bins = 256;
> drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c:365: 
> mac->multicast_filter_bins = priv->plat->multicast_filter_bins;
> drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c:369:    if 
> (mac->multicast_filter_bins)
> drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c:370: 
> mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:7240: 
> priv->hw->multicast_filter_bins =
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:7243: 
> ilog2(priv->hw->multicast_filter_bins);
> drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c:570: 
> plat->multicast_filter_bins = HASH_TABLE_SIZE;
> drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c:707: 
> plat->multicast_filter_bins = HASH_TABLE_SIZE;
> drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c:479: 
> plat_dat->multicast_filter_bins = 0;
> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c:456:    int 
> numhashregs = (hw->multicast_filter_bins >> 5);
> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c:485: 
> (netdev_mc_count(dev) > hw->multicast_filter_bins)) {
> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c:1057: 
> mac->multicast_filter_bins = priv->plat->multicast_filter_bins;
> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c:1061:    if 
> (mac->multicast_filter_bins)
> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c:1062: 
> mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
> drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c:43: 
> plat_dat->multicast_filter_bins = HASH_TABLE_SIZE;
>
> and
>
> drivers/net/ethernet/stmicro/stmmac/common.h:265:#define 
> HASH_TABLE_SIZE 64
>
>
> From the search results, the default value of multicast_filter_bins
> may be meaningful. And I think that even if some hardware does not
> support this feature, it should still be overridden in its own directory.

There is a DT property 'snps,multicast-filter-bins' available to 
override the default value for a platform however this property is not 
taken into consideration in case of xgmac. That being said, 
stmmac_platform.c logic can be modified to extend the property use for 
xgmac also however the value will be overridden later based on hash 
table size read from the HW Feature register. So only zero value will be 
usable via DT in that case. Hence I thought of setting it to 0 in the 
else part of the code I modified in this patch. If you think I should 
extend the DT property for xgmac, I can modify the patch too.

Thanks,

-Nikunj

>
>
> Thanks,
> Yanteng
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ