> From: Michael Qiu <[email protected]> > > 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. >
Thanks Tiago, few comments below. Ian > Co-authored-by: Mark Kavanagh <[email protected]> > Co-authored-by: Tiago Lam <[email protected]> > > Signed-off-by: Michael Qiu <[email protected]> > Signed-off-by: Mark Kavanagh <[email protected]> > Signed-off-by: Tiago Lam <[email protected]> > Acked-by: Eelco Chaudron <[email protected]> > --- > lib/dp-packet.c | 57 > ++++++++++++++++++++++++++++++++++++------------------- > lib/dp-packet.h | 33 ++++++++++++++++++++++++++++++++ > lib/netdev-dpdk.c | 1 + > 3 files changed, 72 insertions(+), 19 deletions(-) > > diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 4c197b6..1b9503c > 100644 > --- a/lib/dp-packet.c > +++ b/lib/dp-packet.c > @@ -163,39 +163,58 @@ 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 *b, size_t headroom) { > + struct dp_packet *new_buffer; > + uint32_t pkt_len = dp_packet_size(b); > + > + /* copy multi-seg data */ Minor: Can you capitalize the first word and add a period to the end of the sentence for this comment. > + if (b->source == DPBUF_DPDK && !rte_pktmbuf_is_contiguous(&b->mbuf)) { > + void *dst = NULL; > + struct rte_mbuf *mbuf = CONST_CAST(struct rte_mbuf *, > + &b->mbuf); > + > + new_buffer = dp_packet_new_with_headroom(pkt_len, headroom); > + dst = dp_packet_data(new_buffer); > + dp_packet_set_size(new_buffer, pkt_len); > + > + if (!rte_pktmbuf_read(mbuf, 0, pkt_len, dst)) { > + dp_packet_delete(new_buffer); > + return NULL; > + } > + } else { > + new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(b), > + dp_packet_size(b), > + headroom); > + } > + > + dp_packet_copy_common_members(new_buffer, b); > + > + 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. */ > struct dp_packet * > -dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t > headroom) > +dp_packet_clone_with_headroom(const struct dp_packet *b, size_t > +headroom) > { > struct dp_packet *new_buffer; > + uint32_t pkt_len = dp_packet_size(b); > > - 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)); > + new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(b), > + pkt_len, > + headroom); > > -#ifdef DPDK_NETDEV > - new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags; > -#else > - new_buffer->rss_hash_valid = buffer->rss_hash_valid; > -#endif > + dp_packet_copy_common_members(new_buffer, b); > > + new_buffer->rss_hash_valid = b->rss_hash_valid; > 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 > + new_buffer->rss_hash = b->rss_hash; > } > > 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 > 38f11c4..bae1882 100644 > --- a/lib/dp-packet.h > +++ b/lib/dp-packet.h > @@ -91,6 +91,9 @@ static inline void dp_packet_set_size(struct dp_packet *, > uint32_t); > static inline uint16_t dp_packet_get_allocated(const struct dp_packet *); > static inline void dp_packet_set_allocated(struct dp_packet *, > uint16_t); > > +static inline void > +dp_packet_copy_mbuf_flags(struct dp_packet *dst, const struct > +dp_packet *src); > + > void *dp_packet_resize_l2(struct dp_packet *, int increment); > void *dp_packet_resize_l2_5(struct dp_packet *, int increment); > static inline void *dp_packet_eth(const struct dp_packet *); @@ > -119,6 +122,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 *); @@ > -129,6 +135,10 @@ struct dp_packet *dp_packet_clone_data_with_headroom(const > void *, size_t, > size_t headroom); > static inline void dp_packet_delete(struct dp_packet *); > > +static inline void > +dp_packet_copy_common_members(struct dp_packet *new_b, > + const struct dp_packet *b); > + > static inline void *dp_packet_at(const struct dp_packet *, size_t offset, > size_t size); > static inline void *dp_packet_at_assert(const struct dp_packet *, @@ > -187,6 +197,17 @@ dp_packet_delete(struct dp_packet *b) > } > } > > +/* Copies the following fields into the 'new_b', which represent the > +common > + * fields between DPDK and non-DPDK packets: l2_pad_size, l2_5_ofs, > +l3_ofs, > + * l4_ofs, cutlen, packet_type and md. */ static inline void > +dp_packet_copy_common_members(struct dp_packet *new_b, > + const struct dp_packet *b) { > + memcpy(&new_b->l2_pad_size, &b->l2_pad_size, > + sizeof(struct dp_packet) - > + offsetof(struct dp_packet, l2_pad_size)); } > + > /* If 'b' contains at least 'offset + size' bytes of data, returns a > pointer to > * byte 'offset'. Otherwise, returns a null pointer. */ > static inline void * > @@ -680,6 +701,18 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t s) > { > b->mbuf.buf_len = s; > } > + > +static inline 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->ol_flags = buf_src->ol_flags; > + buf_dst->packet_type = buf_src->packet_type; > + buf_dst->tx_offload = buf_src->tx_offload; buf_src is not a pointer, should be 'buf_src.' instead of 'buf_src->' above for all members of the struct. > +} > #else > static inline bool > dp_packet_equal(const struct dp_packet *a, const struct dp_packet > *b) diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > f5044ca..d5e70d3 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -2364,6 +2364,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++; > } > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
