[ovs-dev] [PATCHv4 06/11] Add support for connection tracking.

Joe Stringer joestringer at nicira.com
Wed Oct 7 22:38:16 UTC 2015


On 7 October 2015 at 11:11, Daniele Di Proietto <diproiettod at vmware.com> wrote:
> Hi Joe,
>
> I have a couple of minor comments inline
>
> On 02/10/2015 22:16, "Joe Stringer" <joestringer at nicira.com> wrote:
>
>>This patch adds a new action and fields to OVS that allow connection
>>tracking to be performed. This support works in conjunction with the
>>Linux kernel support merged into the Linux-4.3 development cycle.
> [...]
>>diff --git a/lib/odp-util.c b/lib/odp-util.c
>>index c173623..a570afa 100644
>>--- a/lib/odp-util.c
>>+++ b/lib/odp-util.c
> [...]
>>@@ -2605,6 +2790,25 @@ scan_tcp_flags(const char *s, ovs_be16 *key,
>>ovs_be16 *mask)
>> }
>>
>> static int
>>+scan_ct_state(const char *s, uint32_t *key, uint32_t *mask)
>>+{
>>+    uint32_t flags, fmask;
>>+    int n;
>>+
>>+    n = parse_flags(s, ct_state_to_string, ')', NULL, NULL, &flags,
>>+                    CS_SUPPORTED_MASK, mask ? &fmask : NULL);
>>+
>
> Should we scan these flags as OVS_CS_F* (the odp format)?
> Here they're treated as CS_* (the OpenFlow format).
>
> In this case it could be something like
>
> n = parse_flags(s, odp_ct_state_to_string, ')', NULL, NULL, &flags,
>                 ovs_to_odp_ct_state(CS_SUPPORTED_MASK, false),
>                 mask ? &fmask : NULL);
>
>
>
>>+    if (n >= 0) {
>>+        *key = flags;
>>+        if (mask) {
>>+            *mask = fmask;
>>+        }
>>+        return n;
>>+    }
>>+    return 0;
>>+}
>>+
>>+static int
>> scan_frag(const char *s, uint8_t *key, uint8_t *mask)
>> {
>>     int n;
> [...]
>>
>>
>>diff --git a/tests/odp.at b/tests/odp.at
>>index 61edde3..16d7411 100644
>>--- a/tests/odp.at
>>+++ b/tests/odp.at
>>@@ -86,6 +86,11 @@ sed '/bos=0/{
>> s/^/ODP_FIT_TOO_LITTLE: /
>> }' < odp-in.txt > odp-out.txt
>>
>>+dnl Some fields are always printed for this test, because wildcards
>>aren't
>>+dnl specified. We can skip these.
>>+sed -i 's/\(skb_mark(0)\),\(ct\)/\1,ct_state(0),ct_zone(0),\2/'
>>odp-out.txt
>>+sed -i
>>'s/\(skb_mark([[^)]]*)\),\(recirc\)/\1,ct_state(0),ct_zone(0),\2/'
>>odp-out.txt
>>+
>> AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [`cat
>>odp-out.txt`
>> ])
>> AT_CLEANUP
>>@@ -153,6 +158,10 @@
>>s/\(eth([[^)]]*)\),*/\1,eth_type(0x8100),vlan(vid=99,pcp=7),encap(/
>> s/$/)/' odp-base.txt
>>
>>  echo
>>+ echo '# Valid forms with conntrack fields.'
>>+ sed 's/\(eth([[^)]]*)\),/\1,ct_state(+trk),ct_zone(5/0xff),/'
>>odp-base.txt
>
> I think we should escape the forward slash and use a hex prefix for the
> zone
>
> ...ct_zone(0x5\/0xff)...
>
>
> With this fix, the testcase should detect the above issue.

Thanks again, those fixes look right to me. I'll roll them in.



More information about the dev mailing list