On Fri, Mar 08, 2019 at 02:34:07PM -0800, Eric Dumazet wrote:
>
>
> On 03/08/2019 02:22 PM, Guillaume Nault wrote:
> > On Fri, Mar 08, 2019 at 01:33:02PM -0800, Eric Dumazet wrote:
> >>
> >>
> >> On 03/08/2019 01:09 PM, Guillaume Nault wrote:
> >>> @@ -216,7 +216,12 @@ struct sock *tcp_get_cookie_sock(struct sock *sk,
> >>> struct sk_buff *skb,
> >>> refcount_set(&req->rsk_refcnt, 1);
> >>> tcp_sk(child)->tsoffset = tsoff;
> >>> sock_rps_save_rxhash(child, skb);
> >>> - inet_csk_reqsk_queue_add(sk, req, child);
> >>> + if (!inet_csk_reqsk_queue_add(sk, req, child)) {
> >>> + bh_unlock_sock(child);
> >>> + sock_put(child);
> >>> + child = NULL;
> >>> + reqsk_put(req);
> >>
> >> Since we use reqsk_free(req) in the same function, we can use
> >> reqsk_free(req)
> >> here as well ?
> >>
> > That was my first approach, but reqsk_free() doesn't like it:
> >
> > static inline void reqsk_free(struct request_sock *req)
> > {
> > /* temporary debugging */
> > WARN_ON_ONCE(refcount_read(&req->rsk_refcnt) != 0);
> > ...
> > }
>
> Oh right, there is this refcount_set(&req->rsk_refcnt, 1) before the call
> to inet_csk_reqsk_queue_add(sk, req, child);
>
> So just change the TFO case only :)
>
Well.. refcount is 1 in the TFO case too.
Long term, do we want to keep the WARN_ON_ONCE()? If so, we should
probably remove the comment.
