From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [RFC PATCH v2] ptr_ring: linked list fallback Date: Wed, 28 Feb 2018 17:43:06 +0200 Message-ID: <20180228174100-mutt-send-email-mst@kernel.org> References: <1519607771-20613-1-git-send-email-mst@redhat.com> <01aff5eb-a92f-2170-05f7-664220985070@redhat.com> <20180226223252-mutt-send-email-mst@kernel.org> <0316016a-717b-9d3f-5aef-dccaf34d0fae@redhat.com> <20180227190703-mutt-send-email-mst@kernel.org> <20180228060845-mutt-send-email-mst@kernel.org> <11ef5721-1a4c-f216-9f46-08a0ad0ca49d@redhat.com> <20180228155121-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: linux-kernel@vger.kernel.org, John Fastabend , netdev@vger.kernel.org, David Miller To: Jason Wang Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Wed, Feb 28, 2018 at 10:20:33PM +0800, Jason Wang wrote: > > > On 2018年02月28日 22:01, Michael S. Tsirkin wrote: > > On Wed, Feb 28, 2018 at 02:28:21PM +0800, Jason Wang wrote: > > > > > > On 2018年02月28日 12:09, Michael S. Tsirkin wrote: > > > > > > Or we can add plist to a union: > > > > > > > > > > > > > > > > > > struct sk_buff { > > > > > > union { > > > > > > struct { > > > > > > /* These two members must be first. */ > > > > > > struct sk_buff *next; > > > > > > struct sk_buff *prev; > > > > > > union { > > > > > > struct net_device *dev; > > > > > > /* Some protocols might use this space to store information, > > > > > > * while device pointer would be NULL. > > > > > > * UDP receive path is one user. > > > > > > */ > > > > > > unsigned long dev_scratch; > > > > > > }; > > > > > > }; > > > > > > struct rb_node rbnode; /* used in netem & tcp stack */ > > > > > > + struct plist plist; /* For use with ptr_ring */ > > > > > > }; > > > > > > > > > > > This look ok. > > > > > > > > > > > > For XDP, we need to embed plist in struct xdp_buff too, > > > > > > Right - that's pretty straightforward, isn't it? > > > > > Yes, it's not clear to me this is really needed for XDP consider the lock > > > > > contention it brings. > > > > > > > > > > Thanks > > > > The contention is only when the ring overflows into the list though. > > > > > > > Right, but there's usually a mismatch of speed between producer and > > > consumer. In case of a fast producer, we may get this contention very > > > frequently. > > > > > > Thanks > > This is not true in my experiments. In my experiments, ring size of 4k > > bytes is enough to see packet drops in single %s of cases. > > > > To you have workloads where rings are full most of the time? > > E.g using xdp_redirect to redirect packets from ixgbe to tap. In my test, > ixgeb can produce ~8Mpps. But vhost can only consume ~3.5Mpps. Then you are better off just using a small ring and dropping packets early, right? > > > > One other nice side effect of this patch is that instead of dropping > > packets quickly it slows down producer to match consumer speeds. > > In some case, producer may not want to be slowed down, e.g in devmap which > can redirect packets into several different interfaces. > > IOW, it can go either way in theory, we will need to test and see the effect. > > > > Yes. > > Thanks