kern/109815: wrong interface identifier at pfil_hooks for vlans + if_bridge

[Bruce M. Simpson]

Hi,

Eygene Ryabinkin wrote:
> Sure, I can test it, but then I need to know what problems are cured
> by your patch, or I just should watch if it will not break something.
>
>   
My concern is that I want to make sure that all these changes to the 
ether_input path work OK together.

The M_PROMISC flag is set further down when it's determined that a frame 
flowing into ether_input() was received promiscuously, and therefore 
Layer 3 protocols (e.g. IP) may not want to see it.

>> In NetBSD, after if_bridge is given a chance to claim an input frame, the ifp 
>> may be changed if the bridge needs to forward locally.
>>     
>
> In my case if_bridge drops off the packet because firewall fails to
> recognize the packet as good: the interface that is passed to a
> pfil_hooks is bad (I mean not the one expected).
>   
The ifp which your patch changes is that of the mbuf chain when 
bridge_input determines it is not for the bridge, but should be 
forwarded locally. The patch forces a locally forwarded frame to have 
the same ifp as it had when it came into bridge_input. I can foresee 
problems if the same Ethernet destination address exists on multiple 
bridge member interfaces.

The latest version of p4 bms_netdev now updates the cached ifp in 
ether_input() if bridge_input() changed it in this way.

NetBSD consistently uses pfil_hooks for the if_bridge *and* ether_input 
paths, FreeBSD currently calls ipfw directly for ether_input, which may 
make a difference to the behaviour which you are seeing with VLANs.

Not understanding if_bridge fully, or the coupling of ipfw with 
if_ethersubr.c, I would hope that Andrew and others have more to say on 
this.

> Will try to see if your patch makes any difference for the 7-CURRENT,
> but I have no system at hand to test it, sorry.
>   
The patch is extracted from p4 therefore it should apply against 
CURRENT. I haven't updated the patch yet, the latest code is in p4.

We won't be able to eliminate the DEV_CARP checks in this spin. I did 
exchange an idea with Andrew late last night whereby a list of addresses 
other than ether_dhost is maintained for each ifnet. Input paths then 
check this in addition to or instead of ether_dhost.

I've added this to the Wiki.

I've been working particularly hard lately so I'm not 100% clear.

Thanks,
BMS