On Fri, Nov 20, 2015 at 9:00 AM, Reyk Floeter <r...@openbsd.org> wrote:
> On Fri, Nov 20, 2015 at 11:27:05PM +1000, David Gwynne wrote: > > > > > On 20 Nov 2015, at 11:23 PM, Reyk Floeter <r...@openbsd.org> wrote: > > > > > > On Fri, Nov 20, 2015 at 03:36:40PM +1000, David Gwynne wrote: > > >> IFF_OACTIVE means the hardware ring is full, not if it is busy. > > >> > > >> perhaps a better check is to see whether there are pending packets > > >> on the send queue? > > >> > > >> i could also argue we dont need the check at all, but this is less > > >> of a semantic change. > > >> > > >> ok? > > >> > > > > > > OK > > > > > > I added this check in the initial commit of trunk(4) more than 10 > > > years ago. I don't remember a particular reason - there wasn't even a > > > production use yet. I initially wrote trunk to use it for failover on > > > some firewalls, but it was not going into production before it was > > > committed to OpenBSD. > > > > > > So the reason for the flag might just be a historical one: back in the > > > days, I heard that 10 years is a long time in IT, there was not much > > > reference about implementing such a "cloner" correctly. I must have > > > looked at vlan(4) or bridge(4) and decided that it was the way to do. > > > I mean, before mpi@, working in the network stack was tough and very > > > different: you didn't ask, never refered to any documentation, just > > > relied on the traditions and repeated what was done by the BSD heroes > > > in the past. > > > > thats exactly right. please dont take this as an accusation of > negligence, this is just mopping up some of that inherited code. > > > > im upset about my own use of IFQ_POLL() in a bunch of my own drivers, > but really i am only guilty of what you described above too. ie, we're not > guilty, we were just following best practices at the time. > > > > Hehe, i'm not feeling accused. I think it is a lot of fun to see all > the progress and cleanup happening. I just wanted to tell a story ... > and to point out that some of the obvious mistakes of the past weren't > so obvious to us back then. > > > how would you feel if i simply removed the check? > > > > fine! > > Reyk > > > >> Index: if_trunk.c > > >> =================================================================== > > >> RCS file: /cvs/src/sys/net/if_trunk.c,v > > >> retrieving revision 1.124 > > >> diff -u -p -r1.124 if_trunk.c > > >> --- if_trunk.c 20 Nov 2015 05:33:54 -0000 1.124 > > >> +++ if_trunk.c 20 Nov 2015 05:35:07 -0000 > > >> @@ -296,7 +296,7 @@ trunk_port_create(struct trunk_softc *tr > > >> return (ENOSPC); > > >> > > >> /* New trunk port has to be in an idle state */ > > >> - if (ifp->if_flags & IFF_OACTIVE) > > >> + if (!ifq_empty(&ifp->if_snd)) > > >> return (EBUSY); > > >> > > >> /* Check if port has already been associated to a trunk */ > > > > > > -- > > > > -- > > I have a trunk usage currently, and when the trunk is created, the first data to go through suffer a very high latency. After less than 5 seconds, everything runs smoothly (same latency as each link) Could it be related ? -- --------------------------------------------------------------------------------------------------------------------- () ascii ribbon campaign - against html e-mail /\