diff mbox series

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

Message ID 1539188552-129083-7-git-send-email-tiago.lam@intel.com
State Changes Requested
Delegated to: Ian Stokes
Headers show
Series Support multi-segment mbufs | expand

Commit Message

Lam, Tiago Oct. 10, 2018, 4:22 p.m. UTC
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 | 191 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 177 insertions(+), 18 deletions(-)

Comments

Stokes, Ian Oct. 22, 2018, 2:30 p.m. UTC | #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.
> 
> 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>

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
Stokes, Ian Dec. 17, 2018, 8:07 p.m. UTC | #2
On 10/22/2018 3:30 PM, Stokes, Ian 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.
>>
>> 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>
> 
> Thanks for this Tiago,
> 
> It looks ok on inspection, will need a rebase however in the next revision.
> 
> Ian
> 

Quick follow up on this Tiago, I spotted some errors introduced in 
Travis after applying this patch (Note this was testing this series on 
top of 35fe9ef ("dpif-netdev: Add vlan to mask for flow_put 
operation.") as per the cover note.

https://travis-ci.org/istokes/ovs/jobs/469153535
https://travis-ci.org/istokes/ovs/jobs/469153543

These should be fixed during the rebase also.

Thanks
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
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

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);
 }