Differential D19071
Get TCP CDG CC working if net.inet.tcp.cc.cdg.smoothing_factor = 0 Authored by tuexen on Feb 3 2019, 1:09 PM. Tags None Referenced Files
Subscribers
Details When using the CDG TCP congestion control, setting the sysctl variable net.inet.tcp.cc.cdg.smoothing_factor to 0 should disabled the smoothing. Right now it results in a panic due to a division by zero. This patch fixes this. This problem was reported in PR 193762. Run this packetdrill script, which triggers the bug:
Diff Detail
Event TimelineComment Actions Thank you for fixing this. Minor nit: this change will leave any pending qdiffmin_q/qdiffmax_q entries from the qdiffsample UMA zone in limbo until the control block is destroyed. I think it's ok to ignore though. Comment Actions Actually, we probably need to flush the qdiffmin_q and qdiffmax_q lists in the sample_q_size == 0 branch so that a change back to non-zero smoothing_factor doesn't start operating with stale data. Comment Actions Suggest moving stailq flush loops out of cdg_cb_destroy() into an inline function, changing smoothing_factor sysctl to a SYSCTL_PROC with custom handler similar to the exp_backoff_scale sysctl, and calling flush function from both cdg_cb_destroy() and sysctl handler when smoothing_factor is set to zero Comment Actions If I read the code right, changing smoothing_factor does no impact any existing cc_var, but only future ones. So there is no need for an operation you suggested. The only reason for a SYSCTL_PROC I see, is to enforce that smoothing_factor is between 0 and INT_MAX to avoid an overflow when assigning it to sample_q_size, which is declared as an int. But that is a different issue than addressing this division by zero error. Comment Actions Agreed and apologies, I misread/miunderstood the code. I also touched base with one of the original CDG authors to sanity check and they agree this change is a good idea. |