* [PATCH net-next] bridge: mrp: Rework the MRP netlink interface
@ 2020-05-26 14:22 Horatiu Vultur
2020-05-27 8:59 ` Nikolay Aleksandrov
0 siblings, 1 reply; 3+ messages in thread
From: Horatiu Vultur @ 2020-05-26 14:22 UTC (permalink / raw)
To: nikolay, davem, kuba, roopa, mkubecek, netdev, linux-kernel, bridge
Cc: Horatiu Vultur
This patch rework the MRP netlink interface. Before, each attribute
represented a binary structure which made it hard to be extended.
Therefore update the MRP netlink interface such that each existing
attribute to be a nested attribute which contains the fields of the
binary structures.
In this way the MRP netlink interface can be extended without breaking
the backwards compatibility. It is also using strict checking for
attributes under the MRP top attribute.
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
include/uapi/linux/if_bridge.h | 64 ++++++++-
net/bridge/br_mrp.c | 8 +-
net/bridge/br_mrp_netlink.c | 248 ++++++++++++++++++++++++++++-----
net/bridge/br_private_mrp.h | 2 +-
4 files changed, 272 insertions(+), 50 deletions(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index bd8c95488f161..5a43eb86c93bf 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -169,17 +169,69 @@ enum {
__IFLA_BRIDGE_MRP_MAX,
};
+#define IFLA_BRIDGE_MRP_MAX (__IFLA_BRIDGE_MRP_MAX - 1)
+
+enum {
+ IFLA_BRIDGE_MRP_INSTANCE_UNSPEC,
+ IFLA_BRIDGE_MRP_INSTANCE_RING_ID,
+ IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX,
+ IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX,
+ __IFLA_BRIDGE_MRP_INSTANCE_MAX,
+};
+
+#define IFLA_BRIDGE_MRP_INSTANCE_MAX (__IFLA_BRIDGE_MRP_INSTANCE_MAX - 1)
+
+enum {
+ IFLA_BRIDGE_MRP_PORT_STATE_UNSPEC,
+ IFLA_BRIDGE_MRP_PORT_STATE_STATE,
+ __IFLA_BRIDGE_MRP_PORT_STATE_MAX,
+};
+
+#define IFLA_BRIDGE_MRP_PORT_STATE_MAX (__IFLA_BRIDGE_MRP_PORT_STATE_MAX - 1)
+
+enum {
+ IFLA_BRIDGE_MRP_PORT_ROLE_UNSPEC,
+ IFLA_BRIDGE_MRP_PORT_ROLE_ROLE,
+ __IFLA_BRIDGE_MRP_PORT_ROLE_MAX,
+};
+
+#define IFLA_BRIDGE_MRP_PORT_ROLE_MAX (__IFLA_BRIDGE_MRP_PORT_ROLE_MAX - 1)
+
+enum {
+ IFLA_BRIDGE_MRP_RING_STATE_UNSPEC,
+ IFLA_BRIDGE_MRP_RING_STATE_RING_ID,
+ IFLA_BRIDGE_MRP_RING_STATE_STATE,
+ __IFLA_BRIDGE_MRP_RING_STATE_MAX,
+};
+
+#define IFLA_BRIDGE_MRP_RING_STATE_MAX (__IFLA_BRIDGE_MRP_RING_STATE_MAX - 1)
+
+enum {
+ IFLA_BRIDGE_MRP_RING_ROLE_UNSPEC,
+ IFLA_BRIDGE_MRP_RING_ROLE_RING_ID,
+ IFLA_BRIDGE_MRP_RING_ROLE_ROLE,
+ __IFLA_BRIDGE_MRP_RING_ROLE_MAX,
+};
+
+#define IFLA_BRIDGE_MRP_RING_ROLE_MAX (__IFLA_BRIDGE_MRP_RING_ROLE_MAX - 1)
+
+enum {
+ IFLA_BRIDGE_MRP_START_TEST_UNSPEC,
+ IFLA_BRIDGE_MRP_START_TEST_RING_ID,
+ IFLA_BRIDGE_MRP_START_TEST_INTERVAL,
+ IFLA_BRIDGE_MRP_START_TEST_MAX_MISS,
+ IFLA_BRIDGE_MRP_START_TEST_PERIOD,
+ __IFLA_BRIDGE_MRP_START_TEST_MAX,
+};
+
+#define IFLA_BRIDGE_MRP_START_TEST_MAX (__IFLA_BRIDGE_MRP_START_TEST_MAX - 1)
+
struct br_mrp_instance {
__u32 ring_id;
__u32 p_ifindex;
__u32 s_ifindex;
};
-struct br_mrp_port_role {
- __u32 ring_id;
- __u32 role;
-};
-
struct br_mrp_ring_state {
__u32 ring_id;
__u32 ring_state;
@@ -197,8 +249,6 @@ struct br_mrp_start_test {
__u32 period;
};
-#define IFLA_BRIDGE_MRP_MAX (__IFLA_BRIDGE_MRP_MAX - 1)
-
struct bridge_stp_xstats {
__u64 transition_blk;
__u64 transition_fwd;
diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
index 528d767eb026f..8ea59504ef47a 100644
--- a/net/bridge/br_mrp.c
+++ b/net/bridge/br_mrp.c
@@ -376,24 +376,24 @@ int br_mrp_set_port_state(struct net_bridge_port *p,
* note: already called with rtnl_lock
*/
int br_mrp_set_port_role(struct net_bridge_port *p,
- struct br_mrp_port_role *role)
+ enum br_mrp_port_role_type role)
{
struct br_mrp *mrp;
if (!p || !(p->flags & BR_MRP_AWARE))
return -EINVAL;
- mrp = br_mrp_find_id(p->br, role->ring_id);
+ mrp = br_mrp_find_port(p->br, p);
if (!mrp)
return -EINVAL;
- if (role->role == BR_MRP_PORT_ROLE_PRIMARY)
+ if (role == BR_MRP_PORT_ROLE_PRIMARY)
rcu_assign_pointer(mrp->p_port, p);
else
rcu_assign_pointer(mrp->s_port, p);
- br_mrp_port_switchdev_set_role(p, role->role);
+ br_mrp_port_switchdev_set_role(p, role);
return 0;
}
diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
index 4a08a99519b04..cfad5d1cff050 100644
--- a/net/bridge/br_mrp_netlink.c
+++ b/net/bridge/br_mrp_netlink.c
@@ -8,19 +8,204 @@
static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = {
[IFLA_BRIDGE_MRP_UNSPEC] = { .type = NLA_REJECT },
- [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_EXACT_LEN,
- .len = sizeof(struct br_mrp_instance)},
- [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_U32 },
- [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_EXACT_LEN,
- .len = sizeof(struct br_mrp_port_role)},
- [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_EXACT_LEN,
- .len = sizeof(struct br_mrp_ring_state)},
- [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_EXACT_LEN,
- .len = sizeof(struct br_mrp_ring_role)},
- [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_EXACT_LEN,
- .len = sizeof(struct br_mrp_start_test)},
+ [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_NESTED },
+ [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_NESTED },
+ [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_NESTED },
+ [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_NESTED },
+ [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_NESTED },
+ [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_NESTED },
};
+static const struct nla_policy
+br_mrp_instance_policy[IFLA_BRIDGE_MRP_INSTANCE_MAX + 1] = {
+ [IFLA_BRIDGE_MRP_INSTANCE_UNSPEC] = { .type = NLA_REJECT },
+ [IFLA_BRIDGE_MRP_INSTANCE_RING_ID] = { .type = NLA_U32 },
+ [IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX] = { .type = NLA_U32 },
+ [IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX] = { .type = NLA_U32 },
+};
+
+int br_mrp_instance_parse(struct net_bridge *br, struct nlattr *attr,
+ int cmd, struct netlink_ext_ack *extack)
+{
+ struct nlattr *tb[IFLA_BRIDGE_MRP_INSTANCE_MAX + 1];
+ struct br_mrp_instance inst;
+ int err;
+
+ err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_INSTANCE_MAX, attr,
+ br_mrp_instance_policy, extack);
+ if (err)
+ return err;
+
+ if (!tb[IFLA_BRIDGE_MRP_INSTANCE_RING_ID] ||
+ !tb[IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX] ||
+ !tb[IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX])
+ return -EINVAL;
+
+ memset(&inst, 0, sizeof(inst));
+
+ inst.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_INSTANCE_RING_ID]);
+ inst.p_ifindex = nla_get_u32(tb[IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX]);
+ inst.s_ifindex = nla_get_u32(tb[IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX]);
+
+ if (cmd == RTM_SETLINK)
+ return br_mrp_add(br, &inst);
+ else
+ return br_mrp_del(br, &inst);
+
+ return 0;
+}
+
+static const struct nla_policy
+br_mrp_port_state_policy[IFLA_BRIDGE_MRP_PORT_STATE_MAX + 1] = {
+ [IFLA_BRIDGE_MRP_PORT_STATE_UNSPEC] = { .type = NLA_REJECT },
+ [IFLA_BRIDGE_MRP_PORT_STATE_STATE] = { .type = NLA_U32 },
+};
+
+int br_mrp_port_state_parse(struct net_bridge_port *p, struct nlattr *attr,
+ struct netlink_ext_ack *extack)
+{
+ struct nlattr *tb[IFLA_BRIDGE_MRP_PORT_STATE_MAX + 1];
+ enum br_mrp_port_state_type state;
+ int err;
+
+ err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_PORT_STATE_MAX, attr,
+ br_mrp_port_state_policy, extack);
+ if (err)
+ return err;
+
+ if (!tb[IFLA_BRIDGE_MRP_PORT_STATE_STATE])
+ return -EINVAL;
+
+ state = nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE_STATE]);
+
+ return br_mrp_set_port_state(p, state);
+}
+
+static const struct nla_policy
+br_mrp_port_role_policy[IFLA_BRIDGE_MRP_PORT_ROLE_MAX + 1] = {
+ [IFLA_BRIDGE_MRP_PORT_ROLE_UNSPEC] = { .type = NLA_REJECT },
+ [IFLA_BRIDGE_MRP_PORT_ROLE_ROLE] = { .type = NLA_U32 },
+};
+
+int br_mrp_port_role_parse(struct net_bridge_port *p, struct nlattr *attr,
+ struct netlink_ext_ack *extack)
+{
+ struct nlattr *tb[IFLA_BRIDGE_MRP_PORT_ROLE_MAX + 1];
+ enum br_mrp_port_role_type role;
+ int err;
+
+ err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_PORT_ROLE_MAX, attr,
+ br_mrp_port_role_policy, extack);
+ if (err)
+ return err;
+
+ if (!tb[IFLA_BRIDGE_MRP_PORT_ROLE_ROLE])
+ return -EINVAL;
+
+ role = nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_ROLE_ROLE]);
+
+ return br_mrp_set_port_role(p, role);
+}
+
+static const struct nla_policy
+br_mrp_ring_state_policy[IFLA_BRIDGE_MRP_RING_STATE_MAX + 1] = {
+ [IFLA_BRIDGE_MRP_RING_STATE_UNSPEC] = { .type = NLA_REJECT },
+ [IFLA_BRIDGE_MRP_RING_STATE_RING_ID] = { .type = NLA_U32 },
+ [IFLA_BRIDGE_MRP_RING_STATE_STATE] = { .type = NLA_U32 },
+};
+
+int br_mrp_ring_state_parse(struct net_bridge *br, struct nlattr *attr,
+ struct netlink_ext_ack *extack)
+{
+ struct nlattr *tb[IFLA_BRIDGE_MRP_RING_STATE_MAX + 1];
+ struct br_mrp_ring_state state;
+ int err;
+
+ err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_RING_STATE_MAX, attr,
+ br_mrp_ring_state_policy, extack);
+ if (err)
+ return err;
+
+ if (!tb[IFLA_BRIDGE_MRP_RING_STATE_RING_ID] ||
+ !tb[IFLA_BRIDGE_MRP_RING_STATE_STATE])
+ return -EINVAL;
+
+ memset(&state, 0x0, sizeof(state));
+
+ state.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_STATE_RING_ID]);
+ state.ring_state = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_STATE_STATE]);
+
+ return br_mrp_set_ring_state(br, &state);
+}
+
+static const struct nla_policy
+br_mrp_ring_role_policy[IFLA_BRIDGE_MRP_RING_ROLE_MAX + 1] = {
+ [IFLA_BRIDGE_MRP_RING_ROLE_UNSPEC] = { .type = NLA_REJECT },
+ [IFLA_BRIDGE_MRP_RING_ROLE_RING_ID] = { .type = NLA_U32 },
+ [IFLA_BRIDGE_MRP_RING_ROLE_ROLE] = { .type = NLA_U32 },
+};
+
+int br_mrp_ring_role_parse(struct net_bridge *br, struct nlattr *attr,
+ struct netlink_ext_ack *extack)
+{
+ struct nlattr *tb[IFLA_BRIDGE_MRP_RING_ROLE_MAX + 1];
+ struct br_mrp_ring_role role;
+ int err;
+
+ err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_RING_ROLE_MAX, attr,
+ br_mrp_ring_role_policy, extack);
+ if (err)
+ return err;
+
+ if (!tb[IFLA_BRIDGE_MRP_RING_ROLE_RING_ID] ||
+ !tb[IFLA_BRIDGE_MRP_RING_ROLE_ROLE])
+ return -EINVAL;
+
+ memset(&role, 0x0, sizeof(role));
+
+ role.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_ROLE_RING_ID]);
+ role.ring_role = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_ROLE_ROLE]);
+
+ return br_mrp_set_ring_role(br, &role);
+}
+
+static const struct nla_policy
+br_mrp_start_test_policy[IFLA_BRIDGE_MRP_START_TEST_MAX + 1] = {
+ [IFLA_BRIDGE_MRP_START_TEST_UNSPEC] = { .type = NLA_REJECT },
+ [IFLA_BRIDGE_MRP_START_TEST_RING_ID] = { .type = NLA_U32 },
+ [IFLA_BRIDGE_MRP_START_TEST_INTERVAL] = { .type = NLA_U32 },
+ [IFLA_BRIDGE_MRP_START_TEST_MAX_MISS] = { .type = NLA_U32 },
+ [IFLA_BRIDGE_MRP_START_TEST_PERIOD] = { .type = NLA_U32 },
+};
+
+int br_mrp_start_test_parse(struct net_bridge *br, struct nlattr *attr,
+ struct netlink_ext_ack *extack)
+{
+ struct nlattr *tb[IFLA_BRIDGE_MRP_START_TEST_MAX + 1];
+ struct br_mrp_start_test test;
+ int err;
+
+ err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_START_TEST_MAX, attr,
+ br_mrp_start_test_policy, extack);
+ if (err)
+ return err;
+
+ if (!tb[IFLA_BRIDGE_MRP_START_TEST_RING_ID] ||
+ !tb[IFLA_BRIDGE_MRP_START_TEST_INTERVAL] ||
+ !tb[IFLA_BRIDGE_MRP_START_TEST_MAX_MISS] ||
+ !tb[IFLA_BRIDGE_MRP_START_TEST_PERIOD])
+ return -EINVAL;
+
+ memset(&test, 0x0, sizeof(test));
+
+ test.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_RING_ID]);
+ test.interval = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_INTERVAL]);
+ test.max_miss = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_MAX_MISS]);
+ test.period = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_PERIOD]);
+
+ return br_mrp_start_test(br, &test);
+}
+
int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
{
@@ -44,58 +229,45 @@ int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
return err;
if (tb[IFLA_BRIDGE_MRP_INSTANCE]) {
- struct br_mrp_instance *instance =
- nla_data(tb[IFLA_BRIDGE_MRP_INSTANCE]);
-
- if (cmd == RTM_SETLINK)
- err = br_mrp_add(br, instance);
- else
- err = br_mrp_del(br, instance);
+ err = br_mrp_instance_parse(br, tb[IFLA_BRIDGE_MRP_INSTANCE],
+ cmd, extack);
if (err)
return err;
}
if (tb[IFLA_BRIDGE_MRP_PORT_STATE]) {
- enum br_mrp_port_state_type state =
- nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE]);
-
- err = br_mrp_set_port_state(p, state);
+ err = br_mrp_port_state_parse(p, tb[IFLA_BRIDGE_MRP_PORT_STATE],
+ extack);
if (err)
return err;
}
if (tb[IFLA_BRIDGE_MRP_PORT_ROLE]) {
- struct br_mrp_port_role *role =
- nla_data(tb[IFLA_BRIDGE_MRP_PORT_ROLE]);
-
- err = br_mrp_set_port_role(p, role);
+ err = br_mrp_port_role_parse(p, tb[IFLA_BRIDGE_MRP_PORT_ROLE],
+ extack);
if (err)
return err;
}
if (tb[IFLA_BRIDGE_MRP_RING_STATE]) {
- struct br_mrp_ring_state *state =
- nla_data(tb[IFLA_BRIDGE_MRP_RING_STATE]);
-
- err = br_mrp_set_ring_state(br, state);
+ err = br_mrp_ring_state_parse(br,
+ tb[IFLA_BRIDGE_MRP_RING_STATE],
+ extack);
if (err)
return err;
}
if (tb[IFLA_BRIDGE_MRP_RING_ROLE]) {
- struct br_mrp_ring_role *role =
- nla_data(tb[IFLA_BRIDGE_MRP_RING_ROLE]);
-
- err = br_mrp_set_ring_role(br, role);
+ err = br_mrp_ring_role_parse(br, tb[IFLA_BRIDGE_MRP_RING_ROLE],
+ extack);
if (err)
return err;
}
if (tb[IFLA_BRIDGE_MRP_START_TEST]) {
- struct br_mrp_start_test *test =
- nla_data(tb[IFLA_BRIDGE_MRP_START_TEST]);
-
- err = br_mrp_start_test(br, test);
+ err = br_mrp_start_test_parse(br,
+ tb[IFLA_BRIDGE_MRP_START_TEST],
+ extack);
if (err)
return err;
}
diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
index 2921a4b59f8e7..a0f53cc3ab85c 100644
--- a/net/bridge/br_private_mrp.h
+++ b/net/bridge/br_private_mrp.h
@@ -37,7 +37,7 @@ int br_mrp_del(struct net_bridge *br, struct br_mrp_instance *instance);
int br_mrp_set_port_state(struct net_bridge_port *p,
enum br_mrp_port_state_type state);
int br_mrp_set_port_role(struct net_bridge_port *p,
- struct br_mrp_port_role *role);
+ enum br_mrp_port_role_type role);
int br_mrp_set_ring_state(struct net_bridge *br,
struct br_mrp_ring_state *state);
int br_mrp_set_ring_role(struct net_bridge *br, struct br_mrp_ring_role *role);
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] bridge: mrp: Rework the MRP netlink interface
2020-05-26 14:22 [PATCH net-next] bridge: mrp: Rework the MRP netlink interface Horatiu Vultur
@ 2020-05-27 8:59 ` Nikolay Aleksandrov
2020-05-27 12:06 ` Horatiu Vultur
0 siblings, 1 reply; 3+ messages in thread
From: Nikolay Aleksandrov @ 2020-05-27 8:59 UTC (permalink / raw)
To: Horatiu Vultur, davem, kuba, roopa, mkubecek, netdev,
linux-kernel, bridge
On 26/05/2020 17:22, Horatiu Vultur wrote:
> This patch rework the MRP netlink interface. Before, each attribute
> represented a binary structure which made it hard to be extended.
> Therefore update the MRP netlink interface such that each existing
> attribute to be a nested attribute which contains the fields of the
> binary structures.
> In this way the MRP netlink interface can be extended without breaking
> the backwards compatibility. It is also using strict checking for
> attributes under the MRP top attribute.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
> include/uapi/linux/if_bridge.h | 64 ++++++++-
> net/bridge/br_mrp.c | 8 +-
> net/bridge/br_mrp_netlink.c | 248 ++++++++++++++++++++++++++++-----
> net/bridge/br_private_mrp.h | 2 +-
> 4 files changed, 272 insertions(+), 50 deletions(-)
>
Hi Horatiu,
I think the functions below should be static. Also you could do better with the error
reporting. It seems extack is passed down, so you could return something meaningful for
each missing value.
net/bridge/br_mrp_netlink.c:27:5: warning: no previous prototype for ‘br_mrp_instance_parse’ [-Wmissing-prototypes]
27 | int br_mrp_instance_parse(struct net_bridge *br, struct nlattr *attr,
| ^~~~~~~~~~~~~~~~~~~~~
net/bridge/br_mrp_netlink.c:64:5: warning: no previous prototype for ‘br_mrp_port_state_parse’ [-Wmissing-prototypes]
64 | int br_mrp_port_state_parse(struct net_bridge_port *p, struct nlattr *attr,
| ^~~~~~~~~~~~~~~~~~~~~~~
net/bridge/br_mrp_netlink.c:90:5: warning: no previous prototype for ‘br_mrp_port_role_parse’ [-Wmissing-prototypes]
90 | int br_mrp_port_role_parse(struct net_bridge_port *p, struct nlattr *attr,
| ^~~~~~~~~~~~~~~~~~~~~~
net/bridge/br_mrp_netlink.c:117:5: warning: no previous prototype for ‘br_mrp_ring_state_parse’ [-Wmissing-prototypes]
117 | int br_mrp_ring_state_parse(struct net_bridge *br, struct nlattr *attr,
| ^~~~~~~~~~~~~~~~~~~~~~~
net/bridge/br_mrp_netlink.c:148:5: warning: no previous prototype for ‘br_mrp_ring_role_parse’ [-Wmissing-prototypes]
148 | int br_mrp_ring_role_parse(struct net_bridge *br, struct nlattr *attr,
| ^~~~~~~~~~~~~~~~~~~~~~
net/bridge/br_mrp_netlink.c:181:5: warning: no previous prototype for ‘br_mrp_start_test_parse’ [-Wmissing-prototypes]
181 | int br_mrp_start_test_parse(struct net_bridge *br, struct nlattr *attr,
| ^~~~~~~~~~~~~~~~~~~~~~~
Cheers,
Nik
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index bd8c95488f161..5a43eb86c93bf 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -169,17 +169,69 @@ enum {
> __IFLA_BRIDGE_MRP_MAX,
> };
>
> +#define IFLA_BRIDGE_MRP_MAX (__IFLA_BRIDGE_MRP_MAX - 1)
> +
> +enum {
> + IFLA_BRIDGE_MRP_INSTANCE_UNSPEC,
> + IFLA_BRIDGE_MRP_INSTANCE_RING_ID,
> + IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX,
> + IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX,
> + __IFLA_BRIDGE_MRP_INSTANCE_MAX,
> +};
> +
> +#define IFLA_BRIDGE_MRP_INSTANCE_MAX (__IFLA_BRIDGE_MRP_INSTANCE_MAX - 1)
> +
> +enum {
> + IFLA_BRIDGE_MRP_PORT_STATE_UNSPEC,
> + IFLA_BRIDGE_MRP_PORT_STATE_STATE,
> + __IFLA_BRIDGE_MRP_PORT_STATE_MAX,
> +};
> +
> +#define IFLA_BRIDGE_MRP_PORT_STATE_MAX (__IFLA_BRIDGE_MRP_PORT_STATE_MAX - 1)
> +
> +enum {
> + IFLA_BRIDGE_MRP_PORT_ROLE_UNSPEC,
> + IFLA_BRIDGE_MRP_PORT_ROLE_ROLE,
> + __IFLA_BRIDGE_MRP_PORT_ROLE_MAX,
> +};
> +
> +#define IFLA_BRIDGE_MRP_PORT_ROLE_MAX (__IFLA_BRIDGE_MRP_PORT_ROLE_MAX - 1)
> +
> +enum {
> + IFLA_BRIDGE_MRP_RING_STATE_UNSPEC,
> + IFLA_BRIDGE_MRP_RING_STATE_RING_ID,
> + IFLA_BRIDGE_MRP_RING_STATE_STATE,
> + __IFLA_BRIDGE_MRP_RING_STATE_MAX,
> +};
> +
> +#define IFLA_BRIDGE_MRP_RING_STATE_MAX (__IFLA_BRIDGE_MRP_RING_STATE_MAX - 1)
> +
> +enum {
> + IFLA_BRIDGE_MRP_RING_ROLE_UNSPEC,
> + IFLA_BRIDGE_MRP_RING_ROLE_RING_ID,
> + IFLA_BRIDGE_MRP_RING_ROLE_ROLE,
> + __IFLA_BRIDGE_MRP_RING_ROLE_MAX,
> +};
> +
> +#define IFLA_BRIDGE_MRP_RING_ROLE_MAX (__IFLA_BRIDGE_MRP_RING_ROLE_MAX - 1)
> +
> +enum {
> + IFLA_BRIDGE_MRP_START_TEST_UNSPEC,
> + IFLA_BRIDGE_MRP_START_TEST_RING_ID,
> + IFLA_BRIDGE_MRP_START_TEST_INTERVAL,
> + IFLA_BRIDGE_MRP_START_TEST_MAX_MISS,
> + IFLA_BRIDGE_MRP_START_TEST_PERIOD,
> + __IFLA_BRIDGE_MRP_START_TEST_MAX,
> +};
> +
> +#define IFLA_BRIDGE_MRP_START_TEST_MAX (__IFLA_BRIDGE_MRP_START_TEST_MAX - 1)
> +
> struct br_mrp_instance {
> __u32 ring_id;
> __u32 p_ifindex;
> __u32 s_ifindex;
> };
>
> -struct br_mrp_port_role {
> - __u32 ring_id;
> - __u32 role;
> -};
> -
> struct br_mrp_ring_state {
> __u32 ring_id;
> __u32 ring_state;
> @@ -197,8 +249,6 @@ struct br_mrp_start_test {
> __u32 period;
> };
>
> -#define IFLA_BRIDGE_MRP_MAX (__IFLA_BRIDGE_MRP_MAX - 1)
> -
> struct bridge_stp_xstats {
> __u64 transition_blk;
> __u64 transition_fwd;
> diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> index 528d767eb026f..8ea59504ef47a 100644
> --- a/net/bridge/br_mrp.c
> +++ b/net/bridge/br_mrp.c
> @@ -376,24 +376,24 @@ int br_mrp_set_port_state(struct net_bridge_port *p,
> * note: already called with rtnl_lock
> */
> int br_mrp_set_port_role(struct net_bridge_port *p,
> - struct br_mrp_port_role *role)
> + enum br_mrp_port_role_type role)
> {
> struct br_mrp *mrp;
>
> if (!p || !(p->flags & BR_MRP_AWARE))
> return -EINVAL;
>
> - mrp = br_mrp_find_id(p->br, role->ring_id);
> + mrp = br_mrp_find_port(p->br, p);
>
> if (!mrp)
> return -EINVAL;
>
> - if (role->role == BR_MRP_PORT_ROLE_PRIMARY)
> + if (role == BR_MRP_PORT_ROLE_PRIMARY)
> rcu_assign_pointer(mrp->p_port, p);
> else
> rcu_assign_pointer(mrp->s_port, p);
>
> - br_mrp_port_switchdev_set_role(p, role->role);
> + br_mrp_port_switchdev_set_role(p, role);
>
> return 0;
> }
> diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
> index 4a08a99519b04..cfad5d1cff050 100644
> --- a/net/bridge/br_mrp_netlink.c
> +++ b/net/bridge/br_mrp_netlink.c
> @@ -8,19 +8,204 @@
>
> static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = {
> [IFLA_BRIDGE_MRP_UNSPEC] = { .type = NLA_REJECT },
> - [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_EXACT_LEN,
> - .len = sizeof(struct br_mrp_instance)},
> - [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_U32 },
> - [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_EXACT_LEN,
> - .len = sizeof(struct br_mrp_port_role)},
> - [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_EXACT_LEN,
> - .len = sizeof(struct br_mrp_ring_state)},
> - [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_EXACT_LEN,
> - .len = sizeof(struct br_mrp_ring_role)},
> - [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_EXACT_LEN,
> - .len = sizeof(struct br_mrp_start_test)},
> + [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_NESTED },
> + [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_NESTED },
> };
>
> +static const struct nla_policy
> +br_mrp_instance_policy[IFLA_BRIDGE_MRP_INSTANCE_MAX + 1] = {
> + [IFLA_BRIDGE_MRP_INSTANCE_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_MRP_INSTANCE_RING_ID] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX] = { .type = NLA_U32 },
> +};
> +
> +int br_mrp_instance_parse(struct net_bridge *br, struct nlattr *attr,
> + int cmd, struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_MRP_INSTANCE_MAX + 1];
> + struct br_mrp_instance inst;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_INSTANCE_MAX, attr,
> + br_mrp_instance_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_MRP_INSTANCE_RING_ID] ||
> + !tb[IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX] ||
> + !tb[IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX])
> + return -EINVAL;
> +
> + memset(&inst, 0, sizeof(inst));
> +
> + inst.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_INSTANCE_RING_ID]);
> + inst.p_ifindex = nla_get_u32(tb[IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX]);
> + inst.s_ifindex = nla_get_u32(tb[IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX]);
> +
> + if (cmd == RTM_SETLINK)
> + return br_mrp_add(br, &inst);
> + else
> + return br_mrp_del(br, &inst);
> +
> + return 0;
> +}
> +
> +static const struct nla_policy
> +br_mrp_port_state_policy[IFLA_BRIDGE_MRP_PORT_STATE_MAX + 1] = {
> + [IFLA_BRIDGE_MRP_PORT_STATE_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_MRP_PORT_STATE_STATE] = { .type = NLA_U32 },
> +};
> +
> +int br_mrp_port_state_parse(struct net_bridge_port *p, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_MRP_PORT_STATE_MAX + 1];
> + enum br_mrp_port_state_type state;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_PORT_STATE_MAX, attr,
> + br_mrp_port_state_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_MRP_PORT_STATE_STATE])
> + return -EINVAL;
> +
> + state = nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE_STATE]);
> +
> + return br_mrp_set_port_state(p, state);
> +}
> +
> +static const struct nla_policy
> +br_mrp_port_role_policy[IFLA_BRIDGE_MRP_PORT_ROLE_MAX + 1] = {
> + [IFLA_BRIDGE_MRP_PORT_ROLE_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_MRP_PORT_ROLE_ROLE] = { .type = NLA_U32 },
> +};
> +
> +int br_mrp_port_role_parse(struct net_bridge_port *p, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_MRP_PORT_ROLE_MAX + 1];
> + enum br_mrp_port_role_type role;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_PORT_ROLE_MAX, attr,
> + br_mrp_port_role_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_MRP_PORT_ROLE_ROLE])
> + return -EINVAL;
> +
> + role = nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_ROLE_ROLE]);
> +
> + return br_mrp_set_port_role(p, role);
> +}
> +
> +static const struct nla_policy
> +br_mrp_ring_state_policy[IFLA_BRIDGE_MRP_RING_STATE_MAX + 1] = {
> + [IFLA_BRIDGE_MRP_RING_STATE_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_MRP_RING_STATE_RING_ID] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_MRP_RING_STATE_STATE] = { .type = NLA_U32 },
> +};
> +
> +int br_mrp_ring_state_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_MRP_RING_STATE_MAX + 1];
> + struct br_mrp_ring_state state;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_RING_STATE_MAX, attr,
> + br_mrp_ring_state_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_MRP_RING_STATE_RING_ID] ||
> + !tb[IFLA_BRIDGE_MRP_RING_STATE_STATE])
> + return -EINVAL;
> +
> + memset(&state, 0x0, sizeof(state));
> +
> + state.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_STATE_RING_ID]);
> + state.ring_state = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_STATE_STATE]);
> +
> + return br_mrp_set_ring_state(br, &state);
> +}
> +
> +static const struct nla_policy
> +br_mrp_ring_role_policy[IFLA_BRIDGE_MRP_RING_ROLE_MAX + 1] = {
> + [IFLA_BRIDGE_MRP_RING_ROLE_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_MRP_RING_ROLE_RING_ID] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_MRP_RING_ROLE_ROLE] = { .type = NLA_U32 },
> +};
> +
> +int br_mrp_ring_role_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_MRP_RING_ROLE_MAX + 1];
> + struct br_mrp_ring_role role;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_RING_ROLE_MAX, attr,
> + br_mrp_ring_role_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_MRP_RING_ROLE_RING_ID] ||
> + !tb[IFLA_BRIDGE_MRP_RING_ROLE_ROLE])
> + return -EINVAL;
> +
> + memset(&role, 0x0, sizeof(role));
> +
> + role.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_ROLE_RING_ID]);
> + role.ring_role = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_ROLE_ROLE]);
> +
> + return br_mrp_set_ring_role(br, &role);
> +}
> +
> +static const struct nla_policy
> +br_mrp_start_test_policy[IFLA_BRIDGE_MRP_START_TEST_MAX + 1] = {
> + [IFLA_BRIDGE_MRP_START_TEST_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_MRP_START_TEST_RING_ID] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_MRP_START_TEST_INTERVAL] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_MRP_START_TEST_MAX_MISS] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_MRP_START_TEST_PERIOD] = { .type = NLA_U32 },
> +};
> +
> +int br_mrp_start_test_parse(struct net_bridge *br, struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_MRP_START_TEST_MAX + 1];
> + struct br_mrp_start_test test;
> + int err;
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_START_TEST_MAX, attr,
> + br_mrp_start_test_policy, extack);
> + if (err)
> + return err;
> +
> + if (!tb[IFLA_BRIDGE_MRP_START_TEST_RING_ID] ||
> + !tb[IFLA_BRIDGE_MRP_START_TEST_INTERVAL] ||
> + !tb[IFLA_BRIDGE_MRP_START_TEST_MAX_MISS] ||
> + !tb[IFLA_BRIDGE_MRP_START_TEST_PERIOD])
> + return -EINVAL;
> +
> + memset(&test, 0x0, sizeof(test));
> +
> + test.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_RING_ID]);
> + test.interval = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_INTERVAL]);
> + test.max_miss = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_MAX_MISS]);
> + test.period = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_PERIOD]);
> +
> + return br_mrp_start_test(br, &test);
> +}
> +
> int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
> struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
> {
> @@ -44,58 +229,45 @@ int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
> return err;
>
> if (tb[IFLA_BRIDGE_MRP_INSTANCE]) {
> - struct br_mrp_instance *instance =
> - nla_data(tb[IFLA_BRIDGE_MRP_INSTANCE]);
> -
> - if (cmd == RTM_SETLINK)
> - err = br_mrp_add(br, instance);
> - else
> - err = br_mrp_del(br, instance);
> + err = br_mrp_instance_parse(br, tb[IFLA_BRIDGE_MRP_INSTANCE],
> + cmd, extack);
> if (err)
> return err;
> }
>
> if (tb[IFLA_BRIDGE_MRP_PORT_STATE]) {
> - enum br_mrp_port_state_type state =
> - nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE]);
> -
> - err = br_mrp_set_port_state(p, state);
> + err = br_mrp_port_state_parse(p, tb[IFLA_BRIDGE_MRP_PORT_STATE],
> + extack);
> if (err)
> return err;
> }
>
> if (tb[IFLA_BRIDGE_MRP_PORT_ROLE]) {
> - struct br_mrp_port_role *role =
> - nla_data(tb[IFLA_BRIDGE_MRP_PORT_ROLE]);
> -
> - err = br_mrp_set_port_role(p, role);
> + err = br_mrp_port_role_parse(p, tb[IFLA_BRIDGE_MRP_PORT_ROLE],
> + extack);
> if (err)
> return err;
> }
>
> if (tb[IFLA_BRIDGE_MRP_RING_STATE]) {
> - struct br_mrp_ring_state *state =
> - nla_data(tb[IFLA_BRIDGE_MRP_RING_STATE]);
> -
> - err = br_mrp_set_ring_state(br, state);
> + err = br_mrp_ring_state_parse(br,
> + tb[IFLA_BRIDGE_MRP_RING_STATE],
> + extack);
> if (err)
> return err;
> }
>
> if (tb[IFLA_BRIDGE_MRP_RING_ROLE]) {
> - struct br_mrp_ring_role *role =
> - nla_data(tb[IFLA_BRIDGE_MRP_RING_ROLE]);
> -
> - err = br_mrp_set_ring_role(br, role);
> + err = br_mrp_ring_role_parse(br, tb[IFLA_BRIDGE_MRP_RING_ROLE],
> + extack);
> if (err)
> return err;
> }
>
> if (tb[IFLA_BRIDGE_MRP_START_TEST]) {
> - struct br_mrp_start_test *test =
> - nla_data(tb[IFLA_BRIDGE_MRP_START_TEST]);
> -
> - err = br_mrp_start_test(br, test);
> + err = br_mrp_start_test_parse(br,
> + tb[IFLA_BRIDGE_MRP_START_TEST],
> + extack);
> if (err)
> return err;
> }
> diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
> index 2921a4b59f8e7..a0f53cc3ab85c 100644
> --- a/net/bridge/br_private_mrp.h
> +++ b/net/bridge/br_private_mrp.h
> @@ -37,7 +37,7 @@ int br_mrp_del(struct net_bridge *br, struct br_mrp_instance *instance);
> int br_mrp_set_port_state(struct net_bridge_port *p,
> enum br_mrp_port_state_type state);
> int br_mrp_set_port_role(struct net_bridge_port *p,
> - struct br_mrp_port_role *role);
> + enum br_mrp_port_role_type role);
> int br_mrp_set_ring_state(struct net_bridge *br,
> struct br_mrp_ring_state *state);
> int br_mrp_set_ring_role(struct net_bridge *br, struct br_mrp_ring_role *role);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] bridge: mrp: Rework the MRP netlink interface
2020-05-27 8:59 ` Nikolay Aleksandrov
@ 2020-05-27 12:06 ` Horatiu Vultur
0 siblings, 0 replies; 3+ messages in thread
From: Horatiu Vultur @ 2020-05-27 12:06 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: davem, kuba, roopa, mkubecek, netdev, linux-kernel, bridge
The 05/27/2020 11:59, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 26/05/2020 17:22, Horatiu Vultur wrote:
> > This patch rework the MRP netlink interface. Before, each attribute
> > represented a binary structure which made it hard to be extended.
> > Therefore update the MRP netlink interface such that each existing
> > attribute to be a nested attribute which contains the fields of the
> > binary structures.
> > In this way the MRP netlink interface can be extended without breaking
> > the backwards compatibility. It is also using strict checking for
> > attributes under the MRP top attribute.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> > include/uapi/linux/if_bridge.h | 64 ++++++++-
> > net/bridge/br_mrp.c | 8 +-
> > net/bridge/br_mrp_netlink.c | 248 ++++++++++++++++++++++++++++-----
> > net/bridge/br_private_mrp.h | 2 +-
> > 4 files changed, 272 insertions(+), 50 deletions(-)
> >
>
> Hi Horatiu,
> I think the functions below should be static. Also you could do better with the error
> reporting. It seems extack is passed down, so you could return something meaningful for
> each missing value.
Hi Nik,
Yes, they should be static, they should not be used outside of this
file.
I will create a new patch, where I will fix this and add more
meaningful messages in case of a missing value.
>
> net/bridge/br_mrp_netlink.c:27:5: warning: no previous prototype for ‘br_mrp_instance_parse’ [-Wmissing-prototypes]
> 27 | int br_mrp_instance_parse(struct net_bridge *br, struct nlattr *attr,
> | ^~~~~~~~~~~~~~~~~~~~~
> net/bridge/br_mrp_netlink.c:64:5: warning: no previous prototype for ‘br_mrp_port_state_parse’ [-Wmissing-prototypes]
> 64 | int br_mrp_port_state_parse(struct net_bridge_port *p, struct nlattr *attr,
> | ^~~~~~~~~~~~~~~~~~~~~~~
> net/bridge/br_mrp_netlink.c:90:5: warning: no previous prototype for ‘br_mrp_port_role_parse’ [-Wmissing-prototypes]
> 90 | int br_mrp_port_role_parse(struct net_bridge_port *p, struct nlattr *attr,
> | ^~~~~~~~~~~~~~~~~~~~~~
> net/bridge/br_mrp_netlink.c:117:5: warning: no previous prototype for ‘br_mrp_ring_state_parse’ [-Wmissing-prototypes]
> 117 | int br_mrp_ring_state_parse(struct net_bridge *br, struct nlattr *attr,
> | ^~~~~~~~~~~~~~~~~~~~~~~
> net/bridge/br_mrp_netlink.c:148:5: warning: no previous prototype for ‘br_mrp_ring_role_parse’ [-Wmissing-prototypes]
> 148 | int br_mrp_ring_role_parse(struct net_bridge *br, struct nlattr *attr,
> | ^~~~~~~~~~~~~~~~~~~~~~
> net/bridge/br_mrp_netlink.c:181:5: warning: no previous prototype for ‘br_mrp_start_test_parse’ [-Wmissing-prototypes]
> 181 | int br_mrp_start_test_parse(struct net_bridge *br, struct nlattr *attr,
> | ^~~~~~~~~~~~~~~~~~~~~~~
>
>
> Cheers,
> Nik
>
> > diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> > index bd8c95488f161..5a43eb86c93bf 100644
> > --- a/include/uapi/linux/if_bridge.h
> > +++ b/include/uapi/linux/if_bridge.h
> > @@ -169,17 +169,69 @@ enum {
> > __IFLA_BRIDGE_MRP_MAX,
> > };
> >
> > +#define IFLA_BRIDGE_MRP_MAX (__IFLA_BRIDGE_MRP_MAX - 1)
> > +
> > +enum {
> > + IFLA_BRIDGE_MRP_INSTANCE_UNSPEC,
> > + IFLA_BRIDGE_MRP_INSTANCE_RING_ID,
> > + IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX,
> > + IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX,
> > + __IFLA_BRIDGE_MRP_INSTANCE_MAX,
> > +};
> > +
> > +#define IFLA_BRIDGE_MRP_INSTANCE_MAX (__IFLA_BRIDGE_MRP_INSTANCE_MAX - 1)
> > +
> > +enum {
> > + IFLA_BRIDGE_MRP_PORT_STATE_UNSPEC,
> > + IFLA_BRIDGE_MRP_PORT_STATE_STATE,
> > + __IFLA_BRIDGE_MRP_PORT_STATE_MAX,
> > +};
> > +
> > +#define IFLA_BRIDGE_MRP_PORT_STATE_MAX (__IFLA_BRIDGE_MRP_PORT_STATE_MAX - 1)
> > +
> > +enum {
> > + IFLA_BRIDGE_MRP_PORT_ROLE_UNSPEC,
> > + IFLA_BRIDGE_MRP_PORT_ROLE_ROLE,
> > + __IFLA_BRIDGE_MRP_PORT_ROLE_MAX,
> > +};
> > +
> > +#define IFLA_BRIDGE_MRP_PORT_ROLE_MAX (__IFLA_BRIDGE_MRP_PORT_ROLE_MAX - 1)
> > +
> > +enum {
> > + IFLA_BRIDGE_MRP_RING_STATE_UNSPEC,
> > + IFLA_BRIDGE_MRP_RING_STATE_RING_ID,
> > + IFLA_BRIDGE_MRP_RING_STATE_STATE,
> > + __IFLA_BRIDGE_MRP_RING_STATE_MAX,
> > +};
> > +
> > +#define IFLA_BRIDGE_MRP_RING_STATE_MAX (__IFLA_BRIDGE_MRP_RING_STATE_MAX - 1)
> > +
> > +enum {
> > + IFLA_BRIDGE_MRP_RING_ROLE_UNSPEC,
> > + IFLA_BRIDGE_MRP_RING_ROLE_RING_ID,
> > + IFLA_BRIDGE_MRP_RING_ROLE_ROLE,
> > + __IFLA_BRIDGE_MRP_RING_ROLE_MAX,
> > +};
> > +
> > +#define IFLA_BRIDGE_MRP_RING_ROLE_MAX (__IFLA_BRIDGE_MRP_RING_ROLE_MAX - 1)
> > +
> > +enum {
> > + IFLA_BRIDGE_MRP_START_TEST_UNSPEC,
> > + IFLA_BRIDGE_MRP_START_TEST_RING_ID,
> > + IFLA_BRIDGE_MRP_START_TEST_INTERVAL,
> > + IFLA_BRIDGE_MRP_START_TEST_MAX_MISS,
> > + IFLA_BRIDGE_MRP_START_TEST_PERIOD,
> > + __IFLA_BRIDGE_MRP_START_TEST_MAX,
> > +};
> > +
> > +#define IFLA_BRIDGE_MRP_START_TEST_MAX (__IFLA_BRIDGE_MRP_START_TEST_MAX - 1)
> > +
> > struct br_mrp_instance {
> > __u32 ring_id;
> > __u32 p_ifindex;
> > __u32 s_ifindex;
> > };
> >
> > -struct br_mrp_port_role {
> > - __u32 ring_id;
> > - __u32 role;
> > -};
> > -
> > struct br_mrp_ring_state {
> > __u32 ring_id;
> > __u32 ring_state;
> > @@ -197,8 +249,6 @@ struct br_mrp_start_test {
> > __u32 period;
> > };
> >
> > -#define IFLA_BRIDGE_MRP_MAX (__IFLA_BRIDGE_MRP_MAX - 1)
> > -
> > struct bridge_stp_xstats {
> > __u64 transition_blk;
> > __u64 transition_fwd;
> > diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> > index 528d767eb026f..8ea59504ef47a 100644
> > --- a/net/bridge/br_mrp.c
> > +++ b/net/bridge/br_mrp.c
> > @@ -376,24 +376,24 @@ int br_mrp_set_port_state(struct net_bridge_port *p,
> > * note: already called with rtnl_lock
> > */
> > int br_mrp_set_port_role(struct net_bridge_port *p,
> > - struct br_mrp_port_role *role)
> > + enum br_mrp_port_role_type role)
> > {
> > struct br_mrp *mrp;
> >
> > if (!p || !(p->flags & BR_MRP_AWARE))
> > return -EINVAL;
> >
> > - mrp = br_mrp_find_id(p->br, role->ring_id);
> > + mrp = br_mrp_find_port(p->br, p);
> >
> > if (!mrp)
> > return -EINVAL;
> >
> > - if (role->role == BR_MRP_PORT_ROLE_PRIMARY)
> > + if (role == BR_MRP_PORT_ROLE_PRIMARY)
> > rcu_assign_pointer(mrp->p_port, p);
> > else
> > rcu_assign_pointer(mrp->s_port, p);
> >
> > - br_mrp_port_switchdev_set_role(p, role->role);
> > + br_mrp_port_switchdev_set_role(p, role);
> >
> > return 0;
> > }
> > diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
> > index 4a08a99519b04..cfad5d1cff050 100644
> > --- a/net/bridge/br_mrp_netlink.c
> > +++ b/net/bridge/br_mrp_netlink.c
> > @@ -8,19 +8,204 @@
> >
> > static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = {
> > [IFLA_BRIDGE_MRP_UNSPEC] = { .type = NLA_REJECT },
> > - [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_EXACT_LEN,
> > - .len = sizeof(struct br_mrp_instance)},
> > - [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_U32 },
> > - [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_EXACT_LEN,
> > - .len = sizeof(struct br_mrp_port_role)},
> > - [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_EXACT_LEN,
> > - .len = sizeof(struct br_mrp_ring_state)},
> > - [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_EXACT_LEN,
> > - .len = sizeof(struct br_mrp_ring_role)},
> > - [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_EXACT_LEN,
> > - .len = sizeof(struct br_mrp_start_test)},
> > + [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_NESTED },
> > + [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_NESTED },
> > + [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_NESTED },
> > + [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_NESTED },
> > + [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_NESTED },
> > + [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_NESTED },
> > };
> >
> > +static const struct nla_policy
> > +br_mrp_instance_policy[IFLA_BRIDGE_MRP_INSTANCE_MAX + 1] = {
> > + [IFLA_BRIDGE_MRP_INSTANCE_UNSPEC] = { .type = NLA_REJECT },
> > + [IFLA_BRIDGE_MRP_INSTANCE_RING_ID] = { .type = NLA_U32 },
> > + [IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX] = { .type = NLA_U32 },
> > + [IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX] = { .type = NLA_U32 },
> > +};
> > +
> > +int br_mrp_instance_parse(struct net_bridge *br, struct nlattr *attr,
> > + int cmd, struct netlink_ext_ack *extack)
> > +{
> > + struct nlattr *tb[IFLA_BRIDGE_MRP_INSTANCE_MAX + 1];
> > + struct br_mrp_instance inst;
> > + int err;
> > +
> > + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_INSTANCE_MAX, attr,
> > + br_mrp_instance_policy, extack);
> > + if (err)
> > + return err;
> > +
> > + if (!tb[IFLA_BRIDGE_MRP_INSTANCE_RING_ID] ||
> > + !tb[IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX] ||
> > + !tb[IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX])
> > + return -EINVAL;
> > +
> > + memset(&inst, 0, sizeof(inst));
> > +
> > + inst.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_INSTANCE_RING_ID]);
> > + inst.p_ifindex = nla_get_u32(tb[IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX]);
> > + inst.s_ifindex = nla_get_u32(tb[IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX]);
> > +
> > + if (cmd == RTM_SETLINK)
> > + return br_mrp_add(br, &inst);
> > + else
> > + return br_mrp_del(br, &inst);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct nla_policy
> > +br_mrp_port_state_policy[IFLA_BRIDGE_MRP_PORT_STATE_MAX + 1] = {
> > + [IFLA_BRIDGE_MRP_PORT_STATE_UNSPEC] = { .type = NLA_REJECT },
> > + [IFLA_BRIDGE_MRP_PORT_STATE_STATE] = { .type = NLA_U32 },
> > +};
> > +
> > +int br_mrp_port_state_parse(struct net_bridge_port *p, struct nlattr *attr,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct nlattr *tb[IFLA_BRIDGE_MRP_PORT_STATE_MAX + 1];
> > + enum br_mrp_port_state_type state;
> > + int err;
> > +
> > + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_PORT_STATE_MAX, attr,
> > + br_mrp_port_state_policy, extack);
> > + if (err)
> > + return err;
> > +
> > + if (!tb[IFLA_BRIDGE_MRP_PORT_STATE_STATE])
> > + return -EINVAL;
> > +
> > + state = nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE_STATE]);
> > +
> > + return br_mrp_set_port_state(p, state);
> > +}
> > +
> > +static const struct nla_policy
> > +br_mrp_port_role_policy[IFLA_BRIDGE_MRP_PORT_ROLE_MAX + 1] = {
> > + [IFLA_BRIDGE_MRP_PORT_ROLE_UNSPEC] = { .type = NLA_REJECT },
> > + [IFLA_BRIDGE_MRP_PORT_ROLE_ROLE] = { .type = NLA_U32 },
> > +};
> > +
> > +int br_mrp_port_role_parse(struct net_bridge_port *p, struct nlattr *attr,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct nlattr *tb[IFLA_BRIDGE_MRP_PORT_ROLE_MAX + 1];
> > + enum br_mrp_port_role_type role;
> > + int err;
> > +
> > + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_PORT_ROLE_MAX, attr,
> > + br_mrp_port_role_policy, extack);
> > + if (err)
> > + return err;
> > +
> > + if (!tb[IFLA_BRIDGE_MRP_PORT_ROLE_ROLE])
> > + return -EINVAL;
> > +
> > + role = nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_ROLE_ROLE]);
> > +
> > + return br_mrp_set_port_role(p, role);
> > +}
> > +
> > +static const struct nla_policy
> > +br_mrp_ring_state_policy[IFLA_BRIDGE_MRP_RING_STATE_MAX + 1] = {
> > + [IFLA_BRIDGE_MRP_RING_STATE_UNSPEC] = { .type = NLA_REJECT },
> > + [IFLA_BRIDGE_MRP_RING_STATE_RING_ID] = { .type = NLA_U32 },
> > + [IFLA_BRIDGE_MRP_RING_STATE_STATE] = { .type = NLA_U32 },
> > +};
> > +
> > +int br_mrp_ring_state_parse(struct net_bridge *br, struct nlattr *attr,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct nlattr *tb[IFLA_BRIDGE_MRP_RING_STATE_MAX + 1];
> > + struct br_mrp_ring_state state;
> > + int err;
> > +
> > + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_RING_STATE_MAX, attr,
> > + br_mrp_ring_state_policy, extack);
> > + if (err)
> > + return err;
> > +
> > + if (!tb[IFLA_BRIDGE_MRP_RING_STATE_RING_ID] ||
> > + !tb[IFLA_BRIDGE_MRP_RING_STATE_STATE])
> > + return -EINVAL;
> > +
> > + memset(&state, 0x0, sizeof(state));
> > +
> > + state.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_STATE_RING_ID]);
> > + state.ring_state = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_STATE_STATE]);
> > +
> > + return br_mrp_set_ring_state(br, &state);
> > +}
> > +
> > +static const struct nla_policy
> > +br_mrp_ring_role_policy[IFLA_BRIDGE_MRP_RING_ROLE_MAX + 1] = {
> > + [IFLA_BRIDGE_MRP_RING_ROLE_UNSPEC] = { .type = NLA_REJECT },
> > + [IFLA_BRIDGE_MRP_RING_ROLE_RING_ID] = { .type = NLA_U32 },
> > + [IFLA_BRIDGE_MRP_RING_ROLE_ROLE] = { .type = NLA_U32 },
> > +};
> > +
> > +int br_mrp_ring_role_parse(struct net_bridge *br, struct nlattr *attr,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct nlattr *tb[IFLA_BRIDGE_MRP_RING_ROLE_MAX + 1];
> > + struct br_mrp_ring_role role;
> > + int err;
> > +
> > + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_RING_ROLE_MAX, attr,
> > + br_mrp_ring_role_policy, extack);
> > + if (err)
> > + return err;
> > +
> > + if (!tb[IFLA_BRIDGE_MRP_RING_ROLE_RING_ID] ||
> > + !tb[IFLA_BRIDGE_MRP_RING_ROLE_ROLE])
> > + return -EINVAL;
> > +
> > + memset(&role, 0x0, sizeof(role));
> > +
> > + role.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_ROLE_RING_ID]);
> > + role.ring_role = nla_get_u32(tb[IFLA_BRIDGE_MRP_RING_ROLE_ROLE]);
> > +
> > + return br_mrp_set_ring_role(br, &role);
> > +}
> > +
> > +static const struct nla_policy
> > +br_mrp_start_test_policy[IFLA_BRIDGE_MRP_START_TEST_MAX + 1] = {
> > + [IFLA_BRIDGE_MRP_START_TEST_UNSPEC] = { .type = NLA_REJECT },
> > + [IFLA_BRIDGE_MRP_START_TEST_RING_ID] = { .type = NLA_U32 },
> > + [IFLA_BRIDGE_MRP_START_TEST_INTERVAL] = { .type = NLA_U32 },
> > + [IFLA_BRIDGE_MRP_START_TEST_MAX_MISS] = { .type = NLA_U32 },
> > + [IFLA_BRIDGE_MRP_START_TEST_PERIOD] = { .type = NLA_U32 },
> > +};
> > +
> > +int br_mrp_start_test_parse(struct net_bridge *br, struct nlattr *attr,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct nlattr *tb[IFLA_BRIDGE_MRP_START_TEST_MAX + 1];
> > + struct br_mrp_start_test test;
> > + int err;
> > +
> > + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_START_TEST_MAX, attr,
> > + br_mrp_start_test_policy, extack);
> > + if (err)
> > + return err;
> > +
> > + if (!tb[IFLA_BRIDGE_MRP_START_TEST_RING_ID] ||
> > + !tb[IFLA_BRIDGE_MRP_START_TEST_INTERVAL] ||
> > + !tb[IFLA_BRIDGE_MRP_START_TEST_MAX_MISS] ||
> > + !tb[IFLA_BRIDGE_MRP_START_TEST_PERIOD])
> > + return -EINVAL;
> > +
> > + memset(&test, 0x0, sizeof(test));
> > +
> > + test.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_RING_ID]);
> > + test.interval = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_INTERVAL]);
> > + test.max_miss = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_MAX_MISS]);
> > + test.period = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_TEST_PERIOD]);
> > +
> > + return br_mrp_start_test(br, &test);
> > +}
> > +
> > int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
> > struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
> > {
> > @@ -44,58 +229,45 @@ int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
> > return err;
> >
> > if (tb[IFLA_BRIDGE_MRP_INSTANCE]) {
> > - struct br_mrp_instance *instance =
> > - nla_data(tb[IFLA_BRIDGE_MRP_INSTANCE]);
> > -
> > - if (cmd == RTM_SETLINK)
> > - err = br_mrp_add(br, instance);
> > - else
> > - err = br_mrp_del(br, instance);
> > + err = br_mrp_instance_parse(br, tb[IFLA_BRIDGE_MRP_INSTANCE],
> > + cmd, extack);
> > if (err)
> > return err;
> > }
> >
> > if (tb[IFLA_BRIDGE_MRP_PORT_STATE]) {
> > - enum br_mrp_port_state_type state =
> > - nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE]);
> > -
> > - err = br_mrp_set_port_state(p, state);
> > + err = br_mrp_port_state_parse(p, tb[IFLA_BRIDGE_MRP_PORT_STATE],
> > + extack);
> > if (err)
> > return err;
> > }
> >
> > if (tb[IFLA_BRIDGE_MRP_PORT_ROLE]) {
> > - struct br_mrp_port_role *role =
> > - nla_data(tb[IFLA_BRIDGE_MRP_PORT_ROLE]);
> > -
> > - err = br_mrp_set_port_role(p, role);
> > + err = br_mrp_port_role_parse(p, tb[IFLA_BRIDGE_MRP_PORT_ROLE],
> > + extack);
> > if (err)
> > return err;
> > }
> >
> > if (tb[IFLA_BRIDGE_MRP_RING_STATE]) {
> > - struct br_mrp_ring_state *state =
> > - nla_data(tb[IFLA_BRIDGE_MRP_RING_STATE]);
> > -
> > - err = br_mrp_set_ring_state(br, state);
> > + err = br_mrp_ring_state_parse(br,
> > + tb[IFLA_BRIDGE_MRP_RING_STATE],
> > + extack);
> > if (err)
> > return err;
> > }
> >
> > if (tb[IFLA_BRIDGE_MRP_RING_ROLE]) {
> > - struct br_mrp_ring_role *role =
> > - nla_data(tb[IFLA_BRIDGE_MRP_RING_ROLE]);
> > -
> > - err = br_mrp_set_ring_role(br, role);
> > + err = br_mrp_ring_role_parse(br, tb[IFLA_BRIDGE_MRP_RING_ROLE],
> > + extack);
> > if (err)
> > return err;
> > }
> >
> > if (tb[IFLA_BRIDGE_MRP_START_TEST]) {
> > - struct br_mrp_start_test *test =
> > - nla_data(tb[IFLA_BRIDGE_MRP_START_TEST]);
> > -
> > - err = br_mrp_start_test(br, test);
> > + err = br_mrp_start_test_parse(br,
> > + tb[IFLA_BRIDGE_MRP_START_TEST],
> > + extack);
> > if (err)
> > return err;
> > }
> > diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
> > index 2921a4b59f8e7..a0f53cc3ab85c 100644
> > --- a/net/bridge/br_private_mrp.h
> > +++ b/net/bridge/br_private_mrp.h
> > @@ -37,7 +37,7 @@ int br_mrp_del(struct net_bridge *br, struct br_mrp_instance *instance);
> > int br_mrp_set_port_state(struct net_bridge_port *p,
> > enum br_mrp_port_state_type state);
> > int br_mrp_set_port_role(struct net_bridge_port *p,
> > - struct br_mrp_port_role *role);
> > + enum br_mrp_port_role_type role);
> > int br_mrp_set_ring_state(struct net_bridge *br,
> > struct br_mrp_ring_state *state);
> > int br_mrp_set_ring_role(struct net_bridge *br, struct br_mrp_ring_role *role);
> >
>
--
/Horatiu
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-27 10:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-26 14:22 [PATCH net-next] bridge: mrp: Rework the MRP netlink interface Horatiu Vultur
2020-05-27 8:59 ` Nikolay Aleksandrov
2020-05-27 12:06 ` Horatiu Vultur
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).