netdev - Re: [PATCH net 2/2] RDS: TCP: Synchrnozize accept() and connect() paths on t_conn

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b103ecc8-e224-b657-4ae9-acb39d9cd4f7@oracle.com>
Date:	Mon, 2 May 2016 11:08:37 -0700
From:	Santosh Shilimkar <santosh.shilimkar@...cle.com>
To:	Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:	netdev@...r.kernel.org, rds-devel@....oracle.com,
	davem@...emloft.net
Subject: Re: [PATCH net 2/2] RDS: TCP: Synchrnozize accept() and connect()
 paths on t_conn_lock.

On 5/2/2016 9:43 AM, Sowmini Varadhan wrote:
> On (05/02/16 09:33), Santosh Shilimkar wrote:
>>> +	mutex_unlock(&tc->t_conn_lock);
>> Just wondering whether the spin_lock() would better here considering
>> entry into rds_tcp_conn_connect() & rds_tcp_accept_one() might be
>> from softirq context. Ignore it if its not applicable.
>
> It's not from softirq context (both are workqs), but I used a mutex
> to follow c_cm_lock (which I considered reusing, given that it
> is only IB specific?) But spin_lock vs mutex may not be a big
> differentiator here- this is really a one-time start up (corner-case)
> issue in the control path.
>
That should be fine then.

>>> 	rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
>> Like patch 1/2, probably we can leverage return value of above.
>      :
>> You probably don't need the local 'conn_state' and below should work.
>> 	if (!rds_conn_connecting(conn) && !rds_conn_up(conn))
>
> see explanation for comment to 1/2.
>
Yep.

 > +rst_nsk:
 > +	/* rest the newly returned accept sock and bail */
s/rest/reset

With typo fixed,
Acked-by: Santosh Shilimkar <santosh.shilimkar@...cle.com>