Message ID | 20100408.012617.98660541.davem@davemloft.net |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Thu, 2010-04-08 at 01:26 -0700, David Miller wrote: > Fix this by setting skb->ip_summed in the common non-data packet > constructor. It already is setting skb->csum to zero. > Signed-off-by: David S. Miller <davem@davemloft.net> > > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c > index f181b78..00afbb0 100644 > --- a/net/ipv4/tcp_output.c > +++ b/net/ipv4/tcp_output.c > @@ -349,6 +349,7 @@ static inline void TCP_ECN_send(struct sock *sk, struct sk_buff *skb, > */ > static void tcp_init_nondata_skb(struct sk_buff *skb, u32 seq, u8 flags) > { > + skb->ip_summed = CHECKSUM_PARTIAL; > skb->csum = 0; > > TCP_SKB_CB(skb)->flags = flags; There might be trivial value in using the struct layout order for the sets avoiding crossing cachelines. from: static void tcp_init_nondata_skb(struct sk_buff *skb, u32 seq, u8 flags) { skb->ip_summed = CHECKSUM_PARTIAL; 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; } to: static void tcp_init_nondata_skb(struct sk_buff *skb, u32 seq, u8 flags) { skb->ip_summed = CHECKSUM_PARTIAL; skb->csum = 0; TCP_SKB_CB(skb)->seq = seq; if (flags & (TCPCB_FLAG_SYN | TCPCB_FLAG_FIN)) seq++; TCP_SKB_CB(skb)->end_seq = seq; TCP_SKB_CB(skb)->sacked = 0; TCP_SKB_CB(skb)->flags = flags; skb_shinfo(skb)->gso_size = 0; skb_shinfo(skb)->gso_segs = 1; skb_shinfo(skb)->gso_type = 0; } -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 08, 2010 at 01:26:17AM -0700, David Miller wrote: > > Back in commit 04a0551c87363f100b04d28d7a15a632b70e18e7 > ("loopback: Drop obsolete ip_summed setting") we stopped > setting CHECKSUM_UNNECESSARY in the loopback xmit. > > This is because such a setting was a lie since it implies that the > checksum field of the packet is properly filled in. > > Instead what happens normally is that CHECKSUM_PARTIAL is set and > skb->csum is calculated as needed. > > But this was only happening for TCP data packets (via the > skb->ip_summed assignment done in tcp_sendmsg()). It doesn't > happen for non-data packets like ACKs etc. I don't understand. If said packet was going across the network, how could it possibly have worked? Cheers,
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index f181b78..00afbb0 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -349,6 +349,7 @@ static inline void TCP_ECN_send(struct sock *sk, struct sk_buff *skb, */ static void tcp_init_nondata_skb(struct sk_buff *skb, u32 seq, u8 flags) { + skb->ip_summed = CHECKSUM_PARTIAL; skb->csum = 0; TCP_SKB_CB(skb)->flags = flags;
Back in commit 04a0551c87363f100b04d28d7a15a632b70e18e7 ("loopback: Drop obsolete ip_summed setting") we stopped setting CHECKSUM_UNNECESSARY in the loopback xmit. This is because such a setting was a lie since it implies that the checksum field of the packet is properly filled in. Instead what happens normally is that CHECKSUM_PARTIAL is set and skb->csum is calculated as needed. But this was only happening for TCP data packets (via the skb->ip_summed assignment done in tcp_sendmsg()). It doesn't happen for non-data packets like ACKs etc. Fix this by setting skb->ip_summed in the common non-data packet constructor. It already is setting skb->csum to zero. But this reminds us that we still have things like ip_output.c's ip_dev_loopback_xmit() which sets skb->ip_summed to the value CHECKSUM_UNNECESSARY, which Herbert's patch teaches us is not valid. So we'll have to address that at some point too. Signed-off-by: David S. Miller <davem@davemloft.net> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html