| Message ID | d62f5a2b-ec47-6cee-4cf1-0d1ea18dee56@gateware.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: u_ether: Fix host MAC address case | expand |
Hi Konrad, On 4/26/23 12:17, Konrad Gräfe wrote: > > As the CDC-ECM specification states the host MAC address must be sent to > the host as an uppercase hexadecimal string: > The Unicode character is chosen from the set of values 30h through > 39h and 41h through 46h (0-9 and A-F). > > However, snprintf(.., "%pm", ..) generates a lowercase MAC address > string. While most host drivers are tolerant to this, UsbNcm.sys on > Windows 10 is not. Instead it uses a different MAC address with all > bytes set to zero including and after the first byte containing a > lowercase letter. On Windows 11 Microsoft fixed it, but apparently they > did not backport the fix. > > This change fixes the issue by upper-casing the MAC to comply with the > specification. > > Signed-off-by: Konrad Gräfe <k.graefe@gateware.de> > --- > V1 -> V2: Fixed checkpatch.pl warnings > When sending a new version, please use the -v option of git format-patch to add the v2 prefix in the [PATCH] header, see https://lore.kernel.org/lkml/20230417065005.24967-1-yu.tu@amlogic.com/ (here a v7). c.f. https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format Additionally, please use scripts/get_maintainer.pl on your patch to know who to put in the recipient list, here you're missing linux-kernel@vger.kernel.org which is always added. If I'm not mistaken, it's basically used for archival purposes but also so people don't have to subscribe or search through all mailing list to know what's going on globally in the kernel community. > drivers/usb/gadget/function/u_ether.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > The patch should be the mail body and not attached, git send-email should be able to get you through the hoops. I've seen https://git-send-email.io/ mentioned in various places so maybe that could help you get started. See https://lore.kernel.org/lkml/20230417065005.24967-2-yu.tu@amlogic.com/ and yours: https://lore.kernel.org/all/f147dabd-5c39-31c2-0ff0-f72745d7cd3f@gateware.de/ In yours, you can see """ [-- Attachment #1.1.2: 0001-usb-gadget-u_ether-Fix-host-MAC-address-case.patch --] [-- Type: text/x-patch, Size: 812 bytes --] """ this is not good. The point of the mailing list patch submission process is to allow in-line review, people will typically comment in-line directly, see https://lore.kernel.org/lkml/20230426111358.xh3gbhlvxj46ggi5@CAB-WSD-L081021/ for example. Adding the patch as an attachment doesn't allow this review process to happen. There also are a few robots applying patches directly from the various mailing lists and running some build tests automatically, I am not sure they are developed with patch as an attachment in mind. c.f. https://www.kernel.org/doc/html/latest/process/submitting-patches.html#no-mime-no-links-no-compression-no-attachments-just-plain-text For the "i'm not so sure" part of my mail now. I don't know if this matches stable backport policy but I believe it could/should. So maybe add: """ Cc: stable@vger.kernel.org """ before your Signed-off-by, c.f. https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html I think you should add: """ Fixes: bcd4a1c40bee ("usb: gadget: u_ether: construct with default values and add setters/getters") """ since (I believe) this is fixing an issue introduced in that commit. c.f. https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes and https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes Cheers, Quentin
On Wed, Apr 26, 2023 at 12:17:53PM +0200, Konrad Gräfe wrote: > > As the CDC-ECM specification states the host MAC address must be sent to > the host as an uppercase hexadecimal string: > The Unicode character is chosen from the set of values 30h through > 39h and 41h through 46h (0-9 and A-F). > > However, snprintf(.., "%pm", ..) generates a lowercase MAC address > string. While most host drivers are tolerant to this, UsbNcm.sys on > Windows 10 is not. Instead it uses a different MAC address with all > bytes set to zero including and after the first byte containing a > lowercase letter. On Windows 11 Microsoft fixed it, but apparently they > did not backport the fix. > > This change fixes the issue by upper-casing the MAC to comply with the > specification. > > Signed-off-by: Konrad Gräfe <k.graefe@gateware.de> > --- > V1 -> V2: Fixed checkpatch.pl warnings There is no "v2" in the subject line, so our tools will get confused and have no idea this is a newer patch. Please fix up and send a v3? > snprintf(host_addr, len, "%pm", dev->host_mac); Is there no option to print a mac address with all uppercase? If not, why not add that instead as it's needed here and maybe other places, right? thanks, greg k-h
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index 6956ad8ba8dd..250734e090fc 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -958,6 +958,7 @@ EXPORT_SYMBOL_GPL(gether_get_host_addr); int gether_get_host_addr_cdc(struct net_device *net, char *host_addr, int len) { struct eth_dev *dev; + int i, slen; if (len < 13) return -EINVAL; @@ -965,7 +966,13 @@ int gether_get_host_addr_cdc(struct net_device *net, char *host_addr, int len) dev = netdev_priv(net); snprintf(host_addr, len, "%pm", dev->host_mac); - return strlen(host_addr); + + slen = strlen(host_addr); + + for (i = 0; i < slen; i++) + host_addr[i] = toupper(host_addr[i]); + + return slen; } EXPORT_SYMBOL_GPL(gether_get_host_addr_cdc);
As the CDC-ECM specification states the host MAC address must be sent to the host as an uppercase hexadecimal string: The Unicode character is chosen from the set of values 30h through 39h and 41h through 46h (0-9 and A-F). However, snprintf(.., "%pm", ..) generates a lowercase MAC address string. While most host drivers are tolerant to this, UsbNcm.sys on Windows 10 is not. Instead it uses a different MAC address with all bytes set to zero including and after the first byte containing a lowercase letter. On Windows 11 Microsoft fixed it, but apparently they did not backport the fix. This change fixes the issue by upper-casing the MAC to comply with the specification. Signed-off-by: Konrad Gräfe <k.graefe@gateware.de> --- V1 -> V2: Fixed checkpatch.pl warnings drivers/usb/gadget/function/u_ether.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)