Re: [PATCH net-next v2] net: pppoe: implement GRO support

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Felix Fietkau <nbd@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