Re: syslogd tls read write events
ok beck@
On Fri, Sep 18, 2015 at 5:17 PM, Alexander Bluhm
wrote:
> On Fri, Sep 18, 2015 at 05:04:55PM -0600, Bob Beck wrote:
>> ... and just to be clear, you only need to event_del when you are
>> switching to wanting a write when you did want a read before, and vice
>> versa... correct?
>
> Yes. When I am doing reads and get a TLS_WANT_POLLOUT, I have to
> do a event_set() on the write event to change the callback to read.
> As there might be an event scheduled on the event, I have to do an
> event_del() before event_set().
>
> That's why the changing case is a bit more complex.
>
> After success I have to change back the other event, so I need
> event_del() and event_set() again.
>
> bluhm
>
>> if that's the case it reads ok...
>>
>>
>> On Fri, Sep 18, 2015 at 4:55 PM, Alexander Bluhm
>> wrote:
>> > Hi,
>> >
>> > I discovered what caused the strange event loss in syslogd during
>> > the hackaton. I had mixed EV_READ and EV_WRITE events on the ev_read
>> > and ev_write event structures. The correct way is to use each event
>> > for its read and write purpose and instead switch the handler. Then
>> > libevent is no longer confused.
>> >
>> > ok?
>> >
>> > bluhm
>> >
>> > Index: usr.sbin/syslogd/evbuffer_tls.c
>> > ===
>> > RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/evbuffer_tls.c,v
>> > retrieving revision 1.7
>> > diff -u -p -r1.7 evbuffer_tls.c
>> > --- usr.sbin/syslogd/evbuffer_tls.c 10 Sep 2015 18:32:06 - 1.7
>> > +++ usr.sbin/syslogd/evbuffer_tls.c 18 Sep 2015 22:53:53 -
>> > @@ -44,6 +44,9 @@
>> > /* prototypes */
>> >
>> > void bufferevent_read_pressure_cb(struct evbuffer *, size_t, size_t, void
>> > *);
>> > +static void buffertls_readcb(int, short, void *);
>> > +static void buffertls_writecb(int, short, void *);
>> > +static void buffertls_handshakecb(int, short, void *);
>> > int evtls_read(struct evbuffer *, int, int, struct tls *);
>> > int evtls_write(struct evbuffer *, int, struct tls *);
>> >
>> > @@ -96,29 +99,30 @@ buffertls_readcb(int fd, short event, vo
>> > res = evtls_read(bufev->input, fd, howmuch, ctx);
>> > switch (res) {
>> > case TLS_WANT_POLLIN:
>> > - event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb,
>> > - buftls);
>> > - goto reschedule;
>> > + bufferevent_add(&bufev->ev_read, bufev->timeout_read);
>> > + return;
>> > case TLS_WANT_POLLOUT:
>> > - event_set(&bufev->ev_read, fd, EV_WRITE, buffertls_readcb,
>> > + event_del(&bufev->ev_write);
>> > + event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_readcb,
>> > buftls);
>> > - goto reschedule;
>> > + bufferevent_add(&bufev->ev_write, bufev->timeout_write);
>> > + return;
>> > case -1:
>> > - if (errno == EAGAIN || errno == EINTR)
>> > - goto reschedule;
>> > - /* error case */
>> > what |= EVBUFFER_ERROR;
>> > break;
>> > case 0:
>> > - /* eof case */
>> > what |= EVBUFFER_EOF;
>> > break;
>> > }
>> > if (res <= 0)
>> > goto error;
>> >
>> > - event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb, buftls);
>> > - bufferevent_add(&bufev->ev_read, bufev->timeout_read);
>> > + event_del(&bufev->ev_write);
>> > + event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_writecb,
>> > buftls);
>> > + if (bufev->enabled & EV_READ)
>> > + bufferevent_add(&bufev->ev_read, bufev->timeout_read);
>> > + if (EVBUFFER_LENGTH(bufev->output) != 0 && bufev->enabled &
>> > EV_WRITE)
>> > + bufferevent_add(&bufev->ev_write, bufev->timeout_write);
>> >
>> > /* See if this callbacks meets the water marks */
>> > len = EVBUFFER_LENGTH(bufev->input);
>> > @@ -137,10 +141,6 @@ buffertls_readcb(int fd, short event, vo
>> > (*bufev->readcb)(bufev, bufev->cbarg);
>> > return;
>> >
>> > - reschedule:
>> > - bufferevent_add(&bufev->ev_read, bufev->timeout_read);
>> > - return;
>> > -
>> > error:
>> > (*bufev->errorcb)(bufev, what, bufev->cbarg);
>> > }
>> > @@ -163,22 +163,18 @@ buffertls_writecb(int fd, short event, v
>> > res = evtls_write(bufev->output, fd, ctx);
>> > switch (res) {
>> > case TLS_WANT_POLLIN:
>> > - event_set(&bufev->ev_write, fd, EV_READ,
>> > + event_del(&bufev->ev_read);
>> > + event_set(&bufev->ev_read, fd, EV_READ,
>> > buffertls_writecb, buftls);
>> > - goto reschedule;
>> > + bufferevent_add(&bufev->ev_read,
>> > bufev->timeo
Re: syslogd tls read write events
On Fri, Sep 18, 2015 at 05:04:55PM -0600, Bob Beck wrote:
> ... and just to be clear, you only need to event_del when you are
> switching to wanting a write when you did want a read before, and vice
> versa... correct?
Yes. When I am doing reads and get a TLS_WANT_POLLOUT, I have to
do a event_set() on the write event to change the callback to read.
As there might be an event scheduled on the event, I have to do an
event_del() before event_set().
That's why the changing case is a bit more complex.
After success I have to change back the other event, so I need
event_del() and event_set() again.
bluhm
> if that's the case it reads ok...
>
>
> On Fri, Sep 18, 2015 at 4:55 PM, Alexander Bluhm
> wrote:
> > Hi,
> >
> > I discovered what caused the strange event loss in syslogd during
> > the hackaton. I had mixed EV_READ and EV_WRITE events on the ev_read
> > and ev_write event structures. The correct way is to use each event
> > for its read and write purpose and instead switch the handler. Then
> > libevent is no longer confused.
> >
> > ok?
> >
> > bluhm
> >
> > Index: usr.sbin/syslogd/evbuffer_tls.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/evbuffer_tls.c,v
> > retrieving revision 1.7
> > diff -u -p -r1.7 evbuffer_tls.c
> > --- usr.sbin/syslogd/evbuffer_tls.c 10 Sep 2015 18:32:06 - 1.7
> > +++ usr.sbin/syslogd/evbuffer_tls.c 18 Sep 2015 22:53:53 -
> > @@ -44,6 +44,9 @@
> > /* prototypes */
> >
> > void bufferevent_read_pressure_cb(struct evbuffer *, size_t, size_t, void
> > *);
> > +static void buffertls_readcb(int, short, void *);
> > +static void buffertls_writecb(int, short, void *);
> > +static void buffertls_handshakecb(int, short, void *);
> > int evtls_read(struct evbuffer *, int, int, struct tls *);
> > int evtls_write(struct evbuffer *, int, struct tls *);
> >
> > @@ -96,29 +99,30 @@ buffertls_readcb(int fd, short event, vo
> > res = evtls_read(bufev->input, fd, howmuch, ctx);
> > switch (res) {
> > case TLS_WANT_POLLIN:
> > - event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb,
> > - buftls);
> > - goto reschedule;
> > + bufferevent_add(&bufev->ev_read, bufev->timeout_read);
> > + return;
> > case TLS_WANT_POLLOUT:
> > - event_set(&bufev->ev_read, fd, EV_WRITE, buffertls_readcb,
> > + event_del(&bufev->ev_write);
> > + event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_readcb,
> > buftls);
> > - goto reschedule;
> > + bufferevent_add(&bufev->ev_write, bufev->timeout_write);
> > + return;
> > case -1:
> > - if (errno == EAGAIN || errno == EINTR)
> > - goto reschedule;
> > - /* error case */
> > what |= EVBUFFER_ERROR;
> > break;
> > case 0:
> > - /* eof case */
> > what |= EVBUFFER_EOF;
> > break;
> > }
> > if (res <= 0)
> > goto error;
> >
> > - event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb, buftls);
> > - bufferevent_add(&bufev->ev_read, bufev->timeout_read);
> > + event_del(&bufev->ev_write);
> > + event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_writecb,
> > buftls);
> > + if (bufev->enabled & EV_READ)
> > + bufferevent_add(&bufev->ev_read, bufev->timeout_read);
> > + if (EVBUFFER_LENGTH(bufev->output) != 0 && bufev->enabled &
> > EV_WRITE)
> > + bufferevent_add(&bufev->ev_write, bufev->timeout_write);
> >
> > /* See if this callbacks meets the water marks */
> > len = EVBUFFER_LENGTH(bufev->input);
> > @@ -137,10 +141,6 @@ buffertls_readcb(int fd, short event, vo
> > (*bufev->readcb)(bufev, bufev->cbarg);
> > return;
> >
> > - reschedule:
> > - bufferevent_add(&bufev->ev_read, bufev->timeout_read);
> > - return;
> > -
> > error:
> > (*bufev->errorcb)(bufev, what, bufev->cbarg);
> > }
> > @@ -163,22 +163,18 @@ buffertls_writecb(int fd, short event, v
> > res = evtls_write(bufev->output, fd, ctx);
> > switch (res) {
> > case TLS_WANT_POLLIN:
> > - event_set(&bufev->ev_write, fd, EV_READ,
> > + event_del(&bufev->ev_read);
> > + event_set(&bufev->ev_read, fd, EV_READ,
> > buffertls_writecb, buftls);
> > - goto reschedule;
> > + bufferevent_add(&bufev->ev_read,
> > bufev->timeout_read);
> > + return;
> > case TLS_WANT_POLLOUT:
> > - event_set(&bufev->ev_write, fd, EV_WRITE,
> > - b
Re: syslogd tls read write events
... and just to be clear, you only need to event_del when you are
switching to wanting a write when you did want a read before, and vice
versa... correct?
if that's the case it reads ok...
On Fri, Sep 18, 2015 at 4:55 PM, Alexander Bluhm
wrote:
> Hi,
>
> I discovered what caused the strange event loss in syslogd during
> the hackaton. I had mixed EV_READ and EV_WRITE events on the ev_read
> and ev_write event structures. The correct way is to use each event
> for its read and write purpose and instead switch the handler. Then
> libevent is no longer confused.
>
> ok?
>
> bluhm
>
> Index: usr.sbin/syslogd/evbuffer_tls.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/evbuffer_tls.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 evbuffer_tls.c
> --- usr.sbin/syslogd/evbuffer_tls.c 10 Sep 2015 18:32:06 - 1.7
> +++ usr.sbin/syslogd/evbuffer_tls.c 18 Sep 2015 22:53:53 -
> @@ -44,6 +44,9 @@
> /* prototypes */
>
> void bufferevent_read_pressure_cb(struct evbuffer *, size_t, size_t, void *);
> +static void buffertls_readcb(int, short, void *);
> +static void buffertls_writecb(int, short, void *);
> +static void buffertls_handshakecb(int, short, void *);
> int evtls_read(struct evbuffer *, int, int, struct tls *);
> int evtls_write(struct evbuffer *, int, struct tls *);
>
> @@ -96,29 +99,30 @@ buffertls_readcb(int fd, short event, vo
> res = evtls_read(bufev->input, fd, howmuch, ctx);
> switch (res) {
> case TLS_WANT_POLLIN:
> - event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb,
> - buftls);
> - goto reschedule;
> + bufferevent_add(&bufev->ev_read, bufev->timeout_read);
> + return;
> case TLS_WANT_POLLOUT:
> - event_set(&bufev->ev_read, fd, EV_WRITE, buffertls_readcb,
> + event_del(&bufev->ev_write);
> + event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_readcb,
> buftls);
> - goto reschedule;
> + bufferevent_add(&bufev->ev_write, bufev->timeout_write);
> + return;
> case -1:
> - if (errno == EAGAIN || errno == EINTR)
> - goto reschedule;
> - /* error case */
> what |= EVBUFFER_ERROR;
> break;
> case 0:
> - /* eof case */
> what |= EVBUFFER_EOF;
> break;
> }
> if (res <= 0)
> goto error;
>
> - event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb, buftls);
> - bufferevent_add(&bufev->ev_read, bufev->timeout_read);
> + event_del(&bufev->ev_write);
> + event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_writecb, buftls);
> + if (bufev->enabled & EV_READ)
> + bufferevent_add(&bufev->ev_read, bufev->timeout_read);
> + if (EVBUFFER_LENGTH(bufev->output) != 0 && bufev->enabled & EV_WRITE)
> + bufferevent_add(&bufev->ev_write, bufev->timeout_write);
>
> /* See if this callbacks meets the water marks */
> len = EVBUFFER_LENGTH(bufev->input);
> @@ -137,10 +141,6 @@ buffertls_readcb(int fd, short event, vo
> (*bufev->readcb)(bufev, bufev->cbarg);
> return;
>
> - reschedule:
> - bufferevent_add(&bufev->ev_read, bufev->timeout_read);
> - return;
> -
> error:
> (*bufev->errorcb)(bufev, what, bufev->cbarg);
> }
> @@ -163,22 +163,18 @@ buffertls_writecb(int fd, short event, v
> res = evtls_write(bufev->output, fd, ctx);
> switch (res) {
> case TLS_WANT_POLLIN:
> - event_set(&bufev->ev_write, fd, EV_READ,
> + event_del(&bufev->ev_read);
> + event_set(&bufev->ev_read, fd, EV_READ,
> buffertls_writecb, buftls);
> - goto reschedule;
> + bufferevent_add(&bufev->ev_read, bufev->timeout_read);
> + return;
> case TLS_WANT_POLLOUT:
> - event_set(&bufev->ev_write, fd, EV_WRITE,
> - buffertls_writecb, buftls);
> - goto reschedule;
> + bufferevent_add(&bufev->ev_write,
> bufev->timeout_write);
> + return;
> case -1:
> - if (errno == EAGAIN || errno == EINTR ||
> - errno == EINPROGRESS)
> - goto reschedule;
> - /* error case */
> what |= EVBUFFER_ERROR;
> break;
> case 0:
> - /* eof case */
> what |= EVBUFFER_EOF;
> break;
>
syslogd tls read write events
Hi,
I discovered what caused the strange event loss in syslogd during
the hackaton. I had mixed EV_READ and EV_WRITE events on the ev_read
and ev_write event structures. The correct way is to use each event
for its read and write purpose and instead switch the handler. Then
libevent is no longer confused.
ok?
bluhm
Index: usr.sbin/syslogd/evbuffer_tls.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/evbuffer_tls.c,v
retrieving revision 1.7
diff -u -p -r1.7 evbuffer_tls.c
--- usr.sbin/syslogd/evbuffer_tls.c 10 Sep 2015 18:32:06 - 1.7
+++ usr.sbin/syslogd/evbuffer_tls.c 18 Sep 2015 22:53:53 -
@@ -44,6 +44,9 @@
/* prototypes */
void bufferevent_read_pressure_cb(struct evbuffer *, size_t, size_t, void *);
+static void buffertls_readcb(int, short, void *);
+static void buffertls_writecb(int, short, void *);
+static void buffertls_handshakecb(int, short, void *);
int evtls_read(struct evbuffer *, int, int, struct tls *);
int evtls_write(struct evbuffer *, int, struct tls *);
@@ -96,29 +99,30 @@ buffertls_readcb(int fd, short event, vo
res = evtls_read(bufev->input, fd, howmuch, ctx);
switch (res) {
case TLS_WANT_POLLIN:
- event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb,
- buftls);
- goto reschedule;
+ bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ return;
case TLS_WANT_POLLOUT:
- event_set(&bufev->ev_read, fd, EV_WRITE, buffertls_readcb,
+ event_del(&bufev->ev_write);
+ event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_readcb,
buftls);
- goto reschedule;
+ bufferevent_add(&bufev->ev_write, bufev->timeout_write);
+ return;
case -1:
- if (errno == EAGAIN || errno == EINTR)
- goto reschedule;
- /* error case */
what |= EVBUFFER_ERROR;
break;
case 0:
- /* eof case */
what |= EVBUFFER_EOF;
break;
}
if (res <= 0)
goto error;
- event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb, buftls);
- bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ event_del(&bufev->ev_write);
+ event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_writecb, buftls);
+ if (bufev->enabled & EV_READ)
+ bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ if (EVBUFFER_LENGTH(bufev->output) != 0 && bufev->enabled & EV_WRITE)
+ bufferevent_add(&bufev->ev_write, bufev->timeout_write);
/* See if this callbacks meets the water marks */
len = EVBUFFER_LENGTH(bufev->input);
@@ -137,10 +141,6 @@ buffertls_readcb(int fd, short event, vo
(*bufev->readcb)(bufev, bufev->cbarg);
return;
- reschedule:
- bufferevent_add(&bufev->ev_read, bufev->timeout_read);
- return;
-
error:
(*bufev->errorcb)(bufev, what, bufev->cbarg);
}
@@ -163,22 +163,18 @@ buffertls_writecb(int fd, short event, v
res = evtls_write(bufev->output, fd, ctx);
switch (res) {
case TLS_WANT_POLLIN:
- event_set(&bufev->ev_write, fd, EV_READ,
+ event_del(&bufev->ev_read);
+ event_set(&bufev->ev_read, fd, EV_READ,
buffertls_writecb, buftls);
- goto reschedule;
+ bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ return;
case TLS_WANT_POLLOUT:
- event_set(&bufev->ev_write, fd, EV_WRITE,
- buffertls_writecb, buftls);
- goto reschedule;
+ bufferevent_add(&bufev->ev_write, bufev->timeout_write);
+ return;
case -1:
- if (errno == EAGAIN || errno == EINTR ||
- errno == EINPROGRESS)
- goto reschedule;
- /* error case */
what |= EVBUFFER_ERROR;
break;
case 0:
- /* eof case */
what |= EVBUFFER_EOF;
break;
}
@@ -186,8 +182,11 @@ buffertls_writecb(int fd, short event, v
goto error;
}
- event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_writecb, buftls);
- if (EVBUFFER_LENGTH(bufev->output) != 0)
+ event_del(&bufev->ev_read);
+ event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb, buftls);
+ if (bufev->enabled & EV_READ)
+ bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+
Re: kill struct inpcbtable's inpt_lastport
On Fri, Sep 18, 2015 at 11:05:55PM +0200, Vincent Gross wrote: > On 09/18/15 15:18, David Hill wrote: > > Is this 'if (count)' statement needed? We know first > last, so count > > will always be positive. lastport will always be set. > > > if last == first, then the if statement will be false and lastport will > > be uninitialized, I believe. > > > > Both remarks are true, but I think it is better to keep a more extensive > refactoring in a separate diff, refactoring that shall get rid of this > yucky code duplication. > Well, this code changes the current behavior. I'd at least change lastport to be initialized to 0 to keep the behavior the same. It was previously set to 0 with M_ZERO. > -- > Vincent Gross >
Re: kill struct inpcbtable's inpt_lastport
On 09/18/15 15:18, David Hill wrote: > Is this 'if (count)' statement needed? We know first > last, so count > will always be positive. lastport will always be set. > if last == first, then the if statement will be false and lastport will > be uninitialized, I believe. > Both remarks are true, but I think it is better to keep a more extensive refactoring in a separate diff, refactoring that shall get rid of this yucky code duplication. -- Vincent Gross
intro.8 should reference rcctl(8)
Reference the rcctl(8) utility in intro.8 Regards, Index: intro.8 === RCS file: /cvs/src/share/man/man8/intro.8,v retrieving revision 1.25 diff -u -p -r1.25 intro.8 --- intro.8 26 Aug 2014 19:33:48 - 1.25 +++ intro.8 18 Sep 2015 20:26:32 - @@ -82,8 +82,11 @@ httpd_flags=NO Thus it is not started by default. To enable or disable daemon processes, administrators should edit the file -.Xr rc.conf.local 8 , -which overrides +.Xr rc.conf.local 8 +or use the rcctl(8) utility. +The +.Xr rc.conf.local 8 +file overrides variable assignments in .Xr rc.conf 8 . So to enable .Xr httpd 8 , @@ -283,7 +286,8 @@ You can find more information by startin .Sh SEE ALSO .Xr afterboot 8 , .Xr rc 8 , -.Xr rc.conf 8 +.Xr rc.conf 8 , +.Xr rcctl 8 .Sh HISTORY The .Nm intro
Re: Brainy: a few bugs
> From: Maxime Villard > Date: Fri, 11 Sep 2015 21:18:18 +0200 > > _23/ USE-AFTER-FREE: sys/dev/sun/z8530ms.c rev1.2 This one's fixed now as well. Thanks.
rc.8 should reference rcctl(8)
Reference the rcctl(8) utility in rc.8. Regards, Index: rc.8 === RCS file: /cvs/src/share/man/man8/rc.8,v retrieving revision 1.40 diff -u -p -r1.40 rc.8 --- rc.822 Jul 2014 07:38:52 - 1.40 +++ rc.818 Sep 2015 19:59:55 - @@ -204,6 +204,7 @@ at boot time .Xr netstart 8 , .Xr rc.conf 8 , .Xr rc.d 8 , +.Xr rcctl 8 , .Xr rc.shutdown 8 .Sh HISTORY The
afterboot.8 should reference rcctl(8)
Reference the rcctl(8) utility from afterboot.8. Regards, Index: afterboot.8 === RCS file: /cvs/src/share/man/man8/afterboot.8,v retrieving revision 1.147 diff -u -p -r1.147 afterboot.8 --- afterboot.8 30 Jul 2015 08:03:49 - 1.147 +++ afterboot.8 18 Sep 2015 19:50:27 - @@ -400,7 +400,8 @@ is in turn influenced by the configurati Again this script should not be changed by administrators: site-specific changes should be made to .Pq freshly created if necessary -.Pa /etc/rc.conf.local . +.Pa /etc/rc.conf.local +or by using the rcctl(8) utility. .Pp Any commands which should be run before the system sets its secure level should be made to @@ -605,6 +606,7 @@ is contained within .Xr dmesg 8 , .Xr ifconfig 8 , .Xr intro 8 , +.Xr rcctl 8 , .Xr sysctl 8 .Sh HISTORY This document first appeared in
rc.d.8 should reference rcctl(8)
Reference the rcctl(8) utility in rc.d.8. Regards, Index: rc.d.8 === RCS file: /cvs/src/share/man/man8/rc.d.8,v retrieving revision 1.28 diff -u -p -r1.28 rc.d.8 --- rc.d.8 25 Feb 2015 23:01:28 - 1.28 +++ rc.d.8 18 Sep 2015 19:23:39 - @@ -176,7 +176,8 @@ for currently running daemons. .Sh SEE ALSO .Xr rc 8 , .Xr rc.conf 8 , -.Xr rc.subr 8 +.Xr rc.subr 8 , +.Xr rcctl 8 .Sh HISTORY The .Pa /etc/rc.d
rc.conf.8 should ref rcctl(8)
Reference the rcctl(8) utility in rc.conf.8 (which discusses rc.conf.local). Rob Index: rc.conf.8 === RCS file: /cvs/src/share/man/man8/rc.conf.8,v retrieving revision 1.26 diff -u -p -r1.26 rc.conf.8 --- rc.conf.8 4 May 2015 22:29:04 - 1.26 +++ rc.conf.8 18 Sep 2015 19:06:50 - @@ -50,7 +50,9 @@ It is advisable to leave .Nm rc.conf untouched, and instead create and edit a new .Nm rc.conf.local -file. +file or use the +.Xr rcctl 8 +utility. Since only the last assignment to any variable takes effect, variables set in this file override variables previously set in .Nm rc.conf . @@ -219,7 +221,8 @@ amd_master=/etc/amd/master # AMD 'master .Xr init 8 , .Xr intro 8 , .Xr rc 8 , -.Xr rc.d 8 +.Xr rc.d 8 , +.Xr rcctl 8 .Sh HISTORY The .Nm
Re: Use M_ZERO in malloc(9)
On Fri, Sep 18, 2015 at 12:31:25PM -0400, Michael McConville wrote:
> Michael McConville wrote:
> > Martin Pieuchot wrote:
> > > On 18/09/15(Fri) 11:47, Michael McConville wrote:
> > > > Index: arch/arm/xscale/pxa27x_udc.c
> > > > ===
> > > > RCS file: /cvs/src/sys/arch/arm/xscale/pxa27x_udc.c,v
> > > > retrieving revision 1.31
> > > > diff -u -p -r1.31 pxa27x_udc.c
> > > > --- arch/arm/xscale/pxa27x_udc.c15 May 2015 13:32:08 -
> > > > 1.31
> > > > +++ arch/arm/xscale/pxa27x_udc.c18 Sep 2015 15:40:49 -
> > > > @@ -973,9 +973,7 @@ pxaudc_allocx(struct usbf_bus *bus)
> > > > if (xfer != NULL)
> > > > SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next);
> > > > else
> > > > - xfer = malloc(sizeof(struct pxaudc_xfer), M_USB,
> > > > M_NOWAIT);
> > > > - if (xfer != NULL)
> > > > - bzero(xfer, sizeof(struct pxaudc_xfer));
> > > > + xfer = malloc(sizeof(struct pxaudc_xfer), M_USB,
> > > > M_NOWAIT | M_ZERO);
> > > > return xfer;
> > > > }
> > >
> > > In case you just removed "xfer" from the free list you're loosing the
> > > bzero.
> >
> > Ah, good catch. Sorry about that.
>
> Does this look better?
>
>
> Index: arch/arm/xscale/pxa27x_udc.c
> ===
> RCS file: /cvs/src/sys/arch/arm/xscale/pxa27x_udc.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 pxa27x_udc.c
> --- arch/arm/xscale/pxa27x_udc.c 15 May 2015 13:32:08 - 1.31
> +++ arch/arm/xscale/pxa27x_udc.c 18 Sep 2015 16:24:55 -
> @@ -972,10 +972,10 @@ pxaudc_allocx(struct usbf_bus *bus)
> xfer = SIMPLEQ_FIRST(&sc->sc_free_xfers);
> if (xfer != NULL)
missing {
> SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next);
> - else
> - xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT);
> - if (xfer != NULL)
> bzero(xfer, sizeof(struct pxaudc_xfer));
> + else
} else
> + xfer = malloc(sizeof(struct pxaudc_xfer), M_USB,
> + M_NOWAIT | M_ZERO);
> return xfer;
> }
>
> Index: isofs/cd9660/cd9660_vfsops.c
> ===
> RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vfsops.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 cd9660_vfsops.c
> --- isofs/cd9660/cd9660_vfsops.c 9 Sep 2015 15:59:19 - 1.71
> +++ isofs/cd9660/cd9660_vfsops.c 18 Sep 2015 16:25:00 -
> @@ -361,8 +361,7 @@ iso_mountfs(devvp, mp, p, argp)
>
> rootp = (struct iso_directory_record *)pri->root_directory_record;
>
> - isomp = malloc(sizeof *isomp, M_ISOFSMNT, M_WAITOK);
> - bzero((caddr_t)isomp, sizeof *isomp);
> + isomp = malloc(sizeof *isomp, M_ISOFSMNT, M_WAITOK | M_ZERO);
> isomp->logical_block_size = logical_block_size;
> isomp->volume_space_size = isonum_733 (pri->volume_space_size);
> bcopy (rootp, isomp->root, sizeof isomp->root);
> Index: kern/sys_process.c
> ===
> RCS file: /cvs/src/sys/kern/sys_process.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 sys_process.c
> --- kern/sys_process.c20 Jan 2015 19:43:21 - 1.67
> +++ kern/sys_process.c18 Sep 2015 16:25:00 -
> @@ -346,10 +346,10 @@ sys_ptrace(struct proc *p, void *v, regi
> /* Just set the trace flag. */
> atomic_setbits_int(&tr->ps_flags, PS_TRACED);
> tr->ps_oppid = tr->ps_pptr->ps_pid;
> - if (tr->ps_ptstat == NULL)
> + if (tr->ps_ptstat == NULL) {
> tr->ps_ptstat = malloc(sizeof(*tr->ps_ptstat),
> - M_SUBPROC, M_WAITOK);
> - memset(tr->ps_ptstat, 0, sizeof(*tr->ps_ptstat));
> + M_SUBPROC, M_WAITOK | M_ZERO);
> + }
> return (0);
>
> case PT_WRITE_I: /* XXX no separate I and D spaces */
>
Re: Use M_ZERO in malloc(9)
> Does this look better?
> Index: arch/arm/xscale/pxa27x_udc.c
> if (xfer != NULL)
> SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next);
> - else
> - xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT);
> - if (xfer != NULL)
> bzero(xfer, sizeof(struct pxaudc_xfer));
> + else
> + xfer = malloc(sizeof(struct pxaudc_xfer), M_USB,
> + M_NOWAIT | M_ZERO);
No, until you add a pair of { } braces.
Re: Use M_ZERO in malloc(9)
Michael McConville wrote:
> Michael McConville wrote:
> > Martin Pieuchot wrote:
> > > On 18/09/15(Fri) 11:47, Michael McConville wrote:
> > > > Index: arch/arm/xscale/pxa27x_udc.c
> > > > ===
> > > > RCS file: /cvs/src/sys/arch/arm/xscale/pxa27x_udc.c,v
> > > > retrieving revision 1.31
> > > > diff -u -p -r1.31 pxa27x_udc.c
> > > > --- arch/arm/xscale/pxa27x_udc.c15 May 2015 13:32:08 -
> > > > 1.31
> > > > +++ arch/arm/xscale/pxa27x_udc.c18 Sep 2015 15:40:49 -
> > > > @@ -973,9 +973,7 @@ pxaudc_allocx(struct usbf_bus *bus)
> > > > if (xfer != NULL)
> > > > SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next);
> > > > else
> > > > - xfer = malloc(sizeof(struct pxaudc_xfer), M_USB,
> > > > M_NOWAIT);
> > > > - if (xfer != NULL)
> > > > - bzero(xfer, sizeof(struct pxaudc_xfer));
> > > > + xfer = malloc(sizeof(struct pxaudc_xfer), M_USB,
> > > > M_NOWAIT | M_ZERO);
> > > > return xfer;
> > > > }
> > >
> > > In case you just removed "xfer" from the free list you're loosing the
> > > bzero.
> >
> > Ah, good catch. Sorry about that.
>
> Does this look better?
Needed braces. I'll move slower next time. Sorry for noise.
Index: arch/arm/xscale/pxa27x_udc.c
===
RCS file: /cvs/src/sys/arch/arm/xscale/pxa27x_udc.c,v
retrieving revision 1.31
diff -u -p -r1.31 pxa27x_udc.c
--- arch/arm/xscale/pxa27x_udc.c15 May 2015 13:32:08 - 1.31
+++ arch/arm/xscale/pxa27x_udc.c18 Sep 2015 16:33:49 -
@@ -970,12 +970,13 @@ pxaudc_allocx(struct usbf_bus *bus)
struct usbf_xfer *xfer;
xfer = SIMPLEQ_FIRST(&sc->sc_free_xfers);
- if (xfer != NULL)
+ if (xfer != NULL) {
SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next);
- else
- xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT);
- if (xfer != NULL)
bzero(xfer, sizeof(struct pxaudc_xfer));
+ } else {
+ xfer = malloc(sizeof(struct pxaudc_xfer), M_USB,
+ M_NOWAIT | M_ZERO);
+ }
return xfer;
}
Re: Use M_ZERO in malloc(9)
Michael McConville wrote:
> Martin Pieuchot wrote:
> > On 18/09/15(Fri) 11:47, Michael McConville wrote:
> > > Index: arch/arm/xscale/pxa27x_udc.c
> > > ===
> > > RCS file: /cvs/src/sys/arch/arm/xscale/pxa27x_udc.c,v
> > > retrieving revision 1.31
> > > diff -u -p -r1.31 pxa27x_udc.c
> > > --- arch/arm/xscale/pxa27x_udc.c 15 May 2015 13:32:08 - 1.31
> > > +++ arch/arm/xscale/pxa27x_udc.c 18 Sep 2015 15:40:49 -
> > > @@ -973,9 +973,7 @@ pxaudc_allocx(struct usbf_bus *bus)
> > > if (xfer != NULL)
> > > SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next);
> > > else
> > > - xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT);
> > > - if (xfer != NULL)
> > > - bzero(xfer, sizeof(struct pxaudc_xfer));
> > > + xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT |
> > > M_ZERO);
> > > return xfer;
> > > }
> >
> > In case you just removed "xfer" from the free list you're loosing the
> > bzero.
>
> Ah, good catch. Sorry about that.
Does this look better?
Index: arch/arm/xscale/pxa27x_udc.c
===
RCS file: /cvs/src/sys/arch/arm/xscale/pxa27x_udc.c,v
retrieving revision 1.31
diff -u -p -r1.31 pxa27x_udc.c
--- arch/arm/xscale/pxa27x_udc.c15 May 2015 13:32:08 - 1.31
+++ arch/arm/xscale/pxa27x_udc.c18 Sep 2015 16:24:55 -
@@ -972,10 +972,10 @@ pxaudc_allocx(struct usbf_bus *bus)
xfer = SIMPLEQ_FIRST(&sc->sc_free_xfers);
if (xfer != NULL)
SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next);
- else
- xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT);
- if (xfer != NULL)
bzero(xfer, sizeof(struct pxaudc_xfer));
+ else
+ xfer = malloc(sizeof(struct pxaudc_xfer), M_USB,
+ M_NOWAIT | M_ZERO);
return xfer;
}
Index: isofs/cd9660/cd9660_vfsops.c
===
RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vfsops.c,v
retrieving revision 1.71
diff -u -p -r1.71 cd9660_vfsops.c
--- isofs/cd9660/cd9660_vfsops.c9 Sep 2015 15:59:19 - 1.71
+++ isofs/cd9660/cd9660_vfsops.c18 Sep 2015 16:25:00 -
@@ -361,8 +361,7 @@ iso_mountfs(devvp, mp, p, argp)
rootp = (struct iso_directory_record *)pri->root_directory_record;
- isomp = malloc(sizeof *isomp, M_ISOFSMNT, M_WAITOK);
- bzero((caddr_t)isomp, sizeof *isomp);
+ isomp = malloc(sizeof *isomp, M_ISOFSMNT, M_WAITOK | M_ZERO);
isomp->logical_block_size = logical_block_size;
isomp->volume_space_size = isonum_733 (pri->volume_space_size);
bcopy (rootp, isomp->root, sizeof isomp->root);
Index: kern/sys_process.c
===
RCS file: /cvs/src/sys/kern/sys_process.c,v
retrieving revision 1.67
diff -u -p -r1.67 sys_process.c
--- kern/sys_process.c 20 Jan 2015 19:43:21 - 1.67
+++ kern/sys_process.c 18 Sep 2015 16:25:00 -
@@ -346,10 +346,10 @@ sys_ptrace(struct proc *p, void *v, regi
/* Just set the trace flag. */
atomic_setbits_int(&tr->ps_flags, PS_TRACED);
tr->ps_oppid = tr->ps_pptr->ps_pid;
- if (tr->ps_ptstat == NULL)
+ if (tr->ps_ptstat == NULL) {
tr->ps_ptstat = malloc(sizeof(*tr->ps_ptstat),
- M_SUBPROC, M_WAITOK);
- memset(tr->ps_ptstat, 0, sizeof(*tr->ps_ptstat));
+ M_SUBPROC, M_WAITOK | M_ZERO);
+ }
return (0);
case PT_WRITE_I: /* XXX no separate I and D spaces */
Re: Use M_ZERO in malloc(9)
Martin Pieuchot wrote: > On 18/09/15(Fri) 11:47, Michael McConville wrote: > > Index: arch/arm/xscale/pxa27x_udc.c > > === > > RCS file: /cvs/src/sys/arch/arm/xscale/pxa27x_udc.c,v > > retrieving revision 1.31 > > diff -u -p -r1.31 pxa27x_udc.c > > --- arch/arm/xscale/pxa27x_udc.c15 May 2015 13:32:08 - 1.31 > > +++ arch/arm/xscale/pxa27x_udc.c18 Sep 2015 15:40:49 - > > @@ -973,9 +973,7 @@ pxaudc_allocx(struct usbf_bus *bus) > > if (xfer != NULL) > > SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next); > > else > > - xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT); > > - if (xfer != NULL) > > - bzero(xfer, sizeof(struct pxaudc_xfer)); > > + xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT | > > M_ZERO); > > return xfer; > > } > > In case you just removed "xfer" from the free list you're loosing the > bzero. Ah, good catch. Sorry about that.
Re: Use M_ZERO in malloc(9)
On 18/09/15(Fri) 11:47, Michael McConville wrote: > Index: arch/arm/xscale/pxa27x_udc.c > === > RCS file: /cvs/src/sys/arch/arm/xscale/pxa27x_udc.c,v > retrieving revision 1.31 > diff -u -p -r1.31 pxa27x_udc.c > --- arch/arm/xscale/pxa27x_udc.c 15 May 2015 13:32:08 - 1.31 > +++ arch/arm/xscale/pxa27x_udc.c 18 Sep 2015 15:40:49 - > @@ -973,9 +973,7 @@ pxaudc_allocx(struct usbf_bus *bus) > if (xfer != NULL) > SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next); > else > - xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT); > - if (xfer != NULL) > - bzero(xfer, sizeof(struct pxaudc_xfer)); > + xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT | > M_ZERO); > return xfer; > } In case you just removed "xfer" from the free list you're loosing the bzero.
[PATCH] SSH tunnels without root permissions
Hi everyone,
The following patch makes it possible to build SSH layer 2 (and layer
3) tunnels without using root permissions when connecting.
This is achieved by root setting up everything beforehand so sshd
doesn't have to do it. However, the old functionality of sshd setting
things up with root permissions is preserved.
What my patch does is:
1. Check the current value of network interface parameters
2. If change is needed try to do it:
* Fail like it failed before if no permissions
* Or set the parameters and proceed like before
3. If no changes needed just continue without fail.
After the patch is applied and sshd is rebuilt, installed and
restarted one can set up SSH layer 2 tunnel server as follows:
* groupadd tunnel
* useradd -m -c "Test User" -G tunnel test
* chown :tunnel /dev/tun? && chmod g+rw /dev/tun?
* for i in 0 1 2 3; do printf "create\nlink0\nup\n" >/etc/hostname.tun$i; done
* for i in 0 1 2 3; do sh /etc/netstart tun$i; done
Now things should look like this:
* ls -l /dev/tun0
crw-rw 1 root tunnel 40, 0 Sep 18 14:52 /dev/tun0
* ifconfig tun0
tun0: flags=9803 mtu 1500
lladdr fe:e1:ba:d1:6b:94
priority: 0
groups: tun
status: no carrier
Last, config sshd to allow tunnels:
* echo "PermitTunnel ethernet" >>/etc/ssh/sshd_config
* kill -HUP `cat /var/run/sshd.pid`
And connect from client (client needs permissions, check yours):
* ssh -w any -o Tunnel=ethernet test@192.168.1.134
Is this patch the correct way of achieving what's described above?
Thanks,
Ossi Herrala
Index: usr.bin/ssh/misc.c
===
RCS file: /cvs/src/usr.bin/ssh/misc.c,v
retrieving revision 1.97
diff -u -p -r1.97 misc.c
--- usr.bin/ssh/misc.c 24 Apr 2015 01:36:00 - 1.97
+++ usr.bin/ssh/misc.c 18 Sep 2015 14:08:27 -
@@ -664,19 +664,21 @@ tun_open(int tun, int mode)
if (ioctl(sock, SIOCGIFFLAGS, &ifr) == -1)
goto failed;
- /* Set interface mode */
- ifr.ifr_flags &= ~IFF_UP;
- if (mode == SSH_TUNMODE_ETHERNET)
- ifr.ifr_flags |= IFF_LINK0;
- else
- ifr.ifr_flags &= ~IFF_LINK0;
- if (ioctl(sock, SIOCSIFFLAGS, &ifr) == -1)
- goto failed;
+ if (((mode == SSH_TUNMODE_ETHERNET) && !(ifr.ifr_flags & IFF_LINK0)) ||
+ ((mode != SSH_TUNMODE_ETHERNET) && ifr.ifr_flags & IFF_LINK0)) {
+ /* Set interface mode */
+ ifr.ifr_flags &= ~IFF_UP;
+ ifr.ifr_flags ^= IFF_LINK0;
+ if (ioctl(sock, SIOCSIFFLAGS, &ifr) == -1)
+ goto failed;
+ }
- /* Bring interface up */
- ifr.ifr_flags |= IFF_UP;
- if (ioctl(sock, SIOCSIFFLAGS, &ifr) == -1)
- goto failed;
+ if (!(ifr.ifr_flags & IFF_UP)) {
+ /* Bring interface up */
+ ifr.ifr_flags |= IFF_UP;
+ if (ioctl(sock, SIOCSIFFLAGS, &ifr) == -1)
+ goto failed;
+ }
close(sock);
return (fd);
Use M_ZERO in malloc(9)
Index: arch/arm/xscale/pxa27x_udc.c
===
RCS file: /cvs/src/sys/arch/arm/xscale/pxa27x_udc.c,v
retrieving revision 1.31
diff -u -p -r1.31 pxa27x_udc.c
--- arch/arm/xscale/pxa27x_udc.c15 May 2015 13:32:08 - 1.31
+++ arch/arm/xscale/pxa27x_udc.c18 Sep 2015 15:40:49 -
@@ -973,9 +973,7 @@ pxaudc_allocx(struct usbf_bus *bus)
if (xfer != NULL)
SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next);
else
- xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT);
- if (xfer != NULL)
- bzero(xfer, sizeof(struct pxaudc_xfer));
+ xfer = malloc(sizeof(struct pxaudc_xfer), M_USB, M_NOWAIT |
M_ZERO);
return xfer;
}
Index: isofs/cd9660/cd9660_vfsops.c
===
RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vfsops.c,v
retrieving revision 1.71
diff -u -p -r1.71 cd9660_vfsops.c
--- isofs/cd9660/cd9660_vfsops.c9 Sep 2015 15:59:19 - 1.71
+++ isofs/cd9660/cd9660_vfsops.c18 Sep 2015 15:40:50 -
@@ -361,8 +361,7 @@ iso_mountfs(devvp, mp, p, argp)
rootp = (struct iso_directory_record *)pri->root_directory_record;
- isomp = malloc(sizeof *isomp, M_ISOFSMNT, M_WAITOK);
- bzero((caddr_t)isomp, sizeof *isomp);
+ isomp = malloc(sizeof *isomp, M_ISOFSMNT, M_WAITOK | M_ZERO);
isomp->logical_block_size = logical_block_size;
isomp->volume_space_size = isonum_733 (pri->volume_space_size);
bcopy (rootp, isomp->root, sizeof isomp->root);
Index: kern/sys_process.c
===
RCS file: /cvs/src/sys/kern/sys_process.c,v
retrieving revision 1.67
diff -u -p -r1.67 sys_process.c
--- kern/sys_process.c 20 Jan 2015 19:43:21 - 1.67
+++ kern/sys_process.c 18 Sep 2015 15:40:50 -
@@ -346,10 +346,10 @@ sys_ptrace(struct proc *p, void *v, regi
/* Just set the trace flag. */
atomic_setbits_int(&tr->ps_flags, PS_TRACED);
tr->ps_oppid = tr->ps_pptr->ps_pid;
- if (tr->ps_ptstat == NULL)
+ if (tr->ps_ptstat == NULL) {
tr->ps_ptstat = malloc(sizeof(*tr->ps_ptstat),
- M_SUBPROC, M_WAITOK);
- memset(tr->ps_ptstat, 0, sizeof(*tr->ps_ptstat));
+ M_SUBPROC, M_WAITOK | M_ZERO);
+ }
return (0);
case PT_WRITE_I: /* XXX no separate I and D spaces */
Re: rtsock diff
On 2015/09/18 09:05, David Hill wrote:
> Hello -
>
> I believe the wrong var is being free'd. While here, add the size.
>
> Index: sys/net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.170
> diff -u -p -r1.170 rtsock.c
> --- sys/net/rtsock.c 11 Sep 2015 16:58:00 - 1.170
> +++ sys/net/rtsock.c 18 Sep 2015 13:04:34 -
> @@ -166,7 +166,7 @@ route_usrreq(struct socket *so, int req,
> else
> error = raw_attach(so, (int)(long)nam);
> if (error) {
> - free(rp, M_PCB, 0);
> + free(rop, M_PCB, sizeof(struct routecb));
> splx(s);
> return (error);
> }
>
I agree, I think this was missed in r1.110
Re: kill struct inpcbtable's inpt_lastport
On Sun, Sep 13, 2015 at 11:49:45AM +0200, Vincent Gross wrote:
> On 09/13/15 10:37, Claudio Jeker wrote:
> > On Sun, Sep 13, 2015 at 12:18:10AM +0200, Vincent Gross wrote:
> >> On 09/12/15 22:10, Claudio Jeker wrote:
> >>> On Sat, Sep 12, 2015 at 02:40:59PM +0200, Vincent Gross wrote:
> inpt_lastport is never read without being written before, and only
> in_pcbbind()
> and in6_pcbsetport() are using it. This diff removes inpt_lastport from
> struct inpcbtable and turns it into a local variable where it is used.
>
> Ok ?
> >>> Reads OK but can not be applied because something wrapped some lines.
> >>
> >
> > Lines are now fixed but now all the tabs got replaced by spaces. So the
> > thing still fails to apply.
> >
>
> How about now ?
>
>
> Index: sys/netinet/in_pcb.c
> ===
> RCS file: /cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.179
> diff -u -p -r1.179 in_pcb.c
> --- sys/netinet/in_pcb.c 11 Sep 2015 15:29:47 - 1.179
> +++ sys/netinet/in_pcb.c 12 Sep 2015 12:22:03 -
> @@ -199,7 +199,6 @@ in_pcbinit(struct inpcbtable *table, int
> &table->inpt_lhash);
> if (table->inpt_lhashtbl == NULL)
> panic("in_pcbinit: hashinit failed for lport");
> - table->inpt_lastport = 0;
> table->inpt_count = 0;
> arc4random_buf(&table->inpt_key, sizeof(table->inpt_key));
> }
> @@ -281,9 +280,8 @@ in_pcbbind(struct inpcb *inp, struct mbu
> {
> struct socket *so = inp->inp_socket;
> struct inpcbtable *table = inp->inp_table;
> - u_int16_t *lastport = &inp->inp_table->inpt_lastport;
> struct sockaddr_in *sin;
> - u_int16_t lport = 0;
> + u_int16_t lastport, lport = 0;
> int wild = 0, reuseport = (so->so_options & SO_REUSEPORT);
> int error;
>
> @@ -391,16 +389,16 @@ in_pcbbind(struct inpcb *inp, struct mbu
>*/
> count = first - last;
> if (count)
Is this 'if (count)' statement needed? We know first > last, so count
will always be positive. lastport will always be set.
> - *lastport = first - arc4random_uniform(count);
> + lastport = first - arc4random_uniform(count);
>
> do {
> if (count-- < 0)/* completely used? */
> return (EADDRNOTAVAIL);
> - --*lastport;
> - if (*lastport > first || *lastport < last)
> - *lastport = first;
> - lport = htons(*lastport);
> - } while (in_baddynamic(*lastport,
> so->so_proto->pr_protocol) ||
> + --lastport;
> + if (lastport > first || lastport < last)
> + lastport = first;
> + lport = htons(lastport);
> + } while (in_baddynamic(lastport,
> so->so_proto->pr_protocol) ||
> in_pcblookup(table, &zeroin_addr, 0,
> &inp->inp_laddr, lport, wild, inp->inp_rtableid));
> } else {
> @@ -409,16 +407,16 @@ in_pcbbind(struct inpcb *inp, struct mbu
>*/
> count = last - first;
> if (count)
if last == first, then the if statement will be false and lastport will
be uninitialized, I believe.
> - *lastport = first + arc4random_uniform(count);
> + lastport = first + arc4random_uniform(count);
>
> do {
> if (count-- < 0)/* completely used? */
> return (EADDRNOTAVAIL);
> - ++*lastport;
> - if (*lastport < first || *lastport > last)
> - *lastport = first;
> - lport = htons(*lastport);
> - } while (in_baddynamic(*lastport,
> so->so_proto->pr_protocol) ||
> + ++lastport;
> + if (lastport < first || lastport > last)
> + lastport = first;
> + lport = htons(lastport);
> + } while (in_baddynamic(lastport,
> so->so_proto->pr_protocol) ||
> in_pcblookup(table, &zeroin_addr, 0,
> &inp->inp_laddr, lport, wild, inp->inp_rtableid));
> }
> Index: sys/netinet/in_pcb.h
> ===
> RCS file: /cvs/src/sys/netinet/in_pcb.h,v
> retrieving revision 1.89
> diff -u -p -r1.89 in_pcb.h
> --- sys/netinet/in_pcb.h 16 Apr 2015 19:24:13 -
Re: kill struct inpcbtable's inpt_lastport
On Fri, Sep 18, 2015 at 02:58:40PM +0200, Vincent Gross wrote:
> On 09/13/15 11:49, Vincent Gross wrote:
> > On 09/13/15 10:37, Claudio Jeker wrote:
> >> On Sun, Sep 13, 2015 at 12:18:10AM +0200, Vincent Gross wrote:
> >>> On 09/12/15 22:10, Claudio Jeker wrote:
> On Sat, Sep 12, 2015 at 02:40:59PM +0200, Vincent Gross wrote:
> > inpt_lastport is never read without being written before, and only
> > in_pcbbind()
> > and in6_pcbsetport() are using it. This diff removes inpt_lastport from
> > struct inpcbtable and turns it into a local variable where it is used.
> >
> > Ok ?
> Reads OK but can not be applied because something wrapped some lines.
> >>>
> >>
> >> Lines are now fixed but now all the tabs got replaced by spaces. So the
> >> thing still fails to apply.
> >>
> >
> > How about now ?
> >
> >
> > Index: sys/netinet/in_pcb.c
> > ===
> > RCS file: /cvs/src/sys/netinet/in_pcb.c,v
> > retrieving revision 1.179
> > diff -u -p -r1.179 in_pcb.c
> > --- sys/netinet/in_pcb.c11 Sep 2015 15:29:47 - 1.179
> > +++ sys/netinet/in_pcb.c12 Sep 2015 12:22:03 -
> > @@ -199,7 +199,6 @@ in_pcbinit(struct inpcbtable *table, int
> > &table->inpt_lhash);
> > if (table->inpt_lhashtbl == NULL)
> > panic("in_pcbinit: hashinit failed for lport");
> > - table->inpt_lastport = 0;
> > table->inpt_count = 0;
> > arc4random_buf(&table->inpt_key, sizeof(table->inpt_key));
> > }
> > @@ -281,9 +280,8 @@ in_pcbbind(struct inpcb *inp, struct mbu
> > {
> > struct socket *so = inp->inp_socket;
> > struct inpcbtable *table = inp->inp_table;
> > - u_int16_t *lastport = &inp->inp_table->inpt_lastport;
> > struct sockaddr_in *sin;
> > - u_int16_t lport = 0;
> > + u_int16_t lastport, lport = 0;
> > int wild = 0, reuseport = (so->so_options & SO_REUSEPORT);
> > int error;
> >
> > @@ -391,16 +389,16 @@ in_pcbbind(struct inpcb *inp, struct mbu
> > */
> > count = first - last;
> > if (count)
> > - *lastport = first - arc4random_uniform(count);
> > + lastport = first - arc4random_uniform(count);
> >
> > do {
> > if (count-- < 0)/* completely used? */
> > return (EADDRNOTAVAIL);
> > - --*lastport;
> > - if (*lastport > first || *lastport < last)
> > - *lastport = first;
> > - lport = htons(*lastport);
> > - } while (in_baddynamic(*lastport,
> > so->so_proto->pr_protocol) ||
> > + --lastport;
> > + if (lastport > first || lastport < last)
> > + lastport = first;
> > + lport = htons(lastport);
> > + } while (in_baddynamic(lastport,
> > so->so_proto->pr_protocol) ||
> > in_pcblookup(table, &zeroin_addr, 0,
> > &inp->inp_laddr, lport, wild, inp->inp_rtableid));
> > } else {
> > @@ -409,16 +407,16 @@ in_pcbbind(struct inpcb *inp, struct mbu
> > */
> > count = last - first;
> > if (count)
> > - *lastport = first + arc4random_uniform(count);
> > + lastport = first + arc4random_uniform(count);
> >
> > do {
> > if (count-- < 0)/* completely used? */
> > return (EADDRNOTAVAIL);
> > - ++*lastport;
> > - if (*lastport < first || *lastport > last)
> > - *lastport = first;
> > - lport = htons(*lastport);
> > - } while (in_baddynamic(*lastport,
> > so->so_proto->pr_protocol) ||
> > + ++lastport;
> > + if (lastport < first || lastport > last)
> > + lastport = first;
> > + lport = htons(lastport);
> > + } while (in_baddynamic(lastport,
> > so->so_proto->pr_protocol) ||
> > in_pcblookup(table, &zeroin_addr, 0,
> > &inp->inp_laddr, lport, wild, inp->inp_rtableid));
> > }
> > Index: sys/netinet/in_pcb.h
> > ===
> > RCS file: /cvs/src/sys/netinet/in_pcb.h,v
> > retrieving revision 1.89
> > diff -u -p -r1.89 in_pcb.h
> > --- sys/netinet/in_pcb.h16 Apr 2015 19:24:13 - 1.89
> > +++ sys/netinet/in_pcb.h12 Sep 2015 12:22:03 -
> > @@ -152,7 +152,6 @@ struct inpcbtable {
>
rtsock diff
Hello -
I believe the wrong var is being free'd. While here, add the size.
Index: sys/net/rtsock.c
===
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.170
diff -u -p -r1.170 rtsock.c
--- sys/net/rtsock.c11 Sep 2015 16:58:00 - 1.170
+++ sys/net/rtsock.c18 Sep 2015 13:04:34 -
@@ -166,7 +166,7 @@ route_usrreq(struct socket *so, int req,
else
error = raw_attach(so, (int)(long)nam);
if (error) {
- free(rp, M_PCB, 0);
+ free(rop, M_PCB, sizeof(struct routecb));
splx(s);
return (error);
}
Re: [patch]rcs: mark unlink as (void)
On Mon, Jun 15, 2015 at 09:56:18PM +0200, Fritjof Bornebusch wrote: > Hi tech@, > > mark this unlink(2) call as *(void)*, as there is no need to check the return > value. > This makes it more consistent to all other unlink(2) calls, since they are > marked as *(void)* as > well. > > Regards, > --F. > Ping > > Index: co.c > === > RCS file: /cvs/src/usr.bin/rcs/co.c,v > retrieving revision 1.121 > diff -u -p -r1.121 co.c > --- co.c 13 Jun 2015 20:15:21 - 1.121 > +++ co.c 15 Jun 2015 19:50:12 - > @@ -553,7 +553,7 @@ checkout_file_has_diffs(RCSFILE *rfp, RC > ret = diffreg(dst, tempfile, bp, D_FORCEASCII); > > buf_free(bp); > - unlink(tempfile); > + (void)unlink(tempfile); > free(tempfile); > > return (ret); >
Re: [patch]rcs: usage functions above the main ones
On Mon, Jun 15, 2015 at 11:42:10AM +0100, Nicholas Marriott wrote:
>
> this seems fine to me
>
Ping ...
>
> On Sun, Jun 14, 2015 at 10:38:40PM +0200, Fritjof Bornebusch wrote:
> > Hi tech@,
> >
> > most of the tools implements the *usage* function above the *main* function.
> > This patch makes it more consistent to these tools and where the different
> > *usage*
> > functions are implemented in rcs in general.
> >
> > Any comments?
> >
> > Regards,
> > --F.
> >
> >
> > Index: co.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/co.c,v
> > retrieving revision 1.121
> > diff -u -p -r1.121 co.c
> > --- co.c13 Jun 2015 20:15:21 - 1.121
> > +++ co.c14 Jun 2015 20:21:41 -
> > @@ -43,6 +43,17 @@ static void checkout_err_nobranch(RCSFIL
> > const char *, int);
> > static int checkout_file_has_diffs(RCSFILE *, RCSNUM *, const char *);
> >
> > +__dead void
> > +checkout_usage(void)
> > +{
> > + fprintf(stderr,
> > + "usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n"
> > + " [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n"
> > + " [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n");
> > +
> > + exit(1);
> > +}
> > +
> > int
> > checkout_main(int argc, char **argv)
> > {
> > @@ -216,17 +227,6 @@ checkout_main(int argc, char **argv)
> > }
> >
> > return (ret);
> > -}
> > -
> > -__dead void
> > -checkout_usage(void)
> > -{
> > - fprintf(stderr,
> > - "usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n"
> > - " [-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n"
> > - " [-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n");
> > -
> > - exit(1);
> > }
> >
> > /*
> > Index: ident.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/ident.c,v
> > retrieving revision 1.30
> > diff -u -p -r1.30 ident.c
> > --- ident.c 2 Oct 2014 06:23:15 - 1.30
> > +++ ident.c 14 Jun 2015 20:21:41 -
> > @@ -41,6 +41,14 @@ static int flags = 0;
> > static voidident_file(const char *, FILE *);
> > static voidident_line(FILE *);
> >
> > +__dead void
> > +ident_usage(void)
> > +{
> > + fprintf(stderr, "usage: ident [-qV] [file ...]\n");
> > +
> > + exit(1);
> > +}
> > +
> > int
> > ident_main(int argc, char **argv)
> > {
> > @@ -158,12 +166,4 @@ ident_line(FILE *fp)
> > out:
> > if (bp != NULL)
> > buf_free(bp);
> > -}
> > -
> > -__dead void
> > -ident_usage(void)
> > -{
> > - fprintf(stderr, "usage: ident [-qV] [file ...]\n");
> > -
> > - exit(1);
> > }
> > Index: merge.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/merge.c,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 merge.c
> > --- merge.c 10 Oct 2014 08:15:25 - 1.9
> > +++ merge.c 14 Jun 2015 20:21:41 -
> > @@ -32,6 +32,15 @@
> > #include "rcsprog.h"
> > #include "diff.h"
> >
> > +__dead void
> > +merge_usage(void)
> > +{
> > + fprintf(stderr,
> > + "usage: merge [-EepqV] [-L label] file1 file2 file3\n");
> > +
> > + exit(D_ERROR);
> > +}
> > +
> > int
> > merge_main(int argc, char **argv)
> > {
> > @@ -108,13 +117,4 @@ merge_main(int argc, char **argv)
> > buf_free(bp);
> >
> > return (status);
> > -}
> > -
> > -__dead void
> > -merge_usage(void)
> > -{
> > - (void)fprintf(stderr,
> > - "usage: merge [-EepqV] [-L label] file1 file2 file3\n");
> > -
> > - exit(D_ERROR);
> > }
> > Index: rcsclean.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
> > retrieving revision 1.54
> > diff -u -p -r1.54 rcsclean.c
> > --- rcsclean.c 16 Jan 2015 06:40:11 - 1.54
> > +++ rcsclean.c 14 Jun 2015 20:21:41 -
> > @@ -43,6 +43,16 @@ static int uflag = 0;
> > static int flags = 0;
> > static char *locker = NULL;
> >
> > +__dead void
> > +rcsclean_usage(void)
> > +{
> > + fprintf(stderr,
> > + "usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n"
> > + "[-u[rev]] [-xsuffixes] [-ztz] [file ...]\n");
> > +
> > + exit(1);
> > +}
> > +
> > int
> > rcsclean_main(int argc, char **argv)
> > {
> > @@ -116,16 +126,6 @@ rcsclean_main(int argc, char **argv)
> > rcsclean_file(argv[i], rev_str);
> >
> > return (0);
> > -}
> > -
> > -__dead void
> > -rcsclean_usage(void)
> > -{
> > - fprintf(stderr,
> > - "usage: rcsclean [-TV] [-kmode] [-n[rev]] [-q[rev]] [-r[rev]]\n"
> > - "[-u[rev]] [-xsuffixes] [-ztz] [file ...]\n");
> > -
> > - exit(1);
> > }
> >
> > static void
> > Index: rcsdiff.c
> > ===
> > RCS file: /cvs/src/usr.bin/rcs/rcsdiff.c,v
> > retrieving revision 1.83
> > diff
Re: [patch]diff: uninitialized values
On Wed, Jun 17, 2015 at 09:19:28PM +0200, Fritjof Bornebusch wrote:
> On Wed, Jun 17, 2015 at 08:53:57PM +0200, Fritjof Bornebusch wrote:
> > Hi tech@,
> >
> > *edp1* and *edp2* could be used uninitialized, if *goto closem;* is called.
> >
>
> Such initializers hiding a false positive, cause the compiler does not
> understand this case can never happen.
> -> warning: 'edp1' may be used uninitialized in this function
> -> warning: 'edp2' may be used uninitialized in this function
>
> Sorry for beeing not that clear.
>
Ping
> > Regards,
> > --F.
> >
> >
> > Index: diffdir.c
> > ===
> > RCS file: /cvs/src/usr.bin/diff/diffdir.c,v
> > retrieving revision 1.43
> > diff -u -p -r1.43 diffdir.c
> > --- diffdir.c 16 Jan 2015 06:40:07 - 1.43
> > +++ diffdir.c 17 Jun 2015 18:50:57 -
> > @@ -48,8 +48,8 @@ static void diffit(struct dirent *, char
> > void
> > diffdir(char *p1, char *p2, int flags)
> > {
> > - struct dirent *dent1, **dp1, **edp1, **dirp1 = NULL;
> > - struct dirent *dent2, **dp2, **edp2, **dirp2 = NULL;
> > + struct dirent *dent1, **dp1, **edp1 = NULL, **dirp1 = NULL;
> > + struct dirent *dent2, **dp2, **edp2 = NULL, **dirp2 = NULL;
> > size_t dirlen1, dirlen2;
> > char path1[PATH_MAX], path2[PATH_MAX];
> > int pos;
> >
>
Re: kill struct inpcbtable's inpt_lastport
On 09/13/15 11:49, Vincent Gross wrote:
> On 09/13/15 10:37, Claudio Jeker wrote:
>> On Sun, Sep 13, 2015 at 12:18:10AM +0200, Vincent Gross wrote:
>>> On 09/12/15 22:10, Claudio Jeker wrote:
On Sat, Sep 12, 2015 at 02:40:59PM +0200, Vincent Gross wrote:
> inpt_lastport is never read without being written before, and only
> in_pcbbind()
> and in6_pcbsetport() are using it. This diff removes inpt_lastport from
> struct inpcbtable and turns it into a local variable where it is used.
>
> Ok ?
Reads OK but can not be applied because something wrapped some lines.
>>>
>>
>> Lines are now fixed but now all the tabs got replaced by spaces. So the
>> thing still fails to apply.
>>
>
> How about now ?
>
>
> Index: sys/netinet/in_pcb.c
> ===
> RCS file: /cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.179
> diff -u -p -r1.179 in_pcb.c
> --- sys/netinet/in_pcb.c 11 Sep 2015 15:29:47 - 1.179
> +++ sys/netinet/in_pcb.c 12 Sep 2015 12:22:03 -
> @@ -199,7 +199,6 @@ in_pcbinit(struct inpcbtable *table, int
> &table->inpt_lhash);
> if (table->inpt_lhashtbl == NULL)
> panic("in_pcbinit: hashinit failed for lport");
> - table->inpt_lastport = 0;
> table->inpt_count = 0;
> arc4random_buf(&table->inpt_key, sizeof(table->inpt_key));
> }
> @@ -281,9 +280,8 @@ in_pcbbind(struct inpcb *inp, struct mbu
> {
> struct socket *so = inp->inp_socket;
> struct inpcbtable *table = inp->inp_table;
> - u_int16_t *lastport = &inp->inp_table->inpt_lastport;
> struct sockaddr_in *sin;
> - u_int16_t lport = 0;
> + u_int16_t lastport, lport = 0;
> int wild = 0, reuseport = (so->so_options & SO_REUSEPORT);
> int error;
>
> @@ -391,16 +389,16 @@ in_pcbbind(struct inpcb *inp, struct mbu
>*/
> count = first - last;
> if (count)
> - *lastport = first - arc4random_uniform(count);
> + lastport = first - arc4random_uniform(count);
>
> do {
> if (count-- < 0)/* completely used? */
> return (EADDRNOTAVAIL);
> - --*lastport;
> - if (*lastport > first || *lastport < last)
> - *lastport = first;
> - lport = htons(*lastport);
> - } while (in_baddynamic(*lastport,
> so->so_proto->pr_protocol) ||
> + --lastport;
> + if (lastport > first || lastport < last)
> + lastport = first;
> + lport = htons(lastport);
> + } while (in_baddynamic(lastport,
> so->so_proto->pr_protocol) ||
> in_pcblookup(table, &zeroin_addr, 0,
> &inp->inp_laddr, lport, wild, inp->inp_rtableid));
> } else {
> @@ -409,16 +407,16 @@ in_pcbbind(struct inpcb *inp, struct mbu
>*/
> count = last - first;
> if (count)
> - *lastport = first + arc4random_uniform(count);
> + lastport = first + arc4random_uniform(count);
>
> do {
> if (count-- < 0)/* completely used? */
> return (EADDRNOTAVAIL);
> - ++*lastport;
> - if (*lastport < first || *lastport > last)
> - *lastport = first;
> - lport = htons(*lastport);
> - } while (in_baddynamic(*lastport,
> so->so_proto->pr_protocol) ||
> + ++lastport;
> + if (lastport < first || lastport > last)
> + lastport = first;
> + lport = htons(lastport);
> + } while (in_baddynamic(lastport,
> so->so_proto->pr_protocol) ||
> in_pcblookup(table, &zeroin_addr, 0,
> &inp->inp_laddr, lport, wild, inp->inp_rtableid));
> }
> Index: sys/netinet/in_pcb.h
> ===
> RCS file: /cvs/src/sys/netinet/in_pcb.h,v
> retrieving revision 1.89
> diff -u -p -r1.89 in_pcb.h
> --- sys/netinet/in_pcb.h 16 Apr 2015 19:24:13 - 1.89
> +++ sys/netinet/in_pcb.h 12 Sep 2015 12:22:03 -
> @@ -152,7 +152,6 @@ struct inpcbtable {
> struct inpcbhead *inpt_hashtbl, *inpt_lhashtbl;
> SIPHASH_KEY inpt_key;
> u_longinpt_hash, inpt_lhash;
> - u_int16_t inpt_lastp
Re: reuse pf state ids to "hash" packets onto trunk members
On 2015/09/18 13:36, Martin Pieuchot wrote: > On 18/09/15(Fri) 15:55, David Gwynne wrote: > > hashing bits of packet headers to tie connections to particular > > physical interfaces within a trunk turns out to be fairly expensive. > > in my very unscientific testing it is about 20% of the cost of udp > > traffic generated with tcpbench -u. > > > > we could tune or change the hash. eg, going from siphash 2 4 to > > siphash 1 1 halves the overhead of hashing. however, it occurred > > to me that sometimes we already know about connections. why not > > reuse that info if it is available? > > Why not, but I'd argue that's orthogonal to the fact that siphash > 2 4 has a high cost. > > > this lets pf embed the state id into the mbuf as a "flow id" so > > other subsystems can use it. eg, trunk can pull it out and use it. > > > > this diff steals the pad field in mbuf packet headers and uses it > > to embed a flow id. it makes pf fill it in, and trunk use it. this > > avoids the cost of hashing in trunk altogether. > > > > it could be used in other places too, eg, picking an upstream when > > we're going multipath routing. > > I've been through RFC 2992 again and indeed I believe we could use that. as far as trunk(4) goes, we're ok from the perspective of 802.3-2000 section 43.2.1 says f)Frame ordering must be maintained for certain sequences of frame exchanges between MAC Clients (known as conversations, see 1.4). The Distributor ensures that all frames of a given conversation are passed to a single port. For any given port, the Collector is required to pass frames to the MAC Client in the order that they are received from that port. The Collector is otherwise free to select frames received from the aggregated ports in any order. Since there are no means for frames to be mis-ordered on a single link, this guarantees that frame ordering is maintained for any conversation. so we're OK from that perspective. > What about carp(4) and bridge(4)? I don't think it applies to bridge, load balancing is done at a lower level there (i.e. you'd have trunk as a member of a bridge if you wanted to balance across links). Probably the same for carp, there might be some opportunity, but it's already a bit of a minefield to have things working nicely with pfsync/defer in various different situations.
Re: reuse pf state ids to "hash" packets onto trunk members
On 18/09/15(Fri) 15:55, David Gwynne wrote:
> hashing bits of packet headers to tie connections to particular
> physical interfaces within a trunk turns out to be fairly expensive.
> in my very unscientific testing it is about 20% of the cost of udp
> traffic generated with tcpbench -u.
>
> we could tune or change the hash. eg, going from siphash 2 4 to
> siphash 1 1 halves the overhead of hashing. however, it occurred
> to me that sometimes we already know about connections. why not
> reuse that info if it is available?
Why not, but I'd argue that's orthogonal to the fact that siphash
2 4 has a high cost.
> this lets pf embed the state id into the mbuf as a "flow id" so
> other subsystems can use it. eg, trunk can pull it out and use it.
>
> this diff steals the pad field in mbuf packet headers and uses it
> to embed a flow id. it makes pf fill it in, and trunk use it. this
> avoids the cost of hashing in trunk altogether.
>
> it could be used in other places too, eg, picking an upstream when
> we're going multipath routing.
I've been through RFC 2992 again and indeed I believe we could use that.
What about carp(4) and bridge(4)?
>
> thoughts?
>
> Index: sys/mbuf.h
> ===
> RCS file: /cvs/src/sys/sys/mbuf.h,v
> retrieving revision 1.196
> diff -u -p -r1.196 mbuf.h
> --- sys/mbuf.h14 Aug 2015 05:25:29 - 1.196
> +++ sys/mbuf.h18 Sep 2015 05:47:30 -
> @@ -125,7 +125,7 @@ structpkthdr {
> SLIST_HEAD(packet_tags, m_tag) tags;/* list of packet tags */
> int len; /* total packet length */
> u_int16_ttagsset; /* mtags attached */
> - u_int16_tpad;
> + u_int16_tflowid;/* pseudo unique flow id */
> u_int16_tcsum_flags;/* checksum flags */
> u_int16_tether_vtag;/* Ethernet 802.1p+Q vlan tag */
> u_intph_rtableid; /* routing table id */
> @@ -236,6 +236,10 @@ struct mbuf {
> #define MT_FTABLE 5 /* fragment reassembly header */
> #define MT_CONTROL 6 /* extra-data protocol message */
> #define MT_OOBDATA 7 /* expedited data */
> +
> +/* flowid field */
> +#define M_FLOWID_VALID 0x8000 /* is the flowid set */
> +#define M_FLOWID_MASK0x7fff /* flow id to map to path */
>
> /* flags to m_get/MGET */
> #define M_DONTWAIT M_NOWAIT
> Index: net/pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.944
> diff -u -p -r1.944 pf.c
> --- net/pf.c 13 Sep 2015 17:53:44 - 1.944
> +++ net/pf.c 18 Sep 2015 05:47:30 -
> @@ -6531,11 +6531,15 @@ done:
> pd.m->m_pkthdr.pf.qid = qid;
> if (pd.dir == PF_IN && s && s->key[PF_SK_STACK])
> pd.m->m_pkthdr.pf.statekey = s->key[PF_SK_STACK];
> - if (pd.dir == PF_OUT &&
> - pd.m->m_pkthdr.pf.inp && !pd.m->m_pkthdr.pf.inp->inp_pf_sk &&
> - s && s->key[PF_SK_STACK] && !s->key[PF_SK_STACK]->inp) {
> - pd.m->m_pkthdr.pf.inp->inp_pf_sk = s->key[PF_SK_STACK];
> - s->key[PF_SK_STACK]->inp = pd.m->m_pkthdr.pf.inp;
> + if (pd.dir == PF_OUT && s) {
> + if (pd.m->m_pkthdr.pf.inp &&
> + !pd.m->m_pkthdr.pf.inp->inp_pf_sk &&
> + s->key[PF_SK_STACK] && !s->key[PF_SK_STACK]->inp) {
> + pd.m->m_pkthdr.pf.inp->inp_pf_sk = s->key[PF_SK_STACK];
> + s->key[PF_SK_STACK]->inp = pd.m->m_pkthdr.pf.inp;
> + }
> + pd.m->m_pkthdr.flowid = M_FLOWID_VALID |
> + (M_FLOWID_MASK & bemtoh64(&s->id));
> }
>
> /*
> Index: net/if_trunk.c
> ===
> RCS file: /cvs/src/sys/net/if_trunk.c,v
> retrieving revision 1.111
> diff -u -p -r1.111 if_trunk.c
> --- net/if_trunk.c10 Sep 2015 16:41:30 - 1.111
> +++ net/if_trunk.c18 Sep 2015 05:47:30 -
> @@ -985,6 +990,9 @@ trunk_hashmbuf(struct mbuf *m, SIPHASH_K
> #endif
> SIPHASH_CTX ctx;
>
> + if (m->m_pkthdr.flowid & M_FLOWID_VALID)
> + return (m->m_pkthdr.flowid & M_FLOWID_MASK);
> +
> SipHash24_Init(&ctx, key);
> off = sizeof(*eh);
> if (m->m_len < off)
>
Re: Brainy: a few bugs
On Fri, Sep 11, 2015 at 09:18:18PM +0200, Maxime Villard wrote: > _20/ UNINITIALIZED VARIABLE: sys/arch/sgi/dev/if_iec.c rev1.14 Fixed. Thank you.
Re: doas closefrom
On 18/09/15(Fri) 12:02, Sebastian Benoit wrote: > Ted Unangst(t...@tedunangst.com) on 2015.09.17 21:12:28 -0400: > > Sebastian Benoit wrote: > > > ok, but in other places we have closefrom(STDERR_FILENO + 1) > > > > is that really more clear? it only makes sense if you know stderr is 2. What's point of being different than the rest of the tree? :) I don't see how closefrom(3) is clearer than closefrom(STDERR_FILENO + 1) either.
Re: reuse pf state ids to "hash" packets onto trunk members
On 2015/09/18 20:18, David Gwynne wrote: > > > On 18 Sep 2015, at 6:17 pm, Stuart Henderson wrote: > > > > On 2015-09-18, David Gwynne wrote: > >> this lets pf embed the state id into the mbuf as a "flow id" so > >> other subsystems can use it. eg, trunk can pull it out and use it. > > > > I like this but it does change the path distribution. Previously all > > flows from host A to host B were bound to a single path, now they are > > spread across paths. This is good for making fuller use of paths but can > > make fault diagnosis harder. > > > > I tried to work out the best way to make this optional when I sent my > > earlier L4-hash diff (using PF states for this has similar results > > and is far more elegant) but didn't settle on an approach. Switches > > doing this usually have a single global setting (e.g. sysctl), which > > seems a bit of a blunt instrument but would be easier to apply to areas > > other than trunk (e.g. multipath routing). We could use an ioctl if > > ifconfig(8) isn't full already, though I don't think we actually need > > any more options than "L3-bound" and "per-flow" so using the existing > > link0 scaffolding would be an easier way to do this per-trunk but > > I didn't really get a feel for whether people thought that was > > good enough. > > the only "fault" ive experienced with hashing algorithms is that they all > tend to make the wrong decision, so fixing it has been rotating through their > different options till one sucks less than the others. > > is that what you're arguing for here? > > dlg I meant faults like bad fibre, media converter, switchport, metro ethernet circuit hanging off an interface, etc. In those cases it's useful to be able to make it go over a different path by switching source IP to track things down. Also sometimes you can get insight into a problem by noticing that connections from host X in a subnet to host A have problems, but from Y to A don't, though this more applies where the person noticing the problem isn't the person who is running the system doing the load balancing.
Re: reuse pf state ids to "hash" packets onto trunk members
> On 18 Sep 2015, at 6:17 pm, Stuart Henderson wrote: > > On 2015-09-18, David Gwynne wrote: >> this lets pf embed the state id into the mbuf as a "flow id" so >> other subsystems can use it. eg, trunk can pull it out and use it. > > I like this but it does change the path distribution. Previously all > flows from host A to host B were bound to a single path, now they are > spread across paths. This is good for making fuller use of paths but can > make fault diagnosis harder. > > I tried to work out the best way to make this optional when I sent my > earlier L4-hash diff (using PF states for this has similar results > and is far more elegant) but didn't settle on an approach. Switches > doing this usually have a single global setting (e.g. sysctl), which > seems a bit of a blunt instrument but would be easier to apply to areas > other than trunk (e.g. multipath routing). We could use an ioctl if > ifconfig(8) isn't full already, though I don't think we actually need > any more options than "L3-bound" and "per-flow" so using the existing > link0 scaffolding would be an easier way to do this per-trunk but > I didn't really get a feel for whether people thought that was > good enough. the only "fault" ive experienced with hashing algorithms is that they all tend to make the wrong decision, so fixing it has been rotating through their different options till one sucks less than the others. is that what you're arguing for here? dlg
libsa: explicit_bzero
Hi,
I noted that in libsa, explicit_bzero just calls bzero.
Giving that now we support softraid fulldisk encryption, a compiler
optimisation could make the current version of explicit_bzero in libsa
going nop, leaving encryption keys (or other sensible material) in
memory.
The following patch copy libkern code for explicit_bzero into libsa.
Comments ? OK ?
--
Sebastien Marie
Index: explicit_bzero.c
===
RCS file: /cvs/src/sys/lib/libsa/explicit_bzero.c,v
retrieving revision 1.1
diff -u -p -r1.1 explicit_bzero.c
--- explicit_bzero.c9 Oct 2012 12:03:51 - 1.1
+++ explicit_bzero.c18 Sep 2015 10:01:06 -
@@ -6,11 +6,19 @@
#include
+__attribute__((weak)) void __explicit_bzero_hook(void *, size_t);
+
+__attribute__((weak)) void
+__explicit_bzero_hook(void *buf, size_t len)
+{
+}
+
/*
* explicit_bzero - don't let the compiler optimize away bzero
*/
void
explicit_bzero(void *p, size_t n)
{
- bzero(p, n);
+ memset(p, 0, n);
+ __explicit_bzero_hook(p, n);
}
Re: doas closefrom
Ted Unangst(t...@tedunangst.com) on 2015.09.17 21:12:28 -0400: > Sebastian Benoit wrote: > > ok, but in other places we have closefrom(STDERR_FILENO + 1) > > is that really more clear? it only makes sense if you know stderr is 2. sure, but writing closefrom(3) requires the same or equivalent knowledge, no? > if you sometimes forget which is 1 and which is 2, then the macro only makes > it more confusing because now you have to decide what comes after stderr. is > it stdin? or stdout? if you sometimes forget whats 1 and 2 you have probably more problems than that ;-) anyway, i only wanted to point this out, in case someone thinks this is some matter of style. i'm fine with the 3.
Re: openvpn-2.3.8p1 segv in libcrypto BN_bn2dec on OpenBSD/i386 current Sep 16, 2015
> CC'ing tech@.
>
> The last commit to bn_print.c is wrong, it dereferences t while it's still
> NULL.
>
> Backout diff below.
Argh, sorry about that. This is how it should have been done (diff
against 1.25)
Index: bn_print.c
===
RCS file: /OpenBSD/src/lib/libssl/src/crypto/bn/bn_print.c,v
retrieving revision 1.25
diff -u -p -r1.25 bn_print.c
--- bn_print.c 13 Sep 2015 16:02:11 - 1.25
+++ bn_print.c 18 Sep 2015 09:06:42 -
@@ -114,14 +114,14 @@ BN_bn2dec(const BIGNUM *a)
BIGNUM *t = NULL;
BN_ULONG *bn_data = NULL, *lp;
- if (BN_is_zero(t)) {
- buf = malloc(BN_is_negative(t) + 2);
+ if (BN_is_zero(a)) {
+ buf = malloc(BN_is_negative(a) + 2);
if (buf == NULL) {
BNerr(BN_F_BN_BN2DEC, ERR_R_MALLOC_FAILURE);
goto err;
}
p = buf;
- if (BN_is_negative(t))
+ if (BN_is_negative(a))
*(p++) = '-';
*(p++) = '0';
*(p++) = '\0';
ipmi(4) - Support ipmitool IOCTL etc.
I've prepared a set of patches to make OpenBSD's ipmi(4) work with ipmitool via /dev/ipmi*, following what FreeBSD did. I'd like to hear feedback from developers and users. https://github.com/uebayasi/openbsd-ipmi I'm also looking for someone whose machine supports BT (block transfer) interface. I believe the BT support is broken. Need testers to fix it.
Re: mpsafe ip_carp
> On 13 Sep 2015, at 6:34 pm, David Gwynne wrote:
>
> i did this yesterday, but havent had a chance to beat on it properly
> yet.
>
> if anyone would like to give it a go it would be much appreciated.
> im particularly interested in stability while carp configuration
> is made or changed. if it happens to keep handling packets, thats
> great, but not blowing up when you run ifconfig is the important
> bit atm.
this diff depended on SRPL_INSTER_AFTER_LOCKED, which i thought i had committed
but just found still in my tree.
its now in, so this should apply and work now.
dlg
>
> Index: ip_carp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.271
> diff -u -p -r1.271 ip_carp.c
> --- ip_carp.c 12 Sep 2015 20:51:35 - 1.271
> +++ ip_carp.c 12 Sep 2015 21:07:42 -
> @@ -48,6 +48,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -93,7 +94,9 @@ struct carp_mc_entry {
> enum { HMAC_ORIG=0, HMAC_NOV6LL=1, HMAC_MAX=2 };
>
> struct carp_vhost_entry {
> - LIST_ENTRY(carp_vhost_entry)vhost_entries;
> + struct srpl_entry vhost_entries;
> + struct refcnt vhost_refcnt;
> +
> struct carp_softc *parent_sc;
> int vhe_leader;
> int vhid;
> @@ -114,6 +117,12 @@ struct carp_vhost_entry {
> struct sockaddr_dl vhe_sdl; /* for IPv6 ndp balancing */
> };
>
> +void carp_vh_ref(void *, void *);
> +void carp_vh_unref(void *, void *);
> +
> +struct srpl_rc carp_vh_rc =
> +SRPL_RC_INITIALIZER(carp_vh_ref, carp_vh_unref, NULL);
> +
> struct carp_softc {
> struct arpcom sc_ac;
> #define sc_if sc_ac.ac_if
> @@ -124,7 +133,9 @@ struct carp_softc {
> #ifdef INET6
> struct ip6_moptions sc_im6o;
> #endif /* INET6 */
> - TAILQ_ENTRY(carp_softc) sc_list;
> +
> + struct srpl_entry sc_list;
> + struct refcnt sc_refcnt;
>
> int sc_suppress;
> int sc_bow_out;
> @@ -137,7 +148,7 @@ struct carp_softc {
>
> char sc_curlladdr[ETHER_ADDR_LEN];
>
> - LIST_HEAD(__carp_vhosthead, carp_vhost_entry) carp_vhosts;
> + struct srpl carp_vhosts;
> int sc_vhe_count;
> u_int8_t sc_vhids[CARP_MAXNODES];
> u_int8_t sc_advskews[CARP_MAXNODES];
> @@ -162,13 +173,19 @@ struct carp_softc {
> struct carp_vhost_entry *cur_vhe; /* current active vhe */
> };
>
> +void carp_sc_ref(void *, void *);
> +void carp_sc_unref(void *, void *);
> +
> +struct srpl_rc carp_sc_rc =
> +SRPL_RC_INITIALIZER(carp_sc_ref, carp_sc_unref, NULL);
> +
> int carp_opts[CARPCTL_MAXID] = { 0, 1, 0, LOG_CRIT }; /* XXX for now */
> struct carpstats carpstats;
>
> int carp_send_all_recur = 0;
>
> struct carp_if {
> - TAILQ_HEAD(, carp_softc) vhif_vrs;
> + struct srpl vhif_vrs;
> };
>
> #define CARP_LOG(l, sc, s)
> \
> @@ -250,7 +267,9 @@ carp_hmac_prepare(struct carp_softc *sc)
> struct carp_vhost_entry *vhe;
> u_int8_t i;
>
> - LIST_FOREACH(vhe, &sc->carp_vhosts, vhost_entries) {
> + KERNEL_ASSERT_LOCKED(); /* touching carp_vhosts */
> +
> + SRPL_FOREACH_LOCKED(vhe, &sc->carp_vhosts, vhost_entries) {
> for (i = 0; i < HMAC_MAX; i++) {
> carp_hmac_prepare_ctx(vhe, i);
> }
> @@ -579,11 +598,12 @@ carp_proto_input_c(struct ifnet *ifp, st
> else
> cif = (struct carp_if *)ifp->if_carp;
>
> - TAILQ_FOREACH(sc, &cif->vhif_vrs, sc_list) {
> + KERNEL_ASSERT_LOCKED(); /* touching vhif_vrs + carp_vhosts */
> + SRPL_FOREACH_LOCKED(sc, &cif->vhif_vrs, sc_list) {
> if (af == AF_INET &&
> ismulti != IN_MULTICAST(sc->sc_peer.s_addr))
> continue;
> - LIST_FOREACH(vhe, &sc->carp_vhosts, vhost_entries) {
> + SRPL_FOREACH_LOCKED(vhe, &sc->carp_vhosts, vhost_entries) {
> if (vhe->vhid == ch->carp_vhid)
> goto found;
> }
> @@ -749,7 +769,9 @@ carp_clone_create(struct if_clone *ifc,
> if (!sc)
> return (ENOMEM);
>
> - LIST_INIT(&sc->carp_vhosts);
> + refcnt_init(&sc->sc_refcnt);
> +
> + SRPL_INIT(&sc->carp_vhosts);
> sc->sc_vhe_count = 0;
> if (carp_new_vhost(sc, 0, 0)) {
> free(sc, M_DEVBUF, sizeof(*sc));
> @@ -801,6 +823,8 @@ carp_new_vhost(struct carp_softc *sc, in
> if (vhe == NULL)
> return (ENOMEM);
>
> + refcnt_init(&vhe->vhost_refcnt);
> + carp_sc_ref(NULL, sc); /* give a sc ref to the vhe */
> vhe->parent_sc = sc;
> vhe->vhid = vhid;
> vhe->advskew = advskew;
> @@ -809,18 +833,23 @@ carp_new_vhost(struct carp_softc *sc, in
> timeout_set(&vhe->md_tmo, carp_master_down, vhe);
> timeout_set(&vhe->md6_tmo, carp_master_down, vhe);
>
> + KERNEL_ASSERT_LOCKED(); /* touching carp_vhosts */
> +
>
Re: reuse pf state ids to "hash" packets onto trunk members
On 2015-09-18, David Gwynne wrote: > this lets pf embed the state id into the mbuf as a "flow id" so > other subsystems can use it. eg, trunk can pull it out and use it. I like this but it does change the path distribution. Previously all flows from host A to host B were bound to a single path, now they are spread across paths. This is good for making fuller use of paths but can make fault diagnosis harder. I tried to work out the best way to make this optional when I sent my earlier L4-hash diff (using PF states for this has similar results and is far more elegant) but didn't settle on an approach. Switches doing this usually have a single global setting (e.g. sysctl), which seems a bit of a blunt instrument but would be easier to apply to areas other than trunk (e.g. multipath routing). We could use an ioctl if ifconfig(8) isn't full already, though I don't think we actually need any more options than "L3-bound" and "per-flow" so using the existing link0 scaffolding would be an easier way to do this per-trunk but I didn't really get a feel for whether people thought that was good enough.
[patch] tame regress for "dns" / "cmsg"
Hi,
In the internal conversion of _TM_* to TAME_*, some bits were lost for
"dns" and "cmsg" in the `tamereq' array.
The initial version of tamereq array (in 1.39) was (for interesting
bits):
{ "malloc", _TM_SELF | _TM_MALLOC },
{ "unix", _TM_SELF | _TM_RW | _TM_UNIX },
{ "dns",TAME_MALLOC | _TM_DNSPATH },
{ "cmsg", TAME_UNIX | _TM_CMSG },
and was been converted to (1.40, and -current):
{ "malloc", TAME_SELF | TAME_MALLOC },
{ "unix", TAME_SELF | TAME_RW | TAME_UNIX },
{ "dns",TAME_MALLOC | TAME_DNSPATH },
{ "cmsg", TAME_UNIX | TAME_CMSG },
but TAME_MALLOC (1.39) don't mean the same thing than TAME_MALLOC (1.40).
in 1.39, expanded flags were:
TAME_DNS = _TM_SELF | _TM_MALLOC | _TM_DNSPATH
TAME_CMSG = _TM_SELF | _TM_RW | _TM_UNIX | TAME_CMSG
The following patch restore the behaviour, and make the regress too work
again.
Comments ? OK ?
--
Sebastien Marie
Index: kern_tame.c
===
RCS file: /cvs/src/sys/kern/kern_tame.c,v
retrieving revision 1.41
diff -u -p -r1.41 kern_tame.c
--- kern_tame.c 13 Sep 2015 17:08:03 - 1.41
+++ kern_tame.c 18 Sep 2015 07:41:24 -
@@ -218,8 +218,8 @@ static const struct {
{ "tmppath",TAME_SELF | TAME_RW | TAME_TMPPATH },
{ "inet", TAME_SELF | TAME_RW | TAME_INET },
{ "unix", TAME_SELF | TAME_RW | TAME_UNIX },
- { "cmsg", TAME_UNIX | TAME_CMSG },
- { "dns",TAME_MALLOC | TAME_DNSPATH },
+ { "cmsg", TAME_SELF | TAME_RW | TAME_UNIX | TAME_CMSG },
+ { "dns",TAME_SELF | TAME_MALLOC | TAME_DNSPATH },
{ "ioctl", TAME_IOCTL },
{ "getpw", TAME_SELF | TAME_MALLOC | TAME_RW | TAME_GETPW
},
{ "proc", TAME_PROC },
