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 listen queue callback function #291

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gresolio
Copy link

@gresolio gresolio commented Nov 18, 2024

Hi NetX support team,

In order to handle TCP connections on the same port, the host application must know when a connect request is added to the listen queue. Currently, the nx_tcp_listen_queue_current must be manually polled, since NetX doesn't provide any callbacks to inform about queued connections.

This pull request implements a callback function, which allows to notify the host application of a new connect request in the listen queue. NX_DISABLE_EXTENDED_NOTIFY_SUPPORT symbol has been used to control the callback, similar to nx_tcp_socket_syn_received_notify and other optional callbacks (disabled by default, the NX_ENABLE_EXTENDED_NOTIFY_SUPPORT must be defined in the config to enable it).

Please review. Hopefully the feature is useful enough to be merged into the main codebase.

@fdesbiens
Copy link

Hi @gresolio.

Thank you for this contribution. It makes sense to me. Since it is small, I will see if someone else could review it this week.

Would you be so kind as to create an Eclipse account referencing the same email used to sign your commits, please? Once this is done, you will be able to sign the Eclipse Contributor Agreement (ECA) electronically. We cannot merge your contribution without a signed ECA on file.

You can use this link to create the account and sign the ECA:
https://accounts.eclipse.org/user/login?destination=user/eca

@gresolio
Copy link
Author

Hi @fdesbiens,

Thanks for the quick response.
Good, I will create an Eclipse account to sign the agreement. But I have a question regarding the email:

At the moment, my commit email address is the noreply email address configured in GitHub settings, with the following format:
[email protected]

Is it possible to use such a noreply email address to sign the agreement?
(I suspect no, it will not match the email used to register an Eclipse account).

If no, do I need to create a dedicated public email and update the pull request?
Or is there perhaps some functionality to link the GitHub account to the Eclipse account?

p.s. I ask, because I have never signed Eclipse Contributor Agreement (ECA), and don't know the details.
ECA FAQ provides similar info: "Be sure to use the same email address when you register for the account that you intend to use on Git commit records."

@fdesbiens
Copy link

@gresolio This is the first time I have heard of a case like yours. Please email [email protected] and ask them how to ensure your pull requests pass the automated ECA check we enforce on our repositories. They should have a solution for you.

@gresolio gresolio force-pushed the listen-queue-notify branch from c636982 to 6ed72a9 Compare November 22, 2024 15:59
@gresolio
Copy link
Author

@fdesbiens
I have successfully created an Eclipse account and signed the agreement.
Pull request has also been updated, so the commit email matches ECA agreement.
The "eclipsefdn/eca" check is now successful. Please review.

@fdesbiens
Copy link

@fdesbiens I have successfully created an Eclipse account and signed the agreement. Pull request has also been updated, so the commit email matches ECA agreement. The "eclipsefdn/eca" check is now successful. Please review.

Thank you, @gresolio.

@yuxinzhou5 Can you review this contribution, please?

@gresolio
Copy link
Author

gresolio commented Dec 6, 2024

@yuxinzhou5 One of the tests has failed. Is there anything I can help with?
Or is this a general issue with the test, that is not caused by the pull request?

@fdesbiens
Copy link

I looked at the logs and it seems we are hitting a timeout on a TLS test.

The following tests FAILED:
6 - default_build_coverage::override_tls_1_1_openssl_echo_server_nx_secure_echo_client_test (Timeout)
Errors while running CTest
Error: Process completed with exit code 8.

Given the nature of the changes in this pull request, I think we could merge anyway and figure out what is happening with the test on our own. What do you think, @yuxinzhou5?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants