From owner-freebsd-net@FreeBSD.ORG Mon Mar 5 14:35:13 2007 Return-Path: X-Original-To: freebsd-net@freebsd.org Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 4C7DA16A401 for ; Mon, 5 Mar 2007 14:35:13 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.freebsd.org (Postfix) with ESMTP id A1AEB13C4B3 for ; Mon, 5 Mar 2007 14:35:12 +0000 (UTC) (envelope-from andre@freebsd.org) Received: (qmail 30633 invoked from network); 5 Mar 2007 14:07:12 -0000 Received: from c00l3r.networx.ch (HELO [127.0.0.1]) ([62.48.2.2]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 5 Mar 2007 14:07:12 -0000 Message-ID: <45EC2AA8.8060302@freebsd.org> Date: Mon, 05 Mar 2007 15:35:20 +0100 From: Andre Oppermann User-Agent: Thunderbird 1.5.0.9 (Windows/20061207) MIME-Version: 1.0 To: Yar Tikhiy References: <45E8B964.2090200@incunabulum.net> <20070303215359.GB40430@comp.chem.msu.su> <45EA0756.2000107@incunabulum.net> <20070304070458.GG40430@comp.chem.msu.su> <45EB750A.90105@incunabulum.net> <20070305142411.GC57253@comp.chem.msu.su> In-Reply-To: <20070305142411.GC57253@comp.chem.msu.su> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-net@freebsd.org, Bruce M Simpson Subject: Re: [PATCH] Ethernet cleanup; 802.1p input and M_PROMISC X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Mar 2007 14:35:13 -0000 Yar Tikhiy wrote: > On Mon, Mar 05, 2007 at 01:40:26AM +0000, Bruce M Simpson wrote: >> Yar Tikhiy wrote: >>> Now I see your point, thanks! Well, at least in theory, the driver >>> shouldn't call ether_input() if the interface isn't running. OTOH, >>> the interface shouldn't be getting traffic if it's !UP. However, >>> I suspect that not all drivers handle IFF_UP fully or even can do >>> it at all due to hardware limitations. As I understand it, in an >>> ideal world a !UP interface should be deaf and dumb and not interfering >>> in any way with the network still connected to it physically. >>> Therefore discarding inbound traffic from a !UP interface may be a >>> necessary workaround, but it may not be enough. All that boils >>> down to this: The IFF_UP check in ether_input() is more to a sanity >>> check than to the way for IFF_UP to work. Therefore we can add the >>> IFF_DRV_RUNNING sanity check there, too, for completeness. >>> >> Thanks for your explanation. >> >> I'm still not sure I understand why IFF_DRV_RUNNING should be checked >> for in ether_input(). >> >> There is a pretty clear reason for checking for IFF_UP in ether_input(); >> an interface which is configured administratively down should not be >> bringing traffic into the stack, regardless of whether it is a hardware >> device or a pseudo-device. IFF_UP has been in since 4.2BSD; it is more >> or less integral to how the BSD network stack operates. There are >> situations in which a pseudo-device or hardware device could incorrectly >> call ether_input() with such traffic. >> >> Reading , IFF_DRV_RUNNING is documented as meaning 'resources >> are allocated for this device'. Surely such a check is redundant and not >> relevant to the operation of ether_input()? As far as I can tell it is >> similar to the old meaning of IFF_RUNNING, and there are legitimate >> situations in which the hardware or its queues may have stopped >> processing temporarily whilst the interface may be administratively up >> (and thus accepting traffic). >> >> Please correct me if I'm wrong or point out situations where it's >> important IFF_DRV_RUNNING state is checked outside of a driver. Sorry if >> I seem obtuse, but I'm sure I'm missing some detail here. > > My concern is that, with possible callers of ether_input() being > not really *from* but *on behalf* of the interface, e.g., in Netgraph, > IFF_DRV_RUNNING can be a way for the interface driver to tell us: > I'm not ready yet, so don't believe anyone who pretends he has a > packet from me. > > E.g., a vlan(4) interface gets IFF_DRV_RUNNING set only if it is > properly attached to an Ethernet interface (known as the vlan's > parent). AFAIK this is a totally legitimate use of IFF_DRV_RUNNING. > Now assume that a vlan interface is UP but not RUNNING because it's > detached from the parent. If a buggy Netgraph node or another > source of synthetic traffic decides to inject a packet as though > it comes in from the said vlan interface, handling the packet as > usual will be bogus. > > IMHO the IFF_UP check in ether_input() is mostly for a similar > purpose: If all callers of ether_input() were in real and conformant > interface drivers, we shouldn't bother re-checking IFF_UP in > ether_input() either because the driver of a down interface wouldn't > call ether_input() for it in the first place. > > So I view the checks in ether_input() as a way to work around broken > drivers and simplify synthetic callers of ether_input(). In fact, > the whole first part of ether_input() is for that: It essentially > verifies the caller's conformance. I mean the checks for the proper > mbuf flags and length, recvif, etc. > > Of course, we can omit the check for IFF_DRV_RUNNING if we think > that synthetic traffic from an unready interface is OK. But I'm > afraid we shouldn't. > > In addition, I wonder if we can move the conformance checks to a > wrapper function so that conformant drivers don't have to pay the > performance penalty of the "just in case" checks per each inbound > Ethernet packet. Then make it a KASSERT() if it only catches buggy drivers. -- Andre