From 29042e19f2c602fabe4705b5b719550b4627639c Mon Sep 17 00:00:00 2001 From: Jon Paul Maloy Date: Wed, 22 Jul 2015 10:11:18 -0400 Subject: tipc: let function tipc_msg_reverse() expand header when needed The shortest TIPC message header, for cluster local CONNECTED messages, is 24 bytes long. With this format, the fields "dest_node" and "orig_node" are optimized away, since they in reality are redundant in this particular case. However, the absence of these fields leads to code inconsistencies that are difficult to handle in some cases, especially when we need to reverse or reject messages at the socket layer. In this commit, we concentrate the handling of the absent fields to one place, by letting the function tipc_msg_reverse() reallocate the buffer and expand the header to 32 bytes when necessary. This means that the socket code now can assume that the two previously absent fields are present in the header when a message needs to be rejected. This opens up for some further simplifications of the socket code. Reviewed-by: Ying Xue Signed-off-by: Jon Maloy Signed-off-by: David S. Miller --- net/tipc/msg.c | 67 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 26 deletions(-) (limited to 'net/tipc/msg.c') diff --git a/net/tipc/msg.c b/net/tipc/msg.c index 08b4cc7d496d..4339aab93034 100644 --- a/net/tipc/msg.c +++ b/net/tipc/msg.c @@ -463,43 +463,58 @@ bool tipc_msg_make_bundle(struct sk_buff **skb, struct tipc_msg *msg, /** * tipc_msg_reverse(): swap source and destination addresses and add error code - * @buf: buffer containing message to be reversed - * @dnode: return value: node where to send message after reversal - * @err: error code to be set in message - * Consumes buffer if failure + * @own_node: originating node id for reversed message + * @skb: buffer containing message to be reversed; may be replaced. + * @err: error code to be set in message, if any + * Consumes buffer at failure * Returns true if success, otherwise false */ -bool tipc_msg_reverse(u32 own_addr, struct sk_buff *buf, u32 *dnode, - int err) +bool tipc_msg_reverse(u32 own_node, struct sk_buff **skb, u32 *dnode, int err) { - struct tipc_msg *msg = buf_msg(buf); + struct sk_buff *_skb = *skb; + struct tipc_msg *hdr = buf_msg(_skb); struct tipc_msg ohdr; - uint rdsz = min_t(uint, msg_data_sz(msg), MAX_FORWARD_SIZE); + int dlen = min_t(uint, msg_data_sz(hdr), MAX_FORWARD_SIZE); - if (skb_linearize(buf)) + if (skb_linearize(_skb)) goto exit; - msg = buf_msg(buf); - if (msg_dest_droppable(msg)) + hdr = buf_msg(_skb); + if (msg_dest_droppable(hdr)) goto exit; - if (msg_errcode(msg)) + if (msg_errcode(hdr)) goto exit; - memcpy(&ohdr, msg, msg_hdr_sz(msg)); - msg_set_errcode(msg, err); - msg_set_origport(msg, msg_destport(&ohdr)); - msg_set_destport(msg, msg_origport(&ohdr)); - msg_set_prevnode(msg, own_addr); - if (!msg_short(msg)) { - msg_set_orignode(msg, msg_destnode(&ohdr)); - msg_set_destnode(msg, msg_orignode(&ohdr)); + + /* Take a copy of original header before altering message */ + memcpy(&ohdr, hdr, msg_hdr_sz(hdr)); + + /* Never return SHORT header; expand by replacing buffer if necessary */ + if (msg_short(hdr)) { + *skb = tipc_buf_acquire(BASIC_H_SIZE + dlen); + if (!*skb) + goto exit; + memcpy((*skb)->data + BASIC_H_SIZE, msg_data(hdr), dlen); + kfree_skb(_skb); + _skb = *skb; + hdr = buf_msg(_skb); + memcpy(hdr, &ohdr, BASIC_H_SIZE); + msg_set_hdr_sz(hdr, BASIC_H_SIZE); } - msg_set_size(msg, msg_hdr_sz(msg) + rdsz); - skb_trim(buf, msg_size(msg)); - skb_orphan(buf); - *dnode = msg_orignode(&ohdr); + + /* Now reverse the concerned fields */ + msg_set_errcode(hdr, err); + msg_set_origport(hdr, msg_destport(&ohdr)); + msg_set_destport(hdr, msg_origport(&ohdr)); + msg_set_destnode(hdr, msg_prevnode(&ohdr)); + msg_set_prevnode(hdr, own_node); + msg_set_orignode(hdr, own_node); + msg_set_size(hdr, msg_hdr_sz(hdr) + dlen); + *dnode = msg_destnode(hdr); + skb_trim(_skb, msg_size(hdr)); + skb_orphan(_skb); return true; exit: - kfree_skb(buf); - *dnode = 0; + kfree_skb(_skb); + *skb = NULL; return false; } -- cgit v1.2.3 From bcd3ffd4f6d7c994c93be2ab8598fdfb2952a1f1 Mon Sep 17 00:00:00 2001 From: Jon Paul Maloy Date: Wed, 22 Jul 2015 10:11:19 -0400 Subject: tipc: introduce new tipc_sk_respond() function Currently, we use the code sequence if (msg_reverse()) tipc_link_xmit_skb() at numerous locations in socket.c. The preparation of arguments for these calls, as well as the sequence itself, makes the code unecessarily complex. In this commit, we introduce a new function, tipc_sk_respond(), that performs this call combination. We also replace some, but not yet all, of these explicit call sequences with calls to the new function. Notably, we let the function tipc_sk_proto_rcv() use the new function to directly send out PROBE_REPLY messages, instead of deferring this to the calling tipc_sk_rcv() function, as we do now. Reviewed-by: Ying Xue Signed-off-by: Jon Maloy Signed-off-by: David S. Miller --- net/tipc/msg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'net/tipc/msg.c') diff --git a/net/tipc/msg.c b/net/tipc/msg.c index 4339aab93034..b6cc58ec7346 100644 --- a/net/tipc/msg.c +++ b/net/tipc/msg.c @@ -469,7 +469,7 @@ bool tipc_msg_make_bundle(struct sk_buff **skb, struct tipc_msg *msg, * Consumes buffer at failure * Returns true if success, otherwise false */ -bool tipc_msg_reverse(u32 own_node, struct sk_buff **skb, u32 *dnode, int err) +bool tipc_msg_reverse(u32 own_node, struct sk_buff **skb, int err) { struct sk_buff *_skb = *skb; struct tipc_msg *hdr = buf_msg(_skb); @@ -508,7 +508,6 @@ bool tipc_msg_reverse(u32 own_node, struct sk_buff **skb, u32 *dnode, int err) msg_set_prevnode(hdr, own_node); msg_set_orignode(hdr, own_node); msg_set_size(hdr, msg_hdr_sz(hdr) + dlen); - *dnode = msg_destnode(hdr); skb_trim(_skb, msg_size(hdr)); skb_orphan(_skb); return true; -- cgit v1.2.3 From cda3696d3d26eb798c94de0dab5bd66ddb5627cb Mon Sep 17 00:00:00 2001 From: Jon Paul Maloy Date: Wed, 22 Jul 2015 10:11:20 -0400 Subject: tipc: clean up socket layer message reception When a message is received in a socket, one of the call chains tipc_sk_rcv()->tipc_sk_enqueue()->filter_rcv()(->tipc_sk_proto_rcv()) or tipc_sk_backlog_rcv()->filter_rcv()(->tipc_sk_proto_rcv()) are followed. At each of these levels we may encounter situations where the message may need to be rejected, or a new message produced for transfer back to the sender. Despite recent improvements, the current code for doing this is perceived as awkward and hard to follow. Leveraging the two previous commits in this series, we now introduce a more uniform handling of such situations. We let each of the functions in the chain itself produce/reverse the message to be returned to the sender, but also perform the actual forwarding. This simplifies the necessary logics within each function. Reviewed-by: Ying Xue Signed-off-by: Jon Maloy Signed-off-by: David S. Miller --- net/tipc/msg.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'net/tipc/msg.c') diff --git a/net/tipc/msg.c b/net/tipc/msg.c index b6cc58ec7346..562c926a51cc 100644 --- a/net/tipc/msg.c +++ b/net/tipc/msg.c @@ -520,17 +520,15 @@ exit: /** * tipc_msg_lookup_dest(): try to find new destination for named message * @skb: the buffer containing the message. - * @dnode: return value: next-hop node, if destination found - * @err: return value: error code to use, if message to be rejected + * @err: error code to be used by caller if lookup fails * Does not consume buffer * Returns true if a destination is found, false otherwise */ -bool tipc_msg_lookup_dest(struct net *net, struct sk_buff *skb, - u32 *dnode, int *err) +bool tipc_msg_lookup_dest(struct net *net, struct sk_buff *skb, int *err) { struct tipc_msg *msg = buf_msg(skb); - u32 dport; - u32 own_addr = tipc_own_addr(net); + u32 dport, dnode; + u32 onode = tipc_own_addr(net); if (!msg_isdata(msg)) return false; @@ -543,15 +541,15 @@ bool tipc_msg_lookup_dest(struct net *net, struct sk_buff *skb, return false; if (msg_reroute_cnt(msg)) return false; - *dnode = addr_domain(net, msg_lookup_scope(msg)); + dnode = addr_domain(net, msg_lookup_scope(msg)); dport = tipc_nametbl_translate(net, msg_nametype(msg), - msg_nameinst(msg), dnode); + msg_nameinst(msg), &dnode); if (!dport) return false; msg_incr_reroute_cnt(msg); - if (*dnode != own_addr) - msg_set_prevnode(msg, own_addr); - msg_set_destnode(msg, *dnode); + if (dnode != onode) + msg_set_prevnode(msg, onode); + msg_set_destnode(msg, dnode); msg_set_destport(msg, dport); *err = TIPC_OK; return true; -- cgit v1.2.3