| Message ID | 20221201163916.30808-2-phil@nwl.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | tests: xlate: generic.txlate to pass replay test | expand |
On Thu, Dec 01, 2022 at 05:39:10PM +0100, Phil Sutter wrote: > Sadly, '-C' is in use already for --change-counters (even though > ebtables-nft does not implement this), so add a long-option only. It is > needed for xlate testsuite in replay mode, which will use '--check' > instead of '-C'. Hm, yet another of those exotic deviations (from ip{6}tables) in ebtables. This -C is not supported by ebtables-nft, right? If so, according to manpage, ebtables -C takes start_nr[:end_nr]. Maybe there is a chance to get this aligned with other ip{6}tables tools by checking if optarg is available? Otherwise, really check the ruleset? BTW, I'm re-reading the ebtables manpage, not sure how this feature -C was supposed to be used. Do you understand the usecase?
On Thu, Dec 08, 2022 at 10:40:22PM +0100, Pablo Neira Ayuso wrote: > On Thu, Dec 01, 2022 at 05:39:10PM +0100, Phil Sutter wrote: > > Sadly, '-C' is in use already for --change-counters (even though > > ebtables-nft does not implement this), so add a long-option only. It is > > needed for xlate testsuite in replay mode, which will use '--check' > > instead of '-C'. > > Hm, yet another of those exotic deviations (from ip{6}tables) in > ebtables. > > This -C is not supported by ebtables-nft, right? If so, > according to manpage, ebtables -C takes start_nr[:end_nr]. > > Maybe there is a chance to get this aligned with other ip{6}tables > tools by checking if optarg is available? Otherwise, really check the > ruleset? > > BTW, I'm re-reading the ebtables manpage, not sure how this feature -C > was supposed to be used. Do you understand the usecase? Yes, it's odd - so fits perfectly the rest of ebtables syntax. ;) There are two ways to use it: 1) ebtables -C <CHAIN> <RULENO> <PCNT> <BCNT> 2) ebtables -C <CHAIN> <PCNT> <BCNT> <RULESPEC> So I could check if the two parameters following the chain name are numbers or not to distinguish between --change-counters and --check, but it's ugly and with ebtables-nft not supporting one of them makes things actually worse. We need --check only for internal purposes, let's please just leave it like this - there are much more important things to work on. Cheers, Phil
On Fri, Dec 09, 2022 at 01:41:24AM +0100, Phil Sutter wrote: > On Thu, Dec 08, 2022 at 10:40:22PM +0100, Pablo Neira Ayuso wrote: > > On Thu, Dec 01, 2022 at 05:39:10PM +0100, Phil Sutter wrote: > > > Sadly, '-C' is in use already for --change-counters (even though > > > ebtables-nft does not implement this), so add a long-option only. It is > > > needed for xlate testsuite in replay mode, which will use '--check' > > > instead of '-C'. > > > > Hm, yet another of those exotic deviations (from ip{6}tables) in > > ebtables. > > > > This -C is not supported by ebtables-nft, right? If so, > > according to manpage, ebtables -C takes start_nr[:end_nr]. > > > > Maybe there is a chance to get this aligned with other ip{6}tables > > tools by checking if optarg is available? Otherwise, really check the > > ruleset? > > > > BTW, I'm re-reading the ebtables manpage, not sure how this feature -C > > was supposed to be used. Do you understand the usecase? > > Yes, it's odd - so fits perfectly the rest of ebtables syntax. ;) > > There are two ways to use it: > > 1) ebtables -C <CHAIN> <RULENO> <PCNT> <BCNT> > 2) ebtables -C <CHAIN> <PCNT> <BCNT> <RULESPEC> > > So I could check if the two parameters following the chain name are > numbers or not to distinguish between --change-counters and --check, but > it's ugly and with ebtables-nft not supporting one of them makes things > actually worse. > > We need --check only for internal purposes, let's please just leave it > like this - there are much more important things to work on. OK, just an idea in case there is a need for getting ebtables more aligned with other xtables userspace.
On Fri, Dec 09, 2022 at 04:23:49PM +0100, Pablo Neira Ayuso wrote: > On Fri, Dec 09, 2022 at 01:41:24AM +0100, Phil Sutter wrote: > > On Thu, Dec 08, 2022 at 10:40:22PM +0100, Pablo Neira Ayuso wrote: > > > On Thu, Dec 01, 2022 at 05:39:10PM +0100, Phil Sutter wrote: > > > > Sadly, '-C' is in use already for --change-counters (even though > > > > ebtables-nft does not implement this), so add a long-option only. It is > > > > needed for xlate testsuite in replay mode, which will use '--check' > > > > instead of '-C'. > > > > > > Hm, yet another of those exotic deviations (from ip{6}tables) in > > > ebtables. > > > > > > This -C is not supported by ebtables-nft, right? If so, > > > according to manpage, ebtables -C takes start_nr[:end_nr]. > > > > > > Maybe there is a chance to get this aligned with other ip{6}tables > > > tools by checking if optarg is available? Otherwise, really check the > > > ruleset? > > > > > > BTW, I'm re-reading the ebtables manpage, not sure how this feature -C > > > was supposed to be used. Do you understand the usecase? > > > > Yes, it's odd - so fits perfectly the rest of ebtables syntax. ;) > > > > There are two ways to use it: > > > > 1) ebtables -C <CHAIN> <RULENO> <PCNT> <BCNT> > > 2) ebtables -C <CHAIN> <PCNT> <BCNT> <RULESPEC> > > > > So I could check if the two parameters following the chain name are > > numbers or not to distinguish between --change-counters and --check, but > > it's ugly and with ebtables-nft not supporting one of them makes things > > actually worse. > > > > We need --check only for internal purposes, let's please just leave it > > like this - there are much more important things to work on. > > OK, just an idea in case there is a need for getting ebtables more > aligned with other xtables userspace. I'd love to, but the syntax is so far off, it's almost futile. :( Cheers, Phil
On Fri, Dec 09, 2022 at 05:51:55PM +0100, Phil Sutter wrote: > On Fri, Dec 09, 2022 at 04:23:49PM +0100, Pablo Neira Ayuso wrote: > > On Fri, Dec 09, 2022 at 01:41:24AM +0100, Phil Sutter wrote: > > > On Thu, Dec 08, 2022 at 10:40:22PM +0100, Pablo Neira Ayuso wrote: > > > > On Thu, Dec 01, 2022 at 05:39:10PM +0100, Phil Sutter wrote: > > > > > Sadly, '-C' is in use already for --change-counters (even though > > > > > ebtables-nft does not implement this), so add a long-option only. It is > > > > > needed for xlate testsuite in replay mode, which will use '--check' > > > > > instead of '-C'. > > > > > > > > Hm, yet another of those exotic deviations (from ip{6}tables) in > > > > ebtables. > > > > > > > > This -C is not supported by ebtables-nft, right? If so, > > > > according to manpage, ebtables -C takes start_nr[:end_nr]. > > > > > > > > Maybe there is a chance to get this aligned with other ip{6}tables > > > > tools by checking if optarg is available? Otherwise, really check the > > > > ruleset? > > > > > > > > BTW, I'm re-reading the ebtables manpage, not sure how this feature -C > > > > was supposed to be used. Do you understand the usecase? > > > > > > Yes, it's odd - so fits perfectly the rest of ebtables syntax. ;) > > > > > > There are two ways to use it: > > > > > > 1) ebtables -C <CHAIN> <RULENO> <PCNT> <BCNT> > > > 2) ebtables -C <CHAIN> <PCNT> <BCNT> <RULESPEC> > > > > > > So I could check if the two parameters following the chain name are > > > numbers or not to distinguish between --change-counters and --check, but > > > it's ugly and with ebtables-nft not supporting one of them makes things > > > actually worse. > > > > > > We need --check only for internal purposes, let's please just leave it > > > like this - there are much more important things to work on. > > > > OK, just an idea in case there is a need for getting ebtables more > > aligned with other xtables userspace. > > I'd love to, but the syntax is so far off, it's almost futile. :( That's just one way to put it.
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c index c5fc338575f67..7214a767ffe96 100644 --- a/iptables/xtables-eb.c +++ b/iptables/xtables-eb.c @@ -198,6 +198,7 @@ struct option ebt_original_options[] = { "delete-chain" , optional_argument, 0, 'X' }, { "init-table" , no_argument , 0, 11 }, { "concurrent" , no_argument , 0, 13 }, + { "check" , required_argument, 0, 14 }, { 0 } }; @@ -730,6 +731,7 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table, case 'N': /* Make a user defined chain */ case 'E': /* Rename chain */ case 'X': /* Delete chain */ + case 14: /* check a rule */ /* We allow -N chainname -P policy */ if (command == 'N' && c == 'P') { command = c; @@ -907,7 +909,8 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table, if (!OPT_COMMANDS) xtables_error(PARAMETER_PROBLEM, "No command specified"); - if (command != 'A' && command != 'D' && command != 'I' && command != 'C') + if (command != 'A' && command != 'D' && + command != 'I' && command != 'C' && command != 14) xtables_error(PARAMETER_PROBLEM, "Command and option do not match"); if (c == 'i') { @@ -1088,7 +1091,7 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table, argv[optind]); if (command != 'A' && command != 'I' && - command != 'D' && command != 'C') + command != 'D' && command != 'C' && command != 14) xtables_error(PARAMETER_PROBLEM, "Extensions only for -A, -I, -D and -C"); } @@ -1109,7 +1112,7 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table, /* Do the final checks */ if (command == 'A' || command == 'I' || - command == 'D' || command == 'C') { + command == 'D' || command == 'C' || command == 14) { for (xtrm_i = cs.matches; xtrm_i; xtrm_i = xtrm_i->next) xtables_option_mfcall(xtrm_i->match); @@ -1161,6 +1164,9 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table, } else if (command == 'D') { ret = delete_entry(h, chain, *table, &cs, rule_nr - 1, rule_nr_end, flags & OPT_VERBOSE); + } else if (command == 14) { + ret = nft_cmd_rule_check(h, chain, *table, + &cs, 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')
Sadly, '-C' is in use already for --change-counters (even though ebtables-nft does not implement this), so add a long-option only. It is needed for xlate testsuite in replay mode, which will use '--check' instead of '-C'. Signed-off-by: Phil Sutter <phil@nwl.cc> --- iptables/xtables-eb.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)