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;
BR
Simon
2015-01-07 9:11 GMT+01:00, Simon Mages <[email protected]>:
> 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" <[email protected]>:
>
>> [(@#*$&(*# control-enter keybinding]
>>
>> On Mon, Jan 5, 2015 at 7:15 PM, Philip Guenther <[email protected]>
>> wrote:
>> > On Mon, Jan 5, 2015 at 11:01 AM, Ted Unangst <[email protected]>
>> 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
>>
>
