I'm probably not enough of an expert on ipv4/ipv6 setup to be much help in review, but below are a few nits so you can at least know I looked through it.
On 08/21/2013 03:44 PM, Richard Maciel wrote: > Server will now bind in two ports: one for tcp and one > for tcp6. It only will fail if both protocol fails. > > A future patch will add configuration options to allow > the user to disable each protocol individually. > > Signed-off-by: Richard Maciel <[email protected]> > --- > src/tcsd/svrside.c | 304 > ++++++++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 241 insertions(+), 63 deletions(-) > > diff --git a/src/tcsd/svrside.c b/src/tcsd/svrside.c > index 08ca1c8..ef56eb5 100644 > --- a/src/tcsd/svrside.c > +++ b/src/tcsd/svrside.c > @@ -27,6 +27,7 @@ > #include <arpa/inet.h> > #include <errno.h> > #include <getopt.h> > +#include <sys/select.h> > #include "trousers/tss.h" > #include "trousers_types.h" > #include "tcs_tsp.h" > @@ -46,6 +47,13 @@ extern char *optarg; > int sd; > char *tcsd_config_file = NULL; > > +struct srv_sock_info { > + int sd; > + int domain; // AF_INET or AF_INET6 > + socklen_t addr_len; > +}; > +#define MAX_IP_PROTO 2 > + > static void > tcsd_shutdown(void) > { > @@ -211,17 +219,181 @@ reload_config(void) > return result; > } > > +int setup_ipv4_socket(struct srv_sock_info *ssi) > +{ > + struct sockaddr_in serv_addr; > + int sd, opt; > + > + ssi->sd = -1; > + > + // Initialization of IPv4 socket. > + sd = socket(AF_INET, SOCK_STREAM, 0); > + if (sd < 0) { > + LogWarn("Failed IPv4 socket: %s", strerror(errno)); > + goto err; > + } > + > + memset(&serv_addr, 0, sizeof (serv_addr)); This memset should work, but memset in general is prone to compiler optimization, so I'd rather we wrap all memset() calls throughout TrouSerS in __tcsd_memset() for consistency. > + serv_addr.sin_family = AF_INET; > + serv_addr.sin_port = htons(tcsd_options.port); > + > + /* If no remote_ops are defined, restrict connections to localhost > + * only at the socket. */ > + if (tcsd_options.remote_ops[0] == 0) > + serv_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); > + else > + serv_addr.sin_addr.s_addr = htonl(INADDR_ANY); > + > + opt = 1; > + setsockopt(sd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)); > + if (bind(sd, (struct sockaddr *) &serv_addr, sizeof (serv_addr)) < 0) { > + LogWarn("Failed IPv4 bind: %s", strerror(errno)); > + goto err; > + } > + > + if (listen(sd, TCSD_MAX_SOCKETS_QUEUED) < 0) { > + LogWarn("Failed IPv4 listen: %s", strerror(errno)); > + goto err; > + } > + > + ssi->domain = AF_INET; > + ssi->sd = sd; > + ssi->addr_len = sizeof(serv_addr); > + > + return 0; > + > + err: > + if (sd != -1) > + close(sd); > + > + return -1; > +} > + > +int setup_ipv6_socket(struct srv_sock_info *ssi) > +{ > + struct sockaddr_in6 serv6_addr; > + int sd6, opt; > + > + ssi->sd = -1; > + > + sd6 = socket(AF_INET6, SOCK_STREAM, 0); > + if (sd6 < 0) { > + LogWarn("Failed IPv6 socket: %s", strerror(errno)); > + goto err; > + } > + > + memset(&serv6_addr, 0, sizeof (serv6_addr)); Ditto. > + serv6_addr.sin6_family = AF_INET6; > + serv6_addr.sin6_port = htons(tcsd_options.port); > + > + /* If no remote_ops are defined, restrict connections to localhost > + * only at the socket. */ > + if (tcsd_options.remote_ops[0] == 0) > + serv6_addr.sin6_addr = in6addr_loopback; > + else > + serv6_addr.sin6_addr = in6addr_any; > + > +#ifdef __linux__ > + opt = 1; > + if(setsockopt(sd6, IPPROTO_IPV6, IPV6_V6ONLY, &opt, sizeof(opt)) == -1) > { > + LogWarn("Could not set IPv6 socket option properly.\n"); > + goto err; > + } > +#endif > + > + opt = 1; opt=1; is duplicated and should be moved above the ifdef I also am not an expert enough here, but anytime I see an #ifdef __linux__ I'd like to see a comment talking about what is going on that is system specific, and if other OSes might need to do something different. What about SOLARIS for instance? > + setsockopt(sd6, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)); > + if (bind(sd6, (struct sockaddr *) &serv6_addr, sizeof (serv6_addr)) < > 0) { > + LogWarn("Failed IPv6 bind: %s", strerror(errno)); > + goto err; > + } > + > + if (listen(sd6, TCSD_MAX_SOCKETS_QUEUED) < 0) { > + LogWarn("Failed IPv6 listen: %s", strerror(errno)); > + goto err; > + } > + > + ssi->domain = AF_INET6; > + ssi->sd = sd6; > + ssi->addr_len = sizeof(serv6_addr); > + > + return 0; > + > + err: > + if (sd != -1) > + close(sd6); > + > + return -1; > +} > + <snip> > -#ifndef SOLARIS > + #ifndef SOLARIS Don't think we need to be indenting #ifndefs ------------------------------------------------------------------------------ Introducing Performance Central, a new site from SourceForge and AppDynamics. Performance Central is your source for news, insights, analysis and resources for efficient Application Performance Management. Visit us today! http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk _______________________________________________ TrouSerS-tech mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/trousers-tech
