James R T <jamestio...@gmail.com> writes: > Hi Simon, > > I have filled out the copyright assignment form accordingly.
Have you heard back from the FSF on this? It would be nice to review
and merge your patch eventually.
/Simon
> The traceroute implementation by Dmitry Butskoy does not have this
> issue. Hence, it could be said that this patch would allow the
> traceroute implementation in `inetutils` to be more consistent with
> it. That said, no investigation has been done on how said
> implementation avoids or fixes this issue nor have I checked if this
> issue was ever brought up for discussion for that implementation in
> the first place.
>
> I also have not checked whether this issue is present in
> implementations other than the one written by Dmitry, such as Dublin
> Traceroute or Paris Traceroute.
>
> Best regards,
> James Raphael Tiovalen
>
> On Mon, Apr 24, 2023 at 3:12 PM Simon Josefsson <si...@josefsson.org> wrote:
>>
>> Hi. Thank you -- I have sent you the copyright assignment form
>> separately. Meanwhile, could you say something about how other
>> traceroute implementations behave here? Does this patch make ours more
>> consistent with the rest, or is this an area where implementations
>> behave differently?
>>
>> /Simon
>>
>> James Raphael Tiovalen <jamestio...@gmail.com> writes:
>>
>> > Currently, traceroute resets the timeout to 3 seconds within the same
>> > probe when undesired ICMP traffic is received on the socket
>> > continuously, which causes it to hang or delay in timeout.
>> >
>> > This commit fixes this issue by setting the timeout to the remaining
>> > time within the same probe while processing the undesired ICMP packet
>> > instead of resetting the timeout to 3 seconds. The next probe is sent
>> > either when it times out or when the desired ICMP message is received.
>> >
>> > The fix can be verified by sending continuous ICMP traffic on the
>> > traceroute socket. Traceroute should not hang and there should not be
>> > any delays in the timeout.
>> >
>> > Signed-off-by: James Raphael Tiovalen <jamestio...@gmail.com>
>> > ---
>> > src/traceroute.c | 35 +++++++++++++++++++++++++++--------
>> > 1 file changed, 27 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/src/traceroute.c b/src/traceroute.c
>> > index e96f2307..f0e8c242 100644
>> > --- a/src/traceroute.c
>> > +++ b/src/traceroute.c
>> > @@ -359,11 +359,18 @@ do_try (trace_t * trace, const int hop,
>> > fd_set readset;
>> > int ret, tries, readonly = 0;
>> > struct timeval now, time;
>> > + struct timeval start, end;
>> > + long sec_elapsed = 0;
>> > + long usec_elapsed = 0;
>> > double triptime = 0.0;
>> > uint32_t prev_addr = 0;
>> >
>> > printf (" %2d ", hop);
>> >
>> > + memset (&time, 0, sizeof (time)); /* 64-bit issue. */
>> > + memset (&start, 0, sizeof (start)); /* 64-bit issue. */
>> > + memset (&end, 0, sizeof (end)); /* 64-bit issue. */
>> > +
>> > for (tries = 0; tries < max_tries; tries++)
>> > {
>> > int save_errno;
>> > @@ -372,18 +379,19 @@ do_try (trace_t * trace, const int hop,
>> > FD_ZERO (&readset);
>> > FD_SET (fd, &readset);
>> >
>> > - memset (&time, 0, sizeof (time)); /* 64-bit issue. */
>> > - time.tv_sec = opt_wait;
>> > - time.tv_usec = 0;
>> > -
>> > if (!readonly)
>> > - trace_write (trace);
>> > + {
>> > + time.tv_sec = opt_wait;
>> > + time.tv_usec = 0;
>> > + trace_write (trace);
>> > + }
>> >
>> > errno = 0;
>> > ret = select (fd + 1, &readset, NULL, NULL, &time);
>> > save_errno = errno;
>> >
>> > gettimeofday (&now, NULL);
>> > + gettimeofday (&start, NULL);
>> >
>> > now.tv_usec -= trace->tsent.tv_usec;
>> > now.tv_sec -= trace->tsent.tv_sec;
>> > @@ -425,9 +433,20 @@ do_try (trace_t * trace, const int hop,
>> > if (rc < 0)
>> > {
>> > /* FIXME: printf ("Some error ocurred\n"); */
>> > - tries--;
>> > - readonly = 1;
>> > - continue;
>> > + gettimeofday (&end, NULL);
>> > + sec_elapsed = (end.tv_sec - start.tv_sec);
>> > + usec_elapsed = (end.tv_usec - start.tv_usec);
>> > + if (usec_elapsed < 0)
>> > + usec_elapsed = 0;
>> > + time.tv_sec -= sec_elapsed;
>> > + time.tv_usec -= usec_elapsed;
>> > + if (time.tv_sec >= 0 && time.tv_usec >= 0)
>> > + {
>> > + tries--;
>> > + if (!readonly)
>> > + readonly = 1;
>> > + continue;
>> > + }
>> > }
>> > else
>> > {
>
>
signature.asc
Description: PGP signature
