[Click] Race condition behavior with single thread Click Linux module

[Eddie Kohler]

Hi Thomas,

You have exactly hit on the Click situation: a handler can execute in parallel 
with packet forwarding.  This is because handlers are called from the context 
of *the process that wrote the handler*, not the Click thread.

You may need to implement your own thread safety here.  Alternatively, you can 
try some experimental code in the current anonymous CVS.  If you set a handler 
flag to contain Handler::EXCLUSIVE, Click will ensure that that handler is 
called exclusively of any tasks.  This is experimental; let us know if it 
works for you.

Eddie



Paine, Thomas Asa wrote:
> 	I was under the assumption that click running in kernel time, by
> default, ran under a single thread.  We have run into an issue when we
> have a read/write handler on an element clearing a hashmap that is a
> member of that element.  If the map is cleared through a function called
> from the static handler (hashmap.clear()), we kernel panic (the map is
> updated with every packet passing through the element and at high packet
> rates).  
> 	If the handler calls a helper function that uses a timer within
> the element (timer.schedule_now()), this does not happen.  This suggests
> to me there is a race condition on the element's state and that there is
> actually more than one thread running.  Am I correct here, and if so
> should I assume locks are needed for thread safety with regards to
> proc's view into an element and its static handlers?
> 
> 
> In write_handler() below I have commented what causes the panic and what
> does not.
> 
> 
> 
> int FilterTables::initialize(ErrorHandler *errh){
>   timer.initialize(this);
>   timer.schedule_now();
> }
> 
> Packet* FilterTables::simple_action(Packet *p){ .
> .
>   _byteMap->insert(ip, _byteMap->find(ip)+len); .
> .
> }
> 
> void FilterTables::add_handlers(){
> .
> .
>   // Write Handlers
>   add_write_handler("reset", write_handler, (void *)H_RESET); .
> .
> }
> 
> 
> int FilterTables::write_handler(const String &in_str, Element *e, void
> *thunk, ErrorHandler *errh){
>   FilterTables *c = (FilterTables *)e;
> .
> .
>   switch ((intptr_t)thunk) {
>    case H_RESET:
>     // this will cause a panic, but not always the first time called
> (which again acts like a race condition)
>     c->reset();
>     // but this will not cause a panic and clears the map
>     timer.schedule_now();	
>     return 0;
> .
> .
> }
> 
> void FilterTables::reset(){
>   _byteMap->clear();
> }
> 
> 
> void FilterTables::run_timer(){
>   reset();
> }
> 
> 
> 
> Thanks,
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  
>    Thomas Paine (paineta at uwec.edu) 
>    University of Wisconsin - Eau Claire 
>    garbage foo(garbage g){return(g);}
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 
> 
> _______________________________________________
> click mailing list
> click at amsterdam.lcs.mit.edu
> https://amsterdam.lcs.mit.edu/mailman/listinfo/click