All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@nvidia.com>
To: Anton Danilov <littlesmilingcloud@gmail.com>
Cc: netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	David Ahern <dsahern@kernel.org>, Simon Horman <horms@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v4 03/10] gre: make gre_parse_header() report a drop reason
Date: Wed, 23 Sep 2026 17:59:31 +0300	[thread overview]
Message-ID: <20260923145931.GA2859441@shredder> (raw)
In-Reply-To: <20260922221507.3268127-4-littlesmilingcloud@gmail.com>

On Wed, Sep 23, 2026 at 01:15:00AM +0300, Anton Danilov wrote:
> diff --git a/net/ipv4/gre_demux.c b/net/ipv4/gre_demux.c
> index 96fd7dc6d82d..e117056525f0 100644
> --- a/net/ipv4/gre_demux.c
> +++ b/net/ipv4/gre_demux.c
> @@ -56,28 +56,35 @@ int gre_del_protocol(const struct gre_protocol *proto, u8 version)
>  }
>  EXPORT_SYMBOL_GPL(gre_del_protocol);
>  
> -/* Fills in tpi and returns header length to be pulled.
> +/* Fills in tpi, including the header length to be pulled in tpi->hdr_len,
> + * and returns SKB_NOT_DROPPED_YET, or the reason to drop the packet if the
> + * header is rejected.
>   * Note that caller must use pskb_may_pull() before pulling GRE header.
> + *
> + * @icmp_err is set by the ICMP error handlers, which only get a part of
> + * the original packet: a checksum failure does not reject the header then,
> + * the checksum is still computed and the rest of the header is parsed.
>   */
> -int gre_parse_header(struct sk_buff *skb, struct tnl_ptk_info *tpi,
> -		     bool *csum_err, __be16 proto, int nhs)
> +enum skb_drop_reason
> +gre_parse_header(struct sk_buff *skb, struct tnl_ptk_info *tpi,
> +		 bool icmp_err, __be16 proto, int nhs)
>  {
>  	const struct gre_base_hdr *greh;
>  	__be32 *options;
>  	int hdr_len;
>  
>  	if (unlikely(!pskb_may_pull(skb, nhs + sizeof(struct gre_base_hdr))))
> -		return -EINVAL;
> +		return SKB_DROP_REASON_HDR_TRUNC;

Please use pskb_may_pull_reason() as is done throughout the kernel.
AFAICT, there's only one user of HDR_TRUNC (in tun) and it should also
be converted to pskb_may_pull_reason() so that we could remove
HDR_TRUNC. Can you send a patch?

>  
>  	greh = (struct gre_base_hdr *)(skb->data + nhs);
>  	if (unlikely(greh->flags & (GRE_VERSION | GRE_ROUTING)))
> -		return -EINVAL;
> +		return SKB_DROP_REASON_GRE_INVALID_HDR;

[...]

>  static int gre_rcv(struct sk_buff *skb)
>  {
> +	enum skb_drop_reason reason = SKB_DROP_REASON_NOT_SPECIFIED;
>  	const struct gre_protocol *proto;
>  	u8 ver;
>  	int ret;
>  
> -	if (!pskb_may_pull(skb, 12))
> +	reason = pskb_may_pull_reason(skb, 12);
> +	if (reason)
>  		goto drop;
>  
>  	ver = skb->data[1]&0x7f;
> -	if (ver >= GREPROTO_MAX)
> +	if (ver >= GREPROTO_MAX) {
> +		reason = SKB_DROP_REASON_UNHANDLED_PROTO;

This should be TUNNEL_INVALID_HDR to be consistent with the hunk above.

>  		goto drop;
> +	}
>  
>  	rcu_read_lock();
>  	proto = rcu_dereference(gre_proto[ver]);
> @@ -167,11 +177,11 @@ static int gre_rcv(struct sk_buff *skb)
>  drop_nohandler:
>  	rcu_read_unlock();
>  	dev_core_stats_rx_nohandler_inc(skb->dev);
> -	kfree_skb(skb);
> +	kfree_skb_reason(skb, SKB_DROP_REASON_UNHANDLED_PROTO);

This looks correct.

>  	return NET_RX_DROP;
>  drop:
>  	dev_core_stats_rx_dropped_inc(skb->dev);
> -	kfree_skb(skb);
> +	kfree_skb_reason(skb, reason);
>  	return NET_RX_DROP;
>  }

  reply	other threads:[~2026-09-23 15:00 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-22 22:14 [PATCH net-next v4 00/10] tunnels: add core and gre drop reasons Anton Danilov
2026-09-22 22:14 ` [PATCH net-next v4 01/10] ip_tunnel: add drop reasons to the generic RX path Anton Danilov
2026-09-22 22:14 ` [PATCH net-next v4 02/10] ip6_tunnel: " Anton Danilov
2026-09-22 22:15 ` [PATCH net-next v4 03/10] gre: make gre_parse_header() report a drop reason Anton Danilov
2026-09-23 14:59   ` Ido Schimmel [this message]
2026-09-25 23:12     ` Anton Danilov
2026-09-22 22:15 ` [PATCH net-next v4 04/10] ip_tunnel: add __iptunnel_pull_header_reason() Anton Danilov
2026-09-23 15:41   ` Ido Schimmel
2026-09-25 23:12     ` Anton Danilov
2026-09-28  6:46       ` Ido Schimmel
2026-09-22 22:15 ` [PATCH net-next v4 05/10] ip_gre: add drop reasons to the RX path Anton Danilov
2026-09-22 22:15 ` [PATCH net-next v4 06/10] ip6_gre: " Anton Danilov
2026-09-22 22:15 ` [PATCH net-next v4 07/10] ip_tunnel: add drop reasons to the transmit path Anton Danilov
2026-09-23 15:53   ` Ido Schimmel
2026-09-22 22:15 ` [PATCH net-next v4 08/10] ip_gre: " Anton Danilov
2026-09-22 22:15 ` [PATCH net-next v4 09/10] ip6_gre: make prepare_ip6gre_xmit_other() void Anton Danilov
2026-09-22 22:15 ` [PATCH net-next v4 10/10] ip6_tunnel: add drop reasons to the transmit path Anton Danilov
2026-09-23 14:19 ` [PATCH net-next v4 00/10] tunnels: add core and gre drop reasons Ido Schimmel
2026-09-25 23:11   ` Anton Danilov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260923145931.GA2859441@shredder \
    --to=idosch@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=littlesmilingcloud@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.