| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
In sysfs_device_get() we should firstly check whether
devpath pointer is NULL before accessing it
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Wu Bo <wubo40@huawei.com>
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
If strdup() returns NULL, find_devtree() just return NULL to
indicate this function execute failed.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Wu Bo <wubo40@huawei.com>
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Both in iscsiadm and iscsistart, a list is initialized to management
struct iscsi_param which allocated by idbm_alloc_user_param().
While both of iscsiadm and iscsistart did not free these iscsi_param
when process exit, this patch add a free flow when process exit.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Wu Bo <wubo40@huawei.com>
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
if_nameindex() returns NULL on error, if it returns NULL, we
just print the error info and return NULL.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Wu Bo <wubo40@huawei.com>
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
If malloc() returns NULL on fail, we should return -ENOMEM to
avoid NULL pointer dereference.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Wu Bo <wubo40@huawei.com>
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
If calloc() returns NULL just return ENOMEM;
if strdup() returns NULL we should free dev and return errno.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Both qedi_open() and bnx2x_open() perform 15 times retry
to make system call open() successful, while this retry
mechanism can not make sure open() can execute successfully.
So this patch verify return value of open(), if it is still
INVALID_FD just print error info and return the errno.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
System call open() might return -1 if an error occurred which
should be taken into consideration.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Wu Bo <wubo40@huawei.com>
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Check if t is valid before accessing it.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Wu Bo <wubo40@huawei.com>
|
| |/ /
| |
| |
| | |
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
|
| |\ \
| | |
| | | |
iscsiadm: Optimize the the verification of mode paramters by doing so sooner and in one place.
|
| |/ /
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The mode parameters should be check earlier when recognize the
mode, rather than mutiple calls after all commandline parameters
are parsed.
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Signed-off-by: Wu Bo <wubo40@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
|
| |\ \
| | |
| | | |
iscsid: Poll timeout value to 1 minute for iscsid
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Description
===========
This patch has the following two changes
------------------------------------------
Change 1: Specify the poll timeout value to 1 minute as third parameter
to the function iscsid_exec_req() when called from sync_session()
Reason: Currently the poll timeout value sent sent to iscsid_response()
from iscsid_exec_req() is "info->iscsid_req_tmo" which is -1 as set in
"iscsi_sysfs_for_each_session()". When iscsid_response() receives this
-1, it sets the timeout value to ISCSID_REQ_TIMEOUT (1000 ms) and also
sets a local variable "poll_wait" to 1. There is a while loop below this
which checks the value of "poll_wait". If "poll_wait" is set to 1, then
it calls "continue". For sessions which are giving continuous connection
errors (like target service stopped OR target node is shutdown OR any
other continuous connection errors etc ...), this results in an
indefinite while loop, as nothing will be written to the poll fd,
resulting in further sessions not getting synced (or recovered) when the
iscsid is restarted due to any reason (either manual restart of iscsid
or rpm install). Poll timeout of 1 minute seems to be a reasonable value
for the slow connections
Change 2: Change the return error code from the function
iscsid_response() to ISCSI_ERR_SESSION_NOT_CONNECTED when the poll times
out and returns error 0
Reason: Currently the iscsid_response() function returns
ISCSI_ERR_ISCSID_NOTCONN error code in case if the poll times out and
the poll_wait variable is 0 (i.e poll timeout sent to
iscsid_response() was -1). Returning this error code doesn't
seem to be correct for the following two reasons:
a. ISCSI_ERR_ISCSID_NOTCONN should be returned only when we
are not able to connect to iscsid but we are in iscsid_response()
function indicating that we were able to connect to iscsid
successfully
b. When ISCSI_ERR_ISCSID_NOTCONN is returned, the
sync_session() will retry the request till 30 retries are reached. This
causes an overlap of multiple "iscsi login task" in
kernel and the kernel will return an error to the
user-space indicating "Login/Text in progress. Cannot start new task.".
This is repeated continuously which results in session not getting
recovered even after it comes back up. Also at one point of time we
observed that the kernel panics in "iscsi_sw_tcp_conn_set_param()" while
trying to set the param ISCSI_PARAM_DATADGST_EN. We have a kernel fix
for this panic which is in review for upstream
|
| |\ \ \
| | | |
| | | | |
iscsid: fix logging level when starting and shutting down daemon
|
| | | | | |
|
| |/ / /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
The command:
> iscsiadm -m host -C stats -H <mac_addr>
Copies in the user-supplied mac address without checking its
length, but the maximum length is 17 characters. This overflows
the next field in the structure, or if libc checking is
enabled, produces this error:
> *** buffer overflow detected ***: iscsiadm terminated
Fix this by checking the length of the supply MAC address
before copying it into the array. Some extra debugging
error messages were added in the process.
|
| |\ \ \
| | | |
| | | | |
Fix memory leak in sysfs_get_str
|
| | |/ /
| | |
| | |
| | |
| | |
| | |
| | | |
strlen(sysfs_value) return 0 do not mean sysfs_value
is NULL and this memory should be freed
Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
|
| |\ \ \
| | | |
| | | | |
libopeniscsiusr: Compare with max int instead of max long
|
| | |/ /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This compares value member of int_list_tbl struct which is of unsigned
int type.
struct int_list_tbl {
const char *name;
unsigned int value;
};
Clang compiler reports this comparison when
-Wtautological-constant-out-of-range-compare is enabled
| idbm.c:1042:2: error: result of comparison of constant 18446744073709551615 with expression of type 'unsigned int' is always true [-Werror,-Wtautological-constant-out-of-range-compare]
| _rec_int_list(SESSION_CHAP_ALGS, recs, node, session.auth.chap_algs,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| idbm.c:263:23: note: expanded from macro '_rec_int_list'
| if (_org->_name[_i] != ~0UL) { \
| ~~~~~~~~~~~~~~~ ^ ~~~~
Since max value for int can be less than unsinged long e.g. on LP64 its
better to use UINT_MAX here
Signed-off-by: Khem Raj <raj.khem@gmail.com>
|
| |/ /
| |
| |
| |
| |
| | |
Fixed some quoting and spacing, but also cleaned
up list of transports, as they were seriously
out of date.
|
| | |
| |
| |
| |
| | |
This patch is to update the todo list. Tasks are suggested by The
Lee-Man
|
| | |
| |
| |
| |
| |
| |
| |
| |
| | |
Commit 46183ffd36c9 added the "-W"/"--no_wait" flag to iscsiadm,
in "node" mode, when doing loginall. This commit adds this functionality
to mode node when logging into a single target.
The man page was also updated, as was the README file, where
appropriate.
|
| |\ \
| | |
| | | |
iscsiadm buffer overflow regression when discovering many targets at …
|
| |/ /
| |
| |
| |
| |
| | |
int_list type didn't zero the output string, so as the rec struct was reused
repeatedly during discovery it would keep growing with repeated values
triggering a strcat buffer overflow
|
| |\ \
| | |
| | | |
Check Invalid Session id for stop connection
|
| |/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Description:
If the initiator is rebooting then after the reboot, it will try to
resync (recreate) the existing the connections by reading the sysfs.
While initiator is doing this, i.e when the initiator tries to connect
to the target but if the target service is not yet started, then the
initiator connection will fail. The session id is also not yet assigned
and it will be at its initial value 0xFFFFFFFF which is invalid. The
session id is assigned a valid value only after a successful connection.
Since the connection is failed, the initiator code will queue the
connection for re-open. The connection state is still at
ISCSI_CONN_STATE_XPT_WAIT as its very first login attemp after the
reboot.
Due to my Pull #206 request the code will invoke the stop connection to
decrement the socket_fd reference count to properly close the connecion
(details are in pull request #206). But since the session id is not
valid, the stop connection will fail and the code will go ahead and
queue the re-open without attempting the connect again. This is repeated
till 120 seconds (stop connection failing and requeuing the reopen
without invoking connect) and the connection will be shutdown resulting
the storage unavailable.
Fix:
We need to check the validity of the session id before calling the stop
connection. If the session id is valid then only invoke the stop
connection. Due to this, the code will go ahead and attempt the connet
call. If the target service comes up anytime in 120 seconds, then the
connect will be successful and we will get connected to the target.
|
| |\ \
| | |
| | | |
Add ability to attempt target logins asynchronously
|
| |/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Add a "no wait" flag to iscsiadm, for use when it is logging into
all targets of a certain "type", i.e. using the "--loginaall=" option,
which iscsiadm will pass to the login_by_startup() routine. The
wait/no-wait code was already present there, so just hook into
it.
NOTE: this means that "iscsiadm -m node ... -L=all -W" will return
success if it is able to send a login request for each node, whether
or not the logins succeed.
The man page and usage info was also updated to document this new flag.
|
| | | |
|
| |\ \
| | |
| | | |
use openssl RAND_bytes() for mutual CHAP challenge buffer generation
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | | |
openssl's RAND_bytes() is now used instead, so this can be dropped.
Suggested-by: Marcus Meissner <meissner@suse.de>
Signed-off-by: David Disseldorp <ddiss@suse.de>
|
| |/ /
| |
| |
| |
| |
| |
| |
| | |
48a4e5b475836bcb952fb53a8bde45bdf68fe38f added an openssl dependency, so
use it for obtaining random buffers via RAND_bytes().
Suggested-by: Marcus Meissner <meissner@suse.de>
Signed-off-by: David Disseldorp <ddiss@suse.de>
|
| |\ \
| | |
| | | |
Misspelled socket name might cause confusion to inexperienced user.
|
| |/ /
| |
| |
| | |
Signed-off-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
|
| |\ \
| | |
| | | |
iscsi/libopeniscsiusr:Fix libopeniscsiusr_node.h file is not installed
|
| | | | |
|
| |\ \ \
| | | |
| | | | |
iscsi: Add break to while loop
|
| | | | | |
|
| |\ \ \ \
| |/ / /
|/| | | |
iscsi: fix fd leak
|
| |/ / / |
|
| |\ \ \
| | | |
| | | | |
Fix zero length array usage, and some iscsiuio compiler issues
|
| | | | |
| | | |
| | | |
| | | |
| | | | |
This makes the compiler happy when searching for uio%d sysfs
directories.
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
When playing with IPv6 IP addresses, which are described
in C using a union, and the gcc-10 compiler didn't like
that we were not specifying which member of the union
we wanted to address of, when in fact all members have
the same address. The fix is to be more explicit.
|
| |/ / /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
A common practice in C coding, over the years, has been to
add a zero-length array at the end of the structure when trying
to represent a possibly-empty array of bytes that may be
appended to the struct. But the gcc-10 compiler does not
like such structures, taking the zero-length literally.
The following errors are fixed by this commit:
> iscsiadm.c: In function ‘session_stats’:
> iscsiadm.c:939:56: error: array subscript ‘(<unknown>) + -1’ is outside the bounds of an interior zero-length array ‘struct iscsi_stats_custom[0]’ [-Werror=zero-length-bounds]
> 939 | (unsigned long long)rsp.u.getstats.stats.custom[i].value);
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
> In file included from initiator.h:29,
> from iscsiadm.c:36:
> ../include/iscsi_if.h:844:28: note: while referencing ‘custom’
> 844 | struct iscsi_stats_custom custom[0]
> | ^~~~~~
> iscsiadm.c:938:56: error: array subscript ‘(<unknown>) + -1’ is outside the bounds of an interior zero-length array ‘struct iscsi_stats_custom[0]’ [-Werror=zero-length-bounds]
> 938 | printf("\t%s: %llu\n", rsp.u.getstats.stats.custom[i].desc,
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
> In file included from initiator.h:29,
> from iscsiadm.c:36:
> ../include/iscsi_if.h:844:28: note: while referencing ‘custom’
> 844 | struct iscsi_stats_custom custom[0]
> | ^~~~~~
> cc1: all warnings being treated as errors
The work around is to convert the two "custom[0]" structure members to
use "custom[]".
|
| |\ \ \
| | | |
| | | | |
Add iscsi-init.service
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Per Fedora Packaging Guidelines [1], initial configuration of a service
should happen in a one-off init service in order to ensure idempotency,
and not in the %post directive of the RPM spec.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1493296
[1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Initial_Service_Setup/
|
| |\ \ \ \
| |/ / /
|/| | | |
Proper disconnect of TCP connection
|
| |/ / /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
1. Due to configuration issues, the logins from iSCSI initiator were getting rejected by the target
2. The initiator was retrying the login again and again
3. Each time the initiator tries to log in, the host number gets incremented by 1
4. At one point of time, the host number reached 65535
5. During the login process, once the TCP connection is established, the initiator tries to set the host parameters for the network interface if its not the default interface
6. While setting these host parameters, it will try to do a lookup of the host based upon the host number
7. The host number in "iscsi_uevent" structure is uint32_t. This is given as an argument to the scsi_host_lookup() function
8. This scsi_host_lookup() function takes it as unsigned short. So, when it receives the host number above 65535, the value is wrapped and starts from 0 again
9. Thus the incorrect value of host number is received by the scsi_host_lookup() function and hence it returns with error that the host is not existing in the list
10. Due to this "host not found error", the open-iscsi will retry this particular connection again and again
11. In this each retry, it will disconnect and then connect again with the same connection pointer, i.e it re-opens the connection multiple times till 120 seconds timeout
12. During these 120 seconds, observed that its trying to re-open the connection aroung 400+ times with each time disconnect and connect
13. After 120 seconds, the connection and session will be destroyed
14. So, while doing multiple retries of connect and disconnect during the 120 seconds, when the connect is successful it will try to bind the connection to the session
15. When it binds the connection and session, the reference count for the socket is incremented
16. When it disconnects, its trying to close the socket with close(sockfd) system call
17. This close() system call is entering into the kernel and NOT going forward till the networking layrer to call tcp_close() to send the FIN packet to the target
18. Its not going till tcp_close() because the reference count of the socket is still 1
19. So, the initiator is not sending the FIN packet to target and hence target is timing out and sending FIN after its timeout. This happens for all the retries (400+)
20. At some point, when this FIN packet is received by the initiator, the connection was destroyed and the memory was re-used for some other purpose and hence we see the panic
Fix:
==
Fix is to decrement the reference count of the socket fd after disconnect by calling the stop connection
Corrected the indentation for the change in the function iscsi_login_eh()
|
