Skip to content
  • Eric Joyner's avatar
    intel iflib drivers: correct initialization of tx_cidx_processed · 088a0b27
    Eric Joyner authored
    From Jake:
    
    In r341156 ("Fix first-packet completion", 2018-11-28) a hack to work
    around a delta calculation determining how many descriptors were used
    was added to ixl_isc_tx_credits_update_dwb.
    
    The same fix was also applied to the em and igb drivers in r340310, and
    to ix in r341156.
    
    The hack checked the case where prev and cur were equal, and then added
    one. This works, because by the time we do the delta check, we already
    know there is at least one packet available, so the delta should be at
    least one.
    
    However, it's not a complete fix, and as indicated by the comment is
    really a hack to work around the real bug.
    
    The real problem is that the first time that we transmit a packet,
    tx_cidx_processed will be set to point to the start of the ring.
    Ultimately, the credits_update function expects it to point to the
    *last* descriptor that was processed. Since we haven't yet processed any
    descriptors, pointing it to 0 results in this incorrect calculation.
    
    Fix the initialization code to have it point to the end of the ring
    instead. One way to think about this, is that we are setting the value
    to be one prior to the first available descriptor.
    
    Doing so, corrects the delta calculation in all cases. The original fix
    only works if the first packet has exactly one descriptor. Otherwise, we
    will report 1 less than the correct value.
    
    As part of this fix, also update the MPASS assertions to match the real
    expectations. First, ensure that prev is not equal to cur, since this
    should never happen. Second, remove the assertion about prev==0 || delta
    != 0. It looks like that originated from when the em driver was
    converted to iflib. It seems like it was supposed to ensure that delta
    was non-zero. However, because we originally returned 0 delta for the
    first calculation, the "prev == 0" was tacked on.
    
    Instead, replace this with a check that delta is greater than zero,
    after the correction necessary when the ring pointers wrap around.
    
    This new solution should fix the same bug as r341156 did, but in a more
    robust way.
    
    Submitted by:	Jacob Keller <jacob.e.keller@intel.com>
    Reviewed by:	shurd@
    Sponsored by:	Intel Corporation
    Differential Revision:	https://reviews.freebsd.org/D18545
    088a0b27