test/dgram: Add support for IPv6 link local scope IDs in received UDP datagrams by pekkanikander · Pull Request #14500 · nodejs/node

pekkanikander · 2017-07-26T12:03:14Z

A new test case to verify that IPv6 UDP datagrams
received from a link-local source address do contain
the scope ID suffix in the rinfo address field.

Add IPv6 link local scope ID suffix to the
rinfo address in those received upd6 datagrams
whose source address is a link local address.

When a packet is received from a link-local source
address, if the address does not contain the scope
ID suffix, it is impossible to reply back to the
sender, as the kernel is not able to determine
the right network interface to send the packet
through and returns with an error.

Ref: #1649

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

test, dgram

mscdex · 2017-07-26T17:05:04Z

Changes to libuv need to go upstream first. Also FWIW, Windows support seems to be missing.

pekkanikander · 2017-07-26T17:08:48Z

Any chance of getting any help with Windows? I haven't used Windows since 2001 or so.

mscdex · 2017-07-26T17:12:43Z

@pekkanikander You might ask when posting your issue/PR on the libuv repo.

bzoz · 2017-08-16T15:25:24Z

@pekkanikander as you found in the libuv/libuv#1445, Windows uses interface numbers as scope id. This makes Windows change in tcp_wrap.cc much simpler:

// Add an interface identifier to a link local address
if (IN6_IS_ADDR_LINKLOCAL(&a6->sin6_addr)) {
    sprintf(ip + strlen(ip), "%%%d", a6->sin6_scope_id);
}

With this and following change to the test-dgram-udp6-link-local-address.js:

// Map the interface into its address, with interface ID
    .map((iface) => iface.address + '%' + iface.scopeid)

the test passes on Windows.

jasnell · 2017-08-24T04:54:18Z

Marking this a being blocked on libuv

uv_if_indextoname() is used to convert an IPv6 scope_id to an interface identifier string such as %eth0 or %lo. uv_if_indextoiid() returns an IPv6 interface identifier. On Unix it calls uv_if_indextoname(). On Windows it uses snprintf() to return the numeric interface identifier as a string. Refs: nodejs/node#14500 PR-URL: #1445 Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

pekkanikander · 2017-11-08T11:52:02Z

Manually updated, with the the required changes from upstream libuv 1.16.0.

cjihrig · 2017-11-10T18:54:18Z

@pekkanikander please remove the libuv commit and rebase this PR. libuv 1.16.1 is now on master.

pekkanikander · 2017-11-10T19:40:38Z

Updated.

pekkanikander · 2017-11-13T09:03:13Z

@bnoordhuis Ben, any idea when you will have time to review this PR? I have some time to handle review comments this week, but next week not. Next week I will be travelling with a quite busy schedule.

lpinca · 2020-07-14T13:59:00Z

@sxa yes it makes sense. I think that fix should go in a separate PR. Feel free to open it. We can then rebase this on top of it.

Zone IDs on Linux are network interface names. The regex we use to determine valid IPs does not allow for non-alphanumeric characters in the zone ID suffix. Some machines (including the RHEL Linux/s390x machines from Marist) have zone IDs with a '.' character in them which the regex in net.isIP rejects. This changes the regex. Ref: nodejs#14500 Signed-off-by: Stewart Addison <sxa@uk.ibm.com>

Zone IDs on Linux are network interface names. The regex we use to determine valid IPs does not allow for non-alphanumeric characters in the zone ID suffix. Some machines (including the RHEL Linux/s390x machines from Marist) have zone IDs with a '.' character in them which the regex in net.isIP rejects. This changes the regex. Ref: #14500 Signed-off-by: Stewart Addison <sxa@uk.ibm.com> PR-URL: #34364 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

nodejs-github-bot · 2020-07-20T11:27:49Z

CI: https://ci.nodejs.org/job/node-test-pull-request/32409/

sxa · 2020-07-20T12:51:00Z

The new test passes on LinuxOne after #34364 landed. @Trott do you still have a concern over this given that node-test-commit-windows-fanned looks ok? Everything finally looks green here - maybe we'll get this in before the three year mark :-D

Trott · 2020-07-21T02:47:59Z

@Trott do you still have a concern over this given that node-test-commit-windows-fanned looks ok?

Yes. It's very suspicious that no tests have failed. I'm concerned.

No, just kidding, OMG, yes, let's land this thing.

tests passing

lpinca · 2020-07-21T05:11:27Z

Do it!

sxa

Last push was by @Trott and the landing tool is objecting that there have been no more formal approvals since the last push, so I'm going to approve Trott's changes from a few weeks back and let this land :-D

Add IPv6 link local scope ID suffix to the rinfo address in those received upd6 datagrams whose source address is a link local address. Add a new test case, test-dgram-udp6-link-local-address, to verify that IPv6 UDP datagrams received from a link-local source address do contain the scope ID suffix in the rinfo address field. When a packet is received from a link-local source address, if the address does not contain the scope ID suffix, it is impossible to reply back to the sender, as the kernel is not able to determine the right network interface to send the packet through and returns with an error. Ref: #1649 PR-URL: #14500 Refs: #1649 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Stewart X Addison <sxa@uk.ibm.com>

sxa · 2020-07-21T11:02:41Z

Landed in bdf6827 - thanks everyone - we got this PR in five days before it got to it's third birthday 😅 🍾

pekkanikander · 2020-07-21T11:25:44Z

A big thank you 🙏🙏 for your perseverance and efforts on getting this humble fix finally landed!

Zone IDs on Linux are network interface names. The regex we use to determine valid IPs does not allow for non-alphanumeric characters in the zone ID suffix. Some machines (including the RHEL Linux/s390x machines from Marist) have zone IDs with a '.' character in them which the regex in net.isIP rejects. This changes the regex. Ref: #14500 Signed-off-by: Stewart Addison <sxa@uk.ibm.com> PR-URL: #34364 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Add IPv6 link local scope ID suffix to the rinfo address in those received upd6 datagrams whose source address is a link local address. Add a new test case, test-dgram-udp6-link-local-address, to verify that IPv6 UDP datagrams received from a link-local source address do contain the scope ID suffix in the rinfo address field. When a packet is received from a link-local source address, if the address does not contain the scope ID suffix, it is impossible to reply back to the sender, as the kernel is not able to determine the right network interface to send the packet through and returns with an error. Ref: #1649 PR-URL: #14500 Refs: #1649 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Stewart X Addison <sxa@uk.ibm.com>

Zone IDs on Linux are network interface names. The regex we use to determine valid IPs does not allow for non-alphanumeric characters in the zone ID suffix. Some machines (including the RHEL Linux/s390x machines from Marist) have zone IDs with a '.' character in them which the regex in net.isIP rejects. This changes the regex. Ref: #14500 Signed-off-by: Stewart Addison <sxa@uk.ibm.com> PR-URL: #34364 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Add IPv6 link local scope ID suffix to the rinfo address in those received upd6 datagrams whose source address is a link local address. Add a new test case, test-dgram-udp6-link-local-address, to verify that IPv6 UDP datagrams received from a link-local source address do contain the scope ID suffix in the rinfo address field. When a packet is received from a link-local source address, if the address does not contain the scope ID suffix, it is impossible to reply back to the sender, as the kernel is not able to determine the right network interface to send the packet through and returns with an error. Ref: #1649 PR-URL: #14500 Refs: #1649 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Stewart X Addison <sxa@uk.ibm.com>

Notable changes: dgram: * (SEMVER-MINOR) add IPv6 scope id suffix to received udp6 dgrams (Pekka Nikander) #14500 doc: * add AshCripps to collaborators (AshCripps) #34494 * add HarshithaKP to collaborators (Harshitha K P) #34417 * add rexagod to collaborators (Pranshu Srivastava) #34457 * add release key for Richard Lau (Richard Lau) #34397 events: * (SEMVER-MINOR) expand NodeEventTarget functionality (Anna Henningsen) #34057 src: * (SEMVER-MINOR) allow preventing SetPromiseRejectCallback (Shelley Vohr) #34387 * (SEMVER-MINOR) allow setting a dir for all diagnostic output (AshCripps) #33584 worker: * (SEMVER-MINOR) make MessagePort inherit from EventTarget (Anna Henningsen) #34057 PR-URL: TODO

Jeewes · 2020-08-12T18:58:53Z

This has been a joy to follow. Good job people. 👍

nodejs-github-bot added the test Issues and PRs related to the tests. label Jul 26, 2017

pekkanikander force-pushed the feature-udp6-received-link-local-address-scope-id branch from f53d11e to 842bd12 Compare July 26, 2017 12:03

pekkanikander force-pushed the feature-udp6-received-link-local-address-scope-id branch from 47e6c7b to f0e506c Compare July 26, 2017 12:21

pekkanikander changed the title ~~test: add test-dgram-udp6-link-local-address~~ test/dgram: Add support for IPv6 link local scope IDs in received UDP datagrams Jul 26, 2017

pekkanikander force-pushed the feature-udp6-received-link-local-address-scope-id branch from f0e506c to 79392ac Compare July 26, 2017 14:57

mscdex added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Jul 26, 2017

This was referenced Jul 27, 2017

unix: add if_indextoname libuv/libuv#1443

Closed

add uv_if_indextoname and uv_if_indextoiid libuv/libuv#1445

Closed

jasnell requested a review from bnoordhuis August 8, 2017 23:37

jasnell added the blocked PRs that are blocked by other issues or PRs. label Aug 24, 2017

jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Aug 24, 2017

pekkanikander force-pushed the feature-udp6-received-link-local-address-scope-id branch from 79392ac to 2ccca4c Compare November 8, 2017 11:50

pekkanikander force-pushed the feature-udp6-received-link-local-address-scope-id branch from 2ccca4c to de58e96 Compare November 10, 2017 19:39

sxa mentioned this pull request Jul 14, 2020

net: allow wider regex in interface name #34364

Closed

3 tasks

lpinca removed the stalled Issues and PRs that are stalled. label Jul 21, 2020

sxa approved these changes Jul 21, 2020

View reviewed changes

sxa closed this Jul 21, 2020

ruyadorno mentioned this pull request Jul 28, 2020

v14.7.0 proposal #34542

Merged

codebytere mentioned this pull request Sep 28, 2020

v12.19.0 proposal #35401

Merged

mhdawson mentioned this pull request Jan 18, 2022

pummel/test-worker-take-heapsnapshot - crash in debug mode on arm #41204

Closed

Uh oh!

Conversation

pekkanikander commented Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Uh oh!

mscdex commented Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pekkanikander commented Jul 26, 2017

Uh oh!

mscdex commented Jul 26, 2017

Uh oh!

bzoz commented Aug 16, 2017

Uh oh!

jasnell commented Aug 24, 2017

Uh oh!

pekkanikander commented Nov 8, 2017

Uh oh!

cjihrig commented Nov 10, 2017

Uh oh!

pekkanikander commented Nov 10, 2017

Uh oh!

pekkanikander commented Nov 13, 2017

Uh oh!

lpinca commented Jul 14, 2020

Uh oh!

nodejs-github-bot commented Jul 20, 2020

Uh oh!

sxa commented Jul 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Trott commented Jul 21, 2020

Uh oh!

lpinca commented Jul 21, 2020

Uh oh!

sxa left a comment

Choose a reason for hiding this comment

Uh oh!

sxa commented Jul 21, 2020

Uh oh!

pekkanikander commented Jul 21, 2020

Uh oh!

Jeewes commented Aug 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

pekkanikander commented Jul 26, 2017 •

edited

Loading

mscdex commented Jul 26, 2017 •

edited

Loading

sxa commented Jul 20, 2020 •

edited

Loading

Jeewes commented Aug 12, 2020 •

edited

Loading