Re: [PATCH rdma-next V1 1/9] IB/ipoib: Add warning message when changing the MTU in UD over the max range

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 16, 2017 at 01:12:18PM -0700, Jason Gunthorpe wrote:
> On Sun, Jan 15, 2017 at 10:35:43AM +0200, Leon Romanovsky wrote:
> > On Fri, Jan 13, 2017 at 02:27:21PM -0700, Jason Gunthorpe wrote:
> > > On Fri, Jan 13, 2017 at 10:31:27PM +0200, Leon Romanovsky wrote:
> > >
> > > > > This is about what happens if the multicast MTU < unicast MTU - which
> > > > > is *exactly* the same case on RC and UD.
> > > > >
> > > > > IPoIB cannot send a 64k multicast MTU on RC either.
> > > >
> > > > Multicast is sent in datagram despite being in connected mode and for
> > > > datagram the MTU is limited by IB
> > >
> > > I am aware of that. Read you patch again:
> > >
> > > > +	if (priv->mcast_mtu < priv->admin_mtu)
> > > > +		ipoib_warn(priv, "MTU must be smaller than mcast_mtu (%u)\n",
> > >
> > > This check is fails in RC mode as well.
> > >
> > > And we already check if the requested MTU is beyond the port
> > > capability:
> > >
> > > +        if (new_mtu > IPOIB_UD_MTU(priv->max_ib_mtu))
> > > +                return -EINVAL;
> > >
> > > So I have *no idea* why you'd want to check it against the multicast
> > > group too..
> > >
> >
> > Port MTU capability can be higher than link MTU. In this flow, the user
> > can try to set manually MTU which will be higher than SM provided.
> >
> > Immediately after that print, we have a following piece of code:
> >  236         dev->mtu = min(priv->mcast_mtu, priv->admin_mtu);
>
> Maybe the better fix is to take that min out than generate a
> warning.

I agree with you, in theory it should work.

Let's hope that we will find such brave man who will
remove this line. The removal of this one line will
require from him to do extensive testing and reviewing
different code paths.

Thanks

>
> Jason

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux