Re: [RFC PATCH 2/9] iscsi: associate endpoints with a host
I managed to fix the iSER endpoint issue by making endpoints created without a host valid again. Once I had iSER working, I went ahead and made it network namespace aware as well. Only tested with software roce (rxe) against the kernel target. I think the net_exit code might need to do a bit more with iSER. I'm going to look into that, then I'll merge some of these patches that fix earlier patches together, and get a new clean version of the set posted. Chris Leech (3): iscsi iser: fix iser, allow virtual endpoints again iscsi iser: direct network namespace support for endpoints iscsi iser: enable network namespace awareness in iser drivers/infiniband/ulp/iser/iscsi_iser.c | 13 --- drivers/scsi/iscsi_tcp.c | 10 ++ drivers/scsi/iscsi_tcp.h | 1 - drivers/scsi/libiscsi.c | 12 +++ drivers/scsi/scsi_transport_iscsi.c | 45 +++- include/scsi/libiscsi.h | 4 +++ include/scsi/scsi_transport_iscsi.h | 9 - 7 files changed, 71 insertions(+), 23 deletions(-) -- 2.39.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/20230421050521.49903-1-cleech%40redhat.com.
Re: [RFC PATCH 2/9] iscsi: associate endpoints with a host
On Wed, Feb 08, 2023 at 09:40:50AM -0800, Lee Duncan wrote: > Right now the iscsi_endpoint is only linked to a connection once that > connection has been established. For net namespace filtering of the > sysfs objects, associate an endpoint with the host that it was > allocated for when it is created. > > Signed-off-by: Chris Leech > Signed-off-by: Lee Duncan > --- > > diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c > b/drivers/infiniband/ulp/iser/iscsi_iser.c > index 6b7603765383..212fa7aa9810 100644 > --- a/drivers/infiniband/ulp/iser/iscsi_iser.c > +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c > @@ -802,7 +802,7 @@ static struct iscsi_endpoint > *iscsi_iser_ep_connect(struct Scsi_Host *shost, > struct iser_conn *iser_conn; > struct iscsi_endpoint *ep; > > - ep = iscsi_create_endpoint(0); > + ep = iscsi_create_endpoint(shost, 0); > if (!ep) > return ERR_PTR(-ENOMEM); I started trying[1] to look at iSER, and I think this is a problem. iSER is the only iSCSI driver that uses endpoint objects, but does not require then to be bound to a host. That means that iscsi_iser_ep_connect can be called with a null shost. So this fails, and not in a new namespace. It just breaks iSER entirely. I think we need to preserve support for the iscsi_endpoint device having a virtual device path for iSER. Also, enabling net namespace support for iSER might require the ability to create an endpoint directly in a namespace instead of on a host. Kind of like the create_session discussion for iscsi_tcp. - Chris [1] I say trying, becuase before going and borrowing an RDMA setup I thought I'd give the kernel target and either siw or rxe a try. The isert module seems to have issues with siw, and I think maybe any iWARP, where setting enable_iser on a port will try and re-use the TCP port number and fail due to it being in use. With rxe my host failed, but that's becuase of this create_endpoint issue. -- 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/20230420164232.GA27885%40localhost.
Re: [RFC PATCH 2/9] iscsi: associate endpoints with a host
On Tue, Mar 14, 2023 at 05:23:26PM +0100, Hannes Reinecke wrote: > On 2/8/23 18:40, Lee Duncan wrote: > > From: Lee Duncan > > @@ -230,6 +230,7 @@ iscsi_create_endpoint(int dd_size) > > ep->id = id; > > ep->dev.class = _endpoint_class; > > + ep->dev.parent = >shost_gendev; > > dev_set_name(>dev, "ep-%d", id); > > err = device_register(>dev); > > if (err) > > Umm... doesn't this change the sysfs layout? > IE won't the endpoint node be moved under the Scsi_Host directory? > > But even if it does: do we care? It does, but it shouldn't matter. The Open-iSCSI tools look under the subsystem, not the device path. Being a child of the host makes more sense then being a floating virtual device. I just re-tested with bnx2i to make sure moving an endpoint devpath in sysfs didn't break anything. - Chris -- 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/20230412023125.GA110%40localhost.
Re: [RFC PATCH 2/9] iscsi: associate endpoints with a host
On Mar 14, 2023, at 9:23 AM, Hannes Reinecke wrote: > > On 2/8/23 18:40, Lee Duncan wrote: >> From: Lee Duncan >> Right now the iscsi_endpoint is only linked to a connection once that >> connection has been established. For net namespace filtering of the >> sysfs objects, associate an endpoint with the host that it was >> allocated for when it is created. >> Signed-off-by: Chris Leech >> Signed-off-by: Lee Duncan >> --- >> drivers/infiniband/ulp/iser/iscsi_iser.c | 2 +- >> drivers/scsi/be2iscsi/be_iscsi.c | 2 +- >> drivers/scsi/bnx2i/bnx2i_iscsi.c | 2 +- >> drivers/scsi/cxgbi/libcxgbi.c| 2 +- >> drivers/scsi/qedi/qedi_iscsi.c | 2 +- >> drivers/scsi/qla4xxx/ql4_os.c| 2 +- >> drivers/scsi/scsi_transport_iscsi.c | 3 ++- >> include/scsi/scsi_transport_iscsi.h | 6 +- >> 8 files changed, 13 insertions(+), 8 deletions(-) >> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c >> b/drivers/infiniband/ulp/iser/iscsi_iser.c >> index 620ae5b2d80d..d38c909b462f 100644 >> --- a/drivers/infiniband/ulp/iser/iscsi_iser.c >> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c >> @@ -802,7 +802,7 @@ static struct iscsi_endpoint >> *iscsi_iser_ep_connect(struct Scsi_Host *shost, >> struct iser_conn *iser_conn; >> struct iscsi_endpoint *ep; >> - ep = iscsi_create_endpoint(0); >> +ep = iscsi_create_endpoint(shost, 0); >> if (!ep) >> return ERR_PTR(-ENOMEM); >> diff --git a/drivers/scsi/be2iscsi/be_iscsi.c >> b/drivers/scsi/be2iscsi/be_iscsi.c >> index 8aeaddc93b16..c893d193f5ef 100644 >> --- a/drivers/scsi/be2iscsi/be_iscsi.c >> +++ b/drivers/scsi/be2iscsi/be_iscsi.c >> @@ -1168,7 +1168,7 @@ beiscsi_ep_connect(struct Scsi_Host *shost, struct >> sockaddr *dst_addr, >> return ERR_PTR(ret); >> } >> - ep = iscsi_create_endpoint(sizeof(struct beiscsi_endpoint)); >> +ep = iscsi_create_endpoint(shost, sizeof(struct beiscsi_endpoint)); >> if (!ep) { >> ret = -ENOMEM; >> return ERR_PTR(ret); >> diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c >> b/drivers/scsi/bnx2i/bnx2i_iscsi.c >> index a3c800e04a2e..ac63e93e07c6 100644 >> --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c >> +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c >> @@ -384,7 +384,7 @@ static struct iscsi_endpoint *bnx2i_alloc_ep(struct >> bnx2i_hba *hba) >> struct bnx2i_endpoint *bnx2i_ep; >> u32 ec_div; >> - ep = iscsi_create_endpoint(sizeof(*bnx2i_ep)); >> +ep = iscsi_create_endpoint(hba->shost, sizeof(*bnx2i_ep)); >> if (!ep) { >> printk(KERN_ERR "bnx2i: Could not allocate ep\n"); >> return NULL; >> diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c >> index af281e271f88..94edf8e1fb0c 100644 >> --- a/drivers/scsi/cxgbi/libcxgbi.c >> +++ b/drivers/scsi/cxgbi/libcxgbi.c >> @@ -2926,7 +2926,7 @@ struct iscsi_endpoint *cxgbi_ep_connect(struct >> Scsi_Host *shost, >> goto release_conn; >> } >> - ep = iscsi_create_endpoint(sizeof(*cep)); >> +ep = iscsi_create_endpoint(shost, sizeof(*cep)); >> if (!ep) { >> err = -ENOMEM; >> pr_info("iscsi alloc ep, OOM.\n"); >> diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c >> index 31ec429104e2..4baf1dbb8e92 100644 >> --- a/drivers/scsi/qedi/qedi_iscsi.c >> +++ b/drivers/scsi/qedi/qedi_iscsi.c >> @@ -931,7 +931,7 @@ qedi_ep_connect(struct Scsi_Host *shost, struct sockaddr >> *dst_addr, >> return ERR_PTR(-ENXIO); >> } >> - ep = iscsi_create_endpoint(sizeof(struct qedi_endpoint)); >> +ep = iscsi_create_endpoint(shost, sizeof(struct qedi_endpoint)); >> if (!ep) { >> QEDI_ERR(>dbg_ctx, "endpoint create fail\n"); >> ret = -ENOMEM; >> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c >> index 005502125b27..acebf9c92c04 100644 >> --- a/drivers/scsi/qla4xxx/ql4_os.c >> +++ b/drivers/scsi/qla4xxx/ql4_os.c >> @@ -1717,7 +1717,7 @@ qla4xxx_ep_connect(struct Scsi_Host *shost, struct >> sockaddr *dst_addr, >> } >> ha = iscsi_host_priv(shost); >> -ep = iscsi_create_endpoint(sizeof(struct qla_endpoint)); >> +ep = iscsi_create_endpoint(shost, sizeof(struct qla_endpoint)); >> if (!ep) { >> ret = -ENOMEM; >> return ERR_PTR(ret); >> diff --git a/drivers/scsi/scsi_transport_iscsi.c >> b/drivers/scsi/scsi_transport_iscsi.c >> index be69cea9c6f8..86bafdb862a5 100644 >> --- a/drivers/scsi/scsi_transport_iscsi.c >> +++ b/drivers/scsi/scsi_transport_iscsi.c >> @@ -204,7 +204,7 @@ static struct attribute_group iscsi_endpoint_group = { >> }; >>struct iscsi_endpoint * >> -iscsi_create_endpoint(int dd_size) >> +iscsi_create_endpoint(struct Scsi_Host *shost, int dd_size) >> { >> struct iscsi_endpoint *ep; >> int err, id; >> @@ -230,6 +230,7 @@ iscsi_create_endpoint(int dd_size) >> ep->id = id; >> ep->dev.class =
Re: [RFC PATCH 2/9] iscsi: associate endpoints with a host
On 2/8/23 18:40, Lee Duncan wrote: From: Lee Duncan Right now the iscsi_endpoint is only linked to a connection once that connection has been established. For net namespace filtering of the sysfs objects, associate an endpoint with the host that it was allocated for when it is created. Signed-off-by: Chris Leech Signed-off-by: Lee Duncan --- drivers/infiniband/ulp/iser/iscsi_iser.c | 2 +- drivers/scsi/be2iscsi/be_iscsi.c | 2 +- drivers/scsi/bnx2i/bnx2i_iscsi.c | 2 +- drivers/scsi/cxgbi/libcxgbi.c| 2 +- drivers/scsi/qedi/qedi_iscsi.c | 2 +- drivers/scsi/qla4xxx/ql4_os.c| 2 +- drivers/scsi/scsi_transport_iscsi.c | 3 ++- include/scsi/scsi_transport_iscsi.h | 6 +- 8 files changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c index 620ae5b2d80d..d38c909b462f 100644 --- a/drivers/infiniband/ulp/iser/iscsi_iser.c +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c @@ -802,7 +802,7 @@ static struct iscsi_endpoint *iscsi_iser_ep_connect(struct Scsi_Host *shost, struct iser_conn *iser_conn; struct iscsi_endpoint *ep; - ep = iscsi_create_endpoint(0); + ep = iscsi_create_endpoint(shost, 0); if (!ep) return ERR_PTR(-ENOMEM); diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c index 8aeaddc93b16..c893d193f5ef 100644 --- a/drivers/scsi/be2iscsi/be_iscsi.c +++ b/drivers/scsi/be2iscsi/be_iscsi.c @@ -1168,7 +1168,7 @@ beiscsi_ep_connect(struct Scsi_Host *shost, struct sockaddr *dst_addr, return ERR_PTR(ret); } - ep = iscsi_create_endpoint(sizeof(struct beiscsi_endpoint)); + ep = iscsi_create_endpoint(shost, sizeof(struct beiscsi_endpoint)); if (!ep) { ret = -ENOMEM; return ERR_PTR(ret); diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index a3c800e04a2e..ac63e93e07c6 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -384,7 +384,7 @@ static struct iscsi_endpoint *bnx2i_alloc_ep(struct bnx2i_hba *hba) struct bnx2i_endpoint *bnx2i_ep; u32 ec_div; - ep = iscsi_create_endpoint(sizeof(*bnx2i_ep)); + ep = iscsi_create_endpoint(hba->shost, sizeof(*bnx2i_ep)); if (!ep) { printk(KERN_ERR "bnx2i: Could not allocate ep\n"); return NULL; diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c index af281e271f88..94edf8e1fb0c 100644 --- a/drivers/scsi/cxgbi/libcxgbi.c +++ b/drivers/scsi/cxgbi/libcxgbi.c @@ -2926,7 +2926,7 @@ struct iscsi_endpoint *cxgbi_ep_connect(struct Scsi_Host *shost, goto release_conn; } - ep = iscsi_create_endpoint(sizeof(*cep)); + ep = iscsi_create_endpoint(shost, sizeof(*cep)); if (!ep) { err = -ENOMEM; pr_info("iscsi alloc ep, OOM.\n"); diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c index 31ec429104e2..4baf1dbb8e92 100644 --- a/drivers/scsi/qedi/qedi_iscsi.c +++ b/drivers/scsi/qedi/qedi_iscsi.c @@ -931,7 +931,7 @@ qedi_ep_connect(struct Scsi_Host *shost, struct sockaddr *dst_addr, return ERR_PTR(-ENXIO); } - ep = iscsi_create_endpoint(sizeof(struct qedi_endpoint)); + ep = iscsi_create_endpoint(shost, sizeof(struct qedi_endpoint)); if (!ep) { QEDI_ERR(>dbg_ctx, "endpoint create fail\n"); ret = -ENOMEM; diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c index 005502125b27..acebf9c92c04 100644 --- a/drivers/scsi/qla4xxx/ql4_os.c +++ b/drivers/scsi/qla4xxx/ql4_os.c @@ -1717,7 +1717,7 @@ qla4xxx_ep_connect(struct Scsi_Host *shost, struct sockaddr *dst_addr, } ha = iscsi_host_priv(shost); - ep = iscsi_create_endpoint(sizeof(struct qla_endpoint)); + ep = iscsi_create_endpoint(shost, sizeof(struct qla_endpoint)); if (!ep) { ret = -ENOMEM; return ERR_PTR(ret); diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index be69cea9c6f8..86bafdb862a5 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -204,7 +204,7 @@ static struct attribute_group iscsi_endpoint_group = { }; struct iscsi_endpoint * -iscsi_create_endpoint(int dd_size) +iscsi_create_endpoint(struct Scsi_Host *shost, int dd_size) { struct iscsi_endpoint *ep; int err, id; @@ -230,6 +230,7 @@ iscsi_create_endpoint(int dd_size) ep->id = id; ep->dev.class = _endpoint_class; + ep->dev.parent = >shost_gendev; dev_set_name(>dev, "ep-%d", id); err = device_register(>dev); if (err) Umm... doesn't this change the sysfs layout? IE won't the endpoint node be moved under the
[RFC PATCH 2/9] iscsi: associate endpoints with a host
From: Lee Duncan Right now the iscsi_endpoint is only linked to a connection once that connection has been established. For net namespace filtering of the sysfs objects, associate an endpoint with the host that it was allocated for when it is created. Signed-off-by: Chris Leech Signed-off-by: Lee Duncan --- drivers/infiniband/ulp/iser/iscsi_iser.c | 2 +- drivers/scsi/be2iscsi/be_iscsi.c | 2 +- drivers/scsi/bnx2i/bnx2i_iscsi.c | 2 +- drivers/scsi/cxgbi/libcxgbi.c| 2 +- drivers/scsi/qedi/qedi_iscsi.c | 2 +- drivers/scsi/qla4xxx/ql4_os.c| 2 +- drivers/scsi/scsi_transport_iscsi.c | 3 ++- include/scsi/scsi_transport_iscsi.h | 6 +- 8 files changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c index 620ae5b2d80d..d38c909b462f 100644 --- a/drivers/infiniband/ulp/iser/iscsi_iser.c +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c @@ -802,7 +802,7 @@ static struct iscsi_endpoint *iscsi_iser_ep_connect(struct Scsi_Host *shost, struct iser_conn *iser_conn; struct iscsi_endpoint *ep; - ep = iscsi_create_endpoint(0); + ep = iscsi_create_endpoint(shost, 0); if (!ep) return ERR_PTR(-ENOMEM); diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c index 8aeaddc93b16..c893d193f5ef 100644 --- a/drivers/scsi/be2iscsi/be_iscsi.c +++ b/drivers/scsi/be2iscsi/be_iscsi.c @@ -1168,7 +1168,7 @@ beiscsi_ep_connect(struct Scsi_Host *shost, struct sockaddr *dst_addr, return ERR_PTR(ret); } - ep = iscsi_create_endpoint(sizeof(struct beiscsi_endpoint)); + ep = iscsi_create_endpoint(shost, sizeof(struct beiscsi_endpoint)); if (!ep) { ret = -ENOMEM; return ERR_PTR(ret); diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index a3c800e04a2e..ac63e93e07c6 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -384,7 +384,7 @@ static struct iscsi_endpoint *bnx2i_alloc_ep(struct bnx2i_hba *hba) struct bnx2i_endpoint *bnx2i_ep; u32 ec_div; - ep = iscsi_create_endpoint(sizeof(*bnx2i_ep)); + ep = iscsi_create_endpoint(hba->shost, sizeof(*bnx2i_ep)); if (!ep) { printk(KERN_ERR "bnx2i: Could not allocate ep\n"); return NULL; diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c index af281e271f88..94edf8e1fb0c 100644 --- a/drivers/scsi/cxgbi/libcxgbi.c +++ b/drivers/scsi/cxgbi/libcxgbi.c @@ -2926,7 +2926,7 @@ struct iscsi_endpoint *cxgbi_ep_connect(struct Scsi_Host *shost, goto release_conn; } - ep = iscsi_create_endpoint(sizeof(*cep)); + ep = iscsi_create_endpoint(shost, sizeof(*cep)); if (!ep) { err = -ENOMEM; pr_info("iscsi alloc ep, OOM.\n"); diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c index 31ec429104e2..4baf1dbb8e92 100644 --- a/drivers/scsi/qedi/qedi_iscsi.c +++ b/drivers/scsi/qedi/qedi_iscsi.c @@ -931,7 +931,7 @@ qedi_ep_connect(struct Scsi_Host *shost, struct sockaddr *dst_addr, return ERR_PTR(-ENXIO); } - ep = iscsi_create_endpoint(sizeof(struct qedi_endpoint)); + ep = iscsi_create_endpoint(shost, sizeof(struct qedi_endpoint)); if (!ep) { QEDI_ERR(>dbg_ctx, "endpoint create fail\n"); ret = -ENOMEM; diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c index 005502125b27..acebf9c92c04 100644 --- a/drivers/scsi/qla4xxx/ql4_os.c +++ b/drivers/scsi/qla4xxx/ql4_os.c @@ -1717,7 +1717,7 @@ qla4xxx_ep_connect(struct Scsi_Host *shost, struct sockaddr *dst_addr, } ha = iscsi_host_priv(shost); - ep = iscsi_create_endpoint(sizeof(struct qla_endpoint)); + ep = iscsi_create_endpoint(shost, sizeof(struct qla_endpoint)); if (!ep) { ret = -ENOMEM; return ERR_PTR(ret); diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index be69cea9c6f8..86bafdb862a5 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -204,7 +204,7 @@ static struct attribute_group iscsi_endpoint_group = { }; struct iscsi_endpoint * -iscsi_create_endpoint(int dd_size) +iscsi_create_endpoint(struct Scsi_Host *shost, int dd_size) { struct iscsi_endpoint *ep; int err, id; @@ -230,6 +230,7 @@ iscsi_create_endpoint(int dd_size) ep->id = id; ep->dev.class = _endpoint_class; + ep->dev.parent = >shost_gendev; dev_set_name(>dev, "ep-%d", id); err = device_register(>dev); if (err) diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h index 34c03707fb6e..268ccaac1c05 100644