2020-10-13 02:52:35

by Henrik Bjoernlund

[permalink] [raw]
Subject: [PATCH net-next v5 00/10] net: bridge: cfm: Add support for Connectivity Fault Management(CFM)

Connectivity Fault Management (CFM) is defined in 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’s equipment.

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

The primary CFM protocol shims are called Maintenance Points (MPs).
A MP can be either a MEP or a MHF.
The MEP:
-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 terminates/validates CFM frames on its level.
-It can only exist on a port that is related to a bridge.
The MHF:
-It is the Maintenance Domain Intermediate Point
(MIP) Half Function (MHF) described in 802.1Q section 19.3.
-It is created on a specific level (1-7).
-It is extracting/injecting certain CFM frame on this level.
-It can only exist on a port that is related to a bridge.
-Currently not supported.

There are defined the following CFM protocol functions:
-Continuity Check
-Loopback. Currently not supported.
-Linktrace. Currently not supported.

This CFM component supports create/delete of MEP instances and configuration of
the different CFM protocols. Also status information can be fetched and delivered
through notification due to defect status change.

The user interacts with CFM using the 'cfm' user space client program,
the client talks with the kernel using netlink.

Any notification emitted by CFM from the kernel can be monitored in user space
by starting 'cfm_server' program.

Currently this 'cfm' and 'cfm_server' programs are standalone placed in a cfm
repository https://github.com/microchip-ung/cfm but it is considered to integrate
this into 'iproute2'.

v1 -> v2
Added the CFM switchdev interface and also added utilization by calling the
interface from the kernel CFM implementation trying to offload CFM functionality
to HW. This offload (CFM driver) is currently not implemented.

Corrections based on RCF comments:
-The single CFM kernel implementation Patch is broken up into three patches.
-Changed the list of MEP instances from list_head to hlist_head.
-Removed unnecessary RCU list traversing.
-Solved RCU unlocking problem.
-Removed unnecessary comments.
-Added ASSERT_RTNL() where required.
-Shaping up on error messages.
-Correction NETLINK br_fill_ifinfo() to be able to handle 'filter_mask'
with multiple flags asserted.

v2 -> v3
The switchdev definition and utilization has been removed as there was no
switchdev implementation.
Some compiling issues are fixed as Reported-by: kernel test robot <[email protected]>.

v3 -> v4
Fixed potential crash during hlist walk where elements are removed.
Giving all commits unique titles.
NETLINK implementation split into three commits.
Commit "bridge: cfm: Bridge port remove" is merged with
commit "bridge: cfm: Kernel space implementation of CFM. MEP create/delete."

v4 -> v5
Reordered members in struct net_bridge to bring member frame_type_list to the
first cache line.
Helper functions nla_get_mac() and nla_get_maid() are removed.
The NLA_POLICY_NESTED() macro is used to initialize the br_cfm_policy array.
Fixed reverse xmas tree.

Reviewed-by: Horatiu Vultur <[email protected]>
Signed-off-by: Henrik Bjoernlund <[email protected]>

Henrik Bjoernlund (10):
net: bridge: extend the process of special frames
bridge: cfm: Add BRIDGE_CFM to Kconfig.
bridge: uapi: cfm: Added EtherType used by the CFM protocol.
bridge: cfm: Kernel space implementation of CFM. MEP create/delete.
bridge: cfm: Kernel space implementation of CFM. CCM frame TX added.
bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.
bridge: cfm: Netlink SET configuration Interface.
bridge: cfm: Netlink GET configuration Interface.
bridge: cfm: Netlink GET status Interface.
bridge: cfm: Netlink Notifications.

include/uapi/linux/cfm_bridge.h | 70 +++
include/uapi/linux/if_bridge.h | 125 +++++
include/uapi/linux/if_ether.h | 1 +
include/uapi/linux/rtnetlink.h | 2 +
net/bridge/Kconfig | 11 +
net/bridge/Makefile | 2 +
net/bridge/br_cfm.c | 884 ++++++++++++++++++++++++++++++++
net/bridge/br_cfm_netlink.c | 726 ++++++++++++++++++++++++++
net/bridge/br_device.c | 4 +
net/bridge/br_if.c | 1 +
net/bridge/br_input.c | 33 +-
net/bridge/br_mrp.c | 19 +-
net/bridge/br_netlink.c | 115 ++++-
net/bridge/br_private.h | 77 ++-
net/bridge/br_private_cfm.h | 147 ++++++
15 files changed, 2194 insertions(+), 23 deletions(-)
create mode 100644 include/uapi/linux/cfm_bridge.h
create mode 100644 net/bridge/br_cfm.c
create mode 100644 net/bridge/br_cfm_netlink.c
create mode 100644 net/bridge/br_private_cfm.h

--
2.28.0


2020-10-13 03:30:51

by Henrik Bjoernlund

[permalink] [raw]
Subject: [PATCH net-next v5 06/10] bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.

This is the third commit of the implementation of the CFM protocol
according to 802.1Q section 12.14.

Functionality is extended with CCM frame reception.
The MEP instance now contains CCM based status information.
Most important is the CCM defect status indicating if correct
CCM frames are received with the expected interval.

Signed-off-by: Henrik Bjoernlund <[email protected]>
Reviewed-by: Horatiu Vultur <[email protected]>
Acked-by: Nikolay Aleksandrov <[email protected]>
---
include/uapi/linux/cfm_bridge.h | 10 ++
net/bridge/br_cfm.c | 269 ++++++++++++++++++++++++++++++++
net/bridge/br_private_cfm.h | 32 ++++
3 files changed, 311 insertions(+)

diff --git a/include/uapi/linux/cfm_bridge.h b/include/uapi/linux/cfm_bridge.h
index 84a3817da90b..4be195cc6b70 100644
--- a/include/uapi/linux/cfm_bridge.h
+++ b/include/uapi/linux/cfm_bridge.h
@@ -20,6 +20,10 @@
CFM_IF_STATUS_TLV_LENGTH)
#define CFM_FRAME_PRIO 7
#define CFM_CCM_TLV_OFFSET 70
+#define CFM_CCM_PDU_MAID_OFFSET 10
+#define CFM_CCM_PDU_MEPID_OFFSET 8
+#define CFM_CCM_PDU_SEQNR_OFFSET 4
+#define CFM_CCM_PDU_TLV_OFFSET 74
#define CFM_CCM_ITU_RESERVED_SIZE 16

struct br_cfm_common_hdr {
@@ -29,6 +33,12 @@ struct br_cfm_common_hdr {
__u8 tlv_offset;
};

+struct br_cfm_status_tlv {
+ __u8 type;
+ __be16 length;
+ __u8 value;
+};
+
enum br_cfm_opcodes {
BR_CFM_OPCODE_CCM = 0x1,
};
diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
index 4b7af1adcd6a..d928de5de155 100644
--- a/net/bridge/br_cfm.c
+++ b/net/bridge/br_cfm.c
@@ -101,6 +101,56 @@ static u32 interval_to_pdu(enum br_cfm_ccm_interval interval)
return 0;
}

+/* Convert the CCM PDU value to interval on interface. */
+static u32 pdu_to_interval(u32 value)
+{
+ switch (value) {
+ case 0:
+ return BR_CFM_CCM_INTERVAL_NONE;
+ case 1:
+ return BR_CFM_CCM_INTERVAL_3_3_MS;
+ case 2:
+ return BR_CFM_CCM_INTERVAL_10_MS;
+ case 3:
+ return BR_CFM_CCM_INTERVAL_100_MS;
+ case 4:
+ return BR_CFM_CCM_INTERVAL_1_SEC;
+ case 5:
+ return BR_CFM_CCM_INTERVAL_10_SEC;
+ case 6:
+ return BR_CFM_CCM_INTERVAL_1_MIN;
+ case 7:
+ return BR_CFM_CCM_INTERVAL_10_MIN;
+ }
+ return BR_CFM_CCM_INTERVAL_NONE;
+}
+
+static void ccm_rx_timer_start(struct br_cfm_peer_mep *peer_mep)
+{
+ u32 interval_us;
+
+ interval_us = interval_to_us(peer_mep->mep->cc_config.exp_interval);
+ /* Function ccm_rx_dwork must be called with 1/4
+ * of the configured CC 'expected_interval'
+ * in order to detect CCM defect after 3.25 interval.
+ */
+ queue_delayed_work(system_wq, &peer_mep->ccm_rx_dwork,
+ usecs_to_jiffies(interval_us / 4));
+}
+
+static void cc_peer_enable(struct br_cfm_peer_mep *peer_mep)
+{
+ memset(&peer_mep->cc_status, 0, sizeof(peer_mep->cc_status));
+ peer_mep->ccm_rx_count_miss = 0;
+
+ ccm_rx_timer_start(peer_mep);
+}
+
+static void cc_peer_disable(struct br_cfm_peer_mep *peer_mep)
+{
+ cancel_delayed_work_sync(&peer_mep->ccm_rx_dwork);
+}
+
static struct sk_buff *ccm_frame_build(struct br_cfm_mep *mep,
const struct br_cfm_cc_ccm_tx_info *const tx_info)

@@ -231,6 +281,200 @@ static void ccm_tx_work_expired(struct work_struct *work)
usecs_to_jiffies(interval_us));
}

+/* This function is called with 1/4 of the configured CC 'expected_interval'
+ * in order to detect CCM defect after 3.25 interval.
+ */
+static void ccm_rx_work_expired(struct work_struct *work)
+{
+ struct br_cfm_peer_mep *peer_mep;
+ struct delayed_work *del_work;
+
+ del_work = to_delayed_work(work);
+ peer_mep = container_of(del_work, struct br_cfm_peer_mep, ccm_rx_dwork);
+
+ /* After 13 counts (4 * 3,25) then 3.25 intervals are expired */
+ if (peer_mep->ccm_rx_count_miss < 13) {
+ /* 3.25 intervals are NOT expired without CCM reception */
+ peer_mep->ccm_rx_count_miss++;
+
+ /* Start timer again */
+ ccm_rx_timer_start(peer_mep);
+ } else {
+ /* 3.25 intervals are expired without CCM reception.
+ * CCM defect detected
+ */
+ peer_mep->cc_status.ccm_defect = true;
+ }
+}
+
+static u32 ccm_tlv_extract(struct sk_buff *skb, u32 index,
+ struct br_cfm_peer_mep *peer_mep)
+{
+ __be32 *s_tlv;
+ __be32 _s_tlv;
+ u32 h_s_tlv;
+ u8 *e_tlv;
+ u8 _e_tlv;
+
+ e_tlv = skb_header_pointer(skb, index, sizeof(_e_tlv), &_e_tlv);
+ if (!e_tlv)
+ return 0;
+
+ /* TLV is present - get the status TLV */
+ s_tlv = skb_header_pointer(skb,
+ index,
+ sizeof(_s_tlv), &_s_tlv);
+ if (!s_tlv)
+ return 0;
+
+ h_s_tlv = ntohl(*s_tlv);
+ if ((h_s_tlv >> 24) == CFM_IF_STATUS_TLV_TYPE) {
+ /* Interface status TLV */
+ peer_mep->cc_status.tlv_seen = true;
+ peer_mep->cc_status.if_tlv_value = (h_s_tlv & 0xFF);
+ }
+
+ if ((h_s_tlv >> 24) == CFM_PORT_STATUS_TLV_TYPE) {
+ /* Port status TLV */
+ peer_mep->cc_status.tlv_seen = true;
+ peer_mep->cc_status.port_tlv_value = (h_s_tlv & 0xFF);
+ }
+
+ /* The Sender ID TLV is not handled */
+ /* The Organization-Specific TLV is not handled */
+
+ /* Return the length of this tlv.
+ * This is the length of the value field plus 3 bytes for size of type
+ * field and length field
+ */
+ return ((h_s_tlv >> 8) & 0xFFFF) + 3;
+}
+
+/* note: already called with rcu_read_lock */
+static int br_cfm_frame_rx(struct net_bridge_port *port, struct sk_buff *skb)
+{
+ u32 mdlevel, interval, size, index, max;
+ const struct br_cfm_common_hdr *hdr;
+ struct br_cfm_peer_mep *peer_mep;
+ const struct br_cfm_maid *maid;
+ struct br_cfm_common_hdr _hdr;
+ struct br_cfm_maid _maid;
+ struct br_cfm_mep *mep;
+ struct net_bridge *br;
+ __be32 *snumber;
+ __be32 _snumber;
+ __be16 *mepid;
+ __be16 _mepid;
+
+ if (port->state == BR_STATE_DISABLED)
+ return 0;
+
+ hdr = skb_header_pointer(skb, 0, sizeof(_hdr), &_hdr);
+ if (!hdr)
+ return 1;
+
+ br = port->br;
+ mep = br_mep_find_ifindex(br, port->dev->ifindex);
+ if (unlikely(!mep))
+ /* No MEP on this port - must be forwarded */
+ return 0;
+
+ mdlevel = hdr->mdlevel_version >> 5;
+ if (mdlevel > mep->config.mdlevel)
+ /* The level is above this MEP level - must be forwarded */
+ return 0;
+
+ if ((hdr->mdlevel_version & 0x1F) != 0) {
+ /* Invalid version */
+ mep->status.version_unexp_seen = true;
+ return 1;
+ }
+
+ if (mdlevel < mep->config.mdlevel) {
+ /* The level is below this MEP level */
+ mep->status.rx_level_low_seen = true;
+ return 1;
+ }
+
+ if (hdr->opcode == BR_CFM_OPCODE_CCM) {
+ /* CCM PDU received. */
+ /* MA ID is after common header + sequence number + MEP ID */
+ maid = skb_header_pointer(skb,
+ CFM_CCM_PDU_MAID_OFFSET,
+ sizeof(_maid), &_maid);
+ if (!maid)
+ return 1;
+ if (memcmp(maid->data, mep->cc_config.exp_maid.data,
+ sizeof(maid->data)))
+ /* MA ID not as expected */
+ return 1;
+
+ /* MEP ID is after common header + sequence number */
+ mepid = skb_header_pointer(skb,
+ CFM_CCM_PDU_MEPID_OFFSET,
+ sizeof(_mepid), &_mepid);
+ if (!mepid)
+ return 1;
+ peer_mep = br_peer_mep_find(mep, (u32)ntohs(*mepid));
+ if (!peer_mep)
+ return 1;
+
+ /* Interval is in common header flags */
+ interval = hdr->flags & 0x07;
+ if (mep->cc_config.exp_interval != pdu_to_interval(interval))
+ /* Interval not as expected */
+ return 1;
+
+ /* A valid CCM frame is received */
+ if (peer_mep->cc_status.ccm_defect) {
+ peer_mep->cc_status.ccm_defect = false;
+
+ /* Start CCM RX timer */
+ ccm_rx_timer_start(peer_mep);
+ }
+
+ peer_mep->cc_status.seen = true;
+ peer_mep->ccm_rx_count_miss = 0;
+
+ /* RDI is in common header flags */
+ peer_mep->cc_status.rdi = (hdr->flags & 0x80) ? true : false;
+
+ /* Sequence number is after common header */
+ snumber = skb_header_pointer(skb,
+ CFM_CCM_PDU_SEQNR_OFFSET,
+ sizeof(_snumber), &_snumber);
+ if (!snumber)
+ return 1;
+ if (ntohl(*snumber) != (mep->ccm_rx_snumber + 1))
+ /* Unexpected sequence number */
+ peer_mep->cc_status.seq_unexp_seen = true;
+
+ mep->ccm_rx_snumber = ntohl(*snumber);
+
+ /* TLV end is after common header + sequence number + MEP ID +
+ * MA ID + ITU reserved
+ */
+ index = CFM_CCM_PDU_TLV_OFFSET;
+ max = 0;
+ do { /* Handle all TLVs */
+ size = ccm_tlv_extract(skb, index, peer_mep);
+ index += size;
+ max += 1;
+ } while (size != 0 && max < 4); /* Max four TLVs possible */
+
+ return 1;
+ }
+
+ mep->status.opcode_unexp_seen = true;
+
+ return 1;
+}
+
+static struct br_frame_type cfm_frame_type __read_mostly = {
+ .type = cpu_to_be16(ETH_P_CFM),
+ .frame_handler = br_cfm_frame_rx,
+};
+
int br_cfm_mep_create(struct net_bridge *br,
const u32 instance,
struct br_cfm_mep_create *const create,
@@ -295,6 +539,9 @@ int br_cfm_mep_create(struct net_bridge *br,
INIT_HLIST_HEAD(&mep->peer_mep_list);
INIT_DELAYED_WORK(&mep->ccm_tx_dwork, ccm_tx_work_expired);

+ if (hlist_empty(&br->mep_list))
+ br_add_frame(br, &cfm_frame_type);
+
hlist_add_tail_rcu(&mep->head, &br->mep_list);

return 0;
@@ -310,6 +557,7 @@ static void mep_delete_implementation(struct net_bridge *br,

/* Empty and free peer MEP list */
hlist_for_each_entry_safe(peer_mep, n_store, &mep->peer_mep_list, head) {
+ cancel_delayed_work_sync(&peer_mep->ccm_rx_dwork);
hlist_del_rcu(&peer_mep->head);
kfree_rcu(peer_mep, rcu);
}
@@ -319,6 +567,9 @@ static void mep_delete_implementation(struct net_bridge *br,
RCU_INIT_POINTER(mep->b_port, NULL);
hlist_del_rcu(&mep->head);
kfree_rcu(mep, rcu);
+
+ if (hlist_empty(&br->mep_list))
+ br_del_frame(br, &cfm_frame_type);
}

int br_cfm_mep_delete(struct net_bridge *br,
@@ -379,6 +630,7 @@ int br_cfm_cc_config_set(struct net_bridge *br,
const struct br_cfm_cc_config *const config,
struct netlink_ext_ack *extack)
{
+ struct br_cfm_peer_mep *peer_mep;
struct br_cfm_mep *mep;

ASSERT_RTNL();
@@ -394,7 +646,18 @@ int br_cfm_cc_config_set(struct net_bridge *br,
if (memcmp(config, &mep->cc_config, sizeof(*config)) == 0)
return 0;

+ if (config->enable && !mep->cc_config.enable)
+ /* CC is enabled */
+ hlist_for_each_entry(peer_mep, &mep->peer_mep_list, head)
+ cc_peer_enable(peer_mep);
+
+ if (!config->enable && mep->cc_config.enable)
+ /* CC is disabled */
+ hlist_for_each_entry(peer_mep, &mep->peer_mep_list, head)
+ cc_peer_disable(peer_mep);
+
mep->cc_config = *config;
+ mep->ccm_rx_snumber = 0;
mep->ccm_tx_snumber = 1;

return 0;
@@ -435,6 +698,10 @@ int br_cfm_cc_peer_mep_add(struct net_bridge *br, const u32 instance,

peer_mep->mepid = mepid;
peer_mep->mep = mep;
+ INIT_DELAYED_WORK(&peer_mep->ccm_rx_dwork, ccm_rx_work_expired);
+
+ if (mep->cc_config.enable)
+ cc_peer_enable(peer_mep);

hlist_add_tail_rcu(&peer_mep->head, &mep->peer_mep_list);

@@ -464,6 +731,8 @@ int br_cfm_cc_peer_mep_remove(struct net_bridge *br, const u32 instance,
return -ENOENT;
}

+ cc_peer_disable(peer_mep);
+
hlist_del_rcu(&peer_mep->head);
kfree_rcu(peer_mep, rcu);

diff --git a/net/bridge/br_private_cfm.h b/net/bridge/br_private_cfm.h
index 8d1b449acfbf..6a2294c0ea79 100644
--- a/net/bridge/br_private_cfm.h
+++ b/net/bridge/br_private_cfm.h
@@ -43,6 +43,8 @@ struct br_cfm_cc_config {
/* Expected received CCM PDU interval. */
/* Transmitting CCM PDU interval when CCM tx is enabled. */
enum br_cfm_ccm_interval exp_interval;
+
+ bool enable; /* Enable/disable CCM PDU handling */
};

int br_cfm_cc_config_set(struct net_bridge *br,
@@ -87,6 +89,31 @@ int br_cfm_cc_ccm_tx(struct net_bridge *br, const u32 instance,
const struct br_cfm_cc_ccm_tx_info *const tx_info,
struct netlink_ext_ack *extack);

+struct br_cfm_mep_status {
+ /* Indications that an OAM PDU has been seen. */
+ bool opcode_unexp_seen; /* RX of OAM PDU with unexpected opcode */
+ bool version_unexp_seen; /* RX of OAM PDU with unexpected version */
+ bool rx_level_low_seen; /* Rx of OAM PDU with level low */
+};
+
+struct br_cfm_cc_peer_status {
+ /* This CCM related status is based on the latest received CCM PDU. */
+ u8 port_tlv_value; /* Port Status TLV value */
+ u8 if_tlv_value; /* Interface Status TLV value */
+
+ /* CCM has not been received for 3.25 intervals */
+ bool ccm_defect;
+
+ /* (RDI == 1) for last received CCM PDU */
+ bool rdi;
+
+ /* Indications that a CCM PDU has been seen. */
+ bool seen; /* CCM PDU received */
+ bool tlv_seen; /* CCM PDU with TLV received */
+ /* CCM PDU with unexpected sequence number received */
+ bool seq_unexp_seen;
+};
+
struct br_cfm_mep {
/* list header of MEP instances */
struct hlist_node head;
@@ -101,6 +128,8 @@ struct br_cfm_mep {
unsigned long ccm_tx_end;
struct delayed_work ccm_tx_dwork;
u32 ccm_tx_snumber;
+ u32 ccm_rx_snumber;
+ struct br_cfm_mep_status status;
bool rdi;
struct rcu_head rcu;
};
@@ -108,7 +137,10 @@ struct br_cfm_mep {
struct br_cfm_peer_mep {
struct hlist_node head;
struct br_cfm_mep *mep;
+ struct delayed_work ccm_rx_dwork;
u32 mepid;
+ struct br_cfm_cc_peer_status cc_status;
+ u32 ccm_rx_count_miss;
struct rcu_head rcu;
};

--
2.28.0

2020-10-15 07:24:57

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v5 06/10] bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.

On Mon, 12 Oct 2020 14:04:24 +0000 Henrik Bjoernlund wrote:
> +struct br_cfm_status_tlv {
> + __u8 type;
> + __be16 length;
> + __u8 value;
> +};

This structure is unused (and likely not what you want, since it will
have 2 1 byte while unless you mark length as __packed).

2020-10-15 07:27:40

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v5 06/10] bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.

On Mon, 12 Oct 2020 14:04:24 +0000 Henrik Bjoernlund wrote:
> + /* This CCM related status is based on the latest received CCM PDU. */
> + u8 port_tlv_value; /* Port Status TLV value */
> + u8 if_tlv_value; /* Interface Status TLV value */
> +
> + /* CCM has not been received for 3.25 intervals */
> + bool ccm_defect;
> +
> + /* (RDI == 1) for last received CCM PDU */
> + bool rdi;
> +
> + /* Indications that a CCM PDU has been seen. */
> + bool seen; /* CCM PDU received */
> + bool tlv_seen; /* CCM PDU with TLV received */
> + /* CCM PDU with unexpected sequence number received */
> + bool seq_unexp_seen;

Please consider using a u8 bitfield rather than a bunch of bools,
if any of this structures are expected to have many instances.
That'd save space.

2020-10-15 07:56:30

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v5 00/10] net: bridge: cfm: Add support for Connectivity Fault Management(CFM)

On Mon, 12 Oct 2020 14:04:18 +0000 Henrik Bjoernlund wrote:
> Connectivity Fault Management (CFM) is defined in 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’s equipment.

Please wrap the cover letter and commit messages to 70 chars.

> Reviewed-by: Horatiu Vultur <[email protected]>
> Signed-off-by: Henrik Bjoernlund <[email protected]>

You have two spaces after the name in many tags.

2020-10-15 12:20:36

by Henrik Bjoernlund

[permalink] [raw]
Subject: Re: [PATCH net-next v5 06/10] bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.

Thanks for your review. Comments below.
Regards
Henrik

The 10/14/2020 16:26, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Mon, 12 Oct 2020 14:04:24 +0000 Henrik Bjoernlund wrote:
> > + /* This CCM related status is based on the latest received CCM PDU. */
> > + u8 port_tlv_value; /* Port Status TLV value */
> > + u8 if_tlv_value; /* Interface Status TLV value */
> > +
> > + /* CCM has not been received for 3.25 intervals */
> > + bool ccm_defect;
> > +
> > + /* (RDI == 1) for last received CCM PDU */
> > + bool rdi;
> > +
> > + /* Indications that a CCM PDU has been seen. */
> > + bool seen; /* CCM PDU received */
> > + bool tlv_seen; /* CCM PDU with TLV received */
> > + /* CCM PDU with unexpected sequence number received */
> > + bool seq_unexp_seen;
>
> Please consider using a u8 bitfield rather than a bunch of bools,
> if any of this structures are expected to have many instances.
> That'd save space.

I have changed as requested.

--
/Henrik

2020-10-15 12:23:23

by Henrik Bjoernlund

[permalink] [raw]
Subject: Re: [PATCH net-next v5 06/10] bridge: cfm: Kernel space implementation of CFM. CCM frame RX added.

Thanks for your review. Comments below.
Regards
Henrik

The 10/14/2020 16:16, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Mon, 12 Oct 2020 14:04:24 +0000 Henrik Bjoernlund wrote:
> > +struct br_cfm_status_tlv {
> > + __u8 type;
> > + __be16 length;
> > + __u8 value;
> > +};
>
> This structure is unused (and likely not what you want, since it will
> have 2 1 byte while unless you mark length as __packed).

I have changed as requested.

--
/Henrik

2020-10-15 17:59:33

by Henrik Bjoernlund

[permalink] [raw]
Subject: Re: [PATCH net-next v5 00/10] net: bridge: cfm: Add support for Connectivity Fault Management(CFM)

Thanks for your review. Comments below.
Regards
Henrik

The 10/14/2020 15:58, Jakub Kicinski wrote:>
> On Mon, 12 Oct 2020 14:04:18 +0000 Henrik Bjoernlund wrote:
> > Connectivity Fault Management (CFM) is defined in 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’s equipment.
>
> Please wrap the cover letter and commit messages to 70 chars.
>

I will do that,

> > Reviewed-by: Horatiu Vultur <[email protected]>
> > Signed-off-by: Henrik Bjoernlund <[email protected]>
>
> You have two spaces after the name in many tags.

I will change as requested.

--
/Henrik