-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
socket sendmsg(), recvmsg() methods #50809
Comments
This is the rewritten-from-scratch implementation of the The features that are missing:
These should be very easy to implement. If no one takes up the task of Thanks Kalman Gergely |
the tester application |
This is a duplicate (although updated patch) from bug bpo-1194378. It would |
Hi, I'm afraid there may have been some duplication of effort This patch is based on Heiko's original code, but changes the Comments/flames welcome too. You'll see a few XXX comments in I didn't add a facility to receive into only part of a buffer in |
I'm +1 on adding it but it requires unit tests. Also, recvmsg/sendmsg appear to be *Nix only functions so someone with a windows box would need to test this to see if it blows up. |
I just ran across yet another implementation of sendmsg support for python sockets, whose feature set seems to complement Kalman Gergely's implementation: (Out of curiosity, people have been submitting requests for, and patches with implementations of, this feature for five or more years now and they're never accepted. Is there some opposition to sendmsg support in Python? It's a missing feature that bites me pretty regularly.) |
I've been digging into the patch. Is there a reason sendmsg() wants an iterable of buffers instead of just accepting a str? The list-of-buffers more closely matches the underlying syscall but I'm not sure what the python benefit is, especially when recvmsg() only returns a single value (it only creates 1 iovec under the covers). Python doesn't have "readv" like methods so making sendmsg/recvmsg work like recv/send (straight strings) seems like the way to go. Also, the "y*" format character for packing/unpacking tuples is no longer supported - I'm assuming it used to mean buffers. Does anyone have a good reference for using recvmsg/sendmsg? I read the man pages and googled around but couldn't find anything. I have no experience with using the calls in-the-wild. |
Additional data point: the perl version takes a single scalar (instead of a list of scalars) for use with sendmsg() |
one of the other sprinters just pointed out that Modules/_multiprocessing.c (py3k branch) uses sendmsg/recvmsg internally to pass file descriptors back and forth. The code is very short and readable. |
Thanks for your interest! I'm actually still working on the Yes, you could just use b"".join() with sendmsg() (and get As you might know, gather-write with sendmsg() can give a The patch is for 3.x, BTW - "y*" is valid there (and does take a As for a good reference, I haven't personally seen one. There's The question of whether to use CMSG_NXTHDR() to step to the next @wim: Yes, the rfc3542 module from that package looks as if it would be The objects in that package seem a bit overcomplicated, though, Maybe the problem of testing patches well has been putting people |
OK, here's a new version as a work in progress. A lot of the new There are a couple of changes to the interface (hopefully the Since the ancillary buffer size argument is now more likely to I've also used socket.error instead of ValueError when rejecting The code is now much more paranoid about checking the results of I'd still like to add tests for receiving some of the RFC 3542 |
I just found that the IPv6 tests don't get skipped when IPv6 is |
Here is a new version of the patch; I've added some tests which As well as Linux, I've tested it on an old (unsupported) FreeBSD The main issue was that when truncating ancillary data, FreeBSD The warning could perhaps be disabled for systems like this, After some investigation, I've stuck with using CMSG_FIRSTHDR() The KAME IPv6 userspace utilities at [1] include several programs The alternative would be to add CMSG_SPACE(size) to the pointer |
Would you like to upload your patch to http://codereview.appspot.com/? It would make reviewing easier. |
OK. I don't like creating/using a Google account, but here it is: |
New version with minor changes. Will also upload at http://codereview.appspot.com/1487041/show |
Optional patch to replace SocketTCPTest, etc. with the classes from the sendmsg patch. |
Updated the patch to use the new BEGIN/END_SELECT_LOOP macros. Also fixed a bug I spotted while doing this whereby when a Will also upload at http://codereview.appspot.com/1487041/show |
What needs to happen to get recvmsg() supported in Python? recvmsg() is required to get get transparent UDP proxies working under Linux using tproxy, the code needs to run recvmsg() to be able to find out what the original destination address was for the the packet. Thanks Brian May |
On 05/22/11 03:14, Brian May wrote:
Hello Brian! It's been a while I had a look at that code. As far as I remember though Gergely Kalman |
On Mon 23 May 2011, Gergely Kálmán wrote:
Erm, have you seen the separately-implemented patch I posted at |
Hello, Are there any problems applying the v5 version of the patch to 3.3? Also is there any remote chance for a backport to 2.7? Thanks |
No, indeed this is a lot better. |
On Tue 24 May 2011, Brian May wrote:
Well, it still works for me, apart from a trivial patch conflict: Antoine did previously question the necessity of all the various Apart from that, I don't know. Perhaps a review by someone
I'd be happy to do one, but I'm pretty sure python.org's 2.x line |
Have tested my code with this patch, the recvmsg(...) call seems to work fine. Also had a half-hearted attempt at porting to Python 2.7, but didn't get past compiling, the code requires BEGIN_SELECT_LOOP and END_SELECT_LOOP macros that aren't defined in Python 2.7 - I tried copying the definitions from Python 3.3, but that didn't work either. Not sure if it is worth the effort if Python 2.7 is closed to new features. Brian May |
Well, I guess that the only reason is that no committer is motivated
Sounds like a job for raw sockets, no? (well, you need CAP_NET_RAW) In short, I think that you just need to find a core developer |
Modules/_multiprocessing already has code using sendmsg/recvmsg, |
On Tue 31 May 2011, Brian May wrote:
Well, if it's any use to anyone, here is a backport to 2.x (the Also attaching a new 3.x patch which fixes some deprecated markup |
Attached patch (ssl_fixes_v1) makes the presence of the sendmsg/recvmsg methods in the ssl module conditional on their being present in the underlying socket module. However, in doing this, I noticed that these methods will, at best, work during the time between connection and the socket going secure and were not added to the list of methods that the SSL is documented as exposing. Perhaps we should just ditch them entirely? |
+1 |
Well, apparently the SSLSocket object is meant to be usable in clear |
That's the part I'm questioning though. I'm not clear why you'd ever do that instead of doing everything on the original socket before invoking ssl.wrap_socket. What I missed on the original patch before committing it (mea culpa) is that the SSL part is neither documented nor tested properly (the tests only check that it is disallowed on a secured SSLSocket, not that it works on a connected-but-not-secured-yet SSLSocket object). The absence of proper tests and documentation is the main reason I'm tempted to just revert those parts of the patch that touch the ssl module and its tests. |
Bill, do you know?
Then perhaps raise NotImplementedError, so that people know it's deliberate and not an oversight. |
New changeset fd10d042b41d by Nick Coghlan in branch 'default': |
As you can see, I just pushed a change that removed the new methods from SSLSocket objects. If anyone wants to step up with a valid use case (not already covered by wrap_socket), preferably with a patch to add them back that includes proper tests and documentation changes, please open a new feature request and attach the new patch to that issue. |
Regarding the 'missing methods' aspect, the SSL docs are already pretty clear that SSLSocket objects don't expose the full socket API: http://docs.python.org/dev/library/ssl#ssl-sockets Those docs are actually what really got me started down the path of wondering whether or not these new methods even belong on SSLSocket in the first place. |
And the Windows buildbots are now happy (at least with respect to this change, anyway - they're still griping about a few other issues). I don't know if it's feasible to support these new APIs at the socket module level on Windows, but any patches along those lines should also be placed in a new issue rather than being added to this one. |
The OS X buildbots show some failures: |
Here's a patch skipping testFDPassSeparate and |
As noted by Antoine, the OS X 10.5 buildbots are also failing. |
On Tue 23 Aug 2011, Nick Coghlan wrote:
Hi, sorry about the trouble caused by the broken tests, but |
It seems to fail consistently on every OS X version. |
Putting this back to open until we decide what to do about the OS X test failures. It sounds like it could really do with some more poking and prodding to figure out whether or not it poses a potential security risk or is just a relatively cosmetic problem with the API, so I'm reluctant to just skip the failing tests at this point. |
I'll take a look at this next week, when I'm more on-line again. Bill 2011/8/25 Charles-François Natali <report@bugs.python.org>:
|
If Bill gets a chance to investigate this before the weekend, great, otherwise my plan to stop making noise in the buildbot results will be to:
That way, if these failures are due to underlying OS bugs or limitations (as they appear to be), we'll get a clear indication in the buildbots when Apple have fixed the relevant problems. |
Closing the feature request as complete. The remaining Mac OS X buildbot issues now have their own tracker item: bpo-12958 |
I'm guessing these things are due to interaction with some Apple Bill On Wed, Sep 7, 2011 at 4:01 PM, Nick Coghlan <report@bugs.python.org> wrote:
|
The feature patch for sendmsg/recvmsg support came with a swathe of new tests, and the failures are in those new tests rather than anything breaking in the old ones. As Charles-François noted though, it doesn't look like the feature implementation itself is doing anything wrong, just that there are limits to what Mac OS X allows us to do with it (hence why I closed this feature request and opened issue bpo-12958 to cover the task of updating the test suite to accurately reflect that situation). |
New changeset e64bec91ac91 by Richard Oudkerk in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: