[PATCH] Ethernet cleanup; 802.1p input and M_PROMISC
Bruce M Simpson
bms at incunabulum.net
Mon Mar 5 14:42:10 UTC 2007
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
Perhaps Kip, who is working on 10GbE performance just now, can advise
> 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
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.
More information about the freebsd-net