Re: [PATCH] ionic: fix 16bit math issue when PAGE_SIZE >= 64KB

[David Christensen <drc@linux.vnet.ibm.com>]

On 9/11/23 5:14 PM, Jacob Keller wrote:
> 
> 
> On 9/11/2023 3:22 PM, David Christensen wrote:
>> The function ionic_rx_fill() uses 16bit math when calculating the
>> the number of pages required for an RX descriptor given an interface
>> MTU setting. If the system PAGE_SIZE >= 64KB, the frag_len and
>> remain_len values will always be 0, causing unnecessary scatter-
>> gather elements to be assigned to the RX descriptor, up to the
>> maximum number of scatter-gather elements per descriptor.
>>
>> A similar change in ionic_rx_frags() is implemented for symmetry,
>> but has not been observed as an issue since scatter-gather
>> elements are not necessary for such larger page sizes.
>>
>> Fixes: 4b0a7539a372 ("ionic: implement Rx page reuse")
>> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
>> ---
> 
> Given this is a bug fix, it should probably have a subject of [PATCH
> net] or [net] to indicate its targeting the net tree.

Will resend v2 with updated Subject line.

> 
> I'm not sure I follow the logic for frag_len and remain_len always being
> zero, since typecasting unsigned values truncates the higher bytes
> (technically its guaranteed by the standard to result in the smallest
> value congruent modulo 2^16 for a 16bit typecast), so if page_offset was
> non-zero then the resulting with the typecast should be as well.. but
> either way its definitely not going to work as desired.

Sorry, tried condensing the explanation too much. I'm not sure how 
frequently buf_info->page_offset is non-zero, but when 
ionic_rx_page_alloc() allocates a new page, as happens during initial 
driver load, it explicitly sets buf_info->page_offset to 0.  As a 
result, the remain_len value never decreases from the original frame 
size (e.g. 1522) while frag_len is always calculated as 0 ((min_t(u16, 
0x5F2, (0x1_0000 - 0) -> 0).

I'll update the the description in v2.

Dave