On 1/22/2018 7:36 PM, Alexander Duyck wrote:

On Mon, Jan 22, 2018 at 6:04 PM, Michael S. Tsirkin [off-list ref] wrote:

quoted

On Mon, Jan 22, 2018 at 05:34:37PM -0800, Samudrala, Sridhar wrote:

quoted

On 1/22/2018 4:05 PM, Michael S. Tsirkin wrote:

quoted

On Mon, Jan 22, 2018 at 03:27:40PM -0800, Samudrala, Sridhar wrote:

quoted

quoted

quoted

You could probably
even handle the Tx queue selection via a simple eBPF program and map
since the input for whatever is used to select Tx should be pretty
simple, destination MAC, source NUMA node, etc, and the data-set
shouldn't be too large.

That sounds interesting. A separate device might make this kind of setup
a bit easier.  Sridhar, did you look into creating a separate device for
the virtual bond device at all?  It does not have to be in a separate
module, that kind of refactoring can come later, but once we commit to
using the same single device as virtio, we can't change that.

No. I haven't looked into creating a separate device. If we are going to
create a new
device, i guess it has to be of a new device type with its own driver.

Well not necessarily - just a separate netdev ops.
Kind of like e.g. vlans share a driver with the main driver.

Not sure what you meant by vlans sharing a driver with the main driver.
IIUC, vlans are supported via 802.1q driver and  creates a netdev of type
'vlan'
with vlan_netdev_ops

But nothing prevents a single module from registering
multiple ops.

Just to clarify, it seems like what you are suggesting is just adding
the "master" as a separate set of netdev ops or netdevice and to have
virtio spawn two network devices, one slave and one master, if the
BACKUP bit is set. Do I have that right?

I am good with the code still living in the virtio driver and
consolidation with other similar implementations and further
improvement could probably happen later as part of some refactor.

Here are the netdev_ops that are currently supported by virtio_net

    static const struct net_device_ops virtnet_netdev = {
             .ndo_open            = virtnet_open,
             .ndo_stop            = virtnet_close,
             .ndo_start_xmit      = start_xmit,
             .ndo_validate_addr   = eth_validate_addr,
             .ndo_set_mac_address = virtnet_set_mac_address,
             .ndo_set_rx_mode     = virtnet_set_rx_mode,
             .ndo_get_stats64     = virtnet_stats,
             .ndo_vlan_rx_add_vid = virtnet_vlan_rx_add_vid,
             .ndo_vlan_rx_kill_vid = virtnet_vlan_rx_kill_vid,
    #ifdef CONFIG_NET_POLL_CONTROLLER
             .ndo_poll_controller = virtnet_netpoll,
    #endif
             .ndo_bpf                = virtnet_xdp,
             .ndo_xdp_xmit           = virtnet_xdp_xmit,
             .ndo_xdp_flush          = virtnet_xdp_flush,
             .ndo_features_check     = passthru_features_check,
    };

Now if we want to create 2 netdevs associated with a single 
virtio_device, do we want these ops
supported by master or slave netdev? I guess these should be supported 
by the slave netdev.
So do we want the netdev_ops of master simply call the slave netdev_ops 
for most the the cases
except for the ndo_start_xmit() which will decide between virtio or vf 
datapath?

what about ethtool_ops? we need to sync up link state between master and 
slave netdevs and
the userspace needs to be aware of 2 type of virtio_net devices.

Is this complexity really required to support a feature that can only be 
supported/useful for
simple guest network configurations that can be controlled by 
hypervisor.   virtio_net device
that could be accelerated via a passthru device and support live migration.

If a guest admin needs to configure any complex network configurations 
in the guest, i think those
scenarios can be supported via bond/bridge/team setups and live 
migration may not be a requirement
for such usecases.

Thanks
Sridhar