Virtualization - May 2018 - [PATCH net-next v9 3/4] virtio_net: Extend virtio to use VF datapath when available

On 4/30/2018 12:20 AM, Jiri Pirko wrote:
>
>>> Now I try to change mac of the failover master:
>>> [root at test1 ~]# ip link set ens3 addr 52:54:00:b2:a7:f3
>>> RTNETLINK answers: Operation not supported
>>>
>>> That I did expect to work. I would expect this would change the mac
of
>>> the master and both standby and primary slaves.
>> If a VF is untrusted, a VM will not able to change its MAC and moreover
> Note that at this point, I have no VF. So I'm not sure why you mention
> that.
>
>
>> in this mode we are assuming that the hypervisor has assigned the MAC
and
>> guest is not expected to change the MAC.
> Wait, for ordinary old-fashioned virtio_net, as a VM user, I can change
> mac and all works fine. How is this different? Change mac on "failover
> instance" should work and should propagate the mac down to its slaves.
>
>
>> For the initial implementation, i would propose not allowing the guest
to
>> change the MAC of failover or standby dev.
> I see no reason for such restriction.
>
It is true that a VM user can change mac address of a normal virtio-net
interface,
however when it is in STANDBY mode i think we should not allow this change
specifically
because we are creating a failover instance based on a MAC that is assigned by
the
hypervisor.

Moreover,? in a cloud environment i would think that PF/hypervisor assigns a MAC
to
the VF and it cannot be changed by the guest.

So for the initial implementation, do you see any issues with having this
restriction
in STANDBY mode.

Wed, May 02, 2018 at 02:20:26AM CEST, sridhar.samudrala at intel.com
wrote:
>On 4/30/2018 12:20 AM, Jiri Pirko wrote:
>> 
>> > > Now I try to change mac of the failover master:
>> > > [root at test1 ~]# ip link set ens3 addr 52:54:00:b2:a7:f3
>> > > RTNETLINK answers: Operation not supported
>> > > 
>> > > That I did expect to work. I would expect this would change
the mac of
>> > > the master and both standby and primary slaves.
>> > If a VF is untrusted, a VM will not able to change its MAC and
moreover
>> Note that at this point, I have no VF. So I'm not sure why you
mention
>> that.
>> 
>> 
>> > in this mode we are assuming that the hypervisor has assigned the
MAC and
>> > guest is not expected to change the MAC.
>> Wait, for ordinary old-fashioned virtio_net, as a VM user, I can change
>> mac and all works fine. How is this different? Change mac on
"failover
>> instance" should work and should propagate the mac down to its
slaves.
>> 
>> 
>> > For the initial implementation, i would propose not allowing the
guest to
>> > change the MAC of failover or standby dev.
>> I see no reason for such restriction.
>> 
>
>It is true that a VM user can change mac address of a normal virtio-net
interface,
>however when it is in STANDBY mode i think we should not allow this change
specifically
>because we are creating a failover instance based on a MAC that is assigned
by the
>hypervisor.
>
>Moreover,? in a cloud environment i would think that PF/hypervisor assigns a
MAC to
>the VF and it cannot be changed by the guest.
So that is easy. You allow the change of the mac and in the "failover"
mac change implementation you propagate the change down to slaves. If
one slave does not support the change, you bail out. And since VF does
not allow it as you say, once it will be enslaved, the mac change could
not be done. Seems like a correct behavior to me and is in-sync with how
bond/team behaves.

>
>So for the initial implementation, do you see any issues with having this
restriction
>in STANDBY mode.
>
>

On 5/2/2018 12:50 AM, Jiri Pirko wrote:
> Wed, May 02, 2018 at 02:20:26AM CEST, sridhar.samudrala at intel.com wrote:
>> On 4/30/2018 12:20 AM, Jiri Pirko wrote:
>>>>> Now I try to change mac of the failover master:
>>>>> [root at test1 ~]# ip link set ens3 addr 52:54:00:b2:a7:f3
>>>>> RTNETLINK answers: Operation not supported
>>>>>
>>>>> That I did expect to work. I would expect this would change
the mac of
>>>>> the master and both standby and primary slaves.
>>>> If a VF is untrusted, a VM will not able to change its MAC and
moreover
>>> Note that at this point, I have no VF. So I'm not sure why you
mention
>>> that.
>>>
>>>
>>>> in this mode we are assuming that the hypervisor has assigned
the MAC and
>>>> guest is not expected to change the MAC.
>>> Wait, for ordinary old-fashioned virtio_net, as a VM user, I can
change
>>> mac and all works fine. How is this different? Change mac on
"failover
>>> instance" should work and should propagate the mac down to its
slaves.
>>>
>>>
>>>> For the initial implementation, i would propose not allowing
the guest to
>>>> change the MAC of failover or standby dev.
>>> I see no reason for such restriction.
>>>
>> It is true that a VM user can change mac address of a normal virtio-net
interface,
>> however when it is in STANDBY mode i think we should not allow this
change specifically
>> because we are creating a failover instance based on a MAC that is
assigned by the
>> hypervisor.
>>
>> Moreover,? in a cloud environment i would think that PF/hypervisor
assigns a MAC to
>> the VF and it cannot be changed by the guest.
> So that is easy. You allow the change of the mac and in the
"failover"
> mac change implementation you propagate the change down to slaves. If
> one slave does not support the change, you bail out. And since VF does
> not allow it as you say, once it will be enslaved, the mac change could
> not be done. Seems like a correct behavior to me and is in-sync with how
> bond/team behaves.
If we allow the mac to be changed when the VF is not yet plugged in
and the guest changes the mac, then VF cannot join the failover when
it is hot plugged with the original mac assigned by the hypervisor.
Specifically there could be timing window during the live migration where
VF would be unplugged at the source and if we allow the guest to change the
failover mac, then VF would not be able to register with the failover after
the VM is migrated to the destination.
>
>> So for the initial implementation, do you see any issues with having
this restriction
>> in STANDBY mode.
>>
>>

On Wed, May 02, 2018 at 09:50:21AM +0200, Jiri Pirko
wrote:
> Wed, May 02, 2018 at 02:20:26AM CEST, sridhar.samudrala at intel.com wrote:
> >On 4/30/2018 12:20 AM, Jiri Pirko wrote:
> >> 
> >> > > Now I try to change mac of the failover master:
> >> > > [root at test1 ~]# ip link set ens3 addr
52:54:00:b2:a7:f3
> >> > > RTNETLINK answers: Operation not supported
> >> > > 
> >> > > That I did expect to work. I would expect this would
change the mac of
> >> > > the master and both standby and primary slaves.
> >> > If a VF is untrusted, a VM will not able to change its MAC
and moreover
> >> Note that at this point, I have no VF. So I'm not sure why you
mention
> >> that.
> >> 
> >> 
> >> > in this mode we are assuming that the hypervisor has assigned
the MAC and
> >> > guest is not expected to change the MAC.
> >> Wait, for ordinary old-fashioned virtio_net, as a VM user, I can
change
> >> mac and all works fine. How is this different? Change mac on
"failover
> >> instance" should work and should propagate the mac down to
its slaves.
> >> 
> >> 
> >> > For the initial implementation, i would propose not allowing
the guest to
> >> > change the MAC of failover or standby dev.
> >> I see no reason for such restriction.
> >> 
> >
> >It is true that a VM user can change mac address of a normal virtio-net
interface,
> >however when it is in STANDBY mode i think we should not allow this
change specifically
> >because we are creating a failover instance based on a MAC that is
assigned by the
> >hypervisor.
> >
> >Moreover,? in a cloud environment i would think that PF/hypervisor
assigns a MAC to
> >the VF and it cannot be changed by the guest.
> 
> So that is easy. You allow the change of the mac and in the
"failover"
> mac change implementation you propagate the change down to slaves. If
> one slave does not support the change, you bail out. And since VF does
I wish people would say primary/standby and not "VF" :)
> not allow it as you say, once it will be enslaved, the mac change could
> not be done. Seems like a correct behavior to me

what if primary does not allow mac changes and is attached after
mac is changed on standy?

> and is in-sync with how
> bond/team behaves.
I think in the end virtio can just block MAC changes for simplicity.

I personally don't see softmac as a must have feature in v1,
we can add it later.

What's the situation with init scripts and whether it's
possible to make them work well would be a better question.
> 
> >
> >So for the initial implementation, do you see any issues with having
this restriction
> >in STANDBY mode.
> >
> >