Re: [PATCH ipvs 1/2] ipvs: Do tcp/udp checksumming prior to tunnel xmit

To: Alex Gartrell <agartrell@xxxxxx>
Subject: Re: [PATCH ipvs 1/2] ipvs: Do tcp/udp checksumming prior to tunnel xmit
Cc: "horms@xxxxxxxxxxxx" <horms@xxxxxxxxxxxx>, "lvs-devel@xxxxxxxxxxxxxxx" <lvs-devel@xxxxxxxxxxxxxxx>, kernel-team <Kernel-team@xxxxxx>
From: Julian Anastasov <ja@xxxxxx>
Date: Fri, 25 Jul 2014 11:53:24 +0300 (EEST)

On Fri, 25 Jul 2014, Alex Gartrell wrote:

> The driver in question is mlx4_en version 2.2-1
> In this case, we have MLX4_TUNNEL_OFFLOAD_MODE turned off, so
> hw_enc_features is set to 0.


> >     Also, can you clarify again which test has
> >the problem with broken csums? All 3 tests? Or it
> >depends on enabled HW csums?
> We run into problems with packets captured from NF_INET_LOCAL_OUT when
> we have hardware checksumming enabled.  Disabling hardware checksumming
> fixes the problem by removing the bits from the features and makes it
> act roughly the same as just setting the encapsulation bit

        ok, so it matches my understanding. I was
afraid you have problems with HW csum disabled.

> >From my reading, the pseudo code for it is basically as follows
> Features = device_features # which has a bunch of stuff, including full
> checksumming
> If skb->encapsulated:
>     Features = device_encapsulated_features # which is zero
> If needs_gso(skb, features) # requires that gso params are set and that
> the features have it
>     # gso segment stuff
> Else
>     If skb->ip_summed == CHECKSUM_PARTIAL: # This is us
>          If skb->encapsulation:
>              skb_set_inner_transport_header(skb,
> skb_checksum_start_offset(skb))
>          Else
>              set_transport_header(skb, skb_checksum_start_offset(skb))
>          If !(features & NETIF_F_ALL_CSUM): # Basically equal to
> skb->encapsulation for us
>              If skb_checksum_help(skb) # will do the software checksumming
> we need
>                     # do error stuff

        I understand all this, that is the reason for
my patch.

> Taking a second look, I think that the problem with my approach is
> that it won¹t take advantage of gso offload for ipip encapsulation.
> Strictly speaking, I don¹t think we need it (the volume of ipvs
> Traffic originating from the host itself is almost always going to
> be negligible).  That said, taking advantage is clearly the right
> way to do it.
> With that in mind, my only problem with your patch is that ipv4 is
> inconsistent with ipv6.  Can we do something like this?  (Apologies,
> I¹m stuck with outlook at home and it is ruining the formatting)  The
> specific change is that we use the same iptunnel_handle_offloads
> Invocation in ipv4 and ipv6.  It¹s disconcerting to me that it¹s unclear
> if we can do IP6IP6 with gso offload ‹ any thoughts there?

        That is my doubt too. iptunnel_handle_offloads is
called only for IPv4 tunnels, even in net/ipv6/. OTOH,
ip6_tnl_xmit2() does everything directly, that is why
I choosed the same thing for IPv6. I don't see value to use
instead of SKB_GSO_IPIP. Looking at ipv4_offload_init()
for IPv4 there is offload support for TCP, UDP and IPIP.
ipv6_offload_init() shows support for TCP, UDP,
via ipv6_exthdrs_offload_init for IPPROTO_ROUTING and
IPPROTO_DSTOPTS), sit_offload (IPPROTO_IPV6). So, it
may be possible to use iptunnel_handle_offloads for IPv6.

        We may need help from experts in this area. So,
I'll post the patch as RFC on netdev after little
research for GRO/GSO.


Julian Anastasov <ja@xxxxxx>
<Prev in Thread] Current Thread [Next in Thread>