RE: [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Sunday, July 29, 2018 11:48 AM > To: Vakul Garg > Cc: netdev@vger.kernel.org; bor...@mellanox.com; > avia...@mellanox.com; davejwat...@fb.com > Subject: Re: [net-next v6 1/2] net/tls: Use socket data_ready callback on > record availability > > From: Vakul Garg > Date: Sun, 29 Jul 2018 06:01:29 + > > > Could you please correct me if my counter-reasoning behind changing > > the socket callback is wrong? > > Ok, after stufying the code a bit I agree with your analysis. Thanks. Kindly advise, if I need to resubmit/rebase the patch.
Re: [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
From: Vakul Garg Date: Sun, 29 Jul 2018 06:01:29 + > Could you please correct me if my counter-reasoning behind changing > the socket callback is wrong? Ok, after stufying the code a bit I agree with your analysis.
RE: [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
Hi David Could you please correct me if my counter-reasoning behind changing the socket callback is wrong? Thanks & Regards Vakul > -Original Message- > From: Vakul Garg > Sent: Wednesday, July 25, 2018 11:22 AM > To: David Miller > Cc: netdev@vger.kernel.org; bor...@mellanox.com; > avia...@mellanox.com; davejwat...@fb.com > Subject: RE: [net-next v6 1/2] net/tls: Use socket data_ready callback on > record availability > > > > > -Original Message- > > From: David Miller [mailto:da...@davemloft.net] > > Sent: Wednesday, July 25, 2018 1:43 AM > > To: Vakul Garg > > Cc: netdev@vger.kernel.org; bor...@mellanox.com; > avia...@mellanox.com; > > davejwat...@fb.com > > Subject: Re: [net-next v6 1/2] net/tls: Use socket data_ready callback > > on record availability > > > > From: Vakul Garg > > Date: Tue, 24 Jul 2018 15:44:02 +0530 > > > > > On receipt of a complete tls record, use socket's saved data_ready > > > callback instead of state_change callback. > > > > > > Signed-off-by: Vakul Garg > > > > I don't think this is correct. > > > > Here, the stream parser has given us a complete TLS record. > > > > But we haven't decrypted this packet yet. It sits on the stream > > parser's queue to be processed by tls_sw_recvmsg(), not the saved > > socket's receive queue. > > I understand that at this point in code, the TLS record is still queued in > encrypted state. But the decryption happens inline when tls_sw_recvmsg() > gets invokved. > So it should be ok to notify the waiting context about the availability of > data > as soon as we could collect a full TLS record. > > For new data availability notification, sk_data_ready callback should be more > more appropriate. It points to sock_def_readable() which wakes up > specifically for EPOLLIN event. > > This is in contrast to the socket callback sk_state_change which points to > sock_def_wakeup() which issues a wakeup unconditionally (without event > mask).
RE: [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Wednesday, July 25, 2018 1:43 AM > To: Vakul Garg > Cc: netdev@vger.kernel.org; bor...@mellanox.com; > avia...@mellanox.com; davejwat...@fb.com > Subject: Re: [net-next v6 1/2] net/tls: Use socket data_ready callback on > record availability > > From: Vakul Garg > Date: Tue, 24 Jul 2018 15:44:02 +0530 > > > On receipt of a complete tls record, use socket's saved data_ready > > callback instead of state_change callback. > > > > Signed-off-by: Vakul Garg > > I don't think this is correct. > > Here, the stream parser has given us a complete TLS record. > > But we haven't decrypted this packet yet. It sits on the stream parser's > queue to be processed by tls_sw_recvmsg(), not the saved socket's receive > queue. I understand that at this point in code, the TLS record is still queued in encrypted state. But the decryption happens inline when tls_sw_recvmsg() gets invokved. So it should be ok to notify the waiting context about the availability of data as soon as we could collect a full TLS record. For new data availability notification, sk_data_ready callback should be more more appropriate. It points to sock_def_readable() which wakes up specifically for EPOLLIN event. This is in contrast to the socket callback sk_state_change which points to sock_def_wakeup() which issues a wakeup unconditionally (without event mask).
Re: [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
From: Vakul Garg Date: Tue, 24 Jul 2018 15:44:02 +0530 > On receipt of a complete tls record, use socket's saved data_ready > callback instead of state_change callback. > > Signed-off-by: Vakul Garg I don't think this is correct. Here, the stream parser has given us a complete TLS record. But we haven't decrypted this packet yet. It sits on the stream parser's queue to be processed by tls_sw_recvmsg(), not the saved socket's receive queue.
[net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
On receipt of a complete tls record, use socket's saved data_ready callback instead of state_change callback. Signed-off-by: Vakul Garg --- net/tls/tls_sw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 0c2d029c9d4c..fee1240eff92 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1028,7 +1028,7 @@ static void tls_queue(struct strparser *strp, struct sk_buff *skb) ctx->recv_pkt = skb; strp_pause(strp); - strp->sk->sk_state_change(strp->sk); + ctx->saved_data_ready(strp->sk); } static void tls_data_ready(struct sock *sk) -- 2.13.6
