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. 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.
Index: net/if_bridge.c
===================================================================
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.279
diff -u -p -r1.279 if_bridge.c
--- net/if_bridge.c 30 May 2016 12:56:16 -0000 1.279
+++ net/if_bridge.c 6 Jun 2016 10:46:01 -0000
@@ -1129,7 +1129,7 @@ bridge_process(struct ifnet *ifp, struct
} else
#endif /* NGIF */
bridge_ifinput(ifp, mc);
-
+
bridgeintr_frame(sc, ifp, m);
return;
}
@@ -1225,14 +1225,15 @@ bridge_broadcast(struct bridge_softc *sc
if (bridge_filterrule(&p->bif_brlout, eh, m) ==
BRL_ACTION_BLOCK)
continue;
- bridge_localbroadcast(sc, dst_if, eh, m);
-
/*
* Don't retransmit out of the same interface where
* the packet was received from.
*/
if (dst_if->if_index == ifp->if_index)
continue;
+
+ bridge_localbroadcast(sc, dst_if, eh, m);
+
#if NMPW > 0
/*
* Split horizon: avoid broadcasting messages from wire to
