tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/18035 IFF_SIMPLEX vs bridge(4)

>> The patch [...] is almost ludicrously simple:

>>      if ((bif->bif_flags & IFBIF_LEARNING) != 0 &&
>>          ETHER_IS_MULTICAST(eh->ether_shost) == 0 &&
>> +        
>> memcmp(etherbroadcastaddr,eh->ether_dhost,sizeof(etherbroadcastaddr)) &&
>>          (eh->ether_shost[0] == 0 &&
>>           eh->ether_shost[1] == 0 &&
>>           eh->ether_shost[2] == 0 &&
> You should be using ETHER_IS_MULTICAST(eh->ether_dhost) instead of
> memcmp().  That is the standard way.

The semantics are different, though; I found no ETHER_IS_BROADCAST
macro, so I went with memcmp().  It's possible that ETHER_IS_MULTICAST
actually gives the correct semantics, but, since I have no easy way to
test for this problem with multicast-but-not-broadcast packets, I went
for the minimal change.

> I'm wondering whether the 's' in ether_shost in the original code is
> a typo.

Possibly, but I think probably not.  The effect is "never learn
multicast addresses", which strikes me as the right thing.

> AFAIK no protocols send ethernet packets with a multicast source
> address.

I don't know of any either, but I'm definitely not ready to say there
are none.  (I can easily imagine some sort of load-balancing setup that
uses a multicast MAC as if it were an ordinary MAC, for example.)

> And if the ETHER_IS_MULTICAST() check is supposed to ensure that no
> multicast addresses are put into the bridge routing table, the check
> should better be moved inside bridge_rtupdate().

Perhaps.  I don't consider myself competent to judge that, so I didn't
meddle with it.  It does seem to me that the current placement has the
effect of "never automatically learn multicast, but allow them to be
inserted by specific action", which seems to me like TRT: do the
usually-right thing automatically, but allow it to be overridden.

/~\ The ASCII                           der Mouse
\ / Ribbon Campaign
 X  Against HTML      
/ \ Email!           7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B

Home | Main Index | Thread Index | Old Index