> -----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