All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlad Yasevich <vyasevich@gmail.com>
To: Hannes Frederic Sowa <hannes@stressinduktion.org>,
	netdev@vger.kernel.org
Cc: sd@quesysnail.net, bcodding@redhat.com
Subject: Re: [PATCH net] ipv6: don't use CHECKSUM_PARTIAL on MSG_MORE/UDP_CORK sockets
Date: Tue, 20 Oct 2015 17:39:47 -0400	[thread overview]
Message-ID: <5626B4A3.6080900@gmail.com> (raw)
In-Reply-To: <1445351922-8463-1-git-send-email-hannes@stressinduktion.org>

On 10/20/2015 10:38 AM, Hannes Frederic Sowa wrote:
> MSG_MORE might cause the packet to get fragmented in the end when
> passed down to the flush function and the transhdrlen check alone is
> not sufficient to protect against fragmentation. Instead check if the
> socket user intends to add more data to the socket on the first packet.
> 
> This broke checksum calculation for UDPv6 for NFS protocols.
> 
> Fixes: 32dce968dd987 ("ipv6: Allow for partial checksums on non-ufo packets")
> Cc: Vlad Yasevich <vyasevich@gmail.com>
> Tested-by: Sabrina Dubroca <sd@quesysnail.net>
> Tested-by: Benjamin Coddington <bcodding@redhat.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  net/ipv6/ip6_output.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 61d403e..95c5780 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1317,6 +1317,7 @@ emsgsize:
>  	 * sums only work when transhdrlen is set.
>  	 */
>  	if (transhdrlen && sk->sk_protocol == IPPROTO_UDP &&
> +	    !(flags & MSG_MORE) &&
>  	    length + fragheaderlen < mtu &&
>  	    rt->dst.dev->features & NETIF_F_V6_CSUM &&
>  	    !exthdrlen)
> 

Hmm... so while this solves this problem by simply avoiding the combination of
skb #1 having CHECKSUM_PARTIAL and others having CHECKSUM_NONE, I think the actual
problem is a bit deeper.
The above combination seems to work for me since udp6_hwcsum_outgoing() corrects
the checksum.  However, my testing so far has been on nics that have NETIF_F_V6_CSUM,
but without UFO support.

On such systems a simple test of using MSG_MORE an IPv6 udp socket sending 200 bytes
followed by 2000 bytes works correctly.

I am now wondering if this might be UFO related instead and looking for a nic that
has UFO support.

-vlad

  parent reply	other threads:[~2015-10-20 21:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-20 14:38 [PATCH net] ipv6: don't use CHECKSUM_PARTIAL on MSG_MORE/UDP_CORK sockets Hannes Frederic Sowa
2015-10-20 14:55 ` Vlad Yasevich
2015-10-20 21:39 ` Vlad Yasevich [this message]
2015-10-21  9:51   ` Hannes Frederic Sowa
2015-10-21 12:52   ` Hannes Frederic Sowa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5626B4A3.6080900@gmail.com \
    --to=vyasevich@gmail.com \
    --cc=bcodding@redhat.com \
    --cc=hannes@stressinduktion.org \
    --cc=netdev@vger.kernel.org \
    --cc=sd@quesysnail.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.