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
