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
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Hi, thank you for the pull request.
I've left a suggestion about the changes.
Thank you :)
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Please add NEWS entry: https://devguide.python.org/committing/?highlight=blurb#what-s-new-and-news-entries
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
| .. versionadded:: 3.8 | ||
|
|
||
| - :const:`IPPROTO_UDPLITE` is a variant on UDP which allows you to specify | ||
| what portion of a packet to cover with the checksum. It adds the methods |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Please rephrase.
IPPROTO_UDPLITE doesn't add the methods but allows to use UDPLITE_SEND_CSCOV / UDPLITE_RECV_CSCOV in setsockopt
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Does the new version make this better in your eyes?
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @asvetlov: please review the changes made to this pull request. |
|
Thanks! |
At the moment you can definitely use UDPLITE sockets on Linux systems, but it would be good if this support were formalized such that you can detect support at runtime easily.
At the moment, to make and use a UDPLITE socket requires something like the following code:
```
>>> import socket
>>> a = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 136)
>>> b = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 136)
>>> a.bind(('localhost', 44444))
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.setsockopt(136, 10, 16)
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.setsockopt(136, 10, 32)
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.setsockopt(136, 10, 64)
>>> b.sendto(b'test'*256, ('localhost', 44444))
```
If you look at this through Wireshark, you can see that the packets are different in that the checksums and checksum coverages change.
With the pull request that I am submitting momentarily, you could do the following code instead:
```
>>> import socket
>>> a = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, socket.IPPROTO_UDPLITE)
>>> b = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, socket.IPPROTO_UDPLITE)
>>> a.bind(('localhost', 44444))
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.set_send_checksum_coverage(16)
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.set_send_checksum_coverage(32)
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.set_send_checksum_coverage(64)
>>> b.sendto(b'test'*256, ('localhost', 44444))
```
One can also detect support for UDPLITE just by checking
```
>>> hasattr(socket, 'IPPROTO_UDPLITE')
```
https://bugs.python.org/issue37345
At the moment you can definitely use UDPLITE sockets on Linux systems, but it would be good if this support were formalized such that you can detect support at runtime easily.
At the moment, to make and use a UDPLITE socket requires something like the following code:
```
>>> import socket
>>> a = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 136)
>>> b = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 136)
>>> a.bind(('localhost', 44444))
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.setsockopt(136, 10, 16)
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.setsockopt(136, 10, 32)
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.setsockopt(136, 10, 64)
>>> b.sendto(b'test'*256, ('localhost', 44444))
```
If you look at this through Wireshark, you can see that the packets are different in that the checksums and checksum coverages change.
With the pull request that I am submitting momentarily, you could do the following code instead:
```
>>> import socket
>>> a = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, socket.IPPROTO_UDPLITE)
>>> b = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, socket.IPPROTO_UDPLITE)
>>> a.bind(('localhost', 44444))
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.set_send_checksum_coverage(16)
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.set_send_checksum_coverage(32)
>>> b.sendto(b'test'*256, ('localhost', 44444))
>>> b.set_send_checksum_coverage(64)
>>> b.sendto(b'test'*256, ('localhost', 44444))
```
One can also detect support for UDPLITE just by checking
```
>>> hasattr(socket, 'IPPROTO_UDPLITE')
```
https://bugs.python.org/issue37345
At the moment you can definitely use UDPLITE sockets on Linux systems, but it would be good if this support were formalized such that you can detect support at runtime easily.
At the moment, to make and use a UDPLITE socket requires something like the following code:
If you look at this through Wireshark, you can see that the packets are different in that the checksums and checksum coverages change.
With the pull request that I am submitting momentarily, you could do the following code instead:
One can also detect support for UDPLITE just by checking
https://bugs.python.org/issue37345