Re: [Bridge] [PATCH RFC 4/7] bridge: cfm: Kernel space implementation of CFM.
Thanks for the review. Comments below. The 09/04/2020 08:02, Jakub Kicinski wrote: > > On Fri, 4 Sep 2020 09:15:24 + Henrik Bjoernlund wrote: > > + rcu_read_lock(); > > + b_port = rcu_dereference(mep->b_port); > > + if (!b_port) > > + return NULL; > > + skb = dev_alloc_skb(CFM_CCM_MAX_FRAME_LENGTH); > > + if (!skb) > > + return NULL; > > net/bridge/br_cfm.c:171:23: warning: context imbalance in 'ccm_frame_build' - > different lock contexts for basic block I will assure that rcu_read_unlock() is called before any return. -- /Henrik
Re: [Bridge] [PATCH RFC 6/7] bridge: cfm: Netlink Notifications.
Thanks for the review. Comments below. The 09/08/2020 13:54, Nikolay Aleksandrov wrote: > > On Fri, 2020-09-04 at 09:15 +, Henrik Bjoernlund wrote: > > This is the implementation of Netlink notifications out of CFM. > > > > Notifications are initiated whenever a state change happens in CFM. > > > [snip] > > + *count = 0; > > + > > + rcu_read_lock(); > > + list_for_each_entry_rcu(mep, &br->mep_list, head) > > + * count += 1; > > please remove the extra space > I have removed the extra space. This space was added to satify checkpatch as without this space it gives this error: CHECK: spaces preferred around that '*' (ctx:ExV) #136: FILE: net/bridge/br_cfm.c:883: + *count += 1; ^ > > + rcu_read_unlock(); > > + > > + return 0; > > +} > > + > > -- /Henrik
[Bridge] [PATCH RFC 6/7] bridge: cfm: Netlink Notifications.
This is the implementation of Netlink notifications out of CFM.
Notifications are initiated whenever a state change happens in CFM.
IFLA_BRIDGE_CFM:
Points to the CFM information.
IFLA_BRIDGE_CFM_MEP_STATUS_INFO:
This indicate that the MEP instance status are following.
IFLA_BRIDGE_CFM_CC_PEER_STATUS_INFO:
This indicate that the peer MEP status are following.
CFM nested attribute has the following attributes in next level.
IFLA_BRIDGE_CFM_MEP_STATUS_INSTANCE:
The MEP instance number of the delivered status.
The type is NLA_U32.
IFLA_BRIDGE_CFM_MEP_STATUS_OPCODE_UNEXP_SEEN:
The MEP instance received CFM PDU with unexpected Opcode.
The type is NLA_U32 (bool).
IFLA_BRIDGE_CFM_MEP_STATUS_VERSION_UNEXP_SEEN:
The MEP instance received CFM PDU with unexpected version.
The type is NLA_U32 (bool).
IFLA_BRIDGE_CFM_MEP_STATUS_RX_LEVEL_LOW_SEEN:
The MEP instance received CCM PDU with MD level lower than
configured level. This frame is discarded.
The type is NLA_U32 (bool).
IFLA_BRIDGE_CFM_CC_PEER_STATUS_INSTANCE:
The MEP instance number of the delivered status.
The type is NLA_U32.
IFLA_BRIDGE_CFM_CC_PEER_STATUS_PEER_MEPID:
The added Peer MEP ID of the delivered status.
The type is NLA_U32.
IFLA_BRIDGE_CFM_CC_PEER_STATUS_CCM_DEFECT:
The CCM defect status.
The type is NLA_U32 (bool).
True means no CCM frame is received for 3.25 intervals.
IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL.
IFLA_BRIDGE_CFM_CC_PEER_STATUS_RDI:
The last received CCM PDU RDI.
The type is NLA_U32 (bool).
IFLA_BRIDGE_CFM_CC_PEER_STATUS_PORT_TLV_VALUE:
The last received CCM PDU Port Status TLV value field.
The type is NLA_U8.
IFLA_BRIDGE_CFM_CC_PEER_STATUS_IF_TLV_VALUE:
The last received CCM PDU Interface Status TLV value field.
The type is NLA_U8.
IFLA_BRIDGE_CFM_CC_PEER_STATUS_SEEN:
A CCM frame has been received from Peer MEP.
The type is NLA_U32 (bool).
This is cleared after GETLINK IFLA_BRIDGE_CFM_CC_PEER_STATUS_INFO.
IFLA_BRIDGE_CFM_CC_PEER_STATUS_TLV_SEEN:
A CCM frame with TLV has been received from Peer MEP.
The type is NLA_U32 (bool).
This is cleared after GETLINK IFLA_BRIDGE_CFM_CC_PEER_STATUS_INFO.
IFLA_BRIDGE_CFM_CC_PEER_STATUS_SEQ_UNEXP_SEEN:
A CCM frame with unexpected sequence number has been received
from Peer MEP.
The type is NLA_U32 (bool).
When a sequence number is not one higher than previously received
then it is unexpected.
This is cleared after GETLINK IFLA_BRIDGE_CFM_CC_PEER_STATUS_INFO.
Signed-off-by: Henrik Bjoernlund
---
net/bridge/br_cfm.c | 43 +
net/bridge/br_cfm_netlink.c | 24 +++-
net/bridge/br_netlink.c | 76 -
net/bridge/br_private.h | 22 ++-
4 files changed, 144 insertions(+), 21 deletions(-)
diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
index e38cc3e8f262..b7fed2c1d8ec 100644
--- a/net/bridge/br_cfm.c
+++ b/net/bridge/br_cfm.c
@@ -155,6 +155,13 @@ static void ccm_rx_timer_start(struct br_cfm_peer_mep
*peer_mep)
usecs_to_jiffies(interval_us / 4));
}
+static void br_cfm_notify(int event, const struct net_bridge_port *port)
+{
+ u32 filter = RTEXT_FILTER_CFM_STATUS;
+
+ return br_info_notify(event, port->br, NULL, filter);
+}
+
static void cc_peer_enable(struct br_cfm_peer_mep *peer_mep)
{
memset(&peer_mep->cc_status, 0, sizeof(peer_mep->cc_status));
@@ -299,6 +306,7 @@ static void ccm_tx_work_expired(struct work_struct *work)
static void ccm_rx_work_expired(struct work_struct *work)
{
struct br_cfm_peer_mep *peer_mep;
+ struct net_bridge_port *b_port;
struct delayed_work *del_work;
del_work = to_delayed_work(work);
@@ -318,6 +326,11 @@ static void ccm_rx_work_expired(struct work_struct *work)
peer_mep->cc_status.ccm_defect = true;
/* Change in CCM defect status - notify */
+ rcu_read_lock();
+ b_port = rcu_dereference(peer_mep->mep->b_port);
+ if (b_port)
+ br_cfm_notify(RTM_NEWLINK, b_port);
+ rcu_read_unlock();
}
}
@@ -445,6 +458,7 @@ static int br_cfm_frame_rx(struct net_bridge_port *port,
struct sk_buff *skb)
peer_mep->cc_status.ccm_defect = false;
/* Change in CCM defect status - notify */
+ br_cfm_notify(RTM_NEWLINK, port);
/* Start CCM RX timer */
ccm_rx_timer_start(peer_mep);
@@ -874,6 +888,35 @@ int br_cfm_cc_counters_clear(struct net_bridge *br, const
u32 instance,
return 0;
}
+int br_cfm_mep_count(struct net_bridge *br, u32 *count)
+{
+ struct br_cfm_mep *mep;
+ *count = 0;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(mep, &br->mep_list, head)
+ * count += 1;
+ rcu_read_
[Bridge] [PATCH RFC 0/7] 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. The kernel will try to offload the requests to the HW via switchdev API (not implemented yet). 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'. Reviewed-by: Horatiu Vultur Signed-off-by: Henrik Bjoernlund Henrik Bjoernlund (7): 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. bridge: cfm: Netlink Interface. bridge: cfm: Netlink Notifications. bridge: cfm: Bridge port remove. include/uapi/linux/cfm_bridge.h | 75 +++ 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 | 936 net/bridge/br_cfm_netlink.c | 690 +++ net/bridge/br_device.c | 4 + net/bridge/br_if.c | 1 + net/bridge/br_input.c | 31 +- net/bridge/br_mrp.c | 19 +- net/bridge/br_netlink.c | 126 - net/bridge/br_private.h | 82 ++- net/bridge/br_private_cfm.h | 242 + 15 files changed, 2326 insertions(+), 21 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
[Bridge] [PATCH RFC 7/7] bridge: cfm: Bridge port remove.
This is addition of CFM functionality to delete MEP instances
on a port that is removed from the bridge.
A MEP can only exist on a port that is related to a bridge.
Signed-off-by: Henrik Bjoernlund
---
net/bridge/br_cfm.c | 13 +
net/bridge/br_if.c | 1 +
net/bridge/br_private.h | 6 ++
3 files changed, 20 insertions(+)
diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
index b7fed2c1d8ec..c724ce020ce3 100644
--- a/net/bridge/br_cfm.c
+++ b/net/bridge/br_cfm.c
@@ -921,3 +921,16 @@ bool br_cfm_created(struct net_bridge *br)
{
return !list_empty(&br->mep_list);
}
+
+/* Deletes the CFM instances on a specific bridge port
+ * note: called under rtnl_lock
+ */
+void br_cfm_port_del(struct net_bridge *br, struct net_bridge_port *port)
+{
+ struct br_cfm_mep *mep;
+
+ list_for_each_entry_rcu(mep, &br->mep_list, head,
+ lockdep_rtnl_is_held())
+ if (mep->create.ifindex == port->dev->ifindex)
+ mep_delete_implementation(br, mep);
+}
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index a0e9a7937412..f7d2f472ae24 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -334,6 +334,7 @@ static void del_nbp(struct net_bridge_port *p)
spin_unlock_bh(&br->lock);
br_mrp_port_del(br, p);
+ br_cfm_port_del(br, p);
br_ifinfo_notify(RTM_DELLINK, NULL, p);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 53bcbdd21f34..5617255f0c0c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1369,6 +1369,7 @@ 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);
+void br_cfm_port_del(struct net_bridge *br, struct net_bridge_port *p);
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,
@@ -1393,6 +1394,11 @@ static inline bool br_cfm_created(struct net_bridge *br)
return false;
}
+static inline void br_cfm_port_del(struct net_bridge *br,
+ struct net_bridge_port *p)
+{
+}
+
static inline int br_cfm_config_fill_info(struct sk_buff *skb, struct
net_bridge *br)
{
return -EOPNOTSUPP;
--
2.28.0
Re: [Bridge] [PATCH RFC 6/7] bridge: cfm: Netlink Notifications.
Thanks for the review. Comments below.
The 09/08/2020 13:54, Nikolay Aleksandrov wrote:
>
> On Fri, 2020-09-04 at 09:15 +, Henrik Bjoernlund wrote:
> > This is the implementation of Netlink notifications out of CFM.
> >
> > Notifications are initiated whenever a state change happens in CFM.
> >
> [snip]
> > @@ -445,6 +458,7 @@ static int br_cfm_frame_rx(struct net_bridge_port
> > *port, struct sk_buff *skb)
> > peer_mep->cc_status.ccm_defect = false;
> >
> > /* Change in CCM defect status - notify */
> > + br_cfm_notify(RTM_NEWLINK, port);
> >
> > /* Start CCM RX timer */
> > ccm_rx_timer_start(peer_mep);
> > @@ -874,6 +888,35 @@ int br_cfm_cc_counters_clear(struct net_bridge *br,
> > const u32 instance,
> > return 0;
> > }
> >
> > +int br_cfm_mep_count(struct net_bridge *br, u32 *count)
> > +{
> > + struct br_cfm_mep *mep;
>
> Leave a blank line between local variable definitions and code.
>
I will change that as suggested.
> > + *count = 0;
> > +
> > + rcu_read_lock();
> > + list_for_each_entry_rcu(mep, &br->mep_list, head)
> > + * count += 1;
>
> please remove the extra space
>
I will change that as suggested.
> > + rcu_read_unlock();
> > +
> > + return 0;
> > +}
> > +
> > +int br_cfm_peer_mep_count(struct net_bridge *br, u32 *count)
> > +{
> > + struct br_cfm_peer_mep *peer_mep;
> > + struct br_cfm_mep *mep;
>
> Leave a blank line between local variable definitions and code.
>
I will change that as suggested.
> > + *count = 0;
> > +
> > + rcu_read_lock();
> > + list_for_each_entry_rcu(mep, &br->mep_list, head) {
> > + list_for_each_entry_rcu(peer_mep, &mep->peer_mep_list, head)
> > + * count += 1;
>
> please remove the extra space
>
I will change that as suggested.
> > + }
> > + rcu_read_unlock();
> > +
> > + return 0;
> > +}
> > +
> > bool br_cfm_created(struct net_bridge *br)
> > {
> > return !list_empty(&br->mep_list);
> > diff --git a/net/bridge/br_cfm_netlink.c b/net/bridge/br_cfm_netlink.c
> > index 4e39aab1cd0b..13664ac8608a 100644
> > --- a/net/bridge/br_cfm_netlink.c
> > +++ b/net/bridge/br_cfm_netlink.c
> > @@ -582,7 +582,9 @@ int br_cfm_config_fill_info(struct sk_buff *skb, struct
> > net_bridge *br)
> > return -EMSGSIZE;
> > }
> >
> > -int br_cfm_status_fill_info(struct sk_buff *skb, struct net_bridge *br)
> > +int br_cfm_status_fill_info(struct sk_buff *skb,
> > + struct net_bridge *br,
> > + bool getlink)
> > {
> > struct nlattr *tb, *cfm_tb;
> > struct br_cfm_mep *mep;
> > @@ -613,10 +615,12 @@ int br_cfm_status_fill_info(struct sk_buff *skb,
> > struct net_bridge *br)
> > 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;
> > + if (getlink) { /* Only clear if this is a GETLINK */
> > + /* 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);
> >
> > @@ -662,10 +666,12 @@ int br_cfm_status_fill_info(struct sk_buff *skb,
> > struct net_bridge *br)
> > 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;
> > + if (getlink) { /* Only clear if this is a GETLINK */
> > + /* 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;
>
> Why clear these on GETLINK? This sounds like it should be a set op.
>
The idea is that when getting the CFM status any '_seen' indications are
cleared so that they are giving information about what was seen since
last get.
I assume this is a get/clear atomic operations as rcu is held.
If you think this must be changed to a seperate clear_set operation I
will ofc do it.
> > + }
> >
> > nla_nest_end(skb, tb);
> > }
> > diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> >
[Bridge] [PATCH RFC 1/7] net: bridge: extend the process of special frames
This patch extends the processing of frames in the bridge. Currently MRP
frames needs special processing and the current implementation doesn't
allow a nice way to process different frame types. Therefore try to
improve this by adding a list that contains frame types that need
special processing. This list is iterated for each input frame and if
there is a match based on frame type then these functions will be called
and decide what to do with the frame. It can process the frame then the
bridge doesn't need to do anything or don't process so then the bridge
will do normal forwarding.
Signed-off-by: Henrik Bjoernlund
---
net/bridge/br_device.c | 1 +
net/bridge/br_input.c | 31 ++-
net/bridge/br_mrp.c | 19 +++
net/bridge/br_private.h | 18 --
4 files changed, 58 insertions(+), 11 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 9a2fb4aa1a10..a9232db03108 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -473,6 +473,7 @@ void br_dev_setup(struct net_device *dev)
spin_lock_init(&br->lock);
INIT_LIST_HEAD(&br->port_list);
INIT_HLIST_HEAD(&br->fdb_list);
+ INIT_LIST_HEAD(&br->ftype_list);
#if IS_ENABLED(CONFIG_BRIDGE_MRP)
INIT_LIST_HEAD(&br->mrp_list);
#endif
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 59a318b9f646..0f475b21094c 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -254,6 +254,21 @@ static int nf_hook_bridge_pre(struct sk_buff *skb, struct
sk_buff **pskb)
return RX_HANDLER_CONSUMED;
}
+/* Return 0 if the frame was not processed otherwise 1
+ * note: already called with rcu_read_lock
+ */
+static int br_process_frame_type(struct net_bridge_port *p,
+struct sk_buff *skb)
+{
+ struct br_frame_type *tmp;
+
+ list_for_each_entry_rcu(tmp, &p->br->ftype_list, list) {
+ if (unlikely(tmp->type == skb->protocol))
+ return tmp->func(p, skb);
+ }
+ return 0;
+}
+
/*
* Return NULL if skb is handled
* note: already called with rcu_read_lock
@@ -343,7 +358,7 @@ static rx_handler_result_t br_handle_frame(struct sk_buff
**pskb)
}
}
- if (unlikely(br_mrp_process(p, skb)))
+ if (unlikely(br_process_frame_type(p, skb)))
return RX_HANDLER_PASS;
forward:
@@ -380,3 +395,17 @@ rx_handler_func_t *br_get_rx_handler(const struct
net_device *dev)
return br_handle_frame;
}
+
+void br_add_frame(struct net_bridge *br, struct br_frame_type *ft)
+{
+ list_add_rcu(&ft->list, &br->ftype_list);
+}
+
+void br_del_frame(struct net_bridge *br, struct br_frame_type *ft)
+{
+ struct br_frame_type *tmp;
+
+ list_for_each_entry(tmp, &br->ftype_list, list)
+ if (ft == tmp)
+ list_del_rcu(&ft->list);
+}
diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
index b36689e6e7cb..0428e1785041 100644
--- a/net/bridge/br_mrp.c
+++ b/net/bridge/br_mrp.c
@@ -6,6 +6,13 @@
static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1 };
static const u8 mrp_in_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x3
};
+static int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb);
+
+static struct br_frame_type mrp_frame_type __read_mostly = {
+ .type = cpu_to_be16(ETH_P_MRP),
+ .func = br_mrp_process,
+};
+
static bool br_mrp_is_ring_port(struct net_bridge_port *p_port,
struct net_bridge_port *s_port,
struct net_bridge_port *port)
@@ -445,6 +452,9 @@ static void br_mrp_del_impl(struct net_bridge *br, struct
br_mrp *mrp)
list_del_rcu(&mrp->list);
kfree_rcu(mrp, rcu);
+
+ if (list_empty(&br->mrp_list))
+ br_del_frame(br, &mrp_frame_type);
}
/* Adds a new MRP instance.
@@ -493,6 +503,9 @@ int br_mrp_add(struct net_bridge *br, struct
br_mrp_instance *instance)
spin_unlock_bh(&br->lock);
rcu_assign_pointer(mrp->s_port, p);
+ if (list_empty(&br->mrp_list))
+ br_add_frame(br, &mrp_frame_type);
+
INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
INIT_DELAYED_WORK(&mrp->in_test_work, br_mrp_in_test_work_expired);
list_add_tail_rcu(&mrp->list, &br->mrp_list);
@@ -1172,15 +1185,13 @@ static int br_mrp_rcv(struct net_bridge_port *p,
* normal forwarding.
* note: already called with rcu_read_lock
*/
-int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb)
+static int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb)
{
/* If there is no MRP instance do normal forwarding */
if (likely(!(p->flags & BR_MRP_AWARE)))
goto out;
- if (unlikely(skb->protocol == htons(ETH_P_MRP)))
- return br_mrp_rcv(p, skb, p->dev);
-
+ return br_mrp_rcv(p,
[Bridge] [PATCH RFC 5/7] bridge: cfm: Netlink Interface.
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. SETLINK: IFLA_BRIDGE_CFM: Indicate that the following attributes are CFM. IFLA_BRIDGE_CFM_MEP_CREATE: This indicate that a MEP instance must be created. IFLA_BRIDGE_CFM_MEP_DELETE: This indicate that a MEP instance must be deleted. IFLA_BRIDGE_CFM_MEP_CONFIG: This indicate that a MEP instance must be configured. IFLA_BRIDGE_CFM_CC_CONFIG: This indicate that a MEP instance Continuity Check (CC) functionality must be configured. IFLA_BRIDGE_CFM_CC_PEER_MEP_ADD: This indicate that a CC Peer MEP must be added. IFLA_BRIDGE_CFM_CC_PEER_MEP_REMOVE: This indicate that a CC Peer MEP must be removed. IFLA_BRIDGE_CFM_CC_CCM_TX: This indicate that the CC transmitted CCM PDU must be configured. IFLA_BRIDGE_CFM_CC_RDI: This indicate that the CC transmitted CCM PDU RDI must be configured. GETLINK: Request filter RTEXT_FILTER_CFM_CONFIG: Indicating that CFM configuration information must be delivered. IFLA_BRIDGE_CFM: Points to the CFM information. IFLA_BRIDGE_CFM_MEP_CREATE_INFO: This indicate that MEP instance create parameters are following. IFLA_BRIDGE_CFM_MEP_CONFIG_INFO: This indicate that MEP instance config parameters are following. IFLA_BRIDGE_CFM_CC_CONFIG_INFO: This indicate that MEP instance CC functionality parameters are following. IFLA_BRIDGE_CFM_CC_RDI_INFO: This indicate that CC transmitted CCM PDU RDI parameters are following. IFLA_BRIDGE_CFM_CC_CCM_TX_INFO: This indicate that CC transmitted CCM PDU parameters are following. IFLA_BRIDGE_CFM_CC_PEER_MEP_INFO: This indicate that the added peer MEP IDs are following. Request filter RTEXT_FILTER_CFM_STATUS: Indicating that CFM status information must be delivered. IFLA_BRIDGE_CFM: Points to the CFM information. IFLA_BRIDGE_CFM_MEP_STATUS_INFO: This indicate that the MEP instance status are following. IFLA_BRIDGE_CFM_CC_PEER_STATUS_INFO: This indicate that the peer MEP status are following. CFM nested attribute has the following attributes in next level. SETLINK and GETLINK RTEXT_FILTER_CFM_CONFIG: IFLA_BRIDGE_CFM_MEP_CREATE_INSTANCE: The created MEP instance number. The type is u32. IFLA_BRIDGE_CFM_MEP_CREATE_DOMAIN: The created MEP domain. The type is u32 (br_cfm_domain). It must be BR_CFM_PORT. This means that CFM frames are transmitted and received directly on the port - untagged. Not in a VLAN. IFLA_BRIDGE_CFM_MEP_CREATE_DIRECTION: The created MEP direction. The type is u32 (br_cfm_mep_direction). It must be BR_CFM_MEP_DIRECTION_DOWN. This means that CFM frames are transmitted and received on the port. Not in the bridge. IFLA_BRIDGE_CFM_MEP_CREATE_IFINDEX: The created MEP residence port ifindex. The type is u32 (ifindex). IFLA_BRIDGE_CFM_MEP_DELETE_INSTANCE: The deleted MEP instance number. The type is u32. IFLA_BRIDGE_CFM_MEP_CONFIG_INSTANCE: The configured MEP instance number. The type is u32. IFLA_BRIDGE_CFM_MEP_CONFIG_UNICAST_MAC: The configured MEP unicast MAC address. The type is 6*u8 (array). This is used as SMAC in all transmitted CFM frames. IFLA_BRIDGE_CFM_MEP_CONFIG_MDLEVEL: The configured MEP unicast MD level. The type is u32. It must be in the range 1-7. No CFM frames are passing through this MEP on lower levels. IFLA_BRIDGE_CFM_MEP_CONFIG_MEPID: The configured MEP ID. The type is u32. It must be in the range 0-0x1FFF. This MEP ID is inserted in any transmitted CCM frame. IFLA_BRIDGE_CFM_CC_CONFIG_INSTANCE: The configured MEP instance number. The type is u32. IFLA_BRIDGE_CFM_CC_CONFIG_ENABLE: The Continuity Check (CC) functionality is enabled or disabled. The type is u32 (bool). IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL: The CC expected receive interval of CCM frames. The type is u32 (br_cfm_ccm_interval). This is also the transmission interval of CCM frames when enabled. IFLA_BRIDGE_CFM_CC_CONFIG_EXP_MAID: The CC expected receive MAID in CCM frames. The type is CFM_MAID_LENGTH*u8. This is MAID is also inserted in transmitted CCM frames. IFLA_BRIDGE_CFM_CC_PEER_MEP_INSTANCE: The configured MEP instance number. The type is u32. IFLA_BRIDGE_CFM_CC_PEER_MEPID:
Re: [Bridge] [PATCH RFC 4/7] bridge: cfm: Kernel space implementation of CFM.
Thanks for the review. Comments below. The 09/08/2020 13:16, Nikolay Aleksandrov wrote: > > On Fri, 2020-09-04 at 09:15 +, Henrik Bjoernlund wrote: > > This is 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’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_mep_status_get() > > br_cfm_mep_counters_get() > > br_cfm_mep_counters_clear() > > br_cfm_cc_config_set() > > br_cfm_cc_peer_mep_add() > > br_cfm_cc_peer_mep_remove() > > br_cfm_cc_rdi_set() > > br_cfm_cc_ccm_tx() > > br_cfm_cc_status_get() > > br_cfm_cc_counters_get() > > br_cfm_cc_counters_clear() > > br_cfm_cc_peer_status_get() > > > > 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 contain status and counter information that can be > > retrieved by br_cfm_mep_status_get() and > > br_cfm_mep_counters_get(). > > > > A MEP counters can be cleared by br_cfm_mep_counters_clear(). > > > > 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. > > > > A MEP can be configured to start or stop transmission of CCM frames by > > br_cfm_cc_ccm_tx() > > The CCM will be transmitted for a selected period in seconds. > > Must call this function before timeout to keep transmission alive. > > > > A MEP transmitting CCM can be configured with inserted RDI in PDU by > > br_cfm_cc_rdi_set() > > > > A MEP contain Continuity Check status and counter information > > that can be retrieved by br_cfm_cc_status_get() and > > br_cfm_cc_counters_get(). > > > > A MEP Continuity Check counters can be cleared > > by br_cfm_cc_counters_clear(). > > > > A MEP contain Peer MEP Continuity Check status information that > > can be retrieved by br_cfm_cc_peer_status_get(). > > > > Signed-off-by: Henrik Bjoernlund > > --- > > include/uapi/linux/cfm_bridge.h | 75 +++ > > net/bridge/Makefile | 2 + > > net/bridge/br_cfm.c | 880 > > net/bridge/br_private.h | 16 + > > net/bridge/br_private_cfm.h | 242 + > > 5 files changed, 1215 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 > > > > This is a large single patch, do you think it can be broken down into pieces? > I'll review it like this now, but it would be much easier if it's in smaller > logical pieces. > In general are you sure there are no holes in the structs being assigned > directly? Since you use memcmp() and such, you could end up surprised. :) > I will try and break this patch up into logical pieces. I will assure that when called from br_cfm_netlink.c the struct is memset to zero before members are set. > > diff --git a/include/uapi/linux/cfm_bridge.h > > b/include/uapi/linux/cfm_bridge.h > > new file mode 100644 > > index ..389ea1e1f68e > > --- /dev/null > > +++ b/include/uapi/linux/cfm_bridge.h > > @@ -0,0 +1,75 @@ > > +/* 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 ETHER_HEADER_LENGTH (6+6+4+2) > > +#define CFM_MAID_LENGTH 48 > > +#define CFM_CCM_PDU_LENGTH 75 > > +#define CFM_PORT_STATUS_TLV_LENGTH 4 > > +#define CFM_IF_STATUS_TLV_LENGTH 4 > > +#define CFM_IF_STATUS_TLV_TYPE 4
[Bridge] [PATCH RFC 2/7] bridge: cfm: Add BRIDGE_CFM to Kconfig.
This makes it possible to include or exclude the CFM
protocol according to 802.1Q section 12.14.
Signed-off-by: Henrik Bjoernlund
---
net/bridge/Kconfig | 11 +++
net/bridge/br_device.c | 3 +++
net/bridge/br_private.h | 3 +++
3 files changed, 17 insertions(+)
diff --git a/net/bridge/Kconfig b/net/bridge/Kconfig
index 80879196560c..3c8ded7d3e84 100644
--- a/net/bridge/Kconfig
+++ b/net/bridge/Kconfig
@@ -73,3 +73,14 @@ config BRIDGE_MRP
Say N to exclude this support and reduce the binary size.
If unsure, say N.
+
+config BRIDGE_CFM
+ bool "CFM protocol"
+ depends on BRIDGE
+ help
+ If you say Y here, then the Ethernet bridge will be able to run CFM
+ protocol according to 802.1Q section 12.14
+
+ Say N to exclude this support and reduce the binary size.
+
+ If unsure, say N.
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index a9232db03108..d12f5626a4b1 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -476,6 +476,9 @@ void br_dev_setup(struct net_device *dev)
INIT_LIST_HEAD(&br->ftype_list);
#if IS_ENABLED(CONFIG_BRIDGE_MRP)
INIT_LIST_HEAD(&br->mrp_list);
+#endif
+#if IS_ENABLED(CONFIG_BRIDGE_CFM)
+ INIT_LIST_HEAD(&br->mep_list);
#endif
spin_lock_init(&br->hash_lock);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index e67c6d9e8bea..6294a3e51a33 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -445,6 +445,9 @@ struct net_bridge {
#if IS_ENABLED(CONFIG_BRIDGE_MRP)
struct list_headmrp_list;
#endif
+#if IS_ENABLED(CONFIG_BRIDGE_CFM)
+ struct list_headmep_list;
+#endif
};
struct br_input_skb_cb {
--
2.28.0
[Bridge] [PATCH RFC 3/7] bridge: uapi: cfm: Added EtherType used by the CFM protocol.
This EtherType is used by all CFM protocal frames transmitted according to 802.1Q section 12.14. Signed-off-by: Henrik Bjoernlund --- include/uapi/linux/if_ether.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h index d6de2b167448..a0b637911d3c 100644 --- a/include/uapi/linux/if_ether.h +++ b/include/uapi/linux/if_ether.h @@ -99,6 +99,7 @@ #define ETH_P_1588 0x88F7 /* IEEE 1588 Timesync */ #define ETH_P_NCSI 0x88F8 /* NCSI protocol*/ #define ETH_P_PRP 0x88FB /* IEC 62439-3 PRP/HSRv0*/ +#define ETH_P_CFM 0x8902 /* Connectivity Fault Management */ #define ETH_P_FCOE 0x8906 /* Fibre Channel over Ethernet */ #define ETH_P_IBOE 0x8915 /* Infiniband over Ethernet */ #define ETH_P_TDLS 0x890D /* TDLS */ -- 2.28.0
[Bridge] [PATCH net-next] netfilter: ebt_stp: Remove unused macro BPDU_TYPE_TCN
BPDU_TYPE_TCN is never used after it was introduced.
So better to remove it.
Reported-by: Hulk Robot
Signed-off-by: Wang Hai
---
net/bridge/netfilter/ebt_stp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/bridge/netfilter/ebt_stp.c b/net/bridge/netfilter/ebt_stp.c
index 0d6d20c9105e..8f68afda5f81 100644
--- a/net/bridge/netfilter/ebt_stp.c
+++ b/net/bridge/netfilter/ebt_stp.c
@@ -15,7 +15,6 @@
#include
#define BPDU_TYPE_CONFIG 0
-#define BPDU_TYPE_TCN 0x80
struct stp_header {
u8 dsap;
--
2.17.1
Re: [Bridge] [PATCH RFC 7/7] bridge: cfm: Bridge port remove.
Thanks for the review. I will update the next version as suggested.
The 09/08/2020 13:58, Nikolay Aleksandrov wrote:
>
> On Fri, 2020-09-04 at 09:15 +, Henrik Bjoernlund wrote:
> > This is addition of CFM functionality to delete MEP instances
> > on a port that is removed from the bridge.
> > A MEP can only exist on a port that is related to a bridge.
> >
> > Signed-off-by: Henrik Bjoernlund
> > ---
> > net/bridge/br_cfm.c | 13 +
> > net/bridge/br_if.c | 1 +
> > net/bridge/br_private.h | 6 ++
> > 3 files changed, 20 insertions(+)
> >
> > diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
> > index b7fed2c1d8ec..c724ce020ce3 100644
> > --- a/net/bridge/br_cfm.c
> > +++ b/net/bridge/br_cfm.c
> > @@ -921,3 +921,16 @@ bool br_cfm_created(struct net_bridge *br)
> > {
> > return !list_empty(&br->mep_list);
> > }
> > +
> > +/* Deletes the CFM instances on a specific bridge port
> > + * note: called under rtnl_lock
> > + */
> > +void br_cfm_port_del(struct net_bridge *br, struct net_bridge_port *port)
> > +{
> > + struct br_cfm_mep *mep;
> > +
> > + list_for_each_entry_rcu(mep, &br->mep_list, head,
> > + lockdep_rtnl_is_held())
>
> Use standard/non-rcu list traversing, rtnl is already held.
>
> > + if (mep->create.ifindex == port->dev->ifindex)
> > + mep_delete_implementation(br, mep);
> > +}
> > diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> > index a0e9a7937412..f7d2f472ae24 100644
> > --- a/net/bridge/br_if.c
> > +++ b/net/bridge/br_if.c
> > @@ -334,6 +334,7 @@ static void del_nbp(struct net_bridge_port *p)
> > spin_unlock_bh(&br->lock);
> >
> > br_mrp_port_del(br, p);
> > + br_cfm_port_del(br, p);
> >
> > br_ifinfo_notify(RTM_DELLINK, NULL, p);
> >
> > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > index 53bcbdd21f34..5617255f0c0c 100644
> > --- a/net/bridge/br_private.h
> > +++ b/net/bridge/br_private.h
> > @@ -1369,6 +1369,7 @@ 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);
> > +void br_cfm_port_del(struct net_bridge *br, struct net_bridge_port *p);
> > 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,
> > @@ -1393,6 +1394,11 @@ static inline bool br_cfm_created(struct net_bridge
> > *br)
> > return false;
> > }
> >
> > +static inline void br_cfm_port_del(struct net_bridge *br,
> > +struct net_bridge_port *p)
> > +{
> > +}
> > +
> > static inline int br_cfm_config_fill_info(struct sk_buff *skb, struct
> > net_bridge *br)
> > {
> > return -EOPNOTSUPP;
>
--
/Henrik
Re: [Bridge] [PATCH RFC 0/7] net: bridge: cfm: Add support for Connectivity Fault Management(CFM)
Hi Nik, Thanks a lot for reviewing. -Original Message- From: Nikolay Aleksandrov Sent: 7. september 2020 15:56 To: step...@networkplumber.org; Horatiu Vultur - M31836 Cc: bridge@lists.linux-foundation.org; Henrik Bjoernlund - M31679 ; da...@davemloft.net; linux-ker...@vger.kernel.org; j...@mellanox.com; net...@vger.kernel.org; Roopa Prabhu ; ido...@mellanox.com; k...@kernel.org; UNGLinuxDriver Subject: Re: [PATCH RFC 0/7] net: bridge: cfm: Add support for Connectivity Fault Management(CFM) EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >On Sun, 2020-09-06 at 20:21 +0200, Horatiu Vultur wrote: >> The 09/04/2020 15:44, Stephen Hemminger wrote: >> > On Fri, 4 Sep 2020 09:15:20 + >> > Henrik Bjoernlund wrote: >> > >> > > Connectivity Fault Management (CFM) is defined in 802.1Q section 12.14. >> > > >> > > >[snip] >> > > 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'. >> > > >> > > Reviewed-by: Horatiu Vultur >> > > Signed-off-by: Henrik Bjoernlund >> > > >> >> Hi Stephen, >> >> > Could this be done in userspace? It is a control plane protocol. >> > Could it be done by using eBPF? >> >> I might be able to answer this. We have not considered this approach >> of using eBPF. Because we want actually to push this in HW extending >> switchdev API. I know that this series doesn't cover the switchdev >> part but we posted like this because we wanted to get some feedback >> from community. We had a similar approach for MRP, where we extended >> the bridge and switchdev API, so we tought that is the way to go forward. >> >> Regarding eBPF, I can't say that it would work or not because I lack >> knowledge in this. >> >> > Adding more code in bridge impacts a large number of users of Linux >> > distros. >> > It creates bloat and potential security vulnerabilities. > >Hi, >I also had the same initial thought - this really doesn't seem to affect the >bridge in any way, it's only collecting and transmitting information. I get >that you'd like to use the bridge as a passthrough device to switchdev to >program your hw, could you share what would be offloaded more specifically ? > Yes. The HW will offload the periodic sending of CCM frames, and the reception. If CCM frames are not received as expected, it will raise an interrupt. This means that all the functionality provided in this series will be offloaded to HW. The offloading is very important on our HW where we have a small CPU, serving many ports, with a high frequency of CFM frames. The HW also support Link-Trace and Loop-back, which we may come back to later. >All you do - snooping and blocking these packets can easily be done from user- >space with the help of ebtables, but since we need to have a software >implementation/fallback of anything being offloaded via switchdev we might >need this after all, I'd just prefer to push as much as possible to user-space. > >I plan to review the individual patches tomorrow. > >Thanks, > Nik
Re: [Bridge] [PATCH RFC 2/7] bridge: cfm: Add BRIDGE_CFM to Kconfig.
Thanks for your review. I will update in the next version as suggested.
Regards
Henrik
The 09/08/2020 12:18, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Fri, 2020-09-04 at 09:15 +, Henrik Bjoernlund wrote:
> > This makes it possible to include or exclude the CFM
> > protocol according to 802.1Q section 12.14.
> >
> > Signed-off-by: Henrik Bjoernlund
> > ---
> > net/bridge/Kconfig | 11 +++
> > net/bridge/br_device.c | 3 +++
> > net/bridge/br_private.h | 3 +++
> > 3 files changed, 17 insertions(+)
> >
> > diff --git a/net/bridge/Kconfig b/net/bridge/Kconfig
> > index 80879196560c..3c8ded7d3e84 100644
> > --- a/net/bridge/Kconfig
> > +++ b/net/bridge/Kconfig
> > @@ -73,3 +73,14 @@ config BRIDGE_MRP
> > Say N to exclude this support and reduce the binary size.
> >
> > If unsure, say N.
> > +
> > +config BRIDGE_CFM
> > + bool "CFM protocol"
> > + depends on BRIDGE
> > + help
> > + If you say Y here, then the Ethernet bridge will be able to run CFM
> > + protocol according to 802.1Q section 12.14
> > +
> > + Say N to exclude this support and reduce the binary size.
> > +
> > + If unsure, say N.
> > diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> > index a9232db03108..d12f5626a4b1 100644
> > --- a/net/bridge/br_device.c
> > +++ b/net/bridge/br_device.c
> > @@ -476,6 +476,9 @@ void br_dev_setup(struct net_device *dev)
> > INIT_LIST_HEAD(&br->ftype_list);
> > #if IS_ENABLED(CONFIG_BRIDGE_MRP)
> > INIT_LIST_HEAD(&br->mrp_list);
> > +#endif
> > +#if IS_ENABLED(CONFIG_BRIDGE_CFM)
> > + INIT_LIST_HEAD(&br->mep_list);
> > #endif
> > spin_lock_init(&br->hash_lock);
> >
> > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> > index e67c6d9e8bea..6294a3e51a33 100644
> > --- a/net/bridge/br_private.h
> > +++ b/net/bridge/br_private.h
> > @@ -445,6 +445,9 @@ struct net_bridge {
> > #if IS_ENABLED(CONFIG_BRIDGE_MRP)
> > struct list_headmrp_list;
> > #endif
> > +#if IS_ENABLED(CONFIG_BRIDGE_CFM)
> > + struct list_headmep_list;
> > +#endif
> > };
> >
> > struct br_input_skb_cb {
>
> Looks good, perhaps also can use hlist to reduce the head size in net_bridge.
>
--
/Henrik
Re: [Bridge] [PATCH RFC 1/7] net: bridge: extend the process of special frames
Thanks for your review. I will update in the next version as suggested.
Regards
Henrik
The 09/08/2020 12:12, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Fri, 2020-09-04 at 09:15 +, Henrik Bjoernlund wrote:
> > This patch extends the processing of frames in the bridge. Currently MRP
> > frames needs special processing and the current implementation doesn't
> > allow a nice way to process different frame types. Therefore try to
> > improve this by adding a list that contains frame types that need
> > special processing. This list is iterated for each input frame and if
> > there is a match based on frame type then these functions will be called
> > and decide what to do with the frame. It can process the frame then the
> > bridge doesn't need to do anything or don't process so then the bridge
> > will do normal forwarding.
> >
> > Signed-off-by: Henrik Bjoernlund
> > ---
> > net/bridge/br_device.c | 1 +
> > net/bridge/br_input.c | 31 ++-
> > net/bridge/br_mrp.c | 19 +++
> > net/bridge/br_private.h | 18 --
> > 4 files changed, 58 insertions(+), 11 deletions(-)
> >
>
> Hi,
> First I must say I do like this approach, thanks for generalizing it.
> You can switch to a hlist so that there's just 1 pointer in the head.
> I don't think you need list when you're walking only in one direction.
> A few more minor comments below.
>
> > diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> > index 9a2fb4aa1a10..a9232db03108 100644
> > --- a/net/bridge/br_device.c
> > +++ b/net/bridge/br_device.c
> > @@ -473,6 +473,7 @@ void br_dev_setup(struct net_device *dev)
> > spin_lock_init(&br->lock);
> > INIT_LIST_HEAD(&br->port_list);
> > INIT_HLIST_HEAD(&br->fdb_list);
> > + INIT_LIST_HEAD(&br->ftype_list);
> > #if IS_ENABLED(CONFIG_BRIDGE_MRP)
> > INIT_LIST_HEAD(&br->mrp_list);
> > #endif
> > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> > index 59a318b9f646..0f475b21094c 100644
> > --- a/net/bridge/br_input.c
> > +++ b/net/bridge/br_input.c
> > @@ -254,6 +254,21 @@ static int nf_hook_bridge_pre(struct sk_buff *skb,
> > struct sk_buff **pskb)
> > return RX_HANDLER_CONSUMED;
> > }
> >
> > +/* Return 0 if the frame was not processed otherwise 1
> > + * note: already called with rcu_read_lock
> > + */
> > +static int br_process_frame_type(struct net_bridge_port *p,
> > + struct sk_buff *skb)
> > +{
> > + struct br_frame_type *tmp;
> > +
> > + list_for_each_entry_rcu(tmp, &p->br->ftype_list, list) {
> > + if (unlikely(tmp->type == skb->protocol))
> > + return tmp->func(p, skb);
> > + }
>
> Nit: you can drop the {}.
>
> > + return 0;
> > +}
> > +
> > /*
> > * Return NULL if skb is handled
> > * note: already called with rcu_read_lock
> > @@ -343,7 +358,7 @@ static rx_handler_result_t br_handle_frame(struct
> > sk_buff **pskb)
> > }
> > }
> >
> > - if (unlikely(br_mrp_process(p, skb)))
> > + if (unlikely(br_process_frame_type(p, skb)))
> > return RX_HANDLER_PASS;
> >
> > forward:
> > @@ -380,3 +395,17 @@ rx_handler_func_t *br_get_rx_handler(const struct
> > net_device *dev)
> >
> > return br_handle_frame;
> > }
> > +
> > +void br_add_frame(struct net_bridge *br, struct br_frame_type *ft)
> > +{
> > + list_add_rcu(&ft->list, &br->ftype_list);
> > +}
> > +
> > +void br_del_frame(struct net_bridge *br, struct br_frame_type *ft)
> > +{
> > + struct br_frame_type *tmp;
> > +
> > + list_for_each_entry(tmp, &br->ftype_list, list)
> > + if (ft == tmp)
> > + list_del_rcu(&ft->list);
> > +}
> > diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> > index b36689e6e7cb..0428e1785041 100644
> > --- a/net/bridge/br_mrp.c
> > +++ b/net/bridge/br_mrp.c
> > @@ -6,6 +6,13 @@
> > static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1
> > };
> > static const u8 mrp_in_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0,
> > 0x3 };
> >
> > +static int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb);
> > +
> > +static struct br_frame_type mrp_frame_type __read_mostly = {
> > + .type = cpu_to_be16(ETH_P_MRP),
> > + .func = br_mrp_process,
> > +};
> > +
> > static bool br_mrp_is_ring_port(struct net_bridge_port *p_port,
> > struct net_bridge_port *s_port,
> > struct net_bridge_port *port)
> > @@ -445,6 +452,9 @@ static void br_mrp_del_impl(struct net_bridge *br,
> > struct br_mrp *mrp)
> >
> > list_del_rcu(&mrp->list);
> > kfree_rcu(mrp, rcu);
> > +
> > + if (list_empty(&br->mrp_list))
> > + br_del_frame(br, &mrp_frame_type);
> > }
> >
> > /* Adds a new MRP instance.
> > @@ -493,6 +503,9 @@ int br_mrp_add(struct n
Re: [Bridge] [PATCH RFC 5/7] bridge: cfm: Netlink Interface.
Thanks for the review. Comments below.
The 09/08/2020 13:47, Nikolay Aleksandrov wrote:
>
> On Fri, 2020-09-04 at 09:15 +, 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 ..4e39aab1cd0b
> > --- /dev/null
> > +++ b/net/bridge/br_cfm_netlink.c
> > @@ -0,0 +1,684 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +#include
> > +
> > +#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.
>
I think they make a difference - you can write nicely in one line:
config.unicast_mac = nla_get_mac(tb[IFLA_BRIDGE_CFM_MEP_CONFIG_UNICAST_MAC]);
Also you can argue the same with functions nla_get_u32() and nla_get_u8 () they
are one liner functions.
I you think it must be changed I will ofc do it.
> > +
> > +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
>
I have removed this. Not used anyway.
> > +
> > +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] =
[Bridge] [PATCH RFC 4/7] bridge: cfm: Kernel space implementation of CFM.
This is 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’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_mep_status_get()
br_cfm_mep_counters_get()
br_cfm_mep_counters_clear()
br_cfm_cc_config_set()
br_cfm_cc_peer_mep_add()
br_cfm_cc_peer_mep_remove()
br_cfm_cc_rdi_set()
br_cfm_cc_ccm_tx()
br_cfm_cc_status_get()
br_cfm_cc_counters_get()
br_cfm_cc_counters_clear()
br_cfm_cc_peer_status_get()
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 contain status and counter information that can be
retrieved by br_cfm_mep_status_get() and
br_cfm_mep_counters_get().
A MEP counters can be cleared by br_cfm_mep_counters_clear().
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.
A MEP can be configured to start or stop transmission of CCM frames by
br_cfm_cc_ccm_tx()
The CCM will be transmitted for a selected period in seconds.
Must call this function before timeout to keep transmission alive.
A MEP transmitting CCM can be configured with inserted RDI in PDU by
br_cfm_cc_rdi_set()
A MEP contain Continuity Check status and counter information
that can be retrieved by br_cfm_cc_status_get() and
br_cfm_cc_counters_get().
A MEP Continuity Check counters can be cleared
by br_cfm_cc_counters_clear().
A MEP contain Peer MEP Continuity Check status information that
can be retrieved by br_cfm_cc_peer_status_get().
Signed-off-by: Henrik Bjoernlund
---
include/uapi/linux/cfm_bridge.h | 75 +++
net/bridge/Makefile | 2 +
net/bridge/br_cfm.c | 880
net/bridge/br_private.h | 16 +
net/bridge/br_private_cfm.h | 242 +
5 files changed, 1215 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 ..389ea1e1f68e
--- /dev/null
+++ b/include/uapi/linux/cfm_bridge.h
@@ -0,0 +1,75 @@
+/* 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 ETHER_HEADER_LENGTH(6+6+4+2)
+#define CFM_MAID_LENGTH48
+#define CFM_CCM_PDU_LENGTH 75
+#define CFM_PORT_STATUS_TLV_LENGTH 4
+#define CFM_IF_STATUS_TLV_LENGTH 4
+#define CFM_IF_STATUS_TLV_TYPE 4
+#define CFM_PORT_STATUS_TLV_TYPE 2
+#define CFM_ENDE_TLV_TYPE 0
+#define CFM_CCM_MAX_FRAME_LENGTH (ETHER_HEADER_LENGTH+\
+CFM_CCM_PDU_LENGTH+\
+CFM_PORT_STATUS_TLV_LENGTH+\
+CFM_IF_STATUS_TLV_LENGTH)
+#define CFM_FRAME_PRIO 7
+#define CFM_CCM_OPCODE 1
+#define CFM_CCM_TLV_OFFSET 70
+#define CFM_CCM_PDU_MAID_OFFSET10
+#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 {
+ __u8 mdlevel_version;
+ __u8 opcode;
+ __u8 flags;
+ __u8 tlv_offset;
+};
+
+struct br_cfm_status_tlv {
+ __u8 type;
+ __be16 length;
+ __u8 value;
+};
+
+enum br_cfm_opcodes {
+ BR_CFM_OPCODE_CCM = 0x1,
+ BR_CFM_OPCODE_LBR = 0x2,
+ BR_CFM_OPCODE_LBM = 0x3,
+ BR_CFM_OPCODE_LTR = 0x4,
+ BR_CFM_OPC
[Bridge] [PATCH 7/7 net-next] net: bridge: delete duplicated words
Drop repeated words in net/bridge/. Signed-off-by: Randy Dunlap Cc: "David S. Miller" Cc: Jakub Kicinski Cc: Roopa Prabhu Cc: Nikolay Aleksandrov Cc: bridge@lists.linux-foundation.org --- net/bridge/br_ioctl.c |2 +- net/bridge/br_vlan.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) --- linux-next-20200917.orig/net/bridge/br_ioctl.c +++ linux-next-20200917/net/bridge/br_ioctl.c @@ -103,7 +103,7 @@ static int add_del_if(struct net_bridge /* * Legacy ioctl's through SIOCDEVPRIVATE - * This interface is deprecated because it was too difficult to + * This interface is deprecated because it was too difficult * to do the translation for 32/64bit ioctl compatibility. */ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) --- linux-next-20200917.orig/net/bridge/br_vlan.c +++ linux-next-20200917/net/bridge/br_vlan.c @@ -140,7 +140,7 @@ static int __vlan_vid_del(struct net_dev return err == -EOPNOTSUPP ? 0 : err; } -/* Returns a master vlan, if it didn't exist it gets created. In all cases a +/* Returns a master vlan, if it didn't exist it gets created. In all cases * a reference is taken to the master vlan before returning. */ static struct net_bridge_vlan *
