RE: [PATCH net v2 2/2] net: ethernet: oa_tc6: Improvement in buffer overflow handling

[Selvamani Rajagopal]

> -----Original Message-----
> From: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Monday, June 29, 2026 7:30 PM
> Subject: Re: [PATCH net v2 2/2] net: ethernet: oa_tc6: Improvement in buffer overflow handling
> 
> 
> This sounds rather scary. The driver seems to have no length
> information to confirm it got all the chunks. So if it missed
> a middle chunk we will never know? At the very least this seems
> like something we should increment rx_error for, not just rx_dropped?

If middle chunk is lost, we won't know. If we look at prcs_rx_chunk_payload, it relies on start_valid 
and end_valid bits only. start_byte_offset and end_byte_offset are always associated with the above
two bits.

When rx buffer overflow occurs, we don't know whether to trust the data chunk or lost, if lost,
how many lost, so, we drop everything and start looking for new frame indicated by "start_bit"
That's why drop count is incremented. 

If we dig deeper, though we increment the dropped count by one. It is possible that we may have lost 
more than one frame.

Function process_spi_data_rx_buf  bails out of the "for loop" with number_of_rx_chunks,
after EAGAIN error is seen. We don't go through all the rx chunks that are already received, when 
we start looking for next data chunks with "start_bit". 

That improvement is for another time. I didn't want to complicate the changes for this effort.

> 
> Regarding the patch itself, I'm not clear on why we need to look
> for new frame. Will we not notice the start bit immediately and
> call oa_tc6_allocate_rx_skb() (if there is indeed a start bit in the
> stream?)
> 

The chunk with "start_bit" could be next chunk or "nth" chunk. We don't know.
We keep throwing away incoming data chunks until we see a chunk with data_bit.

> So handling skb-already-exists in oa_tc6_allocate_rx_skb() seems
> like enough to start a new frame.


I don't think we can rely on this. What if we get two continuous data chunks with "start_bit" set
without end_valid bit? Later skb would overwrite the previous one. (either due to corruption or lost chunks)


> 
> Sashiko has another comment:

Let me read at the Sashiko review 

> 
> https://sashiko.dev/#/patchset/20260626-fix-race-condition-and-crash-v2-1-
> b6c5c10e604f@xxxxxxxxxx
> <https://sashiko.dev/#/patchset/20260626-fix-race-condition-and-crash-v2-1-b6c5c10e604f@xxxxxxxxxx
> =sashiko.dev>
> --
> pw-bot: cr