diff mbox series

[net-next,v2,3/4] octeon_ep: implement xmit_more in transmit

Message ID 20231024145119.2366588-4-srasheed@marvell.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Cleanup and optimizations to transmit code | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 1388 this patch: 1388
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1388 this patch: 1388
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 47 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Shinas Rasheed Oct. 24, 2023, 2:51 p.m. UTC
Add xmit_more handling in tx datapath for octeon_ep pf.

Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
---
V2:
  - Updated changelog to have imperative tone.
V1: https://lore.kernel.org/all/20231023114449.2362147-3-srasheed@marvell.com/

 .../ethernet/marvell/octeon_ep/octep_config.h |  2 +-
 .../ethernet/marvell/octeon_ep/octep_main.c   | 19 +++++++++++++++----
 2 files changed, 16 insertions(+), 5 deletions(-)

Comments

Jakub Kicinski Oct. 25, 2023, 12:21 a.m. UTC | #1
On Tue, 24 Oct 2023 07:51:18 -0700 Shinas Rasheed wrote:
>  	iq->host_write_index = wi;
> +	if (xmit_more &&
> +	    (atomic_read(&iq->instr_pending) <
> +	     (iq->max_count - OCTEP_WAKE_QUEUE_THRESHOLD)) &&
> +	    iq->fill_cnt < iq->fill_threshold)
> +		return NETDEV_TX_OK;

Does this guarantee that a full-sized skb can be accommodated?
If so - consider stopping stopping the queue when the condition
is not true. The recommended way of implementing 'driver flow control'
is to stop the queue once next packet may not fit, and then use
netif_xmit_stopped() when deciding whether we need to flush or we can
trust xmit_more. see 
https://www.kernel.org/doc/html/next/networking/driver.html#transmit-path-guidelines

>  	/* Flush the hw descriptor before writing to doorbell */
>  	wmb();
> -
> -	/* 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;
Shinas Rasheed Oct. 26, 2023, 7:57 a.m. UTC | #2
Hi Jakub, 

Again, thanks for your review.

> Does this guarantee that a full-sized skb can be accommodated?
>> I assume by full-sized skb you mean a non-linear skb with MAX_SG_FRAGS elements in frags array.  Yes, this can be accommodated and the hardware uses separate SG list memory to siphon these skb frags instead of obtaining them from the same tx hardware queue. What I'm trying to say is, this means that a single tx descriptor will be enough for a full-sized skb as well, as hardware can pick up SG frags from separate memory and doesn't require separate descriptors.

>If so - consider stopping stopping the queue when the condition is not true.
>> We do stop the queue if tx queue is full, as in octep_iq_full_check earlier on.

>The recommended way of implementing 'driver flow control'
is to stop the queue once next packet may not fit, and then use
netif_xmit_stopped() when deciding whether we need to flush or we can
trust xmit_more. see 
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.kernel.org_doc_html_next_networking_driver.html-23transmit-2Dpath-2Dguidelines&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=1OxLD4y-oxrlgQ1rjXgWtmLz1pnaDjD96sDq-cKUwK4&m=FyJHTb5Z2u9DTFSYPU38S5kPcP5KvwGWzY-DPcqOl1gdnm7ToZhTFpyvhLMqh1hd&s=dBMmwfWKAi0UH3nrz7j9kYnAodDjuN3LZ5tC2aL_Prs&e= 

>> In the skeleton code above, as I understand each tx desc holds a skb frag and hence there can be possibility of receiving a full-sized skb, stopping the queue but on receiving another normal skb we should observe our queue to be stopped. This doesn't arise in our case as even if the skb is full-sized, it will only use a single tx descriptor so we can be sure if queue has been stopped, the write index will only be updated once posted (and read) tx descriptors are processed in napi context and queues awoken.

Please correct me if I'm wrong anywhere (sorry if so) to further my understanding, and again thanks for your time!
Eric Dumazet Oct. 26, 2023, 8:28 a.m. UTC | #3
On Thu, Oct 26, 2023 at 9:58 AM Shinas Rasheed <srasheed@marvell.com> wrote:
>
> Hi Jakub,
>
> Again, thanks for your review.
>
> > Does this guarantee that a full-sized skb can be accommodated?
> >> I assume by full-sized skb you mean a non-linear skb with MAX_SG_FRAGS elements in frags array.  Yes, this can be accommodated and the hardware uses separate SG list memory to siphon these skb frags instead of obtaining them from the same tx hardware queue. What I'm trying to say is, this means that a single tx descriptor will be enough for a full-sized skb as well, as hardware can pick up SG frags from separate memory and doesn't require separate descriptors.
>
> >If so - consider stopping stopping the queue when the condition is not true.
> >> We do stop the queue if tx queue is full, as in octep_iq_full_check earlier on.
>
> >The recommended way of implementing 'driver flow control'
> is to stop the queue once next packet may not fit, and then use
> netif_xmit_stopped() when deciding whether we need to flush or we can
> trust xmit_more. see
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.kernel.org_doc_html_next_networking_driver.html-23transmit-2Dpath-2Dguidelines&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=1OxLD4y-oxrlgQ1rjXgWtmLz1pnaDjD96sDq-cKUwK4&m=FyJHTb5Z2u9DTFSYPU38S5kPcP5KvwGWzY-DPcqOl1gdnm7ToZhTFpyvhLMqh1hd&s=dBMmwfWKAi0UH3nrz7j9kYnAodDjuN3LZ5tC2aL_Prs&e=
>
> >> In the skeleton code above, as I understand each tx desc holds a skb frag and hence there can be possibility of receiving a full-sized skb, stopping the queue but on receiving another normal skb we should observe our queue to be stopped. This doesn't arise in our case as even if the skb is full-sized, it will only use a single tx descriptor so we can be sure if queue has been stopped, the write index will only be updated once posted (and read) tx descriptors are processed in napi context and queues awoken.
>
> Please correct me if I'm wrong anywhere (sorry if so) to further my understanding, and again thanks for your time!

Fact that octep_start_xmit() can return NETDEV_TX_BUSY is very suspicious.

I do not think a driver can implement xmit_more and keep
NETDEV_TX_BUSY at the same time.

Please make sure to remove NETDEV_TX_BUSY first, by stopping the queue earlier.
Jakub Kicinski Oct. 26, 2023, 2:44 p.m. UTC | #4
On Thu, 26 Oct 2023 07:57:54 +0000 Shinas Rasheed wrote:
> >The recommended way of implementing 'driver flow control'  
> is to stop the queue once next packet may not fit, and then use
> netif_xmit_stopped() when deciding whether we need to flush or we can
> trust xmit_more. see 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.kernel.org_doc_html_next_networking_driver.html-23transmit-2Dpath-2Dguidelines&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=1OxLD4y-oxrlgQ1rjXgWtmLz1pnaDjD96sDq-cKUwK4&m=FyJHTb5Z2u9DTFSYPU38S5kPcP5KvwGWzY-DPcqOl1gdnm7ToZhTFpyvhLMqh1hd&s=dBMmwfWKAi0UH3nrz7j9kYnAodDjuN3LZ5tC2aL_Prs&e= 
> 
> >> In the skeleton code above, as I understand each tx desc holds a skb frag and hence there can be possibility of receiving a full-sized skb, stopping the queue but on receiving another normal skb we should observe our queue to be stopped. This doesn't arise in our case as even if the skb is full-sized, it will only use a single tx descriptor so we can be sure if queue has been stopped, the write index will only be updated once posted (and read) tx descriptors are processed in napi context and queues awoken.  
> 
> Please correct me if I'm wrong anywhere (sorry if so) to further my understanding, and again thanks for your time!

Could you please do some training on how to use normal / mailing list
style email at Marvell? Multiple people from Marvell can't quote
replies correctly, it makes it hard to have a conversation and help
y'all.
Shinas Rasheed Oct. 27, 2023, 11:25 a.m. UTC | #5
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, October 26, 2023 8:15 PM
> To: Shinas Rasheed <srasheed@marvell.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Haseeb Gani
> <hgani@marvell.com>; Vimlesh Kumar <vimleshk@marvell.com>;
> egallen@redhat.com; mschmidt@redhat.com; pabeni@redhat.com;
> horms@kernel.org; davem@davemloft.net; wizhao@redhat.com;
> konguyen@redhat.com; Veerasenareddy Burru <vburru@marvell.com>;
> Sathesh B Edara <sedara@marvell.com>; Eric Dumazet
> <edumazet@google.com>
> Subject: Re: [EXT] Re: [PATCH net-next v2 3/4] octeon_ep: implement
> xmit_more in transmit
> 
> On Thu, 26 Oct 2023 07:57:54 +0000 Shinas Rasheed wrote:
> > >The recommended way of implementing 'driver flow control'
> > is to stop the queue once next packet may not fit, and then use
> > netif_xmit_stopped() when deciding whether we need to flush or we can
> > trust xmit_more. see
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__www.kernel.org_doc_html_next_networking_driver.html-23transmit-
> 2Dpath-2Dguidelines&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=1OxLD4y-
> oxrlgQ1rjXgWtmLz1pnaDjD96sDq-
> cKUwK4&m=FyJHTb5Z2u9DTFSYPU38S5kPcP5KvwGWzY-
> DPcqOl1gdnm7ToZhTFpyvhLMqh1hd&s=dBMmwfWKAi0UH3nrz7j9kYnAodDj
> uN3LZ5tC2aL_Prs&e=
> >
> > >> In the skeleton code above, as I understand each tx desc holds a skb frag
> and hence there can be possibility of receiving a full-sized skb, stopping the
> queue but on receiving another normal skb we should observe our queue to
> be stopped. This doesn't arise in our case as even if the skb is full-sized, it will
> only use a single tx descriptor so we can be sure if queue has been stopped,
> the write index will only be updated once posted (and read) tx descriptors
> are processed in napi context and queues awoken.
> >
> > Please correct me if I'm wrong anywhere (sorry if so) to further my
> understanding, and again thanks for your time!
> 
> Could you please do some training on how to use normal / mailing list
> style email at Marvell? Multiple people from Marvell can't quote
> replies correctly, it makes it hard to have a conversation and help
> y'all.
Hi Jacub, apologizing for format errors on my part, will try to rectify in coming mails. Sorry again.


> -----Original Message-----
> From: Eric Dumazet <edumazet@google.com>
> Sent: Thursday, October 26, 2023 1:59 PM
> To: Shinas Rasheed <srasheed@marvell.com>
> Cc: Jakub Kicinski <kuba@kernel.org>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Haseeb Gani <hgani@marvell.com>; Vimlesh Kumar
> <vimleshk@marvell.com>; egallen@redhat.com; mschmidt@redhat.com;
> pabeni@redhat.com; horms@kernel.org; davem@davemloft.net;
> wizhao@redhat.com; konguyen@redhat.com; Veerasenareddy Burru
> <vburru@marvell.com>; Sathesh B Edara <sedara@marvell.com>
> Subject: Re: [EXT] Re: [PATCH net-next v2 3/4] octeon_ep: implement
> xmit_more in transmit
> 
> Fact that octep_start_xmit() can return NETDEV_TX_BUSY is very suspicious.
> 
> I do not think a driver can implement xmit_more and keep
> NETDEV_TX_BUSY at the same time.
> 
> Please make sure to remove NETDEV_TX_BUSY first, by stopping the queue
> earlier.

Hi Eric, I think I understand your point and shall submit an updated patch. Thanks your time!
David Laight Oct. 28, 2023, 6:38 a.m. UTC | #6
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)
Shinas Rasheed Oct. 30, 2023, 2:14 p.m. UTC | #7
Hi,

I understand the window is closed, but just replying to a pending comment on the thread.

> -----Original Message-----
> From: David Laight <David.Laight@ACULAB.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 way the octeon hardware works is that it expects number of newly updated packets to be written to the doorbell register,
which effectively increments the doorbell count which shall be decremented by hardware as it reads these packets. So in essence,
the doorbell count also indicates outstanding packets to be read by hardware.
 
> 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.

Can you elaborate what you are suggesting here to do? The driver is trying to make use of the 'xmit_more'
hint from the network stack, as any network driver might opt to do. I think avoiding continuous PCIe posts for each packet
shall still be wasteful as the hardware can bulk read from the queue if we give it a batch of packets.

> 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.

By the next packet the kernel sends, the xmit_more should be set
as far I understand, right? (as the xmit_more bool is set if skb->next is present, if the transmit path follows dev_hard_start_xmit).
David Laight Oct. 30, 2023, 3:29 p.m. UTC | #8
From: Shinas Rasheed <srasheed@marvell.com>
> Sent: 30 October 2023 14:15
> 
> Hi,
> 
> I understand the window is closed, but just replying to a pending comment on the thread.
> 
> > -----Original Message-----
> > From: David Laight <David.Laight@ACULAB.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 way the octeon hardware works is that it expects number of newly updated packets
> to be written to the doorbell register,which effectively increments the doorbell
> count which shall be decremented by hardware as it reads these packets. So in essence,
> the doorbell count also indicates outstanding packets to be read by hardware.

Unusual - I wouldn't call that a doorbell register.

> > 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.
> 
> Can you elaborate what you are suggesting here to do? The driver is trying
> to make use of the 'xmit_more' hint from the network stack, as any network
> driver might opt to do.

There are some drivers where waking up the MAC engine is expensive.
If you need to do a PCIe read then they are expensive.
There might also be drivers that need to send a USB message.
I don't actually know which one it was added for.

> I think avoiding continuous PCIe posts for each packet shall still be wasteful
> as the hardware can bulk read from the queue if we give it a batch of packets.

If you do writes for every packet then the hardware can get on with
sending the first packet and might be able to do bulk reads
for the next packet(s) when that finishes.

The extra code you are adding could easily (waving hands)
be more expensive than the posted PCIe write.
(Especially if you have to add an atomic operation.)

Unless, of course, you have to wait for it to send that batch
of packets before you can give it any more.
Which would be rather entirely broken and would really require
you do the write in the end-of-transit path.

> > 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.
> 
> By the next packet the kernel sends, the xmit_more should be set
> as far I understand, right? (as the xmit_more bool is set if skb->next
> is present, if the transmit path follows dev_hard_start_xmit).

The loop is something like:
	while (get_packet()) {
		per_cpu->xmit_more = !queue_empty();
		if (transmit_packet() != TX_OK)
			break;
	}
So if a packet is added while all the transmit setup code is running
it isn't detected.
I managed to repeatedly get that to loop when xmit_more wasn't set
and in a driver where the 'doorbell' write wasn't entirely trivial.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Shinas Rasheed Nov. 2, 2023, 1:24 p.m. UTC | #9
Hi David,

> -----Original Message-----
> From: David Laight <David.Laight@ACULAB.COM>
> Sent: Monday, October 30, 2023 9:00 PM
> To: Shinas Rasheed <srasheed@marvell.com>; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> > > > -	/* 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 way the octeon hardware works is that it expects number of newly
> updated packets
> > to be written to the doorbell register,which effectively increments the
> doorbell
> > count which shall be decremented by hardware as it reads these packets.
> So in essence,
> > the doorbell count also indicates outstanding packets to be read by
> hardware.
> 
> Unusual - I wouldn't call that a doorbell register.

I double checked in earlier implementations as well as the reference manuals.
This is how the hardware register is prescribed to be used.

> If you do writes for every packet then the hardware can get on with
> sending the first packet and might be able to do bulk reads
> for the next packet(s) when that finishes.
> 
> The extra code you are adding could easily (waving hands)
> be more expensive than the posted PCIe write.
> (Especially if you have to add an atomic operation.)
> 
> Unless, of course, you have to wait for it to send that batch
> of packets before you can give it any more.
> Which would be rather entirely broken and would really require
> you do the write in the end-of-transit path.

The atomic operation is replaced in the very next patch. Other than that,
what is it that you suggest we should 'fix' in this implementation of handling xmit_more?

> The loop is something like:
> 	while (get_packet()) {
> 		per_cpu->xmit_more = !queue_empty();
> 		if (transmit_packet() != TX_OK)
> 			break;
> 	}
> So if a packet is added while all the transmit setup code is running
> it isn't detected.
> I managed to repeatedly get that to loop when xmit_more wasn't set
> and in a driver where the 'doorbell' write wasn't entirely trivial.

How are you suggesting we handle or diagnose such a case, in the driver? Can you provide an example
reference which handles adequately this special case? 

When the net-next opens again, I can submit the patches again with the added changes you suggested. Thanks for reviewing!

Shinas
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_config.h b/drivers/net/ethernet/marvell/octeon_ep/octep_config.h
index 1622a6ebf036..ed8b1ace56b9 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_config.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_config.h
@@ -15,7 +15,7 @@ 
 /* Tx Queue: maximum descriptors per ring */
 #define OCTEP_IQ_MAX_DESCRIPTORS    1024
 /* Minimum input (Tx) requests to be enqueued to ring doorbell */
-#define OCTEP_DB_MIN                1
+#define OCTEP_DB_MIN                8
 /* Packet threshold for Tx queue interrupt */
 #define OCTEP_IQ_INTR_THRESHOLD     0x0
 
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 1c02304677c9..1a88a6bf0598 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -818,6 +818,7 @@  static netdev_tx_t octep_start_xmit(struct sk_buff *skb,
 	struct octep_iq *iq;
 	skb_frag_t *frag;
 	u16 nr_frags, si;
+	int xmit_more;
 	u16 q_no, wi;
 
 	if (skb_put_padto(skb, ETH_ZLEN))
@@ -895,18 +896,28 @@  static netdev_tx_t octep_start_xmit(struct sk_buff *skb,
 	}
 
 	netdev_tx_sent_queue(iq->netdev_q, skb->len);
+
+	xmit_more = netdev_xmit_more();
+
 	skb_tx_timestamp(skb);
 	atomic_inc(&iq->instr_pending);
+	iq->fill_cnt++;
 	wi++;
 	if (wi == iq->max_count)
 		wi = 0;
 	iq->host_write_index = wi;
+	if (xmit_more &&
+	    (atomic_read(&iq->instr_pending) <
+	     (iq->max_count - OCTEP_WAKE_QUEUE_THRESHOLD)) &&
+	    iq->fill_cnt < iq->fill_threshold)
+		return NETDEV_TX_OK;
+
 	/* Flush the hw descriptor before writing to doorbell */
 	wmb();
-
-	/* 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;
 
 dma_map_sg_err: