Page MenuHomeFreeBSD

pkelsey (Patrick Kelsey)
User

Projects

User Details

User Since
Jun 2 2014, 3:47 PM (237 w, 1 d)

Recent Activity

Nov 14 2018

pkelsey accepted D17758: Fix flags collision causing inability to enable CBQ in ALTQ.

To clarify 'the symptom is that a kernel with CBQ support and not CODEL fails to load a QoS policy with the obscure error "pfctl: DIOCADDALTQ: Cannot allocate memory."', to experience this failure it is also required that the queue configuration include the BORROW flag ('borrow' keyword in pf.conf), as that sets the bit that is misinterpreted by rmclass as requesting CODEL.

Nov 14 2018, 4:55 PM

Nov 13 2018

pkelsey accepted D17979: Fix leaks caused by ifc_nhwtxqs never being initialized.

Looks good to me.

Nov 13 2018, 7:36 PM

Aug 23 2018

pkelsey committed rS338260: Fix warning about crossing INT32_MAX boundary in computation of constant value..
Fix warning about crossing INT32_MAX boundary in computation of constant value.
Aug 23 2018, 5:41 PM
pkelsey committed rS338253: Extend tbrsize heuristic in pfctl(8) to provide a sensible value for.
Extend tbrsize heuristic in pfctl(8) to provide a sensible value for
Aug 23 2018, 4:10 PM
pkelsey closed D16852: Update pfctl(8) tbrsize heuristic for high bandwidth interfaces.
Aug 23 2018, 4:10 PM
pkelsey added a comment to D16852: Update pfctl(8) tbrsize heuristic for high bandwidth interfaces.

This seems straightforward, reasonable, and sufficient. Longer-term, I'd wonder about some sort of arithmetic approach here rather than this kind of hand-scaling?

Aug 23 2018, 3:33 AM

Aug 22 2018

pkelsey created D16852: Update pfctl(8) tbrsize heuristic for high bandwidth interfaces.
Aug 22 2018, 8:59 PM
pkelsey updated the test plan for D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.
Aug 22 2018, 7:40 PM
pkelsey committed rS338209: Extended pf(4) ioctl interface and pfctl(8) to allow bandwidths of.
Extended pf(4) ioctl interface and pfctl(8) to allow bandwidths of
Aug 22 2018, 7:39 PM
pkelsey closed D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.
Aug 22 2018, 7:39 PM

Aug 19 2018

pkelsey updated the diff for D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.

Incorporated all reviewer comments up to this point.

Aug 19 2018, 5:25 PM
pkelsey updated the test plan for D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.
Aug 19 2018, 5:02 PM
pkelsey added inline comments to D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.
Aug 19 2018, 3:57 PM

Aug 18 2018

pkelsey added inline comments to D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.
Aug 18 2018, 4:23 PM
pkelsey added a comment to D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.

It'd be really awesome if you could also see about writing some basic tests (both as pfctl parser tests, to check we can now set higher bitrates for shaping, and as a basic test that shaping actually happens), but that's certainly not blocking.

Aug 18 2018, 4:20 PM
pkelsey added inline comments to D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.
Aug 18 2018, 4:06 PM
pkelsey updated the test plan for D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.
Aug 18 2018, 10:19 AM
pkelsey updated the summary of D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.
Aug 18 2018, 10:18 AM
pkelsey created D16782: Fix 32-bit queue bandwidth limitation in pf and in the ALTQ token bucket regulator and HFSC scheduler.
Aug 18 2018, 9:59 AM

Aug 4 2018

pkelsey committed rS337281: Mark the send queue ready so ALTQ is available..
Mark the send queue ready so ALTQ is available.
Aug 4 2018, 1:45 AM

Jul 25 2018

pkelsey committed rS336727: ALTQ support for iflib..
ALTQ support for iflib.
Jul 25 2018, 10:46 PM
pkelsey closed D16433: Add ALTQ support to iflib.
Jul 25 2018, 10:46 PM

Jul 24 2018

pkelsey created D16433: Add ALTQ support to iflib.
Jul 24 2018, 8:04 PM

Mar 7 2018

pkelsey added a comment to D14048: Modify handling of TCP_RFC7413 (TFO) kernel option.

Going to commit this soon as the review has been open for nearly one month and has been idle for two weeks.

Mar 7 2018, 4:47 PM
pkelsey added inline comments to D14554: Support printing of the TCP fast open client side cache.
Mar 7 2018, 4:43 PM
pkelsey closed D14048: Modify handling of TCP_RFC7413 (TFO) kernel option.
Mar 7 2018, 4:42 PM
pkelsey closed D14047: TCP Fast Open (TFO) [RFC7413] Client-side Implementation (take two).
Mar 7 2018, 4:42 PM
pkelsey updated the diff for D14047: TCP Fast Open (TFO) [RFC7413] Client-side Implementation (take two).

Addressed reviewer comments.

Mar 7 2018, 4:42 PM
pkelsey added a comment to D14047: TCP Fast Open (TFO) [RFC7413] Client-side Implementation (take two).

Going to commit this soon as there has not been any further review comments in nearly three weeks.

Mar 7 2018, 4:42 PM

Mar 6 2018

pkelsey accepted D14554: Support printing of the TCP fast open client side cache.

Looks good. Thanks.

Mar 6 2018, 3:46 AM

Feb 26 2018

pkelsey committed rS330035: Ensure signed comparison to avoid false trip of assert during VNET teardown..
Ensure signed comparison to avoid false trip of assert during VNET teardown.
Feb 26 2018, 8:31 PM
pkelsey committed rS330002: Greatly reduce the number of #ifdefs supporting the TCP_RFC7413 kernel option..
Greatly reduce the number of #ifdefs supporting the TCP_RFC7413 kernel option.
Feb 26 2018, 3:04 AM
pkelsey committed rS330001: This is an implementation of the client side of TCP Fast Open (TFO).
This is an implementation of the client side of TCP Fast Open (TFO)
Feb 26 2018, 2:53 AM
pkelsey committed rS330000: Fix harmless locking bug in tfp_fastopen_check_cookie()..
Fix harmless locking bug in tfp_fastopen_check_cookie().
Feb 26 2018, 2:43 AM
pkelsey closed D14046: Fix harmless locking bug in tcp_fastopen_check_cookie().
Feb 26 2018, 2:43 AM

Jan 31 2018

pkelsey added inline comments to D14047: TCP Fast Open (TFO) [RFC7413] Client-side Implementation (take two).
Jan 31 2018, 3:39 AM

Jan 30 2018

pkelsey added inline comments to D14047: TCP Fast Open (TFO) [RFC7413] Client-side Implementation (take two).
Jan 30 2018, 6:45 AM

Jan 28 2018

pkelsey added inline comments to D14047: TCP Fast Open (TFO) [RFC7413] Client-side Implementation (take two).
Jan 28 2018, 3:35 PM

Jan 25 2018

pkelsey committed rS328392: MFC of r305169:.
MFC of r305169:
Jan 25 2018, 7:27 AM
pkelsey committed rS328391: MFC of r316630, r316631, r316632, r316633, r316634.
MFC of r316630, r316631, r316632, r316633, r316634
Jan 25 2018, 7:19 AM
pkelsey created D14048: Modify handling of TCP_RFC7413 (TFO) kernel option.
Jan 25 2018, 7:10 AM
pkelsey added a child revision for D14047: TCP Fast Open (TFO) [RFC7413] Client-side Implementation (take two): D14048: Modify handling of TCP_RFC7413 (TFO) kernel option.
Jan 25 2018, 7:09 AM
pkelsey created D14047: TCP Fast Open (TFO) [RFC7413] Client-side Implementation (take two).
Jan 25 2018, 6:18 AM
pkelsey added a child revision for D14046: Fix harmless locking bug in tcp_fastopen_check_cookie(): D14047: TCP Fast Open (TFO) [RFC7413] Client-side Implementation (take two).
Jan 25 2018, 6:18 AM
pkelsey created D14046: Fix harmless locking bug in tcp_fastopen_check_cookie().
Jan 25 2018, 6:12 AM
pkelsey abandoned D13718: TCP Fast Open (TFO) [RFC7413] Client-side Implementation.

Breaking into separate reviews.

Jan 25 2018, 6:01 AM
pkelsey committed rD51393: Partially document __FreeBSD_version value 1101507..
Partially document __FreeBSD_version value 1101507.
Jan 25 2018, 2:58 AM
pkelsey committed rS328386: MFC r316648:.
MFC r316648:
Jan 25 2018, 2:45 AM

Jan 4 2018

pkelsey added inline comments to D13718: TCP Fast Open (TFO) [RFC7413] Client-side Implementation.
Jan 4 2018, 3:57 AM

Jan 2 2018

pkelsey added inline comments to D13718: TCP Fast Open (TFO) [RFC7413] Client-side Implementation.
Jan 2 2018, 9:44 PM
pkelsey added reviewers for D13718: TCP Fast Open (TFO) [RFC7413] Client-side Implementation: hiren, tuexen.
Jan 2 2018, 9:25 PM
pkelsey added a comment to D13718: TCP Fast Open (TFO) [RFC7413] Client-side Implementation.

Awesome! Thank you for this work!

Now, sorry to be a PITA but you should separate out this into 3 reviews: 1) fast-open ON by default removing TCP_RFC7413 2) Bug fix for the issue in tcp_fastopen_check_cookie() 3) Client side TFO implementation.

I can do this I suppose, although I have to say it's hard for me to see the value in doing it as (2) and (3) seem pretty easy to evaluate in the current patchset, as (2) consists of moving a lock acquisition earlier in a small function and ensuring release on all the exit paths, and (3) is the mechanical removal of #ifdefs and the related rename of a kernel option that is used in one place. Perhaps because I don't think I fully understand the practical motivation for this separation, I'm also not sure whether or not it matters how I synthesize the new versions of the code - should they all be independent diffs against -current (which seems weird as it can hide interactions between the changes from review), or is there some preferred sequence (should the locking bug be presented as happening before or after all the other changes to same function)?

Motivation: easier on reviewers.
Also, I don't think you'd want to commit this as is, do you? These are 3 separate changes and I prefer 3 different commits. But as you are the one doing actual work, you can decide how to go about it. :-)
I'd get 2) and 3) committed first as they are small and then generate 1) for larger review. But again, your call. :-)

Jan 2 2018, 7:40 PM
pkelsey added a comment to D13718: TCP Fast Open (TFO) [RFC7413] Client-side Implementation.

I agree with hiren... Splitting this up in three patch sets makes a lot of sense and makes the review much simpler. While doing that, you could also change the name of the sysctl variables net.inet.tcp.fastopen.server_enabled to net.inet.tcp.fastopen.server_enable and net.inet.tcp.fastopen.client_enabled to net.inet.tcp.fastopen.client_enable to be consistent with other sysctl variables controlling the usage of features.

With respect to 'enable' vs 'enabled', I considered this issue, but I didn't find the consistency you are claiming. I saw that both are used, and I was unable to discern any rule as to when one was employed versus the other, so I picked the one that reads as a status indicator as that seemed like it would be the more frequent reason they would be read. As seen on an r327445 kernel (with my TFO patches applied, hence the grep -v), 'enabled' is used in 8 names (5 of them are loader tunables, and an overlapping 5 are runtime writeable):

$ sysctl -aN | grep -v fastopen | grep enabled | wc -l

8
Jan 2 2018, 6:18 PM
pkelsey added a comment to D13718: TCP Fast Open (TFO) [RFC7413] Client-side Implementation.

I agree with hiren... Splitting this up in three patch sets makes a lot of sense and makes the review much simpler. While doing that, you could also change the name of the sysctl variables net.inet.tcp.fastopen.server_enabled to net.inet.tcp.fastopen.server_enable and net.inet.tcp.fastopen.client_enabled to net.inet.tcp.fastopen.client_enable to be consistent with other sysctl variables controlling the usage of features.

Jan 2 2018, 5:59 PM
pkelsey added a comment to D13718: TCP Fast Open (TFO) [RFC7413] Client-side Implementation.

Awesome! Thank you for this work!

Now, sorry to be a PITA but you should separate out this into 3 reviews: 1) fast-open ON by default removing TCP_RFC7413 2) Bug fix for the issue in tcp_fastopen_check_cookie() 3) Client side TFO implementation.

Jan 2 2018, 5:47 PM

Dec 31 2017

pkelsey created D13718: TCP Fast Open (TFO) [RFC7413] Client-side Implementation.
Dec 31 2017, 8:56 PM

Oct 1 2017

pkelsey committed rS324181: The soisconnected() call removed from syncache_socket() in r307966 was.
The soisconnected() call removed from syncache_socket() in r307966 was
Oct 1 2017, 11:37 PM

Jun 5 2017

pkelsey committed rD50322: Update my PGP key..
Update my PGP key.
Jun 5 2017, 5:08 PM

Apr 25 2017

pkelsey closed D10322: Remove unnecessary check for NULL mbuf in soreceive_generic() by committing rS317421: Remove unnecessary check for NULL mbuf in soreceive_generic()..
Apr 25 2017, 7:54 PM
pkelsey committed rS317421: Remove unnecessary check for NULL mbuf in soreceive_generic()..
Remove unnecessary check for NULL mbuf in soreceive_generic().
Apr 25 2017, 7:54 PM

Apr 16 2017

pkelsey committed rS317035: Fix userland tools that don't check the format of routing socket.
Fix userland tools that don't check the format of routing socket
Apr 16 2017, 7:17 PM
pkelsey closed D10330: Fix bugs in routing socket use by userland tools by committing rS317035: Fix userland tools that don't check the format of routing socket.
Apr 16 2017, 7:17 PM

Apr 10 2017

pkelsey committed rD50150: Document __FreeBSD_version 1200028.
Document __FreeBSD_version 1200028
Apr 10 2017, 6:18 PM
pkelsey committed rS316683: Bump __FreeBSD_version due to r316648, rename of.
Bump __FreeBSD_version due to r316648, rename of
Apr 10 2017, 5:59 PM

Apr 9 2017

pkelsey updated the diff for D10330: Fix bugs in routing socket use by userland tools.

break -> continue in the addition to sbin/routed/table.c

Apr 9 2017, 6:09 PM
pkelsey added inline comments to D10330: Fix bugs in routing socket use by userland tools.
Apr 9 2017, 6:02 PM
pkelsey added a comment to D10330: Fix bugs in routing socket use by userland tools.
In D10330#213998, @ae wrote:

Looks good to me. Also I want to note, that all

do 
 read();
} while()

loops are affected to the problem described in rS303374. It would be nice to fix this problem too. :)

Apr 9 2017, 5:48 PM
pkelsey added inline comments to D10330: Fix bugs in routing socket use by userland tools.
Apr 9 2017, 5:01 PM
pkelsey updated the summary of D10330: Fix bugs in routing socket use by userland tools.
Apr 9 2017, 6:51 AM
pkelsey added a reviewer for D10330: Fix bugs in routing socket use by userland tools: network.
Apr 9 2017, 6:48 AM
pkelsey added a member for network: pkelsey.
Apr 9 2017, 6:48 AM
pkelsey created D10330: Fix bugs in routing socket use by userland tools.
Apr 9 2017, 6:46 AM
pkelsey committed rS316648: Corrected misspelled versions of rendezvous..
Corrected misspelled versions of rendezvous.
Apr 9 2017, 2:00 AM
pkelsey closed D10313: rendevous -> rendezvous by committing rS316648: Corrected misspelled versions of rendezvous..
Apr 9 2017, 2:00 AM

Apr 8 2017

pkelsey added a reviewer for D10322: Remove unnecessary check for NULL mbuf in soreceive_generic(): bms.
Apr 8 2017, 6:02 PM
pkelsey created D10322: Remove unnecessary check for NULL mbuf in soreceive_generic().
Apr 8 2017, 6:00 PM
pkelsey created D10318: Remove redundant checks of rtm_type in route_output().
Apr 8 2017, 3:24 PM
pkelsey committed rS316634: Fixed typo in comment found while reading commit email for fix of.
Fixed typo in comment found while reading commit email for fix of
Apr 8 2017, 4:51 AM
pkelsey committed rS316633: Fixed typo in comment..
Fixed typo in comment.
Apr 8 2017, 4:46 AM
pkelsey committed rS316632: Fixed typo..
Fixed typo.
Apr 8 2017, 4:42 AM
pkelsey committed rS316631: Fix typo in comment..
Fix typo in comment.
Apr 8 2017, 4:37 AM
pkelsey committed rS316630: Fix typo..
Fix typo.
Apr 8 2017, 4:34 AM
pkelsey created D10313: rendevous -> rendezvous.
Apr 8 2017, 4:10 AM

Feb 15 2017

pkelsey accepted D9606: Remove compatibility with old libpcap..

Previously the code is hosted at Google code and that become read-only.

I think Luigi have another repository here: https://github.com/luigirizzo/netmap-libpcap but the code haven't been updated for some time.

The main motivation is that the pcap internal interfaces changes quite frequently, and it have become more and more burdensome to maintain the compatibility shims with older pcap versions (see rS313695, for instance, that we technically need additional #ifdef's to maintain compatibility with older pcap versions).

Feb 15 2017, 8:10 PM
pkelsey added a comment to D9606: Remove compatibility with old libpcap..

This makes sense to me from the standpoint of the removed code being unnecessary given that it is being maintained as part of a version of libpcap that doesn't require it.

Feb 15 2017, 2:40 PM
pkelsey accepted D9465: Remove unnecessary ifdef soup from struct tcpcb.
Feb 15 2017, 1:29 AM
pkelsey added a comment to D9465: Remove unnecessary ifdef soup from struct tcpcb.

We are trying to keep the same size and layout for a given machine type. The issue is that the TCPPCAP stuff used up uint64_t padding, and consumed different amounts of it on LP64 and LP32 systems, hence the #ifdef.

Feb 15 2017, 1:27 AM

Feb 14 2017

pkelsey added a comment to D9465: Remove unnecessary ifdef soup from struct tcpcb.

I don't have any TFO-specific complaints with removing those #ifdefs. I put them there out of uncertainty about any existing policy on userland-ABI concerns and they made it through code review. In hindsight, the #else parts of the #ifdefs should have had two items - t_Xopaque[] and t_Xspare[], because I was never of the mind that the space used by TFO might ever be used instead by something else that would then could only be enabled mutually-exclusively with TFO.

Feb 14 2017, 10:06 PM

Feb 9 2017

pkelsey added a comment to D5017: More than 65K connection from single application.

I think that "collision" is ok.

Of course it IS a new behaviour, but that is what we are trying to get. new behaviour.

Feb 9 2017, 5:55 PM · network
pkelsey added a comment to D5017: More than 65K connection from single application.

This is the source comment that should have been submitted with my 'request for change' action, had I pressed the buttons in a way that didn't dispose of it instead.

Feb 9 2017, 5:23 PM · network
pkelsey requested changes to D5017: More than 65K connection from single application.
Feb 9 2017, 5:15 PM · network

Feb 3 2017

pkelsey committed rS313168: Fix VIMAGE-related bugs in TFO. The autokey callout vnet context was.
Fix VIMAGE-related bugs in TFO. The autokey callout vnet context was
Feb 3 2017, 5:03 PM

Oct 15 2016

pkelsey committed rS307337: Fix cases where the TFO pending counter would leak references, and eventually….
Fix cases where the TFO pending counter would leak references, and eventually…
Oct 15 2016, 1:41 AM
pkelsey closed D8235: Fix TFO pending counter leaks by committing rS307337: Fix cases where the TFO pending counter would leak references, and eventually….
Oct 15 2016, 1:41 AM

Oct 14 2016

pkelsey added a member for transport: pkelsey.
Oct 14 2016, 2:04 AM

Oct 13 2016

pkelsey updated the diff for D8235: Fix TFO pending counter leaks.

Adjusted label names and comments

Oct 13 2016, 7:23 PM

Oct 12 2016

pkelsey retitled D8235: Fix TFO pending counter leaks from to Fix TFO pending counter leaks.
Oct 12 2016, 10:56 PM
pkelsey added a comment to D8218: Close t_tfo_pending leaks.
In D8218#170900, @jtl wrote:

This is functionally equivalent, but I don't think it is clearer. I think it is less clear that the conditional decrement is placed after the tfo_done label as it will never execute after a goto tfo_done.

I placed it there as a matter of future-proofing. It doesn't hurt anything for it to be after the tfo_done label. But, if someone makes a code change to add a new goto tfo_done, it will still hit this check.

Because you obviously have strong feelings for exactly how this code should be (and this doesn't really impact me), I've opened a bug and assigned it to you. You can fix it at your convenience.

Oct 12 2016, 8:52 PM
pkelsey added a comment to D8218: Close t_tfo_pending leaks.

This is functionally equivalent, but I don't think it is clearer. I think it is less clear that the conditional decrement is placed after the tfo_done label as it will never execute after a goto tfo_done. There is clear one-to-one correspondence between creating a TFO socket and not needing this decrement that I think is blurred by this positioning because the decrement is not an action that is common to 'done' and 'tfo_done', it is unique to 'done'. I think we would be better off if the code wasn't arranging to cancel a conditional action because it was placed in a path it never needs to be in.

Oct 12 2016, 7:05 PM

Oct 11 2016

pkelsey added a comment to D8218: Close t_tfo_pending leaks.
In D8218#170623, @jtl wrote:

I can only identify one circumstance where the t_tfo_pending counter on a listen socket will be incremented without a corresponding decrement ever occurring - when all of the following conditions are met:

  1. TFO is enabled in the system
  2. A TFO SYN with an invalid TFO cookie matches a listen socket that has TFO enabled
  3. The current t_tfo_pending count on that socket is <= so_qlimit / 2

The other way is...

  1. TFO enabled
  2. t_tfo_pending count <= so_qlimit / 2
  3. SYN without a TFO option, followed by a SYN with a valid TFO cookie and the same IPs and port numbers

See line-specific comments in the patch.

I don't see in-line comments. Perhaps you can resubmit them?

Oct 11 2016, 7:59 PM