| Message ID | 1695134522-126655-2-git-send-email-guwen@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/smc: implement virtual ISM extension and loopback-ism | expand |
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 > >
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 >> >>
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
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 --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))
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(-)