Date:   Fri, 3 Dec 2021 16:57:47 -0800
From:   Jakub Kicinski <kuba@kernel.org>
To:     =?UTF-8?B?QmrDuHJu?= Mork <bjorn@mork.no>,
        Lee Jones <lee.jones@linaro.org>
Cc:     linux-kernel@vger.kernel.org, stable@vger.kernel.org,
        Oliver Neukum <oliver@neukum.org>,
        "David S. Miller" <davem@davemloft.net>, linux-usb@vger.kernel.org,
        netdev@vger.kernel.org
Subject: Re: [PATCH 1/1] net: cdc_ncm: Allow for dwNtbOutMaxSize to be unset
 or zero
Message-ID: <20211203165747.7e1e7554@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
In-Reply-To: <87wnklivun.fsf@miraculix.mork.no>
References: <20211202143437.1411410-1-lee.jones@linaro.org>
        <87wnklivun.fsf@miraculix.mork.no>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, 03 Dec 2021 15:52:48 +0100 Bj=C3=B8rn Mork wrote:
> Lee Jones <lee.jones@linaro.org> writes:
>=20
> > diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> > index 24753a4da7e60..e303b522efb50 100644
> > --- a/drivers/net/usb/cdc_ncm.c
> > +++ b/drivers/net/usb/cdc_ncm.c
> > @@ -181,6 +181,8 @@ static u32 cdc_ncm_check_tx_max(struct usbnet *dev,=
 u32 new_tx)
> >  		min =3D ctx->max_datagram_size + ctx->max_ndp_size + sizeof(struct u=
sb_cdc_ncm_nth32);
> > =20
> >  	max =3D min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx->ncm_parm=
.dwNtbOutMaxSize));
> > +	if (max =3D=3D 0)
> > +		max =3D CDC_NCM_NTB_MAX_SIZE_TX; /* dwNtbOutMaxSize not set */
> > =20
> >  	/* some devices set dwNtbOutMaxSize too low for the above default */
> >  	min =3D min(min, max); =20
>=20
> I believe this is the best possible fix, considering the regressions
> anything stricter might cause.
>=20
> We know of at least one MBIM device where dwNtbOutMaxSize is as low as
> 2048.
>=20
> According to the MBIM spec, the minimum and default value for
> wMaxSegmentSize is also 2048.  This implies that the calculated "min"
> value is at least 2076, which is why we need that odd looking
>=20
>   min =3D min(min, max);
>=20
> So let's just fix this specific zero case without breaking the
> non-conforming devices.
>=20
>=20
> Reviewed-by: Bj=C3=B8rn Mork <bjorn@mork.no>

Applied to net, thanks!