From 1c81a9b3aaa26f290c23dee8629579b3ebb994b3 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 3 Feb 2023 02:11:13 +0200 Subject: [PATCH 1/4] net: enetc: simplify enetc_num_stack_tx_queues() We keep a pointer to the xdp_prog in the private netdev structure as well; what's replicated per RX ring is done so just for more convenient access from the NAPI poll procedure. Simplify enetc_num_stack_tx_queues() by looking at priv->xdp_prog rather than iterating through the information replicated per RX ring. Signed-off-by: Vladimir Oltean Reviewed-by: Simon Horman Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/freescale/enetc/enetc.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c index 159ae740ba3c..3a80f259b17e 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.c +++ b/drivers/net/ethernet/freescale/enetc/enetc.c @@ -28,11 +28,9 @@ EXPORT_SYMBOL_GPL(enetc_port_mac_wr); static int enetc_num_stack_tx_queues(struct enetc_ndev_priv *priv) { int num_tx_rings = priv->num_tx_rings; - int i; - for (i = 0; i < priv->num_rx_rings; i++) - if (priv->rx_ring[i]->xdp.prog) - return num_tx_rings - num_possible_cpus(); + if (priv->xdp_prog) + return num_tx_rings - num_possible_cpus(); return num_tx_rings; } From 46a0ecf93b6d188379efe05cdde8564f1fe7fdad Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 3 Feb 2023 02:11:14 +0200 Subject: [PATCH 2/4] net: enetc: allow the enetc_reconfigure() callback to fail enetc_reconfigure() was modified in commit c33bfaf91c4c ("net: enetc: set up XDP program under enetc_reconfigure()") to take an optional callback that runs while the netdev is down, but this callback currently cannot fail. Code up the error handling so that the interface is restarted with the old resources if the callback fails. Signed-off-by: Vladimir Oltean Reviewed-by: Simon Horman Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/freescale/enetc/enetc.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c index 3a80f259b17e..5d7eeb1b5a23 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.c +++ b/drivers/net/ethernet/freescale/enetc/enetc.c @@ -2574,8 +2574,11 @@ static int enetc_reconfigure(struct enetc_ndev_priv *priv, bool extended, * without reconfiguration. */ if (!netif_running(priv->ndev)) { - if (cb) - cb(priv, ctx); + if (cb) { + err = cb(priv, ctx); + if (err) + return err; + } return 0; } @@ -2596,8 +2599,11 @@ static int enetc_reconfigure(struct enetc_ndev_priv *priv, bool extended, enetc_free_rxtx_rings(priv); /* Interface is down, run optional callback now */ - if (cb) - cb(priv, ctx); + if (cb) { + err = cb(priv, ctx); + if (err) + goto out_restart; + } enetc_assign_tx_resources(priv, tx_res); enetc_assign_rx_resources(priv, rx_res); @@ -2606,6 +2612,10 @@ static int enetc_reconfigure(struct enetc_ndev_priv *priv, bool extended, return 0; +out_restart: + enetc_setup_bdrs(priv, extended); + enetc_start(priv->ndev); + enetc_free_rx_resources(rx_res, priv->num_rx_rings); out_free_tx_res: enetc_free_tx_resources(tx_res, priv->num_tx_rings); out: From 4ea1dd743eb6c76dec6ed28fae7e1629961a55af Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 3 Feb 2023 02:11:15 +0200 Subject: [PATCH 3/4] net: enetc: recalculate num_real_tx_queues when XDP program attaches Since the blamed net-next commit, enetc_setup_xdp_prog() no longer goes through enetc_open(), and therefore, the function which was supposed to detect whether a BPF program exists (in order to crop some TX queues from network stack usage), enetc_num_stack_tx_queues(), no longer gets called. We can move the netif_set_real_num_rx_queues() call to enetc_alloc_msix() (probe time), since it is a runtime invariant. We can do the same thing with netif_set_real_num_tx_queues(), and let enetc_reconfigure_xdp_cb() explicitly recalculate and change the number of stack TX queues. Fixes: c33bfaf91c4c ("net: enetc: set up XDP program under enetc_reconfigure()") Signed-off-by: Vladimir Oltean Reviewed-by: Simon Horman Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/freescale/enetc/enetc.c | 35 ++++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c index 5d7eeb1b5a23..e18a6c834eb4 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.c +++ b/drivers/net/ethernet/freescale/enetc/enetc.c @@ -2454,7 +2454,6 @@ int enetc_open(struct net_device *ndev) { struct enetc_ndev_priv *priv = netdev_priv(ndev); struct enetc_bdr_resource *tx_res, *rx_res; - int num_stack_tx_queues; bool extended; int err; @@ -2480,16 +2479,6 @@ int enetc_open(struct net_device *ndev) goto err_alloc_rx; } - num_stack_tx_queues = enetc_num_stack_tx_queues(priv); - - err = netif_set_real_num_tx_queues(ndev, num_stack_tx_queues); - if (err) - goto err_set_queues; - - err = netif_set_real_num_rx_queues(ndev, priv->num_rx_rings); - if (err) - goto err_set_queues; - enetc_tx_onestep_tstamp_init(priv); enetc_assign_tx_resources(priv, tx_res); enetc_assign_rx_resources(priv, rx_res); @@ -2498,8 +2487,6 @@ int enetc_open(struct net_device *ndev) return 0; -err_set_queues: - enetc_free_rx_resources(rx_res, priv->num_rx_rings); err_alloc_rx: enetc_free_tx_resources(tx_res, priv->num_tx_rings); err_alloc_tx: @@ -2683,9 +2670,18 @@ EXPORT_SYMBOL_GPL(enetc_setup_tc_mqprio); static int enetc_reconfigure_xdp_cb(struct enetc_ndev_priv *priv, void *ctx) { struct bpf_prog *old_prog, *prog = ctx; - int i; + int num_stack_tx_queues; + int err, i; old_prog = xchg(&priv->xdp_prog, prog); + + num_stack_tx_queues = enetc_num_stack_tx_queues(priv); + err = netif_set_real_num_tx_queues(priv->ndev, num_stack_tx_queues); + if (err) { + xchg(&priv->xdp_prog, old_prog); + return err; + } + if (old_prog) bpf_prog_put(old_prog); @@ -2906,6 +2902,7 @@ EXPORT_SYMBOL_GPL(enetc_ioctl); int enetc_alloc_msix(struct enetc_ndev_priv *priv) { struct pci_dev *pdev = priv->si->pdev; + int num_stack_tx_queues; int first_xdp_tx_ring; int i, n, err, nvec; int v_tx_rings; @@ -2982,6 +2979,16 @@ int enetc_alloc_msix(struct enetc_ndev_priv *priv) } } + num_stack_tx_queues = enetc_num_stack_tx_queues(priv); + + err = netif_set_real_num_tx_queues(priv->ndev, num_stack_tx_queues); + if (err) + goto fail; + + err = netif_set_real_num_rx_queues(priv->ndev, priv->num_rx_rings); + if (err) + goto fail; + first_xdp_tx_ring = priv->num_tx_rings - num_possible_cpus(); priv->xdp_tx_ring = &priv->tx_ring[first_xdp_tx_ring]; From 800db2d125c2bc22c448e2386c3518e663d6db71 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 3 Feb 2023 02:11:16 +0200 Subject: [PATCH 4/4] net: enetc: ensure we always have a minimum number of TXQs for stack Currently it can happen that an mqprio qdisc is installed with num_tc 8, and this will reserve 8 (out of 8) TXQs for the network stack. Then we can attach an XDP program, and this will crop 2 TXQs, leaving just 6 for mqprio. That's not what the user requested, and we should fail it. On the other hand, if mqprio isn't requested, we still give the 8 TXQs to the network stack (with hashing among a single traffic class), but then, cropping 2 TXQs for XDP is fine, because the user didn't explicitly ask for any number of TXQs, so no expectations are violated. Simply put, the logic that mqprio should impose a minimum number of TXQs for the network never existed. Let's say (more or less arbitrarily) that without mqprio, the driver expects a minimum number of TXQs equal to the number of CPUs (on NXP LS1028A, that is either 1, or 2). And with mqprio, mqprio gives the minimum required number of TXQs. Signed-off-by: Vladimir Oltean Reviewed-by: Simon Horman Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/freescale/enetc/enetc.c | 14 ++++++++++++++ drivers/net/ethernet/freescale/enetc/enetc.h | 3 +++ 2 files changed, 17 insertions(+) diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c index e18a6c834eb4..1c0aeaa13cde 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.c +++ b/drivers/net/ethernet/freescale/enetc/enetc.c @@ -2626,6 +2626,7 @@ int enetc_setup_tc_mqprio(struct net_device *ndev, void *type_data) if (!num_tc) { netdev_reset_tc(ndev); netif_set_real_num_tx_queues(ndev, num_stack_tx_queues); + priv->min_num_stack_tx_queues = num_possible_cpus(); /* Reset all ring priorities to 0 */ for (i = 0; i < priv->num_tx_rings; i++) { @@ -2656,6 +2657,7 @@ int enetc_setup_tc_mqprio(struct net_device *ndev, void *type_data) /* Reset the number of netdev queues based on the TC count */ netif_set_real_num_tx_queues(ndev, num_tc); + priv->min_num_stack_tx_queues = num_tc; netdev_set_num_tc(ndev, num_tc); @@ -2702,9 +2704,20 @@ static int enetc_reconfigure_xdp_cb(struct enetc_ndev_priv *priv, void *ctx) static int enetc_setup_xdp_prog(struct net_device *ndev, struct bpf_prog *prog, struct netlink_ext_ack *extack) { + int num_xdp_tx_queues = prog ? num_possible_cpus() : 0; struct enetc_ndev_priv *priv = netdev_priv(ndev); bool extended; + if (priv->min_num_stack_tx_queues + num_xdp_tx_queues > + priv->num_tx_rings) { + NL_SET_ERR_MSG_FMT_MOD(extack, + "Reserving %d XDP TXQs does not leave a minimum of %d TXQs for network stack (total %d available)", + num_xdp_tx_queues, + priv->min_num_stack_tx_queues, + priv->num_tx_rings); + return -EBUSY; + } + extended = !!(priv->active_offloads & ENETC_F_RX_TSTAMP); /* The buffer layout is changing, so we need to drain the old @@ -2989,6 +3002,7 @@ int enetc_alloc_msix(struct enetc_ndev_priv *priv) if (err) goto fail; + priv->min_num_stack_tx_queues = num_possible_cpus(); first_xdp_tx_ring = priv->num_tx_rings - num_possible_cpus(); priv->xdp_tx_ring = &priv->tx_ring[first_xdp_tx_ring]; diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h index 1fe8dfd6b6d4..e21d096c5a90 100644 --- a/drivers/net/ethernet/freescale/enetc/enetc.h +++ b/drivers/net/ethernet/freescale/enetc/enetc.h @@ -369,6 +369,9 @@ struct enetc_ndev_priv { struct psfp_cap psfp_cap; + /* Minimum number of TX queues required by the network stack */ + unsigned int min_num_stack_tx_queues; + struct phylink *phylink; int ic_mode; u32 tx_ictt;