* [PATCH net-2.6.25 1/3] [TCP]: cleanup tcp_parse_options deep indented switch @ 2008-01-03 11:00 Ilpo Järvinen 2008-01-03 11:00 ` [PATCH net-2.6.25 2/3] [TCP]: Urgent parameter effect can be simplified Ilpo Järvinen 2008-01-04 4:37 ` [PATCH net-2.6.25 1/3] [TCP]: cleanup tcp_parse_options deep indented switch David Miller 0 siblings, 2 replies; 6+ messages in thread From: Ilpo Järvinen @ 2008-01-03 11:00 UTC (permalink / raw) To: David Miller; +Cc: netdev Removed case indentation level & combined some nested ifs, mostly within 80 lines now. This is a leftover from indent patch, it just had to be done manually to avoid messing it up completely. Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> --- net/ipv4/tcp_input.c | 135 +++++++++++++++++++++++++------------------------- 1 files changed, 67 insertions(+), 68 deletions(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 18e099c..d5b6adf 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -3278,81 +3278,80 @@ void tcp_parse_options(struct sk_buff *skb, struct tcp_options_received *opt_rx, int opsize; switch (opcode) { - case TCPOPT_EOL: + case TCPOPT_EOL: + return; + case TCPOPT_NOP: /* Ref: RFC 793 section 3.1 */ + length--; + continue; + default: + opsize = *ptr++; + if (opsize < 2) /* "silly options" */ return; - case TCPOPT_NOP: /* Ref: RFC 793 section 3.1 */ - length--; - continue; - default: - opsize=*ptr++; - if (opsize < 2) /* "silly options" */ - return; - if (opsize > length) - return; /* don't parse partial options */ - switch (opcode) { - case TCPOPT_MSS: - if (opsize==TCPOLEN_MSS && th->syn && !estab) { - u16 in_mss = ntohs(get_unaligned((__be16 *)ptr)); - if (in_mss) { - if (opt_rx->user_mss && opt_rx->user_mss < in_mss) - in_mss = opt_rx->user_mss; - opt_rx->mss_clamp = in_mss; - } - } - break; - case TCPOPT_WINDOW: - if (opsize==TCPOLEN_WINDOW && th->syn && !estab) - if (sysctl_tcp_window_scaling) { - __u8 snd_wscale = *(__u8 *) ptr; - opt_rx->wscale_ok = 1; - if (snd_wscale > 14) { - if (net_ratelimit()) - printk(KERN_INFO "tcp_parse_options: Illegal window " - "scaling value %d >14 received.\n", - snd_wscale); - snd_wscale = 14; - } - opt_rx->snd_wscale = snd_wscale; - } - break; - case TCPOPT_TIMESTAMP: - if (opsize==TCPOLEN_TIMESTAMP) { - if ((estab && opt_rx->tstamp_ok) || - (!estab && sysctl_tcp_timestamps)) { - opt_rx->saw_tstamp = 1; - opt_rx->rcv_tsval = ntohl(get_unaligned((__be32 *)ptr)); - opt_rx->rcv_tsecr = ntohl(get_unaligned((__be32 *)(ptr+4))); - } + if (opsize > length) + return; /* don't parse partial options */ + switch (opcode) { + case TCPOPT_MSS: + if (opsize == TCPOLEN_MSS && th->syn && !estab) { + u16 in_mss = ntohs(get_unaligned((__be16 *)ptr)); + if (in_mss) { + if (opt_rx->user_mss && + opt_rx->user_mss < in_mss) + in_mss = opt_rx->user_mss; + opt_rx->mss_clamp = in_mss; } - break; - case TCPOPT_SACK_PERM: - if (opsize==TCPOLEN_SACK_PERM && th->syn && !estab) { - if (sysctl_tcp_sack) { - opt_rx->sack_ok = 1; - tcp_sack_reset(opt_rx); - } + } + break; + case TCPOPT_WINDOW: + if (opsize == TCPOLEN_WINDOW && th->syn && + !estab && sysctl_tcp_window_scaling) { + __u8 snd_wscale = *(__u8 *)ptr; + opt_rx->wscale_ok = 1; + if (snd_wscale > 14) { + if (net_ratelimit()) + printk(KERN_INFO "tcp_parse_options: Illegal window " + "scaling value %d >14 received.\n", + snd_wscale); + snd_wscale = 14; } - break; + opt_rx->snd_wscale = snd_wscale; + } + break; + case TCPOPT_TIMESTAMP: + if ((opsize == TCPOLEN_TIMESTAMP) && + ((estab && opt_rx->tstamp_ok) || + (!estab && sysctl_tcp_timestamps))) { + opt_rx->saw_tstamp = 1; + opt_rx->rcv_tsval = ntohl(get_unaligned((__be32 *)ptr)); + opt_rx->rcv_tsecr = ntohl(get_unaligned((__be32 *)(ptr+4))); + } + break; + case TCPOPT_SACK_PERM: + if (opsize == TCPOLEN_SACK_PERM && th->syn && + !estab && sysctl_tcp_sack) { + opt_rx->sack_ok = 1; + tcp_sack_reset(opt_rx); + } + break; - case TCPOPT_SACK: - if ((opsize >= (TCPOLEN_SACK_BASE + TCPOLEN_SACK_PERBLOCK)) && - !((opsize - TCPOLEN_SACK_BASE) % TCPOLEN_SACK_PERBLOCK) && - opt_rx->sack_ok) { - TCP_SKB_CB(skb)->sacked = (ptr - 2) - (unsigned char *)th; - } - break; + case TCPOPT_SACK: + if ((opsize >= (TCPOLEN_SACK_BASE + TCPOLEN_SACK_PERBLOCK)) && + !((opsize - TCPOLEN_SACK_BASE) % TCPOLEN_SACK_PERBLOCK) && + opt_rx->sack_ok) { + TCP_SKB_CB(skb)->sacked = (ptr - 2) - (unsigned char *)th; + } + break; #ifdef CONFIG_TCP_MD5SIG - case TCPOPT_MD5SIG: - /* - * The MD5 Hash has already been - * checked (see tcp_v{4,6}_do_rcv()). - */ - break; + case TCPOPT_MD5SIG: + /* + * The MD5 Hash has already been + * checked (see tcp_v{4,6}_do_rcv()). + */ + break; #endif - } + } - ptr+=opsize-2; - length-=opsize; + ptr += opsize-2; + length -= opsize; } } } -- 1.5.0.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-2.6.25 2/3] [TCP]: Urgent parameter effect can be simplified. 2008-01-03 11:00 [PATCH net-2.6.25 1/3] [TCP]: cleanup tcp_parse_options deep indented switch Ilpo Järvinen @ 2008-01-03 11:00 ` Ilpo Järvinen 2008-01-03 11:00 ` [PATCH net-2.6.25 3/3] [TCP]: Perform setting of common control fields in one place Ilpo Järvinen 2008-01-04 4:38 ` [PATCH net-2.6.25 2/3] [TCP]: Urgent parameter effect can be simplified David Miller 2008-01-04 4:37 ` [PATCH net-2.6.25 1/3] [TCP]: cleanup tcp_parse_options deep indented switch David Miller 1 sibling, 2 replies; 6+ messages in thread From: Ilpo Järvinen @ 2008-01-03 11:00 UTC (permalink / raw) To: David Miller; +Cc: netdev Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> --- net/ipv4/tcp_output.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index f6d279a..6c7cd0a 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -2488,7 +2488,7 @@ static int tcp_xmit_probe_skb(struct sock *sk, int urgent) * end to send an ack. Don't queue or clone SKB, just * send it. */ - TCP_SKB_CB(skb)->seq = urgent ? tp->snd_una : tp->snd_una - 1; + TCP_SKB_CB(skb)->seq = tp->snd_una - !urgent; TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(skb)->seq; TCP_SKB_CB(skb)->when = tcp_time_stamp; return tcp_transmit_skb(sk, skb, 0, GFP_ATOMIC); -- 1.5.0.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-2.6.25 3/3] [TCP]: Perform setting of common control fields in one place 2008-01-03 11:00 ` [PATCH net-2.6.25 2/3] [TCP]: Urgent parameter effect can be simplified Ilpo Järvinen @ 2008-01-03 11:00 ` Ilpo Järvinen 2008-01-04 4:39 ` David Miller 2008-01-04 4:38 ` [PATCH net-2.6.25 2/3] [TCP]: Urgent parameter effect can be simplified David Miller 1 sibling, 1 reply; 6+ messages in thread From: Ilpo Järvinen @ 2008-01-03 11:00 UTC (permalink / raw) To: David Miller; +Cc: netdev In case of segments which are purely for control without any data (SYN/ACK/FIN/RST), many fields are set to common values in multiple places. i386 results: $ gcc --version gcc (GCC) 4.1.2 20070626 (Red Hat 4.1.2-13) $ codiff tcp_output.o.old tcp_output.o.new net/ipv4/tcp_output.c: tcp_xmit_probe_skb | -48 tcp_send_ack | -56 tcp_retransmit_skb | -79 tcp_connect | -43 tcp_send_active_reset | -35 tcp_make_synack | -42 tcp_send_fin | -48 7 functions changed, 351 bytes removed net/ipv4/tcp_output.c: tcp_init_nondata_skb | +90 1 function changed, 90 bytes added tcp_output.o.mid: 8 functions changed, 90 bytes added, 351 bytes removed, diff: -261 Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> --- net/ipv4/tcp_output.c | 91 +++++++++++++++++++----------------------------- 1 files changed, 36 insertions(+), 55 deletions(-) diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 6c7cd0a..89f0188 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -327,6 +327,26 @@ static inline void TCP_ECN_send(struct sock *sk, struct sk_buff *skb, } } +/* Constructs common control bits of non-data skb. If SYN/FIN is present, + * auto increment end seqno. + */ +static void tcp_init_nondata_skb(struct sk_buff *skb, u32 seq, u8 flags) +{ + skb->csum = 0; + + TCP_SKB_CB(skb)->flags = flags; + TCP_SKB_CB(skb)->sacked = 0; + + skb_shinfo(skb)->gso_segs = 1; + skb_shinfo(skb)->gso_size = 0; + skb_shinfo(skb)->gso_type = 0; + + TCP_SKB_CB(skb)->seq = seq; + if (flags & (TCPCB_FLAG_SYN | TCPCB_FLAG_FIN)) + seq++; + TCP_SKB_CB(skb)->end_seq = seq; +} + static void tcp_build_and_update_options(__be32 *ptr, struct tcp_sock *tp, __u32 tstamp, __u8 **md5_hash) { @@ -1864,12 +1884,10 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb) (TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN) && tp->snd_una == (TCP_SKB_CB(skb)->end_seq - 1)) { if (!pskb_trim(skb, 0)) { - TCP_SKB_CB(skb)->seq = TCP_SKB_CB(skb)->end_seq - 1; - skb_shinfo(skb)->gso_segs = 1; - skb_shinfo(skb)->gso_size = 0; - skb_shinfo(skb)->gso_type = 0; + /* Reuse, even though it does some unnecessary work */ + tcp_init_nondata_skb(skb, TCP_SKB_CB(skb)->end_seq - 1, + TCP_SKB_CB(skb)->flags); skb->ip_summed = CHECKSUM_NONE; - skb->csum = 0; } } @@ -2068,16 +2086,9 @@ void tcp_send_fin(struct sock *sk) /* Reserve space for headers and prepare control bits. */ skb_reserve(skb, MAX_TCP_HEADER); - skb->csum = 0; - TCP_SKB_CB(skb)->flags = (TCPCB_FLAG_ACK | TCPCB_FLAG_FIN); - TCP_SKB_CB(skb)->sacked = 0; - skb_shinfo(skb)->gso_segs = 1; - skb_shinfo(skb)->gso_size = 0; - skb_shinfo(skb)->gso_type = 0; - /* FIN eats a sequence byte, write_seq advanced by tcp_queue_skb(). */ - TCP_SKB_CB(skb)->seq = tp->write_seq; - TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(skb)->seq + 1; + tcp_init_nondata_skb(skb, tp->write_seq, + TCPCB_FLAG_ACK | TCPCB_FLAG_FIN); tcp_queue_skb(sk, skb); } __tcp_push_pending_frames(sk, mss_now, TCP_NAGLE_OFF); @@ -2101,16 +2112,9 @@ void tcp_send_active_reset(struct sock *sk, gfp_t priority) /* Reserve space for headers and prepare control bits. */ skb_reserve(skb, MAX_TCP_HEADER); - skb->csum = 0; - TCP_SKB_CB(skb)->flags = (TCPCB_FLAG_ACK | TCPCB_FLAG_RST); - TCP_SKB_CB(skb)->sacked = 0; - skb_shinfo(skb)->gso_segs = 1; - skb_shinfo(skb)->gso_size = 0; - skb_shinfo(skb)->gso_type = 0; - + tcp_init_nondata_skb(skb, tcp_acceptable_seq(sk), + TCPCB_FLAG_ACK | TCPCB_FLAG_RST); /* Send it off. */ - TCP_SKB_CB(skb)->seq = tcp_acceptable_seq(sk); - TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(skb)->seq; TCP_SKB_CB(skb)->when = tcp_time_stamp; if (tcp_transmit_skb(sk, skb, 0, priority)) NET_INC_STATS(LINUX_MIB_TCPABORTFAILED); @@ -2198,12 +2202,11 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst, TCP_ECN_make_synack(req, th); th->source = inet_sk(sk)->sport; th->dest = ireq->rmt_port; - TCP_SKB_CB(skb)->seq = tcp_rsk(req)->snt_isn; - TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(skb)->seq + 1; - TCP_SKB_CB(skb)->sacked = 0; - skb_shinfo(skb)->gso_segs = 1; - skb_shinfo(skb)->gso_size = 0; - skb_shinfo(skb)->gso_type = 0; + /* Setting of flags are superfluous here for callers (and ECE is + * not even correctly set) + */ + tcp_init_nondata_skb(skb, tcp_rsk(req)->snt_isn, + TCPCB_FLAG_SYN | TCPCB_FLAG_ACK); th->seq = htonl(TCP_SKB_CB(skb)->seq); th->ack_seq = htonl(tcp_rsk(req)->rcv_isn + 1); if (req->rcv_wnd == 0) { /* ignored for retransmitted syns */ @@ -2235,7 +2238,6 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst, NULL) ); - skb->csum = 0; th->doff = (tcp_header_size >> 2); TCP_INC_STATS(TCP_MIB_OUTSEGS); @@ -2327,16 +2329,9 @@ int tcp_connect(struct sock *sk) /* Reserve space for headers. */ skb_reserve(buff, MAX_TCP_HEADER); - TCP_SKB_CB(buff)->flags = TCPCB_FLAG_SYN; - TCP_ECN_send_syn(sk, buff); - TCP_SKB_CB(buff)->sacked = 0; - skb_shinfo(buff)->gso_segs = 1; - skb_shinfo(buff)->gso_size = 0; - skb_shinfo(buff)->gso_type = 0; - buff->csum = 0; tp->snd_nxt = tp->write_seq; - TCP_SKB_CB(buff)->seq = tp->write_seq++; - TCP_SKB_CB(buff)->end_seq = tp->write_seq; + tcp_init_nondata_skb(buff, tp->write_seq++, TCPCB_FLAG_SYN); + TCP_ECN_send_syn(sk, buff); /* Send it off. */ TCP_SKB_CB(buff)->when = tcp_time_stamp; @@ -2441,15 +2436,9 @@ void tcp_send_ack(struct sock *sk) /* Reserve space for headers and prepare control bits. */ skb_reserve(buff, MAX_TCP_HEADER); - buff->csum = 0; - TCP_SKB_CB(buff)->flags = TCPCB_FLAG_ACK; - TCP_SKB_CB(buff)->sacked = 0; - skb_shinfo(buff)->gso_segs = 1; - skb_shinfo(buff)->gso_size = 0; - skb_shinfo(buff)->gso_type = 0; + tcp_init_nondata_skb(buff, tcp_acceptable_seq(sk), TCPCB_FLAG_ACK); /* Send it off, this clears delayed acks for us. */ - TCP_SKB_CB(buff)->seq = TCP_SKB_CB(buff)->end_seq = tcp_acceptable_seq(sk); TCP_SKB_CB(buff)->when = tcp_time_stamp; tcp_transmit_skb(sk, buff, 0, GFP_ATOMIC); } @@ -2477,19 +2466,11 @@ static int tcp_xmit_probe_skb(struct sock *sk, int urgent) /* Reserve space for headers and set control bits. */ skb_reserve(skb, MAX_TCP_HEADER); - skb->csum = 0; - TCP_SKB_CB(skb)->flags = TCPCB_FLAG_ACK; - TCP_SKB_CB(skb)->sacked = 0; - skb_shinfo(skb)->gso_segs = 1; - skb_shinfo(skb)->gso_size = 0; - skb_shinfo(skb)->gso_type = 0; - /* Use a previous sequence. This should cause the other * end to send an ack. Don't queue or clone SKB, just * send it. */ - TCP_SKB_CB(skb)->seq = tp->snd_una - !urgent; - TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(skb)->seq; + tcp_init_nondata_skb(skb, tp->snd_una - !urgent, TCPCB_FLAG_ACK); TCP_SKB_CB(skb)->when = tcp_time_stamp; return tcp_transmit_skb(sk, skb, 0, GFP_ATOMIC); } -- 1.5.0.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-2.6.25 3/3] [TCP]: Perform setting of common control fields in one place 2008-01-03 11:00 ` [PATCH net-2.6.25 3/3] [TCP]: Perform setting of common control fields in one place Ilpo Järvinen @ 2008-01-04 4:39 ` David Miller 0 siblings, 0 replies; 6+ messages in thread From: David Miller @ 2008-01-04 4:39 UTC (permalink / raw) To: ilpo.jarvinen; +Cc: netdev From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi> Date: Thu, 3 Jan 2008 13:00:07 +0200 > In case of segments which are purely for control without any > data (SYN/ACK/FIN/RST), many fields are set to common values > in multiple places. ... > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> It's nice to see Arnaldo's tools get some usage :-) Applied, thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-2.6.25 2/3] [TCP]: Urgent parameter effect can be simplified. 2008-01-03 11:00 ` [PATCH net-2.6.25 2/3] [TCP]: Urgent parameter effect can be simplified Ilpo Järvinen 2008-01-03 11:00 ` [PATCH net-2.6.25 3/3] [TCP]: Perform setting of common control fields in one place Ilpo Järvinen @ 2008-01-04 4:38 ` David Miller 1 sibling, 0 replies; 6+ messages in thread From: David Miller @ 2008-01-04 4:38 UTC (permalink / raw) To: ilpo.jarvinen; +Cc: netdev From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi> Date: Thu, 3 Jan 2008 13:00:06 +0200 > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> Applied. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-2.6.25 1/3] [TCP]: cleanup tcp_parse_options deep indented switch 2008-01-03 11:00 [PATCH net-2.6.25 1/3] [TCP]: cleanup tcp_parse_options deep indented switch Ilpo Järvinen 2008-01-03 11:00 ` [PATCH net-2.6.25 2/3] [TCP]: Urgent parameter effect can be simplified Ilpo Järvinen @ 2008-01-04 4:37 ` David Miller 1 sibling, 0 replies; 6+ messages in thread From: David Miller @ 2008-01-04 4:37 UTC (permalink / raw) To: ilpo.jarvinen; +Cc: netdev From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi> Date: Thu, 3 Jan 2008 13:00:05 +0200 > Removed case indentation level & combined some nested ifs, mostly > within 80 lines now. This is a leftover from indent patch, it > just had to be done manually to avoid messing it up completely. > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> Applied. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-01-04 4:39 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-01-03 11:00 [PATCH net-2.6.25 1/3] [TCP]: cleanup tcp_parse_options deep indented switch Ilpo Järvinen 2008-01-03 11:00 ` [PATCH net-2.6.25 2/3] [TCP]: Urgent parameter effect can be simplified Ilpo Järvinen 2008-01-03 11:00 ` [PATCH net-2.6.25 3/3] [TCP]: Perform setting of common control fields in one place Ilpo Järvinen 2008-01-04 4:39 ` David Miller 2008-01-04 4:38 ` [PATCH net-2.6.25 2/3] [TCP]: Urgent parameter effect can be simplified David Miller 2008-01-04 4:37 ` [PATCH net-2.6.25 1/3] [TCP]: cleanup tcp_parse_options deep indented switch David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox