netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] tcp: add tcp_warn_once() common helper
@ 2024-10-20 14:50 Jason Xing
  2024-10-20 14:50 ` [PATCH net-next 1/2] tcp: add a common helper to debug the underlying issue Jason Xing
  2024-10-20 14:50 ` [PATCH net-next 2/2] tcp: add more warn of socket in tcp_send_loss_probe() Jason Xing
  0 siblings, 2 replies; 9+ messages in thread
From: Jason Xing @ 2024-10-20 14:50 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, dsahern, ncardwell; +Cc: netdev, Jason Xing

From: Jason Xing <kernelxing@tencent.com>

Paolo Abeni suggested we can introduce a new helper to cover more cases
in the future for better debug.

Jason Xing (2):
  tcp: add a common helper to debug the underlying issue
  tcp: add more warn of socket in tcp_send_loss_probe()

 include/net/tcp.h     | 31 ++++++++++++++++++++-----------
 net/ipv4/tcp_output.c |  4 +---
 2 files changed, 21 insertions(+), 14 deletions(-)

-- 
2.37.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net-next 1/2] tcp: add a common helper to debug the underlying issue
  2024-10-20 14:50 [PATCH net-next 0/2] tcp: add tcp_warn_once() common helper Jason Xing
@ 2024-10-20 14:50 ` Jason Xing
  2024-10-20 16:18   ` David Ahern
  2024-10-20 14:50 ` [PATCH net-next 2/2] tcp: add more warn of socket in tcp_send_loss_probe() Jason Xing
  1 sibling, 1 reply; 9+ messages in thread
From: Jason Xing @ 2024-10-20 14:50 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, dsahern, ncardwell; +Cc: netdev, Jason Xing

From: Jason Xing <kernelxing@tencent.com>

Following the commit c8770db2d544 ("tcp: check skb is non-NULL
in tcp_rto_delta_us()"), we decided to add a helper so that it's
easier to get verbose warning on either cases.

Link: https://lore.kernel.org/all/5632e043-bdba-4d75-bc7e-bf58014492fd@redhat.com/
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
Cc: Neal Cardwell <ncardwell@google.com>
---
 include/net/tcp.h | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 739a9fb83d0c..cac7bbff61ce 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2430,6 +2430,22 @@ void tcp_plb_update_state(const struct sock *sk, struct tcp_plb_state *plb,
 void tcp_plb_check_rehash(struct sock *sk, struct tcp_plb_state *plb);
 void tcp_plb_update_state_upon_rto(struct sock *sk, struct tcp_plb_state *plb);
 
+static inline void tcp_warn_once(const struct sock *sk, bool cond, const char *str)
+{
+	WARN_ONCE(cond,
+		  "%s"
+		  "out:%u sacked:%u lost:%u retrans:%u "
+		  "tlp_high_seq:%u sk_state:%u ca_state:%u "
+		  "advmss:%u mss_cache:%u pmtu:%u\n",
+		  str,
+		  tcp_sk(sk)->packets_out, tcp_sk(sk)->sacked_out,
+		  tcp_sk(sk)->lost_out, tcp_sk(sk)->retrans_out,
+		  tcp_sk(sk)->tlp_high_seq, sk->sk_state,
+		  inet_csk(sk)->icsk_ca_state,
+		  tcp_sk(sk)->advmss, tcp_sk(sk)->mss_cache,
+		  inet_csk(sk)->icsk_pmtu_cookie);
+}
+
 /* At how many usecs into the future should the RTO fire? */
 static inline s64 tcp_rto_delta_us(const struct sock *sk)
 {
@@ -2441,17 +2457,7 @@ static inline s64 tcp_rto_delta_us(const struct sock *sk)
 
 		return rto_time_stamp_us - tcp_sk(sk)->tcp_mstamp;
 	} else {
-		WARN_ONCE(1,
-			"rtx queue empty: "
-			"out:%u sacked:%u lost:%u retrans:%u "
-			"tlp_high_seq:%u sk_state:%u ca_state:%u "
-			"advmss:%u mss_cache:%u pmtu:%u\n",
-			tcp_sk(sk)->packets_out, tcp_sk(sk)->sacked_out,
-			tcp_sk(sk)->lost_out, tcp_sk(sk)->retrans_out,
-			tcp_sk(sk)->tlp_high_seq, sk->sk_state,
-			inet_csk(sk)->icsk_ca_state,
-			tcp_sk(sk)->advmss, tcp_sk(sk)->mss_cache,
-			inet_csk(sk)->icsk_pmtu_cookie);
+		tcp_warn_once(sk, 1, "rtx queue empty: ");
 		return jiffies_to_usecs(rto);
 	}
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net-next 2/2] tcp: add more warn of socket in tcp_send_loss_probe()
  2024-10-20 14:50 [PATCH net-next 0/2] tcp: add tcp_warn_once() common helper Jason Xing
  2024-10-20 14:50 ` [PATCH net-next 1/2] tcp: add a common helper to debug the underlying issue Jason Xing
@ 2024-10-20 14:50 ` Jason Xing
  2024-10-21  2:37   ` Jason Xing
  2024-10-21  5:52   ` kernel test robot
  1 sibling, 2 replies; 9+ messages in thread
From: Jason Xing @ 2024-10-20 14:50 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, dsahern, ncardwell; +Cc: netdev, Jason Xing

From: Jason Xing <kernelxing@tencent.com>

Add two fields to print in the helper which here covers tcp_send_loss_probe().

Link: https://lore.kernel.org/all/5632e043-bdba-4d75-bc7e-bf58014492fd@redhat.com/
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
Cc: Neal Cardwell <ncardwell@google.com>
---
 include/net/tcp.h     | 5 ++++-
 net/ipv4/tcp_output.c | 4 +---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index cac7bbff61ce..68eb03758950 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2434,14 +2434,17 @@ static inline void tcp_warn_once(const struct sock *sk, bool cond, const char *s
 {
 	WARN_ONCE(cond,
 		  "%s"
+		  "cwnd:%u "
 		  "out:%u sacked:%u lost:%u retrans:%u "
 		  "tlp_high_seq:%u sk_state:%u ca_state:%u "
-		  "advmss:%u mss_cache:%u pmtu:%u\n",
+		  "mss:%u advmss:%u mss_cache:%u pmtu:%u\n",
 		  str,
+		  tcp_snd_cwnd(tcp_sk(sk)),
 		  tcp_sk(sk)->packets_out, tcp_sk(sk)->sacked_out,
 		  tcp_sk(sk)->lost_out, tcp_sk(sk)->retrans_out,
 		  tcp_sk(sk)->tlp_high_seq, sk->sk_state,
 		  inet_csk(sk)->icsk_ca_state,
+		  tcp_current_mss((struct sock *)sk),
 		  tcp_sk(sk)->advmss, tcp_sk(sk)->mss_cache,
 		  inet_csk(sk)->icsk_pmtu_cookie);
 }
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 054244ce5117..295bc0741772 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2954,9 +2954,7 @@ void tcp_send_loss_probe(struct sock *sk)
 	}
 	skb = skb_rb_last(&sk->tcp_rtx_queue);
 	if (unlikely(!skb)) {
-		WARN_ONCE(tp->packets_out,
-			  "invalid inflight: %u state %u cwnd %u mss %d\n",
-			  tp->packets_out, sk->sk_state, tcp_snd_cwnd(tp), mss);
+		tcp_warn_once(sk, tp->packets_out, NULL);
 		smp_store_release(&inet_csk(sk)->icsk_pending, 0);
 		return;
 	}
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 1/2] tcp: add a common helper to debug the underlying issue
  2024-10-20 14:50 ` [PATCH net-next 1/2] tcp: add a common helper to debug the underlying issue Jason Xing
@ 2024-10-20 16:18   ` David Ahern
  2024-10-20 16:34     ` Jason Xing
  0 siblings, 1 reply; 9+ messages in thread
From: David Ahern @ 2024-10-20 16:18 UTC (permalink / raw)
  To: Jason Xing, davem, edumazet, kuba, pabeni, ncardwell; +Cc: netdev, Jason Xing

On 10/20/24 8:50 AM, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> Following the commit c8770db2d544 ("tcp: check skb is non-NULL
> in tcp_rto_delta_us()"), we decided to add a helper so that it's
> easier to get verbose warning on either cases.
> 
> Link: https://lore.kernel.org/all/5632e043-bdba-4d75-bc7e-bf58014492fd@redhat.com/
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> ---
>  include/net/tcp.h | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 739a9fb83d0c..cac7bbff61ce 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2430,6 +2430,22 @@ void tcp_plb_update_state(const struct sock *sk, struct tcp_plb_state *plb,
>  void tcp_plb_check_rehash(struct sock *sk, struct tcp_plb_state *plb);
>  void tcp_plb_update_state_upon_rto(struct sock *sk, struct tcp_plb_state *plb);
>  
> +static inline void tcp_warn_once(const struct sock *sk, bool cond, const char *str)
> +{
> +	WARN_ONCE(cond,
> +		  "%s"
> +		  "out:%u sacked:%u lost:%u retrans:%u "
> +		  "tlp_high_seq:%u sk_state:%u ca_state:%u "
> +		  "advmss:%u mss_cache:%u pmtu:%u\n",

format lines should not be split across lines. Yes, I realize the
existing code is, but since you are moving it and making changes to it
this can be fixed as well.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 1/2] tcp: add a common helper to debug the underlying issue
  2024-10-20 16:18   ` David Ahern
@ 2024-10-20 16:34     ` Jason Xing
  2024-10-20 16:40       ` David Ahern
  0 siblings, 1 reply; 9+ messages in thread
From: Jason Xing @ 2024-10-20 16:34 UTC (permalink / raw)
  To: David Ahern; +Cc: davem, edumazet, kuba, pabeni, ncardwell, netdev, Jason Xing

Hello David,

On Mon, Oct 21, 2024 at 12:18 AM David Ahern <dsahern@kernel.org> wrote:
>
> On 10/20/24 8:50 AM, Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Following the commit c8770db2d544 ("tcp: check skb is non-NULL
> > in tcp_rto_delta_us()"), we decided to add a helper so that it's
> > easier to get verbose warning on either cases.
> >
> > Link: https://lore.kernel.org/all/5632e043-bdba-4d75-bc7e-bf58014492fd@redhat.com/
> > Suggested-by: Paolo Abeni <pabeni@redhat.com>
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > Cc: Neal Cardwell <ncardwell@google.com>
> > ---
> >  include/net/tcp.h | 28 +++++++++++++++++-----------
> >  1 file changed, 17 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index 739a9fb83d0c..cac7bbff61ce 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -2430,6 +2430,22 @@ void tcp_plb_update_state(const struct sock *sk, struct tcp_plb_state *plb,
> >  void tcp_plb_check_rehash(struct sock *sk, struct tcp_plb_state *plb);
> >  void tcp_plb_update_state_upon_rto(struct sock *sk, struct tcp_plb_state *plb);
> >
> > +static inline void tcp_warn_once(const struct sock *sk, bool cond, const char *str)
> > +{
> > +     WARN_ONCE(cond,
> > +               "%s"
> > +               "out:%u sacked:%u lost:%u retrans:%u "
> > +               "tlp_high_seq:%u sk_state:%u ca_state:%u "
> > +               "advmss:%u mss_cache:%u pmtu:%u\n",
>
> format lines should not be split across lines. Yes, I realize the
> existing code is, but since you are moving it and making changes to it
> this can be fixed as well.

Thanks for reminding me. Actually before submitting this series, I
noticed this warning. I was thinking it looks a little ugly if we are
going to add more information in this function?

This function could be like this:
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 739a9fb83d0c..8b8d94bb1746 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2430,6 +2430,19 @@ void tcp_plb_update_state(const struct sock
*sk, struct tcp_plb_state *plb,
 void tcp_plb_check_rehash(struct sock *sk, struct tcp_plb_state *plb);
 void tcp_plb_update_state_upon_rto(struct sock *sk, struct tcp_plb_state *plb);

+static inline void tcp_warn_once(const struct sock *sk, bool cond,
const char *str)
+{
+       WARN_ONCE(cond,
+                 "%sout:%u sacked:%u lost:%u retrans:%u
tlp_high_seq:%u sk_state:%u ca_state:%u advmss:%u mss_cache:%u
pmtu:%u\n",
+                 str,
+                 tcp_sk(sk)->packets_out, tcp_sk(sk)->sacked_out,
+                 tcp_sk(sk)->lost_out, tcp_sk(sk)->retrans_out,
+                 tcp_sk(sk)->tlp_high_seq, sk->sk_state,
+                 inet_csk(sk)->icsk_ca_state,
+                 tcp_sk(sk)->advmss, tcp_sk(sk)->mss_cache,
+                 inet_csk(sk)->icsk_pmtu_cookie);
+}
+
That quoted line seems a little long... Do you think this format is
fine with you? If so, I will adjust it in the next version.

Thanks,
Jason

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 1/2] tcp: add a common helper to debug the underlying issue
  2024-10-20 16:34     ` Jason Xing
@ 2024-10-20 16:40       ` David Ahern
  2024-10-20 16:50         ` Jason Xing
  0 siblings, 1 reply; 9+ messages in thread
From: David Ahern @ 2024-10-20 16:40 UTC (permalink / raw)
  To: Jason Xing; +Cc: davem, edumazet, kuba, pabeni, ncardwell, netdev, Jason Xing

On 10/20/24 10:34 AM, Jason Xing wrote:
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 739a9fb83d0c..8b8d94bb1746 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2430,6 +2430,19 @@ void tcp_plb_update_state(const struct sock
> *sk, struct tcp_plb_state *plb,
>  void tcp_plb_check_rehash(struct sock *sk, struct tcp_plb_state *plb);
>  void tcp_plb_update_state_upon_rto(struct sock *sk, struct tcp_plb_state *plb);
> 
> +static inline void tcp_warn_once(const struct sock *sk, bool cond,
> const char *str)
> +{
> +       WARN_ONCE(cond,
> +                 "%sout:%u sacked:%u lost:%u retrans:%u
> tlp_high_seq:%u sk_state:%u ca_state:%u advmss:%u mss_cache:%u
> pmtu:%u\n",
> +                 str,
> +                 tcp_sk(sk)->packets_out, tcp_sk(sk)->sacked_out,
> +                 tcp_sk(sk)->lost_out, tcp_sk(sk)->retrans_out,
> +                 tcp_sk(sk)->tlp_high_seq, sk->sk_state,
> +                 inet_csk(sk)->icsk_ca_state,
> +                 tcp_sk(sk)->advmss, tcp_sk(sk)->mss_cache,
> +                 inet_csk(sk)->icsk_pmtu_cookie);
> +}
> +
> That quoted line seems a little long... Do you think this format is
> fine with you? If so, I will adjust it in the next version.
> 

Format strings are an exception to the 80-column rule. Strings should
not be split to allow for grep to find a match, for example.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 1/2] tcp: add a common helper to debug the underlying issue
  2024-10-20 16:40       ` David Ahern
@ 2024-10-20 16:50         ` Jason Xing
  0 siblings, 0 replies; 9+ messages in thread
From: Jason Xing @ 2024-10-20 16:50 UTC (permalink / raw)
  To: David Ahern; +Cc: davem, edumazet, kuba, pabeni, ncardwell, netdev, Jason Xing

On Mon, Oct 21, 2024 at 12:41 AM David Ahern <dsahern@kernel.org> wrote:
>
> On 10/20/24 10:34 AM, Jason Xing wrote:
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index 739a9fb83d0c..8b8d94bb1746 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -2430,6 +2430,19 @@ void tcp_plb_update_state(const struct sock
> > *sk, struct tcp_plb_state *plb,
> >  void tcp_plb_check_rehash(struct sock *sk, struct tcp_plb_state *plb);
> >  void tcp_plb_update_state_upon_rto(struct sock *sk, struct tcp_plb_state *plb);
> >
> > +static inline void tcp_warn_once(const struct sock *sk, bool cond,
> > const char *str)
> > +{
> > +       WARN_ONCE(cond,
> > +                 "%sout:%u sacked:%u lost:%u retrans:%u
> > tlp_high_seq:%u sk_state:%u ca_state:%u advmss:%u mss_cache:%u
> > pmtu:%u\n",
> > +                 str,
> > +                 tcp_sk(sk)->packets_out, tcp_sk(sk)->sacked_out,
> > +                 tcp_sk(sk)->lost_out, tcp_sk(sk)->retrans_out,
> > +                 tcp_sk(sk)->tlp_high_seq, sk->sk_state,
> > +                 inet_csk(sk)->icsk_ca_state,
> > +                 tcp_sk(sk)->advmss, tcp_sk(sk)->mss_cache,
> > +                 inet_csk(sk)->icsk_pmtu_cookie);
> > +}
> > +
> > That quoted line seems a little long... Do you think this format is
> > fine with you? If so, I will adjust it in the next version.
> >
>
> Format strings are an exception to the 80-column rule. Strings should
> not be split to allow for grep to find a match, for example.

Thanks. I got it. I will use the above code snippet which can pass the
checkpatch script.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 2/2] tcp: add more warn of socket in tcp_send_loss_probe()
  2024-10-20 14:50 ` [PATCH net-next 2/2] tcp: add more warn of socket in tcp_send_loss_probe() Jason Xing
@ 2024-10-21  2:37   ` Jason Xing
  2024-10-21  5:52   ` kernel test robot
  1 sibling, 0 replies; 9+ messages in thread
From: Jason Xing @ 2024-10-21  2:37 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, dsahern, ncardwell; +Cc: netdev, Jason Xing

On Sun, Oct 20, 2024 at 10:50 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> From: Jason Xing <kernelxing@tencent.com>
>
> Add two fields to print in the helper which here covers tcp_send_loss_probe().
>
> Link: https://lore.kernel.org/all/5632e043-bdba-4d75-bc7e-bf58014492fd@redhat.com/
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> ---
>  include/net/tcp.h     | 5 ++++-
>  net/ipv4/tcp_output.c | 4 +---
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index cac7bbff61ce..68eb03758950 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2434,14 +2434,17 @@ static inline void tcp_warn_once(const struct sock *sk, bool cond, const char *s
>  {
>         WARN_ONCE(cond,
>                   "%s"
> +                 "cwnd:%u "
>                   "out:%u sacked:%u lost:%u retrans:%u "
>                   "tlp_high_seq:%u sk_state:%u ca_state:%u "
> -                 "advmss:%u mss_cache:%u pmtu:%u\n",
> +                 "mss:%u advmss:%u mss_cache:%u pmtu:%u\n",
>                   str,
> +                 tcp_snd_cwnd(tcp_sk(sk)),
>                   tcp_sk(sk)->packets_out, tcp_sk(sk)->sacked_out,
>                   tcp_sk(sk)->lost_out, tcp_sk(sk)->retrans_out,
>                   tcp_sk(sk)->tlp_high_seq, sk->sk_state,
>                   inet_csk(sk)->icsk_ca_state,
> +                 tcp_current_mss((struct sock *)sk),
>                   tcp_sk(sk)->advmss, tcp_sk(sk)->mss_cache,
>                   inet_csk(sk)->icsk_pmtu_cookie);
>  }
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 054244ce5117..295bc0741772 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2954,9 +2954,7 @@ void tcp_send_loss_probe(struct sock *sk)
>         }
>         skb = skb_rb_last(&sk->tcp_rtx_queue);
>         if (unlikely(!skb)) {
> -               WARN_ONCE(tp->packets_out,
> -                         "invalid inflight: %u state %u cwnd %u mss %d\n",
> -                         tp->packets_out, sk->sk_state, tcp_snd_cwnd(tp), mss);
> +               tcp_warn_once(sk, tp->packets_out, NULL);

Sorry, I noticed the warning:
In function ‘tcp_warn_once’,
    inlined from ‘tcp_send_loss_probe’ at ../net/ipv4/tcp_output.c:2957:3:
../include/net/tcp.h:2436:19: warning: ‘%s’ directive argument is null
[-Wformat-overflow=]
 2436 |                   "%s"
      |                   ^~~~

I think It should be:
tcp_warn_once(sk, tp->packets_out, "");

Will handle this soon.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 2/2] tcp: add more warn of socket in tcp_send_loss_probe()
  2024-10-20 14:50 ` [PATCH net-next 2/2] tcp: add more warn of socket in tcp_send_loss_probe() Jason Xing
  2024-10-21  2:37   ` Jason Xing
@ 2024-10-21  5:52   ` kernel test robot
  1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2024-10-21  5:52 UTC (permalink / raw)
  To: Jason Xing, davem, edumazet, kuba, pabeni, dsahern, ncardwell
  Cc: oe-kbuild-all, netdev, Jason Xing

Hi Jason,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-Xing/tcp-add-a-common-helper-to-debug-the-underlying-issue/20241020-225356
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241020145029.27725-3-kerneljasonxing%40gmail.com
patch subject: [PATCH net-next 2/2] tcp: add more warn of socket in tcp_send_loss_probe()
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20241021/202410211313.7YBoFLNS-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241021/202410211313.7YBoFLNS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410211313.7YBoFLNS-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from arch/x86/include/asm/bug.h:99,
                    from include/linux/bug.h:5,
                    from include/linux/fortify-string.h:6,
                    from include/linux/string.h:390,
                    from include/linux/bitmap.h:13,
                    from include/linux/cpumask.h:12,
                    from arch/x86/include/asm/cpumask.h:5,
                    from arch/x86/include/asm/msr.h:11,
                    from arch/x86/include/asm/tsc.h:10,
                    from arch/x86/include/asm/timex.h:6,
                    from include/linux/timex.h:67,
                    from include/linux/time32.h:13,
                    from include/linux/time.h:60,
                    from include/linux/skbuff.h:15,
                    from include/linux/tcp.h:17,
                    from include/net/tcp.h:20,
                    from net/ipv4/tcp_output.c:40:
   In function 'tcp_warn_once',
       inlined from 'tcp_warn_once' at include/net/tcp.h:2433:20,
       inlined from 'tcp_send_loss_probe' at net/ipv4/tcp_output.c:2957:3:
>> include/net/tcp.h:2436:19: warning: '%s' directive argument is null [-Wformat-overflow=]
    2436 |                   "%s"
         |                   ^~~~
   include/asm-generic/bug.h:106:31: note: in definition of macro '__WARN_printf'
     106 |                 __warn_printk(arg);                                     \
         |                               ^~~
   include/linux/once_lite.h:31:25: note: in expansion of macro 'WARN'
      31 |                         func(__VA_ARGS__);                              \
         |                         ^~~~
   include/asm-generic/bug.h:152:9: note: in expansion of macro 'DO_ONCE_LITE_IF'
     152 |         DO_ONCE_LITE_IF(condition, WARN, 1, format)
         |         ^~~~~~~~~~~~~~~
   include/net/tcp.h:2435:9: note: in expansion of macro 'WARN_ONCE'
    2435 |         WARN_ONCE(cond,
         |         ^~~~~~~~~
   include/net/tcp.h: In function 'tcp_send_loss_probe':
   include/net/tcp.h:2436:20: note: format string is defined here
    2436 |                   "%s"
         |                    ^~


vim +2436 include/net/tcp.h

1a91bb7c3ebf95 Mubashir Adnan Qureshi 2022-10-26  2432  
4fe1493c15028c Jason Xing             2024-10-20  2433  static inline void tcp_warn_once(const struct sock *sk, bool cond, const char *str)
4fe1493c15028c Jason Xing             2024-10-20  2434  {
4fe1493c15028c Jason Xing             2024-10-20  2435  	WARN_ONCE(cond,
4fe1493c15028c Jason Xing             2024-10-20 @2436  		  "%s"
d21098f05ea727 Jason Xing             2024-10-20  2437  		  "cwnd:%u "
4fe1493c15028c Jason Xing             2024-10-20  2438  		  "out:%u sacked:%u lost:%u retrans:%u "
4fe1493c15028c Jason Xing             2024-10-20  2439  		  "tlp_high_seq:%u sk_state:%u ca_state:%u "
d21098f05ea727 Jason Xing             2024-10-20  2440  		  "mss:%u advmss:%u mss_cache:%u pmtu:%u\n",
4fe1493c15028c Jason Xing             2024-10-20  2441  		  str,
d21098f05ea727 Jason Xing             2024-10-20  2442  		  tcp_snd_cwnd(tcp_sk(sk)),
4fe1493c15028c Jason Xing             2024-10-20  2443  		  tcp_sk(sk)->packets_out, tcp_sk(sk)->sacked_out,
4fe1493c15028c Jason Xing             2024-10-20  2444  		  tcp_sk(sk)->lost_out, tcp_sk(sk)->retrans_out,
4fe1493c15028c Jason Xing             2024-10-20  2445  		  tcp_sk(sk)->tlp_high_seq, sk->sk_state,
4fe1493c15028c Jason Xing             2024-10-20  2446  		  inet_csk(sk)->icsk_ca_state,
d21098f05ea727 Jason Xing             2024-10-20  2447  		  tcp_current_mss((struct sock *)sk),
4fe1493c15028c Jason Xing             2024-10-20  2448  		  tcp_sk(sk)->advmss, tcp_sk(sk)->mss_cache,
4fe1493c15028c Jason Xing             2024-10-20  2449  		  inet_csk(sk)->icsk_pmtu_cookie);
4fe1493c15028c Jason Xing             2024-10-20  2450  }
4fe1493c15028c Jason Xing             2024-10-20  2451  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-10-21  5:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-20 14:50 [PATCH net-next 0/2] tcp: add tcp_warn_once() common helper Jason Xing
2024-10-20 14:50 ` [PATCH net-next 1/2] tcp: add a common helper to debug the underlying issue Jason Xing
2024-10-20 16:18   ` David Ahern
2024-10-20 16:34     ` Jason Xing
2024-10-20 16:40       ` David Ahern
2024-10-20 16:50         ` Jason Xing
2024-10-20 14:50 ` [PATCH net-next 2/2] tcp: add more warn of socket in tcp_send_loss_probe() Jason Xing
2024-10-21  2:37   ` Jason Xing
2024-10-21  5:52   ` kernel test robot

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).