[PATCH iproute2 v5 0/7] configure: add support for libdir option

This series add support for the libdir parameter in iproute2 configure
script. The idea is to make use of the fact that packaging systems may
assume that 'configure' comes from autotools allowing a syntax similar
to the autotools one, and using it to tell iproute2 where the distro
expects to find its lib files.

Patches 1-2 fix a parsing issue on current configure options, that may
trigger an endless loop when no value is provided with some options;

Patch 3 fixes a parsing issue bailing out when more than one value is
provided for a single option;

Patch 4 simplifies options parsing, moving semantic checks out of the
while loop processing options;

Patch 5 introduces support for the --opt=value style on current options,
for uniformity;

Patch 6 adds the --prefix option, that may be used by some packaging
systems when calling the configure script;

Patch 7 finally adds the --libdir option, and also drops the static
LIBDIR var from the Makefile.

Changelog:
----------
v4 -> v5
  - bail out when multiple values are provided with a single option
  - simplify option parsing and reduce code duplication, as suggested
    by Phil Sutter
  - remove a nasty eval on libdir option processing

v3 -> v4
  - fix parsing issue on '--include_dir' and '--libbpf_dir'
  - split '--opt value' and '--opt=value' use cases, avoid code
    duplication moving semantic checks on value to dedicated functions

v2 -> v3
  - fix parsing error on prefix and libdir options.

v1 -> v2
  - consolidate '--opt value' and '--opt=value' use cases, as suggested
    by David Ahern.
  - added patch 2 to manage the --prefix option, used by the Debian
    packaging system, as reported by Luca Boccassi, and use it when
    setting lib directory.

Andrea Claudi (7):
  configure: fix parsing issue on include_dir option
  configure: fix parsing issue on libbpf_dir option
  configure: fix parsing issue with more than one value per option
  configure: simplify options parsing
  configure: support --param=value style
  configure: add the --prefix option
  configure: add the --libdir option

 Makefile  |  7 ++---
 configure | 78 +++++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 63 insertions(+), 22 deletions(-)

-- 
2.31.1

configure is stuck in an endless loop if '--include_dir' option is used
without a value:

$ ./configure --include_dir
./configure: line 506: shift: 2: shift count out of range
./configure: line 506: shift: 2: shift count out of range
[...]

Fix it splitting 'shift 2' into two consecutive shifts, and making the
second one conditional to the number of remaining arguments.

A check is also provided after the while loop to verify the include dir
exists; this avoid to produce an erroneous configuration.

Fixes: a9c3d70d902a ("configure: add options ability")
Signed-off-by: Andrea Claudi <redacted>
---
 configure | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 7f4f3bd9..ea9051ab 100755
--- a/configure
+++ b/configure

@@ -485,7 +485,7 @@ usage()
 {
 	cat <<EOF
 Usage: $0 [OPTIONS]
-	--include_dir		Path to iproute2 include dir
+	--include_dir <dir>	Path to iproute2 include dir
 	--libbpf_dir		Path to libbpf DESTDIR
 	--libbpf_force		Enable/disable libbpf by force. Available options:
 				  on: require link against libbpf, quit config if no libbpf support

@@ -502,8 +502,9 @@ else
 	while true; do
 		case "$1" in
 			--include_dir)
-				INCLUDE=$2
-				shift 2 ;;
+				shift
+				INCLUDE="$1"
+				[ "$#" -gt 0 ] && shift ;;
 			--libbpf_dir)
 				LIBBPF_DIR="$2"
 				shift 2 ;;

@@ -523,6 +524,8 @@ else
 	done
 fi
 
+[ -d "$INCLUDE" ] || usage 1
+
 echo "# Generated config based on" $INCLUDE >$CONFIG
 quiet_config >> $CONFIG
 

-- 
2.31.1

configure is stuck in an endless loop if '--libbpf_dir' option is used
without a value:

$ ./configure --libbpf_dir
./configure: line 515: shift: 2: shift count out of range
./configure: line 515: shift: 2: shift count out of range
[...]

Fix it splitting 'shift 2' into two consecutive shifts, and making the
second one conditional to the number of remaining arguments.

A check is also provided after the while loop to verify the libbpf dir
exists; also, as LIBBPF_DIR does not have a default value, configure bails
out if the user does not specify a value after --libbpf_dir, thus avoiding
to produce an erroneous configuration.

Fixes: 7ae2585b865a ("configure: convert LIBBPF environment variables to command-line options")
Signed-off-by: Andrea Claudi <redacted>
---
 configure | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index ea9051ab..0f304206 100755
--- a/configure
+++ b/configure

@@ -486,7 +486,7 @@ usage()
 	cat <<EOF
 Usage: $0 [OPTIONS]
 	--include_dir <dir>	Path to iproute2 include dir
-	--libbpf_dir		Path to libbpf DESTDIR
+	--libbpf_dir <dir>	Path to libbpf DESTDIR
 	--libbpf_force		Enable/disable libbpf by force. Available options:
 				  on: require link against libbpf, quit config if no libbpf support
 				  off: disable libbpf probing

@@ -506,8 +506,9 @@ else
 				INCLUDE="$1"
 				[ "$#" -gt 0 ] && shift ;;
 			--libbpf_dir)
-				LIBBPF_DIR="$2"
-				shift 2 ;;
+				shift
+				LIBBPF_DIR="$1"
+				[ "$#" -gt 0 ] && shift ;;
 			--libbpf_force)
 				if [ "$2" != 'on' ] && [ "$2" != 'off' ]; then
 					usage 1

@@ -525,6 +526,9 @@ else
 fi
 
 [ -d "$INCLUDE" ] || usage 1
+if [ "${LIBBPF_DIR-unused}" != "unused" ]; then
+	[ -d "$LIBBPF_DIR" ] || usage 1
+fi
 
 echo "# Generated config based on" $INCLUDE >$CONFIG
 quiet_config >> $CONFIG

-- 
2.31.1

With commit a9c3d70d902a ("configure: add options ability") users are no
more able to provide wrong command lines like:

$ ./configure --include_dir foo bar

The script simply bails out when user provides more than one value for a
single option. However, in doing so, it breaks backward compatibility with
some packaging system, which expects unknown options to be ignored.

Commit a3272b93725a ("configure: restore backward compatibility") fix this
issue, but makes it possible again for users to provide wrong command lines
such as the one above.

This fixes the issue simply ignoring autoconf-like options such as
'--opt=value'.

Fixes: a3272b93725a ("configure: restore backward compatibility")
Signed-off-by: Andrea Claudi <redacted>
---
 configure | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 0f304206..9ec19a5b 100755
--- a/configure
+++ b/configure

@@ -517,10 +517,12 @@ else
 				shift 2 ;;
 			-h | --help)
 				usage 0 ;;
+			--*)
+				shift ;;
 			"")
 				break ;;
 			*)
-				shift 1 ;;
+				usage 1 ;;
 		esac
 	done
 fi

-- 
2.31.1

This commit simplifies options parsing moving all the code not related to
parsing out of the case statement.

- The conditional shift after the assignments is moved right after the
  case, reducing code duplication.
- The semantic checks on the LIBBPF_FORCE value is moved after the loop
  like we already did for INCLUDE and LIBBPF_DIR.
- Finally, the loop condition is changed to check remaining arguments, thus
  making it possible to get rid of the null string case break.

As a bonus, now the help message states that on or off should follow
--libbpf_force

Signed-off-by: Andrea Claudi <redacted>
---
 configure | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/configure b/configure
index 9ec19a5b..26e06eb8 100755
--- a/configure
+++ b/configure

@@ -485,12 +485,12 @@ usage()
 {
 	cat <<EOF
 Usage: $0 [OPTIONS]
-	--include_dir <dir>	Path to iproute2 include dir
-	--libbpf_dir <dir>	Path to libbpf DESTDIR
-	--libbpf_force		Enable/disable libbpf by force. Available options:
-				  on: require link against libbpf, quit config if no libbpf support
-				  off: disable libbpf probing
-	-h | --help		Show this usage info
+	--include_dir <dir>		Path to iproute2 include dir
+	--libbpf_dir <dir>		Path to libbpf DESTDIR
+	--libbpf_force <on|off>		Enable/disable libbpf by force. Available options:
+					  on: require link against libbpf, quit config if no libbpf support
+					  off: disable libbpf probing
+	-h | --help			Show this usage info
 EOF
 	exit $1
 }

@@ -499,31 +499,25 @@ EOF
 if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then
 	INCLUDE="$1"
 else
-	while true; do
+	while [ "$#" -gt 0 ]; do
 		case "$1" in
 			--include_dir)
 				shift
-				INCLUDE="$1"
-				[ "$#" -gt 0 ] && shift ;;
+				INCLUDE="$1" ;;
 			--libbpf_dir)
 				shift
-				LIBBPF_DIR="$1"
-				[ "$#" -gt 0 ] && shift ;;
+				LIBBPF_DIR="$1" ;;
 			--libbpf_force)
-				if [ "$2" != 'on' ] && [ "$2" != 'off' ]; then
-					usage 1
-				fi
-				LIBBPF_FORCE=$2
-				shift 2 ;;
+				shift
+				LIBBPF_FORCE="$1" ;;
 			-h | --help)
 				usage 0 ;;
 			--*)
-				shift ;;
-			"")
-				break ;;
+				;;
 			*)
 				usage 1 ;;
 		esac
+		[ "$#" -gt 0 ] && shift
 	done
 fi
 

@@ -531,6 +525,11 @@ fi
 if [ "${LIBBPF_DIR-unused}" != "unused" ]; then
 	[ -d "$LIBBPF_DIR" ] || usage 1
 fi
+if [ "${LIBBPF_FORCE-unused}" != "unused" ]; then
+	if [ "$LIBBPF_FORCE" != 'on' ] && [ "$LIBBPF_FORCE" != 'off' ]; then
+		usage 1
+	fi
+fi
 
 echo "# Generated config based on" $INCLUDE >$CONFIG
 quiet_config >> $CONFIG

-- 
2.31.1

This commit makes it possible to specify values for configure params
using the common autotools configure syntax '--param=value'.

Signed-off-by: Andrea Claudi <redacted>
---
 configure | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/configure b/configure
index 26e06eb8..9a2645d9 100755
--- a/configure
+++ b/configure

@@ -504,12 +504,18 @@ else
 			--include_dir)
 				shift
 				INCLUDE="$1" ;;
+			--include_dir=*)
+				INCLUDE="${1#*=}" ;;
 			--libbpf_dir)
 				shift
 				LIBBPF_DIR="$1" ;;
+			--libbpf_dir=*)
+				LIBBPF_DIR="${1#*=}" ;;
 			--libbpf_force)
 				shift
 				LIBBPF_FORCE="$1" ;;
+			--libbpf_force=*)
+				LIBBPF_FORCE="${1#*=}" ;;
 			-h | --help)
 				usage 0 ;;
 			--*)

-- 
2.31.1

This commit add the '--prefix' option to the iproute2 configure script.

This mimics the '--prefix' option that autotools configure provides, and
will be used later to allow users or packagers to set the lib directory.

Signed-off-by: Andrea Claudi <redacted>
---
 configure | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/configure b/configure
index 9a2645d9..05e23eff 100755
--- a/configure
+++ b/configure

@@ -3,6 +3,7 @@
 # This is not an autoconf generated configure
 
 INCLUDE="$PWD/include"
+PREFIX="/usr"
 
 # Output file which is input to Makefile
 CONFIG=config.mk

@@ -490,6 +491,7 @@ Usage: $0 [OPTIONS]
 	--libbpf_force <on|off>		Enable/disable libbpf by force. Available options:
 					  on: require link against libbpf, quit config if no libbpf support
 					  off: disable libbpf probing
+	--prefix <dir>			Path prefix of the lib files to install
 	-h | --help			Show this usage info
 EOF
 	exit $1

@@ -516,6 +518,11 @@ else
 				LIBBPF_FORCE="$1" ;;
 			--libbpf_force=*)
 				LIBBPF_FORCE="${1#*=}" ;;
+			--prefix)
+				shift
+				PREFIX="$1" ;;
+			--prefix=*)
+				PREFIX="${1#*=}" ;;
 			-h | --help)
 				usage 0 ;;
 			--*)

@@ -536,6 +543,7 @@ if [ "${LIBBPF_FORCE-unused}" != "unused" ]; then
 		usage 1
 	fi
 fi
+[ -z "$PREFIX" ] && usage 1
 
 echo "# Generated config based on" $INCLUDE >$CONFIG
 quiet_config >> $CONFIG

-- 
2.31.1

This commit allows users/packagers to choose a lib directory to store
iproute2 lib files.

At the moment iproute2 ship lib files in /usr/lib and offers no way to
modify this setting. However, according to the FHS, distros may choose
"one or more variants of the /lib directory on systems which support
more than one binary format" (e.g. /usr/lib64 on Fedora).

As Luca states in commit a3272b93725a ("configure: restore backward
compatibility"), packaging systems may assume that 'configure' is from
autotools, and try to pass it some parameters.

Allowing the '--libdir=/path/to/libdir' syntax, we can use this to our
advantage, and let the lib directory to be chosen by the distro
packaging system.

Note that LIBDIR uses "\${prefix}/lib" as default value because autoconf
allows this to be expanded to the --prefix value at configure runtime.
"\${prefix}" is replaced with the PREFIX value in check_lib_dir().

Signed-off-by: Andrea Claudi <redacted>
---
 Makefile  |  7 ++++---
 configure | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 5eddd504..f6214534 100644
--- a/Makefile
+++ b/Makefile

@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Top level Makefile for iproute2
 
+-include config.mk
+
 ifeq ("$(origin V)", "command line")
 VERBOSE = $(V)
 endif

@@ -13,7 +15,6 @@ MAKEFLAGS += --no-print-directory
 endif
 
 PREFIX?=/usr
-LIBDIR?=$(PREFIX)/lib
 SBINDIR?=/sbin
 CONFDIR?=/etc/iproute2
 NETNS_RUN_DIR?=/var/run/netns

@@ -69,7 +70,7 @@ SUBDIRS=lib ip tc bridge misc netem genl tipc devlink rdma dcb man vdpa
 LIBNETLINK=../lib/libutil.a ../lib/libnetlink.a
 LDLIBS += $(LIBNETLINK)
 
-all: config
+all: config.mk
 	@set -e; \
 	for i in $(SUBDIRS); \
 	do echo; echo $$i; $(MAKE) -C $$i; done

@@ -89,7 +90,7 @@ help:
 	@echo "Make Arguments:"
 	@echo " V=[0|1]             - set build verbosity level"
 
-config:
+config.mk:
 	@if [ ! -f config.mk -o configure -nt config.mk ]; then \
 		sh configure $(KERNEL_INCLUDE); \
 	fi

diff --git a/configure b/configure
index 05e23eff..8ddff43c 100755
--- a/configure
+++ b/configure

@@ -4,6 +4,7 @@
 
 INCLUDE="$PWD/include"
 PREFIX="/usr"
+LIBDIR="\${prefix}/lib"
 
 # Output file which is input to Makefile
 CONFIG=config.mk

@@ -149,6 +150,15 @@ EOF
 	rm -f $TMPDIR/ipttest.c $TMPDIR/ipttest
 }
 
+check_lib_dir()
+{
+	LIBDIR=$(echo $LIBDIR | sed "s|\${prefix}|$PREFIX|")
+
+	echo -n "lib directory: "
+	echo "$LIBDIR"
+	echo "LIBDIR:=$LIBDIR" >> $CONFIG
+}
+
 check_ipt()
 {
 	if ! grep TC_CONFIG_XT $CONFIG > /dev/null; then

@@ -487,6 +497,7 @@ usage()
 	cat <<EOF
 Usage: $0 [OPTIONS]
 	--include_dir <dir>		Path to iproute2 include dir
+	--libdir <dir>			Path to iproute2 lib dir
 	--libbpf_dir <dir>		Path to libbpf DESTDIR
 	--libbpf_force <on|off>		Enable/disable libbpf by force. Available options:
 					  on: require link against libbpf, quit config if no libbpf support

@@ -508,6 +519,11 @@ else
 				INCLUDE="$1" ;;
 			--include_dir=*)
 				INCLUDE="${1#*=}" ;;
+			--libdir)
+				shift
+				LIBDIR="$1" ;;
+			--libdir=*)
+				LIBDIR="${1#*=}" ;;
 			--libbpf_dir)
 				shift
 				LIBBPF_DIR="$1" ;;

@@ -544,6 +560,7 @@ if [ "${LIBBPF_FORCE-unused}" != "unused" ]; then
 	fi
 fi
 [ -z "$PREFIX" ] && usage 1
+[ -z "$LIBDIR" ] && usage 1
 
 echo "# Generated config based on" $INCLUDE >$CONFIG
 quiet_config >> $CONFIG

@@ -568,6 +585,7 @@ if ! grep -q TC_CONFIG_NO_XT $CONFIG; then
 fi
 
 echo
+check_lib_dir
 if ! grep -q TC_CONFIG_NO_XT $CONFIG; then
 	echo -n "iptables modules directory: "
 	check_ipt_lib_dir

-- 
2.31.1

Hi Andrea,

On Thu, Oct 14, 2021 at 10:50:55AM +0200, Andrea Claudi wrote:
[...]

Assuming config.mk may be missing (as dash-prefix is used).

Dropping this leads to trouble if config.mk is missing or didn't define
it. Can't you just leave it in place? Usually config.mk would override
it anyway, no?

Rest of the series looks great, thanks a lot for spending the extra
cycles giving it the mirror finish it has now. :)

Cheers, Phil

On Thu, Oct 14, 2021 at 12:10:53PM +0200, Phil Sutter wrote:

Hi Andrea,

On Thu, Oct 14, 2021 at 10:50:55AM +0200, Andrea Claudi wrote:
[...]

quoted

diff --git a/Makefile b/Makefile
index 5eddd504..f6214534 100644
--- a/Makefile
+++ b/Makefile

@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Top level Makefile for iproute2
 
+-include config.mk
+

Assuming config.mk may be missing (as dash-prefix is used).

quoted

 ifeq ("$(origin V)", "command line")
 VERBOSE = $(V)
 endif

@@ -13,7 +15,6 @@ MAKEFLAGS += --no-print-directory
 endif
 
 PREFIX?=/usr
-LIBDIR?=$(PREFIX)/lib

Dropping this leads to trouble if config.mk is missing or didn't define
it. Can't you just leave it in place? Usually config.mk would override
it anyway, no?

config.mk may miss at the first make call, but the "all" target calls
config.mk, which in turns re-generate it. Thus LIBDIR is defined when
the target all executes.

Also, LIBDIR must be defined in config.mk, as a default value for it is
provided in configure, and will be used if the user does not provide it
at command line.

I verified this deleting config.mk and printing DEFINES in Makefile to
verify it includes the correct path for LIBDIR.

On Thu, Oct 14, 2021 at 01:02:41PM +0200, Andrea Claudi wrote:

On Thu, Oct 14, 2021 at 12:10:53PM +0200, Phil Sutter wrote:

quoted

Hi Andrea,

On Thu, Oct 14, 2021 at 10:50:55AM +0200, Andrea Claudi wrote:
[...]

quoted

diff --git a/Makefile b/Makefile
index 5eddd504..f6214534 100644
--- a/Makefile
+++ b/Makefile

@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Top level Makefile for iproute2
 
+-include config.mk
+

Assuming config.mk may be missing (as dash-prefix is used).

quoted

 ifeq ("$(origin V)", "command line")
 VERBOSE = $(V)
 endif

@@ -13,7 +15,6 @@ MAKEFLAGS += --no-print-directory
 endif
 
 PREFIX?=/usr
-LIBDIR?=$(PREFIX)/lib

Dropping this leads to trouble if config.mk is missing or didn't define
it. Can't you just leave it in place? Usually config.mk would override
it anyway, no?

config.mk may miss at the first make call, but the "all" target calls
config.mk, which in turns re-generate it. Thus LIBDIR is defined when
the target all executes.

Ah, I forgot the call to configure from make. So full series:

Acked-by: Phil Sutter <phil@nwl.cc>

Thanks, Phil

On 10/14/21 2:50 AM, Andrea Claudi wrote:

This series add support for the libdir parameter in iproute2 configure
script. The idea is to make use of the fact that packaging systems may
assume that 'configure' comes from autotools allowing a syntax similar
to the autotools one, and using it to tell iproute2 where the distro
expects to find its lib files.

Patches 1-2 fix a parsing issue on current configure options, that may
trigger an endless loop when no value is provided with some options;

Patch 3 fixes a parsing issue bailing out when more than one value is
provided for a single option;

Patch 4 simplifies options parsing, moving semantic checks out of the
while loop processing options;

Patch 5 introduces support for the --opt=value style on current options,
for uniformity;

Patch 6 adds the --prefix option, that may be used by some packaging
systems when calling the configure script;

Patch 7 finally adds the --libdir option, and also drops the static
LIBDIR var from the Makefile.

applied to net-next. Thanks for working on this.

On 10/15/21 6:02 PM, David Ahern wrote:

applied to net-next. Thanks for working on this.

sigh. iproute2-next