Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

traceroute6: remove -l flag #1023

Closed
wants to merge 1 commit into from
Closed

traceroute6: remove -l flag #1023

wants to merge 1 commit into from

Conversation

llfw
Copy link
Contributor

@llfw llfw commented Jan 11, 2024

The -l flag was used to tell traceroute6(8) to show both hostname and address for each hop. However, traceroute(8) already does this by default, and there's no reason for traceroute6 to behave differently.

Make this the default behaviour, and accept -l for backward compatibility as a no-op flag.

@llfw
Copy link
Contributor Author

llfw commented Jan 11, 2024

someone suggested this behaviour might be because IPv6 addresses are longer, however in practice, there's basically no difference:

[1? 7!] lexi@hemlock ~
% traceroute6 -Il bbc.co.uk
traceroute6: Warning: bbc.co.uk has multiple addresses; using 2a04:4e42:400::81
traceroute6 to bbc.co.uk (2a04:4e42:400::81) from 2001:8b0:aab5:106::12, 64 hops max, 20 byte packets
 1  VLAN106.CSW1.EDEN.LE-FAY.ORG (2001:8b0:aab5:106::1)  0.292 ms  0.225 ms  0.200 ms
 2  SFP1.CR1.EDEN.LE-FAY.ORG (2001:8b0:aab5:fffd::2)  0.263 ms  0.217 ms  0.191 ms
 3  y.witless.thn.aa.net.uk (2001:8b0:0:53::134)  6.176 ms  6.237 ms  6.462 ms
 4  o.aimless.tch.aa.net.uk (2001:8b0:0:53::105)  5.795 ms  6.751 ms  6.528 ms
 5  2001:7f8:4::d361:1 (2001:7f8:4::d361:1)  6.680 ms  7.126 ms  6.844 ms
 6  2a04:4e42:400::81 (2a04:4e42:400::81)  6.636 ms  6.884 ms  6.956 ms

[8!] lexi@hemlock ~
% traceroute -I bbc.co.uk
traceroute: Warning: bbc.co.uk has multiple addresses; using 151.101.0.81
traceroute to bbc.co.uk (151.101.0.81), 64 hops max, 48 byte packets
 1  VLAN106.CSW2.EDEN.LE-FAY.ORG (10.1.6.1)  0.268 ms  0.181 ms  0.155 ms
 2  SFP1.CR1.EDEN.LE-FAY.ORG (10.255.0.6)  0.191 ms  0.176 ms  0.176 ms
 3  y.witless.thn.aa.net.uk (90.155.53.134)  5.985 ms  5.591 ms  6.007 ms
 4  o.aimless.tch.aa.net.uk (90.155.53.105)  6.103 ms  5.742 ms  5.804 ms
 5  195.66.225.91 (195.66.225.91)  6.257 ms  6.631 ms  6.391 ms
 6  151.101.0.81 (151.101.0.81)  6.179 ms  6.237 ms  6.299 ms

the only long line of output from traceroute6 is because it prints the source address by default.

@jlduran
Copy link
Contributor

jlduran commented Jan 11, 2024

Nice catch!
I would have attempted to fix it first. Take a look at netstat(1)'s same (and working) inetname() implementation:

/*
* Construct an Internet address representation.
* If the nflag has been supplied, give
* numeric value, otherwise try for symbolic name.
*/
static char *
inetname(struct sockaddr *sa)
{
char *cp = 0;
static char line[NI_MAXHOST];
struct hostent *hp;
struct in_addr in;
#ifdef INET6
if (sa->sa_family == AF_INET6) {
if (memcmp(&((struct sockaddr_in6 *)sa)->sin6_addr,
&in6addr_any, sizeof(in6addr_any)) == 0)
strcpy(line, "*");
else
getnameinfo(sa, sa->sa_len, line, sizeof(line), NULL, 0,
nflag ? NI_NUMERICHOST : 0);
return (line);
}
#endif
in = ((struct sockaddr_in *)sa)->sin_addr;
if (!nflag && in.s_addr != INADDR_ANY) {
hp = gethostbyaddr((char *)&in, sizeof (in), AF_INET);
if (hp)
cp = hp->h_name;
}
if (in.s_addr == INADDR_ANY)
strcpy(line, "*");
else if (cp)
snprintf(line, sizeof(line), "%s", cp);
else {
in.s_addr = ntohl(in.s_addr);
#define C(x) ((x) & 0xff)
snprintf(line, sizeof(line), "%u.%u.%u.%u", C(in.s_addr >> 24),
C(in.s_addr >> 16), C(in.s_addr >> 8), C(in.s_addr));
}
return (line);
}

Given the time, and the fact that nobody noticed it, removing it could also be another option, although, more cleanup is needed.

EDIT: I have tested traceroute6(8), and it works as expected:

$ traceroute6 www.freebsd.org
traceroute6 to web.geo.freebsd.org (2610:1c1:1:606c::50:2) from 2600:1f18:2533:2000:0000:0000:0bad:cafe, 64 hops max, 28 byte packets
 1  2620:107:4000:2210:8000:0:3ec:3d17  0.733 ms
    2620:107:4000:2210:8000:0:f405:4c3  16.000 ms
    2620:107:4000:2210:8000:0:3ec:3d21  12.197 ms
 2  2620:107:4000:d652::f003:8c45  0.388 ms
    2620:107:4000:cf52::f003:1043  0.326 ms
    2620:107:4000:a753::f000:3462  0.333 ms
 3  2620:107:4000:cfff::f202:7841  0.877 ms
    2620:107:4000:cfff::f209:a217  1.147 ms
    2620:107:4000:cfff::f207:7141  0.848 ms
 4  2620:107:4000:c5c0::f3fd:3  0.560 ms  0.519 ms
    2620:107:4000:c5c0::f3fd:2  0.643 ms
 5  2620:107:4000:cfff::f202:d4c3  1.082 ms  0.868 ms
    2620:107:4000:cfff::f202:d4c5  0.964 ms
 6  2620:107:4000:8001::21  0.889 ms  0.552 ms  0.581 ms
 7  2620:107:4008:b986::2  0.670 ms  0.662 ms  0.672 ms
 8  ae5.mpr1.ewr4.us.zip.zayo.com  7.696 ms  12.175 ms  14.540 ms
 9  2001:438:fffe::24ba  7.915 ms  7.671 ms  7.521 ms
10  cs89-cs80.nyinternet.net  7.809 ms  7.408 ms  7.442 ms
11  2610:1c1::803  9.646 ms  7.789 ms  7.686 ms
12  wfe0-main.nyi.freebsd.org  7.489 ms  7.831 ms  7.482 ms
$ traceroute6 -l www.freebsd.org
traceroute6 to web.geo.freebsd.org (2610:1c1:1:606c::50:2) from 2600:1f18:2533:2000:0000:0000:0bad:cafe, 64 hops max, 28 byte packets
 1  2620:107:4000:2210:8000:0:3ec:3cdd (2620:107:4000:2210:8000:0:3ec:3cdd)  0.718 ms
    2620:107:4000:2210:8000:0:3ec:3ce5 (2620:107:4000:2210:8000:0:3ec:3ce5)  9.357 ms
    2620:107:4000:2210:8000:0:f405:395 (2620:107:4000:2210:8000:0:f405:395)  4.991 ms
 2  2620:107:4000:cf53::f003:1061 (2620:107:4000:cf53::f003:1061)  0.320 ms
    2620:107:4000:e111::f004:5828 (2620:107:4000:e111::f004:5828)  0.308 ms
    2620:107:4000:cfff::f200:d0f0 (2620:107:4000:cfff::f200:d0f0)  0.424 ms
 3  2620:107:4000:cfff::f20b:ca87 (2620:107:4000:cfff::f20b:ca87)  1.056 ms
    2620:107:4000:cfff::f209:a391 (2620:107:4000:cfff::f209:a391)  0.513 ms
    2620:107:4000:cfff::f200:cad1 (2620:107:4000:cfff::f200:cad1)  1.117 ms
 4  2620:107:4000:c5c0::f3fd:1 (2620:107:4000:c5c0::f3fd:1)  0.986 ms
    2620:107:4000:c5c0::f3fd:3 (2620:107:4000:c5c0::f3fd:3)  0.552 ms  0.513 ms
 5  2620:107:4000:cfff::f202:d447 (2620:107:4000:cfff::f202:d447)  0.977 ms
    2620:107:4000:cfff::f202:d4c3 (2620:107:4000:cfff::f202:d4c3)  1.458 ms
    2620:107:4000:cfff::f202:d5c5 (2620:107:4000:cfff::f202:d5c5)  1.189 ms
 6  2620:107:4000:8001::21 (2620:107:4000:8001::21)  0.661 ms  0.748 ms  0.702 ms
 7  2620:107:4008:b986::2 (2620:107:4008:b986::2)  0.685 ms  0.669 ms  0.683 ms
 8  ae5.mpr1.ewr4.us.zip.zayo.com (2001:438:ffff::407d:1feb)  13.750 ms  7.914 ms  8.224 ms
 9  2001:438:fffe::24ba (2001:438:fffe::24ba)  7.461 ms  7.817 ms  7.476 ms
10  cs89-cs80.nyinternet.net (2610:1c1::2502)  7.578 ms  7.485 ms  10.033 ms
11  2610:1c1::803 (2610:1c1::803)  7.447 ms  7.564 ms  7.648 ms
12  wfe0-main.nyi.freebsd.org (2610:1c1:1:606c::50:2)  7.591 ms  7.857 ms  7.816 ms
$ traceroute6 -n www.freebsd.org
traceroute6 to web.geo.freebsd.org (2610:1c1:1:606c::50:2) from 2600:1f18:2533:2000:0000:0000:0bad:cafe, 64 hops max, 28 byte packets
 1  2620:107:4000:2210:8000:0:3ec:3df9  0.692 ms
    2620:107:4000:2210:8000:0:f405:4f7  0.701 ms
    2620:107:4000:2210:8000:0:3ec:3d35  0.833 ms
 2  2620:107:4000:cfff::f200:d0e8  0.465 ms
    2620:107:4000:e111::f004:5827  0.350 ms
    2620:107:4000:cf53::f003:1063  0.346 ms
 3  2620:107:4000:cfff::f20b:ca07  0.799 ms
    2620:107:4000:cfff::f20b:ca85  1.286 ms
    2620:107:4000:cfff::f209:a297  0.712 ms
 4  2620:107:4000:c5c0::f3fd:0  0.600 ms
    2620:107:4000:c5c0::f3fd:3  0.626 ms
    2620:107:4000:c5c0::f3fd:0  0.558 ms
 5  2620:107:4000:cfff::f202:d4c3  1.363 ms  0.839 ms
    2620:107:4000:cfff::f202:d5c5  0.482 ms
 6  2620:107:4000:8001::21  0.976 ms  1.132 ms  1.326 ms
 7  2620:107:4008:b986::2  1.500 ms  0.761 ms  0.736 ms
 8  2001:438:ffff::407d:1feb  7.706 ms  7.519 ms  7.624 ms
 9  2001:438:fffe::24ba  7.469 ms  7.426 ms  7.492 ms
10  2610:1c1::2502  7.836 ms  8.675 ms  7.523 ms
11  2610:1c1::803  7.487 ms  7.599 ms  7.628 ms
12  2610:1c1:1:606c::50:2  7.724 ms  7.863 ms  7.631 ms

@jlduran
Copy link
Contributor

jlduran commented Jan 11, 2024

Sorry for the noise, originally I had misinterpreted the pull request.
I like the idea of unifying the output of The KAME project network utilities with the original BSD ones, or even better, unifying common code.

@llfw
Copy link
Contributor Author

llfw commented Jan 12, 2024

i assume the eventual plan is to merge traceroute6 into traceroute, as was done with ping, but that's obviously a much larger project :-) in the mean time, removing behaviour differences seems useful to reduce the amount of future work needed for that.

@bsdimp
Copy link
Member

bsdimp commented Jan 29, 2024

This looks sane to me as well.
I'll see if it still applies.
I'll also add a date bump to the man page (hmmm, maybe we should script that)

@bsdimp bsdimp self-assigned this Jan 29, 2024
@bsdimp bsdimp added the ready label Jan 29, 2024
@jlduran
Copy link
Contributor

jlduran commented Jan 29, 2024

There is also a mention to traceroute6 -l in share/examples/IPv6/USAGE, this is a very outdated document (historical?). Perhaps it should be updated as well .

@bsdimp
Copy link
Member

bsdimp commented Jan 30, 2024

There is also a mention to traceroute6 -l in share/examples/IPv6/USAGE, this is a very outdated document (historical?). Perhaps it should be updated as well .

The document hasn't meaningfully changed since 2002. I think updating it for this might be misguided effort. Maybe we should just retire it, though the network folks can say better than I. I kinda think that it is orthogonal to this issue. I don't know ipv6 well enough to know what else in the doc is dated.

@llfw
Copy link
Contributor Author

llfw commented Jan 30, 2024

that document spends a lot of time talking about a prefix command that doesn't seem to exist anymore. rather than updating it, it might be more sensible to remove it in favour of the Handbook, which already explains most of this and is more up-to-date.

The -l flag was used to tell traceroute6(8) to show both hostname and
address for each hop.  However, traceroute(8) already does this by
default, and there's no reason for traceroute6 to behave differently.

Make this the default behaviour, and accept -l for backward
compatibility as a no-op flag.
@llfw
Copy link
Contributor Author

llfw commented Jan 31, 2024

i pushed a rebase (no conflicts) and the .Dd update (this would be a handy thing to check in a git-commit hook, if we have any of those?).

@bsdimp
Copy link
Member

bsdimp commented Feb 2, 2024

Pushed in as 0a49be7

@bsdimp bsdimp closed this Feb 2, 2024
@bsdimp bsdimp added merged and removed ready labels Feb 2, 2024
freebsd-git pushed a commit that referenced this pull request Feb 2, 2024
The -l flag was used to tell traceroute6(8) to show both hostname and
address for each hop.  However, traceroute(8) already does this by
default, and there's no reason for traceroute6 to behave differently.

Make this the default behaviour, and accept -l for backward
compatibility as a no-op flag.

Reviewed by: imp
Pull Request: #1023
@llfw llfw deleted the traceroute-l branch February 3, 2024 21:00
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Aug 1, 2024
The -l flag was used to tell traceroute6(8) to show both hostname and
address for each hop.  However, traceroute(8) already does this by
default, and there's no reason for traceroute6 to behave differently.

Make this the default behaviour, and accept -l for backward
compatibility as a no-op flag.

Reviewed by: imp
Pull Request: freebsd/freebsd-src#1023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants