netdev - Re: [PATCH net-next 6/6] mvneta: recycle buffers

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]

Date:   Wed, 24 Mar 2021 10:28:35 +0100
From:   Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Matteo Croce <mcroce@...ux.microsoft.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        Saeed Mahameed <saeedm@...dia.com>,
        David Ahern <dsahern@...il.com>,
        Saeed Mahameed <saeed@...nel.org>, Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH net-next 6/6] mvneta: recycle buffers

[...]
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index a635cf84608a..8b3250394703 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -2332,7 +2332,7 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
> >  	if (!skb)
> >  		return ERR_PTR(-ENOMEM);
> >  
> > -	page_pool_release_page(rxq->page_pool, virt_to_page(xdp->data));
> > +	skb_mark_for_recycle(skb, virt_to_page(xdp->data), &xdp->rxq->mem);
> >  
> >  	skb_reserve(skb, xdp->data - xdp->data_hard_start);
> >  	skb_put(skb, xdp->data_end - xdp->data);
> > @@ -2344,7 +2344,7 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
> >  		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> >  				skb_frag_page(frag), skb_frag_off(frag),
> >  				skb_frag_size(frag), PAGE_SIZE);
> > -		page_pool_release_page(rxq->page_pool, skb_frag_page(frag));
> > +		skb_mark_for_recycle(skb, skb_frag_page(frag), &xdp->rxq->mem);
> >  	}
> >  
> >  	return skb;
> 
> This cause skb_mark_for_recycle() to set 'skb->pp_recycle=1' multiple
> times, for the same SKB.  (copy-pasted function below signature to help
> reviewers).
> 
> This makes me question if we need an API for setting this per page
> fragment?
> Or if the API skb_mark_for_recycle() need to walk the page fragments in
> the SKB and set the info stored in the page for each?

Considering just performances, I guess it is better open-code here since the
driver already performs a loop over fragments to build the skb, but I guess
this approach is quite risky and I would prefer to have a single utility
routine to take care of linear area + fragments. What do you think?

Regards,
Lorenzo

> 
> 
> -- 
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)