[ovs-dev] [PATCH 7/8] Add support for connection tracking helper/ALGs.

[Joe Stringer]

On 11 September 2015 at 16:41, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
> With the comments below,
>
> Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
>
>
> Sorry for repeating some comments over and over, no offense intended.

It's actually kinda useful as a checklist :-)

>> On Sep 9, 2015, at 7:00 PM, Joe Stringer <joestringer at nicira.com> wrote:
>>
>> This patch adds support for specifying a "helper" or ALG to assist
>> connection tracking for protocols that consist of multiple streams.
>> Initially, only support for FTP is included.
>>
>> Below is an example set of flows to allow FTP control connections from
>> port 1->2 to establish active data connections in the reverse direction:
>>
>>    priority=1,action=drop
>>    priority=10,arp,action=normal
>>    in_port=1,tcp,action=ct(alg=ftp,commit),2
>>    in_port=2,tcp,ct_state=-trk,action=ct(table=1)
>>    table=1,in_port=2,tcp,ct_state=+trk-new+est,action=1
>
> +est would be sufficient?

Dropped -new.

>>    table=1,in_port=2,tcp,ct_state=+trk+rel,action=ct(commit),1
>
> +rel would be sufficient?

True. I don't intend on going through all of the examples to change this though.

> Also, will this try to commit all the packets, or only the first one?

All. It could be more strict, but for demonstration purposes it's
sufficient to show the previously undemonstrated flag.

>> @@ -1112,6 +1125,15 @@ parse_conntrack_action(const char *s_, struct ofpbuf *actions)
>>                     s = tail;
>>                     continue;
>>                 }
>> +                if (ovs_scan(s, "helper=%n", &n)) {
>> +                    s += n;
>> +                    helper_len = strcspn(s, delimiters_end);
>> +                    if (helper_len > 15) {
>> +                        return -EINVAL;
>> +                    }
>> +                    helper = s;
>> +                    s += helper_len;
>> +                }
>
> Will this accept zero-length helper string/name?

It does, but the kernel rejects it. I can update to reject this too.

<snip>

I updated the priorities and comments on the tests.

>> +dnl Active FTP requests from p0->p1 should work fine.
>> +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v -o wget0.log])
>> +AT_CHECK([conntrack -L 2>&1 | FORMAT_CT(10.1.1.2) | grep -v "FIN"], [0], [dnl
>> +TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> [[ASSURED]] mark=0 zone=1 helper=ftp use=2
>> +TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> [[ASSURED]] mark=0 zone=2 helper=ftp use=2
>> +TIME_WAIT src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> [[ASSURED]] mark=0 zone=1 use=1
>> +TIME_WAIT src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> [[ASSURED]] mark=0 zone=2 use=1
>> +])
>> +
>> +AT_CHECK([conntrack -F 2>/dev/null])
>> +
>> +dnl Passive FTP requests from p0->p1 should work fine.
>> +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o wget0.log])
>> +AT_CHECK([conntrack -L 2>&1 | FORMAT_CT(10.1.1.2) | grep -v "FIN"], [0], [dnl
>> +TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> [[ASSURED]] mark=0 zone=1 helper=ftp use=2
>> +TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> [[ASSURED]] mark=0 zone=1 use=1
>> +TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> [[ASSURED]] mark=0 zone=2 helper=ftp use=2
>> +TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> [[ASSURED]] mark=0 zone=2 use=1
>> +])
>> +
>> +OVS_TRAFFIC_VSWITCHD_STOP
>> +AT_CLEANUP
>
> I had to install pyftpdlib to run these tests, would be nice to have that documented somewhere.

All of the conntrack tests also require conntrack-tools, and some of
the tunnel tests require new enough "ip" and new enough kernel to run.
I could put this information with a new section around using the
"check-kernel" target.

Thanks,
Joe