On Wed, Jun 16, 2010 at 01:37:19PM +0100, Stuart Henderson wrote:
> On 2010/06/16 20:19, Patrick Coleman wrote:
> > >>
> > >
> > > trunk(4) places trunkports into promiscuous mode anyway,
> > > so would it make more sense just to skip the IFF_PROMISC
> > > check here? in which case, there would be no need to
> > > preserve ifp_orig.
> >
> > That makes sense. I wasn't sure if that promiscuous check was there
> > for another reason, so I erred on the side of leaving it in.
>
> oh, of course, this check isn't only used for trunk...
> I think we need to wait until Claudio's around for his input
> and do some testing in various scenarios.
>
> This whole block was added in r1.28 when bridge(4) was added.
>
I think something like the attached version may be the best solution.
I don't like to do the bcmp() for every unicast packet since network cards
come with nice mac filters that make this superfluous.
>From code inspection bridge(4) should handle this case correctly and
trunk(4) will behave better because of this. Then there is carp(4) and
vlan(4) which reenter ether_input(). Both should do the right thing
(vlan checks the flags and carp does a lookup based on the dest mac addr).
In the end such a change needs a lot of testing.
I would like to have a better interface to program the mac address filters
since some cards are now able to filter on multiple unicast addrs at the
same time. This would allow us to not have to enable promisc mode on
carpdevs.
Another issue are unicast IP packets with multicast mac addrs. (e.g. for
carp active active clusters). We need to somehow filter those as well or
bad things happen.
--
:wq Claudio
Index: if_ethersubr.c
===================================================================
RCS file: /cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.144
diff -u -p -r1.144 if_ethersubr.c
--- if_ethersubr.c 3 Jun 2010 16:15:00 -0000 1.144
+++ if_ethersubr.c 22 Jun 2010 16:07:00 -0000
@@ -677,7 +677,7 @@ ether_input(ifp0, eh, m)
* is for us. Drop otherwise.
*/
if ((m->m_flags & (M_BCAST|M_MCAST)) == 0 &&
- (ifp->if_flags & IFF_PROMISC)) {
+ (ifp->if_flags & IFF_PROMISC || ifp0->if_flags & IFF_PROMISC)) {
if (bcmp(ac->ac_enaddr, (caddr_t)eh->ether_dhost,
ETHER_ADDR_LEN)) {
m_freem(m);
