-----Original Message-----
From: Jon Maloy [mailto:jon.ma...@ericsson.com] 
Sent: Friday, July 08, 2016 4:21 AM
To: tipc-discussion@lists.sourceforge.net; 
parthasarathy.bhuvara...@ericsson.com; Xue, Ying; richard.a...@ericsson.com; 
jon.ma...@ericsson.com
Cc: ma...@donjonn.com
Subject: [PATCH net-next v2 1/2] tipc: make bearer packet filtering generic

In commit 5b7066c3dd24 ("tipc: stricter filtering of packets in bearer
layer") we introduced a method of filtering out messages while a bearer is 
being reset, to avoid that links may be re-created and come back in working 
state while we are still in the process of shutting them down.

This solution works well, but is limited to only work with L2 media, which is 
insufficient with the increasing use of UDP as carrier media.

We now replace this solution with a more generic one, by introducing a new 
boolean field "blocked" in the generic struct tipc_bearer. This field will be 
set and reset at the same locations as with the previous solution, while the 
packet filtering is moved to the generic code for the sending side. On the 
receiving side, the filtering is still done in media specific code, but now 
including the UDP bearer.

Signed-off-by: Jon Maloy <jon.ma...@ericsson.com>
---
 net/tipc/bearer.c    | 77 ++++++++++++++++++++++++++--------------------------
 net/tipc/bearer.h    |  1 +
 net/tipc/udp_media.c |  2 +-
 3 files changed, 41 insertions(+), 39 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c index e7244d6..6d4c2b3 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -56,6 +56,13 @@ static struct tipc_media * const media_info_array[] = {
        NULL
 };
 
+static struct tipc_bearer *bearer_get(struct net *net, int bearer_id) {
+       struct tipc_net *tn = tipc_net(net);
+
+       return rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+}
+
 static void bearer_disable(struct net *net, struct tipc_bearer *b);
 
 /**
@@ -302,6 +309,7 @@ restart:
        b->domain = disc_domain;
        b->net_plane = bearer_id + 'A';
        b->priority = priority;
+       b->up = true;
 
        res = tipc_disc_create(net, b, &b->bcast_addr, &skb);
        if (res) {
@@ -339,16 +347,25 @@ static int tipc_reset_bearer(struct net *net, struct 
tipc_bearer *b)
  */
 void tipc_bearer_reset_all(struct net *net)  {
-       struct tipc_net *tn = tipc_net(net);
        struct tipc_bearer *b;
        int i;
 
        rcu_read_lock();
        for (i = 0; i < MAX_BEARERS; i++) {
-               b = rcu_dereference_rtnl(tn->bearer_list[i]);
+               b = bearer_get(net, i);
+               if (b)
+                       b->up = false;
+       }
+       for (i = 0; i < MAX_BEARERS; i++) {
+               b = bearer_get(net, i);
                if (b)
                        tipc_reset_bearer(net, b);
        }
+       for (i = 0; i < MAX_BEARERS; i++) {
+               b = bearer_get(net, i);
+               if (b)
+                       b->up = true;
+       }

If we are under rcu_read lock, it means that we are staying on read side. 
Regarding RCU locking semantic, we cannot modify RCU variable on read side. As 
you know, bearer instance is projected by RTNL lock. If we want to change it, 
we must hold it on write side. However, as we must reset links on reception 
path, it means that it's inappropriate to hold RTNL on the context. I cannot 
figure more better approach except that we perform this in another process 
context asynchronously like workqueue. But this is really not what we want to 
do.

Regards,
Ying

        rcu_read_unlock();
 }
 
@@ -363,8 +380,9 @@ static void bearer_disable(struct net *net, struct 
tipc_bearer *b)
        int bearer_id = b->identity;
 
        pr_info("Disabling bearer <%s>\n", b->name);
-       b->media->disable_media(b);
+       b->up = false;
        tipc_node_delete_links(net, bearer_id);
+       b->media->disable_media(b);
        RCU_INIT_POINTER(b->media_ptr, NULL);
        if (b->link_req)
                tipc_disc_delete(b->link_req);
@@ -421,22 +439,16 @@ int tipc_l2_send_msg(struct net *net, struct sk_buff 
*skb,  {
        struct net_device *dev;
        int delta;
-       void *tipc_ptr;
 
        dev = (struct net_device *)rcu_dereference_rtnl(b->media_ptr);
        if (!dev)
                return 0;
 
-       /* Send RESET message even if bearer is detached from device */
-       tipc_ptr = rcu_dereference_rtnl(dev->tipc_ptr);
-       if (unlikely(!tipc_ptr && !msg_is_reset(buf_msg(skb))))
-               goto drop;
-
-       delta = dev->hard_header_len - skb_headroom(skb);
-       if ((delta > 0) &&
-           pskb_expand_head(skb, SKB_DATA_ALIGN(delta), 0, GFP_ATOMIC))
-               goto drop;
-
+       delta = SKB_DATA_ALIGN(dev->hard_header_len - skb_headroom(skb));
+       if ((delta > 0) && pskb_expand_head(skb, delta, 0, GFP_ATOMIC)) {
+               kfree_skb(skb);
+               return 0;
+       }
        skb_reset_network_header(skb);
        skb->dev = dev;
        skb->protocol = htons(ETH_P_TIPC);
@@ -444,9 +456,6 @@ int tipc_l2_send_msg(struct net *net, struct sk_buff *skb,
                        dev->dev_addr, skb->len);
        dev_queue_xmit(skb);
        return 0;
-drop:
-       kfree_skb(skb);
-       return 0;
 }
 
 int tipc_bearer_mtu(struct net *net, u32 bearer_id) @@ -468,12 +477,12 @@ void 
tipc_bearer_xmit_skb(struct net *net, u32 bearer_id,
                          struct sk_buff *skb,
                          struct tipc_media_addr *dest)
 {
-       struct tipc_net *tn = tipc_net(net);
+       struct tipc_msg *hdr = buf_msg(skb);
        struct tipc_bearer *b;
 
        rcu_read_lock();
-       b = rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
-       if (likely(b))
+       b = bearer_get(net, bearer_id);
+       if (likely(b && (b->up || msg_is_reset(hdr))))
                b->media->send_msg(net, skb, b, dest);
        else
                kfree_skb(skb);
@@ -486,7 +495,6 @@ void tipc_bearer_xmit(struct net *net, u32 bearer_id,
                      struct sk_buff_head *xmitq,
                      struct tipc_media_addr *dst)
 {
-       struct tipc_net *tn = net_generic(net, tipc_net_id);
        struct tipc_bearer *b;
        struct sk_buff *skb, *tmp;
 
@@ -494,12 +502,15 @@ void tipc_bearer_xmit(struct net *net, u32 bearer_id,
                return;
 
        rcu_read_lock();
-       b = rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+       b = bearer_get(net, bearer_id);
        if (unlikely(!b))
                __skb_queue_purge(xmitq);
        skb_queue_walk_safe(xmitq, skb, tmp) {
                __skb_dequeue(xmitq);
-               b->media->send_msg(net, skb, b, dst);
+               if (likely(b->up || msg_is_reset(buf_msg(skb))))
+                       b->media->send_msg(net, skb, b, dst);
+               else
+                       kfree(skb);
        }
        rcu_read_unlock();
 }
@@ -516,8 +527,8 @@ void tipc_bearer_bc_xmit(struct net *net, u32 bearer_id,
        struct tipc_msg *hdr;
 
        rcu_read_lock();
-       b = rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
-       if (unlikely(!b))
+       b = bearer_get(net, bearer_id);
+       if (unlikely(!b || !b->up))
                __skb_queue_purge(xmitq);
        skb_queue_walk_safe(xmitq, skb, tmp) {
                hdr = buf_msg(skb);
@@ -547,7 +558,7 @@ static int tipc_l2_rcv_msg(struct sk_buff *skb, struct 
net_device *dev,
 
        rcu_read_lock();
        b = rcu_dereference_rtnl(dev->tipc_ptr);
-       if (likely(b && (skb->pkt_type <= PACKET_BROADCAST))) {
+       if (likely(b && b->up && (skb->pkt_type <= PACKET_BROADCAST))) {
                skb->next = NULL;
                tipc_rcv(dev_net(dev), skb, b);
                rcu_read_unlock();
@@ -572,18 +583,9 @@ static int tipc_l2_device_event(struct notifier_block *nb, 
unsigned long evt,  {
        struct net_device *dev = netdev_notifier_info_to_dev(ptr);
        struct net *net = dev_net(dev);
-       struct tipc_net *tn = tipc_net(net);
        struct tipc_bearer *b;
-       int i;
 
        b = rtnl_dereference(dev->tipc_ptr);
-       if (!b) {
-               for (i = 0; i < MAX_BEARERS; b = NULL, i++) {
-                       b = rtnl_dereference(tn->bearer_list[i]);
-                       if (b && (b->media_ptr == dev))
-                               break;
-               }
-       }
        if (!b)
                return NOTIFY_DONE;
 
@@ -594,11 +596,10 @@ static int tipc_l2_device_event(struct notifier_block 
*nb, unsigned long evt,
                if (netif_carrier_ok(dev))
                        break;
        case NETDEV_UP:
-               rcu_assign_pointer(dev->tipc_ptr, b);
+               b->up = true;
                break;
        case NETDEV_GOING_DOWN:
-               RCU_INIT_POINTER(dev->tipc_ptr, NULL);
-               synchronize_net();
+               b->up = false;
                tipc_reset_bearer(net, b);
                break;
        case NETDEV_CHANGEMTU:
diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h index f1e6db5..6f51ed8 100644
--- a/net/tipc/bearer.h
+++ b/net/tipc/bearer.h
@@ -150,6 +150,7 @@ struct tipc_bearer {
        u32 identity;
        struct tipc_link_req *link_req;
        char net_plane;
+       bool up;
 };
 
 struct tipc_bearer_names {
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c index b016c01..c7f52bf 
100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -224,7 +224,7 @@ static int tipc_udp_recv(struct sock *sk, struct sk_buff 
*skb)
        rcu_read_lock();
        b = rcu_dereference_rtnl(ub->bearer);
 
-       if (b) {
+       if (b && b->up) {
                tipc_rcv(sock_net(sk), skb, b);
                rcu_read_unlock();
                return 0;
--
1.9.1

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Reply via email to