Re: [PATCH net-next v2 4/8] ip_gre: add drop reasons to the RX path
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-14 04:37:22
Also in:
lkml
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 0 · Low: 1 (1) - [Low] Claim: labelling GRE RX pull failures SKB_DROP_REASON_HDR_TRUNC misattributes allocation failures, both for the two new `if… Pre-existing issues: - [Low] Claim: the new family-agnostic SKB_DROP_REASON_GRE_TUNNEL_NOT_FOUND is wired up only in… -- ip_gre: add drop reasons to the RX path This adds SKB_DROP_REASON_GRE_TUNNEL_NOT_FOUND and threads a drop reason output parameter through erspan_rcv(), __ipgre_rcv() and ipgre_rcv() so that gre_rcv() can pass a specific reason to kfree_skb_reason(). Header pull failures are reported as SKB_DROP_REASON_HDR_TRUNC and metadata dst allocation failures as SKB_DROP_REASON_NOMEM. A couple of small questions below, both about the reason values rather than the plumbing.
quoted hunk ↗ jump to hunk
diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h index 6ae7a604722d3..fa8bd552122ff 100644 --- a/include/net/dropreason-core.h +++ b/include/net/dropreason-core.h
[ ... ]
quoted hunk ↗ jump to hunk
@@ -637,6 +638,11 @@ enum skb_drop_reason { SKB_DROP_REASON_GRE_INVALID_HDR, /** @SKB_DROP_REASON_GRE_CSUM: GRE checksum error */ SKB_DROP_REASON_GRE_CSUM, + /** + * @SKB_DROP_REASON_GRE_TUNNEL_NOT_FOUND: no GRE tunnel found for the + * endpoints and the key the packet carries. + */ + SKB_DROP_REASON_GRE_TUNNEL_NOT_FOUND,
[Severity: Low]
This isn't an issue introduced by this patch, but the new reason is
documented in family-neutral wording ("no GRE tunnel found for the
endpoints and the key") while only net/ipv4/ip_gre.c sets it in this
commit. Until the IPv6 side is converted, a GRE-over-IPv6 packet with no
matching tunnel still takes:
net/ipv6/ip6_gre.c:gre_rcv() -> ip6gre_rcv() -> kfree_skb_reason(skb,
SKB_DROP_REASON_NOT_SPECIFIED)
so ip6gre, ip6gretap and ERSPAN-over-IPv6 lookup failures are still
indistinguishable from unspecified drops.
For the record, this is closed later in the same series: the following
patch, "ip6_gre: add drop reasons to the RX path", converts
__ip6gre_rcv()/ip6gre_rcv() and ip6erspan_rcv() to the same output
parameter scheme and ends both lookup-failure paths with
*reason = SKB_DROP_REASON_GRE_TUNNEL_NOT_FOUND. Would it be worth
mentioning in this changelog that the IPv6 counterpart follows, so the
intermediate state is not read as a scope gap?
quoted hunk ↗ jump to hunk
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index 1894c5746a737..4d9bb6d186ae5 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -356,15 +362,17 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi, ip_tunnel_rcv(tunnel, skb, tpi, tun_dst, log_ecn_error); return PACKET_RCVD; } + *reason = SKB_DROP_REASON_GRE_TUNNEL_NOT_FOUND; return PACKET_REJECT; drop: - kfree_skb(skb); + kfree_skb_reason(skb, SKB_DROP_REASON_HDR_TRUNC); return PACKET_RCVD; }
[Severity: Low]
The only way to reach this drop label in erspan_rcv() is the
__iptunnel_pull_header() failure, and that helper does not only fail on
truncation:
include/net/ip_tunnels.h:iptunnel_pull_offloads() {
if (skb_is_gso(skb)) {
int err;
err = skb_unclone(skb, GFP_ATOMIC);
if (unlikely(err))
return err;
...
}
So for a cloned GSO skb under memory pressure the drop is an allocation
failure but gets reported as SKB_DROP_REASON_HDR_TRUNC. The commit
message says "The header pull failures reuse SKB_DROP_REASON_HDR_TRUNC",
which covers the pskb_may_pull() cases well, but does it also intend to
cover the skb_unclone() case?
Note the errno cannot be used to tell the two apart, since
__iptunnel_pull_header() returns -ENOMEM for every failure mode, and
there is precedent for the coarse label in the other direction:
vxlan_rcv() tags every __iptunnel_pull_header() failure, truncation
included, as SKB_DROP_REASON_NOMEM. The two new
*reason = SKB_DROP_REASON_HDR_TRUNC assignments on the pskb_may_pull()
branches above match the HDR_TRUNC kernel-doc exactly, so this question
is only about the drop label.
quoted hunk ↗ jump to hunk
@@ -400,22 +408,25 @@ static int __ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi, tun_id = key32_to_tunnel_id(tpi->key); tun_dst = ip_tun_rx_dst(skb, flags, tun_id, 0); - if (!tun_dst) + if (!tun_dst) { + *reason = SKB_DROP_REASON_NOMEM; return PACKET_REJECT; + } } ip_tunnel_rcv(tunnel, skb, tpi, tun_dst, log_ecn_error); return PACKET_RCVD; } + *reason = SKB_DROP_REASON_GRE_TUNNEL_NOT_FOUND; return PACKET_NEXT; drop: - kfree_skb(skb); + kfree_skb_reason(skb, SKB_DROP_REASON_HDR_TRUNC); return PACKET_RCVD; }
The same question applies here: this drop label is reached only from __iptunnel_pull_header(skb, hdr_len, tpi->proto, raw_proto, false) < 0, so the skb_unclone(GFP_ATOMIC) failure inside iptunnel_pull_offloads() is also reported as SKB_DROP_REASON_HDR_TRUNC on the main IPv4 GRE receive path. [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260913034937.875068-1-littlesmilingcloud%40gmail.com