Re: [PATCH 11/11] iscsi: force destroy sesions when a network namespace exits

2023-04-11 Thread Hannes Reinecke

On 4/11/23 20:19, Chris Leech wrote:

On Tue, Apr 11, 2023 at 08:21:22AM +0200, Hannes Reinecke wrote:

On 4/10/23 21:10, Chris Leech wrote:

The namespace is gone, so there is no userspace to clean up.
Force close all the sessions.

This should be enough for software transports, there's no implementation
of migrating physical iSCSI hosts between network namespaces currently.


Ah, you shouldn't have mentioned that.
(Not quite sure how being namespace-aware relates to migration, though.)
We should be checking/modifying the iSCSI offload drivers, too.
But maybe with a later patch.


I shouldn't have left that opening ;-)

The idea with this design is to keep everything rooted on the
iscsi_host, and for physical HBAs those stay assigned to init_net.
With this patch set, offload drivers remain unusable in a net namespace
other than init_net. They simply are not visible.

By migration, I was implying the possibilty of assigment of an HBA
iscsi_host into a namespace like you can do with a network interface.
Such an iscsi_host would then need to be migrated back to init_net on
namespace exit.

I don't think it works to try and share an iscsi_host across namespaces,
and manage different sessions. The iSCSI HBAs have a limited number of
network configurations, exposed as iscsi_iface objects, and I don't want
to go down the road of figuring out how to share those.


Ah, yes, indeed.

Quite some iSCSI offloads create the network session internally (or 
don't even have one), so making them namespace aware will be tricky.


But then I guess we should avoid creating offload sessions from other 
namespaces; preferably by a patch for the kernel such that userspace can 
run unmodified.


Cheers,

Hannes
--
Dr. Hannes ReineckeKernel Storage Architect
h...@suse.de  +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

--
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/e7b55e2d-4bd1-eabe-43b6-ef00da69935a%40suse.de.


Re: [RFC PATCH 2/9] iscsi: associate endpoints with a host

2023-04-11 Thread Chris Leech
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 = &iscsi_endpoint_class;
> > +   ep->dev.parent = &shost->shost_gendev;
> > dev_set_name(&ep->dev, "ep-%d", id);
> > err = device_register(&ep->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: [PATCH 11/11] iscsi: force destroy sesions when a network namespace exits

2023-04-11 Thread Chris Leech
On Tue, Apr 11, 2023 at 08:21:22AM +0200, Hannes Reinecke wrote:
> On 4/10/23 21:10, Chris Leech wrote:
> > The namespace is gone, so there is no userspace to clean up.
> > Force close all the sessions.
> > 
> > This should be enough for software transports, there's no implementation
> > of migrating physical iSCSI hosts between network namespaces currently.
> > 
> Ah, you shouldn't have mentioned that.
> (Not quite sure how being namespace-aware relates to migration, though.)
> We should be checking/modifying the iSCSI offload drivers, too.
> But maybe with a later patch.

I shouldn't have left that opening ;-)

The idea with this design is to keep everything rooted on the
iscsi_host, and for physical HBAs those stay assigned to init_net.
With this patch set, offload drivers remain unusable in a net namespace
other than init_net. They simply are not visible.

By migration, I was implying the possibilty of assigment of an HBA
iscsi_host into a namespace like you can do with a network interface.
Such an iscsi_host would then need to be migrated back to init_net on
namespace exit.

I don't think it works to try and share an iscsi_host across namespaces,
and manage different sessions. The iSCSI HBAs have a limited number of
network configurations, exposed as iscsi_iface objects, and I don't want
to go down the road of figuring out how to share those.

- 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/20230411181945.GB1234639%40localhost.


Re: [RFC PATCH 5/9] iscsi: set netns for iscsi_tcp hosts

2023-04-11 Thread Chris Leech
On Tue, Apr 11, 2023 at 08:58:54AM +0200, Hannes Reinecke wrote:
> On 4/11/23 02:21, Chris Leech wrote:
> > diff --git a/include/scsi/scsi_transport_iscsi.h 
> > b/include/scsi/scsi_transport_iscsi.h
> > index 0c3fd690ecf8..4d8a3d770bed 100644
> > --- a/include/scsi/scsi_transport_iscsi.h
> > +++ b/include/scsi/scsi_transport_iscsi.h
> > @@ -79,6 +79,9 @@ struct iscsi_transport {
> > struct iscsi_cls_session *(*create_session) (struct iscsi_endpoint *ep,
> > uint16_t cmds_max, uint16_t qdepth,
> > uint32_t sn);
> > +   struct iscsi_cls_session *(*create_unbound_session) (struct net *net,
> > +   uint16_t cmds_max, uint16_t qdepth,
> > +   uint32_t sn);
> > void (*destroy_session) (struct iscsi_cls_session *session);
> > struct iscsi_cls_conn *(*create_conn) (struct iscsi_cls_session *sess,
> > uint32_t cid);
> 
> I'm not _that_ happy with these two functions; but can't really see a way
> around it.
> Can't we rename the 'unbound' version to
> 'create_session_ns' or something?

Yes, in my mind I was matching the netlink commands, but those are
create_session and create_bound_session. I got it exactly backwards
with which one had the additional text.

I'm OK with changing to a shorter name, like the one you suggested.

Thanks,
- 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/20230411180337.GA1234639%40localhost.