Re: [PATCH nft] tests: shell: add packetpath test for meta ibrhwdr

[Florian Westphal <fw@strlen.de>]

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> The test checks that the packets are processed by the bridge device and
> not forwarded.

OK, I think it would make sense to also check that we can do something
useful with the packet.

> +
> +ip -net "$ns1" link set veth0 up
> +ip -net "$ns2" link set veth0 up
> +ip -net "$ns3" link set veth1 up
> +ip -net "$ns2" link set veth1 up
> +ip -net "$ns2" link set br0 up
> +
> +ip -net "$ns1" addr add 10.1.1.10/24 dev veth0
> +ip -net "$ns3" addr add 10.1.1.20/24 dev veth1
> +
> +ip netns exec "$ns2" $NFT -f /dev/stdin <<"EOF"
> +table bridge nat {
> +	chain PREROUTING {
> +		type filter hook prerouting priority 0; policy accept;
> +		ether daddr de:ad:00:00:be:ef meta pkttype set host ether daddr set meta ibrhwdr

While this indeed is enough to check the bridge 'redirect to input'
maybe it would make sense to also check that we can do something useful
with it?

The bridge has no ip address, so I don't really see the point why anyone
would redirect the packet locally to begin with.

> +table bridge donotprocess {
> +	chain FORWARD {
> +		type filter hook forward priority 0; policy accept;
> +		ip protocol icmp ether saddr da:d3:00:01:02:03 counter
> +	}
> +}
> +EOF
> +
> +ip netns exec "$ns1" ping -c 1 10.1.1.20 || true
> +
> +set +e
> +
> +ip netns exec "$ns2" $NFT list table bridge process | grep 'counter packets 0'
> +if [ $? -eq 0 ]
> +then
> +	exit 1

I think it would be nice to display WHERE its failing so that someone
looking at testout.log doesn't have to re-run with added "set -x" or "echo
failed at".

To give some ideas (this isn't fleshed out):

diff --git a/tests/shell/testcases/packetpath/bridge_pass_up b/tests/shell/testcases/packetpath/bridge_pass_up
--- a/tests/shell/testcases/packetpath/bridge_pass_up
+++ b/tests/shell/testcases/packetpath/bridge_pass_up
@@ -38,14 +38,18 @@ ip -net "$ns3" link set veth1 up
 ip -net "$ns2" link set veth1 up
 ip -net "$ns2" link set br0 up
 
+ip netns exec "$ns2" sysctl -q net.ipv4.ip_forward=1
+
 ip -net "$ns1" addr add 10.1.1.10/24 dev veth0
 ip -net "$ns3" addr add 10.1.1.20/24 dev veth1
+ip -net "$ns2" addr add 10.1.1.1/24 dev br0
 
 ip netns exec "$ns2" $NFT -f /dev/stdin <<"EOF"
 table bridge nat {
 	chain PREROUTING {
 		type filter hook prerouting priority 0; policy accept;
-		ether daddr de:ad:00:00:be:ef meta pkttype set host ether daddr set meta ibrhwdr
+		ether daddr de:ad:00:00:be:ef meta pkttype set host ether daddr set meta ibrhwdr meta mark set 1
 	}
 }
 
@@ -62,9 +66,19 @@ table bridge donotprocess {
 		ip protocol icmp ether saddr da:d3:00:01:02:03 counter
 	}
 }
+
+table ip process {
+	chain FORWARD {
+		type filter hook forward priority 0; policy accept;
+		ip protocol icmp mark 1 counter
+	}
+
+}
 EOF

ALTERNATIVELY one could check INPUT instead (no fwd sysctl)
by also doing 'ip daddr set 10.1.1.1' in bridge or ip prerouting
and then checking if the packet made it to the ip input hook.

I only ever saw 3 cases of this 'bridge hijacking' implemented
via meta ibrhwdr:

1. Push packets up to mangle them via NAT.  In most cases
people did this with br_netfilter + iptables, but it would be nice
to make sure we have a working alternative to that thing.

2. Push packet to local host/application, e.g. for TPROXY intercept.
3. Push packet to local host for forwarding/policy routing.

I think that 'it arrived in ip input or forwarding'
would be good because then the above (tproxy, nat, etc. should "just work").

But if you think checking bridge input is enough, thats fine.