[ovs-dev] [PATCH] dpif-netdev: Remove an extra memcpy of packet data from datapath-upcall interface for bridges with datapath_type=netdev.

[Pravin Shelar]

On Tue, Jun 3, 2014 at 10:33 AM, Ryan Wilson <wryan at nicira.com> wrote:
> When a bridge of datatype type netdev receives a packet, it copies the
> packet from the NIC to a buffer in userspace. Currently, when making
> an upcall, the packet is again copied to the upcall's buffer. However,
> this extra copy is not necessary when the datapath exists in userspace
> as the upcall can directly access the packet data.
>
> This patch eliminates this extra copy of the packet data in most cases.
> In cases where the packet may still be used later by callers of
> dp_netdev_execute_actions, making a copy of the packet data is still
> necessary.
>
> Signed-off-by: Ryan Wilson <wryan at nicira.com>
> ---
> v2: Addressed Jarno's comment to use direct pointer assignment for
> upcall->packet instead of ofpbuf_set().
> ---
>  lib/dpif-netdev.c |   15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 91c83d6..c89ae20 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2024,7 +2024,6 @@ dp_netdev_input(struct dp_netdev *dp, struct ofpbuf *packet,
>                                     miniflow_hash_5tuple(&key.flow, 0)
>                                     % dp->n_handlers,
>                                     DPIF_UC_MISS, &key.flow, NULL);
> -        ofpbuf_delete(packet);
>      }
>  }
>
> @@ -2063,7 +2062,6 @@ dp_netdev_output_userspace(struct dp_netdev *dp, struct ofpbuf *packet,
>          if (userdata) {
>              buf_size += NLA_ALIGN(userdata->nla_len);
>          }
> -        buf_size += ofpbuf_size(packet);
>          ofpbuf_init(buf, buf_size);
>
>          /* Put ODP flow. */
> @@ -2078,15 +2076,14 @@ dp_netdev_output_userspace(struct dp_netdev *dp, struct ofpbuf *packet,
>                                            NLA_ALIGN(userdata->nla_len));
>          }
>
> -        ofpbuf_set_data(&upcall->packet,
> -                        ofpbuf_put(buf, ofpbuf_data(packet), ofpbuf_size(packet)));
> -        ofpbuf_set_size(&upcall->packet, ofpbuf_size(packet));
> +        upcall->packet = *packet;
>

This would not work with DPDK. ofpbuf from dpdk is special case where
ofpbuf and data are allocated from same memory object. Therefore
moving ofpbuf->data is nontrivial.

To make it work we need atleast following covered.
1. Define flag for source of ofpbuf header. So that we can track
header and data independently.
2. Fix dpdk ofpbuf destructor to free correct dpdk memory object.

Thanks,
Pravin.