On Tue, Nov 20, 2012 at 02:19:53PM -0800, Gurucharan Shetty wrote: > On Tue, Nov 20, 2012 at 1:14 PM, Ben Pfaff <[email protected]> wrote: > > > > On Tue, Nov 20, 2012 at 03:40:50AM -0800, Gurucharan Shetty 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]> > > > > I know that you told me this in person just yesterday, but can you > > remind me why we need to make ofport a read/write column? > > If I do not do it, this is what happens. > From my notes (sorry for the cryptic messages): > > * Start openvswitch and create a few internal interfaces. > * Stop openvswitch, remove kernel module, re-insmod it and start ovsdb-server. > * Start ovs-vswitchd in gdb. > > * Code stores the value of ofport in the "struct if_cfg -> ofport" > datastructure for each interface. > * Go through each interface, and through iface_clear_db_record make > its ofport as -1. Commit it to DB. Values can actually be seen in the > database at this point and all are -1 for ofport column (with > ovs-vsctl get). > > * Call "iface_create" for each interface. > - For the first interface, if_cfg->cfg->ofport is -1. And when > iface_set_ofp_port() is called with a +ve > value (ofport value), ovsdb_idl_txn_write is called and row->new is > set because the 2 values of ofport is different(one is -1 and the > other +ve). > > - ovsdb_idl_txn_commit is called which commits this +ve value to DB. > > - In ovsdb_idl_txn_commit, ovsdb_idl_txn_disassemble is called which > changes all the rows associated with "all other" interfaces to their > "old" values (the old values have ofport as +ve) in the memory. > > -iface_create is called for second interface. Now, > if_cfg->cfg->ofport is +ve and we are setting the same value with > iface_set_ofp_port(). Since values are same, DB is not updated. > > - So this creates a situation where except for one interface, all > interfaces have their ofport set as "-1".
I think that you have uncovered a pre-existing bug in ovs-vswitchd. ovs-vswitchd is supposed to set the ofport column to -1 only if there is an error that prevents the port from being created. But it sounds like, instead, we populate every ofport column with -1 before we attempt to add the ports. This is probably a mistake. I would think that, instead, we should only modify ofport once we either know the new OpenFlow port number or know that the port cannot be created. I think that would resolve the problem. Can you take a look at solving it that way? Thanks, Ben. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
