On Tue, 2015-12-01 at 16:04 +0900, Lorenzo Colitti wrote:

> +     /* Don't race with BH socket closes such as inet_csk_listen_stop. */
> +     local_bh_disable();
> +     bh_lock_sock(sk);
> +
> +     if (!sock_flag(sk, SOCK_DEAD)) {
> +             smp_wmb();  /* Be consistent with tcp_reset */

This barrier is absolutely useless here, it must be moved _after_
sk->sk_err = ECONNABORTED.

Since you copied tcp_reset() code, please do not reorder barriers.

Also, comment in tcp_reset() is better IMO

/* This barrier is coupled with smp_rmb() in tcp_poll() */



> +             sk->sk_err = ECONNABORTED;
> +             sk->sk_error_report(sk);

Why not using tcp_need_reset() here, otherwise we might send spurious
RST for some tcp states.

Ideally, packetdrill tests would be nice.

> +             tcp_send_active_reset(sk, GFP_ATOMIC);
> +             tcp_done(sk);
> +     }

Please do not send a v5 before letting other people comment on v3 & v4,
no need to flood netdev with many versions sent the same day.

Thanks.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to