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

quoted

diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c

b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c

quoted

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)

quoted

 	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.

[ ... ]

quoted

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c

b/drivers/net/ethernet/freescale/enetc/enetc_pf.c

quoted

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,

quoted

 	}
 }

-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?

LS1028A, i.MX95/94 and later i.MX series and S32N7 are all 64-bit architectures.
So this not a real issue.