Skip to contentSkip to conversation

i40e: Add programming descriptors to cleaned_count

PATCH In review Alexander Duyck 4 participants 6 messages Oct 22, 2017 – Oct 27, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 i40e: Add programming descriptors to cleaned_count

From: Alexander Duyck <[email protected]>

This patch updates the i40e driver to include programming descriptors in the cleaned_count. Without this change it becomes possible for us to leak memory as we don't trigger a large enough allocation when the time comes to allocate new buffers and we end up overwriting a number of rx_buffers equal to the number of programming descriptors we encountered.

Fixes 0e626ff7ccbf ("i40e: Fix support for flow director programming status")
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c |    1 +
 1 file changed, 1 insertion(+)
drivers/net/ethernet/intel/i40e/i40e_txrx.c +1 -0
index 368722defacb..d6d352a6e6ea 100644
@@ -2102,6 +2102,7 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
 
 		if (unlikely(i40e_rx_is_programming_status(qword))) {
 			i40e_clean_programming_status(rx_ring, rx_desc, qword);
+			cleaned_count++;
 			continue;
 		}
 		size = (qword & I40E_RXD_QW1_LENGTH_PBUF_MASK) >>
Tested-by on v1 1/1
#1 lore raw
Quoted 12 lines · Alexander Duyck

From: Alexander Duyck <[email protected]>

This patch updates the i40e driver to include programming descriptors in
the cleaned_count. Without this change it becomes possible for us to leak
memory as we don't trigger a large enough allocation when the time comes to
allocate new buffers and we end up overwriting a number of rx_buffers equal
to the number of programming descriptors we encountered.

Quoted 3 lines
Fixes 0e626ff7ccbf ("i40e: Fix support for flow director programming

status")

This patch solves the remaining memory leak we've seen, so

Regards,
Anders

#2 lore raw

Yes can confirm that after adding patch:

[jkirsher/net-queue PATCH] i40e: Add programming descriptors to cleaned_count

There is no memleak.

W dniu 2017-10-22 o?20:01, Anders K. Pedersen | Cohaesio pisze:

Quoted 22 lines · Alexander Duyck
Quoted 12 lines

From: Alexander Duyck <[email protected]>

This patch updates the i40e driver to include programming descriptors in
the cleaned_count. Without this change it becomes possible for us to leak
memory as we don't trigger a large enough allocation when the time comes to
allocate new buffers and we end up overwriting a number of rx_buffers equal
to the number of programming descriptors we encountered.

Quoted 3 lines
Fixes 0e626ff7ccbf ("i40e: Fix support for flow director programming

status")

This patch solves the remaining memory leak we've seen, so

Regards,
Anders

#3 lore raw
Quoted 9 lines · Pawe? Staszewski

Yes can confirm that after adding patch:

[jkirsher/net-queue PATCH] i40e: Add programming descriptors to cleaned_count

There is no memleak.

Somehow this patch isn't present in the current net-next repo. Shouldn't it be there?

Cheers,
Pavlos

#4 lore raw

from today it is in net.git

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/log/?qt=grep&q=i40e

It will be later in net-next

Also can You please tell me what firmware You are using with Your nics ?

Those are X710 ?

Thanks

Pawe?

W dniu 2017-10-27 o?23:20, Pavlos Parissis pisze:

Quoted 16 lines · Pawe? Staszewski
Quoted 9 lines

Yes can confirm that after adding patch:

[jkirsher/net-queue PATCH] i40e: Add programming descriptors to cleaned_count

There is no memleak.

Somehow this patch isn't present in the current net-next repo. Shouldn't it be there?

Cheers,
Pavlos

#5 lore raw
Quoted 7 lines · Pawe? Staszewski

from today it is in net.git

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/log/?qt=grep&q=i40e

It will be later in net-next

I have to admit I am not that familiar with repo layout, I though net-next holds what is going to be pushed to Linus's repo and potentially some parts to linux-stable.

Quoted 2 lines

Also can You please tell me what firmware You are using with Your nics ?

1.1752.0., but we use the firmware version that comes from HP, as we use HP servers, see details here
https://communities.intel.com/message/501682#501682

Quoted 1 line

Those are X710 ?

Correct.

Cheers,
Pavlos