Received: by 2002:a05:6a10:1d13:0:0:0:0 with SMTP id pp19csp1585131pxb; Sun, 22 Aug 2021 22:46:22 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwSlLDbwq4UJxEGBGsHWAGVFURxV55E73snjdXZgNSOg69XSg3IGcqrZiVUUgvQ4Lazdagh X-Received: by 2002:a05:6402:22b0:: with SMTP id cx16mr35488311edb.185.1629697582252; Sun, 22 Aug 2021 22:46:22 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1629697582; cv=none; d=google.com; s=arc-20160816; b=W+DRUCqtluoeVmFD/FkPlxCa0MCNbYV1B+TVs7O9UhKEjLk8jovLegyM8Iq2bMnN/u qct+/NxP2fXty4oOp7LIcfDY+XGT1odYwC7Mrv2EgpGqbvSoq4LI2cbqvo4MPbV8B4oz HiB9EKyPWUEG/A4zIyxrPAwEznu9G4gOwUXzzS8/Ng0B9ttDQHTOasHMqLhm3gSk8znZ YxsYjE9RA9BvzJXt4oLNX6mlZLjFUp7sc+aqIuvsBzg4Xa0GCNvzpHsyk8hj0d+ixuN0 jKkYtzygPQ9ewNkvTKdPntCilP4W4Fm0SZ/uw5i4gXY5cPvn+fK5m65kCwotmCyCx9uh grqw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:from:references :cc:to:subject:dkim-signature; bh=Fl3YLeNPw73kTCd+tayVreDVmY39xWbUXsGFSI8fAJs=; b=kHh+jCqLqEtyjfe+hwOAPi9NTfGfncZ6n0VqggzT5upnnxzFA4v46oQjcp+FZEza3t O3FKsLdcoPosVZblSooLFqgKgVmORrOIhhA9ZUg9YK97ROg1T5DnVuyS+jKksMNGwkLp Hw5Lcw9KYa20UNcUELAhtGr4nNUTV2t9ZLTAtiwu1RsDIQdnhguqmFh/Ubc4ZdGawUwV vJQTXinXuK0URdgax4d+ZhXxb89JKkTUPpo0Hy5W1Hf2Qcn8HGjb1vNMbf3Aol8916Pe Ek5LNLbOFh9JfPkNEIuTJ/u7qB1V6qYjGY47v2x+2REuWmEIAI4IUDweJW55LMeiEljK kgBA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@virtuozzo.com header.s=relay header.b=BKgjhm6J; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=virtuozzo.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id t9si13517973ejj.606.2021.08.22.22.45.51; Sun, 22 Aug 2021 22:46:22 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@virtuozzo.com header.s=relay header.b=BKgjhm6J; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=virtuozzo.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234671AbhHWFpT (ORCPT + 99 others); Mon, 23 Aug 2021 01:45:19 -0400 Received: from relay.sw.ru ([185.231.240.75]:57712 "EHLO relay.sw.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231267AbhHWFpS (ORCPT ); Mon, 23 Aug 2021 01:45:18 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=virtuozzo.com; s=relay; h=Content-Type:MIME-Version:Date:Message-ID:From: Subject; bh=Fl3YLeNPw73kTCd+tayVreDVmY39xWbUXsGFSI8fAJs=; b=BKgjhm6JRLLJVwNPQ M5iIcdxkT+Z2i8RPmYlvvcyCWT4GPLbgZuMaBdHv7yjTkN2pwyEQH50VcdDxSW3lbAVry6Z36zMSY eGjCk8cNWp0T0iiJROU6Kl2ZRWgTdCthMWZu220lZks6UZ9FcVFSf+cRakuoTZUHCRYvCdhqt5hpo =; Received: from [10.93.0.56] by relay.sw.ru with esmtp (Exim 4.94.2) (envelope-from ) id 1mI2l4-008Y2I-Hh; Mon, 23 Aug 2021 08:44:30 +0300 Subject: Re: [PATCH NET v4 3/7] ipv6: use skb_expand_head in ip6_xmit To: Christoph Paasch Cc: "David S. Miller" , Hideaki YOSHIFUJI , David Ahern , Jakub Kicinski , Eric Dumazet , netdev , LKML , kernel@openvz.org, Julian Wiedmann References: <77f3e358-c75e-b0bf-ca87-6f8297f5593c@virtuozzo.com> From: Vasily Averin Message-ID: <7d71f2cc-3fff-beff-b82b-d0a81fb60429@virtuozzo.com> Date: Mon, 23 Aug 2021 08:44:29 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 8/22/21 8:13 PM, Christoph Paasch wrote: > On Sun, Aug 22, 2021 at 10:04 AM Christoph Paasch > wrote: >> >> Hello Vasily, >> >> On Fri, Aug 20, 2021 at 11:21 PM Vasily Averin wrote: >>> >>> On 8/21/21 1:44 AM, Christoph Paasch wrote: >>>> (resend without html - thanks gmail web-interface...) >>>> On Fri, Aug 20, 2021 at 3:41 PM Christoph Paasch >>>>> AFAICS, this is because pskb_expand_head (called from >>>>> skb_expand_head) is not adjusting skb->truesize when skb->sk is set >>>>> (which I guess is the case in this particular scenario). I'm not >>>>> sure what the proper fix would be though... >>> >>> Could you please elaborate? >>> it seems to me skb_realloc_headroom used before my patch called pskb_expand_head() too >>> and did not adjusted skb->truesize too. Am I missed something perhaps? >>> >>> The only difference in my patch is that skb_clone can be not called, >>> though I do not understand how this can affect skb->truesize. >> >> I *believe* that the difference is that after skb_clone() skb->sk is >> NULL and thus truesize will be adjusted. >> >> I will try to confirm that with some more debugging. > > Yes indeed. > > Before your patch: > [ 19.154039] ip6_xmit before realloc truesize 4864 sk? 000000002ccd6868 > [ 19.155230] ip6_xmit after realloc truesize 5376 sk? 0000000000000000 > > skb->sk is not set and thus truesize will be adjusted. This looks strange for me. skb should not lost sk reference. Could you please clarify where exactly you cheked it? sk on newly allocated skb is set on line 291 net/ipv6/ip6_output.c::ip6_xmit() 282 if (unlikely(skb_headroom(skb) < head_room)) { 283 struct sk_buff *skb2 = skb_realloc_headroom(skb, head_room); 284 if (!skb2) { 285 IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)), 286 IPSTATS_MIB_OUTDISCARDS); 287 kfree_skb(skb); 288 return -ENOBUFS; 289 } 290 if (skb->sk) 291 skb_set_owner_w(skb2, skb->sk); <<<<< here 292 consume_skb(skb); 293 skb = skb2; 294 } > With your change: > [ 15.092933] ip6_xmit before realloc truesize 4864 sk? 00000000072930fd > [ 15.094131] ip6_xmit after realloc truesize 4864 sk? 00000000072930fd > > skb->sk is set and thus truesize is not adjusted. In this case skb_set_owner_w() is called inside skb_expand_head() net/ipv6/ip6_output.c::ip6_xmit() 265 if (unlikely(head_room > skb_headroom(skb))) { 266 skb = skb_expand_head(skb, head_room); 267 if (!skb) { 268 IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS); 269 return -ENOBUFS; 270 } 271 } net/core/skbuff.c::skb_expand_head() 1813 if (skb_shared(skb)) { 1814 struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); 1815 1816 if (likely(nskb)) { 1817 if (skb->sk) 1818 skb_set_owner_w(nskb, skb->sk); <<<< here 1819 consume_skb(skb); 1820 } else { 1821 kfree_skb(skb); 1822 } 1823 skb = nskb; 1824 } So I do not understand how this can happen. With my patch: a) if skb is not shared -- it should keep original skb->sk b) if skb is shared -- new skb should set sk if it was set on original skb. Your results can be explained if you looked and skb->sk and truesize right after skb_realloc_headroom() call but before following skb_set_owner_w(). Could you please check it? Thank you, Vasily Averin