From 2f8b6161cca5fb34b0065e2eac8bb2b61b7bfe87 Mon Sep 17 00:00:00 2001 From: Dima Chumak Date: Wed, 30 Jun 2021 14:56:32 +0300 Subject: [PATCH 01/14] net/mlx5: Lag, fix multipath lag activation When handling FIB_EVENT_ENTRY_REPLACE event for a new multipath route, lag activation can be missed if a stale (struct lag_mp)->mfi pointer exists, which was associated with an older multipath route that had been removed. Normally, when a route is removed, it triggers mlx5_lag_fib_event(), which handles FIB_EVENT_ENTRY_DEL and clears mfi pointer. But, if mlx5_lag_check_prereq() condition isn't met, for example when eswitch is in legacy mode, the fib event is skipped and mfi pointer becomes stale. Fix by resetting mfi pointer to NULL in mlx5_deactivate_lag(). Fixes: 8a66e4585979 ("net/mlx5: Change ownership model for lag") Signed-off-by: Dima Chumak Reviewed-by: Roi Dayan Reviewed-by: Mark Bloch Signed-off-by: Saeed Mahameed --- drivers/net/ethernet/mellanox/mlx5/core/lag.c | 1 + drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c | 8 ++++++++ drivers/net/ethernet/mellanox/mlx5/core/lag_mp.h | 2 ++ 3 files changed, 11 insertions(+) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag.c index 5c043c5cc403..40ef60f562b4 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/lag.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag.c @@ -277,6 +277,7 @@ static int mlx5_deactivate_lag(struct mlx5_lag *ldev) int err; ldev->flags &= ~MLX5_LAG_MODE_FLAGS; + mlx5_lag_mp_reset(ldev); MLX5_SET(destroy_lag_in, in, opcode, MLX5_CMD_OP_DESTROY_LAG); err = mlx5_cmd_exec_in(dev0, destroy_lag, in); diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c b/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c index c4bf8b679541..516bfc2bd797 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c @@ -302,6 +302,14 @@ static int mlx5_lag_fib_event(struct notifier_block *nb, return NOTIFY_DONE; } +void mlx5_lag_mp_reset(struct mlx5_lag *ldev) +{ + /* Clear mfi, as it might become stale when a route delete event + * has been missed, see mlx5_lag_fib_route_event(). + */ + ldev->lag_mp.mfi = NULL; +} + int mlx5_lag_mp_init(struct mlx5_lag *ldev) { struct lag_mp *mp = &ldev->lag_mp; diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.h b/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.h index 258ac7b2964e..729c839397a8 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.h @@ -21,11 +21,13 @@ struct lag_mp { #ifdef CONFIG_MLX5_ESWITCH +void mlx5_lag_mp_reset(struct mlx5_lag *ldev); int mlx5_lag_mp_init(struct mlx5_lag *ldev); void mlx5_lag_mp_cleanup(struct mlx5_lag *ldev); #else /* CONFIG_MLX5_ESWITCH */ +static inline void mlx5_lag_mp_reset(struct mlx5_lag *ldev) {}; static inline int mlx5_lag_mp_init(struct mlx5_lag *ldev) { return 0; } static inline void mlx5_lag_mp_cleanup(struct mlx5_lag *ldev) {} From 8e7e2e8ed0e251138926838b7933f8eb6dd56b12 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Sat, 21 Aug 2021 15:05:11 +0300 Subject: [PATCH 02/14] net/mlx5: Remove all auxiliary devices at the unregister event The call to mlx5_unregister_device() means that mlx5_core driver is removed. In such scenario, we need to disregard all other flags like attach/detach and forcibly remove all auxiliary devices. Fixes: a5ae8fc9058e ("net/mlx5e: Don't create devices during unload flow") Tested-and-Reported-by: Yicong Yang Signed-off-by: Leon Romanovsky Signed-off-by: Saeed Mahameed --- drivers/net/ethernet/mellanox/mlx5/core/dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dev.c b/drivers/net/ethernet/mellanox/mlx5/core/dev.c index def2156e50ee..20bb37266254 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/dev.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/dev.c @@ -397,7 +397,7 @@ int mlx5_register_device(struct mlx5_core_dev *dev) void mlx5_unregister_device(struct mlx5_core_dev *dev) { mutex_lock(&mlx5_intf_mutex); - dev->priv.flags |= MLX5_PRIV_FLAGS_DISABLE_ALL_ADEV; + dev->priv.flags = MLX5_PRIV_FLAGS_DISABLE_ALL_ADEV; mlx5_rescan_drivers_locked(dev); mutex_unlock(&mlx5_intf_mutex); } From 9a5f9cc794e17cf6ed2a5bb215d2e8b6832db444 Mon Sep 17 00:00:00 2001 From: Roi Dayan Date: Sun, 22 Aug 2021 10:14:58 +0300 Subject: [PATCH 03/14] net/mlx5e: Fix possible use-after-free deleting fdb rule After neigh-update-add failure we are still with a slow path rule but the driver always assume the rule is an fdb rule. Fix neigh-update-del by checking slow path tc flag on the flow. Also fix neigh-update-add for when neigh-update-del fails the same. Fixes: 5dbe906ff1d5 ("net/mlx5e: Use a slow path rule instead if vxlan neighbour isn't available") Signed-off-by: Roi Dayan Reviewed-by: Paul Blakey Signed-off-by: Saeed Mahameed --- drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c index 2e846b741280..1c44c6c345f5 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c @@ -147,7 +147,7 @@ void mlx5e_tc_encap_flows_add(struct mlx5e_priv *priv, mlx5e_rep_queue_neigh_stats_work(priv); list_for_each_entry(flow, flow_list, tmp_list) { - if (!mlx5e_is_offloaded_flow(flow)) + if (!mlx5e_is_offloaded_flow(flow) || !flow_flag_test(flow, SLOW)) continue; attr = flow->attr; esw_attr = attr->esw_attr; @@ -188,7 +188,7 @@ void mlx5e_tc_encap_flows_del(struct mlx5e_priv *priv, int err; list_for_each_entry(flow, flow_list, tmp_list) { - if (!mlx5e_is_offloaded_flow(flow)) + if (!mlx5e_is_offloaded_flow(flow) || flow_flag_test(flow, SLOW)) continue; attr = flow->attr; esw_attr = attr->esw_attr; From ca6891f9b27db7764bba0798202b0a21d0dc909c Mon Sep 17 00:00:00 2001 From: Maor Dickman Date: Thu, 12 Aug 2021 14:30:39 +0300 Subject: [PATCH 04/14] net/mlx5: E-Switch, Set vhca id valid flag when creating indir fwd group When indirect forward group is created, flow is added with vhca id but without setting vhca id valid flag which violates the PRM. Fix by setting the missing flag, vhca id valid. Fixes: 34ca65352ddf ("net/mlx5: E-Switch, Indirect table infrastructure") Signed-off-by: Maor Dickman Reviewed-by: Roi Dayan Signed-off-by: Saeed Mahameed --- drivers/net/ethernet/mellanox/mlx5/core/esw/indir_table.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/indir_table.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/indir_table.c index 3da7becc1069..425c91814b34 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/indir_table.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/indir_table.c @@ -364,6 +364,7 @@ static int mlx5_create_indir_fwd_group(struct mlx5_eswitch *esw, dest.type = MLX5_FLOW_DESTINATION_TYPE_VPORT; dest.vport.num = e->vport; dest.vport.vhca_id = MLX5_CAP_GEN(esw->dev, vhca_id); + dest.vport.flags = MLX5_FLOW_DEST_VPORT_VHCA_ID; e->fwd_rule = mlx5_add_flow_rules(e->ft, spec, &flow_act, &dest, 1); if (IS_ERR(e->fwd_rule)) { mlx5_destroy_flow_group(e->fwd_grp); From f9d196bd632b8b79261ec3366c30ec3923ea9a02 Mon Sep 17 00:00:00 2001 From: Dmytro Linkin Date: Thu, 24 Jun 2021 13:37:36 +0300 Subject: [PATCH 05/14] net/mlx5e: Use correct eswitch for stack devices with lag If link aggregation is used within stack devices driver rejects encap rules if PF of the VF tunnel device is down. This happens because route resolved for other PF and its eswitch instance is used to determine correct vport. To fix that use devcom feature to retrieve other eswitch instance if failed to find vport for the 1st eswitch and LAG is active. Fixes: 10742efc20a4 ("net/mlx5e: VF tunnel TX traffic offloading") Signed-off-by: Dmytro Linkin Reviewed-by: Roi Dayan Signed-off-by: Saeed Mahameed --- .../net/ethernet/mellanox/mlx5/core/en_tc.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c index d273758255c3..6eba574c5a36 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c @@ -1338,6 +1338,7 @@ bool mlx5e_tc_is_vf_tunnel(struct net_device *out_dev, struct net_device *route_ int mlx5e_tc_query_route_vport(struct net_device *out_dev, struct net_device *route_dev, u16 *vport) { struct mlx5e_priv *out_priv, *route_priv; + struct mlx5_devcom *devcom = NULL; struct mlx5_core_dev *route_mdev; struct mlx5_eswitch *esw; u16 vhca_id; @@ -1349,7 +1350,24 @@ int mlx5e_tc_query_route_vport(struct net_device *out_dev, struct net_device *ro route_mdev = route_priv->mdev; vhca_id = MLX5_CAP_GEN(route_mdev, vhca_id); + if (mlx5_lag_is_active(out_priv->mdev)) { + /* In lag case we may get devices from different eswitch instances. + * If we failed to get vport num, it means, mostly, that we on the wrong + * eswitch. + */ + err = mlx5_eswitch_vhca_id_to_vport(esw, vhca_id, vport); + if (err != -ENOENT) + return err; + + devcom = out_priv->mdev->priv.devcom; + esw = mlx5_devcom_get_peer_data(devcom, MLX5_DEVCOM_ESW_OFFLOADS); + if (!esw) + return -ENODEV; + } + err = mlx5_eswitch_vhca_id_to_vport(esw, vhca_id, vport); + if (devcom) + mlx5_devcom_release_peer_data(devcom, MLX5_DEVCOM_ESW_OFFLOADS); return err; } From 6cc64770fb386b10a64a1fe09328396de7bb5262 Mon Sep 17 00:00:00 2001 From: Wentao_Liang Date: Thu, 19 Aug 2021 22:30:05 +0800 Subject: [PATCH 06/14] net/mlx5: DR, fix a potential use-after-free bug In line 849 (#1), "mlx5dr_htbl_put(cur_htbl);" drops the reference to cur_htbl and may cause cur_htbl to be freed. However, cur_htbl is subsequently used in the next line, which may result in an use-after-free bug. Fix this by calling mlx5dr_err() before the cur_htbl is put. Signed-off-by: Wentao_Liang Signed-off-by: Saeed Mahameed --- drivers/net/ethernet/mellanox/mlx5/core/steering/dr_rule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_rule.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_rule.c index 43356fad53de..ffdfb5a94b14 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_rule.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_rule.c @@ -846,9 +846,9 @@ again: new_htbl = dr_rule_rehash(rule, nic_rule, cur_htbl, ste_location, send_ste_list); if (!new_htbl) { - mlx5dr_htbl_put(cur_htbl); mlx5dr_err(dmn, "Failed creating rehash table, htbl-log_size: %d\n", cur_htbl->chunk_size); + mlx5dr_htbl_put(cur_htbl); } else { cur_htbl = new_htbl; } From d0efb16294d145d157432feda83877ae9d7cdf37 Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Thu, 26 Aug 2021 12:46:01 -0700 Subject: [PATCH 07/14] net: don't unconditionally copy_from_user a struct ifreq for socket ioctls A common implementation of isatty(3) involves calling a ioctl passing a dummy struct argument and checking whether the syscall failed -- bionic and glibc use TCGETS (passing a struct termios), and musl uses TIOCGWINSZ (passing a struct winsize). If the FD is a socket, we will copy sizeof(struct ifreq) bytes of data from the argument and return -EFAULT if that fails. The result is that the isatty implementations may return a non-POSIX-compliant value in errno in the case where part of the dummy struct argument is inaccessible, as both struct termios and struct winsize are smaller than struct ifreq (at least on arm64). Although there is usually enough stack space following the argument on the stack that this did not present a practical problem up to now, with MTE stack instrumentation it's more likely for the copy to fail, as the memory following the struct may have a different tag. Fix the problem by adding an early check for whether the ioctl is a valid socket ioctl, and return -ENOTTY if it isn't. Fixes: 44c02a2c3dc5 ("dev_ioctl(): move copyin/copyout to callers") Link: https://linux-review.googlesource.com/id/I869da6cf6daabc3e4b7b82ac979683ba05e27d4d Signed-off-by: Peter Collingbourne Cc: # 4.19 Signed-off-by: David S. Miller --- include/linux/netdevice.h | 4 ++++ net/socket.c | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index eaf5bb008aa9..d65ce093e5a7 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -4012,6 +4012,10 @@ int netdev_rx_handler_register(struct net_device *dev, void netdev_rx_handler_unregister(struct net_device *dev); bool dev_valid_name(const char *name); +static inline bool is_socket_ioctl_cmd(unsigned int cmd) +{ + return _IOC_TYPE(cmd) == SOCK_IOC_TYPE; +} int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr, bool *need_copyout); int dev_ifconf(struct net *net, struct ifconf *, int); diff --git a/net/socket.c b/net/socket.c index 0b2dad3bdf7f..8808b3617dac 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1109,7 +1109,7 @@ static long sock_do_ioctl(struct net *net, struct socket *sock, rtnl_unlock(); if (!err && copy_to_user(argp, &ifc, sizeof(struct ifconf))) err = -EFAULT; - } else { + } else if (is_socket_ioctl_cmd(cmd)) { struct ifreq ifr; bool need_copyout; if (copy_from_user(&ifr, argp, sizeof(struct ifreq))) @@ -1118,6 +1118,8 @@ static long sock_do_ioctl(struct net *net, struct socket *sock, if (!err && need_copyout) if (copy_to_user(argp, &ifr, sizeof(struct ifreq))) return -EFAULT; + } else { + err = -ENOTTY; } return err; } @@ -3306,6 +3308,8 @@ static int compat_ifr_data_ioctl(struct net *net, unsigned int cmd, struct ifreq ifreq; u32 data32; + if (!is_socket_ioctl_cmd(cmd)) + return -ENOTTY; if (copy_from_user(ifreq.ifr_name, u_ifreq32->ifr_name, IFNAMSIZ)) return -EFAULT; if (get_user(data32, &u_ifreq32->ifr_data)) From 84c5fb8c4264ec12ef9d21905c562d2297a0234e Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Mon, 23 Aug 2021 17:01:46 -0700 Subject: [PATCH 08/14] ice: fix Tx queue iteration for Tx timestamp enablement The driver accidentally copied the ice_for_each_rxq iterator when implementing enablement of the ptp_tx bit for the Tx rings. We still load the Tx rings and set the ptp_tx field, but we iterate over the count of the num_rxq. If the number of Tx and Rx queues differ, this could either cause a buffer overrun when accessing the tx_rings list if num_txq is greater than num_rxq, or it could cause us to fail to enable Tx timestamps for some rings. This was not noticed originally as we generally have the same number of Tx and Rx queues. Fixes: ea9b847cda64 ("ice: enable transmit timestamps for E810 devices") Signed-off-by: Jacob Keller Tested-by: Gurucharan G Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_ptp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c index 9e3ddb9b8b51..f54148fb0e21 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c @@ -22,7 +22,7 @@ static void ice_set_tx_tstamp(struct ice_pf *pf, bool on) return; /* Set the timestamp enable flag for all the Tx rings */ - ice_for_each_rxq(vsi, i) { + ice_for_each_txq(vsi, i) { if (!vsi->tx_rings[i]) continue; vsi->tx_rings[i]->ptp_tx = on; From 1f0cbb3e8916142382551c336065cbcbfb77a11e Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Mon, 23 Aug 2021 17:01:47 -0700 Subject: [PATCH 09/14] ice: remove dead code for allocating pin_config We have code in the ice driver which allocates the pin_config structure if n_pins is > 0, but we never set n_pins to be greater than zero. There's no reason to keep this code until we actually have pin_config support. Remove this. We can re-add it properly when we implement support for pin_config for E810-T devices. Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins") Signed-off-by: Jacob Keller Tested-by: Gurucharan G Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_ptp.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c index f54148fb0e21..09d74e94feae 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c @@ -1064,17 +1064,6 @@ static long ice_ptp_create_clock(struct ice_pf *pf) info = &pf->ptp.info; dev = ice_pf_to_dev(pf); - /* Allocate memory for kernel pins interface */ - if (info->n_pins) { - info->pin_config = devm_kcalloc(dev, info->n_pins, - sizeof(*info->pin_config), - GFP_KERNEL); - if (!info->pin_config) { - info->n_pins = 0; - return -ENOMEM; - } - } - /* Attempt to register the clock before enabling the hardware. */ clock = ptp_clock_register(info, dev); if (IS_ERR(clock)) From 4dd0d5c33c3ebf24a07cae6141648aeb7ba56072 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Mon, 23 Aug 2021 17:01:48 -0700 Subject: [PATCH 10/14] ice: add lock around Tx timestamp tracker flush The driver didn't take the lock while flushing the Tx tracker, which could cause a race where one thread is trying to read timestamps out while another thread is trying to read the tracker to check the timestamps. Avoid this by ensuring that flushing is locked against read accesses. Fixes: ea9b847cda64 ("ice: enable transmit timestamps for E810 devices") Signed-off-by: Jacob Keller Tested-by: Gurucharan G Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_ptp.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c index 09d74e94feae..ae0980f14c80 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c @@ -1267,6 +1267,8 @@ ice_ptp_flush_tx_tracker(struct ice_pf *pf, struct ice_ptp_tx *tx) { u8 idx; + spin_lock(&tx->lock); + for (idx = 0; idx < tx->len; idx++) { u8 phy_idx = idx + tx->quad_offset; @@ -1279,6 +1281,8 @@ ice_ptp_flush_tx_tracker(struct ice_pf *pf, struct ice_ptp_tx *tx) tx->tstamps[idx].skb = NULL; } } + + spin_unlock(&tx->lock); } /** From 9ee313433c483e4a6ecd517c38c0f8aee1962c53 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Mon, 23 Aug 2021 17:01:49 -0700 Subject: [PATCH 11/14] ice: restart periodic outputs around time changes When we enabled auxiliary input/output support for the E810 device, we forgot to add logic to restart the output when we change time. This is important as the periodic output will be incorrect after a time change otherwise. This unfortunately includes the adjust time function, even though it uses an atomic hardware interface. The atomic adjustment can still cause the pin output to stall permanently, so we need to stop and restart it. Introduce wrapper functions to temporarily disable and then re-enable the clock outputs. Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins") Signed-off-by: Jacob Keller Tested-by: Sunitha D Mekala Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_ptp.c | 49 ++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c index ae0980f14c80..05cc5870e4ef 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c @@ -688,6 +688,41 @@ err: return -EFAULT; } +/** + * ice_ptp_disable_all_clkout - Disable all currently configured outputs + * @pf: pointer to the PF structure + * + * Disable all currently configured clock outputs. This is necessary before + * certain changes to the PTP hardware clock. Use ice_ptp_enable_all_clkout to + * re-enable the clocks again. + */ +static void ice_ptp_disable_all_clkout(struct ice_pf *pf) +{ + uint i; + + for (i = 0; i < pf->ptp.info.n_per_out; i++) + if (pf->ptp.perout_channels[i].ena) + ice_ptp_cfg_clkout(pf, i, NULL, false); +} + +/** + * ice_ptp_enable_all_clkout - Enable all configured periodic clock outputs + * @pf: pointer to the PF structure + * + * Enable all currently configured clock outputs. Use this after + * ice_ptp_disable_all_clkout to reconfigure the output signals according to + * their configuration. + */ +static void ice_ptp_enable_all_clkout(struct ice_pf *pf) +{ + uint i; + + for (i = 0; i < pf->ptp.info.n_per_out; i++) + if (pf->ptp.perout_channels[i].ena) + ice_ptp_cfg_clkout(pf, i, &pf->ptp.perout_channels[i], + false); +} + /** * ice_ptp_gpio_enable_e810 - Enable/disable ancillary features of PHC * @info: the driver's PTP info structure @@ -783,12 +818,17 @@ ice_ptp_settime64(struct ptp_clock_info *info, const struct timespec64 *ts) goto exit; } + /* Disable periodic outputs */ + ice_ptp_disable_all_clkout(pf); + err = ice_ptp_write_init(pf, &ts64); ice_ptp_unlock(hw); if (!err) ice_ptp_update_cached_phctime(pf); + /* Reenable periodic outputs */ + ice_ptp_enable_all_clkout(pf); exit: if (err) { dev_err(ice_pf_to_dev(pf), "PTP failed to set time %d\n", err); @@ -842,8 +882,14 @@ static int ice_ptp_adjtime(struct ptp_clock_info *info, s64 delta) return -EBUSY; } + /* Disable periodic outputs */ + ice_ptp_disable_all_clkout(pf); + err = ice_ptp_write_adj(pf, delta); + /* Reenable periodic outputs */ + ice_ptp_enable_all_clkout(pf); + ice_ptp_unlock(hw); if (err) { @@ -1543,6 +1589,9 @@ void ice_ptp_release(struct ice_pf *pf) if (!pf->ptp.clock) return; + /* Disable periodic outputs */ + ice_ptp_disable_all_clkout(pf); + ice_clear_ptp_clock_index(pf); ptp_clock_unregister(pf->ptp.clock); pf->ptp.clock = NULL; From b357d9717be7f95fde2c6c4650b186a995b71e59 Mon Sep 17 00:00:00 2001 From: Brett Creeley Date: Tue, 24 Aug 2021 12:27:53 -0700 Subject: [PATCH 12/14] ice: Only lock to update netdev dev_addr commit 3ba7f53f8bf1 ("ice: don't remove netdev->dev_addr from uc sync list") introduced calls to netif_addr_lock_bh() and netif_addr_unlock_bh() in the driver's ndo_set_mac() callback. This is fine since the driver is updated the netdev's dev_addr, but since this is a spinlock, the driver cannot sleep when the lock is held. Unfortunately the functions to add/delete MAC filters depend on a mutex. This was causing a trace with the lock debug kernel config options enabled when changing the mac address via iproute. [ 203.273059] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:281 [ 203.273065] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 6698, name: ip [ 203.273068] Preemption disabled at: [ 203.273068] [] ice_set_mac_address+0x8b/0x1c0 [ice] [ 203.273097] CPU: 31 PID: 6698 Comm: ip Tainted: G S W I 5.14.0-rc4 #2 [ 203.273100] Hardware name: Intel Corporation S2600WFT/S2600WFT, BIOS SE5C620.86B.02.01.0010.010620200716 01/06/2020 [ 203.273102] Call Trace: [ 203.273107] dump_stack_lvl+0x33/0x42 [ 203.273113] ? ice_set_mac_address+0x8b/0x1c0 [ice] [ 203.273124] ___might_sleep.cold.150+0xda/0xea [ 203.273131] mutex_lock+0x1c/0x40 [ 203.273136] ice_remove_mac+0xe3/0x180 [ice] [ 203.273155] ? ice_fltr_add_mac_list+0x20/0x20 [ice] [ 203.273175] ice_fltr_prepare_mac+0x43/0xa0 [ice] [ 203.273194] ice_set_mac_address+0xab/0x1c0 [ice] [ 203.273206] dev_set_mac_address+0xb8/0x120 [ 203.273210] dev_set_mac_address_user+0x2c/0x50 [ 203.273212] do_setlink+0x1dd/0x10e0 [ 203.273217] ? __nla_validate_parse+0x12d/0x1a0 [ 203.273221] __rtnl_newlink+0x530/0x910 [ 203.273224] ? __kmalloc_node_track_caller+0x17f/0x380 [ 203.273230] ? preempt_count_add+0x68/0xa0 [ 203.273236] ? _raw_spin_lock_irqsave+0x1f/0x30 [ 203.273241] ? kmem_cache_alloc_trace+0x4d/0x440 [ 203.273244] rtnl_newlink+0x43/0x60 [ 203.273245] rtnetlink_rcv_msg+0x13a/0x380 [ 203.273248] ? rtnl_calcit.isra.40+0x130/0x130 [ 203.273250] netlink_rcv_skb+0x4e/0x100 [ 203.273256] netlink_unicast+0x1a2/0x280 [ 203.273258] netlink_sendmsg+0x242/0x490 [ 203.273260] sock_sendmsg+0x58/0x60 [ 203.273263] ____sys_sendmsg+0x1ef/0x260 [ 203.273265] ? copy_msghdr_from_user+0x5c/0x90 [ 203.273268] ? ____sys_recvmsg+0xe6/0x170 [ 203.273270] ___sys_sendmsg+0x7c/0xc0 [ 203.273272] ? copy_msghdr_from_user+0x5c/0x90 [ 203.273274] ? ___sys_recvmsg+0x89/0xc0 [ 203.273276] ? __netlink_sendskb+0x50/0x50 [ 203.273278] ? mod_objcg_state+0xee/0x310 [ 203.273282] ? __dentry_kill+0x114/0x170 [ 203.273286] ? get_max_files+0x10/0x10 [ 203.273288] __sys_sendmsg+0x57/0xa0 [ 203.273290] do_syscall_64+0x37/0x80 [ 203.273295] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 203.273296] RIP: 0033:0x7f8edf96e278 [ 203.273298] Code: 89 02 48 c7 c0 ff ff ff ff eb b5 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 8d 05 25 63 2c 00 8b 00 85 c0 75 17 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 58 c3 0f 1f 80 00 00 00 00 41 54 41 89 d4 55 [ 203.273300] RSP: 002b:00007ffcb8bdac08 EFLAGS: 00000246 ORIG_RAX: 000000000000002e [ 203.273303] RAX: ffffffffffffffda RBX: 000000006115e0ae RCX: 00007f8edf96e278 [ 203.273304] RDX: 0000000000000000 RSI: 00007ffcb8bdac70 RDI: 0000000000000003 [ 203.273305] RBP: 0000000000000000 R08: 0000000000000001 R09: 00007ffcb8bda5b0 [ 203.273306] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001 [ 203.273306] R13: 0000555e10092020 R14: 0000000000000000 R15: 0000000000000005 Fix this by only locking when changing the netdev->dev_addr. Also, make sure to restore the old netdev->dev_addr on any failures. Fixes: 3ba7f53f8bf1 ("ice: don't remove netdev->dev_addr from uc sync list") Signed-off-by: Brett Creeley Tested-by: Gurucharan G Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/ice/ice_main.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index fe2ded775f25..a8bd512d5b45 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -5122,6 +5122,7 @@ static int ice_set_mac_address(struct net_device *netdev, void *pi) struct ice_hw *hw = &pf->hw; struct sockaddr *addr = pi; enum ice_status status; + u8 old_mac[ETH_ALEN]; u8 flags = 0; int err = 0; u8 *mac; @@ -5144,8 +5145,13 @@ static int ice_set_mac_address(struct net_device *netdev, void *pi) } netif_addr_lock_bh(netdev); + ether_addr_copy(old_mac, netdev->dev_addr); + /* change the netdev's MAC address */ + memcpy(netdev->dev_addr, mac, netdev->addr_len); + netif_addr_unlock_bh(netdev); + /* Clean up old MAC filter. Not an error if old filter doesn't exist */ - status = ice_fltr_remove_mac(vsi, netdev->dev_addr, ICE_FWD_TO_VSI); + status = ice_fltr_remove_mac(vsi, old_mac, ICE_FWD_TO_VSI); if (status && status != ICE_ERR_DOES_NOT_EXIST) { err = -EADDRNOTAVAIL; goto err_update_filters; @@ -5168,13 +5174,12 @@ err_update_filters: if (err) { netdev_err(netdev, "can't set MAC %pM. filter update failed\n", mac); + netif_addr_lock_bh(netdev); + ether_addr_copy(netdev->dev_addr, old_mac); netif_addr_unlock_bh(netdev); return err; } - /* change the netdev's MAC address */ - memcpy(netdev->dev_addr, mac, netdev->addr_len); - netif_addr_unlock_bh(netdev); netdev_dbg(vsi->netdev, "updated MAC address to %pM\n", netdev->dev_addr); From 0d55649d2ad7296acfda9127e1d05518d025734a Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 27 Aug 2021 15:25:41 +0200 Subject: [PATCH 13/14] net: phy: marvell10g: fix broken PHY interrupts for anyone after us in the driver probe list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enabling interrupts via device tree for the internal PHYs on the mv88e6390 DSA switch does not work. The driver insists to use poll mode. Stage one debugging shows that the fwnode_mdiobus_phy_device_register function calls fwnode_irq_get properly, and phy->irq is set to a valid interrupt line initially. But it is then cleared. Stage two debugging shows that it is cleared here: phy_probe: /* Disable the interrupt if the PHY doesn't support it * but the interrupt is still a valid one */ if (!phy_drv_supports_irq(phydrv) && phy_interrupt_is_valid(phydev)) phydev->irq = PHY_POLL; Okay, so does the "Marvell 88E6390 Family" PHY driver not have the .config_intr and .handle_interrupt function pointers? Yes it does. Stage three debugging shows that the PHY device does not attempt a probe against the "Marvell 88E6390 Family" driver, but against the "mv88x3310" driver. Okay, so why does the "mv88x3310" driver match on a mv88x6390 internal PHY? The PHY IDs (MARVELL_PHY_ID_88E6390_FAMILY vs MARVELL_PHY_ID_88X3310) are way different. Stage four debugging has us looking through: phy_device_register -> device_add -> bus_probe_device -> device_initial_probe -> __device_attach -> bus_for_each_drv -> driver_match_device -> drv->bus->match -> phy_bus_match Okay, so as we said, the MII_PHYSID1 of mv88e6390 does not match the mv88x3310 driver's PHY mask & ID, so why would phy_bus_match return... Ahh, phy_bus_match calls a shortcircuit method, phydrv->match_phy_device, and does not even bother to compare the PHY ID if that is implemented. So of course, we go inside the marvell10g.c driver and sure enough, it implements .match_phy_device and does not bother to check the PHY ID. What's interesting though is that at the end of the device_add() from phy_device_register(), the driver for the internal PHYs _is_ the proper "Marvell 88E6390 Family". This is because "mv88x3310" ends up failing to probe after all, and __device_attach_driver(), to quote: /* * Ignore errors returned by ->probe so that the next driver can try * its luck. */ The next (and only other) driver that matches is the 6390 driver. For this one, phy_probe doesn't fail, and everything expects to work as normal, EXCEPT phydev->irq has already been cleared by the previous unsuccessful probe of a driver which did not implement PHY interrupts, and therefore cleared that IRQ. Okay, so it is not just Marvell 6390 that has PHY interrupts broken. Stuff like Atheros, Aquantia, Broadcom, Qualcomm work because they are lexicographically before Marvell, and stuff like NXP, Realtek, Vitesse are broken. This goes to show how fragile it is to reset phydev->irq = PHY_POLL from the actual beginning of phy_probe itself. That seems like an actual bug of its own too, since phy_probe has side effects which are not restored on probe failure, but the line of thought probably was, the same driver will attempt probe again, so it doesn't matter. Well, looks like it does. Maybe it would make more sense to move the phydev->irq clearing after the actual device_add() in phy_device_register() completes, and the bound driver is the actual final one. (also, a bit frightening that drivers are permitted to bypass the MDIO bus matching in such a trivial way and perform PHY reads and writes from the .match_phy_device method, on devices that do not even belong to them. In the general case it might not be guaranteed that the MDIO accesses one driver needs to make to figure out whether to match on a device is safe for all other PHY devices) Fixes: a5de4be0aaaa ("net: phy: marvell10g: fix differentiation of 88X3310 from 88X3340") Signed-off-by: Vladimir Oltean Tested-by: Marek Behún Signed-off-by: Marek Behún Reviewed-by: Andrew Lunn Link: https://lore.kernel.org/r/20210827132541.28953-1-kabel@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/phy/marvell10g.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c index 53a433442803..f4d758f8a1ee 100644 --- a/drivers/net/phy/marvell10g.c +++ b/drivers/net/phy/marvell10g.c @@ -987,11 +987,19 @@ static int mv3310_get_number_of_ports(struct phy_device *phydev) static int mv3310_match_phy_device(struct phy_device *phydev) { + if ((phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD] & + MARVELL_PHY_ID_MASK) != MARVELL_PHY_ID_88X3310) + return 0; + return mv3310_get_number_of_ports(phydev) == 1; } static int mv3340_match_phy_device(struct phy_device *phydev) { + if ((phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD] & + MARVELL_PHY_ID_MASK) != MARVELL_PHY_ID_88X3310) + return 0; + return mv3310_get_number_of_ports(phydev) == 4; } From 57f780f1c43362b86fd23d20bd940e2468237716 Mon Sep 17 00:00:00 2001 From: Sudarsana Reddy Kalluru Date: Fri, 27 Aug 2021 04:52:25 -0700 Subject: [PATCH 14/14] atlantic: Fix driver resume flow. Driver crashes when restoring from the Hibernate. In the resume flow, driver need to clean up the older nic/vec objects and re-initialize them. Fixes: 8aaa112a57c1d ("net: atlantic: refactoring pm logic") Signed-off-by: Sudarsana Reddy Kalluru Signed-off-by: Igor Russkikh Signed-off-by: David S. Miller --- drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c index 59253846e885..f26d03735619 100644 --- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c +++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c @@ -417,6 +417,9 @@ static int atl_resume_common(struct device *dev, bool deep) pci_restore_state(pdev); if (deep) { + /* Reinitialize Nic/Vecs objects */ + aq_nic_deinit(nic, !nic->aq_hw->aq_nic_cfg->wol); + ret = aq_nic_init(nic); if (ret) goto err_exit;