On 3/13/2019 10:09 AM, Ian Stokes wrote:
On 2/26/2019 10:38 AM, Ilya Maximets wrote:
1. No reason to have mbuf related APIs in a generic code.
2. Not only RSS/checksums should be invalidated in case of tunnel
decapsulation or sending to 'ring' ports.
In order to fix two above issues, new function
'dp_packet_reset_offload' introduced. In order to clean up/unify
the code and simplify addition of new offloading features to non-DPDK
version of dp_packet, introduced 'ol_flags' bitmask. Additionally
reduced code complexity in 'dp_packet_clone_with_headroom' by using
already existent generic APIs.
> Unfortunately, we still need to have a special case for mbuf
initialization inside 'dp_packet_init__()'.
'dp_packet_init_specific()' introduced for this purpose as a generic
API for initialization of the implementation-specific fields.
Thanks for this Ilya, apologies for the delay with the series, should
have more time to look at it this week.
One minor comment below, but other than that seems ok.
Acked-by: Flavio Leitner <f...@sysclose.org>
Signed-off-by: Ilya Maximets <i.maxim...@samsung.com>
---
lib/dp-packet.c | 15 ++++------
lib/dp-packet.h | 75 ++++++++++++++++++++---------------------------
lib/netdev-dpdk.c | 6 ++--
lib/netdev.c | 4 +--
4 files changed, 41 insertions(+), 59 deletions(-)
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 93b0e9c84..f8207ffc2 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -30,9 +30,10 @@ dp_packet_init__(struct dp_packet *b, size_t
allocated, enum dp_packet_source so
b->source = source;
dp_packet_reset_offsets(b);
pkt_metadata_init(&b->md, 0);
- dp_packet_rss_invalidate(b);
- dp_packet_mbuf_init(b);
dp_packet_reset_cutlen(b);
+ dp_packet_reset_offload(b);
+ /* Initialize implementation-specific fields of dp_packet. */
+ dp_packet_init_specific(b);
/* By default assume the packet type to be Ethernet. */
b->packet_type = htonl(PT_ETH);
}
@@ -173,16 +174,10 @@ 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;
-#else
- new_buffer->rss_hash_valid = buffer->rss_hash_valid;
#endif
- 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
+ if (dp_packet_rss_valid(buffer)) {
+ dp_packet_set_rss_hash(new_buffer,
dp_packet_get_rss_hash(buffer));
}
return new_buffer;
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index c6672f6be..b34dada78 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -46,6 +46,13 @@ enum OVS_PACKED_ENUM dp_packet_source {
#define DP_PACKET_CONTEXT_SIZE 64
+#ifndef DPDK_NETDEV
+/* Bit masks for the 'ol_flags' member of the 'dp_packet' structure. */
+enum dp_packet_offload_mask {
+ DP_PACKET_OL_RSS_HASH_MASK = 0x1, /* Is the 'rss_hash' valid? */
+};
Is there a specific reason you've used an enum instead of a define here
as its just a single value.
Is the expectation that the entries for the offload mask will be
expanded in the future to be more than just RSS_HASH_MASK?
Ah, please disregard, I've just spotted that it is expanded later in the
series.
I've applied this to master.
Thanks
Ian
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
