Re: fuse_opt.h and clang 6

[Todd C. Miller]

On Sat, 07 Apr 2018 21:33:24 +0200, Jeremie Courreges-Anglas wrote:

> At least ports/archivers/fuse-zip fails to build with clang 6, because
> of stricter diagnostics:
>
> -->8--
> c++ -c -O2 -pipe-I/usr/local/include main.cpp \
> -Ilib \
> -o main.o
> main.cpp:138:5: error: constant expression evaluates to -1 which cannot be na
> rrowed to type 'unsigned long' [-Wc++11-narrowing]
> FUSE_OPT_KEY("-h",  KEY_HELP),
> ^
> /usr/include/fuse_opt.h:51:33: note: expanded from macro 'FUSE_OPT_KEY'
> #define FUSE_OPT_KEY(t, k)  { t, -1, k }
>  ^~
> main.cpp:138:5: note: insert an explicit cast to silence this issue
> FUSE_OPT_KEY("-h",  KEY_HELP),
> ^
> --8<--
>
> Full log at:
>
>   https://junkpile.org/p/failures/i386-20180407/archivers/fuse-zip.log
>
> Proposed fix below.  ok?

OK millert@

 - todd

Re: random numbers for efiboot

[Theo de Raadt]

Mark Kettenis  wrote:

> The UEFI firmware for the MACCHIATObin implements the EFI Random
> Number Generator Protocol.  That makes it possible to implement the
> mdrandom() function for arm64's EFIBOOT.  The random data is XORed
> onto the buffer so bad random data can't hurt us.
> 
> The code is written such that it can be easily dropped into our other
> EFI bootloaders.  That raises a question though.  On amd64 we already
> have an mdrandom() implementation.  Should I call the function
> fwrandom() instead and and an
> 
> #ifdef FWRANDOM
>fwrandom(rnddata, sizeof(rnddata));
> #endif
> 
> next to the mdrandom() call in boot.c?

Yes, it should call every thing like that.

random numbers for efiboot

[Mark Kettenis]

The UEFI firmware for the MACCHIATObin implements the EFI Random
Number Generator Protocol.  That makes it possible to implement the
mdrandom() function for arm64's EFIBOOT.  The random data is XORed
onto the buffer so bad random data can't hurt us.

The code is written such that it can be easily dropped into our other
EFI bootloaders.  That raises a question though.  On amd64 we already
have an mdrandom() implementation.  Should I call the function
fwrandom() instead and and an

#ifdef FWRANDOM
   fwrandom(rnddata, sizeof(rnddata));
#endif

next to the mdrandom() call in boot.c?


Index: arch/arm64/stand/efiboot/Makefile
===
RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/Makefile,v
retrieving revision 1.4
diff -u -p -r1.4 Makefile
--- arch/arm64/stand/efiboot/Makefile   31 Mar 2018 17:43:53 -  1.4
+++ arch/arm64/stand/efiboot/Makefile   7 Apr 2018 21:04:50 -
@@ -8,8 +8,8 @@ PROG=   BOOTAA64.EFI
 OBJFMT=binary
 INSTALL_STRIP=
 BINDIR=/usr/mdec
-SRCS=  start.S self_reloc.c efiboot.c conf.c exec.c efidev.c efipxe.c
-SRCS+= fdt.c
+SRCS=  start.S self_reloc.c efiboot.c conf.c exec.c efidev.c
+SRCS+= efipxe.c efirng.c fdt.c
 
 S= ${.CURDIR}/../../../..
 EFIDIR=${S}/stand/efi
@@ -43,7 +43,7 @@ CPPFLAGS+=-I${S} -I. -I${.CURDIR}
 CPPFLAGS+= -I${EFIDIR}/include -I${EFIDIR}/include/arm64
 CPPFLAGS+= -D_STANDALONE
 CPPFLAGS+= -DSMALL -DSLOW -DNOBYFOUR -D__INTERNAL_LIBSA_CREAD
-CPPFLAGS+= -DNEEDS_HEAP_H
+CPPFLAGS+= -DNEEDS_HEAP_H -DMDRANDOM
 COPTS+=-Wno-attributes -Wno-format
 COPTS+=-ffreestanding -fno-stack-protector
 COPTS+=-fshort-wchar -fPIC -fno-builtin
Index: arch/arm64/stand/efiboot/conf.c
===
RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/conf.c,v
retrieving revision 1.14
diff -u -p -r1.14 conf.c
--- arch/arm64/stand/efiboot/conf.c 31 Mar 2018 17:44:57 -  1.14
+++ arch/arm64/stand/efiboot/conf.c 7 Apr 2018 21:04:50 -
@@ -36,7 +36,7 @@
 #include "efidev.h"
 #include "efipxe.h"
 
-const char version[] = "0.12";
+const char version[] = "0.13";
 intdebug = 0;
 
 struct fs_ops file_system[] = {
Index: arch/arm64/stand/efiboot/efirng.c
===
RCS file: arch/arm64/stand/efiboot/efirng.c
diff -N arch/arm64/stand/efiboot/efirng.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ arch/arm64/stand/efiboot/efirng.c   7 Apr 2018 21:04:50 -
@@ -0,0 +1,87 @@
+/* $OpenBSD$   */
+
+/*
+ * Copyright (c) 2018 Mark Kettenis 
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include 
+
+#include 
+#include 
+
+#include "eficall.h"
+#include "libsa.h"
+
+extern EFI_BOOT_SERVICES   *BS;
+
+/* Random Number Generator Protocol */
+
+#define EFI_RNG_PROTOCOL_GUID \
+{ 0x3152bca5, 0xeade, 0x433d, {0x86, 0x2e, 0xc0, 0x1c, 0xdc, 0x29, 0x1f, 
0x44} }
+
+INTERFACE_DECL(_EFI_RNG_PROTOCOL);
+
+typedef EFI_GUID EFI_RNG_ALGORITHM;
+
+typedef
+EFI_STATUS
+(EFIAPI *EFI_RNG_GET_INFO) (
+IN struct _EFI_RNG_PROTOCOL*This,
+IN  OUT UINTN  *RNGAlgorithmListSize,
+OUT EFI_RNG_ALGORITHM  *RNGAlgorithmList
+);
+
+typedef
+EFI_STATUS
+(EFIAPI *EFI_RNG_GET_RNG) (
+IN struct _EFI_RNG_PROTOCOL*This,
+IN EFI_RNG_ALGORITHM   *RNGAlgorithm, OPTIONAL
+IN UINTN   RNGValueLength,
+OUT UINT8  *RNGValue
+);
+
+typedef struct _EFI_RNG_PROTOCOL {
+   EFI_RNG_GET_INFOGetInfo;
+   EFI_RNG_GET_RNG GetRNG;
+} EFI_RNG_PROTOCOL;
+
+static EFI_GUIDrng_guid = EFI_RNG_PROTOCOL_GUID;
+
+void
+mdrandom(char *buf, size_t buflen)
+{
+   EFI_STATUS   status;
+   EFI_RNG_PROTOCOL*rng = NULL;
+   UINT8   *random;
+   size_t   i;
+
+   status = EFI_CALL(BS->LocateProtocol, _guid, NULL, (void **));
+   if (rng == NULL || EFI_ERROR(status))
+   return;
+
+   random = 

fuse_opt.h and clang 6

[Jeremie Courreges-Anglas]

At least ports/archivers/fuse-zip fails to build with clang 6, because
of stricter diagnostics:

-->8--
c++ -c -O2 -pipe-I/usr/local/include main.cpp \
-Ilib \
-o main.o
main.cpp:138:5: error: constant expression evaluates to -1 which cannot be 
narrowed to type 'unsigned long' [-Wc++11-narrowing]
FUSE_OPT_KEY("-h",  KEY_HELP),
^
/usr/include/fuse_opt.h:51:33: note: expanded from macro 'FUSE_OPT_KEY'
#define FUSE_OPT_KEY(t, k)  { t, -1, k }
 ^~
main.cpp:138:5: note: insert an explicit cast to silence this issue
FUSE_OPT_KEY("-h",  KEY_HELP),
^
--8<--

Full log at:

  https://junkpile.org/p/failures/i386-20180407/archivers/fuse-zip.log

Proposed fix below.  ok?


Index: fuse_opt.h
===
RCS file: /d/cvs/src/lib/libfuse/fuse_opt.h,v
retrieving revision 1.4
diff -u -p -p -u -r1.4 fuse_opt.h
--- fuse_opt.h  20 May 2014 13:22:06 -  1.4
+++ fuse_opt.h  7 Apr 2018 19:28:55 -
@@ -48,7 +48,7 @@ int fuse_opt_parse(struct fuse_args *, v
 
 #define FUSE_OPT_IS_OPT_KEY(t) (t->off == (unsigned long)-1)
 
-#define FUSE_OPT_KEY(t, k) { t, -1, k }
+#define FUSE_OPT_KEY(t, k) { t, (unsigned long)-1, k }
 #define FUSE_OPT_END   { NULL, 0, 0 }
 #define FUSE_OPT_KEY_OPT   -1
 #define FUSE_OPT_KEY_NONOPT-2

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: video(1): measure with mono clock (tester wanted)

[Benjamin Baier]

On Fri, 6 Apr 2018 09:28:54 -0500
Scott Cheloha  wrote:

> So that your stats stay correct if the system clock is changed.
> 
> This is simple enough to suggest that it's correct at a glance,
> but I have no hardware to test this with.  Can anyone confirm that
> this works?

No regressions.
Greetings Ben

$ /usr/X11R6/bin/video -vvv 
video device /dev/video:
  encodings: yuy2
  frame sizes (width x height, in pixels) and rates (in frames per second):
320x240: 30, 15
352x288: 30, 15
424x240: 30, 15
640x360: 30, 15
640x480: 30, 15
800x448: 15
960x544: 10
1280x720: 10
  controls: brightness, contrast, saturation, hue, gamma, sharpness
Xv adaptor 0, Intel(R) Textured Video:
  encodings: yuy2, uyvy
  max size: 3286x1200
using yuy2 encoding
using frame size 640x480 (614400 bytes)
using default frame rate
video: got ConfigureNotify event1.34, fps: 14.94520
video: got ConfigureNotify event
video: got ConfigureNotify event
video: got KeyPress event: 18.05, fps: 14.40465

run time: 18.330074 seconds
frames grabbed: 265
frames played: 264
played fps: 14.348006

> 
> Index: app/video/video.c
> ===
> RCS file: /cvs/xenocara/app/video/video.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 video.c
> --- app/video/video.c 26 Nov 2016 11:49:15 -  1.23
> +++ app/video/video.c 6 Apr 2018 14:21:26 -
> @@ -30,6 +30,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> @@ -1635,7 +1636,7 @@ int
>  stream(struct video *vid)
>  {
>   struct xdsp *x = >xdsp;
> - struct timeval tp_start, tp_now, tp_run;
> + struct timespec tp_start, tp_now, tp_run;
>   struct itimerval frit;
>   double run_time;
>   uint8_t *src;
> @@ -1643,7 +1644,7 @@ stream(struct video *vid)
>   int sequence = 20, ret, err, todo, done;
>  
>   /* Guard against uninitialized variable in case no frame is grabbed. */
> - gettimeofday(_start, NULL);
> + clock_gettime(CLOCK_MONOTONIC, _start);
>  
>   if (vid->fps && !vid->nofps) {
>   fus = 100 / vid->fps;
> @@ -1759,21 +1760,21 @@ stream(struct video *vid)
>   frames_played++;
>  
>   if (frames_played == 0)
> - gettimeofday(_start, NULL);
> + clock_gettime(CLOCK_MONOTONIC, _start);
>  
>   if (vid->verbose > 1 && frames_played > 0 &&
>   (frames_played) % sequence == 0) {
> - gettimeofday(_now, NULL);
> - timersub(_now, _start, _run);
> + clock_gettime(CLOCK_MONOTONIC, _now);
> + timespecsub(_now, _start, _run);
>   run_time = tp_run.tv_sec +
> - (double)tp_run.tv_usec / 100;
> + tp_run.tv_nsec / 10.0;
>   fprintf(stderr, "frames: %08ld, seconds: "
>   "%09.2f, fps: %08.5f\r", frames_played,
>   run_time, ((double)frames_played) / run_time);
>   fflush(stderr);
>   }
>   }
> - gettimeofday(_now, NULL);
> + clock_gettime(CLOCK_MONOTONIC, _now);
>  
>   if (vid->fps) {
>   timerclear(_value);
> @@ -1793,8 +1794,8 @@ stream(struct video *vid)
>   fprintf(stderr, "\n");
>  
>   if (vid->verbose > 0) {
> - timersub(_now, _start, _run);
> - run_time = tp_run.tv_sec + (double)tp_run.tv_usec / 100;
> + timespecsub(_now, _start, _run);
> + run_time = tp_run.tv_sec + tp_run.tv_nsec / 10.0;
>   fprintf(stderr, "run time: %f seconds\n", run_time);
>   fprintf(stderr, "frames grabbed: %ld\n", frames_grabbed);
>   fprintf(stderr, "frames played: %ld\n", frames_played + 1);
> 

Multiple RTCs

[Mark Kettenis]

I have an RK3399 system that has two RTCs.  One of those is the RTC
integrated on the RK808 PMIC that is a companion chip to the RK3399.
The second one is an ISL1208 I2C chip.  Only the ISL1208 is battery
powered, so obviously we want to use that one.  But rkpmic(4) attaches
after islrtc(4) on this board and therefore it wins.  The diff below
makes sure the RTC of the RK808 PMIC is only enabled if no other RTC
installed itself.

ok?


Index: dev/fdt/rkpmic.c
===
RCS file: /cvs/src/sys/dev/fdt/rkpmic.c,v
retrieving revision 1.4
diff -u -p -r1.4 rkpmic.c
--- dev/fdt/rkpmic.c25 Feb 2018 20:43:33 -  1.4
+++ dev/fdt/rkpmic.c7 Apr 2018 18:31:41 -
@@ -127,7 +127,8 @@ rkpmic_attach(struct device *parent, str
sc->sc_todr.cookie = sc;
sc->sc_todr.todr_gettime = rkpmic_gettime;
sc->sc_todr.todr_settime = rkpmic_settime;
-   todr_handle = >sc_todr;
+   if (todr_handle == NULL)
+   todr_handle = >sc_todr;
 
if (OF_is_compatible(node, "rockchip,rk805")) {
chip = "RK805";

Re: Duplicate Ethernet PHYs

[Mark Kettenis]

> Date: Wed, 4 Apr 2018 11:56:28 +0200 (CEST)
> From: Mark Kettenis 
> 
> With the growing popularity of SoCs, external PHYs are making a bit of
> a comeback.  In quite a few cases I'm seeing rgephy(4) show up twice,
> once on address 0 and once on some other address.  But there is only
> one PHY soldered onto the board.  This is a little bit annoying but
> mostly harmless.  The mii(4) layer will use the first one that shows
> up and everything works.  But now I'm seeing the same thing happening
> with a Micrel PHY that shows up as ukphy(4).  And here it is causing
> issues since the mii(4) attempts to "isolate" PHYs that it isn't using
> and the Micrel PHY actually implements that functionality.
> 
> One way to solve the issue is to add the MIIF_NOISOLATE flag to the
> mii_attach() call.  However that still means the PHY shows up twice in
> dmesg.  So here is an alternative solution.  The mii_attach() function
> has an offloc argument that kan be used to tell the mii(4) layer to
> attach the first, second, third, etc. PHY it finds when scanning the
> bus.  The diff below makes dwge(4) set offloc to 0 (to attach the
> first PHY found) if phyloc is set to MII_PHY_ANY.  This means we
> attach the PHY with address 0 instead of its real address, but that is
> ok.
> 
> This code is a little bit ugly.  An alternative would be to modify
> mii_attach(4) such that it no longer panics if phyloc != MII_PHY_ANY
> and offloc != MII_OFFSET_ANY.  Then we could simply set offloc to 0 in
> the mii_attach() call.
> 
> comments? ok?

Nobody?

> Index: dev/ic/dwc_gmac.c
> ===
> RCS file: /cvs/src/sys/dev/ic/dwc_gmac.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 dwc_gmac.c
> --- dev/ic/dwc_gmac.c 29 Jun 2017 17:36:16 -  1.8
> +++ dev/ic/dwc_gmac.c 4 Apr 2018 09:47:29 -
> @@ -222,8 +222,8 @@ dwc_gmac_attach(struct dwc_gmac_softc *s
>  
>   ifmedia_init(>mii_media, 0, dwc_gmac_ifmedia_upd,
>   dwc_gmac_ifmedia_sts);
> - mii_attach((void *)sc, mii, 0x, phyloc, MII_OFFSET_ANY,
> - MIIF_DOPAUSE);
> + mii_attach((void *)sc, mii, 0x, phyloc,
> + (phyloc == MII_PHY_ANY) ? 0 : MII_OFFSET_ANY, MIIF_DOPAUSE);
>  
>   if (LIST_EMPTY(>mii_phys)) {
>   printf("%s: no PHY found!\n", sc->sc_dev.dv_xname);
> 
> 

Re: patch(1): remove unused header

[Todd C. Miller]

On Sat, 07 Apr 2018 10:46:01 +0200, Anton Lindqvist wrote:

> Since patch(1) no longer invokes ed(1), pathnames.h can be removed.
> _PATH_TMP is still used inside patch.c but including paths.h is
> sufficient.

OK millert@

 - todd

Re: fstat -r flag to display rdomains on sockets

[Stuart Henderson]

On 2018/04/07 13:33, Claudio Jeker wrote:
> On Sat, Apr 07, 2018 at 11:51:39AM +0100, Stuart Henderson wrote:
> > On 2018/04/07 12:29, Peter J. Philipp wrote:
> > > Hi,
> > > 
> > > I've been running iked for a while now and have been able to guess which 
> > > iked
> > > belongs to which rdomain by the cpu counter but as I'm using the other 
> > > iked
> > > more the cpu counter is about the same and it's confusing when I have to 
> > > restart iked with route exec.
> > > 
> > > I introduce the -r flag to fstat in order to display rdomains on sockets 
> > > in
> > > order to find the right iked.  The fix is simple but because some peoples
> > > scripts may depend on the old output I made it an extra flag.
> > 
> > If you're just trying to find the pid there's an existing way,
> > 'pgrep -T 2 iked'. But I do think it's useful to print this in fstat
> > output, especially when a single process uses multiple tables.
> > 
> > How about skipping the -r flag, but printing it after the existing
> > "internet [...] addr:port", and only printing if it's a non-default
> > table? This should minimise breakage, and it's not like the format
> > is cast in stone (spliced sockets were added in the past).
> > 
> > I wonder about using "rtable" instead of "rdomain" in the text, it
> > would be more accurate (but then I do see "rdomain" in some other
> > programs like bgpd).
> > 
> 
> I agree that this should print rtable. bgpd is a bit special when it comes
> to rdomain vs rtable. It uses both but for different contexts (e.g. bgpd
> is using an rdomain to do nexthop validation but can update any rtable).
> 
> Sockets shown by fstat are always bound to an rtable and therefor should
> print rtable and not rdomain. With that change it is OK claudio@

bluhm, does this work for you too?

Rationale for printing the extra text afterwards rather than before
is that working with fstat output very often involves printing the
"Nth" word in a line of output, and that then remains unchanged for
the existing information.

Index: fstat.c
===
RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
retrieving revision 1.92
diff -u -p -r1.92 fstat.c
--- fstat.c 6 Apr 2018 14:05:06 -   1.92
+++ fstat.c 7 Apr 2018 11:36:39 -
@@ -740,11 +740,15 @@ socktrans(struct kinfo_file *kf)
printf("* internet %s", stype);
getinetproto(kf->so_protocol);
print_inet_details(kf);
+   if (kf->inp_rtableid)
+   printf(" rtable %u", kf->inp_rtableid);
break;
case AF_INET6:
printf("* internet6 %s", stype);
getinetproto(kf->so_protocol);
print_inet6_details(kf);
+   if (kf->inp_rtableid)
+   printf(" rtable %u", kf->inp_rtableid);
break;
case AF_UNIX:
/* print address of pcb and connected pcb */

Re: [PATCH 1/7] em: Print error code and phy/mac type

[Stefan Fritsch]

On Fri, 6 Apr 2018, Jonathan Gray wrote:

> On Thu, Apr 05, 2018 at 09:57:17PM +0200, Stefan Fritsch wrote:
> > Print the error code if hardware initialization failed.
> > 
> > If EM_DEBUG is defined, print the phy/mac type during attach.
> 
> ok jsg@
> 
> Though these are just enum values we assign that don't come from the
> hardware.
> 
> mac_type is set based on pci product and in some cases revision.
> phy_type is mostly set based on hw->phy_id from PHY_ID1/PHY_ID2.

My workflow was looking at the code path through the em driver for this 
hardware, and then look at linux/freebsd what they do differently for the 
same hardware. Having these two values was necessary for that.

Re: [PATCH 7/7] em: Add magic delay for HP elitebook 820 G3

[Stefan Fritsch]

On Fri, 6 Apr 2018, Jonathan Gray wrote:

> On Thu, Apr 05, 2018 at 09:57:23PM +0200, Stefan Fritsch wrote:
> > Add another magic 1ms delay that seems to help with some remaining
> > issues on an HP elitebook 820 G3 with i219LM. A printf() at the same
> > place helps, too.
> 
> Could you explain what the problem here was and why this place was
> chosen to add the delay?

The problems fixed by patches 1-6 were that there were these kinds of 
errors

em0: Hardware Initialization Failed
em0: Unable to initialize the hardware

either during em_attach (in this case em0 would simply not be available in 
ifconfig) or during ifconfig up. The problems only appeared if there was 
no link (or no cable plugged in) during boot or during resume.

The fixes 1 to 5 made the problem appear much less often, but it still 
appeared sometimes. I had some printfs added and I have never seen the 
problem with the printf present. In the end I traced the "fix" to the 
single printf at this exact location and I replaced it with a delay.

We added Patch 6 (the E1000_TARC0_CB_MULTIQ_2_REQ erratum) later, to fix 
different problems with watchdog timeouts that were only recoverable by a 
reboot, when pulling the cable or when the link was flaky.

> 
> > ---
> >  sys/dev/pci/if_em_hw.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git sys/dev/pci/if_em_hw.c sys/dev/pci/if_em_hw.c
> > index 7709a4c5805..d122e727875 100644
> > --- sys/dev/pci/if_em_hw.c
> > +++ sys/dev/pci/if_em_hw.c
> > @@ -1493,6 +1493,8 @@ em_init_hw(struct em_hw *hw)
> > /* Set the media type and TBI compatibility */
> > em_set_media_type(hw);
> >  
> > +   /* Magic delay that improves problems with i219LM on HP Elitebook */
> > +   msec_delay(1);
> > /* Must be called after em_set_media_type because media_type is used */
> > em_initialize_hardware_bits(hw);
> >  
> > -- 
> > 2.13.0
> > 
> 

Re: fstat -r flag to display rdomains on sockets

[Claudio Jeker]

On Sat, Apr 07, 2018 at 11:51:39AM +0100, Stuart Henderson wrote:
> On 2018/04/07 12:29, Peter J. Philipp wrote:
> > Hi,
> > 
> > I've been running iked for a while now and have been able to guess which 
> > iked
> > belongs to which rdomain by the cpu counter but as I'm using the other iked
> > more the cpu counter is about the same and it's confusing when I have to 
> > restart iked with route exec.
> > 
> > I introduce the -r flag to fstat in order to display rdomains on sockets in
> > order to find the right iked.  The fix is simple but because some peoples
> > scripts may depend on the old output I made it an extra flag.
> 
> If you're just trying to find the pid there's an existing way,
> 'pgrep -T 2 iked'. But I do think it's useful to print this in fstat
> output, especially when a single process uses multiple tables.
> 
> How about skipping the -r flag, but printing it after the existing
> "internet [...] addr:port", and only printing if it's a non-default
> table? This should minimise breakage, and it's not like the format
> is cast in stone (spliced sockets were added in the past).
> 
> I wonder about using "rtable" instead of "rdomain" in the text, it
> would be more accurate (but then I do see "rdomain" in some other
> programs like bgpd).
> 

I agree that this should print rtable. bgpd is a bit special when it comes
to rdomain vs rtable. It uses both but for different contexts (e.g. bgpd
is using an rdomain to do nexthop validation but can update any rtable).

Sockets shown by fstat are always bound to an rtable and therefor should
print rtable and not rdomain. With that change it is OK claudio@

> Index: fstat.c
> ===
> RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
> retrieving revision 1.92
> diff -u -p -r1.92 fstat.c
> --- fstat.c   6 Apr 2018 14:05:06 -   1.92
> +++ fstat.c   7 Apr 2018 10:43:56 -
> @@ -740,11 +740,15 @@ socktrans(struct kinfo_file *kf)
>   printf("* internet %s", stype);
>   getinetproto(kf->so_protocol);
>   print_inet_details(kf);
> + if (kf->inp_rtableid)
> + printf(" rdomain %u", kf->inp_rtableid);
>   break;
>   case AF_INET6:
>   printf("* internet6 %s", stype);
>   getinetproto(kf->so_protocol);
>   print_inet6_details(kf);
> + if (kf->inp_rtableid)
> + printf(" rdomain %u", kf->inp_rtableid);
>   break;
>   case AF_UNIX:
>   /* print address of pcb and connected pcb */
> 

-- 
:wq Claudio

Re: fstat -r flag to display rdomains on sockets

[Alexander Bluhm]

On Sat, Apr 07, 2018 at 12:29:37PM +0200, Peter J. Philipp wrote:
> I introduce the -r flag to fstat in order to display rdomains on sockets in
> order to find the right iked.  The fix is simple but because some peoples
> scripts may depend on the old output I made it an extra flag.

Is is a rdomain?  I think rtable is the correct name here.

If we print the rtable only when is not the default, we can avoid
an extra option, don't break many scripts, and make more obvious
if something is non standard.

root nc 437853* rtable 7 internet dgram udp *:12345

bluhm

Index: usr.bin/fstat/fstat.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/fstat/fstat.c,v
retrieving revision 1.92
diff -u -p -r1.92 fstat.c
--- usr.bin/fstat/fstat.c   6 Apr 2018 14:05:06 -   1.92
+++ usr.bin/fstat/fstat.c   7 Apr 2018 11:04:54 -
@@ -724,6 +724,10 @@ socktrans(struct kinfo_file *kf)
stype = stypename[kf->so_type];
}
 
+   printf("*");
+   if (kf->inp_rtableid)
+   printf(" rtable %u", kf->inp_rtableid);
+
/*
 * protocol specific formatting
 *
@@ -737,18 +741,18 @@ socktrans(struct kinfo_file *kf)
 */
switch (kf->so_family) {
case AF_INET:
-   printf("* internet %s", stype);
+   printf(" internet %s", stype);
getinetproto(kf->so_protocol);
print_inet_details(kf);
break;
case AF_INET6:
-   printf("* internet6 %s", stype);
+   printf(" internet6 %s", stype);
getinetproto(kf->so_protocol);
print_inet6_details(kf);
break;
case AF_UNIX:
/* print address of pcb and connected pcb */
-   printf("* unix %s", stype);
+   printf(" unix %s", stype);
if (kf->so_pcb) {
printf(" ");
hide((void *)(uintptr_t)kf->so_pcb);
@@ -769,19 +773,19 @@ socktrans(struct kinfo_file *kf)
break;
case AF_MPLS:
/* print protocol number and socket address */
-   printf("* mpls %s", stype);
+   printf(" mpls %s", stype);
printf(" %d ", kf->so_protocol);
hide((void *)(uintptr_t)kf->f_data);
break;
case AF_ROUTE:
/* print protocol number and socket address */
-   printf("* route %s", stype);
+   printf(" route %s", stype);
printf(" %d ", kf->so_protocol);
hide((void *)(uintptr_t)kf->f_data);
break;
default:
/* print protocol number and socket address */
-   printf("* %d %s", kf->so_family, stype);
+   printf(" %d %s", kf->so_family, stype);
printf(" %d ", kf->so_protocol);
hide((void *)(uintptr_t)kf->f_data);
}

Re: fstat -r flag to display rdomains on sockets

[Stuart Henderson]

On 2018/04/07 12:29, Peter J. Philipp wrote:
> Hi,
> 
> I've been running iked for a while now and have been able to guess which iked
> belongs to which rdomain by the cpu counter but as I'm using the other iked
> more the cpu counter is about the same and it's confusing when I have to 
> restart iked with route exec.
> 
> I introduce the -r flag to fstat in order to display rdomains on sockets in
> order to find the right iked.  The fix is simple but because some peoples
> scripts may depend on the old output I made it an extra flag.

If you're just trying to find the pid there's an existing way,
'pgrep -T 2 iked'. But I do think it's useful to print this in fstat
output, especially when a single process uses multiple tables.

How about skipping the -r flag, but printing it after the existing
"internet [...] addr:port", and only printing if it's a non-default
table? This should minimise breakage, and it's not like the format
is cast in stone (spliced sockets were added in the past).

I wonder about using "rtable" instead of "rdomain" in the text, it
would be more accurate (but then I do see "rdomain" in some other
programs like bgpd).

Index: fstat.c
===
RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
retrieving revision 1.92
diff -u -p -r1.92 fstat.c
--- fstat.c 6 Apr 2018 14:05:06 -   1.92
+++ fstat.c 7 Apr 2018 10:43:56 -
@@ -740,11 +740,15 @@ socktrans(struct kinfo_file *kf)
printf("* internet %s", stype);
getinetproto(kf->so_protocol);
print_inet_details(kf);
+   if (kf->inp_rtableid)
+   printf(" rdomain %u", kf->inp_rtableid);
break;
case AF_INET6:
printf("* internet6 %s", stype);
getinetproto(kf->so_protocol);
print_inet6_details(kf);
+   if (kf->inp_rtableid)
+   printf(" rdomain %u", kf->inp_rtableid);
break;
case AF_UNIX:
/* print address of pcb and connected pcb */

Re: ksh: support 64 bit numbers on 32 bit archs

[Theo Buehler]

On Wed, Apr 04, 2018 at 07:18:43PM +0200, Tobias Stoeckmann wrote:
> On Wed, Apr 04, 2018 at 01:30:52PM +0200, Theo Buehler wrote:
> > > +/* convert uint64_t to base N string */
> > > 
> > >  char *
> > > -ulton(long unsigned int n, int base)
> > > +u64ton(uint64_t n, int base)
> > >  {
> > >   char *p;
> > >   static char buf [20];
> > 
> > I don't think it's actually an issue since I don't see how n can ever be
> > that large, but 20 is too small to hold a NUL-terminated base 10 version
> > of UINT64_MAX, as 18446744073709551615 has 20 digits. Thus,
> > u64ton(UINT64_MAX, 10) will write to and return buf[-1].
> > 
> > With bases < 10 you'll have trouble with buf[21] (base 8 will need
> > buf[23] and base 2 will need buf[65]).
> > 
> > Since all three callers of u64ton() use base 10 anyway, doing buf[21]
> > would be enough for now. I wonder if we should get rid of the base
> > argument and simplify accordingly (that would be a separate diff).
> 
> I fully agree with your spottings. It's actually worse than that.
> Take a look at tree.c:
> 
>   p = ulton((neg) ? -n : n, 10);
>   if (neg)
>   *--p = '-';

ouch.

> It means that we would need buf[22]. But I don't want to fix it in this
> patch, because this issue is not related to a switch from 32 to 64 bits
> but an already existing issue (and it's theoretical, it would take
> an int of size 64 bit if you look above that code snippet).

Agreed.

> What I want to do with another patch:
> 
> - Remove the "n" argument, it's always 10
> - Instead, add a "char prefix" argument which could be '-' for negative
>   numbers. Nobody should ever change a returned pointer like that.
> - Fix the comment above the function

Sounds good to me.

> But this is just an issue like the other existing integer overflows
> which have already been fixed in test and expr. I want to fix them in
> the same way in ksh, thus a unification of the code bases should be
> first priority.

Yes. Go ahead. I'm ok with the patch as it is.

fstat -r flag to display rdomains on sockets

[Peter J. Philipp]

Hi,

I've been running iked for a while now and have been able to guess which iked
belongs to which rdomain by the cpu counter but as I'm using the other iked
more the cpu counter is about the same and it's confusing when I have to 
restart iked with route exec.

I introduce the -r flag to fstat in order to display rdomains on sockets in
order to find the right iked.  The fix is simple but because some peoples
scripts may depend on the old output I made it an extra flag.

Here is how it looks like with and without the -r flag:

uranus$ fstat -rp 84169
USER CMD  PID   FD MOUNTINUM  MODE R/WSZ|DV
_ikediked   84169 text /  105245  -r-xr-xr-x r   321760
_ikediked   84169   wd /var   129920  drwxr-xr-x r  512
_ikediked   84169 root /var   129920  drwxr-xr-x r  512
_ikediked   841690 /  105143  crw-rw-rw-rw null
_ikediked   841691 /  105143  crw-rw-rw-rw null
_ikediked   841692 /  105143  crw-rw-rw-rw null
_ikediked   841693* 30 raw 2 0xd2f8f844  
_ikediked   841694* rdomain 2 internet dgram udp *:500   
_ikediked   841695* rdomain 2 internet dgram udp *:4500
_ikediked   841696* rdomain 2 internet6 dgram udp *:500
_ikediked   841697* rdomain 2 internet6 dgram udp *:4500
_ikediked   841698* unix stream 0xd3eb4180 <-> 0xd3eb4100
_ikediked   84169   10* unix stream 0xd3eb4280 <-> 0xd3eb4200
_ikediked   84169   11 kqueue 0xd32a8ad4 0 state: W

uranus$ fstat -p 47863
USER CMD  PID   FD MOUNTINUM  MODE R/WSZ|DV
_ikediked   47863 text /  105245  -r-xr-xr-x r   321760
_ikediked   47863   wd /var   129920  drwxr-xr-x r  512
_ikediked   47863 root /var   129920  drwxr-xr-x r  512
_ikediked   478630 /  105143  crw-rw-rw-rw null
_ikediked   478631 /  105143  crw-rw-rw-rw null
_ikediked   478632 /  105143  crw-rw-rw-rw null
_ikediked   478633* 30 raw 2 0xd3258e88
_ikediked   478634* internet dgram udp *:500   
_ikediked   478635* internet dgram udp *:4500  
_ikediked   478636* internet6 dgram udp *:500  
_ikediked   478637* internet6 dgram udp *:4500 
_ikediked   478638* unix stream 0xd3e67c00 <-> 0xd3e67b80  
_ikediked   47863   10* unix stream 0xd3e67d00 <-> 0xd3e67c80  
_ikediked   47863   11 kqueue 0xd32a8e7c 0 state: W

patch below signature.

Regards,
-peter

Index: fstat.1
===
RCS file: /cvs/src/usr.bin/fstat/fstat.1,v
retrieving revision 1.56
diff -u -p -u -r1.56 fstat.1
--- fstat.1 16 Mar 2018 16:58:26 -  1.56
+++ fstat.1 7 Apr 2018 10:27:52 -
@@ -37,7 +37,7 @@
 .Nd display status of open files
 .Sh SYNOPSIS
 .Nm fstat
-.Op Fl fnosv
+.Op Fl fnorsv
 .Op Fl M Ar core
 .Op Fl N Ar system
 .Op Fl p Ar pid
@@ -86,6 +86,8 @@ Useful for checking progress as a proces
 This information is only visible to the user or superuser.
 .It Fl p Ar pid
 Report all files open by the specified process.
+.It Fl r
+Report which rdomain a socket belongs to.
 .It Fl s
 Report per file io statistics in two additional columns
 .Sq XFERS
Index: fstat.c
===
RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
retrieving revision 1.92
diff -u -p -u -r1.92 fstat.c
--- fstat.c 6 Apr 2018 14:05:06 -   1.92
+++ fstat.c 7 Apr 2018 10:27:52 -
@@ -98,6 +98,7 @@ int   oflg;   /* display file offset */
 intsflg;   /* display file xfer/bytes counters */
 intvflg;   /* display errors in locating kernel data objects etc... */
 intcflg;   /* fuser only */
+intrflg;   /* display rdomains */
 
 intfuser;  /* 1 if we are fuser, 0 if we are fstat */
 intsigno;  /* signal to send (fuser only) */
@@ -148,7 +149,7 @@ main(int argc, char *argv[])
arg = -1;
what = KERN_FILE_BYPID;
nlistf = memf = NULL;
-   oflg = 0;
+   rflg = oflg = 0;
 
/* are we fstat(1) or fuser(1)? */
if (strcmp(__progname, "fuser") == 0) {
@@ -156,7 +157,7 @@ main(int argc, char *argv[])
optstr = "cfks:uM:N:";
} else {
fuser = 0;
-   optstr = "fnop:su:vN:M:";
+   optstr = "fnop:rsu:vN:M:";
}
 
/*
@@ -203,6 +204,9 @@ main(int argc, char *argv[])
}
what = 

Re: ksh: support 64 bit numbers on 32 bit archs

[Tobias Stoeckmann]

On Wed, Apr 04, 2018 at 01:30:52PM +0200, Theo Buehler wrote:
> > +/* convert uint64_t to base N string */
> > 
> >  char *
> > -ulton(long unsigned int n, int base)
> > +u64ton(uint64_t n, int base)
> >  {
> > char *p;
> > static char buf [20];
> 
> I don't think it's actually an issue since I don't see how n can ever be
> that large, but 20 is too small to hold a NUL-terminated base 10 version
> of UINT64_MAX, as 18446744073709551615 has 20 digits. Thus,
> u64ton(UINT64_MAX, 10) will write to and return buf[-1].
> 
> With bases < 10 you'll have trouble with buf[21] (base 8 will need
> buf[23] and base 2 will need buf[65]).
> 
> Since all three callers of u64ton() use base 10 anyway, doing buf[21]
> would be enough for now. I wonder if we should get rid of the base
> argument and simplify accordingly (that would be a separate diff).

I fully agree with your spottings. It's actually worse than that.
Take a look at tree.c:

p = ulton((neg) ? -n : n, 10);
if (neg)
*--p = '-';

It means that we would need buf[22]. But I don't want to fix it in this
patch, because this issue is not related to a switch from 32 to 64 bits
but an already existing issue (and it's theoretical, it would take
an int of size 64 bit if you look above that code snippet).

What I want to do with another patch:

- Remove the "n" argument, it's always 10
- Instead, add a "char prefix" argument which could be '-' for negative
  numbers. Nobody should ever change a returned pointer like that.
- Fix the comment above the function

But this is just an issue like the other existing integer overflows
which have already been fixed in test and expr. I want to fix them in
the same way in ksh, thus a unification of the code bases should be
first priority.


Tobias

patch(1): remove unused header

[Anton Lindqvist]

Hi,
Since patch(1) no longer invokes ed(1), pathnames.h can be removed.
_PATH_TMP is still used inside patch.c but including paths.h is
sufficient.

Comments? OK?

diff --git usr.bin/patch/patch.c usr.bin/patch/patch.c
index fef7df6466e..af142599340 100644
--- usr.bin/patch/patch.c
+++ usr.bin/patch/patch.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -42,7 +43,6 @@
 #include "pch.h"
 #include "inp.h"
 #include "backupfile.h"
-#include "pathnames.h"
 #include "ed.h"
 
 mode_t filemode = 0644;
diff --git usr.bin/patch/pathnames.h usr.bin/patch/pathnames.h
deleted file mode 100644
index 397e3fabe37..000
--- usr.bin/patch/pathnames.h
+++ /dev/null
@@ -1,11 +0,0 @@
-/* $OpenBSD: pathnames.h,v 1.1 2003/07/29 20:10:17 millert Exp $   */
-
-/*
- * Placed in the public domain by Todd C. Miller 
- * on July 29, 2003.
- */
-
-#include 
-
-#define_PATH_ED"/bin/ed"
-#define_PATH_MKDIR "/bin/mkdir"
diff --git usr.bin/patch/pch.c usr.bin/patch/pch.c
index 539754d814b..ac0645fe434 100644
--- usr.bin/patch/pch.c
+++ usr.bin/patch/pch.c
@@ -41,7 +41,6 @@
 #include "common.h"
 #include "util.h"
 #include "pch.h"
-#include "pathnames.h"
 
 /* Patch (diff listing) abstract type. */
 
diff --git usr.bin/patch/util.c usr.bin/patch/util.c
index f9d47906300..2ab5f1f0e28 100644
--- usr.bin/patch/util.c
+++ usr.bin/patch/util.c
@@ -43,7 +43,6 @@
 #include "common.h"
 #include "util.h"
 #include "backupfile.h"
-#include "pathnames.h"
 
 /* Rename a file, copying it if necessary. */

this fixes gif(4) on 6.3

[Peter J. Philipp]

Hello,

Yesterday I wrote to misc@ with this:

https://marc.info/?l=openbsd-misc=152302592426018=2

I apologize with the inline paste, thunderbird is just not good enough for
this stuff.

Anyhow I have produced this patch after upgrading the 6.2 box to 6.3.  It all
works now:

Here is my config:

gif1: flags=8051 mtu 1280
index 12 priority 0 llprio 3
groups: gif
tunnel: inet6 fd43:5602:29bd:16:0:dead:beef:1 -> 
fd43:5602:29bd:16:0:dead:beef:16 ttl 64 nodf rdomain 2
inet 172.16.16.10 --> 172.16.16.16 netmask 0xff00
inet6 fe80::290:bff:fe19:5604%gif1 ->  prefixlen 64 scopeid 0xc

uranus$ ping6 fe80::baae:edff:fe73:a76c%gif1  
PING fe80::baae:edff:fe73:a76c%gif1 (fe80::baae:edff:fe73:a76c%gif1): 56 data 
bytes
64 bytes from fe80::baae:edff:fe73:a76c%gif1: icmp_seq=0 hlim=64 time=8.767 ms
64 bytes from fe80::baae:edff:fe73:a76c%gif1: icmp_seq=1 hlim=64 time=9.854 ms
^C
--- fe80::baae:edff:fe73:a76c%gif1 ping statistics ---
2 packets transmitted, 2 packets received, 0.0% packet loss
round-trip min/avg/max/std-dev = 8.767/9.311/9.854/0.543 ms
uranus$ ping 172.16.16.16
PING 172.16.16.16 (172.16.16.16): 56 data bytes
64 bytes from 172.16.16.16: icmp_seq=0 ttl=255 time=10.329 ms
64 bytes from 172.16.16.16: icmp_seq=1 ttl=255 time=12.994 ms
^C
--- 172.16.16.16 ping statistics ---
2 packets transmitted, 2 packets received, 0.0% packet loss
round-trip min/avg/max/std-dev = 10.329/11.662/12.994/1.332 ms

The patch follows:


Index: if_gif.c
===
RCS file: /cvs/src/sys/net/if_gif.c,v
retrieving revision 1.113
diff -u -p -u -r1.113 if_gif.c
--- if_gif.c15 Mar 2018 21:01:18 -  1.113
+++ if_gif.c7 Apr 2018 07:59:54 -
@@ -338,7 +338,7 @@ gif_send(struct gif_softc *sc, struct mb
ip6->ip6_flow = htonl(flow);
ip6->ip6_vfc |= IPV6_VERSION;
ip6->ip6_plen = htons(len);
-   ip6->ip6_nxt = IPPROTO_GRE;
+   ip6->ip6_nxt = proto;
ip6->ip6_hlim = ttl;
ip6->ip6_src = sc->sc_tunnel.t_src6;
ip6->ip6_dst = sc->sc_tunnel.t_dst6;


Best regards,
-peter