public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH] build: Fix errors of TCP_REPAIR_* undeclared
@ 2025-09-01  9:45 Yumei Huang
  2025-09-01 21:02 ` Stefano Brivio
  0 siblings, 1 reply; 3+ messages in thread
From: Yumei Huang @ 2025-09-01  9:45 UTC (permalink / raw)
  To: passt-dev; +Cc: sbrivio, dgibson, yuhuang

Fix the following errors on systems with glibc < 2.29:

tcp.c: In function ‘tcp_flow_repair_on’:
tcp.c:2787:38: error: ‘TCP_REPAIR_ON’ undeclared (first use in this function); did you mean ‘TCP_REPAIR’?
  if ((rc = repair_set(c, conn->sock, TCP_REPAIR_ON)))
                                      ^~~~~~~~~~~~~
                                      TCP_REPAIR
tcp.c:2787:38: note: each undeclared identifier is reported only once for each function it appears in
tcp.c: In function ‘tcp_flow_repair_off’:
tcp.c:2807:38: error: ‘TCP_REPAIR_OFF’ undeclared (first use in this function); did you mean ‘TCP_REPAIR’?
  if ((rc = repair_set(c, conn->sock, TCP_REPAIR_OFF)))
                                      ^~~~~~~~~~~~~~
                                      TCP_REPAIR
make: *** [Makefile:94: passt] Error 1

Signed-off-by: Yumei Huang <yuhuang@redhat.com>
---
 linux_dep.h    | 6 ++++++
 passt-repair.c | 1 +
 2 files changed, 7 insertions(+)

diff --git a/linux_dep.h b/linux_dep.h
index 240f50a..c200444 100644
--- a/linux_dep.h
+++ b/linux_dep.h
@@ -135,6 +135,12 @@ struct tcp_info_linux {
 #define CLOSE_RANGE_UNSHARE	(1U << 1)
 #endif
 
+#ifndef TCP_REPAIR_ON
+#define TCP_REPAIR_ON           1
+#define TCP_REPAIR_OFF          0
+#define TCP_REPAIR_OFF_NO_WP    -1      /* Turn off without window probes */
+#endif
+
 __attribute__ ((weak))
 /* cppcheck-suppress funcArgNamesDifferent */
 int close_range(unsigned int first, unsigned int last, int flags) {
diff --git a/passt-repair.c b/passt-repair.c
index 8c59d7e..c3c140f 100644
--- a/passt-repair.c
+++ b/passt-repair.c
@@ -40,6 +40,7 @@
 #include <linux/seccomp.h>
 
 #include "seccomp_repair.h"
+#include "linux_dep.h"
 
 #define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in UAPI */
 #define REPAIR_EXT		".repair"
-- 
@@ -40,6 +40,7 @@
 #include <linux/seccomp.h>
 
 #include "seccomp_repair.h"
+#include "linux_dep.h"
 
 #define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in UAPI */
 #define REPAIR_EXT		".repair"
-- 
2.44.0


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

* Re: [PATCH] build: Fix errors of TCP_REPAIR_* undeclared
  2025-09-01  9:45 [PATCH] build: Fix errors of TCP_REPAIR_* undeclared Yumei Huang
@ 2025-09-01 21:02 ` Stefano Brivio
       [not found]   ` <CANsz47k97ovnU1_CQPOH+yi3-xHeo78bsWmMgqaPBO+mTUOqzg@mail.gmail.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Stefano Brivio @ 2025-09-01 21:02 UTC (permalink / raw)
  To: Yumei Huang; +Cc: passt-dev, dgibson

Thanks for the patch! A couple of comments, all minor:

On Mon,  1 Sep 2025 17:45:10 +0800
Yumei Huang <yuhuang@redhat.com> wrote:

> Fix the following errors on systems with glibc < 2.29:

Before anything, it's customary to credit the original author of a
patch, even if it was (very) different. Something like:

Based on an original patch by Dongsheng, fix ...

and I think you should also Cc: them (they might want to test / comment
/ review), you can fetch the contact from
https://bugs.passt.top/show_bug.cgi?id=121.

> 
> tcp.c: In function ‘tcp_flow_repair_on’:
> tcp.c:2787:38: error: ‘TCP_REPAIR_ON’ undeclared (first use in this function); did you mean ‘TCP_REPAIR’?
>   if ((rc = repair_set(c, conn->sock, TCP_REPAIR_ON)))
>                                       ^~~~~~~~~~~~~
>                                       TCP_REPAIR
> tcp.c:2787:38: note: each undeclared identifier is reported only once for each function it appears in
> tcp.c: In function ‘tcp_flow_repair_off’:
> tcp.c:2807:38: error: ‘TCP_REPAIR_OFF’ undeclared (first use in this function); did you mean ‘TCP_REPAIR’?
>   if ((rc = repair_set(c, conn->sock, TCP_REPAIR_OFF)))
>                                       ^~~~~~~~~~~~~~
>                                       TCP_REPAIR
> make: *** [Makefile:94: passt] Error 1
> 
> Signed-off-by: Yumei Huang <yuhuang@redhat.com>

Just before this tag, we typically add any references, always using the
"Link:" tag for simplicity.

Many other projects use separate tags such as "Bug:", "Bugzilla:",
"Closes:" and so on, but we don't, so that it's a bit easier to
automate parsing, if needed. In this case that would be:

Link: https://bugs.passt.top/show_bug.cgi?id=121

and perhaps a Reported-by: tag would also be good to have, here.

> ---
>  linux_dep.h    | 6 ++++++
>  passt-repair.c | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/linux_dep.h b/linux_dep.h
> index 240f50a..c200444 100644
> --- a/linux_dep.h
> +++ b/linux_dep.h
> @@ -135,6 +135,12 @@ struct tcp_info_linux {
>  #define CLOSE_RANGE_UNSHARE	(1U << 1)
>  #endif
>  
> +#ifndef TCP_REPAIR_ON
> +#define TCP_REPAIR_ON           1
> +#define TCP_REPAIR_OFF          0
> +#define TCP_REPAIR_OFF_NO_WP    -1      /* Turn off without window probes */

We use tabs for indentation, and a full tab is displayed as 8 characters
wide. For example:

#define TCP_REPAIR_ON		1
                      ^^ two tabs here, so that TCP_REPAIR_OFF_NO_WP is
                      aligned too

If you need a suggestion for editor showing whitespace (marking tabs and
spaces separately), I use Geany and it does that. I think others use
Emacs, I'm not sure how it works there.

> +#endif
> +
>  __attribute__ ((weak))
>  /* cppcheck-suppress funcArgNamesDifferent */
>  int close_range(unsigned int first, unsigned int last, int flags) {
> diff --git a/passt-repair.c b/passt-repair.c
> index 8c59d7e..c3c140f 100644
> --- a/passt-repair.c
> +++ b/passt-repair.c
> @@ -40,6 +40,7 @@
>  #include <linux/seccomp.h>
>  
>  #include "seccomp_repair.h"
> +#include "linux_dep.h"
>  
>  #define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in UAPI */
>  #define REPAIR_EXT		".repair"

The rest looks good to me!

-- 
Stefano


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

* Re: [PATCH] build: Fix errors of TCP_REPAIR_* undeclared
       [not found]   ` <CANsz47k97ovnU1_CQPOH+yi3-xHeo78bsWmMgqaPBO+mTUOqzg@mail.gmail.com>
@ 2025-09-02  7:27     ` Stefano Brivio
  0 siblings, 0 replies; 3+ messages in thread
From: Stefano Brivio @ 2025-09-02  7:27 UTC (permalink / raw)
  To: Yumei Huang; +Cc: passt-dev, dgibson

On Tue, 2 Sep 2025 10:54:35 +0800
Yumei Huang <yuhuang@redhat.com> wrote:

> Thank you Stefano for all the comments.  Sent v2 to address all the issues,
> just that..
> 
> On Tue, Sep 2, 2025 at 5:02 AM Stefano Brivio <sbrivio@redhat.com> wrote:
> 
> > Thanks for the patch! A couple of comments, all minor:
> >
> > On Mon,  1 Sep 2025 17:45:10 +0800
> > Yumei Huang <yuhuang@redhat.com> wrote:
> >  
> > > Fix the following errors on systems with glibc < 2.29:  
> >
> > Before anything, it's customary to credit the original author of a
> > patch, even if it was (very) different. Something like:
> >
> > Based on an original patch by Dongsheng, fix ...
> >
> > and I think you should also Cc: them (they might want to test / comment
> > / review), you can fetch the contact from
> > https://bugs.passt.top/show_bug.cgi?id=121.
> >  
> > >
> > > tcp.c: In function ‘tcp_flow_repair_on’:
> > > tcp.c:2787:38: error: ‘TCP_REPAIR_ON’ undeclared (first use in this  
> > function); did you mean ‘TCP_REPAIR’?  
> > >   if ((rc = repair_set(c, conn->sock, TCP_REPAIR_ON)))
> > >                                       ^~~~~~~~~~~~~
> > >                                       TCP_REPAIR
> > > tcp.c:2787:38: note: each undeclared identifier is reported only once  
> > for each function it appears in  
> > > tcp.c: In function ‘tcp_flow_repair_off’:
> > > tcp.c:2807:38: error: ‘TCP_REPAIR_OFF’ undeclared (first use in this  
> > function); did you mean ‘TCP_REPAIR’?  
> > >   if ((rc = repair_set(c, conn->sock, TCP_REPAIR_OFF)))
> > >                                       ^~~~~~~~~~~~~~
> > >                                       TCP_REPAIR
> > > make: *** [Makefile:94: passt] Error 1
> > >
> > > Signed-off-by: Yumei Huang <yuhuang@redhat.com>  
> >
> > Just before this tag, we typically add any references, always using the
> > "Link:" tag for simplicity.
> >
> > Many other projects use separate tags such as "Bug:", "Bugzilla:",
> > "Closes:" and so on, but we don't, so that it's a bit easier to
> > automate parsing, if needed. In this case that would be:
> >
> > Link: https://bugs.passt.top/show_bug.cgi?id=121
> >
> > and perhaps a Reported-by: tag would also be good to have, here.
> >  
> > > ---
> > >  linux_dep.h    | 6 ++++++
> > >  passt-repair.c | 1 +
> > >  2 files changed, 7 insertions(+)
> > >
> > > diff --git a/linux_dep.h b/linux_dep.h
> > > index 240f50a..c200444 100644
> > > --- a/linux_dep.h
> > > +++ b/linux_dep.h
> > > @@ -135,6 +135,12 @@ struct tcp_info_linux {
> > >  #define CLOSE_RANGE_UNSHARE  (1U << 1)
> > >  #endif
> > >
> > > +#ifndef TCP_REPAIR_ON
> > > +#define TCP_REPAIR_ON           1
> > > +#define TCP_REPAIR_OFF          0
> > > +#define TCP_REPAIR_OFF_NO_WP    -1      /* Turn off without window  
> > probes */
> >
> > We use tabs for indentation, and a full tab is displayed as 8 characters
> > wide. For example:
> >
> > #define TCP_REPAIR_ON           1
> >                       ^^ two tabs here, so that TCP_REPAIR_OFF_NO_WP is
> >                       aligned too
> >
> > I did replace the spaces here with two tabs in v2,  and it shows aligned  
> in my editors both vim and vscode, but it seems it's not in gmail.
> Tried 'set tabstop=8' and removed 'set expandtab' in the .vimrc file, and
> got the same result. Not sure what's going wrong...

I'm not sure either, just mind that you shouldn't rely on the web
interface of Gmail because it won't show you emails / patches exactly
as they are. But v2 looks good to me, so I guess you fixed it. :)

Two more things:

1. look at two paragraph above this one, you'll see that quoting is not
   correct:

> > I did replace ...
> in my editors

it looks as if "I did replace ..." is from my reply, not from yours.
I've seen this issue with people using Gmail but as far as I understood
it's a known issue on Gmail side and they don't plan to fix it.

2. your reply contains an HTML version. While I configured passt mailing
   lists to be tolerant of that, I think that many upstream mailing
   lists (e.g. kernel lists) will just drop your message altogether if
   it contains an HTML part.

In the end, I would suggest you use an email client instead, which
makes it so much easier to work with code. I use claws-mail, other
popular options include Thunderbird, Sylpheed, mutt.

Maybe it can all be fixed with Gmail configurations, I'm not sure, but
I've seen people struggling quite a bit before deciding to use an email
client (at least for lists and patches).

-- 
Stefano


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

end of thread, other threads:[~2025-09-02  7:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-01  9:45 [PATCH] build: Fix errors of TCP_REPAIR_* undeclared Yumei Huang
2025-09-01 21:02 ` Stefano Brivio
     [not found]   ` <CANsz47k97ovnU1_CQPOH+yi3-xHeo78bsWmMgqaPBO+mTUOqzg@mail.gmail.com>
2025-09-02  7:27     ` Stefano Brivio

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

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 IMAP folder(s).