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-boun...@openvswitch.org [mailto:discuss-boun...@openvswitch.org]
On Behalf Of Ben Pfaff
Sent: Thursday, June 28, 2012 8:54 PM
To: Amit Tewari
Cc: 'b...@openvswitch.org'; 'discuss@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@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. .
-----------------------------------------------------------------------------------------------------------------------_______________________________________________
discuss mailing list
discuss@openvswitch.org
http://openvswitch.org/mailman/listinfo/discuss
