No subject

above, it sounds to me like maybe this logic will change a bit more.

> > > +static int enetc4_pf_wq_task_init(struct enetc_si *si)
> > > +{
> > > +	char wq_name[24];
> > > +
> > > +	INIT_WORK(&si->rx_mode_task, enetc4_pf_do_set_rx_mode);
> > > +	snprintf(wq_name, sizeof(wq_name), "enetc-%s", pci_name(si->pdev));
> > > +	si->workqueue = create_singlethread_workqueue(wq_name);
> > > +	if (!si->workqueue)
> > > +		return -ENOMEM;
> > > +
> > > +	return 0;
> > > +}
> > 
> > Naming scheme is inconsistent here: the function is called "pf" but
> > takes "si" as argument. Same for enetc4_pf_do_set_rx_mode() where the
> > rx_mode_task is part of the station interface structure.
> 
> So change 'pf' to 'psi'?

Sounds better.

> > > +	struct hlist_head mac_list; /* MAC address filter table */
> > 
> > One thing I don't understand is why, given this implementation and
> > final effect, you even bother to keep the mac_list persistently in
> > struct enetc_pf. You have:
> > 
> > enetc4_pf_do_set_rx_mode()
> > -> enetc4_pf_flush_si_mac_exact_filter(ENETC_MAC_FILTER_TYPE_ALL)
> >    -> hlist_for_each_entry_safe(&pf->mac_list)
> >       -> enetc_mac_list_del_entry()
> > 
> > which practically deletes all &pf->mac_list elements every time.
> > So why even store them persistently in the first place? Why not just
> > create an on-stack INIT_HLIST_HEAD() list?
> 
> The enetc4_pf_add_si_mac_exact_filter() and
> enetc4_pf_add_si_mac_exact_filter() are used for all Sis, but only
> PF can access MAFT, and PSI and VSIs may share the same MAFT
> entry, so we need to store them in struct enetc_pf. Although we
> have not added VFs support yet, for such shared functions, we
> should design its implementation from the beginning, rather than
> modifying them when we add the VFs support.

Ok. We need to find a way in which the code also makes sense today
(who knows how much time will pass until VSIs are also supported in the
mainline kernel - we all hope "as soon as possible" but have to plan for
the worst). I don't disagree with the existence of &pf->mac_list,
but it seems slightly inconsistent with the fact that you rebuild it
(for now, completely, but I understand that it the future it will be
just partially) each time ndo_set_rx_mode() is called.

Are you aware of __dev_uc_sync() / __dev_mc_sync()? They are helpers
with explicit sync/unsync callbacks per address, so you don't have to
manually walk using netdev_for_each_uc_addr() / netdev_for_each_mc_addr()
each time, and instead act only on the delta. I haven't thought this
suggestion through, but with you mentioning future VSI mailbox support
for MAC filtering, maybe it is helpful if the PSI's MAC filters are also
structured in this way.