On Mon, Jan 14, 2019 at 7:32 AM Darrell Ball <[email protected]> wrote:

> Thanks for the patch
>
> On Wed, Jan 9, 2019 at 7:33 AM David Marchand <[email protected]>
> wrote:
>
> The ftp alg deals with packets in two ways for the command connection:
>> either they are inspected and can be mangled when nat is enabled
>> (CT_FTP_CTL_INTEREST) or they just go through without being modified
>> (CT_FTP_CTL_OTHER).
>>
>> For CT_FTP_CTL_INTEREST packets, we must both adjust the packet tcp seq
>> number by the connection current offset, then prepare for the next
>> packets by setting an accumulated offset in the ct object.
>
>
>> The tests are updated so that ftp+NAT checks send multiple commands in a
>> single tcp command connection:
>>
>>
> 1/ Please update the commit message for the 2nd and 3rd paragraphs,
>    including use case and a correction.
>
> The ftp alg deals with packets in two ways for the command connection:
> either they are inspected and can be mangled when nat is enabled
> (CT_FTP_CTL_INTEREST) or they just go through without being modified
> (CT_FTP_CTL_OTHER).
>
> For CT_FTP_CTL_INTEREST packets, we must both adjust the packet tcp seq
> number by the connection current offset, then prepare for the next
> packets by setting an accumulated offset in the ct object.  However, this
> was not
> done for multiple CT_FTP_CTL_INTEREST packets for the same connection.
> This is relevant for handling multiple child data connections that also
> need natting.
>
> The tests are updated so that some ftp+NAT tests send multiple port
> commands or
> other similar commands for a single control connection. Wget is not able
> to do this,
> so switch to lftp.
>

> 2/ A couple conntrack.c comments.
>

> 3/ I requested changing the number of 'ls' commands below, since it is a
> stronger test.
>

Sure.



> 4/ I added another test below for reverse skew.
>

Well, adding a DNAT test at this point cannot work.
I can move it to the second patch.



> 5/ Please consider adding me as co-author
>

Ok.


I will wait for your reply on the 32bits wrapping fix before sending a v4.
Thanks.

-- 
David Marchand
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to