diff mbox series

[iptables] extensions: connmark: remove non-working translation

Message ID 20180219114755.27051-1-fw@strlen.de
State Accepted
Delegated to: Pablo Neira
Headers show
Series [iptables] extensions: connmark: remove non-working translation | expand

Commit Message

Florian Westphal Feb. 19, 2018, 11:47 a.m. UTC
... and return 0 so output reflects that no translation was performed.

iptables-translate -A I -j CONNMARK --save-mark --mask 0xff
nft # -A I -j CONNMARK --save-mark --mask 0xff

The translation that was performed:
nft add rule ip mangle PREROUTING counter meta mark set ct mark and 0xff

will clear (zero) most bits:
  [ meta load mark => reg 1 ]
  [ bitwise reg 1 = (reg=1 & 0x000000ff ) ^ 0x00000000 ]
  [ ct set mark with reg 1 ]

The xtables module however does this:

newmark = (ct->mark & ~info->ctmask) ^
           (skb->mark & info->nfmask);

I.e., for ctmark mask defines what to clear,
for nfmark what to keep, i.e. we're supposed to only alter the lower
bits of the ctmark.

nftables can't do this at the moment because bitwise operator RHS
requires immediate values.

same is true for 'restore'.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 extensions/libxt_CONNMARK.c      | 22 ++++++++++------------
 extensions/libxt_CONNMARK.txlate |  6 ------
 2 files changed, 10 insertions(+), 18 deletions(-)

Comments

Pablo Neira Ayuso Feb. 19, 2018, 5:25 p.m. UTC | #1
On Mon, Feb 19, 2018 at 12:47:55PM +0100, Florian Westphal wrote:
> ... and return 0 so output reflects that no translation was performed.
> 
> iptables-translate -A I -j CONNMARK --save-mark --mask 0xff
> nft # -A I -j CONNMARK --save-mark --mask 0xff
> 
> The translation that was performed:
> nft add rule ip mangle PREROUTING counter meta mark set ct mark and 0xff
> 
> will clear (zero) most bits:
>   [ meta load mark => reg 1 ]
>   [ bitwise reg 1 = (reg=1 & 0x000000ff ) ^ 0x00000000 ]
>   [ ct set mark with reg 1 ]
> 
> The xtables module however does this:
> 
> newmark = (ct->mark & ~info->ctmask) ^
>            (skb->mark & info->nfmask);
> 
> I.e., for ctmark mask defines what to clear,
> for nfmark what to keep, i.e. we're supposed to only alter the lower
> bits of the ctmark.
> 
> nftables can't do this at the moment because bitwise operator RHS
> requires immediate values.
> 
> same is true for 'restore'.

OK. Please push this.

Please place this somewhere in the wiki so we don't forget we need
some plumbing to support this usecase.

Thanks !
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c
index c7933464101b..94984cdc8a21 100644
--- a/extensions/libxt_CONNMARK.c
+++ b/extensions/libxt_CONNMARK.c
@@ -371,20 +371,18 @@  static int connmark_tg_xlate(struct xt_xlate *xl,
 				     info->ctmark, ~info->ctmask);
 		break;
 	case XT_CONNMARK_SAVE:
-		xt_xlate_add(xl, "ct mark set mark");
-		if (!(info->nfmask == UINT32_MAX &&
-		    info->ctmask == UINT32_MAX)) {
-			if (info->nfmask == info->ctmask)
-				xt_xlate_add(xl, " and 0x%x", info->nfmask);
-		}
+		if (info->nfmask == info->ctmask &&
+		    info->nfmask == UINT32_MAX)
+			xt_xlate_add(xl, "ct mark set mark");
+		else
+			return 0;
 		break;
 	case XT_CONNMARK_RESTORE:
-		xt_xlate_add(xl, "meta mark set ct mark");
-		if (!(info->nfmask == UINT32_MAX &&
-		    info->ctmask == UINT32_MAX)) {
-			if (info->nfmask == info->ctmask)
-				xt_xlate_add(xl, " and 0x%x", info->nfmask);
-		}
+		if (info->nfmask == info->ctmask &&
+		    info->nfmask == UINT32_MAX)
+			xt_xlate_add(xl, "meta mark set ct mark");
+		else
+			return 0;
 		break;
 	}
 
diff --git a/extensions/libxt_CONNMARK.txlate b/extensions/libxt_CONNMARK.txlate
index a47cbb2b00db..ce40ae5ea65e 100644
--- a/extensions/libxt_CONNMARK.txlate
+++ b/extensions/libxt_CONNMARK.txlate
@@ -16,11 +16,5 @@  nft add rule ip mangle PREROUTING counter ct mark set ct mark or 0x16
 iptables-translate -t mangle -A PREROUTING -j CONNMARK --save-mark
 nft add rule ip mangle PREROUTING counter ct mark set mark
 
-iptables-translate -t mangle -A PREROUTING -j CONNMARK --save-mark --mask 0x12
-nft add rule ip mangle PREROUTING counter ct mark set mark and 0x12
-
 iptables-translate -t mangle -A PREROUTING -j CONNMARK --restore-mark
 nft add rule ip mangle PREROUTING counter meta mark set ct mark
-
-iptables-translate -t mangle -A PREROUTING -j CONNMARK --restore-mark --mask 0x12
-nft add rule ip mangle PREROUTING counter meta mark set ct mark and 0x12