Message ID | 7ad85d0d66f3dcbebbd9d2ee6896cfedd1620812.1576499809.git.pabeni@redhat.com |
---|---|
State | Accepted, archived |
Delegated to: | Matthieu Baerts |
Headers | show |
Series | Squash-to: tcp: Check for filled TCP option space before SACK | expand |
Hi Paolo, On 16/12/2019 13:37, Paolo Abeni wrote: > Less invasive checks, so that the number of branchs when hitting > the sack code path decreses compared to the current vanilla tree. > -- > Some addictional check in tcp_established_options() is needed, otherwise > the tcp header will be corrupted when mptcp_established_options() fully > consumes the TCP option space. This condition can be reached without > MPTCP, so not sure if the result patch is still worthy for -net ?!? Thank you for the patch! It looks good to me! I guess we will need to update the commit message for "tcp: Check for filled TCP option space before SACK" :) Cheers, Matth
Hi Paolo, On 16/12/2019 14:05, Matthieu Baerts wrote: > Hi Paolo, > > On 16/12/2019 13:37, Paolo Abeni wrote: >> Less invasive checks, so that the number of branchs when hitting >> the sack code path decreses compared to the current vanilla tree. >> -- >> Some addictional check in tcp_established_options() is needed, otherwise >> the tcp header will be corrupted when mptcp_established_options() fully >> consumes the TCP option space. This condition can be reached without >> MPTCP, so not sure if the result patch is still worthy for -net ?!? > > > Thank you for the patch! > > It looks good to me! - b474fba77e6f: "squashed" in "tcp: Check for filled TCP option space before SACK" - 5920a223da80: "Signed-off-by" + "Co-developed-by" - 7ed2a1728574: conflict in t/mptcp-Handle-MP_CAPABLE-options-for-outgoing-connections - b8aff0a1a12d..66880827b48f: result > I guess we will need to update the commit message for "tcp: Check for > filled TCP option space before SACK" :) What can I put here? :) v1: - less invasive checks (Eric Dumazet) Should we also mention that with MPTCP and TS, we can use all the option space? I will launch the tests + export soon. Cheers, Matt
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 710ab45badfa..e797ca6c6d7d 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -748,19 +748,20 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb size += TCPOLEN_TSTAMP_ALIGNED; } - if (size + TCPOLEN_SACK_BASE_ALIGNED >= MAX_TCP_OPTION_SPACE) - return size; - eff_sacks = tp->rx_opt.num_sacks + tp->rx_opt.dsack; if (unlikely(eff_sacks)) { const unsigned int remaining = MAX_TCP_OPTION_SPACE - size; + if (unlikely(remaining < TCPOLEN_SACK_BASE_ALIGNED + + TCPOLEN_SACK_PERBLOCK)) + return size; + opts->num_sack_blocks = min_t(unsigned int, eff_sacks, (remaining - TCPOLEN_SACK_BASE_ALIGNED) / TCPOLEN_SACK_PERBLOCK); - if (likely(opts->num_sack_blocks)) - size += TCPOLEN_SACK_BASE_ALIGNED + - opts->num_sack_blocks * TCPOLEN_SACK_PERBLOCK; + + size += TCPOLEN_SACK_BASE_ALIGNED + + opts->num_sack_blocks * TCPOLEN_SACK_PERBLOCK; } return size;