A Question about setting discovery_auth values

[Lee Duncan]

Hi all:

I have been playing with target discovery authentication (CHAP) for my
iSCSI targets, and I noticed something I don't understand.

Once I set a discovery mutual userid and password in directory
/sys/kernel/config/target/iscsi/discovery_auth, I see that the value for
"authenticate_target" gets set to 1.

But if I clear out the userid_mutual and password_mutual fields,
authenticate_target is still set.

Looking at the code, the only way it seems I can clear this value is by
writing the literal string "NULL" to either userid_mutual or
password_mutual.

If this is correct, can somebody explain why this is not a bug? It seems
like software trying to interpret the value of these attributes will
have to test for both "" and "NULL" to see if the field is logically empty.

[Nicholas A. Bellinger]

Hi Lee,

Apologies for the long delayed response. Comments below.

Post by Lee Duncan
I have been playing with target discovery authentication (CHAP) for my
iSCSI targets, and I noticed something I don't understand.
Once I set a discovery mutual userid and password in directory
/sys/kernel/config/target/iscsi/discovery_auth, I see that the value for
"authenticate_target" gets set to 1.
But if I clear out the userid_mutual and password_mutual fields,
authenticate_target is still set.
Looking at the code, the only way it seems I can clear this value is by
writing the literal string "NULL" to either userid_mutual or
password_mutual.
If this is correct, can somebody explain why this is not a bug? It seems
like software trying to interpret the value of these attributes will
have to test for both "" and "NULL" to see if the field is logically empty.

This is correct. Existing rtslib code does this translation between
"NULL" and "", and masks this peculiarity from end-users in targetcli.

In retrospect the decision to use NULL here to clear the existing value
was not the ideal solution, but at this point changing how this works
(eg: ABI breakage) is not really an option.

--nab

[Nicholas A. Bellinger]

Apologies for the top-post, but my current mail agent sucks. :)
No problem on the delay.
What about handling "NULL" and "" as meaning "clear the current value?
I could supply a patch ...

While writing the earlier response, I was trying to remember why "" was
deemed insufficient in the first place for clearing the current value,
but alas the original reasoning escapes me atm.

It could very well have to do with some early user-space (eg: lio-utils)
during state re-creation had a distinction between an empty value and
explicitly clearing a value, but I still don't recall exactly why this
mattered.

That said, there was some reasoning behind this, and I'd like to
remember why first before making a change that could negatively effect
user-space.

--nab

[Lee Duncan]

Post by Nicholas A. Bellinger

Apologies for the top-post, but my current mail agent sucks. :)
No problem on the delay.
What about handling "NULL" and "" as meaning "clear the current value?
I could supply a patch ...

While writing the earlier response, I was trying to remember why "" was
deemed insufficient in the first place for clearing the current value,
but alas the original reasoning escapes me atm.

Perhaps the reason for this is that writing an empty string from the
shell is not easy. Trying to do an 'echo -n "" > some_file" gets
optimized into not writing anything out. I had to use something like:
'echo -n "\0" > some_file" to get it to write an empty string.

Post by Nicholas A. Bellinger
It could very well have to do with some early user-space (eg: lio-utils)
during state re-creation had a distinction between an empty value and
explicitly clearing a value, but I still don't recall exactly why this
mattered.
That said, there was some reasoning behind this, and I'd like to
remember why first before making a change that could negatively effect
user-space.

If you remember please let me know, as I've patched our kernel to accept
these NULL values.

Post by Nicholas A. Bellinger
--nab

[Nicholas A. Bellinger]

Post by Lee Duncan

Post by Nicholas A. Bellinger

Apologies for the top-post, but my current mail agent sucks. :)
No problem on the delay.
What about handling "NULL" and "" as meaning "clear the current value?
I could supply a patch ...

While writing the earlier response, I was trying to remember why "" was
deemed insufficient in the first place for clearing the current value,
but alas the original reasoning escapes me atm.

Perhaps the reason for this is that writing an empty string from the
shell is not easy. Trying to do an 'echo -n "" > some_file" gets
'echo -n "\0" > some_file" to get it to write an empty string.

Mmmm, yes this sounds familiar..

Post by Lee Duncan

Post by Nicholas A. Bellinger
It could very well have to do with some early user-space (eg: lio-utils)
during state re-creation had a distinction between an empty value and
explicitly clearing a value, but I still don't recall exactly why this
mattered.
That said, there was some reasoning behind this, and I'd like to
remember why first before making a change that could negatively effect
user-space.

If you remember please let me know, as I've patched our kernel to accept
these NULL values.

Ok, in that case I'm OK with entertaining a patch as long no regressions
are introduced that break existing user-space.

Adding Jerome to the CC' on the user-space side for good measure.

Thanks Lee!

--nab