Re: cdc_ncm: Specific Huawei E3372h firmware version stuck in NTB-32

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

 



Yeah, I think definitely this can be a good Idea.
I tried to write some code - it may well crash your kernel, I am sending it in now purely for review and discussion purposes.


On Fri, 7 Jul 2017, Bjørn Mork wrote:

Date: Fri, 7 Jul 2017 10:55:07
From: Bjørn Mork <bjorn@xxxxxxx>
To: Christian Panton <christian@xxxxxxxxxx>
Cc: linux-usb@xxxxxxxxxxxxxxx, Enrico Mioso <mrkiko.rs@xxxxxxxxx>
Subject: Re: cdc_ncm: Specific Huawei E3372h firmware version stuck in NTB-32

[CCing Enrico]

Christian Panton <christian@xxxxxxxxxx> writes:

Found a solution.

I have two mobile broadband Huawei E3372h devices, one with firmware
21.200.07.01.26 (aka the 200-version) and one with firmware
21.318.01.00.541 (aka the 318-version).

Whereas the 200-version works perfectly with a recent kernel (4.10),
the latter never manages to exchange any IP-packets. Upon debugging
USB-traces, I found that the 200-version correctly switches to NTB-16,
whereas the 318-version stays in NTB-32 mode.

And both these firmwares use the cdc_ncm class driver directly, and not
the huawei_cdc_ncm driver?  I.e. they appear as true CDC NCM class
devices

They appear to use the huawei_cdc_ncm driver. I am not at all familiar with driver nor kernel architecture, so I am sorry if I am being a bit vague.

OK, then all bets regarding specs are off since this is a vendor
specific function. They can do whatever they want with it.

Could you test if

echo Y  >/sys/class/net/xxx/cdc_ncm/ndp_to_end

makes any difference, where xxx is the name of your wwan netdev?

It did not make any difference. See root cause below.

No, that is expected in this case.  The huawei_cdc_ncm driver already
enforce this.


Section 7.2 "Using Alternate Settings to Reset an NCM Function":

"Whenever alternate setting 0 is selected by the host, the function
 shall:
 ..
 - reset the NTB format to NTB-16
“

I downloaded the spec, and started to investigate. Comparing Windows driver traces to the spec, I noticed that the Windows driver queried GET_NTB_FORMAT, prior to setting it to NTB-32. It was always 0x0001h (aka. NTB-32), directly after coming out of alternate setting = 0.

So I threw together a small python-usb script, to see what was different between the two devices I had.

My testing sequence is:

	alt_setting = 1
	alt_setting = 0
	GET_NTB_PARAMETERS
	GET_NTB_FORMAT (#A)
	SET_CRC_MODE
	SET_NTB_FORMAT
	GET_NTB_FORMAT (#B)
	alt_setting = 1
	GET_NTB_FORMAT (#C)
	SET_NTB_FORMAT
	GET_NTB_FORMAT (#D)

BOTH devices come out of alt_setting = 0 (after step #A) with GET_NTB_FORMAT = 0x0001h!

Readout of GET_NTB_FORMAT:

#A 	200: 0x0001		318: 0x0001 <— bug in both
#B	200: 0x0000		318: 0x0000
#C 	200: 0x0000		318: 0x0001 <— bug in 318
#D 	200: 0x0000		318: 0x0000

So it is clear, that the 318-firmware resets to NTB-32 after alt_setting = 1

It is clearly non-standard behaviour.

The spec concerning SET_NTB_FORMAT reads:
 "The host shall only send this  command while the NCM Data Interface is in alternate setting 0.”

Yes, and violating this is known to cause issues with other devices.

So I blindly added:

usbnet_write_cmd(dev, USB_CDC_SET_NTB_FORMAT,
				       USB_TYPE_CLASS | USB_DIR_OUT
				       | USB_RECIP_INTERFACE,
				       USB_CDC_NCM_NTB16_FORMAT,
				       iface_no, NULL, 0);

 in cdc_ncm_bind_common after the interface was switched back with alt_setting = 1. Now the device works.

I have zero experience whether this should or could be implemented in the driver, nor do I have any kernel development experience.

A solution in pseudocode:

If GET_NTB_FORMAT == 0x0001 { // no restraints on alt setting according to spec
	// device is still in NTB-32 mode, which clearly indicates
	// a firmware that is not in the accordance with the spec
	Run SET_NTB_FORMAT(0x0000)
}


Maybe we can do that?  GET_NTB_FORMAT is allowed in altsetting 1 if the
device supports multiple formats.  And as it should always return
USB_CDC_NCM_NTB16_FORMAT (0) here, the SET_NTB_FORMAT will never be
sent unless the device is buggy.  So we do not violate the spec.

What do you think, Enrico?



Bjørn


diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d103a1d4fb36..5c46a19bb560 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -768,8 +768,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	u8 *buf;
 	int len;
 	int temp;
+	int err;
 	u8 iface_no;
 	struct usb_cdc_parsed_header hdr;
+	u8 curr_ndp_format;

 	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
@@ -856,6 +858,24 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 		goto error2;
 	}

+	// ??????????
+	err = usbnet_read_cmd(dev, USB_CDC_GET_NTB_FORMAT,
+			      USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE,
+			      0, iface_no, &curr_ndp_format, sizeof(u8));
+	if (err)
+		goto error2;
+
+	if (curr_ndp_format == USB_CDC_NCM_NTB32_FORMAT) {
+		err = usbnet_write_cmd(dev, USB_CDC_SET_NTB_FORMAT,
+				       USB_TYPE_CLASS | USB_DIR_OUT
+				       | USB_RECIP_INTERFACE,
+				       USB_CDC_NCM_NTB16_FORMAT,
+				       iface_no, NULL, 0);
+
+		if (err)
+			goto error2;
+	}
+
 	/* initialize basic device settings */
 	if (cdc_ncm_init(dev))
 		goto error2;

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux