Re: [PATCH v3 4/8] conntrack: introduce ct_cmd_list

[Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>]

Hi Pablo,

On Mon, 15 Mar 2021 at 18:17, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Fri, Jan 29, 2021 at 10:24:48PM +0100, Mikhail Sennikovsky wrote:
> > As a multicommand support preparation, add support for the
> > ct_cmd_list, which represents a list of ct_cmd elements.
> > Currently only a single entry generated from the command line
> > arguments is created.
> >
> > Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
> > ---
> >  src/conntrack.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 66 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/conntrack.c b/src/conntrack.c
> > index 4783825..1719ca9 100644
> > --- a/src/conntrack.c
> > +++ b/src/conntrack.c
> > @@ -598,6 +598,19 @@ static unsigned int addr_valid_flags[ADDR_VALID_FLAGS_MAX] = {
> >       CT_OPT_REPL_SRC | CT_OPT_REPL_DST,
> >  };
> >
> > +#define CT_COMMANDS_LOAD_FILE_ALLOWED ( 0 \
> > +                                             | CT_CREATE       \
> > +                                             | CT_UPDATE_BIT   \
>
> This should CT_UPDATE.
>
> > +                                             | CT_DELETE       \
> > +                                             | CT_FLUSH        \
> > +                                             | EXP_CREATE      \
> > +                                             | EXP_DELETE      \
> > +                                             | EXP_FLUSH       \
>
> Do you need expectations too? The expectation support for the
> conntrack command line tool is limited IIRC.
Actually I do not need expectations, and I agree they can be removed for now.

> I would probably collapse patch 4/8 and 5/8, it should be easy to
> review, it all basically new code to support for the batching mode.
I could squash the 3/ 4/ and 5/8 for sure.
Again the goal was to make the changes more granular and easier for
review, since all these parts are independent.
So the 3/, 4/ and 5/8 are kind of "preparation" commits for the "real"
--load-file functionality.
If you say it's better to squash them, I can surely do it.

Regards,
Mikhail