[ovs-dev] [PATCH v2] netlink: removed incorrect optimization

Gregory Rose gvrose8192 at gmail.com
Fri Jun 4 20:13:00 UTC 2021



On 6/2/2021 3:00 PM, Toms Atteka wrote:
> This optimization caused FLOW_TNL_F_UDPIF flag not to be used in
> hash calculation for geneve tunnel when revalidating flows which
> resulted in different cache hash values and incorrect behaviour.
> 
> Added test to prevent regression.
> 
> Reported-at: https://github.com/vmware-tanzu/antrea/issues/897
> Signed-off-by: Toms Atteka <cpp.code.lv at gmail.com>

Hi Toms,

I think the patch is fine but it should have a 'Fixes' tag.

Thanks,

- Greg

> ---
>   lib/tun-metadata.c      |  2 +-
>   tests/system-traffic.at | 54 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
> index c0b0ae044..828db72f5 100644
> --- a/lib/tun-metadata.c
> +++ b/lib/tun-metadata.c
> @@ -828,7 +828,7 @@ tun_metadata_to_geneve_nlattr(const struct flow_tnl *tun,
>           } else {
>               tun_metadata_to_geneve_nlattr_mask(key, tun, flow, b);
>           }
> -    } else if (flow->metadata.present.len || is_mask) {
> +    } else {
>           nl_msg_put_unspec(b, OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS,
>                             tun->metadata.opts.gnv,
>                             flow->metadata.present.len);
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index fb5b9a36d..483cc7e83 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -574,6 +574,60 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PI
>   OVS_TRAFFIC_VSWITCHD_STOP
>   AT_CLEANUP
>   
> +AT_SETUP([datapath - ping over geneve tunnel, delete flow regression])
> +OVS_CHECK_GENEVE()
> +
> +OVS_TRAFFIC_VSWITCHD_START()
> +ADD_BR([br-underlay])
> +
> +AT_DATA([flows.txt], [dnl
> +priority=100,icmp actions=resubmit(,10)
> +priority=0 actions=NORMAL
> +table=10, priority=100, ip, actions=ct(table=20,zone=65520)
> +table=20, priority=200, ip, ct_state=-new+trk, actions=resubmit(,30)
> +table=20, priority=100, ip, ct_state=+new, actions=resubmit(,30)
> +table=20, priority=50, ip, actions=DROP
> +table=30, priority=100, ip, actions=ct(commit,table=40,zone=65520)
> +table=40, actions=normal
> +])
> +
> +AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
> +
> +ADD_NAMESPACES(at_ns0)
> +
> +dnl Set up underlay link from host into the namespace using veth pair.
> +ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
> +AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
> +AT_CHECK([ip link set dev br-underlay up])
> +
> +dnl Set up tunnel endpoints on OVS outside the namespace and with a native
> +dnl linux device inside the namespace.
> +ADD_OVS_TUNNEL([geneve], [br0], [at_gnv0], [172.31.1.1], [10.1.1.100/24])
> +ADD_NATIVE_TUNNEL([geneve], [ns_gnv0], [at_ns0], [172.31.1.100], [10.1.1.1/24],
> +                  [vni 0])
> +
> +dnl First, check the underlay
> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], [0], [dnl
> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> +])
> +
> +dnl ping over tunnel should work
> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PING], [0], [dnl
> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> +])
> +
> +AT_CHECK([ovs-ofctl del-flows br0 "ct_state=+new"])
> +
> +dnl ping should not go through after removal of the flow
> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PING], [0], [dnl
> +7 packets transmitted, 0 received, 100% packet loss, time 0ms
> +])
> +
> +OVS_TRAFFIC_VSWITCHD_STOP(["/|ERR|/d
> +/|WARN|/d"])
> +AT_CLEANUP
> +
>   AT_SETUP([datapath - flow resume with geneve tun_metadata])
>   OVS_CHECK_GENEVE()
>   
> 


More information about the dev mailing list