diff mbox

[ovs-dev,2/5] lib/dp-packet: copy additional packet info when do packet copy

Message ID 1497850143-3116-3-git-send-email-qdy220091330@gmail.com
State Deferred
Headers show

Commit Message

Michael Qiu June 19, 2017, 5:29 a.m. UTC
From: Michael Qiu <qiudayu@chinac.com>

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 <qiudayu@chinac.com>
---
 lib/dp-packet.c   | 3 +++
 lib/netdev-dpdk.c | 4 ++++
 2 files changed, 7 insertions(+)

Comments

Mark Kavanagh June 26, 2017, 3:07 p.m. UTC | #1
>From: Michael Qiu [mailto:qdy220091330@gmail.com]
>Sent: Monday, June 19, 2017 6:29 AM
>To: dev@openvswitch.org
>Cc: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; blp@ovn.org; dball@vmware.com; Michael Qiu
><qiudayu@chinac.com>
>Subject: [PATCH 2/5] lib/dp-packet: copy additional packet info when do packet copy
>
>From: Michael Qiu <qiudayu@chinac.com>
>
>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 <qiudayu@chinac.com>
>---
> 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 ee2c449..9f872a1 100644
>--- a/lib/dp-packet.c
>+++ b/lib/dp-packet.c
>@@ -179,6 +179,9 @@ dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t
>headroom)
>     new_buffer->packet_type = buffer->packet_type;
> #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;

Hi Michael,

In what use case would this information be useful?

Mirroring, perhaps?


> #else
>     new_buffer->rss_hash_valid = buffer->rss_hash_valid;
> #endif
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index bba4de3..0485872 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -1804,6 +1804,10 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch
>*batch)
>         memcpy(rte_pktmbuf_mtod(pkts[newcnt], void *),
>                dp_packet_data(batch->packets[i]), size);
>
>+        pkts[newcnt]->nb_segs = batch->packets[i]->mbuf.nb_segs;
>+        pkts[newcnt]->ol_flags = batch->packets[i]->mbuf.ol_flags;
>+        pkts[newcnt]->packet_type = batch->packets[i]->mbuf.packet_type;
>+        pkts[newcnt]->tx_offload = batch->packets[i]->mbuf.tx_offload;

Could you give an example of when it would be useful to copy info from mbufs which originate from non-DPDK source?
AFIA, non-DPDK sources don't currently update these particular fields - if I've missed something though, please let me know.

>         rte_pktmbuf_data_len(pkts[newcnt]) = size;
>         rte_pktmbuf_pkt_len(pkts[newcnt]) = size;
>
>--
>1.8.3.1
仇大玉 June 27, 2017, 2:30 a.m. UTC | #2
在 2017/6/26 23:07, Kavanagh, Mark B 写道:
>> From: Michael Qiu [mailto:qdy220091330@gmail.com]
>> Sent: Monday, June 19, 2017 6:29 AM
>> To: dev@openvswitch.org
>> Cc: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; blp@ovn.org; dball@vmware.com; Michael Qiu
>> <qiudayu@chinac.com>
>> Subject: [PATCH 2/5] lib/dp-packet: copy additional packet info when do packet copy
>>
>> From: Michael Qiu <qiudayu@chinac.com>
>>
>> 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 <qiudayu@chinac.com>
>> ---
>> 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 ee2c449..9f872a1 100644
>> --- a/lib/dp-packet.c
>> +++ b/lib/dp-packet.c
>> @@ -179,6 +179,9 @@ dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t
>> headroom)
>>      new_buffer->packet_type = buffer->packet_type;
>> #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;
> Hi Michael,
>
> In what use case would this information be useful?
>
> Mirroring, perhaps?

When doing offloading, we need tx_offload flag(although upstream does 
not support yet), also when doing packet copy, we need  nb_segs field.

>
>> #else
>>      new_buffer->rss_hash_valid = buffer->rss_hash_valid;
>> #endif
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index bba4de3..0485872 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1804,6 +1804,10 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch
>> *batch)
>>          memcpy(rte_pktmbuf_mtod(pkts[newcnt], void *),
>>                 dp_packet_data(batch->packets[i]), size);
>>
>> +        pkts[newcnt]->nb_segs = batch->packets[i]->mbuf.nb_segs;
>> +        pkts[newcnt]->ol_flags = batch->packets[i]->mbuf.ol_flags;
>> +        pkts[newcnt]->packet_type = batch->packets[i]->mbuf.packet_type;
>> +        pkts[newcnt]->tx_offload = batch->packets[i]->mbuf.tx_offload;
> Could you give an example of when it would be useful to copy info from mbufs which originate from non-DPDK source?
> AFIA, non-DPDK sources don't currently update these particular fields - if I've missed something though, please let me know.

What happens when a packet from DPDK be copied twice?
DPDK(VM virtio)--(flooding)->non-DPDK(patch port)--(copy)->DPDK(NIC)?


>
>>          rte_pktmbuf_data_len(pkts[newcnt]) = size;
>>          rte_pktmbuf_pkt_len(pkts[newcnt]) = size;
>>
>> --
>> 1.8.3.1
>
Mark Kavanagh June 30, 2017, 3:03 p.m. UTC | #3
>-----Original Message-----

>From: 仇大玉 [mailto:qiudayu@chinac.com]

>Sent: Tuesday, June 27, 2017 3:31 AM

>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Michael Qiu

><qdy220091330@gmail.com>; dev@openvswitch.org

>Cc: blp@ovn.org; dball@vmware.com

>Subject: Re: [PATCH 2/5] lib/dp-packet: copy additional packet info when do

>packet copy

>

>

>

>在 2017/6/26 23:07, Kavanagh, Mark B 写道:

>>> From: Michael Qiu [mailto:qdy220091330@gmail.com]

>>> Sent: Monday, June 19, 2017 6:29 AM

>>> To: dev@openvswitch.org

>>> Cc: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; blp@ovn.org;

>dball@vmware.com; Michael Qiu

>>> <qiudayu@chinac.com>

>>> Subject: [PATCH 2/5] lib/dp-packet: copy additional packet info when do

>packet copy

>>>

>>> From: Michael Qiu <qiudayu@chinac.com>

>>>

>>> 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 <qiudayu@chinac.com>

>>> ---

>>> 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 ee2c449..9f872a1 100644

>>> --- a/lib/dp-packet.c

>>> +++ b/lib/dp-packet.c

>>> @@ -179,6 +179,9 @@ dp_packet_clone_with_headroom(const struct dp_packet

>*buffer, size_t

>>> headroom)

>>>      new_buffer->packet_type = buffer->packet_type;

>>> #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;

>> Hi Michael,

>>

>> In what use case would this information be useful?

>>

>> Mirroring, perhaps?

>

>When doing offloading, we need tx_offload flag(although upstream does

>not support yet), also when doing packet copy, we need  nb_segs field.


Yes, I understand that; I guess what I was really asking is 'what is the series of actions for which this information would be useful?'.

Looking at the code, my guess is that the info is relevant when a packet (which is marked for some form of offload) is cloned (odp_execute_clone), and that cloned packet is sent out to a phy port (via odp_execute_actions).
	
>

>>

>>> #else

>>>      new_buffer->rss_hash_valid = buffer->rss_hash_valid;

>>> #endif

>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c

>>> index bba4de3..0485872 100644

>>> --- a/lib/netdev-dpdk.c

>>> +++ b/lib/netdev-dpdk.c

>>> @@ -1804,6 +1804,10 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid,

>struct dp_packet_batch

>>> *batch)

>>>          memcpy(rte_pktmbuf_mtod(pkts[newcnt], void *),

>>>                 dp_packet_data(batch->packets[i]), size);

>>>

>>> +        pkts[newcnt]->nb_segs = batch->packets[i]->mbuf.nb_segs;

>>> +        pkts[newcnt]->ol_flags = batch->packets[i]->mbuf.ol_flags;

>>> +        pkts[newcnt]->packet_type = batch->packets[i]->mbuf.packet_type;

>>> +        pkts[newcnt]->tx_offload = batch->packets[i]->mbuf.tx_offload;

>> Could you give an example of when it would be useful to copy info from mbufs

>which originate from non-DPDK source?

>> AFIA, non-DPDK sources don't currently update these particular fields - if

>I've missed something though, please let me know.

>

>What happens when a packet from DPDK be copied twice?

>DPDK(VM virtio)--(flooding)->non-DPDK(patch port)--(copy)->DPDK(NIC)?


Perfect - that makes sense.

>

>

>>

>>>          rte_pktmbuf_data_len(pkts[newcnt]) = size;

>>>          rte_pktmbuf_pkt_len(pkts[newcnt]) = size;

>>>

>>> --

>>> 1.8.3.1

>>
diff mbox

Patch

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index ee2c449..9f872a1 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -179,6 +179,9 @@  dp_packet_clone_with_headroom(const struct dp_packet *buffer, size_t headroom)
     new_buffer->packet_type = buffer->packet_type;
 #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
index bba4de3..0485872 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1804,6 +1804,10 @@  dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch *batch)
         memcpy(rte_pktmbuf_mtod(pkts[newcnt], void *),
                dp_packet_data(batch->packets[i]), size);
 
+        pkts[newcnt]->nb_segs = batch->packets[i]->mbuf.nb_segs;
+        pkts[newcnt]->ol_flags = batch->packets[i]->mbuf.ol_flags;
+        pkts[newcnt]->packet_type = batch->packets[i]->mbuf.packet_type;
+        pkts[newcnt]->tx_offload = batch->packets[i]->mbuf.tx_offload;
         rte_pktmbuf_data_len(pkts[newcnt]) = size;
         rte_pktmbuf_pkt_len(pkts[newcnt]) = size;