[ovs-discuss] [PATCH] lib/ofp-util.c, lib/vconn.c, lib/rconn.c, lib/ofp-errors.c:: OFPERR_OFPBRC_BAD_VERSION generated from switch when there is version mismatch.

Amit Tewari Amit.Tewari at nechclst.in
Wed Jul 4 07:05:39 UTC 2012


Hi Ben,



I am sending patch for generating OFPERR_OFPBRC_BAD_VERSION error in case of version mismatch in

openflow packets from controller to switch. This patch is for latest release 1.6.1.



I have also patched lib/ofp-errors.c file so that OFPERR_OFPBRC_BAD_VERSION error packet contains switch

supported version i.e OFP10_VERSION.



Design description and rationale -

 ---------------------------------------

 We are returning OFPERR_OFPBRC_BAD_VERSION error in vconn.c file in do_recv routine

 for version mismatch for all openflow packets from controller and we prevent reset of

 connection in rconn.c file in rconn_recv routine by checking for OFPERR_OFPBRC_BAD_VERSION

 error from do_recv routine.

 Then we are checking the version mismatch between openflow packets in ofp util.c ofputil_lookup_openflow_message routine,

 this routine was returning earlier BAD_SUBTYPE error so we put a check for version mistmatch in this routine and

 return BAD_VERSION error and if version matches and if header type doesn't matches then still it returns BAD_SUBTYPE error.



 When we return BAD_VERSION error the error packet was having version of controller request packet and

 not switch supported version, so we put a check in ofp-errors.c file for setting version in error packet to switch

 supported version i.e OFP10_VERSION for BAD_VERSION error.



Testing after merging patch in OVS 1.6.1

 -------------------------------------------------

   1.   Verify OFPET_BAD_REQUEST OFPBRC_BAD_VERSION code message from

        switch to controller for  all openflow packets mentioned in file ofproto.c

        in handle_openflow__ routine when there is "version mismatch".

2.      Verify request and reply message from controller to switch for all openflow packets

     mentioned in file ofproto.c in handle_openflow__ routine when there is "version matches".



Patch

---------

  lib/ofp-util.c               |   3 +++

  lib/vconn.c |    2 ++

  lib/rconn.c |    3 ++-

  lib/ofp-errorc.c | 3 +++

  4 files changed, 10 insertions(+), 1 deletions(-)



diff --git a/lib/ofp-util.c b/lib/ofp-util.c

index 15340f6..7591f0f 100644

--- a/lib/ofp-util.c

+++ b/lib/ofp-util.c

@@ -355,6 +355,9 @@ ofputil_lookup_openflow_message(const struct ofputil_msg_category *cat,

         }

     }



+    if (version != type->ofp_version) {

+        return OFPERR_OFPBRC_BAD_VERSION;

+    }

     VLOG_WARN_RL(&bad_ofmsg_rl, "received %s of unknown type %"PRIu32,

                  cat->name, value);

     return cat->missing_error;



diff --git a/lib/vconn.c b/lib/vconn.c

index 5da5026..47622c6 100644

--- a/lib/vconn.c

+++ b/lib/vconn.c

@@ -555,6 +555,8 @@ do_recv(struct vconn *vconn, struct ofpbuf **msgp)

                             "%s: received OpenFlow version 0x%02"PRIx8" "

                             "!= expected %02x",

                             vconn->name, oh->version, vconn->version);

+                retval = OFPERR_OFPBRC_BAD_VERSION;

+                return retval;

             }

             ofpbuf_delete(*msgp);

             retval = EPROTO;



diff --git a/lib/rconn.c b/lib/rconn.c

index 1b69b8f..097d5ae 100644

--- a/lib/rconn.c

+++ b/lib/rconn.c

@@ -24,6 +24,7 @@

 #include "coverage.h"

 #include "ofp-util.h"

 #include "ofpbuf.h"

+#include "ofp-errors.h"

 #include "openflow/openflow.h"

 #include "poll-loop.h"

 #include "sat-math.h"

@@ -521,7 +522,7 @@ rconn_recv(struct rconn *rc)

     if (rc->state & (S_ACTIVE | S_IDLE)) {

         struct ofpbuf *buffer;

         int error = vconn_recv(rc->vconn, &buffer);

-        if (!error) {

+        if (!error || error == OFPERR_OFPBRC_BAD_VERSION) {

             copy_to_monitor(rc, buffer);

             if (rc->probably_admitted || is_admitted_msg(buffer)

                 || time_now() - rc->last_connected >= 30) {





diff --git a/lib/ofp-errors.c b/lib/ofp-errors.c

index 028475e..2af01ff 100644

--- a/lib/ofp-errors.c

+++ b/lib/ofp-errors.c

@@ -187,6 +187,9 @@ ofperr_encode_reply(enum ofperr error, const struct ofp_header *oh)

     uint16_t len = ntohs(oh->length);



     domain = ofperr_domain_from_version(oh->version);

+    if (error == OFPERR_OFPBRC_BAD_VERSION) {

+        domain = ofperr_domain_from_version(OFP10_VERSION);

+    }

     return ofperr_encode_msg__(error, domain, oh->xid, oh, MIN(len, 64));

 }







Regards

Amit Tewari

NEC HCL ST



-----Original Message-----

From: discuss-bounces at openvswitch.org [mailto:discuss-bounces at openvswitch.org] On Behalf Of Ben Pfaff

Sent: Thursday, June 28, 2012 8:54 PM

To: Amit Tewari

Cc: 'bugs at openvswitch.org'; 'discuss at openvswitch.org'

Subject: Re: [ovs-discuss] [PATCH] ofproto/ofproto.c, lib/vconn.c:: OFPBRC_BAD_VERSION generated from switch when there is version mismatch.



On Thu, Jun 28, 2012 at 06:16:06AM +0000, Amit Tewari wrote:

> As per our analysis and requirement we provided the patch for selected openflow messages which carry useful data in data field for controller from switch.

>

> Please confirm whether you have applied the patch we provided earlier?



No.  It doesn't make sense to add a special case for every message in

the middle of the code.  If we need to handle this at all, we should

handle it in a single place without duplicate special cases.



Also, your patch was against an old version of Open vSwitch.  We only

accept patches against the newest version.



> Please let me know if am missing any openflow message.



See, that's the problem.  You *can* miss some messages if you do it your

way.

_______________________________________________

discuss mailing list

discuss at openvswitch.org

http://openvswitch.org/mailman/listinfo/discuss



DISCLAIMER:

-----------------------------------------------------------------------------------------------------------------------

The contents of this e-mail and any attachment(s) are confidential and
intended

for the named recipient(s) only. 

It shall not attach any liability on the originator or NECHCL or its

affiliates. Any views or opinions presented in 

this email are solely those of the author and may not necessarily reflect the

opinions of NECHCL or its affiliates. 

Any form of reproduction, dissemination, copying, disclosure, modification,

distribution and / or publication of 

this message without the prior written consent of the author of this e-mail is

strictly prohibited. If you have 

received this email in error please delete it and notify the sender

immediately. .

-----------------------------------------------------------------------------------------------------------------------
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://openvswitch.org/pipermail/ovs-discuss/attachments/20120704/ca24e5a8/attachment-0001.html>


More information about the discuss mailing list