Page MenuHomeFreeBSD

kristof (Kristof Provost)
User

Projects

User Details

User Since
Sep 28 2014, 7:22 PM (238 w, 3 d)

Recent Activity

Thu, Apr 18

kristof added a comment to D19960: Remove support for RFC2675.

Burn it with fire! This code is not used, and pretty much can't be used, so let's get rid of it.

Thu, Apr 18, 5:19 PM
kristof added a comment to D19957: devfs: Add 'devfsrules_vnet_jail'.

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.

Thu, Apr 18, 3:01 PM
kristof created D19957: devfs: Add 'devfsrules_vnet_jail'.
Thu, Apr 18, 3:00 PM
kristof added a reviewer for D19838: Fix a memory leak in pw when invoked with -V or -R: bapt.

cc bapt who knows more about this code than I do.

Thu, Apr 18, 11:02 AM
kristof accepted D19952: Add a bugs section to pflog man page.

LGTM. It may be useful to point at 122773 in the commit message for extra context.

Thu, Apr 18, 10:42 AM
kristof added inline comments to D19939: Some cleanups for the PF chapter in the handbook.
Thu, Apr 18, 10:29 AM

Wed, Apr 17

kristof added a comment to D19838: Fix a memory leak in pw when invoked with -V or -R.

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?

Wed, Apr 17, 11:19 AM

Tue, Apr 9

kristof accepted D19861: Update and clarify pflog man page.
Tue, Apr 9, 12:32 PM

Sat, Mar 30

kristof accepted D19757: if_bridge(4): Complete bpf auditing of local traffic over the bridge.
Sat, Mar 30, 4:53 PM

Mar 23 2019

kristof accepted D19614: if_bridge(4): ensure all traffic passing over the bridge is accounted for.
Mar 23 2019, 12:27 PM

Mar 19 2019

kristof accepted D19317: Use IN_foo() macros from sys/netinet/in.h inplace of handcrafted code.
Mar 19 2019, 7:34 AM

Mar 15 2019

kristof added a comment to D19586: if_bridge: Give bpf a shot at packets passed over the bridge.

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.

Mar 15 2019, 2:33 PM
kristof added a reviewer for D19586: if_bridge: Give bpf a shot at packets passed over the bridge: philip.
Mar 15 2019, 2:16 PM
kristof added a comment to D19111: Summary: widen net_epoch coverage up to all packet processing.
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 15 2019, 1:56 PM

Mar 14 2019

kristof accepted D19578: if_bridge(4): Fix module teardown .

Oh yes, of course.

Mar 14 2019, 6:00 PM
kristof added a comment to D19578: if_bridge(4): Fix module teardown .

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?

Mar 14 2019, 5:49 PM
kristof added a comment to D19578: if_bridge(4): Fix module teardown .

I'll try to take a more in-depth look later today.

Mar 14 2019, 6:34 AM

Mar 13 2019

kristof accepted D19573: ether: centralize fake hwaddr generation.

LGTM.

Mar 13 2019, 7:49 PM
kristof updated the diff for D19558: pf :Use counter(9) in pf tables..

Add comment

Mar 13 2019, 4:17 PM

Mar 12 2019

kristof updated the summary of D19558: pf :Use counter(9) in pf tables..
Mar 12 2019, 12:47 PM
kristof added a reviewer for D19558: pf :Use counter(9) in pf tables.: network.
Mar 12 2019, 12:46 PM
kristof created D19558: pf :Use counter(9) in pf tables..
Mar 12 2019, 12:45 PM
kristof accepted D19530: Followup to PR231977: Mention that /etc/pf.conf must be created first.
Mar 12 2019, 10:07 AM

Mar 8 2019

kristof added a comment to D19459: subversion: update commit message template to allow URLs in PR field.

@bdrewery and I both independently had the same idea. He mentioned it in D19426.

Ah, I hadn't looked at that review yet.

Mar 8 2019, 5:20 PM
kristof added a comment to D19459: subversion: update commit message template to allow URLs in PR field.

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 8 2019, 9:37 AM

Mar 5 2019

kristof updated the summary of D19248: tun: VIMAGE fix for if_tun cloner.
Mar 5 2019, 9:24 AM
kristof updated the diff for D19248: tun: VIMAGE fix for if_tun cloner.

'\0'

Mar 5 2019, 9:24 AM

Mar 4 2019

kristof added a comment to D19248: tun: VIMAGE fix for if_tun cloner.

If there are no objections I'm going to commit this soon.

Mar 4 2019, 6:34 PM

Feb 27 2019

kristof updated the diff for D19248: tun: VIMAGE fix for if_tun cloner.

Fix issues pointed out by hrs.

Feb 27 2019, 2:57 PM
kristof updated the diff for D19248: tun: VIMAGE fix for if_tun cloner.

Fix tun_clone_match() to not match on e.g. tunnel.

Feb 27 2019, 2:28 PM
kristof added inline comments to D19248: tun: VIMAGE fix for if_tun cloner.
Feb 27 2019, 1:50 PM
kristof added inline comments to D19248: tun: VIMAGE fix for if_tun cloner.
Feb 27 2019, 1:34 PM
kristof updated the diff for D19248: tun: VIMAGE fix for if_tun cloner.

Remove stray debug lines

Feb 27 2019, 9:52 AM
kristof updated the diff for D19248: tun: VIMAGE fix for if_tun cloner.

Ensure that unit numbers are system-unique.

Feb 27 2019, 9:37 AM

Feb 24 2019

kristof added a comment to D19317: Use IN_foo() macros from sys/netinet/in.h inplace of handcrafted code.

The pf bit is good.
Everything else looks good to me as well, modulo the remark karels had about the XXX comment.

Feb 24 2019, 8:25 PM

Feb 21 2019

kristof added a comment to D19111: Summary: widen net_epoch coverage up to all packet processing.

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 21 2019, 12:50 PM

Feb 20 2019

kristof added a comment to D19248: tun: VIMAGE fix for if_tun cloner.

Thanks. The original reporter of the bug discovered that too.

Feb 20 2019, 12:54 PM

Feb 19 2019

kristof added reviewers for D19248: tun: VIMAGE fix for if_tun cloner: network, bz.
Feb 19 2019, 6:28 PM
kristof created D19248: tun: VIMAGE fix for if_tun cloner.
Feb 19 2019, 6:27 PM

Feb 11 2019

kristof added a comment to D19111: Summary: widen net_epoch coverage up to all packet processing.

This one boots, but panics when I kldload pfsync:

Feb 11 2019, 9:44 PM

Feb 10 2019

kristof accepted D19131: Decrease the time the kernel takes to install a new PF config with a large number of queues.
Feb 10 2019, 1:46 PM

Feb 9 2019

kristof added a comment to D19124: Fix HFSC configuration bug introduced in r343287.

I have no objections to the patch, but I don't know enough about HFSC to meaningfully review this, I'm afraid.

Feb 9 2019, 1:58 PM

Feb 8 2019

kristof added a comment to D19111: Summary: widen net_epoch coverage up to all packet processing.

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.

Feb 8 2019, 3:15 PM
kristof added a comment to D19111: Summary: widen net_epoch coverage up to all packet processing.

This panics my test vm:

Feb 8 2019, 10:43 AM

Feb 2 2019

kristof accepted D18924: bridge: Fix spurious warnings about capabilities.

LGTM

Feb 2 2019, 10:10 AM

Jan 29 2019

kristof added a comment to D18951: New pfil(9).

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 29 2019, 8:34 PM

Jan 28 2019

kristof accepted D19005: ifconfig: fix endianness bug displaying pfsync interfaces.
Jan 28 2019, 5:44 PM
kristof added inline comments to D19005: ifconfig: fix endianness bug displaying pfsync interfaces.
Jan 28 2019, 5:10 PM

Jan 25 2019

kristof added a comment to D18951: New pfil(9).

This panics during the netipsec and pf regression tests:

Jan 25 2019, 1:57 AM

Jan 24 2019

kristof accepted D18919: In ifconfig(8), don't build, sort and search all system addresses when performing a non-status action on a single interface.

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 24 2019, 9:04 AM
kristof added inline comments to D18919: In ifconfig(8), don't build, sort and search all system addresses when performing a non-status action on a single interface.
Jan 24 2019, 3:45 AM
kristof added inline comments to D18919: In ifconfig(8), don't build, sort and search all system addresses when performing a non-status action on a single interface.
Jan 24 2019, 3:31 AM
kristof added inline comments to D18919: In ifconfig(8), don't build, sort and search all system addresses when performing a non-status action on a single interface.
Jan 24 2019, 3:13 AM
kristof accepted D18918: Don't re-evaluate ALTQ kernel configuration due to events on non-ALTQ interfaces.
Jan 24 2019, 2:58 AM

Jan 21 2019

kristof added a reviewer for D18909: pfctl: Point users to net.pf.request_maxcount if large requests are rejected: network.
Jan 21 2019, 3:50 AM
kristof created D18909: pfctl: Point users to net.pf.request_maxcount if large requests are rejected.
Jan 21 2019, 3:50 AM

Jan 18 2019

kristof added a comment to D18882: Fix pfsync breaking carp.

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 18 2019, 12:41 AM

Jan 15 2019

kristof added a comment to D18759: Improve pf.conf parsing speed for large numbers of queues.

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 15 2019, 12:12 AM

Jan 12 2019

kristof accepted D18759: Improve pf.conf parsing speed for large numbers of queues.
Jan 12 2019, 5:55 AM

Jan 8 2019

kristof accepted D18779: Don't set if_linkmib for vlan(4)..

LGTM.

Jan 8 2019, 8:14 AM

Jan 7 2019

kristof added a comment to D18759: Improve pf.conf parsing speed for large numbers of queues.

I’ll try to review asap, but I’m on holiday wirh limited internet access right now.

Jan 7 2019, 9:05 PM

Dec 29 2018

kristof created D18679: libxo: Fix XML output if a container name is a number.
Dec 29 2018, 12:52 PM

Dec 8 2018

kristof created D18483: pf: Fix endless loop on NAT exhaustion with sticky-address.
Dec 8 2018, 3:07 PM

Dec 5 2018

kristof added a comment to D18373: pfsync: Performance improvement.
In D18373#392613, @eri wrote:

Not a blocker but:

  • It would also be nice to have measure if the other side can keep up with the blast of state updates now?
  • Even better, provide the same bucket mechanism on reception so it can be distributed on the various cores
Dec 5 2018, 7:35 PM
kristof added a comment to D18373: pfsync: Performance improvement.
In D18373#392610, @eri wrote:

Can you please measure the latency of syncing states with this change against previous latency?

Dec 5 2018, 7:25 PM

Dec 3 2018

kristof updated the diff for D18373: pfsync: Performance improvement.

Fix build with ! VIMAGE

Dec 3 2018, 11:04 AM
kristof updated the diff for D18373: pfsync: Performance improvement.

Address man page remark.

Dec 3 2018, 8:38 AM

Dec 2 2018

kristof updated the diff for D18373: pfsync: Performance improvement.

Minor improvements, style and such.
Small man page addition.

Dec 2 2018, 5:20 PM
kristof abandoned D17994: pfsync: Insert static trace points.

Abandoned in favour of D18373.

Dec 2 2018, 4:53 PM
kristof abandoned D17992: pfsync: Reduce contention on PFSYNC_LOCK().

Abandoned in favour of D18373.

Dec 2 2018, 4:49 PM
kristof abandoned D17993: pfsync: Call pfsyncintr() directly from pfsync_msg_intr() rather than scheduling a swi.

Abandoned in favour of D18373.

Dec 2 2018, 4:49 PM

Dec 1 2018

kristof added inline comments to D17376: IPv6 Fragmentation Regression Tests from OpenBSD.
Dec 1 2018, 5:24 PM

Nov 29 2018

kristof updated the diff for D18373: pfsync: Performance improvement.

Single pfsyncintr swi, rather than one per bucket.

Nov 29 2018, 9:16 PM
kristof added a comment to D18373: pfsync: Performance improvement.
In D18373#390595, @mjg wrote:

once more i don't have a full picture so can't give a proper review.

Well, the remarks you've had have been pretty helpful so far.

Nov 29 2018, 7:08 PM

Nov 28 2018

kristof added a comment to D18373: pfsync: Performance improvement.

And here's my flame graph: https://people.freebsd.org/~kp/pfsync/buckets.svg

Nov 28 2018, 10:44 PM
kristof added a comment to D18373: pfsync: Performance improvement.

This is a slightly rough first version. I'm sure there are some remaining style issues (and there might be cleanup issues too).

Nov 28 2018, 10:40 PM
kristof added reviewers for D18373: pfsync: Performance improvement: network, mjg.
Nov 28 2018, 10:40 PM
kristof added a comment to D17992: pfsync: Reduce contention on PFSYNC_LOCK().

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 28 2018, 10:40 PM
kristof created D18373: pfsync: Performance improvement.
Nov 28 2018, 10:37 PM

Nov 26 2018

kristof added a comment to D17994: pfsync: Insert static trace points.
In D17994#389260, @eri wrote:

Can you add some text to the manual pages for documenting the feature? Possibly linking to some example?

Nov 26 2018, 10:28 AM

Nov 22 2018

kristof added a comment to D17992: pfsync: Reduce contention on PFSYNC_LOCK().
In D17992#387985, @mjg wrote:

Single-threaded processing is definitely a bottle neck. Perhaps you can have more than one thread pushing stuff and/or some of it can be done by submitting threads instead.

Nov 22 2018, 8:19 PM

Nov 21 2018

kristof added a comment to D17992: pfsync: Reduce contention on PFSYNC_LOCK().
In D17992#386171, @mjg wrote:

So both ring and swi kicking code are significant players. I think a simple and probably good enough solution would just add more rings, perhaps based on the number of hardware threads. Assuming the traffic is hashed to distribute among them, the rings could mostly remain unshared with unrelated threads. Sending out of the traffic would just combine data from all rings.

Nov 21 2018, 7:22 PM

Nov 17 2018

kristof added a comment to D17992: pfsync: Reduce contention on PFSYNC_LOCK().
In D17992#385087, @eri wrote:

What needs to be considered here is the assumption of pfsync is that a state created in pf will be synched at shortest possible cycle to the cluster member.
By defering that assumption is relaxed so figuring out baselines of before and after this change would make this more easy to reason about.

Nov 17 2018, 7:49 PM
kristof added a comment to D17992: pfsync: Reduce contention on PFSYNC_LOCK().

Here are flame graphs for my test setup:

  • plain forwarding
  • pf only
  • unpatched pfsync
  • pfsync with this (and the next) patch
Nov 17 2018, 1:51 PM

Nov 16 2018

kristof added a comment to D17992: pfsync: Reduce contention on PFSYNC_LOCK().

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 16 2018, 6:08 PM

Nov 15 2018

kristof accepted D2266: I can find no reason to allow packets with both SYN and FIN bits set past this point in the code. The packet should be dropped and not massaged as it is here..

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.

Nov 15 2018, 8:54 PM
kristof added a comment to D17992: pfsync: Reduce contention on PFSYNC_LOCK().
In D17992#384518, @mjg wrote:

I think the approach taken here is iffy. Basic problem with this is that even if there is no lock contention anymore, you are still suffering from bouncing cache lines. Also swi_sched probably does not appreciate being called very often.

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 15 2018, 6:53 AM

Nov 14 2018

kristof created D17994: pfsync: Insert static trace points.
Nov 14 2018, 9:26 PM
kristof created D17993: pfsync: Call pfsyncintr() directly from pfsync_msg_intr() rather than scheduling a swi.
Nov 14 2018, 9:25 PM
kristof created D17992: pfsync: Reduce contention on PFSYNC_LOCK().
Nov 14 2018, 9:25 PM

Nov 1 2018

kristof updated the diff for D17734: pf: Limit the fragment entry queue length to 64 per bucket..

Formatting change.

Nov 1 2018, 10:58 AM
kristof added a comment to D17505: pfsync: Allow module to be unloaded.

I'm not absolutely sure that all possible races are fixed. There still could be dangling ifnet pointers. But that's up to your justification. If you are sure everything is covered, feel free to remove.

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.

Nov 1 2018, 10:54 AM

Oct 28 2018

kristof added a reviewer for D17734: pf: Limit the fragment entry queue length to 64 per bucket.: network.
Oct 28 2018, 6:31 AM
kristof updated the summary of D17733: pf: Split the fragment reassembly queue into smaller parts.
Oct 28 2018, 6:31 AM
kristof updated the summary of D17732: pf: Count holes rather than fragments for reassembly.
Oct 28 2018, 6:30 AM
kristof created D17734: pf: Limit the fragment entry queue length to 64 per bucket..
Oct 28 2018, 6:30 AM
kristof created D17733: pf: Split the fragment reassembly queue into smaller parts.
Oct 28 2018, 6:30 AM
kristof created D17732: pf: Count holes rather than fragments for reassembly.
Oct 28 2018, 6:29 AM
kristof abandoned D17634: pf tests: Test ':0' ignoring link-local addresses.

Committed as r339836

Oct 28 2018, 6:00 AM
kristof closed D1309: VIMAGE PF fixes #1.

Assorted pf VIMAGE fixes have been done, and pf is now usable inside VIMAGE jails.

Oct 28 2018, 5:59 AM