[ovs-dev] [runt-flows 5/8] datapath: Report memory allocation errors in flow_extract().

[Ben Pfaff]

On Fri, Aug 27, 2010 at 11:36:11AM -0700, Jesse Gross wrote:
> On Fri, Aug 27, 2010 at 9:47 AM, Ben Pfaff <blp at nicira.com> wrote:
> > On Thu, Aug 26, 2010 at 02:56:04PM -0700, Jesse Gross wrote:
> >> On Fri, Aug 13, 2010 at 10:55 AM, Ben Pfaff <blp at nicira.com> wrote:
> >> > Until now flow_extract() has simply returned a bogus flow when memory
> >> > allocation errors occurred.  This fixes the problem by propagating the
> >> > error to the caller.
> >> >
> >> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> >>
> >> Hmm, that's pretty nasty behavior that we had before.
> >>
> >> One general comment about this is that the behavior seems a little
> >> asymmetrical for the different protocol types: why switch to this new
> >> style for IP but not for ARP?  Why are the L4 protocols different from
> >> L3?  Why can't we just do all of this at the beginning?  The answer to
> >> all of these questions is (I assume) "IP options".  However, it took a
> >> little more thought to convince myself that it is OK than seems
> >> necessary.
> >
> > We could do it all at the beginning, if we are willing to pull 98 bytes
> > of headers:
> >
> >        14      Ethernet
> >        4       VLAN
> >        60      max IP header
> >        20      TCP/UDP/ICMP header
> >        --
> >        98
> >
> > The 64 bytes we currently pull covers any of the protocols we support as
> > long as there are no IP options.  I was reluctant to increase the amount
> > we pull.  But maybe it's not a big deal.  What do you think?
> 
> I don't think it is a good idea to increase the amount that we pull.
> Xen puts the first 64 or 72 bytes (depending on the version) of the
> packet in the linear data area and the rest is paged.  If we increase
> the pull amount we'll end up reallocating and copying for every packet
> larger than the value that Xen chooses.
> 
> I think that this just needs to be commented better.

I added this comment:

	/*
	 * We would really like to pull as many bytes as we could possibly
	 * want to parse into the linear data area.  Currently that is:
	 *
	 *    14     Ethernet header
	 *     4     VLAN header
	 *    60     max IP header with options
	 *    20     max TCP/UDP/ICMP header (don't care about options)
	 *    --
	 *    98
	 *
	 * But Xen only allocates 64 or 72 bytes for the linear data area in
	 * netback, which means that we would reallocate and copy the skb's
	 * linear data on every packet if we did that.  So instead just pull 64
	 * bytes, which is always sufficient without IP options, and then check
	 * whether we need to pull more later when we look at the IP header.
	 */

Maybe too verbose...