Re: commit 64ff3b938ec6782e6585a83d5459b98b0c3f6eb8 breaks rlogin

On Sat, 21 Feb 2009, Herbert Xu wrote:

On Mon, Feb 09, 2009 at 03:28:09PM +0200, Ilpo Järvinen wrote:

Yes, that should work too except the gcc catchable typo.

Thanks, so let's try again for net-next:

tcp: Always set urgent pointer if it's beyond snd_nxt

Our TCP stack does not set the urgent flag if the urgent pointer
does not fit in 16 bits, i.e., if it is more than 64K from the
sequence number of a packet.

This behaviour is different from the BSDs, and clearly contradicts
the purpose of urgent mode, which is to send the notification
(though not necessarily the associated data) as soon as possible.
Our current behaviour may in fact delay the urgent notification
indefinitely if the receiver window does not open up.

Simply matching BSD however may break legacy applications which
incorrectly rely on the out-of-band delivery of urgent data, and
conversely the in-band delivery of non-urgent data.

Alexey Kuznetsov suggested a safe solution of following BSD only
if the urgent pointer itself has not yet been transmitted. This
way we guarantee that when the remote end sees the packet with
non-urgent data marked as urgent due to wrap-around we would have
advanced the urgent pointer beyond, either to the actual urgent
data or to an as-yet untransmitted packet.

The only potential downside is that applications on the remote
end may see multiple SIGURG notifications. However, this would
occur anyway with other TCP stacks. More importantly, the outcome
of such a duplicate notification is likely to be harmless since
the signal itself does not carry any information other than the
fact that we're in urgent mode.

Thanks to Ilpo Järvinen for fixing a critical bug in this and
Jeff Chua for reporting that bug.

Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index dda42f0..f5263c8 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -663,10 +663,14 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
th->urg_ptr = 0;

/* The urg_mode check is necessary during a below snd_una win probe */
- if (unlikely(tcp_urg_mode(tp) &&
- between(tp->snd_up, tcb->seq + 1, tcb->seq + 0xFFFF))) {
- th->urg_ptr = htons(tp->snd_up - tcb->seq);
- th->urg = 1;
+ if (unlikely(tcp_urg_mode(tp) && before(tcb->seq, tp->snd_up))) {
+ if (before(tp->snd_up, tcb->seq + 0x10000)) {
+ th->urg_ptr = htons(tp->snd_up - tcb->seq);
+ th->urg = 1;
+ } else if (after(tcb->seq + 0xFFFF, tp->snd_nxt)) {
+ th->urg_ptr = 0xFFFF;
+ th->urg = 1;
+ }
}

tcp_options_write((__be32 *)(th + 1), tp, &opts, &md5_hash_location);

I thought this through already at the last time... so I think it really
should work in all cases I could think of (including the below window
probes whose existance is not very obvious before they bite back :-)):

Acked-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxx>

But I mostly agree with Linus that URG is legacy that would be good
to just deprecate instead of trying to "fix" it.


--
i.

Relevant Pages

  • Re: TCP Urgent Pointer Usage
    ... the byte indicated by the urgent pointer field in the packet is ... existence of a single byte of OOB data within the TCP data stream. ... The urgent pointer is a positive offset from ...
    (microsoft.public.dotnet.general)
  • Re: commit 64ff3b938ec6782e6585a83d5459b98b0c3f6eb8 breaks rlogin
    ... Our TCP stack does not set the urgent flag if the urgent pointer ... Our current behaviour may in fact delay the urgent notification ...
    (Linux-Kernel)
  • Re: TCP out of band data and read()
    ... TCP has no mechanism to transport ... but the sockets API automatically treats the byte that the urgent ... Which is what the sockets API does. ... what happens to the byte that the urgent pointer ...
    (comp.unix.programmer)
  • Re: FTP Error 426
    ... TCP also provides a means to communicate to the receiver of data that at some point further along in the data stream than the receiver is currently reading there is urgent data. ... correct description is "command" not "subcommand", specifically an "FTP service command", see section 4.1.3. ...
    (bit.listserv.ibm-main)
  • throughtput detector in linux or window
    ... I would like to test my TCP, UDP, RTP date rate ... It is very urgent. ...
    (comp.os.linux.networking)