Re: IPv6 Support for umb(4)

2020-05-01 Thread Stuart Henderson
On 2020/05/01 20:10, Gerhard Roth wrote:
> On 4/30/20 11:07 PM, Stuart Henderson wrote:
> > On 2020/04/30 20:32, Gerhard Roth wrote:
> > > Hi Theo,
> > > 
> > > is umb really working that differently for a P2P interface? I think it is
> > > very similar to ppp(4) and IPv6. The standard way is to obtain the IP
> > > address via PPP protocol. Just like this, umb(4) obtains the IP address 
> > > via
> > > MBIM protocol.
> > 
> > PPP is quite different, it only negotiates a link-local v6 address. If you
> > want a routable address you must use slaac, dhcpv6, or static config.
> > 
> > > That's what is implemented with this and former umb(4) patches. With the
> > > current patch you can just do "ifconfig umb0 inet eui64" or "ifconfig umb0
> > > -inet6" anytime, whether the interface is up or not.
> > > 
> > > But then there seem to be strange ISPs in Japan as detected by job@. They
> > > don't provide any IPv6 address via MBIM protocol but rather use the 
> > > standard
> > > IPv6 SLAAC protocol. It strange; just as if ppp(4) would need DHCP to 
> > > obtain
> > > its IPv4 address. But still, it seems to exist and users can still work 
> > > with
> > > umb(4) if they do "ifconfig umb0 inet6 autoconf".
> > 
> > What happens if an ISP provides a v6 address via MBIM-config and the 
> > interface
> > is set to "inet6 autoconf", does it still work OK? That's what most people
> > will try. Since we disabled IPv6 by default, IPv6 users already know how to
> > use "inet6 autoconf".
> 
> I just can't tell. All providers here in Germany will give me an IPv6
> address via MBIM. There's no way I could test this case. Especially not in
> times of Corona.

That's exactly the case I mean. What happens if you are on a network like
that which gives a v6 address via MBIM and you also set "inet6 autoconf" -
does that work ok?

If that works then we can just tell people to use autoconf, it won't be
necessary but will work for your standard case, will work for the ISP that
Job found, and matches what people would normally do anyway.



Re: IPv6 Support for umb(4)

2020-05-01 Thread Gerhard Roth

On 4/30/20 11:07 PM, Stuart Henderson wrote:

On 2020/04/30 20:32, Gerhard Roth wrote:

Hi Theo,

is umb really working that differently for a P2P interface? I think it is
very similar to ppp(4) and IPv6. The standard way is to obtain the IP
address via PPP protocol. Just like this, umb(4) obtains the IP address via
MBIM protocol.


PPP is quite different, it only negotiates a link-local v6 address. If you
want a routable address you must use slaac, dhcpv6, or static config.


That's what is implemented with this and former umb(4) patches. With the
current patch you can just do "ifconfig umb0 inet eui64" or "ifconfig umb0
-inet6" anytime, whether the interface is up or not.

But then there seem to be strange ISPs in Japan as detected by job@. They
don't provide any IPv6 address via MBIM protocol but rather use the standard
IPv6 SLAAC protocol. It strange; just as if ppp(4) would need DHCP to obtain
its IPv4 address. But still, it seems to exist and users can still work with
umb(4) if they do "ifconfig umb0 inet6 autoconf".


What happens if an ISP provides a v6 address via MBIM-config and the interface
is set to "inet6 autoconf", does it still work OK? That's what most people
will try. Since we disabled IPv6 by default, IPv6 users already know how to
use "inet6 autoconf".


I just can't tell. All providers here in Germany will give me an IPv6 
address via MBIM. There's no way I could test this case. Especially not 
in times of Corona.


Gerhard





I also feel noone is going to read the manual page, find this piece
of text, and understand it.  Honestly, I don't understand this piece
of text.  I'm not going to set the AUTOCONF6 flag.  How does one even
set it?

ifconfig: AUTOCONF6: bad value

Of course not, but I am ironically trying to show this documentation
chunk doesn't help at all.  People can't act upon it properly.

I still argue umb's inet6 should work absolutely as much like regular
interfaces, or it is useless.  People are not going to treat this
interface differently and then gain successful inet6.  If inet6 can't
work naturally and easily, but instead is a special snowflake, that
is just plain dumb.


Unfortunately mobile data is a special snowflake because you get some
providers who say "to conserve licenses with the network vendor you can
have either an IPv4 address or an IPv6 address but not both at the same
time" so you need a way to do something which isn't required on normal
ethernet.


+.Pp
+To use IPv6, configure a link-local address.
+If the device is able to connect to the ISP's network but doesn't
+show an IPv6 address, setting the
+.Sy AUTOCONF6
+flag on the interface before bringing it up may help.
+.Ed


Showing the actual command to type will help a lot here.

On 2020/04/30 20:52, Gerhard Roth wrote:

It it too much to expect users to read the ifconfig man page?


Printed, it is 28 pages of A4.

Compare with the wifi drivers, you have to look at ifconfig(8) if
you want all the details, but EXAMPLES in iwm(4) (and I think all the other
drivers) is enough for a quick bare-bones config. That seems a reasonable
model.





Re: IPv6 Support for umb(4)

2020-04-30 Thread Jason McIntyre
On Thu, Apr 30, 2020 at 03:33:56PM -0600, Theo de Raadt wrote:
> Jason McIntyre  wrote:
> 
> > On Thu, Apr 30, 2020 at 10:07:14PM +0100, Stuart Henderson wrote:
> > > 
> > > On 2020/04/30 20:52, Gerhard Roth wrote:
> > > > It it too much to expect users to read the ifconfig man page?
> > > 
> > > Printed, it is 28 pages of A4.
> > > 
> > 
> > ouch.
> > 
> > > Compare with the wifi drivers, you have to look at ifconfig(8) if
> > > you want all the details, but EXAMPLES in iwm(4) (and I think all the 
> > > other
> > > drivers) is enough for a quick bare-bones config. That seems a reasonable
> > > model.
> > > 
> > 
> > i think we have lost our way a bit with ifconfig.8. the need to avoid
> > repitition drove it, but bridge broke it.
> > 
> > we should start farming out all of the subsections back to their
> > respective pages.
> 
> Respective pages?!  The respective page for ifconfig commands *is* the
> ifconfig page.  The drivers impliment various lowlevel and system
> behaviours, but ifconfig is documenting the commands.
> 

there is more than one way to do it. that's all.

> > umb.4 is one screenful, but how to use it with
> > ifconfig is at the end of ifconfig.8. that can;t be optimal.
> 
> Since the beginning of umb, I've begged for it to stop being
> a such special snowflake, and to search for common functionality with
> other drivers, to hide the specialness.
> 
> But the previous comment remains.  These are ifconfig commands.
> They belong in the ifconfig manual page.  We don't describe ls options
> in filesystem manual pages, we describe them in the ls man page.
> 
> > of course there's an issue, and it's a big one: IEEE 802.11. farming
> > that out would inflate a lot of pages, and require care to keep
> > consistent.
> 
> It simply makes no sense.  Driver options aren't being explained.
> All of those drivers have interfaces to low-level network stack.
> The network stack is told what to do with the ifconfig command,
> using ifconfig options.  The docs are in the right place.
> 

since you disagree, it won;t happen.

but i don;t agree that the docs are in the right place - there is
no "right place". farming it out would solve some of ifconfig's
problems, and replace them with some other. it's a trade off. that's
all. i think the trade off would be worth it.

an analogy: carp documents some sysctls. but you use sysctl to set them.

jmc



Re: IPv6 Support for umb(4)

2020-04-30 Thread Stuart Henderson
On 2020/04/30 22:28, Jason McIntyre wrote:
> On Thu, Apr 30, 2020 at 10:07:14PM +0100, Stuart Henderson wrote:
> > 
> > On 2020/04/30 20:52, Gerhard Roth wrote:
> > > It it too much to expect users to read the ifconfig man page?
> > 
> > Printed, it is 28 pages of A4.
> > 
> 
> ouch.

admittedly one of them just has 'HISTORY' on which is 1 line, but still ;)

The formatting (I am using "MANPAGER=mupdf man -T pdf ifconfig") to see
this is lovely though!



Re: IPv6 Support for umb(4)

2020-04-30 Thread Theo de Raadt
Jason McIntyre  wrote:

> On Thu, Apr 30, 2020 at 10:07:14PM +0100, Stuart Henderson wrote:
> > 
> > On 2020/04/30 20:52, Gerhard Roth wrote:
> > > It it too much to expect users to read the ifconfig man page?
> > 
> > Printed, it is 28 pages of A4.
> > 
> 
> ouch.
> 
> > Compare with the wifi drivers, you have to look at ifconfig(8) if
> > you want all the details, but EXAMPLES in iwm(4) (and I think all the other
> > drivers) is enough for a quick bare-bones config. That seems a reasonable
> > model.
> > 
> 
> i think we have lost our way a bit with ifconfig.8. the need to avoid
> repitition drove it, but bridge broke it.
> 
> we should start farming out all of the subsections back to their
> respective pages.

Respective pages?!  The respective page for ifconfig commands *is* the
ifconfig page.  The drivers impliment various lowlevel and system
behaviours, but ifconfig is documenting the commands.

> umb.4 is one screenful, but how to use it with
> ifconfig is at the end of ifconfig.8. that can;t be optimal.

Since the beginning of umb, I've begged for it to stop being
a such special snowflake, and to search for common functionality with
other drivers, to hide the specialness.

But the previous comment remains.  These are ifconfig commands.
They belong in the ifconfig manual page.  We don't describe ls options
in filesystem manual pages, we describe them in the ls man page.

> of course there's an issue, and it's a big one: IEEE 802.11. farming
> that out would inflate a lot of pages, and require care to keep
> consistent.

It simply makes no sense.  Driver options aren't being explained.
All of those drivers have interfaces to low-level network stack.
The network stack is told what to do with the ifconfig command,
using ifconfig options.  The docs are in the right place.



Re: IPv6 Support for umb(4)

2020-04-30 Thread Jason McIntyre
On Thu, Apr 30, 2020 at 10:07:14PM +0100, Stuart Henderson wrote:
> 
> On 2020/04/30 20:52, Gerhard Roth wrote:
> > It it too much to expect users to read the ifconfig man page?
> 
> Printed, it is 28 pages of A4.
> 

ouch.

> Compare with the wifi drivers, you have to look at ifconfig(8) if
> you want all the details, but EXAMPLES in iwm(4) (and I think all the other
> drivers) is enough for a quick bare-bones config. That seems a reasonable
> model.
> 

i think we have lost our way a bit with ifconfig.8. the need to avoid
repitition drove it, but bridge broke it.

we should start farming out all of the subsections back to their
respective pages. umb.4 is one screenful, but how to use it with
ifconfig is at the end of ifconfig.8. that can;t be optimal.

of course there's an issue, and it's a big one: IEEE 802.11. farming
that out would inflate a lot of pages, and require care to keep
consistent.

thinks...

jmc



Re: IPv6 Support for umb(4)

2020-04-30 Thread Stuart Henderson
On 2020/04/30 20:32, Gerhard Roth wrote:
> Hi Theo,
> 
> is umb really working that differently for a P2P interface? I think it is
> very similar to ppp(4) and IPv6. The standard way is to obtain the IP
> address via PPP protocol. Just like this, umb(4) obtains the IP address via
> MBIM protocol.

PPP is quite different, it only negotiates a link-local v6 address. If you
want a routable address you must use slaac, dhcpv6, or static config.

> That's what is implemented with this and former umb(4) patches. With the
> current patch you can just do "ifconfig umb0 inet eui64" or "ifconfig umb0
> -inet6" anytime, whether the interface is up or not.
> 
> But then there seem to be strange ISPs in Japan as detected by job@. They
> don't provide any IPv6 address via MBIM protocol but rather use the standard
> IPv6 SLAAC protocol. It strange; just as if ppp(4) would need DHCP to obtain
> its IPv4 address. But still, it seems to exist and users can still work with
> umb(4) if they do "ifconfig umb0 inet6 autoconf".

What happens if an ISP provides a v6 address via MBIM-config and the interface
is set to "inet6 autoconf", does it still work OK? That's what most people
will try. Since we disabled IPv6 by default, IPv6 users already know how to
use "inet6 autoconf".

> > I also feel noone is going to read the manual page, find this piece
> > of text, and understand it.  Honestly, I don't understand this piece
> > of text.  I'm not going to set the AUTOCONF6 flag.  How does one even
> > set it?
> > 
> > ifconfig: AUTOCONF6: bad value
> > 
> > Of course not, but I am ironically trying to show this documentation
> > chunk doesn't help at all.  People can't act upon it properly.
> > 
> > I still argue umb's inet6 should work absolutely as much like regular
> > interfaces, or it is useless.  People are not going to treat this
> > interface differently and then gain successful inet6.  If inet6 can't
> > work naturally and easily, but instead is a special snowflake, that
> > is just plain dumb.

Unfortunately mobile data is a special snowflake because you get some
providers who say "to conserve licenses with the network vendor you can
have either an IPv4 address or an IPv6 address but not both at the same
time" so you need a way to do something which isn't required on normal
ethernet.

> > > > +.Pp
> > > > +To use IPv6, configure a link-local address.
> > > > +If the device is able to connect to the ISP's network but doesn't
> > > > +show an IPv6 address, setting the
> > > > +.Sy AUTOCONF6
> > > > +flag on the interface before bringing it up may help.
> > > > +.Ed

Showing the actual command to type will help a lot here.

On 2020/04/30 20:52, Gerhard Roth wrote:
> It it too much to expect users to read the ifconfig man page?

Printed, it is 28 pages of A4.

Compare with the wifi drivers, you have to look at ifconfig(8) if
you want all the details, but EXAMPLES in iwm(4) (and I think all the other
drivers) is enough for a quick bare-bones config. That seems a reasonable
model.



Re: IPv6 Support for umb(4)

2020-04-30 Thread Gerhard Roth

On 4/30/20 8:04 PM, Theo de Raadt wrote:

I also feel noone is going to read the manual page, find this piece
of text, and understand it.  Honestly, I don't understand this piece
of text.  I'm not going to set the AUTOCONF6 flag.  How does one even
set it?

ifconfig: AUTOCONF6: bad value

Of course not, but I am ironically trying to show this documentation
chunk doesn't help at all.  People can't act upon it properly.


Well then, we should fix the ifconfig(8) man page first, because it 
talks about the AUTOCONF6 flag, too.


It it too much to expect users to read the ifconfig man page?



Re: IPv6 Support for umb(4)

2020-04-30 Thread Gerhard Roth

Hi Theo,

is umb really working that differently for a P2P interface? I think it 
is very similar to ppp(4) and IPv6. The standard way is to obtain the IP 
address via PPP protocol. Just like this, umb(4) obtains the IP address 
via MBIM protocol.


That's what is implemented with this and former umb(4) patches. With the 
current patch you can just do "ifconfig umb0 inet eui64" or "ifconfig 
umb0 -inet6" anytime, whether the interface is up or not.


But then there seem to be strange ISPs in Japan as detected by job@. 
They don't provide any IPv6 address via MBIM protocol but rather use the 
standard IPv6 SLAAC protocol. It strange; just as if ppp(4) would need 
DHCP to obtain its IPv4 address. But still, it seems to exist and users 
can still work with umb(4) if they do "ifconfig umb0 inet6 autoconf".


I don't see any way, how this different behavior could be "unified" in 
umb(4). It's a problem with the ISP, not with the driver.



Gerhard


On 4/30/20 8:04 PM, Theo de Raadt wrote:

I don't know the answers.

But if umb works differently it will suck.

I also feel noone is going to read the manual page, find this piece
of text, and understand it.  Honestly, I don't understand this piece
of text.  I'm not going to set the AUTOCONF6 flag.  How does one even
set it?

ifconfig: AUTOCONF6: bad value

Of course not, but I am ironically trying to show this documentation
chunk doesn't help at all.  People can't act upon it properly.

I still argue umb's inet6 should work absolutely as much like regular
interfaces, or it is useless.  People are not going to treat this
interface differently and then gain successful inet6.  If inet6 can't
work naturally and easily, but instead is a special snowflake, that
is just plain dumb.

Gerhard Roth  wrote:


On 4/30/20 4:03 PM, Theo de Raadt wrote:

Is that still the true behaviour?  I think it isn't, the "before up"
aspect is gone isn't it?


That's right for IP configuration via MBIM and I deleted the "before
up" from the first sentence.

But wasn't sure for the SLAAC case. Will autoconf work if I set the
flag after the interface is up? Should I delete the "before up" here,
too?

Gerhard




+.Pp
+To use IPv6, configure a link-local address.
+If the device is able to connect to the ISP's network but doesn't
+show an IPv6 address, setting the
+.Sy AUTOCONF6
+flag on the interface before bringing it up may help.
+.Ed





Re: IPv6 Support for umb(4)

2020-04-30 Thread Theo de Raadt
I'm completely lost.

I said it should work as much as a regular device does.  I suggested
the hardware always come up in software mode.  And then, the driver
fakes as if it is doing the inet6 stuff at the moment, whereas it is
already done.  So that the upper level network stack needs nothing
different.

Done right, this would require *NO* driver documentation.  Which people
won't discover.

So there's a diff, which I can't test because my specific provider
here doesn't actually do inet6.  Have to trust others.  But driver diff
either provides transparent behaviour, or it behaves as described in
this manaual page chunk in which case it isn't transparent.

I don't know which.

Gerhard Roth  wrote:

> On 4/30/20 4:03 PM, Theo de Raadt wrote:
> > Is that still the true behaviour?  I think it isn't, the "before up"
> > aspect is gone isn't it?
> 
> That's right for IP configuration via MBIM and I deleted the "before
> up" from the first sentence.
> 
> But wasn't sure for the SLAAC case. Will autoconf work if I set the
> flag after the interface is up? Should I delete the "before up" here,
> too?
> 
> Gerhard
> 
> >
> >
> > +.Pp
> > +To use IPv6, configure a link-local address.
> > +If the device is able to connect to the ISP's network but doesn't
> > +show an IPv6 address, setting the
> > +.Sy AUTOCONF6
> > +flag on the interface before bringing it up may help.
> > +.Ed
> >
> 



Re: IPv6 Support for umb(4)

2020-04-30 Thread Theo de Raadt
I don't know the answers.

But if umb works differently it will suck.

I also feel noone is going to read the manual page, find this piece
of text, and understand it.  Honestly, I don't understand this piece
of text.  I'm not going to set the AUTOCONF6 flag.  How does one even
set it?

ifconfig: AUTOCONF6: bad value

Of course not, but I am ironically trying to show this documentation
chunk doesn't help at all.  People can't act upon it properly.

I still argue umb's inet6 should work absolutely as much like regular
interfaces, or it is useless.  People are not going to treat this
interface differently and then gain successful inet6.  If inet6 can't
work naturally and easily, but instead is a special snowflake, that
is just plain dumb.

Gerhard Roth  wrote:

> On 4/30/20 4:03 PM, Theo de Raadt wrote:
> > Is that still the true behaviour?  I think it isn't, the "before up"
> > aspect is gone isn't it?
> 
> That's right for IP configuration via MBIM and I deleted the "before
> up" from the first sentence.
> 
> But wasn't sure for the SLAAC case. Will autoconf work if I set the
> flag after the interface is up? Should I delete the "before up" here,
> too?
> 
> Gerhard
> 
> >
> >
> > +.Pp
> > +To use IPv6, configure a link-local address.
> > +If the device is able to connect to the ISP's network but doesn't
> > +show an IPv6 address, setting the
> > +.Sy AUTOCONF6
> > +flag on the interface before bringing it up may help.
> > +.Ed
> >



Re: IPv6 Support for umb(4)

2020-04-30 Thread Gerhard Roth

On 4/30/20 4:03 PM, Theo de Raadt wrote:

Is that still the true behaviour?  I think it isn't, the "before up"
aspect is gone isn't it?


That's right for IP configuration via MBIM and I deleted the "before up" 
from the first sentence.


But wasn't sure for the SLAAC case. Will autoconf work if I set the flag 
after the interface is up? Should I delete the "before up" here, too?


Gerhard




+.Pp
+To use IPv6, configure a link-local address.
+If the device is able to connect to the ISP's network but doesn't
+show an IPv6 address, setting the
+.Sy AUTOCONF6
+flag on the interface before bringing it up may help.
+.Ed





Re: IPv6 Support for umb(4)

2020-04-30 Thread Theo de Raadt
Is that still the true behaviour?  I think it isn't, the "before up"
aspect is gone isn't it?


+.Pp
+To use IPv6, configure a link-local address.
+If the device is able to connect to the ISP's network but doesn't
+show an IPv6 address, setting the
+.Sy AUTOCONF6
+flag on the interface before bringing it up may help.
+.Ed



Re: IPv6 Support for umb(4)

2020-04-30 Thread Gerhard Roth
On Mon, 27 Apr 2020 16:59:22 +0200
Gerhard Roth  wrote:

> On 4/27/20 4:53 PM, Theo de Raadt wrote:
> > Gerhard Roth  wrote:
> >   
> >> Hi Theo,
> >>
> >> On 4/27/20 4:39 PM, Theo de Raadt wrote:  
> >>> Is this code in umb_decode_ip_configuration() reached again, if
> >>> you do a late ifconfig (don't set inet6 at up time, but set it
> >>> later)  
> >>
> >> no, seting inet6 later doesn't work. On MBIM level I have to tell the
> >> device *before* the CONNECT whether I want IPv6 support or not. And
> >> there is no way to change that selection later on while we are
> >> connected.
> >>
> >> The only way to do this transparently would be an implicit disconnect
> >> followed by a reconnect in if_umb.c. But then I would need some trigger
> >> that is called every time someone does 'ifconfig umb0 inet6 eui64' or
> >> 'ifconfig umb0 -inet6'. And I'm not sure whether the implicit
> >> temporary link loss is appreciated by the user.  
> > 
> > Then this diff seems incorrect.
> > 
> > The alternative of disconnecting, is not nice.
> > 
> > Perhaps umb can always request inet6 at startup, but not actually expose
> > it to the network stack.  Then enabling the software inet6 flag can expose
> > inet6 to the network layer, disabling the inet6 flag can remove exposure
> > to the network layer, but the actual address and support is always attempted
> > by the driver?
> >   
> 
> 
> That would work. Will try this.


So here is an updated diff that behaves in the proposed way.
umb_decode_ip_configuration() just stores the IP addresses in softc
and the real configuration is now done in umb_configure().
The later we also call, when our address-change hook is called.

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.418 Feb 2020 08:09:37 -  1.10
+++ share/man/man4/umb.430 Apr 2020 11:47:55 -
@@ -40,6 +40,13 @@ 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.
+If the device is able to connect to the ISP's network but doesn't
+show an IPv6 address, setting the
+.Sy AUTOCONF6
+flag on the interface before bringing it up may help.
+.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.33
diff -u -p -u -p -r1.33 if_umb.c
--- sys/dev/usb/if_umb.c27 Apr 2020 11:16:51 -  1.33
+++ sys/dev/usb/if_umb.c30 Apr 2020 11:18:54 -
@@ -147,6 +147,9 @@ void umb_newstate(struct umb_softc *, 
 voidumb_state_task(void *);
 voidumb_up(struct umb_softc *);
 voidumb_down(struct umb_softc *, int);
+#ifdef INET6
+voidumb_addr6_change(void *);
+#endif
 
 voidumb_get_response_task(void *);
 
@@ -164,11 +167,10 @@ intumb_decode_packet_service(struct u
 int umb_decode_signal_state(struct umb_softc *, void *, int);
 int umb_decode_connect_info(struct umb_softc *, void *, int);
 voidumb_clear_addr(struct umb_softc *);
-int umb_add_inet_config(struct umb_softc *, struct in_addr, u_int,
-   struct in_addr);
-int umb_add_inet6_config(struct umb_softc *, struct in6_addr *,
-   u_int, struct in6_addr *);
+int umb_add_inet_config(struct umb_softc *);
+int umb_add_inet6_config(struct umb_softc *);
 voidumb_send_inet_proposal(struct umb_softc *, int);
+int umb_configure(struct umb_softc *);
 int umb_decode_ip_configuration(struct umb_softc *, void *, int);
 voidumb_rx(struct umb_softc *);
 voidumb_rxeof(struct usbd_xfer *, void *, usbd_status);
@@ -477,6 +479,11 @@ umb_attach(struct device *parent, struct
USB_TASK_TYPE_GENERIC);
timeout_set(>sc_statechg_timer, umb_statechg_timeout, sc);
 
+#ifdef INET6
+   task_set(>sc_atask, umb_addr6_change, sc);
+   task_set(>sc_ctask, (void (*)(void *))umb_configure, sc);
+#endif
+
if (usbd_open_pipe_intr(uaa->iface, ctrl_ep, USBD_SHORT_XFER_OK,
>sc_ctrl_pipe, sc, >sc_intr_msg, sizeof (sc->sc_intr_msg),
umb_intr, USBD_DEFAULT_INTERVAL)) {
@@ -530,6 +537,10 @@ umb_attach(struct device *parent, struct
 #if NBPFILTER > 0
bpfattach(>if_bpf, ifp, DLT_LOOP, sizeof(uint32_t));
 #endif
+#if INET6
+   if_addrhook_add(ifp, >sc_atask);
+#endif
+
/*
 * Open the device now so that we are able to query device information.
 * XXX maybe close when done?
@@ -553,6 +564,9 @@ 

Re: IPv6 Support for umb(4)

2020-04-27 Thread Gerhard Roth

On 4/27/20 4:53 PM, Theo de Raadt wrote:

Gerhard Roth  wrote:


Hi Theo,

On 4/27/20 4:39 PM, Theo de Raadt wrote:

Is this code in umb_decode_ip_configuration() reached again, if
you do a late ifconfig (don't set inet6 at up time, but set it
later)


no, seting inet6 later doesn't work. On MBIM level I have to tell the
device *before* the CONNECT whether I want IPv6 support or not. And
there is no way to change that selection later on while we are
connected.

The only way to do this transparently would be an implicit disconnect
followed by a reconnect in if_umb.c. But then I would need some trigger
that is called every time someone does 'ifconfig umb0 inet6 eui64' or
'ifconfig umb0 -inet6'. And I'm not sure whether the implicit
temporary link loss is appreciated by the user.


Then this diff seems incorrect.

The alternative of disconnecting, is not nice.

Perhaps umb can always request inet6 at startup, but not actually expose
it to the network stack.  Then enabling the software inet6 flag can expose
inet6 to the network layer, disabling the inet6 flag can remove exposure
to the network layer, but the actual address and support is always attempted
by the driver?




That would work. Will try this.



Re: IPv6 Support for umb(4)

2020-04-27 Thread Theo de Raadt
Gerhard Roth  wrote:

> Hi Theo,
> 
> On 4/27/20 4:39 PM, Theo de Raadt wrote:
> > Is this code in umb_decode_ip_configuration() reached again, if
> > you do a late ifconfig (don't set inet6 at up time, but set it
> > later)
> 
> no, seting inet6 later doesn't work. On MBIM level I have to tell the
> device *before* the CONNECT whether I want IPv6 support or not. And
> there is no way to change that selection later on while we are
> connected.
> 
> The only way to do this transparently would be an implicit disconnect
> followed by a reconnect in if_umb.c. But then I would need some trigger
> that is called every time someone does 'ifconfig umb0 inet6 eui64' or
> 'ifconfig umb0 -inet6'. And I'm not sure whether the implicit
> temporary link loss is appreciated by the user.

Then this diff seems incorrect.

The alternative of disconnecting, is not nice.

Perhaps umb can always request inet6 at startup, but not actually expose
it to the network stack.  Then enabling the software inet6 flag can expose
inet6 to the network layer, disabling the inet6 flag can remove exposure
to the network layer, but the actual address and support is always attempted
by the driver?



Re: IPv6 Support for umb(4)

2020-04-27 Thread Gerhard Roth

Hi Theo,

On 4/27/20 4:39 PM, Theo de Raadt wrote:

Is this code in umb_decode_ip_configuration() reached again, if
you do a late ifconfig (don't set inet6 at up time, but set it
later)


no, seting inet6 later doesn't work. On MBIM level I have to tell the 
device *before* the CONNECT whether I want IPv6 support or not. And 
there is no way to change that selection later on while we are connected.


The only way to do this transparently would be an implicit disconnect
followed by a reconnect in if_umb.c. But then I would need some trigger
that is called every time someone does 'ifconfig umb0 inet6 eui64' or 
'ifconfig umb0 -inet6'. And I'm not sure whether the implicit temporary 
link loss is appreciated by the user.


Gerhard




That is how other network interfaces work.  I'm trying to make
sure this behaviour isn't too weird (ie. requiring a down, then up).

Gerhard Roth  wrote:


And since IPv6 is now optional for umb(4), we can just skip
evaluation of the IPv6 part of the IP configuration, if it
wasn't enabled.

Gerhard


Index: sys/dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 if_umb.c
--- sys/dev/usb/if_umb.c27 Apr 2020 11:16:51 -  1.33
+++ sys/dev/usb/if_umb.c27 Apr 2020 13:56:09 -
@@ -1937,6 +1937,10 @@ tryv6:;
/*
 * IPv6 configuation
 */
+   if ((sc->sc_flags & UMBFLG_NO_INET6) ||
+   in6ifa_ifpforlinklocal(GET_IFP(sc), 0) == NULL)
+   goto done;
+
avail_v6 = letoh32(ic->ipv6_available);
if (avail_v6 == 0) {
if (ifp->if_flags & IFF_DEBUG)





Re: IPv6 Support for umb(4)

2020-04-27 Thread Theo de Raadt
Is this code in umb_decode_ip_configuration() reached again, if
you do a late ifconfig (don't set inet6 at up time, but set it
later)

That is how other network interfaces work.  I'm trying to make
sure this behaviour isn't too weird (ie. requiring a down, then up).

Gerhard Roth  wrote:

> And since IPv6 is now optional for umb(4), we can just skip
> evaluation of the IPv6 part of the IP configuration, if it
> wasn't enabled.
> 
> Gerhard
> 
> 
> Index: sys/dev/usb/if_umb.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.33
> diff -u -p -u -p -r1.33 if_umb.c
> --- sys/dev/usb/if_umb.c  27 Apr 2020 11:16:51 -  1.33
> +++ sys/dev/usb/if_umb.c  27 Apr 2020 13:56:09 -
> @@ -1937,6 +1937,10 @@ tryv6:;
>   /*
>* IPv6 configuation
>*/
> + if ((sc->sc_flags & UMBFLG_NO_INET6) ||
> + in6ifa_ifpforlinklocal(GET_IFP(sc), 0) == NULL)
> + goto done;
> +
>   avail_v6 = letoh32(ic->ipv6_available);
>   if (avail_v6 == 0) {
>   if (ifp->if_flags & IFF_DEBUG)
> 



Re: IPv6 Support for umb(4)

2020-04-27 Thread Gerhard Roth
And since IPv6 is now optional for umb(4), we can just skip
evaluation of the IPv6 part of the IP configuration, if it
wasn't enabled.

Gerhard


Index: sys/dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 if_umb.c
--- sys/dev/usb/if_umb.c27 Apr 2020 11:16:51 -  1.33
+++ sys/dev/usb/if_umb.c27 Apr 2020 13:56:09 -
@@ -1937,6 +1937,10 @@ tryv6:;
/*
 * IPv6 configuation
 */
+   if ((sc->sc_flags & UMBFLG_NO_INET6) ||
+   in6ifa_ifpforlinklocal(GET_IFP(sc), 0) == NULL)
+   goto done;
+
avail_v6 = letoh32(ic->ipv6_available);
if (avail_v6 == 0) {
if (ifp->if_flags & IFF_DEBUG)



Re: IPv6 Support for umb(4)

2020-04-27 Thread Gerhard Roth
Hello Claudio,

On Mon, 27 Apr 2020 11:51:50 +0200 Claudio Jeker  
wrote:
> On Mon, Apr 27, 2020 at 10:26:01AM +0200, Gerhard Roth wrote:
> > Should we change umb(4) so that it only grabs an IPv6 address
> > in case somebody does a "ifconfig umb0 inet6 eui64" first?
> > 
> > Anyone willing to ok the patch below?  
> 
> see below
>  
> > On 2/19/20 9:19 AM, Gerhard Roth wrote:  
> > > On Wed, 19 Feb 2020 08:45:39 +0100 Claudio Jeker 
> > >  wrote:  
> > > > On Tue, Feb 18, 2020 at 11:16:54PM +, 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 -  1.10
> > > +++ share/man/man4/umb.4  19 Feb 2020 08:14:01 -
> > > @@ -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.  
> 
> I think I asked this already, how does one know if autoconf is needed or
> not. Is that only device specific or also provider dependent?
> Adding autoconf will enable slaacd(8) on the device. What kind of troubles
> could result from this on a device that do not need it?
> In general this paragraph does not really help me to understand the
> situation better.

Basically, the MBIM protocol allows us to obtain the IPv6
address the ISP has given to us with the MBIM_CID_IP_CONFIGURATION
query. And all the provider I've met have done so, so far.

Yet job@ had a Japanese ISP that didn't give any IP address
via the MBIM protocol but wanted us to use regular IPv6
protocols to obtain the IP address.

Now this is a little bit too much detail for a man page.
And there is no technical way to find out what is required apart
from trial and error.

So I rephrased the sentence to:

If the device is able to connect to the ISP's network
but doesn't show an IPv6 address, setting the
AUTOCONF6 on the interface before bringing it up may help.

(see updated diff below)
Does that sound better?

> 
> > > +.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  
> 
> In my opinion this is a bad example, it mixes a lot of different options
> which are not explained to the user in that man page. Also I think it is
> bad practice to put everything on one line. ifconfig(8) is rather
> sensitive when it comes to multiple commands in a single invocation.
> If I look at this and compare it with my hostname.umb0 file
> pin 
> up
> I have more questions, what is apn and why would I need, where do I find
> what I need to put there. ifconfig umb0 does not show anything named apn.

The Access Point Name (APN) shows up in ifconfig as soon as you
set one. Explaining the meaning of the APN is IMHO beyond the
scope of a section 4 man page. For example, we don't explain
the meaning of SSID for WiFi, either.


> Also why is -roaming after inet6 eui64 autoconf? Isn't -roaming default?
> 
> Ideally the first paragraph explains the IPv6 setup well enough that no
> example config is needed here.

Ok, removed the example.


> 
> > >   .Sh HARDWARE
> > >   The following devices should work:
> > >   .Pp
> > > Index: sys/dev/usb/if_umb.c
> > > ===
> > > RCS file: 

Re: IPv6 Support for umb(4)

2020-04-27 Thread Claudio Jeker
On Mon, Apr 27, 2020 at 10:26:01AM +0200, Gerhard Roth wrote:
> Should we change umb(4) so that it only grabs an IPv6 address
> in case somebody does a "ifconfig umb0 inet6 eui64" first?
> 
> Anyone willing to ok the patch below?

see below
 
> On 2/19/20 9:19 AM, Gerhard Roth wrote:
> > On Wed, 19 Feb 2020 08:45:39 +0100 Claudio Jeker  
> > wrote:
> > > On Tue, Feb 18, 2020 at 11:16:54PM +, 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.418 Feb 2020 08:09:37 -  1.10
> > +++ share/man/man4/umb.419 Feb 2020 08:14:01 -
> > @@ -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.

I think I asked this already, how does one know if autoconf is needed or
not. Is that only device specific or also provider dependent?
Adding autoconf will enable slaacd(8) on the device. What kind of troubles
could result from this on a device that do not need it?
In general this paragraph does not really help me to understand the
situation better.

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

In my opinion this is a bad example, it mixes a lot of different options
which are not explained to the user in that man page. Also I think it is
bad practice to put everything on one line. ifconfig(8) is rather
sensitive when it comes to multiple commands in a single invocation.
If I look at this and compare it with my hostname.umb0 file
pin 
up
I have more questions, what is apn and why would I need, where do I find
what I need to put there. ifconfig umb0 does not show anything named apn.
Also why is -roaming after inet6 eui64 autoconf? Isn't -roaming default?

Ideally the first paragraph explains the IPv6 setup well enough that no
example config is needed here.

> >   .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.c18 Feb 2020 08:09:37 -  1.32
> > +++ sys/dev/usb/if_umb.c18 Feb 2020 12:35:45 -
> > @@ -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));
> > 

The code diff is fine by me.

-- 
:wq Claudio



Re: IPv6 Support for umb(4)

2020-04-27 Thread Gerhard Roth

Should we change umb(4) so that it only grabs an IPv6 address
in case somebody does a "ifconfig umb0 inet6 eui64" first?

Anyone willing to ok the patch below?



On 2/19/20 9:19 AM, Gerhard Roth wrote:

On Wed, 19 Feb 2020 08:45:39 +0100 Claudio Jeker  
wrote:

On Tue, Feb 18, 2020 at 11:16:54PM +, 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.418 Feb 2020 08:09:37 -  1.10
+++ share/man/man4/umb.419 Feb 2020 08:14:01 -
@@ -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.c18 Feb 2020 08:09:37 -  1.32
+++ sys/dev/usb/if_umb.c18 Feb 2020 12:35:45 -
@@ -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));





Re: IPv6 Support for umb(4)

2020-02-19 Thread Gerhard Roth
On Wed, 19 Feb 2020 08:45:39 +0100 Claudio Jeker  
wrote:
> On Tue, Feb 18, 2020 at 11:16:54PM +, 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.418 Feb 2020 08:09:37 -  1.10
+++ share/man/man4/umb.419 Feb 2020 08:14:01 -
@@ -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.c18 Feb 2020 08:09:37 -  1.32
+++ sys/dev/usb/if_umb.c18 Feb 2020 12:35:45 -
@@ -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));



Re: IPv6 Support for umb(4)

2020-02-18 Thread Claudio Jeker
On Tue, Feb 18, 2020 at 11:16:54PM +, 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.c18 Feb 2020 08:09:37 -  1.32
> > +++ sys/dev/usb/if_umb.c18 Feb 2020 12:35:45 -
> > @@ -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



Re: IPv6 Support for umb(4)

2020-02-18 Thread 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 -  1.32
> +++ sys/dev/usb/if_umb.c  18 Feb 2020 12:35:45 -
> @@ -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));



Re: IPv6 Support for umb(4)

2020-02-18 Thread Gerhard Roth
On Tue, 18 Feb 2020 12:11:05 + Stuart Henderson  
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.c18 Feb 2020 08:09:37 -  1.32
+++ sys/dev/usb/if_umb.c18 Feb 2020 12:35:45 -
@@ -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));



Re: IPv6 Support for umb(4)

2020-02-18 Thread Stuart Henderson
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.



Re: IPv6 Support for umb(4)

2020-02-17 Thread 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  
> 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 
> > >  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: 576 up / 1440 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(, data + off, sizeof (ipv6elem));
> > > > > + memcpy(, ipv6elem.addr, sizeof (addr6));
> > > > > +
> > > > > + off = letoh32(ic->ipv6_gwoffs);
> > > > > + memcpy(, 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 

Re: IPv6 Support for umb(4)

2020-02-17 Thread Gerhard Roth
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  
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  
> > 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: 576 up / 1440 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(, data + off, sizeof (ipv6elem));
> > > > +   memcpy(, ipv6elem.addr, sizeof (addr6));
> > > > +
> > > > +   off = letoh32(ic->ipv6_gwoffs);
> > > > +   memcpy(, 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
> > 

Re: IPv6 Support for umb(4)

2020-02-17 Thread Claudio Jeker
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  
> 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: 
> > 576 up / 1440 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(, data + off, sizeof (ipv6elem));
> > > + memcpy(, ipv6elem.addr, sizeof (addr6));
> > > +
> > > + off = letoh32(ic->ipv6_gwoffs);
> > > + memcpy(, 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 -  1.417
> +++ sbin/ifconfig/ifconfig.c  28 Jan 2020 12:16:23 -
> @@ -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 *), 0, sizeof(mi));
>   

Re: IPv6 Support for umb(4)

2020-02-04 Thread 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: 
576 up / 1440 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: 576 up / 2100 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 -  1.417
> +++ sbin/ifconfig/ifconfig.c  28 Jan 2020 12:16:23 -
> @@ -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 *), 0, sizeof(mi));
>   ifr.ifr_data = (caddr_t)
> @@ -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, [i], astr, sizeof (astr)));
> + }
> + for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> + if (memcmp([i], _any,
> + sizeof (mi.ipv6dns[i])) == 0)
> + break;
> + printf("%s %s", n++ ? "" : "\tdns",
> + inet_ntop(AF_INET6, [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 -  1.9
> +++ share/man/man4/umb.4  28 Jan 2020 12:16:23 -
> @@ -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 -  1.31
> +++ sys/dev/usb/if_umb.c  4 Feb 2020 07:50:30 -
> @@ -43,6 +43,14 @@
>  #include 
>  #include 
>
> +#ifdef INET6
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#endif
> +
>  #include 
>
>  #include 
> @@ -158,7 +166,9 @@ intumb_decode_connect_info(struct umb
>  void  

Re: IPv6 Support for umb(4)

2020-02-04 Thread Gerhard Roth
Hi Alex,

thanks for looking into it.


On Tue, 4 Feb 2020 00:20:42 +0100 Alexander Bluhm  
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: 
> 576 up / 1440 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(, data + off, sizeof (ipv6elem));
> > +   memcpy(, ipv6elem.addr, sizeof (addr6));
> > +
> > +   off = letoh32(ic->ipv6_gwoffs);
> > +   memcpy(, 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.c27 Dec 2019 14:34:46 -  1.417
+++ sbin/ifconfig/ifconfig.c28 Jan 2020 12:16:23 -
@@ -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 *), 0, sizeof(mi));
ifr.ifr_data = (caddr_t)
@@ -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, [i], 

Re: IPv6 Support for umb(4)

2020-02-03 Thread Alexander Bluhm
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: 
576 up / 1440 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(, data + off, sizeof (ipv6elem));
> + memcpy(, ipv6elem.addr, sizeof (addr6));
> +
> + off = letoh32(ic->ipv6_gwoffs);
> + memcpy(, 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



Re: IPv6 Support for umb(4)

2020-02-03 Thread Gerhard Roth
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  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 -  1.417
> +++ sbin/ifconfig/ifconfig.c  23 Jan 2020 09:24:38 -
> @@ -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 *), 0, sizeof(mi));
>   ifr.ifr_data = (caddr_t)
> @@ -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, [i], astr, sizeof (astr)));
> + }
> + for (i = 0; i < UMB_MAX_DNSSRV; i++) {
> + if (memcmp([i], _any,
> + sizeof (mi.ipv6dns[i])) == 0)
> + break;
> + printf("%s %s", n++ ? "" : "\tdns",
> + inet_ntop(AF_INET6, [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 -  1.9
> +++ share/man/man4/umb.4  28 Jan 2020 09:04:20 -
> @@ -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 -  1.31
> +++ sys/dev/usb/if_umb.c  28 Jan 2020 09:08:16 -
> @@ -43,6 +43,14 @@
>  #include 
>  #include 
>  
> +#ifdef INET6
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#endif
> +
>  #include 
>  
>  #include 
> @@ -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",
> + 

Re: IPv6 Support for umb(4)

2020-01-29 Thread Job Snijders
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 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 898103010455217 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 0xff00
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