size for free() in if_bwfm_pci.c

[Michael W. Bombardieri]

Hello,

In bwfm_pci_dmamem_alloc() and bwfm_pci_dmamem_free(), bdm points
to a single struct not an array. When freeing it we can just use
the struct size. Does this look correct?

- Michael


Index: if_bwfm_pci.c
===
RCS file: /cvs/src/sys/dev/pci/if_bwfm_pci.c,v
retrieving revision 1.3
diff -u -p -u -r1.3 if_bwfm_pci.c
--- if_bwfm_pci.c   1 Jan 2018 22:41:56 -   1.3
+++ if_bwfm_pci.c   3 Jan 2018 07:26:39 -
@@ -752,7 +752,7 @@ free:
 destroy:
bus_dmamap_destroy(sc->sc_dmat, bdm->bdm_map);
 bdmfree:
-   free(bdm, M_DEVBUF, 0);
+   free(bdm, M_DEVBUF, sizeof(*bdm));
 
return (NULL);
 }
@@ -763,7 +763,7 @@ bwfm_pci_dmamem_free(struct bwfm_pci_sof
bus_dmamem_unmap(sc->sc_dmat, bdm->bdm_kva, bdm->bdm_size);
bus_dmamem_free(sc->sc_dmat, >bdm_seg, 1);
bus_dmamap_destroy(sc->sc_dmat, bdm->bdm_map);
-   free(bdm, M_DEVBUF, 0);
+   free(bdm, M_DEVBUF, sizeof(*bdm));
 }
 
 /*

Re: malloc cleanup and small optimization (step 2)

[kshe]

On Sat, 30 Dec 2017 11:50:53 +, Otto Moerbeek wrote:
> Slightly different diff: instead of initing all chunk_info structs in
> page, do it on demand at first use. Measurements show that a lot of
> programs only use a few chunk_info structs, so it is a bit wasteful to
> always initialize all of them. Also de-inline the init code for
> readability.

Perhaps a point of detail, but how about setting up the bitmap with

i = p->total;
memset(p->bits, 0xff, sizeof(p->bits[0]) * (i / MALLOC_BITS));
if (i % MALLOC_BITS != 0)
p->bits[i / MALLOC_BITS] = (1U << (i % MALLOC_BITS)) - 1;

or, since p->total should never be zero,

i = p->total - 1;
memset(p->bits, 0xff, sizeof(p->bits[0]) * (i / MALLOC_BITS));
p->bits[i / MALLOC_BITS] = (2U << (i % MALLOC_BITS)) - 1;

instead of

for (i = 0; p->total - i >= MALLOC_BITS; i += MALLOC_BITS)
p->bits[i / MALLOC_BITS] = (u_short)~0U;
if (i < p->total)
p->bits[i / MALLOC_BITS] = 0;
for (; i < p->total; i++)
p->bits[i / MALLOC_BITS] |= (u_short)1U << (i % MALLOC_BITS);

at the end of the new init_chunk_info() function?

Besides, a few lines above those loops, I think p->shift could directly
be set to MALLOC_MINSHIFT when bits == 0, without having to recalculate
it from MALLOC_MINSIZE.

Regards,

kshe

Re: Bug in dd's args.c, invalid check for error

[Philip Guenther]

On Tue, Jan 2, 2018 at 3:06 PM, Ingo Schwarze  wrote:
...

> OK for the patch at the end?
>
> Testing - before:
>
...

> --- args.c  16 Aug 2016 16:44:55 -  1.28
> +++ args.c  2 Jan 2018 22:47:19 -
> @@ -406,8 +406,9 @@ get_off(char *val)
> off_t num, t;
> char *expr;
>
> +   errno = 0;
> num = strtoll(val, , 0);
> -   if (num == LLONG_MAX)   /* Overflow. */
> +   if (num == LLONG_MAX && errno == ERANGE)/* Overflow. */
> err(1, "%s", oper);
> if (expr == val)/* No digits. */
> errx(1, "%s: illegal numeric value", oper);
>

ok guenther@

Re: mg: extract child status with WEXITSTATUS

[Scott Cheloha]

On Mon, Jan 01, 2018 at 09:07:25PM -0700, Todd C. Miller wrote:
> On Mon, 01 Jan 2018 19:54:07 -0600, Scott Cheloha wrote:
> 
> > Hey,
> >
> > In the mg(1) *compile* buffer, currently you get incorrect
> > output like:
> >
> > Command exited abnormally with code 256 at [...]
> >
> > Using the W* macros in  corrects this:
> >
> > Command exited abnormally with code 1 at [...]
> 
> Is it worth using an explicit message if the command was terminated
> by a signal?

Like in lieu of 128+WTERMSIG?  I don't personally see my jobs in mg
get killed all that often, but if I did I think I'd prefer something
with the signal name, sure.

While we're at it, I'd like to move the timestamp left so it's separate
from the other output.  I'd also like to always print the exit status,
as "abnormally" is inapplicable for programs like diff and grep.

Thoughts?

--
Scott Cheloha

Index: usr.bin/mg/grep.c
===
RCS file: /cvs/src/usr.bin/mg/grep.c,v
retrieving revision 1.45
diff -u -p -r1.45 grep.c
--- usr.bin/mg/grep.c   12 Oct 2017 14:12:00 -  1.45
+++ usr.bin/mg/grep.c   3 Jan 2018 01:24:09 -
@@ -4,6 +4,8 @@
 
 #include 
 #include 
+#include 
+
 #include 
 #include 
 #include 
@@ -180,7 +182,7 @@ compile_mode(const char *name, const cha
char*buf;
size_t   sz;
ssize_t  len;
-   int  ret, n;
+   int  ret, n, signo;
char cwd[NFILEN], qcmd[NFILEN];
char timestr[NTIME];
time_t   t;
@@ -226,17 +228,19 @@ compile_mode(const char *name, const cha
t = time(NULL);
strftime(timestr, sizeof(timestr), "%a %b %e %T %Y", localtime());
addline(bp, "");
-   if (ret != 0)
-   addlinef(bp, "Command exited abnormally with code %d"
-   " at %s", ret, timestr);
-   else
-   addlinef(bp, "Command finished at %s", timestr);
+   if (WIFEXITED(ret)) {
+   addlinef(bp, "[%s] Command exited with status %d",
+   timestr, WEXITSTATUS(ret));
+   } else {
+   signo = WTERMSIG(ret);
+   addlinef(bp, "[%s] Command killed by %s: %s",
+   timestr, sys_signame[signo], strsignal(signo));
+   }
 
bp->b_dotp = bfirstlp(bp);
bp->b_modes[0] = name_mode("fundamental");
bp->b_modes[1] = name_mode("compile");
bp->b_nmodes = 1;
-
compile_buffer = bp;
 
if (chdir(cwd) == -1) {

Re: ASMedia ASM1061 SATA support for ahci(4)

[Jonathan Matthew]

On Tue, Jan 02, 2018 at 11:25:55PM +0100, Mark Kettenis wrote:
> This cip advertises itself as a PCI IDE controller, but actually
> implements an AHCI interface.  Adding it to the ahci_devices lists,
> without any quirks makes ahci(4) work.  It doesn't actually work with
> our pciide(4) driver!
> 
> ok?

ok.  I think this is just a mistake in the device's PCI config, because
the ASMedia controllers seem to be AHCI-only.  Working around it like this
seems reasonable to me.

> 
> 
> Index: dev/pci/ahci_pci.c
> ===
> RCS file: /cvs/src/sys/dev/pci/ahci_pci.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 ahci_pci.c
> --- dev/pci/ahci_pci.c27 May 2017 14:16:45 -  1.13
> +++ dev/pci/ahci_pci.c2 Jan 2018 22:07:30 -
> @@ -109,6 +109,13 @@ static const struct ahci_device ahci_dev
>   { PCI_VENDOR_ATI,   PCI_PRODUCT_ATI_SBX00_SATA_6,
>   NULL,   ahci_ati_sb700_attach },
>  
> + { PCI_VENDOR_ASMEDIA,   PCI_PRODUCT_ASMEDIA_ASM1061_SATA },
> +
>   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_6SERIES_AHCI_1,
>   NULL,   ahci_intel_attach },
>   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_6SERIES_AHCI_2,
> 

Re: ASMedia ASM1061 SATA support for ahci(4)

[David Gwynne]

> On 3 Jan 2018, at 08:25, Mark Kettenis  wrote:
> 
> This cip advertises itself as a PCI IDE controller, but actually
> implements an AHCI interface.  Adding it to the ahci_devices lists,
> without any quirks makes ahci(4) work.  It doesn't actually work with
> our pciide(4) driver!
> 
> ok?

im not angry, just disappointed.

ok.

> 
> 
> Index: dev/pci/ahci_pci.c
> ===
> RCS file: /cvs/src/sys/dev/pci/ahci_pci.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 ahci_pci.c
> --- dev/pci/ahci_pci.c27 May 2017 14:16:45 -  1.13
> +++ dev/pci/ahci_pci.c2 Jan 2018 22:07:30 -
> @@ -109,6 +109,13 @@ static const struct ahci_device ahci_dev
>   { PCI_VENDOR_ATI,   PCI_PRODUCT_ATI_SBX00_SATA_6,
>   NULL,   ahci_ati_sb700_attach },
> 
> + { PCI_VENDOR_ASMEDIA,   PCI_PRODUCT_ASMEDIA_ASM1061_SATA },
> +
>   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_6SERIES_AHCI_1,
>   NULL,   ahci_intel_attach },
>   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_6SERIES_AHCI_2,
> 

Re: Bug in dd's args.c, invalid check for error

[Ingo Schwarze]

Hi,

Bulat Musin wrote on Tue, Jan 02, 2018 at 09:47:39PM +0300:

> /bin/dd/args.c r1.28
> get_off(char *val)
> 
>   num = strtoll(val, , 0);
>   if (num == LLONG_MAX)   /* Overflow. */
>   err(1, "%s", oper);
> 
> Incorrect checking of overflow.
> Firstly, set errno to 0 before calling strtoll.
> Secondly: check of errno == ERANGE. =>

Clearly an edge case, but i think you are right that it ought to
be fixed, if only to avoid the bad example.  I also checked that
clobbering the previous value of errno is not a problem here.

OK for the patch at the end?

Testing - before:

 $ /obin/dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x7ffe
2+0 records in
2+0 records out
2 bytes transferred in 0.000 secs (116877 bytes/sec)
 $ echo $?
0
 $ /obin/dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x7fff
dd: skip: Undefined error: 0/* wrong */
 $ echo $?
1   /* wrong */
 $ /obin/dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x8000
dd: skip: Result too large
 $ echo $?
1

Testing - after:

 $ dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x7ffe
2+0 records in
2+0 records out
2 bytes transferred in 0.000 secs (230920 bytes/sec)
 $ echo $?
0
 $ dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x7fff
2+0 records in
2+0 records out
2 bytes transferred in 0.000 secs (78453 bytes/sec)
 $ echo $?
0
 $ dd if=/dev/zero of=/dev/null bs=1 count=2 skip=0x8000
dd: skip: Result too large
 $ echo $?
1

Yours,
  Ingo


Index: args.c
===
RCS file: /cvs/src/bin/dd/args.c,v
retrieving revision 1.28
diff -u -p -r1.28 args.c
--- args.c  16 Aug 2016 16:44:55 -  1.28
+++ args.c  2 Jan 2018 22:47:19 -
@@ -406,8 +406,9 @@ get_off(char *val)
off_t num, t;
char *expr;
 
+   errno = 0;
num = strtoll(val, , 0);
-   if (num == LLONG_MAX)   /* Overflow. */
+   if (num == LLONG_MAX && errno == ERANGE)/* Overflow. */
err(1, "%s", oper);
if (expr == val)/* No digits. */
errx(1, "%s: illegal numeric value", oper);

ASMedia ASM1061 SATA support for ahci(4)

[Mark Kettenis]

This cip advertises itself as a PCI IDE controller, but actually
implements an AHCI interface.  Adding it to the ahci_devices lists,
without any quirks makes ahci(4) work.  It doesn't actually work with
our pciide(4) driver!

ok?


Index: dev/pci/ahci_pci.c
===
RCS file: /cvs/src/sys/dev/pci/ahci_pci.c,v
retrieving revision 1.13
diff -u -p -r1.13 ahci_pci.c
--- dev/pci/ahci_pci.c  27 May 2017 14:16:45 -  1.13
+++ dev/pci/ahci_pci.c  2 Jan 2018 22:07:30 -
@@ -109,6 +109,13 @@ static const struct ahci_device ahci_dev
{ PCI_VENDOR_ATI,   PCI_PRODUCT_ATI_SBX00_SATA_6,
NULL,   ahci_ati_sb700_attach },
 
+   { PCI_VENDOR_ASMEDIA,   PCI_PRODUCT_ASMEDIA_ASM1061_SATA },
+
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_6SERIES_AHCI_1,
NULL,   ahci_intel_attach },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_6SERIES_AHCI_2,

Bug in dd's args.c, invalid check for error

[Bulat Musin]

/bin/dd/args.c r1.28

get_off(char *val)

Code:

num = strtoll(val, , 0);
if (num == LLONG_MAX)   /* Overflow. */
err(1, "%s", oper);
if (expr == val)/* No digits. */
errx(1, "%s: illegal numeric value", oper);

Incorrect checking of overflow.
Firstly, set errno to 0 before calling strtoll.
Secondly: check of errno == ERANGE. =>

errno = 0;
num = strtoll(val, , 0);
if (errno == ERANGE && num == LLONG_MAX)/* Overflow. */
err(1, "%s", oper);
if (expr == val)/* No digits. */
errx(1, "%s: illegal numeric value", oper);


https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/bin/dd/args.c?rev=1.28

Re: pckbd: go back to using table 2 by default

[Dave Voutila]

> On Jan 2, 2018, at 13:37, Job Snijders  wrote:
> 
> I often observed on my Thinkpad x270 that after an upgrade via bsd.rd,
> the first reboot resulted in keystrokes being garbage (and at second
> reboot everything was fine again).

You just explained why I saw what I saw this morning post-upgrade on my x270. I 
feel better now knowing it wasn’t just me. :-)

-Dave

Re: pckbd: go back to using table 2 by default

[Job Snijders]

Hi all,

I often observed on my Thinkpad x270 that after an upgrade via bsd.rd,
the first reboot resulted in keystrokes being garbage (and at second
reboot everything was fine again).

The below patch seems to be an improvement.

Kind regards,

Job

On Tue, Jan 02, 2018 at 09:36:49AM -0600, joshua stein wrote:
> In 2007 I changed this code to use table 3 by default, falling back 
> on table 2 (the previous default) or 1.  This was done just to make 
> the keyboard on the OQO model 01 work, and these devices are long 
> gone.
> 
> 10 years later, some newer Lenovo machines seem to have trouble 
> trying this mode which can occasionally leave the keyboard in a 
> state where it generates bogus keys when typing.  It also causes a 
> long delay when booting because the table changes have to timeout:
> 
> pckbd: trying table 3
> pckbc_cmd: lost 0xee
> pckbc_cmd: timeout
> pckbd: table set of 3 failed
> pckbd: trying table 2
> pckbc_cmd: lost 0xee
> pckbc_cmd: timeout
> pckbd: table set of 2 failed
> pckbd: trying table 1
> pckbc_cmd: lost 0xee
> pckbc_cmd: timeout
> pckbd: table set of 1 failed
> pckbd: settling on table 1
> 
> This patch reverts back to using table 2 by default and if it says 
> it was already in table 2, leaves it alone rather than forcibly 
> changing to that mode again, which is how Linux behaves.
> 
> 
> Index: sys/dev/pckbc/pckbd.c
> ===
> RCS file: /cvs/src/sys/dev/pckbc/pckbd.c,v
> retrieving revision 1.43
> diff -u -p -u -p -r1.43 pckbd.c
> --- sys/dev/pckbc/pckbd.c 14 Apr 2016 07:06:03 -  1.43
> +++ sys/dev/pckbc/pckbd.c 2 Jan 2018 15:21:05 -
> @@ -214,8 +214,7 @@ int
>  pckbd_set_xtscancode(pckbc_tag_t kbctag, pckbc_slot_t kbcslot,
>  struct pckbd_internal *id)
>  {
> - /* default to have the 8042 translate the keyboard with table 3. */
> - int table = 3;
> + int table;
>  
>   if (pckbc_xt_translation(kbctag)) {
>  #ifdef DEBUG
> @@ -234,12 +233,21 @@ pckbd_set_xtscancode(pckbc_tag_t kbctag,
>   if (id != NULL)
>   id->t_translating = 0;
>   } else {
> - if (id != NULL)
> + if (id != NULL) {
>   id->t_translating = 1;
> + if (id->t_table == 0) {
> + /*
> +  * Don't bother explicitly setting into set 2,
> +  * it's the default.
> +  */
> + id->t_table = 2;
> + return (0);
> + }
> + }
>   }
>  
>   /* keep falling back until we hit a table that looks usable. */
> - for (; table >= 1; table--) {
> + for (table = 3; table >= 1; table--) {
>   u_char cmd[2];
>  #ifdef DEBUG
>   printf("pckbd: trying table %d\n", table);
> 

Re: sync i386

[Martin Pieuchot]

On 02/01/18(Tue) 17:35, Mark Kettenis wrote:
> > Date: Tue, 2 Jan 2018 16:02:16 +0100
> > From: Martin Pieuchot 
> > 
> > We're no longer using the 'mtx_lock' field, so remove it.
> > 
> > While here remove the 'volatile' keyword from amd64's 'struct mutex'.
> 
> You mean the mtx_owner member.
> 
> > ok?
> 
> I think that is wrong.  Unless we always access that member using
> explicit atomic operations, we have to prevent the compiler from
> thinking that the variable can't change behind its back.  So mtx_owner
> needs to be volatile everywhere.

Fine with me, updated diff below.

Index: arch/alpha/include/mutex.h
===
RCS file: /cvs/src/sys/arch/alpha/include/mutex.h,v
retrieving revision 1.8
diff -u -p -r1.8 mutex.h
--- arch/alpha/include/mutex.h  20 Apr 2017 13:57:29 -  1.8
+++ arch/alpha/include/mutex.h  2 Jan 2018 16:44:58 -
@@ -31,7 +31,7 @@
 #include 
 
 struct mutex {
-   void *mtx_owner;
+   volatile void *mtx_owner;
int mtx_wantipl;
int mtx_oldipl;
 #ifdef WITNESS
Index: arch/hppa/include/mutex.h
===
RCS file: /cvs/src/sys/arch/hppa/include/mutex.h,v
retrieving revision 1.7
diff -u -p -r1.7 mutex.h
--- arch/hppa/include/mutex.h   20 Apr 2017 13:57:29 -  1.7
+++ arch/hppa/include/mutex.h   2 Jan 2018 16:45:07 -
@@ -39,7 +39,7 @@ struct mutex {
 #endif
int mtx_wantipl;
int mtx_oldipl;
-   void *mtx_owner;
+   volatile void *mtx_owner;
 #ifdef WITNESS
struct lock_object mtx_lock_obj;
 #endif
Index: arch/i386/i386/genassym.cf
===
RCS file: /cvs/src/sys/arch/i386/i386/genassym.cf,v
retrieving revision 1.39
diff -u -p -r1.39 genassym.cf
--- arch/i386/i386/genassym.cf  15 Mar 2016 03:17:51 -  1.39
+++ arch/i386/i386/genassym.cf  2 Jan 2018 14:56:57 -
@@ -136,7 +136,6 @@ member  ih_next
 endif
 
 struct mutex
-member mtx_lock
 member mtx_wantipl
 member mtx_oldipl
 member mtx_owner
Index: arch/i386/include/mutex.h
===
RCS file: /cvs/src/sys/arch/i386/include/mutex.h,v
retrieving revision 1.9
diff -u -p -r1.9 mutex.h
--- arch/i386/include/mutex.h   20 Apr 2017 13:57:29 -  1.9
+++ arch/i386/include/mutex.h   2 Jan 2018 16:45:13 -
@@ -29,15 +29,10 @@
 
 #include 
 
-/*
- * XXX - we don't really need the mtx_lock field, we can use mtx_oldipl
- *  as the lock to save some space.
- */
 struct mutex {
-   volatile int mtx_lock;
int mtx_wantipl;
int mtx_oldipl;
-   void *mtx_owner;
+   volatile void *mtx_owner;
 #ifdef WITNESS
struct lock_object mtx_lock_obj;
 #endif
@@ -59,10 +54,10 @@ struct mutex {
 
 #ifdef WITNESS
 #define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
-   { 0, __MUTEX_IPL(ipl), 0, NULL, MTX_LO_INITIALIZER(name, flags) }
+   { __MUTEX_IPL((ipl)), 0, NULL, MTX_LO_INITIALIZER(name, flags) }
 #else
 #define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
-   { 0, __MUTEX_IPL(ipl), 0, NULL }
+   { __MUTEX_IPL((ipl)), 0, NULL }
 #endif
 
 void __mtx_init(struct mutex *, int);
Index: arch/m88k/include/mutex.h
===
RCS file: /cvs/src/sys/arch/m88k/include/mutex.h,v
retrieving revision 1.5
diff -u -p -r1.5 mutex.h
--- arch/m88k/include/mutex.h   20 Apr 2017 13:57:29 -  1.5
+++ arch/m88k/include/mutex.h   2 Jan 2018 16:45:17 -
@@ -33,7 +33,7 @@ struct mutex {
volatile int mtx_lock;  /* mutex.S relies upon this field being first */
int mtx_wantipl;
int mtx_oldipl;
-   void *mtx_owner;
+   volatile void *mtx_owner;
 #ifdef WITNESS
struct lock_object mtx_lock_obj;
 #endif
Index: arch/mips64/include/mutex.h
===
RCS file: /cvs/src/sys/arch/mips64/include/mutex.h,v
retrieving revision 1.2
diff -u -p -r1.2 mutex.h
--- arch/mips64/include/mutex.h 20 Apr 2017 13:57:30 -  1.2
+++ arch/mips64/include/mutex.h 2 Jan 2018 16:45:23 -
@@ -31,7 +31,7 @@
 #include 
 
 struct mutex {
-   void *mtx_owner;
+   volatile void *mtx_owner;
int mtx_wantipl;
int mtx_oldipl;
 #ifdef WITNESS

Re: sync i386

[Mark Kettenis]

> Date: Tue, 2 Jan 2018 16:02:16 +0100
> From: Martin Pieuchot 
> 
> We're no longer using the 'mtx_lock' field, so remove it.
> 
> While here remove the 'volatile' keyword from amd64's 'struct mutex'.

You mean the mtx_owner member.

> ok?

I think that is wrong.  Unless we always access that member using
explicit atomic operations, we have to prevent the compiler from
thinking that the variable can't change behind its back.  So mtx_owner
needs to be volatile everywhere.

> Index: i386/i386/genassym.cf
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/genassym.cf,v
> retrieving revision 1.39
> diff -u -p -r1.39 genassym.cf
> --- i386/i386/genassym.cf 15 Mar 2016 03:17:51 -  1.39
> +++ i386/i386/genassym.cf 2 Jan 2018 14:56:57 -
> @@ -136,7 +136,6 @@ memberih_next
>  endif
>  
>  struct mutex
> -member   mtx_lock
>  member   mtx_wantipl
>  member   mtx_oldipl
>  member   mtx_owner
> Index: i386/include/mutex.h
> ===
> RCS file: /cvs/src/sys/arch/i386/include/mutex.h,v
> retrieving revision 1.9
> diff -u -p -r1.9 mutex.h
> --- i386/include/mutex.h  20 Apr 2017 13:57:29 -  1.9
> +++ i386/include/mutex.h  2 Jan 2018 14:59:07 -
> @@ -29,12 +29,7 @@
>  
>  #include 
>  
> -/*
> - * XXX - we don't really need the mtx_lock field, we can use mtx_oldipl
> - *as the lock to save some space.
> - */
>  struct mutex {
> - volatile int mtx_lock;
>   int mtx_wantipl;
>   int mtx_oldipl;
>   void *mtx_owner;
> @@ -59,10 +54,10 @@ struct mutex {
>  
>  #ifdef WITNESS
>  #define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
> - { 0, __MUTEX_IPL(ipl), 0, NULL, MTX_LO_INITIALIZER(name, flags) }
> + { __MUTEX_IPL((ipl)), 0, NULL, MTX_LO_INITIALIZER(name, flags) }
>  #else
>  #define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
> - { 0, __MUTEX_IPL(ipl), 0, NULL }
> + { __MUTEX_IPL((ipl)), 0, NULL }
>  #endif
>  
>  void __mtx_init(struct mutex *, int);
> Index: amd64/include/mutex.h
> ===
> RCS file: /cvs/src/sys/arch/amd64/include/mutex.h,v
> retrieving revision 1.8
> diff -u -p -r1.8 mutex.h
> --- amd64/include/mutex.h 20 Apr 2017 13:57:29 -  1.8
> +++ amd64/include/mutex.h 2 Jan 2018 14:59:16 -
> @@ -32,7 +32,7 @@
>  struct mutex {
>   int mtx_wantipl;
>   int mtx_oldipl;
> - volatile void *mtx_owner;
> + void *mtx_owner;
>  #ifdef WITNESS
>   struct lock_object mtx_lock_obj;
>  #endif
> 
> 

Re: openbsd code coverage

[Marc Espie]

On Tue, Jan 02, 2018 at 12:40:33AM +0100, Aron Diehl wrote:
> Hi,
> 
> 
> I'm working on measuring OpenBSD code coverage.
> 
> 
> Code coverage of what? The regression-tests make target?
> 
> Cheers
> Aron

Obviously.

ktrace firefox freeze my box

[Martin Pieuchot]

on -current amd64, simply doing "$ ktrace -p $pid_of_firefox" is enough
to freeze my box:

OpenBSD 6.2-current (GENERIC.MP) #313: Mon Jan  1 17:51:21 MST 2018
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8238301184 (7856MB)
avail mem = 7981707264 (7611MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xccbfd000 (65 entries)
bios0: vendor LENOVO version "N14ET26W (1.04 )" date 01/23/2015
bios0: LENOVO 20BS006BGE
acpi0 at bios0: rev 2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP SLIC ASF! HPET ECDT APIC MCFG SSDT SSDT SSDT SSDT SSDT 
SSDT SSDT SSDT SSDT SSDT PCCT SSDT UEFI MSDM BATB FPDT UEFI DMAR
acpi0: wakeup devices LID_(S4) SLPB(S3) IGBE(S4) EXP2(S4) XHCI(S3) EHC1(S3)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 14318179 Hz
acpiec0 at acpi0
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-5500U CPU @ 2.40GHz, 2295.07 MHz
cpu0: 
FPU,VME,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,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SENSOR,ARAT
cpu0: 256KB 64b/line 8-way L2 cache
acpihpet0: recalibrated TSC frequency 2394462310 Hz
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM) i7-5500U CPU @ 2.40GHz, 2294.69 MHz
cpu1: 
FPU,VME,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,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SENSOR,ARAT
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 1, core 0, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: Intel(R) Core(TM) i7-5500U CPU @ 2.40GHz, 2294.69 MHz
cpu2: 
FPU,VME,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,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SENSOR,ARAT
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 1, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: Intel(R) Core(TM) i7-5500U CPU @ 2.40GHz, 2294.69 MHz
cpu3: 
FPU,VME,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,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SENSOR,ARAT
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 1, core 1, package 0
ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 40 pins
acpimcfg0 at acpi0 addr 0xf800, bus 0-63
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus -1 (PEG_)
acpiprt2 at acpi0: bus 3 (EXP1)
acpiprt3 at acpi0: bus 4 (EXP2)
acpiprt4 at acpi0: bus -1 (EXP3)
acpiprt5 at acpi0: bus -1 (EXP6)
acpicpu0 at acpi0: C3(200@233 mwait.1@0x40), C2(200@148 mwait.1@0x33), 
C1(1000@1 mwait.1), PSS
acpicpu1 at acpi0: C3(200@233 mwait.1@0x40), C2(200@148 mwait.1@0x33), 
C1(1000@1 mwait.1), PSS
acpicpu2 at acpi0: C3(200@233 mwait.1@0x40), C2(200@148 mwait.1@0x33), 
C1(1000@1 mwait.1), PSS
acpicpu3 at acpi0: C3(200@233 mwait.1@0x40), C2(200@148 mwait.1@0x33), 
C1(1000@1 mwait.1), PSS
acpipwrres0 at acpi0: PUBS, resource for XHCI, EHC1
acpipwrres1 at acpi0: NVP3, resource for PEG_
acpipwrres2 at acpi0: NVP2, resource for PEG_
acpitz0 at acpi0: critical temperature is 128 degC
acpibtn0 at acpi0: LID_
acpibtn1 at acpi0: SLPB
"LEN0071" at acpi0 not configured
"LEN0048" at acpi0 not configured
acpibat0 at acpi0: BAT0 model "00HW003" serial   887 type LiP oem "SMP"
acpiac0 at acpi0: AC unit online
acpithinkpad0 at acpi0
"PNP0C14" at acpi0 not configured
"PNP0C14" at acpi0 not configured
"PNP0C14" at acpi0 not configured
"INT340F" at acpi0 not configured
acpivideo0 at acpi0: VID_
acpivout at acpivideo0 not configured
acpivideo1 at acpi0: VID_
cpu0: Enhanced SpeedStep 2295 MHz: speeds: 2401, 2400, 2300, 2100, 2000, 1900, 
1700, 1600, 1400, 1300, 1200, 1000, 900, 800, 600, 500 MHz
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel Core 5G Host" rev 0x09

pckbd: go back to using table 2 by default

[joshua stein]

In 2007 I changed this code to use table 3 by default, falling back 
on table 2 (the previous default) or 1.  This was done just to make 
the keyboard on the OQO model 01 work, and these devices are long 
gone.

10 years later, some newer Lenovo machines seem to have trouble 
trying this mode which can occasionally leave the keyboard in a 
state where it generates bogus keys when typing.  It also causes a 
long delay when booting because the table changes have to timeout:

pckbd: trying table 3
pckbc_cmd: lost 0xee
pckbc_cmd: timeout
pckbd: table set of 3 failed
pckbd: trying table 2
pckbc_cmd: lost 0xee
pckbc_cmd: timeout
pckbd: table set of 2 failed
pckbd: trying table 1
pckbc_cmd: lost 0xee
pckbc_cmd: timeout
pckbd: table set of 1 failed
pckbd: settling on table 1

This patch reverts back to using table 2 by default and if it says 
it was already in table 2, leaves it alone rather than forcibly 
changing to that mode again, which is how Linux behaves.


Index: sys/dev/pckbc/pckbd.c
===
RCS file: /cvs/src/sys/dev/pckbc/pckbd.c,v
retrieving revision 1.43
diff -u -p -u -p -r1.43 pckbd.c
--- sys/dev/pckbc/pckbd.c   14 Apr 2016 07:06:03 -  1.43
+++ sys/dev/pckbc/pckbd.c   2 Jan 2018 15:21:05 -
@@ -214,8 +214,7 @@ int
 pckbd_set_xtscancode(pckbc_tag_t kbctag, pckbc_slot_t kbcslot,
 struct pckbd_internal *id)
 {
-   /* default to have the 8042 translate the keyboard with table 3. */
-   int table = 3;
+   int table;
 
if (pckbc_xt_translation(kbctag)) {
 #ifdef DEBUG
@@ -234,12 +233,21 @@ pckbd_set_xtscancode(pckbc_tag_t kbctag,
if (id != NULL)
id->t_translating = 0;
} else {
-   if (id != NULL)
+   if (id != NULL) {
id->t_translating = 1;
+   if (id->t_table == 0) {
+   /*
+* Don't bother explicitly setting into set 2,
+* it's the default.
+*/
+   id->t_table = 2;
+   return (0);
+   }
+   }
}
 
/* keep falling back until we hit a table that looks usable. */
-   for (; table >= 1; table--) {
+   for (table = 3; table >= 1; table--) {
u_char cmd[2];
 #ifdef DEBUG
printf("pckbd: trying table %d\n", table);

TCP/UDP/etc input path w/o KERNEL_LOCK()

[Martin Pieuchot]

New year, new diff.  Assuming we can live with the kqueue(2) races,
here's a diff to remove the KERNEL_LOCK() from all pr_input() routines.

I'd appreciate tests.

Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.533
diff -u -p -r1.533 if.c
--- net/if.c2 Jan 2018 12:52:17 -   1.533
+++ net/if.c2 Jan 2018 14:32:05 -
@@ -926,7 +926,6 @@ if_netisr(void *unused)
 {
int n, t = 0;
 
-   KERNEL_LOCK();
NET_LOCK();
 
while ((n = netisr) != 0) {
@@ -940,8 +939,11 @@ if_netisr(void *unused)
atomic_clearbits_int(, n);
 
 #if NETHER > 0
-   if (n & (1 << NETISR_ARP))
+   if (n & (1 << NETISR_ARP)) {
+   KERNEL_LOCK();
arpintr();
+   KERNEL_UNLOCK();
+   }
 #endif
if (n & (1 << NETISR_IP))
ipintr();
@@ -950,35 +952,52 @@ if_netisr(void *unused)
ip6intr();
 #endif
 #if NPPP > 0
-   if (n & (1 << NETISR_PPP))
+   if (n & (1 << NETISR_PPP)) {
+   KERNEL_LOCK();
pppintr();
+   KERNEL_UNLOCK();
+   }
 #endif
 #if NBRIDGE > 0
-   if (n & (1 << NETISR_BRIDGE))
+   if (n & (1 << NETISR_BRIDGE)) {
+   KERNEL_LOCK();
bridgeintr();
+   KERNEL_UNLOCK();
+   }
 #endif
 #if NSWITCH > 0
-   if (n & (1 << NETISR_SWITCH))
+   if (n & (1 << NETISR_SWITCH)) {
+   KERNEL_LOCK();
switchintr();
+   KERNEL_UNLOCK();
+   }
 #endif
 #if NPPPOE > 0
-   if (n & (1 << NETISR_PPPOE))
+   if (n & (1 << NETISR_PPPOE)) {
+   KERNEL_LOCK();
pppoeintr();
+   KERNEL_UNLOCK();
+   }
 #endif
 #ifdef PIPEX
-   if (n & (1 << NETISR_PIPEX))
+   if (n & (1 << NETISR_PIPEX)) {
+   KERNEL_LOCK();
pipexintr();
+   KERNEL_UNLOCK();
+   }
 #endif
t |= n;
}
 
 #if NPFSYNC > 0
-   if (t & (1 << NETISR_PFSYNC))
+   if (t & (1 << NETISR_PFSYNC)) {
+   KERNEL_LOCK();
pfsyncintr();
+   KERNEL_UNLOCK();
+   }
 #endif
 
NET_UNLOCK();
-   KERNEL_UNLOCK();
 }
 
 void

sync i386

[Martin Pieuchot]

We're no longer using the 'mtx_lock' field, so remove it.

While here remove the 'volatile' keyword from amd64's 'struct mutex'.

ok?

Index: i386/i386/genassym.cf
===
RCS file: /cvs/src/sys/arch/i386/i386/genassym.cf,v
retrieving revision 1.39
diff -u -p -r1.39 genassym.cf
--- i386/i386/genassym.cf   15 Mar 2016 03:17:51 -  1.39
+++ i386/i386/genassym.cf   2 Jan 2018 14:56:57 -
@@ -136,7 +136,6 @@ member  ih_next
 endif
 
 struct mutex
-member mtx_lock
 member mtx_wantipl
 member mtx_oldipl
 member mtx_owner
Index: i386/include/mutex.h
===
RCS file: /cvs/src/sys/arch/i386/include/mutex.h,v
retrieving revision 1.9
diff -u -p -r1.9 mutex.h
--- i386/include/mutex.h20 Apr 2017 13:57:29 -  1.9
+++ i386/include/mutex.h2 Jan 2018 14:59:07 -
@@ -29,12 +29,7 @@
 
 #include 
 
-/*
- * XXX - we don't really need the mtx_lock field, we can use mtx_oldipl
- *  as the lock to save some space.
- */
 struct mutex {
-   volatile int mtx_lock;
int mtx_wantipl;
int mtx_oldipl;
void *mtx_owner;
@@ -59,10 +54,10 @@ struct mutex {
 
 #ifdef WITNESS
 #define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
-   { 0, __MUTEX_IPL(ipl), 0, NULL, MTX_LO_INITIALIZER(name, flags) }
+   { __MUTEX_IPL((ipl)), 0, NULL, MTX_LO_INITIALIZER(name, flags) }
 #else
 #define MUTEX_INITIALIZER_FLAGS(ipl, name, flags) \
-   { 0, __MUTEX_IPL(ipl), 0, NULL }
+   { __MUTEX_IPL((ipl)), 0, NULL }
 #endif
 
 void __mtx_init(struct mutex *, int);
Index: amd64/include/mutex.h
===
RCS file: /cvs/src/sys/arch/amd64/include/mutex.h,v
retrieving revision 1.8
diff -u -p -r1.8 mutex.h
--- amd64/include/mutex.h   20 Apr 2017 13:57:29 -  1.8
+++ amd64/include/mutex.h   2 Jan 2018 14:59:16 -
@@ -32,7 +32,7 @@
 struct mutex {
int mtx_wantipl;
int mtx_oldipl;
-   volatile void *mtx_owner;
+   void *mtx_owner;
 #ifdef WITNESS
struct lock_object mtx_lock_obj;
 #endif

Re: Export IPsec flows via snmpd(8)

[Martin Pieuchot]

On 19/12/17(Tue) 18:06, Marco Pfatschbacher wrote:
> On Tue, Dec 19, 2017 at 12:43:48PM +0100, Martin Pieuchot wrote:
> > I'd like to see some information about my tunnels in my NMS.
> 
> Nice. I would find that very useful :)
> 
> > The problem is that there's not standard MIB for this and most vendor
> > MIBs are huge and are not easy to implement.
> 
> What about https://tools.ietf.org/html/rfc4807 ?

This MIB is about the "Policy Database Configuration" which, as far as I
understand, would be useful to export the content of isakmpd.policy(5).

I'm more interested into something like the "IPsec Flow Monitoring"
https://www.ietf.org/archive/id/draft-ietf-ipsec-flow-monitoring-mib-02.txt
However this is an archived & expired draft.

So I looked at both Cisco & Juniper MIBs, but implementing any of them
is a lot of work and do not always make sense with our IPsec stack.
That's why I'm asking for inputs :)

Re: look up interface names for IPv6 "cannot forward" errors

[Martin Pieuchot]

On 02/01/18(Tue) 20:44, Darren Tucker wrote:
> I rearranged my network and ended up getting these in messages:
> 
> cannot forward src fe80:9::a691:b1ff:fe1f:d2a0, dst 
> 2001:44b8:3110:fb00:200:5eff:fe00:10a, nxt 58, rcvif 9, outif 1
> 
> I guess I could run ifconfig and count interfaces, but I'm lazy and the
> kernel knows what they are.

No need to count, just look at the 'index' output in ifconfig(8).

> cannot forward src fe80:9::a691:b1ff:fe1f:d2a0, dst 
> 2001:44b8:3110:fb00:200:5eff:fe00:10a, nxt 58, rcvif carp0, outif em0
> 
> ok?  (caveat: kernel n00b, don't assume I know what I'm doing)
> 
> Index: netinet6/ip6_forward.c
> ===
> RCS file: /cvs/src/sys/netinet6/ip6_forward.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 ip6_forward.c
> --- netinet6/ip6_forward.c30 Jun 2017 11:29:15 -  1.95
> +++ netinet6/ip6_forward.c2 Jan 2018 08:44:38 -
> @@ -86,7 +86,7 @@ ip6_forward(struct mbuf *m, struct rtent
>  {
>   struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
>   struct sockaddr_in6 *dst, sin6;
> - struct ifnet *ifp = NULL;
> + struct ifnet *ifp = NULL, *ifp_dst = NULL;
>   int error = 0, type = 0, code = 0;
>   struct mbuf *mcopy = NULL;
>  #ifdef IPSEC
> @@ -197,12 +197,27 @@ reroute:
>   ip6_log_time = time_uptime;
>   inet_ntop(AF_INET6, >ip6_src, src6, sizeof(src6));
>   inet_ntop(AF_INET6, >ip6_dst, dst6, sizeof(dst6));
> - log(LOG_DEBUG,
> - "cannot forward "
> - "src %s, dst %s, nxt %d, rcvif %u, outif %u\n",
> - src6, dst6,
> - ip6->ip6_nxt,
> - m->m_pkthdr.ph_ifidx, rt->rt_ifidx);
> + ifp = if_get(m->m_pkthdr.ph_ifidx);
> + ifp_dst = if_get(rt->rt_ifidx);
> + if (ifp != NULL && ifp_dst != NULL) {
> + log(LOG_DEBUG,
> + "cannot forward src %s, dst %s, "
> + "nxt %d, rcvif %s, outif %s\n",
> + src6, dst6,
> + ip6->ip6_nxt,
> + ifp->if_xname, ifp_dst->if_xname);
> + } else {
> + log(LOG_DEBUG,
> + "cannot forward src %s, dst %s, "
> + "nxt %d, rcvif %u, outif %u\n",
> + src6, dst6,
> + ip6->ip6_nxt,
> + m->m_pkthdr.ph_ifidx, rt->rt_ifidx);
> + }
> + if_put(ifp_dst);
> + ifp_dst = NULL;
> + if_put(ifp);
> + ifp = NULL;
>   }
>   if (mcopy)
>   icmp6_error(mcopy, ICMP6_DST_UNREACH,
> 
> -- 
> Darren Tucker (dtucker at dtucker.net)
> GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
> Good judgement comes with experience. Unfortunately, the experience
> usually comes from bad judgement.
> 

look up interface names for IPv6 "cannot forward" errors

[Darren Tucker]

I rearranged my network and ended up getting these in messages:

cannot forward src fe80:9::a691:b1ff:fe1f:d2a0, dst 
2001:44b8:3110:fb00:200:5eff:fe00:10a, nxt 58, rcvif 9, outif 1

I guess I could run ifconfig and count interfaces, but I'm lazy and the
kernel knows what they are.

cannot forward src fe80:9::a691:b1ff:fe1f:d2a0, dst 
2001:44b8:3110:fb00:200:5eff:fe00:10a, nxt 58, rcvif carp0, outif em0

ok?  (caveat: kernel n00b, don't assume I know what I'm doing)

Index: netinet6/ip6_forward.c
===
RCS file: /cvs/src/sys/netinet6/ip6_forward.c,v
retrieving revision 1.95
diff -u -p -r1.95 ip6_forward.c
--- netinet6/ip6_forward.c  30 Jun 2017 11:29:15 -  1.95
+++ netinet6/ip6_forward.c  2 Jan 2018 08:44:38 -
@@ -86,7 +86,7 @@ ip6_forward(struct mbuf *m, struct rtent
 {
struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
struct sockaddr_in6 *dst, sin6;
-   struct ifnet *ifp = NULL;
+   struct ifnet *ifp = NULL, *ifp_dst = NULL;
int error = 0, type = 0, code = 0;
struct mbuf *mcopy = NULL;
 #ifdef IPSEC
@@ -197,12 +197,27 @@ reroute:
ip6_log_time = time_uptime;
inet_ntop(AF_INET6, >ip6_src, src6, sizeof(src6));
inet_ntop(AF_INET6, >ip6_dst, dst6, sizeof(dst6));
-   log(LOG_DEBUG,
-   "cannot forward "
-   "src %s, dst %s, nxt %d, rcvif %u, outif %u\n",
-   src6, dst6,
-   ip6->ip6_nxt,
-   m->m_pkthdr.ph_ifidx, rt->rt_ifidx);
+   ifp = if_get(m->m_pkthdr.ph_ifidx);
+   ifp_dst = if_get(rt->rt_ifidx);
+   if (ifp != NULL && ifp_dst != NULL) {
+   log(LOG_DEBUG,
+   "cannot forward src %s, dst %s, "
+   "nxt %d, rcvif %s, outif %s\n",
+   src6, dst6,
+   ip6->ip6_nxt,
+   ifp->if_xname, ifp_dst->if_xname);
+   } else {
+   log(LOG_DEBUG,
+   "cannot forward src %s, dst %s, "
+   "nxt %d, rcvif %u, outif %u\n",
+   src6, dst6,
+   ip6->ip6_nxt,
+   m->m_pkthdr.ph_ifidx, rt->rt_ifidx);
+   }
+   if_put(ifp_dst);
+   ifp_dst = NULL;
+   if_put(ifp);
+   ifp = NULL;
}
if (mcopy)
icmp6_error(mcopy, ICMP6_DST_UNREACH,

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.