| Message ID | 1532953230-79945-3-git-send-email-oulijun@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | The follow up updates for hns | expand |
On Mon, Jul 30, 2018 at 08:20:26PM +0800, Lijun Ou wrote: > This patch mainly improves the create ah verb, includes add > IB_AH_GRH enable bit and assign the value for hoplimit of > address vector. Didn't update the comment after removing the IB_AH_GRH > Signed-off-by: Lijun Ou <oulijun@huawei.com> > V1 -> V2: > - Remove IB_AH_GRH macro judgement because the hip08 is only > RoCE driver > drivers/infiniband/hw/hns/hns_roce_ah.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/hw/hns/hns_roce_ah.c b/drivers/infiniband/hw/hns/hns_roce_ah.c > index 14efa3b..4100a1b 100644 > +++ b/drivers/infiniband/hw/hns/hns_roce_ah.c > @@ -48,6 +48,7 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, > struct device *dev = hr_dev->dev; > struct hns_roce_ah *ah; > u16 vlan_tag = 0xffff; > + struct in6_addr in6; > const struct ib_global_route *grh = rdma_ah_read_grh(ah_attr); > > ah = kzalloc(sizeof(*ah), GFP_ATOMIC); > @@ -55,7 +56,18 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, > return ERR_PTR(-ENOMEM); > > /* Get mac address */ > - memcpy(ah->av.mac, ah_attr->roce.dmac, ETH_ALEN); > + memcpy(&in6, grh->dgid.raw, sizeof(grh->dgid.raw)); > + if (rdma_is_multicast_addr(&in6)) { > + rdma_get_mcast_mac(&in6, ah->av.mac); This looks wrong, we don't stuff an 'in6' into the mac, I expect the driver to use the MAC provided by the core code... What is it doing? No other driver seems to do this? Is the core code missing something? > + } else { > + u8 *dmac = rdma_ah_retrieve_dmac(ah_attr); > + > + if (!dmac) { This can't actually fail, the core code guarentees the ah addr is roce type for roce drivers. > + kfree(ah); > + return ERR_PTR(-EINVAL); > + } > + memcpy(ah->av.mac, dmac, ETH_ALEN); Wrongly indented > + } > > gid_attr = ah_attr->grh.sgid_attr; > if (is_vlan_dev(gid_attr->ndev)) > @@ -68,7 +80,7 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, > > ah->av.port_pd = cpu_to_be32(to_hr_pd(ibpd)->pdn | > (rdma_ah_get_port_num(ah_attr) << > - HNS_ROCE_PORT_NUM_SHIFT)); > + HNS_ROCE_PORT_NUM_SHIFT)); ??? > ah->av.gid_index = grh->sgid_index; > ah->av.vlan = cpu_to_le16(vlan_tag); > dev_dbg(dev, "gid_index = 0x%x,vlan = 0x%x\n", ah->av.gid_index, > @@ -80,6 +92,10 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, > memcpy(ah->av.dgid, grh->dgid.raw, HNS_ROCE_GID_SIZE); > ah->av.sl_tclass_flowlabel = cpu_to_le32(rdma_ah_get_sl(ah_attr) << > HNS_ROCE_SL_SHIFT); > + ah->av.sl_tclass_flowlabel |= cpu_to_le32((grh->traffic_class << > + HNS_ROCE_TCLASS_SHIFT) | > + grh->flow_label); This doesn't do anything since the struct is zero'd and this is the only place writing to sl_tclass_flowlabel. > + ah->av.hop_limit = grh->hop_limit; You should probably make a patch that just does this. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
在 2018/7/31 10:41, Jason Gunthorpe 写道: > On Mon, Jul 30, 2018 at 08:20:26PM +0800, Lijun Ou wrote: >> This patch mainly improves the create ah verb, includes add >> IB_AH_GRH enable bit and assign the value for hoplimit of >> address vector. > > Didn't update the comment after removing the IB_AH_GRH > >> Signed-off-by: Lijun Ou <oulijun@huawei.com> >> V1 -> V2: >> - Remove IB_AH_GRH macro judgement because the hip08 is only >> RoCE driver >> drivers/infiniband/hw/hns/hns_roce_ah.c | 20 ++++++++++++++++++-- >> 1 file changed, 18 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/infiniband/hw/hns/hns_roce_ah.c b/drivers/infiniband/hw/hns/hns_roce_ah.c >> index 14efa3b..4100a1b 100644 >> +++ b/drivers/infiniband/hw/hns/hns_roce_ah.c >> @@ -48,6 +48,7 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, >> struct device *dev = hr_dev->dev; >> struct hns_roce_ah *ah; >> u16 vlan_tag = 0xffff; >> + struct in6_addr in6; >> const struct ib_global_route *grh = rdma_ah_read_grh(ah_attr); >> >> ah = kzalloc(sizeof(*ah), GFP_ATOMIC); >> @@ -55,7 +56,18 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, >> return ERR_PTR(-ENOMEM); >> >> /* Get mac address */ >> - memcpy(ah->av.mac, ah_attr->roce.dmac, ETH_ALEN); >> + memcpy(&in6, grh->dgid.raw, sizeof(grh->dgid.raw)); >> + if (rdma_is_multicast_addr(&in6)) { >> + rdma_get_mcast_mac(&in6, ah->av.mac); > > This looks wrong, we don't stuff an 'in6' into the mac, I expect the > driver to use the MAC provided by the core code... > > What is it doing? No other driver seems to do this? Is the core code > missing something? > >> + } else { >> + u8 *dmac = rdma_ah_retrieve_dmac(ah_attr); >> + >> + if (!dmac) { > > This can't actually fail, the core code guarentees the ah addr is roce > type for roce drivers. > >> + kfree(ah); >> + return ERR_PTR(-EINVAL); >> + } >> + memcpy(ah->av.mac, dmac, ETH_ALEN); > > Wrongly indented > >> + } >> >> gid_attr = ah_attr->grh.sgid_attr; >> if (is_vlan_dev(gid_attr->ndev)) >> @@ -68,7 +80,7 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, >> >> ah->av.port_pd = cpu_to_be32(to_hr_pd(ibpd)->pdn | >> (rdma_ah_get_port_num(ah_attr) << >> - HNS_ROCE_PORT_NUM_SHIFT)); >> + HNS_ROCE_PORT_NUM_SHIFT)); > > ??? > >> ah->av.gid_index = grh->sgid_index; >> ah->av.vlan = cpu_to_le16(vlan_tag); >> dev_dbg(dev, "gid_index = 0x%x,vlan = 0x%x\n", ah->av.gid_index, >> @@ -80,6 +92,10 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, >> memcpy(ah->av.dgid, grh->dgid.raw, HNS_ROCE_GID_SIZE); >> ah->av.sl_tclass_flowlabel = cpu_to_le32(rdma_ah_get_sl(ah_attr) << >> HNS_ROCE_SL_SHIFT); >> + ah->av.sl_tclass_flowlabel |= cpu_to_le32((grh->traffic_class << >> + HNS_ROCE_TCLASS_SHIFT) | >> + grh->flow_label); > > This doesn't do anything since the struct is zero'd and this is the > only place writing to sl_tclass_flowlabel. > >> + ah->av.hop_limit = grh->hop_limit; > > You should probably make a patch that just does this. > > Jason > ok, thanks. I will fix it and send the new patch. > . > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/hw/hns/hns_roce_ah.c b/drivers/infiniband/hw/hns/hns_roce_ah.c index 14efa3b..4100a1b 100644 --- a/drivers/infiniband/hw/hns/hns_roce_ah.c +++ b/drivers/infiniband/hw/hns/hns_roce_ah.c @@ -48,6 +48,7 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, struct device *dev = hr_dev->dev; struct hns_roce_ah *ah; u16 vlan_tag = 0xffff; + struct in6_addr in6; const struct ib_global_route *grh = rdma_ah_read_grh(ah_attr); ah = kzalloc(sizeof(*ah), GFP_ATOMIC); @@ -55,7 +56,18 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, return ERR_PTR(-ENOMEM); /* Get mac address */ - memcpy(ah->av.mac, ah_attr->roce.dmac, ETH_ALEN); + memcpy(&in6, grh->dgid.raw, sizeof(grh->dgid.raw)); + if (rdma_is_multicast_addr(&in6)) { + rdma_get_mcast_mac(&in6, ah->av.mac); + } else { + u8 *dmac = rdma_ah_retrieve_dmac(ah_attr); + + if (!dmac) { + kfree(ah); + return ERR_PTR(-EINVAL); + } + memcpy(ah->av.mac, dmac, ETH_ALEN); + } gid_attr = ah_attr->grh.sgid_attr; if (is_vlan_dev(gid_attr->ndev)) @@ -68,7 +80,7 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, ah->av.port_pd = cpu_to_be32(to_hr_pd(ibpd)->pdn | (rdma_ah_get_port_num(ah_attr) << - HNS_ROCE_PORT_NUM_SHIFT)); + HNS_ROCE_PORT_NUM_SHIFT)); ah->av.gid_index = grh->sgid_index; ah->av.vlan = cpu_to_le16(vlan_tag); dev_dbg(dev, "gid_index = 0x%x,vlan = 0x%x\n", ah->av.gid_index, @@ -80,6 +92,10 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd, memcpy(ah->av.dgid, grh->dgid.raw, HNS_ROCE_GID_SIZE); ah->av.sl_tclass_flowlabel = cpu_to_le32(rdma_ah_get_sl(ah_attr) << HNS_ROCE_SL_SHIFT); + ah->av.sl_tclass_flowlabel |= cpu_to_le32((grh->traffic_class << + HNS_ROCE_TCLASS_SHIFT) | + grh->flow_label); + ah->av.hop_limit = grh->hop_limit; return &ah->ibah; }
This patch mainly improves the create ah verb, includes add IB_AH_GRH enable bit and assign the value for hoplimit of address vector. Signed-off-by: Lijun Ou <oulijun@huawei.com> --- V1 -> V2: - Remove IB_AH_GRH macro judgement because the hip08 is only RoCE driver --- drivers/infiniband/hw/hns/hns_roce_ah.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)