Re: Concurrent logins to different interfaces of same iscsi target and login timeout
Hi Lee,
Thanks for adding the async login support to upstream. I've ran some tests
using the iscsiadm built from there
and would like to ask:
1. How is it possible to gather the async logins return status? if
understood correctly, the proposed way
is to lookup for the connections in the output of "iscsiadm -m session"
after the async logins were launched.
Currently, I am using a sampling loop, checking at 1 second intervals the
output of iscsiadm -m session for
presence of expected connections targets and portals and breaks if all were
found or not found within
the expected timeout interval, which for the default iscsi settings is
considered as following:
(120 seconds timeout per connection login) * (number of connections) /
(number of workers)
Is there a better way? I am not sure how to gather the error status when a
connection not able to login in such case.
2. Would it also be supported for non-login-all mode? For "iscsiadm -m node
-T target -p portal -I interface --login"
I get same timeouts with/without the --no-wait flag, meaning the test waits
240 seconds in case two connections
are down when using a single node login worker for both cases, so I assume
it currently doesn't apply for this login mode.
-- Simulating one portal down (2 connections down) with one worker, using
node login without --no-wait
# python3 ./initiator.py -j 1 -i 10.35.18.220 10.35.18.156 -d
10.35.18.156
2020-08-18 15:59:01,874 INFO(MainThread) Removing prior sessions and
nodes
2020-08-18 15:59:01,882 INFO(MainThread) Deleting all nodes
2020-08-18 15:59:01,893 INFO(MainThread) No active sessions
2020-08-18 15:59:01,943 INFO(MainThread) Setting 10.35.18.156 as
invalid address for target iqn.2003-01.org.vm-18-220.iqn2
2020-08-18 15:59:01,943 INFO(MainThread) Setting 10.35.18.156 as
invalid address for target iqn.2003-01.org.vm-18-220.iqn1
2020-08-18 15:59:01,943 INFO(MainThread) Discovered connections:
{('iqn.2003-01.org.vm-18-220.iqn1', '0.0.0.0:0,0'),
('iqn.2003-01.org.vm-18-220.iqn2', '0.0.0.0:0,0'),
('iqn.2003-01.org.vm-18-220.iqn2', '10.35.18.220:3260,1'),
('iqn.2003-01.org.vm-18-220.iqn1', '10.35.18.220:3260,1')}
2020-08-18 15:59:01,944 INFO(MainThread) Adding node for target
iqn.2003-01.org.vm-18-220.iqn1 portal 0.0.0.0:0,0
2020-08-18 15:59:01,956 INFO(MainThread) Adding node for target
iqn.2003-01.org.vm-18-220.iqn2 portal 0.0.0.0:0,0
2020-08-18 15:59:01,968 INFO(MainThread) Adding node for target
iqn.2003-01.org.vm-18-220.iqn2 portal 10.35.18.220:3260,1
2020-08-18 15:59:01,980 INFO(MainThread) Adding node for target
iqn.2003-01.org.vm-18-220.iqn1 portal 10.35.18.220:3260,1
2020-08-18 15:59:01,995 INFO(login_0) Login to target
iqn.2003-01.org.vm-18-220.iqn1 portal 0.0.0.0:0,0 (nowait=False)
2020-08-18 16:01:02,019 INFO(login_0) Login to target
iqn.2003-01.org.vm-18-220.iqn2 portal 0.0.0.0:0,0 (nowait=False)
2020-08-18 16:01:02,028 ERROR (MainThread) Job failed: Command
['iscsiadm', '--mode', 'node', '--targetname',
'iqn.2003-01.org.vm-18-220.iqn1', '--interface', 'default', '--portal',
'0.0.0.0:0,0', '--login'] failed rc=8 out='Logging in to [iface: default,
target: iqn.2003-01.org.vm-18-220.iqn1, portal: 0.0.0.0,0]' err='iscsiadm:
Could not login to [iface: default, target: iqn.2003-01.org.vm-18-220.iqn1,
portal: 0.0.0.0,0].\niscsiadm: initiator reported error (8 - connection
timed out)\niscsiadm: Could not log into all portals'
2020-08-18 16:03:02,045 INFO(login_0) Login to target
iqn.2003-01.org.vm-18-220.iqn2 portal 10.35.18.220:3260,1 (nowait=False)
2020-08-18 16:03:02,053 ERROR (MainThread) Job failed: Command
['iscsiadm', '--mode', 'node', '--targetname',
'iqn.2003-01.org.vm-18-220.iqn2', '--interface', 'default', '--portal',
'0.0.0.0:0,0', '--login'] failed rc=8 out='Logging in to [iface: default,
target: iqn.2003-01.org.vm-18-220.iqn2, portal: 0.0.0.0,0]' err='iscsiadm:
Could not login to [iface: default, target: iqn.2003-01.org.vm-18-220.iqn2,
portal: 0.0.0.0,0].\niscsiadm: initiator reported error (8 - connection
timed out)\niscsiadm: Could not log into all portals'
2020-08-18 16:03:02,321 INFO(login_0) Login to target
iqn.2003-01.org.vm-18-220.iqn1 portal 10.35.18.220:3260,1 (nowait=False)
2020-08-18 16:03:02,695 INFO(MainThread) Connecting completed in
240.752s
-- Simulating one portal down (2 connections down) with one worker, using
node login with --no-wait
# python3 ./initiator.py -j 1 -i 10.35.18.220 10.35.18.156 -d
10.35.18.156 --nowait
2020-08-18 16:16:05,802 INFO(MainThread) Removing prior sessions and
nodes
2020-08-18 16:16:06,075 INFO(MainThread) Deleting all nodes
2020-08-18 16:16:06,090 INFO(MainThread) No active sessions
2020-08-18 16:16:06,130 INFO(MainThread) Setting 10.35.18.156 as
invalid address for target iqn.2003-01.org.vm-18-220.iqn2
2020-08-18 16:16:06,131 INFO(MainThread) Setting 10.35.18.156 as
invalid address for target iqn.2003-01.org.vm-18
Re: [PATCH v7 1/6] net: introduce helper sendpage_ok() in include/linux/net.h
I think we should go for something simple like this instead:
---
>From 4867e158ee86ebd801b4c267e8f8a4a762a71343 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Tue, 18 Aug 2020 18:19:23 +0200
Subject: net: bypass ->sendpage for slab pages
Sending Slab or tail pages into ->sendpage will cause really strange
delayed oops. Prevent it right in the networking code instead of
requiring drivers to work around the fact.
Signed-off-by: Christoph Hellwig
---
net/socket.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/socket.c b/net/socket.c
index dbbe8ea7d395da..fbc82eb96d18ce 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3638,7 +3638,12 @@ EXPORT_SYMBOL(kernel_getpeername);
int kernel_sendpage(struct socket *sock, struct page *page, int offset,
size_t size, int flags)
{
- if (sock->ops->sendpage)
+ /*
+* sendpage does manipulates the refcount of the passed in page, which
+* does not work for Slab pages, or for tails of non-__GFP_COMP
+* high order pages.
+*/
+ if (sock->ops->sendpage && !PageSlab(page) && page_count(page) > 0)
return sock->ops->sendpage(sock, page, offset, size, flags);
return sock_no_sendpage(sock, page, offset, size, flags);
--
2.28.0
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818162404.GA27196%40lst.de.
Re: [PATCH v7 1/6] net: introduce helper sendpage_ok() in include/linux/net.h
On Wed, Aug 19, 2020 at 12:33:37AM +0800, Coly Li wrote: > On 2020/8/19 00:24, Christoph Hellwig wrote: > > I think we should go for something simple like this instead: > > This idea is fine to me. Should a warning message be through here? IMHO > the driver still sends an improper page in, fix it in silence is too > kind or over nice to the buggy driver(s). I don't think a warning is a good idea. An API that does the right thing underneath and doesn't require boiler plate code in most callers is the right API. -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/20200818194930.GA31966%40lst.de.
[PATCH v6 1/6] net: introduce helper sendpage_ok() in include/linux/net.h
The original problem was from nvme-over-tcp code, who mistakenly uses
kernel_sendpage() to send pages allocated by __get_free_pages() without
__GFP_COMP flag. Such pages don't have refcount (page_count is 0) on
tail pages, sending them by kernel_sendpage() may trigger a kernel panic
from a corrupted kernel heap, because these pages are incorrectly freed
in network stack as page_count 0 pages.
This patch introduces a helper sendpage_ok(), it returns true if the
checking page,
- is not slab page: PageSlab(page) is false.
- has page refcount: page_count(page) is not zero
All drivers who want to send page to remote end by kernel_sendpage()
may use this helper to check whether the page is OK. If the helper does
not return true, the driver should try other non sendpage method (e.g.
sock_no_sendpage()) to handle the page.
Signed-off-by: Coly Li
Cc: Chaitanya Kulkarni
Cc: Christoph Hellwig
Cc: Hannes Reinecke
Cc: Jan Kara
Cc: Jens Axboe
Cc: Mikhail Skorzhinskii
Cc: Philipp Reisner
Cc: Sagi Grimberg
Cc: Vlastimil Babka
Cc: sta...@vger.kernel.org
---
include/linux/net.h | 16
1 file changed, 16 insertions(+)
diff --git a/include/linux/net.h b/include/linux/net.h
index d48ff1180879..a807fad31958 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -286,6 +287,21 @@ do {
\
#define net_get_random_once_wait(buf, nbytes) \
get_random_once_wait((buf), (nbytes))
+/*
+ * E.g. XFS meta- & log-data is in slab pages, or bcache meta
+ * data pages, or other high order pages allocated by
+ * __get_free_pages() without __GFP_COMP, which have a page_count
+ * of 0 and/or have PageSlab() set. We cannot use send_page for
+ * those, as that does get_page(); put_page(); and would cause
+ * either a VM_BUG directly, or __page_cache_release a page that
+ * would actually still be referenced by someone, leading to some
+ * obscure delayed Oops somewhere else.
+ */
+static inline bool sendpage_ok(struct page *page)
+{
+ return (!PageSlab(page) && page_count(page) >= 1);
+}
+
int kernel_sendmsg(struct socket *sock, struct msghdr *msg, struct kvec *vec,
size_t num, size_t len);
int kernel_sendmsg_locked(struct sock *sk, struct msghdr *msg,
--
2.26.2
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818124736.5790-2-colyli%40suse.de.
[PATCH v7 6/6] libceph: use sendpage_ok() in ceph_tcp_sendpage()
In libceph, ceph_tcp_sendpage() does the following checks before handle the page by network layer's zero copy sendpage method, if (page_count(page) >= 1 && !PageSlab(page)) This check is exactly what sendpage_ok() does. This patch replace the open coded checks by sendpage_ok() as a code cleanup. Signed-off-by: Coly Li Cc: Ilya Dryomov Cc: Jeff Layton --- net/ceph/messenger.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index 27d6ab11f9ee..6a349da7f013 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -575,7 +575,7 @@ static int ceph_tcp_sendpage(struct socket *sock, struct page *page, * coalescing neighboring slab objects into a single frag which * triggers one of hardened usercopy checks. */ - if (page_count(page) >= 1 && !PageSlab(page)) + if (sendpage_ok(page)) sendpage = sock->ops->sendpage; else sendpage = sock_no_sendpage; -- 2.26.2 -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/20200818131227.37020-7-colyli%40suse.de.
[PATCH v7 2/6] nvme-tcp: check page by sendpage_ok() before calling kernel_sendpage()
Currently nvme_tcp_try_send_data() doesn't use kernel_sendpage() to
send slab pages. But for pages allocated by __get_free_pages() without
__GFP_COMP, which also have refcount as 0, they are still sent by
kernel_sendpage() to remote end, this is problematic.
The new introduced helper sendpage_ok() checks both PageSlab tag and
page_count counter, and returns true if the checking page is OK to be
sent by kernel_sendpage().
This patch fixes the page checking issue of nvme_tcp_try_send_data()
with sendpage_ok(). If sendpage_ok() returns true, send this page by
kernel_sendpage(), otherwise use sock_no_sendpage to handle this page.
Signed-off-by: Coly Li
Cc: Chaitanya Kulkarni
Cc: Christoph Hellwig
Cc: Hannes Reinecke
Cc: Jan Kara
Cc: Jens Axboe
Cc: Mikhail Skorzhinskii
Cc: Philipp Reisner
Cc: Sagi Grimberg
Cc: Vlastimil Babka
Cc: sta...@vger.kernel.org
---
drivers/nvme/host/tcp.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 62fbaecdc960..902fe742762b 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -912,12 +912,11 @@ static int nvme_tcp_try_send_data(struct nvme_tcp_request
*req)
else
flags |= MSG_MORE | MSG_SENDPAGE_NOTLAST;
- /* can't zcopy slab pages */
- if (unlikely(PageSlab(page))) {
- ret = sock_no_sendpage(queue->sock, page, offset, len,
+ if (sendpage_ok(page)) {
+ ret = kernel_sendpage(queue->sock, page, offset, len,
flags);
} else {
- ret = kernel_sendpage(queue->sock, page, offset, len,
+ ret = sock_no_sendpage(queue->sock, page, offset, len,
flags);
}
if (ret <= 0)
--
2.26.2
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818131227.37020-3-colyli%40suse.de.
[PATCH v6 0/6] Introduce sendpage_ok() to detect misused sendpage in network related drivers
This series was original by a bug fix in nvme-over-tcp driver which only
checked whether a page was allocated from slab allcoator, but forgot to
check its page_count: The page handled by sendpage should be neither a
Slab page nor 0 page_count page.
As Sagi Grimberg suggested, the original fix is refind to a more common
inline routine:
static inline bool sendpage_ok(struct page *page)
{
return (!PageSlab(page) && page_count(page) >= 1);
}
If sendpage_ok() returns true, the checking page can be handled by the
zero copy sendpage method in network layer.
The first patch in this series introduces sendpage_ok() in header file
include/linux/net.h, the second patch fixes the page checking issue in
nvme-over-tcp driver, the third patch adds page_count check by using
sendpage_ok() in do_tcp_sendpages() as Eric Dumazet suggested, and all
rested patches just replace existing open coded checks with the inline
sendpage_ok() routine.
Coly Li
Cc: Chaitanya Kulkarni
Cc: Chris Leech
Cc: Christoph Hellwig
Cc: Cong Wang
Cc: David S. Miller
Cc: Hannes Reinecke
Cc: Ilya Dryomov
Cc: Jan Kara
Cc: Jeff Layton
Cc: Jens Axboe
Cc: Lee Duncan
Cc: Mike Christie
Cc: Mikhail Skorzhinskii
Cc: Philipp Reisner
Cc: Sagi Grimberg
Cc: sta...@vger.kernel.org
Cc: Vasily Averin
Cc: Vlastimil Babka
---
Changelog:
v6: fix page check in do_tcp_sendpages(), and replace other open coded
checks with sendpage_ok() in libceph, iscsi drivers.
v5, include linux/mm.h in include/linux/net.h
v4, change sendpage_ok() as an inline helper, and post it as
separate patch.
v3, introduce a more common sendpage_ok()
v2, fix typo in patch subject
v1, the initial version.
Coly Li (6):
net: introduce helper sendpage_ok() in include/linux/net.h
nvme-tcp: check page by sendpage_ok() before calling kernel_sendpage()
tcp: use sendpage_ok() to detect misused .sendpage
drbd: code cleanup by using sendpage_ok() to check page for
kernel_sendpage()
scsi: libiscsi: use sendpage_ok() in iscsi_tcp_segment_map()
libceph: use sendpage_ok() in ceph_tcp_sendpage()
drivers/block/drbd/drbd_main.c | 2 +-
drivers/nvme/host/tcp.c| 7 +++
drivers/scsi/libiscsi_tcp.c| 2 +-
include/linux/net.h| 16
net/ceph/messenger.c | 2 +-
net/ipv4/tcp.c | 3 ++-
6 files changed, 24 insertions(+), 8 deletions(-)
--
2.26.2
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818124736.5790-1-colyli%40suse.de.
[PATCH v6 5/6] scsi: libiscsi: use sendpage_ok() in iscsi_tcp_segment_map()
In iscsci driver, iscsi_tcp_segment_map() uses the following code to
check whether the page should or not be handled by sendpage:
if (!recv && page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)))
The "page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)" part is to
make sure the page can be sent to network layer's zero copy path. This
part is exactly what sendpage_ok() does.
This patch uses use sendpage_ok() in iscsi_tcp_segment_map() to replace
the original open coded checks.
Signed-off-by: Coly Li
Cc: Vasily Averin
Cc: Cong Wang
Cc: Mike Christie
Cc: Lee Duncan
Cc: Chris Leech
Cc: Christoph Hellwig
Cc: Hannes Reinecke
---
drivers/scsi/libiscsi_tcp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index 6ef93c7af954..31cd8487c16e 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -128,7 +128,7 @@ static void iscsi_tcp_segment_map(struct iscsi_segment
*segment, int recv)
* coalescing neighboring slab objects into a single frag which
* triggers one of hardened usercopy checks.
*/
- if (!recv && page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)))
+ if (!recv && sendpage_ok(sg_page(sg)))
return;
if (recv) {
--
2.26.2
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818124736.5790-6-colyli%40suse.de.
Re: [PATCH v7 1/6] net: introduce helper sendpage_ok() in include/linux/net.h
On 2020/8/19 00:24, Christoph Hellwig wrote:
> I think we should go for something simple like this instead:
This idea is fine to me. Should a warning message be through here? IMHO
the driver still sends an improper page in, fix it in silence is too
kind or over nice to the buggy driver(s).
And maybe the fix in nvme-tcp driver and do_tcp_sendpages() are still
necessary. I am not network expert, this is my opinion for reference.
Coly Li
> ---
> From 4867e158ee86ebd801b4c267e8f8a4a762a71343 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig
> Date: Tue, 18 Aug 2020 18:19:23 +0200
> Subject: net: bypass ->sendpage for slab pages
>
> Sending Slab or tail pages into ->sendpage will cause really strange
> delayed oops. Prevent it right in the networking code instead of
> requiring drivers to work around the fact.
>
> Signed-off-by: Christoph Hellwig
> ---
> net/socket.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/socket.c b/net/socket.c
> index dbbe8ea7d395da..fbc82eb96d18ce 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -3638,7 +3638,12 @@ EXPORT_SYMBOL(kernel_getpeername);
> int kernel_sendpage(struct socket *sock, struct page *page, int offset,
> size_t size, int flags)
> {
> - if (sock->ops->sendpage)
> + /*
> + * sendpage does manipulates the refcount of the passed in page, which
> + * does not work for Slab pages, or for tails of non-__GFP_COMP
> + * high order pages.
> + */
> + if (sock->ops->sendpage && !PageSlab(page) && page_count(page) > 0)
> return sock->ops->sendpage(sock, page, offset, size, flags);
>
> return sock_no_sendpage(sock, page, offset, size, flags);
>
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/217ec0ec-3c5a-a8ed-27d9-c634f0b9a045%40suse.de.
[PATCH v7 1/6] net: introduce helper sendpage_ok() in include/linux/net.h
The original problem was from nvme-over-tcp code, who mistakenly uses
kernel_sendpage() to send pages allocated by __get_free_pages() without
__GFP_COMP flag. Such pages don't have refcount (page_count is 0) on
tail pages, sending them by kernel_sendpage() may trigger a kernel panic
from a corrupted kernel heap, because these pages are incorrectly freed
in network stack as page_count 0 pages.
This patch introduces a helper sendpage_ok(), it returns true if the
checking page,
- is not slab page: PageSlab(page) is false.
- has page refcount: page_count(page) is not zero
All drivers who want to send page to remote end by kernel_sendpage()
may use this helper to check whether the page is OK. If the helper does
not return true, the driver should try other non sendpage method (e.g.
sock_no_sendpage()) to handle the page.
Signed-off-by: Coly Li
Cc: Chaitanya Kulkarni
Cc: Christoph Hellwig
Cc: Hannes Reinecke
Cc: Jan Kara
Cc: Jens Axboe
Cc: Mikhail Skorzhinskii
Cc: Philipp Reisner
Cc: Sagi Grimberg
Cc: Vlastimil Babka
Cc: sta...@vger.kernel.org
---
include/linux/net.h | 16
1 file changed, 16 insertions(+)
diff --git a/include/linux/net.h b/include/linux/net.h
index d48ff1180879..05db8690f67e 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -286,6 +287,21 @@ do {
\
#define net_get_random_once_wait(buf, nbytes) \
get_random_once_wait((buf), (nbytes))
+/*
+ * E.g. XFS meta- & log-data is in slab pages, or bcache meta
+ * data pages, or other high order pages allocated by
+ * __get_free_pages() without __GFP_COMP, which have a page_count
+ * of 0 and/or have PageSlab() set. We cannot use send_page for
+ * those, as that does get_page(); put_page(); and would cause
+ * either a VM_BUG directly, or __page_cache_release a page that
+ * would actually still be referenced by someone, leading to some
+ * obscure delayed Oops somewhere else.
+ */
+static inline bool sendpage_ok(struct page *page)
+{
+ return !PageSlab(page) && page_count(page) >= 1;
+}
+
int kernel_sendmsg(struct socket *sock, struct msghdr *msg, struct kvec *vec,
size_t num, size_t len);
int kernel_sendmsg_locked(struct sock *sk, struct msghdr *msg,
--
2.26.2
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818131227.37020-2-colyli%40suse.de.
[PATCH v6 4/6] drbd: code cleanup by using sendpage_ok() to check page for kernel_sendpage()
In _drbd_send_page() a page is checked by following code before sending it by kernel_sendpage(), (page_count(page) < 1) || PageSlab(page) If the check is true, this page won't be send by kernel_sendpage() and handled by sock_no_sendpage(). This kind of check is exactly what macro sendpage_ok() does, which is introduced into include/linux/net.h to solve a similar send page issue in nvme-tcp code. This patch uses macro sendpage_ok() to replace the open coded checks to page type and refcount in _drbd_send_page(), as a code cleanup. Signed-off-by: Coly Li Cc: Philipp Reisner Cc: Sagi Grimberg --- drivers/block/drbd/drbd_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c index cb687ccdbd96..55dc0c91781e 100644 --- a/drivers/block/drbd/drbd_main.c +++ b/drivers/block/drbd/drbd_main.c @@ -1553,7 +1553,7 @@ static int _drbd_send_page(struct drbd_peer_device *peer_device, struct page *pa * put_page(); and would cause either a VM_BUG directly, or * __page_cache_release a page that would actually still be referenced * by someone, leading to some obscure delayed Oops somewhere else. */ - if (drbd_disable_sendpage || (page_count(page) < 1) || PageSlab(page)) + if (drbd_disable_sendpage || !sendpage_ok(page)) return _drbd_no_send_page(peer_device, page, offset, size, msg_flags); msg_flags |= MSG_NOSIGNAL; -- 2.26.2 -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/20200818124736.5790-5-colyli%40suse.de.
[PATCH v6 2/6] nvme-tcp: check page by sendpage_ok() before calling kernel_sendpage()
Currently nvme_tcp_try_send_data() doesn't use kernel_sendpage() to
send slab pages. But for pages allocated by __get_free_pages() without
__GFP_COMP, which also have refcount as 0, they are still sent by
kernel_sendpage() to remote end, this is problematic.
The new introduced helper sendpage_ok() checks both PageSlab tag and
page_count counter, and returns true if the checking page is OK to be
sent by kernel_sendpage().
This patch fixes the page checking issue of nvme_tcp_try_send_data()
with sendpage_ok(). If sendpage_ok() returns true, send this page by
kernel_sendpage(), otherwise use sock_no_sendpage to handle this page.
Signed-off-by: Coly Li
Cc: Chaitanya Kulkarni
Cc: Christoph Hellwig
Cc: Hannes Reinecke
Cc: Jan Kara
Cc: Jens Axboe
Cc: Mikhail Skorzhinskii
Cc: Philipp Reisner
Cc: Sagi Grimberg
Cc: Vlastimil Babka
Cc: sta...@vger.kernel.org
---
drivers/nvme/host/tcp.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 62fbaecdc960..902fe742762b 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -912,12 +912,11 @@ static int nvme_tcp_try_send_data(struct nvme_tcp_request
*req)
else
flags |= MSG_MORE | MSG_SENDPAGE_NOTLAST;
- /* can't zcopy slab pages */
- if (unlikely(PageSlab(page))) {
- ret = sock_no_sendpage(queue->sock, page, offset, len,
+ if (sendpage_ok(page)) {
+ ret = kernel_sendpage(queue->sock, page, offset, len,
flags);
} else {
- ret = kernel_sendpage(queue->sock, page, offset, len,
+ ret = sock_no_sendpage(queue->sock, page, offset, len,
flags);
}
if (ret <= 0)
--
2.26.2
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818124736.5790-3-colyli%40suse.de.
[PATCH v7 4/6] drbd: code cleanup by using sendpage_ok() to check page for kernel_sendpage()
In _drbd_send_page() a page is checked by following code before sending it by kernel_sendpage(), (page_count(page) < 1) || PageSlab(page) If the check is true, this page won't be send by kernel_sendpage() and handled by sock_no_sendpage(). This kind of check is exactly what macro sendpage_ok() does, which is introduced into include/linux/net.h to solve a similar send page issue in nvme-tcp code. This patch uses macro sendpage_ok() to replace the open coded checks to page type and refcount in _drbd_send_page(), as a code cleanup. Signed-off-by: Coly Li Cc: Philipp Reisner Cc: Sagi Grimberg --- drivers/block/drbd/drbd_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c index cb687ccdbd96..55dc0c91781e 100644 --- a/drivers/block/drbd/drbd_main.c +++ b/drivers/block/drbd/drbd_main.c @@ -1553,7 +1553,7 @@ static int _drbd_send_page(struct drbd_peer_device *peer_device, struct page *pa * put_page(); and would cause either a VM_BUG directly, or * __page_cache_release a page that would actually still be referenced * by someone, leading to some obscure delayed Oops somewhere else. */ - if (drbd_disable_sendpage || (page_count(page) < 1) || PageSlab(page)) + if (drbd_disable_sendpage || !sendpage_ok(page)) return _drbd_no_send_page(peer_device, page, offset, size, msg_flags); msg_flags |= MSG_NOSIGNAL; -- 2.26.2 -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/20200818131227.37020-5-colyli%40suse.de.
[PATCH v7 3/6] tcp: use sendpage_ok() to detect misused .sendpage
commit a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab
objects") adds the checks for Slab pages, but the pages don't have
page_count are still missing from the check.
Network layer's sendpage method is not designed to send page_count 0
pages neither, therefore both PageSlab() and page_count() should be
both checked for the sending page. This is exactly what sendpage_ok()
does.
This patch uses sendpage_ok() in do_tcp_sendpages() to detect misused
.sendpage, to make the code more robust.
Fixes: a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab objects")
Suggested-by: Eric Dumazet
Signed-off-by: Coly Li
Cc: Vasily Averin
Cc: David S. Miller
Cc: sta...@vger.kernel.org
---
net/ipv4/tcp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 31f3b858db81..2135ee7c806d 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -970,7 +970,8 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page
*page, int offset,
long timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
if (IS_ENABLED(CONFIG_DEBUG_VM) &&
- WARN_ONCE(PageSlab(page), "page must not be a Slab one"))
+ WARN_ONCE(!sendpage_ok(page),
+ "page must not be a Slab one and have page_count > 0"))
return -EINVAL;
/* Wait for a connection to finish. One exception is TCP Fast Open
--
2.26.2
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818131227.37020-4-colyli%40suse.de.
[PATCH v6 3/6] tcp: use sendpage_ok() to detect misused .sendpage
commit a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab
objects") adds the checks for Slab pages, but the pages don't have
page_count are still missing from the check.
Network layer's sendpage method is not designed to send page_count 0
pages neither, therefore both PageSlab() and page_count() should be
both checked for the sending page. This is exactly what sendpage_ok()
does.
This patch uses sendpage_ok() in do_tcp_sendpages() to detect misused
.sendpage, to make the code more robust.
Fixes: a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab objects")
Suggested-by: Eric Dumazet
Signed-off-by: Coly Li
Cc: Vasily Averin
Cc: David S. Miller
Cc: sta...@vger.kernel.org
---
net/ipv4/tcp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 31f3b858db81..d96c7549895a 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -970,7 +970,8 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page
*page, int offset,
long timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
if (IS_ENABLED(CONFIG_DEBUG_VM) &&
- WARN_ONCE(PageSlab(page), "page must not be a Slab one"))
+ WARN_ONCE(!sendpage_ok(page)),
+ "page must not be a Slab one and have page_count > 0")
return -EINVAL;
/* Wait for a connection to finish. One exception is TCP Fast Open
--
2.26.2
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818124736.5790-4-colyli%40suse.de.
[PATCH v6 6/6] libceph: use sendpage_ok() in ceph_tcp_sendpage()
In libceph, ceph_tcp_sendpage() does the following checks before handle the page by network layer's zero copy sendpage method, if (page_count(page) >= 1 && !PageSlab(page)) This check is exactly what sendpage_ok() does. This patch replace the open coded checks by sendpage_ok() as a code cleanup. Signed-off-by: Coly Li Cc: Ilya Dryomov Cc: Jeff Layton --- net/ceph/messenger.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index 27d6ab11f9ee..6a349da7f013 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -575,7 +575,7 @@ static int ceph_tcp_sendpage(struct socket *sock, struct page *page, * coalescing neighboring slab objects into a single frag which * triggers one of hardened usercopy checks. */ - if (page_count(page) >= 1 && !PageSlab(page)) + if (sendpage_ok(page)) sendpage = sock->ops->sendpage; else sendpage = sock_no_sendpage; -- 2.26.2 -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/20200818124736.5790-7-colyli%40suse.de.
[PATCH v7 0/6] Introduce sendpage_ok() to detect misused sendpage in network related drivers
This series was original by a bug fix in nvme-over-tcp driver which only
checked whether a page was allocated from slab allcoator, but forgot to
check its page_count: The page handled by sendpage should be neither a
Slab page nor 0 page_count page.
As Sagi Grimberg suggested, the original fix is refind to a more common
inline routine:
static inline bool sendpage_ok(struct page *page)
{
return (!PageSlab(page) && page_count(page) >= 1);
}
If sendpage_ok() returns true, the checking page can be handled by the
zero copy sendpage method in network layer.
The first patch in this series introduces sendpage_ok() in header file
include/linux/net.h, the second patch fixes the page checking issue in
nvme-over-tcp driver, the third patch adds page_count check by using
sendpage_ok() in do_tcp_sendpages() as Eric Dumazet suggested, and all
rested patches just replace existing open coded checks with the inline
sendpage_ok() routine.
Coly Li
Cc: Chaitanya Kulkarni
Cc: Chris Leech
Cc: Christoph Hellwig
Cc: Cong Wang
Cc: David S. Miller
Cc: Eric Dumazet
Cc: Hannes Reinecke
Cc: Ilya Dryomov
Cc: Jan Kara
Cc: Jeff Layton
Cc: Jens Axboe
Cc: Lee Duncan
Cc: Mike Christie
Cc: Mikhail Skorzhinskii
Cc: Philipp Reisner
Cc: Sagi Grimberg
Cc: Vasily Averin
Cc: Vlastimil Babka
---
Changelog:
v7: remove outer brackets from the return line of sendpage_ok() as
Eric Dumazet suggested.
v6: fix page check in do_tcp_sendpages(), as Eric Dumazet suggested.
replace other open coded checks with sendpage_ok() in libceph,
iscsi drivers.
v5, include linux/mm.h in include/linux/net.h
v4, change sendpage_ok() as an inline helper, and post it as
separate patch, as Christoph Hellwig suggested.
v3, introduce a more common sendpage_ok() as Sagi Grimberg suggested.
v2, fix typo in patch subject
v1, the initial version.
Coly Li (6):
net: introduce helper sendpage_ok() in include/linux/net.h
nvme-tcp: check page by sendpage_ok() before calling kernel_sendpage()
tcp: use sendpage_ok() to detect misused .sendpage
drbd: code cleanup by using sendpage_ok() to check page for
kernel_sendpage()
scsi: libiscsi: use sendpage_ok() in iscsi_tcp_segment_map()
libceph: use sendpage_ok() in ceph_tcp_sendpage()
drivers/block/drbd/drbd_main.c | 2 +-
drivers/nvme/host/tcp.c| 7 +++
drivers/scsi/libiscsi_tcp.c| 2 +-
include/linux/net.h| 16
net/ceph/messenger.c | 2 +-
net/ipv4/tcp.c | 3 ++-
6 files changed, 24 insertions(+), 8 deletions(-)
--
2.26.2
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818131227.37020-1-colyli%40suse.de.
[PATCH v7 5/6] scsi: libiscsi: use sendpage_ok() in iscsi_tcp_segment_map()
In iscsci driver, iscsi_tcp_segment_map() uses the following code to
check whether the page should or not be handled by sendpage:
if (!recv && page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)))
The "page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)" part is to
make sure the page can be sent to network layer's zero copy path. This
part is exactly what sendpage_ok() does.
This patch uses use sendpage_ok() in iscsi_tcp_segment_map() to replace
the original open coded checks.
Signed-off-by: Coly Li
Cc: Vasily Averin
Cc: Cong Wang
Cc: Mike Christie
Cc: Lee Duncan
Cc: Chris Leech
Cc: Christoph Hellwig
Cc: Hannes Reinecke
---
drivers/scsi/libiscsi_tcp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index 6ef93c7af954..31cd8487c16e 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -128,7 +128,7 @@ static void iscsi_tcp_segment_map(struct iscsi_segment
*segment, int recv)
* coalescing neighboring slab objects into a single frag which
* triggers one of hardened usercopy checks.
*/
- if (!recv && page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)))
+ if (!recv && sendpage_ok(sg_page(sg)))
return;
if (recv) {
--
2.26.2
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/20200818131227.37020-6-colyli%40suse.de.
Re: [PATCH v7 1/6] net: introduce helper sendpage_ok() in include/linux/net.h
On 2020/8/19 03:49, Christoph Hellwig wrote: > On Wed, Aug 19, 2020 at 12:33:37AM +0800, Coly Li wrote: >> On 2020/8/19 00:24, Christoph Hellwig wrote: >>> I think we should go for something simple like this instead: >> >> This idea is fine to me. Should a warning message be through here? IMHO >> the driver still sends an improper page in, fix it in silence is too >> kind or over nice to the buggy driver(s). > > I don't think a warning is a good idea. An API that does the right > thing underneath and doesn't require boiler plate code in most callers > is the right API. > Then I don't have more comment. Thanks. Coly Li -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/04408ff6-f765-8f3e-ead9-aec55043e469%40suse.de.
Antw: [EXT] [PATCH v6 1/6] net: introduce helper sendpage_ok() in include/linux/net.h
>>> Coly Li schrieb am 18.08.2020 um 14:47 in Nachricht
<20200818124736.5790-2-col...@suse.de>:
> The original problem was from nvme-over-tcp code, who mistakenly uses
> kernel_sendpage() to send pages allocated by __get_free_pages() without
> __GFP_COMP flag. Such pages don't have refcount (page_count is 0) on
> tail pages, sending them by kernel_sendpage() may trigger a kernel panic
> from a corrupted kernel heap, because these pages are incorrectly freed
> in network stack as page_count 0 pages.
>
> This patch introduces a helper sendpage_ok(), it returns true if the
> checking page,
> - is not slab page: PageSlab(page) is false.
> - has page refcount: page_count(page) is not zero
>
> All drivers who want to send page to remote end by kernel_sendpage()
> may use this helper to check whether the page is OK. If the helper does
> not return true, the driver should try other non sendpage method (e.g.
> sock_no_sendpage()) to handle the page.
>
> Signed-off-by: Coly Li
> Cc: Chaitanya Kulkarni
> Cc: Christoph Hellwig
> Cc: Hannes Reinecke
> Cc: Jan Kara
> Cc: Jens Axboe
> Cc: Mikhail Skorzhinskii
> Cc: Philipp Reisner
> Cc: Sagi Grimberg
> Cc: Vlastimil Babka
> Cc: sta...@vger.kernel.org
> ---
> include/linux/net.h | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/net.h b/include/linux/net.h
> index d48ff1180879..a807fad31958 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -286,6 +287,21 @@ do {
> \
> #define net_get_random_once_wait(buf, nbytes)\
> get_random_once_wait((buf), (nbytes))
>
> +/*
> + * E.g. XFS meta- & log-data is in slab pages, or bcache meta
> + * data pages, or other high order pages allocated by
> + * __get_free_pages() without __GFP_COMP, which have a page_count
> + * of 0 and/or have PageSlab() set. We cannot use send_page for
> + * those, as that does get_page(); put_page(); and would cause
> + * either a VM_BUG directly, or __page_cache_release a page that
> + * would actually still be referenced by someone, leading to some
> + * obscure delayed Oops somewhere else.
> + */
Actually I think this comment is somewhat mis-placed:
It should describe what the function does (check for specific properties of a
page), but not where this function might be used. Most notably, because the use
(from where it is called) may change over time, while the function will still
do the same thing.
> +static inline bool sendpage_ok(struct page *page)
> +{
> + return (!PageSlab(page) && page_count(page) >= 1);
> +}
> +
> int kernel_sendmsg(struct socket *sock, struct msghdr *msg, struct kvec
> *vec,
> size_t num, size_t len);
> int kernel_sendmsg_locked(struct sock *sk, struct msghdr *msg,
> --
> 2.26.2
>
> --
> You received this message because you are subscribed to the Google Groups
> "open-iscsi" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to open-iscsi+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/open-iscsi/20200818124736.5790-2-colyli%40s
> use.de.
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/open-iscsi/5F3CBF5302A10003AB07%40gwsmtp.uni-regensburg.de.
