netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [iproute PATCH 0/3] Fix and test ssfilter
@ 2018-08-14 12:18 Phil Sutter
  2018-08-14 12:18 ` [iproute PATCH 1/3] ss: Review ssfilter Phil Sutter
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Phil Sutter @ 2018-08-14 12:18 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Samuel Mannehed

This series contains a fix for ssfilter and introduces a testscript to
verify correct functionality.

Phil Sutter (3):
  ss: Review ssfilter
  testsuite: Prepare for ss tests
  testsuite: Add a first ss test validating ssfilter

 misc/ssfilter.y               |  36 ++++++++++++++-----------
 testsuite/Makefile            |   2 +-
 testsuite/lib/generic.sh      |  37 ++++++++++----------------
 testsuite/tests/ss/ss1.dump   | Bin 0 -> 720 bytes
 testsuite/tests/ss/ssfilter.t |  48 ++++++++++++++++++++++++++++++++++
 5 files changed, 84 insertions(+), 39 deletions(-)
 create mode 100644 testsuite/tests/ss/ss1.dump
 create mode 100755 testsuite/tests/ss/ssfilter.t

-- 
2.18.0

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

* [iproute PATCH 1/3] ss: Review ssfilter
  2018-08-14 12:18 [iproute PATCH 0/3] Fix and test ssfilter Phil Sutter
@ 2018-08-14 12:18 ` Phil Sutter
  2018-08-14 12:18 ` [iproute PATCH 2/3] testsuite: Prepare for ss tests Phil Sutter
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Phil Sutter @ 2018-08-14 12:18 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Samuel Mannehed

The original problem was ssfilter rejecting single expressions if
enclosed in braces, such as:

| sport = 22 or ( dport = 22 )

This is fixed by allowing 'expr' to be an 'exprlist' enclosed in braces.
The no longer required recursion in 'exprlist' being an 'exprlist'
enclosed in braces is dropped.

In addition to that, a few other things are changed:

* Remove pointless 'null' prefix in 'appled' before 'exprlist'.
* For simple equals matches, '=' operator was required for ports but not
  allowed for hosts. Make this consistent by making '=' operator
  optional in both cases.

Reported-by: Samuel Mannehed <samuel@cendio.se>
Fixes: b2038cc0b2403 ("ssfilter: Eliminate shift/reduce conflicts")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ssfilter.y | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/misc/ssfilter.y b/misc/ssfilter.y
index 88d4229a9b241..0413dddaa7584 100644
--- a/misc/ssfilter.y
+++ b/misc/ssfilter.y
@@ -42,24 +42,22 @@ static void yyerror(char *s)
 %nonassoc '!'
 
 %%
-applet: null exprlist
+applet: exprlist
         {
-                *yy_ret = $2;
-                $$ = $2;
+                *yy_ret = $1;
+                $$ = $1;
         }
         | null
         ;
+
 null:   /* NOTHING */ { $$ = NULL; }
         ;
+
 exprlist: expr
         | '!' expr
         {
                 $$ = alloc_node(SSF_NOT, $2);
         }
-        | '(' exprlist ')'
-        {
-                $$ = $2;
-        }
         | exprlist '|' expr
         {
                 $$ = alloc_node(SSF_OR, $1);
@@ -77,13 +75,21 @@ exprlist: expr
         }
         ;
 
-expr:	DCOND HOSTCOND
+eq:	'='
+	| /* nothing */
+	;
+
+expr:	'(' exprlist ')'
+	{
+		$$ = $2;
+	}
+	| DCOND eq HOSTCOND
         {
-		$$ = alloc_node(SSF_DCOND, $2);
+		$$ = alloc_node(SSF_DCOND, $3);
         }
-        | SCOND HOSTCOND
+        | SCOND eq HOSTCOND
         {
-		$$ = alloc_node(SSF_SCOND, $2);
+		$$ = alloc_node(SSF_SCOND, $3);
         }
         | DPORT GEQ HOSTCOND
         {
@@ -101,7 +107,7 @@ expr:	DCOND HOSTCOND
         {
                 $$ = alloc_node(SSF_NOT, alloc_node(SSF_D_GE, $3));
         }
-        | DPORT '=' HOSTCOND
+        | DPORT eq HOSTCOND
         {
 		$$ = alloc_node(SSF_DCOND, $3);
         }
@@ -126,7 +132,7 @@ expr:	DCOND HOSTCOND
         {
                 $$ = alloc_node(SSF_NOT, alloc_node(SSF_S_GE, $3));
         }
-        | SPORT '=' HOSTCOND
+        | SPORT eq HOSTCOND
         {
 		$$ = alloc_node(SSF_SCOND, $3);
         }
@@ -134,7 +140,7 @@ expr:	DCOND HOSTCOND
         {
 		$$ = alloc_node(SSF_NOT, alloc_node(SSF_SCOND, $3));
         }
-        | DEVNAME '=' DEVCOND
+        | DEVNAME eq DEVCOND
         {
 		$$ = alloc_node(SSF_DEVCOND, $3);
         }
@@ -142,7 +148,7 @@ expr:	DCOND HOSTCOND
         {
 		$$ = alloc_node(SSF_NOT, alloc_node(SSF_DEVCOND, $3));
         }
-        | FWMARK '=' MARKMASK
+        | FWMARK eq MARKMASK
         {
                 $$ = alloc_node(SSF_MARKMASK, $3);
         }
-- 
2.18.0

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

* [iproute PATCH 2/3] testsuite: Prepare for ss tests
  2018-08-14 12:18 [iproute PATCH 0/3] Fix and test ssfilter Phil Sutter
  2018-08-14 12:18 ` [iproute PATCH 1/3] ss: Review ssfilter Phil Sutter
@ 2018-08-14 12:18 ` Phil Sutter
  2018-08-14 12:18 ` [iproute PATCH 3/3] testsuite: Add a first ss test validating ssfilter Phil Sutter
  2018-08-15 21:33 ` [iproute PATCH 0/3] Fix and test ssfilter Stephen Hemminger
  3 siblings, 0 replies; 5+ messages in thread
From: Phil Sutter @ 2018-08-14 12:18 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Samuel Mannehed

This merges the shared bits from ts_tc() and ts_ip() into a common
function for being wrapped by the first ones and adds a third ts_ss()
for testing ss commands.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 testsuite/Makefile       |  2 +-
 testsuite/lib/generic.sh | 37 ++++++++++++++-----------------------
 2 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/testsuite/Makefile b/testsuite/Makefile
index 2a54e5c845e65..8fcbc557ff9a7 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -65,7 +65,7 @@ endif
 		TMP_ERR=`mktemp /tmp/tc_testsuite.XXXXXX`; \
 		TMP_OUT=`mktemp /tmp/tc_testsuite.XXXXXX`; \
 		STD_ERR="$$TMP_ERR" STD_OUT="$$TMP_OUT" \
-		TC="$$i/tc/tc" IP="$$i/ip/ip" DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
+		TC="$$i/tc/tc" IP="$$i/ip/ip" SS=$$i/misc/ss DEV="$(DEV)" IPVER="$@" SNAME="$$i" \
 		ERRF="$(RESULTS_DIR)/$@.$$o.err" $(KENV) $(PREFIX) tests/$@ > $(RESULTS_DIR)/$@.$$o.out; \
 		if [ "$$?" = "127" ]; then \
 			echo "SKIPPED"; \
diff --git a/testsuite/lib/generic.sh b/testsuite/lib/generic.sh
index 8cef20fa1b280..f92260fc40cf3 100644
--- a/testsuite/lib/generic.sh
+++ b/testsuite/lib/generic.sh
@@ -26,16 +26,17 @@ ts_skip()
     exit 127
 }
 
-ts_tc()
+__ts_cmd()
 {
+	CMD=$1; shift
 	SCRIPT=$1; shift
 	DESC=$1; shift
 
-	$TC $@ 2> $STD_ERR > $STD_OUT
+	$CMD $@ 2> $STD_ERR > $STD_OUT
 
 	if [ -s $STD_ERR ]; then
 		ts_err "${SCRIPT}: ${DESC} failed:"
-		ts_err "command: $TC $@"
+		ts_err "command: $CMD $@"
 		ts_err "stderr output:"
 		ts_err_cat $STD_ERR
 		if [ -s $STD_OUT ]; then
@@ -50,29 +51,19 @@ ts_tc()
 	fi
 }
 
-ts_ip()
+ts_tc()
 {
-	SCRIPT=$1; shift
-	DESC=$1; shift
+	__ts_cmd "$TC" "$@"
+}
 
-	$IP $@ 2> $STD_ERR > $STD_OUT
-        RET=$?
+ts_ip()
+{
+	__ts_cmd "$IP" "$@"
+}
 
-	if [ -s $STD_ERR ] || [ "$RET" != "0" ]; then
-		ts_err "${SCRIPT}: ${DESC} failed:"
-		ts_err "command: $IP $@"
-		ts_err "stderr output:"
-		ts_err_cat $STD_ERR
-		if [ -s $STD_OUT ]; then
-			ts_err "stdout output:"
-			ts_err_cat $STD_OUT
-		fi
-	elif [ -s $STD_OUT ]; then
-		echo "${SCRIPT}: ${DESC} succeeded with output:"
-		cat $STD_OUT
-	else
-		echo "${SCRIPT}: ${DESC} succeeded"
-	fi
+ts_ss()
+{
+	__ts_cmd "$SS" "$@"
 }
 
 ts_qdisc_available()
-- 
2.18.0

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

* [iproute PATCH 3/3] testsuite: Add a first ss test validating ssfilter
  2018-08-14 12:18 [iproute PATCH 0/3] Fix and test ssfilter Phil Sutter
  2018-08-14 12:18 ` [iproute PATCH 1/3] ss: Review ssfilter Phil Sutter
  2018-08-14 12:18 ` [iproute PATCH 2/3] testsuite: Prepare for ss tests Phil Sutter
@ 2018-08-14 12:18 ` Phil Sutter
  2018-08-15 21:33 ` [iproute PATCH 0/3] Fix and test ssfilter Stephen Hemminger
  3 siblings, 0 replies; 5+ messages in thread
From: Phil Sutter @ 2018-08-14 12:18 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Samuel Mannehed

This tests a few ssfilter expressions by selecting sockets from a TCP
dump file. The dump was created using the following command:

| ss -ntaD testsuite/tests/ss/ss1.dump

It is fed into ss via TCPDIAG_FILE environment variable.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 testsuite/tests/ss/ss1.dump   | Bin 0 -> 720 bytes
 testsuite/tests/ss/ssfilter.t |  48 ++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)
 create mode 100644 testsuite/tests/ss/ss1.dump
 create mode 100755 testsuite/tests/ss/ssfilter.t

diff --git a/testsuite/tests/ss/ss1.dump b/testsuite/tests/ss/ss1.dump
new file mode 100644
index 0000000000000000000000000000000000000000..9c273231c78418593cabda324ca20d5a6d41e1aa
GIT binary patch
literal 720
zcmYdbU|<koU}A81#K^!c$-uzG1r!hiVj=)DnwkbEe>Nin11kdun3n(~QOsv#0-E2u
z3TO>b6#}61K{9Mm>1mU45ek8<2e$al?_I?phHf4@A7mgq)YMKS^Irfxb<qmH`3z!5
zI?&An@_`;h1*}l+IW<)G-$HV~2v7|(Quu?kWB@U8m~jCOCpJ!4Kn5Uz1}J+jQk<|d
zaDxLs0Vs!J4=`@#c`%6mJHX%s)dr$e(D>kZgGJtnxD>cjP}t=IBMn#FbAjW2%?&j3
Wu$m7G%#dh=`5=oj%@O8f3p)VpCN7Tv

literal 0
HcmV?d00001

diff --git a/testsuite/tests/ss/ssfilter.t b/testsuite/tests/ss/ssfilter.t
new file mode 100755
index 0000000000000..e74f1765cb723
--- /dev/null
+++ b/testsuite/tests/ss/ssfilter.t
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+. lib/generic.sh
+
+# % ./misc/ss -Htna
+# LISTEN  0    128    0.0.0.0:22       0.0.0.0:*
+# ESTAB   0    0     10.0.0.1:22      10.0.0.1:36266
+# ESTAB   0    0     10.0.0.1:36266   10.0.0.1:22
+# ESTAB   0    0     10.0.0.1:22      10.0.0.2:50312
+export TCPDIAG_FILE="$(dirname $0)/ss1.dump"
+
+ts_log "[Testing ssfilter]"
+
+ts_ss "$0" "Match dport = 22" -Htna dport = 22
+test_on "ESTAB    0           0                 10.0.0.1:36266           10.0.0.1:22"
+
+ts_ss "$0" "Match dport 22" -Htna dport 22
+test_on "ESTAB    0           0                 10.0.0.1:36266           10.0.0.1:22"
+
+ts_ss "$0" "Match (dport)" -Htna '( dport = 22 )'
+test_on "ESTAB    0           0                 10.0.0.1:36266           10.0.0.1:22"
+
+ts_ss "$0" "Match src = 0.0.0.0" -Htna src = 0.0.0.0
+test_on "LISTEN     0           128                0.0.0.0:22             0.0.0.0:*"
+
+ts_ss "$0" "Match src 0.0.0.0" -Htna src 0.0.0.0
+test_on "LISTEN     0           128                0.0.0.0:22             0.0.0.0:*"
+
+ts_ss "$0" "Match src sport" -Htna src 0.0.0.0 sport = 22
+test_on "LISTEN     0           128                0.0.0.0:22             0.0.0.0:*"
+
+ts_ss "$0" "Match src and sport" -Htna src 0.0.0.0 and sport = 22
+test_on "LISTEN     0           128                0.0.0.0:22             0.0.0.0:*"
+
+ts_ss "$0" "Match src and sport and dport" -Htna src 10.0.0.1 and sport = 22 and dport = 50312
+test_on "ESTAB    0           0                 10.0.0.1:22           10.0.0.2:50312"
+
+ts_ss "$0" "Match src and sport and (dport)" -Htna 'src 10.0.0.1 and sport = 22 and ( dport = 50312 )'
+test_on "ESTAB    0           0                 10.0.0.1:22           10.0.0.2:50312"
+
+ts_ss "$0" "Match src and (sport and dport)" -Htna 'src 10.0.0.1 and ( sport = 22 and dport = 50312 )'
+test_on "ESTAB    0           0                 10.0.0.1:22           10.0.0.2:50312"
+
+ts_ss "$0" "Match (src and sport) and dport" -Htna '( src 10.0.0.1 and sport = 22 ) and dport = 50312'
+test_on "ESTAB    0           0                 10.0.0.1:22           10.0.0.2:50312"
+
+ts_ss "$0" "Match (src or src) and dst" -Htna '( src 0.0.0.0 or src 10.0.0.1 ) and dst 10.0.0.2'
+test_on "ESTAB    0           0                 10.0.0.1:22           10.0.0.2:50312"
-- 
2.18.0

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

* Re: [iproute PATCH 0/3] Fix and test ssfilter
  2018-08-14 12:18 [iproute PATCH 0/3] Fix and test ssfilter Phil Sutter
                   ` (2 preceding siblings ...)
  2018-08-14 12:18 ` [iproute PATCH 3/3] testsuite: Add a first ss test validating ssfilter Phil Sutter
@ 2018-08-15 21:33 ` Stephen Hemminger
  3 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2018-08-15 21:33 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netdev, Samuel Mannehed

On Tue, 14 Aug 2018 14:18:05 +0200
Phil Sutter <phil@nwl.cc> wrote:

> This series contains a fix for ssfilter and introduces a testscript to
> verify correct functionality.
> 
> Phil Sutter (3):
>   ss: Review ssfilter
>   testsuite: Prepare for ss tests
>   testsuite: Add a first ss test validating ssfilter
> 
>  misc/ssfilter.y               |  36 ++++++++++++++-----------
>  testsuite/Makefile            |   2 +-
>  testsuite/lib/generic.sh      |  37 ++++++++++----------------
>  testsuite/tests/ss/ss1.dump   | Bin 0 -> 720 bytes
>  testsuite/tests/ss/ssfilter.t |  48 ++++++++++++++++++++++++++++++++++
>  5 files changed, 84 insertions(+), 39 deletions(-)
>  create mode 100644 testsuite/tests/ss/ss1.dump
>  create mode 100755 testsuite/tests/ss/ssfilter.t
> 

Applied

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

end of thread, other threads:[~2018-08-16  0:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-14 12:18 [iproute PATCH 0/3] Fix and test ssfilter Phil Sutter
2018-08-14 12:18 ` [iproute PATCH 1/3] ss: Review ssfilter Phil Sutter
2018-08-14 12:18 ` [iproute PATCH 2/3] testsuite: Prepare for ss tests Phil Sutter
2018-08-14 12:18 ` [iproute PATCH 3/3] testsuite: Add a first ss test validating ssfilter Phil Sutter
2018-08-15 21:33 ` [iproute PATCH 0/3] Fix and test ssfilter Stephen Hemminger

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