On Sun, Aug 11, 2019 at 12:30 PM Darrell Ball <[email protected]> wrote:
>
> I did some further testing and ran into another issue; in this case, one, I 
> did not expect.
>
> I added an additional sending of packets at the end of the test after this 
> check:
>
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
> ])
>
> Below is new code
>
> dnl Do it again
> dnl Send ICMP and UDP traffic
> NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], 
> [0], [dnl
> 3 packets transmitted, 3 received, 0% packet loss, time 0ms
> ])
> AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 
> packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000
>  actions=resubmit(,0)"])
>
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sort], [0], 
> [dnl
> icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=<cleared>,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=<cleared>,type=0,code=0),zone=5
> udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),zone=5
> ])
>
> dnl Wait until the timeout expire.
> dnl We intend to wait a bit longer, because conntrack does not recycle the 
> entry right after it is expired.
> sleep 5
>
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
> ])
>
> The test fails bcoz the second time with short timeouts, the conntrack 
> entries are not cleanup up quickly
>
> @@ -0,0 +1,2 @@
> +icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=<cleared>,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=<cleared>,type=0,code=0),zone=5
> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),zone=5


Thanks for testing!   This test actually catch a kernel bug when ovs
kernel handles conntrack cache.  It works for me on my ubuntu xenial
VM with 4.4 kernel.

Since this requires upstream kernel change, it will be backported to
OVS once the fix gets upstream.

Thanks,

-Yi-Hung

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index f85d0a2572f6..ad48b559bcde 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -76,6 +76,7 @@ enum ovs_ct_nat {
 /* Conntrack action context for execution. */
 struct ovs_conntrack_info {
        struct nf_conntrack_helper *helper;
+       struct nf_ct_timeout *nf_ct_timeout;
        struct nf_conntrack_zone zone;
        struct nf_conn *ct;
        u8 commit : 1;
@@ -745,6 +746,13 @@ static bool skb_nfct_cached(struct net *net,
                if (help && rcu_access_pointer(help->helper) != info->helper)
                        return false;
        }
+       if (info->nf_ct_timeout) {
+               struct nf_conn_timeout *timeout_ext;
+
+               timeout_ext = nf_ct_timeout_find(ct);
+               if (!timeout_ext || info->nf_ct_timeout != timeout_ext->timeout)
+                       return false;
+       }
        /* Force conntrack entry direction to the current packet? */
        if (info->force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) {
                /* Delete the conntrack entry if confirmed, else just release
@@ -1704,6 +1712,8 @@ int ovs_ct_copy_action(struct net *net, const
struct nlattr *attr,
                                      ct_info.timeout))
                        pr_info_ratelimited("Failed to associated timeout "
                                            "policy `%s'\n", ct_info.timeout);
+               else
+                       ct_info.nf_ct_timeout =
nf_ct_timeout_find(ct_info.ct)->timeout;
        }

        if (helper) {
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to