diff mbox

[ovs-dev,1/5] lib/dp-packet: init the mbuf to zero when build with DPDK

Message ID 1493705445-5774-2-git-send-email-qdy220091330@gmail.com
State Changes Requested
Headers show

Commit Message

Michael Qiu May 2, 2017, 6:10 a.m. UTC 
From: Michael Qiu <qiudayu@chinac.com>

When building with DPDK, and using xmalloc() to get a new packet,
field mbuf of the packet will not be initialized, but it's very important for
DPDK port when copying the data to DPDK mbuf, because if ol_flags
and other info are random values, DPDK driver may hang.

Signed-off-by: Michael Qiu <qiudayu@chinac.com>
---
 lib/dp-packet.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Ben Pfaff May 4, 2017, 9:19 p.m. UTC | #1 
On Tue, May 02, 2017 at 02:10:41PM +0800, Michael Qiu wrote:
> From: Michael Qiu <qiudayu@chinac.com>
> 
> When building with DPDK, and using xmalloc() to get a new packet,
> field mbuf of the packet will not be initialized, but it's very important for
> DPDK port when copying the data to DPDK mbuf, because if ol_flags
> and other info are random values, DPDK driver may hang.
> 
> Signed-off-by: Michael Qiu <qiudayu@chinac.com>
> ---
>  lib/dp-packet.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index 793b54f..109947c 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -132,6 +132,9 @@ struct dp_packet *
>  dp_packet_new(size_t size)
>  {
>      struct dp_packet *b = xmalloc(sizeof *b);
> +#ifdef DPDK_NETDEV
> +    memset(&(b->mbuf), 0, sizeof(struct rte_mbuf));
> +#endif
>      dp_packet_init(b, size);
>      return b;
>  }

Thanks for working to improve DPDK support in OVS.  This seems correct
as far as it goes, but shouldn't it happen in dp_packet_init__() so that
it applies to all dp_packet initialization paths, instead of just the
packets allocated this way?
Darrell Ball May 4, 2017, 11 p.m. UTC | #2 
On 5/1/17, 11:10 PM, "ovs-dev-bounces@openvswitch.org on behalf of Michael Qiu" <ovs-dev-bounces@openvswitch.org on behalf of qdy220091330@gmail.com> wrote:

    From: Michael Qiu <qiudayu@chinac.com>
    
    When building with DPDK, and using xmalloc() to get a new packet,
    field mbuf of the packet will not be initialized, but it's very important for
    DPDK port when copying the data to DPDK mbuf, because if ol_flags
    and other info are random values, DPDK driver may hang.
    
    Signed-off-by: Michael Qiu <qiudayu@chinac.com>
    ---
     lib/dp-packet.c | 3 +++
     1 file changed, 3 insertions(+)
    
    diff --git a/lib/dp-packet.c b/lib/dp-packet.c
    index 793b54f..109947c 100644
    --- a/lib/dp-packet.c
    +++ b/lib/dp-packet.c
    @@ -132,6 +132,9 @@ struct dp_packet *
     dp_packet_new(size_t size)
     {
         struct dp_packet *b = xmalloc(sizeof *b);
    +#ifdef DPDK_NETDEV
    +    memset(&(b->mbuf), 0, sizeof(struct rte_mbuf));

In addition to the comment Ben had for this patch, can you also investigate:

1) Which fields need initializing for multi-seg to work
and potentially only initialize those, if there are only a few, for example,
rather than memset the whole struct.


    +#endif
         dp_packet_init(b, size);
         return b;
     }
    -- 
    1.8.3.1
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Michael Qiu May 5, 2017, 5:15 a.m. UTC | #3 
Hi, Ben


在 2017/5/5 7:00, Darrell Ball 写道:
>
> On 5/1/17, 11:10 PM, "ovs-dev-bounces@openvswitch.org on behalf of Michael Qiu" <ovs-dev-bounces@openvswitch.org on behalf of qdy220091330@gmail.com> wrote:
>
>      From: Michael Qiu <qiudayu@chinac.com>
>
>      When building with DPDK, and using xmalloc() to get a new packet,
>      field mbuf of the packet will not be initialized, but it's very important for
>      DPDK port when copying the data to DPDK mbuf, because if ol_flags
>      and other info are random values, DPDK driver may hang.
>
>      Signed-off-by: Michael Qiu <qiudayu@chinac.com>
>      ---
>       lib/dp-packet.c | 3 +++
>       1 file changed, 3 insertions(+)
>
>      diff --git a/lib/dp-packet.c b/lib/dp-packet.c
>      index 793b54f..109947c 100644
>      --- a/lib/dp-packet.c
>      +++ b/lib/dp-packet.c
>      @@ -132,6 +132,9 @@ struct dp_packet *
>       dp_packet_new(size_t size)
>       {
>           struct dp_packet *b = xmalloc(sizeof *b);
>      +#ifdef DPDK_NETDEV
>      +    memset(&(b->mbuf), 0, sizeof(struct rte_mbuf));
>
> In addition to the comment Ben had for this patch, can you also investigate:
>
> 1) Which fields need initializing for multi-seg to work
> and potentially only initialize those, if there are only a few, for example,
> rather than memset the whole struct.

Now, we have four. So I chose to memset the whole struct.

>
>
>      +#endif
>           dp_packet_init(b, size);
>           return b;
>       }
>      --
>      1.8.3.1
>
>      _______________________________________________
>      dev mailing list
>      dev@openvswitch.org
>      https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
diff mbox

Patch

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 793b54f..109947c 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -132,6 +132,9 @@  struct dp_packet *
 dp_packet_new(size_t size)
 {
     struct dp_packet *b = xmalloc(sizeof *b);
+#ifdef DPDK_NETDEV
+    memset(&(b->mbuf), 0, sizeof(struct rte_mbuf));
+#endif
     dp_packet_init(b, size);
     return b;
 }