netdev - Re: [RFC PATCH v3 0/3] Enable virtio_net to act as a backup for a passthru device

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: <94062cc0-fe88-86c6-3dc8-40cb8422f9df@intel.com>
Date:   Tue, 20 Feb 2018 08:26:33 -0800
From:   "Samudrala, Sridhar" <sridhar.samudrala@...el.com>
To:     Jakub Kicinski <kubakici@...pl>,
        Alexander Duyck <alexander.duyck@...il.com>
Cc:     "Michael S. Tsirkin" <mst@...hat.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>,
        virtualization@...ts.linux-foundation.org,
        virtio-dev@...ts.oasis-open.org,
        "Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
        "Duyck, Alexander H" <alexander.h.duyck@...el.com>,
        Jason Wang <jasowang@...hat.com>,
        Siwei Liu <loseweigh@...il.com>
Subject: Re: [RFC PATCH v3 0/3] Enable virtio_net to act as a backup for a
 passthru device

On 2/18/2018 10:11 PM, Jakub Kicinski wrote:
> On Sat, 17 Feb 2018 09:12:01 -0800, Alexander Duyck wrote:
>>>> We noticed a couple of issues with this approach during testing.
>>>> - As both 'bypass' and 'backup' netdevs are associated with the same
>>>>    virtio pci device, udev tries to rename both of them with the same name
>>>>    and the 2nd rename will fail. This would be OK as long as the first netdev
>>>>    to be renamed is the 'bypass' netdev, but the order in which udev gets
>>>>    to rename the 2 netdevs is not reliable.
>>> Out of curiosity - why do you link the master netdev to the virtio
>>> struct device?
>> The basic idea of all this is that we wanted this to work with an
>> existing VM image that was using virtio. As such we were trying to
>> make it so that the bypass interface takes the place of the original
>> virtio and get udev to rename the bypass to what the original
>> virtio_net was.
> That makes sense.  Is it udev/naming that you're most concerned about
> here?  I.e. what's the user space app that expects the netdev to be
> linked?  This is just out of curiosity, the linking of netdevs to
> devices is a bit of a PITA in the switchdev eswitch mode world, with
> libvirt expecting only certain devices to be there..  Right now we're
> not linking VF reprs, which breaks naming.  I wanted to revisit that.

For live migration usecase, userspace is only aware of one virtio_net 
interface and
it doesn't expect it to be linked with any lower dev.  So it should be 
fine even if the
lower netdev is not present. Only the master netdev should be assigned 
the same
name so that userspace configuration scripts  in the VM don't need to 
change.

>
>>> FWIW two solutions that immediately come to mind is to export "backup"
>>> as phys_port_name of the backup virtio link and/or assign a name to the
>>> master like you are doing already.  I think team uses team%d and bond
>>> uses bond%d, soft naming of master devices seems quite natural in this
>>> case.
>> I figured I had overlooked something like that.. Thanks for pointing
>> this out. Okay so I think the phys_port_name approach might resolve
>> the original issue. If I am reading things correctly what we end up
>> with is the master showing up as "ens1" for example and the backup
>> showing up as "ens1nbackup". Am I understanding that right?
> Yes, provided systemd is new enough.

Yes. I did a quick test to confirm that adding ndo_phys_port_name() to 
virtio_net
ndo_ops fixes the udev naming issue with 2 virtio netdevs.  This is on 
fedora27.


>
>> The problem with the team/bond%d approach is that it creates a new
>> netdevice and so it would require guest configuration changes.
>>
>>> IMHO phys_port_name == "backup" if BACKUP bit is set on slave virtio
>>> link is quite neat.
>> I agree. For non-"backup" virio_net devices would it be okay for us to
>> just return -EOPNOTSUPP? I assume it would be and that way the legacy
>> behavior could be maintained although the function still exists.
> That's my understanding too.