> > > >2016/10/28 17:47, Kavanagh, Mark B : >>> Currently, when doing packet copy, lots of DPDK mbuf's info >>> will be missed, like packet type, ol_flags, etc. >>> Those information is very important for DPDK to do >>> packets processing. >>> >>> Signed-off-by: Michael Qiu <[email protected]> >>> Signed-off-by: Jijiang Liu <[email protected]> >>> --- >>> lib/dp-packet.c | 3 +++ >>> lib/netdev-dpdk.c | 4 ++++ >>> 2 files changed, 7 insertions(+) >>> >>> diff --git a/lib/dp-packet.c b/lib/dp-packet.c >>> index bf8522e..619f651 100644 >>> --- a/lib/dp-packet.c >>> +++ b/lib/dp-packet.c >>> @@ -175,6 +175,9 @@ dp_packet_clone_with_headroom(const struct dp_packet >>> *buffer, size_t >>> headroom) >>> new_buffer->cutlen = buffer->cutlen; >>> #ifdef DPDK_NETDEV >>> new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags; >>> + new_buffer->mbuf.tx_offload = buffer->mbuf.tx_offload; >>> + new_buffer->mbuf.packet_type = buffer->mbuf.packet_type; >>> + new_buffer->mbuf.nb_segs = buffer->mbuf.nb_segs; >>> #else >>> new_buffer->rss_hash_valid = buffer->rss_hash_valid; >>> #endif >>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> This doesn't apply cleanly for me - apply change described below to resolve. > >Hi, Mark > >Sorry, my patchset is based on version tag 2.6, I will make V2 patch >which based on the latest code. > >>> index 27b4ee2..ad92504 100644 >>> --- a/lib/netdev-dpdk.c >>> +++ b/lib/netdev-dpdk.c >>> @@ -1589,6 +1589,10 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, >>> struct >dp_packet_batch >>> *batch) >>> memcpy(rte_pktmbuf_mtod(mbufs[newcnt], void *), >>> dp_packet_data(batch->packets[i]), size); >>> >>> + mbufs[newcnt]->nb_segs = batch->packets[i]->mbuf.nb_segs; >>> + mbufs[newcnt]->ol_flags = batch->packets[i]->mbuf.ol_flags; >>> + mbufs[newcnt]->packet_type = batch->packets[i]->mbuf.packet_type; >> I'm not sure if this change is needed; mbuf->packet_type is only useful on >> the NIC Rx path, >right? Please let me know if I've missed something. > >Yes, you are right. There are two reasons: > >First, it does no harm when we copy it in tx path, at least, I haven't >found any, if I'm wrong, show me please.
Hmm, I can't think of any examples off the top of my head, but I'd be worried that it could lead to inconsistencies between the packet_type copied from the batch mbuf and the actual headers present in the packet following processing by OvS. More on this below. > >Second, we could reuse this field to carry tunnel info(tun_type) when do >header push, thus in dpdk tx path, we could easily get the packet's >tunnel info like vxlan, IPIP, GRE, etc. and do the tunnel offloads settings. IMO this requires a number of changes which should be contained in their own separate patch(set); as part of that patchset, the packet_type could be copied from 'batch->packets[i]' into 'mbufs[newcnt]'. One caveat - how will 'packet_type' be populated for packets which don't originate from the NIC? Presumably, the additional processing required to set packet_type for such packets on the datapath would kill performance. > >> >>> + mbufs[newcnt]->tx_offload = batch->packets[i]->mbuf.tx_offload; >>> rte_pktmbuf_data_len(mbufs[newcnt]) = size; >>> rte_pktmbuf_pkt_len(mbufs[newcnt]) = size; >> The mbuf array is named 'pkts' as opposed to 'mbufs' in the latest upstream >> codebase; >renaming the array allows the patch to apply cleanly. > >OK, I will post the newest code to fix this. > >>> -- >>> 1.8.3.1 >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
