Re: [PATCH net 1/2] tg3: Fix for diasllow rx coalescing time to be 0
On 08/02/2016 09:13 PM, skallam wrote: From: Satish Baddipadige When the rx coalescing time is 0, interrupts are not generated from the controller and rx path hangs. To avoid this rx hang, updating the driver to not allow rx coalescing time to be 0. Signed-off-by: Satish Baddipadige Signed-off-by: Siva Reddy Kallam Signed-off-by: Michael Chan --- drivers/net/ethernet/broadcom/tg3.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index ff300f7..f3c6c91 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -14014,6 +14014,7 @@ static int tg3_set_coalesce(struct net_device *dev, struct ethtool_coalesce *ec) } if ((ec->rx_coalesce_usecs > MAX_RXCOL_TICKS) || + (!ec->rx_coalesce_usecs) || (ec->tx_coalesce_usecs > MAX_TXCOL_TICKS) || (ec->rx_max_coalesced_frames > MAX_RXMAX_FRAMES) || (ec->tx_max_coalesced_frames > MAX_TXMAX_FRAMES) || Should anything then happen with: /* No rx interrupts will be generated if both are zero */ if ((ec->rx_coalesce_usecs == 0) && (ec->rx_max_coalesced_frames == 0)) return -EINVAL; which is the next block of code? The logic there seems to suggest that it was intended to be able to have an rx_coalesce_usecs of 0 and rely on packet arrival to trigger an interrupt. Presumably setting rx_max_coalesced_frames to 1 to disable interrupt coalescing. happy benchmarking, rick jones
Re: [PATCH net 1/2] tg3: Fix for diasllow rx coalescing time to be 0
On Wed, Aug 3, 2016 at 9:04 AM, Rick Jones wrote: > > Should anything then happen with: > > /* No rx interrupts will be generated if both are zero */ > if ((ec->rx_coalesce_usecs == 0) && > (ec->rx_max_coalesced_frames == 0)) > return -EINVAL; > > > which is the next block of code? The logic there seems to suggest that it > was intended to be able to have an rx_coalesce_usecs of 0 and rely on packet > arrival to trigger an interrupt. Presumably setting rx_max_coalesced_frames > to 1 to disable interrupt coalescing. > I remember writing this block of code over 10 years ago for early generations of the chip. Newer chips seem to behave differently and rx_coalesce_usecs can never be zero. So this block can be removed now that the condition can never be true. We should probably leave a comment there for future reference.
Re: [PATCH net 1/2] tg3: Fix for diasllow rx coalescing time to be 0
On Thu, Aug 4, 2016 at 3:45 AM, Michael Chan wrote: > On Wed, Aug 3, 2016 at 9:04 AM, Rick Jones wrote: >> >> Should anything then happen with: >> >> /* No rx interrupts will be generated if both are zero */ >> if ((ec->rx_coalesce_usecs == 0) && >> (ec->rx_max_coalesced_frames == 0)) >> return -EINVAL; >> >> >> which is the next block of code? The logic there seems to suggest that it >> was intended to be able to have an rx_coalesce_usecs of 0 and rely on packet >> arrival to trigger an interrupt. Presumably setting rx_max_coalesced_frames >> to 1 to disable interrupt coalescing. >> > > I remember writing this block of code over 10 years ago for early > generations of the chip. Newer chips seem to behave differently and > rx_coalesce_usecs can never be zero. So this block can be removed now > that the condition can never be true. We should probably leave a > comment there for future reference. Thanks Rick for identifying this. Thanks Michael for your inputs. I will submit a patch with removal of this block of code and add a comment for future reference.
