Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions benchmark/dgram/send-to-ip.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Measure the send rate to a literal IP destination. The destination needs no
// name resolution, so this isolates the per-send overhead the default lookup
// pays before the packet reaches the socket.
'use strict';

const common = require('../common.js');
const dgram = require('dgram');
const PORT = common.PORT;

// `n` is the number of send requests queued each round. Keep it high (>10) so
// the measurement reflects send overhead rather than event loop cycles.
const bench = common.createBenchmark(main, {
n: [100],
dur: [5],
});

function main({ dur, n }) {
const chunk = Buffer.allocUnsafe(1);
let sent = 0;
const socket = dgram.createSocket('udp4');

function onsend() {
if (sent++ % n === 0) {
setImmediate(() => {
for (let i = 0; i < n; i++) {
socket.send(chunk, PORT, '127.0.0.1', onsend);
}
});
}
}

socket.on('listening', () => {
bench.start();
onsend();

setTimeout(() => {
bench.end(sent);
process.exit(0);
}, dur * 1000);
});

socket.bind(PORT);
}
2 changes: 2 additions & 0 deletions doc/api/dgram.md
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,8 @@ changes:
* `recvBufferSize` {number} Sets the `SO_RCVBUF` socket value.
* `sendBufferSize` {number} Sets the `SO_SNDBUF` socket value.
* `lookup` {Function} Custom lookup function. **Default:** [`dns.lookup()`][].
When the default is used, a literal IP address of the socket's family

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only when the default is used? Is it because the previous behaviour would always call custom function, and not doing so would be breaking?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a stacked PR that does it in general, I was just concerned about it being major if we do it for custom lookup methods, so I decided to split it. I am fine to have it as one PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave it as is to keep it non-major for this PR 👍 .

resolves to itself without calling [`dns.lookup()`][].
* `signal` {AbortSignal} An AbortSignal that may be used to close a socket.
* `receiveBlockList` {net.BlockList} `receiveBlockList` can be used for discarding
inbound datagram to specific IP addresses, IP ranges, or IP subnets. This does not
Expand Down
20 changes: 15 additions & 5 deletions lib/internal/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {
const { codes: {
ERR_SOCKET_BAD_TYPE,
} } = require('internal/errors');
const { isIP } = require('internal/net');
const { UDP } = internalBinding('udp_wrap');
const { guessHandleType } = require('internal/util');
const {
Expand All @@ -28,13 +29,22 @@ function lookup6(lookup, address, callback) {
return lookup(address || '::1', 6, callback);
}

// A literal IP of the socket's family resolves to itself, so skip dns.lookup().
// Defer with nextTick to keep the callback async (e.g. bind()'s 'listening').
function defaultLookup(address, family, callback) {
if (isIP(address) === family) {
process.nextTick(callback, null, address, family);
return;
}
if (dns === undefined) {
dns = require('dns');
}
return dns.lookup(address, family, callback);
}

function newHandle(type, lookup) {
if (lookup === undefined) {
if (dns === undefined) {
dns = require('dns');
}

lookup = dns.lookup;
lookup = defaultLookup;
} else {
validateFunction(lookup, 'lookup');
}
Expand Down
14 changes: 8 additions & 6 deletions test/parallel/test-dgram-custom-lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ const assert = require('assert');
const dgram = require('dgram');
const dns = require('dns');

const originalLookup = dns.lookup;

{
// Verify that the provided lookup function is called.
const lookup = common.mustCall((host, family, callback) => {
dns.lookup(host, family, callback);
originalLookup(host, family, callback);
});

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

{
// Verify that lookup defaults to dns.lookup().
const originalLookup = dns.lookup;

// Verify that the default lookup forwards host names to dns.lookup().
dns.lookup = common.mustCall((host, family, callback) => {
dns.lookup = originalLookup;
originalLookup(host, family, callback);
assert.strictEqual(host, 'example.invalid');
assert.strictEqual(family, 4);
callback(null, '127.0.0.1', 4);
});

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

socket.bind(common.mustCall(() => {
socket.bind(0, 'example.invalid', common.mustCall(() => {
socket.close();
}));
}
Expand Down
80 changes: 80 additions & 0 deletions test/parallel/test-dgram-default-lookup-ip.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
'use strict';

// The default dgram lookup resolves a literal IP address of the socket's own
// family to itself, without calling dns.lookup(). Each case below stubs the
// process-global dns.lookup(), so they run sequentially to keep one case from
// observing another's stub.

const common = require('../common');
const assert = require('assert');
const dgram = require('dgram');
const dns = require('dns');

const originalLookup = dns.lookup;

function ipv4SendSkipsLookup(next) {
dns.lookup = common.mustNotCall('dns.lookup() ran for an IPv4 literal');

const receiver = dgram.createSocket('udp4');
const sender = dgram.createSocket('udp4');

receiver.on('message', common.mustCall((msg) => {
assert.strictEqual(msg.toString(), 'payload');
dns.lookup = originalLookup;
receiver.close();
sender.close();
next();
}));

receiver.bind(0, '127.0.0.1', common.mustCall(() => {
sender.send('payload', receiver.address().port, '127.0.0.1', common.mustCall());
}));
}

function ipv6BindSkipsLookup(next) {
if (!common.hasIPv6) {
next();
return;
}

dns.lookup = common.mustNotCall('dns.lookup() ran for an IPv6 literal');

const socket = dgram.createSocket('udp6');

socket.bind(0, '::1', common.mustCall(() => {
dns.lookup = originalLookup;
socket.close();
next();
}));
}

function mismatchedFamilyFallsThrough(next) {
// '::1' is not an IPv4 literal, so a udp4 socket still resolves it via
// dns.lookup() rather than short-circuiting.
dns.lookup = common.mustCall((host, family, callback) => {
dns.lookup = originalLookup;
assert.strictEqual(host, '::1');
assert.strictEqual(family, 4);
callback(null, '127.0.0.1', 4);
});

const socket = dgram.createSocket('udp4');

socket.bind(0, '::1', common.mustCall(() => {
socket.close();
next();
}));
}

const cases = [
ipv4SendSkipsLookup,
ipv6BindSkipsLookup,
mismatchedFamilyFallsThrough,
];

(function runNext() {
const testCase = cases.shift();
if (testCase !== undefined) {
testCase(runNext);
}
})();
13 changes: 7 additions & 6 deletions test/sequential/test-dgram-implicit-bind-failure.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,20 @@ const common = require('../common');
const assert = require('assert');
const EventEmitter = require('events');
const dgram = require('dgram');
const dns = require('dns');
const { kStateSymbol } = require('internal/dgram');
const mockError = new Error('fake DNS');

// Monkey patch dns.lookup() so that it always fails.
dns.lookup = function(address, family, callback) {
const socket = dgram.createSocket('udp4');

// Fail the implicit bind by making the handle's address resolution fail. A
// literal bind address is not passed to dns.lookup(), so patching dns.lookup()
// would not be observed here.
socket[kStateSymbol].handle.lookup = function(address, callback) {
process.nextTick(() => { callback(mockError); });
};

const socket = dgram.createSocket('udp4');

socket.on(EventEmitter.errorMonitor, common.mustCall((err) => {
// The DNS lookup should fail since it is monkey patched. At that point in
// The bind should fail since the lookup is monkey patched. At that point in
// time, the send queue should be populated with the send() operation.
assert.strictEqual(err, mockError);
assert(Array.isArray(socket[kStateSymbol].queue));
Expand Down
Loading