* [iptables PATCH] ebtables-compat: parser cleanups
@ 2015-02-12 10:44 Arturo Borrero Gonzalez
2015-02-12 12:19 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Arturo Borrero Gonzalez @ 2015-02-12 10:44 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
Kill:
* commented code in the parser
* ebtables daemon stuff
* ebtables 'atomic' operations
We can bring back the code later and get in shape if required.
Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
iptables/xtables-eb.c | 167 ++-----------------------------------------------
1 file changed, 7 insertions(+), 160 deletions(-)
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index efbb3cd..632a3a0 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -282,12 +282,6 @@ static 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 }
};
@@ -425,10 +419,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"
@@ -440,10 +430,7 @@ static void print_help(const struct xtables_target *t,
"--set-counters -c chain\n"
" pcnt bcnt : set the counters of the to be added rule\n"
"--modprobe -M program : try to insert modules using this program\n"
-"--concurrent : use a file lock to support concurrent scripts\n"
"--version -V : print package version\n\n"
-"Environment variable:\n"
-/*ATOMIC_ENV_VARIABLE " : if set <FILE> (see above) will equal its value"*/
"\n\n");
for (; m != NULL; m = m->next) {
printf("\n");
@@ -453,9 +440,6 @@ static void print_help(const struct xtables_target *t,
printf("\n");
t->help();
}
-
-// if (table->help)
-// table->help(ebt_hooknames);
}
/* Execute command L */
@@ -791,10 +775,6 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table)
chain = optarg;
selected_chain = get_current_chain(chain);
flags |= OPT_COMMAND;
- /*if (!(replace->flags & OPT_KERNELDATA))
- ebt_get_kernel_table(replace, 0);*/
- /*if (optarg && (optarg[0] == '-' || !strcmp(optarg, "!")))
- ebt_print_error2("No chain name specified");*/
if (c == 'N') {
ret = nft_chain_user_add(h, chain, *table);
break;
@@ -876,27 +856,6 @@ print_zero:
if (flags & OPT_ZERO && c != 'L')
goto print_zero;
}
-
-#ifdef SILENT_DAEMON
- if (c== 'L' && exec_style == EXEC_STYLE_DAEMON)
- xtables_error(PARAMETER_PROBLEM,
- "-L not supported in daemon mode");
-#endif
-
- /*if (!(replace->flags & OPT_KERNELDATA))
- ebt_get_kernel_table(replace, 0);
- i = -1;
- if (optind < argc && argv[optind][0] != '-') {
- if ((i = ebt_get_chainnr(replace, argv[optind])) == -1)
- ebt_print_error2("Chain '%s' doesn't exist", argv[optind]);
- optind++;
- }
- if (i != -1) {
- if (c == 'Z')
- zerochain = i;
- else
- replace->selected_chain = i;
- }*/
break;
case 'V': /* Version */
if (OPT_COMMANDS)
@@ -909,11 +868,6 @@ print_zero:
printf("%s %s\n", prog_name, prog_vers);
exit(0);
case 'h': /* Help */
-#ifdef SILENT_DAEMON
- if (exec_style == EXEC_STYLE_DAEMON)
- xtables_error(PARAMETER_PROBLEM,
- "-h not supported in daemon mode");
-#endif
if (OPT_COMMANDS)
xtables_error(PARAMETER_PROBLEM,
"Multiple commands are not allowed");
@@ -921,25 +875,16 @@ print_zero:
/* All other arguments should be extension names */
while (optind < argc) {
- /*struct ebt_u_match *m;
- struct ebt_u_watcher *w;*/
-
if (!strcasecmp("list_extensions", argv[optind])) {
ebt_list_extensions(xtables_targets, cs.matches);
exit(0);
}
- /*if ((m = ebt_find_match(argv[optind])))
- ebt_add_match(new_entry, m);
- else if ((w = ebt_find_watcher(argv[optind])))
- ebt_add_watcher(new_entry, w);
- else {*/
- if (!(t = xtables_find_target(argv[optind], XTF_TRY_LOAD)))
- xtables_error(PARAMETER_PROBLEM,"Extension '%s' not found", argv[optind]);
- if (flags & OPT_JUMP)
- xtables_error(PARAMETER_PROBLEM,"Sorry, you can only see help for one target extension at a time");
- flags |= OPT_JUMP;
- cs.target = t;
- //}
+ if (!(t = xtables_find_target(argv[optind], XTF_TRY_LOAD)))
+ xtables_error(PARAMETER_PROBLEM,"Extension '%s' not found", argv[optind]);
+ if (flags & OPT_JUMP)
+ xtables_error(PARAMETER_PROBLEM,"Sorry, you can only see help for one target extension at a time");
+ flags |= OPT_JUMP;
+ cs.target = t;
optind++;
}
break;
@@ -1152,65 +1097,6 @@ big_iface_length:
"Use --Lmac2 with -L");
flags |= LIST_MAC2;
break;
- case 8 : /* atomic-commit */
-/* if (exec_style == EXEC_STYLE_DAEMON)
- ebt_print_error2("--atomic-commit is not supported in daemon mode");
- 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 */
- /* if (exec_style == EXEC_STYLE_DAEMON) {
- if (c == 7) {
- ebt_print_error2("--atomic-init is not supported in daemon mode");
- } else if (c == 10)
- ebt_print_error2("--atomic-save is not supported in daemon mode");
- ebt_print_error2("--init-table is not supported in daemon mode");
- }
- 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 (exec_style == EXEC_STYLE_DAEMON)
- ebt_print_error2("--atomic is not supported in daemon mode");
- 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 : *//* concurrent */
- /*signal(SIGINT, sighandler);
- signal(SIGTERM, sighandler);
- use_lockfd = 1;
- break;*/
case 1 :
if (!strcmp(optarg, "!"))
ebt_check_inverse2(optarg, argc, argv);
@@ -1248,21 +1134,6 @@ big_iface_length:
goto check_extension;
}
}
- /*
- if (w == NULL && c == '?')
- ebt_print_error2("Unknown argument: '%s'", argv[optind - 1], (char)optopt, (char)c);
- else if (w == NULL) {
- if (!strcmp(t->name, "standard"))
- ebt_print_error2("Unknown argument: don't forget the -t option");
- else
- ebt_print_error2("Target-specific option does not correspond with specified target");
- }
- if (ebt_errormsg[0] != '\0')
- return -1;
- if (w->used == 0) {
- ebt_add_watcher(new_entry, w);
- w->used = 1;
- }*/
check_extension:
if (command != 'A' && command != 'I' &&
command != 'D' && command != 'C')
@@ -1272,13 +1143,6 @@ check_extension:
ebt_invert = 0;
}
- /* Just in case we didn't catch an error */
- /*if (ebt_errormsg[0] != '\0')
- return -1;
-
- if (!(table = ebt_find_table(replace->name)))
- ebt_print_error2("Bad table name");*/
-
if (command == 'h' && !(flags & OPT_ZERO)) {
print_help(cs.target, cs.matches, *table);
if (exec_style == EXEC_STYLE_PRG)
@@ -1342,24 +1206,7 @@ check_extension:
} else if (command == 'D') {
ret = delete_entry(h, chain, *table, &cs, rule_nr - 1,
rule_nr_end, flags&OPT_VERBOSE);
- } /*else if (replace->command == 'C') {
- ebt_change_counters(replace, new_entry, rule_nr, rule_nr_end, &(new_entry->cnt_surplus), chcounter);
- if (ebt_errormsg[0] != '\0')
- return -1;
- }*/
- /* Commands -N, -E, -X, --atomic-commit, --atomic-commit, --atomic-save,
- * --init-table fall through */
-
- /*if (ebt_errormsg[0] != '\0')
- return -1;
- if (table->check)
- table->check(replace);
-
- if (exec_style == EXEC_STYLE_PRG) {*//* Implies ebt_errormsg[0] == '\0' */
- /*ebt_deliver_table(replace);
-
- if (replace->nentries)
- ebt_deliver_counters(replace);*/
+ }
ebt_cs_clean(&cs);
return ret;
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [iptables PATCH] ebtables-compat: parser cleanups
2015-02-12 10:44 [iptables PATCH] ebtables-compat: parser cleanups Arturo Borrero Gonzalez
@ 2015-02-12 12:19 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2015-02-12 12:19 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: netfilter-devel
On Thu, Feb 12, 2015 at 11:44:08AM +0100, Arturo Borrero Gonzalez wrote:
> Kill:
> * commented code in the parser
> * ebtables daemon stuff
> * ebtables 'atomic' operations
>
> We can bring back the code later and get in shape if required.
Users will likely run their old scripts, and those will break if they
use these options. See below.
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
> iptables/xtables-eb.c | 167 ++-----------------------------------------------
> 1 file changed, 7 insertions(+), 160 deletions(-)
>
> diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
> index efbb3cd..632a3a0 100644
> --- a/iptables/xtables-eb.c
> +++ b/iptables/xtables-eb.c
> @@ -282,12 +282,6 @@ static 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 },
I would need to double check, but I think that depending on the option
we can:
#1 Bail out with an message: "This option is not supported, sorry", so
the user knows that options is not implemented in compat. It would
still break some stuff, but the user will know.
#2 If the option is superfluous when running over nft, then just
silently turn them into noop.
Please, have a look at this.
> { 0 }
> };
>
> @@ -425,10 +419,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"
> @@ -440,10 +430,7 @@ static void print_help(const struct xtables_target *t,
> "--set-counters -c chain\n"
> " pcnt bcnt : set the counters of the to be added rule\n"
> "--modprobe -M program : try to insert modules using this program\n"
> -"--concurrent : use a file lock to support concurrent scripts\n"
> "--version -V : print package version\n\n"
> -"Environment variable:\n"
> -/*ATOMIC_ENV_VARIABLE " : if set <FILE> (see above) will equal its value"*/
> "\n\n");
> for (; m != NULL; m = m->next) {
> printf("\n");
> @@ -453,9 +440,6 @@ static void print_help(const struct xtables_target *t,
> printf("\n");
> t->help();
> }
> -
> -// if (table->help)
> -// table->help(ebt_hooknames);
> }
>
> /* Execute command L */
> @@ -791,10 +775,6 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table)
> chain = optarg;
> selected_chain = get_current_chain(chain);
> flags |= OPT_COMMAND;
> - /*if (!(replace->flags & OPT_KERNELDATA))
> - ebt_get_kernel_table(replace, 0);*/
> - /*if (optarg && (optarg[0] == '-' || !strcmp(optarg, "!")))
> - ebt_print_error2("No chain name specified");*/
> if (c == 'N') {
> ret = nft_chain_user_add(h, chain, *table);
> break;
> @@ -876,27 +856,6 @@ print_zero:
> if (flags & OPT_ZERO && c != 'L')
> goto print_zero;
> }
> -
> -#ifdef SILENT_DAEMON
> - if (c== 'L' && exec_style == EXEC_STYLE_DAEMON)
> - xtables_error(PARAMETER_PROBLEM,
> - "-L not supported in daemon mode");
> -#endif
> -
> - /*if (!(replace->flags & OPT_KERNELDATA))
> - ebt_get_kernel_table(replace, 0);
> - i = -1;
> - if (optind < argc && argv[optind][0] != '-') {
> - if ((i = ebt_get_chainnr(replace, argv[optind])) == -1)
> - ebt_print_error2("Chain '%s' doesn't exist", argv[optind]);
> - optind++;
> - }
> - if (i != -1) {
> - if (c == 'Z')
> - zerochain = i;
> - else
> - replace->selected_chain = i;
> - }*/
> break;
> case 'V': /* Version */
> if (OPT_COMMANDS)
> @@ -909,11 +868,6 @@ print_zero:
> printf("%s %s\n", prog_name, prog_vers);
> exit(0);
> case 'h': /* Help */
> -#ifdef SILENT_DAEMON
> - if (exec_style == EXEC_STYLE_DAEMON)
> - xtables_error(PARAMETER_PROBLEM,
> - "-h not supported in daemon mode");
> -#endif
> if (OPT_COMMANDS)
> xtables_error(PARAMETER_PROBLEM,
> "Multiple commands are not allowed");
> @@ -921,25 +875,16 @@ print_zero:
>
> /* All other arguments should be extension names */
> while (optind < argc) {
> - /*struct ebt_u_match *m;
> - struct ebt_u_watcher *w;*/
> -
> if (!strcasecmp("list_extensions", argv[optind])) {
> ebt_list_extensions(xtables_targets, cs.matches);
> exit(0);
> }
> - /*if ((m = ebt_find_match(argv[optind])))
> - ebt_add_match(new_entry, m);
> - else if ((w = ebt_find_watcher(argv[optind])))
> - ebt_add_watcher(new_entry, w);
> - else {*/
> - if (!(t = xtables_find_target(argv[optind], XTF_TRY_LOAD)))
> - xtables_error(PARAMETER_PROBLEM,"Extension '%s' not found", argv[optind]);
> - if (flags & OPT_JUMP)
> - xtables_error(PARAMETER_PROBLEM,"Sorry, you can only see help for one target extension at a time");
> - flags |= OPT_JUMP;
> - cs.target = t;
> - //}
> + if (!(t = xtables_find_target(argv[optind], XTF_TRY_LOAD)))
> + xtables_error(PARAMETER_PROBLEM,"Extension '%s' not found", argv[optind]);
> + if (flags & OPT_JUMP)
> + xtables_error(PARAMETER_PROBLEM,"Sorry, you can only see help for one target extension at a time");
> + flags |= OPT_JUMP;
> + cs.target = t;
> optind++;
> }
> break;
> @@ -1152,65 +1097,6 @@ big_iface_length:
> "Use --Lmac2 with -L");
> flags |= LIST_MAC2;
> break;
> - case 8 : /* atomic-commit */
> -/* if (exec_style == EXEC_STYLE_DAEMON)
> - ebt_print_error2("--atomic-commit is not supported in daemon mode");
> - 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 */
> - /* if (exec_style == EXEC_STYLE_DAEMON) {
> - if (c == 7) {
> - ebt_print_error2("--atomic-init is not supported in daemon mode");
> - } else if (c == 10)
> - ebt_print_error2("--atomic-save is not supported in daemon mode");
> - ebt_print_error2("--init-table is not supported in daemon mode");
> - }
> - 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 (exec_style == EXEC_STYLE_DAEMON)
> - ebt_print_error2("--atomic is not supported in daemon mode");
> - 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 : *//* concurrent */
> - /*signal(SIGINT, sighandler);
> - signal(SIGTERM, sighandler);
> - use_lockfd = 1;
> - break;*/
> case 1 :
> if (!strcmp(optarg, "!"))
> ebt_check_inverse2(optarg, argc, argv);
> @@ -1248,21 +1134,6 @@ big_iface_length:
> goto check_extension;
> }
> }
> - /*
> - if (w == NULL && c == '?')
> - ebt_print_error2("Unknown argument: '%s'", argv[optind - 1], (char)optopt, (char)c);
> - else if (w == NULL) {
> - if (!strcmp(t->name, "standard"))
> - ebt_print_error2("Unknown argument: don't forget the -t option");
> - else
> - ebt_print_error2("Target-specific option does not correspond with specified target");
> - }
> - if (ebt_errormsg[0] != '\0')
> - return -1;
> - if (w->used == 0) {
> - ebt_add_watcher(new_entry, w);
> - w->used = 1;
> - }*/
> check_extension:
> if (command != 'A' && command != 'I' &&
> command != 'D' && command != 'C')
> @@ -1272,13 +1143,6 @@ check_extension:
> ebt_invert = 0;
> }
>
> - /* Just in case we didn't catch an error */
> - /*if (ebt_errormsg[0] != '\0')
> - return -1;
> -
> - if (!(table = ebt_find_table(replace->name)))
> - ebt_print_error2("Bad table name");*/
> -
> if (command == 'h' && !(flags & OPT_ZERO)) {
> print_help(cs.target, cs.matches, *table);
> if (exec_style == EXEC_STYLE_PRG)
> @@ -1342,24 +1206,7 @@ check_extension:
> } else if (command == 'D') {
> ret = delete_entry(h, chain, *table, &cs, rule_nr - 1,
> rule_nr_end, flags&OPT_VERBOSE);
> - } /*else if (replace->command == 'C') {
> - ebt_change_counters(replace, new_entry, rule_nr, rule_nr_end, &(new_entry->cnt_surplus), chcounter);
> - if (ebt_errormsg[0] != '\0')
> - return -1;
> - }*/
> - /* Commands -N, -E, -X, --atomic-commit, --atomic-commit, --atomic-save,
> - * --init-table fall through */
> -
> - /*if (ebt_errormsg[0] != '\0')
> - return -1;
> - if (table->check)
> - table->check(replace);
> -
> - if (exec_style == EXEC_STYLE_PRG) {*//* Implies ebt_errormsg[0] == '\0' */
> - /*ebt_deliver_table(replace);
> -
> - if (replace->nentries)
> - ebt_deliver_counters(replace);*/
> + }
>
> ebt_cs_clean(&cs);
> return ret;
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-02-12 12:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-12 10:44 [iptables PATCH] ebtables-compat: parser cleanups Arturo Borrero Gonzalez
2015-02-12 12:19 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).