Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 5/6] net: hns3: prevent unnecessary MAC TNL interrupt
From: Huazhong Tan @ 2019-08-16  8:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
	jakub.kicinski, Huazhong Tan
In-Reply-To: <1565942982-12105-1-git-send-email-tanhuazhong@huawei.com>

MAC TNL interrupt is used to collect statistic info about
link status changing suddenly when netdev is running.

But when stopping netdev, the enabled MAC TNL interrupt is
unnecessary, and may add some noises to the statistic info.
So this patch disables it before stopping MAC.

Fixes: a63457878b12 ("net: hns3: Add handling of MAC tunnel interruption")
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
Reviewed-by: Yunsheng Lin <linyunsheng@huawei.com>
---
 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 24b59f0..9d64c43 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -6380,6 +6380,8 @@ static void hclge_ae_stop(struct hnae3_handle *handle)
 	for (i = 0; i < handle->kinfo.num_tqps; i++)
 		hclge_reset_tqp(handle, i);
 
+	hclge_config_mac_tnl_int(hdev, false);
+
 	/* Mac disable */
 	hclge_cfg_mac_mode(hdev, false);
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 3/6] net: hns3: fix error and incorrect format
From: Huazhong Tan @ 2019-08-16  8:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
	jakub.kicinski, Guojia Liao, Weihang Li, Jian Shen,
	Guangbin Huang, Huazhong Tan
In-Reply-To: <1565942982-12105-1-git-send-email-tanhuazhong@huawei.com>

From: Guojia Liao <liaoguojia@huawei.com>

The pointer type parameter should be declare as const for preventing
from its pointed value being unexpected modified.

The uninitialized variable can not be return directly. The default
return value is 0 if no abnormal result.

This patch fixes the preceding two errors, deletes redundant
declaration of a function and align one parameter.

Signed-off-by: Guojia Liao <liaoguojia@huawei.com>
Signed-off-by: Weihang Li <liweihang@hisilicon.com>
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hnae3.c             | 2 +-
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c      | 2 +-
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 7 ++++---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 1 -
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
index 6bbba15..528f624 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
@@ -46,7 +46,7 @@ void hnae3_set_client_init_flag(struct hnae3_client *client,
 EXPORT_SYMBOL(hnae3_set_client_init_flag);
 
 static int hnae3_get_client_init_flag(struct hnae3_client *client,
-				       struct hnae3_ae_dev *ae_dev)
+				      struct hnae3_ae_dev *ae_dev)
 {
 	int inited = 0;
 
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 4260653..0332d6f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -704,7 +704,7 @@ static int hns3_get_link_ksettings(struct net_device *netdev,
 	return 0;
 }
 
-static int hns3_check_ksettings_param(struct net_device *netdev,
+static int hns3_check_ksettings_param(const struct net_device *netdev,
 				      const struct ethtool_link_ksettings *cmd)
 {
 	struct hnae3_handle *handle = hns3_get_handle(netdev);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 1d8dee1..24b59f0 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -1128,6 +1128,7 @@ static void hclge_parse_link_mode(struct hclge_dev *hdev, u8 speed_ability)
 	else if (media_type == HNAE3_MEDIA_TYPE_BACKPLANE)
 		hclge_parse_backplane_link_mode(hdev, speed_ability);
 }
+
 static void hclge_parse_cfg(struct hclge_cfg *cfg, struct hclge_desc *desc)
 {
 	struct hclge_cfg_param_cmd *req;
@@ -4364,8 +4365,8 @@ int hclge_bind_ring_with_vector(struct hclge_vport *vport,
 	struct hclge_dev *hdev = vport->back;
 	struct hnae3_ring_chain_node *node;
 	struct hclge_desc desc;
-	struct hclge_ctrl_vector_chain_cmd *req
-		= (struct hclge_ctrl_vector_chain_cmd *)desc.data;
+	struct hclge_ctrl_vector_chain_cmd *req =
+		(struct hclge_ctrl_vector_chain_cmd *)desc.data;
 	enum hclge_cmd_status status;
 	enum hclge_opcode_type op;
 	u16 tqp_type_and_id;
@@ -8656,7 +8657,7 @@ static int hclge_init_client_instance(struct hnae3_client *client,
 		}
 	}
 
-	return ret;
+	return 0;
 
 clear_nic:
 	hdev->nic_client = NULL;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index f6d9b57..7c28933 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -1000,7 +1000,6 @@ int hclge_buffer_alloc(struct hclge_dev *hdev);
 int hclge_rss_init_hw(struct hclge_dev *hdev);
 void hclge_rss_indir_init_cfg(struct hclge_dev *hdev);
 
-int hclge_inform_reset_assert_to_vf(struct hclge_vport *vport);
 void hclge_mbx_handler(struct hclge_dev *hdev);
 int hclge_reset_tqp(struct hnae3_handle *handle, u16 queue_id);
 void hclge_reset_vf_queue(struct hclge_vport *vport, u16 queue_id);
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 6/6] net: hns3: add phy_attached_info() to the hns3 driver
From: Huazhong Tan @ 2019-08-16  8:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
	jakub.kicinski, Yonglong Liu, Huazhong Tan
In-Reply-To: <1565942982-12105-1-git-send-email-tanhuazhong@huawei.com>

From: Yonglong Liu <liuyonglong@huawei.com>

This patch adds the call to phy_attached_info() to the hns3 driver
to identify which exact PHY drivers and models is in use.

Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
Reviewed-by: Yunsheng Lin <linyunsheng@huawei.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
index abb1b43..dc4dfd4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
@@ -231,6 +231,8 @@ int hclge_mac_connect_phy(struct hnae3_handle *handle)
 	linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
 			   phydev->advertising);
 
+	phy_attached_info(phydev);
+
 	return 0;
 }
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 4/6] net: hns3: change print level of RAS error log from warning to error
From: Huazhong Tan @ 2019-08-16  8:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
	jakub.kicinski, Xiaofei Tan, Weihang Li, Huazhong Tan
In-Reply-To: <1565942982-12105-1-git-send-email-tanhuazhong@huawei.com>

From: Xiaofei Tan <tanxiaofei@huawei.com>

This patch changes print level of RAS error log from warning to error.
Because RAS error and its recovery process could cause application
failure. Also uses %u instead of %d when the parameter is unsigned.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
Signed-off-by: Weihang Li <liweihang@hisilicon.com>
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c | 88 +++++++++++-----------
 1 file changed, 44 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
index 5ce9a8a..2425b3f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
@@ -637,8 +637,8 @@ static void hclge_log_error(struct device *dev, char *reg,
 {
 	while (err->msg) {
 		if (err->int_msk & err_sts) {
-			dev_warn(dev, "%s %s found [error status=0x%x]\n",
-				 reg, err->msg, err_sts);
+			dev_err(dev, "%s %s found [error status=0x%x]\n",
+				reg, err->msg, err_sts);
 			if (err->reset_level &&
 			    err->reset_level != HNAE3_NONE_RESET)
 				set_bit(err->reset_level, reset_requests);
@@ -1163,8 +1163,8 @@ static int hclge_handle_mpf_ras_error(struct hclge_dev *hdev,
 
 	status = le32_to_cpu(*(desc_data + 3)) & BIT(0);
 	if (status) {
-		dev_warn(dev, "SSU_ECC_MULTI_BIT_INT_1 ssu_mem32_ecc_mbit_err found [error status=0x%x]\n",
-			 status);
+		dev_err(dev, "SSU_ECC_MULTI_BIT_INT_1 ssu_mem32_ecc_mbit_err found [error status=0x%x]\n",
+			status);
 		set_bit(HNAE3_GLOBAL_RESET, &ae_dev->hw_err_reset_req);
 	}
 
@@ -1200,8 +1200,8 @@ static int hclge_handle_mpf_ras_error(struct hclge_dev *hdev,
 	desc_data = (__le32 *)&desc[5];
 	status = le32_to_cpu(*(desc_data + 1));
 	if (status) {
-		dev_warn(dev, "PPU_MPF_ABNORMAL_INT_ST1 %s found\n",
-			 "rpu_rx_pkt_ecc_mbit_err");
+		dev_err(dev,
+			"PPU_MPF_ABNORMAL_INT_ST1 rpu_rx_pkt_ecc_mbit_err found\n");
 		set_bit(HNAE3_GLOBAL_RESET, &ae_dev->hw_err_reset_req);
 	}
 
@@ -1379,17 +1379,17 @@ static int hclge_log_rocee_axi_error(struct hclge_dev *hdev)
 		return ret;
 	}
 
-	dev_info(dev, "AXI1: %08X %08X %08X %08X %08X %08X\n",
-		 le32_to_cpu(desc[0].data[0]), le32_to_cpu(desc[0].data[1]),
-		 le32_to_cpu(desc[0].data[2]), le32_to_cpu(desc[0].data[3]),
-		 le32_to_cpu(desc[0].data[4]), le32_to_cpu(desc[0].data[5]));
-	dev_info(dev, "AXI2: %08X %08X %08X %08X %08X %08X\n",
-		 le32_to_cpu(desc[1].data[0]), le32_to_cpu(desc[1].data[1]),
-		 le32_to_cpu(desc[1].data[2]), le32_to_cpu(desc[1].data[3]),
-		 le32_to_cpu(desc[1].data[4]), le32_to_cpu(desc[1].data[5]));
-	dev_info(dev, "AXI3: %08X %08X %08X %08X\n",
-		 le32_to_cpu(desc[2].data[0]), le32_to_cpu(desc[2].data[1]),
-		 le32_to_cpu(desc[2].data[2]), le32_to_cpu(desc[2].data[3]));
+	dev_err(dev, "AXI1: %08X %08X %08X %08X %08X %08X\n",
+		le32_to_cpu(desc[0].data[0]), le32_to_cpu(desc[0].data[1]),
+		le32_to_cpu(desc[0].data[2]), le32_to_cpu(desc[0].data[3]),
+		le32_to_cpu(desc[0].data[4]), le32_to_cpu(desc[0].data[5]));
+	dev_err(dev, "AXI2: %08X %08X %08X %08X %08X %08X\n",
+		le32_to_cpu(desc[1].data[0]), le32_to_cpu(desc[1].data[1]),
+		le32_to_cpu(desc[1].data[2]), le32_to_cpu(desc[1].data[3]),
+		le32_to_cpu(desc[1].data[4]), le32_to_cpu(desc[1].data[5]));
+	dev_err(dev, "AXI3: %08X %08X %08X %08X\n",
+		le32_to_cpu(desc[2].data[0]), le32_to_cpu(desc[2].data[1]),
+		le32_to_cpu(desc[2].data[2]), le32_to_cpu(desc[2].data[3]));
 
 	return 0;
 }
@@ -1408,12 +1408,12 @@ static int hclge_log_rocee_ecc_error(struct hclge_dev *hdev)
 		return ret;
 	}
 
-	dev_info(dev, "ECC1: %08X %08X %08X %08X %08X %08X\n",
-		 le32_to_cpu(desc[0].data[0]), le32_to_cpu(desc[0].data[1]),
-		 le32_to_cpu(desc[0].data[2]), le32_to_cpu(desc[0].data[3]),
-		 le32_to_cpu(desc[0].data[4]), le32_to_cpu(desc[0].data[5]));
-	dev_info(dev, "ECC2: %08X %08X %08X\n", le32_to_cpu(desc[1].data[0]),
-		 le32_to_cpu(desc[1].data[1]), le32_to_cpu(desc[1].data[2]));
+	dev_err(dev, "ECC1: %08X %08X %08X %08X %08X %08X\n",
+		le32_to_cpu(desc[0].data[0]), le32_to_cpu(desc[0].data[1]),
+		le32_to_cpu(desc[0].data[2]), le32_to_cpu(desc[0].data[3]),
+		le32_to_cpu(desc[0].data[4]), le32_to_cpu(desc[0].data[5]));
+	dev_err(dev, "ECC2: %08X %08X %08X\n", le32_to_cpu(desc[1].data[0]),
+		le32_to_cpu(desc[1].data[1]), le32_to_cpu(desc[1].data[2]));
 
 	return 0;
 }
@@ -1442,9 +1442,9 @@ static int hclge_log_rocee_ovf_error(struct hclge_dev *hdev)
 			  le32_to_cpu(desc[0].data[0]);
 		while (err->msg) {
 			if (err->int_msk == err_sts) {
-				dev_warn(dev, "%s [error status=0x%x] found\n",
-					 err->msg,
-					 le32_to_cpu(desc[0].data[0]));
+				dev_err(dev, "%s [error status=0x%x] found\n",
+					err->msg,
+					le32_to_cpu(desc[0].data[0]));
 				break;
 			}
 			err++;
@@ -1452,13 +1452,13 @@ static int hclge_log_rocee_ovf_error(struct hclge_dev *hdev)
 	}
 
 	if (le32_to_cpu(desc[0].data[1]) & HCLGE_ROCEE_OVF_ERR_INT_MASK) {
-		dev_warn(dev, "ROCEE TSP OVF [error status=0x%x] found\n",
-			 le32_to_cpu(desc[0].data[1]));
+		dev_err(dev, "ROCEE TSP OVF [error status=0x%x] found\n",
+			le32_to_cpu(desc[0].data[1]));
 	}
 
 	if (le32_to_cpu(desc[0].data[2]) & HCLGE_ROCEE_OVF_ERR_INT_MASK) {
-		dev_warn(dev, "ROCEE SCC OVF [error status=0x%x] found\n",
-			 le32_to_cpu(desc[0].data[2]));
+		dev_err(dev, "ROCEE SCC OVF [error status=0x%x] found\n",
+			le32_to_cpu(desc[0].data[2]));
 	}
 
 	return 0;
@@ -1486,10 +1486,10 @@ hclge_log_and_clear_rocee_ras_error(struct hclge_dev *hdev)
 
 	if (status & HCLGE_ROCEE_AXI_ERR_INT_MASK) {
 		if (status & HCLGE_ROCEE_RERR_INT_MASK)
-			dev_warn(dev, "ROCEE RAS AXI rresp error\n");
+			dev_err(dev, "ROCEE RAS AXI rresp error\n");
 
 		if (status & HCLGE_ROCEE_BERR_INT_MASK)
-			dev_warn(dev, "ROCEE RAS AXI bresp error\n");
+			dev_err(dev, "ROCEE RAS AXI bresp error\n");
 
 		reset_type = HNAE3_FUNC_RESET;
 
@@ -1499,7 +1499,7 @@ hclge_log_and_clear_rocee_ras_error(struct hclge_dev *hdev)
 	}
 
 	if (status & HCLGE_ROCEE_ECC_INT_MASK) {
-		dev_warn(dev, "ROCEE RAS 2bit ECC error\n");
+		dev_err(dev, "ROCEE RAS 2bit ECC error\n");
 		reset_type = HNAE3_GLOBAL_RESET;
 
 		ret = hclge_log_rocee_ecc_error(hdev);
@@ -1640,16 +1640,16 @@ pci_ers_result_t hclge_handle_hw_ras_error(struct hnae3_ae_dev *ae_dev)
 
 	/* Handling Non-fatal HNS RAS errors */
 	if (status & HCLGE_RAS_REG_NFE_MASK) {
-		dev_warn(dev,
-			 "HNS Non-Fatal RAS error(status=0x%x) identified\n",
-			 status);
+		dev_err(dev,
+			"HNS Non-Fatal RAS error(status=0x%x) identified\n",
+			status);
 		hclge_handle_all_ras_errors(hdev);
 	}
 
 	/* Handling Non-fatal Rocee RAS errors */
 	if (hdev->pdev->revision >= 0x21 &&
 	    status & HCLGE_RAS_REG_ROCEE_ERR_MASK) {
-		dev_warn(dev, "ROCEE Non-Fatal RAS error identified\n");
+		dev_err(dev, "ROCEE Non-Fatal RAS error identified\n");
 		hclge_handle_rocee_ras_error(ae_dev);
 	}
 
@@ -1728,8 +1728,8 @@ static void hclge_handle_over_8bd_err(struct hclge_dev *hdev,
 		return;
 	}
 
-	dev_warn(dev, "PPU_PF_ABNORMAL_INT_ST over_8bd_no_fe found, vf_id(%d), queue_id(%d)\n",
-		 vf_id, q_id);
+	dev_err(dev, "PPU_PF_ABNORMAL_INT_ST over_8bd_no_fe found, vf_id(%u), queue_id(%u)\n",
+		vf_id, q_id);
 
 	if (vf_id) {
 		if (vf_id >= hdev->num_alloc_vport) {
@@ -1746,8 +1746,8 @@ static void hclge_handle_over_8bd_err(struct hclge_dev *hdev,
 
 		ret = hclge_inform_reset_assert_to_vf(&hdev->vport[vf_id]);
 		if (ret)
-			dev_warn(dev, "inform reset to vf(%d) failed %d!\n",
-				 hdev->vport->vport_id, ret);
+			dev_err(dev, "inform reset to vf(%u) failed %d!\n",
+				hdev->vport->vport_id, ret);
 	} else {
 		set_bit(HNAE3_FUNC_RESET, reset_requests);
 	}
@@ -1793,8 +1793,8 @@ static int hclge_handle_mpf_msix_error(struct hclge_dev *hdev,
 	status = le32_to_cpu(*(desc_data + 2)) &
 			HCLGE_PPU_MPF_INT_ST2_MSIX_MASK;
 	if (status)
-		dev_warn(dev, "PPU_MPF_ABNORMAL_INT_ST2 rx_q_search_miss found [dfx status=0x%x\n]",
-			 status);
+		dev_err(dev, "PPU_MPF_ABNORMAL_INT_ST2 rx_q_search_miss found [dfx status=0x%x\n]",
+			status);
 
 	/* clear all main PF MSIx errors */
 	ret = hclge_clear_hw_msix_error(hdev, desc, true, mpf_bd_num);
@@ -1988,7 +1988,7 @@ void hclge_handle_all_hns_hw_errors(struct hnae3_ae_dev *ae_dev)
 
 	/* Handle Non-fatal HNS RAS errors */
 	if (status & HCLGE_RAS_REG_NFE_MASK) {
-		dev_warn(dev, "HNS hw error(RAS) identified during init\n");
+		dev_err(dev, "HNS hw error(RAS) identified during init\n");
 		hclge_handle_all_ras_errors(hdev);
 	}
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH net-next 0/6] net: hns3: add some cleanups & bugfix
From: Huazhong Tan @ 2019-08-16  8:09 UTC (permalink / raw)
  To: davem
  Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
	jakub.kicinski, Huazhong Tan

This patch-set includes cleanups and bugfix for the HNS3 ethernet
controller driver.

[patch 01/06 - 03/06] adds some cleanups.

[patch 04/06] changes the print level of RAS.

[patch 05/06] fixes a bug related to MAC TNL.

[patch 06/06] adds phy_attached_info().

Guojia Liao (3):
  net: hns3: add or modify comments
  net: hns3: modify redundant initialization of variable
  net: hns3: fix error and incorrect format

Huazhong Tan (1):
  net: hns3: prevent unnecessary MAC TNL interrupt

Xiaofei Tan (1):
  net: hns3: change print level of RAS error log from warning to error

Yonglong Liu (1):
  net: hns3: add phy_attached_info() to the hns3 driver

 drivers/net/ethernet/hisilicon/hns3/hnae3.c        |  9 +--
 drivers/net/ethernet/hisilicon/hns3/hnae3.h        | 12 +--
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c    |  8 +-
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h    |  8 +-
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c |  6 +-
 .../ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c |  3 +
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c | 90 +++++++++++-----------
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 17 ++--
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h    |  1 -
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c |  2 +-
 .../ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c    |  2 +
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c  |  2 +-
 .../ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c   |  2 +-
 .../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c  | 11 ++-
 14 files changed, 89 insertions(+), 84 deletions(-)

-- 
2.7.4


^ permalink raw reply

* Re: [PATCH] arm64: do_csum: implement accelerated scalar version
From: Shaokun Zhang @ 2019-08-16  8:15 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Ard Biesheuvel, linux-arm-kernel, netdev,
	ilias.apalodimas, huanglingyan (A), steve.capper
In-Reply-To: <20190815164609.GI2015@fuggles.cambridge.arm.com>

Hi Will,

On 2019/8/16 0:46, Will Deacon wrote:
> On Thu, May 16, 2019 at 11:14:35AM +0800, Zhangshaokun wrote:
>> On 2019/5/15 17:47, Will Deacon wrote:
>>> On Mon, Apr 15, 2019 at 07:18:22PM +0100, Robin Murphy wrote:
>>>> On 12/04/2019 10:52, Will Deacon wrote:
>>>>> I'm waiting for Robin to come back with numbers for a C implementation.
>>>>>
>>>>> Robin -- did you get anywhere with that?
>>>>
>>>> Still not what I would call finished, but where I've got so far (besides an
>>>> increasingly elaborate test rig) is as below - it still wants some unrolling
>>>> in the middle to really fly (and actual testing on BE), but the worst-case
>>>> performance already equals or just beats this asm version on Cortex-A53 with
>>>> GCC 7 (by virtue of being alignment-insensitive and branchless except for
>>>> the loop). Unfortunately, the advantage of C code being instrumentable does
>>>> also come around to bite me...
>>>
>>> Is there any interest from anybody in spinning a proper patch out of this?
>>> Shaokun?
>>
>> HiSilicon's Kunpeng920(Hi1620) benefits from do_csum optimization, if Ard and
>> Robin are ok, Lingyan or I can try to do it.
>> Of course, if any guy posts the patch, we are happy to test it.
>> Any will be ok.
> 
> I don't mind who posts it, but Robin is super busy with SMMU stuff at the
> moment so it probably makes more sense for you or Lingyan to do it.

Thanks for restarting this topic, I or Lingyan will do it soon.

Thanks,
Shaokun

> 
> Will
> 
> .
> 


^ permalink raw reply

* Re: [PATCH net-next] r8152: divide the tx and rx bottom functions
From: Eric Dumazet @ 2019-08-16  8:19 UTC (permalink / raw)
  To: Hayes Wang, netdev@vger.kernel.org; +Cc: nic_swsd, linux-kernel@vger.kernel.org
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2F18D470D@RTITMBSVM03.realtek.com.tw>



On 8/16/19 10:10 AM, Hayes Wang wrote:
> Eric Dumazet [mailto:eric.dumazet@gmail.com]
>> Sent: Friday, August 16, 2019 2:40 PM
> [...]
>> tasklet and NAPI are scheduled on the same core (the current
>> cpu calling napi_schedule() or tasklet_schedule())
>>
>> I would rather not add this dubious tasklet, and instead try to understand
>> what is wrong in this driver ;)
>>
>> The various napi_schedule() calls are suspect IMO.
> 
> The original method as following.
> 
> static int r8152_poll(struct napi_struct *napi, int budget)
> {
> 	struct r8152 *tp = container_of(napi, struct r8152, napi);
> 	int work_done;
> 
> 	work_done = rx_bottom(tp, budget); <-- RX
> 	bottom_half(tp); <-- Tx (tx_bottom)
> 	[...]
> 
> The rx_bottom and tx_bottom would only be called in r8152_poll.
> That is, tx_bottom wouldn't be run unless rx_bottom is finished.
> And, rx_bottom would be called if tx_bottom is running.
> 
> If the traffic is busy. rx_bottom or tx_bottom may take a lot
> of time to deal with the packets. And the one would increase
> the latency time for the other one.
> 
> Therefore, when I separate the tx_bottom and rx_bottom to
> different tasklet and napi, the callback functions of tx and
> rx may schedule the tasklet and napi to different cpu. Then,
> the rx_bottom and tx_bottom may be run at the same time.


Your patch makes absolutely no guarantee of doing what you
want, I am afraid.

> 
> Take our arm platform for example. There are five cpus to
> handle the interrupt of USB host controller. When the rx is
> completed, cpu #1 may handle the interrupt and napi would
> be scheduled. When the tx is finished, cpu #2 may handle
> the interrupt and the tasklet is scheduled. Then, napi is
> run on cpu #1, and tasklet is run on cpu #2.
> 
>> Also rtl8152_start_xmit() uses skb_queue_tail(&tp->tx_queue, skb);
>>
>> But I see nothing really kicking the transmit if tx_free is empty ?
> 
> Tx callback function "write_bulk_callback" would deal with it.
> The callback function would check if there are packets waiting
> to be sent.

Which callback ?

After an idle period (no activity, no prior packets being tx-completed ...),
a packet is sent by the upper stack, enters the ndo_start_xmit() of a network driver.

This driver ndo_start_xmit() simply adds an skb to a local list, and returns.

Where/how is scheduled this callback ?

Some kind of timer ?
An (unrelated) incoming packet ?

> 
> 
> Best Regards,
> Hayes
> 
> 

^ permalink raw reply

* [PATCH REPOST 0/2] can: flexcan: fix PM and wakeup issue
From: Joakim Zhang @ 2019-08-16  8:20 UTC (permalink / raw)
  To: mkl@pengutronix.de, linux-can@vger.kernel.org, sean@geanix.com
  Cc: wg@grandegger.com, netdev@vger.kernel.org, dl-linux-imx,
	Joakim Zhang

This patch set intends to fix Flecan PM and wakeup issue.

Joakim Zhang (2):
  can: flexcan: fix deadlock when using self wakeup
  can: flexcan: add LPSR mode support for i.MX7D

 drivers/net/can/flexcan.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH REPOST 1/2] can: flexcan: fix deadlock when using self wakeup
From: Joakim Zhang @ 2019-08-16  8:20 UTC (permalink / raw)
  To: mkl@pengutronix.de, linux-can@vger.kernel.org, sean@geanix.com
  Cc: wg@grandegger.com, netdev@vger.kernel.org, dl-linux-imx,
	Joakim Zhang
In-Reply-To: <20190816081749.19300-1-qiangqing.zhang@nxp.com>

As reproted by Sean Nyekjaer below:
When suspending, when there is still can traffic on the interfaces the
flexcan immediately wakes the platform again. As it should :-). But it
throws this error msg:
[ 3169.378661] PM: noirq suspend of devices failed

On the way down to suspend the interface that throws the error message does
call flexcan_suspend but fails to call flexcan_noirq_suspend. That means the
flexcan_enter_stop_mode is called, but on the way out of suspend the driver
only calls flexcan_resume and skips flexcan_noirq_resume, thus it doesn't call
flexcan_exit_stop_mode. This leaves the flexcan in stop mode, and with the
current driver it can't recover from this even with a soft reboot, it requires
a hard reboot.

The best way to exit stop mode is in Wake Up interrupt context, and then
suspend() and resume() functions can be symmetric. However, stop mode
request and ack will be controlled by SCU(System Control Unit) firmware(manage
clock,power,stop mode, etc. by Cortex-M4 core) in coming i.MX8(QM/QXP). And SCU
firmware interface can't be available in interrupt context.

For compatibillity, the wake up mechanism can't be symmetric, so we need
in_stop_mode hack.

Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
Reported-by: Sean Nyekjaer <sean@geanix.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>

Changelog:
V1->V2:
	* add Reported-by tag.
	* rebase on patch: can:flexcan:fix stop mode acknowledgment.
V2->V3:
	* move into a patch set.
---
 drivers/net/can/flexcan.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 56fa98d7aa90..de2bf71b335b 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -282,6 +282,7 @@ struct flexcan_priv {
 	const struct flexcan_devtype_data *devtype_data;
 	struct regulator *reg_xceiver;
 	struct flexcan_stop_mode stm;
+	bool in_stop_mode;
 
 	/* Read and Write APIs */
 	u32 (*read)(void __iomem *addr);
@@ -1636,6 +1637,8 @@ static int __maybe_unused flexcan_suspend(struct device *device)
 			err = flexcan_enter_stop_mode(priv);
 			if (err)
 				return err;
+
+			priv->in_stop_mode = true;
 		} else {
 			err = flexcan_chip_disable(priv);
 			if (err)
@@ -1660,6 +1663,15 @@ static int __maybe_unused flexcan_resume(struct device *device)
 		netif_device_attach(dev);
 		netif_start_queue(dev);
 		if (device_may_wakeup(device)) {
+			if (priv->in_stop_mode) {
+				flexcan_enable_wakeup_irq(priv, false);
+				err = flexcan_exit_stop_mode(priv);
+				if (err)
+					return  err;
+
+				priv->in_stop_mode = false;
+			}
+
 			disable_irq_wake(dev->irq);
 		} else {
 			err = flexcan_chip_enable(priv);
@@ -1675,6 +1687,11 @@ static int __maybe_unused flexcan_noirq_suspend(struct device *device)
 	struct net_device *dev = dev_get_drvdata(device);
 	struct flexcan_priv *priv = netdev_priv(dev);
 
+	/* Need to enable wakeup interrupt in noirq suspend stage. Otherwise,
+	 * it will trigger continuously wakeup interrupt if the wakeup event
+	 * comes before noirq suspend stage, and simultaneously it has enter
+	 * the stop mode.
+	 */
 	if (netif_running(dev) && device_may_wakeup(device))
 		flexcan_enable_wakeup_irq(priv, true);
 
@@ -1687,11 +1704,17 @@ static int __maybe_unused flexcan_noirq_resume(struct device *device)
 	struct flexcan_priv *priv = netdev_priv(dev);
 	int err;
 
+	/* Need to exit stop mode in noirq resume stage. Otherwise, it will
+	 * trigger continuously wakeup interrupt if the wakeup event comes,
+	 * and simultaneously it has still in stop mode.
+	 */
 	if (netif_running(dev) && device_may_wakeup(device)) {
 		flexcan_enable_wakeup_irq(priv, false);
 		err = flexcan_exit_stop_mode(priv);
 		if (err)
 			return err;
+
+		priv->in_stop_mode = false;
 	}
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related

* [PATCH REPOST 2/2] can: flexcan: add LPSR mode support for i.MX7D
From: Joakim Zhang @ 2019-08-16  8:20 UTC (permalink / raw)
  To: mkl@pengutronix.de, linux-can@vger.kernel.org, sean@geanix.com
  Cc: wg@grandegger.com, netdev@vger.kernel.org, dl-linux-imx,
	Joakim Zhang
In-Reply-To: <20190816081749.19300-1-qiangqing.zhang@nxp.com>

For i.MX7D LPSR mode, the controller will lost power and got the
configuration state lost after system resume back. (coming i.MX8QM/QXP
will also completely power off the domain, the controller state will be
lost and needs restore).
So we need to set pinctrl state again and re-start chip to do
re-configuration after resume.

For wakeup case, it should not set pinctrl to sleep state by
pinctrl_pm_select_sleep_state.
For interface is not up before suspend case, we don't need
re-configure as it will be configured by user later by interface up.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>

ChangeLog:
V1->V2:
	* rebase on linux-can/testing.
	* move into a patch set.
---
 drivers/net/can/flexcan.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index de2bf71b335b..b3edaf6a5a61 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -25,6 +25,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/regmap.h>
 
 #define DRV_NAME			"flexcan"
@@ -1640,9 +1641,9 @@ static int __maybe_unused flexcan_suspend(struct device *device)
 
 			priv->in_stop_mode = true;
 		} else {
-			err = flexcan_chip_disable(priv);
-			if (err)
-				return err;
+			flexcan_chip_stop(dev);
+
+			pinctrl_pm_select_sleep_state(device);
 		}
 		netif_stop_queue(dev);
 		netif_device_detach(dev);
@@ -1674,7 +1675,9 @@ static int __maybe_unused flexcan_resume(struct device *device)
 
 			disable_irq_wake(dev->irq);
 		} else {
-			err = flexcan_chip_enable(priv);
+			pinctrl_pm_select_default_state(device);
+
+			err = flexcan_chip_start(dev);
 			if (err)
 				return err;
 		}
-- 
2.17.1


^ permalink raw reply related

* Re: WARNING in is_bpf_text_address
From: Will Deacon @ 2019-08-16  8:21 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: syzbot, akpm, ast, bpf, daniel, davem, dvyukov, hawk, hdanton,
	jakub.kicinski, johannes.berg, johannes, john.fastabend, kafai,
	linux-kernel, longman, mingo, netdev, paulmck, peterz,
	songliubraving, syzkaller-bugs, tglx, tj, torvalds, will.deacon,
	xdp-newbies, yhs
In-Reply-To: <456d0da6-3e16-d3fc-ecf6-7abb410bf689@acm.org>

On Thu, Aug 15, 2019 at 06:39:56PM -0700, Bart Van Assche wrote:
> On 8/15/19 12:51 AM, Will Deacon wrote:
> > On Sat, Aug 10, 2019 at 05:24:06PM -0700, syzbot wrote:
> > > The bug was bisected to:
> > > 
> > > commit a0b0fd53e1e67639b303b15939b9c653dbe7a8c4
> > > Author: Bart Van Assche <bvanassche@acm.org>
> > > Date:   Thu Feb 14 23:00:46 2019 +0000
> > > 
> > >      locking/lockdep: Free lock classes that are no longer in use
> > > 
> > > bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=152f6a9da00000
> > > final crash:    https://syzkaller.appspot.com/x/report.txt?x=172f6a9da00000
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=132f6a9da00000
> > 
> > I know you don't think much to these reports, but please could you have a
> > look (even if it's just to declare it a false positive)?
> 
> Had you already noticed the following message?
> 
> https://lore.kernel.org/bpf/d76d7a63-7854-e92d-30cb-52546d333ffe@iogearbox.net/
> 
> From that message: "Hey Bart, don't think it's related in any way to your
> commit. I'll allocate some time on working on this issue today, thanks!"

Apologies, but I hadn't received that when I sent my initial email. Anyway,
just wanted to make sure somebody was looking into it!

Will

^ permalink raw reply

* Re: [PATCH net] tunnel: fix dev null pointer dereference when send pkg larger than mtu in collect_md mode
From: Eric Dumazet @ 2019-08-16  8:23 UTC (permalink / raw)
  To: Hangbin Liu
  Cc: netdev, Stefano Brivio, wenxu, Alexei Starovoitov,
	David S . Miller
In-Reply-To: <20190816032418.GX18865@dhcp-12-139.nay.redhat.com>



On 8/16/19 5:24 AM, Hangbin Liu wrote:
> Hi Eric,
> 
> Thanks for the review.
> On Thu, Aug 15, 2019 at 11:16:58AM +0200, Eric Dumazet wrote:
>>> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
>>> index 38c02bb62e2c..c6713c7287df 100644
>>> --- a/net/ipv4/ip_tunnel.c
>>> +++ b/net/ipv4/ip_tunnel.c
>>> @@ -597,6 +597,9 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>>>  		goto tx_error;
>>>  	}
>>>  
>>> +	if (skb_dst(skb) && !skb_dst(skb)->dev)
>>> +		skb_dst(skb)->dev = rt->dst.dev;
>>> +
>>
>>
>> IMO this looks wrong.
>> This dst seems shared. 
> 
> If the dst is shared, it may cause some problem. Could you point me where the
> dst may be shared possibly?
>

dst are inherently shared.

This is why we have a refcount on them.

Only when the dst has been allocated by the current thread we can make changes on them.


^ permalink raw reply

* AW: can: tcan4x5x: spi bits_per_word issue on Raspberry PI
From: FIXED-TERM Buecheler Konstantin (ETAS-SEC/ECT-Mu) @ 2019-08-16  8:26 UTC (permalink / raw)
  To: Patrick Menschel, Marc Kleine-Budde, linux-can@vger.kernel.org,
	netdev@vger.kernel.org
  Cc: Dan Murphy
In-Reply-To: <e6577cc2-89fc-9428-b73e-47f41eff2949@posteo.de>

> >> Now I have another really confusing problem. Anything I write to SPI is written little endian. The tcan chip expects big endian.
> >> Anything I read from SPI is treated as little endian but is big endian. Does anyone know why this happens?
> >> Is there a flag or something I can set for the SPI device/wire to fix this?
> >
> > Have you changed the bits_per_word to 8? Then you read just a stream
> > of bytes. If you tread them as an u32 they are in host order.
> >

@Marc
Yes, I changed bits_per_word to 8. Since the PI does not support any values apart from 
8 and 9 this seems to be the only way.

> > Marc
> >
> 
> 
> Hi,
> 
> from my experience with SPIDEV on RPI, the driver uses a char array for I/O.
> As the RPI code is build little endian, logically little endian comes out of SPI. You
> basically have to invert the bit and byte order by hand for a big endian slave.
> 

@Patrick, Marc
You both are right. This seems to be the problem. The SPI driver uses char arrays
and the tcan driver treats them as u32. 

I will try to change the byte order by hand to get it running for my project. But in the long 
run, this does not seem to be a proper solution... 

Regards, 
Konstantin 


> Clock Phase and Clock Polarity are also an issue on the RPI as at least SPIDEV
> kindly overlooks any options set previously.
> I had my share of this while writing a test app for a MAX31855 IC and ended up
> casting a little endian array to a big endian structure.
> 
> Regards,
> Patrick


^ permalink raw reply

* Re: AW: can: tcan4x5x: spi bits_per_word issue on Raspberry PI
From: Marc Kleine-Budde @ 2019-08-16  8:37 UTC (permalink / raw)
  To: FIXED-TERM Buecheler Konstantin (ETAS-SEC/ECT-Mu),
	Patrick Menschel, linux-can@vger.kernel.org,
	netdev@vger.kernel.org
  Cc: Dan Murphy
In-Reply-To: <47108d803086402c83d1073f3e3a62bb@escrypt.com>


[-- Attachment #1.1: Type: text/plain, Size: 1583 bytes --]

On 8/16/19 10:26 AM, FIXED-TERM Buecheler Konstantin (ETAS-SEC/ECT-Mu)
wrote:
>>> Have you changed the bits_per_word to 8? Then you read just a stream
>>> of bytes. If you tread them as an u32 they are in host order.
> 
> @Marc
> Yes, I changed bits_per_word to 8. Since the PI does not support any values apart from 
> 8 and 9 this seems to be the only way.

ok

>> from my experience with SPIDEV on RPI, the driver uses a char array for I/O.
>> As the RPI code is build little endian, logically little endian comes out of SPI. You
>> basically have to invert the bit and byte order by hand for a big endian slave.
> 
> @Patrick, Marc
> You both are right. This seems to be the problem. The SPI driver uses char arrays
> and the tcan driver treats them as u32. 

No, the tcan configures the spi device or rather the host driver that
the word len should be 32 bit. If you change the value to 8 you break
the assumptions in the driver.

> I will try to change the byte order by hand to get it running for my project. But in the long 
> run, this does not seem to be a proper solution... 

Indeed, that's the wrong approach. I don't have any HW here to test, but
I'll prepare a patch. BTW: the driver is broken in several ways, when it
comes to the regmap configuration.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* RE: [PATCH net-next] r8152: divide the tx and rx bottom functions
From: Hayes Wang @ 2019-08-16  9:08 UTC (permalink / raw)
  To: Eric Dumazet, netdev@vger.kernel.org
  Cc: nic_swsd, linux-kernel@vger.kernel.org
In-Reply-To: <68015004-fb60-f6c6-05b0-610466223cf5@gmail.com>

Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Friday, August 16, 2019 4:20 PM
[...]
> Which callback ?

The USB device has two endpoints for Tx and Rx.
If I submit tx or rx URB to the USB host controller,
the relative callback functions would be called, when
they are finished. For rx, it is read_bulk_callback.
For tx, it is write_bulk_callback.

> After an idle period (no activity, no prior packets being tx-completed ...),
> a packet is sent by the upper stack, enters the ndo_start_xmit() of a network
> driver.
> 
> This driver ndo_start_xmit() simply adds an skb to a local list, and returns.

Base on the current method (without tasklet), when
ndo_start_xmit() is called, napi_schedule is called only
if there is at least one free buffer (!list_empty(&tp->tx_free))
to transmit the packet. Then, the flow would be as following.

    - Call r8152_poll
     -- Call bottom_half
      --- Call tx_bottom
       ---- Call r8152_tx_agg_fill
        ----- submit tx urb

    - Call write_bulk_callback if tx is completed

When the tx transfer is completed, write_bulk_callback would
be called. And it would check if there is any tx packet
in &tp->tx_queue and determine whether it is necessary to
schedule the napi again or not.

> Where/how is scheduled this callback ?

For tx, you could find the following code in r8152_tx_agg_fill().

	usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2),
			  agg->head, (int)(tx_data - (u8 *)agg->head),
			  (usb_complete_t)write_bulk_callback, agg);
	ret = usb_submit_urb(agg->urb, GFP_ATOMIC);

For rx you could find the following code in r8152_submit_rx().

	usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
			  agg->buffer, tp->rx_buf_sz,
			  (usb_complete_t)read_bulk_callback, agg);

	ret = usb_submit_urb(agg->urb, mem_flags);

> Some kind of timer ?
> An (unrelated) incoming packet ?

When the rx or tx is completed, a interrupt of USB
host controller would be triggered. Then, the callback
functions would be called.

Best Regards,
Hayes



^ permalink raw reply

* Re: [PATCH bpf-next 3/5] i40e: Enable XDP_SKIP_BPF option for AF_XDP sockets
From: kbuild test robot @ 2019-08-16  9:21 UTC (permalink / raw)
  To: Sridhar Samudrala
  Cc: kbuild-all, magnus.karlsson, bjorn.topel, netdev, bpf,
	sridhar.samudrala, intel-wired-lan, maciej.fijalkowski,
	tom.herbert
In-Reply-To: <1565840783-8269-4-git-send-email-sridhar.samudrala@intel.com>

[-- Attachment #1: Type: text/plain, Size: 6746 bytes --]

Hi Sridhar,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Sridhar-Samudrala/xsk-Convert-bool-zc-field-in-struct-xdp_umem-to-a-u32-bitmap/20190816-144642
base:   https://kernel.googlesource.com/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-lkp (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/net/ethernet/intel/i40e/i40e_txrx.c: In function 'i40e_run_xdp':
>> drivers/net/ethernet/intel/i40e/i40e_txrx.c:2215:9: error: implicit declaration of function 'xsk_umem_rcv'; did you mean 'xsk_rcv'? [-Werror=implicit-function-declaration]
      err = xsk_umem_rcv(umem, xdp);
            ^~~~~~~~~~~~
            xsk_rcv
   drivers/net/ethernet/intel/i40e/i40e_txrx.c: In function 'i40e_finalize_xdp_rx':
>> drivers/net/ethernet/intel/i40e/i40e_txrx.c:2322:4: error: implicit declaration of function 'xsk_umem_flush'; did you mean 'xsk_umem_fq_reuse'? [-Werror=implicit-function-declaration]
       xsk_umem_flush(umem);
       ^~~~~~~~~~~~~~
       xsk_umem_fq_reuse
   cc1: some warnings being treated as errors
--
   drivers/net/ethernet/intel/i40e/i40e_xsk.c: In function 'i40e_run_xdp_zc':
>> drivers/net/ethernet/intel/i40e/i40e_xsk.c:199:9: error: implicit declaration of function 'xsk_umem_rcv'; did you mean 'xsk_rcv'? [-Werror=implicit-function-declaration]
      err = xsk_umem_rcv(rx_ring->xsk_umem, xdp);
            ^~~~~~~~~~~~
            xsk_rcv
   cc1: some warnings being treated as errors

vim +2215 drivers/net/ethernet/intel/i40e/i40e_txrx.c

  2190	
  2191	/**
  2192	 * i40e_run_xdp - run an XDP program
  2193	 * @rx_ring: Rx ring being processed
  2194	 * @xdp: XDP buffer containing the frame
  2195	 **/
  2196	static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring,
  2197					    struct xdp_buff *xdp)
  2198	{
  2199		int err, result = I40E_XDP_PASS;
  2200		struct i40e_ring *xdp_ring;
  2201		struct bpf_prog *xdp_prog;
  2202		struct xdp_umem *umem;
  2203		u32 act;
  2204	
  2205		rcu_read_lock();
  2206		xdp_prog = READ_ONCE(rx_ring->xdp_prog);
  2207	
  2208		if (!xdp_prog)
  2209			goto xdp_out;
  2210	
  2211		prefetchw(xdp->data_hard_start); /* xdp_frame write */
  2212	
  2213		umem = xdp_get_umem_from_qid(rx_ring->netdev, rx_ring->queue_index);
  2214		if (xsk_umem_skip_bpf(umem)) {
> 2215			err = xsk_umem_rcv(umem, xdp);
  2216			result = !err ? I40E_XDP_REDIR : I40E_XDP_CONSUMED;
  2217			goto xdp_out;
  2218		}
  2219	
  2220		act = bpf_prog_run_xdp(xdp_prog, xdp);
  2221		switch (act) {
  2222		case XDP_PASS:
  2223			break;
  2224		case XDP_TX:
  2225			xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
  2226			result = i40e_xmit_xdp_tx_ring(xdp, xdp_ring);
  2227			break;
  2228		case XDP_REDIRECT:
  2229			err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog);
  2230			result = !err ? I40E_XDP_REDIR : I40E_XDP_CONSUMED;
  2231			break;
  2232		default:
  2233			bpf_warn_invalid_xdp_action(act);
  2234			/* fall through */
  2235		case XDP_ABORTED:
  2236			trace_xdp_exception(rx_ring->netdev, xdp_prog, act);
  2237			/* fall through -- handle aborts by dropping packet */
  2238		case XDP_DROP:
  2239			result = I40E_XDP_CONSUMED;
  2240			break;
  2241		}
  2242	xdp_out:
  2243		rcu_read_unlock();
  2244		return ERR_PTR(-result);
  2245	}
  2246	
  2247	/**
  2248	 * i40e_rx_buffer_flip - adjusted rx_buffer to point to an unused region
  2249	 * @rx_ring: Rx ring
  2250	 * @rx_buffer: Rx buffer to adjust
  2251	 * @size: Size of adjustment
  2252	 **/
  2253	static void i40e_rx_buffer_flip(struct i40e_ring *rx_ring,
  2254					struct i40e_rx_buffer *rx_buffer,
  2255					unsigned int size)
  2256	{
  2257	#if (PAGE_SIZE < 8192)
  2258		unsigned int truesize = i40e_rx_pg_size(rx_ring) / 2;
  2259	
  2260		rx_buffer->page_offset ^= truesize;
  2261	#else
  2262		unsigned int truesize = SKB_DATA_ALIGN(i40e_rx_offset(rx_ring) + size);
  2263	
  2264		rx_buffer->page_offset += truesize;
  2265	#endif
  2266	}
  2267	
  2268	/**
  2269	 * i40e_xdp_ring_update_tail - Updates the XDP Tx ring tail register
  2270	 * @xdp_ring: XDP Tx ring
  2271	 *
  2272	 * This function updates the XDP Tx ring tail register.
  2273	 **/
  2274	void i40e_xdp_ring_update_tail(struct i40e_ring *xdp_ring)
  2275	{
  2276		/* Force memory writes to complete before letting h/w
  2277		 * know there are new descriptors to fetch.
  2278		 */
  2279		wmb();
  2280		writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail);
  2281	}
  2282	
  2283	/**
  2284	 * i40e_update_rx_stats - Update Rx ring statistics
  2285	 * @rx_ring: rx descriptor ring
  2286	 * @total_rx_bytes: number of bytes received
  2287	 * @total_rx_packets: number of packets received
  2288	 *
  2289	 * This function updates the Rx ring statistics.
  2290	 **/
  2291	void i40e_update_rx_stats(struct i40e_ring *rx_ring,
  2292				  unsigned int total_rx_bytes,
  2293				  unsigned int total_rx_packets)
  2294	{
  2295		u64_stats_update_begin(&rx_ring->syncp);
  2296		rx_ring->stats.packets += total_rx_packets;
  2297		rx_ring->stats.bytes += total_rx_bytes;
  2298		u64_stats_update_end(&rx_ring->syncp);
  2299		rx_ring->q_vector->rx.total_packets += total_rx_packets;
  2300		rx_ring->q_vector->rx.total_bytes += total_rx_bytes;
  2301	}
  2302	
  2303	/**
  2304	 * i40e_finalize_xdp_rx - Bump XDP Tx tail and/or flush redirect map
  2305	 * @rx_ring: Rx ring
  2306	 * @xdp_res: Result of the receive batch
  2307	 *
  2308	 * This function bumps XDP Tx tail and/or flush redirect map, and
  2309	 * should be called when a batch of packets has been processed in the
  2310	 * napi loop.
  2311	 **/
  2312	void i40e_finalize_xdp_rx(struct i40e_ring *rx_ring, unsigned int xdp_res)
  2313	{
  2314		if (xdp_res & I40E_XDP_REDIR) {
  2315			struct xdp_umem *umem;
  2316	
  2317			umem = rx_ring->xsk_umem;
  2318			if (!umem)
  2319				umem = xdp_get_umem_from_qid(rx_ring->netdev,
  2320							     rx_ring->queue_index);
  2321			if (xsk_umem_skip_bpf(umem))
> 2322				xsk_umem_flush(umem);
  2323			else
  2324				xdp_do_flush_map();
  2325		}
  2326	
  2327		if (xdp_res & I40E_XDP_TX) {
  2328			struct i40e_ring *xdp_ring =
  2329				rx_ring->vsi->xdp_rings[rx_ring->queue_index];
  2330	
  2331			i40e_xdp_ring_update_tail(xdp_ring);
  2332		}
  2333	}
  2334	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27706 bytes --]

^ permalink raw reply

* Re: linux-next: Signed-off-by missing for commits in the net-next tree
From: Andy Grover @ 2019-08-16  9:15 UTC (permalink / raw)
  To: Gerd Rausch, Stephen Rothwell, David Miller, Networking,
	Chris Mason
  Cc: Linux Next Mailing List, Linux Kernel Mailing List, Andy Grover,
	Chris Mason
In-Reply-To: <8fd20efa-8e3d-eca2-8adf-897428a2f9ad@oracle.com>

On 8/16/19 3:06 PM, Gerd Rausch wrote:
> Hi,
> 
> Just added the e-mail addresses I found using a simple "google search",
> in order to reach out to the original authors of these commits:
> Chris Mason and Andy Grover.
> 
> I'm hoping they still remember their work from 7-8 years ago.

Yes looks like what I was working on. What did you need from me? It's
too late to amend the commitlogs...

-- Andy

> 
> Thanks,
> 
>   Gerd
> 
> On 15/08/2019 14.53, Stephen Rothwell wrote:
>> Hi all,
>>
>> Commits
>>
>>   11740ef44829 ("rds: check for excessive looping in rds_send_xmit")
>>   65dedd7fe1f2 ("RDS: limit the number of times we loop in rds_send_xmit")
>>
>> are missing a Signed-off-by from their authors.
>>

^ permalink raw reply

* [PATCH v2 00/10] Add definition for the number of standard PCI BARs
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, linux-kernel, linux-pci, Sebastian Ott,
	Gerald Schaefer, H. Peter Anvin, Giuseppe Cavallaro,
	Alexandre Torgue, Matt Porter, Alexandre Bounine, Peter Jones,
	Bartlomiej Zolnierkiewicz, Cornelia Huck, Alex Williamson,
	Andrew Murray, Jose Abreu, kvm, linux-fbdev, netdev, x86,
	linux-s390

Code that iterates over all standard PCI BARs typically uses
PCI_STD_RESOURCE_END, but this is error-prone because it requires
"i <= PCI_STD_RESOURCE_END" rather than something like
"i < PCI_STD_NUM_BARS". We could add such a definition and use it the same
way PCI_SRIOV_NUM_BARS is used. There is already the definition
PCI_BAR_COUNT for s390 only. Thus, this patchset introduces it globally.

Changes in v2:
  - Reverse checks in pci_iomap_range,pci_iomap_wc_range.
  - Refactor loops in vfio_pci to keep PCI_STD_RESOURCES.
  - Add 2 new patches to replace the magic constant with new define.
  - Split net patch in v1 to separate stmmac and dwc-xlgmac patches.

Denis Efremov (10):
  PCI: Add define for the number of standard PCI BARs
  s390/pci: Loop using PCI_STD_NUM_BARS
  x86/PCI: Loop using PCI_STD_NUM_BARS
  stmmac: pci: Loop using PCI_STD_NUM_BARS
  net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS
  rapidio/tsi721: Loop using PCI_STD_NUM_BARS
  efifb: Loop using PCI_STD_NUM_BARS
  vfio_pci: Loop using PCI_STD_NUM_BARS
  PCI: hv: Use PCI_STD_NUM_BARS
  PCI: Use PCI_STD_NUM_BARS

 arch/s390/include/asm/pci.h                      |  5 +----
 arch/s390/include/asm/pci_clp.h                  |  6 +++---
 arch/s390/pci/pci.c                              | 16 ++++++++--------
 arch/s390/pci/pci_clp.c                          |  6 +++---
 arch/x86/pci/common.c                            |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |  4 ++--
 drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c   |  2 +-
 drivers/pci/controller/pci-hyperv.c              | 10 +++++-----
 drivers/pci/pci.c                                | 11 ++++++-----
 drivers/pci/quirks.c                             |  4 ++--
 drivers/rapidio/devices/tsi721.c                 |  2 +-
 drivers/vfio/pci/vfio_pci.c                      | 11 +++++++----
 drivers/vfio/pci/vfio_pci_config.c               | 10 ++++++----
 drivers/vfio/pci/vfio_pci_private.h              |  4 ++--
 drivers/video/fbdev/efifb.c                      |  2 +-
 include/linux/pci.h                              |  2 +-
 include/uapi/linux/pci_regs.h                    |  1 +
 17 files changed, 51 insertions(+), 47 deletions(-)

-- 
2.21.0


^ permalink raw reply

* [PATCH v2 01/10] PCI: Add define for the number of standard PCI BARs
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, linux-kernel, linux-pci, Sebastian Ott,
	Gerald Schaefer, H. Peter Anvin, Giuseppe Cavallaro,
	Alexandre Torgue, Matt Porter, Alexandre Bounine, Peter Jones,
	Bartlomiej Zolnierkiewicz, Cornelia Huck, Alex Williamson,
	Andrew Murray, Jose Abreu, kvm, linux-fbdev, netdev, x86,
	linux-s390
In-Reply-To: <20190816092437.31846-1-efremov@linux.com>

Code that iterates over all standard PCI BARs typically uses
PCI_STD_RESOURCE_END. However, it requires the "unusual" loop condition
"i <= PCI_STD_RESOURCE_END" rather than something more standard like
"i < PCI_STD_NUM_BARS".

This patch adds the definition PCI_STD_NUM_BARS which is equivalent to
"PCI_STD_RESOURCE_END + 1" and updates loop conditions to use it.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/pci/quirks.c          | 2 +-
 include/linux/pci.h           | 2 +-
 include/uapi/linux/pci_regs.h | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 208aacf39329..02bdf3a0231e 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -475,7 +475,7 @@ static void quirk_extend_bar_to_page(struct pci_dev *dev)
 {
 	int i;
 
-	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		struct resource *r = &dev->resource[i];
 
 		if (r->flags & IORESOURCE_MEM && resource_size(r) < PAGE_SIZE) {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9e700d9f9f28..7b9590d5dc2d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -76,7 +76,7 @@ enum pci_mmap_state {
 enum {
 	/* #0-5: standard PCI resources */
 	PCI_STD_RESOURCES,
-	PCI_STD_RESOURCE_END = 5,
+	PCI_STD_RESOURCE_END = PCI_STD_RESOURCES + PCI_STD_NUM_BARS - 1,
 
 	/* #6: expansion ROM resource */
 	PCI_ROM_RESOURCE,
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index f28e562d7ca8..68b571d491eb 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -34,6 +34,7 @@
  * of which the first 64 bytes are standardized as follows:
  */
 #define PCI_STD_HEADER_SIZEOF	64
+#define PCI_STD_NUM_BARS	6	/* Number of standard BARs */
 #define PCI_VENDOR_ID		0x00	/* 16 bits */
 #define PCI_DEVICE_ID		0x02	/* 16 bits */
 #define PCI_COMMAND		0x04	/* 16 bits */
-- 
2.21.0


^ permalink raw reply related

* [PATCH v2 04/10] stmmac: pci: Loop using PCI_STD_NUM_BARS
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, Giuseppe Cavallaro, Alexandre Torgue, netdev,
	linux-pci, linux-kernel
In-Reply-To: <20190816092437.31846-1-efremov@linux.com>

Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
'i <= PCI_STD_RESOURCE_END'.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 86f9c07a38cf..cfe496cdd78b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -258,7 +258,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 	}
 
 	/* Get the base address of device */
-	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (pci_resource_len(pdev, i) == 0)
 			continue;
 		ret = pcim_iomap_regions(pdev, BIT(i), pci_name(pdev));
@@ -296,7 +296,7 @@ static void stmmac_pci_remove(struct pci_dev *pdev)
 
 	stmmac_dvr_remove(&pdev->dev);
 
-	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (pci_resource_len(pdev, i) == 0)
 			continue;
 		pcim_iounmap_regions(pdev, BIT(i));
-- 
2.21.0


^ permalink raw reply related

* [PATCH v2 05/10] net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Denis Efremov, Jose Abreu, netdev, linux-pci, linux-kernel
In-Reply-To: <20190816092437.31846-1-efremov@linux.com>

Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
'i <= PCI_STD_RESOURCE_END'.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c b/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
index 386bafe74c3f..fa8604d7b797 100644
--- a/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
+++ b/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
@@ -34,7 +34,7 @@ static int xlgmac_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
 		return ret;
 	}
 
-	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (pci_resource_len(pcidev, i) == 0)
 			continue;
 		ret = pcim_iomap_regions(pcidev, BIT(i), XLGMAC_DRV_NAME);
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH net-next] r8152: divide the tx and rx bottom functions
From: Eric Dumazet @ 2019-08-16  9:27 UTC (permalink / raw)
  To: Hayes Wang, Eric Dumazet, netdev@vger.kernel.org
  Cc: nic_swsd, linux-kernel@vger.kernel.org
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2F18D47C8@RTITMBSVM03.realtek.com.tw>



On 8/16/19 11:08 AM, Hayes Wang wrote:
> Eric Dumazet [mailto:eric.dumazet@gmail.com]
>> Sent: Friday, August 16, 2019 4:20 PM
> [...]
>> Which callback ?
> 
> The USB device has two endpoints for Tx and Rx.
> If I submit tx or rx URB to the USB host controller,
> the relative callback functions would be called, when
> they are finished. For rx, it is read_bulk_callback.
> For tx, it is write_bulk_callback.
> 
>> After an idle period (no activity, no prior packets being tx-completed ...),
>> a packet is sent by the upper stack, enters the ndo_start_xmit() of a network
>> driver.
>>
>> This driver ndo_start_xmit() simply adds an skb to a local list, and returns.
> 
> Base on the current method (without tasklet), when
> ndo_start_xmit() is called, napi_schedule is called only
> if there is at least one free buffer (!list_empty(&tp->tx_free))
> to transmit the packet. Then, the flow would be as following.

Very uncommon naming conventions really :/


Maybe you would avoid messing with a tasklet (we really try to get rid
of tasklets in general) using two NAPI, one for TX, one for RX.

Some drivers already use two NAPI, it is fine.

This might avoid the ugly dance in r8152_poll(),
calling napi_schedule(napi) after napi_complete_done() !


^ permalink raw reply

* Re: [PATCH] rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ
From: Jian-Hong Pan @ 2019-08-16  9:27 UTC (permalink / raw)
  To: Tony Chuang
  Cc: Kalle Valo, David S . Miller, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux@endlessm.com
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D1892A52@RTITMBSVM04.realtek.com.tw>

Tony Chuang <yhchuang@realtek.com> 於 2019年8月16日 週五 下午4:07寫道:
>
> Hi,
>
> A few more questions below
>
> > > From: Jian-Hong Pan [mailto:jian-hong@endlessm.com]
> > >
> > > There is a mass of jobs between spin lock and unlock in the hardware
> > > IRQ which will occupy much time originally. To make system work more
> > > efficiently, this patch moves the jobs to the soft IRQ (bottom half) to
> > > reduce the time in hardware IRQ.
> > >
> > > Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> > > ---
> > >  drivers/net/wireless/realtek/rtw88/pci.c | 36 +++++++++++++++++++-----
> > >  1 file changed, 29 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
> > > b/drivers/net/wireless/realtek/rtw88/pci.c
> > > index 00ef229552d5..355606b167c6 100644
> > > --- a/drivers/net/wireless/realtek/rtw88/pci.c
> > > +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> > > @@ -866,12 +866,29 @@ static irqreturn_t rtw_pci_interrupt_handler(int
> > irq,
> > > void *dev)
> > >  {
> > >     struct rtw_dev *rtwdev = dev;
> > >     struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
> > > -   u32 irq_status[4];
> > > +   unsigned long flags;
> > >
> > > -   spin_lock(&rtwpci->irq_lock);
> > > +   spin_lock_irqsave(&rtwpci->irq_lock, flags);
>
> I think you can use 'spin_lock()' here as it's in IRQ context?

Ah!  You are right!  The interrupts are already disabled in the
interrupt handler.  So, there is no need to disable more once.  I can
tweak it.

> > >     if (!rtwpci->irq_enabled)
> > >             goto out;
> > >
> > > +   /* disable RTW PCI interrupt to avoid more interrupts before the end of
> > > +    * thread function
> > > +    */
> > > +   rtw_pci_disable_interrupt(rtwdev, rtwpci);
>
>
> Why do we need rtw_pci_disable_interrupt() here.
> Have you done any experiment and decided to add this.
> If you have can you share your results to me?

I got this idea "Avoid back to back interrupt" from Intel WiFi's driver.
https://elixir.bootlin.com/linux/v5.3-rc4/source/drivers/net/wireless/intel/iwlwifi/pcie/rx.c#L2071

So, I disable rtw_pci interrupt here in first half IRQ.  (Re-enable in
bottom half)

>
> > > +out:
> > > +   spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
>
> spin_unlock()
>
> > > +
> > > +   return IRQ_WAKE_THREAD;
> > > +}
> > > +
> > > +static irqreturn_t rtw_pci_interrupt_threadfn(int irq, void *dev)
> > > +{
> > > +   struct rtw_dev *rtwdev = dev;
> > > +   struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
> > > +   unsigned long flags;
> > > +   u32 irq_status[4];
> > > +
> > >     rtw_pci_irq_recognized(rtwdev, rtwpci, irq_status);
> > >
> > >     if (irq_status[0] & IMR_MGNTDOK)
> > > @@ -891,8 +908,11 @@ static irqreturn_t rtw_pci_interrupt_handler(int
> > irq,
> > > void *dev)
> > >     if (irq_status[0] & IMR_ROK)
> > >             rtw_pci_rx_isr(rtwdev, rtwpci, RTW_RX_QUEUE_MPDU);
> > >
> > > -out:
> > > -   spin_unlock(&rtwpci->irq_lock);
> > > +   /* all of the jobs for this interrupt have been done */
> > > +   spin_lock_irqsave(&rtwpci->irq_lock, flags);
> >
> > Shouldn't we protect the ISRs above?
> >
> > This patch could actually reduce the time of IRQ.
> > But I think I need to further test it with PCI MSI interrupt.
> > https://patchwork.kernel.org/patch/11081539/
> >
> > Maybe we could drop the "rtw_pci_[enable/disable]_interrupt" when MSI
> > Is enabled with this patch.
> >
> > > +   if (rtw_flag_check(rtwdev, RTW_FLAG_RUNNING))
> > > +           rtw_pci_enable_interrupt(rtwdev, rtwpci);

Then, re-enable rtw_pci interrupt here in bottom half of the IRQ.
Here is the place where Intel WiFi re-enable interrupts.
https://elixir.bootlin.com/linux/v5.3-rc4/source/drivers/net/wireless/intel/iwlwifi/pcie/rx.c#L1959

Now, we can go back to the question "Shouldn't we protect the ISRs above?"

1. What does the lock: rtwpci->irq_lock protect for?
According to the code, seems only rtw_pci interrupt's state which is
enabled or not.

2. How about the ISRs you mentioned?
This part will only be executed if there is a fresh rtw_pci interrupt.
The first half already disabled rtw_pci interrupt, so there is no more
fresh rtw_pci interrupt until rtw_pci interrupt is enabled again.
Therefor, the rtwpci->irq_lock only wraps the rtw_pci interrupt
enablement.

If there is any more entry that I missed and will interfere, please let me know.

Thank you
Jian-Hong Pan

^ permalink raw reply

* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Jordan Glover @ 2019-08-16  9:34 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andy Lutomirski, Daniel Colascione, Song Liu, Kees Cook,
	Networking, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team,
	Lorenz Bauer, Jann Horn, Greg KH, Linux API, LSM List
In-Reply-To: <20190815230808.2o2qe7a72cwdce2m@ast-mbp.dhcp.thefacebook.com>

On Thursday, August 15, 2019 11:08 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Thu, Aug 15, 2019 at 11:36:43AM -0700, Andy Lutomirski wrote:
>
> > On Thu, Aug 15, 2019 at 10:29 AM Alexei Starovoitov
> > alexei.starovoitov@gmail.com wrote:
> >
> > > On Thu, Aug 15, 2019 at 11:24:54AM +0000, Jordan Glover wrote:
> > >
> > > > systemd --user processes aren't "less privileged". The are COMPLETELY unprivileged.
> > > > Granting them cap_bpf is the same as granting it to every other unprivileged user
> > > > process. Also unprivileged user process can start systemd --user process with any
> > > > command they like.
> > >
> > > systemd itself is trusted. It's the same binary whether it runs as pid=1
> > > or as pid=123. One of the use cases is to make IPAddressDeny= work with --user.
> > > Subset of that feature already works with AmbientCapabilities=CAP_NET_ADMIN.
> > > CAP_BPF is a natural step in the same direction.
> >
> > I have the feeling that we're somehow speaking different languages.
> > What, precisely, do you mean when you say "systemd itself is trusted"?
> > Do you mean "the administrator trusts that the /lib/systemd/systemd
> > binary is not malicious"? Do you mean "the administrator trusts that
> > the running systemd process is not malicious"?
>
> please see
> https://github.com/systemd/systemd/commit/4c1567f29aeb60a6741874bca8a8e3a0bd69ed01
> I'm not advocating for or against this approach.
> Call it 'security hole' or 'better security'.
> There are two categories of people for any feature like this.
> My point that there is a demand to use bpf for non-root and CAP_NET_ADMIN
> level of privileges is acceptable.
> Another option is to relax all of bpf to CAP_NET_ADMIN instead of CAP_SYS_ADMIN.
> But CAP_BPF is clearly better way.
>

Do you realize it's not possible to grant CAP_NET_ADMIN or any other CAP in
"systemd --user" service? Trying to do so will fail with:
"Failed to apply ambient capabilities (before UID change): Operation not permitted"

I think it's crucial to clear that point to avoid confusion in this discussion
where people are talking about different things.

On the other hand running "systemd --system" service with:

User=nobody
AmbientCapabilities=CAP_NET_ADMIN

is perfectly legit and clears some security concerns as only privileged user
can start such service.

Jordan

^ permalink raw reply

* Re: [PATCH v12 3/5] dt-bindings: can: tcan4x5x: Add DT bindings for TCAN4x5X driver
From: Marc Kleine-Budde @ 2019-08-16  9:38 UTC (permalink / raw)
  To: Dan Murphy, wg, davem; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <20190509161109.10499-3-dmurphy@ti.com>


[-- Attachment #1.1: Type: text/plain, Size: 3240 bytes --]

On 5/9/19 6:11 PM, Dan Murphy wrote:
> DT binding documentation for TI TCAN4x5x driver.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v12 - No changes - https://lore.kernel.org/patchwork/patch/1052300/
> 
> v11 - No changes - https://lore.kernel.org/patchwork/patch/1051178/
> v10 - No changes - https://lore.kernel.org/patchwork/patch/1050488/
> v9 - No Changes - https://lore.kernel.org/patchwork/patch/1050118/
> v8 - No Changes - https://lore.kernel.org/patchwork/patch/1047981/
> v7 - Made device state optional - https://lore.kernel.org/patchwork/patch/1047218/
> v6 - No changes - https://lore.kernel.org/patchwork/patch/1042445/
> 
>  .../devicetree/bindings/net/can/tcan4x5x.txt  | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/tcan4x5x.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/tcan4x5x.txt b/Documentation/devicetree/bindings/net/can/tcan4x5x.txt
> new file mode 100644
> index 000000000000..c388f7d9feb1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/tcan4x5x.txt
> @@ -0,0 +1,37 @@
> +Texas Instruments TCAN4x5x CAN Controller
> +================================================
> +
> +This file provides device node information for the TCAN4x5x interface contains.
> +
> +Required properties:
> +	- compatible: "ti,tcan4x5x"
> +	- reg: 0
> +	- #address-cells: 1
> +	- #size-cells: 0
> +	- spi-max-frequency: Maximum frequency of the SPI bus the chip can
> +			     operate at should be less than or equal to 18 MHz.
> +	- data-ready-gpios: Interrupt GPIO for data and error reporting.
> +	- device-wake-gpios: Wake up GPIO to wake up the TCAN device.
> +
> +See Documentation/devicetree/bindings/net/can/m_can.txt for additional
> +required property details.
> +
> +Optional properties:
> +	- reset-gpios: Hardwired output GPIO. If not defined then software
> +		       reset.
> +	- device-state-gpios: Input GPIO that indicates if the device is in
> +			      a sleep state or if the device is active.
> +
> +Example:
> +tcan4x5x: tcan4x5x@0 {
> +		compatible = "ti,tcan4x5x";
> +		reg = <0>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		spi-max-frequency = <10000000>;
> +		bosch,mram-cfg = <0x0 0 0 32 0 0 1 1>;
> +		data-ready-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;

Can you convert this into a proper interrupt property? E.g.:

>                 interrupt-parent = <&gpio4>;
>                 interrupts = <13 0x2>;

See:
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt#L21
https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/tree/drivers/net/can/spi/mcp251x.c?h=mcp251x#n945

> +		device-state-gpios = <&gpio3 21 GPIO_ACTIVE_HIGH>;
> +		device-wake-gpios = <&gpio1 15 GPIO_ACTIVE_HIGH>;
> +		reset-gpios = <&gpio1 27 GPIO_ACTIVE_LOW>;
> +};

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox