[PATCH net 0/2] tg3: Disallow 0 rx coalesce time and correctly report RSS queues in tg3_get_rxnfc

From: Siva Reddy Kallam <siva.kallam@broadcom.com>

First patch:
        Diasllow rx coalescing time to be 0

Second patch:
        Report the correct number of RSS queues through tg3_get_rxnfc

Satish Baddipadige (1):
  tg3: Fix for diasllow rx coalescing time to be 0

Siva Reddy Kallam (1):
  tg3: Report the correct number of RSS queues through tg3_get_rxnfc

 drivers/net/ethernet/broadcom/tg3.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

From: Siva Reddy Kallam <siva.kallam@broadcom.com>

This patch remove the wrong substraction from info->data in
tg3_get_rxnfc function. Without this patch, the number of RSS
queues reported is less by one.

Reported-by: Michal Soltys <redacted>
Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index f3c6c91..6592612 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c

@@ -12552,10 +12552,6 @@ static int tg3_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
 				info->data = TG3_RSS_MAX_NUM_QS;
 		}
 
-		/* The first interrupt vector only
-		 * handles link interrupts.
-		 */
-		info->data -= 1;
 		return 0;
 
 	default:

-- 
1.7.1

From: Satish Baddipadige <redacted>

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 <redacted>
Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 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) ||

-- 
1.7.1

On 08/02/2016 09:13 PM, skallam 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.

happy benchmarking,

rick jones

From: skallam <siva.kallam@broadcom.com>
Date: Wed,  3 Aug 2016 09:43:58 +0530

First patch:
        Diasllow rx coalescing time to be 0

Second patch:
        Report the correct number of RSS queues through tg3_get_rxnfc

Series applied, thanks.

On Wed, Aug 3, 2016 at 9:04 AM, Rick Jones [off-list ref] 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.