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