[ovs-dev,v13,03/11] dp-packet: Handle multi-seg mbufs in helper funcs.

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@intel.com>

Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
Signed-off-by: Tiago Lam <tiago.lam@intel.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Flavio Leitner <fbl@sysclose.org>
---
 lib/dp-packet.c |   4 +-
 lib/dp-packet.h | 192 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 178 insertions(+), 18 deletions(-)

On 1/9/2019 6:05 PM, Tiago Lam 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.
> 
> 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.
> 

Hi Tiago, thanks for the patch, while reviewing a later patch in the 
series I noticed there are 2 or 3 functions changed in this patch that 
are changed/removed altogether at a later point. Maybe this was because 
of a legacy approach and so they were not needed any more?

It would be good to avoid introducing these changes if they are going to 
be removed in a later patch unless they are necessary.

I've the functions in particular  below if you can take a look and confirm.

> 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>
> Acked-by: Eelco Chaudron <echaudro@redhat.com>
> Acked-by: Flavio Leitner <fbl@sysclose.org>
> ---
>   lib/dp-packet.c |   4 +-
>   lib/dp-packet.h | 192 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>   2 files changed, 178 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index 93b0e9c..f54119d 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 72a8043..8947477 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) {

This is renamed and reworked in patch 06 of the series 'dp-packet: Add 
support for data "linearization"' with dp_packet_may_pull().

Could those changes be made here or are they required only when 
linearization is introduced?


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

The changes above are removed in patch 06 'dp-packet: Add support for 
data "linearization"'.

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

The changes above are removed in patch 06 'dp-packet: Add support for 
data "linearization"'.

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

The changes above are removed in patch 06 'dp-packet: Add support for 
data "linearization"'.

>       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

The changes above are removed in patch 06 'dp-packet: Add support for 
data "linearization"'.

Thanks
Ian

>       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,54 @@ 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 = NULL;
> +    const struct rte_mbuf *m_b = NULL;
> +    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)
>   {
> @@ -534,7 +688,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
> @@ -632,6 +786,13 @@ dp_packet_has_flow_mark(struct dp_packet *p, uint32_t *mark)
>   }
>   
>   #else /* DPDK_NETDEV */
> +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)
>   {
> @@ -806,10 +967,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);
>   }
>   
>

On 10/01/2019 11:17, Ian Stokes wrote:
> On 1/9/2019 6:05 PM, Tiago Lam 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.
>>
>> 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.
>>
> 
> Hi Tiago, thanks for the patch, while reviewing a later patch in the 
> series I noticed there are 2 or 3 functions changed in this patch that 
> are changed/removed altogether at a later point. Maybe this was because 
> of a legacy approach and so they were not needed any more?
> 
> It would be good to avoid introducing these changes if they are going to 
> be removed in a later patch unless they are necessary.
> 
> I've the functions in particular  below if you can take a look and confirm.

Thanks for the analysis, Ian.

This is indeed because of different approaches that were taken and then
merged to the linearization patch without much thought on changes made
to earlier patches. I'll remove those pieces from this patch.

Just another reply below.

> 
>> 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>
>> Acked-by: Eelco Chaudron <echaudro@redhat.com>
>> Acked-by: Flavio Leitner <fbl@sysclose.org>
>> ---
>>   lib/dp-packet.c |   4 +-
>>   lib/dp-packet.h | 192 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>>   2 files changed, 178 insertions(+), 18 deletions(-)
>>
>> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
>> index 93b0e9c..f54119d 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 72a8043..8947477 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) {
> 
> This is renamed and reworked in patch 06 of the series 'dp-packet: Add 
> support for data "linearization"' with dp_packet_may_pull().
> 
> Could those changes be made here or are they required only when 
> linearization is introduced?

I believe they are only for the linearization, so I'll move it there.

Thanks,
Tiago.

> 
> 
>> +    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
>> +
> 
> The changes above are removed in patch 06 'dp-packet: Add support for 
> data "linearization"'.
> 
>>       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
>> +
> 
> The changes above are removed in patch 06 'dp-packet: Add support for 
> data "linearization"'.
> 
>>       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
>> +
> 
> The changes above are removed in patch 06 'dp-packet: Add support for 
> data "linearization"'.
> 
>>       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
> 
> The changes above are removed in patch 06 'dp-packet: Add support for 
> data "linearization"'.
> 
> Thanks
> Ian
> 
>>       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,54 @@ 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 = NULL;
>> +    const struct rte_mbuf *m_b = NULL;
>> +    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)
>>   {
>> @@ -534,7 +688,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
>> @@ -632,6 +786,13 @@ dp_packet_has_flow_mark(struct dp_packet *p, uint32_t *mark)
>>   }
>>   
>>   #else /* DPDK_NETDEV */
>> +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)
>>   {
>> @@ -806,10 +967,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);
>>   }
>>   
>>
>