| Message ID | 20230122100526.2302556-1-lvivier@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | virtio_net: vdpa: update MAC address when it is generated by virtio-net | expand |
On Sun, Jan 22, 2023 at 11:05:22AM +0100, Laurent Vivier wrote: > When the MAC address is not provided by the vdpa device virtio_net > driver assigns a random one without notifying the device. > The consequence, in the case of mlx5_vdpa, is the internal routing > tables of the device are not updated and this can block the > communication between two namespaces. > > To fix this problem, use virtnet_send_command(VIRTIO_NET_CTRL_MAC) > to set the address from virtnet_probe() when the MAC address is > randomly assigned from virtio_net. > > While I was testing this change I found 3 other bugs in vdpa_sim_net: > > - vdpa_sim_net sets the VIRTIO_NET_F_MAC even if no MAC address is > provided. So virtio_net doesn't generate a random MAC address and > the MAC address appears to be 00:00:00:00:00:00 > > - vdpa_sim_net never processes the command and virtnet_send_command() > hangs in an infinite loop. To avoid a kernel crash add a timeout > in the loop. > > - To allow vdpa_sim_net to process the command, replace the cpu_relax() > in the loop by a schedule(). vdpa_sim_net uses a workqueue to process > the queue, and if we don't allow the kernel to schedule, the queue > is not processed and the loop is infinite. I'd split these things out as opposed to a series unless there's a dependency I missed. All this reminds me of https://lore.kernel.org/r/20221226074908.8154-5-jasowang%40redhat.com how is this patch different/better? Pls also CC people involved in that original discussion. Thanks! > Laurent Vivier (4): > virtio_net: notify MAC address change on device initialization > virtio_net: add a timeout in virtnet_send_command() > vdpa_sim_net: don't always set VIRTIO_NET_F_MAC > virtio_net: fix virtnet_send_command() with vdpa_sim_net > > drivers/net/virtio_net.c | 21 +++++++++++++++++++-- > drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 6 ++++++ > 2 files changed, 25 insertions(+), 2 deletions(-) > > -- > 2.39.0 >
On 1/22/23 11:23, Michael S. Tsirkin wrote: > On Sun, Jan 22, 2023 at 11:05:22AM +0100, Laurent Vivier wrote: >> When the MAC address is not provided by the vdpa device virtio_net >> driver assigns a random one without notifying the device. >> The consequence, in the case of mlx5_vdpa, is the internal routing >> tables of the device are not updated and this can block the >> communication between two namespaces. >> >> To fix this problem, use virtnet_send_command(VIRTIO_NET_CTRL_MAC) >> to set the address from virtnet_probe() when the MAC address is >> randomly assigned from virtio_net. >> >> While I was testing this change I found 3 other bugs in vdpa_sim_net: >> >> - vdpa_sim_net sets the VIRTIO_NET_F_MAC even if no MAC address is >> provided. So virtio_net doesn't generate a random MAC address and >> the MAC address appears to be 00:00:00:00:00:00 >> >> - vdpa_sim_net never processes the command and virtnet_send_command() >> hangs in an infinite loop. To avoid a kernel crash add a timeout >> in the loop. >> >> - To allow vdpa_sim_net to process the command, replace the cpu_relax() >> in the loop by a schedule(). vdpa_sim_net uses a workqueue to process >> the queue, and if we don't allow the kernel to schedule, the queue >> is not processed and the loop is infinite. > > I'd split these things out as opposed to a series unless there's > a dependency I missed. We needed to fix virtio_net before fixing vdpa_sim_net otherwise the virtnet_send_command() hangs when we define the vdpa device with "vdpa dev" but without a MAC address. > All this reminds me of > https://lore.kernel.org/r/20221226074908.8154-5-jasowang%40redhat.com > > how is this patch different/better? > Pls also CC people involved in that original discussion. I was not aware of the Jason's series. It seems to address better the problem, except it triggers the ASSERT_RTNL() in virtnet_send_command() when it is called from virtnet_probe(). I will remove patches 2 and 4 from my series. PATCH 3 can be sent on independently too. Thanks, Laurent