Re: [PATCH net-next 2/4] atl1c: improve performance by avoiding unnecessary pcie writes on xmit

[Eric Dumazet]

On 5/11/21 9:05 PM, Gatis Peisenieks wrote:
> The kernel has xmit_more facility that hints the networking driver xmit
> path about whether more packets are coming soon. This information can be
> used to avoid unnecessary expensive PCIe transaction per tx packet at a
> slight increase in latency.
> 
> Max TX pps on Mikrotik 10/25G NIC in a Threadripper 3960X system
> improved from 1150Kpps to 1700Kpps.
> 
> Signed-off-by: Gatis Peisenieks <gatis@xxxxxxxxxxxx>
> ---
>  drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> index 28c30d5288e4..2a8ab51b0ed9 100644
> --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> @@ -2211,8 +2211,8 @@ static int atl1c_tx_map(struct atl1c_adapter *adapter,
>  	return -1;
>  }
>  
> -static void atl1c_tx_queue(struct atl1c_adapter *adapter, struct sk_buff *skb,
> -			   struct atl1c_tpd_desc *tpd, enum atl1c_trans_queue type)
> +static void atl1c_tx_queue(struct atl1c_adapter *adapter,
> +			   enum atl1c_trans_queue type)
>  {
>  	struct atl1c_tpd_ring *tpd_ring = &adapter->tpd_ring[type];
>  	u16 reg;
> @@ -2238,6 +2238,7 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
>  
>  	if (atl1c_tpd_avail(adapter, type) < tpd_req) {
>  		/* no enough descriptor, just stop queue */
> +		atl1c_tx_queue(adapter, type);
>  		netif_stop_queue(netdev);
>  		return NETDEV_TX_BUSY;
>  	}
> @@ -2246,6 +2247,7 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
>  
>  	/* do TSO and check sum */
>  	if (atl1c_tso_csum(adapter, skb, &tpd, type) != 0) {
> +		atl1c_tx_queue(adapter, type);
>  		dev_kfree_skb_any(skb);
>  		return NETDEV_TX_OK;
>  	}
> @@ -2270,8 +2272,11 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
>  		atl1c_tx_rollback(adapter, tpd, type);
>  		dev_kfree_skb_any(skb);
>  	} else {
> -		netdev_sent_queue(adapter->netdev, skb->len);
> -		atl1c_tx_queue(adapter, skb, tpd, type);
> +		bool more = netdev_xmit_more();
> +
> +		__netdev_sent_queue(adapter->netdev, skb->len, more);


This is probably buggy.

You must check and use the return code of this function,
as in :

	bool door_bell = __netdev_sent_queue(adapter->netdev, skb->len, netdev_xmit_more());

	if (door_bell)
		atl1c_tx_queue(adapter, type);


> +		if (!more)
> +			atl1c_tx_queue(adapter, type);
>  	}
>  
>  	return NETDEV_TX_OK;
>