[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5ded2e5b-958e-eca3-76ad-909ebf79234e@mellanox.com>
Date: Thu, 11 Jul 2019 07:21:51 +0000
From: Paul Blakey <paulb@...lanox.com>
To: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
CC: Jiri Pirko <jiri@...lanox.com>, Roi Dayan <roid@...lanox.com>,
Yossi Kuperman <yossiku@...lanox.com>,
Oz Shlomo <ozsh@...lanox.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
David Miller <davem@...emloft.net>,
Aaron Conole <aconole@...hat.com>,
Zhike Wang <wangzhike@...com>, Justin Pettit <jpettit@....org>,
John Hurley <john.hurley@...ronome.com>,
Rony Efraim <ronye@...lanox.com>,
"nst-kernel@...hat.com" <nst-kernel@...hat.com>,
Simon Horman <simon.horman@...ronome.com>
Subject: Re: [PATCH net-next iproute2 2/3] tc: Introduce tc ct action
On 7/9/2019 6:36 PM, Marcelo Ricardo Leitner wrote:
> 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@...lanox.com>
>>>> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
>>>> Signed-off-by: Yossi Kuperman <yossiku@...lanox.com>
>>>> Acked-by: Jiri Pirko <jiri@...lanox.com>
>>>> Acked-by: Roi Dayan <roid@...lanox.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?
It is optional in the overall syntax.
But I split it into modes:
clear, commit, and "restore" (I unofficial call it like that, because it
usually used to get the +est state on the packet and can restore nat, it
doesn't actually restore anything for the first packet on the -trk rule)
It is mandatory in the second mode (commit), if you don't specify commit
or clear, you can only use the third form - "restore", which is to send
to ct on some optional zone, and optionally and restore nat (so we get
ct [zone] [nat]).
I think this syntax is easy, maybe I can label them as the modes of
operation above (then I'll need to name the restore one better :)).
If there is a different syntax you think might be easier I'll change to
that.
Thanks,
Paul.
Powered by blists - more mailing lists