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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e8b9b82644de7acbe7a7f8059d17ed7908f3df17.camel@redhat.com>
Date: Tue, 13 Feb 2024 12:20:56 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Shannon Nelson <shannon.nelson@....com>, netdev@...r.kernel.org, 
	davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com
Cc: brett.creeley@....com, drivers@...sando.io
Subject: Re: [PATCH v3 net-next 4/9] ionic: add initial framework for XDP
 support

On Fri, 2024-02-09 at 16:48 -0800, Shannon Nelson wrote:
[...]
> +static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
> +{
> +	struct xdp_rxq_info *rxq_info;
> +	int err;
> +
> +	rxq_info = kzalloc(sizeof(*rxq_info), GFP_KERNEL);
> +	if (!rxq_info)
> +		return -ENOMEM;
> +
> +	err = xdp_rxq_info_reg(rxq_info, q->lif->netdev, q->index, napi_id);
> +	if (err) {
> +		dev_err(q->dev, "Queue %d xdp_rxq_info_reg failed, err %d\n",
> +			q->index, err);

You can avoid some little code duplication with the usual
		goto err;

//...
err:
	kfree(rxq_info);
	return err;

> +		kfree(rxq_info);
> +		return err;
> +	}
> +
> +	err = xdp_rxq_info_reg_mem_model(rxq_info, MEM_TYPE_PAGE_ORDER0, NULL);
> +	if (err) {
> +		dev_err(q->dev, "Queue %d xdp_rxq_info_reg_mem_model failed, err %d\n",
> +			q->index, err);
> +		xdp_rxq_info_unreg(rxq_info);

and using the same label here.

> +		kfree(rxq_info);
> +		return err;
> +	}
> +
> +	q->xdp_rxq_info = rxq_info;
> +
> +	return 0;
> +}
> +
> +static int ionic_xdp_queues_config(struct ionic_lif *lif)
> +{
> +	unsigned int i;
> +	int err;
> +
> +	if (!lif->rxqcqs)
> +		return 0;
> +
> +	/* There's no need to rework memory if not going to/from NULL program.
> +	 * If there is no lif->xdp_prog, there should also be no q.xdp_rxq_info
> +	 * This way we don't need to keep an *xdp_prog in every queue struct.
> +	 */
> +	if (!lif->xdp_prog == !lif->rxqcqs[0]->q.xdp_rxq_info)
> +		return 0;
> +
> +	for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
> +		struct ionic_queue *q = &lif->rxqcqs[i]->q;
> +
> +		if (q->xdp_rxq_info) {
> +			ionic_xdp_unregister_rxq_info(q);

You can reduce the nesting level adding a 'continue' here:
			continue;
		}

		err = ionic_xdp_register_rxq_info(q, lif->rxqcqs[i]->napi.napi_id);
		// ...

> +		} else {
> +			err = ionic_xdp_register_rxq_info(q, lif->rxqcqs[i]->napi.napi_id);
> +			if (err) {
> +				dev_err(lif->ionic->dev, "failed to register RX queue %d info for XDP, err %d\n",
> +					i, err);
> +				goto err_out;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +
> +err_out:
> +	for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++)
> +		ionic_xdp_unregister_rxq_info(&lif->rxqcqs[i]->q);
> +
> +	return err;
> +}
> +
> +static int ionic_xdp_config(struct net_device *netdev, struct netdev_bpf *bpf)
> +{
> +	struct ionic_lif *lif = netdev_priv(netdev);
> +	struct bpf_prog *old_prog;
> +	u32 maxfs;
> +
> +	if (test_bit(IONIC_LIF_F_SPLIT_INTR, lif->state)) {
> +#define XDP_ERR_SPLIT "XDP not available with split Tx/Rx interrupts"
> +		NL_SET_ERR_MSG_MOD(bpf->extack, XDP_ERR_SPLIT);
> +		netdev_info(lif->netdev, XDP_ERR_SPLIT);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (!ionic_xdp_is_valid_mtu(lif, netdev->mtu, bpf->prog)) {
> +#define XDP_ERR_MTU "MTU is too large for XDP without frags support"
> +		NL_SET_ERR_MSG_MOD(bpf->extack, XDP_ERR_MTU);
> +		netdev_info(lif->netdev, XDP_ERR_MTU);
> +		return -EINVAL;
> +	}
> +
> +	maxfs = __le32_to_cpu(lif->identity->eth.max_frame_size) - VLAN_ETH_HLEN;
> +	if (bpf->prog)
> +		maxfs = min_t(u32, maxfs, IONIC_XDP_MAX_LINEAR_MTU);
> +	netdev->max_mtu = maxfs;
> +
> +	if (!netif_running(netdev)) {
> +		old_prog = xchg(&lif->xdp_prog, bpf->prog);
> +	} else {
> +		mutex_lock(&lif->queue_lock);
> +		ionic_stop_queues_reconfig(lif);
> +		old_prog = xchg(&lif->xdp_prog, bpf->prog);
> +		ionic_start_queues_reconfig(lif);
> +		mutex_unlock(&lif->queue_lock);
> +	}
> +
> +	if (old_prog)
> +		bpf_prog_put(old_prog);
> +
> +	return 0;
> +}
> +
> +static int ionic_xdp(struct net_device *netdev, struct netdev_bpf *bpf)
> +{
> +	switch (bpf->command) {
> +	case XDP_SETUP_PROG:
> +		return ionic_xdp_config(netdev, bpf);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static const struct net_device_ops ionic_netdev_ops = {
>  	.ndo_open               = ionic_open,
>  	.ndo_stop               = ionic_stop,
>  	.ndo_eth_ioctl		= ionic_eth_ioctl,
>  	.ndo_start_xmit		= ionic_start_xmit,
> +	.ndo_bpf		= ionic_xdp,
>  	.ndo_get_stats64	= ionic_get_stats64,
>  	.ndo_set_rx_mode	= ionic_ndo_set_rx_mode,
>  	.ndo_set_features	= ionic_set_features,
> @@ -2755,6 +2922,7 @@ static void ionic_swap_queues(struct ionic_qcq *a, struct ionic_qcq *b)
>  	swap(a->q.base,       b->q.base);
>  	swap(a->q.base_pa,    b->q.base_pa);
>  	swap(a->q.info,       b->q.info);
> +	swap(a->q.xdp_rxq_info, b->q.xdp_rxq_info);
>  	swap(a->q_base,       b->q_base);
>  	swap(a->q_base_pa,    b->q_base_pa);
>  	swap(a->q_size,       b->q_size);
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> index 61548b3eea93..61fa4ea4f04c 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> @@ -51,6 +51,9 @@ struct ionic_rx_stats {
>  	u64 alloc_err;
>  	u64 hwstamp_valid;
>  	u64 hwstamp_invalid;
> +	u64 xdp_drop;
> +	u64 xdp_aborted;
> +	u64 xdp_pass;
>  };
>  
>  #define IONIC_QCQ_F_INITED		BIT(0)
> @@ -135,6 +138,9 @@ struct ionic_lif_sw_stats {
>  	u64 hw_rx_over_errors;
>  	u64 hw_rx_missed_errors;
>  	u64 hw_tx_aborted_errors;
> +	u64 xdp_drop;
> +	u64 xdp_aborted;
> +	u64 xdp_pass;
>  };
>  
>  enum ionic_lif_state_flags {
> @@ -230,6 +236,7 @@ struct ionic_lif {
>  	struct ionic_phc *phc;
>  
>  	struct dentry *dentry;
> +	struct bpf_prog *xdp_prog;
>  };
>  
>  struct ionic_phc {
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_stats.c b/drivers/net/ethernet/pensando/ionic/ionic_stats.c
> index 1f6022fb7679..2fb20173b2c6 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_stats.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_stats.c
> @@ -27,6 +27,9 @@ static const struct ionic_stat_desc ionic_lif_stats_desc[] = {
>  	IONIC_LIF_STAT_DESC(hw_rx_over_errors),
>  	IONIC_LIF_STAT_DESC(hw_rx_missed_errors),
>  	IONIC_LIF_STAT_DESC(hw_tx_aborted_errors),
> +	IONIC_LIF_STAT_DESC(xdp_drop),
> +	IONIC_LIF_STAT_DESC(xdp_aborted),
> +	IONIC_LIF_STAT_DESC(xdp_pass),
>  };
>  
>  static const struct ionic_stat_desc ionic_port_stats_desc[] = {
> @@ -149,6 +152,9 @@ static const struct ionic_stat_desc ionic_rx_stats_desc[] = {
>  	IONIC_RX_STAT_DESC(hwstamp_invalid),
>  	IONIC_RX_STAT_DESC(dropped),
>  	IONIC_RX_STAT_DESC(vlan_stripped),
> +	IONIC_RX_STAT_DESC(xdp_drop),
> +	IONIC_RX_STAT_DESC(xdp_aborted),
> +	IONIC_RX_STAT_DESC(xdp_pass),
>  };
>  
>  #define IONIC_NUM_LIF_STATS ARRAY_SIZE(ionic_lif_stats_desc)
> @@ -185,6 +191,9 @@ static void ionic_add_lif_rxq_stats(struct ionic_lif *lif, int q_num,
>  	stats->rx_csum_error += rxstats->csum_error;
>  	stats->rx_hwstamp_valid += rxstats->hwstamp_valid;
>  	stats->rx_hwstamp_invalid += rxstats->hwstamp_invalid;
> +	stats->xdp_drop += rxstats->xdp_drop;
> +	stats->xdp_aborted += rxstats->xdp_aborted;
> +	stats->xdp_pass += rxstats->xdp_pass;
>  }
>  
>  static void ionic_get_lif_stats(struct ionic_lif *lif,
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> index aee38979a9d7..07a17be94d4d 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> @@ -177,7 +177,7 @@ static bool ionic_rx_buf_recycle(struct ionic_queue *q,
>  	if (page_to_nid(buf_info->page) != numa_mem_id())
>  		return false;
>  
> -	size = ALIGN(used, IONIC_PAGE_SPLIT_SZ);
> +	size = ALIGN(used, q->xdp_rxq_info ? IONIC_PAGE_SIZE : IONIC_PAGE_SPLIT_SZ);
>  	buf_info->page_offset += size;
>  	if (buf_info->page_offset >= IONIC_PAGE_SIZE)
>  		return false;
> @@ -287,6 +287,54 @@ static struct sk_buff *ionic_rx_copybreak(struct ionic_queue *q,
>  	return skb;
>  }
>  
> +static bool ionic_run_xdp(struct ionic_rx_stats *stats,
> +			  struct net_device *netdev,
> +			  struct ionic_queue *rxq,
> +			  struct ionic_buf_info *buf_info,
> +			  int len)
> +{
> +	u32 xdp_action = XDP_ABORTED;
> +	struct bpf_prog *xdp_prog;
> +	struct xdp_buff xdp_buf;
> +
> +	xdp_prog = READ_ONCE(rxq->lif->xdp_prog);
> +	if (!xdp_prog)
> +		return false;
> +
> +	xdp_init_buff(&xdp_buf, IONIC_PAGE_SIZE, rxq->xdp_rxq_info);
> +	xdp_prepare_buff(&xdp_buf, ionic_rx_buf_va(buf_info),
> +			 0, len, false);
> +
> +	dma_sync_single_range_for_cpu(rxq->dev, ionic_rx_buf_pa(buf_info),
> +				      0, len,
> +				      DMA_FROM_DEVICE);

in case of XDP_PASS the same buf will be synched twice ?!? 

Cheers,

Paolo


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ