[ofa-general] Re: [PATCH] opensm & osm_console: modified console framework to support multiple connections

[Sasha Khapyorsky]

On 15:06 Thu 01 Nov     , Timothy A. Meier wrote:
>  Sasha Khapyorsky wrote:
> > Hi Tim,
> >
> > On 10:53 Thu 01 Nov     , Timothy A. Meier wrote:
> >   
> >>> I think we need to close threading issue first. Then patch series of 2
> >>> looks fine for me.
> >>>
> >>>         
> >>  I really think the "thread-per-session" would be a more flexible and  
> >> powerful
> >>  design. Setting up and maintaining threads might seem more complex at 
> >> first,
> >>  but it makes servicing requests/commands much more simple because 
> >> everything  is
> >>  in its own context.
> >>     
> >
> > And require proper locking, thread termination handling, etc.. Which is
> > not always easy even with full featured pthread library, and especially
> > hard with limited cl_thread*() primitives... I didn't analyze submitted
> > code in this aspect - just tried to save the time... :)
> >
> >   
>  Understood. I will be careful, I promise. ;^)
> 
>  [Fortunately I have other source code to examine.]
> >>  The previous Console used a polling mechanism, and I found an edge case  
> >> condition
> >>  which allowed one connection to block the other.
> >>     
> >
> > How? There is no "blocking" commands? Right?
> >
> >   
>  You are right, no blocking "commands" (this is a requirement).
> 
>  As I said, it is sort of an edge case, but it illustrates the vulnerability
>  of connections in a single threaded model interfering with each other
>  (not intentional of course).
> 
>  osm_console.c: handle_osm_connection() method
> 
>  when a second connection is attempted (and successfully made within the
>  same thread) a blocking io call is used (getline()) to query the user
>  about killing the other connection. If this goes unanswered, the original
>  connection is blocked.

Seems like usage bug. This should not be used without select() or
poll().

>  Fundamentally, using a thread-per-session design formalizes the need
>  to keep session specific data/resources separate. In a single threaded
>  design, it would be more difficult to implement and enforce this policy.

It is just matter of design style - at least personally I don't need
such enforcements.

> >>  Thread-per-session (or  thread
> >>  per connection) makes it difficult for one session to influence another.
> >>
> >>  The number of threads/connections would be limited. Other than the normal
> >>  multi-threading issues, are there other thread hazards in OFED/OpenSM 
> >> that I
> >>  need to be aware of?
> >>     
> >
> > Another reason is to not get too much cpus from another OpenSM threads
> > (which mainly are responsible for IB MADs processing).
> >
> >   
>  I agree. The maximum # of connections is a #define and currently set to 5
>  (4 useful, and 1 just to provide feedback that no more connections are 
>  available).
> 
>  There will be a couple of connection management commands (something like 
>  "usage"
>  and "kill", etc..) to make sure sessions don't get out of control. In 
>  general,
>  I don't expect much of a CPU load, or any additional burden on OpenSM.
> 
>  In any case, a single-threaded, multi-connection design would also need to 
>  be
>  sensitive to the CPU load it places on the system.

Single thread cannot load more than 1 CPU on typically multiprocessors
machine.

> > Sasha
> >
> >   
>  Finally, a multi-threaded design would remove the non-blocking io
>  restriction/requirement. Individual threads would be free to block
>  (if they choose) and would only block themselves. This would allow
>  for some more rich (multi-input) commands. Specifically, I need
>  something like this to accept ACC/PW info when authenticating/authorizing
>  over SSL/TSL.
> 
>  Other concerns? Convinced yet?

I am not really convinced and feel you are also not. I think it is better
to start from something now - it will be possible to rework things later.

Sasha