Can you combine it with second patch, otherwise it introduces a bug.
On Tue, Jun 3, 2014 at 4:22 PM, Ryan Wilson 76511 <wr...@vmware.com> wrote: > 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
