From d64f12553eb6ceb67ed6f6a5b2ceb6c061d375ba Mon Sep 17 00:00:00 2001 From: Garrett D'Amore Date: Tue, 8 Aug 2017 21:19:09 -0700 Subject: fixes #44 open protocol by "name" (symbol) instead number fixes #38 Make protocols "pluggable", or at least optional This is a breaking change, as we've done away with the central registered list of protocols, and instead demand the user call nng_xxx_open() where xxx is a protocol name. (We did keep a table around in the compat framework though.) There is a nice way for protocols to plug in via an nni_proto_open(), where they can use a generic constructor that they use to build a protocol specific constructor (passing their ops vector in.) --- src/core/device.c | 4 +- src/core/protocol.c | 83 ++++---------------------------- src/core/protocol.h | 36 ++++++++++---- src/core/socket.c | 134 ++++++++++++++++------------------------------------ src/core/socket.h | 7 +-- 5 files changed, 82 insertions(+), 182 deletions(-) (limited to 'src/core') diff --git a/src/core/device.c b/src/core/device.c index cd0c8dfc..81aafc3d 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -80,8 +80,8 @@ nni_device(nni_sock *sock1, nni_sock *sock2) rv = NNG_EINVAL; goto out; } - if ((sock1->s_peer != sock2->s_protocol) || - (sock2->s_peer != sock1->s_protocol)) { + if ((sock1->s_peer_id.p_id != sock2->s_self_id.p_id) || + (sock2->s_peer_id.p_id != sock1->s_self_id.p_id)) { rv = NNG_EINVAL; goto out; } diff --git a/src/core/protocol.c b/src/core/protocol.c index a60454de..7faf9068 100644 --- a/src/core/protocol.c +++ b/src/core/protocol.c @@ -1,5 +1,6 @@ // // Copyright 2017 Garrett D'Amore +// Copyright 2017 Capitar IT Group BV // // This software is supplied under the terms of the MIT License, a // copy of which should be located in the distribution where this @@ -13,82 +14,14 @@ // Protocol related stuff - generically. -// The list of protocols is hardwired. This is reasonably unlikely to -// change, as adding new protocols is not something intended to be done -// outside of the core. -extern nni_proto nni_bus_proto; -extern nni_proto nni_pair_proto; -extern nni_proto nni_rep_proto; -extern nni_proto nni_req_proto; -extern nni_proto nni_pub_proto; -extern nni_proto nni_sub_proto; -extern nni_proto nni_push_proto; -extern nni_proto nni_pull_proto; -extern nni_proto nni_surveyor_proto; -extern nni_proto nni_respondent_proto; - -static nni_proto *protocols[] = { - // clang-format off - &nni_bus_proto, - &nni_pair_proto, - &nni_rep_proto, - &nni_req_proto, - &nni_pub_proto, - &nni_sub_proto, - &nni_push_proto, - &nni_pull_proto, - &nni_surveyor_proto, - &nni_respondent_proto, - NULL - // clang-format on -}; - -nni_proto * -nni_proto_find(uint16_t num) -{ - int i; - nni_proto *p; - - for (i = 0; (p = protocols[i]) != NULL; i++) { - if (p->proto_self == num) { - break; - } - } - return (p); -} - -const char * -nni_proto_name(uint16_t num) -{ - nni_proto *p; - - if ((p = nni_proto_find(num)) == NULL) { - return (NULL); - } - return (p->proto_name); -} - -uint16_t -nni_proto_number(const char *name) -{ - nni_proto *p; - int i; - - for (i = 0; (p = protocols[i]) != NULL; i++) { - if (strcmp(p->proto_name, name) == 0) { - return (p->proto_self); - } - } - return (NNG_PROTO_NONE); -} - -uint16_t -nni_proto_peer(uint16_t num) +int +nni_proto_open(nng_socket *sockidp, const nni_proto *proto) { - nni_proto *p; + int rv; + nni_sock *sock; - if ((p = nni_proto_find(num)) == NULL) { - return (NNG_PROTO_NONE); + if ((rv = nni_sock_open(&sock, proto)) == 0) { + *sockidp = nni_sock_id(sock); // Keep socket held open. } - return (p->proto_peer); + return (rv); } diff --git a/src/core/protocol.h b/src/core/protocol.h index 3a133469..14954b49 100644 --- a/src/core/protocol.h +++ b/src/core/protocol.h @@ -1,5 +1,6 @@ // // Copyright 2017 Garrett D'Amore +// Copyright 2017 Capitar IT Group BV // // This software is supplied under the terms of the MIT License, a // copy of which should be located in the distribution where this @@ -47,7 +48,7 @@ struct nni_proto_pipe_ops { }; struct nni_proto_sock_ops { - // sock_initf creates the protocol instance, which will be stored on + // sock_init creates the protocol instance, which will be stored on // the socket. This is run without the sock lock held, and allocates // storage or other resources for the socket. int (*sock_init)(void **, nni_sock *); @@ -82,15 +83,31 @@ struct nni_proto_sock_ops { nni_msg *(*sock_sfilter)(void *, nni_msg *); }; +typedef struct nni_proto_id { + uint16_t p_id; + const char *p_name; +} nni_proto_id; + struct nni_proto { - uint16_t proto_self; // our 16-bit D - uint16_t proto_peer; // who we peer with (ID) - const char * proto_name; // Our name + uint32_t proto_version; // Ops vector version + nni_proto_id proto_self; // Our identity + nni_proto_id proto_peer; // Peer identity uint32_t proto_flags; // Protocol flags const nni_proto_sock_ops *proto_sock_ops; // Per-socket opeations const nni_proto_pipe_ops *proto_pipe_ops; // Per-pipe operations. }; +// We quite intentionally use a signature where the upper word is nonzero, +// which ensures that if we get garbage we will reject it. This is more +// likely to mismatch than all zero bytes would. The actual version is +// stored in the lower word; this is not semver -- the numbers are just +// increasing - we doubt it will increase more than a handful of times +// during the life of the project. If we add a new version, please keep +// the old version around -- it may be possible to automatically convert +// older versions in the future. +#define NNI_PROTOCOL_V0 0x50520000 // "pr\0\0" +#define NNI_PROTOCOL_VERSION NNI_PROTOCOL_V0 + // These flags determine which operations make sense. We use them so that // we can reject attempts to create notification fds for operations that make // no sense. @@ -98,11 +115,10 @@ struct nni_proto { #define NNI_PROTO_FLAG_SND 2 // Protocol can send #define NNI_PROTO_FLAG_SNDRCV 3 // Protocol can both send & recv -// These functions are not used by protocols, but rather by the socket -// core implementation. The lookups can be used by transports as well. -extern nni_proto * nni_proto_find(uint16_t); -extern const char *nni_proto_name(uint16_t); -extern uint16_t nni_proto_number(const char *); -extern uint16_t nni_proto_peer(uint16_t); +// nni_proto_open is called by the protocol to create a socket instance +// with its ops vector. The intent is that applications will only see +// the single protocol-specific constructure, like nng_pair_v0_open(), +// which should just be a thin wrapper around this. +extern int nni_proto_open(nng_socket *, const nni_proto *); #endif // CORE_PROTOCOL_H diff --git a/src/core/socket.c b/src/core/socket.c index f2dbb573..f01379a8 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -83,7 +83,7 @@ nni_sock_pipe_start(nni_pipe *pipe) // We're closing, bail out. return (NNG_ECLOSED); } - if (nni_pipe_peer(pipe) != sock->s_peer) { + if (nni_pipe_peer(pipe) != sock->s_peer_id.p_id) { // Peer protocol mismatch. return (NNG_EPROTO); } @@ -175,7 +175,7 @@ nni_sock_pipe_ready(nni_sock *sock, nni_pipe *pipe) nni_mtx_unlock(&sock->s_mx); return (NNG_ECLOSED); } - if (nni_pipe_peer(pipe) != sock->s_peer) { + if (nni_pipe_peer(pipe) != sock->s_peer_id.p_id) { nni_mtx_unlock(&sock->s_mx); return (NNG_EPROTO); } @@ -305,47 +305,6 @@ nni_sock_unnotify(nni_sock *sock, nni_notify *notify) NNI_FREE_STRUCT(notify); } -static nni_msg * -nni_sock_nullfilter(void *arg, nni_msg *mp) -{ - NNI_ARG_UNUSED(arg); - return (mp); -} - -static int -nni_sock_nullgetopt(void *arg, int num, void *data, size_t *szp) -{ - NNI_ARG_UNUSED(arg); - NNI_ARG_UNUSED(num); - NNI_ARG_UNUSED(data); - NNI_ARG_UNUSED(szp); - return (NNG_ENOTSUP); -} - -static int -nni_sock_nullsetopt(void *arg, int num, const void *data, size_t sz) -{ - NNI_ARG_UNUSED(arg); - NNI_ARG_UNUSED(num); - NNI_ARG_UNUSED(data); - NNI_ARG_UNUSED(sz); - return (NNG_ENOTSUP); -} - -static void -nni_sock_nullop(void *arg) -{ - NNI_ARG_UNUSED(arg); -} - -static int -nni_sock_nullstartpipe(void *arg) -{ - NNI_ARG_UNUSED(arg); - - return (0); -} - static void * nni_sock_ctor(uint32_t id) { @@ -456,23 +415,23 @@ nni_sock_sys_fini(void) nni_mtx_fini(&nni_sock_lk); } -// nn_sock_open creates the underlying socket. int -nni_sock_open(nni_sock **sockp, uint16_t pnum) +nni_sock_open(nni_sock **sockp, const nni_proto *proto) { nni_sock * sock; - nni_proto * proto; int rv; nni_proto_sock_ops *sops; nni_proto_pipe_ops *pops; uint32_t sockid; + if (proto->proto_version != NNI_PROTOCOL_VERSION) { + // unsupported protocol version + return (NNG_ENOTSUP); + } + if ((rv = nni_init()) != 0) { return (rv); } - if ((proto = nni_proto_find(pnum)) == NULL) { - return (NNG_ENOTSUP); - } rv = nni_objhash_alloc(nni_socks, &sockid, (void **) &sock); if (rv != 0) { @@ -480,40 +439,20 @@ nni_sock_open(nni_sock **sockp, uint16_t pnum) } // We make a copy of the protocol operations. - sock->s_protocol = proto->proto_self; - sock->s_peer = proto->proto_peer; + sock->s_self_id = proto->proto_self; + sock->s_peer_id = proto->proto_peer; sock->s_flags = proto->proto_flags; sock->s_sock_ops = *proto->proto_sock_ops; + sock->s_pipe_ops = *proto->proto_pipe_ops; sops = &sock->s_sock_ops; - if (sops->sock_sfilter == NULL) { - sops->sock_sfilter = nni_sock_nullfilter; - } - if (sops->sock_rfilter == NULL) { - sops->sock_rfilter = nni_sock_nullfilter; - } - if (sops->sock_getopt == NULL) { - sops->sock_getopt = nni_sock_nullgetopt; - } - if (sops->sock_setopt == NULL) { - sops->sock_setopt = nni_sock_nullsetopt; - } - if (sops->sock_close == NULL) { - sops->sock_close = nni_sock_nullop; - } - if (sops->sock_open == NULL) { - sops->sock_open = nni_sock_nullop; - } - sock->s_pipe_ops = *proto->proto_pipe_ops; - pops = &sock->s_pipe_ops; - if (pops->pipe_start == NULL) { - pops->pipe_start = nni_sock_nullstartpipe; - } - if (pops->pipe_stop == NULL) { - pops->pipe_stop = nni_sock_nullop; - } + NNI_ASSERT(sock->s_sock_ops.sock_open != NULL); + NNI_ASSERT(sock->s_sock_ops.sock_close != NULL); + + NNI_ASSERT(sock->s_pipe_ops.pipe_start != NULL); + NNI_ASSERT(sock->s_pipe_ops.pipe_stop != NULL); - if ((rv = sops->sock_init(&sock->s_data, sock)) != 0) { + if ((rv = sock->s_sock_ops.sock_init(&sock->s_data, sock)) != 0) { nni_objhash_unref(nni_socks, sockid); return (rv); } @@ -730,7 +669,9 @@ nni_sock_sendmsg(nni_sock *sock, nni_msg *msg, nni_time expire) } besteffort = sock->s_besteffort; - msg = sock->s_sock_ops.sock_sfilter(sock->s_data, msg); + if (sock->s_sock_ops.sock_sfilter != NULL) { + msg = sock->s_sock_ops.sock_sfilter(sock->s_data, msg); + } nni_mtx_unlock(&sock->s_mx); if (msg == NULL) { @@ -780,9 +721,11 @@ nni_sock_recvmsg(nni_sock *sock, nni_msg **msgp, nni_time expire) if (rv != 0) { return (rv); } - nni_mtx_lock(&sock->s_mx); - msg = sock->s_sock_ops.sock_rfilter(sock->s_data, msg); - nni_mtx_unlock(&sock->s_mx); + if (sock->s_sock_ops.sock_rfilter != NULL) { + nni_mtx_lock(&sock->s_mx); + msg = sock->s_sock_ops.sock_rfilter(sock->s_data, msg); + nni_mtx_unlock(&sock->s_mx); + } if (msg != NULL) { break; } @@ -797,13 +740,13 @@ nni_sock_recvmsg(nni_sock *sock, nni_msg **msgp, nni_time expire) uint16_t nni_sock_proto(nni_sock *sock) { - return (sock->s_protocol); + return (sock->s_self_id.p_id); } uint16_t nni_sock_peer(nni_sock *sock) { - return (sock->s_peer); + return (sock->s_peer_id.p_id); } nni_duration @@ -924,10 +867,13 @@ nni_sock_setopt(nni_sock *sock, int opt, const void *val, size_t size) nni_mtx_unlock(&sock->s_mx); return (NNG_ECLOSED); } - rv = sock->s_sock_ops.sock_setopt(sock->s_data, opt, val, size); - if (rv != NNG_ENOTSUP) { - nni_mtx_unlock(&sock->s_mx); - return (rv); + if (sock->s_sock_ops.sock_setopt != NULL) { + rv = + sock->s_sock_ops.sock_setopt(sock->s_data, opt, val, size); + if (rv != NNG_ENOTSUP) { + nni_mtx_unlock(&sock->s_mx); + return (rv); + } } switch (opt) { case NNG_OPT_LINGER: @@ -970,11 +916,15 @@ nni_sock_getopt(nni_sock *sock, int opt, void *val, size_t *sizep) nni_mtx_unlock(&sock->s_mx); return (NNG_ECLOSED); } - rv = sock->s_sock_ops.sock_getopt(sock->s_data, opt, val, sizep); - if (rv != NNG_ENOTSUP) { - nni_mtx_unlock(&sock->s_mx); - return (rv); + if (sock->s_sock_ops.sock_getopt != NULL) { + rv = sock->s_sock_ops.sock_getopt( + sock->s_data, opt, val, sizep); + if (rv != NNG_ENOTSUP) { + nni_mtx_unlock(&sock->s_mx); + return (rv); + } } + switch (opt) { case NNG_OPT_LINGER: rv = nni_getopt_duration(&sock->s_linger, val, sizep); diff --git a/src/core/socket.h b/src/core/socket.h index 54c1b4da..6824b04b 100644 --- a/src/core/socket.h +++ b/src/core/socket.h @@ -25,8 +25,9 @@ struct nni_socket { nni_list_node s_node; - uint16_t s_protocol; - uint16_t s_peer; + nni_proto_id s_self_id; + nni_proto_id s_peer_id; + uint32_t s_flags; nni_proto_pipe_ops s_pipe_ops; @@ -67,7 +68,7 @@ extern void nni_sock_sys_fini(void); extern int nni_sock_find(nni_sock **, uint32_t); extern void nni_sock_rele(nni_sock *); -extern int nni_sock_open(nni_sock **, uint16_t); +extern int nni_sock_open(nni_sock **, const nni_proto *); extern void nni_sock_close(nni_sock *); extern void nni_sock_closeall(void); extern int nni_sock_shutdown(nni_sock *); -- cgit v1.2.3-70-g09d2