[PATCH] iptables: cleanup FIXME

[PATCH] iptables: cleanup FIXME

[Michael Estner]

Remove obsolet FIXME since struct ebt_entry has no flags var.
Update the debug output.

Signed-off-by: Michael Estner <michaelestner@web.de>
---
 iptables/nft-bridge.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 922ce983..f5deaa93 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -373,9 +373,8 @@ static bool nft_bridge_is_same(const struct iptables_command_state *cs_a,
 	int i;

 	if (a->ethproto != b->ethproto ||
-	    /* FIXME: a->flags != b->flags || */
 	    a->invflags != b->invflags) {
-		DEBUGP("different proto/flags/invflags\n");
+		DEBUGP("different proto/invflags\n");
 		return false;
 	}

--
2.25.1

Re: [PATCH] iptables: cleanup FIXME

[Phil Sutter]

On Thu, May 23, 2024 at 04:50:58PM +0200, Michael Estner wrote:
> Remove obsolet FIXME since struct ebt_entry has no flags var.
> Update the debug output.

It never had, but there's bitmask and I wonder if it should compare
those values instead.

Cheers, Phil

[PATCH] iptables: cleanup FIXME

[Michael Estner]

I checked bitmask in the ebt_entry struct in iptables/xshared.h
Should be compared here since bitmask needs to be the first
field in the struct ebt_entry.

[PATCH] iptables: cleanup FIXME

[Michael Estner]

Rework FIXME since struct ebt_entry has no flags var.
Use variable bitmask instead.
Update the debug output.

Signed-off-by: Michael Estner <michaelestner@web.de>
---
 iptables/nft-bridge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 922ce983..f4a3c69a 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -373,9 +373,9 @@ static bool nft_bridge_is_same(const struct iptables_command_state *cs_a,
 	int i;

 	if (a->ethproto != b->ethproto ||
-	    /* FIXME: a->flags != b->flags || */
+	    a->bitmask != b->bitmask ||
 	    a->invflags != b->invflags) {
-		DEBUGP("different proto/flags/invflags\n");
+		DEBUGP("different proto/bitmask/invflags\n");
 		return false;
 	}

--
2.25.1

Re: [PATCH] iptables: cleanup FIXME

[Phil Sutter]

Hi Michael,

On Fri, May 24, 2024 at 03:24:51PM +0200, Michael Estner wrote:
> I checked bitmask in the ebt_entry struct in iptables/xshared.h
> Should be compared here since bitmask needs to be the first
> field in the struct ebt_entry.

The reason why 'bitmask' has to be the first field is that in kernel
space, the first bit in it is used to distinguish list element types
between 'struct ebt_entries' and 'struct ebt_entry'. See
EBT_ENTRY_OR_ENTRIES define and the related comment in
include/uapi/linux/netfilter_bridge/ebtables.h for reference.

While it seems sensible to do, I wonder why things seem to work fine
even without it. Do we find a corner-case which makes it necessary to
compare 'bitmask'? Or the other way round, is there a case which breaks
if we do?

Cheers, Phil