netdev - Re: [PATCH net-next 5/6] ionic: implement support for rx sgl

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: <20191022163732.102f357c@cakuba.netronome.com>
Date:   Tue, 22 Oct 2019 16:37:32 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Shannon Nelson <snelson@...sando.io>
Cc:     netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH net-next 5/6] ionic: implement support for rx sgl

On Tue, 22 Oct 2019 13:31:12 -0700, Shannon Nelson wrote:
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> index ab6663d94f42..8c96f5fe43a2 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> @@ -34,52 +34,107 @@ static inline struct netdev_queue *q_to_ndq(struct ionic_queue *q)
>  	return netdev_get_tx_queue(q->lif->netdev, q->index);
>  }
>  
> -static void ionic_rx_recycle(struct ionic_queue *q, struct ionic_desc_info *desc_info,
> -			     struct sk_buff *skb)
> +static struct sk_buff *ionic_rx_skb_alloc(struct ionic_queue *q,
> +					  unsigned int len, bool frags)
>  {
> -	struct ionic_rxq_desc *old = desc_info->desc;
> -	struct ionic_rxq_desc *new = q->head->desc;
> +	struct ionic_lif *lif = q->lif;
> +	struct ionic_rx_stats *stats;
> +	struct net_device *netdev;
> +	struct sk_buff *skb;
> +
> +	netdev = lif->netdev;
> +	stats = q_to_rx_stats(q);
>  
> -	new->addr = old->addr;
> -	new->len = old->len;
> +	if (frags)
> +		skb = napi_get_frags(&q_to_qcq(q)->napi);
> +	else
> +		skb = netdev_alloc_skb_ip_align(netdev, len);
>  
> -	ionic_rxq_post(q, true, ionic_rx_clean, skb);
> +	if (unlikely(!skb)) {
> +		net_warn_ratelimited("%s: SKB alloc failed on %s!\n",
> +				     netdev->name, q->name);
> +		stats->alloc_err++;
> +		return NULL;
> +	}
> +
> +	return skb;
>  }
>  
> -static bool ionic_rx_copybreak(struct ionic_queue *q, struct ionic_desc_info *desc_info,
> -			       struct ionic_cq_info *cq_info, struct sk_buff **skb)
> +static struct sk_buff *ionic_rx_frags(struct ionic_queue *q,
> +				      struct ionic_desc_info *desc_info,
> +				      struct ionic_cq_info *cq_info)
>  {
>  	struct ionic_rxq_comp *comp = cq_info->cq_desc;
> -	struct ionic_rxq_desc *desc = desc_info->desc;
> -	struct net_device *netdev = q->lif->netdev;
>  	struct device *dev = q->lif->ionic->dev;
> -	struct sk_buff *new_skb;
> -	u16 clen, dlen;
> -
> -	clen = le16_to_cpu(comp->len);
> -	dlen = le16_to_cpu(desc->len);
> -	if (clen > q->lif->rx_copybreak) {
> -		dma_unmap_single(dev, (dma_addr_t)le64_to_cpu(desc->addr),
> -				 dlen, DMA_FROM_DEVICE);
> -		return false;
> -	}
> +	struct ionic_page_info *page_info;
> +	struct sk_buff *skb;
> +	unsigned int i;
> +	u16 frag_len;
> +	u16 len;
>  
> -	new_skb = netdev_alloc_skb_ip_align(netdev, clen);
> -	if (!new_skb) {
> -		dma_unmap_single(dev, (dma_addr_t)le64_to_cpu(desc->addr),
> -				 dlen, DMA_FROM_DEVICE);
> -		return false;
> -	}
> +	page_info = &desc_info->pages[0];
> +	len = le16_to_cpu(comp->len);
>  
> -	dma_sync_single_for_cpu(dev, (dma_addr_t)le64_to_cpu(desc->addr),
> -				clen, DMA_FROM_DEVICE);
> +	prefetch(page_address(page_info->page) + NET_IP_ALIGN);
>  
> -	memcpy(new_skb->data, (*skb)->data, clen);
> +	skb = ionic_rx_skb_alloc(q, len, true);
> +	if (unlikely(!skb))
> +		return NULL;
>  
> -	ionic_rx_recycle(q, desc_info, *skb);
> -	*skb = new_skb;
> +	i = comp->num_sg_elems + 1;
> +	do {
> +		if (unlikely(!page_info->page)) {
> +			dev_kfree_skb(skb);
> +			return NULL;
> +		}

Would you not potentially free the napi->skb here? is that okay?