Received: by 2002:a05:7412:b10a:b0:f3:1519:9f41 with SMTP id az10csp1217757rdb; Fri, 1 Dec 2023 09:51:28 -0800 (PST) X-Google-Smtp-Source: AGHT+IFCXS6EyRtzxsOsxYJzJy48o/HxuODwOeCdx8nb4yaUKhlyKw4lvpIcO3COBfHwcPWb+gUW X-Received: by 2002:a05:6a20:d704:b0:189:c0e1:4979 with SMTP id iz4-20020a056a20d70400b00189c0e14979mr33275506pzb.29.1701453088154; Fri, 01 Dec 2023 09:51:28 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701453088; cv=none; d=google.com; s=arc-20160816; b=G9fpGpttQbV23azbEUNH0H+BQcjOgrCUaSQnTM0yTWKTi8VcKEbuGMf9qsbzeMuI0r SBPboRI5Enawsdd0JDiYbzJZKjb+DXSFihipz0hgGxLCtyL3GdfcpKYn4KijXRtcAsEz vGFwlRgQgk9A6wvtN020+Oxr6XWQir8jNd2qcUE5Y3mA7vaAFAsNtkHOmyfBeMUcIRtw EOPu04Un0Gbpa4tJJkaBp5hL+JC6R6hSEwaWzVlCG8P8uqAT3BqYzoJY86FbN16EjuCo KLjm6ctf9pyxGSaysULRxuuGsBELRduLtRYDX/hJdxR5YBr5r8y/Dqo+8YklZ/xOLJnY 9/0g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:cc:to:subject :message-id:date:from:in-reply-to:references:mime-version :dkim-signature; bh=n3pnThC3OPQt1u7swR/CEmmm2L1BbqHWt2vccyU+wqY=; fh=O4xcZbnTvYIkvIcg+CmJI5R4FuZXAq/ZFrTIQWC0Mvw=; b=QFWfVmACFgbYieb5FDEeUAP47l4KV3lraAAP00IFBYKhT9zoPCsd5W18GwCWhxbEDL f2MrYOCVn6UUJdd/dBN0ogxbOFQlZCwHhefKF/Bs9ORl/tkXl3dq/4DQfQlKihVi5pTV al12WiqIuDy8wWDYJ23JUkgampNyv1KQJpq+jdW0C69BR77R79PtcQHPvSCfEaqT4eYR eS0Of67tcNTejY7UPZthKUwCB4arx/7WhGYcFL/WZjBuycR4xkpiNc8eh3x+ntaJ9cfL lhpXgRuS+hadwkKI1Aq3yeK2r2jOBQwdkWrNUl6vpxAqwn9K77hiB+W+ibLTgmlVX939 1FRg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=dFnb0GoW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from howler.vger.email (howler.vger.email. [23.128.96.34]) by mx.google.com with ESMTPS id cl19-20020a056a02099300b005aaefc07ccfsi3991599pgb.36.2023.12.01.09.51.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Dec 2023 09:51:28 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=dFnb0GoW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by howler.vger.email (Postfix) with ESMTP id 76B9A828FAF5; Fri, 1 Dec 2023 09:51:25 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at howler.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1378448AbjLARvK (ORCPT + 99 others); Fri, 1 Dec 2023 12:51:10 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44658 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229454AbjLARvJ (ORCPT ); Fri, 1 Dec 2023 12:51:09 -0500 Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0C8F5AD for ; Fri, 1 Dec 2023 09:51:15 -0800 (PST) Received: by mail-ed1-x52b.google.com with SMTP id 4fb4d7f45d1cf-54c77d011acso75a12.1 for ; Fri, 01 Dec 2023 09:51:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1701453073; x=1702057873; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=n3pnThC3OPQt1u7swR/CEmmm2L1BbqHWt2vccyU+wqY=; b=dFnb0GoWAqwCCJgSyyNchhPslfWB1/pjpjSaF7b3RaPqQkAlf8xrITTLEF+WWgGI4t fASC1mMdLnQQ9/xQu56JXkMlcQzF7whgRuX80ZsDwxmYJtTCRhjTuWVj9uT1gwbwE3Eu d3oLDOuN33ic1mD5qvdgXQ+13E22lROqJ+zGIz6SlPe9BQ6g9jTMnBaST8vWsjTlTOWZ 12vo0Bwinsodc4bCxJnqprKV8U7Qlcv9lrLEjE4JSx7XPj3egtAvuRUhdHkh5yKj+cpT FvPKoufQOvhqjQ5wTcL4SNgs8b3JYjG7/NDxYHlkVdJEYPp2yPUffjJDREfM9nTjEUYQ NVCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701453073; x=1702057873; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=n3pnThC3OPQt1u7swR/CEmmm2L1BbqHWt2vccyU+wqY=; b=NgehJbMPqvzxp8okSaQJ0Zxm3njzvxjvCpEldReh4FWuxV3ZTcvHW5BZ3BsUOOPSlh Zyl5syxKpIOPyb+wu4mSoNigAmL+oQTASk4uDoGfgBXsdfdAz59SjTVicHFFLFilBoCQ TCtTlV6fMa7SPZ7qgwd10ibvHC4NbAsIvE5vxZonN7xKgTVDmhVARaBRPHst28o7d+Ct mQXpU2sAXKwIm02KTBG1tCbNYkef1I7tiOWJ/FYdOB24CxaaD4+8rMEQ1B1t4MxTKRfc nl5aIURVwEp4/jggmGDDKk7TcebB0kgn/fY23EjeRKVzph+fwA30ONcSVU+RKkHTgpuT FVmQ== X-Gm-Message-State: AOJu0YzuEzax1pAFh9wIBEZivkV84pDdDtd4yHEoNUqXwSuxRGdHtjaQ dK1W5bAbgC4jV1PkQQrW5k9v3OXJWUL7Ybd4bDd3lg== X-Received: by 2002:a50:aacf:0:b0:54b:321:ef1a with SMTP id r15-20020a50aacf000000b0054b0321ef1amr152058edc.6.1701453073216; Fri, 01 Dec 2023 09:51:13 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Eric Dumazet Date: Fri, 1 Dec 2023 18:51:02 +0100 Message-ID: Subject: Re: [PATCH v2 net-next] packet: add a generic drop reason for receive To: Yan Zhai Cc: netdev@vger.kernel.org, "David S. Miller" , Jakub Kicinski , Paolo Abeni , Willem de Bruijn , Weongyo Jeong , linux-kernel@vger.kernel.org, kernel-team@cloudflare.com, Jesper Brouer Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.4 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on howler.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Fri, 01 Dec 2023 09:51:25 -0800 (PST) On Fri, Dec 1, 2023 at 6:43=E2=80=AFPM Yan Zhai wrote: > > Commit da37845fdce2 ("packet: uses kfree_skb() for errors.") switches > from consume_skb to kfree_skb to improve error handling. However, this > could bring a lot of noises when we monitor real packet drops in > kfree_skb[1], because in tpacket_rcv or packet_rcv only packet clones > can be freed, not actual packets. > > Adding a generic drop reason to allow distinguish these "clone drops". > > [1]: https://lore.kernel.org/netdev/CABWYdi00L+O30Q=3DZah28QwZ_5RU-xcxLFU= K2Zj08A8MrLk9jzg@mail.gmail.com/ > Fixes: da37845fdce2 ("packet: uses kfree_skb() for errors.") > Suggested-by: Eric Dumazet > Signed-off-by: Yan Zhai > --- > include/net/dropreason-core.h | 6 ++++++ > net/packet/af_packet.c | 22 +++++++++++++--------- > 2 files changed, 19 insertions(+), 9 deletions(-) > > diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.= h > index 3c70ad53a49c..278e4c7d465c 100644 > --- a/include/net/dropreason-core.h > +++ b/include/net/dropreason-core.h > @@ -86,6 +86,7 @@ > FN(IPV6_NDISC_NS_OTHERHOST) \ > FN(QUEUE_PURGE) \ > FN(TC_ERROR) \ > + FN(PACKET_SOCK_ERROR) \ > FNe(MAX) > > /** > @@ -378,6 +379,11 @@ enum skb_drop_reason { > SKB_DROP_REASON_QUEUE_PURGE, > /** @SKB_DROP_REASON_TC_ERROR: generic internal tc error. */ > SKB_DROP_REASON_TC_ERROR, > + /** > + * @SKB_DROP_REASON_PACKET_SOCK_ERROR: generic packet socket erro= rs > + * after its filter matches an incoming packet. > + */ > + SKB_DROP_REASON_PACKET_SOCK_ERROR, > /** > * @SKB_DROP_REASON_MAX: the maximum of core drop reasons, which > * shouldn't be used as a real 'reason' - only for tracing code g= en > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index a84e00b5904b..0a7c05d8fe9f 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -2128,6 +2128,7 @@ static int packet_rcv(struct sk_buff *skb, struct n= et_device *dev, > int skb_len =3D skb->len; > unsigned int snaplen, res; > bool is_drop_n_account =3D false; Why keeping is_drop_n_account then ? > + enum skb_drop_reason drop_reason =3D SKB_CONSUMED; > > if (skb->pkt_type =3D=3D PACKET_LOOPBACK) > goto drop; > @@ -2161,6 +2162,10 @@ static int packet_rcv(struct sk_buff *skb, struct = net_device *dev, > res =3D run_filter(skb, sk, snaplen); > if (!res) > goto drop_n_restore; > + > + /* skb will only be "consumed" not "dropped" before this */ > + drop_reason =3D SKB_DROP_REASON_PACKET_SOCK_ERROR; > + > if (snaplen > res) > snaplen =3D res; > > @@ -2227,10 +2232,7 @@ static int packet_rcv(struct sk_buff *skb, struct = net_device *dev, > skb->len =3D skb_len; > } > drop: > - if (!is_drop_n_account) Because after your patch it will be set but never read. > - consume_skb(skb); > - else > - kfree_skb(skb); > + kfree_skb_reason(skb, drop_reason); > return 0; > } > > @@ -2253,6 +2255,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct = net_device *dev, > bool is_drop_n_account =3D false; Same remark here. > unsigned int slot_id =3D 0; > int vnet_hdr_sz =3D 0; > + enum skb_drop_reason drop_reason =3D SKB_CONSUMED; > > /* struct tpacket{2,3}_hdr is aligned to a multiple of TPACKET_AL= IGNMENT. > * We may add members to them until current aligned size without = forcing > @@ -2355,6 +2358,10 @@ static int tpacket_rcv(struct sk_buff *skb, struct= net_device *dev, > vnet_hdr_sz =3D 0; > } > } > + > + /* skb will only be "consumed" not "dropped" before this */ > + drop_reason =3D SKB_DROP_REASON_PACKET_SOCK_ERROR; > + > spin_lock(&sk->sk_receive_queue.lock); > h.raw =3D packet_current_rx_frame(po, skb, > TP_STATUS_KERNEL, (macoff+snaplen= )); > @@ -2498,10 +2505,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct= net_device *dev, > skb->len =3D skb_len; > } > drop: > - if (!is_drop_n_account) > - consume_skb(skb); > - else > - kfree_skb(skb); > + kfree_skb_reason(skb, drop_reason); > return 0; > > drop_n_account: > @@ -2510,7 +2514,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct = net_device *dev, > is_drop_n_account =3D true; > > sk->sk_data_ready(sk); > - kfree_skb(copy_skb); > + kfree_skb_reason(copy_skb, drop_reason); > goto drop_n_restore; > } > > -- > 2.30.2 >