[PATCH v4 net-next 04/14] net: enetc: add MAC filter for i.MX95 ENETC PF

[Vladimir Oltean]

On Tue, Mar 18, 2025 at 05:19:51AM +0200, Wei Fang wrote:
> You are right, but I'm afraid of the Coverity will report an issue, because the
> pf->mac_list and pf->num_mfe are protected by the mac_list_lock in other
> functions. And enetc4_pf_destroy_mac_list() will be called in other function
> in the subsequent patches. I don't think it is unnecessary.

Sorry, but I can only take the presented code at face value. If the
Coverity tool signals an issue, you can still triage it and explain why
it is a false positive. Or, if it is a real issue, you can add locking
and provide a good justification for it. But the justification is
missing now.

> So for your question about Rx packet loss, although it is a very corner
> case, the solution I can think of is that we can use temporary MAC hash
> filters before deleting MAFT entries and delete them after adding the
> MAFT entries. Can you accept this proposal?

That sounds good. I'm thinking, for each MAC address filter, maybe you
need an information whether it is programmed to hardware as an exact
match filter or a hash filter, and make use of that functionality here:
temporarily make all filters be hash-based ones, and then see how many
you can convert to exact matches. With something like this, it should
also be easier to maximize the use of the exact match table. Currently,
AFAIU, if you have 5 MAC address filters, they will all be stored as hash
filters, even if 4 of them could have been exact matches. Maybe that can
also be improved with such extra information.

> > > +static int enetc4_pf_set_mac_exact_filter(struct enetc_pf *pf, int type)
> > > +{
> > > +	int max_num_mfe = pf->caps.mac_filter_num;
> > > +	struct net_device *ndev = pf->si->ndev;
> > > +	struct enetc_mac_addr *mac_tbl;
> > > +	struct netdev_hw_addr *ha;
> > > +	u8 si_mac[ETH_ALEN];
> > > +	int mac_cnt = 0;
> > > +	int err;
> > > +
> > > +	mac_tbl = kcalloc(max_num_mfe, sizeof(*mac_tbl), GFP_KERNEL);
> > 
> > Can't you know ahead of time, based on netdev_uc_count(), whether you
> > will have space for exact match filters, and avoid unnecessary
> > allocations if not? enetc_mac_list_is_available() seems way too late.
> 
> I can add a check before alloc mac_tbl, but enetc_mac_list_is_available()
> is still needed, because enetc4_pf_add_si_mac_exact_filter() is a common
> function for all Sis, not only for PSI.