Re: MBIM Patch (Round 2)
On Tue, 7 Jun 2016 16:31:21 +0100 Stuart Henderson <s...@spacehopper.org> wrote: > On 2016/06/07 14:39, Gerhard Roth wrote: > > > > Now I get an IP address from my provider, I want something like this: > > > > > > > > inet 10.75.178.41 --> 10.75.178.42 netmask 0xfffc > > > > > > > > But if I used SIOCAIFADDR I would get this instead: > > > > > > > > inet 0.0.0.1 --> 0.0.0.2 netmask 0xff00 > > > > inet 10.75.178.41 --> 10.75.178.42 netmask 0xfffc > > > > > > > > And deleting the old one first seems more racy. > > > > > > Why? > > > > Because either there is a time without any IP address or there > > is one with two addresses. Neither seems right. > > Is there a problem to have no address? That is what happens with > gif(4), tun(4) etc, even with ethernet interfaces, when they don't > have a particular address configured on them. > > There doesn't seem a particular problem with dhclient removing > the old address before it adds a new one.. > > > See above. That's the current OpenBSD model for PPP. > > There are several models :-) > > The 0.0.0.1 thing is a specific hack for sppp/pppoe which has > "magic" values of 0.0.0.0 and 0.0.0.1 and a way to request a > specific hardcoded address. This is because *both* sides > propose addresses via IPCP (they are "equal" peers as far > as configuration is concerned), they have to agree and can > NAK each other. > > It's not the only way though, even for PPP; ppp(4) doesn't use > this mechanism, it has separate configuration for pppd instead. > When we had ppp(8), that didn't, either. > > As far as I understand MBIM addressing is driven from the "mobile > network" side, we just set the address they are telling us to use > without a way to propose our own address. If that's correct then > we don't need a way to set "our side's" address at all, or have > a way to tie down the remote side's address, so address > configuration can be a lot simpler than any PPP interface. I not sure if it is possible to send packets from an interface that has no addresses assigned to it. And then, the MBIM standard says nothing about the network segment. My provider just gives me some non-routable 10.x.x.x IP, but I think that it would be totally legal to assign some public IPv4 address to me. In that case it is mandatory that we assign this address to our local interface. > > > > But when you lose the connection with the network or you manually set > > > the interface down, this IP address once given to you by the provider > > > is no longer yours and must not be used anymore. > > The same can apply in other cases (wifi, wired ethernet etc) too, > but we don't do anything to remove it from those interfaces (though > it's easy enough to remove using ifstated if needed). For WiFi and Ethernet it is dhclient that will remove the old addresses.
Re: MBIM Patch (Round 2)
On Tue, 7 Jun 2016 13:08:49 +0200 Martin Pieuchot <m...@openbsd.org> wrote:
> On 07/06/16(Tue) 11:53, Gerhard Roth wrote:
> > On Mon, 6 Jun 2016 10:30:11 +0200 Martin Pieuchot <m...@openbsd.org> wrote:
> > > On 01/06/16(Wed) 17:20, Gerhard Roth wrote:
> > > Any reason for using a different license in documentation an code?
> >
> > Different in what sense? Which paragraph do you mean?
>
> This is a BSD 2-clauses the code is ISC.
>
> > I just copied it from some other man page in share/man/man4. Can't
> > remember which one it was. But they all say pretty much the same thing.
>
> Well if you don't mind, please use /usr/share/misc/license.template :)
Seriously, you don't mean that!
Did you take a look at that file?
>
> > > > +/*
> > > > + * Some devices are picky about too frequent control messages.
> > > > + * Query device state not more often than every 0.5 secs.
> > > > + */
> > > > +struct timeval umbim_update_rate = { 0, 50 };
> > > ^
> > > This variable seems unused. What it is for?
> >
> > Some remnant from an earlier version.
> >
> >
> > >
> > > > +int umbim_delay = 4000;
> > >
> > > What is this supposed to be?
> >
> > A value?
> >
> > A litte delay in between the requests sent to the device.
> > Using a global variable helps to tune the value via DDB in the
> > development stage.
>
> You're paraphrasing your code :) My unclear question was just why
> do you need such delay? ;)
Well, ask the firmware writer. I don't know.
>
> > > > + break;
> > > > + case SIOCSIFFLAGS:
> > > > + usb_add_task(sc->sc_udev, >sc_umbim_task);
> > >
> > > I guess you want to wait for the task to complete here. You're not
> > > respecting the API by returning directly. But why using a task? To
> > > serialize up/down operations?
> >
> > No, waiting here is not an option. In order to get a complete configuration
> > of the device you need a mobile connection with the next base station.
> > Would you really want "ifconfig up" to block indefinitely when you're in
> > the basement without network access?
> >
> > The task is there, because going "up" needs a lot of steps with
> > requests being sent to device and then waiting for the device to
> > respond to them.
>
> I understand you point. My concern is just that SIOCSIFFLAGS indicates
> that the interface is down/up once the ioclt(2) is finished. It doesn't
> seem to be the case in your driver, I just hope this is not a problem.
How about WiFi? AFAIK it only starts scanning for an access point
but doesn't block until it finished association. So why should it
be a problem here?
>
> > > > + if (ifr->ifr_mtu > ifp->if_hardmtu) {
> > > > + error = EINVAL;
> > > > + break;
> > > > + }
> > > > + ifp->if_mtu = ifr->ifr_mtu;
> > > > + break;
> > > > + case SIOCGIFMTU:
> > > > + ifr->ifr_mtu = ifp->if_mtu;
> > > > + break;
> > > > + case SIOCGIFHARDMTU:
> > > > + ifr->ifr_hardmtu = ifp->if_hardmtu;
> > > > + break;
> > > > + case SIOCAIFADDR:
> > >
> > > It is accepted that this ioctl(2) implicitly bring the interface up.
> >
> > But this is a point-to-point interface and behaves a little bit
> > different from regular Ethernet interfaces. You're not supposed to
> > add IP destination addresses manually. Your provider will tell them to you.
> > And in case you do try to add them manually, that will not get a
> > working network interface.
>
> I'm not saying it makes sense. I'm just explaining how it is.
>
> > > What about routing domains? Did you try your device in a rdomain != 0?
> >
> > No I didn't try them. But what could happen? This is a point-to-point
> > interface. No more routing decisions should be necessary on that level.
>
> What could happen? Good question. I don't know.
>
> > > > +void
> > > > +umbim_linkstate(struct umbim_softc *sc, int state)
> > > > +{
> > > > + struct ifnet *ifp = GET_IFP(sc);
> > > > + int s;
> > > > +
> > > > + s = splnet();
> > > > +
Re: MBIM Patch (Round 2)
On Mon, 6 Jun 2016 10:30:11 +0200 Martin Pieuchot <m...@openbsd.org> wrote: > On 01/06/16(Wed) 17:20, Gerhard Roth wrote: > > [...] > > Thanks for all the feedback. > > More comments inline. Replies too. > > > Index: sbin/ifconfig/ifconfig.c > > === > > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v > > retrieving revision 1.322 > > diff -u -p -u -p -r1.322 ifconfig.c > > --- sbin/ifconfig/ifconfig.c3 May 2016 17:52:33 - 1.322 > > +++ sbin/ifconfig/ifconfig.c1 Jun 2016 14:32:18 - > > @@ -107,6 +107,8 @@ > > #include > > > > #include "brconfig.h" > > +#include > > +#include > > Does USB mobile broadband interfaces share a spec with non-USB devices? > In other words would it make sense to move (some of) the defines in net/ > rather than usb/? No, the MBIM spec is pure USB stuff that was schemed by usb.org. > In any case these two include lines should be before the standard ones > and under #ifndef SMALL. That's right. > [...] > > Index: share/man/man4/umbim.4 > > === > > RCS file: share/man/man4/umbim.4 > > diff -N share/man/man4/umbim.4 > > --- /dev/null 1 Jan 1970 00:00:00 - > > +++ share/man/man4/umbim.4 1 Jun 2016 14:32:18 - > > @@ -0,0 +1,95 @@ > > +.\" Copyright (c) 2016 genua mbH > > +.\" All rights reserved. > > +.\" > > +.\" Redistribution and use in source and binary forms, with or without > > +.\" modification, are permitted provided that the following conditions > > +.\" are met: > > +.\" > > +.\"- Redistributions of source code must retain the above copyright > > +.\" notice, this list of conditions and the following disclaimer. > > +.\"- Redistributions in binary form must reproduce the above > > +.\" copyright notice, this list of conditions and the following > > +.\" disclaimer in the documentation and/or other materials provided > > +.\" with the distribution. > > +.\" > > +.\" THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > > +.\" "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > +.\" LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS > > +.\" FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE > > +.\" COPYRIGHT HOLDERS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, > > +.\" INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, > > +.\" BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; > > +.\" LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER > > +.\" CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT > > +.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN > > +.\" ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE > > +.\" POSSIBILITY OF SUCH DAMAGE. > > Any reason for using a different license in documentation an code? Different in what sense? Which paragraph do you mean? I just copied it from some other man page in share/man/man4. Can't remember which one it was. But they all say pretty much the same thing. > > > +.Sh CAVEATS > > +The > > +.Nm > > +driver currently does not support IPv6 addresses. > > +.Pp > > +Roaming can be prevented by the driver. This feature hasn't been tested. > > +Please don't kill me in case your phone bills rack up sky high. > > I wouldn't put the last sentence in this manual. The license already > say that. Ok, it's gone. > > > Index: sys/dev/usb/if_umbim.c > > === > > RCS file: sys/dev/usb/if_umbim.c > > diff -N sys/dev/usb/if_umbim.c > > --- /dev/null 1 Jan 1970 00:00:00 - > > +++ sys/dev/usb/if_umbim.c 1 Jun 2016 14:32:19 - > > @@ -0,0 +1,2441 @@ > > +/* $OpenBSD$ */ > > + > > +/* > > + * Copyright (c) 2016 genua mbH > > + * All rights reserved. > > + * > > + * Permission to use, copy, modify, and distribute this software for any > > + * purpose with or without fee is hereby granted, provided that the above > > + * copyright notice and this permission notice appear in all copies. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIE
Re: MBIM Patch (Round 2)
On 01.06.2016 20:28, Theo de Raadt wrote: As I said, we could still change the name of the interface to 'ubm' while keeping 'umbim' as the name of the driver. No, I don't understand the proposal. I think it should be ubm throughout, or I am threatening to rename ix(4) to a 8 character name. Changing the name from 'mbim' to 'umbim' already took me serveral hours because a simple sed(1) can't do the job. Don't have that much lifetime left to go through this boredom once again. BTW: if "ifconfig umbim0" is annoyingly long, we could just replace ifconfig(8) with ip(8). That would save even more keystrokes ;)
Re: MBIM Patch (Round 2)
On 01.06.2016 17:32, Theo de Raadt wrote: - renamed it from 'mbim' to 'umbim' I tried 'ubm' as proposed by Theo but that felt weird. Esp. when changing the prefixes of macros whose names were derived from the MBIM standard. I suggested that because ifconfig umbim0 ... Is annoyingly long. As I said, we could still change the name of the interface to 'ubm' while keeping 'umbim' as the name of the driver. Should I?
Re: MBIM Patch - Part 2 of 4
On Mon, 23 May 2016 17:47:28 +0200 Martin Pieuchot <m...@openbsd.org> wrote: > On 23/05/16(Mon) 16:51, Gerhard Roth wrote: > > On Mon, 23 May 2016 16:18:29 +0200 Martin Pieuchot <m...@openbsd.org> wrote: > > > On 23/05/16(Mon) 15:38, Gerhard Roth wrote: > > > > > > This is crazy :) No driver should ever modify `ia' directly. This > > > code should call in_control() via the ioctl path. > > > > As mentioned in a previous mail: this was mostly copied from > > if_spppsubr.c:sppp_set_ip_addrs(). But doing an SIOCSIFADDR > > ioctl() from inside the kernel seems weird, too. > > SIOCAIFADDR/SIOCDIFADDR It is the way to go. The driver should not > manipulate addresses or route entry. Not manipulating the route entries is simple to fix. Will do that. But using SIOCAIFADDR/SIOCDIFADDR seems rather awkward since in_control() requires a 'struct socket *so' argument (even though it does nothing with it, except checking 'so->so_state & SS_PRIV'). Creating a socket inside the driver for this sole purpose seems just as weird as setting up a fake struct socket. Are you really sure, this is the better way to go?
Re: MBIM Patch - Part 2 of 4
On Mon, 23 May 2016 17:47:28 +0200 Martin Pieuchot <m...@openbsd.org> wrote:
> On 23/05/16(Mon) 16:51, Gerhard Roth wrote:
> > On Mon, 23 May 2016 16:18:29 +0200 Martin Pieuchot <m...@openbsd.org> wrote:
> > > On 23/05/16(Mon) 15:38, Gerhard Roth wrote:
> > > > This is part 2 of the MBIM patch. It adds the mbim driver to i386
> > > > +/*
> > > > + * Some devices are picky about too frequent control messages.
> > > > + * Query device state not more often than every 0.5 secs.
> > > > + */
> > > > +struct timeval mbim_update_rate = { 0, 50 };
> > > > +int mbim_delay = 4000;
> > > > +
> > > > +/*
> > > > + * Normally, MBIM devices are detected by their interface class and
> > > > subclass.
> > > > + * But for some models that have multiple configurations, it is better
> > > > to
> > > > + * match by vendor and product id so that we can select the desired
> > > > + * configuration ourselves.
> > > > + *
> > > > + * OTOH, some devices identifiy themself als an MBIM device but fail
> > > > to speak
> > > > + * the MBIM protocol.
> > > > + */
> > >
> > > Why is it better? This is just working around usb_probe_and_attach()
> > > and require developer to add an entry for every device we need to
> > > support.
> >
> > I just thought that some modules that are already in use say with a
> > umsm config would otherwise change to mbim and break the setup. The idea
> > was to keep existing modules first as they are now and start adding
> > new one to the list.
>
> Well the rule is first match win. Since umsm(4) uses a vendor/product
> table as long as you don't add your device to this table it should be
> ok.
>
Now I remember why I needed to match on VID/PID, too. I have one
MBIM module (Sierra Wireless MC8305) that shows some strange behaviour:
If you read the configuration descriptor #1 and then set
configuration #2, the device resets itself and shortly disapears
from the USB bus.
When matching VID/PID, I can prevent usbd_probe_and_attach() from reading
configuration #1.
Without matching VID/PID, usbd_probe_and_attach() will read configuration #1
to try matching all the interfaces provided by the device. But then I not
able to switch to configuration #2 anymore.
I'm quite sure this is a firmware bug and the only workaround I have
is the match by VID/PID. That still allows to match other MBIM devices
not listed in the table to be matched by Class/SubClass.
Any suggestions?
Gerhard
Re: MBIM Patch - Part 1 of 4
On Sat, 28 May 2016 09:18:13 +0200 Stefan Sperling <s...@stsp.name> wrote: > On Mon, May 23, 2016 at 03:37:32PM +0200, Gerhard Roth wrote: > > I have this stuff around quite a while now, but since patrick@ asked > > me repeatedly, I think it is time to share this now. > > > > This is a patch that adds support for the "Mobile Broadband Interface > > Model" (MBIM) from http://www.usb.org/. It allows to attach UMTS/LTE > > USB devices as a network interface. > > > > I'm not asking for OKs at this time, but hope that some people will > > give it test. I tested with two different Sierra Wireless modules > > (EM8805, MC8305) and each one behaved a little bit different. > > > > In order to configure the network interface, this is what you do: > > > > # ifconfig mbim0 pin 1234 apn internet.t-mobile > > # ifconfig mbim0 inet 0.0.0.1 0.0.0.2 > > # route delete default > > # route add -ifp mbim0 default 0.0.0.2 > > # ifconfig mbim0 up > > > > The mbim interface is a point-to-point type interface and will > > update the default route, once it is registered in the network. > > As others have already said, I think the problem with these diffs > is that you're trying to solve too many problems at once. > > I believe the problems this driver should solve first and foremost are getting > a network link and getting an IP assigned to the interface so people can use > that IP to set up whatever routing they want. > > Could mbim in theory support IPv6? We don't need to do anything about > this now. But the design shouldn't assume it will only support IPv4. Yes mbim does support IPv6, but the code is still missing. I just don't have a SIM card for a provider that gives me an IPv6 address. So I could only implement it without testing. > > Don't worry about breaking existing umsm(4) setups. > Since this driver provides a much simpler user interface than umsm coupled > with pppd(8) I'd suggest to eventually prefer attaching mbim instead of umsm > where possible. I hate having to copy pppd config files around for umsm. > If I lost those files I'd have to spend a day or so to get the setup working > again because I don't remember (and don't want to remember) all the AT and > up/down scripts I used to get umsm to work. That's why I think mbim is > promising. > > > To get extended information on the interface use: > > > > # ifconfig mbim0 devinfo > > I don't see the need to add a new subcommand for this. > If this information is important why is it not shown as part of the > default 'ifconfig mbim0' output? If it is not important, why show it? > > I agree with Theo that the ifconfig diff is too large. And it seems you're > exposing too many details at this layer. How much of it can be removed while > still solving the problem of getting a network link and an IP assigned? > Can you compress status information on the 'status:' line, instead of printing > seperate lines for data items such as pin, error state, rssi, ber and so on? > How much of this information do users really need? Already removed the 'devinfo' part and compressed the information onto fewer lines. That will come with the next update of the patch.
Re: MBIM Patch - Part 2 of 4
On 23.05.2016 17:47, Martin Pieuchot wrote: On 23/05/16(Mon) 16:51, Gerhard Roth wrote: Why do you need to set a default route in the first place? Just like PPP this was designed as a point-to-point interface. The idea is that once you get an uplink, all traffic should be routed through it. Sure, my point is can you add the route *once* you get an uplink? Ok, all of this was meant to be an intermediate solution ;) The idea was that once the MBIM driver was accepted into the OpenBSD kernel, the information provided by the MBIM_CID_IP_CONFIGURATION msg would generate an extended version of the RTM_IFINFO routing message. This message would pass the DNS server and the default gateway that was reported by the provider up to any listeners as RTAX_DNS1, RTAX_DNS2, and RTAX_GATEWAY. Then I would write a daemon sitting there and listening for this kind of routing messages and once it receives them, perform the required actions, i.e. updating /etc/resolv.conf and/or setting the default route. But that was planned as a second step. The first step would be the simple solution we have there right now. And because it would go away later on, I didn't give it too much thought. Gerhard
Re: MBIM Patch - Part 2 of 4
On Mon, 23 May 2016 16:18:29 +0200 Martin Pieuchot <m...@openbsd.org> wrote:
> On 23/05/16(Mon) 15:38, Gerhard Roth wrote:
> > This is part 2 of the MBIM patch. It adds the mbim driver to i386
>
> Comments inline.
Replies too.
>
> > Index: sys/dev/usb/if_mbim.c
> > ===
> > RCS file: sys/dev/usb/if_mbim.c
> > diff -N sys/dev/usb/if_mbim.c
> > --- /dev/null 1 Jan 1970 00:00:00 -
> > +++ sys/dev/usb/if_mbim.c 23 May 2016 09:50:08 -
> > +
> > +struct cfdriver mbim_cd = {
> > + NULL, "mbim", DV_DULL
> > +};
> > +
> > +struct cfattach mbim_ca = {
> > + sizeof (struct mbim_softc),
> > + mbim_match,
> > + mbim_attach,
> > + mbim_detach,
> > + mbim_activate,
> > +};
>
> This struct can be const.
Will do
>
> > +/*
> > + * Some devices are picky about too frequent control messages.
> > + * Query device state not more often than every 0.5 secs.
> > + */
> > +struct timeval mbim_update_rate = { 0, 50 };
> > +int mbim_delay = 4000;
> > +
> > +/*
> > + * Normally, MBIM devices are detected by their interface class and
> > subclass.
> > + * But for some models that have multiple configurations, it is better to
> > + * match by vendor and product id so that we can select the desired
> > + * configuration ourselves.
> > + *
> > + * OTOH, some devices identifiy themself als an MBIM device but fail to
> > speak
> > + * the MBIM protocol.
> > + */
>
> Why is it better? This is just working around usb_probe_and_attach()
> and require developer to add an entry for every device we need to
> support.
I just thought that some modules that are already in use say with a
umsm config would otherwise change to mbim and break the setup. The idea
was to keep existing modules first as they are now and start adding
new one to the list.
>
> > +int
> > +mbim_match(struct device *parent, void *match, void *aux)
> > +{
> > + struct usb_attach_arg *uaa = aux;
> > + usb_interface_descriptor_t *id;
> > +
> > + if (usb_lookup(mbim_blacklist, uaa->vendor, uaa->product) != NULL)
> > + return UMATCH_NONE;
> > + if (mbim_lookup(uaa->vendor, uaa->product) != NULL)
> > + return UMATCH_VENDOR_PRODUCT;
> > + if (!uaa->iface)
> > + return UMATCH_NONE;
> > + if (!uaa->iface ||
> > + (id = usbd_get_interface_descriptor(uaa->iface)) == NULL)
> > + return UMATCH_NONE;
> > + if (id->bInterfaceClass != UICLASS_CDC ||
> > + id->bInterfaceSubClass !=
> > + UISUBCLASS_MOBILE_BROADBAND_INTERFACE_MODEL ||
> > + id->bNumEndpoints != 1)
> > + return UMATCH_NONE;
> > +
> > + return UMATCH_DEVCLASS_DEVSUBCLASS;
> > +}
> > +
> > +void
> > +mbim_attach(struct device *parent, struct device *self, void *aux)
> > +{
> > + struct mbim_softc *sc = (struct mbim_softc *)self;
> > + struct usb_attach_arg *uaa = aux;
> > + usbd_status status;
> > + struct usbd_desc_iter iter;
> > + const usb_descriptor_t *desc;
> > + int v;
> > + struct mbim_descriptor *md;
> > + int i;
> > + struct usbd_interface *ctrl_iface = NULL;
> > + int ctrl_ep;
> > + uint8_t data_ifaceno;
> > + usb_interface_descriptor_t *id;
> > + usb_config_descriptor_t *cd;
> > + usb_endpoint_descriptor_t *ed;
> > + int s;
> > + struct ifnet *ifp;
> > + int hard_mtu;
> > +
> > + sc->sc_udev = uaa->device;
> > +
> > + if (uaa->configno < 0) {
> > + uaa->configno = mbim_lookup(uaa->vendor, uaa->product)->confno;
> > + DPRINTF("%s: switching to config #%d\n", DEVNAM(sc),
> > + uaa->configno);
> > + status = usbd_set_config_no(sc->sc_udev, uaa->configno, 1);
> > + if (status) {
> > + printf("%s: failed to switch to config #%d: %s\n",
> > + DEVNAM(sc), uaa->configno, usbd_errstr(status));
> > + goto fail;
> > + }
> > + }
> > +
> > + sc->sc_ver_maj = sc->sc_ver_min = -1;
> > + usbd_desc_iter_init(sc->sc_udev, );
> > + hard_mtu = MBIM_MAXSEGSZ_MINVAL;
> > + while ((desc = usbd_desc_iter_next())) {
> > + if (desc->bDescriptorType != UDESC_CS_INTERFACE)
> > +
Re: MBIM Patch - Part 1 of 4
On Mon, 23 May 2016 15:54:36 +0200 Martin Pieuchot <m...@openbsd.org> wrote: > On 23/05/16(Mon) 15:37, Gerhard Roth wrote: > > I have this stuff around quite a while now, but since patrick@ asked > > me repeatedly, I think it is time to share this now. > > > > This is a patch that adds support for the "Mobile Broadband Interface > > Model" (MBIM) from http://www.usb.org/. It allows to attach UMTS/LTE > > USB devices as a network interface. > > That's great! > > > I'm not asking for OKs at this time, but hope that some people will > > give it test. I tested with two different Sierra Wireless modules > > (EM8805, MC8305) and each one behaved a little bit different. > > > > In order to configure the network interface, this is what you do: > > > > # ifconfig mbim0 pin 1234 apn internet.t-mobile > > I'd like to bikeshed early, we're trying to prefix all new USB driver > name with 'u'. So I'd suggest a rename when this goes in tree. Of course no problem. Will fix that with the next version of the patch. > > > # ifconfig mbim0 inet 0.0.0.1 0.0.0.2 > > # route delete default > > # route add -ifp mbim0 default 0.0.0.2 > > # ifconfig mbim0 up > > > > The mbim interface is a point-to-point type interface and will > > update the default route, once it is registered in the network. > > Can't you insert the route later instead of updating it? Have to try. I fact this part of the code is more or less copied from sppp_update_gw() in if_spppsubr.c > > > [...] uhub.c may be needed: some modules appear at the usb bus just > > to detach themselves again and then reappear shortly after. > > Do you know why? On which controller? Oh, I don't think this is a problem of the controller. It's the device itself that just detaches and re-attaches again. Guess the firmware has a bug. Here's an example: usbd_fill_iface_data: bad max packet size usbd_fill_iface_data: bad max packet size ugen2 at uhub4 port 2 "Sierra Wireless, Incorporated EM8805" rev 2.00/0.00 addr 3 ugen2: setting configuration index 0 failed ugen2 detached mbim1 at uhub4 port 2 "Sierra Wireless, Incorporated EM8805" rev 2.00/0.06 addr 3 Gerhard
MBIM Patch - Part 1 of 4
I have this stuff around quite a while now, but since patrick@ asked
me repeatedly, I think it is time to share this now.
This is a patch that adds support for the "Mobile Broadband Interface
Model" (MBIM) from http://www.usb.org/. It allows to attach UMTS/LTE
USB devices as a network interface.
I'm not asking for OKs at this time, but hope that some people will
give it test. I tested with two different Sierra Wireless modules
(EM8805, MC8305) and each one behaved a little bit different.
In order to configure the network interface, this is what you do:
# ifconfig mbim0 pin 1234 apn internet.t-mobile
# ifconfig mbim0 inet 0.0.0.1 0.0.0.2
# route delete default
# route add -ifp mbim0 default 0.0.0.2
# ifconfig mbim0 up
The mbim interface is a point-to-point type interface and will
update the default route, once it is registered in the network.
To get extended information on the interface use:
# ifconfig mbim0 devinfo
The patch comes in 4 parts. This is the first part that provides
some changes required to the rest of the kernel. Esp. the change
in uhub.c may be needed: some modules appear at the usb bus just
to detach themselves again and then reappear shortly after.
After patching, please do a
# cd /sys/dev/usb
# make
to rebuild usbdevs.h and usbdevs_data.h.
In case of problems, please add
option MBIM_DEBUG
to generic and set the variable 'mbim_debug'.
Have fun,
Gerhard
Index: sys/dev/usb/uhub.c
===
RCS file: /cvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.88
diff -u -p -u -p -r1.88 uhub.c
--- sys/dev/usb/uhub.c 29 Nov 2015 16:30:48 - 1.88
+++ sys/dev/usb/uhub.c 23 May 2016 09:50:08 -
@@ -523,7 +523,9 @@ uhub_port_connect(struct uhub_softc *sc,
{
struct usbd_port *up = >sc_hub->hub->ports[port-1];
int speed;
+ int retry = 1;
+again:
/* We have a connect status change, handle it. */
usbd_clear_port_feature(sc->sc_hub, port, UHF_C_PORT_CONNECTION);
@@ -613,6 +615,11 @@ uhub_port_connect(struct uhub_softc *sc,
* some other serious problem. Since we cannot leave
* at 0 we have to disable the port instead.
*/
+ if (retry--) {
+ printf("%s: port %d: retrying\n", DEVNAME(sc), port);
+ goto again;
+ }
+
printf("%s: device problem, disabling port %d\n", DEVNAME(sc),
port);
usbd_clear_port_feature(sc->sc_hub, port, UHF_PORT_ENABLE);
Index: sys/dev/usb/usb.h
===
RCS file: /cvs/src/sys/dev/usb/usb.h,v
retrieving revision 1.54
diff -u -p -u -p -r1.54 usb.h
--- sys/dev/usb/usb.h 28 Feb 2016 17:57:50 - 1.54
+++ sys/dev/usb/usb.h 23 May 2016 09:50:08 -
@@ -508,6 +508,7 @@ typedef struct usb_port_status usb_port_
#define UISUBCLASS_ETHERNET_NETWORKING_CONTROL_MODEL 6
#define UISUBCLASS_ATM_NETWORKING_CONTROL_MODEL 7
#define UISUBCLASS_MOBILE_DIRECT_LINE_MODEL10
+#define UISUBCLASS_MOBILE_BROADBAND_INTERFACE_MODEL 14
#define UIPROTO_CDC_AT 1
#define UICLASS_HID0x03
@@ -545,6 +546,7 @@ typedef struct usb_port_status usb_port_
#define UICLASS_CDC_DATA 0x0a
#define UISUBCLASS_DATA 0
+#define UIPROTO_DATA_MBIM0x02/* MBIM */
#define UIPROTO_DATA_ISDNBRI 0x30/* Physical iface */
#define UIPROTO_DATA_HDLC0x31/* HDLC */
#define UIPROTO_DATA_TRANSPARENT 0x32/* Transparent */
Index: sys/dev/usb/usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.664
diff -u -p -u -p -r1.664 usbdevs
--- sys/dev/usb/usbdevs 20 May 2016 02:04:24 - 1.664
+++ sys/dev/usb/usbdevs 23 May 2016 09:50:08 -
@@ -3832,7 +3832,9 @@ product SIERRA AC881U 0x6856 881U
product SIERRA AC885U 0x6880 885U
product SIERRA C01SW 0x6890 C01SW
product SIERRA USB305 0x68a3 USB305
+product SIERRA MC8305 0x9011 MC8305
product SIERRA MC8355 0x9013 MC8355
+product SIERRA EM8805 0x9041 EM8805
product SIERRA AIRCARD_770S0x9053 Aircard 770S
/* Sigmatel products */
Index: sys/net/if_types.h
===
RCS file: /cvs/src/sys/net/if_types.h,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 if_types.h
--- sys/net/if_types.h 7 Mar 2016 19:33:26 - 1.20
+++ sys/net/if_types.h 23 May 2016 09:50:09 -
@@ -268,5 +268,6 @@
#defineIFT_CARP0xf7/* Common Address Redundancy
Protocol */
#define IFT_BLUETOOTH 0xf8/* Bluetooth */
#define IFT_PFLOW 0xf9/*
MBIM Patch - Part 4 of 4
This is the final patch for MBIM which adds a manual page for mbim(4). Index: share/man/man4/Makefile === RCS file: /cvs/src/share/man/man4/Makefile,v retrieving revision 1.621 diff -u -p -u -p -r1.621 Makefile --- share/man/man4/Makefile 11 May 2016 21:52:49 - 1.621 +++ share/man/man4/Makefile 23 May 2016 09:50:08 - @@ -33,9 +33,9 @@ MAN= aac.4 ac97.4 acphy.4 \ ix.4 ixgb.4 jmb.4 jme.4 jmphy.4 \ kate.4 km.4 ksyms.4 kue.4 lc.4 lge.4 lii.4 lisa.4 lm.4 \ lmenv.4 lmn.4 lmtemp.4 lo.4 lpt.4 lxtphy.4 luphy.4 \ - maestro.4 mainbus.4 malo.4 maxds.4 maxtmp.4 mbg.4 midi.4 mii.4 mfi.4 \ - mfii.4 mlphy.4 moscom.4 mos.4 mpe.4 mpath.4 mpi.4 mpii.4 mpu.4 msk.4 \ - mpw.4 msts.4 mtd.4 mtdphy.4 multicast.4 mtio.4 myx.4 \ + maestro.4 mainbus.4 malo.4 maxds.4 maxtmp.4 mbg.4 mbim.4 midi.4 mii.4 \ + mfi.4 mfii.4 mlphy.4 moscom.4 mos.4 mpe.4 mpath.4 mpi.4 mpii.4 mpu.4 \ + msk.4 mpw.4 msts.4 mtd.4 mtdphy.4 multicast.4 mtio.4 myx.4 \ ne.4 neo.4 nep.4 netintro.4 nfe.4 nge.4 nmea.4 \ nsclpcsio.4 nsgphy.4 nsphy.4 nsphyter.4 null.4 nviic.4 nvme.4 nvt.4 \ oce.4 ohci.4 options.4 onewire.4 oosiop.4 osiop.4 otus.4 \ Index: share/man/man4/mbim.4 === RCS file: share/man/man4/mbim.4 diff -N share/man/man4/mbim.4 --- /dev/null 1 Jan 1970 00:00:00 - +++ share/man/man4/mbim.4 23 May 2016 09:50:08 - @@ -0,0 +1,95 @@ +.\" Copyright (c) 2016 genua mbH +.\" All rights reserved. +.\" +.\" Redistribution and use in source and binary forms, with or without +.\" modification, are permitted provided that the following conditions +.\" are met: +.\" +.\"- Redistributions of source code must retain the above copyright +.\" notice, this list of conditions and the following disclaimer. +.\"- Redistributions in binary form must reproduce the above +.\" copyright notice, this list of conditions and the following +.\" disclaimer in the documentation and/or other materials provided +.\" with the distribution. +.\" +.\" THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +.\" "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +.\" LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +.\" FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +.\" COPYRIGHT HOLDERS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +.\" INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +.\" BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +.\" LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +.\" CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +.\" ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +.\" POSSIBILITY OF SUCH DAMAGE. +.\" +.\" $OpenBSD$ +.\" +.Dd $Mdocdate$ +.Dt MBIM 4 +.Os +.Sh NAME +.Nm mbim +.Nd USB Mobile Broadband Interface Model +.Sh SYNOPSIS +.Cd "mbim* at uhub?" +.Sh DESCRIPTION +The +.Nm +driver provides support for USB MBIM devices. Those devices allow to +establish connections via celluar networks such as GPRS, UMTS, LTE, etc. +.Pp +The +.Nm +device appears as a regular point-to-point network interface, +transporting raw IP frames. +.Pp +Required configuration parameters like PIN and APN have to be set +via +.Xr ifconfig 8 . +Once the SIM card has has been unlocked with the correct PIN, it +will remain in this state until the device is power-cycled. +In case the device is connected to an "always-on" USB port, +it is possible to connect to a provider without entering the +PIN again even afer a reboot of the system. +.Pp +If a default gateway route is configured for the +.Nm +network interface, the driver will modify the destination IP address +dynamically, according to the information sent by the network provider. +.Sh HARDWARE +The following devices are known to be supported by the +.Nm +driver: +.Pp +.Bl -tag -width Ds -offset indent -compact +.It Tn Sierra Wireless MC8305 +.It Tn Sierra Wireless EM8805 +.El +.Pp +There are probably a lot more devices that also work flawlessly. +If some devices fail to provide a confirming MBIM implementation, +their vendor and product IDs should be added to the driver's blacklist +manually. Since most device offer multiple interfaces, blacklisted ones +will probably be attached by some other driver, e.g. +.Xr umsm 4 . +.Sh SEE ALSO +.Xr intro 4 , +.Xr netintro 4 , +.Xr usb 4 , +.Xr hostname.if 5 , +.Xr ifconfig 8 +.Xr route 8 +.Rs +.%T "Universal Serial Bus Communications Class Subclass Specification for Mobile Broadband Interface Model" +.%U http://www.usb.org/developers/docs/devclass_docs/MBIM10Errata1_073013.zip +.Re +.Sh CAVEATS +The +.Nm +driver currently does not support IPv6 addresses. +.Pp +Roaming can be prevented by the driver.
MBIM Patch - Part 3 of 4
Part 3 of the MBIM patch updates ifconfig(8).
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.267
diff -u -p -u -p -r1.267 ifconfig.8
--- sbin/ifconfig/ifconfig.86 Apr 2016 10:07:14 - 1.267
+++ sbin/ifconfig/ifconfig.823 May 2016 09:50:08 -
@@ -519,6 +519,8 @@ tunnel
.Xr vxlan 4 )
.It
.Xr vlan 4
+.It
+.Xr mbim 4
.El
.\" BRIDGE
.Sh BRIDGE
@@ -1645,6 +1647,71 @@ will be assigned 802.1Q tag 5.
Disassociate from the parent interface.
This breaks the link between the vlan interface and its parent,
clears its vlan tag, flags, and link address, and shuts the interface down.
+.El
+.\" MBIM
+.Sh MBIM
+.nr nS 1
+.Bk -words
+.Nm ifconfig
+.Ar mbim-interface
+.Op Cm pin Ar pin
+.Op Cm chgpin Ar oldpin Ar newpin
+.Op Cm puk Ar puk Ar newpin
+.Op Oo Fl Oc Ns Cm apn Ar apn
+.Op Oo Fl Oc Ns Cm class Ar class,class,...
+.Op Oo Fl Oc Ns Cm roaming
+.Ek
+.nr nS 0
+.Pp
+The following options are available for an
+.Xr mbim 4
+interface:
+.Bl -tag -width Ds
+.It Cm pin Ar pin
+Enter the PIN required to unlock the SIM card. Most SIM cards will not
+allow to establish a network association without providing a PIN.
+.It Cm chgpin Ar oldpin Ar newpin
+Permanently changes the PIN of the SIM card from the current value
+.Ar oldpin
+to
+.Ar newpin .
+.It Cm puk Ar puk Ar newpin
+Sets the PIN of the SIM card to
+.Ar newpin
+using the PUK
+.Ar puk
+to validate the request.
+.It Cm apn Ar apn
+Set the "Access Point Name" required by your network provider.
+.It Fl apn
+Clear the current "Access Point Name" value.
+.It Cm class
+List all available cell classes.
+.It Cm class Ar class,class,...
+Set the preferred cell classes. Apart from those listed by
+.Nm Cm class
+the following aliases can be used:
+.Ar 4G,
+.Ar 3G,
+and
+.Ar 2G.
+.It Fl class
+Clear any cell class preferences.
+.It Cm roaming
+Enable data roaming.
+.It Fl roaming
+Disable data roaming.
+.It Cm up
+As soon as the interface is marked as "up", the
+.Xr mbim 4
+device will try to establish a data connection with the service provider.
+.It Cm down
+Marking the interface as "down" will terminate any existing data connection
+and deregister with the service provider.
+.It Cm devinfo
+Provides some additional information about the
+.Xr mbim 4
+device, e.g. the IMEI number.
.El
.Sh EXAMPLES
Assign the
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.322
diff -u -p -u -p -r1.322 ifconfig.c
--- sbin/ifconfig/ifconfig.c3 May 2016 17:52:33 - 1.322
+++ sbin/ifconfig/ifconfig.c23 May 2016 09:50:08 -
@@ -107,6 +107,8 @@
#include
#include "brconfig.h"
+#include
+#include
#define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
#define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
@@ -143,8 +145,10 @@ intLflag = 1;
intshowmediaflag;
intshowcapsflag;
+intshowdevinfoflag;
intshownet80211chans;
intshownet80211nodes;
+intshowclasses;
void notealias(const char *, int);
void setifaddr(const char *, int);
@@ -275,6 +279,19 @@ void unsetifdesc(const char *, int);
void printifhwfeatures(const char *, int);
void setpair(const char *, int);
void unsetpair(const char *, int);
+void mbim_status(void);
+void mbim_devinfo(const char *, int);
+void mbim_printclasses(char *, int);
+intmbim_parse_classes(const char *);
+void mbim_setpin(const char *, int);
+void mbim_chgpin(const char *, const char *);
+void mbim_puk(const char *, const char *);
+void mbim_pinop(int, int, const char *, const char *);
+void mbim_apn(const char *, int);
+void mbim_setclass(const char *, int);
+void mbim_roaming(const char *, int);
+void utf16_to_char(uint16_t *, int, char *, size_t);
+intchar_to_utf16(const char *, uint16_t *, size_t);
#else
void setignore(const char *, int);
#endif
@@ -486,6 +503,16 @@ const struct cmd {
{ "-descr", 1, 0, unsetifdesc },
{ "wol",IFXF_WOL, 0, setifxflags },
{ "-wol", -IFXF_WOL, 0, setifxflags },
+ { "devinfo",NEXTARG0, 0, mbim_devinfo },
+ { "pin",NEXTARG,0, mbim_setpin },
+ { "chgpin", NEXTARG2, 0, NULL, mbim_chgpin },
+ { "puk",NEXTARG2, 0, NULL, mbim_puk },
+ { "apn",NEXTARG,0, mbim_apn },
+ { "-apn", -1, 0, mbim_apn },
+ { "class", NEXTARG0, 0, mbim_setclass },
+ { "-class", -1, 0, mbim_setclass },
+ { "roaming",1, 0, mbim_roaming },
+ { "-roaming", 0, 0,
MBIM Patch - Part 2 of 4
This is part 2 of the MBIM patch. It adds the mbim driver to i386
and amd64 kernels.
Index: sys/arch/amd64/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.418
diff -u -p -u -p -r1.418 GENERIC
--- sys/arch/amd64/conf/GENERIC 7 May 2016 23:10:50 - 1.418
+++ sys/arch/amd64/conf/GENERIC 23 May 2016 09:50:08 -
@@ -279,6 +279,7 @@ urtw* at uhub?# Realtek 8187
rsu* at uhub?# Realtek RTL8188SU/RTL8191SU/RTL8192SU
urtwn* at uhub?# Realtek RTL8188CU/RTL8192CU
udcf* at uhub?# Gude Expert mouseCLOCK
+mbim* at uhub?# Mobile Broadband Interface Model
uthum* at uhidev? # TEMPerHUM sensor
ugold* at uhidev? # gold TEMPer sensor
utrh* at uhidev? # USBRH sensor
Index: sys/arch/i386/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/i386/conf/GENERIC,v
retrieving revision 1.814
diff -u -p -u -p -r1.814 GENERIC
--- sys/arch/i386/conf/GENERIC 24 Apr 2016 17:30:31 - 1.814
+++ sys/arch/i386/conf/GENERIC 23 May 2016 09:50:08 -
@@ -14,6 +14,9 @@ include "../../../conf/GENERIC"
maxusers 80 # estimated number of users
option USER_PCICONF# user-space PCI configuration
+option USB_DEBUG
+#optionUHUB_DEBUG
+option EHCI_DEBUG
#optionVM86# Virtual 8086 emulation
option KVM86 # Kernel Virtual 8086 emulation
@@ -314,6 +317,7 @@ rsu*at uhub?# Realtek
RTL8188SU/RTL81
urtwn* at uhub?# Realtek RTL8188CU/RTL8192CU
udcf* at uhub?# Gude Expert mouseCLOCK
umbg* at uhub?# Meinberg Funkuhren USB5131
+mbim* at uhub?# Mobile Broadband Interface Model
uthum* at uhidev? # TEMPerHUM sensor
ugold* at uhidev? # gold TEMPer sensor
utrh* at uhidev? # USBRH sensor
Index: sys/dev/usb/files.usb
===
RCS file: /cvs/src/sys/dev/usb/files.usb,v
retrieving revision 1.126
diff -u -p -u -p -r1.126 files.usb
--- sys/dev/usb/files.usb 8 Jan 2016 15:54:13 - 1.126
+++ sys/dev/usb/files.usb 23 May 2016 09:50:08 -
@@ -397,6 +397,11 @@ device otus: ether, ifnet, ifmedia, wlan
attach otus at uhub
file dev/usb/if_otus.c otus
+# Mobile Broadband Interface Model
+device mbim: ifnet, ifmedia
+attach mbim at uhub
+file dev/usb/if_mbim.c mbim
+
# USB logical device
device usbf {}
attach usbf at usbdev
Index: sys/dev/usb/if_mbim.c
===
RCS file: sys/dev/usb/if_mbim.c
diff -N sys/dev/usb/if_mbim.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ sys/dev/usb/if_mbim.c 23 May 2016 09:50:08 -
@@ -0,0 +1,2487 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2016 genua mbH
+ * All rights reserved.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/*
+ * Mobile Broadband Interface Model
+ * http://www.usb.org/developers/docs/devclass_docs/MBIM-Compliance-1.0.pdf
+ */
+#include "bpfilter.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#if NBPFILTER > 0
+#include
+#endif
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#ifdef MBIM_DEBUG
+#define DPRINTF(x...) \
+ do { if (mbim_debug) log(LOG_DEBUG, x); } while (0)
+
+#define DPRINTFN(n, x...) \
+ do { if (mbim_debug >= (n)) log(LOG_DEBUG, x); } while (0)
+
+#define DDUMPN(n, b, l)
\
+ do {\
+ if (mbim_debug >= (n)) \
+
snmpd: some values need casting
Hi,
ber_add_integer() can ASN.1 encode integers of up to 64 bit. Yet for
some types (e.g. SNMP_T_TIMETICKS, SNMP_T_GAUGE32, ..) the MIB says that
the value must no exceed 2^32-1. We should cast the value to u_int32_t
to avoid that e.g. Gauge32 carries a value larger than 32 bit.
One special thing is the interface baud rate, where the MIB in RFC 2863
states that the value should be truncated to 4,294,967,295 in case it is
"greater than the maximum value reportable by this object".
Gerhard
Index: usr.sbin/snmpd/mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.80
diff -u -p -u -p -r1.80 mib.c
--- usr.sbin/snmpd/mib.c17 Nov 2015 12:30:23 - 1.80
+++ usr.sbin/snmpd/mib.c28 Jan 2016 12:19:01 -
@@ -155,7 +155,7 @@ mib_getsys(struct oid *oid, struct ber_o
break;
case 3:
ticks = smi_getticks();
- *elm = ber_add_integer(*elm, ticks);
+ *elm = ber_add_integer(*elm, (u_int32_t)ticks);
ber_set_header(*elm, BER_CLASS_APPLICATION, SNMP_T_TIMETICKS);
break;
case 4:
@@ -463,7 +463,8 @@ mib_hrsystemuptime(struct oid *oid, stru
if (sysctl(mib, 2, , , NULL, 0) == -1)
return (-1);
- *elm = ber_add_integer(*elm, (now - boottime.tv_sec) * 100);
+ *elm = ber_add_integer(*elm,
+ (u_int32_t)((now - boottime.tv_sec) * 100));
ber_set_header(*elm, BER_CLASS_APPLICATION, SNMP_T_TIMETICKS);
return (0);
@@ -1079,6 +1080,7 @@ mib_iftable(struct oid *oid, struct ber_
size_t len;
int ifq;
int mib[] = { CTL_NET, PF_INET, IPPROTO_IP, 0, 0 };
+ u_int32_tbaudrate;
/* Get and verify the current row index */
idx = o->bo_id[OIDIDX_ifEntry];
@@ -1115,7 +1117,11 @@ mib_iftable(struct oid *oid, struct ber_
ber = ber_add_integer(ber, kif->if_mtu);
break;
case 5:
- ber = ber_add_integer(ber, kif->if_baudrate);
+ if (kif->if_baudrate <= UINT32_MAX)
+ baudrate = kif->if_baudrate;
+ else
+ baudrate = UINT32_MAX;
+ ber = ber_add_integer(ber, baudrate);
ber_set_header(ber, BER_CLASS_APPLICATION, SNMP_T_GAUGE32);
break;
case 6:
@@ -1145,7 +1151,7 @@ mib_iftable(struct oid *oid, struct ber_
ber = ber_add_integer(ber, i);
break;
case 9:
- ber = ber_add_integer(ber, kif->if_ticks);
+ ber = ber_add_integer(ber, (u_int32_t)kif->if_ticks);
ber_set_header(ber, BER_CLASS_APPLICATION, SNMP_T_TIMETICKS);
break;
case 10:
@@ -1328,7 +1334,7 @@ int
mib_ifstacklast(struct oid *oid, struct ber_oid *o, struct ber_element **elm)
{
struct ber_element *ber = *elm;
- ber = ber_add_integer(ber, kr_iflastchange());
+ ber = ber_add_integer(ber, (u_int32_t)kr_iflastchange());
ber_set_header(ber, BER_CLASS_APPLICATION, SNMP_T_TIMETICKS);
return (0);
}
@@ -1667,7 +1673,7 @@ mib_pfinfo(struct oid *oid, struct ber_o
else
runtime = 0;
runtime *= 100;
- *elm = ber_add_integer(*elm, runtime);
+ *elm = ber_add_integer(*elm, (u_int32_t)runtime);
ber_set_header(*elm, BER_CLASS_APPLICATION, SNMP_T_TIMETICKS);
break;
case 3:
@@ -2168,7 +2174,7 @@ mib_pftables(struct oid *oid, struct ber
break;
case 20:
tzero = (time(NULL) - ts.pfrts_tzero) * 100;
- ber = ber_add_integer(ber, tzero);
+ ber = ber_add_integer(ber, (u_int32_t)tzero);
ber_set_header(ber, BER_CLASS_APPLICATION, SNMP_T_TIMETICKS);
break;
case 21:
@@ -2224,7 +2230,8 @@ mib_pftableaddrs(struct oid *oid, struct
ber = ber_add_integer(ber, as.pfras_a.pfra_net);
break;
case 4:
- ber = ber_add_integer(ber, (time(NULL) - as.pfras_tzero) * 100);
+ ber = ber_add_integer(ber,
+ (u_int32_t)((time(NULL) - as.pfras_tzero) * 100));
ber_set_header(ber, BER_CLASS_APPLICATION, SNMP_T_TIMETICKS);
break;
case 5:
@@ -3044,7 +3051,8 @@ mib_ipstat(struct oid *oid, struct ber_o
for (i = 0;
(u_int)i < (sizeof(mapping) / sizeof(mapping[0])); i++) {
if (oid->o_oid[OIDIDX_ip] == mapping[i].m_id) {
- *elm = ber_add_integer(*elm, *mapping[i].m_ptr);
+ *elm = ber_add_integer(*elm,
+ (u_int32_t)*mapping[i].m_ptr);
ber_set_header(*elm,
Exclude invalid sensors from the sensors MIB
Sensors marked as invalid should be excluded by snmpd(8) from the sensors
MIB just as sysctl(8) excludes them from the 'hw.sensors' tree.
Gerhard
Index: usr.sbin/snmpd/mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.79
diff -u -p -u -p -r1.79 mib.c
--- usr.sbin/snmpd/mib.c8 Oct 2015 08:29:21 - 1.79
+++ usr.sbin/snmpd/mib.c17 Nov 2015 10:39:31 -
@@ -2556,7 +2556,7 @@ mib_sensors(struct oid *oid, struct ber_
}
for (j = 0; j < SENSOR_MAX_TYPES; j++) {
mib[3] = j;
- for (k = 0; k < sensordev.maxnumt[j]; k++, n++) {
+ for (k = 0; k < sensordev.maxnumt[j]; k++) {
mib[4] = k;
if (sysctl(mib, 5,
, , NULL, 0) == -1) {
@@ -2566,8 +2566,11 @@ mib_sensors(struct oid *oid, struct ber_
break;
return (-1);
}
+ if (sensor.flags & SENSOR_FINVALID)
+ continue;
if (n == idx)
goto found;
+ n++;
}
}
}
snmpd loses ARP table information
Hi,
snmpd pernanently loses its ARP table information:
# snmpctl walk 127.0.0.1 oid ipNetToMediaPhysAddress
ipNetToMediaPhysAddress.2.192.168.16.1="xx:xx:xx:xx:xx:xx"
ipNetToMediaPhysAddress.2.192.168.16.126="xx:xx:xx:xx:xx:xx"
ipNetToMediaPhysAddress.2.192.168.19.132="xx:xx:xx:xx:xx:xx"
Query the whole tree and this information is gone:
# snmpctl walk 127.0.0.1 oid iso.org>/dev/null
# snmpctl walk 127.0.0.1 oid ipNetToMediaPhysAddress
0=6
The reason is that several OIDs in mib.c call kr_updateif()
and this function deletes the kif_node and the restores it
via fetchifs(). But then the ARP information held by the
old kif_node is gone.
There is no need to delete the kif_node prior to calling
fetchifs() since it only calls rtmsg_process() which is
also called during normal processing of messages from the
routing socket.
And while here, also handle RTM_DESYNC messages.
Gerhard
Index: usr.sbin/snmpd/kroute.c
===
RCS file: /cvs/src/usr.sbin/snmpd/kroute.c,v
retrieving revision 1.31
diff -u -p -u -p -r1.31 kroute.c
--- usr.sbin/snmpd/kroute.c 18 Jul 2015 00:27:32 - 1.31
+++ usr.sbin/snmpd/kroute.c 2 Nov 2015 11:41:51 -
@@ -365,12 +365,6 @@ kr_iflastchange(void)
int
kr_updateif(u_int if_index)
{
- struct kif_node *kn;
-
- if ((kn = kif_find(if_index)) != NULL)
- kif_remove(kn);
-
- /* Do not update the interface address list */
return (fetchifs(if_index));
}
@@ -1380,6 +1374,12 @@ rtmsg_process(char *buf, int len)
break;
case RTM_IFANNOUNCE:
if_announce(next);
+ break;
+ case RTM_DESYNC:
+ kr_shutdown();
+ if (fetchifs(0) == -1)
+ fatalx("rtmsg_process: fetchifs");
+ ktable_init();
break;
default:
/* ignore for now */
Fix for handling SNMP GETBULK Requests
Hi,
there's a bug in snmpd that breaks GETBULK requests for multiple OIDs.
Example:
# OID1=1.3.6.1.2.1.1.1
# OID2=1.3.6.1.2.1.31.1.1.1.1
# snmpbulkget -Cr3 -c public -v2c localhost $OID1
SNMPv2-MIB::sysDescr.0 = STRING: OpenBSD null 5.7 GENERIC#123 i386
SNMPv2-MIB::sysObjectID.0 = OID: OPENBSD-BASE-MIB::openBSDDefaultObjectID
SNMPv2-MIB::sysUpTime.0 = Timeticks: (68096) 0:11:20.96
# snmpbulkget -Cr3 -c public -v2c localhost $OID2
IF-MIB::ifName.1 = STRING: em0
IF-MIB::ifName.2 = STRING: iwi0
IF-MIB::ifName.3 = STRING: enc0
# snmpbulkget -Cr3 -c public -v2c localhost $OID1 $OID2
SNMPv2-MIB::sysDescr.0 = STRING: OpenBSD null 5.7 GENERIC#123 i386
IF-MIB::ifName.1 = STRING: em0
IF-MIB::ifName.2 = STRING: iwi0
IF-MIB::ifName.3 = STRING: enc0
Each query for a single OID delivers three repetitions (as requested by
-Cr3). But the query for two OIDs skips the repetitions for the first
one. If we add more OIDs, only the last OID of the query will have
repetitions in the reply.
The reason is that we must link the result of each mps_getbulkreq()
to the end of the previous list and not the start of it.
With the patch below, we get the desired result:
# snmpbulkget -Cr3 -c public -v2c localhost $OID1 $OID2
SNMPv2-MIB::sysDescr.0 = STRING: OpenBSD null 5.7 GENERIC#123 i386
SNMPv2-MIB::sysObjectID.0 = OID: OPENBSD-BASE-MIB::openBSDDefaultObjectID
SNMPv2-MIB::sysUpTime.0 = Timeticks: (239) 0:00:02.39
IF-MIB::ifName.1 = STRING: em0
IF-MIB::ifName.2 = STRING: iwi0
IF-MIB::ifName.3 = STRING: enc0
Gerhard
Index: mps.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mps.c,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 mps.c
--- mps.c 16 Jan 2015 00:05:13 - 1.20
+++ mps.c 3 Jun 2015 08:48:42 -
@@ -289,7 +289,7 @@ mps_getnextreq(struct snmp_message *msg,
int
mps_getbulkreq(struct snmp_message *msg, struct ber_element **root,
-struct ber_oid *o, int max)
+struct ber_element **end, struct ber_oid *o, int max)
{
struct ber_element *c, *d, *e;
size_t len;
@@ -297,14 +297,17 @@ mps_getbulkreq(struct snmp_message *msg,
j = max;
c = *root;
+ *end = NULL;
for (d = NULL, len = 0; j 0; j--) {
e = ber_add_sequence(NULL);
if (c == NULL)
c = e;
ret = mps_getnextreq(msg, e, o);
- if (ret == 1)
+ if (ret == 1) {
+ *root = c;
return (1);
+ }
if (ret == -1) {
ber_free_elements(e);
if (d == NULL)
@@ -319,6 +322,7 @@ mps_getbulkreq(struct snmp_message *msg,
if (d != NULL)
ber_link_elements(d, e);
d = e;
+ *end = d;
}
*root = c;
Index: snmpd.h
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
retrieving revision 1.59
diff -u -p -u -p -r1.59 snmpd.h
--- snmpd.h 16 Jan 2015 00:05:13 - 1.59
+++ snmpd.h 3 Jun 2015 09:02:38 -
@@ -396,6 +396,7 @@ struct snmp_message {
struct ber_element *sm_c;
struct ber_element *sm_next;
struct ber_element *sm_last;
+ struct ber_element *sm_end;
u_int8_t sm_data[READ_BUF_SIZE];
size_t sm_datalen;
@@ -638,7 +639,7 @@ int mps_getreq(struct snmp_message *,
int mps_getnextreq(struct snmp_message *, struct ber_element *,
struct ber_oid *);
int mps_getbulkreq(struct snmp_message *, struct ber_element **,
- struct ber_oid *, int);
+ struct ber_element **, struct ber_oid *, int);
int mps_setreq(struct snmp_message *, struct ber_element *,
struct ber_oid *);
int mps_set(struct ber_oid *, void *, long long);
Index: snmpe.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.40
diff -u -p -u -p -r1.40 snmpe.c
--- snmpe.c 16 Jan 2015 00:05:13 - 1.40
+++ snmpe.c 3 Jun 2015 09:06:31 -
@@ -374,6 +374,7 @@ snmpe_parsevarbinds(struct snmp_message
break;
case 1:
msg-sm_c = NULL;
+ msg-sm_end = NULL;
switch (msg-sm_context) {
@@ -409,7 +410,8 @@ snmpe_parsevarbinds(struct snmp_message
case SNMP_C_GETBULKREQ:
ret = mps_getbulkreq(msg, msg-sm_c,
- o, msg-sm_maxrepetitions);
+ msg-sm_end,
Re: cdce(4) and MBIM
Hi Ingo, sorry to disappoint you, but that won't work. First of all, if_cdce.c doesn't do NCM encoding and second, even if it did this still wouldn't work because it just describes how data packets are encapsulated. But to talk to an MBIM device, you need a different set of control messages (e.g. to set the PIN, to tell the device to connect to the network, etc.) I'm currently working on an MBIM driver for OpenBSD, but it will take some more time until it's ready for sending out patches. Gerhard On Tue, 20 Jan 2015 23:31:17 +0100 Ingo Feinerer feine...@logic.at wrote: Has anyone experiences with USB devices (e.g. UMTS modem) offering Mobile Broadband Interface Model (MBIM) interfaces? According to www.usb.org/developers/docs/devclass_docs/MBIM10Errata1_073013.zip MBIM is comparable to NCM (The largest deviation from NCM 1.0 is that devices transfer raw IP packets instead of packets with 802.3 headers.) which is supported by cdce(4). By hardcoding vendor and device ID in sys/dev/usb/if_cdce.c I managed to have a modem with MBIM interface attach via cdce: cdce0 at uhub0 port 4 configuration 1 interface 0 MediaTek Inc Product rev 2.00/3.00 addr 2 cdce0: address XX:XX:XX:XX:XX:XX ifconfig appears to work as well cdce0: flags=8802BROADCAST,SIMPLEX,MULTICAST mtu 1500 lladdr XX:XX:XX:XX:XX:XX priority: 0 but dhclient gets no response. /var/log/messages contains cdce0: watchdog timeout cdce0: usb error on tx: TIMEOUT Any ideas? Best regards, Ingo
Re: run(4) firmware update; please test
On Fri, 16 May 2014 13:07:24 +0200 Stefan Sperling s...@openbsd.org wrote: On Fri, May 16, 2014 at 02:44:10PM +0400, Dinar Talypov wrote: Hi, I have tested with D-Link DWA-140 rev B2G: run0 at uhub0 port 1 Ralink 11n Adapter rev 1.10/1.01 addr 2 run0: MAC/BBP RT3071 (rev 0x021C), RF RT3022 (MIMO 2T2R), address 14:d6:4d:49:73:4e with this diff nothing works. It doesn't find any access point. Are you able to help me debug this by providing more details? Are you really sure it's the firmware update that's causing the problem? Are you able to try your run(4) device with FreeBSD-current (10 isn't new enough)? They claim to support your device and use the updated firmware. He's using a model that requires a firmware-blob different from the one of all other testers (run-rt2870 vs. run-rt3071). That might explain, why he's the only one who got troubles with the new FW. On topic: new FW works fine on a SparkLAN WPER-150GN: run0 at uhub1 port 3 Ralink 802.11 n WLAN rev 2.00/1.01 addr 3 run0: MAC/BBP RT3070 (rev 0x0201), RF RT3020 (MIMO 1T1R), address 00:0e:8e:xx:xx:xx
pf dropping window updates and acks
In January bluhm@ introduced 'data_end' to pf.c:tcp_track_full().
Now this breaks the handling of non-data packets. They may be rejected
because the SEQ_GEQ(src-seqhi, data_end) check fails.
The patch below should fix this.
Gerhard
Index: sys/net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.841
diff -u -p -u -p -r1.841 pf.c
--- sys/net/pf.c9 Oct 2013 09:32:01 - 1.841
+++ sys/net/pf.c11 Oct 2013 09:57:20 -
@@ -3940,7 +3940,7 @@ pf_tcp_track_full(struct pf_pdesc *pd, s
if (seq == end) {
/* Ease sequencing restrictions on no data packets */
seq = src-seqlo;
- end = seq;
+ data_end = end = seq;
}
ackskew = dst-seqlo - ack;
Fw: SNMPv3 engine id discovery
Anybody willing to ok that patch?
Gerhard
Begin forwarded message:
Date: Fri, 16 Aug 2013 10:24:02 +0200
From: Gerhard Roth gr...@genua.de
To: tech@openbsd.org
Subject: SNMPv3 engine id discovery
Hi,
in SNMPv3 engine id discovery is done by sending a noAuthNoPriv request
to the SNMP agent. The agent should reply with a usmStatsUnknownEngineIDs
report containing the authoritative engine id.
In case snmpd was configured with a minimum seclevel higher than none,
a usmStatsUnsupportedSecLevels report was generated instead.
The fix below delays checking the required seclevel until after engine
id discovery has been handled.
Ok?
Gerhard
Index: usr.sbin/snmpd/snmpe.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 snmpe.c
--- usr.sbin/snmpd/snmpe.c 29 Mar 2013 12:53:41 - 1.33
+++ usr.sbin/snmpd/snmpe.c 16 Aug 2013 08:05:19 -
@@ -530,8 +530,7 @@ snmpe_parse(struct sockaddr_storage *ss,
goto parsefail;
msg-sm_flags = *flagstr;
- if (MSG_SECLEVEL(msg) env-sc_min_seclevel ||
- msg-sm_secmodel != SNMP_SEC_USM) {
+ if (msg-sm_secmodel != SNMP_SEC_USM) {
/* XXX currently only USM supported */
errstr = unsupported security model;
stats-snmp_usmbadseclevel++;
Index: usr.sbin/snmpd/usm.c
===
RCS file: /cvs/src/usr.sbin/snmpd/usm.c,v
retrieving revision 1.6
diff -u -p -u -p -r1.6 usm.c
--- usr.sbin/snmpd/usm.c24 Jan 2013 09:30:27 - 1.6
+++ usr.sbin/snmpd/usm.c16 Aug 2013 08:05:19 -
@@ -287,6 +287,13 @@ usm_decode(struct snmp_message *msg, str
msg-sm_engine_boots = (u_int32_t)engine_boots;
msg-sm_engine_time = (u_int32_t)engine_time;
+ if (MSG_SECLEVEL(msg) env-sc_min_seclevel) {
+ *errp = security level too low;
+ msg-sm_usmerr = OIDVAL_usmErrSecLevel;
+ stats-snmp_usmbadseclevel++;
+ goto done;
+ }
+
memcpy(msg-sm_username, user, userlen);
msg-sm_username[userlen] = '\0';
msg-sm_user = usm_finduser(msg-sm_username);
SNMPv3 engine id discovery
Hi,
in SNMPv3 engine id discovery is done by sending a noAuthNoPriv request
to the SNMP agent. The agent should reply with a usmStatsUnknownEngineIDs
report containing the authoritative engine id.
In case snmpd was configured with a minimum seclevel higher than none,
a usmStatsUnsupportedSecLevels report was generated instead.
The fix below delays checking the required seclevel until after engine
id discovery has been handled.
Ok?
Gerhard
Index: usr.sbin/snmpd/snmpe.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 snmpe.c
--- usr.sbin/snmpd/snmpe.c 29 Mar 2013 12:53:41 - 1.33
+++ usr.sbin/snmpd/snmpe.c 16 Aug 2013 08:05:19 -
@@ -530,8 +530,7 @@ snmpe_parse(struct sockaddr_storage *ss,
goto parsefail;
msg-sm_flags = *flagstr;
- if (MSG_SECLEVEL(msg) env-sc_min_seclevel ||
- msg-sm_secmodel != SNMP_SEC_USM) {
+ if (msg-sm_secmodel != SNMP_SEC_USM) {
/* XXX currently only USM supported */
errstr = unsupported security model;
stats-snmp_usmbadseclevel++;
Index: usr.sbin/snmpd/usm.c
===
RCS file: /cvs/src/usr.sbin/snmpd/usm.c,v
retrieving revision 1.6
diff -u -p -u -p -r1.6 usm.c
--- usr.sbin/snmpd/usm.c24 Jan 2013 09:30:27 - 1.6
+++ usr.sbin/snmpd/usm.c16 Aug 2013 08:05:19 -
@@ -287,6 +287,13 @@ usm_decode(struct snmp_message *msg, str
msg-sm_engine_boots = (u_int32_t)engine_boots;
msg-sm_engine_time = (u_int32_t)engine_time;
+ if (MSG_SECLEVEL(msg) env-sc_min_seclevel) {
+ *errp = security level too low;
+ msg-sm_usmerr = OIDVAL_usmErrSecLevel;
+ stats-snmp_usmbadseclevel++;
+ goto done;
+ }
+
memcpy(msg-sm_username, user, userlen);
msg-sm_username[userlen] = '\0';
msg-sm_user = usm_finduser(msg-sm_username);
dhcpd uses bad current time
In dhcpd, variable cur_time is set only once per dispatch loop.
Unfortunately, this is done before the poll(2) call. Since poll(2)
may sleep for an arbitrary amount of time, the value of cur_time
might refer to some long ago point in time. When message dispatching
is done, timeouts and lease ends are calculated based on this
outdated cur_time value that was set before the call to poll(2).
It's fine to set cur_time only once per loop, but we should do it after
the blocking poll(2) syscall so that its value is more accurate.
ok?
Gerhard
Index: usr.sbin/dhcpd/dispatch.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/dispatch.c,v
retrieving revision 1.30
diff -u -p -r1.30 dispatch.c
--- usr.sbin/dhcpd/dispatch.c 19 Apr 2013 21:25:39 - 1.30
+++ usr.sbin/dhcpd/dispatch.c 15 May 2013 08:09:17 -
@@ -306,8 +306,6 @@ dispatch(void)
* still a timeout registered, time out the poll
* call then.
*/
- time(cur_time);
-another:
if (timeouts) {
if (timeouts-when = cur_time) {
struct dhcpd_timeout *t = timeouts;
@@ -315,7 +313,7 @@ another:
(*(t-func))(t-what);
t-next = free_timeouts;
free_timeouts = t;
- goto another;
+ continue;
}
/*
@@ -360,6 +358,7 @@ another:
case 0:
continue; /* no packets */
}
+ time(cur_time);
for (i = 0, l = protocols; l; l = l-next) {
struct interface_info *ip = l-local;
Re: dhcpd uses bad current time
On Wed, 15 May 2013 10:15:54 +0200
Gerhard Roth gr...@genua.de wrote:
In dhcpd, variable cur_time is set only once per dispatch loop.
Unfortunately, this is done before the poll(2) call. Since poll(2)
may sleep for an arbitrary amount of time, the value of cur_time
might refer to some long ago point in time. When message dispatching
is done, timeouts and lease ends are calculated based on this
outdated cur_time value that was set before the call to poll(2).
It's fine to set cur_time only once per loop, but we should do it after
the blocking poll(2) syscall so that its value is more accurate.
ok?
Gerhard
Sorry, the previous version will break timeout handling when there
are no incoming packets. I notice the moment that I sent the mail.
So let's keep all the old code and just add another update of cur_time
after poll(2).
Gerhard
Index: usr.sbin/dhcpd/dispatch.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/dispatch.c,v
retrieving revision 1.30
diff -u -p -r1.30 dispatch.c
--- usr.sbin/dhcpd/dispatch.c 19 Apr 2013 21:25:39 - 1.30
+++ usr.sbin/dhcpd/dispatch.c 15 May 2013 08:26:49 -
@@ -360,6 +360,7 @@ another:
case 0:
continue; /* no packets */
}
+ time(cur_time);
for (i = 0, l = protocols; l; l = l-next) {
struct interface_info *ip = l-local;
Re: dhcpd uses bad current time
On Wed, 15 May 2013 10:24:22 +0200 Otto Moerbeek o...@drijf.net wrote: On Wed, May 15, 2013 at 10:15:54AM +0200, Gerhard Roth wrote: In dhcpd, variable cur_time is set only once per dispatch loop. Unfortunately, this is done before the poll(2) call. Since poll(2) may sleep for an arbitrary amount of time, the value of cur_time might refer to some long ago point in time. When message dispatching is done, timeouts and lease ends are calculated based on this outdated cur_time value that was set before the call to poll(2). It's fine to set cur_time only once per loop, but we should do it after the blocking poll(2) syscall so that its value is more accurate. ok? Gerhard I don't think the first call to time() should be left out. cur_time is potentially used in the first iteration of the loop. Well, it is set in main(), so it is initialized. But nevertheless you're that we should keep it because it is required for correct timeout handling if there are no incoming packets. Just sent out a second diff that keeps the time(2) call at the top of the loop. Gerhard
Re: check for device_lookup result in vscsi
Mike,
but it does check in vscsiopen(). Hence no userland program should be
able to call vscsiioctl() for a non-existant device because the open()
already failed. At least that's true as long as vscsi devices can't
disappear during run-time.
Gerhard
On Fri, 10 May 2013 14:44:39 +0200 Mike Belopuhov m...@belopuhov.com wrote:
On Fri, May 03, 2013 at 16:19 +0200, Mike Belopuhov wrote:
hi,
while looking for the device_unref bugs, i found that
vscsi doesn't check if device_lookup has returned a
valid return value.
ok?
anyone?
diff --git sys/dev/vscsi.c sys/dev/vscsi.c
index 3da371c..db65642 100644
--- sys/dev/vscsi.c
+++ sys/dev/vscsi.c
@@ -296,6 +296,9 @@ vscsiioctl(dev_t dev, u_long cmd, caddr_t addr, int
flags, struct proc *p)
int read = 0;
int err = 0;
+ if (sc == NULL)
+ return (ENXIO);
+
rw_enter_write(sc-sc_ioc_lock);
switch (cmd) {
@@ -476,6 +479,9 @@ vscsipoll(dev_t dev, int events, struct proc *p)
struct vscsi_softc *sc = DEV2SC(dev);
int revents = 0;
+ if (sc == NULL)
+ return (ENXIO);
+
if (events (POLLIN | POLLRDNORM)) {
mtx_enter(sc-sc_state_mtx);
if (!TAILQ_EMPTY(sc-sc_ccb_i2t))
@@ -494,9 +500,14 @@ vscsipoll(dev_t dev, int events, struct proc *p)
int
vscsikqfilter(dev_t dev, struct knote *kn)
-{
+{
struct vscsi_softc *sc = DEV2SC(dev);
- struct klist *klist = sc-sc_sel.si_note;
+ struct klist *klist;
+
+ if (sc == NULL)
+ return (ENXIO);
+
+ klist = sc-sc_sel.si_note;
switch (kn-kn_filter) {
case EVFILT_READ:
Re: Add Soekris comBIOS detection to bios(4) on i386/amd64
On Mon, 4 Mar 2013 06:02:19 -0500 Matt Dainty m...@bodgit-n-scarper.com wrote:
* Matt Dainty m...@bodgit-n-scarper.com [2013-02-20 19:30:43]:
Attached are two patches for bios(4) on i386 amd64 that add support
for detecting the comBIOS on Soekris hardware, which then fills in the
hw.vendor hw.product sysctl variables as this hardware lacks any
SMBIOS to provide them. The idea is then these can be used in the
GPIO/LED driver for the net6501 I posted a while ago to simplify the
match logic.
I've tested this with a net6501 amd64 so I would appreciate testing
on any other Soekris hardware under i386; net4501, net4801 net5501.
I'm lacking a test report for a net4801, however as I've had reports
of success for all other Soekris board types, I'd be surprised if it
didn't work.
Hi,
I can confirm that your patch works for i386 on net4501:
# sysctl hw
hw.machine=i386
hw.model=Geode(TM) Integrated Processor by National Semi (Geode by NSC
586-class)
hw.ncpu=1
hw.byteorder=1234
hw.pagesize=4096
...
hw.cpuspeed=267
hw.vendor=Soekris Engineering
hw.product=net4801
hw.version=1
hw.physmem=133754880
hw.usermem=133742592
hw.ncpufound=1
hw.allowpowerdown=1
Gerhard
Also any other hardware that lacks SMBIOS and in the case of i386
doesn't need to disable probing of SMBIOS with flags 0x0008.
I would still appreciate some testing on other systems that lack any
SMBIOS, just to make sure it doesn't cause problems there. However as
this code walks over the same area of memory, I would again be
surprised if this caused problems when the SMBIOS probe didn't.
Matt
--- sys/arch/amd64/amd64/bios.c.origTue Feb 19 01:56:56 2013
+++ sys/arch/amd64/amd64/bios.c Wed Feb 20 08:37:12 2013
@@ -95,6 +95,7 @@
vaddr_t va;
paddr_t pa, end;
u_int8_t *p;
+ int smbiosrev = 0;
/* see if we have SMBIOS extentions */
for (p = ISA_HOLE_VADDR(SMBIOS_START);
@@ -137,6 +138,10 @@
printf(: SMBIOS rev. %d.%d @ 0x%lx (%d entries),
hdr-majrev, hdr-minrev, hdr-addr, hdr-count);
+ smbiosrev = hdr-majrev * 100 + hdr-minrev;
+ if (hdr-minrev 10)
+ smbiosrev = hdr-majrev * 100 + hdr-minrev * 10;
+
bios.cookie = 0;
if (smbios_find_table(SMBIOS_TYPE_BIOS, bios)) {
sb = bios.tblhdr;
@@ -158,6 +163,39 @@
break;
}
printf(\n);
+
+ /* No SMBIOS extensions, go looking for Soekris comBIOS */
+ if (!smbiosrev) {
+ const char *signature = Soekris Engineering;
+
+ for (p = ISA_HOLE_VADDR(SMBIOS_START);
+ p = (u_int8_t *)ISA_HOLE_VADDR(SMBIOS_END -
+ (strlen(signature) - 1)); p++)
+ if (!memcmp(p, signature, strlen(signature))) {
+ hw_vendor = malloc(strlen(signature) + 1,
+ M_DEVBUF, M_NOWAIT);
+ if (hw_vendor)
+ strlcpy(hw_vendor, signature,
+ strlen(signature) + 1);
+ p += strlen(signature);
+ break;
+ }
+
+ for (; hw_vendor
+ p = (u_int8_t *)ISA_HOLE_VADDR(SMBIOS_END - 6); p++)
+ /*
+* Search only for net6501 in the comBIOS as that's
+* the only Soekris platform that can run amd64
+*/
+ if (!memcmp(p, net6501, 7)) {
+ hw_prod = malloc(8, M_DEVBUF, M_NOWAIT);
+ if (hw_prod) {
+ memcpy(hw_prod, p, 7);
+ hw_prod[7] = '\0';
+ }
+ break;
+ }
+ }
#if NACPI 0
{
--- sys/arch/i386/i386/bios.c.orig Tue Feb 19 06:36:42 2013
+++ sys/arch/i386/i386/bios.c Wed Feb 20 08:58:17 2013
@@ -330,6 +330,43 @@
printf(\n);
+ /* No SMBIOS extensions, go looking for Soekris comBIOS */
+ if (!(flags BIOSF_SMBIOS) !smbiosrev) {
+ const char *signature = Soekris Engineering;
+
+ for (va = ISA_HOLE_VADDR(SMBIOS_START);
+ va = (u_int8_t *)ISA_HOLE_VADDR(SMBIOS_END -
+ (strlen(signature) - 1)); va++)
+ if (!memcmp((u_int8_t *)va, signature,
+ strlen(signature))) {
+ hw_vendor = malloc(strlen(signature) + 1,
+ M_DEVBUF, M_NOWAIT);
+ if (hw_vendor)
+ strlcpy(hw_vendor, signature,
+ strlen(signature) + 1);
+ va += strlen(signature);
+
Re: Add Soekris comBIOS detection to bios(4) on i386/amd64
On Mon, 4 Mar 2013 15:18:45 +0100 Gerhard Roth gerhard_r...@genua.de wrote:
On Mon, 4 Mar 2013 06:02:19 -0500 Matt Dainty m...@bodgit-n-scarper.com
wrote:
* Matt Dainty m...@bodgit-n-scarper.com [2013-02-20 19:30:43]:
Attached are two patches for bios(4) on i386 amd64 that add support
for detecting the comBIOS on Soekris hardware, which then fills in the
hw.vendor hw.product sysctl variables as this hardware lacks any
SMBIOS to provide them. The idea is then these can be used in the
GPIO/LED driver for the net6501 I posted a while ago to simplify the
match logic.
I've tested this with a net6501 amd64 so I would appreciate testing
on any other Soekris hardware under i386; net4501, net4801 net5501.
I'm lacking a test report for a net4801, however as I've had reports
of success for all other Soekris board types, I'd be surprised if it
didn't work.
Hi,
I can confirm that your patch works for i386 on net4501:
# sysctl hw
hw.machine=i386
hw.model=Geode(TM) Integrated Processor by National Semi (Geode by NSC
586-class)
hw.ncpu=1
hw.byteorder=1234
hw.pagesize=4096
...
hw.cpuspeed=267
hw.vendor=Soekris Engineering
hw.product=net4801
hw.version=1
hw.physmem=133754880
hw.usermem=133742592
hw.ncpufound=1
hw.allowpowerdown=1
Gerhard
Sorry,
stupid me mixed up 4801 and 4501 :(
But as you see from the sysctl output, I tested on 4801. So this should
fill up your gap.
Gerhard
Also any other hardware that lacks SMBIOS and in the case of i386
doesn't need to disable probing of SMBIOS with flags 0x0008.
I would still appreciate some testing on other systems that lack any
SMBIOS, just to make sure it doesn't cause problems there. However as
this code walks over the same area of memory, I would again be
surprised if this caused problems when the SMBIOS probe didn't.
Matt
--- sys/arch/amd64/amd64/bios.c.orig Tue Feb 19 01:56:56 2013
+++ sys/arch/amd64/amd64/bios.c Wed Feb 20 08:37:12 2013
@@ -95,6 +95,7 @@
vaddr_t va;
paddr_t pa, end;
u_int8_t *p;
+ int smbiosrev = 0;
/* see if we have SMBIOS extentions */
for (p = ISA_HOLE_VADDR(SMBIOS_START);
@@ -137,6 +138,10 @@
printf(: SMBIOS rev. %d.%d @ 0x%lx (%d entries),
hdr-majrev, hdr-minrev, hdr-addr, hdr-count);
+ smbiosrev = hdr-majrev * 100 + hdr-minrev;
+ if (hdr-minrev 10)
+ smbiosrev = hdr-majrev * 100 + hdr-minrev * 10;
+
bios.cookie = 0;
if (smbios_find_table(SMBIOS_TYPE_BIOS, bios)) {
sb = bios.tblhdr;
@@ -158,6 +163,39 @@
break;
}
printf(\n);
+
+ /* No SMBIOS extensions, go looking for Soekris comBIOS */
+ if (!smbiosrev) {
+ const char *signature = Soekris Engineering;
+
+ for (p = ISA_HOLE_VADDR(SMBIOS_START);
+ p = (u_int8_t *)ISA_HOLE_VADDR(SMBIOS_END -
+ (strlen(signature) - 1)); p++)
+ if (!memcmp(p, signature, strlen(signature))) {
+ hw_vendor = malloc(strlen(signature) + 1,
+ M_DEVBUF, M_NOWAIT);
+ if (hw_vendor)
+ strlcpy(hw_vendor, signature,
+ strlen(signature) + 1);
+ p += strlen(signature);
+ break;
+ }
+
+ for (; hw_vendor
+ p = (u_int8_t *)ISA_HOLE_VADDR(SMBIOS_END - 6); p++)
+ /*
+ * Search only for net6501 in the comBIOS as that's
+ * the only Soekris platform that can run amd64
+ */
+ if (!memcmp(p, net6501, 7)) {
+ hw_prod = malloc(8, M_DEVBUF, M_NOWAIT);
+ if (hw_prod) {
+ memcpy(hw_prod, p, 7);
+ hw_prod[7] = '\0';
+ }
+ break;
+ }
+ }
#if NACPI 0
{
--- sys/arch/i386/i386/bios.c.origTue Feb 19 06:36:42 2013
+++ sys/arch/i386/i386/bios.c Wed Feb 20 08:58:17 2013
@@ -330,6 +330,43 @@
printf(\n);
+ /* No SMBIOS extensions, go looking for Soekris comBIOS */
+ if (!(flags BIOSF_SMBIOS) !smbiosrev) {
+ const char *signature = Soekris Engineering;
+
+ for (va = ISA_HOLE_VADDR(SMBIOS_START);
+ va = (u_int8_t *)ISA_HOLE_VADDR(SMBIOS_END -
+ (strlen(signature) - 1)); va++)
+ if (!memcmp((u_int8_t *)va, signature,
+ strlen(signature))) {
+ hw_vendor = malloc(strlen(signature) + 1,
+ M_DEVBUF, M_NOWAIT
Re: [PATCH] snmpd readonly mode
On Mon, 4 Mar 2013 15:56:36 +0100 Ilya Bakulin ilya_baku...@genua.de wrote: Hi list, We have a small issue with snmpd daemon in OpenBSD. If people use SNMPv2c, they should explicitly set read-write community name to some [probably random-generated] string, because otherwise everybody is able to alter values of some SNMP nodes (the default value for read-write community is private, which is not very secure, probably). Attached is the patch that adds new configuration file parameter, nowrite, with values yes and no, that disallows any write attempts to any SNMP node regardless of specified read-write community string. $ snmpset -c private -v2c 127.0.0.1 system.sysContact.0 s SOME_CRAP Error in packet. Reason: (readOnly) The two parties used do not have access to use the specified SNMP PDU. Failed object: SNMPv2-MIB::sysContact.0 Hope you will find it useful. // Ilya Just a little bike-shedding: - The term readonly seems more common to me than nowrite. - All members of struct snmpd have a 'sc_' prefix. You should stick to that style. Gerhard
Re: [PATCH] snmpd readonly mode
On Mon, 4 Mar 2013 17:28:17 +0100 Ilya Bakulin ilya_baku...@genua.de wrote: On Monday 04 March 2013 16:13:09 Gerhard Roth wrote: Just a little bike-shedding: - The term readonly seems more common to me than nowrite. I didn't use readonly because there is already a keyword read-only with the token name READONLY, and I didn't want to introduce yet another readonly* variable :-) Hi Ilya, how about using the existing 'read-write' keyword? read-write [no | community string] Gerhard - All members of struct snmpd have a 'sc_' prefix. You should stick to that style. Gerhard Agree, not using sc_ is bad. // Ilya
Re: cloneable tun
On 11/29/2012 01:33 PM, Reyk Floeter wrote:
On Thu, Nov 29, 2012 at 10:59 AM, Mike Belopuhov m...@belopuhov.com wrote:
But currently /dev/tunN is usable from any programming language that
that can do reads and writes. With Reyk's changes you need to do an
ioctl even for basic usage, which is at best quirky in languages other
than C/C++. That feels like a step backward to me.
sure, we can totally leave tun for legacy use in the shell scripts.
so i guess reyk should go ahead and implement a dynamic tun interface
(dun?) with whatever semantics we need and want.
Or even better duh? ;-) I wrote this diff because I wanted to
experiment with clonable device nodes, I still don't like the fact
that you have to MAKEDEV a device per dynamic interface, and because
it adds some extra flexibility. But it doesn't have to go anywhere, I
didn't even waste much time with writing it yesterday in the
afternoon.
btw., I like C and it is still my favorite language (sorry, CS
people). But it shouldn't be a problem to do simple ioctls with most
other languages except shell scripts.
#!/usr/bin/perl
require sys/ioctl.ph;
$TUNSIFUNIT = _IOC(IOC_INOUT, ord('t'), 90, 4);
open(TUN0, +/dev/tun0) or die open;
ioctl(TUN0, $TUNSIFUNIT, $unit = pack(i, -1)) or die ioctl $!;
print Returned: tun.unpack(i, $unit).\n;
close(TUN0);
reyk
I like cloning devices. But instead of needing an ioclt(2), wouldn't
it be simpler to adapt the SVR4 API where the first parameter of
cdevsw open() functions is an 'dev_t *dev'. Thus the open() routine
can automatically do the cloning (or maybe not if minor is 0) by
returning the device-Id of the cloned instance. and there is no need
for using additional ioctl(2)s.
Of course it requires a one-time API change with large impact.
Gerhard
Re: [PATCH] mlockall() problem in OpenBSD 5.2
On Thu, 08 Nov 2012 16:22:41 -0500 Ted Unangst t...@tedunangst.com wrote: On Thu, Nov 08, 2012 at 13:34, Ilya Bakulin wrote: The problem seems to be in uvm_map_pageable_all() function (sys/uvm/uvm_map.c). This function is a special case of uvm_map_pageable, which tries to mlockall() all mapped memory regions. Prior to calling uvm_map_pageable_wire(), which actually does locking, it tries to count how many memory bytes will be locked, and compares this number with uvmexp.wiredmax, which is set by RLIMIT_MEMLOCK. The problem is that counting algorithm doesn't take into account that some pages have VM_PROT_NONE flag set and hence won't be locked anyway. Later in uvm_map_pageable_wire() these pages are skipped when doing actual job. I don't know if this is right. Should prot_none pages not be wired? I think the opposite should happen. prot_none pages should be locked as well. The app may be using prot_none as a way to protect its super secret secrets from itself. It certainly wouldn't want them being swapped out. As long as they have VM_PROT_NONE, they can't be accessed and wiring them is just a waste of resources. If your scenario applies then uvm_map_protect() kicks in. It takes care of wiring pages if the protection changes from VM_PROT_NONE to some different value, though I have to admit that this happens only in case the VM_MAP_WIREFUTURE flag was specified. But that looks acceptable to me. Gerhard
Re: [PATCH] mlockall() problem in OpenBSD 5.2
I did a similar change recently (http://marc.info/?l=openbsd-cvsm=135055003602935w=2). Therefore I think that Ilya's patch is valid and should be applied. If anyone is willing to ok, I can commit it. Gerhard On 11/08/2012 01:34 PM, Ilya Bakulin wrote: Hi list, after upgrade on OpenBSD 5.2 we observe the following message from ntpd: Oct 22 17:20:13 gg74 ntpd[2918]: ntpd 4.2.6p2@1.2194-o Tue Oct 16 20:26:47 UTC 2012 (1) Oct 22 17:20:13 gg74 ntpd[10103]: mlockall(): Cannot allocate memory Oct 22 17:20:13 gg74 ntpd[10103]: signal_no_reset: signal 13 had flags 12 Oct 22 17:20:13 gg74 ntpd[10103]: proto: precision = 6.390 usec ... This doesn't prevent ntpd from starting, however. We tried to debug this problem. This occurs when an application tries to set resource limits (more precisely, RLIMIT_MEMLOCK) to some relatively low value and then does mlockall(). The problem seems to be in uvm_map_pageable_all() function (sys/uvm/uvm_map.c). This function is a special case of uvm_map_pageable, which tries to mlockall() all mapped memory regions. Prior to calling uvm_map_pageable_wire(), which actually does locking, it tries to count how many memory bytes will be locked, and compares this number with uvmexp.wiredmax, which is set by RLIMIT_MEMLOCK. The problem is that counting algorithm doesn't take into account that some pages have VM_PROT_NONE flag set and hence won't be locked anyway. Later in uvm_map_pageable_wire() these pages are skipped when doing actual job. Attached patch fixes the problem on OpenBSD 5.2. I'm also attaching a simple test application, that can be used to reproduce the bug. Just compile and run as root (otherwise mlockall() doesn't work anyway). On OpenBSD 5.2 the RLIMIT_MEMLOCK limit will be significantly higher than on OpenBSD 5.1. After applying my patch they will be almost the same. Thank you for your attention. // Ilya
Re: [PATCH] mlockall() problem in OpenBSD 5.2
On 11/08/2012 02:08 PM, Stuart Henderson wrote: Oh talking of RLIMIT reminds me...can someone who knows this area take a look at http://thread.gmane.org/gmane.os.aeriebsd.general/100 please? To me the fix looks reasonable. Limiting the stack size below the current usage shouldn't be allowed. And returning EINVAL is the right error code according to SUS (The limit specified cannot be lowered because current usage is already higher than the limit.). However, the EINVAL error should then be added to the man page (lib/libc/sys/getrlimit.2). That's missing in Mickey's patch. Gerhard
Re: DNS options for sppp(4)
On 10/25/2012 10:44 AM, LEVAI Daniel wrote: On p, okt 05, 2012 at 13:46:19 +0200, Gerhard Roth wrote: Hi, I trying to revive part of an old patch that was submitted by mpf in 2007. If adds support for DNS server negotiation to the IPCP part of sppp(4). If the PPP server provides IP addresses for DNS servers, they will appear in the ifconfig(8) output. A simple ifstated(8) script could then be used to re-write resolv.conf, e.g. # ifconfig pppoe | sed -n 's/^[[:space:]]*dns.:/nameserver/p' [...] Works for me. Cool feature, I like it :) Thanks! Daniel So anybody out there willing to give me an ok?
DNS options for sppp(4)
Hi,
I trying to revive part of an old patch that was submitted by mpf
in 2007. If adds support for DNS server negotiation to the IPCP
part of sppp(4). If the PPP server provides IP addresses for
DNS servers, they will appear in the ifconfig(8) output.
A simple ifstated(8) script could then be used to re-write resolv.conf,
e.g.
# ifconfig pppoe | sed -n 's/^[[:space:]]*dns.:/nameserver/p'
Gerhard
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.257
diff -u -p -r1.257 ifconfig.c
--- sbin/ifconfig/ifconfig.c6 Sep 2012 19:41:59 - 1.257
+++ sbin/ifconfig/ifconfig.c5 Oct 2012 11:19:36 -
@@ -4265,6 +4265,12 @@ sppp_status(void)
if (spa.flags AUTHFLAG_NORECHALLENGE)
printf(norechallenge );
putchar('\n');
+ if (spr.defs.dns_addrs[0] != INADDR_ANY)
+ printf(\tdns1: %s\n,
+ inet_ntoa(*(struct in_addr *)spr.defs.dns_addrs[0]));
+ if (spr.defs.dns_addrs[1] != INADDR_ANY)
+ printf(\tdns2: %s\n,
+ inet_ntoa(*(struct in_addr *)spr.defs.dns_addrs[1]));
}
void
Index: sys/net/if_sppp.h
===
RCS file: /cvs/src/sys/net/if_sppp.h,v
retrieving revision 1.17
diff -u -p -r1.17 if_sppp.h
--- sys/net/if_sppp.h 29 Jan 2012 10:21:54 - 1.17
+++ sys/net/if_sppp.h 5 Oct 2012 11:19:45 -
@@ -118,6 +118,7 @@ struct sppp {
time_t pp_last_receive;/* peer's last sign of life */
time_t pp_last_activity; /* second of last payload data s/r */
enum ppp_phase pp_phase;/* phase we're currently in */
+ u_int32_t dns_addrs[2];/* primary and secondary DNS server */
int state[IDX_COUNT]; /* state machine */
u_char confid[IDX_COUNT]; /* id of last configuration request */
int rst_counter[IDX_COUNT]; /* restart counter */
Index: sys/net/if_spppsubr.c
===
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.98
diff -u -p -r1.98 if_spppsubr.c
--- sys/net/if_spppsubr.c 24 Jul 2012 15:16:20 - 1.98
+++ sys/net/if_spppsubr.c 5 Oct 2012 11:19:45 -
@@ -167,6 +167,8 @@
#define IPCP_OPT_ADDRESSES 1 /* both IP addresses; deprecated */
#define IPCP_OPT_COMPRESSION 2 /* IP compression protocol (VJ) */
#define IPCP_OPT_ADDRESS 3 /* local IP address */
+#define IPCP_OPT_PRIMDNS 129 /* primary remote dns address */
+#define IPCP_OPT_SECDNS131 /* secondary remote dns address
*/
#define IPV6CP_OPT_IFID1 /* interface identifier */
#define IPV6CP_OPT_COMPRESSION 2 /* IPv6 compression protocol */
@@ -2687,6 +2689,7 @@ sppp_ipcp_init(struct sppp *sp)
sp-ipcp.flags = 0;
sp-state[IDX_IPCP] = STATE_INITIAL;
sp-fail_counter[IDX_IPCP] = 0;
+ bzero(sp-dns_addrs, sizeof(sp-dns_addrs));
#if defined (__FreeBSD__)
callout_handle_init(sp-ch[IDX_IPCP]);
#endif
@@ -2954,6 +2957,9 @@ sppp_ipcp_RCN_nak(struct sppp *sp, struc
struct ifnet *ifp = sp-pp_if;
int debug = ifp-if_flags IFF_DEBUG;
u_int32_t wantaddr;
+ u_int32_t dns;
+ int dnsix;
+ int dns_changed = 0;
len -= 4;
@@ -2995,6 +3001,17 @@ sppp_ipcp_RCN_nak(struct sppp *sp, struc
}
}
break;
+ case IPCP_OPT_PRIMDNS:
+ case IPCP_OPT_SECDNS:
+ if (len = 6 p[1] == 6) {
+ dnsix = (*p == IPCP_OPT_PRIMDNS) ? 0 : 1;
+ bcopy(p + 2, dns, sizeof dns);
+ if (sp-dns_addrs[dnsix] != dns) {
+ sp-dns_addrs[dnsix] = dns;
+ dns_changed = 1;
+ }
+ }
+ break;
#ifdef notyet
case IPCP_OPT_COMPRESS:
/*
@@ -3004,6 +3021,8 @@ sppp_ipcp_RCN_nak(struct sppp *sp, struc
#endif
}
}
+ if ((ifp-if_flags IFF_UP) dns_changed)
+ rt_ifmsg(ifp);
if (debug)
addlog(\n);
}
@@ -3109,12 +3128,13 @@ sppp_ipcp_tlf(struct sppp *sp)
/* we no longer need LCP */
sp-lcp.protos = ~(1 IDX_IPCP);
sppp_lcp_check_and_close(sp);
+ bzero(sp-dns_addrs, sizeof(sp-dns_addrs));
}
HIDE void
sppp_ipcp_scr(struct sppp *sp)
{
- char opt[6 /* compression */ + 6 /* address */];
+ char opt[6 /* compression */ + 6 /* address */ + 12 /* dns addresses
*/];
u_int32_t ouraddr;
int i = 0;
@@ -3143,6 +3163,16 @@ sppp_ipcp_scr(struct
Problem in vr_stop()
Hi,
we observed mysterious panics while stopping vr interfaces. This was due
to vr_stop() trying to stop the transfers but then not waiting for them
to really finish but rather remove their DMA buffer mappings immediately.
The patch below uses a loop that was copied from vr_setcfg() (!IFM_ACTIVE)
to wait for transfers to finish.
Gerhard
Index: sys/dev/pci/if_vr.c
===
RCS file: /cvs/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.114
diff -u -p -r1.114 if_vr.c
--- sys/dev/pci/if_vr.c 30 Jan 2012 09:11:30 - 1.114
+++ sys/dev/pci/if_vr.c 18 Sep 2012 10:48:24 -
@@ -1542,6 +1542,17 @@ vr_stop(struct vr_softc *sc)
VR_SETBIT16(sc, VR_COMMAND, VR_CMD_STOP);
VR_CLRBIT16(sc, VR_COMMAND, (VR_CMD_RX_ON|VR_CMD_TX_ON));
+
+ /* wait for xfers to shutdown */
+ for (i = VR_TIMEOUT; i 0; i--) {
+ DELAY(10);
+ if (!(CSR_READ_2(sc, VR_COMMAND) (VR_CMD_TX_ON|VR_CMD_RX_ON)))
+ break;
+ }
+#ifdef VR_DEBUG
+ if (i == 0)
+ printf(%s: rx shutdown error!\n, sc-sc_dev.dv_xname);
+#endif
CSR_WRITE_2(sc, VR_IMR, 0x);
CSR_WRITE_4(sc, VR_TXADDR, 0x);
CSR_WRITE_4(sc, VR_RXADDR, 0x);
Bug in calibration of local APIC timer
Hi,
I spotted a problem with the calibration of the local APIC clock. For
the second CPU you sometimes see impossibly high clock rates like
in the following example:
cpu0: Intel(R) Core(TM) i3-2100 CPU @ 3.10GHz (GenuineIntel 686-class) 3.10
GHz
cpu0:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,PCLMUL,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,POPCNT,XSAVE,AVX
real mem = 2124333056 (2025MB)
avail mem = 2033119232 (1938MB)
mainbus0 at root
bios0 at mainbus0: AT/286+ BIOS, date 10/20/10, SMBIOS rev. 2.7 @ 0xeadc0
(105 entries)
bios0: vendor American Megatrends Inc. version 1.1a date 09/28/2011
bios0: Supermicro GS0300
acpi at bios0 function 0x0 not configured
pcibios at bios0 function 0x1a not configured
bios0: ROM list: 0xc/0x8000
mainbus0: Intel MP Specification (Version 1.4)
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: apic clock running at 249MHz
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Core(TM) i3-2100 CPU @ 3.10GHz (GenuineIntel 686-class) 7.73
GHz
The primary cause for this is the wrong speed detected for the APIC
clock. Above it claims to be 249 MHz, when in fact it is 100 MHz.
The main trouble with that error is that now all delay(9) calls are
wrong by the same factor.
In order to calibrate the local APIC timer the i8254 timer is used for
reference. This timer is initialized with TIMER_FREQ / hz and then
counts down to one within 1/hz seconds after which it starts another
cycle. So this kinda looks like a saw-tooth.
The code picks an arbitrary value of the counter and stores it in
'starttick'. Then it loops hz times (which should take a total of one
second). The first inner do-while loop waits for the current counter
to become larger than 'starttick', i.e. for a new timer cycle to
start. The second do-while loop waits for the counter to be decreased
to the same value as 'starttick'. Both do-while loops together are
expected to take 1/hz seconds. To visualize this, imagine a time-
diagram with the saw-tooth counter value and a horizontal line at the
height of 'starttick'.
Now if the value of 'starttick' by chance is very large, the code might
miss the small time window where the counter in fact is larger than it.
The chance of a miss is even larger due to the call to i8254_delay()
where we're blind to the counter values. In the extreme case where
starttick is set to the counters initial value (TIMER_FREQ / hz), this
would be an endless loop cause the counter will never carry a value
larger than this. Same problem happens if the value of 'starttick' is
rather small.
In both cases (very large or very small 'starttick') some cycles of the
i8254 counter may be missed and we have to run longer until hz cycles
are detected. Once we leave the for-loop which is expected to run for
one second, the local APIC timer will have made a much larger progress
than expected and the code assumes that it's running faster than fact
it is.
The patch below fixes the problem:
- Neither call i8254_delay() nor lapic_gettick() inside the calibration
loop. Only keep an eye on the i8254 counter. We don't need these calls
and permanently reading the counter reduces the possibility of missing
cycles.
- Because comparison against an arbitrary start value is problematic,
we first wait for the current cycle to finish. Then inside the
calibration loop, we just wait until hz complete cycles have passed.
Gerhard
Index: sys/arch/amd64/amd64/lapic.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
retrieving revision 1.27
diff -u -p -r1.27 lapic.c
--- sys/arch/amd64/amd64/lapic.c20 Sep 2010 06:33:46 - 1.27
+++ sys/arch/amd64/amd64/lapic.c17 Sep 2012 17:55:48 -
@@ -305,6 +305,20 @@ lapic_initclocks(void)
extern int gettick(void); /* XXX put in header file */
extern u_long rtclock_tval; /* XXX put in header file */
+static __inline void
+wait_next_cycle(void)
+{
+ unsigned int tick, tlast;
+
+ tlast = (1 16); /* i8254 counter has 16 bits at most */
+ for (;;) {
+ tick = gettick();
+ if (tick tlast)
+ return;
+ tlast = tick;
+ }
+}
+
/*
* Calibrate the local apic count-down timer (which is running at
* bus-clock speed) vs. the i8254 counter/timer (which is running at
@@ -319,8 +333,7 @@ extern u_long rtclock_tval; /* XXX put i
void
lapic_calibrate_timer(struct cpu_info *ci)
{
- unsigned int starttick, tick1, tick2, endtick;
- unsigned int startapic, apic1, apic2, endapic;
+ unsigned int startapic, endapic;
u_int64_t dtick, dapic, tmp;
long rf = read_rflags();
int i;
@@ -337,27 +350,20 @@ lapic_calibrate_timer(struct cpu_info *c
i82489_writereg(LAPIC_ICR_TIMER, 0x8000);
disable_intr();
-
Re: SNMPv3 Support
Hi Reyk,
On Wed, 18 Jul 2012 21:42:34 +0200, Reyk Floeter r...@openbsd.org wrote:
How about:
noAuthNoPriv- none
authNoPriv - auth
authPriv- encr
Is there a better alternative for encr? Maybe just enc (I know it
would complicate the grammar because it's a reserved keyword) or
something more abstract like high or all?
the yacc grammar was exactly why I chose encr instead of enc. ;)
I changed it to enc now. But now none, auth and enc are
reserved words. Hope nobody uses them as community string or
elsewhere.
Also noticed that you wanted me to keep the auth keyword in front
of hmac-*. Fixed this one, too.
Find the updated patch below.
Gerhard
Index: usr.sbin/snmpd/Makefile
===
RCS file: /cvs/src/usr.sbin/snmpd/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- usr.sbin/snmpd/Makefile 20 Mar 2012 03:01:26 - 1.8
+++ usr.sbin/snmpd/Makefile 18 Jul 2012 14:03:06 -
@@ -4,9 +4,9 @@ PROG= snmpd
MAN= snmpd.8 snmpd.conf.5
SRCS= parse.y ber.c log.c control.c snmpe.c \
mps.c trap.c mib.c smi.c kroute.c snmpd.c timer.c \
- pf.c
+ pf.c usm.c
-LDADD= -levent -lutil -lkvm
+LDADD= -levent -lutil -lkvm -lcrypto
DPADD= ${LIBEVENT} ${LIBUTIL}
CFLAGS+= -Wall -I${.CURDIR}
CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
Index: usr.sbin/snmpd/ber.3
===
RCS file: /cvs/src/usr.sbin/snmpd/ber.3,v
retrieving revision 1.9
diff -u -p -r1.9 ber.3
--- usr.sbin/snmpd/ber.325 Feb 2010 09:59:55 - 1.9
+++ usr.sbin/snmpd/ber.318 Jul 2012 14:03:06 -
@@ -50,9 +50,11 @@
.Nm ber_write_elements ,
.Nm ber_set_readbuf ,
.Nm ber_read_elements ,
+.Nm ber_getpos ,
.Nm ber_free_elements ,
.Nm ber_calc_len ,
.Nm ber_set_application ,
+.Nm ber_set_writecallback
.Nm ber_free
.Nd parse ASN.1 with Basic Encoding Rules
.Sh SYNOPSIS
@@ -121,6 +123,8 @@
.Fn ber_set_readbuf struct ber *ber void *buf size_t len
.Ft struct
.Fn ber_element *ber_read_elements struct ber *ber struct ber_element
*root
+.Ft off_t
+.Fn ber_getpos struct ber_element *elm
.Ft void
.Fn ber_free_elements struct ber_element *root
.Ft size_t
@@ -128,6 +132,8 @@
.Ft void
.Fn ber_set_application struct ber *ber unsigned long (*cb)(struct ber_element
*)
.Ft void
+.Fn ber_set_writecallback struct ber_element *elm void (*cb)(void *arg, size_t
offs) void *arg
+.Ft void
.Fn ber_free struct ber *ber
.Sh DESCRIPTION
The
@@ -188,8 +194,10 @@ struct ber_oid {
.Fn ber_write_elements ,
.Fn ber_set_readbuf ,
.Fn ber_read_elements ,
+.Fn ber_getpos ,
.Fn ber_free_elements ,
.Fn ber_set_application ,
+.Fn ber_set_writecallback ,
.Fn ber_free
.Sh RETURN VALUES
Upon successful completion
Index: usr.sbin/snmpd/ber.c
===
RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v
retrieving revision 1.23
diff -u -p -r1.23 ber.c
--- usr.sbin/snmpd/ber.c20 Sep 2010 08:30:13 - 1.23
+++ usr.sbin/snmpd/ber.c18 Jul 2012 14:03:06 -
@@ -618,6 +618,7 @@ ber_scanf_elements(struct ber_element *b
void**ptr;
size_t *len, ret = 0, n = strlen(fmt);
char**s;
+ off_t *pos;
struct ber_oid *o;
struct ber_element *parent[_MAX_SEQ], **e;
@@ -695,6 +696,11 @@ ber_scanf_elements(struct ber_element *b
goto fail;
ret++;
break;
+ case 'p':
+ pos = va_arg(ap, off_t *);
+ *pos = ber_getpos(ber);
+ ret++;
+ continue;
case '{':
case '(':
if (ber-be_encoding != BER_TYPE_SEQUENCE
@@ -712,7 +718,7 @@ ber_scanf_elements(struct ber_element *b
goto fail;
ber = parent[level--];
ret++;
- continue;
+ break;
default:
goto fail;
}
@@ -808,6 +814,12 @@ ber_read_elements(struct ber *ber, struc
return root;
}
+off_t
+ber_getpos(struct ber_element *elm)
+{
+ return elm-be_offs;
+}
+
void
ber_free_elements(struct ber_element *root)
{
@@ -867,6 +879,8 @@ ber_dump_element(struct ber *ber, struct
uint8_t u;
ber_dump_header(ber, root);
+ if (root-be_cb)
+ root-be_cb(root-be_cbarg, ber-br_wptr - ber-br_wbuf);
switch (root-be_encoding) {
case BER_TYPE_BOOLEAN:
@@ -1068,6 +1082,7 @@ ber_read_element(struct ber *ber, struct
elm-be_type = type;
elm-be_len = len;
Re: SNMPv3 Support
On Wed, 18 Jul 2012 16:51:27 +0200, Mike Belopuhov m...@crypt.org.ru wrote: On Wed, Jul 18, 2012 at 4:16 PM, Gerhard Roth wrote: same here, wouldn't it be possible to match the ipsec.conf grammar and ignore the SNMPv3 naming a bit? auth hmac-sha1 authkey fooobar enc aes enckey dkjdkj - instead of - hmac sha authpass foobar cipher aes privpass dkjdkj or maybe authpass and encpass, but what does priv mean. So instead of user name [authpass pass hmac [MD5|SHA]] \ [privpass pass cipher [DES|AES]] let's use user name [hmac-[md5|sha1] authkey key] \ [enc [des|aes] enckey key] Is that ipsec.conf like enough? why weren't all the other _priv instances renamed to _encr? is there any value in keeping SNMP_MSGFLAG_PRIV and such around? uu_privkey looks a bif of an alien alongside uu_authkey. I agree with Reyk that the configuration of snmpd should be possible to anyone who has not read all the SNMPv3 RFCs. OTOH, shouldn't the code somehow reflect which part of the RFCs it implements? Otherwise its hard to understand whats happening. And unfortunately the RFCs use the word priv or privacy consistenly instead of encryption. Gerhard
SNMPv3 Support
Hi all, below you'll find a patch that adds basic SNMPv3 support to OpenBSD's snmpd(8). When I say basic that's because of some limitations: - Traps are still sent via SNMPv2 protocol. They can neither be authenticated nor encrypted. - Transport mode is still UDP. Not additional transport subsystems were added. - Only the User-based Security Model (USM, RFC3414) is supported. View-Based Access Control (VACM, RFC3415) is not included. Just to provide you a little background, I'll explain some details below. Three security levels are defined in RFC3411: 1) noAuthNoPriv: no authentication, no encryption 2) authNoPriv: with authentication, without encryption 3) authPriv: with authentication, with encryption There is a new keyword 'seclevel' in snmpd.conf(5) that allows to define the minimum security level required by snmpd(8). Any requirement higher than noAuthNoPriv will disable SNMPv2 support. The USM offers: - Verification of message contents and authentication of the sender USM adds a HMAC to the SNMP message. The HMAC is calculated over the whole message with the HMAC portion set to zeroes. According to RFC3414 the defined HMAC algorithms are HMAC-MD5-96 and HMAC-SHA-96. The key is derived from an authentication passphrase. - Encryption of the PDU USM encypts only a part of the message, the scoped PDU while the SNMP header remains plaintext. RFC3414 defines only CBC DES but RFC3826 adds CFB128 AES 128 encryption (although this is not part of STD62). The IV is derived from an encryption passphrase. - Protection agains replay attacks The non-authoritative SNMP engines have to synchronize their clocks with the authoritative SNMP engine. RFC3414 demands to reject any SNMPv3 message that has a timestamp that differs more than 150 seconds from the local clock. The USM users together with their HMAC and encryption passphrases have to be defined in snmpd.conf(5). The code already supports multiple users, though without VACM there's not much sense to it. Gerhard [demime 1.01d removed an attachment of type application/octet-stream which had a name of snmpv3.patch]
Re: SNMPv3 Support
On Tue, 17 Jul 2012 11:21:04 +0200, Gerhard Roth gerhard_r...@genua.de wrote:
Hi all,
below you'll find a patch that adds basic SNMPv3 support to OpenBSD's
snmpd(8). When I say basic that's because of some limitations:
- Traps are still sent via SNMPv2 protocol. They can neither be
authenticated nor encrypted.
- Transport mode is still UDP. Not additional transport subsystems
were added.
- Only the User-based Security Model (USM, RFC3414) is supported.
View-Based Access Control (VACM, RFC3415) is not included.
Just to provide you a little background, I'll explain some details
below.
Three security levels are defined in RFC3411:
1) noAuthNoPriv: no authentication, no encryption
2) authNoPriv: with authentication, without encryption
3) authPriv: with authentication, with encryption
There is a new keyword 'seclevel' in snmpd.conf(5) that allows to
define the minimum security level required by snmpd(8). Any requirement
higher than noAuthNoPriv will disable SNMPv2 support.
The USM offers:
- Verification of message contents and authentication of the sender
USM adds a HMAC to the SNMP message. The HMAC is calculated over
the whole message with the HMAC portion set to zeroes.
According to RFC3414 the defined HMAC algorithms are HMAC-MD5-96
and HMAC-SHA-96. The key is derived from an authentication
passphrase.
- Encryption of the PDU
USM encypts only a part of the message, the scoped PDU while the
SNMP header remains plaintext. RFC3414 defines only CBC DES but
RFC3826 adds CFB128 AES 128 encryption (although this is not
part of STD62). The IV is derived from an encryption passphrase.
- Protection agains replay attacks
The non-authoritative SNMP engines have to synchronize their
clocks with the authoritative SNMP engine. RFC3414 demands
to reject any SNMPv3 message that has a timestamp that differs
more than 150 seconds from the local clock.
The USM users together with their HMAC and encryption passphrases
have to be defined in snmpd.conf(5). The code already supports multiple
users, though without VACM there's not much sense to it.
Gerhard
[demime 1.01d removed an attachment of type application/octet-stream which had
a name of snmpv3.patch]
Sorry for using an attachment. Here's the patch inlined:
Index: Makefile
===
RCS file: /cvs/src/usr.sbin/snmpd/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile20 Mar 2012 03:01:26 - 1.8
+++ Makefile17 Jul 2012 06:49:17 -
@@ -4,9 +4,9 @@ PROG= snmpd
MAN= snmpd.8 snmpd.conf.5
SRCS= parse.y ber.c log.c control.c snmpe.c \
mps.c trap.c mib.c smi.c kroute.c snmpd.c timer.c \
- pf.c
+ pf.c usm.c
-LDADD= -levent -lutil -lkvm
+LDADD= -levent -lutil -lkvm -lcrypto
DPADD= ${LIBEVENT} ${LIBUTIL}
CFLAGS+= -Wall -I${.CURDIR}
CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
Index: ber.3
===
RCS file: /cvs/src/usr.sbin/snmpd/ber.3,v
retrieving revision 1.9
diff -u -p -r1.9 ber.3
--- ber.3 25 Feb 2010 09:59:55 - 1.9
+++ ber.3 17 Jul 2012 06:44:44 -
@@ -50,9 +50,11 @@
.Nm ber_write_elements ,
.Nm ber_set_readbuf ,
.Nm ber_read_elements ,
+.Nm ber_getpos ,
.Nm ber_free_elements ,
.Nm ber_calc_len ,
.Nm ber_set_application ,
+.Nm ber_set_callback
.Nm ber_free
.Nd parse ASN.1 with Basic Encoding Rules
.Sh SYNOPSIS
@@ -121,6 +123,8 @@
.Fn ber_set_readbuf struct ber *ber void *buf size_t len
.Ft struct
.Fn ber_element *ber_read_elements struct ber *ber struct ber_element
*root
+.Ft off_t
+.Fn ber_getpos struct ber_element *elm
.Ft void
.Fn ber_free_elements struct ber_element *root
.Ft size_t
@@ -128,6 +132,8 @@
.Ft void
.Fn ber_set_application struct ber *ber unsigned long (*cb)(struct ber_element
*)
.Ft void
+.Fn ber_set_callback struct ber_element *elm void (*cb)(void *arg, size_t offs)
void *arg
+.Ft void
.Fn ber_free struct ber *ber
.Sh DESCRIPTION
The
@@ -188,8 +194,10 @@ struct ber_oid {
.Fn ber_write_elements ,
.Fn ber_set_readbuf ,
.Fn ber_read_elements ,
+.Fn ber_getpos ,
.Fn ber_free_elements ,
.Fn ber_set_application ,
+.Fn ber_set_callback ,
.Fn ber_free
.Sh RETURN VALUES
Upon successful completion
Index: ber.c
===
RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v
retrieving revision 1.23
diff -u -p -r1.23 ber.c
--- ber.c 20 Sep 2010 08:30:13 - 1.23
+++ ber.c 17 Jul 2012 06:44:44 -
@@ -618,6 +618,7 @@ ber_scanf_elements(struct ber_element *b
void**ptr;
size_t *len, ret = 0, n = strlen(fmt);
char**s
mkdep(1) misses compilation errors
Hi,
the exit status of a shell pipeline is the exit status of the last
command in the pipeline. Since mkdep(1) pipes the output of ${CC} into
sed(1), the following check of '$?' checks the exit status of sed(1)
and not the of of cc(1).
This is nasty if some modification broke the compilation of a particular
source file. It cannot be compiled, but mkdep(1) exits with zero and
'make depend' completes without error. Since the source couldn't be
compiled, the resulting depend file will not list this source. Without
having any dependencies, make(1) will not try to rebuild the derived
targets. Unless I do a 'make clean', I won't even notice that my
build is broken.
Below is a possible patch to fix this.
Gerhard
--- usr.bin/mkdep/mkdep.gcc.sh 2008/08/28 09:39:44 1.15
+++ usr.bin/mkdep/mkdep.gcc.sh 2012/06/25 10:47:29
@@ -89,14 +89,9 @@ TMP=`mktemp /tmp/mkdep.XX` || exit 1
trap 'rm -f $TMP ; trap 2 ; kill -2 $$' 1 2 3 13 15
if [ x$file = x ]; then
- ${CC:-cc} -M $@
+ ${CC:-cc} -M $@ $TMP
else
- ${CC:-cc} -M $@ cat $file
-fi |
-if [ x$pflag = x ]; then
- sed -e 's; \./; ;g' $TMP
-else
- sed -e 's;\.o[ ]*:; :;' -e 's; \./; ;g' $TMP
+ ${CC:-cc} -M $@ cat $file $TMP
fi
if [ $? != 0 ]; then
@@ -105,15 +100,24 @@ if [ $? != 0 ]; then
exit 1
fi
+postproc() {
+ in=$1
+ if [ x$pflag = x ]; then
+ sed -e 's; \./; ;g' $in
+ else
+ sed -e 's;\.o[ ]*:; :;' -e 's; \./; ;g' $in
+ fi
+}
+
if [ $append = 1 ]; then
- cat $TMP $D
+ postproc $TMP $D
if [ $? != 0 ]; then
echo 'mkdep: append failed.'
rm -f $TMP
exit 1
fi
else
- mv -f $TMP $D
+ postproc $TMP $D
if [ $? != 0 ]; then
echo 'mkdep: rename failed.'
rm -f $TMP
Gerhard
--
genua
Gesellschaft fuer Netzwerk- und Unix-Administration mbH
Domagkstrasse 7, 85551 Kirchheim bei Muenchen
tel +49 89 991950-0, fax -999, www.genua.de
Geschaeftsfuehrer: Dr. Magnus Harlander, Dr. Michaela Harlander,
Bernhard Schneck. Amtsgericht Muenchen HRB 98238
IP Routing Fix
Hi everybody,
there is code in ether_output() that handles the case that we have to
use a gateway to send the packet. This code does a lookup for the route
required to reach the gateway.
In case there is a gateway for which no direct route exists (surely a
stupid thing, but possible during reconfiguration), rtalloc1() will
return the same gateway routing entry as the one we're trying to resolve
in ether_output(). Hence this would set 'rt-rt_gwroute' to point to
'rt'.
This can later lead to a panic when rtrequest1() is processing the
RTM_DELETE request. RTFREE() of the 'rt-rt_gwroute' would return the
memory to the pool if RTF_UP flag is not set. Back in rtrequest1(),
dereferencing 'rt' would access freed memory.
There should always exist a direct route to the gateway or else it is
unreachable. There is no such concept as a gateway to reach a gateway.
We can only route for the one next hop and never beyond. If this
requirement is fulfilled, a gateway routing entry can never reference
another gateway routing entry and hence there are no more self
references.
The patch below adds a check to ether_output(). The same modification
is also required for atm_output(), fddi_output(), and nd6_output(),
but the code is so much identical that I omitted the patches to them.
Regards,
Gerhard
RCS file: src/sys/net/if_ethersubr.c,v
retrieving revision 1.151
diff -u -p -r1.151 src/sys/net/if_ethersubr.c
--- src/sys/net/if_ethersubr.c 2011/07/09 01:47:18 1.151
+++ src/sys/net/if_ethersubr.c 2012/06/22 09:40:03
@@ -270,8 +270,19 @@ ether_output(ifp0, m0, dst, rt0)
lookup:
rt-rt_gwroute = rtalloc1(rt-rt_gateway,
RT_REPORT, ifp-if_rdomain);
- if ((rt = rt-rt_gwroute) == NULL)
+ if (rt-rt_gwroute == NULL)
senderr(EHOSTUNREACH);
+
+ /*
+* There should be an immediate route to
the
+* gateway.
+*/
+ if (rt-rt_gwroute-rt_flags
RTF_GATEWAY) {
+ rtfree(rt-rt_gwroute);
+ rt-rt_gwroute = NULL;
+ senderr(EHOSTUNREACH);
+ }
+ rt = rt-rt_gwroute;
}
}
if (rt-rt_flags RTF_REJECT)
--
genua
Gesellschaft fuer Netzwerk- und Unix-Administration mbH
Domagkstrasse 7, 85551 Kirchheim bei Muenchen
tel +49 89 991950-0, fax -999, www.genua.de
Geschaeftsfuehrer: Dr. Magnus Harlander, Dr. Michaela Harlander,
Bernhard Schneck. Amtsgericht Muenchen HRB 98238
Memory leak in snmpd(8)
Hi,
with the OPENBSD-CARP-MIB a memory leak was introduced to snmpd(8):
RCS file: mib.c,v
retrieving revision 1.52
diff -u -p -r1.52 mib.c
--- mib.c 2012/03/20 03:01:26 1.52
+++ mib.c 2012/05/24 12:53:35
@@ -2713,7 +2713,7 @@ mib_carpiftable(struct oid *oid, struct ber_oid *o,
st
/* Get and verify the current row index */
idx = o-bo_id[OIDIDX_carpIfEntry];
- if ((cif = mib_carpifget(cif, idx)) == NULL) {
+ if (mib_carpifget(cif, idx) == NULL) {
free(cif);
return (1);
}
Gerhard
--
GeNUA, Gesellschaft fCr Netzwerk- und Unix-Administration mbH
Domagkstrasse 7, 85551 Kirchheim bei Muenchen
tel +49 89 991950-0, fax -999, www.genua.de
Geschaeftsfuehrer: Dr. Magnus Harlander, Dr. Michaela Harlander,
Bernhard Schneck. Amtsgericht Muenchen HRB 98238
Re: Memory leak in snmpd(8)
On Thu, 24 May 2012 16:16:02 +0200, Kenneth R Westerback
kwesterb...@rogers.com wrote:
On Thu, May 24, 2012 at 01:54:36PM +0200, Gerhard Roth wrote:
Hi,
with the OPENBSD-CARP-MIB a memory leak was introduced to snmpd(8):
RCS file: mib.c,v
retrieving revision 1.52
diff -u -p -r1.52 mib.c
--- mib.c 2012/03/20 03:01:26 1.52
+++ mib.c 2012/05/24 12:53:35
@@ -2713,7 +2713,7 @@ mib_carpiftable(struct oid *oid, struct ber_oid
*o,
st
/* Get and verify the current row index */
idx = o-bo_id[OIDIDX_carpIfEntry];
- if ((cif = mib_carpifget(cif, idx)) == NULL) {
+ if (mib_carpifget(cif, idx) == NULL) {
free(cif);
return (1);
}
Gerhard
Calling mib_carpget() seems a tad over complex. Wouldn't the diff
below make it cleaner? Untested except by gcc.
And doesn't the socket 's' leak too, or does SIOCGVH returning -1
mean 's' was closed?
Ken
Index: mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.52
diff -u -p -r1.52 mib.c
--- mib.c 20 Mar 2012 03:01:26 - 1.52
+++ mib.c 24 May 2012 14:11:22 -
@@ -1360,7 +1360,7 @@ intmib_carpstats(struct oid *, struct
int mib_carpiftable(struct oid *, struct ber_oid *, struct ber_element
**);
int mib_carpifnum(struct oid *, struct ber_oid *, struct ber_element
**);
struct carpif
- *mib_carpifget(struct carpif *, u_int);
+ *mib_carpifget(u_int);
int mib_memiftable(struct oid *, struct ber_oid *, struct ber_element
**);
static struct oid openbsd_mib[] = {
@@ -2647,9 +2647,10 @@ mib_carpifnum(struct oid *oid, struct be
}
struct carpif *
-mib_carpifget(struct carpif *cif, u_int idx)
+mib_carpifget(u_int idx)
{
struct kif *kif;
+ struct carpif *cif;
int s;
struct ifreq ifr;
struct carpreq carpr;
@@ -2692,9 +2693,12 @@ mib_carpifget(struct carpif *cif, u_int
if (ioctl(s, SIOCGVH, (caddr_t)ifr) == -1)
return (NULL);
- memset(cif, 0, sizeof(struct carpif));
- memcpy(cif-carpr, carpr, sizeof(struct carpreq));
- memcpy(cif-kif, kif, sizeof(struct kif));
+ cif = malloc(sizeof(struct carpif));
+ if (cif != NULL) {
+ memset(cif, 0, sizeof(struct carpif));
+ memcpy(cif-carpr, carpr, sizeof(struct carpreq));
+ memcpy(cif-kif, kif, sizeof(struct kif));
+ }
close(s);
@@ -2707,16 +2711,11 @@ mib_carpiftable(struct oid *oid, struct
u_int32_tidx;
struct carpif *cif;
- if ((cif = malloc(sizeof(struct carpif))) == NULL)
- return (1);
-
/* Get and verify the current row index */
idx = o-bo_id[OIDIDX_carpIfEntry];
- if ((cif = mib_carpifget(cif, idx)) == NULL) {
- free(cif);
+ if ((cif = mib_carpifget(idx)) == NULL)
return (1);
- }
/* Tables need to prepend the OID on their own */
o-bo_id[OIDIDX_carpIfEntry] = cif-kif.if_index;
Hi Ken,
I like your patch; it's better than mine.
And you are right about the socket fd leak. If an ioctl()
would close the filedes passed in that would be a very
awkward API.
Gerhard
--
GeNUA, Gesellschaft fCr Netzwerk- und Unix-Administration mbH
Domagkstrasse 7, 85551 Kirchheim bei Muenchen
tel +49 89 991950-0, fax -999, www.genua.de
Geschaeftsfuehrer: Dr. Magnus Harlander, Dr. Michaela Harlander,
Bernhard Schneck. Amtsgericht Muenchen HRB 98238
