2018-05-15 18:21:41

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 00/10] Misc. Bug Fixes and clean-ups for HNS3 Driver

This patch-set mainly introduces various bug fixes, cleanups and one
very small enhancement to existing HN3 driver code.

Fuyun Liang (7):
net: hns3: Fix for deadlock problem occurring when unregistering
ae_algo
net: hns3: Fix for the null pointer problem occurring when
initializing ae_dev failed
net: hns3: Add a check for client instance init state
net: hns3: Change return type of hnae3_register_ae_dev
net: hns3: Change return type of hnae3_register_ae_algo
net: hns3: Change return value in hnae3_register_client
net: hns3: Fixes the missing PCI iounmap for various legs

Peng Li (1):
net: hns3: Add support of .sriov_configure in HNS3 driver

Yunsheng Lin (2):
net: hns3: Fixes the back pressure setting When sriov is enabled
net: hns3: Fix for fiber link up problem

drivers/net/ethernet/hisilicon/hns3/hnae3.c | 44 ++++++------
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 5 +-
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 78 +++++++++++++++++++++-
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 55 +++------------
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c | 45 +++++++++++--
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h | 5 ++
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 10 +--
7 files changed, 164 insertions(+), 78 deletions(-)

--
2.7.4




2018-05-15 18:22:07

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 06/10] net: hns3: Change return value in hnae3_register_client

From: Fuyun Liang <[email protected]>

A client includes many client instance. Just like ae_algo, Initializing
client instance failed does not represent registering client failed.
The action of registering client just is adding client to the client
list and the result always is true. This patch changes the return
value of hnae3_register_client form a variable value to a fixed value,
makes the function always return ok.

Signed-off-by: Fuyun Liang <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
index 3b1c396..bd3c232 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
@@ -98,7 +98,7 @@ int hnae3_register_client(struct hnae3_client *client)
exit:
mutex_unlock(&hnae3_common_lock);

- return ret;
+ return 0;
}
EXPORT_SYMBOL(hnae3_register_client);

--
2.7.4



2018-05-15 18:22:25

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 10/10] net: hns3: Fixes the missing PCI iounmap for various legs

From: Fuyun Liang <[email protected]>

We call pcim_iomap in hclge_pci_init, pcim_iounmap should be called
in error handle of hclge_init_ae_dev.

We call pcim_iomap in hclge_pci_init, but do not call pcim_iounmap in
hclge_pci_uninit. When we remove the hclge.ko and insert it again, a
problem that pci can not map will happen. pcim_iounmap need to be called
in hclge_pci_uninit.

Fixes: 46a3df9f9718 ("net: hns3: Add HNS3 Acceleration Engine & Compatibility Layer Support")
Signed-off-by: Fuyun Liang <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 75d9b8c..46435c8 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5425,6 +5425,7 @@ static void hclge_pci_uninit(struct hclge_dev *hdev)
{
struct pci_dev *pdev = hdev->pdev;

+ pcim_iounmap(pdev, hdev->hw.io_base);
pci_free_irq_vectors(pdev);
pci_clear_master(pdev);
pci_release_mem_regions(pdev);
@@ -5589,6 +5590,7 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
err_cmd_uninit:
hclge_destroy_cmd_queue(&hdev->hw);
err_pci_uninit:
+ pcim_iounmap(pdev, hdev->hw.io_base);
pci_clear_master(pdev);
pci_release_regions(pdev);
pci_disable_device(pdev);
--
2.7.4



2018-05-15 18:23:32

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 07/10] net: hns3: Fixes the back pressure setting When sriov is enabled

From: Yunsheng Lin <[email protected]>

When sriov is enabled, the Qset and tc mapping is not longer one
to one relation.

This patch fixes it by mapping all pf and vf's Qset to tc.

Fixes: 848440544b41 ("net: hns3: Add support of TX Scheduler & Shaper to HNS3 driver")
Signed-off-by: Yunsheng Lin <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c | 45 +++++++++++++++++++---
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h | 5 +++
2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
index c69ecab..262c125 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
@@ -500,7 +500,8 @@ static int hclge_tm_qs_schd_mode_cfg(struct hclge_dev *hdev, u16 qs_id, u8 mode)
return hclge_cmd_send(&hdev->hw, &desc, 1);
}

-static int hclge_tm_qs_bp_cfg(struct hclge_dev *hdev, u8 tc)
+static int hclge_tm_qs_bp_cfg(struct hclge_dev *hdev, u8 tc, u8 grp_id,
+ u32 bit_map)
{
struct hclge_bp_to_qs_map_cmd *bp_to_qs_map_cmd;
struct hclge_desc desc;
@@ -511,9 +512,8 @@ static int hclge_tm_qs_bp_cfg(struct hclge_dev *hdev, u8 tc)
bp_to_qs_map_cmd = (struct hclge_bp_to_qs_map_cmd *)desc.data;

bp_to_qs_map_cmd->tc_id = tc;
-
- /* Qset and tc is one by one mapping */
- bp_to_qs_map_cmd->qs_bit_map = cpu_to_le32(1 << tc);
+ bp_to_qs_map_cmd->qs_group_id = grp_id;
+ bp_to_qs_map_cmd->qs_bit_map = cpu_to_le32(bit_map);

return hclge_cmd_send(&hdev->hw, &desc, 1);
}
@@ -1167,6 +1167,41 @@ static int hclge_pfc_setup_hw(struct hclge_dev *hdev)
hdev->tm_info.hw_pfc_map);
}

+/* Each Tc has a 1024 queue sets to backpress, it divides to
+ * 32 group, each group contains 32 queue sets, which can be
+ * represented by u32 bitmap.
+ */
+static int hclge_bp_setup_hw(struct hclge_dev *hdev, u8 tc)
+{
+ struct hclge_vport *vport = hdev->vport;
+ u32 i, k, qs_bitmap;
+ int ret;
+
+ for (i = 0; i < HCLGE_BP_GRP_NUM; i++) {
+ qs_bitmap = 0;
+
+ for (k = 0; k < hdev->num_alloc_vport; k++) {
+ u16 qs_id = vport->qs_offset + tc;
+ u8 grp, sub_grp;
+
+ grp = hnae_get_field(qs_id, HCLGE_BP_GRP_ID_M,
+ HCLGE_BP_GRP_ID_S);
+ sub_grp = hnae_get_field(qs_id, HCLGE_BP_SUB_GRP_ID_M,
+ HCLGE_BP_SUB_GRP_ID_S);
+ if (i == grp)
+ qs_bitmap |= (1 << sub_grp);
+
+ vport++;
+ }
+
+ ret = hclge_tm_qs_bp_cfg(hdev, tc, i, qs_bitmap);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int hclge_mac_pause_setup_hw(struct hclge_dev *hdev)
{
bool tx_en, rx_en;
@@ -1218,7 +1253,7 @@ int hclge_pause_setup_hw(struct hclge_dev *hdev)
dev_warn(&hdev->pdev->dev, "set pfc pause failed:%d\n", ret);

for (i = 0; i < hdev->tm_info.num_tc; i++) {
- ret = hclge_tm_qs_bp_cfg(hdev, i);
+ ret = hclge_bp_setup_hw(hdev, i);
if (ret)
return ret;
}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
index 2dbe177..c2b6e8a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
@@ -89,6 +89,11 @@ struct hclge_pg_shapping_cmd {
__le32 pg_shapping_para;
};

+#define HCLGE_BP_GRP_NUM 32
+#define HCLGE_BP_SUB_GRP_ID_S 0
+#define HCLGE_BP_SUB_GRP_ID_M GENMASK(4, 0)
+#define HCLGE_BP_GRP_ID_S 5
+#define HCLGE_BP_GRP_ID_M GENMASK(9, 5)
struct hclge_bp_to_qs_map_cmd {
u8 tc_id;
u8 rsvd[2];
--
2.7.4



2018-05-15 18:24:30

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 08/10] net: hns3: Fix for fiber link up problem

From: Yunsheng Lin <[email protected]>

When hclge_ae_start is called, hdev->hw.mac.link may be set
to one after up/down multi-times, which does not correspond to
the link state of netdev when the netdev is up.

This fixes it by setting hdev->hw.mac.link to zero when
hclge_ae_start is called.

Fixes: 46a3df9f9718 ("net: hns3: Add HNS3 Acceleration Engine & Compatibility Layer Support")
Signed-off-by: Yunsheng Lin <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index d060903..75d9b8c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -3756,6 +3756,7 @@ static int hclge_ae_start(struct hnae3_handle *handle)
hclge_cfg_mac_mode(hdev, true);
clear_bit(HCLGE_STATE_DOWN, &hdev->state);
mod_timer(&hdev->service_timer, jiffies + HZ);
+ hdev->hw.mac.link = 0;

/* reset tqp stats */
hclge_reset_tqp_stats(handle);
@@ -3792,7 +3793,6 @@ static void hclge_ae_stop(struct hnae3_handle *handle)

/* reset tqp stats */
hclge_reset_tqp_stats(handle);
- hclge_update_link_status(hdev);
}

static int hclge_get_mac_vlan_cmd_status(struct hclge_vport *vport,
--
2.7.4



2018-05-15 18:24:37

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 05/10] net: hns3: Change return type of hnae3_register_ae_algo

From: Fuyun Liang <[email protected]>

The ae_algo is used by many ae_devs. It is not only belong to just a
ae_dev. Initializing ae_dev failed does not represent registering ae_algo
failed. Because the action of registering ae_algo just is adding ae_algo
to the ae_algo list and it is always is true, it make no sense to define
return type as int.

This patch changes the return type of hnae3_register_ae_algo from int to
void.

Signed-off-by: Fuyun Liang <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.c | 4 +---
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 2 +-
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 4 +++-
drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 4 +++-
4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
index cb93295..3b1c396 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
@@ -121,7 +121,7 @@ EXPORT_SYMBOL(hnae3_unregister_client);
* @ae_algo: AE algorithm
* NOTE: the duplicated name will not be checked
*/
-int hnae3_register_ae_algo(struct hnae3_ae_algo *ae_algo)
+void hnae3_register_ae_algo(struct hnae3_ae_algo *ae_algo)
{
const struct pci_device_id *id;
struct hnae3_ae_dev *ae_dev;
@@ -160,8 +160,6 @@ int hnae3_register_ae_algo(struct hnae3_ae_algo *ae_algo)
}

mutex_unlock(&hnae3_common_lock);
-
- return ret;
}
EXPORT_SYMBOL(hnae3_register_ae_algo);

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index ea6e6ea..2f266ef 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -519,7 +519,7 @@ void hnae3_register_ae_dev(struct hnae3_ae_dev *ae_dev);
void hnae3_unregister_ae_dev(struct hnae3_ae_dev *ae_dev);

void hnae3_unregister_ae_algo(struct hnae3_ae_algo *ae_algo);
-int hnae3_register_ae_algo(struct hnae3_ae_algo *ae_algo);
+void hnae3_register_ae_algo(struct hnae3_ae_algo *ae_algo);

void hnae3_unregister_client(struct hnae3_client *client);
int hnae3_register_client(struct hnae3_client *client);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index c2501b1..d060903 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -6250,7 +6250,9 @@ static int hclge_init(void)
{
pr_info("%s is initializing\n", HCLGE_NAME);

- return hnae3_register_ae_algo(&ae_algo);
+ hnae3_register_ae_algo(&ae_algo);
+
+ return 0;
}

static void hclge_exit(void)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index b7578c6..f1f4a17 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1852,7 +1852,9 @@ static int hclgevf_init(void)
{
pr_info("%s is initializing\n", HCLGEVF_NAME);

- return hnae3_register_ae_algo(&ae_algovf);
+ hnae3_register_ae_algo(&ae_algovf);
+
+ return 0;
}

static void hclgevf_exit(void)
--
2.7.4



2018-05-15 18:24:53

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 03/10] net: hns3: Add a check for client instance init state

From: Fuyun Liang <[email protected]>

If the client instance is initializd failed, we do not need to uninit it.
This patch adds a state check to check init state of client instance.

Fixes: 38caee9d3ee8 ("net: hns3: Add support of the HNAE3 framework")
Signed-off-by: Fuyun Liang <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.c | 15 ++++++++++++---
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 1 +
2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
index ab2e72c..21cb0c5 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
@@ -50,13 +50,22 @@ static int hnae3_match_n_instantiate(struct hnae3_client *client,
/* now, (un-)instantiate client by calling lower layer */
if (is_reg) {
ret = ae_dev->ops->init_client_instance(client, ae_dev);
- if (ret)
+ if (ret) {
dev_err(&ae_dev->pdev->dev,
"fail to instantiate client\n");
- return ret;
+ return ret;
+ }
+
+ hnae_set_bit(ae_dev->flag, HNAE3_CLIENT_INITED_B, 1);
+ return 0;
+ }
+
+ if (hnae_get_bit(ae_dev->flag, HNAE3_CLIENT_INITED_B)) {
+ ae_dev->ops->uninit_client_instance(client, ae_dev);
+
+ hnae_set_bit(ae_dev->flag, HNAE3_CLIENT_INITED_B, 0);
}

- ae_dev->ops->uninit_client_instance(client, ae_dev);
return 0;
}

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 804ea83..ffec231 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -52,6 +52,7 @@
#define HNAE3_DEV_INITED_B 0x0
#define HNAE3_DEV_SUPPORT_ROCE_B 0x1
#define HNAE3_DEV_SUPPORT_DCB_B 0x2
+#define HNAE3_CLIENT_INITED_B 0x3

#define HNAE3_DEV_SUPPORT_ROCE_DCB_BITS (BIT(HNAE3_DEV_SUPPORT_DCB_B) |\
BIT(HNAE3_DEV_SUPPORT_ROCE_B))
--
2.7.4



2018-05-15 18:24:59

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 09/10] net: hns3: Add support of .sriov_configure in HNS3 driver

From: Peng Li <[email protected]>

As HNS3 driver will enable SRIOV default and enable all VFs the
HW support, if PF and VF driver compiled to kernel, VF driver
will work on host default, it is not right.

This patch adds support for hns3_driver.sriov_configure to support
user configs the VF_num, and do not enable sriov default.

Signed-off-by: Peng Li <[email protected]>
Suggested-by: Zhou Wang <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 58 ++++++++++++++-----------
1 file changed, 32 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index ac75b5d..e85ff38 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1511,29 +1511,6 @@ static bool hns3_is_phys_func(struct pci_dev *pdev)
return false;
}

-static int get_num_req_vfs(struct pci_dev *pdev)
-{
- /* a variable vf num will be supported later */
- return pci_sriov_get_totalvfs(pdev);
-}
-
-static void hns3_enable_sriov(struct pci_dev *pdev)
-{
- int num_req_vfs = get_num_req_vfs(pdev);
- int ret;
-
- /* Enable SRIOV */
- if (!num_req_vfs)
- return;
-
- dev_info(&pdev->dev, "active VFs(%d) found, enabling SRIOV\n",
- num_req_vfs);
-
- ret = pci_enable_sriov(pdev, num_req_vfs);
- if (ret)
- dev_err(&pdev->dev, "SRIOV enable failed %d\n", ret);
-}
-
static void hns3_disable_sriov(struct pci_dev *pdev)
{
/* If our VFs are assigned we cannot shut down SR-IOV
@@ -1578,9 +1555,6 @@ static int hns3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)

hnae3_register_ae_dev(ae_dev);

- if (hns3_is_phys_func(pdev) && IS_ENABLED(CONFIG_PCI_IOV))
- hns3_enable_sriov(pdev);
-
return 0;
}

@@ -1597,11 +1571,43 @@ static void hns3_remove(struct pci_dev *pdev)
hnae3_unregister_ae_dev(ae_dev);
}

+/**
+ * hns3_pci_sriov_configure
+ * @pdev: pointer to a pci_dev structure
+ * @num_vfs: number of VFs to allocate
+ *
+ * Enable or change the number of VFs. Called when the user updates the number
+ * of VFs in sysfs.
+ **/
+int hns3_pci_sriov_configure(struct pci_dev *pdev, int num_vfs)
+{
+ int ret;
+
+ if (!(hns3_is_phys_func(pdev) && IS_ENABLED(CONFIG_PCI_IOV))) {
+ dev_warn(&pdev->dev, "Can not config SRIOV\n");
+ return -EINVAL;
+ }
+
+ if (num_vfs) {
+ ret = pci_enable_sriov(pdev, num_vfs);
+ if (ret)
+ dev_err(&pdev->dev, "SRIOV enable failed %d\n", ret);
+ } else if (!pci_vfs_assigned(pdev)) {
+ pci_disable_sriov(pdev);
+ } else {
+ dev_warn(&pdev->dev,
+ "Unable to free VFs because some are assigned to VMs.\n");
+ }
+
+ return 0;
+}
+
static struct pci_driver hns3_driver = {
.name = hns3_driver_name,
.id_table = hns3_pci_tbl,
.probe = hns3_probe,
.remove = hns3_remove,
+ .sriov_configure = hns3_pci_sriov_configure,
};

/* set default feature to hns3 */
--
2.7.4



2018-05-15 18:25:10

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 02/10] net: hns3: Fix for the null pointer problem occurring when initializing ae_dev failed

From: Fuyun Liang <[email protected]>

When initializing ae_dev failed during loading hclge.ko, the drvdata will
be set to null. When removing hns3.ko, we get a null ae_dev. It causes the
null pointer problem.

This patch removes pci_set_drvdata from error handle of hclge_init_ae_dev
to fix the bug, since pci_set_drvdata has been called in hns3_remove.
Also, we do not need to uninit the ae_dev which is not initialized. And
it may be the one which is initialized failed.

Fixes: 46a3df9f9718 ("net: hns3: Add HNS3 Acceleration Engine & Compatibility Layer Support")
Signed-off-by: Fuyun Liang <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.c | 6 ++++++
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 5 +----
drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 6 ++----
3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
index 1686ceb..ab2e72c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
@@ -168,6 +168,9 @@ void hnae3_unregister_ae_algo(struct hnae3_ae_algo *ae_algo)
mutex_lock(&hnae3_common_lock);
/* Check if there are matched ae_dev */
list_for_each_entry(ae_dev, &hnae3_ae_dev_list, node) {
+ if (!hnae_get_bit(ae_dev->flag, HNAE3_DEV_INITED_B))
+ continue;
+
id = pci_match_id(ae_algo->pdev_id_table, ae_dev->pdev);
if (!id)
continue;
@@ -256,6 +259,9 @@ void hnae3_unregister_ae_dev(struct hnae3_ae_dev *ae_dev)
mutex_lock(&hnae3_common_lock);
/* Check if there are matched ae_algo */
list_for_each_entry(ae_algo, &hnae3_ae_algo_list, node) {
+ if (!hnae_get_bit(ae_dev->flag, HNAE3_DEV_INITED_B))
+ continue;
+
id = pci_match_id(ae_algo->pdev_id_table, ae_dev->pdev);
if (!id)
continue;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 343197a..c2501b1 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5379,7 +5379,7 @@ static int hclge_pci_init(struct hclge_dev *hdev)
ret = pci_enable_device(pdev);
if (ret) {
dev_err(&pdev->dev, "failed to enable PCI device\n");
- goto err_no_drvdata;
+ return ret;
}

ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
@@ -5417,8 +5417,6 @@ static int hclge_pci_init(struct hclge_dev *hdev)
pci_release_regions(pdev);
err_disable_device:
pci_disable_device(pdev);
-err_no_drvdata:
- pci_set_drvdata(pdev, NULL);

return ret;
}
@@ -5594,7 +5592,6 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
pci_clear_master(pdev);
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
out:
return ret;
}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 2dbffce..b7578c6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1563,7 +1563,7 @@ static int hclgevf_pci_init(struct hclgevf_dev *hdev)
ret = pci_enable_device(pdev);
if (ret) {
dev_err(&pdev->dev, "failed to enable PCI device\n");
- goto err_no_drvdata;
+ return ret;
}

ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
@@ -1595,8 +1595,7 @@ static int hclgevf_pci_init(struct hclgevf_dev *hdev)
pci_release_regions(pdev);
err_disable_device:
pci_disable_device(pdev);
-err_no_drvdata:
- pci_set_drvdata(pdev, NULL);
+
return ret;
}

@@ -1608,7 +1607,6 @@ static void hclgevf_pci_uninit(struct hclgevf_dev *hdev)
pci_clear_master(pdev);
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}

static int hclgevf_init_hdev(struct hclgevf_dev *hdev)
--
2.7.4



2018-05-15 18:25:11

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 04/10] net: hns3: Change return type of hnae3_register_ae_dev

From: Fuyun Liang <[email protected]>

If hclge.ko has not been inserted, the value of ret always is zero
in hnae3_register_ae_dev. If hclge.ko has been inserted, the value
of ret is zero or non zero. Different execution ways have different
results. It is confusing.

The ae_dev which is initialized failed can be reinitialized when we
remove hclge.ko and insert it again. For the case initializing client
instance, it is just like the case initializing ae_dev. The main function
of hnae3_register_ae_dev is adding the ae_dev to ad_dev list. Because
adding ae_dev is always ok, we does not need to return any in this
function.

This patch changes the return type of hnae3_register_ae_dev from int
to void.

Signed-off-by: Fuyun Liang <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.c | 5 +----
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 2 +-
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 4 +---
3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
index 21cb0c5..cb93295 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
@@ -203,7 +203,7 @@ EXPORT_SYMBOL(hnae3_unregister_ae_algo);
* @ae_dev: the AE device
* NOTE: the duplicated name will not be checked
*/
-int hnae3_register_ae_dev(struct hnae3_ae_dev *ae_dev)
+void hnae3_register_ae_dev(struct hnae3_ae_dev *ae_dev)
{
const struct pci_device_id *id;
struct hnae3_ae_algo *ae_algo;
@@ -224,7 +224,6 @@ int hnae3_register_ae_dev(struct hnae3_ae_dev *ae_dev)

if (!ae_dev->ops) {
dev_err(&ae_dev->pdev->dev, "ae_dev ops are null\n");
- ret = -EOPNOTSUPP;
goto out_err;
}

@@ -251,8 +250,6 @@ int hnae3_register_ae_dev(struct hnae3_ae_dev *ae_dev)

out_err:
mutex_unlock(&hnae3_common_lock);
-
- return ret;
}
EXPORT_SYMBOL(hnae3_register_ae_dev);

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index ffec231..ea6e6ea 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -515,7 +515,7 @@ struct hnae3_handle {
#define hnae_get_bit(origin, shift) \
hnae_get_field((origin), (0x1 << (shift)), (shift))

-int hnae3_register_ae_dev(struct hnae3_ae_dev *ae_dev);
+void hnae3_register_ae_dev(struct hnae3_ae_dev *ae_dev);
void hnae3_unregister_ae_dev(struct hnae3_ae_dev *ae_dev);

void hnae3_unregister_ae_algo(struct hnae3_ae_algo *ae_algo);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index af9e90f..ac75b5d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1576,9 +1576,7 @@ static int hns3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
ae_dev->dev_type = HNAE3_DEV_KNIC;
pci_set_drvdata(pdev, ae_dev);

- ret = hnae3_register_ae_dev(ae_dev);
- if (ret)
- return ret;
+ hnae3_register_ae_dev(ae_dev);

if (hns3_is_phys_func(pdev) && IS_ENABLED(CONFIG_PCI_IOV))
hns3_enable_sriov(pdev);
--
2.7.4



2018-05-15 18:25:49

by Salil Mehta

[permalink] [raw]
Subject: [PATCH net-next 01/10] net: hns3: Fix for deadlock problem occurring when unregistering ae_algo

From: Fuyun Liang <[email protected]>

When hnae3_unregister_ae_algo is called by PF, pci_disable_sriov is
called. And then, hns3_remove is called by VF. We get deadlocked in
this case.

Since VF pci device is dependent on PF pci device, When PF pci device
is removed, VF pci device must be removed. Also, To solve the deadlock
problem, VF pci device should be removed before PF pci device is removed.

This patch moves pci_enable/disable_sriov from hclge to hns3 to solve
the deadlock problem.

Also, we do not need to return EPROBE_DEFER in hnae3_register_ae_dev,
because SRIOV is no longer enabled in the context calling
hnae3_register_ae_dev. Mutex_trylock can be replaced with mutex_lock.

Fixes: 424eb834a9be ("net: hns3: Unified HNS3 {VF|PF} Ethernet Driver for hip08 SoC")
Signed-off-by: Fuyun Liang <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.c | 12 +---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 74 +++++++++++++++++++++-
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 42 ++----------
3 files changed, 79 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
index 02145f2..1686ceb 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
@@ -196,17 +196,9 @@ int hnae3_register_ae_dev(struct hnae3_ae_dev *ae_dev)
const struct pci_device_id *id;
struct hnae3_ae_algo *ae_algo;
struct hnae3_client *client;
- int ret = 0, lock_acquired;
+ int ret = 0;

- /* we can get deadlocked if SRIOV is being enabled in context to probe
- * and probe gets called again in same context. This can happen when
- * pci_enable_sriov() is called to create VFs from PF probes context.
- * Therefore, for simplicity uniformly defering further probing in all
- * cases where we detect contention.
- */
- lock_acquired = mutex_trylock(&hnae3_common_lock);
- if (!lock_acquired)
- return -EPROBE_DEFER;
+ mutex_lock(&hnae3_common_lock);

list_add_tail(&ae_dev->node, &hnae3_ae_dev_list);

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 4031174..af9e90f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1487,6 +1487,68 @@ static const struct net_device_ops hns3_nic_netdev_ops = {
.ndo_set_vf_vlan = hns3_ndo_set_vf_vlan,
};

+static bool hns3_is_phys_func(struct pci_dev *pdev)
+{
+ u32 dev_id = pdev->device;
+
+ switch (dev_id) {
+ case HNAE3_DEV_ID_GE:
+ case HNAE3_DEV_ID_25GE:
+ case HNAE3_DEV_ID_25GE_RDMA:
+ case HNAE3_DEV_ID_25GE_RDMA_MACSEC:
+ case HNAE3_DEV_ID_50GE_RDMA:
+ case HNAE3_DEV_ID_50GE_RDMA_MACSEC:
+ case HNAE3_DEV_ID_100G_RDMA_MACSEC:
+ return true;
+ case HNAE3_DEV_ID_100G_VF:
+ case HNAE3_DEV_ID_100G_RDMA_DCB_PFC_VF:
+ return false;
+ default:
+ dev_warn(&pdev->dev, "un-recognized pci device-id %d",
+ dev_id);
+ }
+
+ return false;
+}
+
+static int get_num_req_vfs(struct pci_dev *pdev)
+{
+ /* a variable vf num will be supported later */
+ return pci_sriov_get_totalvfs(pdev);
+}
+
+static void hns3_enable_sriov(struct pci_dev *pdev)
+{
+ int num_req_vfs = get_num_req_vfs(pdev);
+ int ret;
+
+ /* Enable SRIOV */
+ if (!num_req_vfs)
+ return;
+
+ dev_info(&pdev->dev, "active VFs(%d) found, enabling SRIOV\n",
+ num_req_vfs);
+
+ ret = pci_enable_sriov(pdev, num_req_vfs);
+ if (ret)
+ dev_err(&pdev->dev, "SRIOV enable failed %d\n", ret);
+}
+
+static void hns3_disable_sriov(struct pci_dev *pdev)
+{
+ /* If our VFs are assigned we cannot shut down SR-IOV
+ * without causing issues, so just leave the hardware
+ * available but disabled
+ */
+ if (pci_vfs_assigned(pdev)) {
+ dev_warn(&pdev->dev,
+ "disabling driver while VFs are assigned\n");
+ return;
+ }
+
+ pci_disable_sriov(pdev);
+}
+
/* hns3_probe - Device initialization routine
* @pdev: PCI device information struct
* @ent: entry in hns3_pci_tbl
@@ -1514,7 +1576,14 @@ static int hns3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
ae_dev->dev_type = HNAE3_DEV_KNIC;
pci_set_drvdata(pdev, ae_dev);

- return hnae3_register_ae_dev(ae_dev);
+ ret = hnae3_register_ae_dev(ae_dev);
+ if (ret)
+ return ret;
+
+ if (hns3_is_phys_func(pdev) && IS_ENABLED(CONFIG_PCI_IOV))
+ hns3_enable_sriov(pdev);
+
+ return 0;
}

/* hns3_remove - Device removal routine
@@ -1524,6 +1593,9 @@ static void hns3_remove(struct pci_dev *pdev)
{
struct hnae3_ae_dev *ae_dev = pci_get_drvdata(pdev);

+ if (hns3_is_phys_func(pdev) && IS_ENABLED(CONFIG_PCI_IOV))
+ hns3_disable_sriov(pdev);
+
hnae3_unregister_ae_dev(ae_dev);
}

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 316ec842..343197a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -1473,21 +1473,8 @@ static int hclge_alloc_vport(struct hclge_dev *hdev)
hdev->vport = vport;
hdev->num_alloc_vport = num_vport;

-#ifdef CONFIG_PCI_IOV
- /* Enable SRIOV */
- if (hdev->num_req_vfs) {
- dev_info(&pdev->dev, "active VFs(%d) found, enabling SRIOV\n",
- hdev->num_req_vfs);
- ret = pci_enable_sriov(hdev->pdev, hdev->num_req_vfs);
- if (ret) {
- hdev->num_alloc_vfs = 0;
- dev_err(&pdev->dev, "SRIOV enable failed %d\n",
- ret);
- return ret;
- }
- }
- hdev->num_alloc_vfs = hdev->num_req_vfs;
-#endif
+ if (IS_ENABLED(CONFIG_PCI_IOV))
+ hdev->num_alloc_vfs = hdev->num_req_vfs;

for (i = 0; i < num_vport; i++) {
vport->back = hdev;
@@ -2946,21 +2933,6 @@ static void hclge_service_task(struct work_struct *work)
hclge_service_complete(hdev);
}

-static void hclge_disable_sriov(struct hclge_dev *hdev)
-{
- /* If our VFs are assigned we cannot shut down SR-IOV
- * without causing issues, so just leave the hardware
- * available but disabled
- */
- if (pci_vfs_assigned(hdev->pdev)) {
- dev_warn(&hdev->pdev->dev,
- "disabling driver while VFs are assigned\n");
- return;
- }
-
- pci_disable_sriov(hdev->pdev);
-}
-
struct hclge_vport *hclge_get_vport(struct hnae3_handle *handle)
{
/* VF handle has no client */
@@ -5540,7 +5512,7 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
ret = hclge_map_tqp(hdev);
if (ret) {
dev_err(&pdev->dev, "Map tqp error, ret = %d.\n", ret);
- goto err_sriov_disable;
+ goto err_msi_irq_uninit;
}

if (hdev->hw.mac.media_type == HNAE3_MEDIA_TYPE_COPPER) {
@@ -5548,7 +5520,7 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
if (ret) {
dev_err(&hdev->pdev->dev,
"mdio config fail ret=%d\n", ret);
- goto err_sriov_disable;
+ goto err_msi_irq_uninit;
}
}

@@ -5612,9 +5584,6 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
err_mdiobus_unreg:
if (hdev->hw.mac.phydev)
mdiobus_unregister(hdev->hw.mac.mdio_bus);
-err_sriov_disable:
- if (IS_ENABLED(CONFIG_PCI_IOV))
- hclge_disable_sriov(hdev);
err_msi_irq_uninit:
hclge_misc_irq_uninit(hdev);
err_msi_uninit:
@@ -5717,9 +5686,6 @@ static void hclge_uninit_ae_dev(struct hnae3_ae_dev *ae_dev)

set_bit(HCLGE_STATE_DOWN, &hdev->state);

- if (IS_ENABLED(CONFIG_PCI_IOV))
- hclge_disable_sriov(hdev);
-
if (hdev->service_timer.function)
del_timer_sync(&hdev->service_timer);
if (hdev->service_task.func)
--
2.7.4



2018-05-16 15:33:58

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next 00/10] Misc. Bug Fixes and clean-ups for HNS3 Driver

From: Salil Mehta <[email protected]>
Date: Tue, 15 May 2018 19:20:04 +0100

> This patch-set mainly introduces various bug fixes, cleanups and one
> very small enhancement to existing HN3 driver code.

Series applied, thank you.

2018-05-16 19:49:42

by kernel test robot

[permalink] [raw]
Subject: [RFC PATCH] net: hns3: hns3_pci_sriov_configure() can be static


Fixes: fdb793670a00 ("net: hns3: Add support of .sriov_configure in HNS3 driver")
Signed-off-by: Fengguang Wu <[email protected]>
---
hns3_enet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index e85ff38..3617b9d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1579,7 +1579,7 @@ static void hns3_remove(struct pci_dev *pdev)
* Enable or change the number of VFs. Called when the user updates the number
* of VFs in sysfs.
**/
-int hns3_pci_sriov_configure(struct pci_dev *pdev, int num_vfs)
+static int hns3_pci_sriov_configure(struct pci_dev *pdev, int num_vfs)
{
int ret;


2018-05-16 19:50:04

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH net-next 09/10] net: hns3: Add support of .sriov_configure in HNS3 driver

Hi Peng,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url: https://github.com/0day-ci/linux/commits/Salil-Mehta/Misc-Bug-Fixes-and-clean-ups-for-HNS3-Driver/20180516-211239
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:266:16: sparse: expression using sizeof(void)
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:266:16: sparse: expression using sizeof(void)
>> drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:1582:5: sparse: symbol 'hns3_pci_sriov_configure' was not declared. Should it be static?
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:2513:21: sparse: expression using sizeof(void)
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:2706:22: sparse: expression using sizeof(void)
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:2706:22: sparse: expression using sizeof(void)

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation