lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 8 Sep 2020 13:54:49 +0000
From:   Nikolay Aleksandrov <nikolay@...dia.com>
To:     "bridge@...ts.linux-foundation.org" 
        <bridge@...ts.linux-foundation.org>,
        "henrik.bjoernlund@...rochip.com" <henrik.bjoernlund@...rochip.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jiri@...lanox.com" <jiri@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Roopa Prabhu <roopa@...dia.com>,
        "idosch@...lanox.com" <idosch@...lanox.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>
CC:     "horatiu.vultur@...rochip.com" <horatiu.vultur@...rochip.com>
Subject: Re: [PATCH RFC 6/7] bridge: cfm: Netlink Notifications.

On Fri, 2020-09-04 at 09:15 +0000, Henrik Bjoernlund wrote:
> This is the implementation of Netlink notifications out of CFM.
> 
> Notifications are initiated whenever a state change happens in CFM.
> 
[snip]
> @@ -445,6 +458,7 @@ static int br_cfm_frame_rx(struct net_bridge_port *port, struct sk_buff *skb)
>  			peer_mep->cc_status.ccm_defect = false;
>  
>  			/* Change in CCM defect status - notify */
> +			br_cfm_notify(RTM_NEWLINK, port);
>  
>  			/* Start CCM RX timer */
>  			ccm_rx_timer_start(peer_mep);
> @@ -874,6 +888,35 @@ int br_cfm_cc_counters_clear(struct net_bridge *br, const u32 instance,
>  	return 0;
>  }
>  
> +int br_cfm_mep_count(struct net_bridge *br, u32 *count)
> +{
> +	struct br_cfm_mep *mep;

Leave a blank line between local variable definitions and code.

> +	*count = 0;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(mep, &br->mep_list, head)
> +		* count += 1;

please remove the extra space

> +	rcu_read_unlock();
> +
> +	return 0;
> +}
> +
> +int br_cfm_peer_mep_count(struct net_bridge *br, u32 *count)
> +{
> +	struct br_cfm_peer_mep *peer_mep;
> +	struct br_cfm_mep *mep;

Leave a blank line between local variable definitions and code.

> +	*count = 0;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(mep, &br->mep_list, head) {
> +		list_for_each_entry_rcu(peer_mep, &mep->peer_mep_list, head)
> +			* count += 1;

please remove the extra space

> +	}
> +	rcu_read_unlock();
> +
> +	return 0;
> +}
> +
>  bool br_cfm_created(struct net_bridge *br)
>  {
>  	return !list_empty(&br->mep_list);
> diff --git a/net/bridge/br_cfm_netlink.c b/net/bridge/br_cfm_netlink.c
> index 4e39aab1cd0b..13664ac8608a 100644
> --- a/net/bridge/br_cfm_netlink.c
> +++ b/net/bridge/br_cfm_netlink.c
> @@ -582,7 +582,9 @@ int br_cfm_config_fill_info(struct sk_buff *skb, struct net_bridge *br)
>  	return -EMSGSIZE;
>  }
>  
> -int br_cfm_status_fill_info(struct sk_buff *skb, struct net_bridge *br)
> +int br_cfm_status_fill_info(struct sk_buff *skb,
> +			    struct net_bridge *br,
> +			    bool getlink)
>  {
>  	struct nlattr *tb, *cfm_tb;
>  	struct br_cfm_mep *mep;
> @@ -613,10 +615,12 @@ int br_cfm_status_fill_info(struct sk_buff *skb, struct net_bridge *br)
>  				mep->status.rx_level_low_seen))
>  			goto nla_put_failure;
>  
> -		/* Clear all 'seen' indications */
> -		mep->status.opcode_unexp_seen = false;
> -		mep->status.version_unexp_seen = false;
> -		mep->status.rx_level_low_seen = false;
> +		if (getlink) { /* Only clear if this is a GETLINK */
> +			/* Clear all 'seen' indications */
> +			mep->status.opcode_unexp_seen = false;
> +			mep->status.version_unexp_seen = false;
> +			mep->status.rx_level_low_seen = false;
> +		}
>  
>  		nla_nest_end(skb, tb);
>  
> @@ -662,10 +666,12 @@ int br_cfm_status_fill_info(struct sk_buff *skb, struct net_bridge *br)
>  					peer_mep->cc_status.seq_unexp_seen))
>  				goto nla_put_failure;
>  
> -			/* Clear all 'seen' indications */
> -			peer_mep->cc_status.seen = false;
> -			peer_mep->cc_status.tlv_seen = false;
> -			peer_mep->cc_status.seq_unexp_seen = false;
> +			if (getlink) { /* Only clear if this is a GETLINK */
> +				/* Clear all 'seen' indications */
> +				peer_mep->cc_status.seen = false;
> +				peer_mep->cc_status.tlv_seen = false;
> +				peer_mep->cc_status.seq_unexp_seen = false;

Why clear these on GETLINK? This sounds like it should be a set op.

> +			}
>  
>  			nla_nest_end(skb, tb);
>  		}
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 6de5cb1295f6..f2e885521f4f 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -94,9 +94,11 @@ static size_t br_get_link_af_size_filtered(const struct net_device *dev,
>  {
>  	struct net_bridge_vlan_group *vg = NULL;
>  	struct net_bridge_port *p = NULL;
> -	struct net_bridge *br;
> -	int num_vlan_infos;
> +	struct net_bridge *br = NULL;
> +	u32 num_cfm_peer_mep_infos;
> +	u32 num_cfm_mep_infos;
>  	size_t vinfo_sz = 0;
> +	int num_vlan_infos;
>  
>  	rcu_read_lock();
>  	if (netif_is_bridge_port(dev)) {
> @@ -115,6 +117,52 @@ static size_t br_get_link_af_size_filtered(const struct net_device *dev,
>  	/* Each VLAN is returned in bridge_vlan_info along with flags */
>  	vinfo_sz += num_vlan_infos * nla_total_size(sizeof(struct bridge_vlan_info));
>  
> +	if (!(filter_mask & RTEXT_FILTER_CFM_STATUS))
> +		return vinfo_sz;
> +
> +	if (!br)
> +		return vinfo_sz;
> +
> +	/* CFM status info must be added */
> +	if (br_cfm_mep_count(br, &num_cfm_mep_infos))
> +		return vinfo_sz;
> +
> +	if (br_cfm_peer_mep_count(br, &num_cfm_peer_mep_infos))
> +		return vinfo_sz;
> +

Can these return non-0 at all?

> +	vinfo_sz += nla_total_size(0);	/* IFLA_BRIDGE_CFM */
> +	/* For each status struct the MEP instance (u32) is added */
> +	/* MEP instance (u32) + br_cfm_mep_status */
> +	vinfo_sz += num_cfm_mep_infos *
> +		     /*IFLA_BRIDGE_CFM_MEP_STATUS_INSTANCE */
> +		    (nla_total_size(sizeof(u32))
> +		     /* IFLA_BRIDGE_CFM_MEP_STATUS_OPCODE_UNEXP_SEEN */
> +		     + nla_total_size(sizeof(u32))
> +		     /* IFLA_BRIDGE_CFM_MEP_STATUS_VERSION_UNEXP_SEEN */
> +		     + nla_total_size(sizeof(u32))
> +		     /* IFLA_BRIDGE_CFM_MEP_STATUS_RX_LEVEL_LOW_SEEN */
> +		     + nla_total_size(sizeof(u32)));
> +	/* MEP instance (u32) + br_cfm_cc_peer_status */
> +	vinfo_sz += num_cfm_peer_mep_infos *
> +		     /* IFLA_BRIDGE_CFM_CC_PEER_STATUS_INSTANCE */
> +		    (nla_total_size(sizeof(u32))
> +		     /* IFLA_BRIDGE_CFM_CC_PEER_STATUS_PEER_MEPID */
> +		     + nla_total_size(sizeof(u32))
> +		     /* IFLA_BRIDGE_CFM_CC_PEER_STATUS_CCM_DEFECT */
> +		     + nla_total_size(sizeof(u32))
> +		     /* IFLA_BRIDGE_CFM_CC_PEER_STATUS_RDI */
> +		     + nla_total_size(sizeof(u32))
> +		     /* IFLA_BRIDGE_CFM_CC_PEER_STATUS_PORT_TLV_VALUE */
> +		     + nla_total_size(sizeof(u8))
> +		     /* IFLA_BRIDGE_CFM_CC_PEER_STATUS_IF_TLV_VALUE */
> +		     + nla_total_size(sizeof(u8))
> +		     /* IFLA_BRIDGE_CFM_CC_PEER_STATUS_SEEN */
> +		     + nla_total_size(sizeof(u32))
> +		     /* IFLA_BRIDGE_CFM_CC_PEER_STATUS_TLV_SEEN */
> +		     + nla_total_size(sizeof(u32))
> +		     /* IFLA_BRIDGE_CFM_CC_PEER_STATUS_SEQ_UNEXP_SEEN */
> +		     + nla_total_size(sizeof(u32)));
> +
>  	return vinfo_sz;
>  }
>  
> @@ -378,7 +426,8 @@ static int br_fill_ifvlaninfo(struct sk_buff *skb,
>  static int br_fill_ifinfo(struct sk_buff *skb,
>  			  const struct net_bridge_port *port,
>  			  u32 pid, u32 seq, int event, unsigned int flags,
> -			  u32 filter_mask, const struct net_device *dev)
> +			  u32 filter_mask, const struct net_device *dev,
> +			  bool getlink)
>  {
>  	u8 operstate = netif_running(dev) ? dev->operstate : IF_OPER_DOWN;
>  	struct net_bridge *br;
> @@ -515,7 +564,7 @@ static int br_fill_ifinfo(struct sk_buff *skb,
>  			goto nla_put_failure;
>  
>  		rcu_read_lock();
> -		err = br_cfm_status_fill_info(skb, br);
> +		err = br_cfm_status_fill_info(skb, br, getlink);
>  		rcu_read_unlock();
>  
>  		if (err)
> @@ -533,11 +582,9 @@ static int br_fill_ifinfo(struct sk_buff *skb,
>  	return -EMSGSIZE;
>  }
>  
> -/* Notify listeners of a change in bridge or port information */
> -void br_ifinfo_notify(int event, const struct net_bridge *br,
> -		      const struct net_bridge_port *port)
> +void br_info_notify(int event, const struct net_bridge *br,
> +		    const struct net_bridge_port *port, u32 filter)
>  {
> -	u32 filter = RTEXT_FILTER_BRVLAN_COMPRESSED;
>  	struct net_device *dev;
>  	struct sk_buff *skb;
>  	int err = -ENOBUFS;
> @@ -562,7 +609,7 @@ void br_ifinfo_notify(int event, const struct net_bridge *br,
>  	if (skb == NULL)
>  		goto errout;
>  
> -	err = br_fill_ifinfo(skb, port, 0, 0, event, 0, filter, dev);
> +	err = br_fill_ifinfo(skb, port, 0, 0, event, 0, filter, dev, false);
>  	if (err < 0) {
>  		/* -EMSGSIZE implies BUG in br_nlmsg_size() */
>  		WARN_ON(err == -EMSGSIZE);
> @@ -575,6 +622,15 @@ void br_ifinfo_notify(int event, const struct net_bridge *br,
>  	rtnl_set_sk_err(net, RTNLGRP_LINK, err);
>  }
>  
> +/* Notify listeners of a change in bridge or port information */
> +void br_ifinfo_notify(int event, const struct net_bridge *br,
> +		      const struct net_bridge_port *port)
> +{
> +	u32 filter = RTEXT_FILTER_BRVLAN_COMPRESSED;
> +
> +	return br_info_notify(event, br, port, filter);
> +}
> +
>  /*
>   * Dump information about all ports, in response to GETLINK
>   */
> @@ -591,7 +647,7 @@ int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
>  		return 0;
>  
>  	return br_fill_ifinfo(skb, port, pid, seq, RTM_NEWLINK, nlflags,
> -			      filter_mask, dev);
> +			      filter_mask, dev, true);
>  }
>  
>  static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index fe36592f7525..53bcbdd21f34 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -1370,7 +1370,11 @@ int br_cfm_parse(struct net_bridge *br, struct net_bridge_port *p,
>  int br_cfm_rx_frame_process(struct net_bridge_port *p, struct sk_buff *skb);
>  bool br_cfm_created(struct net_bridge *br);
>  int br_cfm_config_fill_info(struct sk_buff *skb, struct net_bridge *br);
> -int br_cfm_status_fill_info(struct sk_buff *skb, struct net_bridge *br);
> +int br_cfm_status_fill_info(struct sk_buff *skb,
> +			    struct net_bridge *br,
> +			    bool getlink);
> +int br_cfm_mep_count(struct net_bridge *br, u32 *count);
> +int br_cfm_peer_mep_count(struct net_bridge *br, u32 *count);
>  #else
>  static inline int br_cfm_parse(struct net_bridge *br, struct net_bridge_port *p,
>  			       struct nlattr *attr, int cmd,
> @@ -1394,7 +1398,19 @@ static inline int br_cfm_config_fill_info(struct sk_buff *skb, struct net_bridge
>  	return -EOPNOTSUPP;
>  }
>  
> -static inline int br_cfm_status_fill_info(struct sk_buff *skb, struct net_bridge *br)
> +static inline int br_cfm_status_fill_info(struct sk_buff *skb,
> +					  struct net_bridge *br,
> +					  bool getlink)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int br_cfm_mep_count(struct net_bridge *br, u32 *count)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int br_cfm_peer_mep_count(struct net_bridge *br, u32 *count)
>  {
>  	return -EOPNOTSUPP;
>  }
> @@ -1406,6 +1422,8 @@ int br_netlink_init(void);
>  void br_netlink_fini(void);
>  void br_ifinfo_notify(int event, const struct net_bridge *br,
>  		      const struct net_bridge_port *port);
> +void br_info_notify(int event, const struct net_bridge *br,
> +		    const struct net_bridge_port *port, u32 filter);
>  int br_setlink(struct net_device *dev, struct nlmsghdr *nlmsg, u16 flags,
>  	       struct netlink_ext_ack *extack);
>  int br_dellink(struct net_device *dev, struct nlmsghdr *nlmsg, u16 flags);

Powered by blists - more mailing lists