netdev - Re: [oss-drivers] Re: [PATCH net-next v2 7/9] nfp: add metadata to each flow offload

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: <20170629112213.GB24136@vergenet.net>
Date:   Thu, 29 Jun 2017 13:22:14 +0200
From:   Simon Horman <simon.horman@...ronome.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        oss-drivers@...ronome.com,
        Pieter Jansen van Vuuren 
        <pieter.jansenvanvuuren@...ronome.com>
Subject: Re: [oss-drivers] Re: [PATCH net-next v2 7/9] nfp: add metadata to
 each flow offload

On Thu, Jun 29, 2017 at 01:42:50AM -0700, Jakub Kicinski wrote:
> On Thu, 29 Jun 2017 10:14:29 +0200, Simon Horman wrote:
> > > > +	/* Checking if buffer is empty. */
> > > > +	if (ring->head == ring->tail) {
> > > > +		*mask_id = freed_id;
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	memcpy(&temp_id, &ring->buf[ring->tail], NFP_FLOWER_MASK_ELEMENT_RS);
> > > > +	*mask_id = temp_id;
> > > > +	memcpy(&ring->buf[ring->tail], &freed_id, NFP_FLOWER_MASK_ELEMENT_RS);
> > > > +	ring->tail = (ring->tail + NFP_FLOWER_MASK_ELEMENT_RS) %
> > > > +		     (NFP_FLOWER_MASK_ENTRY_RS * NFP_FLOWER_MASK_ELEMENT_RS);
> > > > +
> > > > +	getnstimeofday64(&now);
> > > > +	delta = timespec64_sub(now, priv->mask_ids.last_used[*mask_id]);
> > > > +
> > > > +	if (timespec64_to_ns(&delta) < NFP_FL_MASK_REUSE_TIME_NS) {
> > > > +		nfp_release_mask_id(app, *mask_id);  
> > > 
> > > nfp_release_mask_id() will reset the time stamp and put the mask at the
> > > end of the queue.  Is that OK?  
> > 
> > I discussed this with Pieter. He believes that it is ok as it would be too
> > early to use the entry and its better put it to the back of the list and
> > skip to the next one.
> 
> But we shouldn't update the "last use" time if the grace period haven't
> elapsed otherwise we can live lock (I know, unlikely).  Could we simply
> move the time check right after the:
> 
> 	*mask_id = temp_id;
> 
> line?  I.e. move the check before we actually pick the mask off the
> queue?

Thanks, we now have a solution along those lines working.