Commit a10f9bfe authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'devlink-errors-fmsg'



Przemek Kitszel says:

====================
devlink: retain error in struct devlink_fmsg

Extend devlink fmsg to retain error (patch 1),
so drivers could omit error checks after devlink_fmsg_*() (patches 2-10),
and finally enforce future uses to follow this practice by change to
return void (patch 11)

Note that it was compile tested only.

bloat-o-meter for whole series:
add/remove: 8/18 grow/shrink: 23/40 up/down: 2017/-5833 (-3816)
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 7ce69360 0050629c
Loading
Loading
Loading
Loading
+8 −21
Original line number Diff line number Diff line
@@ -154,33 +154,20 @@ int pdsc_fw_reporter_diagnose(struct devlink_health_reporter *reporter,
			      struct netlink_ext_ack *extack)
{
	struct pdsc *pdsc = devlink_health_reporter_priv(reporter);
	int err;

	mutex_lock(&pdsc->config_lock);

	if (test_bit(PDSC_S_FW_DEAD, &pdsc->state))
		err = devlink_fmsg_string_pair_put(fmsg, "Status", "dead");
		devlink_fmsg_string_pair_put(fmsg, "Status", "dead");
	else if (!pdsc_is_fw_good(pdsc))
		err = devlink_fmsg_string_pair_put(fmsg, "Status", "unhealthy");
		devlink_fmsg_string_pair_put(fmsg, "Status", "unhealthy");
	else
		err = devlink_fmsg_string_pair_put(fmsg, "Status", "healthy");

		devlink_fmsg_string_pair_put(fmsg, "Status", "healthy");
	mutex_unlock(&pdsc->config_lock);

	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "State",
					pdsc->fw_status &
						~PDS_CORE_FW_STS_F_GENERATION);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "Generation",
					pdsc->fw_generation >> 4);
	if (err)
		return err;
	devlink_fmsg_u32_pair_put(fmsg, "State",
				  pdsc->fw_status & ~PDS_CORE_FW_STS_F_GENERATION);
	devlink_fmsg_u32_pair_put(fmsg, "Generation", pdsc->fw_generation >> 4);
	devlink_fmsg_u32_pair_put(fmsg, "Recoveries", pdsc->fw_recoveries);

	return devlink_fmsg_u32_pair_put(fmsg, "Recoveries",
					 pdsc->fw_recoveries);
	return 0;
}
+32 −61
Original line number Diff line number Diff line
@@ -104,20 +104,21 @@ static int bnxt_fw_diagnose(struct devlink_health_reporter *reporter,
	struct bnxt *bp = devlink_health_reporter_priv(reporter);
	struct bnxt_fw_health *h = bp->fw_health;
	u32 fw_status, fw_resets;
	int rc;

	if (test_bit(BNXT_STATE_IN_FW_RESET, &bp->state))
		return devlink_fmsg_string_pair_put(fmsg, "Status", "recovering");
	if (test_bit(BNXT_STATE_IN_FW_RESET, &bp->state)) {
		devlink_fmsg_string_pair_put(fmsg, "Status", "recovering");
		return 0;
	}

	if (!h->status_reliable)
		return devlink_fmsg_string_pair_put(fmsg, "Status", "unknown");
	if (!h->status_reliable) {
		devlink_fmsg_string_pair_put(fmsg, "Status", "unknown");
		return 0;
	}

	mutex_lock(&h->lock);
	fw_status = bnxt_fw_health_readl(bp, BNXT_FW_HEALTH_REG);
	if (BNXT_FW_IS_BOOTING(fw_status)) {
		rc = devlink_fmsg_string_pair_put(fmsg, "Status", "initializing");
		if (rc)
			goto unlock;
		devlink_fmsg_string_pair_put(fmsg, "Status", "initializing");
	} else if (h->severity || fw_status != BNXT_FW_STATUS_HEALTHY) {
		if (!h->severity) {
			h->severity = SEVERITY_FATAL;
@@ -126,58 +127,35 @@ static int bnxt_fw_diagnose(struct devlink_health_reporter *reporter,
			devlink_health_report(h->fw_reporter,
					      "FW error diagnosed", h);
		}
		rc = devlink_fmsg_string_pair_put(fmsg, "Status", "error");
		if (rc)
			goto unlock;
		rc = devlink_fmsg_u32_pair_put(fmsg, "Syndrome", fw_status);
		if (rc)
			goto unlock;
		devlink_fmsg_string_pair_put(fmsg, "Status", "error");
		devlink_fmsg_u32_pair_put(fmsg, "Syndrome", fw_status);
	} else {
		rc = devlink_fmsg_string_pair_put(fmsg, "Status", "healthy");
		if (rc)
			goto unlock;
		devlink_fmsg_string_pair_put(fmsg, "Status", "healthy");
	}

	rc = devlink_fmsg_string_pair_put(fmsg, "Severity",
	devlink_fmsg_string_pair_put(fmsg, "Severity",
				     bnxt_health_severity_str(h->severity));
	if (rc)
		goto unlock;

	if (h->severity) {
		rc = devlink_fmsg_string_pair_put(fmsg, "Remedy",
		devlink_fmsg_string_pair_put(fmsg, "Remedy",
					     bnxt_health_remedy_str(h->remedy));
		if (rc)
			goto unlock;
		if (h->remedy == REMEDY_DEVLINK_RECOVER) {
			rc = devlink_fmsg_string_pair_put(fmsg, "Impact",
		if (h->remedy == REMEDY_DEVLINK_RECOVER)
			devlink_fmsg_string_pair_put(fmsg, "Impact",
						     "traffic+ntuple_cfg");
			if (rc)
				goto unlock;
		}
	}

unlock:
	mutex_unlock(&h->lock);
	if (rc || !h->resets_reliable)
		return rc;
	if (!h->resets_reliable)
		return 0;

	fw_resets = bnxt_fw_health_readl(bp, BNXT_FW_RESET_CNT_REG);
	rc = devlink_fmsg_u32_pair_put(fmsg, "Resets", fw_resets);
	if (rc)
		return rc;
	rc = devlink_fmsg_u32_pair_put(fmsg, "Arrests", h->arrests);
	if (rc)
		return rc;
	rc = devlink_fmsg_u32_pair_put(fmsg, "Survivals", h->survivals);
	if (rc)
		return rc;
	rc = devlink_fmsg_u32_pair_put(fmsg, "Discoveries", h->discoveries);
	if (rc)
		return rc;
	rc = devlink_fmsg_u32_pair_put(fmsg, "Fatalities", h->fatalities);
	if (rc)
		return rc;
	return devlink_fmsg_u32_pair_put(fmsg, "Diagnoses", h->diagnoses);
	devlink_fmsg_u32_pair_put(fmsg, "Resets", fw_resets);
	devlink_fmsg_u32_pair_put(fmsg, "Arrests", h->arrests);
	devlink_fmsg_u32_pair_put(fmsg, "Survivals", h->survivals);
	devlink_fmsg_u32_pair_put(fmsg, "Discoveries", h->discoveries);
	devlink_fmsg_u32_pair_put(fmsg, "Fatalities", h->fatalities);
	devlink_fmsg_u32_pair_put(fmsg, "Diagnoses", h->diagnoses);
	return 0;
}

static int bnxt_fw_dump(struct devlink_health_reporter *reporter,
@@ -203,19 +181,12 @@ static int bnxt_fw_dump(struct devlink_health_reporter *reporter,

	rc = bnxt_get_coredump(bp, BNXT_DUMP_LIVE, data, &dump_len);
	if (!rc) {
		rc = devlink_fmsg_pair_nest_start(fmsg, "core");
		if (rc)
			goto exit;
		rc = devlink_fmsg_binary_pair_put(fmsg, "data", data, dump_len);
		if (rc)
			goto exit;
		rc = devlink_fmsg_u32_pair_put(fmsg, "size", dump_len);
		if (rc)
			goto exit;
		rc = devlink_fmsg_pair_nest_end(fmsg);
		devlink_fmsg_pair_nest_start(fmsg, "core");
		devlink_fmsg_binary_pair_put(fmsg, "data", data, dump_len);
		devlink_fmsg_u32_pair_put(fmsg, "size", dump_len);
		devlink_fmsg_pair_nest_end(fmsg);
	}

exit:
	vfree(data);
	return rc;
}
+56 −161
Original line number Diff line number Diff line
@@ -315,136 +315,76 @@ void hinic_devlink_unregister(struct hinic_devlink_priv *priv)
	devlink_unregister(devlink);
}

static int chip_fault_show(struct devlink_fmsg *fmsg,
static void chip_fault_show(struct devlink_fmsg *fmsg,
			    struct hinic_fault_event *event)
{
	const char * const level_str[FAULT_LEVEL_MAX + 1] = {
		"fatal", "reset", "flr", "general", "suggestion", "Unknown"};
	u8 fault_level;
	int err;

	fault_level = (event->event.chip.err_level < FAULT_LEVEL_MAX) ?
		event->event.chip.err_level : FAULT_LEVEL_MAX;
	if (fault_level == FAULT_LEVEL_SERIOUS_FLR) {
		err = devlink_fmsg_u32_pair_put(fmsg, "Function level err func_id",
	if (fault_level == FAULT_LEVEL_SERIOUS_FLR)
		devlink_fmsg_u32_pair_put(fmsg, "Function level err func_id",
					  (u32)event->event.chip.func_id);
		if (err)
			return err;
	}

	err = devlink_fmsg_u8_pair_put(fmsg, "module_id", event->event.chip.node_id);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "err_type", (u32)event->event.chip.err_type);
	if (err)
		return err;

	err = devlink_fmsg_string_pair_put(fmsg, "err_level", level_str[fault_level]);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "err_csr_addr",
	devlink_fmsg_u8_pair_put(fmsg, "module_id", event->event.chip.node_id);
	devlink_fmsg_u32_pair_put(fmsg, "err_type", (u32)event->event.chip.err_type);
	devlink_fmsg_string_pair_put(fmsg, "err_level", level_str[fault_level]);
	devlink_fmsg_u32_pair_put(fmsg, "err_csr_addr",
				  event->event.chip.err_csr_addr);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "err_csr_value",
	devlink_fmsg_u32_pair_put(fmsg, "err_csr_value",
				  event->event.chip.err_csr_value);
	if (err)
		return err;

	return 0;
}

static int fault_report_show(struct devlink_fmsg *fmsg,
static void fault_report_show(struct devlink_fmsg *fmsg,
			      struct hinic_fault_event *event)
{
	const char * const type_str[FAULT_TYPE_MAX + 1] = {
		"chip", "ucode", "mem rd timeout", "mem wr timeout",
		"reg rd timeout", "reg wr timeout", "phy fault", "Unknown"};
	u8 fault_type;
	int err;

	fault_type = (event->type < FAULT_TYPE_MAX) ? event->type : FAULT_TYPE_MAX;

	err = devlink_fmsg_string_pair_put(fmsg, "Fault type", type_str[fault_type]);
	if (err)
		return err;

	err = devlink_fmsg_binary_pair_put(fmsg, "Fault raw data",
					   event->event.val, sizeof(event->event.val));
	if (err)
		return err;
	devlink_fmsg_string_pair_put(fmsg, "Fault type", type_str[fault_type]);
	devlink_fmsg_binary_pair_put(fmsg, "Fault raw data", event->event.val,
				     sizeof(event->event.val));

	switch (event->type) {
	case FAULT_TYPE_CHIP:
		err = chip_fault_show(fmsg, event);
		if (err)
			return err;
		chip_fault_show(fmsg, event);
		break;
	case FAULT_TYPE_UCODE:
		err = devlink_fmsg_u8_pair_put(fmsg, "Cause_id", event->event.ucode.cause_id);
		if (err)
			return err;
		err = devlink_fmsg_u8_pair_put(fmsg, "core_id", event->event.ucode.core_id);
		if (err)
			return err;
		err = devlink_fmsg_u8_pair_put(fmsg, "c_id", event->event.ucode.c_id);
		if (err)
			return err;
		err = devlink_fmsg_u8_pair_put(fmsg, "epc", event->event.ucode.epc);
		if (err)
			return err;
		devlink_fmsg_u8_pair_put(fmsg, "Cause_id", event->event.ucode.cause_id);
		devlink_fmsg_u8_pair_put(fmsg, "core_id", event->event.ucode.core_id);
		devlink_fmsg_u8_pair_put(fmsg, "c_id", event->event.ucode.c_id);
		devlink_fmsg_u8_pair_put(fmsg, "epc", event->event.ucode.epc);
		break;
	case FAULT_TYPE_MEM_RD_TIMEOUT:
	case FAULT_TYPE_MEM_WR_TIMEOUT:
		err = devlink_fmsg_u32_pair_put(fmsg, "Err_csr_ctrl",
		devlink_fmsg_u32_pair_put(fmsg, "Err_csr_ctrl",
					  event->event.mem_timeout.err_csr_ctrl);
		if (err)
			return err;
		err = devlink_fmsg_u32_pair_put(fmsg, "err_csr_data",
		devlink_fmsg_u32_pair_put(fmsg, "err_csr_data",
					  event->event.mem_timeout.err_csr_data);
		if (err)
			return err;
		err = devlink_fmsg_u32_pair_put(fmsg, "ctrl_tab",
		devlink_fmsg_u32_pair_put(fmsg, "ctrl_tab",
					  event->event.mem_timeout.ctrl_tab);
		if (err)
			return err;
		err = devlink_fmsg_u32_pair_put(fmsg, "mem_index",
		devlink_fmsg_u32_pair_put(fmsg, "mem_index",
					  event->event.mem_timeout.mem_index);
		if (err)
			return err;
		break;
	case FAULT_TYPE_REG_RD_TIMEOUT:
	case FAULT_TYPE_REG_WR_TIMEOUT:
		err = devlink_fmsg_u32_pair_put(fmsg, "Err_csr", event->event.reg_timeout.err_csr);
		if (err)
			return err;
		devlink_fmsg_u32_pair_put(fmsg, "Err_csr", event->event.reg_timeout.err_csr);
		break;
	case FAULT_TYPE_PHY_FAULT:
		err = devlink_fmsg_u8_pair_put(fmsg, "Op_type", event->event.phy_fault.op_type);
		if (err)
			return err;
		err = devlink_fmsg_u8_pair_put(fmsg, "port_id", event->event.phy_fault.port_id);
		if (err)
			return err;
		err = devlink_fmsg_u8_pair_put(fmsg, "dev_ad", event->event.phy_fault.dev_ad);
		if (err)
			return err;

		err = devlink_fmsg_u32_pair_put(fmsg, "csr_addr", event->event.phy_fault.csr_addr);
		if (err)
			return err;
		err = devlink_fmsg_u32_pair_put(fmsg, "op_data", event->event.phy_fault.op_data);
		if (err)
			return err;
		devlink_fmsg_u8_pair_put(fmsg, "Op_type", event->event.phy_fault.op_type);
		devlink_fmsg_u8_pair_put(fmsg, "port_id", event->event.phy_fault.port_id);
		devlink_fmsg_u8_pair_put(fmsg, "dev_ad", event->event.phy_fault.dev_ad);
		devlink_fmsg_u32_pair_put(fmsg, "csr_addr", event->event.phy_fault.csr_addr);
		devlink_fmsg_u32_pair_put(fmsg, "op_data", event->event.phy_fault.op_data);
		break;
	default:
		break;
	}

	return 0;
}

static int hinic_hw_reporter_dump(struct devlink_health_reporter *reporter,
@@ -452,75 +392,30 @@ static int hinic_hw_reporter_dump(struct devlink_health_reporter *reporter,
				  struct netlink_ext_ack *extack)
{
	if (priv_ctx)
		return fault_report_show(fmsg, priv_ctx);
		fault_report_show(fmsg, priv_ctx);

	return 0;
}

static int mgmt_watchdog_report_show(struct devlink_fmsg *fmsg,
				     struct hinic_mgmt_watchdog_info *watchdog_info)
static void mgmt_watchdog_report_show(struct devlink_fmsg *fmsg,
				      struct hinic_mgmt_watchdog_info *winfo)
{
	int err;

	err = devlink_fmsg_u32_pair_put(fmsg, "Mgmt deadloop time_h", watchdog_info->curr_time_h);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "time_l", watchdog_info->curr_time_l);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "task_id", watchdog_info->task_id);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "sp", watchdog_info->sp);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "stack_current_used", watchdog_info->curr_used);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "peak_used", watchdog_info->peak_used);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "\n Overflow_flag", watchdog_info->is_overflow);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "stack_top", watchdog_info->stack_top);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "stack_bottom", watchdog_info->stack_bottom);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "mgmt_pc", watchdog_info->pc);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "lr", watchdog_info->lr);
	if (err)
		return err;

	err = devlink_fmsg_u32_pair_put(fmsg, "cpsr", watchdog_info->cpsr);
	if (err)
		return err;

	err = devlink_fmsg_binary_pair_put(fmsg, "Mgmt register info",
					   watchdog_info->reg, sizeof(watchdog_info->reg));
	if (err)
		return err;

	err = devlink_fmsg_binary_pair_put(fmsg, "Mgmt dump stack(start from sp)",
					   watchdog_info->data, sizeof(watchdog_info->data));
	if (err)
		return err;

	return 0;
	devlink_fmsg_u32_pair_put(fmsg, "Mgmt deadloop time_h", winfo->curr_time_h);
	devlink_fmsg_u32_pair_put(fmsg, "time_l", winfo->curr_time_l);
	devlink_fmsg_u32_pair_put(fmsg, "task_id", winfo->task_id);
	devlink_fmsg_u32_pair_put(fmsg, "sp", winfo->sp);
	devlink_fmsg_u32_pair_put(fmsg, "stack_current_used", winfo->curr_used);
	devlink_fmsg_u32_pair_put(fmsg, "peak_used", winfo->peak_used);
	devlink_fmsg_u32_pair_put(fmsg, "\n Overflow_flag", winfo->is_overflow);
	devlink_fmsg_u32_pair_put(fmsg, "stack_top", winfo->stack_top);
	devlink_fmsg_u32_pair_put(fmsg, "stack_bottom", winfo->stack_bottom);
	devlink_fmsg_u32_pair_put(fmsg, "mgmt_pc", winfo->pc);
	devlink_fmsg_u32_pair_put(fmsg, "lr", winfo->lr);
	devlink_fmsg_u32_pair_put(fmsg, "cpsr", winfo->cpsr);
	devlink_fmsg_binary_pair_put(fmsg, "Mgmt register info", winfo->reg,
				     sizeof(winfo->reg));
	devlink_fmsg_binary_pair_put(fmsg, "Mgmt dump stack(start from sp)",
				     winfo->data, sizeof(winfo->data));
}

static int hinic_fw_reporter_dump(struct devlink_health_reporter *reporter,
@@ -528,7 +423,7 @@ static int hinic_fw_reporter_dump(struct devlink_health_reporter *reporter,
				  struct netlink_ext_ack *extack)
{
	if (priv_ctx)
		return mgmt_watchdog_report_show(fmsg, priv_ctx);
		mgmt_watchdog_report_show(fmsg, priv_ctx);

	return 0;
}
+133 −331

File changed.

Preview size limit exceeded, changes collapsed.

+12 −37
Original line number Diff line number Diff line
@@ -889,36 +889,16 @@ int mlx5_fw_tracer_trigger_core_dump_general(struct mlx5_core_dev *dev)
	return 0;
}

static int
static void
mlx5_devlink_fmsg_fill_trace(struct devlink_fmsg *fmsg,
			     struct mlx5_fw_trace_data *trace_data)
{
	int err;

	err = devlink_fmsg_obj_nest_start(fmsg);
	if (err)
		return err;

	err = devlink_fmsg_u64_pair_put(fmsg, "timestamp", trace_data->timestamp);
	if (err)
		return err;

	err = devlink_fmsg_bool_pair_put(fmsg, "lost", trace_data->lost);
	if (err)
		return err;

	err = devlink_fmsg_u8_pair_put(fmsg, "event_id", trace_data->event_id);
	if (err)
		return err;

	err = devlink_fmsg_string_pair_put(fmsg, "msg", trace_data->msg);
	if (err)
		return err;

	err = devlink_fmsg_obj_nest_end(fmsg);
	if (err)
		return err;
	return 0;
	devlink_fmsg_obj_nest_start(fmsg);
	devlink_fmsg_u64_pair_put(fmsg, "timestamp", trace_data->timestamp);
	devlink_fmsg_bool_pair_put(fmsg, "lost", trace_data->lost);
	devlink_fmsg_u8_pair_put(fmsg, "event_id", trace_data->event_id);
	devlink_fmsg_string_pair_put(fmsg, "msg", trace_data->msg);
	devlink_fmsg_obj_nest_end(fmsg);
}

int mlx5_fw_tracer_get_saved_traces_objects(struct mlx5_fw_tracer *tracer,
@@ -927,7 +907,6 @@ int mlx5_fw_tracer_get_saved_traces_objects(struct mlx5_fw_tracer *tracer,
	struct mlx5_fw_trace_data *straces = tracer->st_arr.straces;
	u32 index, start_index, end_index;
	u32 saved_traces_index;
	int err;

	if (!straces[0].timestamp)
		return -ENOMSG;
@@ -940,22 +919,18 @@ int mlx5_fw_tracer_get_saved_traces_objects(struct mlx5_fw_tracer *tracer,
		start_index = 0;
	end_index = (saved_traces_index - 1) & (SAVED_TRACES_NUM - 1);

	err = devlink_fmsg_arr_pair_nest_start(fmsg, "dump fw traces");
	if (err)
		goto unlock;
	devlink_fmsg_arr_pair_nest_start(fmsg, "dump fw traces");
	index = start_index;
	while (index != end_index) {
		err = mlx5_devlink_fmsg_fill_trace(fmsg, &straces[index]);
		if (err)
			goto unlock;
		mlx5_devlink_fmsg_fill_trace(fmsg, &straces[index]);

		index = (index + 1) & (SAVED_TRACES_NUM - 1);
	}

	err = devlink_fmsg_arr_pair_nest_end(fmsg);
unlock:
	devlink_fmsg_arr_pair_nest_end(fmsg);
	mutex_unlock(&tracer->st_arr.lock);
	return err;

	return 0;
}

static void mlx5_fw_tracer_update_db(struct work_struct *work)
Loading