On Sat, Jun 6, 2015 at 7:01 PM, Yuchung Cheng <ych...@google.com> wrote:
> On Fri, Jun 5, 2015 at 1:23 PM, Kenneth Klette Jonassen
> <kenne...@ifi.uio.no> wrote:
>>> nice patch. I would like to review it more thoroughly but I have some
>>> quick comments.
>>>
>>> given that cdg didn't include hystart. it'd be nice to have a module knob to
>>> enable and disable hystart for experiments.
>>
>> We could include a knob to disable PRR for delay backoffs as well.
>> Neither are/were available in FreeBSD.
> Seems to belong to a different bigger change to make PRR option for CC
> in general? or is it easy to do in CDG locally?
Yes, I agree. A global option is probably the better alternative. We
could use it to experiment with PRR's effects in other CCs etc.
>
> Understood. My previous comment was not clear: while the shadow_wnd
> can be over the clamp, it's fine because we always clamp the cwnd in
> tcp_reno_cong_avoid()?
>
> In the future if we want to dump shadow_wnd via TCP_CC_INFO, it'll
> be good to know the real value, not the clamped one set by apps.
Ah ok. Yes, we do not need to clamp shadow_wnd in cong_avoid(). We can
change it to only deal with integer overflow instead.
>>>> + /* If non-ECN: */
>>>> + if (tp->prior_ssthresh) {
>>> no need to check prior_ssthresh here b/c it's checked in
>>> tcp_undo_cwnd_reduction() already.
>>
>> If we get ECE from the receiver (ECN signal), then cwnd undo is disabled.
>>
>> The above check ensures that the loss tolerance heuristic can only be
>> used if cwnd undo is possible.
>>
>> As an alternative, perhaps ssthresh could be extended with a second
>> parameter to indicate why it was invoked:
>> ssthresh(sk, TCP_SIGNAL_LOSS)
>> ssthresh(sk, TCP_SIGNAL_ECN)
>> ssthresh(sk, TCP_SIGNAL_PRIVATE)
>>
>> Then tcp_cdg_ssthresh could do:
>> iff signal == TCP_SIGNAL_LOSS && use_tolerance, use loss tolerance heuristic.
>> iff signal == TCP_SIGNAL_PRIVATE, calculate delay backoff cwnd
> maybe passing the ack "flag" is good enough?
>
On second thought, this touches a lot of code for a small benefit.
Is it okay if we just do:
bool is_ecn = (tp->prior_ssthresh == 0);
?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
