netdev - Re: [PATCH net-next v3] net: pppoe: implement GRO/GSO support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <706bb6df-7cf9-41d1-8041-4248252404dd@redhat.com>
Date: Thu, 14 Aug 2025 11:00:38 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Felix Fietkau <nbd@....name>, netdev@...r.kernel.org,
 Michal Ostrowski <mostrows@...thlink.net>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, David Ahern <dsahern@...nel.org>,
 Simon Horman <horms@...nel.org>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3] net: pppoe: implement GRO/GSO support

On 8/11/25 11:57 AM, Felix Fietkau wrote:
> @@ -1173,6 +1174,161 @@ static struct pernet_operations pppoe_net_ops = {
>  	.size = sizeof(struct pppoe_net),
>  };
>  
> +static u16
> +compare_pppoe_header(struct pppoe_hdr *phdr, struct pppoe_hdr *phdr2)
> +{
> +	return (__force __u16)((phdr->sid ^ phdr2->sid) |
> +			       (phdr->tag[0].tag_type ^ phdr2->tag[0].tag_type));

I'm sorry for the late feedback.

I see that the pppoe rcv() code ignores the type and ver fields, but I
guess it should be better to match them here, to ensure that the
segmented packet sequence matches the pre-aggregation one.

You could cast the phdr* to u32* and compare such integer.

> +}
> +
> +static __be16 pppoe_hdr_proto(struct pppoe_hdr *phdr)
> +{
> +	switch (phdr->tag[0].tag_type) {
> +	case cpu_to_be16(PPP_IP):
> +		return cpu_to_be16(ETH_P_IP);
> +	case cpu_to_be16(PPP_IPV6):
> +		return cpu_to_be16(ETH_P_IPV6);
> +	default:
> +		return 0;
> +	}
> +

Minor nit: unneeded empty line above

> +}
> +
> +static struct sk_buff *pppoe_gro_receive(struct list_head *head,
> +					 struct sk_buff *skb)
> +{
> +	const struct packet_offload *ptype;
> +	unsigned int hlen, off_pppoe;
> +	struct sk_buff *pp = NULL;
> +	struct pppoe_hdr *phdr;
> +	struct sk_buff *p;
> +	int flush = 1;
> +	__be16 type;
> +
> +	off_pppoe = skb_gro_offset(skb);
> +	hlen = off_pppoe + sizeof(*phdr);
> +	phdr = skb_gro_header(skb, hlen + 2, off_pppoe);
> +	if (unlikely(!phdr))
> +		goto out;
> +
> +	/* ignore packets with padding or invalid length */
> +	if (skb_gro_len(skb) != be16_to_cpu(phdr->length) + hlen)
> +		goto out;

What about filtering for phdr->code == 0 (session data) to avoid useless
late processing?

Thanks,

Paolo