Re: [Pandora] Component::notify()

[Simon Patarin]

> Hi (all?)

You feel a little bit lonely, don't you? I have warned you! :)

> Please imagine the following stack:
> 
> @tcpscan [ $reject = false ];
> @demux [ $algo = "tcpcnx"] < @mycomponent >
> 
> This will pass each TCP flow (both ways) to am instance of a "mycomponent"
> component

Yes.
 
> What happens if @mycomponent just wants few packets?

He's in trouble (but may I ask why do you need it in the first place?)
 
> mycomponent when it's finished will return "true" from mycomponent::add()
> method so tcpcnx will destroy the branch with demux::notify(), that's fine
> 
> but next packets of the same TCP flow will come again and then another
> branch/instance of a "mycomponent" will be created, but remember:
> 
> we are no longer interested in this TCP flow!

I agree that there is some sort of a problem here. The only case where I
have faced the same kind of problems is when you receive 3 or more TCP
resets in a row. You understand that the connection is finished as soon as
you receive the first one, so I used to clean the branch, but each
following reset triggers the creation (and the immediate destruction) of a
new branch. That was annoying, so instead of cleaning the branch
immediately I just reset the timeout value for the branch to a very low
value (2 seconds), you can see this in the TCPPortDemux component.
 
> my propossal:
> 
> add to the Packet class a new virtual method (name it ::accept()) so 
> DemuxComponent::add() in this case would be something like:
[...]
> 
>       if (!pkt->accept()) {        <<< Here
>       cleanPacket(pkt);          <<< Here
>       return false;              <<< Here
>       }                            <<< Here
[...]

I have a problem with your proposal: how do you implement the accept
method for e.g. the TCPPacket class? how can you make it generic? how does
the packet know that it should not be accepted? I can't figure this out.
 
> I'm supossing there's no other method for doing this already

Unfortunately, you're right, except by maintining artificially in life
your component (i.e. never returning true, but instead silently discarding
superfluous packets) and relying and pandora's timeout to collect your
component after some time of inactivity. You know the generic
"timeout" option don't you? any component accepts it: 
@mycomponent [$timeout = 30] means that if no packets are forwarded to
this component during 30 seconds it will be automatically cleaned up.
The timeout is reset each time a packet is received, if you want it to be
absolute, you just need to specify $resched = false, in this case the
component "lives" 30 seconds at max. 

> Why do I think this should be this way? 
> 
> _because kmap is in the demux component_

Yes, there are no other places to do it. However, I'd be more inclined to
make another component, let's say "demuxonce" that is a clone of demux
except for a few things: in notify, instead of removing completely the
branch, I'll replace:

  if (b < 0) kmap.removeVal(b);

by 

  if (b < 0) kmap.atPut(kmap.keyAt(b), -b);

-b is not a valid branch value (it must be strictly negative and 0 is the
default value -- when no branch exists), so you can detect it in the
add() method just after:

if (b == 0)
{
  ...
}

you can add:

else if (b > 0) {
  cleanPacket(pkt);
  return false;
}

The final issue is to remove the assertion in the destructor because the
size will not be 0 anymore, however you do not have to worry about
deleting data in the map: it is automatically allocated.

If you do not want to make a new component, I'm sure that this could be
controlled with an option, well, actually this would be much better with
an option! The last problem is to know how much time you want the branch
to be "fobidden". Here it is forever, if you want to make it a paramater
it becomes a little bit trickier: you have to remove the marked branch in
the map (with kmap.removeVal) after some specified amount of time. You can
look at the sched_manager class (in libpandora) and the testcomponents.cc
file (in pandora_components) to have an idea on how this could be done.

HTH,

Simon