Re: security(8) doesn't know about mailbox locks
Hi,
Philippe Meunier wrote on Fri, Oct 21, 2016 at 12:35:46PM -0400:
> When cron runs /etc/daily, that script runs df and netstat and the
> output is sent by email to root. On my system, emails to root are
> forwarded to local user meunier using /root/.forward. The forwarding
> itself temporarily creates a lock file in /var/mail:
>
> -rw--- 1 root wheel 0 Oct 21 23:55 meunier.lock
>
> At the same time, /etc/daily runs /usr/libexec/security. The
> check_mailboxes function in that file loops over all the files in
> /var/mail and checks whether the owner of the file matches the name of
> the file. If check_mailboxes happens to be running exactly at the
> same time as the system is forwarding /etc/daily's first email, then
> check_mailboxes sees meunier.lock, the check for that file fails, and
> the result is another email sent to root:
>
> Running security(8):
>
> Checking mailbox ownership.
> user meunier.lock mailbox is owned by root
>
> So I think the check_mailboxes function in /usr/libexec/security
> should either skip lock files or check them in a different way...
I don't think checking ownership of lock files helps anything,
so i'd indeed suggest to simply skip them.
OK for the patch below?
Ingo
Index: security
===
RCS file: /cvs/src/libexec/security/security,v
retrieving revision 1.36
diff -u -p -r1.36 security
--- security21 Jul 2015 19:07:13 - 1.36
+++ security22 Oct 2016 06:25:15 -
@@ -455,6 +455,7 @@ sub check_mailboxes {
nag !(opendir my $dh, $dir), "opendir: $dir: $!" and return;
foreach my $name (readdir $dh) {
next if $name =~ /^\.\.?$/;
+ next if $name =~ /.\.lock$/;
my ($mode, $fuid, $fgid) = (stat "$dir/$name")[2,4,5];
unless (defined $mode) {
nag !$!{ENOENT}, "stat: $dir/$name: $!";
Re: minor typo in aldap.c comment
Committed. I'm surprised to see that function using strtoumax()...to parse a 2 character hex number. Mosquito, meet pile driver. Anyone using ypldapd who could improve that? Philip Guenther
minor typo in aldap.c comment
Index: aldap.c === RCS file: /cvs/src/usr.sbin/ypldap/aldap.c,v retrieving revision 1.32 diff -u -p -r1.32 aldap.c --- aldap.c 27 Apr 2016 10:53:27 - 1.32 +++ aldap.c 22 Oct 2016 02:42:13 - @@ -1198,7 +1198,7 @@ isu8cont(unsigned char c) /* * Parse a LDAP value * notes: - * the argument u should be a NULL terminated sequence of ASCII bytes. + * the argument p should be a NULL terminated sequence of ASCII bytes. */ char * parseval(char *p, size_t len)
Re: per cpu mbuf statistics
> On 22 Oct 2016, at 3:14 AM, Alexander Bluhm wrote: > > On Fri, Oct 21, 2016 at 05:02:49PM +1000, David Gwynne wrote: >> this uses the recently added percpu counter api as the backend for >> the mbstats collection. >> >> ok? > > OK bluhm@ cheers. > >> +mbstat = counters_realloc(mbstat, MBSTAT_COUNT, M_DEVBUF); > > There is no man page entry for counters_realloc() and cpumem_realloc(). i dont like the name yet, so i was waiting until i figure out a better one before documenting it. dlg
typo in the errata
Hi, I believe it was a typo diff --git a/errata58.html b/errata58.html index 9cc93b7..aecabd9 100644 --- a/errata58.html +++ b/errata58.html @@ -176,7 +176,7 @@ A source code patch exists which remedies this problem. 009: RELIABILITY FIX: Dec 3, 2015 All architectures -A NULL pointer deference could be triggered by a crafted certificate sent to +A NULL pointer dereference could be triggered by a crafted certificate sent to services configured to verify client certificates on TLS/SSL connections. https://ftp.openbsd.org/pub/OpenBSD/patches/5.8/common/009_clientcert.patch.sig";> A source code patch exists which remedies this problem. Sergey
Re: acpiec on acer aspire S7 with CURRENT
> Date: Fri, 21 Oct 2016 10:37:08 +0300
> From: Paul Irofti
>
> On Tue, Oct 18, 2016 at 09:01:59PM -0700, Ilya Kaliman wrote:
> > Thanks! Now everything seems to work. Minor tweak - maybe need extra
> > newline after "acpiec0 at acpi0"
>
> New diff that fixes the printfs, removes dead code (suggested by
> guenther@) and takes care of some small style(9) nits. OK?
I'm not really happy about the use of the static variable in the
resource parsing function. It feels like the aml_parse_resource API
should be changed to pass the resource number to the callback
function.
> Index: acpidev.h
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpidev.h,v
> retrieving revision 1.38
> diff -u -p -u -p -r1.38 acpidev.h
> --- acpidev.h 12 Aug 2015 05:59:54 - 1.38
> +++ acpidev.h 21 Oct 2016 07:35:22 -
> @@ -323,10 +323,12 @@ struct acpiec_softc {
> int sc_ecbusy;
>
> /* command/status register */
> + bus_size_t sc_ec_sc;
> bus_space_tag_t sc_cmd_bt;
> bus_space_handle_t sc_cmd_bh;
>
> /* data register */
> + bus_size_t sc_ec_data;
> bus_space_tag_t sc_data_bt;
> bus_space_handle_t sc_data_bh;
>
> Index: acpiec.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpiec.c,v
> retrieving revision 1.54
> diff -u -p -u -p -r1.54 acpiec.c
> --- acpiec.c 23 Aug 2016 18:26:21 - 1.54
> +++ acpiec.c 21 Oct 2016 07:35:22 -
> @@ -48,7 +48,7 @@ voidacpiec_write(struct acpiec_softc *
>
> int acpiec_getcrs(struct acpiec_softc *,
> struct acpi_attach_args *);
> -int acpiec_getregister(const u_int8_t *, int, int *, bus_size_t *);
> +int acpiec_parse_resources(union acpi_resource *, void *);
>
> void acpiec_wait(struct acpiec_softc *, u_int8_t, u_int8_t);
> void acpiec_sci_event(struct acpiec_softc *);
> @@ -285,15 +285,16 @@ acpiec_attach(struct device *parent, str
> return;
> }
>
> + printf("\n");
> if (acpiec_getcrs(sc, aa)) {
> - printf(": Failed to read resource settings\n");
> + printf("%s: Failed to read resource settings\n", DEVNAME(sc));
> return;
> }
>
> sc->sc_acpi->sc_ec = sc;
>
> if (acpiec_reg(sc)) {
> - printf(": Failed to register address space\n");
> + printf("%s: Failed to register address space\n", DEVNAME(sc));
> return;
> }
>
> @@ -305,15 +306,13 @@ acpiec_attach(struct device *parent, str
> acpi_set_gpehandler(sc->sc_acpi, sc->sc_gpe, acpiec_gpehandler,
> sc, 1);
> #endif
> -
> +
> if (aml_evalname(sc->sc_acpi, sc->sc_devnode, "_GLK", 0, NULL, &res))
> sc->sc_glk = 0;
> else if (res.type != AML_OBJTYPE_INTEGER)
> sc->sc_glk = 0;
> else
> sc->sc_glk = res.v_integer ? 1 : 0;
> -
> - printf("\n");
> }
>
> void
> @@ -366,68 +365,75 @@ acpiec_gpehandler(struct acpi_softc *acp
> return (0);
> }
>
> -/* parse the resource buffer to get a 'register' value */
> int
> -acpiec_getregister(const u_int8_t *buf, int size, int *type, bus_size_t
> *addr)
> +acpiec_parse_resources(union acpi_resource *crs, void *arg)
> {
> - int len, hlen;
> + struct acpiec_softc *sc = arg;
> + int type = AML_CRSTYPE(crs);
>
> -#define RES_TYPE_MASK 0x80
> -#define RES_LENGTH_MASK 0x07
> -#define RES_TYPE_IOPORT 0x47
> -#define RES_TYPE_ENDTAG 0x79
> + static int argno = 0;
>
> - if (size <= 0)
> - return (0);
> -
> - if (*buf & RES_TYPE_MASK) {
> - /* large resource */
> - if (size < 3)
> - return (1);
> - len = (int)buf[1] + 256 * (int)buf[2];
> - hlen = 3;
> - } else {
> - /* small resource */
> - len = buf[0] & RES_LENGTH_MASK;
> - hlen = 1;
> + switch (argno) {
> + case 0:
> + if (type != SR_IOPORT) {
> + printf("%s: Unexpected resource #%d type %d\n",
> + DEVNAME(sc), argno, type);
> + break;
> + }
> + sc->sc_data_bt = sc->sc_acpi->sc_iot;
> + sc->sc_ec_data = crs->sr_ioport._max;
> + break;
> + case 1:
> + if (type != SR_IOPORT) {
> + printf("%s: Unexpected resource #%d type %d\n",
> + DEVNAME(sc), argno, type);
> + break;
> + }
> + sc->sc_cmd_bt = sc->sc_acpi->sc_iot;
> + sc->sc_ec_sc = crs->sr_ioport._max;
> + break;
> + case 2:
> + if (!sc->sc_acpi->sc_hw_reduced) {
> + printf("%s: Not running
Re: log mutex
On Thu, Oct 20, 2016 at 09:26:16PM +0200, Mark Kettenis wrote:
> I don't think putting a lock in msgbuf_putchar us a good idea. We
> deliberately did not put a lock in kprintf() to make sure it can still
> be used when we're in ddb without hitting a deadlock. Instead we put
> the lock (kprintf_mutex) in the higher-level functions.
I thought the kprintf_mutex is there so that multiple cpu write
line by line and don't mix characters on the display.
kprintf_mutex is not sufficient to protect msgbuf, the functions
log(), logpri(), addlog() do not use it.
Also I have to protect msgbuf in the syscalls. That would mean I
have to spread the kprintf_mutex over subr_prf.c and subr_log.c.
Having a mutex in multiple C files makes it hard to understand.
Better one simple locking mechanism for one purpose.
The ddb problem can be fixed easily, just don't take the mutex when
in ddb. I also check for panicstr, so the logging functions may
panic while holding the lock.
ok?
bluhm
Index: kern/subr_log.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/subr_log.c,v
retrieving revision 1.48
diff -u -p -r1.48 subr_log.c
--- kern/subr_log.c 23 Jun 2016 15:41:42 - 1.48
+++ kern/subr_log.c 21 Oct 2016 18:47:03 -
@@ -59,6 +59,10 @@
#include
#include
+#ifdef DDB
+#include
+#endif
+
#include
#define LOG_RDPRI (PZERO + 1)
@@ -79,6 +83,7 @@ int msgbufmapped; /* is the message bu
struct msgbuf *msgbufp;/* the mapped buffer, itself. */
struct msgbuf *consbufp; /* console message buffer. */
struct file *syslogf;
+struct mutex log_mtx = MUTEX_INITIALIZER(IPL_HIGH);
void filt_logrdetach(struct knote *kn);
int filt_logread(struct knote *kn, long hint);
@@ -140,13 +145,23 @@ initconsbuf(void)
void
msgbuf_putchar(struct msgbuf *mbp, const char c)
{
- int s;
+ int s, use_mtx = 1;
if (mbp->msg_magic != MSG_MAGIC)
/* Nothing we can do */
return;
- s = splhigh();
+ if (panicstr)
+ use_mtx = 0;
+#ifdef DDB
+ if (db_is_active)
+ use_mtx = 0;
+#endif
+ if (use_mtx)
+ mtx_enter(&log_mtx);
+ else
+ s = splhigh();
+
mbp->msg_bufc[mbp->msg_bufx++] = c;
mbp->msg_bufl = lmin(mbp->msg_bufl+1, mbp->msg_bufs);
if (mbp->msg_bufx < 0 || mbp->msg_bufx >= mbp->msg_bufs)
@@ -157,7 +172,11 @@ msgbuf_putchar(struct msgbuf *mbp, const
mbp->msg_bufr = 0;
mbp->msg_bufd++;
}
- splx(s);
+
+ if (use_mtx)
+ mtx_leave(&log_mtx);
+ else
+ splx(s);
}
int
@@ -186,19 +205,21 @@ logread(dev_t dev, struct uio *uio, int
{
struct msgbuf *mbp = msgbufp;
size_t l;
- int s, error = 0;
+ int error = 0;
- s = splhigh();
+ mtx_enter(&log_mtx);
while (mbp->msg_bufr == mbp->msg_bufx) {
if (flag & IO_NDELAY) {
- error = EWOULDBLOCK;
- goto out;
+ mtx_leave(&log_mtx);
+ return (EWOULDBLOCK);
}
logsoftc.sc_state |= LOG_RDWAIT;
- error = tsleep(mbp, LOG_RDPRI | PCATCH,
+ error = msleep(mbp, &log_mtx, LOG_RDPRI | PCATCH,
"klog", 0);
- if (error)
- goto out;
+ if (error) {
+ mtx_leave(&log_mtx);
+ return (error);
+ }
}
logsoftc.sc_state &= ~LOG_RDWAIT;
@@ -209,13 +230,17 @@ logread(dev_t dev, struct uio *uio, int
"<%d>klog: dropped %ld byte%s, message buffer full\n",
LOG_KERN|LOG_WARNING, mbp->msg_bufd,
mbp->msg_bufd == 1 ? "" : "s");
+ mtx_leave(&log_mtx);
error = uiomove(buf, ulmin(l, sizeof(buf) - 1), uio);
if (error)
- goto out;
+ return (error);
+ mtx_enter(&log_mtx);
mbp->msg_bufd = 0;
}
while (uio->uio_resid > 0) {
+ char *buf;
+
if (mbp->msg_bufx >= mbp->msg_bufr)
l = mbp->msg_bufx - mbp->msg_bufr;
else
@@ -223,31 +248,34 @@ logread(dev_t dev, struct uio *uio, int
l = ulmin(l, uio->uio_resid);
if (l == 0)
break;
- error = uiomove(&mbp->msg_bufc[mbp->msg_bufr], l, uio);
+ buf = &mbp->msg_bufc[mbp->msg_bufr];
+ mtx_leave(&log_mtx);
+ error = uiomove(buf, l, uio);
if (error)
- break;
+ return (error);
+ mtx_enter(&log_mtx);
mbp->m
Re: acpiec on acer aspire S7 with CURRENT
Works for me. Ok.
acpiprt11 at acpi0: bus -1 (PEG1)
acpiprt12 at acpi0: bus -1 (PEG2)
acpiec0 at acpi0
acpiec0: Not running on HW-Reduced ACPI type 8
acpiec0: invalid resource #3 type 134
acpicpu0 at acpi0: C3(200@506 mwait.1@0x60), C2(200@148 mwait.1@0x33),
C1(1000@1 mwait.1), PSS
acpicpu1 at acpi0: C3(200@506 mwait.1@0x60), C2(200@148 mwait.1@0x33),
C1(1000@1 mwait.1), PSS
Thanks,
Ilya
On Fri, Oct 21, 2016 at 12:37 AM, Paul Irofti wrote:
> On Tue, Oct 18, 2016 at 09:01:59PM -0700, Ilya Kaliman wrote:
>> Thanks! Now everything seems to work. Minor tweak - maybe need extra
>> newline after "acpiec0 at acpi0"
>
> New diff that fixes the printfs, removes dead code (suggested by
> guenther@) and takes care of some small style(9) nits. OK?
>
>
> Index: acpidev.h
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpidev.h,v
> retrieving revision 1.38
> diff -u -p -u -p -r1.38 acpidev.h
> --- acpidev.h 12 Aug 2015 05:59:54 - 1.38
> +++ acpidev.h 21 Oct 2016 07:35:22 -
> @@ -323,10 +323,12 @@ struct acpiec_softc {
> int sc_ecbusy;
>
> /* command/status register */
> + bus_size_t sc_ec_sc;
> bus_space_tag_t sc_cmd_bt;
> bus_space_handle_t sc_cmd_bh;
>
> /* data register */
> + bus_size_t sc_ec_data;
> bus_space_tag_t sc_data_bt;
> bus_space_handle_t sc_data_bh;
>
> Index: acpiec.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpiec.c,v
> retrieving revision 1.54
> diff -u -p -u -p -r1.54 acpiec.c
> --- acpiec.c23 Aug 2016 18:26:21 - 1.54
> +++ acpiec.c21 Oct 2016 07:35:22 -
> @@ -48,7 +48,7 @@ void acpiec_write(struct acpiec_softc *
>
> intacpiec_getcrs(struct acpiec_softc *,
> struct acpi_attach_args *);
> -intacpiec_getregister(const u_int8_t *, int, int *, bus_size_t
> *);
> +intacpiec_parse_resources(union acpi_resource *, void *);
>
> void acpiec_wait(struct acpiec_softc *, u_int8_t, u_int8_t);
> void acpiec_sci_event(struct acpiec_softc *);
> @@ -285,15 +285,16 @@ acpiec_attach(struct device *parent, str
> return;
> }
>
> + printf("\n");
> if (acpiec_getcrs(sc, aa)) {
> - printf(": Failed to read resource settings\n");
> + printf("%s: Failed to read resource settings\n", DEVNAME(sc));
> return;
> }
>
> sc->sc_acpi->sc_ec = sc;
>
> if (acpiec_reg(sc)) {
> - printf(": Failed to register address space\n");
> + printf("%s: Failed to register address space\n", DEVNAME(sc));
> return;
> }
>
> @@ -305,15 +306,13 @@ acpiec_attach(struct device *parent, str
> acpi_set_gpehandler(sc->sc_acpi, sc->sc_gpe, acpiec_gpehandler,
> sc, 1);
> #endif
> -
> +
> if (aml_evalname(sc->sc_acpi, sc->sc_devnode, "_GLK", 0, NULL, &res))
> sc->sc_glk = 0;
> else if (res.type != AML_OBJTYPE_INTEGER)
> sc->sc_glk = 0;
> else
> sc->sc_glk = res.v_integer ? 1 : 0;
> -
> - printf("\n");
> }
>
> void
> @@ -366,68 +365,75 @@ acpiec_gpehandler(struct acpi_softc *acp
> return (0);
> }
>
> -/* parse the resource buffer to get a 'register' value */
> int
> -acpiec_getregister(const u_int8_t *buf, int size, int *type, bus_size_t
> *addr)
> +acpiec_parse_resources(union acpi_resource *crs, void *arg)
> {
> - int len, hlen;
> + struct acpiec_softc *sc = arg;
> + int type = AML_CRSTYPE(crs);
>
> -#define RES_TYPE_MASK 0x80
> -#define RES_LENGTH_MASK 0x07
> -#define RES_TYPE_IOPORT0x47
> -#define RES_TYPE_ENDTAG0x79
> + static int argno = 0;
>
> - if (size <= 0)
> - return (0);
> -
> - if (*buf & RES_TYPE_MASK) {
> - /* large resource */
> - if (size < 3)
> - return (1);
> - len = (int)buf[1] + 256 * (int)buf[2];
> - hlen = 3;
> - } else {
> - /* small resource */
> - len = buf[0] & RES_LENGTH_MASK;
> - hlen = 1;
> + switch (argno) {
> + case 0:
> + if (type != SR_IOPORT) {
> + printf("%s: Unexpected resource #%d type %d\n",
> + DEVNAME(sc), argno, type);
> + break;
> + }
> + sc->sc_data_bt = sc->sc_acpi->sc_iot;
> + sc->sc_ec_data = crs->sr_ioport._max;
> + break;
> + case 1:
> + if (type != SR_IOPORT) {
> + printf("%s: Unexpected resource #%d type %d\n",
> + DEVNAME(sc), argno, type)
Re: pf_route pf_pdesc
On Fri, Oct 21, 2016 at 08:07:21PM +1300, Richard Procter wrote:
> On 21/10/2016, at 2:03 AM, Alexander Bluhm wrote:
> > On Thu, Oct 20, 2016 at 10:53:17AM +0200, Claudio Jeker wrote:
> >>> Unfortunately pf_route() is called from pfsync which has no idea
> >>> of packet descriptors. As I do not want to rewrite pfsync, I create
> >>> a temporary pf_pdesc on the stack.
> >>
> >> I'm OK with the direction but am wondering if setting only that little
> >> information in the pf_pdesc is save in the future when that pd is passed
> >> further down.
> >
> > Henning hat the same concern. Now I fill the pf_pdesc completely
> > with pf_setup_pdesc().
>
> I'm sympathetic to the direction. However the patch raises to three the
> number of definitions of union pf_headers (the others in pf.c and
> if_pflog.c). At minimum the comment on pf_headers in pf.c:130 should be
> updated to reflect this.
We could put the union pf_headers into a separate header file.
Henning, I hope you don't mind that I used a current license.template
with your old pf_headers code.
Do we want to go this way?
bluhm
Index: net/if_pflog.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pflog.c,v
retrieving revision 1.74
diff -u -p -r1.74 if_pflog.c
--- net/if_pflog.c 29 Apr 2016 08:55:03 - 1.74
+++ net/if_pflog.c 21 Oct 2016 17:51:41 -
@@ -62,6 +62,7 @@
#endif /* INET6 */
#include
+#include
#include
#define PFLOGMTU (32768 + MHLEN + MLEN)
@@ -297,16 +298,7 @@ pflog_bpfcopy(const void *src_arg, void
u_intcount;
u_char *dst, *mdst;
int afto, hlen, mlen, off;
- union pf_headers {
- struct tcphdr tcp;
- struct udphdr udp;
- struct icmp icmp;
-#ifdef INET6
- struct icmp6_hdricmp6;
- struct mld_hdr mld;
- struct nd_neighbor_solicit nd_ns;
-#endif /* INET6 */
- } pdhdrs;
+ union pf_headers pdhdrs;
struct pf_pdesc pd;
struct pf_addr osaddr, odaddr;
Index: net/if_pfsync.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.235
diff -u -p -r1.235 if_pfsync.c
--- net/if_pfsync.c 4 Oct 2016 13:54:32 - 1.235
+++ net/if_pfsync.c 21 Oct 2016 17:52:04 -
@@ -61,9 +61,13 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
#include
+#include
#include
#include
@@ -87,6 +91,7 @@
#define PF_DEBUGNAME "pfsync: "
#include
+#include
#include
#include
@@ -1732,6 +1737,8 @@ void
pfsync_undefer(struct pfsync_deferral *pd, int drop)
{
struct pfsync_softc *sc = pfsyncif;
+ struct pf_pdesc pdesc;
+ union pf_headers pdhdrs;
splsoftassert(IPL_SOFTNET);
@@ -1743,17 +1750,22 @@ pfsync_undefer(struct pfsync_deferral *p
m_freem(pd->pd_m);
else {
if (pd->pd_st->rule.ptr->rt == PF_ROUTETO) {
+ if (pf_setup_pdesc(&pdesc, &pdhdrs,
+ pd->pd_st->key[PF_SK_WIRE]->af,
+ pd->pd_st->direction, pd->pd_st->rt_kif,
+ pd->pd_m, NULL) != PF_PASS) {
+ m_freem(pd->pd_m);
+ goto out;
+ }
switch (pd->pd_st->key[PF_SK_WIRE]->af) {
case AF_INET:
- pf_route(&pd->pd_m, pd->pd_st->rule.ptr,
- pd->pd_st->direction,
- pd->pd_st->rt_kif->pfik_ifp, pd->pd_st);
+ pf_route(&pd->pd_m, &pdesc,
+ pd->pd_st->rule.ptr, pd->pd_st);
break;
#ifdef INET6
case AF_INET6:
- pf_route6(&pd->pd_m, pd->pd_st->rule.ptr,
- pd->pd_st->direction,
- pd->pd_st->rt_kif->pfik_ifp, pd->pd_st);
+ pf_route6(&pd->pd_m, &pdesc,
+ pd->pd_st->rule.ptr, pd->pd_st);
break;
#endif /* INET6 */
}
@@ -1772,7 +1784,7 @@ pfsync_undefer(struct pfsync_deferral *p
}
}
}
-
+ out:
pool_put(&sc->sc_pool, pd);
}
Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.993
diff -u -p -r1.993 pf.c
--- net/pf.c20 Oct 2016 23:18:43 - 1.993
+++ net/pf.c21 Oct 2016 17:52:26 -
@@ -77,6 +
Re: per cpu mbuf statistics
On Fri, Oct 21, 2016 at 05:02:49PM +1000, David Gwynne wrote: > this uses the recently added percpu counter api as the backend for > the mbstats collection. > > ok? OK bluhm@ > + mbstat = counters_realloc(mbstat, MBSTAT_COUNT, M_DEVBUF); There is no man page entry for counters_realloc() and cpumem_realloc().
Re: PF once rule should not trigger removal of parent anchor rule
Hello Mike,
(I'm putting tech@ back)
> Or some other changes if expire has happened with the deferred removal in
> the thread. What I saying is basically that the last fix I did for once
> rules was tested in the scenario you've described.
sorry I've messed up my description well enough to obfuscate the story.
So let me start again. Below is Petr's test case running on BSD kernel from
Sept 2 (day before the I've changed handling of ONCE rules)
OpenBSD 6.0-current (GENERIC) #2234: Fri Sep 2 06:27:57 MDT 2016
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC
echo 'anchor "foo/*"' | pfctl -f -
pfctl -sr
# anchor "foo/*" all
echo 'pass' | pfctl -a foo/bar -f -
echo 'pass once' | pfctl -a foo/once-bar -f -
pfctl -a "*" -sr
# anchor "foo/*" all {
# anchor "bar" all {
# pass all flags S/SA
# }
# anchor "once-bar" all {
# pass all flags S/SA once
# }
# }
# let some network traffic to hit once rule, any packet will do
# show rules in main anchor again:
pfctl -a "*" -sr
# ?! like no rules here?
# let's anchors we have in kernel
pfctl -sA -v
# foo
# foo/bar
# foo/once-bar
# so there are some anchors, let's see ho foo/once-bar looks like
pfctl -a foo/once-bar -sr
# nothing, it's good. So it looks like the once rule, also removed
# top level anchor foo/*. We think it's not quite right, the issue
# has been pointed out by Petr. He also came with test case.
Now I'll repeat same test case with more recent kernel (from Oct 1st).
OpenBSD 6.0-current (GENERIC.MP) #2512: Sat Oct 1 10:04:59 MDT 2016
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
echo 'anchor "foo/*"' | pfctl -f -
pfctl -sr
# anchor "foo/*" all
echo 'pass' | pfctl -a foo/bar -f -
echo 'pass once' | pfctl -a foo/once-bar -f -
pfctl -a "*" -sr
# anchor "foo/*" all {
# anchor "bar" all {
# pass all flags S/SA
# }
# anchor "once-bar" all {
# pass all flags S/SA once
# }
# }
# let some network traffic to hit once rule, any packet will do
# show rules in main anchor again:
pfctl -a "*" -sr
# anchor "foo/*" all {
# anchor "bar" all {
# pass all flags S/SA
# }
# anchor "once-bar" all {
# }
# }
# this looks better, but still not perfect as 'once-bar' anchor
# remains there and must be removed manually (pfctl -a foo/bar -Fr)
There are two problems in code from Sept 2 (and earlier), when it comes to ONCE
rules and anchors:
(1) once rule always removes its parent anchor rule, regardless if
the ruleset got empty or not.
(2) the ruleset, which used to hold once rule is not cleaned up
automatically
The (1) got fixed by change to once rules introduced on Sept 3rd. It's probably
worth to check what was wrong in the old PF (Sept 2nd and earlier).
At that time we used to call pf_purge_rule() instead of marking rule as
expired (code below comes from pf_test_rule()):
3799if (r->rule_flag & PFRULE_ONCE)
3800pf_purge_rule(ruleset, r, aruleset, a);
Petr's test case populates arguments to pf_purge_rule() as follows:
ruleset:foo/once-bar
rule: pass once all # the rule, we are removing
aruleset: is main ruleset in this particular case
arule: anchor "foo/*" (the only rule we have in main ruleset)
The code of old pf_purge_rule() (Sept 2nd or earlier), just blindly
removed the root-anchor rule from root ruleset in this case. The old
pf_purge_rule() (pf_ioctl.c @ 1.276) looks as follows:
310 pf_purge_rule(struct pf_ruleset *ruleset, struct pf_rule *rule,
311 struct pf_ruleset *aruleset, struct pf_rule *arule)
312 {
313 u_int32_tnr = 0;
314
315 KASSERT(ruleset != NULL && rule != NULL);
316
317 pf_rm_rule(ruleset->rules.active.ptr, rule);
318 ruleset->rules.active.rcount--;
319 TAILQ_FOREACH(rule, ruleset->rules.active.ptr, entries)
320 rule->nr = nr++;
321 ruleset->rules.active.ticket++;
322 pf_calc_skip_steps(ruleset->rules.active.ptr);
323
324 /* remove the parent anchor rule */
325 if (nr == 0 && arule && aruleset) {
326 pf_rm_rule(aruleset->rules.active.ptr, arule);
327 aruleset->rules.active.rcount--;
328 TAILQ_FOREACH(rule, aruleset->rules.active.ptr, entries)
329 rule->nr = nr++;
330 aruleset->rules.active.ticket++;
331 pf_calc_skip_steps(aruleset->rules.active.ptr);
332 }
333 }
lines 317 - 323 remove the rule from foo/once-bar, which becomes empty. The nr
at line 321 is still zero, because TAILQ_FOREACH() at 319 does not run
Re: tcpdump(8): teach how to read ofp
On Wed, Oct 19, 2016 at 12:28:13PM +0200, Rafael Zalamena wrote:
> This is the first step to teach tcpdump(8) how to read OpenFlow protocol
> packets. I've implemented support for most messages switchd(8) uses and
> currently only for OpenFlow 1.3.5, but we might reuse some functions for
> OpenFlow 1.0 as well. For now we don't support showing strings of
> definitions instead of numbers.
>
> Right now we might not be able to tcpdump local switchd(8) <-> switch(4),
> however for remote switchd(8) using TCP it is possible (remote switch(4),
> HP device, Mininet etc...).
>
> Supported messages:
> * Hello (with bitmap, there is a diff for switchd(8) as well);
> * Error (with encapsulated ofp header printing);
> * Echo Request/Reply;
> * Features Request/Reply;
> * Set Config;
> * Packet-in (with encapsulated ethernet printing);
> * Packet-out (with encapsulated ethernet printing);
> * Flow removed;
>
> Missing:
> * Multipart request/reply;
> * Flow mod;
I've updated the diff with the following changes:
- Removed useless whitespaces/tabs;
- Added missing spaces in "[truncated]" messages;
- Recalculate remaining packet length in ofp_print() instead of trusting
tcp_print() length;
- Implemented flow_mod support along with instructions;
ok?
Index: Makefile
===
RCS file: /home/obsdcvs/src/usr.sbin/tcpdump/Makefile,v
retrieving revision 1.59
diff -u -p -r1.59 Makefile
--- Makefile14 Oct 2015 04:55:17 - 1.59
+++ Makefile18 Oct 2016 10:19:50 -
@@ -47,7 +47,7 @@ SRCS= tcpdump.c addrtoname.c privsep.c p
print-ip6.c print-ip6opts.c print-icmp6.c print-dhcp6.c print-frag6.c \
print-bgp.c print-ospf6.c print-ripng.c print-rt6.c print-stp.c \
print-etherip.c print-lwres.c print-lldp.c print-cdp.c print-pflog.c \
- print-pfsync.c pf_print_state.c \
+ print-pfsync.c pf_print_state.c print-ofp.c \
print-udpencap.c print-carp.c \
print-802_11.c print-iapp.c print-mpls.c print-slow.c \
gmt2local.c savestr.c setsignal.c in_cksum.c
Index: interface.h
===
RCS file: /home/obsdcvs/src/usr.sbin/tcpdump/interface.h,v
retrieving revision 1.67
diff -u -p -r1.67 interface.h
--- interface.h 11 Jul 2016 00:27:50 - 1.67
+++ interface.h 21 Oct 2016 13:45:12 -
@@ -274,6 +274,7 @@ extern void mpls_print(const u_char *, u
extern void lldp_print(const u_char *, u_int);
extern void slow_print(const u_char *, u_int);
extern void gtp_print(const u_char *, u_int, u_short, u_short);
+extern void ofp_print(const u_char *);
#ifdef INET6
extern void ip6_print(const u_char *, u_int);
Index: print-tcp.c
===
RCS file: /home/obsdcvs/src/usr.sbin/tcpdump/print-tcp.c,v
retrieving revision 1.35
diff -u -p -r1.35 print-tcp.c
--- print-tcp.c 16 Nov 2015 00:16:39 - 1.35
+++ print-tcp.c 21 Oct 2016 13:47:34 -
@@ -123,6 +123,10 @@ static struct tcp_seq_hash tcp_seq_hash[
#endif
#define NETBIOS_SSN_PORT 139
+/* OpenFlow TCP ports. */
+#define OLD_OFP_PORT 6633
+#define OFP_PORT 6653
+
static int tcp_cksum(const struct ip *ip, const struct tcphdr *tp, int len)
{
union phu {
@@ -665,6 +669,9 @@ tcp_print(const u_char *bp, u_int length
} else {
if (sport == BGP_PORT || dport == BGP_PORT)
bgp_print(bp, length);
+ else if (sport == OLD_OFP_PORT || dport == OLD_OFP_PORT ||
+ sport == OFP_PORT || dport == OFP_PORT)
+ ofp_print(bp);
#if 0
else if (sport == NETBIOS_SSN_PORT || dport == NETBIOS_SSN_PORT)
nbt_tcp_print(bp, length);
--- /dev/null Fri Oct 21 17:26:48 2016
+++ print-ofp.c Fri Oct 21 17:25:11 2016
@@ -0,0 +1,1166 @@
+/* $$ */
+
+/*
+ * Copyright (c) 2016 Rafael Zalamena
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+
+#include
+#include
+#include
+
+#include "interface.h"
+
+/* Size of action header without the padding. */
+#define AH_UNPADDED(offsetof(struct ofp_action_header, ah_pad))
+
+voidofp_print_hello(const u_char *, u_int, u_int);
+voidofp_print_
Default case for PHY address lookup on octeon
Could someone with a Portwell CAM-0100 test this diff to verify that
the built-in network interfaces still attach? The diff makes a PHY
address lookup bail out rather than use bogus data if the system board
is unhandled.
Index: arch/octeon/dev/cn30xxgmx.c
===
RCS file: src/sys/arch/octeon/dev/cn30xxgmx.c,v
retrieving revision 1.26
diff -u -p -r1.26 cn30xxgmx.c
--- arch/octeon/dev/cn30xxgmx.c 4 Aug 2016 13:10:31 - 1.26
+++ arch/octeon/dev/cn30xxgmx.c 21 Oct 2016 14:05:31 -
@@ -196,10 +215,13 @@ cn30xxgmx_port_phy_addr(int port)
return port - 16; /* GMX1: eth[0-3] */
return -1;
- default:
+ case BOARD_TYPE_CN3010_EVB_HS5:
if (port >= nitems(octeon_eth_phy_table))
return -1;
return octeon_eth_phy_table[port];
+
+ default:
+ return -1;
}
}
Index: arch/octeon/include/octeonvar.h
===
RCS file: src/sys/arch/octeon/include/octeonvar.h,v
retrieving revision 1.31
diff -u -p -r1.31 octeonvar.h
--- arch/octeon/include/octeonvar.h 16 Jul 2016 10:19:55 - 1.31
+++ arch/octeon/include/octeonvar.h 21 Oct 2016 14:05:31 -
@@ -178,6 +178,7 @@ struct octeon_fau_map {
* NB: BOARD_TYPE_UBIQUITI_E100 is also used by other vendors, but we don't run
* on those boards yet.
*/
+#defineBOARD_TYPE_CN3010_EVB_HS5 11
#defineBOARD_TYPE_UBIQUITI_E10020002
#defineBOARD_TYPE_UBIQUITI_E20020003
#defineBOARD_TYPE_DSR_500 20015
Re: fix ILP32 switch(4) build
Jonathan Gray writes:
> /usr/src/sys/net/switchofp.c: In function 'swofp_ox_cmp_ether_addr':
> /usr/src/sys/net/switchofp.c:1758: warning: integer constant is too large for
> 'long' type
> /usr/src/sys/net/switchofp.c: In function 'swofp_ox_match_ether_addr':
> /usr/src/sys/net/switchofp.c:2746: warning: integer constant is too large for
> 'long' type
> *** Error 1 in /usr/src/sys/arch/i386/compile/GENERIC (Makefile:1034
> 'switchofp.o')
Sure.
> Index: switchofp.c
> ===
> RCS file: /cvs/src/sys/net/switchofp.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 switchofp.c
> --- switchofp.c 19 Oct 2016 08:34:53 - 1.14
> +++ switchofp.c 21 Oct 2016 11:52:51 -
> @@ -1755,7 +1755,7 @@ swofp_ox_cmp_ether_addr(struct ofp_ox_ma
> struct ofp_ox_match *key, int strict)
> {
> uint64_t tmth, tmask, kmth, kmask;
> - uint64_t eth_mask = 0x;
> + uint64_t eth_mask = 0xULL;
>
>
> if (OFP_OXM_GET_FIELD(target) != OFP_OXM_GET_FIELD(key))
> @@ -2743,7 +2743,7 @@ int
> swofp_ox_match_ether_addr(struct switch_flow_classify *swfcl,
> struct ofp_ox_match *oxm)
> {
> - uint64_t eth_mask = 0x;
> + uint64_t eth_mask = 0xULL;
> uint64_t in, mth, mask;
>
> switch (OFP_OXM_GET_FIELD(oxm)) {
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
fix ILP32 switch(4) build
/usr/src/sys/net/switchofp.c: In function 'swofp_ox_cmp_ether_addr':
/usr/src/sys/net/switchofp.c:1758: warning: integer constant is too large for
'long' type
/usr/src/sys/net/switchofp.c: In function 'swofp_ox_match_ether_addr':
/usr/src/sys/net/switchofp.c:2746: warning: integer constant is too large for
'long' type
*** Error 1 in /usr/src/sys/arch/i386/compile/GENERIC (Makefile:1034
'switchofp.o')
Index: switchofp.c
===
RCS file: /cvs/src/sys/net/switchofp.c,v
retrieving revision 1.14
diff -u -p -r1.14 switchofp.c
--- switchofp.c 19 Oct 2016 08:34:53 - 1.14
+++ switchofp.c 21 Oct 2016 11:52:51 -
@@ -1755,7 +1755,7 @@ swofp_ox_cmp_ether_addr(struct ofp_ox_ma
struct ofp_ox_match *key, int strict)
{
uint64_t tmth, tmask, kmth, kmask;
- uint64_t eth_mask = 0x;
+ uint64_t eth_mask = 0xULL;
if (OFP_OXM_GET_FIELD(target) != OFP_OXM_GET_FIELD(key))
@@ -2743,7 +2743,7 @@ int
swofp_ox_match_ether_addr(struct switch_flow_classify *swfcl,
struct ofp_ox_match *oxm)
{
- uint64_t eth_mask = 0x;
+ uint64_t eth_mask = 0xULL;
uint64_t in, mth, mask;
switch (OFP_OXM_GET_FIELD(oxm)) {
Re: snmpd(8): teach how to fork+exec
On Fri, Oct 21, 2016 at 01:26:36PM +0200, Jeremie Courreges-Anglas wrote: > Rafael Zalamena writes: > > On Fri, Oct 14, 2016 at 06:47:09PM +0200, Rafael Zalamena wrote: > >> On Mon, Sep 26, 2016 at 03:45:59PM +0200, Rafael Zalamena wrote: > >> ---snip--- > > > > I got feedback from jca@ that the trap handler wasn't working, so after > > trying to reproduce the problem myself I found one 'env' global variable > > that was not being set and the child process was dying silently. > > (thanks jca@ !) > > > > Instead of depending on snmpe.c:snmpe env initialization (p_init), I'm > > now calling smi_setenv() to do that in the main() function so all children > > get the same behaviour. Also note that we don't have an 'extern' env in > > smi.c anymore. > > > > ok? > > Works fine here, but then I don't understand the relationship between > static struct snmpd *env in smi.c and struct snmpd *env in snmpe.c. smi.c had a "extern struct snmpd *env" and that variable was only being set during the snmpe initialzation (p_init). Since with fork+exec the child process runs entirely from scratch (no memory / socket sharing with the parent process), we need to set it somewhere else. It is a known problem that everyone that used to set things in the p_init and expected it to work for everyother process was wrong. sunil@ found this the hard-way when he found out that p_env wasn't being set for his process and he noticed that now p_init is ran in the child process already. Before fork+exec the p_init() functions were run by the parent process. To fix the current problem I made the 'env' for smi.c to be a local file global variable and set it in the main() process for every child.
Re: snmpd source address
Jeremie Courreges-Anglas writes:
> Jeremie Courreges-Anglas writes:
>
>> Jeremie Courreges-Anglas writes:
>>
>>> j...@wxcvbn.org (Jeremie Courreges-Anglas) writes:
>>>
SNMP uses UDP and snmpd listens on a single address. This means that
you can have issues with the source address of the replies sent by
snmpd. "listen on $loopback_address":
http://marc.info/?l=openbsd-misc&m=147445870822415&w=2
seems to be unsufficient, but I don't understand why it would fail.
The diff below should fix the source address selection for replies.
Configuring the source address for traps is a different problem and will
be implemented in a different diff.
>>>
>>> Updated diff that implements a "source-address" parameter for trap
>>> receivers, as discussed with Reyk.
>>>
WIP, only tested on a single box so far. Thoughts / oks?
>>>
>>> source-address lightly tested with:
>>> trap receiver 127.0.0.1
>>> trap receiver 127.0.0.1 source-address 192.168.0.44
>>> trap receiver fe80::1%lo0 source-address ::1
>>>
>>> Comments and test reports welcome.
>>
>> Notes regarding source-address:
>> - only try to handle an address, not a hostname. I didn't want
>> to handle the additional complexity as I'm not sure it's worth it.
>> - say we have "trap receiver somehostname source-address fd00::1".
>> Should we try to filter out potential IPv4 addresses and find an
>> appropriate IPv6 address? The diff below would error out if the first
>> record found is an IPv4.
>
> I decided to go ahead and skip records in case of AF mismatch. Lightly
> tested with:
> - /etc/snmpd.conf
> trap receiver somehostname source-address ::1
> - $ ifconfig lo1
> lo1: flags=8049 mtu 32768
> index 6 priority 0 llprio 3
> groups: lo
> inet 192.0.2.1 netmask 0xff00
> inet6 ::1 prefixlen 128
> inet6 fe80::1%lo1 prefixlen 64 scopeid 0x6
> inet6 2001:db8::1 prefixlen 64
> - /etc/hosts
> 192.0.2.1 somehostname
> 2001:db8::1 somehostname
>
> Comments and test reports still welcome. :)
Ping. Now with manpage bits.
Index: parse.y
===
RCS file: /d/cvs/src/usr.sbin/snmpd/parse.y,v
retrieving revision 1.39
diff -u -p -r1.39 parse.y
--- parse.y 21 Jun 2016 21:35:25 - 1.39
+++ parse.y 21 Oct 2016 11:28:15 -
@@ -98,9 +98,10 @@ static intnctlsocks = 0;
struct address *host_v4(const char *);
struct address *host_v6(const char *);
int host_dns(const char *, struct addresslist *,
- int, in_port_t, struct ber_oid *, char *);
+ int, in_port_t, struct ber_oid *, char *,
+ struct address *);
int host(const char *, struct addresslist *,
- int, in_port_t, struct ber_oid *, char *);
+ int, in_port_t, struct ber_oid *, char *, char *);
typedef struct {
union {
@@ -127,10 +128,11 @@ typedef struct {
%token SYSTEM CONTACT DESCR LOCATION NAME OBJECTID SERVICES RTFILTER
%token READONLY READWRITE OCTETSTRING INTEGER COMMUNITY TRAP RECEIVER
%token SECLEVEL NONE AUTH ENC USER AUTHKEY ENCKEY ERROR DISABLED
-%token SOCKET RESTRICTED AGENTX HANDLE DEFAULT
+%token SOCKET RESTRICTED AGENTX HANDLE DEFAULT SRCADDR
%token STRING
%token NUMBER
%typehostcmn
+%typesrcaddr
%typeoptwrite yesno seclevel socktype
%type objtype cmd
%type oid hostoid trapoid
@@ -200,7 +202,8 @@ main: LISTEN ON STRING {
struct address *h;
TAILQ_INIT(&al);
- if (host($3, &al, 1, SNMPD_PORT, NULL, NULL) <= 0) {
+ if (host($3, &al, 1, SNMPD_PORT, NULL, NULL, NULL)
+ <= 0) {
yyerror("invalid ip address: %s", $3);
free($3);
YYERROR;
@@ -445,9 +448,13 @@ hostcmn: /* empty */
{ $$ = NULL; }
| COMMUNITY STRING { $$ = $2; }
;
-hostdef: STRING hostoid hostcmn{
+srcaddr: /* empty */ { $$ = NULL; }
+ | SRCADDR STRING{ $$ = $2; }
+ ;
+
+hostdef: STRING hostoid hostcmn srcaddr{
if (host($1, hlist, 1,
- SNMPD_TRAPPORT, $2, $3) <= 0) {
+ SNMPD_TRAPPORT, $2, $3, $4) <= 0) {
yyerror("invalid host: %s", $1);
free($1);
YYERROR;
@@ -636,6 +643,7 @@ lookup(char *s)
{ "seclevel", SECLEVEL },
{ "services",
Re: snmpd(8): teach how to fork+exec
Rafael Zalamena writes:
> On Fri, Oct 14, 2016 at 06:47:09PM +0200, Rafael Zalamena wrote:
>> On Mon, Sep 26, 2016 at 03:45:59PM +0200, Rafael Zalamena wrote:
>> > Lets teach snmpd(8) how to fork+exec using the proc.c file from the latest
>> > switchd(8) diff.
>> >
>> > Note 1: I just tested the basic operations: startup and teardown.
>> > Note 2: the kill with close will be implemented in another diff with the
>> > ps_pid removal.
>> >
>>
>> I've update the diff with the latest proc.c changes.
>>
>> Note: I still have to implement kill with close().
>>
>
> I got feedback from jca@ that the trap handler wasn't working, so after
> trying to reproduce the problem myself I found one 'env' global variable
> that was not being set and the child process was dying silently.
> (thanks jca@ !)
>
> Instead of depending on snmpe.c:snmpe env initialization (p_init), I'm
> now calling smi_setenv() to do that in the main() function so all children
> get the same behaviour. Also note that we don't have an 'extern' env in
> smi.c anymore.
>
> ok?
Works fine here, but then I don't understand the relationship between
static struct snmpd *env in smi.c and struct snmpd *env in snmpe.c.
> Index: proc.c
> ===
> RCS file: /home/obsdcvs/src/usr.sbin/snmpd/proc.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 proc.c
> --- proc.c7 Dec 2015 16:05:56 - 1.20
> +++ proc.c14 Oct 2016 15:42:19 -
> @@ -1,7 +1,7 @@
> /* $OpenBSD: proc.c,v 1.20 2015/12/07 16:05:56 reyk Exp $ */
>
> /*
> - * Copyright (c) 2010 - 2014 Reyk Floeter
> + * Copyright (c) 2010 - 2016 Reyk Floeter
> * Copyright (c) 2008 Pierre-Yves Ritschard
> *
> * Permission to use, copy, modify, and distribute this software for any
> @@ -22,6 +22,7 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> @@ -34,8 +35,12 @@
>
> #include "snmpd.h"
>
> -void proc_open(struct privsep *, struct privsep_proc *,
> - struct privsep_proc *, size_t);
> +void proc_exec(struct privsep *, struct privsep_proc *, unsigned int,
> + int, char **);
> +void proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
> +void proc_open(struct privsep *, int, int);
> +void proc_accept(struct privsep *, int, enum privsep_procid,
> + unsigned int);
> void proc_close(struct privsep *);
> int proc_ispeer(struct privsep_proc *, unsigned int, enum privsep_procid);
> void proc_shutdown(struct privsep_proc *);
> @@ -55,204 +60,383 @@ proc_ispeer(struct privsep_proc *procs,
> return (0);
> }
>
> -void
> -proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc)
> +enum privsep_procid
> +proc_getid(struct privsep_proc *procs, unsigned int nproc,
> +const char *proc_name)
> {
> - unsigned int i, j, src, dst;
> - struct privsep_pipes*pp;
> + struct privsep_proc *p;
> + unsigned int proc;
>
> - /*
> - * Allocate pipes for all process instances (incl. parent)
> - *
> - * - ps->ps_pipes: N:M mapping
> - * N source processes connected to M destination processes:
> - * [src][instances][dst][instances], for example
> - * [PROC_RELAY][3][PROC_CA][3]
> - *
> - * - ps->ps_pp: per-process 1:M part of ps->ps_pipes
> - * Each process instance has a destination array of socketpair fds:
> - * [dst][instances], for example
> - * [PROC_PARENT][0]
> - */
> - for (src = 0; src < PROC_MAX; src++) {
> - /* Allocate destination array for each process */
> - if ((ps->ps_pipes[src] = calloc(ps->ps_ninstances,
> - sizeof(struct privsep_pipes))) == NULL)
> - fatal("proc_init: calloc");
> + for (proc = 0; proc < nproc; proc++) {
> + p = &procs[proc];
> + if (strcmp(p->p_title, proc_name))
> + continue;
>
> - for (i = 0; i < ps->ps_ninstances; i++) {
> - pp = &ps->ps_pipes[src][i];
> + return (p->p_id);
> + }
>
> - for (dst = 0; dst < PROC_MAX; dst++) {
> - /* Allocate maximum fd integers */
> - if ((pp->pp_pipes[dst] =
> - calloc(ps->ps_ninstances,
> - sizeof(int))) == NULL)
> - fatal("proc_init: calloc");
> + return (PROC_MAX);
> +}
>
> - /* Mark fd as unused */
> - for (j = 0; j < ps->ps_ninstances; j++)
> - pp->pp_pipes[dst][j] = -1;
> - }
> - }
> - }
> +void
> +proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
> +int argc, char **argv)
> +{
> + unsigned int proc, nargc, i, proc_i;
Re: snmpd(8): teach how to fork+exec
On Fri, Oct 14, 2016 at 06:47:09PM +0200, Rafael Zalamena wrote:
> On Mon, Sep 26, 2016 at 03:45:59PM +0200, Rafael Zalamena wrote:
> > Lets teach snmpd(8) how to fork+exec using the proc.c file from the latest
> > switchd(8) diff.
> >
> > Note 1: I just tested the basic operations: startup and teardown.
> > Note 2: the kill with close will be implemented in another diff with the
> > ps_pid removal.
> >
>
> I've update the diff with the latest proc.c changes.
>
> Note: I still have to implement kill with close().
>
I got feedback from jca@ that the trap handler wasn't working, so after
trying to reproduce the problem myself I found one 'env' global variable
that was not being set and the child process was dying silently.
(thanks jca@ !)
Instead of depending on snmpe.c:snmpe env initialization (p_init), I'm
now calling smi_setenv() to do that in the main() function so all children
get the same behaviour. Also note that we don't have an 'extern' env in
smi.c anymore.
ok?
Index: proc.c
===
RCS file: /home/obsdcvs/src/usr.sbin/snmpd/proc.c,v
retrieving revision 1.20
diff -u -p -r1.20 proc.c
--- proc.c 7 Dec 2015 16:05:56 - 1.20
+++ proc.c 14 Oct 2016 15:42:19 -
@@ -1,7 +1,7 @@
/* $OpenBSD: proc.c,v 1.20 2015/12/07 16:05:56 reyk Exp $ */
/*
- * Copyright (c) 2010 - 2014 Reyk Floeter
+ * Copyright (c) 2010 - 2016 Reyk Floeter
* Copyright (c) 2008 Pierre-Yves Ritschard
*
* Permission to use, copy, modify, and distribute this software for any
@@ -22,6 +22,7 @@
#include
#include
+#include
#include
#include
#include
@@ -34,8 +35,12 @@
#include "snmpd.h"
-voidproc_open(struct privsep *, struct privsep_proc *,
- struct privsep_proc *, size_t);
+voidproc_exec(struct privsep *, struct privsep_proc *, unsigned int,
+ int, char **);
+voidproc_setup(struct privsep *, struct privsep_proc *, unsigned int);
+voidproc_open(struct privsep *, int, int);
+voidproc_accept(struct privsep *, int, enum privsep_procid,
+ unsigned int);
voidproc_close(struct privsep *);
int proc_ispeer(struct privsep_proc *, unsigned int, enum privsep_procid);
voidproc_shutdown(struct privsep_proc *);
@@ -55,204 +60,383 @@ proc_ispeer(struct privsep_proc *procs,
return (0);
}
-void
-proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc)
+enum privsep_procid
+proc_getid(struct privsep_proc *procs, unsigned int nproc,
+const char *proc_name)
{
- unsigned int i, j, src, dst;
- struct privsep_pipes*pp;
+ struct privsep_proc *p;
+ unsigned int proc;
- /*
-* Allocate pipes for all process instances (incl. parent)
-*
-* - ps->ps_pipes: N:M mapping
-* N source processes connected to M destination processes:
-* [src][instances][dst][instances], for example
-* [PROC_RELAY][3][PROC_CA][3]
-*
-* - ps->ps_pp: per-process 1:M part of ps->ps_pipes
-* Each process instance has a destination array of socketpair fds:
-* [dst][instances], for example
-* [PROC_PARENT][0]
-*/
- for (src = 0; src < PROC_MAX; src++) {
- /* Allocate destination array for each process */
- if ((ps->ps_pipes[src] = calloc(ps->ps_ninstances,
- sizeof(struct privsep_pipes))) == NULL)
- fatal("proc_init: calloc");
+ for (proc = 0; proc < nproc; proc++) {
+ p = &procs[proc];
+ if (strcmp(p->p_title, proc_name))
+ continue;
- for (i = 0; i < ps->ps_ninstances; i++) {
- pp = &ps->ps_pipes[src][i];
+ return (p->p_id);
+ }
- for (dst = 0; dst < PROC_MAX; dst++) {
- /* Allocate maximum fd integers */
- if ((pp->pp_pipes[dst] =
- calloc(ps->ps_ninstances,
- sizeof(int))) == NULL)
- fatal("proc_init: calloc");
+ return (PROC_MAX);
+}
- /* Mark fd as unused */
- for (j = 0; j < ps->ps_ninstances; j++)
- pp->pp_pipes[dst][j] = -1;
- }
- }
- }
+void
+proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
+int argc, char **argv)
+{
+ unsigned int proc, nargc, i, proc_i;
+ char**nargv;
+ struct privsep_proc *p;
+ char num[32];
+ int fd;
+
+ /* Prepare the new process argv. */
+ nargv = calloc(argc + 5, sizeof(char *));
+ if (nargv == NULL)
+
Re: acpiec on acer aspire S7 with CURRENT
On Tue, Oct 18, 2016 at 09:01:59PM -0700, Ilya Kaliman wrote:
> Thanks! Now everything seems to work. Minor tweak - maybe need extra
> newline after "acpiec0 at acpi0"
New diff that fixes the printfs, removes dead code (suggested by
guenther@) and takes care of some small style(9) nits. OK?
Index: acpidev.h
===
RCS file: /cvs/src/sys/dev/acpi/acpidev.h,v
retrieving revision 1.38
diff -u -p -u -p -r1.38 acpidev.h
--- acpidev.h 12 Aug 2015 05:59:54 - 1.38
+++ acpidev.h 21 Oct 2016 07:35:22 -
@@ -323,10 +323,12 @@ struct acpiec_softc {
int sc_ecbusy;
/* command/status register */
+ bus_size_t sc_ec_sc;
bus_space_tag_t sc_cmd_bt;
bus_space_handle_t sc_cmd_bh;
/* data register */
+ bus_size_t sc_ec_data;
bus_space_tag_t sc_data_bt;
bus_space_handle_t sc_data_bh;
Index: acpiec.c
===
RCS file: /cvs/src/sys/dev/acpi/acpiec.c,v
retrieving revision 1.54
diff -u -p -u -p -r1.54 acpiec.c
--- acpiec.c23 Aug 2016 18:26:21 - 1.54
+++ acpiec.c21 Oct 2016 07:35:22 -
@@ -48,7 +48,7 @@ void acpiec_write(struct acpiec_softc *
intacpiec_getcrs(struct acpiec_softc *,
struct acpi_attach_args *);
-intacpiec_getregister(const u_int8_t *, int, int *, bus_size_t *);
+intacpiec_parse_resources(union acpi_resource *, void *);
void acpiec_wait(struct acpiec_softc *, u_int8_t, u_int8_t);
void acpiec_sci_event(struct acpiec_softc *);
@@ -285,15 +285,16 @@ acpiec_attach(struct device *parent, str
return;
}
+ printf("\n");
if (acpiec_getcrs(sc, aa)) {
- printf(": Failed to read resource settings\n");
+ printf("%s: Failed to read resource settings\n", DEVNAME(sc));
return;
}
sc->sc_acpi->sc_ec = sc;
if (acpiec_reg(sc)) {
- printf(": Failed to register address space\n");
+ printf("%s: Failed to register address space\n", DEVNAME(sc));
return;
}
@@ -305,15 +306,13 @@ acpiec_attach(struct device *parent, str
acpi_set_gpehandler(sc->sc_acpi, sc->sc_gpe, acpiec_gpehandler,
sc, 1);
#endif
-
+
if (aml_evalname(sc->sc_acpi, sc->sc_devnode, "_GLK", 0, NULL, &res))
sc->sc_glk = 0;
else if (res.type != AML_OBJTYPE_INTEGER)
sc->sc_glk = 0;
else
sc->sc_glk = res.v_integer ? 1 : 0;
-
- printf("\n");
}
void
@@ -366,68 +365,75 @@ acpiec_gpehandler(struct acpi_softc *acp
return (0);
}
-/* parse the resource buffer to get a 'register' value */
int
-acpiec_getregister(const u_int8_t *buf, int size, int *type, bus_size_t *addr)
+acpiec_parse_resources(union acpi_resource *crs, void *arg)
{
- int len, hlen;
+ struct acpiec_softc *sc = arg;
+ int type = AML_CRSTYPE(crs);
-#define RES_TYPE_MASK 0x80
-#define RES_LENGTH_MASK 0x07
-#define RES_TYPE_IOPORT0x47
-#define RES_TYPE_ENDTAG0x79
+ static int argno = 0;
- if (size <= 0)
- return (0);
-
- if (*buf & RES_TYPE_MASK) {
- /* large resource */
- if (size < 3)
- return (1);
- len = (int)buf[1] + 256 * (int)buf[2];
- hlen = 3;
- } else {
- /* small resource */
- len = buf[0] & RES_LENGTH_MASK;
- hlen = 1;
+ switch (argno) {
+ case 0:
+ if (type != SR_IOPORT) {
+ printf("%s: Unexpected resource #%d type %d\n",
+ DEVNAME(sc), argno, type);
+ break;
+ }
+ sc->sc_data_bt = sc->sc_acpi->sc_iot;
+ sc->sc_ec_data = crs->sr_ioport._max;
+ break;
+ case 1:
+ if (type != SR_IOPORT) {
+ printf("%s: Unexpected resource #%d type %d\n",
+ DEVNAME(sc), argno, type);
+ break;
+ }
+ sc->sc_cmd_bt = sc->sc_acpi->sc_iot;
+ sc->sc_ec_sc = crs->sr_ioport._max;
+ break;
+ case 2:
+ if (!sc->sc_acpi->sc_hw_reduced) {
+ printf("%s: Not running on HW-Reduced ACPI type %d\n",
+ DEVNAME(sc), type);
+ break;
+ }
+ /* XXX: handle SCI GPIO */
+ break;
+ default:
+ printf("%s: invalid resource #%d type %d\n",
+ DEVNAME(sc), argno, type);
}
- /* XXX todo: decode other types */
Re: pf_route pf_pdesc
On 21/10/2016, at 2:03 AM, Alexander Bluhm wrote:
> On Thu, Oct 20, 2016 at 10:53:17AM +0200, Claudio Jeker wrote:
>>> Unfortunately pf_route() is called from pfsync which has no idea
>>> of packet descriptors. As I do not want to rewrite pfsync, I create
>>> a temporary pf_pdesc on the stack.
>>
>> I'm OK with the direction but am wondering if setting only that little
>> information in the pf_pdesc is save in the future when that pd is passed
>> further down.
>
> Henning hat the same concern. Now I fill the pf_pdesc completely
> with pf_setup_pdesc().
I'm sympathetic to the direction. However the patch raises to three the
number of definitions of union pf_headers (the others in pf.c and
if_pflog.c). At minimum the comment on pf_headers in pf.c:130 should be
updated to reflect this.
best,
Richard.
>
> ok?
>
> bluhm
>
> Index: net/if_pfsync.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pfsync.c,v
> retrieving revision 1.235
> diff -u -p -u -p -r1.235 if_pfsync.c
> --- net/if_pfsync.c 4 Oct 2016 13:54:32 - 1.235
> +++ net/if_pfsync.c 20 Oct 2016 12:19:31 -
> @@ -61,9 +61,13 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -1732,6 +1736,17 @@ void
> pfsync_undefer(struct pfsync_deferral *pd, int drop)
> {
> struct pfsync_softc *sc = pfsyncif;
> + struct pf_pdesc pdesc;
> + union pf_headers {
> + struct tcphdr tcp;
> + struct udphdr udp;
> + struct icmp icmp;
> +#ifdef INET6
> + struct icmp6_hdricmp6;
> + struct mld_hdr mld;
> + struct nd_neighbor_solicit nd_ns;
> +#endif /* INET6 */
> + } pdhdrs;
>
> splsoftassert(IPL_SOFTNET);
>
> @@ -1743,17 +1758,22 @@ pfsync_undefer(struct pfsync_deferral *p
> m_freem(pd->pd_m);
> else {
> if (pd->pd_st->rule.ptr->rt == PF_ROUTETO) {
> + if (pf_setup_pdesc(&pdesc, &pdhdrs,
> + pd->pd_st->key[PF_SK_WIRE]->af,
> + pd->pd_st->direction, pd->pd_st->rt_kif,
> + pd->pd_m, NULL) != PF_PASS) {
> + m_freem(pd->pd_m);
> + goto out;
> + }
> switch (pd->pd_st->key[PF_SK_WIRE]->af) {
> case AF_INET:
> - pf_route(&pd->pd_m, pd->pd_st->rule.ptr,
> - pd->pd_st->direction,
> - pd->pd_st->rt_kif->pfik_ifp, pd->pd_st);
> + pf_route(&pd->pd_m, &pdesc,
> + pd->pd_st->rule.ptr, pd->pd_st);
> break;
> #ifdef INET6
> case AF_INET6:
> - pf_route6(&pd->pd_m, pd->pd_st->rule.ptr,
> - pd->pd_st->direction,
> - pd->pd_st->rt_kif->pfik_ifp, pd->pd_st);
> + pf_route6(&pd->pd_m, &pdesc,
> + pd->pd_st->rule.ptr, pd->pd_st);
> break;
> #endif /* INET6 */
> }
> @@ -1772,7 +1792,7 @@ pfsync_undefer(struct pfsync_deferral *p
> }
> }
> }
> -
> + out:
> pool_put(&sc->sc_pool, pd);
> }
>
> Index: net/pf.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.992
> diff -u -p -u -p -r1.992 pf.c
> --- net/pf.c 18 Oct 2016 13:28:01 - 1.992
> +++ net/pf.c 20 Oct 2016 11:29:54 -
> @@ -5764,7 +5764,7 @@ pf_rtlabel_match(struct pf_addr *addr, s
> }
>
> void
> -pf_route(struct mbuf **m, struct pf_rule *r, int dir, struct ifnet *oifp,
> +pf_route(struct mbuf **m, struct pf_pdesc *pd, struct pf_rule *r,
> struct pf_state *s)
> {
> struct mbuf *m0, *m1;
> @@ -5777,8 +5777,7 @@ pf_route(struct mbuf **m, struct pf_rule
> int error = 0;
> unsigned int rtableid;
>
> - if (m == NULL || *m == NULL || r == NULL ||
> - (dir != PF_IN && dir != PF_OUT) || oifp == NULL)
> + if (m == NULL || *m == NULL || r == NULL)
> panic("pf_route: invalid parameters");
>
> if ((*m)->m_pkthdr.pf.routed++ > 3) {
> @@ -5791,7 +5790,7 @@ pf_route(struct mbuf **m, struct pf_rule
> if ((m0 = m_dup_pkt(*m, max_linkhdr, M_NOWAIT)) == NULL)
> return;
> } else {
> - if ((r->rt == PF_REPLYTO) == (r->direction == dir))
> + if ((r->rt == PF_REPLYTO) == (r->direction == pd->dir))
> return;
> m0
per cpu mbuf statistics
this uses the recently added percpu counter api as the backend for
the mbstats collection.
ok?
Index: kern/init_main.c
===
RCS file: /cvs/src/sys/kern/init_main.c,v
retrieving revision 1.260
diff -u -p -r1.260 init_main.c
--- kern/init_main.c21 Oct 2016 06:27:50 - 1.260
+++ kern/init_main.c21 Oct 2016 07:01:18 -
@@ -415,6 +415,8 @@ main(void *framep)
prof_init();
#endif
+ mbcpuinit();/* enable per cpu mbuf data */
+
/* init exec and emul */
init_exec();
Index: kern/kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.316
diff -u -p -r1.316 kern_sysctl.c
--- kern/kern_sysctl.c 8 Oct 2016 21:31:56 - 1.316
+++ kern/kern_sysctl.c 21 Oct 2016 07:01:18 -
@@ -62,6 +62,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -390,9 +391,24 @@ kern_sysctl(int *name, u_int namelen, vo
case KERN_FILE:
return (sysctl_file(name + 1, namelen - 1, oldp, oldlenp, p));
#endif
- case KERN_MBSTAT:
- return (sysctl_rdstruct(oldp, oldlenp, newp, &mbstat,
- sizeof(mbstat)));
+ case KERN_MBSTAT: {
+ extern struct cpumem *mbstat;
+ uint64_t counters[MBSTAT_COUNT];
+ struct mbstat mbs;
+ unsigned int i;
+
+ memset(&mbs, 0, sizeof(mbs));
+ counters_read(mbstat, counters, MBSTAT_COUNT);
+ for (i = 0; i < MBSTAT_TYPES; i++)
+ mbs.m_mtypes[i] = counters[i];
+
+ mbs.m_drops = counters[MBSTAT_DROPS];
+ mbs.m_wait = counters[MBSTAT_WAIT];
+ mbs.m_drain = counters[MBSTAT_DRAIN];
+
+ return (sysctl_rdstruct(oldp, oldlenp, newp,
+ &mbs, sizeof(mbs)));
+ }
#if defined(GPROF) || defined(DDBPROF)
case KERN_PROF:
return (sysctl_doprof(name + 1, namelen - 1, oldp, oldlenp,
Index: kern/uipc_mbuf.c
===
RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.233
diff -u -p -r1.233 uipc_mbuf.c
--- kern/uipc_mbuf.c10 Oct 2016 00:41:17 - 1.233
+++ kern/uipc_mbuf.c21 Oct 2016 07:01:18 -
@@ -83,6 +83,7 @@
#include
#include
#include
+#include
#include
#include
@@ -99,9 +100,11 @@
#include
#endif /* NPF > 0 */
-struct mbstat mbstat; /* mbuf stats */
-struct mutex mbstatmtx = MUTEX_INITIALIZER(IPL_NET);
-struct pool mbpool;/* mbuf pool */
+/* mbuf stats */
+COUNTERS_BOOT_MEMORY(mbstat_boot, MBSTAT_COUNT);
+struct cpumem *mbstat = COUNTERS_BOOT_INITIALIZER(mbstat_boot);
+/* mbuf pools */
+struct pool mbpool;
struct pool mtagpool;
/* mbuf cluster pools */
@@ -173,6 +176,12 @@ mbinit(void)
}
void
+mbcpuinit()
+{
+ mbstat = counters_realloc(mbstat, MBSTAT_COUNT, M_DEVBUF);
+}
+
+void
nmbclust_update(void)
{
unsigned int i, n;
@@ -204,14 +213,21 @@ struct mbuf *
m_get(int nowait, int type)
{
struct mbuf *m;
+ struct counters_ref cr;
+ uint64_t *counters;
+ int s;
+
+ KDASSERT(type < MT_NTYPES);
m = pool_get(&mbpool, nowait == M_WAIT ? PR_WAITOK : PR_NOWAIT);
if (m == NULL)
return (NULL);
- mtx_enter(&mbstatmtx);
- mbstat.m_mtypes[type]++;
- mtx_leave(&mbstatmtx);
+ s = splnet();
+ counters = counters_enter(&cr, mbstat);
+ counters[type]++;
+ counters_leave(&cr, mbstat);
+ splx(s);
m->m_type = type;
m->m_next = NULL;
@@ -230,14 +246,21 @@ struct mbuf *
m_gethdr(int nowait, int type)
{
struct mbuf *m;
+ struct counters_ref cr;
+ uint64_t *counters;
+ int s;
+
+ KDASSERT(type < MT_NTYPES);
m = pool_get(&mbpool, nowait == M_WAIT ? PR_WAITOK : PR_NOWAIT);
if (m == NULL)
return (NULL);
- mtx_enter(&mbstatmtx);
- mbstat.m_mtypes[type]++;
- mtx_leave(&mbstatmtx);
+ s = splnet();
+ counters = counters_enter(&cr, mbstat);
+ counters[type]++;
+ counters_leave(&cr, mbstat);
+ splx(s);
m->m_type = type;
@@ -349,13 +372,18 @@ struct mbuf *
m_free(struct mbuf *m)
{
struct mbuf *n;
+ struct counters_ref cr;
+ uint64_t *counters;
+ int s;
if (m == NULL)
return (NULL);
- mtx_enter(&mbstatmtx);
- mbstat.m_mtypes[m->m_type]--;
- mtx_leave(&mbstatmtx);
+ s = splnet();
+ counters = counters_enter(&cr, mbstat);
+ counters[m->m_type]--;
+ counters_leave(&cr, mbstat);
+ splx(s);
n = m->m_next;
if (m->m_flags & M_ZEROIZE) {
Index: sys/mbuf.h
