All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] wg-quick: linux: add support for nft and prefer it
@ 2019-12-10 15:48 Jason A. Donenfeld
  2019-12-10 16:52 ` Jordan Glover
  2019-12-10 17:31 ` Vasili Pupkin
  0 siblings, 2 replies; 20+ messages in thread
From: Jason A. Donenfeld @ 2019-12-10 15:48 UTC (permalink / raw)
  To: wireguard, jwollrath, dkg, diggest

If nft(8) is installed, use it. These rules should be identical to the
iptables-restore(8) ones, with the advantage that cleanup is easy
because we use custom table names.
---
Hey folks,

I'd appreciate a review from some of the nftables experts on this list
who requested this.

Thanks,
Jason

 src/tools/wg-quick/linux.bash | 59 +++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/src/tools/wg-quick/linux.bash b/src/tools/wg-quick/linux.bash
index 9bfeed1b..dde02951 100755
--- a/src/tools/wg-quick/linux.bash
+++ b/src/tools/wg-quick/linux.bash
@@ -95,7 +95,7 @@ add_if() {
 del_if() {
 	local table
 	[[ $HAVE_SET_DNS -eq 0 ]] || unset_dns
-	[[ $HAVE_SET_IPTABLES -eq 0 ]] || remove_iptables
+	[[ $HAVE_SET_FIREWALL -eq 0 ]] || remove_firewall
 	if [[ -z $TABLE || $TABLE == auto ]] && get_fwmark table && [[ $(wg show "$INTERFACE" allowed-ips) =~ /0(\ |$'\n'|$) ]]; then
 		while [[ $(ip -4 rule show 2>/dev/null) == *"lookup $table"* ]]; do
 			cmd ip -4 rule delete table $table
@@ -181,22 +181,30 @@ get_fwmark() {
 	return 0
 }
 
-remove_iptables() {
-	local line iptables found restore
-	for iptables in iptables ip6tables; do
-		restore="" found=0
-		while read -r line; do
-			[[ $line == "*"* || $line == COMMIT || $line == "-A "*"-m comment --comment \"wg-quick(8) rule for $INTERFACE\""* ]] || continue
-			[[ $line == "-A"* ]] && found=1
-			printf -v restore '%s%s\n' "$restore" "${line/#-A/-D}"
-		done < <($iptables-save 2>/dev/null)
-		[[ $found -ne 1 ]] || echo -n "$restore" | cmd $iptables-restore -n
-	done
+remove_firewall() {
+	if type -p nft >/dev/null; then
+		local table nftcmd
+		while read -r table; do
+			[[ $table == *" wg-quick-$INTERFACE" ]] && printf -v nftcmd '%sdelete %s\n' "$nftcmd" "$table"
+		done < <(nft list tables 2>/dev/null)
+		[[ -z $nftcmd ]] || echo -n "$nftcmd" | cmd nft -f -
+	else
+		local line iptables found restore
+		for iptables in iptables ip6tables; do
+			restore="" found=0
+			while read -r line; do
+				[[ $line == "*"* || $line == COMMIT || $line == "-A "*"-m comment --comment \"wg-quick(8) rule for $INTERFACE\""* ]] || continue
+				[[ $line == "-A"* ]] && found=1
+				printf -v restore '%s%s\n' "$restore" "${line/#-A/-D}"
+			done < <($iptables-save 2>/dev/null)
+			[[ $found -ne 1 ]] || echo -n "$restore" | cmd $iptables-restore -n
+		done
+	fi
 }
 
-HAVE_SET_IPTABLES=0
+HAVE_SET_FIREWALL=0
 add_default() {
-	local table proto i iptables
+	local table i
 	if ! get_fwmark table; then
 		table=51820
 		while [[ -n $(ip -4 route show table $table 2>/dev/null) || -n $(ip -6 route show table $table 2>/dev/null) ]]; do
@@ -204,21 +212,32 @@ add_default() {
 		done
 		cmd wg set "$INTERFACE" fwmark $table
 	fi
-	proto=-4 iptables=iptables
-	[[ $1 == *:* ]] && proto=-6 iptables=ip6tables
+	local proto=-4 iptables=iptables pf=ip
+	[[ $1 == *:* ]] && proto=-6 iptables=ip6tables pf=ip6
 	cmd ip $proto route add "$1" dev "$INTERFACE" table $table
 	cmd ip $proto rule add not fwmark $table table $table
 	cmd ip $proto rule add table main suppress_prefixlength 0
 
-	local marker="-m comment --comment \"wg-quick(8) rule for $INTERFACE\"" restore=$'*raw\n'
+	local marker="-m comment --comment \"wg-quick(8) rule for $INTERFACE\"" restore=$'*raw\n' nftable="wg-quick-$INTERFACE" nftcmd 
+	printf -v nftcmd '%sadd table %s %s\n' "$nftcmd" "$pf" "$nftable"
+	printf -v nftcmd '%sadd chain %s %s preraw { type filter hook prerouting priority raw; }\n' "$nftcmd" "$pf" "$nftable"
+	printf -v nftcmd '%sadd chain %s %s premangle { type filter hook prerouting priority mangle; }\n' "$nftcmd" "$pf" "$nftable"
+	printf -v nftcmd '%sadd chain %s %s postmangle { type filter hook postrouting priority mangle; }\n' "$nftcmd" "$pf" "$nftable"
 	for i in "${ADDRESSES[@]}"; do
 		[[ ( $proto == -4 && $i != *:* ) || ( $proto == -6 && $i == *:* ) ]] || continue
 		printf -v restore '%s-I PREROUTING ! -i %s -d %s -m addrtype ! --src-type LOCAL -j DROP %s\n' "$restore" "$INTERFACE" "${i%/*}" "$marker"
+		printf -v nftcmd '%sadd rule %s %s preraw iifname != %s %s daddr %s fib saddr type != local drop\n' "$nftcmd" "$pf" "$nftable" "$INTERFACE" "$pf" "${i%/*}"
 	done
 	printf -v restore '%sCOMMIT\n*mangle\n-I POSTROUTING -m mark --mark %d -p udp -j CONNMARK --save-mark %s\n-I PREROUTING -p udp -j CONNMARK --restore-mark %s\nCOMMIT\n' "$restore" $table "$marker" "$marker"
+	printf -v nftcmd '%sadd rule %s %s postmangle meta l4proto udp mark %d ct mark set mark \n' "$nftcmd" "$pf" "$nftable" $table
+	printf -v nftcmd '%sadd rule %s %s premangle meta l4proto udp meta mark set ct mark \n' "$nftcmd" "$pf" "$nftable"
 	[[ $proto == -4 ]] && cmd sysctl -q net.ipv4.conf.all.src_valid_mark=1
-	echo -n "$restore" | cmd $iptables-restore -n
-	HAVE_SET_IPTABLES=1
+	if type -p nft >/dev/null; then
+		echo -n "$nftcmd" | cmd nft -f -
+	else
+		echo -n "$restore" | cmd $iptables-restore -n
+	fi
+	HAVE_SET_FIREWALL=1
 	return 0
 }
 
@@ -323,7 +342,7 @@ cmd_down() {
 	[[ $SAVE_CONFIG -eq 0 ]] || save_config
 	del_if
 	unset_dns || true
-	remove_iptables || true
+	remove_firewall || true
 	execute_hooks "${POST_DOWN[@]}"
 }
 
-- 
2.24.0

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 15:48 [PATCH] wg-quick: linux: add support for nft and prefer it Jason A. Donenfeld
@ 2019-12-10 16:52 ` Jordan Glover
  2019-12-10 16:54   ` Jason A. Donenfeld
  2019-12-10 17:31 ` Vasili Pupkin
  1 sibling, 1 reply; 20+ messages in thread
From: Jordan Glover @ 2019-12-10 16:52 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: jwollrath, wireguard

On Tuesday, December 10, 2019 3:48 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:

> If nft(8) is installed, use it. These rules should be identical to the
> iptables-restore(8) ones, with the advantage that cleanup is easy
> because we use custom table names.
>

I wonder if nft should be used only if iptables isn't installed instead.
Nowadays iptables has nft backend which I believe is default and will
translate iptables rules to nft automatically. On my system iptables rules
from wg-quck are already shown in "nft list ruleset".

I'm not sure if this work in reverse - are nft rules automatically translated
to iptables and shown in iptables-save? If not then using iptables of available
seems more versatile for the job.

Jordan
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 16:52 ` Jordan Glover
@ 2019-12-10 16:54   ` Jason A. Donenfeld
  2019-12-10 17:05     ` Jordan Glover
  2019-12-10 17:12     ` Roman Mamedov
  0 siblings, 2 replies; 20+ messages in thread
From: Jason A. Donenfeld @ 2019-12-10 16:54 UTC (permalink / raw)
  To: Jordan Glover; +Cc: jwollrath, wireguard

On Tue, Dec 10, 2019 at 5:52 PM Jordan Glover
<Golden_Miller83@protonmail.ch> wrote:
>
> On Tuesday, December 10, 2019 3:48 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> > If nft(8) is installed, use it. These rules should be identical to the
> > iptables-restore(8) ones, with the advantage that cleanup is easy
> > because we use custom table names.
> >
>
> I wonder if nft should be used only if iptables isn't installed instead.
> Nowadays iptables has nft backend which I believe is default and will
> translate iptables rules to nft automatically. On my system iptables rules
> from wg-quck are already shown in "nft list ruleset".
>
> I'm not sure if this work in reverse - are nft rules automatically translated
> to iptables and shown in iptables-save? If not then using iptables of available
> seems more versatile for the job.

iptables rules and nftables rules can co-exist just fine, without any
translation needed. Indeed if your iptables is symlinked to
iptables-nft, then you'll insert nftables rules when you try to insert
iptables rules, but it really doesn't matter much either way (AFAIK).
I figured I'd prefer nftables over iptables if available because I
presume, without any metrics, that nftables is probably faster and
slicker or something.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 16:54   ` Jason A. Donenfeld
@ 2019-12-10 17:05     ` Jordan Glover
  2019-12-10 17:11       ` Jason A. Donenfeld
  2019-12-10 17:12     ` Roman Mamedov
  1 sibling, 1 reply; 20+ messages in thread
From: Jordan Glover @ 2019-12-10 17:05 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: jwollrath, wireguard

On Tuesday, December 10, 2019 4:54 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:

> On Tue, Dec 10, 2019 at 5:52 PM Jordan Glover
> Golden_Miller83@protonmail.ch wrote:
>
> > On Tuesday, December 10, 2019 3:48 PM, Jason A. Donenfeld Jason@zx2c4.com wrote:
> >
> > > If nft(8) is installed, use it. These rules should be identical to the
> > > iptables-restore(8) ones, with the advantage that cleanup is easy
> > > because we use custom table names.
> >
> > I wonder if nft should be used only if iptables isn't installed instead.
> > Nowadays iptables has nft backend which I believe is default and will
> > translate iptables rules to nft automatically. On my system iptables rules
> > from wg-quck are already shown in "nft list ruleset".
> > I'm not sure if this work in reverse - are nft rules automatically translated
> > to iptables and shown in iptables-save? If not then using iptables of available
> > seems more versatile for the job.
>
> iptables rules and nftables rules can co-exist just fine, without any
> translation needed. Indeed if your iptables is symlinked to
> iptables-nft, then you'll insert nftables rules when you try to insert
> iptables rules, but it really doesn't matter much either way (AFAIK).
> I figured I'd prefer nftables over iptables if available because I
> presume, without any metrics, that nftables is probably faster and
> slicker or something.

As I said before, my concern is more about people being fully aware of state
of their firewall rather than if it technically works.

Jordan
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 17:05     ` Jordan Glover
@ 2019-12-10 17:11       ` Jason A. Donenfeld
  0 siblings, 0 replies; 20+ messages in thread
From: Jason A. Donenfeld @ 2019-12-10 17:11 UTC (permalink / raw)
  To: Jordan Glover; +Cc: jwollrath, wireguard

On Tue, Dec 10, 2019 at 6:05 PM Jordan Glover
<Golden_Miller83@protonmail.ch> wrote:
>
> On Tuesday, December 10, 2019 4:54 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> > On Tue, Dec 10, 2019 at 5:52 PM Jordan Glover
> > Golden_Miller83@protonmail.ch wrote:
> >
> > > On Tuesday, December 10, 2019 3:48 PM, Jason A. Donenfeld Jason@zx2c4.com wrote:
> > >
> > > > If nft(8) is installed, use it. These rules should be identical to the
> > > > iptables-restore(8) ones, with the advantage that cleanup is easy
> > > > because we use custom table names.
> > >
> > > I wonder if nft should be used only if iptables isn't installed instead.
> > > Nowadays iptables has nft backend which I believe is default and will
> > > translate iptables rules to nft automatically. On my system iptables rules
> > > from wg-quck are already shown in "nft list ruleset".
> > > I'm not sure if this work in reverse - are nft rules automatically translated
> > > to iptables and shown in iptables-save? If not then using iptables of available
> > > seems more versatile for the job.
> >
> > iptables rules and nftables rules can co-exist just fine, without any
> > translation needed. Indeed if your iptables is symlinked to
> > iptables-nft, then you'll insert nftables rules when you try to insert
> > iptables rules, but it really doesn't matter much either way (AFAIK).
> > I figured I'd prefer nftables over iptables if available because I
> > presume, without any metrics, that nftables is probably faster and
> > slicker or something.
>
> As I said before, my concern is more about people being fully aware of state
> of their firewall rather than if it technically works.

Ahh, I see what you're wondering. Well, `wg-quick` shows `[#] nft -f
-` in it's output, so an administrator should be aware that running
`nft list ruleset` or similar is how to gain some visibility, I
suppose.

Perhaps when we eventually can drop RHEL7 support, we'll even drop
iptables support all together.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 16:54   ` Jason A. Donenfeld
  2019-12-10 17:05     ` Jordan Glover
@ 2019-12-10 17:12     ` Roman Mamedov
  2019-12-10 17:28       ` Davide Depau
                         ` (3 more replies)
  1 sibling, 4 replies; 20+ messages in thread
From: Roman Mamedov @ 2019-12-10 17:12 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: jwollrath, wireguard

On Tue, 10 Dec 2019 17:54:49 +0100
"Jason A. Donenfeld" <Jason@zx2c4.com> wrote:

> iptables rules and nftables rules can co-exist just fine, without any
> translation needed. Indeed if your iptables is symlinked to
> iptables-nft, then you'll insert nftables rules when you try to insert
> iptables rules, but it really doesn't matter much either way (AFAIK).
> I figured I'd prefer nftables over iptables if available because I
> presume, without any metrics, that nftables is probably faster and
> slicker or something.

nftables is slower than iptables across pretty much every metric[1][2]. It
only wins where a pathological case is used for the iptables counterpart (e.g.
tons of single IPs as individual rules and without ipset). It is a disaster
that it is purported to be the iptables replacement, just for the syntax and
non-essential whistles such as updating rules in place or something. And
personally I don't prefer the new syntax either. It's the systemd and
pulseaudio story all over again, where something more convoluted, less reliable
and of lower quality is passed for a replacement of stuff that actually worked,
but was deemed "unsexy" and arbitrarly declared as deprecated.

[1] http://www.diva-portal.org/smash/get/diva2:1212650/FULLTEXT01.pdf
[2] https://developers.redhat.com/blog/2017/04/11/benchmarking-nftables/

-- 
With respect,
Roman
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 17:12     ` Roman Mamedov
@ 2019-12-10 17:28       ` Davide Depau
  2019-12-10 17:33       ` Matthias Urlichs
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Davide Depau @ 2019-12-10 17:28 UTC (permalink / raw)
  To: Roman Mamedov; +Cc: jwollrath, wireguard


[-- Attachment #1.1: Type: text/plain, Size: 1528 bytes --]

On Tue, Dec 10, 2019 at 6:13 PM Roman Mamedov <rm@romanrm.net> wrote:

> nftables is slower than iptables across pretty much every metric[1][2]. It
> only wins where a pathological case is used for the iptables counterpart
> (e.g.
> tons of single IPs as individual rules and without ipset). It is a disaster
> that it is purported to be the iptables replacement, just for the syntax
> and
> non-essential whistles such as updating rules in place or something. And
> personally I don't prefer the new syntax either. It's the systemd and
> pulseaudio story all over again, where something more convoluted, less
> reliable
> and of lower quality is passed for a replacement of stuff that actually
> worked,
> but was deemed "unsexy" and arbitrarly declared as deprecated.
>
> [1] http://www.diva-portal.org/smash/get/diva2:1212650/FULLTEXT01.pdf
> [2] https://developers.redhat.com/blog/2017/04/11/benchmarking-nftables/


I'm seeing the pages you linked are dated 2018 and 2017. I'm seeing this
article [1] dated June 2018 talks about an "important improvement of
performance", and though I don't see any evidence backing the statement I'd
expect more improvements given than more than one year has passed.
Do you know whether the worse performance you're talking about is still the
case on recent Linux releases?

I'd say +1 for nftables but just for the syntax which I do like better.
I'll leave the discussion on performance to experts.

[1]
https://www.zevenet.com/knowledge-base/nftlb/nftlb-benchmarks-and-performance-keys/

[-- Attachment #1.2: Type: text/html, Size: 2270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 148 bytes --]

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 15:48 [PATCH] wg-quick: linux: add support for nft and prefer it Jason A. Donenfeld
  2019-12-10 16:52 ` Jordan Glover
@ 2019-12-10 17:31 ` Vasili Pupkin
  2019-12-10 17:38   ` Jason A. Donenfeld
  1 sibling, 1 reply; 20+ messages in thread
From: Vasili Pupkin @ 2019-12-10 17:31 UTC (permalink / raw)
  To: Jason A. Donenfeld, wireguard, jwollrath, dkg

On 10.12.2019 18:48, Jason A. Donenfeld wrote:

> restore '%s-I PREROUTING ! -i %s -d %s -m addrtype ! --src-type LOCAL -j DROP
> nftcmd '%sadd rule %s %s preraw iifname != %s %s daddr %s fib saddr type != local drop


I am trying to understand the rulesets. When you check the type of the 
source address of the incoming packet its type just can't be local to 
our machine, it is the address of the sender. The source address of the 
packet can only be local if the packet was sent from the same machine. 
Isn't this part of the rule redundant?
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 17:12     ` Roman Mamedov
  2019-12-10 17:28       ` Davide Depau
@ 2019-12-10 17:33       ` Matthias Urlichs
  2019-12-10 17:36       ` Jason A. Donenfeld
  2019-12-10 21:56       ` Vasili Pupkin
  3 siblings, 0 replies; 20+ messages in thread
From: Matthias Urlichs @ 2019-12-10 17:33 UTC (permalink / raw)
  To: wireguard

On 10.12.19 18:12, Roman Mamedov wrote:
> It's the systemd and
> pulseaudio story all over again

By that metric I can only assume that nft is a huge improvement over
iptables.

We have to deal with iptables vs. nfstables, just like we have to deal
with various vendor kernels. Complaining on-list about either is not
helpful.

In any case, rants about software that is not wireguard are off-topic
here AFAICT. Please stop.

-- 
-- Matthias Urlichs

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 17:12     ` Roman Mamedov
  2019-12-10 17:28       ` Davide Depau
  2019-12-10 17:33       ` Matthias Urlichs
@ 2019-12-10 17:36       ` Jason A. Donenfeld
  2019-12-10 18:00         ` Roman Mamedov
  2019-12-10 18:58         ` Jordan Glover
  2019-12-10 21:56       ` Vasili Pupkin
  3 siblings, 2 replies; 20+ messages in thread
From: Jason A. Donenfeld @ 2019-12-10 17:36 UTC (permalink / raw)
  To: Roman Mamedov, Daniel Kahn Gillmor; +Cc: jwollrath, wireguard

Hi Roman,

On Tue, Dec 10, 2019 at 6:12 PM Roman Mamedov <rm@romanrm.net> wrote:
>
> On Tue, 10 Dec 2019 17:54:49 +0100
> "Jason A. Donenfeld" <Jason@zx2c4.com> wrote:
>
> > iptables rules and nftables rules can co-exist just fine, without any
> > translation needed. Indeed if your iptables is symlinked to
> > iptables-nft, then you'll insert nftables rules when you try to insert
> > iptables rules, but it really doesn't matter much either way (AFAIK).
> > I figured I'd prefer nftables over iptables if available because I
> > presume, without any metrics, that nftables is probably faster and
> > slicker or something.
>
> nftables is slower than iptables across pretty much every metric[1][2]. It
> only wins where a pathological case is used for the iptables counterpart (e.g.
> tons of single IPs as individual rules and without ipset). It is a disaster
> that it is purported to be the iptables replacement, just for the syntax and
> non-essential whistles such as updating rules in place or something. And
> personally I don't prefer the new syntax either. It's the systemd and
> pulseaudio story all over again, where something more convoluted, less reliable
> and of lower quality is passed for a replacement of stuff that actually worked,
> but was deemed "unsexy" and arbitrarly declared as deprecated.
>
> [1] http://www.diva-portal.org/smash/get/diva2:1212650/FULLTEXT01.pdf
> [2] https://developers.redhat.com/blog/2017/04/11/benchmarking-nftables/

That bachelors thesis says in the abstract, "Latency was measured
through the round-trip time of ICMP packets while throughput was
measured by generating UDP traffic using iPerf3. The results showed
that, when using linear look-ups, nftables performs worse than
iptables when using small frame sizes and when using large rulesets.
If the frame size was fairly large and rule-set fairly small, nftables
was often performed slightly better both in terms of latency and in
terms of throughput. When using indexed data structures, performance
of both firewalls was very similar regardless of frame size or
rule-set size. Minor, but statistically significant, differences were
found both in favour of and against nftables, depending on the exact
parameters used." So maybe it doesn't actually matter?

On the other hand, if what you say is actually true in our case, and
nftables is utter crap, then perhaps we should scrap this nft(8) patch
all together and just keep pure iptables(8). DKG - you seemed to want
nft(8) support, though. How would you feel about that sort of
conclusion?

Jason
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 17:31 ` Vasili Pupkin
@ 2019-12-10 17:38   ` Jason A. Donenfeld
  0 siblings, 0 replies; 20+ messages in thread
From: Jason A. Donenfeld @ 2019-12-10 17:38 UTC (permalink / raw)
  To: Vasili Pupkin; +Cc: jwollrath, WireGuard mailing list

On Tue, Dec 10, 2019 at 6:30 PM Vasili Pupkin <diggest@gmail.com> wrote:
>
> On 10.12.2019 18:48, Jason A. Donenfeld wrote:
>
> > restore '%s-I PREROUTING ! -i %s -d %s -m addrtype ! --src-type LOCAL -j DROP
> > nftcmd '%sadd rule %s %s preraw iifname != %s %s daddr %s fib saddr type != local drop
>
>
> I am trying to understand the rulesets. When you check the type of the
> source address of the incoming packet its type just can't be local to
> our machine, it is the address of the sender. The source address of the
> packet can only be local if the packet was sent from the same machine.
> Isn't this part of the rule redundant?

Those lines are supposed to do the same thing, by the way. If I
screwed up and they differ subtly, please let me know.

The ! --src-type LOCAL thing makes it so that you can still ping
yourself locally. "Allow loopback." This also has the side effect of
letting in dangerous packets that are masquerading as 127/8, but only
if you've explicitly opted in to net.ipv4.conf.lo.route_localnet=1 and
maybe one other safety nob, which nobody in their right mind does for
obvious reasons.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 17:36       ` Jason A. Donenfeld
@ 2019-12-10 18:00         ` Roman Mamedov
  2019-12-10 18:58         ` Jordan Glover
  1 sibling, 0 replies; 20+ messages in thread
From: Roman Mamedov @ 2019-12-10 18:00 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: jwollrath, wireguard

On Tue, 10 Dec 2019 18:36:06 +0100
"Jason A. Donenfeld" <Jason@zx2c4.com> wrote:

> That bachelors thesis says in the abstract, "Latency was measured
> through the round-trip time of ICMP packets while throughput was
> measured by generating UDP traffic using iPerf3. The results showed
> that, when using linear look-ups, nftables performs worse than
> iptables when using small frame sizes and when using large rulesets.

Smallest possible frame sizes are what matters the most when testing any
router or firewall setup, because only then you will hit the packet-per-second
limits of the actual firewalling/routing engine. Good performance at large
frame sizes is not an impressive achievent, there you will just hit
on-the-wire bandwidth limits sooner than the CPU toll of processing rulesets
or routing lookups for each of those frames will begin to matter.

> On the other hand, if what you say is actually true in our case, and
> nftables is utter crap, then perhaps we should scrap this nft(8) patch
> all together and just keep pure iptables(8). DKG - you seemed to want
> nft(8) support, though. How would you feel about that sort of
> conclusion?

Even with my view of it I do not argue for removing nftables support from
your tools, realistically it's probably not going anywhere, or at least not
soon enough, just thought I should point out that "nftables is faster" should
not be so naturally assumed to be the case, and if I dare to say that
everyone should decide for themselves what tools they prefer, and to carefully
weigh all benefits and downsides of the proposed alternatives -- not just come
along obediently with some external party that "knows what is best for you" and
declares something deprecated out of their own arbitrary reasons.

-- 
With respect,
Roman
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 17:36       ` Jason A. Donenfeld
  2019-12-10 18:00         ` Roman Mamedov
@ 2019-12-10 18:58         ` Jordan Glover
  2019-12-10 19:15           ` Jason A. Donenfeld
  1 sibling, 1 reply; 20+ messages in thread
From: Jordan Glover @ 2019-12-10 18:58 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: jwollrath, wireguard

On Tuesday, December 10, 2019 5:36 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:

>
> On the other hand, if what you say is actually true in our case, and
> nftables is utter crap, then perhaps we should scrap this nft(8) patch
> all together and just keep pure iptables(8). DKG - you seemed to want
> nft(8) support, though. How would you feel about that sort of
> conclusion?
>
> Jason

The only scenario where you really want to use nft is where iptables command
doesn't exist. I don't know how realistic scenario it is but I assume it can
happen in the wild. Otherwise calling iptables will take care of both iptables
and nftables automatically if those are supported on system. That's why I
proposed to invert current patch logic.

Jordan
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 18:58         ` Jordan Glover
@ 2019-12-10 19:15           ` Jason A. Donenfeld
  2019-12-10 20:30             ` Jordan Glover
  0 siblings, 1 reply; 20+ messages in thread
From: Jason A. Donenfeld @ 2019-12-10 19:15 UTC (permalink / raw)
  To: Jordan Glover; +Cc: jwollrath, wireguard

On Tue, Dec 10, 2019 at 7:58 PM Jordan Glover
<Golden_Miller83@protonmail.ch> wrote:
>
> On Tuesday, December 10, 2019 5:36 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> >
> > On the other hand, if what you say is actually true in our case, and
> > nftables is utter crap, then perhaps we should scrap this nft(8) patch
> > all together and just keep pure iptables(8). DKG - you seemed to want
> > nft(8) support, though. How would you feel about that sort of
> > conclusion?
> >
> > Jason
>
> The only scenario where you really want to use nft is where iptables command
> doesn't exist. I don't know how realistic scenario it is but I assume it can
> happen in the wild. Otherwise calling iptables will take care of both iptables
> and nftables automatically if those are supported on system. That's why I
> proposed to invert current patch logic.

I reason about things a bit differently. For me, the decision is
between these two categories:

A) iptables-nft points to iptables and is available for people who
want a nft-only system. So, code against the iptables API, and mandate
that users either have iptables or iptables-nft installed, which isn't
unreasonable, considering the easy availability of each.

B) nft is the future and should be used whenever available. Support
iptables as a fallback though for old systems, and remove it as soon
as we can.

Attitudes that fall somewhere between (A) and (B) are much less
interesting to me.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 19:15           ` Jason A. Donenfeld
@ 2019-12-10 20:30             ` Jordan Glover
  2019-12-10 20:34               ` Jason A. Donenfeld
  0 siblings, 1 reply; 20+ messages in thread
From: Jordan Glover @ 2019-12-10 20:30 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: jwollrath, wireguard

On Tuesday, December 10, 2019 7:15 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:

> On Tue, Dec 10, 2019 at 7:58 PM Jordan Glover
> Golden_Miller83@protonmail.ch wrote:
>
> > On Tuesday, December 10, 2019 5:36 PM, Jason A. Donenfeld Jason@zx2c4.com wrote:
> >
> > > On the other hand, if what you say is actually true in our case, and
> > > nftables is utter crap, then perhaps we should scrap this nft(8) patch
> > > all together and just keep pure iptables(8). DKG - you seemed to want
> > > nft(8) support, though. How would you feel about that sort of
> > > conclusion?
> > > Jason
> >
> > The only scenario where you really want to use nft is where iptables command
> > doesn't exist. I don't know how realistic scenario it is but I assume it can
> > happen in the wild. Otherwise calling iptables will take care of both iptables
> > and nftables automatically if those are supported on system. That's why I
> > proposed to invert current patch logic.
>
> I reason about things a bit differently. For me, the decision is
> between these two categories:
>
> A) iptables-nft points to iptables and is available for people who
> want a nft-only system. So, code against the iptables API, and mandate
> that users either have iptables or iptables-nft installed, which isn't
> unreasonable, considering the easy availability of each.
>
> B) nft is the future and should be used whenever available. Support
> iptables as a fallback though for old systems, and remove it as soon
> as we can.
>
> Attitudes that fall somewhere between (A) and (B) are much less
> interesting to me.

Isn't future goal to drop those firewall hacks altogether? The future of
nft may be irrelevant then and effort should go for iptables which works
on more systems

Jordan
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 20:30             ` Jordan Glover
@ 2019-12-10 20:34               ` Jason A. Donenfeld
  0 siblings, 0 replies; 20+ messages in thread
From: Jason A. Donenfeld @ 2019-12-10 20:34 UTC (permalink / raw)
  To: Jordan Glover; +Cc: jwollrath, wireguard

On Tue, Dec 10, 2019 at 9:30 PM Jordan Glover
<Golden_Miller83@protonmail.ch> wrote:
>
> On Tuesday, December 10, 2019 7:15 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> > On Tue, Dec 10, 2019 at 7:58 PM Jordan Glover
> > Golden_Miller83@protonmail.ch wrote:
> >
> > > On Tuesday, December 10, 2019 5:36 PM, Jason A. Donenfeld Jason@zx2c4.com wrote:
> > >
> > > > On the other hand, if what you say is actually true in our case, and
> > > > nftables is utter crap, then perhaps we should scrap this nft(8) patch
> > > > all together and just keep pure iptables(8). DKG - you seemed to want
> > > > nft(8) support, though. How would you feel about that sort of
> > > > conclusion?
> > > > Jason
> > >
> > > The only scenario where you really want to use nft is where iptables command
> > > doesn't exist. I don't know how realistic scenario it is but I assume it can
> > > happen in the wild. Otherwise calling iptables will take care of both iptables
> > > and nftables automatically if those are supported on system. That's why I
> > > proposed to invert current patch logic.
> >
> > I reason about things a bit differently. For me, the decision is
> > between these two categories:
> >
> > A) iptables-nft points to iptables and is available for people who
> > want a nft-only system. So, code against the iptables API, and mandate
> > that users either have iptables or iptables-nft installed, which isn't
> > unreasonable, considering the easy availability of each.
> >
> > B) nft is the future and should be used whenever available. Support
> > iptables as a fallback though for old systems, and remove it as soon
> > as we can.
> >
> > Attitudes that fall somewhere between (A) and (B) are much less
> > interesting to me.
>
> Isn't future goal to drop those firewall hacks altogether? The future of
> nft may be irrelevant then and effort should go for iptables which works
> on more systems

Yes, but that means likely kernel patches, which means a very very
long deployment timeline.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 17:12     ` Roman Mamedov
                         ` (2 preceding siblings ...)
  2019-12-10 17:36       ` Jason A. Donenfeld
@ 2019-12-10 21:56       ` Vasili Pupkin
  2019-12-10 22:09         ` Jason A. Donenfeld
  3 siblings, 1 reply; 20+ messages in thread
From: Vasili Pupkin @ 2019-12-10 21:56 UTC (permalink / raw)
  To: Roman Mamedov, Jason A. Donenfeld; +Cc: jwollrath, wireguard

On 10.12.2019 20:12, Roman Mamedov wrote:
> On Tue, 10 Dec 2019 17:54:49 +0100
> "Jason A. Donenfeld" <Jason@zx2c4.com> wrote:
>
>> iptables rules and nftables rules can co-exist just fine, without any
>> translation needed. Indeed if your iptables is symlinked to
>> iptables-nft, then you'll insert nftables rules when you try to insert
>> iptables rules, but it really doesn't matter much either way (AFAIK).
>> I figured I'd prefer nftables over iptables if available because I
>> presume, without any metrics, that nftables is probably faster and
>> slicker or something.
> nftables is slower than iptables across pretty much every metric[1][2]. It
> only wins where a pathological case is used for the iptables counterpart (e.g.
> tons of single IPs as individual rules and without ipset). It is a disaster
> that it is purported to be the iptables replacement, just for the syntax and
> non-essential whistles such as updating rules in place or something. And
> personally I don't prefer the new syntax either. It's the systemd and
> pulseaudio story all over again, where something more convoluted, less reliable
> and of lower quality is passed for a replacement of stuff that actually worked,
> but was deemed "unsexy" and arbitrarly declared as deprecated.
>
> [1] http://www.diva-portal.org/smash/get/diva2:1212650/FULLTEXT01.pdf
> [2] https://developers.redhat.com/blog/2017/04/11/benchmarking-nftables/
>
As far as I know both of them are maintained in the same repository and 
both use the same userspace library to interact with the kernel and down 
there all the rules are translated into BPF code which in turn is 
compiled into machine code by kernel BPF JIT compiler. So identical 
rules should show exactly the same performance. nft syntax is odd and 
its curvy braces aren't easy to pass in command line, it is more 
difficult to delete rules from nft table, but it also allows to arrange 
rules into separate chains easier and also provide a native way to 
define sets which are then processed very efficiently as seen in [2]. 
Anyway netfilter infrastructure developers have chosen nft as 
replacement to iptables and it will slowly phase it out, but this will 
be a very slow process and it is already going for 10 years now, 
everything is written in iptables.

I don't see a big problem. Just choose any of the console command 
present on the system and print warning if neither of them is found. 
wg-quick shows its log in console and these rules will not cause any 
problems in most setups anyway. If this is such an issue then it can be 
opted by a command line argument (i.e. use iptables, nft or none) but it 
is already an overshoot. Personally I don't even think that the issue 
should be fixed in wireguard. The weak host model affects any other 
network setups and all the other vpn's just as wireguard and even if 
wg-quick patch it somehow for wireguard interface the end system will 
still be open to attacks on other interfaces. The strong host model 
should be chosen as default by kernel developers or configured system 
wide by an administrator.

Also.. what about other systems, windows, android etc?
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 21:56       ` Vasili Pupkin
@ 2019-12-10 22:09         ` Jason A. Donenfeld
  2019-12-10 22:27           ` Vasili Pupkin
  0 siblings, 1 reply; 20+ messages in thread
From: Jason A. Donenfeld @ 2019-12-10 22:09 UTC (permalink / raw)
  To: Vasili Pupkin; +Cc: jwollrath, wireguard

On Tue, Dec 10, 2019 at 11:03 PM Vasili Pupkin <diggest@gmail.com> wrote:
> As far as I know both of them are maintained in the same repository and
> both use the same userspace library to interact with the kernel and down
> there all the rules are translated into BPF code which in turn is
> compiled into machine code by kernel BPF JIT compiler.

"bpfilter" is a WIP, but that's not today how iptables or nftables
work, at all. I'm not sure your statement about userspace is entirely
correct either.
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 22:09         ` Jason A. Donenfeld
@ 2019-12-10 22:27           ` Vasili Pupkin
  2019-12-12 11:21             ` Jason A. Donenfeld
  0 siblings, 1 reply; 20+ messages in thread
From: Vasili Pupkin @ 2019-12-10 22:27 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: jwollrath, wireguard

On 11.12.2019 1:09, Jason A. Donenfeld wrote:
> On Tue, Dec 10, 2019 at 11:03 PM Vasili Pupkin <diggest@gmail.com> wrote:
>> As far as I know both of them are maintained in the same repository and
>> both use the same userspace library to interact with the kernel and down
>> there all the rules are translated into BPF code which in turn is
>> compiled into machine code by kernel BPF JIT compiler.
> "bpfilter" is a WIP, but that's not today how iptables or nftables
> work, at all. I'm not sure your statement about userspace is entirely
> correct either.
>
May be it is a road map plan and there will be no difference in 
performance when it will be implemented then. I just recall an 
announcement somewhere so it is just a speculation from my side, sorry
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

* Re: [PATCH] wg-quick: linux: add support for nft and prefer it
  2019-12-10 22:27           ` Vasili Pupkin
@ 2019-12-12 11:21             ` Jason A. Donenfeld
  0 siblings, 0 replies; 20+ messages in thread
From: Jason A. Donenfeld @ 2019-12-12 11:21 UTC (permalink / raw)
  To: wireguard; +Cc: jwollrath

I think in the end we'll ship the nftables code. Fedora is defaulting
their stuff to nftables now [1][2]. That means systemd-networkd might
need or want (speculation) to update their firewall-util.c [3] to
support it. And knowing their attitudes on this sort of thing, that
means they'll probably (speculation) sunset iptables support and start
mandating nftables-enabled kernels. That in turn means non-nftables
kernels will probably become fewer and fewer. Some readers on this
list might vomit at that kind of reasoning, but I think it nonetheless
might reflect a practical reality of the ecosystem that wg-quick(8)
lives in. So at the moment, we'll support both iptables(8) and nft(8),
preferring the latter if it exists.

[1] https://fedoraproject.org/wiki/Changes/firewalld_default_to_nftables
[2] https://fedoraproject.org/wiki/Changes/iptables-nft-default
[3] https://github.com/systemd/systemd/blob/master/src/shared/firewall-util.c
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

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

end of thread, other threads:[~2019-12-12 11:21 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 15:48 [PATCH] wg-quick: linux: add support for nft and prefer it Jason A. Donenfeld
2019-12-10 16:52 ` Jordan Glover
2019-12-10 16:54   ` Jason A. Donenfeld
2019-12-10 17:05     ` Jordan Glover
2019-12-10 17:11       ` Jason A. Donenfeld
2019-12-10 17:12     ` Roman Mamedov
2019-12-10 17:28       ` Davide Depau
2019-12-10 17:33       ` Matthias Urlichs
2019-12-10 17:36       ` Jason A. Donenfeld
2019-12-10 18:00         ` Roman Mamedov
2019-12-10 18:58         ` Jordan Glover
2019-12-10 19:15           ` Jason A. Donenfeld
2019-12-10 20:30             ` Jordan Glover
2019-12-10 20:34               ` Jason A. Donenfeld
2019-12-10 21:56       ` Vasili Pupkin
2019-12-10 22:09         ` Jason A. Donenfeld
2019-12-10 22:27           ` Vasili Pupkin
2019-12-12 11:21             ` Jason A. Donenfeld
2019-12-10 17:31 ` Vasili Pupkin
2019-12-10 17:38   ` Jason A. Donenfeld

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.