> 
> 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.
> 
> Co-authored-by: Mark Kavanagh <mark.b.kavan...@intel.com>
> 
> Signed-off-by: Mark Kavanagh <mark.b.kavan...@intel.com>
> Signed-off-by: Tiago Lam <tiago....@intel.com>
> ---
>  lib/dp-packet.h | 252
> +++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 205 insertions(+), 47 deletions(-)
> 
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index e79fb24..4d4b420 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -80,6 +80,11 @@ struct dp_packet {
>      };
>  };
> 
> +#ifdef DPDK_NETDEV
> +#define MBUF_BUF_END(BUF_ADDR, BUF_LEN) \
> +    (char *) (((char *) BUF_ADDR) + BUF_LEN)
> +#endif
> +
>  static inline void *dp_packet_data(const struct dp_packet *);
>  static inline void dp_packet_set_data(struct dp_packet *, void *);
>  static inline void *dp_packet_base(const struct dp_packet *);
> @@ -133,6 +138,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 void * dp_packet_at_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 *);
> 
> @@ -180,40 +189,6 @@ dp_packet_delete(struct dp_packet *b)
>      }
>  }
> 
> -/* If 'b' contains at least 'offset + size' bytes of data, returns a pointer 
> to
> - * byte 'offset'.  Otherwise, returns a null pointer. */
> -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;
> -}
> -
> -/* Returns a pointer to byte 'offset' in 'b', which must contain at least
> - * 'offset + size' bytes of data. */
> -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;
> -}
> -
> -/* 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)
> -{
> -    return (char *) dp_packet_data(b) + dp_packet_size(b);
> -}
> -
> -/* Returns a pointer to byte following the last byte allocated for use (but
> - * not necessarily in use) in 'b'. */
> -static inline void *
> -dp_packet_end(const struct dp_packet *b)
> -{
> -    return (char *) dp_packet_base(b) + dp_packet_get_allocated(b);
> -}
> -
>  /* Returns the number of bytes of headroom in 'b', that is, the number of
> bytes
>   * of unused space in dp_packet 'b' before the data that is in use.  (Most
>   * commonly, the data in a dp_packet is at its beginning, and thus the
> @@ -224,19 +199,63 @@ dp_packet_headroom(const struct dp_packet *b)
>      return (char *) dp_packet_data(b) - (char *) dp_packet_base(b);
>  }
> 
> +#ifdef DPDK_NETDEV
> +static inline struct rte_mbuf *
> +dp_packet_mbuf_tail(const struct dp_packet *b)
> +{
> +    struct rte_mbuf *buf = CONST_CAST(struct rte_mbuf *, &b->mbuf);
> +    if (b->source == DPBUF_DPDK) {
> +
> +        /* Find last segment where data ends, meaning the tail of the chained
> +           mbufs is there */
> +        struct rte_mbuf *pmbuf;
> +        while (buf) {
> +            if (MBUF_BUF_END(buf->buf_addr, buf->buf_len) !=
> +                rte_pktmbuf_mtod_offset(buf, char *, buf->data_len)) {
> +                break;
> +            }
> +
> +            pmbuf = buf;
> +            buf = buf->next;
> +        }
> +
> +        return (buf == NULL) ? pmbuf : buf;
> +    } else {
> +        return buf;
> +    }
> +}
> +#endif

dp_packet_tail() seems to perform the exact same function. Can these functions 
be consolidated into one?

Am I right to assume that the tail will not necessarily be the last segment? 
Otherwise would rte_pktmbuf_lastseg() 
(http://dpdk.org/doc/api/rte__mbuf_8h.html#a0c2d001cd113362dd123d663210afcbb) 
be useful here?

> +
>  /* Returns the number of bytes that may be appended to the tail end of
>   * dp_packet 'b' before the dp_packet must be reallocated. */
>  static inline size_t
>  dp_packet_tailroom(const struct dp_packet *b)
>  {
> +#ifdef DPDK_NETDEV
> +    struct rte_mbuf *mbuf = dp_packet_mbuf_tail(b);
> +
> +    size_t tailroom = 0;
> +    while (mbuf) {
> +        tailroom += rte_pktmbuf_tailroom(mbuf);
> +
> +        mbuf = mbuf->next;
> +    }
> +
> +    return tailroom;
> +#else
>      return (char *) dp_packet_end(b) - (char *) dp_packet_tail(b);
> +#endif
>  }
> 
>  /* Clears any data from 'b'. */
>  static inline void
>  dp_packet_clear(struct dp_packet *b)
>  {
> +#ifdef DPDK_NETDEV
> +    rte_pktmbuf_reset(&b->mbuf);
> +#else
>      dp_packet_set_data(b, dp_packet_base(b));
> +#endif
>      dp_packet_set_size(b, 0);
>  }
> 
> @@ -355,10 +374,37 @@ 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->l4_ofs == UINT16_MAX) {
> +        return 0;
> +    }
> +
> +    struct rte_mbuf *hmbuf = CONST_CAST(struct rte_mbuf *, &b->mbuf);
> +    struct rte_mbuf *tmbuf = dp_packet_tail(b);
> +
> +    size_t l4_size = 0;
> +    if (hmbuf == tmbuf) {
> +        l4_size = hmbuf->data_len - b->l4_ofs;
> +    } else {
> +        l4_size = hmbuf->data_len - b->l4_ofs;
> +        hmbuf = hmbuf->next;
> +
> +        while (hmbuf) {
> +            l4_size += tmbuf->data_len;
> +
> +            hmbuf = hmbuf->next;
> +        }
> +    }
> +
> +    l4_size -= dp_packet_l2_pad_size(b);
> +
> +    return l4_size;
> +#else
>      return b->l4_ofs != UINT16_MAX
>          ? (const char *)dp_packet_tail(b) - (const char *)dp_packet_l4(b)
>          - dp_packet_l2_pad_size(b)
>          : 0;
> +#endif
>  }
> 
>  static inline const void *
> @@ -493,7 +539,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
> @@ -501,7 +547,119 @@ dp_packet_set_allocated(struct dp_packet *b,
> uint16_t s)
>  {
>      b->mbuf.buf_len = s;
>  }
> +
> +static inline void *
> +dp_packet_at_offset(const struct dp_packet *b, size_t offset)
> +{
> +    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;
> +    } else {
> +        return (char *) dp_packet_data(b) + offset;
> +    }
> +}
> +
> +/* If 'b' contains at least 'offset + size' bytes of data, returns a pointer 
> to
> + * byte 'offset'.  Otherwise, returns a null pointer. */
> +static inline void *
> +dp_packet_at(const struct dp_packet *b, size_t offset, size_t size)
> +{
> +    return offset + size <= dp_packet_size(b)
> +        ? dp_packet_at_offset(b, offset)
> +        : NULL;
> +}
> +
> +/* Returns a pointer to byte 'offset' in 'b', which must contain at least
> + * 'offset + size' bytes of data. */
> +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 dp_packet_at_offset(b, offset);
> +}
> +
> +/* 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)
> +{
> +    struct rte_mbuf *buf = CONST_CAST(struct rte_mbuf *, &b->mbuf);
> +    if (b->source == DPBUF_DPDK) {
> +
> +        /* Find last segment where data ends, meaning the tail of the chained
> +           mbufs is there */
> +        struct rte_mbuf *pmbuf;
> +        while (buf) {
> +            if (MBUF_BUF_END(buf->buf_addr, buf->buf_len) !=
> +                rte_pktmbuf_mtod_offset(buf, char *, buf->data_len)) {
> +                break;
> +            }
> +
> +            pmbuf = buf;
> +            buf = buf->next;
> +        }
> +
> +        buf = (buf == NULL) ? pmbuf : buf;
> +    }
> +
> +    return rte_pktmbuf_mtod_offset(buf, void *, buf->data_len);
> +}
> +
> +/* Returns a pointer to byte following the last byte allocated for use (but
> + * not necessarily in use) in 'b'. */
> +static inline void *
> +dp_packet_end(const struct dp_packet *b)
> +{
> +    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;
> +    } else {
> +        return (char *) dp_packet_base(b) + dp_packet_get_allocated(b);
> +    }
> +}
>  #else
> +/* If 'b' contains at least 'offset + size' bytes of data, returns a pointer 
> to
> + * byte 'offset'.  Otherwise, returns a null pointer. */
> +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;
> +}
> +
> +/* Returns a pointer to byte 'offset' in 'b', which must contain at least
> + * 'offset + size' bytes of data. */
> +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;
> +}
> +
> +/* 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)
> +{
> +    return (char *) dp_packet_data(b) + dp_packet_size(b);
> +}
> +
> +/* Returns a pointer to byte following the last byte allocated for use (but
> + * not necessarily in use) in 'b'. */
> +static inline void *
> +dp_packet_end(const struct dp_packet *b)
> +{
> +    return (char *) dp_packet_base(b) + dp_packet_get_allocated(b);
> +}
> +
>  static inline void *
>  dp_packet_base(const struct dp_packet *b)
>  {
> @@ -514,6 +672,18 @@ dp_packet_set_base(struct dp_packet *b, void *d)
>      b->base_ = d;
>  }
> 
> +static inline uint16_t
> +dp_packet_get_allocated(const struct dp_packet *b)
> +{
> +    return b->allocated_;
> +}
> +
> +static inline void
> +dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> +{
> +    b->allocated_ = s;
> +}
> +
>  static inline uint32_t
>  dp_packet_size(const struct dp_packet *b)
>  {
> @@ -537,18 +707,6 @@ __packet_set_data(struct dp_packet *b, uint16_t v)
>  {
>      b->data_ofs = v;
>  }
> -
> -static inline uint16_t
> -dp_packet_get_allocated(const struct dp_packet *b)
> -{
> -    return b->allocated_;
> -}
> -
> -static inline void
> -dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> -{
> -    b->allocated_ = s;
> -}
>  #endif

Is it necessary to move the set_allocated and get_allocated functions above?

> 
>  static inline void
> --
> 2.7.4
> 
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to