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

From: Nikolay Aleksandrov
Date: Tue Oct 06 2020 - 10:29:49 EST


On Thu, 2020-10-01 at 10:30 +0000, 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<E2><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 <horatiu.vultur@xxxxxxxxxxxxx>
> Signed-off-by: Henrik Bjoernlund <henrik.bjoernlund@xxxxxxxxxxxxx>
> ---

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);
> +}