Re: [PATCH net-next 1/8] bridge: mcast: Centralize netlink attribute parsing
From: Nikolay Aleksandrov <razor@blackwall.org>
Date: 2022-12-05 11:34:15
Also in:
bridge
On 05/12/2022 09:42, Ido Schimmel wrote:
quoted hunk ↗ jump to hunk
Netlink attributes are currently passed deep in the MDB creation call chain, making it difficult to add new attributes. In addition, some validity checks are performed under the multicast lock although they can be performed before it is ever acquired. As a first step towards solving these issues, parse the RTM_{NEW,DEL}MDB messages into a configuration structure, relieving other functions from the need to handle raw netlink attributes. Subsequent patches will convert the MDB code to use this configuration structure. This is consistent with how other rtnetlink objects are handled, such as routes and nexthops. Signed-off-by: Ido Schimmel <idosch@nvidia.com> --- net/bridge/br_mdb.c | 120 ++++++++++++++++++++++++++++++++++++++++ net/bridge/br_private.h | 7 +++ 2 files changed, 127 insertions(+)diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c index 321be94c445a..c53050e47a0f 100644 --- a/net/bridge/br_mdb.c +++ b/net/bridge/br_mdb.c@@ -974,6 +974,116 @@ static int __br_mdb_add(struct net *net, struct net_bridge *br, return ret; } +static int br_mdb_config_attrs_init(struct nlattr *set_attrs, + struct br_mdb_config *cfg, + struct netlink_ext_ack *extack) +{ + struct nlattr *mdb_attrs[MDBE_ATTR_MAX + 1]; + int err; + + err = nla_parse_nested(mdb_attrs, MDBE_ATTR_MAX, set_attrs, + br_mdbe_attrs_pol, extack); + if (err) + return err; + + if (mdb_attrs[MDBE_ATTR_SOURCE] && + !is_valid_mdb_source(mdb_attrs[MDBE_ATTR_SOURCE], + cfg->entry->addr.proto, extack)) + return -EINVAL; + + __mdb_entry_to_br_ip(cfg->entry, &cfg->group, mdb_attrs); + + return 0; +} + +static int br_mdb_config_init(struct net *net, struct sk_buff *skb, + struct nlmsghdr *nlh, struct br_mdb_config *cfg, + struct netlink_ext_ack *extack) +{
I just noticed the skb argument is unused. Does it get used in a future change? Also a minor nit - I think nlh can be a const, nlmsg_parse_deprecated already uses a const nlh.