* [PATCH net-next 7/7] nfp: abm: refuse RED offload with harddrop set
From: Jakub Kicinski @ 2018-11-09 3:50 UTC (permalink / raw)
To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20181109035039.25823-1-jakub.kicinski@netronome.com>
RED Qdisc will now inform the drivers about the state of the harddrop
flag. Refuse to offload in case harddrop is set.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/qdisc.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index 979afb3ea855..bb05f9ee0401 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -83,6 +83,11 @@ nfp_abm_red_check_params(struct nfp_abm_link *alink,
opt->parent, opt->handle);
return false;
}
+ if (opt->set.is_harddrop) {
+ nfp_warn(cpp, "RED offload failed - harddrop is not supported (p:%08x h:%08x)\n",
+ opt->parent, opt->handle);
+ return false;
+ }
if (opt->set.min != opt->set.max) {
nfp_warn(cpp, "RED offload failed - unsupported min/max parameters (p:%08x h:%08x)\n",
opt->parent, opt->handle);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] net: smsc95xx: Fix MTU range
From: David Miller @ 2018-11-09 3:55 UTC (permalink / raw)
To: stefan.wahren
Cc: steve.glendinning, UNGLinuxDriver, RaghuramChary.Jallipalli,
netdev, linux-usb
In-Reply-To: <1541705906-2662-1-git-send-email-stefan.wahren@i2se.com>
From: Stefan Wahren <stefan.wahren@i2se.com>
Date: Thu, 8 Nov 2018 20:38:26 +0100
> The commit f77f0aee4da4 ("net: use core MTU range checking in USB NIC
> drivers") introduce a common MTU handling for usbnet. But it's missing
> the necessary changes for smsc95xx. So set the MTU range accordingly.
>
> This patch has been tested on a Raspberry Pi 3.
>
> Fixes: f77f0aee4da4 ("net: use core MTU range checking in USB NIC drivers")
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next] sfc: use the new __netdev_tx_sent_queue BQL optimisation
From: David Miller @ 2018-11-09 4:01 UTC (permalink / raw)
To: ecree; +Cc: linux-net-drivers, netdev
In-Reply-To: <31ee2a24-103c-ac0a-9e60-b3204bd61167@solarflare.com>
From: Edward Cree <ecree@solarflare.com>
Date: Thu, 8 Nov 2018 19:47:19 +0000
> As added in 3e59020abf0f ("net: bql: add __netdev_tx_sent_queue()"), which
> see for performance rationale.
>
> Signed-off-by: Edward Cree <ecree@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH 08/20] octeontx2-af: Alloc and config NPC MCAM entry at a time
From: Sunil Kovvuri @ 2018-11-09 4:20 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Netdev List, David S. Miller, linux-soc, Sunil Goutham
In-Reply-To: <CAK8P3a2=CvyXkPexQHH6oTzUccJo=ayNgz0R9Lpsd3-YmNxxEg@mail.gmail.com>
On Fri, Nov 9, 2018 at 2:13 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Nov 8, 2018 at 7:37 PM <sunil.kovvuri@gmail.com> wrote:
> > @@ -666,4 +668,20 @@ struct npc_mcam_unmap_counter_req {
> > u8 all; /* Unmap all entries using this counter ? */
> > };
> >
> > +struct npc_mcam_alloc_and_write_entry_req {
> > + struct mbox_msghdr hdr;
> > + struct mcam_entry entry_data;
> > + u16 ref_entry;
> > + u8 priority; /* Lower or higher w.r.t ref_entry */
> > + u8 intf; /* Rx or Tx interface */
> > + u8 enable_entry;/* Enable this MCAM entry ? */
> > + u8 alloc_cntr; /* Allocate counter and map ? */
> > +};
>
> I noticed that this structure requires padding at the end because
> struct mbox_msghdr has a 32-bit alignment requirement. For
> data structures in an interface, I'd recommend avoiding that kind
> of padding and adding reserved fields or widening the types
> accordingly.
>
When there are multiple messages in the mailbox, each message starts
at a 16byte aligned offset. So struct mbox_msghdr is always aligned.
I think adding reserved fields is not needed here.
===
struct mbox_msghdr *otx2_mbox_alloc_msg_rsp(struct otx2_mbox *mbox, int devid,
int size, int size_rsp)
{
size = ALIGN(size, MBOX_MSG_ALIGN);
===
Is this what you were referring to ?
Sunil.
> I also noticed a similar problem in struct mbox_msghdr. Maybe
> use the 'pahole' tool to check for this kind of padding in the
> API structures.
>
> Arnd
^ permalink raw reply
* [Patch net-next 00/11] add code optimization for VF reset and some new reset feature
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
Currently hardware supports below reset:
1. VF reset: triggered by sending cmd to IMP(Integrated Management
Processor). Only reset specific VF function and do not affect
other PF or VF.
2. PF reset: triggered by sending cmd to IMP. Only reset specific PF
and it's VF.
3. PF FLR: triggered by PCIe subsystem. Only reset specific PF and
it's VF.
4. VF FLR: triggered by PCIe subsystem. Only reset specific VF function
and do not affect other PF or VF.
5. Core reset: triggered by writing to register. Reset most hardware
unit, such as SSU, which affects all the PF and VF.
6. Global reset: triggered by writing to register. Reset all hardware
unit, which affects all the PF and VF.
7. IMP reset: triggered by IMU(Intelligent Management Unit) when
IMP is not longer feeding IMU's watchdog. IMU will reload the IMP
firmware and IMP will perform global reset after firmware reloading,
which affects all the PF and VF.
Current driver only support PF/VF reset, incomplete core and global
reset(lacking the vf reset handling). So this patchset adds complete
reset support in hns3 driver.
Also, this patchset contains some optimization related to reset.
Huazhong Tan (11):
net: hns3: add reset_hdev to reinit the hdev in VF's reset process
net: hns3: adjust VF's reset process
net: hns3: add reset handling for VF when doing PF reset
net: hns3: add reset handling for VF when doing Core/Global/IMP reset
net: hns3: stop handling command queue while resetting VF
net: hns3: add error handler for hclgevf_reset()
net: hns3: stop napi polling when HNS3_NIC_STATE_DOWN is set
net: hns3: implement the IMP reset processing for PF
net: hns3: add PCIe FLR support for PF
net: hns3: do VF's pci re-initialization while PF doing FLR
net: hns3: add PCIe FLR support for VF
drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h | 6 +
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 11 +-
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 66 +++-
drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 6 +-
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c | 11 +-
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 185 +++++++++-
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 11 +
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c | 35 +-
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c | 13 +-
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 404 +++++++++++++++------
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h | 36 +-
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c | 18 +-
12 files changed, 638 insertions(+), 164 deletions(-)
--
2.7.4
^ permalink raw reply
* [Patch net-next 01/11] net: hns3: add reset_hdev to reinit the hdev in VF's reset process
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541772476-41478-1-git-send-email-tanhuazhong@huawei.com>
When doing reset, the reset handling function only need to
reinitialize hardware, it makes sense to add a function to
do that job. Also the error handling of hclgevf_init_hdev is
different when it is used in reset process.
This patch adds reset_hdev to reinitialize hardware when resetting.
Also, this patch removes the hclgevf_dev_ongoing_full_reset because
it is unused now.
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 71 ++++++++++------------
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h | 14 -----
2 files changed, 31 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 29da480..cdeb231 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -10,8 +10,7 @@
#define HCLGEVF_NAME "hclgevf"
-static int hclgevf_init_hdev(struct hclgevf_dev *hdev);
-static void hclgevf_uninit_hdev(struct hclgevf_dev *hdev);
+static int hclgevf_reset_hdev(struct hclgevf_dev *hdev);
static struct hnae3_ae_algo ae_algovf;
static const struct pci_device_id ae_algovf_pci_tbl[] = {
@@ -209,12 +208,6 @@ static int hclgevf_alloc_tqps(struct hclgevf_dev *hdev)
struct hclgevf_tqp *tqp;
int i;
- /* if this is on going reset then we need to re-allocate the TPQs
- * since we cannot assume we would get same number of TPQs back from PF
- */
- if (hclgevf_dev_ongoing_reset(hdev))
- devm_kfree(&hdev->pdev->dev, hdev->htqp);
-
hdev->htqp = devm_kcalloc(&hdev->pdev->dev, hdev->num_tqps,
sizeof(struct hclgevf_tqp), GFP_KERNEL);
if (!hdev->htqp)
@@ -258,12 +251,6 @@ static int hclgevf_knic_setup(struct hclgevf_dev *hdev)
new_tqps = kinfo->rss_size * kinfo->num_tc;
kinfo->num_tqps = min(new_tqps, hdev->num_tqps);
- /* if this is on going reset then we need to re-allocate the hnae queues
- * as well since number of TPQs from PF might have changed.
- */
- if (hclgevf_dev_ongoing_reset(hdev))
- devm_kfree(&hdev->pdev->dev, kinfo->tqp);
-
kinfo->tqp = devm_kcalloc(&hdev->pdev->dev, kinfo->num_tqps,
sizeof(struct hnae3_queue *), GFP_KERNEL);
if (!kinfo->tqp)
@@ -1141,7 +1128,7 @@ static int hclgevf_reset_stack(struct hclgevf_dev *hdev)
hclgevf_notify_client(hdev, HNAE3_UNINIT_CLIENT);
/* re-initialize the hclge device */
- ret = hclgevf_init_hdev(hdev);
+ ret = hclgevf_reset_hdev(hdev);
if (ret) {
dev_err(&hdev->pdev->dev,
"hclge device re-init failed, VF is disabled!\n");
@@ -1615,10 +1602,6 @@ static void hclgevf_ae_stop(struct hnae3_handle *handle)
static void hclgevf_state_init(struct hclgevf_dev *hdev)
{
- /* if this is on going reset then skip this initialization */
- if (hclgevf_dev_ongoing_reset(hdev))
- return;
-
/* setup tasks for the MBX */
INIT_WORK(&hdev->mbx_service_task, hclgevf_mailbox_service_task);
clear_bit(HCLGEVF_STATE_MBX_SERVICE_SCHED, &hdev->state);
@@ -1660,10 +1643,6 @@ static int hclgevf_init_msi(struct hclgevf_dev *hdev)
int vectors;
int i;
- /* if this is on going reset then skip this initialization */
- if (hclgevf_dev_ongoing_reset(hdev))
- return 0;
-
if (hnae3_get_bit(hdev->ae_dev->flag, HNAE3_DEV_SUPPORT_ROCE_B))
vectors = pci_alloc_irq_vectors(pdev,
hdev->roce_base_msix_offset + 1,
@@ -1720,10 +1699,6 @@ static int hclgevf_misc_irq_init(struct hclgevf_dev *hdev)
{
int ret = 0;
- /* if this is on going reset then skip this initialization */
- if (hclgevf_dev_ongoing_reset(hdev))
- return 0;
-
hclgevf_get_misc_vector(hdev);
ret = request_irq(hdev->misc_vector.vector_irq, hclgevf_misc_irq_handle,
@@ -1853,14 +1828,6 @@ static int hclgevf_pci_init(struct hclgevf_dev *hdev)
struct hclgevf_hw *hw;
int ret;
- /* check if we need to skip initialization of pci. This will happen if
- * device is undergoing VF reset. Otherwise, we would need to
- * re-initialize pci interface again i.e. when device is not going
- * through *any* reset or actually undergoing full reset.
- */
- if (hclgevf_dev_ongoing_reset(hdev))
- return 0;
-
ret = pci_enable_device(pdev);
if (ret) {
dev_err(&pdev->dev, "failed to enable PCI device\n");
@@ -1949,17 +1916,41 @@ static int hclgevf_query_vf_resource(struct hclgevf_dev *hdev)
return 0;
}
-static int hclgevf_init_hdev(struct hclgevf_dev *hdev)
+static int hclgevf_reset_hdev(struct hclgevf_dev *hdev)
{
struct pci_dev *pdev = hdev->pdev;
int ret;
- /* check if device is on-going full reset(i.e. pcie as well) */
- if (hclgevf_dev_ongoing_full_reset(hdev)) {
- dev_warn(&pdev->dev, "device is going full reset\n");
- hclgevf_uninit_hdev(hdev);
+ ret = hclgevf_cmd_init(hdev);
+ if (ret) {
+ dev_err(&pdev->dev, "cmd failed %d\n", ret);
+ return ret;
+ }
+
+ ret = hclgevf_rss_init_hw(hdev);
+ if (ret) {
+ dev_err(&hdev->pdev->dev,
+ "failed(%d) to initialize RSS\n", ret);
+ return ret;
+ }
+
+ ret = hclgevf_init_vlan_config(hdev);
+ if (ret) {
+ dev_err(&hdev->pdev->dev,
+ "failed(%d) to initialize VLAN config\n", ret);
+ return ret;
}
+ dev_info(&hdev->pdev->dev, "Reset done\n");
+
+ return 0;
+}
+
+static int hclgevf_init_hdev(struct hclgevf_dev *hdev)
+{
+ struct pci_dev *pdev = hdev->pdev;
+ int ret;
+
ret = hclgevf_pci_init(hdev);
if (ret) {
dev_err(&pdev->dev, "PCI initialization failed\n");
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
index 0ea4c9b..29ae4b6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
@@ -196,20 +196,6 @@ struct hclgevf_dev {
u32 flag;
};
-static inline bool hclgevf_dev_ongoing_reset(struct hclgevf_dev *hdev)
-{
- return (hdev &&
- (test_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state)) &&
- (hdev->reset_level == HNAE3_VF_RESET));
-}
-
-static inline bool hclgevf_dev_ongoing_full_reset(struct hclgevf_dev *hdev)
-{
- return (hdev &&
- (test_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state)) &&
- (hdev->reset_level == HNAE3_VF_FULL_RESET));
-}
-
int hclgevf_send_mbx_msg(struct hclgevf_dev *hdev, u16 code, u16 subcode,
const u8 *msg_data, u8 msg_len, bool need_resp,
u8 *resp_data, u16 resp_len);
--
2.7.4
^ permalink raw reply related
* [Patch net-next 02/11] net: hns3: adjust VF's reset process
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541772476-41478-1-git-send-email-tanhuazhong@huawei.com>
Currently when VF need to reset itself, it will send a cmd to PF,
after receiving the VF reset requset, PF sends a cmd to inform
VF to enter the reset process and send a cmd to firmware to do the
actual reset for the VF, it is possible that firmware has resetted
the VF, but VF has not entered the reset process, which may cause
IO not stopped problem when firmware is resetting VF.
This patch fixes it by adjusting the VF reset process, when VF
need to reset itself, it will enter the reset process first, and
it will tell the PF to send cmd to firmware to reset itself.
Add member reset_pending to struct hclgevf_dev, which indicates that
there is reset event need to be processed by the VF's reset task, and
the VF's reset task chooses the highest-level one and clears other
low-level one when it processes reset_pending.
hclge_inform_reset_assert_to_vf function is unused now, but it will
be used to support the PF reset with VF working, so declare it in
the header file.
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 1 +
drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 3 +-
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 1 +
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c | 22 ++----
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 80 ++++++++++++++--------
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h | 2 +
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c | 2 +-
7 files changed, 63 insertions(+), 48 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 5b3b104..510dd08 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -124,6 +124,7 @@ enum hnae3_reset_notify_type {
enum hnae3_reset_type {
HNAE3_VF_RESET,
+ HNAE3_VF_FUNC_RESET,
HNAE3_VF_FULL_RESET,
HNAE3_FUNC_RESET,
HNAE3_CORE_RESET,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index cfd6a71..5a38226 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -593,7 +593,8 @@ static inline void hns3_write_reg(void __iomem *base, u32 reg, u32 value)
static inline bool hns3_dev_ongoing_func_reset(struct hnae3_ae_dev *ae_dev)
{
- return (ae_dev && (ae_dev->reset_type == HNAE3_FUNC_RESET));
+ return (ae_dev && (ae_dev->reset_type == HNAE3_FUNC_RESET ||
+ ae_dev->reset_type == HNAE3_VF_FUNC_RESET));
}
#define hns3_read_dev(a, reg) \
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index 36f3413..6df4cfe 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -775,6 +775,7 @@ static inline int hclge_get_queue_id(struct hnae3_queue *queue)
return tqp->index;
}
+int hclge_inform_reset_assert_to_vf(struct hclge_vport *vport);
int hclge_cfg_mac_speed_dup(struct hclge_dev *hdev, int speed, u8 duplex);
int hclge_set_vlan_filter(struct hnae3_handle *handle, __be16 proto,
u16 vlan_id, bool is_kill);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
index f890022..013bc7a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
@@ -79,7 +79,7 @@ static int hclge_send_mbx_msg(struct hclge_vport *vport, u8 *msg, u16 msg_len,
return status;
}
-static int hclge_inform_reset_assert_to_vf(struct hclge_vport *vport)
+int hclge_inform_reset_assert_to_vf(struct hclge_vport *vport)
{
u8 msg_data[2];
u8 dest_vfid;
@@ -363,24 +363,10 @@ static void hclge_reset_vf(struct hclge_vport *vport,
int ret;
dev_warn(&hdev->pdev->dev, "PF received VF reset request from VF %d!",
- mbx_req->mbx_src_vfid);
-
- /* Acknowledge VF that PF is now about to assert the reset for the VF.
- * On receiving this message VF will get into pending state and will
- * start polling for the hardware reset completion status.
- */
- ret = hclge_inform_reset_assert_to_vf(vport);
- if (ret) {
- dev_err(&hdev->pdev->dev,
- "PF fail(%d) to inform VF(%d)of reset, reset failed!\n",
- ret, vport->vport_id);
- return;
- }
+ vport->vport_id);
- dev_warn(&hdev->pdev->dev, "PF is now resetting VF %d.\n",
- mbx_req->mbx_src_vfid);
- /* reset this virtual function */
- hclge_func_reset_cmd(hdev, mbx_req->mbx_src_vfid);
+ ret = hclge_func_reset_cmd(hdev, vport->vport_id);
+ hclge_gen_resp_to_vf(vport, mbx_req, ret, NULL, 0);
}
static bool hclge_cmd_crq_empty(struct hclge_hw *hw)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index cdeb231..7a1462a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -855,6 +855,9 @@ static int hclgevf_unmap_ring_from_vector(
struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
int ret, vector_id;
+ if (test_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state))
+ return 0;
+
vector_id = hclgevf_get_vector_index(hdev, vector);
if (vector_id < 0) {
dev_err(&handle->pdev->dev,
@@ -1141,10 +1144,34 @@ static int hclgevf_reset_stack(struct hclgevf_dev *hdev)
return 0;
}
+static int hclgevf_reset_prepare_wait(struct hclgevf_dev *hdev)
+{
+ int ret = 0;
+
+ switch (hdev->reset_type) {
+ case HNAE3_VF_FUNC_RESET:
+ ret = hclgevf_send_mbx_msg(hdev, HCLGE_MBX_RESET, 0, NULL,
+ 0, true, NULL, sizeof(u8));
+ break;
+ default:
+ break;
+ }
+
+ dev_info(&hdev->pdev->dev, "prepare reset(%d) wait done, ret:%d\n",
+ hdev->reset_type, ret);
+
+ return ret;
+}
+
static int hclgevf_reset(struct hclgevf_dev *hdev)
{
+ struct hnae3_ae_dev *ae_dev = pci_get_drvdata(hdev->pdev);
int ret;
+ /* Initialize ae_dev reset status as well, in case enet layer wants to
+ * know if device is undergoing reset
+ */
+ ae_dev->reset_type = hdev->reset_type;
hdev->reset_count++;
rtnl_lock();
@@ -1153,6 +1180,8 @@ static int hclgevf_reset(struct hclgevf_dev *hdev)
rtnl_unlock();
+ hclgevf_reset_prepare_wait(hdev);
+
/* check if VF could successfully fetch the hardware reset completion
* status from the hardware
*/
@@ -1186,28 +1215,19 @@ static int hclgevf_reset(struct hclgevf_dev *hdev)
return ret;
}
-static int hclgevf_do_reset(struct hclgevf_dev *hdev)
-{
- int status;
- u8 respmsg;
-
- status = hclgevf_send_mbx_msg(hdev, HCLGE_MBX_RESET, 0, NULL,
- 0, false, &respmsg, sizeof(u8));
- if (status)
- dev_err(&hdev->pdev->dev,
- "VF reset request to PF failed(=%d)\n", status);
-
- return status;
-}
-
static enum hnae3_reset_type hclgevf_get_reset_level(struct hclgevf_dev *hdev,
unsigned long *addr)
{
enum hnae3_reset_type rst_level = HNAE3_NONE_RESET;
- if (test_bit(HNAE3_VF_RESET, addr)) {
- rst_level = HNAE3_VF_RESET;
- clear_bit(HNAE3_VF_RESET, addr);
+ /* return the highest priority reset level amongst all */
+ if (test_bit(HNAE3_VF_FULL_RESET, addr)) {
+ rst_level = HNAE3_VF_FULL_RESET;
+ clear_bit(HNAE3_VF_FULL_RESET, addr);
+ clear_bit(HNAE3_VF_FUNC_RESET, addr);
+ } else if (test_bit(HNAE3_VF_FUNC_RESET, addr)) {
+ rst_level = HNAE3_VF_FUNC_RESET;
+ clear_bit(HNAE3_VF_FUNC_RESET, addr);
}
return rst_level;
@@ -1225,7 +1245,7 @@ static void hclgevf_reset_event(struct pci_dev *pdev,
hclgevf_get_reset_level(hdev,
&hdev->default_reset_request);
else
- hdev->reset_level = HNAE3_VF_RESET;
+ hdev->reset_level = HNAE3_VF_FUNC_RESET;
/* reset of this VF requested */
set_bit(HCLGEVF_RESET_REQUESTED, &hdev->reset_state);
@@ -1328,9 +1348,15 @@ static void hclgevf_reset_service_task(struct work_struct *work)
*/
hdev->reset_attempts = 0;
- ret = hclgevf_reset(hdev);
- if (ret)
- dev_err(&hdev->pdev->dev, "VF stack reset failed.\n");
+ hdev->last_reset_time = jiffies;
+ while ((hdev->reset_type =
+ hclgevf_get_reset_level(hdev, &hdev->reset_pending))
+ != HNAE3_NONE_RESET) {
+ ret = hclgevf_reset(hdev);
+ if (ret)
+ dev_err(&hdev->pdev->dev,
+ "VF stack reset failed %d.\n", ret);
+ }
} else if (test_and_clear_bit(HCLGEVF_RESET_REQUESTED,
&hdev->reset_state)) {
/* we could be here when either of below happens:
@@ -1359,19 +1385,17 @@ static void hclgevf_reset_service_task(struct work_struct *work)
*/
if (hdev->reset_attempts > 3) {
/* prepare for full reset of stack + pcie interface */
- hdev->reset_level = HNAE3_VF_FULL_RESET;
+ set_bit(HNAE3_VF_FULL_RESET, &hdev->reset_pending);
/* "defer" schedule the reset task again */
set_bit(HCLGEVF_RESET_PENDING, &hdev->reset_state);
} else {
hdev->reset_attempts++;
- /* request PF for resetting this VF via mailbox */
- ret = hclgevf_do_reset(hdev);
- if (ret)
- dev_warn(&hdev->pdev->dev,
- "VF rst fail, stack will call\n");
+ set_bit(hdev->reset_level, &hdev->reset_pending);
+ set_bit(HCLGEVF_RESET_PENDING, &hdev->reset_state);
}
+ hclgevf_reset_task_schedule(hdev);
}
clear_bit(HCLGEVF_STATE_RST_HANDLING, &hdev->state);
@@ -1982,7 +2006,7 @@ static int hclgevf_init_hdev(struct hclgevf_dev *hdev)
}
hclgevf_state_init(hdev);
- hdev->reset_level = HNAE3_VF_RESET;
+ hdev->reset_level = HNAE3_VF_FUNC_RESET;
ret = hclgevf_misc_irq_init(hdev);
if (ret) {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
index 29ae4b6..e6abc4e 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
@@ -148,6 +148,8 @@ struct hclgevf_dev {
unsigned long default_reset_request;
unsigned long last_reset_time;
enum hnae3_reset_type reset_level;
+ unsigned long reset_pending;
+ enum hnae3_reset_type reset_type;
#define HCLGEVF_RESET_REQUESTED 0
#define HCLGEVF_RESET_PENDING 1
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
index 558e3b8..d3519f7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
@@ -267,7 +267,7 @@ void hclgevf_mbx_async_handler(struct hclgevf_dev *hdev)
* has been completely reset. After this stack should
* eventually be re-initialized.
*/
- hdev->reset_level = HNAE3_VF_RESET;
+ set_bit(HNAE3_VF_FULL_RESET, &hdev->reset_pending);
set_bit(HCLGEVF_RESET_PENDING, &hdev->reset_state);
hclgevf_reset_task_schedule(hdev);
--
2.7.4
^ permalink raw reply related
* [Patch net-next 04/11] net: hns3: add reset handling for VF when doing Core/Global/IMP reset
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541772476-41478-1-git-send-email-tanhuazhong@huawei.com>
When a Core/Global/IMP reset occurs, the hardware sets the reset status
register of all PF/VF and reports a reset interrupt to all PF/VF and
firmware.
When receiving the reset interrupt:
1. The firmware will wait for 100 ms before resetting the hardware and
clear the reset status register of all PF when hardware reset is done.
2. The PF/VF driver needs to down the netdev within 100 ms and then wait
for hardware reset to finish.
3. After firmware clearing the reset status register of all PF, the PF
driver reinitializes the hardware and clear the reset status register
of it's VF.
4. After PF driver clearing the reset status register of VF, the VF driver
reinitializes the hardware.
This patch mainly add handling for the step 4.
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 50 ++++++++++++++++------
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h | 8 ++++
2 files changed, 46 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 9405517..fe97cc7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1226,7 +1226,12 @@ static enum hnae3_reset_type hclgevf_get_reset_level(struct hclgevf_dev *hdev,
enum hnae3_reset_type rst_level = HNAE3_NONE_RESET;
/* return the highest priority reset level amongst all */
- if (test_bit(HNAE3_VF_FULL_RESET, addr)) {
+ if (test_bit(HNAE3_VF_RESET, addr)) {
+ rst_level = HNAE3_VF_RESET;
+ clear_bit(HNAE3_VF_RESET, addr);
+ clear_bit(HNAE3_VF_PF_FUNC_RESET, addr);
+ clear_bit(HNAE3_VF_FUNC_RESET, addr);
+ } else if (test_bit(HNAE3_VF_FULL_RESET, addr)) {
rst_level = HNAE3_VF_FULL_RESET;
clear_bit(HNAE3_VF_FULL_RESET, addr);
clear_bit(HNAE3_VF_FUNC_RESET, addr);
@@ -1447,24 +1452,36 @@ static void hclgevf_clear_event_cause(struct hclgevf_dev *hdev, u32 regclr)
hclgevf_write_dev(&hdev->hw, HCLGEVF_VECTOR0_CMDQ_SRC_REG, regclr);
}
-static bool hclgevf_check_event_cause(struct hclgevf_dev *hdev, u32 *clearval)
+static enum hclgevf_evt_cause hclgevf_check_evt_cause(struct hclgevf_dev *hdev,
+ u32 *clearval)
{
- u32 cmdq_src_reg;
+ u32 cmdq_src_reg, rst_ing_reg;
/* fetch the events from their corresponding regs */
cmdq_src_reg = hclgevf_read_dev(&hdev->hw,
HCLGEVF_VECTOR0_CMDQ_SRC_REG);
+ if (BIT(HCLGEVF_VECTOR0_RST_INT_B) & cmdq_src_reg) {
+ rst_ing_reg = hclgevf_read_dev(&hdev->hw, HCLGEVF_RST_ING);
+ dev_info(&hdev->pdev->dev,
+ "receive reset interrupt 0x%x!\n", rst_ing_reg);
+ set_bit(HNAE3_VF_RESET, &hdev->reset_pending);
+ set_bit(HCLGEVF_RESET_PENDING, &hdev->reset_state);
+ cmdq_src_reg &= ~BIT(HCLGEVF_VECTOR0_RST_INT_B);
+ *clearval = cmdq_src_reg;
+ return HCLGEVF_VECTOR0_EVENT_RST;
+ }
+
/* check for vector0 mailbox(=CMDQ RX) event source */
if (BIT(HCLGEVF_VECTOR0_RX_CMDQ_INT_B) & cmdq_src_reg) {
cmdq_src_reg &= ~BIT(HCLGEVF_VECTOR0_RX_CMDQ_INT_B);
*clearval = cmdq_src_reg;
- return true;
+ return HCLGEVF_VECTOR0_EVENT_MBX;
}
dev_dbg(&hdev->pdev->dev, "vector 0 interrupt from unknown source\n");
- return false;
+ return HCLGEVF_VECTOR0_EVENT_OTHER;
}
static void hclgevf_enable_vector(struct hclgevf_misc_vector *vector, bool en)
@@ -1474,19 +1491,28 @@ static void hclgevf_enable_vector(struct hclgevf_misc_vector *vector, bool en)
static irqreturn_t hclgevf_misc_irq_handle(int irq, void *data)
{
+ enum hclgevf_evt_cause event_cause;
struct hclgevf_dev *hdev = data;
u32 clearval;
hclgevf_enable_vector(&hdev->misc_vector, false);
- if (!hclgevf_check_event_cause(hdev, &clearval))
- goto skip_sched;
-
- hclgevf_mbx_handler(hdev);
+ event_cause = hclgevf_check_evt_cause(hdev, &clearval);
- hclgevf_clear_event_cause(hdev, clearval);
+ switch (event_cause) {
+ case HCLGEVF_VECTOR0_EVENT_RST:
+ hclgevf_reset_task_schedule(hdev);
+ break;
+ case HCLGEVF_VECTOR0_EVENT_MBX:
+ hclgevf_mbx_handler(hdev);
+ break;
+ default:
+ break;
+ }
-skip_sched:
- hclgevf_enable_vector(&hdev->misc_vector, true);
+ if (event_cause != HCLGEVF_VECTOR0_EVENT_OTHER) {
+ hclgevf_clear_event_cause(hdev, clearval);
+ hclgevf_enable_vector(&hdev->misc_vector, true);
+ }
return IRQ_HANDLED;
}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
index 2735414..e37897d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
@@ -31,6 +31,8 @@
#define HCLGEVF_VECTOR0_CMDQ_SRC_REG 0x27100
/* CMDQ register bits for RX event(=MBX event) */
#define HCLGEVF_VECTOR0_RX_CMDQ_INT_B 1
+/* RST register bits for RESET event */
+#define HCLGEVF_VECTOR0_RST_INT_B 2
#define HCLGEVF_TQP_RESET_TRY_TIMES 10
/* Reset related Registers */
@@ -60,6 +62,12 @@
#define HCLGEVF_S_IP_BIT BIT(3)
#define HCLGEVF_V_TAG_BIT BIT(4)
+enum hclgevf_evt_cause {
+ HCLGEVF_VECTOR0_EVENT_RST,
+ HCLGEVF_VECTOR0_EVENT_MBX,
+ HCLGEVF_VECTOR0_EVENT_OTHER,
+};
+
/* states of hclgevf device & tasks */
enum hclgevf_states {
/* device states */
--
2.7.4
^ permalink raw reply related
* [Patch net-next 05/11] net: hns3: stop handling command queue while resetting VF
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541772476-41478-1-git-send-email-tanhuazhong@huawei.com>
According to hardware's description, after the reset occurs, the driver
needs to re-initialize the command queue before sending and receiving
any commands. Therefore, the VF's driver needs to identify the command
queue needs to re-initialize with HCLGEVF_STATE_CMD_DISABLE, and does
not allow sending or receiving commands before the re-initialization.
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c | 13 ++++++++++++-
drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 3 +++
drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h | 6 ++++++
drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c | 14 ++++++++++++++
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c
index b917acf..d5765c8 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c
@@ -189,7 +189,8 @@ int hclgevf_cmd_send(struct hclgevf_hw *hw, struct hclgevf_desc *desc, int num)
spin_lock_bh(&hw->cmq.csq.lock);
- if (num > hclgevf_ring_space(&hw->cmq.csq)) {
+ if (num > hclgevf_ring_space(&hw->cmq.csq) ||
+ test_bit(HCLGEVF_STATE_CMD_DISABLE, &hdev->state)) {
spin_unlock_bh(&hw->cmq.csq.lock);
return -EBUSY;
}
@@ -338,6 +339,16 @@ int hclgevf_cmd_init(struct hclgevf_dev *hdev)
spin_unlock_bh(&hdev->hw.cmq.crq.lock);
spin_unlock_bh(&hdev->hw.cmq.csq.lock);
+ clear_bit(HCLGEVF_STATE_CMD_DISABLE, &hdev->state);
+
+ /* Check if there is new reset pending, because the higher level
+ * reset may happen when lower level reset is being processed.
+ */
+ if (hclgevf_is_reset_pending(hdev)) {
+ set_bit(HCLGEVF_STATE_CMD_DISABLE, &hdev->state);
+ return -EBUSY;
+ }
+
/* get firmware version */
ret = hclgevf_cmd_query_firmware_version(&hdev->hw, &version);
if (ret) {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index fe97cc7..5da9053 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1162,6 +1162,8 @@ static int hclgevf_reset_prepare_wait(struct hclgevf_dev *hdev)
break;
}
+ set_bit(HCLGEVF_STATE_CMD_DISABLE, &hdev->state);
+
dev_info(&hdev->pdev->dev, "prepare reset(%d) wait done, ret:%d\n",
hdev->reset_type, ret);
@@ -1467,6 +1469,7 @@ static enum hclgevf_evt_cause hclgevf_check_evt_cause(struct hclgevf_dev *hdev,
"receive reset interrupt 0x%x!\n", rst_ing_reg);
set_bit(HNAE3_VF_RESET, &hdev->reset_pending);
set_bit(HCLGEVF_RESET_PENDING, &hdev->reset_state);
+ set_bit(HCLGEVF_STATE_CMD_DISABLE, &hdev->state);
cmdq_src_reg &= ~BIT(HCLGEVF_VECTOR0_RST_INT_B);
*clearval = cmdq_src_reg;
return HCLGEVF_VECTOR0_EVENT_RST;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
index e37897d..ffe3d49 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
@@ -79,6 +79,7 @@ enum hclgevf_states {
HCLGEVF_STATE_RST_HANDLING,
HCLGEVF_STATE_MBX_SERVICE_SCHED,
HCLGEVF_STATE_MBX_HANDLING,
+ HCLGEVF_STATE_CMD_DISABLE,
};
#define HCLGEVF_MPF_ENBALE 1
@@ -212,6 +213,11 @@ struct hclgevf_dev {
u32 flag;
};
+static inline bool hclgevf_is_reset_pending(struct hclgevf_dev *hdev)
+{
+ return !!hdev->reset_pending;
+}
+
int hclgevf_send_mbx_msg(struct hclgevf_dev *hdev, u16 code, u16 subcode,
const u8 *msg_data, u8 msg_len, bool need_resp,
u8 *resp_data, u16 resp_len);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
index 01a028a..ef9c8e6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
@@ -40,6 +40,9 @@ static int hclgevf_get_mbx_resp(struct hclgevf_dev *hdev, u16 code0, u16 code1,
}
while ((!hdev->mbx_resp.received_resp) && (i < HCLGEVF_MAX_TRY_TIMES)) {
+ if (test_bit(HCLGEVF_STATE_CMD_DISABLE, &hdev->state))
+ return -EIO;
+
udelay(HCLGEVF_SLEEP_USCOEND);
i++;
}
@@ -148,6 +151,11 @@ void hclgevf_mbx_handler(struct hclgevf_dev *hdev)
crq = &hdev->hw.cmq.crq;
while (!hclgevf_cmd_crq_empty(&hdev->hw)) {
+ if (test_bit(HCLGEVF_STATE_CMD_DISABLE, &hdev->state)) {
+ dev_info(&hdev->pdev->dev, "vf crq need init\n");
+ return;
+ }
+
desc = &crq->desc[crq->next_to_use];
req = (struct hclge_mbx_pf_to_vf_cmd *)desc->data;
@@ -249,6 +257,12 @@ void hclgevf_mbx_async_handler(struct hclgevf_dev *hdev)
/* process all the async queue messages */
while (tail != hdev->arq.head) {
+ if (test_bit(HCLGEVF_STATE_CMD_DISABLE, &hdev->state)) {
+ dev_info(&hdev->pdev->dev,
+ "vf crq need init in async\n");
+ return;
+ }
+
msg_q = hdev->arq.msg_q[hdev->arq.head];
switch (msg_q[0]) {
--
2.7.4
^ permalink raw reply related
* [Patch net-next 06/11] net: hns3: add error handler for hclgevf_reset()
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541772476-41478-1-git-send-email-tanhuazhong@huawei.com>
Since hclgevf_reset() may fail for some reasons, so it needs an error
handler to deal with it. When VF reset failed, VF can only be restored
by a higher level reset asserted by PF. So, it needs to reinitialize
its command queue, then it can respond to higher level reset.
Also, this patch adds error logging in the hclgevf_notify_client().
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 51 ++++++++++++++++------
1 file changed, 37 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 5da9053..4dfac6c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1086,11 +1086,17 @@ static int hclgevf_notify_client(struct hclgevf_dev *hdev,
{
struct hnae3_client *client = hdev->nic_client;
struct hnae3_handle *handle = &hdev->nic;
+ int ret;
if (!client->ops->reset_notify)
return -EOPNOTSUPP;
- return client->ops->reset_notify(handle, type);
+ ret = client->ops->reset_notify(handle, type);
+ if (ret)
+ dev_err(&hdev->pdev->dev, "notify nic client failed %d(%d)\n",
+ type, ret);
+
+ return ret;
}
static int hclgevf_reset_wait(struct hclgevf_dev *hdev)
@@ -1133,7 +1139,9 @@ static int hclgevf_reset_stack(struct hclgevf_dev *hdev)
int ret;
/* uninitialize the nic client */
- hclgevf_notify_client(hdev, HNAE3_UNINIT_CLIENT);
+ ret = hclgevf_notify_client(hdev, HNAE3_UNINIT_CLIENT);
+ if (ret)
+ return ret;
/* re-initialize the hclge device */
ret = hclgevf_reset_hdev(hdev);
@@ -1144,7 +1152,9 @@ static int hclgevf_reset_stack(struct hclgevf_dev *hdev)
}
/* bring up the nic client again */
- hclgevf_notify_client(hdev, HNAE3_INIT_CLIENT);
+ ret = hclgevf_notify_client(hdev, HNAE3_INIT_CLIENT);
+ if (ret)
+ return ret;
return 0;
}
@@ -1183,11 +1193,15 @@ static int hclgevf_reset(struct hclgevf_dev *hdev)
rtnl_lock();
/* bring down the nic to stop any ongoing TX/RX */
- hclgevf_notify_client(hdev, HNAE3_DOWN_CLIENT);
+ ret = hclgevf_notify_client(hdev, HNAE3_DOWN_CLIENT);
+ if (ret)
+ goto err_reset_lock;
rtnl_unlock();
- hclgevf_reset_prepare_wait(hdev);
+ ret = hclgevf_reset_prepare_wait(hdev);
+ if (ret)
+ goto err_reset;
/* check if VF could successfully fetch the hardware reset completion
* status from the hardware
@@ -1198,28 +1212,37 @@ static int hclgevf_reset(struct hclgevf_dev *hdev)
dev_err(&hdev->pdev->dev,
"VF failed(=%d) to fetch H/W reset completion status\n",
ret);
-
- dev_warn(&hdev->pdev->dev, "VF reset failed, disabling VF!\n");
- rtnl_lock();
- hclgevf_notify_client(hdev, HNAE3_UNINIT_CLIENT);
-
- rtnl_unlock();
- return ret;
+ goto err_reset;
}
rtnl_lock();
/* now, re-initialize the nic client and ae device*/
ret = hclgevf_reset_stack(hdev);
- if (ret)
+ if (ret) {
dev_err(&hdev->pdev->dev, "failed to reset VF stack\n");
+ goto err_reset_lock;
+ }
/* bring up the nic to enable TX/RX again */
- hclgevf_notify_client(hdev, HNAE3_UP_CLIENT);
+ ret = hclgevf_notify_client(hdev, HNAE3_UP_CLIENT);
+ if (ret)
+ goto err_reset_lock;
rtnl_unlock();
return ret;
+err_reset_lock:
+ rtnl_unlock();
+err_reset:
+ /* When VF reset failed, only the higher level reset asserted by PF
+ * can restore it, so re-initialize the command queue to receive
+ * this higher reset event.
+ */
+ hclgevf_cmd_init(hdev);
+ dev_err(&hdev->pdev->dev, "failed to reset VF\n");
+
+ return ret;
}
static enum hnae3_reset_type hclgevf_get_reset_level(struct hclgevf_dev *hdev,
--
2.7.4
^ permalink raw reply related
* [Patch net-next 07/11] net: hns3: stop napi polling when HNS3_NIC_STATE_DOWN is set
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541772476-41478-1-git-send-email-tanhuazhong@huawei.com>
When calling napi_disable during reset down process, if NAPIF_STATE_MISSED
is set, napi_complete will call __napi_schedule to do the polling again.
So this patch uses HNS3_NIC_STATE_DOWN to ensure the polling is not
scheduled again.
Also, when napi_complete returns true, it means polling is scheduled
again, it is not neccssary to enable the interrupt.
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index a42c8e5..0cd397c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -415,9 +415,6 @@ static void hns3_nic_net_down(struct net_device *netdev)
const struct hnae3_ae_ops *ops;
int i;
- if (test_and_set_bit(HNS3_NIC_STATE_DOWN, &priv->state))
- return;
-
/* disable vectors */
for (i = 0; i < priv->vector_num; i++)
hns3_vector_disable(&priv->tqp_vector[i]);
@@ -439,6 +436,11 @@ static void hns3_nic_net_down(struct net_device *netdev)
static int hns3_nic_net_stop(struct net_device *netdev)
{
+ struct hns3_nic_priv *priv = netdev_priv(netdev);
+
+ if (test_and_set_bit(HNS3_NIC_STATE_DOWN, &priv->state))
+ return 0;
+
netif_tx_stop_all_queues(netdev);
netif_carrier_off(netdev);
@@ -2699,6 +2701,7 @@ static void hns3_update_new_int_gl(struct hns3_enet_tqp_vector *tqp_vector)
static int hns3_nic_common_poll(struct napi_struct *napi, int budget)
{
+ struct hns3_nic_priv *priv = netdev_priv(napi->dev);
struct hns3_enet_ring *ring;
int rx_pkt_total = 0;
@@ -2707,6 +2710,11 @@ static int hns3_nic_common_poll(struct napi_struct *napi, int budget)
bool clean_complete = true;
int rx_budget;
+ if (unlikely(test_bit(HNS3_NIC_STATE_DOWN, &priv->state))) {
+ napi_complete(napi);
+ return 0;
+ }
+
/* Since the actual Tx work is minimal, we can give the Tx a larger
* budget and be more aggressive about cleaning up the Tx descriptors.
*/
@@ -2731,9 +2739,11 @@ static int hns3_nic_common_poll(struct napi_struct *napi, int budget)
if (!clean_complete)
return budget;
- napi_complete(napi);
- hns3_update_new_int_gl(tqp_vector);
- hns3_mask_vector_irq(tqp_vector, 1);
+ if (likely(!test_bit(HNS3_NIC_STATE_DOWN, &priv->state)) &&
+ napi_complete(napi)) {
+ hns3_update_new_int_gl(tqp_vector);
+ hns3_mask_vector_irq(tqp_vector, 1);
+ }
return rx_pkt_total;
}
--
2.7.4
^ permalink raw reply related
* [Patch net-next 08/11] net: hns3: implement the IMP reset processing for PF
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541772476-41478-1-git-send-email-tanhuazhong@huawei.com>
The current code only print the prompt message after receiving
the IMP reset interrupt and does not perform the corresponding driver
reset operation. This patch implements the missing IMP reset handling
in the driver.
1. The driver sets the HCLGE_STATE_CMD_DISABLE to stop sending command
after receiving the IMP reset interrupt.
2. The driver needs to notify the hardware to reload the IMP firmware.
3. The IMP firmware reloading makes the reset time of hardware longer,
so it is necessary to extend the driver's waiting time to wait for
the hardware reset to complete.
4. In hclge_check_event_cause, IMP reset event should have higher
priority than other events.
Also, after clearing HCLGE_STATE_CMD_DISABLE in the hclge_cmd_init(),
it needs to check whether there is a pending reset, if so, just set
the HCLGE_STATE_CMD_DISABLE back and return.
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c | 11 ++++++++-
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 27 +++++++++++++++-------
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 8 +++++++
3 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
index 690f62e..8af0cef 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
@@ -350,11 +350,20 @@ int hclge_cmd_init(struct hclge_dev *hdev)
hdev->hw.cmq.crq.next_to_use = 0;
hclge_cmd_init_regs(&hdev->hw);
- clear_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
spin_unlock_bh(&hdev->hw.cmq.crq.lock);
spin_unlock_bh(&hdev->hw.cmq.csq.lock);
+ clear_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
+
+ /* Check if there is new reset pending, because the higher level
+ * reset may happen when lower level reset is being processed.
+ */
+ if ((hclge_is_reset_pending(hdev))) {
+ set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
+ return -EBUSY;
+ }
+
ret = hclge_cmd_query_firmware_version(&hdev->hw, &version);
if (ret) {
dev_err(&hdev->pdev->dev,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index ba6ea6b..7dbab29 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2144,6 +2144,14 @@ static u32 hclge_check_event_cause(struct hclge_dev *hdev, u32 *clearval)
*/
/* check for vector0 reset event sources */
+ if (BIT(HCLGE_VECTOR0_IMPRESET_INT_B) & rst_src_reg) {
+ dev_info(&hdev->pdev->dev, "IMP reset interrupt\n");
+ set_bit(HNAE3_IMP_RESET, &hdev->reset_pending);
+ set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
+ *clearval = BIT(HCLGE_VECTOR0_IMPRESET_INT_B);
+ return HCLGE_VECTOR0_EVENT_RST;
+ }
+
if (BIT(HCLGE_VECTOR0_GLOBALRESET_INT_B) & rst_src_reg) {
dev_info(&hdev->pdev->dev, "global reset interrupt\n");
set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
@@ -2160,13 +2168,6 @@ static u32 hclge_check_event_cause(struct hclge_dev *hdev, u32 *clearval)
return HCLGE_VECTOR0_EVENT_RST;
}
- if (BIT(HCLGE_VECTOR0_IMPRESET_INT_B) & rst_src_reg) {
- dev_info(&hdev->pdev->dev, "IMP reset interrupt\n");
- set_bit(HNAE3_IMP_RESET, &hdev->reset_pending);
- *clearval = BIT(HCLGE_VECTOR0_IMPRESET_INT_B);
- return HCLGE_VECTOR0_EVENT_RST;
- }
-
/* check for vector0 mailbox(=CMDQ RX) event source */
if (BIT(HCLGE_VECTOR0_RX_CMDQ_INT_B) & cmdq_src_reg) {
cmdq_src_reg &= ~BIT(HCLGE_VECTOR0_RX_CMDQ_INT_B);
@@ -2352,11 +2353,15 @@ static int hclge_notify_roce_client(struct hclge_dev *hdev,
static int hclge_reset_wait(struct hclge_dev *hdev)
{
#define HCLGE_RESET_WATI_MS 100
-#define HCLGE_RESET_WAIT_CNT 5
+#define HCLGE_RESET_WAIT_CNT 200
u32 val, reg, reg_bit;
u32 cnt = 0;
switch (hdev->reset_type) {
+ case HNAE3_IMP_RESET:
+ reg = HCLGE_GLOBAL_RESET_REG;
+ reg_bit = HCLGE_IMP_RESET_BIT;
+ break;
case HNAE3_GLOBAL_RESET:
reg = HCLGE_GLOBAL_RESET_REG;
reg_bit = HCLGE_GLOBAL_RESET_BIT;
@@ -2561,6 +2566,7 @@ static int hclge_reset_prepare_down(struct hclge_dev *hdev)
static int hclge_reset_prepare_wait(struct hclge_dev *hdev)
{
+ u32 reg_val;
int ret = 0;
switch (hdev->reset_type) {
@@ -2583,6 +2589,11 @@ static int hclge_reset_prepare_wait(struct hclge_dev *hdev)
*/
set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
break;
+ case HNAE3_IMP_RESET:
+ reg_val = hclge_read_dev(&hdev->hw, HCLGE_PF_OTHER_INT_REG);
+ hclge_write_dev(&hdev->hw, HCLGE_PF_OTHER_INT_REG,
+ BIT(HCLGE_VECTOR0_IMP_RESET_INT_B) | reg_val);
+ break;
default:
break;
}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index 8ab974c..b0e683e 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -97,6 +97,7 @@ enum HLCGE_PORT_TYPE {
#define HCLGE_NETWORK_PORT_ID_M GENMASK(3, 0)
/* Reset related Registers */
+#define HCLGE_PF_OTHER_INT_REG 0x20600
#define HCLGE_MISC_RESET_STS_REG 0x20700
#define HCLGE_MISC_VECTOR_INT_STS 0x20800
#define HCLGE_GLOBAL_RESET_REG 0x20A00
@@ -116,6 +117,8 @@ enum HLCGE_PORT_TYPE {
/* CMDQ register bits for RX event(=MBX event) */
#define HCLGE_VECTOR0_RX_CMDQ_INT_B 1
+#define HCLGE_VECTOR0_IMP_RESET_INT_B 1
+
#define HCLGE_MAC_DEFAULT_FRAME \
(ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN + ETH_DATA_LEN)
#define HCLGE_MAC_MIN_FRAME 64
@@ -775,6 +778,11 @@ static inline int hclge_get_queue_id(struct hnae3_queue *queue)
return tqp->index;
}
+static inline bool hclge_is_reset_pending(struct hclge_dev *hdev)
+{
+ return !!hdev->reset_pending;
+}
+
int hclge_inform_reset_assert_to_vf(struct hclge_vport *vport);
int hclge_cfg_mac_speed_dup(struct hclge_dev *hdev, int speed, u8 duplex);
int hclge_set_vlan_filter(struct hnae3_handle *handle, __be16 proto,
--
2.7.4
^ permalink raw reply related
* [Patch net-next 09/11] net: hns3: add PCIe FLR support for PF
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541772476-41478-1-git-send-email-tanhuazhong@huawei.com>
This patch implements the .reset_prepare and .reset_done
ops from pci framework to support the PF FLR.
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 9 ++-
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 20 +++++++
drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 2 +
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 67 ++++++++++++++++++++++
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 1 +
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c | 2 +
6 files changed, 100 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index f15f595..f69d39f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -127,6 +127,7 @@ enum hnae3_reset_type {
HNAE3_VF_FUNC_RESET,
HNAE3_VF_PF_FUNC_RESET,
HNAE3_VF_FULL_RESET,
+ HNAE3_FLR_RESET,
HNAE3_FUNC_RESET,
HNAE3_CORE_RESET,
HNAE3_GLOBAL_RESET,
@@ -134,6 +135,11 @@ enum hnae3_reset_type {
HNAE3_NONE_RESET,
};
+enum hnae3_flr_state {
+ HNAE3_FLR_DOWN,
+ HNAE3_FLR_DONE,
+};
+
struct hnae3_vector_info {
u8 __iomem *io_addr;
int vector;
@@ -299,7 +305,8 @@ struct hnae3_ae_dev {
struct hnae3_ae_ops {
int (*init_ae_dev)(struct hnae3_ae_dev *ae_dev);
void (*uninit_ae_dev)(struct hnae3_ae_dev *ae_dev);
-
+ void (*flr_prepare)(struct hnae3_ae_dev *ae_dev);
+ void (*flr_done)(struct hnae3_ae_dev *ae_dev);
int (*init_client_instance)(struct hnae3_client *client,
struct hnae3_ae_dev *ae_dev);
void (*uninit_client_instance)(struct hnae3_client *client,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 0cd397c..e527898 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1851,9 +1851,29 @@ static pci_ers_result_t hns3_slot_reset(struct pci_dev *pdev)
return PCI_ERS_RESULT_DISCONNECT;
}
+static void hns3_reset_prepare(struct pci_dev *pdev)
+{
+ struct hnae3_ae_dev *ae_dev = pci_get_drvdata(pdev);
+
+ dev_info(&pdev->dev, "hns3 flr prepare\n");
+ if (ae_dev && ae_dev->ops && ae_dev->ops->flr_prepare)
+ ae_dev->ops->flr_prepare(ae_dev);
+}
+
+static void hns3_reset_done(struct pci_dev *pdev)
+{
+ struct hnae3_ae_dev *ae_dev = pci_get_drvdata(pdev);
+
+ dev_info(&pdev->dev, "hns3 flr done\n");
+ if (ae_dev && ae_dev->ops && ae_dev->ops->flr_done)
+ ae_dev->ops->flr_done(ae_dev);
+}
+
static const struct pci_error_handlers hns3_err_handler = {
.error_detected = hns3_error_detected,
.slot_reset = hns3_slot_reset,
+ .reset_prepare = hns3_reset_prepare,
+ .reset_done = hns3_reset_done,
};
static struct pci_driver hns3_driver = {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index bd6ee5c..10ff18a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -594,7 +594,9 @@ static inline void hns3_write_reg(void __iomem *base, u32 reg, u32 value)
static inline bool hns3_dev_ongoing_func_reset(struct hnae3_ae_dev *ae_dev)
{
return (ae_dev && (ae_dev->reset_type == HNAE3_FUNC_RESET ||
+ ae_dev->reset_type == HNAE3_FLR_RESET ||
ae_dev->reset_type == HNAE3_VF_FUNC_RESET ||
+ ae_dev->reset_type == HNAE3_VF_FULL_RESET ||
ae_dev->reset_type == HNAE3_VF_PF_FUNC_RESET));
}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 7dbab29..ab90108 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2374,6 +2374,8 @@ static int hclge_reset_wait(struct hclge_dev *hdev)
reg = HCLGE_FUN_RST_ING;
reg_bit = HCLGE_FUN_RST_ING_B;
break;
+ case HNAE3_FLR_RESET:
+ break;
default:
dev_err(&hdev->pdev->dev,
"Wait for unsupported reset type: %d\n",
@@ -2381,6 +2383,20 @@ static int hclge_reset_wait(struct hclge_dev *hdev)
return -EINVAL;
}
+ if (hdev->reset_type == HNAE3_FLR_RESET) {
+ while (!test_bit(HNAE3_FLR_DONE, &hdev->flr_state) &&
+ cnt++ < HCLGE_RESET_WAIT_CNT)
+ msleep(HCLGE_RESET_WATI_MS);
+
+ if (!test_bit(HNAE3_FLR_DONE, &hdev->flr_state)) {
+ dev_err(&hdev->pdev->dev,
+ "flr wait timeout: %d\n", cnt);
+ return -EBUSY;
+ }
+
+ return 0;
+ }
+
val = hclge_read_dev(&hdev->hw, reg);
while (hnae3_get_bit(val, reg_bit) && cnt < HCLGE_RESET_WAIT_CNT) {
msleep(HCLGE_RESET_WATI_MS);
@@ -2488,6 +2504,12 @@ static void hclge_do_reset(struct hclge_dev *hdev)
set_bit(HNAE3_FUNC_RESET, &hdev->reset_pending);
hclge_reset_task_schedule(hdev);
break;
+ case HNAE3_FLR_RESET:
+ dev_info(&pdev->dev, "FLR requested\n");
+ /* schedule again to check later */
+ set_bit(HNAE3_FLR_RESET, &hdev->reset_pending);
+ hclge_reset_task_schedule(hdev);
+ break;
default:
dev_warn(&pdev->dev,
"Unsupported reset type: %d\n", hdev->reset_type);
@@ -2519,6 +2541,9 @@ static enum hnae3_reset_type hclge_get_reset_level(struct hclge_dev *hdev,
} else if (test_bit(HNAE3_FUNC_RESET, addr)) {
rst_level = HNAE3_FUNC_RESET;
clear_bit(HNAE3_FUNC_RESET, addr);
+ } else if (test_bit(HNAE3_FLR_RESET, addr)) {
+ rst_level = HNAE3_FLR_RESET;
+ clear_bit(HNAE3_FLR_RESET, addr);
}
return rst_level;
@@ -2555,6 +2580,8 @@ static int hclge_reset_prepare_down(struct hclge_dev *hdev)
switch (hdev->reset_type) {
case HNAE3_FUNC_RESET:
+ /* fall through */
+ case HNAE3_FLR_RESET:
ret = hclge_set_all_vf_rst(hdev, true);
break;
default:
@@ -2589,6 +2616,14 @@ static int hclge_reset_prepare_wait(struct hclge_dev *hdev)
*/
set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
break;
+ case HNAE3_FLR_RESET:
+ /* There is no mechanism for PF to know if VF has stopped IO
+ * for now, just wait 100 ms for VF to stop IO
+ */
+ msleep(100);
+ set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
+ set_bit(HNAE3_FLR_DOWN, &hdev->flr_state);
+ break;
case HNAE3_IMP_RESET:
reg_val = hclge_read_dev(&hdev->hw, HCLGE_PF_OTHER_INT_REG);
hclge_write_dev(&hdev->hw, HCLGE_PF_OTHER_INT_REG,
@@ -2647,6 +2682,8 @@ static int hclge_reset_prepare_up(struct hclge_dev *hdev)
switch (hdev->reset_type) {
case HNAE3_FUNC_RESET:
+ /* fall through */
+ case HNAE3_FLR_RESET:
ret = hclge_set_all_vf_rst(hdev, false);
break;
default:
@@ -6917,6 +6954,34 @@ static void hclge_state_uninit(struct hclge_dev *hdev)
cancel_work_sync(&hdev->mbx_service_task);
}
+static void hclge_flr_prepare(struct hnae3_ae_dev *ae_dev)
+{
+#define HCLGE_FLR_WAIT_MS 100
+#define HCLGE_FLR_WAIT_CNT 50
+ struct hclge_dev *hdev = ae_dev->priv;
+ int cnt = 0;
+
+ clear_bit(HNAE3_FLR_DOWN, &hdev->flr_state);
+ clear_bit(HNAE3_FLR_DONE, &hdev->flr_state);
+ set_bit(HNAE3_FLR_RESET, &hdev->default_reset_request);
+ hclge_reset_event(hdev->pdev, NULL);
+
+ while (!test_bit(HNAE3_FLR_DOWN, &hdev->flr_state) &&
+ cnt++ < HCLGE_FLR_WAIT_CNT)
+ msleep(HCLGE_FLR_WAIT_MS);
+
+ if (!test_bit(HNAE3_FLR_DOWN, &hdev->flr_state))
+ dev_err(&hdev->pdev->dev,
+ "flr wait down timeout: %d\n", cnt);
+}
+
+static void hclge_flr_done(struct hnae3_ae_dev *ae_dev)
+{
+ struct hclge_dev *hdev = ae_dev->priv;
+
+ set_bit(HNAE3_FLR_DONE, &hdev->flr_state);
+}
+
static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
{
struct pci_dev *pdev = ae_dev->pdev;
@@ -7575,6 +7640,8 @@ static void hclge_get_link_mode(struct hnae3_handle *handle,
static const struct hnae3_ae_ops hclge_ops = {
.init_ae_dev = hclge_init_ae_dev,
.uninit_ae_dev = hclge_uninit_ae_dev,
+ .flr_prepare = hclge_flr_prepare,
+ .flr_done = hclge_flr_done,
.init_client_instance = hclge_init_client_instance,
.uninit_client_instance = hclge_uninit_client_instance,
.map_ring_to_vector = hclge_map_ring_to_vector,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index b0e683e..ca90b66 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -597,6 +597,7 @@ struct hclge_dev {
struct hclge_misc_vector misc_vector;
struct hclge_hw_stats hw_stats;
unsigned long state;
+ unsigned long flr_state;
unsigned long last_reset_time;
enum hnae3_reset_type reset_type;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
index f15b807..4c7e7bd 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
@@ -90,6 +90,8 @@ int hclge_inform_reset_assert_to_vf(struct hclge_vport *vport)
if (hdev->reset_type == HNAE3_FUNC_RESET)
reset_type = HNAE3_VF_PF_FUNC_RESET;
+ else if (hdev->reset_type == HNAE3_FLR_RESET)
+ reset_type = HNAE3_VF_FULL_RESET;
else
return -EINVAL;
--
2.7.4
^ permalink raw reply related
* [Patch net-next 10/11] net: hns3: do VF's pci re-initialization while PF doing FLR
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541772476-41478-1-git-send-email-tanhuazhong@huawei.com>
While doing PF FLR, VF's PCIe configuration space will be cleared, so
the pci and vector of VF should be re-initialized in the VF's reset
process while PF doing FLR.
Also, this patch fixes some memory not freed problem when pci
re-initialization is done during reset process.
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 24 +++++++--
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 57 ++++++++++++++++++++--
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h | 1 +
3 files changed, 74 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index e527898..8d07ec6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -3849,21 +3849,31 @@ static int hns3_reset_notify_init_enet(struct hnae3_handle *handle)
/* Carrier off reporting is important to ethtool even BEFORE open */
netif_carrier_off(netdev);
+ ret = hns3_nic_alloc_vector_data(priv);
+ if (ret)
+ return ret;
+
hns3_restore_coal(priv);
ret = hns3_nic_init_vector_data(priv);
if (ret)
- return ret;
+ goto err_dealloc_vector;
ret = hns3_init_all_ring(priv);
- if (ret) {
- hns3_nic_uninit_vector_data(priv);
- priv->ring_data = NULL;
- }
+ if (ret)
+ goto err_uninit_vector;
set_bit(HNS3_NIC_STATE_INITED, &priv->state);
return ret;
+
+err_uninit_vector:
+ hns3_nic_uninit_vector_data(priv);
+ priv->ring_data = NULL;
+err_dealloc_vector:
+ hns3_nic_dealloc_vector_data(priv);
+
+ return ret;
}
static int hns3_reset_notify_uninit_enet(struct hnae3_handle *handle)
@@ -3887,6 +3897,10 @@ static int hns3_reset_notify_uninit_enet(struct hnae3_handle *handle)
hns3_store_coal(priv);
+ ret = hns3_nic_dealloc_vector_data(priv);
+ if (ret)
+ netdev_err(netdev, "dealloc vector error\n");
+
ret = hns3_uninit_all_ring(priv);
if (ret)
netdev_err(netdev, "uninit ring error\n");
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 4dfac6c..8f1c681 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1766,6 +1766,7 @@ static int hclgevf_init_msi(struct hclgevf_dev *hdev)
hdev->vector_irq = devm_kcalloc(&pdev->dev, hdev->num_msi,
sizeof(int), GFP_KERNEL);
if (!hdev->vector_irq) {
+ devm_kfree(&pdev->dev, hdev->vector_status);
pci_free_irq_vectors(pdev);
return -ENOMEM;
}
@@ -1777,6 +1778,8 @@ static void hclgevf_uninit_msi(struct hclgevf_dev *hdev)
{
struct pci_dev *pdev = hdev->pdev;
+ devm_kfree(&pdev->dev, hdev->vector_status);
+ devm_kfree(&pdev->dev, hdev->vector_irq);
pci_free_irq_vectors(pdev);
}
@@ -2001,11 +2004,52 @@ static int hclgevf_query_vf_resource(struct hclgevf_dev *hdev)
return 0;
}
+static int hclgevf_pci_reset(struct hclgevf_dev *hdev)
+{
+ struct pci_dev *pdev = hdev->pdev;
+ int ret = 0;
+
+ if (hdev->reset_type == HNAE3_VF_FULL_RESET &&
+ test_bit(HCLGEVF_STATE_IRQ_INITED, &hdev->state)) {
+ hclgevf_misc_irq_uninit(hdev);
+ hclgevf_uninit_msi(hdev);
+ clear_bit(HCLGEVF_STATE_IRQ_INITED, &hdev->state);
+ }
+
+ if (!test_bit(HCLGEVF_STATE_IRQ_INITED, &hdev->state)) {
+ pci_set_master(pdev);
+ ret = hclgevf_init_msi(hdev);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "failed(%d) to init MSI/MSI-X\n", ret);
+ return ret;
+ }
+
+ ret = hclgevf_misc_irq_init(hdev);
+ if (ret) {
+ hclgevf_uninit_msi(hdev);
+ dev_err(&pdev->dev, "failed(%d) to init Misc IRQ(vector0)\n",
+ ret);
+ return ret;
+ }
+
+ set_bit(HCLGEVF_STATE_IRQ_INITED, &hdev->state);
+ }
+
+ return ret;
+}
+
static int hclgevf_reset_hdev(struct hclgevf_dev *hdev)
{
struct pci_dev *pdev = hdev->pdev;
int ret;
+ ret = hclgevf_pci_reset(hdev);
+ if (ret) {
+ dev_err(&pdev->dev, "pci reset failed %d\n", ret);
+ return ret;
+ }
+
ret = hclgevf_cmd_init(hdev);
if (ret) {
dev_err(&pdev->dev, "cmd failed %d\n", ret);
@@ -2076,6 +2120,8 @@ static int hclgevf_init_hdev(struct hclgevf_dev *hdev)
goto err_misc_irq_init;
}
+ set_bit(HCLGEVF_STATE_IRQ_INITED, &hdev->state);
+
ret = hclgevf_configure(hdev);
if (ret) {
dev_err(&pdev->dev, "failed(%d) to fetch configuration\n", ret);
@@ -2123,16 +2169,21 @@ static int hclgevf_init_hdev(struct hclgevf_dev *hdev)
hclgevf_cmd_uninit(hdev);
err_cmd_queue_init:
hclgevf_pci_uninit(hdev);
+ clear_bit(HCLGEVF_STATE_IRQ_INITED, &hdev->state);
return ret;
}
static void hclgevf_uninit_hdev(struct hclgevf_dev *hdev)
{
hclgevf_state_uninit(hdev);
- hclgevf_misc_irq_uninit(hdev);
+
+ if (test_bit(HCLGEVF_STATE_IRQ_INITED, &hdev->state)) {
+ hclgevf_misc_irq_uninit(hdev);
+ hclgevf_uninit_msi(hdev);
+ hclgevf_pci_uninit(hdev);
+ }
+
hclgevf_cmd_uninit(hdev);
- hclgevf_uninit_msi(hdev);
- hclgevf_pci_uninit(hdev);
}
static int hclgevf_init_ae_dev(struct hnae3_ae_dev *ae_dev)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
index ffe3d49..028c52e 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
@@ -73,6 +73,7 @@ enum hclgevf_states {
/* device states */
HCLGEVF_STATE_DOWN,
HCLGEVF_STATE_DISABLED,
+ HCLGEVF_STATE_IRQ_INITED,
/* task states */
HCLGEVF_STATE_SERVICE_SCHED,
HCLGEVF_STATE_RST_SERVICE_SCHED,
--
2.7.4
^ permalink raw reply related
* [Patch net-next 11/11] net: hns3: add PCIe FLR support for VF
From: Huazhong Tan @ 2018-11-09 14:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1541772476-41478-1-git-send-email-tanhuazhong@huawei.com>
This patch implements the .reset_prepare and .reset_done
ops from pci framework to support the VF FLR.
This patch uses hclgevf_set_def_reset_request() and
hclgevf_reset_event() to handle FLR, so when
hdev->default_reset_request is non zero, it means there is
some reset requseted by hclgevf_set_def_reset_request() need
to be processed. Also get the hdev from the ae_dev because
hclgevf_reset_event is called with handle being NULL.
Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
---
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 65 +++++++++++++++++++++-
.../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h | 1 +
2 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 8f1c681..6b4d147 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1099,6 +1099,32 @@ static int hclgevf_notify_client(struct hclgevf_dev *hdev,
return ret;
}
+static void hclgevf_flr_done(struct hnae3_ae_dev *ae_dev)
+{
+ struct hclgevf_dev *hdev = ae_dev->priv;
+
+ set_bit(HNAE3_FLR_DONE, &hdev->flr_state);
+}
+
+static int hclgevf_flr_poll_timeout(struct hclgevf_dev *hdev,
+ unsigned long delay_us,
+ unsigned long wait_cnt)
+{
+ unsigned long cnt = 0;
+
+ while (!test_bit(HNAE3_FLR_DONE, &hdev->flr_state) &&
+ cnt++ < wait_cnt)
+ usleep_range(delay_us, delay_us * 2);
+
+ if (!test_bit(HNAE3_FLR_DONE, &hdev->flr_state)) {
+ dev_err(&hdev->pdev->dev,
+ "flr wait timeout\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
static int hclgevf_reset_wait(struct hclgevf_dev *hdev)
{
#define HCLGEVF_RESET_WAIT_US 20000
@@ -1113,6 +1139,11 @@ static int hclgevf_reset_wait(struct hclgevf_dev *hdev)
val = hclgevf_read_dev(&hdev->hw, HCLGEVF_RST_ING);
dev_info(&hdev->pdev->dev, "checking vf resetting status: %x\n", val);
+ if (hdev->reset_type == HNAE3_FLR_RESET)
+ return hclgevf_flr_poll_timeout(hdev,
+ HCLGEVF_RESET_WAIT_US,
+ HCLGEVF_RESET_WAIT_CNT);
+
ret = readl_poll_timeout(hdev->hw.io_base + HCLGEVF_RST_ING, val,
!(val & HCLGEVF_RST_ING_BITS),
HCLGEVF_RESET_WAIT_US,
@@ -1168,6 +1199,9 @@ static int hclgevf_reset_prepare_wait(struct hclgevf_dev *hdev)
ret = hclgevf_send_mbx_msg(hdev, HCLGE_MBX_RESET, 0, NULL,
0, true, NULL, sizeof(u8));
break;
+ case HNAE3_FLR_RESET:
+ set_bit(HNAE3_FLR_DOWN, &hdev->flr_state);
+ break;
default:
break;
}
@@ -1267,6 +1301,9 @@ static enum hnae3_reset_type hclgevf_get_reset_level(struct hclgevf_dev *hdev,
} else if (test_bit(HNAE3_VF_FUNC_RESET, addr)) {
rst_level = HNAE3_VF_FUNC_RESET;
clear_bit(HNAE3_VF_FUNC_RESET, addr);
+ } else if (test_bit(HNAE3_FLR_RESET, addr)) {
+ rst_level = HNAE3_FLR_RESET;
+ clear_bit(HNAE3_FLR_RESET, addr);
}
return rst_level;
@@ -1275,11 +1312,12 @@ static enum hnae3_reset_type hclgevf_get_reset_level(struct hclgevf_dev *hdev,
static void hclgevf_reset_event(struct pci_dev *pdev,
struct hnae3_handle *handle)
{
- struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
+ struct hnae3_ae_dev *ae_dev = pci_get_drvdata(pdev);
+ struct hclgevf_dev *hdev = ae_dev->priv;
dev_info(&hdev->pdev->dev, "received reset request from VF enet\n");
- if (!hdev->default_reset_request)
+ if (hdev->default_reset_request)
hdev->reset_level =
hclgevf_get_reset_level(hdev,
&hdev->default_reset_request);
@@ -1301,6 +1339,27 @@ static void hclgevf_set_def_reset_request(struct hnae3_ae_dev *ae_dev,
set_bit(rst_type, &hdev->default_reset_request);
}
+static void hclgevf_flr_prepare(struct hnae3_ae_dev *ae_dev)
+{
+#define HCLGEVF_FLR_WAIT_MS 100
+#define HCLGEVF_FLR_WAIT_CNT 50
+ struct hclgevf_dev *hdev = ae_dev->priv;
+ int cnt = 0;
+
+ clear_bit(HNAE3_FLR_DOWN, &hdev->flr_state);
+ clear_bit(HNAE3_FLR_DONE, &hdev->flr_state);
+ set_bit(HNAE3_FLR_RESET, &hdev->default_reset_request);
+ hclgevf_reset_event(hdev->pdev, NULL);
+
+ while (!test_bit(HNAE3_FLR_DOWN, &hdev->flr_state) &&
+ cnt++ < HCLGEVF_FLR_WAIT_CNT)
+ msleep(HCLGEVF_FLR_WAIT_MS);
+
+ if (!test_bit(HNAE3_FLR_DOWN, &hdev->flr_state))
+ dev_err(&hdev->pdev->dev,
+ "flr wait down timeout: %d\n", cnt);
+}
+
static u32 hclgevf_get_fw_version(struct hnae3_handle *handle)
{
struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
@@ -2310,6 +2369,8 @@ static unsigned long hclgevf_ae_dev_reset_cnt(struct hnae3_handle *handle)
static const struct hnae3_ae_ops hclgevf_ops = {
.init_ae_dev = hclgevf_init_ae_dev,
.uninit_ae_dev = hclgevf_uninit_ae_dev,
+ .flr_prepare = hclgevf_flr_prepare,
+ .flr_done = hclgevf_flr_done,
.init_client_instance = hclgevf_init_client_instance,
.uninit_client_instance = hclgevf_uninit_client_instance,
.start = hclgevf_ae_start,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
index 028c52e..4c5ea7e 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.h
@@ -161,6 +161,7 @@ struct hclgevf_dev {
struct hclgevf_misc_vector misc_vector;
struct hclgevf_rss_cfg rss_cfg;
unsigned long state;
+ unsigned long flr_state;
unsigned long default_reset_request;
unsigned long last_reset_time;
enum hnae3_reset_type reset_level;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 11/20] octeontx2-af: Add support for stripping STAG/CTAG
From: Sunil Kovvuri @ 2018-11-09 4:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Netdev List, David S. Miller, linux-soc, Tomasz Duszynski,
Sunil Goutham
In-Reply-To: <CAK8P3a3qJVhDq1Jp6mNiy-pgXWVxDvuGuKOZPFpYhZvt94pabw@mail.gmail.com>
On Fri, Nov 9, 2018 at 2:17 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Nov 8, 2018 at 7:37 PM <sunil.kovvuri@gmail.com> wrote:
>
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/npc.h b/drivers/net/ethernet/marvell/octeontx2/af/npc.h
> > index f98b011..3f7e5e6 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/npc.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/npc.h
> > @@ -259,4 +259,34 @@ struct nix_rx_action {
> > #endif
> > };
> >
> > +struct nix_rx_vtag_action {
> > +#if defined(__BIG_ENDIAN_BITFIELD)
> > + u64 rsvd_63_48 :16;
> > + u64 vtag1_valid :1;
> > + u64 vtag1_type :3;
> > + u64 rsvd_43 :1;
> > + u64 vtag1_lid :3;
> > + u64 vtag1_relptr :8;
> > + u64 rsvd_31_16 :16;
> > + u64 vtag0_valid :1;
> > + u64 vtag0_type :3;
> > + u64 rsvd_11 :1;
> > + u64 vtag0_lid :3;
> > + u64 vtag0_relptr :8;
> > +#else
> > + u64 vtag0_relptr :8;
> > + u64 vtag0_lid :3;
> > + u64 rsvd_11 :1;
> > + u64 vtag0_type :3;
> > + u64 vtag0_valid :1;
> > + u64 rsvd_31_16 :16;
> > + u64 vtag1_relptr :8;
> > + u64 vtag1_lid :3;
> > + u64 rsvd_43 :1;
> > + u64 vtag1_type :3;
> > + u64 vtag1_valid :1;
> > + u64 rsvd_63_48 :16;
> > +#endif
> > +};
>
> Here is another instance of bitfields in an interface structure. As
> before, please try to avoid doing that and use bit shifts and masks
> instead.
>
> Arnd
No, this struct is not part of communication interface.
This is used to fill up a register in a bit more readable fashion
instead of plain bit shifts.
===
struct nix_rx_vtag_action vtag_action;
*(u64 *)&vtag_action = 0;
vtag_action.vtag0_valid = 1;
/* must match type set in NIX_VTAG_CFG */
vtag_action.vtag0_type = 0;
vtag_action.vtag0_lid = NPC_LID_LA;
vtag_action.vtag0_relptr = 12;
entry.vtag_action = *(u64 *)&vtag_action;
/* Set TAG 'action' */
rvu_write64(rvu, blkaddr, NPC_AF_MCAMEX_BANKX_TAG_ACT(index, actbank),
entry->vtag_action);
===
Thanks,
Sunil.
^ permalink raw reply
* Re: [PATCH 00/20] octeontx2-af: NPC MCAM support and FLR handling
From: Sunil Kovvuri @ 2018-11-09 4:34 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Netdev List, David S. Miller, linux-soc, Sunil Goutham
In-Reply-To: <CAK8P3a2iCea-d9duHtakUx7BFgZuFgp92vejpss2eBboZ-ybwg@mail.gmail.com>
On Fri, Nov 9, 2018 at 2:32 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Nov 8, 2018 at 7:36 PM <sunil.kovvuri@gmail.com> wrote:
> >
> > From: Sunil Goutham <sgoutham@marvell.com>
>
> Hmm, I noticed that you use a different address as the patch author
> and the submitter. I'm guessing that "Sunil Goutham" and
> "Sunil Kovvuri" actually refer to the same person, and you just
> need to pick which of the two email addresses you want to use
> for public communication, but that's not obvious here.
>
> However, if there are actually two different Sunil's here, then
> you need to add that second Signed-off-by.
>
No, it's just me.
Sometimes code indentation becomes messy and difficult to read, if i use
corporate mail server to submit patches. So i have been using gmail.
> I've taken a look at all the patches now, and found very little
> sticking out that warranted a comment from my side, and
> no real show-stoppers. That said, I found this series overall
> much harder to understand than the previous ones, and don't
> even know what to ask about it. My feeling is that it's probably
> all fine, but that is purely based on a review of the individual
> pieces, not the overall design and how they fit together. With the
> earlier patches that I managed to get a better understanding
> of, that seemed reasonable as well.
>
> Arnd
^ permalink raw reply
* Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Alexei Starovoitov @ 2018-11-09 4:35 UTC (permalink / raw)
To: Edward Cree
Cc: Martin KaFai Lau, Yonghong Song, Alexei Starovoitov,
Daniel Borkmann, Network Development, Kernel Team
In-Reply-To: <ca4cb188-744d-5274-b12a-59fa3efc68f4@solarflare.com>
On Thu, Nov 08, 2018 at 10:56:55PM +0000, Edward Cree wrote:
> On 08/11/18 19:42, Alexei Starovoitov wrote:
> > same link let's continue at 1pm PST.
> So, one thing we didn't really get onto was maps, and you mentioned that it
> wasn't really clear what I was proposing there.
let's discuss ground rules first.
1. the patches should be upstreamable in both kernel _and_ llvm
For example you propose 'prog_symbol' down below.
I'm sure you can do it in your assembler. We can also do it in pahole
(because it operates on dwarf inside elf), but there is no way for us
to do it in llvm. Because symbol table gen comes last. dwarf, btf sections have
been completed before that. llvm 'fixup' logic also done before symbol table.
So to add something like 'prog_symbol' into .BTF.ext would mean that would need
an extra pass after the last one that messes with the stuff already finalized.
That will get nacked.
Another example: in our first llvm->btf patchset we were using llvm's dwarf to
generate btf. That got nacked and we had to do 2k+ lines complete rewrite
to generate btf from llvm ir. That forced us to generate btf slightly
differently than from dwarf. The difference is not fundamental (like bitfields).
But it drives the point that elf format is _secondary_ to #1 rule above.
Another example: you're proposing to teach bpf backend to recognize
____bpf_map* name. That is not something we can upstream in llvm either.
Similarly on the kernel api side we decided to craft an api in a way
that what is passed to the kernel is returned back in the same way.
Sort of like if kernel understands 'struct bpf_func_info' from patch 5
it should speak back in the same language.
I think it's important to agree on that principle to continue discussion.
Another key point is obvious, but worth repeating.
kernel abi is cast in stone. elf format is not.
which means that what was defined in include/uapi/linux/btf.h is fixed.
That is the format of .BTF section that llvm/pahole emits.
Whereas .BTF.ext section is _not_ defined in kernel uapi.
It defines the elf format and we can change it the future.
The format of .BTF.ext is the protocol between libbpf and llvm.
We define .BTF.ext in libbpf with libbpf license and coding style
and independently define it in llvm with its license and its coding
style.
We need to discuss .BTF.ext and make sure it's extensible, so we can
upgrade libbpf and llvm independently of each other,
but it doesn't have the same requirements as kernel abi.
Like in the kernel abi the extensibility of any structure means
that uknown fields should be zero.
Ex: sys_bpf() accepts 'bpf_attr' and size.
If user's size is larger than kernel size. All uknown fields
must be zero.
In case of libbpf <-> llvm that is not the case.
libbpf _has_ to deal with non-zero unknown fields in some way.
Like it can print warning, but it has to accept the extended format.
Otherwise upgrading llvm will not be possible without upgrading libbpf.
Similarly when both llvm is new and libbpf is new, but kernel is old,
libbpf has to pass to the kernel only the fields it knows about.
(though it may understand all of them).
So libbpf has to deal will all combinations: old/new kernel,
old/new llvm (and corresponding .BTF and .BTF.ext sections).
Now my understanding that you're mainly concerned with elf file format, correct?
I'm making this guess, because you're arguing strongly about KIND_MAP
and blasting our __btf_map* hack.
I'm all for improving this part, but it cannot go into .BTF,
because we already have an api to pass btf_key_type_id, btf_value_type_id
in MAP_CREATE syscall.
Adding new BTF_KIND_MAP into .BTF would be pointless, since we cannot
change MAP_CREATE.
As discussed on the call, currently we're designing KIND_VARIABLE
that will describe global/static variables in .BTF and corresponding
prog_load api changes for the kernel. We'll be talking about it
at bpf uconf. Imo current 'struct bpf_map_def map_foo;' hack
that libbpf/iproute2 and other loaders are dealing with
will be cleaned up by this KIND_VARIABLE BTF support.
Because of rule #1 we cannot pattern match 'bpf_map_def' name
in llvm. All global variables have to be dealt in the common way
by the llvm. We can add new __builtin_ specifically for map creation
and ask folks to start using new interface in .c programs for
map creation. All that is exciting, but I'd like to table that
discussion and focus on this patch set which is about adding
bpf_func_infos, BTF vs BTF.ext split, instances vs types.
> What I have in mind comes in two parts:
> 1) map type. A new BTF_KIND_MAP with metadata 'key_type', 'value_type'
> (both are type_ids referencing other BTF type records), describing the
> type "map from key_type to value_type".
> 2) record in the 'instances' table. This would have a name_off (the
> name of the map), a type_id (pointing at a BTF_KIND_MAP in the 'types'
> table), and potentially also some indication of what symbol (from
> section 'maps') refers to this map. This is pretty much the exact
> same metadata that a function in the 'instances' table has, the only
> differences being
> (a) function's type_id points at a BTF_KIND_FUNC record
> (b) function's symbol indication refers from .text section
> (c) in future functions may be nested inside other functions, whereas
> AIUI a map can't live inside a function. (But a variable, which is
> the other thing that would want to go in an 'instances' table, can.)
> So the 'instances' table record structure looks like
>
> struct btf_instance {
> __u32 type_id; /* Type of object declared. An index into type section */
> __u32 name_off; /* Name of object. An offset into string section */
> __u32 parent; /* Containing object if any (else 0). An index into instance section */
> };
I have two issues with above structure:
1. it's not naturally extensible.
Meaning it's possible to define new struct by bumping version in btf
header, but it's not clean.
2. parent field has to be checked for loops
Both of these issues are resolved by existing BTF and kernel code.
BTF layout is extensible. BTF kernel verifier already checks for loops.
Hence I propose to use existing 'struct btf_type' for these new
instances section instead of inventing new format.
> and we extend the BTF header:
>
> struct btf_header {
> __u16 magic;
> __u8 version;
> __u8 flags;
> __u32 hdr_len;
>
> /* All offsets are in bytes relative to the end of this header */
> __u32 type_off; /* offset of type section */
> __u32 type_len; /* length of type section */
> __u32 str_off; /* offset of string section */
> __u32 str_len; /* length of string section */
> __u32 inst_off; /* offset of instance section */
> __u32 inst_len; /* length of instance section */
The addition of above two fields is certainly doable
and fits design of BTF well. Kernel side already has code to
deal with multiple BTF sections.
We don't even need to bump version for that.
That's great, but the key point here is that btf.h is both
kernel abi and file format.
If llvm emits this instance section into elf file
libbpf loader should better pass it to the kernel as well.
Otherwise having populated instance section in elf and
empty instances for the kernel will make kernel patch to uapi/btf.h
non-upstreamble. Why ? Because we do not add fields to uapi structs
that kernel will not be using.
I suspect you're actually proposing to pass populated instance section
to the kernel as part of BTF_LOAD sys_bpf command. So all good.
Just making sure we're on the same page. Correct?
So the tweak to your 'struct btf_instance' proposal
is to use 'struct btf_type' of variable length instead
in this instances section.
And define new BTF_KIND_FUNC that can only be used
in the instances.
In the future we will add BTF_KIND_VARIABLE in there too.
As far as type section there we will use BTF_KIND_FUNC_PROTO only.
It will always have empty name and can have full names
for function arguments.
BTF_KIND_FUNC in instances section must have non-zero name_off
(which will describe function name) and type_id pointing
to BTF_KIND_FUNC_PROTO in type section.
Multiple KIND_FUNC potentially can point to the same KIND_FUNC_PROTO
if all argument names are the same or all of them are empty.
(and types match, of course).
I'm not counting on overall BTF size compression because of that,
but it's kinda nice to have this option.
> Then in the .BTF.ext section, we have both
>
> struct bpf_func_info {
> __u32 prog_symbol; /* Index of symbol giving address of subprog */
> __u32 inst_id; /* Index into instance section */
> }
Right, for .BPF.ext section we can keep existing
'struct bpf_func_info' with the minor tweak:
struct bpf_func_info {
__u32 insn_offset;
__u32 inst_id; // this is now index in the instance section
};
Since instance section will have KIND_FUNC and KIND_VARIABLE in the future
the kernel has to check that 'bpf_func_info->inst_id' in .BTF.ext
points to KIND_FUNC in .BTF.
How does this sound?
If we agree to that it will unblock us for this patch set
and for follow on patch set that adds 'struct bpf_line_info'
based on the same principles.
>
> struct bpf_map_info {
> {
> __u32 map_symbol; /* Index of symbol creating this map */
> __u32 inst_id; /* Index into instance section */
> }
The map discussion I'd like to table for now due to reasons
outlined above.
> think this question of maps should be discussed in tomorrow's
> call, since it is when we start having other kinds of instances
turned out most of us have a conflict, so the earliest is 1:30pm on Friday.
still works for you?
^ permalink raw reply
* Re: [Patch net-next v2] net: move __skb_checksum_complete*() to skbuff.c
From: David Miller @ 2018-11-09 4:39 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, sbrivio
In-Reply-To: <20181108220542.17944-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu, 8 Nov 2018 14:05:42 -0800
> __skb_checksum_complete_head() and __skb_checksum_complete()
> are both declared in skbuff.h, they fit better in skbuff.c
> than datagram.c.
>
> Cc: Stefano Brivio <sbrivio@redhat.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/9] Use __vlan_hwaccel_*() helpers
From: David Miller @ 2018-11-09 4:45 UTC (permalink / raw)
To: mirq-linux
Cc: netdev, ajit.khaparde, kuznet, bridge, xiyou.wangcong, coreteam,
fw, yoshfuji, jhs, jiri, kadlec, linux-rdma, mlindner,
netfilter-devel, nikolay, pablo, roopa, sathya.perla,
somnath.kotur, sriharsha.basavapatna, stephen, swise, tariqt
In-Reply-To: <cover.1541718583.git.mirq-linux@rere.qmqm.pl>
From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Fri, 09 Nov 2018 00:17:58 +0100
> This series removes from networking core and driver code an assumption
> about how VLAN tag presence is stored in an skb. This will allow to free
> up overloading of VLAN.CFI bit to incidate tag's presence.
This looks good, series applied.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] tcp_bbr: update comments to reflect pacing_margin_percent
From: David Miller @ 2018-11-09 4:46 UTC (permalink / raw)
To: ncardwell; +Cc: netdev, ycheng, soheil, edumazet
In-Reply-To: <20181109025400.38021-1-ncardwell@google.com>
From: Neal Cardwell <ncardwell@google.com>
Date: Thu, 8 Nov 2018 21:54:00 -0500
> Recently, in commit ab408b6dc744 ("tcp: switch tcp and sch_fq to new
> earliest departure time model"), the TCP BBR code switched to a new
> approach of using an explicit bbr_pacing_margin_percent for shaving a
> pacing rate "haircut", rather than the previous implict
> approach. Update an old comment to reflect the new approach.
>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/7] nfp: abm: move code and improve parameter validation
From: David Miller @ 2018-11-09 4:49 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20181109035039.25823-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Thu, 8 Nov 2018 19:50:32 -0800
> This set starts by separating Qdisc handling code into a new file.
> Next two patches allow early access to TLV-based capabilities during
> probe, previously the capabilities were parsed just before netdevs
> were registered, but its cleaner to do some basic validation earlier
> and avoid cleanup work.
>
> Next three patches improve RED's parameter validation. First we provide
> a more precise message about why offload failed (and move the parameter
> validation to a helper). Next we make sure we don't set the top bit
> in the 32 bit max RED threshold value. Because FW is treating the value
> as signed it reportedly causes slow downs (unnecessary queuing and
> marking) when top bit is set with recent firmwares. Last (and perhaps
> least importantly) we offload the harddrop parameter of the Qdisc.
> We don't plan to offload harddrop RED, but it seems prudent to make
> sure user didn't set that flag as device behaviour would have differed.
Series applied, thanks Jakub.
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Saeed Mahameed @ 2018-11-09 4:52 UTC (permalink / raw)
To: dsahern@gmail.com, pstaszewski@itcare.pl, brouer@redhat.com
Cc: netdev@vger.kernel.org, yoel@kviknet.dk
In-Reply-To: <754d9d5d-efd2-52e0-cb2b-13caf15f0737@gmail.com>
On Thu, 2018-11-08 at 17:42 -0700, David Ahern wrote:
> On 11/8/18 5:40 PM, Paweł Staszewski wrote:
> >
> > W dniu 08.11.2018 o 17:32, David Ahern pisze:
> > > On 11/8/18 9:27 AM, Paweł Staszewski wrote:
> > > > > > What hardware is this?
> > > > > >
> > > > mellanox connectx 4
> > > > ethtool -i enp175s0f0
> > > > driver: mlx5_core
> > > > version: 5.0-0
> > > > firmware-version: 12.21.1000 (SM_2001000001033)
> > > > expansion-rom-version:
> > > > bus-info: 0000:af:00.0
> > > > supports-statistics: yes
> > > > supports-test: yes
> > > > supports-eeprom-access: no
> > > > supports-register-dump: no
> > > > supports-priv-flags: yes
> > > >
> > > > ethtool -i enp175s0f1
> > > > driver: mlx5_core
> > > > version: 5.0-0
> > > > firmware-version: 12.21.1000 (SM_2001000001033)
> > > > expansion-rom-version:
> > > > bus-info: 0000:af:00.1
> > > > supports-statistics: yes
> > > > supports-test: yes
> > > > supports-eeprom-access: no
> > > > supports-register-dump: no
> > > > supports-priv-flags: yes
> > > >
> > > > > > Start with:
> > > > > >
> > > > > > echo 1 > /sys/kernel/debug/tracing/events/xdp/enable
> > > > > > cat /sys/kernel/debug/tracing/trace_pipe
> > > > > cat /sys/kernel/debug/tracing/trace_pipe
> > > > > <idle>-0 [045] ..s. 68469.467752:
> > > > > xdp_devmap_xmit:
> > > > > ndo_xdp_xmit map_id=32 map_index=5 action=REDIRECT sent=0
> > > > > drops=1
> > > > > from_ifindex=4 to_ifindex=5 err=-6
> > > FIB lookup is good, the redirect is happening, but the mlx5
> > > driver does
> > > not like it.
> > >
> > > I think the -6 is coming from the mlx5 driver and the packet is
> > > getting
> > > dropped. Perhaps this check in mlx5e_xdp_xmit:
> > >
> > > if (unlikely(sq_num >= priv->channels.num))
> > > return -ENXIO;
> > I removed that part and recompiled - but after running now xdp_fwd
> > i
> > have kernel pamic :)
>
hh, no please don't do such thing :)
It must be because the tx netdev has less tx queues than the rx netdev.
or the rx netdev rings are bound to a high cpu indexes.
anyway, best practice is to open #cores RX/TX netdev on both sides
ethtool -L enp175s0f0 combined $(nproc)
ethtool -L enp175s0f1 combined $(nproc)
> Jesper or one of the Mellanox folks needs to respond about the config
> needed to run XDP with this NIC. I don't have a 40G or 100G card to
> play
> with.
^ permalink raw reply
* Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
From: Aleksa Sarai @ 2018-11-09 15:06 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Aleksa Sarai, Steven Rostedt, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Jonathan Corbet, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
Brendan Gregg, Christian Brauner, netdev, linux-doc, linux-kernel
In-Reply-To: <20181109161551.6b96bd7d932c71432ac65e83@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 5076 bytes --]
On 2018-11-09, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
> > index ee696efec99f..c4dfafd43e11 100644
> > --- a/arch/x86/include/asm/ptrace.h
> > +++ b/arch/x86/include/asm/ptrace.h
> > @@ -172,6 +172,7 @@ static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
> > return regs->sp;
> > }
> > #endif
> > +#define stack_addr(regs) ((unsigned long *) kernel_stack_pointer(regs))
>
> No, you should use kernel_stack_pointer(regs) itself instead of stack_addr().
>
> >
> > #define GET_IP(regs) ((regs)->ip)
> > #define GET_FP(regs) ((regs)->bp)
> > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> > index b0d1e81c96bb..eb4da885020c 100644
> > --- a/arch/x86/kernel/kprobes/core.c
> > +++ b/arch/x86/kernel/kprobes/core.c
> > @@ -69,8 +69,6 @@
> > DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
> > DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> >
> > -#define stack_addr(regs) ((unsigned long *)kernel_stack_pointer(regs))
>
> I don't like keeping this meaningless macro... this should be replaced with generic
> kernel_stack_pointer() macro.
Sure. This patch was just an example -- I can remove stack_addr() all
over.
> > - if (regs)
> > - save_stack_address(trace, regs->ip, nosched);
> > + if (regs) {
> > + /* XXX: Currently broken -- stack_addr(regs) doesn't match entry. */
> > + addr = regs->ip;
>
> Since this part is for storing regs->ip as a top of call-stack, this
> seems correct code. Stack unwind will be done next block.
This comment was referring to the usage of stack_addr(). stack_addr()
doesn't give you the right result (it isn't the address of the return
address -- it's slightly wrong). This is the main issue I was having --
am I doing something wrong here?
> > + //addr = ftrace_graph_ret_addr(current, &state.graph_idx, addr, stack_addr(regs));
>
> so func graph return trampoline address will be shown only when unwinding stack entries.
> I mean func-graph tracer is not used as an event, so it never kicks stackdump.
Just to make sure I understand what you're saying -- func-graph trace
will never actually call __ftrace_stack_trace? Because if it does, then
this code will be necessary (and then I'm a bit confused why the
unwinder has func-graph trace code -- if stack traces are never taken
under func-graph then the code in the unwinder is not necessary)
My reason for commenting this out is because at this point "state" isn't
initialised and thus .graph_idx would not be correctly handled during
unwind (and it's the same reason I commented it out later).
> > + addr = kretprobe_ret_addr(current, addr, stack_addr(regs));
>
> But since kretprobe will be an event, which can kick the stackdump.
> BTW, from kretprobe, regs->ip should always be the trampoline handler,
> see arch/x86/kernel/kprobes/core.c:772 :-)
> So it must be fixed always.
Right, but kretprobe_ret_addr() is returning the *original* return
address (and we need to do an (addr == kretprobe_trampoline)). The
real problem is that stack_addr(regs) isn't the same as it is during
kretprobe setup (but kretprobe_ret_addr() works everywhere else).
> > @@ -1856,6 +1870,41 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
> > }
> > NOKPROBE_SYMBOL(pre_handler_kretprobe);
> >
> > +unsigned long kretprobe_ret_addr(struct task_struct *tsk, unsigned long ret,
> > + unsigned long *retp)
> > +{
> > + struct kretprobe_instance *ri;
> > + unsigned long flags = 0;
> > + struct hlist_head *head;
> > + bool need_lock;
> > +
> > + if (likely(ret != (unsigned long) &kretprobe_trampoline))
> > + return ret;
> > +
> > + need_lock = !kretprobe_hash_is_locked(tsk);
> > + if (WARN_ON(need_lock))
> > + kretprobe_hash_lock(tsk, &head, &flags);
> > + else
> > + head = kretprobe_inst_table_head(tsk);
>
> This may not work unless this is called from the kretprobe handler context,
> since if we are out of kretprobe handler context, another CPU can lock the
> hash table and it can be detected by kretprobe_hash_is_locked();.
Yeah, I noticed this as well when writing it (but needed a quick impl
that I could test). I will fix this, thanks!
By is_kretprobe_handler_context() I imagine you are referring to
checking is_kretprobe(current_kprobe())?
> So, we should check we are in the kretprobe handler context if tsk == current,
> if not, we definately can lock the hash lock without any warning. This can
> be something like;
>
> if (is_kretprobe_handler_context()) {
> // kretprobe_hash_lock(current == tsk) has been locked by caller
> if (tsk != current && kretprobe_hash(tsk) != kretprobe_hash(current))
> // the hash of tsk and current can be same.
> need_lock = true;
> } else
> // we should take a lock for tsk.
> need_lock = true;
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
From: Aleksa Sarai @ 2018-11-09 15:10 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Josh Poimboeuf, Aleksa Sarai, Steven Rostedt, Naveen N. Rao,
Anil S Keshavamurthy, David S. Miller, Jonathan Corbet,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, Shuah Khan,
Alexei Starovoitov, Daniel Borkmann, Brendan Gregg,
Christian Brauner, netdev, linux-doc
In-Reply-To: <20181109162632.9a3694eea8ac8056302a8262@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2907 bytes --]
On 2018-11-09, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> On Thu, 8 Nov 2018 08:44:37 -0600
> Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> > On Thu, Nov 08, 2018 at 07:04:48PM +1100, Aleksa Sarai wrote:
> > > On 2018-11-08, Aleksa Sarai <cyphar@cyphar.com> wrote:
> > > > I will attach what I have at the moment to hopefully explain what the
> > > > issue I've found is (re-using the kretprobe architecture but with the
> > > > shadow-stack idea).
> > >
> > > Here is the patch I have at the moment (it works, except for the
> > > question I have about how to handle the top-level pt_regs -- I've marked
> > > that code with XXX).
> > >
> > > --
> > > Aleksa Sarai
> > > Senior Software Engineer (Containers)
> > > SUSE Linux GmbH
> > > <https://www.cyphar.com/>
> > >
> > > --8<---------------------------------------------------------------------
> > >
> > > Since the return address is modified by kretprobe, the various unwinders
> > > can produce invalid and confusing stack traces. ftrace mostly solved
> > > this problem by teaching each unwinder how to find the original return
> > > address for stack trace purposes. This same technique can be applied to
> > > kretprobes by simply adding a pointer to where the return address was
> > > replaced in the stack, and then looking up the relevant
> > > kretprobe_instance when a stack trace is requested.
> > >
> > > [WIP: This is currently broken because the *first entry* will not be
> > > overwritten since it looks like the stack pointer is different
> > > when we are provided pt_regs. All other addresses are correctly
> > > handled.]
> >
> > When you see this problem, what does regs->ip point to? If it's
> > pointing to generated code, then we don't _currently_ have a way of
> > dealing with that. If it's pointing to a real function, we can fix that
> > with unwind hints.
>
> As I replied, If the stackdump is called from kretprobe event, regs->ip
> always points trampoline function. Otherwise (maybe from kprobe event,
> or panic, BUG etc.) it always be the address which the event occurs.
>
> So fixing regs->ip is correct.
The problem is that the pointer to the *return address* is wrong
(kernel_stack_pointer() gives you a different result than the function
entry), it's not that regs->ip is wrong. And I'm sure that it's "wrong"
because it's not possible for "regs->ip == kretprobe_trampoline" unless
you are in a stack frame that has been modified by the kretprobe core.
I will take a closer look at this over the weekend -- I posted the patch
to try to help explain what the underlying issue I was trying to solve
with this patch series is (and why I don't think the ftrace changes
proposed in the thread will completely fix them).
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox