From: Horatiu Vultur <horatiu.vultur@microchip.com>
To: <jiri@resnulli.us>, <ivecera@redhat.com>, <davem@davemloft.net>,
<kuba@kernel.org>, <roopa@nvidia.com>, <nikolay@nvidia.com>,
<rasmus.villemoes@prevas.dk>, <andrew@lunn.ch>,
<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<bridge@lists.linux-foundation.org>
Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
Subject: [PATCH net-next v2 3/4] bridge: mrp: Extend br_mrp_switchdev to detect better the errors
Date: Wed, 27 Jan 2021 21:52:40 +0100 [thread overview]
Message-ID: <20210127205241.2864728-4-horatiu.vultur@microchip.com> (raw)
In-Reply-To: <20210127205241.2864728-1-horatiu.vultur@microchip.com>
This patch extends the br_mrp_switchdev functions to be able to have a
better understanding what cause the issue and if the SW needs to be used
as a backup.
There are the following cases:
- when the code is compiled without CONFIG_NET_SWITCHDEV. In this case
return success so the SW can continue with the protocol. Depending
on the function, it returns 0 or BR_MRP_SW.
- when code is compiled with CONFIG_NET_SWITCHDEV and the driver doesn't
implement any MRP callbacks. In this case the HW can't run MRP so it
just returns -EOPNOTSUPP. So the SW will stop further to configure the
node.
- when code is compiled with CONFIG_NET_SWITCHDEV and the driver fully
supports any MRP functionality. In this case the SW doesn't need to do
anything. The functions will return 0 or BR_MRP_HW.
- when code is compiled with CONFIG_NET_SWITCHDEV and the HW can't run
completely the protocol but it can help the SW to run it. For
example, the HW can't support completely MRM role(can't detect when it
stops receiving MRP Test frames) but it can redirect these frames to
CPU. In this case it is possible to have a SW fallback. The SW will
try initially to call the driver with sw_backup set to false, meaning
that the HW should implement completely the role. If the driver returns
-EOPNOTSUPP, the SW will try again with sw_backup set to false,
meaning that the SW will detect when it stops receiving the frames but
it needs HW support to redirect the frames to CPU. In case the driver
returns 0 then the SW will continue to configure the node accordingly.
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
net/bridge/br_mrp_switchdev.c | 171 +++++++++++++++++++++-------------
net/bridge/br_private_mrp.h | 24 +++--
2 files changed, 118 insertions(+), 77 deletions(-)
diff --git a/net/bridge/br_mrp_switchdev.c b/net/bridge/br_mrp_switchdev.c
index ed547e03ace1..ffd844e9be6b 100644
--- a/net/bridge/br_mrp_switchdev.c
+++ b/net/bridge/br_mrp_switchdev.c
@@ -4,6 +4,30 @@
#include "br_private_mrp.h"
+static enum br_mrp_hw_support
+br_mrp_switchdev_port_obj(struct net_bridge *br,
+ const struct switchdev_obj *obj, bool add)
+{
+ int err;
+
+ if (add)
+ err = switchdev_port_obj_add(br->dev, obj, NULL);
+ else
+ err = switchdev_port_obj_del(br->dev, obj);
+
+ /* In case of success just return and notify the SW that doesn't need
+ * to do anything
+ */
+ if (!err)
+ return BR_MRP_HW;
+
+ if (err != -EOPNOTSUPP)
+ return BR_MRP_NONE;
+
+ /* Continue with SW backup */
+ return BR_MRP_SW;
+}
+
int br_mrp_switchdev_add(struct net_bridge *br, struct br_mrp *mrp)
{
struct switchdev_obj_mrp mrp_obj = {
@@ -14,14 +38,11 @@ int br_mrp_switchdev_add(struct net_bridge *br, struct br_mrp *mrp)
.ring_id = mrp->ring_id,
.prio = mrp->prio,
};
- int err;
- err = switchdev_port_obj_add(br->dev, &mrp_obj.obj, NULL);
+ if (!IS_ENABLED(CONFIG_NET_SWITCHDEV))
+ return 0;
- if (err && err != -EOPNOTSUPP)
- return err;
-
- return 0;
+ return switchdev_port_obj_add(br->dev, &mrp_obj.obj, NULL);
}
int br_mrp_switchdev_del(struct net_bridge *br, struct br_mrp *mrp)
@@ -33,40 +54,54 @@ int br_mrp_switchdev_del(struct net_bridge *br, struct br_mrp *mrp)
.s_port = NULL,
.ring_id = mrp->ring_id,
};
- int err;
-
- err = switchdev_port_obj_del(br->dev, &mrp_obj.obj);
- if (err && err != -EOPNOTSUPP)
- return err;
+ if (!IS_ENABLED(CONFIG_NET_SWITCHDEV))
+ return 0;
- return 0;
+ return switchdev_port_obj_del(br->dev, &mrp_obj.obj);
}
-int br_mrp_switchdev_set_ring_role(struct net_bridge *br,
- struct br_mrp *mrp,
- enum br_mrp_ring_role_type role)
+enum br_mrp_hw_support
+br_mrp_switchdev_set_ring_role(struct net_bridge *br, struct br_mrp *mrp,
+ enum br_mrp_ring_role_type role)
{
struct switchdev_obj_ring_role_mrp mrp_role = {
.obj.orig_dev = br->dev,
.obj.id = SWITCHDEV_OBJ_ID_RING_ROLE_MRP,
.ring_role = role,
.ring_id = mrp->ring_id,
+ .sw_backup = false,
};
+ enum br_mrp_hw_support support;
int err;
- if (role == BR_MRP_RING_ROLE_DISABLED)
- err = switchdev_port_obj_del(br->dev, &mrp_role.obj);
- else
+ if (!IS_ENABLED(CONFIG_NET_SWITCHDEV))
+ return BR_MRP_SW;
+
+ support = br_mrp_switchdev_port_obj(br, &mrp_role.obj,
+ role != BR_MRP_RING_ROLE_DISABLED);
+ if (support != BR_MRP_SW)
+ return support;
+
+ /* If the driver can't configure to run completely the protocol in HW,
+ * then try again to configure the HW so the SW can run the protocol.
+ */
+ mrp_role.sw_backup = true;
+ if (role != BR_MRP_RING_ROLE_DISABLED)
err = switchdev_port_obj_add(br->dev, &mrp_role.obj, NULL);
+ else
+ err = switchdev_port_obj_del(br->dev, &mrp_role.obj);
- return err;
+ if (!err)
+ return BR_MRP_SW;
+
+ return BR_MRP_NONE;
}
-int br_mrp_switchdev_send_ring_test(struct net_bridge *br,
- struct br_mrp *mrp, u32 interval,
- u8 max_miss, u32 period,
- bool monitor)
+enum br_mrp_hw_support
+br_mrp_switchdev_send_ring_test(struct net_bridge *br, struct br_mrp *mrp,
+ u32 interval, u8 max_miss, u32 period,
+ bool monitor)
{
struct switchdev_obj_ring_test_mrp test = {
.obj.orig_dev = br->dev,
@@ -77,14 +112,11 @@ int br_mrp_switchdev_send_ring_test(struct net_bridge *br,
.period = period,
.monitor = monitor,
};
- int err;
- if (interval == 0)
- err = switchdev_port_obj_del(br->dev, &test.obj);
- else
- err = switchdev_port_obj_add(br->dev, &test.obj, NULL);
+ if (!IS_ENABLED(CONFIG_NET_SWITCHDEV))
+ return BR_MRP_SW;
- return err;
+ return br_mrp_switchdev_port_obj(br, &test.obj, interval != 0);
}
int br_mrp_switchdev_set_ring_state(struct net_bridge *br,
@@ -97,19 +129,17 @@ int br_mrp_switchdev_set_ring_state(struct net_bridge *br,
.ring_state = state,
.ring_id = mrp->ring_id,
};
- int err;
-
- err = switchdev_port_obj_add(br->dev, &mrp_state.obj, NULL);
- if (err && err != -EOPNOTSUPP)
- return err;
+ if (!IS_ENABLED(CONFIG_NET_SWITCHDEV))
+ return 0;
- return 0;
+ return switchdev_port_obj_add(br->dev, &mrp_state.obj, NULL);
}
-int br_mrp_switchdev_set_in_role(struct net_bridge *br, struct br_mrp *mrp,
- u16 in_id, u32 ring_id,
- enum br_mrp_in_role_type role)
+enum br_mrp_hw_support
+br_mrp_switchdev_set_in_role(struct net_bridge *br, struct br_mrp *mrp,
+ u16 in_id, u32 ring_id,
+ enum br_mrp_in_role_type role)
{
struct switchdev_obj_in_role_mrp mrp_role = {
.obj.orig_dev = br->dev,
@@ -118,15 +148,32 @@ int br_mrp_switchdev_set_in_role(struct net_bridge *br, struct br_mrp *mrp,
.in_id = mrp->in_id,
.ring_id = mrp->ring_id,
.i_port = rtnl_dereference(mrp->i_port)->dev,
+ .sw_backup = false,
};
+ enum br_mrp_hw_support support;
int err;
- if (role == BR_MRP_IN_ROLE_DISABLED)
- err = switchdev_port_obj_del(br->dev, &mrp_role.obj);
- else
+ if (!IS_ENABLED(CONFIG_NET_SWITCHDEV))
+ return BR_MRP_SW;
+
+ support = br_mrp_switchdev_port_obj(br, &mrp_role.obj,
+ role != BR_MRP_IN_ROLE_DISABLED);
+ if (support != BR_MRP_NONE)
+ return support;
+
+ /* If the driver can't configure to run completely the protocol in HW,
+ * then try again to configure the HW so the SW can run the protocol.
+ */
+ mrp_role.sw_backup = true;
+ if (role != BR_MRP_IN_ROLE_DISABLED)
err = switchdev_port_obj_add(br->dev, &mrp_role.obj, NULL);
+ else
+ err = switchdev_port_obj_del(br->dev, &mrp_role.obj);
+
+ if (!err)
+ return BR_MRP_SW;
- return err;
+ return BR_MRP_NONE;
}
int br_mrp_switchdev_set_in_state(struct net_bridge *br, struct br_mrp *mrp,
@@ -138,18 +185,16 @@ int br_mrp_switchdev_set_in_state(struct net_bridge *br, struct br_mrp *mrp,
.in_state = state,
.in_id = mrp->in_id,
};
- int err;
-
- err = switchdev_port_obj_add(br->dev, &mrp_state.obj, NULL);
- if (err && err != -EOPNOTSUPP)
- return err;
+ if (!IS_ENABLED(CONFIG_NET_SWITCHDEV))
+ return 0;
- return 0;
+ return switchdev_port_obj_add(br->dev, &mrp_state.obj, NULL);
}
-int br_mrp_switchdev_send_in_test(struct net_bridge *br, struct br_mrp *mrp,
- u32 interval, u8 max_miss, u32 period)
+enum br_mrp_hw_support
+br_mrp_switchdev_send_in_test(struct net_bridge *br, struct br_mrp *mrp,
+ u32 interval, u8 max_miss, u32 period)
{
struct switchdev_obj_in_test_mrp test = {
.obj.orig_dev = br->dev,
@@ -159,14 +204,11 @@ int br_mrp_switchdev_send_in_test(struct net_bridge *br, struct br_mrp *mrp,
.in_id = mrp->in_id,
.period = period,
};
- int err;
- if (interval == 0)
- err = switchdev_port_obj_del(br->dev, &test.obj);
- else
- err = switchdev_port_obj_add(br->dev, &test.obj, NULL);
+ if (!IS_ENABLED(CONFIG_NET_SWITCHDEV))
+ return BR_MRP_SW;
- return err;
+ return br_mrp_switchdev_port_obj(br, &test.obj, interval != 0);
}
int br_mrp_port_switchdev_set_state(struct net_bridge_port *p,
@@ -177,14 +219,11 @@ int br_mrp_port_switchdev_set_state(struct net_bridge_port *p,
.id = SWITCHDEV_ATTR_ID_MRP_PORT_STATE,
.u.mrp_port_state = state,
};
- int err;
- err = switchdev_port_attr_set(p->dev, &attr);
- if (err && err != -EOPNOTSUPP)
- br_warn(p->br, "error setting offload MRP state on port %u(%s)\n",
- (unsigned int)p->port_no, p->dev->name);
+ if (!IS_ENABLED(CONFIG_NET_SWITCHDEV))
+ return 0;
- return err;
+ return switchdev_port_attr_set(p->dev, &attr);
}
int br_mrp_port_switchdev_set_role(struct net_bridge_port *p,
@@ -195,11 +234,9 @@ int br_mrp_port_switchdev_set_role(struct net_bridge_port *p,
.id = SWITCHDEV_ATTR_ID_MRP_PORT_ROLE,
.u.mrp_port_role = role,
};
- int err;
- err = switchdev_port_attr_set(p->dev, &attr);
- if (err && err != -EOPNOTSUPP)
- return err;
+ if (!IS_ENABLED(CONFIG_NET_SWITCHDEV))
+ return 0;
- return 0;
+ return switchdev_port_attr_set(p->dev, &attr);
}
diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
index 31e666ae6955..e941dad398cf 100644
--- a/net/bridge/br_private_mrp.h
+++ b/net/bridge/br_private_mrp.h
@@ -79,24 +79,28 @@ int br_mrp_start_in_test(struct net_bridge *br,
/* br_mrp_switchdev.c */
int br_mrp_switchdev_add(struct net_bridge *br, struct br_mrp *mrp);
int br_mrp_switchdev_del(struct net_bridge *br, struct br_mrp *mrp);
-int br_mrp_switchdev_set_ring_role(struct net_bridge *br, struct br_mrp *mrp,
- enum br_mrp_ring_role_type role);
+enum br_mrp_hw_support
+br_mrp_switchdev_set_ring_role(struct net_bridge *br, struct br_mrp *mrp,
+ enum br_mrp_ring_role_type role);
int br_mrp_switchdev_set_ring_state(struct net_bridge *br, struct br_mrp *mrp,
enum br_mrp_ring_state_type state);
-int br_mrp_switchdev_send_ring_test(struct net_bridge *br, struct br_mrp *mrp,
- u32 interval, u8 max_miss, u32 period,
- bool monitor);
+enum br_mrp_hw_support
+br_mrp_switchdev_send_ring_test(struct net_bridge *br, struct br_mrp *mrp,
+ u32 interval, u8 max_miss, u32 period,
+ bool monitor);
int br_mrp_port_switchdev_set_state(struct net_bridge_port *p,
enum br_mrp_port_state_type state);
int br_mrp_port_switchdev_set_role(struct net_bridge_port *p,
enum br_mrp_port_role_type role);
-int br_mrp_switchdev_set_in_role(struct net_bridge *br, struct br_mrp *mrp,
- u16 in_id, u32 ring_id,
- enum br_mrp_in_role_type role);
+enum br_mrp_hw_support
+br_mrp_switchdev_set_in_role(struct net_bridge *br, struct br_mrp *mrp,
+ u16 in_id, u32 ring_id,
+ enum br_mrp_in_role_type role);
int br_mrp_switchdev_set_in_state(struct net_bridge *br, struct br_mrp *mrp,
enum br_mrp_in_state_type state);
-int br_mrp_switchdev_send_in_test(struct net_bridge *br, struct br_mrp *mrp,
- u32 interval, u8 max_miss, u32 period);
+enum br_mrp_hw_support
+br_mrp_switchdev_send_in_test(struct net_bridge *br, struct br_mrp *mrp,
+ u32 interval, u8 max_miss, u32 period);
/* br_mrp_netlink.c */
int br_mrp_ring_port_open(struct net_device *dev, u8 loc);
--
2.27.0
next prev parent reply other threads:[~2021-01-27 20:56 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-27 20:52 [PATCH net-next v2 0/4] bridge: mrp: Extend br_mrp_switchdev_* Horatiu Vultur
2021-01-27 20:52 ` [PATCH net-next v2 1/4] switchdev: mrp: Extend ring_role_mrp and in_role_mrp Horatiu Vultur
2021-01-27 20:52 ` [PATCH net-next v2 2/4] bridge: mrp: Add 'enum br_mrp_hw_support' Horatiu Vultur
2021-01-27 20:52 ` Horatiu Vultur [this message]
2021-01-27 20:52 ` [PATCH net-next v2 4/4] bridge: mrp: Update br_mrp to use new return values of br_mrp_switchdev Horatiu Vultur
2021-01-30 3:01 ` [PATCH net-next v2 0/4] bridge: mrp: Extend br_mrp_switchdev_* Jakub Kicinski
2021-02-02 7:40 ` Rasmus Villemoes
2021-02-02 19:50 ` Jakub Kicinski
2021-02-02 20:06 ` Horatiu Vultur
2021-02-02 20:49 ` Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210127205241.2864728-4-horatiu.vultur@microchip.com \
--to=horatiu.vultur@microchip.com \
--cc=andrew@lunn.ch \
--cc=bridge@lists.linux-foundation.org \
--cc=davem@davemloft.net \
--cc=ivecera@redhat.com \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nikolay@nvidia.com \
--cc=rasmus.villemoes@prevas.dk \
--cc=roopa@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).