Hi Amit, your patch is corrupted. (It is double-spaced, at least.)
Please re-send in a form that can be applied directly, e.g. try emailing
it to yourself then applying with "git am".
On Wed, Jul 04, 2012 at 07:05:39AM +0000, Amit Tewari wrote:
> 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
