- User Since
- Sep 28 2014, 7:22 PM (246 w, 1 d)
Sun, Jun 16
Given that it's possible to use blacklistd with ipfw as well (at least, I believe it is), it should probably get its own chapter, with pf and ipfw subchapters.
Sat, Jun 15
Wed, Jun 12
Okay, thanks. That should indeed just work. The 'pf_check_proto_cksum()' flow, assuming there's no hardware assist, might break. I suspect that hardware which uses unmapped mbufs is always going to have checksum offload, so that's probably not an issue either.
What happens if a firewall is enabled and an unmapped mbuf is passed through pfil(9)?
I suspect that, if a pfil hook is hit, we'd also have to copy it in, just like when a checksum needs to be updated.
May 15 2019
There are sadly still a lot of panics triggered by these tests.
May 6 2019
That seems like a good idea. I'll try to bring it up during the BSDCan devsummit.
Apr 18 2019
Burn it with fire! This code is not used, and pretty much can't be used, so let's get rid of it.
Rather annoyingly things like ezjail can't directly use the devfsrules_vnet_jail name. They must specify the number. See https://svnweb.freebsd.org/base/head/libexec/rc/rc.d/jail?view=markup#l238
I have no idea how to address that, but this at the very least will hint users in the direction of what they may want for vnet jails.
cc bapt who knows more about this code than I do.
LGTM. It may be useful to point at 122773 in the commit message for extra context.
Apr 17 2019
It's not clear to me how this leaks memory. Can you go into a bit more detail about how you found the memory leak, and the codepath it follows?
Apr 9 2019
Mar 30 2019
Mar 23 2019
Mar 19 2019
Mar 15 2019
I don't see any obvious problems, but I don't think I know this part of the code well enough to say for sure.
panic: Assertion in_epoch(net_epoch_preempt) failed at /usr/src/sys/netinet6/nd6_rtr.c:874 cpuid = 3 time = 1552657485 KDB: stack backtrace: db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe0091c86300 vpanic() at vpanic+0x19d/frame 0xfffffe0091c86350 panic() at panic+0x43/frame 0xfffffe0091c863b0 defrouter_select_fib() at defrouter_select_fib+0x52a/frame 0xfffffe0091c86460 defrouter_del() at defrouter_del+0x15f/frame 0xfffffe0091c864a0 nd6_purge() at nd6_purge+0x1af/frame 0xfffffe0091c864f0 if_detach_internal() at if_detach_internal+0x7f6/frame 0xfffffe0091c86570 if_detach() at if_detach+0x3d/frame 0xfffffe0091c86590 epair_clone_destroy() at epair_clone_destroy+0x98/frame 0xfffffe0091c865e0 if_clone_destroyif() at if_clone_destroyif+0x175/frame 0xfffffe0091c86630 if_clone_destroy() at if_clone_destroy+0x205/frame 0xfffffe0091c86680 ifioctl() at ifioctl+0x3de/frame 0xfffffe0091c86750 kern_ioctl() at kern_ioctl+0x28a/frame 0xfffffe0091c867c0 sys_ioctl() at sys_ioctl+0x15d/frame 0xfffffe0091c86890 amd64_syscall() at amd64_syscall+0x276/frame 0xfffffe0091c869b0 fast_syscall_common() at fast_syscall_common+0x101/frame 0xfffffe0091c869b0 --- syscall (54, FreeBSD ELF64, sys_ioctl), rip = 0x8004852ca, rsp = 0x7fffffffe228, rbp = 0x7fffffffe240 --- KDB: enter: panic [ thread pid 93197 tid 100466 ] Stopped at kdb_enter+0x3b: movq $0,kdb_why db>
During the sys/netinet/fibs_test:slaac_on_nondefault_fib6 test, I believe.
Mar 14 2019
Do we need to virtualise the bridge_rtnode_zone? Doesn't the cleanup call to bridge_rtflush() take care of all of the allocations already?
I'll try to take a more in-depth look later today.
Mar 13 2019
Mar 12 2019
Mar 8 2019
Ah, I hadn't looked at that review yet.
https://bugs.freebsd.org/12345 redirects to https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=12345. Wouldn't the first form be better? It's shorter and likely easier to map to whatever bug tracker we might move to after bugzilla (if we ever do change). It also looks more like the phabricator links.
Mar 5 2019
Mar 4 2019
If there are no objections I'm going to commit this soon.
Feb 27 2019
Fix issues pointed out by hrs.
Fix tun_clone_match() to not match on e.g. tunnel.
Remove stray debug lines
Ensure that unit numbers are system-unique.
Feb 24 2019
The pf bit is good.
Everything else looks good to me as well, modulo the remark karels had about the XXX comment.
Feb 21 2019
New panic, I suspect while an interface is being moved into or out of a vnet jail.
The pf tests seem to trigger a lot of edge cases, so you may want to see if you can get them to run too. They don't need much setup (basically, pkg install scapy kyua, then kldload pfsync ; cd /usr/tests/sys/netpfil/pf ; sudo kyua test).
Feb 20 2019
Thanks. The original reporter of the bug discovered that too.
Feb 19 2019
Feb 11 2019
This one boots, but panics when I kldload pfsync:
Feb 10 2019
Feb 9 2019
I have no objections to the patch, but I don't know enough about HFSC to meaningfully review this, I'm afraid.
Feb 8 2019
I'm pretty sure that ipv6_activate_all_interfaces="YES" in /etc/rc.conf is the trigger for the previous panic.
That also matches nicely with the backtrace. Presumably nd6_dad_timer() doesn't enter the NET_EPOCH.
This panics my test vm:
Feb 2 2019
Jan 29 2019
Did you happen to do any benchmarking on this? I'd have expected "Sync pfil hooks epoch(9)" to give us a (small) performance improvement, but my initial test shows a small reduction in forwarding performance (with pf loaded).
Jan 28 2019
Jan 25 2019
This panics during the netipsec and pf regression tests:
Jan 24 2019
To be clear: I don't object to your change. It's just that ifconfig is a bit of a mess and any opportunity for cleanup is tempting. Especially if I don't have to do it.
Jan 21 2019
Jan 18 2019
Yeah, looking at my original commit I did manage to lose that flag. I’ve not tested yet, but this patch is almost certainly correct.
Jan 15 2019
Also, I’d really, really like some basic tests for altq. It should be reasonably straightforward to do something based on the existing pf tests. At least, if altq can work on top of epairs.
Jan 12 2019
Jan 8 2019
Jan 7 2019
I’ll try to review asap, but I’m on holiday wirh limited internet access right now.
Dec 29 2018
Dec 8 2018
Dec 5 2018
Dec 3 2018
Fix build with ! VIMAGE
Address man page remark.
Dec 2 2018
Minor improvements, style and such.
Small man page addition.
Dec 1 2018
Nov 29 2018
Single pfsyncintr swi, rather than one per bucket.
Well, the remarks you've had have been pretty helpful so far.
Nov 28 2018
And here's my flame graph: https://people.freebsd.org/~kp/pfsync/buckets.svg
This is a slightly rough first version. I'm sure there are some remaining style issues (and there might be cleanup issues too).
I have an alternative version in https://reviews.freebsd.org/D18373.
It splits pfsync up into buckets, with their own lock. Performance is slightly better than this, and it's in many ways a simpler change.
Nov 26 2018
Nov 22 2018
Nov 21 2018
Nov 17 2018
Here are flame graphs for my test setup:
- plain forwarding
- pf only
- unpatched pfsync
- pfsync with this (and the next) patch
Nov 16 2018
I've been thinking about what we could do to split up the lock, rather than take this approach, and I'm not sure how.
Nov 15 2018
For what it's worth, OpenBSD don't drop here but henning did add a /* XXX why clear instead of drop? */ comment a few years ago.
My performance tests show an improvement of 80-90% with this change. The swi_sched is costly, yes, but it's a pretty significant improvement in throughput.
Nov 14 2018
Nov 1 2018
I'm not currently aware of any remaining issues here. It's certainly possible that some still exist, but I've seen no reports and the tests don't provoke them.
I think of this change mostly as a 'I will support this now.' statement.