[ovs-dev] [PATCH] conntrack: Fix tcp payload length in case multi-segments.

[Darrell Ball]

Hi Zhike

Thanks for clarifying
There is presently no support for multi-segment mbufs in OVS, so the patch
would
not be needed and in general patches are only proposed at the time they are
relevant.

Also note that in the hypothetical case of multisegment mbufs, the packet
would be linearized at
this point.

Also note that this patch still relies on pointers..

As Ilya already mentioned there is no need to use multi-segment mbufs on
OVS as large buffers
can be used instead.

On Sun, Nov 10, 2019 at 9:50 PM 王志克 <wangzhike at jd.com> wrote:

> Hi Darrell,
>
> In TSO case, the packet may use multi-segments mbuf, and I do not think we
> need to make it linearal. In this case, we can NOT use pointer to calculate
> the tcp length.
>
> Br,
>
> Zhike Wang
> JDCloud, Product Development, IaaS
>
> ------------------------------------------------------------------------------------------------
> Mobile／+86 13466719566
> E- mail／wangzhike at jd.com
> Address／5F Building A,North-Star Century Center,8 Beichen West
> Street,Chaoyang District Beijing
> Https://JDCloud.com
>
> ------------------------------------------------------------------------------------------------
>
>
> From: Darrell Ball [mailto:dlu998 at gmail.com]
> Sent: Saturday, November 09, 2019 8:12 AM
> To: Zhike Wang
> Cc: ovs dev; 王志克
> Subject: Re: [ovs-dev] [PATCH] conntrack: Fix tcp payload length in case
> multi-segments.
>
> Thanks for the patch
>
> Would you mind describing the use case that this patch is aiming to
> support ?
>
> On Fri, Nov 8, 2019 at 1:23 AM Zhike Wang <wangzk320 at 163.com> wrote:
> Signed-off-by: Zhike Wang <wangzk320 at 163.com>
> ---
>  lib/conntrack-private.h | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> index 590f139..1d21f6e 100644
> --- a/lib/conntrack-private.h
> +++ b/lib/conntrack-private.h
> @@ -233,13 +233,17 @@ conn_update_expiration(struct conntrack *ct, struct
> conn *conn,
>  static inline uint32_t
>  tcp_payload_length(struct dp_packet *pkt)
>  {
> -    const char *tcp_payload = dp_packet_get_tcp_payload(pkt);
> -    if (tcp_payload) {
> -        return ((char *) dp_packet_tail(pkt) - dp_packet_l2_pad_size(pkt)
> -                - tcp_payload);
> -    } else {
> -        return 0;
> +    size_t l4_size = dp_packet_l4_size(pkt);
> +
> +    if (OVS_LIKELY(l4_size >= TCP_HEADER_LEN)) {
> +        struct tcp_header *tcp = dp_packet_l4(pkt);
> +        int tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4;
> +
> +        if (OVS_LIKELY(tcp_len >= TCP_HEADER_LEN && tcp_len <= l4_size)) {
> +            return (l4_size - tcp_len);
> +        }
>
> Maybe I missed something, but it looks like the same calculation is
> arrived at.
>
>      }
> +    return 0;
>  }
>
>  #endif /* conntrack-private.h */
> --
> 1.8.3.1
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>