mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/3] net: bridge: cfm: notify userspace on CFM config changes
@ 2026-10-05  4:38 Abdul Wasey
  2026-10-05  4:38 ` [PATCH net-next v2 1/3] net: bridge: cfm: count CFM config attributes in the link message size Abdul Wasey
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Abdul Wasey @ 2026-10-05  4:38 UTC (permalink / raw)
  To: netdev
  Cc: razor, idosch, davem, edumazet, kuba, pabeni, horms,
	horatiu.vultur, bridge, linux-kernel

CFM status changes are sent to userspace, config changes are not, so
the only way to see a MEP being created, a peer added or CCM
transmission started is to poll RTM_GETLINK. Patch 3 adds the
notification. Patches 1 and 2 fix the two problems the AI review found
in v1.

Patch 1 makes br_get_link_af_size_filtered() count the CFM config
attributes. Without it the notification overflows its skb once a MEP
has about 32 peers, and br_info_notify() hits its WARN_ON. Patch 2 puts
an empty IFLA_BRIDGE_CFM nest in the message when the bridge has no
MEPs, so deleting the last MEP is visible to a listener.

Tested with virtme-ng on net-next: 300 peer MEPs and config changes
give one notification each (6248 byte CFM payload) and no warning;
deleting the last MEP gives a notification with an empty
IFLA_BRIDGE_CFM; a request that fails before changing anything sends
nothing, one that fails half way still sends one. Also built with
CONFIG_BRIDGE_CFM off.

v2:
- new patch 1, count the config attributes in the message size
- new patch 2, empty CFM nest for a bridge with no MEPs
- patch 3: unchanged, rebased on current net-next

v1: https://lore.kernel.org/netdev/20261004041320.2684045-1-w453y.me@gmail.com/

Abdul Wasey (3):
  net: bridge: cfm: count CFM config attributes in the link message size
  net: bridge: cfm: send an empty CFM nest when a bridge has no MEPs
  net: bridge: cfm: notify userspace on CFM config changes

 net/bridge/br_cfm_netlink.c | 32 ++++++++++++++++++-------
 net/bridge/br_netlink.c     | 47 ++++++++++++++++++++++++++++++++++---
 2 files changed, 67 insertions(+), 12 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 1/3] net: bridge: cfm: count CFM config attributes in the link message size
  2026-10-05  4:38 [PATCH net-next v2 0/3] net: bridge: cfm: notify userspace on CFM config changes Abdul Wasey
@ 2026-10-05  4:38 ` Abdul Wasey
  2026-10-07 19:38   ` netdev-bot+sashiko
  2026-10-05  4:38 ` [PATCH net-next v2 2/3] net: bridge: cfm: send an empty CFM nest when a bridge has no MEPs Abdul Wasey
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Abdul Wasey @ 2026-10-05  4:38 UTC (permalink / raw)
  To: netdev
  Cc: razor, idosch, davem, edumazet, kuba, pabeni, horms,
	horatiu.vultur, bridge, linux-kernel

br_get_link_af_size_filtered() only counts the CFM attributes when
RTEXT_FILTER_CFM_STATUS is set. With RTEXT_FILTER_CFM_CONFIG alone it
returns before counting anything, while br_fill_ifinfo() still writes
the whole config, about 250 bytes per MEP and 20 per peer MEP.

Dumps cope with that, they just get a bigger buffer. A message built by
br_info_notify() can't, it fails with -EMSGSIZE and hits the WARN_ON
there. No notification asks for the CFM config today, but the next
patch adds one.

Count the config attributes when RTEXT_FILTER_CFM_CONFIG is set.

Signed-off-by: Abdul Wasey <w453y.me@gmail.com>
---
 net/bridge/br_netlink.c | 42 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index f65b8b6ca..5160e801d 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -90,6 +90,36 @@ static int br_get_num_vlan_infos(struct net_bridge_vlan_group *vg,
 	return num_vlans;
 }
 
+static size_t br_cfm_config_info_size(u32 num_meps, u32 num_peer_meps)
+{
+	size_t mep_sz, peer_sz;
+
+	/* IFLA_BRIDGE_CFM_MEP_CREATE_INFO: instance, domain, direction,
+	 * ifindex
+	 */
+	mep_sz = nla_total_size(4 * nla_total_size(sizeof(u32)));
+	/* IFLA_BRIDGE_CFM_MEP_CONFIG_INFO: instance, unicast mac, mdlevel,
+	 * mepid
+	 */
+	mep_sz += nla_total_size(3 * nla_total_size(sizeof(u32)) +
+				 nla_total_size(ETH_ALEN));
+	/* IFLA_BRIDGE_CFM_CC_CONFIG_INFO: instance, enable, interval, maid */
+	mep_sz += nla_total_size(3 * nla_total_size(sizeof(u32)) +
+				 nla_total_size(CFM_MAID_LENGTH));
+	/* IFLA_BRIDGE_CFM_CC_RDI_INFO: instance, rdi */
+	mep_sz += nla_total_size(2 * nla_total_size(sizeof(u32)));
+	/* IFLA_BRIDGE_CFM_CC_CCM_TX_INFO: instance, dmac, seq no update,
+	 * period, if tlv, if tlv value, port tlv, port tlv value
+	 */
+	mep_sz += nla_total_size(5 * nla_total_size(sizeof(u32)) +
+				 nla_total_size(ETH_ALEN) +
+				 2 * nla_total_size(sizeof(u8)));
+	/* IFLA_BRIDGE_CFM_CC_PEER_MEP_INFO: instance, peer mepid */
+	peer_sz = nla_total_size(2 * nla_total_size(sizeof(u32)));
+
+	return num_meps * mep_sz + num_peer_meps * peer_sz;
+}
+
 static size_t br_get_link_af_size_filtered(const struct net_device *dev,
 					   u32 filter_mask)
 {
@@ -122,17 +152,25 @@ static size_t br_get_link_af_size_filtered(const struct net_device *dev,
 	if (p && vg && (filter_mask & RTEXT_FILTER_MST))
 		vinfo_sz += br_mst_info_size(vg);
 
-	if (!(filter_mask & RTEXT_FILTER_CFM_STATUS))
+	if (!(filter_mask & (RTEXT_FILTER_CFM_CONFIG | RTEXT_FILTER_CFM_STATUS)))
 		return vinfo_sz;
 
 	if (!br)
 		return vinfo_sz;
 
-	/* CFM status info must be added */
 	br_cfm_mep_count(br, &num_cfm_mep_infos);
 	br_cfm_peer_mep_count(br, &num_cfm_peer_mep_infos);
 
 	vinfo_sz += nla_total_size(0);	/* IFLA_BRIDGE_CFM */
+
+	if (filter_mask & RTEXT_FILTER_CFM_CONFIG)
+		vinfo_sz += br_cfm_config_info_size(num_cfm_mep_infos,
+						    num_cfm_peer_mep_infos);
+
+	if (!(filter_mask & RTEXT_FILTER_CFM_STATUS))
+		return vinfo_sz;
+
+	/* CFM status info must be added */
 	/* For each status struct the MEP instance (u32) is added */
 	/* MEP instance (u32) + br_cfm_mep_status */
 	vinfo_sz += num_cfm_mep_infos *
-- 
2.53.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 2/3] net: bridge: cfm: send an empty CFM nest when a bridge has no MEPs
  2026-10-05  4:38 [PATCH net-next v2 0/3] net: bridge: cfm: notify userspace on CFM config changes Abdul Wasey
  2026-10-05  4:38 ` [PATCH net-next v2 1/3] net: bridge: cfm: count CFM config attributes in the link message size Abdul Wasey
@ 2026-10-05  4:38 ` Abdul Wasey
  2026-10-07 19:38   ` netdev-bot+sashiko
  2026-10-05  4:38 ` [PATCH net-next v2 3/3] net: bridge: cfm: notify userspace on CFM config changes Abdul Wasey
  2026-10-07 13:50 ` [PATCH net-next v2 0/3] " Ido Schimmel
  3 siblings, 1 reply; 8+ messages in thread
From: Abdul Wasey @ 2026-10-05  4:38 UTC (permalink / raw)
  To: netdev
  Cc: razor, idosch, davem, edumazet, kuba, pabeni, horms,
	horatiu.vultur, bridge, linux-kernel

br_fill_ifinfo() leaves IFLA_BRIDGE_CFM out when the bridge has no
MEPs. For a dump that is fine, but a notification sent after the last
MEP is deleted then looks like any other RTM_NEWLINK for the bridge,
and a listener can't tell that the CFM config is now empty.

Put an empty IFLA_BRIDGE_CFM nest in the message instead when CFM is
asked for and the bridge has no MEPs. Ports still get none, as before.

Signed-off-by: Abdul Wasey <w453y.me@gmail.com>
---
 net/bridge/br_netlink.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 5160e801d..68ec4529f 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -603,7 +603,10 @@ static int br_fill_ifinfo(struct sk_buff *skb,
 		struct nlattr *cfm_nest = NULL;
 		int err;
 
-		if (!br_cfm_created(br) || port)
+		/* A bridge with no MEPs gets an empty IFLA_BRIDGE_CFM, so a
+		 * listener can tell that the last MEP is gone.
+		 */
+		if (!IS_ENABLED(CONFIG_BRIDGE_CFM) || port)
 			goto done;
 
 		cfm_nest = nla_nest_start(skb, IFLA_BRIDGE_CFM);
-- 
2.53.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v2 3/3] net: bridge: cfm: notify userspace on CFM config changes
  2026-10-05  4:38 [PATCH net-next v2 0/3] net: bridge: cfm: notify userspace on CFM config changes Abdul Wasey
  2026-10-05  4:38 ` [PATCH net-next v2 1/3] net: bridge: cfm: count CFM config attributes in the link message size Abdul Wasey
  2026-10-05  4:38 ` [PATCH net-next v2 2/3] net: bridge: cfm: send an empty CFM nest when a bridge has no MEPs Abdul Wasey
@ 2026-10-05  4:38 ` Abdul Wasey
  2026-10-07 19:38   ` netdev-bot+sashiko
  2026-10-07 13:50 ` [PATCH net-next v2 0/3] " Ido Schimmel
  3 siblings, 1 reply; 8+ messages in thread
From: Abdul Wasey @ 2026-10-05  4:38 UTC (permalink / raw)
  To: netdev
  Cc: razor, idosch, davem, edumazet, kuba, pabeni, horms,
	horatiu.vultur, bridge, linux-kernel

CFM status changes are sent to userspace by br_cfm_notify(), but config
changes are not. br_afspec() calls br_cfm_parse() without the "changed"
pointer, so creating a MEP, adding a peer or starting CCM transmission
never sends an RTM_NEWLINK. Today the only way to see these changes is
to poll RTM_GETLINK with RTEXT_FILTER_CFM_CONFIG.

Send one RTM_NEWLINK with RTEXT_FILTER_CFM_CONFIG from br_cfm_parse()
once any group in the request has been applied. If a later group in the
same request fails, still send it, since the earlier groups already
changed the config.

br_ifinfo_notify() is not used because it asks for
RTEXT_FILTER_BRVLAN_COMPRESSED, so the message would not carry the CFM
attributes. This calls br_info_notify() the same way br_cfm_notify()
does for status.

Signed-off-by: Abdul Wasey <w453y.me@gmail.com>
---
 net/bridge/br_cfm_netlink.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/net/bridge/br_cfm_netlink.c b/net/bridge/br_cfm_netlink.c
index 91b9922dc..56309e6f4 100644
--- a/net/bridge/br_cfm_netlink.c
+++ b/net/bridge/br_cfm_netlink.c
@@ -382,6 +382,7 @@ 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];
+	bool changed = false;
 	int err;
 
 	/* When this function is called for a port then the br pointer is
@@ -399,59 +400,72 @@ int br_cfm_parse(struct net_bridge *br, struct net_bridge_port *p,
 		err = br_mep_create_parse(br, tb[IFLA_BRIDGE_CFM_MEP_CREATE],
 					  extack);
 		if (err)
-			return err;
+			goto out;
+		changed = true;
 	}
 
 	if (tb[IFLA_BRIDGE_CFM_MEP_DELETE]) {
 		err = br_mep_delete_parse(br, tb[IFLA_BRIDGE_CFM_MEP_DELETE],
 					  extack);
 		if (err)
-			return err;
+			goto out;
+		changed = true;
 	}
 
 	if (tb[IFLA_BRIDGE_CFM_MEP_CONFIG]) {
 		err = br_mep_config_parse(br, tb[IFLA_BRIDGE_CFM_MEP_CONFIG],
 					  extack);
 		if (err)
-			return err;
+			goto out;
+		changed = true;
 	}
 
 	if (tb[IFLA_BRIDGE_CFM_CC_CONFIG]) {
 		err = br_cc_config_parse(br, tb[IFLA_BRIDGE_CFM_CC_CONFIG],
 					 extack);
 		if (err)
-			return err;
+			goto out;
+		changed = true;
 	}
 
 	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;
+			goto out;
+		changed = true;
 	}
 
 	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;
+			goto out;
+		changed = true;
 	}
 
 	if (tb[IFLA_BRIDGE_CFM_CC_RDI]) {
 		err = br_cc_rdi_parse(br, tb[IFLA_BRIDGE_CFM_CC_RDI],
 				      extack);
 		if (err)
-			return err;
+			goto out;
+		changed = true;
 	}
 
 	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;
+			goto out;
+		changed = true;
 	}
 
-	return 0;
+out:
+	/* Earlier groups may have been applied even if a later one failed */
+	if (changed)
+		br_info_notify(RTM_NEWLINK, br, NULL, RTEXT_FILTER_CFM_CONFIG);
+
+	return err;
 }
 
 int br_cfm_config_fill_info(struct sk_buff *skb, struct net_bridge *br)
-- 
2.53.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 0/3] net: bridge: cfm: notify userspace on CFM config changes
  2026-10-05  4:38 [PATCH net-next v2 0/3] net: bridge: cfm: notify userspace on CFM config changes Abdul Wasey
                   ` (2 preceding siblings ...)
  2026-10-05  4:38 ` [PATCH net-next v2 3/3] net: bridge: cfm: notify userspace on CFM config changes Abdul Wasey
@ 2026-10-07 13:50 ` Ido Schimmel
  3 siblings, 0 replies; 8+ messages in thread
From: Ido Schimmel @ 2026-10-07 13:50 UTC (permalink / raw)
  To: Abdul Wasey
  Cc: netdev, razor, davem, edumazet, kuba, pabeni, horms,
	horatiu.vultur, bridge, linux-kernel

On Mon, Oct 05, 2026 at 04:38:19AM +0000, Abdul Wasey wrote:
> CFM status changes are sent to userspace, config changes are not, so
> the only way to see a MEP being created, a peer added or CCM
> transmission started is to poll RTM_GETLINK. Patch 3 adds the
> notification. Patches 1 and 2 fix the two problems the AI review found
> in v1.

Given [1], I don't see a reason to review / apply this series. Will add
a deprecation warning to CFM and MRP.

[1] https://lore.kernel.org/netdev/20261006134441.3696739-1-w453y.me@gmail.com/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 1/3] net: bridge: cfm: count CFM config attributes in the link message size
  2026-10-05  4:38 ` [PATCH net-next v2 1/3] net: bridge: cfm: count CFM config attributes in the link message size Abdul Wasey
@ 2026-10-07 19:38   ` netdev-bot+sashiko
  0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-10-07 19:38 UTC (permalink / raw)
  To: w453y.me
  Cc: netdev, razor, idosch, davem, edumazet, kuba, pabeni, horms,
	horatiu.vultur, bridge, linux-kernel

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message is wrong about the dump path and the patch
  has no Fixes: tag.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the CFM
  status sizing in br_get_link_af_size_filtered() leaves out the per-…

--

net: bridge: cfm: count CFM config attributes in the link message size

With this patch, br_get_link_af_size_filtered() counts the CFM config
attributes (MEP create/config, CC config, RDI, CCM TX and peer MEP info)
when RTEXT_FILTER_CFM_CONFIG is set. Before, only RTEXT_FILTER_CFM_STATUS
caused any CFM sizing. The change prepares for a later patch that sends a
CFM config notification through br_info_notify().

> Dumps cope with that, they just get a bigger buffer. A message built by
> br_info_notify() can't, it fails with -EMSGSIZE and hits the WARN_ON
> there. No notification asks for the CFM config today, but the next
> patch adds one.

[Severity: Medium]

Is this an accurate description of the dump path?

br_get_link_af_size_filtered() is also the .get_link_af_size callback in
br_af_ops, so it sizes the dump buffer as well:

rtnetlink_rcv_msg()
    min_dump_alloc = rtnl_calcit(skb, nlh)
        if_nlmsg_size(dev, ext_filter_mask)
            rtnl_link_get_af_size()
                br_get_link_af_size_filtered()

So the bigger buffer that dumps get is sized by the same estimate that
undercounts the CFM config.

Before this patch, a PF_BRIDGE RTM_GETLINK dump with only
RTEXT_FILTER_CFM_CONFIG in IFLA_EXT_MASK added nothing for CFM to
min_dump_alloc. For the bridge master, br_fill_ifinfo() still wrote about
248 bytes per MEP and 20 bytes per peer MEP.

If that single message does not fit in the dump skb, the skb is still
empty, so rtnl_bridge_getlink() passes the error back to userspace:

net/core/rtnetlink.c:rtnl_bridge_getlink() {
    ...
				if (err < 0 && err != -EOPNOTSUPP) {
					if (likely(skb->len))
						break;

					goto out_err;
				}
    ...
}

That happens at roughly 15 MEPs for the first skb, and at roughly 130
MEPs with a 32 KiB receive buffer.

Doesn't that mean this patch also fixes a -EMSGSIZE dump failure that
userspace can see today? If so, could the commit message say that? It
could also carry a Fixes: tag for commit 5e312fc0e7fb ("bridge: cfm:
Netlink GET configuration Interface"), so the fix is picked up for net
and stable.

> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index f65b8b6ca97c0..5160e801d62db 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c

[ ... ]

> @@ -122,17 +152,25 @@ static size_t br_get_link_af_size_filtered(const struct net_device *dev,

[ ... ]

> -	/* CFM status info must be added */
>  	br_cfm_mep_count(br, &num_cfm_mep_infos);
>  	br_cfm_peer_mep_count(br, &num_cfm_peer_mep_infos);
>  
>  	vinfo_sz += nla_total_size(0);	/* IFLA_BRIDGE_CFM */
> +
> +	if (filter_mask & RTEXT_FILTER_CFM_CONFIG)
> +		vinfo_sz += br_cfm_config_info_size(num_cfm_mep_infos,
> +						    num_cfm_peer_mep_infos);
> +
> +	if (!(filter_mask & RTEXT_FILTER_CFM_STATUS))
> +		return vinfo_sz;
> +
> +	/* CFM status info must be added */
>  	/* For each status struct the MEP instance (u32) is added */
>  	/* MEP instance (u32) + br_cfm_mep_status */
>  	vinfo_sz += num_cfm_mep_infos *

[Severity: Medium]

This is a pre-existing issue, not one introduced by this patch. Does the
status part of this estimate leave out the nest header for each object?

br_cfm_status_fill_info() opens a nest for every MEP and every peer MEP:

net/bridge/br_cfm_netlink.c:br_cfm_status_fill_info() {
    ...
	hlist_for_each_entry_rcu(mep, &br->mep_list, head) {
		tb = nla_nest_start(skb, IFLA_BRIDGE_CFM_MEP_STATUS_INFO);
    ...
		hlist_for_each_entry_rcu(peer_mep, &mep->peer_mep_list, head) {
			tb = nla_nest_start(skb,
					    IFLA_BRIDGE_CFM_CC_PEER_STATUS_INFO);
    ...
}

The status sums in br_get_link_af_size_filtered() count only the leaf
attributes. That gives 32 bytes per MEP against 36 written, and 72 bytes
per peer MEP against 76 written.

The new br_cfm_config_info_size() does wrap each group in
nla_total_size(), so the config and status parts of this function now
count differently.

br_cfm_notify() already sends a status notification for the bridge
master:

net/bridge/br_cfm.c:br_cfm_notify() {
	u32 filter = RTEXT_FILTER_CFM_STATUS;

	br_info_notify(event, port->br, NULL, filter);
}

For a bridge master, br_nlmsg_size() leaves only about 400 bytes of
slack. The shortfall grows by 4 bytes per MEP and per peer MEP, and the
kernel does not limit either count.

With a few hundred peer MEPs, can br_fill_ifinfo() return -EMSGSIZE,
which would trigger WARN_ON(err == -EMSGSIZE) in br_info_notify() and
drop the notification? The trigger can be ccm_rx_work_expired() or
br_cfm_frame_rx() when a CCM frame arrives.

Would it make sense to add nla_total_size(0) to the per-MEP and
per-peer-MEP status terms while this function is being changed?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005043822.3106580-1-w453y.me%40gmail.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 2/3] net: bridge: cfm: send an empty CFM nest when a bridge has no MEPs
  2026-10-05  4:38 ` [PATCH net-next v2 2/3] net: bridge: cfm: send an empty CFM nest when a bridge has no MEPs Abdul Wasey
@ 2026-10-07 19:38   ` netdev-bot+sashiko
  0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-10-07 19:38 UTC (permalink / raw)
  To: w453y.me
  Cc: netdev, razor, idosch, davem, edumazet, kuba, pabeni, horms,
	horatiu.vultur, bridge, linux-kernel

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] At this commit alone, the new `!IS_ENABLED(CONFIG_BRIDGE_CFM)`
  branch in br_fill_ifinfo() (net/bridge/br_netlink.c:609) cannot send…
- [Low] The patch also changes GETLINK dump replies, and the commit
  message does not say so plainly.
- [Low] This patch removed the only caller of br_cfm_created() (in
  br_fill_ifinfo()), and nothing calls it any more.

Pre-existing issues:
- [Low] In br_fill_ifinfo(), two earlier sections jump to `done` and skip
  the CFM block: - the VLAN section: `if (!vg ||…

--

net: bridge: cfm: send an empty CFM nest when a bridge has no MEPs

This patch changes br_fill_ifinfo() so that a bridge master with no MEPs
gets an empty IFLA_BRIDGE_CFM nest when CFM config or status is requested.
Before, it got no nest at all. The goal is to let notification listeners
see that the last MEP has been deleted. Ports still get no CFM nest.

> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 5160e801d62db..68ec4529f0d2b 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -603,7 +603,10 @@ static int br_fill_ifinfo(struct sk_buff *skb,
>  		struct nlattr *cfm_nest = NULL;
>  		int err;
>  
> -		if (!br_cfm_created(br) || port)
> +		/* A bridge with no MEPs gets an empty IFLA_BRIDGE_CFM, so a
> +		 * listener can tell that the last MEP is gone.
> +		 */
> +		if (!IS_ENABLED(CONFIG_BRIDGE_CFM) || port)
>  			goto done;

[Severity: Low]
With only this patch applied, can any notification carry the new empty
nest?

This block only runs when filter_mask contains RTEXT_FILTER_CFM_CONFIG or
RTEXT_FILTER_CFM_STATUS. The generic bridge notification path asks for
neither:

net/bridge/br_netlink.c:br_ifinfo_notify() {
    ...
	u32 filter = RTEXT_FILTER_BRVLAN_COMPRESSED;
    ...
}

The IFLA_BRIDGE_CFM case in br_afspec() does not set *changed. Also,
br_cfm_notify() only fires from the peer MEP CCM paths, which need a live
MEP.

So at this point in the series, deleting the last MEP sends no
CFM-filtered RTM_NEWLINK. The only visible change is in RTM_GETLINK
replies.

The next patch in the series, "net: bridge: cfm: notify userspace on CFM
config changes", seems to fix this. It sets changed after a successful
br_mep_delete_parse() and adds this to br_cfm_parse():

	if (changed)
		br_info_notify(RTM_NEWLINK, br, NULL, RTEXT_FILTER_CFM_CONFIG);

Could the commit message mention that it depends on that follow-up patch?

Even at the end of the series, one case still seems to send no
CFM_CONFIG notification. When a port is deleted, its MEPs are removed
implicitly:

del_nbp()
  br_cfm_port_del()
    mep_delete_implementation()

In that path only br_ifinfo_notify(RTM_DELLINK, NULL, p) is sent for the
port. How is a listener expected to notice that the MEPs are gone?

[Severity: Low]
This also changes RTM_GETLINK and dump replies, not just notifications.

Before, a bridge with no MEPs took goto done with an empty IFLA_AF_SPEC,
and the epilogue cancelled that nest:

done:
	if (af) {
		if (nlmsg_get_pos(skb) - (void *)af > nla_attr_size(0))
			nla_nest_end(skb, af);
		else
			nla_nest_cancel(skb, af);
	}

Now, with CONFIG_BRIDGE_CFM enabled, every bridge master dumped with
RTEXT_FILTER_CFM_CONFIG or RTEXT_FILTER_CFM_STATUS gets an IFLA_AF_SPEC
that holds an empty IFLA_BRIDGE_CFM. Before, the reply for such a bridge
had no IFLA_AF_SPEC at all.

The commit message says:

    For a dump that is fine, but a notification sent after the last
    MEP is deleted then looks like any other RTM_NEWLINK for the bridge,

Could the commit message also say that dump output changes for bridges
without MEPs? Userspace can no longer treat the presence of
IFLA_BRIDGE_CFM as meaning "MEPs are configured".

The buffer sizing in br_get_link_af_size_filtered() already counts the
empty nest, so that part looks fine.

[Severity: Low]
This isn't a bug, but this removes the last caller of br_cfm_created().
Three pieces are left behind:

- the definition in net/bridge/br_cfm.c
- the prototype in net/bridge/br_private.h
- the !CONFIG_BRIDGE_CFM static inline stub in net/bridge/br_private.h

The later patches in the series add no new user. Should these be removed
here too?

[Severity: Low]
This is a pre-existing issue and was not introduced by this patch. Two
earlier sections of br_fill_ifinfo() can jump to done before the CFM
block runs:

	if (!vg || !READ_ONCE(vg->num_vlans)) {
		rcu_read_unlock();
		goto done;
	}
    ...
		if (!br_mrp_enabled(br) || port)
			goto done;

Suppose an RTM_GETLINK request combines RTEXT_FILTER_CFM_* with either
RTEXT_FILTER_BRVLAN* on a bridge with no VLANs, or RTEXT_FILTER_MRP on a
bridge without MRP. The reply then has no IFLA_BRIDGE_CFM at all: neither
the new empty nest nor real MEP data when MEPs exist.

Is that intended for combined filter masks? The notification added later
in the series uses RTEXT_FILTER_CFM_CONFIG alone, so it does not hit
these early exits.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005043822.3106580-1-w453y.me%40gmail.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v2 3/3] net: bridge: cfm: notify userspace on CFM config changes
  2026-10-05  4:38 ` [PATCH net-next v2 3/3] net: bridge: cfm: notify userspace on CFM config changes Abdul Wasey
@ 2026-10-07 19:38   ` netdev-bot+sashiko
  0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-10-07 19:38 UTC (permalink / raw)
  To: w453y.me
  Cc: netdev, razor, idosch, davem, edumazet, kuba, pabeni, horms,
	horatiu.vultur, bridge, linux-kernel

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Config-change notifications are only sent for changes made
  through br_cfm_parse().

--

net: bridge: cfm: notify userspace on CFM config changes

This patch makes br_cfm_parse() send an RTM_NEWLINK with
RTEXT_FILTER_CFM_CONFIG once any CFM config group in the request has
been applied. The notification is sent even if a later group in the
same request fails. It uses br_info_notify() the same way
br_cfm_notify() already does for status.

> diff --git a/net/bridge/br_cfm_netlink.c b/net/bridge/br_cfm_netlink.c
> index 91b9922dc3f25..56309e6f4b1be 100644
> --- a/net/bridge/br_cfm_netlink.c
> +++ b/net/bridge/br_cfm_netlink.c

[ ... ]

> @@ -399,59 +400,72 @@ int br_cfm_parse(struct net_bridge *br, struct net_bridge_port *p,

[ ... ]

>  	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;
> +			goto out;
> +		changed = true;
>  	}
>  
> -	return 0;
> +out:
> +	/* Earlier groups may have been applied even if a later one failed */
> +	if (changed)
> +		br_info_notify(RTM_NEWLINK, br, NULL, RTEXT_FILTER_CFM_CONFIG);
> +
> +	return err;
>  }

[Severity: High]

This sends a CFM config notification for changes that come in through
br_cfm_parse(). The kernel also changes the reported CFM config in a
couple of other places. Do those still go unreported?

The commit message says:

  Today the only way to see these changes is
  to poll RTM_GETLINK with RTEXT_FILTER_CFM_CONFIG.

One such place is ccm_tx_work_expired() in net/bridge/br_cfm.c, when the
transmission period ends:

	if (time_before_eq(mep->ccm_tx_end, jiffies)) {
		/* Transmission period has ended */
		mep->cc_ccm_tx_info.period = 0;
		return;
	}

br_cfm_config_fill_info() reports this field as
IFLA_BRIDGE_CFM_CC_CCM_TX_PERIOD. With this patch a listener sees
period=N when CCM transmission starts. Does it ever see the period drop
to 0 when transmission stops on its own?

The other place is port removal. del_nbp()->br_cfm_port_del() deletes
every MEP whose ifindex matches the port being removed:

	hlist_for_each_entry_safe(mep, n_store, &br->mep_list, head)
		if (mep->create.ifindex == port->dev->ifindex)
			mep_delete_implementation(br, mep);

After that, the only notification in del_nbp() is:

	br_ifinfo_notify(RTM_DELLINK, NULL, p);

That is a port message using RTEXT_FILTER_BRVLAN_COMPRESSED. Also,
br_fill_ifinfo() never puts IFLA_BRIDGE_CFM in port messages.

The previous patch in this series ("net: bridge: cfm: send an empty CFM
nest when a bridge has no MEPs") adds the empty nest so a listener can
tell that the last MEP is gone. If that last MEP is removed because its
port leaves the bridge (ip link del, or ip link set ... nomaster), no
bridge CFM config message is sent at all.

In these two cases, would a listener that relies only on the new
notifications keep deleted MEPs and non-zero CCM_TX periods in its view,
so it still has to poll?

Should ccm_tx_work_expired() and br_cfm_port_del() also send an
RTEXT_FILTER_CFM_CONFIG notification?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005043822.3106580-1-w453y.me%40gmail.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-10-07 19:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-05  4:38 [PATCH net-next v2 0/3] net: bridge: cfm: notify userspace on CFM config changes Abdul Wasey
2026-10-05  4:38 ` [PATCH net-next v2 1/3] net: bridge: cfm: count CFM config attributes in the link message size Abdul Wasey
2026-10-07 19:38   ` netdev-bot+sashiko
2026-10-05  4:38 ` [PATCH net-next v2 2/3] net: bridge: cfm: send an empty CFM nest when a bridge has no MEPs Abdul Wasey
2026-10-07 19:38   ` netdev-bot+sashiko
2026-10-05  4:38 ` [PATCH net-next v2 3/3] net: bridge: cfm: notify userspace on CFM config changes Abdul Wasey
2026-10-07 19:38   ` netdev-bot+sashiko
2026-10-07 13:50 ` [PATCH net-next v2 0/3] " Ido Schimmel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®