Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
bpo-27584: New addition of vSockets to the python socket module
Support for AF_VSOCK on Linux only
  • Loading branch information
caavery committed Sep 5, 2017
commit 2f7bdaa659ea27559c8eb84e04f5a4e52dfdea31
20 changes: 20 additions & 0 deletions Doc/library/socket.rst
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ created. Socket addresses are represented as follows:

.. versionadded:: 3.6

- :const:`AF_VSOCK` allows communication between virtual machines and
their hosts. The sockets are represented as a (CID, port) tuple

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation is off here:

- :const:`AF_VSOCK` allows [...]
  their hosts. [...]

  Availability: Linux >= 4.8. QEMU >= 2.8.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: (CID, port) -> ``(CID, port)``

where the context ID or CID and port are integers.

Availability: Linux >= 4.8. QEMU >= 2.8.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to mention QEMU here? I'd say just document kernel version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I thought it would be better informationally to have it in but if it is not acceptable I can remove it.


.. versionadded:: 3.7

- Certain other address families (:const:`AF_PACKET`, :const:`AF_CAN`)
support specific representations.

Expand Down Expand Up @@ -395,6 +403,18 @@ Constants

.. versionadded:: 3.6


.. data:: AF_VSOCK
IOCTL_VM_SOCKETS_GET_LOCAL_CID
VMADDR*
SO_VM*

Constants for Linux host/guest communication.

Availability: Linux >= 4.8.

.. versionadded:: 3.7

.. data:: AF_LINK

Availability: BSD, OSX.
Expand Down
113 changes: 112 additions & 1 deletion Lib/test/test_socket.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
HOST = support.HOST
MSG = 'Michael Gilfix was here\u1234\r\n'.encode('utf-8') ## test unicode string and carriage return

VSOCKPORT = 1234

try:
import _thread as thread
import threading
Expand All @@ -44,6 +46,23 @@
except ImportError:
_socket = None

def getCid():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: Can we use an under_score name here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is get_cid acceptable?

import struct
import fcntl

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to check if fcntl is available on the platform.


if not os.path.exists("/dev/vsock"):
return None
try:
fd = open("/dev/vsock", "rb")
except:
return None
try:
r = fcntl.ioctl(fd, socket.IOCTL_VM_SOCKETS_GET_LOCAL_CID, " ")
except:
fd.close()
return None
fd.close()
return struct.unpack("I", r)[0]

def _have_socket_can():
"""Check whether CAN sockets are supported on this host."""
Expand Down Expand Up @@ -85,6 +104,12 @@ def _have_socket_alg():
s.close()
return True

def _have_socket_vsock():
"""Check whether AF_VSOCK sockets are supported on this host."""
if (getCid() == None):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use a shorter version:

return getCid() is not None

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is get_cid acceptable?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry wrong comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this syntax. It looks like return the cid if it is not None or do nothing and continue. getCid will return an integer or None. I want to translate that into return True or False in _have_socket_vsock().

@musically-ut musically-ut Jul 3, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caavery read @berkerpeksag's suggestion as a contraction of the following:

    foo = getCid is not None
    return foo

foo will contain a boolean value which will be returned.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's what I was trying to say. Sorry for being unclear!

And +1 for renaming it to get_cid, thanks!

return False
return True

HAVE_SOCKET_CAN = _have_socket_can()

HAVE_SOCKET_CAN_ISOTP = _have_socket_can_isotp()
Expand All @@ -93,6 +118,8 @@ def _have_socket_alg():

HAVE_SOCKET_ALG = _have_socket_alg()

HAVE_SOCKET_VSOCK = _have_socket_vsock()

# Size in bytes of the int type
SIZEOF_INT = array.array("i").itemsize

Expand Down Expand Up @@ -203,7 +230,6 @@ def setUp(self):
except OSError:
self.skipTest('unable to bind RDS socket')


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't make unrelated cosmetic changes.

class ThreadableTest:
"""Threadable Test class

Expand Down Expand Up @@ -387,6 +413,40 @@ def clientTearDown(self):
self.cli = None
ThreadableTest.clientTearDown(self)

@unittest.skipUnless(HAVE_SOCKET_VSOCK,
'VSOCK sockets required for this test.')
@unittest.skipUnless(getCid() != 2,
"This test can only be run on a virtual guest.")
class ThreadedVSOCKSocketStreamTest(unittest.TestCase, ThreadableTest):

def __init__(self, methodName = 'runTest'):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: No need to add spaces around =.

methodName= 'runTest'

Same for line 411:

methodName=methodName

unittest.TestCase.__init__(self, methodName = methodName)
ThreadableTest.__init__(self)

def setUp(self):
self.serv = socket.socket(socket.AF_VSOCK, socket.SOCK_STREAM)
self.addCleanup(self.serv.close)
self.serv.bind((socket.VMADDR_CID_ANY, VSOCKPORT))
self.serv.listen()
self.serverExplicitReady()
self.conn, self.connaddr = self.serv.accept()
self.addCleanup(self.conn.close)

def clientSetUp(self):
time.sleep(0.1)
self.cli = socket.socket(socket.AF_VSOCK, socket.SOCK_STREAM)
self.addCleanup(self.cli.close)
cid = getCid()
self.cli.connect((cid, VSOCKPORT))

def testStream(self):
msg = self.conn.recv(1024)
self.assertEqual(msg, MSG)

def _testStream(self):
self.cli.send(MSG)
self.cli.close()

class SocketConnectedTest(ThreadedTCPSocketTest):
"""Socket tests for client-server connection.

Expand Down Expand Up @@ -1874,6 +1934,53 @@ def _testCongestion(self):
self.assertIn(self.serv, r)


@unittest.skipUnless(HAVE_SOCKET_VSOCK,
'VSOCK sockets required for this test.')
class BasicVSOCKTest(unittest.TestCase):

def testCrucialConstants(self):
socket.AF_VSOCK

def testVSOCKConstants(self):
socket.SO_VM_SOCKETS_BUFFER_SIZE
socket.SO_VM_SOCKETS_BUFFER_MIN_SIZE
socket.SO_VM_SOCKETS_BUFFER_MAX_SIZE
socket.VMADDR_CID_ANY
socket.VMADDR_PORT_ANY
socket.VMADDR_CID_HOST
socket.VM_SOCKETS_INVALID_VERSION
socket.IOCTL_VM_SOCKETS_GET_LOCAL_CID

def testCreateSocket(self):
with socket.socket(socket.AF_VSOCK, socket.SOCK_STREAM) as s:
pass

def testSocketBufferSize(self):
with socket.socket(socket.AF_VSOCK, socket.SOCK_STREAM) as s:
orig_max = s.getsockopt(socket.AF_VSOCK,
socket.SO_VM_SOCKETS_BUFFER_MAX_SIZE)
orig = s.getsockopt(socket.AF_VSOCK,
socket.SO_VM_SOCKETS_BUFFER_SIZE)
orig_min = s.getsockopt(socket.AF_VSOCK,
socket.SO_VM_SOCKETS_BUFFER_MIN_SIZE)

s.setsockopt(socket.AF_VSOCK,
socket.SO_VM_SOCKETS_BUFFER_MAX_SIZE, orig_max * 2)
s.setsockopt(socket.AF_VSOCK,
socket.SO_VM_SOCKETS_BUFFER_SIZE, orig * 2)
s.setsockopt(socket.AF_VSOCK,
socket.SO_VM_SOCKETS_BUFFER_MIN_SIZE, orig_min * 2)

self.assertEqual(orig_max * 2,
s.getsockopt(socket.AF_VSOCK,
socket.SO_VM_SOCKETS_BUFFER_MAX_SIZE))
self.assertEqual(orig * 2,
s.getsockopt(socket.AF_VSOCK,
socket.SO_VM_SOCKETS_BUFFER_SIZE))
self.assertEqual(orig_min * 2,
s.getsockopt(socket.AF_VSOCK,
socket.SO_VM_SOCKETS_BUFFER_MIN_SIZE))

@unittest.skipUnless(thread, 'Threading required for this test.')
class BasicTCPTest(SocketConnectedTest):

Expand Down Expand Up @@ -5681,6 +5788,10 @@ def test_main():
tests.extend([BasicCANTest, CANTest])
tests.extend([BasicRDSTest, RDSTest])
tests.append(LinuxKernelCryptoAPI)
tests.extend([
BasicVSOCKTest,
ThreadedVSOCKSocketStreamTest
])
tests.extend([
CmsgMacroTests,
SendmsgUDPTest,
Expand Down
93 changes: 91 additions & 2 deletions Modules/socketmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,14 @@ makesockaddr(SOCKET_T sockfd, struct sockaddr *addr, size_t addrlen, int proto)
}
#endif /* AF_NETLINK */

#if defined(AF_VSOCK)
case AF_VSOCK:
{
struct sockaddr_vm *a = (struct sockaddr_vm *) addr;
return Py_BuildValue("II", a->svm_cid, a->svm_port);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the line failing in the Travis. https://travis-ci.org/python/cpython/jobs/248412030#L2063

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have the proper headers.
checking for linux/vm_sockets.h... no

So it looks like my checking for HAVE_LINUX_VM_SOCKETS_H is faulty.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so please update that, and push in your branch, the bots will pick up the changes and will test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've restored the original aclocal.m4 and fixed the issues you pointed out.

Thank you for your review,

Cathy

}
#endif /* AF_VSOCK */

#ifdef ENABLE_IPV6
case AF_INET6:
{
Expand Down Expand Up @@ -1586,6 +1594,32 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args,
}
#endif

#if defined(AF_VSOCK)
case AF_VSOCK:
{
struct sockaddr_vm* addr;
int port, cid;
addr = (struct sockaddr_vm *)addr_ret;
memset(addr, 0, sizeof(struct sockaddr_vm));
if (!PyTuple_Check(args)) {
PyErr_Format(
PyExc_TypeError,
"getsockaddrarg: "
"AF_VSOCK address must be tuple, not %.500s",
Py_TYPE(args)->tp_name);
return 0;
}
if (!PyArg_ParseTuple(args, "II:getsockaddrarg", &cid, &port))
return 0;
addr->svm_family = s->sock_family;
addr->svm_port = port;
addr->svm_cid = cid;
*len_ret = sizeof(*addr);
return 1;
}
#endif


#ifdef AF_RDS
case AF_RDS:
/* RDS sockets use sockaddr_in: fall-through */
Expand Down Expand Up @@ -2103,6 +2137,14 @@ getsockaddrlen(PySocketSockObject *s, socklen_t *len_ret)
}
#endif

#if defined(AF_VSOCK)
case AF_VSOCK:
{
*len_ret = sizeof (struct sockaddr_vm);
return 1;
}
#endif

#ifdef AF_RDS
case AF_RDS:
/* RDS sockets use sockaddr_in: fall-through */
Expand Down Expand Up @@ -2598,6 +2640,21 @@ sock_setsockopt(PySocketSockObject *s, PyObject *args)
unsigned int optlen;
PyObject *none;

#ifdef AF_VSOCK
if (s->sock_family == AF_VSOCK) {
uint64_t vflag;
/* setsockopt(level, opt, flag) */
if (PyArg_ParseTuple(args, "iiK:setsockopt",
&level, &optname, &vflag)) {
// level should always be set to AF_VSOCK
res = setsockopt(s->sock_fd, level, optname,
(void*)&vflag, sizeof vflag);
goto done;
}
return NULL;
}
#endif

/* setsockopt(level, opt, flag) */
if (PyArg_ParseTuple(args, "iii:setsockopt",
&level, &optname, &flag)) {
Expand Down Expand Up @@ -2668,20 +2725,39 @@ sock_getsockopt(PySocketSockObject *s, PyObject *args)
int res;
PyObject *buf;
socklen_t buflen = 0;
int flag = 0;
socklen_t flagsize;

if (!PyArg_ParseTuple(args, "ii|i:getsockopt",
&level, &optname, &buflen))
return NULL;

if (buflen == 0) {
int flag = 0;
socklen_t flagsize = sizeof flag;
#ifdef AF_VSOCK
if (s->sock_family == AF_VSOCK) {
uint64_t vflag = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been starring at this block for a minute until I realized that it handles uint64_t instead of int. Please add a comment that this block that explains the difference.

flagsize = sizeof vflag;
res = getsockopt(s->sock_fd, level, optname,
(void *)&vflag, &flagsize);
if (res < 0)
return s->errorhandler();
return PyLong_FromLong(vflag);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an uint64_t, you have to use PyLong_FromUnsignedLong().

}
#endif
flagsize = sizeof flag;
res = getsockopt(s->sock_fd, level, optname,
(void *)&flag, &flagsize);
if (res < 0)
return s->errorhandler();
return PyLong_FromLong(flag);
}
#ifdef AF_VSOCK
if (s->sock_family == AF_VSOCK) {
PyErr_SetString(PyExc_OSError,
"getsockopt string buffer not allowed");
return NULL;
}
#endif
if (buflen <= 0 || buflen > 1024) {
PyErr_SetString(PyExc_OSError,
"getsockopt buflen out of range");
Expand Down Expand Up @@ -6645,6 +6721,19 @@ PyInit__socket(void)
PyModule_AddIntMacro(m, NETLINK_CRYPTO);
#endif
#endif /* AF_NETLINK */

#ifdef AF_VSOCK
PyModule_AddIntConstant(m, "AF_VSOCK", AF_VSOCK);
PyModule_AddIntConstant(m, "SO_VM_SOCKETS_BUFFER_SIZE", 0);
PyModule_AddIntConstant(m, "SO_VM_SOCKETS_BUFFER_MIN_SIZE", 1);
PyModule_AddIntConstant(m, "SO_VM_SOCKETS_BUFFER_MAX_SIZE", 2);
PyModule_AddIntConstant(m, "VMADDR_CID_ANY", 0xffffffff);
PyModule_AddIntConstant(m, "VMADDR_PORT_ANY", 0xffffffff);
PyModule_AddIntConstant(m, "VMADDR_CID_HOST", 2);
PyModule_AddIntConstant(m, "VM_SOCKETS_INVALID_VERSION", 0xffffffff);
PyModule_AddIntConstant(m, "IOCTL_VM_SOCKETS_GET_LOCAL_CID", _IO(7, 0xb9));
#endif

#ifdef AF_ROUTE
/* Alias to emulate 4.4BSD */
PyModule_AddIntMacro(m, AF_ROUTE);
Expand Down
7 changes: 7 additions & 0 deletions Modules/socketmodule.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,13 @@ typedef int socklen_t;
#define SOL_ALG 279
#endif

#ifdef HAVE_LINUX_VM_SOCKETS_H
#include <linux/vm_sockets.h>
#ifndef AF_VSOCK
#define AF_VSOCK 40
#endif
#endif

/* Linux 3.19 */
#ifndef ALG_SET_AEAD_ASSOCLEN
#define ALG_SET_AEAD_ASSOCLEN 4
Expand Down
Loading