From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 63DFFC433E7 for ; Tue, 13 Oct 2020 16:50:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 13B8C252DC for ; Tue, 13 Oct 2020 16:50:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1602607845; bh=UzdiRRE828PH0FyGjY1BxSN00JwHbWSHKpnKAGihyyo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=dIgepqrgmbwfKzSZSjGu3rTJu1qm/sd4ggxbmTBeeH4zxfolMjyVEiUFQBqcu8T+c wyS7pjjg6qW2kq9AKtpY4PDKxmb2h89AjOpSH/QLWSDVZHXk7NIeTZCSAK8Bv/7sWx POjcFZqhfH4/emSvvKt1Rm0FNWgXmllBK1AQ2wD0= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728521AbgJMQum (ORCPT ); Tue, 13 Oct 2020 12:50:42 -0400 Received: from mail.kernel.org ([198.145.29.99]:33688 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727696AbgJMQum (ORCPT ); Tue, 13 Oct 2020 12:50:42 -0400 Received: from kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com (unknown [163.114.132.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 603F4252DA; Tue, 13 Oct 2020 16:50:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1602607841; bh=UzdiRRE828PH0FyGjY1BxSN00JwHbWSHKpnKAGihyyo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=JthnFAEiOjojkDtYrg5+vYMr4MBL+WwWol21n0YV5LRb/LRkp3OkughkYTXt43sTb rO6dQ16m/DBvanVTS4NhW/ALlfDRysObBGjoftWZEj05kWm/Q55346gtTz2jne3DaL Lj/XQGrWJWl+wC2WybYKXTb0QrV1KRzt2eK7tggs= Date: Tue, 13 Oct 2020 09:50:38 -0700 From: Jakub Kicinski To: Aleksandr Nogikh Cc: Dmitry Vyukov , David Miller , Johannes Berg , Eric Dumazet , Andrey Konovalov , Marco Elver , LKML , netdev , linux-wireless , Aleksandr Nogikh , Florian Westphal Subject: Re: [PATCH 1/2] net: store KCOV remote handle in sk_buff Message-ID: <20201013095038.61ba8f55@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> In-Reply-To: References: <20201007101726.3149375-1-a.nogikh@gmail.com> <20201007101726.3149375-2-a.nogikh@gmail.com> <20201009161558.57792e1a@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> <20201010081431.1f2d9d0d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, 13 Oct 2020 18:59:28 +0300 Aleksandr Nogikh wrote: > On Mon, 12 Oct 2020 at 09:04, Dmitry Vyukov wrote: > > > > On Sat, Oct 10, 2020 at 5:14 PM Jakub Kicinski wrote: > > > > > > On Sat, 10 Oct 2020 09:54:57 +0200 Dmitry Vyukov wrote: > > > > On Sat, Oct 10, 2020 at 1:16 AM Jakub Kicinski wrote: > [...] > > > > > Could you use skb_extensions for this? > > > > > > > > Why? If for space, this is already under a non-production ifdef. > > > > > > I understand, but the skb_ext infra is there for uncommon use cases > > > like this one. Any particular reason you don't want to use it? > > > The slight LoC increase? > > > > > > Is there any precedent for adding the kcov field to other performance > > > critical structures? > > It would be great to come to some conclusion on where exactly to store > kcov_handle. Technically, it is possible to use skb extensions for the > purpose, though it will indeed slightly increase the complexity. > > Jakub, you think that kcov_handle should be added as an skb extension, > right? That'd be preferable. I understand with current use cases it doesn't really matter, but history shows people come up with all sort of wonderful use cases down the line. And when they do they rarely go back and fix such fundamental minutiae. > Though I do not really object to moving the field, it still seems to > me that sk_buff itself is a better place. Right now skb extensions > store values that are local to specific protocols and that are only > meaningful in the context of these protocols (correct me if I'm > wrong). Although this patch only adds remote kcov coverage to the wifi > code, kcov_handle can be meaningful for other protocols as well - just > like the already existing sk_buff fields. So adding kcov_handle to skb > extensions will break this logical separation. It's not as much protocols as subsystems. The values are meaningful to a subsystem which inserts them, that doesn't mean single layer of the stack. If it was about storing layer's context we would just use skb->cb. So I think the kcov use matches pretty well. Florian, what's your take?