Re: [PATCH net-next 08/15] net: enetc: move mac_filter from struct enetc_pf to struct enetc_si

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] On 32-bit platforms, the 64-bit MAC hash table is truncated to 32 bits when programming the hardware, causing MAC addresses that hash to the upper 32 bits of the table to be silently dropped.
--

commit 3178ee41440e498f50696b0d334e000fdca4eaa7
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: move mac_filter from struct enetc_pf to struct enetc_si
    
The mac_filter array currently resides in struct enetc_pf and is used to
track unicast and multicast MAC address filters for the PF. Since struct
enetc_si is the common structure shared between the PF and VF drivers,
move mac_filter into struct enetc_si to prepare for MAC filter support
in the VF driver.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index 4cff2f1237f18..f6920ded9f7ef 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -225,11 +225,12 @@ static void enetc4_pf_set_mac_hash_filter(struct enetc_pf *pf, int type)
>  	struct net_device *ndev = pf->si->ndev;
>  	struct enetc_mac_filter *mac_filter;
>  	struct enetc_hw *hw = &pf->si->hw;
> +	struct enetc_si *si = pf->si;
>  	struct netdev_hw_addr *ha;
>  
>  	netif_addr_lock_bh(ndev);
>  	if (type & ENETC_MAC_FILTER_TYPE_UC) {
> -		mac_filter = &pf->mac_filter[UC];
> +		mac_filter = &si->mac_filter[UC];

[Severity: High]
This isn't a bug introduced by this patch, but on 32-bit platforms, does
dereferencing mac_hash_table truncate the 64-bit filter down to 32 bits?

The mac_hash_table is declared as DECLARE_BITMAP(mac_hash_table, 64),
which evaluates to an array of two unsigned long elements on 32-bit systems.
When configuring the hardware later in this function:

enetc4_pf_set_mac_hash_filter() {
        ...
        enetc4_pf_set_si_uc_hash_filter(hw, 0, *mac_filter->mac_hash_table);
}

Dereferencing mac_hash_table directly like this only passes the first
unsigned long. Since enetc4_pf_set_si_uc_hash_filter() takes a u64
parameter, the 32-bit value will be zero-extended, effectively losing the
upper 32 bits of the MAC hash.

[ ... ]

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index 38fcc7b5fc6da..209a3503609f5 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -111,10 +111,9 @@ static void enetc_set_mac_ht_flt(struct enetc_si *si, int si_idx, int type,
>  	}
>  }
>  
> -static void enetc_sync_mac_filters(struct enetc_pf *pf)
> +static void enetc_sync_mac_filters(struct enetc_si *si)
>  {
> -	struct enetc_mac_filter *f = pf->mac_filter;
> -	struct enetc_si *si = pf->si;
> +	struct enetc_mac_filter *f = si->mac_filter;
>  	int i, pos;

[Severity: High]
This is a pre-existing issue, but does enetc_sync_mac_filters() also truncate
the hash table on 32-bit architectures?

enetc_sync_mac_filters() {
        ...
        enetc_set_mac_ht_flt(si, 0, i, *f->mac_hash_table);
}

Like in enetc4_pf_set_mac_hash_filter(), passing *f->mac_hash_table only
provides the first 32 bits on 32-bit platforms. Furthermore,
enetc_set_mac_ht_flt() accepts an unsigned long for the hash:

static void enetc_set_mac_ht_flt(struct enetc_si *si, int si_idx, int type,
                                 unsigned long hash)
{
        ...
        enetc_port_wr(&si->hw, ENETC_PSIUMHFR1(si_idx),
                      upper_32_bits(hash));
}

Since hash is an unsigned long, upper_32_bits(hash) will always evaluate to 0
on 32-bit platforms. Is there a risk that any unicast or multicast MAC
addresses that hash to bits 32-63 will be silently dropped by the hardware
filter?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605065550.3038579-1-wei.fang@oss.nxp.com?part=8