On Wed, Mar 19, 2014 at 8:44 AM, Jarno Rajahalme <[email protected]> wrote:
>
> On Mar 18, 2014, at 8:19 PM, Pravin Shelar <[email protected]> wrote:
>
> On Tue, Mar 18, 2014 at 3:56 PM, Jarno Rajahalme <[email protected]>
> wrote:
>
>
> On Mar 18, 2014, at 1:53 PM, Pravin <[email protected]> wrote:
>
> DPDK can receive multiple packets but current netdev API does
> not allow that.  Following patch allows dpif-netdev receive batch
> of packet in a rx_recv() call for any netdev port.  This will be
> used by dpdk-netdev.
>
> Signed-off-by: Pravin B Shelar <[email protected]>
> ---
> lib/automake.mk       |    1 +
> lib/dpif-netdev.c     |   51
> ++++++++++++++++++-------------------------------
> lib/dpif-netdev.h     |   40 ++++++++++++++++++++++++++++++++++++++
> lib/netdev-dummy.c    |   26 +++++++++----------------
> lib/netdev-linux.c    |   20 ++++++++++++++++---
> lib/netdev-provider.h |   24 +++++------------------
> lib/netdev.c          |   24 +++++------------------
> lib/netdev.h          |    2 +-
> lib/packets.c         |    9 +++++++++
> lib/packets.h         |    1 +
> 10 files changed, 107 insertions(+), 91 deletions(-)
> create mode 100644 lib/dpif-netdev.h
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index e22165c..832b7f9 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -52,6 +52,7 @@ lib_libopenvswitch_la_SOURCES = \
>      lib/dhparams.h \
>      lib/dirs.h \
>      lib/dpif-netdev.c \
> +     lib/dpif-netdev.h \
>      lib/dpif-provider.h \
>      lib/dpif.c \
>      lib/dpif.h \
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 3bbfd2a..d78fb25 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -34,6 +34,7 @@
> #include "classifier.h"
> #include "csum.h"
> #include "dpif.h"
> +#include "dpif-netdev.h"
> #include "dpif-provider.h"
> #include "dummy.h"
> #include "dynamic-string.h"
> @@ -68,10 +69,6 @@ VLOG_DEFINE_THIS_MODULE(dpif_netdev);
> /* Configuration parameters. */
> enum { MAX_FLOWS = 65536 };     /* Maximum number of flows in flow table. */
>
> -/* Enough headroom to add a vlan tag, plus an extra 2 bytes to allow IP
> - * headers to be aligned on a 4-byte boundary.  */
> -enum { DP_NETDEV_HEADROOM = 2 + VLAN_HEADER_LEN };
> -
> /* Queues. */
> enum { N_QUEUES = 2 };          /* Number of queues for dpif_recv(). */
> enum { MAX_QUEUE_LEN = 128 };   /* Maximum number of packets per queue. */
> @@ -1443,6 +1440,7 @@ dpif_netdev_execute(struct dpif *dpif, struct
> dpif_execute *execute)
> {
>    struct dp_netdev *dp = get_dp_netdev(dpif);
>    struct pkt_metadata *md = &execute->md;
> +    struct ofpbuf *packet;
>    struct flow key;
>
>    if (execute->packet->size < ETH_HEADER_LEN ||
> @@ -1453,8 +1451,9 @@
>
> (struct dpif *dpif, struct dpif_execute *execute)
>
>    /* Extract flow key. */
>    flow_extract(execute->packet, md, &key);
>
> +    packet = ofpbuf_clone(execute->packet);
>    ovs_rwlock_rdlock(&dp->port_rwlock);
> -    dp_netdev_execute_actions(dp, &key, execute->packet, md,
> execute->actions,
> +    dp_netdev_execute_actions(dp, &key, packet, md, execute->actions,
>                              execute->actions_len);
>    ovs_rwlock_unlock(&dp->port_rwlock);
>
>
> It is not clear to me why the packet should be cloned here, especially since
> you are already copying the data in the userspace action. Also, it seems
> this will leak memory in cases where there is no OUTPUT or USERSPACE action,
> as the dp_execute_action() will not be called in those cases (e.g., a drop
> flow).
>
>
> In Model I am following DKDP dpdk-recv generate ofpbuf and send consumes it.
> That means dpdk-send calls ofpbuf_delete() on every buffer dpif-netdev
> sends to it.
> Here in dp_execute_action()  I need to clone ofpbuf so that dpdk-send
> can release it. execute->packet passed can not freed by
> ofpbuf_delete().
> I missed drop case, I will fix the code.
>
>
> I may be wrong but it seems to me that this generates unnecessary memory
> allocations/frees for the ofpbufs themselves. On recv interface you could
> use an array ofpbufs instead of pointers, and have the send to unknit
> instead of delete. Not using additional indirection in the recv array may
> also be more cache friendly.
>

In next patch series I am going to move mbuf to ofpbuf and then dpdk
would return ofpbuf object rather than mbuf for a packet. After than
change we can get rid of alloc and free calls in dpdk recv and send. I
need to patch DPDK to get that working therefore I have not sent that
patch yet.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to