tipc-discussion — User questions & problem reports, project news, developer discussion

Re: [tipc-discussion] [PATCH] tipc: add loopback device tracking

[Jon M. <jon...@er...>]

Hi John,
When looking at this again, I think we can make another small improvement, which will make the code look a little bit nicer:

+ static inline void tipc_loopback_trace(struct net *net, struct sk_buff_head* pkts)
+{
+             if (unlikely(tipc_net(net)->loopback_trace)
+                        tipc_clone_to_loopback(net, pkts);
+}

This will result in the same binary code from the compiler, but we don't need to make the call conditional directly in the data path.

Otherwise this is still ok with me. Just remember to put [net-next] instead of [PATCH] in the patch header, and run scripts/checkpatch on it.

///jon

> -----Original Message-----
> From: John Rutherford <joh...@de...>
> Sent: 13-Jun-19 21:58
> To: tip...@li...
> Subject: [tipc-discussion] [PATCH] tipc: add loopback device tracking
> 
> Since node internal messages are passed directly to socket it is not possible to
> observe
> 
> this message exchange via tcpdump or wireshark.
> 
> 
> 
> We now remedy this by making it possible to clone such messages and send
> the clones to the
> 
> loopback interface.  The clones are dropped at reception and have no
> functional role except
> 
> making the traffic visible.
> 
> 
> 
> The feature is turned on/off by enabling/disabling the loopback "bearer"
> "eth:lo".
> 
> Signed-off-by: John Rutherford <joh...@de...>
> 
> Signed-off-by: Jon Maloy <jon...@er...>
> 
> ---
> 
> net/tipc/bcast.c   |  5 +++-
> 
> net/tipc/bearer.c  | 67
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> net/tipc/bearer.h  |  3 +++
> 
> net/tipc/core.c    |  5 +++-
> 
> net/tipc/core.h    |  9 ++++++++
> 
> net/tipc/netlink.c |  2 +-
> 
> net/tipc/node.c    |  2 ++
> 
> net/tipc/topsrv.c  |  3 +++
> 
> 8 files changed, 93 insertions(+), 3 deletions(-)
> 
> 
> 
> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
> 
> index 6c997d4..db40129 100644
> 
> --- a/net/tipc/bcast.c
> 
> +++ b/net/tipc/bcast.c
> 
> @@ -406,8 +406,11 @@ int tipc_mcast_xmit(struct net *net, struct
> sk_buff_head *pkts,
> 
>                                                rc = tipc_bcast_xmit(net, pkts, cong_link_cnt);
> 
>                }
> 
> -              if (dests->local)
> 
> +             if (dests->local) {
> 
> +                             if (unlikely(tipc_loopback_trace(net)))
> 
> +
> + tipc_clone_to_loopback(net,
> &localq);
> 
>                                tipc_sk_mcast_rcv(net, &localq, &inputq);
> 
> +             }
> 
> exit:
> 
>                /* This queue should normally be empty by now */
> 
>                __skb_queue_purge(pkts);
> 
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> 
> index 2bed658..27b4fd7 100644
> 
> --- a/net/tipc/bearer.c
> 
> +++ b/net/tipc/bearer.c
> 
> @@ -836,6 +836,12 @@ int __tipc_nl_bearer_disable(struct sk_buff *skb,
> struct genl_info *info)
> 
>                 name = nla_data(attrs[TIPC_NLA_BEARER_NAME]);
> 
> +             if (!strcmp(name, "eth:lo")) {
> 
> +                             tipc_net(net)->loopback_trace = false;
> 
> +                             pr_info("Disabled packet tracing on
> + loopback
> interface\n");
> 
> +                             return 0;
> 
> +             }
> 
> +
> 
>                bearer = tipc_bearer_find(net, name);
> 
>                if (!bearer)
> 
>                                return -EINVAL;
> 
> @@ -881,6 +887,12 @@ int __tipc_nl_bearer_enable(struct sk_buff *skb,
> struct genl_info *info)
> 
>                 bearer = nla_data(attrs[TIPC_NLA_BEARER_NAME]);
> 
> +             if (!strcmp(bearer, "eth:lo")) {
> 
> +                             tipc_net(net)->loopback_trace = true;
> 
> +                             pr_info("Enabled packet tracing on
> + loopback
> interface\n");
> 
> +                             return 0;
> 
> +             }
> 
> +
> 
>                if (attrs[TIPC_NLA_BEARER_DOMAIN])
> 
>                                domain =
> nla_get_u32(attrs[TIPC_NLA_BEARER_DOMAIN]);
> 
> @@ -1021,6 +1033,61 @@ int tipc_nl_bearer_set(struct sk_buff *skb, struct
> genl_info *info)
> 
>                return err;
> 
> }
> 
> +void tipc_clone_to_loopback(struct net *net, struct sk_buff_head
> +*xmitq)
> 
> +{
> 
> +             struct net_device *dev = net->loopback_dev;
> 
> +             struct sk_buff *skb, *_skb;
> 
> +             int exp;
> 
> +
> 
> +             skb_queue_walk(xmitq, _skb) {
> 
> +                             skb = pskb_copy(_skb, GFP_ATOMIC);
> 
> +                             if (!skb)
> 
> +                                             continue;
> 
> +                             exp = SKB_DATA_ALIGN(dev->hard_header_len
> + -
> skb_headroom(skb));
> 
> +                             if (exp > 0 && pskb_expand_head(skb, exp,
> + 0,
> GFP_ATOMIC)) {
> 
> +                                             kfree_skb(skb);
> 
> +                                             continue;
> 
> +                             }
> 
> +                             skb_reset_network_header(skb);
> 
> +                             skb->dev = dev;
> 
> +                             skb->protocol = htons(ETH_P_TIPC);
> 
> +                             dev_hard_header(skb, dev, ETH_P_TIPC,
> dev->dev_addr,
> 
> +
> + dev->dev_addr,
> skb->len);
> 
> +                             dev_queue_xmit(skb);
> 
> +             }
> 
> +}
> 
> +
> 
> +static int tipc_loopback_rcv_pkt(struct sk_buff *skb, struct net_device
> *dev,
> 
> +                                                             struct
> packet_type *pt, struct net_device *od)
> 
> +{
> 
> +             consume_skb(skb);
> 
> +             return NET_RX_SUCCESS;
> 
> +}
> 
> +
> 
> +int tipc_attach_loopback(struct net *net)
> 
> +{
> 
> +             struct net_device *dev = net->loopback_dev;
> 
> +             struct tipc_net *tn = tipc_net(net);
> 
> +
> 
> +             if (!dev)
> 
> +                             return -ENODEV;
> 
> +             dev_hold(dev);
> 
> +             tn->loopback_pt.dev = dev;
> 
> +             tn->loopback_pt.type = htons(ETH_P_TIPC);
> 
> +             tn->loopback_pt.func = tipc_loopback_rcv_pkt;
> 
> +             tn->loopback_trace = false;
> 
> +             dev_add_pack(&tn->loopback_pt);
> 
> +             return 0;
> 
> +}
> 
> +
> 
> +void tipc_detach_loopback(struct net *net)
> 
> +{
> 
> +             struct tipc_net *tn = tipc_net(net);
> 
> +
> 
> +             dev_remove_pack(&tn->loopback_pt);
> 
> +             dev_put(net->loopback_dev);
> 
> +}
> 
> +
> 
> static int __tipc_nl_add_media(struct tipc_nl_msg *msg,
> 
>                                                       struct tipc_media *media, int nlflags)
> 
> {
> 
> diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
> 
> index 7f4c569..ef7fad9 100644
> 
> --- a/net/tipc/bearer.h
> 
> +++ b/net/tipc/bearer.h
> 
> @@ -232,6 +232,9 @@ void tipc_bearer_xmit(struct net *net, u32
> bearer_id,
> 
>                                      struct tipc_media_addr *dst);
> 
> void tipc_bearer_bc_xmit(struct net *net, u32 bearer_id,
> 
>                                                 struct sk_buff_head *xmitq);
> 
> +void tipc_clone_to_loopback(struct net *net, struct sk_buff_head
> +*xmitq);
> 
> +int tipc_attach_loopback(struct net *net);
> 
> +void tipc_detach_loopback(struct net *net);
> 
>  /* check if device MTU is too low for tipc headers */
> 
> static inline bool tipc_mtu_bad(struct net_device *dev, unsigned int
> reserve)
> 
> diff --git a/net/tipc/core.c b/net/tipc/core.c
> 
> index ed536c0..1867687 100644
> 
> --- a/net/tipc/core.c
> 
> +++ b/net/tipc/core.c
> 
> @@ -81,7 +81,9 @@ static int __net_init tipc_init_net(struct net *net)
> 
>                err = tipc_bcast_init(net);
> 
>                if (err)
> 
>                                goto out_bclink;
> 
> -
> 
> +             err = tipc_attach_loopback(net);
> 
> +             if (err)
> 
> +                             goto out_bclink;
> 
>                return 0;
> 
>  out_bclink:
> 
> @@ -94,6 +96,7 @@ static int __net_init tipc_init_net(struct net *net)
> 
>  static void __net_exit tipc_exit_net(struct net *net)
> 
> {
> 
> +             tipc_detach_loopback(net);
> 
>                tipc_net_stop(net);
> 
>                tipc_bcast_stop(net);
> 
>                tipc_nametbl_stop(net);
> 
> diff --git a/net/tipc/core.h b/net/tipc/core.h
> 
> index 7a68e1b..de164ac 100644
> 
> --- a/net/tipc/core.h
> 
> +++ b/net/tipc/core.h
> 
> @@ -125,6 +125,10 @@ struct tipc_net {
> 
>                 /* Cluster capabilities */
> 
>                u16 capabilities;
> 
> +
> 
> +             /* Tracing of node internal messages */
> 
> +             struct packet_type loopback_pt;
> 
> +             bool loopback_trace;
> 
> };
> 
>  static inline struct tipc_net *tipc_net(struct net *net)
> 
> @@ -152,6 +156,11 @@ static inline struct tipc_topsrv *tipc_topsrv(struct
> net *net)
> 
>                return tipc_net(net)->topsrv;
> 
> }
> 
> +static inline bool tipc_loopback_trace(struct net *net)
> 
> +{
> 
> +             return tipc_net(net)->loopback_trace;
> 
> +}
> 
> +
> 
> static inline unsigned int tipc_hashfn(u32 addr)
> 
> {
> 
>                return addr & (NODE_HTABLE_SIZE - 1);
> 
> diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c
> 
> index 99bd166..d6165ad 100644
> 
> --- a/net/tipc/netlink.c
> 
> +++ b/net/tipc/netlink.c
> 
> @@ -261,7 +261,7 @@ struct genl_family tipc_genl_family __ro_after_init =
> {
> 
>                .version                = TIPC_GENL_V2_VERSION,
> 
>                .hdrsize                = 0,
> 
>                .maxattr               = TIPC_NLA_MAX,
> 
> -              .policy = tipc_nl_policy,
> 
> +             .policy                   = tipc_nl_policy,
> 
>                .netnsok              = true,
> 
>                .module                               = THIS_MODULE,
> 
>                .ops                       = tipc_genl_v2_ops,
> 
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> 
> index 9e106d3..415d02d 100644
> 
> --- a/net/tipc/node.c
> 
> +++ b/net/tipc/node.c
> 
> @@ -1439,6 +1439,8 @@ int tipc_node_xmit(struct net *net, struct
> sk_buff_head *list,
> 
>                int rc;
> 
>                 if (in_own_node(net, dnode)) {
> 
> +                             if (unlikely(tipc_loopback_trace(net)))
> 
> +
> + tipc_clone_to_loopback(net,
> list);
> 
>                                tipc_sk_rcv(net, list);
> 
>                                return 0;
> 
>                }
> 
> diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
> 
> index f345662..37f07e7 100644
> 
> --- a/net/tipc/topsrv.c
> 
> +++ b/net/tipc/topsrv.c
> 
> @@ -40,6 +40,7 @@
> 
> #include "socket.h"
> 
> #include "addr.h"
> 
> #include "msg.h"
> 
> +#include "bearer.h"
> 
> #include <net/sock.h>
> 
> #include <linux/module.h>
> 
> @@ -608,6 +609,8 @@ static void tipc_topsrv_kern_evt(struct net *net,
> struct tipc_event *evt)
> 
>                memcpy(msg_data(buf_msg(skb)), evt, sizeof(*evt));
> 
>                skb_queue_head_init(&evtq);
> 
>                __skb_queue_tail(&evtq, skb);
> 
> +             if (unlikely(tipc_loopback_trace(net)))
> 
> +                             tipc_clone_to_loopback(net, &evtq);
> 
>                tipc_sk_rcv(net, &evtq);
> 
> }
> 
> --
> 
> 2.11.0
> 
> 
> 
> 
> 
> ---
> This email has been checked for viruses by AVG.
> https://www.avg.com
> 
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

[tipc-discussion] [net] tipc: fix issues with early FAILOVER_MSG from peer

[Tuong L. <tuo...@de...>]

It appears that a FAILOVER_MSG can come from peer even when the failure
link is resetting (i.e. just after the 'node_write_unlock()'...). This
means the failover procedure on the node has not been started yet.
The situation is as follows:

         node1                                node2
  linkb          linka                  linka        linkb
    |              |                      |            |
    |              |                      x failure    |
    |              |                  RESETTING        |
    |              |                      |            |
    |              x failure            RESET          |
    |          RESETTING             FAILINGOVER       |
    |              |   (FAILOVER_MSG)     |            |
    |<-------------------------------------------------|
    | *FAILINGOVER |                      |            |
    |              | (dummy FAILOVER_MSG) |            |
    |------------------------------------------------->|
    |            RESET                    |            | FAILOVER_END
    |         FAILINGOVER               RESET          |
    .              .                      .            .
    .              .                      .            .
    .              .                      .            .

Once this happens, the link failover procedure will be triggered
wrongly on the receiving node since the node isn't in FAILINGOVER state
but then another link failover will be carried out.
The consequences are:

1) A peer might get stuck in FAILINGOVER state because the 'sync_point'
was set, reset and set incorrectly, the criteria to end the failover
would not be met, it could keep waiting for a message that has already
received.

2) The early FAILOVER_MSG(s) could be queued in the link failover
deferdq but would be purged or not pulled out because the 'drop_point'
was not set correctly.

3) The early FAILOVER_MSG(s) could be dropped too.

4) The dummy FAILOVER_MSG could make the peer leaving FAILINGOVER state
shortly, but later on it would be restarted.

The same situation can also happen when the link is in PEER_RESET state
and a FAILOVER_MSG arrives.

The commit resolves the issues by forcing the link down immediately, so
the failover procedure will be started normally (which is the same as
when receiving a FAILOVER_MSG and the link is in up state).

Also, the function "tipc_node_link_failover()" is toughen to avoid such
a situation from happening.

Signed-off-by: Tuong Lien <tuo...@de...>
---
 net/tipc/link.c |  1 -
 net/tipc/node.c | 10 +++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index d5ed509e0660..bcfb0a4ab485 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1762,7 +1762,6 @@ void tipc_link_failover_prepare(struct tipc_link *l, struct tipc_link *tnl,
 	 * node has entered SELF_DOWN_PEER_LEAVING and both peer nodes
 	 * would have to start over from scratch instead.
 	 */
-	WARN_ON(l && tipc_link_is_up(l));
 	tnl->drop_point = 1;
 	tnl->failover_reasm_skb = NULL;
 
diff --git a/net/tipc/node.c b/net/tipc/node.c
index e4dba865105e..65644642c091 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -777,9 +777,9 @@ static void tipc_node_link_up(struct tipc_node *n, int bearer_id,
  *	   disturbance, wrong session, etc.)
  *	3. Link <1B-2B> up
  *	4. Link endpoint 2A down (e.g. due to link tolerance timeout)
- *	5. Node B starts failover onto link <1B-2B>
+ *	5. Node 2 starts failover onto link <1B-2B>
  *
- *	==> Node A does never start link/node failover!
+ *	==> Node 1 does never start link/node failover!
  *
  * @n: tipc node structure
  * @l: link peer endpoint failingover (- can be NULL)
@@ -794,6 +794,10 @@ static void tipc_node_link_failover(struct tipc_node *n, struct tipc_link *l,
 	if (!tipc_link_is_up(tnl))
 		return;
 
+	/* Don't rush, failure link may be in the process of resetting */
+	if (l && !tipc_link_is_reset(l))
+		return;
+
 	tipc_link_fsm_evt(tnl, LINK_SYNCH_END_EVT);
 	tipc_node_fsm_evt(n, NODE_SYNCH_END_EVT);
 
@@ -1719,7 +1723,7 @@ static bool tipc_node_check_state(struct tipc_node *n, struct sk_buff *skb,
 	/* Initiate or update failover mode if applicable */
 	if ((usr == TUNNEL_PROTOCOL) && (mtyp == FAILOVER_MSG)) {
 		syncpt = oseqno + exp_pkts - 1;
-		if (pl && tipc_link_is_up(pl)) {
+		if (pl && !tipc_link_is_reset(pl)) {
 			__tipc_node_link_down(n, &pb_id, xmitq, &maddr);
 			trace_tipc_node_link_down(n, true,
 						  "node link down <- failover!");
-- 
2.13.7

[tipc-discussion] [PATCH] tipc: add loopback device tracking

[John R. <joh...@de...>]

Since node internal messages are passed directly to socket it is not
possible to observe

this message exchange via tcpdump or wireshark.

 

We now remedy this by making it possible to clone such messages and send the
clones to the

loopback interface.  The clones are dropped at reception and have no
functional role except

making the traffic visible.

 

The feature is turned on/off by enabling/disabling the loopback "bearer"
"eth:lo".

Signed-off-by: John Rutherford <joh...@de...>

Signed-off-by: Jon Maloy <jon...@er...>

---

net/tipc/bcast.c   |  5 +++-

net/tipc/bearer.c  | 67
++++++++++++++++++++++++++++++++++++++++++++++++++++++

net/tipc/bearer.h  |  3 +++

net/tipc/core.c    |  5 +++-

net/tipc/core.h    |  9 ++++++++

net/tipc/netlink.c |  2 +-

net/tipc/node.c    |  2 ++

net/tipc/topsrv.c  |  3 +++

8 files changed, 93 insertions(+), 3 deletions(-)

 

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c

index 6c997d4..db40129 100644

--- a/net/tipc/bcast.c

+++ b/net/tipc/bcast.c

@@ -406,8 +406,11 @@ int tipc_mcast_xmit(struct net *net, struct
sk_buff_head *pkts,

                                               rc = tipc_bcast_xmit(net,
pkts, cong_link_cnt);

               }

-              if (dests->local)

+             if (dests->local) {

+                             if (unlikely(tipc_loopback_trace(net)))

+                                             tipc_clone_to_loopback(net,
&localq);

                               tipc_sk_mcast_rcv(net, &localq, &inputq);

+             }

exit:

               /* This queue should normally be empty by now */

               __skb_queue_purge(pkts);

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c

index 2bed658..27b4fd7 100644

--- a/net/tipc/bearer.c

+++ b/net/tipc/bearer.c

@@ -836,6 +836,12 @@ int __tipc_nl_bearer_disable(struct sk_buff *skb,
struct genl_info *info)

                name = nla_data(attrs[TIPC_NLA_BEARER_NAME]);

+             if (!strcmp(name, "eth:lo")) {

+                             tipc_net(net)->loopback_trace = false;

+                             pr_info("Disabled packet tracing on loopback
interface\n");

+                             return 0;

+             }

+

               bearer = tipc_bearer_find(net, name);

               if (!bearer)

                               return -EINVAL;

@@ -881,6 +887,12 @@ int __tipc_nl_bearer_enable(struct sk_buff *skb, struct
genl_info *info)

                bearer = nla_data(attrs[TIPC_NLA_BEARER_NAME]);

+             if (!strcmp(bearer, "eth:lo")) {

+                             tipc_net(net)->loopback_trace = true;

+                             pr_info("Enabled packet tracing on loopback
interface\n");

+                             return 0;

+             }

+

               if (attrs[TIPC_NLA_BEARER_DOMAIN])

                               domain =
nla_get_u32(attrs[TIPC_NLA_BEARER_DOMAIN]);

@@ -1021,6 +1033,61 @@ int tipc_nl_bearer_set(struct sk_buff *skb, struct
genl_info *info)

               return err;

}

+void tipc_clone_to_loopback(struct net *net, struct sk_buff_head *xmitq)

+{

+             struct net_device *dev = net->loopback_dev;

+             struct sk_buff *skb, *_skb;

+             int exp;

+

+             skb_queue_walk(xmitq, _skb) {

+                             skb = pskb_copy(_skb, GFP_ATOMIC);

+                             if (!skb)

+                                             continue;

+                             exp = SKB_DATA_ALIGN(dev->hard_header_len -
skb_headroom(skb));

+                             if (exp > 0 && pskb_expand_head(skb, exp, 0,
GFP_ATOMIC)) {

+                                             kfree_skb(skb);

+                                             continue;

+                             }

+                             skb_reset_network_header(skb);

+                             skb->dev = dev;

+                             skb->protocol = htons(ETH_P_TIPC);

+                             dev_hard_header(skb, dev, ETH_P_TIPC,
dev->dev_addr,

+                                                             dev->dev_addr,
skb->len);

+                             dev_queue_xmit(skb);

+             }

+}

+

+static int tipc_loopback_rcv_pkt(struct sk_buff *skb, struct net_device
*dev,

+                                                             struct
packet_type *pt, struct net_device *od)

+{

+             consume_skb(skb);

+             return NET_RX_SUCCESS;

+}

+

+int tipc_attach_loopback(struct net *net)

+{

+             struct net_device *dev = net->loopback_dev;

+             struct tipc_net *tn = tipc_net(net);

+

+             if (!dev)

+                             return -ENODEV;

+             dev_hold(dev);

+             tn->loopback_pt.dev = dev;

+             tn->loopback_pt.type = htons(ETH_P_TIPC);

+             tn->loopback_pt.func = tipc_loopback_rcv_pkt;

+             tn->loopback_trace = false;

+             dev_add_pack(&tn->loopback_pt);

+             return 0;

+}

+

+void tipc_detach_loopback(struct net *net)

+{

+             struct tipc_net *tn = tipc_net(net);

+

+             dev_remove_pack(&tn->loopback_pt);

+             dev_put(net->loopback_dev);

+}

+

static int __tipc_nl_add_media(struct tipc_nl_msg *msg,

                                                      struct tipc_media
*media, int nlflags)

{

diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h

index 7f4c569..ef7fad9 100644

--- a/net/tipc/bearer.h

+++ b/net/tipc/bearer.h

@@ -232,6 +232,9 @@ void tipc_bearer_xmit(struct net *net, u32 bearer_id,

                                     struct tipc_media_addr *dst);

void tipc_bearer_bc_xmit(struct net *net, u32 bearer_id,

                                                struct sk_buff_head *xmitq);

+void tipc_clone_to_loopback(struct net *net, struct sk_buff_head *xmitq);

+int tipc_attach_loopback(struct net *net);

+void tipc_detach_loopback(struct net *net);

 /* check if device MTU is too low for tipc headers */

static inline bool tipc_mtu_bad(struct net_device *dev, unsigned int
reserve)

diff --git a/net/tipc/core.c b/net/tipc/core.c

index ed536c0..1867687 100644

--- a/net/tipc/core.c

+++ b/net/tipc/core.c

@@ -81,7 +81,9 @@ static int __net_init tipc_init_net(struct net *net)

               err = tipc_bcast_init(net);

               if (err)

                               goto out_bclink;

-

+             err = tipc_attach_loopback(net);

+             if (err)

+                             goto out_bclink;

               return 0;

 out_bclink:

@@ -94,6 +96,7 @@ static int __net_init tipc_init_net(struct net *net)

 static void __net_exit tipc_exit_net(struct net *net)

{

+             tipc_detach_loopback(net);

               tipc_net_stop(net);

               tipc_bcast_stop(net);

               tipc_nametbl_stop(net);

diff --git a/net/tipc/core.h b/net/tipc/core.h

index 7a68e1b..de164ac 100644

--- a/net/tipc/core.h

+++ b/net/tipc/core.h

@@ -125,6 +125,10 @@ struct tipc_net {

                /* Cluster capabilities */

               u16 capabilities;

+

+             /* Tracing of node internal messages */

+             struct packet_type loopback_pt;

+             bool loopback_trace;

};

 static inline struct tipc_net *tipc_net(struct net *net)

@@ -152,6 +156,11 @@ static inline struct tipc_topsrv *tipc_topsrv(struct
net *net)

               return tipc_net(net)->topsrv;

}

+static inline bool tipc_loopback_trace(struct net *net)

+{

+             return tipc_net(net)->loopback_trace;

+}

+

static inline unsigned int tipc_hashfn(u32 addr)

{

               return addr & (NODE_HTABLE_SIZE - 1);

diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c

index 99bd166..d6165ad 100644

--- a/net/tipc/netlink.c

+++ b/net/tipc/netlink.c

@@ -261,7 +261,7 @@ struct genl_family tipc_genl_family __ro_after_init = {

               .version                = TIPC_GENL_V2_VERSION,

               .hdrsize                = 0,

               .maxattr               = TIPC_NLA_MAX,

-              .policy = tipc_nl_policy,

+             .policy                   = tipc_nl_policy,

               .netnsok              = true,

               .module                               = THIS_MODULE,

               .ops                       = tipc_genl_v2_ops,

diff --git a/net/tipc/node.c b/net/tipc/node.c

index 9e106d3..415d02d 100644

--- a/net/tipc/node.c

+++ b/net/tipc/node.c

@@ -1439,6 +1439,8 @@ int tipc_node_xmit(struct net *net, struct
sk_buff_head *list,

               int rc;

                if (in_own_node(net, dnode)) {

+                             if (unlikely(tipc_loopback_trace(net)))

+                                             tipc_clone_to_loopback(net,
list);

                               tipc_sk_rcv(net, list);

                               return 0;

               }

diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c

index f345662..37f07e7 100644

--- a/net/tipc/topsrv.c

+++ b/net/tipc/topsrv.c

@@ -40,6 +40,7 @@

#include "socket.h"

#include "addr.h"

#include "msg.h"

+#include "bearer.h"

#include <net/sock.h>

#include <linux/module.h>

@@ -608,6 +609,8 @@ static void tipc_topsrv_kern_evt(struct net *net, struct
tipc_event *evt)

               memcpy(msg_data(buf_msg(skb)), evt, sizeof(*evt));

               skb_queue_head_init(&evtq);

               __skb_queue_tail(&evtq, skb);

+             if (unlikely(tipc_loopback_trace(net)))

+                             tipc_clone_to_loopback(net, &evtq);

               tipc_sk_rcv(net, &evtq);

}

-- 

2.11.0

 



---
This email has been checked for viruses by AVG.
https://www.avg.com

[tipc-discussion] [iproute2-next v5] tipc: support interface name when activating UDP bearer

[Hoang Le <hoa...@de...>]

Support for indicating interface name has an ip address in parallel
with specifying ip address when activating UDP bearer.
This liberates the user from keeping track of the current ip address
for each device.

Old command syntax:
$tipc bearer enable media udp name NAME localip IP

New command syntax:
$tipc bearer enable media udp name NAME [localip IP|dev DEVICE]

v2:
    - Removed initial value for fd
    - Fixed the returning value for cmd_bearer_validate_and_get_addr
      to make its consistent with using: zero or non-zero
v3: - Switch to use helper 'get_ifname' to retrieve interface name
v4: - Replace legacy SIOCGIFADDR by netlink
v5: - Fix leaky rtnl_handle

Acked-by: Ying Xue <yin...@wi...>
Signed-off-by: Hoang Le <hoa...@de...>
---
 tipc/bearer.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 87 insertions(+), 5 deletions(-)

diff --git a/tipc/bearer.c b/tipc/bearer.c
index 1f3a4d44441e..e17e2477c1ad 100644
--- a/tipc/bearer.c
+++ b/tipc/bearer.c
@@ -19,10 +19,12 @@
 #include <linux/tipc_netlink.h>
 #include <linux/tipc.h>
 #include <linux/genetlink.h>
+#include <linux/if.h>
 
 #include <libmnl/libmnl.h>
 #include <sys/socket.h>
 
+#include "utils.h"
 #include "cmdl.h"
 #include "msg.h"
 #include "bearer.h"
@@ -68,7 +70,7 @@ static void cmd_bearer_enable_l2_help(struct cmdl *cmdl, char *media)
 static void cmd_bearer_enable_udp_help(struct cmdl *cmdl, char *media)
 {
 	fprintf(stderr,
-		"Usage: %s bearer enable [OPTIONS] media %s name NAME localip IP [UDP OPTIONS]\n\n"
+		"Usage: %s bearer enable [OPTIONS] media %s name NAME [localip IP|device DEVICE] [UDP OPTIONS]\n\n"
 		"OPTIONS\n"
 		" domain DOMAIN		- Discovery domain\n"
 		" priority PRIORITY	- Bearer priority\n\n"
@@ -119,6 +121,74 @@ static int generate_multicast(short af, char *buf, int bufsize)
 	return 0;
 }
 
+static struct ifreq ifr = {};
+static int nl_dump_addr_filter(struct nlmsghdr *nlh, void *arg)
+{
+	struct ifaddrmsg *ifa = NLMSG_DATA(nlh);
+	char *r_addr = (char *)arg;
+	int len = nlh->nlmsg_len;
+	struct rtattr *addr_attr;
+
+	if (ifr.ifr_ifindex != ifa->ifa_index)
+		return 0;
+
+	if (strlen(r_addr) > 0)
+		return 1;
+
+	addr_attr = parse_rtattr_one(IFA_ADDRESS, IFA_RTA(ifa),
+				     len - NLMSG_LENGTH(sizeof(*ifa)));
+	if (!addr_attr)
+		return 0;
+
+	if (ifa->ifa_family == AF_INET) {
+		struct sockaddr_in ip4addr;
+		memcpy(&ip4addr.sin_addr, RTA_DATA(addr_attr),
+		       sizeof(struct in_addr));
+		if (inet_ntop(AF_INET, &ip4addr.sin_addr, r_addr,
+			      INET_ADDRSTRLEN) == NULL)
+			return 0;
+	} else if (ifa->ifa_family == AF_INET6) {
+		struct sockaddr_in6 ip6addr;
+		memcpy(&ip6addr.sin6_addr, RTA_DATA(addr_attr),
+		       sizeof(struct in6_addr));
+		if (inet_ntop(AF_INET6, &ip6addr.sin6_addr, r_addr,
+			      INET6_ADDRSTRLEN) == NULL)
+			return 0;
+	}
+	return 1;
+}
+
+static int cmd_bearer_validate_and_get_addr(const char *name, char *r_addr)
+{
+	struct rtnl_handle rth ={ .fd = -1 };
+
+	memset(&ifr, 0, sizeof(ifr));
+	if (!name || !r_addr || get_ifname(ifr.ifr_name, name))
+		return 0;
+
+	ifr.ifr_ifindex = ll_name_to_index(ifr.ifr_name);
+	if (!ifr.ifr_ifindex)
+		return 0;
+
+	/* remove from cache */
+	ll_drop_by_index(ifr.ifr_ifindex);
+
+	if (rtnl_open(&rth, 0) < 0)
+		return 0;
+
+	if (rtnl_addrdump_req(&rth, AF_UNSPEC, 0) < 0) {
+		rtnl_close(&rth);
+		return 0;
+	}
+
+	if (rtnl_dump_filter(&rth, nl_dump_addr_filter, r_addr) < 0) {
+		rtnl_close(&rth);
+		return 0;
+	}
+	rtnl_close(&rth);
+	return 1;
+}
+
 static int nl_add_udp_enable_opts(struct nlmsghdr *nlh, struct opt *opts,
 				  struct cmdl *cmdl)
 {
@@ -136,13 +206,25 @@ static int nl_add_udp_enable_opts(struct nlmsghdr *nlh, struct opt *opts,
 		.ai_family = AF_UNSPEC,
 		.ai_socktype = SOCK_DGRAM
 	};
+	char addr[INET6_ADDRSTRLEN] = {0};
 
-	if (!(opt = get_opt(opts, "localip"))) {
-		fprintf(stderr, "error, udp bearer localip missing\n");
-		cmd_bearer_enable_udp_help(cmdl, "udp");
+	opt = get_opt(opts, "device");
+	if (opt && !cmd_bearer_validate_and_get_addr(opt->val, addr)) {
+		fprintf(stderr, "error, no device name available\n");
 		return -EINVAL;
 	}
-	locip = opt->val;
+
+	if (strlen(addr) > 0) {
+		locip = addr;
+	} else {
+		opt = get_opt(opts, "localip");
+		if (!opt) {
+			fprintf(stderr, "error, udp bearer localip/device missing\n");
+			cmd_bearer_enable_udp_help(cmdl, "udp");
+			return -EINVAL;
+		}
+		locip = opt->val;
+	}
 
 	if ((opt = get_opt(opts, "remoteip")))
 		remip = opt->val;
-- 
2.17.1

[tipc-discussion] [iproute2-next v4] tipc: support interface name when activating UDP bearer

[Hoang Le <hoa...@de...>]

Support for indicating interface name has an ip address in parallel
with specifying ip address when activating UDP bearer.
This liberates the user from keeping track of the current ip address
for each device.

Old command syntax:
$tipc bearer enable media udp name NAME localip IP

New command syntax:
$tipc bearer enable media udp name NAME [localip IP|dev DEVICE]

v2:
    - Removed initial value for fd
    - Fixed the returning value for cmd_bearer_validate_and_get_addr
      to make its consistent with using: zero or non-zero
v3:
    - Switch to use helper 'get_ifname' to retrieve interface name

v4:
    - Replace legacy SIOCGIFADDR using by netlink

Acked-by: Ying Xue <yin...@wi...>
Signed-off-by: Hoang Le <hoa...@de...>
---
 tipc/bearer.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 84 insertions(+), 5 deletions(-)

diff --git a/tipc/bearer.c b/tipc/bearer.c
index 1f3a4d44441e..367ec8a2630f 100644
--- a/tipc/bearer.c
+++ b/tipc/bearer.c
@@ -19,10 +19,12 @@
 #include <linux/tipc_netlink.h>
 #include <linux/tipc.h>
 #include <linux/genetlink.h>
+#include <linux/if.h>
 
 #include <libmnl/libmnl.h>
 #include <sys/socket.h>
 
+#include "utils.h"
 #include "cmdl.h"
 #include "msg.h"
 #include "bearer.h"
@@ -68,7 +70,7 @@ static void cmd_bearer_enable_l2_help(struct cmdl *cmdl, char *media)
 static void cmd_bearer_enable_udp_help(struct cmdl *cmdl, char *media)
 {
 	fprintf(stderr,
-		"Usage: %s bearer enable [OPTIONS] media %s name NAME localip IP [UDP OPTIONS]\n\n"
+		"Usage: %s bearer enable [OPTIONS] media %s name NAME [localip IP|device DEVICE] [UDP OPTIONS]\n\n"
 		"OPTIONS\n"
 		" domain DOMAIN		- Discovery domain\n"
 		" priority PRIORITY	- Bearer priority\n\n"
@@ -119,6 +121,71 @@ static int generate_multicast(short af, char *buf, int bufsize)
 	return 0;
 }
 
+static struct ifreq ifr;
+static int nl_dump_addr_filter(struct nlmsghdr *nlh, void *arg)
+{
+	struct ifaddrmsg *ifa = NLMSG_DATA(nlh);
+	char *r_addr = (char *)arg;
+	int len = nlh->nlmsg_len;
+	struct rtattr *addr_attr;
+
+	if (ifr.ifr_ifindex != ifa->ifa_index)
+		return 0;
+
+	if (strlen(r_addr) > 0)
+		return 1;
+
+	addr_attr = parse_rtattr_one(IFA_ADDRESS, IFA_RTA(ifa),
+				     len - NLMSG_LENGTH(sizeof(*ifa)));
+	if (!addr_attr)
+		return 0;
+
+	if (ifa->ifa_family == AF_INET) {
+		struct sockaddr_in ip4addr;
+		memcpy(&ip4addr.sin_addr, RTA_DATA(addr_attr),
+		       sizeof(struct in_addr));
+		if (inet_ntop(AF_INET, &ip4addr.sin_addr, r_addr,
+			      INET_ADDRSTRLEN) == NULL)
+			return 0;
+	} else if (ifa->ifa_family == AF_INET6) {
+		struct sockaddr_in6 ip6addr;
+		memcpy(&ip6addr.sin6_addr, RTA_DATA(addr_attr),
+		       sizeof(struct in6_addr));
+		if (inet_ntop(AF_INET6, &ip6addr.sin6_addr, r_addr,
+			      INET6_ADDRSTRLEN) == NULL)
+			return 0;
+	}
+	return 1;
+}
+
+static int cmd_bearer_validate_and_get_addr(const char *name, char *r_addr)
+{
+	struct rtnl_handle rth = { .fd = -1 };
+
+	memset(&ifr, 0, sizeof(ifr));
+	if (!name || !r_addr || get_ifname(ifr.ifr_name, name))
+		return 0;
+
+	ifr.ifr_ifindex = ll_name_to_index(ifr.ifr_name);
+	if (!ifr.ifr_ifindex)
+		return 0;
+
+	/* remove from cache */
+	ll_drop_by_index(ifr.ifr_ifindex);
+
+	if (rtnl_open(&rth, 0) < 0)
+		return 0;
+
+	if (rtnl_addrdump_req(&rth, AF_UNSPEC, 0) < 0)
+		return 0;
+
+	if (rtnl_dump_filter(&rth, nl_dump_addr_filter, r_addr) < 0)
+		return 0;
+
+	rtnl_close(&rth);
+	return 1;
+}
+
 static int nl_add_udp_enable_opts(struct nlmsghdr *nlh, struct opt *opts,
 				  struct cmdl *cmdl)
 {
@@ -136,13 +203,25 @@ static int nl_add_udp_enable_opts(struct nlmsghdr *nlh, struct opt *opts,
 		.ai_family = AF_UNSPEC,
 		.ai_socktype = SOCK_DGRAM
 	};
+	char addr[INET6_ADDRSTRLEN] = {0};
 
-	if (!(opt = get_opt(opts, "localip"))) {
-		fprintf(stderr, "error, udp bearer localip missing\n");
-		cmd_bearer_enable_udp_help(cmdl, "udp");
+	opt = get_opt(opts, "device");
+	if (opt && !cmd_bearer_validate_and_get_addr(opt->val, addr)) {
+		fprintf(stderr, "error, no device name available\n");
 		return -EINVAL;
 	}
-	locip = opt->val;
+
+	if (strlen(addr) > 0) {
+		locip = addr;
+	} else {
+		opt = get_opt(opts, "localip");
+		if (!opt) {
+			fprintf(stderr, "error, udp bearer localip/device missing\n");
+			cmd_bearer_enable_udp_help(cmdl, "udp");
+			return -EINVAL;
+		}
+		locip = opt->val;
+	}
 
 	if ((opt = get_opt(opts, "remoteip")))
 		remip = opt->val;
-- 
2.17.1

Re: [tipc-discussion] [PATCH RFC] tipc: include retrans failure detection for unicast

[Jon M. <jon...@er...>]

Hi Tuong,
I have only minor comments to this. See below.
Post this to netdev (net-next) first, and I will post my modified patch on top of it later.

Acked-by: Jon Maloy <jon...@er...>

///jon

> -----Original Message-----
> From: Tuong Lien <tuo...@de...>
> Sent: 12-Jun-19 07:22
> To: tip...@li...; Jon Maloy
> <jon...@er...>; ma...@do...; yin...@wi...
> Subject: [PATCH RFC] tipc: include retrans failure detection for unicast
> 
> In patch series, commit 9195948fbf34 ("tipc: improve TIPC throughput by
> Gap ACK blocks"), as for simplicity, the repeated retransmit failures'
> detection in the function - "tipc_link_retrans()" was kept there for broadcast
> retransmissions only.
> 
> This commit now reapplies this feature for link unicast retransmissions that has
> been done via the function - "tipc_link_advance_transmq()".
> 
> Signed-off-by: Tuong Lien <tuo...@de...>
> ---
>  net/tipc/link.c | 96 ++++++++++++++++++++++++++++++++++++++-----------
> --------
>  1 file changed, 65 insertions(+), 31 deletions(-)
> 
> diff --git a/net/tipc/link.c b/net/tipc/link.c index f5cd986e1e50..ffe3880a7b79
> 100644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -249,9 +249,9 @@ static void tipc_link_build_bc_init_msg(struct
> tipc_link *l,
>  					struct sk_buff_head *xmitq);
>  static bool tipc_link_release_pkts(struct tipc_link *l, u16 to);  static u16
> tipc_build_gap_ack_blks(struct tipc_link *l, void *data); -static void
> tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16 gap,
> -				      struct tipc_gap_ack_blks *ga,
> -				      struct sk_buff_head *xmitq);
> +static int tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16 gap,
> +				     struct tipc_gap_ack_blks *ga,
> +				     struct sk_buff_head *xmitq);
> 
>  /*
>   *  Simple non-static link routines (i.e. referenced outside this file) @@ -
> 1044,16 +1044,53 @@ static void tipc_link_advance_backlog(struct tipc_link
> *l,
>  	l->snd_nxt = seqno;
>  }
> 
> -static void link_retransmit_failure(struct tipc_link *l, struct sk_buff *skb)
> +/**
> + * link_retransmit_failure() - Detect if retransmit failures repeated

"Detect repeated retransmit failures"

> + * @l: tipc link sender
> + * @r: tipc link receiver (= l in case of unicast)
> + * @from: seqno of the 1st packet in the retransmit request
> + * @rc: returned code
> + *
> + * Return: true if the repeated retransmit failures happen, otherwise
s/happen/happens/

> + * false
> + */
> +static bool link_retransmit_failure(struct tipc_link *l, struct tipc_link *r,
> +				    u16 from, int *rc)
>  {
> -	struct tipc_msg *hdr = buf_msg(skb);
> +	struct sk_buff *skb = skb_peek(&l->transmq);
> +	struct tipc_msg *hdr;
> 
> -	pr_warn("Retransmission failure on link <%s>\n", l->name);
> -	link_print(l, "State of link ");
> -	pr_info("Failed msg: usr %u, typ %u, len %u, err %u\n",
> -		msg_user(hdr), msg_type(hdr), msg_size(hdr),
> msg_errcode(hdr));
> -	pr_info("sqno %u, prev: %x, src: %x\n",
> -		msg_seqno(hdr), msg_prevnode(hdr), msg_orignode(hdr));
> +	if (!skb)
> +		return false;
> +
> +	hdr = buf_msg(skb);
> +
> +	/* Detect repeated retransmit failures on same packet */
> +	if (r->prev_from != from) {
> +		r->prev_from = from;
> +		r->stale_limit = jiffies + msecs_to_jiffies(r->tolerance);
> +		r->stale_cnt = 0;
> +	} else if (++r->stale_cnt > 99 && time_after(jiffies, r->stale_limit)) {
> +		pr_warn("Retransmission failure on link <%s>\n", l->name);
> +			link_print(l, "State of link ");
> +		pr_info("Failed msg: usr %u, typ %u, len %u, err %u\n",
> +			msg_user(hdr), msg_type(hdr),
> +			msg_size(hdr), msg_errcode(hdr));
> +		pr_info("sqno %u, prev: %x, src: %x\n",
> +			msg_seqno(hdr), msg_prevnode(hdr),
> msg_orignode(hdr));
> +
> +		trace_tipc_list_dump(&l->transmq, true, "retrans failure!");
> +		trace_tipc_link_dump(l, TIPC_DUMP_NONE, "retrans failure!");
> +		trace_tipc_link_dump(r, TIPC_DUMP_NONE, "retrans failure!");
> +
> +		if (link_is_bc_sndlink(l))
> +			*rc = TIPC_LINK_DOWN_EVT;
> +
> +		*rc = tipc_link_fsm_evt(l, LINK_FAILURE_EVT);
> +		return true;
> +	}
> +
> +	return false;
>  }
> 
>  /* tipc_link_retrans() - retransmit one or more packets @@ -1062,6 +1099,8

"zero ore more" is more correct here.

> @@ static void link_retransmit_failure(struct tipc_link *l, struct sk_buff *skb)
>   * @from: retransmit from (inclusive) this sequence number
>   * @to: retransmit to (inclusive) this sequence number
>   * xmitq: queue for accumulating the retransmitted packets
> + *
> + * Note: this retrans is for broadcast only!
>   */
>  static int tipc_link_retrans(struct tipc_link *l, struct tipc_link *r,
>  			     u16 from, u16 to, struct sk_buff_head *xmitq) @@ -

Since this one now is used only for broadcast, I suggest we rename it to tipc_link_bc_retrans(), as per convention elsewhere.


> 1070,6 +1109,7 @@ static int tipc_link_retrans(struct tipc_link *l, struct
> tipc_link *r,
>  	u16 bc_ack = l->bc_rcvlink->rcv_nxt - 1;
>  	u16 ack = l->rcv_nxt - 1;
>  	struct tipc_msg *hdr;
> +	int rc = 0;
> 
>  	if (!skb)
>  		return 0;
> @@ -1077,20 +1117,9 @@ static int tipc_link_retrans(struct tipc_link *l,
> struct tipc_link *r,
>  		return 0;
> 
>  	trace_tipc_link_retrans(r, from, to, &l->transmq);
> -	/* Detect repeated retransmit failures on same packet */
> -	if (r->prev_from != from) {
> -		r->prev_from = from;
> -		r->stale_limit = jiffies + msecs_to_jiffies(r->tolerance);
> -		r->stale_cnt = 0;
> -	} else if (++r->stale_cnt > 99 && time_after(jiffies, r->stale_limit)) {
> -		link_retransmit_failure(l, skb);
> -		trace_tipc_list_dump(&l->transmq, true, "retrans failure!");
> -		trace_tipc_link_dump(l, TIPC_DUMP_NONE, "retrans failure!");
> -		trace_tipc_link_dump(r, TIPC_DUMP_NONE, "retrans failure!");
> -		if (link_is_bc_sndlink(l))
> -			return TIPC_LINK_DOWN_EVT;
> -		return tipc_link_fsm_evt(l, LINK_FAILURE_EVT);
> -	}
> +
> +	if (link_retransmit_failure(l, r, from, &rc))
> +		return rc;
> 
>  	skb_queue_walk(&l->transmq, skb) {
>  		hdr = buf_msg(skb);
> @@ -1325,16 +1354,19 @@ static u16 tipc_build_gap_ack_blks(struct
> tipc_link *l, void *data)
>   * @ga: buffer pointer to Gap ACK blocks from peer
>   * @xmitq: queue for accumulating the retransmitted packets if any
>   */
> -static void tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16
> gap,
> -				      struct tipc_gap_ack_blks *ga,
> -				      struct sk_buff_head *xmitq)
> +static int tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16 gap,
> +				     struct tipc_gap_ack_blks *ga,
> +				     struct sk_buff_head *xmitq)
>  {
>  	struct sk_buff *skb, *_skb, *tmp;
>  	struct tipc_msg *hdr;
>  	u16 bc_ack = l->bc_rcvlink->rcv_nxt - 1;
>  	u16 ack = l->rcv_nxt - 1;
> -	u16 seqno;
> -	u16 n = 0;
> +	u16 seqno, n = 0;
> +	int rc = 0;
> +
> +	if (gap && link_retransmit_failure(l, l, acked + 1, &rc))
> +		return rc;
> 
>  	skb_queue_walk_safe(&l->transmq, skb, tmp) {
>  		seqno = buf_seqno(skb);
> @@ -1369,6 +1401,8 @@ static void tipc_link_advance_transmq(struct
> tipc_link *l, u16 acked, u16 gap,
>  			goto next_gap_ack;
>  		}
>  	}
> +
> +	return 0;
>  }
> 
>  /* tipc_link_build_state_msg: prepare link state message for transmission
> @@ -1919,7 +1953,7 @@ static int tipc_link_proto_rcv(struct tipc_link *l,
> struct sk_buff *skb,
>  			tipc_link_build_proto_msg(l, STATE_MSG, 0, reply,
>  						  rcvgap, 0, 0, xmitq);
> 
> -		tipc_link_advance_transmq(l, ack, gap, ga, xmitq);
> +		rc = tipc_link_advance_transmq(l, ack, gap, ga, xmitq);
> 
>  		/* If NACK, retransmit will now start at right position */
>  		if (gap)
> --
> 2.13.7

[tipc-discussion] [PATCH RFC] tipc: include retrans failure detection for unicast

[Tuong L. <tuo...@de...>]

In patch series, commit 9195948fbf34 ("tipc: improve TIPC throughput by
Gap ACK blocks"), as for simplicity, the repeated retransmit failures'
detection in the function - "tipc_link_retrans()" was kept there for
broadcast retransmissions only.

This commit now reapplies this feature for link unicast retransmissions
that has been done via the function - "tipc_link_advance_transmq()".

Signed-off-by: Tuong Lien <tuo...@de...>
---
 net/tipc/link.c | 96 ++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 65 insertions(+), 31 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index f5cd986e1e50..ffe3880a7b79 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -249,9 +249,9 @@ static void tipc_link_build_bc_init_msg(struct tipc_link *l,
 					struct sk_buff_head *xmitq);
 static bool tipc_link_release_pkts(struct tipc_link *l, u16 to);
 static u16 tipc_build_gap_ack_blks(struct tipc_link *l, void *data);
-static void tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16 gap,
-				      struct tipc_gap_ack_blks *ga,
-				      struct sk_buff_head *xmitq);
+static int tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16 gap,
+				     struct tipc_gap_ack_blks *ga,
+				     struct sk_buff_head *xmitq);
 
 /*
  *  Simple non-static link routines (i.e. referenced outside this file)
@@ -1044,16 +1044,53 @@ static void tipc_link_advance_backlog(struct tipc_link *l,
 	l->snd_nxt = seqno;
 }
 
-static void link_retransmit_failure(struct tipc_link *l, struct sk_buff *skb)
+/**
+ * link_retransmit_failure() - Detect if retransmit failures repeated
+ * @l: tipc link sender
+ * @r: tipc link receiver (= l in case of unicast)
+ * @from: seqno of the 1st packet in the retransmit request
+ * @rc: returned code
+ *
+ * Return: true if the repeated retransmit failures happen, otherwise
+ * false
+ */
+static bool link_retransmit_failure(struct tipc_link *l, struct tipc_link *r,
+				    u16 from, int *rc)
 {
-	struct tipc_msg *hdr = buf_msg(skb);
+	struct sk_buff *skb = skb_peek(&l->transmq);
+	struct tipc_msg *hdr;
 
-	pr_warn("Retransmission failure on link <%s>\n", l->name);
-	link_print(l, "State of link ");
-	pr_info("Failed msg: usr %u, typ %u, len %u, err %u\n",
-		msg_user(hdr), msg_type(hdr), msg_size(hdr), msg_errcode(hdr));
-	pr_info("sqno %u, prev: %x, src: %x\n",
-		msg_seqno(hdr), msg_prevnode(hdr), msg_orignode(hdr));
+	if (!skb)
+		return false;
+
+	hdr = buf_msg(skb);
+
+	/* Detect repeated retransmit failures on same packet */
+	if (r->prev_from != from) {
+		r->prev_from = from;
+		r->stale_limit = jiffies + msecs_to_jiffies(r->tolerance);
+		r->stale_cnt = 0;
+	} else if (++r->stale_cnt > 99 && time_after(jiffies, r->stale_limit)) {
+		pr_warn("Retransmission failure on link <%s>\n", l->name);
+			link_print(l, "State of link ");
+		pr_info("Failed msg: usr %u, typ %u, len %u, err %u\n",
+			msg_user(hdr), msg_type(hdr),
+			msg_size(hdr), msg_errcode(hdr));
+		pr_info("sqno %u, prev: %x, src: %x\n",
+			msg_seqno(hdr), msg_prevnode(hdr), msg_orignode(hdr));
+
+		trace_tipc_list_dump(&l->transmq, true, "retrans failure!");
+		trace_tipc_link_dump(l, TIPC_DUMP_NONE, "retrans failure!");
+		trace_tipc_link_dump(r, TIPC_DUMP_NONE, "retrans failure!");
+
+		if (link_is_bc_sndlink(l))
+			*rc = TIPC_LINK_DOWN_EVT;
+
+		*rc = tipc_link_fsm_evt(l, LINK_FAILURE_EVT);
+		return true;
+	}
+
+	return false;
 }
 
 /* tipc_link_retrans() - retransmit one or more packets
@@ -1062,6 +1099,8 @@ static void link_retransmit_failure(struct tipc_link *l, struct sk_buff *skb)
  * @from: retransmit from (inclusive) this sequence number
  * @to: retransmit to (inclusive) this sequence number
  * xmitq: queue for accumulating the retransmitted packets
+ *
+ * Note: this retrans is for broadcast only!
  */
 static int tipc_link_retrans(struct tipc_link *l, struct tipc_link *r,
 			     u16 from, u16 to, struct sk_buff_head *xmitq)
@@ -1070,6 +1109,7 @@ static int tipc_link_retrans(struct tipc_link *l, struct tipc_link *r,
 	u16 bc_ack = l->bc_rcvlink->rcv_nxt - 1;
 	u16 ack = l->rcv_nxt - 1;
 	struct tipc_msg *hdr;
+	int rc = 0;
 
 	if (!skb)
 		return 0;
@@ -1077,20 +1117,9 @@ static int tipc_link_retrans(struct tipc_link *l, struct tipc_link *r,
 		return 0;
 
 	trace_tipc_link_retrans(r, from, to, &l->transmq);
-	/* Detect repeated retransmit failures on same packet */
-	if (r->prev_from != from) {
-		r->prev_from = from;
-		r->stale_limit = jiffies + msecs_to_jiffies(r->tolerance);
-		r->stale_cnt = 0;
-	} else if (++r->stale_cnt > 99 && time_after(jiffies, r->stale_limit)) {
-		link_retransmit_failure(l, skb);
-		trace_tipc_list_dump(&l->transmq, true, "retrans failure!");
-		trace_tipc_link_dump(l, TIPC_DUMP_NONE, "retrans failure!");
-		trace_tipc_link_dump(r, TIPC_DUMP_NONE, "retrans failure!");
-		if (link_is_bc_sndlink(l))
-			return TIPC_LINK_DOWN_EVT;
-		return tipc_link_fsm_evt(l, LINK_FAILURE_EVT);
-	}
+
+	if (link_retransmit_failure(l, r, from, &rc))
+		return rc;
 
 	skb_queue_walk(&l->transmq, skb) {
 		hdr = buf_msg(skb);
@@ -1325,16 +1354,19 @@ static u16 tipc_build_gap_ack_blks(struct tipc_link *l, void *data)
  * @ga: buffer pointer to Gap ACK blocks from peer
  * @xmitq: queue for accumulating the retransmitted packets if any
  */
-static void tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16 gap,
-				      struct tipc_gap_ack_blks *ga,
-				      struct sk_buff_head *xmitq)
+static int tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16 gap,
+				     struct tipc_gap_ack_blks *ga,
+				     struct sk_buff_head *xmitq)
 {
 	struct sk_buff *skb, *_skb, *tmp;
 	struct tipc_msg *hdr;
 	u16 bc_ack = l->bc_rcvlink->rcv_nxt - 1;
 	u16 ack = l->rcv_nxt - 1;
-	u16 seqno;
-	u16 n = 0;
+	u16 seqno, n = 0;
+	int rc = 0;
+
+	if (gap && link_retransmit_failure(l, l, acked + 1, &rc))
+		return rc;
 
 	skb_queue_walk_safe(&l->transmq, skb, tmp) {
 		seqno = buf_seqno(skb);
@@ -1369,6 +1401,8 @@ static void tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16 gap,
 			goto next_gap_ack;
 		}
 	}
+
+	return 0;
 }
 
 /* tipc_link_build_state_msg: prepare link state message for transmission
@@ -1919,7 +1953,7 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
 			tipc_link_build_proto_msg(l, STATE_MSG, 0, reply,
 						  rcvgap, 0, 0, xmitq);
 
-		tipc_link_advance_transmq(l, ack, gap, ga, xmitq);
+		rc = tipc_link_advance_transmq(l, ack, gap, ga, xmitq);
 
 		/* If NACK, retransmit will now start at right position */
 		if (gap)
-- 
2.13.7

Re: [tipc-discussion] memory leak in tipc_buf_acquire

[Jon M. <jon...@er...>]

> -----Original Message-----
> From: net...@vg... <net...@vg...> On
> Behalf Of Xin Long
> Sent: 9-Jun-19 14:45
> To: syzbot <syz...@sy...>
> Cc: davem <da...@da...>; Jon Maloy <jon...@er...>;
> LKML <lin...@vg...>; network dev
> <ne...@vg...>; syzkaller-bugs <syzkaller-
> bu...@go...>; tip...@li...; Ying Xue
> <yin...@wi...>
> Subject: Re: memory leak in tipc_buf_acquire
> 
> On Sat, May 25, 2019 at 5:18 AM syzbot
> <syz...@sy...> wrote:
> >
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit:    4dde821e Merge tag 'xfs-5.2-fixes-1' of git://git.kernel.o..
> > git tree:       upstream
> > console output:
> > https://protect2.fireeye.com/url?k=2a9bdca3-761109b5-2a9b9c38-
> 862f14a9
> > 365e-
> 4ed8fb52eb782aab&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%
> 2F
> > log.txt%3Fx%3D107db73aa00000 kernel config:
> > https://protect2.fireeye.com/url?k=a1863015-fd0ce503-a186708e-
> 862f14a9
> > 365e-
> 04056f2111354660&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%
> 2F
> > .config%3Fx%3D61dd9e15a761691d dashboard link:
> > https://protect2.fireeye.com/url?k=ba923b23-e618ee35-ba927bb8-
> 862f14a9365e-
> 6187e2f343fe3a3e&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fbug
> %3Fextid%3D78fbe679c8ca8d264a8d
> > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro:      https://protect2.fireeye.com/url?k=27ea1a58-7b60cf4e-
> 27ea5ac3-862f14a9365e-
> f17f13fb7f100fa1&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2
> Frepro.syz%3Fx%3D162bd84ca00000
> > C reproducer:   https://protect2.fireeye.com/url?k=dbe4799b-876eac8d-
> dbe43900-862f14a9365e-
> 787a755a5a1800d6&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fx
> %2Frepro.c%3Fx%3D160c605ca00000
> >
> Looks we need to purge each member's deferredq list in tipc_group_delete():
> diff --git a/net/tipc/group.c b/net/tipc/group.c index 992be61..23823eb
> 100644
> --- a/net/tipc/group.c
> +++ b/net/tipc/group.c
> @@ -218,6 +218,7 @@ void tipc_group_delete(struct net *net, struct
> tipc_group *grp)
> 
>   rbtree_postorder_for_each_entry_safe(m, tmp, tree, tree_node) {
>   tipc_group_proto_xmit(grp, m, GRP_LEAVE_MSG, &xmitq);
> + __skb_queue_purge(&m->deferredq);
>   list_del(&m->list);
>   kfree(m);
>   }

Yes, I think you are right. I'll check it further.

Thanks
///jon

> 
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syz...@sy...
> >
> > type=1400 audit(1558701681.775:36): avc:  denied  { map } for
> > pid=7128 comm="syz-executor987" path="/root/syz-executor987656147"
> dev="sda1"
> > ino=15900 scontext=unconfined_u:system_r:insmod_t:s0-s0:c0.c1023
> > tcontext=unconfined_u:object_r:user_home_t:s0 tclass=file permissive=1
> > executing program executing program executing program executing
> > program
> > BUG: memory leak
> > unreferenced object 0xffff88810df83c00 (size 512):
> >    comm "softirq", pid 0, jiffies 4294942354 (age 19.830s)
> >    hex dump (first 32 bytes):
> >      38 1a 0d 0f 81 88 ff ff 38 1a 0d 0f 81 88 ff ff  8.......8.......
> >      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >    backtrace:
> >      [<000000009375ee42>] kmemleak_alloc_recursive
> > include/linux/kmemleak.h:55 [inline]
> >      [<000000009375ee42>] slab_post_alloc_hook mm/slab.h:439 [inline]
> >      [<000000009375ee42>] slab_alloc_node mm/slab.c:3269 [inline]
> >      [<000000009375ee42>] kmem_cache_alloc_node+0x153/0x2a0
> mm/slab.c:3579
> >      [<000000004c563922>] __alloc_skb+0x6e/0x210 net/core/skbuff.c:198
> >      [<00000000ec87bfa1>] alloc_skb_fclone include/linux/skbuff.h:1107
> > [inline]
> >      [<00000000ec87bfa1>] tipc_buf_acquire+0x2f/0x80 net/tipc/msg.c:66
> >      [<00000000d151ef84>] tipc_msg_create+0x37/0xe0 net/tipc/msg.c:98
> >      [<000000008bb437b0>] tipc_group_create_event+0xb3/0x1b0
> > net/tipc/group.c:679
> >      [<00000000947b1d0f>] tipc_group_proto_rcv+0x569/0x640
> > net/tipc/group.c:781
> >      [<00000000b75ab039>] tipc_sk_proto_rcv net/tipc/socket.c:1996
> [inline]
> >      [<00000000b75ab039>] tipc_sk_filter_rcv+0x9ac/0xf20
> > net/tipc/socket.c:2163
> >      [<000000000dab7a6c>] tipc_sk_enqueue net/tipc/socket.c:2255 [inline]
> >      [<000000000dab7a6c>] tipc_sk_rcv+0x494/0x8a0
> net/tipc/socket.c:2306
> >      [<00000000023a7ddd>] tipc_node_xmit+0x196/0x1f0
> net/tipc/node.c:1442
> >      [<00000000337dd9eb>] tipc_node_xmit_skb net/tipc/node.c:1491
> [inline]
> >      [<00000000337dd9eb>] tipc_node_distr_xmit+0x7d/0x120
> > net/tipc/node.c:1506
> >      [<00000000b6375182>] tipc_group_delete+0xe6/0x130
> net/tipc/group.c:224
> >      [<000000000361ba2b>] tipc_sk_leave+0x57/0xb0
> net/tipc/socket.c:2925
> >      [<000000009df90505>] tipc_release+0x7b/0x5e0 net/tipc/socket.c:584
> >      [<000000009f3189da>] __sock_release+0x4b/0xe0 net/socket.c:607
> >      [<00000000d3568ee0>] sock_close+0x1b/0x30 net/socket.c:1279
> >      [<00000000266a6215>] __fput+0xed/0x300 fs/file_table.c:280
> >
> > BUG: memory leak
> > unreferenced object 0xffff888111895400 (size 1024):
> >    comm "softirq", pid 0, jiffies 4294942354 (age 19.830s)
> >    hex dump (first 32 bytes):
> >      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >    backtrace:
> >      [<00000000e2e2855e>] kmemleak_alloc_recursive
> > include/linux/kmemleak.h:55 [inline]
> >      [<00000000e2e2855e>] slab_post_alloc_hook mm/slab.h:439 [inline]
> >      [<00000000e2e2855e>] slab_alloc_node mm/slab.c:3269 [inline]
> >      [<00000000e2e2855e>] kmem_cache_alloc_node_trace+0x15b/0x2a0
> > mm/slab.c:3597
> >      [<00000000a5030ce7>] __do_kmalloc_node mm/slab.c:3619 [inline]
> >      [<00000000a5030ce7>] __kmalloc_node_track_caller+0x38/0x50
> > mm/slab.c:3634
> >      [<0000000039212451>] __kmalloc_reserve.isra.0+0x40/0xb0
> > net/core/skbuff.c:142
> >      [<00000000307cb4cf>] __alloc_skb+0xa0/0x210 net/core/skbuff.c:210
> >      [<00000000ec87bfa1>] alloc_skb_fclone include/linux/skbuff.h:1107
> > [inline]
> >      [<00000000ec87bfa1>] tipc_buf_acquire+0x2f/0x80 net/tipc/msg.c:66
> >      [<00000000d151ef84>] tipc_msg_create+0x37/0xe0 net/tipc/msg.c:98
> >      [<000000008bb437b0>] tipc_group_create_event+0xb3/0x1b0
> > net/tipc/group.c:679
> >      [<00000000947b1d0f>] tipc_group_proto_rcv+0x569/0x640
> > net/tipc/group.c:781
> >      [<00000000b75ab039>] tipc_sk_proto_rcv net/tipc/socket.c:1996
> [inline]
> >      [<00000000b75ab039>] tipc_sk_filter_rcv+0x9ac/0xf20
> > net/tipc/socket.c:2163
> >      [<000000000dab7a6c>] tipc_sk_enqueue net/tipc/socket.c:2255 [inline]
> >      [<000000000dab7a6c>] tipc_sk_rcv+0x494/0x8a0
> net/tipc/socket.c:2306
> >      [<00000000023a7ddd>] tipc_node_xmit+0x196/0x1f0
> net/tipc/node.c:1442
> >      [<00000000337dd9eb>] tipc_node_xmit_skb net/tipc/node.c:1491
> [inline]
> >      [<00000000337dd9eb>] tipc_node_distr_xmit+0x7d/0x120
> > net/tipc/node.c:1506
> >      [<00000000b6375182>] tipc_group_delete+0xe6/0x130
> net/tipc/group.c:224
> >      [<000000000361ba2b>] tipc_sk_leave+0x57/0xb0
> net/tipc/socket.c:2925
> >      [<000000009df90505>] tipc_release+0x7b/0x5e0 net/tipc/socket.c:584
> >      [<000000009f3189da>] __sock_release+0x4b/0xe0 net/socket.c:607
> >
> > BUG: memory leak
> > unreferenced object 0xffff88810e63de00 (size 512):
> >    comm "softirq", pid 0, jiffies 4294943548 (age 7.890s)
> >    hex dump (first 32 bytes):
> >      38 10 0d 0f 81 88 ff ff 38 10 0d 0f 81 88 ff ff  8.......8.......
> >      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >    backtrace:
> >      [<000000009375ee42>] kmemleak_alloc_recursive
> > include/linux/kmemleak.h:55 [inline]
> >      [<000000009375ee42>] slab_post_alloc_hook mm/slab.h:439 [inline]
> >      [<000000009375ee42>] slab_alloc_node mm/slab.c:3269 [inline]
> >      [<000000009375ee42>] kmem_cache_alloc_node+0x153/0x2a0
> mm/slab.c:3579
> >      [<000000004c563922>] __alloc_skb+0x6e/0x210 net/core/skbuff.c:198
> >      [<00000000ec87bfa1>] alloc_skb_fclone include/linux/skbuff.h:1107
> > [inline]
> >      [<00000000ec87bfa1>] tipc_buf_acquire+0x2f/0x80 net/tipc/msg.c:66
> >      [<00000000d151ef84>] tipc_msg_create+0x37/0xe0 net/tipc/msg.c:98
> >      [<000000008bb437b0>] tipc_group_create_event+0xb3/0x1b0
> > net/tipc/group.c:679
> >      [<00000000947b1d0f>] tipc_group_proto_rcv+0x569/0x640
> > net/tipc/group.c:781
> >      [<00000000b75ab039>] tipc_sk_proto_rcv net/tipc/socket.c:1996
> [inline]
> >      [<00000000b75ab039>] tipc_sk_filter_rcv+0x9ac/0xf20
> > net/tipc/socket.c:2163
> >      [<000000000dab7a6c>] tipc_sk_enqueue net/tipc/socket.c:2255 [inline]
> >      [<000000000dab7a6c>] tipc_sk_rcv+0x494/0x8a0
> net/tipc/socket.c:2306
> >      [<00000000023a7ddd>] tipc_node_xmit+0x196/0x1f0
> net/tipc/node.c:1442
> >      [<00000000337dd9eb>] tipc_node_xmit_skb net/tipc/node.c:1491
> [inline]
> >      [<00000000337dd9eb>] tipc_node_distr_xmit+0x7d/0x120
> > net/tipc/node.c:1506
> >      [<00000000b6375182>] tipc_group_delete+0xe6/0x130
> net/tipc/group.c:224
> >      [<000000000361ba2b>] tipc_sk_leave+0x57/0xb0
> net/tipc/socket.c:2925
> >      [<000000009df90505>] tipc_release+0x7b/0x5e0 net/tipc/socket.c:584
> >      [<000000009f3189da>] __sock_release+0x4b/0xe0 net/socket.c:607
> >      [<00000000d3568ee0>] sock_close+0x1b/0x30 net/socket.c:1279
> >      [<00000000266a6215>] __fput+0xed/0x300 fs/file_table.c:280
> >
> >
> >
> > ---
> > This bug is generated by a bot. It may contain errors.
> > See https://protect2.fireeye.com/url?k=6424a141-38ae7457-6424e1da-
> 862f14a9365e-
> 29d28d4e37c9de97&q=1&u=https%3A%2F%2Fgoo.gl%2FtpsmEJ for more
> information about syzbot.
> > syzbot engineers can be reached at syz...@go....
> >
> > syzbot will keep track of this bug report. See:
> > https://protect2.fireeye.com/url?k=e3266fe7-bfacbaf1-e3262f7c-
> 862f14a9365e-
> 80a14dc097f61dfe&q=1&u=https%3A%2F%2Fgoo.gl%2FtpsmEJ%23status
> for how to communicate with syzbot.
> > syzbot can test patches for this bug, for details see:
> > https://protect2.fireeye.com/url?k=26e2f1e8-7a6824fe-26e2b173-
> 862f14a9
> > 365e-
> e569ee98f89625d4&q=1&u=https%3A%2F%2Fgoo.gl%2FtpsmEJ%23testing-
> pa
> > tches

Re: [tipc-discussion] memory leak in tipc_buf_acquire

[Xin L. <luc...@gm...>]

On Sat, May 25, 2019 at 5:18 AM syzbot
<syz...@sy...> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    4dde821e Merge tag 'xfs-5.2-fixes-1' of git://git.kernel.o..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=107db73aa00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=61dd9e15a761691d
> dashboard link: https://syzkaller.appspot.com/bug?extid=78fbe679c8ca8d264a8d
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=162bd84ca00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=160c605ca00000
>
Looks we need to purge each member's deferredq list in tipc_group_delete():
diff --git a/net/tipc/group.c b/net/tipc/group.c
index 992be61..23823eb 100644
--- a/net/tipc/group.c
+++ b/net/tipc/group.c
@@ -218,6 +218,7 @@ void tipc_group_delete(struct net *net, struct
tipc_group *grp)

  rbtree_postorder_for_each_entry_safe(m, tmp, tree, tree_node) {
  tipc_group_proto_xmit(grp, m, GRP_LEAVE_MSG, &xmitq);
+ __skb_queue_purge(&m->deferredq);
  list_del(&m->list);
  kfree(m);
  }

> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syz...@sy...
>
> type=1400 audit(1558701681.775:36): avc:  denied  { map } for  pid=7128
> comm="syz-executor987" path="/root/syz-executor987656147" dev="sda1"
> ino=15900 scontext=unconfined_u:system_r:insmod_t:s0-s0:c0.c1023
> tcontext=unconfined_u:object_r:user_home_t:s0 tclass=file permissive=1
> executing program
> executing program
> executing program
> executing program
> BUG: memory leak
> unreferenced object 0xffff88810df83c00 (size 512):
>    comm "softirq", pid 0, jiffies 4294942354 (age 19.830s)
>    hex dump (first 32 bytes):
>      38 1a 0d 0f 81 88 ff ff 38 1a 0d 0f 81 88 ff ff  8.......8.......
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<000000009375ee42>] kmemleak_alloc_recursive
> include/linux/kmemleak.h:55 [inline]
>      [<000000009375ee42>] slab_post_alloc_hook mm/slab.h:439 [inline]
>      [<000000009375ee42>] slab_alloc_node mm/slab.c:3269 [inline]
>      [<000000009375ee42>] kmem_cache_alloc_node+0x153/0x2a0 mm/slab.c:3579
>      [<000000004c563922>] __alloc_skb+0x6e/0x210 net/core/skbuff.c:198
>      [<00000000ec87bfa1>] alloc_skb_fclone include/linux/skbuff.h:1107
> [inline]
>      [<00000000ec87bfa1>] tipc_buf_acquire+0x2f/0x80 net/tipc/msg.c:66
>      [<00000000d151ef84>] tipc_msg_create+0x37/0xe0 net/tipc/msg.c:98
>      [<000000008bb437b0>] tipc_group_create_event+0xb3/0x1b0
> net/tipc/group.c:679
>      [<00000000947b1d0f>] tipc_group_proto_rcv+0x569/0x640
> net/tipc/group.c:781
>      [<00000000b75ab039>] tipc_sk_proto_rcv net/tipc/socket.c:1996 [inline]
>      [<00000000b75ab039>] tipc_sk_filter_rcv+0x9ac/0xf20
> net/tipc/socket.c:2163
>      [<000000000dab7a6c>] tipc_sk_enqueue net/tipc/socket.c:2255 [inline]
>      [<000000000dab7a6c>] tipc_sk_rcv+0x494/0x8a0 net/tipc/socket.c:2306
>      [<00000000023a7ddd>] tipc_node_xmit+0x196/0x1f0 net/tipc/node.c:1442
>      [<00000000337dd9eb>] tipc_node_xmit_skb net/tipc/node.c:1491 [inline]
>      [<00000000337dd9eb>] tipc_node_distr_xmit+0x7d/0x120
> net/tipc/node.c:1506
>      [<00000000b6375182>] tipc_group_delete+0xe6/0x130 net/tipc/group.c:224
>      [<000000000361ba2b>] tipc_sk_leave+0x57/0xb0 net/tipc/socket.c:2925
>      [<000000009df90505>] tipc_release+0x7b/0x5e0 net/tipc/socket.c:584
>      [<000000009f3189da>] __sock_release+0x4b/0xe0 net/socket.c:607
>      [<00000000d3568ee0>] sock_close+0x1b/0x30 net/socket.c:1279
>      [<00000000266a6215>] __fput+0xed/0x300 fs/file_table.c:280
>
> BUG: memory leak
> unreferenced object 0xffff888111895400 (size 1024):
>    comm "softirq", pid 0, jiffies 4294942354 (age 19.830s)
>    hex dump (first 32 bytes):
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<00000000e2e2855e>] kmemleak_alloc_recursive
> include/linux/kmemleak.h:55 [inline]
>      [<00000000e2e2855e>] slab_post_alloc_hook mm/slab.h:439 [inline]
>      [<00000000e2e2855e>] slab_alloc_node mm/slab.c:3269 [inline]
>      [<00000000e2e2855e>] kmem_cache_alloc_node_trace+0x15b/0x2a0
> mm/slab.c:3597
>      [<00000000a5030ce7>] __do_kmalloc_node mm/slab.c:3619 [inline]
>      [<00000000a5030ce7>] __kmalloc_node_track_caller+0x38/0x50
> mm/slab.c:3634
>      [<0000000039212451>] __kmalloc_reserve.isra.0+0x40/0xb0
> net/core/skbuff.c:142
>      [<00000000307cb4cf>] __alloc_skb+0xa0/0x210 net/core/skbuff.c:210
>      [<00000000ec87bfa1>] alloc_skb_fclone include/linux/skbuff.h:1107
> [inline]
>      [<00000000ec87bfa1>] tipc_buf_acquire+0x2f/0x80 net/tipc/msg.c:66
>      [<00000000d151ef84>] tipc_msg_create+0x37/0xe0 net/tipc/msg.c:98
>      [<000000008bb437b0>] tipc_group_create_event+0xb3/0x1b0
> net/tipc/group.c:679
>      [<00000000947b1d0f>] tipc_group_proto_rcv+0x569/0x640
> net/tipc/group.c:781
>      [<00000000b75ab039>] tipc_sk_proto_rcv net/tipc/socket.c:1996 [inline]
>      [<00000000b75ab039>] tipc_sk_filter_rcv+0x9ac/0xf20
> net/tipc/socket.c:2163
>      [<000000000dab7a6c>] tipc_sk_enqueue net/tipc/socket.c:2255 [inline]
>      [<000000000dab7a6c>] tipc_sk_rcv+0x494/0x8a0 net/tipc/socket.c:2306
>      [<00000000023a7ddd>] tipc_node_xmit+0x196/0x1f0 net/tipc/node.c:1442
>      [<00000000337dd9eb>] tipc_node_xmit_skb net/tipc/node.c:1491 [inline]
>      [<00000000337dd9eb>] tipc_node_distr_xmit+0x7d/0x120
> net/tipc/node.c:1506
>      [<00000000b6375182>] tipc_group_delete+0xe6/0x130 net/tipc/group.c:224
>      [<000000000361ba2b>] tipc_sk_leave+0x57/0xb0 net/tipc/socket.c:2925
>      [<000000009df90505>] tipc_release+0x7b/0x5e0 net/tipc/socket.c:584
>      [<000000009f3189da>] __sock_release+0x4b/0xe0 net/socket.c:607
>
> BUG: memory leak
> unreferenced object 0xffff88810e63de00 (size 512):
>    comm "softirq", pid 0, jiffies 4294943548 (age 7.890s)
>    hex dump (first 32 bytes):
>      38 10 0d 0f 81 88 ff ff 38 10 0d 0f 81 88 ff ff  8.......8.......
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<000000009375ee42>] kmemleak_alloc_recursive
> include/linux/kmemleak.h:55 [inline]
>      [<000000009375ee42>] slab_post_alloc_hook mm/slab.h:439 [inline]
>      [<000000009375ee42>] slab_alloc_node mm/slab.c:3269 [inline]
>      [<000000009375ee42>] kmem_cache_alloc_node+0x153/0x2a0 mm/slab.c:3579
>      [<000000004c563922>] __alloc_skb+0x6e/0x210 net/core/skbuff.c:198
>      [<00000000ec87bfa1>] alloc_skb_fclone include/linux/skbuff.h:1107
> [inline]
>      [<00000000ec87bfa1>] tipc_buf_acquire+0x2f/0x80 net/tipc/msg.c:66
>      [<00000000d151ef84>] tipc_msg_create+0x37/0xe0 net/tipc/msg.c:98
>      [<000000008bb437b0>] tipc_group_create_event+0xb3/0x1b0
> net/tipc/group.c:679
>      [<00000000947b1d0f>] tipc_group_proto_rcv+0x569/0x640
> net/tipc/group.c:781
>      [<00000000b75ab039>] tipc_sk_proto_rcv net/tipc/socket.c:1996 [inline]
>      [<00000000b75ab039>] tipc_sk_filter_rcv+0x9ac/0xf20
> net/tipc/socket.c:2163
>      [<000000000dab7a6c>] tipc_sk_enqueue net/tipc/socket.c:2255 [inline]
>      [<000000000dab7a6c>] tipc_sk_rcv+0x494/0x8a0 net/tipc/socket.c:2306
>      [<00000000023a7ddd>] tipc_node_xmit+0x196/0x1f0 net/tipc/node.c:1442
>      [<00000000337dd9eb>] tipc_node_xmit_skb net/tipc/node.c:1491 [inline]
>      [<00000000337dd9eb>] tipc_node_distr_xmit+0x7d/0x120
> net/tipc/node.c:1506
>      [<00000000b6375182>] tipc_group_delete+0xe6/0x130 net/tipc/group.c:224
>      [<000000000361ba2b>] tipc_sk_leave+0x57/0xb0 net/tipc/socket.c:2925
>      [<000000009df90505>] tipc_release+0x7b/0x5e0 net/tipc/socket.c:584
>      [<000000009f3189da>] __sock_release+0x4b/0xe0 net/socket.c:607
>      [<00000000d3568ee0>] sock_close+0x1b/0x30 net/socket.c:1279
>      [<00000000266a6215>] __fput+0xed/0x300 fs/file_table.c:280
>
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syz...@go....
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches

Re: [tipc-discussion] [PATCH RFC 0/2] tipc: link changeover issues

[Jon M. <jon...@er...>]

Both patches acked by me.

///jon


> -----Original Message-----
> From: Tuong Lien <tuo...@de...>
> Sent: 4-Jun-19 01:23
> To: tip...@li...; Jon Maloy
> <jon...@er...>; ma...@do...; yin...@wi...
> Subject: [PATCH RFC 0/2] tipc: link changeover issues
> 
> This patch series is to resolve some issues found with the current link
> changeover mechanism, it also includes an optimization for the link synching.
> 
> Tuong Lien (2):
>   tipc: optimize link synching mechanism
>   tipc: fix changeover issues due to large packet
> 
>  net/tipc/link.c | 118
> +++++++++++++++++++++++++++++++++++++++++++++++++-------
>  net/tipc/msg.c  |  62 +++++++++++++++++++++++++++++  net/tipc/msg.h  |
> 28 +++++++++++++-
>  net/tipc/node.c |   6 ++-
>  net/tipc/node.h |   6 ++-
>  5 files changed, 202 insertions(+), 18 deletions(-)
> 
> --
> 2.13.7

[tipc-discussion] [PATCH RFC 2/2] tipc: fix changeover issues due to large packet

[Tuong L. <tuo...@de...>]

In conjunction with changing the interfaces' MTU (e.g. especially in
the case of a bonding) where the TIPC links are brought up and down
in a short time, a couple of issues were detected with the current link
changeover mechanism:

1) When one link is up but immediately forced down again, the failover
procedure will be carried out in order to failover all the messages in
the link's transmq queue onto the other working link. The link and node
state is also set to FAILINGOVER as part of the process. The message
will be transmited in form of a FAILOVER_MSG, so its size is plus of 40
bytes (= the message header size). There is no problem if the original
message size is not larger than the link's MTU - 40, and indeed this is
the max size of a normal payload messages. However, in the situation
above, because the link has just been up, the messages in the link's
transmq are almost SYNCH_MSGs which had been generated by the link
synching procedure, then their size might reach the max value already!
When the FAILOVER_MSG is built on the top of such a SYNCH_MSG, its size
will exceed the link's MTU. As a result, the messages are dropped
silently and the failover procedure will never end up, the link will
not be able to exit the FAILINGOVER state, so cannot be re-established.

2) The same scenario above can happen more easily in case the MTU of
the links is set differently or when changing. In that case, as long as
a large message in the failure link's transmq queue was built and
fragmented with its link's MTU > the other link's one, the issue will
happen (there is no need of a link synching in advance).

3) The link synching procedure also faces with the same issue but since
the link synching is only started upon receipt of a SYNCH_MSG, dropping
the message will not result in a state deadlock, but it is not expected
as design.

The 1) & 3) issues are resolved by the previous commit 81e4dd94b214
("tipc: optimize link synching mechanism") by generating only a dummy
SYNCH_MSG (i.e. without data) at the link synching, so the size of a
FAILOVER_MSG if any then will never exceed the link's MTU.

For the 2) issue, the only solution is trying to fragment the messages
in the failure link's transmq queue according to the working link's MTU
so they can be failovered then. A new function is made to accomplish
this, it will still be a TUNNEL PROTOCOL/FAILOVER MSG but if the
original message size is too large, it will be fragmented & reassembled
at the receiving side.

Signed-off-by: Tuong Lien <tuo...@de...>
---
 net/tipc/link.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++--------
 net/tipc/msg.c  | 62 ++++++++++++++++++++++++++++++++++++++
 net/tipc/msg.h  | 18 ++++++++++-
 3 files changed, 158 insertions(+), 14 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 6924cf1e526f..3d2d2e4e4f14 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -182,6 +182,7 @@ struct tipc_link {
 
 	/* Fragmentation/reassembly */
 	struct sk_buff *reasm_buf;
+	struct sk_buff *reasm_tnlmsg;
 
 	/* Broadcast */
 	u16 ackers;
@@ -899,8 +900,10 @@ void tipc_link_reset(struct tipc_link *l)
 	l->backlog[TIPC_CRITICAL_IMPORTANCE].len = 0;
 	l->backlog[TIPC_SYSTEM_IMPORTANCE].len = 0;
 	kfree_skb(l->reasm_buf);
+	kfree_skb(l->reasm_tnlmsg);
 	kfree_skb(l->failover_reasm_skb);
 	l->reasm_buf = NULL;
+	l->reasm_tnlmsg = NULL;
 	l->failover_reasm_skb = NULL;
 	l->rcv_unacked = 0;
 	l->snd_nxt = 1;
@@ -943,6 +946,9 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
 	int rc = 0;
 
 	if (unlikely(msg_size(hdr) > mtu)) {
+		pr_warn("Purging list (len = %d), head msg <%d, %d>: %d\n",
+			skb_queue_len(list), msg_user(hdr),
+			msg_type(hdr), msg_size(hdr));
 		skb_queue_purge(list);
 		return -EMSGSIZE;
 	}
@@ -1212,6 +1218,7 @@ static int tipc_link_tnl_rcv(struct tipc_link *l, struct sk_buff *skb,
 			     struct sk_buff_head *inputq)
 {
 	struct sk_buff **reasm_skb = &l->failover_reasm_skb;
+	struct sk_buff **reasm_tnlmsg = &l->reasm_tnlmsg;
 	struct sk_buff_head *fdefq = &l->failover_deferdq;
 	struct tipc_msg *hdr = buf_msg(skb);
 	struct sk_buff *iskb;
@@ -1219,25 +1226,44 @@ static int tipc_link_tnl_rcv(struct tipc_link *l, struct sk_buff *skb,
 	int rc = 0;
 	u16 seqno;
 
-	/* SYNCH_MSG */
-	if (msg_type(hdr) == SYNCH_MSG)
-		goto drop;
+	if (msg_type(hdr) == SYNCH_MSG) {
+		kfree_skb(skb);
+		return 0;
+	}
 
-	/* FAILOVER_MSG */
-	if (!tipc_msg_extract(skb, &iskb, &ipos)) {
-		pr_warn_ratelimited("Cannot extract FAILOVER_MSG, defq: %d\n",
-				    skb_queue_len(fdefq));
-		return rc;
+	/* No fragmentation? */
+	if (likely(!msg_nof_fragms(hdr))) {
+		if (!tipc_msg_extract(skb, &iskb, &ipos)) {
+			pr_warn_ratelimited("Unable to extract msg, defq: %d\n",
+					    skb_queue_len(fdefq));
+			return 0;
+		}
+		kfree_skb(skb);
+	} else {
+		/* Set fragment type for buf_append */
+		if (msg_fragm_no(hdr) == 1)
+			msg_set_type(hdr, FIRST_FRAGMENT);
+		else if (msg_fragm_no(hdr) < msg_nof_fragms(hdr))
+			msg_set_type(hdr, FRAGMENT);
+		else
+			msg_set_type(hdr, LAST_FRAGMENT);
+
+		if (!tipc_buf_append(reasm_tnlmsg, &skb)) {
+			if (*reasm_tnlmsg || link_is_bc_rcvlink(l))
+				return 0;
+
+			pr_warn_ratelimited("Unable to reassemble tunnel msg\n");
+			return tipc_link_fsm_evt(l, LINK_FAILURE_EVT);
+		}
+		iskb = skb;
 	}
 
 	do {
 		seqno = buf_seqno(iskb);
-
 		if (unlikely(less(seqno, l->drop_point))) {
 			kfree_skb(iskb);
 			continue;
 		}
-
 		if (unlikely(seqno != l->drop_point)) {
 			__tipc_skb_queue_sorted(fdefq, seqno, iskb);
 			continue;
@@ -1251,8 +1277,6 @@ static int tipc_link_tnl_rcv(struct tipc_link *l, struct sk_buff *skb,
 			break;
 	} while ((iskb = __tipc_skb_dequeue(fdefq, l->drop_point)));
 
-drop:
-	kfree_skb(skb);
 	return rc;
 }
 
@@ -1635,15 +1659,18 @@ void tipc_link_tnl_prepare(struct tipc_link *l, struct tipc_link *tnl,
 	struct sk_buff *skb, *tnlskb;
 	struct tipc_msg *hdr, tnlhdr;
 	struct sk_buff_head *queue = &l->transmq;
-	struct sk_buff_head tmpxq, tnlq;
+	struct sk_buff_head tmpxq, tnlq, frags;
 	u16 pktlen, pktcnt, seqno = l->snd_nxt;
+	bool pktcnt_need_update = false;
 	u16 syncpt;
+	int rc;
 
 	if (!tnl)
 		return;
 
 	skb_queue_head_init(&tnlq);
 	skb_queue_head_init(&tmpxq);
+	skb_queue_head_init(&frags);
 
 	/* At least one packet required for safe algorithm => add dummy */
 	skb = tipc_msg_create(TIPC_LOW_IMPORTANCE, TIPC_DIRECT_MSG,
@@ -1699,6 +1726,39 @@ void tipc_link_tnl_prepare(struct tipc_link *l, struct tipc_link *tnl,
 		if (queue == &l->backlogq)
 			msg_set_seqno(hdr, seqno++);
 		pktlen = msg_size(hdr);
+
+		/* Tunnel link MTU is not large enough? This could be
+		 * due to:
+		 * 1) Link MTU has just changed or set differently;
+		 * 2) Or FAILOVER on the top of a SYNCH message
+		 *
+		 * The 2nd case should not happen if peer supports
+		 * TIPC_TUNNEL_ENHANCED
+		 */
+		if (pktlen > tnl->mtu - INT_H_SIZE) {
+			if (mtyp == FAILOVER_MSG &&
+			    (tnl->peer_caps & TIPC_TUNNEL_ENHANCED)) {
+				rc = tipc_msg_fragment(skb, &tnlhdr, tnl->mtu,
+						       &frags);
+				if (rc) {
+					pr_warn("%sunable to frag msg: rc %d\n",
+						link_co_err, rc);
+					return;
+				}
+				pktcnt += skb_queue_len(&frags) - 1;
+				pktcnt_need_update = true;
+				skb_queue_splice_tail_init(&frags, &tnlq);
+				continue;
+			}
+			/* Unluckily, peer doesn't have TIPC_TUNNEL_ENHANCED
+			 * => Just warn it and return?
+			 */
+			pr_warn_ratelimited("%stoo large msg <%d, %d>: %d!\n",
+					    link_co_err, msg_user(hdr),
+					    msg_type(hdr), msg_size(hdr));
+			return;
+		}
+
 		msg_set_size(&tnlhdr, pktlen + INT_H_SIZE);
 		tnlskb = tipc_buf_acquire(pktlen + INT_H_SIZE, GFP_ATOMIC);
 		if (!tnlskb) {
@@ -1714,6 +1774,12 @@ void tipc_link_tnl_prepare(struct tipc_link *l, struct tipc_link *tnl,
 		goto tnl;
 	}
 
+	if (pktcnt_need_update)
+		skb_queue_walk(&tnlq, skb) {
+			hdr = buf_msg(skb);
+			msg_set_msgcnt(hdr, pktcnt);
+		}
+
 	tipc_link_xmit(tnl, &tnlq, xmitq);
 
 	if (mtyp == FAILOVER_MSG) {
diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index f48e5857210f..dfa6eeb40013 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -244,6 +244,68 @@ bool tipc_msg_validate(struct sk_buff **_skb)
 }
 
 /**
+ * tipc_msg_fragment - build a fragment skb list for TIPC message
+ *
+ * @skb: TIPC message skb
+ * @hdr: internal msg header to be put on the top of the fragments
+ * @pktmax: max size of a fragment incl. the header
+ * @frags: returned fragment skb list
+ *
+ * Returns 0 if the fragmentation is successful, otherwise: -EINVAL
+ * or -ENOMEM
+ */
+int tipc_msg_fragment(struct sk_buff *skb, const struct tipc_msg *hdr,
+		      int pktmax, struct sk_buff_head *frags)
+{
+	int pktno, nof_fragms, dsz, dmax, eat;
+	struct tipc_msg *_hdr;
+	struct sk_buff *_skb;
+	u8 *data;
+
+	/* Non-linear buffer? */
+	if (skb_linearize(skb))
+		return -ENOMEM;
+
+	data = (u8 *)skb->data;
+	dsz = msg_size(buf_msg(skb));
+	dmax = pktmax - INT_H_SIZE;
+
+	if (dsz <= dmax || !dmax)
+		return -EINVAL;
+
+	nof_fragms = dsz / dmax + 1;
+
+	for (pktno = 1; pktno <= nof_fragms; pktno++) {
+		if (pktno < nof_fragms)
+			eat = dmax;
+		else
+			eat = dsz % dmax;
+
+		_skb = tipc_buf_acquire(INT_H_SIZE + eat, GFP_ATOMIC);
+		if (!_skb)
+			goto error;
+
+		skb_orphan(_skb);
+		__skb_queue_tail(frags, _skb);
+
+		skb_copy_to_linear_data(_skb, hdr, INT_H_SIZE);
+		skb_copy_to_linear_data_offset(_skb, INT_H_SIZE, data, eat);
+		data += eat;
+
+		_hdr = buf_msg(_skb);
+		msg_set_fragm_no(_hdr, pktno);
+		msg_set_nof_fragms(_hdr, nof_fragms);
+		msg_set_size(_hdr, INT_H_SIZE + eat);
+	}
+	return 0;
+
+error:
+	__skb_queue_purge(frags);
+	__skb_queue_head_init(frags);
+	return -ENOMEM;
+}
+
+/**
  * tipc_msg_build - create buffer chain containing specified header and data
  * @mhdr: Message header, to be prepended to data
  * @m: User message
diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index baf937bfa702..fbfdf7843d07 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -721,12 +721,26 @@ static inline void msg_set_last_bcast(struct tipc_msg *m, u32 n)
 	msg_set_bits(m, 4, 16, 0xffff, n);
 }
 
+static inline u32 msg_nof_fragms(struct tipc_msg *m)
+{
+	return msg_bits(m, 4, 0, 0xffff);
+}
+
+static inline void msg_set_nof_fragms(struct tipc_msg *m, u32 n)
+{
+	msg_set_bits(m, 4, 0, 0xffff, n);
+}
+
+static inline u32 msg_fragm_no(struct tipc_msg *m)
+{
+	return msg_bits(m, 4, 16, 0xffff);
+}
+
 static inline void msg_set_fragm_no(struct tipc_msg *m, u32 n)
 {
 	msg_set_bits(m, 4, 16, 0xffff, n);
 }
 
-
 static inline u16 msg_next_sent(struct tipc_msg *m)
 {
 	return msg_bits(m, 4, 0, 0xffff);
@@ -1045,6 +1059,8 @@ bool tipc_msg_bundle(struct sk_buff *skb, struct tipc_msg *msg, u32 mtu);
 bool tipc_msg_make_bundle(struct sk_buff **skb, struct tipc_msg *msg,
 			  u32 mtu, u32 dnode);
 bool tipc_msg_extract(struct sk_buff *skb, struct sk_buff **iskb, int *pos);
+int tipc_msg_fragment(struct sk_buff *skb, const struct tipc_msg *hdr,
+		      int pktmax, struct sk_buff_head *frags);
 int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m,
 		   int offset, int dsz, int mtu, struct sk_buff_head *list);
 bool tipc_msg_lookup_dest(struct net *net, struct sk_buff *skb, int *err);
-- 
2.13.7

[tipc-discussion] [PATCH RFC 1/2] tipc: optimize link synching mechanism

[Tuong L. <tuo...@de...>]

This commit is along with the latter commit 4ec6a14c3933 ("tipc: fix
changeover issues due to large packet") to resolve the issues with the
link changeover mechanism. See that commit for details.

Basically, for the link synching, from now on, we will send only one
single ("dummy") SYNCH message to peer. The SYNCH message does not
contain any data, just a header conveying the synch point to the peer.

A new node capability flag ("TIPC_TUNNEL_ENHANCED") is introduced for
backward compatible!

Suggested-by: Jon Maloy <jon...@er...>
Signed-off-by: Tuong Lien <tuo...@de...>
---
 net/tipc/link.c | 26 ++++++++++++++++++++++++++
 net/tipc/msg.h  | 10 ++++++++++
 net/tipc/node.c |  6 ++++--
 net/tipc/node.h |  6 ++++--
 4 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index f5cd986e1e50..6924cf1e526f 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1637,6 +1637,7 @@ void tipc_link_tnl_prepare(struct tipc_link *l, struct tipc_link *tnl,
 	struct sk_buff_head *queue = &l->transmq;
 	struct sk_buff_head tmpxq, tnlq;
 	u16 pktlen, pktcnt, seqno = l->snd_nxt;
+	u16 syncpt;
 
 	if (!tnl)
 		return;
@@ -1656,6 +1657,31 @@ void tipc_link_tnl_prepare(struct tipc_link *l, struct tipc_link *tnl,
 	tipc_link_xmit(l, &tnlq, &tmpxq);
 	__skb_queue_purge(&tmpxq);
 
+	/* Link Synching:
+	 * From now on, send only one single ("dummy") SYNCH message
+	 * to peer. The SYNCH message does not contain any data, just
+	 * a header conveying the synch point to the peer.
+	 */
+	if (mtyp == SYNCH_MSG && (tnl->peer_caps & TIPC_TUNNEL_ENHANCED)) {
+		tnlskb = tipc_msg_create(TUNNEL_PROTOCOL, SYNCH_MSG,
+					 INT_H_SIZE, 0, l->addr,
+					 tipc_own_addr(l->net),
+					 0, 0, 0);
+		if (!tnlskb) {
+			pr_warn("%sunable to create dummy SYNCH_MSG\n",
+				link_co_err);
+			return;
+		}
+
+		hdr = buf_msg(tnlskb);
+		syncpt = l->snd_nxt + skb_queue_len(&l->backlogq) - 1;
+		msg_set_syncpt(hdr, syncpt);
+		msg_set_bearer_id(hdr, l->peer_bearer_id);
+		__skb_queue_tail(&tnlq, tnlskb);
+		tipc_link_xmit(tnl, &tnlq, xmitq);
+		return;
+	}
+
 	/* Initialize reusable tunnel packet header */
 	tipc_msg_init(tipc_own_addr(l->net), &tnlhdr, TUNNEL_PROTOCOL,
 		      mtyp, INT_H_SIZE, l->addr);
diff --git a/net/tipc/msg.h b/net/tipc/msg.h
index 8de02ad6e352..baf937bfa702 100644
--- a/net/tipc/msg.h
+++ b/net/tipc/msg.h
@@ -877,6 +877,16 @@ static inline void msg_set_msgcnt(struct tipc_msg *m, u16 n)
 	msg_set_bits(m, 9, 16, 0xffff, n);
 }
 
+static inline u16 msg_syncpt(struct tipc_msg *m)
+{
+	return msg_bits(m, 9, 16, 0xffff);
+}
+
+static inline void msg_set_syncpt(struct tipc_msg *m, u16 n)
+{
+	msg_set_bits(m, 9, 16, 0xffff, n);
+}
+
 static inline u32 msg_conn_ack(struct tipc_msg *m)
 {
 	return msg_bits(m, 9, 16, 0xffff);
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 9e106d3ed187..2a8399cf5525 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1645,7 +1645,6 @@ static bool tipc_node_check_state(struct tipc_node *n, struct sk_buff *skb,
 	int usr = msg_user(hdr);
 	int mtyp = msg_type(hdr);
 	u16 oseqno = msg_seqno(hdr);
-	u16 iseqno = msg_seqno(msg_get_wrapped(hdr));
 	u16 exp_pkts = msg_msgcnt(hdr);
 	u16 rcv_nxt, syncpt, dlv_nxt, inputq_len;
 	int state = n->state;
@@ -1744,7 +1743,10 @@ static bool tipc_node_check_state(struct tipc_node *n, struct sk_buff *skb,
 
 	/* Initiate synch mode if applicable */
 	if ((usr == TUNNEL_PROTOCOL) && (mtyp == SYNCH_MSG) && (oseqno == 1)) {
-		syncpt = iseqno + exp_pkts - 1;
+		if (n->capabilities & TIPC_TUNNEL_ENHANCED)
+			syncpt = msg_syncpt(hdr);
+		else
+			syncpt = msg_seqno(msg_get_wrapped(hdr)) + exp_pkts - 1;
 		if (!tipc_link_is_up(l))
 			__tipc_node_link_up(n, bearer_id, xmitq);
 		if (n->state == SELF_UP_PEER_UP) {
diff --git a/net/tipc/node.h b/net/tipc/node.h
index c0bf49ea3de4..291d0ecd4101 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -53,7 +53,8 @@ enum {
 	TIPC_NODE_ID128       = (1 << 5),
 	TIPC_LINK_PROTO_SEQNO = (1 << 6),
 	TIPC_MCAST_RBCTL      = (1 << 7),
-	TIPC_GAP_ACK_BLOCK    = (1 << 8)
+	TIPC_GAP_ACK_BLOCK    = (1 << 8),
+	TIPC_TUNNEL_ENHANCED  = (1 << 9)
 };
 
 #define TIPC_NODE_CAPABILITIES (TIPC_SYN_BIT           |  \
@@ -64,7 +65,8 @@ enum {
 				TIPC_NODE_ID128        |   \
 				TIPC_LINK_PROTO_SEQNO  |   \
 				TIPC_MCAST_RBCTL       |   \
-				TIPC_GAP_ACK_BLOCK)
+				TIPC_GAP_ACK_BLOCK     |   \
+				TIPC_TUNNEL_ENHANCED)
 #define INVALID_BEARER_ID -1
 
 void tipc_node_stop(struct net *net);
-- 
2.13.7

[tipc-discussion] [PATCH RFC 0/2] tipc: link changeover issues

[Tuong L. <tuo...@de...>]

This patch series is to resolve some issues found with the current link
changeover mechanism, it also includes an optimization for the link
synching.

Tuong Lien (2):
  tipc: optimize link synching mechanism
  tipc: fix changeover issues due to large packet

 net/tipc/link.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++-------
 net/tipc/msg.c  |  62 +++++++++++++++++++++++++++++
 net/tipc/msg.h  |  28 +++++++++++++-
 net/tipc/node.c |   6 ++-
 net/tipc/node.h |   6 ++-
 5 files changed, 202 insertions(+), 18 deletions(-)

-- 
2.13.7

Re: [tipc-discussion] [PATCH v2] tipc: Avoid copying bytes beyond the supplied data

[Ying X. <yin...@wi...>]

On 5/23/19 4:46 AM, Chris Packham wrote:
> On most distros that is generated from include/uapi in the kernel source 
> and packaged as part of libc or a kernel-headers package. So once this 
> patch is accepted and makes it into the distros 
> /usr/include/linux/tipc_config.h will have this fix.

Thanks for the clarification. You are right, so it's unnecessary to make
any change.

Re: [tipc-discussion] WARNING: locking bug in rhashtable_walk_enter

[Jon M. <jon...@er...>]

> -----Original Message-----
> From: net...@vg... <net...@vg...>
> On Behalf Of syzbot
> Sent: 21-May-19 17:08
> To: da...@da...; huj...@hu...; Jon Maloy
> <jon...@er...>; lin...@vg...;
> ne...@vg...; syz...@go...; tipc-
> dis...@li...; wan...@hu...;
> yin...@wi...
> Subject: Re: WARNING: locking bug in rhashtable_walk_enter
> 
> syzbot has bisected this bug to:
> 
> commit 7e27e8d6130c5e88fac9ddec4249f7f2337fe7f8
> Author: Junwei Hu <huj...@hu...>
> Date:   Thu May 16 02:51:15 2019 +0000
> 
>      tipc: switch order of device registration to fix a crash

This bug was fixed by:
commit 526f5b851a96 (" tipc: fix modprobe tipc failed after switch order of device registration")

///jon

> 
> bisection log:  https://protect2.fireeye.com/url?k=c4fa0d6e-982e08d1-
> c4fa4df5-868f633dbf25-
> 265eeab89c3c62e3&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2
> Fbisect.txt%3Fx%3D1285d39ca00000
> start commit:   f49aa1de Merge tag 'for-5.2-rc1-tag' of git://git.kernel.o..
> git tree:       upstream
> final crash:    https://protect2.fireeye.com/url?k=f892c3bf-a446c600-
> f8928324-868f633dbf25-
> a1f259f41c2e3c70&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2F
> report.txt%3Fx%3D1185d39ca00000
> console output: https://protect2.fireeye.com/url?k=a265e997-feb1ec28-
> a265a90c-868f633dbf25-
> 3536c18414f0985d&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2
> Flog.txt%3Fx%3D1685d39ca00000
> kernel config:  https://protect2.fireeye.com/url?k=e063d9ce-bcb7dc71-
> e0639955-868f633dbf25-
> 34fb69deb3d9bb60&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%
> 2F.config%3Fx%3Dfc045131472947d7
> dashboard link: https://protect2.fireeye.com/url?k=b1ac8435-ed78818a-
> b1acc4ae-868f633dbf25-
> 92f95bff1ae4b05d&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fbug
> %3Fextid%3D6440134c13554d3abfb0
> syz repro:      https://protect2.fireeye.com/url?k=0473f5e5-58a7f05a-
> 0473b57e-868f633dbf25-
> 3d8da37dd852ac3b&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2
> Frepro.syz%3Fx%3D10c586bca00000
> C reproducer:   https://protect2.fireeye.com/url?k=0797bac5-5b43bf7a-
> 0797fa5e-868f633dbf25-
> ba5da04a9604be86&q=1&u=https%3A%2F%2Fsyzkaller.appspot.com%2Fx%2
> Frepro.c%3Fx%3D14759fb2a00000
> 
> Reported-by: syz...@sy...
> Fixes: 7e27e8d6130c ("tipc: switch order of device registration to fix a
> crash")
> 
> For information about bisection process see:
> https://protect2.fireeye.com/url?k=dd8443d3-8150466c-dd840348-
> 868f633dbf25-
> e8766eb633d4e43d&q=1&u=https%3A%2F%2Fgoo.gl%2FtpsmEJ%23bisectio
> n

Re: [tipc-discussion] [PATCH v2] tipc: Avoid copying bytes beyond the supplied data

[Ying X. <yin...@wi...>]

On 5/20/19 11:45 AM, Chris Packham wrote:
> TLV_SET is called with a data pointer and a len parameter that tells us
> how many bytes are pointed to by data. When invoking memcpy() we need
> to careful to only copy len bytes.
> 
> Previously we would copy TLV_LENGTH(len) bytes which would copy an extra
> 4 bytes past the end of the data pointer which newer GCC versions
> complain about.
> 
>  In file included from test.c:17:
>  In function 'TLV_SET',
>      inlined from 'test' at test.c:186:5:
>  /usr/include/linux/tipc_config.h:317:3:
>  warning: 'memcpy' forming offset [33, 36] is out of the bounds [0, 32]
>  of object 'bearer_name' with type 'char[32]' [-Warray-bounds]
>      memcpy(TLV_DATA(tlv_ptr), data, tlv_len);
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  test.c: In function 'test':
>  test.c::161:10: note:
>  'bearer_name' declared here
>      char bearer_name[TIPC_MAX_BEARER_NAME];
>           ^~~~~~~~~~~
> 
> We still want to ensure any padding bytes at the end are initialised, do
> this with a explicit memset() rather than copy bytes past the end of
> data. Apply the same logic to TCM_SET.
> 
> Signed-off-by: Chris Packham <chr...@al...>

Acked-by: Ying Xue <yin...@wi...>


But please make the same changes in usr/include/linux/tipc_config.h

> ---
> 
> Changes in v2:
> - Ensure padding bytes are initialised in both TLV_SET and TCM_SET
> 
>  include/uapi/linux/tipc_config.h | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/tipc_config.h b/include/uapi/linux/tipc_config.h
> index 4b2c93b1934c..4955e1a9f1bc 100644
> --- a/include/uapi/linux/tipc_config.h
> +++ b/include/uapi/linux/tipc_config.h
> @@ -307,8 +307,10 @@ static inline int TLV_SET(void *tlv, __u16 type, void *data, __u16 len)
>  	tlv_ptr = (struct tlv_desc *)tlv;
>  	tlv_ptr->tlv_type = htons(type);
>  	tlv_ptr->tlv_len  = htons(tlv_len);
> -	if (len && data)
> -		memcpy(TLV_DATA(tlv_ptr), data, tlv_len);
> +	if (len && data) {
> +		memcpy(TLV_DATA(tlv_ptr), data, len);
> +		memset(TLV_DATA(tlv_ptr) + len, 0, TLV_SPACE(len) - tlv_len);
> +	}
>  	return TLV_SPACE(len);
>  }
>  
> @@ -405,8 +407,10 @@ static inline int TCM_SET(void *msg, __u16 cmd, __u16 flags,
>  	tcm_hdr->tcm_len   = htonl(msg_len);
>  	tcm_hdr->tcm_type  = htons(cmd);
>  	tcm_hdr->tcm_flags = htons(flags);
> -	if (data_len && data)
> +	if (data_len && data) {
>  		memcpy(TCM_DATA(msg), data, data_len);
> +		memset(TCM_DATA(msg) + data_len, 0, TCM_SPACE(data_len) - msg_len);
> +	}
>  	return TCM_SPACE(data_len);
>  }
>  
> 

Re: [tipc-discussion] [PATCH v4] tipc: fix modprobe tipc failed after switch order of device registration

[Jon M. <jon...@er...>]

Reviewed and tested. I found no problems with this.

Reviewed-by: Jon Maloy <jon...@er...>


> -----Original Message-----
> From: net...@vg... <net...@vg...>
> On Behalf Of David Miller
> Sent: 20-May-19 13:46
> To: huj...@hu...
> Cc: Jon Maloy <jon...@er...>; yin...@wi...;
> wil...@gm...; sf...@ca...;
> ne...@vg...; tip...@li...; linux-
> ke...@vg...; min...@hu...
> Subject: Re: [PATCH v4] tipc: fix modprobe tipc failed after switch order of
> device registration
> 
> From: hujunwei <huj...@hu...>
> Date: Mon, 20 May 2019 14:43:59 +0800
> 
> > From: Junwei Hu <huj...@hu...>
> >
> > Error message printed:
> > modprobe: ERROR: could not insert 'tipc': Address family not supported
> > by protocol.
> > when modprobe tipc after the following patch: switch order of device
> > registration, commit 7e27e8d6130c
> > ("tipc: switch order of device registration to fix a crash")
> >
> > Because sock_create_kern(net, AF_TIPC, ...) called by
> > tipc_topsrv_create_listener() in the initialization process of
> > tipc_init_net(), so tipc_socket_init() must be execute before that.
> > Meanwhile, tipc_net_id need to be initialized when sock_create()
> > called, and tipc_socket_init() is no need to be called for each namespace.
> >
> > I add a variable tipc_topsrv_net_ops, and split the
> > register_pernet_subsys() of tipc into two parts, and split
> > tipc_socket_init() with initialization of pernet params.
> >
> > By the way, I fixed resources rollback error when tipc_bcast_init()
> > failed in tipc_init_net().
> >
> > Fixes: 7e27e8d6130c ("tipc: switch order of device registration to fix
> > a crash")
> > Signed-off-by: Junwei Hu <huj...@hu...>
> > Reported-by: Wang Wang <wan...@hu...>
> > Reported-by: syz...@sy...
> > Reviewed-by: Kang Zhou <zho...@hu...>
> > Reviewed-by: Suanming Mou <mou...@hu...>
> 
> Applied.

Re: [tipc-discussion] [PATCH v4] tipc: fix modprobe tipc failed after switch order of device registration

[David M. <da...@da...>]

From: hujunwei <huj...@hu...>
Date: Mon, 20 May 2019 14:43:59 +0800

> From: Junwei Hu <huj...@hu...>
> 
> Error message printed:
> modprobe: ERROR: could not insert 'tipc': Address family not
> supported by protocol.
> when modprobe tipc after the following patch: switch order of
> device registration, commit 7e27e8d6130c
> ("tipc: switch order of device registration to fix a crash")
> 
> Because sock_create_kern(net, AF_TIPC, ...) called by
> tipc_topsrv_create_listener() in the initialization process
> of tipc_init_net(), so tipc_socket_init() must be execute before that.
> Meanwhile, tipc_net_id need to be initialized when sock_create()
> called, and tipc_socket_init() is no need to be called for each namespace.
> 
> I add a variable tipc_topsrv_net_ops, and split the
> register_pernet_subsys() of tipc into two parts, and split
> tipc_socket_init() with initialization of pernet params.
> 
> By the way, I fixed resources rollback error when tipc_bcast_init()
> failed in tipc_init_net().
> 
> Fixes: 7e27e8d6130c ("tipc: switch order of device registration to fix a crash")
> Signed-off-by: Junwei Hu <huj...@hu...>
> Reported-by: Wang Wang <wan...@hu...>
> Reported-by: syz...@sy...
> Reviewed-by: Kang Zhou <zho...@hu...>
> Reviewed-by: Suanming Mou <mou...@hu...>

Applied.

Re: [tipc-discussion] [PATCH] tipc: fix modprobe tipc failed after switch order of device registration

[David M. <da...@da...>]

From: hujunwei <huj...@hu...>
Date: Fri, 17 May 2019 19:27:34 +0800

> From: Junwei Hu <huj...@hu...>
> 
> Error message printed:
> modprobe: ERROR: could not insert 'tipc': Address family not
> supported by protocol.
> when modprobe tipc after the following patch: switch order of
> device registration, commit 7e27e8d6130c
> ("tipc: switch order of device registration to fix a crash")
> 
> Because sock_create_kern(net, AF_TIPC, ...) is called by
> tipc_topsrv_create_listener() in the initialization process
> of tipc_net_ops, tipc_socket_init() must be execute before that.
> 
> I move tipc_socket_init() into function tipc_init_net().
> 
> Fixes: 7e27e8d6130c
> ("tipc: switch order of device registration to fix a crash")
> Signed-off-by: Junwei Hu <huj...@hu...>
> Reported-by: Wang Wang <wan...@hu...>
> Reviewed-by: Kang Zhou <zho...@hu...>
> Reviewed-by: Suanming Mou <mou...@hu...>

Applied.

Re: [tipc-discussion] [PATCH] tipc: switch order of device registration to fix a crash

[David M. <da...@da...>]

From: hujunwei <huj...@hu...>
Date: Thu, 16 May 2019 10:51:15 +0800

> From: Junwei Hu <huj...@hu...>
> 
> When tipc is loaded while many processes try to create a TIPC socket,
> a crash occurs:
>  PANIC: Unable to handle kernel paging request at virtual
>  address "dfff20000000021d"
>  pc : tipc_sk_create+0x374/0x1180 [tipc]
>  lr : tipc_sk_create+0x374/0x1180 [tipc]
>    Exception class = DABT (current EL), IL = 32 bits
>  Call trace:
>   tipc_sk_create+0x374/0x1180 [tipc]
>   __sock_create+0x1cc/0x408
>   __sys_socket+0xec/0x1f0
>   __arm64_sys_socket+0x74/0xa8
>  ...
> 
> This is due to race between sock_create and unfinished
> register_pernet_device. tipc_sk_insert tries to do
> "net_generic(net, tipc_net_id)".
> but tipc_net_id is not initialized yet.
> 
> So switch the order of the two to close the race.
> 
> This can be reproduced with multiple processes doing socket(AF_TIPC, ...)
> and one process doing module removal.
> 
> Fixes: a62fbccecd62 ("tipc: make subscriber server support net namespace")
> Signed-off-by: Junwei Hu <huj...@hu...>
> Reported-by: Wang Wang <wan...@hu...>
> Reviewed-by: Xiaogang Wang <wan...@hu...>

Applied and queued up for -stable.

Re: [tipc-discussion] [PATCH net v1] tipc: fix hanging clients using poll with EPOLLOUT flag

[David M. <da...@da...>]

From: Parthasarathy Bhuvaragan <par...@gm...>
Date: Thu,  9 May 2019 07:13:42 +0200

> commit 517d7c79bdb398 ("tipc: fix hanging poll() for stream sockets")
> introduced a regression for clients using non-blocking sockets.
> After the commit, we send EPOLLOUT event to the client even in
> TIPC_CONNECTING state. This causes the subsequent send() to fail
> with ENOTCONN, as the socket is still not in TIPC_ESTABLISHED state.
> 
> In this commit, we:
> - improve the fix for hanging poll() by replacing sk_data_ready()
>   with sk_state_change() to wake up all clients.
> - revert the faulty updates introduced by commit 517d7c79bdb398
>   ("tipc: fix hanging poll() for stream sockets").
> 
> Fixes: 517d7c79bdb398 ("tipc: fix hanging poll() for stream sockets")
> Signed-off-by: Parthasarathy Bhuvaragan <par...@gm...>
> Acked-by: Jon Maloy <jon...@er...>

Applied and queued up for -stable.

[tipc-discussion] [PATCH net v1] tipc: fix hanging clients using poll with EPOLLOUT flag

[Parthasarathy B. <par...@gm...>]

commit 517d7c79bdb398 ("tipc: fix hanging poll() for stream sockets")
introduced a regression for clients using non-blocking sockets.
After the commit, we send EPOLLOUT event to the client even in
TIPC_CONNECTING state. This causes the subsequent send() to fail
with ENOTCONN, as the socket is still not in TIPC_ESTABLISHED state.

In this commit, we:
- improve the fix for hanging poll() by replacing sk_data_ready()
  with sk_state_change() to wake up all clients.
- revert the faulty updates introduced by commit 517d7c79bdb398
  ("tipc: fix hanging poll() for stream sockets").

Fixes: 517d7c79bdb398 ("tipc: fix hanging poll() for stream sockets")
Signed-off-by: Parthasarathy Bhuvaragan <par...@gm...>
Acked-by: Jon Maloy <jon...@er...>
---
 net/tipc/socket.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index b542f14ed444..2851937f6e32 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -734,11 +734,11 @@ static __poll_t tipc_poll(struct file *file, struct socket *sock,
 
 	switch (sk->sk_state) {
 	case TIPC_ESTABLISHED:
-	case TIPC_CONNECTING:
 		if (!tsk->cong_link_cnt && !tsk_conn_cong(tsk))
 			revents |= EPOLLOUT;
 		/* fall through */
 	case TIPC_LISTEN:
+	case TIPC_CONNECTING:
 		if (!skb_queue_empty(&sk->sk_receive_queue))
 			revents |= EPOLLIN | EPOLLRDNORM;
 		break;
@@ -2041,7 +2041,7 @@ static bool tipc_sk_filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
 			if (msg_data_sz(hdr))
 				return true;
 			/* Empty ACK-, - wake up sleeping connect() and drop */
-			sk->sk_data_ready(sk);
+			sk->sk_state_change(sk);
 			msg_set_dest_droppable(hdr, 1);
 			return false;
 		}
-- 
2.21.0

Re: [tipc-discussion] [PATCH net v1] tipc: fix hanging clients using poll with EPOLLOUT flag

[Jon M. <jon...@er...>]

Acked-by: Jon Maloy <jon...@er...>

Thanks, Partha.
///jon

> -----Original Message-----
> From: Parthasarathy Bhuvaragan <par...@gm...>
> Sent: 8-May-19 04:52
> To: tip...@li...; Jon Maloy
> <jon...@er...>; yin...@wi...
> Cc: Parthasarathy Bhuvaragan <par...@gm...>
> Subject: [PATCH net v1] tipc: fix hanging clients using poll with EPOLLOUT flag
> 
> commit 517d7c79bdb398 ("tipc: fix hanging poll() for stream sockets")
> introduced a regression for clients using non-blocking sockets.
> After the commit, we send EPOLLOUT event to the client even in
> TIPC_CONNECTING state. This causes the subsequent send() to fail with
> ENOTCONN, as the socket is still not in TIPC_ESTABLISHED state.
> 
> In this commit, we:
> - improve the fix for hanging poll() by replacing sk_data_ready()
>   with sk_state_change() to wake up all clients.
> - revert the faulty updates introduced by commit 517d7c79bdb398
>   ("tipc: fix hanging poll() for stream sockets").
> 
> Fixes: 517d7c79bdb398 ("tipc: fix hanging poll() for stream sockets")
> Signed-off-by: Parthasarathy Bhuvaragan
> <par...@gm...>
> ---
>  net/tipc/socket.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c index
> b542f14ed444..2851937f6e32 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -734,11 +734,11 @@ static __poll_t tipc_poll(struct file *file, struct
> socket *sock,
> 
>  	switch (sk->sk_state) {
>  	case TIPC_ESTABLISHED:
> -	case TIPC_CONNECTING:
>  		if (!tsk->cong_link_cnt && !tsk_conn_cong(tsk))
>  			revents |= EPOLLOUT;
>  		/* fall through */
>  	case TIPC_LISTEN:
> +	case TIPC_CONNECTING:
>  		if (!skb_queue_empty(&sk->sk_receive_queue))
>  			revents |= EPOLLIN | EPOLLRDNORM;
>  		break;
> @@ -2041,7 +2041,7 @@ static bool tipc_sk_filter_connect(struct tipc_sock
> *tsk, struct sk_buff *skb)
>  			if (msg_data_sz(hdr))
>  				return true;
>  			/* Empty ACK-, - wake up sleeping connect() and
> drop */
> -			sk->sk_data_ready(sk);
> +			sk->sk_state_change(sk);
>  			msg_set_dest_droppable(hdr, 1);
>  			return false;
>  		}
> --
> 2.21.0

[tipc-discussion] [PATCH net v1] tipc: fix hanging clients using poll with EPOLLOUT flag

[Parthasarathy B. <par...@gm...>]

commit 517d7c79bdb398 ("tipc: fix hanging poll() for stream sockets")
introduced a regression for clients using non-blocking sockets.
After the commit, we send EPOLLOUT event to the client even in
TIPC_CONNECTING state. This causes the subsequent send() to fail
with ENOTCONN, as the socket is still not in TIPC_ESTABLISHED state.

In this commit, we:
- improve the fix for hanging poll() by replacing sk_data_ready()
  with sk_state_change() to wake up all clients.
- revert the faulty updates introduced by commit 517d7c79bdb398
  ("tipc: fix hanging poll() for stream sockets").

Fixes: 517d7c79bdb398 ("tipc: fix hanging poll() for stream sockets")
Signed-off-by: Parthasarathy Bhuvaragan <par...@gm...>
---
 net/tipc/socket.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index b542f14ed444..2851937f6e32 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -734,11 +734,11 @@ static __poll_t tipc_poll(struct file *file, struct socket *sock,
 
 	switch (sk->sk_state) {
 	case TIPC_ESTABLISHED:
-	case TIPC_CONNECTING:
 		if (!tsk->cong_link_cnt && !tsk_conn_cong(tsk))
 			revents |= EPOLLOUT;
 		/* fall through */
 	case TIPC_LISTEN:
+	case TIPC_CONNECTING:
 		if (!skb_queue_empty(&sk->sk_receive_queue))
 			revents |= EPOLLIN | EPOLLRDNORM;
 		break;
@@ -2041,7 +2041,7 @@ static bool tipc_sk_filter_connect(struct tipc_sock *tsk, struct sk_buff *skb)
 			if (msg_data_sz(hdr))
 				return true;
 			/* Empty ACK-, - wake up sleeping connect() and drop */
-			sk->sk_data_ready(sk);
+			sk->sk_state_change(sk);
 			msg_set_dest_droppable(hdr, 1);
 			return false;
 		}
-- 
2.21.0

Re: [tipc-discussion] [net-next] tipc: fix missing Name entries due to half-failover

[David M. <da...@da...>]

From: Tuong Lien <tuo...@de...>
Date: Thu,  2 May 2019 17:23:23 +0700

> TIPC link can temporarily fall into "half-establish" that only one of
> the link endpoints is ESTABLISHED and starts to send traffic, PROTOCOL
> messages, whereas the other link endpoint is not up (e.g. immediately
> when the endpoint receives ACTIVATE_MSG, the network interface goes
> down...).
> 
> This is a normal situation and will be settled because the link
> endpoint will be eventually brought down after the link tolerance time.
> 
> However, the situation will become worse when the second link is
> established before the first link endpoint goes down,
> For example:
> 
>    1. Both links <1A-2A>, <1B-2B> down
>    2. Link endpoint 2A up, but 1A still down (e.g. due to network
>       disturbance, wrong session, etc.)
>    3. Link <1B-2B> up
>    4. Link endpoint 2A down (e.g. due to link tolerance timeout)
>    5. Node B starts failover onto link <1B-2B>
> 
>    ==> Node A does never start link failover.
> 
> When the "half-failover" situation happens, two consequences have been
> observed:
> 
> a) Peer link/node gets stuck in FAILINGOVER state;
> b) Traffic or user messages that peer node is trying to failover onto
> the second link can be partially or completely dropped by this node.
> 
> The consequence a) was actually solved by commit c140eb166d68 ("tipc:
> fix failover problem"), but that commit didn't cover the b). It's due
> to the fact that the tunnel link endpoint has never been prepared for a
> failover, so the 'l->drop_point' (and the other data...) is not set
> correctly. When a TUNNEL_MSG from peer node arrives on the link,
> depending on the inner message's seqno and the current 'l->drop_point'
> value, the message can be dropped (- treated as a duplicate message) or
> processed.
> At this early stage, the traffic messages from peer are likely to be
> NAME_DISTRIBUTORs, this means some name table entries will be missed on
> the node forever!
> 
> The commit resolves the issue by starting the FAILOVER process on this
> node as well. Another benefit from this solution is that we ensure the
> link will not be re-established until the failover ends.
> 
> Acked-by: Jon Maloy <jon...@er...>
> Signed-off-by: Tuong Lien <tuo...@de...>

Applied, thank you.