Re: rtl8168e-vl dropping tftp ack

[Stefan Bader <stefan.bader@canonical.com>]

[-- Attachment #1: Type: text/plain, Size: 3408 bytes --]

On 18.04.2013 23:55, Francois Romieu wrote:
> Stefan Bader <stefan.bader@canonical.com> :
> [...]
>> This was taken with a 3.9-rc7 kernel running on the Xen host, listening on the
>> tftp server. First run is with the kernel driver and the second pass with
> 
> Please try the patch below with current kernel and capture traffic on
> both the tftp server and r8169 interfaces.
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 4ecbe64..c1cd9f6 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -5747,7 +5747,7 @@ err_out:
>  	return -EIO;
>  }
>  
> -static inline void rtl8169_tso_csum(struct rtl8169_private *tp,
> +static inline bool rtl8169_tso_csum(struct rtl8169_private *tp,
>  				    struct sk_buff *skb, u32 *opts)
>  {
>  	const struct rtl_tx_desc_info *info = tx_desc_info + tp->txd_version;
> @@ -5760,6 +5760,15 @@ static inline void rtl8169_tso_csum(struct rtl8169_private *tp,
>  	} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>  		const struct iphdr *ip = ip_hdr(skb);
>  
> +		if (unlikely(skb->len < ETH_ZLEN &&
> +		    (tp->mac_version == RTL_GIGA_MAC_VER_34))) {
> +			if (skb_padto(skb, ETH_ZLEN))
> +				return false;
> +			skb_checksum_help(skb);
> +			skb_put(skb, ETH_ZLEN - skb->len);
> +			return true;
> +		}
> +
>  		if (ip->protocol == IPPROTO_TCP)
>  			opts[offset] |= info->checksum.tcp;
>  		else if (ip->protocol == IPPROTO_UDP)
> @@ -5767,6 +5776,7 @@ static inline void rtl8169_tso_csum(struct rtl8169_private *tp,
>  		else
>  			WARN_ON_ONCE(1);
>  	}
> +	return true;
>  }
>  
>  static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
> @@ -5790,25 +5800,26 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
>  	if (unlikely(le32_to_cpu(txd->opts1) & DescOwn))
>  		goto err_stop_0;
>  
> +	opts[1] = cpu_to_le32(rtl8169_tx_vlan_tag(skb));
> +	opts[0] = DescOwn;
> +
> +	if (!rtl8169_tso_csum(tp, skb, opts))
> +		goto err_update_stats_0;
> +
>  	len = skb_headlen(skb);
>  	mapping = dma_map_single(d, skb->data, len, DMA_TO_DEVICE);
>  	if (unlikely(dma_mapping_error(d, mapping))) {
>  		if (net_ratelimit())
>  			netif_err(tp, drv, dev, "Failed to map TX DMA!\n");
> -		goto err_dma_0;
> +		goto err_free_skb_1;
>  	}
>  
>  	tp->tx_skb[entry].len = len;
>  	txd->addr = cpu_to_le64(mapping);
>  
> -	opts[1] = cpu_to_le32(rtl8169_tx_vlan_tag(skb));
> -	opts[0] = DescOwn;
> -
> -	rtl8169_tso_csum(tp, skb, opts);
> -
>  	frags = rtl8169_xmit_frags(tp, skb, opts);
>  	if (frags < 0)
> -		goto err_dma_1;
> +		goto err_unmap_2;
>  	else if (frags)
>  		opts[0] |= FirstFrag;
>  	else {
> @@ -5854,10 +5865,11 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
>  
>  	return NETDEV_TX_OK;
>  
> -err_dma_1:
> +err_unmap_2:
>  	rtl8169_unmap_tx_skb(d, tp->tx_skb + entry, txd);
> -err_dma_0:
> +err_free_skb_1:
>  	dev_kfree_skb(skb);
> +err_update_stats_0:
>  	dev->stats.tx_dropped++;
>  	return NETDEV_TX_OK;
>  
> 

Might be a while till I will be able to run any tests since msi was so generous
to provide an uefi bios that can brick itself from their efi shell. *sigh*
I had tried the combination of your change and Hayes' but that would cause
panics in skb_checksum_help (not sure which of the two) on boot.

-Stefan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]