Skip to content

Commit

Permalink
Refactor option handling APIs.
Browse files Browse the repository at this point in the history 
This makes the APIs use string keys, and largely eliminates the use of
integer option IDs altogether.  The underlying registration for options
is also now a bit richer, letting protcols and transports declare the
actual options they use, rather than calling down into each entry point
carte blanche and relying on ENOTSUP.

This code may not be as fast as the integers was, but it is more intuitive,
easier to extend, and is not on any hot code paths.  (If you're diddling
options on a hot code path you're doing something wrong.)
  • Loading branch information
@gdamore
gdamore committed Sep 27, 2017
1 parent 86a96e5 commit 64db0f0
Show file tree
Hide file tree
Showing 45 changed files with 1,543 additions and 1,328 deletions.
8 changes: 4 additions & 4 deletions perf/perf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,9 +360,9 @@ throughput_server(const char *addr, int msgsize, int count)
if ((rv = nng_pair_open(&s)) != 0) {
die("nng_socket: %s", nng_strerror(rv));
}
rv = nng_setopt_int(s, nng_optid_recvbuf, 128);
rv = nng_setopt_int(s, NNG_OPT_RECVBUF, 128);
if (rv != 0) {
die("nng_setopt(nng_optid_recvbuf): %s", nng_strerror(rv));
die("nng_setopt(nng_opt_recvbuf): %s", nng_strerror(rv));
}

// XXX: set no delay
Expand Down Expand Up @@ -420,9 +420,9 @@ throughput_client(const char *addr, int msgsize, int count)
// XXX: set no delay
// XXX: other options (TLS in the future?, Linger?)

rv = nng_setopt_int(s, nng_optid_sendbuf, 128);
rv = nng_setopt_int(s, NNG_OPT_SENDBUF, 128);
if (rv != 0) {
die("nng_setopt(nng_optid_sendbuf): %s", nng_strerror(rv));
die("nng_setopt(nng_opt_sendbuf): %s", nng_strerror(rv));
}

if ((rv = nng_dial(s, addr, NULL, 0)) != 0) {
Expand Down
23 changes: 13 additions & 10 deletions src/core/defs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,19 @@ typedef struct nng_event nni_event;
typedef struct nng_notify nni_notify;

// These are our own names.
typedef struct nni_socket nni_sock;
typedef struct nni_ep nni_ep;
typedef struct nni_pipe nni_pipe;
typedef struct nni_tran nni_tran;
typedef struct nni_tran_ep nni_tran_ep;
typedef struct nni_tran_pipe nni_tran_pipe;

typedef struct nni_proto_sock_ops nni_proto_sock_ops;
typedef struct nni_proto_pipe_ops nni_proto_pipe_ops;
typedef struct nni_proto nni_proto;
typedef struct nni_socket nni_sock;
typedef struct nni_ep nni_ep;
typedef struct nni_pipe nni_pipe;
typedef struct nni_tran nni_tran;
typedef struct nni_tran_ep nni_tran_ep;
typedef struct nni_tran_ep_option nni_tran_ep_option;
typedef struct nni_tran_pipe nni_tran_pipe;
typedef struct nni_tran_pipe_option nni_tran_pipe_option;

typedef struct nni_proto_sock_ops nni_proto_sock_ops;
typedef struct nni_proto_pipe_ops nni_proto_pipe_ops;
typedef struct nni_proto_sock_option nni_proto_sock_option;
typedef struct nni_proto nni_proto;

typedef struct nni_plat_mtx nni_mtx;
typedef struct nni_plat_cv nni_cv;
Expand Down
8 changes: 4 additions & 4 deletions src/core/device.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ nni_device(nni_sock *sock1, nni_sock *sock2)

// No timeouts.
sz = sizeof(never);
if ((nni_sock_setopt(sock1, nng_optid_recvtimeo, &never, sz) != 0) ||
(nni_sock_setopt(sock2, nng_optid_recvtimeo, &never, sz) != 0) ||
(nni_sock_setopt(sock1, nng_optid_sendtimeo, &never, sz) != 0) ||
(nni_sock_setopt(sock2, nng_optid_sendtimeo, &never, sz) != 0)) {
if ((nni_sock_setopt(sock1, NNG_OPT_RECVTIMEO, &never, sz) != 0) ||
(nni_sock_setopt(sock2, NNG_OPT_RECVTIMEO, &never, sz) != 0) ||
(nni_sock_setopt(sock1, NNG_OPT_SENDTIMEO, &never, sz) != 0) ||
(nni_sock_setopt(sock2, NNG_OPT_SENDTIMEO, &never, sz) != 0)) {
// This should never happen.
rv = NNG_EINVAL;
goto out;
Expand Down
64 changes: 44 additions & 20 deletions src/core/endpt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ nni_ep_acc_cb(void *arg)
break;
case NNG_ECLOSED:
case NNG_ECANCELED:
// Canceled or closed, no furhter action.
// Canceled or closed, no further action.
break;
case NNG_ECONNABORTED:
case NNG_ECONNRESET:
Expand Down Expand Up @@ -587,38 +587,62 @@ nni_ep_pipe_remove(nni_ep *ep, nni_pipe *pipe)
}

int
nni_ep_setopt(nni_ep *ep, int opt, const void *val, size_t sz, int check)
nni_ep_setopt(nni_ep *ep, const char *name, const void *val, size_t sz)
{
int rv;
nni_tran_ep_option *eo;

if (ep->ep_ops.ep_setopt == NULL) {
return (NNG_ENOTSUP);
if (strcmp(name, NNG_OPT_URL) == 0) {
return (NNG_EREADONLY);
}
nni_mtx_lock(&ep->ep_mtx);
if (check && ep->ep_started) {

for (eo = ep->ep_ops.ep_options; eo && eo->eo_name; eo++) {
int rv;

if (strcmp(eo->eo_name, name) != 0) {
continue;
}
if (eo->eo_setopt == NULL) {
return (NNG_EREADONLY);
}
nni_mtx_lock(&ep->ep_mtx);
// XXX: Consider removing this test.
if (ep->ep_started) {
nni_mtx_unlock(&ep->ep_mtx);
return (NNG_ESTATE);
}
rv = eo->eo_setopt(ep->ep_data, val, sz);
nni_mtx_unlock(&ep->ep_mtx);
return (NNG_ESTATE);
return (rv);
}
rv = ep->ep_ops.ep_setopt(ep->ep_data, opt, val, sz);
nni_mtx_unlock(&ep->ep_mtx);
return (rv);

// XXX: socket fallback
return (NNG_ENOTSUP);
}

int
nni_ep_getopt(nni_ep *ep, int opt, void *valp, size_t *szp)
nni_ep_getopt(nni_ep *ep, const char *name, void *valp, size_t *szp)
{
int rv;
nni_tran_ep_option *eo;

if (opt == nng_optid_url) {
if (strcmp(name, NNG_OPT_URL) == 0) {
return (nni_getopt_str(ep->ep_url, valp, szp));
}
if (ep->ep_ops.ep_getopt == NULL) {
return (NNG_ENOTSUP);

for (eo = ep->ep_ops.ep_options; eo && eo->eo_name; eo++) {
int rv;
if (strcmp(eo->eo_name, name) != 0) {
continue;
}
if (eo->eo_getopt == NULL) {
return (NNG_EWRITEONLY);
}
nni_mtx_lock(&ep->ep_mtx);
rv = eo->eo_getopt(ep->ep_data, valp, szp);
nni_mtx_unlock(&ep->ep_mtx);
return (rv);
}
nni_mtx_lock(&ep->ep_mtx);
rv = ep->ep_ops.ep_getopt(ep->ep_data, opt, valp, szp);
nni_mtx_unlock(&ep->ep_mtx);
return (rv);

return (nni_sock_getopt(ep->ep_sock, name, valp, szp));
}

void
Expand Down
38 changes: 19 additions & 19 deletions src/core/endpt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,25 @@
#ifndef CORE_ENDPT_H
#define CORE_ENDPT_H

extern int nni_ep_sys_init(void);
extern void nni_ep_sys_fini(void);
extern nni_tran * nni_ep_tran(nni_ep *);
extern nni_sock * nni_ep_sock(nni_ep *);
extern int nni_ep_find(nni_ep **, uint32_t);
extern int nni_ep_hold(nni_ep *);
extern void nni_ep_rele(nni_ep *);
extern uint32_t nni_ep_id(nni_ep *);
extern int nni_ep_create_dialer(nni_ep **, nni_sock *, const char *);
extern int nni_ep_create_listener(nni_ep **, nni_sock *, const char *);
extern void nni_ep_stop(nni_ep *);
extern int nni_ep_shutdown(nni_ep *);
extern void nni_ep_close(nni_ep *);
extern int nni_ep_dial(nni_ep *, int);
extern int nni_ep_listen(nni_ep *, int);
extern void nni_ep_list_init(nni_list *);
extern int nni_ep_setopt(nni_ep *, int, const void *, size_t, int);
extern int nni_ep_getopt(nni_ep *, int, void *, size_t *);
extern int nni_ep_pipe_add(nni_ep *ep, nni_pipe *);
extern int nni_ep_sys_init(void);
extern void nni_ep_sys_fini(void);
extern nni_tran *nni_ep_tran(nni_ep *);
extern nni_sock *nni_ep_sock(nni_ep *);
extern int nni_ep_find(nni_ep **, uint32_t);
extern int nni_ep_hold(nni_ep *);
extern void nni_ep_rele(nni_ep *);
extern uint32_t nni_ep_id(nni_ep *);
extern int nni_ep_create_dialer(nni_ep **, nni_sock *, const char *);
extern int nni_ep_create_listener(nni_ep **, nni_sock *, const char *);
extern void nni_ep_stop(nni_ep *);
extern int nni_ep_shutdown(nni_ep *);
extern void nni_ep_close(nni_ep *);
extern int nni_ep_dial(nni_ep *, int);
extern int nni_ep_listen(nni_ep *, int);
extern void nni_ep_list_init(nni_list *);
extern int nni_ep_setopt(nni_ep *, const char *, const void *, size_t);
extern int nni_ep_getopt(nni_ep *, const char *, void *, size_t *);
extern int nni_ep_pipe_add(nni_ep *ep, nni_pipe *);
extern void nni_ep_pipe_remove(nni_ep *, nni_pipe *);
extern const char *nni_ep_url(nni_ep *);

Expand Down
41 changes: 10 additions & 31 deletions src/core/options.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,23 +344,6 @@ nni_option_lookup(const char *name)
return (id);
}

const char *
nni_option_name(int id)
{
nni_option *opt;
const char *name = NULL;

nni_mtx_lock(&nni_option_lk);
NNI_LIST_FOREACH (&nni_options, opt) {
if (id == opt->o_id) {
name = opt->o_name;
break;
}
}
nni_mtx_unlock(&nni_option_lk);
return (name);
}

int
nni_option_register(const char *name, int *idp)
{
Expand Down Expand Up @@ -390,6 +373,15 @@ nni_option_sys_fini(void)
nni_option_nextid = 0;
}

int nni_optid_raw;
int nni_optid_recvmaxsz;
int nni_optid_maxttl;
int nni_optid_protocol;
int nni_optid_transport;
int nni_optid_locaddr;
int nni_optid_remaddr;
int nni_optid_surveyor_surveytime;

int
nni_option_sys_init(void)
{
Expand All @@ -398,28 +390,15 @@ nni_option_sys_init(void)
nni_option_nextid = 0x10000;
int rv;

#define OPT_REGISTER(o) nni_option_register(nng_opt_##o, &nng_optid_##o)
#define OPT_REGISTER(o) nni_option_register(nng_opt_##o, &nni_optid_##o)
// Register our well-known options.
if (((rv = OPT_REGISTER(raw)) != 0) ||
((rv = OPT_REGISTER(linger)) != 0) ||
((rv = OPT_REGISTER(recvbuf)) != 0) ||
((rv = OPT_REGISTER(sendbuf)) != 0) ||
((rv = OPT_REGISTER(recvtimeo)) != 0) ||
((rv = OPT_REGISTER(sendtimeo)) != 0) ||
((rv = OPT_REGISTER(reconnmint)) != 0) ||
((rv = OPT_REGISTER(reconnmaxt)) != 0) ||
((rv = OPT_REGISTER(recvmaxsz)) != 0) ||
((rv = OPT_REGISTER(maxttl)) != 0) ||
((rv = OPT_REGISTER(protocol)) != 0) ||
((rv = OPT_REGISTER(transport)) != 0) ||
((rv = OPT_REGISTER(locaddr)) != 0) ||
((rv = OPT_REGISTER(remaddr)) != 0) ||
((rv = OPT_REGISTER(recvfd)) != 0) ||
((rv = OPT_REGISTER(sendfd)) != 0) ||
((rv = OPT_REGISTER(url)) != 0) ||
((rv = OPT_REGISTER(req_resendtime)) != 0) ||
((rv = OPT_REGISTER(sub_subscribe)) != 0) ||
((rv = OPT_REGISTER(sub_unsubscribe)) != 0) ||
((rv = OPT_REGISTER(surveyor_surveytime)) != 0)) {
nni_option_sys_fini();
return (rv);
Expand Down
10 changes: 10 additions & 0 deletions src/core/options.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,14 @@ extern const char *nni_option_name(int);
extern int nni_option_sys_init(void);
extern void nni_option_sys_fini(void);

extern int nni_optid_raw;
extern int nni_optid_recvmaxsz;
extern int nni_optid_maxttl;
extern int nni_optid_protocol;
extern int nni_optid_transport;
extern int nni_optid_locaddr;
extern int nni_optid_remaddr;
extern int nni_optid_req_resendtime;
extern int nni_optid_surveyor_surveytime;

#endif // CORE_OPTIONS_H
26 changes: 12 additions & 14 deletions src/core/pipe.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

#include "core/nng_impl.h"

#include <string.h>

// This file contains functions relating to pipes.
//
// Operations on pipes (to the transport) are generally blocking operations,
Expand Down Expand Up @@ -281,8 +283,7 @@ nni_pipe_create(nni_ep *ep, void *tdata)
rv = nni_idhash_alloc(nni_pipes, &p->p_id, p);
nni_mtx_unlock(&nni_pipe_lk);

if ((rv != 0) ||
((rv = nni_ep_pipe_add(ep, p)) != 0) ||
if ((rv != 0) || ((rv = nni_ep_pipe_add(ep, p)) != 0) ||
((rv = nni_sock_pipe_add(sock, p)) != 0)) {
nni_pipe_destroy(p);
}
Expand All @@ -291,21 +292,18 @@ nni_pipe_create(nni_ep *ep, void *tdata)
}

int
nni_pipe_getopt(nni_pipe *p, int opt, void *val, size_t *szp)
nni_pipe_getopt(nni_pipe *p, const char *name, void *val, size_t *szp)
{
int rv = NNG_ENOTSUP;
nni_tran_pipe_option *po;

if (opt == nng_optid_url) {
return (nni_getopt_str(p->p_url, val, szp));
}
if (p->p_tran_ops.p_getopt != NULL) {
rv = p->p_tran_ops.p_getopt(p->p_tran_data, opt, val, szp);
}
if (rv == NNG_ENOTSUP) {
// Maybe its a generic socket option?
rv = nni_sock_getopt(p->p_sock, opt, val, szp);
for (po = p->p_tran_ops.p_options; po && po->po_name; po++) {
if (strcmp(po->po_name, name) != 0) {
continue;
}
return (po->po_getopt(p->p_tran_data, val, szp));
}
return (rv);
// Maybe the endpoint knows?
return (nni_ep_getopt(p->p_ep, name, val, szp));
}

void
Expand Down
2 changes: 1 addition & 1 deletion src/core/pipe.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ extern void nni_pipe_start(nni_pipe *);

extern uint16_t nni_pipe_proto(nni_pipe *);
extern uint16_t nni_pipe_peer(nni_pipe *);
extern int nni_pipe_getopt(nni_pipe *, int, void *, size_t *sizep);
extern int nni_pipe_getopt(nni_pipe *, const char *, void *, size_t *);

// nni_pipe_get_proto_data gets the protocol private data set with the
// nni_pipe_set_proto_data function. No locking is performed.
Expand Down
13 changes: 9 additions & 4 deletions src/core/protocol.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ struct nni_proto_pipe_ops {
void (*pipe_stop)(void *);
};

struct nni_proto_sock_option {
const char *pso_name;
int (*pso_getopt)(void *, void *, size_t *);
int (*pso_setopt)(void *, const void *, size_t);
};

struct nni_proto_sock_ops {
// sock_init creates the protocol instance, which will be stored on
// the socket. This is run without the sock lock held, and allocates
Expand All @@ -68,10 +74,6 @@ struct nni_proto_sock_ops {
// it can signal the socket worker threads to exit.
void (*sock_close)(void *);

// Option manipulation. These may be NULL.
int (*sock_setopt)(void *, int, const void *, size_t);
int (*sock_getopt)(void *, int, void *, size_t *);

// Receive filter. This may be NULL, but if it isn't, then
// messages coming into the system are routed here just before being
// delivered to the application. To drop the message, the prtocol
Expand All @@ -81,6 +83,9 @@ struct nni_proto_sock_ops {
// Send filter. This may be NULL, but if it isn't, then messages
// here are filtered just after they come from the application.
nni_msg *(*sock_sfilter)(void *, nni_msg *);

// Options. Must not be NULL. Final entry should have NULL name.
nni_proto_sock_option *sock_options;
};

typedef struct nni_proto_id {
Expand Down
Loading

0 comments on commit 64db0f0

Please sign in to comment.