[iptables PATCH] ebtables: Dump atomic waste

[iptables PATCH] ebtables: Dump atomic waste

[Phil Sutter]

With ebtables-nft.8 now educating people about the missing
functionality, get rid of atomic remains in source code. This eliminates
mostly comments except for --atomic-commit which was treated as alias of
--init-table. People not using the latter are probably trying to
atomic-commit from an atomic-file which in turn is not supported, so no
point keeping it.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-eb.c | 53 -------------------------------------------
 1 file changed, 53 deletions(-)

diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index 6c58adaa66c1e..6e35f58ee685f 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -211,10 +211,6 @@ struct option ebt_original_options[] =
 	{ "new-chain"      , required_argument, 0, 'N' },
 	{ "rename-chain"   , required_argument, 0, 'E' },
 	{ "delete-chain"   , optional_argument, 0, 'X' },
-	{ "atomic-init"    , no_argument      , 0, 7   },
-	{ "atomic-commit"  , no_argument      , 0, 8   },
-	{ "atomic-file"    , required_argument, 0, 9   },
-	{ "atomic-save"    , no_argument      , 0, 10  },
 	{ "init-table"     , no_argument      , 0, 11  },
 	{ "concurrent"     , no_argument      , 0, 13  },
 	{ 0 }
@@ -320,10 +316,6 @@ static void print_help(const struct xtables_target *t,
 "--new-chain -N chain          : create a user defined chain\n"
 "--rename-chain -E old new     : rename a chain\n"
 "--delete-chain -X [chain]     : delete a user defined chain\n"
-"--atomic-commit               : update the kernel w/t table contained in <FILE>\n"
-"--atomic-init                 : put the initial kernel table into <FILE>\n"
-"--atomic-save                 : put the current kernel table into <FILE>\n"
-"--atomic-file file            : set <FILE> to file\n\n"
 "Options:\n"
 "--proto  -p [!] proto         : protocol hexadecimal, by name or LENGTH\n"
 "--src    -s [!] address[/mask]: source mac address\n"
@@ -1087,54 +1079,9 @@ print_zero:
 					       "Use --Lmac2 with -L");
 			flags |= LIST_MAC2;
 			break;
-		case 8 : /* atomic-commit */
-/*
-			replace->command = c;
-			if (OPT_COMMANDS)
-				ebt_print_error2("Multiple commands are not allowed");
-			replace->flags |= OPT_COMMAND;
-			if (!replace->filename)
-				ebt_print_error2("No atomic file specified");*/
-			/* Get the information from the file */
-			/*ebt_get_table(replace, 0);*/
-			/* We don't want the kernel giving us its counters,
-			 * they would overwrite the counters extracted from
-			 * the file */
-			/*replace->num_counters = 0;*/
-			/* Make sure the table will be written to the kernel */
-			/*free(replace->filename);
-			replace->filename = NULL;
-			break;*/
-		/*case 7 :*/ /* atomic-init */
-		/*case 10:*/ /* atomic-save */
 		case 11: /* init-table */
 			nft_cmd_table_flush(h, *table, false);
 			return 1;
-		/*
-			replace->command = c;
-			if (OPT_COMMANDS)
-				ebt_print_error2("Multiple commands are not allowed");
-			if (c != 11 && !replace->filename)
-				ebt_print_error2("No atomic file specified");
-			replace->flags |= OPT_COMMAND;
-			{
-				char *tmp = replace->filename;*/
-
-				/* Get the kernel table */
-				/*replace->filename = NULL;
-				ebt_get_kernel_table(replace, c == 10 ? 0 : 1);
-				replace->filename = tmp;
-			}
-			break;
-		case 9 :*/ /* atomic */
-			/*
-			if (OPT_COMMANDS)
-				ebt_print_error2("--atomic has to come before the command");*/
-			/* A possible memory leak here, but this is not
-			 * executed in daemon mode */
-			/*replace->filename = (char *)malloc(strlen(optarg) + 1);
-			strcpy(replace->filename, optarg);
-			break; */
 		case 13 :
 			break;
 		case 1 :
-- 
2.32.0

Re: [iptables PATCH] ebtables: Dump atomic waste

[Pablo Neira Ayuso]

Hi Phil,

On Fri, Jul 30, 2021 at 12:37:15PM +0200, Phil Sutter wrote:
> With ebtables-nft.8 now educating people about the missing
> functionality, get rid of atomic remains in source code. This eliminates
> mostly comments except for --atomic-commit which was treated as alias of
> --init-table. People not using the latter are probably trying to
> atomic-commit from an atomic-file which in turn is not supported, so no
> point keeping it.

That's fine.

If there's any need in the future for emulating this in the future, it
should be possible to map atomic-save to ebtables-save and
atomic-commit to ebtables-restore.

Anyway, this one of the exotic options in ebtables that makes it
different from ip,ip6,arptables. Given there are better tools now that
are aligned with the more orthodox approach, this should be OK.

Thanks.

Re: [iptables PATCH] ebtables: Dump atomic waste

[Phil Sutter]

On Mon, Aug 02, 2021 at 11:04:04AM +0200, Pablo Neira Ayuso wrote:
> Hi Phil,
> 
> On Fri, Jul 30, 2021 at 12:37:15PM +0200, Phil Sutter wrote:
> > With ebtables-nft.8 now educating people about the missing
> > functionality, get rid of atomic remains in source code. This eliminates
> > mostly comments except for --atomic-commit which was treated as alias of
> > --init-table. People not using the latter are probably trying to
> > atomic-commit from an atomic-file which in turn is not supported, so no
> > point keeping it.
> 
> That's fine.
> 
> If there's any need in the future for emulating this in the future, it
> should be possible to map atomic-save to ebtables-save and
> atomic-commit to ebtables-restore.

I had considered that, but the binary format of atomic-file drove me
off: If we can't support existing atomic-files easily, we better deny
unless someone has a strong argument to do it. And then I'd try to
support it fully, so it's not a half-ass solution with a catch. :)

> Anyway, this one of the exotic options in ebtables that makes it
> different from ip,ip6,arptables. Given there are better tools now that
> are aligned with the more orthodox approach, this should be OK.

Let's hope most users went with the familiar save/restore approach
instead of opening a whole new can for ebtables alone.

Thanks, Phil

Re: [iptables PATCH] ebtables: Dump atomic waste

[Pablo Neira Ayuso]

On Mon, Aug 02, 2021 at 01:05:55PM +0200, Phil Sutter wrote:
> On Mon, Aug 02, 2021 at 11:04:04AM +0200, Pablo Neira Ayuso wrote:
> > Hi Phil,
> > 
> > On Fri, Jul 30, 2021 at 12:37:15PM +0200, Phil Sutter wrote:
> > > With ebtables-nft.8 now educating people about the missing
> > > functionality, get rid of atomic remains in source code. This eliminates
> > > mostly comments except for --atomic-commit which was treated as alias of
> > > --init-table. People not using the latter are probably trying to
> > > atomic-commit from an atomic-file which in turn is not supported, so no
> > > point keeping it.
> > 
> > That's fine.
> > 
> > If there's any need in the future for emulating this in the future, it
> > should be possible to map atomic-save to ebtables-save and
> > atomic-commit to ebtables-restore.
> 
> I had considered that, but the binary format of atomic-file drove me
> off: If we can't support existing atomic-files easily, we better deny
> unless someone has a strong argument to do it. And then I'd try to
> support it fully, so it's not a half-ass solution with a catch. :)

That's sensible.

> > Anyway, this one of the exotic options in ebtables that makes it
> > different from ip,ip6,arptables. Given there are better tools now that
> > are aligned with the more orthodox approach, this should be OK.
> 
> Let's hope most users went with the familiar save/restore approach
> instead of opening a whole new can for ebtables alone.

Let's do that.

Thanks.

Re: [iptables PATCH] ebtables: Dump atomic waste

[Pablo Neira Ayuso]

offlist

On Mon, Aug 02, 2021 at 01:51:27PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Aug 02, 2021 at 01:05:55PM +0200, Phil Sutter wrote:
> > On Mon, Aug 02, 2021 at 11:04:04AM +0200, Pablo Neira Ayuso wrote:
> > > Hi Phil,
> > > 
> > > On Fri, Jul 30, 2021 at 12:37:15PM +0200, Phil Sutter wrote:
> > > > With ebtables-nft.8 now educating people about the missing
> > > > functionality, get rid of atomic remains in source code. This eliminates
> > > > mostly comments except for --atomic-commit which was treated as alias of
> > > > --init-table. People not using the latter are probably trying to
> > > > atomic-commit from an atomic-file which in turn is not supported, so no
> > > > point keeping it.
> > > 
> > > That's fine.
> > > 
> > > If there's any need in the future for emulating this in the future, it
> > > should be possible to map atomic-save to ebtables-save and
> > > atomic-commit to ebtables-restore.
> > 
> > I had considered that, but the binary format of atomic-file drove me
> > off: If we can't support existing atomic-files easily, we better deny
> > unless someone has a strong argument to do it. And then I'd try to
> > support it fully, so it's not a half-ass solution with a catch. :)
> 
> That's sensible.

I did not know the file format is different, I never used exotic
ebtables features ever myself.

Re: [iptables PATCH] ebtables: Dump atomic waste

[Pablo Neira Ayuso]

On Mon, Aug 02, 2021 at 01:59:02PM +0200, Pablo Neira Ayuso wrote:
> offlist

actually not :)

Re: [iptables PATCH] ebtables: Dump atomic waste

[Phil Sutter]

On Mon, Aug 02, 2021 at 01:59:30PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Aug 02, 2021 at 01:59:02PM +0200, Pablo Neira Ayuso wrote:
> > offlist
> 
> actually not :)

:D

The file is written by store_table_in_file() in communication.c, writing
a struct ebt_replace plus a number of struct ebt_entries. At this point
I stopped, assuming that translating back and forth would involve too
much legacy cruft.

Cheers, Phil

Re: [iptables PATCH] ebtables: Dump atomic waste

[Pablo Neira Ayuso]

On Mon, Aug 02, 2021 at 02:54:48PM +0200, Phil Sutter wrote:
> On Mon, Aug 02, 2021 at 01:59:30PM +0200, Pablo Neira Ayuso wrote:
> > On Mon, Aug 02, 2021 at 01:59:02PM +0200, Pablo Neira Ayuso wrote:
> > > offlist
> > 
> > actually not :)
> 
> :D
> 
> The file is written by store_table_in_file() in communication.c, writing
> a struct ebt_replace plus a number of struct ebt_entries. At this point
> I stopped, assuming that translating back and forth would involve too
> much legacy cruft.

Hm, so it looks like a memory dump of the ruleset blob representation
into a file.