Re: avoid uninitialised attr in rasops_scrollback()
> Thanks, looks good/ok. Shouldn't the eraserows call in > rasops_doswitch() also use scr->rs_defattr for the attr argument? Yes, indeed. Good catch!
Re: 5GHz AP RSSI measurement problem
I believe you are correct to correlate RSSI & probe response. My understanding is that 802.11 beacon is a management frame to synchronize networks. Essentially the beacon interval determines the multiple for the DTIM & may include additional capability frames. The beacon blinks at a configurable interval (100 MS) & establishes a countdown for broadcast eligibility. In some implementations the reception of blinks may indicate congestion or interference & perhaps infer signal strength. Regards Patrick > On Apr 30, 2018, at 3:55 AM, Stefan Sperling wrote: > > I've ran into what seems to be a fairly modern dual-band AP (issued > by a French ISP). This AP camps on channel 112. This channel requires > radar detection which may explain the behaviour described below. > > The AP sends 5GHz beacons with a ridicously low RSSI while no client > is connected, and OpenBSD prefers the 2GHz band... > > + b8:ee:0e:cb:b3:081 +58 54M ess privacy rsn "ESSID" > + b8:ee:0e:cb:b3:09 112+6 54M ess privacy rsn "ESSID" > > ... unless we get lucky and the most recently measured RSSI is obtained > from a probe response instead of a beacon: > > + b8:ee:0e:cb:b3:081 +58 54M ess privacy rsn "ESSID" > + b8:ee:0e:cb:b3:09 112 +61 54M ess privacy rsn "ESSID" > > tcpdump confirms that beacons are received with a low RSSI of -10dBm > as long as no other client is connected (nevermind that the positive > dBm values shown here are bogus; that is a separate issue): > > 10:18:59.773885 802.11 flags=0<>: beacon, timestamp 974393344059, interval > 100, > caps=2421, ssid (ESSID), rates 6M* > 9M > 12M* 18M 24M* 36M 48M 54M, ds (chan 112), tim 0x0003, country 'FR ', > channel 36 limit 23dB, channel 40 limit 23dB, channel 44 limit 23dB, channel > 48 > limit 23dB, channel 52 limit 23dB, channel 56 limit 23dB, channel 60 limit > 23dB, channel 64 limit 23dB, channel 100 limit 23dB, channel 104 limit 23dB, > channel 108 limit 23dB, channel 112 limit 23dB, channel 116 limit 23dB, > channel > 132 limit 23dB, channel 136 limit 23dB, channel 140 limit 23dB, power > constraint 0dB, noise -127dBm> > > Whereas probe responses consistently arrive with much more promising > RSSI values of about -60dBm: > > 10:18:58.889338 802.11 flags=8: probe response, timestamp 974392459305, > interval 100, caps=2421, ssid > (ESSID), rates 6M* 9M 12M* 18M 24M* 36M 48M 54M, ds (chan 112), country 'FR ', > channel 36 limit 23dB, channel 40 limit 23dB, channel 44 limit 23dB, channel > 48 > limit 23dB, channel 52 limit 23dB, channel 56 limit 23dB, channel 60 limit > 23dB, channel 64 limit 23dB, channel 100 limit 23dB, channel 104 limit 23dB, > channel 108 limit 23dB, channel 112 limit 23dB, channel 116 limit 23dB, > channel > 132 limit 23dB, channel 136 limit 23dB, channel 140 limit 23dB, power > constraint 0dB, noise -127dBm> > > I have no idea if and where the 802.11 standard describes this behaviour. > Maybe there is a way to tell the real RSSI even from beacon frames? > Does anyone know more about this? > > Setting aside concerns about my lack of understanding of the underlying > reason for this behaviour, the hack below is sufficient to make this AP > show up as a strong contender in the candidate list and be preferred > over 2GHz as it should be. > > Should I commit this hack? I don't see any downsides. > > Index: ieee80211_input.c > === > RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v > retrieving revision 1.200 > diff -u -p -r1.200 ieee80211_input.c > --- ieee80211_input.c 29 Apr 2018 12:11:48 - 1.200 > +++ ieee80211_input.c 30 Apr 2018 08:32:58 - > @@ -1689,13 +1689,26 @@ ieee80211_recv_probe_resp(struct ieee802 > memcpy(ni->ni_essid, &ssid[2], ssid[1]); > } > IEEE80211_ADDR_COPY(ni->ni_bssid, wh->i_addr3); > - ni->ni_rssi = rxi->rxi_rssi; > + /* XXX validate channel # */ > + ni->ni_chan = &ic->ic_channels[chan]; > + if (ic->ic_state == IEEE80211_S_SCAN && > + IEEE80211_IS_CHAN_5GHZ(ni->ni_chan)) { > + /* > + * During a scan on 5Ghz, prefer RSSI measured for probe > + * response frames. i.e. don't allow beacons to lower the > + * measured RSSI. Some 5GHz APs send beacons with much > + * less Tx power than they use for probe responses. > + */ > + if (isprobe) > + ni->ni_rssi = rxi->rxi_rssi; > + else if (ni->ni_rssi < rxi->rxi_rssi) > + ni->ni_rssi = rxi->rxi_rssi; > + } else > + ni->ni_rssi = rxi->rxi_rssi; > ni->ni_rstamp = rxi->rxi_tstamp; > memcpy(ni->ni_tstamp, tstamp, sizeof(ni->ni_tstamp)); > ni->ni_intval = bintval; > ni->ni_capinfo = capinfo; > - /* XXX validate channel # */ > - ni->ni_chan = &ic->ic_channels[chan]; > ni->ni_erp = erp; > /* NB: must be after n
Re: avoid uninitialised attr in rasops_scrollback()
On Tue, May 01, 2018 at 05:03:26PM +, Miod Vallat wrote: > > scrollback isn't part of wsdisplay_emulops but rather wsdisplay_accessops. > > It isn't clear how to properly get an attr in wsscrollback() > > GETCHAR/getchar is part of wsmoused and sc->sc_accessops->getchar > > may be NULL. > > Ok, ignore that. There is a simpler solution. Thanks, looks good/ok. Shouldn't the eraserows call in rasops_doswitch() also use scr->rs_defattr for the attr argument? Index: rasops.c === RCS file: /cvs/src/sys/dev/rasops/rasops.c,v retrieving revision 1.53 diff -u -p -r1.53 rasops.c --- rasops.c27 Apr 2018 21:36:12 - 1.53 +++ rasops.c2 May 2018 01:53:38 - @@ -1373,6 +1373,7 @@ struct rasops_screen { int rs_visible; int rs_crow; int rs_ccol; + long rs_defattr; int rs_sbscreens; #define RS_SCROLLBACK_SCREENS 5 @@ -1412,10 +1413,11 @@ rasops_alloc_screen(void *v, void **cook scr->rs_visible = (ri->ri_nscreens == 0); scr->rs_crow = -1; scr->rs_ccol = -1; + scr->rs_defattr = *attrp; for (i = 0; i < scr->rs_dispoffset; i++) { scr->rs_bs[i].uc = ' '; - scr->rs_bs[i].attr = *attrp; + scr->rs_bs[i].attr = scr->rs_defattr; } if (ri->ri_bs && scr->rs_visible) { @@ -1425,7 +1427,8 @@ rasops_alloc_screen(void *v, void **cook } else { for (i = 0; i < ri->ri_rows * ri->ri_cols; i++) { scr->rs_bs[scr->rs_dispoffset + i].uc = ' '; - scr->rs_bs[scr->rs_dispoffset + i].attr = *attrp; + scr->rs_bs[scr->rs_dispoffset + i].attr = + scr->rs_defattr; } } @@ -1474,12 +1477,10 @@ rasops_doswitch(void *v) struct rasops_info *ri = v; struct rasops_screen *scr = ri->ri_switchcookie; int row, col; - long attr; rasops_cursor(ri, 0, 0, 0); ri->ri_active->rs_visible = 0; - ri->ri_alloc_attr(ri, 0, 0, 0, &attr); - ri->ri_eraserows(ri, 0, ri->ri_rows, attr); + ri->ri_eraserows(ri, 0, ri->ri_rows, scr->rs_defattr); ri->ri_active = scr; ri->ri_active->rs_visible = 1; ri->ri_active->rs_visibleoffset = ri->ri_active->rs_dispoffset; @@ -1906,7 +1907,6 @@ rasops_scrollback(void *v, void *cookie, struct rasops_info *ri = v; struct rasops_screen *scr = cookie; int row, col, oldvoff; - long attr; oldvoff = scr->rs_visibleoffset; @@ -1927,7 +1927,7 @@ rasops_scrollback(void *v, void *cookie, return; rasops_cursor(ri, 0, 0, 0); - ri->ri_eraserows(ri, 0, ri->ri_rows, attr); + ri->ri_eraserows(ri, 0, ri->ri_rows, scr->rs_defattr); for (row = 0; row < ri->ri_rows; row++) { for (col = 0; col < ri->ri_cols; col++) { int off = row * scr->rs_ri->ri_cols + col +
Re: in_ioctl(): use read lock for SIOCGIF*
On Wed, May 02, 2018 at 01:58:06AM +0200, Theo Buehler wrote: > Similarly to what we did for ifioctl() a few months back, we can factor > out the handling of reading ioctls into a new function, in_ioctl_get(). > Treat these cases before grabbing the NET_LOCK() in in_ioctl(). > > I only moved and copied a few pieces of code around and did not try to > make any simplifications to keep the idea of the refactoring clearly > visible. This results in a bit of code duplication, but it is not too > bad. > > We can think about further simplifications and introducing a helper > function or two in a later step. > > Index: sys/netinet/in.c > === > RCS file: /var/cvs/src/sys/netinet/in.c,v > retrieving revision 1.150 > diff -u -p -r1.150 in.c > --- sys/netinet/in.c 30 Apr 2018 19:07:44 - 1.150 > +++ sys/netinet/in.c 1 May 2018 23:02:04 - > @@ -84,6 +84,7 @@ > > void in_socktrim(struct sockaddr_in *); > > +int in_ioctl_get(u_long cmd, caddr_t data, struct ifnet *ifp); Bad habit from LibreSSL work... I will remove the variable names in the prototype before committing.
in_ioctl(): use read lock for SIOCGIF*
Similarly to what we did for ifioctl() a few months back, we can factor out the handling of reading ioctls into a new function, in_ioctl_get(). Treat these cases before grabbing the NET_LOCK() in in_ioctl(). I only moved and copied a few pieces of code around and did not try to make any simplifications to keep the idea of the refactoring clearly visible. This results in a bit of code duplication, but it is not too bad. We can think about further simplifications and introducing a helper function or two in a later step. Index: sys/netinet/in.c === RCS file: /var/cvs/src/sys/netinet/in.c,v retrieving revision 1.150 diff -u -p -r1.150 in.c --- sys/netinet/in.c30 Apr 2018 19:07:44 - 1.150 +++ sys/netinet/in.c1 May 2018 23:02:04 - @@ -84,6 +84,7 @@ void in_socktrim(struct sockaddr_in *); +int in_ioctl_get(u_long cmd, caddr_t data, struct ifnet *ifp); void in_purgeaddr(struct ifaddr *); int in_addhost(struct in_ifaddr *, struct sockaddr_in *); int in_scrubhost(struct in_ifaddr *, struct sockaddr_in *); @@ -220,6 +221,14 @@ in_ioctl(u_long cmd, caddr_t data, struc if (ifp == NULL) return (ENXIO); + switch (cmd) { + case SIOCGIFADDR: + case SIOCGIFNETMASK: + case SIOCGIFDSTADDR: + case SIOCGIFBRDADDR: + return in_ioctl_get(cmd, data, ifp); + } + NET_LOCK(); TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { @@ -230,7 +239,6 @@ in_ioctl(u_long cmd, caddr_t data, struc } switch (cmd) { - case SIOCAIFADDR: case SIOCDIFADDR: if (ifra->ifra_addr.sin_family == AF_INET) { @@ -279,12 +287,7 @@ in_ioctl(u_long cmd, caddr_t data, struc error = EPERM; goto err; } - /* FALLTHROUGH */ - case SIOCGIFADDR: - case SIOCGIFNETMASK: - case SIOCGIFDSTADDR: - case SIOCGIFBRDADDR: if (ia && satosin(&ifr->ifr_addr)->sin_addr.s_addr) { for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) { if ((ifa->ifa_addr->sa_family == AF_INET) && @@ -302,31 +305,6 @@ in_ioctl(u_long cmd, caddr_t data, struc break; } switch (cmd) { - - case SIOCGIFADDR: - *satosin(&ifr->ifr_addr) = ia->ia_addr; - break; - - case SIOCGIFBRDADDR: - if ((ifp->if_flags & IFF_BROADCAST) == 0) { - error = EINVAL; - break; - } - *satosin(&ifr->ifr_dstaddr) = ia->ia_broadaddr; - break; - - case SIOCGIFDSTADDR: - if ((ifp->if_flags & IFF_POINTOPOINT) == 0) { - error = EINVAL; - break; - } - *satosin(&ifr->ifr_dstaddr) = ia->ia_dstaddr; - break; - - case SIOCGIFNETMASK: - *satosin(&ifr->ifr_addr) = ia->ia_sockmask; - break; - case SIOCSIFDSTADDR: if ((ifp->if_flags & IFF_POINTOPOINT) == 0) { error = EINVAL; @@ -422,6 +400,73 @@ err: NET_UNLOCK(); return (error); } + +int +in_ioctl_get(u_long cmd, caddr_t data, struct ifnet *ifp) +{ + struct ifreq *ifr = (struct ifreq *)data; + struct ifaddr *ifa; + struct in_ifaddr *ia = NULL; + int error = 0; + + NET_RLOCK(); + + TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) { + if (ifa->ifa_addr->sa_family == AF_INET) { + ia = ifatoia(ifa); + break; + } + } + + if (ia && satosin(&ifr->ifr_addr)->sin_addr.s_addr) { + for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) { + if ((ifa->ifa_addr->sa_family == AF_INET) && + ifatoia(ifa)->ia_addr.sin_addr.s_addr == + satosin(&ifr->ifr_addr)->sin_addr.s_addr) { + ia = ifatoia(ifa); + break; + } + } + } + if (ia == NULL) { + error = EADDRNOTAVAIL; + goto err; + } + + switch(cmd) { + case SIOCGIFADDR: + *satosin(&ifr->ifr_addr) = ia->ia_addr; + break; + + case SIOCGIFBRDADDR: + if ((ifp->if_flags & IFF_BROADCAST) == 0) { + error = EINVAL; + break; + } + *satosin(&ifr->ifr_dstaddr) = ia->ia_broadaddr; + break; + + case SIOCGIFDSTADDR: + if ((ifp->if_flags & IFF_POINTOPOINT) == 0) { + error = EINVAL; + break; + } + *satosin(&ifr->ifr_dstaddr) = ia->
reduce hppa interrupt guts awfulness
[this is long, and hppa-specific, feel free to ignore if you don't care about OpenBSD/hppa. Noone does anyway nowadays] At the beginning of the hppa port, no interrupts were shared - the processor allows for 32 interrupt sources and the existing designs made sure to allocate distinct interrupts to every device. Every device, but the PS/2 keyboard and mouse controller. So mickey added a kluge about 15 years ago for them when I wrote the keyboard controller driver. [Yes, really 15 years ago now... time flies] And then the port grew PCI bus support, where interrupts have to be shareable, by design. So the interrupt code got some form of shared interrupts, but, well, the interrupt handling code is definitely not for the faint of heart. Yet it works... to some extent. This mostly involves magic, and the ability to understand locore0.s. Because most of the native hppa drivers have been designed with exclusive interrupts in mind, the hppa kernel configuration has many hardwired interrupt vector lines (`irq' locators). This is the reason why you have distinct foo0 and foo1 lines in the kernel configuration, even for devices which driver do not use interrupts at all, such as sti(4). While working on ``card-mode'' dino(4) support [coming soon], I ended up with having more than one dino(4) attachment to the same parent. To my surprise, both were using the same interrupt, which - in the current state of the interrupt handling code, which I won't touch without a ten-feet pole - meant interrupts from the first device were completely ignored, because the second one would happily overwrite all the relevant interrupt structures for that interrupt bit. This happens, because of this in mbsubmatch() in sys/arch/hppa/hppa/mainbus.c: int mbsubmatch(parent, match, aux) struct device *parent; void *match, *aux; { [..] if ((ret = (*cf->cf_attach->ca_match)(parent, match, aux))) ca->ca_irq = cf->hppacf_irq; return ret; } This means that "everytime a kernel configuration line matches, we pick its irq value". And while many devices have stricter checks based on their physical addresses, PCI bridges such as dino(4) don't. So, with a kernel configuration having dino0 at phantomas? irq 26# PCI bus bridge on [AB]* dino1 at phantomas? irq 25 both lines would match, and both dino controllers would end up using - but not sharing! - irq 25. This was not noticed because systems with two dino attachments AT THE SAME PARENT do not exist in their DEFAULT configuration. However, adding support for cardmode dino changes the rules. And one can end up with THREE dino controllers although the kernel only allows for dino0 and dino1. The only correct way to address this is to relax this fixed irq assignments and let the kernel pick unused interrupt vectors. In fact, this logic was already in use on higher-end elroy(4) systems. The following diff does: - dynamic allocation of interrupts for dino(4) and gsc(4). - only mention irq locators for devices which use interrupts. - for gsc(4), this actually postpones interrupt allocation until gsc(4) attaches, so asp(4)/lasi(4)/wax(4) won't route the interrupt yet, but the gsc(4) child will take care of this. - because of hardwired interrupt line assignment on the pre-PCI systems, the logic looking for a free interrupt pin looks from 31 to 0, while it used to search in the opposite order. This WILL affect all elroy systems (C3xxx, J) but shouldn't have any noticeable effect. Even though gsckbc will pick interrupt 26, systems with gsckbc will have the asp/lasi/wax controller attaching early enough to leave irq 26 available by the time gsckbc attacheѕ. - remove the irq locator for the drivers which do not need a fixed assignment. siop and moongoose could benefit from this but don't work at the moment so this is left as an exercize to future kernel hackers. A side effect from this is that we do not need multiple sti(4) attachment lines, so PCI sti(4) devices will now attach as sti0 rather than sti2. With this diff, multiple dino(4) devices attached to the same parent will use different interrupt sources, and more than two dino(4) devices can attach. Tested on 715/75 (old-gen using asp), 715/100/XC (new-gen using lasi), B132L+, B180L, C240, B2000, C3650. B2000 and C3650 test have been done with sys/dev/ic/com.c downgraded to 1.166 in order to avoid freezes (bug resolution ongoing and not related to this diff). 715 and B1xx tests have been done with sys/dev/pckbc/pckbd.c downgraded to 1.43 (bug resolution ongoing and not related to this diff). $ cat levandes.vari Index: conf/GENERIC === RCS file: /OpenBSD/src/sys/arch/hppa/conf/GENERIC,v retrieving revision 1.175 diff -u -p -r1.175 GENERIC --- conf/GENERIC14 Feb 2018 23:51:49 - 1.
Re: Push the netlock down in in_control()
On Tue, May 01, 2018 at 07:08:59PM +0200, Theo Buehler wrote: > I tested this as well as I could, but I don't usually use IPv6, so I > dabbled a bit with it on my home network and I tried to use the netinet6 > regress tests, only with moderate success. > > A test from people who actually use IPv6 would be welcome. Works fine for me so far on a X230 in an IPv6 only network with every day usage, package upgrades and backups over the wire.
Re: Push the netlock down in in_control()
On 01/05/18(Tue) 19:08, Theo Buehler wrote: > On Mon, Apr 30, 2018 at 02:55:21PM +0200, Martin Pieuchot wrote: > > On 30/04/18(Mon) 12:00, Theo Buehler wrote: > > > With mpi's encouragement and guidance, here's a diff that reduces the > > > scope of the NET_LOCK() a bit. > > > > > > in_control() is the only caller of mrt_ioctl() and the latter is a > > > simple function only requiring a read lock. > > > > > > There are only a handful callers of in_ioctl(). The two switches create > > > relatively tangled codepaths, so this will need some refactoring before > > > we can push the lock further down and split the read and write cases. > > > For now, establish a single exit point, grab the netlock at the > > > beginning and release it at the end. > > > > That makes sense, the same could be done for in6_ioctl(). > > Here's the corresponding diff for in6_control(). > > Push the diff down into mrt6_ioctl() and in6_ioctl(). Much like the IPv4 > version, mrt6_ioctl() only needs a read lock. > > in6_ioctl() is a bit messier. It starts off with a call to nd6_ioctl(), > which only needs a read lock. In the diff, I pushed the NET_RLOCK() down > into nd6_ioctl(), but perhaps it would be better to keep the locking > inside in6_ioctl() No! It's much better to push it down. We must push it down everywhere until it dies. Diff is ok mpi@
Re: libkvm requires kvm_getargv before kvm_getenv when both needed
Vadim Zhukov wrote: > 2018-05-01 21:53 GMT+03:00 Theo de Raadt : > > ktrace makes the problem more clear: > > > > 25908 ps CALL > > sysctl(1.55.75675.1,0xed0cc78,0x7f7cd3d8,0,0) > > 25908 ps RET sysctl -1 errno 14 Bad address > > And that's it, thanks! > > Now little ps(1) is happy. But now there's a question, about > kvm_getargv() and kvm_getenv(): what behaviour do we want? > > a) They use same space, overwriting each other results (this is what > happens now, and noone complains). > > b) Their working space should be independent of each other. This > isn't hard, just splitting kd->argbuf into kd->argbuf and > kd->envbuf. Seems a bit saner. > > I'm fine with any direction. The patch below implements (a), since > it's less patching. Is it okay, or should it be (b)? I think (b) would be the better solution, this seems very fragile. Todd and Guenther -- what do you think?
Re: Push the netlock down in in_control()
looks reasonable to me. OK On Tue, May 01, 2018 at 07:08:59PM +0200, Theo Buehler wrote: > On Mon, Apr 30, 2018 at 02:55:21PM +0200, Martin Pieuchot wrote: > > On 30/04/18(Mon) 12:00, Theo Buehler wrote: > > > With mpi's encouragement and guidance, here's a diff that reduces the > > > scope of the NET_LOCK() a bit. > > > > > > in_control() is the only caller of mrt_ioctl() and the latter is a > > > simple function only requiring a read lock. > > > > > > There are only a handful callers of in_ioctl(). The two switches create > > > relatively tangled codepaths, so this will need some refactoring before > > > we can push the lock further down and split the read and write cases. > > > For now, establish a single exit point, grab the netlock at the > > > beginning and release it at the end. > > > > That makes sense, the same could be done for in6_ioctl(). > > Here's the corresponding diff for in6_control(). > > Push the diff down into mrt6_ioctl() and in6_ioctl(). Much like the IPv4 > version, mrt6_ioctl() only needs a read lock. > > in6_ioctl() is a bit messier. It starts off with a call to nd6_ioctl(), > which only needs a read lock. In the diff, I pushed the NET_RLOCK() down > into nd6_ioctl(), but perhaps it would be better to keep the locking > inside in6_ioctl(), like this: > > switch (cmd) { > case SIOCGIFINFO_IN6: > case SIOCGNBRINFO_IN6: > NET_RLOCK(); > error = nd6_ioctl(cmd, data, ifp); > NET_RUNLOCK(); > return error; > } > > I tested this as well as I could, but I don't usually use IPv6, so I > dabbled a bit with it on my home network and I tried to use the netinet6 > regress tests, only with moderate success. > > A test from people who actually use IPv6 would be welcome. > > Index: sys/netinet6/in6.c > === > RCS file: /var/cvs/src/sys/netinet6/in6.c,v > retrieving revision 1.222 > diff -u -p -r1.222 in6.c > --- sys/netinet6/in6.c24 Apr 2018 19:53:38 - 1.222 > +++ sys/netinet6/in6.c1 May 2018 09:14:07 - > @@ -183,7 +183,6 @@ in6_control(struct socket *so, u_long cm > int privileged; > int error; > > - NET_LOCK(); > privileged = 0; > if ((so->so_state & SS_PRIV) != 0) > privileged++; > @@ -200,7 +199,6 @@ in6_control(struct socket *so, u_long cm > break; > } > > - NET_UNLOCK(); > return error; > } > > @@ -211,12 +209,11 @@ in6_ioctl(u_long cmd, caddr_t data, stru > struct in6_ifaddr *ia6 = NULL; > struct in6_aliasreq *ifra = (struct in6_aliasreq *)data; > struct sockaddr_in6 *sa6; > + int error = 0; > > if (ifp == NULL) > return (ENXIO); > > - NET_ASSERT_LOCKED(); > - > switch (cmd) { > case SIOCGIFINFO_IN6: > case SIOCGNBRINFO_IN6: > @@ -252,13 +249,16 @@ in6_ioctl(u_long cmd, caddr_t data, stru > case SIOCSIFNETMASK: > /* >* Do not pass those ioctl to driver handler since they are not > - * properly setup. Instead just error out. > + * properly set up. Instead just error out. >*/ > return (EINVAL); > default: > sa6 = NULL; > break; > } > + > + NET_LOCK(); > + > if (sa6 && sa6->sin6_family == AF_INET6) { > if (IN6_IS_ADDR_LINKLOCAL(&sa6->sin6_addr)) { > if (sa6->sin6_addr.s6_addr16[1] == 0) { > @@ -267,12 +267,15 @@ in6_ioctl(u_long cmd, caddr_t data, stru > htons(ifp->if_index); > } else if (sa6->sin6_addr.s6_addr16[1] != > htons(ifp->if_index)) { > - return (EINVAL);/* link ID contradicts > */ > + error = EINVAL; /* link ID contradicts */ > + goto err; > } > if (sa6->sin6_scope_id) { > if (sa6->sin6_scope_id != > - (u_int32_t)ifp->if_index) > - return (EINVAL); > + (u_int32_t)ifp->if_index) { > + error = EINVAL; > + goto err; > + } > sa6->sin6_scope_id = 0; /* XXX: good way? */ > } > } > @@ -289,8 +292,10 @@ in6_ioctl(u_long cmd, caddr_t data, stru >* interface address from the day one, we consider "remove the >* first one" semantics to be not preferable. >*/ > - if (ia6 == NULL) > - return (EADDRNOTAVAIL); > + if (ia6 == NULL) { > + error = EADDRNOTAVAIL; > +
Re: libkvm requires kvm_getargv before kvm_getenv when both needed
2018-05-01 21:53 GMT+03:00 Theo de Raadt : > ktrace makes the problem more clear: > > 25908 ps CALL > sysctl(1.55.75675.1,0xed0cc78,0x7f7cd3d8,0,0) > 25908 ps RET sysctl -1 errno 14 Bad address And that's it, thanks! Now little ps(1) is happy. But now there's a question, about kvm_getargv() and kvm_getenv(): what behaviour do we want? a) They use same space, overwriting each other results (this is what happens now, and noone complains). b) Their working space should be independent of each other. This isn't hard, just splitting kd->argbuf into kd->argbuf and kd->envbuf. Seems a bit saner. I'm fine with any direction. The patch below implements (a), since it's less patching. Is it okay, or should it be (b)? -- WBR, Vadim Zhukov Index: kvm_proc.c === RCS file: /cvs/src/lib/libkvm/kvm_proc.c,v retrieving revision 1.58 diff -u -p -r1.58 kvm_proc.c --- kvm_proc.c 7 Nov 2016 00:26:33 - 1.58 +++ kvm_proc.c 1 May 2018 19:23:01 - @@ -458,12 +458,14 @@ kvm_arg_sysctl(kvm_t *kd, pid_t pid, int { size_t len, orglen; int mib[4], ret; - char *buf; + void *buf; orglen = env ? kd->nbpg : 8 * kd->nbpg; /* XXX - should be ARG_MAX */ - if (kd->argbuf == NULL && - (kd->argbuf = _kvm_malloc(kd, orglen)) == NULL) - return (NULL); + + buf = _kvm_realloc(kd, kd->argbuf, orglen); + if (buf == NULL) + return NULL; + kd->argbuf = buf; again: mib[0] = CTL_KERN;
Add 1500000 baud entry to gettytab(5)
The various Rockchip SoCs seem to favour 150 for the serial console speed. And since for some of those we still have to rely on closed source firmware components, I think I'll have to bite the bullit and properly support this. Here is a first diff that adds an appropriate entry to gettytab(5). Not entirely sure whether the 150-baud alias is necessary. But I don't think it'll hurt us. ok? Index: etc/gettytab === RCS file: /cvs/src/etc/gettytab,v retrieving revision 1.5 diff -u -p -r1.5 gettytab --- etc/gettytab10 Dec 2013 20:56:59 - 1.5 +++ etc/gettytab1 May 2018 18:51:45 - @@ -47,6 +47,8 @@ std.57600|57600-baud:\ :sp#57600: std.115200|115200-baud:\ :sp#115200: +std.150|150-baud:\ + :sp#150: # # Dial in rotary tables, speed selection via 'break'
Re: libkvm requires kvm_getargv before kvm_getenv when both needed
ktrace makes the problem more clear: 25908 ps CALL sysctl(1.55.75675.1,0xed0cc78,0x7f7cd3d8,0,0) 25908 ps RET sysctl -1 errno 14 Bad address
wrong realloc in libkvm
Hi all. The "p = realloc(p, size)" idiom is obviously wrong. Since both places to be fixed were similar, I went further and unified the code as well. Note that "argv" is later initialized from kd->argv, so there is no problem in reusing it here. The amd64 is happy. Okay? -- WBR, Vadim Zhukov Index: kvm_proc.c === RCS file: /cvs/src/lib/libkvm/kvm_proc.c,v retrieving revision 1.58 diff -u -p -r1.58 kvm_proc.c --- kvm_proc.c 7 Nov 2016 00:26:33 - 1.58 +++ kvm_proc.c 1 May 2018 18:41:59 - @@ -262,6 +262,7 @@ kvm_argv(kvm_t *kd, const struct kinfo_p char *np, *cp, *ep, *ap, **argv; u_long oaddr = -1; int len, cc; + size_t argc; /* * Check that there aren't an unreasonable number of arguments, @@ -270,22 +271,19 @@ kvm_argv(kvm_t *kd, const struct kinfo_p if (narg > ARG_MAX || addr < VM_MIN_ADDRESS || addr >= VM_MAXUSER_ADDRESS) return (0); - if (kd->argv == 0) { - /* -* Try to avoid reallocs. -*/ - kd->argc = MAX(narg + 1, 32); - kd->argv = _kvm_reallocarray(kd, NULL, kd->argc, - sizeof(*kd->argv)); - if (kd->argv == 0) - return (0); - } else if (narg + 1 > kd->argc) { - kd->argc = MAX(2 * kd->argc, narg + 1); - kd->argv = (char **)_kvm_reallocarray(kd, kd->argv, kd->argc, - sizeof(*kd->argv)); - if (kd->argv == 0) - return (0); - } + if (kd->argv == 0) + argc = MAX(narg + 1, 32); + else if (narg + 1 > kd->argc) + argc = MAX(2 * kd->argc, narg + 1); + else + goto argv_allocated; + argv = _kvm_reallocarray(kd, kd->argv, argc, sizeof(*kd->argv)); + if (argv == 0) + return (0); + kd->argv = argv; + kd->argc = argc; + +argv_allocated: if (kd->argspc == 0) { kd->argspc = _kvm_malloc(kd, kd->nbpg); if (kd->argspc == 0)
libkvm requires kvm_getargv before kvm_getenv when both needed
Hi all. So I finally got bored of ps not displaying command args when "-e" is present. Yes, ps(1) is broken: compare end of lines in output of "ps -ww" and "ps -eww". And IIRC it behaves this way long enough, but I always thought that it's me not missing something in ps(1) manual. Bad zhuk@. This is not a ps(1) bug, though: the simple diff below "fixes" it. Yep, calling kvm_getargv(3) before kvm_getenv(3) makes everyone happy again. I've tried to dive into libkvm but went out of oxygen. The only problem I found was misuse of reallocarray(), to be addressed in another letter. So, does any libkvm hacker have any clues where to look for this argv-envp bug? I'm not sure that I'll find the root issue myself fast enough (in less than half a year). -- WBR, Vadim Zhukov Index: print.c === RCS file: /cvs/src/bin/ps/print.c,v retrieving revision 1.69 diff -u -p -r1.69 print.c --- print.c 8 Sep 2016 15:11:29 - 1.69 +++ print.c 1 May 2018 18:29:52 - @@ -118,6 +118,7 @@ command(const struct kinfo_proc *kp, VAR left = INT_MAX; if (needenv && kd != NULL) { + argv = kvm_getargv(kd, kp, termwidth); argv = kvm_getenvv(kd, kp, termwidth); if ((p = argv) != NULL) { while (*p) {
Re: const for BIO_new, BIO_set and BIO_{f,s}_*
On Mon, Apr 30, 2018 at 09:39:18PM +0200, Theo Buehler wrote: > Here is a straightforward diff that converts BIO_new(), BIO_set() and the > BIO_{f,s}_*() functions to match OpenSSL wrt const. This was part of a > larger diff that was run through a bulk by sthen and produced no fallout. > There were a few more of these in sthen's bulk: Index: evp/bio_b64.c === RCS file: /var/cvs/src/lib/libcrypto/evp/bio_b64.c,v retrieving revision 1.20 diff -u -p -r1.20 bio_b64.c --- evp/bio_b64.c 7 Feb 2015 13:19:15 - 1.20 +++ evp/bio_b64.c 1 May 2018 15:52:07 - @@ -91,7 +91,7 @@ typedef struct b64_struct { char tmp[B64_BLOCK_SIZE]; } BIO_B64_CTX; -static BIO_METHOD methods_b64 = { +static const BIO_METHOD methods_b64 = { .type = BIO_TYPE_BASE64, .name = "base64 encoding", .bwrite = b64_write, @@ -103,7 +103,7 @@ static BIO_METHOD methods_b64 = { .callback_ctrl = b64_callback_ctrl }; -BIO_METHOD * +const BIO_METHOD * BIO_f_base64(void) { return (&methods_b64); Index: evp/bio_enc.c === RCS file: /var/cvs/src/lib/libcrypto/evp/bio_enc.c,v retrieving revision 1.20 diff -u -p -r1.20 bio_enc.c --- evp/bio_enc.c 2 May 2017 03:59:44 - 1.20 +++ evp/bio_enc.c 1 May 2018 15:52:07 - @@ -87,7 +87,7 @@ typedef struct enc_struct { char buf[ENC_BLOCK_SIZE + BUF_OFFSET + 2]; } BIO_ENC_CTX; -static BIO_METHOD methods_enc = { +static const BIO_METHOD methods_enc = { .type = BIO_TYPE_CIPHER, .name = "cipher", .bwrite = enc_write, @@ -98,7 +98,7 @@ static BIO_METHOD methods_enc = { .callback_ctrl = enc_callback_ctrl }; -BIO_METHOD * +const BIO_METHOD * BIO_f_cipher(void) { return (&methods_enc); Index: evp/bio_md.c === RCS file: /var/cvs/src/lib/libcrypto/evp/bio_md.c,v retrieving revision 1.14 diff -u -p -r1.14 bio_md.c --- evp/bio_md.c11 Jul 2014 08:44:48 - 1.14 +++ evp/bio_md.c1 May 2018 15:52:07 - @@ -74,7 +74,7 @@ static int md_new(BIO *h); static int md_free(BIO *data); static long md_callback_ctrl(BIO *h, int cmd, bio_info_cb *fp); -static BIO_METHOD methods_md = { +static const BIO_METHOD methods_md = { .type = BIO_TYPE_MD, .name = "message digest", .bwrite = md_write, @@ -86,7 +86,7 @@ static BIO_METHOD methods_md = { .callback_ctrl = md_callback_ctrl }; -BIO_METHOD * +const BIO_METHOD * BIO_f_md(void) { return (&methods_md); Index: evp/evp.h === RCS file: /var/cvs/src/lib/libcrypto/evp/evp.h,v retrieving revision 1.58 diff -u -p -r1.58 evp.h --- evp/evp.h 20 Feb 2018 18:05:28 - 1.58 +++ evp/evp.h 1 May 2018 15:52:07 - @@ -651,9 +651,9 @@ int EVP_CIPHER_CTX_ctrl(EVP_CIPHER_CTX * int EVP_CIPHER_CTX_rand_key(EVP_CIPHER_CTX *ctx, unsigned char *key); #ifndef OPENSSL_NO_BIO -BIO_METHOD *BIO_f_md(void); -BIO_METHOD *BIO_f_base64(void); -BIO_METHOD *BIO_f_cipher(void); +const BIO_METHOD *BIO_f_md(void); +const BIO_METHOD *BIO_f_base64(void); +const BIO_METHOD *BIO_f_cipher(void); void BIO_set_cipher(BIO *b, const EVP_CIPHER *c, const unsigned char *k, const unsigned char *i, int enc); #endif
Re: Push the netlock down in in_control()
On Mon, Apr 30, 2018 at 02:55:21PM +0200, Martin Pieuchot wrote: > On 30/04/18(Mon) 12:00, Theo Buehler wrote: > > With mpi's encouragement and guidance, here's a diff that reduces the > > scope of the NET_LOCK() a bit. > > > > in_control() is the only caller of mrt_ioctl() and the latter is a > > simple function only requiring a read lock. > > > > There are only a handful callers of in_ioctl(). The two switches create > > relatively tangled codepaths, so this will need some refactoring before > > we can push the lock further down and split the read and write cases. > > For now, establish a single exit point, grab the netlock at the > > beginning and release it at the end. > > That makes sense, the same could be done for in6_ioctl(). Here's the corresponding diff for in6_control(). Push the diff down into mrt6_ioctl() and in6_ioctl(). Much like the IPv4 version, mrt6_ioctl() only needs a read lock. in6_ioctl() is a bit messier. It starts off with a call to nd6_ioctl(), which only needs a read lock. In the diff, I pushed the NET_RLOCK() down into nd6_ioctl(), but perhaps it would be better to keep the locking inside in6_ioctl(), like this: switch (cmd) { case SIOCGIFINFO_IN6: case SIOCGNBRINFO_IN6: NET_RLOCK(); error = nd6_ioctl(cmd, data, ifp); NET_RUNLOCK(); return error; } I tested this as well as I could, but I don't usually use IPv6, so I dabbled a bit with it on my home network and I tried to use the netinet6 regress tests, only with moderate success. A test from people who actually use IPv6 would be welcome. Index: sys/netinet6/in6.c === RCS file: /var/cvs/src/sys/netinet6/in6.c,v retrieving revision 1.222 diff -u -p -r1.222 in6.c --- sys/netinet6/in6.c 24 Apr 2018 19:53:38 - 1.222 +++ sys/netinet6/in6.c 1 May 2018 09:14:07 - @@ -183,7 +183,6 @@ in6_control(struct socket *so, u_long cm int privileged; int error; - NET_LOCK(); privileged = 0; if ((so->so_state & SS_PRIV) != 0) privileged++; @@ -200,7 +199,6 @@ in6_control(struct socket *so, u_long cm break; } - NET_UNLOCK(); return error; } @@ -211,12 +209,11 @@ in6_ioctl(u_long cmd, caddr_t data, stru struct in6_ifaddr *ia6 = NULL; struct in6_aliasreq *ifra = (struct in6_aliasreq *)data; struct sockaddr_in6 *sa6; + int error = 0; if (ifp == NULL) return (ENXIO); - NET_ASSERT_LOCKED(); - switch (cmd) { case SIOCGIFINFO_IN6: case SIOCGNBRINFO_IN6: @@ -252,13 +249,16 @@ in6_ioctl(u_long cmd, caddr_t data, stru case SIOCSIFNETMASK: /* * Do not pass those ioctl to driver handler since they are not -* properly setup. Instead just error out. +* properly set up. Instead just error out. */ return (EINVAL); default: sa6 = NULL; break; } + + NET_LOCK(); + if (sa6 && sa6->sin6_family == AF_INET6) { if (IN6_IS_ADDR_LINKLOCAL(&sa6->sin6_addr)) { if (sa6->sin6_addr.s6_addr16[1] == 0) { @@ -267,12 +267,15 @@ in6_ioctl(u_long cmd, caddr_t data, stru htons(ifp->if_index); } else if (sa6->sin6_addr.s6_addr16[1] != htons(ifp->if_index)) { - return (EINVAL);/* link ID contradicts */ + error = EINVAL; /* link ID contradicts */ + goto err; } if (sa6->sin6_scope_id) { if (sa6->sin6_scope_id != - (u_int32_t)ifp->if_index) - return (EINVAL); + (u_int32_t)ifp->if_index) { + error = EINVAL; + goto err; + } sa6->sin6_scope_id = 0; /* XXX: good way? */ } } @@ -289,8 +292,10 @@ in6_ioctl(u_long cmd, caddr_t data, stru * interface address from the day one, we consider "remove the * first one" semantics to be not preferable. */ - if (ia6 == NULL) - return (EADDRNOTAVAIL); + if (ia6 == NULL) { + error = EADDRNOTAVAIL; + goto err; + } /* FALLTHROUGH */ case SIOCAIFADDR_IN6: /* @@ -298,11 +303,14 @@ in6_ioctl(u_long cmd, caddr_t data, stru * the corresponding
Re: avoid uninitialised attr in rasops_scrollback()
> scrollback isn't part of wsdisplay_emulops but rather wsdisplay_accessops. > It isn't clear how to properly get an attr in wsscrollback() > GETCHAR/getchar is part of wsmoused and sc->sc_accessops->getchar > may be NULL. Ok, ignore that. There is a simpler solution. Index: rasops.c === RCS file: /OpenBSD/src/sys/dev/rasops/rasops.c,v retrieving revision 1.53 diff -u -p -r1.53 rasops.c --- rasops.c27 Apr 2018 21:36:12 - 1.53 +++ rasops.c1 May 2018 17:02:43 - @@ -1373,6 +1373,7 @@ struct rasops_screen { int rs_visible; int rs_crow; int rs_ccol; + long rs_defattr; int rs_sbscreens; #define RS_SCROLLBACK_SCREENS 5 @@ -1412,10 +1413,11 @@ rasops_alloc_screen(void *v, void **cook scr->rs_visible = (ri->ri_nscreens == 0); scr->rs_crow = -1; scr->rs_ccol = -1; + scr->rs_defattr = *attrp; for (i = 0; i < scr->rs_dispoffset; i++) { scr->rs_bs[i].uc = ' '; - scr->rs_bs[i].attr = *attrp; + scr->rs_bs[i].attr = scr->rs_defattr; } if (ri->ri_bs && scr->rs_visible) { @@ -1425,7 +1427,8 @@ rasops_alloc_screen(void *v, void **cook } else { for (i = 0; i < ri->ri_rows * ri->ri_cols; i++) { scr->rs_bs[scr->rs_dispoffset + i].uc = ' '; - scr->rs_bs[scr->rs_dispoffset + i].attr = *attrp; + scr->rs_bs[scr->rs_dispoffset + i].attr = + scr->rs_defattr; } } @@ -1906,7 +1909,6 @@ rasops_scrollback(void *v, void *cookie, struct rasops_info *ri = v; struct rasops_screen *scr = cookie; int row, col, oldvoff; - long attr; oldvoff = scr->rs_visibleoffset; @@ -1927,7 +1929,7 @@ rasops_scrollback(void *v, void *cookie, return; rasops_cursor(ri, 0, 0, 0); - ri->ri_eraserows(ri, 0, ri->ri_rows, attr); + ri->ri_eraserows(ri, 0, ri->ri_rows, scr->rs_defattr); for (row = 0; row < ri->ri_rows; row++) { for (col = 0; col < ri->ri_cols; col++) { int off = row * scr->rs_ri->ri_cols + col +
a first batch of const for x509
Here's a few X509_* functions with added const. This was part of a bulk by sthen with no fallout. One minor difference: the diff I sent to Stuart contained a mistake in X509_signature_print(). Most of these are straightforward. X509_ALGOR_get0() required a bit of churn in *{pub,priv}_decode* functions. Among those, gost stands out with an ugly cast, but I'd rather not touch these more than necessary. Index: asn1/t_x509.c === RCS file: /cvs/src/lib/libcrypto/asn1/t_x509.c,v retrieving revision 1.29 diff -u -p -r1.29 t_x509.c --- asn1/t_x509.c 25 Apr 2018 19:58:53 - 1.29 +++ asn1/t_x509.c 1 May 2018 16:18:46 - @@ -321,7 +321,7 @@ X509_signature_dump(BIO *bp, const ASN1_ } int -X509_signature_print(BIO *bp, X509_ALGOR *sigalg, ASN1_STRING *sig) +X509_signature_print(BIO *bp, const X509_ALGOR *sigalg, const ASN1_STRING *sig) { int sig_nid; if (BIO_puts(bp, "Signature Algorithm: ") <= 0) Index: asn1/x_algor.c === RCS file: /cvs/src/lib/libcrypto/asn1/x_algor.c,v retrieving revision 1.21 diff -u -p -r1.21 x_algor.c --- asn1/x_algor.c 24 Jul 2015 15:09:52 - 1.21 +++ asn1/x_algor.c 1 May 2018 16:18:46 - @@ -176,8 +176,8 @@ X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OB } void -X509_ALGOR_get0(ASN1_OBJECT **paobj, int *pptype, void **ppval, -X509_ALGOR *algor) +X509_ALGOR_get0(const ASN1_OBJECT **paobj, int *pptype, const void **ppval, +const X509_ALGOR *algor) { if (paobj) *paobj = algor->algorithm; Index: asn1/x_x509a.c === RCS file: /cvs/src/lib/libcrypto/asn1/x_x509a.c,v retrieving revision 1.14 diff -u -p -r1.14 x_x509a.c --- asn1/x_x509a.c 14 Feb 2015 15:28:39 - 1.14 +++ asn1/x_x509a.c 1 May 2018 16:18:46 - @@ -154,7 +154,7 @@ aux_get(X509 *x) } int -X509_alias_set1(X509 *x, unsigned char *name, int len) +X509_alias_set1(X509 *x, const unsigned char *name, int len) { X509_CERT_AUX *aux; if (!name) { @@ -172,7 +172,7 @@ X509_alias_set1(X509 *x, unsigned char * } int -X509_keyid_set1(X509 *x, unsigned char *id, int len) +X509_keyid_set1(X509 *x, const unsigned char *id, int len) { X509_CERT_AUX *aux; if (!id) { @@ -210,7 +210,7 @@ X509_keyid_get0(X509 *x, int *len) } int -X509_add1_trust_object(X509 *x, ASN1_OBJECT *obj) +X509_add1_trust_object(X509 *x, const ASN1_OBJECT *obj) { X509_CERT_AUX *aux; ASN1_OBJECT *objtmp; @@ -232,7 +232,7 @@ err: } int -X509_add1_reject_object(X509 *x, ASN1_OBJECT *obj) +X509_add1_reject_object(X509 *x, const ASN1_OBJECT *obj) { X509_CERT_AUX *aux; ASN1_OBJECT *objtmp; Index: dh/dh_ameth.c === RCS file: /cvs/src/lib/libcrypto/dh/dh_ameth.c,v retrieving revision 1.14 diff -u -p -r1.14 dh_ameth.c --- dh/dh_ameth.c 29 Jan 2017 17:49:22 - 1.14 +++ dh/dh_ameth.c 1 May 2018 16:18:46 - @@ -78,8 +78,8 @@ dh_pub_decode(EVP_PKEY *pkey, X509_PUBKE const unsigned char *p, *pm; int pklen, pmlen; int ptype; - void *pval; - ASN1_STRING *pstr; + const void *pval; + const ASN1_STRING *pstr; X509_ALGOR *palg; ASN1_INTEGER *public_key = NULL; DH *dh = NULL; @@ -185,8 +185,8 @@ dh_priv_decode(EVP_PKEY *pkey, PKCS8_PRI const unsigned char *p, *pm; int pklen, pmlen; int ptype; - void *pval; - ASN1_STRING *pstr; + const void *pval; + const ASN1_STRING *pstr; X509_ALGOR *palg; ASN1_INTEGER *privkey = NULL; DH *dh = NULL; Index: dsa/dsa_ameth.c === RCS file: /cvs/src/lib/libcrypto/dsa/dsa_ameth.c,v retrieving revision 1.23 diff -u -p -r1.23 dsa_ameth.c --- dsa/dsa_ameth.c 29 Jan 2017 17:49:22 - 1.23 +++ dsa/dsa_ameth.c 1 May 2018 16:18:46 - @@ -75,8 +75,8 @@ dsa_pub_decode(EVP_PKEY *pkey, X509_PUBK const unsigned char *p, *pm; int pklen, pmlen; int ptype; - void *pval; - ASN1_STRING *pstr; + const void *pval; + const ASN1_STRING *pstr; X509_ALGOR *palg; ASN1_INTEGER *public_key = NULL; @@ -184,8 +184,8 @@ dsa_priv_decode(EVP_PKEY *pkey, PKCS8_PR const unsigned char *p, *pm; int pklen, pmlen; int ptype; - void *pval; - ASN1_STRING *pstr; + const void *pval; + const ASN1_STRING *pstr; X509_ALGOR *palg; ASN1_INTEGER *privkey = NULL; BN_CTX *ctx = NULL; Index: ec/ec_ameth.c === RCS file: /cvs/src/lib/libcrypto/ec/ec_ameth.c,v retrieving revision 1.19 diff -u -p -r1.19 ec_ameth.c --- ec/ec_ameth.c
Re: rssi comparison threshold
On 2018 May 01 (Tue) at 11:20:54 +0200 (+0200), Stefan Sperling wrote: :On Mon, Apr 30, 2018 at 08:57:23AM +0200, Peter Hessler wrote: :> On 2018 Apr 29 (Sun) at 11:51:26 +0200 (+0200), Stefan Sperling wrote: :> :This diff tries to avoid situations where background scans play :> :ping-pong between different APs with nearly equal RSSI, as :> :observed by phessler. :> : :> :Not all drivers represent RSSI values in dBm or percentage, so the :> :diff includes the possibility for drivers to override the new RSSI :> :comparison function. However, since the threshold is rather low :> :applying this to all drivers for now should not do any harm, unless :> :there is a driver where the RSSI value range is ridiculously small. :> :I'm not aware of any such driver at present. :> : :> :> I'm concerned about two things. :> :> The usage is confusing, because you pass two incompatible things to be :> compared. I would prefer ieee80211_node_cmprssi(ic, rssi_one, rssi_two). : :Agreed. Updated diff below. I've chosen to make it compare two nodes :since the function lives in the nodes namespace. It makes the function :a bit less flexible as it cannot be used to compare naked rssi values. :But that's not really needed for the problem at hand; after all, we :want to compare APs. : :> Also, in the case where ni->ni_rssi has a very weak signal, the second :> comparison can underflow. : :Indeed. Thanks for catching this! : OK :Index: ieee80211_node.c :=== :RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v :retrieving revision 1.129 :diff -u -p -r1.129 ieee80211_node.c :--- ieee80211_node.c 28 Apr 2018 14:49:07 - 1.129 :+++ ieee80211_node.c 30 Apr 2018 07:30:43 - :@@ -67,6 +67,8 @@ u_int8_t ieee80211_node_getrssi(struct i : const struct ieee80211_node *); : int ieee80211_node_checkrssi(struct ieee80211com *, : const struct ieee80211_node *); :+int ieee80211_node_cmprssi(struct ieee80211com *, :+const struct ieee80211_node *, const struct ieee80211_node *); : void ieee80211_setup_node(struct ieee80211com *, struct ieee80211_node *, : const u_int8_t *); : void ieee80211_free_node(struct ieee80211com *, struct ieee80211_node *); :@@ -133,6 +135,7 @@ ieee80211_node_attach(struct ifnet *ifp) : ic->ic_node_copy = ieee80211_node_copy; : ic->ic_node_getrssi = ieee80211_node_getrssi; : ic->ic_node_checkrssi = ieee80211_node_checkrssi; :+ ic->ic_node_cmprssi = ieee80211_node_cmprssi; : ic->ic_scangen = 1; : ic->ic_max_nnodes = ieee80211_cache_size; : :@@ -761,12 +764,15 @@ ieee80211_end_scan(struct ifnet *ifp) : : if (ic->ic_caps & IEEE80211_C_SCANALLBAND) { : if (IEEE80211_IS_CHAN_2GHZ(ni->ni_chan) && :- (selbs2 == NULL || ni->ni_rssi > selbs2->ni_rssi)) :+ (selbs2 == NULL || :+ ic->ic_node_cmprssi(ic, ni, selbs2) > 0)) : selbs2 = ni; : else if (IEEE80211_IS_CHAN_5GHZ(ni->ni_chan) && :- (selbs5 == NULL || ni->ni_rssi > selbs5->ni_rssi)) :+ (selbs5 == NULL || :+ ic->ic_node_cmprssi(ic, ni, selbs5) > 0)) : selbs5 = ni; :- } else if (selbs == NULL || ni->ni_rssi > selbs->ni_rssi) :+ } else if (selbs == NULL || :+ ic->ic_node_cmprssi(ic, ni, selbs) > 0) : selbs = ni; : } : :@@ -782,9 +788,12 @@ ieee80211_end_scan(struct ifnet *ifp) :*/ : if (selbs5 && selbs5->ni_rssi > min_5ghz_rssi) : selbs = selbs5; :- else if (selbs5 && selbs2) :- selbs = (selbs5->ni_rssi >= selbs2->ni_rssi ? selbs5 : selbs2); :- else if (selbs2) :+ else if (selbs5 && selbs2) { :+ if (ic->ic_node_cmprssi(ic, selbs5, selbs2) >= 0) :+ selbs = selbs5; :+ else :+ selbs = selbs2; :+ } else if (selbs2) : selbs = selbs2; : else if (selbs5) : selbs = selbs5; :@@ -989,6 +998,38 @@ ieee80211_node_checkrssi(struct ieee8021 : IEEE80211_RSSI_THRES_2GHZ : : IEEE80211_RSSI_THRES_5GHZ; : return (ni->ni_rssi >= (u_int8_t)thres); :+} :+ :+/* :+ * Determine if RSSI values of two nodes are significantly different. :+ * This function assumes RSSI values are represented either in dBm or :+ * as a percentage of ic_max_rssi. Drivers should override this function :+ * in case their RSSI values use a different representation. :+ */ :+int :+ieee80211_node_cmprssi(struct ieee80211com *ic, :+const struct ieee80211_node *ni1, const struct ieee80211_node *ni2) :+{ :+ uint8_t thres; :+ :+ if (ic->ic_max_rssi) :+ thres = IEEE80211_RSSI_CMP_THRES_RATIO; :+ else :+ thres = IEEE80211_RSSI_CMP_THRES_DBM; :+ :+
Re: vmd: def nitems() locally
Mike Larkin wrote: > On Mon, Apr 30, 2018 at 07:49:20PM -0300, Gleydson Soares wrote: > > hi, > > > > following diff defines nitems locally and stop > > including > > Has this been done elsewhere in the tree? Is this the new paradigm > we will be adopting? I don't have a strong opinion one way or the > other, just want to be consistent. I prefer the current "document the problem on #include " line approach, until we make a decision about making this available in userland. Which would require a robust assessment of impact on the entire ports ecosystem, and consider how it may bleed into other projects. I don't believe we have reached that point yet, so I prefer if it remains as-is.
Re: vmd: def nitems() locally
On Mon, Apr 30, 2018 at 07:49:20PM -0300, Gleydson Soares wrote: > hi, > > following diff defines nitems locally and stop > including Has this been done elsewhere in the tree? Is this the new paradigm we will be adopting? I don't have a strong opinion one way or the other, just want to be consistent. -ml > Index: control.c > === > RCS file: /cvs/src/usr.sbin/vmd/control.c,v > retrieving revision 1.22 > diff -u -p -r1.22 control.c > --- control.c 8 Sep 2017 06:24:31 - 1.22 > +++ control.c 30 Apr 2018 22:45:22 - > @@ -17,7 +17,6 @@ > * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > */ > > -#include/* nitems */ > #include > #include > #include > Index: priv.c > === > RCS file: /cvs/src/usr.sbin/vmd/priv.c,v > retrieving revision 1.13 > diff -u -p -r1.13 priv.c > --- priv.c11 Nov 2017 02:50:07 - 1.13 > +++ priv.c30 Apr 2018 22:45:22 - > @@ -16,7 +16,6 @@ > * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > */ > > -#include/* nitems */ > #include > #include > #include > Index: vmd.c > === > RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v > retrieving revision 1.84 > diff -u -p -r1.84 vmd.c > --- vmd.c 25 Apr 2018 15:49:48 - 1.84 > +++ vmd.c 30 Apr 2018 22:45:22 - > @@ -16,7 +16,6 @@ > * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > */ > > -#include/* nitems */ > #include > #include > #include > Index: vmd.h > === > RCS file: /cvs/src/usr.sbin/vmd/vmd.h,v > retrieving revision 1.68 > diff -u -p -r1.68 vmd.h > --- vmd.h 27 Apr 2018 12:15:10 - 1.68 > +++ vmd.h 30 Apr 2018 22:45:22 - > @@ -35,6 +35,10 @@ > #ifndef VMD_H > #define VMD_H > > +#ifndef nitems > +#define nitems(_a)(sizeof((_a)) / sizeof((_a)[0])) > +#endif > + > #define SET(_v, _m) ((_v) |= (_m)) > #define CLR(_v, _m) ((_v) &= ~(_m)) > #define ISSET(_v, _m)((_v) & (_m)) > Index: vmm.c > === > RCS file: /cvs/src/usr.sbin/vmd/vmm.c,v > retrieving revision 1.81 > diff -u -p -r1.81 vmm.c > --- vmm.c 13 Apr 2018 17:12:44 - 1.81 > +++ vmm.c 30 Apr 2018 22:45:22 - > @@ -16,7 +16,6 @@ > * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > */ > > -#include/* nitems */ > #include > #include > #include
Re: remove 2 obsolete libraries from Xenocara
On Sat, Apr 28, 2018 at 07:24:28PM +0200, Matthieu Herrb wrote: > Hi, > > libXfontcache and libXxf86misc are implementing the client part of X > extensions that have been disabled/removed for a while in the X > server. So builing them or linking to them is useless. > > The patch below stops building them in xenocara. A few ports where > using libXxf86misc. Rebuilding them without this library just requires > an update to WANTLIB (patch below, too). > > To test make sure you build xenocara with an empty /usr/X11R6 or that > you remove all files removed from sets manually. > > PS: This also prepares for the switch to xorgproto and xserver 1.20. > > oks? I've been running this on my x230 for a few days without any problems. I don't use any of the impacted ports, though. fwiw, I'm ok with this removal.
Hyper-V network: let hvn_iff handle promisc mode activation
Hi, A user has reported an issue with not having a proper promiscuous mode on reddit a while back but as of now didn't manage to test the diff. If somebody is running OpenBSD on Hyper-V, please test the diff below and see if there are any regressions (like increased CPU load) without running tcpdump and then try running 'tcpdump -nvvi hvn0' and see if that works: one indicator is the PROMISC flag in the ifconfig output that should get set when tcpdump is started and cleared once it's finished. I'm not certain if Hyper-V will allow you to sniff a whole lot of traffic on the virtual switch, however, one of the goals here is to instruct RNDIS to enter "all multicast capture" mode when one or more multicast addresses are configured. Therefore one of the goals is to attempt to support CARP operation on the virtual switch. commit 6e6b001dae79505e3e0fbca663c31f9eb6da285b Author: Mike Belopuhov Date: Sun Mar 11 13:38:21 2018 +0100 Add support for promisc mode diff --git sys/dev/pv/if_hvn.c sys/dev/pv/if_hvn.c index 3ca35165565..6d5e919b01c 100644 --- sys/dev/pv/if_hvn.c +++ sys/dev/pv/if_hvn.c @@ -134,11 +134,10 @@ struct hvn_softc { bus_dma_tag_tsc_dmat; struct arpcomsc_ac; struct ifmedia sc_media; int sc_link_state; - int sc_promisc; /* NVS protocol */ int sc_proto; uint32_t sc_nvstid; uint8_t sc_nvsrsp[HVN_NVS_MSGSIZE]; @@ -208,11 +207,10 @@ void hvn_rxeof(struct hvn_softc *, caddr_t, uint32_t, struct mbuf_list *); void hvn_rndis_complete(struct hvn_softc *, caddr_t, uint32_t); inthvn_rndis_output(struct hvn_softc *, struct hvn_tx_desc *); void hvn_rndis_status(struct hvn_softc *, caddr_t, uint32_t); inthvn_rndis_query(struct hvn_softc *, uint32_t, void *, size_t *); inthvn_rndis_set(struct hvn_softc *, uint32_t, void *, size_t); -inthvn_rndis_open(struct hvn_softc *); inthvn_rndis_close(struct hvn_softc *); void hvn_rndis_detach(struct hvn_softc *); struct cfdriver hvn_cd = { NULL, "hvn", DV_IFNET @@ -401,26 +399,44 @@ hvn_link_status(struct hvn_softc *sc) } int hvn_iff(struct hvn_softc *sc) { - /* XXX */ - sc->sc_promisc = 0; + struct ifnet *ifp = &sc->sc_ac.ac_if; + uint32_t filter = 0; + int rv; - return (0); + ifp->if_flags &= ~IFF_ALLMULTI; + + if ((ifp->if_flags & IFF_PROMISC) || sc->sc_ac.ac_multirangecnt > 0) { + ifp->if_flags |= IFF_ALLMULTI; + filter = NDIS_PACKET_TYPE_PROMISCUOUS; + } else { + filter = NDIS_PACKET_TYPE_BROADCAST | + NDIS_PACKET_TYPE_DIRECTED; + if (sc->sc_ac.ac_multicnt > 0) { + ifp->if_flags |= IFF_ALLMULTI; + filter |= NDIS_PACKET_TYPE_ALL_MULTICAST; + } + } + + rv = hvn_rndis_set(sc, OID_GEN_CURRENT_PACKET_FILTER, + &filter, sizeof(filter)); + if (rv) + DPRINTF("%s: failed to set RNDIS filter to %#x\n", + sc->sc_dev.dv_xname, filter); + return (rv); } void hvn_init(struct hvn_softc *sc) { struct ifnet *ifp = &sc->sc_ac.ac_if; hvn_stop(sc); - hvn_iff(sc); - - if (hvn_rndis_open(sc) == 0) { + if (hvn_iff(sc) == 0) { ifp->if_flags |= IFF_RUNNING; ifq_clr_oactive(&ifp->if_snd); } } @@ -1723,31 +1739,10 @@ hvn_rndis_set(struct hvn_softc *sc, uint32_t oid, void *data, size_t length) hvn_free_cmd(sc, rc); return (rv); } -int -hvn_rndis_open(struct hvn_softc *sc) -{ - uint32_t filter; - int rv; - - if (sc->sc_promisc) - filter = NDIS_PACKET_TYPE_PROMISCUOUS; - else - filter = NDIS_PACKET_TYPE_BROADCAST | - NDIS_PACKET_TYPE_ALL_MULTICAST | - NDIS_PACKET_TYPE_DIRECTED; - - rv = hvn_rndis_set(sc, OID_GEN_CURRENT_PACKET_FILTER, - &filter, sizeof(filter)); - if (rv) - DPRINTF("%s: failed to set RNDIS filter to %#x\n", - sc->sc_dev.dv_xname, filter); - return (rv); -} - int hvn_rndis_close(struct hvn_softc *sc) { uint32_t filter = 0; int rv;
Re: route(8): stop debugging route monitor
On Tue, May 01, 2018 at 11:53:16AM +0200, Sebastian Benoit wrote: > Florian Obser(flor...@openbsd.org) on 2018.04.30 18:27:52 +0200: > > The -d flag should be a no-op in monitor mode since it does not modify > > the routing table. > > > > However, if -d is provided route monitor lists all interfaces and > > their associated addresses and exits. This is confusing, unexpected > > and no longer needed (if ever). > > > > Make -d a proper no-op for route monitor and get rid of the interfaces > > function which didn't use the correct sysctl idiom anyway. > > > > OK? > > if nobody complains about loosing that "feature", ok benno@ > I didn't know about it but I could have used this from time to time. Still it feels overloading the debug flag with that is not the right way. It is useful since this is what getifaddrs(3) uses in the background. > > diff --git route.c route.c > > index 85e76621dd3..738de3a8cde 100644 > > --- route.c > > +++ route.c > > @@ -115,7 +115,6 @@ int rtmsg(int, int, int, uint8_t); > > __dead void usage(char *); > > voidset_metric(char *, int); > > voidinet_makenetandmask(u_int32_t, struct sockaddr_in *, int); > > -voidinterfaces(void); > > voidgetlabel(char *); > > int gettable(const char *); > > int rdomain(int, char **); > > @@ -1069,36 +1068,6 @@ prefixlen(int af, char *s) > > return (len == max); > > } > > > > -void > > -interfaces(void) > > -{ > > - size_t needed; > > - int mib[6]; > > - char *buf = NULL, *lim, *next; > > - struct rt_msghdr *rtm; > > - > > - mib[0] = CTL_NET; > > - mib[1] = PF_ROUTE; > > - mib[2] = 0; /* protocol */ > > - mib[3] = 0; /* wildcard address family */ > > - mib[4] = NET_RT_IFLIST; > > - mib[5] = 0; /* no flags */ > > - if (sysctl(mib, 6, NULL, &needed, NULL, 0) < 0) > > - err(1, "route-sysctl-estimate"); > > - if (needed) { > > - if ((buf = malloc(needed)) == NULL) > > - err(1, "malloc"); > > - if (sysctl(mib, 6, buf, &needed, NULL, 0) < 0) > > - err(1, "actual retrieval of interface table"); > > - lim = buf + needed; > > - for (next = buf; next < lim; next += rtm->rtm_msglen) { > > - rtm = (struct rt_msghdr *)next; > > - print_rtmsg(rtm, rtm->rtm_msglen); > > - } > > - free(buf); > > - } > > -} > > - > > void > > monitor(int argc, char *argv[]) > > { > > @@ -1107,10 +1076,6 @@ monitor(int argc, char *argv[]) > > time_t now; > > > > verbose = 1; > > - if (debugonly) { > > - interfaces(); > > - exit(0); > > - } > > for (;;) { > > if ((n = read(s, msg, sizeof(msg))) == -1) { > > if (errno == EINTR) > > > > -- > > I'm not entirely sure you are real. > > > -- :wq Claudio
Re: pf pledge and ioctls
Nick Owens(misch...@offblast.org) on 2018.04.30 18:40:34 -0700: > hi tech@, > > i've written a program in go which gathers information from pf for > exporting to prometheus. my go program invokes several ioctls on /dev/pf to > do this. however, i recently looked at pledge'ing my program, but i've > noticed that not all of the pf ioctls that i use are included in the pf > pledge. currently the missing ioctls my program needs are DIOCGETQUEUES, > and DIOCGETQSTATS, but i wonder if there is a reason why these and other > ioctls are not included in the pf pledge. > > so some questions - would there be a problem with adding these ioctls? what > about adding the rest of pf ioctls to the pf pledge? and last, would it > make sense to split the pf pledge to "pf" for full control and "rpf" for > read only access such as what my metrics gathering program needs? Not all features are useable when pledged. Features exposed through pledge are always balanced by what programms need, what still is understandable by users of pledge, what is considered safe and by how programs should be designed. In this case, you can use priveledge seperation and move the functions that cannot be pledged into a seperate process.
SMCCC 1.1 support for psci(4)
So after adding a quick hack to mitigate Spectre variant 2 to ARM Trusted Firmware (ATF), ARM actually designed a proper solution that minimizes the performance loss and makes the presence of the workaround detectable. This is all documented in an update of the SMC Calling Convention (SMCCC) standard. The diff below implements support for this solution while keeping support for the hack. While ARM strongly suggests vendors to update to a version of ATF that implements SMCCC 1.1 the current ATF for the Marvell ARMADA 8040 hasn't been updated yet (but does include the initial hack). Unfortunately the SMCCC 1.1 implementation in ATF doesn't quite implement the spec. As a result we have to check whether the workaround is implemented by issuing the relevant calls on each of the CPUs that might be affected. This is important for big.LITTLE designs such as the RK3399 that include both Cortex-A53 cores that aren't vulnerable and Cortex-A72 cores that are. ok? Index: dev/fdt/psci.c === RCS file: /cvs/src/sys/dev/fdt/psci.c,v retrieving revision 1.6 diff -u -p -r1.6 psci.c --- dev/fdt/psci.c 23 Feb 2018 19:08:56 - 1.6 +++ dev/fdt/psci.c 1 May 2018 09:35:14 - @@ -31,14 +31,19 @@ extern void (*cpuresetfn)(void); extern void (*powerdownfn)(void); -#define PSCI_VERSION 0x8400 -#define SYSTEM_OFF 0x8408 -#define SYSTEM_RESET 0x8409 +#define SMCCC_VERSION 0x8000 +#define SMCCC_ARCH_FEATURES0x8001 +#define SMCCC_ARCH_WORKAROUND_10x80008000 + +#define PSCI_VERSION 0x8400 #ifdef __LP64__ -#define CPU_ON 0xc403 +#define CPU_ON 0xc403 #else -#define CPU_ON 0x8403 +#define CPU_ON 0x8403 #endif +#define SYSTEM_OFF 0x8408 +#define SYSTEM_RESET 0x8409 +#define PSCI_FEATURES 0x840a struct psci_softc { struct devicesc_dev; @@ -48,6 +53,9 @@ struct psci_softc { uint32_t sc_system_off; uint32_t sc_system_reset; uint32_t sc_cpu_on; + + uint32_t sc_version; + uint32_t sc_smccc_version; }; struct psci_softc *psci_sc; @@ -60,6 +68,12 @@ void psci_powerdown(void); extern register_t hvc_call(register_t, register_t, register_t, register_t); extern register_t smc_call(register_t, register_t, register_t, register_t); +int32_t smccc_version(void); +int32_t smccc_arch_features(uint32_t); + +uint32_t psci_version(void); +int32_t psci_features(uint32_t); + struct cfattach psci_ca = { sizeof(struct psci_softc), psci_match, psci_attach }; @@ -84,7 +98,6 @@ psci_attach(struct device *parent, struc struct psci_softc *sc = (struct psci_softc *)self; struct fdt_attach_args *faa = aux; char method[128]; - uint32_t version; if (OF_getprop(faa->fa_node, "method", method, sizeof(method))) { if (strcmp(method, "hvc") == 0) @@ -114,8 +127,18 @@ psci_attach(struct device *parent, struc psci_sc = sc; - version = psci_version(); - printf(": PSCI %d.%d\n", version >> 16, version & 0x); + sc->sc_version = psci_version(); + printf(": PSCI %d.%d", sc->sc_version >> 16, sc->sc_version & 0x); + + if (sc->sc_version >= 0x1) { + if (psci_features(SMCCC_VERSION) == PSCI_SUCCESS) { + sc->sc_smccc_version = smccc_version(); + printf(", SMCCC %d.%d", sc->sc_smccc_version >> 16, + sc->sc_smccc_version & 0x); + } + } + + printf("\n"); if (sc->sc_system_off != 0) powerdownfn = psci_powerdown; @@ -123,22 +146,11 @@ psci_attach(struct device *parent, struc cpuresetfn = psci_reset; } -uint32_t -psci_version(void) -{ - struct psci_softc *sc = psci_sc; - - if (sc && sc->sc_callfn && sc->sc_psci_version != 0) - return (*sc->sc_callfn)(sc->sc_psci_version, 0, 0, 0); - - /* No version support; return 0.0. */ - return 0; -} - void psci_reset(void) { struct psci_softc *sc = psci_sc; + if (sc->sc_callfn) (*sc->sc_callfn)(sc->sc_system_reset, 0, 0, 0); } @@ -147,10 +159,111 @@ void psci_powerdown(void) { struct psci_softc *sc = psci_sc; + if (sc->sc_callfn) (*sc->sc_callfn)(sc->sc_system_off, 0, 0, 0); } +/* + * Firmware-based workaround for CVE-2017-5715. We pick the + * appropriate mechanism based on the PSCI and SMCCC versions. Note + * that ARM Trusted Firmware violates the SMCCC 1.1 specification and + * only reports the presence of the appropriate workaround on CPUs + * that are actually vulnerable. That's why we determine the + * appropriate workaround the first time around. + */ + +void +psci_flush_bp_none(void) +{ +} + +void +psc
Re: route(8): sync p_rttables to netstat(1) version
ok Florian Obser(flor...@openbsd.org) on 2018.04.30 18:26:46 +0200: > Sync p_rttables() to netstat(1) version. Pointed out by claudio and > mpi. > > Remaining differences are pledge and priority handling which only > route(8) has. > > While here switch flushroutes to get_sysctl() function. > > OK? > > diff --git route.c route.c > index d93374578c5..85e76621dd3 100644 > --- route.c > +++ route.c > @@ -281,7 +281,7 @@ int > flushroutes(int argc, char **argv) > { > size_t needed; > - int mib[7], rlen, seqno, af = AF_UNSPEC; > + int mib[7], mcnt, rlen, seqno, af = AF_UNSPEC; > char *buf = NULL, *next, *lim = NULL; > struct rt_msghdr *rtm; > struct sockaddr *sa; > @@ -333,21 +333,10 @@ flushroutes(int argc, char **argv) > mib[4] = NET_RT_DUMP; > mib[5] = prio; > mib[6] = tableid; > - while (1) { > - if (sysctl(mib, 7, NULL, &needed, NULL, 0) == -1) > - err(1, "route-sysctl-estimate"); > - if (needed == 0) > - break; > - if ((buf = realloc(buf, needed)) == NULL) > - err(1, "realloc"); > - if (sysctl(mib, 7, buf, &needed, NULL, 0) == -1) { > - if (errno == ENOMEM) > - continue; > - err(1, "actual retrieval of routing table"); > - } > - lim = buf + needed; > - break; > - } > + mcnt = 7; > + > + needed = get_sysctl(mib, mcnt, &buf); > + lim = buf + needed; > > if (pledge("stdio dns", NULL) == -1) > err(1, "pledge"); > diff --git show.c show.c > index 5898a19cd45..68731c78591 100644 > --- show.c > +++ show.c > @@ -107,6 +107,29 @@ char *routename6(struct sockaddr_in6 *); > char *netname4(in_addr_t, struct sockaddr_in *); > char *netname6(struct sockaddr_in6 *, struct sockaddr_in6 *); > > +size_t > +get_sysctl(const int *mib, u_int mcnt, char **buf) > +{ > + size_t needed; > + > + while (1) { > + if (sysctl(mib, mcnt, NULL, &needed, NULL, 0) == -1) > + err(1, "sysctl-estimate"); > + if (needed == 0) > + break; > + if ((*buf = realloc(*buf, needed)) == NULL) > + err(1, NULL); > + if (sysctl(mib, mcnt, *buf, &needed, NULL, 0) == -1) { > + if (errno == ENOMEM) > + continue; > + err(1, "sysctl"); > + } > + break; > + } > + > + return needed; > +} > + > /* > * Print routing tables. > */ > @@ -116,7 +139,7 @@ p_rttables(int af, u_int tableid, char prio) > struct rt_msghdr *rtm; > char *buf = NULL, *next, *lim = NULL; > size_t needed; > - int mib[7]; > + int mib[7], mcnt; > struct sockaddr *sa; > > mib[0] = CTL_NET; > @@ -126,22 +149,10 @@ p_rttables(int af, u_int tableid, char prio) > mib[4] = NET_RT_DUMP; > mib[5] = prio; > mib[6] = tableid; > + mcnt = 7; > > - while (1) { > - if (sysctl(mib, 7, NULL, &needed, NULL, 0) == -1) > - err(1, "route-sysctl-estimate"); > - if (needed == 0) > - break; > - if ((buf = realloc(buf, needed)) == NULL) > - err(1, NULL); > - if (sysctl(mib, 7, buf, &needed, NULL, 0) == -1) { > - if (errno == ENOMEM) > - continue; > - err(1, "sysctl of routing table"); > - } > - lim = buf + needed; > - break; > - } > + needed = get_sysctl(mib, mcnt, &buf); > + lim = buf + needed; > > if (pledge("stdio dns", NULL) == -1) > err(1, "pledge"); > @@ -156,9 +167,8 @@ p_rttables(int af, u_int tableid, char prio) > sa = (struct sockaddr *)(next + rtm->rtm_hdrlen); > p_rtentry(rtm); > } > - free(buf); > - buf = NULL; > } > + free(buf); > } > > /* > diff --git show.h show.h > index 03999b7fdd7..461d5a39c5e 100644 > --- show.h > +++ show.h > @@ -34,6 +34,7 @@ void p_sockaddr(struct sockaddr *, struct sockaddr > *, int, int); > char *routename(struct sockaddr *); > char *netname(struct sockaddr *, struct sockaddr *); > char *mpls_op(u_int32_t); > +size_tget_sysctl(const int *, u_int, char **); > > extern int nflag; > extern int Fflag; > > > -- > I'm not entirely sure you are real. >
Re: route(8): stop debugging route monitor
Florian Obser(flor...@openbsd.org) on 2018.04.30 18:27:52 +0200: > The -d flag should be a no-op in monitor mode since it does not modify > the routing table. > > However, if -d is provided route monitor lists all interfaces and > their associated addresses and exits. This is confusing, unexpected > and no longer needed (if ever). > > Make -d a proper no-op for route monitor and get rid of the interfaces > function which didn't use the correct sysctl idiom anyway. > > OK? if nobody complains about loosing that "feature", ok benno@ > diff --git route.c route.c > index 85e76621dd3..738de3a8cde 100644 > --- route.c > +++ route.c > @@ -115,7 +115,6 @@ intrtmsg(int, int, int, uint8_t); > __dead void usage(char *); > void set_metric(char *, int); > void inet_makenetandmask(u_int32_t, struct sockaddr_in *, int); > -void interfaces(void); > void getlabel(char *); > int gettable(const char *); > int rdomain(int, char **); > @@ -1069,36 +1068,6 @@ prefixlen(int af, char *s) > return (len == max); > } > > -void > -interfaces(void) > -{ > - size_t needed; > - int mib[6]; > - char *buf = NULL, *lim, *next; > - struct rt_msghdr *rtm; > - > - mib[0] = CTL_NET; > - mib[1] = PF_ROUTE; > - mib[2] = 0; /* protocol */ > - mib[3] = 0; /* wildcard address family */ > - mib[4] = NET_RT_IFLIST; > - mib[5] = 0; /* no flags */ > - if (sysctl(mib, 6, NULL, &needed, NULL, 0) < 0) > - err(1, "route-sysctl-estimate"); > - if (needed) { > - if ((buf = malloc(needed)) == NULL) > - err(1, "malloc"); > - if (sysctl(mib, 6, buf, &needed, NULL, 0) < 0) > - err(1, "actual retrieval of interface table"); > - lim = buf + needed; > - for (next = buf; next < lim; next += rtm->rtm_msglen) { > - rtm = (struct rt_msghdr *)next; > - print_rtmsg(rtm, rtm->rtm_msglen); > - } > - free(buf); > - } > -} > - > void > monitor(int argc, char *argv[]) > { > @@ -1107,10 +1076,6 @@ monitor(int argc, char *argv[]) > time_t now; > > verbose = 1; > - if (debugonly) { > - interfaces(); > - exit(0); > - } > for (;;) { > if ((n = read(s, msg, sizeof(msg))) == -1) { > if (errno == EINTR) > > -- > I'm not entirely sure you are real. >
Re: 5GHz AP RSSI measurement problem
On Tue, May 01, 2018 at 10:22:22AM +0100, Stuart Henderson wrote: > On 2018/05/01 10:48, Stefan Sperling wrote: > > > On 2018/04/30 11:08, Stefan Sperling wrote: > > > > Derp. A dBm value of -10 would of course be better than -60. > > > > > > > > Whatever the numbers shown by tcpdump really mean, the probe response's > > > > one is better!!! > > > > > > Better as in "more accurate". But as the reported value is ridiculously > > > high rather than too low, why wasn't 5GHz selected anyway? > > > > What I wrote in my first mail was not very clear (I wrote it in > > a hurry before leaving to catch a train). > > > > The kernel compares the RSSI values which are shown in debug output. > > For reference, the scan debug printfs below again show a 2GHz beacon > > vs. a 5GHz "low power" beacon, where measured RSSI on 2Ghz is represented > > as "58" and on 5Ghz is represented as "6": > > > > + b8:ee:0e:cb:b3:081 +58 54M ess privacy rsn "ESSID" > > + b8:ee:0e:cb:b3:09 112+6 54M ess privacy rsn "ESSID" > > > > The "non-reduced" Tx power frames, i.e. probe responses or beacons while > > a client is associated, closely matched Tx power seen on 2GHz: > > > > + b8:ee:0e:cb:b3:081 +58 54M ess privacy rsn "ESSID" > > + b8:ee:0e:cb:b3:09 112 +61 54M ess privacy rsn "ESSID" > > What do the values represent here? > > I've been reading them as sign-flipped dBm signal strength because > (before the +6 outlier) the numbers all matched that (i.e. -61dBm for > 5GHz and -58dBm for 2GHz looks right for an access point at typical > power levels about 30m away). I've done some quick digging: The values come from iwm(4). Which means they are derived as dBm from values reported by hardware and then converted into a percentage. See iwm_calc_rssi() and its caller iwm_rx_rx_mpdu(). I've been wondering if we should make iwm(4) report values in dBm instead of as a percentage. But a percentage may be more intuitive to users. I'll leave this as it is -- since pirofti@ told me he might be working on fixing the RSSI situation I'd rather leave such decisions to him. > I didn't interpret them as % in this case because it would be unusual > for the 5GHz signal to be stronger than the 2GHz though that is > possible and if that's the case, your diff makes sense. It seems % would be the correct interpretation. I was sitting right next to the AP and I'm not surprised that both bands show similar values at such close range.
Re: 5GHz AP RSSI measurement problem
On 2018/05/01 10:48, Stefan Sperling wrote: > > On 2018/04/30 11:08, Stefan Sperling wrote: > > > Derp. A dBm value of -10 would of course be better than -60. > > > > > > Whatever the numbers shown by tcpdump really mean, the probe response's > > > one is better!!! > > > > Better as in "more accurate". But as the reported value is ridiculously > > high rather than too low, why wasn't 5GHz selected anyway? > > What I wrote in my first mail was not very clear (I wrote it in > a hurry before leaving to catch a train). > > The kernel compares the RSSI values which are shown in debug output. > For reference, the scan debug printfs below again show a 2GHz beacon > vs. a 5GHz "low power" beacon, where measured RSSI on 2Ghz is represented > as "58" and on 5Ghz is represented as "6": > > + b8:ee:0e:cb:b3:081 +58 54M ess privacy rsn "ESSID" > + b8:ee:0e:cb:b3:09 112+6 54M ess privacy rsn "ESSID" > > The "non-reduced" Tx power frames, i.e. probe responses or beacons while > a client is associated, closely matched Tx power seen on 2GHz: > > + b8:ee:0e:cb:b3:081 +58 54M ess privacy rsn "ESSID" > + b8:ee:0e:cb:b3:09 112 +61 54M ess privacy rsn "ESSID" What do the values represent here? I've been reading them as sign-flipped dBm signal strength because (before the +6 outlier) the numbers all matched that (i.e. -61dBm for 5GHz and -58dBm for 2GHz looks right for an access point at typical power levels about 30m away). I didn't interpret them as % in this case because it would be unusual for the 5GHz signal to be stronger than the 2GHz though that is possible and if that's the case, your diff makes sense. > There is a huge amount of material in the 802.11 specs about tx power > management and spectrum management but I haven't read any of that. And even then more reading is needed to understand it all, the specs just provide the framework for implementation, but the actual requirements are in local regulations which aren't in the specs. But fortunately for us (at least in client mode ;) this is mostly AP-led.
Re: rssi comparison threshold
On Mon, Apr 30, 2018 at 08:57:23AM +0200, Peter Hessler wrote: > On 2018 Apr 29 (Sun) at 11:51:26 +0200 (+0200), Stefan Sperling wrote: > :This diff tries to avoid situations where background scans play > :ping-pong between different APs with nearly equal RSSI, as > :observed by phessler. > : > :Not all drivers represent RSSI values in dBm or percentage, so the > :diff includes the possibility for drivers to override the new RSSI > :comparison function. However, since the threshold is rather low > :applying this to all drivers for now should not do any harm, unless > :there is a driver where the RSSI value range is ridiculously small. > :I'm not aware of any such driver at present. > : > > I'm concerned about two things. > > The usage is confusing, because you pass two incompatible things to be > compared. I would prefer ieee80211_node_cmprssi(ic, rssi_one, rssi_two). Agreed. Updated diff below. I've chosen to make it compare two nodes since the function lives in the nodes namespace. It makes the function a bit less flexible as it cannot be used to compare naked rssi values. But that's not really needed for the problem at hand; after all, we want to compare APs. > Also, in the case where ni->ni_rssi has a very weak signal, the second > comparison can underflow. Indeed. Thanks for catching this! Index: ieee80211_node.c === RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v retrieving revision 1.129 diff -u -p -r1.129 ieee80211_node.c --- ieee80211_node.c28 Apr 2018 14:49:07 - 1.129 +++ ieee80211_node.c30 Apr 2018 07:30:43 - @@ -67,6 +67,8 @@ u_int8_t ieee80211_node_getrssi(struct i const struct ieee80211_node *); int ieee80211_node_checkrssi(struct ieee80211com *, const struct ieee80211_node *); +int ieee80211_node_cmprssi(struct ieee80211com *, +const struct ieee80211_node *, const struct ieee80211_node *); void ieee80211_setup_node(struct ieee80211com *, struct ieee80211_node *, const u_int8_t *); void ieee80211_free_node(struct ieee80211com *, struct ieee80211_node *); @@ -133,6 +135,7 @@ ieee80211_node_attach(struct ifnet *ifp) ic->ic_node_copy = ieee80211_node_copy; ic->ic_node_getrssi = ieee80211_node_getrssi; ic->ic_node_checkrssi = ieee80211_node_checkrssi; + ic->ic_node_cmprssi = ieee80211_node_cmprssi; ic->ic_scangen = 1; ic->ic_max_nnodes = ieee80211_cache_size; @@ -761,12 +764,15 @@ ieee80211_end_scan(struct ifnet *ifp) if (ic->ic_caps & IEEE80211_C_SCANALLBAND) { if (IEEE80211_IS_CHAN_2GHZ(ni->ni_chan) && - (selbs2 == NULL || ni->ni_rssi > selbs2->ni_rssi)) + (selbs2 == NULL || + ic->ic_node_cmprssi(ic, ni, selbs2) > 0)) selbs2 = ni; else if (IEEE80211_IS_CHAN_5GHZ(ni->ni_chan) && - (selbs5 == NULL || ni->ni_rssi > selbs5->ni_rssi)) + (selbs5 == NULL || + ic->ic_node_cmprssi(ic, ni, selbs5) > 0)) selbs5 = ni; - } else if (selbs == NULL || ni->ni_rssi > selbs->ni_rssi) + } else if (selbs == NULL || + ic->ic_node_cmprssi(ic, ni, selbs) > 0) selbs = ni; } @@ -782,9 +788,12 @@ ieee80211_end_scan(struct ifnet *ifp) */ if (selbs5 && selbs5->ni_rssi > min_5ghz_rssi) selbs = selbs5; - else if (selbs5 && selbs2) - selbs = (selbs5->ni_rssi >= selbs2->ni_rssi ? selbs5 : selbs2); - else if (selbs2) + else if (selbs5 && selbs2) { + if (ic->ic_node_cmprssi(ic, selbs5, selbs2) >= 0) + selbs = selbs5; + else + selbs = selbs2; + } else if (selbs2) selbs = selbs2; else if (selbs5) selbs = selbs5; @@ -989,6 +998,38 @@ ieee80211_node_checkrssi(struct ieee8021 IEEE80211_RSSI_THRES_2GHZ : IEEE80211_RSSI_THRES_5GHZ; return (ni->ni_rssi >= (u_int8_t)thres); +} + +/* + * Determine if RSSI values of two nodes are significantly different. + * This function assumes RSSI values are represented either in dBm or + * as a percentage of ic_max_rssi. Drivers should override this function + * in case their RSSI values use a different representation. + */ +int +ieee80211_node_cmprssi(struct ieee80211com *ic, +const struct ieee80211_node *ni1, const struct ieee80211_node *ni2) +{ + uint8_t thres; + + if (ic->ic_max_rssi) + thres = IEEE80211_RSSI_CMP_THRES_RATIO; + else + thres = IEEE80211_RSSI_CMP_THRES_DBM; + + /* cap threshold to prevent overflow in calculations below */ + while (thres > 0) { + if (ni1->ni_rssi + thres >= ni1
Re: 5GHz AP RSSI measurement problem
On Mon, Apr 30, 2018 at 01:35:59PM +0100, Stuart Henderson wrote: > On 2018/04/30 10:55, Stefan Sperling wrote: > > The AP sends 5GHz beacons with a ridicously low RSSI while no client > > is connected, and OpenBSD prefers the 2GHz band... > > Surely it has to be the receiver that is adding the RSSI infornation? > The AP can't know. Indeed. What I meant is that this AP seems to send 5Ghz beacons with reduced Tx power for some reason. > Seems it would either have to be the client side measuring incorrectly, > or the AP is really sending beacons at low power. If the latter, that > _could_ be a mechanism to reduce power consumption in idle mode... > Let them connect at 2GHz then whack up the power and steer them towards > 5GHz after association... Yes, something like that seems to be happening. It could also be related to radar since low power beacons would carry less risk of causing interference with radar on channel 112. There is a huge amount of material in the 802.11 specs about tx power management and spectrum management but I haven't read any of that. > Anyway it would be interesting to see how a spectrum analyser looks with > this access point :) For all intents and purposes, trusting the RSSI reported by Intel hardware seems good enough to me :) > On 2018/04/30 11:08, Stefan Sperling wrote: > > Derp. A dBm value of -10 would of course be better than -60. > > > > Whatever the numbers shown by tcpdump really mean, the probe response's > > one is better!!! > > Better as in "more accurate". But as the reported value is ridiculously > high rather than too low, why wasn't 5GHz selected anyway? What I wrote in my first mail was not very clear (I wrote it in a hurry before leaving to catch a train). The kernel compares the RSSI values which are shown in debug output. For reference, the scan debug printfs below again show a 2GHz beacon vs. a 5GHz "low power" beacon, where measured RSSI on 2Ghz is represented as "58" and on 5Ghz is represented as "6": + b8:ee:0e:cb:b3:081 +58 54M ess privacy rsn "ESSID" + b8:ee:0e:cb:b3:09 112+6 54M ess privacy rsn "ESSID" The "non-reduced" Tx power frames, i.e. probe responses or beacons while a client is associated, closely matched Tx power seen on 2GHz: + b8:ee:0e:cb:b3:081 +58 54M ess privacy rsn "ESSID" + b8:ee:0e:cb:b3:09 112 +61 54M ess privacy rsn "ESSID" In the above cases, the RSSI value comparisons which happened in the kernel were 6 < 58 without my diff and 61 < 58 with my diff. We never pick the 5GHz AP in the former situation. Of course, there is a race: We would pick the 5GHz AP if its node record was created due to a probe response and no subsequent beacon had reset ni_rssi to a low value before we choose an AP. This occasionally happened in my observations but most of the time we picked the 2GHz AP. In my first mail, values I've quoted from scan debug output and values I've shown from tcpdump were derived from different frames. Debug output for those frames captured by tcpdump would have been 10 and 60 rather than 6 and 61, I suppose.