On 9/6/17, 5:12 AM, "[email protected] on behalf of Zoltán
Balogh" <[email protected] on behalf of
[email protected]> wrote:
DPDK uses dp-packet pool for storing received packets. The pool is
reused by rxq_recv funcions of the DPDK netdevs. The datapath is
capable to modify the packet_type property of packets. For instance
when encapsulated L3 packets are received on a ptap gre port.
In this case the packet_type property of struct dp_packet can be
modified and later the same dp_packet with the modified packet_type
can be reused in the rxq_rec function, so it can contain corrupted
data.
The dp_packet_batch_init_cutlen() in the rxq_recv functions iterates
over dp_packets and sets their cutlen. So I modified this function
to set packet_type to Ethernet for the dp_packets as well. I also
renamed this function because of the added functionality.
The dp_packet_batch_init_cutlen() iterates over batch->count dp_packet.
Therefore setting of batch->count = nb_rx needs to be done before the
former function is invoked. This is an additional fix.
Signed-off-by: Zoltán Balogh <[email protected]>
Signed-off-by: László Sürü <[email protected]>
Co-authored-by: László Sürü <[email protected]>
CC: Jan Scheurich <[email protected]>
CC: Sugesh Chandran <[email protected]>
CC: Darrell Ball <[email protected]>
---
lib/dp-packet.h | 4 +++-
lib/netdev-dpdk.c | 5 +++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 046f3ab..0046d0a 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -805,12 +805,14 @@ dp_packet_delete_batch(struct dp_packet_batch *batch,
bool may_steal)
}
static inline void
-dp_packet_batch_init_cutlen(struct dp_packet_batch *batch)
+dp_packet_batch_init_transient_fields(struct dp_packet_batch *batch)
[Darrell]
Can we just call this function dp_packet_batch_init_packet_fields() now that it
has wider scope than just cutlen,
as the name makes it clear that we are initing the individual packet fields of
a batch, rather than the batch context fields ?
‘transient’ is maybe implied ?
{
struct dp_packet *packet;
DP_PACKET_BATCH_FOR_EACH (packet, batch) {
dp_packet_reset_cutlen(packet);
+ /* Set packet_type to Ethernet. */
+ packet->packet_type = PACKET_TYPE_BE(OFPHTN_ONF, 0x0000);
[Darrell] The original dp_packet init code for PTAP has
b->packet_type = htonl(PT_ETH);
This form seems preferred and intuitive (comment would even be unnecessary);
can we use this form ?
Also, the original dp_packet init code for PTAP was in dp_packet_init__ and
then moved to dp_packet_init_dpdk
I think we can just remove it from there, as it is redundant with the new code.
}
}
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index f58e9be..2422741 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1644,8 +1644,9 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
nb_rx, dropped);
rte_spinlock_unlock(&dev->stats_lock);
- dp_packet_batch_init_cutlen(batch);
batch->count = (int) nb_rx;
[Darrell] Although nothing to do with this patch, can we remove the cast
‘(int)’ above ?
+ dp_packet_batch_init_transient_fields(batch);
[Darrell]
Good catch; this should work better than initializing 0 of the entries (
+
return 0;
}
@@ -1684,8 +1685,8 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq, struct
dp_packet_batch *batch)
rte_spinlock_unlock(&dev->stats_lock);
}
- dp_packet_batch_init_cutlen(batch);
batch->count = nb_rx;
+ dp_packet_batch_init_transient_fields(batch);
[Darrell]
Same comment as above.
return 0;
}
--
1.9.1
_______________________________________________
dev mailing list
[email protected]
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIFAw&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=zcjvUT1lz85oUJWStkTlRzIdXuQnJ4HL-n6oiK99iuM&s=KmEO2izD43oNVigXXu6TlPCLNruTmVtKMGdxVb6ANYs&e=
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
