Looks good. A few minor comments in line.
Acked-by: Andy Zhou <az...@nicira.com>
It would be good to add some more back ground information in the commit message.
On Wed, Sep 17, 2014 at 4:08 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
> This was required for old compatibility code. Now vswitchd
> has dropped it. This support was always deprecated, so
> finally removing it.
>
> Signed-off-by: Pravin B Shelar <pshe...@nicira.com>
> ---
> datapath/datapath.c | 8 --------
> datapath/vport.c | 46 ++++++++++++++--------------------------------
> datapath/vport.h | 6 +-----
> 3 files changed, 15 insertions(+), 45 deletions(-)
>
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index ed9d7bd..3e5ff72 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -1831,10 +1831,6 @@ static int ovs_vport_cmd_new(struct sk_buff *skb,
> struct genl_info *info)
> if (IS_ERR(vport))
> goto exit_unlock_free;
>
> - err = 0;
> - if (a[OVS_VPORT_ATTR_STATS])
> - ovs_vport_set_stats(vport, nla_data(a[OVS_VPORT_ATTR_STATS]));
> -
> err = ovs_vport_cmd_fill_info(vport, reply, info->snd_portid,
> info->snd_seq, 0, OVS_VPORT_CMD_NEW);
> BUG_ON(err < 0);
> @@ -1878,10 +1874,6 @@ static int ovs_vport_cmd_set(struct sk_buff *skb,
> struct genl_info *info)
> goto exit_unlock_free;
> }
>
> - if (a[OVS_VPORT_ATTR_STATS])
> - ovs_vport_set_stats(vport, nla_data(a[OVS_VPORT_ATTR_STATS]));
> -
> -
> if (a[OVS_VPORT_ATTR_UPCALL_PID]) {
> err = ovs_vport_set_upcall_portids(vport,
>
> a[OVS_VPORT_ATTR_UPCALL_PID]);
> diff --git a/datapath/vport.c b/datapath/vport.c
> index cf7f917..f7e66d1 100644
> --- a/datapath/vport.c
> +++ b/datapath/vport.c
> @@ -243,26 +243,6 @@ void ovs_vport_del(struct vport *vport)
> }
>
> /**
> - * ovs_vport_set_stats - sets offset device stats
> - *
> - * @vport: vport on which to set stats
> - * @stats: stats to set
> - *
> - * Provides a set of transmit, receive, and error stats to be added as an
> - * offset to the collected data when stats are retrieved. Some devices may
> not
> - * support setting the stats, in which case the result will always be
> - * -EOPNOTSUPP.
> - *
> - * Must be called with ovs_mutex.
> - */
> -void ovs_vport_set_stats(struct vport *vport, struct ovs_vport_stats *stats)
> -{
> - spin_lock_bh(&vport->stats_lock);
> - vport->offset_stats = *stats;
> - spin_unlock_bh(&vport->stats_lock);
> -}
> -
> -/**
> * ovs_vport_get_stats - retrieve device stats
> *
> * @vport: vport from which to retrieve the stats
> @@ -276,25 +256,27 @@ void ovs_vport_get_stats(struct vport *vport, struct
> ovs_vport_stats *stats)
> {
> int i;
>
> - /* We potentially have 3 sources of stats that need to be
> + stats->rx_bytes = 0;
> + stats->rx_packets = 0;
> + stats->tx_bytes = 0;
> + stats->tx_packets = 0;
It may be better to move this block below the spin_unlock_bh, so that clear and
set are continuous.
> +
> + /* We potentially have two surces of stats that need to be
> * combined: those we have collected (split into err_stats and
> - * percpu_stats), offset_stats from set_stats(), and device
> - * error stats from netdev->get_stats() (for errors that happen
> - * downstream and therefore aren't reported through our
> - * vport_record_error() function).
> - * Stats from first two sources are merged and reported by ovs over
> + * percpu_stats), and device error stats from netdev->get_stats()
> + * (for errors that happen downstream and therefore aren't
> + * reported through our vport_record_error() function).
> + * Stats from first source are reported by ovs over
> * OVS_VPORT_ATTR_STATS.
> * netdev-stats can be directly read over netlink-ioctl.
> */
>
> spin_lock_bh(&vport->stats_lock);
>
> - *stats = vport->offset_stats;
> -
> - stats->rx_errors += vport->err_stats.rx_errors;
> - stats->tx_errors += vport->err_stats.tx_errors;
> - stats->tx_dropped += vport->err_stats.tx_dropped;
> - stats->rx_dropped += vport->err_stats.rx_dropped;
> + stats->rx_errors = vport->err_stats.rx_errors;
> + stats->tx_errors = vport->err_stats.tx_errors;
> + stats->tx_dropped = vport->err_stats.tx_dropped;
> + stats->rx_dropped = vport->err_stats.rx_dropped;
>
> spin_unlock_bh(&vport->stats_lock);
>
> diff --git a/datapath/vport.h b/datapath/vport.h
> index 8c3da05..07a9d5d 100644
> --- a/datapath/vport.h
> +++ b/datapath/vport.h
> @@ -45,7 +45,6 @@ void ovs_vport_del(struct vport *);
>
> struct vport *ovs_vport_locate(struct net *net, const char *name);
>
> -void ovs_vport_set_stats(struct vport *, struct ovs_vport_stats *);
> void ovs_vport_get_stats(struct vport *, struct ovs_vport_stats *);
>
> int ovs_vport_set_options(struct vport *, struct nlattr *options);
> @@ -101,10 +100,8 @@ struct vport_portids {
> * @dp_hash_node: Element in @datapath->ports hash table in datapath.c.
> * @ops: Class structure.
> * @percpu_stats: Points to per-CPU statistics used and maintained by vport
> - * @stats_lock: Protects @err_stats and @offset_stats.
> + * @stats_lock: Protects @err_stats.
> * @err_stats: Points to error statistics used and maintained by vport
> - * @offset_stats: Added to actual statistics as a sop to compatibility with
> - * XAPI for Citrix XenServer. Deprecated.
> */
> struct vport {
> struct rcu_head rcu;
> @@ -120,7 +117,6 @@ struct vport {
>
> spinlock_t stats_lock;
> struct vport_err_stats err_stats;
> - struct ovs_vport_stats offset_stats;
> };
>
> /**
> --
> 1.9.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
