Hi László,
On Fri, Sep 05, 2014 at 10:18:25AM +0200, Sárközi, László wrote:
> Dear haproxy list,
>
> We've been working on a project that involves using haproxy to proxy
> between multiple networks represented as multiple network namespaces on the
> proxy host.
(...)
That's really great. I'm having some comments about this patch series
however, please find them inline with the patches :
> diff --git a/include/types/server.h b/include/types/server.h
> index 94f9a0f..9d5c6ed 100644
> --- a/include/types/server.h
> +++ b/include/types/server.h
> @@ -85,6 +85,8 @@ enum srv_admin {
> #define SRV_F_BACKUP 0x0001 /* this server is a backup server */
> #define SRV_F_MAPPORTS 0x0002 /* this server uses mapped ports */
> #define SRV_F_NON_STICK 0x0004 /* never add connections allocated
> to this server to a stick table */
> +#define SRV_F_USE_CONFIGURED_NAMESPACE 0x0010
> +#define SRV_F_USE_NAMESPACE_FROM_PROXY_PROTOCOL 0x0020
Are you sure you couldn't find shorter names for these macros ? I'm generally
not that much picky about this, but they're *really* large. Eg, please try
something along SRV_F_USE_CONF_NS and SRV_F_USE_NS_FROM_PP or something like
this that sounds good to you.
> diff --git a/src/backend.c b/src/backend.c
> index a96b767..9b10cd3 100644
> --- a/src/backend.c
> +++ b/src/backend.c
> @@ -719,6 +719,45 @@ int assign_server(struct session *s)
> return err;
> }
>
> +/*
> + * This function assigns a server connection network namespace.
> + * The network namespace is taken from the configuration file.
> + * If flag SRV_F_USE_NAMESPACE_FROM_PROXY_PROTOCOL is set server connection
> + * relay the client network namespace. If flag SRV_F_USE_CONFIGURED_NAMESPACE
> + * is set server connection namespace comes form configuration.
> + *
> + * It may return :
> + * SRV_STATUS_OK if everything is OK.
> + * SRV_STATUS_INTERNAL for other unrecoverable errors.
> + *
> + * Upon successful return, the connection flag CO_FL_NAMESPACE_SET and
> + * CO_FL_NAMESPACE_RECV and network namespace are set.
> + *
> + */
> +int set_srv_conn_network_namespace(struct session *s, struct connection
> *cli_conn, struct connection *srv_conn)
> +{
> + if ((objt_server(s->target)->flags &
> SRV_F_USE_NAMESPACE_FROM_PROXY_PROTOCOL) &&
> + (objt_server(s->target)->flags &
> SRV_F_USE_CONFIGURED_NAMESPACE) )
> + return SRV_STATUS_INTERNAL;
> +
> + if ((objt_server(s->target)->flags &
> SRV_F_USE_NAMESPACE_FROM_PROXY_PROTOCOL) &&
> + (cli_conn->flags & CO_FL_NAMESPACE_SET)){
> +
> + conn_clear_network_namespace(srv_conn);
> + srv_conn->network_namespace =
> strdup(cli_conn->network_namespace);
> + srv_conn->flags |= CO_FL_NAMESPACE_SET;
> + srv_conn->flags |= CO_FL_NAMESPACE_RECV;
> + }
> +
> + if (objt_server(s->target)->flags & SRV_F_USE_CONFIGURED_NAMESPACE){
> + conn_clear_network_namespace(srv_conn);
> +
> + srv_conn->network_namespace =
> (objt_server(s->target)->network_namespace);
> + srv_conn->flags |= CO_FL_NAMESPACE_SET;
Please, use a single indent level above.
> + }
> +
> + return SRV_STATUS_OK;
> +}
>
> /*
> * This function assigns a server address to a session, and sets SN_ADDR_SET.
> @@ -802,6 +841,10 @@ int assign_server_address(struct session *s)
> return SRV_STATUS_INTERNAL;
> }
>
> + /* Configure network namespace */
> + if(set_srv_conn_network_namespace(s,cli_conn,srv_conn) ==
> SRV_STATUS_INTERNAL)
And similarly for the name, probably "set_srv_conn_netns" should be
enough for this function.
> + return SRV_STATUS_INTERNAL;
> +
> s->flags |= SN_ADDR_SET;
> return SRV_STATUS_OK;
> }
> diff --git a/src/connection.c b/src/connection.c
> index 3af6d9a..e0b6172 100644
> --- a/src/connection.c
> +++ b/src/connection.c
> @@ -217,6 +217,16 @@ void conn_update_sock_polling(struct connection *c)
> c->flags = f;
> }
>
> +void conn_clear_network_namespace(struct connection* conn)
> +{
> + if((conn->flags & CO_FL_NAMESPACE_RECV) && (conn->network_namespace !=
> NULL))
> + {
> + free(conn->network_namespace);
> + conn->network_namespace = NULL;
> + conn->flags &= ~CO_FL_NAMESPACE_RECV;
> + }
It seems to me that this test is unnecessary and that the code could
be performed unconditionally. We'd better do it in this case, as history
has shown that many bugs arise from conditions that are improperly
converted when flags change later.
> +}
> +
> /* This handshake handler waits a PROXY protocol header at the beginning of
> the
> * raw data stream. The header looks like this :
> *
> diff --git a/src/haproxy.c b/src/haproxy.c
> index 13c3d26..df02401 100644
> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -1176,6 +1176,9 @@ void deinit(void)
> free(s->agent.bi);
> free(s->agent.bo);
> free((char*)s->conf.file);
> + if (s->network_namespace != NULL) {
> + free(s->network_namespace);
> + }
The test is undesired and not needed, please simply call free(), it
performs the test itself.
> free(s);
> s = s_next;
> }/* end while(s) */
> @@ -1187,6 +1190,8 @@ void deinit(void)
> LIST_DEL(&l->by_bind);
> free(l->name);
> free(l->counters);
> + if(l->network_namespace != NULL)
> + free(l->network_namespace);
same here.
> free(l);
> }
>
> diff --git a/src/namespace.c b/src/namespace.c
> new file mode 100644
> index 0000000..20d027d
> --- /dev/null
> +++ b/src/namespace.c
> @@ -0,0 +1,74 @@
> +#include <common/namespace.h>
> +
> +#define _GNU_SOURCE
> +
> +#include <stdio.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <sys/socket.h>
> +#include <sched.h>
(...)
I have not found a way to prevent this file from being built since
namespace.o is unconditionally added to the Makefile's OBJS, but it
specifically relies on Linux. The best way generally consists in
adding a new option to the makefile just like for CTTPROXY, which
references the new file. For example :
ifneq ($(USE_NS),)
OPTIONS_CFLAGS += -DCONFIG_HAP_NS ### only if needed
OPTIONS_OBJS += src/namespace.o
BUILD_OPTIONS += $(call ignore_implicit,USE_NS)
endif
> +
> +static int open_named_namespace(const char *ns_name) {
Please indicate the expected return values above so that new users do
proper error checking. I think that would be enough :
/* Opens the namespace <ns_name> and returns the FD or -1 in case of
* error (check errno).
*/
> + char buf[512];
> + int fd;
> +
> + snprintf(buf, sizeof(buf), "/var/run/netns/%s", ns_name);
Just out of curiosity, is it the location defined by LSB/FHS/whatever standard
or is it on a specific distro ? We're seeing haproxy embedded into various
products and non-standard locations rarely follow the ones of "fat" distros.
Also, a test on the snprintf() result would be desired, since a user could
craft too large a string to cause it to fail and have open() try to open
anything (think about shared hosting environments where APIs are used to
let users add config sections for example).
> + fd = open(buf, O_RDONLY);
> +
> + return fd;
Also, just a matter of style, above you changed to spaces for indents, please
stick to doc/coding-style.txt to ensure consistency and optimal participation
across contributors.
> +}
> +
Same below about the return value.
> +static int open_current_namespace() {
> + char buf[512];
> + int fd;
> +
> + snprintf(buf, sizeof(buf), "/proc/%d/ns/net", getpid());
> + fd = open(buf, O_RDONLY);
> +
> + return fd;
> +}
> +
return value etc.
> +static int socketat(int default_ns, int target_ns, int domain, int type, int
> protocol) {
> + int sock;
> +
> + if (setns(target_ns, CLONE_NEWNET) == -1)
> + return -1;
> +
> + sock = socket(domain, type, protocol);
> +
> + if (setns(default_ns, CLONE_NEWNET) == -1) {
> + close(sock);
> + return -1;
> + }
> +
> + return sock;
> +}
> +
> +int namespace_socket(const char *ns_name, int domain, int type, int
> protocol) {
> + int default_ns, target_ns, sock;
> +
> + if (ns_name) {
> + default_ns = open_current_namespace();
> + if (default_ns == -1) {
> + return -1;
> + }
> +
> + target_ns = open_named_namespace(ns_name);
> + if (target_ns == -1) {
> + close(default_ns);
> + return -1;
> + }
> +
> + sock = socketat(default_ns, target_ns, domain, type, protocol);
> +
> + close(target_ns);
> + close(default_ns);
> + }
> + else {
> + sock = socket(domain, type, protocol);
> + }
Given that the function above aims at replacing a call to socket(),
some changes will be needed for performance. First, it should be
inlined and an unlikely() put around if (ns_name) so that we don't
hurt too much the most common case.
Second, it becomes critically important that the target_ns remains
open and assigned to each ns so that we don't have to perform that
very expensive open()/close() dance for every socket. Also, haproxy
is designed to run chrooted, so that won't work above because /proc
or /var will not exist in the chroot. And open() is blocking, so
that can become catastrophically slow on a machine with moderate I/O
load.
The proper solution in my opinion is to have a list of (namespace name,
namespace fd) built at startup time, and to store the namespace fd in
the server/listener structs once they're resolved after parsing. Then
you can don't need namespace_socket() anymore and can directly use
socketat().
> + return sock;
> +}
> diff --git a/src/proto_tcp.c b/src/proto_tcp.c
> index 72dc92b..94cb091 100644
> --- a/src/proto_tcp.c
> +++ b/src/proto_tcp.c
> @@ -33,6 +33,7 @@
> #include <common/errors.h>
> #include <common/mini-clist.h>
> #include <common/standard.h>
> +#include <common/namespace.h>
>
> #include <types/global.h>
> #include <types/capture.h>
> @@ -284,6 +285,7 @@ int tcp_connect_server(struct connection *conn, int data,
> int delack)
> struct server *srv;
> struct proxy *be;
> struct conn_src *src;
> + int namespace_set = conn->flags & CO_FL_NAMESPACE_SET;
Isn't the namespace a property of the server instead of the connection,
given that you configure it on the server line ? That makes me think that
this flag (or at least this test) is not needed and that the server's NS
will be enough.
> conn->flags = CO_FL_WAIT_L4_CONN; /* connection in progress */
>
> @@ -301,7 +303,13 @@ int tcp_connect_server(struct connection *conn, int
> data, int delack)
> return SN_ERR_INTERNAL;
> }
>
> - if ((fd = conn->t.sock.fd = socket(conn->addr.to.ss_family,
> SOCK_STREAM, IPPROTO_TCP)) == -1) {
> + if (namespace_set) {
> + fd = conn->t.sock.fd =
> namespace_socket(conn->network_namespace, conn->addr.to.ss_family,
> SOCK_STREAM, IPPROTO_TCP);
> + } else {
> + fd = conn->t.sock.fd = socket(conn->addr.to.ss_family,
> SOCK_STREAM, IPPROTO_TCP);
> + }
These ones will be fused into a common socketat() call.
> + if (fd == -1) {
> qfprintf(stderr, "Cannot get a server socket.\n");
>
> if (errno == ENFILE) {
> @@ -732,10 +740,18 @@ int tcp_bind_listener(struct listener *listener, char
> *errmsg, int errlen)
> fd = listener->fd;
> ext = (fd >= 0);
>
> - if (!ext && (fd = socket(listener->addr.ss_family, SOCK_STREAM,
> IPPROTO_TCP)) == -1) {
> - err |= ERR_RETRYABLE | ERR_ALERT;
> - msg = "cannot create listening socket";
> - goto tcp_return;
> + if (!ext) {
> + if (listener->options & LI_O_NAMESPACE_SET) {
> + fd = namespace_socket(listener->network_namespace,
> listener->addr.ss_family, SOCK_STREAM, IPPROTO_TCP);
> + } else {
> + fd = socket(listener->addr.ss_family, SOCK_STREAM,
> IPPROTO_TCP);
> + }
These ones will be fused into a common socketat() call as well.
> +
> + if (fd == -1) {
> + err |= ERR_RETRYABLE | ERR_ALERT;
> + msg = "cannot create listening socket";
> + goto tcp_return;
> + }
> }
>
> if (fd >= global.maxsock) {
> @@ -1994,6 +2010,29 @@ static int bind_parse_interface(char **args, int
> cur_arg, struct proxy *px, stru
> }
> #endif
>
> +/* parse the "namespace" bind keyword */
> +static int bind_parse_namespace(char **args, int cur_arg, struct proxy *px,
> struct bind_conf *conf, char **err)
> +{
> + struct listener *l;
> + char *namespace = NULL;
> +
> + if (!*args[cur_arg + 1]) {
> + memprintf(err, "'%s' : missing namespace id", args[cur_arg]);
> + return ERR_ALERT | ERR_FATAL;
> + }
> + namespace = args[cur_arg + 1];
> +
> + list_for_each_entry(l, &conf->listeners, by_bind) {
> + l->network_namespace = strdup(namespace);
> + if(l->network_namespace == NULL){
> + Alert("Cannot allocate memory for network namespace:
> '%s'.\n",args[cur_arg + 1]);
> + return ERR_ALERT | ERR_FATAL;
> + }
> + l->options |= LI_O_NAMESPACE_SET;
> + }
> + return 0;
> +}
> +
> static struct cfg_kw_list cfg_kws = {ILH, {
> { CFG_LISTEN, "tcp-request", tcp_parse_tcp_req },
> { CFG_LISTEN, "tcp-response", tcp_parse_tcp_rep },
> @@ -2053,6 +2092,7 @@ static struct bind_kw_list bind_kws = { "TCP", { }, {
> { "v4v6", bind_parse_v4v6, 0 }, /* force socket to
> bind to IPv4+IPv6 */
> { "v6only", bind_parse_v6only, 0 }, /* force socket to
> bind to IPv6 only */
> #endif
> + { "namespace", bind_parse_namespace, 1 },
> /* the versions with the NULL parse function*/
> { "defer-accept", NULL, 0 },
> { "interface", NULL, 1 },
> diff --git a/src/server.c b/src/server.c
> index fdb63cc..7fab99a 100644
> --- a/src/server.c
> +++ b/src/server.c
> @@ -1501,6 +1501,23 @@ int parse_server(const char *file, int linenum, char
> **args, struct proxy *curpr
> err_code |= ERR_ALERT | ERR_FATAL;
> goto out;
> }
> + else if (!defsrv && !strcmp(args[cur_arg],
> "namespace")) {
> + // FIXME: set a flag indicating that the
> namespace has been set
I think your flag is present below, it simply is an or() of the two SRV_F_*
flags that are set.
> + char *arg = args[cur_arg + 1];
> + if (!strcmp(arg, "fromproxy")) {
> + newsrv->flags |=
> SRV_F_USE_NAMESPACE_FROM_PROXY_PROTOCOL;
> + } else {
> + newsrv->network_namespace =
> strdup(args[cur_arg + 1]);
> + if( newsrv->network_namespace == NULL)
> + {
> + Alert("Cannot allocate memory
> for network namespace: '%s'.\n",args[cur_arg + 1]);
> + err_code |= ERR_ALERT |
> ERR_FATAL;
> + goto out;
> + }
> + newsrv->flags |=
> SRV_F_USE_CONFIGURED_NAMESPACE;
> + }
> + cur_arg += 2;
> + }
> else {
> static int srv_dumped;
> struct srv_kw *kw;
> diff --git a/src/session.c b/src/session.c
> index aeaa7e1..10532ae 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -91,6 +91,11 @@ int session_accept(struct listener *l, int cfd, struct
> sockaddr_storage *addr)
> cli_conn->flags |= CO_FL_ADDR_FROM_SET;
> cli_conn->target = &l->obj_type;
>
> + if (l->options & LI_O_NAMESPACE_SET) {
> + cli_conn->flags |= CO_FL_NAMESPACE_SET;
> + cli_conn->network_namespace = l->network_namespace;
Maybe once we have a list of (ns, fd) we'll be able to store the fd only
instead of the ns here, because we try to limit the connection struct to
the smallest possible size.
> + }
> +
> if (unlikely((s = pool_alloc2(pool2_session)) == NULL))
> goto out_free_conn;
>
> --
> 1.9.1
>
Second patch :
> diff --git a/include/types/connection.h b/include/types/connection.h
> index 89f4f38..a75d45d 100644
> --- a/include/types/connection.h
> +++ b/include/types/connection.h
> @@ -334,6 +334,7 @@ struct proxy_hdr_v2 {
> #define PP2_TYPE_SSL 0x20
> #define PP2_TYPE_SSL_VERSION 0x21
> #define PP2_TYPE_SSL_CN 0x22
> +#define PP2_TYPE_NETWORK_NAMESPACE 0x30
Could you please avoid to fragment the values here, I fear the day we need
a contiguous array and we can't find one anymore. Let's start at 0x23 for
now. Note, since we're in development, we can change these numbers later,
so let's not consider the PPv2 spec as "official" whenever it comes to what
we stuff in haproxy-dev.
> +/*
> + * Get tlv data from tlv struct. Function return value is length of data
> + */
I don't want to be picky here but apparently you return a string, not
data, since you stop on the first zero.
> +static int get_tlv_data(struct tlv * src,char *dst, int dst_length)
> +{
> + int length = get_tlv_length(src);
> + if(dst_length < length)
> + return -1;
> + strncpy(dst, (char *)src->value, length);
> + dst[length] = '\0';
> + return length;
Also, you should use strlen(dst) for the return value, because "length" might
be wrong if a zero is present in the TLV entry.
> +static int search_tlv(int type, unsigned int len, char *tlv_packets)
> +{
> + int parsed_length = 0;
> +
> + while (parsed_length <= len && type != tlv_packets[parsed_length])
> + parsed_length += get_tlv_length((struct tlv*)
> &tlv_packets[parsed_length]);
> +
> + return parsed_length;
> +}
It's a good idea to have provided this function, I'm sure it will meet many
new usages!
> /* This handshake handler waits a PROXY protocol header at the beginning of
> the
> * raw data stream. The header looks like this :
> *
> @@ -255,6 +286,7 @@ int conn_recv_proxy(struct connection *conn, int flag)
> char *line, *end;
> struct proxy_hdr_v2 *hdr_v2;
> const char v2sig[] = PP2_SIGNATURE;
> + int tlv_length = 0;
>
> /* we might have been called just after an asynchronous shutr */
> if (conn->flags & CO_FL_SOCK_RD_SH)
> @@ -432,6 +464,7 @@ int conn_recv_proxy(struct connection *conn, int flag)
>
> switch (hdr_v2->ver_cmd & PP2_CMD_MASK) {
> case 0x01: /* PROXY command */
> +
> switch (hdr_v2->fam) {
> case 0x11: /* TCPv4 */
> ((struct sockaddr_in *)&conn->addr.from)->sin_family =
> AF_INET;
Extra empty line above. Please try to avoid adding unnecessary newlines
out of the scope you're working in, they add more context to the patch
and make its backports and fixes a little bit more painful. I'm sure it's
a left-over from a debugging session though, I'm not complaining, just
reporting :-)
> @@ -441,6 +474,7 @@ int conn_recv_proxy(struct connection *conn, int flag)
> ((struct sockaddr_in *)&conn->addr.to)->sin_addr.s_addr
> = hdr_v2->addr.ip4.dst_addr;
> ((struct sockaddr_in *)&conn->addr.to)->sin_port =
> hdr_v2->addr.ip4.dst_port;
> conn->flags |= CO_FL_ADDR_FROM_SET | CO_FL_ADDR_TO_SET;
> + tlv_length = ntohs(hdr_v2->len) - PP2_ADDR_LEN_INET;
> break;
> case 0x21: /* TCPv6 */
> ((struct sockaddr_in6 *)&conn->addr.from)->sin6_family
> = AF_INET6;
> @@ -450,8 +484,40 @@ int conn_recv_proxy(struct connection *conn, int flag)
> memcpy(&((struct sockaddr_in6
> *)&conn->addr.to)->sin6_addr, hdr_v2->addr.ip6.dst_addr, 16);
> ((struct sockaddr_in6 *)&conn->addr.to)->sin6_port =
> hdr_v2->addr.ip6.dst_port;
> conn->flags |= CO_FL_ADDR_FROM_SET | CO_FL_ADDR_TO_SET;
> + tlv_length = ntohs(hdr_v2->len) - PP2_ADDR_LEN_INET6;
> break;
> }
> +
> + /*tlv parsing*/
> + if(tlv_length)
> + {
> + // data from:
> + char * tlv_packets = &trash.str[trash.len-tlv_length];
> +
> + int tlv_first_byte = 0;
> +
> + /*search tlv data, we interested in*/
> + tlv_first_byte = search_tlv(PP2_TYPE_NETWORK_NAMESPACE,
> tlv_length, tlv_packets);
> +
> + /*if tlv_lenght*/
> + if(tlv_first_byte < tlv_length){
> + int network_namspace_leght = 0;
> + network_namspace_leght = get_tlv_length((struct
> tlv*) &tlv_packets[tlv_first_byte]) + 1;
> + /*if the buffer is not enough for namespace*/
> + conn_clear_network_namespace(conn);
> + conn->network_namespace = (char*)
> malloc(network_namspace_leght * sizeof(char));
> + conn->flags |= CO_FL_NAMESPACE_RECV;
> +
> + if(-1 == get_tlv_data((struct tlv*)
> &tlv_packets[tlv_first_byte], conn->network_namespace,
> network_namspace_leght)){
> + goto bad_header;
> + }
> + else
> + {
> + conn->flags |= CO_FL_NAMESPACE_SET;
> + }
> + }
> + }
> +
Please clean up the block above, it's highly out of style compared to the
rest, and even contains some typos ("lenght" and "leght" instead of "length",
"namspace"). Just a few hints, spaces around language keywords/operators/
braces/comments, isolated braces, disturbing "if(result==subject)", etc.
I'm sure your code deserves a bit more love.
> /* unsupported protocol, keep local connection address */
> break;
> case 0x00: /* LOCAL command */
> @@ -601,7 +667,6 @@ int make_proxy_line_v1(char *buf, int buf_len, struct
> sockaddr_storage *src, str
> return ret;
> }
>
> -#ifdef USE_OPENSSL
> static int make_tlv(char *dest, int dest_len, char type, uint16_t length,
> char *value)
> {
> struct tlv *tlv;
> @@ -617,18 +682,18 @@ static int make_tlv(char *dest, int dest_len, char
> type, uint16_t length, char *
> memcpy(tlv->value, value, length);
> return length + sizeof(*tlv);
> }
> -#endif
>
> int make_proxy_line_v2(char *buf, int buf_len, struct server *srv, struct
> connection *remote)
> {
> const char pp2_signature[] = PP2_SIGNATURE;
> int ret = 0;
> + int tlv_len = 0;
> struct proxy_hdr_v2 *hdr = (struct proxy_hdr_v2 *)buf;
> struct sockaddr_storage null_addr = {0};
> struct sockaddr_storage *src = &null_addr;
> struct sockaddr_storage *dst = &null_addr;
> +
> #ifdef USE_OPENSSL
> - int tlv_len = 0;
> char *value = NULL;
> struct tlv_ssl *tlv;
> int ssl_tlv_len = 0;
> @@ -643,6 +708,7 @@ int make_proxy_line_v2(char *buf, int buf_len, struct
> server *srv, struct connec
> src = &remote->addr.from;
> dst = &remote->addr.to;
> }
> +
> if (src && dst && src->ss_family == dst->ss_family && src->ss_family ==
> AF_INET) {
> if (buf_len < PP2_HDR_LEN_INET)
> return 0;
> @@ -707,6 +773,15 @@ int make_proxy_line_v2(char *buf, int buf_len, struct
> server *srv, struct connec
> ret += ssl_tlv_len;
> }
> #endif
> + //PP2_TYPE_NETWORK_NAMESPACE
> +
> + if( remote && (remote->flags & CO_FL_NAMESPACE_SET) ){
> + if ((buf_len - ret) < sizeof(struct tlv))
> + return 0;
Disturbing indents/spaces.
> +
> + tlv_len = make_tlv(&buf[ret], buf_len,
> PP2_TYPE_NETWORK_NAMESPACE, strlen(remote->network_namespace),
> remote->network_namespace);
> + ret += tlv_len;
> + }
Possibly it will make sense to have both the namespace name and its length
in the structures so that you don't need to run strlen() over the name for
every PPv2 line you build. That could probably be done as part of the
(namespace,fd) list design I think, because maybe you'll only need
approximately this for each already opened namespace :
struct open_ns {
char *namespace_str;
char *namespace_len;
int namespace_fd;
};
Then providing a pointer to such a struct into each listener/server should
work reasonably well for all your uses. You can also consider that by design
the default one is inserted first into the list.
Overall, that's great, I'm seeing this as a really useful feature of 1.6
that will gain some traction from users. We always need to have good stuff
at the beginning to incite users to test development releases and detect
the new bugs we add, this is a great opportunity to please everyone :-)
Thanks!
Willy
