bridge callbacks in if_ed.c?

[Sam Leffler]

On Sunday 05 September 2004 02:53 pm, Andre Oppermann wrote:
> Gleb Smirnoff wrote:
> > On Sun, Sep 05, 2004 at 02:20:36PM -0700, Luigi Rizzo wrote:
> > L> > I see that bridge callbacks are still living in if_ed.c
> > L> > from FreeBSD 2.x times. See if_ed.c:2816. I think this is
> > L> > not correct.
> > L> >
> > L> > Bridge code is called from ether_input(), which is
> > L> > indirectly called from if_ed.c:2836.
> > L> >
> > L> > Any objections about attached patch?
> > L>
> > L> there are performance reasons to do this way -- grabbing
> > L> the entire packet is expensive because it is done via programmed
> > L> I/O, so the current code only grabs the header, does the
> > L> filtering, and grabs the rest of the packet only if
> > L> needed.
> >
> > Well, what percentage of packets is usually dropped by bridge in
> > normal operation? Performance degradation applies only to them.
>
> That depends on how many systems are behind the bridge.  Every packet
> not needing to go to the other side is dropped.
>
> > L> Probably the current code runs bridge_in_ptr() twice, but I
> > L> believe this is still cheaper than grabbing all packets
> > L> entirely.
> > L> I'd rather not apply the patch unless you can show that
> > L> the current code leads to incorrect behaviour.
> >
> > The problem is with layer intermixing. ed(4) is a device
> > driver, it must pass its frames to Ethernet stack
> > without any hacks. By the way ed(4) is the only driver
> > which does this.
>
> Yes.  And I fully agree with Matt here.  Nobody in their right mind
> would use ed(4) for *anything* these days.  The best would probably
> to remove ed(4) alltogether.  (Only half kidding).
>
> I fully support removing this hack from ed(4) to get the API right
> again.  *If* someone is actually affected by this I'm going to buy
> him a shiny fxp card off Ebay for $3.59.  I don't want to offend
> Luigi at all but the times are over to support such major layering
> violations these days for such an obscure piece of hardware.  This
> 6-current now and we don't even support 386 CPUs anymore.
>
> > Actually I'm working on the problem decribed here
> >
> > http://lists.freebsd.org/mailman/htdig/freebsd-net/2004-May/003881.html
> >
> > and one of the approaches I'm considering is to push the
> > block (lines 569-615) from if_ethersubr.c into bridge.c. This
> > probably requires small changes to bridge_in()/bdg_forward()
> > logic, so it's caller must take care. We have only two callers
> > now - ether_input(), which is OK and if_ed, which looks like
> > a hack.
>
> I'm not sure if I can follow the graph correctly.  Shouldn't the
> straight line between where ng_ether_input and ng_ether_rcvdata
> branch be disconnected?  The way it is drawn there looks like the
> packet is arrving double in the upper half?
>
> > P.S. Sam said, that there are plans to convert bridge to use pfil-hooks.
> > If this happens, the code in if_ed.c will be on the way again.
>
> No.  What will move to pfil_hooks is the firewalling within the bridge
> code and the layer2 ethernet firewalling.  The bridging code as such
> will stay where it is.

Well, that's what _you_ want to do :).  What I started on last year was a 
complete purge of special-purpose hooks in the network stack; replacing them 
with pfil hooks (adding new hooks to do this).  Revamping the bridge code was 
part of this work and to do it I had to eliminate the API used by the ed 
driver.  As to whether you're planning to follow the path I started is 
another matter (I pointed Gleb at you because you seemed to be going in that 
direction and I figured you'd "see the light" after looking at the p4 branch 
code I sent you :)).

Back to the original question, the issue is that for some devices you can save 
a lot by fetching only the header in order to do the bridge-routing logic.  
However since modern devices dma the entire packet into a pre-allocated 
buffer you typically gain little by this optimizatoin.  This can still be a 
win as you can short-circuit some work by making this check before forming a 
complete packet and passing it up the stack but for most systems I don't 
figure this is important.  If you're trying to build a high-performance 
bridge you're likely going to collapse a lot of this logic anyway and the 
bridge code in question would be combined with other logic.

So my vote is to nuke the special code in the ed driver (and anywhere else; I 
recall there being one other driver that had similar logic) and make the 
bridge api more opaque.

	Sam