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

[Long Li]

Thank you, Alexander.

The patch looks good to me. With this patch, do you see other problems with VPP?

I'd like to get a review from @Sam Andrew<mailto:samandrew at microsoft.com>.

Acked-by: Long Li <longli at microsoft.com>


From: Alexander Skorichenko <askorichenko at netgate.com>
Sent: Tuesday, July 2, 2024 1:49 AM
To: stephen at networkplumber.org
Cc: Sam Andrew <samandrew at microsoft.com>; ferruh.yigit at amd.com; andrew.rybchenko at oktetlabs.ru; Long Li <longli at microsoft.com>; Wei Hu <weh at microsoft.com>; dev at dpdk.org; Matthew Smith <mgsmith at netgate.com>
Subject: Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices

You don't often get email from askorichenko at netgate.com<mailto:askorichenko at netgate.com>. Learn why this is important<https://aka.ms/LearnAboutSenderIdentification>
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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/dev/attachments/20240703/b423ea5a/attachment.htm>