[ovs-dev] [PATCH 5/8] datapath: Add support for tunnel fragmentation.

Jesse Gross jesse at nicira.com
Wed Aug 18 17:05:19 UTC 2010


On Wed, Aug 18, 2010 at 12:49 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Wed, Aug 18, 2010 at 12:08:49AM -0700, Jesse Gross wrote:
> > Up until now it was assumed that encapsulated packets larger than
> > the MTU would be fragmented by the IP stack.  However, some
> > tunneling protocols provide their own fragmentation mechanism.  This
> > adds the necessary support to the generic tunnel code to support
> > fragmentation.
> >
> > Signed-off-by: Jesse Gross <jesse at nicira.com>
>
> It seems totally bizarre to me that "local_df" is set to 1 to *allow*
> fragmentation.  I never would have guessed without the comment that you
> added (which is not new in this patch).
>

I agree, it's totally counterintuitive to me.


>
> The build_header member in struct tnl_ops could use a comment.
>

Sure.


>
> get_random_bytes() is pretty expensive.  Most of the network stack uses
> net_random() or random32() (same thing), can you use that instead?
>

I'm not sure that the cost is really all that big a deal here since it only
happens when we setup tunnels (we also use get_random_bytes() for generating
a MAC address at this time).  Getting initial IP IDs, TCP ISNs, etc. all
come from a more secure random pool (and as Steve Hemminger mentioned
random32() is really for statistical simulation).  I would have used
get_random_int() as the best solution but it isn't exported.


>
> It's hard to evaluate the use of the new 'frag_id' member of struct
> tnl_vport, since it's treated as write-only in this commit.  atomic_t's
> are relatively expensive on SMP, but maybe fragmentation is inherently
> expensive enough that it doesn't matter?
>

Fragmentation is already a very slow path, so if you go down it you've
already lost.  I thought about doing something better but it would blow up
data structures, which has a cost for everybody.  It's just not worth it.
 IP IDs also use atomic_t's, so we're in good company.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20100818/456ca6f3/attachment-0003.html>


More information about the dev mailing list