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

fail fast and display hint when --first_ttl=x --max_ttl=y and x>y #521

Open
gberche-orange opened this issue Feb 4, 2025 · 16 comments
Open

Comments

@gberche-orange
Copy link

Following thread at #505 (comment) thanks to @rewolff explaining behavior of mtr for tcp protocol below

< to really care about the RTT to his intended destination to compare results with ICMP vs TCP [...]
> You should be able to get such behaviour from the commandline by setting min-ttl and max-ttl to values that are very close (not sure if they need to differ by 0 or 1) and sufficiently large.

When specifying --first-ttl 90 --max-ttl 90 --tcp ..., I'm surprised to see that the single reported host in the hubs section (which is also the final destination) has an associated count=30 in the json report, and the interactive display.

sample json report output with --first-ttl 90 --max-ttl 90
# mtr  -j -4 -c 100 --tcp -P 443 --timeout 2 --gracetime 2 --no-dns --first-ttl 90 --max-ttl 90 myhost.my-redacted-domain.org 
{
    "report": {
        "mtr": {
            "src": "mtr-exporter-29",
            "dst": "myhost.my-redacted-domain.org",
            "tos": 0,
            "tests": 100,
            "psize": "64",
            "bitpattern": "0x00"
        },
        "hubs": [
            {
                "count": 30,
                "host": "192.168.25.3",
                "Loss%": 0.0,
                "Snt": 100,
                "Last": 2.793,
                "Avg": 2.535,
                "Best": 1.85,
                "Wrst": 8.97,
                "StDev": 1.041
            }
        ]
    }
}

How is computed this 30 value ?
I suspect this comes from the 30 default value.

mtr/man/mtr.8.in

Lines 420 to 422 in c68adc1

.B \-m \fINUM\fR, \fB\-\-max-ttl \fINUM
Specifies the maximum number of hops (max time-to-live value) traceroute will
probe. Default is 30.

Shouldn't instead the count be assigned the specified value by the -min--ttl flag (which is equal to --max-ttl flag) instead ?

If I understand correctly, count in hubsrepresents the number of index of the intermediate gateway discovered before reaching the destination

sample json report output with --first-ttl 1 --max-ttl 90

{                                                                                                                                                                                          
    "report": {            
        "mtr": {                         
            "src": "mtr-exporter-31",                 
            "dst": "myhost2.my-redacted-domain.org",
            "tos": 0,
            "tests": 10,
            "psize": "64",
            "bitpattern": "0x00"
        },
        "hubs": [
            {
                "count": 1,
                "host": "???",
                "Loss%": 100.0,
                "Snt": 10,
                "Last": 0.0,
                "Avg": 0.0,
                "Best": 0.0,
                "Wrst": 0.0,
                "StDev": 0.0
            },
            {
                "count": 2,
                "host": "10.16.38.143",
                "Loss%": 0.0,
                "Snt": 10,
                "Last": 0.241,
                "Avg": 0.191,
                "Best": 0.128,
                "Wrst": 0.249,
                "StDev": 0.034
            },
[...]
            {
                "count": 7,
                "host": "192.168.61.32",
                "Loss%": 0.0,
                "Snt": 10,
                "Last": 1.857,
                "Avg": 3.454,
                "Best": 1.857,
                "Wrst": 7.712,
                "StDev": 1.839
            } 
        ]    
    }                      
@rewolff
Copy link
Collaborator

rewolff commented Feb 4, 2025

mtr should report "actual measurements" as much as possible, and not try to second-guess what you're trying to do or want to see.

The --minttl and --maxttl options seem to work. I can "chop" part of a route from the results with them.

Similarly it behaves as you expect when I specify a min and max between actual hops and 30.

This would indicate to me that the OS might be preventing us from sending packets with the TTL set to 90....

@gberche-orange
Copy link
Author

Thanks for your prompt response !

This would indicate to me that the OS might be preventing us from sending packets with the TTL set to 90....

Trying on my system to set the ttl for the nc command to assign ttl and inspect it using tcpdump and tshark confirms that the OS is properly sending the traffic with IPV4 ttl higher than 30 (90).

nc --help
#> nc: invalid option -- '-'
#> usage: nc [-46CDdFhklNnrStUuvZz] [-I length] [-i interval] [-M ttl]
#>           [-m minttl] [-O length] [-P proxy_username] [-p source_port]
#>           [-q seconds] [-s sourceaddr] [-T keyword] [-V rtable] [-W recvlimit]
#>           [-w timeout] [-X proxy_protocol] [-x proxy_address[:port]]
#>           [destination] [port]

man nc
#>     -M ttl  Set the TTL / hop limit of outgoing packets.
#>
#>     -m minttl
#>             Ask the kernel to drop incoming packets whose TTL / hop limit is under minttl.

sudo tcpdump -n -i any host 192.168.25.3 -w /tmp/nc.pcap 

nc  -M 90  -v myhost.my-redacted-domain.org443
#> Connection to myhost.my-redacted-domain.org(192.168.25.3) 443 port [tcp/https] succeeded!

tshark -V -r /tmp/nc.pcap | grep "Time to Live"
#>    Time to Live: 90
#>    Time to Live: 49
#>    Time to Live: 90
#>    Time to Live: 90
#>    Time to Live: 49
#>    Time to Live: 49
#>    Time to Live: 64
#>    Time to Live: 64

Surprisingly, I don't observe TTL higher than 30 for mtr

mtr -v
#> mtr 0.95

tcpdump -n -i any host 192.168.25.3 -w /tmp/mtr.pcap &

mtr -j --first-ttl 90 --max-ttl 90  myhost.my-redacted-domain.org:443
#> {
#>     "report": {
#>         "mtr": {
#>             "src": "42d4442c-6276-4f04-b496-9638245cd4e4",
#>             "dst": "myhost.my-redacted-domain.org",
#>             "tos": 0,
#>             "tests": 10,
#>             "psize": "64",
#>             "bitpattern": "0x00"
#>         },
#>         "hubs": [
#>             {
#>                 "count": 30,
#>                 "host": "myhost.my-redacted-domain.org",
#>                 "Loss%": 0.0,
#>                 "Snt": 10,
#>                 "Last": 2.429,
#>                 "Avg": 2.578,
#>                 "Best": 2.148,
#>                 "Wrst": 3.566,
#>                 "StDev": 0.528
#>             }
#>         ]
#>     }
#> }


tshark -V -r /tmp/mtr.pcap | grep "Time to Live"
#>    Time to Live: 30
#>    Time to Live: 49
#>    Time to Live: 31
#>    Time to Live: 49
#>    Time to Live: 30
#>    Time to Live: 49
#>    Time to Live: 30
#>    Time to Live: 49
#>    Time to Live: 30
#>    Time to Live: 49
#>    Time to Live: 30
#>    Time to Live: 49
#>    Time to Live: 30
#>    Time to Live: 49
#>    Time to Live: 30
#>    Time to Live: 49
#>    Time to Live: 30
#>    Time to Live: 49
#>    Time to Live: 30
#>    Time to Live: 49
#>    Time to Live: 30
#>    Time to Live: 49

@rewolff
Copy link
Collaborator

rewolff commented Feb 4, 2025

Haha! Found it!

You cannot set first ttl larger than last(max) ttl. Duh! This is checked at the time of parsing the arguments.

So first set max to 90 THEN set first to 90 as well. Works! Checked!

If you wish to get just one probe depth, then set max to the desired probe depth and then set min to 1000.... less variables involved.

@gberche-orange
Copy link
Author

gberche-orange commented Feb 4, 2025

Thanks a lot ! Inverting the two options , mtr indeed preserves the request TTL as shown in tcpdump captures, and properly returns the count to the assigned TTL for the outgoing tcp connection (not the one received from the response tcp packets, which are likely hidden by the posix interface that mtr uses to open the tcp connection)

Sample output with --max-ttl 90 --first-ttl 1000
tcpdump -n -i any host 10.118.43.212 -w /tmp/mtr.pcap &

# mtr -j --max-ttl 90 --first-ttl 1000  myhost.my-redacted-domain.org 443
#{                                                                                                        
#    "report": {     
#        "mtr": {    
#            "src": "42d4442c-6276-4f04-b496-9638245cd4e4",
#            "dst": "myhost.my-redacted-domain.org",
#            "tos": 0,
#            "tests": 10,
#            "psize": "64",
#            "bitpattern": "0x00"
#        },          
#        "hubs": [   
#            {       
#                "count": 90,
#                "host": "myhost.my-redacted-domain.org",
#                "Loss%": 0.0,
#                "Snt": 10,
#                "Last": 2.22,
#                "Avg": 2.553,
#                "Best": 2.182,
#                "Wrst": 3.834,
#                "StDev": 0.492
#            }
            
tshark -V -r /tmp/mtr.pcap | grep "Time to Live"                                                                 
#    Time to Live: 90
#    Time to Live: 49
#    Time to Live: 90                                                                                     
#    Time to Live: 49                                                                                     
#    Time to Live: 90 
#    Time to Live: 49    
#    Time to Live: 90      
#    Time to Live: 49            
#    Time to Live: 90
#    Time to Live: 49
#    Time to Live: 90
#    Time to Live: 49        
#    Time to Live: 90                                                                                                             
#    Time to Live: 49         
#    Time to Live: 90      
#    Time to Live: 49         
#    Time to Live: 90         
#    Time to Live: 49          
#    Time to Live: 90          
#    Time to Live: 49          

Would it make sense to have mtr fail-fast upon receiving apparently conflicting options, and possibly hinting about the flag order for when it is intended that first-ttl >= max_ttl ?

mtr/ui/mtr.c

Lines 478 to 486 in c68adc1

case 'f':
ctl->fstTTL =
strtonum_or_err(optarg, "invalid argument", STRTO_INT);
if (ctl->fstTTL > ctl->maxTTL) {
ctl->fstTTL = ctl->maxTTL;
}
if (ctl->fstTTL < 1) { /* prevent 0 hop */
ctl->fstTTL = 1;
}

@gberche-orange gberche-orange changed the title incorrect reported count=30 when first_ttl=max_ttl=<large-value> fail fast and display hint when --first_ttl=x --max_ttl=y and x>y Feb 4, 2025
@rewolff
Copy link
Collaborator

rewolff commented Feb 4, 2025

How about: "Warning: unable to set first-ttl to %d limited to max-ttl=%d. "

On the other hand. Just moving the check to AFTER all arguments have been parsed is probably better. And maybe we should bomb out if you set them "wrong". On the other hand, someone might have stumbled on the "funny" behavior and now depend on it....

@rewolff
Copy link
Collaborator

rewolff commented Feb 4, 2025

How is this?

abra2:~/mtr> ./mtr --first-ttl 91 --max-ttl 90 hydra
./mtr: firstTTL(91) cannot be larger than maxTTL(90). 

@gberche-orange
Copy link
Author

How is this?

abra2:~/mtr> ./mtr --first-ttl 91 --max-ttl 90 hydra
./mtr: firstTTL(91) cannot be larger than maxTTL(90). 

This seems right to me, thanks !

@gberche-orange
Copy link
Author

What happens with --first-ttl=91 --max-ttl=91 in you upcoming version ?

@rewolff
Copy link
Collaborator

rewolff commented Feb 4, 2025 via email

@rewolff
Copy link
Collaborator

rewolff commented Feb 4, 2025

I used -r this time as it shows exactly the same behavior as the json output. First is old second is new otherwise same commandline.

 30.|-- www.bitwizard.nl           0.0%     1   14.2  14.2  14.2  14.2   0.0
 90.|-- www.bitwizard.nl           0.0%     1   14.2  14.2  14.2  14.2   0.0

@gberche-orange
Copy link
Author

Looks great, thanks! Are the related code changes already available on github ?

@rewolff
Copy link
Collaborator

rewolff commented Feb 5, 2025 via email

@gberche-orange
Copy link
Author

thanks for 7a8f545 !

@gberche-orange
Copy link
Author

BTW, I noticed the tests are failing fot this commit see https://github.com/traviscross/mtr/actions/runs/13157461888/job/36717793013, is it unrelated false detection or a regression ?

@rewolff
Copy link
Collaborator

rewolff commented Feb 6, 2025

There was a previous issue, it seems someone added tests that require "root" to run (mtr sends funky packets which require root to submit to the networking stack), which of course on github isn't going to fly.

@yvs2014
Copy link

yvs2014 commented Feb 6, 2025

on github isn't going to fly

Hi everyone,

it runs inside of VM in scopes of which its privileges are granted. I.e. something like sudo mtr -r localhost is supposed to work in these sandboxed scopes, isn't it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants