Re: nd6_output() and NULL routes
On Mon, May 23, 2016 at 04:55:06PM +0200, Martin Pieuchot wrote:
> +++ net/pf.c 23 May 2016 14:43:19 -
> +++ net/pf_norm.c 23 May 2016 14:48:01 -
I have tested the pf part with regress/sys/net/pf_forward. This
is OK bluhm@.
> @@ -1532,42 +1527,17 @@ nd6_output(struct ifnet *ifp, struct mbu
...
> - /*
> - * Since nd6_is_addr_neighbor() internally calls nd6_lookup(),
> - * the condition below is not very efficient. But we believe
> - * it is tolerable, because this should be a rare case.
> - */
> - if (nd6_is_addr_neighbor(dst, ifp)) {
> - rt = nd6_lookup(&dst->sin6_addr, 1, ifp,
> - ifp->if_rdomain);
> - if (rt != NULL) {
> - created = 1;
> - ln = (struct llinfo_nd6 *)rt->rt_llinfo;
> - }
> - }
I cannot see where the nd6_lookup() has moved to. Where does it
happen now?
> + if (!ISSET(rt->rt_flags, RTF_LLINFO)) {
> + char addr[INET6_ADDRSTRLEN];
> + log(LOG_DEBUG, "%s: %s: route contains no ND information\n",
> + __func__, inet_ntop(AF_INET6,
> + &satosin6(rt_key(rt))->sin6_addr, addr, sizeof(addr)));
> + m_freem(m);
> + return (EINVAL);
> }
I think EINVAL is for invalid input from user land. Would EHOSTUNREACH
be more appropriate?
There should be more real world testing with the ether_output() and
nd6_output() part. Can you commit pf and send the diff that assumes
rt != NULL for v4 and v6 output?
bluhm
Re: Patch for man to ignore trailing spaces in man.conf
> From: todd.mil...@courtesan.com
> To: mayukh_b...@hotmail.com
> CC: tech@openbsd.org; schwa...@openbsd.org
> Subject: Re: Patch for man to ignore trailing spaces in man.conf
> Date: Mon, 23 May 2016 10:01:54 -0600
>
> Your patch got munged by your mailer. Here's a different patch
> that achieves the same thing and also removes the requirement that
> the last line in man.conf end with a newline.
>
> - todd
>
> Index: usr.bin/mandoc/manpath.c
> ===
> RCS file: /cvs/src/usr.bin/mandoc/manpath.c,v
> retrieving revision 1.17
> diff -u -p -u -r1.17 manpath.c
> --- usr.bin/mandoc/manpath.c 7 Nov 2015 17:58:52 - 1.17
> +++ usr.bin/mandoc/manpath.c 23 May 2016 15:59:09 -
> @@ -173,13 +173,12 @@ manconf_file(struct manconf *conf, const
>
> while ((linelen = getline(&line, &linesz, stream)) != -1) {
> cp = line;
> - ep = cp + linelen;
> - if (ep[-1] != '\n')
> - break;
> - *--ep = '\0';
> + ep = cp + linelen - 1;
> + while (ep> cp && isspace((unsigned char)*ep))
> + *ep-- = '\0';
> while (isspace((unsigned char)*cp))
> cp++;
> - if (*cp == '#')
> + if (cp == ep || *cp == '#')
> continue;
>
> for (tok = 0; tok < sizeof(toks)/sizeof(toks[0]); tok++) {
Thanks much. I like your patch better too.
Cheers,
Mayukh
Re: Patch for man to ignore trailing spaces in man.conf
Hi Todd,
Todd C. Miller wrote on Mon, May 23, 2016 at 10:01:54AM -0600:
> Your patch got munged by your mailer. Here's a different patch
> that achieves the same thing and also removes the requirement that
> the last line in man.conf end with a newline.
OK schwarze@.
Thanks,
Ingo
> Index: usr.bin/mandoc/manpath.c
> ===
> RCS file: /cvs/src/usr.bin/mandoc/manpath.c,v
> retrieving revision 1.17
> diff -u -p -u -r1.17 manpath.c
> --- usr.bin/mandoc/manpath.c 7 Nov 2015 17:58:52 - 1.17
> +++ usr.bin/mandoc/manpath.c 23 May 2016 15:59:09 -
> @@ -173,13 +173,12 @@ manconf_file(struct manconf *conf, const
>
> while ((linelen = getline(&line, &linesz, stream)) != -1) {
> cp = line;
> - ep = cp + linelen;
> - if (ep[-1] != '\n')
> - break;
> - *--ep = '\0';
> + ep = cp + linelen - 1;
> + while (ep > cp && isspace((unsigned char)*ep))
> + *ep-- = '\0';
> while (isspace((unsigned char)*cp))
> cp++;
> - if (*cp == '#')
> + if (cp == ep || *cp == '#')
> continue;
>
> for (tok = 0; tok < sizeof(toks)/sizeof(toks[0]); tok++) {
>
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: libfuse: null-terminate argv (fuse_opt_insert_arg)
On Fri, May 20, 2016 at 10:57:10PM +0800, Ray Lai wrote:
> On Thu, 19 May 2016 18:57:50 +0200
> Hiltjo Posthuma wrote:
>
> > Hi peoples,
> >
> > This diff makes sure to NUL-terminate argv when parsing options in libfuse.
> > The upstream/other libfuse does it this way. This fixes an issue with the
> > sysutils/sshfs port, it uses execvp(3) on the fuse_args argv and this gave
> > an error "bad address".
> >
> >
> > Index: fuse_opt.c
> > ===
> > RCS file: /cvs/src/lib/libfuse/fuse_opt.c,v
> > retrieving revision 1.15
> > diff -u -p -r1.15 fuse_opt.c
> > --- fuse_opt.c 19 Oct 2015 17:24:07 - 1.15
> > +++ fuse_opt.c 19 May 2016 16:50:39 -
> > @@ -353,7 +353,7 @@ fuse_opt_insert_arg(struct fuse_args *ar
> > if (p < 0 || p > args->argc)
> > return (-1);
> >
> > - av = reallocarray(args->argv, args->argc + 1, sizeof(*av));
> > + av = reallocarray(args->argv, args->argc + 2, sizeof(*av));
> > if (av == NULL)
> > return (-1);
> >
> > @@ -365,6 +365,7 @@ fuse_opt_insert_arg(struct fuse_args *ar
> >
> > args->argc++;
> > args->argv = av;
> > + args->argv[args->argc] = NULL;
> > for (i = p; i < args->argc; i++) {
> > next_arg = args->argv[i];
> > args->argv[i] = this_arg;
> >
> >
> > [0] upstream libfuse fuse_opt_add_arg:
> > https://github.com/libfuse/libfuse/blob/master/lib/fuse_opt.c#L62
> >
>
> This looks correct, but don't forget about free_argv() and alloc_argv().
>
I discussed with Ray his concerns of the patch off-list. We agreed there is no
memory leak or invalid use of argv AFAICT.
Can this patch be applied to base libfuse?
--
Kind regards,
Hiltjo
Re: MBIM Patch - Part 2 of 4
On 23/05/16(Mon) 16:40, Stuart Henderson wrote: > What I don't understand is why routing to a point-to-point interface needs > anything other than the interface name to be used for the destination. It doesn't really need anything else. At least not from the network stack point of view. Well I'm lying, it needs somebody who can do the plumbing :)
Re: Patch for man to ignore trailing spaces in man.conf
Your patch got munged by your mailer. Here's a different patch
that achieves the same thing and also removes the requirement that
the last line in man.conf end with a newline.
- todd
Index: usr.bin/mandoc/manpath.c
===
RCS file: /cvs/src/usr.bin/mandoc/manpath.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 manpath.c
--- usr.bin/mandoc/manpath.c7 Nov 2015 17:58:52 - 1.17
+++ usr.bin/mandoc/manpath.c23 May 2016 15:59:09 -
@@ -173,13 +173,12 @@ manconf_file(struct manconf *conf, const
while ((linelen = getline(&line, &linesz, stream)) != -1) {
cp = line;
- ep = cp + linelen;
- if (ep[-1] != '\n')
- break;
- *--ep = '\0';
+ ep = cp + linelen - 1;
+ while (ep > cp && isspace((unsigned char)*ep))
+ *ep-- = '\0';
while (isspace((unsigned char)*cp))
cp++;
- if (*cp == '#')
+ if (cp == ep || *cp == '#')
continue;
for (tok = 0; tok < sizeof(toks)/sizeof(toks[0]); tok++) {
Re: Patch for man to ignore trailing spaces in man.conf
Hi Mayukh, On Sun, May 22, 2016 at 11:17:18PM -0700, Mayukh Bose wrote: > [...] > Here's my patch to fix this: > [...] Ironically, your patch about whitespace handling has mangled whitespace. Can you send it again? Maybe your MUA tried sending it as some sort of formatted text instead of plain text. -- Gregor
Re: MBIM Patch - Part 2 of 4
On 23/05/16(Mon) 16:51, Gerhard Roth wrote:
> On Mon, 23 May 2016 16:18:29 +0200 Martin Pieuchot 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.
> > > +void
> > > +mbim_watchdog(struct ifnet *ifp)
> > > +{
> > > + struct mbim_softc *sc = ifp->if_softc;
> > > +
> > > + if (usbd_is_dying(sc->sc_udev))
> > > + return;
> > > +
> > > + ifp->if_oerrors++;
> > > + log(LOG_WARNING, "%s: watchdog timeout\n", DEVNAM(sc));
> >
> > Watchdog are run in a task now, so it should be possible to stop/start
> > devices even if you need to sleep.
>
> What do you mean? That there's no need for the usbd_is_dying() call?
I mean that most of the watchdog for USB drivers do nothing
because historically they could not call _stop() and _start().
PCI devices generally print a message then call _init() which does
_stop() and _start(). It should be possible to do the same for USB
devices now.
> >
> > 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.
> > > +/* Code copied from if_spppsubr.c */
> > > +void
> > > +mbim_update_gw(struct ifnet *ifp)
> > > +{
> > > + unsigned int tid;
> > > +
> > > + /* update routing table */
> > > + for (tid = 0; tid <= RT_TABLEID_MAX; tid++) {
> > > + while (rtable_walk(tid, AF_INET, mbim_update_gw_walker, ifp) ==
> > > + EAGAIN)
> > > + ; /* nothing */
> > > + }
> > > +}
> > > +
> > > +int
> > > +mbim_update_gw_walker(struct rtentry *rt, void *arg, unsigned int id)
> > > +{
> > > + struct ifnet *ifp = arg;
> > > +
> > > + if (rt->rt_ifidx == ifp->if_index) {
> > > + if (rt->rt_ifa->ifa_dstaddr->sa_family !=
> > > + rt->rt_gateway->sa_family ||
> > > + !ISSET(rt->rt_flags, RTF_GATEWAY))
> > > + return 0; /* do not modify non-gateway routes */
> > > + log(LOG_INFO, "%s: update gw %s -> %s\n", DEVNAM(ifp->if_softc),
> > > + mbim_ntop(rt->rt_gateway),
> > > + mbim_ntop(rt->rt_ifa->ifa_dstaddr));
> > > + rt_setgate(rt, rt->rt_ifa->ifa_dstaddr);
> > > + }
> >
> > This is the kind of horrors I have been removing during the past years.
> >
> > 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?
> What other sensible routing could there be? Only routing some selected IP
> addresses through your mobile uplink doesn't seem like the normal use case.
I'm not talking about "sensible routing", I'm saying that copy/paste
programing is bad, especially if you copy the wrong pattern that we have
in the kernel.
> > > +void
> > > +mbim_rxeof(struct usbd_xfer *xfer, void *priv, usbd_status status)
> > > +{
> > > + struct mbim_softc *sc = priv;
> > > + struct ifnet *ifp = GET_IFP(sc);
> > > +
> > > + if (usbd_is_dying(sc->sc_udev) || !(ifp->if_flags & IFF_RUNNING))
> > > + return;
> > > +
> > > + if (status != USBD_NORMAL_COMPLETION) {
> > > + if (status == USBD_NOT_STARTED || status == USBD_CANCELLED)
> > > + return;
> > > + if (usbd_ratecheck(&sc->sc_rx_ratechk))
> > > + DPRINTF("%s: rx error: %s\n", DEVNAM(sc),
> > > + usbd_errstr(sta
Re: MBIM Patch - Part 2 of 4
> > 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. > > > > What other sensible routing could there be? Only routing some selected IP > > addresses through your mobile uplink doesn't seem like the normal use case. > > Routing just certain destinations over a particular PPP is definitely > valid, and I think the same applies here. (Maybe you have a fast ISP and a > cheap ISP and want to use the fast one for some VPN or important traffic). > > What I don't understand is why routing to a point-to-point interface needs > anything other than the interface name to be used for the destination. Such a choice being made at the driver level is quite a layer violation. Pants on fire...
Re: MBIM Patch - Part 2 of 4
On 2016/05/23 16:51, Gerhard Roth wrote: > > This is the kind of horrors I have been removing during the past years. > > > > 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. > > What other sensible routing could there be? Only routing some selected IP > addresses through your mobile uplink doesn't seem like the normal use case. Routing just certain destinations over a particular PPP is definitely valid, and I think the same applies here. (Maybe you have a fast ISP and a cheap ISP and want to use the fast one for some VPN or important traffic). What I don't understand is why routing to a point-to-point interface needs anything other than the interface name to be used for the destination.
Patch for man to ignore trailing spaces in man.conf
Hi all, I used pkg_add to add jdk and after installation, it told me to
add /usr/local/jdk-1.7.0/man to man.conf. Therefore, I googled on modifying
man.conf and came across the OpenBSD man page
online:http://man.openbsd.org/OpenBSD-current/man5/man.conf.5
I cut-and-pasted the EXAMPLES section into /etc/man.conf exactly as stated from
the webpage and then added an extra line for the jdk manpath:manpath
/usr/share/man
manpath /usr/X11R6/man
manpath /usr/local/man
manpath /usr/local/jdk-1.7.0/man
And after this, "man java" worked fine, but "man man", "man printf", "man
man.conf" etc. stopped working! After a bit of debugging, I found that the
problem was that the manpath lines for the first two lines had trailing spaces
at the end (I had cut/pasted them from the webpage above, but it appears the
page has the trailing spaces in it as well). So I figured I should fix man so
that it properly handles trailing spaces in man.conf. Here's my patch to fix
this:
$ cvs -d anon...@anoncvs.usa.openbsd.org:/cvs diffcvs server: Diffing .Index:
manpath.c===RCS
file: /cvs/src/usr.bin/mandoc/manpath.c,vretrieving revision 1.17diff -u -p
-r1.17 manpath.c--- manpath.c 7 Nov 2015 17:58:52 - 1.17+++
manpath.c 23 May 2016 06:15:00 -@@ -176,7 +176,14 @@ manconf_file(struct
manconf *conf, constep = cp + linelen;if
(ep[-1] != '\n')break;- *--ep = '\0';+
ep--;+ while (ep > cp && isspace(*ep)) {+
*ep = '\0';+ ep--;+ }+
if (ep == cp)+ continue;+while
(isspace((unsigned char)*cp))cp++;if
(*cp == '#')
Cheers,Mayukh
Re: MBIM Patch - Part 2 of 4
> > 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. Please start simple, and solve that problem later. Too much arriving in one set of diffs.
Re: MBIM Patch - Part 1 of 4
On 23/05/16(Mon) 16:18, Gerhard Roth wrote: > > > 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 Yes please. sppp_* way of doing things should die. > > > [...] 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 This looks like a timing issue before reloading the descriptor. You could try to increase the timeout in usbd_new_device(). Alternatively, does the diff below help? It might be incomplete, but the code fetching the device descriptor in our stack is for sure outdated. Index: usb_subr.c === RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v retrieving revision 1.123 diff -u -p -r1.123 usb_subr.c --- usb_subr.c 23 May 2016 11:31:12 - 1.123 +++ usb_subr.c 23 May 2016 14:40:29 - @@ -1048,7 +1048,12 @@ usbd_new_device(struct device *parent, s dev->def_ep_desc.bDescriptorType = UDESC_ENDPOINT; dev->def_ep_desc.bEndpointAddress = USB_CONTROL_ENDPOINT; dev->def_ep_desc.bmAttributes = UE_CONTROL; - USETW(dev->def_ep_desc.wMaxPacketSize, USB_MAX_IPACKET); + + if (speed == USB_SPEED_LOW) + USETW(dev->def_ep_desc.wMaxPacketSize, USB_MAX_IPACKET); + else + USETW(dev->def_ep_desc.wMaxPacketSize, 64); + dev->def_ep_desc.bInterval = 0; dev->quirks = &usbd_no_quirk;
Re: nd6_output() and NULL routes
On 23/05/16(Mon) 15:59, Alexander Bluhm wrote:
> On Mon, May 23, 2016 at 02:47:02PM +0200, Martin Pieuchot wrote:
> > @@ -750,7 +750,15 @@ pf_refragment6(struct mbuf **m0, struct
> > if (ifp == NULL) {
> > ip6_forward(m, 0);
> > } else if ((u_long)m->m_pkthdr.len <= ifp->if_mtu) {
> > - nd6_output(ifp, m, dst, NULL);
> > + struct rtentry *rt;
> > + rt = rtalloc(sin6tosa(dst), RT_RESOLVE,
> > + m->m_pkthdr.ph_rtableid);
>
> Should we place the rtalloc() outside of the loop and use the same
> route or all fragments?
That would be a performance improvement!
> > @@ -1507,20 +1507,15 @@ nd6_output(struct ifnet *ifp, struct mbu
> > struct mbuf *m = m0;
> > struct rtentry *rt = rt0;
> > struct llinfo_nd6 *ln = NULL;
> > - int created = 0, error = 0;
> > + int error = 0;
> >
> > if (IN6_IS_ADDR_MULTICAST(&dst->sin6_addr))
> > goto sendpkt;
>
> No kassert rt0 != NULL in here in nd6_output() ?
nop because next step is to merge nd6_output() and nd6_storelladdr()
into nd6_resolve() and match the behavior of arpresolve(). So the
KASSERT() in ether_output() should be enough.
Index: net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.971
diff -u -p -r1.971 pf.c
--- net/pf.c23 May 2016 12:26:28 - 1.971
+++ net/pf.c23 May 2016 14:43:19 -
@@ -5665,11 +5665,13 @@ pf_route6(struct mbuf **m, struct pf_rul
{
struct mbuf *m0;
struct sockaddr_in6 *dst, sin6;
+ struct rtentry *rt = NULL;
struct ip6_hdr *ip6;
struct ifnet*ifp = NULL;
struct pf_addr naddr;
struct pf_src_node *sns[PF_SN_MAX];
struct m_tag*mtag;
+ unsigned int rtableid;
if (m == NULL || *m == NULL || r == NULL ||
(dir != PF_IN && dir != PF_OUT) || oifp == NULL)
@@ -5702,6 +5704,7 @@ pf_route6(struct mbuf **m, struct pf_rul
dst->sin6_family = AF_INET6;
dst->sin6_len = sizeof(*dst);
dst->sin6_addr = ip6->ip6_dst;
+ rtableid = m0->m_pkthdr.ph_rtableid;
if (!r->rt) {
m0->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
@@ -5754,7 +5757,13 @@ pf_route6(struct mbuf **m, struct pf_rul
if ((mtag = m_tag_find(m0, PACKET_TAG_PF_REASSEMBLED, NULL))) {
(void) pf_refragment6(&m0, mtag, dst, ifp);
} else if ((u_long)m0->m_pkthdr.len <= ifp->if_mtu) {
- nd6_output(ifp, m0, dst, NULL);
+ rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
+ if (rt == NULL) {
+ ip6stat.ip6s_noroute++;
+ goto bad;
+ }
+ nd6_output(ifp, m0, dst, rt);
+ rtfree(rt);
} else {
icmp6_error(m0, ICMP6_PACKET_TOO_BIG, 0, ifp->if_mtu);
}
Index: net/pf_norm.c
===
RCS file: /cvs/src/sys/net/pf_norm.c,v
retrieving revision 1.183
diff -u -p -r1.183 pf_norm.c
--- net/pf_norm.c 24 Nov 2015 13:37:16 - 1.183
+++ net/pf_norm.c 23 May 2016 14:48:01 -
@@ -687,6 +687,7 @@ pf_refragment6(struct mbuf **m0, struct
{
struct mbuf *m = *m0, *t;
struct pf_fragment_tag *ftag = (struct pf_fragment_tag *)(mtag + 1);
+ struct rtentry *rt = NULL;
u_int32_tmtu;
u_int16_thdrlen, extoff, maxlen;
u_int8_t proto;
@@ -742,6 +743,16 @@ pf_refragment6(struct mbuf **m0, struct
DPFPRINTF(LOG_NOTICE, "refragment error %d", error);
action = PF_DROP;
}
+
+ if (ifp == NULL) {
+ rt = rtalloc(sin6tosa(dst), RT_RESOLVE,
+ m->m_pkthdr.ph_rtableid);
+ if (rt == NULL) {
+ ip6stat.ip6s_noroute++;
+ error = -1;
+ }
+ }
+
for (t = m; m; m = t) {
t = m->m_nextpkt;
m->m_nextpkt = NULL;
@@ -750,7 +761,7 @@ pf_refragment6(struct mbuf **m0, struct
if (ifp == NULL) {
ip6_forward(m, 0);
} else if ((u_long)m->m_pkthdr.len <= ifp->if_mtu) {
- nd6_output(ifp, m, dst, NULL);
+ nd6_output(ifp, m, dst, rt);
} else {
icmp6_error(m, ICMP6_PACKET_TOO_BIG, 0,
ifp->if_mtu);
@@ -759,6 +770,7 @@ pf_refragment6(struct mbuf **m0, struct
m_freem(m);
}
}
+ rtfree(rt);
return
Re: MBIM Patch - Part 2 of 4
On Mon, 23 May 2016 16:18:29 +0200 Martin Pieuchot 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, &iter);
> > + hard_mtu = MBIM_MAXSEGSZ_MINVAL;
> > + while ((desc = usbd_desc_iter_next(&iter))) {
> > + if (desc->bDescriptorType != UDESC_CS_INTERFACE)
> > + continue;
> > + switch (desc->bDescriptorSubtype) {
> > + case UDESCSUB_MBIM:
> > + md = (struct mbim_descriptor *)desc;
> > + v = UGETW(md->bcdMBIMVersion);
> > + sc->sc_ver_maj = MBIM_VER_MAJOR(v);
> > + sc->sc_ver_min = MBIM_VER_MINOR(v);
> > + sc->sc_ctrl_len = UGETW(md->wMaxControlMessage);
> > + /* Never trust a USB device! Could try to exploit us */
> > + if (sc->sc_ctrl_len < MBIM_CTRLMSG_MINLEN ||
> > + sc->sc_ctrl_len > MBIM_CTRLMSG_MAXLEN) {
> > + printf("%s: control message len %d out of "
> > + "bounds [%d .. %d]\n", DEVNAM(sc),
>
Re: MBIM Patch - Part 2 of 4
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.
> 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.
> +/*
> + * 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.
> +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, &iter);
> + hard_mtu = MBIM_MAXSEGSZ_MINVAL;
> + while ((desc = usbd_desc_iter_next(&iter))) {
> + if (desc->bDescriptorType != UDESC_CS_INTERFACE)
> + continue;
> + switch (desc->bDescriptorSubtype) {
> + case UDESCSUB_MBIM:
> + md = (struct mbim_descriptor *)desc;
> + v = UGETW(md->bcdMBIMVersion);
> + sc->sc_ver_maj = MBIM_VER_MAJOR(v);
> + sc->sc_ver_min = MBIM_VER_MINOR(v);
> + sc->sc_ctrl_len = UGETW(md->wMaxControlMessage);
> + /* Never trust a USB device! Could try to exploit us */
> + if (sc->sc_ctrl_len < MBIM_CTRLMSG_MINLEN ||
> + sc->sc_ctrl_len > MBIM_CTRLMSG_MAXLEN) {
> + printf("%s: control message len %d out of "
> + "bounds [%d .. %d]\n", DEVNAM(sc),
> + sc->sc_ctrl_len, MBIM_CTRLMSG_MINLEN,
> + MBIM_CTRLMSG_MAXLEN);
> + /* cont. anyway */
> + }
> + sc->sc_maxpktlen = UGETW(md->wMaxSegmentSize);
> + if (sc->sc_maxpktlen < MBIM_MAXSEGSZ_MINVAL) {
> + printf("%s: ignoring invalid segment size %d\n",
> +
Re: MBIM Patch - Part 1 of 4
On Mon, 23 May 2016 15:54:36 +0200 Martin Pieuchot 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
Re: nd6_output() and NULL routes
On Mon, May 23, 2016 at 02:47:02PM +0200, Martin Pieuchot wrote:
> @@ -5506,6 +5506,7 @@ pf_route(struct mbuf **m, struct pf_rule
> int error = 0;
> unsigned int rtableid;
>
> +
> if (m == NULL || *m == NULL || r == NULL ||
> (dir != PF_IN && dir != PF_OUT) || oifp == NULL)
> panic("pf_route: invalid parameters");
No double empty line please.
> @@ -750,7 +750,15 @@ pf_refragment6(struct mbuf **m0, struct
> if (ifp == NULL) {
> ip6_forward(m, 0);
> } else if ((u_long)m->m_pkthdr.len <= ifp->if_mtu) {
> - nd6_output(ifp, m, dst, NULL);
> + struct rtentry *rt;
> + rt = rtalloc(sin6tosa(dst), RT_RESOLVE,
> + m->m_pkthdr.ph_rtableid);
Should we place the rtalloc() outside of the loop and use the same
route or all fragments?
> + if (rt == NULL) {
> + ip6stat.ip6s_noroute++;
m_freem(m) is missing, you leak a mbuf in this case.
> + } else {
> + nd6_output(ifp, m, dst, rt);
> + rtfree(rt);
> + }
> } else {
> icmp6_error(m, ICMP6_PACKET_TOO_BIG, 0,
> ifp->if_mtu);
> @@ -1507,20 +1507,15 @@ nd6_output(struct ifnet *ifp, struct mbu
> struct mbuf *m = m0;
> struct rtentry *rt = rt0;
> struct llinfo_nd6 *ln = NULL;
> - int created = 0, error = 0;
> + int error = 0;
>
> if (IN6_IS_ADDR_MULTICAST(&dst->sin6_addr))
> goto sendpkt;
No kassert rt0 != NULL in here in nd6_output() ?
>
> - /*
> - * next hop determination.
> - */
> - if (rt0 != NULL) {
> - error = rt_checkgate(rt0, &rt);
> - if (error) {
> - m_freem(m);
> - return (error);
> - }
> + error = rt_checkgate(rt0, &rt);
> + if (error) {
> + m_freem(m);
> + return (error);
> }
>
> if (nd6_need_cache(ifp) == 0)
Re: MBIM Patch - Part 3 of 4
This is a highly invasive change to ifconfig, for such a minor feature. I suspect this will overflow the ramdisks.
Re: MBIM Patch - Part 1 of 4
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.
> # 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?
> [...] 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?
> 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.c29 Nov 2015 16:30:48 - 1.88
> +++ sys/dev/usb/uhub.c23 May 2016 09:50:08 -
> @@ -523,7 +523,9 @@ uhub_port_connect(struct uhub_softc *sc,
> {
> struct usbd_port *up = &sc->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);
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->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. Thi
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_roaming
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)) \
+ mbim
bridge, carp and HW-vlan
HW-vlan breaks our if_input() design because vlan packets are
decapsulated before calling if_input().
So it doesn't matter in which order you configured your pseudo-driver,
they all have to deal with this layer violation.
Here's a jumbo diff for carp(4) and bridge(4).
Index: net/if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.278
diff -u -p -r1.278 if_bridge.c
--- net/if_bridge.c 12 Apr 2016 06:20:30 - 1.278
+++ net/if_bridge.c 23 May 2016 13:17:08 -
@@ -1063,6 +1063,18 @@ bridge_process(struct ifnet *ifp, struct
if ((sc->sc_if.if_flags & (IFF_UP|IFF_RUNNING)) != (IFF_UP|IFF_RUNNING))
goto reenqueue;
+#if NVLAN > 0
+ /*
+* If the underlying interface removed the VLAN header itself,
+* add it back.
+*/
+ if (ISSET(m->m_flags, M_VLANTAG)) {
+ m = vlan_inject(m, ETHERTYPE_VLAN, m->m_pkthdr.ether_vtag);
+ if (m == NULL)
+ return;
+ }
+#endif
+
#if NBPFILTER > 0
if (sc->sc_if.if_bpf)
bpf_mtap_ether(sc->sc_if.if_bpf, m, BPF_DIRECTION_IN);
Index: netinet/ip_carp.c
===
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.289
diff -u -p -r1.289 ip_carp.c
--- netinet/ip_carp.c 18 May 2016 03:46:03 - 1.289
+++ netinet/ip_carp.c 23 May 2016 13:14:48 -
@@ -80,6 +80,10 @@
#include
#endif
+#if NVLAN > 0
+#include
+#endif
+
#include
struct carp_mc_entry {
@@ -1398,6 +1402,15 @@ carp_input(struct ifnet *ifp0, struct mb
struct carp_if *cif;
struct carp_softc *sc;
struct srp_ref sr;
+
+#if NVLAN > 0
+ /*
+* If the underlying interface removed the VLAN header itself,
+* it's not for us.
+*/
+ if (ISSET(m->m_flags, M_VLANTAG))
+ return (0);
+#endif
eh = mtod(m, struct ether_header *);
cif = (struct carp_if *)cookie;
nd6_output() and NULL routes
This is the companion diff to the ether_output() one. Since bluhm@
mentioned it, here it is!
The idea is, once again, to stop inserting cloned route entries in L2
functions. The only place that insert cloned route entries should be
inside rtalloc(9) when RT_RESOLVE is passed.
Index: net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.971
diff -u -p -r1.971 pf.c
--- net/pf.c23 May 2016 12:26:28 - 1.971
+++ net/pf.c23 May 2016 12:39:54 -
@@ -5506,6 +5506,7 @@ pf_route(struct mbuf **m, struct pf_rule
int error = 0;
unsigned int rtableid;
+
if (m == NULL || *m == NULL || r == NULL ||
(dir != PF_IN && dir != PF_OUT) || oifp == NULL)
panic("pf_route: invalid parameters");
@@ -5665,11 +5666,13 @@ pf_route6(struct mbuf **m, struct pf_rul
{
struct mbuf *m0;
struct sockaddr_in6 *dst, sin6;
+ struct rtentry *rt = NULL;
struct ip6_hdr *ip6;
struct ifnet*ifp = NULL;
struct pf_addr naddr;
struct pf_src_node *sns[PF_SN_MAX];
struct m_tag*mtag;
+ unsigned int rtableid;
if (m == NULL || *m == NULL || r == NULL ||
(dir != PF_IN && dir != PF_OUT) || oifp == NULL)
@@ -5702,6 +5705,7 @@ pf_route6(struct mbuf **m, struct pf_rul
dst->sin6_family = AF_INET6;
dst->sin6_len = sizeof(*dst);
dst->sin6_addr = ip6->ip6_dst;
+ rtableid = m0->m_pkthdr.ph_rtableid;
if (!r->rt) {
m0->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
@@ -5754,7 +5758,13 @@ pf_route6(struct mbuf **m, struct pf_rul
if ((mtag = m_tag_find(m0, PACKET_TAG_PF_REASSEMBLED, NULL))) {
(void) pf_refragment6(&m0, mtag, dst, ifp);
} else if ((u_long)m0->m_pkthdr.len <= ifp->if_mtu) {
- nd6_output(ifp, m0, dst, NULL);
+ rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
+ if (rt == NULL) {
+ ip6stat.ip6s_noroute++;
+ goto bad;
+ }
+ nd6_output(ifp, m0, dst, rt);
+ rtfree(rt);
} else {
icmp6_error(m0, ICMP6_PACKET_TOO_BIG, 0, ifp->if_mtu);
}
Index: net/pf_norm.c
===
RCS file: /cvs/src/sys/net/pf_norm.c,v
retrieving revision 1.183
diff -u -p -r1.183 pf_norm.c
--- net/pf_norm.c 24 Nov 2015 13:37:16 - 1.183
+++ net/pf_norm.c 23 May 2016 12:39:55 -
@@ -750,7 +750,15 @@ pf_refragment6(struct mbuf **m0, struct
if (ifp == NULL) {
ip6_forward(m, 0);
} else if ((u_long)m->m_pkthdr.len <= ifp->if_mtu) {
- nd6_output(ifp, m, dst, NULL);
+ struct rtentry *rt;
+ rt = rtalloc(sin6tosa(dst), RT_RESOLVE,
+ m->m_pkthdr.ph_rtableid);
+ if (rt == NULL) {
+ ip6stat.ip6s_noroute++;
+ } else {
+ nd6_output(ifp, m, dst, rt);
+ rtfree(rt);
+ }
} else {
icmp6_error(m, ICMP6_PACKET_TOO_BIG, 0,
ifp->if_mtu);
Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.179
diff -u -p -r1.179 nd6.c
--- netinet6/nd6.c 17 May 2016 08:29:14 - 1.179
+++ netinet6/nd6.c 23 May 2016 12:39:55 -
@@ -1507,20 +1507,15 @@ nd6_output(struct ifnet *ifp, struct mbu
struct mbuf *m = m0;
struct rtentry *rt = rt0;
struct llinfo_nd6 *ln = NULL;
- int created = 0, error = 0;
+ int error = 0;
if (IN6_IS_ADDR_MULTICAST(&dst->sin6_addr))
goto sendpkt;
- /*
-* next hop determination.
-*/
- if (rt0 != NULL) {
- error = rt_checkgate(rt0, &rt);
- if (error) {
- m_freem(m);
- return (error);
- }
+ error = rt_checkgate(rt0, &rt);
+ if (error) {
+ m_freem(m);
+ return (error);
}
if (nd6_need_cache(ifp) == 0)
@@ -1532,42 +1527,17 @@ nd6_output(struct ifnet *ifp, struct mbu
* At this point, the destination of the packet must be a unicast
* or an anycast address(i.e. not a multicast).
*/
-
- /* Look up the neighbor cache for the nexthop */
- if (rt != NULL && (rt->rt_flags & RT
Unused USB ioctl(2)s
No point in keeping these. ok?
Index: sys/dev/usb/ugen.c
===
RCS file: /cvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.93
diff -u -p -r1.93 ugen.c
--- sys/dev/usb/ugen.c 17 Mar 2016 21:36:48 - 1.93
+++ sys/dev/usb/ugen.c 23 May 2016 11:41:32 -
@@ -953,7 +953,6 @@ ugen_do_ioctl(struct ugen_softc *sc, int
struct usb_endpoint_desc *ed;
usb_endpoint_descriptor_t *edesc;
struct usb_alt_interface *ai;
- struct usb_string_desc *si;
u_int8_t conf, alt;
DPRINTFN(5, ("ugenioctl: cmd=%08lx\n", cmd));
@@ -1130,16 +1129,6 @@ ugen_do_ioctl(struct ugen_softc *sc, int
error = uiomove((void *)cdesc, len, &uio);
free(cdesc, M_TEMP, 0);
return (error);
- }
- case USB_GET_STRING_DESC:
- {
- int len;
- si = (struct usb_string_desc *)addr;
- err = usbd_get_string_desc(sc->sc_udev, si->usd_string_index,
- si->usd_language_id, &si->usd_desc, &len);
- if (err)
- return (EINVAL);
- break;
}
case USB_DO_REQUEST:
{
Index: sys/dev/usb/uhid.c
===
RCS file: /cvs/src/sys/dev/usb/uhid.c,v
retrieving revision 1.65
diff -u -p -r1.65 uhid.c
--- sys/dev/usb/uhid.c 3 Mar 2016 18:13:24 - 1.65
+++ sys/dev/usb/uhid.c 23 May 2016 11:41:20 -
@@ -357,8 +357,7 @@ int
uhid_do_ioctl(struct uhid_softc *sc, u_long cmd, caddr_t addr,
int flag, struct proc *p)
{
- usbd_status err;
- int rc, size;
+ int rc;
DPRINTFN(2, ("uhidioctl: cmd=%lx\n", cmd));
@@ -392,17 +391,6 @@ uhid_do_ioctl(struct uhid_softc *sc, u_l
usbd_fill_deviceinfo(sc->sc_hdev.sc_udev,
(struct usb_device_info *)addr, 1);
break;
-
-case USB_GET_STRING_DESC:
- {
- struct usb_string_desc *si = (struct usb_string_desc *)addr;
- err = usbd_get_string_desc(sc->sc_hdev.sc_udev,
- si->usd_string_index,
- si->usd_language_id, &si->usd_desc, &size);
- if (err)
- return (EINVAL);
- break;
- }
case USB_GET_REPORT_DESC:
case USB_GET_REPORT:
Index: sys/dev/usb/umodem.c
===
RCS file: /cvs/src/sys/dev/usb/umodem.c,v
retrieving revision 1.59
diff -u -p -r1.59 umodem.c
--- sys/dev/usb/umodem.c14 Mar 2015 03:38:50 - 1.59
+++ sys/dev/usb/umodem.c23 May 2016 11:40:41 -
@@ -47,7 +47,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -121,7 +120,6 @@ voidumodem_rts(struct umodem_softc *, i
void umodem_break(struct umodem_softc *, int);
void umodem_set_line_state(struct umodem_softc *);
intumodem_param(void *, int, struct termios *);
-intumodem_ioctl(void *, int, u_long, caddr_t, int, struct proc *);
intumodem_open(void *, int portno);
void umodem_close(void *, int portno);
void umodem_intr(struct usbd_xfer *, void *, usbd_status);
@@ -130,7 +128,7 @@ struct ucom_methods umodem_methods = {
umodem_get_status,
umodem_set,
umodem_param,
- umodem_ioctl,
+ NULL,
umodem_open,
umodem_close,
NULL,
@@ -562,38 +560,6 @@ umodem_param(void *addr, int portno, str
return (1);
}
return (0);
-}
-
-int
-umodem_ioctl(void *addr, int portno, u_long cmd, caddr_t data, int flag,
-struct proc *p)
-{
- struct umodem_softc *sc = addr;
- int error = 0;
-
- if (usbd_is_dying(sc->sc_udev))
- return (EIO);
-
- DPRINTF(("umodem_ioctl: cmd=0x%08lx\n", cmd));
-
- switch (cmd) {
- case USB_GET_CM_OVER_DATA:
- *(int *)data = sc->sc_cm_over_data;
- break;
-
- case USB_SET_CM_OVER_DATA:
- if (*(int *)data != sc->sc_cm_over_data) {
- /* XXX change it */
- }
- break;
-
- default:
- DPRINTF(("umodem_ioctl: unknown\n"));
- error = ENOTTY;
- break;
- }
-
- return (error);
}
void
Index: sys/dev/usb/uplcom.c
===
RCS file: /cvs/src/sys/dev/usb/uplcom.c,v
retrieving revision 1.66
diff -u -p -r1.66 uplcom.c
--- sys/dev/usb/uplcom.c14 Mar 2015 03:38:50 - 1.66
+++ sys/dev/usb/uplcom.c23 May 2016 11:39:55 -
@@ -119,9 +119,6 @@ void uplcom_rts(struct uplcom_softc *, i
void uplcom_break(struct uplcom_softc *, int);
void uplcom_set_line_state(struct uplcom_softc *);
void uplcom_get_status(void *, int portno, u_char
Re: Enforce ether_output() requirements
On 23/05/16(Mon) 13:52, Alexander Bluhm wrote:
> On Wed, May 18, 2016 at 10:56:34PM +0200, Martin Pieuchot wrote:
> > Diff below convert the last offender explicitly passing NULL to an
> > if_output() function: pf_route().
>
> There is another one in phyint_send6() at netinet6/ip6_mroute.c:
> /*
> * We just call if_output instead of nd6_output here, since
> * we need no ND for a multicast forwarded packet...right?
> */
> error = ifp->if_output(ifp, mb_copy, sin6tosa(&ro.ro_dst),
> NULL);
This is ok because the packet has M_MCAST set.
> I wonder wether pf_route6() should pass the route to nd6_output()
> for consistency. But pf_refragment6() also uses NULL, and I guess
> IPv6 has much more issues.
This is my next diff.
> > I'm interested in tests but keep in mind that it is highly probable
> > that some code paths still path a NULL pointer as last argument, if
> > rtalloc(9) fails and its return is not checked. That's why I added
> > a KASSERT(), to fix these cases.
>
> I have tested your route-to code with /usr/src/regress/sys/net/pf_forward.
> Some tests fail, it seems that we have issues with path mtu discovery
> in current. But it is not related to this change. So OK bluhm@
> for the pf_route() part.
Thanks.
>
> >
> > Index: net/if_ethersubr.c
> > ===
> > RCS file: /cvs/src/sys/net/if_ethersubr.c,v
> > retrieving revision 1.236
> > diff -u -p -r1.236 if_ethersubr.c
> > --- net/if_ethersubr.c 18 May 2016 20:15:14 - 1.236
> > +++ net/if_ethersubr.c 18 May 2016 20:45:47 -
> > @@ -193,8 +193,12 @@ ether_output(struct ifnet *ifp, struct m
> > struct mbuf *mcopy = NULL;
> > struct ether_header *eh;
> > struct arpcom *ac = (struct arpcom *)ifp;
> > + sa_family_t af = dst->sa_family;
> > int error = 0;
> >
> > + KASSERT(rt != NULL || ISSET(m->m_flags, M_MCAST|M_BCAST) ||
> > + af == AF_UNSPEC || af == pseudo_AF_HDRCMPLT);
> > +
> > #ifdef DIAGNOSTIC
> > if (ifp->if_rdomain != rtable_l2(m->m_pkthdr.ph_rtableid)) {
> > printf("%s: trying to send packet on wrong domain. "
> > @@ -208,7 +212,7 @@ ether_output(struct ifnet *ifp, struct m
> > if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) != (IFF_UP|IFF_RUNNING))
> > senderr(ENETDOWN);
> >
> > - switch (dst->sa_family) {
> > + switch (af) {
> > case AF_INET:
> > error = arpresolve(ifp, rt, m, dst, edst);
> > if (error)
> > Index: net/pf.c
> > ===
> > RCS file: /cvs/src/sys/net/pf.c,v
> > retrieving revision 1.970
> > diff -u -p -r1.970 pf.c
> > --- net/pf.c3 May 2016 12:13:38 - 1.970
> > +++ net/pf.c18 May 2016 20:45:49 -
> > @@ -5574,6 +5574,12 @@ pf_route(struct mbuf **m, struct pf_rule
> > s->rt_addr.v4.s_addr;
> > ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
> > }
> > +
> > + rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
> > + if (rt == NULL) {
> > + ipstat.ips_noroute++;
> > + goto bad;
> > + }
> > }
> > if (ifp == NULL)
> > goto bad;
> > @@ -5602,7 +5608,7 @@ pf_route(struct mbuf **m, struct pf_rule
> > ipstat.ips_outswcsum++;
> > ip->ip_sum = in_cksum(m0, ip->ip_hl << 2);
> > }
> > - error = ifp->if_output(ifp, m0, sintosa(dst), NULL);
> > + error = ifp->if_output(ifp, m0, sintosa(dst), rt);
> > goto done;
> > }
> >
> > @@ -5631,7 +5637,7 @@ pf_route(struct mbuf **m, struct pf_rule
> > m1 = m0->m_nextpkt;
> > m0->m_nextpkt = 0;
> > if (error == 0)
> > - error = ifp->if_output(ifp, m0, sintosa(dst), NULL);
> > + error = ifp->if_output(ifp, m0, sintosa(dst), rt);
> > else
> > m_freem(m0);
> > }
> > Index: netinet/if_ether.c
> > ===
> > RCS file: /cvs/src/sys/netinet/if_ether.c,v
> > retrieving revision 1.207
> > diff -u -p -r1.207 if_ether.c
> > --- netinet/if_ether.c 18 May 2016 20:15:14 - 1.207
> > +++ netinet/if_ether.c 18 May 2016 20:45:50 -
> > @@ -294,40 +294,28 @@ arpresolve(struct ifnet *ifp, struct rte
> > return (0);
> > }
> >
> > - if (rt0 != NULL) {
> > - error = rt_checkgate(rt0, &rt);
> > - if (error) {
> > - m_freem(m);
> > - return (error);
> > - }
> > + error = rt_checkgate(rt0, &rt);
> > + if (error) {
> > + m_freem(m);
> > + return (error);
> > + }
> >
> > - if ((rt->rt_flags & RTF_LLINFO) == 0) {
> > -
Re: Enforce ether_output() requirements
On Wed, May 18, 2016 at 10:56:34PM +0200, Martin Pieuchot wrote:
> Diff below convert the last offender explicitly passing NULL to an
> if_output() function: pf_route().
There is another one in phyint_send6() at netinet6/ip6_mroute.c:
/*
* We just call if_output instead of nd6_output here, since
* we need no ND for a multicast forwarded packet...right?
*/
error = ifp->if_output(ifp, mb_copy, sin6tosa(&ro.ro_dst),
NULL);
I wonder wether pf_route6() should pass the route to nd6_output()
for consistency. But pf_refragment6() also uses NULL, and I guess
IPv6 has much more issues.
> I'm interested in tests but keep in mind that it is highly probable
> that some code paths still path a NULL pointer as last argument, if
> rtalloc(9) fails and its return is not checked. That's why I added
> a KASSERT(), to fix these cases.
I have tested your route-to code with /usr/src/regress/sys/net/pf_forward.
Some tests fail, it seems that we have issues with path mtu discovery
in current. But it is not related to this change. So OK bluhm@
for the pf_route() part.
>
> Index: net/if_ethersubr.c
> ===
> RCS file: /cvs/src/sys/net/if_ethersubr.c,v
> retrieving revision 1.236
> diff -u -p -r1.236 if_ethersubr.c
> --- net/if_ethersubr.c18 May 2016 20:15:14 - 1.236
> +++ net/if_ethersubr.c18 May 2016 20:45:47 -
> @@ -193,8 +193,12 @@ ether_output(struct ifnet *ifp, struct m
> struct mbuf *mcopy = NULL;
> struct ether_header *eh;
> struct arpcom *ac = (struct arpcom *)ifp;
> + sa_family_t af = dst->sa_family;
> int error = 0;
>
> + KASSERT(rt != NULL || ISSET(m->m_flags, M_MCAST|M_BCAST) ||
> + af == AF_UNSPEC || af == pseudo_AF_HDRCMPLT);
> +
> #ifdef DIAGNOSTIC
> if (ifp->if_rdomain != rtable_l2(m->m_pkthdr.ph_rtableid)) {
> printf("%s: trying to send packet on wrong domain. "
> @@ -208,7 +212,7 @@ ether_output(struct ifnet *ifp, struct m
> if ((ifp->if_flags & (IFF_UP|IFF_RUNNING)) != (IFF_UP|IFF_RUNNING))
> senderr(ENETDOWN);
>
> - switch (dst->sa_family) {
> + switch (af) {
> case AF_INET:
> error = arpresolve(ifp, rt, m, dst, edst);
> if (error)
> Index: net/pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.970
> diff -u -p -r1.970 pf.c
> --- net/pf.c 3 May 2016 12:13:38 - 1.970
> +++ net/pf.c 18 May 2016 20:45:49 -
> @@ -5574,6 +5574,12 @@ pf_route(struct mbuf **m, struct pf_rule
> s->rt_addr.v4.s_addr;
> ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
> }
> +
> + rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
> + if (rt == NULL) {
> + ipstat.ips_noroute++;
> + goto bad;
> + }
> }
> if (ifp == NULL)
> goto bad;
> @@ -5602,7 +5608,7 @@ pf_route(struct mbuf **m, struct pf_rule
> ipstat.ips_outswcsum++;
> ip->ip_sum = in_cksum(m0, ip->ip_hl << 2);
> }
> - error = ifp->if_output(ifp, m0, sintosa(dst), NULL);
> + error = ifp->if_output(ifp, m0, sintosa(dst), rt);
> goto done;
> }
>
> @@ -5631,7 +5637,7 @@ pf_route(struct mbuf **m, struct pf_rule
> m1 = m0->m_nextpkt;
> m0->m_nextpkt = 0;
> if (error == 0)
> - error = ifp->if_output(ifp, m0, sintosa(dst), NULL);
> + error = ifp->if_output(ifp, m0, sintosa(dst), rt);
> else
> m_freem(m0);
> }
> Index: netinet/if_ether.c
> ===
> RCS file: /cvs/src/sys/netinet/if_ether.c,v
> retrieving revision 1.207
> diff -u -p -r1.207 if_ether.c
> --- netinet/if_ether.c18 May 2016 20:15:14 - 1.207
> +++ netinet/if_ether.c18 May 2016 20:45:50 -
> @@ -294,40 +294,28 @@ arpresolve(struct ifnet *ifp, struct rte
> return (0);
> }
>
> - if (rt0 != NULL) {
> - error = rt_checkgate(rt0, &rt);
> - if (error) {
> - m_freem(m);
> - return (error);
> - }
> + error = rt_checkgate(rt0, &rt);
> + if (error) {
> + m_freem(m);
> + return (error);
> + }
>
> - if ((rt->rt_flags & RTF_LLINFO) == 0) {
> - log(LOG_DEBUG, "%s: %s: route contains no arp"
> - " information\n", __func__, inet_ntop(AF_INET,
> - &satosin(rt_key(rt))->sin_addr, addr,
> - sizeof(addr)
