netdev - Re: [PATCH 2/5] virtio_net: Add a virtqueue for outbound control commands

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]

Date:	Wed, 28 Jan 2009 23:35:13 +1030
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Alex Williamson <alex.williamson@...com>
Cc:	netdev@...r.kernel.org, markmc@...hat.com, kvm@...r.kernel.org
Subject: Re: [PATCH 2/5] virtio_net: Add a virtqueue for outbound control commands

On Tuesday 27 January 2009 14:30:06 Alex Williamson wrote:
> Hi Rusty,
> 
> On Tue, 2009-01-27 at 13:52 +1030, Rusty Russell wrote:
> > On Saturday 17 January 2009 07:43:23 Alex Williamson wrote:
> > > + return status ? -EFAULT : 0;
> > 
> > This is wrong. Currently this can't happen, right? But you put it in
> > so someone in future may want some kind of return from the commands.
> > In which case, they'll want to see the value.
> > 
> > If we're sure they never want to see the value, then we don't need to
> > be synchronous at all: just spin if add_buf() fails.
> 
> In my qemu series of patches it can happen if the command isn't properly
> defined

ie. guest bug.

> , something bad happens

???  You're going to tell me to read the code, aren't you? :)

> , or the command is unrecognized.

If we go for feature bits, this is also a guest bug.  And I think we should, since that's what feature bits are for.

> As I
> was hashing this out, I first had more errnos, but I wasn't sure how
> extensively to fill out the error returns, and eventually defaulted to
> ok/fail.  Should I expand on these some?  Suggestions for a reasonable
> small yet complete set?  Should we use Linux errno values and let other
> OS virtio-net implementations create a switch table?

Not error codes.  In future we may have a command where return codes are meaningful, but I'm pretty sure they'll be command specific so we don't know how to define them now anyway.  AFAICT that's the only real justification for defining this interface as sync.

So I think just defining 0 on success is fine.  Defining that to always happen to well-formed commands is even better :)

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html