- User Since
- Sep 28 2014, 7:22 PM (268 w, 3 d)
Tue, Nov 19
Wed, Nov 13
Tue, Nov 12
Fri, Oct 25
Oct 17 2019
Oct 15 2019
Oct 13 2019
Oct 12 2019
Minor nit: Mention where this is mentioned (i.e. the comments to the function).
Other than that this looks like a good idea to me.
Oct 5 2019
Oct 4 2019
This happens on a riscv machine, running from an mdroot (although I'm not sure if that's relevant to trigger it), not running devd during the aio_test:md_waitcomplete regression test.
That test opens /dev/mdX and tries to perform an aio write on it, which ends up failing. I'm not sure I fully understand the intent behind the test, but it revealed that the aio code considered that to be an unsafe. The safety check code thinks that mp->mnt_stat.f_iosize is relevant, and because of the lack of VFS_STATFS call that was still set to 0 for devfs.
Sep 25 2019
Sep 11 2019
Sep 8 2019
Other than the typo this looks good to me.
Sep 5 2019
Sep 4 2019
Aug 16 2019
This probably also wants this:
Aug 15 2019
Aug 14 2019
Aug 12 2019
And this is wrong, or at least very confusing, in firewall_init():
Aug 11 2019
I seem to run into issues running the ipfw_basic test:
Jul 31 2019
Jul 29 2019
I think I'm happy with this.
I'll give Tom a bit of time to add any more remarks he might have, but I think we can commit this soon.
Jul 28 2019
Remove more bits, as suggested by thj
Jul 27 2019
Jul 26 2019
I'd want Tom to have a look too, but I think we're pretty close to something ready to commit.
Also, I get 'install: /usr/tests/sys/netpfil/common/pass_block: No such file or directory' trying to install world.
This patch is missing this:
Jul 25 2019
Jul 8 2019
I think I now understand better how the problem happens. It's specific to epair.
It's indeed a race between if_vmove() moving the interface back into its original vnet, and the epair interface getting destroyed. Destruction of epairs is special, because we remove two interfaces at once.
See epair_clone_destroy(). There we if_detach() (through ether_ifdetach()) the other interface as well. If that interface has been moved out of the vnet by then the if_detach() will fail, but this does not return an error (if_detach_internal() does). So we end up freeing the ifp, while if_vmove() is reinserting it in another vnet, leading to the described panic.
Jul 7 2019
Good point. While this does make things a lot better (as in, nearly perfectly reliable panic running the pf tests, to being able to loop the tests all night, when combined with D20869), I think you're right that the race is still there.
Jul 6 2019
Jun 16 2019
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.
Jun 15 2019
Jun 12 2019
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: