* [PATCH net-next] strparser: Return if socket does not have required number of bytes
@ 2019-01-30 7:31 Vakul Garg
2019-01-31 5:59 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Vakul Garg @ 2019-01-30 7:31 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, doronrk, Vakul Garg
Function strp_data_ready() should peek the associated socket to check
whether it has the required number of bytes available before queueing
work or initiating socket read via strp_read_sock(). This saves cpu
cycles because strp_read_sock() is called only when required amount of
data is available.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
net/strparser/strparser.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index da1a676860ca..38f8d8d8f4ad 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -384,6 +384,14 @@ void strp_data_ready(struct strparser *strp)
if (unlikely(strp->stopped) || strp->paused)
return;
+ /* If the socket does not contain the number bytes required by
+ * stream parser context to proceed, return silently.
+ */
+ if (strp->need_bytes) {
+ if (strp_peek_len(strp) < strp->need_bytes)
+ return;
+ }
+
/* This check is needed to synchronize with do_strp_work.
* do_strp_work acquires a process lock (lock_sock) whereas
* the lock held here is bh_lock_sock. The two locks can be
@@ -396,11 +404,6 @@ void strp_data_ready(struct strparser *strp)
return;
}
- if (strp->need_bytes) {
- if (strp_peek_len(strp) < strp->need_bytes)
- return;
- }
-
if (strp_read_sock(strp) == -ENOMEM)
queue_work(strp_wq, &strp->work);
}
--
2.13.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] strparser: Return if socket does not have required number of bytes
2019-01-30 7:31 [PATCH net-next] strparser: Return if socket does not have required number of bytes Vakul Garg
@ 2019-01-31 5:59 ` David Miller
2019-01-31 6:25 ` Vakul Garg
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2019-01-31 5:59 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson, doronrk
From: Vakul Garg <vakul.garg@nxp.com>
Date: Wed, 30 Jan 2019 07:31:44 +0000
> Function strp_data_ready() should peek the associated socket to check
> whether it has the required number of bytes available before queueing
> work or initiating socket read via strp_read_sock(). This saves cpu
> cycles because strp_read_sock() is called only when required amount of
> data is available.
>
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
You can't do this, I think. It's racy and the user socket owned check
is absolutely necessary before you do the need bytes check.
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH net-next] strparser: Return if socket does not have required number of bytes
2019-01-31 5:59 ` David Miller
@ 2019-01-31 6:25 ` Vakul Garg
0 siblings, 0 replies; 3+ messages in thread
From: Vakul Garg @ 2019-01-31 6:25 UTC (permalink / raw)
To: David Miller; +Cc: netdev, borisp, aviadye, davejwatson, doronrk
> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Thursday, January 31, 2019 11:30 AM
> To: Vakul Garg <vakul.garg@nxp.com>
> Cc: netdev@vger.kernel.org; borisp@mellanox.com;
> aviadye@mellanox.com; davejwatson@fb.com; doronrk@fb.com
> Subject: Re: [PATCH net-next] strparser: Return if socket does not have
> required number of bytes
>
> From: Vakul Garg <vakul.garg@nxp.com>
> Date: Wed, 30 Jan 2019 07:31:44 +0000
>
> > Function strp_data_ready() should peek the associated socket to check
> > whether it has the required number of bytes available before queueing
> > work or initiating socket read via strp_read_sock(). This saves cpu
> > cycles because strp_read_sock() is called only when required amount of
> > data is available.
> >
> > Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
>
> You can't do this, I think. It's racy and the user socket owned check is
> absolutely necessary before you do the need bytes check.
Do you mean to say that 'peek_len' operation can't be invoked if the socket is already owned by some other context?
My understanding by reading following link is that we can to peek_len operation without acquiring sock lock.
https://elixir.bootlin.com/linux/v5.0-rc4/source/include/linux/net.h#L191
Can you please kindly elaborate the problem?
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-01-31 6:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-30 7:31 [PATCH net-next] strparser: Return if socket does not have required number of bytes Vakul Garg
2019-01-31 5:59 ` David Miller
2019-01-31 6:25 ` Vakul Garg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.