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

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Mikhail,

On Wed, Mar 17, 2021 at 07:20:55PM +0100, Mikhail Sennikovsky wrote:
> 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.

I think it should be possible to do some pretty print, something like:

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

One possibility is that we skip the pretty print by now, ie. you
rebase your patch on top of conntrack-tools, get it merged upstream.
Then incrementally we look at adding the 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.

So the batch processing stops on the first error, right?

> > 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.

I have pushed them out, any mistake please let me know I'll fix it.

Thanks.