netdev - Re: getting warn once around skb_try

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 Jul 2012 15:35:29 +0300
From:	Shlomo Pongartz <shlomop@...lanox.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	Or Gerlitz <ogerlitz@...lanox.com>,
	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Erez Shitrit <erezsh@...lanox.com>
Subject: Re: getting warn once around skb_try_coalesce

On 7/10/2012 2:22 PM, Eric Dumazet wrote:
> On Tue, 2012-07-10 at 13:14 +0200, Eric Dumazet wrote:
>> On Tue, 2012-07-10 at 12:18 +0200, Eric Dumazet wrote:
>>> On Tue, 2012-07-10 at 12:54 +0300, Or Gerlitz wrote:
>>>> Hi Dave, Eric,
>>>>
>>>> Another trace that I see here with net-next is this one-time warning. I
>>>> get it always
>>>> on the passive side of TCP, something that seems related to GRO, it
>>>> happens only with
>>>> IPoIB, not with mlx4_en and igb (when igb get to work on net-next...)
>>>>
>>>> The latest commit in this area is bad43ca8325f493dcaa0896c2f036276af059c7e
>>>> "net: introduce skb_try_coalesce()" from Eric.
>>>>
>>>> Or.
>>>>
>>>> -----------[ cut here ]------------
>>>> WARNING: at net/core/skbuff.c:3413 skb_try_coalesce+0x1f8/0x31d()
>>> This warning catch skb truesize offenders, most probably its a driver
>>> issue.
>>>
>> By the way, this driver allocates not enough tailroom in skbs, so IP/TCP
>> stacks need to reallocate skb head to pull IP/TCP headers. Thats not
>> efficient.
>>
>> I suggest using following patch :
> And of course we also can fix the truesize bug.
> (Not sure it will fix the warning, but worth trying)
>
> Since this driver allocates a full page, it must use the PAGE_SIZE, not
> the used part in the fragment
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> index 5c1bc99..e611a924 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> @@ -123,7 +123,7 @@ static void ipoib_ud_skb_put_frags(struct ipoib_dev_priv *priv,
>
>   		skb_frag_size_set(frag, size);
>   		skb->data_len += size;
> -		skb->truesize += size;
> +		skb->truesize += PAGE_SIZE;
>   	} else
>   		skb_put(skb, length);
>
> @@ -159,7 +159,7 @@ static struct sk_buff *ipoib_alloc_rx_skb(struct net_device *dev, int id)
>   	u64 *mapping;
>
>   	if (ipoib_ud_need_sg(priv->max_ib_mtu))
> -		buf_size = IPOIB_UD_HEAD_SIZE;
> +		buf_size = IPOIB_UD_HEAD_SIZE + 128; /* reserve some tailroom for IP/TCP headers */
>   	else
>   		buf_size = IPOIB_UD_BUF_SIZE(priv->max_ib_mtu);
>
>
>
>
> .
>
Hi,

I've applied the patch and there are no more warnings. Thanks.
Can you please elaborate on this issue which was there from day one and 
AFAIK never manifested itself.

Best regards,
S.P.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html