[iptables PATCH] ebtables: Include 'bitmask' value when comparing rules

[iptables PATCH] ebtables: Include 'bitmask' value when comparing rules

[Phil Sutter]

The former FIXME comment pointed at the fact that struct ebt_entry does
not have a 'flags' field (unlike struct ipt_ip). In fact, ebt_entry's
equivalent is 'bitmask' field. Comparing that instead is the right
thing to do, even though it does not seem to make a difference in
practice: No rule options alter just the bitmask value, nor is it
possible to fill an associated field with default values (e.g. all-zero
MAC and mask).

Since the situation described above might change and there is a slight
performance improvement in some cases (e.g. comparing rules differing
only by specified/omitted source/dest MAC address), add the check
anyway.

Suggested-by: Michael Estner <michaelestner@web.de>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 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 922ce98385400..f4a3c69ac1660 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.43.0

Re: [iptables PATCH] ebtables: Include 'bitmask' value when comparing rules

[Phil Sutter]

On Wed, Jun 12, 2024 at 02:41:09PM +0200, Phil Sutter wrote:
> The former FIXME comment pointed at the fact that struct ebt_entry does
> not have a 'flags' field (unlike struct ipt_ip). In fact, ebt_entry's
> equivalent is 'bitmask' field. Comparing that instead is the right
> thing to do, even though it does not seem to make a difference in
> practice: No rule options alter just the bitmask value, nor is it
> possible to fill an associated field with default values (e.g. all-zero
> MAC and mask).
> 
> Since the situation described above might change and there is a slight
> performance improvement in some cases (e.g. comparing rules differing
> only by specified/omitted source/dest MAC address), add the check
> anyway.
> 
> Suggested-by: Michael Estner <michaelestner@web.de>
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Also applied.