* [nft PATCH] tproxy: Drop artificial port printing restriction
@ 2023-11-02 13:52 Phil Sutter
2023-11-02 15:56 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Phil Sutter @ 2023-11-02 13:52 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
It does not make much sense to omit printing the port expression if it's
not a value expression: On one hand, input allows for more advanced
uses. On the other, if it is in-kernel, best nft can do is to try and
print it no matter what. Just ignoring ruleset elements can't be
correct.
Fixes: 2be1d52644cf7 ("src: Add tproxy support")
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1721
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
src/statement.c | 2 +-
tests/py/inet/tproxy.t | 2 ++
tests/py/inet/tproxy.t.json | 35 ++++++++++++++++++++++++++++++++++
tests/py/inet/tproxy.t.payload | 12 ++++++++++++
4 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/src/statement.c b/src/statement.c
index 475611664946a..f5176e6d87f95 100644
--- a/src/statement.c
+++ b/src/statement.c
@@ -989,7 +989,7 @@ static void tproxy_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
expr_print(stmt->tproxy.addr, octx);
}
}
- if (stmt->tproxy.port && stmt->tproxy.port->etype == EXPR_VALUE) {
+ if (stmt->tproxy.port) {
if (!stmt->tproxy.addr)
nft_print(octx, " ");
nft_print(octx, ":");
diff --git a/tests/py/inet/tproxy.t b/tests/py/inet/tproxy.t
index d23bbcb56cdcd..9901df75a91a8 100644
--- a/tests/py/inet/tproxy.t
+++ b/tests/py/inet/tproxy.t
@@ -19,3 +19,5 @@ meta l4proto 17 tproxy ip to :50080;ok
meta l4proto 17 tproxy ip6 to :50080;ok
meta l4proto 17 tproxy to :50080;ok
ip daddr 0.0.0.0/0 meta l4proto 6 tproxy ip to :2000;ok
+
+meta l4proto 6 tproxy ip to 127.0.0.1:symhash mod 2 map { 0 : 23, 1 : 42 };ok
diff --git a/tests/py/inet/tproxy.t.json b/tests/py/inet/tproxy.t.json
index 7b3b11c49205a..71b6fd2f678dd 100644
--- a/tests/py/inet/tproxy.t.json
+++ b/tests/py/inet/tproxy.t.json
@@ -183,3 +183,38 @@
}
}
]
+
+# meta l4proto 6 tproxy ip to 127.0.0.1:symhash mod 2 map { 0 : 23, 1 : 42 }
+[
+ {
+ "match": {
+ "left": {
+ "meta": {
+ "key": "l4proto"
+ }
+ },
+ "op": "==",
+ "right": 6
+ }
+ },
+ {
+ "tproxy": {
+ "addr": "127.0.0.1",
+ "family": "ip",
+ "port": {
+ "map": {
+ "data": {
+ "set": [
+ [ 0, 23 ],
+ [ 1, 42 ]
+ ]
+ },
+ "key": {
+ "symhash": { "mod": 2 }
+ }
+ }
+ }
+ }
+ }
+]
+
diff --git a/tests/py/inet/tproxy.t.payload b/tests/py/inet/tproxy.t.payload
index 24bf8f6002f8f..2f41904261144 100644
--- a/tests/py/inet/tproxy.t.payload
+++ b/tests/py/inet/tproxy.t.payload
@@ -61,3 +61,15 @@ inet x y
[ immediate reg 1 0x0000d007 ]
[ tproxy ip port reg 1 ]
+# meta l4proto 6 tproxy ip to 127.0.0.1:symhash mod 2 map { 0 : 23, 1 : 42 }
+__map%d x b size 2
+__map%d x 0
+ element 00000000 : 00001700 0 [end] element 00000001 : 00002a00 0 [end]
+inet x y
+ [ meta load l4proto => reg 1 ]
+ [ cmp eq reg 1 0x00000006 ]
+ [ immediate reg 1 0x0100007f ]
+ [ hash reg 2 = symhash() % mod 2 ]
+ [ lookup reg 2 set __map%d dreg 2 ]
+ [ tproxy ip addr reg 1 port reg 2 ]
+
--
2.41.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [nft PATCH] tproxy: Drop artificial port printing restriction
2023-11-02 13:52 [nft PATCH] tproxy: Drop artificial port printing restriction Phil Sutter
@ 2023-11-02 15:56 ` Pablo Neira Ayuso
2023-11-02 15:58 ` Pablo Neira Ayuso
2023-11-02 17:23 ` Phil Sutter
0 siblings, 2 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2023-11-02 15:56 UTC (permalink / raw)
To: Phil Sutter; +Cc: netfilter-devel
On Thu, Nov 02, 2023 at 02:52:58PM +0100, Phil Sutter wrote:
> It does not make much sense to omit printing the port expression if it's
> not a value expression: On one hand, input allows for more advanced
> uses. On the other, if it is in-kernel, best nft can do is to try and
> print it no matter what. Just ignoring ruleset elements can't be
> correct.
>
> Fixes: 2be1d52644cf7 ("src: Add tproxy support")
> Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1721
> Signed-off-by: Phil Sutter <phil@nwl.cc>
Great work Phil.
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> src/statement.c | 2 +-
> tests/py/inet/tproxy.t | 2 ++
> tests/py/inet/tproxy.t.json | 35 ++++++++++++++++++++++++++++++++++
> tests/py/inet/tproxy.t.payload | 12 ++++++++++++
> 4 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/src/statement.c b/src/statement.c
> index 475611664946a..f5176e6d87f95 100644
> --- a/src/statement.c
> +++ b/src/statement.c
> @@ -989,7 +989,7 @@ static void tproxy_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
> expr_print(stmt->tproxy.addr, octx);
> }
> }
> - if (stmt->tproxy.port && stmt->tproxy.port->etype == EXPR_VALUE) {
> + if (stmt->tproxy.port) {
> if (!stmt->tproxy.addr)
> nft_print(octx, " ");
> nft_print(octx, ":");
> diff --git a/tests/py/inet/tproxy.t b/tests/py/inet/tproxy.t
> index d23bbcb56cdcd..9901df75a91a8 100644
> --- a/tests/py/inet/tproxy.t
> +++ b/tests/py/inet/tproxy.t
> @@ -19,3 +19,5 @@ meta l4proto 17 tproxy ip to :50080;ok
> meta l4proto 17 tproxy ip6 to :50080;ok
> meta l4proto 17 tproxy to :50080;ok
> ip daddr 0.0.0.0/0 meta l4proto 6 tproxy ip to :2000;ok
> +
> +meta l4proto 6 tproxy ip to 127.0.0.1:symhash mod 2 map { 0 : 23, 1 : 42 };ok
> diff --git a/tests/py/inet/tproxy.t.json b/tests/py/inet/tproxy.t.json
> index 7b3b11c49205a..71b6fd2f678dd 100644
> --- a/tests/py/inet/tproxy.t.json
> +++ b/tests/py/inet/tproxy.t.json
> @@ -183,3 +183,38 @@
> }
> }
> ]
> +
> +# meta l4proto 6 tproxy ip to 127.0.0.1:symhash mod 2 map { 0 : 23, 1 : 42 }
> +[
> + {
> + "match": {
> + "left": {
> + "meta": {
> + "key": "l4proto"
> + }
> + },
> + "op": "==",
> + "right": 6
> + }
> + },
> + {
> + "tproxy": {
> + "addr": "127.0.0.1",
> + "family": "ip",
> + "port": {
> + "map": {
> + "data": {
> + "set": [
> + [ 0, 23 ],
> + [ 1, 42 ]
> + ]
> + },
> + "key": {
> + "symhash": { "mod": 2 }
> + }
> + }
> + }
> + }
> + }
> +]
> +
> diff --git a/tests/py/inet/tproxy.t.payload b/tests/py/inet/tproxy.t.payload
> index 24bf8f6002f8f..2f41904261144 100644
> --- a/tests/py/inet/tproxy.t.payload
> +++ b/tests/py/inet/tproxy.t.payload
> @@ -61,3 +61,15 @@ inet x y
> [ immediate reg 1 0x0000d007 ]
> [ tproxy ip port reg 1 ]
>
> +# meta l4proto 6 tproxy ip to 127.0.0.1:symhash mod 2 map { 0 : 23, 1 : 42 }
> +__map%d x b size 2
> +__map%d x 0
> + element 00000000 : 00001700 0 [end] element 00000001 : 00002a00 0 [end]
> +inet x y
> + [ meta load l4proto => reg 1 ]
> + [ cmp eq reg 1 0x00000006 ]
> + [ immediate reg 1 0x0100007f ]
> + [ hash reg 2 = symhash() % mod 2 ]
> + [ lookup reg 2 set __map%d dreg 2 ]
> + [ tproxy ip addr reg 1 port reg 2 ]
> +
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [nft PATCH] tproxy: Drop artificial port printing restriction
2023-11-02 15:56 ` Pablo Neira Ayuso
@ 2023-11-02 15:58 ` Pablo Neira Ayuso
2023-11-02 17:14 ` Phil Sutter
2023-11-02 17:23 ` Phil Sutter
1 sibling, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2023-11-02 15:58 UTC (permalink / raw)
To: Phil Sutter; +Cc: netfilter-devel
On Thu, Nov 02, 2023 at 04:56:37PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Nov 02, 2023 at 02:52:58PM +0100, Phil Sutter wrote:
[...]
> > diff --git a/src/statement.c b/src/statement.c
> > index 475611664946a..f5176e6d87f95 100644
> > --- a/src/statement.c
> > +++ b/src/statement.c
> > @@ -989,7 +989,7 @@ static void tproxy_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
> > expr_print(stmt->tproxy.addr, octx);
> > }
> > }
> > - if (stmt->tproxy.port && stmt->tproxy.port->etype == EXPR_VALUE) {
> > + if (stmt->tproxy.port) {
Question: is this pattern used elsewhere?
The original author of this might have taken (copied) this code from
an existing statement?
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [nft PATCH] tproxy: Drop artificial port printing restriction
2023-11-02 15:58 ` Pablo Neira Ayuso
@ 2023-11-02 17:14 ` Phil Sutter
0 siblings, 0 replies; 5+ messages in thread
From: Phil Sutter @ 2023-11-02 17:14 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, Máté Eckl
On Thu, Nov 02, 2023 at 04:58:43PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Nov 02, 2023 at 04:56:37PM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Nov 02, 2023 at 02:52:58PM +0100, Phil Sutter wrote:
> [...]
> > > diff --git a/src/statement.c b/src/statement.c
> > > index 475611664946a..f5176e6d87f95 100644
> > > --- a/src/statement.c
> > > +++ b/src/statement.c
> > > @@ -989,7 +989,7 @@ static void tproxy_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
> > > expr_print(stmt->tproxy.addr, octx);
> > > }
> > > }
> > > - if (stmt->tproxy.port && stmt->tproxy.port->etype == EXPR_VALUE) {
> > > + if (stmt->tproxy.port) {
>
> Question: is this pattern used elsewhere?
>
> The original author of this might have taken (copied) this code from
> an existing statement?
A quick grep for EXPR_VALUE didn't turn up anything suspicious.
Maybe Máté recalls why he did things this way back in 2018. FWIW, the
EXPR_VALUE check was already there in v1 of his patch.
Cheers, Phil
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [nft PATCH] tproxy: Drop artificial port printing restriction
2023-11-02 15:56 ` Pablo Neira Ayuso
2023-11-02 15:58 ` Pablo Neira Ayuso
@ 2023-11-02 17:23 ` Phil Sutter
1 sibling, 0 replies; 5+ messages in thread
From: Phil Sutter @ 2023-11-02 17:23 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Thu, Nov 02, 2023 at 04:56:32PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Nov 02, 2023 at 02:52:58PM +0100, Phil Sutter wrote:
> > It does not make much sense to omit printing the port expression if it's
> > not a value expression: On one hand, input allows for more advanced
> > uses. On the other, if it is in-kernel, best nft can do is to try and
> > print it no matter what. Just ignoring ruleset elements can't be
> > correct.
> >
> > Fixes: 2be1d52644cf7 ("src: Add tproxy support")
> > Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1721
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
>
> Great work Phil.
Hey, I'm just deleting code which gets in the way. ;)
> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
Patch applied, thanks for your review.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-02 17:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-02 13:52 [nft PATCH] tproxy: Drop artificial port printing restriction Phil Sutter
2023-11-02 15:56 ` Pablo Neira Ayuso
2023-11-02 15:58 ` Pablo Neira Ayuso
2023-11-02 17:14 ` Phil Sutter
2023-11-02 17:23 ` Phil Sutter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.