Re: [PATCH net-next v5] net: airoha: add HW GRO offload support

[Paolo Abeni <pabeni@redhat.com>]

On 9/3/26 5:45 PM, Lorenzo Bianconi wrote:
> On Sep 03, Paolo Abeni wrote:
>> On 9/3/26 2:35 PM, Lorenzo Bianconi wrote:
>>>> On 8/31/26 8:34 AM, Lorenzo Bianconi wrote:
>>>>> Add hardware GRO offload support to the airoha_eth driver, leveraging
>>>>> the EN7581/AN7583 SoC's 8 dedicated LRO hardware queues mapped to RX
>>>>> queues 24-31. HW GRO offloading does not support Scatter-Gather (SG) so
>>>>> it is required to increase the page_pool allocation order to 2 for RX
>>>>> queues 24-31 (LRO queues).
>>>>> Since HW GRO is configured per-QDMA and shared across all devices using
>>>>> it, HW GRO is mutually exclusive with multiple devices bound to the
>>>>> same QDMA block. Call airoha_update_netdev_features() in
>>>>> airoha_dev_set_qdma() so that NETIF_F_GRO_HW availability is re-evaluated
>>>>> whenever the QDMA user count changes (device registration and runtime QDMA
>>>>> migration).
>>>>> Set CHECKSUM_PARTIAL with pseudo-header checksum on aggregated packets
>>>>> so that L3-forwarded traffic is correctly handled by the GSO/TSO path
>>>>> on the egress device.
>>>>> The HW does not report the per-segment MSS (msg3[31:16] only reports
>>>>> the max aggregated size), so the gso_size of an aggregated packet is
>>>>> just an approximation computed as DIV_ROUND_UP(data_len, agg_count).
>>>>
>>>> What is the <max aggregated size> exactly? I read it as the maximum size
>>>> of the aggregated segments, am I correct?
>>>
>>> Hi Paolo,
>>>
>>> with "max aggregated size" I refer to the length of the aggregated TCP
>>> packet (composed by multiple segments). This value is reported via
>>> QDMA_DESC_LEN_MASK field in the DMA descriptor. Moreover, the hw reports
>>> the exact number of the aggregated segments via QDMA_ETH_RXMSG_AGG_COUNT_MASK
>>> field.
>>>
>>>>
>>>> Also any more details on how the aggregation engine works? i.e. can it
>>>> aggregate "random" segment sizes (i.e. 200 - 300 - 400) or does it
>>>> respect HW_GRO layout? (i.e. all segments except the last one must have
>>>> equal size, the can be smaller).
>>>
>>> The hw engine, for each LRO queue, is configured with:
>>>
>>> - CDM_LRO_AGG_NUM_MASK: max number of segments for each aggregated TCP packet
>>>                         (64 in the current configuration).
>>> - CDM_LRO_AGG_SIZE_MASK: max size of the aggregated TCP packet (composed by
>>>                          multiple segments). 16KB in the current configuration.
>>> - CDM_LRO_AGG_TIME_MASK: max timeout to compose the aggregated TCP packet.
>>>
>>> In order to validate the scenario, I run the following test:
>>>
>>> TCP client:
>>> ------------
>>> - disable TSO/GSO
>>> - set MSS to 256 bytes (iperf3 -M option)
>>>
>>> I can see multiple 310 bytes TCP segments on the wire
>>>
>>> TCP server: (where rx-gro-hw is enabled):
>>> -------------------------------------------
>>> - the engine aggregates ~64 segments in a ~16Kbyes TCP packet
>>>
>>>   so gso_size ~ 16KB / 64 ~ 256B
>>>
>>> I guess this would be the behaviour even if the original packets
>>> have different size (not sure if it is a real use-case).
>> Lacking more details from the vendor, I think you need to use a
>> pktdrill-like sender, explicitly sets the segment lengths to some not
>> mergeable (like, i.e. 200-300-400) but otherwise fitting a GRO packet
>> (i.e. same hdr except for the sequence number and push flag allowed only
>> in the last packet), ensure that the aggregation timeout lasts long
>> enough to receive all of them, and check if the engine really aggregates
>> them or not.
> 
> I think in the example you pointed out (length 200,300,400) the hw engine will
> create a single TCP packet composed by 3 segments (with gso_size = 300) while
> sw GRO will just push sigle skbs. I agree this is just a GRO approximation.
> If it is not enough I am fine to switch back to LRO implementation.
In such a case I think LRO should be needed. I suggest to explicitly
validate the statement, it should be feasible, and the possible gain
relevant (should the engine instead do the right thing).

/P