Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3e714766-f2cd-3bca-3611-22a87848fec7@oracle.com>
Date:   Mon, 7 Mar 2022 10:12:25 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.1
Subject: Re: [PATCH 0/2]scsi:libiscsi: Add iscsi_cls_conn device to sysfs
 correctly
Content-Language: en-US
To:     Wenchao Hao <haowenchao@huawei.com>, Lee Duncan <lduncan@suse.com>,
        Chris Leech <cleech@redhat.com>,
        "James E . J . Bottomley" <jejb@linux.ibm.com>,
        "Martin K . Petersen" <martin.petersen@oracle.com>,
        open-iscsi@googlegroups.com, linux-scsi@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Wu Bo <wubo40@huawei.com>, Zhiqiang Liu <liuzhiqiang26@huawei.com>
References: <20220308005654.2281343-1-haowenchao@huawei.com>
From:   Mike Christie <michael.christie@oracle.com>
In-Reply-To: <20220308005654.2281343-1-haowenchao@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UURYRFZlbkVLQ21GT2JUNDFLWEVnSGVYNVdSbTNSTzNMN2FlRTc4NUZYVHdt?=
 =?utf-8?B?MDZVbjV1ZU5wZW92dU5VeDFKQ0QvVENXbTl1UG44YkNIWHhtL2Z0NmhkbWNQ?=
 =?utf-8?B?OERlVXBnbTVQM1BBSzJ4UXB5N0JYZzM2K0tCNTFBbmhwZnNaN3M0ZVdHMnJE?=
 =?utf-8?B?M2hXWG5nM0xha0pPakRSZElzaEZ0d1dxaitWVnNHWlFNUis2LzZkYWhtS2Fx?=
 =?utf-8?B?NC9hR1M1R3dDUEFPZjJlY0FlcE5mdDRVaXJnNWZuUzlRdTFWdVlhem9xbGJK?=
 =?utf-8?B?R0xCVTJOVWtFUUgrSjVRd2xDa1VXbi9KQ2Z0V1l0Y1BJWFdITEtTelhyeU5X?=
 =?utf-8?B?UU44d2JDN05tU3RaUUhYRjM4UXpaYmxOTkwyRlNhZytwTG5zSUtKeTcyL1kz?=
 =?utf-8?B?eVVCQ0doTGFnam1tVkgyN1R5OWg2bFhJQ0Fyb1U3bUxxOFRWd20zOXZFUGUw?=
 =?utf-8?B?dWgvdlVVN3Vmbk5YbS8zY1c1dk9rNk5yRDRzdGtlR0NxUHVJNE5sV3RhZm1j?=
 =?utf-8?B?Q1BPanVDN2MvckhsaVd5ZHo1T1ZtdlA4ZkswRmllYi8wQnJIWXF4YksvMmNM?=
 =?utf-8?B?Qk55UDZZL2JQMlY5ZlFzRWwxek1SYmdZdC9HY3B6QThZVE1HR0QrRkljUitW?=
 =?utf-8?B?VGg4UXRoKzl6dDNodGtDa04yMnBPT2lDcHZiVFIybWw1aFVHS2hIY3pwRkpD?=
 =?utf-8?B?U051ZnlPVVBoK2J3SEcwa0xTdUZvbTh1dm5KTXF4UGl3UE51K1g2d01MU0I2?=
 =?utf-8?B?aXlYWHJqdUhLYTFaYVZRV2hvTXJvcHFLbDRMa1NpQ0ViaG92MEtUTnFSaEFV?=
 =?utf-8?B?MmJuaVk2d3hFL1NuYXlyL2ZkVE94alkrNXZKVXpybW5xT2xkWXBIWlFhZzVY?=
 =?utf-8?B?a2dMMGJ1NVJUdXhuMGZJYjJ5SmkyOHBQc0RpU0FrdmpIZDVjYytsbVdYcWdK?=
 =?utf-8?B?dmdaeFh0WEJJd3k5Nm45WGwrVXdLOGZvS1k1VlpmT2QvYTFXeWEvRzNxOVdr?=
 =?utf-8?B?U01iQUNSWTRiSS9yU1pwUE5SMUQyczdpTGNCM051T2dNT0lrM25zOTB3OE9q?=
 =?utf-8?B?R1dFZFMrMVR4N2Z0SlduOHR0ZXBjekR5NzJWZUlwb01id0tyOWgvNUxKWWNv?=
 =?utf-8?B?Z05rUGpVNElURitiaFYyWXNYQ0VPVHJ4SStGemRsOGNPaGlaQVMzU01SSHdr?=
 =?utf-8?B?ZlBaMW1uSGhMZDIxNlg2aGRNTlRXN2dCRzhoZUIrQ2JNZFBEVEFVNjJWczkw?=
 =?utf-8?B?akZpYWIxN1NVWDdFVkM3SGJ4NG5CYjJSdkI5aFdsTnJCTGJveUM1a2NVWWxv?=
 =?utf-8?B?UG5DZW1DcGREcE9ZU1E2dmI3dEhLcXliSEk3Vk1oMktMOTZPR25nd04xNVRz?=
 =?utf-8?B?eDE1ajA5RkpqL01xUFNmbTNYYXdIemlBQ0FFd1p0c1hFUTN2ZlFybkdhOTQ5?=
 =?utf-8?B?WlFGaUZZOVltb3l6MDk0SUozbUdnZWZPWlJOUE56cFdpR0piU0pSdnZLUXU3?=
 =?utf-8?B?YkhDKzNwYlVvOTQ4eDdCZ0FnRGpsVXpMSCtUR1BmVk5ocEE0dlVoZUd3cFYr?=
 =?utf-8?B?SlNUdWo5b3JsSXk1ZnUzQkVlZkRXNldUeHRHOVpKd1lqK0ZvVWtvOFRNaUpt?=
 =?utf-8?B?djJiSFlreXdUTnJjSWxnd3BLeGZqNEg0enRpQmhuc1NwYzVEQkIwUzhvaGlV?=
 =?utf-8?B?ZnBJaWFwWmRYNy9RUGpUUGhXbjUyeHd1QkZldU1ybk81MFk2Uk5xNktmd2h2?=
 =?utf-8?B?RVRrQytwVzdpSlkzdk9UcW9YSWpkMzRjUGxKOVRBL1RqVCtkdFQxTXpsZTkz?=
 =?utf-8?B?WE5BSTVYSHZiVFJOd1podytIUkF3cXRHZERueUJNcEJpT1FYdlVRdlViaUdu?=
 =?utf-8?B?Uzg1SkZmWmhkOXFlZndUNXFYeVdjU2RuSmhMZWxRbXVIS3JuMURHdzE4b2M5?=
 =?utf-8?B?Vzk3VlY0ZHBKV1Q0TE41VGFicisxemdhMnB2Z0dIZG9iZ0Z6M0hqUlc4MHJ2?=
 =?utf-8?B?QWh1RjAzK3lPeWZ0MXBncVlFT3hKRW5hR1I4elFQRHJIUWFpZ1ZCZ2xLUTB6?=
 =?utf-8?B?SVppZ3RuSEsvUkJMOXpKdXJXcGZvQ3hEdy83aVV4NDhLdmM4NU44czBLZVE2?=
 =?utf-8?B?bUpSdFp4TU5CRlhpWUhaQWRTK2dKT01valJsVW00blhIS3FqTXFRTlNiZFM5?=
 =?utf-8?B?OFlhbHBpbEk1Qk5PMG9LMWJTSEdzQmc4cHhiUlFMSzNPOTA2M2RtVWh0OWNR?=
 =?utf-8?B?RlowVHhTS3ViMktrcHFIME1BVkJnPT0=?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 18153971-f3ef-49f5-c28a-08da0055455e
X-MS-Exchange-CrossTenant-AuthSource: DM5PR10MB1466.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2022 16:12:26.3811
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: UosB/MSbsMmLZwmbtZnRnyidk0/TuN7eKrGtPv3c+OcmU75xSvaKviv79jr7i+PJfi/zIvPlg7mUYxBBT/tXlJpoPD6ZCdCjPCR10qajW1E=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR10MB2024
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10279 signatures=690470
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=800 spamscore=0
 phishscore=0 bulkscore=0 adultscore=0 malwarescore=0 suspectscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2203070094
X-Proofpoint-GUID: kTqFDDNYy3eXqL9O3ehEytKv0T8qj_Au
X-Proofpoint-ORIG-GUID: kTqFDDNYy3eXqL9O3ehEytKv0T8qj_Au
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 3/7/22 6:56 PM, Wenchao Hao wrote:
> We found a NULL pointer dereference in iscsi_sw_tcp_conn_get_param(),
> the root reason is we did sysfs addition wrong.
> 
> iscsi_create_conn() expose iscsi_cls_conn to sysfs while the related
> resources are not initialized. So we should delay the calling of
> device_add() until these resources has been initialized.
> 
> This patchset solve this issue by changing iscsi_conn_setup() and works 
> well for iscsi_tcp.
> 

Overall I think you need to also fix up the drivers. It just makes it a
nicer driver API where the LLDs don't know about sysfs and doesn't have
to worry about it.

Let's start with just this first piece where we handle sysfs in the lib
and class like you are doing in this patchset. We can do the LLDs
interaction with the lib in a second patchset to make this easier and fix
the initial bug and cleanup some code.

In a separate patchset, we can then go deeper and maybe just merge/kill some
of the lib/class interface since every driver except qla4xxx hooks into the
lib. So we have this distinction just for that one driver's session mode
and that doesn't make a lot of sense.