Hi Ben,
Design description and rationale -
---------------------------------------
We are adding switch configuration message (OFPT_FEATURE_REQUEST) and
statistics message (OFPT_STATS_REQUEST) as a condition in vconn.c file
to prevent the reset of connection on receiving incorrect version in
feature request and stat request packets from controller.
Checking the version mismatch between stat request / feature request
packets and version supported by switch in ofproto.c file. If versions
are different then return the error packet to controller.
Testing after merging patch in OVS 1.4.1
-------------------------------------------------
1. Verify OFPET_BAD_REQUEST OFPBRC_BAD_VERSION code message from
switch to controller for switch description statistics, aggregate statistics,
table statistics, port statistics and queue statistics when there is mismatch
version.
2. Verify stats request and reply message from controller to switch
for switch description statistics, aggregate statistics, table statistics,
port statistics and queue statistics when version match.
3. Verify OFPET_BAD_REQUEST OFPBRC_BAD_VERSION code message from switch
to controller for feature request message when there is mismatch version.
4. Verify feature request and feature reply messages from controller to
switch for switch description statistics, aggregate statistics, table
statistics,
flow statistics and queue statistics when version match.
Patch
---------
ofproto/ofproto.c | 21 +++++++++++++++++++++
lib/vconn.c | 4 +++-
2 files changed, 24 insertions(+), 1 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 9057215..0a7fbcc 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2894,6 +2894,9 @@ handle_openflow__(struct ofconn *ofconn, const struct
ofpbuf *msg)
return handle_echo_request(ofconn, oh);
case OFPUTIL_OFPT_FEATURES_REQUEST:
+ if (oh->version != OFP_VERSION) {
+ return ofp_mkerr(OFPET_BAD_REQUEST,OFPBRC_BAD_VERSION);
+ }
return handle_features_request(ofconn, oh);
case OFPUTIL_OFPT_GET_CONFIG_REQUEST:
@@ -2933,23 +2936,41 @@ handle_openflow__(struct ofconn *ofconn, const struct
ofpbuf *msg)
/* Statistics requests. */
case OFPUTIL_OFPST_DESC_REQUEST:
+ if (oh->version != OFP_VERSION) {
+ return ofp_mkerr(OFPET_BAD_REQUEST,OFPBRC_BAD_VERSION);
+ }
return handle_desc_stats_request(ofconn, msg->data);
case OFPUTIL_OFPST_FLOW_REQUEST:
case OFPUTIL_NXST_FLOW_REQUEST:
+ if (oh->version != OFP_VERSION) {
+ return ofp_mkerr(OFPET_BAD_REQUEST,OFPBRC_BAD_VERSION);
+ }
return handle_flow_stats_request(ofconn, msg->data);
case OFPUTIL_OFPST_AGGREGATE_REQUEST:
case OFPUTIL_NXST_AGGREGATE_REQUEST:
+ if (oh->version != OFP_VERSION) {
+ return ofp_mkerr(OFPET_BAD_REQUEST,OFPBRC_BAD_VERSION);
+ }
return handle_aggregate_stats_request(ofconn, msg->data);
case OFPUTIL_OFPST_TABLE_REQUEST:
+ if (oh->version != OFP_VERSION) {
+ return ofp_mkerr(OFPET_BAD_REQUEST,OFPBRC_BAD_VERSION);
+ }
return handle_table_stats_request(ofconn, msg->data);
case OFPUTIL_OFPST_PORT_REQUEST:
+ if (oh->version != OFP_VERSION) {
+ return ofp_mkerr(OFPET_BAD_REQUEST,OFPBRC_BAD_VERSION);
+ }
return handle_port_stats_request(ofconn, msg->data);
case OFPUTIL_OFPST_QUEUE_REQUEST:
+ if (oh->version != OFP_VERSION) {
+ return ofp_mkerr(OFPET_BAD_REQUEST,OFPBRC_BAD_VERSION);
+ }
return handle_queue_stats_request(ofconn, msg->data);
case OFPUTIL_MSG_INVALID:
diff --git a/lib/vconn.c b/lib/vconn.c
index 6ea9366..c499623 100644
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -546,7 +546,9 @@ do_recv(struct vconn *vconn, struct ofpbuf **msgp)
&& oh->type != OFPT_ERROR
&& oh->type != OFPT_ECHO_REQUEST
&& oh->type != OFPT_ECHO_REPLY
- && oh->type != OFPT_VENDOR)
+ && oh->type != OFPT_VENDOR
+ && oh->type != OFPT_FEATURES_REQUEST
+ && oh->type != OFPT_STATS_REQUEST)
{
if (vconn->version < 0) {
VLOG_ERR_RL(&bad_ofmsg_rl,
Regards,
Prabina
-----Original Message-----
From: [email protected] [mailto:[email protected]]
On Behalf Of Ben Pfaff
Sent: Tuesday, May 22, 2012 11:40 PM
To: Varun Gupta
Cc: [email protected]
Subject: Re: [ovs-discuss] OpenVSwitch - "OFPBRC_BAD_VERSION" error packet not
received when there is version mismatch between stats request and stats reply
Can you provide a patch? I'll apply it, if it's reasonable.
On Tue, May 22, 2012 at 07:40:49AM +0000, Varun Gupta wrote:
> The two issues I raised (stats request/reply and feature request/reply) were
> found when we were evaluating the switch's response in case the controller
> behave indifferently.
> Our objective is to make OVS more robust and respond with an error
> (OFPBRC_BAD_VERSION) in the cases of 'OFPET_BAD_REQUEST' error type as
> mentioned in spec (1.0):
>
> -------------------------------------------
>
> For the OFPET_BAD_REQUEST error type, the following codes are currently de-
> fined:
> /* ofp_error_msg 'code' values for OFPET_BAD_REQUEST. 'data' contains at least
> * the first 64 bytes of the failed request. */
> enum ofp_bad_request_code {
> OFPBRC_BAD_VERSION, /* ofp_header.version not supported. */
> ----------
> ----------
> };
>
> ------------------------------------------
>
> Regards,
> Varun
>
> -----Original Message-----
> From: Ben Pfaff [mailto:[email protected]]
> Sent: Monday, May 21, 2012 10:04 PM
> To: Varun Gupta
> Cc: [email protected]
> Subject: Re: [ovs-discuss] OpenVSwitch - "OFPBRC_BAD_VERSION" error packet
> not received when there is version mismatch between stats request and stats
> reply
>
> On Mon, May 21, 2012 at 10:50:19AM +0000, Varun Gupta wrote:
> > Switch is not notifying the controller for OFPBRC_BAD_VERSION, /*
> > ofp_header.version not supported. */ when there is version mismatch
> > between stats request and stats reply. The switch was resetting the
> > connection and only a warning message was generated on switch in log
> > (/var/log/messages) before resetting connection.
>
> It's hard for me to consider this a serious bug, because it's a bug
> in the controller to send an OpenFlow message of a version that was
> not negotiated in the OpenFlow version negotiation. I don't know why
> you'd do that.
>
>
>
> 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
[email protected]
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
[email protected]
http://openvswitch.org/mailman/listinfo/discuss
