Re: [PATCH RFC 5/7] bridge: cfm: Netlink Interface.

From: Nikolay Aleksandrov
Date: Tue Sep 08 2020 - 10:37:59 EST


On Fri, 2020-09-04 at 09:15 +0000, Henrik Bjoernlund wrote:
> This is the implementation of CFM netlink configuration
> and status information interface.
>
> Add new nested netlink attributes. These attributes are used by the
> user space to create/delete/configure CFM instances and get status.
> Also they are used by the kernel to notify the user space when changes
> in any status happens.
[snip]
> __u64 transition_fwd;
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index 9b814c92de12..fdd408f6a5d2 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -779,6 +779,8 @@ enum {
> #define RTEXT_FILTER_BRVLAN_COMPRESSED (1 << 2)
> #define RTEXT_FILTER_SKIP_STATS (1 << 3)
> #define RTEXT_FILTER_MRP (1 << 4)
> +#define RTEXT_FILTER_CFM_CONFIG (1 << 5)
> +#define RTEXT_FILTER_CFM_STATUS (1 << 6)
>
> /* End of information exported to user level */
>
> diff --git a/net/bridge/Makefile b/net/bridge/Makefile
> index ddc0a9192348..4702702a74d3 100644
> --- a/net/bridge/Makefile
> +++ b/net/bridge/Makefile
> @@ -28,4 +28,4 @@ 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
> +bridge-$(CONFIG_BRIDGE_CFM) += br_cfm.o br_cfm_netlink.o
> diff --git a/net/bridge/br_cfm_netlink.c b/net/bridge/br_cfm_netlink.c
> new file mode 100644
> index 000000000000..4e39aab1cd0b
> --- /dev/null
> +++ b/net/bridge/br_cfm_netlink.c
> @@ -0,0 +1,684 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <net/genetlink.h>
> +
> +#include "br_private.h"
> +#include "br_private_cfm.h"
> +
> +static inline struct mac_addr nla_get_mac(const struct nlattr *nla)
> +{
> + struct mac_addr mac;
> +
> + nla_memcpy(&mac.addr, nla, sizeof(mac.addr));
> +
> + return mac;
> +}
> +
> +static inline struct br_cfm_maid nla_get_maid(const struct nlattr *nla)
> +{
> + struct br_cfm_maid maid;
> +
> + nla_memcpy(&maid.data, nla, sizeof(maid.data));
> +
> + return maid;
> +}

IMO, these 1-line helpers don't really help readability.

> +
> +static inline int nla_put_u64(struct sk_buff *skb, int attrtype, u64 value)
> +{
> + u64 tmp = value;
> +
> + return nla_put(skb, attrtype, sizeof(u64), &tmp);
> +}

What?! Read include/net/netlink.h

> +
> +static const struct nla_policy
> +br_cfm_policy[IFLA_BRIDGE_CFM_MAX + 1] = {
> + [IFLA_BRIDGE_CFM_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_CFM_MEP_CREATE] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_CFM_MEP_DELETE] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_CFM_MEP_CONFIG] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_CFM_CC_CONFIG] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_CFM_CC_PEER_MEP_ADD] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_CFM_CC_PEER_MEP_REMOVE] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_CFM_CC_RDI] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_CFM_CC_CCM_TX] = { .type = NLA_NESTED },
> +};
> +
> +static const struct nla_policy
> +br_cfm_mep_create_policy[IFLA_BRIDGE_CFM_MEP_CREATE_MAX + 1] = {
> + [IFLA_BRIDGE_CFM_MEP_CREATE_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_CFM_MEP_CREATE_INSTANCE] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_MEP_CREATE_DOMAIN] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_MEP_CREATE_DIRECTION] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_MEP_CREATE_IFINDEX] = { .type = NLA_U32 },
> +};
> +
> +static const struct nla_policy
> +br_cfm_mep_delete_policy[IFLA_BRIDGE_CFM_MEP_DELETE_MAX + 1] = {
> + [IFLA_BRIDGE_CFM_MEP_DELETE_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_CFM_MEP_DELETE_INSTANCE] = { .type = NLA_U32 },
> +};
> +
> +static const struct nla_policy
> +br_cfm_mep_config_policy[IFLA_BRIDGE_CFM_MEP_CONFIG_MAX + 1] = {
> + [IFLA_BRIDGE_CFM_MEP_CONFIG_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_CFM_MEP_CONFIG_INSTANCE] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_MEP_CONFIG_UNICAST_MAC] = NLA_POLICY_ETH_ADDR,
> + [IFLA_BRIDGE_CFM_MEP_CONFIG_MDLEVEL] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_MEP_CONFIG_MEPID] = { .type = NLA_U32 },
> +};
> +
> +static const struct nla_policy
> +br_cfm_cc_config_policy[IFLA_BRIDGE_CFM_CC_CONFIG_MAX + 1] = {
> + [IFLA_BRIDGE_CFM_CC_CONFIG_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_CFM_CC_CONFIG_INSTANCE] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_CONFIG_ENABLE] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_CONFIG_EXP_MAID] = {
> + .type = NLA_BINARY, .len = CFM_MAID_LENGTH },
> +};
> +
> +static const struct nla_policy
> +br_cfm_cc_peer_mep_policy[IFLA_BRIDGE_CFM_CC_PEER_MEP_MAX + 1] = {
> + [IFLA_BRIDGE_CFM_CC_PEER_MEP_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_CFM_CC_PEER_MEP_INSTANCE] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_PEER_MEPID] = { .type = NLA_U32 },
> +};
> +
> +static const struct nla_policy
> +br_cfm_cc_rdi_policy[IFLA_BRIDGE_CFM_CC_RDI_MAX + 1] = {
> + [IFLA_BRIDGE_CFM_CC_RDI_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_CFM_CC_RDI_INSTANCE] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_RDI_RDI] = { .type = NLA_U32 },
> +};
> +
> +static const struct nla_policy
> +br_cfm_cc_ccm_tx_policy[IFLA_BRIDGE_CFM_CC_CCM_TX_MAX + 1] = {
> + [IFLA_BRIDGE_CFM_CC_CCM_TX_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_CFM_CC_CCM_TX_INSTANCE] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_CCM_TX_DMAC] = NLA_POLICY_ETH_ADDR,
> + [IFLA_BRIDGE_CFM_CC_CCM_TX_SEQ_NO_UPDATE] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_CCM_TX_PERIOD] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_CCM_TX_IF_TLV] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_CCM_TX_IF_TLV_VALUE] = { .type = NLA_U8 },
> + [IFLA_BRIDGE_CFM_CC_CCM_TX_PORT_TLV] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_CCM_TX_PORT_TLV_VALUE] = { .type = NLA_U8 },
> +};
> +
> +static int br_mep_create_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_CFM_MEP_CREATE_MAX + 1];
> + struct br_cfm_mep_create create;
> + u32 instance;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_CFM_MEP_CREATE_MAX, attr,
> + br_cfm_mep_create_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_CFM_MEP_CREATE_INSTANCE] ||
> + !tb[IFLA_BRIDGE_CFM_MEP_CREATE_DOMAIN] ||
> + !tb[IFLA_BRIDGE_CFM_MEP_CREATE_DIRECTION] ||
> + !tb[IFLA_BRIDGE_CFM_MEP_CREATE_IFINDEX]) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Missing attribute: INSTANCE or DOMAIN or DIRECTION or VID or IFINDEX");

Break these into separate errors.

> + return -EINVAL;
> + }
> +
> + memset(&create, 0, sizeof(create));
> +
> + instance = nla_get_u32(tb[IFLA_BRIDGE_CFM_MEP_CREATE_INSTANCE]);
> + create.domain = nla_get_u32(tb[IFLA_BRIDGE_CFM_MEP_CREATE_DOMAIN]);
> + create.direction = nla_get_u32(tb[IFLA_BRIDGE_CFM_MEP_CREATE_DIRECTION]);
> + create.ifindex = nla_get_u32(tb[IFLA_BRIDGE_CFM_MEP_CREATE_IFINDEX]);
> +
> + return br_cfm_mep_create(br, instance, &create, extack);
> +}
> +
> +static int br_mep_delete_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_CFM_MEP_DELETE_MAX + 1];
> + u32 instance;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_CFM_MEP_DELETE_MAX, attr,
> + br_cfm_mep_delete_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_CFM_MEP_CREATE_INSTANCE]) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Missing attribute: INSTANCE");

"Missing instance attribute". Same for all similar messages.

> + return -EINVAL;
> + }
> +
> + instance = nla_get_u32(tb[IFLA_BRIDGE_CFM_MEP_CREATE_INSTANCE]);
> +
> + return br_cfm_mep_delete(br, instance, extack);
> +}
> +
> +static int br_mep_config_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_CFM_MEP_CONFIG_MAX + 1];
> + struct br_cfm_mep_config config;
> + u32 instance;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_CFM_MEP_CONFIG_MAX, attr,
> + br_cfm_mep_config_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_CFM_MEP_CONFIG_INSTANCE] ||
> + !tb[IFLA_BRIDGE_CFM_MEP_CONFIG_UNICAST_MAC] ||
> + !tb[IFLA_BRIDGE_CFM_MEP_CONFIG_MDLEVEL] ||
> + !tb[IFLA_BRIDGE_CFM_MEP_CONFIG_MEPID]) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Missing attribute: INSTANCE or UNICAST_MAC or MDLEVEL or MEPID or VID");

Break these into separate errors.

> + return -EINVAL;
> + }
> +
> + memset(&config, 0, sizeof(config));
> +
> + instance = nla_get_u32(tb[IFLA_BRIDGE_CFM_MEP_CONFIG_INSTANCE]);
> + config.unicast_mac = nla_get_mac(tb[IFLA_BRIDGE_CFM_MEP_CONFIG_UNICAST_MAC]);
> + config.mdlevel = nla_get_u32(tb[IFLA_BRIDGE_CFM_MEP_CONFIG_MDLEVEL]);
> + config.mepid = nla_get_u32(tb[IFLA_BRIDGE_CFM_MEP_CONFIG_MEPID]);
> + config.vid = 0;
> +
> + return br_cfm_mep_config_set(br, instance, &config, extack);
> +}
> +
> +static int br_cc_config_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_CFM_CC_CONFIG_MAX + 1];
> + struct br_cfm_cc_config config;
> + u32 instance;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_CFM_CC_CONFIG_MAX, attr,
> + br_cfm_cc_config_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_CFM_CC_CONFIG_INSTANCE] ||
> + !tb[IFLA_BRIDGE_CFM_CC_CONFIG_ENABLE] ||
> + !tb[IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL] ||
> + !tb[IFLA_BRIDGE_CFM_CC_CONFIG_EXP_MAID]) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Missing attribute: INSTANCE or ENABLE or INTERVAL or PRIORITY or MAID");

Break these into separate errors.

> + return -EINVAL;
> + }
> +
> + memset(&config, 0, sizeof(config));
> +
> + instance = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_CONFIG_INSTANCE]);
> + config.enable = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_CONFIG_ENABLE]);
> + config.exp_interval = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL]);
> + config.exp_priority = 0;
> + config.exp_maid = nla_get_maid(tb[IFLA_BRIDGE_CFM_CC_CONFIG_EXP_MAID]);
> +
> + return br_cfm_cc_config_set(br, instance, &config, extack);
> +}
> +
> +static int br_cc_peer_mep_add_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_CFM_CC_PEER_MEP_MAX + 1];
> + u32 instance, peer_mep_id;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_CFM_CC_PEER_MEP_MAX, attr,
> + br_cfm_cc_peer_mep_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_CFM_CC_PEER_MEP_INSTANCE] ||
> + !tb[IFLA_BRIDGE_CFM_CC_PEER_MEPID]) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Missing attribute: INSTANCE or PEER_MEP_ID");

Break these into separate errors.

> + return -EINVAL;
> + }
> +
> + instance = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_PEER_MEP_INSTANCE]);
> + peer_mep_id = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_PEER_MEPID]);
> +
> + return br_cfm_cc_peer_mep_add(br, instance, peer_mep_id, extack);
> +}
> +
> +static int br_cc_peer_mep_remove_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_CFM_CC_PEER_MEP_MAX + 1];
> + u32 instance, peer_mep_id;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_CFM_CC_PEER_MEP_MAX, attr,
> + br_cfm_cc_peer_mep_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_CFM_CC_PEER_MEP_INSTANCE] ||
> + !tb[IFLA_BRIDGE_CFM_CC_PEER_MEPID]) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Missing attribute: INSTANCE or PEER_MEP_ID");

Break these into separate errors.

> + return -EINVAL;
> + }
> +
> + instance = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_PEER_MEP_INSTANCE]);
> + peer_mep_id = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_PEER_MEPID]);
> +
> + return br_cfm_cc_peer_mep_remove(br, instance, peer_mep_id, extack);
> +}
> +
> +static int br_cc_rdi_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_CFM_CC_RDI_MAX + 1];
> + u32 instance, rdi;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_CFM_CC_RDI_MAX, attr,
> + br_cfm_cc_rdi_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_CFM_CC_RDI_INSTANCE] ||
> + !tb[IFLA_BRIDGE_CFM_CC_RDI_RDI]) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Missing attribute: INSTANCE or RDI");

Break these into separate errors.

> + return -EINVAL;
> + }
> +
> + instance = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_RDI_INSTANCE]);
> + rdi = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_RDI_RDI]);
> +
> + return br_cfm_cc_rdi_set(br, instance, rdi, extack);
> +}
> +
> +static int br_cc_ccm_tx_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_CFM_CC_CCM_TX_MAX + 1];
> + u32 instance;
> + struct br_cfm_cc_ccm_tx_info tx_info;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_CFM_CC_CCM_TX_MAX, attr,
> + br_cfm_cc_ccm_tx_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_CFM_CC_CCM_TX_INSTANCE] ||
> + !tb[IFLA_BRIDGE_CFM_CC_CCM_TX_DMAC] ||
> + !tb[IFLA_BRIDGE_CFM_CC_CCM_TX_SEQ_NO_UPDATE] ||
> + !tb[IFLA_BRIDGE_CFM_CC_CCM_TX_PERIOD] ||
> + !tb[IFLA_BRIDGE_CFM_CC_CCM_TX_IF_TLV] ||
> + !tb[IFLA_BRIDGE_CFM_CC_CCM_TX_IF_TLV_VALUE] ||
> + !tb[IFLA_BRIDGE_CFM_CC_CCM_TX_PORT_TLV] ||
> + !tb[IFLA_BRIDGE_CFM_CC_CCM_TX_PORT_TLV_VALUE]) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Missing attribute: INSTANCE or PRIORITY or DEI or DMAC or SEQ_NO_UPDATE or PERIOD or IF_TLV or IF_TLV_VALUE or PORT_TLV or PORT_TLV_VALUE");

Break these into separate errors.

> + return -EINVAL;
> + }
> +
> + instance = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_RDI_INSTANCE]);
> + tx_info.priority = 0;
> + tx_info.dei = 0;
> + tx_info.dmac = nla_get_mac(tb[IFLA_BRIDGE_CFM_CC_CCM_TX_DMAC]);
> + tx_info.seq_no_update = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_CCM_TX_SEQ_NO_UPDATE]);
> + tx_info.period = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_CCM_TX_PERIOD]);
> + tx_info.if_tlv = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_CCM_TX_IF_TLV]);
> + tx_info.if_tlv_value = nla_get_u8(tb[IFLA_BRIDGE_CFM_CC_CCM_TX_IF_TLV_VALUE]);
> + tx_info.port_tlv = nla_get_u32(tb[IFLA_BRIDGE_CFM_CC_CCM_TX_PORT_TLV]);
> + tx_info.port_tlv_value = nla_get_u8(tb[IFLA_BRIDGE_CFM_CC_CCM_TX_PORT_TLV_VALUE]);
> +
> + return br_cfm_cc_ccm_tx(br, instance, &tx_info, extack);
> +}
> +
> +int br_cfm_parse(struct net_bridge *br, struct net_bridge_port *p,
> + struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_CFM_MAX + 1];
> + int err;
> +
> + /* When this function is called for a port then the br pointer is
> + * invalid, therefor set the br to point correctly
> + */
> + if (p)
> + br = p->br;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_CFM_MAX, attr,
> + br_cfm_policy, extack);
> + if (err)
> + return err;
> +
> + if (tb[IFLA_BRIDGE_CFM_MEP_CREATE]) {
> + err = br_mep_create_parse(br, tb[IFLA_BRIDGE_CFM_MEP_CREATE],
> + extack);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_CFM_MEP_DELETE]) {
> + err = br_mep_delete_parse(br, tb[IFLA_BRIDGE_CFM_MEP_DELETE],
> + extack);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_CFM_MEP_CONFIG]) {
> + err = br_mep_config_parse(br, tb[IFLA_BRIDGE_CFM_MEP_CONFIG],
> + extack);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_CFM_CC_CONFIG]) {
> + err = br_cc_config_parse(br, tb[IFLA_BRIDGE_CFM_CC_CONFIG],
> + extack);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_CFM_CC_PEER_MEP_ADD]) {
> + err = br_cc_peer_mep_add_parse(br, tb[IFLA_BRIDGE_CFM_CC_PEER_MEP_ADD],
> + extack);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_CFM_CC_PEER_MEP_REMOVE]) {
> + err = br_cc_peer_mep_remove_parse(br, tb[IFLA_BRIDGE_CFM_CC_PEER_MEP_REMOVE],
> + extack);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_CFM_CC_RDI]) {
> + err = br_cc_rdi_parse(br, tb[IFLA_BRIDGE_CFM_CC_RDI],
> + extack);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_CFM_CC_CCM_TX]) {
> + err = br_cc_ccm_tx_parse(br, tb[IFLA_BRIDGE_CFM_CC_CCM_TX],
> + extack);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +int br_cfm_config_fill_info(struct sk_buff *skb, struct net_bridge *br)
> +{
> + struct nlattr *tb, *cfm_tb;
> + struct br_cfm_mep *mep;
> + struct br_cfm_peer_mep *peer_mep;
> +
> + cfm_tb = nla_nest_start_noflag(skb, IFLA_BRIDGE_CFM);

Why _noflag everywhere?

> + if (!cfm_tb)
> + return -EMSGSIZE;
> +
> + list_for_each_entry_rcu(mep, &br->mep_list, head) {
> + tb = nla_nest_start_noflag(skb, IFLA_BRIDGE_CFM_MEP_CREATE_INFO);
> + if (!tb)
> + goto nla_info_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_CREATE_INSTANCE,
> + mep->instance))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_CREATE_DOMAIN,
> + mep->create.domain))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_CREATE_DIRECTION,
> + mep->create.direction))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_CREATE_IFINDEX,
> + mep->create.ifindex))
> + goto nla_put_failure;
> +
> + nla_nest_end(skb, tb);
> +
> + tb = nla_nest_start_noflag(skb, IFLA_BRIDGE_CFM_MEP_CONFIG_INFO);
> +
> + if (!tb)
> + goto nla_info_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_CONFIG_INSTANCE,
> + mep->instance))
> + goto nla_put_failure;
> +
> + if (nla_put(skb, IFLA_BRIDGE_CFM_MEP_CONFIG_UNICAST_MAC,
> + sizeof(mep->config.unicast_mac.addr),
> + mep->config.unicast_mac.addr))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_CONFIG_MDLEVEL,
> + mep->config.mdlevel))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_CONFIG_MEPID,
> + mep->config.mepid))
> + goto nla_put_failure;
> +
> + nla_nest_end(skb, tb);
> +
> + tb = nla_nest_start_noflag(skb, IFLA_BRIDGE_CFM_CC_CONFIG_INFO);
> +
> + if (!tb)
> + goto nla_info_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_CONFIG_INSTANCE,
> + mep->instance))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_CONFIG_ENABLE,
> + mep->cc_config.enable))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL,
> + mep->cc_config.exp_interval))
> + goto nla_put_failure;
> +
> + if (nla_put(skb, IFLA_BRIDGE_CFM_CC_CONFIG_EXP_MAID,
> + sizeof(mep->cc_config.exp_maid.data),
> + mep->cc_config.exp_maid.data))
> + goto nla_put_failure;
> +
> + nla_nest_end(skb, tb);
> +
> + tb = nla_nest_start_noflag(skb, IFLA_BRIDGE_CFM_CC_RDI_INFO);
> +
> + if (!tb)
> + goto nla_info_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_RDI_INSTANCE,
> + mep->instance))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_RDI_RDI,
> + mep->rdi))
> + goto nla_put_failure;
> +
> + nla_nest_end(skb, tb);
> +
> + tb = nla_nest_start_noflag(skb, IFLA_BRIDGE_CFM_CC_CCM_TX_INFO);
> +
> + if (!tb)
> + goto nla_info_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_CCM_TX_INSTANCE,
> + mep->instance))
> + goto nla_put_failure;
> +
> + if (nla_put(skb, IFLA_BRIDGE_CFM_CC_CCM_TX_DMAC,
> + sizeof(mep->cc_ccm_tx_info.dmac),
> + mep->cc_ccm_tx_info.dmac.addr))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_CCM_TX_SEQ_NO_UPDATE,
> + mep->cc_ccm_tx_info.seq_no_update))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_CCM_TX_PERIOD,
> + mep->cc_ccm_tx_info.period))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_CCM_TX_IF_TLV,
> + mep->cc_ccm_tx_info.if_tlv))
> + goto nla_put_failure;
> +
> + if (nla_put_u8(skb, IFLA_BRIDGE_CFM_CC_CCM_TX_IF_TLV_VALUE,
> + mep->cc_ccm_tx_info.if_tlv_value))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_CCM_TX_PORT_TLV,
> + mep->cc_ccm_tx_info.port_tlv))
> + goto nla_put_failure;
> +
> + if (nla_put_u8(skb, IFLA_BRIDGE_CFM_CC_CCM_TX_PORT_TLV_VALUE,
> + mep->cc_ccm_tx_info.port_tlv_value))
> + goto nla_put_failure;
> +
> + nla_nest_end(skb, tb);
> +
> + list_for_each_entry_rcu(peer_mep, &mep->peer_mep_list, head) {
> + tb = nla_nest_start_noflag(skb, IFLA_BRIDGE_CFM_CC_PEER_MEP_INFO);
> +
> + if (!tb)
> + goto nla_info_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_PEER_MEP_INSTANCE,
> + mep->instance))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_PEER_MEPID,
> + peer_mep->mepid))
> + goto nla_put_failure;
> +
> + nla_nest_end(skb, tb);
> + }
> + }
> + nla_nest_end(skb, cfm_tb);
> +
> + return 0;
> +
> +nla_put_failure:
> + nla_nest_cancel(skb, tb);
> +
> +nla_info_failure:
> + nla_nest_cancel(skb, cfm_tb);
> +
> + return -EMSGSIZE;
> +}
> +
> +int br_cfm_status_fill_info(struct sk_buff *skb, struct net_bridge *br)
> +{
> + struct nlattr *tb, *cfm_tb;
> + struct br_cfm_mep *mep;
> + struct br_cfm_peer_mep *peer_mep;
> +
> + cfm_tb = nla_nest_start_noflag(skb, IFLA_BRIDGE_CFM);
> + if (!cfm_tb)
> + return -EMSGSIZE;
> +
> + list_for_each_entry_rcu(mep, &br->mep_list, head) {
> + tb = nla_nest_start_noflag(skb, IFLA_BRIDGE_CFM_MEP_STATUS_INFO);
> + if (!tb)
> + goto nla_info_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_STATUS_INSTANCE,
> + mep->instance))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_STATUS_OPCODE_UNEXP_SEEN,
> + mep->status.opcode_unexp_seen))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_STATUS_VERSION_UNEXP_SEEN,
> + mep->status.version_unexp_seen))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_MEP_STATUS_RX_LEVEL_LOW_SEEN,
> + mep->status.rx_level_low_seen))
> + goto nla_put_failure;
> +
> + /* Clear all 'seen' indications */
> + mep->status.opcode_unexp_seen = false;
> + mep->status.version_unexp_seen = false;
> + mep->status.rx_level_low_seen = false;
> +
> + nla_nest_end(skb, tb);
> +
> + list_for_each_entry_rcu(peer_mep, &mep->peer_mep_list, head) {
> + tb = nla_nest_start_noflag(skb, IFLA_BRIDGE_CFM_CC_PEER_STATUS_INFO);
> +
> + if (!tb)
> + goto nla_info_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_PEER_STATUS_INSTANCE,
> + mep->instance))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_PEER_STATUS_PEER_MEPID,
> + peer_mep->mepid))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_PEER_STATUS_CCM_DEFECT,
> + peer_mep->cc_status.ccm_defect))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_PEER_STATUS_RDI,
> + peer_mep->cc_status.rdi))
> + goto nla_put_failure;
> +
> + if (nla_put_u8(skb, IFLA_BRIDGE_CFM_CC_PEER_STATUS_PORT_TLV_VALUE,
> + peer_mep->cc_status.port_tlv_value))
> + goto nla_put_failure;
> +
> + if (nla_put_u8(skb, IFLA_BRIDGE_CFM_CC_PEER_STATUS_IF_TLV_VALUE,
> + peer_mep->cc_status.if_tlv_value))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_PEER_STATUS_SEEN,
> + peer_mep->cc_status.seen))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_PEER_STATUS_TLV_SEEN,
> + peer_mep->cc_status.tlv_seen))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, IFLA_BRIDGE_CFM_CC_PEER_STATUS_SEQ_UNEXP_SEEN,
> + peer_mep->cc_status.seq_unexp_seen))
> + goto nla_put_failure;
> +
> + /* Clear all 'seen' indications */
> + peer_mep->cc_status.seen = false;
> + peer_mep->cc_status.tlv_seen = false;
> + peer_mep->cc_status.seq_unexp_seen = false;
> +
> + nla_nest_end(skb, tb);
> + }
> + }
> + nla_nest_end(skb, cfm_tb);
> +
> + return 0;
> +
> +nla_put_failure:
> + nla_nest_cancel(skb, tb);
> +
> +nla_info_failure:
> + nla_nest_cancel(skb, cfm_tb);
> +
> + return -EMSGSIZE;
> +}
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 8a71c60fa357..6de5cb1295f6 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -16,6 +16,7 @@
>
> #include "br_private.h"
> #include "br_private_stp.h"
> +#include "br_private_cfm.h"
> #include "br_private_tunnel.h"
>
> static int __get_num_vlan_infos(struct net_bridge_vlan_group *vg,
> @@ -481,6 +482,48 @@ static int br_fill_ifinfo(struct sk_buff *skb,
> nla_nest_end(skb, af);
> }
>
> + if (filter_mask & RTEXT_FILTER_CFM_CONFIG) {
> + struct nlattr *af;
> + int err;
> +
> + if (!br_cfm_created(br) || port)
> + goto done;
> +
> + af = nla_nest_start_noflag(skb, IFLA_AF_SPEC);
> + if (!af)
> + goto nla_put_failure;
> +
> + rcu_read_lock();
> + err = br_cfm_config_fill_info(skb, br);
> + rcu_read_unlock();
> +
> + if (err)
> + goto nla_put_failure;
> +
> + nla_nest_end(skb, af);
> + }
> +
> + if (filter_mask & RTEXT_FILTER_CFM_STATUS) {
> + struct nlattr *af;
> + int err;
> +
> + if (!br_cfm_created(br) || port)
> + goto done;
> +
> + af = nla_nest_start_noflag(skb, IFLA_AF_SPEC);
> + if (!af)
> + goto nla_put_failure;
> +
> + rcu_read_lock();
> + err = br_cfm_status_fill_info(skb, br);
> + rcu_read_unlock();
> +
> + if (err)
> + goto nla_put_failure;
> +
> + nla_nest_end(skb, af);
> + }

I actually noticed this just now, you can't have multiple IFLA_AF_SPEC
attributes. It seems we already have that problem with MRP.
Since filter_mask is a mask one could request multiple rtext attributes.

> +
> done:
> nlmsg_end(skb, nlh);
> return 0;
> @@ -542,7 +585,9 @@ int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
>
> if (!port && !(filter_mask & RTEXT_FILTER_BRVLAN) &&
> !(filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED) &&
> - !(filter_mask & RTEXT_FILTER_MRP))
> + !(filter_mask & RTEXT_FILTER_MRP) &&
> + !(filter_mask & RTEXT_FILTER_CFM_CONFIG) &&
> + !(filter_mask & RTEXT_FILTER_CFM_STATUS))
> return 0;
>
> return br_fill_ifinfo(skb, port, pid, seq, RTM_NEWLINK, nlflags,
> @@ -704,6 +749,11 @@ static int br_afspec(struct net_bridge *br,
> if (err)
> return err;
> break;
> + case IFLA_BRIDGE_CFM:
> + err = br_cfm_parse(br, p, attr, cmd, extack);
> + if (err)
> + return err;
> + break;
> }
> }
>
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 385a6a6aa17e..fe36592f7525 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -1365,9 +1365,20 @@ static inline int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br)
>
> /* br_cfm.c */
> #if IS_ENABLED(CONFIG_BRIDGE_CFM)
> +int br_cfm_parse(struct net_bridge *br, struct net_bridge_port *p,
> + struct nlattr *attr, int cmd, struct netlink_ext_ack *extack);
> int br_cfm_rx_frame_process(struct net_bridge_port *p, struct sk_buff *skb);
> bool br_cfm_created(struct net_bridge *br);
> +int br_cfm_config_fill_info(struct sk_buff *skb, struct net_bridge *br);
> +int br_cfm_status_fill_info(struct sk_buff *skb, struct net_bridge *br);
> #else
> +static inline int br_cfm_parse(struct net_bridge *br, struct net_bridge_port *p,
> + struct nlattr *attr, int cmd,
> + struct netlink_ext_ack *extack)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> static inline int br_cfm_rx_frame_process(struct net_bridge_port *p, struct sk_buff *skb)
> {
> return -EOPNOTSUPP;
> @@ -1377,6 +1388,16 @@ static inline bool br_cfm_created(struct net_bridge *br)
> {
> return false;
> }
> +
> +static inline int br_cfm_config_fill_info(struct sk_buff *skb, struct net_bridge *br)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int br_cfm_status_fill_info(struct sk_buff *skb, struct net_bridge *br)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif
>
> /* br_netlink.c */