diff mbox series

[net-next,01/18] net/smc: decouple ism_dev from SMC-D device dump

Message ID 1695134522-126655-2-git-send-email-guwen@linux.alibaba.com
State New
Headers
Series net/smc: implement virtual ISM extension and loopback-ism |

Commit Message

Wen Gu Sept. 19, 2023, 2:41 p.m. UTC 
  This patch helps to decouple ISM device from SMC-D device, allowing
different underlying device forms, such as virtual ISM devices.

Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
---
 net/smc/smc_ism.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Simon Horman Sept. 21, 2023, 8:41 p.m. UTC | #1 
On Tue, Sep 19, 2023 at 10:41:45PM +0800, Wen Gu wrote:
> This patch helps to decouple ISM device from SMC-D device, allowing
> different underlying device forms, such as virtual ISM devices.
> 
> Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
> ---
>  net/smc/smc_ism.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
> index fbee249..0045fee 100644
> --- a/net/smc/smc_ism.c
> +++ b/net/smc/smc_ism.c
> @@ -230,12 +230,11 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
>  	char smc_pnet[SMC_MAX_PNETID_LEN + 1];
>  	struct smc_pci_dev smc_pci_dev;
>  	struct nlattr *port_attrs;
> +	struct device *priv_dev;
>  	struct nlattr *attrs;
> -	struct ism_dev *ism;
>  	int use_cnt = 0;
>  	void *nlh;
>  
> -	ism = smcd->priv;
>  	nlh = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
>  			  &smc_gen_nl_family, NLM_F_MULTI,
>  			  SMC_NETLINK_GET_DEV_SMCD);
> @@ -250,7 +249,10 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
>  	if (nla_put_u8(skb, SMC_NLA_DEV_IS_CRIT, use_cnt > 0))
>  		goto errattr;
>  	memset(&smc_pci_dev, 0, sizeof(smc_pci_dev));

Hi Wen Gu,

priv_dev is uninitialised here.

> -	smc_set_pci_values(to_pci_dev(ism->dev.parent), &smc_pci_dev);
> +	if (smcd->ops->get_dev)
> +		priv_dev = smcd->ops->get_dev(smcd);

It is conditionally initialised here.

> +	if (priv_dev->parent)

But unconditionally dereferenced here.

As flagged by clang-16 W=1, and Smatch

> +		smc_set_pci_values(to_pci_dev(priv_dev->parent), &smc_pci_dev);
>  	if (nla_put_u32(skb, SMC_NLA_DEV_PCI_FID, smc_pci_dev.pci_fid))
>  		goto errattr;
>  	if (nla_put_u16(skb, SMC_NLA_DEV_PCI_CHID, smc_pci_dev.pci_pchid))
> -- 
> 1.8.3.1
> 
>
Wen Gu Sept. 22, 2023, 8:05 a.m. UTC | #2 
On 2023/9/22 04:41, Simon Horman wrote:
> On Tue, Sep 19, 2023 at 10:41:45PM +0800, Wen Gu wrote:
>> This patch helps to decouple ISM device from SMC-D device, allowing
>> different underlying device forms, such as virtual ISM devices.
>>
>> Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
>> ---
>>   net/smc/smc_ism.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
>> index fbee249..0045fee 100644
>> --- a/net/smc/smc_ism.c
>> +++ b/net/smc/smc_ism.c
>> @@ -230,12 +230,11 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
>>   	char smc_pnet[SMC_MAX_PNETID_LEN + 1];
>>   	struct smc_pci_dev smc_pci_dev;
>>   	struct nlattr *port_attrs;
>> +	struct device *priv_dev;
>>   	struct nlattr *attrs;
>> -	struct ism_dev *ism;
>>   	int use_cnt = 0;
>>   	void *nlh;
>>   
>> -	ism = smcd->priv;
>>   	nlh = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
>>   			  &smc_gen_nl_family, NLM_F_MULTI,
>>   			  SMC_NETLINK_GET_DEV_SMCD);
>> @@ -250,7 +249,10 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
>>   	if (nla_put_u8(skb, SMC_NLA_DEV_IS_CRIT, use_cnt > 0))
>>   		goto errattr;
>>   	memset(&smc_pci_dev, 0, sizeof(smc_pci_dev));
> 
> Hi Wen Gu,
> 
> priv_dev is uninitialised here.
> 
>> -	smc_set_pci_values(to_pci_dev(ism->dev.parent), &smc_pci_dev);
>> +	if (smcd->ops->get_dev)
>> +		priv_dev = smcd->ops->get_dev(smcd);
> 
> It is conditionally initialised here.
> 
>> +	if (priv_dev->parent)
> 
> But unconditionally dereferenced here.
> 
> As flagged by clang-16 W=1, and Smatch
> 

Hi Simon. Yes, I fixed it in v3. Thank you!

>> +		smc_set_pci_values(to_pci_dev(priv_dev->parent), &smc_pci_dev);
>>   	if (nla_put_u32(skb, SMC_NLA_DEV_PCI_FID, smc_pci_dev.pci_fid))
>>   		goto errattr;
>>   	if (nla_put_u16(skb, SMC_NLA_DEV_PCI_CHID, smc_pci_dev.pci_pchid))
>> -- 
>> 1.8.3.1
>>
>>
Gerd Bayer Sept. 22, 2023, 6:13 p.m. UTC | #3 
On Fri, 2023-09-22 at 16:05 +0800, Wen Gu wrote:
> On 2023/9/22 04:41, Simon Horman wrote:
> > On Tue, Sep 19, 2023 at 10:41:45PM +0800, Wen Gu wrote:
> > 
> > priv_dev is uninitialised here.
> > 
> > > -       smc_set_pci_values(to_pci_dev(ism->dev.parent),
> > > &smc_pci_dev);
> > > +       if (smcd->ops->get_dev)
> > > +               priv_dev = smcd->ops->get_dev(smcd);
> > 
> > It is conditionally initialised here.
> > 
> > > +       if (priv_dev->parent)
> > 
> > But unconditionally dereferenced here.
> > 
> > As flagged by clang-16 W=1, and Smatch
> > 
> 
> Hi Simon. Yes, I fixed it in v3. Thank you!

Hi Wen Gu,

seems like there is some email filter at work. Neither v2 nor v3 made
it to the netdev mailing list - nor to patchwork.kernel.org.
There's traces of Wenjia's replies and your replies to her - but not
the original mail.

Could you please check? Thanks!
Gerd
Wen Gu Sept. 23, 2023, 9:24 a.m. UTC | #4 
On 2023/9/23 02:13, Gerd Bayer wrote:

> Hi Wen Gu,
> 
> seems like there is some email filter at work. Neither v2 nor v3 made
> it to the netdev mailing list - nor to patchwork.kernel.org.
> There's traces of Wenjia's replies and your replies to her - but not
> the original mail.
> 
> Could you please check? Thanks!
> Gerd

Yes, it is ture. v2 and v3 was refused by ver.kernel.org.

I will send the v4 based on Wenjia's comments as soon as possible,
and add CC of you, Sandy, Niklas and Halil in v4 in case the filter
happens again.

Thank you very much for your reminder!

Regards,
Wen Gu
diff mbox series

Patch

diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
index fbee249..0045fee 100644
--- a/net/smc/smc_ism.c
+++ b/net/smc/smc_ism.c
@@ -230,12 +230,11 @@  static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
 	char smc_pnet[SMC_MAX_PNETID_LEN + 1];
 	struct smc_pci_dev smc_pci_dev;
 	struct nlattr *port_attrs;
+	struct device *priv_dev;
 	struct nlattr *attrs;
-	struct ism_dev *ism;
 	int use_cnt = 0;
 	void *nlh;
 
-	ism = smcd->priv;
 	nlh = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
 			  &smc_gen_nl_family, NLM_F_MULTI,
 			  SMC_NETLINK_GET_DEV_SMCD);
@@ -250,7 +249,10 @@  static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
 	if (nla_put_u8(skb, SMC_NLA_DEV_IS_CRIT, use_cnt > 0))
 		goto errattr;
 	memset(&smc_pci_dev, 0, sizeof(smc_pci_dev));
-	smc_set_pci_values(to_pci_dev(ism->dev.parent), &smc_pci_dev);
+	if (smcd->ops->get_dev)
+		priv_dev = smcd->ops->get_dev(smcd);
+	if (priv_dev->parent)
+		smc_set_pci_values(to_pci_dev(priv_dev->parent), &smc_pci_dev);
 	if (nla_put_u32(skb, SMC_NLA_DEV_PCI_FID, smc_pci_dev.pci_fid))
 		goto errattr;
 	if (nla_put_u16(skb, SMC_NLA_DEV_PCI_CHID, smc_pci_dev.pci_pchid))