Re: pipex(4): kill pipexintr()

2020-07-06 Thread Vitaliy Makkoveev
On Mon, Jul 06, 2020 at 08:47:23PM +0200, Martin Pieuchot wrote: > On 06/07/20(Mon) 19:23, Vitaliy Makkoveev wrote: > > > On 6 Jul 2020, at 17:36, Martin Pieuchot wrote: > > [...] > > Unfortunately you can’t be sure about NET_LOCK() status while you are > > in p

Re: userland clock_gettime proof of concept

2020-07-06 Thread Vitaliy Makkoveev
> On 5 Jul 2020, at 20:31, Paul Irofti wrote: > > On Fri, Jul 03, 2020 at 06:36:39PM +0300, Paul Irofti wrote: >> >> >> În 3 iulie 2020 17:55:25 EEST, Mark Kettenis a >> scris: Date: Fri, 3 Jul 2020 15:13:22 +0200 From: Robert Nagy On 02/07/20 00:31 +0100, Stuart

Re: userland clock_gettime proof of concept

2020-07-06 Thread Vitaliy Makkoveev
Sorry for late reaction. At least VirtualBox based virtual machines started to panic with the recent kernel. I reverted your diff and panics stopped. Screenshot attached.

Re: Make pipex more common for pppac and pppx

2020-08-16 Thread Vitaliy Makkoveev
On Sat, Aug 15, 2020 at 05:42:06PM +0900, YASUOKA Masahiko wrote: > Let me update the diff. A bug found by the test. > Hello Yasuoka. I like your idea to kill `pipex_iface_context'. I had trying to keep it by myself and this was wrong way. Could you rework your diff to be against the recent

Re: pppoe: start without kernel lock

2020-08-16 Thread Vitaliy Makkoveev
On Sun, Aug 16, 2020 at 08:44:07PM +0200, Klemens Nanni wrote: > On Sun, Aug 16, 2020 at 07:04:46PM +0200, Klemens Nanni wrote: > > Make sppp(4)/pppoe(4) use the ifq API to send packets outside the big > > lock. > > > > As far as I understand, pppoe_output() does not require NET_LOCK() since > >

Re: pppac(4): destroy sessions the same way as pppx(4) does

2020-08-16 Thread Vitaliy Makkoveev
On Sat, Aug 15, 2020 at 02:01:52PM +0900, YASUOKA Masahiko wrote: > On Wed, 12 Aug 2020 12:26:22 +0300 > Vitaliy Makkoveev wrote: > > We destroy pppx(4) related sessions while we performing PIPEXDSESSION > > command. But with pppac(4) we set session's state to > > PIPEX_

Re: Remove unnecessary field from struct msgbuf

2020-08-16 Thread Vitaliy Makkoveev
The diff looks good for me. I’ll recompile system with your diff tomorrow. > On 16 Aug 2020, at 14:35, Visa Hankala wrote: > > The msg_bufl field of struct msgbuf is written but never read. The value > was used by kernfs which is no longer present, so the code could be > cleaned up a little by

Re: pf log user and group

2021-01-11 Thread Vitaliy Makkoveev
ok mvs@ > On 11 Jan 2021, at 19:49, Alexander Bluhm wrote: > > Hi, > > Sometimes an uid is logged in pflog(4) although the logopt of the > rule does not specify it. Check the option again for the log rule > in case another rule has triggered a socket lookup. Remove logopt > group, it is not

Re: pair: send without kernel lock

2021-01-07 Thread Vitaliy Makkoveev
> On 7 Jan 2021, at 04:14, Klemens Nanni wrote: > > pair(4)'s output path can run without kernel lock just fine. > > NB: Looking at CVS log, it seems this was not done during import because > IFXF_MPSSAFE became a thing afterwards. > > Feedback? Objections? OK? > Looks good to me. ok mvs >

Re: tcpdump: use unsigned char in isprint

2020-11-30 Thread Vitaliy Makkoveev
On Sun, Nov 29, 2020 at 07:21:53PM -0800, Greg Steuck wrote: > Vitaliy Makkoveev writes: > > > Hi. > > > > For me `ch' is unwanted here. > > > > Index: usr.sbin/tcpdump/util.c > > === > &

Re: tcpdump: use unsigned char in isprint

2020-11-29 Thread Vitaliy Makkoveev
Hi. For me `ch' is unwanted here. Index: usr.sbin/tcpdump/util.c === RCS file: /cvs/src/usr.sbin/tcpdump/util.c,v retrieving revision 1.30 diff -u -p -r1.30 util.c --- usr.sbin/tcpdump/util.c 24 Jan 2020 22:46:37 - 1.30

Re: tcpdump: Don't link to libl and remove reference to yydebug

2020-12-03 Thread Vitaliy Makkoveev
> On 3 Dec 2020, at 13:20, Martin Vahlensieck > wrote: > > Hi Hi. > > This is unused. It has been in there since the import from NetBSD. > Their logs show that tcpgram.y and tcplex.l have been removed in 1995. > I am not sure what the policy is for the getopt(3) call: Should Y be > removed

Re: Remove variable count from kqueue_scan()

2020-12-14 Thread Vitaliy Makkoveev
ok mvs@ > On 14 Dec 2020, at 18:40, Visa Hankala wrote: > > In function kqueue_scan(), variables count and nkev follow relation > count + nkev == maxevents. The code becomes a bit simpler if count > is removed. > > OK? > > Index: kern/kern_event.c >

pipex(4)/npppd(8): remove dummy PIPEX{G,S}MODE ioctl(2) calls

2020-12-29 Thread Vitaliy Makkoveev
This time pipex(4) related ioctl(2) calls PIPEX{S,G}MODE are pretty dummy and were kept for backward compatibility reasons. The diff below removes them. ok? Index: share/man/man4/pipex.4 === RCS file:

pppoe_dispatch_disc_pkt() convert `off' argument to local variables

2020-12-29 Thread Vitaliy Makkoveev
pppoe_dispatch_disc_pkt() has `off' argument which is always passed as 0. The diff below converts this argument to local variable. Also this function breaks style(9) but I'll fix it with separate diff. ok? Index: sys/net/if_pppoe.c

Fix pppoe_dispatch_disc_pkt definition to be in accordance with style(9)

2020-12-30 Thread Vitaliy Makkoveev
Subj. Index: sys/net/if_pppoe.c === RCS file: /cvs/src/sys/net/if_pppoe.c,v retrieving revision 1.74 diff -u -p -r1.74 if_pppoe.c --- sys/net/if_pppoe.c 30 Dec 2020 12:10:39 - 1.74 +++ sys/net/if_pppoe.c 30 Dec 2020

Re: pppoe: input without kernel lock

2021-01-03 Thread Vitaliy Makkoveev
ok mvs@ > On 4 Jan 2021, at 00:23, Klemens Nanni wrote: > > On Wed, Dec 30, 2020 at 01:10:33AM +0300, Vitaliy Makkoveev wrote: >> For me these if_{get,put}(9) dances are useless. This `ph_ifidx’ is >> related to ethernet device and used to find pppoe(4) relate

pipex(4): remove unused `pipex_iface_context' struct

2021-01-03 Thread Vitaliy Makkoveev
Index: sys/net/pipex.h === RCS file: /cvs/src/sys/net/pipex.h,v retrieving revision 1.29 diff -u -p -r1.29 pipex.h --- sys/net/pipex.h 2 Jan 2021 13:15:15 - 1.29 +++ sys/net/pipex.h 3 Jan 2021 20:10:47 - @@

Re: uvm_fault: access_type fixup for wired mapping

2021-01-13 Thread Vitaliy Makkoveev
On Tue, Jan 12, 2021 at 09:41:15AM -0300, Martin Pieuchot wrote: > Diff below moves `access_type' to the context structure passed down to > the various routines and fix a regression introduced in a previous > refactoring. > > `access_type' is overwritten for wired mapping and the value of >

Re: bpf_mtap_ether doesnt need to encode packet priority

2021-01-16 Thread Vitaliy Makkoveev
ok mvs@ > On 15 Jan 2021, at 04:14, David Gwynne wrote: > > bpf should be showing what will be or has been on the wire, which is > what the ether_vtag in the mbuf has. the prio is either about to be > decoded from the tag on the wya into the stack, or has been encoded by > vlan(4) on the way

Re: bpf(4) doesn't have to keep track of nonblocking state itself

2021-01-19 Thread Vitaliy Makkoveev
ok mvs@ > On 19 Jan 2021, at 03:11, David Gwynne wrote: > > vfs does it for us. > > ok? > > Index: bpf.c > === > RCS file: /cvs/src/sys/net/bpf.c,v > retrieving revision 1.202 > diff -u -p -r1.202 bpf.c > --- bpf.c 17 Jan

Re: IPPROTO_SCTP

2021-01-18 Thread Vitaliy Makkoveev
ok mvs@ On Mon, Jan 18, 2021 at 12:13:32PM +, Stuart Henderson wrote: > can I add IPPROTO_SCTP to in.h? only one port wants it at the > moment, but I think I've seen others in the past. > > Index: netinet/in.h > === > RCS file:

struct process: mark `ps_oppid' as atomic

2021-01-13 Thread Vitaliy Makkoveev
Subj. It's integer so assignments are atomic. Index: sys/sys/proc.h === RCS file: /cvs/src/sys/sys/proc.h,v retrieving revision 1.304 diff -u -p -r1.304 proc.h --- sys/sys/proc.h 11 Jan 2021 13:55:53 - 1.304 +++

Re: Cache parent's pid as `ps_ppid' and use it instead of `ps_pptr->ps_pid'.

2021-01-13 Thread Vitaliy Makkoveev
On Wed, Jan 13, 2021 at 02:10:25PM -0300, Martin Pieuchot wrote: > On 02/01/21(Sat) 21:54, Vitaliy Makkoveev wrote: > > This allows us to unlock getppid(2). Also NetBSD, DragonflyBSD and OSX > > do the same. > > Seems the way to go, two comments below. > > > @@ -69

Re: route sourceaddr: simplify code & get out of ART

2021-01-23 Thread Vitaliy Makkoveev
Hello. According the code `ifaddr’ struct has `ifa_refcnt’ field. Also it seems `ifa’ could exist while corresponding `ifp’ was destroyed. Is this true for `rt’ case? Should `ifa_refcnt' be bumped while you return `ifa’? > On 9 Jan 2021, at 20:50, Denis Fondras wrote: > > This diff place the

Unlock getppid(2)

2021-01-17 Thread Vitaliy Makkoveev
Now we cache parent's pid as atomic `ps_ppid'. It's time to unlock getppid(2). Index: sys/kern/syscalls.master === RCS file: /cvs/src/sys/kern/syscalls.master,v retrieving revision 1.207 diff -u -p -r1.207 syscalls.master ---

Re: sysctl ip.forwarding 2

2021-01-15 Thread Vitaliy Makkoveev
On Fri, Jan 15, 2021 at 02:07:56PM +0100, Alexander Bluhm wrote: > Hi, > > As documented in sysctl(2) net.inet.ip.forwarding can be 2. > > netinet/ip_output.c:448 > if (ipsec_in_use && (flags & IP_FORWARDING) && (ipforwarding == 2) && > > Current input validation prevents this. > #

rw_assert_wrlock((9) man page fix

2021-01-15 Thread Vitaliy Makkoveev
rw_assert_wrlock(9) has prototype "void rw_assert_wrlock(struct rwlock *)". Index: share/man/man9/rwlock.9 === RCS file: /cvs/src/share/man/man9/rwlock.9,v retrieving revision 1.25 diff -u -p -r1.25 rwlock.9 ---

Re: pppoe: input without kernel lock

2021-01-04 Thread Vitaliy Makkoveev
Still ok by me. > On 4 Jan 2021, at 03:46, Klemens Nanni wrote: > > On Tue, Dec 29, 2020 at 11:18:26PM +0100, Claudio Jeker wrote: >> Generally I would prefer to go for direct dispatch and not use netisr. >> This removes a queue and a scheduling point and should help reduce the >> latency in

bridge(4): don't call if_deactivate() at destroy.

2021-01-01 Thread Vitaliy Makkoveev
Don't call if_deactivate() in bridge_clone_destroy(). Following if_detach() will do this. Index: sys/net/if_bridge.c === RCS file: /cvs/src/sys/net/if_bridge.c,v retrieving revision 1.345 diff -u -p -r1.345 if_bridge.c ---

switch(4): don't call if_deactivate() at destroy.

2021-01-01 Thread Vitaliy Makkoveev
Don't call if_deactivate() in switch_clone_destroy(). Following if_detach() will do this. Index: sys/net/if_switch.c === RCS file: /cvs/src/sys/net/if_switch.c,v retrieving revision 1.38 diff -u -p -r1.38 if_switch.c ---

Cache parent's pid as `ps_ppid' and use it instead of `ps_pptr->ps_pid'.

2021-01-02 Thread Vitaliy Makkoveev
This allows us to unlock getppid(2). Also NetBSD, DragonflyBSD and OSX do the same. Index: kern/exec_elf.c === RCS file: /cvs/src/sys/kern/exec_elf.c,v retrieving revision 1.156 diff -u -p -r1.156 exec_elf.c --- kern/exec_elf.c 7

Re: pppoe: input without kernel lock

2020-12-29 Thread Vitaliy Makkoveev
Hi. > On 29 Dec 2020, at 22:48, Klemens Nanni wrote: > > Earlier this year `struct pppoe_softc' was annotated with lock comments > showing no member being protected by KERNEL_LOCK() alone. > > After further review of the code paths starting from pppoeintr() I also > could not find sleeping

Re: npppd: result of getifaddrs() not used?

2020-12-29 Thread Vitaliy Makkoveev
> On 30 Dec 2020, at 01:08, Sebastian Benoit wrote: > > It seems to me that this call to getifaddrs() is actually not needed. > > ok? Indeed it is. ok mvs > > diff --git usr.sbin/npppd/pppoe/pppoed.c usr.sbin/npppd/pppoe/pppoed.c > index 5b3f09dccb1..bae41732199 100644 > ---

PF_UNIX sockets unlocking

2021-02-04 Thread Vitaliy Makkoveev
The latest version of the PF_UNIX sockets unlocking diff. The new `unp_lock' rwlock(9) was used as solock()'s backend to protect the whole layer. We release solock() while we perform all vnode(9) related operations. Since fifo subsystem uses unix sockets as backend this is required to enforce

npppd(8)/pppac(4): remove dummy TUNSIFMODE ioctl(2) call

2021-01-29 Thread Vitaliy Makkoveev
Since OpenBSD 6.7 npppd(8) can't work over tun(4) anymore. I propose to remove dummy TUNSIFMODE ioctl(2) call. Index: sys/net/if_pppx.c === RCS file: /cvs/src/sys/net/if_pppx.c,v retrieving revision 1.106 diff -u -p -r1.106 if_pppx.c

Re: ipsec crypto kernel lock

2021-06-16 Thread Vitaliy Makkoveev
Hi, crypto_getreq() and crypto_freereq() don’t require kernel lock. > On 16 Jun 2021, at 23:05, Alexander Bluhm wrote: > > Hi, > > I have seen a kernel crash with while forwarding and encrypting > much traffic through OpenBSD IPsec gateways running iked. > > kernel: protection fault trap,

Re: ifnewlladdr spl

2021-06-12 Thread Vitaliy Makkoveev
Is it expected interrupt handlers modify ifp->if_flags? > On 10 Jun 2021, at 20:17, Alexander Bluhm wrote: > > Hi, > > I have seen this crash trace on a 6.6 based system, but I think the > bug exists still in -current. It happened when an ixl(4) interface > was removed from trunk(4). > >

Re: ipsec crypto kernel lock

2021-06-17 Thread Vitaliy Makkoveev
On Thu, Jun 17, 2021 at 03:19:11PM +0200, Alexander Bluhm wrote: > On Thu, Jun 17, 2021 at 10:09:47AM +0200, Martin Pieuchot wrote: > > It's not clear to me which field is the KERNEL_LOCK() protecting. Is it > > the access to `swcr_sessions'? Is it a reference? If so grabbing/releasing > > the

Re: ipsec crypto kernel lock

2021-06-16 Thread Vitaliy Makkoveev
ok mvs@ > On 17 Jun 2021, at 01:06, Alexander Bluhm wrote: > > On Wed, Jun 16, 2021 at 11:58:48PM +0300, Vitaliy Makkoveev wrote: >> crypto_getreq() and crypto_freereq() don???t require kernel lock. > > Indeed, new diff. > > ok? > >

Re: ifnewlladdr spl

2021-06-18 Thread Vitaliy Makkoveev
On Fri, Jun 18, 2021 at 07:18:53PM +0200, Alexander Bluhm wrote: > On Thu, Jun 17, 2021 at 12:27:42PM +0300, Vitaliy Makkoveev wrote: > > > Is this the correct version of the diff? Not tested yet. > > It survived a full regress run. > > > Hypothetically we could have

crypto(9): remove unused members from 'cryptocap' structure

2021-06-18 Thread Vitaliy Makkoveev
I'm lurking in crypto(9) and I found 'cryptocap' structure has unused members. Do we want to keep them or they could gone? We don't export this structure so this diff doesn't break ABI. Index: sys/crypto/crypto.c === RCS file:

Re: ifnewlladdr spl

2021-06-17 Thread Vitaliy Makkoveev
On Thu, Jun 17, 2021 at 02:03:03AM +0200, Alexander Bluhm wrote: > Thanks dlg@ for the hint with the ixl(4) fixes in current. I will > try so solve my 6.6 problem with that. > > On Wed, Jun 16, 2021 at 10:19:03AM +0200, Martin Pieuchot wrote: > > The diff discussed in this thread reduces the

Re: running network stack forwarding in parallel

2021-05-13 Thread Vitaliy Makkoveev
On Thu, May 13, 2021 at 01:15:05PM +0200, Hrvoje Popovski wrote: > On 13.5.2021. 1:25, Vitaliy Makkoveev wrote: > > It seems this lock order issue is not parallel diff specific. > > > > Yes, you are right ... it seemed familiar but i couldn't reproduce it > on lapc tr

Re: Use atomic op for UVM map refcount

2021-05-18 Thread Vitaliy Makkoveev
On Tue, May 18, 2021 at 01:24:42PM +0200, Martin Pieuchot wrote: > On 18/05/21(Tue) 12:07, Mark Kettenis wrote: > > > Date: Tue, 18 May 2021 12:02:19 +0200 > > > From: Martin Pieuchot > > > > > > This allows us to not rely on the KERNEL_LOCK() to check reference > > > counts. > > > > > > Also

Re: Fix IPsec NAT-T for L2TP/IPsec

2021-05-12 Thread Vitaliy Makkoveev
On Wed, May 12, 2021 at 07:11:09PM +0900, YASUOKA Masahiko wrote: > Hi, > > Radek reported a problem to misc@ that multiple Windows clients behind a NAT > cannot use a L2TP/IPsec server simultaneously. > > https://marc.info/?t=16099681611=1=2 > > There is two problems. First is pipex(4)

Re: Fix IPsec NAT-T for L2TP/IPsec

2021-05-12 Thread Vitaliy Makkoveev
> On 12 May 2021, at 18:42, YASUOKA Masahiko wrote: > > On Wed, 12 May 2021 17:26:51 +0300 > Vitaliy Makkoveev wrote: >> On Wed, May 12, 2021 at 07:11:09PM +0900, YASUOKA Masahiko wrote: >>> Radek reported a problem to misc@ that multiple Windows clients behind

Re: Fix IPsec NAT-T for L2TP/IPsec

2021-05-12 Thread Vitaliy Makkoveev
ok mvs@ > On 13 May 2021, at 02:43, YASUOKA Masahiko wrote: > > On Wed, 12 May 2021 19:15:29 +0300 > Vitaliy Makkoveev wrote: >>> On 12 May 2021, at 18:42, YASUOKA Masahiko wrote: >>> On Wed, 12 May 2021 17:26:51 +0300 >>> Vitaliy Makkoveev wrote:

Re: running network stack forwarding in parallel

2021-05-12 Thread Vitaliy Makkoveev
It seems this lock order issue is not parallel diff specific. > On 12 May 2021, at 12:58, Hrvoje Popovski wrote: > > On 21.4.2021. 21:36, Alexander Bluhm wrote: >> We need more MP preassure to find such bugs and races. I think now >> is a good time to give this diff broader testing and commit

Re: Fix IPsec NAT-T for L2TP/IPsec

2021-05-13 Thread Vitaliy Makkoveev
On Thu, May 13, 2021 at 09:20:22AM +0900, YASUOKA Masahiko wrote: > On Wed, 12 May 2021 19:11:09 +0900 (JST) > YASUOKA Masahiko wrote: > > Radek reported a problem to misc@ that multiple Windows clients behind > > a NAT cannot use a L2TP/IPsec server simultaneously. > > > >

Re: running network stack forwarding in parallel

2021-05-16 Thread Vitaliy Makkoveev
> On 14 May 2021, at 14:43, Martin Pieuchot wrote: > > On 13/05/21(Thu) 14:50, Vitaliy Makkoveev wrote: >> On Thu, May 13, 2021 at 01:15:05PM +0200, Hrvoje Popovski wrote: >>> On 13.5.2021. 1:25, Vitaliy Makkoveev wrote: >>>> It seems this lock order

Re: let ipv4_check remember if the ip checksum was good

2021-06-02 Thread Vitaliy Makkoveev
On Wed, Jun 02, 2021 at 11:09:13AM +1000, David Gwynne wrote: > if a bridge checked the ipv4 checksum and it was good, we can avoid > checking it again in ip_input. > > ok? > ok mvs@ > Index: ip_input.c > === > RCS file:

Re: timeout.h: remove API documentation comment

2021-05-28 Thread Vitaliy Makkoveev
On Thu, May 27, 2021 at 06:29:51PM -0500, Scott Cheloha wrote: > All of this information (and more) is in the timeout(9) manpage. > > Can I kill this comment? ok mvs@ > > Index: timeout.h > === > RCS file:

Re: Kill SS_ASYNC

2021-06-01 Thread Vitaliy Makkoveev
On Sat, May 29, 2021 at 10:33:17AM +0200, Martin Pieuchot wrote: > The socket flag SS_ASYNC is redundant with the socket buffer flag > SB_ASYNC. Both are set & unset via FIOASYNC. So the diff below gets > rid of SS_ASYNC. > > Checking states on socket buffers will help reduce the scope of the >

Re: forwarding in parallel with ipsec panic

2021-07-07 Thread Vitaliy Makkoveev
On Wed, Jul 07, 2021 at 02:09:23PM +0200, Alexander Bluhm wrote: > On Wed, Jul 07, 2021 at 12:52:26PM +0200, Hrvoje Popovski wrote: > > On 7.7.2021. 12:46, Hrvoje Popovski wrote: > > > Panic can be triggered when i have parallel diff and sending traffic > > Different panic on same setup ... > >

Re: forwarding in parallel with ipsec panic

2021-07-07 Thread Vitaliy Makkoveev
On Wed, Jul 07, 2021 at 11:07:08PM +0200, Hrvoje Popovski wrote: > On 7.7.2021. 22:36, Vitaliy Makkoveev wrote: > > Thanks. ipsp_spd_lookup() stopped panic in pool_get(9). > > > > I guess the panics continue because simultaneous modifications of > > 'tdbp->tdb_policy

Re: forwarding in parallel with ipsec panic

2021-07-07 Thread Vitaliy Makkoveev
On Wed, Jul 07, 2021 at 10:01:59PM +0200, Hrvoje Popovski wrote: > On 7.7.2021. 19:38, Vitaliy Makkoveev wrote: > > Hi, > > > > It seems the first the first panic occured because ipsp_spd_lookup() > > modifies tdbp->tdb_policy_head and simultaneous execution breaks

Re: forwarding in parallel with ipsec panic

2021-07-08 Thread Vitaliy Makkoveev
On Thu, Jul 08, 2021 at 08:08:23AM +0200, Hrvoje Popovski wrote: > On 8.7.2021. 0:10, Vitaliy Makkoveev wrote: > > On Wed, Jul 07, 2021 at 11:07:08PM +0200, Hrvoje Popovski wrote: > >> On 7.7.2021. 22:36, Vitaliy Makkoveev wrote: > >>> Thanks. ipsp_spd_lookup

Re: forwarding in parallel

2021-07-09 Thread Vitaliy Makkoveev
Hi, If I understood your diff right, pipex(4) is also affected through: ip_local() -> ip_deliver() -> (*pr_input)() -> gre_input() -> gre_input_key() -> gre_input_1() -> pipex_pptp_input() > On 7 Jul 2021, at 00:05, Alexander Bluhm wrote: > >

Re: crypto(9): remove unused members from 'cryptocap' structure

2021-07-09 Thread Vitaliy Makkoveev
On Tue, Jun 29, 2021 at 05:25:41PM +0200, Alexander Bluhm wrote: > On Fri, Jun 18, 2021 at 11:49:25PM +0300, Vitaliy Makkoveev wrote: > > I'm lurking in crypto(9) and I found 'cryptocap' structure has unused > > members. Do we want to keep them or they could gone? > >

Re: Do not spin on the NET_LOCK() in kqueue

2021-07-10 Thread Vitaliy Makkoveev
Hi, In filt_solisten_common() you touches `so_qlen’ only. It’s not related to buffer and not protected by introduced `sb_mtx’ so the solock() replacement in filt_solisten*() is wrong. However, in filt_solisten_common() you only checks is `so_qlen’ != 0 condition and such check could be performed

ipsec: document locks of some structures

2021-07-10 Thread Vitaliy Makkoveev
The diff below documents locks of 'ipsec_ids', 'ipsec_acquire' and 'ipsec_policy' structures. I marked `ipa_pcb' as immutable, but we never set or access it and I want to remove it with the next diff. Index: sys/netinet/ip_ipsp.h

ipsec: remove unused `PolicyHead' from 'sockaddr_encap' structure

2021-07-10 Thread Vitaliy Makkoveev
This member is never set or used. Also I kept 'SENT_IP6' definition for prevent the potential break of third party software. Is it ok to redefine it to '0x0002'? At least openswan wants this [1]. 1. https://github.com/xelerance/Openswan/blob/master/include/openswan/ipsec_encap.h#L20 Index:

Re: Do not spin on the NET_LOCK() in kqueue

2021-07-10 Thread Vitaliy Makkoveev
Thanks for explanation. I missed the last commit to sys/kern/uipc_socket.c > On 10 Jul 2021, at 22:56, Martin Pieuchot wrote: > > On 10/07/21(Sat) 21:53, Vitaliy Makkoveev wrote: >> Hi, >> >> In filt_solisten_common() you touches `so_qlen’ only. It’s not >> re

Re: sysctl net.inet.ip.arpqueued read only

2021-04-29 Thread Vitaliy Makkoveev
On Thu, Apr 29, 2021 at 09:31:57AM -0700, Greg Steuck wrote: > Alexander Bluhm writes: > >> I like this too. I somehow got the impression that macros are severely > >> frowned upon and didn't offer this kind of interface before. > >> > >> If you get this submitted, I can do a pass through the

Re: Update the remaining SYSCTL_INT_READONLY cases

2021-05-01 Thread Vitaliy Makkoveev
> On 1 May 2021, at 19:08, Greg Steuck wrote: > > Vitaliy Makkoveev writes: > >> On Fri, Apr 30, 2021 at 10:14:31PM -0700, Greg Steuck wrote: >> Hi, you missing KERN_SYSVMSG, KERN_SYSVSEM, KERN_SYSVSHM variables. The >> rest diff is ok by me. > > G

Re: Update the remaining SYSCTL_INT_READONLY cases

2021-05-01 Thread Vitaliy Makkoveev
On Fri, Apr 30, 2021 at 10:14:31PM -0700, Greg Steuck wrote: > Vitaliy Makkoveev writes: > > > On Thu, Apr 29, 2021 at 09:31:57AM -0700, Greg Steuck wrote: > >> Alexander Bluhm writes: > >> >> I like this too. I somehow got the impression that macros are seve

Re: running network stack forwarding in parallel

2021-04-21 Thread Vitaliy Makkoveev
On Wed, Apr 21, 2021 at 09:36:11PM +0200, Alexander Bluhm wrote: > Hi, > > For a while we are running network without kernel lock, but with a > network lock. The latter is an exclusive sleeping rwlock. > > It is possible to run the forwarding path in parallel on multiple > cores. I use ix(4)

Re: arpinit_done arpinit()

2021-04-23 Thread Vitaliy Makkoveev
On Fri, Apr 23, 2021 at 11:19:01PM +0200, Alexander Bluhm wrote: > Hi, > > Setting variable arpinit_done is not MP save if we want ot execute > arp_rtrequest() in parallel. Move initialization to arpinit() > function. > > ok? > ok mvs@ > bluhm > > Index: netinet/if_ether.c >

Re: sysctl net.inet.ip.arpqueued read only

2021-04-23 Thread Vitaliy Makkoveev
> On 23 Apr 2021, at 20:39, Alexander Bluhm wrote: > > Hi, > > The variable la_hold_total contains the number of packets currently > in the arp queue. So the sysctl net.inet.ip.arpqueued must be read > only. In if_ether.c include the header with the decalration of > la_hold_total to ensure

Re: sysctl net.inet.ip.arpqueued read only

2021-04-27 Thread Vitaliy Makkoveev
On Tue, Apr 27, 2021 at 09:12:56PM +0200, Alexander Bluhm wrote: > On Tue, Apr 27, 2021 at 12:18:02PM -0600, Theo de Raadt wrote: > > Actually, your variation seems pretty good. Is there any reason to not > > use this type of define? > > This would look like this. > > I think sysctl_int() and

Re: [External] : arpcache mbuf if_output reinsert

2021-04-28 Thread Vitaliy Makkoveev
> On 29 Apr 2021, at 01:38, Alexandr Nedvedicky > wrote: > > Hello, > > >> Such a diff is below. I will test extensively towmorrow. If anyone >> wants to try now, be my guest. >> >> Is the comment correct? > >I think comment is not quite right. > > > > the packet gets inserted

Re: [External] : arp mbuf delist

2021-04-28 Thread Vitaliy Makkoveev
> On 28 Apr 2021, at 22:40, Alexander Bluhm wrote: > > On Wed, Apr 28, 2021 at 04:01:42PM +0200, Alexandr Nedvedicky wrote: >> On Wed, Apr 28, 2021 at 03:31:41PM +0200, Claudio Jeker wrote: >>> Another option is to assert on the condition. It is an error case >>> that should not exist. >>>

Re: [External] : arp mbuf delist

2021-04-28 Thread Vitaliy Makkoveev
> On 28 Apr 2021, at 23:40, Alexander Bluhm wrote: > > On Wed, Apr 28, 2021 at 11:13:05PM +0300, Vitaliy Makkoveev wrote: >> Also we have two cases where `la_mq??? is not empty: >> 1. This thread put it back while performed ifp->if_output >> 2. Concurrent thread

Re: arp mbuf delist

2021-04-27 Thread Vitaliy Makkoveev
> On 28 Apr 2021, at 00:45, Alexander Bluhm wrote: > > On Sun, Apr 25, 2021 at 06:08:17PM +1000, Jonathan Matthew wrote: >> On Sun, Apr 25, 2021 at 09:44:16AM +0200, Alexandr Nedvedicky wrote: >> This already exists, it's called mq_delist() > > Here is the diff with mq_delist(). > >> We'd

Re: arp locking

2021-04-27 Thread Vitaliy Makkoveev
On Tue, Apr 27, 2021 at 07:27:43PM +0200, Alexander Bluhm wrote: > Hi, > > I would like to document the locking mechanism of the global variables > in ARP. > > The global arp_list is protected by net lock. This is not sufficent > when we switch to shared netlock. So I added a mutex for

Re: [External] : arp mbuf delist

2021-04-27 Thread Vitaliy Makkoveev
On Wed, Apr 28, 2021 at 12:10:31AM +0200, Alexandr Nedvedicky wrote: > Hello, > > On Tue, Apr 27, 2021 at 11:45:19PM +0200, Alexander Bluhm wrote: > > On Sun, Apr 25, 2021 at 06:08:17PM +1000, Jonathan Matthew wrote: > > > On Sun, Apr 25, 2021 at 09:44:16AM +0200, Alexandr Nedvedicky wrote: > > >

Re: sysctl net.inet.ip.arpqueued read only

2021-04-28 Thread Vitaliy Makkoveev
On Wed, Apr 28, 2021 at 12:06:26PM +0200, Alexander Bluhm wrote: > On Tue, Apr 27, 2021 at 08:22:20PM -0700, Greg Steuck wrote: > > Vitaliy Makkoveev writes: > > > > > On Tue, Apr 27, 2021 at 09:12:56PM +0200, Alexander Bluhm wrote: > > >> On Tue, Apr 27, 202

Re: [External] : arpcache mbuf if_output reinsert

2021-04-28 Thread Vitaliy Makkoveev
> On 29 Apr 2021, at 02:48, Alexandr Nedvedicky > wrote: > > On Thu, Apr 29, 2021 at 02:30:48AM +0300, Vitaliy Makkoveev wrote: >> >> >>> On 29 Apr 2021, at 02:20, Alexandr Nedvedicky >>> wrote: >>> >>> >>>>

Re: [External] : arpcache mbuf if_output reinsert

2021-04-28 Thread Vitaliy Makkoveev
> On 29 Apr 2021, at 02:20, Alexandr Nedvedicky > wrote: > > >> >> This time arpcache() is only called by netisr() with both kernel and >> exclusive net locks held. RTM_DELETE message processing will also call >> ifp->if_rtrequest() with exclusive netlock held. >> >> So this while() loop

Re: sysctl net.inet.ip.arpqueued read only

2021-04-25 Thread Vitaliy Makkoveev
> On 26 Apr 2021, at 01:43, Theo de Raadt wrote: > > I am not a fan of this strange behaviour, where the min+max values > have additional behaviours. It is too surprising, and surprising > often turns into error-prone. Agreed. Also according sysctl_int_bounded() code this behaviour looks

pppac(4): remove `sc_dead' logic

2021-02-09 Thread Vitaliy Makkoveev
`sc_dead' is used to prevent pppac_ioctl() be called on dying pppac(4) interface. But now if_detach() makes dying `ifp' inaccessible and waits for references which are in-use. This logic is not required anymore. Also I moved if_detach() before klist_invalidate() to prevent the case while

Re: PF_UNIX sockets unlocking

2021-02-09 Thread Vitaliy Makkoveev
On Tue, Feb 09, 2021 at 05:20:33PM +0100, Alexander Bluhm wrote: > On Thu, Feb 04, 2021 at 03:07:44PM +0300, Vitaliy Makkoveev wrote: > > I hope someone else will try it and gives positive feedback which allow > > to push it forward. > > OK bluhm@ > Thanks. > >

Re: interface group name validation

2021-02-10 Thread Vitaliy Makkoveev
On Tue, Feb 09, 2021 at 11:08:09PM +0100, Alexander Bluhm wrote: > Hi, > > Next try to fix syzkaller crash > https://syzkaller.appspot.com/bug?id=54e16dc5bce6929e14b42e2f1379f1c18f62be43 > > Interface group names must fit into IFNAMSIZ and be unique. But > the kernel makes the unique check

route sockets: simplify route_attach() error path

2021-02-10 Thread Vitaliy Makkoveev
Do soreserve() before `rop' allocation. It doesn't require protocol control block be attached to socket. Also we always call `pr_attach' in thread context so we always have `curproc'. ok? Index: sys/net/rtsock.c === RCS file:

Re: isakmpd link dynamically

2021-02-10 Thread Vitaliy Makkoveev
On Wed, Feb 10, 2021 at 06:33:49PM +0100, Alexander Bluhm wrote: > Hi, > > Every time we ship a libcrypto erratum, we have to relink isakmpd. > I think that isakmpd and iked are in /sbin due to a historic mistake. > Probably it is for people who mount /usr via NFS over IPsec. > > Moving isakmpd

unlock sys_sendsyslog

2021-03-10 Thread Vitaliy Makkoveev
Since UNIX domain sockets are unlocked it makes sense to unlock sys_sendsyslog too. Console output still requires kernel lock to be held but this path is only followed while `syslogf' socket is not set. New `syslogf_rwlock' used to protect `syslogf' access. ok? Index: sys/kern/subr_log.c

Re: sendsyslog kernel buffer

2021-03-07 Thread Vitaliy Makkoveev
> On 7 Mar 2021, at 02:17, Alexander Bluhm wrote: > > Hi > > Early daemons like dhcpleased, slaacd, unwind, resolvd are started > before syslogd. This results in ugly sendsyslog: dropped 1 message > logs and the real message is lost. > > Changing the start order of syslogd and and network

Re: sendsyslog kernel buffer

2021-03-08 Thread Vitaliy Makkoveev
roblem with log data though. > > Is double copyin a problem? I think error != EFAULT should catch > all cases. > > On Mon, Mar 08, 2021 at 01:37:51AM +0300, Vitaliy Makkoveev wrote: >> I wonder they were not buffered. But does it make sense to drop the >> most recent me

Re: have m_copydata use a void * instead of caddr_t

2021-02-23 Thread Vitaliy Makkoveev
ok mvs@ > On 23 Feb 2021, at 12:31, David Gwynne wrote: > > i'm not a fan of having to cast to caddr_t when we have modern > inventions like void *s we can take advantage of. > > ok? > > Index: share/man/man9/mbuf.9 > === > RCS

Re: ip_fragment ip6_fragment

2021-02-27 Thread Vitaliy Makkoveev
> On 26 Feb 2021, at 14:08, Alexander Bluhm wrote: > > Hi, > > I always bothered me that ip_fragment() and ip6_fragment() behave > sligtly differently. Unify them and use an mlist to simplify the > fragment list. > > - The functions ip_fragment() and ip6_fragment() always consume the mbuf.

UNIX domain sockets: move garbage collector to `systqmp'

2021-02-16 Thread Vitaliy Makkoveev
There are no fallout reports after UNIX sockets unlocking, so I propose to move moves garbage collector to `systqmp'. unp_gc() touches nothing which requires kernel lock to be held. Index: sys/kern/uipc_usrreq.c === RCS file:

Re: ifg_refcnt atomic operation

2021-02-06 Thread Vitaliy Makkoveev
> On 6 Feb 2021, at 15:23, Alexander Bluhm wrote: > > On Sat, Feb 06, 2021 at 05:04:20PM +1000, David Gwynne wrote: >> refcnt_init starts counting at 1, while the existing code starts at 0. Do >> the crashes stop because we never fully release all the references and >> never free it now? > >

Re: veb(4), a virtual ethernet bridge (that could replace bridge(4)?)

2021-02-21 Thread Vitaliy Makkoveev
Hello. > + ifp->if_ioctl = veb_ioctl; > + ifp->if_input = veb_input; > + //ifp->if_rtrequest = veb_rtrequest; > + ifp->if_output = veb_output; > + ifp->if_enqueue = veb_enqueue; Could you replace c++ style comment in veb_clone_create()? > +veb_clone_destroy(struct ifnet

Re: UNIX domain sockets: move garbage collector to `systqmp'

2021-02-18 Thread Vitaliy Makkoveev
ping > On 16 Feb 2021, at 16:13, Vitaliy Makkoveev wrote: > > There are no fallout reports after UNIX sockets unlocking, so I propose > to move moves garbage collector to `systqmp'. unp_gc() touches nothing > which requires kernel lock to be held. > > Index: s

switch(4): fix netlock assertion within ifpromisc()

2021-02-19 Thread Vitaliy Makkoveev
As it was reported [1] switch(4) triggers NET_ASSERT_LOCKED() while we perform ifconfig(8) destroy. ifpromisc() requires netlock to be held. This is true while switch_port_detach() and underlay ifpromisc() called through switch_ioctl(). But while we destroy switch(4) interface we call ifpromisc()

Re: mbuf leak ip_insertoptions

2021-02-22 Thread Vitaliy Makkoveev
ok mvs@ > On 22 Feb 2021, at 18:51, Alexander Bluhm wrote: > > Hi, > > ip_insertoptions() may prepend a mbuf. In this case "goto bad" has > to free the new chain. Currently we leak the new mbuf in front of > the old chain. NetBSD has fixed this bug here: > > >

Re: syslogd iovcnt

2021-09-03 Thread Vitaliy Makkoveev
ok mvs@ > On 3 Sep 2021, at 19:04, Alexander Bluhm wrote: > > Hi, > > Use a define for the iov array size in syslogd. This is better > than passing the magic number 6 around and checking at runtime > whether its fits. > > ok? > > bluhm > > Index: usr.sbin/syslogd/syslogd.c >

Re: syslogd sendmsg

2021-09-10 Thread Vitaliy Makkoveev
On Fri, Sep 10, 2021 at 07:40:58PM +0200, Alexander Bluhm wrote: > Hi, > > After converting syslogd messages to iov, UDP can use sendmsg(2) > instead of snprintf(). > > ok? > I have one question below, but the diff is ok by mvs@ if it is not significant. It is expected we could drop

rtfree(): "rt->rt_refcnt > 0" assertion

2021-09-14 Thread Vitaliy Makkoveev
We have weird `rt_refcnt' check in rtfree(): 497 rtfree(struct rtentry *rt) 498 { ... 504 refcnt = (int)atomic_dec_int_nv(>rt_refcnt); 505 if (refcnt <= 0) { 506 KASSERT(!ISSET(rt->rt_flags, RTF_UP)); 507 KASSERT(!RT_ROOT(rt)); 508

<    1   2   3   4   5   6   7   8   >