On 19 July 2017 at 06:46, Sugesh Chandran <[email protected]> wrote:
> Openvswitch datapath recirculates packets for tunneling, i.e.
> the incoming packets are encapsulated at first pass. Further actions are
> applied on encapsulated packets on the second pass after recirculating.
> The proposed patch compute and append the post tunnel actions at the time of
> translation itself instead of recirculating at datapath. These actions are
> solely depends on tunnel attributes so there is no need of datapath
> recirculation.
>
> By avoiding the recirculation at datapath, the patch offers upto 30%
> performance improvement for VxLAN tunneling in our testing.
> The action execution logic is also extended with new CLONE action to define
> the packet cloning when the actions are combined. The lenght in the CLONE
> action specifies the size of nested action set.
>
> Signed-off-by: Sugesh Chandran <[email protected]>
> Signed-off-by: Zoltán Balogh <[email protected]>
> Co-authored-by: Zoltán Balogh <[email protected]>
>
> v4
> - Rebased on latest master.
> - Coding style fixes
> - Provide comment for tunnel-push without post actions.
> - Corrected new packet-aware test suites to pass userspace testsuites.
> - Changes on cache_steal function to use memcpy and ofpbuf_put_uninit
> functions.
>
> v3
> - Rebased on latest master
> - Changed the xlate_cache copy function to avoid expensive ref update
> operations.
> - Updated the new packet-aware test cases to handle the non-recirc tunnel
> case.
> - Updated the commit message with performance results.
>
> v2
> - Rebased on latest master.
> - Updated newely added packet-aware test case to honor tunnel combine
> actions.
> - Folded related patches into single patch based on Joe's comments.
> - Do the translation only once for tunnel combine instead of two.
>
>
> Sugesh Chandran (3):
> xlate: Clear tunnel mask along with other fields while combine
> actions.
> tunneling: Calculate and update packet l4_offset in tunnel push.
> tunneling: Avoid datapath-recirc by combining recirc actions at xlate.
>
> lib/dpif-netdev.c | 18 +--
> lib/netdev-native-tnl.c | 2 +
> ofproto/ofproto-dpif-xlate-cache.c | 32 +++-
> ofproto/ofproto-dpif-xlate-cache.h | 13 +-
> ofproto/ofproto-dpif-xlate.c | 238
> +++++++++++++++++++++++++++-
> ofproto/ofproto-dpif.c | 3 +-
> tests/packet-type-aware.at | 27 ++--
> tests/system-userspace-packet-type-aware.at | 24 +--
> 8 files changed, 296 insertions(+), 61 deletions(-)
>
> --
> 2.7.4
>
Thanks Sugesh and Zoltán, I applied this series to master with the
following minor style incremental to the last patch:
diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 5dbf5833dfb8..ef579409b0d0 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -139,7 +139,7 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet
*packet, struct flow_tnl *tnl,
*
* This function sets the IP header's ip_tot_len field (which should be zeroed
* as part of 'header') and puts its value into '*ip_tot_size' as well. Also
- * updates IP header checksum.
+ * updates IP header checksum, as well as the l3 and l4 offsets in 'packet'.
*
* Return pointer to the L4 header added to 'packet'. */
void *
diff --git a/ofproto/ofproto-dpif-xlate-cache.c
b/ofproto/ofproto-dpif-xlate-cache.c
index 6947f2fd318d..d319d287eadb 100644
--- a/ofproto/ofproto-dpif-xlate-cache.c
+++ b/ofproto/ofproto-dpif-xlate-cache.c
@@ -300,9 +300,10 @@ xlate_cache_steal_entries(struct xlate_cache
*dst, struct xlate_cache *src)
if (!dst || !src) {
return;
}
- void *p;
struct ofpbuf *src_entries = &src->entries;
struct ofpbuf *dst_entries = &dst->entries;
+ void *p;
+
p = ofpbuf_put_uninit(dst_entries, src_entries->size);
memcpy(p, src_entries->data, src_entries->size);
ofpbuf_clear(src_entries);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 0177f5c15b71..7f7adb280eaf 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3326,13 +3326,13 @@ validate_and_combine_post_tnl_actions(struct
xlate_ctx *ctx,
/* Update the CLONE action only when combined. */
nl_msg_end_nested(ctx->odp_actions, clone_ofs);
} else {
+ nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
/* XXX : There is no real use-case for a tunnel push without
* any post actions. However keeping it now
* as is to make the 'make check' happy. Should remove when all the
* make check tunnel test case does something meaningful on a
* tunnel encap packets.
*/
- nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data);
}
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
