[PATCH net-next 0/2] net: hns3: updates for -next

[PATCH net-next 0/2] net: hns3: updates for -next

[Huazhong Tan]

This series adds support for pushing link status to VFs for
the HNS3 ethernet driver.

Guangbin Huang (2):
  net: hns3: PF add support for pushing link status to VFs
  net: hns3: VF not request link status when PF support push link status
    feature

 drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h    |  3 ++
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 35 +++++++++++++++++++++-
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h    |  1 +
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c | 12 ++++----
 .../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c  |  8 +++--
 .../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h  |  1 +
 .../ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c   |  6 ++++
 7 files changed, 55 insertions(+), 11 deletions(-)

-- 
2.7.4

[PATCH net-next 1/2] net: hns3: PF add support for pushing link status to VFs

[Huazhong Tan]

From: Guangbin Huang <huangguangbin2@huawei.com>

Previously, VF updates its link status every second by send query command
to PF in periodic service task. If link stats of PF is changed, VF may
need at most one second to update its link status.

To reduce delay of link status between PF and VFs, PF actively push its
link status to VFs when its link status is updated. And to let VF know
PF supports this new feature, the link status changed mailbox command
adds one bit to indicate it.

Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h    |  3 ++
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 35 +++++++++++++++++++++-
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h    |  1 +
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c | 12 ++++----
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h b/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
index 33defa4..a2c17af 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
@@ -172,4 +172,7 @@ struct hclgevf_mbx_arq_ring {
 		(arq.tail = (arq.tail + 1) % HCLGE_MBX_MAX_ARQ_MSG_NUM)
 #define hclge_mbx_head_ptr_move_arq(arq) \
 		(arq.head = (arq.head + 1) % HCLGE_MBX_MAX_ARQ_MSG_NUM)
+
+/* PF immediately push link status to VFs when link status changed */
+#define HCLGE_MBX_PUSH_LINK_STATUS_EN			BIT(0)
 #endif
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 1c17fdc..dc06986 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2880,6 +2880,28 @@ static int hclge_get_mac_phy_link(struct hclge_dev *hdev, int *link_status)
 	return hclge_get_mac_link_status(hdev, link_status);
 }
 
+static void hclge_push_link_status(struct hclge_dev *hdev)
+{
+	struct hclge_vport *vport;
+	int ret;
+	u16 i;
+
+	for (i = 0; i < pci_num_vf(hdev->pdev); i++) {
+		vport = &hdev->vport[i + HCLGE_VF_VPORT_START_NUM];
+
+		if (!test_bit(HCLGE_VPORT_STATE_ALIVE, &vport->state) ||
+		    vport->vf_info.link_state != IFLA_VF_LINK_STATE_AUTO)
+			continue;
+
+		ret = hclge_push_vf_link_status(vport);
+		if (ret) {
+			dev_err(&hdev->pdev->dev,
+				"failed to push link status to vf%u, ret = %d\n",
+				i, ret);
+		}
+	}
+}
+
 static void hclge_update_link_status(struct hclge_dev *hdev)
 {
 	struct hnae3_handle *rhandle = &hdev->vport[0].roce;
@@ -2908,6 +2930,7 @@ static void hclge_update_link_status(struct hclge_dev *hdev)
 			rclient->ops->link_status_change(rhandle, state);
 
 		hdev->hw.mac.link = state;
+		hclge_push_link_status(hdev);
 	}
 
 	clear_bit(HCLGE_STATE_LINK_UPDATING, &hdev->state);
@@ -3246,14 +3269,24 @@ static int hclge_set_vf_link_state(struct hnae3_handle *handle, int vf,
 {
 	struct hclge_vport *vport = hclge_get_vport(handle);
 	struct hclge_dev *hdev = vport->back;
+	int link_state_old;
+	int ret;
 
 	vport = hclge_get_vf_vport(hdev, vf);
 	if (!vport)
 		return -EINVAL;
 
+	link_state_old = vport->vf_info.link_state;
 	vport->vf_info.link_state = link_state;
 
-	return 0;
+	ret = hclge_push_vf_link_status(vport);
+	if (ret) {
+		vport->vf_info.link_state = link_state_old;
+		dev_err(&hdev->pdev->dev,
+			"failed to push vf%d link status, ret = %d\n", vf, ret);
+	}
+
+	return ret;
 }
 
 static u32 hclge_check_event_cause(struct hclge_dev *hdev, u32 *clearval)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index c1aaf7c5..ff1d473 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -1089,4 +1089,5 @@ void hclge_report_hw_error(struct hclge_dev *hdev,
 			   enum hnae3_hw_error_type type);
 void hclge_inform_vf_promisc_info(struct hclge_vport *vport);
 void hclge_dbg_dump_rst_info(struct hclge_dev *hdev);
+int hclge_push_vf_link_status(struct hclge_vport *vport);
 #endif
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
index c88607b..5512ffe 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
@@ -490,16 +490,14 @@ static void hclge_get_vf_media_type(struct hclge_vport *vport,
 	resp_msg->len = HCLGE_VF_MEDIA_TYPE_LENGTH;
 }
 
-static int hclge_get_link_info(struct hclge_vport *vport,
-			       struct hclge_mbx_vf_to_pf_cmd *mbx_req)
+int hclge_push_vf_link_status(struct hclge_vport *vport)
 {
 #define HCLGE_VF_LINK_STATE_UP		1U
 #define HCLGE_VF_LINK_STATE_DOWN	0U
 
 	struct hclge_dev *hdev = vport->back;
 	u16 link_status;
-	u8 msg_data[8];
-	u8 dest_vfid;
+	u8 msg_data[9];
 	u16 duplex;
 
 	/* mac.link can only be 0 or 1 */
@@ -520,11 +518,11 @@ static int hclge_get_link_info(struct hclge_vport *vport,
 	memcpy(&msg_data[0], &link_status, sizeof(u16));
 	memcpy(&msg_data[2], &hdev->hw.mac.speed, sizeof(u32));
 	memcpy(&msg_data[6], &duplex, sizeof(u16));
-	dest_vfid = mbx_req->mbx_src_vfid;
+	msg_data[8] = HCLGE_MBX_PUSH_LINK_STATUS_EN;
 
 	/* send this requested info to VF */
 	return hclge_send_mbx_msg(vport, msg_data, sizeof(msg_data),
-				  HCLGE_MBX_LINK_STAT_CHANGE, dest_vfid);
+				  HCLGE_MBX_LINK_STAT_CHANGE, vport->vport_id);
 }
 
 static void hclge_get_link_mode(struct hclge_vport *vport,
@@ -794,7 +792,7 @@ void hclge_mbx_handler(struct hclge_dev *hdev)
 			hclge_get_vf_tcinfo(vport, &resp_msg);
 			break;
 		case HCLGE_MBX_GET_LINK_STATUS:
-			ret = hclge_get_link_info(vport, req);
+			ret = hclge_push_vf_link_status(vport);
 			if (ret)
 				dev_err(&hdev->pdev->dev,
 					"failed to inform link stat to VF, ret = %d\n",
-- 
2.7.4

[PATCH net-next 2/2] net: hns3: VF not request link status when PF support push link status feature

[Huazhong Tan]

From: Guangbin Huang <huangguangbin2@huawei.com>

To reduce the processing of unnecessary mailbox command when PF supports
actively push its link status to VFs, VFs stop sending request link
status command in periodic service task in this case.

Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 8 +++++---
 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h | 1 +
 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c  | 6 ++++++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 07aa26b..07066c4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -2340,10 +2340,11 @@ static void hclgevf_periodic_service_task(struct hclgevf_dev *hdev)
 	if (!(hdev->serv_processed_cnt % HCLGEVF_STATS_TIMER_INTERVAL))
 		hclgevf_tqps_update_stats(handle);
 
-	/* request the link status from the PF. PF would be able to tell VF
-	 * about such updates in future so we might remove this later
+	/* VF does not need to request link status when this bit is set, because
+	 * PF will push its link status to VFs when link status changed.
 	 */
-	hclgevf_request_link_info(hdev);
+	if (!test_bit(HCLGEVF_STATE_PF_PUSH_LINK_STATUS, &hdev->state))
+		hclgevf_request_link_info(hdev);
 
 	hclgevf_update_link_mode(hdev);
 
@@ -2657,6 +2658,7 @@ static int hclgevf_ae_start(struct hnae3_handle *handle)
 	struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
 
 	clear_bit(HCLGEVF_STATE_DOWN, &hdev->state);
+	clear_bit(HCLGEVF_STATE_PF_PUSH_LINK_STATUS, &hdev->state);
 
 	hclgevf_reset_tqp_stats(handle);
 
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
index ade6e7f..956095b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
@@ -152,6 +152,7 @@ enum hclgevf_states {
 	HCLGEVF_STATE_LINK_UPDATING,
 	HCLGEVF_STATE_PROMISC_CHANGED,
 	HCLGEVF_STATE_RST_FAIL,
+	HCLGEVF_STATE_PF_PUSH_LINK_STATUS,
 };
 
 struct hclgevf_mac {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
index 5b2dcd9..9b17735 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
@@ -276,6 +276,7 @@ void hclgevf_mbx_async_handler(struct hclgevf_dev *hdev)
 	u8 duplex;
 	u32 speed;
 	u32 tail;
+	u8 flag;
 	u8 idx;
 
 	/* we can safely clear it now as we are at start of the async message
@@ -300,11 +301,16 @@ void hclgevf_mbx_async_handler(struct hclgevf_dev *hdev)
 			link_status = msg_q[1];
 			memcpy(&speed, &msg_q[2], sizeof(speed));
 			duplex = (u8)msg_q[4];
+			flag = (u8)msg_q[5];
 
 			/* update upper layer with new link link status */
 			hclgevf_update_link_status(hdev, link_status);
 			hclgevf_update_speed_duplex(hdev, speed, duplex);
 
+			if (flag & HCLGE_MBX_PUSH_LINK_STATUS_EN)
+				set_bit(HCLGEVF_STATE_PF_PUSH_LINK_STATUS,
+					&hdev->state);
+
 			break;
 		case HCLGE_MBX_LINK_STAT_MODE:
 			idx = (u8)msg_q[1];
-- 
2.7.4

Re: [PATCH net-next 1/2] net: hns3: PF add support for pushing link status to VFs

[Jakub Kicinski]

On Tue, 13 Apr 2021 14:17:00 +0800 Huazhong Tan wrote:
> +static void hclge_push_link_status(struct hclge_dev *hdev)
> +{
> +	struct hclge_vport *vport;
> +	int ret;
> +	u16 i;
> +
> +	for (i = 0; i < pci_num_vf(hdev->pdev); i++) {
> +		vport = &hdev->vport[i + HCLGE_VF_VPORT_START_NUM];
> +
> +		if (!test_bit(HCLGE_VPORT_STATE_ALIVE, &vport->state) ||
> +		    vport->vf_info.link_state != IFLA_VF_LINK_STATE_AUTO)
> +			continue;
> +
> +		ret = hclge_push_vf_link_status(vport);
> +		if (ret) {
> +			dev_err(&hdev->pdev->dev,
> +				"failed to push link status to vf%u, ret = %d\n",
> +				i, ret);

Isn't this error printed twice? Here and...

> +}
> +
>  static void hclge_update_link_status(struct hclge_dev *hdev)
>  {
>  	struct hnae3_handle *rhandle = &hdev->vport[0].roce;

> @@ -3246,14 +3269,24 @@ static int hclge_set_vf_link_state(struct hnae3_handle *handle, int vf,
>  {
>  	struct hclge_vport *vport = hclge_get_vport(handle);
>  	struct hclge_dev *hdev = vport->back;
> +	int link_state_old;
> +	int ret;
>  
>  	vport = hclge_get_vf_vport(hdev, vf);
>  	if (!vport)
>  		return -EINVAL;
>  
> +	link_state_old = vport->vf_info.link_state;
>  	vport->vf_info.link_state = link_state;
>  
> -	return 0;
> +	ret = hclge_push_vf_link_status(vport);
> +	if (ret) {
> +		vport->vf_info.link_state = link_state_old;
> +		dev_err(&hdev->pdev->dev,
> +			"failed to push vf%d link status, ret = %d\n", vf, ret);
> +	}

... here?

Otherwise the patches LGTM.

Re: [PATCH net-next 1/2] net: hns3: PF add support for pushing link status to VFs

[Huazhong Tan]

On 2021/4/14 1:18, Jakub Kicinski wrote:
> On Tue, 13 Apr 2021 14:17:00 +0800 Huazhong Tan wrote:
>> +static void hclge_push_link_status(struct hclge_dev *hdev)
>> +{
>> +	struct hclge_vport *vport;
>> +	int ret;
>> +	u16 i;
>> +
>> +	for (i = 0; i < pci_num_vf(hdev->pdev); i++) {
>> +		vport = &hdev->vport[i + HCLGE_VF_VPORT_START_NUM];
>> +
>> +		if (!test_bit(HCLGE_VPORT_STATE_ALIVE, &vport->state) ||
>> +		    vport->vf_info.link_state != IFLA_VF_LINK_STATE_AUTO)
>> +			continue;
>> +
>> +		ret = hclge_push_vf_link_status(vport);
>> +		if (ret) {
>> +			dev_err(&hdev->pdev->dev,
>> +				"failed to push link status to vf%u, ret = %d\n",
>> +				i, ret);
> Isn't this error printed twice? Here and...

They are in different contexts. here will be called to
update the link status of all VFs when the underlying
link status is changed, while the below one is called
when the admin set up the specific VF link status.


Thanks.


>
>> +}
>> +
>>   static void hclge_update_link_status(struct hclge_dev *hdev)
>>   {
>>   	struct hnae3_handle *rhandle = &hdev->vport[0].roce;
>> @@ -3246,14 +3269,24 @@ static int hclge_set_vf_link_state(struct hnae3_handle *handle, int vf,
>>   {
>>   	struct hclge_vport *vport = hclge_get_vport(handle);
>>   	struct hclge_dev *hdev = vport->back;
>> +	int link_state_old;
>> +	int ret;
>>   
>>   	vport = hclge_get_vf_vport(hdev, vf);
>>   	if (!vport)
>>   		return -EINVAL;
>>   
>> +	link_state_old = vport->vf_info.link_state;
>>   	vport->vf_info.link_state = link_state;
>>   
>> -	return 0;
>> +	ret = hclge_push_vf_link_status(vport);
>> +	if (ret) {
>> +		vport->vf_info.link_state = link_state_old;
>> +		dev_err(&hdev->pdev->dev,
>> +			"failed to push vf%d link status, ret = %d\n", vf, ret);
>> +	}
> ... here?
>
> Otherwise the patches LGTM.
>
> .

Re: [PATCH net-next 1/2] net: hns3: PF add support for pushing link status to VFs

[Jakub Kicinski]

On Wed, 14 Apr 2021 09:51:38 +0800 Huazhong Tan wrote:
> On 2021/4/14 1:18, Jakub Kicinski wrote:
> > On Tue, 13 Apr 2021 14:17:00 +0800 Huazhong Tan wrote:  
> >> +static void hclge_push_link_status(struct hclge_dev *hdev)
> >> +{
> >> +	struct hclge_vport *vport;
> >> +	int ret;
> >> +	u16 i;
> >> +
> >> +	for (i = 0; i < pci_num_vf(hdev->pdev); i++) {
> >> +		vport = &hdev->vport[i + HCLGE_VF_VPORT_START_NUM];
> >> +
> >> +		if (!test_bit(HCLGE_VPORT_STATE_ALIVE, &vport->state) ||
> >> +		    vport->vf_info.link_state != IFLA_VF_LINK_STATE_AUTO)
> >> +			continue;
> >> +
> >> +		ret = hclge_push_vf_link_status(vport);
> >> +		if (ret) {
> >> +			dev_err(&hdev->pdev->dev,
> >> +				"failed to push link status to vf%u, ret = %d\n",
> >> +				i, ret);  
> > Isn't this error printed twice? Here and...  
> 
> They are in different contexts. here will be called to
> update the link status of all VFs when the underlying
> link status is changed, while the below one is called
> when the admin set up the specific VF link status.

I see.

Re: [PATCH net-next 1/2] net: hns3: PF add support for pushing link status to VFs

[Huazhong Tan]

On 2021/4/15 0:42, Jakub Kicinski wrote:
> On Wed, 14 Apr 2021 09:51:38 +0800 Huazhong Tan wrote:
>> On 2021/4/14 1:18, Jakub Kicinski wrote:
>>> On Tue, 13 Apr 2021 14:17:00 +0800 Huazhong Tan wrote:
>>>> +static void hclge_push_link_status(struct hclge_dev *hdev)
>>>> +{
>>>> +	struct hclge_vport *vport;
>>>> +	int ret;
>>>> +	u16 i;
>>>> +
>>>> +	for (i = 0; i < pci_num_vf(hdev->pdev); i++) {
>>>> +		vport = &hdev->vport[i + HCLGE_VF_VPORT_START_NUM];
>>>> +
>>>> +		if (!test_bit(HCLGE_VPORT_STATE_ALIVE, &vport->state) ||
>>>> +		    vport->vf_info.link_state != IFLA_VF_LINK_STATE_AUTO)
>>>> +			continue;
>>>> +
>>>> +		ret = hclge_push_vf_link_status(vport);
>>>> +		if (ret) {
>>>> +			dev_err(&hdev->pdev->dev,
>>>> +				"failed to push link status to vf%u, ret = %d\n",
>>>> +				i, ret);
>>> Isn't this error printed twice? Here and...
>> They are in different contexts. here will be called to
>> update the link status of all VFs when the underlying
>> link status is changed, while the below one is called
>> when the admin set up the specific VF link status.
> I see.


So this error will be printed twice only if these two cases
happen at the same, do you mean to add some keyword to
distinguish them?


> .

Re: [PATCH net-next 1/2] net: hns3: PF add support for pushing link status to VFs

[Jakub Kicinski]

On Thu, 15 Apr 2021 09:11:03 +0800 Huazhong Tan wrote:
> >> They are in different contexts. here will be called to
> >> update the link status of all VFs when the underlying
> >> link status is changed, while the below one is called
> >> when the admin set up the specific VF link status.  
> > I see.  
> 
> So this error will be printed twice only if these two cases
> happen at the same, do you mean to add some keyword to
> distinguish them?

No, it's fine but please repost - looks like the patches were 
removed from patchwork already.

Re: [PATCH net-next 1/2] net: hns3: PF add support for pushing link status to VFs

[Huazhong Tan]

On 2021/4/15 9:53, Jakub Kicinski wrote:
> On Thu, 15 Apr 2021 09:11:03 +0800 Huazhong Tan wrote:
>>>> They are in different contexts. here will be called to
>>>> update the link status of all VFs when the underlying
>>>> link status is changed, while the below one is called
>>>> when the admin set up the specific VF link status.
>>> I see.
>> So this error will be printed twice only if these two cases
>> happen at the same, do you mean to add some keyword to
>> distinguish them?
> No, it's fine but please repost - looks like the patches were
> removed from patchwork already.


Will resend it, thanks.


> .