On 4 Jan 2023, at 18:57, Ilya Maximets wrote:
> On 1/4/23 16:28, Eelco Chaudron wrote:
>> The dpif_execute_helper_cb() function is supposed to add the
>> OVS_ACTION_ATTR_SET(OVS_KEY_ATTR_TUNNEL()) action to the
>> list of actions when passing it down to the kernel.
>>
>> This function was only checking if the IPv4 destination
>> address was set, not both. This patch fixes this, including
>> a datapath testcase.
>>
>> Fixes: 076caa2fb077 ("ofproto: Meter translation.")
>> Signed-off-by: Eelco Chaudron <[email protected]>
>> ---
>>
>> v2: Changed capture file name to txt as it's was not in pcap format.
>> v3: Using ovs-pcap rather than relying on the tcpdump output.
>>
>> lib/dpif.c | 2 +-
>> tests/system-traffic.at | 46
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/dpif.c b/lib/dpif.c
>> index 40f5fe446..fe4db83fb 100644
>> --- a/lib/dpif.c
>> +++ b/lib/dpif.c
>> @@ -1213,7 +1213,7 @@ dpif_execute_helper_cb(void *aux_, struct
>> dp_packet_batch *packets_,
>> /* The Linux kernel datapath throws away the tunnel information
>> * that we supply as metadata. We have to use a "set" action to
>> * supply it. */
>> - if (md->tunnel.ip_dst) {
>> + if (flow_tnl_dst_is_set(&md->tunnel)) {
>> odp_put_tunnel_action(&md->tunnel, &execute_actions, NULL);
>> }
>> ofpbuf_put(&execute_actions, action,
>> NLA_ALIGN(action->nla_len));
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index e5403519f..a43c7fd9a 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -855,6 +855,52 @@ 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 - slow_action on geneve6 tunnel])
>> +AT_SKIP_IF([test $HAVE_TCPDUMP = no])
>> +OVS_CHECK_TUNNEL_TSO()
>> +OVS_CHECK_GENEVE_UDP6ZEROCSUM()
>> +
>> +OVS_TRAFFIC_VSWITCHD_START()
>> +ADD_BR([br-underlay])
>> +
>> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>> +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, "fc00::1/64", [], [], "nodad")
>> +AT_CHECK([ip addr add dev br-underlay "fc00::100/64" nodad])
>> +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_TUNNEL6([geneve], [br0], [at_gnv0], [fc00::1], [10.1.1.100/24])
>> +ADD_NATIVE_TUNNEL6([geneve], [ns_gnv0], [at_ns0], [fc00::100],
>> [10.1.1.1/24],
>> + [vni 0 udp6zerocsumtx udp6zerocsumrx])
>> +AT_CHECK([ovs-ofctl add-flow br0 "table=37,actions=at_gnv0"])
>> +
>> +OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::100])
>> +
>> +dnl First, check the underlay.
>> +NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00::100 |
>> FORMAT_PING], [0], [dnl
>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> +])
>> +
>> +dnl Start tcpdump to capture the encapsulated packets.
>> +NETNS_DAEMONIZE([at_ns0], [tcpdump -U -i p0 -w p0.pcap], [tcpdump.pid])
>> +sleep 1
>> +
>> +dnl Generate a single packet trough the controler that needs an ARP
>> modification
>> +AT_CHECK([ovs-ofctl -O OpenFlow15 packet-out br0 "in_port=controller
>> packet=fffffffffffffa163e949d8008060001080006040001fa163e949d80c0a820300000000000000a0000fe
>>
>> actions=set_field:0xa0000f4->reg1,move:NXM_NX_XXREG0[[64..95]]->NXM_OF_ARP_SPA[[]],resubmit(,37)"])
>> +sleep 1
>> +
>> +dnl Stop OVS and tcpdump and verify the results.
>> +OVS_TRAFFIC_VSWITCHD_STOP
>> +
>> +AT_CHECK([ovs-pcap p0.pcap | grep -Eq
>> "^[[[:xdigit:]]]{24}86dd60000000003a1140fc000000000000000000000000000100fc000000000000000000000000000001[[[:xdigit:]]]{4}17c1003a00000000655800000000fffffffffffffa163e949d8008060001080006040001[[[:xdigit:]]]{12}0a0000f40000000000000a0000fe$"])
>
> Unfortunately, this breaks the userspace testsuite because
> of the UDP checksum being calculated in the pcap, but all-zero
> in the grep match. Do we need the udp6zerocsum* stuff?
Yes, I noticed the email from intel yesterday :( I have a new rev masking out
the checksum, so it should work everywhere.
Will run it trough all data paths and sent out a v4…
//Eelco
>> +AT_CLEANUP
>> +
>> AT_SETUP([datapath - ping over gre tunnel by simulated packets])
>> OVS_CHECK_TUNNEL_TSO()
>> OVS_CHECK_MIN_KERNEL(3, 10)
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
