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_NETDEVnew_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?
Ian _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
