Re: [PATCH] bugfix to make do-resolve to use DNS cache
On Thu, Nov 07, 2019 at 08:40:26AM +0100, Baptiste wrote:
> Hi Willy,
>
> Please find the patch updated. I also cleared a '{' '}' that I added on a
> if condition. This would make the code "cleaner" but should not be part of
> this patch at all.
> The new patch is in attachment.
Applied, thank you Baptiste!
> Sorry again for the mess.
No problem, shit happens.
Willy
Re: [PATCH] bugfix to make do-resolve to use DNS cache
Hi Willy, Jarno,
Sorry, I did forgot those 2 printf that were here for debugging purpose
only.
I can resend the patch tonight.
Baptiste
On Wed, Nov 6, 2019 at 7:43 AM Willy Tarreau wrote:
> Hi Baptiste,
>
> thanks for the fix, but before taking it, are you really sure it's
> the version you wanted to send ? There are a couple of debugging
> printf() left so I could remove them by hand but maybe you intended
> to send a different patch, thus I'd rather let you double-check.
>
> thanks,
> Willy
>
> On Tue, Nov 05, 2019 at 10:04:30AM +0100, Baptiste wrote:
> > diff --git a/src/action.c b/src/action.c
> > index 7684202..36eedc8 100644
> > --- a/src/action.c
> > +++ b/src/action.c
> > @@ -73,6 +73,7 @@ int check_trk_action(struct act_rule *rule, struct
> proxy *px, char **err)
> > int act_resolution_cb(struct dns_requester *requester, struct
> dns_nameserver *nameserver)
> > {
> > struct stream *stream;
> > +printf("%s %d\n", __FUNCTION__, __LINE__);
> >
> > if (requester->resolution == NULL)
> > return 0;
> > @@ -89,6 +90,7 @@ int act_resolution_cb(struct dns_requester *requester,
> struct dns_nameserver *na
> > int act_resolution_error_cb(struct dns_requester *requester, int
> error_code)
> > {
> > struct stream *stream;
> > +printf("%s %d\n", __FUNCTION__, __LINE__);
> >
> > if (requester->resolution == NULL)
> > return 0;
> > diff --git a/src/dns.c b/src/dns.c
> > index 15d40a1..d5bf449 100644
> > --- a/src/dns.c
> > +++ b/src/dns.c
> > @@ -363,8 +363,9 @@ void dns_trigger_resolution(struct dns_requester
> *req)
> >* valid */
> > exp = tick_add(res->last_resolution, resolvers->hold.valid);
> > if (resolvers->t && (res->status != RSLV_STATUS_VALID ||
> > - !tick_isset(res->last_resolution) || tick_is_expired(exp,
> now_ms)))
> > + !tick_isset(res->last_resolution) || tick_is_expired(exp,
> now_ms))) {
> > task_wakeup(resolvers->t, TASK_WOKEN_OTHER);
> > + }
> > }
> >
> >
> > @@ -2150,8 +2151,13 @@ enum act_return dns_action_do_resolve(struct
> act_rule *rule, struct proxy *px,
> > struct dns_resolution *resolution;
> > struct sample *smp;
> > char *fqdn;
> > + struct dns_requester *req;
> > + struct dns_resolvers *resolvers;
> > + struct dns_resolution *res;
> > + int exp;
> >
> > /* we have a response to our DNS resolution */
> > + use_cache:
> > if (s->dns_ctx.dns_requester &&
> s->dns_ctx.dns_requester->resolution != NULL) {
> > resolution = s->dns_ctx.dns_requester->resolution;
> > if (resolution->step == RSLV_STEP_RUNNING) {
> > @@ -2211,6 +2217,22 @@ enum act_return dns_action_do_resolve(struct
> act_rule *rule, struct proxy *px,
> >
> > s->dns_ctx.parent = rule;
> > dns_link_resolution(s, OBJ_TYPE_STREAM, 0);
> > +
> > + /* Check if there is a fresh enough response in the cache of our
> associated resolution */
> > + req = s->dns_ctx.dns_requester;
> > + if (!req || !req->resolution) {
> > + dns_trigger_resolution(s->dns_ctx.dns_requester);
> > + return ACT_RET_YIELD;
> > + }
> > + res = req->resolution;
> > + resolvers = res->resolvers;
> > +
> > + exp = tick_add(res->last_resolution, resolvers->hold.valid);
> > + if (resolvers->t && res->status == RSLV_STATUS_VALID &&
> tick_isset(res->last_resolution)
> > +&& !tick_is_expired(exp, now_ms)) {
> > + goto use_cache;
> > + }
> > +
> > dns_trigger_resolution(s->dns_ctx.dns_requester);
> > return ACT_RET_YIELD;
> > }
> > --
> > 2.7.4
> >
>
>
Re: [PATCH] bugfix to make do-resolve to use DNS cache
Hi,
On Tue, Nov 05, Baptiste wrote:
> David Birdsong reported a bug last week about http do-resolve action not
> using the DNS cache.
> The patch in attachment fixes this issue.
> There is no github issue associated to this bug.
> Backport status is up to 2.0.
Quick question: are the printf's there on purpose or leftover debug
outputs ?
-Jarno
@@ -73,6 +73,7 @@ int check_trk_action(struct act_rule *rule, struct proxy *px,
char **err)
int act_resolution_cb(struct dns_requester *requester, struct dns_nameserver
*nameserver)
{
struct stream *stream;
+printf("%s %d\n", __FUNCTION__, __LINE__);
if (requester->resolution == NULL)
return 0;
@@ -89,6 +90,7 @@ int act_resolution_cb(struct dns_requester *requester, struct
dns_nameserver *na
int act_resolution_error_cb(struct dns_requester *requester, int error_code)
{
struct stream *stream;
+printf("%s %d\n", __FUNCTION__, __LINE__);
if (requester->resolution == NULL)
return 0;
--
Jarno Huuskonen
Re: [PATCH] bugfix to make do-resolve to use DNS cache
Hi Baptiste,
thanks for the fix, but before taking it, are you really sure it's
the version you wanted to send ? There are a couple of debugging
printf() left so I could remove them by hand but maybe you intended
to send a different patch, thus I'd rather let you double-check.
thanks,
Willy
On Tue, Nov 05, 2019 at 10:04:30AM +0100, Baptiste wrote:
> diff --git a/src/action.c b/src/action.c
> index 7684202..36eedc8 100644
> --- a/src/action.c
> +++ b/src/action.c
> @@ -73,6 +73,7 @@ int check_trk_action(struct act_rule *rule, struct proxy
> *px, char **err)
> int act_resolution_cb(struct dns_requester *requester, struct dns_nameserver
> *nameserver)
> {
> struct stream *stream;
> +printf("%s %d\n", __FUNCTION__, __LINE__);
>
> if (requester->resolution == NULL)
> return 0;
> @@ -89,6 +90,7 @@ int act_resolution_cb(struct dns_requester *requester,
> struct dns_nameserver *na
> int act_resolution_error_cb(struct dns_requester *requester, int error_code)
> {
> struct stream *stream;
> +printf("%s %d\n", __FUNCTION__, __LINE__);
>
> if (requester->resolution == NULL)
> return 0;
> diff --git a/src/dns.c b/src/dns.c
> index 15d40a1..d5bf449 100644
> --- a/src/dns.c
> +++ b/src/dns.c
> @@ -363,8 +363,9 @@ void dns_trigger_resolution(struct dns_requester *req)
>* valid */
> exp = tick_add(res->last_resolution, resolvers->hold.valid);
> if (resolvers->t && (res->status != RSLV_STATUS_VALID ||
> - !tick_isset(res->last_resolution) || tick_is_expired(exp, now_ms)))
> + !tick_isset(res->last_resolution) || tick_is_expired(exp, now_ms)))
> {
> task_wakeup(resolvers->t, TASK_WOKEN_OTHER);
> + }
> }
>
>
> @@ -2150,8 +2151,13 @@ enum act_return dns_action_do_resolve(struct act_rule
> *rule, struct proxy *px,
> struct dns_resolution *resolution;
> struct sample *smp;
> char *fqdn;
> + struct dns_requester *req;
> + struct dns_resolvers *resolvers;
> + struct dns_resolution *res;
> + int exp;
>
> /* we have a response to our DNS resolution */
> + use_cache:
> if (s->dns_ctx.dns_requester && s->dns_ctx.dns_requester->resolution !=
> NULL) {
> resolution = s->dns_ctx.dns_requester->resolution;
> if (resolution->step == RSLV_STEP_RUNNING) {
> @@ -2211,6 +2217,22 @@ enum act_return dns_action_do_resolve(struct act_rule
> *rule, struct proxy *px,
>
> s->dns_ctx.parent = rule;
> dns_link_resolution(s, OBJ_TYPE_STREAM, 0);
> +
> + /* Check if there is a fresh enough response in the cache of our
> associated resolution */
> + req = s->dns_ctx.dns_requester;
> + if (!req || !req->resolution) {
> + dns_trigger_resolution(s->dns_ctx.dns_requester);
> + return ACT_RET_YIELD;
> + }
> + res = req->resolution;
> + resolvers = res->resolvers;
> +
> + exp = tick_add(res->last_resolution, resolvers->hold.valid);
> + if (resolvers->t && res->status == RSLV_STATUS_VALID &&
> tick_isset(res->last_resolution)
> +&& !tick_is_expired(exp, now_ms)) {
> + goto use_cache;
> + }
> +
> dns_trigger_resolution(s->dns_ctx.dns_requester);
> return ACT_RET_YIELD;
> }
> --
> 2.7.4
>
Re: [PATCH] bugfix to make do-resolve to use DNS cache
Thanks! On Tue, Nov 5, 2019, 1:04 AM Baptiste wrote: > Hi there, > > David Birdsong reported a bug last week about http do-resolve action not > using the DNS cache. > The patch in attachment fixes this issue. > There is no github issue associated to this bug. > Backport status is up to 2.0. > > Baptiste >
[PATCH] bugfix to make do-resolve to use DNS cache
Hi there,
David Birdsong reported a bug last week about http do-resolve action not
using the DNS cache.
The patch in attachment fixes this issue.
There is no github issue associated to this bug.
Backport status is up to 2.0.
Baptiste
From 74e1328ef08de6740c30b5b5989d1413bb904742 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann
Date: Wed, 30 Oct 2019 16:06:53 +0100
Subject: [PATCH] BUG/MINOR: action: do-resolve now use cached response
As reported by David Birdsong on the ML, the HTTP action do-resolve does
not use the DNS cache.
Actually, the action is "registred" to the resolution for said name to
be resolved and wait until an other requester triggers the it. Once the
resolution is finished, then the action is updated with the result.
To trigger this, you must have a server with runtime DNS resolution
enabled and run a do-resolve action with the same fqdn AND they use the
same resolvers section.
This patch fixes this behavior by ensuring the resolution associated to
the action has a valid answer which is not considered as expired. If
those conditions are valid, then we can use it (it's the "cache").
Backport status: 2.0
---
src/action.c | 2 ++
src/dns.c| 24 +++-
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/src/action.c b/src/action.c
index 7684202..36eedc8 100644
--- a/src/action.c
+++ b/src/action.c
@@ -73,6 +73,7 @@ int check_trk_action(struct act_rule *rule, struct proxy *px, char **err)
int act_resolution_cb(struct dns_requester *requester, struct dns_nameserver *nameserver)
{
struct stream *stream;
+printf("%s %d\n", __FUNCTION__, __LINE__);
if (requester->resolution == NULL)
return 0;
@@ -89,6 +90,7 @@ int act_resolution_cb(struct dns_requester *requester, struct dns_nameserver *na
int act_resolution_error_cb(struct dns_requester *requester, int error_code)
{
struct stream *stream;
+printf("%s %d\n", __FUNCTION__, __LINE__);
if (requester->resolution == NULL)
return 0;
diff --git a/src/dns.c b/src/dns.c
index 15d40a1..d5bf449 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -363,8 +363,9 @@ void dns_trigger_resolution(struct dns_requester *req)
* valid */
exp = tick_add(res->last_resolution, resolvers->hold.valid);
if (resolvers->t && (res->status != RSLV_STATUS_VALID ||
- !tick_isset(res->last_resolution) || tick_is_expired(exp, now_ms)))
+ !tick_isset(res->last_resolution) || tick_is_expired(exp, now_ms))) {
task_wakeup(resolvers->t, TASK_WOKEN_OTHER);
+ }
}
@@ -2150,8 +2151,13 @@ enum act_return dns_action_do_resolve(struct act_rule *rule, struct proxy *px,
struct dns_resolution *resolution;
struct sample *smp;
char *fqdn;
+ struct dns_requester *req;
+ struct dns_resolvers *resolvers;
+ struct dns_resolution *res;
+ int exp;
/* we have a response to our DNS resolution */
+ use_cache:
if (s->dns_ctx.dns_requester && s->dns_ctx.dns_requester->resolution != NULL) {
resolution = s->dns_ctx.dns_requester->resolution;
if (resolution->step == RSLV_STEP_RUNNING) {
@@ -2211,6 +2217,22 @@ enum act_return dns_action_do_resolve(struct act_rule *rule, struct proxy *px,
s->dns_ctx.parent = rule;
dns_link_resolution(s, OBJ_TYPE_STREAM, 0);
+
+ /* Check if there is a fresh enough response in the cache of our associated resolution */
+ req = s->dns_ctx.dns_requester;
+ if (!req || !req->resolution) {
+ dns_trigger_resolution(s->dns_ctx.dns_requester);
+ return ACT_RET_YIELD;
+ }
+ res = req->resolution;
+ resolvers = res->resolvers;
+
+ exp = tick_add(res->last_resolution, resolvers->hold.valid);
+ if (resolvers->t && res->status == RSLV_STATUS_VALID && tick_isset(res->last_resolution)
+ && !tick_is_expired(exp, now_ms)) {
+ goto use_cache;
+ }
+
dns_trigger_resolution(s->dns_ctx.dns_requester);
return ACT_RET_YIELD;
}
--
2.7.4
