Hello Tiago,

On Thu, Jan 10, 2019 at 1:14 AM Tiago Lam <[email protected]> wrote:

> Previous commits have added support to the dp_packet API to handle
> multi-segmented packets, where data is not stored contiguously in
> memory. However, in some cases, it is inevitable and data must be
> provided contiguously. Examples of such cases are when performing csums
> over the entire packet data, or when write()'ing to a file descriptor
> (for a tap interface, for example). For such cases, the dp_packet API
> has been extended to provide a way to transform a multi-segmented
> DPBUF_DPDK packet into a DPBUF_MALLOC system packet (at the expense of a
> copy of memory). If the packet's data is already stored in memory
> contigously then there's no need to convert the packet.
>
> Thus, the main use cases that were assuming that a dp_packet's data is
> always held contiguously in memory were changed to make use of the new
> "linear functions" in the dp_packet API when there's a need to traverse
> the entire's packet data. Per the example above, when the packet's data
> needs to be write() to the tap's file descriptor, or when the conntrack
> module needs to verify a packet's checksum, the data is now linearized.
>
> Additionally, the miniflow_extract() function has been modified to check
> if the respective packet headers don't span across multiple mbufs. This
> requirement is needed to guarantee that callers can assume headers are
> always in contiguous memory.
>
> Signed-off-by: Tiago Lam <[email protected]>
> ---
>
>
[snip]


> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index a003546..970aaf2 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
>
> [snip]


> @@ -356,6 +379,39 @@ dp_packet_try_pull(struct dp_packet *b, size_t size)
>          ? dp_packet_pull(b, size) : NULL;
>  }
>
> +/* Reads 'size' bytes from 'offset' in 'b', linearly, to 'ptr', if 'buf'
> is
> + * NULL. Otherwise, if a 'buf' is provided, it must have 'size' bytes,
> and the
> + * data will be copied there, iff it is found to be non-linear. */
> +static inline ssize_t
> +dp_packet_read_data(const struct dp_packet *b, size_t offset, size_t size,
> +                    void **ptr, void *buf) {
> +    /* Zero copy */
> +    if ((*ptr = dp_packet_at(b, offset, size)) != NULL) {
> +        return 0;
> +    }
> +
> +    /* Copy available linear data */
> +    if (buf == NULL) {
> +#ifdef DPDK_NETDEV
> +        size_t mofs = offset;
> +        const struct rte_mbuf *mbuf = dp_packet_mbuf_from_offset(b,
> &mofs);
> +        *ptr = dp_packet_at(b, offset, mbuf->data_len - mofs);
> +
> +        return size - (mbuf->data_len - mofs);
> +#else
> +        /* Non-DPDK dp_packets should always hit the above condition */
> +        ovs_assert(1);
> +#endif
> +    }
> +
> +    /* Copy all data */
> +
> +    *ptr = buf;
> +    dp_packet_copy_from_offset(b, offset, size, buf);
> +
> +    return 0;
> +}
> +
>  static inline bool
>  dp_packet_is_eth(const struct dp_packet *b)
>  {
>

This part triggers build (non fatal o_O) warnings on fedora 28.
I did not take time to analyse this yet, tell me if you can have a look.


In file included from /vagrant/lib/packets.c:35:
In function ‘dp_packet_copy_from_offset’,
    inlined from ‘dp_packet_read_data.part.16.constprop’ at
/vagrant/lib/dp-packet.h:410:5,
    inlined from ‘dp_packet_read_data.constprop’ at
/vagrant/lib/dp-packet.h:386:1,
    inlined from ‘packet_csum_continue’ at /vagrant/lib/packets.c:1705:15:
/vagrant/lib/dp-packet.h:1058:5: warning: argument 1 null where non-null
expected [-Wnonnull]
     memcpy(buf, (char *)dp_packet_data(b) + offset, size);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ./lib/string.h:20,
                 from /vagrant/lib/packets.h:23,
                 from /vagrant/lib/packets.c:18:
/vagrant/lib/packets.c: In function ‘packet_csum_continue’:
/usr/include/string.h:42:14: note: in a call to function ‘memcpy’ declared
here
 extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
              ^~~~~~
In file included from /vagrant/lib/packets.c:35:
In function ‘dp_packet_copy_from_offset’,
    inlined from ‘dp_packet_read_data.part.16.constprop’ at
/vagrant/lib/dp-packet.h:410:5,
    inlined from ‘dp_packet_read_data.constprop’ at
/vagrant/lib/dp-packet.h:386:1,
    inlined from ‘packet_crc32c’ at /vagrant/lib/packets.c:1732:15:
/vagrant/lib/dp-packet.h:1058:5: warning: argument 1 null where non-null
expected [-Wnonnull]
     memcpy(buf, (char *)dp_packet_data(b) + offset, size);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ./lib/string.h:20,
                 from /vagrant/lib/packets.h:23,
                 from /vagrant/lib/packets.c:18:
/vagrant/lib/packets.c: In function ‘packet_crc32c’:
/usr/include/string.h:42:14: note: in a call to function ‘memcpy’ declared
here
 extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
              ^~~~~~


-- 
David Marchand
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to