Re: iptables-save - suggest patch to add functionality

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 12, 2018 at 11:58:01PM +0100, Alban Vidal wrote:
[...]
> diff --git a/iptables/ip6tables-save.c b/iptables/ip6tables-save.c
> index 8e3a6afd..a94beffc 100644

Please, send us patches in git-format-patch, include a patch
description and add your Signed-off-by tag.

More comments below.

> --- a/iptables/ip6tables-save.c
> +++ b/iptables/ip6tables-save.c
> @@ -19,11 +19,15 @@
>  #include "ip6tables.h"
>  #include "ip6tables-multi.h"
>  
> -static int show_counters;
> +static int show_counters = false;

No need to initialize code that is implicitly initialized because of
being in the bss.

> +
> +/* if true (opt -Z, --zero): Reset to zero counters of the chains */

No need for comment.

> +static int rst_chain_counters = false;

I would call this:

        display_zero_counters

This is not resetting counters, it just displays them as zero. Same
comment applies to documentation.

>  static const struct option options[] = {
>  	{.name = "counters", .has_arg = false, .val = 'c'},
>  	{.name = "dump",     .has_arg = false, .val = 'd'},
> +	{.name = "zero",     .has_arg = false, .val = 'Z'},
>  	{.name = "table",    .has_arg = true,  .val = 't'},
>  	{.name = "modprobe", .has_arg = true,  .val = 'M'},
>  	{.name = "file",     .has_arg = true,  .val = 'f'},
> @@ -96,7 +100,13 @@ static int do_output(const char *tablename)
>  			struct xt_counters count;
>  			printf("%s ",
>  			       ip6tc_get_policy(chain, &count, h));
> -			printf("[%llu:%llu]\n", (unsigned long long)count.pcnt, (unsigned long long)count.bcnt);
> +			if (!rst_chain_counters) {
> +				/* Default value, print count */

No need for comment above.

> +				printf("[%llu:%llu]\n", (unsigned long long)count.pcnt, (unsigned long long)count.bcnt);
> +			} else {
> +				/* Reset to zero counters of the chains */
> +				printf("[0:0]\n");
> +			}
>  		} else {
>  			printf("- [0:0]\n");
>  		}
> @@ -146,15 +156,17 @@ int ip6tables_save_main(int argc, char *argv[])
>  	init_extensions6();
>  #endif
>  
> -	while ((c = getopt_long(argc, argv, "bcdt:M:f:", options, NULL)) != -1) {
> +	while ((c = getopt_long(argc, argv, "bcZdt:M:f:", options, NULL)) != -1) {
>  		switch (c) {
>  		case 'b':
>  			fprintf(stderr, "-b/--binary option is not implemented\n");
>  			break;
>  		case 'c':
> -			show_counters = 1;
> +			show_counters = true;

Do this update in a separated patch: One patch per logical change.

> +			break;
> +		case 'Z':
> +			rst_chain_counters = true;
>  			break;
> -
>  		case 't':
>  			/* Select specific table. */
>  			tablename = optarg;
> diff --git a/iptables/iptables-save.8.in b/iptables/iptables-save.8.in
> index 51e11f3e..200d6448 100644
> --- a/iptables/iptables-save.8.in
> +++ b/iptables/iptables-save.8.in
> @@ -24,10 +24,10 @@ iptables-save \(em dump iptables rules
>  ip6tables-save \(em dump iptables rules
>  .SH SYNOPSIS
>  \fBiptables\-save\fP [\fB\-M\fP \fImodprobe\fP] [\fB\-c\fP]
> -[\fB\-t\fP \fItable\fP] [\fB\-f\fP \fIfilename\fP]
> +[\fB\-Z\fP] [\fB\-t\fP \fItable\fP] [\fB\-f\fP \fIfilename\fP]
>  .P
>  \fBip6tables\-save\fP [\fB\-M\fP \fImodprobe\fP] [\fB\-c\fP]
> -[\fB\-t\fP \fItable\fP] [\fB\-f\fP \fIfilename\fP]
> +[\fB\-Z\fP] [\fB\-t\fP \fItable\fP] [\fB\-f\fP \fIfilename\fP]
>  .SH DESCRIPTION
>  .PP
>  .B iptables-save
> @@ -45,19 +45,24 @@ Specify a filename to log the output to. If not specified, iptables-save
>  will log to STDOUT.
>  .TP
>  \fB\-c\fR, \fB\-\-counters\fR
> -include the current values of all packet and byte counters in the output
> +Include the current values of all packet and byte counters in the output.
   ^

Same thing as above, no unrelated changes in this patch.

> +.TP
> +\fB\-Z\fR, \fB\-\-zero\fR
> +Reset to zero counters of the chains.

This is not resetting anything, instead I'd propose:

Display zero packet and byte chain counters when saving the ruleset.

>  .TP
>  \fB\-t\fR, \fB\-\-table\fR \fItablename\fP
> -restrict output to only one table. If not specified, output includes all
> +Restrict output to only one table. If not specified, output includes all
>  available tables.
>  .SH BUGS
>  None known as of iptables-1.2.1 release
>  .SH AUTHORS
> -Harald Welte <laforge@xxxxxxxxxxxx>
> +Harald Welte <laforge@xxxxxxxxxxxx>,
> +.br
> +Rusty Russell <rusty@xxxxxxxxxxxxxxx>,
>  .br
> -Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> +Andras Kis-Szabo <kisza@xxxxxxxxxx> contributed ip6tables-save,
>  .br
> -Andras Kis-Szabo <kisza@xxxxxxxxxx> contributed ip6tables-save.
> +Alban Vidal <alban.vidal@xxxxxxxxxx> contributed ip[6]tables-save.

Again, this information is there for historical reasons: git is
already leaving a record on this. *A lot* of people have contributed
to iptables and they are not listed there :-).

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux