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] [day] [month] [year] [list]

Message-ID: <5fce419c-622f-4eda-ab92-86deaf0db5ca@redhat.com>
Date: Fri, 25 Jul 2025 14:54:06 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>,
 Felix Fietkau <nbd@....name>
Cc: 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

On 7/25/25 2:42 PM, Alexander Lobakin wrote:
> 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.

Thanks Olek, I was almost missing this!

I agree with exporting GPL-only the symbols.

Thanks,

Paolo