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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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.