Re: [PATCH] huawei: add basic support for ME909u-521
Hi, On Fri, Sep 16, 2016 at 09:59:14AM +0200, Aleksander Morgado wrote: > Hey, > > Code-wise review of the patch only here; see comments below. > > On Thu, Sep 15, 2016 at 6:57 PM, Stefan Eichenberger > wrote: > > > > The ME909u-512 does theoretically support the DHCP feature over AT but if > > this is done right after the NDISDUP command, it seems that the firmware > > has some trouble with it because the firmware will not attach the > > Ethernet header afterwards. This can be seen by doing a tcpdump while > > pinging e.g. 8.8.8.8. The request has a size of 98 Bytes, while the > > replies only have a size of 84 Bytes. The first 14 Bytes are missing > > which is exactly the size of the Ethernet header. > > > > To reproduce this issue, I've tried the command sequence manually: > > mmcli -m 0 --command 'AT^NDISDUP=1,1,"gprs.swisscom.ch"' > > mmcli -m 0 --command "AT+CREG?" > > mmcli -m 0 --command "AT^NDISSTATQRY?" > > mmcli -m 0 --command "AT+CGREG?" > > mmcli -m 0 --command "AT^DHCP?" > > > > The error appears in with this sequence > > > > mmcli -m 0 --command 'AT^NDISDUP=1,1,"gprs.swisscom.ch"' > > mmcli -m 0 --command "AT+CREG?" > > mmcli -m 0 --command "AT^NDISSTATQRY?" > > mmcli -m 0 --command "AT+CGREG?" > > > > The error does not appear in this sequence > > > > This commit applies to 1.6.0 and adds a udev option for this modem, > > which disables the DHCP over AT command feature. > > > > So calling AT^DHCP actually modifies the behavior of the network > interface? Have we seen something like this in any other Huawei modem? I have to say the ME909u has fw version 12.636.11.01.00. We also use a ME609 with fw version 12.107.08.01.00 where it works correctly and with the ME909s which has as far as I know a HiSilicon chipset it works too. > > > Signed-off-by: Stefan Eichenberger > > --- > > plugins/huawei/77-mm-huawei-net-port-types.rules | 3 ++ > > plugins/huawei/mm-broadband-bearer-huawei.c | 50 > > ++-- > > 2 files changed, 42 insertions(+), 11 deletions(-) > > > > diff --git a/plugins/huawei/77-mm-huawei-net-port-types.rules > > b/plugins/huawei/77-mm-huawei-net-port-types.rules > > index f60f1f8..d35f2d5 100644 > > --- a/plugins/huawei/77-mm-huawei-net-port-types.rules > > +++ b/plugins/huawei/77-mm-huawei-net-port-types.rules > > @@ -6,6 +6,9 @@ ENV{ID_VENDOR_ID}!="12d1", GOTO="mm_huawei_port_types_end" > > # MU609 does not support getportmode (crashes modem with default firmware) > > ATTRS{idProduct}=="1573", ENV{ID_MM_HUAWEI_DISABLE_GETPORTMODE}="1" > > > > +# MU909u does not support DHCP properly, it can happen that the Ethernet > > frames do not attach the Ethernet header afterwards. > > +ATTRS{idProduct}=="1573", ENV{ID_MM_HUAWEI_DISABLE_DHCP}="1" > > + > > # Mark the modem and at port flags for ModemManager > > SUBSYSTEMS=="usb", ATTRS{bInterfaceClass}=="ff", > > ATTRS{bInterfaceSubClass}=="01", ATTRS{bInterfaceProtocol}=="01", > > ENV{ID_MM_HUAWEI_MODEM_PORT}="1" > > SUBSYSTEMS=="usb", ATTRS{bInterfaceClass}=="ff", > > ATTRS{bInterfaceSubClass}=="01", ATTRS{bInterfaceProtocol}=="02", > > ENV{ID_MM_HUAWEI_AT_PORT}="1" > > diff --git a/plugins/huawei/mm-broadband-bearer-huawei.c > > b/plugins/huawei/mm-broadband-bearer-huawei.c > > index 60a91e5..11782c3 100644 > > --- a/plugins/huawei/mm-broadband-bearer-huawei.c > > +++ b/plugins/huawei/mm-broadband-bearer-huawei.c > > @@ -465,17 +465,34 @@ connect_3gpp_context_step (Connect3gppContext *ctx) > > g_object_ref (ctx->self)); > > return; > > > > -case CONNECT_3GPP_CONTEXT_STEP_IP_CONFIG: > > -mm_base_modem_at_command_full (ctx->modem, > > - ctx->primary, > > - "^DHCP?", > > - 3, > > - FALSE, > > - FALSE, > > - NULL, > > - > > (GAsyncReadyCallback)connect_dhcp_check_ready, > > - g_object_ref (ctx->self)); > > -return; > > +case CONNECT_3GPP_CONTEXT_STEP_IP_CONFIG: { > > +GUdevClient *client; > > +GUdevDevice *data_device; > > + > > Don't call it "data_device"; as it's not; it's a tty_device. > > > +// ME909u has a problem with DHCP over AT. If it's done right > > after NDSIDUP > > +// the modem doesn't send the Ethernet header anymore which > > confuses the network stack > > No comments with //, use only /* this */ even for single line comments :) > > > +client = g_udev_client_new (NULL); > > +data_device = (g_udev_client_query_by_subsystem_and_name ( > > + client, > > + "tty", > > + mm_port_get_device > > (&ctx->primary->parent.parent))); > > No "&primary->parent.parent". I'm assuming you ne
Re: [PATCH] huawei: add basic support for ME909u-521
Hi Dan, On Thu, Sep 15, 2016 at 01:00:35PM -0500, Dan Williams wrote: > On Thu, 2016-09-15 at 18:57 +0200, Stefan Eichenberger wrote: > > The ME909u-512 does theoretically support the DHCP feature over AT > > but if > > this is done right after the NDISDUP command, it seems that the > > firmware > > has some trouble with it because the firmware will not attach the > > Ethernet header afterwards. This can be seen by doing a tcpdump while > > pinging e.g. 8.8.8.8. The request has a size of 98 Bytes, while the > > replies only have a size of 84 Bytes. The first 14 Bytes are missing > > which is exactly the size of the Ethernet header. > > > > To reproduce this issue, I've tried the command sequence manually: > > mmcli -m 0 --command 'AT^NDISDUP=1,1,"gprs.swisscom.ch"' > > mmcli -m 0 --command "AT+CREG?" > > mmcli -m 0 --command "AT^NDISSTATQRY?" > > mmcli -m 0 --command "AT+CGREG?" > > mmcli -m 0 --command "AT^DHCP?" > > > > The error appears in with this sequence > > > > mmcli -m 0 --command 'AT^NDISDUP=1,1,"gprs.swisscom.ch"' > > mmcli -m 0 --command "AT+CREG?" > > mmcli -m 0 --command "AT^NDISSTATQRY?" > > mmcli -m 0 --command "AT+CGREG?" > > Have you tested out how long the firmware needs before it can take the > AT^DHCP without problems? I was doing the test this morning again. It seems that it's not really time related. I can wait as long as I want between AT+CGREG? and AT^DHCP? if I do it the Ethernet header will be missing. But when I do a dhcp request over the Ethernet interface between the two steps, then I can do the AT^DHCP? without problems. So this was working for me: mmcli -m 0 --command 'AT^NDISDUP=1,1,"gprs.swisscom.ch"' mmcli -m 0 --command "AT+CREG?" mmcli -m 0 --command "AT^NDISSTATQRY?" mmcli -m 0 --command "AT+CGREG?" udhcpc -i usb0 mmcli -m 0 --command "AT^DHCP?" > > Dan > > > The error does not appear in this sequence > > > > This commit applies to 1.6.0 and adds a udev option for this modem, > > which disables the DHCP over AT command feature. > > > > Signed-off-by: Stefan Eichenberger > > > > --- > > plugins/huawei/77-mm-huawei-net-port-types.rules | 3 ++ > > plugins/huawei/mm-broadband-bearer-huawei.c | 50 > > ++-- > > 2 files changed, 42 insertions(+), 11 deletions(-) > > > > diff --git a/plugins/huawei/77-mm-huawei-net-port-types.rules > > b/plugins/huawei/77-mm-huawei-net-port-types.rules > > index f60f1f8..d35f2d5 100644 > > --- a/plugins/huawei/77-mm-huawei-net-port-types.rules > > +++ b/plugins/huawei/77-mm-huawei-net-port-types.rules > > @@ -6,6 +6,9 @@ ENV{ID_VENDOR_ID}!="12d1", > > GOTO="mm_huawei_port_types_end" > > # MU609 does not support getportmode (crashes modem with default > > firmware) > > ATTRS{idProduct}=="1573", ENV{ID_MM_HUAWEI_DISABLE_GETPORTMODE}="1" > > > > +# MU909u does not support DHCP properly, it can happen that the > > Ethernet frames do not attach the Ethernet header afterwards. > > +ATTRS{idProduct}=="1573", ENV{ID_MM_HUAWEI_DISABLE_DHCP}="1" > > + > > # Mark the modem and at port flags for ModemManager > > SUBSYSTEMS=="usb", ATTRS{bInterfaceClass}=="ff", > > ATTRS{bInterfaceSubClass}=="01", ATTRS{bInterfaceProtocol}=="01", > > ENV{ID_MM_HUAWEI_MODEM_PORT}="1" > > SUBSYSTEMS=="usb", ATTRS{bInterfaceClass}=="ff", > > ATTRS{bInterfaceSubClass}=="01", ATTRS{bInterfaceProtocol}=="02", > > ENV{ID_MM_HUAWEI_AT_PORT}="1" > > diff --git a/plugins/huawei/mm-broadband-bearer-huawei.c > > b/plugins/huawei/mm-broadband-bearer-huawei.c > > index 60a91e5..11782c3 100644 > > --- a/plugins/huawei/mm-broadband-bearer-huawei.c > > +++ b/plugins/huawei/mm-broadband-bearer-huawei.c > > @@ -465,17 +465,34 @@ connect_3gpp_context_step (Connect3gppContext > > *ctx) > > g_object_ref (ctx->self)); > > return; > > > > -case CONNECT_3GPP_CONTEXT_STEP_IP_CONFIG: > > -mm_base_modem_at_command_full (ctx->modem, > > - ctx->primary, > > - "^DHCP?", > > - 3, > > - FALSE, > > - FALSE, > > - NULL, > > - (GAsyncReadyCallback)connect_ > > dhcp_check_ready, > > - g_object_ref (ctx->self)); > > -return; > > +case CONNECT_3GPP_CONTEXT_STEP_IP_CONFIG: { > > +GUdevClient *client; > > +GUdevDevice *data_device; > > + > > +// ME909u has a problem with DHCP over AT. If it's done > > right after NDSIDUP > > +// the modem doesn't send the Ethernet header anymore which > > confuses the network stack > > +client = g_udev_client_new (NULL); > > +data_device = (g_udev_client_query_by_subsystem_and_name ( > > + client, > > + "tty", > > + mm_port_get_device (
Re: [PATCH] huawei: add basic support for ME909u-521
Hey, Code-wise review of the patch only here; see comments below. On Thu, Sep 15, 2016 at 6:57 PM, Stefan Eichenberger wrote: > > The ME909u-512 does theoretically support the DHCP feature over AT but if > this is done right after the NDISDUP command, it seems that the firmware > has some trouble with it because the firmware will not attach the > Ethernet header afterwards. This can be seen by doing a tcpdump while > pinging e.g. 8.8.8.8. The request has a size of 98 Bytes, while the > replies only have a size of 84 Bytes. The first 14 Bytes are missing > which is exactly the size of the Ethernet header. > > To reproduce this issue, I've tried the command sequence manually: > mmcli -m 0 --command 'AT^NDISDUP=1,1,"gprs.swisscom.ch"' > mmcli -m 0 --command "AT+CREG?" > mmcli -m 0 --command "AT^NDISSTATQRY?" > mmcli -m 0 --command "AT+CGREG?" > mmcli -m 0 --command "AT^DHCP?" > > The error appears in with this sequence > > mmcli -m 0 --command 'AT^NDISDUP=1,1,"gprs.swisscom.ch"' > mmcli -m 0 --command "AT+CREG?" > mmcli -m 0 --command "AT^NDISSTATQRY?" > mmcli -m 0 --command "AT+CGREG?" > > The error does not appear in this sequence > > This commit applies to 1.6.0 and adds a udev option for this modem, > which disables the DHCP over AT command feature. > So calling AT^DHCP actually modifies the behavior of the network interface? Have we seen something like this in any other Huawei modem? > Signed-off-by: Stefan Eichenberger > --- > plugins/huawei/77-mm-huawei-net-port-types.rules | 3 ++ > plugins/huawei/mm-broadband-bearer-huawei.c | 50 > ++-- > 2 files changed, 42 insertions(+), 11 deletions(-) > > diff --git a/plugins/huawei/77-mm-huawei-net-port-types.rules > b/plugins/huawei/77-mm-huawei-net-port-types.rules > index f60f1f8..d35f2d5 100644 > --- a/plugins/huawei/77-mm-huawei-net-port-types.rules > +++ b/plugins/huawei/77-mm-huawei-net-port-types.rules > @@ -6,6 +6,9 @@ ENV{ID_VENDOR_ID}!="12d1", GOTO="mm_huawei_port_types_end" > # MU609 does not support getportmode (crashes modem with default firmware) > ATTRS{idProduct}=="1573", ENV{ID_MM_HUAWEI_DISABLE_GETPORTMODE}="1" > > +# MU909u does not support DHCP properly, it can happen that the Ethernet > frames do not attach the Ethernet header afterwards. > +ATTRS{idProduct}=="1573", ENV{ID_MM_HUAWEI_DISABLE_DHCP}="1" > + > # Mark the modem and at port flags for ModemManager > SUBSYSTEMS=="usb", ATTRS{bInterfaceClass}=="ff", > ATTRS{bInterfaceSubClass}=="01", ATTRS{bInterfaceProtocol}=="01", > ENV{ID_MM_HUAWEI_MODEM_PORT}="1" > SUBSYSTEMS=="usb", ATTRS{bInterfaceClass}=="ff", > ATTRS{bInterfaceSubClass}=="01", ATTRS{bInterfaceProtocol}=="02", > ENV{ID_MM_HUAWEI_AT_PORT}="1" > diff --git a/plugins/huawei/mm-broadband-bearer-huawei.c > b/plugins/huawei/mm-broadband-bearer-huawei.c > index 60a91e5..11782c3 100644 > --- a/plugins/huawei/mm-broadband-bearer-huawei.c > +++ b/plugins/huawei/mm-broadband-bearer-huawei.c > @@ -465,17 +465,34 @@ connect_3gpp_context_step (Connect3gppContext *ctx) > g_object_ref (ctx->self)); > return; > > -case CONNECT_3GPP_CONTEXT_STEP_IP_CONFIG: > -mm_base_modem_at_command_full (ctx->modem, > - ctx->primary, > - "^DHCP?", > - 3, > - FALSE, > - FALSE, > - NULL, > - > (GAsyncReadyCallback)connect_dhcp_check_ready, > - g_object_ref (ctx->self)); > -return; > +case CONNECT_3GPP_CONTEXT_STEP_IP_CONFIG: { > +GUdevClient *client; > +GUdevDevice *data_device; > + Don't call it "data_device"; as it's not; it's a tty_device. > +// ME909u has a problem with DHCP over AT. If it's done right after > NDSIDUP > +// the modem doesn't send the Ethernet header anymore which confuses > the network stack No comments with //, use only /* this */ even for single line comments :) > +client = g_udev_client_new (NULL); > +data_device = (g_udev_client_query_by_subsystem_and_name ( > + client, > + "tty", > + mm_port_get_device > (&ctx->primary->parent.parent))); No "&primary->parent.parent". I'm assuming you need a MMPort from a MMPortAt; so you should do: mm_port_get_device (MM_PORT (ctx->primary)) > +if (!data_device || !g_udev_device_get_property_as_boolean > (data_device, "ID_MM_HUAWEI_DISABLE_DHCP")) { Shouldn't we actually error out if !data_device? > +mm_base_modem_at_command_full (ctx->modem, > + ctx->primary, > + "^DHCP?", > + 3, > +
Re: [PATCH] huawei: add basic support for ME909u-521
On Thu, 2016-09-15 at 18:57 +0200, Stefan Eichenberger wrote: > The ME909u-512 does theoretically support the DHCP feature over AT > but if > this is done right after the NDISDUP command, it seems that the > firmware > has some trouble with it because the firmware will not attach the > Ethernet header afterwards. This can be seen by doing a tcpdump while > pinging e.g. 8.8.8.8. The request has a size of 98 Bytes, while the > replies only have a size of 84 Bytes. The first 14 Bytes are missing > which is exactly the size of the Ethernet header. > > To reproduce this issue, I've tried the command sequence manually: > mmcli -m 0 --command 'AT^NDISDUP=1,1,"gprs.swisscom.ch"' > mmcli -m 0 --command "AT+CREG?" > mmcli -m 0 --command "AT^NDISSTATQRY?" > mmcli -m 0 --command "AT+CGREG?" > mmcli -m 0 --command "AT^DHCP?" > > The error appears in with this sequence > > mmcli -m 0 --command 'AT^NDISDUP=1,1,"gprs.swisscom.ch"' > mmcli -m 0 --command "AT+CREG?" > mmcli -m 0 --command "AT^NDISSTATQRY?" > mmcli -m 0 --command "AT+CGREG?" Have you tested out how long the firmware needs before it can take the AT^DHCP without problems? Dan > The error does not appear in this sequence > > This commit applies to 1.6.0 and adds a udev option for this modem, > which disables the DHCP over AT command feature. > > Signed-off-by: Stefan Eichenberger > > --- > plugins/huawei/77-mm-huawei-net-port-types.rules | 3 ++ > plugins/huawei/mm-broadband-bearer-huawei.c | 50 > ++-- > 2 files changed, 42 insertions(+), 11 deletions(-) > > diff --git a/plugins/huawei/77-mm-huawei-net-port-types.rules > b/plugins/huawei/77-mm-huawei-net-port-types.rules > index f60f1f8..d35f2d5 100644 > --- a/plugins/huawei/77-mm-huawei-net-port-types.rules > +++ b/plugins/huawei/77-mm-huawei-net-port-types.rules > @@ -6,6 +6,9 @@ ENV{ID_VENDOR_ID}!="12d1", > GOTO="mm_huawei_port_types_end" > # MU609 does not support getportmode (crashes modem with default > firmware) > ATTRS{idProduct}=="1573", ENV{ID_MM_HUAWEI_DISABLE_GETPORTMODE}="1" > > +# MU909u does not support DHCP properly, it can happen that the > Ethernet frames do not attach the Ethernet header afterwards. > +ATTRS{idProduct}=="1573", ENV{ID_MM_HUAWEI_DISABLE_DHCP}="1" > + > # Mark the modem and at port flags for ModemManager > SUBSYSTEMS=="usb", ATTRS{bInterfaceClass}=="ff", > ATTRS{bInterfaceSubClass}=="01", ATTRS{bInterfaceProtocol}=="01", > ENV{ID_MM_HUAWEI_MODEM_PORT}="1" > SUBSYSTEMS=="usb", ATTRS{bInterfaceClass}=="ff", > ATTRS{bInterfaceSubClass}=="01", ATTRS{bInterfaceProtocol}=="02", > ENV{ID_MM_HUAWEI_AT_PORT}="1" > diff --git a/plugins/huawei/mm-broadband-bearer-huawei.c > b/plugins/huawei/mm-broadband-bearer-huawei.c > index 60a91e5..11782c3 100644 > --- a/plugins/huawei/mm-broadband-bearer-huawei.c > +++ b/plugins/huawei/mm-broadband-bearer-huawei.c > @@ -465,17 +465,34 @@ connect_3gpp_context_step (Connect3gppContext > *ctx) > g_object_ref (ctx->self)); > return; > > -case CONNECT_3GPP_CONTEXT_STEP_IP_CONFIG: > -mm_base_modem_at_command_full (ctx->modem, > - ctx->primary, > - "^DHCP?", > - 3, > - FALSE, > - FALSE, > - NULL, > - (GAsyncReadyCallback)connect_ > dhcp_check_ready, > - g_object_ref (ctx->self)); > -return; > +case CONNECT_3GPP_CONTEXT_STEP_IP_CONFIG: { > +GUdevClient *client; > +GUdevDevice *data_device; > + > +// ME909u has a problem with DHCP over AT. If it's done > right after NDSIDUP > +// the modem doesn't send the Ethernet header anymore which > confuses the network stack > +client = g_udev_client_new (NULL); > +data_device = (g_udev_client_query_by_subsystem_and_name ( > + client, > + "tty", > + mm_port_get_device (&ctx->primary- > >parent.parent))); > +if (!data_device || !g_udev_device_get_property_as_boolean > (data_device, "ID_MM_HUAWEI_DISABLE_DHCP")) { > +mm_base_modem_at_command_full (ctx->modem, > + ctx->primary, > + "^DHCP?", > + 3, > + FALSE, > + FALSE, > + NULL, > + (GAsyncReadyCallback)conn > ect_dhcp_check_ready, > + g_object_ref (ctx- > >self)); > +return; > +} > + > +mm_info("This device (%s) does not support DHCP over AT", > mm_port_g