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

PING: T3634: Fixing do not fragment to Ping #956

Merged
merged 1 commit into from Aug 8, 2021
Merged

PING: T3634: Fixing do not fragment to Ping #956

merged 1 commit into from Aug 8, 2021

Conversation

Cheeze-It
Copy link
Contributor

@Cheeze-It Cheeze-It commented Aug 7, 2021

In this commit we fix the do not fragment capability
for ping commands. Sorry for messing it up earlier :(

Change Summary

Just a small bugfix on the do not fragment command. I misread the man
page for ping on it. I fixed it now.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Component(s) name

ping

Proposed changes

Not much to describe here. Just fixed the command in linux ping.

How to test

Please describe in detail how you tested your changes. Include details of your testing
environment, and the tests you ran. When pasting configs, logs, shell output, backtraces,
and other large chunks of text, surround this text with triple backtics

Normal:
sudo ip vrf exec default /bin/ping 10.0.0.65
PING 10.0.0.65 (10.0.0.65) 56(84) bytes of data.
64 bytes from 10.0.0.65: icmp_seq=1 ttl=64 time=0.109 ms
64 bytes from 10.0.0.65: icmp_seq=2 ttl=64 time=0.531 ms
64 bytes from 10.0.0.65: icmp_seq=3 ttl=64 time=0.173 ms
64 bytes from 10.0.0.65: icmp_seq=4 ttl=64 time=0.184 ms
64 bytes from 10.0.0.65: icmp_seq=5 ttl=64 time=0.207 ms
64 bytes from 10.0.0.65: icmp_seq=6 ttl=64 time=0.177 ms
64 bytes from 10.0.0.65: icmp_seq=7 ttl=64 time=0.192 ms
^C
--- 10.0.0.65 ping statistics ---
7 packets transmitted, 7 received, 0% packet loss, time 6173ms



Do not fragment:
vyos@vyos:~$ ping 10.0.0.65 do-not-fragment
sudo ip vrf exec default /bin/ping -M do 10.0.0.65
PING 10.0.0.65 (10.0.0.65) 56(84) bytes of data.
64 bytes from 10.0.0.65: icmp_seq=1 ttl=64 time=0.091 ms
64 bytes from 10.0.0.65: icmp_seq=2 ttl=64 time=0.171 ms
64 bytes from 10.0.0.65: icmp_seq=3 ttl=64 time=0.151 ms
64 bytes from 10.0.0.65: icmp_seq=4 ttl=64 time=0.194 ms
64 bytes from 10.0.0.65: icmp_seq=5 ttl=64 time=0.166 ms
64 bytes from 10.0.0.65: icmp_seq=6 ttl=64 time=0.181 ms
^C
--- 10.0.0.65 ping statistics ---
6 packets transmitted, 6 received, 0% packet loss, time 5128ms
rtt min/avg/max/mdev = 0.091/0.159/0.194/0.033 ms



Normal, large payload, but it still seems to just fail out:
vyos@vyos:~$ ping 10.0.0.65 size 1473
sudo ip vrf exec default /bin/ping -s 1473 10.0.0.65
PING 10.0.0.65 (10.0.0.65) 1473(1501) bytes of data.
^C
--- 10.0.0.65 ping statistics ---
4 packets transmitted, 0 received, 100% packet loss, time 3073ms



Do not fragment, actually shows the message too long for the MTU:
vyos@vyos:~$ ping 10.0.0.65 size 1473 do-not-fragment
sudo ip vrf exec default /bin/ping -s 1473 -M do 10.0.0.65
PING 10.0.0.65 (10.0.0.65) 1473(1501) bytes of data.
/bin/ping: local error: message too long, mtu=1500
/bin/ping: local error: message too long, mtu=1500
/bin/ping: local error: message too long, mtu=1500
/bin/ping: local error: message too long, mtu=1500
^C
--- 10.0.0.65 ping statistics ---
4 packets transmitted, 0 received, +4 errors, 100% packet loss, time 3072ms

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

In this commit we fix the do not fragment capability
for ping commands. Sorry for messing it up earlier :(
@c-po c-po merged commit 293c317 into vyos:current Aug 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants