> I don't understand why, if you need to know when vswitchd restarts (why do > you?), it should be tied to netdevs.
So if you're monitoring a netdev for packet stats, you need to know if vswitchd restarted so that you can take account of the fact the stats were reset. I suppose it needs to be tied to netdevs to take account of the case when a tunnel is removed and re-added for whatever reason. Does that make sense? If so, an explanation of the sort should be added to the commit message. Ethan > > On Apr 22, 2014 4:09 PM, "Ryan Wilson 76511" <wr...@vmware.com> wrote: >> >> netdev: Add random tag to struct netdev. >> >> >> Before, there would be no way to tell if ovs-vswitchd had >> been restarted or killed via ovsdb logging. Now, a random >> tag will be generated upon initialization of the netdev >> struct which happens when ovs-vswitchd is restarted. A >> change in tag value in ovsdb's Interface table likely >> indicates that ovs-vswitchd has been restarted or killed. >> >> Signed-off-by: Ryan Wilson <wr...@vmware.com> >> >> --- >> >> diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h >> >> index 9d8e67f..ea2e59c 100644 >> --- a/lib/netdev-provider.h >> +++ b/lib/netdev-provider.h >> @@ -40,6 +40,11 @@ struct netdev { >> const struct netdev_class *netdev_class; /* Functions to control >> this device. */ >> >> + /* A random number generated upon initialization of the netdev struct. >> + * If this number changes, the netdev struct has been re-initialized, >> + * likely indicating ovs-vswitchd has restarted. */ >> + uint32_t tag; >> + >> /* 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'. >> diff --git a/lib/netdev.c b/lib/netdev.c >> index 4736a97..70cb3e8 100644 >> --- a/lib/netdev.c >> +++ b/lib/netdev.c >> @@ -340,6 +340,7 @@ netdev_open(const char *name, const char *type, struct >> netdev **netdevp) >> netdev->name = xstrdup(name); >> netdev->change_seq = 1; >> netdev->node = shash_add(&netdev_shash, name, netdev); >> + netdev->tag = random_uint32(); >> >> /* By default enable one rx queue per netdev. */ >> if (netdev->netdev_class->rxq_alloc) { >> @@ -664,6 +665,13 @@ netdev_set_etheraddr(struct netdev *netdev, const >> uint8_t mac[ETH_ADDR_LEN]) >> return netdev->netdev_class->set_etheraddr(netdev, mac); >> } >> >> +/* Retrieves 'netdev''s random tag generated upon initialization. */ >> +int >> +netdev_get_tag(const struct netdev *netdev) >> +{ >> + return (int) netdev->tag; >> +} >> + >> /* Retrieves 'netdev''s MAC address. If successful, returns 0 and copies >> the >> * the MAC address into 'mac'. On failure, returns a positive errno value >> and >> * clears 'mac' to all-zeros. */ >> diff --git a/lib/netdev.h b/lib/netdev.h >> index 7cb3c80..169d595 100644 >> --- a/lib/netdev.h >> +++ b/lib/netdev.h >> @@ -158,6 +158,7 @@ const char *netdev_get_type_from_name(const char *); >> int netdev_get_mtu(const struct netdev *, int *mtup); >> int netdev_set_mtu(const struct netdev *, int mtu); >> int netdev_get_ifindex(const struct netdev *); >> +int netdev_get_tag(const struct netdev *netdev); >> >> /* Packet reception. */ >> int netdev_rxq_open(struct netdev *, struct netdev_rxq **, int id); >> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at >> index a82a9b1..ad2f342 100644 >> --- a/tests/ofproto-macros.at >> +++ b/tests/ofproto-macros.at >> @@ -38,6 +38,31 @@ m4_define([STRIP_DURATION], [[sed >> 's/\bduration=[0-9.]*s/duration=?s/']]) >> m4_define([STRIP_USED], [[sed 's/used:[0-9]\.[0-9]*/used:0.0/']]) >> m4_define([TESTABLE_LOG], [-vPATTERN:ANY:'%c|%p|%m']) >> >> +# OVS_VSWITCHD_CHECK_STDERR >> +# >> +# Checks ovs-vswitchd stderr output on startup for consistent messages. >> +m4_define([OVS_VSWITCHD_CHECK_STDERR], >> + [AT_CHECK([[sed < stderr ' >> +/vlog|INFO|opened log file/d >> +/vswitchd|INFO|ovs-vswitchd (Open vSwitch)/d >> +/reconnect|INFO|/d >> +/ofproto_dpif|INFO|/d >> +/ofproto_dpif|DBG|need revalidate in ofproto_wait_cb()/d >> +/bridge|INFO|/d >> +/connmgr|INFO|/d >> +/ofproto|INFO|using datapath ID/d >> +/ofproto|INFO|datapath ID changed to fedcba9876543210/d']])]) >> + >> +# OVS_VSWITCHD_SETENV >> +# >> +# Sets environment variables for ovs-server and ovs-vswitchd processes. >> +m4_define([OVS_VSWITCHD_SETENV], >> + [OVS_RUNDIR=`pwd`; export OVS_RUNDIR >> + OVS_LOGDIR=`pwd`; export OVS_LOGDIR >> + OVS_DBDIR=`pwd`; export OVS_DBDIR >> + OVS_SYSCONFDIR=`pwd`; export OVS_SYSCONFDIR >> + ON_EXIT([kill `cat ovsdb-server.pid ovs-vswitchd.pid`])]) >> + >> # OVS_VSWITCHD_START([vsctl-args], [vsctl-output], [=override]) >> # >> # Creates a database and starts ovsdb-server, starts ovs-vswitchd >> @@ -52,11 +77,7 @@ m4_define([TESTABLE_LOG], [-vPATTERN:ANY:'%c|%p|%m']) >> # won't work at all (which makes sense because tests should not access a >> # system's real Ethernet devices). >> m4_define([OVS_VSWITCHD_START], >> - [OVS_RUNDIR=`pwd`; export OVS_RUNDIR >> - OVS_LOGDIR=`pwd`; export OVS_LOGDIR >> - OVS_DBDIR=`pwd`; export OVS_DBDIR >> - OVS_SYSCONFDIR=`pwd`; export OVS_SYSCONFDIR >> - ON_EXIT([kill `cat ovsdb-server.pid ovs-vswitchd.pid`]) >> + [OVS_VSWITCHD_SETENV >> >> dnl Create database. >> touch .conf.db.~lock~ >> @@ -75,12 +96,7 @@ m4_define([OVS_VSWITCHD_START], >> dnl Start ovs-vswitchd. >> AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --enable-dummy$3 >> --disable-system --log-file -vvconn -vofproto_dpif], [0], [], [stderr]) >> AT_CAPTURE_FILE([ovs-vswitchd.log]) >> - AT_CHECK([[sed < stderr ' >> -/vlog|INFO|opened log file/d >> -/vswitchd|INFO|ovs-vswitchd (Open vSwitch)/d >> -/reconnect|INFO|/d >> -/ofproto|INFO|using datapath ID/d >> -/ofproto|INFO|datapath ID changed to fedcba9876543210/d']]) >> + OVS_VSWITCHD_CHECK_STDERR >> >> dnl Add bridges, ports, etc. >> AT_CHECK([ovs-vsctl -- add-br br0 -- set bridge br0 datapath-type=dummy >> other-config:datapath-id=fedcba9876543210 >> other-config:hwaddr=aa:55:aa:55:00:00 >> protocols=[[OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13]] fail-mode=secure >> -- $1 m4_if([$2], [], [], [| ${PERL} $srcdir/uuidfilt.pl])], [0], [$2]) >> @@ -112,6 +128,23 @@ m4_define([OVS_VSWITCHD_STOP], >> AT_CHECK([ovs-appctl -t ovs-vswitchd exit]) >> AT_CHECK([ovs-appctl -t ovsdb-server exit])]) >> >> +# RESTART_OVS_VSWITCHD([=override]) >> +# >> +# Restarts ovs-vswitchd. >> +# >> +# If a test needs to use "system" devices (as dummies), then specify >> +# =override (literally) as the first argument. Otherwise, system devices >> +# won't work at all (which makes sense because tests should not access a >> +# system's real Ethernet devices). >> +m4_define([RESTART_OVS_VSWITCHD], >> + [OVS_VSWITCHD_SETENV >> + AT_CHECK([ovs-appctl -t ovs-vswitchd exit]) >> + >> + dnl Restart ovs-vswitchd. >> + AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --enable-dummy$2 >> --disable-system --log-file -vvconn -vofproto_dpif], [0], [], [stderr]) >> + AT_CAPTURE_FILE([ovs-vswitchd.log]) >> + OVS_VSWITCHD_CHECK_STDERR]) >> + >> # ADD_OF_PORTS(BRIDGE, OF_PORT[, OF_PORT...]) >> # >> # Creates a dummy interface with an OpenFlow port number of OF_PORT and >> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at >> index 440bf1a..38d07dc 100644 >> --- a/tests/ovs-vsctl.at >> +++ b/tests/ovs-vsctl.at >> @@ -1192,6 +1192,15 @@ AT_CHECK([RUN_OVS_VSCTL( >> OVS_VSCTL_CLEANUP >> AT_CLEANUP >> >> +AT_SETUP([check interface tag change]) >> +OVS_VSWITCHD_START >> +AT_CHECK([ovs-vsctl --columns=tag list Interface > tag1], [0], [], []) >> +RESTART_OVS_VSWITCHD >> +AT_CHECK([ovs-vsctl --columns=tag list Interface > tag2], [0], [], []) >> +AT_CHECK([diff tag1 tag2], [1], [stdout], []) >> +OVS_VSWITCHD_STOP >> +AT_CLEANUP >> + >> dnl ---------------------------------------------------------------------- >> AT_BANNER([ovs-vsctl add-port -- reserved port names]) >> >> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c >> index f46d002..ebbced8 100644 >> --- a/vswitchd/bridge.c >> +++ b/vswitchd/bridge.c >> @@ -379,6 +379,7 @@ bridge_init(const char *remote) >> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_cfm_remote_opstate); >> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_bfd_status); >> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_lacp_current); >> + ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_tag); >> ovsdb_idl_omit(idl, &ovsrec_interface_col_external_ids); >> >> ovsdb_idl_omit_alert(idl, &ovsrec_controller_col_is_connected); >> @@ -1774,6 +1775,7 @@ iface_refresh_status(struct iface *iface) >> int64_t mtu_64; >> uint8_t mac[ETH_ADDR_LEN]; >> int64_t ifindex64; >> + int64_t tag64; >> int error; >> >> if (iface_is_synthetic(iface)) { >> @@ -1827,6 +1829,9 @@ iface_refresh_status(struct iface *iface) >> ifindex64 = 0; >> } >> ovsrec_interface_set_ifindex(iface->cfg, &ifindex64, 1); >> + >> + tag64 = netdev_get_tag(iface->netdev); >> + ovsrec_interface_set_tag(iface->cfg, tag64); >> } >> >> /* Writes 'iface''s CFM statistics to the database. 'iface' must not be >> @@ -3576,6 +3581,7 @@ iface_clear_db_record(const struct ovsrec_interface >> *if_cfg) >> ovsrec_interface_set_lacp_current(if_cfg, NULL, 0); >> ovsrec_interface_set_statistics(if_cfg, NULL, NULL, 0); >> ovsrec_interface_set_ifindex(if_cfg, NULL, 0); >> + ovsrec_interface_set_tag(if_cfg, 0); >> } >> } >> >> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema >> index 3fb45d1..cedf521 100644 >> --- a/vswitchd/vswitch.ovsschema >> +++ b/vswitchd/vswitch.ovsschema >> @@ -1,6 +1,6 @@ >> {"name": "Open_vSwitch", >> "version": "7.5.0", >> - "cksum": "1448369194 20560", >> + "cksum": "1929109758 20605", >> "tables": { >> "Open_vSwitch": { >> "columns": { >> @@ -172,6 +172,8 @@ >> "name": { >> "type": "string", >> "mutable": false}, >> + "tag": { >> + "type": "integer"}, >> "type": { >> "type": "string"}, >> "options": { >> >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev >> > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev