Re: [net-next v2 04/11] bridge: cfm: Kernel space implementation of CFM.

2020-10-06 Thread Nikolay Aleksandrov
On Thu, 2020-10-01 at 10:30 +, Henrik Bjoernlund wrote:
> This is the first commit of the implementation of the CFM protocol
> according to 802.1Q section 12.14.
> 
> Connectivity Fault Management (CFM) comprises capabilities for
> detecting, verifying, and isolating connectivity failures in
> Virtual Bridged Networks. These capabilities can be used in
> networks operated by multiple independent organizations, each
> with restricted management access to each other<80><99>s equipment.
> 
> CFM functions are partitioned as follows:
> - Path discovery
> - Fault detection
> - Fault verification and isolation
> - Fault notification
> - Fault recovery
> 
> Interface consists of these functions:
> br_cfm_mep_create()
> br_cfm_mep_delete()
> br_cfm_mep_config_set()
> br_cfm_cc_config_set()
> br_cfm_cc_peer_mep_add()
> br_cfm_cc_peer_mep_remove()
> 
> A MEP instance is created by br_cfm_mep_create()
> -It is the Maintenance association End Point
>  described in 802.1Q section 19.2.
> -It is created on a specific level (1-7) and is assuring
>  that no CFM frames are passing through this MEP on lower levels.
> -It initiates and validates CFM frames on its level.
> -It can only exist on a port that is related to a bridge.
> -Attributes given cannot be changed until the instance is
>  deleted.
> 
> A MEP instance can be deleted by br_cfm_mep_delete().
> 
> A created MEP instance has attributes that can be
> configured by br_cfm_mep_config_set().
> 
> A MEP Continuity Check feature can be configured by
> br_cfm_cc_config_set()
> The Continuity Check Receiver state machine can be
> enabled and disabled.
> According to 802.1Q section 19.2.8
> 
> A MEP can have Peer MEPs added and removed by
> br_cfm_cc_peer_mep_add() and br_cfm_cc_peer_mep_remove()
> The Continuity Check feature can maintain connectivity
> status on each added Peer MEP.
> 
> Reviewed-by: Horatiu Vultur  
> Signed-off-by: Henrik Bjoernlund  
> ---

Thank you for breaking the big patch into 3 smaller pieces, but could you please
name them appropriately? I'm sure they add different things, so just give them
something more descriptive. Having the same subject for 3 patches looks odd.

>  include/uapi/linux/cfm_bridge.h |  23 +++
>  net/bridge/Makefile |   2 +
>  net/bridge/br_cfm.c | 263 
>  net/bridge/br_private_cfm.h |  61 
>  4 files changed, 349 insertions(+)
>  create mode 100644 include/uapi/linux/cfm_bridge.h
>  create mode 100644 net/bridge/br_cfm.c
>  create mode 100644 net/bridge/br_private_cfm.h
> 
[snip]
> +
> + mep = kzalloc(sizeof(*mep), GFP_KERNEL);
> + if (!mep)
> + return -ENOMEM;
> +
> + mep->create = *create;
> + mep->instance = instance;
> + rcu_assign_pointer(mep->b_port, p);
> +
> + INIT_HLIST_HEAD(&mep->peer_mep_list);
> +
> + hlist_add_tail_rcu(&mep->head, &br->mep_list);
> +
> + return 0;
> +}
> +
> +static void mep_delete_implementation(struct net_bridge *br,
> +   struct br_cfm_mep *mep)
> +{
> + struct br_cfm_peer_mep *peer_mep;
> +
> + ASSERT_RTNL();
> +
> + /* Empty and free peer MEP list */
> + hlist_for_each_entry(peer_mep, &mep->peer_mep_list, head) {

hlist_for_each_entry_safe()

> + hlist_del_rcu(&peer_mep->head);
> + kfree_rcu(peer_mep, rcu);
> + }
> +
> + RCU_INIT_POINTER(mep->b_port, NULL);
> + hlist_del_rcu(&mep->head);
> + kfree_rcu(mep, rcu);
> +}



[net-next v2 04/11] bridge: cfm: Kernel space implementation of CFM.

2020-10-01 Thread Henrik Bjoernlund
This is the first commit of the implementation of the CFM protocol
according to 802.1Q section 12.14.

Connectivity Fault Management (CFM) comprises capabilities for
detecting, verifying, and isolating connectivity failures in
Virtual Bridged Networks. These capabilities can be used in
networks operated by multiple independent organizations, each
with restricted management access to each other<80><99>s equipment.

CFM functions are partitioned as follows:
- Path discovery
- Fault detection
- Fault verification and isolation
- Fault notification
- Fault recovery

Interface consists of these functions:
br_cfm_mep_create()
br_cfm_mep_delete()
br_cfm_mep_config_set()
br_cfm_cc_config_set()
br_cfm_cc_peer_mep_add()
br_cfm_cc_peer_mep_remove()

A MEP instance is created by br_cfm_mep_create()
-It is the Maintenance association End Point
 described in 802.1Q section 19.2.
-It is created on a specific level (1-7) and is assuring
 that no CFM frames are passing through this MEP on lower levels.
-It initiates and validates CFM frames on its level.
-It can only exist on a port that is related to a bridge.
-Attributes given cannot be changed until the instance is
 deleted.

A MEP instance can be deleted by br_cfm_mep_delete().

A created MEP instance has attributes that can be
configured by br_cfm_mep_config_set().

A MEP Continuity Check feature can be configured by
br_cfm_cc_config_set()
The Continuity Check Receiver state machine can be
enabled and disabled.
According to 802.1Q section 19.2.8

A MEP can have Peer MEPs added and removed by
br_cfm_cc_peer_mep_add() and br_cfm_cc_peer_mep_remove()
The Continuity Check feature can maintain connectivity
status on each added Peer MEP.

Reviewed-by: Horatiu Vultur  
Signed-off-by: Henrik Bjoernlund  
---
 include/uapi/linux/cfm_bridge.h |  23 +++
 net/bridge/Makefile |   2 +
 net/bridge/br_cfm.c | 263 
 net/bridge/br_private_cfm.h |  61 
 4 files changed, 349 insertions(+)
 create mode 100644 include/uapi/linux/cfm_bridge.h
 create mode 100644 net/bridge/br_cfm.c
 create mode 100644 net/bridge/br_private_cfm.h

diff --git a/include/uapi/linux/cfm_bridge.h b/include/uapi/linux/cfm_bridge.h
new file mode 100644
index ..a262a8c0e085
--- /dev/null
+++ b/include/uapi/linux/cfm_bridge.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+
+#ifndef _UAPI_LINUX_CFM_BRIDGE_H_
+#define _UAPI_LINUX_CFM_BRIDGE_H_
+
+#include 
+#include 
+
+#define CFM_MAID_LENGTH48
+
+/* MEP domain */
+enum br_cfm_domain {
+   BR_CFM_PORT,
+   BR_CFM_VLAN,
+};
+
+/* MEP direction */
+enum br_cfm_mep_direction {
+   BR_CFM_MEP_DIRECTION_DOWN,
+   BR_CFM_MEP_DIRECTION_UP,
+};
+
+#endif
diff --git a/net/bridge/Makefile b/net/bridge/Makefile
index ccb394236fbd..ddc0a9192348 100644
--- a/net/bridge/Makefile
+++ b/net/bridge/Makefile
@@ -27,3 +27,5 @@ bridge-$(CONFIG_NET_SWITCHDEV) += br_switchdev.o
 obj-$(CONFIG_NETFILTER) += netfilter/
 
 bridge-$(CONFIG_BRIDGE_MRP)+= br_mrp_switchdev.o br_mrp.o br_mrp_netlink.o
+
+bridge-$(CONFIG_BRIDGE_CFM)+= br_cfm.o
diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
new file mode 100644
index ..86d6e7b73375
--- /dev/null
+++ b/net/bridge/br_cfm.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include 
+#include 
+#include "br_private_cfm.h"
+
+static struct br_cfm_mep *br_mep_find(struct net_bridge *br, u32 instance)
+{
+   struct br_cfm_mep *mep;
+
+   hlist_for_each_entry(mep, &br->mep_list, head)
+   if (mep->instance == instance)
+   return mep;
+
+   return NULL;
+}
+
+static struct br_cfm_mep *br_mep_find_ifindex(struct net_bridge *br,
+ u32 ifindex)
+{
+   struct br_cfm_mep *mep;
+
+   hlist_for_each_entry_rcu(mep, &br->mep_list, head,
+lockdep_rtnl_is_held())
+   if (mep->create.ifindex == ifindex)
+   return mep;
+
+   return NULL;
+}
+
+static struct br_cfm_peer_mep *br_peer_mep_find(struct br_cfm_mep *mep,
+   u32 mepid)
+{
+   struct br_cfm_peer_mep *peer_mep;
+
+   hlist_for_each_entry_rcu(peer_mep, &mep->peer_mep_list, head,
+lockdep_rtnl_is_held())
+   if (peer_mep->mepid == mepid)
+   return peer_mep;
+
+   return NULL;
+}
+
+static struct net_bridge_port *br_mep_get_port(struct net_bridge *br,
+  u32 ifindex)
+{
+   struct net_bridge_port *port;
+
+   list_for_each_entry(port, &br->port_list, list)
+   if (port->dev->ifindex == ifindex)
+   return port;
+
+   return NULL;
+}
+
+int br_cfm_mep_create(struct net_bridge *br,
+