| Message ID | 1530816946-21249-1-git-send-email-pkusunyifeng@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] dpif-netlink-rtnl: Retry a smaller MTU for netdev when MAX_MTU is too large. | expand |
On Thu, Jul 05, 2018 at 11:55:46AM -0700, Yifeng Sun wrote: > When MAX_MTU is larger than hw supported max MTU, > dpif_netlink_rtnl_create will fail. This leads to > testing failure '11: datapath - ping over gre tunnel' > in 'make check-kmod'. > > This patch fixes this issue by retrying a smaller MTU > when MAX_MTU is too large. > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Is this GRE-specific? (Why would only GRE care about large MTUs?)
Hi Ben, Yes, this is GRE-specific, somehow gre_sys doesn't support current MTU which is 65000. `make check-kmod` no. 11 and 21 fail due to this issue. On the kernel side, the error shows up as: [ 6135.119999] gre_sys: Invalid MTU 65000 requested, hw max 1500 It reflects to userspace as error below: dpif_netlink_rtnl|WARN|setting MTU of tunnel gre_sys failed (Invalid argument) Thanks, Yifeng On Fri, Jul 6, 2018 at 10:58 AM, Ben Pfaff <blp@ovn.org> wrote: > On Thu, Jul 05, 2018 at 11:55:46AM -0700, Yifeng Sun wrote: > > When MAX_MTU is larger than hw supported max MTU, > > dpif_netlink_rtnl_create will fail. This leads to > > testing failure '11: datapath - ping over gre tunnel' > > in 'make check-kmod'. > > > > This patch fixes this issue by retrying a smaller MTU > > when MAX_MTU is too large. > > > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > > Is this GRE-specific? (Why would only GRE care about large MTUs?) >
On Thu, Jul 05, 2018 at 11:55:46AM -0700, Yifeng Sun wrote: > When MAX_MTU is larger than hw supported max MTU, > dpif_netlink_rtnl_create will fail. This leads to > testing failure '11: datapath - ping over gre tunnel' > in 'make check-kmod'. > > This patch fixes this issue by retrying a smaller MTU > when MAX_MTU is too large. > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> ... > - int err2 = nl_transact(NETLINK_ROUTE, &request, NULL); > + * https://bugzilla.redhat.com/show_bug.cgi?id=1488484. > + * > + * In case of MAX_MTU exceeds hw max MTU, retry a smaller value. */ > + int err2 = rtnl_set_mtu(name, MAX_MTU, &request) && > + rtnl_set_mtu(name, 1450, &request); Probably, this should be more like: int err2 = rtnl_set_mtu(name, MAX_MTU, &request); if (err2) { err2 = rtnl_set_mtu(name, 1450, &request); } because if you use && then the result is 0 or 1, not 0 or an errno value. Thanks, Ben.
Oh yes, I missed that when I tried to make code look compact. Thanks for pointing it out. Yifeng On Fri, Jul 6, 2018 at 1:17 PM, Ben Pfaff <blp@ovn.org> wrote: > On Thu, Jul 05, 2018 at 11:55:46AM -0700, Yifeng Sun wrote: > > When MAX_MTU is larger than hw supported max MTU, > > dpif_netlink_rtnl_create will fail. This leads to > > testing failure '11: datapath - ping over gre tunnel' > > in 'make check-kmod'. > > > > This patch fixes this issue by retrying a smaller MTU > > when MAX_MTU is too large. > > > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > > ... > > > - int err2 = nl_transact(NETLINK_ROUTE, &request, NULL); > > + * https://bugzilla.redhat.com/show_bug.cgi?id=1488484. > > + * > > + * In case of MAX_MTU exceeds hw max MTU, retry a smaller > value. */ > > + int err2 = rtnl_set_mtu(name, MAX_MTU, &request) && > > + rtnl_set_mtu(name, 1450, &request); > > Probably, this should be more like: > > int err2 = rtnl_set_mtu(name, MAX_MTU, &request); > if (err2) { > err2 = rtnl_set_mtu(name, 1450, &request); > } > > because if you use && then the result is 0 or 1, not 0 or an errno > value. > > Thanks, > > Ben. >
On 7/5/2018 11:55 AM, Yifeng Sun wrote: > When MAX_MTU is larger than hw supported max MTU, > dpif_netlink_rtnl_create will fail. This leads to > testing failure '11: datapath - ping over gre tunnel' > in 'make check-kmod'. > > This patch fixes this issue by retrying a smaller MTU > when MAX_MTU is too large. > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > --- > lib/dpif-netlink-rtnl.c | 27 ++++++++++++++++++--------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c > index bec3fce91e38..8304e7fb3826 100644 > --- a/lib/dpif-netlink-rtnl.c > +++ b/lib/dpif-netlink-rtnl.c > @@ -282,6 +282,19 @@ dpif_netlink_rtnl_verify(const struct netdev_tunnel_config *tnl_cfg, > } > > static int > +rtnl_set_mtu(const char *name, uint32_t mtu, struct ofpbuf *request) > +{ > + 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, mtu); > + > + return nl_transact(NETLINK_ROUTE, request, NULL); > +} > + > +static int > dpif_netlink_rtnl_create(const struct netdev_tunnel_config *tnl_cfg, > const char *name, enum ovs_vport_type type, > const char *kind, uint32_t flags) > @@ -354,15 +367,11 @@ dpif_netlink_rtnl_create(const struct netdev_tunnel_config *tnl_cfg, > type == OVS_VPORT_TYPE_IP6GRE)) { > /* 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, MAX_MTU); > - > - int err2 = nl_transact(NETLINK_ROUTE, &request, NULL); > + * https://bugzilla.redhat.com/show_bug.cgi?id=1488484. > + * > + * In case of MAX_MTU exceeds hw max MTU, retry a smaller value. */ > + int err2 = rtnl_set_mtu(name, MAX_MTU, &request) && > + rtnl_set_mtu(name, 1450, &request); > if (err2) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > This has been bugging me forever and I just never get around to fixing it. Thanks!!! - Greg
diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c index bec3fce91e38..8304e7fb3826 100644 --- a/lib/dpif-netlink-rtnl.c +++ b/lib/dpif-netlink-rtnl.c @@ -282,6 +282,19 @@ dpif_netlink_rtnl_verify(const struct netdev_tunnel_config *tnl_cfg, } static int +rtnl_set_mtu(const char *name, uint32_t mtu, struct ofpbuf *request) +{ + 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, mtu); + + return nl_transact(NETLINK_ROUTE, request, NULL); +} + +static int dpif_netlink_rtnl_create(const struct netdev_tunnel_config *tnl_cfg, const char *name, enum ovs_vport_type type, const char *kind, uint32_t flags) @@ -354,15 +367,11 @@ dpif_netlink_rtnl_create(const struct netdev_tunnel_config *tnl_cfg, type == OVS_VPORT_TYPE_IP6GRE)) { /* 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, MAX_MTU); - - int err2 = nl_transact(NETLINK_ROUTE, &request, NULL); + * https://bugzilla.redhat.com/show_bug.cgi?id=1488484. + * + * In case of MAX_MTU exceeds hw max MTU, retry a smaller value. */ + int err2 = rtnl_set_mtu(name, MAX_MTU, &request) && + rtnl_set_mtu(name, 1450, &request); if (err2) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
When MAX_MTU is larger than hw supported max MTU, dpif_netlink_rtnl_create will fail. This leads to testing failure '11: datapath - ping over gre tunnel' in 'make check-kmod'. This patch fixes this issue by retrying a smaller MTU when MAX_MTU is too large. Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> --- lib/dpif-netlink-rtnl.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)