diff mbox series

[for-next,4/4] RDMA/hns: Add enable judgement for UD vlan

Message ID 1537006137-109531-5-git-send-email-oulijun@huawei.com (mailing list archive)
State Superseded
Headers show
Series Two new features support for hip08 | expand

Commit Message

Lijun Ou Sept. 15, 2018, 10:08 a.m. UTC 
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@huawei.com>
---
 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(-)

Comments

Leon Romanovsky Sept. 15, 2018, 10:27 a.m. UTC | #1 
On Sat, Sep 15, 2018 at 06:08:57PM +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@huawei.com>
> ---
>  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..004b315 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;
>

You are not changing this variable and not using it in any decision making.
This patch most probably doesn't do what you expected from it.

>  	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);
> +		ah->av.vlan_en = vlan_en;
> +	}
>
>  	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,
> +				     ah->av.vlan_en ? 1 : 0);
>  			roce_set_field(ud_sq_wqe->byte_48,
>  				       V2_UD_SEND_WQE_BYTE_48_SGID_INDX_M,
>  				       V2_UD_SEND_WQE_BYTE_48_SGID_INDX_S,
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
> index d04be1c..7ee6ed2 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
> @@ -993,6 +993,8 @@ struct hns_roce_v2_ud_send_wqe {
>  #define	V2_UD_SEND_WQE_BYTE_40_PORTN_S 24
>  #define V2_UD_SEND_WQE_BYTE_40_PORTN_M GENMASK(26, 24)
>
> +#define V2_UD_SEND_WQE_BYTE_40_UD_VLAN_EN_S 30
> +
>  #define	V2_UD_SEND_WQE_BYTE_40_LBI_S 31
>
>  #define	V2_UD_SEND_WQE_DMAC_0_S 0
> --
> 1.9.1
>
Lijun Ou Sept. 17, 2018, 7:02 a.m. UTC | #2 
在 2018/9/15 18:27, Leon Romanovsky 写道:
> On Sat, Sep 15, 2018 at 06:08:57PM +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@huawei.com>
>> ---
>>  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..004b315 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;
>>
> You are not changing this variable and not using it in any decision making.
> This patch most probably doesn't do what you expected from it.
Hi， leon
    The goal that defines av.vlan_en is to configure the vlan_en filed of ud wqe. when judge the device is vlan device. Is ok?

thanks
Lijun Ou
>>  	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);
>> +		ah->av.vlan_en = vlan_en;
>> +	}
>>
>>  	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,
>> +				     ah->av.vlan_en ? 1 : 0);
>>  			roce_set_field(ud_sq_wqe->byte_48,
>>  				       V2_UD_SEND_WQE_BYTE_48_SGID_INDX_M,
>>  				       V2_UD_SEND_WQE_BYTE_48_SGID_INDX_S,
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
>> index d04be1c..7ee6ed2 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
>> @@ -993,6 +993,8 @@ struct hns_roce_v2_ud_send_wqe {
>>  #define	V2_UD_SEND_WQE_BYTE_40_PORTN_S 24
>>  #define V2_UD_SEND_WQE_BYTE_40_PORTN_M GENMASK(26, 24)
>>
>> +#define V2_UD_SEND_WQE_BYTE_40_UD_VLAN_EN_S 30
>> +
>>  #define	V2_UD_SEND_WQE_BYTE_40_LBI_S 31
>>
>>  #define	V2_UD_SEND_WQE_DMAC_0_S 0
>> --
>> 1.9.1
>>
Leon Romanovsky Sept. 17, 2018, 9:55 a.m. UTC | #3 
On Mon, Sep 17, 2018 at 03:02:24PM +0800, oulijun wrote:
> 在 2018/9/15 18:27, Leon Romanovsky 写道:
> > On Sat, Sep 15, 2018 at 06:08:57PM +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@huawei.com>
> >> ---
> >>  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..004b315 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;
> >>
> > You are not changing this variable and not using it in any decision making.
> > This patch most probably doesn't do what you expected from it.
> Hi， leon
>     The goal that defines av.vlan_en is to configure the vlan_en filed of ud wqe. when judge the device is vlan device. Is ok?

No, vlan_en is always false in your patch.

Thanks
Lijun Ou Sept. 17, 2018, 11:06 a.m. UTC | #4 
在 2018/9/17 17:55, Leon Romanovsky 写道:
> On Mon, Sep 17, 2018 at 03:02:24PM +0800, oulijun wrote:
>> 在 2018/9/15 18:27, Leon Romanovsky 写道:
>>> On Sat, Sep 15, 2018 at 06:08:57PM +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@huawei.com>
>>>> ---
>>>>  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..004b315 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;
>>>>
>>> You are not changing this variable and not using it in any decision making.
>>> This patch most probably doesn't do what you expected from it.
>> Hi， leon
>>     The goal that defines av.vlan_en is to configure the vlan_en filed of ud wqe. when judge the device is vlan device. Is ok?
> 
> No, vlan_en is always false in your patch.
> 
> Thanks
> 
Sorry, the code is fixed in error way from my locate place. thanks. I will fixes it in next patch.
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/hns/hns_roce_ah.c b/drivers/infiniband/hw/hns/hns_roce_ah.c
index 0d96c5b..004b315 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);
+		ah->av.vlan_en = vlan_en;
+	}
 
 	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,
+				     ah->av.vlan_en ? 1 : 0);
 			roce_set_field(ud_sq_wqe->byte_48,
 				       V2_UD_SEND_WQE_BYTE_48_SGID_INDX_M,
 				       V2_UD_SEND_WQE_BYTE_48_SGID_INDX_S,
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
index d04be1c..7ee6ed2 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.h
@@ -993,6 +993,8 @@  struct hns_roce_v2_ud_send_wqe {
 #define	V2_UD_SEND_WQE_BYTE_40_PORTN_S 24
 #define V2_UD_SEND_WQE_BYTE_40_PORTN_M GENMASK(26, 24)
 
+#define V2_UD_SEND_WQE_BYTE_40_UD_VLAN_EN_S 30
+
 #define	V2_UD_SEND_WQE_BYTE_40_LBI_S 31
 
 #define	V2_UD_SEND_WQE_DMAC_0_S 0