| Message ID | 1433673371-5355-1-git-send-email-erezsh@mellanox.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Sun, Jun 07, 2015 at 01:36:11PM +0300, Erez Shitrit wrote: > > When switching between modes (datagram / connected) change the MTU > accordingly. > datagram mode up to 4K, connected mode up to (64K - 0x10). Is this a bug fix (describe the user visible impact)? Should it go to -stable? Add a Fixes: line? Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2015-06-08 at 10:42 -0600, Jason Gunthorpe wrote: > On Sun, Jun 07, 2015 at 01:36:11PM +0300, Erez Shitrit wrote: > > > > When switching between modes (datagram / connected) change the MTU > > accordingly. > > datagram mode up to 4K, connected mode up to (64K - 0x10). > > Is this a bug fix (describe the user visible impact)? Should it go to > -stable? Add a Fixes: line? I'm not sure I would call it a bug. Setting the MTU to max automatically is a policy decision more than anything else. Currently, you have to enable connected mode, then set the MTU. Both initscripts and NetworkManager do this for you on Red Hat, so the user sees no difference here. I can't speak to other OSes. I'm OK with setting connected mode MTU to max by default once we get the scatter/gather support for IPoIB added.
On Mon, Jun 08, 2015 at 05:04:42PM -0400, Doug Ledford wrote: > On Mon, 2015-06-08 at 10:42 -0600, Jason Gunthorpe wrote: > > On Sun, Jun 07, 2015 at 01:36:11PM +0300, Erez Shitrit wrote: > > > > > > When switching between modes (datagram / connected) change the MTU > > > accordingly. > > > datagram mode up to 4K, connected mode up to (64K - 0x10). > > > > Is this a bug fix (describe the user visible impact)? Should it go to > > -stable? Add a Fixes: line? > > I'm not sure I would call it a bug. Setting the MTU to max > automatically is a policy decision more than anything else. Currently, > you have to enable connected mode, then set the MTU. Both initscripts > and NetworkManager do this for you on Red Hat, so the user sees no > difference here. I can't speak to other OSes. I'm OK with setting > connected mode MTU to max by default once we get the scatter/gather > support for IPoIB added. I was thinking about the other direction, what happens when you turn connected mode off? Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2015-06-08 at 15:07 -0600, Jason Gunthorpe wrote: > On Mon, Jun 08, 2015 at 05:04:42PM -0400, Doug Ledford wrote: > > On Mon, 2015-06-08 at 10:42 -0600, Jason Gunthorpe wrote: > > > On Sun, Jun 07, 2015 at 01:36:11PM +0300, Erez Shitrit wrote: > > > > > > > > When switching between modes (datagram / connected) change the MTU > > > > accordingly. > > > > datagram mode up to 4K, connected mode up to (64K - 0x10). > > > > > > Is this a bug fix (describe the user visible impact)? Should it go to > > > -stable? Add a Fixes: line? > > > > I'm not sure I would call it a bug. Setting the MTU to max > > automatically is a policy decision more than anything else. Currently, > > you have to enable connected mode, then set the MTU. Both initscripts > > and NetworkManager do this for you on Red Hat, so the user sees no > > difference here. I can't speak to other OSes. I'm OK with setting > > connected mode MTU to max by default once we get the scatter/gather > > support for IPoIB added. > > I was thinking about the other direction, what happens when you turn > connected mode off? It drops back down to IB MTU - 4.
On Tue, Jun 9, 2015 at 12:04 AM, Doug Ledford <dledford@redhat.com> wrote: > On Mon, 2015-06-08 at 10:42 -0600, Jason Gunthorpe wrote: >> On Sun, Jun 07, 2015 at 01:36:11PM +0300, Erez Shitrit wrote: >> > >> > When switching between modes (datagram / connected) change the MTU >> > accordingly. >> > datagram mode up to 4K, connected mode up to (64K - 0x10). >> >> Is this a bug fix (describe the user visible impact)? Should it go to >> -stable? Add a Fixes: line? > > I'm not sure I would call it a bug. Setting the MTU to max > automatically is a policy decision more than anything else. Currently, > you have to enable connected mode, then set the MTU. Both initscripts > and NetworkManager do this for you on Red Hat, so the user sees no > difference here. I can't speak to other OSes. I'm OK with setting The only real reason for a user to switch to CM mode is for the performance he can get, and this is only by the huge MTU the CM gives, so it somehow should be part of the "mode setting". As you said RH does it (in the new versions of its OS's only) via scripting outside of the driver code, other vendor doesn't. > connected mode MTU to max by default once we get the scatter/gather > support for IPoIB added. OK, great. Thanks. > -- > Doug Ledford <dledford@redhat.com> > GPG KeyID: 0E572FDD > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 9, 2015 at 12:04 AM, Doug Ledford <dledford@redhat.com> wrote: > On Mon, 2015-06-08 at 10:42 -0600, Jason Gunthorpe wrote: >> On Sun, Jun 07, 2015 at 01:36:11PM +0300, Erez Shitrit wrote: >> > >> > When switching between modes (datagram / connected) change the MTU >> > accordingly. >> > datagram mode up to 4K, connected mode up to (64K - 0x10). >> >> Is this a bug fix (describe the user visible impact)? Should it go to >> -stable? Add a Fixes: line? > > I'm not sure I would call it a bug. Setting the MTU to max > automatically is a policy decision more than anything else. Currently, > you have to enable connected mode, then set the MTU. Both initscripts > and NetworkManager do this for you on Red Hat, so the user sees no > difference here. I can't speak to other OSes. I'm OK with setting > connected mode MTU to max by default once we get the scatter/gather > support for IPoIB added. Hi Doug, There is no connection to the scatter/gather patch in that fix. > > -- > Doug Ledford <dledford@redhat.com> > GPG KeyID: 0E572FDD > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-06-09 at 14:17 +0300, Erez Shitrit wrote: > On Tue, Jun 9, 2015 at 12:04 AM, Doug Ledford <dledford@redhat.com> wrote: > > On Mon, 2015-06-08 at 10:42 -0600, Jason Gunthorpe wrote: > >> On Sun, Jun 07, 2015 at 01:36:11PM +0300, Erez Shitrit wrote: > >> > > >> > When switching between modes (datagram / connected) change the MTU > >> > accordingly. > >> > datagram mode up to 4K, connected mode up to (64K - 0x10). > >> > >> Is this a bug fix (describe the user visible impact)? Should it go to > >> -stable? Add a Fixes: line? > > > > I'm not sure I would call it a bug. Setting the MTU to max > > automatically is a policy decision more than anything else. Currently, > > you have to enable connected mode, then set the MTU. Both initscripts > > and NetworkManager do this for you on Red Hat, so the user sees no > > difference here. I can't speak to other OSes. I'm OK with setting > > connected mode MTU to max by default once we get the scatter/gather > > support for IPoIB added. > > Hi Doug, > There is no connection to the scatter/gather patch in that fix. Yes, I'm well aware of that. However, 64k MTU in connected mode does order 5 page allocations. That has been the source of a number of bugs in our bug tracking system in the past. I wouldn't make it default to turning on 64k MTU without user intervention for that reason. Once the SG patch for IPoIB is included, that problem goes away and I'm OK making IPoIB turn on the large MTU by default.
On Tue, 2015-06-09 at 09:06 +0300, Erez Shitrit wrote: > On Tue, Jun 9, 2015 at 12:04 AM, Doug Ledford <dledford@redhat.com> wrote: > > On Mon, 2015-06-08 at 10:42 -0600, Jason Gunthorpe wrote: > >> On Sun, Jun 07, 2015 at 01:36:11PM +0300, Erez Shitrit wrote: > >> > > >> > When switching between modes (datagram / connected) change the MTU > >> > accordingly. > >> > datagram mode up to 4K, connected mode up to (64K - 0x10). > >> > >> Is this a bug fix (describe the user visible impact)? Should it go to > >> -stable? Add a Fixes: line? > > > > I'm not sure I would call it a bug. Setting the MTU to max > > automatically is a policy decision more than anything else. Currently, > > you have to enable connected mode, then set the MTU. Both initscripts > > and NetworkManager do this for you on Red Hat, so the user sees no > > difference here. I can't speak to other OSes. I'm OK with setting > > The only real reason for a user to switch to CM mode is for the > performance he can get, and this is only by the huge MTU the CM gives, > so it somehow should be part of the "mode setting". > As you said RH does it (in the new versions of its OS's only) No, we do it in all versions of our OS, but you have to set the MTU parameter, we don't default it to anything for you (but this is no different than setting CM). This is normal device setup. Just like we don't default Ethernet interfaces to jumbo frames, the user needs to set the MTU on that interface. And the user gets benefit at MTUs less than jumbo size. > via > scripting outside of the driver code, other vendor doesn't. > > > connected mode MTU to max by default once we get the scatter/gather > > support for IPoIB added. > > OK, great. Thanks. > > > -- > > Doug Ledford <dledford@redhat.com> > > GPG KeyID: 0E572FDD > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c index 9e1b203..5b74184 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c @@ -232,6 +232,7 @@ int ipoib_set_mode(struct net_device *dev, const char *buf) ipoib_warn(priv, "enabling connected mode " "will cause multicast packet drops\n"); netdev_update_features(dev); + dev_set_mtu(dev, ipoib_cm_max_mtu(dev)); rtnl_unlock(); priv->tx_wr.send_flags &= ~IB_SEND_IP_CSUM;