Linux Virtualization - Jun 2018 - [virtio-dev] Re: [Qemu-devel] [PATCH] qemu: Introduce VIRTIO_NET_F_STANDBY feature bit to virtio_net

On 6/22/2018 5:17 PM, Siwei Liu wrote:
> On Fri, Jun 22, 2018 at 4:40 PM, Siwei Liu <loseweigh at gmail.com>
wrote:
>> On Fri, Jun 22, 2018 at 3:25 PM, Michael S. Tsirkin <mst at
redhat.com> wrote:
>>> On Fri, Jun 22, 2018 at 02:51:11PM -0700, Siwei Liu wrote:
>>>> On Fri, Jun 22, 2018 at 2:29 PM, Michael S. Tsirkin <mst at
redhat.com> wrote:
>>>>> On Fri, Jun 22, 2018 at 01:03:04PM -0700, Siwei Liu wrote:
>>>>>> On Fri, Jun 22, 2018 at 1:00 PM, Siwei Liu
<loseweigh at gmail.com> wrote:
>>>>>>> On Thu, Jun 21, 2018 at 7:32 PM, Michael S. Tsirkin
<mst at redhat.com> wrote:
>>>>>>>> On Thu, Jun 21, 2018 at 06:21:55PM -0700, Siwei
Liu wrote:
>>>>>>>>> On Thu, Jun 21, 2018 at 7:59 AM, Cornelia
Huck <cohuck at redhat.com> wrote:
>>>>>>>>>> On Wed, 20 Jun 2018 22:48:58 +0300
>>>>>>>>>> "Michael S. Tsirkin" <mst
at redhat.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> On Wed, Jun 20, 2018 at 06:06:19PM
+0200, Cornelia Huck wrote:
>>>>>>>>>>>> In any case, I'm not sure
anymore why we'd want the extra uuid.
>>>>>>>>>>> It's mostly so we can have e.g.
multiple devices with same MAC
>>>>>>>>>>> (which some people seem to want in
order to then use
>>>>>>>>>>> then with different containers).
>>>>>>>>>>>
>>>>>>>>>>> But it is also handy for when you
assign a PF, since then you
>>>>>>>>>>> can't set the MAC.
>>>>>>>>>>>
>>>>>>>>>> OK, so what about the following:
>>>>>>>>>>
>>>>>>>>>> - introduce a new feature bit,
VIRTIO_NET_F_STANDBY_UUID that indicates
>>>>>>>>>>    that we have a new uuid field in the
virtio-net config space
>>>>>>>>>> - in QEMU, add a property for
virtio-net that allows to specify a uuid,
>>>>>>>>>>    offer VIRTIO_NET_F_STANDBY_UUID if
set
>>>>>>>>>> - when configuring, set the property to
the group UUID of the vfio-pci
>>>>>>>>>>    device
>>>>>>>>> If feature negotiation fails on
VIRTIO_NET_F_STANDBY_UUID, is it safe
>>>>>>>>> to still expose UUID in the config space on
virtio-pci?
>>>>>>>>
>>>>>>>> Yes but guest is not supposed to read it.
>>>>>>>>
>>>>>>>>> I'm not even sure if it's sane to
expose group UUID on the PCI bridge
>>>>>>>>> where the corresponding vfio-pci device
attached to for a guest which
>>>>>>>>> doesn't support the feature (legacy).
>>>>>>>>>
>>>>>>>>> -Siwei
>>>>>>>> Yes but you won't add the primary behind
such a bridge.
>>>>>>> I assume the UUID feature is a new one besides the
exiting
>>>>>>> VIRTIO_NET_F_STANDBY feature, where I think the
current proposal is,
>>>>>>> if UUID feature is present and supported by guest,
we'll do pairing
>>>>>>> based on UUID; if UUID feature present
>>>>>>                                   ^^^^^^^  is NOT
present
>>>>> Let's go back for a bit.
>>>>>
>>>>> What is wrong with matching by mac?
>>>>>
>>>>> 1. Does not allow multiple NICs with same mac
>>>>> 2. Requires MAC to be programmed by host in the PT device
>>>>>     (which is often possible with VFs but isn't
possible with all PT
>>>>>     devices)
>>>> Might not neccessarily be something wrong, but it's very
limited to
>>>> prohibit the MAC of VF from changing when enslaved by failover.
>>> You mean guest changing MAC? I'm not sure why we prohibit that.
>> I think Sridhar and Jiri might be better person to answer it. My
>> impression was that sync'ing the MAC address change between all 3
>> devices is challenging, as the failover driver uses MAC address to
>> match net_device internally.
Yes. The MAC address is assigned by the hypervisor and it needs to manage the
movement
of the MAC between the PF and VF.? Allowing the guest to change the MAC will
require
synchronization between the hypervisor and the PF/VF drivers. Most of the VF
drivers
don't allow changing guest MAC unless it is a trusted VF.

On Mon, Jun 25, 2018 at 10:54:09AM -0700, Samudrala, Sridhar
wrote:
> > > > > Might not neccessarily be something wrong, but it's
very limited to
> > > > > prohibit the MAC of VF from changing when enslaved by
failover.
> > > > You mean guest changing MAC? I'm not sure why we
prohibit that.
> > > I think Sridhar and Jiri might be better person to answer it. My
> > > impression was that sync'ing the MAC address change between
all 3
> > > devices is challenging, as the failover driver uses MAC address
to
> > > match net_device internally.
> 
> Yes. The MAC address is assigned by the hypervisor and it needs to manage
the movement
> of the MAC between the PF and VF.? Allowing the guest to change the MAC
will require
> synchronization between the hypervisor and the PF/VF drivers. Most of the
VF drivers
> don't allow changing guest MAC unless it is a trusted VF.
OK but it's a policy thing. Maybe it's a trusted VF. Who knows?
For example I can see host just
failing VIRTIO_NET_CTRL_MAC_ADDR_SET if it wants to block it.
I'm not sure why VIRTIO_NET_F_STANDBY has to block it in the guest.

-- 
MST

On Tue, 26 Jun 2018 04:50:25 +0300
"Michael S. Tsirkin" <mst at redhat.com> wrote:
> On Mon, Jun 25, 2018 at 10:54:09AM -0700, Samudrala, Sridhar wrote:
> > > > > > Might not neccessarily be something wrong, but
it's very limited to
> > > > > > prohibit the MAC of VF from changing when enslaved
by failover.
> > > > > You mean guest changing MAC? I'm not sure why we
prohibit that.
> > > > I think Sridhar and Jiri might be better person to answer
it. My
> > > > impression was that sync'ing the MAC address change
between all 3
> > > > devices is challenging, as the failover driver uses MAC
address to
> > > > match net_device internally.  
> > 
> > Yes. The MAC address is assigned by the hypervisor and it needs to
manage the movement
> > of the MAC between the PF and VF.? Allowing the guest to change the
MAC will require
> > synchronization between the hypervisor and the PF/VF drivers. Most of
the VF drivers
> > don't allow changing guest MAC unless it is a trusted VF.  
> 
> OK but it's a policy thing. Maybe it's a trusted VF. Who knows?
> For example I can see host just
> failing VIRTIO_NET_CTRL_MAC_ADDR_SET if it wants to block it.
> I'm not sure why VIRTIO_NET_F_STANDBY has to block it in the guest.
> 
So, what I get from this is that QEMU needs to be able to control all
of standby, uuid, and mac to accommodate the different setups
(respectively have libvirt/management software set it up). Is the host
able to find out respectively define whether a VF is trusted?

On Mon, Jun 25, 2018 at 6:50 PM, Michael S. Tsirkin <mst at redhat.com>
wrote:
> On Mon, Jun 25, 2018 at 10:54:09AM -0700, Samudrala, Sridhar wrote:
>> > > > > Might not neccessarily be something wrong, but
it's very limited to
>> > > > > prohibit the MAC of VF from changing when enslaved
by failover.
>> > > > You mean guest changing MAC? I'm not sure why we
prohibit that.
>> > > I think Sridhar and Jiri might be better person to answer it.
My
>> > > impression was that sync'ing the MAC address change
between all 3
>> > > devices is challenging, as the failover driver uses MAC
address to
>> > > match net_device internally.
>>
>> Yes. The MAC address is assigned by the hypervisor and it needs to
manage the movement
>> of the MAC between the PF and VF.  Allowing the guest to change the MAC
will require
>> synchronization between the hypervisor and the PF/VF drivers. Most of
the VF drivers
>> don't allow changing guest MAC unless it is a trusted VF.
>
> OK but it's a policy thing. Maybe it's a trusted VF. Who knows?
> For example I can see host just
> failing VIRTIO_NET_CTRL_MAC_ADDR_SET if it wants to block it.
> I'm not sure why VIRTIO_NET_F_STANDBY has to block it in the guest.
That's why I think pairing using MAC is fragile IMHO. When VF's MAC
got changed before virtio attempts to match and pair the device, it
ends up with no pairing found out at all. UUID is better.

-Siwei
>
> --
> MST