[Click] Possible bug in userlevel ToDevice run_task function in Click 2.0

jaeyong yoo y.jaeyong at gmail.com
Thu Sep 15 02:06:58 EDT 2011


Hello Eddie,

While I was using Click ToDevice userlevel element, ToDevice suddenly stops
pulling packets from a notifierqueue even though the ActiveNotifier signals
"wake".
After some debugging, I think the problem is the following.

First of all, I'm using PCAP as capture method.

In ToDevice::run_task(Task *) function, when "send_packet(p)" function
returns -ENOBUFS or -EAGAIN, it backs off.
But, when the backoff is just starting (means when _backoff is 0), it uses
add_select(_fd, SELECT_WRITE) function to register a callback and no other
callbacks (fast_reschedule, etc) are registered (it looks reasonable).
But the problem arises when we use PCAP, because _fd is -1 and the
add_select does not properly register callback and eventually run_task never
called again.
Even ActiveNotifier can not wake this run_task up.

I think the problematic code is the following.

    if (r == -ENOBUFS || r == -EAGAIN) {
        assert(!_q);
        _q = p;

        if (!_backoff) {
            _backoff = 1;
            add_select(_fd, SELECT_WRITE);
        } else {
            _timer.schedule_after(Timestamp::make_usec(_backoff));
            if (_backoff < 32768)
                _backoff *= 2;
            if (_debug) {
                Timestamp now = Timestamp::now();
                click_chatter("%{element} backing off for %d at
%{timestamp}\n", this, _backoff, &now);
            }
        }
        return count > 0;

and should be changed something like the following.

    if (r == -ENOBUFS || r == -EAGAIN) {
        assert(!_q);
        _q = p;

        if (!_backoff) {
            _backoff = 1;
#if TODEVICE_ALLOW_PCAP
            _timer.schedule_after(Timestamp::make_usec(_backoff));
#endif

#if TODEVICE_ALLOW_LINUX
            add_select(_fd, SELECT_WRITE);
#endif
        } else {
            _timer.schedule_after(Timestamp::make_usec(_backoff));
            if (_backoff < 32768)
                _backoff *= 2;
            if (_debug) {
                Timestamp now = Timestamp::now();
                click_chatter("%{element} backing off for %d at
%{timestamp}\n", this, _backoff, &now);
            }
        }
        return count > 0;


Best regards,
Jaeyong


More information about the click mailing list