| Message ID | 1477548924-26376-5-git-send-email-qiudayu@chinac.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Daniele Di Proietto |
| Headers | show |
>When doing packet clone, if packet source is from DPDK driver, >multi-segment must be considered, and copy the segment's >data one by one. > >Signed-off-by: Michael Qiu <qiudayu@chinac.com> >Signed-off-by: Jijiang Liu <liujijiang@chinac.com> >--- > lib/dp-packet.c | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > >diff --git a/lib/dp-packet.c b/lib/dp-packet.c >index 619f651..3ca1fde 100644 >--- a/lib/dp-packet.c >+++ b/lib/dp-packet.c >@@ -164,9 +164,28 @@ dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t >headroom) > { > struct dp_packet *new_buffer; > >- new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer), >- dp_packet_size(buffer), >- headroom); >+ uint32_t size = dp_packet_size(buffer); >+ >+ /* copy multi-seg data */ >+ #ifdef DPDK_NETDEV Align compiler guard to far left margin >+ if (buffer->source == DPBUF_DPDK && buffer->mbuf.nb_segs > 1) { >+ struct rte_mbuf *tmbuf = &(buffer->mbuf); This assignment introduces a compiler warning: lib/dp-packet.c:173:34: warning: initialization discards 'const' qualifier from pointer target type [enabled by default] This removes the warning: + struct rte_mbuf *tmbuf = CONST_CAST(struct rte_mbuf *, &(buffer->mbuf)); >+ new_buffer = dp_packet_new_with_headroom(size, headroom); >+ void *dst = dp_packet_put_uninit(new_buffer, size); Why do you need to call dp_packet_put_uninit? This adds an additional (and unneccesary) 'size' bytes to the tail of the dp_packet. >+ uint32_t off_set = 0; >+ >+ while (tmbuf) { >+ rte_memcpy((char *)dst + off_set, >+ rte_pktmbuf_mtod(tmbuf, void *), tmbuf->data_len); >+ off_set += tmbuf->data_len; >+ tmbuf = tmbuf->next; >+ } >+ } >+ else >+ #endif >+ new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer), >+ size, headroom); >+ > new_buffer->l2_pad_size = buffer->l2_pad_size; > new_buffer->l2_5_ofs = buffer->l2_5_ofs; > new_buffer->l3_ofs = buffer->l3_ofs; >-- >1.8.3.1
2016/10/28 18:50, Kavanagh, Mark B : >> When doing packet clone, if packet source is from DPDK driver, >> multi-segment must be considered, and copy the segment's >> data one by one. >> >> Signed-off-by: Michael Qiu <qiudayu@chinac.com> >> Signed-off-by: Jijiang Liu <liujijiang@chinac.com> >> --- >> lib/dp-packet.c | 25 ++++++++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/lib/dp-packet.c b/lib/dp-packet.c >> index 619f651..3ca1fde 100644 >> --- a/lib/dp-packet.c >> +++ b/lib/dp-packet.c >> @@ -164,9 +164,28 @@ dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t >> headroom) >> { >> struct dp_packet *new_buffer; >> >> - new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer), >> - dp_packet_size(buffer), >> - headroom); >> + uint32_t size = dp_packet_size(buffer); >> + >> + /* copy multi-seg data */ >> + #ifdef DPDK_NETDEV > Align compiler guard to far left margin OK, I've got your point, will fix it. > >> + if (buffer->source == DPBUF_DPDK && buffer->mbuf.nb_segs > 1) { >> + struct rte_mbuf *tmbuf = &(buffer->mbuf); > This assignment introduces a compiler warning: > lib/dp-packet.c:173:34: warning: initialization discards 'const' qualifier from pointer target type [enabled by default] > This removes the warning: > + struct rte_mbuf *tmbuf = CONST_CAST(struct rte_mbuf *, &(buffer->mbuf)); Thanks, you are right, and I will fix it. >> + new_buffer = dp_packet_new_with_headroom(size, headroom); >> + void *dst = dp_packet_put_uninit(new_buffer, size); > Why do you need to call dp_packet_put_uninit? This adds an additional (and unneccesary) 'size' bytes to the tail of the dp_packet. First, dp_packet_put_uninit() will finally be called in before, see call path: dp_packet_clone() ---> dp_packet_clone_with_headroom() ---> dp_packet_clone_data_with_headroom() ---> dp_packet_put() ---> dp_packet_put_uninit() Second, dp_packet_put_uninit() does not add an additional 'size' bytes in this case, because dp_packet_prealloc_tailroom() will check if it has 'size' bytes in its tailroom, and we have already allocated enought size here. Third, the new allocated buffer hasn't set the pkt_len(packet size). I use it only becasue it return the data start address here, and we also set the packet size here. > >> + uint32_t off_set = 0; >> + >> + while (tmbuf) { >> + rte_memcpy((char *)dst + off_set, >> + rte_pktmbuf_mtod(tmbuf, void *), tmbuf->data_len); >> + off_set += tmbuf->data_len; >> + tmbuf = tmbuf->next; >> + } >> + } >> + else >> + #endif >> + new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer), >> + size, headroom); >> + >> new_buffer->l2_pad_size = buffer->l2_pad_size; >> new_buffer->l2_5_ofs = buffer->l2_5_ofs; >> new_buffer->l3_ofs = buffer->l3_ofs; >> -- >> 1.8.3.1 >
diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 619f651..3ca1fde 100644 --- a/lib/dp-packet.c +++ b/lib/dp-packet.c @@ -164,9 +164,28 @@ dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t headroom) { struct dp_packet *new_buffer; - new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer), - dp_packet_size(buffer), - headroom); + uint32_t size = dp_packet_size(buffer); + + /* copy multi-seg data */ + #ifdef DPDK_NETDEV + if (buffer->source == DPBUF_DPDK && buffer->mbuf.nb_segs > 1) { + struct rte_mbuf *tmbuf = &(buffer->mbuf); + new_buffer = dp_packet_new_with_headroom(size, headroom); + void *dst = dp_packet_put_uninit(new_buffer, size); + uint32_t off_set = 0; + + while (tmbuf) { + rte_memcpy((char *)dst + off_set, + rte_pktmbuf_mtod(tmbuf, void *), tmbuf->data_len); + off_set += tmbuf->data_len; + tmbuf = tmbuf->next; + } + } + else + #endif + new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(buffer), + size, headroom); + new_buffer->l2_pad_size = buffer->l2_pad_size; new_buffer->l2_5_ofs = buffer->l2_5_ofs; new_buffer->l3_ofs = buffer->l3_ofs;