linux-kernel - Re: [PATCH 1/1] VSOCK: Introduce VM Sockets

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: <1675407061.963254.1360682467627.JavaMail.root@vmware.com>
Date:	Tue, 12 Feb 2013 07:21:07 -0800 (PST)
From:	Andy King <acking@...are.com>
To:	Gerd Hoffmann <kraxel@...hat.com>
Cc:	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	virtualization@...ts.linux-foundation.org,
	gregkh@...uxfoundation.org, davem@...emloft.net,
	pv-drivers@...are.com
Subject: Re: [PATCH 1/1] VSOCK: Introduce VM Sockets

Hi Gerd,

> > +struct vsock_transport {
...
> Whoa.  This has grown *alot*.  Care to explain this please?  Patch
> creating a Documentation/virtual/vsock.txt would be cool.

Yes, it grew because of the notification stuff, which I'd forgotten
about until I removed the vmci header from the core code.  You are
free to use empty functions for these if they don't make any sense
for virtio.  The alternative is for us to move the entire body of
vsock_stream_recv/send into the transport, where we can hide the
notification stuff, but it seems like folks will just end up
duplicating a lot of code then.

> stream_has_data + stream_has_space + stream_rcvhiwat look like they
> are needed for buffer management.  Details please (especially for
> stream_rcvhiwat).

stream_has_data: Returns amount of data available (in bytes) in the
socket's receive buffer, or -1 if empty.

stream_has_space: Returns amount of space available (in bytes) in the
socket's send buffer, or -1 if full.

stream_rcvhiwat: The upper bound on the socket's receive buffer.
Which technically is the same as the value returned by
get_buffer_size(), so perhaps we could substitute that here and
drop this one.

> What is stream_is_active?

For the VMCI transport, it indicates if the underlying queuepair is
still around (i.e., make sure we haven't torn it down while sleeping
in a blocking send or receive).  Perhaps it's not the best name?

> What is *_allow?

It's very basic filtering.  We have specific addresses that we don't
allow, and we look for them in the allow() functions.  You can just
return true if you like.

> What are all those notify_* calls?

They're to do with signaling and flow-control.  Again, they might
not make any sense at all, but it's hard to know without having
written another transport :)

> Why do you need vsock_transport_{send,recv}_notify_data structs?
> Can't this live in vsock_sock->trans?

Those have to be setup on a per-call basis (per-thread), so it's
just easier to have them on the stack of the send/recv calls.  If
you think there's a better name, or a better way to allocate them,
I'm all ears.

Thanks!
- Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/