-
Notifications
You must be signed in to change notification settings - Fork 92
[Deepin-Kernel-SIG] [linux 6.6-y] [Upstream] tcp: save flowlabel and use for receiver repathing #901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Deepin-Kernel-SIG] [linux 6.6-y] [Upstream] tcp: save flowlabel and use for receiver repathing #901
Conversation
mainline inclusion from mainline-v6.7-rc1 category: performance In order to better estimate whether a data packet has been retransmitted or is the result of a TLP, we save the last received ipv6 flowlabel. To make space for this field we resize the "ato" field in inet_connection_sock as the current value of TCP_DELACK_MAX can be fully contained in 8 bits and add a compile_time_assert ensuring this field is the required size. v2: addressed kernel bot feedback about dccp_delack_timer() v3: addressed build error introduced by commit bbf80d7 ("tcp: derive delack_max from rto_min") Signed-off-by: David Morley <[email protected]> Signed-off-by: Neal Cardwell <[email protected]> Signed-off-by: Yuchung Cheng <[email protected]> Tested-by: David Morley <[email protected]> Reviewed-by: Eric Dumazet <[email protected]> Signed-off-by: Paolo Abeni <[email protected]> (cherry picked from commit 95b9a87) Signed-off-by: Wentao Guan <[email protected]>
mainline inclusion from mainline-v6.7-rc1 category: performance This commit changes the data receiver repath behavior to occur after receiving a single duplicate. This can help recover ACK connectivity quicker if a TLP was sent along a nonworking path. For instance, consider the case where we have an initially nonworking forward path and reverse path and subsequently switch to only working forward paths. Before this patch we would have the following behavior. +---------+--------+--------+----------+----------+----------+ | Event | For FL | Rev FL | FP Works | RP Works | Data Del | +---------+--------+--------+----------+----------+----------+ | Initial | A | 1 | N | N | 0 | +---------+--------+--------+----------+----------+----------+ | TLP | A | 1 | N | N | 0 | +---------+--------+--------+----------+----------+----------+ | RTO 1 | B | 1 | Y | N | 1 | +---------+--------+--------+----------+----------+----------+ | RTO 2 | C | 1 | Y | N | 2 | +---------+--------+--------+----------+----------+----------+ | RTO 3 | D | 2 | Y | Y | 3 | +---------+--------+--------+----------+----------+----------+ This patch gets rid of at least RTO 3, avoiding additional unnecessary repaths of a working forward path to a (potentially) nonworking one. In addition, this commit changes the behavior to avoid repathing upon rx of duplicate data if the local endpoint is in CA_Loss (in which case the RTOs will already be changing the outgoing flowlabel). Signed-off-by: David Morley <[email protected]> Signed-off-by: Neal Cardwell <[email protected]> Signed-off-by: Yuchung Cheng <[email protected]> Tested-by: David Morley <[email protected]> Reviewed-by: Eric Dumazet <[email protected]> Signed-off-by: Paolo Abeni <[email protected]> (cherry picked from commit 9394630) Signed-off-by: Wentao Guan <[email protected]>
Reviewer's GuideThis series extends TCP’s IPv6 support by storing the last received IPv6 flowlabel in the connection socket and using it to detect RTO-triggered retransmits, enabling the receiver to opportunistically rehash (repath) outgoing packets. Supporting changes include introducing a new bitfield for lrcv_flowlabel, injecting a helper to update that field on packet receipt, gating repathing on a flowlabel change, and refactoring ATO handling to use min_t and enforce bitfield limits. Sequence diagram for flowlabel-based receiver repathing decisionsequenceDiagram
participant Receiver as TCP Receiver
participant tcp_save_lrcv_flowlabel as tcp_save_lrcv_flowlabel
participant tcp_rcv_spurious_retrans as tcp_rcv_spurious_retrans
participant inet_connection_sock as inet_connection_sock
participant skb as sk_buff
Receiver->>tcp_save_lrcv_flowlabel: On packet receive
tcp_save_lrcv_flowlabel->>inet_connection_sock: Update lrcv_flowlabel if IPv6
Receiver->>tcp_rcv_spurious_retrans: Detect duplicate data segment
tcp_rcv_spurious_retrans->>skb: Check protocol and flowlabel
tcp_rcv_spurious_retrans->>inet_connection_sock: Compare lrcv_flowlabel with skb flowlabel
alt Flowlabel changed and not in TCP_CA_Loss
tcp_rcv_spurious_retrans->>Receiver: Trigger repathing (rehash socket)
end
tcp_rcv_spurious_retrans->>tcp_save_lrcv_flowlabel: Save last flowlabel after spurious retrans
ER diagram for inet_connection_sock and flowlabel trackingerDiagram
INET_CONNECTION_SOCK {
__u32 lrcv_flowlabel
}
INET_CONNECTION_SOCK ||--o{ SK_BUFF : receives
SK_BUFF {
protocol
flowlabel
}
Class diagram for updated inet_connection_sock structureclassDiagram
class inet_connection_sock {
+__u8 quick
+__u8 pingpong
+__u8 retry
+__u32 ato:ATO_BITS
+__u32 lrcv_flowlabel:20
+__u32 unused:4
+unsigned long timeout
+__u32 lrcvtime
+__u16 last_seg_size
}
Class diagram for tcp_save_lrcv_flowlabel helper functionclassDiagram
class tcp_save_lrcv_flowlabel {
+void tcp_save_lrcv_flowlabel(struct sock *sk, const struct sk_buff *skb)
}
class inet_connection_sock
class sk_buff
tcp_save_lrcv_flowlabel --> inet_connection_sock : updates lrcv_flowlabel
tcp_save_lrcv_flowlabel --> sk_buff : reads protocol and flowlabel
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
deepin pr auto review代码审查意见:
总体来说,这些代码改动都是合理的,可以提高代码的可读性、可维护性和性能。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This series enhances TCP by recording the last received IPv6 flowlabel to better detect spurious retransmissions and repath the socket accordingly, and it fixes related build errors in delayed-ACK timer code.
- Store and update a 20-bit
lrcv_flowlabel
in the connection socket state. - Trigger TX hash rebuild on duplicate data with a new flowlabel in
tcp_rcv_spurious_retrans
. - Replace
min(...)
withmin_t(u32, ...)
and add astatic_assert
for ATO bit-field sizing.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
include/net/inet_connection_sock.h | Convert ato to an 8-bit field and add a 20-bit flowlabel. |
include/net/tcp.h | Add static_assert to validate ATO bit-field width. |
net/ipv4/tcp_timer.c | Use min_t(u32, ...) for ATO inflation. |
net/dccp/timer.c | Use min_t(u32, ...) in DCCP delayed-ACK timer. |
net/ipv4/tcp_input.c | Implement tcp_save_lrcv_flowlabel and repathing logic. |
Comments suppressed due to low confidence (3)
include/net/inet_connection_sock.h:122
- [nitpick] Add a comment above or here describing the purpose of
lrcv_flowlabel
(e.g., 'Tracks the last 20-bit IPv6 flowlabel seen by the receiver') and its valid range to aid future maintainers.
lrcv_flowlabel:20, /* last received ipv6 flowlabel */
net/ipv4/tcp_input.c:808
- Introduce tests (unit or integration) to cover the IPv6 flowlabel tracking and repathing logic, ensuring correct behavior under both IPv4 and IPv6.
static void tcp_save_lrcv_flowlabel(struct sock *sk, const struct sk_buff *skb)
include/net/tcp.h:136
ATO_BITS
is defined ininet_connection_sock.h
and may not be visible here; include the header or move the macro definition so thatATO_BITS
is always defined before use.
static_assert((1 << ATO_BITS) > TCP_DELACK_MAX);
*/ | ||
if (TCP_SKB_CB(skb)->seq == tcp_sk(sk)->duplicate_sack[0].start_seq && | ||
#if IS_ENABLED(CONFIG_IPV6) | ||
if (inet_csk(sk)->icsk_ca_state != TCP_CA_Loss && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original check against duplicate_sack[0].start_seq
was removed; without it, repathing could trigger on non-retransmitted segments. Reintroduce the sequence-equality condition to restrict repathing to actual duplicates.
Copilot uses AI. Check for mistakes.
sk_rethink_txhash(sk)) | ||
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPDUPLICATEDATAREHASH); | ||
|
||
/* Save last flowlabel after a spurious retrans. */ | ||
tcp_save_lrcv_flowlabel(sk, skb); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Wrap the if
block and this call in braces to make the conditional scope explicit and avoid accidental unconditional execution after future edits.
sk_rethink_txhash(sk)) | |
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPDUPLICATEDATAREHASH); | |
/* Save last flowlabel after a spurious retrans. */ | |
tcp_save_lrcv_flowlabel(sk, skb); | |
sk_rethink_txhash(sk)) { | |
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPDUPLICATEDATAREHASH); | |
} | |
/* Save last flowlabel after a spurious retrans. */ | |
{ | |
tcp_save_lrcv_flowlabel(sk, skb); | |
} |
Copilot uses AI. Check for mistakes.
struct inet_connection_sock *icsk = inet_csk(sk); | ||
|
||
if (skb->protocol == htons(ETH_P_IPV6)) | ||
icsk->icsk_ack.lrcv_flowlabel = ntohl(ip6_flowlabel(ipv6_hdr(skb))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Mask the result to 20 bits (e.g., & 0xFFFFF
) when assigning to the bit-field to make the truncation explicit and prevent unintended high-bit data.
icsk->icsk_ack.lrcv_flowlabel = ntohl(ip6_flowlabel(ipv6_hdr(skb))); | |
icsk->icsk_ack.lrcv_flowlabel = ntohl(ip6_flowlabel(ipv6_hdr(skb))) & 0xFFFFF; |
Copilot uses AI. Check for mistakes.
349c642
into
deepin-community:linux-6.6.y
This patch series stores the last received ipv6 flowlabel. This last
received flowlabel is then used to help decide whether a packet is
likely an RTO retransmit or the result of a TLP. This new information
is used to better inform the flowlabel change decision for data
receivers.
v2: addressed kernel bot feedback about dccp_delack_timer()
v3: addressed build error introduced by commit bbf80d7 ("tcp:
derive delack_max from rto_min")
David Morley (2):
tcp: record last received ipv6 flowlabel
tcp: change data receiver flowlabel after one dup
include/net/inet_connection_sock.h | 5 ++++-
include/net/tcp.h | 2 ++
net/dccp/timer.c | 4 ++--
net/ipv4/tcp_input.c | 29 ++++++++++++++++++++++++++---
net/ipv4/tcp_timer.c | 2 +-
5 files changed, 35 insertions(+), 7 deletions(-)
--
2.42.0.582.g8ccd20d70d-goog
Summary by Sourcery
Track and store the last received IPv6 flowlabel and use it to detect and repath on spurious TCP retransmissions triggered by remote timeouts.
New Features:
Bug Fixes: