Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 5 Dec 2013 15:10:29 -0800
From:      Adrian Chadd <adrian@freebsd.org>
To:        Adrian Chadd <adrian@freebsd.org>, Michael Tuexen <Michael.Tuexen@lurchi.franken.de>,  Yong-Hyeon Pyun <pyunyh@gmail.com>, Jack F Vogel <jfv@freebsd.org>,  "freebsd-net@freebsd.org list" <freebsd-net@freebsd.org>
Subject:   Re: A small fix for if_em.c, if_igb.c, if_ixgbe.c
Message-ID:  <CAJ-VmonMRgMoJSe8x4Jk6iPB%2BMTV3___hdCd7LnEus=OHNnDmQ@mail.gmail.com>
In-Reply-To: <20131205223711.GB55638@funkthat.com>
References:  <521B9C2A-EECC-4412-9F68-2235320EF324@lurchi.franken.de> <20131202022338.GA3500@michelle.cdnetworks.com> <B9593E83-E687-49E9-ABDC-B2DD615180E9@lurchi.franken.de> <20131203021658.GC2981@michelle.cdnetworks.com> <CAJ-Vmo=kfoPMYjZ0WAtqmoJMz1utXH50SW9N92RA83EMUzY7WA@mail.gmail.com> <B89B1E2D-BAF0-4815-B3AB-EB226F4F76DE@lurchi.franken.de> <CAJ-Vmo=4Zwv5V6ZYDuDLtt%2BowgbvmqyvrnrfnU%2BHeXQ3vAn-KA@mail.gmail.com> <20131205223711.GB55638@funkthat.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 5 December 2013 14:37, John-Mark Gurney <jmg@funkthat.com> wrote:
> Adrian Chadd wrote this message on Thu, Dec 05, 2013 at 14:01 -0800:
>> On 5 December 2013 13:05, Michael Tuexen
>> <Michael.Tuexen@lurchi.franken.de> wrote:
>>
>> > Just to be clear: This would mean that xxx_transmit() would return
>> > an error even if the packet provided in the call xxx_transmit() is
>> > enqueued and not dropped?
>> > This would also be problem with the current SCTP stack.
>>
>> I think it'll return an error only if:
>>
>> * it queued the frame to the tail of the drbd;
>> * it then tried to transmit a frame from the head of the drbd;
>> * it failed to transmit the first frame in the drbd and it couldn't
>> put it back into the queue for whatever reason.
>>
>> So I think it should be "ok enough" for both TCP and SCTP.
>
> IMO it should only return an error if the specific frame failed to be
> sent or queued.  If you cannot determine at return time if the frame
> failed to be transmitted/queued, then it should return success.

For the long term solution, I agree.

> In the above case, if there were other frames queued ahead, and the
> first one failed, then it sounds like the frame may eventually be sent
> and we will end up sending a duplicate frame.

Right. We should also fix this properly.

I think the right thing, long term, is something like this;

* xxx_mq_start_locked() returns whether the head frame was transmitted or not;
* the if_transmit() entry point(s) return whether the given frame was
queued to the software queue or not;
* the if_transmit() entry point(s) ignore the return value of
xxx_mq_start_locked(), as the stack _should_ handle the case of a
frame handed to the driver but dropped.

So, I'd like to get Michael to first test fixing up
xxx_mq_start_locked() to only return an error if it failed to transmit
a frame and the frame was dropped. Then, once we get feedback from
that, I was going to propose that we also do what Michael initially
did - and that's ignore the error from calling xxx_mq_start_locked().
Followed, hopefully, with some comments explaining how this all holds
together.

How's that sound?



-adrian



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmonMRgMoJSe8x4Jk6iPB%2BMTV3___hdCd7LnEus=OHNnDmQ>