Re: [RFC 0/3] DNS proxy fixes

2015-04-08 Thread Patrik Flykt
On Tue, 2015-04-07 at 18:30 +0300, Jukka Rissanen wrote:
 Hi,
 
 Slava noticed that dnsproxy.c does not set request buffer for
 UDP requests but leaves it NULL. This is fixed in patch 1 by Slava.
 I added that patch into this set (and added more details into
 commit message) as this is very much RFC material.
 
 After this patchset the UDP packet resending when the resolver is
...

Applied all three patches, thanks!

Patrik

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman


[RFC 0/3] DNS proxy fixes

2015-04-07 Thread Jukka Rissanen
Hi,

Slava noticed that dnsproxy.c does not set request buffer for
UDP requests but leaves it NULL. This is fixed in patch 1 by Slava.
I added that patch into this set (and added more details into
commit message) as this is very much RFC material.

After this patchset the UDP packet resending when the resolver is
flushed starts to work after many years of failure. The resolver
is flushed every time name servers are updated. This happens even
if we disconnect a service which is very much pointless (some more
patches will be sent to fix this later). What all this means, is that
the UDP packet re-send and failure notifications to calling resolver
client has not been working at all for some time.

For UDP packets that the dnsproxy sends to external DNS servers,
the req-request buffer was NULL. This then meant that the dnsproxy
code that tries to send message to client in a case of failure cannot
really do anything. The successful request was passed correctly
to the client that sent the initial DNS request. So the issue
has been the failure case when the external DNS server is not
responding. There should be no similar issues in TCP side.

Fortunately no one has reported this issue as the client app resolver
(libc) still has its own timeout and everything is fine if there is
an resolver error in ConnMan side. Anyway, the changes in patch 1
activate now some code paths that have not been in use for some time.
So this set needs more testing in order not to introduce more bugs.

Because everything seems to work just fine at the moment with the
current code, it is quite possible that we could remove even more
extra cruft from dnsproxy.c instead of introducing these fixes in
this set. So if we do not apply patch 1, we could save some
memory by removing dead code. By applying patch 1, more memory is
consumed as we need to save the request buffer for each UDP DNS
request.

Patch 2 removes the numserv check in request_timeout() because
it is pointless as we need to send some data to client anyway.
Earlier this code path was not taken for UDP packets (because
the req-request buffer was NULL).

I also refactored the request_timeout() in patch 3 as it had
duplicate code and was hard to follow.


Cheers,
Jukka


Jukka Rissanen (2):
  dnsproxy: Do not check numserv in request timeout
  dnsproxy: Refactor the request_timeout() function

Slava Monich (1):
  dnsproxy: request_data in request_list need the request data for UDP

 src/dnsproxy.c | 67 --
 1 file changed, 32 insertions(+), 35 deletions(-)

-- 
2.1.0

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman