On 07/06/16(Tue) 12:36, Masao Uebayashi wrote:
> On Mon, Jun 06, 2016 at 12:50:04PM +0200, Martin Pieuchot wrote:
> > On 06/06/16(Mon) 16:23, Masao Uebayashi wrote:
> > > On Mon, Jun 06, 2016 at 08:50:49AM +0200, Martin Pieuchot wrote:
> > > > On 06/06/16(Mon) 13:04, Masao Uebayashi wrote:
> > > > > Broadcast frame, coming into a bridge'ed interface, passes if_input() 
> > > > > 3 times,
> > > > > and actually input (ether_input()) twice.
> > > > > 
> > > > > - A frame enters an interface (e.g. pair(4)), the interface calls 
> > > > > if_input()
> > > > >   on it.  The frame is queued in if_input_queue.
> > > > > 
> > > > > - A task running if_input_process() is triggered.  It takes the frame 
> > > > > and
> > > > >   calls bridge_input().  Frame is queued in bridgeintrq.
> > > > > 
> > > > > - bridge_process() dispatches frame as multicast/broadcast (if
> > > > >   (ETHER_IS_MULTICAST())) and calls bridge_ifinput() on it, then 
> > > > > passes the
> > > > >   frame to bridgeintr_frame().
> > > > > 
> > > > > - bridgeintr_frame() calls bridge_broadcast() on it.
> > > > > 
> > > > > - bridge_broadcast() calls bridge_localbroadcast(), which again calls
> > > > >   bridge_ifinput().
> > > > > 
> > > > > bridge_ifinput() is called twice for each broadcast frames.  
> > > > > bridge_ifinput()
> > > > > calls if_input().  Thus 3 if_input() for each.
> > > > > 
> > > > > These duplicate frames confuse pppoe(4), that's why it stops working.
> > > > 
> > > > What do you mean by "confuse pppoe(4)"?  I still don't understand what's
> > > > the link between pppoe(4) and bpf(4) in this case and why BPF matters
> > > > for a kernel driver.
> > > 
> > > - PPPoE client (pppoe(4)) sends a PPPoE Discovery "initiation" frame, 
> > > which is
> > >   broadcast.
> > > 
> > > - PPPoE server (npppd(8)) receives 3 copies of it via bpf(4), then returns
> > >   3 PPPoE Discovery "offer" frames.
> > > 
> > > - pppoe(4) receives 3 "offer" frames and gets confused ... somehow.
> > > 
> > > I don't know the internal of pppoe(4) yet.  pppoe(4) might have a bug, it
> > > might be able to work even if it receives 3 replies at once, I don't know.
> > > 
> > > I don't think that npppd(8) receiving 3 copies of broadcast frames (via
> > > bpf(4)) is an intentional design anyway.
> > 
> > I agree.  Diff below should reduce the number of copies to 2.
> 
> Diff works as expected for me, and reads me good.  OK uebayasi@.
> 
> > In order
> > to remove the last copy somebody has to turn bridge_process() MP-safe
> > and merge it with bridge_input().  
> > 
> > Once this is done, bridge_input() can return "0" for multicast packets
> > and ether_input() will be call directly without the need for requeueing
> > the packet.
> 
> I admit I don't fully understand the intention of if_ih_* handler.  Can
> you safely assume that there is always ether_input() hook just after
> bridge_input()?

Since bridge(4) mostly work with Ethernet drivers, you can assume that
it is after.  Maybe you'll have a vlan_input() before.

Reply via email to