[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230607100825.igma5yifszm327nk@DEN-LT-70577>
Date: Wed, 7 Jun 2023 10:08:25 +0000
From: Daniel Machon <daniel.machon@...rochip.com>
To: Dave Ertman <david.m.ertman@...el.com>
CC: <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net v2 10/10] ice: update reset path for SRIOV LAG support
> Add code to rebuild the LAG resources when rebuilding the state of the
> interface after a reset.
>
> Also added in a function for building per-queue information into the buffer
> used to configure VF queues for LAG fail-over. This improves code reuse.
>
> Due to differences in timing per interface for recovering from a reset, add
> in the ability to retry on non-local dependencies where needed.
>
> Signed-off-by: Dave Ertman <david.m.ertman@...el.com>
> ---
> drivers/net/ethernet/intel/ice/ice_lag.c | 287 +++++++++++++++++++++-
> drivers/net/ethernet/intel/ice/ice_lag.h | 3 +
> drivers/net/ethernet/intel/ice/ice_main.c | 14 +-
> 3 files changed, 300 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
> index ffad9f3a5576..4c07d1b9e338 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
> @@ -997,6 +997,7 @@ static void ice_lag_link_unlink(struct ice_lag *lag, void *ptr)
> * @link: Is this a linking activity
> *
> * If link is false, then primary_swid should be expected to not be valid
> + * This function should never be called in interrupt context.
> */
> static void
> ice_lag_set_swid(u16 primary_swid, struct ice_lag *local_lag,
> @@ -1006,7 +1007,7 @@ ice_lag_set_swid(u16 primary_swid, struct ice_lag *local_lag,
> struct ice_aqc_set_port_params *cmd;
> struct ice_aq_desc desc;
> u16 buf_len, swid;
> - int status;
> + int status, i;
>
> buf_len = struct_size(buf, elem, 1);
> buf = kzalloc(buf_len, GFP_KERNEL);
> @@ -1057,7 +1058,20 @@ ice_lag_set_swid(u16 primary_swid, struct ice_lag *local_lag,
> ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_set_port_params);
>
> cmd->swid = cpu_to_le16(ICE_AQC_PORT_SWID_VALID | swid);
> - status = ice_aq_send_cmd(&local_lag->pf->hw, &desc, NULL, 0, NULL);
> + /* If this is happening in reset context, it is possible that the
> + * primary interface has not finished setting its SWID to SHARED
> + * yet. Allow retries to account for this timing issue between
> + * interfaces.
> + */
> + for (i = 0; i < ICE_LAG_RESET_RETRIES; i++) {
> + status = ice_aq_send_cmd(&local_lag->pf->hw, &desc, NULL, 0,
> + NULL);
> + if (!status)
> + break;
> +
> + usleep_range(1000, 2000);
> + }
> +
> if (status)
> dev_err(ice_pf_to_dev(local_lag->pf), "Error setting SWID in port params %d\n",
> status);
> @@ -1065,7 +1079,7 @@ ice_lag_set_swid(u16 primary_swid, struct ice_lag *local_lag,
>
> /**
> * ice_lag_primary_swid - set/clear the SHARED attrib of primary's SWID
> - * @lag: primary interfaces lag struct
> + * @lag: primary interface's lag struct
> * @link: is this a linking activity
> *
> * Implement setting primary SWID as shared using 0x020B
> @@ -1788,6 +1802,191 @@ static u16 ice_create_lag_recipe(struct ice_hw *hw, const u8 *base_recipe,
> return rid;
> }
>
> +/**
> + * ice_lag_move_vf_nodes_tc_sync - move a VF's nodes for a tc during reset
> + * @lag: primary interfaces lag struct
> + * @dest_hw: HW struct for destination's interface
> + * @vsi_num: VSI index in PF space
> + * @tc: traffic class to move
> + */
> +static void
> +ice_lag_move_vf_nodes_tc_sync(struct ice_lag *lag, struct ice_hw *dest_hw,
> + u16 vsi_num, u8 tc)
> +{
> + u16 num_nodes[ICE_AQC_TOPO_MAX_LEVEL_NUM] = { 0 };
> + struct ice_sched_node *n_prt, *tc_node, *aggnode;
> + u16 numq, valq, buf_size, num_moved, qbuf_size;
> + struct device *dev = ice_pf_to_dev(lag->pf);
> + struct ice_aqc_cfg_txqs_buf *qbuf;
> + struct ice_aqc_move_elem *buf;
> + struct ice_port_info *pi;
> + __le32 teid, parent_teid;
> + struct ice_vsi_ctx *ctx;
> + struct ice_hw *hw;
> + u8 aggl, vsil;
> + u32 tmp_teid;
> + int n;
> +
> + hw = &lag->pf->hw;
> + ctx = ice_get_vsi_ctx(hw, vsi_num);
> + if (!ctx) {
> + dev_warn(dev, "LAG rebuild failed after reset due to VSI Context failure\n");
> + return;
> + }
> +
> + if (!ctx->sched.vsi_node[tc])
> + return;
> +
> + numq = ctx->num_lan_q_entries[tc];
> + teid = ctx->sched.vsi_node[tc]->info.node_teid;
> + tmp_teid = le32_to_cpu(teid);
> + parent_teid = ctx->sched.vsi_node[tc]->info.parent_teid;
> +
> + if (!tmp_teid || !numq)
> + return;
> +
> + if (ice_sched_suspend_resume_elems(hw, 1, &tmp_teid, true))
> + dev_dbg(dev, "Problem suspending traffic during reset rebuild\n");
> +
> + /* reconfig queues for new port */
> + qbuf_size = struct_size(qbuf, queue_info, numq);
> + qbuf = kzalloc(qbuf_size, GFP_KERNEL);
> + if (!qbuf) {
> + dev_warn(dev, "Failure allocating VF queue recfg buffer for reset rebuild\n");
> + goto resume_sync;
> + }
> +
> + /* add the per queue info for the reconfigure command buffer */
> + valq = ice_lag_qbuf_recfg(hw, qbuf, vsi_num, numq, tc);
> + if (!valq) {
> + dev_warn(dev, "Failure to reconfig queues for LAG reset rebuild\n");
> + goto sync_none;
> + }
> +
> + if (ice_aq_cfg_lan_txq(hw, qbuf, qbuf_size, numq, hw->port_info->lport,
> + dest_hw->port_info->lport, NULL)) {
> + dev_warn(dev, "Failure to configure queues for LAG reset rebuild\n");
> + goto sync_qerr;
> + }
> +
> +sync_none:
> + kfree(qbuf);
> +
> + /* find parent in destination tree */
> + pi = dest_hw->port_info;
> + tc_node = ice_sched_get_tc_node(pi, tc);
> + if (!tc_node) {
> + dev_warn(dev, "Failure to find TC node in secondary tree for reset rebuild\n");
> + goto resume_sync;
> + }
> +
> + aggnode = ice_sched_get_agg_node(pi, tc_node, ICE_DFLT_AGG_ID);
> + if (!aggnode) {
> + dev_warn(dev, "Failure to find agg node in secondary tree for reset rebuild\n");
> + goto resume_sync;
> + }
> +
> + aggl = ice_sched_get_agg_layer(dest_hw);
> + vsil = ice_sched_get_vsi_layer(dest_hw);
> +
> + for (n = aggl + 1; n < vsil; n++)
> + num_nodes[n] = 1;
> +
> + for (n = 0; n < aggnode->num_children; n++) {
> + n_prt = ice_sched_get_free_vsi_parent(dest_hw,
> + aggnode->children[n],
> + num_nodes);
> + if (n_prt)
> + break;
> + }
> +
> + /* if no free parent found - add one */
> + if (!n_prt) {
> + u16 num_nodes_added;
> + u32 first_teid;
> + int status;
> +
> + n_prt = aggnode;
> + for (n = aggl + 1; n < vsil; n++) {
> + status = ice_sched_add_nodes_to_layer(pi, tc_node,
> + n_prt, n,
> + num_nodes[n],
> + &first_teid,
> + &num_nodes_added);
> + if (status || num_nodes[n] != num_nodes_added)
> + goto resume_sync;
> +
> + if (num_nodes_added)
> + n_prt = ice_sched_find_node_by_teid(tc_node,
> + first_teid);
> + else
> + n_prt = n_prt->children[0];
> +
> + if (!n_prt) {
> + dev_warn(dev, "Failure to add new parent for LAG reset rebuild\n");
> + goto resume_sync;
> + }
> + }
> + }
> +
> + /* Move node to new parent */
> + buf_size = struct_size(buf, teid, 1);
> + buf = kzalloc(buf_size, GFP_KERNEL);
> + if (!buf) {
> + dev_warn(dev, "Failure to alloc for VF node move in reset rebuild\n");
> + goto resume_sync;
> + }
> +
> + buf->hdr.src_parent_teid = parent_teid;
> + buf->hdr.dest_parent_teid = n_prt->info.node_teid;
> + buf->hdr.num_elems = cpu_to_le16(1);
> + buf->hdr.mode = ICE_AQC_MOVE_ELEM_MODE_KEEP_OWN;
> + buf->teid[0] = teid;
> +
> + if (ice_aq_move_sched_elems(&lag->pf->hw, 1, buf, buf_size, &num_moved,
> + NULL))
> + dev_warn(dev, "Failure to move VF nodes for LAG reset rebuild\n");
> + else
> + ice_sched_update_parent(n_prt, ctx->sched.vsi_node[tc]);
> +
> + kfree(buf);
> + goto resume_sync;
> +
> +sync_qerr:
> + kfree(qbuf);
> +
> +resume_sync:
> + if (ice_sched_suspend_resume_elems(hw, 1, &tmp_teid, false))
> + dev_warn(dev, "Problem restarting traffic for LAG node reset rebuild\n");
> +}
This function looks suspiciously similar to ice_lag_move_vf_node_tc() in
patch #6 :-). Maybe theres room for moving some common code into
separate functions.
> +
> +/**
> + * ice_lag_move_vf_nodes_sync - move vf nodes to active interface
> + * @lag: primary interfaces lag struct
> + * @dest_hw: lport value for currently active port
> + *
> + * This function is used in a reset context, outside of event handling,
> + * to move the VF nodes to the secondary interface when that interface
> + * is the active interface during a reset rebuild
> + */
> +static void
> +ice_lag_move_vf_nodes_sync(struct ice_lag *lag, struct ice_hw *dest_hw)
> +{
> + struct ice_pf *pf;
> + int i, tc;
> +
> + if (!lag->primary || !dest_hw)
> + return;
> +
> + pf = lag->pf;
> + ice_for_each_vsi(pf, i)
> + if (pf->vsi[i] && (pf->vsi[i]->type == ICE_VSI_VF ||
> + pf->vsi[i]->type == ICE_VSI_SWITCHDEV_CTRL))
> + ice_for_each_traffic_class(tc)
> + ice_lag_move_vf_nodes_tc_sync(lag, dest_hw, i,
> + tc);
> +}
> +
> /**
> * ice_init_lag - initialize support for LAG
> * @pf: PF struct
> @@ -1890,3 +2089,85 @@ void ice_deinit_lag(struct ice_pf *pf)
>
> pf->lag = NULL;
> }
> +
> +/**
> + * ice_lag_rebuild - rebuild lag resources after reset
> + * @pf: pointer to local pf struct
> + *
> + * PF resets are promoted to CORER resets when interface in an aggregate. This
> + * means that we need to rebuild the PF resources for the interface. Since
> + * this will happen outside the normal event processing, need to acquire the lag
> + * lock.
> + *
> + * This function will also evaluate the VF resources if this is the primary
> + * interface.
> + */
> +void ice_lag_rebuild(struct ice_pf *pf)
> +{
> + struct ice_lag_netdev_list ndlist;
> + struct ice_lag *lag, *prim_lag;
> + struct list_head *tmp, *n;
> + u8 act_port, loc_port;
> +
> + if (!pf->lag || !pf->lag->bonded)
> + return;
> +
> + mutex_lock(&pf->lag_mutex);
> +
> + lag = pf->lag;
> + if (lag->primary) {
> + prim_lag = lag;
> + } else {
> + struct ice_lag_netdev_list *nl;
> + struct net_device *tmp_nd;
> +
> + INIT_LIST_HEAD(&ndlist.node);
> + rcu_read_lock();
> + for_each_netdev_in_bond_rcu(lag->upper_netdev, tmp_nd) {
> + nl = kzalloc(sizeof(*nl), GFP_KERNEL);
> + if (!nl)
> + break;
> +
> + nl->netdev = tmp_nd;
> + list_add(&nl->node, &ndlist.node);
> + }
> + rcu_read_unlock();
> + lag->netdev_head = &ndlist.node;
> + prim_lag = ice_lag_find_primary(lag);
> + }
> +
> + if (!prim_lag) {
> + dev_dbg(ice_pf_to_dev(pf), "No primary interface in aggregate, can't rebuild\n");
> + goto lag_rebuild_out;
> + }
> +
> + act_port = prim_lag->active_port;
> + loc_port = lag->pf->hw.port_info->lport;
> +
> + /* configure SWID for this port */
> + if (lag->primary) {
> + ice_lag_primary_swid(lag, true);
> + } else {
> + ice_lag_set_swid(prim_lag->pf->hw.port_info->sw_id, lag, true);
> + ice_lag_add_prune_list(prim_lag, pf);
> + if (act_port == loc_port)
> + ice_lag_move_vf_nodes_sync(prim_lag, &pf->hw);
> + }
> +
> + ice_lag_cfg_cp_fltr(lag, true);
> +
> + if (lag->pf_rule_id)
> + if (ice_lag_cfg_dflt_fltr(lag, true))
> + dev_err(ice_pf_to_dev(pf), "Error adding default VSI rule in rebuild\n");
> +
> + ice_clear_rdma_cap(pf);
> +lag_rebuild_out:
> + list_for_each_safe(tmp, n, &ndlist.node) {
> + struct ice_lag_netdev_list *entry;
> +
> + entry = list_entry(tmp, struct ice_lag_netdev_list, node);
> + list_del(&entry->node);
> + kfree(entry);
> + }
> + mutex_unlock(&pf->lag_mutex);
> +}
> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.h b/drivers/net/ethernet/intel/ice/ice_lag.h
> index df4af5184a75..18075b82485a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lag.h
> +++ b/drivers/net/ethernet/intel/ice/ice_lag.h
> @@ -16,6 +16,8 @@ enum ice_lag_role {
>
> #define ICE_LAG_INVALID_PORT 0xFF
>
> +#define ICE_LAG_RESET_RETRIES 5
> +
> struct ice_pf;
> struct ice_vf;
>
> @@ -59,4 +61,5 @@ struct ice_lag_work {
> void ice_lag_move_new_vf_nodes(struct ice_vf *vf);
> int ice_init_lag(struct ice_pf *pf);
> void ice_deinit_lag(struct ice_pf *pf);
> +void ice_lag_rebuild(struct ice_pf *pf);
> #endif /* _ICE_LAG_H_ */
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 7030b2e54d2b..a27381ec37cd 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -636,6 +636,11 @@ static void ice_do_reset(struct ice_pf *pf, enum ice_reset_req reset_type)
>
> dev_dbg(dev, "reset_type 0x%x requested\n", reset_type);
>
> + if (pf->lag && pf->lag->bonded && reset_type == ICE_RESET_PFR) {
> + dev_dbg(dev, "PFR on a bonded interface, promoting to CORER\n");
> + reset_type = ICE_RESET_CORER;
> + }
> +
> ice_prepare_for_reset(pf, reset_type);
>
> /* trigger the reset */
> @@ -719,8 +724,13 @@ static void ice_reset_subtask(struct ice_pf *pf)
> }
>
> /* No pending resets to finish processing. Check for new resets */
> - if (test_bit(ICE_PFR_REQ, pf->state))
> + if (test_bit(ICE_PFR_REQ, pf->state)) {
> reset_type = ICE_RESET_PFR;
> + if (pf->lag && pf->lag->bonded) {
> + dev_dbg(ice_pf_to_dev(pf), "PFR on a bonded interface, promoting to CORER\n");
> + reset_type = ICE_RESET_CORER;
> + }
> + }
> if (test_bit(ICE_CORER_REQ, pf->state))
> reset_type = ICE_RESET_CORER;
> if (test_bit(ICE_GLOBR_REQ, pf->state))
> @@ -7421,6 +7431,8 @@ static void ice_rebuild(struct ice_pf *pf, enum ice_reset_req reset_type)
> clear_bit(ICE_RESET_FAILED, pf->state);
>
> ice_plug_aux_dev(pf);
> + if (ice_is_feature_supported(pf, ICE_F_SRIOV_LAG))
> + ice_lag_rebuild(pf);
> return;
>
> err_vsi_rebuild:
> --
> 2.40.1
>
>
Powered by blists - more mailing lists