[ipset PATCH 0/4] Some testsuite improvements

[ipset PATCH 0/4] Some testsuite improvements

[Phil Sutter]

Patch 1 fixes the reason why xlate testuite failed for me - it was
simply not testing the right binary. Make it adhere to what the regular
testsuite does by calling the built ipset tool instead of the installed
one.

Patch 2 is just bonus, the idea for it came from a "does this even work"
sanity check while debugging the above.

Patch 3 fixes for missing 'netmask' tool on my system. Not entirely
satisfying though, there's no 'sendip', either (but the testsuite may
run without).

Patch 4 avoids a spurious testsuite failure for me. Not sure if it's a
good solution or will just move the spurious failure to others' systems.

Phil Sutter (4):
  tests: xlate: Test built binary by default
  tests: xlate: Make test input valid
  tests: cidr.sh: Add ipcalc fallback
  tests: hash:ip,port.t: 'vrrp' is printed as 'carp'

 tests/cidr.sh               | 32 ++++++++++++++++++++++++++++----
 tests/hash:ip,port.t.list2  |  2 +-
 tests/xlate/ipset-translate |  1 +
 tests/xlate/runtest.sh      | 14 ++++++++++----
 tests/xlate/xlate.t         |  6 +++---
 tests/xlate/xlate.t.nft     |  4 ++--
 6 files changed, 45 insertions(+), 14 deletions(-)
 create mode 120000 tests/xlate/ipset-translate

-- 
2.38.0

[ipset PATCH 1/4] tests: xlate: Test built binary by default

[Phil Sutter]

Testing the host's iptables-translate by default is unintuitive. Since
the ipset-translate symlink is created upon 'make install', add a local
symlink to the repository pointing at a built binary in src/. Using this
by default is consistent with the regular testsuite.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/xlate/ipset-translate | 1 +
 tests/xlate/runtest.sh      | 8 ++------
 2 files changed, 3 insertions(+), 6 deletions(-)
 create mode 120000 tests/xlate/ipset-translate

diff --git a/tests/xlate/ipset-translate b/tests/xlate/ipset-translate
new file mode 120000
index 0000000000000..91980c18bb040
--- /dev/null
+++ b/tests/xlate/ipset-translate
@@ -0,0 +1 @@
+../../src/ipset
\ No newline at end of file
diff --git a/tests/xlate/runtest.sh b/tests/xlate/runtest.sh
index a2a02c05d7573..6a2f80c0d9e61 100755
--- a/tests/xlate/runtest.sh
+++ b/tests/xlate/runtest.sh
@@ -6,14 +6,10 @@ if [ ! -x "$DIFF" ] ; then
 	exit 1
 fi
 
-IPSET_XLATE=$(which ipset-translate)
-if [ ! -x "$IPSET_XLATE" ] ; then
-	echo "ERROR: ipset-translate is not installed yet"
-	exit 1
-fi
+ipset_xlate=${IPSET_XLATE_BIN:-$(dirname $0)/ipset-translate}
 
 TMP=$(mktemp)
-ipset-translate restore < xlate.t &> $TMP
+$ipset_xlate restore < xlate.t &> $TMP
 if [ $? -ne 0 ]
 then
 	cat $TMP
-- 
2.38.0

[ipset PATCH 2/4] tests: xlate: Make test input valid

[Phil Sutter]

Make sure ipset at least accepts the test input by running it against
plain ipset once for sanity. This exposed two issues:

* Set 'hip5' doesn't have comment support, so add the commented elements
  to 'hip6' instead (likely a typo).
* Set 'bip1' range 2.0.0.1-2.1.0.1 exceeds the max allowed for bitmap
  sets. Reduce it accordingly.

Fixes: 7587d1c4b5465 ("tests: add tests ipset to nftables")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/xlate/runtest.sh  | 10 ++++++++++
 tests/xlate/xlate.t     |  6 +++---
 tests/xlate/xlate.t.nft |  4 ++--
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/tests/xlate/runtest.sh b/tests/xlate/runtest.sh
index 6a2f80c0d9e61..8b42f0b414d72 100755
--- a/tests/xlate/runtest.sh
+++ b/tests/xlate/runtest.sh
@@ -6,8 +6,18 @@ if [ ! -x "$DIFF" ] ; then
 	exit 1
 fi
 
+ipset=${IPSET_BIN:-../../src/ipset}
 ipset_xlate=${IPSET_XLATE_BIN:-$(dirname $0)/ipset-translate}
 
+$ipset restore < xlate.t
+rc=$?
+$ipset destroy
+if [ $rc -ne 0 ]
+then
+	echo -e "[\033[0;31mERROR\033[0m] invalid test input"
+	exit 1
+fi
+
 TMP=$(mktemp)
 $ipset_xlate restore < xlate.t &> $TMP
 if [ $? -ne 0 ]
diff --git a/tests/xlate/xlate.t b/tests/xlate/xlate.t
index f09cb202bb6c0..38cbc787bb854 100644
--- a/tests/xlate/xlate.t
+++ b/tests/xlate/xlate.t
@@ -11,8 +11,8 @@ add hip4 192.168.10.0
 create hip5 hash:ip maxelem 24
 add hip5 192.168.10.0
 create hip6 hash:ip comment
-add hip5 192.168.10.1
-add hip5 192.168.10.2 comment "this is a comment"
+add hip6 192.168.10.1
+add hip6 192.168.10.2 comment "this is a comment"
 create ipp1 hash:ip,port
 add ipp1 192.168.10.1,0
 add ipp1 192.168.10.2,5
@@ -23,7 +23,7 @@ create ipp3 hash:ip,port counters
 add ipp3 192.168.10.3,20 packets 5 bytes 3456
 create ipp4 hash:ip,port timeout 4 counters
 add ipp4 192.168.10.3,20 packets 5 bytes 3456
-create bip1 bitmap:ip range 2.0.0.1-2.1.0.1 timeout 5
+create bip1 bitmap:ip range 2.0.0.1-2.0.1.1 timeout 5
 create bip2 bitmap:ip range 10.0.0.0/8 netmask 24 timeout 5
 add bip2 10.10.10.0
 add bip2 10.10.20.0 timeout 12
diff --git a/tests/xlate/xlate.t.nft b/tests/xlate/xlate.t.nft
index 0152a30811258..8fb2a29b9c79f 100644
--- a/tests/xlate/xlate.t.nft
+++ b/tests/xlate/xlate.t.nft
@@ -12,8 +12,8 @@ add element inet global hip4 { 192.168.10.0/24 }
 add set inet global hip5 { type ipv4_addr; size 24; }
 add element inet global hip5 { 192.168.10.0 }
 add set inet global hip6 { type ipv4_addr; }
-add element inet global hip5 { 192.168.10.1 }
-add element inet global hip5 { 192.168.10.2 comment "this is a comment" }
+add element inet global hip6 { 192.168.10.1 }
+add element inet global hip6 { 192.168.10.2 comment "this is a comment" }
 add set inet global ipp1 { type ipv4_addr . inet_proto . inet_service; }
 add element inet global ipp1 { 192.168.10.1 . tcp . 0 }
 add element inet global ipp1 { 192.168.10.2 . tcp . 5 }
-- 
2.38.0

[ipset PATCH 3/4] tests: cidr.sh: Add ipcalc fallback

[Phil Sutter]

If netmask is not available, ipcalc may be a viable replacement.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/cidr.sh | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/tests/cidr.sh b/tests/cidr.sh
index b7d695ae7c0b3..2c4d9399f02dc 100755
--- a/tests/cidr.sh
+++ b/tests/cidr.sh
@@ -37,6 +37,30 @@ NETS="0.0.0.0/1
 
 ipset="../src/ipset"
 
+if which netmask >/dev/null 2>&1; then
+	net_first_addr() {
+		netmask -r $1 | cut -d - -f 1
+	}
+	net_last_addr() {
+		netmask -r $1 | cut -d - -f 2 | cut -d ' ' -f 1
+	}
+elif which ipcalc >/dev/null 2>&1; then
+	net_first_addr() {
+		ipcalc $1 | awk '/^Address:/{print $2}'
+	}
+	net_last_addr() {
+		# Netmask tool prints broadcast address as last one, so
+		# prefer that instead of HostMax. Also fix for /31 and /32
+		# being recognized as special by ipcalc.
+		ipcalc $1 | awk '/^(Hostroute|HostMax):/{out=$2}
+				 /^Broadcast:/{out=$2}
+				 END{print out}'
+	}
+else
+	echo "need either netmask or ipcalc tools"
+	exit 1
+fi
+
 case "$1" in
 net)
     $ipset n test hash:net
@@ -46,9 +70,9 @@ net)
     done <<<"$NETS"
 
     while IFS= read x; do
-    	first=`netmask -r $x | cut -d - -f 1`
+    	first=`net_first_addr $x`
     	$ipset test test $first >/dev/null 2>&1
-    	last=`netmask -r $x | cut -d - -f 2 | cut -d ' ' -f 1`
+    	last=`net_last_addr $x`
     	$ipset test test $last >/dev/null 2>&1
     done <<<"$NETS"
 
@@ -67,9 +91,9 @@ net,port)
 
     n=1
     while IFS= read x; do
-    	first=`netmask -r $x | cut -d - -f 1`
+    	first=`net_first_addr $x`
     	$ipset test test $first,$n >/dev/null 2>&1
-    	last=`netmask -r $x | cut -d - -f 2 | cut -d ' ' -f 1`
+    	last=`net_last_addr $x`
     	$ipset test test $last,$n >/dev/null 2>&1
     	n=$((n+1))
     done <<<"$NETS"
-- 
2.38.0

[ipset PATCH 4/4] tests: hash:ip,port.t: 'vrrp' is printed as 'carp'

[Phil Sutter]

| % grep vrrp /etc/protocols
| carp	112	CARP	vrrp		# Common Address Redundancy Protocol

Nowadays, carp seems to be the preferred name for protocol 112. Simply
change the expected output for lack of idea for a backwards compatible
change which is not simply using another protocol.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/hash:ip,port.t.list2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/hash:ip,port.t.list2 b/tests/hash:ip,port.t.list2
index ffaedb561eb1c..0c5d3a15ef369 100644
--- a/tests/hash:ip,port.t.list2
+++ b/tests/hash:ip,port.t.list2
@@ -6,6 +6,6 @@ Size in memory: 480
 References: 0
 Number of entries: 3
 Members:
+2.0.0.1,carp:0
 2.0.0.1,tcp:80
 2.0.0.1,udp:80
-2.0.0.1,vrrp:0
-- 
2.38.0

Re: [ipset PATCH 0/4] Some testsuite improvements

[Pablo Neira Ayuso]

On Tue, Mar 07, 2023 at 02:58:08PM +0100, Phil Sutter wrote:
> Patch 1 fixes the reason why xlate testuite failed for me - it was
> simply not testing the right binary. Make it adhere to what the regular
> testsuite does by calling the built ipset tool instead of the installed
> one.
> 
> Patch 2 is just bonus, the idea for it came from a "does this even work"
> sanity check while debugging the above.
> 
> Patch 3 fixes for missing 'netmask' tool on my system. Not entirely
> satisfying though, there's no 'sendip', either (but the testsuite may
> run without).
> 
> Patch 4 avoids a spurious testsuite failure for me. Not sure if it's a
> good solution or will just move the spurious failure to others' systems.

LGTM

Re: [ipset PATCH 0/4] Some testsuite improvements

[Phil Sutter]

On Tue, Mar 07, 2023 at 02:58:08PM +0100, Phil Sutter wrote:
> Patch 1 fixes the reason why xlate testuite failed for me - it was
> simply not testing the right binary. Make it adhere to what the regular
> testsuite does by calling the built ipset tool instead of the installed
> one.
> 
> Patch 2 is just bonus, the idea for it came from a "does this even work"
> sanity check while debugging the above.
> 
> Patch 3 fixes for missing 'netmask' tool on my system. Not entirely
> satisfying though, there's no 'sendip', either (but the testsuite may
> run without).
> 
> Patch 4 avoids a spurious testsuite failure for me. Not sure if it's a
> good solution or will just move the spurious failure to others' systems.
> 
> Phil Sutter (4):
>   tests: xlate: Test built binary by default
>   tests: xlate: Make test input valid
>   tests: cidr.sh: Add ipcalc fallback
>   tests: hash:ip,port.t: 'vrrp' is printed as 'carp'

Series applied.