[ovs-dev] [PATCH] datapath: Fix conntrack cache with timeout

Justin Pettit jpettit at ovn.org
Mon Sep 30 20:19:04 UTC 2019


Thanks for the patch, Yi-Hung, and the review, Darrell.  I pushed this to master.

--Justin


> On Sep 29, 2019, at 10:09 AM, Darrell Ball <dlu998 at gmail.com> wrote:
> 
> Thanks for the patch
> 
> Looks good and matches the upstream version, including the rcu deference
> fixup.
> Thanks for remembering to add the requested test incremental, post fix.
> 
> Darrell
> 
> On Fri, Sep 27, 2019 at 2:14 PM Yi-Hung Wei <yihung.wei at gmail.com> wrote:
> 
>> This patch is from the following upstream net-next commit along with
>> an updated system traffic test to avoid regression.
>> 
>> Upstream commit:
>>    commit 7177895154e6a35179d332f4a584d396c50d0612
>>    Author: Yi-Hung Wei <yihung.wei at gmail.com>
>>    Date:   Thu Aug 22 13:17:50 2019 -0700
>> 
>>        openvswitch: Fix conntrack cache with timeout
>> 
>>        This patch addresses a conntrack cache issue with timeout policy.
>>        Currently, we do not check if the timeout extension is set
>> properly in the
>>        cached conntrack entry.  Thus, after packet recirculate from
>> conntrack
>>        action, the timeout policy is not applied properly.  This patch
>> fixes the
>>        aforementioned issue.
>> 
>>        Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct
>> action")
>>        Reported-by: kbuild test robot <lkp at intel.com>
>>        Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
>>        Acked-by: Pravin B Shelar <pshelar at ovn.org>
>>        Signed-off-by: David S. Miller <davem at davemloft.net>
>> 
>> Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
>> ---
>> datapath/conntrack.c    | 13 +++++++++++++
>> tests/system-traffic.at | 18 ++++++++++++++++++
>> 2 files changed, 31 insertions(+)
>> 
>> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
>> index 35a183aeb33a..c6d523758ff1 100644
>> --- a/datapath/conntrack.c
>> +++ b/datapath/conntrack.c
>> @@ -88,6 +88,7 @@ struct ovs_conntrack_info {
>>        struct md_mark mark;
>>        struct md_labels labels;
>>        char timeout[CTNL_TIMEOUT_NAME_MAX];
>> +       struct nf_ct_timeout *nf_ct_timeout;
>> #ifdef CONFIG_NF_NAT_NEEDED
>>        struct nf_nat_range2 range;  /* Only present for SRC NAT and DST
>> NAT. */
>> #endif
>> @@ -750,6 +751,14 @@ 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 !=
>> +                   rcu_dereference(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
>> @@ -1709,6 +1718,10 @@ 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 = rcu_dereference(
>> +                               nf_ct_timeout_find(ct_info.ct)->timeout);
>> +
>>        }
>> 
>>        if (helper) {
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index bfc6bb5b47c7..3d4e365764b5 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -3242,6 +3242,24 @@ sleep 4
>> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0],
>> [dnl
>> ])
>> 
>> +dnl Re-send ICMP and UDP traffic to test conntrack cache
>> +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 4
>> +
>> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0],
>> [dnl
>> +])
>> +
>> OVS_TRAFFIC_VSWITCHD_STOP
>> AT_CLEANUP
>> 
>> --
>> 2.7.4
>> 
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev



More information about the dev mailing list