Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices

[Alexander Skorichenko <askorichenko@netgate.com>]

[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]

Hello,
The failure happens when running under VPP. In terms of dpdk calls the
following sequence of events occurs for a NetVSC device:

Durung device probing stage of VPP
   1. eth_hn_dev_init() memory for a single primary rx_queue is allocated
            dev->data->rx_queues[0] = 0, allocated, but not set yet
            no allocation happened for tx_queues[i] and non-primary
rx_queues[i]

During device setup stage of VPP from VPP's own generic dpdk_device_setup():
   2.  rte_eth_dev_set_mtu()
            currently it segfaults in hn_reinit() when trying to reach into
            dev->data->rx_queues[i], dev->data->tx_queues[i]
   3.  rte_eth_tx_queue_setup()
            dev->data->tx_queues[i] are being allocated and set
   4.  rte_eth_rx_queue_setup()
            dev->data->rx_queues[i] get allocated (i > 0) and set

So rx_queues[0] could be set in step 1, but rx_queues[i] and tx_queues[i]
are still NULL.
Allocating all the remaining rx/tx queues in step 1 would prevent the
crash, but then in steps 3-4 would go through releasing and allocating all
of the queues again.

Another comment regarding the uniform condition introduced in hn_reinit()
in the patch:
        if (dev->data->rx_queues[0] != NULL) ...
Because of the difference between rx/tx queues described above, it's
probably safer to extend the condition to check both rx and tx separately
        if (dev->data->rx_queues[0] != NULL && dev->data->tx_queues[0] !=
NULL)

- Alexander Skorichenko


On Sun, Jun 30, 2024 at 5:40 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Fri, 28 Jun 2024 18:35:03 +0200
> Alexander Skorichenko <askorichenko@netgate.com> wrote:
>
> > Prevent segfault in hn_reinit() caused by changing the MTU for
> > an incompletely initialized device.
> >
> > Signed-off-by: Alexander Skorichenko <askorichenko@netgate.com>
>
> How do you get in that state?
> Maybe the init code should set up these pointers and avoid the problem.
>

[-- Attachment #2: Type: text/html, Size: 2656 bytes --]