Re: [PATCH v4 2/2] virtio_net: Extend virtio to use VF datapath when available

On Mon, Mar 5, 2018 at 7:15 PM, Stephen Hemminger
[off-list ref] wrote:

On Mon, 5 Mar 2018 14:47:20 -0800
Alexander Duyck [off-list ref] wrote:

quoted

On Mon, Mar 5, 2018 at 2:30 PM, Jiri Pirko [off-list ref] wrote:

quoted

Mon, Mar 05, 2018 at 05:11:32PM CET, stephen@networkplumber.org wrote:

quoted

On Mon, 5 Mar 2018 10:21:18 +0100
Jiri Pirko [off-list ref] wrote:

quoted

Sun, Mar 04, 2018 at 10:58:34PM CET, alexander.duyck@gmail.com wrote:

quoted

On Sun, Mar 4, 2018 at 10:50 AM, Jiri Pirko [off-list ref] wrote:

quoted

Sun, Mar 04, 2018 at 07:24:12PM CET, alexander.duyck@gmail.com wrote:

quoted

On Sat, Mar 3, 2018 at 11:13 PM, Jiri Pirko [off-list ref] wrote:

[...]

quoted

quoted

quoted

Currently we only have agreement from Michael on taking this code, as
such we are working with virtio only for now. When the time comes that

If you do duplication of netvsc in-driver bonding in virtio_net, it will
stay there forever. So what you say is: "We will do it halfway now
and promise to fix it later". That later will never happen, I'm pretty
sure. That is why I push for in-driver bonding shared code as a part of
this patchset.

You want this new approach and a copy of netvsc moved into either core
or some module of its own. I say pick an architecture. We are looking
at either 2 netdevs or 3. We are not going to support both because
that will ultimately lead to a terrible user experience and make
things quite confusing.

quoted

+ if you would be pushing first driver to do this, I would understand.
But the first driver is already in. You are pushing second. This is the
time to do the sharing, unification of behaviour. Next time is too late.

That is great, if we want to share then lets share. But what you are
essentially telling us is that we need to fork this solution and
maintain two code paths, one for 2 netdevs, and another for 3. At that
point what is the point in merging them together?

Of course, I vote for the same behaviour for netvsc and virtio_net. That
is my point from the very beginning.

Stephen, what do you think? Could we please make virtio_net and netvsc
behave the same and to use a single code with well-defined checks and
restrictions for this feature?

Eventually, yes both could share common code routines. In reality,
the failover stuff is only a very small part of either driver so
it is not worth stretching to try and cover too much. If you look,
the failover code is just using routines that already exist for
use by bonding, teaming, etc.

Yeah, we consern was also about the code that processes the netdev
notifications and does auto-enslave and all related stuff.

The concern was the driver model. If we expose 3 netdevs or 2 with the
VF driver present. Somehow this is turning into a "merge netvsc into
virtio" think and that isn't the subject that was being asked.

Ideally we want one model for this. Either 3 netdevs or 2. The problem
is 2 causes issues in terms of performance and will limit features of
virtio, but 2 is the precedent set by netvsc. We need to figure out
the path forward for this. There is talk about "sharing" but it is
hard to make these two approaches share code when they are doing two
very different setups and end up presenting themselves as two very
different driver models.

I appreciate this discussion, and it has helped a lot.

Netvsc is stuck with 2 netdev model for the foreseeable future.
We already failed once with the bonding model, and that created a lot of
pain. The current model is working well and have convinced the major distros
to support the two netdev model and don't want to back.

Very open to optimizations and ways to smooth out the rough edges.

Thank you for clarifying this Stephen.

Okay. So with things defined such that we are doing a 2 netdev model
for netvsc, and a 3 netdev model for virtio, is it still in our
interest for us to try making a shared library between the two? In my
mind, the virtnet_bypass becomes the way we go forward for any future
solutions. I say we treat the netvsc approach as a "legacy" approach
and avoid creating any new libraries or drivers to support it, and
instead just focus on the 3 netdev approach as the way this is to be
done going forward. That way we avoid anyone else trying to implement
something like the 2 netdev solution in the future.

So getting back to the code here. Should we split the virtnet_bypass
code out into a separate module? My preference would be to let this
incubate as a part of virtio_net until either there is another user,
or it becomes big enough that it needs to be moved. That said, there
can be arguments for either keeping it in, or making it a module on
its own depending on where we see this going in the future.

- Alex