Backout always allocate per-CPU statistics counters for network
interface descriptor. It panics during attach of em(4) device at
boot.

Always allocate per-CPU statistics counters for network interface
descriptor.

We have the mess in network interface statistics. Only pseudo drivers
do per-CPU counters allocation, all other network

Always allocate per-CPU statistics counters for network interface
descriptor.

We have the mess in network interface statistics. Only pseudo drivers
do per-CPU counters allocation, all other network devices use the old
`if_data'. The network stack partially uses per-CPU counters and
partially use `if_data', but the protection is inconsistent: some times
counters accessed with exclusive netlock, some times with shared
netlock, some times with kernel lock, but without netlock, some times
with another locks.

To make network interfaces statistics more consistent, always allocate
per-CPU counters at interface attachment time and use it instead of
`if_data'. At this step only move counters allocation to the if_attach()
internals. The `if_data' removal will be performed with the following
diffs to make review and tests easier.

ok bluhm

show more ...

Initialize `sc_outputtask' before interface attachment. if_alloc_sadl()
has sleep point, so the uninitialized `sc_outputtask` could be accessed
through ioctl(2) interface.

ok sashan bluhm

Rework pflowioctl() lock dances.

Release netlock and take `sc_lock' rwlock(9) just in the beginning of
pflowioctl() and do corresponding operations in the end. Use `sc_lock'
to protect `sc_dying'.

Rework pflowioctl() lock dances.

Release netlock and take `sc_lock' rwlock(9) just in the beginning of
pflowioctl() and do corresponding operations in the end. Use `sc_lock'
to protect `sc_dying'.

We need to release netlock not only to keep locks order with `sc_lock'
rwlock(9), but also because pflowioctl() calls some operations like
socreate() or soclose() on udp(4) socket. Current implementation has
many relocking places which breaks atomicy, so merge them into one.

The `sc_lock' rwlock(9) is taken during all pflowioctl() call, so
`sc_dying' atomicy is not broken.

Not the ideal solution, but better then we have now.

Tested by Hrvoje Popovski.

Discussed with and ok from sashan

show more ...

slyle(9) fix. No functional changes.

Turn `pflowstats' statistics counters into per-CPU counters to make them
mpsafe.

The weird interactions around `pflow_flows' and `sc_gcounter' replaced
by simple `pflow_flows' increment. Since the f

Turn `pflowstats' statistics counters into per-CPU counters to make them
mpsafe.

The weird interactions around `pflow_flows' and `sc_gcounter' replaced
by simple `pflow_flows' increment. Since the flow sequence is the 32
bits integer, the `sc_gcounter' type replaced by the type of uint32_t.

ok bluhm sashan

show more ...

Turn `pflow_softc' list into SMR list.

Since the revision 1.1182 of net/pf.c netlock is not taken while
export_pflow() called from pf_purge_states(). Current locks order
requires netlock to be taken

Turn `pflow_softc' list into SMR list.

Since the revision 1.1182 of net/pf.c netlock is not taken while
export_pflow() called from pf_purge_states(). Current locks order
requires netlock to be taken before PF_LOCK(), so there is no reason
to turn it back into this path only for optional export_pflow() call.

The `pflowif_list' foreach loop has no context switch within, so SMR
list is better than mutex(9).

Tested by Hrvoje Popovski.

ok sashan bluhm

show more ...

Add spaces around '='. style(9) fix, no functional changes.

Introduce `sc_mtx' mutex(9) to protect the most of pflow_softc
structure. Protect the `send_nam', `sc_flowsrc' and `sc_flowdst'
pflow_softc members by existing `sc_lock' rwlock(9).

This partially fi

Introduce `sc_mtx' mutex(9) to protect the most of pflow_softc
structure. Protect the `send_nam', `sc_flowsrc' and `sc_flowdst'
pflow_softc members by existing `sc_lock' rwlock(9).

This partially fixes locking inconsistency of pflow_softc. The following
work will be done with separate diffs.

Also, pass `sc' instead of NULL to pflow_get_mbuf() while calling from
pflow_sendout_ipfix_tmpl(). This fixes the NULL dereference.

ok bluhm@

show more ...

Remove delayed timeout(9) initialization. timeout_set*() only assign
members of passed timeout structure, this delayed initialization
provides nothing but makes code weird.

ok kn

remove duplicate includes
ok deraadt@ miod@ krw@

Make `so' dereference safe within pflow_output_process().

sosend() has sleep points, so the kernel lock serialisation within
pflow_output_process() doesn't work as expected. The pflow(4) interface
a

Make `so' dereference safe within pflow_output_process().

sosend() has sleep points, so the kernel lock serialisation within
pflow_output_process() doesn't work as expected. The pflow(4) interface
associated socket `so' could be overwritten by concurrent pflowioctl()
thread.

Introduce pflow(4) interface's `sc_lock' rwlock(9) to make `so'
dereference safe. Since the solock() of udp(4) sockets uses netlock as
backend, the `sc_lock' should be taken first. This expands a little
netlock relocking within pflowioctl().

pflow_sendout_mbuf() called by pflow_output_process(), now called
without kernel lock held, so the mp safe counters_pkt(9) used instead
of manual `if_opackets' increment.

Since if_detach() does partial ifnet destruction, now it can't be called
before we finish pflow_output_process() task, otherwise we introduce use
after free for interface counters. In other hand, we need to deny
pflowioctl() to reschedule pflow_output_process() task. The `sc_dyind'
flag introduced for that.

Tested by Hrvoje Popovski.

ok bluhm@

show more ...

prepare for moving struct pf_state from pfvar.h to pfvar_priv.h

pflow obviously looks at the kernel pf state structure to do it's
thing, so it will need the header that provides it. i'm committing
t

prepare for moving struct pf_state from pfvar.h to pfvar_priv.h

pflow obviously looks at the kernel pf state structure to do it's
thing, so it will need the header that provides it. i'm committing
this chunk separately to the actual pf_state move to keep the commits
small and simple.

ok sashan@

show more ...

Remove unused fields from 'pflow_softc' structure.

ok bluhm@

Fix race between pflow_output_process() and pflow_clone_destroy().

Unlink pflow(4) interface from `pflowif_list' before start destruction to
prevent pflow_output_process() being rescheduled. Also wa

Fix race between pflow_output_process() and pflow_clone_destroy().

Unlink pflow(4) interface from `pflowif_list' before start destruction to
prevent pflow_output_process() being rescheduled. Also wait until running
pflow_output_process() task finished.

Problem reported and fix tested by Hrvoje Popovski.

ok bluhm@

show more ...

Simplify solock() and sounlock(). There is no reason to return a value
for the lock operation and to pass a value to the unlock operation.
sofree() still needs an extra flag to know if sounlock() sho

Simplify solock() and sounlock(). There is no reason to return a value
for the lock operation and to pass a value to the unlock operation.
sofree() still needs an extra flag to know if sounlock() should be called
or not. But sofree() is called less often and mostly without keeping the lock.
OK mpi@ mvs@

show more ...

Leave default ifq_maxlen handling to ifq_init()

Most clonable interface drivers (except bridge, enc, loop, pppx,
switch, trunk and vlan) initialise the send queue's length to IFQ_MAXLEN
during *_clo

Leave default ifq_maxlen handling to ifq_init()

Most clonable interface drivers (except bridge, enc, loop, pppx,
switch, trunk and vlan) initialise the send queue's length to IFQ_MAXLEN
during *_clone_create() even though ifq_init(), which is eventually called
through if_attach(), does the same.

Remove all early "ifq_set_maxlen(&ifq->if_snd, IFQ_MAXLEN);" lines to leave
it to ifq_init() and have clonable drivers a tad more in sync.

OK mvs

show more ...

Change users of IFQ_SET_MAXLEN() and IFQ_IS_EMPTY() to use the "new" API.

ok dlg@ tobhe@

kernel: use gettime(9)/getuptime(9) in lieu of time_second(9)/time_uptime(9)

time_second(9) and time_uptime(9) are widely used in the kernel to
quickly get the system UTC or system uptime as a time_

kernel: use gettime(9)/getuptime(9) in lieu of time_second(9)/time_uptime(9)

time_second(9) and time_uptime(9) are widely used in the kernel to
quickly get the system UTC or system uptime as a time_t. However,
time_t is 64-bit everywhere, so it is not generally safe to use them
on 32-bit platforms: you have a split-read problem if your hardware
cannot perform atomic 64-bit reads.

This patch replaces time_second(9) with gettime(9), a safer successor
interface, throughout the kernel. Similarly, time_uptime(9) is replaced
with getuptime(9).

There is a performance cost on 32-bit platforms in exchange for
eliminating the split-read problem: instead of two register reads you
now have a lockless read loop to pull the values from the timehands.
This is really not *too* bad in the grand scheme of things, but
compared to what we were doing before it is several times slower.

There is no performance cost on 64-bit (__LP64__) platforms.

With input from visa@, dlg@, and tedu@.

Several bugs squashed by visa@.

ok kettenis@

show more ...

Use FNONBLOCK instead of SS_NBIO to check/indicate that the I/O mode
for sockets is non-blocking.

This allows us to G/C SS_NBIO. Having to keep the two flags in sync
in a mp-safe way is complicated

Use FNONBLOCK instead of SS_NBIO to check/indicate that the I/O mode
for sockets is non-blocking.

This allows us to G/C SS_NBIO. Having to keep the two flags in sync
in a mp-safe way is complicated.

This change introduce a behavior change in sosplice(), it can now
always block. However this should not matter much due to the socket
lock being taken beforhand.

ok bluhm@, benno@, visa@

show more ...

use 'switch' rathan than an if-else construct to match on the netflow version to in pflow_get_mbuf(),
to match the rest of the file

ok benno@ florian@

Pass the socket to sounlock(), this prepare the terrain for per-socket
locking.

ok visa@, bluhm@

Remove almost unused `flags' argument of suser().

The account flag `ASU' will no longer be set but that makes suser()
mpsafe since it no longer mess with a per-process field.

No objection from mill

Remove almost unused `flags' argument of suser().

The account flag `ASU' will no longer be set but that makes suser()
mpsafe since it no longer mess with a per-process field.

No objection from millert@, ok tedu@, bluhm@

show more ...

Creating a cloned interface could return ENOMEM due to temporary
memory shortage. As it is invoked from a system call, it should
not fail and wait instead.
OK visa@ mpi@

- add one more softnet taskq
NOTE: code still runs with single softnet task. change definition of
SOFTNET_TASKS in net/if.c, if you want to have more than one softnet task

OK mpi@, OK phessler@