Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
MIME-Version: 1.0
References: <20231202161441.221135-1-syoshida@redhat.com> <SJ0PR18MB5216A25BD74AE376FB1E536BDB87A@SJ0PR18MB5216.namprd18.prod.outlook.com>
In-Reply-To: <SJ0PR18MB5216A25BD74AE376FB1E536BDB87A@SJ0PR18MB5216.namprd18.prod.outlook.com>
From:   Eric Dumazet <edumazet@google.com>
Date:   Sun, 3 Dec 2023 12:03:07 +0100
Message-ID: <CANn89iLaCrBGdtVSPZMLM7tnixFfy3wF98aojxkcoXoXEit6og@mail.gmail.com>
Subject: Re: [EXT] [PATCH net v2] ipv4: ip_gre: Avoid skb_pull() failure in ipgre_xmit()
To:     Suman Ghosh <sumang@marvell.com>
Cc:     Shigeru Yoshida <syoshida@redhat.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "dsahern@kernel.org" <dsahern@kernel.org>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sun, Dec 3, 2023 at 7:58=E2=80=AFAM Suman Ghosh <sumang@marvell.com> wro=
te:
>
> Hi Shigeru,
>
> >diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index
> >22a26d1d29a0..5169c3c72cff 100644
> >--- a/net/ipv4/ip_gre.c
> >+++ b/net/ipv4/ip_gre.c
> >@@ -635,15 +635,18 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb,
> >       }
> >
> >       if (dev->header_ops) {
> >+              int pull_len =3D tunnel->hlen + sizeof(struct iphdr);
> >+
> >               if (skb_cow_head(skb, 0))
> >                       goto free_skb;
> >
> >               tnl_params =3D (const struct iphdr *)skb->data;
> >
> >-              /* Pull skb since ip_tunnel_xmit() needs skb->data pointi=
ng
> >-               * to gre header.
> >-               */
> >-              skb_pull(skb, tunnel->hlen + sizeof(struct iphdr));
> >+              if (!pskb_network_may_pull(skb, pull_len))
> [Suman] Since this is transmit path, should we add unlikely() here?

Adding unlikely() is not needed, it is already done generically from
the inline helpers.

Reviewed-by: Eric Dumazet <edumazet@google.com>