| Message ID | 1528734090-220990-4-git-send-email-tiago.lam@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support multi-segment mbufs | expand |
On 11 Jun 2018, at 18:21, Tiago Lam wrote: > When enabled with DPDK OvS deals with two types of packets, the ones > coming from the mempool and the ones locally created by OvS - which > are > copied to mempool mbufs before output. In the latter, the space is > allocated from the system, while in the former the mbufs are allocated > from a mempool, which takes care of initialising them appropriately. > > In the current implementation, during mempool's initialisation of > mbufs, > dp_packet_set_allocated() is called from dp_packet_init_dpdk() without > considering that the allocated space, in the case of multi-segment > mbufs, might be greater than a single mbuf. Furthermore, given that > dp_packet_init_dpdk() is on the code path that's called upon mempool's > initialisation, a call to dp_packet_set_allocated() is redundant, > since > mempool takes care of initialising it. > > To fix this, dp_packet_set_allocated() is no longer called after > initialisation of a mempool, only in dp_packet_init__(), which is > still > called by OvS when initialising locally created packets. > LGTM, Acked-by: Eelco Chaudron <echaudro@redhat.com> > Signed-off-by: Tiago Lam <tiago.lam@intel.com> > --- > lib/dp-packet.c | 3 +-- > lib/dp-packet.h | 2 +- > lib/netdev-dpdk.c | 2 +- > 3 files changed, 3 insertions(+), 4 deletions(-) > > diff --git a/lib/dp-packet.c b/lib/dp-packet.c > index 443c225..782e7c2 100644 > --- a/lib/dp-packet.c > +++ b/lib/dp-packet.c > @@ -99,9 +99,8 @@ dp_packet_use_const(struct dp_packet *b, const void > *data, size_t size) > * buffer. Here, non-transient ovs dp-packet fields are initialized > for > * packets that are part of a DPDK memory pool. */ > void > -dp_packet_init_dpdk(struct dp_packet *b, size_t allocated) > +dp_packet_init_dpdk(struct dp_packet *b) > { > - dp_packet_set_allocated(b, allocated); > b->source = DPBUF_DPDK; > } > > diff --git a/lib/dp-packet.h b/lib/dp-packet.h > index 82e45ad..4c104b6 100644 > --- a/lib/dp-packet.h > +++ b/lib/dp-packet.h > @@ -114,7 +114,7 @@ void dp_packet_use(struct dp_packet *, void *, > size_t); > void dp_packet_use_stub(struct dp_packet *, void *, size_t); > void dp_packet_use_const(struct dp_packet *, const void *, size_t); > > -void dp_packet_init_dpdk(struct dp_packet *, size_t allocated); > +void dp_packet_init_dpdk(struct dp_packet *); > > void dp_packet_init(struct dp_packet *, size_t); > void dp_packet_uninit(struct dp_packet *); > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 468ab36..f546507 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -519,7 +519,7 @@ ovs_rte_pktmbuf_init(struct rte_mempool *mp > OVS_UNUSED, > { > struct rte_mbuf *pkt = _p; > > - dp_packet_init_dpdk((struct dp_packet *) pkt, pkt->buf_len); > + dp_packet_init_dpdk((struct dp_packet *) pkt); > } > > static int > -- > 2.7.4
diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 443c225..782e7c2 100644 --- a/lib/dp-packet.c +++ b/lib/dp-packet.c @@ -99,9 +99,8 @@ dp_packet_use_const(struct dp_packet *b, const void *data, size_t size) * buffer. Here, non-transient ovs dp-packet fields are initialized for * packets that are part of a DPDK memory pool. */ void -dp_packet_init_dpdk(struct dp_packet *b, size_t allocated) +dp_packet_init_dpdk(struct dp_packet *b) { - dp_packet_set_allocated(b, allocated); b->source = DPBUF_DPDK; } diff --git a/lib/dp-packet.h b/lib/dp-packet.h index 82e45ad..4c104b6 100644 --- a/lib/dp-packet.h +++ b/lib/dp-packet.h @@ -114,7 +114,7 @@ void dp_packet_use(struct dp_packet *, void *, size_t); void dp_packet_use_stub(struct dp_packet *, void *, size_t); void dp_packet_use_const(struct dp_packet *, const void *, size_t); -void dp_packet_init_dpdk(struct dp_packet *, size_t allocated); +void dp_packet_init_dpdk(struct dp_packet *); void dp_packet_init(struct dp_packet *, size_t); void dp_packet_uninit(struct dp_packet *); diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 468ab36..f546507 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -519,7 +519,7 @@ ovs_rte_pktmbuf_init(struct rte_mempool *mp OVS_UNUSED, { struct rte_mbuf *pkt = _p; - dp_packet_init_dpdk((struct dp_packet *) pkt, pkt->buf_len); + dp_packet_init_dpdk((struct dp_packet *) pkt); } static int
When enabled with DPDK OvS deals with two types of packets, the ones coming from the mempool and the ones locally created by OvS - which are copied to mempool mbufs before output. In the latter, the space is allocated from the system, while in the former the mbufs are allocated from a mempool, which takes care of initialising them appropriately. In the current implementation, during mempool's initialisation of mbufs, dp_packet_set_allocated() is called from dp_packet_init_dpdk() without considering that the allocated space, in the case of multi-segment mbufs, might be greater than a single mbuf. Furthermore, given that dp_packet_init_dpdk() is on the code path that's called upon mempool's initialisation, a call to dp_packet_set_allocated() is redundant, since mempool takes care of initialising it. To fix this, dp_packet_set_allocated() is no longer called after initialisation of a mempool, only in dp_packet_init__(), which is still called by OvS when initialising locally created packets. Signed-off-by: Tiago Lam <tiago.lam@intel.com> --- lib/dp-packet.c | 3 +-- lib/dp-packet.h | 2 +- lib/netdev-dpdk.c | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-)