Re: [PATCH v3 3/8] conntrack: per-command entries counters

From: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [PATCH v3 3/8] conntrack: per-command entries counters
Date: Wed, 17 Mar 2021 19:20:55 +0100	[thread overview]
Message-ID: <CALHVEJb6dH_RdxvbtLaptN=8-g4QUUtd=+R-p2PrfNBep0XkWA@mail.gmail.com> (raw)
In-Reply-To: <20210315171209.GA24883@salvia>

Hi Pablo,

On Mon, 15 Mar 2021 at 18:12, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> Hi Mikhail,
>
> On Fri, Jan 29, 2021 at 10:24:47PM +0100, Mikhail Sennikovsky wrote:
> > As a multicommand support preparation entry counters need
> > to be made per-command as well, e.g. for the case -D and -I
> > can be executed in a single batch, and we want to have separate
> > counters for them.
>
> How do you plan to use the counters? -F provides no stats though.
Those counters are used to print the number of affected entries for
each command "type" executed.
I.e. prior to the "--load-file" support it was only possible to have a
single command for each conntrack tool invocation,
so a global counter used to print the stats message like
"conntrack v1.4.6 (conntrack-tools): 1 flow entries have been created."
was sufficient.

With the --load-file/-R command support it is possible to have
multiple command types
in a single conntrack tool invocation, e.g. both -I and -D commands as
in example below.

echo "-D -w 123
-I -w 123 -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state
LISTEN -u SEEN_REPLY -t 50 " | conntrack -R -

The per-command counters functionality added here makes it possible to print
those stats info for each command "type" separately.
So as a result of the above command something the following would be printed:

conntrack v1.4.6 (conntrack-tools): 1 flow entries have been created.
conntrack v1.4.6 (conntrack-tools): 3 flow entries have been deleted.

Following your request to make the changes more granular, I moved this
functionality
to this separate "preparation" commit.

>
> It should be possible to do some pretty print for stats.
>
> There is also the -I and -D cases, which might fail. In that case,
> this should probably stop processing on failure?
Are you talking about error handling processing ct_cmd entries?
The way it is done currently is that each failure would result in
exit_error to happen.
This logic actually stays unchanged.

>
> I sent another round of patches based on your that gets things closer
> to the batch support.
Thanks, I'll have a look into them.

Regards,
Mikhail