Re: pf fragment drop stale

2017-06-26 Thread Alexandr Nedvedicky
Hello, On Mon, Jun 26, 2017 at 05:51:08PM +0200, Alexander Bluhm wrote: > On Mon, Jun 26, 2017 at 10:29:24AM +0200, Alexandr Nedvedicky wrote: > > > +#define PF_FRAG_STALE200 /* Limit fragments per second per > > > connection */ > > > I did no

Re: pf_purge_thread() w/o KERNEL_LOCK()

2017-06-26 Thread Alexandr Nedvedicky
On Mon, Jun 26, 2017 at 11:38:35AM +0200, Martin Pieuchot wrote: > The NET_LOCK() is currently what guarantees that accesses to PF data > structures are serialized. So we can drop the KERNEL_LOCK() in the > pf_purge_thread() to reduce contention. > > While here use rwsleep(9) instead of calling N

Re: Potential DoS attack on PF due to infinite loop

2017-07-12 Thread Alexandr Nedvedicky
On Wed, Jul 12, 2017 at 02:40:58PM +0200, Alexander Bluhm wrote: > On Tue, Jul 11, 2017 at 12:29:33PM -0700, Jingmin Zhou wrote: > > The problem is at line 224. When a LB rule is configured to have 65535 as > > the high port, and uint16 variable tmp reaches it, ++(tmp) will wrap around > > and henc

Re: Reducing NET_LOCK() contention

2017-07-27 Thread Alexandr Nedvedicky
Hello, > Hrvoje Popovski confirmed this reduces "ifconfig" pauses. He also > confirmed states are correctly purged. > > So, I'd appreciate reviews and oks. sorry for keeping you waiting. the change looks good. just a small nit, which I don't insist on. > > Index: net/pfvar.h > > ==

Re: Reducing NET_LOCK() contention

2017-08-02 Thread Alexandr Nedvedicky
Hello, On Wed, Aug 02, 2017 at 10:10:51AM +0200, Martin Pieuchot wrote: > On 18/07/17(Tue) 15:55, Martin Pieuchot wrote: > > When forwarding a lot of traffic with 10G interfaces contention on the > > NET_LOCK() is "visible". Each time you type "ifconfig" you can go grab > > a coffee... > > > > T

Re: CID 1452909: Use of untrusted scalar value (pf_table.c)

2017-08-15 Thread Alexandr Nedvedicky
Hello, > I suggest to do two things: add a check in pfr_validate_addr > that is called after every copyin and also perform the check > in pfr_create_kentry before we attempt to use the value. > > OK? change looks good to me. > > P.S. > What does 'k' table and entry prefix stand for in pf_t

Re: pf.4: sync structs with net/pfvar.h

2017-08-27 Thread Alexandr Nedvedicky
Hello, On Sun, Aug 27, 2017 at 10:11:49PM -0400, Lawrence Teo wrote: > This syncs the struct declarations in pf.4 with the latest net/pfvar.h > (r1.465 at the time of writing). > > ok? looks good to me. OK sashan

refactoring of pf_find_or_create_ruleset()

2017-08-31 Thread Alexandr Nedvedicky
Hello, long time ago mpi@ asked, what I would improve in PF to make the code ready for SMP massage. Everybody knows PF is perfect, right? So it took me a while to find a code for facelift. Patch below breaks pf_find_or_create_ruleset() spaghetti to more chunks: pf_find_or_create_ruleset()

Re: refactoring of pf_find_or_create_ruleset()

2017-09-01 Thread Alexandr Nedvedicky
Hello Hrvoje, > Hi, > > with this diff i'm getting this panic: > > # pfctl -nvf /etc/pf.conf > set limit states 100 > set skip on { lo em0 } > block return all > pass all flags S/SA > anchor "test1" on ix1 all { > pass all flags S/SA > } > > > # pfctl -f /etc/pf.conf > uvm_fault(0xff

Re: refactoring of pf_find_or_create_ruleset()

2017-09-04 Thread Alexandr Nedvedicky
Hello, > with this patch i can't trigger panic with or without WITH_PF_LOCK if > that's matter for some reason. anyway below is the patch, which Hrvoje was testing and it worked for him. I'd like to get some OK to proceed to commit. > thank you sasha for great work on MP pf :) I'm v

Re: refactoring of pf_find_or_create_ruleset()

2017-09-05 Thread Alexandr Nedvedicky
Hello, On Mon, Sep 04, 2017 at 09:51:29PM +0200, Alexander Bluhm wrote: > On Mon, Sep 04, 2017 at 10:29:01AM +0200, Alexandr Nedvedicky wrote: > > anyway below is the patch, which Hrvoje was testing and it worked for > > him. > > I'd like to get some OK to proce

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 p

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: 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 o

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 > > wrote: > > > > > > Hi Sasha, > > > > On Fri, 18 Dec 2015, Alexandr Nedvedicky wrote: > > > >>>

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 r

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(&pf_consistency_lock); sure. also mikeb came t

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: 8<

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 +

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: m_resethdr() in if_input_local()

2016-10-13 Thread Alexandr Nedvedicky
Hello, looks O.K. to me. can you also do s/destinated/designated, when you are touching the if_input_local() function already? thanks and regards sasha > ok? > > bluhm > > Index: net/if.c > === > RCS file: /data/mirror/openbsd/cv

Re: pf_route pf_pdesc

2016-10-20 Thread Alexandr Nedvedicky
Hello, On Wed, Oct 19, 2016 at 11:49:56PM +0200, Alexander Bluhm wrote: > Hi, > > 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 later I can call functions that need an pd from > pf_route(). >

Re: let PF to send challenge ack

2016-10-20 Thread Alexandr Nedvedicky
Hello, On Thu, Oct 20, 2016 at 08:45:09PM +0200, Alexander Bluhm wrote: > On Fri, Sep 30, 2016 at 11:55:48PM +0200, Alexandr Nedvedicky wrote: > > The patch makes PF to send 'challenge ACK' for SYN packet, which matches > > session in established state. > > regress

PF once rule should not trigger removal of parent anchor rule

2016-10-20 Thread Alexandr Nedvedicky
Hello, Petr Hoffmann at Oracle discovered a glitch in ONCE rules and anchors. Petr's test case, which shows a misbehavior looks as follows: echo 'anchor "foo/*"' | pfctl -f - pfctl -sr # anchor "foo/*" all echo 'pass' | pfctl -a foo/bar -f - echo 'pass onc

Re: PF once rule should not trigger removal of parent anchor rule

2016-10-21 Thread Alexandr Nedvedicky
Hello Mike, (I'm putting tech@ back) > Or some other changes if expire has happened with the deferred removal in > the thread. What I saying is basically that the last fix I did for once > rules was tested in the scenario you've described. sorry I've messed up my description well enough to obfus

Re: pf_route pf_pdesc

2016-10-22 Thread Alexandr Nedvedicky
Hello, > > Do we want to go this way? > > You got me thinking how else this might be done. > > Note that union pf_headers is used only to calculate the max size of the > header pull buffer. Its members are never referenced. > > So it may be enough to #define PF_MAXHDR_SIZE 28 in pfvar.h (==

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

Re: pf IPv6 hop-by-hop after fragment header

2016-11-18 Thread Alexandr Nedvedicky
Hello, > I found the link http://www.secfu.net/ in one of sthen@'s mails. > There the author mentions that we accept IPv6 hop-by-hop headers > after fragment headers. In fact this is a result of my pf fragment > reassembly, so add an extra check there. > > ok? I'm O.K. with it. Side No

Re: pf overlapping IPv6 fragments

2016-11-18 Thread Alexandr Nedvedicky
Hello, > more strictly here. Drop the whole fragment state if IPv6 fragments > appear which have invalid length, fragment-offset or more-fragment-bit. I like the idea being strict here. I don't like 'goto overlap_fragment'. the 'overlap_fragment' as a name of jump target is bit confusing

Re: pf af-to route output

2016-11-19 Thread Alexandr Nedvedicky
Hello, > The !r->rt case is only used by af-to. pf_route6() calls ip6_output() > to do the work while pf_route() has some custom implementation for > that. It is simpler to call ip_output() or ip6_output() from > pf_test() directly. It looks good to me. I'm O.K. with change. regards sasha

Re: [PATCH] pf: fold pf_headers into pf_pdesc

2016-11-20 Thread Alexandr Nedvedicky
Hello, > As per usual I've broken this into an easier-to-review series > of patches, which is available at http://203.79.107.124/hdr I'm also O.K. with it regards sasha

Re: pf overlapping IPv6 fragments

2016-11-21 Thread Alexandr Nedvedicky
On Mon, Nov 21, 2016 at 10:58:43AM +0100, Alexander Bluhm wrote: > On Fri, Nov 18, 2016 at 11:33:33PM +0100, Alexandr Nedvedicky wrote: > > how about using 'goto free_ipv6_frag' ? It better explains, what's > > going to happen. > > makes sense than

Re: pf af-to route output

2016-11-21 Thread Alexandr Nedvedicky
On Mon, Nov 21, 2016 at 07:11:23PM +0100, Mike Belopuhov wrote: > On Mon, Nov 14, 2016 at 16:38 +0100, Alexander Bluhm wrote: > > Hi, > > > > The !r->rt case is only used by af-to. pf_route6() calls ip6_output() > > to do the work while pf_route() has some custom implementation for > > that. It

Re: pf af-to route output

2016-11-22 Thread Alexandr Nedvedicky
> So we have a kernel implementation that might work for a feature > that makes sense. Especially the reply-to could be useful. But > the parser is too dumb. I think we should fix the parser and then > test the kernel. I absolutely agree. regards sasha

Re: pf af-to route output

2016-11-22 Thread Alexandr Nedvedicky
On Wed, Nov 23, 2016 at 01:43:59AM +0100, Mike Belopuhov wrote: > On 23 November 2016 at 01:42, Alexander Bluhm wrote: > > On Tue, Nov 22, 2016 at 01:44:09PM +0100, Mike Belopuhov wrote: > >> OK, all I wanted to know was if this is know to work and if it has > >> been tested. I'd argue that we do

Re: pf state key link

2016-12-28 Thread Alexandr Nedvedicky
Hello, On Fri, Dec 23, 2016 at 04:21:09PM +0100, Alexander Bluhm wrote: > Hi, > > Christiano Haesbaert has sent me this diff. > > They are setting pkt_sk to NULL if pkt_sk->reverse is not > > pf_statek_key_isvalid(), but the chunk that creates the pkt_sk->reverse >

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% reproducib

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 p

Re: SMP steroids for PF

2015-06-26 Thread Alexandr Nedvedicky
link members pfrkt_workq/pfrke_workq in pfr_ktable/pfr_kentry. 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: &g

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 A

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: /cvs/

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 p

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.. r

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. St

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 change

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

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 what

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 > > > > Hello, > > > > I'm not sure I got everything right in Calgary. So this patch should > > roughly illustr

Re: the very first step towards MULTIPROCESSOR friendly PF

2015-08-27 Thread Alexandr Nedvedicky
> > 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 > >

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 t

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 ex

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 gets

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 m

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 dis

Re: the very first step towards MULTIPROCESSOR friendly PF

2015-09-07 Thread Alexandr Nedvedicky
Hello, please do not use the patch I've sent last Friday. It's wrong. It might work, but it's a dead end in evolution. The thing is I misunderstood (read: ignored) all the constraints behind packets and interrupts in OpenBSD kernel. see mikeb's email below. please do not wast your cycles with t

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

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:

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 r

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 say

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] > > > @@ -

[patch] introducing pfi_kif_check() to better support if_pfsync.c

2015-10-14 Thread Alexandr Nedvedicky
Hello, While reviewing patch, which introduces MP-friendly reference counting for pfi_kif objects (PF stuff), we've found some 'interesting detail' in if_pfsync.c, which I think should get addressed by extra patch. It looks like if_pfsync.c module does not realize, the pfi_kif_get() actually alw

Re: [patch] introducing pfi_kif_check() to better support if_pfsync.c

2015-10-14 Thread Alexandr Nedvedicky
pfi_kif *kif; > + > + if ((kif = pfi_kif_find(kif_name)) != NULL) > return (kif); > > /* create new one */ > Index: net/pfvar.h > === > --- net.orig/pfvar.h > +++ net/pfvar.h > @@ -1

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"). >

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 > wrote: > > > > may be it's kind of bike shading... > > How about make kifs to stick to convention we see for other objects

pfctl always prints warning when flushes ruleset

2017-09-26 Thread Alexandr Nedvedicky
Hello, few users on Solaris don't like to read warning 'Anchor or Ruleset' does not exist: # echo 'pass' |pfctl -a foo -f - # pfctl -a foo -Fa rules cleared pfctl: Anchor or Ruleset does not exist. # the commands above did work well, the 'pfctl: Anchor ...' warning message i

pfctl can do a better job when handling ioctl(2) errors

2017-09-26 Thread Alexandr Nedvedicky
Hello, whenever administrator asks pfctl to modify/remove anchor, which does not exist, the pfctl(8) prints warning 'pfctl: DIOCGETRULES: Invalid argument'. Few users on Solaris wants pfctl(8) to be more helpful. The 'Invalid Argument' (EINVAL) is returned when particular anchor is not found. Pat

Re: pfctl can do a better job when handling ioctl(2) errors

2017-09-26 Thread Alexandr Nedvedicky
Hello Mike, thanks for looking at it. > > Can you please reverse the check so that adding conditions is possible > and the default is in the 'else' branch, i.e. > > if (errno == EINVAL) > errx(1, "Anchor '%s' not found.\n", anchorname); > else >

Re: pfctl always prints warning when flushes ruleset

2017-09-26 Thread Alexandr Nedvedicky
Hello Mike, > > Please make sure a pfctl regress doesn't run into issues with this > diff. Otherwise OK mikeb. > the regress/src/sbin/pfctl did not spot any issues. will commit my change later today. thanks a lot regards sasha

Re: ifioctl() cleanup, step 2

2017-10-11 Thread Alexandr Nedvedicky
Hello, just for my curiosity: why do you leave some returns untreated? is that intentional? just like here: @@ -2048,8 +2048,11 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) case SIOCGVNETID: case SIOCGIFPAIR: case SIOCGIF

Re: ifioctl() cleanup, step 2

2017-10-11 Thread Alexandr Nedvedicky
On Wed, Oct 11, 2017 at 12:43:45PM +0200, Martin Pieuchot wrote: > On 11/10/17(Wed) 12:28, Alexandr Nedvedicky wrote: > > Hello, > > > > just for my curiosity: > > > > why do you leave some returns untreated? is that intentional? > > Yes it is intentio

Re: queue packet in loopback

2017-10-18 Thread Alexandr Nedvedicky
Hello, thanks for great explanation of the problem. I like your approach. I have just two small nitpicks/questions see below. > > Index: net/if_loop.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_loop.c,v > retrievin

Re: queue packet in loopback

2017-10-18 Thread Alexandr Nedvedicky
Hello, On Wed, Oct 18, 2017 at 08:13:09PM +0200, Alexander Bluhm wrote: > On Wed, Oct 18, 2017 at 10:48:30AM +0200, Alexandr Nedvedicky wrote: > > I think you also want to add if_ih_remove() to loop_clone_destroy(). > > Yes, thanks for catching that. > > > I

fine tuning PF_LOCK in pfioctl()

2017-10-27 Thread Alexandr Nedvedicky
Hello, patch below fine tunes PF_LOCK in pfioctl() function. Currently pfioctl() function grabs PF_LOCK() for all operations at line 1006, right before 'the big switch' is entered. The PF_LOCK() gets released once we are done with 'the big switch' at line 2477: 905 int 906 pfioctl(dev_t dev

Re: fine tuning PF_LOCK in pfioctl()

2017-10-30 Thread Alexandr Nedvedicky
Hello, > > patch below move responsibility for PF_LOCK manipulation to particular ioctl > > commands. The change is investment for future. It will allow us to gradually > > move individual PF subsystems out of PF_LOCK scope. > > I'm always afraid when an ioctl(2) logic contains multiple return o

add one more softnet taskq

2017-10-30 Thread Alexandr Nedvedicky
Hello, patch below adds additional softnet taskq. This will allow certain degree of parallelism for packet processing in pf_test(). The current plan is to let packets received by even NICs (even ifindex) to be processed by task0, packets received by odd NICs (odd ifindex) by task1. big thanks sho

Re: add one more softnet taskq

2017-10-30 Thread Alexandr Nedvedicky
Hello, thank you for looking at my changes. updated patch is below. The list of changes is as follows: o SOFTNET_TASKS 1 is defined to 1 o call to task_add(): @@ -1839,5 +1839,5 @@ void ip_send(struct mbuf *m) { mq_enqueue(&ipsend_mq, m); - task_add(soft

Re: add one more softnet taskq

2017-10-30 Thread Alexandr Nedvedicky
Hello, just in case there is yet another brave soul running the diff below: Hrvoje has managed to trip the ASSERT() in ip_send() function at line 1843. I assume the ip6_send() is prone to same error. I'll try to figure out what's going on and send updated diff shortly. Big thanks to Hrv

Re: add one more softnet taskq

2017-10-31 Thread Alexandr Nedvedicky
Hello, On Tue, Oct 31, 2017 at 09:29:10AM +0100, Martin Pieuchot wrote: > On 31/10/17(Tue) 06:56, Alexandr Nedvedicky wrote: > > Hello, > > > > just in case there is yet another brave soul running the diff below: > > > > Hrvoje has managed to trip the

NULL pointer dereference in ip*_send()

2017-11-01 Thread Alexandr Nedvedicky
Hello, the patch [1] I've committed yesterday needs a follow up commit below. the problem was found by Hrvoje. my deep apologize regards sasha [1] https://marc.info/?l=openbsd-tech&m=150944369215209&w=2 8<---8<---8<--8< diff --git a/sys/

Re: un-KERNEL_LOCK() TCP/UDP input & co

2017-11-13 Thread Alexandr Nedvedicky
Hello, your change looks good to me. Though I have a comment/question to your diff. > Index: net/if_vxlan.c > === > RCS file: /cvs/src/sys/net/if_vxlan.c,v > retrieving revision 1.63 > diff -u -p -r1.63 if_vxlan.c > --- net/if_vxlan

Re: un-KERNEL_LOCK() TCP/UDP input & co

2017-11-13 Thread Alexandr Nedvedicky
Hello, > > So I'd like to know, what's the plan for NET_ASSERT_LOCKED() macro? > > > > a) are we going to relax it, so it will test if the net_lock is > > locked? > > Yep, that's already done. cool, thanks a lot. I've just noticed the change is there while ago, while reading

Re: Question about tables in nested anchor on pf since 6.1

2017-11-14 Thread Alexandr Nedvedicky
Hello Leo, this looks like my bad, which goes back to commit [1], which tried to fix 'mix up of anchor names and anchor paths'. I've completely forgot to take care of pfctl/parse.y back then. Please let me know if patch below solves your problem. thank you for great troubleshooting and excellent t

Re: frag6 splassert net lock

2017-11-14 Thread Alexandr Nedvedicky
Hello, I would go for NET_RLOCK() instead of NET_LOCK(), which is currently alias to NET_WLOCK(). My point is the icmp6_reflect(), which is the workhorse for icmp6_error(), is a typical 'READER-user' of network stack. It does not change any network configuration. So it should be fine to let it run

Re: NET_RLOCK in ifioctl()

2017-11-14 Thread Alexandr Nedvedicky
Hello, I like this change and I think it should go in. ok sashan On Mon, Nov 13, 2017 at 04:54:11PM +0100, Theo Buehler wrote: > The diff below pushes the NET_LOCK into ifioctl() and reduces it to > NET_RLOCK where possible. In particular, this will allow SIOCG* requests > to run in parallel. >

Re: avoid pcb lookup in pf forward

2017-11-22 Thread Alexandr Nedvedicky
Hello, your change looks good to me as-is. Though the patch itself drags my attention to line here in pf_test(): > @@ -7072,7 +7083,8 @@ done: > > #ifdef INET6 > /* if reassembled packet passed, create new fragments */ > - if (pf_status.reass && action == PF_PASS && pd.m && fwdir ==

Re: avoid pcb lookup in pf forward

2017-11-22 Thread Alexandr Nedvedicky
Hello, On Wed, Nov 22, 2017 at 01:45:39PM +0100, Alexander Bluhm wrote: > On Wed, Nov 22, 2017 at 09:49:06AM +0100, Alexandr Nedvedicky wrote: > > > /* if reassembled packet passed, create new fragments */ > > > - if (pf_status.reass && action == PF_PASS &&

pfctl and anchors: few more glitches found

2017-11-23 Thread Alexandr Nedvedicky
Hello, patch below fixes various glitches I've found in pfctl. All issues are related to anchors. I did test the patch using regress/sbin/pfctl. Although all tests have passed, I still would like to ask for testing in field, especially if you use some fancy configuration with 'load anchor'. The pa

pfctl rule optimizer: anchor name vs. anchor path mix up

2017-11-24 Thread Alexandr Nedvedicky
Hello, this is yet another occurrence of infamous 'name vs. path mix up' [1]. Leonardo Guardati hit this bug in rule optimizer this time. The patch below is part of 'the big diff' I've sent while ago [2]. OK? thanks and regard sasha [1] https://marc.info/?l=openbsd-tech&m=147289981326044&w=2 [

patching use-after-free and innocent memory leak in pfctl_optimzie.c

2017-11-24 Thread Alexandr Nedvedicky
Hello, the patch below is part of larger diff [1] I've sent earlier. Leonardo seen a pfctl.core, when pfctl_optimize failed to create a radix table. The use after free happens in superblock_free() at 1621: 1618 while ((por = TAILQ_FIRST(&block->sb_rules))) { 1619 TAILQ_RE

pfctl fails to handle nested 'load anchor' properly

2017-11-24 Thread Alexandr Nedvedicky
Hello, this diff is part of the 'big patch' [1] to pfctl I've sent while back. The pfctl fails to handle nested 'load anchor' statements properly, when ruleset is being loaded to non-root anchor (e.g. pfctl -a regress ...), see [1] to find more details about the issue. The first step towards the

Re: patching use-after-free and innocent memory leak in pfctl_optimzie.c

2017-11-25 Thread Alexandr Nedvedicky
On Fri, Nov 24, 2017 at 07:22:58PM +0100, Alexander Bluhm wrote: > On Fri, Nov 24, 2017 at 01:11:08PM +0100, Alexandr Nedvedicky wrote: > > the patch below is part of larger diff [1] I've sent earlier. Leonardo > > seen a > > pfctl.core, when pfctl_optimize failed to

Re: pfctl divert type

2017-11-27 Thread Alexandr Nedvedicky
Hello, > Hi, > > The divert structure uses the port number to indicate that divert-to > or divert-reply is active. Divert packet uses a separate structure. > This is confusing and makes it hard to add new features. It is > better to have a divert type that explicitly says what is configured. >

pfctl regression tests for load anchor

2017-11-27 Thread Alexandr Nedvedicky
Hello, adds two test cases for issues reported by Leonardo. I've created extra pfloadanchors target in regress/sbin/pfctl/Makefile. The 'load anchor ... from ' construct still needs more love to be covered by existing targets. consider output for command 'pfctl -o none -nvf pf113.in':

Re: pf divert type

2017-11-28 Thread Alexandr Nedvedicky
Hello, your change looks good to me as-is. Though I have one small suggestion, which I don't insist on: > Index: sys/netinet/raw_ip.c > === > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/raw_ip.c,v > retrieving revision 1.106

Re: pcb lookup listen PF_TAG_TRANSLATE_LOCALHOST

2017-12-01 Thread Alexandr Nedvedicky
Hello, On Thu, Nov 30, 2017 at 06:09:16PM +0100, Alexander Bluhm wrote: > Hi, > > I would like to simplify the reverse pcb lookup logic. The > PF_TAG_TRANSLATE_LOCALHOST security check predates the divert > feature. It prevents that you accidentally use redirect where a > divert-to would be ap

Re: pf divert socket lookup

2017-12-01 Thread Alexandr Nedvedicky
Hello, On Fri, Dec 01, 2017 at 04:43:30PM +0100, Alexander Bluhm wrote: > Hi, > > I want to make divert lookup similar for all socket types: > > If PF_TAG_DIVERTED is set, pf_find_divert() cannot fail so put an > assert there. Explicitly check all possible divert types, panic > in the default

Re: pf neighbor discovery hop limit

2017-12-04 Thread Alexandr Nedvedicky
Hello, On Mon, Dec 04, 2017 at 02:55:16PM +0100, Alexander Bluhm wrote: > Hi, > > RFC 4861 requires that all neighbor discovery packets have 255 in > their IPv6 header hop limit field. Let pf drop neighbor solicitation, > neighbor advertisement, router solicitation, router advertisement, > and

<    1   2   3   4   5   6   7   >