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 <cje...@diehard.n-r-g.com>
> 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
> > > <alexander.bl...@gmx.net> 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
