From:   =?utf-8?Q?Bj=C3=B8rn_Mork?= <bjorn@mork.no>
To:     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>,
        Jakub Kicinski <kuba@kernel.org>, 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
Organization: m
References: <20211202143437.1411410-1-lee.jones@linaro.org>
Date:   Fri, 03 Dec 2021 15:52:48 +0100
In-Reply-To: <20211202143437.1411410-1-lee.jones@linaro.org> (Lee Jones's
        message of "Thu, 2 Dec 2021 14:34:37 +0000")
Message-ID: <87wnklivun.fsf@miraculix.mork.no>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Lee Jones <lee.jones@linaro.org> writes:

> 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, u=
32 new_tx)
>  		min =3D ctx->max_datagram_size + ctx->max_ndp_size + sizeof(struct usb=
_cdc_ncm_nth32);
>=20=20
>  	max =3D min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx->ncm_parm.d=
wNtbOutMaxSize));
> +	if (max =3D=3D 0)
> +		max =3D CDC_NCM_NTB_MAX_SIZE_TX; /* dwNtbOutMaxSize not set */
>=20=20
>  	/* some devices set dwNtbOutMaxSize too low for the above default */
>  	min =3D min(min, max);

I believe this is the best possible fix, considering the regressions
anything stricter might cause.

We know of at least one MBIM device where dwNtbOutMaxSize is as low as
2048.

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

  min =3D min(min, max);

So let's just fix this specific zero case without breaking the
non-conforming devices.


Reviewed-by: Bj=C3=B8rn Mork <bjorn@mork.no>