[ovs-dev] [PATCH v11 06/14] dp-packet: Handle multi-seg mbufs in helper funcs.

Stokes, Ian ian.stokes at intel.com
Mon Oct 22 14:30:10 UTC 2018


> Most helper functions in dp-packet assume that the data held by a
> dp_packet is contiguous, and perform operations such as pointer arithmetic
> under that assumption. However, with the introduction of multi-segment
> mbufs, where data is non-contiguous, such assumptions are no longer
> possible. Some examples of Such helper functions are dp_packet_tail(),
> dp_packet_tailroom(), dp_packet_end(),
> dp_packet_get_allocated() and dp_packet_at().
> 
> Thus, instead of assuming contiguous data in dp_packet, they  now iterate
> over the (non-contiguous) data in mbufs to perform their calculations.
> 
> Finally, dp_packet_use__() has also been modified to perform the
> initialisation of the packet (and setting the source) before continuing to
> set its size and data length, which now depends on the type of packet.
> 
> Co-authored-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
> 
> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
> Signed-off-by: Tiago Lam <tiago.lam at intel.com>
> Acked-by: Eelco Chaudron <echaudro at redhat.com>
> Acked-by: Flavio Leitner <fbl at sysclose.org>

Thanks for this Tiago,

It looks ok on inspection, will need a rebase however in the next revision.

Ian

> ---
>  lib/dp-packet.c |   4 +-
>  lib/dp-packet.h | 191
> +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 177 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 782e7c2..2aaeaae
> 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -41,11 +41,11 @@ static void
>  dp_packet_use__(struct dp_packet *b, void *base, size_t allocated,
>               enum dp_packet_source source)  {
> +    dp_packet_init__(b, allocated, source);
> +
>      dp_packet_set_base(b, base);
>      dp_packet_set_data(b, base);
>      dp_packet_set_size(b, 0);
> -
> -    dp_packet_init__(b, allocated, source);
>  }
> 
>  /* Initializes 'b' as an empty dp_packet that contains the 'allocated'
> bytes of diff --git a/lib/dp-packet.h b/lib/dp-packet.h index
> 223efe2..78c6f7f 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -133,6 +133,10 @@ static inline void *dp_packet_at(const struct
> dp_packet *, size_t offset,
>                                   size_t size);  static inline void
> *dp_packet_at_assert(const struct dp_packet *,
>                                          size_t offset, size_t size);
> +#ifdef DPDK_NETDEV
> +static inline const struct rte_mbuf *
> +dp_packet_mbuf_from_offset(const struct dp_packet *b, size_t *offset);
> +#endif
>  static inline void *dp_packet_tail(const struct dp_packet *);  static
> inline void *dp_packet_end(const struct dp_packet *);
> 
> @@ -185,9 +189,25 @@ dp_packet_delete(struct dp_packet *b)  static inline
> void *  dp_packet_at(const struct dp_packet *b, size_t offset, size_t
> size)  {
> -    return offset + size <= dp_packet_size(b)
> -           ? (char *) dp_packet_data(b) + offset
> -           : NULL;
> +    if (offset + size > dp_packet_size(b)) {
> +        return NULL;
> +    }
> +
> +#ifdef DPDK_NETDEV
> +    if (b->source == DPBUF_DPDK) {
> +        struct rte_mbuf *buf = CONST_CAST(struct rte_mbuf *, &b->mbuf);
> +
> +        while (buf && offset > buf->data_len) {
> +            offset -= buf->data_len;
> +
> +            buf = buf->next;
> +        }
> +
> +        return buf ? rte_pktmbuf_mtod_offset(buf, char *, offset) : NULL;
> +    }
> +#endif
> +
> +    return (char *) dp_packet_data(b) + offset;
>  }
> 
>  /* Returns a pointer to byte 'offset' in 'b', which must contain at least
> @@ -196,13 +216,23 @@ static inline void *  dp_packet_at_assert(const
> struct dp_packet *b, size_t offset, size_t size)  {
>      ovs_assert(offset + size <= dp_packet_size(b));
> -    return ((char *) dp_packet_data(b)) + offset;
> +    return dp_packet_at(b, offset, size);
>  }
> 
>  /* Returns a pointer to byte following the last byte of data in use in
> 'b'. */  static inline void *  dp_packet_tail(const struct dp_packet *b)
> {
> +#ifdef DPDK_NETDEV
> +    if (b->source == DPBUF_DPDK) {
> +        struct rte_mbuf *buf = CONST_CAST(struct rte_mbuf *, &b->mbuf);
> +        /* Find last segment where data ends, meaning the tail of the
> chained
> +         *  mbufs must be there */
> +        buf = rte_pktmbuf_lastseg(buf);
> +
> +        return rte_pktmbuf_mtod_offset(buf, void *, buf->data_len);
> +    }
> +#endif
>      return (char *) dp_packet_data(b) + dp_packet_size(b);  }
> 
> @@ -211,6 +241,15 @@ dp_packet_tail(const struct dp_packet *b)  static
> inline void *  dp_packet_end(const struct dp_packet *b)  {
> +#ifdef DPDK_NETDEV
> +    if (b->source == DPBUF_DPDK) {
> +        struct rte_mbuf *buf = CONST_CAST(struct rte_mbuf *,
> +&(b->mbuf));
> +
> +        buf = rte_pktmbuf_lastseg(buf);
> +
> +        return (char *) buf->buf_addr + buf->buf_len;
> +    }
> +#endif
>      return (char *) dp_packet_base(b) + dp_packet_get_allocated(b);  }
> 
> @@ -236,6 +275,15 @@ dp_packet_tailroom(const struct dp_packet *b)  static
> inline void  dp_packet_clear(struct dp_packet *b)  {
> +#ifdef DPDK_NETDEV
> +    if (b->source == DPBUF_DPDK) {
> +        /* sets pkt_len and data_len to zero and frees unused mbufs */
> +        dp_packet_set_size(b, 0);
> +        rte_pktmbuf_reset(&b->mbuf);
> +
> +        return;
> +    }
> +#endif
>      dp_packet_set_data(b, dp_packet_base(b));
>      dp_packet_set_size(b, 0);
>  }
> @@ -248,28 +296,47 @@ dp_packet_pull(struct dp_packet *b, size_t size)
>      void *data = dp_packet_data(b);
>      ovs_assert(dp_packet_size(b) - dp_packet_l2_pad_size(b) >= size);
>      dp_packet_set_data(b, (char *) dp_packet_data(b) + size);
> -    dp_packet_set_size(b, dp_packet_size(b) - size);
> +#ifdef DPDK_NETDEV
> +    b->mbuf.pkt_len -= size;
> +#else
> +    b->size_ -= size;
> +#endif
> +
>      return data;
>  }
> 
> +#ifdef DPDK_NETDEV
> +/* Similar to dp_packet_try_pull() but doesn't actually pull any data,
> +only
> + * checks if it could and returns true or false accordingly.
> + *
> + * Valid for dp_packets carrying mbufs only. */ static inline bool
> +dp_packet_mbuf_may_pull(const struct dp_packet *b, size_t size) {
> +    if (size > b->mbuf.data_len) {
> +        return false;
> +    }
> +
> +    return true;
> +}
> +#endif
> +
>  /* If 'b' has at least 'size' bytes of data, removes that many bytes from
> the
>   * head end of 'b' and returns the first byte removed.  Otherwise,
> returns a
>   * null pointer without modifying 'b'. */  static inline void *
> dp_packet_try_pull(struct dp_packet *b, size_t size)  {
> +#ifdef DPDK_NETDEV
> +    if (!dp_packet_mbuf_may_pull(b, size)) {
> +        return NULL;
> +    }
> +#endif
> +
>      return dp_packet_size(b) - dp_packet_l2_pad_size(b) >= size
>          ? dp_packet_pull(b, size) : NULL;  }
> 
>  static inline bool
> -dp_packet_equal(const struct dp_packet *a, const struct dp_packet *b) -{
> -    return dp_packet_size(a) == dp_packet_size(b) &&
> -           !memcmp(dp_packet_data(a), dp_packet_data(b),
> dp_packet_size(a));
> -}
> -
> -static inline bool
>  dp_packet_is_eth(const struct dp_packet *b)  {
>      return b->packet_type == htonl(PT_ETH); @@ -311,6 +378,12 @@
> dp_packet_set_l2_pad_size(struct dp_packet *b, uint8_t pad_size)  static
> inline void *  dp_packet_l2_5(const struct dp_packet *b)  {
> +#ifdef DPDK_NETDEV
> +    if (!dp_packet_mbuf_may_pull(b, b->l2_5_ofs)) {
> +        return NULL;
> +    }
> +#endif
> +
>      return b->l2_5_ofs != UINT16_MAX
>             ? (char *) dp_packet_data(b) + b->l2_5_ofs
>             : NULL;
> @@ -327,6 +400,12 @@ dp_packet_set_l2_5(struct dp_packet *b, void *l2_5)
> static inline void *  dp_packet_l3(const struct dp_packet *b)  {
> +#ifdef DPDK_NETDEV
> +    if (!dp_packet_mbuf_may_pull(b, b->l3_ofs)) {
> +        return NULL;
> +    }
> +#endif
> +
>      return b->l3_ofs != UINT16_MAX
>             ? (char *) dp_packet_data(b) + b->l3_ofs
>             : NULL;
> @@ -341,6 +420,12 @@ dp_packet_set_l3(struct dp_packet *b, void *l3)
> static inline void *  dp_packet_l4(const struct dp_packet *b)  {
> +#ifdef DPDK_NETDEV
> +    if (!dp_packet_mbuf_may_pull(b, b->l4_ofs)) {
> +        return NULL;
> +    }
> +#endif
> +
>      return b->l4_ofs != UINT16_MAX
>             ? (char *) dp_packet_data(b) + b->l4_ofs
>             : NULL;
> @@ -355,6 +440,27 @@ dp_packet_set_l4(struct dp_packet *b, void *l4)
> static inline size_t  dp_packet_l4_size(const struct dp_packet *b)  {
> +#ifdef DPDK_NETDEV
> +    if (b->source == DPBUF_DPDK) {
> +        if (!dp_packet_mbuf_may_pull(b, b->l4_ofs)) {
> +            return 0;
> +        }
> +
> +        struct rte_mbuf *mbuf = CONST_CAST(struct rte_mbuf *, &b->mbuf);
> +        size_t l4_size = mbuf->data_len - b->l4_ofs;
> +
> +        mbuf = mbuf->next;
> +        while (mbuf) {
> +            l4_size += mbuf->data_len;
> +
> +            mbuf = mbuf->next;
> +        }
> +
> +        l4_size -= dp_packet_l2_pad_size(b);
> +
> +        return l4_size;
> +    }
> +#endif
>      return b->l4_ofs != UINT16_MAX
>          ? (const char *)dp_packet_tail(b) - (const char *)dp_packet_l4(b)
>          - dp_packet_l2_pad_size(b)
> @@ -408,6 +514,53 @@ dp_packet_get_nd_payload(const struct dp_packet *b)
> #ifdef DPDK_NETDEV  BUILD_ASSERT_DECL(offsetof(struct dp_packet, mbuf) ==
> 0);
> 
> +static inline const struct rte_mbuf *
> +dp_packet_mbuf_from_offset(const struct dp_packet *b, size_t *offset) {
> +    const struct rte_mbuf *mbuf = &b->mbuf;
> +    while (mbuf && *offset >= mbuf->data_len) {
> +        *offset -= mbuf->data_len;
> +
> +        mbuf = mbuf->next;
> +    }
> +
> +    return mbuf;
> +}
> +
> +static inline bool
> +dp_packet_equal(const struct dp_packet *a, const struct dp_packet *b) {
> +    if (dp_packet_size(a) != dp_packet_size(b)) {
> +        return false;
> +    }
> +
> +    const struct rte_mbuf *m_a, *m_b;
> +    size_t abs_off_a = 0;
> +    size_t abs_off_b = 0;
> +    size_t len = 0;
> +    while (m_a != NULL && m_b != NULL) {
> +        size_t rel_off_a = abs_off_a;
> +        size_t rel_off_b = abs_off_b;
> +        m_a = dp_packet_mbuf_from_offset(a, &rel_off_a);
> +        m_b = dp_packet_mbuf_from_offset(b, &rel_off_b);
> +        if (!m_a || !m_b) {
> +            break;
> +        }
> +
> +        len = MIN(m_a->data_len - rel_off_a, m_b->data_len -
> + rel_off_b);
> +
> +        if (memcmp(rte_pktmbuf_mtod_offset(m_a, char *, rel_off_a),
> +                   rte_pktmbuf_mtod_offset(m_b, char *, rel_off_b),
> +                   len)) {
> +            return false;
> +        }
> +
> +        abs_off_a += len;
> +        abs_off_b += len;
> +    }
> +
> +    return (!m_a && !m_b) ? true : false; }
> +
>  static inline void *
>  dp_packet_base(const struct dp_packet *b)  { @@ -516,7 +669,7 @@
> __packet_set_data(struct dp_packet *b, uint16_t v)  static inline uint16_t
> dp_packet_get_allocated(const struct dp_packet *b)  {
> -    return b->mbuf.buf_len;
> +    return b->mbuf.nb_segs * b->mbuf.buf_len;
>  }
> 
>  static inline void
> @@ -525,6 +678,13 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t
> s)
>      b->mbuf.buf_len = s;
>  }
>  #else
> +static inline bool
> +dp_packet_equal(const struct dp_packet *a, const struct dp_packet *b) {
> +    return dp_packet_size(a) == dp_packet_size(b) &&
> +           !memcmp(dp_packet_data(a), dp_packet_data(b),
> +dp_packet_size(a)); }
> +
>  static inline void *
>  dp_packet_base(const struct dp_packet *b)  { @@ -626,10 +786,9 @@
> dp_packet_set_data(struct dp_packet *b, void *data)  }
> 
>  static inline void
> -dp_packet_reset_packet(struct dp_packet *b, int off)
> +dp_packet_reset_packet(struct dp_packet *b, size_t off)
>  {
> -    dp_packet_set_size(b, dp_packet_size(b) - off);
> -    dp_packet_set_data(b, ((unsigned char *) dp_packet_data(b) + off));
> +    dp_packet_try_pull(b, off);
>      dp_packet_reset_offsets(b);
>  }
> 
> --
> 2.7.4



More information about the dev mailing list