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

[Ben Pfaff]

On Fri, Jan 12, 2018 at 04:51:00PM -0500, Eric Garver wrote:
> On Fri, Jan 12, 2018 at 04:35:06PM -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);
> > > +
> > > +        err = nl_transact(NETLINK_ROUTE, &request, NULL);
> > > +    }
> > >  
> > >  exit:
> > >      ofpbuf_uninit(&request);
> > > -- 
> > > 2.10.2
> > > 
> > 
> > This looks sane to me. It would be nice if James could test it though.
> 
> On second thought.. if SETLINK fails we'll report "err" that the tunnel
> didn't get created, but it actually did. We can either ignore the err
> from SETLINK or destroy the interface. Depends on if you prefer low MTU
> tunnel or no tunnel. :)

I guess it would be slightly better to report no error (and log the real
error), because there's no guarantee that DELLINK would succeed, and
what do we do if it doesn't?