Skip to content
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

Add RDS_CMSG_RXPATH_LATENCY to socket #127383

Open
rruuaanng opened this issue Nov 29, 2024 · 6 comments · May be fixed by #127384
Open

Add RDS_CMSG_RXPATH_LATENCY to socket #127383

rruuaanng opened this issue Nov 29, 2024 · 6 comments · May be fixed by #127384
Labels
type-feature A feature request or enhancement

Comments

@rruuaanng
Copy link
Contributor

rruuaanng commented Nov 29, 2024

Feature or enhancement

Proposal:

This issue suggests adding new RDS constants.

s = /* a RDS type socket */
msg, ancdata, flags, addr = s.recvmsg(2048, socket.CMSG_LEN(5 * 128))
for cmsg_level, cmsg_type, cmsg_data in ancdata:
    if cmsg_type == RDS_CMSG_RXPATH_LATENCY:
        # some processing

It‘s used to query the rx-path(that's receive) latency of RDS. Usually used to optimize RDS communication, such as controlling the number of connections and other strategies

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

@rruuaanng rruuaanng added the type-feature A feature request or enhancement label Nov 29, 2024
@bedevere-app bedevere-app bot linked a pull request Nov 29, 2024 that will close this issue
@erlend-aasland erlend-aasland linked a pull request Nov 29, 2024 that will close this issue
@erlend-aasland erlend-aasland changed the title Add latest RDS constants to socket module Add RDS_CMSG_RXPATH_LATENCY to socket Nov 29, 2024
@erlend-aasland
Copy link
Contributor

This issue suggests adding new RDS constants.

Both the devguide and the issue template instructs you to explain what you are proposing and why you are proposing it. The sentence above does neither of those. Yes, I know this is a trivial issue, but that's no excuse for dismissing our workflow.

Why are you proposing to add RDS_CMSG_RXPATH_LATENCY? What does it do?

@erlend-aasland
Copy link
Contributor

Why are you proposing to add RDS_CMSG_RXPATH_LATENCY? What does it do?

Ask yourself these questions every time you propose a new feature, even if it is just a flag to some syscall wrapper.

@rruuaanng
Copy link
Contributor Author

Thank you for your guidance, I'll do better next time. I modified it and added the description of another PR.
(I think hackers should be familiar with linux, maybe I don't need to explain it too much.)

@erlend-aasland
Copy link
Contributor

I modified it and added the description of another PR.

RDS_CMSG_RXPATH_LATENCY is not an ioctl constant. The description you added does not make sense.

(I think hackers should be familiar with linux, maybe I don't need to explain it too much.)

Read again what I already explained: #127383 (comment)

@erlend-aasland
Copy link
Contributor

Please explain how the constant can be used from a Python perspective using the socket API.

@rruuaanng
Copy link
Contributor Author

I changed the example to a Python-like form, which seems to be more expressive and more suitable for the Python community.
(I admit this is better than my method.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants