I'm happy with that. I'll finish my review soon.
On Tue, Nov 20, 2012 at 04:03:47PM -0800, Gurucharan Shetty wrote: > On Tue, Nov 20, 2012 at 1:29 PM, Justin Pettit <[email protected]> wrote: > > Can you make sure you document this behavior in "vswitchd/vswitch.xml". > > I forgot to do it. This patch probably will need another respin and I > will include it. > In the meanwhile, do you think the following change is good enough. > > Here is an incremental with just the vswitch.xml changes. > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index c9d0dc4..c2786a5 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -1164,6 +1164,9 @@ > port number for the OpenFlow ``local port''). If the interface > cannot be added then Open vSwitch sets this column > to -1.</p> > + <p>When <ref column="ofport_request"/> is not set, Open vSwitch picks > + an appropriate value for this column and then tries to keep the value > + constant across restarts.</p> > </column> > > > > > > Thanks, > > > > --Justin > > > > > > On Nov 20, 2012, at 6:40 AM, Gurucharan Shetty <[email protected]> wrote: > > > >> From: Gurucharan Shetty <[email protected]> > >> > >> Currently, the 'ofport' column in Interface table is > >> ephemeral and is populated by vswitchd everytime it is > >> started or when a new interface is created with vswitchd > >> running. > >> > >> Making it persistent lets vswitchd try and assign the > >> same ofport number to a particular interface across > >> restarts. This is just a fallback option when > >> 'ofport_request' column is empty. > >> > >> Signed-off-by: Gurucharan Shetty <[email protected]> > >> --- > >> vswitchd/bridge.c | 15 ++++++++++----- > >> vswitchd/vswitch.ovsschema | 7 +++---- > >> 2 files changed, 13 insertions(+), 9 deletions(-) > >> > >> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > >> index d1e24d0..8f16432 100644 > >> --- a/vswitchd/bridge.c > >> +++ b/vswitchd/bridge.c > >> @@ -247,6 +247,7 @@ static void iface_refresh_cfm_stats(struct iface *); > >> static void iface_refresh_stats(struct iface *); > >> static void iface_refresh_status(struct iface *); > >> static bool iface_is_synthetic(const struct iface *); > >> +static int64_t iface_pick_ofport(const struct ovsrec_interface *); > >> > >> /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.) > >> * > >> @@ -295,8 +296,7 @@ bridge_init_ofproto(const struct ovsrec_open_vswitch > >> *cfg) > >> iface_hint = xmalloc(sizeof *iface_hint); > >> iface_hint->br_name = br_cfg->name; > >> iface_hint->br_type = br_cfg->datapath_type; > >> - iface_hint->ofp_port = if_cfg->n_ofport_request ? > >> - *if_cfg->ofport_request : OFPP_NONE; > >> + iface_hint->ofp_port = iface_pick_ofport(if_cfg); > >> > >> shash_add(&iface_hints, if_cfg->name, iface_hint); > >> } > >> @@ -322,7 +322,6 @@ bridge_init(const char *remote) > >> idl = ovsdb_idl_create(remote, &ovsrec_idl_class, true); > >> idl_seqno = ovsdb_idl_get_seqno(idl); > >> ovsdb_idl_set_lock(idl, "ovs_vswitchd"); > >> - ovsdb_idl_verify_write_only(idl); > >> > >> ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_cur_cfg); > >> ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_statistics); > >> @@ -347,7 +346,6 @@ bridge_init(const char *remote) > >> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_link_state); > >> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_link_resets); > >> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_mtu); > >> - ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_ofport); > >> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_statistics); > >> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_status); > >> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_cfm_fault); > >> @@ -2545,7 +2543,7 @@ bridge_queue_if_cfg(struct bridge *br, > >> > >> if_cfg->cfg = cfg; > >> if_cfg->parent = parent; > >> - if_cfg->ofport = cfg->n_ofport_request ? *cfg->ofport_request : > >> OFPP_NONE; > >> + if_cfg->ofport = iface_pick_ofport(cfg); > >> hmap_insert(&br->if_cfg_todo, &if_cfg->hmap_node, > >> hash_string(if_cfg->cfg->name, 0)); > >> } > >> @@ -3478,6 +3476,13 @@ iface_is_synthetic(const struct iface *iface) > >> return ovsdb_idl_row_is_synthetic(&iface->cfg->header_); > >> } > >> > >> +static int64_t > >> +iface_pick_ofport(const struct ovsrec_interface *cfg) > >> +{ > >> + int64_t ofport = cfg->n_ofport ? *cfg->ofport : OFPP_NONE; > >> + return cfg->n_ofport_request ? *cfg->ofport_request : ofport; > >> +} > >> + > >> > >> /* Port mirroring. */ > >> > >> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema > >> index fd2bb4b..293a11b 100644 > >> --- a/vswitchd/vswitch.ovsschema > >> +++ b/vswitchd/vswitch.ovsschema > >> @@ -1,6 +1,6 @@ > >> {"name": "Open_vSwitch", > >> - "version": "6.11.1", > >> - "cksum": "2684374538 17324", > >> + "version": "6.11.2", > >> + "cksum": "2033079075 17296", > >> "tables": { > >> "Open_vSwitch": { > >> "columns": { > >> @@ -180,8 +180,7 @@ > >> "external_ids": { > >> "type": {"key": "string", "value": "string", "min": 0, "max": > >> "unlimited"}}, > >> "ofport": { > >> - "type": {"key": "integer", "min": 0, "max": 1}, > >> - "ephemeral": true}, > >> + "type": {"key": "integer", "min": 0, "max": 1}}, > >> "ofport_request": { > >> "type": { > >> "key": {"type": "integer", > >> -- > >> 1.7.9.5 > >> > >> _______________________________________________ > >> dev mailing list > >> [email protected] > >> http://openvswitch.org/mailman/listinfo/dev > > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
