* [PATCH net-next 0/2] tcp: tcp_v4_err() cleanups
@ 2020-05-27 2:48 Eric Dumazet
2020-05-27 2:48 ` [PATCH net-next 1/2] tcp: add tcp_ld_RTO_revert() helper Eric Dumazet
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Eric Dumazet @ 2020-05-27 2:48 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
This series is a followup of patch 239174945dac ("tcp: tcp_v4_err() icmp
skb is named icmp_skb").
Move the RFC 6069 code into a helper, and rename icmp_skb to standard
skb name so that tcp_v4_err() and tcp_v6_err() are using consistent names.
Eric Dumazet (2):
tcp: add tcp_ld_RTO_revert() helper
tcp: rename tcp_v4_err() skb parameter
net/ipv4/tcp_ipv4.c | 103 +++++++++++++++++++++++---------------------
1 file changed, 54 insertions(+), 49 deletions(-)
--
2.27.0.rc0.183.gde8f92d652-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 1/2] tcp: add tcp_ld_RTO_revert() helper
2020-05-27 2:48 [PATCH net-next 0/2] tcp: tcp_v4_err() cleanups Eric Dumazet
@ 2020-05-27 2:48 ` Eric Dumazet
2020-05-27 17:51 ` Neal Cardwell
2020-05-27 2:48 ` [PATCH net-next 2/2] tcp: rename tcp_v4_err() skb parameter Eric Dumazet
2020-05-27 21:57 ` [PATCH net-next 0/2] tcp: tcp_v4_err() cleanups David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2020-05-27 2:48 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
RFC 6069 logic has been implemented for IPv4 only so far,
right in the middle of tcp_v4_err() and was error prone.
Move this code to one helper, to make tcp_v4_err() more
readable and to eventually expand RFC 6069 to IPv6 in
the future.
Also perform sock_owned_by_user() check a bit sooner.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_ipv4.c | 85 ++++++++++++++++++++++++---------------------
1 file changed, 45 insertions(+), 40 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 3ac0a7523923e0f1d959dfa65cf2b73bd6a4af15..8b257a92c98ffdb4618b8cde0937740ad5fe2e64 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -403,6 +403,45 @@ void tcp_req_err(struct sock *sk, u32 seq, bool abort)
}
EXPORT_SYMBOL(tcp_req_err);
+/* TCP-LD (RFC 6069) logic */
+static void tcp_ld_RTO_revert(struct sock *sk, u32 seq)
+{
+ struct inet_connection_sock *icsk = inet_csk(sk);
+ struct tcp_sock *tp = tcp_sk(sk);
+ struct sk_buff *skb;
+ s32 remaining;
+ u32 delta_us;
+
+ if (sock_owned_by_user(sk))
+ return;
+
+ if (seq != tp->snd_una || !icsk->icsk_retransmits ||
+ !icsk->icsk_backoff)
+ return;
+
+ skb = tcp_rtx_queue_head(sk);
+ if (WARN_ON_ONCE(!skb))
+ return;
+
+ icsk->icsk_backoff--;
+ icsk->icsk_rto = tp->srtt_us ? __tcp_set_rto(tp) : TCP_TIMEOUT_INIT;
+ icsk->icsk_rto = inet_csk_rto_backoff(icsk, TCP_RTO_MAX);
+
+ tcp_mstamp_refresh(tp);
+ delta_us = (u32)(tp->tcp_mstamp - tcp_skb_timestamp_us(skb));
+ remaining = icsk->icsk_rto - usecs_to_jiffies(delta_us);
+
+ if (remaining > 0) {
+ inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
+ remaining, TCP_RTO_MAX);
+ } else {
+ /* RTO revert clocked out retransmission.
+ * Will retransmit now.
+ */
+ tcp_retransmit_timer(sk);
+ }
+}
+
/*
* This routine is called by the ICMP module when it gets some
* sort of error condition. If err < 0 then the socket should
@@ -423,17 +462,13 @@ int tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
{
const struct iphdr *iph = (const struct iphdr *)icmp_skb->data;
struct tcphdr *th = (struct tcphdr *)(icmp_skb->data + (iph->ihl << 2));
- struct inet_connection_sock *icsk;
struct tcp_sock *tp;
struct inet_sock *inet;
const int type = icmp_hdr(icmp_skb)->type;
const int code = icmp_hdr(icmp_skb)->code;
struct sock *sk;
- struct sk_buff *skb;
struct request_sock *fastopen;
u32 seq, snd_una;
- s32 remaining;
- u32 delta_us;
int err;
struct net *net = dev_net(icmp_skb->dev);
@@ -476,7 +511,6 @@ int tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
goto out;
}
- icsk = inet_csk(sk);
tp = tcp_sk(sk);
/* XXX (TFO) - tp->snd_una should be ISN (tcp_create_openreq_child() */
fastopen = rcu_dereference(tp->fastopen_rsk);
@@ -521,41 +555,12 @@ int tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
}
err = icmp_err_convert[code].errno;
- /* check if icmp_skb allows revert of backoff
- * (see draft-zimmermann-tcp-lcd) */
- if (code != ICMP_NET_UNREACH && code != ICMP_HOST_UNREACH)
- break;
- if (seq != tp->snd_una || !icsk->icsk_retransmits ||
- !icsk->icsk_backoff || fastopen)
- break;
-
- if (sock_owned_by_user(sk))
- break;
-
- skb = tcp_rtx_queue_head(sk);
- if (WARN_ON_ONCE(!skb))
- break;
-
- icsk->icsk_backoff--;
- icsk->icsk_rto = tp->srtt_us ? __tcp_set_rto(tp) :
- TCP_TIMEOUT_INIT;
- icsk->icsk_rto = inet_csk_rto_backoff(icsk, TCP_RTO_MAX);
-
-
- tcp_mstamp_refresh(tp);
- delta_us = (u32)(tp->tcp_mstamp - tcp_skb_timestamp_us(skb));
- remaining = icsk->icsk_rto -
- usecs_to_jiffies(delta_us);
-
- if (remaining > 0) {
- inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
- remaining, TCP_RTO_MAX);
- } else {
- /* RTO revert clocked out retransmission.
- * Will retransmit now */
- tcp_retransmit_timer(sk);
- }
-
+ /* check if this ICMP message allows revert of backoff.
+ * (see RFC 6069)
+ */
+ if (!fastopen &&
+ (code == ICMP_NET_UNREACH || code == ICMP_HOST_UNREACH))
+ tcp_ld_RTO_revert(sk, seq);
break;
case ICMP_TIME_EXCEEDED:
err = EHOSTUNREACH;
--
2.27.0.rc0.183.gde8f92d652-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next 2/2] tcp: rename tcp_v4_err() skb parameter
2020-05-27 2:48 [PATCH net-next 0/2] tcp: tcp_v4_err() cleanups Eric Dumazet
2020-05-27 2:48 ` [PATCH net-next 1/2] tcp: add tcp_ld_RTO_revert() helper Eric Dumazet
@ 2020-05-27 2:48 ` Eric Dumazet
2020-05-27 17:44 ` Neal Cardwell
2020-05-27 21:57 ` [PATCH net-next 0/2] tcp: tcp_v4_err() cleanups David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2020-05-27 2:48 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
This essentially reverts 4d1a2d9ec1c1 ("Revert Backoff [v3]:
Rename skb to icmp_skb in tcp_v4_err()")
Now we have tcp_ld_RTO_revert() helper, we can use the usual
name for sk_buff parameter, so that tcp_v4_err() and
tcp_v6_err() use similar names.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_ipv4.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 8b257a92c98ffdb4618b8cde0937740ad5fe2e64..3a1e2becb1e8d1e0513e87bdfc0e1d5769ffc8e8 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -458,23 +458,23 @@ static void tcp_ld_RTO_revert(struct sock *sk, u32 seq)
*
*/
-int tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
+int tcp_v4_err(struct sk_buff *skb, u32 info)
{
- const struct iphdr *iph = (const struct iphdr *)icmp_skb->data;
- struct tcphdr *th = (struct tcphdr *)(icmp_skb->data + (iph->ihl << 2));
+ const struct iphdr *iph = (const struct iphdr *)skb->data;
+ struct tcphdr *th = (struct tcphdr *)(skb->data + (iph->ihl << 2));
struct tcp_sock *tp;
struct inet_sock *inet;
- const int type = icmp_hdr(icmp_skb)->type;
- const int code = icmp_hdr(icmp_skb)->code;
+ const int type = icmp_hdr(skb)->type;
+ const int code = icmp_hdr(skb)->code;
struct sock *sk;
struct request_sock *fastopen;
u32 seq, snd_una;
int err;
- struct net *net = dev_net(icmp_skb->dev);
+ struct net *net = dev_net(skb->dev);
sk = __inet_lookup_established(net, &tcp_hashinfo, iph->daddr,
th->dest, iph->saddr, ntohs(th->source),
- inet_iif(icmp_skb), 0);
+ inet_iif(skb), 0);
if (!sk) {
__ICMP_INC_STATS(net, ICMP_MIB_INERRORS);
return -ENOENT;
@@ -524,7 +524,7 @@ int tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
switch (type) {
case ICMP_REDIRECT:
if (!sock_owned_by_user(sk))
- do_redirect(icmp_skb, sk);
+ do_redirect(skb, sk);
goto out;
case ICMP_SOURCE_QUENCH:
/* Just silently ignore these. */
@@ -578,7 +578,7 @@ int tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
if (fastopen && !fastopen->sk)
break;
- ip_icmp_error(sk, icmp_skb, err, th->dest, info, (u8 *)th);
+ ip_icmp_error(sk, skb, err, th->dest, info, (u8 *)th);
if (!sock_owned_by_user(sk)) {
sk->sk_err = err;
--
2.27.0.rc0.183.gde8f92d652-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] tcp: rename tcp_v4_err() skb parameter
2020-05-27 2:48 ` [PATCH net-next 2/2] tcp: rename tcp_v4_err() skb parameter Eric Dumazet
@ 2020-05-27 17:44 ` Neal Cardwell
0 siblings, 0 replies; 6+ messages in thread
From: Neal Cardwell @ 2020-05-27 17:44 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet
On Tue, May 26, 2020 at 10:49 PM Eric Dumazet <edumazet@google.com> wrote:
>
> This essentially reverts 4d1a2d9ec1c1 ("Revert Backoff [v3]:
> Rename skb to icmp_skb in tcp_v4_err()")
>
> Now we have tcp_ld_RTO_revert() helper, we can use the usual
> name for sk_buff parameter, so that tcp_v4_err() and
> tcp_v6_err() use similar names.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] tcp: add tcp_ld_RTO_revert() helper
2020-05-27 2:48 ` [PATCH net-next 1/2] tcp: add tcp_ld_RTO_revert() helper Eric Dumazet
@ 2020-05-27 17:51 ` Neal Cardwell
0 siblings, 0 replies; 6+ messages in thread
From: Neal Cardwell @ 2020-05-27 17:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet
On Tue, May 26, 2020 at 10:49 PM Eric Dumazet <edumazet@google.com> wrote:
>
> RFC 6069 logic has been implemented for IPv4 only so far,
> right in the middle of tcp_v4_err() and was error prone.
>
> Move this code to one helper, to make tcp_v4_err() more
> readable and to eventually expand RFC 6069 to IPv6 in
> the future.
>
> Also perform sock_owned_by_user() check a bit sooner.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
Acked-by: Neal Cardwell <ncardwell@google.com>
Tested-by: Neal Cardwell <ncardwell@google.com>
Nice clean-up. Thanks, Eric! It will be great to have IPv6 RFC 6069
support as well.
thanks,
neal
ps: Tested with the packetdrill script below earlier this morning to
verify that IPv4 RFC 6069 works before and after Eric's patch. Eric
independently wrote a nicer test this morning.
// Establish a connection.
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 32792 <mss 1460,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,wscale 8>
+.010 < . 1:1(0) ack 1 win 257
+0 accept(3, ..., ...) = 4
// Send 10 data segments.
+0 write(4, ..., 14600) = 14600
+0 > P. 1:14601(14600) ack 1
// ICMP says that the network is unreachable
+.010 < icmp unreachable net_unreachable [1:1461(1460)]
// RTO retransmit.
+.216 > . 1:1461(1460) ack 1
+.010 < icmp unreachable net_unreachable [1:1461(1460)]
// RTO retransmit, without exponential backoff.
+.216 > . 1:1461(1460) ack 1
+.010 < icmp unreachable net_unreachable [1:1461(1460)]
// RTO retransmit, without exponential backoff.
+.216 > . 1:1461(1460) ack 1
+.010 < icmp unreachable net_unreachable [1:1461(1460)]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 0/2] tcp: tcp_v4_err() cleanups
2020-05-27 2:48 [PATCH net-next 0/2] tcp: tcp_v4_err() cleanups Eric Dumazet
2020-05-27 2:48 ` [PATCH net-next 1/2] tcp: add tcp_ld_RTO_revert() helper Eric Dumazet
2020-05-27 2:48 ` [PATCH net-next 2/2] tcp: rename tcp_v4_err() skb parameter Eric Dumazet
@ 2020-05-27 21:57 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-05-27 21:57 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet
From: Eric Dumazet <edumazet@google.com>
Date: Tue, 26 May 2020 19:48:48 -0700
> This series is a followup of patch 239174945dac ("tcp: tcp_v4_err() icmp
> skb is named icmp_skb").
>
> Move the RFC 6069 code into a helper, and rename icmp_skb to standard
> skb name so that tcp_v4_err() and tcp_v6_err() are using consistent names.
Series applied, thanks Eric.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-27 21:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27 2:48 [PATCH net-next 0/2] tcp: tcp_v4_err() cleanups Eric Dumazet
2020-05-27 2:48 ` [PATCH net-next 1/2] tcp: add tcp_ld_RTO_revert() helper Eric Dumazet
2020-05-27 17:51 ` Neal Cardwell
2020-05-27 2:48 ` [PATCH net-next 2/2] tcp: rename tcp_v4_err() skb parameter Eric Dumazet
2020-05-27 17:44 ` Neal Cardwell
2020-05-27 21:57 ` [PATCH net-next 0/2] tcp: tcp_v4_err() cleanups David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.