netdev - RE: [Xen-devel] [PATCH net-next v1] xen-netback: make copy batch size configurable

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:   Mon, 13 Nov 2017 11:03:44 +0000
From:   Paul Durrant <Paul.Durrant@...rix.com>
To:     'Jan Beulich' <JBeulich@...e.com>
CC:     Wei Liu <wei.liu2@...rix.com>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
        'Joao Martins' <joao.m.martins@...cle.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [Xen-devel] [PATCH net-next v1] xen-netback: make copy batch size
 configurable

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@...e.com]
> Sent: 13 November 2017 10:50
> To: Paul Durrant <Paul.Durrant@...rix.com>
> Cc: Wei Liu <wei.liu2@...rix.com>; xen-devel@...ts.xenproject.org; 'Joao
> Martins' <joao.m.martins@...cle.com>; netdev@...r.kernel.org
> Subject: Re: [Xen-devel] [PATCH net-next v1] xen-netback: make copy batch
> size configurable
> 
> >>> On 13.11.17 at 11:33, <Paul.Durrant@...rix.com> wrote:
> >> From: Joao Martins [mailto:joao.m.martins@...cle.com]
> >> Sent: 10 November 2017 19:35
> >> --- a/drivers/net/xen-netback/netback.c
> >> +++ b/drivers/net/xen-netback/netback.c
> >> @@ -96,6 +96,11 @@ unsigned int xenvif_hash_cache_size =
> >> XENVIF_HASH_CACHE_SIZE_DEFAULT;
> >>  module_param_named(hash_cache_size, xenvif_hash_cache_size, uint,
> >> 0644);
> 
> Isn't the "owner-write" permission here ...
> 
> >> --- a/drivers/net/xen-netback/rx.c
> >> +++ b/drivers/net/xen-netback/rx.c
> >> @@ -168,11 +168,14 @@ static void xenvif_rx_copy_add(struct
> >> xenvif_queue *queue,
> >>  			       struct xen_netif_rx_request *req,
> >>  			       unsigned int offset, void *data, size_t len)
> >>  {
> >> +	unsigned int batch_size;
> >>  	struct gnttab_copy *op;
> >>  	struct page *page;
> >>  	struct xen_page_foreign *foreign;
> >>
> >> -	if (queue->rx_copy.num == COPY_BATCH_SIZE)
> >> +	batch_size = min(xenvif_copy_batch_size, queue->rx_copy.size);
> >
> > Surely queue->rx_copy.size and xenvif_copy_batch_size are always
> identical?
> > Why do you need this statement (and hence stack variable)?
> 
> ... the answer to your question?

Yes, I guess it could be... but since there's no re-alloc code for the arrays I wonder whether the intention was to make this dynamic or not.

  Paul

> 
> Jan