Re: [RFC PATCH net-next v6 2/2] selftests: net: test PPPoE packets in gro.sh

Willem de Bruijn
2026-03-26T16:29:09Z

Qingfang Deng wrote:
> Add PPPoE test-cases to the GRO selftest.
>
> Signed-off-by: Qingfang Deng <dqfext@gmail.com>
> ---
> v6: new patch
>
> I'm not sure if I should include all the existing IPv4/v6 tests for
> PPPoE. There are tests that hardcode ETH_HLEN as the NH offset, which
> are meant to test the L3 protocol, not the underlying protocol, and
> changing all of them seems too invasive.

Overall this change looks good to me. But indeed all the churn of

(pppoe ? PPPOE_SES_HLEN : 0)

is a bit annoying. Left a few representative snippets below.
I think that could be avoided similar to MAX_HDR_LEN by having a
global definition and using that everywhere. Still a lot of churn, but
at least no repeated open-coded logic.

Also consider whether running all these tests again in PPPoE adds
value, if not, then indeed trim the tests to those that truly add
code coverage.

> @@ -745,7 +777,7 @@ static void add_ipv4_ts_option(void *buf, void *optpkt)
> memcpy(optpkt + tcp_offset + optlen, buf + tcp_offset,
> sizeof(struct tcphdr) + PAYLOAD_LEN);
>
> - iph = (struct iphdr *)(optpkt + ETH_HLEN);
> + iph = (struct iphdr *)(optpkt + ETH_HLEN + (pppoe ? PPPOE_SES_HLEN : 0));
> iph->ihl = 5 + (optlen / 4);
> iph->tot_len = htons(ntohs(iph->tot_len) + optlen);
> iph->check = 0;
> @@ -755,7 +787,7 @@ static void add_ipv4_ts_option(void *buf, void *optpkt)
> static void add_ipv6_exthdr(void *buf, void *optpkt, __u8 exthdr_type, char *ext_payload)
> {
> struct ipv6_opt_hdr *exthdr = (struct ipv6_opt_hdr *)(optpkt + tcp_offset);
> - struct ipv6hdr *iph = (struct ipv6hdr *)(optpkt + ETH_HLEN);
> + struct ipv6hdr *iph = (struct ipv6hdr *)(optpkt + ETH_HLEN + (pppoe ? PPPOE_SES_HLEN : 0));
> char *exthdr_payload_start = (char *)(exthdr + 1);
>
> exthdr->hdrlen = 0;