regression in pflog output

2018-02-08 Thread Alexandr Nedvedicky
Hello, Matthias Pitzl discovered a regression introduced by my earlier commit [1]. Matthias has noticed the pflogd output changes for ruleset here: 8<---8<---8<--8< block out log quick from any to 1.1.1.1 block out log quick from any

Re: pf badopts routing header

2017-12-27 Thread Alexandr Nedvedicky
Hello, On Wed, Dec 27, 2017 at 05:34:28PM +0100, Alexander Bluhm wrote: > Hi, > > pf drops IPv4 packets with options by default. For IPv6 the same > is done for certain option headers. I think we should add the > routing header to this list. > > ok? OK sashan@

Re: pf state key linking

2017-12-27 Thread Alexandr Nedvedicky
Hello, On Tue, Dec 26, 2017 at 10:02:00PM +0100, Alexander Bluhm wrote: > Hi, > > The functions to link the pf state keys always confused me a bit. > I think the new naming and code of the functions is more consistent. > > ok? I like your change. You have my OK with small change here: >

Re: pfctl: zap v4mask and v6mask in host()

2018-07-31 Thread Alexandr Nedvedicky
On Mon, Jul 30, 2018 at 06:08:54PM +0200, Klemens Nanni wrote: > These two just complicate the code. Let's defer checks whether an > explicit mask has been specified to where it's actually set in host_*(). > > My motivation is to reduce address family specific code and perhaps even > merge

Re: pfctl: Simplify host()

2018-07-30 Thread Alexandr Nedvedicky
Hello, On Sun, Jul 29, 2018 at 12:35:22PM +0200, Klemens Nanni wrote: > This gets rid of the `cont' flag and squashes the code a bit. `host_*()' > are pretty self explanatory so I zapped the comments as well. > > Regress tests pass, no issues in production use. > > Feedback? OK? reads OK

Re: pfctl: use mask parameter and zap bits in host_v4()

2018-08-10 Thread Alexandr Nedvedicky
On Thu, Aug 09, 2018 at 12:01:41PM +0200, Klemens Nanni wrote: > On Wed, Aug 01, 2018 at 01:27:48AM +0200, Klemens Nanni wrote: > > Updated diff bringing this function in line with its counterparts by > > using `mask' the same way. > > > > If a mask was specified, `mask' would always equal to

Re: pfctl: remove tiny `-T create' leftover

2018-07-19 Thread Alexandr Nedvedicky
On Fri, Jul 20, 2018 at 01:11:29AM +0200, Klemens Nanni wrote: > Or simply check for `show' and `test'. > > Index: pfctl.c > === > RCS file: /cvs/src/sbin/pfctl/pfctl.c,v > retrieving revision 1.355 > diff -u -p -r1.355 pfctl.c > ---

Re: pfctl: reduce duplicate code, fix typo/free correct buffer

2018-07-15 Thread Alexandr Nedvedicky
Hello, On Sun, Jul 15, 2018 at 02:53:03PM +0200, Klemens Nanni wrote: > Two enhancements, one bug in `filteropts_to_rule()': > > * Merge `once' handling from `anchorrule' and `pfrule' > * Remove/shorten duplicate code block > * Fix typo I introduced with r1.678 that frees the wrong buffer

Re: pfctl: use strtonum in host()

2018-07-23 Thread Alexandr Nedvedicky
Hello, On Mon, Jul 23, 2018 at 11:16:16AM +0200, Klemens Nanni wrote: > strtonum(3) is simpler than checking three cases for `q' and gives nicer > error messages. While here, use `v6mask' as maximum netmask instead of > hardcoding it. > > OK? I'm OK with your change. I have just one small

Re: pfctl: simplify getaddrinfo() error handling in host_dns()

2018-07-23 Thread Alexandr Nedvedicky
On Mon, Jul 23, 2018 at 11:19:50AM +0200, Klemens Nanni wrote: > We're not using `error' anyway so drop the variable and jump to the end. > > OK? OK sashan

if_cloners list is poulated at system boot only

2018-09-09 Thread Alexandr Nedvedicky
Hello, while poking around 'XXXSMP' comments in net/if.c, I've noticed the 'if_cloners_lock' can be removed. The thing is the list of cloners gets populated/modified at system boot only, while kernel attaches device drivers. And because cloners are never removed, the if_clone_detach() function

Re: if_cloners list is poulated at system boot only

2018-09-09 Thread Alexandr Nedvedicky
Hello, On Sun, Sep 09, 2018 at 10:50:08AM +0200, Alexander Bluhm wrote: > On Sun, Sep 09, 2018 at 08:41:07AM +0200, Alexandr Nedvedicky wrote: > > void > > if_clone_attach(struct if_clone *ifc) > > { > > - rw_enter_write(_cloners_lock); > > LIST_IN

Re: pfctl: merge host_v{4,6}() into simpler host_ip()

2018-09-10 Thread Alexandr Nedvedicky
Hello, On Mon, Sep 10, 2018 at 12:46:22AM +0200, Klemens Nanni wrote: > Reduce duplicate code, make similar paths such as the memcpy() calls > more uniform to simplify upcoming diffs and tidy up a bit. > > Feedback? OK? I'm OK with change. regards sasha

Re: pfctl: introduce copy_satopfaddr()

2018-09-10 Thread Alexandr Nedvedicky
On Mon, Sep 10, 2018 at 06:01:05PM +0200, Alexander Bluhm wrote: > On Mon, Sep 10, 2018 at 05:52:43PM +0200, Klemens Nanni wrote: > > On Mon, Sep 10, 2018 at 05:41:56PM +0200, Alexander Bluhm wrote: > > > I would prefer to access pfa.v6 than to rely on the fact that pf_addr > > > contains a union

Re: pfctl: error out early on bad anchor usage

2018-09-08 Thread Alexandr Nedvedicky
On Sat, Sep 08, 2018 at 02:35:04PM +0200, Klemens Nanni wrote: > Updated diff using `mode' so the intent is even clearer now that I also > merged my next diff: > > Fail much earlier when trying to write anchors beginning with "_". > This avoids the duplicate check as well as everything between

pfsync: avoid a recursion on PF_LOCK

2018-09-27 Thread Alexandr Nedvedicky
Hello, patch below is missing piece to stuff, which I commit on n2k18 [1]. Fairly quickly people, who have PF deployed with pfsync (and are willing to experiment), discovered a panic on PF_LOCK recursion. The recursion is identified by stack as follows: login: panic: rw_enter: pf_lock

Re: pfsync: avoid a recursion on PF_LOCK

2018-09-27 Thread Alexandr Nedvedicky
On Thu, Sep 27, 2018 at 11:30:09PM +0200, Hrvoje Popovski wrote: > On 27.9.2018. 18:34, Alexandr Nedvedicky wrote: > > Mentioning parallelism: there is yet another change you need to perform > > in order to get more pf_test() instances running. Currently there > > is only sin

Re: pf: honor quick on anchor rules

2018-10-15 Thread Alexandr Nedvedicky
Hello, > That is still different from the original (2006) behaviour which > would terminate ruleset-evaluation IFF any rules inside the anchor > actually match. Perhaps I still misunderstand the whole anchor thing. let me clarify the proposed change by two examples. Ruleset below

Re: pf/pfctl: use PFR_FB_NONE consistently

2018-10-15 Thread Alexandr Nedvedicky
Hello, On Sat, Oct 13, 2018 at 10:56:21PM +0200, Klemens Nanni wrote: > Replace hardcoded 0 and implicit checks with enum as done in all other > use cases of `pfra_fback'. either way is fine with me, so I'm not going to object your change. OK sashan

Re: pf: honor quick on anchor rules

2018-10-15 Thread Alexandr Nedvedicky
Hello, I just got back on-line after a week. Thank you all for detailed analysis. This particular bug, which Klemens tries to fix is caused by my commit 1.1024: - percpu anchor stacks we actually don't need to pre-allocate per_anchor_stack[], if we use a 'natural' recursion, when

Re: pf: honor quick on anchor rules

2018-10-16 Thread Alexandr Nedvedicky
Hello, > > In my understanding they do, but you are missing the point. Given the > following ruleset: > > pass > anchor quick { > block inet6 > } > block > > With the current fix ruleset-evaluation terminates after the anchor, > before the last block even *if no ipv6-traffic was

Re: pf: honor quick on anchor rules

2018-10-16 Thread Alexandr Nedvedicky
Hello, > This comment is way too much, imho. How about this: > > Unless errors occured, stop iff any rule matched > within quick anchors. I'm fine with your proposal. You have my OK to commit the fix. thanks and regards sashan

Re: odd condition/test in PF lexer

2018-10-29 Thread Alexandr Nedvedicky
Hello, I think time has come to ask for OKs. The updated patch is below. The issue has been found within PF's parse.y here: 5279 } else if (c == '\\') { 5280 if ((next = lgetc(quotec)) == EOF) 5281 return (0); 5282

Re: parse.y: strndup() in cmdline_symset()

2018-11-06 Thread Alexandr Nedvedicky
Hello, the change looks good to me. (I was delaying my OK hoping someone else will chip-in with OK for iked & ipsecctl stuff). IMO It makes sense to keep parse.y code in sync where possible, so go for it. OK sashan. On Thu, Nov 01, 2018 at 04:14:53PM +0800, Michael Mikonos wrote: > Hello, >

Re: pf: honor quick on anchor rules

2018-10-04 Thread Alexandr Nedvedicky
Hi Klemens, > > > Do i misread the manpage somehow? > > No, this is a bug. > Allow me a bit of rubber ducking to explain this bug and ease review: > > The kernel evaluates the ruleset pretty much like we read it: Down to > bottom until quick appears or an error occurs in which case we stop >

odd condition/test in PF lexer

2018-10-04 Thread Alexandr Nedvedicky
Hello, a static analyzer we use for Oracle Solaris recently discovered odd if () test/condition in yylex() here sbin/pfctl/parse.y: 5279 } else if (c == '\\') { 5280 if ((next = lgetc(quotec)) == EOF) 5281 return (0); 5282

Re: pfsync: avoid a recursion on PF_LOCK

2018-10-02 Thread Alexandr Nedvedicky
On Tue, Oct 02, 2018 at 01:14:16AM +0200, Alexander Bluhm wrote: > On Thu, Sep 27, 2018 at 06:34:45PM +0200, Alexandr Nedvedicky wrote: > > OK to pfsync change? > > OK bluhm@, just two style nits > > > + if ((e = ip_output(m, NULL, NULL, IP_RAWOUTPUT, >sc_im

Re: pfctl: bail out early on missing table command, zap wrapper

2019-01-02 Thread Alexandr Nedvedicky
Hello, On Tue, Jan 01, 2019 at 08:17:43PM +0100, Klemens Nanni wrote: > Synopsis is `[-t table -T command [address ...]]', yet tables without > commands are silently ignored: > > $ pfctl -t t > pfctl: /dev/pf: Permission denied > # pfctl -t t ; echo $? > 0 > > Commands

Re: pfctl: zap unused struct segment

2019-01-02 Thread Alexandr Nedvedicky
On Tue, Jan 01, 2019 at 10:26:47PM +0100, Klemens Nanni wrote: > There since import and last used by ALTQ which henning removed in 2004. > > OK? > OK sashan > Index: sbin/pfctl//pfctl.h > === > RCS file:

Re: pfctl: tables: improve namespace collision warnings

2019-01-02 Thread Alexandr Nedvedicky
Hello, I don't object your change. However I hesitate to give OK too. I hope PF users, who have non-trivial rulesets will speak up here. IMO opinion we are hitting limitations of pfctl(8) here. Making warnings more useful requires to introduce some additional hints to pfctl, to better express,

Re: pfctl: zap unused function parameter

2019-01-06 Thread Alexandr Nedvedicky
Hello, On Sat, Jan 05, 2019 at 10:10:07PM +0100, Klemens Nanni wrote: > Never used, probably just copy/pasta since introduction in 2006. > > `-i' and other flags are completely ignored with `-K' anyway. > > OK? > OK sashan@ > Index: pfctl.c >

Re: pfctl: tables: improve namespace collision warnings

2019-01-06 Thread Alexandr Nedvedicky
Hello, > > > As I've said I don't object your change. I agree it does, > > what you intend, however I'm not sure how much it buys. > My intention is to make warnings clear and unambiguous, such that > referred table and anchor names can be copied and pasted into successive > pfctl

Re: pfctl: defuse `-F all -i ...'

2019-01-06 Thread Alexandr Nedvedicky
Hello, > mcbride introduced this code with r1.298 in 2010 but used > > if (*ifaceopt) { > > only to have stsp fix a segfault in r1.299 by changing it to the current > form. > > One might as well assume that my proposed condition was the originally > intended behaviour after all and

Re: Request for testing malloc and multi-threaded applications

2019-01-17 Thread Alexandr Nedvedicky
Hello Otto, I gave it a try with firefox. according to my subjective tests I could not spot any differences with various setting. I've decided to try with some memory benchmarks I could find on github [1]. I did create a fork [2] with my own test runner to try out your diff. To run it just do

PF should honor deprecated IPv6 addresses

2018-12-09 Thread Alexandr Nedvedicky
Hello, the issue has been brought up by claudio@ off-list: > I have this rule in my pf.conf: > > match out on egress received-on tap nat-to (egress) > > Now if I ping6 www.google.com from the VM it should rewrite the IPv6 > address to the one on my egress and all is good but it seems

Re: sys/net/pf*.[ch]: remove useless macros

2018-12-10 Thread Alexandr Nedvedicky
Hello, On Sat, Dec 08, 2018 at 09:25:04AM +0100, Klemens Nanni wrote: > All they do is case conversion^Wconfusion, so remove them. > > Relevant pfvar.h diff at the top, all other hunks were done with sed(1). > > Feedback? Objections? OK? your patch seems to be a follow up to mcbride's

Re: ddb examine hex adjustment

2019-01-09 Thread Alexandr Nedvedicky
Hello, On Wed, Jan 09, 2019 at 02:05:23PM +0100, Alexander Bluhm wrote: > Hi, > > Printing hex values with right adjustment makes it easier to compare > corresponding digits. So I would like the change the ddb x/x output. > > before: > 0x803b1038: ff007c531cf0 >

Re: pfctl: unbreak build under OPT_DEBUG

2019-01-03 Thread Alexandr Nedvedicky
Hello, On Thu, Jan 03, 2019 at 08:22:52PM +0100, Klemens Nanni wrote: > In pfctl_optimize.c r1.39 I removed the `af' parameter from `unmask()' > but accidently zapped the macro's closing paranthese. > > Since DEBUG() is needlessly under an OPT_DEBUG guard here, this was not > effecting normal

Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-02 Thread Alexandr Nedvedicky
Hello, below is diff I plan to commit. I did add a comment to pfctl_reset() and wording in manpage. thanks and regards sashan 8<---8<---8<---8<--- diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8 index 48b2893cfcd..00bd27c200a 100644 ---

Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-02 Thread Alexandr Nedvedicky
Hello, On Tue, Apr 02, 2019 at 12:59:33PM +0200, Petr Hoffmann wrote: > On 02.04.2019 12:06, Klemens Nanni wrote: > >On Tue, Apr 02, 2019 at 11:28:43AM +0200, Petr Hoffmann wrote: > >>would make me believe everything mentioned as OPTIONS in pf.conf(5) is about > >>to be reset. I see e.g. the

Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-03 Thread Alexandr Nedvedicky
Hello, On Tue, Apr 02, 2019 at 11:28:43AM +0200, Petr Hoffmann wrote: > Hi, > > seeing this in the manpage > --8<-- > +.It Fl F Cm Reset > +Reset limits, timeouts and options back to default settings. >

Re: pfctl should allow administrator to flush _anchors

2019-03-25 Thread Alexandr Nedvedicky
Hello, > > > > Isn't -U pretty close to -Fall ? > > > > > > > > > > it is, however -Fall operates on main ruleset only. -Fall also does > > > not reset limits and timeouts. Hence my first idea was to introduce > > > '-FNuke', which kills all rulesets and tables. > > > > > > I

Re: pfctl should allow administrator to flush _anchors

2019-03-26 Thread Alexandr Nedvedicky
On Mon, Mar 25, 2019 at 10:28:40PM -0400, Ted Unangst wrote: > Alexandr Nedvedicky wrote: > > it is, however -Fall operates on main ruleset only. -Fall also does > > not reset limits and timeouts. Hence my first idea was to introduce > > '-FNuke', which kills al

Re: pfctl should allow administrator to flush _anchors

2019-03-26 Thread Alexandr Nedvedicky
Hello, > > > > So how people feel about changing '-Fa' to kill all rules and tables, not > > just > > those, which are attached to main ruleset (root)? > > > > thanks and > > regards > > sashan > > > > IMHO this is a needed feature, but I agree with your hesitation about > using -Fa. This

Re: pfctl should allow administrator to flush _anchors

2019-03-24 Thread Alexandr Nedvedicky
On Sun, Mar 24, 2019 at 09:51:13AM +0100, Denis Fondras wrote: > On Sun, Mar 24, 2019 at 09:24:34AM +0100, Alexandr Nedvedicky wrote: > > I think all the above calls for a new standalone option, which I named as > > 'Unconfigure'. Patch below suggest unconfigure behavior for PF. &

Re: pfctl should allow administrator to flush _anchors

2019-03-24 Thread Alexandr Nedvedicky
Hello, I received a feedback suggesting to come up with better name than 'Nuke' > not sure about the name. > > i've often want a similar operation for network interfaces, which > returns them to 'raw' state. > > and there's also been people who want the same for the routing > table > > if we

Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-05 Thread Alexandr Nedvedicky
Hello, > > +void > > +pfctl_reset(int dev, int opts) > > +{ > > + struct pfctlpf; > > + struct pfr_buffer t; > > + int i; > > + > > + pf.dev = dev; > > + pfctl_init_options(); > > + > > + /* Force reset upon pfctl_load_options() */ > > + pf.debug_set = 1; > > +

Re: introduce 'pfctl -FR' to reset settings to defaults

2019-03-28 Thread Alexandr Nedvedicky
Hello, > > +Flush all of the above (+ reset settings). > This is fine as is, I think. > > > +void pfctl_restore_defaults(int, int); > Why not simply pfctl_reset()? I had used pfctl_reset() at some point of history, then I stopped to like it. Now I like it again. updated diff

Re: Removing PF

2019-04-01 Thread Alexandr Nedvedicky
On Mon, Apr 01, 2019 at 01:04:19PM -, Miod Vallat wrote: > > > Will the bpf JIT changes be done in time for 6.6? I have no doubt > > that "pfctl -p /dev/bfp" can be made to work in time but for a truly > > performant firewall we will need bpf JIT. > > I wrote a vax BPF jit as a simple

Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-08 Thread Alexandr Nedvedicky
Hello, > We should fail hard as in almost all other strdup(3) use cases. > Failure means the system ran out of memory, there's no point in going > any further. > > So just something like > > pf.ifname = strdup("none"); > if (pf.ifname == NULL) > err(1, "%s: strdup",

introduce 'pfctl -FR' to reset settings to defaults

2019-03-26 Thread Alexandr Nedvedicky
Hello, tedu@ has planted idea for diff below here [1]. That particular email is part of thread [2], where various cleanup/unconfigure options for PF are discussed. To keep progressing in small steps I've decided to factor out the first diff here, which introduces '-FR' (a.k.a. reset settings)

Re: pfctl should allow administrator to flush _anchors

2019-02-22 Thread Alexandr Nedvedicky
Hello Klemens, I just need to clarify some details. > > the 'unreferenced' means the anchor is not reachable by any packet. > > like there is no path for packet between main ruleset and that > > particular > > anchor (and all its descendants). > Yes. With the regress suite for

Re: pfctl should allow administrator to flush _anchors

2019-02-22 Thread Alexandr Nedvedicky
Hello, On Fri, Feb 22, 2019 at 10:55:17AM +0100, Klemens Nanni wrote: > On Fri, Feb 22, 2019 at 01:52:24AM +0100, Alexandr Nedvedicky wrote: > > > so far so good. Now let's flush the rules from kernel: > > > > lumpy# ./pfctl -Fr > > rules clea

Re: once rules fix

2019-03-05 Thread Alexandr Nedvedicky
Hello Klemens, On Tue, Mar 05, 2019 at 04:47:33PM +0100, Klemens Nanni wrote: > Thanks! Diff makes sense, see comments inline. I confirm that this > restores intended behaviour and regress is fine as well. > > With those addressed OK kn; or I take care of it after getting an OK. > sashan?

pfctl should allow administrator to flush _anchors

2019-02-21 Thread Alexandr Nedvedicky
Hello, This issue has been reported by one of our customers. consider pf.conf comes with rules as follows: anchor { pass all anchor { block all } } We load pf.conf to kernel and (pfctl -f pf.conf) and display what got loaded:

Re: pfctl should allow administrator to flush _anchors

2019-03-21 Thread Alexandr Nedvedicky
Hello, I'm giving up on identifying unreferenced/orphaned rulesets. It seems to me like too much work/complexity, which invites more troubles. I prefer to keep things simple, hence I'm proposing new modifier for Flush option: -F Nuke Flush all rulesets and tables. The option is meant

Re: pfctl: anchor names must not be empty, unify sanity checks

2019-02-08 Thread Alexandr Nedvedicky
Hello, On Wed, Feb 06, 2019 at 10:20:35PM +0100, Klemens Nanni wrote: > When using anchors, they ought to have a non-empty name or none at all. > > By accident, I discovered the following: > > $ printf 'anchor ""\n' | pfctl -vnf- > pass all no state > > No errors and it parses in

Re: introduce 'pfctl -FR' to reset settings to defaults

2019-04-09 Thread Alexandr Nedvedicky
Hello Klemens, On Tue, Apr 09, 2019 at 04:02:06PM +0200, Klemens Nanni wrote: > OK either way, but see below. > > On Mon, Apr 08, 2019 at 09:56:46AM +0200, Alexandr Nedvedicky wrote: > > + pf.ifname = strdup("none"); > > + if (pf.ifname == NULL) > > +

update to PF pfctl(8) and pf.conf(5) manpages

2019-04-16 Thread Alexandr Nedvedicky
Hello, my oracle fellow pointed out [1] a PF documentation can be improved a bit, when it comes to newly introduced 'pfctl -FR' (a reset flush modifier). I've decided to make manpage changes in separate diff as I expect some discussion on how much detailed the manpage should be. The diff here

enable pfctl to flush all rules and tables

2019-04-16 Thread Alexandr Nedvedicky
Hello, this is a fairly large change to pfctl, which allows PF administrator to purge all anchors from PF driver. Consider ruleset as follows: lumpy# pfctl -a '*' -sr pass all flags S/SA anchor "foo" all { anchor "inbound" from any to { match in on any inet from

Re: update to PF pfctl(8) and pf.conf(5) manpages

2019-04-17 Thread Alexandr Nedvedicky
Hello Ingo, thank you for all your suggestions. I've accepted all of them. updated diff is below. let me just share some thoughts and clarifications here. > > I don't feel strongly about mentioning the defaults either way. > But i tend to think that if something is important enough to provide

Re: enable pfctl to flush all rules and tables

2019-05-15 Thread Alexandr Nedvedicky
Hello Klemens, On Mon, May 13, 2019 at 12:22:34AM +0200, Klemens Nanni wrote: > On Wed, Apr 17, 2019 at 01:28:57AM +0200, Alexandr Nedvedicky wrote: > > The idea has been already discussed few weeks ago [1]. Reusing "-a '*'" > > option > > to tell pfctl to flus

Re: enable pfctl to flush all rules and tables

2019-05-17 Thread Alexandr Nedvedicky
Hello Klemens, I think what you see is actually somewhat desired/expected behavior of current PF. stay tuned for explanation further below. > > Clean up: > > # ./obj/pfctl -Fa -aregress/* > 0 tables deleted. > rules cleared > 0 tables deleted. > rules cleared >

Re: if_netisr(): trade NET_LOCK() for NET_RLOCK()

2019-06-04 Thread Alexandr Nedvedicky
Hello, I see, I owe some clarification to share wider context of this change. On Tue, Jun 04, 2019 at 10:32:57AM -0300, Martin Pieuchot wrote: > On 04/06/19(Tue) 01:37, Alexandr Nedvedicky wrote: > > Hello, > > > > diff below is just cosmetic change, which has

pfsync_sendout() requires PF_LOCK()

2019-06-03 Thread Alexandr Nedvedicky
Hello, this is yet another fall out from my experiments. It has turned out my initial change introduced some time ago [1] omits pfsyncintr() and pfsync_sendout(). Both functions call pfsync_sendout(), which currently requires protection of PF_LOCK(). Without change below the test box was dying

if_netisr(): trade NET_LOCK() for NET_RLOCK()

2019-06-03 Thread Alexandr Nedvedicky
Hello, diff below is just cosmetic change, which has no impact on current functionality, because there is just single network task to deliver packets to IP stack. I just want to push this small change to tree to minimize delta between current and my experimental branch. OK? thanks and regards

pf_state_key_link_reverse() needs atomic ops

2019-06-03 Thread Alexandr Nedvedicky
Hello, I've managed to get pf_test() running in parallel on forwarding path in my experimental tree. And there was some fall out. PF died on ASSERT() in pf_state_key_link_reverse() at line 7371: 7368 pf_state_key_link_reverse(struct pf_state_key *sk, ...) 7369 { 7370 /* Note

Re: if_netisr(): trade NET_LOCK() for NET_RLOCK()

2019-06-05 Thread Alexandr Nedvedicky
Hello, On Wed, Jun 05, 2019 at 10:50:18AM -0300, Martin Pieuchot wrote: > On 04/06/19(Tue) 23:57, Alexandr Nedvedicky wrote: > > Hello, > > > > I see, I owe some clarification to share wider context of this change. > > > > On Tue, Jun 04, 2019 at 10:32:

Re: pf_state_key_link_reverse() needs atomic ops

2019-06-11 Thread Alexandr Nedvedicky
Hello, > > > > > > yes that's correct. the patch above comes from my private branch [1]. > > > mpi@ pointed out in off-line email exchange the patch unlocks local > > > inbound > > > packets too, which is coming bit early. However for forwarding case > > > things > > > seem

Re: bpf(4) free sizes

2019-06-11 Thread Alexandr Nedvedicky
Hello, On Mon, Jun 10, 2019 at 01:53:06PM -0300, Martin Pieuchot wrote: > `bd_bufsize' can change via the BIOCSBLEN ioctl(2) but iff the > descriptor hasn't been linked to an interface. Which means the > buffers haven't been allocated yet. > > ok? reads OK to me. regards sashan > >

Re: pf_state_key_link_reverse() needs atomic ops

2019-06-10 Thread Alexandr Nedvedicky
Hello, sorry for extra delay (was off-line over the weekend). On Sat, Jun 08, 2019 at 09:46:24PM +1000, Jonathan Matthew wrote: > On Tue, Jun 04, 2019 at 01:50:51AM +0200, Alexandr Nedvedicky wrote: > > Hello, > > > > I've managed to get pf_test() running in parallel on

Re: enable pfctl to flush all rules and tables

2019-05-10 Thread Alexandr Nedvedicky
Hello, Petr Hoffmann pointed out three nits off-list. we are better to use errx() instead of fprintf() + exit here: +pfctl_get_anchors(int dev, int opts) +{ + + if (pfra.pfra == NULL) + errx(1, + "%s failed to allocate main anchor, can't continue\n", +

Re: pf(4) man page: sync with net/pfvar.h

2019-05-23 Thread Alexandr Nedvedicky
On Wed, May 22, 2019 at 09:41:00PM -0400, Lawrence Teo wrote: > This syncs the pf(4) man page with the latest net/pfvar.h (r1.490). > > ok? no objections, thanks. OK sashan > > > Index: pf.4 > === > RCS file:

Re: update to PF pfctl(8) and pf.conf(5) manpages

2019-05-08 Thread Alexandr Nedvedicky
Hello, below is third iteration of pf.conf.5 manpage. The new addresses issues as follows: all excessive "'pfctl -F Reset' ..." are gone. I've added simple sentence/paragraph at the end of OPTIONS section: +.Pp +.Dq pfctl -F Reset +restores default values for

[2/3] enable pfctl to flush all rules and tables

2019-05-08 Thread Alexandr Nedvedicky
Hello, second diff makes current implementation of 'pfctl -vsA' (show rules) more generic. It changes current pfctl_show_anchors() to pfctl_walk_anchors() which accepts a callback as argument (pfctl_show_anchor()) to show anchor found in kernel. So existing pfctl_show_anchors() is changed to

[1/3] Re: enable pfctl to flush all rules and tables

2019-05-08 Thread Alexandr Nedvedicky
Hello, looks like the diff is too big for review. so let me slice it to smaller chunks. I've tested the complete diff I have not tested the partial diffs. no issues were discovered by running regress on pfctl. furthermore doing 'pfctl -a "*" -Fa' with my changes in did expected thing: it

Re: enable pfctl to flush all rules and tables

2019-05-08 Thread Alexandr Nedvedicky
Hello, diff 3/3 makes patch complete. It uses pfctl_recurse() introduced in 2/3 to implement operations as follows: pfctl -a "*" -Fa(applies pfctl_call_clearanchors()) pfctl -a "*" -Fr(applies pfctl_call_clearrules()) pfctl -a "*" -FT(applies pfctl_call_cleartables()) All

Re: update to PF pfctl(8) and pf.conf(5) manpages

2019-04-29 Thread Alexandr Nedvedicky
Hello, hit sent too fast... attaching updated diff this time. thanks and regards sashan 8<---8<---8<--8< diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8 index b7e941991ba..5e2c57f6bc2 100644 --- a/sbin/pfctl/pfctl.8 +++

Re: update to PF pfctl(8) and pf.conf(5) manpages

2019-04-29 Thread Alexandr Nedvedicky
Hello, sorry for extra delay. > i have to say upfront that i dislike this idea of dividing options into > classes and then for every option, altering the text to something > unwieldy like: > > This runtime option... > > it reads very poorly, and this page is big enough as is without

Re: update to PF pfctl(8) and pf.conf(5) manpages

2019-04-29 Thread Alexandr Nedvedicky
Hello, comments inline. > > +section 'OPTIONS', for details. > > probably just > > See > .Xr pf.conf.5 > .Sx OPTIONS > for details. fixed > > Set the debug > > .Ar level , > > @@ -1165,6 +1168,11 @@ and > > These keywords correspond to the similar (LOG_)

Re: pf: use proper interface for route-to when it is used with sticky-address

2019-07-10 Thread Alexandr Nedvedicky
Hello Stuart, On Wed, Jul 10, 2019 at 08:19:13PM +0100, Stuart Henderson wrote: > On 2019/07/05 17:09, YASUOKA Masahiko wrote: > > Hi, > > > > Previous diff made src-node have a reference for the kif. My > > colleague pointed out that incrementing the reference count of the kif > > is required.

Re: pf: keep src track for route-to while its state exists

2019-07-01 Thread Alexandr Nedvedicky
Hello, On Mon, Jul 01, 2019 at 02:45:12PM +0900, YASUOKA Masahiko wrote: > Hi, > > The source address tracking (sticky-address) is kept dulring there are > states which refer it. This is mentioned in pf.conf(5). This is true > for translation(nat-to, rdr-to) but it was not true for >

Re: pf: use proper interface for route-to when it is used with sticky-address

2019-07-08 Thread Alexandr Nedvedicky
Hello Yasuoka, > Previous diff made src-node have a reference for the kif. My > colleague pointed out that incrementing the reference count of the kif > is required. > > ok? > very good catch, indeed. Thanks for taking care of it. diff looks good to me. OK sashan

Re: pf: use proper interface for route-to when it is used with sticky-address

2019-07-01 Thread Alexandr Nedvedicky
Hello, On Mon, Jul 01, 2019 at 09:11:28PM +0900, YASUOKA Masahiko wrote: > Hi, > > "route-to" is used with the interface used for the next hop. But if > it is used with a source address track (sticky-address), it sometimes > output the packet to wrong interface. > > Example: > > pass in

Re: let bpf_mtap_hdr take a void * instead of caddr_t for the header

2019-09-11 Thread Alexandr Nedvedicky
On Wed, Sep 11, 2019 at 10:02:24AM +1000, David Gwynne wrote: > this makes it easier to call at least. > > it also brings it in line with bpf_tap_hdr. otherwise there's no > functional change. > > ok? OK sashan@, consistency never hurts. sashan

follow up to 'once rule' expiration

2019-07-18 Thread Alexandr Nedvedicky
Hello, I've just realized my suggestion [1] to lteo@ was not complete. The single atomic_cas() used as I've suggested is not sufficient measure. The code should also do a non-atomic test to check, whether the rule is not expired yet. the non-atomic check deals with scenario, where competing

Time-out like behaviour for ICMP port unreachable errors

2019-11-01 Thread Alexandr Nedvedicky
Hello, I'm sending same patch I've sent to bugs@ to thread [1], which got started by Paul de Weerd few days ago. I've received few questions off-list, which made me to shovel bit more through TCP code in OpenBSD. I hope I can better explain impact of change introduced by patch below. I think it's

Re: Fix rw_assert_unlocked(9)

2019-11-11 Thread Alexandr Nedvedicky
Hello, On Sun, Nov 10, 2019 at 04:11:55PM +0100, Martin Pieuchot wrote: > rw_assert_unlocked(9) should check if the current thread is holding > the lock, not if the lock is held by anyone else. The general check > is racy and I cannot think of any safe way to use it. looks ok. just one

Re: Fix rw_assert_unlocked(9)

2019-11-11 Thread Alexandr Nedvedicky
Hello, On Mon, Nov 11, 2019 at 04:48:43PM +0100, Martin Pieuchot wrote: > On 11/11/19(Mon) 12:07, Alexandr Nedvedicky wrote: > > Hello, > > > > > > On Sun, Nov 10, 2019 at 04:11:55PM +0100, Martin Pieuchot wrote: > > > rw_assert_unlocked(9) should ch

Re: TCP send window underflow

2019-11-11 Thread Alexandr Nedvedicky
Hello, the change looks OK to me. thanks and regards sashan

Re: use tasks and a task_list to manage if_detachhooks

2019-11-05 Thread Alexandr Nedvedicky
Hello David, > if people are ok with this, i'll go through and do the same for the link > state and address change hooks. I like your approach, and I think same change should be done linkstate and addrchange hooks as a follow up commit. I'm OK with your diff. thanks and regards sashan

Re: pfctl: Do not optimize empty rulesets

2019-12-13 Thread Alexandr Nedvedicky
Hello, sorry it dropped on the floor of my INBOX... thanks for reminding. > Yes, the main anchor prints as "" but all that is behind compile time > -DOPT_DEBUG so regular users won't deal with it anyway, so keep the code > simple instead of adding logging around `rs->anchor->path'. > > OK?

Re: attention please: host's IP stack behavior got changed slightly

2019-12-15 Thread Alexandr Nedvedicky
Hello Daniel, thanks for reporting back. > Should the rdr-to rule still work? I fixed it with using the "Port foo" > directive in my sshd config (and a simple "pass in to port foo") in the > meantime. My earlier indeed change omits your usecase. The rdr rule should still work. Patch

attention please: host's IP stack behavior got changed slightly

2019-12-08 Thread Alexandr Nedvedicky
Hello, commit from today [1] makes IP stack more paranoid. Up to now OpenBSD implemented so called 'weak host model' [2]. The today's commit alters that for hosts, which don't forward packets (don't act as routers). Your laptops, desktops and servers now check packet destination address with IP

Re: attention please: host's IP stack behavior got changed slightly

2019-12-16 Thread Alexandr Nedvedicky
Hello, On Mon, Dec 16, 2019 at 03:21:43PM +0100, Claudio Jeker wrote: > On Mon, Dec 16, 2019 at 02:13:50PM +0100, Alexander Bluhm wrote: > > On Sun, Dec 15, 2019 at 03:17:26PM +0100, Alexandr Nedvedicky wrote: > > > Hello Daniel, > > > > > > thanks for reporti

Re: pf.conf.5: Fix adaptive.{start,end} values

2019-10-23 Thread Alexandr Nedvedicky
Hello, I like to swap the order, so adaptive.start goes first. Though I'm not sure the default value for adaptive.start is 5, I think it's 6. Or do I miss something? > -.It Cm adaptive.start Pq 12 states by default > +.It Cm adaptive.start Pq 5 states by default

Re: pf.conf.5: Fix adaptive.{start,end} values

2019-10-23 Thread Alexandr Nedvedicky
On Thu, Oct 24, 2019 at 12:19:05AM +0200, Klemens Nanni wrote: > On Thu, Oct 24, 2019 at 12:10:09AM +0200, Klemens Nanni wrote: > > Someone on IRC (I forgot the name, sorry) mentioned that the > > `adaptive.end' and `adaptive.end' timeout variables had their documented > > default values swapped.

Re: fix kernel crash in pf_ioctl with WITH_PF_LOCK and NET_TASKQ > 1

2019-11-26 Thread Alexandr Nedvedicky
Hello Joerg, your patch looks OK. thank you for testing & fixing. thanks and regard sashan On Tue, Nov 26, 2019 at 10:42:42AM +0100, Joerg Goltermann wrote: > Hello, > > I tested a kernel with WITH_PF_LOCK and NET_TASKQ > 1 and got a > kernel panic during switching between the pf statics

Re: Consistent use of RW_PROC().

2019-11-20 Thread Alexandr Nedvedicky
Hello, On Wed, Nov 20, 2019 at 12:21:04PM +0100, Martin Pieuchot wrote: > As pointed out by sashan@ in a previous diff, unify all checks to have > the same idiom. > > Ok? looks OK to me. thanks and regards sashan

Re: attention please: host's IP stack behavior got changed slightly

2019-12-22 Thread Alexandr Nedvedicky
Hello, On Fri, Dec 20, 2019 at 11:12:15PM +0100, Alexander Bluhm wrote: > On Wed, Dec 18, 2019 at 09:07:35AM +0100, Alexandr Nedvedicky wrote: > > I see. Updated diff below makes ip6_input_if() to explicitly check > > for PF_TAG_TRANSLATE_LOCALHOST tag, when ip6_forward

<    1   2   3   4   5   6   7   >