Re: [PATCH v5 1/1] virtio-net: Add check for mac address while peer is vdpa

[Adrian Moreno]

On 3/1/21 2:36 AM, Cindy Lu wrote:
> On Mon, Mar 1, 2021 at 4:40 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> On Thu, Feb 25, 2021 at 02:14:39PM -0500, Michael S. Tsirkin wrote:
>>> On Fri, Feb 26, 2021 at 12:55:06AM +0800, Cindy Lu wrote:
>>>> While peer is vdpa, sometime qemu get an all zero mac address from the 
>>>> hardware,
>>>> This is not a legal value. Add the check for this.if we get an zero mac 
>>>> address.
>>>> qemu will use the default mac address or the mac address from qemu cmdline
>>>>
>>>> Signed-off-by: Cindy Lu <lulu@redhat.com>
>>>
>>> I guess I will have to rewrite the comments and commit log :(
>>>
>>> It is all saying what does the patch do. We want it to rather
>>> give motivation.
>>>
>>> Sean could you please comment on whether this patch fixes your
>>> config?
>>
>> ping. if I'm to try and merge this work around it's critical
>> that someone with access to hardware confirm it actually works.
>>
> Hi Michael, I have tested this patch in qemu+vdpa+mlx environment.
> it's working fine.

I have also verified it. For the record, I'll expand:

Libvirt definition contains:

    <interface type='vdpa'>
      <mac address='52:54:00:8e:a4:12'/>
      <source dev='/dev/vhost-vdpa-0'/>
      <model type='virtio'/>
      <alias name='net1'/>
      <address type='pci' domain='0x0000' bus='0x06' slot='0x00' 
function='0x0'/>
    </interface>
        
Qemu command line contains:

-add-fd set=3,fd=42,opaque=/dev/vhost-vdpa-0 \
-netdev vhost-vdpa,vhostdev=/dev/fdset/3,id=hostnet1 \
-device
virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:8e:a4:12,bus=pci.4,addr=0x0 
\

The vdpa device is created by mlx_vdpa (Connect-X6 DX) on switchdev mode.

The original problem was:
The mac address in struct virtio_net_config in the vdpa device is zero because
it has not been configured. This does not have implications in the traffic
steering since the card will rely on the eswitch flow configuration to perform
smac/dmac filtering (in my case, this is done through ovs + tcflower hw 
offload).

Since we anyhow have to rely on the mac address having been configured
externally on the eswitch, we can also trust that whoever applied such flow
configuration also gave us the correct mac address in the cmd line. Therefore,
exposing such address to the guest seems like a sane way to tell it what mac
address to use.

Until the vdpa framework / iproute2 tool support configuring the
virtio_net_config struct and such mechanism is used externally to align the
virtio device's mac with the one configured in the eswitch, this patch allows
the guest to have connectivity.


>>
>>>> ---
>>>>  hw/net/virtio-net.c | 10 ++++++++++
>>>>  1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>>> index 9179013ac4..8f36ca5066 100644
>>>> --- a/hw/net/virtio-net.c
>>>> +++ b/hw/net/virtio-net.c
>>>> @@ -126,6 +126,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, 
>>>> uint8_t *config)
>>>>      VirtIONet *n = VIRTIO_NET(vdev);
>>>>      struct virtio_net_config netcfg;
>>>>      NetClientState *nc = qemu_get_queue(n->nic);
>>>> +    static const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
>>>>
>>>>      int ret = 0;
>>>>      memset(&netcfg, 0 , sizeof(struct virtio_net_config));
>>>> @@ -151,6 +152,15 @@ static void virtio_net_get_config(VirtIODevice *vdev, 
>>>> uint8_t *config)
>>>>          ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t 
>>>> *)&netcfg,
>>>>                                     n->config_size);
>>>>          if (ret != -1) {
>>>> +            /*
>>>> +             * Here is a work around, the 0 mac address is not a legal 
>>>> value.
>>>> +             * if we got this from hardware, qemu will use the mac address
>>>> +             * saved in VirtIONet->mac.
>>>> +             */
>>>> +            if (memcmp(&netcfg.mac, &zero, sizeof(zero)) == 0) {
>>>> +                info_report("Get an all zero mac address from hardware");

s/Get/Got/?

>>>> +                memcpy(netcfg.mac, n->mac, ETH_ALEN);
>>>> +            }
>>>>              memcpy(config, &netcfg, n->config_size);
>>>>          }
>>>>      }
>>>> --
>>>> 2.21.3
>>
> 

FWIW:
Tested-by: Adrián Moreno <amorenoz@redhat.com>

-- 
Adrián Moreno