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

[Paolo Abeni <pabeni@redhat.com>]

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