On Tue, 4 Nov 2025 at 06:11, Akihiko Odaki <[email protected]> wrote: > > On 2025/11/04 2:58, Peter Maydell wrote: > > In e1000e_write_packet_to_guest() we don't write data for RX descriptors > > where the buffer address is NULL (as required by the i82574 datasheet > > section 7.1.7.2). However, when we do this we still update desc_offset > > by the amount of data we would have written to the RX descriptor if > > it had a valid buffer pointer, resulting in our dropping that data > > entirely. The data sheet is not 100% clear on the subject, but this > > seems unlikely to be the correct behaviour. > > > > Rearrange the null-descriptor logic so that we don't treat these > > do-nothing descriptors as if we'd really written the data. > > Please make a corresponding change for igb too so that the code of these > two devices will not diverge further.
The igb_core.c version of this function seems to be rather different (and rather easier to read). It has a kind of related bug where igb_write_packet_to_guest() calls igb_write_to_rx_buffers() and assumes that that function has correctly set pdma_st.desc_size to the amount of data written to that descriptor, but the early-return cases from igb_write_to_rx_buffers() don't actually update that field so it will be whatever junk was present from the previous iteration. So it looks to me like there are similar bugs but the code in these two devices is already pretty different and the fixes don't transpose one-to-one. (For instance, the problem with the assert location fixed in patch 3 here doesn't seem to be present in igb, which already does the assert() at the top of its loop in igb_write_payload_frag_to_rx_buffers().) In any case, I don't think it makes sense to look at fixing igb before this patchset has been reviewed and we're confident the fixes are actually right :-) thanks -- PMM
