Opened 3 weeks ago

#23709 new defect

channel: `outgoing_queue` and `incoming_queue` are always empty

Reported by: dgoulet Owned by:
Priority: High Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-sched
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

  1. Let's start with the incoming_queue of a channel_t:

The channel_queue_(var)cell() function is the one taking a single cell and putting it in the in queue.

If the queue is empty, it processes the cell immediately.
If the queue is NOT empty, it puts the cell in the queue and processes the cell immediately if we have cell handlers which is currently always the case.

The "process cell" function is in charge of removing the cell from the queue. So I think you can clearly see the problem with this code flow ;).

  1. Now outgoing_queue of a channel_t:

Inserting a cell in that queue is with channel_write_cell_queue_entry() which does it IFF the outgoing_queue is not empty and channel is open. If the queue is empty, the cell is processed immediately with the write_cell handler.

If the cell was for whatever reason not able to be sent by the write handler, the cell is then put in the outgoing_queue. However, the only possible handler we have right now is channel_tls_write_cell_method() (and packed/var friends) that always return 1 if the tlschan->conn exists. Empirical test shows that it is really ALWAYS the case that a conn exists on the tls channel.

---

Bottom line, those queues are as good as local to a function but we do consider them everywhere in the code such as in channel_flush_some_cells() or channel_more_to_flush().

We should either get rid of them or use them properly. For KIST, this is a big deal though to have them work properly because once data goes from a cmux queue to the outbuf of the connection, libevent will kick in to send() the data from the outbuf of a connection. KIST needs to have this steps where once it puts the data in the outbuf it then knows it has to write to kernel. If it can NOT write to kernel, the data needs to stay in the out queue else libevent will write the data from the outbuf to kernel without the scheduler knowing about it.

But because those queues are basically not usable outside their functions, everytime we queue a cell, it is put immediately in the outbuf. Fortunately, KIST does control a bit of it by doing the flush from the cmux to the outbuf (channel_flush_some_cells()) and then tries to write to kernel what has been flushed. But is seems that tor can put cells in the outbuf in other places which is not ideal...

If we can make the scheduler the *only* one capable of flushing the cmux to the out buf, we could get rid of those queues and only use the cmux queue. The scheduler does NOT care about the incoming_queue at all.

Child Tickets

Change History (0)

Note: See TracTickets for help on using tickets.