diff mbox series

[v5,net-next,10/16] net: openvswitch: propagate errors from dev_get_stats

Message ID 20210108163159.358043-11-olteanv@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Make .ndo_get_stats64 sleepable | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count fail Series longer than 15 patches
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 1 maintainers not CCed: dev@openvswitch.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 3 this patch: 3
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() WARNING: function definition argument 'struct ovs_vport_stats *' should also have an identifier name WARNING: function definition argument 'struct vport *' should also have an identifier name
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Vladimir Oltean Jan. 8, 2021, 4:31 p.m. UTC 
From: Vladimir Oltean <vladimir.oltean@nxp.com>

The dev_get_stats function can now return an error code, so the code
that retrieves vport statistics and sends them through netlink needs to
propagate that error code.

Modify the drastic BUG_ON checks to operate only on the -EMSGSIZE error
code (the only error code previously possible), and not crash the kernel
in case dev_get_stats fails. This is in line with what rtnetlink.c does.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v5:
Still keeping the BUG_ON condition except for the output of
ovs_vport_get_stats.

Changes in v4:
Patch is new (Eric's suggestion).

 net/openvswitch/datapath.c | 25 +++++++++++++++++++------
 net/openvswitch/vport.c    | 10 ++++++++--
 net/openvswitch/vport.h    |  2 +-
 3 files changed, 28 insertions(+), 9 deletions(-)
diff mbox series

Patch

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 9d6ef6cb9b26..160b8dc453da 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1987,7 +1987,10 @@  static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
 			goto nla_put_failure;
 	}
 
-	ovs_vport_get_stats(vport, &vport_stats);
+	err = ovs_vport_get_stats(vport, &vport_stats);
+	if (err)
+		goto error;
+
 	if (nla_put_64bit(skb, OVS_VPORT_ATTR_STATS,
 			  sizeof(struct ovs_vport_stats), &vport_stats,
 			  OVS_VPORT_ATTR_PAD))
@@ -2028,7 +2031,9 @@  struct sk_buff *ovs_vport_cmd_build_info(struct vport *vport, struct net *net,
 
 	retval = ovs_vport_cmd_fill_info(vport, skb, net, portid, seq, 0, cmd,
 					 GFP_KERNEL);
-	BUG_ON(retval < 0);
+	BUG_ON(retval == -EMSGSIZE);
+	if (retval)
+		return ERR_PTR(retval);
 
 	return skb;
 }
@@ -2173,6 +2178,9 @@  static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
 	err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
 				      info->snd_portid, info->snd_seq, 0,
 				      OVS_VPORT_CMD_NEW, GFP_KERNEL);
+	BUG_ON(err == -EMSGSIZE);
+	if (err)
+		goto exit_unlock_free;
 
 	new_headroom = netdev_get_fwd_headroom(vport->dev);
 
@@ -2181,7 +2189,6 @@  static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
 	else
 		netdev_set_rx_headroom(vport->dev, dp->max_headroom);
 
-	BUG_ON(err < 0);
 	ovs_unlock();
 
 	ovs_notify(&dp_vport_genl_family, reply, info);
@@ -2234,7 +2241,9 @@  static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
 	err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
 				      info->snd_portid, info->snd_seq, 0,
 				      OVS_VPORT_CMD_SET, GFP_KERNEL);
-	BUG_ON(err < 0);
+	BUG_ON(err == -EMSGSIZE);
+	if (err)
+		goto exit_unlock_free;
 
 	ovs_unlock();
 	ovs_notify(&dp_vport_genl_family, reply, info);
@@ -2274,7 +2283,9 @@  static int ovs_vport_cmd_del(struct sk_buff *skb, struct genl_info *info)
 	err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
 				      info->snd_portid, info->snd_seq, 0,
 				      OVS_VPORT_CMD_DEL, GFP_KERNEL);
-	BUG_ON(err < 0);
+	BUG_ON(err == -EMSGSIZE);
+	if (err)
+		goto exit_unlock_free;
 
 	/* the vport deletion may trigger dp headroom update */
 	dp = vport->dp;
@@ -2321,7 +2332,9 @@  static int ovs_vport_cmd_get(struct sk_buff *skb, struct genl_info *info)
 	err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
 				      info->snd_portid, info->snd_seq, 0,
 				      OVS_VPORT_CMD_GET, GFP_ATOMIC);
-	BUG_ON(err < 0);
+	BUG_ON(err == -EMSGSIZE);
+	if (err)
+		goto exit_unlock_free;
 	rcu_read_unlock();
 
 	return genlmsg_reply(reply, info);
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index 215a818bf9ce..e66c949fd97a 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -267,11 +267,15 @@  void ovs_vport_del(struct vport *vport)
  *
  * Must be called with ovs_mutex or rcu_read_lock.
  */
-void ovs_vport_get_stats(struct vport *vport, struct ovs_vport_stats *stats)
+int ovs_vport_get_stats(struct vport *vport, struct ovs_vport_stats *stats)
 {
 	struct rtnl_link_stats64 dev_stats;
+	int err;
+
+	err = dev_get_stats(vport->dev, &dev_stats);
+	if (err)
+		return err;
 
-	dev_get_stats(vport->dev, &dev_stats);
 	stats->rx_errors  = dev_stats.rx_errors;
 	stats->tx_errors  = dev_stats.tx_errors;
 	stats->tx_dropped = dev_stats.tx_dropped;
@@ -281,6 +285,8 @@  void ovs_vport_get_stats(struct vport *vport, struct ovs_vport_stats *stats)
 	stats->rx_packets = dev_stats.rx_packets;
 	stats->tx_bytes	  = dev_stats.tx_bytes;
 	stats->tx_packets = dev_stats.tx_packets;
+
+	return 0;
 }
 
 /**
diff --git a/net/openvswitch/vport.h b/net/openvswitch/vport.h
index 1eb7495ac5b4..8927ba5c491b 100644
--- a/net/openvswitch/vport.h
+++ b/net/openvswitch/vport.h
@@ -30,7 +30,7 @@  void ovs_vport_del(struct vport *);
 
 struct vport *ovs_vport_locate(const struct net *net, const char *name);
 
-void ovs_vport_get_stats(struct vport *, struct ovs_vport_stats *);
+int ovs_vport_get_stats(struct vport *, struct ovs_vport_stats *);
 
 int ovs_vport_set_options(struct vport *, struct nlattr *options);
 int ovs_vport_get_options(const struct vport *, struct sk_buff *);