On Mon, Mar 30, 2026 at 8:19 PM Martin KaFai Lau <[email protected]> wrote: > > On 3/30/26 6:35 PM, Kuniyuki Iwashima wrote: > > On Mon, Mar 30, 2026 at 2:26 AM Jiayuan Chen <[email protected]> wrote: > >> > >> > >> On 3/30/26 5:00 PM, [email protected] wrote: > >>>> #if IS_BUILTIN(CONFIG_IPV6) > >>>> case htons(ETH_P_IPV6): > >>>> + if (ipv6_hdr(skb)->nexthdr != IPPROTO_TCP) > >>>> + return -EINVAL; > >>>> + > >>>> ops = &tcp6_request_sock_ops; > >>> For IPv6, ipv6_hdr(skb)->nexthdr gives the type of the header > >>> immediately following the base IPv6 header. If extension headers > >>> are present (hop-by-hop options, routing, etc.), nexthdr would be > >>> the extension header type rather than IPPROTO_TCP, even when the > >>> packet is a valid TCP segment. > >>> > >>> Would it be worth using ipv6_find_hdr() here, similar to > >>> bpf_update_srh_state() in the same file, to walk past any extension > >>> headers? The IPv4 check above is fine since ip_hdr(skb)->protocol > >>> always identifies the transport protocol directly. > >>> > >>> In practice this is very unlikely to matter since TCP SYN packets > >>> with IPv6 extension headers are essentially non-existent, but the > >>> check as written would reject them. > >>> > >>> > >>> --- > >>> AI reviewed your patch. Please fix the bug or email reply why it's not a > >>> bug. > >>> See:https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md > >>> > >>> CI run > >>> summary:https://github.com/kernel-patches/bpf/actions/runs/23735111188 > >> > >> > >> Thanks for the analysis. > >> > >> There are many places in the kernel that check nexthdr directly without > >> walking extension headers. > >> > >> I'd prefer to keep it simple for now and leave ipv6_find_hdr() as a > >> potential future improvement if needed. > > > > +1. > > > > Given this feature is to create a reqsk to process on this running > > host, it does not make sense to support such ext options. > > While at header reading, does it need a pskb_may_pull() before reading?
Ah good point, now that we read skb->data, pskb_may_pull() is needed indeed.
