[ovs-dev] RFC: correct way to deal with runt packets
blp at nicira.com
Tue Aug 10 20:27:52 UTC 2010
Hi Jesse (and everyone else).
In a recent thread, you pointed out that the kernel and user datapaths
mishandle packets that have too-short IP headers. That is, if a frame
has an Ethernet type of 0x0800 (IP), but the Ethernet header is followed
by less than 20 bytes (the minimum length IP header), then the flow will
have dl_type 0x0800 and the L3 header pointer in the skb will point to
the incomplete IP header, with no easy way for code that receives the
skb and flow to tell that the IP header was missing or truncated.
I can think of a few ways to handle this:
1. Zero out the dl_type instead of claiming that it's IP. It's not
really IP, after all, since that header isn't really there.
There is precedent for this behavior because it's what OVS does
for L4: if the TCP or UDP header is missing, we zero out
nw_proto. It's also what I thought OVS was doing already.
On the other hand, I'm not sure that this is great behavior.
It's kind of weird to lie about the contents of an outer header
based on the absence or incorrectness of an inner header.
This choice also implies that extending OVS to support more
protocols will change the interpretation of headers that we
already support. For example, if we add IPv6 parsing, then
suddenly short IPv6 packets will have their dl_type zeroed out
whereas currently it would be 0x86dd regardless of IPv6 header
correctness. This seems odd to me.
This choice also makes it impossible to set up a flow that will
catch short different kinds of short frames, e.g. if you want
short IPv4 packets you also get short ARP packets. I don't know
if I care.
2. Set the L3 header pointer to null. This works in userspace,
where the L3 header pointer is always a pointer. It doesn't work
gracefully in the kernel, because the L3 header pointer is not
necessarily a pointer there and there's no corresponding
"sentinel" value that means "not really set" (as far as I can
3. Leave the dl_type and L3 header pointer set as they are now, but
make code that cares about them itself check that the packet is
long enough. This has the potential advantage of dropping
branches from the flow_extract() code, which is probably the fast
path here, putting all of the checking burden into the code that
actually needs to change headers, which probably doesn't get hit
4. Just drop short packets.
I'm leaning slightly toward #3.
What do you think?
More information about the dev