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
