Thankyou Joe!

Regards
_Sugesh


> -----Original Message-----
> From: Joe Stringer [mailto:[email protected]]
> Sent: Wednesday, July 19, 2017 10:38 PM
> To: Chandran, Sugesh <[email protected]>
> Cc: ovs dev <[email protected]>; Andy Zhou <[email protected]>; Zoltán
> Balogh <[email protected]>
> Subject: Re: [PATCH v4 0/3] tunneling : Improving tunneling performance by
> avoiding dp recirc.
> 
> 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

Reply via email to