Hi Ben, 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? If you found the patch appropriate then we can provide you the patch for error handling of all openflow messages. Then we will provide error handling for following openflow packets- ECHO_REQUEST FEATURE_REQUEST GET_CONFIG_REQUEST SET_CONFIG_REQUEST BARRIER_REQUEST ECHO_REPLY FEATURES_REQUEST OFPST_DESC_REQUEST FLOW_REQUEST AGGREGATE_REQUEST TABLE_REQUEST PORT_REQUEST QUEUE_REQUEST Please let me know if am missing any openflow message. Regards Amit Tewari NEC-HCL ST -----Original Message----- From: discuss-boun...@openvswitch.org [mailto:discuss-boun...@openvswitch.org] On Behalf Of Ben Pfaff Sent: Friday, May 25, 2012 9:48 PM To: Prabina Pattnaik Cc: discuss@openvswitch.org; b...@openvswitch.org; Varun Gupta Subject: Re: [ovs-discuss] [PATCH] ofproto/ofproto.c, lib/vconn.c:: OFPBRC_BAD_VERSION generated from switch when there is version mismatch. I think the handling should be uniform, if we need any at all. On Fri, May 25, 2012 at 01:16:44PM +0000, Prabina Pattnaik wrote: > Hi Ben, > > This is grey out area of openflow 1.0 spec that with which request packet > "OFPBRC_BAD_VERSION" should come. > If we go with remaining request packet like ECHO_REQUEST, BARRIER_REQUEST and > GET_CONFIG_REQUEST, this error is not coming and connection is resetting in > case of BARRIER_REQUEST and GET_CONFIG_REQUEST. > > We haven't handled for above packets because controller is not supposed to > get any useful data in a data field from switch. > It can be supported for OFPBRC_BAD_VERSION error type to handle all request > packets to prevent the reset connection when there is version mismatch > between request and reply packet. > > Regards, > Prabina > > -----Original Message----- > From: Ben Pfaff [mailto:b...@nicira.com] > Sent: Wednesday, May 23, 2012 10:11 PM > To: Prabina Pattnaik > Cc: b...@openvswitch.org; Varun Gupta; 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. > > You made changes to a few randomly selected OpenFlow messages. What > is special about these messages? Why change them and not others? > > On Wed, May 23, 2012 at 11:59:15AM +0000, Prabina Pattnaik wrote: > > 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: discuss-boun...@openvswitch.org > > [mailto:discuss-boun...@openvswitch.org] On Behalf Of Ben Pfaff > > Sent: Tuesday, May 22, 2012 11:40 PM > > To: Varun Gupta > > Cc: b...@openvswitch.org > > 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:b...@nicira.com] > > > Sent: Monday, May 21, 2012 10:04 PM > > > To: Varun Gupta > > > Cc: b...@openvswitch.org > > > 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 > > 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. . > > > > ----------------------------------------------------------------------------------------------------------------------- > > > > 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 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
