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?
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
