e1000: Correct promiscuous multicast filter handling

[kbowling]

Summary

I saw jtl's report in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=140647

It looked like the promiscuous set had been updated since then, but there were still some of the mentioned issues so I went and stole the algorithm from ixgbe(4) which looks correct to me.

[markj]

sys/dev/e1000/if_em.c

1715

Starting from here, this doesn't match the version of em_if_multi_set() in the main branch. Is there an intermediate diff that's missing?

[kbowling]

sys/dev/e1000/if_em.c

1715

Your are right, I had stable/12 checked out for an MFC. Let me rebase onto main

[kbowling]

Rebased onto main

[kbowling]

sys/dev/e1000/if_em.c

1677–1678

This is a boundary condition, it is only called in em_if_multi_set, and I think with this new logic this maintains the correct count. Eyes around this would be appreciated.

[erj]

sys/dev/e1000/if_em.c

1673

It looks like it would be less confusing to rename "cnt" to something like "idx", since that's what it's used for in this function.

1677–1678

Then, if you think of it like an index, then this checks out -- it'd be over the bounds of mta, and so the address can't be copied.

[kbowling]

Address @erj feedback

[markj]

sys/dev/e1000/if_em.c

1718

Why set these flags here? All three of the cases below will set or clear them.

[kbowling]

sys/dev/e1000/if_em.c

1718

My read is this strips away anything unrelated from the four bytes of E1000_RCTL we get from E1000_READ_REG above. Then as we do the following &= and |= we are doing so against the previous state of just these registers. Then finally we write just these UPE and/or MPE registers. Is that initial bit strip, and frobbing only UPE and MPE at the end sensible or unnecessary?

[kbowling]

sys/dev/e1000/if_em.c

1718

Ok I've taken one more look down to digesting what all the relevant macros are and agree with you. I'll prepare a similar change for ixgbe.

[kbowling]

Remove unnecessary bitwise statement noted by @markj

[markj]

sys/dev/e1000/if_em.c

1718

I see, I didn't catch that this was ported from ixgbe. It was just a minor nit but made me wonder if there was a misunderstanding somewhere.