Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible bug in weighted_fair when resouce is 'bit' #717

Closed
nemethf opened this issue Nov 8, 2017 · 5 comments
Closed

Possible bug in weighted_fair when resouce is 'bit' #717

nemethf opened this issue Nov 8, 2017 · 5 comments

Comments

@nemethf
Copy link
Contributor

nemethf commented Nov 8, 2017

I think the current behavior of the wighted_fair scheduler when the resource is set to 'bit' is counter-intuitive. Take a look at the following bess script:

bess.add_tc('root', 'weighted_fair', wid=0, resource='bit')

tagger= SetMetadata(attrs=[{'name': 'tag', 'size': 4, 'value_int': 0}])
split = Split(attribute='tag', size=4)

s::Source() -> tagger -> split

for i in range(2):
  q = Queue()
  name = 't-%s' % i
  bess.add_tc(name=name, parent='root', policy='round_robin', share=1)
  q.attach_task(parent=name)
  split:i -> q -> Sink()

bess.attach_task(s.name, wid=0)

Running the script results in a dead-lock because the split module never sends a packet to the second queue, and therefore the queue never forwards a packet although it always 'runnable'.

+--------+                   +-------------+                   +----------+                   +-----------+             +-------+
|   s    |                   |  setattr0   |                   |  split0  |                   |  queue0   |             | sink0 |
| Source |  :0 81434528 0:   | SetMetadata |  :0 81475136 0:   |  Split   |  :0 81579712 0:   |   Queue   |  :0 32 0:   | Sink  |
|        | ----------------> |             | ----------------> |          | ----------------> | 1023/1024 | ----------> |       |
+--------+                   +-------------+                   +----------+                   +-----------+             +-------+
                                                                 |
                                                                 | :1 0 0:
                                                                 v
                                                               +----------+                   +-----------+
                                                               |  queue1  |                   |   sink1   |
                                                               |  Queue   |  :0 0 0:          |   Sink    |
                                                               |  0/1024  | ----------------> |           |
                                                               +----------+                   +-----------+

<worker 0>
  +-- !default_rr_0            round_robin
      +-- root                 weighted_fair
      |   +-- t-0              round_robin         share: 1
      |   |   +-- !leaf_queue0:0 leaf
      |   +-- t-1              round_robin         share: 1
      |       +-- !leaf_queue1:0 leaf
      +-- !leaf_s:0            leaf

I somehow expected that the scheduler would choose queue0 because queue1 was empty and queue0 would opportunistically work all the time. Now that I understand the current behavior I can live with it, but still wonder whether it is intended. Thanks.

@sangjinhan
Copy link
Member

There is an experimental work-conserving scheduler. If you add the following line at the beginning of the script,

bess.add_worker(wid=0, core=0, scheduler='experimental')

Then t-0 will be scheduled even if t-1 is not 'runnable'.

+--------+                   +-------------+                   +----------+                   +---------+                   +-------+
|   s    |                   |  setattr0   |                   |  split0  |                   | queue0  |                   | sink0 |
| Source |  :0 60931476 0:   | SetMetadata |  :0 60931395 0:   |  Split   |  :0 60931323 0:   |  Queue  |  :0 60931690 0:   | Sink  |
|        | ----------------> |             | ----------------> |          | ----------------> | 64/1024 | ----------------> |       |
+--------+                   +-------------+                   +----------+                   +---------+                   +-------+
                                                                 |
                                                                 | :1 0 0:
                                                                 v
                                                               +----------+                   +---------+
                                                               |  queue1  |                   |  sink1  |
                                                               |  Queue   |  :0 0 0:          |  Sink   |
                                                               |  0/1024  | ----------------> |         |
                                                               +----------+                   +---------+

@nemethf
Copy link
Contributor Author

nemethf commented Nov 14, 2017

The experimental scheduler seems very interesting.

However, it turned out what I really want is something like a WFQ (or General Processor Sharing) packet/traffic scheduler where every session/flow has a minimum guaranteed rate (calculated from associated weights); but if a source is silent, then the excess bandwidth is distributed among the active sessions according to the weights as well. I thought task scheduling might be good for that, but I'm not so sure anymore.

I repeated my original measurement with the scheduler set to 'experimental', and after a while I added another source that is routed to queue1. (I also modified the 'run' command not to clear the pipeline.) These are the results:

$  ./bessctl daemon reset -- run wfq -- show pipeline -- run wfq-2 -- show pipeline
+--------+                    +-------------+                    +----------+                    +----------+                    +-------+
|   s    |                    |  setattr0   |                    |  split0  |                    |  queue0  |                    | sink0 |
| Source |  :0 358922784 0:   | SetMetadata |  :0 358937376 0:   |  Split   |  :0 358980480 0:   |  Queue   |  :0 358885472 0:   | Sink  |
|        | -----------------> |             | -----------------> |          | -----------------> | 384/1024 | -----------------> |       |
+--------+                    +-------------+                    +----------+                    +----------+                    +-------+
                                                                   |
                                                                   | :1 0 0:
                                                                   v
                                                                 +----------+                    +----------+
                                                                 |  queue1  |                    |  sink1   |
                                                                 |  Queue   |  :0 0 0:           |   Sink   |
                                                                 |  0/1024  | -----------------> |          |
                                                                 +----------+                    +----------+


                                                                 +------------------+                    +-----------+
                                                                 |      queue1      |                    |   sink1   |
                                                                 |      Queue       |  :0 324628160 0:   |   Sink    |
                                                                 |    1023/1024     | -----------------> |           |
                                                                 +------------------+                    +-----------+
                                                                   ^
                                                                   | :1 324702944 0:
                                                                   |
+--------+                    +-------------+                    +------------------+                    +-----------+                    +-------+
|   s2   |                    |  setattr1   |                    |      split0      |                    |  queue0   |                    | sink0 |
| Source |  :0 324661088 0:   | SetMetadata |  :0 324679040 0:   |      Split       |  :0 687509184 0:   |   Queue   |  :0 362820224 0:   | Sink  |
|        | -----------------> |             | -----------------> |                  | -----------------> | 1023/1024 | -----------------> |       |
+--------+                    +-------------+                    +------------------+                    +-----------+                    +-------+
                                                                   ^
                                                                   |
                                                                   |
+--------+                    +-------------+                      |
|   s    |  :0 687459040 0:   |  setattr0   |  :0 687475520 0:     |
| Source | -----------------> | SetMetadata | ---------------------+
+--------+                    +-------------+

So in the first phase queue0 served alone (receiving all the available resources), but in the second phase queue0 and queue1 do not equally share the resources: queue1 receives almost all of them.


Do you think it is possible to configure bess to provide a WFQ type of traffic scheduling without writing a new module? Thank you.


For reproducability:

  • wfq.bess
import time
bess.add_worker(wid=0, core=0, scheduler='experimental')
bess.add_tc('root', 'weighted_fair', wid=0, resource='bit')

tagger= SetMetadata(attrs=[{'name': 'tag', 'size': 4, 'value_int': 0}])
split = Split(attribute='tag', size=4)

s::Source() -> tagger -> split

for i in range(2):
  q = Queue()
  name = 't-%s' % i
  bess.add_tc(name=name, parent='root', policy='round_robin', share=1)
  q.attach_task(parent=name)
  split:i -> q -> Sink()

bess.attach_task(s.name, wid=0)

bess.resume_all()
time.sleep(6)
  • wfq-2.bess
import time

bess.pause_all()
tagger2 = SetMetadata(attrs=[{'name': 'tag', 'size': 4, 'value_int': 1}])
s2::Source() -> tagger2
bess.connect_modules(tagger2.name, 'split0')
bess.attach_task(s2.name, wid=0)
bess.resume_all()
time.sleep(10)
bess.pause_all()

@barath
Copy link
Contributor

barath commented Nov 14, 2017

I don't know if it's of use, but we already have a DRR implementation and there is a defunct (but fixable) PR adding Codel to the DRR implementation:

#476

@sangjinhan
Copy link
Member

Thank you for the detailed bug scenario. I found a bug in weighted_fair traffic class when used with the experimental scheduler. I will make a PR for this soon.

@melvinw
Copy link
Contributor

melvinw commented Nov 29, 2017

Merged #729. Closing

@melvinw melvinw closed this as completed Nov 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants