Message ID | 1493705445-5774-2-git-send-email-qdy220091330@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
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?
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
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 --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; }