linux-kernel - Re: [PATCH] vdpa/mlx5: Use random MAC address when no nic vport MAC set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9c797c93-f5a5-468b-9f35-4fc28b9c57f3@nvidia.com>
Date: Fri, 30 Aug 2024 11:43:14 +0200
From: Dragos Tatulea <dtatulea@...dia.com>
To: Cindy Lu <lulu@...hat.com>
Cc: Jason Wang <jasowang@...hat.com>, "Michael S. Tsirkin" <mst@...hat.com>,
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez
 <eperezma@...hat.com>, si-wei.liu@...cle.com, Jiri Pirko <jiri@...dia.com>,
 virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vdpa/mlx5: Use random MAC address when no nic vport MAC
 set


Hi Cindy,

On 30.08.24 11:29, Cindy Lu wrote:
> On Fri, 30 Aug 2024 at 03:03, Dragos Tatulea <dtatulea@...dia.com> wrote:
>>
>>
>>
>> On 29.08.24 12:00, Dragos Tatulea wrote:
>>>
>>>
>>> On 29.08.24 11:05, Cindy Lu wrote:
>>>> On Wed, 28 Aug 2024 at 17:37, Dragos Tatulea <dtatulea@...dia.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 28.08.24 11:00, Cindy Lu wrote:
>>>>>> On Wed, 28 Aug 2024 at 09:51, Jason Wang <jasowang@...hat.com> wrote:
>>>>>>>
>>>>>>> On Wed, Aug 28, 2024 at 12:03 AM Dragos Tatulea <dtatulea@...dia.com> wrote:
>>>>>>>>
>>>>>>>> When the vdpa device is configured without a specific MAC
>>>>>>>> address, the vport MAC address is used. However, this
>>>>>>>> address can be 0 which prevents the driver from properly
>>>>>>>> configuring the MPFS and breaks steering.
>>>>>>>>
>>>>>>>> The solution is to simply generate a random MAC address
>>>>>>>> when no MAC is set on the nic vport.
>>>>>>>>
>>>>>>>> Now it's possible to create a vdpa device without a
>>>>>>>> MAC address and run qemu with this device without needing
>>>>>>>> to configure an explicit MAC address.
>>>>>>>>
>>>>>>>> Signed-off-by: Dragos Tatulea <dtatulea@...dia.com>
>>>>>>>> Reviewed-by: Jiri Pirko <jiri@...dia.com>
>>>>>>>
>>>>>>> Acked-by: Jason Wang <jasowang@...hat.com>
>>>>>>>
>>>>>>> (Adding Cindy for double checking if it has any side effect on Qemu side)
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>> But Now there is a bug in QEMU: if the hardware MAC address does not
>>>>>> match the one in the QEMU command line, it will cause traffic loss.
>>>>>>
>>>>> Why is this a new issue in qemu? qemu in it's current state won't work
>>>>> with a different mac address that the one that is set in HW anyway.
>>>>>
>>>> this is not a new bug. We are trying to fix it because it will cause
>>>> traffic lose without any warning.
>>>> in my fix , this setting (different mac in device and Qemu) will fail
>>>> to load the VM.
>>> Which is a good thing, right? Some feedback to the user that there is
>>> a misconfig. I got bitten by this so many times... Thank you for adding it.
>>>
>>>>
>>>>>> So, Just an FYI here: if your patch merged, it may cause traffic loss.
>>>>>> and now I'm working in the fix it in qemu, the link is
>>>>>> https://patchew.org/QEMU/20240716011349.821777-1-lulu@redhat.com/
>>>>>> The idea of this fix is
>>>>>> There are will only two acceptable situations for qemu:
>>>>>> 1. The hardware MAC address is the same as the MAC address specified
>>>>>> in the QEMU command line, and both MAC addresses are not 0.
>>>>>> 2. The hardware MAC address is not 0, and the MAC address in the QEMU
>>>>>> command line is 0. In this situation, the hardware MAC address will
>>>>>> overwrite the QEMU command line address.
>>>>>>
>>>>> Why would this not work with this patch? This patch simply sets a MAC
>>>>> if the vport doesn't have one set. Which allows for more scenarios to
>>>>> work.
>>>>>
>>>> I do not mean your patch will not work, I just want to make some
>>>> clarify here.Your patch + my fix may cause the VM to fail to load in
>>>> some situations, and this is as expected.
>>>> Your patch is good to merge.
>>> Ack. Thank you for the clarification.
>> (Side note)
>> While looking at another issue I discovered that it's possible to
>> configure a random MAC on the mlx5_vdpa device at VM boot time if
>> device MAC configuration is implemented during during .set_config(). So
>> I was able to boot up a VM with a random MAC address coming from qemu
>> and the traffic worked with this new MAC.
>>
>> So now I'm not sure if this is just by luck or if the .set_config()
>> op should be implemented for the MAC part in our device.
>>
>> Thanks,
>> Dragos
>>
> Hi Dragos，
> For qemu part, I think this is not set from set_config()?  it should
> be from the CVQ?
I see that .set_config() is called during boot time. CVQ commands are
happening only when the MAC is configured from within the VM.

> Usually, we don't recommend using the set_config() function because
> the configuration space should be read-only for modern devices.
> 
Ack

> Now there is a bug in this part of qemu, and we plan to remove the
> code to set_config() in virtio_net_device_realize(), here is the patch
> https://lore.kernel.org/all/CACGkMEvCSKfahpBQLAMmSzdFN-QPhg5Zx+UQVrFX0HsWybZZNA@mail.gmail.com/T/
> and this is still under review
> 
Thanks for the clarification. So if I understand correctly,
there will be no way for qemu to set a random MAC address for vdpa
devices going forward. Unless it is done through the CVQ from within
the VM.


Thanks,
Dragos