netdev - Re: [PATCH v6] sctp: be more restrictive in transport selection on bundled sacks

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]

Message-Id: <20120630.173945.173993639982489712.davem@davemloft.net>
Date:	Sat, 30 Jun 2012 17:39:45 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	nhorman@...driver.com
Cc:	netdev@...r.kernel.org, vyasevich@...il.com,
	linux-sctp@...r.kernel.org
Subject: Re: [PATCH v6] sctp: be more restrictive in transport selection on
 bundled sacks

From: Neil Horman <nhorman@...driver.com>
Date: Sat, 30 Jun 2012 09:04:26 -0400

> It was noticed recently that when we send data on a transport, its possible that
> we might bundle a sack that arrived on a different transport.  While this isn't
> a major problem, it does go against the SHOULD requirement in section 6.4 of RFC
> 2960:
> 
>  An endpoint SHOULD transmit reply chunks (e.g., SACK, HEARTBEAT ACK,
>    etc.) to the same destination transport address from which it
>    received the DATA or control chunk to which it is replying.  This
>    rule should also be followed if the endpoint is bundling DATA chunks
>    together with the reply chunk.
> 
> This patch seeks to correct that.  It restricts the bundling of sack operations
> to only those transports which have moved the ctsn of the association forward
> since the last sack.  By doing this we guarantee that we only bundle outbound
> saks on a transport that has received a chunk since the last sack.  This brings
> us into stricter compliance with the RFC.
> 
> Vlad had initially suggested that we strictly allow only sack bundling on the
> transport that last moved the ctsn forward.  While this makes sense, I was
> concerned that doing so prevented us from bundling in the case where we had
> received chunks that moved the ctsn on multiple transports.  In those cases, the
> RFC allows us to select any of the transports having received chunks to bundle
> the sack on.  so I've modified the approach to allow for that, by adding a state
> variable to each transport that tracks weather it has moved the ctsn since the
> last sack.  This I think keeps our behavior (and performance), close enough to
> our current profile that I think we can do this without a sysctl knob to
> enable/disable it.
> 
> Signed-off-by: Neil Horman <nhorman@...driver.com>
> CC: Vlad Yaseivch <vyasevich@...il.com>
> CC: David S. Miller <davem@...emloft.net>
> CC: linux-sctp@...r.kernel.org
> Reported-by: Michele Baldessari <michele@...hat.com>
> Reported-by: sorin serban <sserban@...hat.com>

Once this has Vlad's ACK I'll apply it.

There has to be a better way to handle this situation, wherein the
responsible party has ACK'd the patch but I just ask for a few coding
style fixups and whatnot.  As it stands now I have to twiddle my
thumbs waiting for the new ACK.

--
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