Re: [PATCH 11/11] iscsi: force destroy sesions when a network namespace exits
On 5/10/23 1:09 PM, michael.chris...@oracle.com wrote:
> On 5/6/23 4:29 PM, 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.
>>
>> Reviewed-by: Hannes Reinecke
>> Signed-off-by: Chris Leech
>> ---
>> drivers/scsi/scsi_transport_iscsi.c | 16
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/scsi/scsi_transport_iscsi.c
>> b/drivers/scsi/scsi_transport_iscsi.c
>> index 15d28186996d..10e9414844d8 100644
>> --- a/drivers/scsi/scsi_transport_iscsi.c
>> +++ b/drivers/scsi/scsi_transport_iscsi.c
>> @@ -5235,9 +5235,25 @@ static int __net_init iscsi_net_init(struct net *net)
>>
>> static void __net_exit iscsi_net_exit(struct net *net)
>> {
>> +struct iscsi_cls_session *session, *tmp;
>> struct iscsi_net *isn;
>> +unsigned long flags;
>> +LIST_HEAD(sessions);
>>
>> isn = net_generic(net, iscsi_net_id);
>> +
>> +spin_lock_irqsave(>sesslock, flags);
>> +list_replace_init(>sesslist, );
>> +spin_unlock_irqrestore(>sesslock, flags);
>> +
>> +/* force session destruction, there is no userspace anymore */
>> +list_for_each_entry_safe(session, tmp, , sess_list) {
>> +device_for_each_child(>dev, NULL,
>> + iscsi_iter_force_destroy_conn_fn);
>> +flush_work(>destroy_work);
>
> I think if this flush_work actually flushed, then we would be doing a use
> after free because the running work would free the session from under us.
>
> We should never have a running destroy_work and be ale to see it on the
> sesslist
> right? Maybe a WARN_ON or something else so it doesn't look like a possible
> bug.
Maybe not a WARN_ON. What happens if there is running destroy_works for this
namespace and we return from this function?
>
>
>> +__iscsi_destroy_session(>destroy_work);
>> +}
>> +
>> netlink_kernel_release(isn->nls);
>> isn->nls = NULL;
>> }
>
--
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/2059dc74-a00b-fd53-f5ce-3dd41bfbf4f1%40oracle.com.
Re: [PATCH 11/11] iscsi: force destroy sesions when a network namespace exits
On 5/6/23 4:29 PM, 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.
>
> Reviewed-by: Hannes Reinecke
> Signed-off-by: Chris Leech
> ---
> drivers/scsi/scsi_transport_iscsi.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/scsi/scsi_transport_iscsi.c
> b/drivers/scsi/scsi_transport_iscsi.c
> index 15d28186996d..10e9414844d8 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -5235,9 +5235,25 @@ static int __net_init iscsi_net_init(struct net *net)
>
> static void __net_exit iscsi_net_exit(struct net *net)
> {
> + struct iscsi_cls_session *session, *tmp;
> struct iscsi_net *isn;
> + unsigned long flags;
> + LIST_HEAD(sessions);
>
> isn = net_generic(net, iscsi_net_id);
> +
> + spin_lock_irqsave(>sesslock, flags);
> + list_replace_init(>sesslist, );
> + spin_unlock_irqrestore(>sesslock, flags);
> +
> + /* force session destruction, there is no userspace anymore */
> + list_for_each_entry_safe(session, tmp, , sess_list) {
> + device_for_each_child(>dev, NULL,
> + iscsi_iter_force_destroy_conn_fn);
> + flush_work(>destroy_work);
I think if this flush_work actually flushed, then we would be doing a use
after free because the running work would free the session from under us.
We should never have a running destroy_work and be ale to see it on the sesslist
right? Maybe a WARN_ON or something else so it doesn't look like a possible
bug.
> + __iscsi_destroy_session(>destroy_work);
> + }
> +
> netlink_kernel_release(isn->nls);
> isn->nls = NULL;
> }
--
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/6ea35c03-09b9-425d-ddcd-8cdbf99f4fe8%40oracle.com.
[PATCH 11/11] iscsi: force destroy sesions when a network namespace exits
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.
Reviewed-by: Hannes Reinecke
Signed-off-by: Chris Leech
---
drivers/scsi/scsi_transport_iscsi.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index 15d28186996d..10e9414844d8 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -5235,9 +5235,25 @@ static int __net_init iscsi_net_init(struct net *net)
static void __net_exit iscsi_net_exit(struct net *net)
{
+ struct iscsi_cls_session *session, *tmp;
struct iscsi_net *isn;
+ unsigned long flags;
+ LIST_HEAD(sessions);
isn = net_generic(net, iscsi_net_id);
+
+ spin_lock_irqsave(>sesslock, flags);
+ list_replace_init(>sesslist, );
+ spin_unlock_irqrestore(>sesslock, flags);
+
+ /* force session destruction, there is no userspace anymore */
+ list_for_each_entry_safe(session, tmp, , sess_list) {
+ device_for_each_child(>dev, NULL,
+ iscsi_iter_force_destroy_conn_fn);
+ flush_work(>destroy_work);
+ __iscsi_destroy_session(>destroy_work);
+ }
+
netlink_kernel_release(isn->nls);
isn->nls = NULL;
}
--
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/20230506232930.195451-12-cleech%40redhat.com.
Re: [PATCH 11/11] iscsi: force destroy sesions when a network namespace exits
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: [PATCH 11/11] iscsi: force destroy sesions when a network namespace exits
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: [PATCH 11/11] iscsi: force destroy sesions when a network namespace exits
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. Signed-off-by: Chris Leech --- drivers/scsi/scsi_transport_iscsi.c | 18 ++ 1 file changed, 18 insertions(+) Reviewed-by: Hannes Reinecke 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/85458436-702f-2e38-c7cc-ff7329731eda%40suse.de.
[PATCH 11/11] iscsi: force destroy sesions when a network namespace exits
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.
Signed-off-by: Chris Leech
---
drivers/scsi/scsi_transport_iscsi.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index 3a068d8eca2d..8fafa8f0e0df 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -5200,9 +5200,27 @@ static int __net_init iscsi_net_init(struct net *net)
static void __net_exit iscsi_net_exit(struct net *net)
{
+ struct iscsi_cls_session *session, *tmp;
+ struct iscsi_transport *transport;
struct iscsi_net *isn;
+ unsigned long flags;
+ LIST_HEAD(sessions);
isn = net_generic(net, iscsi_net_id);
+
+ /* force session destruction, there is no userspace anymore */
+ spin_lock_irqsave(>sesslock, flags);
+ list_for_each_entry_safe(session, tmp, >sesslist, sess_list) {
+ list_move_tail(>sess_list, );
+ }
+ spin_unlock_irqrestore(>sesslock, flags);
+ list_for_each_entry_safe(session, tmp, , sess_list) {
+ device_for_each_child(>dev, NULL,
+ iscsi_iter_force_destroy_conn_fn);
+ transport = session->transport;
+ transport->destroy_session(session);
+ }
+
netlink_kernel_release(isn->nls);
isn->nls = NULL;
}
--
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/20230410191033.1069293-3-cleech%40redhat.com.
