On Fri, Sep 19, 2014 at 2:13 PM, Andy Zhou <az...@nicira.com> wrote:
> Looks good. A few minor comments in line.
> Acked-by: Andy Zhou <az...@nicira.com>
>
I fixed according to comments and pushed to master.
> 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
