[ovs-dev] [PATCH] dpif-netlink-rtnl: Work around MTU bug in kernel GRE driver.

[Ben Pfaff]

On Thu, Jan 18, 2018 at 08:19:06AM +0000, James Page wrote:
> Hi Ben
> 
> On Wed, 17 Jan 2018 at 20:05 Ben Pfaff <blp at ovn.org> wrote:
> 
> > On Tue, Jan 16, 2018 at 02:08:45PM -0500, Eric Garver wrote:
> > > On Tue, Jan 16, 2018 at 09:16:47AM -0800, Ben Pfaff wrote:
> > > > On Tue, Jan 16, 2018 at 10:42:16AM -0500, Eric Garver wrote:
> > > > > On Fri, Jan 12, 2018 at 12:44:59PM -0800, Ben Pfaff wrote:
> > > > > > The kernel GRE driver ignores IFLA_MTU in RTM_NEWLINK requests and
> > > > > > overrides the MTU to 1472 bytes.  This commit works around the
> > problem by
> > > > > > following up a request to create a GRE device with a second
> > request to set
> > > > > > the MTU.
> > > > > >
> > > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1488484
> > > > > > Reported-by: Eric Garver <e at erig.me>
> > > > > > Reported-by: James Page <james.page at ubuntu.com>
> > > > > > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > > > > > ---
> > > > > > This is not properly tested.  It needs to be tested before it
> > makes sense
> > > > > > to commit it.
> > > > > >
> > > > > >  lib/dpif-netlink-rtnl.c | 13 +++++++++++++
> > > > > >  1 file changed, 13 insertions(+)
> > > > > >
> > > > > > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> > > > > > index fe9c8ed7104f..0bf965d29f41 100644
> > > > > > --- a/lib/dpif-netlink-rtnl.c
> > > > > > +++ b/lib/dpif-netlink-rtnl.c
> > > > > > @@ -329,6 +329,19 @@ dpif_netlink_rtnl_create(const struct
> > netdev_tunnel_config *tnl_cfg,
> > > > > >      nl_msg_end_nested(&request, linkinfo_off);
> > > > > >
> > > > > >      err = nl_transact(NETLINK_ROUTE, &request, NULL);
> > > > > > +    if (!err && type == OVS_VPORT_TYPE_GRE) {
> > > > > > +        /* Work around a bug in kernel GRE driver, which ignores
> > IFLA_MTU in
> > > > > > +         * RTM_NEWLINK, by setting the MTU again.  See
> > > > > > +         * https://bugzilla.redhat.com/show_bug.cgi?id=1488484.
> > */
> > > > > > +        ofpbuf_clear(&request);
> > > > > > +        nl_msg_put_nlmsghdr(&request, 0, RTM_SETLINK,
> > > > > > +                            NLM_F_REQUEST | NLM_F_ACK);
> > > > > > +        ofpbuf_put_zeros(&request, sizeof(struct ifinfomsg));
> > > > > > +        nl_msg_put_string(&request, IFLA_IFNAME, name);
> > > > > > +        nl_msg_put_u32(&request, IFLA_MTU, UINT16_MAX);
> > > > >
> > > > > James' testing found that this value will be rejected by the kernel.
> > GRE
> > > > > driver uses ip_tunnel_change_mtu() with the "strict" parameter, so it
> > > > > will return an error instead of clamping the value. GENEVE/VXLAN
> > always
> > > > > clamp the value.
> > > > >
> > > > > I think 65478 is the max_mtu for gretap accounting for optional csum,
> > > > > key, seq in gre_calc_hlen(). It may be slightly higher for how OVS
> > > > > configures gretap.
> > > > >
> > > > >   0xFFF8 - eth(14) - ipv4(20) - gre(16) = 65478
> > > >
> > > > Do you think that it's worthwhile using the maximum possible MTU, or
> > >
> > > I don't think it's worth possibly hitting another one of these issues.
> > >
> > > > should we be a little conservative and use 65400 or 65000?  Either of
> > > > those values would be future-proof, I think.
> > >
> > > I agree 65000 would be okay, but if we're going to do it then lets do it
> > > for everything (GENEVE/VXLAN/GRE) instead of assuming the kernel will
> > > clamp the value for us on newlink/setlink.
> >
> > OK, great.
> >
> > I posted v3 that does this and makes the error handling fix we agreed
> > on:
> > https://patchwork.ozlabs.org/project/openvswitch/list/?series=23965
> 
> 
> retested with your new patches; confirmed that MTU for GRE and VXLAN ports
> are set consistently to 65000.
> 
> Cheers
> 
> James

Thanks for the testing!  I applied these patches to master and
branch-2.9.