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@xxxxxxxxxx>
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
