[PATCH ipvsadm v3 0/5] Debian package fixes

[PATCH ipvsadm v3 0/5] Debian package fixes

[Jeremy Sowden]

The Debian package of ipvsadm carries a few patches.  Some of them have
been around for years.  None of them is specific to Debian, so I'm
forwarding them upstream.

Patch 1 bumps the maximum weight accepted by ipvsadm.
Patch 2 fixes an unclear usage error message.
Patches 3-5 make some improvements to the Makefiles.

Changes since v2:

* Reformat the commit messages of patches 1, 2 & 5 according to the
  kernel's preferred patch format.

Changes since v1:

* The previous version of patch 2 changed the logic of the option parsing
  in such a way that using `-M` after `-6` no longer worked correctly.
  Thisversion just rewords the error message.

Jeremy Sowden (5):
  ipvsadm: increase maximum weight value
  ipvsadm: fix ambiguous usage error message
  Use variable for pkg-config in Makefiles
  Support environmental and command-line `*FLAGS` variable in Makefiles
  Make sure libipvs.a is built before ipvsadm

 Makefile         | 19 +++++++++++--------
 ipvsadm.8        |  2 +-
 ipvsadm.c        |  4 ++--
 libipvs/Makefile | 29 +++++++++++++++--------------
 4 files changed, 29 insertions(+), 25 deletions(-)

-- 
2.45.2

[PATCH ipvsadm v3 1/5] ipvsadm: increase maximum weight value

[Jeremy Sowden]

The IPVS kernel module accepts weight up to 2,147,483,647 (INT_MAX), but
ipvsadm limits it to 65,535.  Raise the user space maximum to match the
kernel.

Link: https://bugs.debian.org/814348
Signed-off-by: Jeremy Sowden <azazel@debian.org>
---
 ipvsadm.8 | 2 +-
 ipvsadm.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipvsadm.8 b/ipvsadm.8
index b3bc822aaafc..3d7541a4bf62 100644
--- a/ipvsadm.8
+++ b/ipvsadm.8
@@ -386,7 +386,7 @@ servers are added or modified.
 .B -w, --weight \fIweight\fP
 \fIWeight\fP is an integer specifying the capacity  of a server
 relative to the others in the pool. The valid values of \fIweight\fP
-are 0 through to 65535. The default is 1. Quiescent servers are
+are 0 through to 2147483647. The default is 1. Quiescent servers are
 specified with a weight of zero. A quiescent server will receive no
 new jobs but still serve the existing jobs, for all scheduling
 algorithms distributed with the Linux Virtual Server. Setting a
diff --git a/ipvsadm.c b/ipvsadm.c
index 663d47ab1138..42f31a20e596 100644
--- a/ipvsadm.c
+++ b/ipvsadm.c
@@ -762,7 +762,7 @@ parse_options(int argc, char **argv, struct ipvs_command_entry *ce,
 		case 'w':
 			set_option(options, OPTC_WEIGHT);
 			if ((ce->dest.weight =
-			     string_to_number(optarg, 0, 65535)) == -1)
+			     string_to_number(optarg, 0, INT_MAX)) == -1)
 				fail(2, "illegal weight specified");
 			break;
 		case 'x':
-- 
2.45.2

[PATCH ipvsadm v3 2/5] ipvsadm: fix ambiguous usage error message

[Jeremy Sowden]

If `-6` is used without `-f`, the usage error message is "-6 used before
-f", which can be misconstrued as warning that both options were used but
in the wrong order.  Reword it.

Link: http://bugs.debian.org/610596
Signed-off-by: Jeremy Sowden <azazel@debian.org>
---
 ipvsadm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipvsadm.c b/ipvsadm.c
index 42f31a20e596..10bf84c85fbe 100644
--- a/ipvsadm.c
+++ b/ipvsadm.c
@@ -833,7 +833,7 @@ parse_options(int argc, char **argv, struct ipvs_command_entry *ce,
 				ce->svc.af = AF_INET6;
 				ce->svc.netmask = 128;
 			} else {
-				fail(2, "-6 used before -f\n");
+				fail(2, "-6 must follow -f\n");
 			}
 			break;
 		case 'o':
-- 
2.45.2

[PATCH ipvsadm v3 3/5] Use variable for pkg-config in Makefiles

[Jeremy Sowden]

Replace hard-coded `pkg-config` with a variable to allow it to be
overridden, which is helpful for cross-building.

Link: https://bugs.debian.org/901275
Signed-off-by: Jeremy Sowden <azazel@debian.org>
---
 Makefile         |  9 +++++----
 libipvs/Makefile | 17 +++++++++--------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 91a2991f6746..d247d4075160 100644
--- a/Makefile
+++ b/Makefile
@@ -36,6 +36,7 @@ RPMSOURCEDIR	= $(shell rpm --eval '%_sourcedir')
 RPMSPECDIR	= $(shell rpm --eval '%_specdir')
 
 CC		= gcc
+PKG_CONFIG	?= pkg-config
 INCLUDE		=
 SBIN		= $(BUILD_ROOT)/sbin
 MANDIR		= usr/man
@@ -66,10 +67,10 @@ OBJS		= ipvsadm.o config_stream.o dynamic_array.o
 LIBS		= -lpopt
 ifneq (0,$(HAVE_NL))
 LIBS		+= $(shell \
-		if which pkg-config > /dev/null 2>&1; then \
-		  if   pkg-config --libs libnl-genl-3.0  2> /dev/null; then :;\
-		  elif pkg-config --libs libnl-2.0       2> /dev/null; then :;\
-		  elif pkg-config --libs libnl-1         2> /dev/null; then :;\
+		if which $(PKG_CONFIG) > /dev/null 2>&1; then \
+		  if   $(PKG_CONFIG) --libs libnl-genl-3.0  2> /dev/null; then :;\
+		  elif $(PKG_CONFIG) --libs libnl-2.0       2> /dev/null; then :;\
+		  elif $(PKG_CONFIG) --libs libnl-1         2> /dev/null; then :;\
 		  fi; \
 		else echo "-lnl"; fi)
 endif
diff --git a/libipvs/Makefile b/libipvs/Makefile
index f845c8b1675b..b31c8eac514d 100644
--- a/libipvs/Makefile
+++ b/libipvs/Makefile
@@ -1,14 +1,15 @@
 # Makefile for libipvs
 
 CC		= gcc
+PKG_CONFIG	?= pkg-config
 CFLAGS		= -Wall -Wunused -Wstrict-prototypes -g -fPIC
 ifneq (0,$(HAVE_NL))
 CFLAGS		+= -DLIBIPVS_USE_NL
 CFLAGS		+= $(shell \
-		if which pkg-config > /dev/null 2>&1; then \
-		  if   pkg-config --cflags libnl-3.0  2> /dev/null; then :; \
-		  elif pkg-config --cflags libnl-2.0  2> /dev/null; then :; \
-		  elif pkg-config --cflags libnl-1    2> /dev/null; then :; \
+		if which $(PKG_CONFIG) > /dev/null 2>&1; then \
+		  if   $(PKG_CONFIG) --cflags libnl-3.0  2> /dev/null; then :; \
+		  elif $(PKG_CONFIG) --cflags libnl-2.0  2> /dev/null; then :; \
+		  elif $(PKG_CONFIG) --cflags libnl-1    2> /dev/null; then :; \
 		  fi; \
 		fi)
 endif
@@ -17,10 +18,10 @@ INCLUDE		+= $(shell if [ -f ../../ip_vs.h ]; then	\
 		     echo "-I../../."; fi;)
 DEFINES		= $(shell if [ ! -f ../../ip_vs.h ]; then	\
 		    echo "-DHAVE_NET_IP_VS_H"; fi;)
-DEFINES		+= $(shell if which pkg-config > /dev/null 2>&1; then \
-			 if   pkg-config --exists libnl-3.0; then :; \
-			 elif pkg-config --exists libnl-2.0; then :; \
-			 elif pkg-config --exists libnl-1; \
+DEFINES		+= $(shell if which $(PKG_CONFIG) > /dev/null 2>&1; then \
+			 if   $(PKG_CONFIG) --exists libnl-3.0; then :; \
+			 elif $(PKG_CONFIG) --exists libnl-2.0; then :; \
+			 elif $(PKG_CONFIG) --exists libnl-1; \
 			 then echo "-DFALLBACK_LIBNL1"; fi; fi)
 
 .PHONY		= all clean install dist distclean rpm rpms
-- 
2.45.2

[PATCH ipvsadm v3 4/5] Support environmental and command-line `*FLAGS` variable in Makefiles

[Jeremy Sowden]

The Makefiles don't use `CPPFLAGS` or `LDFLAGS`, and set `CFLAGS`
unconditionally.  Rename `CFLAGS`, and add `CPPFLAGS` and `LDFLAGS` to the
compilation and linking recipes in order to support the common patterns of
providing these flags via the command-line and the environment.

Signed-off-by: Jeremy Sowden <azazel@debian.org>
---
 Makefile         |  8 ++++----
 libipvs/Makefile | 12 ++++++------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index d247d4075160..d79f72496000 100644
--- a/Makefile
+++ b/Makefile
@@ -47,9 +47,9 @@ INSTALL		= install
 STATIC_LIBS	= libipvs/libipvs.a
 
 ifeq "${ARCH}" "sparc64"
-    CFLAGS = -Wall -Wunused -Wstrict-prototypes -g -m64 -pipe -mcpu=ultrasparc -mcmodel=medlow
+    DEFAULT_CFLAGS = -Wall -Wunused -Wstrict-prototypes -g -m64 -pipe -mcpu=ultrasparc -mcmodel=medlow
 else
-    CFLAGS = -Wall -Wunused -Wstrict-prototypes -g
+    DEFAULT_CFLAGS = -Wall -Wunused -Wstrict-prototypes -g
 endif
 
 
@@ -88,7 +88,7 @@ libs:
 		make -C libipvs
 
 ipvsadm:	$(OBJS) $(STATIC_LIBS)
-		$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
+		$(CC) $(DEFAULT_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
 
 install:        all
 		if [ ! -d $(SBIN) ]; then $(MKDIR) -p $(SBIN); fi
@@ -140,4 +140,4 @@ debs:
 		dpkg-buildpackage
 
 %.o:	%.c
-		$(CC) $(CFLAGS) $(INCLUDE) $(DEFINES) -c -o $@ $<
+		$(CC) $(DEFAULT_CFLAGS) $(CFLAGS) $(INCLUDE) $(DEFINES) $(CPPFLAGS) -c -o $@ $<
diff --git a/libipvs/Makefile b/libipvs/Makefile
index b31c8eac514d..f29671178422 100644
--- a/libipvs/Makefile
+++ b/libipvs/Makefile
@@ -2,10 +2,10 @@
 
 CC		= gcc
 PKG_CONFIG	?= pkg-config
-CFLAGS		= -Wall -Wunused -Wstrict-prototypes -g -fPIC
+DEFAULT_CFLAGS	= -Wall -Wunused -Wstrict-prototypes -g -fPIC
 ifneq (0,$(HAVE_NL))
-CFLAGS		+= -DLIBIPVS_USE_NL
-CFLAGS		+= $(shell \
+DEFINES		+= -DLIBIPVS_USE_NL
+DEFAULT_CFLAGS	+= $(shell \
 		if which $(PKG_CONFIG) > /dev/null 2>&1; then \
 		  if   $(PKG_CONFIG) --cflags libnl-3.0  2> /dev/null; then :; \
 		  elif $(PKG_CONFIG) --cflags libnl-2.0  2> /dev/null; then :; \
@@ -16,7 +16,7 @@ endif
 
 INCLUDE		+= $(shell if [ -f ../../ip_vs.h ]; then	\
 		     echo "-I../../."; fi;)
-DEFINES		= $(shell if [ ! -f ../../ip_vs.h ]; then	\
+DEFINES		+= $(shell if [ ! -f ../../ip_vs.h ]; then	\
 		    echo "-DHAVE_NET_IP_VS_H"; fi;)
 DEFINES		+= $(shell if which $(PKG_CONFIG) > /dev/null 2>&1; then \
 			 if   $(PKG_CONFIG) --exists libnl-3.0; then :; \
@@ -34,10 +34,10 @@ $(STATIC_LIB):	libipvs.o ip_vs_nl_policy.o
 		ar rv $@ $^
 
 $(SHARED_LIB):	libipvs.o ip_vs_nl_policy.o
-		$(CC) -shared -Wl,-soname,$@ -o $@ $^
+		$(CC) $(DEFAULT_CFLAGS) $(CFLAGS) -shared -Wl,-soname,$@ $(LDFLAGS) -o $@ $^
 
 %.o:		%.c
-		$(CC) $(CFLAGS) $(INCLUDE) $(DEFINES) -c -o $@ $<
+		$(CC) $(DEFAULT_CFLAGS) $(CFLAGS) $(INCLUDE) $(DEFINES) $(CPPFLAGS) -c -o $@ $<
 
 clean:
 		rm -f *.[ao] *~ *.orig *.rej core *.so
-- 
2.45.2

[PATCH ipvsadm v3 5/5] Make sure libipvs.a is built before ipvsadm

[Jeremy Sowden]

There is no explicit rule in the top-level Makefile to build libipvs.a.  It
is built by the phony target `libs`.  However, there is no guarantee of the
order in which the prerequisites of the `all` target are built, so make may
attempt to link ipvsadm to libipvs.a before it has finished building
libipvs.a.

Add a rule to express the dependency of `$(STATIC_LIBS)` on `libs`.

Signed-off-by: Jeremy Sowden <azazel@debian.org>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index d79f72496000..2dda19072365 100644
--- a/Makefile
+++ b/Makefile
@@ -90,6 +90,8 @@ libs:
 ipvsadm:	$(OBJS) $(STATIC_LIBS)
 		$(CC) $(DEFAULT_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
 
+$(STATIC_LIBS): libs
+
 install:        all
 		if [ ! -d $(SBIN) ]; then $(MKDIR) -p $(SBIN); fi
 		$(INSTALL) -m 0755 ipvsadm $(SBIN)
-- 
2.45.2

Re: [PATCH ipvsadm v3 0/5] Debian package fixes

[Julian Anastasov]

	Hello,

On Wed, 15 Jan 2025, Jeremy Sowden wrote:

> The Debian package of ipvsadm carries a few patches.  Some of them have
> been around for years.  None of them is specific to Debian, so I'm
> forwarding them upstream.
> 
> Patch 1 bumps the maximum weight accepted by ipvsadm.
> Patch 2 fixes an unclear usage error message.
> Patches 3-5 make some improvements to the Makefiles.
> 
> Changes since v2:
> 
> * Reformat the commit messages of patches 1, 2 & 5 according to the
>   kernel's preferred patch format.
> 
> Changes since v1:
> 
> * The previous version of patch 2 changed the logic of the option parsing
>   in such a way that using `-M` after `-6` no longer worked correctly.
>   Thisversion just rewords the error message.
> 
> Jeremy Sowden (5):
>   ipvsadm: increase maximum weight value
>   ipvsadm: fix ambiguous usage error message
>   Use variable for pkg-config in Makefiles
>   Support environmental and command-line `*FLAGS` variable in Makefiles
>   Make sure libipvs.a is built before ipvsadm

	Patchset looks good to me, thanks!

Acked-by: Julian Anastasov <ja@ssi.bg>

	Simon, please apply to the ipvsadm tree!

>  Makefile         | 19 +++++++++++--------
>  ipvsadm.8        |  2 +-
>  ipvsadm.c        |  4 ++--
>  libipvs/Makefile | 29 +++++++++++++++--------------
>  4 files changed, 29 insertions(+), 25 deletions(-)
> 
> -- 
> 2.45.2

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH ipvsadm v3 0/5] Debian package fixes

[Simon Horman]

On Thu, Jan 16, 2025 at 08:44:28PM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Wed, 15 Jan 2025, Jeremy Sowden wrote:
> 
> > The Debian package of ipvsadm carries a few patches.  Some of them have
> > been around for years.  None of them is specific to Debian, so I'm
> > forwarding them upstream.
> > 
> > Patch 1 bumps the maximum weight accepted by ipvsadm.
> > Patch 2 fixes an unclear usage error message.
> > Patches 3-5 make some improvements to the Makefiles.
> > 
> > Changes since v2:
> > 
> > * Reformat the commit messages of patches 1, 2 & 5 according to the
> >   kernel's preferred patch format.
> > 
> > Changes since v1:
> > 
> > * The previous version of patch 2 changed the logic of the option parsing
> >   in such a way that using `-M` after `-6` no longer worked correctly.
> >   Thisversion just rewords the error message.
> > 
> > Jeremy Sowden (5):
> >   ipvsadm: increase maximum weight value
> >   ipvsadm: fix ambiguous usage error message
> >   Use variable for pkg-config in Makefiles
> >   Support environmental and command-line `*FLAGS` variable in Makefiles
> >   Make sure libipvs.a is built before ipvsadm
> 
> 	Patchset looks good to me, thanks!
> 
> Acked-by: Julian Anastasov <ja@ssi.bg>
> 
> 	Simon, please apply to the ipvsadm tree!

Thanks Jeremy and Julian, applied.

- Make sure libipvs.a is built before ipvsadm
  https://git.kernel.org/pub/scm/utils/kernel/ipvsadm/ipvsadm.git/commit/3c9cdaef8216
- Support environmental and command-line `*FLAGS` variable in Makefiles
  https://git.kernel.org/pub/scm/utils/kernel/ipvsadm/ipvsadm.git/commit/98d83badc6d7
- Use variable for pkg-config in Makefiles
  https://git.kernel.org/pub/scm/utils/kernel/ipvsadm/ipvsadm.git/commit/0d5e6cdae6ab
- ipvsadm: fix ambiguous usage error message
  https://git.kernel.org/pub/scm/utils/kernel/ipvsadm/ipvsadm.git/commit/fd1836223ab5
- ipvsadm: increase maximum weight value
  https://git.kernel.org/pub/scm/utils/kernel/ipvsadm/ipvsadm.git/commit/78b786958803