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...

> +             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 via email to