[PATCH] IB/ipoib: Change To max_cm_mtu when changing mode to connected

[PATCH] IB/ipoib: Change To max_cm_mtu when changing mode to connected

[Erez Shitrit]

When switching between modes (datagram / connected) change the MTU
accordingly.
datagram mode up to 4K, connected mode up to (64K - 0x10).

Signed-off-by: ELi Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 1 +
 1 file changed, 1 insertion(+)

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;
 
-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/ipoib: Change To max_cm_mtu when changing mode to connected

[Jason Gunthorpe]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/ipoib: Change To max_cm_mtu when changing mode to connected

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 941 bytes --]

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.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] IB/ipoib: Change To max_cm_mtu when changing mode to connected

[Jason Gunthorpe]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/ipoib: Change To max_cm_mtu when changing mode to connected

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]

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.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] IB/ipoib: Change To max_cm_mtu when changing mode to connected

[Erez Shitrit]

On Tue, Jun 9, 2015 at 12:04 AM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>               GPG KeyID: 0E572FDD
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/ipoib: Change To max_cm_mtu when changing mode to connected

[Erez Shitrit]

On Tue, Jun 9, 2015 at 12:04 AM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>               GPG KeyID: 0E572FDD
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] IB/ipoib: Change To max_cm_mtu when changing mode to connected

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1650 bytes --]

On Tue, 2015-06-09 at 14:17 +0300, Erez Shitrit wrote:
> On Tue, Jun 9, 2015 at 12:04 AM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 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.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] IB/ipoib: Change To max_cm_mtu when changing mode to connected

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 2258 bytes --]

On Tue, 2015-06-09 at 09:06 +0300, Erez Shitrit wrote:
> On Tue, Jun 9, 2015 at 12:04 AM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >               GPG KeyID: 0E572FDD
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]