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 <net/if.h>, 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
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
