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=-1.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 AC456C43387 for ; Thu, 17 Jan 2019 15:41:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 77B9A20851 for ; Thu, 17 Jan 2019 15:41:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="C1h7e7tU" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728070AbfAQPl5 (ORCPT ); Thu, 17 Jan 2019 10:41:57 -0500 Received: from mail-ed1-f66.google.com ([209.85.208.66]:37569 "EHLO mail-ed1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728044AbfAQPl4 (ORCPT ); Thu, 17 Jan 2019 10:41:56 -0500 Received: by mail-ed1-f66.google.com with SMTP id h15so8719662edb.4 for ; Thu, 17 Jan 2019 07:41:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=/43PbGzUFMmfdlSRiR7ANGk/ZJehHdUfQEOle9CwR+g=; b=C1h7e7tUmTyjnJRUI2IuANAS8uZte2Rkw3zeVmA8Lhc0wXnp41y+bYmERPjzxk5cGv wDYjuAkIm45uyyFmi5vjdl9h1N24lyzoSR/LxSnG6aimxKL1j1LEYl98w0rkXYOVL4Q+ Y4n6iH1c1BeoubT109F/JsYVn3ZZLezq1EPKo7dcJY5OXnyTeBxA8+f/kFmqrp/517ri UzbPRK00gIJrzc1NotmWz4zGVID0CI1le/E77YXwwKGdYnTNFMk56W1anSovtLeJ8dfq yG1fHbN+q6/XnR8C2hOkuO2gRMa8kh7mdg8r1ceiYA4fDyJyqbg0Wjuy93U0CwaKs7/+ XGYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=/43PbGzUFMmfdlSRiR7ANGk/ZJehHdUfQEOle9CwR+g=; b=icAlB5oGoY7vlOUmIBsGXu0PB1ICLNfb4YsbdvdlX176kbA3CCv/t+N4i/8K6wqE3F CfBihGKTWmAcZoviC8yDbsSl+xUfkagD3tLkwHJmTF1tsrzr/AvUN3kbbsR/7NzMGsMX a157noN8BD4W9ZlpUgjovsw7bLOMssAwch8tFWP3k8dSiFzQ6iaJljpgQNviCRf6d/aM CHBeSTzZb/A5MfUFY2L7zdik6FONamZUq3dxQ2E9uIiKMkqHn21nJJaBpDVpIzeySGZE 24X8G7iMklYbEhcrXSgFDdAp46r44JegKntmzPPfUYQXc7PKllEBGfJalvFcgGmTUVuO RPWw== X-Gm-Message-State: AJcUukeHrZPw/rOv9OTCuBFx0h0bWZ0Vzu6CECAIDxm1+eJ9XXkgyqGa Sv73FjfdBtJWTpALcTyGRm7G4sG7Oix2I4zUZQg= X-Google-Smtp-Source: ALg8bN46xhjn4dENmyglcS5S/NIOMq4hDIFbF3mDSV7Qbo00LywrV1IDHZofxqwLmmyaLdqttdDUkqUTA3pJnyr3kL8= X-Received: by 2002:a17:906:61cc:: with SMTP id t12-v6mr10728214ejl.181.1547739714266; Thu, 17 Jan 2019 07:41:54 -0800 (PST) MIME-Version: 1.0 References: <20190114131841.1932-1-maximmi@mellanox.com> <20190114131841.1932-5-maximmi@mellanox.com> In-Reply-To: From: Willem de Bruijn Date: Thu, 17 Jan 2019 10:41:18 -0500 Message-ID: Subject: Re: [PATCH 4/7] net/packet: Ask driver for protocol if not provided by user To: Maxim Mikityanskiy Cc: "David S. Miller" , Saeed Mahameed , Willem de Bruijn , Jason Wang , Eric Dumazet , "netdev@vger.kernel.org" , Eran Ben Elisha , Tariq Toukan Content-Type: text/plain; charset="UTF-8" Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Jan 17, 2019 at 4:10 AM Maxim Mikityanskiy wrote: > > > > > +static void packet_parse_headers(struct sk_buff *skb, struct socket > > *sock) > > > > +{ > > > > + if (!skb->protocol && sock->type == SOCK_RAW) { > > > > + skb_reset_mac_header(skb); > > > > + skb->protocol = dev_parse_header_protocol(skb); > > > > + } > > > > + > > > > + skb_try_probe_transport_header(skb); > > > > +} > > > > > > > > > In relation to the discussion at > > > > > > af_packet: fix raw sockets over 6in4 tunnel > > > http://patchwork.ozlabs.org/patch/1023623/ > > > > > > if adding a new header_ops callback to parse link layer headers, > > > please have it return both protocol and link layer header length. > > Sorry, I miss the point here, can you elaborate more? If all you need is > to have some header_ops callback that returns the L2 header length, > there is one already, it's called parse. Or do you have a specific > reason why you want my callback to also return the header length? The main reason is to avoid multiple indirect function calls, both essentially doing the same: parsing the ll header. But admittedly the instances where dev->header_ops->validate are called are rare. > > This could just be an extension of existing header_ops->validate. > > If you suggest extending an existing function, parse looks more > suitable, but I decided not to touch the existing ones for two reasons: > > 1. I don't want to break the existing code that uses the parse function > and will need to be modified to pass an extra parameter. > > 2. I don't want to spend machine time on copying the destination MAC > when I only need the protocol, and vice versa. > > I'm looking forward to hearing your thoughts about it. header_ops.parse is also a good candidate. As a matter of fact, parse and validate could (eventually) probably be combined. The only direct caller to header_ops.parse appears to be dev_parse_header, so modifying its interface should be fairly straightforward. Allowing a NULL haddr could avoid the address copy cost if not needed. This does require modifying all implementations. But from a quick scan, there appear to be only 8. And only 1 for validate. So changing the implementation is quite acceptable. Another issue, though, would be what to return as protocol if a header does not encode that. Given these non-trivial changes, if you prefer to just add the dedicated new callback, that's fine. We can see independently whether deduplication makes sense. With three ll header parse functions, I think that it will be. But even if so, it is better to do so as a stand-alone noop patch than combining refactoring and new features, anyway. Long story short, sounds good. Thanks.