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

ZeroMQ read handler #566

Closed
wants to merge 1 commit into from
Closed

Conversation

eqvinox
Copy link
Contributor

@eqvinox eqvinox commented May 17, 2017

this is "the other way around" compared to PR #522.
I believe this is far easier to integrate and have less conflicts / impact on other changes in our event loop.

@bingen can you check if this works for your code?

This uses zmq_getsockopt(ZMQ_FD) to create a libfrr read event, which
then wraps zmq_poll and calls an user-specified ZeroMQ read handler.
It's wrapped in a separate library in order to make ZeroMQ support an
installation-time option instead of build-time.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
@eqvinox eqvinox requested a review from bingen May 17, 2017 18:37
@NetDEF-CI
Copy link
Collaborator

Continous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-740/

This is a comment from an EXPERIMENTAL automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

@bingen
Copy link
Member

bingen commented May 18, 2017

I really like this approach and I agree is way better isolated so easier to integrate. The reason we modified thread.c was to replace standard select by zmq_poll, as we had some problems with selects. Sometimes select was detecting some "noise" in the fd that we had to filter. I understand that here you do a zmq_poll after the select (or nowadays a poll) and that should filter the noise too. I'll try it to confirm that it works.
The only problem I see is that we are using ROUTER-DEALER pattern, so the function you use to do zmq_recv wouldn't work for us, as we need to do something like this:

	ret = zmq_msg_init(&zdelim);
	ret = zmq_msg_init(&zmsg);

	ret = zmq_msg_recv(&zdelim, sock, flags);
	ret = zmq_msg_recv(&zmsg, sock,   flags);

If anybody is interested, have a look at this.
I guess one easy solution would be to be able to pass the function used as a callback for funcname_thread_add_read_write as a parameter for funcname_frrzmq_thread_read_msg. If this parameter comes NULL we may fall back to the standard one frrzmq_read_msg. This way we could pass a function of ours with that multiple receive explained above.

@eqvinox
Copy link
Contributor Author

eqvinox commented May 18, 2017

hmm... isn't that functionally the same as 2 receive calls? first an empty one, then the data?

@eqvinox
Copy link
Contributor Author

eqvinox commented May 18, 2017

discussed on slack => will change to 2 handlers, one doing no message receiving, the other receiving all parts of the message (however many there may be)

@eqvinox
Copy link
Contributor Author

eqvinox commented May 23, 2017

will put new version up

@eqvinox eqvinox closed this May 23, 2017
@pguibert6WIND
Copy link
Member

this looks interesting.
so that REQ/REP and ROUTER/DEALER patterns will be supported.

@eqvinox eqvinox deleted the zeromq branch August 21, 2017 13:40
eqvinox added a commit to opensourcerouting/frr that referenced this pull request Aug 28, 2017
This uses zmq_getsockopt(ZMQ_FD) to create a libfrr read event, which
then wraps zmq_poll and calls an user-specified ZeroMQ read handler.
It's wrapped in a separate library in order to make ZeroMQ support an
installation-time option instead of build-time.

Extended to support per-message and per-fragment callbacks as discussed
with Bingen in PR FRRouting#566.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
qlyoung pushed a commit to qlyoung/frr that referenced this pull request Nov 6, 2017
This uses zmq_getsockopt(ZMQ_FD) to create a libfrr read event, which
then wraps zmq_poll and calls an user-specified ZeroMQ read handler.
It's wrapped in a separate library in order to make ZeroMQ support an
installation-time option instead of build-time.

Extended to support per-message and per-fragment callbacks as discussed
with Bingen in PR FRRouting#566.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants