[ovs-dev] [PATCH v8 10/13] dp-packet: copy data from multi-seg. DPDK mbuf
Eelco Chaudron
echaudro at redhat.com
Mon Jun 18 13:10:26 UTC 2018
On 11 Jun 2018, at 18:21, Tiago Lam wrote:
> From: Michael Qiu <qiudayu at chinac.com>
>
> 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.
>
> Also, lots of DPDK mbuf's info is missed during a copy, like packet
> type, ol_flags, etc. That information is very important for DPDK to
> do
> packets processing.
>
> Co-authored-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
>
> Signed-off-by: Michael Qiu <qiudayu at chinac.com>
> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
> ---
> lib/dp-packet.c | 76
> +++++++++++++++++++++++++++++++++++++++++++++++--------
> lib/dp-packet.h | 3 +++
> lib/netdev-dpdk.c | 1 +
> 3 files changed, 69 insertions(+), 11 deletions(-)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index d0fab94..2e65b82 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -48,6 +48,23 @@ dp_packet_use__(struct dp_packet *b, void *base,
> size_t allocated,
> dp_packet_set_size(b, 0);
> }
>
> +#ifdef DPDK_NETDEV
> +void
> +dp_packet_copy_mbuf_flags(struct dp_packet *dst, const struct
> dp_packet *src)
> +{
> + ovs_assert(dst != NULL && src != NULL);
> + struct rte_mbuf *buf_dst = &(dst->mbuf);
> + struct rte_mbuf buf_src = src->mbuf;
> +
> + buf_dst->nb_segs = buf_src.nb_segs;
> + buf_dst->ol_flags = buf_src.ol_flags;
> + buf_dst->packet_type = buf_src.packet_type;
> + buf_dst->tx_offload = buf_src.tx_offload;
> +}
> +#else
> +#define dp_packet_copy_mbuf_flags(arg1, arg2)
> +#endif
> +
> /* Initializes 'b' as an empty dp_packet that contains the
> 'allocated' bytes of
> * memory starting at 'base'. 'base' should be the first byte of a
> region
> * obtained from malloc(). It will be freed (with free()) if 'b' is
> resized or
> @@ -158,6 +175,50 @@ dp_packet_clone(const struct dp_packet *buffer)
> return dp_packet_clone_with_headroom(buffer, 0);
> }
>
> +#ifdef DPDK_NETDEV
> +struct dp_packet *
> +dp_packet_clone_with_headroom(const struct dp_packet *buffer,
> + size_t headroom) {
> + struct dp_packet *new_buffer;
> + uint32_t pkt_len = dp_packet_size(buffer);
> +
> + /* copy multi-seg data */
> + if (buffer->source == DPBUF_DPDK && buffer->mbuf.nb_segs > 1) {
> + uint32_t offset = 0;
> + void *dst = NULL;
> + struct rte_mbuf *tmbuf = CONST_CAST(struct rte_mbuf *,
> + &(buffer->mbuf));
> +
> + new_buffer = dp_packet_new_with_headroom(pkt_len, headroom);
> + dp_packet_set_size(new_buffer, pkt_len + headroom);
> + dst = dp_packet_tail(new_buffer);
> +
The dst is not pointing to the first data, but to the end of the data,
as dp_packet_set_size() has been called. Why not just call dst =
dp_packet_put_uninit().
> + while (tmbuf) {
> + rte_memcpy((char *)dst + offset,
> + rte_pktmbuf_mtod(tmbuf, void *),
> tmbuf->data_len);
> + offset += tmbuf->data_len;
> + tmbuf = tmbuf->next;
Why not using rte_pktmbuf_read() here with a check its not a contiguous
read?
> + }
> + } else {
> + new_buffer =
> dp_packet_clone_data_with_headroom(dp_packet_data(buffer),
> +
> dp_packet_size(buffer),
> + headroom);
> + }
> +
> + /* Copy the following fields into the returned buffer:
> l2_pad_size,
> + * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */
> + memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size,
> + sizeof(struct dp_packet) -
> + offsetof(struct dp_packet, l2_pad_size));
> +
> + dp_packet_copy_mbuf_flags(new_buffer, buffer);
> + if (dp_packet_rss_valid(new_buffer)) {
> + new_buffer->mbuf.hash.rss = buffer->mbuf.hash.rss;
> + }
> +
> + return new_buffer;
> +}
> +#else
> /* Creates and returns a new dp_packet whose data are copied from
> 'buffer'.
> * The returned dp_packet will additionally have 'headroom' bytes of
> * headroom. */
> @@ -165,32 +226,25 @@ struct dp_packet *
> dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t
> headroom)
> {
> struct dp_packet *new_buffer;
> + uint32_t pkt_len = dp_packet_size(buffer);
>
> new_buffer =
> dp_packet_clone_data_with_headroom(dp_packet_data(buffer),
> -
> dp_packet_size(buffer),
> - headroom);
> + pkt_len, headroom);
> +
> /* Copy the following fields into the returned buffer:
> l2_pad_size,
> * l2_5_ofs, l3_ofs, l4_ofs, cutlen, packet_type and md. */
> memcpy(&new_buffer->l2_pad_size, &buffer->l2_pad_size,
> sizeof(struct dp_packet) -
> offsetof(struct dp_packet, l2_pad_size));
>
> -#ifdef DPDK_NETDEV
> - new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags;
> -#else
> new_buffer->rss_hash_valid = buffer->rss_hash_valid;
> -#endif
> -
> if (dp_packet_rss_valid(new_buffer)) {
> -#ifdef DPDK_NETDEV
> - new_buffer->mbuf.hash.rss = buffer->mbuf.hash.rss;
> -#else
> new_buffer->rss_hash = buffer->rss_hash;
> -#endif
> }
>
> return new_buffer;
> }
> +#endif
>
> /* Creates and returns a new dp_packet that initially contains a copy
> of the
> * 'size' bytes of data starting at 'data' with no headroom or
> tailroom. */
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 4946fa3..3852756 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -124,6 +124,9 @@ void dp_packet_init_dpdk(struct dp_packet *);
> void dp_packet_init(struct dp_packet *, size_t);
> void dp_packet_uninit(struct dp_packet *);
>
> +void dp_packet_copy_mbuf_flags(struct dp_packet *dst,
> + const struct dp_packet *src);
> +
> struct dp_packet *dp_packet_new(size_t);
> struct dp_packet *dp_packet_new_with_headroom(size_t, size_t
> headroom);
> struct dp_packet *dp_packet_clone(const struct dp_packet *);
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index efd7c20..9b1fb9a 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2215,6 +2215,7 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid,
> struct dp_packet_batch *batch)
> memcpy(rte_pktmbuf_mtod(pkts[txcnt], void *),
> dp_packet_data(packet), size);
> dp_packet_set_size((struct dp_packet *)pkts[txcnt], size);
> + dp_packet_copy_mbuf_flags((struct dp_packet *)pkts[txcnt],
> packet);
>
> txcnt++;
> }
> --
> 2.7.4
More information about the dev
mailing list