Re: relayd panic

2022-06-05 Thread Alexandr Nedvedicky
should probably get committed. > > > On 2022/06/01 09:16, Alexandr Nedvedicky wrote: > > Hello, > > > > > > > r420-1# rcctl -f start relayd > > > relayd(ok) > > > r420-1# uvm_fault(0xfd862f82f990, 0x0, 0, 1) -> e > > > kernel:

pfctl reports back a table as being always created/added

2022-06-14 Thread Alexandr Nedvedicky
Hello, Jason Mc Intyre (jmc@) reported a bug earlier today reaching me by email off-list. Let me quote from Jason's email: i already have a pf table, adding an address tells me i have created a table. even though the table already existed: # pfctl -tbrutes -Tshow | wc

Re: unlock pf_purge

2022-06-07 Thread Alexandr Nedvedicky
Hello, I like this change and I think this should go in as-is. This is OK sashan@ I also think we should revisit a pf.conf(5) manpage where 'interval' (PFTM_INTERVAL) is described. It currently reads as follows: set timeout variable value frag Seconds before an

Re: pf: pool for pf_anchor

2022-07-20 Thread Alexandr Nedvedicky
Hello, On Tue, Jul 19, 2022 at 10:58:08PM +0200, Alexander Bluhm wrote: > On Tue, Jul 19, 2022 at 07:18:54PM +0200, Moritz Buhl wrote: > > A solution would be to move the allocation of the pf_anchor struct > > into a pool. One final question would be regarding the size of the > > hiwat or

fix NAT round-robin and random

2022-07-20 Thread Alexandr Nedvedicky
Hello, below is a final version of patch for NAT issue discussed at bugs@ [1]. Patch below is updated according to feedback I got from Chris, claudio@ and hrvoje@. The summary of changes is as follows: - prevent infinite loop when packet hits NAT rule as follows: pass out on em0

Re: pf: DIOCXCOMMIT and copyin

2022-07-21 Thread Alexandr Nedvedicky
Hello, On Thu, Jul 21, 2022 at 11:13:28AM +0200, Moritz Buhl wrote: > Hi tech, > > for the other two DIOCX ioctls syzkaller showed that it is possible > to grab netlock while doing copyin. > > The same problem should exist for DIOCXCOMMIT but syzkaller didn't > find it yet. > > In case anybody

Re: [External] : Re: pf igmp icmp6 multicast router alert

2022-04-27 Thread Alexandr Nedvedicky
Hello, On Thu, Apr 28, 2022 at 12:00:09AM +0200, Alexander Bluhm wrote: > On Fri, Apr 22, 2022 at 07:40:17PM +0200, Alexandr Nedvedicky wrote: > > > + case IPPROTO_ICMPV6: > > > + if (!pf_pull_hdr(pd->m, pd->off, , sizeof(icmp6), > > >

Re: [External] : pfsync mutex mpfloor

2022-04-13 Thread Alexandr Nedvedicky
On Wed, Apr 13, 2022 at 09:33:52PM +0200, Alexander Bluhm wrote: > Hi, > > Hrvoje has hit a witness issue in pfsync. > > panic: acquiring blockable sleep lock with spinlock or critical > section held (kernel_lock) _lock > > panic(81f45bb7) at panic+0xbf >

Re: [External] : Re: pf igmp icmp6 multicast router alert

2022-04-28 Thread Alexandr Nedvedicky
Hello, On Thu, Apr 28, 2022 at 12:36:40AM +0200, Alexander Bluhm wrote: > On Wed, Apr 27, 2022 at 11:47:45PM +0200, Alexander Bluhm wrote: > > New diff: > > - make off and end relative to opts array > > - check length of IPv4 options > > - fix call to pf_walk_option > > - add case IP6OPT_PADN > >

Re: [External] : another copyout while holding net/pf lock in pfioctl

2022-04-28 Thread Alexandr Nedvedicky
Hello Moritz, I like your change. There is just one nit see comment below. > > > Index: sys/net/pf_if.c > === > RCS file: /cvs/src/sys/net/pf_if.c,v > retrieving revision 1.103 > diff -u -p -r1.103 pf_if.c > --- sys/net/pf_if.c

Re: [External] : Re: pf igmp icmp6 multicast router alert

2022-04-28 Thread Alexandr Nedvedicky
Hello, On Thu, Apr 28, 2022 at 10:31:33PM +0200, Alexander Bluhm wrote: > > Thanks. regress/sys/netinet6/frag6 found a small issue. If the > icmp6 header is fragmented, we cannot pull the icmp6 header. I had > to copy the fragment check to the beginning of case IPPROTO_ICMPV6. > > This chunk

Re: [External] : Re: add sanity checks to IGMP/MLD

2022-05-02 Thread Alexandr Nedvedicky
Hello, > > Checking that the TTL equals 1 is a good thing. We should prevent > that someone is forwarding such packets. > > The router alert is a hint to routers on the way to look at these > packets. If they are missing, no harm is done. Maybe some multicast > does not work. But there is

add sanity checks to IGMP/MLD

2022-05-02 Thread Alexandr Nedvedicky
hello, bluhm@ has committed a fix [1] which makes pf to accept IGMP/MLD messages. If I remember correct pf(4) was dropping those messages because of Router Alert IP option being present. The IP option is mandatory for IGMP/MLD according to RFCs. For both protocol versions (IPv4, IPv6) standards

Re: [External] : Re: add sanity checks to IGMP/MLD

2022-05-03 Thread Alexandr Nedvedicky
Hello On Tue, May 03, 2022 at 10:44:48AM +0200, Claudio Jeker wrote: > > The RFC does not use the usual MUST to enforce any of this. > So yes, we should probably not be too strict because there is no way to > force accept the packet when pf_walk_header() returns PF_DROP. > > I agree that the

Re: [External] : Re: add sanity checks to IGMP/MLD

2022-05-03 Thread Alexandr Nedvedicky
On Tue, May 03, 2022 at 02:12:36PM +0200, Claudio Jeker wrote: > On Tue, May 03, 2022 at 02:08:33PM +0200, Alexandr Nedvedicky wrote: > > Hello > > > > On Tue, May 03, 2022 at 10:44:48AM +0200, Claudio Jeker wrote: > > > > > > > > The R

Re: [External] : Re: add sanity checks to IGMP/MLD

2022-05-03 Thread Alexandr Nedvedicky
Hello, On Tue, May 03, 2022 at 09:19:44AM +0200, Alexander Bluhm wrote: > On Tue, May 03, 2022 at 12:26:52AM +0200, Alexandr Nedvedicky wrote: > > OK ? or should I also drop a check for link-local source address > > in IPv6? > > The link-local check makes sense. > >

Re: [External] : Re: another syzkaller problem in pf

2022-05-04 Thread Alexandr Nedvedicky
On Tue, May 03, 2022 at 09:31:51PM +0200, Alexander Bluhm wrote: > On Tue, May 03, 2022 at 07:42:34PM +0200, Moritz Buhl wrote: > > commit 4b3977248902c22d96aaebdb5784840debc2631c > > Author: mikeb > > Date: Mon Nov 24 13:22:09 2008 + > > > > Fix splasserts seen in pr 5987 by

pf.conf(5) clarify ICMP sloppy state handling

2022-05-08 Thread Alexandr Nedvedicky
Hello, this tiny update to pf.conf(5) has been prompted here [1] on pf mailing list. By default only ICMP queries are allowed to create state in pf(4). The sloppy option relaxes that so also ICMP replies can create a state. I think this should be also mentioned in pf.conf(5) OK to my suggestion

move memory allocation in pfr_add_tables() outside of NET_LOCK()/PF_LOCK()

2022-05-08 Thread Alexandr Nedvedicky
Hello, diff below reshuffles pfr_create_ktable() so all memory allocations happens outside of NET_LOCK()/PF_LOCK(). pf(4) currently allocates for new table with PR_WAITOK flag, when function is invoked on behalf of ioctl (PFR_FLAG_USERIOCTL flag is set). PR_WAITOK for memory allocation is OK if

Re: [External] : net lock priority

2022-05-08 Thread Alexandr Nedvedicky
Hello, On Fri, May 06, 2022 at 09:50:30PM +0200, Alexander Bluhm wrote: > Hi, > > When creating network load by forwarding packets, SSH gets unusable > and ping time gets above 10 seconds. > > Problem is that while multiple forwarding threads are running with > shared net lock, the exclusive

Re: [External] : Re: pf.conf(5) clarify ICMP sloppy state handling

2022-05-08 Thread Alexandr Nedvedicky
Hello, On Sun, May 08, 2022 at 08:06:57PM +0200, Alexander Bluhm wrote: > On Sun, May 08, 2022 at 06:37:57PM +0200, Alexandr Nedvedicky wrote: > > this tiny update to pf.conf(5) has been prompted here [1] on > > pf mailing list. By default only ICMP queries are allowed > > to

Re: [External] : net lock priority

2022-05-08 Thread Alexandr Nedvedicky
Hello, On Sun, May 08, 2022 at 07:31:46PM +0200, Alexandr Nedvedicky wrote: > Hello, > > On Fri, May 06, 2022 at 09:50:30PM +0200, Alexander Bluhm wrote: > > Hi, > > > > When creating network load by forwarding packets, SSH gets unusable > > and ping time gets a

Re: [External] : net lock priority

2022-05-08 Thread Alexandr Nedvedicky
Hello, On Sun, May 08, 2022 at 10:13:20PM +0200, Alexander Bluhm wrote: > On Sun, May 08, 2022 at 09:19:23PM +0200, Alexander Bluhm wrote: > > I will run my tests with the diff below. > > With the third chunk reboot hangs during reordering libraries in > vmmaplk. So this needs more thought.

Re: [External] : net lock priority

2022-05-09 Thread Alexandr Nedvedicky
Hello, On Mon, May 09, 2022 at 04:34:00PM +0200, Alexander Bluhm wrote: > On Sun, May 08, 2022 at 10:54:01PM +0200, Alexandr Nedvedicky wrote: > > what bothers me is the situation where there are > > more than one reader. The line 350 is executed by > > the fir

Re: [External] : Re: move memory allocation in pfr_add_tables() outside of NET_LOCK()/PF_LOCK()

2022-05-09 Thread Alexandr Nedvedicky
Hello, thanks for taking a look. > > + SLIST_FOREACH(q, , pfrkt_workq) { > > + if (!pfr_ktable_compare(p, q)) { > > + /* > > +* We need no lock here, because `p` is empty, > > +* there

Re: [External] : Re: pf.conf(5) clarify ICMP sloppy state handling

2022-05-09 Thread Alexandr Nedvedicky
Hello, I'm sorry I was too fast with commit. I've just committed what's been suggested by bluhm@: @@ -2186,6 +2186,7 @@ It cannot be used with .Cm modulate state or .Cm synproxy state . +With this option ICMP replies can create states. .It Ar timeout seconds

Re: [External] : Re: move memory allocation in pfr_add_tables() outside of NET_LOCK()/PF_LOCK()

2022-05-09 Thread Alexandr Nedvedicky
Hello, On Tue, May 10, 2022 at 12:18:15AM +0200, Alexander Bluhm wrote: > On Mon, May 09, 2022 at 11:11:03PM +0200, Alexandr Nedvedicky wrote: > > > ... and then we insert a destroyed p > > > > yes. you are right. new diff addresses that with change as follows: > &

Re: [External] : divert packet mutex

2022-05-05 Thread Alexandr Nedvedicky
Hello, On Wed, May 04, 2022 at 10:50:15PM +0200, Alexander Bluhm wrote: > Hi, > > I missed that pf divert packet is not MP safe yet. The problem is > that divert_packet() is called from pf with shared net lock and > sbappendaddr() needs exclusive net lock. > > The direct call from pf in IP

Re: [External] : Re: another syzkaller problem in pf

2022-05-04 Thread Alexandr Nedvedicky
On Wed, May 04, 2022 at 04:26:18PM +0200, Alexander Bluhm wrote: > On Wed, May 04, 2022 at 02:21:11PM +0200, Alexandr Nedvedicky wrote: > > I'm not sure flipping a flag is a right change. In general we don't want > > to hold NET_LOCK()/PF_LOCK() while waiting for memory.

Re: fix NAT round-robin and random

2022-08-02 Thread Alexandr Nedvedicky
Hello, On Mon, Aug 01, 2022 at 06:37:58PM +0200, Hrvoje Popovski wrote: > On 20.7.2022. 22:27, Alexandr Nedvedicky wrote: > > Hello, > > > > below is a final version of patch for NAT issue discussed at bugs@ [1]. > > Patch below is updated according to feedbac

Re: make kernel build without INET6 again (pf_lb.c)

2022-08-30 Thread Alexandr Nedvedicky
surre, looks good to me. OK sashan On Tue, Aug 30, 2022 at 09:45:17PM +0200, Sebastian Benoit wrote: > ok? > > diff --git sys/net/pf_lb.c sys/net/pf_lb.c > index 588115cbff7..905af42e463 100644 > --- sys/net/pf_lb.c > +++ sys/net/pf_lb.c > @@ -519,13 +519,18 @@ pf_map_addr(sa_family_t af,

Re: pf fragment lock

2022-08-22 Thread Alexandr Nedvedicky
Hello, On Mon, Aug 22, 2022 at 08:45:29PM +0200, Alexander Bluhm wrote: > Hi, > > Hrvoje managed to crash the kernel in pf fragment reassembly. > > > r620-1# pfctl -e > > pf enabled > > r620-1# pfctl -f /etc/pf.conf > > uvm_fault(0x824b9278, 0xb7, 0, 2) -> e > > kernel: page fault trap,

yet another follow-up to pfr_add_tables()

2022-10-04 Thread Alexandr Nedvedicky
Hello, diff below fixes my a tiny glitch I've introduced with commit [1] back in May. Fortunately the impact is hardly noticeable (I think). The current code reads as follows: 1495 pfr_add_tables(struct pfr_table *tbl, int size, int *nadd, int flags) 1496 { 1507 for (i = 0; i <

Re: pf.conf / scrub resulting in invalid checksum

2022-10-10 Thread Alexandr Nedvedicky
Hello, On Mon, Oct 10, 2022 at 06:52:00AM +0200, Bjorn Ketelaars wrote: > > (reply also send to tech@) > > In 2011 henning@ removed fiddling with the ip checksum of normalised > packets in sys/net/pf_norm.c (r1.131). Rationale was that the checksum > is always recalculated in all output paths

Re: store pf rules in a tree

2022-08-10 Thread Alexandr Nedvedicky
Hello, On Wed, Aug 10, 2022 at 02:38:16PM +, Stefan Butz wrote: > Hi everyone, > > this mail includes a patch to store pf rules in a red-black tree. > Currently they are stored in a linked list. > My system configured with 16000 rules takes about 10 minutes > to print them out using `pfctl

Re: ipv4 and ipv6 fragment refactoring

2022-08-12 Thread Alexandr Nedvedicky
Hello, On Thu, Aug 11, 2022 at 11:59:55AM +0200, Alexander Bluhm wrote: > Hi, > > ip_fragment() and ip6_fragment() do nearly the same thing, but they > are implemented differently. > > The idea of this diff is to move things around. Then only differences > from the IP standards but not in

Re: ip6 routing header 0 offset

2022-08-12 Thread Alexandr Nedvedicky
Hello, On Thu, Aug 11, 2022 at 09:42:54PM +0200, Alexander Bluhm wrote: > Hi, > > The IPv6 routing header type 0 check should modify *offp only in > case of an error, so that the genrated icmp6 packet has the correct > pointer. After successful return, *offp should not be modified. makes

Re: store pf rules in a tree

2022-08-12 Thread Alexandr Nedvedicky
Hello Stefan, On Wed, Aug 10, 2022 at 02:38:16PM +, Stefan Butz wrote: > Hi everyone, > > this mail includes a patch to store pf rules in a red-black tree. > Currently they are stored in a linked list. > My system configured with 16000 rules takes about 10 minutes > to print them out using

Re: [External] : inpcb lookup ref counting

2022-08-08 Thread Alexandr Nedvedicky
Hello, diff looks good to me as far as I can tell. OK sashan@

patch for 7.2 to fix pfsync panics in pf_state_export()

2023-01-09 Thread Alexandr Nedvedicky
Hello, if you don't use pfsync on 7.2 stop reading now. There are two reports already [1] which have the same cause: NULL pointer dereference in pf_state_export(). dlg@ has fixed this bug in CUURENT back in December [2]. Unfortunately the patch for 7.2 needs a manual merge, because CURRENT

Re: move the pf_state_tree type around

2023-01-02 Thread Alexandr Nedvedicky
Hello, On Tue, Jan 03, 2023 at 03:31:42PM +1000, David Gwynne wrote: > the pf_state_tree type represents the rb tree that pf_state_key structs > go into. currently pf_state_key is declared in pfvar_priv.h (because > it's a kernel private data structure) but pf_state_tree was left in > pfvar.h.

Re: more consistently use "st" as the name for struct pf_state variables

2023-01-05 Thread Alexandr Nedvedicky
Hello, On Wed, Jan 04, 2023 at 09:36:38PM +1000, David Gwynne wrote: > and "stp" for pf_state ** variables. > I agree with established naming conventions. I'm also fine with keeping some exceptions such as `a` and `b` in pf_state_compare_id(), local variables `tail`, `head` in

Re: move the pf_state_tree_id type around

2023-01-04 Thread Alexandr Nedvedicky
Hello, I agree with change as-is. Though I have some suggestions for few finishing touches. see comments inline. On Wed, Jan 04, 2023 at 01:15:54PM +1000, David Gwynne wrote: > move the pf_state_tree_id type from pfvar.h to pfvar_priv.h. > > this is like the pf_state_tree change from yesterday.

Re: help pfsync by extending pf_state_key lifetimes on pf_states

2022-12-04 Thread Alexandr Nedvedicky
Hello, On Sat, Dec 03, 2022 at 09:53:45AM +1000, David Gwynne wrote: > we (mostly sashan@ and me) have a problem where pfsync can be holding a > reference to a pf_state that pf has decided to purge, and then pfsync > crashes because it tries to read the pf_state_key parts of the state, > but

Re: help pfsync by extending pf_state_key lifetimes on pf_states

2022-12-05 Thread Alexandr Nedvedicky
Hello, On Mon, Dec 05, 2022 at 11:46:07AM +1000, David Gwynne wrote: > > yes, you're right. the diff below includes the simple fix to that. > > this demonstrates how subtle the reference counting around the trees > is though. maybe it would be more obvious to take another refcnt > before giving

Re: help pfsync by extending pf_state_key lifetimes on pf_states

2022-12-05 Thread Alexandr Nedvedicky
Hello, > > pf_test_rule uses the skw and sks pointers after pf_state_insert sets > them via pf_create_state. i would happily change pf_test rule so it > reads the pf_state_key pointers out of pf_state rather than carry them > around on the stack like that, but i figured the diff was big enough

Re: move pf_state_key and pf_state_item structs from pfvar.h to pfvar_priv.h

2022-12-15 Thread Alexandr Nedvedicky
Hello, On Fri, Dec 16, 2022 at 01:53:42PM +1000, David Gwynne wrote: > both struct pf_state_key and pf_state_item are kernel private data > structures, and do not need to be visible to userland. > > i would like to move them to pfvar_priv.h to make it more obvious that > they are and should

Re: rename pf_state_key and pf_state_item members

2022-12-19 Thread Alexandr Nedvedicky
Hello, I don't object. diff improces things. Just few bike shedding nits further below. On Mon, Dec 19, 2022 at 04:48:57PM +1000, David Gwynne wrote: > > i hope i didn't mess up the (sk_)states list traversals. could not spot anything wrong there. > > ok? > > Index: pf.c >

Re: rename pf_state_key and pf_state_item members

2022-12-19 Thread Alexandr Nedvedicky
Hello, On Tue, Dec 20, 2022 at 08:10:30AM +1000, David Gwynne wrote: > > > - si->s->direction != s->direction))) { > > > + TAILQ_FOREACH(si, >sk_states, si_entry) { > > > + struct pf_state *tst = si->si_st; > > > > appreciate consistency in your

Re: interface hooks to pf(4) should be using PF_LOCK()/PF_UNLOCK()

2022-11-22 Thread Alexandr Nedvedicky
Hello, this change is required to unhook pf(4) from NET_LOCK(). therefore I'd like to get it in. On Mon, Nov 07, 2022 at 04:51:59AM +1000, David Gwynne wrote: > > > > On 7 Nov 2022, at 4:12 am, Alexandr Nedvedicky wrote: > > > > Hello, > > > > diff bel

Re: splassert on boot

2022-11-23 Thread Alexandr Nedvedicky
Hello, On Thu, Nov 24, 2022 at 08:51:51AM +1000, David Gwynne wrote: > im ok with this, but you need sashan@ to ok it too. he's been working up to > this anyway. > > dlg > > > On 24 Nov 2022, at 06:18, Vitaliy Makkoveev wrote: > > > > On Wed, Nov 23, 2022 at 02:59:05PM -0500, David Hill

Re: pfsync slpassert on boot and panic

2022-11-25 Thread Alexandr Nedvedicky
Hello Hrvoje, On Fri, Nov 25, 2022 at 09:57:15AM +0100, Hrvoje Popovski wrote: > Hi, > > I think that this is similar problem as what David Hill send on tech@ > with subject "splassert on boot" > > I've checkout tree few minutes ago and in there should be > mvs@ "Remove netlock assertion

make state key dereference safe for pfsync(4)

2022-11-16 Thread Alexandr Nedvedicky
Hello, with state key mutex in a tree [1]. I'd like to add yet another diff. during h2k22 David and I split original change [2] into two chunks. OK to commit diff below? thanks and regards sashan [1] https://marc.info/?l=openbsd-cvs=166817856414079=2 [2]

Re: pfsync panic after pf_purge backout

2022-11-28 Thread Alexandr Nedvedicky
Hello, > > > Hi, > > here's panic with WITNESS and this diff on tech@ > https://www.mail-archive.com/tech@openbsd.org/msg72582.html > > I will stop now because I'm not sure what I'm doing and which diffs I'm > testing... > > > r620-1# uvm_fault(0x8248ea28, 0x17, 0, 2) -> e >

Re: pfsync panic after pf_purge backout

2022-11-26 Thread Alexandr Nedvedicky
Hello, On Sat, Nov 26, 2022 at 08:33:28PM +0100, Hrvoje Popovski wrote: > I just need to say that with all pf, pfsync and with pf_purge diffs > after hackaton + this diff on tech@ > https://www.mail-archive.com/tech@openbsd.org/msg72582.html > my production firewall seems stable and it wasn't

Re: more consistently use "st" as the name for struct pf_state variables

2023-01-05 Thread Alexandr Nedvedicky
On Thu, Jan 05, 2023 at 08:32:44PM +1000, David Gwynne wrote: > > > > On 5 Jan 2023, at 18:56, Alexandr Nedvedicky wrote: > > > > Hello, > > > > On Wed, Jan 04, 2023 at 09:36:38PM +1000, David Gwynne wrote: > >> and "stp" for pf_state **

Re: unlock pf_purge

2022-11-06 Thread Alexandr Nedvedicky
Hello, > > claudio had some questions about numbers used by the diff. some of the > maths here is set up so that every pf state purge run is guaranteed to > do a MINUMUM amount of work. by default pf is configured with a purge > interfval of 10 seconds, bu the pf state purge processing runs

Re: make /dev/pf a clonable device

2022-11-06 Thread Alexandr Nedvedicky
On Sun, Nov 06, 2022 at 10:42:49PM +1000, David Gwynne wrote: > this is a small chunk to help sashan@ out with some of the pf ioctl work > he is doing. > > he is looking at allocating config over multiple ioctls, and would like > to be able to throw it away in situations like if the userland

Re: MAKEDEV: there's only one pf(4) device

2022-11-06 Thread Alexandr Nedvedicky
On Sun, Nov 06, 2022 at 01:06:02PM +, Klemens Nanni wrote: > Just like bpf(4)... except bpf actually also creates bpf0 still. > > $ ls -1 /dev/{b,}pf* > /dev/bpf > /dev/bpf0 > /dev/pf > > OK? looks ok to me, I think no on one expects to create /dev/pf0, /dev/pf1

Re: MAKEDEV: there's only one pf(4) device

2022-11-06 Thread Alexandr Nedvedicky
On Sun, Nov 06, 2022 at 04:48:11PM +, Klemens Nanni wrote: > On Sun, Nov 06, 2022 at 05:43:15PM +0100, Alexandr Nedvedicky wrote: > > On Sun, Nov 06, 2022 at 01:06:02PM +, Klemens Nanni wrote: > > > Just like bpf(4)... except bpf actually also creates bpf0 still. >

interface hooks to pf(4) should be using PF_LOCK()/PF_UNLOCK()

2022-11-06 Thread Alexandr Nedvedicky
Hello, diff below is the first step to make pfioctl() _without_ NET_LOCK(). Currently pf_if.c seems to be the only blocker which prevents us from removing all NET_LOCK()/NET_UNLOCK() calls we have in pf(4). diff below passed very basic smoke test. OK to commit? thanks and regards sashan

Re: tweak pfsync status output in ifconfig

2022-11-08 Thread Alexandr Nedvedicky
On Wed, Nov 09, 2022 at 12:09:50AM +1000, David Gwynne wrote: > i'm hacking on pfsync(4) at the moment, and i wasted way too much time > wondering how i broke the pfsync ioctls after i didn't the pfsync_status > output. turns out if you don't have a sync interface set, it skips > output. > > i

Re: adding a mutex to pf_state

2022-11-10 Thread Alexandr Nedvedicky
Hello, David (dlg@) asked me to shrink the scope of the change. The new diff introduces a mutex to pf_state and adjust pf_detach_state() to utilize the newly introduced mutex. I've also added annotation to pf_state members. The members with '?' needs our attention. We need to verify if they are

relax KASSET() to if () in pfsync_grab_snapshot()

2022-11-11 Thread Alexandr Nedvedicky
Hello, Diff below changes KASSERT() to if (). We have to prevent packets to insert state to snapshot queue multiple times. Hrvoje@ can trigger situation where state updates to pfsync peer are more frequent than we are able to send out. OK to go for this simple fix/workaround ? thanks and

simplify handling of 'once' rules in pf(4)

2022-11-07 Thread Alexandr Nedvedicky
Hello, diff below simplifies handling of 'once' rules in pf(4). Currently matching packet marks 'once' rule as expired and puts it to garbage collection list, where the rule waits to be removed from its ruleset by timer. diff below simplifies that. matching packet marks once rule as expired and

Re: simplify handling of 'once' rules in pf(4)

2022-11-07 Thread Alexandr Nedvedicky
Hello, resending the same diff, just updated to current. (pointed out by dlg@) thanks and regards sashan 8<---8<---8<--8< diff --git a/sys/net/pf.c b/sys/net/pf.c index 2c6124e74f2..6295b4eb9d7 100644 --- a/sys/net/pf.c +++ b/sys/net/pf.c

Re: simplify handling of 'once' rules in pf(4)

2022-11-07 Thread Alexandr Nedvedicky
Hello, updated diff. It buys suggestions from Klemens: pf.conf(5) section which covers once rules reads as follows: onceCreates a one shot rule. The first matching packet marks rule as expired. The expired rule is never evaluated then. pfctl(8) does not

adding a mutex to pf_state

2022-11-09 Thread Alexandr Nedvedicky
hello, diff below adds a mutex to pf_state. It fixes a NULL pointer dereference panic reported by Hrvoje sometime ago [1]. Besides adding a mutex to state the diff addresses a race between pfsync and state purge thread. What happened in this particular case was that state expired and its state

pf(4) drops valid IGMP/MLD messages

2023-02-25 Thread Alexandr Nedvedicky
Hello, the issue has been discovered and analyzed by Luca di Gregorio on bugs@ [1]. The thread [1] covers all details. People who wish to read brief summary can skip to Luca's email [2]. To wrap it up the current handling IGMP/MLD messages in pf(4) is exact opposite. I failed to translate

Re: pf(4) drops valid IGMP/MLD messages

2023-02-26 Thread Alexandr Nedvedicky
Hello, > > 8<---8<---8<--8< > > diff --git a/sys/net/pf.c b/sys/net/pf.c > > index 8cb1326a160..c328109026c 100644 > > --- a/sys/net/pf.c > > +++ b/sys/net/pf.c > > @@ -6847,7 +6847,7 @@ pf_walk_header(struct pf_pdesc *pd, struct ip *h, >

Re: assert fail in pfsync_grab_snapshot()

2023-02-21 Thread Alexandr Nedvedicky
Hello Lyndon, this assert has been removed in current (revision 1.310). The complete diff reads as follows: 8<---8<---8<--8< diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c index d279ede9cd6..64a2da195ab 100644 ---

Re: pf(4) drops valid IGMP/MLD messages

2023-03-03 Thread Alexandr Nedvedicky
Hello, On Fri, Mar 03, 2023 at 09:14:38PM +0100, Alexander Bluhm wrote: > On Fri, Mar 03, 2023 at 08:54:51AM +0100, Luca Di Gregorio wrote: > > Hi, just another bit of info about this issue. > > Instead of implementing more and more details of RFC, we should > discuss what the goal is. > > We

Re: pf(4) drops valid IGMP/MLD messages

2023-02-28 Thread Alexandr Nedvedicky
Hello Matthieu, On Tue, Feb 28, 2023 at 02:05:50PM +0100, Matthieu Herrb wrote: > > --- a/sys/net/pf.c > > +++ b/sys/net/pf.c > > @@ -6846,8 +6846,12 @@ pf_walk_header(struct pf_pdesc *pd, struct ip *h, > > u_short *reason) > > pd->proto = h->ip_p; > > /* IGMP packets have router alert

Re: DIOCGETRULE is slow for large rulesets (2/3)

2023-04-26 Thread Alexandr Nedvedicky
Hello, On Thu, Apr 27, 2023 at 06:51:52AM +1000, David Gwynne wrote: > > is that kind of check in KASSET() something you have on your mind? > > perhaps I can trade KASSERT() to regular code: > > > > if (t->pft_unit != minor(dev)) > > return (EPERM); > > i would pass the

Re: DIOCGETRULE is slow for large rulesets (3/3)

2023-04-26 Thread Alexandr Nedvedicky
Hello, updated diff below adopts to recent changes to pf_find_trans() in diff 2/3 [1] in case DIOCGETRULE does not find desired transaction the ioctl(2) call to /dev/pf returns ENXIO. thanks and regards sashan [1] https://marc.info/?l=openbsd-tech=168254555211083=2

Re: DIOCGETRULE is slow for large rulesets (2/3)

2023-04-27 Thread Alexandr Nedvedicky
Hello, Hello, On Thu, Apr 27, 2023 at 10:41:53AM +1000, David Gwynne wrote: > > t could be NULL here. just do the unit check inside the loop? > > > > > if (t->pft_unit != unit) > > return (NULL); > > > > return (t); > > } > > > > just return NULL on unit

DIOCGETRULE is slow for large rulesets (1/3)

2023-04-25 Thread Alexandr Nedvedicky
Hello, below is diff which renames ruleset member `ticket` to `version`. the reason for this is to keep things clean. The word `ticket` will be used to identify transaction, while newly introduced `version` identifies change of particular pf object (ruleset). diff below is simple find/replace.

DIOCGETRULE is slow for large rulesets (2/3)

2023-04-25 Thread Alexandr Nedvedicky
Hello, This is the second diff. It introduces a transaction (pf_trans). It's more or less diff with dead code. It's still worth to note those two chunks in this diff: @@ -1142,10 +1172,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p) return

DIOCGETRULE is slow for large rulesets (complete diff)

2023-04-25 Thread Alexandr Nedvedicky
Hello, below is a complete diff which makes DIOCGETRULE bit more sane. This is the complete change I'd like to commit. It is also more convenient for people who want to test the diff. when running 'pfctl -sr' to show rules pfctl performs several ioctl(2) calls. The first call is DIOCGETRULES to

DIOCGETRULE is slow for large rulesets (3/3)

2023-04-25 Thread Alexandr Nedvedicky
Hello, this is 3rd of three diffs to improve DIOCGETRULE ioctl operation. the summary of changes done here is as follows: - add new members to pf_trans structure so it can hold data for DIOCGETRULE operation - DIOCGETRULES opens transaction and returns ticket/transaction id

Re: DIOCGETRULE is slow for large rulesets (2/3)

2023-04-26 Thread Alexandr Nedvedicky
Hello, On Wed, Apr 26, 2023 at 11:51:26AM +, Gerhard Roth wrote: > On Wed, 2023-04-26 at 13:47 +0200, Alexandr Nedvedicky wrote: > > @@ -293,6 +300,28 @@ pfopen(dev_t dev, int flags, int fmt, struct proc *p) > > ??int > > ??pfclose(dev_t dev, int flags, in

Re: DIOCGETRULE is slow for large rulesets (2/3)

2023-04-26 Thread Alexandr Nedvedicky
Hello, On Wed, Apr 26, 2023 at 11:37:58AM +1000, David Gwynne wrote: > > fail: > > - if (flags & FWRITE) > > - rw_exit_write(_rw); > > - else > > - rw_exit_read(_rw); > > + rw_exit_write(_rw); > > i dont think having the open mode flags affect whether you take a

Re: DIOCGETRULE is slow for large rulesets (3/3)

2023-04-26 Thread Alexandr Nedvedicky
Hello, On Wed, Apr 26, 2023 at 01:25:45AM +0200, Alexandr Nedvedicky wrote: > Hello, > > this is 3rd of three diffs to improve DIOCGETRULE ioctl operation. > the summary of changes done here is as follows: > - add new members to pf_trans structure so it can > hold da

Re: pfioctl: DIOCGETQUEUE: drop net lock

2023-04-28 Thread Alexandr Nedvedicky
Hello, On Fri, Apr 28, 2023 at 09:02:35PM +, Klemens Nanni wrote: > Same logic and argument as for the parent *S ioctl, might as well have > committed them together: > --- > Remove net lock from DIOCGETQUEUES > > Both ticket and number of queues stem from the pf_queues_active

Re: pfctl + bgpd for loop ugliness

2023-04-21 Thread Alexandr Nedvedicky
Hello, On Tue, Apr 18, 2023 at 02:43:26PM +0200, Theo Buehler wrote: > On Tue, Apr 18, 2023 at 02:06:46PM +0200, Claudio Jeker wrote: > > This and the others are IIRC streight from pfctl. So if someone wants a > > free commit :) > > How about this. pfctl and bgpd are the same, except that the

Re: divert packet checksum

2023-04-04 Thread Alexandr Nedvedicky
Hello, On Mon, Apr 03, 2023 at 11:39:54PM +0200, Alexander Bluhm wrote: > Hi, > > When sending IP packets to userland with divert-packet rules, the > checksum may be wrong. Locally generated packets diverted by pf > out rules may have no checksum due to to hardware offloading. > IDS/IPS systems

Re: pf.conf bug

2023-02-06 Thread Alexandr Nedvedicky
Hello, [ cc'ing also tech@ ] On Mon, Feb 06, 2023 at 06:44:38PM +0300, r...@bh0.amt.ru wrote: > >Synopsis:pf.conf bug > >Category:system > >Environment: > System : OpenBSD 7.2 > Details : OpenBSD 7.2 (GENERIC.MP) #6: Sat Jan 21 01:03:04 MST 2023 >

Re: pf max-src-{states,conn} without overload/flush useless?

2023-02-08 Thread Alexandr Nedvedicky
Hello, I did test similar rules on my system OpenBSD 7.2-current (GENERIC.MP) #978: Sun Jan 22 11:41:04 MST 2023 these are my rules: set skip on lo block return# block stateless traffic pass out log# establish keep-state pass in on iwn0 proto tcp from

Re: pf max-src-{states,conn} without overload/flush useless?

2023-02-09 Thread Alexandr Nedvedicky
Hello, On Wed, Feb 08, 2023 at 09:42:11PM -0600, joshua stein wrote: > $ for i in `seq 5` ; do nc 192.168.1.240 22 & done > [2] 68892 > [3] 6303 > [4] 63554 > [5] 87833 > [6] 49997 > $ SSH-2.0-OpenSSH_9.1 > SSH-2.0-OpenSSH_9.1 > SSH-2.0-OpenSSH_9.1 > SSH-2.0-OpenSSH_9.1 > SSH-2.0-OpenSSH_9.1 >

Re: bpf(4) "wait" timeouts

2023-02-12 Thread Alexandr Nedvedicky
Hello, This diff looks good to me. Though I still have some doubts about accuracy of comment here: > return (kn->kn_data > 0); > @@ -1510,6 +1599,15 @@ bpf_catchpacket(struct bpf_d *d, u_char > ++d->bd_dcount; > return; > } > + >

fix NULL pointer dereference in pfsync_bulk_update()

2023-02-12 Thread Alexandr Nedvedicky
Hello, this bug has been found and reported by Hrvoje@ [1]. I took my chance and asked Hrvoje to test a small diff [2]. I would like to ask for OK to commit this fix which makes Hrvoje's test box happy. Diff below is same to one found at bugs@. The thing is that pfsync_bulk_update() function must

Re: fix NULL pointer dereference in pfsync_bulk_update()

2023-02-14 Thread Alexandr Nedvedicky
Hello, On Tue, Feb 14, 2023 at 01:23:16AM +0100, Alexander Bluhm wrote: > On Mon, Feb 13, 2023 at 08:39:39AM +0100, Alexandr Nedvedicky wrote: > > this bug has been found and reported by Hrvoje@ [1]. > > I took my chance and asked Hrvoje to test a small diff [2]. > > I wou

Re: DIOCGETRULE is slow for large rulesets (complete diff)

2023-04-28 Thread Alexandr Nedvedicky
Hello, On Fri, Apr 28, 2023 at 11:18:18AM +, Klemens Nanni wrote: > On Fri, Apr 28, 2023 at 12:55:37PM +0200, Alexandr Nedvedicky wrote: > > Hello, > > [ sending off-list ] > > > > up-to-date complete diff. > > Builds, boots and works faster: > >

Re: huge pfsync rewrite

2023-07-03 Thread Alexandr Nedvedicky
Hello, I went through the recent diff one more time. I could not spot anything wrong. Also my home router was happy with it for quite some time. Unfortunately I'm not using pfsync. However I'm sure hrvoje@ done his best to try to make it to crash and no luck diff survived. Having said earlier it

Re: pf(4) should mention DIOCXEND

2023-07-05 Thread Alexandr Nedvedicky
Hello, On Wed, Jul 05, 2023 at 11:10:11AM +0200, Alexandr Nedvedicky wrote: > > thanks for your help to put my update to pf(4) to shape. > updated diff is below. > diff in my earlier email was wrong. this one is the right one. sorry for extra noise. regards sasha

Re: pf(4) should mention DIOCXEND

2023-07-05 Thread Alexandr Nedvedicky
Hello, diff below includes suggestions from jmc@ and kn@ I'll commit it later today. thanks and regards sashan 8<---8<---8<--8< diff --git a/share/man/man4/pf.4 b/share/man/man4/pf.4 index 92eeb45f657..e9d4231fe97 100644 ---

Re: pf(4) should mention DIOCXEND

2023-07-05 Thread Alexandr Nedvedicky
Hello Jason, thank you for taking a look. More comments in-line. On Tue, Jul 04, 2023 at 09:03:29PM +0100, Jason McIntyre wrote: > > @@ -48,12 +48,25 @@ and retrieve statistics. > > The most commonly used functions are covered by > > .Xr pfctl 8 . > > .Pp > > -Manipulations like loading a

pf(4) should mention DIOCXEND

2023-07-04 Thread Alexandr Nedvedicky
Hello, diff below updates pf(4) manpage to reflect changes [1] which were committed earlier today. does update to pf(4) read OK? thanks and regards sashan [1] https://marc.info/?l=openbsd-cvs=168848058603797=2 https://marc.info/?l=openbsd-cvs=168847042626997=2

Re: pfioctl: drop net lock from SIOC{S,G}LIMIT

2023-05-25 Thread Alexandr Nedvedicky
Hello, On Thu, May 25, 2023 at 07:14:54AM +, Klemens Nanni wrote: > On Thu, May 25, 2023 at 03:28:45AM +, Klemens Nanni wrote: > > On Thu, May 25, 2023 at 03:20:04AM +, Klemens Nanni wrote: > > > pfsync_in_bus() looks like the only place where the static array > > > pf_pool_limits[]

pf(4) may cause relayd(8) to abort

2023-07-31 Thread Alexandr Nedvedicky
Hello, the issue has been reported by Gianni Kapetanakis month ago [1]. It took several emails to figure out relayd(8) exists after hosts got disabled by 'relayctl host dis ...' The thing is that relayd(8) relies on pf(4) to create persistent tables (PFR_TFLAG_PERSIST) as relayd requests that:

<    1   2   3   4   5   6   7   >