Re: [PATCH net-next iproute2 2/3] tc: Introduce tc ct action

[Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>]

On Tue, Jul 09, 2019 at 06:58:36AM +0000, Paul Blakey wrote:
> 
> On 7/8/2019 8:54 PM, Marcelo Ricardo Leitner wrote:
> > On Sun, Jul 07, 2019 at 11:53:47AM +0300, Paul Blakey wrote:
> >> New tc action to send packets to conntrack module, commit
> >> them, and set a zone, labels, mark, and nat on the connection.
> >>
> >> It can also clear the packet's conntrack state by using clear.
> >>
> >> Usage:
> >>     ct clear
> >>     ct commit [force] [zone] [mark] [label] [nat]
> > Isn't the 'commit' also optional? More like
> >      ct [commit [force]] [zone] [mark] [label] [nat]
> >
> >>     ct [nat] [zone]
> >>
> >> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> >> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> >> Signed-off-by: Yossi Kuperman <yossiku@mellanox.com>
> >> Acked-by: Jiri Pirko <jiri@mellanox.com>
> >> Acked-by: Roi Dayan <roid@mellanox.com>
> >> ---
> > ...
> >> +static void
> >> +usage(void)
> >> +{
> >> +	fprintf(stderr,
> >> +		"Usage: ct clear\n"
> >> +		"	ct commit [force] [zone ZONE] [mark MASKED_MARK] [label MASKED_LABEL] [nat NAT_SPEC]\n"
> > Ditto here then.
> 
> 
> In commit msg and here, it means there is multiple modes of operation. I 
> think it's easier to split those.

Yep, that is good.
More below.

> 
> "ct clear" to clear it , not other options can be added here.
> 
> "ct commit  [force].... " sends to conntrack and commit a connection, 
> and only for commit can you specify force mark  label, and nat with 
> nat_spec....
> 
> and the last one, "ct [nat] [zone ZONE]" is to just send the packet to 
> conntrack on some zone [optional], restore nat [optional].
> 
> 
> >
> >> +		"	ct [nat] [zone ZONE]\n"
> >> +		"Where: ZONE is the conntrack zone table number\n"
> >> +		"	NAT_SPEC is {src|dst} addr addr1[-addr2] [port port1[-port2]]\n"
> >> +		"\n");
> >> +	exit(-1);
> >> +}
> > ...
> >
> > The validation below doesn't enforce that commit must be there for
> > such case.
> which case? commit is optional. the above are the three valid patterns.

That's the point. But the 2nd example is saying 'commit' word is
mandatory in that mode. It is written as it is a command that was
selected.

One may use just:
    ct [zone]
And not
    ct commit [zone]
Right?