jason_eggnet.com (Jason Eggleston)
User

Projects

User Details

User Since
Sep 8 2015, 10:30 PM (137 w, 15 h)

Recent Activity

Mon, Mar 26

jason_eggnet.com added a comment to D14816: TCP_INFO for offloaded connections..

Any radical departure from the current behavior of basically creating and copying struct tcp_info to user space would require a different getsockopt() optname at a bare minimum, and we'd still have to support the current TCP_INFO getsockopt() anyway.

Mon, Mar 26, 6:37 PM

Mar 23 2018

jason_eggnet.com abandoned D14808: do not hold in6p lock during malloc in ip6_pcbopts().

malloc and ip options are a bigger problem, going back to the drawing board.

Mar 23 2018, 4:22 PM
jason_eggnet.com created D14808: do not hold in6p lock during malloc in ip6_pcbopts().
Mar 23 2018, 3:50 PM
jason_eggnet.com added a comment to D14802: fix malloc length in ip6_output.c:GET_PKTOPT_VAR.

@melifaro I'm thinking about how to make GET_PKTOPT_VAR a function or perhaps... how to make it much smaller and call a function for the bulk of the logic.

Mar 23 2018, 1:07 AM
jason_eggnet.com updated the diff for D14802: fix malloc length in ip6_output.c:GET_PKTOPT_VAR.

additional fixes

Mar 23 2018, 1:02 AM
jason_eggnet.com created D14802: fix malloc length in ip6_output.c:GET_PKTOPT_VAR.
Mar 23 2018, 12:51 AM
jason_eggnet.com abandoned D14619: refactor ip6_getpcbopt() for better locking and memory management.

I will start a new review to address feedback from @melifaro, this one is broken.

Mar 23 2018, 12:49 AM
jason_eggnet.com updated the diff for D14619: refactor ip6_getpcbopt() for better locking and memory management.

fix malloc length in ip6_output.c:GET_PKTOPT_VAR

Mar 23 2018, 12:43 AM
jason_eggnet.com added inline comments to D14619: refactor ip6_getpcbopt() for better locking and memory management.
Mar 23 2018, 12:38 AM
jason_eggnet.com reopened D14619: refactor ip6_getpcbopt() for better locking and memory management.
Mar 23 2018, 12:08 AM

Mar 22 2018

jason_eggnet.com updated the diff for D14621: fix memory management for fetching options in ip_ctloutput().

fix typo

Mar 22 2018, 7:15 PM
jason_eggnet.com updated the diff for D14621: fix memory management for fetching options in ip_ctloutput().

use long for len passed to malloc in ip_ctloutput() for getting IP_RETOPTS

Mar 22 2018, 6:02 PM
jason_eggnet.com updated the diff for D14621: fix memory management for fetching options in ip_ctloutput().

fix len passed to malloc in ip_ctloutput() for getting IP_RETOPTS

Mar 22 2018, 5:52 PM

Mar 20 2018

jason_eggnet.com added inline comments to D14621: fix memory management for fetching options in ip_ctloutput().
Mar 20 2018, 2:58 AM

Mar 19 2018

jason_eggnet.com updated subscribers of D14621: fix memory management for fetching options in ip_ctloutput().
Mar 19 2018, 11:58 AM

Mar 10 2018

jason_eggnet.com retitled D14623: fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl() from Fix locking for IPSEC_PCBCTL and related functions. to fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl().
Mar 10 2018, 1:07 AM
jason_eggnet.com updated the diff for D14623: fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl().

fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl()

Mar 10 2018, 1:06 AM

Mar 9 2018

jason_eggnet.com added a comment to D14623: fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl().
In D14623#307351, @ae wrote:

Why you need INP_WLOCK() for getsockopt()? Also ipsec_get_pcbpolicy() requieres INP_WLOCK(), and IPv4 version uses WLOCK, but IPv6 version uses RLOCK.

Mar 9 2018, 3:59 PM
jason_eggnet.com updated the diff for D14623: fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl().

fix IPv6 IPSEC_PCBCTL locking to be consistent with IPv4

Mar 9 2018, 3:48 PM

Mar 8 2018

jason_eggnet.com added inline comments to D14540: Several LRO fixes.
Mar 8 2018, 5:36 PM · transport
jason_eggnet.com added reviewers for D14624: simple locking fixes in ip_ctloutput, ip6_ctloutput, rip_ctloutput: transport, network, rwatson.
Mar 8 2018, 4:44 PM
jason_eggnet.com added reviewers for D14623: fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl(): transport, network, ae, rwatson.
Mar 8 2018, 4:44 PM
jason_eggnet.com added reviewers for D14622: handle locking and memory safety for IPV6_PATHMTU in ip6_ctloutput(): transport, network, rwatson.
Mar 8 2018, 4:43 PM
jason_eggnet.com added a reviewer for D14621: fix memory management for fetching options in ip_ctloutput(): rwatson.
Mar 8 2018, 4:43 PM
jason_eggnet.com added reviewers for D14620: improve write locking in ip6_ctloutput() with macros: transport, network, rwatson.
Mar 8 2018, 4:42 PM
jason_eggnet.com added a reviewer for D14619: refactor ip6_getpcbopt() for better locking and memory management: rwatson.
Mar 8 2018, 4:41 PM
jason_eggnet.com added reviewers for D14619: refactor ip6_getpcbopt() for better locking and memory management: transport, network.
Mar 8 2018, 4:41 PM
jason_eggnet.com abandoned D12586: improve inp locking in getsockopt, setsockopt, and related functions.

I am abandoning this in favor of 6 smaller reviews, per @rwatson's general advice to break up the patch.

Mar 8 2018, 4:38 PM
jason_eggnet.com created D14624: simple locking fixes in ip_ctloutput, ip6_ctloutput, rip_ctloutput.
Mar 8 2018, 4:01 PM
jason_eggnet.com created D14623: fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl().
Mar 8 2018, 4:01 PM
jason_eggnet.com created D14622: handle locking and memory safety for IPV6_PATHMTU in ip6_ctloutput().
Mar 8 2018, 4:00 PM
jason_eggnet.com created D14621: fix memory management for fetching options in ip_ctloutput().
Mar 8 2018, 4:00 PM
jason_eggnet.com created D14620: improve write locking in ip6_ctloutput() with macros.
Mar 8 2018, 3:59 PM
jason_eggnet.com created D14619: refactor ip6_getpcbopt() for better locking and memory management.
Mar 8 2018, 3:57 PM

Mar 7 2018

jason_eggnet.com added inline comments to D14540: Several LRO fixes.
Mar 7 2018, 11:13 PM · transport

Feb 13 2018

jason_eggnet.com updated the diff for D14141: fix underflow for cubic_cwnd().

use flag instead of cached value for old max_cwnd tracking in cubic

Feb 13 2018, 7:12 PM
jason_eggnet.com added a comment to D14141: fix underflow for cubic_cwnd().

@jason_eggnet.com why did you move K calculation from post recovery to ack received? I don't think that's correct.

So basically, why not delay the calculation of K until right before it is needed. Use the previous value of K if neither of the inputs have changed.

I am hoping this is just the 'debugging' code to find out why we are getting a stale value of K and not something to commit.
Or I am thoroughly confused at this point. :-)

I think you're confused :) I've updated the commit in the review formally, let me know if you have any questions, I think I've detailed it reasonably well.

Feb 13 2018, 8:24 AM

Feb 11 2018

jason_eggnet.com updated the diff for D14141: fix underflow for cubic_cwnd().

off by one with overflow threshold in cubic_cwnd

Feb 11 2018, 6:38 PM

Feb 10 2018

jason_eggnet.com added a comment to D14141: fix underflow for cubic_cwnd().

@jason_eggnet.com why did you move K calculation from post recovery to ack received? I don't think that's correct.

So basically, why not delay the calculation of K until right before it is needed. Use the previous value of K if neither of the inputs have changed.

I am hoping this is just the 'debugging' code to find out why we are getting a stale value of K and not something to commit.
Or I am thoroughly confused at this point. :-)

Feb 10 2018, 1:22 AM
jason_eggnet.com updated the diff for D14141: fix underflow for cubic_cwnd().

Fix underflow and overflow conditions in the cubic cc

Feb 10 2018, 1:20 AM

Feb 9 2018

jason_eggnet.com added a comment to D14141: fix underflow for cubic_cwnd().

@jason_eggnet.com why did you move K calculation from post recovery to ack received? I don't think that's correct.

Feb 9 2018, 1:22 AM

Feb 8 2018

jason_eggnet.com added a comment to D14141: fix underflow for cubic_cwnd().

@jason_eggnet.com I thought about this some more and while there is no doubt that overflow/underflow due to the function's inputs is possible and needs to be remedied, the cause of overflow in your test case is not in fact the time since congestion being too large, but the bogus value of K, which for a wmax of 1460 bytes (i.e. slightly more than 1 MSS) should be 205 per my quick check:

Feb 8 2018, 5:02 PM

Feb 5 2018

jason_eggnet.com added a comment to D14141: fix underflow for cubic_cwnd().

I remember this one. :-)
jegg, can you please elaborate on 'cwnd < 1smss bad for other parts of the code' part?
Thanks.

A zero cwnd can cause infinite looping (i.e., continual goto again; looping) within tcp_output(). Technically even one byte would be ok but there's no real point to having the cwnd be some fractional mss value.

In general the concept of having a zero cwnd doesn't make any sense.

oh, I know 0 won't do any good. :-)

I meant why not 0 < cwnd < mss. I know we generally stick to multiple of mss for cwnd but just wondering what goes wrong if we don't. (Specially when we are bytes based as opposed to packets based wrt cwnd).

Thanks.

Feb 5 2018, 6:29 PM
jason_eggnet.com added a comment to D14141: fix underflow for cubic_cwnd().

I remember this one. :-)
jegg, can you please elaborate on 'cwnd < 1smss bad for other parts of the code' part?
Thanks.

Feb 5 2018, 6:03 PM

Jan 31 2018

jason_eggnet.com added a comment to D14141: fix underflow for cubic_cwnd().
In D14141#296724, @jtl wrote:

Is this truly an underflow? Or, is it a low number caused by an overflow in one of the earlier calculations? (It might help if you could give a precise example from your testing.)

The distinction is important because this might not be the correct solution to an overflow in one of the earlier calculations.

Jan 31 2018, 5:24 AM
jason_eggnet.com added a reviewer for D14141: fix underflow for cubic_cwnd(): transport.
Jan 31 2018, 4:45 AM
jason_eggnet.com updated subscribers of D14141: fix underflow for cubic_cwnd().
Jan 31 2018, 4:44 AM
jason_eggnet.com created D14141: fix underflow for cubic_cwnd().
Jan 31 2018, 4:43 AM

Dec 7 2017

jason_eggnet.com updated the diff for D12586: improve inp locking in getsockopt, setsockopt, and related functions.

Fix memory leak in ip_ctloutput; add documentation to pcbctl function.

Dec 7 2017, 8:50 PM
jason_eggnet.com updated the diff for D12586: improve inp locking in getsockopt, setsockopt, and related functions.

Missed INP_WUNLOCK() in ipv6 setsockopt() processing

Dec 7 2017, 8:21 PM

Oct 27 2017

jason_eggnet.com updated the diff for D12586: improve inp locking in getsockopt, setsockopt, and related functions.

remove in_pcbref in ctl functions

Oct 27 2017, 2:20 AM

Oct 26 2017

jason_eggnet.com added a comment to D12586: improve inp locking in getsockopt, setsockopt, and related functions.

It looks like a socket is guaranteed to have a single defined inp that is valid (specifically, not free'd) from the time socket or accept is called all the way to the time that close is called.

Oct 26 2017, 10:37 PM
jason_eggnet.com updated the diff for D12586: improve inp locking in getsockopt, setsockopt, and related functions.

allow for inp / in6p being null in certain cases within ip_ctl functions

Oct 26 2017, 4:08 AM
jason_eggnet.com added a comment to D12586: improve inp locking in getsockopt, setsockopt, and related functions.

It looks like there might be cases in ip_ctloutput, ip6_ctloutput, rip_ctloutput, and ip6_raw_ctloutput where the inp/in6p being null is ok.

Oct 26 2017, 3:27 AM
jason_eggnet.com updated the diff for D12586: improve inp locking in getsockopt, setsockopt, and related functions.

update outstanding issues

Oct 26 2017, 3:00 AM

Oct 13 2017

jason_eggnet.com added inline comments to D12586: improve inp locking in getsockopt, setsockopt, and related functions.
Oct 13 2017, 4:00 AM

Oct 10 2017

jason_eggnet.com abandoned D12583: check so_error earlier in sosend_generic.

Sounds good, I'll abandon this.

Oct 10 2017, 10:09 PM
jason_eggnet.com added a comment to D12633: match sendfile() error handling to send().

No problem, thanks :)

Oct 10 2017, 10:04 PM
jason_eggnet.com added a comment to D12633: match sendfile() error handling to send().

Ok, Sean just reverted the previous patch, and this patch is based on the new HEAD.

Oct 10 2017, 12:58 AM
jason_eggnet.com updated the diff for D12633: match sendfile() error handling to send().

rebase patch to new HEAD

Oct 10 2017, 12:57 AM
jason_eggnet.com added a comment to D12583: check so_error earlier in sosend_generic.

We'll probably be abandoning this review in favor of keeping the current behavior and ensuring sendfile behaves the same as send and write.

Oct 10 2017, 12:08 AM
jason_eggnet.com created D12633: match sendfile() error handling to send().
Oct 10 2017, 12:07 AM

Oct 9 2017

jason_eggnet.com added a comment to D12583: check so_error earlier in sosend_generic.

Gleb wants us to hold off on this for the moment, with the first step of making sendfile() work like current send() / write() paths.

Oct 9 2017, 11:39 PM
jason_eggnet.com added a reviewer for D12583: check so_error earlier in sosend_generic: glebius.
Oct 9 2017, 11:38 PM

Oct 5 2017

jason_eggnet.com retitled D12586: improve inp locking in getsockopt, setsockopt, and related functions from ensure inp is locked during setsockopt and getsockopt to improve inp locking in getsockopt, setsockopt, and related functions.
Oct 5 2017, 10:43 PM
jason_eggnet.com updated the diff for D12586: improve inp locking in getsockopt, setsockopt, and related functions.
  • Consistently handle locking in tcp_ipsec_pcbctl(), as well as use rlocks for reads, and wlocks for writes.
Oct 5 2017, 10:33 PM

Oct 4 2017

jason_eggnet.com added a comment to D12586: improve inp locking in getsockopt, setsockopt, and related functions.
In D12586#261038, @ae wrote:

Probably you need to modify netipsec/ipsec_pcb.c too.

Oct 4 2017, 4:16 PM
jason_eggnet.com added a comment to D12586: improve inp locking in getsockopt, setsockopt, and related functions.

This issue was discovered by rare memory corruption when calling ip6_optlen(). The likely culprit was unlocked manipulation of in6p->in6p_outputopts.

Oct 4 2017, 4:09 AM
jason_eggnet.com updated the summary of D12586: improve inp locking in getsockopt, setsockopt, and related functions.
Oct 4 2017, 3:59 AM
jason_eggnet.com updated the diff for D12586: improve inp locking in getsockopt, setsockopt, and related functions.

rename OPTSET2292_REQUIRED to OPTSET2282_EXCLUSIVE

Oct 4 2017, 3:54 AM
jason_eggnet.com created D12586: improve inp locking in getsockopt, setsockopt, and related functions.
Oct 4 2017, 2:03 AM

Oct 3 2017

jason_eggnet.com added a comment to D12583: check so_error earlier in sosend_generic.

Before patch:

Oct 3 2017, 11:10 PM
jason_eggnet.com added a reviewer for D12583: check so_error earlier in sosend_generic: transport.
Oct 3 2017, 11:07 PM
jason_eggnet.com created D12583: check so_error earlier in sosend_generic.
Oct 3 2017, 11:07 PM
jason_eggnet.com added a reviewer for D12575: Check so_error early in sendfile() call: transport.
Oct 3 2017, 2:56 PM
jason_eggnet.com added a comment to D12575: Check so_error early in sendfile() call.

Using packetdrill with this https://github.com/nplab/packetdrill/pull/137

Oct 3 2017, 2:54 PM
jason_eggnet.com created D12575: Check so_error early in sendfile() call.
Oct 3 2017, 2:45 PM

Oct 2 2017

jason_eggnet.com abandoned D12501: count bytes in vn_sendfile when pru_send returns EAGAIN.

Abandoning this due to insufficient proof of a problem.

Oct 2 2017, 8:57 PM
jason_eggnet.com abandoned D12492: Ensure strict error handling of tcp_usr_send.

Abandoning this due to insufficient proof of a problem.

Oct 2 2017, 8:56 PM
jason_eggnet.com added a comment to D12492: Ensure strict error handling of tcp_usr_send.

Based on evaluating this more, feedback in https://reviews.freebsd.org/D12501, and some production analysis, I think the only potential exposure here is tcp_usr_send adding bytes to the socket with sbappendstream followed by tcp_connect returning EAGAIN.

Oct 2 2017, 8:01 PM
jason_eggnet.com added a comment to D12501: count bytes in vn_sendfile when pru_send returns EAGAIN.

We're running a modified patch of this in production where a counter is incremented, and we aren't seeing a counter increment.

Oct 2 2017, 7:44 PM

Sep 26 2017

jason_eggnet.com added a comment to D12501: count bytes in vn_sendfile when pru_send returns EAGAIN.

I think a reasonable question here is, why the gate on rem <= 0 for goto noerr;

Sep 26 2017, 7:00 PM
jason_eggnet.com updated the diff for D12501: count bytes in vn_sendfile when pru_send returns EAGAIN.
  • detect when EINTR and EAGAIN can be squelched in sendfile
Sep 26 2017, 6:12 PM
jason_eggnet.com added inline comments to D12501: count bytes in vn_sendfile when pru_send returns EAGAIN.
Sep 26 2017, 6:08 PM
jason_eggnet.com added a comment to D12501: count bytes in vn_sendfile when pru_send returns EAGAIN.
In D12501#259391, @jtl wrote:

OK, thanks! I agree this concept appears consistent with the current user-space send equivalent.

I'm not positive that pru_send is guaranteed to not return EINTR or ERESTART, so it might be worth considering them.

Also, in this particular context (TCP behavior), note that the EAGAIN about which you are concerned is probably not even possible, since it only occurs in the implicit connect case, which sendfile doesn't support.

Sep 26 2017, 5:44 PM
jason_eggnet.com added a comment to D12501: count bytes in vn_sendfile when pru_send returns EAGAIN.

Ok, let's dig in. pru_send, aka tcp_usr_send calls both tcp_connect in some cases, and tcp_output in most cases. Both calls are after sbappendstream() is called. tcp_connect has a pretty clear if rare path to EAGAIN via in_pcbbind and might be the source of the issues we are seeing.

Sep 26 2017, 3:48 PM
jason_eggnet.com added a comment to D12492: Ensure strict error handling of tcp_usr_send.

I've created another diff for the simpler approach. If that looks better, we can abandon the more extensive changes.

Sep 26 2017, 6:55 AM
jason_eggnet.com created D12501: count bytes in vn_sendfile when pru_send returns EAGAIN.
Sep 26 2017, 6:54 AM
jason_eggnet.com added a comment to D12492: Ensure strict error handling of tcp_usr_send.

The primary goal is to ensure that userspace and the kernel are in lock step as far as how many bytes were actually added to the socket, as long as sendfile returns no error or an error that is not fatal to the connection.

Sep 26 2017, 5:49 AM

Sep 25 2017

jason_eggnet.com added a comment to D12430: tcp: Don't "negotiate" MSS..

I'm not seeing where that was discussed but, sounds good to me.

Sep 25 2017, 6:23 PM
jason_eggnet.com added a comment to D12492: Ensure strict error handling of tcp_usr_send.
In D12492#258950, @jtl wrote:

Can you explain how these are different? Both functionally set error to so->so_error, and then set so->so_error to 0. If (so) is properly locked, it looks like these should be equivalent. If (so) is not properly locked, then the new formulation is not guaranteed to overcome this deficiency.

But, I could be missing something obvious... (It is one of "those days" today. :-) )

Sep 25 2017, 6:04 PM
jason_eggnet.com updated the diff for D12492: Ensure strict error handling of tcp_usr_send.

revert so_error change

Sep 25 2017, 5:50 PM
jason_eggnet.com updated the diff for D12492: Ensure strict error handling of tcp_usr_send.

fix so_error handling

Sep 25 2017, 5:26 PM
jason_eggnet.com removed a dependent revision for D12492: Ensure strict error handling of tcp_usr_send: D12493: improve so_error fetching in vn_sendfile.
Sep 25 2017, 5:15 PM
jason_eggnet.com removed a dependency for D12493: improve so_error fetching in vn_sendfile: D12492: Ensure strict error handling of tcp_usr_send.
Sep 25 2017, 5:15 PM
jason_eggnet.com updated the diff for D12492: Ensure strict error handling of tcp_usr_send.
Sep 25 2017, 5:06 PM
jason_eggnet.com abandoned D12493: improve so_error fetching in vn_sendfile.
Sep 25 2017, 5:00 PM
jason_eggnet.com added a dependency for D12493: improve so_error fetching in vn_sendfile: D12492: Ensure strict error handling of tcp_usr_send.
Sep 25 2017, 4:59 PM
jason_eggnet.com added a dependent revision for D12492: Ensure strict error handling of tcp_usr_send: D12493: improve so_error fetching in vn_sendfile.
Sep 25 2017, 4:59 PM
jason_eggnet.com created D12493: improve so_error fetching in vn_sendfile.
Sep 25 2017, 4:58 PM