Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210511190518.8901-1-gatis@mikrotik.com> <20210511190518.8901-3-gatis@mikrotik.com>
In-Reply-To: <20210511190518.8901-3-gatis@mikrotik.com>
From:   Chris Snook <chris.snook@gmail.com>
Date:   Tue, 11 May 2021 19:39:38 -0700
Message-ID: <CAMXMK6tkPYLLQzq65uFVd-aCWaVHSne16MBEo7o6fGDTDA0rpw@mail.gmail.com>
Subject: Re: [PATCH net-next 2/4] atl1c: improve performance by avoiding
 unnecessary pcie writes on xmit
To:     Gatis Peisenieks <gatis@mikrotik.com>
Cc:     "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        jesse.brandeburg@intel.com, dchickles@marvell.com,
        tully@mikrotik.com, Eric Dumazet <eric.dumazet@gmail.com>,
        netdev <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Increases in latency tend to hurt more on single-queue devices. Has
this been tested on the original gigabit atl1c?

- Chris

On Tue, May 11, 2021 at 12:05 PM Gatis Peisenieks <gatis@mikrotik.com> 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@mikrotik.com>
> ---
>  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);
> +               if (!more)
> +                       atl1c_tx_queue(adapter, type);
>         }
>
>         return NETDEV_TX_OK;
> --
> 2.31.1
>