On Tue, Sep 18, 2018 at 07:40:46PM +0800, oulijun wrote:
> 在 2018/9/18 16:54, Leon Romanovsky 写道:
> > On Mon, Sep 17, 2018 at 09:55:15PM +0800, Lijun Ou wrote:
> >> According to the hardware modification, the vlan of the UD
> >> packet is based on the ud_vlan_en field of ud wqe to
> >> determine whether to add a vlan header to the ud
> >> packet. The ud_vlan_en field is filled by the driver
> >> according to judge the net device.
> >>
> >> Signed-off-by: Lijun Ou <oulijun@xxxxxxxxxx>
> >> ---
> >> V1->V2:
> >> - Assign the vlan_en for true when the device is vlan device
> >> ---
> >> drivers/infiniband/hw/hns/hns_roce_ah.c | 6 +++++-
> >> drivers/infiniband/hw/hns/hns_roce_device.h | 1 +
> >> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 3 +++
> >> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 2 ++
> >> 4 files changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/infiniband/hw/hns/hns_roce_ah.c b/drivers/infiniband/hw/hns/hns_roce_ah.c
> >> index 0d96c5b..9990dc9 100644
> >> --- a/drivers/infiniband/hw/hns/hns_roce_ah.c
> >> +++ b/drivers/infiniband/hw/hns/hns_roce_ah.c
> >> @@ -49,6 +49,7 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd,
> >> struct hns_roce_ah *ah;
> >> u16 vlan_tag = 0xffff;
> >> const struct ib_global_route *grh = rdma_ah_read_grh(ah_attr);
> >> + bool vlan_en = false;
> >>
> >> ah = kzalloc(sizeof(*ah), GFP_ATOMIC);
> >> if (!ah)
> >> @@ -58,8 +59,10 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd,
> >> memcpy(ah->av.mac, ah_attr->roce.dmac, ETH_ALEN);
> >>
> >> gid_attr = ah_attr->grh.sgid_attr;
> >> - if (is_vlan_dev(gid_attr->ndev))
> >> + if (is_vlan_dev(gid_attr->ndev)) {
> >> vlan_tag = vlan_dev_vlan_id(gid_attr->ndev);
> >> + vlan_en = true;
> >> + }
> >>
> >> if (vlan_tag < 0x1000)
> >> vlan_tag |= (rdma_ah_get_sl(ah_attr) &
> >> @@ -71,6 +74,7 @@ struct ib_ah *hns_roce_create_ah(struct ib_pd *ibpd,
> >> HNS_ROCE_PORT_NUM_SHIFT));
> >> ah->av.gid_index = grh->sgid_index;
> >> ah->av.vlan = cpu_to_le16(vlan_tag);
> >> + ah->av.vlan_en = vlan_en;
> >> dev_dbg(dev, "gid_index = 0x%x,vlan = 0x%x\n", ah->av.gid_index,
> >> ah->av.vlan);
> >>
> >> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
> >> index 6dadb12..2461804 100644
> >> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
> >> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
> >> @@ -458,6 +458,7 @@ struct hns_roce_av {
> >> u8 dgid[HNS_ROCE_GID_SIZE];
> >> u8 mac[6];
> >> __le16 vlan;
> >> + bool vlan_en;
> >> };
> >>
> >> struct hns_roce_ah {
> >> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> >> index 4020584..86f793b 100644
> >> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> >> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> >> @@ -370,6 +370,9 @@ static int hns_roce_v2_post_send(struct ib_qp *ibqp,
> >> V2_UD_SEND_WQE_BYTE_40_PORTN_S,
> >> qp->port);
> >>
> >> + roce_set_bit(ud_sq_wqe->byte_40,
> >> + V2_UD_SEND_WQE_BYTE_40_UD_VLAN_EN_S,
> >
> > According to hns code, all those "bool vlan_en" can be replaced to something like this:
> > roce_set_bit(ud_sq_wqe->byte_40, V2_UD_SEND_WQE_BYTE_40_UD_VLAN_EN_S, (ah->av.vlan_tag != cpu_to_le16(0xFFFF)))
> >
> Hi, Leon
> Firstly, thank your advice. At the begining, I also consider your suggestion because the 0xffff is out of the range of vlan.
> because the hardware will distinguish the device whether the vlan tag is 0xffff. if the vlan tag is 0xffff, the hardware will
> distinguish as non-vlan device. Therefore, the old hardware does have this patch.
> But the new design have changed. In order to extend the range of vlan and make it more flexible,
> the hardware have modified the judgement. The hardware distinguish whether it is vlan device according to the vlan_en field.
> In the future, the range of vlan include 0xffff, the new desgin may be more suitable
> Currently, your advice may be the same with this patch and your advice is more concise. is it ok?
I afraid that you missed " != " part of my suggestion, but it doesn't matter.
Your patch is fine too, go for it.
Thanks
Attachment:
signature.asc
Description: PGP signature
