Re: Better/more upd(4) timedelta sensors

2014-12-19 Thread Martin Pieuchot
Hello David,

On 18/12/14(Thu) 00:45, David Higgs wrote:
 While my device does not seem to provide AtRateTimeToFull or 
 AtRateTimeToEmpty, it does have RunTimeToEmpty.  Then I found that 
 SENSOR_TIMEDELTA values are in nanoseconds and that scaling for them was 
 never implemented correctly.
 
 I am confused by the spec [1], though; see 4.2.5 - Battery Measures.  The 
 reported values are supposedly in minutes but hid_info.unit is 0x1001 
 (seconds) and observation of RunTimeToEmpty appears to agree.  I don’t see 
 any other drivers in the tree that pay attention to unit or unit_exponent 
 fields, and don’t want to go down a rabbit hole if there’s no interest.

 As usual, feedback is welcome.

Table 1 of section 3.2.3 indicates that the physical unit for time
reports are in seconds.  So I guess it is a mistake in the document.

I like your diff but I'd prefer to see the scaling done based on the
unit rather than the name.  Why not adding new defines for the various
Physical Unit defined in the table and do the switch on hitem.unit? And
what about the exponent?


 [1] http://www.usb.org/developers/hidpage/pdcv10.pdf
 
 --david
 
 
 Index: upd.c
 ===
 RCS file: /cvs/src/sys/dev/usb/upd.c,v
 retrieving revision 1.12
 diff -u -p -r1.12 upd.c
 --- upd.c 11 Dec 2014 18:50:32 -  1.12
 +++ upd.c 18 Dec 2014 05:02:30 -
 @@ -66,7 +66,11 @@ static struct upd_usage_entry upd_usage_
   { HUP_BATTERY,  HUB_AC_PRESENT,
   SENSOR_INDICATOR,ACPresent },
   { HUP_BATTERY,  HUB_ATRATE_TIMETOFULL,
 - SENSOR_TIMEDELTA,AtRateTimeToFull }
 + SENSOR_TIMEDELTA,AtRateTimeToFull },
 + { HUP_BATTERY,  HUB_ATRATE_TIMETOEMPTY,
 + SENSOR_TIMEDELTA,AtRateTimeToEmpty },
 + { HUP_BATTERY,  HUB_RUNTIMETO_EMPTY,
 + SENSOR_TIMEDELTA,RunTimeToEmpty },
  };
  
  struct upd_report {
 @@ -322,9 +326,9 @@ upd_update_sensors(struct upd_softc *sc,
  int repid)
  {
   struct upd_sensor   *sensor;
 - ulong   hdata, batpres;
 - ulong   adjust;
 - int i;
 + int64_t adjust;
 + ulong   batpres;
 + int hdata, i;
  
   sensor = upd_lookup_sensor(sc, HUP_BATTERY, HUB_BATTERY_PRESENT);
   batpres = sensor ? sensor-ksensor.value : -1;
 @@ -353,6 +357,11 @@ upd_update_sensors(struct upd_softc *sc,
   case HUB_FULLCHARGE_CAPACITY:
   adjust = 1000; /* scale adjust */
   break;
 + case HUB_ATRATE_TIMETOFULL:
 + case HUB_ATRATE_TIMETOEMPTY:
 + case HUB_RUNTIMETO_EMPTY:
 + adjust = 10LL; /* XXX not minutes? */
 + break;
   default:
   adjust = 1; /* no scale adjust */
   break;
 @@ -363,7 +372,7 @@ upd_update_sensors(struct upd_softc *sc,
   sensor-ksensor.value = hdata * adjust;
   sensor-ksensor.status = SENSOR_S_OK;
   sensor-ksensor.flags = ~SENSOR_FINVALID;
 - DPRINTF((%s: hidget data: %lu\n,
 + DPRINTF((%s: hidget data: %d\n,
   sc-sc_sensordev.xname, hdata));
   }
  }
 
 



Re: Better/more upd(4) timedelta sensors

2014-12-19 Thread David Higgs
On Fri, Dec 19, 2014 at 7:17 AM, Martin Pieuchot mpieuc...@nolizard.org wrote:
 Hello David,

 On 18/12/14(Thu) 00:45, David Higgs wrote:
 While my device does not seem to provide AtRateTimeToFull or 
 AtRateTimeToEmpty, it does have RunTimeToEmpty.  Then I found that 
 SENSOR_TIMEDELTA values are in nanoseconds and that scaling for them was 
 never implemented correctly.

 I am confused by the spec [1], though; see 4.2.5 - Battery Measures.  The 
 reported values are supposedly in minutes but hid_info.unit is 0x1001 
 (seconds) and observation of RunTimeToEmpty appears to agree.  I don't see 
 any other drivers in the tree that pay attention to unit or unit_exponent 
 fields, and don't want to go down a rabbit hole if there's no interest.

 As usual, feedback is welcome.

 Table 1 of section 3.2.3 indicates that the physical unit for time
 reports are in seconds.  So I guess it is a mistake in the document.

 I like your diff but I'd prefer to see the scaling done based on the
 unit rather than the name.  Why not adding new defines for the various
 Physical Unit defined in the table and do the switch on hitem.unit? And
 what about the exponent?


Thanks for looking at this.  I've rethought my approach to upd(4) a
bit, and will be addressing bugs prior to adding new sensors.  There
will be a diff to address units in a more coherent fashion soon.

--david



pstat segfault

2014-12-19 Thread Stuart Cassoff
$ pstat -T
221/7030 open files
Segmentation fault



Fix upd(4) sensor refresh

2014-12-19 Thread David Higgs
Split upd_update_sensors() behavior to gather all values prior to updating 
sensor contents.

Because sensors were updated in report_ID order, it could take multiple passes 
of upd_refresh() to update some sensors.  Specifically, battery-related sensors 
“prior” to a change in BatteryPresent would be stale/incorrect until the 2nd 
call to upd_refresh().  Also, change a confusing DPRINTF of report_ID from 
un-prefixed hex to decimal, to be consistent.

This fix is needed for future improvements, where output or behavior of certain 
sensors depends on the current value of more than one report (e.g. 
RemainingCapacity should actually depend on CapacityMode).

As always, feedback is welcome.

--david


Index: upd.c
===
RCS file: /cvs/src/sys/dev/usb/upd.c,v
retrieving revision 1.12
diff -u -p -r1.12 upd.c
--- upd.c   11 Dec 2014 18:50:32 -  1.12
+++ upd.c   19 Dec 2014 05:33:14 -
@@ -77,6 +77,8 @@ struct upd_report {
 struct upd_sensor {
struct ksensor  ksensor;
struct hid_item hitem;
+   int raw_value;
+   int value_read;
int attached;
 };
 
@@ -98,7 +100,8 @@ void upd_attach(struct device *, struct 
 int  upd_detach(struct device *, int);
 
 void upd_refresh(void *);
-void upd_update_sensors(struct upd_softc *, uint8_t *, unsigned int, int);
+void upd_update_values(struct upd_softc *, uint8_t *, unsigned int, int);
+void upd_update_sensors(struct upd_softc *);
 void upd_intr(struct uhidev *, void *, uint);
 struct upd_usage_entry *upd_lookup_usage_entry(const struct hid_item *);
 struct upd_sensor *upd_lookup_sensor(struct upd_softc *, int, int);
@@ -275,7 +278,7 @@ upd_refresh(void *arg)
UHID_FEATURE_REPORT, repid, buf, report-size);
 
if (actlen == -1) {
-   DPRINTF((upd: failed to get report id=%02x\n, repid));
+   DPRINTF((upd: failed to get report id=%d\n, repid));
continue;
}
 
@@ -283,8 +286,10 @@ upd_refresh(void *arg)
if (actlen  report-size)
report-size = actlen;
 
-   upd_update_sensors(sc, buf, report-size, repid);
+   upd_update_values(sc, buf, report-size, repid);
}
+
+   upd_update_sensors(sc);
 }
 
 struct upd_usage_entry *
@@ -318,33 +323,48 @@ upd_lookup_sensor(struct upd_softc *sc, 
 }
 
 void
-upd_update_sensors(struct upd_softc *sc, uint8_t *buf, unsigned int len,
+upd_update_values(struct upd_softc *sc, uint8_t *buf, unsigned int len,
 int repid)
 {
struct upd_sensor   *sensor;
-   ulong   hdata, batpres;
-   ulong   adjust;
int i;
 
+   for (i = 0; i  sc-sc_num_sensors; i++) {
+   sensor = sc-sc_sensors[i];
+   if (sensor-hitem.report_ID == repid  sensor-attached) {
+   sensor-value_read = 1;
+   sensor-raw_value =
+   hid_get_data(buf, len, sensor-hitem.loc);
+   DPRINTF((%s: repid=%d, %s value=%d\n,
+   sc-sc_sensordev.xname, repid,
+   sensor-ksensor.desc, sensor-raw_value));
+   }
+   }
+}
+
+void
+upd_update_sensors(struct upd_softc *sc)
+{
+   struct upd_sensor   *sensor;
+   ulong   adjust;
+   int i, batpres = 0;
+
sensor = upd_lookup_sensor(sc, HUP_BATTERY, HUB_BATTERY_PRESENT);
-   batpres = sensor ? sensor-ksensor.value : -1;
+   if (sensor  sensor-value_read)
+   batpres = sensor-raw_value;
 
for (i = 0; i  sc-sc_num_sensors; i++) {
sensor = sc-sc_sensors[i];
-   if (!(sensor-hitem.report_ID == repid  sensor-attached))
+   sensor-ksensor.flags |= SENSOR_FINVALID;
+   sensor-ksensor.status = SENSOR_S_UNKNOWN;
+   if (!sensor-value_read)
continue;
 
-   /* invalidate battery dependent sensors */
+   /* ignore battery sensors when no battery present */
if (HID_GET_USAGE_PAGE(sensor-hitem.usage) == HUP_BATTERY 
-   batpres = 0) {
-   /* exception to the battery sensor itself */
-   if (HID_GET_USAGE(sensor-hitem.usage) !=
-   HUB_BATTERY_PRESENT) {
-   sensor-ksensor.status = SENSOR_S_UNKNOWN;
-   sensor-ksensor.flags |= SENSOR_FINVALID;
-   continue;
-   }
-   }
+   HID_GET_USAGE(sensor-hitem.usage) != HUB_BATTERY_PRESENT 
+   !batpres)
+   continue;
 
switch 

Re: Dell R630 high interrupts on acpi0

2014-12-19 Thread Jonathan Matthew
On Thu, Dec 18, 2014 at 11:14:54PM +0100, Frederic Nowak wrote:
 Hi!
 
 The diff for extracting memory ranges from ACPI was obviously not tested
 with ACPI disabled...so we definitely have to check if the values in
 pcimem_range make some sense. The diff below now uses the old values in
 case ACPI is disabled or does not return valid values.
 
 I hope this is somewhat interesting for someone else as well :)
 Please let me know if this is just noise or if there is anything else I
 am missing.

This looks like the right direction, except I think we only really want to use
the ACPI range information to add new ranges to the existing one that covers
the 36-bit address space, rather than relying on it for everything.  No point
relying on the ACPI stuff being correct if we don't need to.

I hacked this around a bit today and ended up with the diff below.  This only
deals with ranges outside the 36-bit space, so it makes no difference on most
machines, and on the r630 that needs it, it adds two ranges,
0x0300 to 0x033F and 0x0340 to
0x037F.


Index: arch/amd64/pci/pci_machdep.c
===
RCS file: /cvs/src/sys/arch/amd64/pci/pci_machdep.c,v
retrieving revision 1.60
diff -u -p -r1.60 pci_machdep.c
--- arch/amd64/pci/pci_machdep.c16 Dec 2014 23:13:20 -  1.60
+++ arch/amd64/pci/pci_machdep.c19 Dec 2014 12:54:44 -
@@ -90,12 +90,17 @@
 #include dev/pci/ppbreg.h
 
 #include ioapic.h
+#include acpi.h
 
 #if NIOAPIC  0
 #include machine/i82093var.h
 #include machine/mpbiosvar.h
 #endif
 
+#if NACPI  0
+#include dev/acpi/acpivar.h
+#endif
+
 /*
  * Memory Mapped Configuration space access.
  *
@@ -622,17 +627,13 @@ pci_init_extents(void)
 * here.  As long as vendors continue to support
 * 32-bit operating systems, we should never see BARs
 * outside that region.
-*
-* Dell 13G servers have important devices outside the
-* 36-bit address space.  Until we can extract the address
-* ranges from ACPI, expand the allowed range to suit.
 */
pcimem_ex = extent_create(pcimem, 0, 0xUL,
M_DEVBUF, NULL, 0, EX_NOWAIT);
if (pcimem_ex == NULL)
return;
-   extent_alloc_region(pcimem_ex, 0x400UL,
-   0xfc00UL, EX_NOWAIT);
+   extent_alloc_region(pcimem_ex, 0x10UL,
+   0xfff0UL, EX_NOWAIT);
 
for (bmp = bios_memmap; bmp-type != BIOS_MAP_END; bmp++) {
/*
@@ -657,6 +658,14 @@ pci_init_extents(void)
/* Take out the video buffer area and BIOS areas. */
extent_alloc_region(pcimem_ex, IOM_BEGIN, IOM_SIZE,
EX_CONFLICTOK | EX_NOWAIT);
+
+#if NACPI  0
+   /*
+* Free up any regions outside the 36-bit address space
+* specified via ACPI.
+*/
+   acpi_pciroots_extents(pcimem_ex, 0x10UL);
+#endif
}
 
if (pcibus_ex == NULL) {
@@ -665,7 +674,6 @@ pci_init_extents(void)
}
 }
 
-#include acpi.h
 #if NACPI  0
 void acpi_pci_match(struct device *, struct pci_attach_args *);
 pcireg_t acpi_pci_min_powerstate(pci_chipset_tag_t, pcitag_t);
Index: dev/acpi/acpivar.h
===
RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v
retrieving revision 1.79
diff -u -p -r1.79 acpivar.h
--- dev/acpi/acpivar.h  8 Dec 2014 07:12:37 -   1.79
+++ dev/acpi/acpivar.h  19 Dec 2014 12:54:44 -
@@ -47,6 +47,7 @@ extern u_int8_t acpi_lapic_flags[LAPIC_M
 struct klist;
 struct acpiec_softc;
 struct acpipwrres_softc;
+struct extent;
 
 struct acpivideo_softc {
struct device sc_dev;
@@ -357,6 +358,7 @@ int acpi_acquire_glk(uint32_t *);
 intacpi_release_glk(uint32_t *);
 
 void   acpi_pciroots_attach(struct device *, void *, cfprint_t);
+void   acpi_pciroots_extents(struct extent *, u_int64_t);
 
 #endif
 
Index: dev/acpi/amltypes.h
===
RCS file: /cvs/src/sys/dev/acpi/amltypes.h,v
retrieving revision 1.40
diff -u -p -r1.40 amltypes.h
--- dev/acpi/amltypes.h 7 Sep 2012 19:19:59 -   1.40
+++ dev/acpi/amltypes.h 19 Dec 2014 12:54:44 -
@@ -346,6 +346,12 @@ struct aml_value {
 #define aml_pkglen(v)  ((v)-length)
 #define aml_pkgval(v,i)((v)-v_package[(i)])
 
+struct acpi_pci_range {
+   SLIST_ENTRY(acpi_pci_range) next;
+   uint64_tbase;
+   uint64_tlen;
+};
+
 struct acpi_pci {
TAILQ_ENTRY(acpi_pci)   next;
 
@@ -362,6 +368,8 @@ struct acpi_pci {
int _s3w;
int  

relayd: source-hash and random for redirections

2014-12-19 Thread Reyk Floeter
Hi,

the following diff adds support for source-hash and random modes to
relayd's redirections.  It depends on the latest pf change.

Example:
---snip---
redirect foo {
listen on 0.0.0.0 port 8080
forward to foo check tcp port 80 mode source-hash
#   forward to foo check tcp port 80 mode source-hash my-fixed-key
}
---snap---

Reyk

Index: usr.sbin/relayd/parse.y
===
RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
retrieving revision 1.197
diff -u -p -u -p -r1.197 parse.y
--- usr.sbin/relayd/parse.y 18 Dec 2014 20:55:01 -  1.197
+++ usr.sbin/relayd/parse.y 19 Dec 2014 13:45:45 -
@@ -493,6 +493,9 @@ rdropts_l   : rdropts_l rdroptsl nl
 
 rdroptsl   : forwardmode TO tablespec interface{
if (hashkey != NULL) {
+   memcpy(rdr-conf.key,
+   hashkey, sizeof(rdr-conf.key));
+   rdr-conf.flags |= F_HASHKEY;
free(hashkey);
hashkey = NULL;
}
@@ -776,15 +779,15 @@ tableopts : CHECK tablecheck
switch ($2) {
case RELAY_DSTMODE_LOADBALANCE:
case RELAY_DSTMODE_HASH:
-   case RELAY_DSTMODE_RANDOM:
-   case RELAY_DSTMODE_SRCHASH:
if (rdr != NULL) {
yyerror(mode not supported 
for redirections);
YYERROR;
}
/* FALLTHROUGH */
+   case RELAY_DSTMODE_RANDOM:
case RELAY_DSTMODE_ROUNDROBIN:
+   case RELAY_DSTMODE_SRCHASH:
dstmode = $2;
break;
case RELAY_DSTMODE_LEASTSTATES:
Index: usr.sbin/relayd/pfe_filter.c
===
RCS file: /cvs/src/usr.sbin/relayd/pfe_filter.c,v
retrieving revision 1.53
diff -u -p -u -p -r1.53 pfe_filter.c
--- usr.sbin/relayd/pfe_filter.c27 Apr 2013 16:39:30 -  1.53
+++ usr.sbin/relayd/pfe_filter.c19 Dec 2014 13:45:45 -
@@ -485,9 +485,15 @@ sync_ruleset(struct relayd *env, struct 
}
 
switch (rdr-conf.mode) {
+   case RELAY_DSTMODE_RANDOM:
+   rio.rule.rdr.opts = PF_POOL_RANDOM;
+   break;
case RELAY_DSTMODE_ROUNDROBIN:
rio.rule.rdr.opts = PF_POOL_ROUNDROBIN;
break;
+   case RELAY_DSTMODE_SRCHASH:
+   rio.rule.rdr.opts = PF_POOL_SRCHASH;
+   break;
case RELAY_DSTMODE_LEASTSTATES:
rio.rule.rdr.opts = PF_POOL_LEASTSTATES;
break;
@@ -497,6 +503,9 @@ sync_ruleset(struct relayd *env, struct 
}
if (rdr-conf.flags  F_STICKY)
rio.rule.rdr.opts |= PF_POOL_STICKYADDR;
+   if (rdr-conf.flags  F_HASHKEY)
+   memcpy(rio.rule.rdr.key.key32, rdr-conf.key.data,
+   sizeof(rio.rule.rdr.key.key32));
 
if (rio.rule.rt == PF_ROUTETO) {
memcpy(rio.rule.route, rio.rule.rdr,
Index: usr.sbin/relayd/relayd.conf.5
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v
retrieving revision 1.155
diff -u -p -u -p -r1.155 relayd.conf.5
--- usr.sbin/relayd/relayd.conf.5   18 Dec 2014 21:26:09 -  1.155
+++ usr.sbin/relayd/relayd.conf.5   19 Dec 2014 13:45:45 -
@@ -413,7 +413,7 @@ the source IP address of the client, and
 This mode is only supported by relays.
 .It Ic mode random
 Distributes the outgoing connections randomly through all active hosts.
-This mode is only supported by relays.
+This mode is supported by redirections and relays.
 .It Ic mode roundrobin
 Distributes the outgoing connections using a round-robin scheduler
 through all active hosts.
@@ -423,7 +423,7 @@ This mode is supported by redirections a
 Balances the outgoing connections across the active hosts based on the
 .Ar key
 and the source IP address of the client.
-This mode is only supported by relays.
+This mode is supported by redirections and relays.
 .El
 .Pp
 The optional



Re: Fix upd(4) sensor refresh

2014-12-19 Thread Martin Pieuchot
On 19/12/14(Fri) 08:04, David Higgs wrote:
 Split upd_update_sensors() behavior to gather all values prior to updating 
 sensor contents.
 
 Because sensors were updated in report_ID order, it could take multiple 
 passes of upd_refresh() to update some sensors.  Specifically, 
 battery-related sensors “prior” to a change in BatteryPresent would be 
 stale/incorrect until the 2nd call to upd_refresh().  Also, change a 
 confusing DPRINTF of report_ID from un-prefixed hex to decimal, to be 
 consistent.

What you're saying is that querying HUB_BATTERY_PRESENT after 6 sensors
that depend on its value does not make sense, right?

Well in this case I don't think that sensors should be queried in
reportID order.  I even don't think that sensors should be queried at
all if we're going to discard the result anyway.  It might not matter
that much right now, but if the number of queried sensors grow
s significantly we might end up waiting a lot of time in upd_refresh()
prior to updating anything.

 This fix is needed for future improvements, where output or behavior of 
 certain sensors depends on the current value of more than one report (e.g. 
 RemainingCapacity should actually depend on CapacityMode).

But do you need to read the value of CapacityMode more than once, at the
begging or when you modify it? 

Anyway, when we discussed that with Andre, I suggested to have a table
containing all sensors depending on an other one.   This way you can
start by querying HUB_BATTERY_PRESENT and if it is present, query the
others.   Another advantage is that it will simplify the code because
all the sensors in a table will be in the same page.

Since you're doing some great job with upd(4), here's another idea.  Did
you consider updating the values of the sensors via an asynchronous
function and a callback?  This way the thread does not need to way for
the previous queries.   That would involve adding a new function, let's
say uhidev_get_report_async() that takes a custom callback since there's
no such API for the moment.

Regards,
Martin



Re: Dell R630 high interrupts on acpi0

2014-12-19 Thread Mark Kettenis
 Date: Fri, 19 Dec 2014 23:41:56 +1000
 From: Jonathan Matthew jonat...@d14n.org
 
 On Thu, Dec 18, 2014 at 11:14:54PM +0100, Frederic Nowak wrote:
  Hi!
  
  The diff for extracting memory ranges from ACPI was obviously not tested
  with ACPI disabled...so we definitely have to check if the values in
  pcimem_range make some sense. The diff below now uses the old values in
  case ACPI is disabled or does not return valid values.
  
  I hope this is somewhat interesting for someone else as well :)
  Please let me know if this is just noise or if there is anything else I
  am missing.
 
 This looks like the right direction, except I think we only really want to use
 the ACPI range information to add new ranges to the existing one that covers
 the 36-bit address space, rather than relying on it for everything.  No point
 relying on the ACPI stuff being correct if we don't need to.
 
 I hacked this around a bit today and ended up with the diff below.  This only
 deals with ranges outside the 36-bit space, so it makes no difference on most
 machines, and on the r630 that needs it, it adds two ranges,
 0x0300 to 0x033F and 0x0340 to
 0x037F.

This is going in the right direction.  The way I designed things
though was that the acpi code would build up the complete extent
purely from information provided by _CRS, and set pcimem_ex before
pci_init_extents() gets called.

 Index: arch/amd64/pci/pci_machdep.c
 ===
 RCS file: /cvs/src/sys/arch/amd64/pci/pci_machdep.c,v
 retrieving revision 1.60
 diff -u -p -r1.60 pci_machdep.c
 --- arch/amd64/pci/pci_machdep.c  16 Dec 2014 23:13:20 -  1.60
 +++ arch/amd64/pci/pci_machdep.c  19 Dec 2014 12:54:44 -
 @@ -90,12 +90,17 @@
  #include dev/pci/ppbreg.h
  
  #include ioapic.h
 +#include acpi.h
  
  #if NIOAPIC  0
  #include machine/i82093var.h
  #include machine/mpbiosvar.h
  #endif
  
 +#if NACPI  0
 +#include dev/acpi/acpivar.h
 +#endif
 +
  /*
   * Memory Mapped Configuration space access.
   *
 @@ -622,17 +627,13 @@ pci_init_extents(void)
* here.  As long as vendors continue to support
* 32-bit operating systems, we should never see BARs
* outside that region.
 -  *
 -  * Dell 13G servers have important devices outside the
 -  * 36-bit address space.  Until we can extract the address
 -  * ranges from ACPI, expand the allowed range to suit.
*/
   pcimem_ex = extent_create(pcimem, 0, 0xUL,
   M_DEVBUF, NULL, 0, EX_NOWAIT);
   if (pcimem_ex == NULL)
   return;
 - extent_alloc_region(pcimem_ex, 0x400UL,
 - 0xfc00UL, EX_NOWAIT);
 + extent_alloc_region(pcimem_ex, 0x10UL,
 + 0xfff0UL, EX_NOWAIT);
  
   for (bmp = bios_memmap; bmp-type != BIOS_MAP_END; bmp++) {
   /*
 @@ -657,6 +658,14 @@ pci_init_extents(void)
   /* Take out the video buffer area and BIOS areas. */
   extent_alloc_region(pcimem_ex, IOM_BEGIN, IOM_SIZE,
   EX_CONFLICTOK | EX_NOWAIT);
 +
 +#if NACPI  0
 + /*
 +  * Free up any regions outside the 36-bit address space
 +  * specified via ACPI.
 +  */
 + acpi_pciroots_extents(pcimem_ex, 0x10UL);
 +#endif
   }
  
   if (pcibus_ex == NULL) {
 @@ -665,7 +674,6 @@ pci_init_extents(void)
   }
  }
  
 -#include acpi.h
  #if NACPI  0
  void acpi_pci_match(struct device *, struct pci_attach_args *);
  pcireg_t acpi_pci_min_powerstate(pci_chipset_tag_t, pcitag_t);
 Index: dev/acpi/acpivar.h
 ===
 RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v
 retrieving revision 1.79
 diff -u -p -r1.79 acpivar.h
 --- dev/acpi/acpivar.h8 Dec 2014 07:12:37 -   1.79
 +++ dev/acpi/acpivar.h19 Dec 2014 12:54:44 -
 @@ -47,6 +47,7 @@ extern u_int8_t acpi_lapic_flags[LAPIC_M
  struct klist;
  struct acpiec_softc;
  struct acpipwrres_softc;
 +struct extent;
  
  struct acpivideo_softc {
   struct device sc_dev;
 @@ -357,6 +358,7 @@ int   acpi_acquire_glk(uint32_t *);
  int  acpi_release_glk(uint32_t *);
  
  void acpi_pciroots_attach(struct device *, void *, cfprint_t);
 +void acpi_pciroots_extents(struct extent *, u_int64_t);
  
  #endif
  
 Index: dev/acpi/amltypes.h
 ===
 RCS file: /cvs/src/sys/dev/acpi/amltypes.h,v
 retrieving revision 1.40
 diff -u -p -r1.40 amltypes.h
 --- dev/acpi/amltypes.h   7 Sep 2012 19:19:59 -   1.40
 +++ dev/acpi/amltypes.h   19 Dec 2014 12:54:44 -
 @@ -346,6 +346,12 @@ struct aml_value {
  #define aml_pkglen(v)

Re: pstat segfault

2014-12-19 Thread Stuart Cassoff
On 12/19/14 07:18, Stuart Cassoff wrote:
 $ pstat -T
 221/7030 open files
 Segmentation fault

I suppose more info from me is desired.


$ gdb pstat
GNU gdb 6.3
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type show copying to see the conditions.
There is absolutely no warranty for GDB.  Type show warranty for details.
This GDB was configured as i386-unknown-openbsd5.6...
(no debugging symbols found)

(gdb) run -T
Starting program: /usr/sbin/pstat -T
(no debugging symbols found)
205/7030 open files

Program received signal SIGSEGV, Segmentation fault.
kvm_read (kd=0x0, kva=0, buf=0xcfbc72c4, len=8)
at /usr/src/lib/libkvm/kvm.c:845
845 if (ISALIVE(kd)) {
(gdb) bt
#0  kvm_read (kd=0x0, kva=0, buf=0xcfbc72c4, len=8)
at /usr/src/lib/libkvm/kvm.c:845
#1  0x181e9ce0 in ?? () from /usr/sbin/pstat
#2  0x in ?? ()



OpenBSD 5.6-current (GENERIC) #605: Thu Dec 18 01:56:16 MST 2014
dera...@i386.openbsd.org:/usr/src/sys/arch/i386/compile/GENERIC
cpu0: Intel(R) Pentium(R) 4 CPU 3.00GHz (GenuineIntel 686-class) 3 GHz
cpu0: 
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,CNXT-ID,xTPR,PERF
real mem  = 2683277312 (2558MB)
avail mem = 2627121152 (2505MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: AT/286+ BIOS, date 02/23/04, BIOS32 rev. 0 @ 0xffe90, SMBIOS 
rev. 2.3 @ 0xf0450 (65 entries)
bios0: vendor Dell Computer Corporation version A05 date 02/23/2004
bios0: Dell Computer Corporation Dimension 8300
acpi0 at bios0: rev 0
acpi0: sleep states S0 S1 S3 S4 S5
acpi0: tables DSDT FACP SSDT APIC BOOT
acpi0: wakeup devices VBTN(S4) PCI0(S3) USB0(S3) USB1(S3) USB2(S3) USB3(S3) 
PCI1(S5) KBD_(S3)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 199MHz
ioapic0 at mainbus0: apid 1 pa 0xfec0, version 20, 24 pins
ioapic0: misconfigured as apic 0, remapped to apid 1
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus 2 (PCI1)
acpicpu0 at acpi0
acpibtn0 at acpi0: VBTN
bios0: ROM list: 0xc/0xf800 0xcf800/0x1800! 0xd1000/0x3000
pci0 at mainbus0 bus 0: configuration mode 1 (bios)
pchb0 at pci0 dev 0 function 0 Intel 82875P Host rev 0x02
intelagp0 at pchb0
agp0 at intelagp0: aperture at 0xd000, size 0x1000
ppb0 at pci0 dev 1 function 0 Intel 82875P AGP rev 0x02
pci1 at ppb0 bus 1
vga1 at pci1 dev 0 function 0 NVIDIA GeForce FX 5200 rev 0xa1
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
uhci0 at pci0 dev 29 function 0 Intel 82801EB/ER USB rev 0x02: apic 1 int 16
uhci1 at pci0 dev 29 function 1 Intel 82801EB/ER USB rev 0x02: apic 1 int 19
uhci2 at pci0 dev 29 function 2 Intel 82801EB/ER USB rev 0x02: apic 1 int 18
uhci3 at pci0 dev 29 function 3 Intel 82801EB/ER USB rev 0x02: apic 1 int 16
ehci0 at pci0 dev 29 function 7 Intel 82801EB/ER USB2 rev 0x02: apic 1 int 23
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 Intel EHCI root hub rev 2.00/1.00 addr 1
ppb1 at pci0 dev 30 function 0 Intel 82801BA Hub-to-PCI rev 0xc2
pci2 at ppb1 bus 2
fxp0 at pci2 dev 8 function 0 Intel PRO/100 VE rev 0x02, i82562: apic 1 int 
20, address 00:0c:f1:cd:b7:8d
inphy0 at fxp0 phy 1: i82562ET 10/100 PHY, rev. 0
ichpcib0 at pci0 dev 31 function 0 Intel 82801EB/ER LPC rev 0x02
pciide0 at pci0 dev 31 function 1 Intel 82801EB/ER IDE rev 0x02: DMA, channel 
0 configured to compatibility, channel 1 configured to compatibility
wd0 at pciide0 channel 0 drive 0: ST380011A
wd0: 16-sector PIO, LBA48, 76293MB, 15625 sectors
wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 5
atapiscsi0 at pciide0 channel 1 drive 0
scsibus1 at atapiscsi0: 2 targets
cd0 at scsibus1 targ 0 lun 0: SAMSUNG, DVD-ROM SD-616E, F501 ATAPI 5/cdrom 
removable
cd0(pciide0:1:0): using PIO mode 4, Ultra-DMA mode 2
pciide1 at pci0 dev 31 function 2 Intel 82801EB SATA rev 0x02: DMA, channel 0 
configured to native-PCI, channel 1 configured to native-PCI
pciide1: using apic 1 int 18 for native-PCI interrupt
ichiic0 at pci0 dev 31 function 3 Intel 82801EB/ER SMBus rev 0x02: apic 1 int 
17
iic0 at ichiic0
spdmem0 at iic0 addr 0x50: 256MB DDR SDRAM non-parity PC3200CL3.0
spdmem1 at iic0 addr 0x51: 1GB DDR SDRAM non-parity PC3200CL3.0
spdmem2 at iic0 addr 0x52: 256MB DDR SDRAM non-parity PC3200CL3.0
spdmem3 at iic0 addr 0x53: 1GB DDR SDRAM non-parity PC3200CL3.0
auich0 at pci0 dev 31 function 5 Intel 82801EB/ER AC97 rev 0x02: apic 1 int 
17, ICH5 AC97
ac97: codec id 0x41445370 (Analog Devices AD1980)
ac97: codec features headphone, 20 bit DAC, No 3D Stereo
audio0 at auich0
usb1 at uhci0: USB revision 1.0
uhub1 at usb1 Intel UHCI root hub rev 1.00/1.00 addr 1

Re: pstat segfault

2014-12-19 Thread Ted Unangst
On Fri, Dec 19, 2014 at 13:04, Stuart Cassoff wrote:
 On 12/19/14 07:18, Stuart Cassoff wrote:
 $ pstat -T
 221/7030 open files
 Segmentation fault
 
 I suppose more info from me is desired.

I think that was sufficient. It should be fixed now.



Re: pstat segfault

2014-12-19 Thread Fred

On 12/19/14 18:12, Ted Unangst wrote:

On Fri, Dec 19, 2014 at 13:04, Stuart Cassoff wrote:

On 12/19/14 07:18, Stuart Cassoff wrote:

$ pstat -T
221/7030 open files
Segmentation fault


I suppose more info from me is desired.


I think that was sufficient. It should be fixed now.



Hi Ted,

It's fixed, but if I run pstat as an ordinary user I get:

port:fred /usr/src/usr.sbin/pstat ./pstat -T
pstat: kvm_openfiles: /dev/mem: Permission denied

rather than:

port:fred /usr/src/usr.sbin/pstat sudo ./pstat -T
Password:
1477/7030 open files
  34681 vnodes
0M/8311M swap space

But pstat -f works as expected as an ordinary user.

Cheers

Fred
PS This with pstat.c patched to 1.96



Re: Dell R630 high interrupts on acpi0

2014-12-19 Thread Hrvoje Popovski

On 17.12.2014. 6:34, Philip Guenther wrote:

Uh, ACPI *requires* that C1 exist.  The halt instruction is defined as
entering C1, so not having C1 would mean your CPU lacks a basic
manadatory ia32 instruction.  Hopefully the BIOS docs explain that
you're just disabling deep C-states or something like that.  If not,
yell at the company that made it.

With the exception of C1E, I wouldn't tell a BIOS to disable
C-states unless it was causing the OS to have a problem or you're
actively trying to use the computer to heat your house.  C1E is a
cross between C1 and C3; the issue is that bugs in multiple early
hardware implementations mean it'll behave poorly depending on exactly
how the OS handles it.  This is something to test...and then test
again with each release you install...


Thank you, this is very informative.
I have disabled C states because I have seen ifq.drops and netlivelocks
when I run udp tcpbench over ix interface and C-states are enabled. This 
is between two Dell R620. With Dell R630 I have bigger problems so i 
can't test them right now.


tcpbench client sends around 350kpps for 20s
tcpbench server gets around 212kpps

tcpbench udp server drops:
Without C states and C1E:
net.inet.ip.ifq.maxlen=2048
net.inet.ip.ifq.drops=0
kern.netlivelocks=3

With C states and C1E (DAPC):
net.inet.ip.ifq.maxlen=2048
net.inet.ip.ifq.drops=19143
kern.netlivelocks=86

so I explained to myself that C states are bad for packet bursts and 
these boxes will be uber high speed firewalls :)


and of course after reading you mail I only disable C1E and everything 
is fine.


With C states and without C1E
net.inet.ip.ifq.maxlen=2048
net.inet.ip.ifq.drops=0
kern.netlivelocks=3


Monitor/Mwait - Disabled


I would suggest leaving that on.  We ain't using it *right now*, but,
well, the source tree on my laptop is, and more than ever.  :-)



mwait is enabled :)



NTP

2014-12-19 Thread Theo de Raadt
The ntp daemon included in OpenBSD is our own openntpd, written
from scratch.

openntpd is not vulnerable.

Around 10 years ago it was written by Henning, at my request because
the ntpd source code scared the hell out of us.  At the time
communications with the ntp team showed they had little interest in
removing unused functionality from the ntp.org code, or any help with
our form of source code audit.

Because it was a rewrite, the major benefit in openntpd is that it
priviledge seperated.  If problems like these were found, they would
not be realistically exploitable.  Furthermore openntpd is a modern
piece of code 5000 lines long written using best known practices of
the time, whereas ntp.org's codebase is reportedly 100,000 lines of
unknown or largely unused code, poorly smithed in the past when these
kinds of programming mistakes were not a significant consideration.

This might be a good time to circle the conversation back to the
common practice of:

 srand(time(NULL));

Sorry, getting really jaded.  When will the software vendors WAKE THE
HELL UP?  This is not 2000 anymore.

It has become abundantly clear that it is very difficult to push
lessons regarding better software practices into the greater open
source community and the vendors who live off the teat.



Re: NTP

2014-12-19 Thread trondd
On Fri, Dec 19, 2014 at 8:22 PM, Theo de Raadt dera...@cvs.openbsd.org
wrote:

 whereas ntp.org's codebase is reportedly 100,000 lines of
 unknown or largely unused code


That made me curious.  Is it that bloated?

$ for i in $(find . -name *.[ch]); do cat $i  allcode; done
$ egrep -v '[:blank:]*/?\*' allcode | grep -v ^ *$ | wc -l
  192870

This is ntp-4.2.8  A rough estimate but close enough if we are comparing to
a know solution that is 5000.

Keep up the good work.
Tim.


Re: NTP

2014-12-19 Thread J Sisson
Using the same test on OpenNTPD from OpenBSD-STABLE:

# cd /usr/src/usr.sbin/ntpd/
# for i in $(find . -name *.[ch]); do cat $i  /root/allcode; done
# egrep -v '[:blank:]*/?\*' /root/allcode | grep -v ^ *$ | wc -l
2898

Quite a difference indeed.

On Fri, Dec 19, 2014 at 7:26 PM, trondd tro...@gmail.com wrote:
 On Fri, Dec 19, 2014 at 8:22 PM, Theo de Raadt dera...@cvs.openbsd.org
 wrote:

 whereas ntp.org's codebase is reportedly 100,000 lines of
 unknown or largely unused code


 That made me curious.  Is it that bloated?

 $ for i in $(find . -name *.[ch]); do cat $i  allcode; done
 $ egrep -v '[:blank:]*/?\*' allcode | grep -v ^ *$ | wc -l
   192870

 This is ntp-4.2.8  A rough estimate but close enough if we are comparing to
 a know solution that is 5000.

 Keep up the good work.
 Tim.



-- 
BSD is what happens when Unix programmers port Unix to the x86.
Linux is what happens when x86 programmers write a Unix-like.
Windows is what happens when x86 programmers run all of their
programming textbooks through a blender, eat the ground up
remains of the text, and then code up what they can read in the
toilet 3 days later.



Re: NTP

2014-12-19 Thread Theo de Raadt
 # cd /usr/src/usr.sbin/ntpd/
 # for i in $(find . -name *.[ch]); do cat $i  /root/allcode; done
 # egrep -v '[:blank:]*/?\*' /root/allcode | grep -v ^ *$ | wc -l
 2898

  $ for i in $(find . -name *.[ch]); do cat $i  allcode; done
  $ egrep -v '[:blank:]*/?\*' allcode | grep -v ^ *$ | wc -l
192870
 
  This is ntp-4.2.8  A rough estimate but close enough if we are comparing to
  a know solution that is 5000.

That is a factor of 66x.  Shocking, it is larger than I remembered.

So, what does that extra source code bring to the table?

My perspective is that big code brings holes because fewer people want
to read, audit, and maintain the code.

But surely there must be some benefit.  It has been claimed NTPD is
more accurate.  For that extra size, is it 66x more accurate?  That's
silly.  Even if it is a little bit more accurate, what does 'more
accurate' mean when edge devices that care for more than ~100ms
accuracy are exceedingly rare?

The devices which care for accuracy are doing time distribution, not
time reception at the network edge.  And there lies the problem, I
think.  Much of that code is likely for special purposes in an
ecosystem which includes products, relationships with companies
building products, special modes, special features, labratory code,
etc.  The result was that this team built a NTP daemon for all
purposes great and small:

One NTPD to rule them all,
One NTPD to find them,
One NTPD to bring them all
and on the internet hole them

I am going to guess the serious time distribution people are in
control of this software stack, but 99% of their base is in the edge
devices where the extra complexity is unwarranted?  (Is it time for a
coup?)

I think the NTP people don't even know their own codebase because it
is too large and full of legacy code.  They don't know what parts of
it do, but they don't want to upset their base by removing any of
it.

In another project we recently reviewed, there was code all over the
place for some rotten platforms -- barely limping along -- but the
existance and structure of that code would would be cumbersome and the
effects would be detrimental to all platforms.  Developers trying to
change the code make mistakes -- or worse they get demoralized and
stop trying.  Such codebases purport to be about agility, but as it
tries to reach that goal which is increasingly less relevant, it hurts
the most important goals.  You cannot properly serve 1995 and 2015.

Back to NTPD, I suspect most of the people who struggled through it in
the last decade have been looking for holes to exploit.  It does not
seem healthy.  Healthy projects struggle with the effort of
refactoring their code.  Unhealthy project sit on their past results,
and the future catches up.

Certainly the codebase was also being run by strong-minded people,
I know the type.

The NTP codebase is larger than the SSH codebase.

Explain that, and then explain why noone visibly called them on that.


I recall the conversations I had with the NTP team bit around 10 years
ago.  They did not want help auditing their code.  It was sad, and I
found a more receptive audience in Henning.  Once he wrote openntpd,
we had to face public attacks from NTP team members.  Apparently the
openntpd math was bad and we would break internet time..  internet
still seems fine, except a ntp clients spun up to 100 peers is seeing
a fair number of ntp time distributions points are down...


I am glad to hear NTPD is being rewritten.  Because over time this
will bring safety back back into focus.  PHK, I hope you don't just
focus on the math.  Privsep it, please, but ensure DNS refresh is
possible in the architecture you choose.  There is a time-tested model
which does not require the OS to have this or that non-standard
security feature; we all know we can't mandate use of those features
which barely help anyways.  You rely on privsep everytime you login to
another machine, so consider it.



Very hard to push pressure upstream.