From owner-freebsd-net  Fri Jun  2 11:36:12 2000
Delivered-To: freebsd-net@freebsd.org
Received: from bubba.whistle.com (bubba.whistle.com [207.76.205.7])
	by hub.freebsd.org (Postfix) with ESMTP id B5ED937B602
	for <freebsd-net@freebsd.org>; Fri,  2 Jun 2000 11:36:08 -0700 (PDT)
	(envelope-from archie@whistle.com)
Received: (from archie@localhost)
	by bubba.whistle.com (8.9.3/8.9.2) id LAA09339;
	Fri, 2 Jun 2000 11:35:36 -0700 (PDT)
From: Archie Cobbs <archie@whistle.com>
Message-Id: <200006021835.LAA09339@bubba.whistle.com>
Subject: Re: Patch review request (ng_ether(4))
In-Reply-To: <3937487C.41C67EA6@elischer.org> from Julian Elischer at "Jun 1, 2000 10:39:08 pm"
To: julian@elischer.org (Julian Elischer)
Date: Fri, 2 Jun 2000 11:35:36 -0700 (PDT)
Cc: freebsd-net@freebsd.org
X-Mailer: ELM [version 2.4ME+ PL54 (25)]
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: owner-freebsd-net@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.org

Julian Elischer writes:
> Archie Cobbs wrote:
> > I'd appreciate any reviews of this patch:
> > 
> >   ftp://ftp.whistle.com/pub/archie/misc/NGETHER.patch
> > 
> > What this does is:
> > 
> >   - Change the ng_ether(4) netgraph node type to get rid of the
> >     "divert" and "orphans" hooks, replacing them with these hooks:
> > 
> >       "lower" - connection to the raw device
> >       "upper" - connection to the upper protocol layers
> > 
> >     This should allow much more interesting Ethernet operations
> >     using netgraph, especially things that filter, etc. For example,
> >     bridging can all be done with a netgraph node.
> > 
> >   - Moves all the netgraph-specific functionality out of net/if_ethersubr.c
> >     and into a new file netgraph/ng_ether.c
> > 
> >   - Allows a KLD for ng_ether.ko so your kernel doesn't have to
> >     be compiled with options NETGRAPH to use it. When loaded, all
> >     ethernet interfaces become netgraph nodes as well.. including
> >     PCCARDS inserted later.
> 
> This change removes the ability to only catch packets that are
> unknown to the exisiting infrastructure.(i.e. orphans).
> So you'd have to write code to do that especially in the netgraph
> node..

See below.. 

> The topography of the new interceptions are interesting and useful,
> but I have a couple of concerns.
> 
> 1/ impact. My original design was done as it was, so that in the 
> case of no netgraph option, the path through the driver would
> be unchanged. The new code is implemented so that 
> under all cases there is an extra test and function call,
> as ether_input() calls ether_input2().

I think the impact will be small, pretty much one pointer != NULL
test per packet. The fact that ether_input() has been split into
ether_input() and ether_input2() should not matter because gcc will
optimize away the function call to ether_input2(), because it comes
at the very tail end of ether_input().

> 2/ Compatibility..
> because there is no 'orphans' node, pppoe will probably have to
> pass packets back to ether_input2()
> which will be considerably slower than
> what was going on before.

On second thought I agree with this. I've added back the divert
and orphans hooks with their original functionality (divert is
just an alias for lower).

As soon as I can access freefall again I'll put up a new patch.

-Archie

___________________________________________________________________________
Archie Cobbs   *   Whistle Communications, Inc.  *   http://www.whistle.com


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message