IPv6 Support for umb(4)

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

IPv6 Support for umb(4)

Gerhard Roth-2
Hi,

this patch adds IPv6 support to umb(4).

It will try to obtain a IPv6 address if the kernel is compiled with INET6.
Currently there is no option to disable IPv6 on such a kernel (other than
manually calling "ifconfig umb0 -inet6"). Nor is there a IPv6-only mode which
refrains from obtaining an IPv4 address from the kernel.

To get an IPv6 address, your provider has to offer one. But more importantly
the firmware of your umb(4) device has to have IPv6 support. I stumbled
across two older Sierra Wireless modules (EM8805 and MC3805) that refused
to provide an IPv6 address.

Have fun,

Gerhard


Index: sbin/ifconfig/ifconfig.c
===================================================================
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.417
diff -u -p -u -p -r1.417 ifconfig.c
--- sbin/ifconfig/ifconfig.c 27 Dec 2019 14:34:46 -0000 1.417
+++ sbin/ifconfig/ifconfig.c 23 Jan 2020 09:24:38 -0000
@@ -5666,6 +5666,7 @@ umb_status(void)
  char apn[UMB_APN_MAXLEN+1];
  char pn[UMB_PHONENR_MAXLEN+1];
  int i, n;
+ char astr[INET6_ADDRSTRLEN];
 
  memset((char *)&mi, 0, sizeof(mi));
  ifr.ifr_data = (caddr_t)&mi;
@@ -5830,7 +5831,15 @@ umb_status(void)
  for (i = 0, n = 0; i < UMB_MAX_DNSSRV; i++) {
  if (mi.ipv4dns[i].s_addr == INADDR_ANY)
  break;
- printf("%s %s", n++ ? "" : "\tdns", inet_ntoa(mi.ipv4dns[i]));
+ printf("%s %s", n++ ? "" : "\tdns",
+    inet_ntop(AF_INET, &mi.ipv4dns[i], astr, sizeof (astr)));
+ }
+ for (i = 0; i < UMB_MAX_DNSSRV; i++) {
+ if (memcmp(&mi.ipv6dns[i], &in6addr_any,
+    sizeof (mi.ipv6dns[i])) == 0)
+ break;
+ printf("%s %s", n++ ? "" : "\tdns",
+    inet_ntop(AF_INET6, &mi.ipv6dns[i], astr, sizeof (astr)));
  }
  if (n)
  printf("\n");
Index: share/man/man4/umb.4
===================================================================
RCS file: /cvs/src/share/man/man4/umb.4,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 umb.4
--- share/man/man4/umb.4 23 Nov 2017 20:47:26 -0000 1.9
+++ share/man/man4/umb.4 28 Jan 2020 09:04:20 -0000
@@ -40,6 +40,11 @@ will remain in this state until the MBIM
 In case the device is connected to an "always-on" USB port,
 it may be possible to connect to a provider without entering the
 PIN again even if the system was rebooted.
+.Pp
+If the kernel has been compiled with INET6, the driver will try to
+obtain an IPv6 address from the provider. To succeed with the IPv6
+configuration, both the ISP and the MBIM device have to offer IPv6
+support.
 .Sh HARDWARE
 The following devices should work:
 .Pp
@@ -64,10 +69,6 @@ The following devices should work:
 .%U http://www.usb.org/developers/docs/devclass_docs/MBIM10Errata1_073013.zip
 .Re
 .Sh CAVEATS
-The
-.Nm
-driver does not support IPv6.
-.Pp
 Devices which fail to provide a conforming MBIM implementation will
 probably be attached as some other driver, such as
 .Xr umsm 4 .
Index: sys/dev/usb/if_umb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.31
diff -u -p -u -p -r1.31 if_umb.c
--- sys/dev/usb/if_umb.c 26 Nov 2019 23:04:28 -0000 1.31
+++ sys/dev/usb/if_umb.c 28 Jan 2020 09:08:16 -0000
@@ -43,6 +43,14 @@
 #include <netinet/in_var.h>
 #include <netinet/ip.h>
 
+#ifdef INET6
+#include <netinet/ip6.h>
+#include <netinet6/in6_var.h>
+#include <netinet6/ip6_var.h>
+#include <netinet6/in6_ifattach.h>
+#include <netinet6/nd6.h>
+#endif
+
 #include <machine/bus.h>
 
 #include <dev/usb/usb.h>
@@ -158,7 +166,11 @@ int umb_decode_connect_info(struct umb
 void umb_clear_addr(struct umb_softc *);
 int umb_add_inet_config(struct umb_softc *, struct in_addr, u_int,
     struct in_addr);
-void umb_send_inet_proposal(struct umb_softc *);
+#ifdef INET6
+int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
+    u_int, struct in6_addr *);
+#endif
+void umb_send_inet_proposal(struct umb_softc *, int);
 int umb_decode_ip_configuration(struct umb_softc *, void *, int);
 void umb_rx(struct umb_softc *);
 void umb_rxeof(struct usbd_xfer *, void *, usbd_status);
@@ -800,8 +812,8 @@ umb_input(struct ifnet *ifp, struct mbuf
 #endif /* INET6 */
  default:
  ifp->if_ierrors++;
- DPRINTFN(4, "%s: dropping packet with bad IP version (%d)\n",
-    __func__, ipv);
+ DPRINTFN(4, "%s: dropping packet with bad IP version (af %d)\n",
+    __func__, af);
  m_freem(m);
  return 1;
  }
@@ -902,7 +914,10 @@ umb_rtrequest(struct ifnet *ifp, int req
  struct umb_softc *sc = ifp->if_softc;
 
  if (req == RTM_PROPOSAL) {
- umb_send_inet_proposal(sc);
+ umb_send_inet_proposal(sc, AF_INET);
+#ifdef INET6
+ umb_send_inet_proposal(sc, AF_INET6);
+#endif
  return;
  }
 
@@ -1596,11 +1611,6 @@ umb_decode_connect_info(struct umb_softc
  if (ifp->if_flags & IFF_DEBUG)
  log(LOG_INFO, "%s: connection %s\n", DEVNAM(sc),
     umb_activation(act));
- if ((ifp->if_flags & IFF_DEBUG) &&
-    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_DEFAULT &&
-    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_IPV4)
- log(LOG_DEBUG, "%s: got iptype %d connection\n",
-    DEVNAM(sc), letoh32(ci->iptype));
 
  sc->sc_info.activation = act;
  sc->sc_info.nwerror = letoh32(ci->nwerror);
@@ -1621,9 +1631,16 @@ umb_clear_addr(struct umb_softc *sc)
  struct ifnet *ifp = GET_IFP(sc);
 
  memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
- umb_send_inet_proposal(sc);
+ memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
+ umb_send_inet_proposal(sc, AF_INET);
+#ifdef INET6
+ umb_send_inet_proposal(sc, AF_INET6);
+#endif
  NET_LOCK();
  in_ifdetach(ifp);
+#ifdef INET6
+ in6_ifdetach(ifp);
+#endif
  NET_UNLOCK();
 }
 
@@ -1698,29 +1715,125 @@ umb_add_inet_config(struct umb_softc *sc
     sockaddr_ntop(sintosa(&ifra.ifra_dstaddr), str[2],
     sizeof(str[2])));
  }
- return rv;
+ return 0;
 }
 
+#ifdef INET6
+int
+umb_add_inet6_config(struct umb_softc *sc, struct in6_addr *ip, u_int prefixlen,
+    struct in6_addr *gw)
+{
+ struct ifnet *ifp = GET_IFP(sc);
+ struct in6_aliasreq ifra;
+ struct sockaddr_in6 *sin6, default_sin6;
+ struct rt_addrinfo info;
+ struct rtentry *rt;
+ int rv;
+
+ memset(&ifra, 0, sizeof (ifra));
+ sin6 = &ifra.ifra_addr;
+ sin6->sin6_family = AF_INET6;
+ sin6->sin6_len = sizeof (*sin6);
+ memcpy(&sin6->sin6_addr, ip, sizeof (sin6->sin6_addr));
+
+ sin6 = &ifra.ifra_dstaddr;
+ sin6->sin6_family = AF_INET6;
+ sin6->sin6_len = sizeof (*sin6);
+ memcpy(&sin6->sin6_addr, gw, sizeof (sin6->sin6_addr));
+
+ /* XXX: in6_update_ifa() accepts only 128 bits for P2P interfaces. */
+ prefixlen = 128;
+
+ sin6 = &ifra.ifra_prefixmask;
+ sin6->sin6_family = AF_INET6;
+ sin6->sin6_len = sizeof (*sin6);
+ in6_prefixlen2mask(&sin6->sin6_addr, prefixlen);
+
+ ifra.ifra_lifetime.ia6t_vltime = ND6_INFINITE_LIFETIME;
+ ifra.ifra_lifetime.ia6t_pltime = ND6_INFINITE_LIFETIME;
+
+ rv = in6_ioctl(SIOCAIFADDR_IN6, (caddr_t)&ifra, ifp, 1);
+ if (rv != 0) {
+ printf("%s: unable to set IPv6 address, error %d\n",
+    DEVNAM(ifp->if_softc), rv);
+ return rv;
+ }
+
+ memset(&default_sin6, 0, sizeof(default_sin6));
+ default_sin6.sin6_family = AF_INET6;
+ default_sin6.sin6_len = sizeof (default_sin6);
+
+ memset(&info, 0, sizeof(info));
+ info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
+ info.rti_ifa = ifa_ifwithaddr(sin6tosa(&ifra.ifra_addr),
+    ifp->if_rdomain);
+ info.rti_info[RTAX_DST] = sin6tosa(&default_sin6);
+ info.rti_info[RTAX_NETMASK] = sin6tosa(&default_sin6);
+ info.rti_info[RTAX_GATEWAY] = sin6tosa(&ifra.ifra_dstaddr);
+
+ NET_LOCK();
+ rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
+ NET_UNLOCK();
+ if (rv) {
+ printf("%s: unable to set IPv6 default route, "
+    "error %d\n", DEVNAM(ifp->if_softc), rv);
+ rtm_miss(RTM_MISS, &info, 0, RTP_NONE, 0, rv,
+    ifp->if_rdomain);
+ } else {
+ /* Inform listeners of the new route */
+ rtm_send(rt, RTM_ADD, rv, ifp->if_rdomain);
+ rtfree(rt);
+ }
+
+ if (ifp->if_flags & IFF_DEBUG) {
+ char str[3][INET6_ADDRSTRLEN];
+ log(LOG_INFO, "%s: IPv6 addr %s, mask %s, gateway %s\n",
+    DEVNAM(ifp->if_softc),
+    sockaddr_ntop(sin6tosa(&ifra.ifra_addr), str[0],
+    sizeof(str[0])),
+    sockaddr_ntop(sin6tosa(&ifra.ifra_prefixmask), str[1],
+    sizeof(str[1])),
+    sockaddr_ntop(sin6tosa(&ifra.ifra_dstaddr), str[2],
+    sizeof(str[2])));
+ }
+ return 0;
+}
+#endif
+
 void
-umb_send_inet_proposal(struct umb_softc *sc)
+umb_send_inet_proposal(struct umb_softc *sc, int af)
 {
  struct ifnet *ifp = GET_IFP(sc);
  struct sockaddr_rtdns rtdns;
  struct rt_addrinfo info;
  int i, flag = 0;
+ size_t sz = 0;
 
  memset(&rtdns, 0, sizeof(rtdns));
  memset(&info, 0, sizeof(info));
 
  for (i = 0; i < UMB_MAX_DNSSRV; i++) {
- if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
- break;
- memcpy(rtdns.sr_dns + i * sizeof(struct in_addr),
-    &sc->sc_info.ipv4dns[i], sizeof(struct in_addr));
- flag = RTF_UP;
+ if (af == AF_INET) {
+ sz = sizeof (sc->sc_info.ipv4dns[i]);
+ if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
+ break;
+ memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv4dns[i],
+    sz);
+ flag = RTF_UP;
+#ifdef INET6
+ } if (af == AF_INET6) {
+ sz = sizeof (sc->sc_info.ipv6dns[i]);
+ if (IN6_ARE_ADDR_EQUAL(&sc->sc_info.ipv6dns[i],
+    &in6addr_any))
+ break;
+ memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv6dns[i],
+    sz);
+ flag = RTF_UP;
+#endif
+ }
  }
- rtdns.sr_family = AF_INET;
- rtdns.sr_len = 2 + i * sizeof(struct in_addr);
+ rtdns.sr_family = af;
+ rtdns.sr_len = 2 + i * sz;
  info.rti_info[RTAX_DNS] = srtdnstosa(&rtdns);
 
  rtm_proposal(ifp, &info, flag, RTP_PROPOSAL_UMB);
@@ -1732,7 +1845,7 @@ umb_decode_ip_configuration(struct umb_s
  struct mbim_cid_ip_configuration_info *ic = data;
  struct ifnet *ifp = GET_IFP(sc);
  int s;
- uint32_t avail;
+ uint32_t avail_v4;
  uint32_t val;
  int n, i;
  int off;
@@ -1740,6 +1853,12 @@ umb_decode_ip_configuration(struct umb_s
  struct in_addr addr, gw;
  int state = -1;
  int rv;
+ int hasmtu = 0;
+#ifdef INET6
+ uint32_t avail_v6;
+ struct mbim_cid_ipv6_element ipv6elem;
+ struct in6_addr addr6, gw6;
+#endif
 
  if (len < sizeof (*ic))
  return 0;
@@ -1750,17 +1869,20 @@ umb_decode_ip_configuration(struct umb_s
  }
  s = splnet();
 
+ memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
+ memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
+
  /*
  * IPv4 configuation
  */
- avail = letoh32(ic->ipv4_available);
- if ((avail & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
+ avail_v4 = letoh32(ic->ipv4_available);
+ if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
     (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
  n = letoh32(ic->ipv4_naddr);
  off = letoh32(ic->ipv4_addroffs);
 
  if (n == 0 || off + sizeof (ipv4elem) > len)
- goto done;
+ goto tryv6;
  if (n != 1 && ifp->if_flags & IFF_DEBUG)
  log(LOG_INFO, "%s: more than one IPv4 addr: %d\n",
     DEVNAM(ifp->if_softc), n);
@@ -1780,12 +1902,12 @@ umb_decode_ip_configuration(struct umb_s
  }
 
  memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
- if (avail & MBIM_IPCONF_HAS_DNSINFO) {
+ if (avail_v4 & MBIM_IPCONF_HAS_DNSINFO) {
  n = letoh32(ic->ipv4_ndnssrv);
  off = letoh32(ic->ipv4_dnssrvoffs);
  i = 0;
  while (n-- > 0) {
- if (off + sizeof (uint32_t) > len)
+ if (off + sizeof (addr) > len)
  break;
  memcpy(&addr, data + off, sizeof(addr));
  if (i < UMB_MAX_DNSSRV)
@@ -1798,29 +1920,93 @@ umb_decode_ip_configuration(struct umb_s
     &addr, str, sizeof(str)));
  }
  }
- umb_send_inet_proposal(sc);
+ umb_send_inet_proposal(sc, AF_INET);
  }
-
- if ((avail & MBIM_IPCONF_HAS_MTUINFO)) {
+ if ((avail_v4 & MBIM_IPCONF_HAS_MTUINFO)) {
  val = letoh32(ic->ipv4_mtu);
  if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
+ hasmtu = 1;
  ifp->if_hardmtu = val;
  if (ifp->if_mtu > val)
  ifp->if_mtu = val;
- if (ifp->if_flags & IFF_DEBUG)
- log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), val);
  }
  }
 
- avail = letoh32(ic->ipv6_available);
- if ((ifp->if_flags & IFF_DEBUG) && avail & MBIM_IPCONF_HAS_ADDRINFO) {
- /* XXX FIXME: IPv6 configuation missing */
- log(LOG_INFO, "%s: ignoring IPv6 configuration\n", DEVNAM(sc));
+tryv6:;
+#ifdef INET6
+ /*
+ * IPv6 configuation
+ */
+ avail_v6 = letoh32(ic->ipv6_available);
+ if (avail_v6 == 0) {
+ if (ifp->if_flags & IFF_DEBUG)
+ log(LOG_INFO, "%s: ISP or WWAN module offers no IPv6 "
+    "support\n", DEVNAM(ifp->if_softc));
+ goto done;
+ }
+
+ if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
+    (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
+ n = letoh32(ic->ipv6_naddr);
+ off = letoh32(ic->ipv6_addroffs);
+
+ if (n == 0 || off + sizeof (ipv6elem) > len)
+ goto done;
+ if (n != 1 && ifp->if_flags & IFF_DEBUG)
+ log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
+    DEVNAM(ifp->if_softc), n);
+
+ /* Only pick the first one */
+ memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
+ memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
+
+ off = letoh32(ic->ipv6_gwoffs);
+ memcpy(&gw6, data + off, sizeof (gw6));
+
+ rv = umb_add_inet6_config(sc, &addr6, ipv6elem.prefixlen, &gw6);
+ if (rv == 0)
+ state = UMB_S_UP;
+ }
+
+ if (avail_v6 & MBIM_IPCONF_HAS_DNSINFO) {
+ n = letoh32(ic->ipv6_ndnssrv);
+ off = letoh32(ic->ipv6_dnssrvoffs);
+ i = 0;
+ while (n-- > 0) {
+ if (off + sizeof (addr6) > len)
+ break;
+ memcpy(&addr6, data + off, sizeof(addr6));
+ if (i < UMB_MAX_DNSSRV)
+ sc->sc_info.ipv6dns[i++] = addr6;
+ off += sizeof(addr6);
+ if (ifp->if_flags & IFF_DEBUG) {
+ char str[INET6_ADDRSTRLEN];
+ log(LOG_INFO, "%s: IPv6 nameserver %s\n",
+    DEVNAM(ifp->if_softc), inet_ntop(AF_INET6,
+    &addr6, str, sizeof(str)));
+ }
+ }
+ umb_send_inet_proposal(sc, AF_INET6);
+ }
+
+ if ((avail_v6 & MBIM_IPCONF_HAS_MTUINFO)) {
+ val = letoh32(ic->ipv6_mtu);
+ if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
+ hasmtu = 1;
+ ifp->if_hardmtu = val;
+ if (ifp->if_mtu > val)
+ ifp->if_mtu = val;
+ }
  }
+#endif
+
+done:
+ if (hasmtu && (ifp->if_flags & IFF_DEBUG))
+ log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), ifp->if_hardmtu);
+
  if (state != -1)
  umb_newstate(sc, state, 0);
 
-done:
  splx(s);
  return 1;
 }
@@ -2392,7 +2578,12 @@ umb_send_connect(struct umb_softc *sc, i
  c->passwd_size = htole32(0);
  c->authprot = htole32(MBIM_AUTHPROT_NONE);
  c->compression = htole32(MBIM_COMPRESSION_NONE);
+#ifdef INET6
+ /* XXX FIXME: support IPv6-only mode, too */
+ c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);
+#else
  c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
+#endif
  memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));
  umb_cmd(sc, MBIM_CID_CONNECT, MBIM_CMDOP_SET, c, off);
 done:
Index: sys/dev/usb/if_umb.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_umb.h,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 if_umb.h
--- sys/dev/usb/if_umb.h 26 Aug 2019 15:23:01 -0000 1.5
+++ sys/dev/usb/if_umb.h 23 Jan 2020 09:24:38 -0000
@@ -321,6 +321,7 @@ struct umb_info {
 
 #define UMB_MAX_DNSSRV 2
  struct in_addr ipv4dns[UMB_MAX_DNSSRV];
+ struct in6_addr ipv6dns[UMB_MAX_DNSSRV];
 };
 
 #ifdef _KERNEL
@@ -380,6 +381,6 @@ struct umb_softc {
 
 #define sc_state sc_info.state
 #define sc_roaming sc_info.enable_roaming
- struct umb_info sc_info;
+ struct umb_info sc_info;
 };
 #endif /* _KERNEL */

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Job Snijders-2
On Tue, Jan 28, 2020 at 03:03:47PM +0100, Gerhard Roth wrote:
> this patch adds IPv6 support to umb(4).

OK job@

Tested with 'telnet -6 towel.blinkenlights.nl' on Fibocom L831-EAU on
IIJ MIO's network (Japan), with 'inet6 autoconf' in /etc/hostname.umb0 :-)

job@vurt ~$ doas ifconfig umb0
umb0: flags=208851<UP,POINTOPOINT,RUNNING,SIMPLEX,MULTICAST,AUTOCONF6> mtu 1500
        index 5 priority 6 llprio 3
        roaming enabled registration home network
        state up cell-class LTE rssi -95dBm speed 47.7Mps up 143Mps down
        SIM initialized PIN valid (3 attempts left)
        subscriber-id 440030011055456 ICC-id 8981030000010455217 provider JP DOCOMO
        device L831-EAU-00 v1.0.0 IMEI 862727030473719 firmware L831_V2E.0C.00.07
        APN iijmio.jp
        dns 202.232.2.32 202.232.2.33 2001:240::32 2001:240::33
        groups: egress
        status: active
        inet6 fe80::fa59:71ff:fe9d:67dc%umb0 ->  prefixlen 64 scopeid 0x5
        inet 100.94.237.118 --> 100.94.237.1 netmask 0xffffff00
        inet6 fe80::15:9392:1801%umb0 -> fe80::15:9392:1802%umb0 prefixlen 128 scopeid 0x5
        inet6 2001:240:240c:9431:18e0:d5ed:c293:aafd ->  prefixlen 64 autoconf
        inet6 2001:240:240c:9431:15ac:21ce:8471:27e1 ->  prefixlen 64 autoconf autoconfprivacy pltime 82252 vltime 601067

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Gerhard Roth-2
In reply to this post by Gerhard Roth-2
So far I've got only one ok from job@ and he'd like me to commit this.

So I asking if there are any objections against this going into the base.


Gerhard


On Tue, 28 Jan 2020 15:03:47 +0100 Gerhard Roth <[hidden email]> wrote:

> Hi,
>
> this patch adds IPv6 support to umb(4).
>
> It will try to obtain a IPv6 address if the kernel is compiled with INET6.
> Currently there is no option to disable IPv6 on such a kernel (other than
> manually calling "ifconfig umb0 -inet6"). Nor is there a IPv6-only mode which
> refrains from obtaining an IPv4 address from the kernel.
>
> To get an IPv6 address, your provider has to offer one. But more importantly
> the firmware of your umb(4) device has to have IPv6 support. I stumbled
> across two older Sierra Wireless modules (EM8805 and MC3805) that refused
> to provide an IPv6 address.
>
> Have fun,
>
> Gerhard
>
>
> Index: sbin/ifconfig/ifconfig.c
> ===================================================================
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.417
> diff -u -p -u -p -r1.417 ifconfig.c
> --- sbin/ifconfig/ifconfig.c 27 Dec 2019 14:34:46 -0000 1.417
> +++ sbin/ifconfig/ifconfig.c 23 Jan 2020 09:24:38 -0000
> @@ -5666,6 +5666,7 @@ umb_status(void)
>   char apn[UMB_APN_MAXLEN+1];
>   char pn[UMB_PHONENR_MAXLEN+1];
>   int i, n;
> + char astr[INET6_ADDRSTRLEN];
>  
>   memset((char *)&mi, 0, sizeof(mi));
>   ifr.ifr_data = (caddr_t)&mi;
> @@ -5830,7 +5831,15 @@ umb_status(void)
>   for (i = 0, n = 0; i < UMB_MAX_DNSSRV; i++) {
>   if (mi.ipv4dns[i].s_addr == INADDR_ANY)
>   break;
> - printf("%s %s", n++ ? "" : "\tdns", inet_ntoa(mi.ipv4dns[i]));
> + printf("%s %s", n++ ? "" : "\tdns",
> +    inet_ntop(AF_INET, &mi.ipv4dns[i], astr, sizeof (astr)));
> + }
> + for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> + if (memcmp(&mi.ipv6dns[i], &in6addr_any,
> +    sizeof (mi.ipv6dns[i])) == 0)
> + break;
> + printf("%s %s", n++ ? "" : "\tdns",
> +    inet_ntop(AF_INET6, &mi.ipv6dns[i], astr, sizeof (astr)));
>   }
>   if (n)
>   printf("\n");
> Index: share/man/man4/umb.4
> ===================================================================
> RCS file: /cvs/src/share/man/man4/umb.4,v
> retrieving revision 1.9
> diff -u -p -u -p -r1.9 umb.4
> --- share/man/man4/umb.4 23 Nov 2017 20:47:26 -0000 1.9
> +++ share/man/man4/umb.4 28 Jan 2020 09:04:20 -0000
> @@ -40,6 +40,11 @@ will remain in this state until the MBIM
>  In case the device is connected to an "always-on" USB port,
>  it may be possible to connect to a provider without entering the
>  PIN again even if the system was rebooted.
> +.Pp
> +If the kernel has been compiled with INET6, the driver will try to
> +obtain an IPv6 address from the provider. To succeed with the IPv6
> +configuration, both the ISP and the MBIM device have to offer IPv6
> +support.
>  .Sh HARDWARE
>  The following devices should work:
>  .Pp
> @@ -64,10 +69,6 @@ The following devices should work:
>  .%U http://www.usb.org/developers/docs/devclass_docs/MBIM10Errata1_073013.zip
>  .Re
>  .Sh CAVEATS
> -The
> -.Nm
> -driver does not support IPv6.
> -.Pp
>  Devices which fail to provide a conforming MBIM implementation will
>  probably be attached as some other driver, such as
>  .Xr umsm 4 .
> Index: sys/dev/usb/if_umb.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.31
> diff -u -p -u -p -r1.31 if_umb.c
> --- sys/dev/usb/if_umb.c 26 Nov 2019 23:04:28 -0000 1.31
> +++ sys/dev/usb/if_umb.c 28 Jan 2020 09:08:16 -0000
> @@ -43,6 +43,14 @@
>  #include <netinet/in_var.h>
>  #include <netinet/ip.h>
>  
> +#ifdef INET6
> +#include <netinet/ip6.h>
> +#include <netinet6/in6_var.h>
> +#include <netinet6/ip6_var.h>
> +#include <netinet6/in6_ifattach.h>
> +#include <netinet6/nd6.h>
> +#endif
> +
>  #include <machine/bus.h>
>  
>  #include <dev/usb/usb.h>
> @@ -158,7 +166,11 @@ int umb_decode_connect_info(struct umb
>  void umb_clear_addr(struct umb_softc *);
>  int umb_add_inet_config(struct umb_softc *, struct in_addr, u_int,
>      struct in_addr);
> -void umb_send_inet_proposal(struct umb_softc *);
> +#ifdef INET6
> +int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
> +    u_int, struct in6_addr *);
> +#endif
> +void umb_send_inet_proposal(struct umb_softc *, int);
>  int umb_decode_ip_configuration(struct umb_softc *, void *, int);
>  void umb_rx(struct umb_softc *);
>  void umb_rxeof(struct usbd_xfer *, void *, usbd_status);
> @@ -800,8 +812,8 @@ umb_input(struct ifnet *ifp, struct mbuf
>  #endif /* INET6 */
>   default:
>   ifp->if_ierrors++;
> - DPRINTFN(4, "%s: dropping packet with bad IP version (%d)\n",
> -    __func__, ipv);
> + DPRINTFN(4, "%s: dropping packet with bad IP version (af %d)\n",
> +    __func__, af);
>   m_freem(m);
>   return 1;
>   }
> @@ -902,7 +914,10 @@ umb_rtrequest(struct ifnet *ifp, int req
>   struct umb_softc *sc = ifp->if_softc;
>  
>   if (req == RTM_PROPOSAL) {
> - umb_send_inet_proposal(sc);
> + umb_send_inet_proposal(sc, AF_INET);
> +#ifdef INET6
> + umb_send_inet_proposal(sc, AF_INET6);
> +#endif
>   return;
>   }
>  
> @@ -1596,11 +1611,6 @@ umb_decode_connect_info(struct umb_softc
>   if (ifp->if_flags & IFF_DEBUG)
>   log(LOG_INFO, "%s: connection %s\n", DEVNAM(sc),
>      umb_activation(act));
> - if ((ifp->if_flags & IFF_DEBUG) &&
> -    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_DEFAULT &&
> -    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_IPV4)
> - log(LOG_DEBUG, "%s: got iptype %d connection\n",
> -    DEVNAM(sc), letoh32(ci->iptype));
>  
>   sc->sc_info.activation = act;
>   sc->sc_info.nwerror = letoh32(ci->nwerror);
> @@ -1621,9 +1631,16 @@ umb_clear_addr(struct umb_softc *sc)
>   struct ifnet *ifp = GET_IFP(sc);
>  
>   memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> - umb_send_inet_proposal(sc);
> + memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
> + umb_send_inet_proposal(sc, AF_INET);
> +#ifdef INET6
> + umb_send_inet_proposal(sc, AF_INET6);
> +#endif
>   NET_LOCK();
>   in_ifdetach(ifp);
> +#ifdef INET6
> + in6_ifdetach(ifp);
> +#endif
>   NET_UNLOCK();
>  }
>  
> @@ -1698,29 +1715,125 @@ umb_add_inet_config(struct umb_softc *sc
>      sockaddr_ntop(sintosa(&ifra.ifra_dstaddr), str[2],
>      sizeof(str[2])));
>   }
> - return rv;
> + return 0;
>  }
>  
> +#ifdef INET6
> +int
> +umb_add_inet6_config(struct umb_softc *sc, struct in6_addr *ip, u_int prefixlen,
> +    struct in6_addr *gw)
> +{
> + struct ifnet *ifp = GET_IFP(sc);
> + struct in6_aliasreq ifra;
> + struct sockaddr_in6 *sin6, default_sin6;
> + struct rt_addrinfo info;
> + struct rtentry *rt;
> + int rv;
> +
> + memset(&ifra, 0, sizeof (ifra));
> + sin6 = &ifra.ifra_addr;
> + sin6->sin6_family = AF_INET6;
> + sin6->sin6_len = sizeof (*sin6);
> + memcpy(&sin6->sin6_addr, ip, sizeof (sin6->sin6_addr));
> +
> + sin6 = &ifra.ifra_dstaddr;
> + sin6->sin6_family = AF_INET6;
> + sin6->sin6_len = sizeof (*sin6);
> + memcpy(&sin6->sin6_addr, gw, sizeof (sin6->sin6_addr));
> +
> + /* XXX: in6_update_ifa() accepts only 128 bits for P2P interfaces. */
> + prefixlen = 128;
> +
> + sin6 = &ifra.ifra_prefixmask;
> + sin6->sin6_family = AF_INET6;
> + sin6->sin6_len = sizeof (*sin6);
> + in6_prefixlen2mask(&sin6->sin6_addr, prefixlen);
> +
> + ifra.ifra_lifetime.ia6t_vltime = ND6_INFINITE_LIFETIME;
> + ifra.ifra_lifetime.ia6t_pltime = ND6_INFINITE_LIFETIME;
> +
> + rv = in6_ioctl(SIOCAIFADDR_IN6, (caddr_t)&ifra, ifp, 1);
> + if (rv != 0) {
> + printf("%s: unable to set IPv6 address, error %d\n",
> +    DEVNAM(ifp->if_softc), rv);
> + return rv;
> + }
> +
> + memset(&default_sin6, 0, sizeof(default_sin6));
> + default_sin6.sin6_family = AF_INET6;
> + default_sin6.sin6_len = sizeof (default_sin6);
> +
> + memset(&info, 0, sizeof(info));
> + info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> + info.rti_ifa = ifa_ifwithaddr(sin6tosa(&ifra.ifra_addr),
> +    ifp->if_rdomain);
> + info.rti_info[RTAX_DST] = sin6tosa(&default_sin6);
> + info.rti_info[RTAX_NETMASK] = sin6tosa(&default_sin6);
> + info.rti_info[RTAX_GATEWAY] = sin6tosa(&ifra.ifra_dstaddr);
> +
> + NET_LOCK();
> + rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
> + NET_UNLOCK();
> + if (rv) {
> + printf("%s: unable to set IPv6 default route, "
> +    "error %d\n", DEVNAM(ifp->if_softc), rv);
> + rtm_miss(RTM_MISS, &info, 0, RTP_NONE, 0, rv,
> +    ifp->if_rdomain);
> + } else {
> + /* Inform listeners of the new route */
> + rtm_send(rt, RTM_ADD, rv, ifp->if_rdomain);
> + rtfree(rt);
> + }
> +
> + if (ifp->if_flags & IFF_DEBUG) {
> + char str[3][INET6_ADDRSTRLEN];
> + log(LOG_INFO, "%s: IPv6 addr %s, mask %s, gateway %s\n",
> +    DEVNAM(ifp->if_softc),
> +    sockaddr_ntop(sin6tosa(&ifra.ifra_addr), str[0],
> +    sizeof(str[0])),
> +    sockaddr_ntop(sin6tosa(&ifra.ifra_prefixmask), str[1],
> +    sizeof(str[1])),
> +    sockaddr_ntop(sin6tosa(&ifra.ifra_dstaddr), str[2],
> +    sizeof(str[2])));
> + }
> + return 0;
> +}
> +#endif
> +
>  void
> -umb_send_inet_proposal(struct umb_softc *sc)
> +umb_send_inet_proposal(struct umb_softc *sc, int af)
>  {
>   struct ifnet *ifp = GET_IFP(sc);
>   struct sockaddr_rtdns rtdns;
>   struct rt_addrinfo info;
>   int i, flag = 0;
> + size_t sz = 0;
>  
>   memset(&rtdns, 0, sizeof(rtdns));
>   memset(&info, 0, sizeof(info));
>  
>   for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> - if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
> - break;
> - memcpy(rtdns.sr_dns + i * sizeof(struct in_addr),
> -    &sc->sc_info.ipv4dns[i], sizeof(struct in_addr));
> - flag = RTF_UP;
> + if (af == AF_INET) {
> + sz = sizeof (sc->sc_info.ipv4dns[i]);
> + if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
> + break;
> + memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv4dns[i],
> +    sz);
> + flag = RTF_UP;
> +#ifdef INET6
> + } if (af == AF_INET6) {
> + sz = sizeof (sc->sc_info.ipv6dns[i]);
> + if (IN6_ARE_ADDR_EQUAL(&sc->sc_info.ipv6dns[i],
> +    &in6addr_any))
> + break;
> + memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv6dns[i],
> +    sz);
> + flag = RTF_UP;
> +#endif
> + }
>   }
> - rtdns.sr_family = AF_INET;
> - rtdns.sr_len = 2 + i * sizeof(struct in_addr);
> + rtdns.sr_family = af;
> + rtdns.sr_len = 2 + i * sz;
>   info.rti_info[RTAX_DNS] = srtdnstosa(&rtdns);
>  
>   rtm_proposal(ifp, &info, flag, RTP_PROPOSAL_UMB);
> @@ -1732,7 +1845,7 @@ umb_decode_ip_configuration(struct umb_s
>   struct mbim_cid_ip_configuration_info *ic = data;
>   struct ifnet *ifp = GET_IFP(sc);
>   int s;
> - uint32_t avail;
> + uint32_t avail_v4;
>   uint32_t val;
>   int n, i;
>   int off;
> @@ -1740,6 +1853,12 @@ umb_decode_ip_configuration(struct umb_s
>   struct in_addr addr, gw;
>   int state = -1;
>   int rv;
> + int hasmtu = 0;
> +#ifdef INET6
> + uint32_t avail_v6;
> + struct mbim_cid_ipv6_element ipv6elem;
> + struct in6_addr addr6, gw6;
> +#endif
>  
>   if (len < sizeof (*ic))
>   return 0;
> @@ -1750,17 +1869,20 @@ umb_decode_ip_configuration(struct umb_s
>   }
>   s = splnet();
>  
> + memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> + memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
> +
>   /*
>   * IPv4 configuation
>   */
> - avail = letoh32(ic->ipv4_available);
> - if ((avail & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> + avail_v4 = letoh32(ic->ipv4_available);
> + if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
>      (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
>   n = letoh32(ic->ipv4_naddr);
>   off = letoh32(ic->ipv4_addroffs);
>  
>   if (n == 0 || off + sizeof (ipv4elem) > len)
> - goto done;
> + goto tryv6;
>   if (n != 1 && ifp->if_flags & IFF_DEBUG)
>   log(LOG_INFO, "%s: more than one IPv4 addr: %d\n",
>      DEVNAM(ifp->if_softc), n);
> @@ -1780,12 +1902,12 @@ umb_decode_ip_configuration(struct umb_s
>   }
>  
>   memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> - if (avail & MBIM_IPCONF_HAS_DNSINFO) {
> + if (avail_v4 & MBIM_IPCONF_HAS_DNSINFO) {
>   n = letoh32(ic->ipv4_ndnssrv);
>   off = letoh32(ic->ipv4_dnssrvoffs);
>   i = 0;
>   while (n-- > 0) {
> - if (off + sizeof (uint32_t) > len)
> + if (off + sizeof (addr) > len)
>   break;
>   memcpy(&addr, data + off, sizeof(addr));
>   if (i < UMB_MAX_DNSSRV)
> @@ -1798,29 +1920,93 @@ umb_decode_ip_configuration(struct umb_s
>      &addr, str, sizeof(str)));
>   }
>   }
> - umb_send_inet_proposal(sc);
> + umb_send_inet_proposal(sc, AF_INET);
>   }
> -
> - if ((avail & MBIM_IPCONF_HAS_MTUINFO)) {
> + if ((avail_v4 & MBIM_IPCONF_HAS_MTUINFO)) {
>   val = letoh32(ic->ipv4_mtu);
>   if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
> + hasmtu = 1;
>   ifp->if_hardmtu = val;
>   if (ifp->if_mtu > val)
>   ifp->if_mtu = val;
> - if (ifp->if_flags & IFF_DEBUG)
> - log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), val);
>   }
>   }
>  
> - avail = letoh32(ic->ipv6_available);
> - if ((ifp->if_flags & IFF_DEBUG) && avail & MBIM_IPCONF_HAS_ADDRINFO) {
> - /* XXX FIXME: IPv6 configuation missing */
> - log(LOG_INFO, "%s: ignoring IPv6 configuration\n", DEVNAM(sc));
> +tryv6:;
> +#ifdef INET6
> + /*
> + * IPv6 configuation
> + */
> + avail_v6 = letoh32(ic->ipv6_available);
> + if (avail_v6 == 0) {
> + if (ifp->if_flags & IFF_DEBUG)
> + log(LOG_INFO, "%s: ISP or WWAN module offers no IPv6 "
> +    "support\n", DEVNAM(ifp->if_softc));
> + goto done;
> + }
> +
> + if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> +    (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
> + n = letoh32(ic->ipv6_naddr);
> + off = letoh32(ic->ipv6_addroffs);
> +
> + if (n == 0 || off + sizeof (ipv6elem) > len)
> + goto done;
> + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> +    DEVNAM(ifp->if_softc), n);
> +
> + /* Only pick the first one */
> + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> +
> + off = letoh32(ic->ipv6_gwoffs);
> + memcpy(&gw6, data + off, sizeof (gw6));
> +
> + rv = umb_add_inet6_config(sc, &addr6, ipv6elem.prefixlen, &gw6);
> + if (rv == 0)
> + state = UMB_S_UP;
> + }
> +
> + if (avail_v6 & MBIM_IPCONF_HAS_DNSINFO) {
> + n = letoh32(ic->ipv6_ndnssrv);
> + off = letoh32(ic->ipv6_dnssrvoffs);
> + i = 0;
> + while (n-- > 0) {
> + if (off + sizeof (addr6) > len)
> + break;
> + memcpy(&addr6, data + off, sizeof(addr6));
> + if (i < UMB_MAX_DNSSRV)
> + sc->sc_info.ipv6dns[i++] = addr6;
> + off += sizeof(addr6);
> + if (ifp->if_flags & IFF_DEBUG) {
> + char str[INET6_ADDRSTRLEN];
> + log(LOG_INFO, "%s: IPv6 nameserver %s\n",
> +    DEVNAM(ifp->if_softc), inet_ntop(AF_INET6,
> +    &addr6, str, sizeof(str)));
> + }
> + }
> + umb_send_inet_proposal(sc, AF_INET6);
> + }
> +
> + if ((avail_v6 & MBIM_IPCONF_HAS_MTUINFO)) {
> + val = letoh32(ic->ipv6_mtu);
> + if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
> + hasmtu = 1;
> + ifp->if_hardmtu = val;
> + if (ifp->if_mtu > val)
> + ifp->if_mtu = val;
> + }
>   }
> +#endif
> +
> +done:
> + if (hasmtu && (ifp->if_flags & IFF_DEBUG))
> + log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), ifp->if_hardmtu);
> +
>   if (state != -1)
>   umb_newstate(sc, state, 0);
>  
> -done:
>   splx(s);
>   return 1;
>  }
> @@ -2392,7 +2578,12 @@ umb_send_connect(struct umb_softc *sc, i
>   c->passwd_size = htole32(0);
>   c->authprot = htole32(MBIM_AUTHPROT_NONE);
>   c->compression = htole32(MBIM_COMPRESSION_NONE);
> +#ifdef INET6
> + /* XXX FIXME: support IPv6-only mode, too */
> + c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);
> +#else
>   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
> +#endif
>   memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));
>   umb_cmd(sc, MBIM_CID_CONNECT, MBIM_CMDOP_SET, c, off);
>  done:
> Index: sys/dev/usb/if_umb.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.h,v
> retrieving revision 1.5
> diff -u -p -u -p -r1.5 if_umb.h
> --- sys/dev/usb/if_umb.h 26 Aug 2019 15:23:01 -0000 1.5
> +++ sys/dev/usb/if_umb.h 23 Jan 2020 09:24:38 -0000
> @@ -321,6 +321,7 @@ struct umb_info {
>  
>  #define UMB_MAX_DNSSRV 2
>   struct in_addr ipv4dns[UMB_MAX_DNSSRV];
> + struct in6_addr ipv6dns[UMB_MAX_DNSSRV];
>  };
>  
>  #ifdef _KERNEL
> @@ -380,6 +381,6 @@ struct umb_softc {
>  
>  #define sc_state sc_info.state
>  #define sc_roaming sc_info.enable_roaming
> - struct umb_info sc_info;
> + struct umb_info sc_info;
>  };
>  #endif /* _KERNEL */

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Alexander Bluhm
In reply to this post by Gerhard Roth-2
On Tue, Jan 28, 2020 at 03:03:47PM +0100, Gerhard Roth wrote:
> this patch adds IPv6 support to umb(4).

It breaks my IPv4 setup.

umb0 at uhub0 port 4 configuration 1 interface 6 "Lenovo H5321 gw" rev 2.00/0.00 addr 2
provider Vodafone.de
firmware CXP 901 8700/1 - R3C18

When I apply the diff, my umb device does not get an IPv4 address.

umb0: state going up from 'open' to 'radio on'
umb0: none state unlocked (-1 attempts left)
umb0: set/qry MBIM_CID_SUBSCRIBER_READY_STATUS failed: BUSY
umb0: packet service changed from detached to attaching, class none, speed: 0 up / 0 down
umb0: SIM initialized
umb0: state going up from 'radio on' to 'SIM is ready'
umb0: packet service changed from attaching to attached, class HSPA, speed: 5760000 up / 14400000 down
umb0: state going up from 'SIM is ready' to 'attached'
umb0: connecting ...
umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
umb0: state change timeout
umb0: connecting ...
umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
umb0: state change timeout
umb0: connecting ...
umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
umb0: state change timeout
...

A few comments inline.

> +#ifdef INET6
> +int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
> +    u_int, struct in6_addr *);
> +#endif

Usually I avoid #ifdef for prototypes.  It does not matter whether
the compiler reads them and without #ifdef the code is nicer.

> +tryv6:;

The ; is wrong.

> + if (n == 0 || off + sizeof (ipv6elem) > len)
> + goto done;
> + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> +    DEVNAM(ifp->if_softc), n);
> +
> + /* Only pick the first one */
> + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> +
> + off = letoh32(ic->ipv6_gwoffs);
> + memcpy(&gw6, data + off, sizeof (gw6));

I think we should check the data length like above.

                if (off + sizeof (gw6) > len)
                        goto done;

And IPv4 should get the same check.

> @@ -380,6 +381,6 @@ struct umb_softc {
>
>  #define sc_state sc_info.state
>  #define sc_roaming sc_info.enable_roaming
> - struct umb_info sc_info;
> + struct umb_info sc_info;
>  };
>  #endif /* _KERNEL */

This whitespace chunk is wrong.

bluhm

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Gerhard Roth-2
Hi Alex,

thanks for looking into it.


On Tue, 4 Feb 2020 00:20:42 +0100 Alexander Bluhm <[hidden email]> wrote:

> On Tue, Jan 28, 2020 at 03:03:47PM +0100, Gerhard Roth wrote:
> > this patch adds IPv6 support to umb(4).  
>
> It breaks my IPv4 setup.
>
> umb0 at uhub0 port 4 configuration 1 interface 6 "Lenovo H5321 gw" rev 2.00/0.00 addr 2
> provider Vodafone.de
> firmware CXP 901 8700/1 - R3C18
>
> When I apply the diff, my umb device does not get an IPv4 address.
>
> umb0: state going up from 'open' to 'radio on'
> umb0: none state unlocked (-1 attempts left)
> umb0: set/qry MBIM_CID_SUBSCRIBER_READY_STATUS failed: BUSY
> umb0: packet service changed from detached to attaching, class none, speed: 0 up / 0 down
> umb0: SIM initialized
> umb0: state going up from 'radio on' to 'SIM is ready'
> umb0: packet service changed from attaching to attached, class HSPA, speed: 5760000 up / 14400000 down
> umb0: state going up from 'SIM is ready' to 'attached'
> umb0: connecting ...
> umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT

That looks like the firmware of this Lenovo H5321 doesn't support IPv6.
Well at least it gives a decent error code.


> umb0: state change timeout
> umb0: connecting ...
> umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> umb0: state change timeout
> umb0: connecting ...
> umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> umb0: state change timeout
> ...
>
> A few comments inline.
>
> > +#ifdef INET6
> > +int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
> > +    u_int, struct in6_addr *);
> > +#endif  
>
> Usually I avoid #ifdef for prototypes.  It does not matter whether
> the compiler reads them and without #ifdef the code is nicer.

Removed it.


>
> > +tryv6:;  
>
> The ; is wrong.

Not really, because the label 'tryv6' is immediately followed by
an "#ifdef INET6". So looking just at this label I cannot directly tell
whether there is code that follows or not. And a label with no code
following is a syntax error in C.

I just followed a old "C Style and Coding Standards for SunOS" paper
by Bill Shannon from 1993 that says:

        If a label is not followed by a program statement (e.g.
        if the next token is a closing brace( } )) a NULL
        statement ( ; ) must follow the label.


>
> > + if (n == 0 || off + sizeof (ipv6elem) > len)
> > + goto done;
> > + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> > + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> > +    DEVNAM(ifp->if_softc), n);
> > +
> > + /* Only pick the first one */
> > + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> > + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> > +
> > + off = letoh32(ic->ipv6_gwoffs);
> > + memcpy(&gw6, data + off, sizeof (gw6));  
>
> I think we should check the data length like above.
>
> if (off + sizeof (gw6) > len)
> goto done;
>
> And IPv4 should get the same check.

Thanks for spotting that. Added check to both cases.


>
> > @@ -380,6 +381,6 @@ struct umb_softc {
> >
> >  #define sc_state sc_info.state
> >  #define sc_roaming sc_info.enable_roaming
> > - struct umb_info sc_info;
> > + struct umb_info sc_info;
> >  };
> >  #endif /* _KERNEL */  
>
> This whitespace chunk is wrong.

I think it was wrong before. It's common idiom to add an extra space
to non-pointer members to keep the member names aligned, e.g.

        struct foo {
                int *abc;
                int def;
                int *bar;
        };

Please correct me if I'm wrong.

>
> bluhm


The updated patch below introduces a UMBFLG_NO_INET6 which is set on
receipt of a MBIM_STATUS_NO_DEVICE_SUPPORT in response to a
MBIM_CID_CONNECT. The code will then retry the connect operation in
IPv4-only mode.

That won't give you any IPv6 support, but at least it won't break
your setup.


Gerhard


Index: sbin/ifconfig/ifconfig.c
===================================================================
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.417
diff -u -p -u -p -r1.417 ifconfig.c
--- sbin/ifconfig/ifconfig.c 27 Dec 2019 14:34:46 -0000 1.417
+++ sbin/ifconfig/ifconfig.c 28 Jan 2020 12:16:23 -0000
@@ -5666,6 +5666,7 @@ umb_status(void)
  char apn[UMB_APN_MAXLEN+1];
  char pn[UMB_PHONENR_MAXLEN+1];
  int i, n;
+ char astr[INET6_ADDRSTRLEN];
 
  memset((char *)&mi, 0, sizeof(mi));
  ifr.ifr_data = (caddr_t)&mi;
@@ -5830,7 +5831,15 @@ umb_status(void)
  for (i = 0, n = 0; i < UMB_MAX_DNSSRV; i++) {
  if (mi.ipv4dns[i].s_addr == INADDR_ANY)
  break;
- printf("%s %s", n++ ? "" : "\tdns", inet_ntoa(mi.ipv4dns[i]));
+ printf("%s %s", n++ ? "" : "\tdns",
+    inet_ntop(AF_INET, &mi.ipv4dns[i], astr, sizeof (astr)));
+ }
+ for (i = 0; i < UMB_MAX_DNSSRV; i++) {
+ if (memcmp(&mi.ipv6dns[i], &in6addr_any,
+    sizeof (mi.ipv6dns[i])) == 0)
+ break;
+ printf("%s %s", n++ ? "" : "\tdns",
+    inet_ntop(AF_INET6, &mi.ipv6dns[i], astr, sizeof (astr)));
  }
  if (n)
  printf("\n");
Index: share/man/man4/umb.4
===================================================================
RCS file: /cvs/src/share/man/man4/umb.4,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 umb.4
--- share/man/man4/umb.4 23 Nov 2017 20:47:26 -0000 1.9
+++ share/man/man4/umb.4 28 Jan 2020 12:16:23 -0000
@@ -40,6 +40,11 @@ will remain in this state until the MBIM
 In case the device is connected to an "always-on" USB port,
 it may be possible to connect to a provider without entering the
 PIN again even if the system was rebooted.
+.Pp
+If the kernel has been compiled with INET6, the driver will try to
+obtain an IPv6 address from the provider. To succeed with the IPv6
+configuration, both the ISP and the MBIM device have to offer IPv6
+support.
 .Sh HARDWARE
 The following devices should work:
 .Pp
@@ -64,10 +69,6 @@ The following devices should work:
 .%U http://www.usb.org/developers/docs/devclass_docs/MBIM10Errata1_073013.zip
 .Re
 .Sh CAVEATS
-The
-.Nm
-driver does not support IPv6.
-.Pp
 Devices which fail to provide a conforming MBIM implementation will
 probably be attached as some other driver, such as
 .Xr umsm 4 .
Index: sys/dev/usb/if_umb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.31
diff -u -p -u -p -r1.31 if_umb.c
--- sys/dev/usb/if_umb.c 26 Nov 2019 23:04:28 -0000 1.31
+++ sys/dev/usb/if_umb.c 4 Feb 2020 07:50:30 -0000
@@ -43,6 +43,14 @@
 #include <netinet/in_var.h>
 #include <netinet/ip.h>
 
+#ifdef INET6
+#include <netinet/ip6.h>
+#include <netinet6/in6_var.h>
+#include <netinet6/ip6_var.h>
+#include <netinet6/in6_ifattach.h>
+#include <netinet6/nd6.h>
+#endif
+
 #include <machine/bus.h>
 
 #include <dev/usb/usb.h>
@@ -158,7 +166,9 @@ int umb_decode_connect_info(struct umb
 void umb_clear_addr(struct umb_softc *);
 int umb_add_inet_config(struct umb_softc *, struct in_addr, u_int,
     struct in_addr);
-void umb_send_inet_proposal(struct umb_softc *);
+int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
+    u_int, struct in6_addr *);
+void umb_send_inet_proposal(struct umb_softc *, int);
 int umb_decode_ip_configuration(struct umb_softc *, void *, int);
 void umb_rx(struct umb_softc *);
 void umb_rxeof(struct usbd_xfer *, void *, usbd_status);
@@ -800,8 +810,8 @@ umb_input(struct ifnet *ifp, struct mbuf
 #endif /* INET6 */
  default:
  ifp->if_ierrors++;
- DPRINTFN(4, "%s: dropping packet with bad IP version (%d)\n",
-    __func__, ipv);
+ DPRINTFN(4, "%s: dropping packet with bad IP version (af %d)\n",
+    __func__, af);
  m_freem(m);
  return 1;
  }
@@ -902,7 +912,10 @@ umb_rtrequest(struct ifnet *ifp, int req
  struct umb_softc *sc = ifp->if_softc;
 
  if (req == RTM_PROPOSAL) {
- umb_send_inet_proposal(sc);
+ umb_send_inet_proposal(sc, AF_INET);
+#ifdef INET6
+ umb_send_inet_proposal(sc, AF_INET6);
+#endif
  return;
  }
 
@@ -1596,11 +1609,6 @@ umb_decode_connect_info(struct umb_softc
  if (ifp->if_flags & IFF_DEBUG)
  log(LOG_INFO, "%s: connection %s\n", DEVNAM(sc),
     umb_activation(act));
- if ((ifp->if_flags & IFF_DEBUG) &&
-    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_DEFAULT &&
-    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_IPV4)
- log(LOG_DEBUG, "%s: got iptype %d connection\n",
-    DEVNAM(sc), letoh32(ci->iptype));
 
  sc->sc_info.activation = act;
  sc->sc_info.nwerror = letoh32(ci->nwerror);
@@ -1621,9 +1629,16 @@ umb_clear_addr(struct umb_softc *sc)
  struct ifnet *ifp = GET_IFP(sc);
 
  memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
- umb_send_inet_proposal(sc);
+ memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
+ umb_send_inet_proposal(sc, AF_INET);
+#ifdef INET6
+ umb_send_inet_proposal(sc, AF_INET6);
+#endif
  NET_LOCK();
  in_ifdetach(ifp);
+#ifdef INET6
+ in6_ifdetach(ifp);
+#endif
  NET_UNLOCK();
 }
 
@@ -1698,29 +1713,125 @@ umb_add_inet_config(struct umb_softc *sc
     sockaddr_ntop(sintosa(&ifra.ifra_dstaddr), str[2],
     sizeof(str[2])));
  }
- return rv;
+ return 0;
+}
+
+#ifdef INET6
+int
+umb_add_inet6_config(struct umb_softc *sc, struct in6_addr *ip, u_int prefixlen,
+    struct in6_addr *gw)
+{
+ struct ifnet *ifp = GET_IFP(sc);
+ struct in6_aliasreq ifra;
+ struct sockaddr_in6 *sin6, default_sin6;
+ struct rt_addrinfo info;
+ struct rtentry *rt;
+ int rv;
+
+ memset(&ifra, 0, sizeof (ifra));
+ sin6 = &ifra.ifra_addr;
+ sin6->sin6_family = AF_INET6;
+ sin6->sin6_len = sizeof (*sin6);
+ memcpy(&sin6->sin6_addr, ip, sizeof (sin6->sin6_addr));
+
+ sin6 = &ifra.ifra_dstaddr;
+ sin6->sin6_family = AF_INET6;
+ sin6->sin6_len = sizeof (*sin6);
+ memcpy(&sin6->sin6_addr, gw, sizeof (sin6->sin6_addr));
+
+ /* XXX: in6_update_ifa() accepts only 128 bits for P2P interfaces. */
+ prefixlen = 128;
+
+ sin6 = &ifra.ifra_prefixmask;
+ sin6->sin6_family = AF_INET6;
+ sin6->sin6_len = sizeof (*sin6);
+ in6_prefixlen2mask(&sin6->sin6_addr, prefixlen);
+
+ ifra.ifra_lifetime.ia6t_vltime = ND6_INFINITE_LIFETIME;
+ ifra.ifra_lifetime.ia6t_pltime = ND6_INFINITE_LIFETIME;
+
+ rv = in6_ioctl(SIOCAIFADDR_IN6, (caddr_t)&ifra, ifp, 1);
+ if (rv != 0) {
+ printf("%s: unable to set IPv6 address, error %d\n",
+    DEVNAM(ifp->if_softc), rv);
+ return rv;
+ }
+
+ memset(&default_sin6, 0, sizeof(default_sin6));
+ default_sin6.sin6_family = AF_INET6;
+ default_sin6.sin6_len = sizeof (default_sin6);
+
+ memset(&info, 0, sizeof(info));
+ info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
+ info.rti_ifa = ifa_ifwithaddr(sin6tosa(&ifra.ifra_addr),
+    ifp->if_rdomain);
+ info.rti_info[RTAX_DST] = sin6tosa(&default_sin6);
+ info.rti_info[RTAX_NETMASK] = sin6tosa(&default_sin6);
+ info.rti_info[RTAX_GATEWAY] = sin6tosa(&ifra.ifra_dstaddr);
+
+ NET_LOCK();
+ rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
+ NET_UNLOCK();
+ if (rv) {
+ printf("%s: unable to set IPv6 default route, "
+    "error %d\n", DEVNAM(ifp->if_softc), rv);
+ rtm_miss(RTM_MISS, &info, 0, RTP_NONE, 0, rv,
+    ifp->if_rdomain);
+ } else {
+ /* Inform listeners of the new route */
+ rtm_send(rt, RTM_ADD, rv, ifp->if_rdomain);
+ rtfree(rt);
+ }
+
+ if (ifp->if_flags & IFF_DEBUG) {
+ char str[3][INET6_ADDRSTRLEN];
+ log(LOG_INFO, "%s: IPv6 addr %s, mask %s, gateway %s\n",
+    DEVNAM(ifp->if_softc),
+    sockaddr_ntop(sin6tosa(&ifra.ifra_addr), str[0],
+    sizeof(str[0])),
+    sockaddr_ntop(sin6tosa(&ifra.ifra_prefixmask), str[1],
+    sizeof(str[1])),
+    sockaddr_ntop(sin6tosa(&ifra.ifra_dstaddr), str[2],
+    sizeof(str[2])));
+ }
+ return 0;
 }
+#endif
 
 void
-umb_send_inet_proposal(struct umb_softc *sc)
+umb_send_inet_proposal(struct umb_softc *sc, int af)
 {
  struct ifnet *ifp = GET_IFP(sc);
  struct sockaddr_rtdns rtdns;
  struct rt_addrinfo info;
  int i, flag = 0;
+ size_t sz = 0;
 
  memset(&rtdns, 0, sizeof(rtdns));
  memset(&info, 0, sizeof(info));
 
  for (i = 0; i < UMB_MAX_DNSSRV; i++) {
- if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
- break;
- memcpy(rtdns.sr_dns + i * sizeof(struct in_addr),
-    &sc->sc_info.ipv4dns[i], sizeof(struct in_addr));
- flag = RTF_UP;
+ if (af == AF_INET) {
+ sz = sizeof (sc->sc_info.ipv4dns[i]);
+ if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
+ break;
+ memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv4dns[i],
+    sz);
+ flag = RTF_UP;
+#ifdef INET6
+ } if (af == AF_INET6) {
+ sz = sizeof (sc->sc_info.ipv6dns[i]);
+ if (IN6_ARE_ADDR_EQUAL(&sc->sc_info.ipv6dns[i],
+    &in6addr_any))
+ break;
+ memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv6dns[i],
+    sz);
+ flag = RTF_UP;
+#endif
+ }
  }
- rtdns.sr_family = AF_INET;
- rtdns.sr_len = 2 + i * sizeof(struct in_addr);
+ rtdns.sr_family = af;
+ rtdns.sr_len = 2 + i * sz;
  info.rti_info[RTAX_DNS] = srtdnstosa(&rtdns);
 
  rtm_proposal(ifp, &info, flag, RTP_PROPOSAL_UMB);
@@ -1732,7 +1843,7 @@ umb_decode_ip_configuration(struct umb_s
  struct mbim_cid_ip_configuration_info *ic = data;
  struct ifnet *ifp = GET_IFP(sc);
  int s;
- uint32_t avail;
+ uint32_t avail_v4;
  uint32_t val;
  int n, i;
  int off;
@@ -1740,6 +1851,12 @@ umb_decode_ip_configuration(struct umb_s
  struct in_addr addr, gw;
  int state = -1;
  int rv;
+ int hasmtu = 0;
+#ifdef INET6
+ uint32_t avail_v6;
+ struct mbim_cid_ipv6_element ipv6elem;
+ struct in6_addr addr6, gw6;
+#endif
 
  if (len < sizeof (*ic))
  return 0;
@@ -1750,17 +1867,20 @@ umb_decode_ip_configuration(struct umb_s
  }
  s = splnet();
 
+ memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
+ memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
+
  /*
  * IPv4 configuation
  */
- avail = letoh32(ic->ipv4_available);
- if ((avail & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
+ avail_v4 = letoh32(ic->ipv4_available);
+ if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
     (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
  n = letoh32(ic->ipv4_naddr);
  off = letoh32(ic->ipv4_addroffs);
 
  if (n == 0 || off + sizeof (ipv4elem) > len)
- goto done;
+ goto tryv6;
  if (n != 1 && ifp->if_flags & IFF_DEBUG)
  log(LOG_INFO, "%s: more than one IPv4 addr: %d\n",
     DEVNAM(ifp->if_softc), n);
@@ -1771,6 +1891,8 @@ umb_decode_ip_configuration(struct umb_s
  addr.s_addr = ipv4elem.addr;
 
  off = letoh32(ic->ipv4_gwoffs);
+ if (off + sizeof (gw) > len)
+ goto done;
  memcpy(&gw, data + off, sizeof(gw));
 
  rv = umb_add_inet_config(sc, addr, ipv4elem.prefixlen, gw);
@@ -1780,12 +1902,12 @@ umb_decode_ip_configuration(struct umb_s
  }
 
  memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
- if (avail & MBIM_IPCONF_HAS_DNSINFO) {
+ if (avail_v4 & MBIM_IPCONF_HAS_DNSINFO) {
  n = letoh32(ic->ipv4_ndnssrv);
  off = letoh32(ic->ipv4_dnssrvoffs);
  i = 0;
  while (n-- > 0) {
- if (off + sizeof (uint32_t) > len)
+ if (off + sizeof (addr) > len)
  break;
  memcpy(&addr, data + off, sizeof(addr));
  if (i < UMB_MAX_DNSSRV)
@@ -1798,29 +1920,95 @@ umb_decode_ip_configuration(struct umb_s
     &addr, str, sizeof(str)));
  }
  }
- umb_send_inet_proposal(sc);
+ umb_send_inet_proposal(sc, AF_INET);
  }
-
- if ((avail & MBIM_IPCONF_HAS_MTUINFO)) {
+ if ((avail_v4 & MBIM_IPCONF_HAS_MTUINFO)) {
  val = letoh32(ic->ipv4_mtu);
  if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
+ hasmtu = 1;
  ifp->if_hardmtu = val;
  if (ifp->if_mtu > val)
  ifp->if_mtu = val;
- if (ifp->if_flags & IFF_DEBUG)
- log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), val);
  }
  }
 
- avail = letoh32(ic->ipv6_available);
- if ((ifp->if_flags & IFF_DEBUG) && avail & MBIM_IPCONF_HAS_ADDRINFO) {
- /* XXX FIXME: IPv6 configuation missing */
- log(LOG_INFO, "%s: ignoring IPv6 configuration\n", DEVNAM(sc));
+tryv6:;
+#ifdef INET6
+ /*
+ * IPv6 configuation
+ */
+ avail_v6 = letoh32(ic->ipv6_available);
+ if (avail_v6 == 0) {
+ if (ifp->if_flags & IFF_DEBUG)
+ log(LOG_INFO, "%s: ISP or WWAN module offers no IPv6 "
+    "support\n", DEVNAM(ifp->if_softc));
+ goto done;
+ }
+
+ if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
+    (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
+ n = letoh32(ic->ipv6_naddr);
+ off = letoh32(ic->ipv6_addroffs);
+
+ if (n == 0 || off + sizeof (ipv6elem) > len)
+ goto done;
+ if (n != 1 && ifp->if_flags & IFF_DEBUG)
+ log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
+    DEVNAM(ifp->if_softc), n);
+
+ /* Only pick the first one */
+ memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
+ memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
+
+ off = letoh32(ic->ipv6_gwoffs);
+ if (off + sizeof (gw6) > len)
+ goto done;
+ memcpy(&gw6, data + off, sizeof (gw6));
+
+ rv = umb_add_inet6_config(sc, &addr6, ipv6elem.prefixlen, &gw6);
+ if (rv == 0)
+ state = UMB_S_UP;
+ }
+
+ if (avail_v6 & MBIM_IPCONF_HAS_DNSINFO) {
+ n = letoh32(ic->ipv6_ndnssrv);
+ off = letoh32(ic->ipv6_dnssrvoffs);
+ i = 0;
+ while (n-- > 0) {
+ if (off + sizeof (addr6) > len)
+ break;
+ memcpy(&addr6, data + off, sizeof(addr6));
+ if (i < UMB_MAX_DNSSRV)
+ sc->sc_info.ipv6dns[i++] = addr6;
+ off += sizeof(addr6);
+ if (ifp->if_flags & IFF_DEBUG) {
+ char str[INET6_ADDRSTRLEN];
+ log(LOG_INFO, "%s: IPv6 nameserver %s\n",
+    DEVNAM(ifp->if_softc), inet_ntop(AF_INET6,
+    &addr6, str, sizeof(str)));
+ }
+ }
+ umb_send_inet_proposal(sc, AF_INET6);
+ }
+
+ if ((avail_v6 & MBIM_IPCONF_HAS_MTUINFO)) {
+ val = letoh32(ic->ipv6_mtu);
+ if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
+ hasmtu = 1;
+ ifp->if_hardmtu = val;
+ if (ifp->if_mtu > val)
+ ifp->if_mtu = val;
+ }
  }
+#endif
+
+done:
+ if (hasmtu && (ifp->if_flags & IFF_DEBUG))
+ log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), ifp->if_hardmtu);
+
  if (state != -1)
  umb_newstate(sc, state, 0);
 
-done:
  splx(s);
  return 1;
 }
@@ -2393,6 +2581,11 @@ umb_send_connect(struct umb_softc *sc, i
  c->authprot = htole32(MBIM_AUTHPROT_NONE);
  c->compression = htole32(MBIM_COMPRESSION_NONE);
  c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
+#ifdef INET6
+ /* XXX FIXME: support IPv6-only mode, too */
+ if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
+ c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);
+#endif
  memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));
  umb_cmd(sc, MBIM_CID_CONNECT, MBIM_CMDOP_SET, c, off);
 done:
@@ -2476,6 +2669,20 @@ umb_command_done(struct umb_softc *sc, v
  switch (status) {
  case MBIM_STATUS_SUCCESS:
  break;
+#ifdef INET6
+ case MBIM_STATUS_NO_DEVICE_SUPPORT:
+ if ((cid == MBIM_CID_CONNECT) &&
+    (sc->sc_flags & UMBFLG_NO_INET6) == 0) {
+ sc->sc_flags |= UMBFLG_NO_INET6;
+ if (ifp->if_flags & IFF_DEBUG)
+ log(LOG_ERR,
+    "%s: device does not support IPv6\n",
+    DEVNAM(sc));
+ }
+ /* Re-trigger the connect, this time IPv4 only */
+ usb_add_task(sc->sc_udev, &sc->sc_umb_task);
+ return;
+#endif
  case MBIM_STATUS_NOT_INITIALIZED:
  if (ifp->if_flags & IFF_DEBUG)
  log(LOG_ERR, "%s: SIM not initialized (PIN missing)\n",
Index: sys/dev/usb/if_umb.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_umb.h,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 if_umb.h
--- sys/dev/usb/if_umb.h 26 Aug 2019 15:23:01 -0000 1.5
+++ sys/dev/usb/if_umb.h 4 Feb 2020 07:30:43 -0000
@@ -321,6 +321,7 @@ struct umb_info {
 
 #define UMB_MAX_DNSSRV 2
  struct in_addr ipv4dns[UMB_MAX_DNSSRV];
+ struct in6_addr ipv6dns[UMB_MAX_DNSSRV];
 };
 
 #ifdef _KERNEL
@@ -345,6 +346,7 @@ struct umb_softc {
  int sc_ndp_remainder;
 
 #define UMBFLG_FCC_AUTH_REQUIRED 0x0001
+#define UMBFLG_NO_INET6 0x0002
  uint32_t sc_flags;
  int sc_cid;
 
@@ -380,6 +382,6 @@ struct umb_softc {
 
 #define sc_state sc_info.state
 #define sc_roaming sc_info.enable_roaming
- struct umb_info sc_info;
+ struct umb_info sc_info;
 };
 #endif /* _KERNEL */

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Alexander Bluhm
On Tue, Feb 04, 2020 at 09:16:34AM +0100, Gerhard Roth wrote:
> The updated patch below introduces a UMBFLG_NO_INET6 which is set on
> receipt of a MBIM_STATUS_NO_DEVICE_SUPPORT in response to a
> MBIM_CID_CONNECT. The code will then retry the connect operation in
> IPv4-only mode.
>
> That won't give you any IPv6 support, but at least it won't break
> your setup.

OK bluhm@

Now it works:

umb0: state going up from 'open' to 'radio on'
umb0: packet service changed from unknown to detached, class none, speed: 0 up / 0 down
umb0: none state unlocked (-1 attempts left)
umb0: set/qry MBIM_CID_SUBSCRIBER_READY_STATUS failed: BUSY
umb0: SIM initialized
umb0: state going up from 'radio on' to 'SIM is ready'
umb0: set/qry MBIM_CID_PACKET_SERVICE failed: FAILURE
umb0: packet service changed from detached to attaching, class none, speed: 0 up / 0 down
umb0: packet service changed from attaching to attached, class HSPA, speed: 5760000 up / 14400000 down
umb0: state going up from 'SIM is ready' to 'attached'
umb0: connecting ...
umb0: device does not support IPv6
umb0: connecting ...
umb0: connection activating
umb0: network connected
umb0: connection activated
umb0: state going up from 'attached' to 'connected'
umb0: IPv4 addr 100.67.244.231, mask 255.255.255.240, gateway 100.67.244.226
umb0: IPv4 nameserver 139.7.30.126
umb0: IPv4 nameserver 139.7.30.125
umb0: ISP or WWAN module offers no IPv6 support
umb0: MTU 1500
umb0: state going up from 'connected' to 'up'
umb0: link state changed from down to up
umb0: packet service attached, class custom, speed: 5760000 up / 21000000 down
umb0: unable to set IPv4 default route, error 17
umb0: IPv4 addr 100.67.244.231, mask 255.255.255.240, gateway 100.67.244.226
umb0: IPv4 nameserver 139.7.30.126
umb0: IPv4 nameserver 139.7.30.125
umb0: ISP or WWAN module offers no IPv6 support

> Index: sbin/ifconfig/ifconfig.c
> ===================================================================
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.417
> diff -u -p -u -p -r1.417 ifconfig.c
> --- sbin/ifconfig/ifconfig.c 27 Dec 2019 14:34:46 -0000 1.417
> +++ sbin/ifconfig/ifconfig.c 28 Jan 2020 12:16:23 -0000
> @@ -5666,6 +5666,7 @@ umb_status(void)
>   char apn[UMB_APN_MAXLEN+1];
>   char pn[UMB_PHONENR_MAXLEN+1];
>   int i, n;
> + char astr[INET6_ADDRSTRLEN];
>
>   memset((char *)&mi, 0, sizeof(mi));
>   ifr.ifr_data = (caddr_t)&mi;
> @@ -5830,7 +5831,15 @@ umb_status(void)
>   for (i = 0, n = 0; i < UMB_MAX_DNSSRV; i++) {
>   if (mi.ipv4dns[i].s_addr == INADDR_ANY)
>   break;
> - printf("%s %s", n++ ? "" : "\tdns", inet_ntoa(mi.ipv4dns[i]));
> + printf("%s %s", n++ ? "" : "\tdns",
> +    inet_ntop(AF_INET, &mi.ipv4dns[i], astr, sizeof (astr)));
> + }
> + for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> + if (memcmp(&mi.ipv6dns[i], &in6addr_any,
> +    sizeof (mi.ipv6dns[i])) == 0)
> + break;
> + printf("%s %s", n++ ? "" : "\tdns",
> +    inet_ntop(AF_INET6, &mi.ipv6dns[i], astr, sizeof (astr)));
>   }
>   if (n)
>   printf("\n");
> Index: share/man/man4/umb.4
> ===================================================================
> RCS file: /cvs/src/share/man/man4/umb.4,v
> retrieving revision 1.9
> diff -u -p -u -p -r1.9 umb.4
> --- share/man/man4/umb.4 23 Nov 2017 20:47:26 -0000 1.9
> +++ share/man/man4/umb.4 28 Jan 2020 12:16:23 -0000
> @@ -40,6 +40,11 @@ will remain in this state until the MBIM
>  In case the device is connected to an "always-on" USB port,
>  it may be possible to connect to a provider without entering the
>  PIN again even if the system was rebooted.
> +.Pp
> +If the kernel has been compiled with INET6, the driver will try to
> +obtain an IPv6 address from the provider. To succeed with the IPv6
> +configuration, both the ISP and the MBIM device have to offer IPv6
> +support.
>  .Sh HARDWARE
>  The following devices should work:
>  .Pp
> @@ -64,10 +69,6 @@ The following devices should work:
>  .%U http://www.usb.org/developers/docs/devclass_docs/MBIM10Errata1_073013.zip
>  .Re
>  .Sh CAVEATS
> -The
> -.Nm
> -driver does not support IPv6.
> -.Pp
>  Devices which fail to provide a conforming MBIM implementation will
>  probably be attached as some other driver, such as
>  .Xr umsm 4 .
> Index: sys/dev/usb/if_umb.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.31
> diff -u -p -u -p -r1.31 if_umb.c
> --- sys/dev/usb/if_umb.c 26 Nov 2019 23:04:28 -0000 1.31
> +++ sys/dev/usb/if_umb.c 4 Feb 2020 07:50:30 -0000
> @@ -43,6 +43,14 @@
>  #include <netinet/in_var.h>
>  #include <netinet/ip.h>
>
> +#ifdef INET6
> +#include <netinet/ip6.h>
> +#include <netinet6/in6_var.h>
> +#include <netinet6/ip6_var.h>
> +#include <netinet6/in6_ifattach.h>
> +#include <netinet6/nd6.h>
> +#endif
> +
>  #include <machine/bus.h>
>
>  #include <dev/usb/usb.h>
> @@ -158,7 +166,9 @@ int umb_decode_connect_info(struct umb
>  void umb_clear_addr(struct umb_softc *);
>  int umb_add_inet_config(struct umb_softc *, struct in_addr, u_int,
>      struct in_addr);
> -void umb_send_inet_proposal(struct umb_softc *);
> +int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
> +    u_int, struct in6_addr *);
> +void umb_send_inet_proposal(struct umb_softc *, int);
>  int umb_decode_ip_configuration(struct umb_softc *, void *, int);
>  void umb_rx(struct umb_softc *);
>  void umb_rxeof(struct usbd_xfer *, void *, usbd_status);
> @@ -800,8 +810,8 @@ umb_input(struct ifnet *ifp, struct mbuf
>  #endif /* INET6 */
>   default:
>   ifp->if_ierrors++;
> - DPRINTFN(4, "%s: dropping packet with bad IP version (%d)\n",
> -    __func__, ipv);
> + DPRINTFN(4, "%s: dropping packet with bad IP version (af %d)\n",
> +    __func__, af);
>   m_freem(m);
>   return 1;
>   }
> @@ -902,7 +912,10 @@ umb_rtrequest(struct ifnet *ifp, int req
>   struct umb_softc *sc = ifp->if_softc;
>
>   if (req == RTM_PROPOSAL) {
> - umb_send_inet_proposal(sc);
> + umb_send_inet_proposal(sc, AF_INET);
> +#ifdef INET6
> + umb_send_inet_proposal(sc, AF_INET6);
> +#endif
>   return;
>   }
>
> @@ -1596,11 +1609,6 @@ umb_decode_connect_info(struct umb_softc
>   if (ifp->if_flags & IFF_DEBUG)
>   log(LOG_INFO, "%s: connection %s\n", DEVNAM(sc),
>      umb_activation(act));
> - if ((ifp->if_flags & IFF_DEBUG) &&
> -    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_DEFAULT &&
> -    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_IPV4)
> - log(LOG_DEBUG, "%s: got iptype %d connection\n",
> -    DEVNAM(sc), letoh32(ci->iptype));
>
>   sc->sc_info.activation = act;
>   sc->sc_info.nwerror = letoh32(ci->nwerror);
> @@ -1621,9 +1629,16 @@ umb_clear_addr(struct umb_softc *sc)
>   struct ifnet *ifp = GET_IFP(sc);
>
>   memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> - umb_send_inet_proposal(sc);
> + memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
> + umb_send_inet_proposal(sc, AF_INET);
> +#ifdef INET6
> + umb_send_inet_proposal(sc, AF_INET6);
> +#endif
>   NET_LOCK();
>   in_ifdetach(ifp);
> +#ifdef INET6
> + in6_ifdetach(ifp);
> +#endif
>   NET_UNLOCK();
>  }
>
> @@ -1698,29 +1713,125 @@ umb_add_inet_config(struct umb_softc *sc
>      sockaddr_ntop(sintosa(&ifra.ifra_dstaddr), str[2],
>      sizeof(str[2])));
>   }
> - return rv;
> + return 0;
> +}
> +
> +#ifdef INET6
> +int
> +umb_add_inet6_config(struct umb_softc *sc, struct in6_addr *ip, u_int prefixlen,
> +    struct in6_addr *gw)
> +{
> + struct ifnet *ifp = GET_IFP(sc);
> + struct in6_aliasreq ifra;
> + struct sockaddr_in6 *sin6, default_sin6;
> + struct rt_addrinfo info;
> + struct rtentry *rt;
> + int rv;
> +
> + memset(&ifra, 0, sizeof (ifra));
> + sin6 = &ifra.ifra_addr;
> + sin6->sin6_family = AF_INET6;
> + sin6->sin6_len = sizeof (*sin6);
> + memcpy(&sin6->sin6_addr, ip, sizeof (sin6->sin6_addr));
> +
> + sin6 = &ifra.ifra_dstaddr;
> + sin6->sin6_family = AF_INET6;
> + sin6->sin6_len = sizeof (*sin6);
> + memcpy(&sin6->sin6_addr, gw, sizeof (sin6->sin6_addr));
> +
> + /* XXX: in6_update_ifa() accepts only 128 bits for P2P interfaces. */
> + prefixlen = 128;
> +
> + sin6 = &ifra.ifra_prefixmask;
> + sin6->sin6_family = AF_INET6;
> + sin6->sin6_len = sizeof (*sin6);
> + in6_prefixlen2mask(&sin6->sin6_addr, prefixlen);
> +
> + ifra.ifra_lifetime.ia6t_vltime = ND6_INFINITE_LIFETIME;
> + ifra.ifra_lifetime.ia6t_pltime = ND6_INFINITE_LIFETIME;
> +
> + rv = in6_ioctl(SIOCAIFADDR_IN6, (caddr_t)&ifra, ifp, 1);
> + if (rv != 0) {
> + printf("%s: unable to set IPv6 address, error %d\n",
> +    DEVNAM(ifp->if_softc), rv);
> + return rv;
> + }
> +
> + memset(&default_sin6, 0, sizeof(default_sin6));
> + default_sin6.sin6_family = AF_INET6;
> + default_sin6.sin6_len = sizeof (default_sin6);
> +
> + memset(&info, 0, sizeof(info));
> + info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> + info.rti_ifa = ifa_ifwithaddr(sin6tosa(&ifra.ifra_addr),
> +    ifp->if_rdomain);
> + info.rti_info[RTAX_DST] = sin6tosa(&default_sin6);
> + info.rti_info[RTAX_NETMASK] = sin6tosa(&default_sin6);
> + info.rti_info[RTAX_GATEWAY] = sin6tosa(&ifra.ifra_dstaddr);
> +
> + NET_LOCK();
> + rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
> + NET_UNLOCK();
> + if (rv) {
> + printf("%s: unable to set IPv6 default route, "
> +    "error %d\n", DEVNAM(ifp->if_softc), rv);
> + rtm_miss(RTM_MISS, &info, 0, RTP_NONE, 0, rv,
> +    ifp->if_rdomain);
> + } else {
> + /* Inform listeners of the new route */
> + rtm_send(rt, RTM_ADD, rv, ifp->if_rdomain);
> + rtfree(rt);
> + }
> +
> + if (ifp->if_flags & IFF_DEBUG) {
> + char str[3][INET6_ADDRSTRLEN];
> + log(LOG_INFO, "%s: IPv6 addr %s, mask %s, gateway %s\n",
> +    DEVNAM(ifp->if_softc),
> +    sockaddr_ntop(sin6tosa(&ifra.ifra_addr), str[0],
> +    sizeof(str[0])),
> +    sockaddr_ntop(sin6tosa(&ifra.ifra_prefixmask), str[1],
> +    sizeof(str[1])),
> +    sockaddr_ntop(sin6tosa(&ifra.ifra_dstaddr), str[2],
> +    sizeof(str[2])));
> + }
> + return 0;
>  }
> +#endif
>
>  void
> -umb_send_inet_proposal(struct umb_softc *sc)
> +umb_send_inet_proposal(struct umb_softc *sc, int af)
>  {
>   struct ifnet *ifp = GET_IFP(sc);
>   struct sockaddr_rtdns rtdns;
>   struct rt_addrinfo info;
>   int i, flag = 0;
> + size_t sz = 0;
>
>   memset(&rtdns, 0, sizeof(rtdns));
>   memset(&info, 0, sizeof(info));
>
>   for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> - if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
> - break;
> - memcpy(rtdns.sr_dns + i * sizeof(struct in_addr),
> -    &sc->sc_info.ipv4dns[i], sizeof(struct in_addr));
> - flag = RTF_UP;
> + if (af == AF_INET) {
> + sz = sizeof (sc->sc_info.ipv4dns[i]);
> + if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
> + break;
> + memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv4dns[i],
> +    sz);
> + flag = RTF_UP;
> +#ifdef INET6
> + } if (af == AF_INET6) {
> + sz = sizeof (sc->sc_info.ipv6dns[i]);
> + if (IN6_ARE_ADDR_EQUAL(&sc->sc_info.ipv6dns[i],
> +    &in6addr_any))
> + break;
> + memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv6dns[i],
> +    sz);
> + flag = RTF_UP;
> +#endif
> + }
>   }
> - rtdns.sr_family = AF_INET;
> - rtdns.sr_len = 2 + i * sizeof(struct in_addr);
> + rtdns.sr_family = af;
> + rtdns.sr_len = 2 + i * sz;
>   info.rti_info[RTAX_DNS] = srtdnstosa(&rtdns);
>
>   rtm_proposal(ifp, &info, flag, RTP_PROPOSAL_UMB);
> @@ -1732,7 +1843,7 @@ umb_decode_ip_configuration(struct umb_s
>   struct mbim_cid_ip_configuration_info *ic = data;
>   struct ifnet *ifp = GET_IFP(sc);
>   int s;
> - uint32_t avail;
> + uint32_t avail_v4;
>   uint32_t val;
>   int n, i;
>   int off;
> @@ -1740,6 +1851,12 @@ umb_decode_ip_configuration(struct umb_s
>   struct in_addr addr, gw;
>   int state = -1;
>   int rv;
> + int hasmtu = 0;
> +#ifdef INET6
> + uint32_t avail_v6;
> + struct mbim_cid_ipv6_element ipv6elem;
> + struct in6_addr addr6, gw6;
> +#endif
>
>   if (len < sizeof (*ic))
>   return 0;
> @@ -1750,17 +1867,20 @@ umb_decode_ip_configuration(struct umb_s
>   }
>   s = splnet();
>
> + memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> + memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
> +
>   /*
>   * IPv4 configuation
>   */
> - avail = letoh32(ic->ipv4_available);
> - if ((avail & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> + avail_v4 = letoh32(ic->ipv4_available);
> + if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
>      (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
>   n = letoh32(ic->ipv4_naddr);
>   off = letoh32(ic->ipv4_addroffs);
>
>   if (n == 0 || off + sizeof (ipv4elem) > len)
> - goto done;
> + goto tryv6;
>   if (n != 1 && ifp->if_flags & IFF_DEBUG)
>   log(LOG_INFO, "%s: more than one IPv4 addr: %d\n",
>      DEVNAM(ifp->if_softc), n);
> @@ -1771,6 +1891,8 @@ umb_decode_ip_configuration(struct umb_s
>   addr.s_addr = ipv4elem.addr;
>
>   off = letoh32(ic->ipv4_gwoffs);
> + if (off + sizeof (gw) > len)
> + goto done;
>   memcpy(&gw, data + off, sizeof(gw));
>
>   rv = umb_add_inet_config(sc, addr, ipv4elem.prefixlen, gw);
> @@ -1780,12 +1902,12 @@ umb_decode_ip_configuration(struct umb_s
>   }
>
>   memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> - if (avail & MBIM_IPCONF_HAS_DNSINFO) {
> + if (avail_v4 & MBIM_IPCONF_HAS_DNSINFO) {
>   n = letoh32(ic->ipv4_ndnssrv);
>   off = letoh32(ic->ipv4_dnssrvoffs);
>   i = 0;
>   while (n-- > 0) {
> - if (off + sizeof (uint32_t) > len)
> + if (off + sizeof (addr) > len)
>   break;
>   memcpy(&addr, data + off, sizeof(addr));
>   if (i < UMB_MAX_DNSSRV)
> @@ -1798,29 +1920,95 @@ umb_decode_ip_configuration(struct umb_s
>      &addr, str, sizeof(str)));
>   }
>   }
> - umb_send_inet_proposal(sc);
> + umb_send_inet_proposal(sc, AF_INET);
>   }
> -
> - if ((avail & MBIM_IPCONF_HAS_MTUINFO)) {
> + if ((avail_v4 & MBIM_IPCONF_HAS_MTUINFO)) {
>   val = letoh32(ic->ipv4_mtu);
>   if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
> + hasmtu = 1;
>   ifp->if_hardmtu = val;
>   if (ifp->if_mtu > val)
>   ifp->if_mtu = val;
> - if (ifp->if_flags & IFF_DEBUG)
> - log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), val);
>   }
>   }
>
> - avail = letoh32(ic->ipv6_available);
> - if ((ifp->if_flags & IFF_DEBUG) && avail & MBIM_IPCONF_HAS_ADDRINFO) {
> - /* XXX FIXME: IPv6 configuation missing */
> - log(LOG_INFO, "%s: ignoring IPv6 configuration\n", DEVNAM(sc));
> +tryv6:;
> +#ifdef INET6
> + /*
> + * IPv6 configuation
> + */
> + avail_v6 = letoh32(ic->ipv6_available);
> + if (avail_v6 == 0) {
> + if (ifp->if_flags & IFF_DEBUG)
> + log(LOG_INFO, "%s: ISP or WWAN module offers no IPv6 "
> +    "support\n", DEVNAM(ifp->if_softc));
> + goto done;
> + }
> +
> + if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> +    (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
> + n = letoh32(ic->ipv6_naddr);
> + off = letoh32(ic->ipv6_addroffs);
> +
> + if (n == 0 || off + sizeof (ipv6elem) > len)
> + goto done;
> + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> +    DEVNAM(ifp->if_softc), n);
> +
> + /* Only pick the first one */
> + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> +
> + off = letoh32(ic->ipv6_gwoffs);
> + if (off + sizeof (gw6) > len)
> + goto done;
> + memcpy(&gw6, data + off, sizeof (gw6));
> +
> + rv = umb_add_inet6_config(sc, &addr6, ipv6elem.prefixlen, &gw6);
> + if (rv == 0)
> + state = UMB_S_UP;
> + }
> +
> + if (avail_v6 & MBIM_IPCONF_HAS_DNSINFO) {
> + n = letoh32(ic->ipv6_ndnssrv);
> + off = letoh32(ic->ipv6_dnssrvoffs);
> + i = 0;
> + while (n-- > 0) {
> + if (off + sizeof (addr6) > len)
> + break;
> + memcpy(&addr6, data + off, sizeof(addr6));
> + if (i < UMB_MAX_DNSSRV)
> + sc->sc_info.ipv6dns[i++] = addr6;
> + off += sizeof(addr6);
> + if (ifp->if_flags & IFF_DEBUG) {
> + char str[INET6_ADDRSTRLEN];
> + log(LOG_INFO, "%s: IPv6 nameserver %s\n",
> +    DEVNAM(ifp->if_softc), inet_ntop(AF_INET6,
> +    &addr6, str, sizeof(str)));
> + }
> + }
> + umb_send_inet_proposal(sc, AF_INET6);
> + }
> +
> + if ((avail_v6 & MBIM_IPCONF_HAS_MTUINFO)) {
> + val = letoh32(ic->ipv6_mtu);
> + if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
> + hasmtu = 1;
> + ifp->if_hardmtu = val;
> + if (ifp->if_mtu > val)
> + ifp->if_mtu = val;
> + }
>   }
> +#endif
> +
> +done:
> + if (hasmtu && (ifp->if_flags & IFF_DEBUG))
> + log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), ifp->if_hardmtu);
> +
>   if (state != -1)
>   umb_newstate(sc, state, 0);
>
> -done:
>   splx(s);
>   return 1;
>  }
> @@ -2393,6 +2581,11 @@ umb_send_connect(struct umb_softc *sc, i
>   c->authprot = htole32(MBIM_AUTHPROT_NONE);
>   c->compression = htole32(MBIM_COMPRESSION_NONE);
>   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
> +#ifdef INET6
> + /* XXX FIXME: support IPv6-only mode, too */
> + if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
> + c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);
> +#endif
>   memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));
>   umb_cmd(sc, MBIM_CID_CONNECT, MBIM_CMDOP_SET, c, off);
>  done:
> @@ -2476,6 +2669,20 @@ umb_command_done(struct umb_softc *sc, v
>   switch (status) {
>   case MBIM_STATUS_SUCCESS:
>   break;
> +#ifdef INET6
> + case MBIM_STATUS_NO_DEVICE_SUPPORT:
> + if ((cid == MBIM_CID_CONNECT) &&
> +    (sc->sc_flags & UMBFLG_NO_INET6) == 0) {
> + sc->sc_flags |= UMBFLG_NO_INET6;
> + if (ifp->if_flags & IFF_DEBUG)
> + log(LOG_ERR,
> +    "%s: device does not support IPv6\n",
> +    DEVNAM(sc));
> + }
> + /* Re-trigger the connect, this time IPv4 only */
> + usb_add_task(sc->sc_udev, &sc->sc_umb_task);
> + return;
> +#endif
>   case MBIM_STATUS_NOT_INITIALIZED:
>   if (ifp->if_flags & IFF_DEBUG)
>   log(LOG_ERR, "%s: SIM not initialized (PIN missing)\n",
> Index: sys/dev/usb/if_umb.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.h,v
> retrieving revision 1.5
> diff -u -p -u -p -r1.5 if_umb.h
> --- sys/dev/usb/if_umb.h 26 Aug 2019 15:23:01 -0000 1.5
> +++ sys/dev/usb/if_umb.h 4 Feb 2020 07:30:43 -0000
> @@ -321,6 +321,7 @@ struct umb_info {
>
>  #define UMB_MAX_DNSSRV 2
>   struct in_addr ipv4dns[UMB_MAX_DNSSRV];
> + struct in6_addr ipv6dns[UMB_MAX_DNSSRV];
>  };
>
>  #ifdef _KERNEL
> @@ -345,6 +346,7 @@ struct umb_softc {
>   int sc_ndp_remainder;
>
>  #define UMBFLG_FCC_AUTH_REQUIRED 0x0001
> +#define UMBFLG_NO_INET6 0x0002
>   uint32_t sc_flags;
>   int sc_cid;
>
> @@ -380,6 +382,6 @@ struct umb_softc {
>
>  #define sc_state sc_info.state
>  #define sc_roaming sc_info.enable_roaming
> - struct umb_info sc_info;
> + struct umb_info sc_info;
>  };
>  #endif /* _KERNEL */

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Claudio Jeker
In reply to this post by Gerhard Roth-2
On Tue, Feb 04, 2020 at 09:16:34AM +0100, Gerhard Roth wrote:

> Hi Alex,
>
> thanks for looking into it.
>
>
> On Tue, 4 Feb 2020 00:20:42 +0100 Alexander Bluhm <[hidden email]> wrote:
> > On Tue, Jan 28, 2020 at 03:03:47PM +0100, Gerhard Roth wrote:
> > > this patch adds IPv6 support to umb(4).  
> >
> > It breaks my IPv4 setup.
> >
> > umb0 at uhub0 port 4 configuration 1 interface 6 "Lenovo H5321 gw" rev 2.00/0.00 addr 2
> > provider Vodafone.de
> > firmware CXP 901 8700/1 - R3C18
> >
> > When I apply the diff, my umb device does not get an IPv4 address.
> >
> > umb0: state going up from 'open' to 'radio on'
> > umb0: none state unlocked (-1 attempts left)
> > umb0: set/qry MBIM_CID_SUBSCRIBER_READY_STATUS failed: BUSY
> > umb0: packet service changed from detached to attaching, class none, speed: 0 up / 0 down
> > umb0: SIM initialized
> > umb0: state going up from 'radio on' to 'SIM is ready'
> > umb0: packet service changed from attaching to attached, class HSPA, speed: 5760000 up / 14400000 down
> > umb0: state going up from 'SIM is ready' to 'attached'
> > umb0: connecting ...
> > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
>
> That looks like the firmware of this Lenovo H5321 doesn't support IPv6.
> Well at least it gives a decent error code.
>
>
> > umb0: state change timeout
> > umb0: connecting ...
> > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > umb0: state change timeout
> > umb0: connecting ...
> > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > umb0: state change timeout
> > ...
> >
> > A few comments inline.
> >
> > > +#ifdef INET6
> > > +int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
> > > +    u_int, struct in6_addr *);
> > > +#endif  
> >
> > Usually I avoid #ifdef for prototypes.  It does not matter whether
> > the compiler reads them and without #ifdef the code is nicer.
>
> Removed it.
>
>
> >
> > > +tryv6:;  
> >
> > The ; is wrong.
>
> Not really, because the label 'tryv6' is immediately followed by
> an "#ifdef INET6". So looking just at this label I cannot directly tell
> whether there is code that follows or not. And a label with no code
> following is a syntax error in C.
>
> I just followed a old "C Style and Coding Standards for SunOS" paper
> by Bill Shannon from 1993 that says:
>
> If a label is not followed by a program statement (e.g.
> if the next token is a closing brace( } )) a NULL
> statement ( ; ) must follow the label.
>
>
> >
> > > + if (n == 0 || off + sizeof (ipv6elem) > len)
> > > + goto done;
> > > + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> > > + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> > > +    DEVNAM(ifp->if_softc), n);
> > > +
> > > + /* Only pick the first one */
> > > + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> > > + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> > > +
> > > + off = letoh32(ic->ipv6_gwoffs);
> > > + memcpy(&gw6, data + off, sizeof (gw6));  
> >
> > I think we should check the data length like above.
> >
> > if (off + sizeof (gw6) > len)
> > goto done;
> >
> > And IPv4 should get the same check.
>
> Thanks for spotting that. Added check to both cases.
>
>
> >
> > > @@ -380,6 +381,6 @@ struct umb_softc {
> > >
> > >  #define sc_state sc_info.state
> > >  #define sc_roaming sc_info.enable_roaming
> > > - struct umb_info sc_info;
> > > + struct umb_info sc_info;
> > >  };
> > >  #endif /* _KERNEL */  
> >
> > This whitespace chunk is wrong.
>
> I think it was wrong before. It's common idiom to add an extra space
> to non-pointer members to keep the member names aligned, e.g.
>
> struct foo {
> int *abc;
> int def;
> int *bar;
> };
>
> Please correct me if I'm wrong.
>
> >
> > bluhm
>
>
> The updated patch below introduces a UMBFLG_NO_INET6 which is set on
> receipt of a MBIM_STATUS_NO_DEVICE_SUPPORT in response to a
> MBIM_CID_CONNECT. The code will then retry the connect operation in
> IPv4-only mode.
>
> That won't give you any IPv6 support, but at least it won't break
> your setup.
>

A few whitespace fixes and some comments inline but apart from that
OK claudio@

> Index: sbin/ifconfig/ifconfig.c
> ===================================================================
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.417
> diff -u -p -u -p -r1.417 ifconfig.c
> --- sbin/ifconfig/ifconfig.c 27 Dec 2019 14:34:46 -0000 1.417
> +++ sbin/ifconfig/ifconfig.c 28 Jan 2020 12:16:23 -0000
> @@ -5666,6 +5666,7 @@ umb_status(void)
>   char apn[UMB_APN_MAXLEN+1];
>   char pn[UMB_PHONENR_MAXLEN+1];
>   int i, n;
> + char astr[INET6_ADDRSTRLEN];
>  
>   memset((char *)&mi, 0, sizeof(mi));
>   ifr.ifr_data = (caddr_t)&mi;
> @@ -5830,7 +5831,15 @@ umb_status(void)
>   for (i = 0, n = 0; i < UMB_MAX_DNSSRV; i++) {
>   if (mi.ipv4dns[i].s_addr == INADDR_ANY)
>   break;
> - printf("%s %s", n++ ? "" : "\tdns", inet_ntoa(mi.ipv4dns[i]));
> + printf("%s %s", n++ ? "" : "\tdns",
> +    inet_ntop(AF_INET, &mi.ipv4dns[i], astr, sizeof (astr)));

Please no space between sizeof and (astr).

> + }
> + for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> + if (memcmp(&mi.ipv6dns[i], &in6addr_any,
> +    sizeof (mi.ipv6dns[i])) == 0)
> + break;
> + printf("%s %s", n++ ? "" : "\tdns",
> +    inet_ntop(AF_INET6, &mi.ipv6dns[i], astr, sizeof (astr)));

Same here.

>   }
>   if (n)
>   printf("\n");
> Index: share/man/man4/umb.4
> ===================================================================
> RCS file: /cvs/src/share/man/man4/umb.4,v
> retrieving revision 1.9
> diff -u -p -u -p -r1.9 umb.4
> --- share/man/man4/umb.4 23 Nov 2017 20:47:26 -0000 1.9
> +++ share/man/man4/umb.4 28 Jan 2020 12:16:23 -0000
> @@ -40,6 +40,11 @@ will remain in this state until the MBIM
>  In case the device is connected to an "always-on" USB port,
>  it may be possible to connect to a provider without entering the
>  PIN again even if the system was rebooted.
> +.Pp
> +If the kernel has been compiled with INET6, the driver will try to
> +obtain an IPv6 address from the provider. To succeed with the IPv6
> +configuration, both the ISP and the MBIM device have to offer IPv6
> +support.

Almost all our kernels have been compiled with INET6 support. I would
remove that part and just go for:
The driver will try to obtain an IPv6 address from the provider.
To succeed with the IPv6 configuration, both the ISP and the MBIM device
have to offer IPv6 support.

Btw. new sentence new line in man pages.

>  .Sh HARDWARE
>  The following devices should work:
>  .Pp
> @@ -64,10 +69,6 @@ The following devices should work:
>  .%U http://www.usb.org/developers/docs/devclass_docs/MBIM10Errata1_073013.zip
>  .Re
>  .Sh CAVEATS
> -The
> -.Nm
> -driver does not support IPv6.
> -.Pp
>  Devices which fail to provide a conforming MBIM implementation will
>  probably be attached as some other driver, such as
>  .Xr umsm 4 .
> Index: sys/dev/usb/if_umb.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.31
> diff -u -p -u -p -r1.31 if_umb.c
> --- sys/dev/usb/if_umb.c 26 Nov 2019 23:04:28 -0000 1.31
> +++ sys/dev/usb/if_umb.c 4 Feb 2020 07:50:30 -0000
> @@ -43,6 +43,14 @@
>  #include <netinet/in_var.h>
>  #include <netinet/ip.h>
>  
> +#ifdef INET6
> +#include <netinet/ip6.h>
> +#include <netinet6/in6_var.h>
> +#include <netinet6/ip6_var.h>
> +#include <netinet6/in6_ifattach.h>
> +#include <netinet6/nd6.h>
> +#endif
> +
>  #include <machine/bus.h>
>  
>  #include <dev/usb/usb.h>
> @@ -158,7 +166,9 @@ int umb_decode_connect_info(struct umb
>  void umb_clear_addr(struct umb_softc *);
>  int umb_add_inet_config(struct umb_softc *, struct in_addr, u_int,
>      struct in_addr);
> -void umb_send_inet_proposal(struct umb_softc *);
> +int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
> +    u_int, struct in6_addr *);
> +void umb_send_inet_proposal(struct umb_softc *, int);
>  int umb_decode_ip_configuration(struct umb_softc *, void *, int);
>  void umb_rx(struct umb_softc *);
>  void umb_rxeof(struct usbd_xfer *, void *, usbd_status);
> @@ -800,8 +810,8 @@ umb_input(struct ifnet *ifp, struct mbuf
>  #endif /* INET6 */
>   default:
>   ifp->if_ierrors++;
> - DPRINTFN(4, "%s: dropping packet with bad IP version (%d)\n",
> -    __func__, ipv);
> + DPRINTFN(4, "%s: dropping packet with bad IP version (af %d)\n",
> +    __func__, af);
>   m_freem(m);
>   return 1;
>   }
> @@ -902,7 +912,10 @@ umb_rtrequest(struct ifnet *ifp, int req
>   struct umb_softc *sc = ifp->if_softc;
>  
>   if (req == RTM_PROPOSAL) {
> - umb_send_inet_proposal(sc);
> + umb_send_inet_proposal(sc, AF_INET);
> +#ifdef INET6
> + umb_send_inet_proposal(sc, AF_INET6);
> +#endif
>   return;
>   }
>  
> @@ -1596,11 +1609,6 @@ umb_decode_connect_info(struct umb_softc
>   if (ifp->if_flags & IFF_DEBUG)
>   log(LOG_INFO, "%s: connection %s\n", DEVNAM(sc),
>      umb_activation(act));
> - if ((ifp->if_flags & IFF_DEBUG) &&
> -    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_DEFAULT &&
> -    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_IPV4)
> - log(LOG_DEBUG, "%s: got iptype %d connection\n",
> -    DEVNAM(sc), letoh32(ci->iptype));
>  
>   sc->sc_info.activation = act;
>   sc->sc_info.nwerror = letoh32(ci->nwerror);
> @@ -1621,9 +1629,16 @@ umb_clear_addr(struct umb_softc *sc)
>   struct ifnet *ifp = GET_IFP(sc);
>  
>   memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> - umb_send_inet_proposal(sc);
> + memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
> + umb_send_inet_proposal(sc, AF_INET);
> +#ifdef INET6
> + umb_send_inet_proposal(sc, AF_INET6);
> +#endif
>   NET_LOCK();
>   in_ifdetach(ifp);
> +#ifdef INET6
> + in6_ifdetach(ifp);
> +#endif
>   NET_UNLOCK();
>  }
>  
> @@ -1698,29 +1713,125 @@ umb_add_inet_config(struct umb_softc *sc
>      sockaddr_ntop(sintosa(&ifra.ifra_dstaddr), str[2],
>      sizeof(str[2])));
>   }
> - return rv;
> + return 0;
> +}
> +
> +#ifdef INET6
> +int
> +umb_add_inet6_config(struct umb_softc *sc, struct in6_addr *ip, u_int prefixlen,
> +    struct in6_addr *gw)
> +{
> + struct ifnet *ifp = GET_IFP(sc);
> + struct in6_aliasreq ifra;
> + struct sockaddr_in6 *sin6, default_sin6;
> + struct rt_addrinfo info;
> + struct rtentry *rt;
> + int rv;
> +
> + memset(&ifra, 0, sizeof (ifra));
> + sin6 = &ifra.ifra_addr;
> + sin6->sin6_family = AF_INET6;
> + sin6->sin6_len = sizeof (*sin6);
> + memcpy(&sin6->sin6_addr, ip, sizeof (sin6->sin6_addr));
> +
> + sin6 = &ifra.ifra_dstaddr;
> + sin6->sin6_family = AF_INET6;
> + sin6->sin6_len = sizeof (*sin6);
> + memcpy(&sin6->sin6_addr, gw, sizeof (sin6->sin6_addr));
> +
> + /* XXX: in6_update_ifa() accepts only 128 bits for P2P interfaces. */
> + prefixlen = 128;
> +
> + sin6 = &ifra.ifra_prefixmask;
> + sin6->sin6_family = AF_INET6;
> + sin6->sin6_len = sizeof (*sin6);
> + in6_prefixlen2mask(&sin6->sin6_addr, prefixlen);
> +
> + ifra.ifra_lifetime.ia6t_vltime = ND6_INFINITE_LIFETIME;
> + ifra.ifra_lifetime.ia6t_pltime = ND6_INFINITE_LIFETIME;
> +
> + rv = in6_ioctl(SIOCAIFADDR_IN6, (caddr_t)&ifra, ifp, 1);
> + if (rv != 0) {
> + printf("%s: unable to set IPv6 address, error %d\n",
> +    DEVNAM(ifp->if_softc), rv);
> + return rv;
> + }
> +
> + memset(&default_sin6, 0, sizeof(default_sin6));
> + default_sin6.sin6_family = AF_INET6;
> + default_sin6.sin6_len = sizeof (default_sin6);
> +
> + memset(&info, 0, sizeof(info));
> + info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> + info.rti_ifa = ifa_ifwithaddr(sin6tosa(&ifra.ifra_addr),
> +    ifp->if_rdomain);
> + info.rti_info[RTAX_DST] = sin6tosa(&default_sin6);
> + info.rti_info[RTAX_NETMASK] = sin6tosa(&default_sin6);
> + info.rti_info[RTAX_GATEWAY] = sin6tosa(&ifra.ifra_dstaddr);
> +
> + NET_LOCK();
> + rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
> + NET_UNLOCK();
> + if (rv) {
> + printf("%s: unable to set IPv6 default route, "
> +    "error %d\n", DEVNAM(ifp->if_softc), rv);
> + rtm_miss(RTM_MISS, &info, 0, RTP_NONE, 0, rv,
> +    ifp->if_rdomain);
> + } else {
> + /* Inform listeners of the new route */
> + rtm_send(rt, RTM_ADD, rv, ifp->if_rdomain);
> + rtfree(rt);
> + }
> +
> + if (ifp->if_flags & IFF_DEBUG) {
> + char str[3][INET6_ADDRSTRLEN];
> + log(LOG_INFO, "%s: IPv6 addr %s, mask %s, gateway %s\n",
> +    DEVNAM(ifp->if_softc),
> +    sockaddr_ntop(sin6tosa(&ifra.ifra_addr), str[0],
> +    sizeof(str[0])),
> +    sockaddr_ntop(sin6tosa(&ifra.ifra_prefixmask), str[1],
> +    sizeof(str[1])),
> +    sockaddr_ntop(sin6tosa(&ifra.ifra_dstaddr), str[2],
> +    sizeof(str[2])));
> + }
> + return 0;
>  }
> +#endif
>  
>  void
> -umb_send_inet_proposal(struct umb_softc *sc)
> +umb_send_inet_proposal(struct umb_softc *sc, int af)
>  {
>   struct ifnet *ifp = GET_IFP(sc);
>   struct sockaddr_rtdns rtdns;
>   struct rt_addrinfo info;
>   int i, flag = 0;
> + size_t sz = 0;
>  
>   memset(&rtdns, 0, sizeof(rtdns));
>   memset(&info, 0, sizeof(info));
>  
>   for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> - if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
> - break;
> - memcpy(rtdns.sr_dns + i * sizeof(struct in_addr),
> -    &sc->sc_info.ipv4dns[i], sizeof(struct in_addr));
> - flag = RTF_UP;
> + if (af == AF_INET) {
> + sz = sizeof (sc->sc_info.ipv4dns[i]);
> + if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
> + break;
> + memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv4dns[i],
> +    sz);
> + flag = RTF_UP;
> +#ifdef INET6
> + } if (af == AF_INET6) {

Either make this an else if or add a newline between the } and if.

> + sz = sizeof (sc->sc_info.ipv6dns[i]);
> + if (IN6_ARE_ADDR_EQUAL(&sc->sc_info.ipv6dns[i],
> +    &in6addr_any))
> + break;
> + memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv6dns[i],
> +    sz);
> + flag = RTF_UP;
> +#endif
> + }
>   }
> - rtdns.sr_family = AF_INET;
> - rtdns.sr_len = 2 + i * sizeof(struct in_addr);
> + rtdns.sr_family = af;
> + rtdns.sr_len = 2 + i * sz;
>   info.rti_info[RTAX_DNS] = srtdnstosa(&rtdns);
>  
>   rtm_proposal(ifp, &info, flag, RTP_PROPOSAL_UMB);
> @@ -1732,7 +1843,7 @@ umb_decode_ip_configuration(struct umb_s
>   struct mbim_cid_ip_configuration_info *ic = data;
>   struct ifnet *ifp = GET_IFP(sc);
>   int s;
> - uint32_t avail;
> + uint32_t avail_v4;
>   uint32_t val;
>   int n, i;
>   int off;
> @@ -1740,6 +1851,12 @@ umb_decode_ip_configuration(struct umb_s
>   struct in_addr addr, gw;
>   int state = -1;
>   int rv;
> + int hasmtu = 0;
> +#ifdef INET6
> + uint32_t avail_v6;
> + struct mbim_cid_ipv6_element ipv6elem;
> + struct in6_addr addr6, gw6;
> +#endif
>  
>   if (len < sizeof (*ic))
>   return 0;
> @@ -1750,17 +1867,20 @@ umb_decode_ip_configuration(struct umb_s
>   }
>   s = splnet();
>  
> + memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> + memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
> +
>   /*
>   * IPv4 configuation
>   */
> - avail = letoh32(ic->ipv4_available);
> - if ((avail & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> + avail_v4 = letoh32(ic->ipv4_available);
> + if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
>      (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
>   n = letoh32(ic->ipv4_naddr);
>   off = letoh32(ic->ipv4_addroffs);
>  
>   if (n == 0 || off + sizeof (ipv4elem) > len)
> - goto done;
> + goto tryv6;
>   if (n != 1 && ifp->if_flags & IFF_DEBUG)
>   log(LOG_INFO, "%s: more than one IPv4 addr: %d\n",
>      DEVNAM(ifp->if_softc), n);
> @@ -1771,6 +1891,8 @@ umb_decode_ip_configuration(struct umb_s
>   addr.s_addr = ipv4elem.addr;
>  
>   off = letoh32(ic->ipv4_gwoffs);
> + if (off + sizeof (gw) > len)
> + goto done;
>   memcpy(&gw, data + off, sizeof(gw));
>  
>   rv = umb_add_inet_config(sc, addr, ipv4elem.prefixlen, gw);
> @@ -1780,12 +1902,12 @@ umb_decode_ip_configuration(struct umb_s
>   }
>  
>   memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> - if (avail & MBIM_IPCONF_HAS_DNSINFO) {
> + if (avail_v4 & MBIM_IPCONF_HAS_DNSINFO) {
>   n = letoh32(ic->ipv4_ndnssrv);
>   off = letoh32(ic->ipv4_dnssrvoffs);
>   i = 0;
>   while (n-- > 0) {
> - if (off + sizeof (uint32_t) > len)
> + if (off + sizeof (addr) > len)
>   break;
>   memcpy(&addr, data + off, sizeof(addr));
>   if (i < UMB_MAX_DNSSRV)
> @@ -1798,29 +1920,95 @@ umb_decode_ip_configuration(struct umb_s
>      &addr, str, sizeof(str)));
>   }
>   }
> - umb_send_inet_proposal(sc);
> + umb_send_inet_proposal(sc, AF_INET);
>   }
> -
> - if ((avail & MBIM_IPCONF_HAS_MTUINFO)) {
> + if ((avail_v4 & MBIM_IPCONF_HAS_MTUINFO)) {
>   val = letoh32(ic->ipv4_mtu);
>   if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
> + hasmtu = 1;
>   ifp->if_hardmtu = val;
>   if (ifp->if_mtu > val)
>   ifp->if_mtu = val;
> - if (ifp->if_flags & IFF_DEBUG)
> - log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), val);
>   }
>   }
>  
> - avail = letoh32(ic->ipv6_available);
> - if ((ifp->if_flags & IFF_DEBUG) && avail & MBIM_IPCONF_HAS_ADDRINFO) {
> - /* XXX FIXME: IPv6 configuation missing */
> - log(LOG_INFO, "%s: ignoring IPv6 configuration\n", DEVNAM(sc));
> +tryv6:;
> +#ifdef INET6
> + /*
> + * IPv6 configuation
> + */
> + avail_v6 = letoh32(ic->ipv6_available);
> + if (avail_v6 == 0) {
> + if (ifp->if_flags & IFF_DEBUG)
> + log(LOG_INFO, "%s: ISP or WWAN module offers no IPv6 "
> +    "support\n", DEVNAM(ifp->if_softc));

Wouldn't the WWAN module return an error earlier hitting the
MBIM_STATUS_NO_DEVICE_SUPPORT in MBIM_CID_CONNECT? I would prefer we could
tell the user exactly why IPv6 did not configure. I think in my case it is
the ISP that is to blame but how do I know...

> + goto done;
> + }
> +
> + if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> +    (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
> + n = letoh32(ic->ipv6_naddr);
> + off = letoh32(ic->ipv6_addroffs);
> +
> + if (n == 0 || off + sizeof (ipv6elem) > len)
> + goto done;
> + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> +    DEVNAM(ifp->if_softc), n);
> +
> + /* Only pick the first one */
> + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> +
> + off = letoh32(ic->ipv6_gwoffs);
> + if (off + sizeof (gw6) > len)
> + goto done;
> + memcpy(&gw6, data + off, sizeof (gw6));
> +
> + rv = umb_add_inet6_config(sc, &addr6, ipv6elem.prefixlen, &gw6);
> + if (rv == 0)
> + state = UMB_S_UP;
> + }
> +
> + if (avail_v6 & MBIM_IPCONF_HAS_DNSINFO) {
> + n = letoh32(ic->ipv6_ndnssrv);
> + off = letoh32(ic->ipv6_dnssrvoffs);
> + i = 0;
> + while (n-- > 0) {
> + if (off + sizeof (addr6) > len)
> + break;
> + memcpy(&addr6, data + off, sizeof(addr6));
> + if (i < UMB_MAX_DNSSRV)
> + sc->sc_info.ipv6dns[i++] = addr6;
> + off += sizeof(addr6);
> + if (ifp->if_flags & IFF_DEBUG) {
> + char str[INET6_ADDRSTRLEN];
> + log(LOG_INFO, "%s: IPv6 nameserver %s\n",
> +    DEVNAM(ifp->if_softc), inet_ntop(AF_INET6,
> +    &addr6, str, sizeof(str)));
> + }
> + }
> + umb_send_inet_proposal(sc, AF_INET6);
> + }
> +
> + if ((avail_v6 & MBIM_IPCONF_HAS_MTUINFO)) {
> + val = letoh32(ic->ipv6_mtu);
> + if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
> + hasmtu = 1;
> + ifp->if_hardmtu = val;
> + if (ifp->if_mtu > val)
> + ifp->if_mtu = val;
> + }
>   }
> +#endif
> +
> +done:
> + if (hasmtu && (ifp->if_flags & IFF_DEBUG))
> + log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), ifp->if_hardmtu);
> +
>   if (state != -1)
>   umb_newstate(sc, state, 0);
>  
> -done:
>   splx(s);
>   return 1;
>  }
> @@ -2393,6 +2581,11 @@ umb_send_connect(struct umb_softc *sc, i
>   c->authprot = htole32(MBIM_AUTHPROT_NONE);
>   c->compression = htole32(MBIM_COMPRESSION_NONE);
>   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
> +#ifdef INET6
> + /* XXX FIXME: support IPv6-only mode, too */
> + if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
> + c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);

Is there a reason why MBIM_CONTEXT_IPTYPE_IPV4V6 is used and not
MBIM_CONTEXT_IPTYPE_IPV4ANDV6?

> +#endif
>   memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));
>   umb_cmd(sc, MBIM_CID_CONNECT, MBIM_CMDOP_SET, c, off);
>  done:
> @@ -2476,6 +2669,20 @@ umb_command_done(struct umb_softc *sc, v
>   switch (status) {
>   case MBIM_STATUS_SUCCESS:
>   break;
> +#ifdef INET6
> + case MBIM_STATUS_NO_DEVICE_SUPPORT:
> + if ((cid == MBIM_CID_CONNECT) &&
> +    (sc->sc_flags & UMBFLG_NO_INET6) == 0) {
> + sc->sc_flags |= UMBFLG_NO_INET6;
> + if (ifp->if_flags & IFF_DEBUG)
> + log(LOG_ERR,
> +    "%s: device does not support IPv6\n",
> +    DEVNAM(sc));
> + }
> + /* Re-trigger the connect, this time IPv4 only */
> + usb_add_task(sc->sc_udev, &sc->sc_umb_task);
> + return;
> +#endif
>   case MBIM_STATUS_NOT_INITIALIZED:
>   if (ifp->if_flags & IFF_DEBUG)
>   log(LOG_ERR, "%s: SIM not initialized (PIN missing)\n",
> Index: sys/dev/usb/if_umb.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.h,v
> retrieving revision 1.5
> diff -u -p -u -p -r1.5 if_umb.h
> --- sys/dev/usb/if_umb.h 26 Aug 2019 15:23:01 -0000 1.5
> +++ sys/dev/usb/if_umb.h 4 Feb 2020 07:30:43 -0000
> @@ -321,6 +321,7 @@ struct umb_info {
>  
>  #define UMB_MAX_DNSSRV 2
>   struct in_addr ipv4dns[UMB_MAX_DNSSRV];
> + struct in6_addr ipv6dns[UMB_MAX_DNSSRV];
>  };
>  
>  #ifdef _KERNEL
> @@ -345,6 +346,7 @@ struct umb_softc {
>   int sc_ndp_remainder;
>  
>  #define UMBFLG_FCC_AUTH_REQUIRED 0x0001
> +#define UMBFLG_NO_INET6 0x0002
>   uint32_t sc_flags;
>   int sc_cid;
>  
> @@ -380,6 +382,6 @@ struct umb_softc {
>  
>  #define sc_state sc_info.state
>  #define sc_roaming sc_info.enable_roaming
> - struct umb_info sc_info;
> + struct umb_info sc_info;

Extra space befor sc_info.

>  };
>  #endif /* _KERNEL */
>

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Gerhard Roth-2
Hi Claudio,

thanks for looking at it.

For your questions find my replies below.


On Mon, 17 Feb 2020 17:30:03 +0100 Claudio Jeker <[hidden email]> wrote:

> On Tue, Feb 04, 2020 at 09:16:34AM +0100, Gerhard Roth wrote:
> > Hi Alex,
> >
> > thanks for looking into it.
> >
> >
> > On Tue, 4 Feb 2020 00:20:42 +0100 Alexander Bluhm <[hidden email]> wrote:  
> > > On Tue, Jan 28, 2020 at 03:03:47PM +0100, Gerhard Roth wrote:  
> > > > this patch adds IPv6 support to umb(4).    
> > >
> > > It breaks my IPv4 setup.
> > >
> > > umb0 at uhub0 port 4 configuration 1 interface 6 "Lenovo H5321 gw" rev 2.00/0.00 addr 2
> > > provider Vodafone.de
> > > firmware CXP 901 8700/1 - R3C18
> > >
> > > When I apply the diff, my umb device does not get an IPv4 address.
> > >
> > > umb0: state going up from 'open' to 'radio on'
> > > umb0: none state unlocked (-1 attempts left)
> > > umb0: set/qry MBIM_CID_SUBSCRIBER_READY_STATUS failed: BUSY
> > > umb0: packet service changed from detached to attaching, class none, speed: 0 up / 0 down
> > > umb0: SIM initialized
> > > umb0: state going up from 'radio on' to 'SIM is ready'
> > > umb0: packet service changed from attaching to attached, class HSPA, speed: 5760000 up / 14400000 down
> > > umb0: state going up from 'SIM is ready' to 'attached'
> > > umb0: connecting ...
> > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT  
> >
> > That looks like the firmware of this Lenovo H5321 doesn't support IPv6.
> > Well at least it gives a decent error code.
> >
> >  
> > > umb0: state change timeout
> > > umb0: connecting ...
> > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > > umb0: state change timeout
> > > umb0: connecting ...
> > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > > umb0: state change timeout
> > > ...
> > >
> > > A few comments inline.
> > >  
> > > > +#ifdef INET6
> > > > +int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
> > > > +    u_int, struct in6_addr *);
> > > > +#endif    
> > >
> > > Usually I avoid #ifdef for prototypes.  It does not matter whether
> > > the compiler reads them and without #ifdef the code is nicer.  
> >
> > Removed it.
> >
> >  
> > >  
> > > > +tryv6:;    
> > >
> > > The ; is wrong.  
> >
> > Not really, because the label 'tryv6' is immediately followed by
> > an "#ifdef INET6". So looking just at this label I cannot directly tell
> > whether there is code that follows or not. And a label with no code
> > following is a syntax error in C.
> >
> > I just followed a old "C Style and Coding Standards for SunOS" paper
> > by Bill Shannon from 1993 that says:
> >
> > If a label is not followed by a program statement (e.g.
> > if the next token is a closing brace( } )) a NULL
> > statement ( ; ) must follow the label.
> >
> >  
> > >  
> > > > + if (n == 0 || off + sizeof (ipv6elem) > len)
> > > > + goto done;
> > > > + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> > > > + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> > > > +    DEVNAM(ifp->if_softc), n);
> > > > +
> > > > + /* Only pick the first one */
> > > > + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> > > > + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> > > > +
> > > > + off = letoh32(ic->ipv6_gwoffs);
> > > > + memcpy(&gw6, data + off, sizeof (gw6));    
> > >
> > > I think we should check the data length like above.
> > >
> > > if (off + sizeof (gw6) > len)
> > > goto done;
> > >
> > > And IPv4 should get the same check.  
> >
> > Thanks for spotting that. Added check to both cases.
> >
> >  
> > >  
> > > > @@ -380,6 +381,6 @@ struct umb_softc {
> > > >
> > > >  #define sc_state sc_info.state
> > > >  #define sc_roaming sc_info.enable_roaming
> > > > - struct umb_info sc_info;
> > > > + struct umb_info sc_info;
> > > >  };
> > > >  #endif /* _KERNEL */    
> > >
> > > This whitespace chunk is wrong.  
> >
> > I think it was wrong before. It's common idiom to add an extra space
> > to non-pointer members to keep the member names aligned, e.g.
> >
> > struct foo {
> > int *abc;
> > int def;
> > int *bar;
> > };
> >
> > Please correct me if I'm wrong.
> >  
> > >
> > > bluhm  
> >
> >
> > The updated patch below introduces a UMBFLG_NO_INET6 which is set on
> > receipt of a MBIM_STATUS_NO_DEVICE_SUPPORT in response to a
> > MBIM_CID_CONNECT. The code will then retry the connect operation in
> > IPv4-only mode.
> >
> > That won't give you any IPv6 support, but at least it won't break
> > your setup.
> >  
>
> A few whitespace fixes and some comments inline but apart from that
> OK claudio@
>
> > Index: sbin/ifconfig/ifconfig.c
> > ===================================================================
> > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> > retrieving revision 1.417
> > diff -u -p -u -p -r1.417 ifconfig.c
> > --- sbin/ifconfig/ifconfig.c 27 Dec 2019 14:34:46 -0000 1.417
> > +++ sbin/ifconfig/ifconfig.c 28 Jan 2020 12:16:23 -0000
> > @@ -5666,6 +5666,7 @@ umb_status(void)
> >   char apn[UMB_APN_MAXLEN+1];
> >   char pn[UMB_PHONENR_MAXLEN+1];
> >   int i, n;
> > + char astr[INET6_ADDRSTRLEN];
> >  
> >   memset((char *)&mi, 0, sizeof(mi));
> >   ifr.ifr_data = (caddr_t)&mi;
> > @@ -5830,7 +5831,15 @@ umb_status(void)
> >   for (i = 0, n = 0; i < UMB_MAX_DNSSRV; i++) {
> >   if (mi.ipv4dns[i].s_addr == INADDR_ANY)
> >   break;
> > - printf("%s %s", n++ ? "" : "\tdns", inet_ntoa(mi.ipv4dns[i]));
> > + printf("%s %s", n++ ? "" : "\tdns",
> > +    inet_ntop(AF_INET, &mi.ipv4dns[i], astr, sizeof (astr)));  
>
> Please no space between sizeof and (astr).
>
> > + }
> > + for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> > + if (memcmp(&mi.ipv6dns[i], &in6addr_any,
> > +    sizeof (mi.ipv6dns[i])) == 0)
> > + break;
> > + printf("%s %s", n++ ? "" : "\tdns",
> > +    inet_ntop(AF_INET6, &mi.ipv6dns[i], astr, sizeof (astr)));  
>
> Same here.
>
> >   }
> >   if (n)
> >   printf("\n");
> > Index: share/man/man4/umb.4
> > ===================================================================
> > RCS file: /cvs/src/share/man/man4/umb.4,v
> > retrieving revision 1.9
> > diff -u -p -u -p -r1.9 umb.4
> > --- share/man/man4/umb.4 23 Nov 2017 20:47:26 -0000 1.9
> > +++ share/man/man4/umb.4 28 Jan 2020 12:16:23 -0000
> > @@ -40,6 +40,11 @@ will remain in this state until the MBIM
> >  In case the device is connected to an "always-on" USB port,
> >  it may be possible to connect to a provider without entering the
> >  PIN again even if the system was rebooted.
> > +.Pp
> > +If the kernel has been compiled with INET6, the driver will try to
> > +obtain an IPv6 address from the provider. To succeed with the IPv6
> > +configuration, both the ISP and the MBIM device have to offer IPv6
> > +support.  
>
> Almost all our kernels have been compiled with INET6 support. I would
> remove that part and just go for:
> The driver will try to obtain an IPv6 address from the provider.
> To succeed with the IPv6 configuration, both the ISP and the MBIM device
> have to offer IPv6 support.
>
> Btw. new sentence new line in man pages.
>
> >  .Sh HARDWARE
> >  The following devices should work:
> >  .Pp
> > @@ -64,10 +69,6 @@ The following devices should work:
> >  .%U http://www.usb.org/developers/docs/devclass_docs/MBIM10Errata1_073013.zip
> >  .Re
> >  .Sh CAVEATS
> > -The
> > -.Nm
> > -driver does not support IPv6.
> > -.Pp
> >  Devices which fail to provide a conforming MBIM implementation will
> >  probably be attached as some other driver, such as
> >  .Xr umsm 4 .
> > Index: sys/dev/usb/if_umb.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> > retrieving revision 1.31
> > diff -u -p -u -p -r1.31 if_umb.c
> > --- sys/dev/usb/if_umb.c 26 Nov 2019 23:04:28 -0000 1.31
> > +++ sys/dev/usb/if_umb.c 4 Feb 2020 07:50:30 -0000
> > @@ -43,6 +43,14 @@
> >  #include <netinet/in_var.h>
> >  #include <netinet/ip.h>
> >  
> > +#ifdef INET6
> > +#include <netinet/ip6.h>
> > +#include <netinet6/in6_var.h>
> > +#include <netinet6/ip6_var.h>
> > +#include <netinet6/in6_ifattach.h>
> > +#include <netinet6/nd6.h>
> > +#endif
> > +
> >  #include <machine/bus.h>
> >  
> >  #include <dev/usb/usb.h>
> > @@ -158,7 +166,9 @@ int umb_decode_connect_info(struct umb
> >  void umb_clear_addr(struct umb_softc *);
> >  int umb_add_inet_config(struct umb_softc *, struct in_addr, u_int,
> >      struct in_addr);
> > -void umb_send_inet_proposal(struct umb_softc *);
> > +int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
> > +    u_int, struct in6_addr *);
> > +void umb_send_inet_proposal(struct umb_softc *, int);
> >  int umb_decode_ip_configuration(struct umb_softc *, void *, int);
> >  void umb_rx(struct umb_softc *);
> >  void umb_rxeof(struct usbd_xfer *, void *, usbd_status);
> > @@ -800,8 +810,8 @@ umb_input(struct ifnet *ifp, struct mbuf
> >  #endif /* INET6 */
> >   default:
> >   ifp->if_ierrors++;
> > - DPRINTFN(4, "%s: dropping packet with bad IP version (%d)\n",
> > -    __func__, ipv);
> > + DPRINTFN(4, "%s: dropping packet with bad IP version (af %d)\n",
> > +    __func__, af);
> >   m_freem(m);
> >   return 1;
> >   }
> > @@ -902,7 +912,10 @@ umb_rtrequest(struct ifnet *ifp, int req
> >   struct umb_softc *sc = ifp->if_softc;
> >  
> >   if (req == RTM_PROPOSAL) {
> > - umb_send_inet_proposal(sc);
> > + umb_send_inet_proposal(sc, AF_INET);
> > +#ifdef INET6
> > + umb_send_inet_proposal(sc, AF_INET6);
> > +#endif
> >   return;
> >   }
> >  
> > @@ -1596,11 +1609,6 @@ umb_decode_connect_info(struct umb_softc
> >   if (ifp->if_flags & IFF_DEBUG)
> >   log(LOG_INFO, "%s: connection %s\n", DEVNAM(sc),
> >      umb_activation(act));
> > - if ((ifp->if_flags & IFF_DEBUG) &&
> > -    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_DEFAULT &&
> > -    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_IPV4)
> > - log(LOG_DEBUG, "%s: got iptype %d connection\n",
> > -    DEVNAM(sc), letoh32(ci->iptype));
> >  
> >   sc->sc_info.activation = act;
> >   sc->sc_info.nwerror = letoh32(ci->nwerror);
> > @@ -1621,9 +1629,16 @@ umb_clear_addr(struct umb_softc *sc)
> >   struct ifnet *ifp = GET_IFP(sc);
> >  
> >   memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> > - umb_send_inet_proposal(sc);
> > + memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
> > + umb_send_inet_proposal(sc, AF_INET);
> > +#ifdef INET6
> > + umb_send_inet_proposal(sc, AF_INET6);
> > +#endif
> >   NET_LOCK();
> >   in_ifdetach(ifp);
> > +#ifdef INET6
> > + in6_ifdetach(ifp);
> > +#endif
> >   NET_UNLOCK();
> >  }
> >  
> > @@ -1698,29 +1713,125 @@ umb_add_inet_config(struct umb_softc *sc
> >      sockaddr_ntop(sintosa(&ifra.ifra_dstaddr), str[2],
> >      sizeof(str[2])));
> >   }
> > - return rv;
> > + return 0;
> > +}
> > +
> > +#ifdef INET6
> > +int
> > +umb_add_inet6_config(struct umb_softc *sc, struct in6_addr *ip, u_int prefixlen,
> > +    struct in6_addr *gw)
> > +{
> > + struct ifnet *ifp = GET_IFP(sc);
> > + struct in6_aliasreq ifra;
> > + struct sockaddr_in6 *sin6, default_sin6;
> > + struct rt_addrinfo info;
> > + struct rtentry *rt;
> > + int rv;
> > +
> > + memset(&ifra, 0, sizeof (ifra));
> > + sin6 = &ifra.ifra_addr;
> > + sin6->sin6_family = AF_INET6;
> > + sin6->sin6_len = sizeof (*sin6);
> > + memcpy(&sin6->sin6_addr, ip, sizeof (sin6->sin6_addr));
> > +
> > + sin6 = &ifra.ifra_dstaddr;
> > + sin6->sin6_family = AF_INET6;
> > + sin6->sin6_len = sizeof (*sin6);
> > + memcpy(&sin6->sin6_addr, gw, sizeof (sin6->sin6_addr));
> > +
> > + /* XXX: in6_update_ifa() accepts only 128 bits for P2P interfaces. */
> > + prefixlen = 128;
> > +
> > + sin6 = &ifra.ifra_prefixmask;
> > + sin6->sin6_family = AF_INET6;
> > + sin6->sin6_len = sizeof (*sin6);
> > + in6_prefixlen2mask(&sin6->sin6_addr, prefixlen);
> > +
> > + ifra.ifra_lifetime.ia6t_vltime = ND6_INFINITE_LIFETIME;
> > + ifra.ifra_lifetime.ia6t_pltime = ND6_INFINITE_LIFETIME;
> > +
> > + rv = in6_ioctl(SIOCAIFADDR_IN6, (caddr_t)&ifra, ifp, 1);
> > + if (rv != 0) {
> > + printf("%s: unable to set IPv6 address, error %d\n",
> > +    DEVNAM(ifp->if_softc), rv);
> > + return rv;
> > + }
> > +
> > + memset(&default_sin6, 0, sizeof(default_sin6));
> > + default_sin6.sin6_family = AF_INET6;
> > + default_sin6.sin6_len = sizeof (default_sin6);
> > +
> > + memset(&info, 0, sizeof(info));
> > + info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> > + info.rti_ifa = ifa_ifwithaddr(sin6tosa(&ifra.ifra_addr),
> > +    ifp->if_rdomain);
> > + info.rti_info[RTAX_DST] = sin6tosa(&default_sin6);
> > + info.rti_info[RTAX_NETMASK] = sin6tosa(&default_sin6);
> > + info.rti_info[RTAX_GATEWAY] = sin6tosa(&ifra.ifra_dstaddr);
> > +
> > + NET_LOCK();
> > + rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
> > + NET_UNLOCK();
> > + if (rv) {
> > + printf("%s: unable to set IPv6 default route, "
> > +    "error %d\n", DEVNAM(ifp->if_softc), rv);
> > + rtm_miss(RTM_MISS, &info, 0, RTP_NONE, 0, rv,
> > +    ifp->if_rdomain);
> > + } else {
> > + /* Inform listeners of the new route */
> > + rtm_send(rt, RTM_ADD, rv, ifp->if_rdomain);
> > + rtfree(rt);
> > + }
> > +
> > + if (ifp->if_flags & IFF_DEBUG) {
> > + char str[3][INET6_ADDRSTRLEN];
> > + log(LOG_INFO, "%s: IPv6 addr %s, mask %s, gateway %s\n",
> > +    DEVNAM(ifp->if_softc),
> > +    sockaddr_ntop(sin6tosa(&ifra.ifra_addr), str[0],
> > +    sizeof(str[0])),
> > +    sockaddr_ntop(sin6tosa(&ifra.ifra_prefixmask), str[1],
> > +    sizeof(str[1])),
> > +    sockaddr_ntop(sin6tosa(&ifra.ifra_dstaddr), str[2],
> > +    sizeof(str[2])));
> > + }
> > + return 0;
> >  }
> > +#endif
> >  
> >  void
> > -umb_send_inet_proposal(struct umb_softc *sc)
> > +umb_send_inet_proposal(struct umb_softc *sc, int af)
> >  {
> >   struct ifnet *ifp = GET_IFP(sc);
> >   struct sockaddr_rtdns rtdns;
> >   struct rt_addrinfo info;
> >   int i, flag = 0;
> > + size_t sz = 0;
> >  
> >   memset(&rtdns, 0, sizeof(rtdns));
> >   memset(&info, 0, sizeof(info));
> >  
> >   for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> > - if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
> > - break;
> > - memcpy(rtdns.sr_dns + i * sizeof(struct in_addr),
> > -    &sc->sc_info.ipv4dns[i], sizeof(struct in_addr));
> > - flag = RTF_UP;
> > + if (af == AF_INET) {
> > + sz = sizeof (sc->sc_info.ipv4dns[i]);
> > + if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
> > + break;
> > + memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv4dns[i],
> > +    sz);
> > + flag = RTF_UP;
> > +#ifdef INET6
> > + } if (af == AF_INET6) {  
>
> Either make this an else if or add a newline between the } and if.
>
> > + sz = sizeof (sc->sc_info.ipv6dns[i]);
> > + if (IN6_ARE_ADDR_EQUAL(&sc->sc_info.ipv6dns[i],
> > +    &in6addr_any))
> > + break;
> > + memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv6dns[i],
> > +    sz);
> > + flag = RTF_UP;
> > +#endif
> > + }
> >   }
> > - rtdns.sr_family = AF_INET;
> > - rtdns.sr_len = 2 + i * sizeof(struct in_addr);
> > + rtdns.sr_family = af;
> > + rtdns.sr_len = 2 + i * sz;
> >   info.rti_info[RTAX_DNS] = srtdnstosa(&rtdns);
> >  
> >   rtm_proposal(ifp, &info, flag, RTP_PROPOSAL_UMB);
> > @@ -1732,7 +1843,7 @@ umb_decode_ip_configuration(struct umb_s
> >   struct mbim_cid_ip_configuration_info *ic = data;
> >   struct ifnet *ifp = GET_IFP(sc);
> >   int s;
> > - uint32_t avail;
> > + uint32_t avail_v4;
> >   uint32_t val;
> >   int n, i;
> >   int off;
> > @@ -1740,6 +1851,12 @@ umb_decode_ip_configuration(struct umb_s
> >   struct in_addr addr, gw;
> >   int state = -1;
> >   int rv;
> > + int hasmtu = 0;
> > +#ifdef INET6
> > + uint32_t avail_v6;
> > + struct mbim_cid_ipv6_element ipv6elem;
> > + struct in6_addr addr6, gw6;
> > +#endif
> >  
> >   if (len < sizeof (*ic))
> >   return 0;
> > @@ -1750,17 +1867,20 @@ umb_decode_ip_configuration(struct umb_s
> >   }
> >   s = splnet();
> >  
> > + memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> > + memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
> > +
> >   /*
> >   * IPv4 configuation
> >   */
> > - avail = letoh32(ic->ipv4_available);
> > - if ((avail & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> > + avail_v4 = letoh32(ic->ipv4_available);
> > + if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> >      (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
> >   n = letoh32(ic->ipv4_naddr);
> >   off = letoh32(ic->ipv4_addroffs);
> >  
> >   if (n == 0 || off + sizeof (ipv4elem) > len)
> > - goto done;
> > + goto tryv6;
> >   if (n != 1 && ifp->if_flags & IFF_DEBUG)
> >   log(LOG_INFO, "%s: more than one IPv4 addr: %d\n",
> >      DEVNAM(ifp->if_softc), n);
> > @@ -1771,6 +1891,8 @@ umb_decode_ip_configuration(struct umb_s
> >   addr.s_addr = ipv4elem.addr;
> >  
> >   off = letoh32(ic->ipv4_gwoffs);
> > + if (off + sizeof (gw) > len)
> > + goto done;
> >   memcpy(&gw, data + off, sizeof(gw));
> >  
> >   rv = umb_add_inet_config(sc, addr, ipv4elem.prefixlen, gw);
> > @@ -1780,12 +1902,12 @@ umb_decode_ip_configuration(struct umb_s
> >   }
> >  
> >   memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> > - if (avail & MBIM_IPCONF_HAS_DNSINFO) {
> > + if (avail_v4 & MBIM_IPCONF_HAS_DNSINFO) {
> >   n = letoh32(ic->ipv4_ndnssrv);
> >   off = letoh32(ic->ipv4_dnssrvoffs);
> >   i = 0;
> >   while (n-- > 0) {
> > - if (off + sizeof (uint32_t) > len)
> > + if (off + sizeof (addr) > len)
> >   break;
> >   memcpy(&addr, data + off, sizeof(addr));
> >   if (i < UMB_MAX_DNSSRV)
> > @@ -1798,29 +1920,95 @@ umb_decode_ip_configuration(struct umb_s
> >      &addr, str, sizeof(str)));
> >   }
> >   }
> > - umb_send_inet_proposal(sc);
> > + umb_send_inet_proposal(sc, AF_INET);
> >   }
> > -
> > - if ((avail & MBIM_IPCONF_HAS_MTUINFO)) {
> > + if ((avail_v4 & MBIM_IPCONF_HAS_MTUINFO)) {
> >   val = letoh32(ic->ipv4_mtu);
> >   if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
> > + hasmtu = 1;
> >   ifp->if_hardmtu = val;
> >   if (ifp->if_mtu > val)
> >   ifp->if_mtu = val;
> > - if (ifp->if_flags & IFF_DEBUG)
> > - log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), val);
> >   }
> >   }
> >  
> > - avail = letoh32(ic->ipv6_available);
> > - if ((ifp->if_flags & IFF_DEBUG) && avail & MBIM_IPCONF_HAS_ADDRINFO) {
> > - /* XXX FIXME: IPv6 configuation missing */
> > - log(LOG_INFO, "%s: ignoring IPv6 configuration\n", DEVNAM(sc));
> > +tryv6:;
> > +#ifdef INET6
> > + /*
> > + * IPv6 configuation
> > + */
> > + avail_v6 = letoh32(ic->ipv6_available);
> > + if (avail_v6 == 0) {
> > + if (ifp->if_flags & IFF_DEBUG)
> > + log(LOG_INFO, "%s: ISP or WWAN module offers no IPv6 "
> > +    "support\n", DEVNAM(ifp->if_softc));  
>
> Wouldn't the WWAN module return an error earlier hitting the
> MBIM_STATUS_NO_DEVICE_SUPPORT in MBIM_CID_CONNECT? I would prefer we could
> tell the user exactly why IPv6 did not configure. I think in my case it is
> the ISP that is to blame but how do I know...

I have a T-Mobile SIM card and can get an IPv6 address when used in a
SIMCOM module. Using the same SIM card in an older Sierra Wireless
EM8805 I get neither an IPv6 address, nor an error from the module.

Unfortunately, it seems we have to confine ourselves to not knowing.


>
> > + goto done;
> > + }
> > +
> > + if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> > +    (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
> > + n = letoh32(ic->ipv6_naddr);
> > + off = letoh32(ic->ipv6_addroffs);
> > +
> > + if (n == 0 || off + sizeof (ipv6elem) > len)
> > + goto done;
> > + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> > + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> > +    DEVNAM(ifp->if_softc), n);
> > +
> > + /* Only pick the first one */
> > + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> > + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> > +
> > + off = letoh32(ic->ipv6_gwoffs);
> > + if (off + sizeof (gw6) > len)
> > + goto done;
> > + memcpy(&gw6, data + off, sizeof (gw6));
> > +
> > + rv = umb_add_inet6_config(sc, &addr6, ipv6elem.prefixlen, &gw6);
> > + if (rv == 0)
> > + state = UMB_S_UP;
> > + }
> > +
> > + if (avail_v6 & MBIM_IPCONF_HAS_DNSINFO) {
> > + n = letoh32(ic->ipv6_ndnssrv);
> > + off = letoh32(ic->ipv6_dnssrvoffs);
> > + i = 0;
> > + while (n-- > 0) {
> > + if (off + sizeof (addr6) > len)
> > + break;
> > + memcpy(&addr6, data + off, sizeof(addr6));
> > + if (i < UMB_MAX_DNSSRV)
> > + sc->sc_info.ipv6dns[i++] = addr6;
> > + off += sizeof(addr6);
> > + if (ifp->if_flags & IFF_DEBUG) {
> > + char str[INET6_ADDRSTRLEN];
> > + log(LOG_INFO, "%s: IPv6 nameserver %s\n",
> > +    DEVNAM(ifp->if_softc), inet_ntop(AF_INET6,
> > +    &addr6, str, sizeof(str)));
> > + }
> > + }
> > + umb_send_inet_proposal(sc, AF_INET6);
> > + }
> > +
> > + if ((avail_v6 & MBIM_IPCONF_HAS_MTUINFO)) {
> > + val = letoh32(ic->ipv6_mtu);
> > + if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
> > + hasmtu = 1;
> > + ifp->if_hardmtu = val;
> > + if (ifp->if_mtu > val)
> > + ifp->if_mtu = val;
> > + }
> >   }
> > +#endif
> > +
> > +done:
> > + if (hasmtu && (ifp->if_flags & IFF_DEBUG))
> > + log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), ifp->if_hardmtu);
> > +
> >   if (state != -1)
> >   umb_newstate(sc, state, 0);
> >  
> > -done:
> >   splx(s);
> >   return 1;
> >  }
> > @@ -2393,6 +2581,11 @@ umb_send_connect(struct umb_softc *sc, i
> >   c->authprot = htole32(MBIM_AUTHPROT_NONE);
> >   c->compression = htole32(MBIM_COMPRESSION_NONE);
> >   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
> > +#ifdef INET6
> > + /* XXX FIXME: support IPv6-only mode, too */
> > + if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
> > + c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);  
>
> Is there a reason why MBIM_CONTEXT_IPTYPE_IPV4V6 is used and not
> MBIM_CONTEXT_IPTYPE_IPV4ANDV6?


Yes, I tried MBIM_CONTEXT_IPTYPE_IPV4ANDV6 myself first but to no
avail. The switched to MBIM_CONTEXT_IPTYPE_IPV4V6 and everything
was fine.


Gerhard


>
> > +#endif
> >   memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));
> >   umb_cmd(sc, MBIM_CID_CONNECT, MBIM_CMDOP_SET, c, off);
> >  done:
> > @@ -2476,6 +2669,20 @@ umb_command_done(struct umb_softc *sc, v
> >   switch (status) {
> >   case MBIM_STATUS_SUCCESS:
> >   break;
> > +#ifdef INET6
> > + case MBIM_STATUS_NO_DEVICE_SUPPORT:
> > + if ((cid == MBIM_CID_CONNECT) &&
> > +    (sc->sc_flags & UMBFLG_NO_INET6) == 0) {
> > + sc->sc_flags |= UMBFLG_NO_INET6;
> > + if (ifp->if_flags & IFF_DEBUG)
> > + log(LOG_ERR,
> > +    "%s: device does not support IPv6\n",
> > +    DEVNAM(sc));
> > + }
> > + /* Re-trigger the connect, this time IPv4 only */
> > + usb_add_task(sc->sc_udev, &sc->sc_umb_task);
> > + return;
> > +#endif
> >   case MBIM_STATUS_NOT_INITIALIZED:
> >   if (ifp->if_flags & IFF_DEBUG)
> >   log(LOG_ERR, "%s: SIM not initialized (PIN missing)\n",
> > Index: sys/dev/usb/if_umb.h
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/usb/if_umb.h,v
> > retrieving revision 1.5
> > diff -u -p -u -p -r1.5 if_umb.h
> > --- sys/dev/usb/if_umb.h 26 Aug 2019 15:23:01 -0000 1.5
> > +++ sys/dev/usb/if_umb.h 4 Feb 2020 07:30:43 -0000
> > @@ -321,6 +321,7 @@ struct umb_info {
> >  
> >  #define UMB_MAX_DNSSRV 2
> >   struct in_addr ipv4dns[UMB_MAX_DNSSRV];
> > + struct in6_addr ipv6dns[UMB_MAX_DNSSRV];
> >  };
> >  
> >  #ifdef _KERNEL
> > @@ -345,6 +346,7 @@ struct umb_softc {
> >   int sc_ndp_remainder;
> >  
> >  #define UMBFLG_FCC_AUTH_REQUIRED 0x0001
> > +#define UMBFLG_NO_INET6 0x0002
> >   uint32_t sc_flags;
> >   int sc_cid;
> >  
> > @@ -380,6 +382,6 @@ struct umb_softc {
> >  
> >  #define sc_state sc_info.state
> >  #define sc_roaming sc_info.enable_roaming
> > - struct umb_info sc_info;
> > + struct umb_info sc_info;  
>
> Extra space befor sc_info.
>
> >  };
> >  #endif /* _KERNEL */
> >  
>

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Claudio Jeker
On Tue, Feb 18, 2020 at 08:25:48AM +0100, Gerhard Roth wrote:
> Hi Claudio,
>
> thanks for looking at it.
>
> For your questions find my replies below.
>

Thanks. Bummer about not knowing what the cause of no IPv6 config is.
I guess it is time to get this in an have people play with it.

>
> On Mon, 17 Feb 2020 17:30:03 +0100 Claudio Jeker <[hidden email]> wrote:
> > On Tue, Feb 04, 2020 at 09:16:34AM +0100, Gerhard Roth wrote:
> > > Hi Alex,
> > >
> > > thanks for looking into it.
> > >
> > >
> > > On Tue, 4 Feb 2020 00:20:42 +0100 Alexander Bluhm <[hidden email]> wrote:  
> > > > On Tue, Jan 28, 2020 at 03:03:47PM +0100, Gerhard Roth wrote:  
> > > > > this patch adds IPv6 support to umb(4).    
> > > >
> > > > It breaks my IPv4 setup.
> > > >
> > > > umb0 at uhub0 port 4 configuration 1 interface 6 "Lenovo H5321 gw" rev 2.00/0.00 addr 2
> > > > provider Vodafone.de
> > > > firmware CXP 901 8700/1 - R3C18
> > > >
> > > > When I apply the diff, my umb device does not get an IPv4 address.
> > > >
> > > > umb0: state going up from 'open' to 'radio on'
> > > > umb0: none state unlocked (-1 attempts left)
> > > > umb0: set/qry MBIM_CID_SUBSCRIBER_READY_STATUS failed: BUSY
> > > > umb0: packet service changed from detached to attaching, class none, speed: 0 up / 0 down
> > > > umb0: SIM initialized
> > > > umb0: state going up from 'radio on' to 'SIM is ready'
> > > > umb0: packet service changed from attaching to attached, class HSPA, speed: 5760000 up / 14400000 down
> > > > umb0: state going up from 'SIM is ready' to 'attached'
> > > > umb0: connecting ...
> > > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT  
> > >
> > > That looks like the firmware of this Lenovo H5321 doesn't support IPv6.
> > > Well at least it gives a decent error code.
> > >
> > >  
> > > > umb0: state change timeout
> > > > umb0: connecting ...
> > > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > > > umb0: state change timeout
> > > > umb0: connecting ...
> > > > umb0: set/qry MBIM_CID_CONNECT failed: NO_DEVICE_SUPPORT
> > > > umb0: state change timeout
> > > > ...
> > > >
> > > > A few comments inline.
> > > >  
> > > > > +#ifdef INET6
> > > > > +int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
> > > > > +    u_int, struct in6_addr *);
> > > > > +#endif    
> > > >
> > > > Usually I avoid #ifdef for prototypes.  It does not matter whether
> > > > the compiler reads them and without #ifdef the code is nicer.  
> > >
> > > Removed it.
> > >
> > >  
> > > >  
> > > > > +tryv6:;    
> > > >
> > > > The ; is wrong.  
> > >
> > > Not really, because the label 'tryv6' is immediately followed by
> > > an "#ifdef INET6". So looking just at this label I cannot directly tell
> > > whether there is code that follows or not. And a label with no code
> > > following is a syntax error in C.
> > >
> > > I just followed a old "C Style and Coding Standards for SunOS" paper
> > > by Bill Shannon from 1993 that says:
> > >
> > > If a label is not followed by a program statement (e.g.
> > > if the next token is a closing brace( } )) a NULL
> > > statement ( ; ) must follow the label.
> > >
> > >  
> > > >  
> > > > > + if (n == 0 || off + sizeof (ipv6elem) > len)
> > > > > + goto done;
> > > > > + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> > > > > + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> > > > > +    DEVNAM(ifp->if_softc), n);
> > > > > +
> > > > > + /* Only pick the first one */
> > > > > + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> > > > > + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> > > > > +
> > > > > + off = letoh32(ic->ipv6_gwoffs);
> > > > > + memcpy(&gw6, data + off, sizeof (gw6));    
> > > >
> > > > I think we should check the data length like above.
> > > >
> > > > if (off + sizeof (gw6) > len)
> > > > goto done;
> > > >
> > > > And IPv4 should get the same check.  
> > >
> > > Thanks for spotting that. Added check to both cases.
> > >
> > >  
> > > >  
> > > > > @@ -380,6 +381,6 @@ struct umb_softc {
> > > > >
> > > > >  #define sc_state sc_info.state
> > > > >  #define sc_roaming sc_info.enable_roaming
> > > > > - struct umb_info sc_info;
> > > > > + struct umb_info sc_info;
> > > > >  };
> > > > >  #endif /* _KERNEL */    
> > > >
> > > > This whitespace chunk is wrong.  
> > >
> > > I think it was wrong before. It's common idiom to add an extra space
> > > to non-pointer members to keep the member names aligned, e.g.
> > >
> > > struct foo {
> > > int *abc;
> > > int def;
> > > int *bar;
> > > };
> > >
> > > Please correct me if I'm wrong.
> > >  
> > > >
> > > > bluhm  
> > >
> > >
> > > The updated patch below introduces a UMBFLG_NO_INET6 which is set on
> > > receipt of a MBIM_STATUS_NO_DEVICE_SUPPORT in response to a
> > > MBIM_CID_CONNECT. The code will then retry the connect operation in
> > > IPv4-only mode.
> > >
> > > That won't give you any IPv6 support, but at least it won't break
> > > your setup.
> > >  
> >
> > A few whitespace fixes and some comments inline but apart from that
> > OK claudio@
> >
> > > Index: sbin/ifconfig/ifconfig.c
> > > ===================================================================
> > > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> > > retrieving revision 1.417
> > > diff -u -p -u -p -r1.417 ifconfig.c
> > > --- sbin/ifconfig/ifconfig.c 27 Dec 2019 14:34:46 -0000 1.417
> > > +++ sbin/ifconfig/ifconfig.c 28 Jan 2020 12:16:23 -0000
> > > @@ -5666,6 +5666,7 @@ umb_status(void)
> > >   char apn[UMB_APN_MAXLEN+1];
> > >   char pn[UMB_PHONENR_MAXLEN+1];
> > >   int i, n;
> > > + char astr[INET6_ADDRSTRLEN];
> > >  
> > >   memset((char *)&mi, 0, sizeof(mi));
> > >   ifr.ifr_data = (caddr_t)&mi;
> > > @@ -5830,7 +5831,15 @@ umb_status(void)
> > >   for (i = 0, n = 0; i < UMB_MAX_DNSSRV; i++) {
> > >   if (mi.ipv4dns[i].s_addr == INADDR_ANY)
> > >   break;
> > > - printf("%s %s", n++ ? "" : "\tdns", inet_ntoa(mi.ipv4dns[i]));
> > > + printf("%s %s", n++ ? "" : "\tdns",
> > > +    inet_ntop(AF_INET, &mi.ipv4dns[i], astr, sizeof (astr)));  
> >
> > Please no space between sizeof and (astr).
> >
> > > + }
> > > + for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> > > + if (memcmp(&mi.ipv6dns[i], &in6addr_any,
> > > +    sizeof (mi.ipv6dns[i])) == 0)
> > > + break;
> > > + printf("%s %s", n++ ? "" : "\tdns",
> > > +    inet_ntop(AF_INET6, &mi.ipv6dns[i], astr, sizeof (astr)));  
> >
> > Same here.
> >
> > >   }
> > >   if (n)
> > >   printf("\n");
> > > Index: share/man/man4/umb.4
> > > ===================================================================
> > > RCS file: /cvs/src/share/man/man4/umb.4,v
> > > retrieving revision 1.9
> > > diff -u -p -u -p -r1.9 umb.4
> > > --- share/man/man4/umb.4 23 Nov 2017 20:47:26 -0000 1.9
> > > +++ share/man/man4/umb.4 28 Jan 2020 12:16:23 -0000
> > > @@ -40,6 +40,11 @@ will remain in this state until the MBIM
> > >  In case the device is connected to an "always-on" USB port,
> > >  it may be possible to connect to a provider without entering the
> > >  PIN again even if the system was rebooted.
> > > +.Pp
> > > +If the kernel has been compiled with INET6, the driver will try to
> > > +obtain an IPv6 address from the provider. To succeed with the IPv6
> > > +configuration, both the ISP and the MBIM device have to offer IPv6
> > > +support.  
> >
> > Almost all our kernels have been compiled with INET6 support. I would
> > remove that part and just go for:
> > The driver will try to obtain an IPv6 address from the provider.
> > To succeed with the IPv6 configuration, both the ISP and the MBIM device
> > have to offer IPv6 support.
> >
> > Btw. new sentence new line in man pages.
> >
> > >  .Sh HARDWARE
> > >  The following devices should work:
> > >  .Pp
> > > @@ -64,10 +69,6 @@ The following devices should work:
> > >  .%U http://www.usb.org/developers/docs/devclass_docs/MBIM10Errata1_073013.zip
> > >  .Re
> > >  .Sh CAVEATS
> > > -The
> > > -.Nm
> > > -driver does not support IPv6.
> > > -.Pp
> > >  Devices which fail to provide a conforming MBIM implementation will
> > >  probably be attached as some other driver, such as
> > >  .Xr umsm 4 .
> > > Index: sys/dev/usb/if_umb.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> > > retrieving revision 1.31
> > > diff -u -p -u -p -r1.31 if_umb.c
> > > --- sys/dev/usb/if_umb.c 26 Nov 2019 23:04:28 -0000 1.31
> > > +++ sys/dev/usb/if_umb.c 4 Feb 2020 07:50:30 -0000
> > > @@ -43,6 +43,14 @@
> > >  #include <netinet/in_var.h>
> > >  #include <netinet/ip.h>
> > >  
> > > +#ifdef INET6
> > > +#include <netinet/ip6.h>
> > > +#include <netinet6/in6_var.h>
> > > +#include <netinet6/ip6_var.h>
> > > +#include <netinet6/in6_ifattach.h>
> > > +#include <netinet6/nd6.h>
> > > +#endif
> > > +
> > >  #include <machine/bus.h>
> > >  
> > >  #include <dev/usb/usb.h>
> > > @@ -158,7 +166,9 @@ int umb_decode_connect_info(struct umb
> > >  void umb_clear_addr(struct umb_softc *);
> > >  int umb_add_inet_config(struct umb_softc *, struct in_addr, u_int,
> > >      struct in_addr);
> > > -void umb_send_inet_proposal(struct umb_softc *);
> > > +int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
> > > +    u_int, struct in6_addr *);
> > > +void umb_send_inet_proposal(struct umb_softc *, int);
> > >  int umb_decode_ip_configuration(struct umb_softc *, void *, int);
> > >  void umb_rx(struct umb_softc *);
> > >  void umb_rxeof(struct usbd_xfer *, void *, usbd_status);
> > > @@ -800,8 +810,8 @@ umb_input(struct ifnet *ifp, struct mbuf
> > >  #endif /* INET6 */
> > >   default:
> > >   ifp->if_ierrors++;
> > > - DPRINTFN(4, "%s: dropping packet with bad IP version (%d)\n",
> > > -    __func__, ipv);
> > > + DPRINTFN(4, "%s: dropping packet with bad IP version (af %d)\n",
> > > +    __func__, af);
> > >   m_freem(m);
> > >   return 1;
> > >   }
> > > @@ -902,7 +912,10 @@ umb_rtrequest(struct ifnet *ifp, int req
> > >   struct umb_softc *sc = ifp->if_softc;
> > >  
> > >   if (req == RTM_PROPOSAL) {
> > > - umb_send_inet_proposal(sc);
> > > + umb_send_inet_proposal(sc, AF_INET);
> > > +#ifdef INET6
> > > + umb_send_inet_proposal(sc, AF_INET6);
> > > +#endif
> > >   return;
> > >   }
> > >  
> > > @@ -1596,11 +1609,6 @@ umb_decode_connect_info(struct umb_softc
> > >   if (ifp->if_flags & IFF_DEBUG)
> > >   log(LOG_INFO, "%s: connection %s\n", DEVNAM(sc),
> > >      umb_activation(act));
> > > - if ((ifp->if_flags & IFF_DEBUG) &&
> > > -    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_DEFAULT &&
> > > -    letoh32(ci->iptype) != MBIM_CONTEXT_IPTYPE_IPV4)
> > > - log(LOG_DEBUG, "%s: got iptype %d connection\n",
> > > -    DEVNAM(sc), letoh32(ci->iptype));
> > >  
> > >   sc->sc_info.activation = act;
> > >   sc->sc_info.nwerror = letoh32(ci->nwerror);
> > > @@ -1621,9 +1629,16 @@ umb_clear_addr(struct umb_softc *sc)
> > >   struct ifnet *ifp = GET_IFP(sc);
> > >  
> > >   memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> > > - umb_send_inet_proposal(sc);
> > > + memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
> > > + umb_send_inet_proposal(sc, AF_INET);
> > > +#ifdef INET6
> > > + umb_send_inet_proposal(sc, AF_INET6);
> > > +#endif
> > >   NET_LOCK();
> > >   in_ifdetach(ifp);
> > > +#ifdef INET6
> > > + in6_ifdetach(ifp);
> > > +#endif
> > >   NET_UNLOCK();
> > >  }
> > >  
> > > @@ -1698,29 +1713,125 @@ umb_add_inet_config(struct umb_softc *sc
> > >      sockaddr_ntop(sintosa(&ifra.ifra_dstaddr), str[2],
> > >      sizeof(str[2])));
> > >   }
> > > - return rv;
> > > + return 0;
> > > +}
> > > +
> > > +#ifdef INET6
> > > +int
> > > +umb_add_inet6_config(struct umb_softc *sc, struct in6_addr *ip, u_int prefixlen,
> > > +    struct in6_addr *gw)
> > > +{
> > > + struct ifnet *ifp = GET_IFP(sc);
> > > + struct in6_aliasreq ifra;
> > > + struct sockaddr_in6 *sin6, default_sin6;
> > > + struct rt_addrinfo info;
> > > + struct rtentry *rt;
> > > + int rv;
> > > +
> > > + memset(&ifra, 0, sizeof (ifra));
> > > + sin6 = &ifra.ifra_addr;
> > > + sin6->sin6_family = AF_INET6;
> > > + sin6->sin6_len = sizeof (*sin6);
> > > + memcpy(&sin6->sin6_addr, ip, sizeof (sin6->sin6_addr));
> > > +
> > > + sin6 = &ifra.ifra_dstaddr;
> > > + sin6->sin6_family = AF_INET6;
> > > + sin6->sin6_len = sizeof (*sin6);
> > > + memcpy(&sin6->sin6_addr, gw, sizeof (sin6->sin6_addr));
> > > +
> > > + /* XXX: in6_update_ifa() accepts only 128 bits for P2P interfaces. */
> > > + prefixlen = 128;
> > > +
> > > + sin6 = &ifra.ifra_prefixmask;
> > > + sin6->sin6_family = AF_INET6;
> > > + sin6->sin6_len = sizeof (*sin6);
> > > + in6_prefixlen2mask(&sin6->sin6_addr, prefixlen);
> > > +
> > > + ifra.ifra_lifetime.ia6t_vltime = ND6_INFINITE_LIFETIME;
> > > + ifra.ifra_lifetime.ia6t_pltime = ND6_INFINITE_LIFETIME;
> > > +
> > > + rv = in6_ioctl(SIOCAIFADDR_IN6, (caddr_t)&ifra, ifp, 1);
> > > + if (rv != 0) {
> > > + printf("%s: unable to set IPv6 address, error %d\n",
> > > +    DEVNAM(ifp->if_softc), rv);
> > > + return rv;
> > > + }
> > > +
> > > + memset(&default_sin6, 0, sizeof(default_sin6));
> > > + default_sin6.sin6_family = AF_INET6;
> > > + default_sin6.sin6_len = sizeof (default_sin6);
> > > +
> > > + memset(&info, 0, sizeof(info));
> > > + info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> > > + info.rti_ifa = ifa_ifwithaddr(sin6tosa(&ifra.ifra_addr),
> > > +    ifp->if_rdomain);
> > > + info.rti_info[RTAX_DST] = sin6tosa(&default_sin6);
> > > + info.rti_info[RTAX_NETMASK] = sin6tosa(&default_sin6);
> > > + info.rti_info[RTAX_GATEWAY] = sin6tosa(&ifra.ifra_dstaddr);
> > > +
> > > + NET_LOCK();
> > > + rv = rtrequest(RTM_ADD, &info, 0, &rt, ifp->if_rdomain);
> > > + NET_UNLOCK();
> > > + if (rv) {
> > > + printf("%s: unable to set IPv6 default route, "
> > > +    "error %d\n", DEVNAM(ifp->if_softc), rv);
> > > + rtm_miss(RTM_MISS, &info, 0, RTP_NONE, 0, rv,
> > > +    ifp->if_rdomain);
> > > + } else {
> > > + /* Inform listeners of the new route */
> > > + rtm_send(rt, RTM_ADD, rv, ifp->if_rdomain);
> > > + rtfree(rt);
> > > + }
> > > +
> > > + if (ifp->if_flags & IFF_DEBUG) {
> > > + char str[3][INET6_ADDRSTRLEN];
> > > + log(LOG_INFO, "%s: IPv6 addr %s, mask %s, gateway %s\n",
> > > +    DEVNAM(ifp->if_softc),
> > > +    sockaddr_ntop(sin6tosa(&ifra.ifra_addr), str[0],
> > > +    sizeof(str[0])),
> > > +    sockaddr_ntop(sin6tosa(&ifra.ifra_prefixmask), str[1],
> > > +    sizeof(str[1])),
> > > +    sockaddr_ntop(sin6tosa(&ifra.ifra_dstaddr), str[2],
> > > +    sizeof(str[2])));
> > > + }
> > > + return 0;
> > >  }
> > > +#endif
> > >  
> > >  void
> > > -umb_send_inet_proposal(struct umb_softc *sc)
> > > +umb_send_inet_proposal(struct umb_softc *sc, int af)
> > >  {
> > >   struct ifnet *ifp = GET_IFP(sc);
> > >   struct sockaddr_rtdns rtdns;
> > >   struct rt_addrinfo info;
> > >   int i, flag = 0;
> > > + size_t sz = 0;
> > >  
> > >   memset(&rtdns, 0, sizeof(rtdns));
> > >   memset(&info, 0, sizeof(info));
> > >  
> > >   for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> > > - if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
> > > - break;
> > > - memcpy(rtdns.sr_dns + i * sizeof(struct in_addr),
> > > -    &sc->sc_info.ipv4dns[i], sizeof(struct in_addr));
> > > - flag = RTF_UP;
> > > + if (af == AF_INET) {
> > > + sz = sizeof (sc->sc_info.ipv4dns[i]);
> > > + if (sc->sc_info.ipv4dns[i].s_addr == INADDR_ANY)
> > > + break;
> > > + memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv4dns[i],
> > > +    sz);
> > > + flag = RTF_UP;
> > > +#ifdef INET6
> > > + } if (af == AF_INET6) {  
> >
> > Either make this an else if or add a newline between the } and if.
> >
> > > + sz = sizeof (sc->sc_info.ipv6dns[i]);
> > > + if (IN6_ARE_ADDR_EQUAL(&sc->sc_info.ipv6dns[i],
> > > +    &in6addr_any))
> > > + break;
> > > + memcpy(rtdns.sr_dns + i * sz, &sc->sc_info.ipv6dns[i],
> > > +    sz);
> > > + flag = RTF_UP;
> > > +#endif
> > > + }
> > >   }
> > > - rtdns.sr_family = AF_INET;
> > > - rtdns.sr_len = 2 + i * sizeof(struct in_addr);
> > > + rtdns.sr_family = af;
> > > + rtdns.sr_len = 2 + i * sz;
> > >   info.rti_info[RTAX_DNS] = srtdnstosa(&rtdns);
> > >  
> > >   rtm_proposal(ifp, &info, flag, RTP_PROPOSAL_UMB);
> > > @@ -1732,7 +1843,7 @@ umb_decode_ip_configuration(struct umb_s
> > >   struct mbim_cid_ip_configuration_info *ic = data;
> > >   struct ifnet *ifp = GET_IFP(sc);
> > >   int s;
> > > - uint32_t avail;
> > > + uint32_t avail_v4;
> > >   uint32_t val;
> > >   int n, i;
> > >   int off;
> > > @@ -1740,6 +1851,12 @@ umb_decode_ip_configuration(struct umb_s
> > >   struct in_addr addr, gw;
> > >   int state = -1;
> > >   int rv;
> > > + int hasmtu = 0;
> > > +#ifdef INET6
> > > + uint32_t avail_v6;
> > > + struct mbim_cid_ipv6_element ipv6elem;
> > > + struct in6_addr addr6, gw6;
> > > +#endif
> > >  
> > >   if (len < sizeof (*ic))
> > >   return 0;
> > > @@ -1750,17 +1867,20 @@ umb_decode_ip_configuration(struct umb_s
> > >   }
> > >   s = splnet();
> > >  
> > > + memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> > > + memset(sc->sc_info.ipv6dns, 0, sizeof (sc->sc_info.ipv6dns));
> > > +
> > >   /*
> > >   * IPv4 configuation
> > >   */
> > > - avail = letoh32(ic->ipv4_available);
> > > - if ((avail & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> > > + avail_v4 = letoh32(ic->ipv4_available);
> > > + if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> > >      (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
> > >   n = letoh32(ic->ipv4_naddr);
> > >   off = letoh32(ic->ipv4_addroffs);
> > >  
> > >   if (n == 0 || off + sizeof (ipv4elem) > len)
> > > - goto done;
> > > + goto tryv6;
> > >   if (n != 1 && ifp->if_flags & IFF_DEBUG)
> > >   log(LOG_INFO, "%s: more than one IPv4 addr: %d\n",
> > >      DEVNAM(ifp->if_softc), n);
> > > @@ -1771,6 +1891,8 @@ umb_decode_ip_configuration(struct umb_s
> > >   addr.s_addr = ipv4elem.addr;
> > >  
> > >   off = letoh32(ic->ipv4_gwoffs);
> > > + if (off + sizeof (gw) > len)
> > > + goto done;
> > >   memcpy(&gw, data + off, sizeof(gw));
> > >  
> > >   rv = umb_add_inet_config(sc, addr, ipv4elem.prefixlen, gw);
> > > @@ -1780,12 +1902,12 @@ umb_decode_ip_configuration(struct umb_s
> > >   }
> > >  
> > >   memset(sc->sc_info.ipv4dns, 0, sizeof (sc->sc_info.ipv4dns));
> > > - if (avail & MBIM_IPCONF_HAS_DNSINFO) {
> > > + if (avail_v4 & MBIM_IPCONF_HAS_DNSINFO) {
> > >   n = letoh32(ic->ipv4_ndnssrv);
> > >   off = letoh32(ic->ipv4_dnssrvoffs);
> > >   i = 0;
> > >   while (n-- > 0) {
> > > - if (off + sizeof (uint32_t) > len)
> > > + if (off + sizeof (addr) > len)
> > >   break;
> > >   memcpy(&addr, data + off, sizeof(addr));
> > >   if (i < UMB_MAX_DNSSRV)
> > > @@ -1798,29 +1920,95 @@ umb_decode_ip_configuration(struct umb_s
> > >      &addr, str, sizeof(str)));
> > >   }
> > >   }
> > > - umb_send_inet_proposal(sc);
> > > + umb_send_inet_proposal(sc, AF_INET);
> > >   }
> > > -
> > > - if ((avail & MBIM_IPCONF_HAS_MTUINFO)) {
> > > + if ((avail_v4 & MBIM_IPCONF_HAS_MTUINFO)) {
> > >   val = letoh32(ic->ipv4_mtu);
> > >   if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
> > > + hasmtu = 1;
> > >   ifp->if_hardmtu = val;
> > >   if (ifp->if_mtu > val)
> > >   ifp->if_mtu = val;
> > > - if (ifp->if_flags & IFF_DEBUG)
> > > - log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), val);
> > >   }
> > >   }
> > >  
> > > - avail = letoh32(ic->ipv6_available);
> > > - if ((ifp->if_flags & IFF_DEBUG) && avail & MBIM_IPCONF_HAS_ADDRINFO) {
> > > - /* XXX FIXME: IPv6 configuation missing */
> > > - log(LOG_INFO, "%s: ignoring IPv6 configuration\n", DEVNAM(sc));
> > > +tryv6:;
> > > +#ifdef INET6
> > > + /*
> > > + * IPv6 configuation
> > > + */
> > > + avail_v6 = letoh32(ic->ipv6_available);
> > > + if (avail_v6 == 0) {
> > > + if (ifp->if_flags & IFF_DEBUG)
> > > + log(LOG_INFO, "%s: ISP or WWAN module offers no IPv6 "
> > > +    "support\n", DEVNAM(ifp->if_softc));  
> >
> > Wouldn't the WWAN module return an error earlier hitting the
> > MBIM_STATUS_NO_DEVICE_SUPPORT in MBIM_CID_CONNECT? I would prefer we could
> > tell the user exactly why IPv6 did not configure. I think in my case it is
> > the ISP that is to blame but how do I know...
>
> I have a T-Mobile SIM card and can get an IPv6 address when used in a
> SIMCOM module. Using the same SIM card in an older Sierra Wireless
> EM8805 I get neither an IPv6 address, nor an error from the module.
>
> Unfortunately, it seems we have to confine ourselves to not knowing.
>
>
> >
> > > + goto done;
> > > + }
> > > +
> > > + if ((avail_v4 & (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) ==
> > > +    (MBIM_IPCONF_HAS_ADDRINFO | MBIM_IPCONF_HAS_GWINFO)) {
> > > + n = letoh32(ic->ipv6_naddr);
> > > + off = letoh32(ic->ipv6_addroffs);
> > > +
> > > + if (n == 0 || off + sizeof (ipv6elem) > len)
> > > + goto done;
> > > + if (n != 1 && ifp->if_flags & IFF_DEBUG)
> > > + log(LOG_INFO, "%s: more than one IPv6 addr: %d\n",
> > > +    DEVNAM(ifp->if_softc), n);
> > > +
> > > + /* Only pick the first one */
> > > + memcpy(&ipv6elem, data + off, sizeof (ipv6elem));
> > > + memcpy(&addr6, ipv6elem.addr, sizeof (addr6));
> > > +
> > > + off = letoh32(ic->ipv6_gwoffs);
> > > + if (off + sizeof (gw6) > len)
> > > + goto done;
> > > + memcpy(&gw6, data + off, sizeof (gw6));
> > > +
> > > + rv = umb_add_inet6_config(sc, &addr6, ipv6elem.prefixlen, &gw6);
> > > + if (rv == 0)
> > > + state = UMB_S_UP;
> > > + }
> > > +
> > > + if (avail_v6 & MBIM_IPCONF_HAS_DNSINFO) {
> > > + n = letoh32(ic->ipv6_ndnssrv);
> > > + off = letoh32(ic->ipv6_dnssrvoffs);
> > > + i = 0;
> > > + while (n-- > 0) {
> > > + if (off + sizeof (addr6) > len)
> > > + break;
> > > + memcpy(&addr6, data + off, sizeof(addr6));
> > > + if (i < UMB_MAX_DNSSRV)
> > > + sc->sc_info.ipv6dns[i++] = addr6;
> > > + off += sizeof(addr6);
> > > + if (ifp->if_flags & IFF_DEBUG) {
> > > + char str[INET6_ADDRSTRLEN];
> > > + log(LOG_INFO, "%s: IPv6 nameserver %s\n",
> > > +    DEVNAM(ifp->if_softc), inet_ntop(AF_INET6,
> > > +    &addr6, str, sizeof(str)));
> > > + }
> > > + }
> > > + umb_send_inet_proposal(sc, AF_INET6);
> > > + }
> > > +
> > > + if ((avail_v6 & MBIM_IPCONF_HAS_MTUINFO)) {
> > > + val = letoh32(ic->ipv6_mtu);
> > > + if (ifp->if_hardmtu != val && val <= sc->sc_maxpktlen) {
> > > + hasmtu = 1;
> > > + ifp->if_hardmtu = val;
> > > + if (ifp->if_mtu > val)
> > > + ifp->if_mtu = val;
> > > + }
> > >   }
> > > +#endif
> > > +
> > > +done:
> > > + if (hasmtu && (ifp->if_flags & IFF_DEBUG))
> > > + log(LOG_INFO, "%s: MTU %d\n", DEVNAM(sc), ifp->if_hardmtu);
> > > +
> > >   if (state != -1)
> > >   umb_newstate(sc, state, 0);
> > >  
> > > -done:
> > >   splx(s);
> > >   return 1;
> > >  }
> > > @@ -2393,6 +2581,11 @@ umb_send_connect(struct umb_softc *sc, i
> > >   c->authprot = htole32(MBIM_AUTHPROT_NONE);
> > >   c->compression = htole32(MBIM_COMPRESSION_NONE);
> > >   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
> > > +#ifdef INET6
> > > + /* XXX FIXME: support IPv6-only mode, too */
> > > + if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
> > > + c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);  
> >
> > Is there a reason why MBIM_CONTEXT_IPTYPE_IPV4V6 is used and not
> > MBIM_CONTEXT_IPTYPE_IPV4ANDV6?
>
>
> Yes, I tried MBIM_CONTEXT_IPTYPE_IPV4ANDV6 myself first but to no
> avail. The switched to MBIM_CONTEXT_IPTYPE_IPV4V6 and everything
> was fine.
>
>
> Gerhard
>
>
> >
> > > +#endif
> > >   memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));
> > >   umb_cmd(sc, MBIM_CID_CONNECT, MBIM_CMDOP_SET, c, off);
> > >  done:
> > > @@ -2476,6 +2669,20 @@ umb_command_done(struct umb_softc *sc, v
> > >   switch (status) {
> > >   case MBIM_STATUS_SUCCESS:
> > >   break;
> > > +#ifdef INET6
> > > + case MBIM_STATUS_NO_DEVICE_SUPPORT:
> > > + if ((cid == MBIM_CID_CONNECT) &&
> > > +    (sc->sc_flags & UMBFLG_NO_INET6) == 0) {
> > > + sc->sc_flags |= UMBFLG_NO_INET6;
> > > + if (ifp->if_flags & IFF_DEBUG)
> > > + log(LOG_ERR,
> > > +    "%s: device does not support IPv6\n",
> > > +    DEVNAM(sc));
> > > + }
> > > + /* Re-trigger the connect, this time IPv4 only */
> > > + usb_add_task(sc->sc_udev, &sc->sc_umb_task);
> > > + return;
> > > +#endif
> > >   case MBIM_STATUS_NOT_INITIALIZED:
> > >   if (ifp->if_flags & IFF_DEBUG)
> > >   log(LOG_ERR, "%s: SIM not initialized (PIN missing)\n",
> > > Index: sys/dev/usb/if_umb.h
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/usb/if_umb.h,v
> > > retrieving revision 1.5
> > > diff -u -p -u -p -r1.5 if_umb.h
> > > --- sys/dev/usb/if_umb.h 26 Aug 2019 15:23:01 -0000 1.5
> > > +++ sys/dev/usb/if_umb.h 4 Feb 2020 07:30:43 -0000
> > > @@ -321,6 +321,7 @@ struct umb_info {
> > >  
> > >  #define UMB_MAX_DNSSRV 2
> > >   struct in_addr ipv4dns[UMB_MAX_DNSSRV];
> > > + struct in6_addr ipv6dns[UMB_MAX_DNSSRV];
> > >  };
> > >  
> > >  #ifdef _KERNEL
> > > @@ -345,6 +346,7 @@ struct umb_softc {
> > >   int sc_ndp_remainder;
> > >  
> > >  #define UMBFLG_FCC_AUTH_REQUIRED 0x0001
> > > +#define UMBFLG_NO_INET6 0x0002
> > >   uint32_t sc_flags;
> > >   int sc_cid;
> > >  
> > > @@ -380,6 +382,6 @@ struct umb_softc {
> > >  
> > >  #define sc_state sc_info.state
> > >  #define sc_roaming sc_info.enable_roaming
> > > - struct umb_info sc_info;
> > > + struct umb_info sc_info;  
> >
> > Extra space befor sc_info.
> >
> > >  };
> > >  #endif /* _KERNEL */
> > >  
> >

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Stuart Henderson
In reply to this post by Gerhard Roth-2
On 2020/02/18 08:25, Gerhard Roth wrote:

> > > @@ -2393,6 +2581,11 @@ umb_send_connect(struct umb_softc *sc, i
> > >   c->authprot = htole32(MBIM_AUTHPROT_NONE);
> > >   c->compression = htole32(MBIM_COMPRESSION_NONE);
> > >   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
> > > +#ifdef INET6
> > > + /* XXX FIXME: support IPv6-only mode, too */
> > > + if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
> > > + c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);  
> >
> > Is there a reason why MBIM_CONTEXT_IPTYPE_IPV4V6 is used and not
> > MBIM_CONTEXT_IPTYPE_IPV4ANDV6?
>
>
> Yes, I tried MBIM_CONTEXT_IPTYPE_IPV4ANDV6 myself first but to no
> avail. The switched to MBIM_CONTEXT_IPTYPE_IPV4V6 and everything
> was fine.

Obviously it needs to switch based on INET6, but with the current
mechanism used for handling v6 in OpenBSD, shouldn't it disable v6
unless the interface has a link-local configured? (So the usual method
to enable v6 and bring an interface up would be "inet6 eui64" and "up").
That is how pppoe works.

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Gerhard Roth-2
On Tue, 18 Feb 2020 12:11:05 +0000 Stuart Henderson <[hidden email]> wrote:

> On 2020/02/18 08:25, Gerhard Roth wrote:
> > > > @@ -2393,6 +2581,11 @@ umb_send_connect(struct umb_softc *sc, i
> > > >   c->authprot = htole32(MBIM_AUTHPROT_NONE);
> > > >   c->compression = htole32(MBIM_COMPRESSION_NONE);
> > > >   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
> > > > +#ifdef INET6
> > > > + /* XXX FIXME: support IPv6-only mode, too */
> > > > + if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
> > > > + c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);    
> > >
> > > Is there a reason why MBIM_CONTEXT_IPTYPE_IPV4V6 is used and not
> > > MBIM_CONTEXT_IPTYPE_IPV4ANDV6?  
> >
> >
> > Yes, I tried MBIM_CONTEXT_IPTYPE_IPV4ANDV6 myself first but to no
> > avail. The switched to MBIM_CONTEXT_IPTYPE_IPV4V6 and everything
> > was fine.  
>
> Obviously it needs to switch based on INET6, but with the current
> mechanism used for handling v6 in OpenBSD, shouldn't it disable v6
> unless the interface has a link-local configured? (So the usual method
> to enable v6 and bring an interface up would be "inet6 eui64" and "up").
> That is how pppoe works.


Hi Stuart,

you mean like that?

Gerhard


Index: sys/dev/usb/if_umb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.32
diff -u -p -u -p -r1.32 if_umb.c
--- sys/dev/usb/if_umb.c 18 Feb 2020 08:09:37 -0000 1.32
+++ sys/dev/usb/if_umb.c 18 Feb 2020 12:35:45 -0000
@@ -2583,7 +2583,8 @@ umb_send_connect(struct umb_softc *sc, i
  c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
 #ifdef INET6
  /* XXX FIXME: support IPv6-only mode, too */
- if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
+ if ((sc->sc_flags & UMBFLG_NO_INET6) == 0 &&
+    in6ifa_ifpforlinklocal(GET_IFP(sc), 0) != NULL)
  c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);
 #endif
  memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Stuart Henderson
On 2020/02/18 13:40, Gerhard Roth wrote:

> > > Yes, I tried MBIM_CONTEXT_IPTYPE_IPV4ANDV6 myself first but to no
> > > avail. The switched to MBIM_CONTEXT_IPTYPE_IPV4V6 and everything
> > > was fine.  
> >
> > Obviously it needs to switch based on INET6, but with the current
> > mechanism used for handling v6 in OpenBSD, shouldn't it disable v6
> > unless the interface has a link-local configured? (So the usual method
> > to enable v6 and bring an interface up would be "inet6 eui64" and "up").
> > That is how pppoe works.
>
>
> Hi Stuart,
>
> you mean like that?

Yes, that looks right - sorry I don't have a working umb to test though!

> Index: sys/dev/usb/if_umb.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.32
> diff -u -p -u -p -r1.32 if_umb.c
> --- sys/dev/usb/if_umb.c 18 Feb 2020 08:09:37 -0000 1.32
> +++ sys/dev/usb/if_umb.c 18 Feb 2020 12:35:45 -0000
> @@ -2583,7 +2583,8 @@ umb_send_connect(struct umb_softc *sc, i
>   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
>  #ifdef INET6
>   /* XXX FIXME: support IPv6-only mode, too */
> - if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
> + if ((sc->sc_flags & UMBFLG_NO_INET6) == 0 &&
> +    in6ifa_ifpforlinklocal(GET_IFP(sc), 0) != NULL)
>   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);
>  #endif
>   memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Claudio Jeker
On Tue, Feb 18, 2020 at 11:16:54PM +0000, Stuart Henderson wrote:

> On 2020/02/18 13:40, Gerhard Roth wrote:
> > > > Yes, I tried MBIM_CONTEXT_IPTYPE_IPV4ANDV6 myself first but to no
> > > > avail. The switched to MBIM_CONTEXT_IPTYPE_IPV4V6 and everything
> > > > was fine.  
> > >
> > > Obviously it needs to switch based on INET6, but with the current
> > > mechanism used for handling v6 in OpenBSD, shouldn't it disable v6
> > > unless the interface has a link-local configured? (So the usual method
> > > to enable v6 and bring an interface up would be "inet6 eui64" and "up").
> > > That is how pppoe works.
> >
> >
> > Hi Stuart,
> >
> > you mean like that?
>
> Yes, that looks right - sorry I don't have a working umb to test though!

I guess we should then also adjust the manpage to make sure people know
how to enable IPv6 in hostname.umb0
 

> > Index: sys/dev/usb/if_umb.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> > retrieving revision 1.32
> > diff -u -p -u -p -r1.32 if_umb.c
> > --- sys/dev/usb/if_umb.c 18 Feb 2020 08:09:37 -0000 1.32
> > +++ sys/dev/usb/if_umb.c 18 Feb 2020 12:35:45 -0000
> > @@ -2583,7 +2583,8 @@ umb_send_connect(struct umb_softc *sc, i
> >   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
> >  #ifdef INET6
> >   /* XXX FIXME: support IPv6-only mode, too */
> > - if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
> > + if ((sc->sc_flags & UMBFLG_NO_INET6) == 0 &&
> > +    in6ifa_ifpforlinklocal(GET_IFP(sc), 0) != NULL)
> >   c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);
> >  #endif
> >   memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));
>

--
:wq Claudio

Reply | Threaded
Open this post in threaded view
|

Re: IPv6 Support for umb(4)

Gerhard Roth-2
On Wed, 19 Feb 2020 08:45:39 +0100 Claudio Jeker <[hidden email]> wrote:

> On Tue, Feb 18, 2020 at 11:16:54PM +0000, Stuart Henderson wrote:
> > On 2020/02/18 13:40, Gerhard Roth wrote:  
> > > > > Yes, I tried MBIM_CONTEXT_IPTYPE_IPV4ANDV6 myself first but to no
> > > > > avail. The switched to MBIM_CONTEXT_IPTYPE_IPV4V6 and everything
> > > > > was fine.    
> > > >
> > > > Obviously it needs to switch based on INET6, but with the current
> > > > mechanism used for handling v6 in OpenBSD, shouldn't it disable v6
> > > > unless the interface has a link-local configured? (So the usual method
> > > > to enable v6 and bring an interface up would be "inet6 eui64" and "up").
> > > > That is how pppoe works.  
> > >
> > >
> > > Hi Stuart,
> > >
> > > you mean like that?  
> >
> > Yes, that looks right - sorry I don't have a working umb to test though!  
>
> I guess we should then also adjust the manpage to make sure people know
> how to enable IPv6 in hostname.umb0


Took a look at pppoe.4 and tried to extract what is needed for umb.4.
Updated diff below.

Gerhard


Index: share/man/man4/umb.4
===================================================================
RCS file: /cvs/src/share/man/man4/umb.4,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 umb.4
--- share/man/man4/umb.4 18 Feb 2020 08:09:37 -0000 1.10
+++ share/man/man4/umb.4 19 Feb 2020 08:14:01 -0000
@@ -40,6 +40,19 @@ will remain in this state until the MBIM
 In case the device is connected to an "always-on" USB port,
 it may be possible to connect to a provider without entering the
 PIN again even if the system was rebooted.
+.Pp
+To use IPv6, configure a link-local address before bringing
+the interface up.
+Some devices require the
+.Sy AUTOCONF6
+flag on the interface.
+.Pp
+A typical
+.Pa /etc/hostname.umb0
+looks like this:
+.Bd -literal -offset indent
+pin 1234 apn ISP-APN-Name inet6 eui64 autoconf -roaming up
+.Ed
 .Sh HARDWARE
 The following devices should work:
 .Pp
Index: sys/dev/usb/if_umb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.32
diff -u -p -u -p -r1.32 if_umb.c
--- sys/dev/usb/if_umb.c 18 Feb 2020 08:09:37 -0000 1.32
+++ sys/dev/usb/if_umb.c 18 Feb 2020 12:35:45 -0000
@@ -2583,7 +2583,8 @@ umb_send_connect(struct umb_softc *sc, i
  c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4);
 #ifdef INET6
  /* XXX FIXME: support IPv6-only mode, too */
- if ((sc->sc_flags & UMBFLG_NO_INET6) == 0)
+ if ((sc->sc_flags & UMBFLG_NO_INET6) == 0 &&
+    in6ifa_ifpforlinklocal(GET_IFP(sc), 0) != NULL)
  c->iptype = htole32(MBIM_CONTEXT_IPTYPE_IPV4V6);
 #endif
  memcpy(c->context, umb_uuid_context_internet, sizeof (c->context));