[ovs-dev] [PATCH] gre: Fix ICMP translation for path MTU discovery.

[Jesse Gross]

On Wed, Apr 21, 2010 at 12:16 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Tue, Apr 20, 2010 at 05:22:01PM -0400, Jesse Gross wrote:
> > The translation of fragmentation-needed messages from outside the
> > tunnel to inside didn't quite make the transition from the old
> > GRE implementation to the new one intact.  This fixes a number of
> > minor bugs in the implmentation.  The primary issues are with computing
> > the tunnel header length and comparing the input vs. output values
> > for tunnel parameters such as the key.
>
> How thoroughly do you recommend that I should review this?  I looked it
> over fairly quickly and didn't see anything that looked bad, but I could
> spend some more time on it, if you think I should, and increase my
> understanding.
>
>
More than anything this is just a cleanup patch.  I tested this
functionality before I pushed the big changeset, saw that it was broken but
didn't have disastrous consequences and decided to wait.  None of the logic
here is new - it's all brought over from my path MTU implementation for
Linux GRE devices.  I believe all the PMTUD stuff works now with this patch
so hopefully there won't be too much more in this area.

Obviously you are welcome to review it if you want but if you don't see
anything obviously stupid it's probably unnecessary.


> I did notice one place where code was written like this:
>        if ((flags & A) && (flags & B))
> Usually I would write this as:
>        if ((flags & (A|B)) == (A|B))
> Maybe you did it that way because the "flags" and "A" and "B" are all
> pretty long and would cause the line to wrap in an ugly way.  I suspect
> that GCC generates the same code either way, so it probably makes sense
> to leave it alone, but I did think it was at least worth pointing out.
>

Yeah, it's really hard to read if you write it the other way.  None of this
stuff is performance critical anyways.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20100421/04c46790/attachment-0003.html>