Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1510538

Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set gso_type

From Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
Newsgroups linux.kernel
Subject Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set gso_type
Date 2016-10-27 20:00 +0200
Message-ID <swX85-51F-19@gated-at.bofh.it> (permalink)
References <swjX3-4iF-7@gated-at.bofh.it> <swV6h-3LE-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 10/27/2016 10:26 AM, Eric Dumazet wrote:
> On Wed, 2016-10-26 at 11:09 +1100, Jon Maxwell wrote:
>> We recently encountered a bug where a few customers using ibmveth on the 
>> same LPAR hit an issue where a TCP session hung when large receive was
>> enabled. Closer analysis revealed that the session was stuck because the 
>> one side was advertising a zero window repeatedly.
>>
>> We narrowed this down to the fact the ibmveth driver did not set gso_size 
>> which is translated by TCP into the MSS later up the stack. The MSS is 
>> used to calculate the TCP window size and as that was abnormally large, 
>> it was calculating a zero window, even although the sockets receive buffer 
>> was completely empty. 
>>
>> We were able to reproduce this and worked with IBM to fix this. Thanks Tom 
>> and Marcelo for all your help and review on this.
>>
>> The patch fixes both our internal reproduction tests and our customers tests.
>>
>> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
>> ---
>>  drivers/net/ethernet/ibm/ibmveth.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
>> index 29c05d0..c51717e 100644
>> --- a/drivers/net/ethernet/ibm/ibmveth.c
>> +++ b/drivers/net/ethernet/ibm/ibmveth.c
>> @@ -1182,6 +1182,8 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
>>  	int frames_processed = 0;
>>  	unsigned long lpar_rc;
>>  	struct iphdr *iph;
>> +	bool large_packet = 0;
>> +	u16 hdr_len = ETH_HLEN + sizeof(struct tcphdr);
>>  
>>  restart_poll:
>>  	while (frames_processed < budget) {
>> @@ -1236,10 +1238,28 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
>>  						iph->check = 0;
>>  						iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
>>  						adapter->rx_large_packets++;
>> +						large_packet = 1;
>>  					}
>>  				}
>>  			}
>>  
>> +			if (skb->len > netdev->mtu) {
>> +				iph = (struct iphdr *)skb->data;
>> +				if (be16_to_cpu(skb->protocol) == ETH_P_IP &&
>> +				    iph->protocol == IPPROTO_TCP) {
>> +					hdr_len += sizeof(struct iphdr);
>> +					skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
>> +					skb_shinfo(skb)->gso_size = netdev->mtu - hdr_len;
>> +				} else if (be16_to_cpu(skb->protocol) == ETH_P_IPV6 &&
>> +					   iph->protocol == IPPROTO_TCP) {
>> +					hdr_len += sizeof(struct ipv6hdr);
>> +					skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
>> +					skb_shinfo(skb)->gso_size = netdev->mtu - hdr_len;
>> +				}
>> +				if (!large_packet)
>> +					adapter->rx_large_packets++;
>> +			}
>> +
>>  
> This might break forwarding and PMTU discovery.
>
> You force gso_size to device mtu, regardless of real MSS used by the TCP
> sender.
>
> Don't you have the MSS provided in RX descriptor, instead of guessing
> the value ?
>
>
>
The MSS is not always available unfortunately, so this is the best solution there is at the moment. 

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set  gso_type Eric Dumazet <eric.dumazet@gmail.com> - 2016-10-27 17:50 +0200
  Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set  gso_type Thomas Falcon <tlfalcon@linux.vnet.ibm.com> - 2016-10-27 20:00 +0200
    Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set  gso_type Eric Dumazet <eric.dumazet@gmail.com> - 2016-10-27 20:10 +0200
      Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set gso_type Jonathan Maxwell <jmaxwell37@gmail.com> - 2016-10-30 02:30 +0200
  Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set  gso_type Brian King <brking@linux.vnet.ibm.com> - 2016-11-02 22:50 +0100
    Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set gso_type Jonathan Maxwell <jmaxwell37@gmail.com> - 2016-11-06 22:30 +0100
      Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set  gso_type Brian King <brking@linux.vnet.ibm.com> - 2016-11-09 19:20 +0100
  Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set  gso_type Brian King <brking@linux.vnet.ibm.com> - 2016-11-11 19:20 +0100

csiph-web