> On 21 Jan 2015, at 5:50 am, Simon Mages <mages.si...@googlemail.com> wrote:
>
> Sorry, i did not test the Patch well enough.
>
> It is still broken, but in a different way.
>
> I think tedu forgot in his patch to reset d->bd_rdStart. From my point
> of view it
> should be zero after sleeping in this case. Because if you read again after a
> successful read the timeout wont be processed because of:
>
>> /*
>> * If there's a timeout, bd_rdStart is tagged when we start the read.
>> * we can then figure out when we're done reading.
>> */
>> if (d->bd_rtout != -1 && d->bd_rdStart == 0)
>> d->bd_rdStart = ticks;
>> else
>> d->bd_rdStart = 0;
>
> And zero is all the time smaller then the elapsed time in the second read.
>
> This would fix it:
> if (elapsed < d->bd_rtout) {
> error = tsleep(d, PRINET|PCATCH, "bpf",
> d->bd_rtout - elapsed);
> + d->bd_rdStart = 0;
> } else
> error = EWOULDBLOCK;
yes, that makes sense to me.
ill commit it when im back at work (tuesday the 27th around 11am gmt+10) unless
someone objects. or beats me too it.
dlg
>
> BR
> Simon
>
> 2015-01-07 9:11 GMT+01:00, Simon Mages <mages.si...@googlemail.com>:
>> I tested the patch and its working.
>>
>> I have a small test program already. I create a regression test with it.
>> I'll post the diff here.
>> Am 06.01.2015 04:19 schrieb "Philip Guenther" <guent...@gmail.com>:
>>
>>> [(@#*$&(*# control-enter keybinding]
>>>
>>> On Mon, Jan 5, 2015 at 7:15 PM, Philip Guenther <guent...@gmail.com>
>>> wrote:
>>>> On Mon, Jan 5, 2015 at 11:01 AM, Ted Unangst <t...@tedunangst.com>
>>> wrote:
>>>> ...
>>>>> In the regular timeout case, I'm not sure what you're changing. There
>>>>> is a problem here though. If we're already close to the timeout
>>>>> expiring, we shouldn't sleep the full timeout, only the time left
>>>>> since we began the read.
>>>
>>> Yes, that was what I was trying to convey in my reply to Mages's
>>> earlier post on this bpf issue.
>>>
>>> Your diff looks correct to me, though untested.
>>>
>>> Mages, do you have code this can be tested against? Is there
>>> something you could contribute to form a regress test we could place
>>> under /usr/src/regress/net/ to verify that we got this right and to
>>> catch breakage in the future?
>>>
>>>
>>> Philip Guenther
>>>
>>
>
