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 NORM Transport configuration sockopts #4541

Merged
merged 6 commits into from
Apr 20, 2023
Merged

Conversation

weston-nrl
Copy link
Contributor

NORM transport is currently hard-coded to particular NORM settings and modes. This pull request adds some NORM configuration parameters as socket options to allow for more flexibility in configuring NORM transport for different use cases. Default values and modes remain the same. The pull also makes use of existing socket options ZMQ_RATE (for NORM fixed-rate operation, not used in congestion control modes) and ZMQ_TOS.

src/options.cpp Outdated
@@ -257,6 +257,16 @@ zmq::options_t::options_t () :
can_recv_disconnect_msg (false),
hiccup_msg (),
can_recv_hiccup_msg (false),
#ifdef ZMQ_HAVE_NORM
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for an ifdef here and in the header

include/zmq.h Outdated
/******************************************************************************/
/* NORM transport configuration options */
/******************************************************************************/
#ifdef ZMQ_HAVE_NORM
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't ifdef and move to the DRAFT section, and add to src/zmq_drafts.hpp too

@weston-nrl
Copy link
Contributor Author

Updated as requested to move NORM sockopts over to DRAFT API.

include/zmq.h Outdated
@@ -682,6 +682,23 @@ ZMQ_EXPORT void zmq_threadclose (void *thread_);
#define ZMQ_XSUB_VERBOSE_UNSUBSCRIBE 115
#define ZMQ_TOPICS_COUNT 116

/* DRAFT NORM socket options */
#define ZMQ_NORM_MODE 150
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

number these from 117 and up to follow from the rest

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wasn't sure what to do about that. I thought it might make sense to separate it numerically to leave room for other potential NORM settings, since there are a lot of NORM configuration options that could still be added. I suppose in the end, it doesn't really matter though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@bluca
Copy link
Member

bluca commented Apr 20, 2023

@bluca
Copy link
Member

bluca commented Apr 20, 2023

@bluca bluca merged commit 2d30020 into zeromq:master Apr 20, 2023
19 checks passed
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.

None yet

2 participants