[ovs-dev] [PATCH v4 3/9] dp-packet: Refactor offloading API.
Asaf Penso
asafp at mellanox.com
Sun Feb 17 13:37:32 UTC 2019
Regards,
Asaf Penso
> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org <ovs-dev-
> bounces at openvswitch.org> On Behalf Of Ilya Maximets
> Sent: Friday, February 15, 2019 3:07 PM
> To: ovs-dev at openvswitch.org; Ian Stokes <ian.stokes at intel.com>
> Cc: Roni Bar Yanai <roniba at mellanox.com>; Flavio Leitner
> <fbl at sysclose.org>; Ilya Maximets <i.maximets at samsung.com>
> Subject: [ovs-dev] [PATCH v4 3/9] dp-packet: Refactor offloading API.
>
> 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_offload_invalidate' 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__()'.
>
> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
> ---
> lib/dp-packet.c | 14 ++++-------
> lib/dp-packet.h | 62 ++++++++++++++---------------------------------
> lib/netdev-dpdk.c | 6 ++---
> lib/netdev.c | 4 +--
> 4 files changed, 28 insertions(+), 58 deletions(-)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 93b0e9c84..b5942f815
> 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -30,8 +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);
> +#ifdef DPDK_NETDEV
> dp_packet_mbuf_init(b);
> +#endif
> + dp_packet_offload_invalidate(b);
> dp_packet_reset_cutlen(b);
> /* By default assume the packet type to be Ethernet. */
> b->packet_type = htonl(PT_ETH);
> @@ -173,16 +175,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..685edc90c
> 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -58,8 +58,9 @@ struct dp_packet {
> uint16_t allocated_; /* Number of bytes allocated. */
> uint16_t data_ofs; /* First byte actually in use. */
> uint32_t size_; /* Number of bytes in use. */
> + uint32_t ol_flags; /* Offloading flags. */
I think we can consider even now to use 64bit of flags.
> +#define DP_PACKET_OL_RSS_HASH_MASK 0x1 /* Is the 'rss_hash' valid?
> */
> uint32_t rss_hash; /* Packet hash. */
> - bool rss_hash_valid; /* Is the 'rss_hash' valid? */
> #endif
> enum dp_packet_source source; /* Source of memory allocated as 'base'.
> */
>
> @@ -421,6 +422,16 @@ dp_packet_get_nd_payload(const struct dp_packet
> *b) #ifdef DPDK_NETDEV BUILD_ASSERT_DECL(offsetof(struct dp_packet,
> mbuf) == 0);
>
> +/* This initialization is needed for packets that do not come from DPDK
> + * interfaces, when vswitchd is built with --with-dpdk. */ static
> +inline void dp_packet_mbuf_init(struct dp_packet *p) {
> + p->mbuf.tx_offload = p->mbuf.packet_type = 0;
> + p->mbuf.nb_segs = 1;
> + p->mbuf.next = NULL;
> +}
> +
> static inline void *
> dp_packet_base(const struct dp_packet *b) { @@ -501,24 +512,9 @@
> dp_packet_rss_valid(const struct dp_packet *p) }
>
> static inline void
> -dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED) -{ -}
> -
> -static inline void
> -dp_packet_mbuf_rss_flag_reset(struct dp_packet *p) -{
> - p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
> -}
> -
> -/* This initialization is needed for packets that do not come from DPDK
> - * interfaces, when vswitchd is built with --with-dpdk. */ -static inline void -
> dp_packet_mbuf_init(struct dp_packet *p)
> +dp_packet_offload_invalidate(struct dp_packet *p OVS_UNUSED)
> {
> - p->mbuf.ol_flags = p->mbuf.tx_offload = p->mbuf.packet_type = 0;
> - p->mbuf.nb_segs = 1;
> - p->mbuf.next = NULL;
> + p->mbuf.ol_flags = 0;
> }
>
> static inline bool
> @@ -549,13 +545,6 @@ dp_packet_l4_checksum_bad(const struct
> dp_packet *p)
> PKT_RX_L4_CKSUM_BAD;
> }
>
> -static inline void
> -reset_dp_packet_checksum_ol_flags(struct dp_packet *p) -{
> - p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD |
> PKT_RX_L4_CKSUM_BAD |
> - PKT_RX_IP_CKSUM_GOOD | PKT_RX_IP_CKSUM_BAD);
> -}
> -
> static inline bool
> dp_packet_has_flow_mark(const struct dp_packet *p, uint32_t *mark) {
> @@ -628,29 +617,19 @@ static inline void dp_packet_set_rss_hash(struct
> dp_packet *p, uint32_t hash) {
> p->rss_hash = hash;
> - p->rss_hash_valid = true;
> + p->ol_flags |= DP_PACKET_OL_RSS_HASH_MASK;
> }
>
> static inline bool
> dp_packet_rss_valid(const struct dp_packet *p) {
> - return p->rss_hash_valid;
> + return p->ol_flags & DP_PACKET_OL_RSS_HASH_MASK;
> }
>
> static inline void
> -dp_packet_rss_invalidate(struct dp_packet *p) -{
> - p->rss_hash_valid = false;
> -}
> -
> -static inline void
> -dp_packet_mbuf_rss_flag_reset(struct dp_packet *p OVS_UNUSED) -{ -}
> -
> -static inline void
> -dp_packet_mbuf_init(struct dp_packet *p OVS_UNUSED)
> +dp_packet_offload_invalidate(struct dp_packet *p)
> {
> + p->ol_flags = 0;
> }
>
> static inline bool
> @@ -677,11 +656,6 @@ dp_packet_l4_checksum_bad(const struct
> dp_packet *p OVS_UNUSED)
> return false;
> }
>
> -static inline void
> -reset_dp_packet_checksum_ol_flags(struct dp_packet *p OVS_UNUSED) -{
> -}
> -
> static inline bool
> dp_packet_has_flow_mark(const struct dp_packet *p OVS_UNUSED,
> uint32_t *mark OVS_UNUSED) diff --git a/lib/netdev-dpdk.c
> b/lib/netdev-dpdk.c index f07b10c88..26022a59c 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -3774,11 +3774,11 @@ netdev_dpdk_ring_send(struct netdev *netdev,
> int qid,
> struct dp_packet *packet;
>
> /* When using 'dpdkr' and sending to a DPDK ring, we want to ensure that
> - * the rss hash field is clear. This is because the same mbuf may be
> + * the offload fields are clear. This is because the same mbuf may
> + be
> * modified by the consumer of the ring and return into the datapath
> - * without recalculating the RSS hash. */
> + * without recalculating the RSS hash or revalidating the
> + checksums. */
> DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
> - dp_packet_mbuf_rss_flag_reset(packet);
> + dp_packet_offload_invalidate(packet);
> }
>
> netdev_dpdk_send__(dev, qid, batch, concurrent_txq); diff --git
> a/lib/netdev.c b/lib/netdev.c index 45b50f26c..4f26515dc 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -814,10 +814,10 @@ netdev_pop_header(struct netdev *netdev, struct
> dp_packet_batch *batch)
> DP_PACKET_BATCH_REFILL_FOR_EACH (i, size, packet, batch) {
> packet = netdev->netdev_class->pop_header(packet);
> if (packet) {
> - /* Reset the checksum offload flags if present, to avoid wrong
> + /* Reset the offload flags if present, to avoid wrong
> * interpretation in the further packet processing when
> * recirculated.*/
> - reset_dp_packet_checksum_ol_flags(packet);
> + dp_packet_offload_invalidate(packet);
> dp_packet_batch_refill(batch, packet, i);
> }
> }
> --
> 2.17.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fma
> il.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-
> dev&data=02%7C01%7Casafp%40mellanox.com%7Ce1f7df77aa754e663
> c4708d69346d135%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63
> 6858329718367951&sdata=LimdEf4hzLJxXAe%2BacTi9PFD5Udu1og5IuA
> DJVoAxkk%3D&reserved=0
More information about the dev
mailing list