| Message ID | 1416200496.24093.5.camel@edumazet-glaptop2.roam.corp.google.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On 2014-11-17 07:01, Eric Dumazet wrote: > On Sun, 2014-11-16 at 12:16 -0800, Eric Dumazet wrote: > >> Thanks Denys ! >> >> Could you try following patch ? >> >> Thanks ! > > Hmm.... I have an updated patch, sorry. > > (A memcpy_fromiovec() has to be memcpy_fromiovecend() ) > > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c > index a3d453b94747..c2bbfcd9c0db 100644 > --- a/net/ipv4/tcp_output.c > +++ b/net/ipv4/tcp_output.c > @@ -2998,7 +2998,7 @@ static int tcp_send_syn_data(struct sock *sk, > struct sk_buff *syn) > { > struct tcp_sock *tp = tcp_sk(sk); > struct tcp_fastopen_request *fo = tp->fastopen_req; > - int syn_loss = 0, space, i, err = 0, iovlen = fo->data->msg_iovlen; > + int syn_loss = 0, space, err = 0; > struct sk_buff *syn_data = NULL, *data; > unsigned long last_syn_loss = 0; > > @@ -3031,25 +3031,19 @@ static int tcp_send_syn_data(struct sock *sk, > struct sk_buff *syn) > /* limit to order-0 allocations */ > space = min_t(size_t, space, SKB_MAX_HEAD(MAX_TCP_HEADER)); > > - syn_data = skb_copy_expand(syn, MAX_TCP_HEADER, space, > - sk->sk_allocation); > - if (syn_data == NULL) > + syn_data = sk_stream_alloc_skb(sk, space, sk->sk_allocation); > + if (!syn_data) > goto fallback; > > - for (i = 0; i < iovlen && syn_data->len < space; ++i) { > - struct iovec *iov = &fo->data->msg_iov[i]; > - unsigned char __user *from = iov->iov_base; > - int len = iov->iov_len; > - > - if (syn_data->len + len > space) > - len = space - syn_data->len; > - else if (i + 1 == iovlen) > - /* No more data pending in inet_wait_for_connect() */ > - fo->data = NULL; > + syn_data->ip_summed = CHECKSUM_PARTIAL; > + memcpy(syn_data->cb, syn->cb, sizeof(syn->cb)); > + if (memcpy_fromiovecend(skb_put(syn_data, space), > + fo->data->msg_iov, 0, space)) > + goto fallback; > > - if (skb_add_data(syn_data, from, len)) > - goto fallback; > - } > + /* No more data pending in inet_wait_for_connect() */ > + if (space == fo->size) > + fo->data = NULL; > > /* Queue a data-only packet after the regular SYN for retransmission > */ > data = pskb_copy(syn_data, sk->sk_allocation); > @@ -3101,13 +3095,10 @@ int tcp_connect(struct sock *sk) > return 0; > } > > - buff = alloc_skb_fclone(MAX_TCP_HEADER + 15, sk->sk_allocation); > - if (unlikely(buff == NULL)) > + buff = sk_stream_alloc_skb(sk, 0, sk->sk_allocation); > + if (unlikely(!buff)) > return -ENOBUFS; > > - /* Reserve space for headers. */ > - skb_reserve(buff, MAX_TCP_HEADER); > - > tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN); > tp->retrans_stamp = tcp_time_stamp; > tcp_connect_queue_skb(sk, buff); Installed patch, but will have to wait a while (usually at least 24hours), to be sure if it is stable. Thanks a lot! -- 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 2014-11-17 12:22, Denys Fedoryshchenko wrote: >> Hmm.... I have an updated patch, sorry. >> ... > > Installed patch, but will have to wait a while (usually at least > 24hours), to be sure if it is stable. > > Thanks a lot! Tried updated patch, it seems crashed same after while with it too, and on second test i noticed same value overflow. In debug, after i added alert if sk_forward_alloc > 1147483648 i noticed that on some sockets it continuously increasing this value until it will overflow. I can provide logs if it is interesting. I will try to sysctl fastopen to zero, to make sure if it changes anything. -- 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
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index a3d453b94747..c2bbfcd9c0db 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -2998,7 +2998,7 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn) { struct tcp_sock *tp = tcp_sk(sk); struct tcp_fastopen_request *fo = tp->fastopen_req; - int syn_loss = 0, space, i, err = 0, iovlen = fo->data->msg_iovlen; + int syn_loss = 0, space, err = 0; struct sk_buff *syn_data = NULL, *data; unsigned long last_syn_loss = 0; @@ -3031,25 +3031,19 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn) /* limit to order-0 allocations */ space = min_t(size_t, space, SKB_MAX_HEAD(MAX_TCP_HEADER)); - syn_data = skb_copy_expand(syn, MAX_TCP_HEADER, space, - sk->sk_allocation); - if (syn_data == NULL) + syn_data = sk_stream_alloc_skb(sk, space, sk->sk_allocation); + if (!syn_data) goto fallback; - for (i = 0; i < iovlen && syn_data->len < space; ++i) { - struct iovec *iov = &fo->data->msg_iov[i]; - unsigned char __user *from = iov->iov_base; - int len = iov->iov_len; - - if (syn_data->len + len > space) - len = space - syn_data->len; - else if (i + 1 == iovlen) - /* No more data pending in inet_wait_for_connect() */ - fo->data = NULL; + syn_data->ip_summed = CHECKSUM_PARTIAL; + memcpy(syn_data->cb, syn->cb, sizeof(syn->cb)); + if (memcpy_fromiovecend(skb_put(syn_data, space), + fo->data->msg_iov, 0, space)) + goto fallback; - if (skb_add_data(syn_data, from, len)) - goto fallback; - } + /* No more data pending in inet_wait_for_connect() */ + if (space == fo->size) + fo->data = NULL; /* Queue a data-only packet after the regular SYN for retransmission */ data = pskb_copy(syn_data, sk->sk_allocation); @@ -3101,13 +3095,10 @@ int tcp_connect(struct sock *sk) return 0; } - buff = alloc_skb_fclone(MAX_TCP_HEADER + 15, sk->sk_allocation); - if (unlikely(buff == NULL)) + buff = sk_stream_alloc_skb(sk, 0, sk->sk_allocation); + if (unlikely(!buff)) return -ENOBUFS; - /* Reserve space for headers. */ - skb_reserve(buff, MAX_TCP_HEADER); - tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN); tp->retrans_stamp = tcp_time_stamp; tcp_connect_queue_skb(sk, buff);