There are couple of reasons to remove this support:
* This is used in very old OVS use-case. It is much better
to read stats directly from OVS.
* Forthcoming commit will remove support for setting stats
for vport. The stats update depends on stats-set.
Signed-off-by: Pravin B Shelar <[email protected]>
---
NEWS | 1 +
lib/netdev-bsd.c | 1 -
lib/netdev-dpdk.c | 32 ++++++++--------------------
lib/netdev-dummy.c | 13 ------------
lib/netdev-linux.c | 41 +----------------------------------
lib/netdev-provider.h | 8 -------
lib/netdev-vport.c | 1 -
lib/netdev.c | 13 ------------
lib/netdev.h | 1 -
ofproto/bond.c | 59 ---------------------------------------------------
ofproto/bond.h | 2 --
vswitchd/bridge.c | 2 --
12 files changed, 11 insertions(+), 163 deletions(-)
diff --git a/NEWS b/NEWS
index 2746594..6cbb315 100644
--- a/NEWS
+++ b/NEWS
@@ -31,6 +31,7 @@ Post-v2.3.0
- Experimental support for the Rapid Spanning Tree Protocol
(IEEE 802.1D-2004). More conformance and interoperability testing is
still needed, so this should not be enabled on production environments.
+ - Stats are no longer updated on fake bond interface.
v2.3.0 - 14 Aug 2014
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 32c04a3..5b01299 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -1576,7 +1576,6 @@ netdev_bsd_update_flags(struct netdev *netdev_, enum
netdev_flags off,
NULL, /* get_carrier_resets */ \
NULL, /* set_miimon_interval */ \
netdev_bsd_get_stats, \
- NULL, /* set_stats */ \
\
GET_FEATURES, \
NULL, /* set_advertisement */ \
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 4f9c5c2..8f1fdb5 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -190,7 +190,6 @@ struct netdev_dpdk {
int mtu;
int socket_id;
int buf_size;
- struct netdev_stats stats_offset;
struct netdev_stats stats;
uint8_t hwaddr[ETH_ADDR_LEN];
@@ -950,29 +949,17 @@ netdev_dpdk_get_stats(const struct netdev *netdev, struct
netdev_stats *stats)
ovs_mutex_lock(&dev->mutex);
rte_eth_stats_get(dev->port_id, &rte_stats);
- *stats = dev->stats_offset;
+ memset(stats, 0, sizeof(*stats));
- stats->rx_packets += rte_stats.ipackets;
- stats->tx_packets += rte_stats.opackets;
- stats->rx_bytes += rte_stats.ibytes;
- stats->tx_bytes += rte_stats.obytes;
- stats->rx_errors += rte_stats.ierrors;
- stats->tx_errors += rte_stats.oerrors;
- stats->multicast += rte_stats.imcasts;
+ stats->rx_packets = rte_stats.ipackets;
+ stats->tx_packets = rte_stats.opackets;
+ stats->rx_bytes = rte_stats.ibytes;
+ stats->tx_bytes = rte_stats.obytes;
+ stats->rx_errors = rte_stats.ierrors;
+ stats->tx_errors = rte_stats.oerrors;
+ stats->multicast = rte_stats.imcasts;
- stats->tx_dropped += dev->stats.tx_dropped;
- ovs_mutex_unlock(&dev->mutex);
-
- return 0;
-}
-
-static int
-netdev_dpdk_set_stats(struct netdev *netdev, const struct netdev_stats *stats)
-{
- struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
-
- ovs_mutex_lock(&dev->mutex);
- dev->stats_offset = *stats;
+ stats->tx_dropped = dev->stats.tx_dropped;
ovs_mutex_unlock(&dev->mutex);
return 0;
@@ -1367,7 +1354,6 @@ unlock_dpdk:
netdev_dpdk_get_carrier_resets, \
netdev_dpdk_set_miimon, \
netdev_dpdk_get_stats, \
- netdev_dpdk_set_stats, \
netdev_dpdk_get_features, \
NULL, /* set_advertisements */ \
\
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 1334a67..ca04812 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -970,18 +970,6 @@ netdev_dummy_get_stats(const struct netdev *netdev, struct
netdev_stats *stats)
}
static int
-netdev_dummy_set_stats(struct netdev *netdev, const struct netdev_stats *stats)
-{
- struct netdev_dummy *dev = netdev_dummy_cast(netdev);
-
- ovs_mutex_lock(&dev->mutex);
- dev->stats = *stats;
- ovs_mutex_unlock(&dev->mutex);
-
- return 0;
-}
-
-static int
netdev_dummy_get_ifindex(const struct netdev *netdev)
{
struct netdev_dummy *dev = netdev_dummy_cast(netdev);
@@ -1058,7 +1046,6 @@ static const struct netdev_class dummy_class = {
NULL, /* get_carrier_resets */
NULL, /* get_miimon */
netdev_dummy_get_stats,
- netdev_dummy_set_stats,
NULL, /* get_features */
NULL, /* set_advertisements */
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index e311122..084af4e 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1691,41 +1691,6 @@ netdev_internal_get_stats(const struct netdev *netdev_,
return error;
}
-static int
-netdev_internal_set_stats(struct netdev *netdev,
- const struct netdev_stats *stats)
-{
- struct ovs_vport_stats vport_stats;
- struct dpif_linux_vport vport;
- int err;
-
- put_32aligned_u64(&vport_stats.rx_packets, stats->rx_packets);
- put_32aligned_u64(&vport_stats.tx_packets, stats->tx_packets);
- put_32aligned_u64(&vport_stats.rx_bytes, stats->rx_bytes);
- put_32aligned_u64(&vport_stats.tx_bytes, stats->tx_bytes);
- put_32aligned_u64(&vport_stats.rx_errors, stats->rx_errors);
- put_32aligned_u64(&vport_stats.tx_errors, stats->tx_errors);
- put_32aligned_u64(&vport_stats.rx_dropped, stats->rx_dropped);
- put_32aligned_u64(&vport_stats.tx_dropped, stats->tx_dropped);
-
- dpif_linux_vport_init(&vport);
- vport.cmd = OVS_VPORT_CMD_SET;
- vport.name = netdev_get_name(netdev);
- vport.stats = &vport_stats;
-
- err = dpif_linux_vport_transact(&vport, NULL, NULL);
-
- /* If the vport layer doesn't know about the device, that doesn't mean it
- * doesn't exist (after all were able to open it when netdev_open() was
- * called), it just means that it isn't attached and we'll be getting
- * stats a different way. */
- if (err == ENODEV) {
- err = EOPNOTSUPP;
- }
-
- return err;
-}
-
static void
netdev_linux_read_features(struct netdev_linux *netdev)
{
@@ -2734,7 +2699,7 @@ netdev_linux_update_flags(struct netdev *netdev_, enum
netdev_flags off,
return error;
}
-#define NETDEV_LINUX_CLASS(NAME, CONSTRUCT, GET_STATS, SET_STATS, \
+#define NETDEV_LINUX_CLASS(NAME, CONSTRUCT, GET_STATS, \
GET_FEATURES, GET_STATUS) \
{ \
NAME, \
@@ -2764,7 +2729,6 @@ netdev_linux_update_flags(struct netdev *netdev_, enum
netdev_flags off,
netdev_linux_get_carrier_resets, \
netdev_linux_set_miimon_interval, \
GET_STATS, \
- SET_STATS, \
\
GET_FEATURES, \
netdev_linux_set_advertisements, \
@@ -2807,7 +2771,6 @@ const struct netdev_class netdev_linux_class =
"system",
netdev_linux_construct,
netdev_linux_get_stats,
- NULL, /* set_stats */
netdev_linux_get_features,
netdev_linux_get_status);
@@ -2816,7 +2779,6 @@ const struct netdev_class netdev_tap_class =
"tap",
netdev_linux_construct_tap,
netdev_tap_get_stats,
- NULL, /* set_stats */
netdev_linux_get_features,
netdev_linux_get_status);
@@ -2825,7 +2787,6 @@ const struct netdev_class netdev_internal_class =
"internal",
netdev_linux_construct,
netdev_internal_get_stats,
- netdev_internal_set_stats,
NULL, /* get_features */
netdev_internal_get_status);
diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index c08ef35..7f266fd 100644
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -371,14 +371,6 @@ struct netdev_class {
* (UINT64_MAX). */
int (*get_stats)(const struct netdev *netdev, struct netdev_stats *);
- /* Sets the device stats for 'netdev' to 'stats'.
- *
- * Most network devices won't support this feature and will set this
- * function pointer to NULL, which is equivalent to returning EOPNOTSUPP.
- *
- * Some network devices might only allow setting their stats to 0. */
- int (*set_stats)(struct netdev *netdev, const struct netdev_stats *);
-
/* Stores the features supported by 'netdev' into each of '*current',
* '*advertised', '*supported', and '*peer'. Each value is a bitmap of
* NETDEV_F_* bits.
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 6ab90bf..c258f6b 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -789,7 +789,6 @@ get_stats(const struct netdev *netdev, struct netdev_stats
*stats)
NULL, /* get_carrier_resets */ \
NULL, /* get_miimon */ \
get_stats, \
- NULL, /* set_stats */ \
\
NULL, /* get_features */ \
NULL, /* set_advertisements */ \
diff --git a/lib/netdev.c b/lib/netdev.c
index 0d065e7..fb17626 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -1239,19 +1239,6 @@ netdev_get_stats(const struct netdev *netdev, struct
netdev_stats *stats)
return error;
}
-/* Attempts to change the stats for 'netdev' to those provided in 'stats'.
- * Returns 0 if successful, otherwise a positive errno value.
- *
- * This will probably fail for most network devices. Some devices might only
- * allow setting their stats to 0. */
-int
-netdev_set_stats(struct netdev *netdev, const struct netdev_stats *stats)
-{
- return (netdev->netdev_class->set_stats
- ? netdev->netdev_class->set_stats(netdev, stats)
- : EOPNOTSUPP);
-}
-
/* Attempts to set input rate limiting (policing) policy, such that up to
* 'kbits_rate' kbps of traffic is accepted, with a maximum accumulative burst
* size of 'kbits' kb. */
diff --git a/lib/netdev.h b/lib/netdev.h
index c6249c4..1f91d9a 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -251,7 +251,6 @@ struct netdev *netdev_find_dev_by_in4(const struct in_addr
*);
/* Statistics. */
int netdev_get_stats(const struct netdev *, struct netdev_stats *);
-int netdev_set_stats(struct netdev *, const struct netdev_stats *);
/* Quality of service. */
struct netdev_qos_capabilities {
diff --git a/ofproto/bond.c b/ofproto/bond.c
index 2d04b43..cb6b895 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -132,7 +132,6 @@ struct bond {
struct hmap pr_rule_ops; /* Helps to maintain post recirculation
rules.*/
/* Legacy compatibility. */
- long long int next_fake_iface_update; /* LLONG_MAX if disabled. */
bool lacp_fallback_ab; /* Fallback to active-backup on LACP failure. */
struct ovs_refcount ref_cnt;
@@ -177,8 +176,6 @@ static struct bond_slave *choose_output_slave(const struct
bond *,
struct flow_wildcards *,
uint16_t vlan)
OVS_REQ_RDLOCK(rwlock);
-static void bond_update_fake_slave_stats(struct bond *)
- OVS_REQ_RDLOCK(rwlock);
/* Attempts to parse 's' as the name of a bond balancing mode. If successful,
* stores the mode in '*balance' and returns true. Otherwise returns false
@@ -228,7 +225,6 @@ bond_create(const struct bond_settings *s, struct
ofproto_dpif *ofproto)
hmap_init(&bond->slaves);
list_init(&bond->enabled_slaves);
ovs_mutex_init(&bond->mutex);
- bond->next_fake_iface_update = LLONG_MAX;
ovs_refcount_init(&bond->ref_cnt);
bond->recirc_id = 0;
@@ -432,14 +428,6 @@ bond_reconfigure(struct bond *bond, const struct
bond_settings *s)
revalidate = true;
}
- if (s->fake_iface) {
- if (bond->next_fake_iface_update == LLONG_MAX) {
- bond->next_fake_iface_update = time_msec();
- }
- } else {
- bond->next_fake_iface_update = LLONG_MAX;
- }
-
if (bond->bond_revalidate) {
revalidate = true;
bond->bond_revalidate = false;
@@ -611,12 +599,6 @@ bond_run(struct bond *bond, enum lacp_status lacp_status)
bond_choose_active_slave(bond);
}
- /* Update fake bond interface stats. */
- if (time_msec() >= bond->next_fake_iface_update) {
- bond_update_fake_slave_stats(bond);
- bond->next_fake_iface_update = time_msec() + 1000;
- }
-
revalidate = bond->bond_revalidate;
bond->bond_revalidate = false;
ovs_rwlock_unlock(&rwlock);
@@ -639,10 +621,6 @@ bond_wait(struct bond *bond)
seq_wait(connectivity_seq_get(), slave->change_seq);
}
- if (bond->next_fake_iface_update != LLONG_MAX) {
- poll_timer_wait_until(bond->next_fake_iface_update);
- }
-
if (bond->bond_revalidate) {
poll_immediate_wake();
}
@@ -1813,40 +1791,3 @@ bond_choose_active_slave(struct bond *bond)
VLOG_INFO_RL(&rl, "bond %s: all interfaces disabled", bond->name);
}
}
-
-/* Attempts to make the sum of the bond slaves' statistics appear on the fake
- * bond interface. */
-static void
-bond_update_fake_slave_stats(struct bond *bond)
-{
- struct netdev_stats bond_stats;
- struct bond_slave *slave;
- struct netdev *bond_dev;
-
- memset(&bond_stats, 0, sizeof bond_stats);
-
- HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) {
- struct netdev_stats slave_stats;
-
- if (!netdev_get_stats(slave->netdev, &slave_stats)) {
- /* XXX: We swap the stats here because they are swapped back when
- * reported by the internal device. The reason for this is
- * internal devices normally represent packets going into the
- * system but when used as fake bond device they represent packets
- * leaving the system. We really should do this in the internal
- * device itself because changing it here reverses the counts from
- * the perspective of the switch. However, the internal device
- * doesn't know what type of device it represents so we have to do
- * it here for now. */
- bond_stats.tx_packets += slave_stats.rx_packets;
- bond_stats.tx_bytes += slave_stats.rx_bytes;
- bond_stats.rx_packets += slave_stats.tx_packets;
- bond_stats.rx_bytes += slave_stats.tx_bytes;
- }
- }
-
- if (!netdev_open(bond->name, "system", &bond_dev)) {
- netdev_set_stats(bond_dev, &bond_stats);
- netdev_close(bond_dev);
- }
-}
diff --git a/ofproto/bond.h b/ofproto/bond.h
index 0d9a67a..c487ec2 100644
--- a/ofproto/bond.h
+++ b/ofproto/bond.h
@@ -52,8 +52,6 @@ struct bond_settings {
int up_delay; /* ms before enabling an up slave. */
int down_delay; /* ms before disabling a down slave. */
- /* Legacy compatibility. */
- bool fake_iface; /* Update fake stats for netdev 'name'? */
bool lacp_fallback_ab_cfg; /* Fallback to active-backup on LACP failure.
*/
};
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 8f99d7d..1060719 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -3782,8 +3782,6 @@ port_configure_bond(struct port *port, struct
bond_settings *s)
s->rebalance_interval = 1000;
}
- s->fake_iface = port->cfg->bond_fake_iface;
-
s->lacp_fallback_ab_cfg = smap_get_bool(&port->cfg->other_config,
"lacp-fallback-ab", false);
--
1.9.3
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
