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