On Wed, Jan 30, 2013 at 10:54:54AM +0900, FUJITA Tomonori wrote:
> Better way to fix this?

Oops, sorry for this. In fact I included several same issue.
How about this?

>From cef0fb64167857b0231ae751cd3be401ed3fbd6e Mon Sep 17 00:00:00 2001
Message-Id: 
<cef0fb64167857b0231ae751cd3be401ed3fbd6e.1359512904.git.yamah...@valinux.co.jp>
From: Isaku Yamahata <[email protected]>
Date: Tue, 25 Dec 2012 13:40:04 +0900
Subject: [PATCH] controller/ofp_handler: improve version negotiation

- fix string concatenation. Needs surrounding paren.
- more version checks
- make hello_failed private. (adding "_" prefix)

Signed-off-by: Isaku Yamahata <[email protected]>
---
 ryu/controller/ofp_handler.py |   84 +++++++++++++++++++++++++++++++++--------
 1 file changed, 68 insertions(+), 16 deletions(-)

diff --git a/ryu/controller/ofp_handler.py b/ryu/controller/ofp_handler.py
index add0301..05ac79b 100644
--- a/ryu/controller/ofp_handler.py
+++ b/ryu/controller/ofp_handler.py
@@ -44,7 +44,7 @@ class OFPHandler(app_manager.RyuApp):
         super(OFPHandler, self).__init__(*args, **kwargs)
 
     @staticmethod
-    def hello_failed(datapath, error_desc):
+    def _hello_failed(datapath, error_desc):
         LOG.error(error_desc)
         error_msg = datapath.ofproto_parser.OFPErrorMsg(datapath)
         error_msg.type = datapath.ofproto.OFPET_HELLO_FAILED
@@ -62,13 +62,63 @@ class OFPHandler(app_manager.RyuApp):
         # pre 1.0 is not supported
         elements = getattr(msg, 'elements', None)
         if elements:
-            usable_versions = []
+            switch_versions = []
             for elem in elements:
-                usable_versions += elem.versions or []
+                switch_versions += elem.versions or []
+            switch_versions = sorted(set(switch_versions))
+            usable_versions = [
+                version for version in switch_versions
+                if version in datapath.supported_ofp_version]
+
+            # We didn't send our supported versions for interoperability as
+            # most switches would not understand elements At the moment.
+            # So the switch would think that the negotiated version would
+            # be max(negotiated_versions), but actual usable version is
+            # max(usable_versions).
+            negotiated_versions = [
+                version for version in switch_versions
+                if version <= max(datapath.supported_ofp_version)]
+            if negotiated_versions and not usable_versions:
+                # e.g.
+                # versions of OF 1.0 and 1.1 from switch
+                # max of OF 1.2 from Ryu and supported_ofp_version = (1.2, )
+                # negotiated version = 1.1
+                # usable version = None
+                error_desc = (
+                    'no compatible version found: '
+                    'switch versions 0x%x controller version 0x%x, '
+                    'the negotiated version is 0x%x, '
+                    'but no usable version found. '
+                    'If possible, set the switch to use one of OF version %s'
+                    % (switch_versions, max(datapath.supported_ofp_version),
+                       max(negotiated_versions),
+                       datapath.supported_ofp_version))
+                self._hello_failed(datapath, error_desc)
+                return
+            if (negotiated_versions and usable_versions and
+                max(negotiated_versions) != max(usable_versions)):
+                # e.g.
+                # versions of OF 1.0 and 1.1 from switch
+                # max of OF 1.2 from Ryu and supported_ofp_version = (1.0, 1.2)
+                # negotiated version = 1.1
+                # usable version = 1.0
+                #
+                # TODO: In order to get the version 1.0, Ryu need to send
+                # supported verions.
+                error_desc = (
+                    'no compatible version found: '
+                    'switch versions 0x%x controller version 0x%x, '
+                'the negotiated version is 0x%x but found usable 0x%x. '
+                'If possible, set the switch to use one of OF version %s' % (
+                    switch_versions, max(datapath.supported_ofp_version),
+                    max(negotiated_versions), sorted(usable_versions),
+                    usable_versions))
+                self._hello_failed(datapath, error_desc)
+                return
         else:
-            usable_versions = [version for version
-                               in datapath.supported_ofp_version
-                               if version <= msg.version]
+            usable_versions = sorted(set(version for version
+                                         in datapath.supported_ofp_version
+                                         if version <= msg.version))
             if (usable_versions and
                 max(usable_versions) != min(msg.version,
                                             datapath.ofproto.OFP_VERSION)):
@@ -99,19 +149,21 @@ class OFPHandler(app_manager.RyuApp):
                 # and optionally an ASCII string explaining the situation
                 # in data, and then terminate the connection.
                 version = max(usable_versions)
-                error_desc = 'no compatible version found: '
-                'switch 0x%x controller 0x%x, but found usable 0x%x. '
-                'If possible, set the switch to use OF version 0x%x' % (
-                    msg.version, datapath.ofproto.OFP_VERSION,
-                    version, version)
-                self.hello_failed(error_desc)
+                error_desc = (
+                    'no compatible version found: '
+                    'switch 0x%x controller 0x%x, but found usable 0x%x. '
+                    'If possible, set the switch to use OF version 0x%x' % (
+                        msg.version, datapath.ofproto.OFP_VERSION,
+                        version, version))
+                self._hello_failed(datapath, error_desc)
                 return
 
         if not usable_versions:
-            error_desc = 'unsupported version 0x%x. '
-            'If possible, set the switch to use one of the versions %s' % (
-                msg.version, datapath.supported_ofp_version.keys())
-            self.hello_failed(error_desc)
+            error_desc = (
+                'unsupported version 0x%x. '
+                'If possible, set the switch to use one of the versions %s' % (
+                    msg.version, datapath.supported_ofp_version.keys()))
+            self._hello_failed(datapath, error_desc)
             return
         datapath.set_version(max(usable_versions))
 
-- 
1.7.10.4



> 
> =
> >From 0a2e8b4e287e24b272597bf1d2b5dfcce4d2db2e Mon Sep 17 00:00:00 2001
> From: FUJITA Tomonori <[email protected]>
> Date: Wed, 30 Jan 2013 10:51:36 +0900
> Subject: [PATCH] of: fix hello version error printing
> 
> Fix the following error:
> 
> Traceback (most recent call last):
>   File "/Users/fujita/git/ryu/ryu/controller/controller.py", line 94, in 
> deactivate
>     method(self)
>   File "/Users/fujita/git/ryu/ryu/controller/controller.py", line 172, in 
> _recv_loop
>     self.ev_q.queue(ofp_event.ofp_msg_to_ev(msg))
>   File "/Users/fujita/git/ryu/ryu/controller/dispatcher.py", line 109, in 
> queue
>     self._dispatcher(ev)
>   File "/Users/fujita/git/ryu/ryu/controller/dispatcher.py", line 174, in 
> __call__
>     self.dispatch(ev)
>   File "/Users/fujita/git/ryu/ryu/controller/dispatcher.py", line 191, in 
> dispatch
>     handled = self._dispatch(ev, self.events.get(ev.__class__, []))
>   File "/Users/fujita/git/ryu/ryu/controller/dispatcher.py", line 182, in 
> _dispatch
>     ret = h(ev)
>   File "/Users/fujita/git/ryu/ryu/controller/ofp_handler.py", line 113, in 
> hello_handler
>     msg.version, datapath.supported_ofp_version.keys())
> TypeError: not all arguments converted during string formatting
> 
> Signed-off-by: FUJITA Tomonori <[email protected]>
> ---
>  ryu/controller/ofp_handler.py |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/ryu/controller/ofp_handler.py b/ryu/controller/ofp_handler.py
> index add0301..216848c 100644
> --- a/ryu/controller/ofp_handler.py
> +++ b/ryu/controller/ofp_handler.py
> @@ -108,10 +108,10 @@ class OFPHandler(app_manager.RyuApp):
>                  return
>  
>          if not usable_versions:
> -            error_desc = 'unsupported version 0x%x. '
> -            'If possible, set the switch to use one of the versions %s' % (
> +            error_desc = 'unsupported version 0x%x. ' \
> +                'If possible, set the switch to use one of the versions %s' 
> % (
>                  msg.version, datapath.supported_ofp_version.keys())
> -            self.hello_failed(error_desc)
> +            self.hello_failed(datapath, error_desc)
>              return
>          datapath.set_version(max(usable_versions))
>  
> -- 
> 1.7.4.4
> 
> 
> ------------------------------------------------------------------------------
> Everyone hates slow websites. So do we.
> Make your web apps faster with AppDynamics
> Download AppDynamics Lite for free today:
> http://p.sf.net/sfu/appdyn_d2d_jan
> _______________________________________________
> Ryu-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/ryu-devel
> 

-- 
yamahata

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to