@@ -2450,6 +2450,59 @@ void enetc_start(struct net_device *ndev)
}
EXPORT_SYMBOL_GPL(enetc_start);
+static int enetc_xdp_rxq_mem_model_register(struct enetc_ndev_priv *priv,
+ int rxq)
+{
+ struct enetc_bdr *rx_ring = priv->rx_ring[rxq];
+ int err;
+
+ err = xdp_rxq_info_reg(&rx_ring->xdp.rxq, priv->ndev, rxq, 0);
+ if (err)
+ return err;
+
+ err = xdp_rxq_info_reg_mem_model(&rx_ring->xdp.rxq,
+ MEM_TYPE_PAGE_SHARED, NULL);
+ if (err)
+ xdp_rxq_info_unreg(&rx_ring->xdp.rxq);
+
+ return err;
+}
+
+static void enetc_xdp_rxq_mem_model_unregister(struct enetc_ndev_priv *priv,
+ int rxq)
+{
+ struct enetc_bdr *rx_ring = priv->rx_ring[rxq];
+
+ xdp_rxq_info_unreg_mem_model(&rx_ring->xdp.rxq);
+ xdp_rxq_info_unreg(&rx_ring->xdp.rxq);
+}
+
+static int enetc_xdp_mem_model_register(struct enetc_ndev_priv *priv)
+{
+ int i, err;
+
+ for (i = 0; i < priv->num_rx_rings; i++) {
+ err = enetc_xdp_rxq_mem_model_register(priv, i);
+ if (err)
+ goto rollback;
+ }
+
+ return 0;
+
+rollback:
+ for (; i >= 0; i--)
+ enetc_xdp_rxq_mem_model_unregister(priv, i);
+ return err;
+}
+
+static void enetc_xdp_mem_model_unregister(struct enetc_ndev_priv *priv)
+{
+ int i;
+
+ for (i = 0; i < priv->num_rx_rings; i++)
+ enetc_xdp_rxq_mem_model_unregister(priv, i);
+}
+
int enetc_open(struct net_device *ndev)
{
struct enetc_ndev_priv *priv = netdev_priv(ndev);
@@ -2675,13 +2728,19 @@ static int enetc_reconfigure_xdp_cb(struct enetc_ndev_priv *priv, void *ctx)
int num_stack_tx_queues;
int err, i;
+ if (prog) {
+ err = enetc_xdp_mem_model_register(priv);
+ if (err)
+ return err;
+ }
+
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;
+ goto err_xdp_mem_model_unreg;
}
if (old_prog)
@@ -2698,7 +2757,15 @@ static int enetc_reconfigure_xdp_cb(struct enetc_ndev_priv *priv, void *ctx)
rx_ring->buffer_offset = ENETC_RXB_PAD;
}
+ if (!prog)
+ enetc_xdp_mem_model_unregister(priv);
+
return 0;
+
+err_xdp_mem_model_unreg:
+ if (prog)
+ enetc_xdp_mem_model_unregister(priv);
+ return err;
}
static int enetc_setup_xdp_prog(struct net_device *ndev, struct bpf_prog *prog,
@@ -2954,20 +3021,6 @@ int enetc_alloc_msix(struct enetc_ndev_priv *priv)
bdr->buffer_offset = ENETC_RXB_PAD;
priv->rx_ring[i] = bdr;
- err = xdp_rxq_info_reg(&bdr->xdp.rxq, priv->ndev, i, 0);
- if (err) {
- kfree(v);
- goto fail;
- }
-
- err = xdp_rxq_info_reg_mem_model(&bdr->xdp.rxq,
- MEM_TYPE_PAGE_SHARED, NULL);
- if (err) {
- xdp_rxq_info_unreg(&bdr->xdp.rxq);
- kfree(v);
- goto fail;
- }
-
/* init defaults for adaptive IC */
if (priv->ic_mode & ENETC_IC_RX_ADAPTIVE) {
v->rx_ictt = 0x1;
@@ -3011,10 +3064,7 @@ int enetc_alloc_msix(struct enetc_ndev_priv *priv)
fail:
while (i--) {
struct enetc_int_vector *v = priv->int_vector[i];
- struct enetc_bdr *rx_ring = &v->rx_ring;
- xdp_rxq_info_unreg_mem_model(&rx_ring->xdp.rxq);
- xdp_rxq_info_unreg(&rx_ring->xdp.rxq);
netif_napi_del(&v->napi);
cancel_work_sync(&v->rx_dim.work);
kfree(v);
@@ -3032,10 +3082,7 @@ void enetc_free_msix(struct enetc_ndev_priv *priv)
for (i = 0; i < priv->bdr_int_num; i++) {
struct enetc_int_vector *v = priv->int_vector[i];
- struct enetc_bdr *rx_ring = &v->rx_ring;
- xdp_rxq_info_unreg_mem_model(&rx_ring->xdp.rxq);
- xdp_rxq_info_unreg(&rx_ring->xdp.rxq);
netif_napi_del(&v->napi);
cancel_work_sync(&v->rx_dim.work);
}
In a future patch, the XDP RX queues will have to switch between exposing a shared page memory model or an XSK pool memory model. If we keep the RXQ registration where it currently is (enetc_pf_probe() -> enetc_alloc_msix()), we'll end up needing to unregister the existing RXQs and register new ones. But surprise, registering them can fail, and that can leave us in the unpleasant situation that we can't recover from two consecutive errors. Taking a quick look at net/core/xdp.c, I see that this information seems to only be used for xdp_buff :: rxq (and :: mem) and xdp_frame :: mem, essentially between xdp_init_buff() and xdp_release_frame(). While these 2 might not be under the same NAPI poll cycle, the enetc_reconfigure() procedure does make sure that any XDP buffers in flight are returned to the respective memory "allocator" prior to calling enetc_reconfigure_xdp_cb(). So it seems that the most logical way to place this is no earlier than when it is needed, and unregister no later than when it stops being needed. This also saves us from the impossible condition of two consecutive registration failures, because now there isn't anything to rollback on failure, we can just propagate the error to user space and we're in the same state as before. I don't really understand why don't more drivers do this. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/ethernet/freescale/enetc/enetc.c | 89 +++++++++++++++----- 1 file changed, 68 insertions(+), 21 deletions(-)