netdev - Re: [RFC PATCH] [TCP]: Reworked recovery's TCPCB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 7 Mar 2007 19:48:49 +0200 (EET)
From:	"Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi>
To:	Baruch Even <baruch@...en.org>
cc:	netdev@...r.kernel.org, David Miller <davem@...emloft.net>
Subject: Re: [RFC PATCH] [TCP]: Reworked recovery's TCPCB_LOST marking
 functions

On Tue, 6 Mar 2007, Baruch Even wrote:

> [snip]
> 
> > +		newtp->highest_sack = treq->snt_isn + 1;
> 
> That's the only initialization that you have for highest_sack, I think
> that you should initialize it when a loss is detected to the start_seq
> of the first packet that wasn't acked.

I agree that this problem exists and in fact I was aware of it, wasn't
just too keen to solve that yet. However, I'm not sure if the proposed
solution is adequate solution because the last ACK doesn't necessarily
contain the all-time highest SACK block (e.g., duplicate ACK reordering),
this is only remotely possible to cause entry to recovery but
still. Obviously the recovery cannot (then) be triggered with this:

	/* Not-A-Trick#2 : Classic rule... */
        if (tcp_fackets_out(tp) > tp->reordering)
                return 1;

(would have entered recovery using an earlier ACK that had the highest 
SACK block) but there are other conditions in tcp_time_to_recover as
well. I would just keep it in sync whole the time when using slowpath of 
tcp_ack. Will repost soon it and a bugfixed scoreboard thingie as 
separated patches (against the current net-2.6.22 + those two patches I 
list below, which were mentioned in the patch description btw).

> Didn't review the rest, still need to arrange a proper tree with
> preliminary patches to apply it on. Could you note the kernel you based
> it on and include all patches applied before it?

I had these patches from David Miller (posted to netdev couple of days / 
week ago in four patch serie) on the top of it:
  [TCP]: Abstract out all write queue operations.
  [TCP]: Store retransmit queue packets in RB tree.

...as I warned earlier, the rest in the David patch series probably 
conflict (at least linux/tcp.h I think because of hint removals).

-- 
 i.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html