* [PATCH net-next] tcp: remove unnecessary update for tp->write_seq in tcp_connect()
@ 2024-09-23 15:01 xin.guo
2024-09-23 15:11 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: xin.guo @ 2024-09-23 15:01 UTC (permalink / raw)
To: edumazet; +Cc: netdev, xin.guo
From: "xin.guo" <guoxin0309@gmail.com>
Commit 783237e8daf13("net-tcp: Fast Open client - sending SYN-data")
introduce tcp_connect_queue_skb() and it would overwrite tcp->write_seq,
so it is no need to update tp->write_seq before invoking
tcp_connect_queue_skb()
Signed-off-by: xin.guo <guoxin0309@gmail.com>
---
net/ipv4/tcp_output.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 4fd746b..f255c7d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -4134,7 +4134,7 @@ int tcp_connect(struct sock *sk)
if (unlikely(!buff))
return -ENOBUFS;
- tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN);
+ tcp_init_nondata_skb(buff, tp->write_seq, TCPHDR_SYN);
tcp_mstamp_refresh(tp);
tp->retrans_stamp = tcp_time_stamp_ts(tp);
tcp_connect_queue_skb(sk, buff);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] tcp: remove unnecessary update for tp->write_seq in tcp_connect()
2024-09-23 15:01 xin.guo
@ 2024-09-23 15:11 ` Eric Dumazet
0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2024-09-23 15:11 UTC (permalink / raw)
To: xin.guo; +Cc: netdev
On Mon, Sep 23, 2024 at 5:01 PM xin.guo <guoxin0309@gmail.com> wrote:
>
> From: "xin.guo" <guoxin0309@gmail.com>
>
> Commit 783237e8daf13("net-tcp: Fast Open client - sending SYN-data")
> introduce tcp_connect_queue_skb() and it would overwrite tcp->write_seq,
> so it is no need to update tp->write_seq before invoking
> tcp_connect_queue_skb()
>
> Signed-off-by: xin.guo <guoxin0309@gmail.com>
> ---
> net/ipv4/tcp_output.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 4fd746b..f255c7d 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -4134,7 +4134,7 @@ int tcp_connect(struct sock *sk)
> if (unlikely(!buff))
> return -ENOBUFS;
>
> - tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN);
> + tcp_init_nondata_skb(buff, tp->write_seq, TCPHDR_SYN);
> tcp_mstamp_refresh(tp);
> tp->retrans_stamp = tcp_time_stamp_ts(tp);
> tcp_connect_queue_skb(sk, buff);
This seems fine, but net-next is currently closed.
Documentation/process/maintainer-netdev.rst for the details.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next] tcp: remove unnecessary update for tp->write_seq in tcp_connect()
@ 2024-10-02 6:14 xin.guo
2024-10-02 7:33 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: xin.guo @ 2024-10-02 6:14 UTC (permalink / raw)
To: edumazet; +Cc: netdev, xin.guo
From: "xin.guo" <guoxin0309@gmail.com>
Commit 783237e8daf13("net-tcp: Fast Open client - sending SYN-data")
introduce tcp_connect_queue_skb() and it would overwrite tcp->write_seq,
so it is no need to update tp->write_seq before invoking
tcp_connect_queue_skb()
Signed-off-by: xin.guo <guoxin0309@gmail.com>
---
net/ipv4/tcp_output.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 4fd746b..f255c7d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -4134,7 +4134,7 @@ int tcp_connect(struct sock *sk)
if (unlikely(!buff))
return -ENOBUFS;
- tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN);
+ tcp_init_nondata_skb(buff, tp->write_seq, TCPHDR_SYN);
tcp_mstamp_refresh(tp);
tp->retrans_stamp = tcp_time_stamp_ts(tp);
tcp_connect_queue_skb(sk, buff);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] tcp: remove unnecessary update for tp->write_seq in tcp_connect()
2024-10-02 6:14 [PATCH net-next] tcp: remove unnecessary update for tp->write_seq in tcp_connect() xin.guo
@ 2024-10-02 7:33 ` Eric Dumazet
2024-10-02 9:37 ` Xin Guo
0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2024-10-02 7:33 UTC (permalink / raw)
To: xin.guo; +Cc: netdev
On Wed, Oct 2, 2024 at 8:14 AM xin.guo <guoxin0309@gmail.com> wrote:
>
> From: "xin.guo" <guoxin0309@gmail.com>
>
> Commit 783237e8daf13("net-tcp: Fast Open client - sending SYN-data")
> introduce tcp_connect_queue_skb() and it would overwrite tcp->write_seq,
> so it is no need to update tp->write_seq before invoking
> tcp_connect_queue_skb()
>
> Signed-off-by: xin.guo <guoxin0309@gmail.com>
> ---
> net/ipv4/tcp_output.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 4fd746b..f255c7d 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -4134,7 +4134,7 @@ int tcp_connect(struct sock *sk)
> if (unlikely(!buff))
> return -ENOBUFS;
>
> - tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN);
> + tcp_init_nondata_skb(buff, tp->write_seq, TCPHDR_SYN);
> tcp_mstamp_refresh(tp);
> tp->retrans_stamp = tcp_time_stamp_ts(tp);
> tcp_connect_queue_skb(sk, buff);
> --
> 1.8.3.1
>
At line 3616, there is this comment :
/* FIN eats a sequence byte, write_seq advanced by tcp_queue_skb(). */
I think you need to add a similar one. Future readers will thank you.
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 4fd746bd4d54f621601b20c3821e71370a4a615a..86dea6f022d36cb56ef5678add2bd63132eee20f
100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -4134,7 +4134,10 @@ int tcp_connect(struct sock *sk)
if (unlikely(!buff))
return -ENOBUFS;
- tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN);
+ /* SYN eats a sequence byte, write_seq updated by
+ * tcp_connect_queue_skb()
+ */
+ tcp_init_nondata_skb(buff, tp->write_seq, TCPHDR_SYN);
tcp_mstamp_refresh(tp);
tp->retrans_stamp = tcp_time_stamp_ts(tp);
tcp_connect_queue_skb(sk, buff);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] tcp: remove unnecessary update for tp->write_seq in tcp_connect()
2024-10-02 7:33 ` Eric Dumazet
@ 2024-10-02 9:37 ` Xin Guo
0 siblings, 0 replies; 5+ messages in thread
From: Xin Guo @ 2024-10-02 9:37 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
Thanks Eric, I will send v2 for this patch.
On Wed, Oct 2, 2024 at 3:33 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Oct 2, 2024 at 8:14 AM xin.guo <guoxin0309@gmail.com> wrote:
> >
> > From: "xin.guo" <guoxin0309@gmail.com>
> >
> > Commit 783237e8daf13("net-tcp: Fast Open client - sending SYN-data")
> > introduce tcp_connect_queue_skb() and it would overwrite tcp->write_seq,
> > so it is no need to update tp->write_seq before invoking
> > tcp_connect_queue_skb()
> >
> > Signed-off-by: xin.guo <guoxin0309@gmail.com>
> > ---
> > net/ipv4/tcp_output.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> > index 4fd746b..f255c7d 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -4134,7 +4134,7 @@ int tcp_connect(struct sock *sk)
> > if (unlikely(!buff))
> > return -ENOBUFS;
> >
> > - tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN);
> > + tcp_init_nondata_skb(buff, tp->write_seq, TCPHDR_SYN);
> > tcp_mstamp_refresh(tp);
> > tp->retrans_stamp = tcp_time_stamp_ts(tp);
> > tcp_connect_queue_skb(sk, buff);
> > --
> > 1.8.3.1
> >
>
> At line 3616, there is this comment :
>
> /* FIN eats a sequence byte, write_seq advanced by tcp_queue_skb(). */
>
> I think you need to add a similar one. Future readers will thank you.
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 4fd746bd4d54f621601b20c3821e71370a4a615a..86dea6f022d36cb56ef5678add2bd63132eee20f
> 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -4134,7 +4134,10 @@ int tcp_connect(struct sock *sk)
> if (unlikely(!buff))
> return -ENOBUFS;
>
> - tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN);
> + /* SYN eats a sequence byte, write_seq updated by
> + * tcp_connect_queue_skb()
> + */
> + tcp_init_nondata_skb(buff, tp->write_seq, TCPHDR_SYN);
> tcp_mstamp_refresh(tp);
> tp->retrans_stamp = tcp_time_stamp_ts(tp);
> tcp_connect_queue_skb(sk, buff);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-10-02 9:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-02 6:14 [PATCH net-next] tcp: remove unnecessary update for tp->write_seq in tcp_connect() xin.guo
2024-10-02 7:33 ` Eric Dumazet
2024-10-02 9:37 ` Xin Guo
-- strict thread matches above, loose matches on Subject: below --
2024-09-23 15:01 xin.guo
2024-09-23 15:11 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).