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

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/sys/net

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

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

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

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

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: 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 thanks a lot, I'm O

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

Re: pf_purge_thread() needs NET_LOCK()

2017-01-06 Thread Alexandr Nedvedicky
On Fri, Jan 06, 2017 at 12:21:19PM +0100, Martin Pieuchot wrote: > Right now for pfsync(4), but later it will need it to serialize access > to PF data structures. > > splassert: ip_output: want 1 have 0 > ip_output() at ip_output+0x7d > pfsync_sendout() at pfsync_sendout+0x499 >

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

Re: pf: percpu anchor stacks

2017-03-24 Thread Alexandr Nedvedicky
Hello, I'm attaching patch, which removes stack-as-a-global variable. it's updated patch [1] to current tree. sorry for being pushy advocating my old, rusty patch. thanks and regards sasha [1]

Re: pf: percpu anchor stacks

2017-03-28 Thread Alexandr Nedvedicky
Hello Mike, thank you for looking at my patch. I accept most of your comments. I believe the items below deserve further discussion. > - instead of checking "rv" against 0 in the "break on quick >rule or failure" I'd like to see an actual check against >PF_TEST_* values so that it's

Re: pf: percpu anchor stacks

2017-03-19 Thread Alexandr Nedvedicky
Hello, I've sent different patch [1], which was touching same functions some time ago. The old patch [1] basically splits pf_test_rule() to two functions: pf_test_rule() pf_match_rule(), which walks anchor stack recursively. the recursion depth is limited to 64. the memory foot

Re: delete pf states by exact flow info and speed it up

2017-04-20 Thread Alexandr Nedvedicky
Hello, I finally got to your patch. I'm fine with your to extend pfctl kill operation. I've found just few nits in your change. 1) your manpage diff should include one more change below: 8<---8<---8<--8< diff -r c435e977886a -r

Re: Sun T2000 internal communications

2017-04-19 Thread Alexandr Nedvedicky
Hello, I'm not sure I'll be able to help you. I'm using ldoms on T5 running Solaris 11.2. According to blog [1], the /var/adm/messages in primary domain (ldom0) should give you some hint on what is going on. I hope it will help you to get unstuck. regards sasha [1]

Re: pfctl(8): reduce -k

2017-04-23 Thread Alexandr Nedvedicky
Hello, I'm fine with your pfctl.c change. Although I like your brief version of manpage, wearing admin's hat, I'm somewhat missing line: > -k host | network | label | key | id so how about one small change below: 8<---8<---8<--8<

Re: delete pf states by exact flow info and speed it up

2017-04-23 Thread Alexandr Nedvedicky
Hello, On Sun, Apr 23, 2017 at 12:18:07AM +0200, Hiltjo Posthuma wrote: > On Sat, Apr 22, 2017 at 08:14:06AM +0900, YASUOKA Masahiko wrote: > > On Fri, 21 Apr 2017 13:52:51 +0900 (JST) > > YASUOKA Masahiko wrote: > > > +int > > > +pfctl_parse_host(char *str, struct

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

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

Re: delete pf states by exact flow info and speed it up

2017-04-21 Thread Alexandr Nedvedicky
On Sat, Apr 22, 2017 at 08:14:06AM +0900, YASUOKA Masahiko wrote: > On Fri, 21 Apr 2017 13:52:51 +0900 (JST) > YASUOKA Masahiko wrote: > > +int > > +pfctl_parse_host(char *str, struct pf_rule_addr *addr) > > +{ > (snip) > > + if ((sbs = strchr(s, '[')) != NULL || (sbe =

Re: pf: percpu anchor stacks

2017-05-15 Thread Alexandr Nedvedicky
Hello, > Now *is* the time to commit the first step, the refactoring. Once > that's done we can discuss the introduction of the context. > > Could you come up with such diff? first of all: I have not managed to finish the re-factoring step yet, work is still in progress. I stole some

Re: pfsync and option WITH_PF_LOCK

2017-06-09 Thread Alexandr Nedvedicky
Hello, would it make sense to commit a poor man's solution below, before pfsync(4) will get to shape? The idea is to grab PF_LOCK, just before we pass the data to PF for further processing. regards sasha 8<---8<---8<--8< ---

Re: pfctl: make functions return void, merge two ifs

2017-06-12 Thread Alexandr Nedvedicky
Hello Adam, > It was a rainy evening here, so here's the updated pfctl diff. I'm sorry to hear about the rainy weather [1]. anyway, you might want to run regression test for pfctl. cd $SRC/src/regress/sbin/pfctl cat Makefile # follow instructions just for

Re: pf statekey inp assertion failed

2017-06-21 Thread Alexandr Nedvedicky
Hello, > The problem is that setting the inp pointer in the statekey to NULL > is delayed until the statekey refcounter reaches 0. So the inp > could get linked to another statekey while the mbuf in the socket > buffer was keeping the refcounter at 1. > > The sk->inp should be set to NULL

Re: pf fragment data structure

2017-06-23 Thread Alexandr Nedvedicky
Hello, I've got just one nit, which is probably overcautious, so I don't insist: > @@ -231,8 +245,16 @@ void > pf_free_fragment(struct pf_fragment *frag) > { > struct pf_frent *frent; > + struct pf_frnode*frnode; > > - RB_REMOVE(pf_frag_tree, _frag_tree, frag);

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

Re: pf fragment drop stale

2017-06-26 Thread Alexandr Nedvedicky
Hello, looks good to me, though I still need better explanation for PF_FRAG_STALE. The current comment seems bit misleading to me. > #define PFTM_TS_DIFF_VAL 30 /* Allowed TS diff */ > > +#define PF_FRAG_STALE200 /* Limit fragments per second per > connection */

Re: pppoe(4) vs splnet

2017-05-26 Thread Alexandr Nedvedicky
Hello, > I hope my question does not sound dumb... > > The function pppoe_timeout() grabs NET_LOCK() at line 1059 and then > it goes after splnet() at line 1076, is that intentional? > if so why? do I missing something? I guess the question was dump. your patch deals with NET_LOCK()

Re: pfsync(4) vs splnet()

2017-05-26 Thread Alexandr Nedvedicky
On Fri, May 26, 2017 at 04:10:25PM +0200, Martin Pieuchot wrote: > pfsyncioctl already runs under the NET_LOCK() and pfsync(4) doesn't > touch any hardware so these splnet() are useless. > > ok? looks good to me. OK sashan@

Re: pppoe(4) vs splnet

2017-05-26 Thread Alexandr Nedvedicky
Hello, I hope my question does not sound dumb... The function pppoe_timeout() grabs NET_LOCK() at line 1059 and then it goes after splnet() at line 1076, is that intentional? if so why? do I missing something? 1059 NET_LOCK(s);

Re: ppp vs splnet()

2017-05-27 Thread Alexandr Nedvedicky
Hello, this looks good to me too. OK sashan@ On Fri, May 26, 2017 at 04:22:29PM +0200, Martin Pieuchot wrote: > The global list of softc is used in the input path and need to be > protected by the NET_LOCK(). > > ok? >

Re: enc(4) vs splnet

2017-05-27 Thread Alexandr Nedvedicky
Hello, On Fri, May 26, 2017 at 05:11:22PM +0200, Martin Pieuchot wrote: > The global array of interfaces is accessed in the input path and need > the NET_LOCK(). > > Ok? > looks good to me. OK sashan@

Re: trunk(4) vs splnet

2017-05-27 Thread Alexandr Nedvedicky
Hello, On Fri, May 26, 2017 at 04:54:57PM +0200, Martin Pieuchot wrote: > The global list of softc is *not* accessed in the input path, so it > doesn't need splnet(). > > ioctl(2) handlers are already executed with the NET_LOCK() held, so > splnet() is superfluous. changes look good to me, but

Re: trunk(4) vs splnet

2017-05-27 Thread Alexandr Nedvedicky
Hello, On Sat, May 27, 2017 at 08:45:31PM +0200, Martin Pieuchot wrote: > On 27/05/17(Sat) 17:33, Alexandr Nedvedicky wrote: > > Hello, > > > > On Fri, May 26, 2017 at 04:54:57PM +0200, Martin Pieuchot wrote: > > > The global list of softc is *not* accessed in the i

Re: Test wanted: IPv4 forwarding w/o KERNEL_LOCK()

2017-05-28 Thread Alexandr Nedvedicky
Hello Martin, I think you need to do NET_UNLOCK() before return (error); > > /* Shutdown and remove trunk ports, return on error */ > + NET_LOCK(s); > while ((tp = SLIST_FIRST(>tr_ports)) != NULL) { > if ((error = trunk_port_destroy(tp)) != 0) >

Re: Be careful w/ if_get()

2017-05-28 Thread Alexandr Nedvedicky
On Sun, May 28, 2017 at 08:53:16PM +0200, Martin Pieuchot wrote: > Trying to grab the NET_LOCK() while holding an ifp reference is wrong. > This creates deadlock as found by Hrvoje Popovski. The reason is that > if_detach() sleeps holding the NET_LOCK() while waiting for others > threads to

Re: PF + pflow and NET_LOCK() recursion

2017-05-29 Thread Alexandr Nedvedicky
Hello, On Mon, May 29, 2017 at 02:32:06PM +0200, Martin Pieuchot wrote: > Now that packets are queued the recursion is gone and we can keep > the lock. makes sense. please commit this diff so I can pull back your change from tree to my patch. OK sashan@

Re: pf: percpu anchor stacks

2017-05-19 Thread Alexandr Nedvedicky
Hello, On Fri, May 19, 2017 at 06:10:54PM +0200, Alexander Bluhm wrote: > On Mon, May 15, 2017 at 03:19:19PM +0200, Alexandr Nedvedicky wrote: > > I'm attaching updated final patch, which accepts your suggestion. > > I think this broke sys/net/pf_forward. > http://bluh

let's add PF_LOCK()

2017-05-30 Thread Alexandr Nedvedicky
Hello, patch delivers two changes to PF: it adds PF_LOCK() et. al. At the moment the PF_LOCK() sort of duplicates the current NET_LOCK(). It essentially synchronizes packets with ioctl(2) and timer thread, which purges states. The future work is going to break PF_LOCK into

Re: let's add PF_LOCK()

2017-05-30 Thread Alexandr Nedvedicky
oh, not again... I'm sorry for not attaching patch to the first email On Tue, May 30, 2017 at 02:34:32PM +0200, Alexandr Nedvedicky wrote: > Hello, > > patch delivers two changes to PF: > > it adds PF_LOCK() et. al. At the moment the PF_LOCK() sort of > duplicates th

Re: let's add PF_LOCK()

2017-05-31 Thread Alexandr Nedvedicky
Hello, > > Could you you make 2 definitions for the lock? It doesn't make sense > to enable them by default for now. I'd like to see you diff committed > now with empty defines and an easy way to enable it. > > That means ok mpi@ if the defines to not take/release locks by default. > I

Re: let's add PF_LOCK()

2017-05-31 Thread Alexandr Nedvedicky
Hello Mike, I'd like to ask you to take a one more look to change I'm going to commit. I did one more check of changes with respect to WITH_PF_LOCK and found one more bit to fix. We need to keep pf_purge_expired_fragments() under protection of NET_LOCK()

Re: let's add PF_LOCK()

2017-06-01 Thread Alexandr Nedvedicky
Hello, > > diff -r 6abbb123112a .hgtags > > --- /dev/null Thu Jan 01 00:00:00 1970 + > > +++ b/.hgtags Wed May 31 10:42:50 2017 +0200 > > @@ -0,0 +1,1 @@ > > +d545881e2652dbc0c057691a39a095bce92f441f pf-lock.baseline > > Please be careful and don't include VCS's goo in diffs. >

Re: pfsync and option WITH_PF_LOCK

2017-06-09 Thread Alexandr Nedvedicky
On Fri, Jun 09, 2017 at 03:48:49PM +0200, Hrvoje Popovski wrote: > On 9.6.2017. 14:55, Alexandr Nedvedicky wrote: > > Hello, > > > > > > On Fri, Jun 09, 2017 at 01:11:01PM +0200, Alexander Bluhm wrote: > >> On Fri, Jun 09, 2017 at 10:55:46AM +0200, Alexandr N

Re: pfsync and option WITH_PF_LOCK

2017-06-09 Thread Alexandr Nedvedicky
Hello, On Fri, Jun 09, 2017 at 01:11:01PM +0200, Alexander Bluhm wrote: > On Fri, Jun 09, 2017 at 10:55:46AM +0200, Alexandr Nedvedicky wrote: > > would it make sense to commit a poor man's solution below, before pfsync(4) > > will get to shape? The idea is to grab PF_LOCK, just

Re: pf: remove pfr_{sin,sin6,mask} globals

2017-05-08 Thread Alexandr Nedvedicky
Hello Patrick, your diff looks OK to me. thanks and regards sasha On Mon, May 08, 2017 at 02:56:55PM +0200, Patrick Wildt wrote: > Hi, > > in order to reduce globals so that we can run more parts of pf in > parallel, this diff removes the pfr_sin, pfr_sin6 and pfr_mask > globals. Those are

Re: let's add PF_LOCK()

2017-05-30 Thread Alexandr Nedvedicky
Hello Mike, thanks for looking at my stuff. > > It's great stuff, I think you should move forward with this. > A couple of nits: > > - pfvar*.h doesn't have tabs after #define, just one space fixed > > - do you really want to put MTX in PF_FRAG_MTX_* defines? >why not PF_FRAG_LOCK?

Re: let's add PF_LOCK()

2017-05-30 Thread Alexandr Nedvedicky
Hello Martin, > > rw_exit_write(); > > export_pflow(cur); > > rw_enter_write(); > > + rw_enter_write(_lock); > > } > > This is not needed, you're not diffing against the latest version of > net/pf.c. indeed my tree is old by couple hours.

Re: pf: percpu anchor stacks

2017-05-19 Thread Alexandr Nedvedicky
Hello, would you be able to try patch below to check if it will fix pf_forward failures? thanks a lot and sorry for inconveniences regards sasha 8<---8<---8<--8< diff -r eb40d8d52679 src/sys/net/pf.c --- a/src/sys/net/pf.c Fri May 19

Re: let's add PF_LOCK()

2017-06-05 Thread Alexandr Nedvedicky
Hello, > I don't think it matters. We still need more diffs to be able to test > that. > > Now I'd really like if you could commit this so we continue improving it > in-tree. I've commit the patch _without_ the diff to sys/conf/GENERIC, we can add it later, once more diffs will be in.

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

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: 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 proceed to

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

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.

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: pf.4: sync structs with net/pfvar.h

2017-08-28 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

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

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 >

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

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

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

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

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(>sb_rules))) { 1619

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

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=147289981326044=2 [2]

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

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_PA

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

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

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

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

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=150944369215209=2 8<---8<---8<--8< diff --git

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

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

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

Re: add one more softnet taskq

2017-10-31 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

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 t

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(_mq, m); - task_add(softnettq,

Re: pf route to on loopback

2018-05-10 Thread Alexandr Nedvedicky
Hello, On Wed, May 09, 2018 at 07:05:30PM +0200, Alexander Bluhm wrote: > Hi, > > I while ago I changed pf route-to that it does not send packets > from 127.0.0.1 address to the network. This is necessary for localy > generated icmp packets that would be dropped otherwise. > > Now I found out

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

Re: pf state key linking mbuf assert

2017-12-29 Thread Alexandr Nedvedicky
Hello, On Fri, Dec 29, 2017 at 11:04:07PM +0100, Alexander Bluhm wrote: > On Thu, Dec 28, 2017 at 12:46:45AM +0100, Alexandr Nedvedicky wrote: > > I like your change. You have my OK with small change here: > > I have commitet my diff with a small addition. I fixed a memory &g

Re: pf state key link inp

2017-12-23 Thread Alexandr Nedvedicky
Hello, > ok? sure, no objections/comments OK sashan@

Re: Zap PF_TRANS_ALTQ

2018-01-19 Thread Alexandr Nedvedicky
On Thu, Jan 18, 2018 at 11:15:41PM -0500, Lawrence Teo wrote: > Nothing uses PF_TRANS_ALTQ anymore, so zap it. > > ok? I'm fine with removing it. I'm just concerned about potential impact on port tree as there might be some tool/library still trying to use PF_TRANS_ALTQ. I just don't

Re: Explicitly check PF_TRANS_RULESET

2018-01-19 Thread Alexandr Nedvedicky
On Thu, Jan 18, 2018 at 11:12:59PM -0500, Lawrence Teo wrote: > The pf(4) DIOCX{BEGIN,COMMIT,ROLLBACK} calls support two ruleset types: > PF_TRANS_RULESET and PF_TRANS_TABLE. > > However, their switch statements in pf_ioctl.c only check for > PF_TRANS_TABLE and do not check PF_TRANS_RULESET at

<    1   2   3   4   5   6   7   >