classification
Title: Support needed for AF_RDS family
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Andrew.Grover, brian.curtin, haypo, loewis, neologix, pitrou, python-dev
Priority: normal Keywords: needs review, patch

Created on 2010-01-25 07:27 by Andrew.Grover, last changed 2011-11-10 21:42 by neologix. This issue is now closed.

Files
File name Uploaded Description Edit
socket_rds.diff neologix, 2011-11-01 19:38 review
socket_rds-1.diff neologix, 2011-11-01 21:50 review
socket_rds-2.diff neologix, 2011-11-06 16:29 review
Messages (11)
msg98271 - (view) Author: Andrew Grover (Andrew.Grover) Date: 2010-01-25 07:27
RDS is a reliable datagram protocol used by Oracle clusters for inter-process communication. It is in the Linux kernel, and has a defined address family number. Its use is identical to UDP, except the address family is 21, and the type is SOCK_SEQPACKET.

So, what's this got to do with Python? :)

Apparently Modules/socketmodule.c getsockaddrarg() checks bind() args, and only allows known socket types to bind. Attempting to bind with an RDS socket fails.

It looks pretty straightforward to add support for a new family, but before doing so I wanted to check whether this was likely to be accepted, and also to ask if it wouldn't make more sense for getsockaddrarg() to not default to failing unknown families, but instead letting them through? If the params are wrong for a non-enumerated family, bind() will presumably return an error that the user will get.
msg98272 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-01-25 07:33
A patch has good chances of getting accepted (eventually; it may take several months or years). Test cases and documentation changes should be included.

Supporting generic socket addresses can't work (IMO): how could Python possibly know how to map a Python representation of the address (which is likely a tuple of some kind) onto a struct sockaddr?
msg146802 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-11-01 19:38
Here's a patch adding support for RDS sockets (with test and documentation update).
msg146805 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-11-01 20:44
A couple of things:
- the tests are skipped with "unable to bind RDS socket" here (even under root). Is it expected?
- socket.error is the same as OSError now
- there are some ResourceWarnings abount unclosed sockets when running the tests
msg146813 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-11-01 21:50
> - the tests are skipped with "unable to bind RDS socket" here (even
> under root). Is it expected?

Oops. I did a quick test to force the socket family (because RDS sockets really use sockaddr_in), and apparently I overwrote my original patch... Here's the correct one.
As long as the `rds` module is loaded, all the tests should pass.

> - socket.error is the same as OSError now

I changed this in this patch.
I'll update the other occurrences in test_socket in a separate patch.

> - there are some ResourceWarnings abount unclosed sockets when
> running the tests

Same thing here, I fixed those in my original patch, then somehow overwrote it with a previous version...
msg147077 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-11-05 10:36
So, what do you think of the new patch?
msg147084 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-11-05 12:53
Oops, sorry. I wonder if it would be possible to test the address returned by recvfrom(). Same for the flags and ancillary data in recvmsg().
msg147146 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-11-06 07:40
I think the patch currently lacks a lot of symbolic constants; see my review.
msg147161 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-11-06 16:29
Here's an updated patch:
- address returned by recvfrom()
- recv flags (MSG_PEEK)
- congestion behavior

I've also added a bunch of constants:
- all the typical SO_ constants
- CMSG flags
- RDMA-related options (RDMA is probably one of the most useful features of RDS)
- I chose to let aside INFO flags, since they don't appear in the man pages  (and neither here: https://github.com/agrover/python-rds/blob/master/rdma.py, a ctypes-based RDS implementation I stumbled upon)
- I didn't add control messages tests: they all pertain to RDMA, which is rather tricky to test to test from Python (and my header files don't define them)
msg147185 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-11-06 21:02
Not having tests for the control messages is fine with me - we don't need to test Linux, but Python.
msg147408 - (view) Author: Roundup Robot (python-dev) Date: 2011-11-10 18:23
New changeset 2293ca739223 by Charles-François Natali in branch 'default':
Issue #7777: socket: Add Reliable Datagram Sockets (PF_RDS) support.
http://hg.python.org/cpython/rev/2293ca739223
History
Date User Action Args
2011-11-10 21:42:49neologixsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2011-11-10 18:23:35python-devsetnosy: + python-dev
messages: + msg147408
2011-11-06 21:02:54loewissetmessages: + msg147185
2011-11-06 16:29:56neologixsetfiles: + socket_rds-2.diff

messages: + msg147161
2011-11-06 07:40:15loewissetmessages: + msg147146
2011-11-05 12:53:06pitrousetmessages: + msg147084
2011-11-05 10:36:43neologixsetmessages: + msg147077
2011-11-01 21:50:55neologixsetfiles: + socket_rds-1.diff

messages: + msg146813
2011-11-01 20:44:44pitrousetmessages: + msg146805
2011-11-01 19:38:36neologixsetfiles: + socket_rds.diff

versions: + Python 3.3, - Python 2.7, Python 3.2
keywords: + patch, needs review
nosy: + pitrou, neologix, haypo

messages: + msg146802
stage: needs patch -> patch review
2010-01-25 14:25:07brian.curtinsetpriority: normal
nosy: + brian.curtin

stage: needs patch
2010-01-25 07:33:58loewissetnosy: + loewis

messages: + msg98272
versions: - Python 2.6, Python 2.5, Python 3.1
2010-01-25 07:27:38Andrew.Grovercreate