mbox series

[0/2] Memory corruption due to HW rings allocation

Message ID 20200503162636.5233-1-Renata.Saiakhova@ekinops.com (mailing list archive)
Headers
Series Memory corruption due to HW rings allocation |

Message

Renata Saiakhova May 3, 2020, 4:26 p.m. UTC 
  igb and ixgbe drivers allocate HW rings using rte_eth_dma_zone_reserve(),
which checks first if the memzone exists for a given name, consisting of port
id, queue_id, rx/tx direction, but not for the size, alignment, and socket_id.
If the memzone with a given name exists it is returned, otherwise it is
allocated.
Disconnecting dpdk port from one type of interface (igb) and connecting it
to another type of interface (ixgbe) for the same port id, potentially creates
memory overlap and corruption, because it may require memzone of bigger size.
That's what is happening from switching from igb to ixgbe having the same port
id.

Renata Saiakhova (2):
  librte_ethdev: Introduce a function to release HW rings
  drivers/net: Fix in e1000 and ixgbe HW rings memory overlap

 drivers/net/e1000/igb_rxtx.c             |  2 ++
 drivers/net/ixgbe/ixgbe_rxtx.c           |  2 ++
 lib/librte_ethdev/rte_ethdev.c           | 23 +++++++++++++++++++++++
 lib/librte_ethdev/rte_ethdev_driver.h    | 14 ++++++++++++++
 lib/librte_ethdev/rte_ethdev_version.map |  1 +
 5 files changed, 42 insertions(+)

Comments

Thomas Monjalon May 5, 2020, 11:01 a.m. UTC | #1 
03/05/2020 18:26, Renata Saiakhova:
> igb and ixgbe drivers allocate HW rings using rte_eth_dma_zone_reserve(),
> which checks first if the memzone exists for a given name, consisting of port
> id, queue_id, rx/tx direction, but not for the size, alignment, and socket_id.
> If the memzone with a given name exists it is returned, otherwise it is
> allocated.
> Disconnecting dpdk port from one type of interface (igb) and connecting it
> to another type of interface (ixgbe) for the same port id, potentially creates
> memory overlap and corruption, because it may require memzone of bigger size.
> That's what is happening from switching from igb to ixgbe having the same port
> id.

I don't understand the use case.
Do you mean you close one port and open another one,
so the port id is recycled by ethdev?

Please could you elaborate with some code snippet?
Renata Saiakhova May 5, 2020, 11:19 a.m. UTC | #2 
Hi Thomas,

In our application dpdk port can be connected and disconnected:

Connect:
new_port_id = netdev_dpdk_get_port_by_devargs(dpdk_port->pci_addr_str);

   if (!rte_eth_dev_is_valid_port(new_port_id)) {
      /* Device not found in DPDK, attempt to attach it */
      if (rte_dev_probe(dpdk_port->pci_addr_str)) {
         new_port_id = DPDK_ETH_PORT_ID_INVALID;
      } else {
         new_port_id = netdev_dpdk_get_port_by_devargs(dpdk_port->pci_addr_str);
         if (rte_eth_dev_is_valid_port(new_port_id)) {
            LO_TRACE("Device '%s' attached to DPDK", dpdk_port->pci_addr_str);
         } else {
            /* Attach unsuccessful */
            new_port_id = DPDK_ETH_PORT_ID_INVALID;
         }
      }
   }

Disconnect:

   dp_ConfigureRxQueues(dpdk_port, FALSE);
   rte_eth_dev_set_link_down(port_id);
   rte_eth_dev_stop(port_id);
   rte_eth_dev_close(port_id);

and yes, exactly, port id is recycled by eth_dev. Next time, switching from igb port to ixgbe with the same port and using HW rings of bigger size for ixgbe creates memory corruption.

Kind regards,
Renata
Thomas Monjalon May 5, 2020, 12:35 p.m. UTC | #3 
05/05/2020 13:19, Renata Saiakhova:
> Hi Thomas,
> 
> In our application dpdk port can be connected and disconnected:
> 
> Connect:
> new_port_id = netdev_dpdk_get_port_by_devargs(dpdk_port->pci_addr_str);
> 
>    if (!rte_eth_dev_is_valid_port(new_port_id)) {
>       /* Device not found in DPDK, attempt to attach it */
>       if (rte_dev_probe(dpdk_port->pci_addr_str)) {
>          new_port_id = DPDK_ETH_PORT_ID_INVALID;
>       } else {
>          new_port_id = netdev_dpdk_get_port_by_devargs(dpdk_port->pci_addr_str);
>          if (rte_eth_dev_is_valid_port(new_port_id)) {
>             LO_TRACE("Device '%s' attached to DPDK", dpdk_port->pci_addr_str);
>          } else {
>             /* Attach unsuccessful */
>             new_port_id = DPDK_ETH_PORT_ID_INVALID;
>          }
>       }
>    }
> 
> Disconnect:
> 
>    dp_ConfigureRxQueues(dpdk_port, FALSE);
>    rte_eth_dev_set_link_down(port_id);
>    rte_eth_dev_stop(port_id);
>    rte_eth_dev_close(port_id);
> 
> and yes, exactly, port id is recycled by eth_dev. Next time, switching from igb port to ixgbe with the same port and using HW rings of bigger size for ixgbe creates memory corruption.

I see.

Reusing the same rings for a new port seems really wrong indeed.
Please check if same issue must be fixed in other PMDs.