[ovs-dev] [PATCH] ofproto/ofproto.c: After port_mod + barrier, port config may not be updated

Isaku Yamahata yamahata at valinux.co.jp
Tue Oct 2 10:42:12 UTC 2012

The following OF packets may produce the wrong result as follows.
It depends on how ovs-vswitchd serves OF packets. Sending the OF packets
in single TCP packet will increase the possiblity.

OF request:
  feature request
  port_mod with config LINK DOWN
  feature request

The replies:
  feature reply with port config UP
  feature reply with port config UP (this should be DOWN because
                                     it's after barrier)
  port status with port config = DOWN and port status = DOWN

The direct cause is updte_port_config() @ ofproto/ofproto.c doesn't update
ofputil_phy_port::config. And later the config member is updated
by update_port() in main loop().
It seems that it tries to produce port_status event due to config change.
But changing other OFPC flags doesn't generate port_status event, so it's
consistent not to generate the event when OFPC_LINK_DOWN flag is changed.

Signed-off-by: Isaku Yamahata <yamahata at valinux.co.jp>
I can't find any explicit description in the spec about whether to generate
port_status event when OFPC_ flags are changed.
So which way do we like? generate port_status event or not?
Anyway I think port_status event should be generated (or not generated)
independently on which flags are changed.
 ofproto/ofproto.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 25b1824..1b6821e 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2171,12 +2171,20 @@ update_port_config(struct ofport *port,
     toggle = (config ^ port->pp.config) & mask;
     if (toggle & OFPUTIL_PC_PORT_DOWN) {
+        int error;
         if (config & OFPUTIL_PC_PORT_DOWN) {
-            netdev_turn_flags_off(port->netdev, NETDEV_UP, true);
+            error = netdev_turn_flags_off(port->netdev, NETDEV_UP, true);
         } else {
-            netdev_turn_flags_on(port->netdev, NETDEV_UP, true);
+            error = netdev_turn_flags_on(port->netdev, NETDEV_UP, true);
+        }
+        if (error) {
+            char name[sizeof port->pp.name];
+            ovs_strlcpy(name, port->pp.name, sizeof port->pp.name);
+            VLOG_WARN("can't %s port %s %s",
+                      config & OFPUTIL_PC_PORT_DOWN ? "DOWN" : "UP",
+                      name, strerror(error));
+            toggle &= ~OFPUTIL_PC_PORT_DOWN;
-        toggle &= ~OFPUTIL_PC_PORT_DOWN;
     port->pp.config ^= toggle;

More information about the dev mailing list