On Thu, Jul 18, 2024 at 8:47 PM Willem de Bruijn willemdebruijn.kernel@gmail.com wrote:
On Thu, Jul 18, 2024 at 9:52 PM Praveen Kaligineedi pkaligineedi@google.com wrote:
On Thu, Jul 18, 2024 at 4:07 PM Willem de Bruijn willemdebruijn.kernel@gmail.com wrote:
* segment, then it will count as two descriptors.*/if (last_frag_size > GVE_TX_MAX_BUF_SIZE_DQO) {int last_frag_remain = last_frag_size %GVE_TX_MAX_BUF_SIZE_DQO;/* If the last frag was evenly divisible by* GVE_TX_MAX_BUF_SIZE_DQO, then it will not be* split in the current segment.Is this true even if the segment did not start at the start of the frag?
The comment probably is a bit confusing here. The current segment we are tracking could have a portion in the previous frag. The code assumed that the portion on the previous frag (if present) mapped to only one descriptor. However, that portion could have been split across two descriptors due to the restriction that each descriptor cannot exceed 16KB.
/* If the last frag was evenly divisible by
* GVE_TX_MAX_BUF_SIZE_DQO, then it will not be* split in the current segment.This is true because the smallest multiple of 16KB is 32KB, and the largest gso_size at least for Ethernet will be 9K. But I don't think that that is what is used here as the basis for this statement?
The largest Ethernet gso_size (9K) is less than GVE_TX_MAX_BUF_SIZE_DQO is an implicit assumption made in this patch and in that comment. Bailey, please correct me if I am wrong..
That's the case this fix is trying to address. I will work on simplifying the logic based on your suggestion below so that the fix is easier to follow