From bc8ab5f710b4ab69d82522104aebe9e1629391cd Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Wed, 2 Feb 2011 21:34:49 +0100 Subject: [PATCH] libev/c-ares migration; fix lookup chains - use idle-watchers for unwinding the c-ares stack (thanks to Marc Lehmann for the hint) - use the generic c-ares query & parse logic instead of the integrated call; we need to first do the a then fall back to the aaaa request which the c-ares default one doesn't implement - stop the shutdown watchers when finalizing one client --- citadel/event_client.c | 6 +- citadel/event_client.h | 32 ++-- citadel/modules/c-ares-dns/serv_c-ares-dns.c | 190 +++++++++++-------- citadel/modules/smtp/serv_smtpeventclient.c | 49 +++-- 4 files changed, 162 insertions(+), 115 deletions(-) diff --git a/citadel/event_client.c b/citadel/event_client.c index 5c16cdd5b..4a20ba1ba 100644 --- a/citadel/event_client.c +++ b/citadel/event_client.c @@ -127,6 +127,8 @@ void ShutDownCLient(AsyncIO *IO) { CtdlLogPrintf(CTDL_DEBUG, "EVENT x %d\n", IO->sock); + ev_cleanup_stop(event_base, &IO->abort_by_shutdown); + if (IO->sock != 0) { ev_io_stop(event_base, &IO->send_event); @@ -338,12 +340,12 @@ IO_recv_callback(struct ev_loop *loop, ev_io *watcher, int revents) } void -IO_postdns_callback(struct ev_loop *loop, ev_timer *watcher, int revents) +IO_postdns_callback(struct ev_loop *loop, ev_idle *watcher, int revents) { AsyncIO *IO = watcher->data; CtdlLogPrintf(CTDL_DEBUG, "event: %s\n", __FUNCTION__); - IO->PostDNS(IO); + IO->DNSQuery->PostDNS(IO); } eNextState event_connect_socket(AsyncIO *IO, double conn_timeout, double first_rw_timeout) diff --git a/citadel/event_client.h b/citadel/event_client.h index daf158dcb..57d920407 100644 --- a/citadel/event_client.h +++ b/citadel/event_client.h @@ -20,6 +20,17 @@ typedef eReadState (*IO_LineReaderCallback)(AsyncIO *IO); typedef void (*ParseDNSAnswerCb)(AsyncIO*, unsigned char*, int); typedef void (*FreeDNSReply)(void *DNSData); +typedef struct _DNSQueryParts { + ParseDNSAnswerCb DNS_CB; + IO_CallBack PostDNS; + + int DNSStatus; + void *VParsedDNSReply; + FreeDNSReply DNSReplyFree; + void *Data; +} DNSQueryParts; + + struct AsyncIO { StrBuf *Host; char service[32]; @@ -30,7 +41,6 @@ struct AsyncIO { /* connection related */ int IP6; - struct hostent *HEnt; struct sockaddr_in6 Addr; int sock; @@ -40,8 +50,8 @@ struct AsyncIO { ev_cleanup abort_by_shutdown; ev_timer conn_fail, - rw_timeout, - unwind_stack_timeout; + rw_timeout; + ev_idle unwind_stack; ev_io recv_event, send_event, conn_event; @@ -62,19 +72,11 @@ struct AsyncIO { IO_LineReaderCallback LineReader; /* if we have linereaders, maybe we want to read more lines before the real application logic is called? */ - - int active_dns_event; ev_io dns_recv_event, dns_send_event; struct ares_options DNSOptions; ares_channel DNSChannel; - - ParseDNSAnswerCb DNS_CB; - IO_CallBack PostDNS; - - int DNSStatus; - void *VParsedDNSReply; - FreeDNSReply DNSReplyFree; + DNSQueryParts *DNSQuery; /* Custom data; its expected to contain AsyncIO so we can save malloc()s... */ DeleteHashDataFunc DeleteData; /* so if we have to destroy you, what to do... */ @@ -96,9 +98,11 @@ eNextState InitEventIO(AsyncIO *IO, double conn_timeout, double first_rw_timeout, int ReadFirst); -void IO_postdns_callback(struct ev_loop *loop, ev_timer *watcher, int revents); +void IO_postdns_callback(struct ev_loop *loop, ev_idle *watcher, int revents); + +int QueueQuery(ns_type Type, const char *name, AsyncIO *IO, DNSQueryParts *QueryParts, IO_CallBack PostDNS); +void QueueGetHostByName(AsyncIO *IO, const char *Hostname, DNSQueryParts *QueryParts, IO_CallBack PostDNS); -int QueueQuery(ns_type Type, char *name, AsyncIO *IO, IO_CallBack PostDNS); void QueryCbDone(AsyncIO *IO); void StopClient(AsyncIO *IO); diff --git a/citadel/modules/c-ares-dns/serv_c-ares-dns.c b/citadel/modules/c-ares-dns/serv_c-ares-dns.c index e7a0f57e2..4534d88ef 100644 --- a/citadel/modules/c-ares-dns/serv_c-ares-dns.c +++ b/citadel/modules/c-ares-dns/serv_c-ares-dns.c @@ -69,12 +69,12 @@ static void HostByAddrCb(void *data, struct hostent *hostent) { AsyncIO *IO = data; - IO->DNSStatus = status; + IO->DNSQuery->DNSStatus = status; if (status != ARES_SUCCESS) { // ResolveError(*cb, status); return; } - IO->Data = hostent; + IO->DNSQuery->Data = hostent; /// TODO: howto free this?? } @@ -82,17 +82,17 @@ static void ParseAnswerA(AsyncIO *IO, unsigned char* abuf, int alen) { struct hostent* host; - if (IO->VParsedDNSReply != NULL) - IO->DNSReplyFree(IO->VParsedDNSReply); - IO->VParsedDNSReply = NULL; + if (IO->DNSQuery->VParsedDNSReply != NULL) + IO->DNSQuery->DNSReplyFree(IO->DNSQuery->VParsedDNSReply); + IO->DNSQuery->VParsedDNSReply = NULL; - IO->DNSStatus = ares_parse_a_reply(abuf, alen, &host, NULL, NULL); - if (IO->DNSStatus != ARES_SUCCESS) { + IO->DNSQuery->DNSStatus = ares_parse_a_reply(abuf, alen, &host, NULL, NULL); + if (IO->DNSQuery->DNSStatus != ARES_SUCCESS) { // ResolveError(arg->js_cb, status); return; } - IO->VParsedDNSReply = host; - IO->DNSReplyFree = (FreeDNSReply) ares_free_hostent; + IO->DNSQuery->VParsedDNSReply = host; + IO->DNSQuery->DNSReplyFree = (FreeDNSReply) ares_free_hostent; } @@ -100,17 +100,17 @@ static void ParseAnswerAAAA(AsyncIO *IO, unsigned char* abuf, int alen) { struct hostent* host; - if (IO->VParsedDNSReply != NULL) - IO->DNSReplyFree(IO->VParsedDNSReply); - IO->VParsedDNSReply = NULL; + if (IO->DNSQuery->VParsedDNSReply != NULL) + IO->DNSQuery->DNSReplyFree(IO->DNSQuery->VParsedDNSReply); + IO->DNSQuery->VParsedDNSReply = NULL; - IO->DNSStatus = ares_parse_aaaa_reply(abuf, alen, &host, NULL, NULL); - if (IO->DNSStatus != ARES_SUCCESS) { + IO->DNSQuery->DNSStatus = ares_parse_aaaa_reply(abuf, alen, &host, NULL, NULL); + if (IO->DNSQuery->DNSStatus != ARES_SUCCESS) { // ResolveError(arg->js_cb, status); return; } - IO->VParsedDNSReply = host; - IO->DNSReplyFree = (FreeDNSReply) ares_free_hostent; + IO->DNSQuery->VParsedDNSReply = host; + IO->DNSQuery->DNSReplyFree = (FreeDNSReply) ares_free_hostent; } @@ -118,19 +118,19 @@ static void ParseAnswerCNAME(AsyncIO *IO, unsigned char* abuf, int alen) { struct hostent* host; - if (IO->VParsedDNSReply != NULL) - IO->DNSReplyFree(IO->VParsedDNSReply); - IO->VParsedDNSReply = NULL; + if (IO->DNSQuery->VParsedDNSReply != NULL) + IO->DNSQuery->DNSReplyFree(IO->DNSQuery->VParsedDNSReply); + IO->DNSQuery->VParsedDNSReply = NULL; - IO->DNSStatus = ares_parse_a_reply(abuf, alen, &host, NULL, NULL); - if (IO->DNSStatus != ARES_SUCCESS) { + IO->DNSQuery->DNSStatus = ares_parse_a_reply(abuf, alen, &host, NULL, NULL); + if (IO->DNSQuery->DNSStatus != ARES_SUCCESS) { // ResolveError(arg->js_cb, status); return; } // a CNAME lookup always returns a single record but - IO->VParsedDNSReply = host; - IO->DNSReplyFree = (FreeDNSReply) ares_free_hostent; + IO->DNSQuery->VParsedDNSReply = host; + IO->DNSQuery->DNSReplyFree = (FreeDNSReply) ares_free_hostent; } @@ -138,18 +138,18 @@ static void ParseAnswerMX(AsyncIO *IO, unsigned char* abuf, int alen) { struct ares_mx_reply *mx_out; - if (IO->VParsedDNSReply != NULL) - IO->DNSReplyFree(IO->VParsedDNSReply); - IO->VParsedDNSReply = NULL; + if (IO->DNSQuery->VParsedDNSReply != NULL) + IO->DNSQuery->DNSReplyFree(IO->DNSQuery->VParsedDNSReply); + IO->DNSQuery->VParsedDNSReply = NULL; - IO->DNSStatus = ares_parse_mx_reply(abuf, alen, &mx_out); - if (IO->DNSStatus != ARES_SUCCESS) { + IO->DNSQuery->DNSStatus = ares_parse_mx_reply(abuf, alen, &mx_out); + if (IO->DNSQuery->DNSStatus != ARES_SUCCESS) { // ResolveError(arg->js_cb, status); return; } - IO->VParsedDNSReply = mx_out; - IO->DNSReplyFree = (FreeDNSReply) ares_free_data; + IO->DNSQuery->VParsedDNSReply = mx_out; + IO->DNSQuery->DNSReplyFree = (FreeDNSReply) ares_free_data; } @@ -157,17 +157,17 @@ static void ParseAnswerNS(AsyncIO *IO, unsigned char* abuf, int alen) { struct hostent* host; - if (IO->VParsedDNSReply != NULL) - IO->DNSReplyFree(IO->VParsedDNSReply); - IO->VParsedDNSReply = NULL; + if (IO->DNSQuery->VParsedDNSReply != NULL) + IO->DNSQuery->DNSReplyFree(IO->DNSQuery->VParsedDNSReply); + IO->DNSQuery->VParsedDNSReply = NULL; - IO->DNSStatus = ares_parse_ns_reply(abuf, alen, &host); - if (IO->DNSStatus != ARES_SUCCESS) { + IO->DNSQuery->DNSStatus = ares_parse_ns_reply(abuf, alen, &host); + if (IO->DNSQuery->DNSStatus != ARES_SUCCESS) { // ResolveError(arg->js_cb, status); return; } - IO->VParsedDNSReply = host; - IO->DNSReplyFree = (FreeDNSReply) ares_free_hostent; + IO->DNSQuery->VParsedDNSReply = host; + IO->DNSQuery->DNSReplyFree = (FreeDNSReply) ares_free_hostent; } @@ -175,18 +175,18 @@ static void ParseAnswerSRV(AsyncIO *IO, unsigned char* abuf, int alen) { struct ares_srv_reply *srv_out; - if (IO->VParsedDNSReply != NULL) - IO->DNSReplyFree(IO->VParsedDNSReply); - IO->VParsedDNSReply = NULL; + if (IO->DNSQuery->VParsedDNSReply != NULL) + IO->DNSQuery->DNSReplyFree(IO->DNSQuery->VParsedDNSReply); + IO->DNSQuery->VParsedDNSReply = NULL; - IO->DNSStatus = ares_parse_srv_reply(abuf, alen, &srv_out); - if (IO->DNSStatus != ARES_SUCCESS) { + IO->DNSQuery->DNSStatus = ares_parse_srv_reply(abuf, alen, &srv_out); + if (IO->DNSQuery->DNSStatus != ARES_SUCCESS) { // ResolveError(arg->js_cb, status); return; } - IO->VParsedDNSReply = srv_out; - IO->DNSReplyFree = (FreeDNSReply) ares_free_data; + IO->DNSQuery->VParsedDNSReply = srv_out; + IO->DNSQuery->DNSReplyFree = (FreeDNSReply) ares_free_data; } @@ -194,17 +194,17 @@ static void ParseAnswerTXT(AsyncIO *IO, unsigned char* abuf, int alen) { struct ares_txt_reply *txt_out; - if (IO->VParsedDNSReply != NULL) - IO->DNSReplyFree(IO->VParsedDNSReply); - IO->VParsedDNSReply = NULL; + if (IO->DNSQuery->VParsedDNSReply != NULL) + IO->DNSQuery->DNSReplyFree(IO->DNSQuery->VParsedDNSReply); + IO->DNSQuery->VParsedDNSReply = NULL; - IO->DNSStatus = ares_parse_txt_reply(abuf, alen, &txt_out); - if (IO->DNSStatus != ARES_SUCCESS) { + IO->DNSQuery->DNSStatus = ares_parse_txt_reply(abuf, alen, &txt_out); + if (IO->DNSQuery->DNSStatus != ARES_SUCCESS) { // ResolveError(arg->js_cb, status); return; } - IO->VParsedDNSReply = txt_out; - IO->DNSReplyFree = (FreeDNSReply) ares_free_data; + IO->DNSQuery->VParsedDNSReply = txt_out; + IO->DNSQuery->DNSReplyFree = (FreeDNSReply) ares_free_data; } void QueryCb(void *arg, @@ -215,23 +215,23 @@ void QueryCb(void *arg, { AsyncIO *IO = arg; - IO->DNSStatus = status; + IO->DNSQuery->DNSStatus = status; if (status == ARES_SUCCESS) - IO->DNS_CB(arg, abuf, alen); + IO->DNSQuery->DNS_CB(arg, abuf, alen); else - IO->DNSStatus = status; -/// ev_io_stop(event_base, &IO->dns_io_event); + IO->DNSQuery->DNSStatus = status; +/// ev_io_stop(event_base, &IO->DNSQuery->dns_io_event); - ev_timer_init(&IO->unwind_stack_timeout, - IO_postdns_callback, 0.0, 0); - IO->unwind_stack_timeout.data = IO; - ev_timer_start(event_base, &IO->unwind_stack_timeout); + ev_idle_init(&IO->unwind_stack, + IO_postdns_callback); + IO->unwind_stack.data = IO; + ev_idle_start(event_base, &IO->unwind_stack); CtdlLogPrintf(CTDL_DEBUG, "C-ARES: %s\n", __FUNCTION__); } void QueryCbDone(AsyncIO *IO) { - ev_timer_stop(event_base, &IO->unwind_stack_timeout); + ev_idle_stop(event_base, &IO->unwind_stack); } @@ -245,40 +245,76 @@ void InitC_ares_dns(AsyncIO *IO) ares_init_options(&IO->DNSChannel, &IO->DNSOptions, optmask); } } -int QueueQuery(ns_type Type, char *name, AsyncIO *IO, IO_CallBack PostDNS) + +void QueueGetHostByNameDone(void *Ctx, + int status, + int timeouts, + struct hostent *hostent) +{ + AsyncIO *IO = (AsyncIO *) Ctx; + + IO->DNSQuery->DNSStatus = status; + IO->DNSQuery->VParsedDNSReply = hostent; + IO->DNSQuery->DNSReplyFree = (FreeDNSReply) ares_free_hostent; + + ev_idle_init(&IO->unwind_stack, + IO_postdns_callback); + IO->unwind_stack.data = IO; + ev_idle_start(event_base, &IO->unwind_stack); + CtdlLogPrintf(CTDL_DEBUG, "C-ARES: %s\n", __FUNCTION__); +} + +void QueueGetHostByName(AsyncIO *IO, const char *Hostname, DNSQueryParts *QueryParts, IO_CallBack PostDNS) +{ + IO->DNSQuery = QueryParts; + IO->DNSQuery->PostDNS = PostDNS; + + InitC_ares_dns(IO); + + ares_gethostbyname(IO->DNSChannel, + Hostname, + AF_INET6, /* it falls back to ipv4 in doubt... */ + QueueGetHostByNameDone, + IO); +//get_one_mx_host_ip_done); +} +int QueueQuery(ns_type Type, const char *name, AsyncIO *IO, DNSQueryParts *QueryParts, IO_CallBack PostDNS) { int length, family; char address_b[sizeof(struct in6_addr)]; + IO->DNSQuery = QueryParts; + IO->DNSQuery->PostDNS = PostDNS; + InitC_ares_dns(IO); - IO->PostDNS = PostDNS; + switch(Type) { case ns_t_a: - IO->DNS_CB = ParseAnswerA; + IO->DNSQuery->DNS_CB = ParseAnswerA; break; case ns_t_aaaa: - IO->DNS_CB = ParseAnswerAAAA; + IO->DNSQuery->DNS_CB = ParseAnswerAAAA; break; case ns_t_mx: - IO->DNS_CB = ParseAnswerMX; + IO->DNSQuery->DNS_CB = ParseAnswerMX; break; case ns_t_ns: - IO->DNS_CB = ParseAnswerNS; + IO->DNSQuery->DNS_CB = ParseAnswerNS; break; case ns_t_txt: - IO->DNS_CB = ParseAnswerTXT; + IO->DNSQuery->DNS_CB = ParseAnswerTXT; break; case ns_t_srv: - IO->DNS_CB = ParseAnswerSRV; + IO->DNSQuery->DNS_CB = ParseAnswerSRV; break; case ns_t_cname: - IO->DNS_CB = ParseAnswerCNAME; + IO->DNSQuery->DNS_CB = ParseAnswerCNAME; break; case ns_t_ptr: @@ -305,6 +341,7 @@ int QueueQuery(ns_type Type, char *name, AsyncIO *IO, IO_CallBack PostDNS) return 1; } + static void DNS_send_callback(struct ev_loop *loop, ev_io *watcher, int revents) { AsyncIO *IO = watcher->data; @@ -330,27 +367,23 @@ void SockStateCb(void *data, int sock, int read, int write) if (read) { if ((IO->dns_recv_event.fd != sock) && - (IO->dns_recv_event.fd != 0) && - ((IO->active_dns_event & EV_READ) != 0)) { + (IO->dns_recv_event.fd != 0)) { ev_io_stop(event_base, &IO->dns_recv_event); } IO->dns_recv_event.fd = sock; ev_io_init(&IO->dns_recv_event, DNS_recv_callback, IO->dns_recv_event.fd, EV_READ); IO->dns_recv_event.data = IO; ev_io_start(event_base, &IO->dns_recv_event); - IO->active_dns_event = IO->active_dns_event | EV_READ; } if (write) { if ((IO->dns_send_event.fd != sock) && - (IO->dns_send_event.fd != 0) && - ((IO->active_dns_event & EV_WRITE) != 0)) { + (IO->dns_send_event.fd != 0)) { ev_io_stop(event_base, &IO->dns_send_event); } IO->dns_send_event.fd = sock; ev_io_init(&IO->dns_send_event, DNS_send_callback, IO->dns_send_event.fd, EV_WRITE); IO->dns_send_event.data = IO; ev_io_start(event_base, &IO->dns_send_event); - IO->active_dns_event = IO->active_dns_event | EV_WRITE; } /* @@ -363,11 +396,8 @@ void SockStateCb(void *data, int sock, int read, int write) } */ if ((read == 0) && (write == 0)) { - if ((IO->active_dns_event & EV_READ) != 0) - ev_io_stop(event_base, &IO->dns_recv_event); - if ((IO->active_dns_event & EV_WRITE) != 0) - ev_io_stop(event_base, &IO->dns_send_event); - IO->active_dns_event = 0; + ev_io_stop(event_base, &IO->dns_recv_event); + ev_io_stop(event_base, &IO->dns_send_event); } } diff --git a/citadel/modules/smtp/serv_smtpeventclient.c b/citadel/modules/smtp/serv_smtpeventclient.c index 3c4e6d0c8..5d53110cb 100644 --- a/citadel/modules/smtp/serv_smtpeventclient.c +++ b/citadel/modules/smtp/serv_smtpeventclient.c @@ -160,7 +160,10 @@ typedef struct _stmp_out_msg { const char *mx_port; const char *mx_host; + DNSQueryParts MxLookup; + DNSQueryParts HostLookup; struct hostent *OneMX; +/// struct hostent *HEnt; ParsedURL *Relay; ParsedURL *pCurrRelay; @@ -177,7 +180,8 @@ void DeleteSmtpOutMsg(void *v) SmtpOutMsg *Msg = v; ares_free_data(Msg->AllMX); - + if (Msg->HostLookup.VParsedDNSReply != NULL) + Msg->HostLookup.DNSReplyFree(Msg->HostLookup.VParsedDNSReply); FreeStrBuf(&Msg->msgtext); FreeAsyncIOContents(&Msg->IO); free(Msg); @@ -335,19 +339,19 @@ eNextState mx_connect_relay_ip(AsyncIO *IO) 1); } -void get_one_mx_host_ip_done(void *Ctx, - int status, - int timeouts, - struct hostent *hostent) +eNextState get_one_mx_host_ip_done(AsyncIO *IO) { - AsyncIO *IO = (AsyncIO *) Ctx; SmtpOutMsg *SendMsg = IO->Data; - eNextState State = eAbort; + struct hostent *hostent; + + QueryCbDone(IO); - if ((status == ARES_SUCCESS) && (hostent != NULL) ) { + hostent = SendMsg->HostLookup.VParsedDNSReply; + if ((SendMsg->HostLookup.DNSStatus == ARES_SUCCESS) && + (hostent != NULL) ) { IO->IP6 = hostent->h_addrtype == AF_INET6; - IO->HEnt = hostent; + ////IO->HEnt = hostent; memset(&IO->Addr, 0, sizeof(struct in6_addr)); if (IO->IP6) { @@ -368,15 +372,15 @@ void get_one_mx_host_ip_done(void *Ctx, addr->sin_port = htons(IO->dport); } - SendMsg->IO.HEnt = hostent; + ////SendMsg->IO.HEnt = hostent; SetConnectStatus(IO); - State = InitEventIO(IO, SendMsg, + return InitEventIO(IO, SendMsg, SMTP_C_ConnTimeout, SMTP_C_ReadTimeouts[0], 1); } - if ((State == eAbort) && (IO->sock != -1)) - SMTP_C_Terminate(IO); + else + return SMTP_C_Terminate(IO); } const unsigned short DefaultMXPort = 25; @@ -409,11 +413,17 @@ eNextState get_one_mx_host_ip(AsyncIO *IO) Hostname, SendMsg->IO.dport); - ares_gethostbyname(SendMsg->IO.DNSChannel, - Hostname, - AF_INET6, /* it falls back to ipv4 in doubt... */ - get_one_mx_host_ip_done, - &SendMsg->IO); + if (!QueueQuery(ns_t_a, + Hostname, + &SendMsg->IO, + &SendMsg->HostLookup, + get_one_mx_host_ip_done)) + { + SendMsg->MyQEntry->Status = 5; + StrBufPrintf(SendMsg->MyQEntry->StatusMessage, + "No MX hosts found for <%s>", SendMsg->node); + return IO->NextState; + } return IO->NextState; } @@ -428,7 +438,7 @@ eNextState smtp_resolve_mx_done(AsyncIO *IO) SendMsg->IO.ErrMsg = SendMsg->MyQEntry->StatusMessage; - SendMsg->CurrMX = SendMsg->AllMX = IO->VParsedDNSReply; + SendMsg->CurrMX = SendMsg->AllMX = IO->DNSQuery->VParsedDNSReply; //// TODO: should we remove the current ares context??? get_one_mx_host_ip(IO); return IO->NextState; @@ -444,6 +454,7 @@ eNextState resolve_mx_records(AsyncIO *IO) if (!QueueQuery(ns_t_mx, SendMsg->node, &SendMsg->IO, + &SendMsg->MxLookup, smtp_resolve_mx_done)) { SendMsg->MyQEntry->Status = 5; -- 2.30.2