Re: Set prio when bypassing pf(4)

[Martin Pieuchot]

On 08/06/16(Wed) 21:18, Vincent Gross wrote:
> On Wed, 8 Jun 2016 15:12:23 +0200
> Martin Pieuchot  wrote:
> 
> > On 07/06/16(Tue) 22:02, Stuart Henderson wrote:
> > > On 2016/06/07 21:49, Vincent Gross wrote:  
> > > > 
> > > > It's how henning@ set things up when integrating the new queuing
> > > > mechanism.
> > > > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/kern/uipc_mbuf.c#rev1.160
> > > >  
> > > > > Is there any use for this apart for vlan(4) interfaces?  
> > > > 
> > > > AFAICT, no.   
> > 
> > In this case I'd suggest to make this a vlan(4) specific
> > configuration, is there a problem with that?
> 
> Actually, there is. Consider this setup:
> 
> # ifconfig vlan4 vlan 4 vlandev em0 up
> # ifconfig vlan5 vlan 4 vlandev em1 up
> # ifconfig trunk0 trunkproto failover trunkport vlan4 trunport vlan5 up
> # ifconfig trunk0 10.10.10.50/24
> 
> llprio in vlan4 or vlan5 is useless because they are not initiating
> ARP requests, and adding lookups in trunk would be the Wrong Way.

Ok I think I got it with your example.  I was thinking of overwriting
the default priority in vlan_start() but as you said in your case since
vlan(4) are not initiating the ARP requests this would penalize all the
traffic.

I'm ok with your diff

Re: No need to second guess SXIE_ROUNDUP

[Artturi Alm]

On Thu, Jun 09, 2016 at 12:04:02AM +0100, Tom Cosgrove wrote:
> If (pktlen & 3) == 0, SXIE_ROUNDUP returns pktlen anyway (that's its job):
> it's defined as
> 
> #define SXIE_ROUNDUP(size, unit) (((size) + (unit) - 1) & ~((unit) - 1))
> 
> Thanks
> 
> Tom
> 

Hi,

starting w/bikeshed i'd go for removing rlen and just do pktlen+3, because
of >> 2, but seriously... does this possibly mean you think there was
something wrong with the diff i sent less than 24hrs ago that your diff does
nothing but stomp on? if so, do write your concerns out loud. i do care about
feedback. and in case you honestly were unaware of my diff, apologies, but i
really think leaving rlen and using SXIE_ROUNDUP at all is unnecessary.
and you missed unused variable in the other function using SXIE_ROUNDUP that
you certainly did check i hope, heh.

-Artturi

http://marc.info/?l=openbsd-tech=146537914916112=2

> 
> Index: sys/arch/armv7/sunxi/sxie.c
> ===
> RCS file: /home/OpenBSD/cvs/src/sys/arch/armv7/sunxi/sxie.c,v
> retrieving revision 1.14
> diff -u -p -u -r1.14 sxie.c
> --- sys/arch/armv7/sunxi/sxie.c   13 Apr 2016 11:34:00 -  1.14
> +++ sys/arch/armv7/sunxi/sxie.c   8 Jun 2016 23:03:46 -
> @@ -606,10 +606,7 @@ trynext:
>   m_adj(m, ETHER_ALIGN);
>  
>   /* read the actual packet from fifo XXX through 'align buffer'.. */
> - if (pktlen & 3)
> - rlen = SXIE_ROUNDUP(pktlen, 4);
> - else
> - rlen = pktlen;
> + rlen = SXIE_ROUNDUP(pktlen, 4);
>   bus_space_read_multi_4(sc->sc_iot, sc->sc_ioh,
>   SXIE_RXIO, (uint32_t *)[0], rlen >> 2);
>   memcpy(mtod(m, char *), (char *)[0], pktlen);
> 

Re: No need to second guess SXIE_ROUNDUP

[David Gwynne]

why not roundup() from src/sys/sys/param.h?

> On 9 Jun 2016, at 09:04, Tom Cosgrove  
> wrote:
> 
> If (pktlen & 3) == 0, SXIE_ROUNDUP returns pktlen anyway (that's its job):
> it's defined as
> 
>#define SXIE_ROUNDUP(size, unit) (((size) + (unit) - 1) & ~((unit) - 1))
> 
> Thanks
> 
> Tom
> 
> 
> Index: sys/arch/armv7/sunxi/sxie.c
> ===
> RCS file: /home/OpenBSD/cvs/src/sys/arch/armv7/sunxi/sxie.c,v
> retrieving revision 1.14
> diff -u -p -u -r1.14 sxie.c
> --- sys/arch/armv7/sunxi/sxie.c   13 Apr 2016 11:34:00 -  1.14
> +++ sys/arch/armv7/sunxi/sxie.c   8 Jun 2016 23:03:46 -
> @@ -606,10 +606,7 @@ trynext:
>   m_adj(m, ETHER_ALIGN);
> 
>   /* read the actual packet from fifo XXX through 'align buffer'.. */
> - if (pktlen & 3)
> - rlen = SXIE_ROUNDUP(pktlen, 4);
> - else
> - rlen = pktlen;
> + rlen = SXIE_ROUNDUP(pktlen, 4);
>   bus_space_read_multi_4(sc->sc_iot, sc->sc_ioh,
>   SXIE_RXIO, (uint32_t *)[0], rlen >> 2);
>   memcpy(mtod(m, char *), (char *)[0], pktlen);
> 

No need to second guess SXIE_ROUNDUP

[Tom Cosgrove]

If (pktlen & 3) == 0, SXIE_ROUNDUP returns pktlen anyway (that's its job):
it's defined as

#define SXIE_ROUNDUP(size, unit) (((size) + (unit) - 1) & ~((unit) - 1))

Thanks

Tom


Index: sys/arch/armv7/sunxi/sxie.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/armv7/sunxi/sxie.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 sxie.c
--- sys/arch/armv7/sunxi/sxie.c 13 Apr 2016 11:34:00 -  1.14
+++ sys/arch/armv7/sunxi/sxie.c 8 Jun 2016 23:03:46 -
@@ -606,10 +606,7 @@ trynext:
m_adj(m, ETHER_ALIGN);
 
/* read the actual packet from fifo XXX through 'align buffer'.. */
-   if (pktlen & 3)
-   rlen = SXIE_ROUNDUP(pktlen, 4);
-   else
-   rlen = pktlen;
+   rlen = SXIE_ROUNDUP(pktlen, 4);
bus_space_read_multi_4(sc->sc_iot, sc->sc_ioh,
SXIE_RXIO, (uint32_t *)[0], rlen >> 2);
memcpy(mtod(m, char *), (char *)[0], pktlen);

Re: sqlite3 update

[James Turner]

On Wed, Jun 08, 2016 at 11:33:42PM +0100, Stuart Henderson wrote:
> On 2016/06/03 20:17, James Turner wrote:
> > On Fri, Jun 03, 2016 at 11:24:15PM +0100, Stuart Henderson wrote:
> > > On 2016/06/01 11:22, Stuart Henderson wrote:
> > > > On 2016/06/01 09:09, Landry Breuil wrote:
> > > > > And most importantly, 47 (released next week) requires 3.11.
> > > > > /usr/obj/ports/firefox-47.0beta9/firefox-47.0b9/old-configure:SQLITE_VERSION=3.11.0
> > > > 
> > > > Ok, I'll update the diff this evening.
> > > 
> > > So this was a bit optimistic... 3.11 and 3.13 start making more
> > > changes to the Tcl scripts that replaced the awk scripts and it's
> > > beyond my awk to replicate them. (Also I'm not too sure about the
> > > XXX in Makefile about lempar.c which has changed upstream since
> > > the version where we added it).
> > > 
> > > As I see it there are two options to get us out of the hole.
> > > 
> > > - Pregenerate the files with Tcl and commit them, adapting the
> > > Makefile to use them. This is looking possible for 3.11 though
> > > still a bit of a pain to handle the build system hacks that
> > > will be needed. From a quick look at 3.13 there are more build
> > > system changes and further changes to the Tcl. I for one am
> > > not going to be able to maintain a forked build system with
> > > this many moving pieces.
> > > 
> > > - Switch to the "amalgamation" (4 source files: 3 for the lib,
> > > one for the shell), which is what upstream push people towards
> > > using (and what everybody else that I've seen including
> > > sqlite source uses).
> 
> 
> Does anyone have thoughts about these two as a "for now" option?
> 

I prefer option 2. Switch to the amalgamation with our changes on top.

> 
> > > With this the main file is unfortunately huge but would be easier
> > > by far to update later. Note that we only have a small change to
> > > the actual source code (replacing the RNG code) which is easy enough
> > > to carry across. Most of our changes are to the build infrastructure.
> > > 
> > > As far as Firefox goes, I think sqlite 3.11 should do us until
> > > release, but might be problematic if we want to update in -stable
> > > ports.
> > > 
> > > From what I can tell sqlite upstream *are* careful about ABI.
> > > But it would seem quite a stretch to expect things to work
> > > safely/reliably with two different versions brought into
> > > one address space.
> > > 
> > 
> > So when I talked with Ingo about removing SQLite from base we talked
> > about imported the "amalgamation" into mandoc for local use only and
> > then removing lib/libsqlite3 and usr.bin/sqlite3.
> > 
> > We would then need to updated sqlite3 in ports and update ports to use
> > this instead.
> 
> I think this does make sense. But it's used quite a lot in ports,
> handling a move back to ports is too much work to do before release.
> 

-- 
James Turner

Re: sqlite3 update

[Stuart Henderson]

On 2016/06/03 20:17, James Turner wrote:
> On Fri, Jun 03, 2016 at 11:24:15PM +0100, Stuart Henderson wrote:
> > On 2016/06/01 11:22, Stuart Henderson wrote:
> > > On 2016/06/01 09:09, Landry Breuil wrote:
> > > > And most importantly, 47 (released next week) requires 3.11.
> > > > /usr/obj/ports/firefox-47.0beta9/firefox-47.0b9/old-configure:SQLITE_VERSION=3.11.0
> > > 
> > > Ok, I'll update the diff this evening.
> > 
> > So this was a bit optimistic... 3.11 and 3.13 start making more
> > changes to the Tcl scripts that replaced the awk scripts and it's
> > beyond my awk to replicate them. (Also I'm not too sure about the
> > XXX in Makefile about lempar.c which has changed upstream since
> > the version where we added it).
> > 
> > As I see it there are two options to get us out of the hole.
> > 
> > - Pregenerate the files with Tcl and commit them, adapting the
> > Makefile to use them. This is looking possible for 3.11 though
> > still a bit of a pain to handle the build system hacks that
> > will be needed. From a quick look at 3.13 there are more build
> > system changes and further changes to the Tcl. I for one am
> > not going to be able to maintain a forked build system with
> > this many moving pieces.
> > 
> > - Switch to the "amalgamation" (4 source files: 3 for the lib,
> > one for the shell), which is what upstream push people towards
> > using (and what everybody else that I've seen including
> > sqlite source uses).


Does anyone have thoughts about these two as a "for now" option?


> > With this the main file is unfortunately huge but would be easier
> > by far to update later. Note that we only have a small change to
> > the actual source code (replacing the RNG code) which is easy enough
> > to carry across. Most of our changes are to the build infrastructure.
> > 
> > As far as Firefox goes, I think sqlite 3.11 should do us until
> > release, but might be problematic if we want to update in -stable
> > ports.
> > 
> > From what I can tell sqlite upstream *are* careful about ABI.
> > But it would seem quite a stretch to expect things to work
> > safely/reliably with two different versions brought into
> > one address space.
> > 
> 
> So when I talked with Ingo about removing SQLite from base we talked
> about imported the "amalgamation" into mandoc for local use only and
> then removing lib/libsqlite3 and usr.bin/sqlite3.
> 
> We would then need to updated sqlite3 in ports and update ports to use
> this instead.

I think this does make sense. But it's used quite a lot in ports,
handling a move back to ports is too much work to do before release.

Re: `rt_addr' or the end of `rt_ifa'

[Stuart Henderson]

On 2016/06/08 16:23, Martin Pieuchot wrote:
> Being able to remove the requirement of an configured address for every
> route entry would have multiple benefit:
> 
>   . We could add route before the interface gets an address (useful in
> some p2p configurations)
>   . The kernel wouldn't have to manage stale ifas
>   . The network data structures would be less coupled, making it easier
> to write SMP safe code (yes I found new bugs!)
> 
> In order to achieve such goal we should preserve the "source routing"
> feature.  Which mean that route entry MUST still carry an address.
> 
> So the diff below adds a `rt_addr' field that will for the moment be a
> copy of `rt_ifa->ifa_addr'.  Once all the `rt_ifa' occurrences are
> converted to `rt_addr' we can relax the logic for adding a route.

The diff looks correct to me (and hasn't blown up yet) and
I like the free() cleanup in rtfree().

rt_ifa is shared when multiple route table entries use the same
local address and all the route table entries are updated if the
local address changes (i.e. we don't just change the address
inside rt_ifa->ifa_addr), so this change of approach doesn't
really add extra work if a well-used local address is changed.
(I thought about this for a few minutes because we don't want
a bunch of unexpected extra work for full-table routers).

I checked that rt_addr gets updated correctly when we change
local address, this works fine.

OK sthen@.

Do you want to add 'db_printf(" addr="); db_print_sa(rt->rt_addr);'
to db_show_rtentry as well?

Re: Set prio when bypassing pf(4)

[Mike Belopuhov]

On Mon, Jun 06, 2016 at 23:52 +0200, Vincent Gross wrote:
> On Mon, 6 Jun 2016 17:33:36 +0100
> Stuart Henderson  wrote:
> 
> > On 2016/06/06 16:15, Vincent Gross wrote:
> > > When sending ARP requests, or when writing to a bpf handle (as when
> > > sending DHCP Discover), we bypass pf(4) so we have no way to define
> > > the priority (m->m_pkthdr.pf.prio) of the outgoing packets.
> [...]
> > > 
> > > This diff adds
> > > 1) an if_llprio field to struct ifnet  
> > 
> > struct if_data.. this is used by enough ports that changing the abi
> [...]
> > 
> > > diff --git a/sbin/ifconfig/ifconfig.8 b/sbin/ifconfig/ifconfig.8  
> > 
> > BTW. patch warns about offsets if you apply this to -current.
> > 
> [...]
> > 
> > Other than these points, it seems a useful thing to do, pppoe could
> > use it too.
> > 
> > I wonder what these broken ISP devices are that require the
> > priority field in the vlan frame header to be 0 (aka "prio 1")...
> > 
> 
> r2 below. I moved if_llprio from if_data to struct ifnet, and went from
> u_char to u_int8_t. I also added a bound check in ifioctl().
> 
> Comments ? ok ?
>

We have discussed this here with henning and benno and think that
this diff should go in.  OK mikeb

> Index: sbin/ifconfig/ifconfig.8
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.267
> diff -u -p -r1.267 ifconfig.8
> --- sbin/ifconfig/ifconfig.8  6 Apr 2016 10:07:14 -   1.267
> +++ sbin/ifconfig/ifconfig.8  6 Jun 2016 21:43:46 -
> @@ -327,6 +327,10 @@ Disable special processing at the link l
>  Change the link layer address (MAC address) of the interface.
>  This should be specified as six colon-separated hex values, or can
>  be chosen randomly.
> +.It Cm llprio Ar prio
> +Set the priority for link layer communications
> +.Pf ( Xr arp 4 ,
> +.Xr bpf 4 ) .
>  .It Cm media Op Ar type
>  Set the media type of the interface to
>  .Ar type .
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.322
> diff -u -p -r1.322 ifconfig.c
> --- sbin/ifconfig/ifconfig.c  3 May 2016 17:52:33 -   1.322
> +++ sbin/ifconfig/ifconfig.c  6 Jun 2016 21:43:46 -
> @@ -135,6 +135,7 @@ char  name[IFNAMSIZ];
>  int  flags, xflags, setaddr, setipdst, doalias;
>  u_long   metric, mtu;
>  int  rdomainid;
> +int  llprio;
>  int  clearaddr, s;
>  int  newaddr = 0;
>  int  af = AF_INET;
> @@ -157,6 +158,7 @@ void  addaf(const char *, int);
>  void removeaf(const char *, int);
>  void setifbroadaddr(const char *, int);
>  void setifmtu(const char *, int);
> +void setifllprio(const char *, int);
>  void setifnwid(const char *, int);
>  void setifbssid(const char *, int);
>  void setifnwkey(const char *, int);
> @@ -521,6 +523,7 @@ const struct  cmd {
>   { "instance",   NEXTARG,A_MEDIAINST,setmediainst },
>   { "inst",   NEXTARG,A_MEDIAINST,setmediainst },
>   { "lladdr", NEXTARG,0,  setiflladdr },
> + { "llprio", NEXTARG,0,  setifllprio },
>   { NULL, /*src*/ 0,  0,  setifaddr },
>   { NULL, /*dst*/ 0,  0,  setifdstaddr },
>   { NULL, /*illegal*/0,   0,  NULL },
> @@ -854,6 +857,11 @@ getinfo(struct ifreq *ifr, int create)
>   else
>   rdomainid = ifr->ifr_rdomainid;
>  #endif
> + if (ioctl(s, SIOCGIFLLPRIO, (caddr_t)ifr) < 0)
> + llprio = 0;
> + else
> + llprio = ifr->ifr_llprio;
> +
>   return (0);
>  }
>  
> @@ -1411,6 +1419,21 @@ setifmtu(const char *val, int d)
>  
>  /* ARGSUSED */
>  void
> +setifllprio(const char *val, int d)
> +{
> + const char *errmsg = NULL;
> +
> + (void) strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
> +
> + ifr.ifr_mtu = strtonum(val, 0, UCHAR_MAX, );
> + if (errmsg)
> + errx(1, "mtu %s: %s", val, errmsg);
> + if (ioctl(s, SIOCSIFLLPRIO, (caddr_t)) < 0)
> + warn("SIOCSIFLLPRIO");
> +}
> +
> +/* ARGSUSED */
> +void
>  setifgroup(const char *group_name, int dummy)
>  {
>   struct ifgroupreq ifgr;
> @@ -2894,6 +2917,7 @@ status(int link, struct sockaddr_dl *sdl
>   printf(" metric %lu", metric);
>   if (mtu)
>   printf(" mtu %lu", mtu);
> + printf(" llprio %lu", llprio);
>   putchar('\n');
>  #ifndef SMALL
>   if (showcapsflag)
> Index: sys/net/bpf.c
> ===
> RCS file: /cvs/src/sys/net/bpf.c,v
> retrieving revision 1.141
> diff -u -p -r1.141 bpf.c
> --- sys/net/bpf.c 18 May 2016 03:46:03 -  1.141
> +++ sys/net/bpf.c 6 Jun 2016 21:43:48 -
> @@ -561,6 +561,7 @@ bpfwrite(dev_t dev, struct uio *uio, int
>   }
>  
>   m->m_pkthdr.ph_rtableid = 

Re: Set prio when bypassing pf(4)

[Mike Belopuhov]

On Tue, Jun 07, 2016 at 22:02 +0100, Stuart Henderson wrote:
> On 2016/06/07 21:49, Vincent Gross wrote:
> > 
> > It's how henning@ set things up when integrating the new queuing mechanism.
> > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/kern/uipc_mbuf.c#rev1.160
> > 
> > > Is there any use for this apart for vlan(4) interfaces?
> > 
> > AFAICT, no. 
> 
> My understanding is that it is also meant to be used for interface
> output queues. So you could use this to prioritize ARP over IP
> traffic if you wanted. (Well..you could anyway, but you don't
> have as many options - many switches map the priorities 0-7
> onto 4 queues so there is only lower-priority than the default
> 'prio=3').
> 
> > > Should it
> > > really be part of "struct ifnet" ?
> > > 
> > 
> > sthen@ pointed out that struct if_data was heavily used by our ports, and
> > that such a change would require a version bump. Now, I may have overlooked
> > a better place for it.
> 
> I think it could go at the end of struct if_data without causing
> trouble, though since it doesn't need to be exported to userland
> isn't it better directly in ifnet rather than if_data? (if_data
> is included in RTM_IFINFO's if_msghdr, whereas you need an ioctl
> to see the non-if_data parts of struct ifnet from userland).
> 
> > I don't think we should make a special case for vlan(4), this kind of detail
> > do not belong to the arp(4) or bpf(4) layer.
> 
> btw, since this is a perfect fit for the vlan priority for pppoe
> control packets that I was looking at recently, here's a diff
> to use it there.
>

I think so too.  OK mikeb after vgross' diff goes in.

> Index: if_sppp.h
> ===
> RCS file: /cvs/src/sys/net/if_sppp.h,v
> retrieving revision 1.24
> diff -u -p -r1.24 if_sppp.h
> --- if_sppp.h 30 May 2016 23:30:11 -  1.24
> +++ if_sppp.h 7 Jun 2016 20:53:28 -
> @@ -56,7 +56,6 @@ enum ppp_phase {
>  
>  #define AUTHMAXLEN   256 /* including terminating '\0' */
>  #define AUTHCHALEN   16  /* length of the challenge we send */
> -#define SPPP_CTL_PRIO7   /* priority to use for control packets 
> */
>  
>  /*
>   * Definitions to pass struct sppp data down into the kernel using the
> Index: if_spppsubr.c
> ===
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.153
> diff -u -p -r1.153 if_spppsubr.c
> --- if_spppsubr.c 30 May 2016 23:30:10 -  1.153
> +++ if_spppsubr.c 7 Jun 2016 20:53:28 -
> @@ -914,7 +914,7 @@ sppp_cp_send(struct sppp *sp, u_short pr
>   return;
>   m->m_pkthdr.len = m->m_len = PKTHDRLEN + LCP_HEADER_LEN + len;
>   m->m_pkthdr.ph_ifidx = 0;
> - m->m_pkthdr.pf.prio = SPPP_CTL_PRIO;
> + m->m_pkthdr.pf.prio = sp->pp_if.if_llprio;
>  
>   *mtod(m, u_int16_t *) = htons(proto);
>   lh = (struct lcp_header *)(mtod(m, u_int8_t *) + 2);
> @@ -3992,7 +3992,7 @@ sppp_auth_send(const struct cp *cp, stru
>   if (! m)
>   return;
>   m->m_pkthdr.ph_ifidx = 0;
> - m->m_pkthdr.pf.prio = SPPP_CTL_PRIO;
> + m->m_pkthdr.pf.prio = sp->pp_if.if_llprio;
>  
>   *mtod(m, u_int16_t *) = htons(cp->proto);
>   lh = (struct lcp_header *)(mtod(m, u_int8_t *) + 2);
> 
> Index: if_pppoe.c
> ===
> RCS file: /cvs/src/sys/net/if_pppoe.c,v
> retrieving revision 1.56
> diff -u -p -r1.56 if_pppoe.c
> --- if_pppoe.c30 May 2016 23:30:11 -  1.56
> +++ if_pppoe.c7 Jun 2016 20:53:28 -
> @@ -163,7 +163,7 @@ static void pppoe_timeout(void *);
>  /* sending actual protocol control packets */
>  static int pppoe_send_padi(struct pppoe_softc *);
>  static int pppoe_send_padr(struct pppoe_softc *);
> -static int pppoe_send_padt(unsigned int, u_int, const u_int8_t *);
> +static int pppoe_send_padt(unsigned int, u_int, const u_int8_t *, u_int8_t);
>  
>  /* raw output */
>  static int pppoe_output(struct pppoe_softc *, struct mbuf *);
> @@ -696,7 +696,7 @@ pppoe_data_input(struct mbuf *m)
>  #ifdef PPPOE_TERM_UNKNOWN_SESSIONS
>   printf("pppoe (data): input for unknown session 0x%x, sending 
> PADT\n",
>   session);
> - pppoe_send_padt(m->m_pkthdr.ph_ifidx, session, shost);
> + pppoe_send_padt(m->m_pkthdr.ph_ifidx, session, shost, 0);
>  #endif
>   goto drop;
>   }
> @@ -1011,7 +1011,7 @@ pppoe_send_padi(struct pppoe_softc *sc)
>   m0 = pppoe_get_mbuf(len + PPPOE_HEADERLEN); /* header len + payload 
> len */
>   if (m0 == NULL)
>   return (ENOBUFS);
> - m0->m_pkthdr.pf.prio = SPPP_CTL_PRIO;
> + m0->m_pkthdr.pf.prio = sc->sc_sppp.pp_if.if_llprio;
>  
>   /* fill in pkt */
>   p = mtod(m0, u_int8_t *);
> @@ -1170,7 +1170,8 @@ pppoe_disconnect(struct pppoe_softc *sc)
>   PPPOEDEBUG(("%s: disconnecting\n",
> 

Uninitialised variable in sys/arch/armv7/exynos/crosec.c

[Tom Cosgrove]

Hi

I can't test this :) but it might bite someone who was trying to hack
in this area.

Thanks

Tom


Index: sys/arch/armv7/exynos/crosec.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/armv7/exynos/crosec.c,v
retrieving revision 1.1
diff -u -p -u -r1.1 crosec.c
--- sys/arch/armv7/exynos/crosec.c  26 Jan 2015 02:48:24 -  1.1
+++ sys/arch/armv7/exynos/crosec.c  8 Jun 2016 19:52:58 -
@@ -222,7 +222,7 @@ cros_ec_command_inptr(struct cros_ec_sof
int ret;
 
delay(5);
-   cros_ec_send_command(sc, EC_CMD_GET_COMMS_STATUS, 0,
+   ret = cros_ec_send_command(sc, EC_CMD_GET_COMMS_STATUS, 
0,
NULL, 0,
(uint8_t **), sizeof(*resp));
if (ret < 0)

Re: lockmgr() api removal

[Christian Weisgerber]

On 2016-06-08, Stuart Henderson  wrote:

> That's still testing server side for the contents of the ports tree,
> isn't it? Not as heavily stressed as putting it on the server would
> be, but it still gives it a bit of a workout.

I have now put the patch on the central server in the package
building network.  That will put the server side to the test.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: Set prio when bypassing pf(4)

[Vincent Gross]

On Wed, 8 Jun 2016 15:12:23 +0200
Martin Pieuchot  wrote:

> On 07/06/16(Tue) 22:02, Stuart Henderson wrote:
> > On 2016/06/07 21:49, Vincent Gross wrote:  
> > > 
> > > It's how henning@ set things up when integrating the new queuing
> > > mechanism.
> > > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/kern/uipc_mbuf.c#rev1.160
> > >  
> > > > Is there any use for this apart for vlan(4) interfaces?  
> > > 
> > > AFAICT, no.   
> 
> In this case I'd suggest to make this a vlan(4) specific
> configuration, is there a problem with that?

Actually, there is. Consider this setup:

# ifconfig vlan4 vlan 4 vlandev em0 up
# ifconfig vlan5 vlan 4 vlandev em1 up
# ifconfig trunk0 trunkproto failover trunkport vlan4 trunport vlan5 up
# ifconfig trunk0 10.10.10.50/24

llprio in vlan4 or vlan5 is useless because they are not initiating
ARP requests, and adding lookups in trunk would be the Wrong Way.

This particular exemple might seem far-fetched, but I'm sure there are
plenty worse actually deployed.

[...]

> > > I don't think we should make a special case for vlan(4), this
> > > kind of detail do not belong to the arp(4) or bpf(4) layer.
> 
> Which kind detail are you talking about?  I still don't understand how
> the scope if this problem is beyond vlan(4).  Are you also interested
> in queue priority?
> 

What I meant is arp(4) and bpf(4) and maybe other protocols should not
need to concern themselves about the nature of the device they are
transmitting on.

This problem manifests itself only on vlan(4) so far, because of the
CoS field in the 802.1Q header. But the problem is broader than that
actually, as some network protocols completely bypass pf(4), and it
can have consequences as we go down into the network device stacking.

Re: using srp inside art

[Sebastian Benoit]

Martin Pieuchot(m...@openbsd.org) on 2016.06.08 20:50:29 +0200:
> On 08/06/16(Wed) 19:51, Sebastian Benoit wrote:
> > [...] 
> > i dont see why this would be a problem
> > 
> > however:
> > 
> > +   ... if we were going to use
> > +* the last available route, but it got removed, we'll hit
> > +* the end of the list and then pick the first route.
> > 
> > would this make the first route get more traffic than the others,
> > statistically?
> 
> What do you mean by "statistically"?  The comment apply to route lookups
> done by a CPU while another CPU is modifying the multipath list.  This
> should be a completely negligible amount of lookups.

thanks for explaining that. i thougt this might happen more often.

> Now assuming that you are voluntarily generating a lot of multipath route
> changes to trigger this case, what you call "the first" route will most
> likely be a different route after every change.  So it should not matter ;)

Re: using srp inside art

[Martin Pieuchot]

On 08/06/16(Wed) 19:51, Sebastian Benoit wrote:
> [...] 
> i dont see why this would be a problem
> 
> however:
> 
> + ... if we were going to use
> +* the last available route, but it got removed, we'll hit
> +* the end of the list and then pick the first route.
> 
> would this make the first route get more traffic than the others,
> statistically?

What do you mean by "statistically"?  The comment apply to route lookups
done by a CPU while another CPU is modifying the multipath list.  This
should be a completely negligible amount of lookups.

Now assuming that you are voluntarily generating a lot of multipath route
changes to trigger this case, what you call "the first" route will most
likely be a different route after every change.  So it should not matter ;)

Re: using srp inside art

[Sebastian Benoit]

Jonathan Matthew(jonat...@d14n.org) on 2016.06.06 17:14:53 +1000:
> We've finally got srp and art to the point where we can use srp to manage the
> internal links in the art data structures.  This allows us to do route lookups
> without holding any locks, which is kind of nice.
> 
> As we're not doing unlocked insert, delete or walk (yet), those art functions
> use the locked variants of the srp functions.  Currently the lock that
> protects those operations is the kernel lock.
> 
> Callers to art_lookup and art_match now pass in a pointer to an srp_ref, which
> is always active on return, even if no route is found.  In some situations we
> use these functions while modifying the routing table, in which case the
> kernel lock already protects against concurrent modifications so the srp_ref
> is unnecessary, so we srp_leave it immediately.
> 
> I'd also like to draw attention to the comment in rtable_match.  Is it OK that
> we might choose a multipath route at random if the set of available routes
> changes while we're choosing?

i dont see why this would be a problem

however:

+   ... if we were going to use
+* the last available route, but it got removed, we'll hit
+* the end of the list and then pick the first route.

would this make the first route get more traffic than the others,
statistically?

Re: lockmgr() api removal

[Stuart Henderson]

On 2016/06/08 14:48, Christian Weisgerber wrote:
> On 2016-06-07, Theo de Raadt  wrote:
> 
> > Did I miss a report about nfs server and client?  I know I have not tested
> > this diff.
> 
> I put it on the amd64 package building machines and ran two bulk
> builds with it.  That qualifies as a successful client test.
> 
> I haven't gotten around yet to testing the server side.

Same on i386 (I think I'm on my 3rd or 4th build with it now).

That's still testing server side for the contents of the ports tree,
isn't it? Not as heavily stressed as putting it on the server would
be, but it still gives it a bit of a workout.

Re: lockmgr() api removal

[Christian Weisgerber]

On 2016-06-07, Theo de Raadt  wrote:

> Did I miss a report about nfs server and client?  I know I have not tested
> this diff.

I put it on the amd64 package building machines and ran two bulk
builds with it.  That qualifies as a successful client test.

I haven't gotten around yet to testing the server side.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

`rt_addr' or the end of `rt_ifa'

[Martin Pieuchot]

Being able to remove the requirement of an configured address for every
route entry would have multiple benefit:

  . We could add route before the interface gets an address (useful in
some p2p configurations)
  . The kernel wouldn't have to manage stale ifas
  . The network data structures would be less coupled, making it easier
to write SMP safe code (yes I found new bugs!)

In order to achieve such goal we should preserve the "source routing"
feature.  Which mean that route entry MUST still carry an address.

So the diff below adds a `rt_addr' field that will for the moment be a
copy of `rt_ifa->ifa_addr'.  Once all the `rt_ifa' occurrences are
converted to `rt_addr' we can relax the logic for adding a route.

This is the first of 14 steps.

Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.307
diff -u -p -r1.307 route.c
--- net/route.c 8 Jun 2016 13:26:06 -   1.307
+++ net/route.c 8 Jun 2016 14:19:48 -
@@ -139,6 +139,8 @@
 #include 
 #endif
 
+#define ROUNDUP(a) (a>0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : 
sizeof(long))
+
 /* Give some jitter to hash, to avoid synchronization between routers. */
 static uint32_trt_hashjitter;
 
@@ -151,6 +153,7 @@ struct pool rtentry_pool;   /* pool for r
 struct poolrttimer_pool;   /* pool for rttimer structures */
 
 void   rt_timer_init(void);
+intrt_setaddr(struct rtentry *, struct sockaddr *);
 intrtflushclone1(struct rtentry *, void *, u_int);
 void   rtflushclone(unsigned int, struct rtentry *);
 intrt_if_remove_rtdelete(struct rtentry *, void *, u_int);
@@ -434,7 +437,6 @@ rtref(struct rtentry *rt)
 void
 rtfree(struct rtentry *rt)
 {
-   struct ifaddr   *ifa;
int  refcnt;
 
if (rt == NULL)
@@ -452,16 +454,14 @@ rtfree(struct rtentry *rt)
 
KERNEL_LOCK();
rt_timer_remove_all(rt);
-   ifa = rt->rt_ifa;
-   if (ifa)
-   ifafree(ifa);
+   ifafree(rt->rt_ifa);
rtlabel_unref(rt->rt_labelid);
 #ifdef MPLS
if (rt->rt_flags & RTF_MPLS)
free(rt->rt_llinfo, M_TEMP, sizeof(struct rt_mpls));
 #endif
-   if (rt->rt_gateway)
-   free(rt->rt_gateway, M_RTABLE, 0);
+   free(rt->rt_addr, M_RTABLE, ROUNDUP(rt->rt_addr->sa_len));
+   free(rt->rt_gateway, M_RTABLE, ROUNDUP(rt->rt_gateway->sa_len));
free(rt_key(rt), M_RTABLE, rt_key(rt)->sa_len);
KERNEL_UNLOCK();
 
@@ -486,7 +486,7 @@ rt_sendmsg(struct rtentry *rt, int cmd, 
ifp = if_get(rt->rt_ifidx);
if (ifp != NULL) {
info.rti_info[RTAX_IFP] = sdltosa(ifp->if_sadl);
-   info.rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
+   info.rti_info[RTAX_IFA] = rt->rt_addr;
}
 
rt_missmsg(cmd, , rt->rt_flags, rt->rt_priority, rt->rt_ifidx, 0,
@@ -767,8 +767,6 @@ ifa_ifwithroute(int flags, struct sockad
return (ifa);
 }
 
-#define ROUNDUP(a) (a>0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : 
sizeof(long))
-
 int
 rt_getifa(struct rt_addrinfo *info, u_int rtid)
 {
@@ -958,7 +956,7 @@ rtrequest(int req, struct rt_addrinfo *i
 * will get the new address and interface later.
 */
info->rti_ifa = NULL;
-   info->rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr;
+   info->rti_info[RTAX_IFA] = rt->rt_addr;
}
 
info->rti_flags = rt->rt_flags | (RTF_CLONED|RTF_HOST);
@@ -1090,10 +1088,12 @@ rtrequest(int req, struct rt_addrinfo *i
 * the routing table because the radix MPATH code use
 * it to (re)order routes.
 */
-   if ((error = rt_setgate(rt, info->rti_info[RTAX_GATEWAY]))) {
+   if ((error = rt_setaddr(rt, ifa->ifa_addr)) ||
+   (error = rt_setgate(rt, info->rti_info[RTAX_GATEWAY]))) {
ifafree(ifa);
rtfree(rt->rt_parent);
rtfree(rt->rt_gwroute);
+   free(rt->rt_addr, M_RTABLE, 0);
free(rt->rt_gateway, M_RTABLE, 0);
free(ndst, M_RTABLE, dlen);
pool_put(_pool, rt);
@@ -1124,6 +1124,7 @@ rtrequest(int req, struct rt_addrinfo *i
ifafree(ifa);
rtfree(rt->rt_parent);
rtfree(rt->rt_gwroute);
+   free(rt->rt_addr, M_RTABLE, 0);
free(rt->rt_gateway, M_RTABLE, 0);
free(ndst, M_RTABLE, dlen);
pool_put(_pool, rt);
@@ -1145,6 +1146,24 @@ rtrequest(int req, struct rt_addrinfo *i
rtfree(rt);
   

Re: Set prio when bypassing pf(4)

[Martin Pieuchot]

On 07/06/16(Tue) 22:02, Stuart Henderson wrote:
> On 2016/06/07 21:49, Vincent Gross wrote:
> > 
> > It's how henning@ set things up when integrating the new queuing mechanism.
> > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/kern/uipc_mbuf.c#rev1.160
> > 
> > > Is there any use for this apart for vlan(4) interfaces?
> > 
> > AFAICT, no. 

In this case I'd suggest to make this a vlan(4) specific configuration,
is there a problem with that?

> My understanding is that it is also meant to be used for interface
> output queues. So you could use this to prioritize ARP over IP
> traffic if you wanted. (Well..you could anyway, but you don't
> have as many options - many switches map the priorities 0-7
> onto 4 queues so there is only lower-priority than the default
> 'prio=3').
> 
> > > Should it
> > > really be part of "struct ifnet" ?
> > > 
> > 
> > sthen@ pointed out that struct if_data was heavily used by our ports, and
> > that such a change would require a version bump. Now, I may have overlooked
> > a better place for it.
>
> I think it could go at the end of struct if_data without causing
> trouble, though since it doesn't need to be exported to userland
> isn't it better directly in ifnet rather than if_data? (if_data
> is included in RTM_IFINFO's if_msghdr, whereas you need an ioctl
> to see the non-if_data parts of struct ifnet from userland).

"struct ifnet" would be better.

> > I don't think we should make a special case for vlan(4), this kind of detail
> > do not belong to the arp(4) or bpf(4) layer.

Which kind detail are you talking about?  I still don't understand how
the scope if this problem is beyond vlan(4).  Are you also interested in
queue priority?

MBIM Patch (Round 3)

[Gerhard Roth]

Here comes the next version of the MBIM driver.

Changes since last version:

- incorporated suggestions from mpi@

- renamed to "umb"
Only file "mbim.h" which contains MBIM protocol related stuff
continues to use "mbim" as prefix.

- No longer takes fake addresses nor does it try to restore them


I would be glad to hear from some people trying this with a real MBIM
device.


Gerhard



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.88 Jun 2016 12:52:59 -
@@ -519,6 +519,8 @@ tunnel
 .Xr vxlan 4 )
 .It
 .Xr vlan 4
+.It
+.Xr umb 4
 .El
 .\" BRIDGE
 .Sh BRIDGE
@@ -1645,6 +1647,67 @@ 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
+.\" UMB
+.Sh UMB
+.nr nS 1
+.Bk -words
+.Nm ifconfig
+.Ar umb-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 umb 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 umb 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.
 .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.c8 Jun 2016 12:52:59 -
@@ -107,6 +107,10 @@
 #include 
 
 #include "brconfig.h"
+#ifndef SMALL
+#include 
+#include 
+#endif /* SMALL */
 
 #define MINIMUM(a, b)  (((a) < (b)) ? (a) : (b))
 #define MAXIMUM(a, b)  (((a) > (b)) ? (a) : (b))
@@ -145,6 +149,7 @@ int showmediaflag;
 intshowcapsflag;
 intshownet80211chans;
 intshownet80211nodes;
+intshowclasses;
 
 void   notealias(const char *, int);
 void   setifaddr(const char *, int);
@@ -275,6 +280,18 @@ void   unsetifdesc(const char *, int);
 void   printifhwfeatures(const char *, int);
 void   setpair(const char *, int);
 void   unsetpair(const char *, int);
+void   umb_status(void);
+void   umb_printclasses(char *, int);
+intumb_parse_classes(const char *);
+void   umb_setpin(const char *, int);
+void   umb_chgpin(const char *, const char *);
+void   umb_puk(const char *, const char *);
+void   umb_pinop(int, int, const char *, const char *);
+void   umb_apn(const char *, int);
+void   umb_setclass(const char *, int);
+void   umb_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,15 @@ const struct   cmd {
{ "-descr", 1,  0,  unsetifdesc },
{ "wol",IFXF_WOL,   0,  setifxflags },
{ "-wol",   -IFXF_WOL,  0,  setifxflags },
+   { "pin",NEXTARG,0,  umb_setpin },
+   { "chgpin", NEXTARG2,   0,  NULL, umb_chgpin },
+   { "puk",NEXTARG2,   0,  NULL, umb_puk },
+   { "apn",NEXTARG,0,  umb_apn },
+   { "-apn",   -1, 0,  umb_apn },
+   { "class",  NEXTARG0,   0,  umb_setclass },
+   { "-class", -1, 0,  umb_setclass },
+   { "roaming",1,  0,  

Re: MBIM Patch (Round 2)

[Martin Pieuchot]

On 08/06/16(Wed) 12:44, Gerhard Roth wrote:
> On Wed, 8 Jun 2016 11:31:41 +0100 Stuart Henderson  
> wrote:
> > On 2016/06/08 11:59, Gerhard Roth wrote:
> > > On Wed, 8 Jun 2016 10:54:00 +0100 Stuart Henderson  
> > > wrote:
> > > > On 2016/06/08 11:48, Gerhard Roth wrote:
> > > > > 
> > > > > Currently I do this to get the interface up and running as my default
> > > > > route:
> > > > > 
> > > > >   # ifconfig umb0 pin  apn 
> > > > >   # ifconfig umb0 inet 0.0.0.1 0.0.0.2
> > > > >   # route delete default
> > > > >   # route add -ifp umb0 default 0.0.0.2
> > > > >   # ifconfig umb0 up
> > > > > 
> > > > > Yes it seem strange that the 'route add -ifp ubm0' needs an IP 
> > > > > address,
> > > > > but that's the way it is. That's why I need the fake IP address on
> > > > > the interface right from the beginning. Otherwise the 'route add'
> > > > > would fail.
> > > > 
> > > > Ah yes this is a known strangeness - the address in the "route add"
> > > > can be anything at all though, it doesn't need to be configured on
> > > > the interface.
> > > 
> > > Well, to me it looks as if we need to put it on the interface:
> > > 
> > >   # route add -ifp umb0 default 0.0.0.2
> > >   route: writing to routing socket: Network is unreachable
> > >   add net default: gateway 0.0.0.2: Network is unreachable
> > >   # ifconfig ubm0 inet 0.0.0.1 0.0.0.2
> > >   # route add -ifp umb0 default 0.0.0.2
> > >   add net default: gateway 0.0.0.2
> > > 
> > 
> > hmm... I wonder if that check in route addition can be relaxed,
> > at least for the case where the -ifp interface is point-to-point.
> > 
> > I think you'll find that you *can* use an address other than
> > the one configured on the interface though.
> > 
> > $ ifconfig pppoe1   
> >
> > pppoe1: 
> > flags=48851 mtu 
> > 1500 llprio 3
> > index 18 priority 0
> > dev: em1 state: session
> > sid: 0x1378 PADI retries: 1 PADR retries: 0 time: 1d 12:05:38
> > sppp: phase network authproto chap 
> > groups: pppoe zen egress
> > status: active
> > inet6 fe80::20d:b9ff:fe41:7e48%pppoe1 ->  prefixlen 64 scopeid 0x12
> > inet6 2a02:8011:d00e:3:225:90ff:fec0:77b5 ->  prefixlen 64
> > inet 82.68.199.142 --> 62.3.84.27 netmask 0x
> > 
> > # route delete default  # make sure it's cleared:
> > route: writing to routing socket: No such process
> > delete net default: not in table
> > 
> > # route add default -ifp pppoe1 1.2.3.4
> > add net default: gateway 1.2.3.4
> 
> Ah, I can reproduce that: using an arbitrary IP address here succeeds if
> the interface is UP but fails if it is down.

Being UP or DOWN should not matter.  However the interface needs to have
an address.

That's a limitation I'd like to remove, this would also simplify most of
the code dealing with stale ifas.

Re: 'continue' to appease style gods in i386,amd64 libsa

[Tom Cosgrove]

Hi

> Two nits inline:

Thanks for the feedback.  Updated diff below.

Tom

>>>  8-Jun-16 11:52 >>>
>
> Hi Tom,
>
> Two nits inline:
>
> On Tue, Jun 7, 2016 at 9:47 PM, Tom Cosgrove 
>  wrote:
>
>  Tom Cosgrove 6-Jun-16 21:07 >>>
> >>
> >> As per subject, a couple of empty loop bodies in the i396 and amd64 boot 
> >> blocks.
> >>
> >> Diff below.
> >>
> >> Tom
> >
> > Subsequently found a few more, and a handful of trailing whitespaces.
> >
> > Updated diff below.


Index: sys/arch/amd64/stand/efiboot/efidev.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/efiboot/efidev.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 efidev.c
--- sys/arch/amd64/stand/efiboot/efidev.c   6 May 2016 03:13:52 -   
1.18
+++ sys/arch/amd64/stand/efiboot/efidev.c   8 Jun 2016 11:32:52 -
@@ -568,7 +568,8 @@ efiopen(struct open_file *f, ...)
}
 #endif
for (maj = 0; maj < nbdevs &&
-   strncmp(dev, bdevs[maj], devlen); maj++);
+   strncmp(dev, bdevs[maj], devlen); maj++)
+   continue;
if (maj >= nbdevs) {
printf("Unknown device: ");
for (cp = *file; *cp != ':'; cp++)
Index: sys/arch/amd64/stand/libsa/bioscons.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/bioscons.c,v
retrieving revision 1.11
diff -u -p -u -r1.11 bioscons.c
--- sys/arch/amd64/stand/libsa/bioscons.c   27 May 2016 05:37:51 -  
1.11
+++ sys/arch/amd64/stand/libsa/bioscons.c   8 Jun 2016 11:32:52 -
@@ -155,7 +155,7 @@ com_init(struct consdev *cn)
 
/* A few ms delay for the chip, using the getsecs() API */
while (!(i++ % 1000) && getsecs() < tt)
-   ;
+   continue;
 
/* drain the input buffer */
while (inb(port + com_lsr) & LSR_RXRDY)
@@ -171,7 +171,7 @@ com_getc(dev_t dev)
return (inb(port + com_lsr) & LSR_RXRDY);
 
while ((inb(port + com_lsr) & LSR_RXRDY) == 0)
-   ;
+   continue;
 
return (inb(port + com_data) & 0xff);
 }
@@ -237,7 +237,7 @@ com_putc(dev_t dev, int c)
int port = (com_addr == -1) ? comports[minor(dev)] : com_addr;
 
while ((inb(port + com_lsr) & LSR_TXRDY) == 0)
-   ;
+   continue;
 
outb(port + com_data, c);
 }
Index: sys/arch/amd64/stand/libsa/biosdev.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/biosdev.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 biosdev.c
--- sys/arch/amd64/stand/libsa/biosdev.c1 Oct 2015 20:28:12 -   
1.27
+++ sys/arch/amd64/stand/libsa/biosdev.c8 Jun 2016 11:32:52 -
@@ -560,7 +560,8 @@ biosopen(struct open_file *f, ...)
 #endif
 
for (maj = 0; maj < nbdevs &&
-   strncmp(dev, bdevs[maj], devlen); maj++);
+   strncmp(dev, bdevs[maj], devlen); maj++)
+   continue;
if (maj >= nbdevs) {
printf("Unknown device: ");
for (cp = *file; *cp != ':'; cp++)
@@ -671,7 +672,8 @@ biosdisk_err(u_int error)
register const u_char *p = bidos_errs;
 
while (*p && *p != error)
-   while (*p++);
+   while (*p++)
+   continue;
 
return ++p;
 }
@@ -703,7 +705,8 @@ biosdisk_errno(u_int error)
if (error == 0)
return 0;
 
-   for (p = tab; p->error && p->error != error; p++);
+   for (p = tab; p->error && p->error != error; p++)
+   continue;
 
return p->errno;
 }
Index: sys/arch/amd64/stand/libsa/diskprobe.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/diskprobe.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 diskprobe.c
--- sys/arch/amd64/stand/libsa/diskprobe.c  2 Sep 2015 01:52:26 -   
1.16
+++ sys/arch/amd64/stand/libsa/diskprobe.c  8 Jun 2016 11:32:52 -
@@ -266,7 +266,7 @@ diskprobe(void)
 
/* Checksumming of hard disks */
for (i = 0; disksum(i++) && i < MAX_CKSUMLEN; )
-   ;
+   continue;
bios_cksumlen = i;
 
/* Get space for passing bios_diskinfo stuff to kernel */
Index: sys/arch/amd64/stand/libsa/gateA20.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/gateA20.c,v
retrieving revision 1.2
diff -u -p -u -r1.2 gateA20.c
--- sys/arch/amd64/stand/libsa/gateA20.c21 Mar 2004 21:37:41 -  
1.2
+++ sys/arch/amd64/stand/libsa/gateA20.c8 Jun 2016 11:32:52 -
@@ -74,19 +74,24 @@ gateA20(int on)
}
} else {
 
-   while (inb(IO_KBD + KBSTATP) & KBS_IBF);
+  

Re: MBIM Patch (Round 2)

[Gerhard Roth]

On Wed, 8 Jun 2016 11:31:41 +0100 Stuart Henderson  wrote:
> On 2016/06/08 11:59, Gerhard Roth wrote:
> > On Wed, 8 Jun 2016 10:54:00 +0100 Stuart Henderson  
> > wrote:
> > > On 2016/06/08 11:48, Gerhard Roth wrote:
> > > > 
> > > > Currently I do this to get the interface up and running as my default
> > > > route:
> > > > 
> > > > # ifconfig umb0 pin  apn 
> > > > # ifconfig umb0 inet 0.0.0.1 0.0.0.2
> > > > # route delete default
> > > > # route add -ifp umb0 default 0.0.0.2
> > > > # ifconfig umb0 up
> > > > 
> > > > Yes it seem strange that the 'route add -ifp ubm0' needs an IP address,
> > > > but that's the way it is. That's why I need the fake IP address on
> > > > the interface right from the beginning. Otherwise the 'route add'
> > > > would fail.
> > > 
> > > Ah yes this is a known strangeness - the address in the "route add"
> > > can be anything at all though, it doesn't need to be configured on
> > > the interface.
> > 
> > Well, to me it looks as if we need to put it on the interface:
> > 
> > # route add -ifp umb0 default 0.0.0.2
> > route: writing to routing socket: Network is unreachable
> > add net default: gateway 0.0.0.2: Network is unreachable
> > # ifconfig ubm0 inet 0.0.0.1 0.0.0.2
> > # route add -ifp umb0 default 0.0.0.2
> > add net default: gateway 0.0.0.2
> > 
> 
> hmm... I wonder if that check in route addition can be relaxed,
> at least for the case where the -ifp interface is point-to-point.
> 
> I think you'll find that you *can* use an address other than
> the one configured on the interface though.
> 
> $ ifconfig pppoe1 
>  
> pppoe1: flags=48851 
> mtu 1500 llprio 3
> index 18 priority 0
> dev: em1 state: session
> sid: 0x1378 PADI retries: 1 PADR retries: 0 time: 1d 12:05:38
> sppp: phase network authproto chap 
> groups: pppoe zen egress
> status: active
> inet6 fe80::20d:b9ff:fe41:7e48%pppoe1 ->  prefixlen 64 scopeid 0x12
> inet6 2a02:8011:d00e:3:225:90ff:fec0:77b5 ->  prefixlen 64
> inet 82.68.199.142 --> 62.3.84.27 netmask 0x
> 
> # route delete default# make sure it's cleared:
> route: writing to routing socket: No such process
> delete net default: not in table
> 
> # route add default -ifp pppoe1 1.2.3.4
> add net default: gateway 1.2.3.4

Ah, I can reproduce that: using an arbitrary IP address here succeeds if
the interface is UP but fails if it is down.


> 
> # route -n get 8.8.8.8
>route to: 8.8.8.8
> destination: default
>mask: default
> gateway: 1.2.3.4
>   interface: pppoe1
>  if address: 82.68.199.142
>priority: 8 (static)
>   flags: 
>  use   mtuexpire
>  430 0 0 
> 
> # ping -c1 8.8.8.8
> PING 8.8.8.8 (8.8.8.8): 56 data bytes
> 64 bytes from 8.8.8.8: icmp_seq=0 ttl=58 time=14.027 ms
> --- 8.8.8.8 ping statistics ---
> 1 packets transmitted, 1 packets received, 0.0% packet loss
> round-trip min/avg/max/std-dev = 14.027/14.027/14.027/0.000 ms

Re: MBIM Patch (Round 2)

[Stuart Henderson]

On 2016/06/08 11:59, Gerhard Roth wrote:
> On Wed, 8 Jun 2016 10:54:00 +0100 Stuart Henderson  
> wrote:
> > On 2016/06/08 11:48, Gerhard Roth wrote:
> > > 
> > > Currently I do this to get the interface up and running as my default
> > > route:
> > > 
> > >   # ifconfig umb0 pin  apn 
> > >   # ifconfig umb0 inet 0.0.0.1 0.0.0.2
> > >   # route delete default
> > >   # route add -ifp umb0 default 0.0.0.2
> > >   # ifconfig umb0 up
> > > 
> > > Yes it seem strange that the 'route add -ifp ubm0' needs an IP address,
> > > but that's the way it is. That's why I need the fake IP address on
> > > the interface right from the beginning. Otherwise the 'route add'
> > > would fail.
> > 
> > Ah yes this is a known strangeness - the address in the "route add"
> > can be anything at all though, it doesn't need to be configured on
> > the interface.
> 
> Well, to me it looks as if we need to put it on the interface:
> 
>   # route add -ifp umb0 default 0.0.0.2
>   route: writing to routing socket: Network is unreachable
>   add net default: gateway 0.0.0.2: Network is unreachable
>   # ifconfig ubm0 inet 0.0.0.1 0.0.0.2
>   # route add -ifp umb0 default 0.0.0.2
>   add net default: gateway 0.0.0.2
> 

hmm... I wonder if that check in route addition can be relaxed,
at least for the case where the -ifp interface is point-to-point.

I think you'll find that you *can* use an address other than
the one configured on the interface though.

$ ifconfig pppoe1   
   
pppoe1: flags=48851 
mtu 1500 llprio 3
index 18 priority 0
dev: em1 state: session
sid: 0x1378 PADI retries: 1 PADR retries: 0 time: 1d 12:05:38
sppp: phase network authproto chap 
groups: pppoe zen egress
status: active
inet6 fe80::20d:b9ff:fe41:7e48%pppoe1 ->  prefixlen 64 scopeid 0x12
inet6 2a02:8011:d00e:3:225:90ff:fec0:77b5 ->  prefixlen 64
inet 82.68.199.142 --> 62.3.84.27 netmask 0x

# route delete default  # make sure it's cleared:
route: writing to routing socket: No such process
delete net default: not in table

# route add default -ifp pppoe1 1.2.3.4
add net default: gateway 1.2.3.4

# route -n get 8.8.8.8
   route to: 8.8.8.8
destination: default
   mask: default
gateway: 1.2.3.4
  interface: pppoe1
 if address: 82.68.199.142
   priority: 8 (static)
  flags: 
 use   mtuexpire
 430 0 0 

# ping -c1 8.8.8.8
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: icmp_seq=0 ttl=58 time=14.027 ms
--- 8.8.8.8 ping statistics ---
1 packets transmitted, 1 packets received, 0.0% packet loss
round-trip min/avg/max/std-dev = 14.027/14.027/14.027/0.000 ms

Re: MBIM Patch (Round 2)

[Stuart Henderson]

On 2016/06/08 11:48, Gerhard Roth wrote:
> 
> Currently I do this to get the interface up and running as my default
> route:
> 
>   # ifconfig umb0 pin  apn 
>   # ifconfig umb0 inet 0.0.0.1 0.0.0.2
>   # route delete default
>   # route add -ifp umb0 default 0.0.0.2
>   # ifconfig umb0 up
> 
> Yes it seem strange that the 'route add -ifp ubm0' needs an IP address,
> but that's the way it is. That's why I need the fake IP address on
> the interface right from the beginning. Otherwise the 'route add'
> would fail.

Ah yes this is a known strangeness - the address in the "route add"
can be anything at all though, it doesn't need to be configured on
the interface.

Re: MBIM Patch (Round 2)

[Gerhard Roth]

On Wed, 8 Jun 2016 10:54:00 +0100 Stuart Henderson  wrote:
> On 2016/06/08 11:48, Gerhard Roth wrote:
> > 
> > Currently I do this to get the interface up and running as my default
> > route:
> > 
> > # ifconfig umb0 pin  apn 
> > # ifconfig umb0 inet 0.0.0.1 0.0.0.2
> > # route delete default
> > # route add -ifp umb0 default 0.0.0.2
> > # ifconfig umb0 up
> > 
> > Yes it seem strange that the 'route add -ifp ubm0' needs an IP address,
> > but that's the way it is. That's why I need the fake IP address on
> > the interface right from the beginning. Otherwise the 'route add'
> > would fail.
> 
> Ah yes this is a known strangeness - the address in the "route add"
> can be anything at all though, it doesn't need to be configured on
> the interface.

Well, to me it looks as if we need to put it on the interface:

# route add -ifp umb0 default 0.0.0.2
route: writing to routing socket: Network is unreachable
add net default: gateway 0.0.0.2: Network is unreachable
# ifconfig ubm0 inet 0.0.0.1 0.0.0.2
# route add -ifp umb0 default 0.0.0.2
add net default: gateway 0.0.0.2

armv7 _AFLT

[Artturi Alm]

Hi,

i wish you would consider this:

diff --git a/sys/arch/arm/arm/cpufunc.c b/sys/arch/arm/arm/cpufunc.c
index 00c683e..924cf67 100644
--- a/sys/arch/arm/arm/cpufunc.c
+++ b/sys/arch/arm/arm/cpufunc.c
@@ -575,7 +575,6 @@ armv7_setup()
| CPU_CONTROL_AFE;
 
cpuctrl = CPU_CONTROL_MMU_ENABLE
-   | CPU_CONTROL_AFLT_ENABLE
| CPU_CONTROL_DC_ENABLE
| CPU_CONTROL_BPRD_ENABLE
| CPU_CONTROL_IC_ENABLE;


(fwiw. also FreeBSD did ^above recently.)
Here is example of embarrassing code it would allow fixing:

diff --git a/sys/arch/armv7/sunxi/sxie.c b/sys/arch/armv7/sunxi/sxie.c
index ff607c1..fea695c 100644
--- a/sys/arch/armv7/sunxi/sxie.c
+++ b/sys/arch/armv7/sunxi/sxie.c
@@ -146,8 +146,6 @@
 #define SXIE_MAX_RXD   8
 #define SXIE_MAX_PKT_SIZE  ETHER_MAX_DIX_LEN
 
-#define SXIE_ROUNDUP(size, unit) (((size) + (unit) - 1) & ~((unit) - 1))
-
 struct sxie_softc {
struct device   sc_dev;
struct arpcom   sc_ac;
@@ -454,10 +452,7 @@ sxie_start(struct ifnet *ifp)
 {
struct sxie_softc *sc = ifp->if_softc;
struct mbuf *m;
-   struct mbuf *head;
-   uint8_t *td;
uint32_t fifo;
-   uint32_t txbuf[SXIE_MAX_PKT_SIZE / sizeof(uint32_t)]; /* XXX !!! */
 
if (sc->txf_inuse > 1)
ifq_set_oactive(>if_snd);
@@ -465,9 +460,7 @@ sxie_start(struct ifnet *ifp)
if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(>if_snd))
return;
 
-   td = (uint8_t *)[0];
m = NULL;
-   head = NULL;
 trynext:
m = ifq_deq_begin(>if_snd);
if (m == NULL)
@@ -496,11 +489,10 @@ trynext:
/* set packet length */
SXIWRITE4(sc, SXIE_TXPKTLEN0 + (fifo * 4), m->m_pkthdr.len);
 
-   /* copy the actual packet to fifo XXX through 'align buffer'.. */
-   m_copydata(m, 0, m->m_pkthdr.len, (caddr_t)td);
+   /* write the actual packet to fifo */
bus_space_write_multi_4(sc->sc_iot, sc->sc_ioh,
SXIE_TXIO0 + (fifo * 4),
-   (uint32_t *)td, SXIE_ROUNDUP(m->m_pkthdr.len, 4) >> 2);
+   mtod(m, uint32_t *), (m->m_pkthdr.len + 3) >> 2);
 
/* transmit to PHY from fifo */
SXISET4(sc, SXIE_TXCR0 + (fifo * 4), 1);
@@ -564,8 +556,6 @@ sxie_recv(struct sxie_softc *sc)
struct mbuf *m;
uint16_t pktstat;
int16_t pktlen;
-   int rlen;
-   char rxbuf[SXIE_MAX_PKT_SIZE]; /* XXX !!! */
 trynext:
fbc = SXIREAD4(sc, SXIE_RXFBC);
if (!fbc)
@@ -596,6 +586,9 @@ trynext:
 
if (pktstat & SXIE_RX_ERRMASK || pktlen < ETHER_MIN_LEN) {
ifp->if_ierrors++;
+   /* drain here to implicitly avoid the flushing above */
+   while (SXIREAD4(sc, SXIE_RXFBC) > 0 && pktlen-- > 0)
+   reg = SXIREAD4(sc, SXIE_RXIO);  /* drain fifo */
goto trynext;
}
if (pktlen > SXIE_MAX_PKT_SIZE)
@@ -605,14 +598,9 @@ trynext:
/* XXX m->m_pkthdr.csum_flags ? */
m_adj(m, ETHER_ALIGN);
 
-   /* read the actual packet from fifo XXX through 'align buffer'.. */
-   if (pktlen & 3)
-   rlen = SXIE_ROUNDUP(pktlen, 4);
-   else
-   rlen = pktlen;
+   /* read the actual packet from fifo */
bus_space_read_multi_4(sc->sc_iot, sc->sc_ioh,
-   SXIE_RXIO, (uint32_t *)[0], rlen >> 2);
-   memcpy(mtod(m, char *), (char *)[0], pktlen);
+   SXIE_RXIO, mtod(m, uint32_t *), (pktlen + 3) >> 2);
 
ml_enqueue(, m);
goto trynext;



-Artturi

Re: MBIM Patch (Round 2)

[Stuart Henderson]

On 2016/06/08 09:54, Gerhard Roth wrote:
> On Tue, 7 Jun 2016 16:31:21 +0100 Stuart Henderson  
> 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.

Ah, I don't mean "no addresses assigned at all", I'm talking about
"deleting the old one first seems more racy" that you mentioned
in your mail.

I just mean it's ok to have no address temporarily - before you
connect to the network and have an assigned address, or when you're
regaining link or moving between networks and you remove the address
so you can add a new one.

Based on other interface types, how things work in non-OpenBSD routers,
etc, I'd expect it to work something like this:

interface attached ->
  - no address

ifconfig up (IFF_UP) ->
  - still no address (unless the user has manually set one)
  - attempt connection

connection succeeds or regain link ->
  - either: if no existing address, add it.
  -   or:   if address matches one already configured, keep it.
  -   or:   if address has changed, delete old address and add new.
  - set IFF_RUNNING

lose link ->
  - clear IFF_RUNNING (so no more packets will be sent over
   interface) but keep address.

ifconfig down (IFF_DOWN) ->
  - clear IFF_RUNNING and IFF_DOWN, keep address.


pppoe(4) is more complex because it has IFF_LINK1 aka IFF_AUTO
"dial-on-demand" behaviour which probably nobody uses, plus pppoe
is doing some things wrongly because I have observed cases where
it automatically moves between !IFF_UP and IFF_UP, this is
"administrative status" and meant to only be configured by the
admin, and also it sets IFF_RUNNING ("operational status") when
the link is not connected.

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

Yes, not when link is lost though, only once it has a new lease -
it maintains the old address until that point (with IFF_RUNNING
cleared, so you aren't sourcing traffic from it).

btw, I'm trying to get some hardware together to work on this,
I found a cheap enough card on ebay that looks like it should
work, now I just need to find pigtails and antennas that don't
cost more than the card (and hope I can find somewhere at home
with enough signal to test as I can't get that SIM registered
to my femtocell..)

Re: afterboot(8) ethernet device name

[Mark Kettenis]

> Date: Wed, 8 Jun 2016 11:10:56 +0200
> From: Stefan Sperling 
> 
> I find le0 and lo0 too close to be easily distinguishable, especially
> with a small font. This way, the difference should be more obvious.

ok kettenis@

> Index: afterboot.8
> ===
> RCS file: /cvs/src/share/man/man8/afterboot.8,v
> retrieving revision 1.153
> diff -u -p -r1.153 afterboot.8
> --- afterboot.8   8 Dec 2015 13:36:05 -   1.153
> +++ afterboot.8   8 Jun 2016 09:03:31 -
> @@ -160,7 +160,7 @@ Correct by editing
>  (where
>  .Ar interface
>  is the interface name, e.g.,
> -.Dq le0 )
> +.Dq em0 )
>  and then using
>  .Xr ifconfig 8
>  to manually configure it
> @@ -180,9 +180,9 @@ lo0: flags=8009 m
>  .Pp
>  an Ethernet interface something like:
>  .Bd -literal -offset indent
> -le0: flags=9863
> +em0: flags=9863
>   inet 192.168.4.52 netmask 0xff00 broadcast 192.168.4.255
> - inet6 fe80::5ef0:f0f0%le0 prefixlen 64 scopeid 0x1
> + inet6 fe80::5ef0:f0f0%em0 prefixlen 64 scopeid 0x1
>  .Ed
>  .Pp
>  and a PPP interface something like:
> @@ -208,12 +208,12 @@ Routing tables
>  
>  Internet:
>  DestinationGateway   Flags  Refs Use  Mtu  Interface
> -default192.168.4.254 UGS  0 11098028-  le0
> +default192.168.4.254 UGS  0 11098028-  em0
>  127127.0.0.1 UGRS 00-  lo0
>  127.0.0.1  127.0.0.1 UH   3   24-  lo0
> -192.168.4  link#1UC   00-  le0
> -192.168.4.52   8:0:20:73:b8:4a   UHL  1 6707-  le0
> -192.168.4.254  0:60:3e:99:67:ea  UHL  10-  le0
> +192.168.4  link#1UC   00-  em0
> +192.168.4.52   8:0:20:73:b8:4a   UHL  1 6707-  em0
> +192.168.4.254  0:60:3e:99:67:ea  UHL  10-  em0
>  
>  Internet6:
>  DestinationGateway   Flags  Refs  Use Mtu  Interface
> @@ -222,10 +222,10 @@ DestinationGateway   Flags  
>  :::0.0.0.0/96  ::1   UGRS 0 0   32972  lo0
>  fc80::/10  ::1   UGRS 0 0   32972  lo0
>  fe80::/10  ::1   UGRS 0 0   32972  lo0
> -fe80::%le0/64  link#1UC   0 01500  le0
> +fe80::%em0/64  link#1UC   0 01500  em0
>  fe80::%lo0/64  fe80::1%lo0   U0 0   32972  lo0
>  ff01::/32  ::1   U0 0   32972  lo0
> -ff02::%le0/32  link#1UC   0 01500  le0
> +ff02::%em0/32  link#1UC   0 01500  em0
>  ff02::%lo0/32  fe80::1%lo0   UC   0 0   32972  lo0
>  .Ed
>  .Pp
> 
> 

afterboot(8) ethernet device name

[Stefan Sperling]

I find le0 and lo0 too close to be easily distinguishable, especially
with a small font. This way, the difference should be more obvious.

Index: afterboot.8
===
RCS file: /cvs/src/share/man/man8/afterboot.8,v
retrieving revision 1.153
diff -u -p -r1.153 afterboot.8
--- afterboot.8 8 Dec 2015 13:36:05 -   1.153
+++ afterboot.8 8 Jun 2016 09:03:31 -
@@ -160,7 +160,7 @@ Correct by editing
 (where
 .Ar interface
 is the interface name, e.g.,
-.Dq le0 )
+.Dq em0 )
 and then using
 .Xr ifconfig 8
 to manually configure it
@@ -180,9 +180,9 @@ lo0: flags=8009 m
 .Pp
 an Ethernet interface something like:
 .Bd -literal -offset indent
-le0: flags=9863
+em0: flags=9863
inet 192.168.4.52 netmask 0xff00 broadcast 192.168.4.255
-   inet6 fe80::5ef0:f0f0%le0 prefixlen 64 scopeid 0x1
+   inet6 fe80::5ef0:f0f0%em0 prefixlen 64 scopeid 0x1
 .Ed
 .Pp
 and a PPP interface something like:
@@ -208,12 +208,12 @@ Routing tables
 
 Internet:
 DestinationGateway   Flags  Refs Use  Mtu  Interface
-default192.168.4.254 UGS  0 11098028-  le0
+default192.168.4.254 UGS  0 11098028-  em0
 127127.0.0.1 UGRS 00-  lo0
 127.0.0.1  127.0.0.1 UH   3   24-  lo0
-192.168.4  link#1UC   00-  le0
-192.168.4.52   8:0:20:73:b8:4a   UHL  1 6707-  le0
-192.168.4.254  0:60:3e:99:67:ea  UHL  10-  le0
+192.168.4  link#1UC   00-  em0
+192.168.4.52   8:0:20:73:b8:4a   UHL  1 6707-  em0
+192.168.4.254  0:60:3e:99:67:ea  UHL  10-  em0
 
 Internet6:
 DestinationGateway   Flags  Refs  Use Mtu  Interface
@@ -222,10 +222,10 @@ DestinationGateway   Flags  
 :::0.0.0.0/96  ::1   UGRS 0 0   32972  lo0
 fc80::/10  ::1   UGRS 0 0   32972  lo0
 fe80::/10  ::1   UGRS 0 0   32972  lo0
-fe80::%le0/64  link#1UC   0 01500  le0
+fe80::%em0/64  link#1UC   0 01500  em0
 fe80::%lo0/64  fe80::1%lo0   U0 0   32972  lo0
 ff01::/32  ::1   U0 0   32972  lo0
-ff02::%le0/32  link#1UC   0 01500  le0
+ff02::%em0/32  link#1UC   0 01500  em0
 ff02::%lo0/32  fe80::1%lo0   UC   0 0   32972  lo0
 .Ed
 .Pp

Re: MBIM Patch (Round 2)

[Gerhard Roth]

On Tue, 7 Jun 2016 16:31:21 +0100 Stuart Henderson  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: 'continue' to appease style gods in i386,amd64 libsa

[Tom Cosgrove]

>>> Tom Cosgrove 6-Jun-16 21:07 >>>
>
> As per subject, a couple of empty loop bodies in the i396 and amd64 boot 
> blocks.
>
> Diff below.
>
> Tom

Subsequently found a few more, and a handful of trailing whitespaces.

Updated diff below.

Thanks

Tom


Index: sys/arch/amd64/stand/efiboot/efidev.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/efiboot/efidev.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 efidev.c
--- sys/arch/amd64/stand/efiboot/efidev.c   6 May 2016 03:13:52 -   
1.18
+++ sys/arch/amd64/stand/efiboot/efidev.c   7 Jun 2016 20:40:16 -
@@ -568,7 +568,7 @@ efiopen(struct open_file *f, ...)
}
 #endif
for (maj = 0; maj < nbdevs &&
-   strncmp(dev, bdevs[maj], devlen); maj++);
+   strncmp(dev, bdevs[maj], devlen); maj++) continue;
if (maj >= nbdevs) {
printf("Unknown device: ");
for (cp = *file; *cp != ':'; cp++)
Index: sys/arch/amd64/stand/libsa/bioscons.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/bioscons.c,v
retrieving revision 1.11
diff -u -p -u -r1.11 bioscons.c
--- sys/arch/amd64/stand/libsa/bioscons.c   27 May 2016 05:37:51 -  
1.11
+++ sys/arch/amd64/stand/libsa/bioscons.c   7 Jun 2016 20:40:16 -
@@ -155,7 +155,7 @@ com_init(struct consdev *cn)
 
/* A few ms delay for the chip, using the getsecs() API */
while (!(i++ % 1000) && getsecs() < tt)
-   ;
+   continue;
 
/* drain the input buffer */
while (inb(port + com_lsr) & LSR_RXRDY)
@@ -171,7 +171,7 @@ com_getc(dev_t dev)
return (inb(port + com_lsr) & LSR_RXRDY);
 
while ((inb(port + com_lsr) & LSR_RXRDY) == 0)
-   ;
+   continue;
 
return (inb(port + com_data) & 0xff);
 }
@@ -237,7 +237,7 @@ com_putc(dev_t dev, int c)
int port = (com_addr == -1) ? comports[minor(dev)] : com_addr;
 
while ((inb(port + com_lsr) & LSR_TXRDY) == 0)
-   ;
+   continue;
 
outb(port + com_data, c);
 }
Index: sys/arch/amd64/stand/libsa/biosdev.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/biosdev.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 biosdev.c
--- sys/arch/amd64/stand/libsa/biosdev.c1 Oct 2015 20:28:12 -   
1.27
+++ sys/arch/amd64/stand/libsa/biosdev.c7 Jun 2016 20:40:16 -
@@ -560,7 +560,7 @@ biosopen(struct open_file *f, ...)
 #endif
 
for (maj = 0; maj < nbdevs &&
-   strncmp(dev, bdevs[maj], devlen); maj++);
+   strncmp(dev, bdevs[maj], devlen); maj++) continue;
if (maj >= nbdevs) {
printf("Unknown device: ");
for (cp = *file; *cp != ':'; cp++)
@@ -671,7 +671,8 @@ biosdisk_err(u_int error)
register const u_char *p = bidos_errs;
 
while (*p && *p != error)
-   while (*p++);
+   while (*p++)
+   continue;
 
return ++p;
 }
@@ -703,7 +704,8 @@ biosdisk_errno(u_int error)
if (error == 0)
return 0;
 
-   for (p = tab; p->error && p->error != error; p++);
+   for (p = tab; p->error && p->error != error; p++)
+   continue;
 
return p->errno;
 }
Index: sys/arch/amd64/stand/libsa/diskprobe.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/diskprobe.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 diskprobe.c
--- sys/arch/amd64/stand/libsa/diskprobe.c  2 Sep 2015 01:52:26 -   
1.16
+++ sys/arch/amd64/stand/libsa/diskprobe.c  7 Jun 2016 20:40:16 -
@@ -266,7 +266,7 @@ diskprobe(void)
 
/* Checksumming of hard disks */
for (i = 0; disksum(i++) && i < MAX_CKSUMLEN; )
-   ;
+   continue;
bios_cksumlen = i;
 
/* Get space for passing bios_diskinfo stuff to kernel */
Index: sys/arch/amd64/stand/libsa/gateA20.c
===
RCS file: /home/OpenBSD/cvs/src/sys/arch/amd64/stand/libsa/gateA20.c,v
retrieving revision 1.2
diff -u -p -u -r1.2 gateA20.c
--- sys/arch/amd64/stand/libsa/gateA20.c21 Mar 2004 21:37:41 -  
1.2
+++ sys/arch/amd64/stand/libsa/gateA20.c7 Jun 2016 20:40:16 -
@@ -74,19 +74,19 @@ gateA20(int on)
}
} else {
 
-   while (inb(IO_KBD + KBSTATP) & KBS_IBF);
+   while (inb(IO_KBD + KBSTATP) & KBS_IBF) continue;
 
while (inb(IO_KBD + KBSTATP) & KBS_DIB)
(void)inb(IO_KBD + KBDATAP);
 
outb(IO_KBD + KBCMDP, KBC_CMDWOUT);
-   while (inb(IO_KBD + KBSTATP) & KBS_IBF);
+   while