linux-kernel - Re: [PATCH net-next v2] net: pppoe: implement GRO 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: <46fd972c-14d9-4876-8df5-1212f6530971@intel.com>
Date: Fri, 25 Jul 2025 14:42:12 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Felix Fietkau <nbd@....name>
CC: Paolo Abeni <pabeni@...hat.com>, <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>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v2] net: pppoe: implement GRO support

From: Felix Fietkau <nbd@....name>
Date: Tue, 22 Jul 2025 10:56:10 +0200

> On 22.07.25 10:36, Paolo Abeni wrote:
>> On 7/16/25 10:14 AM, Felix Fietkau wrote:
>>> +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;
>>> +    __be16 type;
>>> +    int flush = 1;
>>
>> Minor nit: please respect the reverse christmas tree order above
> 
> Will do
> 
>>> +    off_pppoe = skb_gro_offset(skb);
>>> +    hlen = off_pppoe + sizeof(*phdr) + 2;
>>> +    phdr = skb_gro_header(skb, hlen, 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 - 2)
>>> +        goto out;
>>> +
>>> +    NAPI_GRO_CB(skb)->network_offsets[NAPI_GRO_CB(skb)->encap_mark]
>>> = hlen;
>>> +
>>> +    type = pppoe_hdr_proto(phdr);
>>> +    if (!type)
>>> +        goto out;
>>> +
>>> +    ptype = gro_find_receive_by_type(type);
>>> +    if (!ptype)
>>> +        goto out;
>>> +
>>> +    flush = 0;
>>> +
>>> +    list_for_each_entry(p, head, list) {
>>> +        struct pppoe_hdr *phdr2;
>>> +
>>> +        if (!NAPI_GRO_CB(p)->same_flow)
>>> +            continue;
>>> +
>>> +        phdr2 = (struct pppoe_hdr *)(p->data + off_pppoe);
>>> +        if (compare_pppoe_header(phdr, phdr2))
>>> +            NAPI_GRO_CB(p)->same_flow = 0;
>>> +    }
>>> +
>>> +    skb_gro_pull(skb, sizeof(*phdr) + 2);
>>> +    skb_gro_postpull_rcsum(skb, phdr, sizeof(*phdr) + 2);
>>> +
>>> +    pp = ptype->callbacks.gro_receive(head, skb);
>>
>> Here you can use INDIRECT_CALL_INET()
> 
> I did that in the initial version, but then I got reports of build
> failures with the patch:
> 
> ERROR: modpost: "inet_gro_receive" [drivers/net/ppp/pppoe.ko] undefined!
> ERROR: modpost: "inet_gro_complete" [drivers/net/ppp/pppoe.ko] undefined!
> 
> Should I leave it out, or export inet_gro_receive/complete?

Could be exported I'd say. This would allows more modules to implement
GRO support.
INDIRECT_CALL() here gives good boosts here, at least I got some after
switching VLAN and TEB code several years ago.

If everyone is fine with exporting, I'd say those need to be exported as
GPL-only.

>>> +
>>> +out:
>>> +    skb_gro_flush_final(skb, pp, flush);
>>> +
>>> +    return pp;

Thanks,
Olek