[ovs-dev] [PATCH] controller: Remote connection option to OpenFlow switch.

Ben Pfaff blp at ovn.org
Thu Aug 3 20:50:25 UTC 2017


On Mon, Jul 31, 2017 at 04:31:23PM +0530, Jai Singh Rana wrote:
> Currently ovn-controller uses default unix domain socket in Open Vswitch's
> "run" directory to connect to OpenFlow switch. If
> ovn-controller/ovsdb-server and vswitchd is running on different systems,
> remote connection method needs to be provided.
> 
> Added configuration option to override default connection by using OVSDB
> external-ids "ovn-ofswitch-remote". Using this external-id, desired tcp
> or unix connection to OpenFlow switch can be specified.
> 
> Tested this by using tcp/unix method configured through external-id
> "ovn-ofswitch-remote" and confirmed connection as flows getting updated in
> Open vSwitch.
> 
> Signed-off-by: Jai Singh Rana <JaiSingh.Rana at caviumnetworks.com>

Thank you for your contribution.

Clang says:

    ../ovn/controller/ofctrl.c:464:28: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
    ../ovn/controller/pinctrl.c:1034:28: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]

GCC gives similar warnings.

+      <dt><code>external_ids:ovn-ofswitch-remote</code></dt>
+      <dd>

The following line seems out of place.  Remove it?
+        <code>ovn-ofswitch-remote</code>

Please capitalize "OpenFlow" correctly:
+        The OpenFLow connection method that this system can connect to reach

Please don't check for NULL before calling free (see the coding style
guide):
+    if (ovn_ofswitch_remote) {
+        free(ovn_ofswitch_remote);
+    }

This patch breaks the logic of what switch to contact into multiple
places.  I don't understand why.  I think that get_ovn_ofswitch_remote()
should always return the right switch to contact, instead of sometimes
returning NULL.

Thanks,

Ben.


More information about the dev mailing list