Hi Joe,
I have a couple of minor comments inline
On 02/10/2015 22:16, "Joe Stringer" <joestrin...@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!
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
