[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9cbf19b5765746b88af6dadc71b99d78@XCH-RTP-016.cisco.com>
Date: Wed, 23 May 2018 11:54:50 +0000
From: "Jon Rosen (jrosen)" <jrosen@...co.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
CC: "David S. Miller" <davem@...emloft.net>,
Willem de Bruijn <willemb@...gle.com>,
Eric Dumazet <edumazet@...gle.com>,
Kees Cook <keescook@...omium.org>,
David Windsor <dwindsor@...il.com>,
"Rosen, Rami" <rami.rosen@...el.com>,
"Reshetova, Elena" <elena.reshetova@...el.com>,
"Mike Maloney" <maloney@...gle.com>,
Benjamin Poirier <bpoirier@...e.com>,
"Thomas Gleixner" <tglx@...utronix.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] packet: track ring entry use using a shadow ring to
prevent RX ring overrun
> > For the ring, there is no requirement to allocate exactly the amount
> > specified by the user request. Safer than relying on shared memory
> > and simpler than the extra allocation in this patch would be to allocate
> > extra shadow memory at the end of the ring (and not mmap that).
> >
> > That still leaves an extra cold cacheline vs using tp_padding.
>
> Given my lack of experience and knowledge in writing kernel code
> it was easier for me to allocate the shadow ring as a separate
> structure. Of course it's not about me and my skills so if it's
> more appropriate to allocate at the tail of the existing ring
> then certainly I can look at doing that.
The memory for the ring is not one contiguous block, it's an array of
blocks of pages (or 'order' sized blocks of pages). I don't think
increasing the size of each of the blocks to provided storage would be
such a good idea as it will risk spilling over into the next order and
wasting lots of memory. I suspect it's also more complex than a single
shadow ring to do both the allocation and the access.
It could be tacked onto the end of the pg_vec[] used to store the
pointers to the blocks. The challenge with that is that a pg_vec[] is
created for each of RX and TX rings so either it would have to
allocate unnecessary storage for TX or the caller will have to say if
extra space should be allocated or not. E.g.:
static struct pgv *alloc_pg_vec(struct tpacket_req *req, int order, int scratch, void **scratch_p)
I'm not sure avoiding the extra allocation and moving it to the
pg_vec[] for the RX ring is going to get the simplification you were
hoping for. Is there another way of storing the shadow ring which
I should consider?
Powered by blists - more mailing lists