[Intel-wired-lan] [PATCH net-next v1 0/4] ice: transmit improvement series

[Intel-wired-lan] [PATCH net-next v1 0/4] ice: transmit improvement series

[Jesse Brandeburg]

Modify the ice driver to make a few modest changes to the transmit
hot path in order to reduce the number of cycles used, particularly
with high intensity workloads such as pktgen or IP forwarding.

Several of these changes are just using simpler API calls in
netdevice.h (welcome to 2015!)

The VSI_DOWN patch is a simple fix to make sure the driver's statements
about holding certain state bits for functions is enforced and remains
that way.

Jesse Brandeburg (4):
  ice: update to newer kernel API
  ice: use prefetch methods
  ice: tighter control over VSI_DOWN state
  ice: use modern kernel API for kick

 drivers/net/ethernet/intel/ice/ice_ethtool.c |  6 ++-
 drivers/net/ethernet/intel/ice/ice_main.c    |  7 ++--
 drivers/net/ethernet/intel/ice/ice_txrx.c    | 40 +++++++++++++-------
 3 files changed, 34 insertions(+), 19 deletions(-)

-- 
2.31.1

[Intel-wired-lan] [PATCH net-next v1 1/4] ice: update to newer kernel API

[Jesse Brandeburg]

Use the netif_tx_* API from netdevice.h which has simpler parameters.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
Testing Hints: test transmit performance and queue
start/stop tests which increment tx_restart counter.
---
 drivers/net/ethernet/intel/ice/ice_txrx.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index bc3ba19dc88f..12a2edd13877 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -304,8 +304,10 @@ static bool ice_clean_tx_irq(struct ice_tx_ring *tx_ring, int napi_budget)
 
 	ice_update_tx_ring_stats(tx_ring, total_pkts, total_bytes);
 
-	netdev_tx_completed_queue(txring_txq(tx_ring), total_pkts,
-				  total_bytes);
+	if (ice_ring_is_xdp(tx_ring))
+		return !!budget;
+
+	netdev_tx_completed_queue(txring_txq(tx_ring), total_pkts, total_bytes);
 
 #define TX_WAKE_THRESHOLD ((s16)(DESC_NEEDED * 2))
 	if (unlikely(total_pkts && netif_carrier_ok(tx_ring->netdev) &&
@@ -314,11 +316,9 @@ static bool ice_clean_tx_irq(struct ice_tx_ring *tx_ring, int napi_budget)
 		 * sees the new next_to_clean.
 		 */
 		smp_mb();
-		if (__netif_subqueue_stopped(tx_ring->netdev,
-					     tx_ring->q_index) &&
+		if (netif_tx_queue_stopped(txring_txq(tx_ring)) &&
 		    !test_bit(ICE_VSI_DOWN, vsi->state)) {
-			netif_wake_subqueue(tx_ring->netdev,
-					    tx_ring->q_index);
+			netif_tx_wake_queue(txring_txq(tx_ring));
 			++tx_ring->tx_stats.restart_q;
 		}
 	}
@@ -1517,7 +1517,7 @@ int ice_napi_poll(struct napi_struct *napi, int budget)
  */
 static int __ice_maybe_stop_tx(struct ice_tx_ring *tx_ring, unsigned int size)
 {
-	netif_stop_subqueue(tx_ring->netdev, tx_ring->q_index);
+	netif_tx_stop_queue(txring_txq(tx_ring));
 	/* Memory barrier before checking head and tail */
 	smp_mb();
 
@@ -1525,8 +1525,8 @@ static int __ice_maybe_stop_tx(struct ice_tx_ring *tx_ring, unsigned int size)
 	if (likely(ICE_DESC_UNUSED(tx_ring) < size))
 		return -EBUSY;
 
-	/* A reprieve! - use start_subqueue because it doesn't call schedule */
-	netif_start_subqueue(tx_ring->netdev, tx_ring->q_index);
+	/* A reprieve! - use start_queue because it doesn't call schedule */
+	netif_tx_start_queue(txring_txq(tx_ring));
 	++tx_ring->tx_stats.restart_q;
 	return 0;
 }
-- 
2.31.1

[Intel-wired-lan] [PATCH net-next v1 1/4] ice: update to newer kernel API

[G, GurucharanX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Jesse
> Brandeburg
> Sent: Tuesday, October 26, 2021 5:38 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH net-next v1 1/4] ice: update to newer kernel
> API
> 
> Use the netif_tx_* API from netdevice.h which has simpler parameters.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
> Testing Hints: test transmit performance and queue start/stop tests which
> increment tx_restart counter.
> ---
>  drivers/net/ethernet/intel/ice/ice_txrx.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 

Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)

[Intel-wired-lan] [PATCH net-next v1 3/4] ice: tighter control over VSI_DOWN state

[Jesse Brandeburg]

The driver had comments to the effect of: This flag should be set before
calling this function. While reviewing code it was found that there were
several violations of this policy, which could introduce hard to find
bugs or races.

Fix the violations of the "VSI DOWN state must be set before calling
ice_down" and make checking the state into code with a WARN_ON.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
Testing Hints (Required if no HSD): legacy-rx private flag
disable/enable forgot to set this previously and is a way to trigger the
down/up path.
---
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 6 ++++--
 drivers/net/ethernet/intel/ice/ice_main.c    | 7 ++++---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 870d02adcfe8..473f025c1cb5 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -1279,8 +1279,10 @@ static int ice_set_priv_flags(struct net_device *netdev, u32 flags)
 	}
 	if (test_bit(ICE_FLAG_LEGACY_RX, change_flags)) {
 		/* down and up VSI so that changes of Rx cfg are reflected. */
-		ice_down(vsi);
-		ice_up(vsi);
+		if (!test_and_set_bit(ICE_VSI_DOWN, vsi->state)) {
+			ice_down(vsi);
+			ice_up(vsi);
+		}
 	}
 	/* don't allow modification of this flag when a single VF is in
 	 * promiscuous mode because it's not supported
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 28815e5a7770..b82dc2642d99 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -6212,14 +6212,15 @@ static void ice_napi_disable_all(struct ice_vsi *vsi)
 /**
  * ice_down - Shutdown the connection
  * @vsi: The VSI being stopped
+ *
+ * Caller of this function is expected to set the vsi->state ICE_DOWN bit
  */
 int ice_down(struct ice_vsi *vsi)
 {
 	int i, tx_err, rx_err, link_err = 0;
 
-	/* Caller of this function is expected to set the
-	 * vsi->state ICE_DOWN bit
-	 */
+	WARN_ON(!test_bit(ICE_VSI_DOWN, vsi->state));
+
 	if (vsi->netdev && vsi->type == ICE_VSI_PF) {
 		if (!ice_is_e810(&vsi->back->hw))
 			ice_ptp_link_change(vsi->back, vsi->back->hw.pf_id, false);
-- 
2.31.1

[Intel-wired-lan] [PATCH net-next v1 3/4] ice: tighter control over VSI_DOWN state

[G, GurucharanX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Jesse
> Brandeburg
> Sent: Tuesday, October 26, 2021 5:38 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH net-next v1 3/4] ice: tighter control over
> VSI_DOWN state
> 
> The driver had comments to the effect of: This flag should be set before calling
> this function. While reviewing code it was found that there were several
> violations of this policy, which could introduce hard to find bugs or races.
> 
> Fix the violations of the "VSI DOWN state must be set before calling ice_down"
> and make checking the state into code with a WARN_ON.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
> Testing Hints (Required if no HSD): legacy-rx private flag disable/enable forgot
> to set this previously and is a way to trigger the down/up path.
> ---
>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 6 ++++--
>  drivers/net/ethernet/intel/ice/ice_main.c    | 7 ++++---
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 

Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)

[Intel-wired-lan] [PATCH net-next v1 4/4] ice: use modern kernel API for kick

[Jesse Brandeburg]

The kernel gained a new interface for drivers to use to combine tail
bump (doorbell) and BQL updates, attempt to use those new interfaces.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_txrx.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index de9247d45c39..3987a0dd0e11 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -1576,6 +1576,7 @@ ice_tx_map(struct ice_tx_ring *tx_ring, struct ice_tx_buf *first,
 	struct sk_buff *skb;
 	skb_frag_t *frag;
 	dma_addr_t dma;
+	bool kick;
 
 	td_tag = off->td_l2tag1;
 	td_cmd = off->td_cmd;
@@ -1657,9 +1658,6 @@ ice_tx_map(struct ice_tx_ring *tx_ring, struct ice_tx_buf *first,
 		tx_buf = &tx_ring->tx_buf[i];
 	}
 
-	/* record bytecount for BQL */
-	netdev_tx_sent_queue(txring_txq(tx_ring), first->bytecount);
-
 	/* record SW timestamp if HW timestamp is not available */
 	skb_tx_timestamp(first->skb);
 
@@ -1688,7 +1686,10 @@ ice_tx_map(struct ice_tx_ring *tx_ring, struct ice_tx_buf *first,
 	ice_maybe_stop_tx(tx_ring, DESC_NEEDED);
 
 	/* notify HW of packet */
-	if (netif_xmit_stopped(txring_txq(tx_ring)) || !netdev_xmit_more())
+	kick = __netdev_tx_sent_queue(txring_txq(tx_ring), first->bytecount,
+				      netdev_xmit_more());
+	if (kick)
+		/* notify HW of packet */
 		writel(i, tx_ring->tail);
 
 	return;
-- 
2.31.1

[Intel-wired-lan] [PATCH net-next v1 4/4] ice: use modern kernel API for kick

[G, GurucharanX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Jesse
> Brandeburg
> Sent: Tuesday, October 26, 2021 5:38 AM
> To: intel-wired-lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH net-next v1 4/4] ice: use modern kernel API
> for kick
> 
> The kernel gained a new interface for drivers to use to combine tail bump
> (doorbell) and BQL updates, attempt to use those new interfaces.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_txrx.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 

Tested-by: Gurucharan G <gurucharanx.g@intel.com> (A Contingent worker at Intel)

[Intel-wired-lan] [PATCH net-next v1 0/4] ice: transmit improvement series

[Nguyen, Anthony L]

On Mon, 2021-10-25 at 17:08 -0700, Jesse Brandeburg wrote:
> Modify the ice driver to make a few modest changes to the transmit
> hot path in order to reduce the number of cycles used, particularly
> with high intensity workloads such as pktgen or IP forwarding.
> 
> Several of these changes are just using simpler API calls in
> netdevice.h (welcome to 2015!)
> 
> The VSI_DOWN patch is a simple fix to make sure the driver's
> statements
> about holding certain state bits for functions is enforced and
> remains
> that way.
> 
> Jesse Brandeburg (4):
> ? ice: update to newer kernel API
> ? ice: use prefetch methods
> ? ice: tighter control over VSI_DOWN state
> ? ice: use modern kernel API for kick

It looks like patch 2 never made it to the list [1]. Seems like the
email issues may be further reaching than kernel.org.

Thanks,
Tony

[1] https://patchwork.ozlabs.org/project/intel-wired-
lan/list/?series=268871

> ?drivers/net/ethernet/intel/ice/ice_ethtool.c |? 6 ++-
> ?drivers/net/ethernet/intel/ice/ice_main.c??? |? 7 ++--
> ?drivers/net/ethernet/intel/ice/ice_txrx.c??? | 40 +++++++++++++-----
> --
> ?3 files changed, 34 insertions(+), 19 deletions(-)
> 

[Intel-wired-lan] [PATCH net-next v1 0/4] ice: transmit improvement series

[Jesse Brandeburg]

On 10/26/2021 9:21 AM, Nguyen, Anthony L wrote:

>> Jesse Brandeburg (4):
>>  ? ice: update to newer kernel API
>>  ? ice: use prefetch methods
>>  ? ice: tighter control over VSI_DOWN state
>>  ? ice: use modern kernel API for kick
> 
> It looks like patch 2 never made it to the list [1]. Seems like the
> email issues may be further reaching than kernel.org.
> 
> Thanks,
> Tony
> 
> [1] https://patchwork.ozlabs.org/project/intel-wired-
> lan/list/?series=268871

Thanks for letting me know!
The issue was root caused to a typo in the To:email address on my end, 
and I attempted to resend the patch with a correct reply-to, it seems to 
have failed to do what I wanted (but did show up on patchwork now) so 
please let me know if you want me to resend the whole series as a v2.