Re: MP-safe TX for cnmac(4)

[Visa Hankala]

On Mon, Apr 25, 2016 at 04:06:08PM +0200, Martin Pieuchot wrote:
> On 25/04/16(Mon) 11:35, Visa Hankala wrote:
> > On Mon, Apr 25, 2016 at 10:20:32AM +0200, Martin Pieuchot wrote:
> > > On 24/04/16(Sun) 16:13, Visa Hankala wrote:
> > > > This adds MP-safe TX for cnmac(4). OK?
> > > 
> > > Would it be possible to do that without mutex?  Having the same pattern
> > > across most of our drivers would reduce the maintenance effort.
> > 
> > The only real use of the mutex is to keep octeon_eth_tick_free() away
> > from sc_sendq while the start routine is running. The queue tracks mbufs
> > that need to be freed after transmission. Unlike many other drivers,
> > there is no TX ring. As long as there is at least a little bit of
> > traffic, octeon_eth_start() can take care of draining the queue. The
> > hardware does not have a transmission complete interrupt, so the timer
> > is there to free transmitted mbufs in case traffic stops altogether for
> > a long time.
> > 
> > What driver would be a good example for this case?
> 
> I don't think we have any example for this case.  But it looks like the
> watchdog problem for which we schedule a task and call intr_barrier()
> after removing the IFF_RUNNING flag.  Now if you think it doesn't make
> sense I'm fine with your approach.

I revised the diff to call the stop routine in octeon_eth_watchdog().
The driver does not enable the watchdog at the moment, though.

Index: arch/octeon/dev/if_cnmac.c
===
RCS file: src/sys/arch/octeon/dev/if_cnmac.c,v
retrieving revision 1.38
diff -u -p -r1.38 if_cnmac.c
--- arch/octeon/dev/if_cnmac.c  13 Apr 2016 11:34:00 -  1.38
+++ arch/octeon/dev/if_cnmac.c  26 Apr 2016 04:52:57 -
@@ -285,6 +285,7 @@ octeon_eth_attach(struct device *parent,
octeon_eth_gsc[sc->sc_port] = sc;
 
ml_init(>sc_sendq);
+   mtx_init(>sc_sendq_mtx, IPL_NET);
sc->sc_soft_req_thresh = 15/* XXX */;
sc->sc_ext_callback_cnt = 0;
 
@@ -317,6 +318,7 @@ octeon_eth_attach(struct device *parent,
strncpy(ifp->if_xname, sc->sc_dev.dv_xname, sizeof(ifp->if_xname));
ifp->if_softc = sc;
ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
+   ifp->if_xflags = IFXF_MPSAFE;
ifp->if_ioctl = octeon_eth_ioctl;
ifp->if_start = octeon_eth_start;
ifp->if_watchdog = octeon_eth_watchdog;
@@ -742,7 +744,7 @@ octeon_eth_ioctl(struct ifnet *ifp, u_lo
error = 0;
}
 
-   octeon_eth_start(ifp);
+   if_start(ifp);
 
splx(s);
return (error);
@@ -959,18 +961,19 @@ octeon_eth_start(struct ifnet *ifp)
struct octeon_eth_softc *sc = ifp->if_softc;
struct mbuf *m;
 
+   if (__predict_false(!cn30xxgmx_link_status(sc->sc_gmx_port))) {
+   IFQ_PURGE(>if_snd);
+   return;
+   }
+
+   mtx_enter(>sc_sendq_mtx);
+
/*
 * performance tuning
 * presend iobdma request 
 */
octeon_eth_send_queue_flush_prefetch(sc);
 
-   if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(>if_snd))
-   goto last;
-
-   if (__predict_false(!cn30xxgmx_link_status(sc->sc_gmx_port)))
-   goto last;
-
for (;;) {
octeon_eth_send_queue_flush_fetch(sc); /* XXX */
 
@@ -980,13 +983,16 @@ octeon_eth_start(struct ifnet *ifp)
 * and bail out.
 */
if (octeon_eth_send_queue_is_full(sc)) {
+   mtx_leave(>sc_sendq_mtx);
return;
}
/* XXX */
 
IFQ_DEQUEUE(>if_snd, m);
-   if (m == NULL)
+   if (m == NULL) {
+   mtx_leave(>sc_sendq_mtx);
return;
+   }
 
OCTEON_ETH_TAP(ifp, m, BPF_DIRECTION_OUT);
 
@@ -1008,8 +1014,9 @@ octeon_eth_start(struct ifnet *ifp)
octeon_eth_send_queue_flush_prefetch(sc);
}
 
-last:
octeon_eth_send_queue_flush_fetch(sc);
+
+   mtx_leave(>sc_sendq_mtx);
 }
 
 void
@@ -1019,13 +1026,15 @@ octeon_eth_watchdog(struct ifnet *ifp)
 
printf("%s: device timeout\n", sc->sc_dev.dv_xname);
 
+   octeon_eth_stop(ifp, 0);
+
octeon_eth_configure(sc);
 
SET(ifp->if_flags, IFF_RUNNING);
ifq_clr_oactive(>if_snd);
ifp->if_timer = 0;
 
-   octeon_eth_start(ifp);
+   if_start(ifp);
 }
 
 int
@@ -1066,6 +1075,8 @@ octeon_eth_stop(struct ifnet *ifp, int d
 {
struct octeon_eth_softc *sc = ifp->if_softc;
 
+   CLR(ifp->if_flags, IFF_RUNNING);
+
timeout_del(>sc_tick_misc_ch);
timeout_del(>sc_tick_free_ch);
timeout_del(>sc_resume_ch);
@@ -1074,13 +1085,12 @@ octeon_eth_stop(struct ifnet *ifp, int d
 
cn30xxgmx_port_enable(sc->sc_gmx_port, 0);
 
-   /* Mark the interface as down and cancel the watchdog timer. */
-   

Re: doas: adjust yyerror() output

[Ted Unangst]

Gleydson Soares wrote:
> 
> I just stumbled over this...
> 
> % doas abc
> syntax error at line 1
> % 
> 
> I took some secs trying to figure out what was wrong with abc's command 
> syntax that I typed out. 
> But bingo,  It was happenned due my doas.conf has a syntax error...  
> Seems that yyerror() doesn't print out the progname's string, 
> 
> sounds better for a quick diagnosis?
> 
> % doas abc
> doas: syntax error at line 1
> %
> 
> sounds better for a quick diagnosis?
> OK?

what about just printing "doas: "?

> 
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.bin/doas/parse.y,v
> retrieving revision 1.14
> diff -u -p -r1.14 parse.y
> --- parse.y   4 Dec 2015 09:41:49 -   1.14
> +++ parse.y   26 Apr 2016 01:37:57 -
> @@ -176,6 +176,7 @@ yyerror(const char *fmt, ...)
>  {
>   va_list va;
>  
> + fprintf(stderr, "%s: ", getprogname());
>   va_start(va, fmt);
>   vfprintf(stderr, fmt, va);
>   va_end(va);
> 

failure to send a udp packet is not a fatal error

[David Gwynne]

the tftp proxy on the firewall is dying these days. i managed to
track the failure down to an error sending the udp packet on.

rather than err, i think it more appropriate to warn and let the
client retry in this situation.

ok?

Index: tftp-proxy.c
===
RCS file: /cvs/src/usr.sbin/tftp-proxy/tftp-proxy.c,v
retrieving revision 1.18
diff -u -p -r1.18 tftp-proxy.c
--- tftp-proxy.c24 Feb 2016 16:34:47 -  1.18
+++ tftp-proxy.c26 Apr 2016 03:41:39 -
@@ -869,7 +869,7 @@ unprivproc_pop(int fd, short events, voi
if (sendto(s, r->buf, r->buflen, 0,
(struct sockaddr *)>addrs.dst,
r->addrs.dst.ss_len) == -1)
-   lerr(1, "%s: unable to send", __func__);
+   lwarn("%s: unable to send", __func__);
 
close(s);
 

doas: adjust yyerror() output

[Gleydson Soares]

I just stumbled over this...

% doas abc
syntax error at line 1
% 

I took some secs trying to figure out what was wrong with abc's command syntax 
that I typed out. 
But bingo,  It was happenned due my doas.conf has a syntax error...  
Seems that yyerror() doesn't print out the progname's string, 

sounds better for a quick diagnosis?

% doas abc
doas: syntax error at line 1
%

sounds better for a quick diagnosis?
OK?

Index: parse.y
===
RCS file: /cvs/src/usr.bin/doas/parse.y,v
retrieving revision 1.14
diff -u -p -r1.14 parse.y
--- parse.y 4 Dec 2015 09:41:49 -   1.14
+++ parse.y 26 Apr 2016 01:37:57 -
@@ -176,6 +176,7 @@ yyerror(const char *fmt, ...)
 {
va_list va;
 
+   fprintf(stderr, "%s: ", getprogname());
va_start(va, fmt);
vfprintf(stderr, fmt, va);
va_end(va);

Re: match RTS522A in rtsx(4)

[Bryan Vyhmeister]

On Tue, Apr 26, 2016 at 09:55:38AM +1000, Jonathan Gray wrote:
> We normally only add usb devices to usbdevs if they are used in drivers
> and sometimes if the device doesn't provide a string of it's own.
> 
> I'd be surprised if a string is not supplied by the device in this case.

You're right. It does have a longer string. I didn't realize that was
how usbdevs worked. Thanks for the clarification.

Bryan

Re: more systrace mop up 2/2: man8.${ARCH}/MAKEDEV.8

[Theo Buehler]

On Mon, Apr 25, 2016 at 09:11:35PM -0400, Ted Unangst wrote:
> Theo Buehler wrote:
> > Index: share/man/man8//man8.alpha/MAKEDEV.8
> 
> please note the comment at the top of these files. they are autogenerated.

:(

and now i see that i simply missed the update in that part of the
tree... sorry for the noise

Re: more systrace mop up 2/2: man8.${ARCH}/MAKEDEV.8

[Ted Unangst]

Theo Buehler wrote:
> Index: share/man/man8//man8.alpha/MAKEDEV.8

please note the comment at the top of these files. they are autogenerated.

more systrace mop up 2/2: man8.${ARCH}/MAKEDEV.8

[Theo Buehler]

Index: share/man/man8//man8.alpha/MAKEDEV.8
===
RCS file: /var/cvs/src/share/man/man8/man8.alpha/MAKEDEV.8,v
retrieving revision 1.70
diff -u -p -r1.70 MAKEDEV.8
--- share/man/man8//man8.alpha/MAKEDEV.812 Mar 2016 18:02:18 -  
1.70
+++ share/man/man8//man8.alpha/MAKEDEV.825 Apr 2016 23:46:41 -
@@ -235,9 +235,6 @@ Raw MIDI devices, see
 .It Ar speaker
 PC speaker, see
 .Xr spkr 4 .
-.It Ar systrace*
-System call tracing device, see
-.Xr systrace 4 .
 .It Ar tun*
 Network tunnel driver, see
 .Xr tun 4 .
Index: share/man/man8//man8.amd64/MAKEDEV.8
===
RCS file: /var/cvs/src/share/man/man8/man8.amd64/MAKEDEV.8,v
retrieving revision 1.78
diff -u -p -r1.78 MAKEDEV.8
--- share/man/man8//man8.amd64/MAKEDEV.812 Mar 2016 18:02:18 -  
1.78
+++ share/man/man8//man8.amd64/MAKEDEV.825 Apr 2016 23:46:47 -
@@ -256,9 +256,6 @@ Raw MIDI devices, see
 .It Ar speaker
 PC speaker, see
 .Xr spkr 4 .
-.It Ar systrace*
-System call tracing device, see
-.Xr systrace 4 .
 .It Ar tun*
 Network tunnel driver, see
 .Xr tun 4 .
Index: share/man/man8//man8.armish/MAKEDEV.8
===
RCS file: /var/cvs/src/share/man/man8/man8.armish/MAKEDEV.8,v
retrieving revision 1.43
diff -u -p -r1.43 MAKEDEV.8
--- share/man/man8//man8.armish/MAKEDEV.8   12 Mar 2016 18:02:18 -  
1.43
+++ share/man/man8//man8.armish/MAKEDEV.8   25 Apr 2016 23:46:51 -
@@ -221,9 +221,6 @@ Ethernet tunnel driver, see
 .It Ar uk*
 Unknown SCSI devices, see
 .Xr uk 4 .
-.It Ar systrace*
-System call tracing device, see
-.Xr systrace 4 .
 .It Ar tuner*
 Tuner devices, see
 .Xr bktr 4 .
Index: share/man/man8//man8.armv7/MAKEDEV.8
===
RCS file: /var/cvs/src/share/man/man8/man8.armv7/MAKEDEV.8,v
retrieving revision 1.10
diff -u -p -r1.10 MAKEDEV.8
--- share/man/man8//man8.armv7/MAKEDEV.812 Mar 2016 18:02:18 -  
1.10
+++ share/man/man8//man8.armv7/MAKEDEV.825 Apr 2016 23:46:55 -
@@ -224,9 +224,6 @@ Ethernet tunnel driver, see
 .It Ar uk*
 Unknown SCSI devices, see
 .Xr uk 4 .
-.It Ar systrace*
-System call tracing device, see
-.Xr systrace 4 .
 .It Ar tuner*
 Tuner devices, see
 .Xr bktr 4 .
Index: share/man/man8//man8.hppa/MAKEDEV.8
===
RCS file: /var/cvs/src/share/man/man8/man8.hppa/MAKEDEV.8,v
retrieving revision 1.65
diff -u -p -r1.65 MAKEDEV.8
--- share/man/man8//man8.hppa/MAKEDEV.8 12 Mar 2016 18:02:18 -  1.65
+++ share/man/man8//man8.hppa/MAKEDEV.8 25 Apr 2016 23:47:01 -
@@ -217,9 +217,6 @@ PPP Multiplexer, see
 .It Ar *random
 In-kernel random data source, see
 .Xr random 4 .
-.It Ar systrace*
-System call tracing device, see
-.Xr systrace 4 .
 .It Ar tun*
 Network tunnel driver, see
 .Xr tun 4 .
Index: share/man/man8//man8.hppa64/MAKEDEV.8
===
RCS file: /var/cvs/src/share/man/man8/man8.hppa64/MAKEDEV.8,v
retrieving revision 1.45
diff -u -p -r1.45 MAKEDEV.8
--- share/man/man8//man8.hppa64/MAKEDEV.8   12 Mar 2016 18:02:18 -  
1.45
+++ share/man/man8//man8.hppa64/MAKEDEV.8   25 Apr 2016 23:47:04 -
@@ -214,9 +214,6 @@ PPP Multiplexer, see
 .It Ar *random
 In-kernel random data source, see
 .Xr random 4 .
-.It Ar systrace*
-System call tracing device, see
-.Xr systrace 4 .
 .It Ar tun*
 Network tunnel driver, see
 .Xr tun 4 .
Index: share/man/man8//man8.i386/MAKEDEV.8
===
RCS file: /var/cvs/src/share/man/man8/man8.i386/MAKEDEV.8,v
retrieving revision 1.103
diff -u -p -r1.103 MAKEDEV.8
--- share/man/man8//man8.i386/MAKEDEV.8 12 Mar 2016 18:02:18 -  1.103
+++ share/man/man8//man8.i386/MAKEDEV.8 25 Apr 2016 23:47:15 -
@@ -259,9 +259,6 @@ Raw MIDI devices, see
 .It Ar speaker
 PC speaker, see
 .Xr spkr 4 .
-.It Ar systrace*
-System call tracing device, see
-.Xr systrace 4 .
 .It Ar tun*
 Network tunnel driver, see
 .Xr tun 4 .
Index: share/man/man8//man8.landisk/MAKEDEV.8
===
RCS file: /var/cvs/src/share/man/man8/man8.landisk/MAKEDEV.8,v
retrieving revision 1.44
diff -u -p -r1.44 MAKEDEV.8
--- share/man/man8//man8.landisk/MAKEDEV.8  12 Mar 2016 18:02:18 -  
1.44
+++ share/man/man8//man8.landisk/MAKEDEV.8  25 Apr 2016 23:47:18 -
@@ -206,9 +206,6 @@ In-kernel random data source, see
 .It Ar rmidi*
 Raw MIDI devices, see
 .Xr midi 4 .
-.It Ar systrace*
-System call tracing device, see
-.Xr systrace 4 .
 .It Ar tun*
 Network tunnel driver, see
 .Xr tun 4 .
Index: share/man/man8//man8.loongson/MAKEDEV.8
===
RCS file: 

more systrace mop up 1/2: distrib/sets/lists

[Theo Buehler]

after the this patch and the next, the following files contain the last
appearances of systrace in /usr/src

regress/lib/libc/glob/files
regress/lib/libc/glob/globtest.in
regress/usr.bin/sdiff/file1
regress/usr.bin/sdiff/file2
regress/usr.bin/sdiff/tabs1
regress/usr.bin/sdiff/tabs2
usr.bin/ssh/sandbox-systrace.c
sys/sys/conf.h
sys/sys/file.h
sys/sys/sysctl.h

Index: distrib/sets/lists//base/md.alpha
===
RCS file: /var/cvs/src/distrib/sets/lists/base/md.alpha,v
retrieving revision 1.1096
diff -u -p -r1.1096 md.alpha
--- distrib/sets/lists//base/md.alpha   20 Apr 2016 14:48:51 -  1.1096
+++ distrib/sets/lists//base/md.alpha   25 Apr 2016 23:38:57 -
@@ -1190,7 +1190,6 @@
 ./usr/libdata/perl5/site_perl/alpha-openbsd/dev/sun/sunkbdvar.ph
 ./usr/libdata/perl5/site_perl/alpha-openbsd/dev/sun/sunmsvar.ph
 ./usr/libdata/perl5/site_perl/alpha-openbsd/dev/sun/uperfio.ph
-./usr/libdata/perl5/site_perl/alpha-openbsd/dev/systrace.ph
 ./usr/libdata/perl5/site_perl/alpha-openbsd/dev/tc
 ./usr/libdata/perl5/site_perl/alpha-openbsd/dev/tc/ascvar.ph
 ./usr/libdata/perl5/site_perl/alpha-openbsd/dev/tc/if_levar.ph
Index: distrib/sets/lists//base/md.amd64
===
RCS file: /var/cvs/src/distrib/sets/lists/base/md.amd64,v
retrieving revision 1.809
diff -u -p -r1.809 md.amd64
--- distrib/sets/lists//base/md.amd64   20 Apr 2016 14:48:51 -  1.809
+++ distrib/sets/lists//base/md.amd64   25 Apr 2016 23:39:00 -
@@ -1220,7 +1220,6 @@
 ./usr/libdata/perl5/site_perl/amd64-openbsd/dev/sun/sunkbdvar.ph
 ./usr/libdata/perl5/site_perl/amd64-openbsd/dev/sun/sunmsvar.ph
 ./usr/libdata/perl5/site_perl/amd64-openbsd/dev/sun/uperfio.ph
-./usr/libdata/perl5/site_perl/amd64-openbsd/dev/systrace.ph
 ./usr/libdata/perl5/site_perl/amd64-openbsd/dev/tc
 ./usr/libdata/perl5/site_perl/amd64-openbsd/dev/tc/ascvar.ph
 ./usr/libdata/perl5/site_perl/amd64-openbsd/dev/tc/if_levar.ph
Index: distrib/sets/lists//base/md.armish
===
RCS file: /var/cvs/src/distrib/sets/lists/base/md.armish,v
retrieving revision 1.597
diff -u -p -r1.597 md.armish
--- distrib/sets/lists//base/md.armish  20 Apr 2016 14:48:51 -  1.597
+++ distrib/sets/lists//base/md.armish  25 Apr 2016 23:39:07 -
@@ -1204,7 +1204,6 @@
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/sun/sunkbdvar.ph
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/sun/sunmsvar.ph
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/sun/uperfio.ph
-./usr/libdata/perl5/site_perl/arm-openbsd/dev/systrace.ph
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/tc
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/tc/ascvar.ph
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/tc/if_levar.ph
Index: distrib/sets/lists//base/md.armv7
===
RCS file: /var/cvs/src/distrib/sets/lists/base/md.armv7,v
retrieving revision 1.214
diff -u -p -r1.214 md.armv7
--- distrib/sets/lists//base/md.armv7   20 Apr 2016 14:48:51 -  1.214
+++ distrib/sets/lists//base/md.armv7   25 Apr 2016 23:39:10 -
@@ -1202,7 +1202,6 @@
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/sun/sunkbdvar.ph
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/sun/sunmsvar.ph
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/sun/uperfio.ph
-./usr/libdata/perl5/site_perl/arm-openbsd/dev/systrace.ph
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/tc
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/tc/ascvar.ph
 ./usr/libdata/perl5/site_perl/arm-openbsd/dev/tc/if_levar.ph
Index: distrib/sets/lists//base/md.hppa
===
RCS file: /var/cvs/src/distrib/sets/lists/base/md.hppa,v
retrieving revision 1.864
diff -u -p -r1.864 md.hppa
--- distrib/sets/lists//base/md.hppa20 Apr 2016 14:48:51 -  1.864
+++ distrib/sets/lists//base/md.hppa25 Apr 2016 23:39:14 -
@@ -1138,7 +1138,6 @@
 ./usr/libdata/perl5/site_perl/hppa-openbsd/dev/sun/sunkbdvar.ph
 ./usr/libdata/perl5/site_perl/hppa-openbsd/dev/sun/sunmsvar.ph
 ./usr/libdata/perl5/site_perl/hppa-openbsd/dev/sun/uperfio.ph
-./usr/libdata/perl5/site_perl/hppa-openbsd/dev/systrace.ph
 ./usr/libdata/perl5/site_perl/hppa-openbsd/dev/tc
 ./usr/libdata/perl5/site_perl/hppa-openbsd/dev/tc/ascvar.ph
 ./usr/libdata/perl5/site_perl/hppa-openbsd/dev/tc/if_levar.ph
Index: distrib/sets/lists//base/md.hppa64
===
RCS file: /var/cvs/src/distrib/sets/lists/base/md.hppa64,v
retrieving revision 1.692
diff -u -p -r1.692 md.hppa64
--- distrib/sets/lists//base/md.hppa64  20 Apr 2016 14:48:51 -  1.692
+++ distrib/sets/lists//base/md.hppa64  25 Apr 2016 23:39:17 -
@@ -1136,7 +1136,6 @@
 ./usr/libdata/perl5/site_perl/hppa64-openbsd/dev/sun/sunkbdvar.ph
 ./usr/libdata/perl5/site_perl/hppa64-openbsd/dev/sun/sunmsvar.ph
 

Re: match RTS522A in rtsx(4)

[Jonathan Gray]

On Mon, Apr 25, 2016 at 03:20:05PM -0700, Bryan Vyhmeister wrote:
> On Sun, Apr 24, 2016 at 05:01:41PM +1000, Jonathan Gray wrote:
> > Device id appears on the latest generation of thinkpads.
> > It looks like it should be compatible with the others?
> > 
> > This requires the pcidevs changes I just committed.
> > 
> > Index: sys/dev/pci/rtsx_pci.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/rtsx_pci.c,v
> > retrieving revision 1.12
> > diff -u -p -r1.12 rtsx_pci.c
> > --- sys/dev/pci/rtsx_pci.c  28 Apr 2015 07:55:13 -  1.12
> > +++ sys/dev/pci/rtsx_pci.c  24 Apr 2016 06:54:39 -
> > @@ -59,6 +59,7 @@ rtsx_pci_match(struct device *parent, vo
> > if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTS5209 ||
> > PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTS5227 ||
> > PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTS5229 ||
> > +   PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTS522A ||
> > PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTS5249 ||
> > PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTL8402 ||
> > PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTL8411 ||
> > Index: share/man/man4/rtsx.4
> > ===
> > RCS file: /cvs/src/share/man/man4/rtsx.4,v
> > retrieving revision 1.8
> > diff -u -p -r1.8 rtsx.4
> > --- share/man/man4/rtsx.4   27 Apr 2015 09:07:49 -  1.8
> > +++ share/man/man4/rtsx.4   24 Apr 2016 06:55:07 -
> > @@ -16,7 +16,7 @@
> >  The
> >  .Nm
> >  driver provides support for the Realtek RTS5209, RTS5227, RTS5229,
> > -RTS5249, RTL8402, RTL8411, and RTL8411B SD card readers.
> > +RTS522A, RTS5249, RTL8402, RTL8411, and RTL8411B SD card readers.
> >  .Pp
> >  The
> >  .Xr sdmmc 4
> 
> I didn't get a chance to test until today but this enables my rtsx(4) on
> the X260. Thank you! I did some brief testing with an old Transcend 8GB
> SDHC card and everything works great so far.

Thanks for testing.

> 
> Any chance to get the diff I posted a few days ago with the usbdev info
> the Sierra Wireless EM7455 committed? I'll post it inline below as well.
> Thank you.
> 
> Bryan

We normally only add usb devices to usbdevs if they are used in drivers
and sometimes if the device doesn't provide a string of it's own.

I'd be surprised if a string is not supplied by the device in this case.

> 
> 
> Index: sys/dev/usb/usbdevs
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> retrieving revision 1.663
> diff -u -p -r1.663 usbdevs
> --- sys/dev/usb/usbdevs   31 Mar 2016 12:27:48 -  1.663
> +++ sys/dev/usb/usbdevs   20 Apr 2016 15:20:31 -
> @@ -3832,6 +3832,7 @@ product SIERRA AC885U   0x6880  885U
>  product SIERRA C01SW 0x6890  C01SW
>  product SIERRA USB3050x68a3  USB305
>  product SIERRA MC83550x9013  MC8355
> +product SIERRA EM74550x9079  EM7455
>  
>  /* Sigmatel products */
>  product SIGMATEL IRDA0x4200  IrDA

Re: match RTS522A in rtsx(4)

[Bryan Vyhmeister]

On Sun, Apr 24, 2016 at 05:01:41PM +1000, Jonathan Gray wrote:
> Device id appears on the latest generation of thinkpads.
> It looks like it should be compatible with the others?
> 
> This requires the pcidevs changes I just committed.
> 
> Index: sys/dev/pci/rtsx_pci.c
> ===
> RCS file: /cvs/src/sys/dev/pci/rtsx_pci.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 rtsx_pci.c
> --- sys/dev/pci/rtsx_pci.c28 Apr 2015 07:55:13 -  1.12
> +++ sys/dev/pci/rtsx_pci.c24 Apr 2016 06:54:39 -
> @@ -59,6 +59,7 @@ rtsx_pci_match(struct device *parent, vo
>   if (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTS5209 ||
>   PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTS5227 ||
>   PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTS5229 ||
> + PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTS522A ||
>   PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTS5249 ||
>   PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTL8402 ||
>   PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_REALTEK_RTL8411 ||
> Index: share/man/man4/rtsx.4
> ===
> RCS file: /cvs/src/share/man/man4/rtsx.4,v
> retrieving revision 1.8
> diff -u -p -r1.8 rtsx.4
> --- share/man/man4/rtsx.4 27 Apr 2015 09:07:49 -  1.8
> +++ share/man/man4/rtsx.4 24 Apr 2016 06:55:07 -
> @@ -16,7 +16,7 @@
>  The
>  .Nm
>  driver provides support for the Realtek RTS5209, RTS5227, RTS5229,
> -RTS5249, RTL8402, RTL8411, and RTL8411B SD card readers.
> +RTS522A, RTS5249, RTL8402, RTL8411, and RTL8411B SD card readers.
>  .Pp
>  The
>  .Xr sdmmc 4

I didn't get a chance to test until today but this enables my rtsx(4) on
the X260. Thank you! I did some brief testing with an old Transcend 8GB
SDHC card and everything works great so far.

Any chance to get the diff I posted a few days ago with the usbdev info
the Sierra Wireless EM7455 committed? I'll post it inline below as well.
Thank you.

Bryan


Index: sys/dev/usb/usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.663
diff -u -p -r1.663 usbdevs
--- sys/dev/usb/usbdevs 31 Mar 2016 12:27:48 -  1.663
+++ sys/dev/usb/usbdevs 20 Apr 2016 15:20:31 -
@@ -3832,6 +3832,7 @@ product SIERRA AC885U 0x6880  885U
 product SIERRA C01SW   0x6890  C01SW
 product SIERRA USB305  0x68a3  USB305
 product SIERRA MC8355  0x9013  MC8355
+product SIERRA EM7455  0x9079  EM7455
 
 /* Sigmatel products */
 product SIGMATEL IRDA  0x4200  IrDA

Re: Clearing environ

[Jeremie Courreges-Anglas]

"Todd C. Miller"  writes:

> On Mon, 25 Apr 2016 16:56:47 -0400, "Ted Unangst" wrote:
>
>> compilers will, however, "miscompile" such code. we should avoid it.
>
> Fair enough.

ok jca@

>  - todd
>
> Index: lib/libc/stdlib/setenv.c
> ===
> RCS file: /cvs/src/lib/libc/stdlib/setenv.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 setenv.c
> --- lib/libc/stdlib/setenv.c  13 Mar 2016 18:34:21 -  1.17
> +++ lib/libc/stdlib/setenv.c  25 Apr 2016 21:09:20 -
> @@ -43,7 +43,7 @@ int
>  putenv(char *str)
>  {
>   char **P, *cp;
> - size_t cnt;
> + size_t cnt = 0;
>   int offset = 0;
>  
>   for (cp = str; *cp && *cp != '='; ++cp)
> @@ -65,13 +65,15 @@ putenv(char *str)
>   }
>  
>   /* create new slot for string */
> - for (P = environ; *P != NULL; P++)
> - ;
> - cnt = P - environ;
> + if (environ != NULL) {
> + for (P = environ; *P != NULL; P++)
> + ;
> + cnt = P - environ;
> + }
>   P = reallocarray(lastenv, cnt + 2, sizeof(char *));
>   if (!P)
>   return (-1);
> - if (lastenv != environ)
> + if (lastenv != environ && environ != NULL)
>   memcpy(P, environ, cnt * sizeof(char *));
>   lastenv = environ = P;
>   environ[cnt] = str;
> @@ -122,15 +124,17 @@ setenv(const char *name, const char *val
>   break;
>   }
>   } else {/* create new slot */
> - size_t cnt;
> + size_t cnt = 0;
>  
> - for (P = environ; *P != NULL; P++)
> - ;
> - cnt = P - environ;
> + if (environ != NULL) {
> + for (P = environ; *P != NULL; P++)
> + ;
> + cnt = P - environ;
> + }
>   P = reallocarray(lastenv, cnt + 2, sizeof(char *));
>   if (!P)
>   return (-1);
> - if (lastenv != environ)
> + if (lastenv != environ && environ != NULL)
>   memcpy(P, environ, cnt * sizeof(char *));
>   lastenv = environ = P;
>   offset = cnt;
>

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

Re: Clearing environ

[Todd C. Miller]

On Mon, 25 Apr 2016 16:56:47 -0400, "Ted Unangst" wrote:

> compilers will, however, "miscompile" such code. we should avoid it.

Fair enough.

 - todd

Index: lib/libc/stdlib/setenv.c
===
RCS file: /cvs/src/lib/libc/stdlib/setenv.c,v
retrieving revision 1.17
diff -u -p -r1.17 setenv.c
--- lib/libc/stdlib/setenv.c13 Mar 2016 18:34:21 -  1.17
+++ lib/libc/stdlib/setenv.c25 Apr 2016 21:09:20 -
@@ -43,7 +43,7 @@ int
 putenv(char *str)
 {
char **P, *cp;
-   size_t cnt;
+   size_t cnt = 0;
int offset = 0;
 
for (cp = str; *cp && *cp != '='; ++cp)
@@ -65,13 +65,15 @@ putenv(char *str)
}
 
/* create new slot for string */
-   for (P = environ; *P != NULL; P++)
-   ;
-   cnt = P - environ;
+   if (environ != NULL) {
+   for (P = environ; *P != NULL; P++)
+   ;
+   cnt = P - environ;
+   }
P = reallocarray(lastenv, cnt + 2, sizeof(char *));
if (!P)
return (-1);
-   if (lastenv != environ)
+   if (lastenv != environ && environ != NULL)
memcpy(P, environ, cnt * sizeof(char *));
lastenv = environ = P;
environ[cnt] = str;
@@ -122,15 +124,17 @@ setenv(const char *name, const char *val
break;
}
} else {/* create new slot */
-   size_t cnt;
+   size_t cnt = 0;
 
-   for (P = environ; *P != NULL; P++)
-   ;
-   cnt = P - environ;
+   if (environ != NULL) {
+   for (P = environ; *P != NULL; P++)
+   ;
+   cnt = P - environ;
+   }
P = reallocarray(lastenv, cnt + 2, sizeof(char *));
if (!P)
return (-1);
-   if (lastenv != environ)
+   if (lastenv != environ && environ != NULL)
memcpy(P, environ, cnt * sizeof(char *));
lastenv = environ = P;
offset = cnt;

Re: Clearing environ

[Ted Unangst]

Todd C. Miller wrote:
> On Mon, 25 Apr 2016 22:39:48 +0200, Jeremie Courreges-Anglas wrote:
> 
> > Agreed, I also had this in mind.  But then, should the memset call with
> > a zero size be avoided?
> > 
> > if (lastenv != environ)
> > memcpy(P, environ, cnt * sizeof(char *));
> 
> I thought about that too.  Strictly speaking it is undefined behavior
> to pass in a NULL pointer, regardless of size.  However, it is safe
> on OpenBSD and we have plenty of other code that assumes this.

compilers will, however, "miscompile" such code. we should avoid it.

Re: Clearing environ

[Jeremie Courreges-Anglas]

"Todd C. Miller"  writes:

> It is already safe to call getenv() and unsetenv() when environ is
> NULL so I think it makes sense for setenv() and putenv() to also
> support this.  However, I'd prefer that we just wrap the loop that
> counts the length of environ in an if (environ != NULL) check rather
> than needlessly check for NULL in each iteration of the loop.  This
> also makes it more obvious that the check is for a NULL environ.

Agreed, I also had this in mind.  But then, should the memset call with
a zero size be avoided?

if (lastenv != environ)
memcpy(P, environ, cnt * sizeof(char *));

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

Re: Clearing environ

[Todd C. Miller]

On Mon, 25 Apr 2016 22:39:48 +0200, Jeremie Courreges-Anglas wrote:

> Agreed, I also had this in mind.  But then, should the memset call with
> a zero size be avoided?
> 
>   if (lastenv != environ)
>   memcpy(P, environ, cnt * sizeof(char *));

I thought about that too.  Strictly speaking it is undefined behavior
to pass in a NULL pointer, regardless of size.  However, it is safe
on OpenBSD and we have plenty of other code that assumes this.

 - todd

Re: Clearing environ

[Todd C. Miller]

It is already safe to call getenv() and unsetenv() when environ is
NULL so I think it makes sense for setenv() and putenv() to also
support this.  However, I'd prefer that we just wrap the loop that
counts the length of environ in an if (environ != NULL) check rather
than needlessly check for NULL in each iteration of the loop.  This
also makes it more obvious that the check is for a NULL environ.

 - todd

--- /usr/src/lib/libc/stdlib/setenv.c   Mon Apr 25 14:15:14 2016
+++ setenv.cMon Apr 25 14:27:32 2016
@@ -43,7 +76,7 @@
 putenv(char *str)
 {
char **P, *cp;
-   size_t cnt;
+   size_t cnt = 0;
int offset = 0;
 
for (cp = str; *cp && *cp != '='; ++cp)
@@ -65,9 +98,11 @@
}
 
/* create new slot for string */
-   for (P = environ; *P != NULL; P++)
-   ;
-   cnt = P - environ;
+   if (environ != NULL) {
+   for (P = environ; *P != NULL; P++)
+   ;
+   cnt = P - environ;
+   }
P = reallocarray(lastenv, cnt + 2, sizeof(char *));
if (!P)
return (-1);
@@ -122,11 +156,13 @@
break;
}
} else {/* create new slot */
-   size_t cnt;
+   size_t cnt = 0;
 
-   for (P = environ; *P != NULL; P++)
-   ;
-   cnt = P - environ;
+   if (environ != NULL) {
+   for (P = environ; *P != NULL; P++)
+   ;
+   cnt = P - environ;
+   }
P = reallocarray(lastenv, cnt + 2, sizeof(char *));
if (!P)
return (-1);

Re: Clearing environ

[Ted Unangst]

Jeremie Courreges-Anglas wrote:
> 
> A few ports make use of clearenv(3), a GNU extension.  This function was
> rejected by POSIX, so what's left?

> I think ports should probably use calloc here if we want to push those
> patches upstream.  But supporting the "environ = NULL" method looks
> cheap.

sigh. more sigh. but ok. looks mostly harmless.

Clearing environ

[Jeremie Courreges-Anglas]

A few ports make use of clearenv(3), a GNU extension.  This function was
rejected by POSIX, so what's left?

If all you want to do is clear the environment before exec(), you can
construct a new environment and use execve, which is specified by POSIX.

Now if a port is not using execve, but clearenv then a bunch of setenv,
what can we do?  It turns out that replacing clearenv with "environ =
NULL" doesn't work, because setenv will later dereference environ.  *BZZT*
"environ = calloc(1, sizeof(*environ))" seems portable, as noted by the
dovecot project

  http://hg.dovecot.org/dovecot-2.0/file/74d9f61e224d/src/lib/env-util.c#l56

I think ports should probably use calloc here if we want to push those
patches upstream.  But supporting the "environ = NULL" method looks
cheap.

Thoughts?

Index: setenv.c
===
RCS file: /cvs/src/lib/libc/stdlib/setenv.c,v
retrieving revision 1.17
diff -u -p -p -u -r1.17 setenv.c
--- setenv.c13 Mar 2016 18:34:21 -  1.17
+++ setenv.c25 Apr 2016 19:29:58 -
@@ -43,7 +43,7 @@ int
 putenv(char *str)
 {
char **P, *cp;
-   size_t cnt;
+   size_t cnt = 0;
int offset = 0;
 
for (cp = str; *cp && *cp != '='; ++cp)
@@ -65,9 +65,8 @@ putenv(char *str)
}
 
/* create new slot for string */
-   for (P = environ; *P != NULL; P++)
-   ;
-   cnt = P - environ;
+   for (P = environ; P != NULL && *P != NULL; P++)
+   cnt++;
P = reallocarray(lastenv, cnt + 2, sizeof(char *));
if (!P)
return (-1);
@@ -122,11 +121,10 @@ setenv(const char *name, const char *val
break;
}
} else {/* create new slot */
-   size_t cnt;
+   size_t cnt = 0;
 
-   for (P = environ; *P != NULL; P++)
-   ;
-   cnt = P - environ;
+   for (P = environ; P != NULL && *P != NULL; P++)
+   cnt++;
P = reallocarray(lastenv, cnt + 2, sizeof(char *));
if (!P)
return (-1);


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

Re: anti-ROP mechanism in libc

[Joerg Jung]

On Mon, Apr 25, 2016 at 03:23:47PM +, Robert Peichaer wrote:
> On Mon, Apr 25, 2016 at 10:57:37AM -0400, Ted Unangst wrote:
> > Theo de Raadt wrote:
> > > + cp -p /usr/lib/$_lib /usr/lib/$_tmplib
> > > + install -o root -g bin -m 0444 $_lib /usr/lib/$_lib &&
> > > + rm -f /usr/lib/$_tmplib ||
> > > + mv /usr/lib/$_tmplib /usr/lib/$_lib
> > 
> > I'm a little confused by what's going on here. If the install fails, do we
> > still want to overwrite the lib?
>  
> 
> If the install fails, the original library file is restored.
> 
> The "install .. && rm .. || mv ..." is identical to if-then-else and could
> be written like this too.

Nitpicking: nope, It is not identical, see:
https://github.com/koalaman/shellcheck/wiki/SC2015

Though, may not matter here.

>   if install -o root -g bin -m 0444 $_lib /usr/lib/$_lib; then
>   rm -f /usr/lib/$_tmplib
>   else
>   mv /usr/lib/$_tmplib /usr/lib/$_lib
>   fi
> 

Re: numerous statfs bugs

[Martin Natano]

On Sun, Apr 24, 2016 at 01:48:04PM +0200, Stefan Kempf wrote:
> 
> Diff reads good to me. Any reason why you changed setting f_mntfromname
> from "fusefs" to "fuse"?

No, it's a typo; updated diff below. Thanks!

natano



Index: isofs/cd9660/cd9660_vfsops.c
===
RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vfsops.c,v
retrieving revision 1.77
diff -u -p -r1.77 cd9660_vfsops.c
--- isofs/cd9660/cd9660_vfsops.c27 Mar 2016 11:39:37 -  1.77
+++ isofs/cd9660/cd9660_vfsops.c19 Apr 2016 18:52:52 -
@@ -383,6 +383,7 @@ iso_mountfs(devvp, mp, p, argp)
mp->mnt_data = (qaddr_t)isomp;
mp->mnt_stat.f_fsid.val[0] = (long)dev;
mp->mnt_stat.f_fsid.val[1] = mp->mnt_vfc->vfc_typenum;
+   mp->mnt_stat.f_namemax = NAME_MAX;
mp->mnt_flag |= MNT_LOCAL;
isomp->im_mountp = mp;
isomp->im_dev = dev;
@@ -650,13 +651,9 @@ cd9660_statfs(mp, sbp, p)
sbp->f_bavail = 0; /* blocks free for non superuser */
sbp->f_files =  0; /* total files */
sbp->f_ffree = 0; /* free file nodes */
-   if (sbp != >mnt_stat) {
-   bcopy(mp->mnt_stat.f_mntonname, sbp->f_mntonname, MNAMELEN);
-   bcopy(mp->mnt_stat.f_mntfromname, sbp->f_mntfromname,
-   MNAMELEN);
-   bcopy(>mnt_stat.mount_info.iso_args,
-   >mount_info.iso_args, sizeof(struct iso_args));
-   }
+   sbp->f_favail = 0; /* file nodes free for non superuser */
+   copy_statfs_info(sbp, mp);
+
return (0);
 }
 
Index: isofs/udf/udf_vfsops.c
===
RCS file: /cvs/src/sys/isofs/udf/udf_vfsops.c,v
retrieving revision 1.49
diff -u -p -r1.49 udf_vfsops.c
--- isofs/udf/udf_vfsops.c  27 Mar 2016 11:39:37 -  1.49
+++ isofs/udf/udf_vfsops.c  19 Apr 2016 18:52:52 -
@@ -264,6 +264,7 @@ udf_mountfs(struct vnode *devvp, struct 
mp->mnt_data = (qaddr_t) ump;
mp->mnt_stat.f_fsid.val[0] = devvp->v_rdev;
mp->mnt_stat.f_fsid.val[1] = mp->mnt_vfc->vfc_typenum;
+   mp->mnt_stat.f_namemax = NAME_MAX;
mp->mnt_flag |= MNT_LOCAL;
 
ump->um_mountp = mp;
@@ -542,6 +543,8 @@ udf_statfs(struct mount *mp, struct stat
sbp->f_bavail = 0;
sbp->f_files = 0;
sbp->f_ffree = 0;
+   sbp->f_favail = 0;
+   copy_statfs_info(sbp, mp);
 
return (0);
 }
Index: kern/vfs_subr.c
===
RCS file: /cvs/src/sys/kern/vfs_subr.c,v
retrieving revision 1.244
diff -u -p -r1.244 vfs_subr.c
--- kern/vfs_subr.c 7 Apr 2016 09:58:11 -   1.244
+++ kern/vfs_subr.c 19 Apr 2016 18:52:52 -
@@ -2276,6 +2276,6 @@ copy_statfs_info(struct statfs *sbp, con
memcpy(sbp->f_mntonname, mp->mnt_stat.f_mntonname, MNAMELEN);
memcpy(sbp->f_mntfromname, mp->mnt_stat.f_mntfromname, MNAMELEN);
memcpy(sbp->f_mntfromspec, mp->mnt_stat.f_mntfromspec, MNAMELEN);
-   memcpy(>mount_info.ufs_args, >mnt_stat.mount_info.ufs_args,
-   sizeof(struct ufs_args));
+   memcpy(>mount_info, >mnt_stat.mount_info,
+   sizeof(union mount_info));
 }
Index: miscfs/fuse/fuse_vfsops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vfsops.c,v
retrieving revision 1.20
diff -u -p -r1.20 fuse_vfsops.c
--- miscfs/fuse/fuse_vfsops.c   27 Mar 2016 11:39:37 -  1.20
+++ miscfs/fuse/fuse_vfsops.c   19 Apr 2016 18:52:52 -
@@ -111,7 +111,9 @@ fusefs_mount(struct mount *mp, const cha
bzero(mp->mnt_stat.f_mntonname, MNAMELEN);
strlcpy(mp->mnt_stat.f_mntonname, path, MNAMELEN);
bzero(mp->mnt_stat.f_mntfromname, MNAMELEN);
-   bcopy("fusefs", mp->mnt_stat.f_mntfromname, sizeof("fusefs"));
+   strlcpy(mp->mnt_stat.f_mntfromname, "fusefs", MNAMELEN);
+   bzero(mp->mnt_stat.f_mntfromspec, MNAMELEN);
+   strlcpy(mp->mnt_stat.f_mntfromspec, "fusefs", MNAMELEN);
 
fuse_device_set_fmp(fmp, 1);
fbuf = fb_setup(0, 0, FBT_INIT, p);
@@ -204,6 +206,8 @@ fusefs_statfs(struct mount *mp, struct s
 
fmp = VFSTOFUSEFS(mp);
 
+   copy_statfs_info(sbp, mp);
+
if (fmp->sess_init) {
fbuf = fb_setup(0, FUSE_ROOT_ID, FBT_STATFS, p);
 
@@ -219,7 +223,9 @@ fusefs_statfs(struct mount *mp, struct s
sbp->f_blocks = fbuf->fb_stat.f_blocks;
sbp->f_files = fbuf->fb_stat.f_files;
sbp->f_ffree = fbuf->fb_stat.f_ffree;
+   sbp->f_favail = fbuf->fb_stat.f_favail;
sbp->f_bsize = fbuf->fb_stat.f_frsize;
+   sbp->f_iosize = fbuf->fb_stat.f_bsize;
sbp->f_namemax = fbuf->fb_stat.f_namemax;
fb_delete(fbuf);
} else {
@@ -227,8 +233,10 @@ fusefs_statfs(struct mount *mp, struct s
sbp->f_bfree = 0;

Re: dc patch

[Chris Cappuccio]

Edgar Pettijohn [ed...@pettijohn-web.com] wrote:
> nevermind just found the elusive "q"
> 

There's always the universal ^D

Re: anti-ROP mechanism in libc

[Miod Vallat]

>> Wait!  Does that mean there is a moment where there is not a valid
>> libc.so installed?  That would be wrong wouldn't it?
>> 
>> Doesn't the install command guarantee atomicity?
>
> Well, this is the same procedure we use during every make build,
> and it works.

It had been fixed to use install -S some years ago because it was not
atomic enough otherwise.

Re: anti-ROP mechanism in libc

[Todd C. Miller]

On Mon, 25 Apr 2016 10:18:56 -0600, "Todd C. Miller" wrote:

> On Mon, 25 Apr 2016 18:04:58 +0200, Mark Kettenis wrote:
> 
> > Wait!  Does that mean there is a moment where there is not a valid
> > libc.so installed?  That would be wrong wouldn't it?
> > 
> > Doesn't the install command guarantee atomicity?
> 
> Seems like it would be safer to just install as /usr/lib/$_tmplib
> and then rename to /usr/lib/$_lib.  Trying to recover from a failed
> copy to /usr/lib/$_lib is potentially nasty (yes, I am aware that
> mv is statically linked).

A better approach is to use install(1)'s -S option (safe mode) which
will copy the file to a temp for you and rename it to the target
name if the copy is successful.  That way install(1) does the
cleanup.  This is how we install libraries via bsd.lib.mk too.

So instead of:

cp -p /usr/lib/$_lib /usr/lib/$_tmplib
install -o root -g bin -m 0444 $_lib /usr/lib/$_lib &&
rm -f /usr/lib/$_tmplib ||
mv /usr/lib/$_tmplib /usr/lib/$_lib

All you need is:

install -c -S -o root -g bin -m 0444 $_lib /usr/lib/$_lib

 - todd

Re: anti-ROP mechanism in libc

[Theo de Raadt]

> On Mon, Apr 25, 2016 at 8:23 AM, Robert Peichaer  wrote:
> > If the install fails, the original library file is restored.
> >
> > The "install .. && rm .. || mv ..." is identical to if-then-else and could
> > be written like this too.
> >
> > if install -o root -g bin -m 0444 $_lib /usr/lib/$_lib; then
> > rm -f /usr/lib/$_tmplib
> > else
> > mv /usr/lib/$_tmplib /usr/lib/$_lib
> > fi
> 
> Shouldn't this use install -S like a normal src build does?  Then the
> need for cleanup on failure goes away, no?

Yes, perhaps.

Re: anti-ROP mechanism in libc

[Ted Unangst]

Robert Peichaer wrote:
> On Mon, Apr 25, 2016 at 10:57:37AM -0400, Ted Unangst wrote:
> > Theo de Raadt wrote:
> > > + cp -p /usr/lib/$_lib /usr/lib/$_tmplib
> > > + install -o root -g bin -m 0444 $_lib /usr/lib/$_lib &&
> > > + rm -f /usr/lib/$_tmplib ||
> > > + mv /usr/lib/$_tmplib /usr/lib/$_lib
> > 
> > I'm a little confused by what's going on here. If the install fails, do we
> > still want to overwrite the lib?
>  
> 
> If the install fails, the original library file is restored.

ok, i didn't realize what tmplib was.

Re: anti-ROP mechanism in libc

[Philip Guenther]

On Mon, Apr 25, 2016 at 8:23 AM, Robert Peichaer  wrote:
> If the install fails, the original library file is restored.
>
> The "install .. && rm .. || mv ..." is identical to if-then-else and could
> be written like this too.
>
> if install -o root -g bin -m 0444 $_lib /usr/lib/$_lib; then
> rm -f /usr/lib/$_tmplib
> else
> mv /usr/lib/$_tmplib /usr/lib/$_lib
> fi

Shouldn't this use install -S like a normal src build does?  Then the
need for cleanup on failure goes away, no?

Re: anti-ROP mechanism in libc

[Theo de Raadt]

> Wait!  Does that mean there is a moment where there is not a valid
> libc.so installed?  That would be wrong wouldn't it?
> 
> Doesn't the install command guarantee atomicity?

Well, this is the same procedure we use during every make build,
and it works.

Re: anti-ROP mechanism in libc

[Todd C. Miller]

On Mon, 25 Apr 2016 18:04:58 +0200, Mark Kettenis wrote:

> Wait!  Does that mean there is a moment where there is not a valid
> libc.so installed?  That would be wrong wouldn't it?
> 
> Doesn't the install command guarantee atomicity?

Seems like it would be safer to just install as /usr/lib/$_tmplib
and then rename to /usr/lib/$_lib.  Trying to recover from a failed
copy to /usr/lib/$_lib is potentially nasty (yes, I am aware that
mv is statically linked).

 - todd

Re: anti-ROP mechanism in libc

[Robert Peichaer]

On Mon, Apr 25, 2016 at 10:57:37AM -0400, Ted Unangst wrote:
> Theo de Raadt wrote:
> > +   cp -p /usr/lib/$_lib /usr/lib/$_tmplib
> > +   install -o root -g bin -m 0444 $_lib /usr/lib/$_lib &&
> > +   rm -f /usr/lib/$_tmplib ||
> > +   mv /usr/lib/$_tmplib /usr/lib/$_lib
> 
> I'm a little confused by what's going on here. If the install fails, do we
> still want to overwrite the lib?
 

If the install fails, the original library file is restored.

The "install .. && rm .. || mv ..." is identical to if-then-else and could
be written like this too.

if install -o root -g bin -m 0444 $_lib /usr/lib/$_lib; then
rm -f /usr/lib/$_tmplib
else
mv /usr/lib/$_tmplib /usr/lib/$_lib
fi

Re: anti-ROP mechanism in libc

[Ted Unangst]

Theo de Raadt wrote:
> + cp -p /usr/lib/$_lib /usr/lib/$_tmplib
> + install -o root -g bin -m 0444 $_lib /usr/lib/$_lib &&
> + rm -f /usr/lib/$_tmplib ||
> + mv /usr/lib/$_tmplib /usr/lib/$_lib

I'm a little confused by what's going on here. If the install fails, do we
still want to overwrite the lib?

acpi: add HPET and EC to skip_hids

[Paul Irofti]

We attach acpihpet(4) and acpiec(4) without using the HID, so do not
report it in the dmesg. Okay?


Index: acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.309
diff -u -p -u -p -r1.309 acpi.c
--- acpi.c  2 Apr 2016 13:54:29 -   1.309
+++ acpi.c  25 Apr 2016 14:52:20 -
@@ -2758,6 +2758,7 @@ const char *acpi_skip_hids[] = {
"INT0800",  /* Intel 82802Firmware Hub Device */
"PNP",  /* 8259-compatible Programmable Interrupt Controller */
"PNP0100",  /* PC-class System Timer */
+   "PNP0103",  /* HPET System Timer */
"PNP0200",  /* PC-class DMA Controller */
"PNP0800",  /* Microsoft Sound System Compatible Device */
"PNP0A03",  /* PCI Bus */
@@ -2766,6 +2767,7 @@ const char *acpi_skip_hids[] = {
"PNP0C01",  /* System Board */
"PNP0C02",  /* PNP Motherboard Resources */
"PNP0C04",  /* x87-compatible Floating Point Processing Unit */
+   "PNP0C09",  /* Embedded Controller Device */
"PNP0C0F",  /* PCI Interrupt Link Device */
NULL
 };

Re: MP-safe TX for cnmac(4)

[Martin Pieuchot]

On 25/04/16(Mon) 11:35, Visa Hankala wrote:
> On Mon, Apr 25, 2016 at 10:20:32AM +0200, Martin Pieuchot wrote:
> > On 24/04/16(Sun) 16:13, Visa Hankala wrote:
> > > This adds MP-safe TX for cnmac(4). OK?
> > 
> > Would it be possible to do that without mutex?  Having the same pattern
> > across most of our drivers would reduce the maintenance effort.
> 
> The only real use of the mutex is to keep octeon_eth_tick_free() away
> from sc_sendq while the start routine is running. The queue tracks mbufs
> that need to be freed after transmission. Unlike many other drivers,
> there is no TX ring. As long as there is at least a little bit of
> traffic, octeon_eth_start() can take care of draining the queue. The
> hardware does not have a transmission complete interrupt, so the timer
> is there to free transmitted mbufs in case traffic stops altogether for
> a long time.
> 
> What driver would be a good example for this case?

I don't think we have any example for this case.  But it looks like the
watchdog problem for which we schedule a task and call intr_barrier()
after removing the IFF_RUNNING flag.  Now if you think it doesn't make
sense I'm fine with your approach.

> > > Index: arch/octeon/dev/if_cnmac.c
> > > ===
> > > RCS file: src/sys/arch/octeon/dev/if_cnmac.c,v
> > > retrieving revision 1.38
> > > diff -u -p -r1.38 if_cnmac.c
> > > --- arch/octeon/dev/if_cnmac.c13 Apr 2016 11:34:00 -  1.38
> > > +++ arch/octeon/dev/if_cnmac.c24 Apr 2016 15:35:04 -
> > > @@ -285,6 +285,7 @@ octeon_eth_attach(struct device *parent,
> > >   octeon_eth_gsc[sc->sc_port] = sc;
> > >  
> > >   ml_init(>sc_sendq);
> > > + mtx_init(>sc_sendq_mtx, IPL_NET);
> > >   sc->sc_soft_req_thresh = 15/* XXX */;
> > >   sc->sc_ext_callback_cnt = 0;
> > >  
> > > @@ -317,6 +318,7 @@ octeon_eth_attach(struct device *parent,
> > >   strncpy(ifp->if_xname, sc->sc_dev.dv_xname, sizeof(ifp->if_xname));
> > >   ifp->if_softc = sc;
> > >   ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
> > > + ifp->if_xflags = IFXF_MPSAFE;
> > >   ifp->if_ioctl = octeon_eth_ioctl;
> > >   ifp->if_start = octeon_eth_start;
> > >   ifp->if_watchdog = octeon_eth_watchdog;
> > > @@ -742,7 +744,7 @@ octeon_eth_ioctl(struct ifnet *ifp, u_lo
> > >   error = 0;
> > >   }
> > >  
> > > - octeon_eth_start(ifp);
> > > + if_start(ifp);
> > >  
> > >   splx(s);
> > >   return (error);
> > > @@ -959,18 +961,19 @@ octeon_eth_start(struct ifnet *ifp)
> > >   struct octeon_eth_softc *sc = ifp->if_softc;
> > >   struct mbuf *m;
> > >  
> > > + if (__predict_false(!cn30xxgmx_link_status(sc->sc_gmx_port))) {
> > > + IFQ_PURGE(>if_snd);
> > > + return;
> > > + }
> > > +
> > > + mtx_enter(>sc_sendq_mtx);
> > > +
> > >   /*
> > >* performance tuning
> > >* presend iobdma request 
> > >*/
> > >   octeon_eth_send_queue_flush_prefetch(sc);
> > >  
> > > - if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(>if_snd))
> > > - goto last;
> > > -
> > > - if (__predict_false(!cn30xxgmx_link_status(sc->sc_gmx_port)))
> > > - goto last;
> > > -
> > >   for (;;) {
> > >   octeon_eth_send_queue_flush_fetch(sc); /* XXX */
> > >  
> > > @@ -980,13 +983,16 @@ octeon_eth_start(struct ifnet *ifp)
> > >* and bail out.
> > >*/
> > >   if (octeon_eth_send_queue_is_full(sc)) {
> > > + mtx_leave(>sc_sendq_mtx);
> > >   return;
> > >   }
> > >   /* XXX */
> > >  
> > >   IFQ_DEQUEUE(>if_snd, m);
> > > - if (m == NULL)
> > > + if (m == NULL) {
> > > + mtx_leave(>sc_sendq_mtx);
> > >   return;
> > > + }
> > >  
> > >   OCTEON_ETH_TAP(ifp, m, BPF_DIRECTION_OUT);
> > >  
> > > @@ -1008,8 +1014,9 @@ octeon_eth_start(struct ifnet *ifp)
> > >   octeon_eth_send_queue_flush_prefetch(sc);
> > >   }
> > >  
> > > -last:
> > >   octeon_eth_send_queue_flush_fetch(sc);
> > > +
> > > + mtx_leave(>sc_sendq_mtx);
> > >  }
> > >  
> > >  void
> > > @@ -1025,7 +1032,7 @@ octeon_eth_watchdog(struct ifnet *ifp)
> > >   ifq_clr_oactive(>if_snd);
> > >   ifp->if_timer = 0;
> > >  
> > > - octeon_eth_start(ifp);
> > > + if_start(ifp);
> > >  }
> > >  
> > >  int
> > > @@ -1066,6 +1073,8 @@ octeon_eth_stop(struct ifnet *ifp, int d
> > >  {
> > >   struct octeon_eth_softc *sc = ifp->if_softc;
> > >  
> > > + CLR(ifp->if_flags, IFF_RUNNING);
> > > +
> > >   timeout_del(>sc_tick_misc_ch);
> > >   timeout_del(>sc_tick_free_ch);
> > >   timeout_del(>sc_resume_ch);
> > > @@ -1074,13 +1083,12 @@ octeon_eth_stop(struct ifnet *ifp, int d
> > >  
> > >   cn30xxgmx_port_enable(sc->sc_gmx_port, 0);
> > >  
> > > - /* Mark the interface as down and cancel the watchdog timer. */
> > > - CLR(ifp->if_flags, IFF_RUNNING);
> > > + intr_barrier(octeon_eth_pow_recv_ih);
> > > + ifq_barrier(>if_snd);
> > > +
> > >   ifq_clr_oactive(>if_snd);

anti-ROP mechanism in libc

[Theo de Raadt]

This change randomizes the order of symbols in libc.so at boot time.

This is done by saving all the independent .so sub-files into an ar
archive, and then relinking them into a new libc.so in random order,
at each boot.  The cost is less than a second on the systems I am
using.

For now, this is only done for libc, because it is generally the most
gadget heavy library; spilled registers are more likely to point
within the libc segment; and also the gadgets are close to system call
stubs.  As a result of the change, gadgets are no longer found at
fixed offsets from spilled registers.

(I have run this on my systems for all base/X libraries, which exposed
no strange behaviour roughly 3 seconds of rebuild time at boot)

I have included the sets changes, to show that a few compile tools
must move into base.  This should allow comp-less installs to continue
working.

My horrible shell scripts were improved by rpe, who also did other
testing.

Index: share/mk/bsd.lib.mk
===
RCS file: /cvs/src/share/mk/bsd.lib.mk,v
retrieving revision 1.74
diff -u -p -u -r1.74 bsd.lib.mk
--- share/mk/bsd.lib.mk 26 Oct 2015 10:43:42 -  1.74
+++ share/mk/bsd.lib.mk 25 Apr 2016 08:58:26 -
@@ -174,6 +174,15 @@ FULLSHLIBNAME=lib${LIB}.so.${SHLIB_MAJOR
 _LIBS+=${FULLSHLIBNAME}
 .endif
 
+.if defined(LIBREBUILD)
+_LIBS+=${FULLSHLIBNAME}.a
+
+.if exists(${.CURDIR}/Symbols.list)
+SYMBOLSMAP=Symbols.map
+.endif
+
+.endif
+
 .if defined(VERSION_SCRIPT)
 ${FULLSHLIBNAME}:  ${VERSION_SCRIPT}
 LDADD+=-Wl,--version-script=${VERSION_SCRIPT}
@@ -209,7 +218,13 @@ ${FULLSHLIBNAME}: ${SOBJS} ${DPADD}
@echo building shared ${LIB} library \(version 
${SHLIB_MAJOR}.${SHLIB_MINOR}\)
@rm -f ${.TARGET}
${CC} -shared ${PICFLAG} -o ${.TARGET} \
-   `${LORDER} ${SOBJS}|tsort -q` ${LDADD}
+   `echo ${SOBJS} | tr ' ' '\n' | sort -R` ${LDADD}
+
+${FULLSHLIBNAME}.a: ${SOBJS}
+   @echo building shared ${LIB} library \(version 
${SHLIB_MAJOR}.${SHLIB_MINOR}\) ar
+   @rm -f ${.TARGET}
+   @echo ${PICFLAG} ${LDADD} > .ldadd
+   ar cq ${FULLSHLIBNAME}.a ${SOBJS} .ldadd ${SYMBOLSMAP}
 
 # all .do files...
 DOBJS+=${OBJS:.o=.do}
@@ -290,6 +305,10 @@ realinstall:
 .if !defined(NOPIC) && defined(SHLIB_MAJOR) && defined(SHLIB_MINOR)
${INSTALL} ${INSTALL_COPY} -S -o ${LIBOWN} -g ${LIBGRP} -m ${LIBMODE} \
${FULLSHLIBNAME} ${DESTDIR}${LIBDIR}
+.if defined(LIBREBUILD)
+   ${INSTALL} ${INSTALL_COPY} -S -o ${LIBOWN} -g ${LIBGRP} -m ${LIBMODE} \
+   ${FULLSHLIBNAME}.a ${DESTDIR}${LIBDIR}
+.endif
 .endif
 .if defined(LINKS) && !empty(LINKS)
 .  for lnk file in ${LINKS}
Index: etc/rc
===
RCS file: /cvs/src/etc/rc,v
retrieving revision 1.474
diff -u -p -u -r1.474 rc
--- etc/rc  29 Dec 2015 19:41:24 -  1.474
+++ etc/rc  28 Mar 2016 22:58:45 -
@@ -158,6 +158,37 @@ make_keys() {
ssh-keygen -A
 }
 
+rebuildlibs() {
+   local _l _liba _libas _tmpdir
+
+   for _liba in /usr/lib/libc.so.*.a; do
+   _liba=$(ls ${_liba%%.[0-9]*}*.a | sort -n | tail -1)
+   for _l in $_libas; do
+   [[ $_l == $_liba ]] && continue 2
+   done
+   _libas="$_libas $_liba"
+   done
+
+   for _liba in $_libas; do
+   _tmpdir=$(mktemp -dq /tmp/_librebuild.) || return
+   (
+   set -o errexit
+   _lib=${_liba#/usr/lib/}
+   _lib=${_lib%.a}
+   _tmplib=$(mktemp $_lib.)
+   cd $_tmpdir
+   ar x ${_liba}
+   cc -shared -o $_lib $(ls *.so | sort -R) $(cat .ldadd)
+   [[ -s $_lib ]] && file $_lib | fgrep -q 'shared object'
+   cp -p /usr/lib/$_lib /usr/lib/$_tmplib
+   install -o root -g bin -m 0444 $_lib /usr/lib/$_lib &&
+   rm -f /usr/lib/$_tmplib ||
+   mv /usr/lib/$_tmplib /usr/lib/$_lib
+   )
+   rm -rf /tmp/_librebuild.${_tmpdir#*.}
+   done
+}
+
 # Check filesystems, optionally by using a fsck(8) flag.
 # Usage: do_fsck [-flag]
 do_fsck() {
@@ -337,6 +368,8 @@ mount -s /usr >/dev/null 2>&1
 mount -s /var >/dev/null 2>&1
 
 random_seed
+
+rebuildlibs
 
 # Clean up left-over files.
 rm -f /etc/nologin /var/spool/lock/LCK.* /var/spool/uucp/STST/*
Index: lib/libc/Makefile
===
RCS file: /cvs/src/lib/libc/Makefile,v
retrieving revision 1.38
diff -u -p -u -r1.38 Makefile
--- lib/libc/Makefile   10 Nov 2015 04:14:03 -  1.38
+++ lib/libc/Makefile   28 Mar 2016 04:08:34 -
@@ -6,6 +6,7 @@
 .include 
 
 LIB=c
+LIBREBUILD=y
 CLEANFILES+=tags Symbols.map
 

Single route lookup when forwarding an IPv4 packet

[Martin Pieuchot]

Diff below makes ip_forward() use the route entry fetched in in_ouraddr().

As you can imagine a proper caching could be done for forwarding using
PF statekey.

This diff has been tested by Hrvoje Popovski who confirmed that the
benchmark performances are similar to the ones using a single cache
entry.

ok?


Index: netinet/ip_input.c
===
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.274
diff -u -p -r1.274 ip_input.c
--- netinet/ip_input.c  25 Apr 2016 12:33:48 -  1.274
+++ netinet/ip_input.c  25 Apr 2016 12:34:46 -
@@ -126,8 +126,8 @@ static struct mbuf_queueipsend_mq;
 
 void   ip_ours(struct mbuf *);
 intip_dooptions(struct mbuf *, struct ifnet *);
-intin_ouraddr(struct mbuf *, struct ifnet *, struct in_addr);
-void   ip_forward(struct mbuf *, struct ifnet *, int);
+intin_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **);
+void   ip_forward(struct mbuf *, struct ifnet *, struct rtentry *, int);
 #ifdef IPSEC
 intip_input_ipsec_fwd_check(struct mbuf *, int);
 intip_input_ipsec_ours_check(struct mbuf *, int);
@@ -223,8 +223,9 @@ ipintr(void)
 void
 ipv4_input(struct mbuf *m)
 {
-   struct ifnet *ifp;
-   struct ip *ip;
+   struct ifnet*ifp;
+   struct rtentry  *rt = NULL;
+   struct ip   *ip;
int hlen, len;
 #if defined(MROUTING) || defined(IPSEC)
int rv;
@@ -341,7 +342,7 @@ ipv4_input(struct mbuf *m)
goto out;
}
 
-   if (in_ouraddr(m, ifp, ip->ip_dst)) {
+   if (in_ouraddr(m, ifp, )) {
ip_ours(m);
goto out;
}
@@ -443,12 +444,13 @@ ipv4_input(struct mbuf *m)
}
 #endif /* IPSEC */
 
-   ip_forward(m, ifp, pfrdr);
+   ip_forward(m, ifp, rt, pfrdr);
if_put(ifp);
return;
 bad:
m_freem(m);
 out:
+   rtfree(rt);
if_put(ifp);
 }
 
@@ -575,9 +577,10 @@ bad:
 }
 
 int
-in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct in_addr ina)
+in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct rtentry **prt)
 {
struct rtentry  *rt;
+   struct ip   *ip;
struct sockaddr_in   sin;
int  match = 0;
 #if NPF > 0
@@ -597,10 +600,12 @@ in_ouraddr(struct mbuf *m, struct ifnet 
}
 #endif
 
+   ip = mtod(m, struct ip *);
+
memset(, 0, sizeof(sin));
sin.sin_len = sizeof(sin);
sin.sin_family = AF_INET;
-   sin.sin_addr = ina;
+   sin.sin_addr = ip->ip_dst;
rt = rtalloc(sintosa(), 0, m->m_pkthdr.ph_rtableid);
if (rtisvalid(rt)) {
if (ISSET(rt->rt_flags, RTF_LOCAL))
@@ -618,7 +623,7 @@ in_ouraddr(struct mbuf *m, struct ifnet 
m->m_flags |= M_BCAST;
}
}
-   rtfree(rt);
+   *prt = rt;
 
if (!match) {
struct ifaddr *ifa;
@@ -630,7 +635,7 @@ in_ouraddr(struct mbuf *m, struct ifnet 
 * address on the interface it was received on.
 */
if (!ISSET(m->m_flags, M_BCAST) ||
-   !IN_CLASSFULBROADCAST(ina.s_addr, ina.s_addr))
+   !IN_CLASSFULBROADCAST(ip->ip_dst.s_addr, ip->ip_dst.s_addr))
return (0);
 
if (ifp->if_rdomain != rtable_l2(m->m_pkthdr.ph_rtableid))
@@ -645,7 +650,7 @@ in_ouraddr(struct mbuf *m, struct ifnet 
if (ifa->ifa_addr->sa_family != AF_INET)
continue;
 
-   if (IN_CLASSFULBROADCAST(ina.s_addr,
+   if (IN_CLASSFULBROADCAST(ip->ip_dst.s_addr,
ifatoia(ifa)->ia_addr.sin_addr.s_addr)) {
match = 1;
break;
@@ -1241,7 +1246,7 @@ ip_dooptions(struct mbuf *m, struct ifne
}
KERNEL_UNLOCK();
if (forward && ipforwarding) {
-   ip_forward(m, ifp, 1);
+   ip_forward(m, ifp, NULL, 1);
return (1);
}
return (0);
@@ -1387,12 +1392,11 @@ int inetctlerrmap[PRC_NCMDS] = {
  * via a source route.
  */
 void
-ip_forward(struct mbuf *m, struct ifnet *ifp, int srcrt)
+ip_forward(struct mbuf *m, struct ifnet *ifp, struct rtentry *rt, int srcrt)
 {
struct mbuf mfake, *mcopy = NULL;
struct ip *ip = mtod(m, struct ip *);
struct sockaddr_in *sin;
-   struct rtentry *rt;
struct route ro;
int error, type = 0, code = 0, destmtu = 0, fake = 0, len;
u_int32_t dest;
@@ -1401,25 +1405,27 @@ ip_forward(struct mbuf *m, struct ifnet 
if (m->m_flags & (M_BCAST|M_MCAST) || in_canforward(ip->ip_dst) == 0) {
ipstat.ips_cantforward++;
m_freem(m);
-   return;
+   goto freecopy;
}
if (ip->ip_ttl <= IPTTLDEC) {
icmp_error(m, 

pledge: serialize (v2 - reordering version)

[Sebastien Marie]

Hi,

sys_pledge() has races when concurrent threads calling it in parallel.

for now, the race isn't really triggerable, but when wlpaths will be
enabled it could be more simple, and will result in leaking memory
(malloc without associated free).

This diff is an attempt to reordering operations in sys_pledge() in
order to avoid these memory leaks. Concurrent threads could still enter
together in sys_pledge(), but the reordering ensures that allocated
memory is always freed, and one thread will see EPERM while another will
see 0 (and its pledging applied).

It is acheived by moving all checks on ps_pledge and ps_pledgepaths
values at end of the call, just before setting them. As these operations
(checks + setting) doesn't sleep (just comparing values or simples
operations) there could be considered as atomic (pledge(2) is under
KERN_LOCK).

The global view of sys_pledge become:
  - parse `requests' (char *) to `flags' (uint64_t)
  - parse `paths' (char *) to `wl' (struct whitepaths)
  (once here: no more sleep)
  - check `flags' and `wl' are setteable (allow only promises
reductions, and setting ps_pledgepaths only once) - free allocated
string on error
  - set `flags' and `wl' to struct proc.

In order to avoid abuse of repeatively call pledge(NULL, BIGLIST) for
DoSing the kernel (1. doing parsing / 2. get error / 3. goto 1), I keep
a check at beginning of `paths' parsing, and commented it accordingly.

Comments ?
-- 
Sebastien Marie


Index: kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.164
diff -u -p -r1.164 kern_pledge.c
--- kern/kern_pledge.c  25 Apr 2016 10:01:23 -  1.164
+++ kern/kern_pledge.c  25 Apr 2016 11:34:08 -
@@ -400,6 +400,7 @@ sys_pledge(struct proc *p, void *v, regi
syscallarg(const char **)paths;
} */*uap = v;
uint64_t flags = 0;
+   struct whitepaths *wl = NULL;
int error;
 
if (SCARG(uap, request)) {
@@ -433,15 +434,6 @@ sys_pledge(struct proc *p, void *v, regi
flags |= f;
}
free(rbuf, M_TEMP, MAXPATHLEN);
-
-   /*
-* if we are already pledged, allow only promises reductions.
-* flags doesn't contain flags outside _USERSET: they will be
-* relearned.
-*/
-   if (ISSET(p->p_p->ps_flags, PS_PLEDGE) &&
-   (((flags | p->p_p->ps_pledge) != p->p_p->ps_pledge)))
-   return (EPERM);
}
 
if (SCARG(uap, paths)) {
@@ -449,13 +441,16 @@ sys_pledge(struct proc *p, void *v, regi
return (EINVAL);
 #else
const char **u = SCARG(uap, paths), *sp;
-   struct whitepaths *wl;
char *path, *rdir = NULL, *cwd = NULL;
size_t pathlen, rdirlen, cwdlen;
 
size_t maxargs = 0;
int i, error;
 
+   /*
+* anticipated check. it doesn't protect from races, but will
+* avoid abuse of calling repeatively pledge(2).
+*/
if (p->p_p->ps_pledgepaths)
return (EPERM);
 
@@ -513,14 +508,8 @@ sys_pledge(struct proc *p, void *v, regi
free(cwd, M_TEMP, cwdlen);
free(path, M_TEMP, MAXPATHLEN);
 
-   if (error) {
-   for (i = 0; i < wl->wl_count; i++)
-   free(wl->wl_paths[i].name,
-   M_TEMP, wl->wl_paths[i].len);
-   free(wl, M_TEMP, wl->wl_size);
-   return (error);
-   }
-   p->p_p->ps_pledgepaths = wl;
+   if (error)
+   goto error;
 
 #ifdef DEBUG_PLEDGE
/* print paths registered as whilelisted (viewed as without 
chroot) */
@@ -535,12 +524,56 @@ sys_pledge(struct proc *p, void *v, regi
 #endif
}
 
+   /*
+* WARNING: all the code below should be done without sleep.
+* it ensures no race between threads.
+*
+* - check for permissions on requested parameters
+* - apply ps_pledgepaths
+* - apply ps_pledge, and set PS_PLEDGE flag
+*/
+
+   /*
+* if we are already pledged, allow only promises reductions.
+* flags doesn't contain flags outside _USERSET: they will be
+* relearned.
+*/
+   if (ISSET(p->p_p->ps_flags, PS_PLEDGE) &&
+   (((flags | p->p_p->ps_pledge) != p->p_p->ps_pledge))) {
+   error = EPERM;
+   goto error;
+   }
+
+   /* we can set ps_pledgepaths only once */
+   if (wl && p->p_p->ps_pledgepaths) {
+   error = EPERM;
+   goto error;
+   }
+
+   /* apply ps_pledgepaths */
+   if (wl)
+   p->p_p->ps_pledgepaths = 

Re: MP-safe TX for cnmac(4)

[Visa Hankala]

On Mon, Apr 25, 2016 at 10:20:32AM +0200, Martin Pieuchot wrote:
> On 24/04/16(Sun) 16:13, Visa Hankala wrote:
> > This adds MP-safe TX for cnmac(4). OK?
> 
> Would it be possible to do that without mutex?  Having the same pattern
> across most of our drivers would reduce the maintenance effort.

The only real use of the mutex is to keep octeon_eth_tick_free() away
from sc_sendq while the start routine is running. The queue tracks mbufs
that need to be freed after transmission. Unlike many other drivers,
there is no TX ring. As long as there is at least a little bit of
traffic, octeon_eth_start() can take care of draining the queue. The
hardware does not have a transmission complete interrupt, so the timer
is there to free transmitted mbufs in case traffic stops altogether for
a long time.

What driver would be a good example for this case?

> > 
> > Index: arch/octeon/dev/if_cnmac.c
> > ===
> > RCS file: src/sys/arch/octeon/dev/if_cnmac.c,v
> > retrieving revision 1.38
> > diff -u -p -r1.38 if_cnmac.c
> > --- arch/octeon/dev/if_cnmac.c  13 Apr 2016 11:34:00 -  1.38
> > +++ arch/octeon/dev/if_cnmac.c  24 Apr 2016 15:35:04 -
> > @@ -285,6 +285,7 @@ octeon_eth_attach(struct device *parent,
> > octeon_eth_gsc[sc->sc_port] = sc;
> >  
> > ml_init(>sc_sendq);
> > +   mtx_init(>sc_sendq_mtx, IPL_NET);
> > sc->sc_soft_req_thresh = 15/* XXX */;
> > sc->sc_ext_callback_cnt = 0;
> >  
> > @@ -317,6 +318,7 @@ octeon_eth_attach(struct device *parent,
> > strncpy(ifp->if_xname, sc->sc_dev.dv_xname, sizeof(ifp->if_xname));
> > ifp->if_softc = sc;
> > ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
> > +   ifp->if_xflags = IFXF_MPSAFE;
> > ifp->if_ioctl = octeon_eth_ioctl;
> > ifp->if_start = octeon_eth_start;
> > ifp->if_watchdog = octeon_eth_watchdog;
> > @@ -742,7 +744,7 @@ octeon_eth_ioctl(struct ifnet *ifp, u_lo
> > error = 0;
> > }
> >  
> > -   octeon_eth_start(ifp);
> > +   if_start(ifp);
> >  
> > splx(s);
> > return (error);
> > @@ -959,18 +961,19 @@ octeon_eth_start(struct ifnet *ifp)
> > struct octeon_eth_softc *sc = ifp->if_softc;
> > struct mbuf *m;
> >  
> > +   if (__predict_false(!cn30xxgmx_link_status(sc->sc_gmx_port))) {
> > +   IFQ_PURGE(>if_snd);
> > +   return;
> > +   }
> > +
> > +   mtx_enter(>sc_sendq_mtx);
> > +
> > /*
> >  * performance tuning
> >  * presend iobdma request 
> >  */
> > octeon_eth_send_queue_flush_prefetch(sc);
> >  
> > -   if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(>if_snd))
> > -   goto last;
> > -
> > -   if (__predict_false(!cn30xxgmx_link_status(sc->sc_gmx_port)))
> > -   goto last;
> > -
> > for (;;) {
> > octeon_eth_send_queue_flush_fetch(sc); /* XXX */
> >  
> > @@ -980,13 +983,16 @@ octeon_eth_start(struct ifnet *ifp)
> >  * and bail out.
> >  */
> > if (octeon_eth_send_queue_is_full(sc)) {
> > +   mtx_leave(>sc_sendq_mtx);
> > return;
> > }
> > /* XXX */
> >  
> > IFQ_DEQUEUE(>if_snd, m);
> > -   if (m == NULL)
> > +   if (m == NULL) {
> > +   mtx_leave(>sc_sendq_mtx);
> > return;
> > +   }
> >  
> > OCTEON_ETH_TAP(ifp, m, BPF_DIRECTION_OUT);
> >  
> > @@ -1008,8 +1014,9 @@ octeon_eth_start(struct ifnet *ifp)
> > octeon_eth_send_queue_flush_prefetch(sc);
> > }
> >  
> > -last:
> > octeon_eth_send_queue_flush_fetch(sc);
> > +
> > +   mtx_leave(>sc_sendq_mtx);
> >  }
> >  
> >  void
> > @@ -1025,7 +1032,7 @@ octeon_eth_watchdog(struct ifnet *ifp)
> > ifq_clr_oactive(>if_snd);
> > ifp->if_timer = 0;
> >  
> > -   octeon_eth_start(ifp);
> > +   if_start(ifp);
> >  }
> >  
> >  int
> > @@ -1066,6 +1073,8 @@ octeon_eth_stop(struct ifnet *ifp, int d
> >  {
> > struct octeon_eth_softc *sc = ifp->if_softc;
> >  
> > +   CLR(ifp->if_flags, IFF_RUNNING);
> > +
> > timeout_del(>sc_tick_misc_ch);
> > timeout_del(>sc_tick_free_ch);
> > timeout_del(>sc_resume_ch);
> > @@ -1074,13 +1083,12 @@ octeon_eth_stop(struct ifnet *ifp, int d
> >  
> > cn30xxgmx_port_enable(sc->sc_gmx_port, 0);
> >  
> > -   /* Mark the interface as down and cancel the watchdog timer. */
> > -   CLR(ifp->if_flags, IFF_RUNNING);
> > +   intr_barrier(octeon_eth_pow_recv_ih);
> > +   ifq_barrier(>if_snd);
> > +
> > ifq_clr_oactive(>if_snd);
> > ifp->if_timer = 0;
> >  
> > -   intr_barrier(octeon_eth_pow_recv_ih);
> > -
> > return 0;
> >  }
> >  
> > @@ -1372,9 +1380,8 @@ octeon_eth_tick_free(void *arg)
> >  {
> > struct octeon_eth_softc *sc = arg;
> > int timo;
> > -   int s;
> >  
> > -   s = splnet();
> > +   mtx_enter(>sc_sendq_mtx);
> > /* XXX */
> > if (ml_len(>sc_sendq) > 0) {
> > 

Re: pledge: remove unneeded check in sys_pledge()

[Jeremie Courreges-Anglas]

Sebastien Marie  writes:

> On Sun, Apr 10, 2016 at 01:54:33PM +0200, Sebastien Marie wrote:
>> Hi,
>> 
>> The following diff removes an unneeded check on flags. It was used
>> historically, when tame(2) promises were passed as bitflags, in order to
>> avoid userland to be able to set flags normally managed by kernel.
>> 
>> Nowadays, flags is build using pledgereq_flags() function which returns
>> bitflag from string in controlled way. So userland can't set high bits
>> in flags.
>> 
>
> ping ?

Makes sense, ok.

>> 
>> Index: kern/kern_pledge.c
>> ===
>> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
>> retrieving revision 1.162
>> diff -u -p -r1.162 kern_pledge.c
>> --- kern/kern_pledge.c   30 Mar 2016 07:49:11 -  1.162
>> +++ kern/kern_pledge.c   10 Apr 2016 11:47:30 -
>> @@ -434,9 +434,6 @@ sys_pledge(struct proc *p, void *v, regi
>>  }
>>  free(rbuf, M_TEMP, MAXPATHLEN);
>>  
>> -if (flags & ~PLEDGE_USERSET)
>> -return (EINVAL);
>> -
>>  if ((p->p_p->ps_flags & PS_PLEDGE)) {
>>  /* Already pledged, only allow reductions */
>>  if (((flags | p->p_p->ps_pledge) & PLEDGE_USERSET) !=


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

Re: pledge: remove unneeded check in sys_pledge()

[Sebastien Marie]

On Sun, Apr 10, 2016 at 01:54:33PM +0200, Sebastien Marie wrote:
> Hi,
> 
> The following diff removes an unneeded check on flags. It was used
> historically, when tame(2) promises were passed as bitflags, in order to
> avoid userland to be able to set flags normally managed by kernel.
> 
> Nowadays, flags is build using pledgereq_flags() function which returns
> bitflag from string in controlled way. So userland can't set high bits
> in flags.
> 

ping ?

> 
> Index: kern/kern_pledge.c
> ===
> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
> retrieving revision 1.162
> diff -u -p -r1.162 kern_pledge.c
> --- kern/kern_pledge.c30 Mar 2016 07:49:11 -  1.162
> +++ kern/kern_pledge.c10 Apr 2016 11:47:30 -
> @@ -434,9 +434,6 @@ sys_pledge(struct proc *p, void *v, regi
>   }
>   free(rbuf, M_TEMP, MAXPATHLEN);
>  
> - if (flags & ~PLEDGE_USERSET)
> - return (EINVAL);
> -
>   if ((p->p_p->ps_flags & PS_PLEDGE)) {
>   /* Already pledged, only allow reductions */
>   if (((flags | p->p_p->ps_pledge) & PLEDGE_USERSET) !=

-- 
Sebastien Marie

Re: ifa_ifwithroute() fix

[Martin Pieuchot]

On 19/04/16(Tue) 10:43, Martin Pieuchot wrote:
> Mart Tõnso reported [0] a weird case related to the use of ifa_ifwithnet().
> 
> The problem is that ifa_ifwithroute() does not always use route entries but
> the poor's man routing table: ifa_ifwithnet().  This is misleading because
> one cannot understand why "# route add" is not coherent with "# route get".
> 
> So I'd like to commit the diff below which always use the route table
> unless an interface index is specified in the gateway.  Mart Tõnso
> confirmed it fixes his issue.
> 
> ok?

Any network hacker to review this?

> 
> [0] https://marc.info/?l=openbsd-misc=146046751201006=2
> 
> 
> Index: net/route.c
> ===
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.298
> diff -u -p -r1.298 route.c
> --- net/route.c   26 Mar 2016 21:56:04 -  1.298
> +++ net/route.c   13 Apr 2016 07:38:11 -
> @@ -740,20 +740,16 @@ ifa_ifwithroute(int flags, struct sockad
>   ifa = ifaof_ifpforaddr(dst, ifp);
>   if_put(ifp);
>   } else {
> - ifa = ifa_ifwithnet(gateway, rtableid);
> - }
> - }
> - if (ifa == NULL) {
> - struct rtentry  *rt = rtalloc(gateway, 0, rtableid);
> - /* The gateway must be local if the same address family. */
> - if (!rtisvalid(rt) || ((rt->rt_flags & RTF_GATEWAY) &&
> - rt_key(rt)->sa_family == dst->sa_family)) {
> + struct rtentry *rt;
> +
> + rt = rtalloc(gateway, RT_RESOLVE, rtableid);
> + if (rt != NULL)
> + ifa = rt->rt_ifa;
>   rtfree(rt);
> - return (NULL);
>   }
> - ifa = rt->rt_ifa;
> - rtfree(rt);
>   }
> + if (ifa == NULL)
> + return (NULL);
>   if (ifa->ifa_addr->sa_family != dst->sa_family) {
>   struct ifaddr   *oifa = ifa;
>   ifa = ifaof_ifpforaddr(dst, ifa->ifa_ifp);
> 

Re: MP-safe TX for cnmac(4)

[Martin Pieuchot]

On 24/04/16(Sun) 16:13, Visa Hankala wrote:
> This adds MP-safe TX for cnmac(4). OK?

Would it be possible to do that without mutex?  Having the same pattern
across most of our drivers would reduce the maintenance effort.

> 
> Index: arch/octeon/dev/if_cnmac.c
> ===
> RCS file: src/sys/arch/octeon/dev/if_cnmac.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 if_cnmac.c
> --- arch/octeon/dev/if_cnmac.c13 Apr 2016 11:34:00 -  1.38
> +++ arch/octeon/dev/if_cnmac.c24 Apr 2016 15:35:04 -
> @@ -285,6 +285,7 @@ octeon_eth_attach(struct device *parent,
>   octeon_eth_gsc[sc->sc_port] = sc;
>  
>   ml_init(>sc_sendq);
> + mtx_init(>sc_sendq_mtx, IPL_NET);
>   sc->sc_soft_req_thresh = 15/* XXX */;
>   sc->sc_ext_callback_cnt = 0;
>  
> @@ -317,6 +318,7 @@ octeon_eth_attach(struct device *parent,
>   strncpy(ifp->if_xname, sc->sc_dev.dv_xname, sizeof(ifp->if_xname));
>   ifp->if_softc = sc;
>   ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
> + ifp->if_xflags = IFXF_MPSAFE;
>   ifp->if_ioctl = octeon_eth_ioctl;
>   ifp->if_start = octeon_eth_start;
>   ifp->if_watchdog = octeon_eth_watchdog;
> @@ -742,7 +744,7 @@ octeon_eth_ioctl(struct ifnet *ifp, u_lo
>   error = 0;
>   }
>  
> - octeon_eth_start(ifp);
> + if_start(ifp);
>  
>   splx(s);
>   return (error);
> @@ -959,18 +961,19 @@ octeon_eth_start(struct ifnet *ifp)
>   struct octeon_eth_softc *sc = ifp->if_softc;
>   struct mbuf *m;
>  
> + if (__predict_false(!cn30xxgmx_link_status(sc->sc_gmx_port))) {
> + IFQ_PURGE(>if_snd);
> + return;
> + }
> +
> + mtx_enter(>sc_sendq_mtx);
> +
>   /*
>* performance tuning
>* presend iobdma request 
>*/
>   octeon_eth_send_queue_flush_prefetch(sc);
>  
> - if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(>if_snd))
> - goto last;
> -
> - if (__predict_false(!cn30xxgmx_link_status(sc->sc_gmx_port)))
> - goto last;
> -
>   for (;;) {
>   octeon_eth_send_queue_flush_fetch(sc); /* XXX */
>  
> @@ -980,13 +983,16 @@ octeon_eth_start(struct ifnet *ifp)
>* and bail out.
>*/
>   if (octeon_eth_send_queue_is_full(sc)) {
> + mtx_leave(>sc_sendq_mtx);
>   return;
>   }
>   /* XXX */
>  
>   IFQ_DEQUEUE(>if_snd, m);
> - if (m == NULL)
> + if (m == NULL) {
> + mtx_leave(>sc_sendq_mtx);
>   return;
> + }
>  
>   OCTEON_ETH_TAP(ifp, m, BPF_DIRECTION_OUT);
>  
> @@ -1008,8 +1014,9 @@ octeon_eth_start(struct ifnet *ifp)
>   octeon_eth_send_queue_flush_prefetch(sc);
>   }
>  
> -last:
>   octeon_eth_send_queue_flush_fetch(sc);
> +
> + mtx_leave(>sc_sendq_mtx);
>  }
>  
>  void
> @@ -1025,7 +1032,7 @@ octeon_eth_watchdog(struct ifnet *ifp)
>   ifq_clr_oactive(>if_snd);
>   ifp->if_timer = 0;
>  
> - octeon_eth_start(ifp);
> + if_start(ifp);
>  }
>  
>  int
> @@ -1066,6 +1073,8 @@ octeon_eth_stop(struct ifnet *ifp, int d
>  {
>   struct octeon_eth_softc *sc = ifp->if_softc;
>  
> + CLR(ifp->if_flags, IFF_RUNNING);
> +
>   timeout_del(>sc_tick_misc_ch);
>   timeout_del(>sc_tick_free_ch);
>   timeout_del(>sc_resume_ch);
> @@ -1074,13 +1083,12 @@ octeon_eth_stop(struct ifnet *ifp, int d
>  
>   cn30xxgmx_port_enable(sc->sc_gmx_port, 0);
>  
> - /* Mark the interface as down and cancel the watchdog timer. */
> - CLR(ifp->if_flags, IFF_RUNNING);
> + intr_barrier(octeon_eth_pow_recv_ih);
> + ifq_barrier(>if_snd);
> +
>   ifq_clr_oactive(>if_snd);
>   ifp->if_timer = 0;
>  
> - intr_barrier(octeon_eth_pow_recv_ih);
> -
>   return 0;
>  }
>  
> @@ -1372,9 +1380,8 @@ octeon_eth_tick_free(void *arg)
>  {
>   struct octeon_eth_softc *sc = arg;
>   int timo;
> - int s;
>  
> - s = splnet();
> + mtx_enter(>sc_sendq_mtx);
>   /* XXX */
>   if (ml_len(>sc_sendq) > 0) {
>   octeon_eth_send_queue_flush_prefetch(sc);
> @@ -1389,7 +1396,7 @@ octeon_eth_tick_free(void *arg)
>timo = 10;
>   timeout_add_msec(>sc_tick_free_ch, 1000 * timo / hz);
>   /* XXX */
> - splx(s);
> + mtx_leave(>sc_sendq_mtx);
>  }
>  
>  /*
> Index: arch/octeon/dev/if_cnmacvar.h
> ===
> RCS file: src/sys/arch/octeon/dev/if_cnmacvar.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 if_cnmacvar.h
> --- arch/octeon/dev/if_cnmacvar.h 8 Oct 2015 14:24:32 -   1.7
> +++ arch/octeon/dev/if_cnmacvar.h 24 Apr 2016 15:35:04 -
> @@ -80,6 +80,7 @@ struct octeon_eth_softc {
>   int64_t sc_hard_done_cnt;
>   int 

Re: Duplicate route lookups

[Martin Pieuchot]

On 19/04/16(Tue) 11:18, Martin Pieuchot wrote:
> These two chunks are equivalent so let's keep one.
> 
> ok?

Nobody?

> 
> Index: netinet6/ip6_forward.c
> ===
> RCS file: /cvs/src/sys/netinet6/ip6_forward.c,v
> retrieving revision 1.88
> diff -u -p -r1.88 ip6_forward.c
> --- netinet6/ip6_forward.c19 Apr 2016 08:23:13 -  1.88
> +++ netinet6/ip6_forward.c19 Apr 2016 08:47:21 -
> @@ -220,35 +220,7 @@ reroute:
>  #endif
>  
>   dst = _forward_rt.ro_dst;
> - if (!srcrt) {
> - /*
> -  * ip6_forward_rt.ro_dst.sin6_addr is equal to ip6->ip6_dst
> -  */
> - if (!rtisvalid(ip6_forward_rt.ro_rt) ||
> - ISSET(ip6_forward_rt.ro_rt->rt_flags, RTF_MPATH) ||
> - ip6_forward_rt.ro_tableid != rtableid) {
> - if (ip6_forward_rt.ro_rt) {
> - rtfree(ip6_forward_rt.ro_rt);
> - ip6_forward_rt.ro_rt = NULL;
> - }
> - /* this probably fails but give it a try again */
> - ip6_forward_rt.ro_tableid = rtableid;
> - ip6_forward_rt.ro_rt = rtalloc_mpath(
> - sin6tosa(_forward_rt.ro_dst),
> - >ip6_src.s6_addr32[0],
> - ip6_forward_rt.ro_tableid);
> - }
> -
> - if (ip6_forward_rt.ro_rt == NULL) {
> - ip6stat.ip6s_noroute++;
> - if (mcopy) {
> - icmp6_error(mcopy, ICMP6_DST_UNREACH,
> - ICMP6_DST_UNREACH_NOROUTE, 0);
> - }
> - m_freem(m);
> - return;
> - }
> - } else if (!rtisvalid(ip6_forward_rt.ro_rt) ||
> + if (!rtisvalid(ip6_forward_rt.ro_rt) ||
>  ISSET(ip6_forward_rt.ro_rt->rt_flags, RTF_MPATH) ||
>  !IN6_ARE_ADDR_EQUAL(>ip6_dst, >sin6_addr) ||
>  ip6_forward_rt.ro_tableid != rtableid) {
> 

Re: Moving away from softnet interrupts

[Martin Pieuchot]

On 20/04/16(Wed) 09:33, Dimitris Papastamos wrote:
> On Mon, Apr 18, 2016 at 10:50:46AM +0200, Martin Pieuchot wrote:
> > The current goal of the Network SMP effort is to have a single CPU
> > process the IP forwarding path in a process context without holding
> > the KERNEL_LOCK().  To achieve this goal we're progressively moving
> > code from the softnet interrupt context to the if_input_task.  In
> > the end we'll completely get rid of this soft-interrupt.
> > 
> > So now would be a good time to know if moving all the code currently
> > run in a soft-interrupt context to a task uncovers any bug.  I'm
> > happily running the diff below on amd64 and macppc, it even gives me
> > a small performance boost.
> > 
> > I'd appreciate more tests especially on exotic archs.
> 
> I've been running with this diff since you posted it on my home router.
> I have not encountered any issues.

Thanks for testing.

I'm still looking for reports on different architectures.