librelist archives

« back to archive

Clones of clones

Clones of clones

From:
Bro, Lars
Date:
2014-11-24 @ 13:32
Hi, List

I have a case where I need to repeat the same packet over and over again 
until it is replaced by a new. The input goes into a FrontDropQueue, and 
every time a packet is pulled for output, a copy is pushed back into the 
queue. When a new packet arrives, it replaces the current.

However, the problem I would like to describe is not related to the 
FrontDropQueue, so below is an example with an ordinary Queue. One packet 
is pushed into the Queue, and I would expect that a continuous infinite 
output  would be generated with copies of that packet.

// makes just one packet
InfiniteSource(LIMIT 1) ->
q::Queue(CAPACITY 1) ->
t::PullTee() ->
// The sink
Discard();
// the loop back
p[1] -> q

Instead it crashes, eating up all memory.

A slight modification in order to rule out PullTee is for example:

InfiniteSource(LIMIT 1) ->
q::Queue(CAPACITY 1) ->
Unqueue() ->
t::Tee();

t[1] -> Discard();
t[0] -> q;

Also crashes. But if it is modified like this:

t[0] -> Discard();
t[1] -> q;

Then it works.

Looking at the code of Tee(), it says

int n = noutputs()
for (int i = 0; i < n - 1; i++)
  if (Packet *q = p->clone())
    output(i).push(q)
  output(n-1).push(p)

This means that the last output is the packet itself, any other outputs 
get clones.

The "working" case is when the packet itself (t[1]) is put back on the 
queue, but the failing cases are when a clone is put back instead. This 
happens with the PullTee() example and with the first Tee() example.

So, we can make any number of clones of a packet and discard them.

But if we make clones and then clones of the clones, it goes wrong.

Now, I need this in a Pull context, as provided by PullTee(). However, 
PullTee() emits a clones on all of its outputs. I suggest a new Push 
element that makes a non-clone. For example

t[1]-> Uniqueify() -> q;

Any opinions?

Mit freundlichen Grüßen
Lars Bro

Siemens A/S
RC-DK MO DEV R&D-A-COC
Borupvang 9
2750 Ballerup, Dänemark
Tel.: +45 4477-4171
mailto:lars.bro@siemens.com

Siemens A/S. Headquarters: Borupvang 9, 2750 Ballerup, Denmark. Tel: +45 
4477 4477 CVR-no. 16 99 30 85

Re: [click] Clones of clones

From:
Braem Bart
Date:
2014-11-25 @ 10:19
Hi Lars and list,

Are you sure this is not just a bug in the cloning, triggered by the 
element flow timing?
It seems Uniqueify would just be hack to work around this issue?

best regards,
Bart

On 24 Nov 2014, at 14:32, Bro, Lars 
<lars.bro@siemens.com<mailto:lars.bro@siemens.com>> wrote:

Hi, List

I have a case where I need to repeat the same packet over and over again 
until it is replaced by a new. The input goes into a FrontDropQueue, and 
every time a packet is pulled for output, a copy is pushed back into the 
queue. When a new packet arrives, it replaces the current.

However, the problem I would like to describe is not related to the 
FrontDropQueue, so below is an example with an ordinary Queue. One packet 
is pushed into the Queue, and I would expect that a continuous infinite 
output  would be generated with copies of that packet.

// makes just one packet
InfiniteSource(LIMIT 1) ->
q::Queue(CAPACITY 1) ->
t::PullTee() ->
// The sink
Discard();
// the loop back
p[1] -> q

Instead it crashes, eating up all memory.

A slight modification in order to rule out PullTee is for example:

InfiniteSource(LIMIT 1) ->
q::Queue(CAPACITY 1) ->
Unqueue() ->
t::Tee();

t[1] -> Discard();
t[0] -> q;

Also crashes. But if it is modified like this:

t[0] -> Discard();
t[1] -> q;

Then it works.

Looking at the code of Tee(), it says

int n = noutputs()
for (int i = 0; i < n - 1; i++)
  if (Packet *q = p->clone())
    output(i).push(q)
  output(n-1).push(p)

This means that the last output is the packet itself, any other outputs 
get clones.

The "working" case is when the packet itself (t[1]) is put back on the 
queue, but the failing cases are when a clone is put back instead. This 
happens with the PullTee() example and with the first Tee() example.

So, we can make any number of clones of a packet and discard them.

But if we make clones and then clones of the clones, it goes wrong.

Now, I need this in a Pull context, as provided by PullTee(). However, 
PullTee() emits a clones on all of its outputs. I suggest a new Push 
element that makes a non-clone. For example

t[1]-> Uniqueify() -> q;

Any opinions?

Mit freundlichen Grüßen
Lars Bro

Siemens A/S
RC-DK MO DEV R&D-A-COC
Borupvang 9
2750 Ballerup, Dänemark
Tel.: +45 4477-4171
mailto:lars.bro@siemens.com

Siemens A/S. Headquarters: Borupvang 9, 2750 Ballerup, Denmark. Tel: +45 
4477 4477 CVR-no. 16 99 30 85

Re: [click] Clones of clones

From:
Bro, Lars
Date:
2014-11-25 @ 14:55
Hi,

I have been looking into the code, and as I can see, it goes like this:

Assume the original packet o, having
o.use_cnt=1
o.data_packet = NULL.

Then we clone it, getting the clone c=o->clone(), having
c.use_cnt=1
c.data_packet=&o

and
o.use_cnt=2

If we do o->kill(), we get
o.use_cnt=1

not freeing o

However, If instead we did c->kill()

Then, we get
c.use_cnt=0, thereby freeing c, calling c.data_packet->kill()
which decrements the counter in o
o.use_cnt=1

Which is back to the beginning.

So, it goes wrong if we do
Packet *p = Packet::make(); // allocate a packet
Packet *p1 = p->clone(); // make a clone
p->kill(); // Keep the packet because there is a clone
Packet *p2 = p1->clone(); // Make another clone
P1-> kill(); // keep the clone because there is a clone to it
…



From: click@librelist.com [mailto:click@librelist.com] On Behalf Of Braem Bart
Sent: 25. november 2014 11:20
To: click@librelist.com
Subject: Re: [click] Clones of clones

Hi Lars and list,

Are you sure this is not just a bug in the cloning, triggered by the 
element flow timing?
It seems Uniqueify would just be hack to work around this issue?

best regards,
Bart

On 24 Nov 2014, at 14:32, Bro, Lars 
<lars.bro@siemens.com<mailto:lars.bro@siemens.com>> wrote:


Hi, List

I have a case where I need to repeat the same packet over and over again 
until it is replaced by a new. The input goes into a FrontDropQueue, and 
every time a packet is pulled for output, a copy is pushed back into the 
queue. When a new packet arrives, it replaces the current.

However, the problem I would like to describe is not related to the 
FrontDropQueue, so below is an example with an ordinary Queue. One packet 
is pushed into the Queue, and I would expect that a continuous infinite 
output  would be generated with copies of that packet.

// makes just one packet
InfiniteSource(LIMIT 1) ->
q::Queue(CAPACITY 1) ->
t::PullTee() ->
// The sink
Discard();
// the loop back
p[1] -> q

Instead it crashes, eating up all memory.

A slight modification in order to rule out PullTee is for example:

InfiniteSource(LIMIT 1) ->
q::Queue(CAPACITY 1) ->
Unqueue() ->
t::Tee();

t[1] -> Discard();
t[0] -> q;

Also crashes. But if it is modified like this:

t[0] -> Discard();
t[1] -> q;

Then it works.

Looking at the code of Tee(), it says

int n = noutputs()
for (int i = 0; i < n - 1; i++)
  if (Packet *q = p->clone())
    output(i).push(q)
  output(n-1).push(p)

This means that the last output is the packet itself, any other outputs 
get clones.

The "working" case is when the packet itself (t[1]) is put back on the 
queue, but the failing cases are when a clone is put back instead. This 
happens with the PullTee() example and with the first Tee() example.

So, we can make any number of clones of a packet and discard them.

But if we make clones and then clones of the clones, it goes wrong.

Now, I need this in a Pull context, as provided by PullTee(). However, 
PullTee() emits a clones on all of its outputs. I suggest a new Push 
element that makes a non-clone. For example

t[1]-> Uniqueify() -> q;

Any opinions?

Mit freundlichen Grüßen
Lars Bro

Siemens A/S
RC-DK MO DEV R&D-A-COC
Borupvang 9
2750 Ballerup, Dänemark
Tel.: +45 4477-4171
mailto:lars.bro@siemens.com

Siemens A/S. Headquarters: Borupvang 9, 2750 Ballerup, Denmark. Tel: +45 
4477 4477 CVR-no. 16 99 30 85

Re: [click] Clones of clones

From:
Tom Barbette
Date:
2014-11-25 @ 15:32
Hi,

Cloning the data_packet if there is a data_packet (= the packet you are
cloning is already a clone) and not the packet itself would allow you to
really kill and destroy the p1, p2, .. P i-1.

That being said, it is not done in current click and not allowed out of the
box as data_packet is private (though it would be easy to change...). That
being said I see no reason why we should keep the current way? In DPDK for
example they ensure that there is never multiple level of indirection by
always cloning the parent. I can propose a patch if it interest the list.

Cheers,

Tom

Hi,



I have been looking into the code, and as I can see, it goes like this:



Assume the original packet o, having

o.use_cnt=1

o.data_packet = NULL.



Then we clone it, getting the clone c=o->clone(), having

c.use_cnt=1

c.data_packet=&o



and

o.use_cnt=2



If we do o->kill(), we get

o.use_cnt=1



not freeing o



However, If instead we did c->kill()



Then, we get

c.use_cnt=0, thereby freeing c, calling c.data_packet->kill()

which decrements the counter in o

o.use_cnt=1



Which is back to the beginning.



So, it goes wrong if we do

Packet *p = Packet::make(); // allocate a packet

Packet *p1 = p->clone(); // make a clone

p->kill(); // Keep the packet because there is a clone

Packet *p2 = p1->clone(); // Make another clone

P1-> kill(); // keep the clone because there is a clone to it

…







*From:* click@librelist.com [mailto:click@librelist.com] *On Behalf Of *Braem
Bart
*Sent:* 25. november 2014 11:20
*To:* click@librelist.com
*Subject:* Re: [click] Clones of clones



Hi Lars and list,



Are you sure this is not just a bug in the cloning, triggered by the
element flow timing?

It seems Uniqueify would just be hack to work around this issue?



best regards,

Bart



On 24 Nov 2014, at 14:32, Bro, Lars <lars.bro@siemens.com> wrote:



  Hi, List



I have a case where I need to repeat the same packet over and over again
until it is replaced by a new. The input goes into a FrontDropQueue, and
every time a packet is pulled for output, a copy is pushed back into the
queue. When a new packet arrives, it replaces the current.



However, the problem I would like to describe is not related to the
FrontDropQueue, so below is an example with an ordinary Queue. One packet
is pushed into the Queue, and I would expect that a continuous infinite
output  would be generated with copies of that packet.



// makes just one packet

InfiniteSource(LIMIT 1) ->

q::Queue(CAPACITY 1) ->

t::PullTee() ->

// The sink

Discard();

// the loop back

p[1] -> q



Instead it crashes, eating up all memory.



A slight modification in order to rule out PullTee is for example:



InfiniteSource(LIMIT 1) ->

q::Queue(CAPACITY 1) ->

Unqueue() ->

t::Tee();



t[1] -> Discard();

t[0] -> q;



Also crashes. But if it is modified like this:



t[0] -> Discard();

t[1] -> q;



Then it works.



Looking at the code of Tee(), it says



int n = noutputs()

for (int i = 0; i < n - 1; i++)

  if (Packet *q = p->clone())

    output(i).push(q)

  output(n-1).push(p)



This means that the last output is the packet itself, any other outputs get
clones.



The "working" case is when the packet itself (t[1]) is put back on the
queue, but the failing cases are when a clone is put back instead. This
happens with the PullTee() example and with the first Tee() example.



So, we can make any number of clones of a packet and discard them.



But if we make clones and then clones of the clones, it goes wrong.



Now, I need this in a Pull context, as provided by PullTee(). However,
PullTee() emits a clones on all of its outputs. I suggest a new Push
element that makes a non-clone. For example



t[1]-> Uniqueify() -> q;



Any opinions?



Mit freundlichen Grüßen

Lars Bro



Siemens A/S

RC-DK MO DEV R&D-A-COC

Borupvang 9

2750 Ballerup, Dänemark

Tel.: +45 4477-4171

mailto:lars.bro@siemens.com <lars.bro@siemens.com>



Siemens A/S. Headquarters: Borupvang 9, 2750 Ballerup, Denmark. Tel: +45
4477 4477 CVR-no. 16 99 30 85

Re: [Click] Clones of clones

From:
Bro, Lars
Date:
2014-11-26 @ 12:11
Hi,

I think it should be fixed, since we must be able to believe that all 
outputs of eg.: Tee() can be treated the same way. If the problem occurs, 
it will be perceived as Click being unstable, since it just crashes (oom 
kill) without warning. On top of this, if one really wants to feed packets
back into a retransmission loop, then PullTee() is more or less the only 
way to do so.

The solution you propose, such as cloning the parent only (via 
data_packet*) seems to be clean and without any side effects, so I would 
say go for it.

Yours,
Lars Bro


Mit freundlichen Grüßen
Lars Bro

Siemens A/S
RC-DK MO DEV R&D-A-COC
Borupvang 9
2750 Ballerup, Dänemark
Tel.: +45 4477-4171
mailto:lars.bro@siemens.com

Siemens A/S. Headquarters: Borupvang 9, 2750 Ballerup, Denmark. Tel: +45 
4477 4477 CVR-no. 16 99 30 85