Hey Joe,
On Wed, Apr 9, 2014 at 6:16 PM, Joe Stringer <j...@wand.net.nz> wrote:
> It had occurred to me at the time, that this was a possible middle ground
> between making netdev change_seq completely global or retaining this
> per-netdev seq as well as having a global seq. When I looked at perf when
> many ports are configured, a majority of the cost seemed to be just in just
> iterating through the ports to check the seq.
>
> Do you have a sense of the impact this has?
>
>
I haven't tested about it. And I'd like to measure it.
I only concerned and tested the benefit of updating to database.
Let me update the thread after testing the configuration perf and latency.
> On 5 April 2014 11:00, Alex Wang <al...@nicira.com> wrote:
>
>> This commit can be seen as a partial revert of commit
>> da4a619179d (netdev: Globally track port status changes)
>> by adding the 'change_seq' to 'struct netdev'.
>>
>> Signed-off-by: Alex Wang <al...@nicira.com>
>> ---
>> lib/netdev-bsd.c | 5 +++++
>> lib/netdev-dpdk.c | 1 +
>> lib/netdev-dummy.c | 2 ++
>> lib/netdev-linux.c | 1 +
>> lib/netdev-provider.h | 17 +++++++++++++++++
>> lib/netdev-vport.c | 3 +++
>> lib/netdev.c | 8 ++++++++
>> lib/netdev.h | 1 +
>> 8 files changed, 38 insertions(+)
>>
>> diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
>> index 9952aef..affd08a 100644
>> --- a/lib/netdev-bsd.c
>> +++ b/lib/netdev-bsd.c
>> @@ -217,6 +217,7 @@ netdev_bsd_cache_cb(const struct rtbsd_change *change,
>> if (is_netdev_bsd_class(netdev_class)) {
>> dev = netdev_bsd_cast(base_dev);
>> dev->cache_valid = 0;
>> + netdev_change_seq_changed(base_dev);
>> seq_change(connectivity_seq_get());
>> }
>> netdev_close(base_dev);
>> @@ -236,6 +237,7 @@ netdev_bsd_cache_cb(const struct rtbsd_change *change,
>> dev = netdev_bsd_cast(netdev);
>> dev->cache_valid = 0;
>> seq_change(connectivity_seq_get());
>> + netdev_change_seq_changed(netdev);
>> netdev_close(netdev);
>> }
>> shash_destroy(&device_shash);
>> @@ -774,6 +776,7 @@ netdev_bsd_set_etheraddr(struct netdev *netdev_,
>> if (!error) {
>> netdev->cache_valid |= VALID_ETHERADDR;
>> memcpy(netdev->etheraddr, mac, ETH_ADDR_LEN);
>> + netdev_change_seq_changed(netdev_);
>> seq_change(connectivity_seq_get());
>> }
>> }
>> @@ -1228,6 +1231,7 @@ netdev_bsd_set_in4(struct netdev *netdev_, struct
>> in_addr addr,
>> netdev->netmask = mask;
>> }
>> }
>> + netdev_change_seq_changed(netdev_);
>> seq_change(connectivity_seq_get());
>> }
>> ovs_mutex_unlock(&netdev->mutex);
>> @@ -1526,6 +1530,7 @@ netdev_bsd_update_flags(struct netdev *netdev_,
>> enum netdev_flags off,
>> new_flags = (old_flags & ~nd_to_iff_flags(off)) |
>> nd_to_iff_flags(on);
>> if (new_flags != old_flags) {
>> error = set_flags(netdev_get_kernel_name(netdev_),
>> new_flags);
>> + netdev_change_seq_changed(netdev_);
>> seq_change(connectivity_seq_get());
>> }
>> }
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 4b36f52..1ab57cc 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -318,6 +318,7 @@ check_link_status(struct netdev_dpdk *dev)
>> rte_eth_link_get_nowait(dev->port_id, &link);
>>
>> if (dev->link.link_status != link.link_status) {
>> + netdev_change_seq_changed(&dev->up);
>> seq_change(connectivity_seq_get());
>>
>> dev->link_reset_cnt++;
>> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
>> index 995e923..359fc06 100644
>> --- a/lib/netdev-dummy.c
>> +++ b/lib/netdev-dummy.c
>> @@ -873,6 +873,7 @@ netdev_dummy_set_etheraddr(struct netdev *netdev,
>> ovs_mutex_lock(&dev->mutex);
>> if (!eth_addr_equals(dev->hwaddr, mac)) {
>> memcpy(dev->hwaddr, mac, ETH_ADDR_LEN);
>> + netdev_change_seq_changed(netdev);
>> seq_change(connectivity_seq_get());
>> }
>> ovs_mutex_unlock(&dev->mutex);
>> @@ -968,6 +969,7 @@ netdev_dummy_update_flags__(struct netdev_dummy
>> *netdev,
>> netdev->flags |= on;
>> netdev->flags &= ~off;
>> if (*old_flagsp != netdev->flags) {
>> + netdev_change_seq_changed(&netdev->up);
>> seq_change(connectivity_seq_get());
>> }
>>
>> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
>> index f9634b6..2f61936 100644
>> --- a/lib/netdev-linux.c
>> +++ b/lib/netdev-linux.c
>> @@ -616,6 +616,7 @@ netdev_linux_changed(struct netdev_linux *dev,
>> unsigned int ifi_flags, unsigned int mask)
>> OVS_REQUIRES(dev->mutex)
>> {
>> + netdev_change_seq_changed(&dev->up);
>> seq_change(connectivity_seq_get());
>>
>> if ((dev->ifi_flags ^ ifi_flags) & IFF_RUNNING) {
>> diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
>> index f233c0c..84b62fb 100644
>> --- a/lib/netdev-provider.h
>> +++ b/lib/netdev-provider.h
>> @@ -38,6 +38,14 @@ struct netdev {
>> const struct netdev_class *netdev_class; /* Functions to control
>> this device. */
>>
>> + /* A sequence number which indicates changes in one of 'netdev''s
>> + * properties. It must be nonzero so that users have a value which
>> + * they may use as a reset when tracking 'netdev'.
>> + *
>> + * Minimally, the sequence number is required to change whenever
>> + * 'netdev''s flags, features, ethernet address, or carrier changes.
>> */
>> + uint64_t change_seq;
>> +
>> /* The following are protected by 'netdev_mutex' (internal to
>> netdev.c). */
>> int n_rxq;
>> int ref_cnt; /* Times this devices was
>> opened. */
>> @@ -45,6 +53,15 @@ struct netdev {
>> struct list saved_flags_list; /* Contains "struct
>> netdev_saved_flags". */
>> };
>>
>> +static inline void
>> +netdev_change_seq_changed(struct netdev *netdev)
>> +{
>> + netdev->change_seq++;
>> + if (!netdev->change_seq) {
>> + netdev->change_seq++;
>> + }
>> +}
>> +
>> const char *netdev_get_type(const struct netdev *);
>> const struct netdev_class *netdev_get_class(const struct netdev *);
>> const char *netdev_get_name(const struct netdev *);
>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>> index f52cceb..3697e27 100644
>> --- a/lib/netdev-vport.c
>> +++ b/lib/netdev-vport.c
>> @@ -212,6 +212,7 @@ netdev_vport_set_etheraddr(struct netdev *netdev_,
>> ovs_mutex_lock(&netdev->mutex);
>> memcpy(netdev->etheraddr, mac, ETH_ADDR_LEN);
>> ovs_mutex_unlock(&netdev->mutex);
>> + netdev_change_seq_changed(netdev_);
>> seq_change(connectivity_seq_get());
>>
>> return 0;
>> @@ -486,6 +487,7 @@ set_tunnel_config(struct netdev *dev_, const struct
>> smap *args)
>>
>> ovs_mutex_lock(&dev->mutex);
>> dev->tnl_cfg = tnl_cfg;
>> + netdev_change_seq_changed(dev_);
>> seq_change(connectivity_seq_get());
>> ovs_mutex_unlock(&dev->mutex);
>>
>> @@ -660,6 +662,7 @@ set_patch_config(struct netdev *dev_, const struct
>> smap *args)
>> ovs_mutex_lock(&dev->mutex);
>> free(dev->peer);
>> dev->peer = xstrdup(peer);
>> + netdev_change_seq_changed(dev_);
>> seq_change(connectivity_seq_get());
>> ovs_mutex_unlock(&dev->mutex);
>>
>> diff --git a/lib/netdev.c b/lib/netdev.c
>> index 4ec1d7d..9f76052 100644
>> --- a/lib/netdev.c
>> +++ b/lib/netdev.c
>> @@ -340,6 +340,7 @@ netdev_open(const char *name, const char *type,
>> struct netdev **netdevp)
>> memset(netdev, 0, sizeof *netdev);
>> netdev->netdev_class = rc->class;
>> netdev->name = xstrdup(name);
>> + netdev->change_seq = 1;
>> netdev->node = shash_add(&netdev_shash, name, netdev);
>>
>> /* By default enable one rx queue per netdev. */
>> @@ -355,6 +356,7 @@ netdev_open(const char *name, const char *type,
>> struct netdev **netdevp)
>> int old_ref_cnt;
>>
>> atomic_add(&rc->ref_cnt, 1, &old_ref_cnt);
>> + netdev_change_seq_changed(netdev);
>> seq_change(connectivity_seq_get());
>> } else {
>> free(netdev->name);
>> @@ -1649,3 +1651,9 @@ restore_all_flags(void *aux OVS_UNUSED)
>> }
>> }
>> }
>> +
>> +uint64_t
>> +netdev_get_change_seq(const struct netdev *netdev)
>> +{
>> + return netdev->change_seq;
>> +}
>> diff --git a/lib/netdev.h b/lib/netdev.h
>> index 432f327..7cb3c80 100644
>> --- a/lib/netdev.h
>> +++ b/lib/netdev.h
>> @@ -284,6 +284,7 @@ int netdev_set_queue(struct netdev *,
>> int netdev_delete_queue(struct netdev *, unsigned int queue_id);
>> int netdev_get_queue_stats(const struct netdev *, unsigned int queue_id,
>> struct netdev_queue_stats *);
>> +uint64_t netdev_get_change_seq(const struct netdev *);
>>
>> struct netdev_queue_dump {
>> struct netdev *netdev;
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
