Date: Mon, 21 Oct 2024 18:18:23 +0000 (UTC) From: Pedro Giffuni <pfg@freebsd.org> To: "maxim@freebsd.org" <maxim@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: b88df1e893c4 - main - Reapply "sbin/ping: allow normal users to specify larger packets" Message-ID: <1791968192.4471640.1729534703845@mail.yahoo.com> In-Reply-To: <39113afb-66e8-da08-14ab-83564c1271b2@maxim.int.ru> References: <202410161840.49GIe8CR000407@gitrepo.freebsd.org> <f0575f7e-e155-85a7-fa7c-5a67af27ef63@maxim.int.ru> <39113afb-66e8-da08-14ab-83564c1271b2@maxim.int.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
------=_Part_4471639_1666353488.1729534703844
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Sorry ...
I completely missed this was causing any trouble. My filters threw the repl=
y into a folder that I don't look at regularly.
Thanks to markj@ for backing it out.=C2=A0
Pedro.
On Friday, October 18, 2024 at 02:41:12 PM GMT-5, maxim@freebsd.org <ma=
xim@freebsd.org> wrote: =20
=20
Hi Pedro,
Unless you have a plan to address this issue please backout this code
as it is clearly wrong.
Thanks,
Maxim
On Wed, 16 Oct 2024, 19:04-0000, Maxim Konovalov wrote:
> Hi Pedro,
>
> No, this is not right.=C2=A0 Let me clarify:
>
> (1) I never told that there are any issues with the tests.=C2=A0 I just
> mumbled that the tests should catch such regression though I never
> checked if they actually did.
>
> (2) The MAXPAYLOAD calculation in the code below is not fully correct.
>
> It should be
>
> 65535 - 20 (ip header) - 8 (icmp part) =3D 65507 without IP options
>
> OR
>
> 65535 - 20 (ip header) - 40 (ip options) - 8 (icmp part) =3D 65467 with
> IP options, ie. whenever you run ping -R.
>
> The code below hardcoded the latter value which is simply wrong.
>
> I wouldn't rely on the fact that you get it from other BSD flavours
> and would recommend to have this code reviewed before committing it.
>
> Maxim
>
--=20
Maxim Konovalov
=20
------=_Part_4471639_1666353488.1729534703844
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
<html><head></head><body><div class=3D"ydp3b5014f4yahoo-style-wrap" style=
=3D"font-family:Helvetica Neue, Helvetica, Arial, sans-serif;font-size:16px=
;"><div></div>
<div dir=3D"ltr" data-setdir=3D"false">Sorry ...</div><div dir=3D"l=
tr" data-setdir=3D"false"><br></div><div dir=3D"ltr" data-setdir=3D"false">=
I completely missed this was causing any trouble. My filters threw the repl=
y into a folder that I don't look at regularly.</div><div dir=3D"ltr" data-=
setdir=3D"false"><br></div><div dir=3D"ltr" data-setdir=3D"false">Thanks to=
markj@ for backing it out. </div><div dir=3D"ltr" data-setdir=3D"fals=
e"><br></div><div dir=3D"ltr" data-setdir=3D"false">Pedro.</div><div><br></=
div>
=20
</div><div id=3D"ydpd8cf4408yahoo_quoted_9936507121" class=3D"ydpd8=
cf4408yahoo_quoted">
<div style=3D"font-family:'Helvetica Neue', Helvetica, Arial, s=
ans-serif;font-size:13px;color:#26282a;">
=20
<div>
On Friday, October 18, 2024 at 02:41:12 PM GMT-5, m=
axim@freebsd.org <maxim@freebsd.org> wrote:
</div>
<div><br></div>
<div><br></div>
=20
=20
<div><div dir=3D"ltr">Hi Pedro,<br clear=3D"none"><br clear=
=3D"none">Unless you have a plan to address this issue please backout this =
code<br clear=3D"none">as it is clearly wrong.<br clear=3D"none"><br clear=
=3D"none">Thanks,<br clear=3D"none"><br clear=3D"none">Maxim<br clear=3D"no=
ne"><div class=3D"ydpd8cf4408yqt2348932468" id=3D"ydpd8cf4408yqtfd32071"><b=
r clear=3D"none">On Wed, 16 Oct 2024, 19:04-0000, Maxim Konovalov wrote:<br=
clear=3D"none"><br clear=3D"none">> Hi Pedro,<br clear=3D"none">><br=
clear=3D"none">> No, this is not right. Let me clarify:<br clear=
=3D"none">><br clear=3D"none">> (1) I never told that there are any i=
ssues with the tests. I just<br clear=3D"none">> mumbled that the =
tests should catch such regression though I never<br clear=3D"none">> ch=
ecked if they actually did.<br clear=3D"none">><br clear=3D"none">> (=
2) The MAXPAYLOAD calculation in the code below is not fully correct.<br cl=
ear=3D"none">><br clear=3D"none">> It should be<br clear=3D"none">>=
;<br clear=3D"none">> 65535 - 20 (ip header) - 8 (icmp part) =3D 65507 w=
ithout IP options<br clear=3D"none">><br clear=3D"none">> OR<br clear=
=3D"none">><br clear=3D"none">> 65535 - 20 (ip header) - 40 (ip optio=
ns) - 8 (icmp part) =3D 65467 with<br clear=3D"none">> IP options, ie. w=
henever you run ping -R.<br clear=3D"none">><br clear=3D"none">> The =
code below hardcoded the latter value which is simply wrong.<br clear=3D"no=
ne">><br clear=3D"none">> I wouldn't rely on the fact that you get it=
from other BSD flavours<br clear=3D"none">> and would recommend to have=
this code reviewed before committing it.<br clear=3D"none">><br clear=
=3D"none">> Maxim<br clear=3D"none">><br clear=3D"none"><br clear=3D"=
none">-- <br clear=3D"none">Maxim Konovalov<br clear=3D"none"></div></div><=
/div>
</div>
</div></body></html>
------=_Part_4471639_1666353488.1729534703844--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1791968192.4471640.1729534703845>