| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <MW5PR11MB581193B29314BF9170D48D49DD5AA@MW5PR11MB5811.namprd11.prod.outlook.com>
Date: Wed, 14 Jun 2023 23:34:17 +0000
From: "Ertman, David M" <david.m.ertman@...el.com>
To: Brett Creeley <bcreeley@....com>, "intel-wired-lan@...ts.osuosl.org"
<intel-wired-lan@...ts.osuosl.org>
CC: "daniel.machon@...rochip.com" <daniel.machon@...rochip.com>,
"simon.horman@...igine.com" <simon.horman@...igine.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH iwl-next v4 06/10] ice: Flesh out implementation of
support for SRIOV on bonded interface
> -----Original Message-----
> From: Brett Creeley <bcreeley@....com>
> Sent: Wednesday, June 14, 2023 2:25 PM
> To: Ertman, David M <david.m.ertman@...el.com>; intel-wired-
> lan@...ts.osuosl.org
> Cc: daniel.machon@...rochip.com; simon.horman@...igine.com;
> netdev@...r.kernel.org
> Subject: Re: [PATCH iwl-next v4 06/10] ice: Flesh out implementation of
> support for SRIOV on bonded interface
>
> On 6/9/2023 2:16 PM, Dave Ertman wrote:
> > Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> >
> >
> > Add in the functions that will allow a VF created on the primary interface
> > of a bond to "fail-over" to another PF interface in the bond and continue
> > to Tx and Rx.
> >
> > Add in an ordered take-down path for the bonded interface.
> >
> > Reviewed-by: Daniel Machon <daniel.machon@...rochip.com>
> > Signed-off-by: Dave Ertman <david.m.ertman@...el.com>
> > ---
> > drivers/net/ethernet/intel/ice/ice_lag.c | 821
> ++++++++++++++++++++++-
> > 1 file changed, 811 insertions(+), 10 deletions(-)
> >
>
> [...]
>
> > +/**
> > + * ice_lag_qbuf_recfg - generate a buffer of queues for a reconfigure
> command
> > + * @hw: HW struct that contains the queue contexts
> > + * @qbuf: pointer to buffer to populate
> > + * @vsi_num: index of the VSI in PF space
> > + * @numq: number of queues to search for
> > + * @tc: traffic class that contains the queues
> > + *
> > + * function returns the number of valid queues in buffer
> > + */
> > +static u16
> > +ice_lag_qbuf_recfg(struct ice_hw *hw, struct ice_aqc_cfg_txqs_buf
> *qbuf,
> > + u16 vsi_num, u16 numq, u8 tc)
> > +{
> > + struct ice_q_ctx *q_ctx;
> > + u16 qid, count = 0;
> > + struct ice_pf *pf;
> > + int i;
> > +
> > + pf = hw->back;
> > + for (i = 0; i < numq; i++) {
> > + q_ctx = ice_get_lan_q_ctx(hw, vsi_num, tc, i);
> > + if (!q_ctx || q_ctx->q_handle == ICE_INVAL_Q_HANDLE) {
>
> Should q_ctx->q_teid be checked against ICE_INVAL_TEID as well? If so,
> the dev_dbg() should also be updated.
Reasonable - added check for ICE_INVAL_TEID and DBG statement.
>
> > + dev_dbg(ice_hw_to_dev(hw), "%s queue %d %s\n",
> __func__,
> > + i, q_ctx ? "INVAL Q HANDLE" : "NO Q CONTEXT");
> > + continue;
> > + }
> > +
> > + qid = pf->vsi[vsi_num]->txq_map[q_ctx->q_handle];
> > + qbuf->queue_info[count].q_handle = cpu_to_le16(qid);
> > + qbuf->queue_info[count].tc = tc;
> > + qbuf->queue_info[count].q_teid = cpu_to_le32(q_ctx->q_teid);
> > + count++;
> > + }
> > +
> > + return count;
> > +}
> > +
> > +/**
> > + * ice_lag_get_sched_parent - locate or create a sched node parent
> > + * @hw: HW struct for getting parent in
> > + * @tc: traffic class on parent/node
> > + */
> > +static struct ice_sched_node *
> > +ice_lag_get_sched_parent(struct ice_hw *hw, u8 tc)
> > +{
> > + struct ice_sched_node *tc_node, *aggnode, *parent = NULL;
> > + u16 num_nodes[ICE_AQC_TOPO_MAX_LEVEL_NUM] = { 0 };
> > + struct ice_port_info *pi = hw->port_info;
> > + struct device *dev;
> > + u8 aggl, vsil;
> > + int n;
> > +
> > + dev = ice_hw_to_dev(hw);
> > +
> > + tc_node = ice_sched_get_tc_node(pi, tc);
> > + if (!tc_node) {
> > + dev_warn(dev, "Failure to find TC node in for LAG move\n");
>
> Nit, but seems like there's a stray "in" in the log message?
Removed "in" in warning
>
> > + return parent;
> > + }
> > +
> > + aggnode = ice_sched_get_agg_node(pi, tc_node, ICE_DFLT_AGG_ID);
> > + if (!aggnode) {
> > + dev_warn(dev, "Failure to find aggregate node for LAG move\n");
> > + return parent;
> > + }
> > +
> > + aggl = ice_sched_get_agg_layer(hw);
> > + vsil = ice_sched_get_vsi_layer(hw);
> > +
> > + for (n = aggl + 1; n < vsil; n++)
> > + num_nodes[n] = 1;
> > +
> > + for (n = 0; n < aggnode->num_children; n++) {
> > + parent = ice_sched_get_free_vsi_parent(hw, aggnode-
> >children[n],
> > + num_nodes);
> > + if (parent)
> > + return parent;
> > + }
> > +
> > + /* if free parent not found - add one */
> > + parent = aggnode;
> > + for (n = aggl + 1; n < vsil; n++) {
> > + u16 num_nodes_added;
> > + u32 first_teid;
> > + int err;
> > +
> > + err = ice_sched_add_nodes_to_layer(pi, tc_node, parent, n,
> > + num_nodes[n], &first_teid,
> > + &num_nodes_added);
> > + if (err || num_nodes[n] != num_nodes_added)
> > + return NULL;
> > +
> > + if (num_nodes_added)
> > + parent = ice_sched_find_node_by_teid(tc_node,
> > + first_teid);
> > + else
> > + parent = parent->children[0];
> > + if (!parent) {
> > + dev_warn(dev, "Failure to add new parent for LAG move\n");
> > + return parent;
> > + }
> > + }
> > +
> > + return parent;
> > +}
> > +
>
> [...]
>
> > /**
> > @@ -625,6 +1321,73 @@ static void ice_lag_monitor_active(struct ice_lag
> *lag, void *ptr)
> > static bool
> > ice_lag_chk_comp(struct ice_lag *lag, void *ptr)
> > {
> > + struct net_device *event_netdev, *event_upper;
> > + struct netdev_notifier_bonding_info *info;
> > + struct netdev_bonding_info *bonding_info;
> > + struct list_head *tmp;
> > + int count = 0;
> > +
> > + if (!lag->primary)
> > + return true;
> > +
> > + event_netdev = netdev_notifier_info_to_dev(ptr);
> > + rcu_read_lock();
> > + event_upper = netdev_master_upper_dev_get_rcu(event_netdev);
> > + rcu_read_unlock();
> > + if (event_upper != lag->upper_netdev)
> > + return true;
> > +
> > + info = (struct netdev_notifier_bonding_info *)ptr;
> > + bonding_info = &info->bonding_info;
> > + lag->bond_mode = bonding_info->master.bond_mode;
> > + if (lag->bond_mode != BOND_MODE_ACTIVEBACKUP) {
> > + netdev_info(lag->netdev, "Bond Mode not ACTIVE-BACKUP\n");
> > + return false;
> > + }
> > +
> > + list_for_each(tmp, lag->netdev_head) {
> > +#if !defined(NO_DCB_SUPPORT) || defined(ADQ_SUPPORT)
> > + struct ice_dcbx_cfg *dcb_cfg, *peer_dcb_cfg;
> > +#endif /* !NO_DCB_SUPPORT || ADQ_SUPPORT */
>
> These #ifdefs don't belong here.
Removed them - thanks for catching this!
>
> > + struct ice_lag_netdev_list *entry;
> > + struct ice_netdev_priv *peer_np;
> > + struct net_device *peer_netdev;
> > + struct ice_vsi *vsi, *peer_vsi;
> > +
> > + entry = list_entry(tmp, struct ice_lag_netdev_list, node);
> > + peer_netdev = entry->netdev;
> > + if (!netif_is_ice(peer_netdev)) {
> > + netdev_info(lag->netdev, "Found non-ice netdev in LAG\n");
> > + return false;
> > + }
> > +
> > + count++;
> > + if (count > 2) {
> > + netdev_info(lag->netdev, "Found more than two netdevs in
> LAG\n");
> > + return false;
> > + }
> > +
> > + peer_np = netdev_priv(peer_netdev);
> > + vsi = ice_get_main_vsi(lag->pf);
> > + peer_vsi = peer_np->vsi;
> > + if (lag->pf->pdev->bus != peer_vsi->back->pdev->bus ||
> > + lag->pf->pdev->slot != peer_vsi->back->pdev->slot) {
> > + netdev_info(lag->netdev, "Found netdev on different device
> in LAG\n");
> > + return false;
> > + }
> > +
> > +#if !defined(NO_DCB_SUPPORT) || defined(ADQ_SUPPORT)
> > + dcb_cfg = &vsi->port_info->qos_cfg.local_dcbx_cfg;
> > + peer_dcb_cfg = &peer_vsi->port_info->qos_cfg.local_dcbx_cfg;
> > + if (memcmp(dcb_cfg, peer_dcb_cfg,
> > + sizeof(struct ice_dcbx_cfg))) {
> > + netdev_info(lag->netdev, "Found netdev with different DCB
> config in LAG\n");
> > + return false;
> > + }
> > +
> > +#endif /* !NO_DCB_SUPPORT || ADQ_SUPPORT */
>
> Ditto on the #ifdefs
Also removed
>
> > + }
> > +
> > return true;
> > }
> >
>
> [...]
Powered by blists - more mailing lists