* [net-next v6 0/2] Minor code cleanup patches
@ 2018-07-24 10:14 Vakul Garg
2018-07-24 10:14 ` [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability Vakul Garg
2018-07-24 10:14 ` [net-next v6 2/2] net/tls: Remove redundant variable assignments and wakeup Vakul Garg
0 siblings, 2 replies; 8+ messages in thread
From: Vakul Garg @ 2018-07-24 10:14 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
This patch series improves tls_sw.c code by:
1) Using correct socket callback for flagging data availability.
2) Removing redundant variable assignments and wakeup callbacks.
Vakul Garg (2):
net/tls: Use socket data_ready callback on record availability
net/tls: Remove redundant variable assignments and wakeup
net/tls/tls_sw.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
--
2.13.6
^ permalink raw reply [flat|nested] 8+ messages in thread
* [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
2018-07-24 10:14 [net-next v6 0/2] Minor code cleanup patches Vakul Garg
@ 2018-07-24 10:14 ` Vakul Garg
2018-07-24 20:12 ` David Miller
2018-07-24 10:14 ` [net-next v6 2/2] net/tls: Remove redundant variable assignments and wakeup Vakul Garg
1 sibling, 1 reply; 8+ messages in thread
From: Vakul Garg @ 2018-07-24 10:14 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
On receipt of a complete tls record, use socket's saved data_ready
callback instead of state_change callback.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [net-next v6 2/2] net/tls: Remove redundant variable assignments and wakeup
2018-07-24 10:14 [net-next v6 0/2] Minor code cleanup patches Vakul Garg
2018-07-24 10:14 ` [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability Vakul Garg
@ 2018-07-24 10:14 ` Vakul Garg
1 sibling, 0 replies; 8+ messages in thread
From: Vakul Garg @ 2018-07-24 10:14 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In function decrypt_skb_update(), the assignment to tls receive context
variable 'decrypted' is redundant as the same is being done in function
tls_sw_recvmsg() after calling decrypt_skb_update(). Also calling callback
function to wakeup processes sleeping on socket data availability is
useless as decrypt_skb_update() is invoked from user processes only. This
patch cleans these up.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
net/tls/tls_sw.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index fee1240eff92..6c71da7b147f 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -679,8 +679,6 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
rxm->offset += tls_ctx->rx.prepend_size;
rxm->full_len -= tls_ctx->rx.overhead_size;
tls_advance_record_sn(sk, &tls_ctx->rx);
- ctx->decrypted = true;
- ctx->saved_data_ready(sk);
return err;
}
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
2018-07-24 10:14 ` [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability Vakul Garg
@ 2018-07-24 20:12 ` David Miller
2018-07-25 5:51 ` Vakul Garg
0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2018-07-24 20:12 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson
From: Vakul Garg <vakul.garg@nxp.com>
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 <vakul.garg@nxp.com>
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
2018-07-24 20:12 ` David Miller
@ 2018-07-25 5:51 ` Vakul Garg
2018-07-29 6:01 ` Vakul Garg
0 siblings, 1 reply; 8+ messages in thread
From: Vakul Garg @ 2018-07-25 5:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, borisp, aviadye, davejwatson
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, July 25, 2018 1:43 AM
> To: Vakul Garg <vakul.garg@nxp.com>
> Cc: netdev@vger.kernel.org; borisp@mellanox.com;
> aviadye@mellanox.com; davejwatson@fb.com
> Subject: Re: [net-next v6 1/2] net/tls: Use socket data_ready callback on
> record availability
>
> From: Vakul Garg <vakul.garg@nxp.com>
> 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 <vakul.garg@nxp.com>
>
> 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).
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
2018-07-25 5:51 ` Vakul Garg
@ 2018-07-29 6:01 ` Vakul Garg
2018-07-29 6:17 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Vakul Garg @ 2018-07-29 6:01 UTC (permalink / raw)
To: David Miller; +Cc: netdev, borisp, aviadye, davejwatson
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 <davem@davemloft.net>
> Cc: netdev@vger.kernel.org; borisp@mellanox.com;
> aviadye@mellanox.com; davejwatson@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:davem@davemloft.net]
> > Sent: Wednesday, July 25, 2018 1:43 AM
> > To: Vakul Garg <vakul.garg@nxp.com>
> > Cc: netdev@vger.kernel.org; borisp@mellanox.com;
> aviadye@mellanox.com;
> > davejwatson@fb.com
> > Subject: Re: [net-next v6 1/2] net/tls: Use socket data_ready callback
> > on record availability
> >
> > From: Vakul Garg <vakul.garg@nxp.com>
> > 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 <vakul.garg@nxp.com>
> >
> > 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).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
2018-07-29 6:01 ` Vakul Garg
@ 2018-07-29 6:17 ` David Miller
2018-07-29 6:20 ` Vakul Garg
0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2018-07-29 6:17 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson
From: Vakul Garg <vakul.garg@nxp.com>
Date: Sun, 29 Jul 2018 06:01:29 +0000
> 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability
2018-07-29 6:17 ` David Miller
@ 2018-07-29 6:20 ` Vakul Garg
0 siblings, 0 replies; 8+ messages in thread
From: Vakul Garg @ 2018-07-29 6:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev, borisp, aviadye, davejwatson
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Sunday, July 29, 2018 11:48 AM
> To: Vakul Garg <vakul.garg@nxp.com>
> Cc: netdev@vger.kernel.org; borisp@mellanox.com;
> aviadye@mellanox.com; davejwatson@fb.com
> Subject: Re: [net-next v6 1/2] net/tls: Use socket data_ready callback on
> record availability
>
> From: Vakul Garg <vakul.garg@nxp.com>
> Date: Sun, 29 Jul 2018 06:01:29 +0000
>
> > 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-07-29 7:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-24 10:14 [net-next v6 0/2] Minor code cleanup patches Vakul Garg
2018-07-24 10:14 ` [net-next v6 1/2] net/tls: Use socket data_ready callback on record availability Vakul Garg
2018-07-24 20:12 ` David Miller
2018-07-25 5:51 ` Vakul Garg
2018-07-29 6:01 ` Vakul Garg
2018-07-29 6:17 ` David Miller
2018-07-29 6:20 ` Vakul Garg
2018-07-24 10:14 ` [net-next v6 2/2] net/tls: Remove redundant variable assignments and wakeup Vakul Garg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).