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

[Eric Garver]

On Fri, Jan 12, 2018 at 02:04:11PM -0800, Ben Pfaff wrote:
> 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?

Right. I'm good with ignoring the SETLINK error.