jlduran / freebsd-src

FreeBSD src tree (read-only mirror)
https://www.FreeBSD.org/
Other
0 stars 0 forks source link

Add a MAXICMPLEN test #99

Open jlduran opened 5 days ago

jlduran commented 5 days ago

https://lists.freebsd.org/archives/dev-commits-src-main/2024-October/027192.html Before:

# ping -s 65507 -c 1 127.0.0.1
PING 127.0.0.1 (127.0.0.1): 65507 data bytes
65515 bytes from 127.0.0.1: icmp_seq=0 ttl=64 time=0.194 ms
...

Now:

# /usr/obj/usr/home/maxim/fbsd/src/amd64.amd64/sbin/ping/ping -s 65507
-c 1 127.0.0.1
ping: packet size too large: 65507 > 65467

Not sure if the tests catch this.

Maxim

jlduran commented 5 days ago

https://lists.freebsd.org/archives/dev-commits-src-main/2024-October/027272.html

After your change:

# ping -s 65507 10.1.10.1
ping: packet size too large: 65507 > 65467

This is the regression Maxim is pointing you at.

Also, after your patch (running from non-root):

ping -s 10000 10.1.10.1 PING 10.1.10.1 (10.1.10.1): 10000 data bytes ping: sendto: Message too long

And this just shows that you didn't even fully achieve what you wanted.

To achieve what you want, you first need to fully revert your patch, and then apply the attached patch. It will consistently disable all the size checks that our ping has, instead of incorrectly and blindly applying a diff from DragonflyBSD.

HOWEVER, PLEASE DO NOT COMMIT the attached patch! Please revert what you already committed and start a normal review process on phabricator. You can use my attached patch as a start. Please put both the #network group and the

secteam group. This is a kind of sensitive change that can't be done by a

drive by commit.

P.S. If you are pinging FreeBSD host, the other side needs to have increased maxfrag to reply:

# sysctl net.inet.ip.maxfragsperpacket=44