diff mbox series

[2/2] drivers/net: Fix in e1000 and ixgbe HW rings memory overlap

Message ID 20200503162636.5233-3-Renata.Saiakhova@ekinops.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series Memory corruption due to HW rings allocation |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Renata Saiakhova May 3, 2020, 4:26 p.m. UTC 
  Delete memzones for HW rings in igb and ixgbe while freeing queues

Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
---
 drivers/net/e1000/igb_rxtx.c   | 2 ++
 drivers/net/ixgbe/ixgbe_rxtx.c | 2 ++
 2 files changed, 4 insertions(+)

Comments

Anatoly Burakov May 5, 2020, 10:28 a.m. UTC | #1 
On 03-May-20 5:26 PM, Renata Saiakhova wrote:
> Delete memzones for HW rings in igb and ixgbe while freeing queues
> 
> Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
> ---

+Thomas

Should this perhaps be fixed in all drivers, not just ixgbe/igb? Is this 
safe to do in multiprocess? I'm not too well versed in ethdev mechanics 
when it comes to multiprocess, presumably the application itself is 
responsible for synchronizing access to ports, so freeing the resources 
should be OK?
Thomas Monjalon May 5, 2020, 10:59 a.m. UTC | #2 
05/05/2020 12:28, Burakov, Anatoly:
> On 03-May-20 5:26 PM, Renata Saiakhova wrote:
> > Delete memzones for HW rings in igb and ixgbe while freeing queues
> > 
> > Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
> > ---
> 
> +Thomas
> 
> Should this perhaps be fixed in all drivers, not just ixgbe/igb? Is this 
> safe to do in multiprocess? I'm not too well versed in ethdev mechanics 
> when it comes to multiprocess, presumably the application itself is 
> responsible for synchronizing access to ports, so freeing the resources 
> should be OK?

The application is responsible of port policy.
If the application decides to close a port,
it must be safe in all threads and processes,
meaning it is application responsibility to not refer to port resources.

About fixing in all drivers, is it something missing in other drivers?
Anatoly Burakov May 5, 2020, 11:36 a.m. UTC | #3 
On 05-May-20 11:59 AM, Thomas Monjalon wrote:
> 05/05/2020 12:28, Burakov, Anatoly:
>> On 03-May-20 5:26 PM, Renata Saiakhova wrote:
>>> Delete memzones for HW rings in igb and ixgbe while freeing queues
>>>
>>> Signed-off-by: Renata Saiakhova <Renata.Saiakhova@ekinops.com>
>>> ---
>>
>> +Thomas
>>
>> Should this perhaps be fixed in all drivers, not just ixgbe/igb? Is this
>> safe to do in multiprocess? I'm not too well versed in ethdev mechanics
>> when it comes to multiprocess, presumably the application itself is
>> responsible for synchronizing access to ports, so freeing the resources
>> should be OK?
> 
> The application is responsible of port policy.
> If the application decides to close a port,
> it must be safe in all threads and processes,
> meaning it is application responsibility to not refer to port resources.
> 
> About fixing in all drivers, is it something missing in other drivers?
> 

I can see several other drivers using the dma_reserve API, so presumably 
they would suffer from the same issue, unless they use this API for a 
different purpose.
diff mbox series

Patch

diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 684fa4ad8..fe80c0f0d 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1884,12 +1884,14 @@  igb_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		eth_igb_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		eth_igb_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 2e20e18c7..977ecf513 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -3368,12 +3368,14 @@  ixgbe_dev_free_queues(struct rte_eth_dev *dev)
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		ixgbe_dev_rx_queue_release(dev->data->rx_queues[i]);
 		dev->data->rx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "rx_ring", i);
 	}
 	dev->data->nb_rx_queues = 0;
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		ixgbe_dev_tx_queue_release(dev->data->tx_queues[i]);
 		dev->data->tx_queues[i] = NULL;
+		rte_eth_dma_zone_free(dev, "tx_ring", i);
 	}
 	dev->data->nb_tx_queues = 0;
 }