> -----Original Message-----
> From: Parthasarathy Bhuvaragan
> Sent: Thursday, 15 September, 2016 08:55
> To: Jon Maloy <ma...@donjonn.com>; Jon Maloy <jon.ma...@ericsson.com>;
> tipc-discussion@lists.sourceforge.net; Ying Xue <ying....@windriver.com>
> Subject: Re: [PATCH net-next v3 00/12] tipc: create socket FSM using sk_state
> only
> 
> On 09/15/2016 02:39 PM, Jon Maloy wrote:
> >
> >
> > On 09/15/2016 08:29 AM, Parthasarathy Bhuvaragan wrote:
> >>
> >> On 09/14/2016 10:20 PM, Jon Maloy wrote:
> >>> Hi Partha,
> >>> It looks good. I am especially happy that you got rid of the "connected"
> >>> field, which I have been requesting for several years.
> >>> I only have a couple of comments:
> >>>
> >>> 1: It disturbs me that TIPC_UNCONNECTED and TIPC_LISTENING sockets
> goes to
> >>> state TIPC_DISCONNECTING before they are deleted, without ever having
> been
> >>> connected. This is counterintuitive and cannot be necessary.
> >> Changing the names of the new states should make the transitions more
> logical
> >> as below:
> >> TIPC_UNCONNECTED => TIPC_OPEN
> >> TIPC_DISCONNECTING => TIPC_CLOSE
> >> So in this case, all open sockets needs to be closed. Its valid for both
> >> connectionless and connection oriented sockets as for connection oriented
> >> sockets there is no difference between DISCONNECTING and CLOSE.
> >
> > Sounds better, although I think TIPC_CLOSING, or maybe TIPC_RELEASING
> would be
> > even better.
> I realize that its better to keep the state TIPC_DISCONNECTING and create a 
> new
> state called TIPC_CLOSE.
> TIPC_DISCONNECTING is set at tipc_shutdown() or when connection probe
> messages
> timeout or when we receive an error message on the connection.
> TIPC_CLOSE is set in all other cases.

Ok. But calling it TIPC_CLOSE is still linguistically wrong. CLOSE is a verb, 
while CLOSING (or CLOSED) in this context is an adjective, which is what you 
use if you want to describe the state of something.

///jon

> >
> >>
> >> I need that we get names for the states correct, as I will export them to
> >> user space for socket diagnostics.
> >>
> >> BTW, why do we even support shutdown() on connectionless sockets? The
> >> function handles states relevant only for connection oriented sockets.
> >>> 2: "connection less" should be "connectionless".  In several places.
> >> I will fix this. I need to fix the description in tipc_poll() too.
> >
> > When you have done this and the above name changes, you can add an Acked-
> by:
> > from me.
> >
> > ///jon
> >
> >>> 3: We really must rename/retype the confusing "handle" field used in
> >>> TIPC_SKB_CB()->handle. I believe it should be made a u32 and called "read"
> >>> or "offset" or similar.  Maybe a separate patch before or after this
> >>> series?
> >> I will look into that, and post it separately.
> >>> Regards
> >>> ///jon
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Parthasarathy Bhuvaragan
> >>>> Sent: Tuesday, 13 September, 2016 06:52
> >>>> To:tipc-discussion@lists.sourceforge.net; Jon
> Maloy<jon.ma...@ericsson.com>;
> >>>> ma...@donjonn.com; Ying Xue<ying....@windriver.com>
> >>>> Subject: [PATCH net-next v3 00/12] tipc: create socket FSM using sk_state
> only
> >>>>
> >>>> The following issues with the current socket layer hinders socket
> diagnostics
> >>>> implementation, which led to this patch series. The series does not add 
> >>>> any
> >>>> functional change.
> >>>>
> >>>> 1. tipc socket state is derived from multiple variables like
> >>>>     sock->state, tsk->probing_state and tsk->connected. This style forces
> >>>>     us to export multiple attributes to the user space, which has to be
> >>>>     backward compatible.
> >>>>
> >>>> 2. Abuse of sock->state cannot be exported to user-space without
> >>>>     requiring tipc specific hacks in the user-space.
> >>>>     - For connection less (CL) sockets sock->state is overloaded to
> >>>>       tipc state SS_READY.
> >>>>     - For connection oriented (CO) listening socket sock->state is
> >>>>       overloaded to tipc state SS_LISTEN.
> >>>>
> >>>> This series is split into three:
> >>>> 1. A bug fix in patch-1
> >>>> 2. Express all tipc states using a single variable. This is done in 
> >>>> patch#2-5.
> >>>> 3. Migrate the new tipc states to sk->sk_state. This is done in 
> >>>> patch#6-12.
> >>>>
> >>>> The figures below represents the FSM after this series:
> >>>>
> >>>> Unconnected Sockets:
> >>>> +------------------+        +--------------------+
> >>>> | TIPC_UNCONNECTED |------->| TIPC_DISCONNECTING |
> >>>> +------------------+        +--------------------+
> >>>>
> >>>> Stream Server Listening Socket:
> >>>> +------------------+        +-------------+
> >>>> | TIPC_UNCONNECTED |------->| TIPC_LISTEN |
> >>>> +------------------+        +-------------+
> >>>>                                    |
> >>>> +--------------------+            |
> >>>> | TIPC_DISCONNECTING |<-----------+
> >>>> +--------------------+
> >>>>
> >>>> Stream Server Data Socket:
> >>>> +-----------------+        +------------------+
> >>>> |TIPC_UNCONNECTED |------> | TIPC_ESTABLISHED |<---+
> >>>> +-----------------+        +------------------+    |
> >>>>                                 ^   |    |          |
> >>>>                                 |   |    +----------+
> >>>>                                 |   v
> >>>> +------------------+      +-------------+
> >>>> |TIPC_DISCONNECTING|<-----|TIPC_PROBING |
> >>>> +------------------+      +-------------+
> >>>>
> >>>> Stream Socket Client:
> >>>> +-----------------+       +-----------------+
> >>>> |TIPC_UNCONNECTED |------>| TIPC_CONNECTING |
> >>>> +-----------------+       +-----------------+
> >>>>                                    |
> >>>>                                    |
> >>>>                                    v
> >>>>                            +------------------+
> >>>>                            | TIPC_ESTABLISHED |<---+
> >>>>                            +------------------+    |
> >>>>                                 ^   |    |         |
> >>>>                                 |   |    +---------+
> >>>>                                 |   v
> >>>> +------------------+      +-------------+
> >>>> |TIPC_DISCONNECTING|<-----|TIPC_PROBING |
> >>>> +------------------+      +-------------+
> >>>>
> >>>> NOTE:
> >>>> This is just a base refractoring required for socket diagnostics.
> >>>> Implementation of TIPC socket diagnostics will be sent as a
> >>>> separate series.
> >>>>
> >>>> ---
> >>>> I plan to submit this series after v4.8 release cycle.
> >>>>
> >>>> v3: - Address comments from Ying Xue<ying....@windriver.com>  in
> >>>>        patch #7, #11.
> >>>>      - Rebase on latest netnext which contains fixes for broadcast NACK
> >>>>        that seems to make ptts regression stable.
> >>>>      - Ran ptts suits for 6000 iterations for 5+ hours.
> >>>>
> >>>> v2: - Address comments from Ying Xue<ying....@windriver.com>  in
> >>>>        patch #4, #5, #12.
> >>>>      - Added a note that the socket diagnostics will be sent as
> >>>>        another series.
> >>>> v1: - I base the following patch series as the first version:
> >>>>        [RFC PATCH v1 00/12] tipc: create socket FSM using sk_state only
> >>>>
> >>>> Parthasarathy Bhuvaragan (12):
> >>>>    tipc: set kern=0 in sk_alloc() during tipc_accept()
> >>>>    tipc: rename tsk->remote to tsk->peer for consistent naming
> >>>>    tipc: remove tsk->connected for connection less sockets
> >>>>    tipc: remove tsk->connected from tipc_sock
> >>>>    tipc: remove probing_intv from tipc_sock
> >>>>    tipc: remove socket state SS_READY
> >>>>    tipc: create TIPC_LISTEN as a new sk_state
> >>>>    tipc: create TIPC_PROBING/TIPC_ESTABLISHED as new sk_states
> >>>>    tipc: create TIPC_UNCONNECTED as a new sk_state
> >>>>    tipc: create TIPC_DISCONNECTING as a new sk_state
> >>>>    tipc: create TIPC_CONNECTING as a new sk_state
> >>>>    tipc: remove SS_CONNECTED sock state
> >>>>
> >>>>   include/uapi/linux/tipc.h |  12 ++
> >>>>   net/tipc/socket.c         | 339
> >>>> ++++++++++++++++++++++++++--------------------
> >>>>   2 files changed, 203 insertions(+), 148 deletions(-)
> >>>>
> >>>> --
> >>>> 2.1.4
> >>
> >
> >


------------------------------------------------------------------------------
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Reply via email to