[PATCH] umb(4) fixes for ifconfig

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] umb(4) fixes for ifconfig

Bryan Vyhmeister-3
This patch accomplishes two things for umb(4) connections. One, some SIM
cards have a plus at the beginning of the phone number while others do
not. In my case, an AT&T Wireless SIM card in the US has the plus where
a T-Mobile SIM card in Germany does not. The output of ifconfig(8)
originally added a plus regardless which caused two plus signs to appear
for me as below. With this patch, ifconfig(8) now adds the plus if it
is not there and does not add the plus if it is already programmed into
the SIM card.

umb0: flags=8951<UP,POINTOPOINT,RUNNING,PROMISC,SIMPLEX,MULTICAST> mtu 1430
        index 5 priority 0 llprio 3
        roaming disabled registration home network
        state #7 cell-class LTE rssi -79dBm speed 47.7Mps up 286Mps down
        SIM initialized PIN valid (3 attempts left)
        subscriber-id 31041088 ICC-id 8901410427 provider AT&T
        device EM7455 IMEI 014582000 firmware SWI9X30C_02.08.
        phone# ++1213555123 APN broadband
        dns 172.26.38.1
        groups: egress
        status: active
        inet 10.45.181.18 --> 10.45.181.17 netmask 0xfffffffc


The second part of the patch also fixes the last digit of the phone
number and mulitple digits of the subscriber-id, ICC-id, and IMEI being
cut off. Both fixes were discussed with Gerhard Roth. I sent him an
original patch but it only partially solved the issues and after
discussion with him, he came up with the patch below which solves and
fully accounts for the issues and possible scenarios. With this patch
the output looks like this.

umb0: flags=8851<UP,POINTOPOINT,RUNNING,SIMPLEX,MULTICAST> mtu 1430
        index 5 priority 0 llprio 3
        roaming disabled registration home network
        state #7 cell-class LTE rssi -103dBm speed 47.7Mps up 286Mps down
        SIM initialized PIN valid (3 attempts left)
        subscriber-id 310410812345678 ICC-id 89014104278812345678 provider AT&T
        device EM7455 IMEI 014582012345678 firmware SWI9X30C_02.08.02.00
        phone# +12135551234 APN broadband
        dns 172.26.38.1
        status: active
        inet 10.84.107.51 --> 10.84.107.52 netmask 0xfffffff8


Bryan


Index: sbin/ifconfig/ifconfig.c
===================================================================
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.332
diff -u -p -r1.332 ifconfig.c
--- sbin/ifconfig/ifconfig.c 8 Nov 2016 14:37:20 -0000 1.332
+++ sbin/ifconfig/ifconfig.c 9 Nov 2016 16:34:26 -0000
@@ -5153,7 +5153,7 @@ umb_status(void)
  printf("\t");
  n = 0;
  if (pn[0])
- printf("%sphone# +%s", n++ ? " " : "", pn);
+ printf("%sphone# +%s", n++ ? " " : "", pn[0] == '+' ? &pn[1] : pn);
  if (apn[0])
  printf("%sAPN %s", n++ ? " " : "", apn);
  printf("\n");
@@ -5323,7 +5323,7 @@ done:
  }
  *out++ = isascii(c) ? (char)c : '?';
  in++;
- inlen -= sizeof (*in);
+ inlen--;
  }
 }

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] umb(4) fixes for ifconfig

Otto Moerbeek
On Wed, Nov 09, 2016 at 08:51:04AM -0800, Bryan Vyhmeister wrote:

> This patch accomplishes two things for umb(4) connections. One, some SIM
> cards have a plus at the beginning of the phone number while others do
> not. In my case, an AT&T Wireless SIM card in the US has the plus where
> a T-Mobile SIM card in Germany does not. The output of ifconfig(8)
> originally added a plus regardless which caused two plus signs to appear
> for me as below. With this patch, ifconfig(8) now adds the plus if it
> is not there and does not add the plus if it is already programmed into
> the SIM card.
>
> umb0: flags=8951<UP,POINTOPOINT,RUNNING,PROMISC,SIMPLEX,MULTICAST> mtu 1430
>         index 5 priority 0 llprio 3
>         roaming disabled registration home network
>         state #7 cell-class LTE rssi -79dBm speed 47.7Mps up 286Mps down
>         SIM initialized PIN valid (3 attempts left)
>         subscriber-id 31041088 ICC-id 8901410427 provider AT&T
>         device EM7455 IMEI 014582000 firmware SWI9X30C_02.08.
>         phone# ++1213555123 APN broadband
>         dns 172.26.38.1
>         groups: egress
>         status: active
>         inet 10.45.181.18 --> 10.45.181.17 netmask 0xfffffffc
>
>
> The second part of the patch also fixes the last digit of the phone
> number and mulitple digits of the subscriber-id, ICC-id, and IMEI being
> cut off. Both fixes were discussed with Gerhard Roth. I sent him an
> original patch but it only partially solved the issues and after
> discussion with him, he came up with the patch below which solves and
> fully accounts for the issues and possible scenarios. With this patch
> the output looks like this.
>
> umb0: flags=8851<UP,POINTOPOINT,RUNNING,SIMPLEX,MULTICAST> mtu 1430
>         index 5 priority 0 llprio 3
>         roaming disabled registration home network
> state #7 cell-class LTE rssi -103dBm speed 47.7Mps up 286Mps down
>         SIM initialized PIN valid (3 attempts left)
> subscriber-id 310410812345678 ICC-id 89014104278812345678 provider AT&T
>         device EM7455 IMEI 014582012345678 firmware SWI9X30C_02.08.02.00
>         phone# +12135551234 APN broadband
>         dns 172.26.38.1
>         status: active
>         inet 10.84.107.51 --> 10.84.107.52 netmask 0xfffffff8
>
>
> Bryan
>
>
> Index: sbin/ifconfig/ifconfig.c
> ===================================================================
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.332
> diff -u -p -r1.332 ifconfig.c
> --- sbin/ifconfig/ifconfig.c 8 Nov 2016 14:37:20 -0000 1.332
> +++ sbin/ifconfig/ifconfig.c 9 Nov 2016 16:34:26 -0000
> @@ -5153,7 +5153,7 @@ umb_status(void)
>   printf("\t");
>   n = 0;
>   if (pn[0])
> - printf("%sphone# +%s", n++ ? " " : "", pn);
> + printf("%sphone# +%s", n++ ? " " : "", pn[0] == '+' ? &pn[1] : pn);
>   if (apn[0])
>   printf("%sAPN %s", n++ ? " " : "", apn);
>   printf("\n");
> @@ -5323,7 +5323,7 @@ done:
>   }
>   *out++ = isascii(c) ? (char)c : '?';
>   in++;
> - inlen -= sizeof (*in);
> + inlen--;
>   }
>  }

Id's say just print the number as-is, no special casing for plus etc.

The inlen chunk is ok.

By some coincidence I was looking at the H5321gw card in my thinkpad.
I'm making some progress; before no valid frames were received at all,
now at least I get some. There seems to be some problem decoding the
header, and that ends up getting offsets wrong (on my card the data
does not follow the header immediately (at offset 0x0c), but at offset
0x20). More to follow later on when I find some more time
investigating this.

        -Otto



Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] umb(4) fixes for ifconfig

Gerhard Roth-2
On Thu, 10 Nov 2016 13:56:00 +0100 Otto Moerbeek <[hidden email]> wrote:

> On Wed, Nov 09, 2016 at 08:51:04AM -0800, Bryan Vyhmeister wrote:
>
> > This patch accomplishes two things for umb(4) connections. One, some SIM
> > cards have a plus at the beginning of the phone number while others do
> > not. In my case, an AT&T Wireless SIM card in the US has the plus where
> > a T-Mobile SIM card in Germany does not. The output of ifconfig(8)
> > originally added a plus regardless which caused two plus signs to appear
> > for me as below. With this patch, ifconfig(8) now adds the plus if it
> > is not there and does not add the plus if it is already programmed into
> > the SIM card.
> >
> > umb0: flags=8951<UP,POINTOPOINT,RUNNING,PROMISC,SIMPLEX,MULTICAST> mtu 1430
> >         index 5 priority 0 llprio 3
> >         roaming disabled registration home network
> >         state #7 cell-class LTE rssi -79dBm speed 47.7Mps up 286Mps down
> >         SIM initialized PIN valid (3 attempts left)
> >         subscriber-id 31041088 ICC-id 8901410427 provider AT&T
> >         device EM7455 IMEI 014582000 firmware SWI9X30C_02.08.
> >         phone# ++1213555123 APN broadband
> >         dns 172.26.38.1
> >         groups: egress
> >         status: active
> >         inet 10.45.181.18 --> 10.45.181.17 netmask 0xfffffffc
> >
> >
> > The second part of the patch also fixes the last digit of the phone
> > number and mulitple digits of the subscriber-id, ICC-id, and IMEI being
> > cut off. Both fixes were discussed with Gerhard Roth. I sent him an
> > original patch but it only partially solved the issues and after
> > discussion with him, he came up with the patch below which solves and
> > fully accounts for the issues and possible scenarios. With this patch
> > the output looks like this.
> >
> > umb0: flags=8851<UP,POINTOPOINT,RUNNING,SIMPLEX,MULTICAST> mtu 1430
> >         index 5 priority 0 llprio 3
> >         roaming disabled registration home network
> > state #7 cell-class LTE rssi -103dBm speed 47.7Mps up 286Mps down
> >         SIM initialized PIN valid (3 attempts left)
> > subscriber-id 310410812345678 ICC-id 89014104278812345678 provider AT&T
> >         device EM7455 IMEI 014582012345678 firmware SWI9X30C_02.08.02.00
> >         phone# +12135551234 APN broadband
> >         dns 172.26.38.1
> >         status: active
> >         inet 10.84.107.51 --> 10.84.107.52 netmask 0xfffffff8
> >
> >
> > Bryan
> >
> >
> > Index: sbin/ifconfig/ifconfig.c
> > ===================================================================
> > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> > retrieving revision 1.332
> > diff -u -p -r1.332 ifconfig.c
> > --- sbin/ifconfig/ifconfig.c 8 Nov 2016 14:37:20 -0000 1.332
> > +++ sbin/ifconfig/ifconfig.c 9 Nov 2016 16:34:26 -0000
> > @@ -5153,7 +5153,7 @@ umb_status(void)
> >   printf("\t");
> >   n = 0;
> >   if (pn[0])
> > - printf("%sphone# +%s", n++ ? " " : "", pn);
> > + printf("%sphone# +%s", n++ ? " " : "", pn[0] == '+' ? &pn[1] : pn);
> >   if (apn[0])
> >   printf("%sAPN %s", n++ ? " " : "", apn);
> >   printf("\n");
> > @@ -5323,7 +5323,7 @@ done:
> >   }
> >   *out++ = isascii(c) ? (char)c : '?';
> >   in++;
> > - inlen -= sizeof (*in);
> > + inlen--;
> >   }
> >  }
>
> Id's say just print the number as-is, no special casing for plus etc.
>
> The inlen chunk is ok.
>
> By some coincidence I was looking at the H5321gw card in my thinkpad.
> I'm making some progress; before no valid frames were received at all,
> now at least I get some. There seems to be some problem decoding the
> header, and that ends up getting offsets wrong (on my card the data
> does not follow the header immediately (at offset 0x0c), but at offset
> 0x20). More to follow later on when I find some more time
> investigating this.
>
> -Otto
>
>


Hi Otto,

that is probably due to my mistake. I assumed the the NCM pointer
structure will always immediately follow the NCM header because
all devices I had for testing did so. But according to the NCM
spec this is not necessary. And some devices apparently format
the buffer like this:

              +--------------------------------+
              |                                |
              |                                v
        [NCM header] [datagram, i.e. payload] [NCM datagram pointer]
                      ^                                |
                      |                                |
                      +--------------------------------+


Could you please the the patch below. It'll look for the NCM pointer
structure at the offset stored in the NCM header instead of just
looking at the data after the header.

Gerhard


Index: sys/dev/usb/if_umb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.4
diff -u -p -u -p -r1.4 if_umb.c
--- sys/dev/usb/if_umb.c 25 Oct 2016 16:31:08 -0000 1.4
+++ sys/dev/usb/if_umb.c 10 Nov 2016 13:03:48 -0000
@@ -1734,9 +1734,12 @@ umb_decap(struct umb_softc *sc, struct u
  hdr16 = (struct ncm_header16 *)buf;
  hsig = UGETDW(hdr16->dwSignature);
  hlen = UGETW(hdr16->wHeaderLength);
+ if (len < hlen)
+ goto toosmall;
  switch (hsig) {
  case NCM_HDR16_SIG:
  blen = UGETW(hdr16->wBlockLength);
+ ptroff = UGETW(hdr16->wNdpIndex);
  if (hlen != sizeof (*hdr16)) {
  DPRINTF("%s: bad header len %d for NTH16 (exp %zu)\n",
     DEVNAM(sc), hlen, sizeof (*hdr16));
@@ -1746,6 +1749,7 @@ umb_decap(struct umb_softc *sc, struct u
  case NCM_HDR32_SIG:
  hdr32 = (struct ncm_header32 *)hdr16;
  blen = UGETDW(hdr32->dwBlockLength);
+ ptroff = UGETDW(hdr32->dwNdpIndex);
  if (hlen != sizeof (*hdr32)) {
  DPRINTF("%s: bad header len %d for NTH32 (exp %zu)\n",
     DEVNAM(sc), hlen, sizeof (*hdr32));
@@ -1757,15 +1761,12 @@ umb_decap(struct umb_softc *sc, struct u
     DEVNAM(sc), hsig);
  goto fail;
  }
- if (len < hlen)
- goto toosmall;
  if (len < blen) {
  DPRINTF("%s: bad NTB len (%d) for %d bytes of data\n",
     DEVNAM(sc), blen, len);
  goto fail;
  }
 
- ptroff = hlen;
  ptr16 = (struct ncm_pointer16 *)(buf + ptroff);
  psig = UGETDW(ptr16->dwSignature);
  ptrlen = UGETW(ptr16->wLength);

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] umb(4) fixes for ifconfig

Otto Moerbeek
On Thu, Nov 10, 2016 at 02:12:55PM +0100, Gerhard Roth wrote:

> On Thu, 10 Nov 2016 13:56:00 +0100 Otto Moerbeek <[hidden email]> wrote:
> > On Wed, Nov 09, 2016 at 08:51:04AM -0800, Bryan Vyhmeister wrote:
> >
> > > This patch accomplishes two things for umb(4) connections. One, some SIM
> > > cards have a plus at the beginning of the phone number while others do
> > > not. In my case, an AT&T Wireless SIM card in the US has the plus where
> > > a T-Mobile SIM card in Germany does not. The output of ifconfig(8)
> > > originally added a plus regardless which caused two plus signs to appear
> > > for me as below. With this patch, ifconfig(8) now adds the plus if it
> > > is not there and does not add the plus if it is already programmed into
> > > the SIM card.
> > >
> > > umb0: flags=8951<UP,POINTOPOINT,RUNNING,PROMISC,SIMPLEX,MULTICAST> mtu 1430
> > >         index 5 priority 0 llprio 3
> > >         roaming disabled registration home network
> > >         state #7 cell-class LTE rssi -79dBm speed 47.7Mps up 286Mps down
> > >         SIM initialized PIN valid (3 attempts left)
> > >         subscriber-id 31041088 ICC-id 8901410427 provider AT&T
> > >         device EM7455 IMEI 014582000 firmware SWI9X30C_02.08.
> > >         phone# ++1213555123 APN broadband
> > >         dns 172.26.38.1
> > >         groups: egress
> > >         status: active
> > >         inet 10.45.181.18 --> 10.45.181.17 netmask 0xfffffffc
> > >
> > >
> > > The second part of the patch also fixes the last digit of the phone
> > > number and mulitple digits of the subscriber-id, ICC-id, and IMEI being
> > > cut off. Both fixes were discussed with Gerhard Roth. I sent him an
> > > original patch but it only partially solved the issues and after
> > > discussion with him, he came up with the patch below which solves and
> > > fully accounts for the issues and possible scenarios. With this patch
> > > the output looks like this.
> > >
> > > umb0: flags=8851<UP,POINTOPOINT,RUNNING,SIMPLEX,MULTICAST> mtu 1430
> > >         index 5 priority 0 llprio 3
> > >         roaming disabled registration home network
> > > state #7 cell-class LTE rssi -103dBm speed 47.7Mps up 286Mps down
> > >         SIM initialized PIN valid (3 attempts left)
> > > subscriber-id 310410812345678 ICC-id 89014104278812345678 provider AT&T
> > >         device EM7455 IMEI 014582012345678 firmware SWI9X30C_02.08.02.00
> > >         phone# +12135551234 APN broadband
> > >         dns 172.26.38.1
> > >         status: active
> > >         inet 10.84.107.51 --> 10.84.107.52 netmask 0xfffffff8
> > >
> > >
> > > Bryan
> > >
> > >
> > > Index: sbin/ifconfig/ifconfig.c
> > > ===================================================================
> > > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> > > retrieving revision 1.332
> > > diff -u -p -r1.332 ifconfig.c
> > > --- sbin/ifconfig/ifconfig.c 8 Nov 2016 14:37:20 -0000 1.332
> > > +++ sbin/ifconfig/ifconfig.c 9 Nov 2016 16:34:26 -0000
> > > @@ -5153,7 +5153,7 @@ umb_status(void)
> > >   printf("\t");
> > >   n = 0;
> > >   if (pn[0])
> > > - printf("%sphone# +%s", n++ ? " " : "", pn);
> > > + printf("%sphone# +%s", n++ ? " " : "", pn[0] == '+' ? &pn[1] : pn);
> > >   if (apn[0])
> > >   printf("%sAPN %s", n++ ? " " : "", apn);
> > >   printf("\n");
> > > @@ -5323,7 +5323,7 @@ done:
> > >   }
> > >   *out++ = isascii(c) ? (char)c : '?';
> > >   in++;
> > > - inlen -= sizeof (*in);
> > > + inlen--;
> > >   }
> > >  }
> >
> > Id's say just print the number as-is, no special casing for plus etc.
> >
> > The inlen chunk is ok.
> >
> > By some coincidence I was looking at the H5321gw card in my thinkpad.
> > I'm making some progress; before no valid frames were received at all,
> > now at least I get some. There seems to be some problem decoding the
> > header, and that ends up getting offsets wrong (on my card the data
> > does not follow the header immediately (at offset 0x0c), but at offset
> > 0x20). More to follow later on when I find some more time
> > investigating this.
> >
> > -Otto
> >
> >
>
>
> Hi Otto,
>
> that is probably due to my mistake. I assumed the the NCM pointer
> structure will always immediately follow the NCM header because
> all devices I had for testing did so. But according to the NCM
> spec this is not necessary. And some devices apparently format
> the buffer like this:
>
>               +--------------------------------+
>               |                                |
>               |                                v
>         [NCM header] [datagram, i.e. payload] [NCM datagram pointer]
>                       ^                                |
>                       |                                |
>                       +--------------------------------+
>
>
> Could you please the the patch below. It'll look for the NCM pointer
> structure at the offset stored in the NCM header instead of just
> looking at the data after the header.
>
> Gerhard

Hi Gerhard,

With this diff I'm able to recieve frames, but performance downloading
over tcp is not good.  (the same as with my hack that just used hlen
0x20).

I have the idea that in some cases packets are not decoded properly.
When pinging from my laptop I see:

14:26:23.034351 ip: 90.142.115.171 > 194.109.6.93: icmp: echo request
14:26:23.107786 ip: 194.109.6.93 > 90.142.115.171: icmp: echo reply
14:26:24.034375 ip: 90.142.115.171 > 194.109.6.93: icmp: echo request
14:26:24.107847 ip: 194.109.6.93 > 90.142.115.171: icmp: echo reply
14:26:24.107849 ip: truncated-ip - 19672 bytes missing!0.0.0.0 >
0.0.0.0: (frag 3072:56@24608+) (DF) [tos 0x43 (EC)]
14:26:25.034391 ip: 90.142.115.171 > 194.109.6.93: icmp: echo request
14:26:25.107834 ip: 194.109.6.93 > 90.142.115.171: icmp: echo reply


If some packets are malformed, it is not a surprise tcp suffers....

But at least I'm able to use the card now...

        -Otto

>
>
> Index: sys/dev/usb/if_umb.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.4
> diff -u -p -u -p -r1.4 if_umb.c
> --- sys/dev/usb/if_umb.c 25 Oct 2016 16:31:08 -0000 1.4
> +++ sys/dev/usb/if_umb.c 10 Nov 2016 13:03:48 -0000
> @@ -1734,9 +1734,12 @@ umb_decap(struct umb_softc *sc, struct u
>   hdr16 = (struct ncm_header16 *)buf;
>   hsig = UGETDW(hdr16->dwSignature);
>   hlen = UGETW(hdr16->wHeaderLength);
> + if (len < hlen)
> + goto toosmall;
>   switch (hsig) {
>   case NCM_HDR16_SIG:
>   blen = UGETW(hdr16->wBlockLength);
> + ptroff = UGETW(hdr16->wNdpIndex);
>   if (hlen != sizeof (*hdr16)) {
>   DPRINTF("%s: bad header len %d for NTH16 (exp %zu)\n",
>      DEVNAM(sc), hlen, sizeof (*hdr16));
> @@ -1746,6 +1749,7 @@ umb_decap(struct umb_softc *sc, struct u
>   case NCM_HDR32_SIG:
>   hdr32 = (struct ncm_header32 *)hdr16;
>   blen = UGETDW(hdr32->dwBlockLength);
> + ptroff = UGETDW(hdr32->dwNdpIndex);
>   if (hlen != sizeof (*hdr32)) {
>   DPRINTF("%s: bad header len %d for NTH32 (exp %zu)\n",
>      DEVNAM(sc), hlen, sizeof (*hdr32));
> @@ -1757,15 +1761,12 @@ umb_decap(struct umb_softc *sc, struct u
>      DEVNAM(sc), hsig);
>   goto fail;
>   }
> - if (len < hlen)
> - goto toosmall;
>   if (len < blen) {
>   DPRINTF("%s: bad NTB len (%d) for %d bytes of data\n",
>      DEVNAM(sc), blen, len);
>   goto fail;
>   }
>  
> - ptroff = hlen;
>   ptr16 = (struct ncm_pointer16 *)(buf + ptroff);
>   psig = UGETDW(ptr16->dwSignature);
>   ptrlen = UGETW(ptr16->wLength);