From: David Laight <David.Laight@ACULAB.COM>
To: 'Shinas Rasheed' <srasheed@marvell.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "hgani@marvell.com" <hgani@marvell.com>,
"vimleshk@marvell.com" <vimleshk@marvell.com>,
"egallen@redhat.com" <egallen@redhat.com>,
"mschmidt@redhat.com" <mschmidt@redhat.com>,
"pabeni@redhat.com" <pabeni@redhat.com>,
"horms@kernel.org" <horms@kernel.org>,
"kuba@kernel.org" <kuba@kernel.org>,
"davem@davemloft.net" <davem@davemloft.net>,
"wizhao@redhat.com" <wizhao@redhat.com>,
"konguyen@redhat.com" <konguyen@redhat.com>,
Veerasenareddy Burru <vburru@marvell.com>,
"Sathesh Edara" <sedara@marvell.com>,
Eric Dumazet <edumazet@google.com>
Subject: RE: [PATCH net-next v2 3/4] octeon_ep: implement xmit_more in transmit
Date: Sat, 28 Oct 2023 06:38:51 +0000 [thread overview]
Message-ID: <0fc50b8e6ff44c43b10481da608c95c3@AcuMS.aculab.com> (raw)
In-Reply-To: <20231024145119.2366588-4-srasheed@marvell.com>
From: Shinas Rasheed
> Sent: 24 October 2023 15:51
>
> Add xmit_more handling in tx datapath for octeon_ep pf.
>
...
> -
> - /* Ring Doorbell to notify the NIC there is a new packet */
> - writel(1, iq->doorbell_reg);
> - iq->stats.instr_posted++;
> + /* Ring Doorbell to notify the NIC of new packets */
> + writel(iq->fill_cnt, iq->doorbell_reg);
> + iq->stats.instr_posted += iq->fill_cnt;
> + iq->fill_cnt = 0;
> return NETDEV_TX_OK;
Does that really need the count?
A 'doorbell' register usually just tells the MAC engine
to go and look at the transmit ring.
It then continues to process transmits until it fails
to find a packet.
So if the transmit is active you don't need to set the bit.
(Although that is actually rather hard to detect.)
The 'xmit_more' flag is useful if (the equivalent of) writing
the doorbell register is expensive since it can be delayed
to a later frame and only done once - adding a slight latency
to the earlier transmits if the mac engine was idle.
I'm not sure how much (if any) performance gain you actually
get from avoiding the writel().
Single PCIe writes are 'posted' and pretty much completely
asynchronous.
The other problem I've seen is that netdev_xmit_more() is
the state of the queue when the transmit was started, not
the current state.
If a packet is added while the earlier transmit setup code
is running (setting up the descriptors etc) the it isn't set.
So the fast path doesn't get taken.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
next prev parent reply other threads:[~2023-10-28 6:38 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-24 14:51 [PATCH net-next v2 0/4] Cleanup and optimizations to transmit code Shinas Rasheed
2023-10-24 14:51 ` [PATCH net-next v2 1/4] octeon_ep: add padding for small packets Shinas Rasheed
2023-10-25 0:15 ` Jakub Kicinski
2023-10-24 14:51 ` [PATCH net-next v2 2/4] octeon_ep: remove dma sync in trasmit path Shinas Rasheed
2023-10-24 14:51 ` [PATCH net-next v2 3/4] octeon_ep: implement xmit_more in transmit Shinas Rasheed
2023-10-25 0:21 ` Jakub Kicinski
2023-10-26 7:57 ` [EXT] " Shinas Rasheed
2023-10-26 8:28 ` Eric Dumazet
2023-10-26 14:44 ` Jakub Kicinski
2023-10-27 11:25 ` Shinas Rasheed
2023-10-28 6:38 ` David Laight [this message]
2023-10-30 14:14 ` Shinas Rasheed
2023-10-30 15:29 ` David Laight
2023-11-02 13:24 ` Shinas Rasheed
2023-10-24 14:51 ` [PATCH net-next v2 4/4] octeon_ep: remove atomic variable usage in Tx data path Shinas Rasheed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0fc50b8e6ff44c43b10481da608c95c3@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=egallen@redhat.com \
--cc=hgani@marvell.com \
--cc=horms@kernel.org \
--cc=konguyen@redhat.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mschmidt@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sedara@marvell.com \
--cc=srasheed@marvell.com \
--cc=vburru@marvell.com \
--cc=vimleshk@marvell.com \
--cc=wizhao@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome