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

On 4/28/26 8:47 AM, Qingfang Deng wrote:
> Add PPPoE test-cases to the GRO selftest. Only run a subset of
> common_tests to avoid changing the hardcoded L3 offsets everywhere.
> Add a new "pppoe_sid" test case to verify that packets with different
> PPPoE session IDs are correctly identified as separate flows and not
> coalesced.
> 
> Signed-off-by: Qingfang Deng <[email protected]>
> ---
> v7:
>  - Do not run all the tests for PPPoE
>  - Add a new test for PPPoE
> v6: https://lore.kernel.org/netdev/[email protected]
> 
>  tools/testing/selftests/drivers/net/config |  2 +
>  tools/testing/selftests/drivers/net/gro.py | 11 +++
>  tools/testing/selftests/net/lib/gro.c      | 99 ++++++++++++++++++----
>  3 files changed, 96 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/testing/selftests/drivers/net/config b/tools/testing/selftests/drivers/net/config
> index fd16994366f4..07e386895b94 100644
> --- a/tools/testing/selftests/drivers/net/config
> +++ b/tools/testing/selftests/drivers/net/config
> @@ -8,5 +8,7 @@ CONFIG_NETCONSOLE=m
>  CONFIG_NETCONSOLE_DYNAMIC=y
>  CONFIG_NETCONSOLE_EXTENDED_LOG=y
>  CONFIG_NETDEVSIM=m
> +CONFIG_PPP=y
> +CONFIG_PPPOE=y
>  CONFIG_VLAN_8021Q=m
>  CONFIG_XDP_SOCKETS=y
> diff --git a/tools/testing/selftests/drivers/net/gro.py b/tools/testing/selftests/drivers/net/gro.py
> index 221f27e57147..ad7c80f7ba96 100755
> --- a/tools/testing/selftests/drivers/net/gro.py
> +++ b/tools/testing/selftests/drivers/net/gro.py
> @@ -313,6 +313,12 @@ def _gro_variants():
>          "ip_frag6", "ip_v6ext_same", "ip_v6ext_diff",
>      ]
>  
> +    # Tests specific to PPPoE
> +    pppoe_tests = [
> +        "data_same", "data_lrg_sml", "data_sml_lrg", "data_lrg_1byte",
> +        "data_burst", "pppoe_sid",
> +    ]
> +
>      for mode in ["sw", "hw", "lro"]:
>          for protocol in ["ipv4", "ipv6", "ipip", "ip6ip6"]:
>              for test_name in common_tests:
> @@ -325,6 +331,11 @@ def _gro_variants():
>                  for test_name in ipv6_tests:
>                      yield mode, protocol, test_name
>  
> +    for mode in ["sw"]:
> +        for protocol in ["pppoev4", "pppoev6"]:
> +            for test_name in pppoe_tests:
> +                yield mode, protocol, test_name
> +
>  
>  @ksft_variants(_gro_variants())
>  def test(cfg, mode, protocol, test_name):
> diff --git a/tools/testing/selftests/net/lib/gro.c b/tools/testing/selftests/net/lib/gro.c
> index 11b16ae5f0e8..0da55b757bcc 100644
> --- a/tools/testing/selftests/net/lib/gro.c
> +++ b/tools/testing/selftests/net/lib/gro.c
> @@ -67,12 +67,14 @@
>  #include <errno.h>
>  #include <error.h>
>  #include <getopt.h>
> +#include <net/ethernet.h>
> +#include <net/if.h>
>  #include <linux/filter.h>
>  #include <linux/if_packet.h>
> +#include <linux/if_pppox.h>
>  #include <linux/ipv6.h>
>  #include <linux/net_tstamp.h>
> -#include <net/ethernet.h>
> -#include <net/if.h>
> +#include <linux/ppp_defs.h>
>  #include <netinet/in.h>
>  #include <netinet/ip.h>
>  #include <netinet/ip6.h>
> @@ -134,6 +136,7 @@ static int total_hdr_len = -1;
>  static int ethhdr_proto = -1;
>  static bool ipip;
>  static bool ip6ip6;
> +static bool pppoe;
>  static uint64_t txtime_ns;
>  static int num_flows = 4;
>  static bool order_check;
> @@ -171,6 +174,22 @@ static void vlog(const char *fmt, ...)
>  	}
>  }
>  
> +static void fill_pppoelayer(void *buf, int payload_len, uint16_t sid)
> +{
> +	struct pppoe_ppp_hdr {
> +		struct pppoe_hdr eh;
> +		__be16 proto;
> +	} *ph = buf;
> +
> +	payload_len += sizeof(struct tcphdr);
> +	ph->eh.type = 1;
> +	ph->eh.ver = 1;
> +	ph->eh.code = 0;
> +	ph->eh.sid = htons(sid);
> +	ph->eh.length = htons(payload_len + sizeof(ph->proto));
> +	ph->proto = htons(proto == PF_INET ? PPP_IP : PPP_IPV6);
> +}
> +
>  static void setup_sock_filter(int fd)
>  {
>  	const int dport_off = tcp_offset + offsetof(struct tcphdr, dest);
> @@ -412,11 +431,15 @@ static void create_packet(void *buf, int seq_offset, int ack_offset,
>  
>  	fill_networklayer(buf + inner_ip_off, payload_len, IPPROTO_TCP);
>  	if (inner_ip_off > ETH_HLEN) {
> -		int encap_proto = (proto == PF_INET) ?
> -				  IPPROTO_IPIP : IPPROTO_IPV6;
> +		if (pppoe) {
> +			fill_pppoelayer(buf + ETH_HLEN, payload_len + ip_hdr_len, 0x1234);
> +		} else {
> +			int encap_proto = (proto == PF_INET) ?
> +					  IPPROTO_IPIP : IPPROTO_IPV6;
>  
> -		fill_networklayer(buf + ETH_HLEN,
> -				  payload_len + ip_hdr_len, encap_proto);
> +			fill_networklayer(buf + ETH_HLEN,
> +					  payload_len + ip_hdr_len, encap_proto);
> +		}
>  	}
>  
>  	fill_datalinklayer(buf);
> @@ -526,7 +549,7 @@ static void send_flags(int fd, struct sockaddr_ll *daddr, int psh, int syn,
>  static void send_data_pkts(int fd, struct sockaddr_ll *daddr,
>  			   int payload_len1, int payload_len2)
>  {
> -	static char buf[ETH_HLEN + IP_MAXPACKET];
> +	static char buf[MAX_HDR_LEN + IP_MAXPACKET];

Why MAX_HDR_LEN? I think you should add a new define alike:

#define L2_HLEN_MAX	(ETH_HLEN + PPPOE_SES_HLEN)

/P