Page MenuHomeFreeBSD

rrs (Randall Stewart)
User

Projects

User Details

User Since
Jan 22 2015, 5:22 AM (358 w, 6 d)

Recent Activity

Tue, Dec 7

rrs requested review of D33325: tcp: Rack in a rare case we can get stuck sending a very small amount..
Tue, Dec 7, 7:54 PM

Mon, Dec 6

rrs closed D33231: tcp: rack fails to send out a TLP after a MTU change.
Mon, Dec 6, 3:01 PM
rrs committed R10:dadbc042500b: tcp: rack fails to send out a TLP after a MTU change (authored by rrs).
tcp: rack fails to send out a TLP after a MTU change
Mon, Dec 6, 2:59 PM

Fri, Dec 3

rrs updated the diff for D33035: tcp: Add hystart++ to our cubic implementation..

Forgot to update the manual pages as well since the mib variables
are now at the CC level not the lower one. Also make sure the internet draft
version is sited in the man.

Fri, Dec 3, 4:08 PM
rrs requested review of D33249: tcp: Congestion control move to using reference counting..
Fri, Dec 3, 3:29 PM

Thu, Dec 2

rrs updated the test plan for D33231: tcp: rack fails to send out a TLP after a MTU change.
Thu, Dec 2, 5:25 PM
rrs requested review of D33231: tcp: rack fails to send out a TLP after a MTU change.
Thu, Dec 2, 11:35 AM
rrs committed R10:dcf2dfed26ed: tcp: unloading a module that is set to default should error. (authored by rrs).
tcp: unloading a module that is set to default should error.
Thu, Dec 2, 11:16 AM
rrs closed D33229: tcp: unloading a module that is set to default should error..
Thu, Dec 2, 11:16 AM
rrs requested review of D33229: tcp: unloading a module that is set to default should error..
Thu, Dec 2, 7:00 AM

Tue, Nov 30

rrs updated the diff for D33035: tcp: Add hystart++ to our cubic implementation..

Update the diff to take into account Neal Cardwell's comments on tcpm.. i.e.
make the comparison for rounds SEQ_GT not SEQ_GEQ.

Tue, Nov 30, 2:40 PM

Mon, Nov 29

rrs accepted D33098: tcp: don't upgrade a lock just for logging.

Looks like just what we agreed too ;)

Mon, Nov 29, 12:18 PM

Fri, Nov 19

rrs added a comment to D33035: tcp: Add hystart++ to our cubic implementation..

Ahh I see if you get a "RTO event" or an "Application limited" event you
remove the flag. But what about NDUPACK.. (where a loss is taking you
out of SS).. and for that matter maybe we should also when we leave
CSS to CA remove this flag...

Fri, Nov 19, 2:41 PM
rrs added a comment to D33035: tcp: Add hystart++ to our cubic implementation..

One thing that is interesting to me about cubic is that it has this flag
"in slow start" that is added to the flags field, but I never see that
flag be removed.. i.e. when we enter CA.

Fri, Nov 19, 2:38 PM

Thu, Nov 18

rrs updated the diff for D33035: tcp: Add hystart++ to our cubic implementation..

Fix Richards magic numbers.

Thu, Nov 18, 9:42 PM
rrs added inline comments to D33035: tcp: Add hystart++ to our cubic implementation..
Thu, Nov 18, 9:34 PM
rrs updated the diff for D33035: tcp: Add hystart++ to our cubic implementation..

With a bit of testing I figured out I had missed a few things. Now this version
works per the draft. Note that I may have found a problem with the draft though.
Am now waiting on an answer from Praveen.

Thu, Nov 18, 9:32 PM
rrs abandoned D33034: tcp: TCP connection that enter the TCPS_CLOSED state should not retain BB logging or logs..
Thu, Nov 18, 9:03 AM

Wed, Nov 17

rrs updated the diff for D33035: tcp: Add hystart++ to our cubic implementation..

Ok this straightens out the allowed flag to be up at the ccv->flags
level and gets rid of the need for a socket option for the specific
allowing of hystart++ on the CC module. Instead if the flag gets
set any CC module that supports hystart++ can then use it.

Wed, Nov 17, 8:25 PM
rrs accepted D33019: Allow to compile RSS without PCBGROUP..
Wed, Nov 17, 7:44 PM
rrs accepted D32965: tcp_timewait: use on stack struct tcptw as last resort.
Wed, Nov 17, 7:44 PM
rrs accepted D32966: Add tcp_freecb() - single place to free tcpcb..
Wed, Nov 17, 7:43 PM
rrs accepted D33020: Remove "options PCBGROUP".

Only question I have Gleb, is does the removal of PCBGROUP have any performance impacts for a user
that was using it and now no longer can?

Wed, Nov 17, 7:42 PM
rrs accepted D33021: inpcb: reduce some aliased functions after removal of PCBGROUP..
Wed, Nov 17, 7:41 PM
rrs accepted D33022: SMR protection for inpcbs.
Wed, Nov 17, 7:40 PM
rrs added a comment to D33022: SMR protection for inpcbs.
In D33022#745736, @bz wrote:

Given we don't seem to have a `man smr` could you at least expand SMR once in your title/description so people can find what they are looking for as it's virtually invisible to the world still (unless one finds the header file)?

Yes, absence of the manual page bugs me. Not even mention in uma(9). I will reword description.

Wed, Nov 17, 7:27 PM
rrs accepted D33023: tcp_hpts: provide tcp_in_hpts()..
Wed, Nov 17, 7:25 PM
rrs accepted D33024: tcp_hpts: make struct tcp_hpts_entry private to the module..

I am usually not a big fan of this type of actions. I like
structures to be in a header file instead of buried in the
.c file.. but if you really think its a good idea.. i guess its ok.. my
feelings are not that strong about it.

Wed, Nov 17, 7:25 PM
rrs accepted D33025: tcp_hpts: rename input queue to drop queue and trim dead code.
Wed, Nov 17, 7:23 PM
rrs accepted D33026: tcp_hpts: rewrite inpcb synchronization.
Wed, Nov 17, 7:22 PM
rrs added a comment to D33035: tcp: Add hystart++ to our cubic implementation..

Note missing yet from this diff is the ctl_output() function so you can actually enable it.

Wed, Nov 17, 4:48 PM
rrs requested review of D33035: tcp: Add hystart++ to our cubic implementation..
Wed, Nov 17, 4:46 PM
rrs requested review of D33034: tcp: TCP connection that enter the TCPS_CLOSED state should not retain BB logging or logs..
Wed, Nov 17, 3:22 PM
rrs closed D32938: tcp: Rack ack war with a mis-behaving firewall or nat with resets..
Wed, Nov 17, 2:48 PM
rrs committed R10:97e28f0f58cd: tcp: Rack ack war with a mis-behaving firewall or nat with resets. (authored by rrs).
tcp: Rack ack war with a mis-behaving firewall or nat with resets.
Wed, Nov 17, 2:48 PM

Tue, Nov 16

rrs added inline comments to D32938: tcp: Rack ack war with a mis-behaving firewall or nat with resets..
Tue, Nov 16, 5:06 PM
rrs added a comment to D32983: tcp: Fix a locking issue.

Firstly I'll note that there are other ways to achieve the proposed outcome than doing an atomic increment (e.g. deferring the increment until next time the wlock is held and the next log event is generated) but I think the atomic increment is the most straight forward option and is why I proposed it for this case. That being said, I'm only really concerned about making sure the "contract" is agreed and maintained, less so about how it's done.

As to the technical concerns about doing an atomic increment in this case, could you please help me understand the problem you think it could cause? My understanding (and I'm certainly no expert here so keen to be educated if I'm missing something) is that:

  • If we hold the inp rlock the tcpcb can't go away so tp->t_logsn will always be a valid memory address to increment
  • If we hold the inp rlock and do the increment atomically we have no possibility of variable corruption if we're racing with other rlock holders who may also try to increment
  • If we hold the inp rlock we've excluded all inp wlock code paths from doing a non-atomic increment of the serial number
  • If a wlock code path does a non-atomic increment of the serial number and then unlocks, the barrier semantics of the unlock force consistency of the serial number such that a subsequent atomic increment in an rlock path would be ensured to operate on the correct wlock-incremented serial number

Am I missing a scenario/sequence of operations where there could be a problem with the inp-rlocked atomic increment approach?

Tue, Nov 16, 4:10 PM

Mon, Nov 15

rrs added a comment to D32983: tcp: Fix a locking issue.

Michael and I have chatted about this. The only way you
can get in this state where you have an INP_RLOCK and
you are responding, is when you are getting a stand-alone
SYN. In that case due to DOS worries Gleb changed the code so that
we would get an RLOCK. Now this is a rare case unless you are
under a DOS attack. And the connection needs to be in the closed
state or its to a connection from a different port number. There
are also a couple of other cases where the SYN-CACHE unpack
fails (but unlikely to have a t_logstate set of course).

Mon, Nov 15, 8:57 PM
rrs added a comment to D32983: tcp: Fix a locking issue.

The whole point of this was that there were absolutely no logs what so ever when
tcp_respond() was used. So prior to this set of commits you *never* saw anything
when we responded to a packet (and that happens in many places in all stacks).
Now I would have to study when we use tcp_respond() with a INP_RLOCK() this
is a very narrow case I would imagine, but if you are not willing to get a INP_WLOCK()
that implies to me you are not changing the TCB... but thats what logging does.

Mon, Nov 15, 1:06 PM

Sun, Nov 14

rrs accepted D32983: tcp: Fix a locking issue.
Sun, Nov 14, 11:32 AM

Sat, Nov 13

rrs accepted D32967: tcp_timers: check for (INP_TIMEWAIT | INP_DROPPED) only once.
Sat, Nov 13, 12:39 PM
rrs accepted D32968: tcp_usr_detach: revert debugging piece from f5cf1e5f5a500..
Sat, Nov 13, 12:38 PM

Fri, Nov 12

rrs updated the test plan for D32938: tcp: Rack ack war with a mis-behaving firewall or nat with resets..
Fri, Nov 12, 11:57 AM

Thu, Nov 11

rrs added a reviewer for D32938: tcp: Rack ack war with a mis-behaving firewall or nat with resets.: tuexen.
Thu, Nov 11, 1:02 PM
rrs requested review of D32938: tcp: Rack ack war with a mis-behaving firewall or nat with resets..
Thu, Nov 11, 1:02 PM
rrs committed R10:d69538633815: Add in the commit revision to the UPDATING file for the CC changes (authored by rrs).
Add in the commit revision to the UPDATING file for the CC changes
Thu, Nov 11, 11:40 AM
rrs committed R10:26cbd0028c50: tcp: Rack may still calculate long RTT on persists probes. (authored by rrs).
tcp: Rack may still calculate long RTT on persists probes.
Thu, Nov 11, 11:38 AM
rrs closed D32897: tcp: Rack may still calculate long RTT on persists probes..
Thu, Nov 11, 11:37 AM
rrs committed R10:b8d60729deef: tcp: Congestion control cleanup. (authored by rrs).
tcp: Congestion control cleanup.
Thu, Nov 11, 11:31 AM
rrs closed D32693: tcp: Congestion control cleanup..
Thu, Nov 11, 11:31 AM

Wed, Nov 10

rrs updated the summary of D32693: tcp: Congestion control cleanup..
Wed, Nov 10, 7:03 PM
rrs updated the summary of D32693: tcp: Congestion control cleanup..
Wed, Nov 10, 6:49 PM
rrs updated the test plan for D32897: tcp: Rack may still calculate long RTT on persists probes..
Wed, Nov 10, 3:56 PM
rrs updated the diff for D32897: tcp: Rack may still calculate long RTT on persists probes..

I had some stray stuff in the previous diff (from the cc changes).

Wed, Nov 10, 3:52 PM

Nov 8 2021

rrs requested review of D32897: tcp: Rack may still calculate long RTT on persists probes..
Nov 8 2021, 8:52 PM
rrs closed D32894: tcp: Printf should be removed..
Nov 8 2021, 4:53 PM
rrs committed R10:477aeb3dd479: tcp: Printf should be removed. (authored by rrs).
tcp: Printf should be removed.
Nov 8 2021, 4:52 PM
rrs added a reviewer for D32894: tcp: Printf should be removed.: tuexen.
Nov 8 2021, 3:23 PM
rrs requested review of D32894: tcp: Printf should be removed..
Nov 8 2021, 3:23 PM

Nov 6 2021

rrs updated the diff for D32693: tcp: Congestion control cleanup..

Address Peter Lei's comments sent to me in email:

Nov 6 2021, 4:33 AM

Nov 5 2021

rrs updated the diff for D32693: tcp: Congestion control cleanup..

Update the man page for mod_cc to include a new kernel configuration section listing the new
options and such.

Nov 5 2021, 9:29 AM

Nov 4 2021

rrs updated the diff for D32693: tcp: Congestion control cleanup..

This update gets it so we blow up if we don't have a CC module in the kernel of one sort or another.

Nov 4 2021, 9:32 PM
rrs added a comment to D32693: tcp: Congestion control cleanup..

Also forgot to mention, per the call I also bumped the module rev number
and added the lock asserts we discussed.

Nov 4 2021, 6:46 PM
rrs updated the diff for D32693: tcp: Congestion control cleanup..

Per the transport call discussion I have updated things to:

  1. be able to add an option for every CC you want to build in the config
  2. be able to add a CC_DEFAULT (its required actually or compile will fail) to specify the default cc
  3. All generic's get CC_NEWRENO and CC_DEFAULT=\"newreno\"
Nov 4 2021, 6:45 PM
rrs added inline comments to D32693: tcp: Congestion control cleanup..
Nov 4 2021, 12:08 PM
rrs updated the diff for D32693: tcp: Congestion control cleanup..

Address the points Lawrence makes that I agree with.

Nov 4 2021, 11:39 AM
rrs added a comment to D32693: tcp: Congestion control cleanup..

We will talk on the telchat today at 10/1am your time.. a bit late.

Nov 4 2021, 11:24 AM

Nov 3 2021

rrs accepted D32718: blackhole(4): disable for locally originated TCP/UDP packets.
Nov 3 2021, 7:53 PM
rrs added a reviewer for D32718: blackhole(4): disable for locally originated TCP/UDP packets: rrs.
Nov 3 2021, 7:52 PM
rrs added inline comments to D32718: blackhole(4): disable for locally originated TCP/UDP packets.
Nov 3 2021, 6:10 PM
rrs accepted D31583: netinet: simplify RSS ifdef statements.

Note that Gleb is working on some changes to hpts though I don't think your changes conflict.

Nov 3 2021, 11:00 AM

Nov 2 2021

rrs updated the diff for D32693: tcp: Congestion control cleanup..

Fix a LOR as well as finish making newreno non-special. Basically we
now don't prohibit it from unloading/being a module. And we harness
the first cc module loaded/in the kernel as the default.

Nov 2 2021, 2:13 PM

Nov 1 2021

rrs updated the diff for D32693: tcp: Congestion control cleanup..

With help from Michael, I now know what gbe's comments mean. Fix all
sentences to have a nl.

Nov 1 2021, 7:10 PM

Oct 29 2021

rrs committed R10:141a53cd58cd: tcp: Rack might retransmit forever. (authored by rrs).
tcp: Rack might retransmit forever.
Oct 29 2021, 9:40 PM
rrs closed D32671: tcp: Rack might retransmit forever..
Oct 29 2021, 9:39 PM
rrs added a comment to D32671: tcp: Rack might retransmit forever..

The base stack does not maintain a sendmap. So what happens is you have
1448 byte segments .. you now must make them fit.. 1148.. so it splits each
sendmap entry into 2 segments...

Oct 29 2021, 9:33 PM
rrs added a comment to D32671: tcp: Rack might retransmit forever..

Are you saying that you only retransmit once if the peer goes away? Could you share the packetdrill script?

Oct 29 2021, 2:31 PM
rrs added inline comments to D32693: tcp: Congestion control cleanup..
Oct 29 2021, 11:42 AM
rrs closed D32717: tcp: Rack at times can miscalculate the RTT from what it thinks is a persists probe respone..
Oct 29 2021, 7:20 AM
rrs committed R10:aeda85278255: tcp: Rack at times can miscalculate the RTT from what it thinks is a persists… (authored by rrs).
tcp: Rack at times can miscalculate the RTT from what it thinks is a persists…
Oct 29 2021, 7:20 AM

Oct 28 2021

rrs updated the diff for D32717: tcp: Rack at times can miscalculate the RTT from what it thinks is a persists probe respone..

Opps there is another issue here, a double bump of rtt_shift that should not be when persists happen. Only
bump it in the timer code.

Oct 28 2021, 8:19 PM
rrs requested review of D32717: tcp: Rack at times can miscalculate the RTT from what it thinks is a persists probe respone..
Oct 28 2021, 7:55 PM
rrs added a reviewer for D32693: tcp: Congestion control cleanup.: rscheff.
Oct 28 2021, 4:58 PM
rrs updated the diff for D32693: tcp: Congestion control cleanup..

Now let's update the manual pages to reflect the current state of affairs. At this
point I think I am done, so reviewers please have a look and give feedback!

Oct 28 2021, 12:37 PM
rrs updated the diff for D32693: tcp: Congestion control cleanup..

This takes some of Lawrences private comments into account. We allow a CC without a size function to
exist and be loaded if it does not have a cb_init function. If there is a cb_init then we fail the load and
print a warning to the console as to why. I will need to next update the man page to talk about various
issues when loading and unloading modules.

Oct 28 2021, 10:55 AM
rrs updated the diff for D32693: tcp: Congestion control cleanup..

Remove the spacing/emacs gifts.

Oct 28 2021, 6:49 AM
rrs updated the diff for D32693: tcp: Congestion control cleanup..

Another fun POLA violation fixed. When unloading a module make it so that:

Oct 28 2021, 6:47 AM

Oct 27 2021

rrs updated the diff for D32693: tcp: Congestion control cleanup..

Fix up to sync up with the way we protect for unload. We have to hold the
lock through the time. This means a fun double lookup and possibly
restart if the size changed.

Oct 27 2021, 9:52 PM
rrs updated the diff for D32693: tcp: Congestion control cleanup..

Just figured out another bit of broken-ness in the existing code. When we switch
CC modules, and we are established we need to run (if we have one) the conn_init()
function otherwise things might not go so well for the CC :)

Oct 27 2021, 8:34 PM
rrs updated the diff for D32693: tcp: Congestion control cleanup..

Fix some bugs I found in the CC switching code. Next I need to test the failure and fallback with and without invariant

Oct 27 2021, 7:22 PM
rrs updated the test plan for D32671: tcp: Rack might retransmit forever..
Oct 27 2021, 5:31 PM
rrs updated the diff for D32693: tcp: Congestion control cleanup..

Cleanup a few typos in the header comments as well as lets get all
modules putting the common functions in there structure not in
the cc_mod_init() by setting a variable now that its in cc.c

Oct 27 2021, 2:10 PM
rrs requested review of D32693: tcp: Congestion control cleanup..
Oct 27 2021, 1:54 PM

Oct 26 2021

rrs added inline comments to D31716: sys: Nuke double-semicolons.
Oct 26 2021, 5:49 PM
rrs committed R10:12752978d32b: tcp: The rack stack can incorrectly have an overflow when calculating a burst… (authored by rrs).
tcp: The rack stack can incorrectly have an overflow when calculating a burst…
Oct 26 2021, 5:20 PM
rrs closed D32668: tcp: The rack stack can incorrectly have an overflow when calculating a burst delay..
Oct 26 2021, 5:20 PM
rrs abandoned D32600: tcp: DSCP Codepoint not being propagated in the rack fast path..
Oct 26 2021, 4:28 PM
rrs accepted D32655: Pass down some IP level setsockopt()s into TCP stack(s).

This method is fine, I will withdraw my fab in favor if this :)

Oct 26 2021, 4:28 PM
rrs added a reviewer for D32671: tcp: Rack might retransmit forever.: glebius.
Oct 26 2021, 2:48 PM
rrs updated the diff for D32668: tcp: The rack stack can incorrectly have an overflow when calculating a burst delay..

While where at it, there is a duplicate srtt that should be just one var at the higher scope. Lets fix
that and make the needed casts when comparing to slot. That way we never can get a
confused compiler :)

Oct 26 2021, 2:37 PM