I've ran into some unexpected issues while perf testing this, lets hold off on looking at this. I'll submit another patch when I've had all the kinks worked out.
Ryan On 6/3/14 2:21 PM, "Ryan Wilson 76511" <wr...@vmware.com> wrote: >Hey Pravin, > >Thanks for the catch here! Turns out the header is already tracked in DPDK >with rte_mbuf's buffer address - sizeof(ofpbuf). Thus, I submitted another >patch that, in free_dpdk_buf(), always gets this header and uses this to >free memory. > >http://patchwork.openvswitch.org/patch/4375/ > > >Let me know if this patch works. > >Cheers, > >Ryan > >On 6/3/14 10:50 AM, "Pravin Shelar" <pshe...@nicira.com> wrote: > >>On Tue, Jun 3, 2014 at 10:33 AM, Ryan Wilson <wr...@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 <wr...@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. > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
