Skip to content

Commit

Permalink
qede: Revise state locking scheme
Browse files Browse the repository at this point in the history
As qede utilizes an internal-reload sequence as result of various
configuration changes, the netif state wouldn't always accurately describe
the status of the configuration.
To compensate, we're storing an internal state of the device, which should
only be accessed under the qede_lock.

This patch fixes and improves several state/lock interactions:
  - The internal state should only be checked while locked.
  - While holding lock, it's preferable to check state rather than
    the netdevice's state.
  - The reload sequence is not 'atomic' - unload and subsequent load
    are not in the same critical section.

This also add the 'locked' variant for the reload, which would later be
used by XDP - useful in the case where the correct sequence is 'lock,
check state and re-configure if good', instead of allowing the reload
itself to make the decision regarding the configurability of the device.

Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Mintz, Yuval authored and David S. Miller committed Nov 30, 2016
1 parent f4fad34 commit 567b3c1
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 75 deletions.
14 changes: 9 additions & 5 deletions drivers/net/ethernet/qlogic/qede/qede.h
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,12 @@ struct qede_fastpath {
#define QEDE_SP_VXLAN_PORT_CONFIG 2
#define QEDE_SP_GENEVE_PORT_CONFIG 3

union qede_reload_args {
u16 mtu;
struct qede_reload_args {
void (*func)(struct qede_dev *edev, struct qede_reload_args *args);
union {
netdev_features_t features;
u16 mtu;
} u;
};

#ifdef CONFIG_DCB
Expand All @@ -348,11 +352,11 @@ void qede_set_dcbnl_ops(struct net_device *ndev);
void qede_config_debug(uint debug, u32 *p_dp_module, u8 *p_dp_level);
void qede_set_ethtool_ops(struct net_device *netdev);
void qede_reload(struct qede_dev *edev,
void (*func)(struct qede_dev *edev,
union qede_reload_args *args),
union qede_reload_args *args);
struct qede_reload_args *args, bool is_locked);
int qede_change_mtu(struct net_device *dev, int new_mtu);
void qede_fill_by_demand_stats(struct qede_dev *edev);
void __qede_lock(struct qede_dev *edev);
void __qede_unlock(struct qede_dev *edev);
bool qede_has_rx_work(struct qede_rx_queue *rxq);
int qede_txq_has_work(struct qede_tx_queue *txq);
void qede_recycle_rx_bd_ring(struct qede_rx_queue *rxq, struct qede_dev *edev,
Expand Down
37 changes: 20 additions & 17 deletions drivers/net/ethernet/qlogic/qede/qede_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,9 @@ static void qede_get_ethtool_stats(struct net_device *dev,

qede_fill_by_demand_stats(edev);

mutex_lock(&edev->qede_lock);
/* Need to protect the access to the fastpath array */
__qede_lock(edev);

for (i = 0; i < QEDE_QUEUE_CNT(edev); i++) {
fp = &edev->fp_array[i];

Expand All @@ -278,7 +280,7 @@ static void qede_get_ethtool_stats(struct net_device *dev,
buf++;
}

mutex_unlock(&edev->qede_lock);
__qede_unlock(edev);
}

static int qede_get_sset_count(struct net_device *dev, int stringset)
Expand Down Expand Up @@ -374,6 +376,8 @@ static int qede_get_link_ksettings(struct net_device *dev,
struct qede_dev *edev = netdev_priv(dev);
struct qed_link_output current_link;

__qede_lock(edev);

memset(&current_link, 0, sizeof(current_link));
edev->ops->common->get_link(edev->cdev, &current_link);

Expand All @@ -393,6 +397,9 @@ static int qede_get_link_ksettings(struct net_device *dev,
base->speed = SPEED_UNKNOWN;
base->duplex = DUPLEX_UNKNOWN;
}

__qede_unlock(edev);

base->port = current_link.port;
base->autoneg = (current_link.autoneg) ? AUTONEG_ENABLE :
AUTONEG_DISABLE;
Expand Down Expand Up @@ -701,8 +708,7 @@ static int qede_set_ringparam(struct net_device *dev,
edev->q_num_rx_buffers = ering->rx_pending;
edev->q_num_tx_buffers = ering->tx_pending;

if (netif_running(edev->ndev))
qede_reload(edev, NULL, NULL);
qede_reload(edev, NULL, false);

return 0;
}
Expand Down Expand Up @@ -787,29 +793,27 @@ static int qede_get_regs_len(struct net_device *ndev)
return -EINVAL;
}

static void qede_update_mtu(struct qede_dev *edev, union qede_reload_args *args)
static void qede_update_mtu(struct qede_dev *edev,
struct qede_reload_args *args)
{
edev->ndev->mtu = args->mtu;
edev->ndev->mtu = args->u.mtu;
}

/* Netdevice NDOs */
int qede_change_mtu(struct net_device *ndev, int new_mtu)
{
struct qede_dev *edev = netdev_priv(ndev);
union qede_reload_args args;
struct qede_reload_args args;

DP_VERBOSE(edev, (NETIF_MSG_IFUP | NETIF_MSG_IFDOWN),
"Configuring MTU size of %d\n", new_mtu);

/* Set the mtu field and re-start the interface if needed*/
args.mtu = new_mtu;

if (netif_running(edev->ndev))
qede_reload(edev, &qede_update_mtu, &args);

qede_update_mtu(edev, &args);
/* Set the mtu field and re-start the interface if needed */
args.u.mtu = new_mtu;
args.func = &qede_update_mtu;
qede_reload(edev, &args, false);

edev->ops->common->update_mtu(edev->cdev, args.mtu);
edev->ops->common->update_mtu(edev->cdev, new_mtu);

return 0;
}
Expand Down Expand Up @@ -893,8 +897,7 @@ static int qede_set_channels(struct net_device *dev,
sizeof(edev->rss_params.rss_ind_table));
}

if (netif_running(dev))
qede_reload(edev, NULL, NULL);
qede_reload(edev, NULL, false);

return 0;
}
Expand Down
Loading

0 comments on commit 567b3c1

Please sign in to comment.