Re: [PATCH 0/2] iscsi-target: Optionally do not discover targets which contain only LUNs that are not accessable by the discoverying initiator v2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Nab,

> > attribute name hide_from_unauthorized

> How about renaming the new attribute to demo_mode_discovery, and using
> a default value to 1 + inverting the current checks?

done.

> Mmm, all fair points above.  Thinking about this some more, I'm fine
> with taking these patches without further cleanups here, and leave
> this as a separate item for now.

If you have any input for me, let me know and I'll take care of it.

> > spin_lock(&tpg->tpg_state_lock);

> tpg->tpg_state is protected every place else by tpg_state_lock.  Your
> right that it's probably overkill for this particular case given the
> purely information nature of SendTargets.

I see and I think it is good here to be consistent as well.

> > With the current code path it is possible that TargetName is printed but
> > TargetAddress is not because the payload buffer runs out of space. Is
> > this okay, or should we change it?

> It's possible this could happen, yes. However the initiator receiving a
> TargetName without any TargetAddresses should simply ignore this entry.

I see. If you want me to test this or change the code, let me know.

> > I often have a configuration where I export multiple demo mode LUNs
> > in different VLANs but I only want initiators from same VLAN access
> > that LUN. To my knowledge that is currently not possible, is it?

> However, splitting up these LUNs across different TargetName
> +TargetPortalGroupTag endpoints, and only associating the individual
> VLAN interfaces as network portals on these endpoints should provide
> the desired effect, no?

I tried it, but it was still discovering all LUNs. Is the config right?

/backstores/fileio create shared-01.v101.campusvl.de /iscsi1/shared-01.v101.campusvl.de size=80G buffered=true
/iscsi create iqn.2013-03.de.campusvl.v101.storage:shared-01.v101.campusvl.de
/iscsi/iqn.2013-03.de.campusvl.v101.storage:shared-01.v101.campusvl.de/tpgt1/portals create 10.101.99.4
/iscsi/iqn.2013-03.de.campusvl.v101.storage:shared-01.v101.campusvl.de/tpgt1/portals create 10.101.99.5
/iscsi/iqn.2013-03.de.campusvl.v101.storage:shared-01.v101.campusvl.de/tpgt1/luns create /backstores/fileio/shared-01.v101.campusvl.de lun=10
/iscsi/iqn.2013-03.de.campusvl.v101.storage:shared-01.v101.campusvl.de/tpgt1/ set attribute authentication=0 demo_mode_write_protect=0 generate_node_acls=1 cache_dynamic_acls=1

/backstores/fileio create shared-01.v102.campusvl.de /iscsi1/shared-01.v102.campusvl.de size=80G buffered=true
/iscsi create iqn.2013-03.de.campusvl.v102.storage:shared-01.v102.campusvl.de
/iscsi/iqn.2013-03.de.campusvl.v102.storage:shared-01.v102.campusvl.de/ create 2
/iscsi/iqn.2013-03.de.campusvl.v102.storage:shared-01.v102.campusvl.de/tpgt2/portals create 10.102.99.4
/iscsi/iqn.2013-03.de.campusvl.v102.storage:shared-01.v102.campusvl.de/tpgt2/portals create 10.102.99.5
/iscsi/iqn.2013-03.de.campusvl.v102.storage:shared-01.v102.campusvl.de/tpgt2/luns create /backstores/fileio/shared-01.v102.campusvl.de lun=10
/iscsi/iqn.2013-03.de.campusvl.v102.storage:shared-01.v102.campusvl.de/tpgt2/ set attribute authentication=0 demo_mode_write_protect=0 generate_node_acls=1 cache_dynamic_acls=1
/iscsi/iqn.2013-03.de.campusvl.v102.storage:shared-01.v102.campusvl.de/ delete 1

When I do a discovery I get both endpoints back.
shared-01.v101.campusvl.iqn.2013-03.de.campusvl.v101.storage:shared-01.v101.campusvl.de and
iqn.2013-03.de.campusvl.v102.storage:shared-01.v102.campusvl.de. Or did I misunderstand
howto set the TargetPortalGroupTag? Or did I miss something else or should it
not do that, if so I would go in the code and search for the problem.

> > +        &iscsi_tpg_attrib_hide_from_unauthorized.attr,

> Extra whitespace here..

I changed all spaces into tabs. I have my vim configured to do
'expandtabs' so everytime I hit 'tab' it puts 8 spaces. I disabled the
expandtabs feature of vim and made sure that the patches only contain
tabs.

> > +	fabric->tf_ops.tpg_check_hide_from_unauthorized=
> > +				&lio_tpg_check_hide_from_unauthorized;

> Give that only iscsi_target_mod needs to know about this attribute,
> there is no reason why it needs to be part of target_core_fabric_ops.
> So that said, it's fine to drop this part.

done.

> > +	int (*tpg_check_hide_from_unauthorized)(struct se_portal_group *);

> Ditto here..

done. I also dropped the now unused lio_tpg_check_hide_from_unauthorized.

> > +                target_name_printed = 0;

> Extra white-space here..  All these changes need to be using TAB spacing
> btw.  ;)

done.

> > +                         && (tpg->tpg_attrib.hide_from_unauthorized == 1)
> > +                         && (! core_tpg_get_initiator_node_acl(&tpg->tpg_se_tpg,

> Kernel style stays that the "&&" operators should be on the preceding
> line. Also, drop the extra whitespace between (! core_tpg_...())

done.

> > +                                if (! target_name_printed) {

> Ditto here on the extra whitespace.

done.

> > +                                        len = sprintf(buf, "TargetName=%s",
> > +                                                tiqn->tiqn);

> Kernel style is to make the following tiqn->tiqn); part here line up
> with the sprintf(, eg:

>     len = sprintf(foo, "%s',
>                   bar);

done.

> +EXPORT_SYMBOL(core_tpg_get_initiator_node_acl);

The patch I was sending you yesterday did _not_ compile because the
prototype for core_tpg_get_initiator_node_acl was missing. I added the
following line to the patch:

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 1330045..504b5b5 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -47,6 +47,8 @@
 #include "iscsi_target_device.h"
 #include "iscsi_target_stat.h"

+#include "../target_core_internal.h"

During testing yesterday I had this line in, however for submitting the patch I
somehow thought it was a relict that I no longer needed so I removed the line
before submitting the patch to you, which was of course wrong. I readded the
line. Please let me know if this line is okay for you or if put the prototype
for core_tpg_get_initiator_node_acl in another header file or want it defined
manually?

> The rest looks reasonable to me.  Nice work.

Prefect. Find v2 of the patches. I also tested them on top of the
iscsi-target: percpu_ida tag starvation regression fixes that you send
out earlier today with the usual test setup:

        - 12 ESX servers, parallel rescan, 24 VMs deployed in parallel
          and powered on in parallel and 24 svMotion in parallel,
          unloading the target during operation, watch the slabinfo and
          reloading it again.

All working perfectly fine. However I see the following log messages
which I did not see before when I did the 24 concurrent svMotion using
XCOPY:

Oct  4 21:38:42 node-62 kernel: [  934.369442] Unable to locate ITT: 0x0000b4aa on CID: 0
Oct  4 21:38:42 node-62 kernel: [  934.374446] Unable to locate RefTaskTag: 0x0000b4aa on CID: 0.

But everything worked afterwards.

I sniffed the discovery. With the default options it looks like:

iSCSI (Text Response)
    Opcode: Text Response (0x24)
    Flags: 0x80
        1... .... = F: Final PDU in sequence
        .0.. .... = C: Text is complete
    TotalAHSLength: 0x00
    DataSegmentLength: 0x000008ac
    LUN: 0000000000000000
    InitiatorTaskTag: 0x00000001
    TargetTransferTag: 0xffffffff
    StatSN: 0xb89e7744
    ExpCmdSN: 0x00000002
    MaxCmdSN: 0x00000002
    Key/Value Pairs
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v102.storage:shared-02.v102.campusvl.de
        KeyValue: TargetAddress=10.102.99.5:3260,2
        KeyValue: TargetAddress=10.102.99.4:3260,2
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v102.storage:shared-01.v102.campusvl.de
        KeyValue: TargetAddress=10.102.99.5:3260,2
        KeyValue: TargetAddress=10.102.99.4:3260,2
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:shared-01.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:shared-02.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-01.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-02.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-03.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-04.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-05.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-06.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-07.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-08.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-09.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-10.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-11.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-12.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1

When disabling demo_mode_discovery I get:

find /sys/kernel/config/target/iscsi -name demo_mode_discovery | while read FILE; do echo 0 > $FILE; done

iSCSI (Text Response)
    Opcode: Text Response (0x24)
    Flags: 0x80
        1... .... = F: Final PDU in sequence
        .0.. .... = C: Text is complete
    TotalAHSLength: 0x00
    DataSegmentLength: 0x000002be
    LUN: 0000000000000000
    InitiatorTaskTag: 0x00000001
    TargetTransferTag: 0xffffffff
    StatSN: 0xa33a9e16
    ExpCmdSN: 0x00000002
    MaxCmdSN: 0x00000002
    Key/Value Pairs
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v102.storage:shared-02.v102.campusvl.de
        KeyValue: TargetAddress=10.102.99.5:3260,2
        KeyValue: TargetAddress=10.102.99.4:3260,2
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v102.storage:shared-01.v102.campusvl.de
        KeyValue: TargetAddress=10.102.99.5:3260,2
        KeyValue: TargetAddress=10.102.99.4:3260,2
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:shared-01.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:shared-02.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
        KeyValue: TargetName=iqn.2013-03.de.campusvl.v101.storage:esx-12.v101.campusvl.de
        KeyValue: TargetAddress=10.101.99.4:3260,1
        KeyValue: TargetAddress=10.101.99.5:3260,1
    Padding: 0000

Btw. I used the line 'tshark -V -r /tmp/discover6.pcap -R iscsi.keyvalue |
less' to extract the above. Comes in handy.

As you can see I also get the targets from TGPT '2' discovered. Any idea what
I'm doing here wrong?

Nab, what do we do about the userland changes? Should I prepare a patch?
If so where can I find the repository for targetcli/rtsadmin?

Cheers,
        Thomas
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux