OK, thanks. There might already be a way to figure out when OVS restarts. Have you looked through the existing configuration schema to try to find one? On Apr 22, 2014 7:18 PM, "Ryan Wilson 76511" <wr...@vmware.com> wrote:
> I agree, this makes much more sense. We could add this restart tag to the > Open_vSwitch table instead, so the controller team could still do > event-based triggering on database changes. Also, this would clearly > separate the cases of a port being deleted and re-added (uuid changing in > Interface table) and OVS restarting (change in restart tag in Open_vSwitch). > > I'm new to OVS, so there are likely better ideas out there. Let me know if > you guys have a better approach to this problem. > > Ryan > > ------------------------------ > *From: *"Ben Pfaff" <b...@nicira.com> > *To: *"Ethan" <et...@nicira.com> > *Cc: *dev@openvswitch.org, "Ryan Wilson 76511" <wr...@vmware.com> > *Sent: *Tuesday, April 22, 2014 6:52:27 PM > *Subject: *Re: [ovs-dev] [PATCH] netdev: Add random tag to struct netdev. > > It would make a lot more sense to have a global indication whether OVS was > restarted. This is a property of OVS, not of a netdev. Otherwise we'll get > subsequent requests to add such a random tag to every single table in the > database. > > You can already tell if a port is deleted and readded by noticing the > change in UUID. > On Apr 22, 2014 6:14 PM, "Ethan Jackson" <et...@nicira.com> wrote: > >> > 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<https://urldefense.proofpoint.com/v1/url?u=http://ofproto-macros.at&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=414d2eefdb07ca1f2066c36e33a7e6b09070f00a30d3d4cc3aced9c5e59a485b>b/tests/ >> ofproto-macros.at<https://urldefense.proofpoint.com/v1/url?u=http://ofproto-macros.at&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=414d2eefdb07ca1f2066c36e33a7e6b09070f00a30d3d4cc3aced9c5e59a485b> >> >> index a82a9b1..ad2f342 100644 >> >> --- >> >> a/tests/ofproto-macros.at<https://urldefense.proofpoint.com/v1/url?u=http://ofproto-macros.at&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=414d2eefdb07ca1f2066c36e33a7e6b09070f00a30d3d4cc3aced9c5e59a485b> >> >> +++ >> >> b/tests/ofproto-macros.at<https://urldefense.proofpoint.com/v1/url?u=http://ofproto-macros.at&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=414d2eefdb07ca1f2066c36e33a7e6b09070f00a30d3d4cc3aced9c5e59a485b> >> >> @@ -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<https://urldefense.proofpoint.com/v1/url?u=http://uuidfilt.pl&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=69493947b5984f6f80f3460b854f0bc8e089750e83e0142faf77da1105eca3c0>])], >> [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<https://urldefense.proofpoint.com/v1/url?u=http://ovs-vsctl.at&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=cdb4082265a4daea2deb33bcd848a004e2eca8323709f0a890e7d3989501c6c5>b/tests/ >> ovs-vsctl.at<https://urldefense.proofpoint.com/v1/url?u=http://ovs-vsctl.at&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=cdb4082265a4daea2deb33bcd848a004e2eca8323709f0a890e7d3989501c6c5> >> >> index 440bf1a..38d07dc 100644 >> >> --- >> >> a/tests/ovs-vsctl.at<https://urldefense.proofpoint.com/v1/url?u=http://ovs-vsctl.at&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=cdb4082265a4daea2deb33bcd848a004e2eca8323709f0a890e7d3989501c6c5> >> >> +++ >> >> b/tests/ovs-vsctl.at<https://urldefense.proofpoint.com/v1/url?u=http://ovs-vsctl.at&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=cdb4082265a4daea2deb33bcd848a004e2eca8323709f0a890e7d3989501c6c5> >> >> @@ -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<https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=7b81305c05395d74875bd5fd35c2b73cf998c44423b94f9cfe2e0f8795cc33ef> >> >> >> > >> > _______________________________________________ >> > dev mailing list >> > dev@openvswitch.org >> > http://openvswitch.org/mailman/listinfo/dev<https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=TfBS78Vw3dzttvXidhbffg%3D%3D%0A&m=aM%2BfJlod3UDyqmr%2FKgRWYmoZrfD0Z6VnVaLQ%2F%2FFzvmY%3D%0A&s=7b81305c05395d74875bd5fd35c2b73cf998c44423b94f9cfe2e0f8795cc33ef> >> > >> > >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev