Re: kern/48104: Incorrect forwarding of broadcast packets by bridge(4)

[Ryota Ozaki <ozaki-r%NetBSD.org@localhost>]

Hi Lloyd,

On Mon, Feb 24, 2014 at 7:39 PM, Ryota Ozaki <ozaki-r%iij.ad.jp@localhost> 
wrote:
>
> (2014/02/21 9:25), Lloyd Parkes wrote:
>> Hi all,
>> I finally got around to running the last test on my changes to bridge(4) and 
>> it all works well.
>>
>> The only real problem I had to think about was that the point in the network 
>> stack for tapping off bridged packets was above the point in the network 
>> stack where packets are injected. This means that when multicasting packets 
>> up the network stack you have to do something to prevent a packet storm. My 
>> initial implementation used a link local mbuf flag, but bridging is 
>> inherently not link local, so I discarded that method.
>>
>> I settled on moving the point at which the bridge taps into the network 
>> stack down the stack so that it is now below the point at which packets are 
>> injected into the stack. In particular, I removed the bridge code from 
>> ether_input() and made the bridge update the struct ifnet if_input field 
>> when an interface is added to or removed from a bridge. The code for 
>> updating the if_input field is purposefully simple and conservative.
>>
>
> I prefer this reconstruction of ether_input and bridge_input.
> Actually I (and my coworker) are thinking the approach to make
> the part simple.
>
> I'm checking and testing your patch.

I've got a problem that iperf -s on a tap with a bridge doesn't work
with your patch. It works w/o your patch. The instructions are
as follows:


# ifconfig bridge0 create
# ifconfig bridge0 up
# ifconfig tap0 create
# ifconfig tap0 192.168.122.67/24 up
# brconfig bridge0 add tap0
# brconfig bridge0 add vioif0
# iperf -s
------------------------------------------------------------
Server listening on TCP port 5001
TCP window size: 32.0 KByte (default)
------------------------------------------------------------
[  4] local 192.168.122.67 port 5001 connected with 192.168.122.1 port 56964
vioif0: rx mbuf allocation failed, error code 55
vioif0: rx mbuf allocation failed, error code 55
vioif0: rx mbuf allocation failed, error code 55
vioif0: rx mbuf allocation failed, error code 55
vioif0: rx mbuf allocation failed, error code 55
vioif0: rx mbuf allocation failed, error code 55


Do you have any idea on the error?

Regards,
  ozaki-r

>
>> There are some interface drivers that call ether_input() directly instead of 
>> indirecting through struct ifnet if_input and that seems to be a requirement 
>> that was missed when porting the drivers from FreeBSD. I'll PR those drivers 
>> once I finish this email and send this patch to kern/48104.
>>
>> Overall this patch removes 107 of old code and adds 85 lines of new code. I 
>> know it's a crude code metric, but I do like increased functionality with 
>> less code.
>
> me too :)
>
> Regards,
>   ozaki-r
>
>>
>> Cheers,
>> Lloyd
>