Re: 8 patches for review
On 2015-07-01 17:22, The Lee-Man wrote: On Monday, June 29, 2015 at 1:38:04 PM UTC-7, Christian Iversen wrote: Hello Open-iSCSI (please CC as I'm not a regular on the list) Please join the list if you are going to submit patches. Allright, noted :) I've been working with iSCSI lately, and thought I could help with a few patches. Here's a description: I am looking at your patches (I'm sure Mike will, as well), but my first response is that your descriptions (and hence the names of your patches) are not very descriptive. Duly noted. For example, for your patch 8, remove dead code is too generic. How about iscsiuio: remove unused ipv6 header. Good idea! These are all more or less just compile-tested. Any comments? Yes, should be run-tested as well if you are going to submit this many changes, IMHO. Also, I'd suggest grouping similar changes together. Why create a patch for each variable removed? If you are cleaning up unused variables, I would group those patches as one. But that might just be me. :) And the two memset() fixes should be one patch. Lastly, you cannot patch open-isns in open-iscsi any longer. You need to submit those changes to https://github.com/gonzoleeman/open-isns Okay. So yeah, I've run-tested the patches in the meantime, and they all work, except the MaxOutstandingR2T had a missing change (which I had locally). I'll try to re-roll the patchset, and return when I've joined the list and fixed the other issues. -- De bedste hilsner, Christian Iversen Systemadministrator, Meebox.net --- Denne e-mail kan indeholde fortrolige oplysninger. Er du ikke den rette modtager, bedes du returnere og slette denne e-mail. --- -- 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 post to this group, send email to open-iscsi@googlegroups.com. Visit this group at http://groups.google.com/group/open-iscsi. For more options, visit https://groups.google.com/d/optout.
8 patches for review
Hello Open-iSCSI
(please CC as I'm not a regular on the list)
I've been working with iSCSI lately, and thought I could help with a few
patches.
Here's a description:
-- 0001-Enable-MaxOutstandingR2T-negotiation-support-in-iscs.patch
This patch enables MaxOutstandingR2T negotiation for sessions.
Currently, max_r2t negotiation is in a weird, half-broken state. The
kernel seems to support max_r2t 1 just fine (at least it connects),
but iscsiadm is hard-coded to use 1, regardless of the setting that
the user sets in the config file, which is very confusing.
I spent more time than I'd like to admit on tracking this one down.. :)
There's even error handling in place for max_r2t 1, but this is never
triggered since the value 1 is hard-coded, and the config file is
ignored. This is the worst of both worlds.
-- 0002-Removed-a-number-of-unused-variables.patch
Quick cleanup, to fix a number of compiler warnings.
-- 0003-Removed-unused-variable-and-computation.patch
Same as 0002.
-- 0004-Added-missing-pointer-dereferencing-memset-would-onl.patch
-- 0005-Added-missing-pointer-dereferencing-memset-would-onl.patch
In the duplicate(!) md5 function MD5Final(), there is a final
memset(ctx, 0, sizeof(ctx));
to clear the context of sensitive data after MD5 calculation - which is
fine.
Unfortunately, they both refer to sizeof(ctx), which is a pointer,
effectively nullifying (no pun intended) the effect of memset(). This is
changed to sizeof(*ctx).
-- 0006-Removed-unused-variable-one.patch
-- 0007-Removed-unused-variable-pdu_text.patch
-- 0008-Removed-dead-code.patch
Should be self-explanatory.
These are all more or less just compile-tested. Any comments?
--
De bedste hilsner / Best Regards,
Christian Iversen
System Engineer
Meebox ApS
Store Kongensgade 40H,
1264 København K
T: +45 3841 3841
--
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 post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.
From 4a62742860cb87cfdbd2527077e27807cb1ef329 Mon Sep 17 00:00:00 2001
From: Christian Iversen c...@iversenit.dk
Date: Mon, 29 Jun 2015 16:19:34 +0200
Subject: [PATCH 8/8] - Removed dead code
---
iscsiuio/src/uip/uip.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/iscsiuio/src/uip/uip.c b/iscsiuio/src/uip/uip.c
index 703cefc..8266b15 100644
--- a/iscsiuio/src/uip/uip.c
+++ b/iscsiuio/src/uip/uip.c
@@ -1023,7 +1023,6 @@ void uip_process(struct uip_stack *ustack, u8_t flag)
struct uip_tcp_ipv4_hdr *tcp_ipv4_hdr = NULL;
struct uip_tcp_hdr *tcp_hdr = NULL;
struct uip_icmpv4_hdr *icmpv4_hdr = NULL;
- struct uip_icmpv6_hdr *icmpv6_hdr = NULL;
struct uip_udp_hdr *udp_hdr = NULL;
/* Drop invalid packets */
@@ -1050,7 +1049,6 @@ void uip_process(struct uip_stack *ustack, u8_t flag)
buf = ustack-network_layer;
buf += sizeof(struct uip_ipv6_hdr);
- icmpv6_hdr = (struct uip_icmpv6_hdr *)buf;
} else {
uint8_t *buf;
--
2.1.4
From 7a2a09620e44623d0663640cddb4fe5c34ac19aa Mon Sep 17 00:00:00 2001
From: Christian Iversen c...@iversenit.dk
Date: Mon, 29 Jun 2015 16:19:17 +0200
Subject: [PATCH 7/8] - Removed unused variable pdu_text
---
usr/login.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/usr/login.c b/usr/login.c
index 8ff33a4..2c9be68 100644
--- a/usr/login.c
+++ b/usr/login.c
@@ -50,11 +50,9 @@ iscsi_add_text(struct iscsi_hdr *pdu, char *data, int max_data_length,
int pdu_length = ntoh24(pdu-dlength);
char *text = data;
char *end = data + max_data_length;
- char *pdu_text;
/* find the end of the current text */
text += pdu_length;
- pdu_text = text;
pdu_length += length;
if (text + length = end) {
--
2.1.4
From fe1f9b0fedc936279dd67a8f21737f1c83a3e427 Mon Sep 17 00:00:00 2001
From: Christian Iversen c...@iversenit.dk
Date: Mon, 29 Jun 2015 16:17:55 +0200
Subject: [PATCH 6/8] - Removed unused variable one
---
usr/initiator_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/usr/initiator_common.c b/usr/initiator_common.c
index b39a82c..d92152d 100644
--- a/usr/initiator_common.c
+++ b/usr/initiator_common.c
@@ -351,7 +351,7 @@ int iscsi_session_set_params(struct iscsi_conn *conn)
{
struct iscsi_session *session = conn-session;
int i, rc;
- uint32_t one = 1, zero = 0;
+ uint32_t zero = 0;
struct connparam {
int param;
int type;
--
2.1.4
From b91a2cf028d667c92508ac35b80d297604ceacc9 Mon Sep 17 00:00:00 2001
From: Christian Iversen c...@iversenit.dk
Date: Mon, 29 Jun 2015 16:15:04 +0200
Subject: [PATCH 5/8] * Added missing pointer dereferencing: memset() would
only clear the first sizeof(voidptr) ctx bytes in MD5Final!
---
usr/md5.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff
Re: Timeout values for iSCSI
On 2010-04-14 19:42, Mike Christie wrote: On 04/14/2010 07:02 AM, Christian Iversen wrote: What I'd like is the following: - Never give up trying (or at least try for a month :) The iscsi initiator almost always tries to reconnect to the target. If it gets a successful login then that fails it will try to relogin until the the user runs some iscsiadm command to logout. If you mean you want it to hold onto IO and not fail it, then you want the replacement_timeout/recovery_timeout. There should be info in the README and iscsid.conf about this. If it is not clear let me know. There's info about replacement_timeout, but no recovery_timeout. Maybe only the former is a valid name? replacement_timeout is the name of the setting in iscsid.conf, but for some dumb reason I named it recovery_timeout in the kernel. Ah, ok. I'll go with replacement_timeout then :) If in the iscsid.conf you see this for node.session.timeo.replacement_timeout then this is what I think you are asking for (that is if you are saying you do not want IO failed) and you want to set the value to 0. # - If the value is 0, IO will be failed immediately. # - If the value is less than 0, IO will remain queued until the session # is logged back in, or until the user runs the logout command. I'm a little unsure about the semantics for failed io. What I want is the iscsi client to see all IO as working, or hanging indefinitely if the server cannot be contacted. Then set the replacement_timeout to -1. Ok. What about these timeouts? node.session.err_timeo.abort_timeout = x node.session.err_timeo.lu_reset_timeout = y node.session.err_timeo.host_reset_timeout = z What are reasonable values for x, y and z, and when are they used? If there is a low-level error, I'd like iscsi to detect this quickly and reconnect right away. (this will happen when there's a failover). Will the following settings work for this purpose: node.conn[0].timeo.noop_out_interval = 2 node.conn[0].timeo.noop_out_timeout = 2 node.session.timeo.replacement_timeout = 86400 Yes. I'll use this then. Per my understanding: This will ping the server every 2. seconds, and wait 2 seconds for a reply. If a connection problem is discovered, the client will try for 24 hours (86400 seconds) to reestablish a connection before giving up and returning IO errors to higher layers. Is this correct? From your description it seems like replacement_timeout Yes. = 0 would cause immediate IO errors in case of connection problems? Or did I misunderstand? Yeah, on newer versions 0 causes the IO to be failed immediately. I wrote that wrong before. Was it different on old versions? In any case, I'll use a value of 86400 for that timeout :) -- Med venlig hilsen / Best regards Christian Iversen Sikkerhed.org ApS Fuglebakkevej 88 E-mail: supp...@sikkerhed.org 1. sal Web: www.sikkerhed.org DK-2000 Frederiksberg Direkte: c...@sikkerhed.org -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
Timeout values for iSCSI
Hi iSCSI guys I've set up iSCSI storage on our servers, using IETD and OpenISCSI. It works and performs great, but I am a little unsure of how to adjust the timeout values properly. On our storage servers, we use heartbeat to achieve HA failover, which works nicely. However, the client machines only try for a fixed amount of time before giving up, so if the failover for some reason does not happen relatively quickly, everything grinds to a halt in a really bad way. I would like to set up open-iscsi to keep trying, preferably at low intervals, and not give up contacting the server. There are quite a few different timeouts, and I have been unable to find any sort of reference documentation for this. Maybe someone here can help? What I'd like is the following: - Never give up trying (or at least try for a month :) - Try every 1 second - Timeout should work for all stages of the session, be it logged in or not. Can anybody help? Please CC me as I'm not on the list. -- Med venlig hilsen / Best regards Christian Iversen Sikkerhed.org ApS Fuglebakkevej 88 E-mail: supp...@sikkerhed.org 1. sal Web: www.sikkerhed.org DK-2000 Frederiksberg Direkte: c...@sikkerhed.org -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
