>-----Original Message-----
>From: Chandran, Sugesh
>Sent: Friday, December 8, 2017 6:01 PM
>To: Kavanagh, Mark B <[email protected]>; [email protected];
>[email protected]
>Cc: Stokes, Ian <[email protected]>; Loftus, Ciara
><[email protected]>; [email protected]
>Subject: RE: [ovs-dev][RFC PATCH V4 6/9] dp-packet: copy mbuf info for packet
>copy
>
>
>
>Regards
>_Sugesh
>
>
>> -----Original Message-----
>> From: Kavanagh, Mark B
>> Sent: Friday, December 8, 2017 12:02 PM
>> To: [email protected]; [email protected]
>> Cc: Stokes, Ian <[email protected]>; Loftus, Ciara
><[email protected]>;
>> [email protected]; Chandran, Sugesh
>> <[email protected]>; Kavanagh, Mark B
>> <[email protected]>
>> Subject: [ovs-dev][RFC PATCH V4 6/9] dp-packet: copy mbuf info for packet
>> copy
>>
>> From: Michael Qiu <[email protected]>
>>
>> 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.
>>
>> Co-authored-by: Mark Kavanagh <[email protected]>
>> [[email protected] rebased]
>>
>> Signed-off-by: Michael Qiu <[email protected]>
>> Signed-off-by: Mark Kavanagh <[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 ad71486..d628037 100644
>> --- a/lib/dp-packet.c
>> +++ b/lib/dp-packet.c
>> @@ -178,6 +178,9 @@ dp_packet_clone_with_headroom(const struct
>> dp_packet *buffer, size_t headroom)
>>
>>  #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;
>[Sugesh] This function get lot many #if, #else with DPDK. It must need a
>cleanup.
>What do you think?

During implementation, I had thought about separating out the code into two 
separate dp_packet_clone_headroom() functions: DPDK-based, and non-DPDK-based, 
within compiler guards. This would improve readability, but at the expense of 
repeated code; I'll do this in the next version, and see how it is received.
 
>Also will it would better if we keep all the mbuf field copy into a different
>function, something like
>copy_dpdk_mbuf_flags(..)?

Do you think that is still warranted if there are two separate 
dp_packet_clone_with_headroom() functions, as previously described?

Thanks,
Mark

>
>>  #else
>>      new_buffer->rss_hash_valid = buffer->rss_hash_valid;  #endif diff --git
>> a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 4167497..8a81690 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1866,6 +1866,10 @@ 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);
>> +        pkts[txcnt]->nb_segs = packet->mbuf.nb_segs;
>> +        pkts[txcnt]->ol_flags = packet->mbuf.ol_flags;
>> +        pkts[txcnt]->packet_type = packet->mbuf.packet_type;
>> +        pkts[txcnt]->tx_offload = packet->mbuf.tx_offload;
>>
>>          txcnt++;
>>      }
>> --
>> 1.9.3

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to