On Tue, May 02, 2017 at 02:10:43PM +0800, Michael Qiu wrote:
> From: Michael Qiu <qiud...@chinac.com>
> 
> When a packet is from DPDK source, and it contains
> multiple segments, data_len is not equal to the
> packet size. This patch fix this issue.
> 
> Signed-off-by: Michael Qiu <qiud...@chinac.com>
> Signed-off-by: Marcin Ksiadz <marcinx.ksi...@intel.com>
> Signed-off-by: Mark Kavanagh <mark.b.kavan...@intel.com>
> Signed-off-by: Przemyslaw Lal <przemyslawx....@intel.com>
> Signed-off-by: Yuanhan Liu <yuanhan....@linux.intel.com>

Thank you for working to improve OVS support for DPDK.

This is a very simple patch.  Can you explain the chain of authorship?

> @@ -415,17 +416,19 @@ dp_packet_size(const struct dp_packet *b)
>  static inline void
>  dp_packet_set_size(struct dp_packet *b, uint32_t v)
>  {
> -    /* netdev-dpdk does not currently support segmentation; consequently, for
> -     * all intents and purposes, 'data_len' (16 bit) and 'pkt_len' (32 bit) 
> may
> -     * be used interchangably.
> -     *
> -     * On the datapath, it is expected that the size of packets
> -     * (and thus 'v') will always be <= UINT16_MAX; this means that there is 
> no
> -     * loss of accuracy in assigning 'v' to 'data_len'.
> +    /*
> +     * Assign current segment length. If total length is greater than
> +     * max data length in a segment, additional calculation is needed
>       */
> -    b->mbuf.data_len = (uint16_t)v;  /* Current seg length. */
> -    b->mbuf.pkt_len = v;             /* Total length of all segments linked 
> to
> -                                      * this segment. */
> +    if (v > (b->mbuf.buf_len - b->mbuf.data_off)) {
> +        b->mbuf.data_len =
> +            (uint16_t) (b->mbuf.buf_len - b->mbuf.data_off);
> +    } else {
> +        b->mbuf.data_len = (uint16_t) v;
> +    }

I don't see any value in the two casts to uint16_t above.  They convert
their operands to uint16_t, but this will also happen just as well
without the casts since the destination has type uint16_t.  Open vSwitch
style omits needless casts.

Also, I think that the above can be written more simply as:
        b->mbuf.data_len = MIN(v, b->mbuf.buf_len - b->mbuf.data_off);


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to