[alexandr.nedvedi...@oracle.com: unused argument at pf_translate() function]

2014-06-21 Thread Alexandr Nedvedicky
Hello, resending to better alias as recommended by p...@benzedrine.cx subscribers. regards sasha - Forwarded message from Alexandr Nedvedicky alexandr.nedvedi...@oracle.com - From: Alexandr Nedvedicky alexandr.nedvedi...@oracle.com To: p...@benzedrine.cx Subject: unused argument

[alexandr.nedvedi...@oracle.com: PF Once rules are not removed from main anchor]

2014-06-21 Thread Alexandr Nedvedicky
Hello, resending to better alias as recommended by p...@benzedrine.cx subscribers. regards sasha - Forwarded message from Alexandr Nedvedicky alexandr.nedvedi...@oracle.com - From: Alexandr Nedvedicky alexandr.nedvedi...@oracle.com To: p...@benzedrine.cx Subject: PF Once rules

Re: PF Once rules are not removed from main anchor

2014-07-02 Thread Alexandr Nedvedicky
Hello, thanks for clarifying things. However, this solution is not correct for us. Perhaps you have some other changes in your tree to make it work. yes, that's correct. We had to make PF SMP friendly. We don't want packet to remove the ONCE rule from its ruleset. Instead the

unused argument in pfr_create_kentry()

2014-09-30 Thread Alexandr Nedvedicky
Hello, while working with PF code we've found the arg1 (flags) of pfr_create_kentry() is unused. the patch is trivial, just in case you are interested. regards sasha cut here to get patch -- Index: pf_table.c === RCS

mismatch for ICMP state created by inound response

2015-05-18 Thread Alexandr Nedvedicky
Hello, during our testing we've discovered small glitch in ICMP state handling. we use simple rule as follows: # pfctl -sr pass in on vnet2 all flags S/SA next we create a local outbound traffic using ping to arbitrary destination over vnet2 interface. This is what we get: # ping

Re: mismatch for ICMP state created by inound response

2015-05-18 Thread Alexandr Nedvedicky
Hello, Thanks for the patch, we'll be investigating this further. my deep apologize, I was too fast on send trigger. the patch is toxic. It breaks the opposite case: pass out on vnet2 all flags S/SA once rule above is used with patch applied we drop the first ICMP reply, so ping stops

copy'n'paste like typo in pf.c

2015-04-05 Thread Alexandr Nedvedicky
Hello, when we ran PF sources through coverity we got an error as follows: 8310 if (ri-r-dst.addr.type == PF_ADDR_TABLE) 8311 pfr_update_stats(ri-r-dst.addr.p.tbl, 8312 s-key[(s-direction == PF_IN)]- 8313

pfi_kif leaks for PBR rules

2015-04-05 Thread Alexandr Nedvedicky
Hello, while testing PBR on Solaris we found out the pfi_kif instances are not removed from pfi_ifs table. We took a look at crashdump and have seen pfik_route counter at those object is still non-zero, while all rules were gone. looking at sources we can see the 'pfik_route' (PFI_KIF_REF_ROUTE)

Re: mismatch for ICMP state created by inound response

2015-05-21 Thread Alexandr Nedvedicky
Hello, On Tue, May 19, 2015 at 14:07 +0200, Alexandr Nedvedicky wrote: Hello Mike, I've reworked patch from yesterday. I've done some quick testing to see if it fixes problem. It looks like it works. I have not tested NAT-64 yet. Also I'd like to come up with test case, which

SMP steroids for PF

2015-06-25 Thread Alexandr Nedvedicky
Hello, attached is SMP patch for PF. consider it as toxic proof of concept as it has paniced my amd64 system (see attached phone-shot). I have to figure out how to debug it yet. The problem is the USB keyboard has died, so I had no chance to type anything. fortunately the issue is 100%

Re: SMP steroids for PF

2015-06-26 Thread Alexandr Nedvedicky
Sorry, fingers were faster than head (again...) regards sasha On Thu, Jun 25, 2015 at 01:39:29PM -0700, Chris Cappuccio wrote: You should re-post as a diff -u !! Alexandr Nedvedicky [alexandr.nedvedi...@oracle.com] wrote: Hello, attached is SMP patch for PF. consider it as toxic proof

Re: SMP steroids for PF

2015-06-26 Thread Alexandr Nedvedicky
On Fri, Jun 26, 2015 at 04:34:06PM +0200, Martin Pieuchot wrote: On 26/06/15(Fri) 16:00, Alexandr Nedvedicky wrote: Hello Martin, I accept or your comments. I just have few quick notes/questions now. 2) I saw that you found some ALTQ leftovers, you have some Solaris (2) I think

Re: SMP steroids for PF

2015-06-26 Thread Alexandr Nedvedicky
. The only idea is to stay as much close to current version as possible. I'll try to break the patch into smaller chunks of changes. And post them. regards sasha On Fri, Jun 26, 2015 at 02:36:38PM +0200, Martin Pieuchot wrote: Hello Sasha, Alexandr Nedvedicky [alexandr.nedvedi

Re: pf_create_state() is sometimes better to use pf_unlink_state()

2015-05-28 Thread Alexandr Nedvedicky
/snip But we'll drop this reference in pf_src_tree_remove_state, then how will sns[PF_SN_NAT] and sns[PF_SN_ROUTE] be different? I think I should take PF class again ;-) I've just realized there is a test in pf_remove_src_node(): 572 if (sn-states 0 || sn-expire

Re: pf_create_state() is sometimes better to use pf_unlink_state()

2015-05-28 Thread Alexandr Nedvedicky
On Thu, May 28, 2015 at 11:43:02AM +0200, Mike Belopuhov wrote: On Thu, May 28, 2015 at 01:17 +0200, Alexandr Nedvedicky wrote: Hello, On Wed, May 27, 2015 at 07:44:15PM +0200, Mike Belopuhov wrote: On Wed, May 27, 2015 at 10:39 +0200, Alexandr Nedvedicky wrote: Hello

Re: pf_create_state() is sometimes better to use pf_unlink_state()

2015-05-27 Thread Alexandr Nedvedicky
Hello, On Wed, May 27, 2015 at 07:44:15PM +0200, Mike Belopuhov wrote: On Wed, May 27, 2015 at 10:39 +0200, Alexandr Nedvedicky wrote: Hello, - if (pf_state_insert(BOUND_IFACE(r, pd-kif), skw, sks, s)) { - pf_state_key_detach(s, PF_SK_STACK

Re: mismatch for ICMP state created by inound response

2015-05-24 Thread Alexandr Nedvedicky
Hello, I have no objections, just a small wish, can you set icmp_dir to -1, if we are not dealing with ICMP? there is a tool we use in Solaris, which yells on us because of uninitialized variable. I know it's false positive, but I've gave up on explaining... patch below combines your

IPv6 reassembly does not work with PBR

2015-05-25 Thread Alexandr Nedvedicky
Hello, It looks like IPv6 fragments are not forwarded properly, when PBR rules are used. My testing rules looks as follow: pass in quick on vnet1 from any to self pass in quick on vnet2 from any to self pass in on vnet1 from any to any route-to 2006::2@vnet2 no state pass in on

Re: mismatch for ICMP state created by inound response

2015-05-21 Thread Alexandr Nedvedicky
Hello, Well, not entirely (: I did it while exploring the code and sent out to provoke further discussion. Today I've talked to reyk@ and we think that it's better to go down a different road: make sure we don't create states on reply packets in the first place. that's actually very

Re: pf_create_state() is sometimes better to use pf_unlink_state()

2015-05-21 Thread Alexandr Nedvedicky
Hello, On Thu, May 21, 2015 at 07:43:51PM +0200, Mike Belopuhov wrote: On Thu, May 21, 2015 at 17:34 +0200, Alexandr Nedvedicky wrote: Hello, Hi, snippet below comes from pf_create_state(): 3559 if (pf_state_insert(BOUND_IFACE(r, pd-kif), skw, sks, s)) { 3560

pf_create_state() is sometimes better to use pf_unlink_state()

2015-05-21 Thread Alexandr Nedvedicky
Hello, snippet below comes from pf_create_state(): 3559 if (pf_state_insert(BOUND_IFACE(r, pd-kif), skw, sks, s)) { 3560 pf_state_key_detach(s, PF_SK_STACK); 3561 pf_state_key_detach(s, PF_SK_WIRE); 3562 *sks = *skw = NULL;

Re: PF SMP: making anchor stack multithreaded

2015-08-08 Thread Alexandr Nedvedicky
Hello, I've reworked the anchor handling so the traversal uses true recursion now. Using recursion here will allow us to implement ruleset locking in nicer fashion. The idea is to split current pf_test_rule() into two functions: pf_test_rule() and pf_match_rule(). pf_step_into_anchor() is

patch fixes IPv6 reassembly when packet hits PBR rule

2015-08-17 Thread Alexandr Nedvedicky
Hello, I'm sending finalized patch. The final shape has been discussed with bluhm@, who was kind enough to do thorough testing on OpenBSD. The patch solves the problem for deployment as follows: Client -- MTU_1 -- PF -- MTU_1 -- Router -- MTU_2 -- Server MTU_2 = MTU_1/2 PF does

potential memory leak when pf_create_state() fails

2015-07-16 Thread Alexandr Nedvedicky
Hello, It seems to me PF might leak rule items when pf_create_state() fails to create state for matching packet. The scenario is as follows: packet matches couple of 'match' rules before it hits a 'pass' rule match rules are kept in `rules` single list, which is a local variable of

sa_family_t is not always equal to u_int8_t

2015-07-16 Thread Alexandr Nedvedicky
Hello, we hit this problem while building PF on Solaris, where sizeof(sa_family_t) == 2 patch below fixes the problem for Solaris. regards sasha cvs diff -p output: --8--8--8-- Index: pfvar.h === RCS file:

potential memory leak in SIOCADDRULE

2015-07-16 Thread Alexandr Nedvedicky
Hello, PF can leak memory in DIOCADDRULE code path in case something goes wrong with rule creation. Our story begins when we do pf_rule_copyin(): 1070 if ((error = pf_rule_copyin(pr-rule, rule, ruleset))) { 1071 pool_put(pf_rule_pl, rule);

Re: sa_family_t is not always equal to u_int8_t

2015-07-16 Thread Alexandr Nedvedicky
On Thu, Jul 16, 2015 at 11:10:06PM +, Miod Vallat wrote: cvs diff -p output: Please send unified diffs (diff -u). The easiest way is to have a diff -up line in your ~/.cvsrc file. Or diff -uNp if you want cvs diff to show new files as well. Miod Sorry, now I got it.. regards

Re: potential memory leak when pf_create_state() fails

2015-07-19 Thread Alexandr Nedvedicky
On Mon, Jul 20, 2015 at 04:27:45AM +0900, Ryan McBride wrote: ok mcbride@ err I took a look at the patch one more time. I've realized PF must bind the rules to state before STATE_INC_COUNTERS() gets called. Not doing so makes PF to play games with dangling pointers to rule from state.

Re: preparing pfi_kif to MP world

2015-10-29 Thread Alexandr Nedvedicky
Hello, On Fri, Oct 16, 2015 at 01:51:31PM +0200, Alexandr Nedvedicky wrote: > On Fri, Oct 16, 2015 at 01:41:50PM +0200, Mike Belopuhov wrote: > > On 16 October 2015 at 13:28, Alexandr Nedvedicky > > <alexandr.nedvedi...@oracle.com> wrote: > > > > > > may be

Patch 1/3 - make DIOCRADDADDRS to accept on IP address per ioctl() call

2015-10-28 Thread Alexandr Nedvedicky
Hello, this is the first patch in series of three. All patches modify PF radix table API such the ioctl() functions accept one IP address per call. The idea has been proposed by Claudio at Varazdin. I still have to untangle pfr_commit_ktable() and DIOCRSETADDRS ioctl. Both seem to be more

Patch 2/3 - make DIOCRDELADDRS to accept on IP address per ioctl() call

2015-10-28 Thread Alexandr Nedvedicky
Hello, this is the second patch in 3-patch series. Patch changes DIOCRDELADDRS ioctl to DIOCRDELADDR, which accepts one IP address only per ioctl(2) call. Patch updates kernel and pfctl(8) only. Other components, which happen to use DIOCRDELADDRS will be updated by extra patch. thanks and

Patch 3/3 - update userland to reflect DIOCRADDADDRS/DIOCRDELADDRS changes

2015-10-28 Thread Alexandr Nedvedicky
Hello, this is the third patch in the first PF radix changes batch. Patch requires earlier patches to be in place, otherwise compilation will fail. Patch updates various user land tools by new PF radix table changes: s/DIOCRADDADDRS/DIOCRADDADDR s/DIOCRDELADDRS/DIOCRDELADDR it's also

Re: preparing pfi_kif to MP world

2015-10-28 Thread Alexandr Nedvedicky
Hello Mike, just a quick question: are you going to commit your pfi_kif_find() et. al.? or more work is needed there? thanks a lot regards sasha > > Turns out this is a rather simple issue that got slightly > complicated by the code diverging quite a bit since the > inception.

Re: Patch 2/3 - make DIOCRDELADDRS to accept on IP address per ioctl() call

2015-10-28 Thread Alexandr Nedvedicky
Hello, > > Index: sbin/pfctl/pfctl_radix.c > > + io.pfrio_size = 1; > > in 1/3 you have annotated like this > > + io.pfrio_size = 1; /* TODO: check .pfrio_size is needed */ > sorry this has leaked out from my internal repo. The .pfrio_size member will be dropped as soon as I'll be

patch saves some cycles by extending pfr_walktree() a bit

2015-10-28 Thread Alexandr Nedvedicky
Hello, This is yet another patch, which 'scratches surface', this time in pf_table.c. As briefly discussed in Varazdin the plan is to clean up pf_table.c a bit, to make implementation of reference handling and further MP stuff bit easier. I've noticed sub-optimal implementation table entries at

Re: preparing pfi_kif to MP world

2015-10-29 Thread Alexandr Nedvedicky
On Thu, Oct 29, 2015 at 02:49:40AM +0100, Mike Belopuhov wrote: > On 28 October 2015 at 18:41, Alexandr Nedvedicky > <alexandr.nedvedi...@oracle.com> wrote: > > Hello Mike, > > > > just a quick question: > > > > are you going to commit your pfi_ki

Re: Patch 1/3 - make DIOCRADDADDRS to accept on IP address per ioctl() call

2015-11-09 Thread Alexandr Nedvedicky
Hello, > On Wed, Oct 28, 2015 at 06:19:48PM +0100, Alexandr Nedvedicky wrote: > > The idea has been proposed by Claudio at Varazdin. > > I guess the idea is to eliminate the workq. Or is ther naother > reason to change it? the primary goal is to kill work queues. > >

Re: Patch 1/3 - make DIOCRADDADDRS to accept on IP address per ioctl() call

2015-11-09 Thread Alexandr Nedvedicky
On Sun, Nov 08, 2015 at 01:18:22PM +0100, Alexander Bluhm wrote: > On Sun, Nov 08, 2015 at 02:37:58AM +0100, Alexander Bluhm wrote: > > > + for (i = 0; (i < size) && (rv == 0); i++) { > > > > rv is unitialized in the first interation > > > > > + io.pfrio_buffer = addr++; > > > +

Re: patch saves some cycles by extending pfr_walktree() a bit

2015-11-09 Thread Alexandr Nedvedicky
Hello, > > For now the code gets more as we have two ways to iterate over the > tree. When you remove the additional work queues I expect many - > diffs. So if this code dupliation is temporary, this aproach is > fine for me. yes code duplication is temporary, I'd like to kill work queues. >

Re: Patch 1/3 - make DIOCRADDADDRS to accept on IP address per ioctl() call

2015-11-09 Thread Alexandr Nedvedicky
> > I'm wondering - how does it affect tools that load several thousands of IPs > into a table? Like spamd, bgpd (for spam lists etc.), or pfctl for IP black > lists (as distributed by ET). > > There are valid use cases with HUGE tables, but I have to admit that I didn't > test your diff yet.

Re: patch - potential use-after-free pfr_set_addrs()

2015-11-03 Thread Alexandr Nedvedicky
On Tue, Nov 03, 2015 at 10:09:49PM +0100, Alexander Bluhm wrote: > On Tue, Nov 03, 2015 at 09:40:38PM +0100, Alexandr Nedvedicky wrote: > > I think the > > right thing is to use goto _skip; in that branch to avoid 499 et. al. > > completely. > > Yes > > > @@ -

patch - potential use-after-free pfr_set_addrs()

2015-11-03 Thread Alexandr Nedvedicky
Hello, Patch fixes potential use-after-free in pf_table.c:pfr_set_addrs(): 463 for (i = 0; i < size; i++) { ... 483 q = pfr_lookup_addr(tmpkt, , 1); 484 if (q != NULL) { 485 ad.pfra_fback =

Re: preparing pfi_kif to MP world

2015-10-16 Thread Alexandr Nedvedicky
On Fri, Oct 16, 2015 at 01:41:50PM +0200, Mike Belopuhov wrote: > On 16 October 2015 at 13:28, Alexandr Nedvedicky > <alexandr.nedvedi...@oracle.com> wrote: > > > > may be it's kind of bike shading... > > How about make kifs to stick to convention we see for othe

Re: preparing pfi_kif to MP world

2015-10-16 Thread Alexandr Nedvedicky
> > Turns out this is a rather simple issue that got slightly > complicated by the code diverging quite a bit since the > inception. Essentially the clr->ifname comes from the > interface specification in the "pfctl -i foo0 -Fs" for > if-bound states (floating states use fake interface "any"). >

patch for two nits around pf_insert_src_node() et. al.

2015-10-10 Thread Alexandr Nedvedicky
Hello, Patch fixes two small nits related to source node table in PF (a.k.a. pf_src_tree_tracking). The first issue comes to `global` argument of pf_insert_src_node(). It is always 0 everywhere in source code. The `global` is supposed to indicate whether particular state is bound to global/main

Re: patch for two nits around pf_insert_src_node() et. al.

2015-10-12 Thread Alexandr Nedvedicky
Hello, The updated patch addresses additional nit found by mpi: > > Here can't you also change: > > > > if ((*sn)->rule.ptr != NULL) > > (*sn)->rule.ptr->src_nodes++; > > > > into: > > > > (*sn)->rule.ptr->src_nodes++; > > > > I don't know enough to

Re: patch for two nits around pf_insert_src_node() et. al.

2015-10-12 Thread Alexandr Nedvedicky
Hello, Richard Procter came back to me in private email with one more nit to fix: we can get rid of if (sn->rule.ptr != NULL) test condition in pfioctl() function as well. The relevant snippet looks as follows: 2188 p = psn->psn_src_nodes; 2189

Re: preparing pfi_kif to MP world

2015-10-13 Thread Alexandr Nedvedicky
Hello, > > > Furthermore existing functions pfi_kif_ref()/pfi_kif_unref() are thrown > > > away > > > in favor of pfi_kif_take()/pfi_kif_rele(), which follow naming convention > > > set by refcnt_init(9). Patch also removes kif reference types (enum > > > pfi_kif_refs). > > > > [snip] > > > @@

Re: the very first step towards MULTIPROCESSOR friendly PF

2015-08-27 Thread Alexandr Nedvedicky
/large snip Assuming the locking in MULTIPROCESSOR goes like: interrupt grabs splsoftnet - ip_input - PF grabs KERNEL_LOCK() We need to take care of ioctl() call path and purge thread. Those need to get synchronize with packets using KERNEL_LOCK(). They should not to mess with

the very first step towards MULTIPROCESSOR friendly PF

2015-08-26 Thread Alexandr Nedvedicky
Hello, I'm not sure I got everything right in Calgary. So this patch should roughly illustrates how I think we should start moving forward to make PF MULTIPROCESSOR friendly. It's quite likely my proposal/way is completely off, I'll be happy if you put me back to ground. The brief summary of

Re: the very first step towards MULTIPROCESSOR friendly PF

2015-08-27 Thread Alexandr Nedvedicky
On Wed, Aug 26, 2015 at 06:12:10PM +0200, Mark Kettenis wrote: Date: Wed, 26 Aug 2015 17:30:14 +0200 From: Alexandr Nedvedicky alexandr.nedvedi...@oracle.com Hello, I'm not sure I got everything right in Calgary. So this patch should roughly illustrates how I think we should start

Re: [PATCH] PF: cksum modification & refactor [0/24]

2015-08-31 Thread Alexandr Nedvedicky
Hello, I'm fine with this change. It certainly posses no thread to SMP branch... > * Initialise pd->pcksum for icmp6 > > - ensures pcksum is set for all known checksummed protocols > - later patches rely on this may be this patch/change should be folded to patch, which really

PF ignores block action when rule contains route-to/dup-to action

2015-08-31 Thread Alexandr Nedvedicky
Hello, Dilli Paudel in Oracle was playing with PF enough to find funny glitch. He used rule as follows: block in on vnic4 from 192.168.1.0/24 to any route-to 172.16.1.1@vnic5 Many people expect the route-to action is somewhat futile as 'block' action takes precedence here, so packet

Re: [PATCH] PF: cksum modification & refactor [3/24]

2015-08-31 Thread Alexandr Nedvedicky
Hello Richard, the code in patch looks good for the first glance. However it seems to me the newly introduced pf_cksum_fixup*() are not called yet. Do you think you can reshuffle changes between your set of patches a bit, so the newly introduced functions will become alive (get called)? Also I

Re: PF ignores block action when rule contains route-to/dup-to action

2015-09-01 Thread Alexandr Nedvedicky
Hello, > > As a side effect the patch breaks block rules with dup-to action. dup-to > > action as a part of block rule might make some sense... So if there is > > someone, who really needs block ... dup-to he should opt for equivalent > > rule using pass ... route-to > > > > Also there is one

Re: the very first step towards MULTIPROCESSOR friendly PF

2015-09-04 Thread Alexandr Nedvedicky
Hello, after reading emails from Philip Guenther and Mark Kettenis, doing some RTFM on locking in OpenBSD kernel I have a new patch. I call it as a smp-step-0. Patch introduces a KERNEL_LOCK() to PF. Many dances with KERNEL_LOCK() happens in pf_test(). From future work point of view there are

Re: PF SMP: making anchor stack multithreaded

2015-09-12 Thread Alexandr Nedvedicky
Hello, re-sending with updated patch version. I'd like to get it in, so I can start moving forward with other things. any O.Ks? thanks and regards sasha On Sat, Aug 08, 2015 at 12:16:26PM +0200, Alexandr Nedvedicky wrote: > Hello, > > I've reworked the anchor handling so the trave

PF SMP: mutex for fragcache

2015-09-12 Thread Alexandr Nedvedicky
Hello, very small first step towards MP(i) friendly PF. Patch adds mutex around fragment cache. Patch adds a lock around fragment cache. Unlike other parts of PF the fragment cache is self-contained subsystem. In that sense we can easily guard its entry points (pf_reassemble(), pf_reassemble6())

Re: [patch] cleaner checksum modification for pf

2015-09-29 Thread Alexandr Nedvedicky
Hello, I've tried Richard's patch on sparc. I took a brief look at its source code. It's essentially what PF is doing on Solaris. The checksum handling in PF on systems with HW assisted checksums is getting tricky for local (out)bound packets. The approach we take on Solaris is as follows:

Re: pf statekey inp chaining

2015-12-03 Thread Alexandr Nedvedicky
Hello, OK sasha On Thu, Dec 03, 2015 at 12:29:15PM +0100, Alexander Bluhm wrote: > On Wed, Dec 02, 2015 at 07:45:09PM +0100, Alexander Bluhm wrote: > > Here is a new version of the diff. This is new: > > Now with feedback from sashan@ > > - merge > - no SS_ISCONNECTED check in tcp as it was

Re: free sizes for most free calls in pf_ioctl

2015-12-03 Thread Alexandr Nedvedicky
Hello, OK regards sasha On Thu, Dec 03, 2015 at 01:21:32PM +0100, Claudio Jeker wrote: > This should cover the simple free calls in pf_ioctl. > > -- > :wq Claudio > > Index: pf_ioctl.c > === > RCS file:

Re: introducing ip_send()/ip6_send() to OpenBSD kernel

2015-12-04 Thread Alexandr Nedvedicky
On Thu, Dec 03, 2015 at 11:00:09PM +0100, Thierry Deval wrote: > Hello Sasha, > > You kept the static prototypes for ip_send_dispatch and > ip6_send_dispatch. > > You'd better avoid mixing static and non-static declarations for the same > functions. ;-) > bluhm sitting next to me spot it too.

introducing ip_send()/ip6_send() to OpenBSD kernel

2015-12-03 Thread Alexandr Nedvedicky
Hello, patch below introduces ip_send() function to OpenBSD kernel. ip_send() function takes an mbuf with packet and passes to ip_output(), which will be running in softnet task. the patch also changes icmp_error()/icmp6_error() to dispatch the ICMP error responses via ip_send(), so both

removing expired once rules in pf_purge_thread()

2015-12-05 Thread Alexandr Nedvedicky
Hello, henning@ and mikeb@ showed some interest to change handling of once rules to the same way as PF has it on Solaris. Just to refresh the audience on once option offered by PF: onceCreates a one shot rule that will remove itself from an active ruleset after the first

Re: introducing ip_send()/ip6_send() to OpenBSD kernel

2015-12-03 Thread Alexandr Nedvedicky
Hello, so after a feedback in a hackroom here is the third version of patch. The summary of changes is as follows: - ip*_send() function use softnettq to dispatch packet - ip*_output() functions running in ip*_send_dispatch() are protected KERNEL_LOCK() and running at

Re: introducing ip_send()/ip6_send() to OpenBSD kernel

2015-12-03 Thread Alexandr Nedvedicky
Hello, mikeb@ found a fundamental problem in my earlier patch. The ip_send() function was using `softnettq` (softnet task queue) to dispatch packet via ip*_output(). Doing so it's risky business as ip*_output() is not unlocked yet. So new patch version introduces a new task: ipsendtq. The

Re: introducing ip_send()/ip6_send() to OpenBSD kernel

2015-12-03 Thread Alexandr Nedvedicky
Hello, below is final patch I'm going to commit. Summary of changes: - softnettq declaration moved to net/if_var.h (by bluhm@) - lock order swapped: KERNEL_LOCK() goes first folllowed by spl (by bluhm@) - long line got fixed (by bluhm@) -

Re: pf unlink remove

2015-12-02 Thread Alexandr Nedvedicky
Hello, OK sasha

Re: removing expired once rules in pf_purge_thread()

2015-12-18 Thread Alexandr Nedvedicky
Hello Richard, > > What has to be granted is there is one 'winner' only, which puts the > > rule to garbage collector list. The pragmatic approach wins here. > > Right. I'll just note though that the patch as it stands allows multiple > winners: consider the window between testing

Re: removing expired once rules in pf_purge_thread()

2015-12-15 Thread Alexandr Nedvedicky
Hello, > It just occurred to me that another possibility would be a match-only > rule that matches one but doesn't involve any purging machinery. Right > now we install ftp-proxy rules as having maximum number of states equal > to 1, however there's a time window between the moment the state is

Re: removing expired once rules in pf_purge_thread()

2015-12-16 Thread Alexandr Nedvedicky
hello, > > > It just occurred to me that another possibility would be a match-only > > > rule that matches one but doesn't involve any purging machinery. Right > > > now we install ftp-proxy rules as having maximum number of states equal > > > to 1, however there's a time window between the

Re: removing expired once rules in pf_purge_thread()

2015-12-15 Thread Alexandr Nedvedicky
Hello, > > > >Another possibility would be to require 'once' rules to be 'quick'. > >This closes the candidacy window and makes its serialisation, to > >preclude multiple matches, more feasible. > > > >Yet another possibility is to drop 'once' rules as too complex to > >

Re: removing expired once rules in pf_purge_thread()

2015-12-16 Thread Alexandr Nedvedicky
Hello, On Wed, Dec 16, 2015 at 02:48:49PM +1300, Richard Procter wrote: > > On Tue, 15 Dec 2015, Mike Belopuhov wrote: > > > >Yet another possibility is to drop 'once' rules as too complex to > > >implement for multiprocessor but I have no idea if this is viable. > > > > It is. And I

PF: reference counting for statekey

2016-01-03 Thread Alexandr Nedvedicky
Hello, there is a sad story behind patch below. I've commit the change on Dec 22snd. Unfortunately many people who run snapshots experienced panic on assert. Markus Lude has been the first who reported the issue: !pf_state_key_isvalid(sk)" failed: file "../../../../net/pf.c", line 6830

Re: Interesting if_get() case

2015-11-20 Thread Alexandr Nedvedicky
Hello, thanks for detailed explanation. > + else { > + struct ifnet *destifp; > + > + destifp = if_get(rt->rt_ifidx); > + if (destifp != NULL) > + destmtu =

Re: rt_ifp and icmp

2015-11-20 Thread Alexandr Nedvedicky
On Fri, Nov 20, 2015 at 02:31:23PM +0100, Martin Pieuchot wrote: > The first rt_ifp of the day, make use of if_get() inside icmp_mtudisc(). > > ok? > OK (however you are probably better to wait for more experienced one OK) BTW I like the way rt_ifidx is defined, it's smart way to move forward

Re: rt_ifp and icmp6

2015-11-20 Thread Alexandr Nedvedicky
Hello, OK regards sasha On Fri, Nov 20, 2015 at 02:33:44PM +0100, Martin Pieuchot wrote: > Some if_get() love in icmp6. > > ok? >

Re: rt_ifp and pf(4)

2015-11-20 Thread Alexandr Nedvedicky
Hello, I have just nit comments, feel free to ignore them. 1) would it be possible to use closing #endif guards as follows: #endif /* NCARP */ 2) another nit here: > @@ -5607,6 +5616,8 @@ pf_route(struct mbuf **m, struct pf_rule > done: > if (r->rt != PF_DUPTO) >

Re: Interesting if_get() case

2015-11-22 Thread Alexandr Nedvedicky
Hello, > > > > > + else { > > > + struct ifnet *destifp; > > > + > > > + destifp = if_get(rt->rt_ifidx); > > > + if (destifp != NULL) > > > + destmtu = destifp->if_mtu; > > > +

Re: rt_ifp and pf(4)

2015-11-22 Thread Alexandr Nedvedicky
On Sat, Nov 21, 2015 at 12:37:58PM +0100, Martin Pieuchot wrote: > On 20/11/15(Fri) 18:05, Alexandr Nedvedicky wrote: > > Hello, > > > > I have just nit comments, feel free to ignore them. > > > > 1) would it be possible to use closing #endif guards as

small glitch in pfctl_show_rules()

2016-05-30 Thread Alexandr Nedvedicky
Hello, Petr Hoffmann discovered glitch in 'pfctl -a "*" -sr' command when it is recursively dumping rulesets from PF kernel module. The ruleset in kernel got created by screw-it.sh shell script: #!/bin/sh pfctl -d echo 'anchor "../bar/*"\nanchor "bar/*"' |pfctl -a foo -f - echo

KASSERT() @ pf_test() is back

2016-02-07 Thread Alexandr Nedvedicky
Hello, I don't expect to see O.K. to patch below. The patch is the part of the change, which has been backed out last weekend. Too many things were wrong so I'm trying to untangle the code a bit. Patch below is for brave hearts, who don't mind to see KASSERT() to fire: Index: net/pf.c

Re: snapshot from 26-Jan-2016 - pfsync panic

2016-01-28 Thread Alexandr Nedvedicky
Hello, > would this commit > http://permalink.gmane.org/gmane.os.openbsd.cvs/152882 > resolve this problem? This commit should fix this issue. can you test it for me? FYI: if_pfsync.c must initialize reference counter at imported statekey to 1. If it is left to 0, then PF trips the assert.

back out ASSERT(...pf.statekey == NULL) @ pf.c:6569

2016-01-30 Thread Alexandr Nedvedicky
Hello, time has come to backout stuff, which has been baked just before Christmas 2015. Although there was some progress over the last few days the things are not improving that fast to become good enough for 5.9 release. I took patch (~sthen/pf-statekey-backout.diff) prepared by sthen last

Re: let's get reference to state key back

2016-03-28 Thread Alexandr Nedvedicky
Hello, > i can't see any problem with this patch. i'm sending 14Mpps ip4 and ip6 > over ix intefaces and creating around 3M to 5M states and all this more > than 24 hours. box is unusable but it's alive :) thank you very much for testing. let's wait day or two to give other folks chance to trip

let's get reference to state key back

2016-03-25 Thread Alexandr Nedvedicky
Hello, this yet another patch, which got backed out in Jan. This time I'd like to commit change, which makes sure IP stack keeps reference to state key along the packet (mbuf). The change should not cause panics, but I'd like to ask testers to watch for memory leaks on state key pool (pfstkey):

Re: let's get reference to state key back

2016-03-25 Thread Alexandr Nedvedicky
Hello, sorry I have not test patch, which I've sent earlier. I boot unpatched kernel... this is the fix one need. updated patch is further below. 8<---8<-8< diff -r 00f90fca186c src/sys/net/pf.c --- a/src/sys/net/pf.c Fri Mar 25 09:29:43 2016 +0100

Re: KASSERT() @ pf_test() is back

2016-03-07 Thread Alexandr Nedvedicky
On Mon, Mar 07, 2016 at 10:54:26AM +0100, Mattieu Baptiste wrote: > On Mon, Mar 7, 2016 at 10:03 AM, Alexandr Nedvedicky > <alexandr.nedvedi...@oracle.com> wrote: > > Hello Mattieu, > > > > Mark Patruck reported panic on KASSERT() in pf_test() yesterday . I've > &

Re: KASSERT() @ pf_test() is back

2016-03-04 Thread Alexandr Nedvedicky
Hello Stuart, thanks for testing it. I'll commit it today. regards sasha On Fri, Mar 04, 2016 at 01:08:42AM +, Stuart Henderson wrote: > On 2016/02/28 13:01, Martin Pieuchot wrote: > > On 08/02/16(Mon) 01:55, Alexandr Nedvedicky wrote: > > > Hello, > > > >

Re: KASSERT() @ pf_test() is back

2016-03-07 Thread Alexandr Nedvedicky
Hello Mattieu, Mark Patruck reported panic on KASSERT() in pf_test() yesterday . I've crafted patch below. Can you try it out? I think we need to apply pf_pkt_addr_changed() on broadcast packets seen by bridge as well. thanks a lot and sorry for inconveniences regards sasha

Re: pf ouraddr

2016-07-18 Thread Alexandr Nedvedicky
Hello, it looks good to me. OK sasha On Mon, Jul 18, 2016 at 10:51:44AM +0200, Alexander Bluhm wrote: > Hi, > > To hide pf internals move code from in_ouraddr() to pf_ouraddr(). > This will also make it possible to implement the same shortcut for > IPv6. > > ok? > > bluhm >

Re: refactor PF option parsing loops

2017-01-24 Thread Alexandr Nedvedicky
Hello Richard, > PF implements six distinct TCP option parsing loops. This patch converts > these to one inline function in pfvar_priv.h, normalises their semantics, > and strips ~100 lines. what is the reason to keep function definition in pfvar_priv.h? I would expect to stick

Re: removing expired once rules in pf_purge_thread()

2016-09-03 Thread Alexandr Nedvedicky
Hello, there was still one more glitch catched by mikeb: I have to sanitize pointer when copying rule to userland. The other thing pointed out by mike is the Expired time should be printed for expired rules only in debug mode output (pfctl -sr -g) Incremental patch is as follows:

Re: removing expired once rules in pf_purge_thread()

2016-09-03 Thread Alexandr Nedvedicky
Hello, > >updated version is below. > > > >comments? O.K.? > > One comment, otherwise ok. > > Could you assert the lock is held otherwise, this might save > effort if/when this code is refactored: > > else > rw_assert_wrlock(_consistency_lock); sure. also mikeb came to

Re: pfctl mixes up anchorname with anchorpath

2016-09-03 Thread Alexandr Nedvedicky
Hello, mikeb pointed out I should not be copy'n'pasting buggy code. I'm just re-sending updated patch with change below: 8<---8<---8<--8< diff -r e2383ec80feb src/sbin/pfctl/pfctl.c --- a/src/sbin/pfctl/pfctl.cSat Sep 03 15:39:17 2016

pfctl mixes up anchorname with anchorpath

2016-09-03 Thread Alexandr Nedvedicky
Hello, One of the teams in Oracle Solaris uses sophisticated naming scheme for PF rulesets. The anchor (ruleset) is identified by something like that: root/whatever:component:name/some-virtual-instance-long-name/inbound That particular team hit a bug in pfctl, when they were trying to load

Re: removing expired once rules in pf_purge_thread()

2016-08-29 Thread Alexandr Nedvedicky
Hello, mikeb has just pointed out the patch fell under the desk asking me to resend it. > henning@ and mikeb@ showed some interest to change handling of once rules to > the same way as PF has it on Solaris. Just to refresh the audience on once > option offered by PF: > > onceCreates a

Re: removing expired once rules in pf_purge_thread()

2016-08-30 Thread Alexandr Nedvedicky
Hello, On Tue, Aug 30, 2016 at 10:53:56AM +1000, David Gwynne wrote: > > > On 17 Dec 2015, at 13:30, Richard Procter <richard.n.proc...@gmail.com> > > wrote: > > > > > > Hi Sasha, > > > > On Fri, 18 Dec 2015, Alexandr Nedvedicky wrote: >

let PF to send challenge ack

2016-09-30 Thread Alexandr Nedvedicky
Hello, patch below makes life easier for clients, which always use same source port, when talking to server (e.g. think of NFS). The scenario we are dealing with is as follows: - client mounts remote NFS share - there is a PF sitting between client and NFS server. the mount

Re: pf_route pf_pdesc

2016-10-27 Thread Alexandr Nedvedicky
Hello, On Wed, Oct 26, 2016 at 11:48:34PM +0200, Alexander Bluhm wrote: > On Wed, Oct 19, 2016 at 11:49:56PM +0200, Alexander Bluhm wrote: > > I would like to pass a struct pf_pdesc to pf_route() like it is > > done in the other pf functions. That means less parameters, more > > consistency and

  1   2   3   4   5   6   7   >