Re: [PATCH 3/6] vsock: add netdev to vhost/virtio vsock

[Bobby Eshleman <bobbyeshleman@gmail.com>]

On Tue, Aug 16, 2022 at 06:15:28PM -0700, Jakub Kicinski wrote:
> On Tue, 16 Aug 2022 08:29:04 +0000 Bobby Eshleman wrote:
> > > We've been burnt in the past by people doing the "let me just pick
> > > these useful pieces out of netdev" thing. Makes life hard both for
> > > maintainers and users trying to make sense of the interfaces.
> > > 
> > > What comes to mind if you're just after queuing is that we already
> > > bastardized the CoDel implementation (include/net/codel_impl.h).
> > > If CoDel is good enough for you maybe that's the easiest way?
> > > Although I suspect that you're after fairness not early drops.
> > > Wireless folks use CoDel as a second layer queuing. (CC: Toke)
> > 
> > That is certainly interesting to me. Sitting next to "codel_impl.h" is
> > "include/net/fq_impl.h", and it looks like it may solve the datagram
> > flooding issue. The downside to this approach is the baking of a
> > specific policy into vsock... which I don't exactly love either.
> > 
> > I'm not seeing too many other of these qdisc bastardizations in
> > include/net, are there any others that you are aware of?
> 
> Just what wireless uses (so codel and fq as you found out), nothing
> else comes to mind.
> 
> > > Eh, I was hoping it was a side channel of an existing virtio_net 
> > > which is not the case. Given the zero-config requirement IDK if 
> > > we'll be able to fit this into netdev semantics :(  
> > 
> > It's certainly possible that it may not fit :/ I feel that it partially
> > depends on what we mean by zero-config. Is it "no config required to
> > have a working socket" or is it "no config required, but also no
> > tuning/policy/etc... supported"?
> 
> The value of tuning vs confusion of a strange netdev floating around
> in the system is hard to estimate upfront. 

I think "a strange netdev floating around" is a total
mischaracterization... vsock is a networking device and it supports
vsock networks. Sure, it is a virtual device and the routing is done in
host software, but the same is true for virtio-net and VM-to-VM vlan.

This patch actually uses netdev for its intended purpose: to support and
manage the transmission of packets via a network device to a network.

Furthermore, it actually prepares vsock to eliminate a "strange" use of
a netdev. The netdev in vsockmon isn't even used to transmit
packets, it's "floating around" for no other reason than it is needed to
support packet capture, which vsock couldn't support because it didn't
have a netdev.

Something smells when we are required to build workaround kernel modules
that use netdev for ciphoning packets off to userspace, when we could
instead be using netdev for its intended purpose and get the same and
more benefit.

> 
> The nice thing about using a built-in fq with no user visible knobs is
> that there's no extra uAPI. We can always rip it out and replace later.
> And it shouldn't be controversial, making the path to upstream smoother.

The issue is that after pulling in fq for one kind of flow management,
then as users observe other flow issues, we will need to re-implement
pfifo, and then TBF, and then we need to build an interface to let users
select one, and to choose queue sizes... and then after awhile we've
needlessly re-implemented huge chunks of the tc system.

I don't see any good reason to restrict vsock users to using suboptimal
and rigid queuing.

Thanks.