[ovs-dev,RFC,v7,05/13] dp-packet: Handle multi-seg mbufs in helper funcs.

Message ID 1527094048-105084-6-git-send-email-tiago.lam@intel.com
State Superseded
Headers show
Series
  • Support multi-segment mbufs
Related show

Commit Message

Tiago Lam May 23, 2018, 4:47 p.m.
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.kavanagh@intel.com>

Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
Signed-off-by: Tiago Lam <tiago.lam@intel.com>
---
 lib/dp-packet.h | 252 +++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 205 insertions(+), 47 deletions(-)

Comments

Loftus, Ciara May 28, 2018, 3:41 p.m. | #1
> 
> 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.kavanagh@intel.com>
> 
> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
> Signed-off-by: Tiago Lam <tiago.lam@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
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Tiago Lam May 30, 2018, 7:21 a.m. | #2
On 28/05/2018 16:41, Loftus, Ciara wrote:
>>
>> 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.kavanagh@intel.com>
>>
>> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>> Signed-off-by: Tiago Lam <tiago.lam@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?
> 

Thanks for pointing that out. I think we will still need both as
internally we want a way to find the tail mbuf and to keep the external
API we have to return the last used byte within the tail mbuf. But
there's definitely space for improvement, so I'll include a refactor of
this in the next iteration.

> 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?
> 

Yeah, exactly! `rte_pktmbuf_lastseg()`, together with
`rte_pktmbuf_tailroom()`, work nicely if the end mbuf contains the last
byte of data in use. But we may have mbufs chained together where the
end mbufs don't contain the last byte of data in use (their `data_len`
is 0). Hence why `dp_packet_mbuf_tail()` tries to find the first mbuf
where the address of data_len != address of buf_len.

>> +
>>  /* 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?
> 

Nope. I'll move them back.

Tiago.

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

Patch

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
+
 /* 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
 
 static inline void