FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

dgram: skip dns.lookup() for literal IP addresses · nodejs/node@9e50bb0 · GitHub

/ node Public

Commit 9e50bb0

Browse files
BridgeAR
authored and
richardlau
committed
dgram: skip dns.lookup() for literal IP addresses
Every unconnected send(), and the implicit bind on first send, resolved the destination through dns.lookup() even when it was already a literal IP. The address resolves to itself, so the call is redundant, and tools that instrument dns.lookup() record a lookup for every datagram sent to an IP. Skip the resolver for a literal IP of the socket's family and report it on the next tick, keeping dns.lookup()'s asynchronous contract. A custom lookup function is still consulted for every address. Refs: DataDog/dd-trace-js#2984 Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de> PR-URL: #64133 Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent b7bf6e3 commit 9e50bb0

6 files changed

Lines changed: 155 additions & 17 deletions

File tree

‎benchmark/dgram/send-to-ip.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Measure the send rate to a literal IP destination. The destination needs no
2+
// name resolution, so this isolates the per-send overhead the default lookup
3+
// pays before the packet reaches the socket.
4+
'use strict';
5+
6+
const common = require('../common.js');
7+
const dgram = require('dgram');
8+
const PORT = common.PORT;
9+
10+
// `n` is the number of send requests queued each round. Keep it high (>10) so
11+
// the measurement reflects send overhead rather than event loop cycles.
12+
const bench = common.createBenchmark(main, {
13+
n: [100],
14+
dur: [5],
15+
});
16+
17+
function main({ dur, n }) {
18+
const chunk = Buffer.allocUnsafe(1);
19+
let sent = 0;
20+
const socket = dgram.createSocket('udp4');
21+
22+
function onsend() {
23+
if (sent++ % n === 0) {
24+
setImmediate(() => {
25+
for (let i = 0; i < n; i++) {
26+
socket.send(chunk, PORT, '127.0.0.1', onsend);
27+
}
28+
});
29+
}
30+
}
31+
32+
socket.on('listening', () => {
33+
bench.start();
34+
onsend();
35+
36+
setTimeout(() => {
37+
bench.end(sent);
38+
process.exit(0);
39+
}, dur * 1000);
40+
});
41+
42+
socket.bind(PORT);
43+
}

‎doc/api/dgram.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,8 @@ changes:
10391039
* `recvBufferSize` {number} Sets the `SO_RCVBUF` socket value.
10401040
* `sendBufferSize` {number} Sets the `SO_SNDBUF` socket value.
10411041
* `lookup` {Function} Custom lookup function. **Default:** [`dns.lookup()`][].
1042+
When the default is used, a literal IP address of the socket's family
1043+
resolves to itself without calling [`dns.lookup()`][].
10421044
* `signal` {AbortSignal} An AbortSignal that may be used to close a socket.
10431045
* `receiveBlockList` {net.BlockList} `receiveBlockList` can be used for discarding
10441046
inbound datagram to specific IP addresses, IP ranges, or IP subnets. This does not

‎lib/internal/dgram.js‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const {
88
const { codes: {
99
ERR_SOCKET_BAD_TYPE,
1010
} } = require('internal/errors');
11+
const { isIP } = require('internal/net');
1112
const { UDP } = internalBinding('udp_wrap');
1213
const { guessHandleType } = require('internal/util');
1314
const {
@@ -28,13 +29,22 @@ function lookup6(lookup, address, callback) {
2829
return lookup(address || '::1', 6, callback);
2930
}
3031

32+
// A literal IP of the socket's family resolves to itself, so skip dns.lookup().
33+
// Defer with nextTick to keep the callback async (e.g. bind()'s 'listening').
34+
function defaultLookup(address, family, callback) {
35+
if (isIP(address) === family) {
36+
process.nextTick(callback, null, address, family);
37+
return;
38+
}
39+
if (dns === undefined) {
40+
dns = require('dns');
41+
}
42+
return dns.lookup(address, family, callback);
43+
}
44+
3145
function newHandle(type, lookup) {
3246
if (lookup === undefined) {
33-
if (dns === undefined) {
34-
dns = require('dns');
35-
}
36-
37-
lookup = dns.lookup;
47+
lookup = defaultLookup;
3848
} else {
3949
validateFunction(lookup, 'lookup');
4050
}

‎test/parallel/test-dgram-custom-lookup.js‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ const assert = require('assert');
44
const dgram = require('dgram');
55
const dns = require('dns');
66

7+
const originalLookup = dns.lookup;
8+
79
{
810
// Verify that the provided lookup function is called.
911
const lookup = common.mustCall((host, family, callback) => {
10-
dns.lookup(host, family, callback);
12+
originalLookup(host, family, callback);
1113
});
1214

1315
const socket = dgram.createSocket({ type: 'udp4', lookup });
@@ -18,17 +20,17 @@ const dns = require('dns');
1820
}
1921

2022
{
21-
// Verify that lookup defaults to dns.lookup().
22-
const originalLookup = dns.lookup;
23-
23+
// Verify that the default lookup forwards host names to dns.lookup().
2424
dns.lookup = common.mustCall((host, family, callback) => {
2525
dns.lookup = originalLookup;
26-
originalLookup(host, family, callback);
26+
assert.strictEqual(host, 'example.invalid');
27+
assert.strictEqual(family, 4);
28+
callback(null, '127.0.0.1', 4);
2729
});
2830

2931
const socket = dgram.createSocket({ type: 'udp4' });
3032

31-
socket.bind(common.mustCall(() => {
33+
socket.bind(0, 'example.invalid', common.mustCall(() => {
3234
socket.close();
3335
}));
3436
}

‎test/parallel/test-dgram-default-lookup-ip.js‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
'use strict';
2+
3+
// The default dgram lookup resolves a literal IP address of the socket's own
4+
// family to itself, without calling dns.lookup(). Each case below stubs the
5+
// process-global dns.lookup(), so they run sequentially to keep one case from
6+
// observing another's stub.
7+
8+
const common = require('../common');
9+
const assert = require('assert');
10+
const dgram = require('dgram');
11+
const dns = require('dns');
12+
13+
const originalLookup = dns.lookup;
14+
15+
function ipv4SendSkipsLookup(next) {
16+
dns.lookup = common.mustNotCall('dns.lookup() ran for an IPv4 literal');
17+
18+
const receiver = dgram.createSocket('udp4');
19+
const sender = dgram.createSocket('udp4');
20+
21+
receiver.on('message', common.mustCall((msg) => {
22+
assert.strictEqual(msg.toString(), 'payload');
23+
dns.lookup = originalLookup;
24+
receiver.close();
25+
sender.close();
26+
next();
27+
}));
28+
29+
receiver.bind(0, '127.0.0.1', common.mustCall(() => {
30+
sender.send('payload', receiver.address().port, '127.0.0.1', common.mustCall());
31+
}));
32+
}
33+
34+
function ipv6BindSkipsLookup(next) {
35+
if (!common.hasIPv6) {
36+
next();
37+
return;
38+
}
39+
40+
dns.lookup = common.mustNotCall('dns.lookup() ran for an IPv6 literal');
41+
42+
const socket = dgram.createSocket('udp6');
43+
44+
socket.bind(0, '::1', common.mustCall(() => {
45+
dns.lookup = originalLookup;
46+
socket.close();
47+
next();
48+
}));
49+
}
50+
51+
function mismatchedFamilyFallsThrough(next) {
52+
// '::1' is not an IPv4 literal, so a udp4 socket still resolves it via
53+
// dns.lookup() rather than short-circuiting.
54+
dns.lookup = common.mustCall((host, family, callback) => {
55+
dns.lookup = originalLookup;
56+
assert.strictEqual(host, '::1');
57+
assert.strictEqual(family, 4);
58+
callback(null, '127.0.0.1', 4);
59+
});
60+
61+
const socket = dgram.createSocket('udp4');
62+
63+
socket.bind(0, '::1', common.mustCall(() => {
64+
socket.close();
65+
next();
66+
}));
67+
}
68+
69+
const cases = [
70+
ipv4SendSkipsLookup,
71+
ipv6BindSkipsLookup,
72+
mismatchedFamilyFallsThrough,
73+
];
74+
75+
(function runNext() {
76+
const testCase = cases.shift();
77+
if (testCase !== undefined) {
78+
testCase(runNext);
79+
}
80+
})();

‎test/sequential/test-dgram-implicit-bind-failure.js‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,20 @@ const common = require('../common');
44
const assert = require('assert');
55
const EventEmitter = require('events');
66
const dgram = require('dgram');
7-
const dns = require('dns');
87
const { kStateSymbol } = require('internal/dgram');
98
const mockError = new Error('fake DNS');
109

11-
// Monkey patch dns.lookup() so that it always fails.
12-
dns.lookup = function(address, family, callback) {
10+
const socket = dgram.createSocket('udp4');
11+
12+
// Fail the implicit bind by making the handle's address resolution fail. A
13+
// literal bind address is not passed to dns.lookup(), so patching dns.lookup()
14+
// would not be observed here.
15+
socket[kStateSymbol].handle.lookup = function(address, callback) {
1316
process.nextTick(() => { callback(mockError); });
1417
};
1518

16-
const socket = dgram.createSocket('udp4');
17-
1819
socket.on(EventEmitter.errorMonitor, common.mustCall((err) => {
19-
// The DNS lookup should fail since it is monkey patched. At that point in
20+
// The bind should fail since the lookup is monkey patched. At that point in
2021
// time, the send queue should be populated with the send() operation.
2122
assert.strictEqual(err, mockError);
2223
assert(Array.isArray(socket[kStateSymbol].queue));

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL