Date: Thu, 21 Nov 2013 01:02:45 +0100 From: Luigi Rizzo <rizzo@iet.unipi.it> To: George Neville-Neil <gnn@freebsd.org> Cc: Adrian Chadd <adrian@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, FreeBSD Net <freebsd-net@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r258328 - head/sys/net Message-ID: <20131121000245.GA30549@onelab2.iet.unipi.it> In-Reply-To: <023E719B-1059-4670-8556-EBAC18A2F007@freebsd.org> References: <201311182258.rAIMwEFd048783@svn.freebsd.org> <CAJ-VmomjQrq39jafTTGQA_EJLSi5j%2BNB=g1sLwCK-KaEfgwrbw@mail.gmail.com> <023E719B-1059-4670-8556-EBAC18A2F007@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Nov 20, 2013 at 05:29:37PM -0500, George Neville-Neil wrote: > > On Nov 20, 2013, at 17:02 , Adrian Chadd <adrian@freebsd.org> wrote: > > > Can we revert this and just quickly put together something else that > > won't potentially come back to bite us with weird side effects? > > > > My suggestions (and I'm happy to do this work if needed): > > > > * create a lightweight mbuf queue representation so an mbuf list isn't > > just the mbuf header pointer; > > * create a new ether input (say, ether_input_multi) that takes an mbuf > > list, so existing driver code does the right thing. > > > > After that it'd be nice to write a set of mbuf list macros for > > abstract the whole queue, dequeue, concat, iterate, etc (like > > sys/queue.h, but for mbufs.) > > > > What do people think? > > > > (I've been doing it for m->next chained things, but not m->m_nextpkt things..) > > I think the right way to do this is to move forwards and not backwards. > This change has the nice effect of not breaking anything else in the > tree while providing us with a feature that will be useful. I am a bit torn on this. George is right - we should move forward, and we have been discussing this trivial change for two years. It is impossible to tell when someone will find the time to implement the mbuf queue and the new method (more on this later). So we should not back it out. At the same time: in principle this change does not break anything, but Robert is right that some of the 50-100 drivers we have might forget to clear m_nextpkt and cause trouble. So it would be nice to implement some protection, maybe something as simple as setting a flag in the first mbuf to tell that this is a chain and not a single mbuf. We have 12 M_PROTO* flags to use. For the next step: I am still wondering what is the best way to implement an alternate ifp->if_input_multi method (which takes an mbuf queue as an argument, and can be checked by the compiler). I am not too fond of having two different input methods, as it complicates life when one has to intercept packets. On the other hand, globally changing all drivers to use the if_input_multi() method is too much trouble, so i suspect we will have to come up with some nice trick, e.g. use a pseudo mbuf with m_type = MT_MBQUEUE and overload the m_hdr fields as queue pointers. cheers luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131121000245.GA30549>