Hi,
Thanks for your reply.
Yar Tikhiy wrote:
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.
I agree with the point you make here about non-conforming drivers;
however there are cogent performance arguments for checking IFF_UP
immediately. If an interface is configured administratively down, it
shouldn't be pumping traffic into the network stack. I do however
realize there are situations where this can happen.
Suppose, for example, the thread which calls ether_input() is scheduled
on another CPU. Dropping such frames immediately on entry into
ether_input() saves tying up a thread for any longer than is absolutely
necessary.
Perhaps Kip, who is working on 10GbE performance just now, can advise
further.
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.
Thanks for explaining this further. Perhaps I should put the check for
IFF_DRV_RUNNING under INVARIANTS or make it a KASSERT?
The code in bms_netdev as it stands bends the rules a little. The IFF_UP
check was in ether_demux() before. The original reason for the
ether_input()/ether_demux() split was to accomodate Netgraph. I must
admit that I hadn't fully mapped out the possible re-entry scenarios
with Netgraph because they may be arbitrarily complicated by its very
nature.
Whilst Netgraph is a cool feature, and one I am very grateful that
FreeBSD has, I wonder if it is OK that we should have checks which
potentially pessimize performance for the main use cases to protect the
stack against Netgraph frames which are bogons, or bugs in Netgraph nodes.
I'm open to hearing more about this, but my own resources (time, money)
are a limiting factor as to what I can do.
Regards,
BMS
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
