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=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,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 4D9BEC433E7 for ; Sat, 10 Oct 2020 03:24:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DCC07221FF for ; Sat, 10 Oct 2020 03:24:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="p1hBXu33" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730800AbgJJDXA (ORCPT ); Fri, 9 Oct 2020 23:23:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58016 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730533AbgJJDKO (ORCPT ); Fri, 9 Oct 2020 23:10:14 -0400 Received: from mail-pg1-x533.google.com (mail-pg1-x533.google.com [IPv6:2607:f8b0:4864:20::533]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E40F5C0613D2 for ; Fri, 9 Oct 2020 20:10:13 -0700 (PDT) Received: by mail-pg1-x533.google.com with SMTP id r10so8790722pgb.10 for ; Fri, 09 Oct 2020 20:10:13 -0700 (PDT) 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=DnmiEoCVE3FOWworSeIf6fY0KcSoT2FqH+xdEj47E94=; b=p1hBXu33Ek1EQG2Kv4RGIC43hAvpzLCYjr6pesSSuDDAGheP3qz5T4BxPcgsMv5QOw FjRByHQPgwgGPIRbVQMSs7qiKIa1uiAGfQLEEhBVnjQu8jBBsDJvrGYTDe/b3NSJy18X Ug7YRCC3NG5f9AUUEEyzVLzhw7ShWE78AHnxo/PIjm1cAFrH4T8AR6FkAuhxjMsgd0nx qPsE9XJ4AFC1vLYSeEq9h8vNIpjfJipBrOHgxfXkpS3KcFCu/0SDfJHMykgyZNsWf/rH HqloF0TJHlLvoDIXiF0cd11TPsVp6bY11jz71pl7R19bWNExLVFW6poBMb73/TvQP+Oe pIcQ== 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=DnmiEoCVE3FOWworSeIf6fY0KcSoT2FqH+xdEj47E94=; b=dRAS+x0tFJLf+LoDCSFWqdpDRaq7On/V0PTzqn0TCbcchR8ZSOblYNmU/Pttv+6+k8 nvFck68bvqYMd1ekz4uA4iocRq4+QXO1P9dr5ggFKLRh9DgPvUOkkL9dda44zPFXIT/g nxseBAMKGPQlF7p3Q/YjbxoXBlTVfOhxsCm0Q+eklG5I/dFVjviLEAh5bzyDpYjOC7gk W3SbV7qgYekgNvcRKItzOhd8EwxyIuUvxHFpJQKXICgkGZvC7x/tiDjXjk1/7ybBdaO2 O+TGZoGSJNrLgPDfW+v1PU6JNLt+X9NAY3yzMTiV9+/Z+PZIH+Sv3z7EzzTO01g97b0b snmA== X-Gm-Message-State: AOAM532bS3ME/gVYV5R/4FKg4J/gh/Ziy/v2WsUsVj+95NrP+BdJJMSY up5D5vXYGEfpIZI+S/fF0g1B8kXZVqC0mMWbktKn80tXG74= X-Google-Smtp-Source: ABdhPJyGy61JCGkjmFB/3aj2TykMWPPz/7FF/mUtgyp19HS3zrsigKukc31RbLjai+WJBpvTBsId9AuD6y1qTLpDj2M= X-Received: by 2002:aa7:96f8:0:b029:152:94c0:7e5 with SMTP id i24-20020aa796f80000b029015294c007e5mr14780330pfq.76.1602299413311; Fri, 09 Oct 2020 20:10:13 -0700 (PDT) MIME-Version: 1.0 References: <20201008012154.11149-1-xiyou.wangcong@gmail.com> In-Reply-To: From: Xie He Date: Fri, 9 Oct 2020 20:10:02 -0700 Message-ID: Subject: Re: [Patch net] ip_gre: set dev->hard_header_len properly To: Cong Wang Cc: Willem de Bruijn , Network Development , syzbot , William Tu Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Oct 9, 2020 at 6:07 PM Cong Wang wrote: > > Looking a bit deeper, I doubt the ipgre_header_ops->create is necessary, > because 1) all other tunnels devices do not have it (ip_tunnel_header_ops > only contains ->parse_protocol); 2) GRE headers are pushed in xmit > anyway, so at least SOCK_DGRAM does not need it; 3) ipgre_header() > creates the GRE header, later ipgre_xmit() pulls it back, then __gre_xmit() > builds GRE header again... Haha, I also don't like ipgre_header_ops->create and want to get rid of it. We are thinking the same :) >From what I see, the flow when sending skbs (when header_ops is used) is like this: 1) First ipgre_header creates the IP header and the GRE base header, leaving space for the GRE optional fields and the "encap" header (the space for the "encap" header should be left before the GRE header, but it is incorrectly left after the GRE header). 2) Then ipgre_xmit pulls the header created by ipgre_header (but leaves the data there). Then it calls __gre_xmit. 3) __gre_xmit calls gre_build_header. gre_build_header will push back the GRE header, read the GRE base header and build the GRE optional fields. 4) Then __gre_xmit calls ip_tunnel_xmit. ip_tunnel_xmit will build the "encap" header by calling ip_tunnel_encap. So what ipgre_header does is actually creating the IP header and the GRE header, and leaving some holes for the GRE optional fields and the "encap" header to be built later. This seems so weird to me. If a user is using an AF_PACKET/RAW socket, the user is supposed to do what the header_ops->create function does (that is, creating two headers and leaving two holes to be filled in later). I think no user would actually do that. That is so weird. Also you said, all other tunnel devices didn't have header_ops->create. I think that is a good argument for getting rid of header_ops->create, too. Also, for IP GRE TAP devices (as I understand, these devices are just like IP GRE devices except it tunnels Ethernet frames instead of network-layer packets), its header_ops will be Ethernet's eth_header_ops (ipgre_tap_setup calls ether_setup, which sets up the Ethernet header_ops). In this case, I think it is logical to keep the header_ops for IP GRE devices as NULL. So that the header_ops of IP GRE devices is consistent with that of IP GRE TAP devices.