[ovs-dev] [runt-flows 5/8] datapath: Report memory allocation errors in flow_extract().
Jesse Gross
jesse at nicira.com
Fri Aug 27 19:36:59 UTC 2010
On Fri, Aug 27, 2010 at 12:31 PM, Ben Pfaff <blp at nicira.com> wrote:
> 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...
Thanks.
More information about the dev
mailing list