Hi Ben, Thanks for the review.
All the comments have been incorporated in Version 2 of the patch. Thanks Niti Rohilla On Thu, Jul 9, 2015 at 5:36 AM, Ben Pfaff <b...@nicira.com> wrote: > On Tue, Jul 07, 2015 at 01:17:36PM +0530, niti1...@gmail.com wrote: > > From: Niti <niti.rohi...@tcs.com> > > > > This patch adds support for Openflow1.4 set/get asynchronous > configuration > > messages. OpenVSwitch already supports set/get asynchronous configuration > > messages for Openflow1.3. In this patch OFPT_SET_ASYNC_CONFIG message > > allows the controllers to set the configuration for OFPT_ROLE_STATUS, > > OFPT_TABLE_STATUS and OFPT_REQUESTFORWARD in addition to the Openflow1.3 > > messages. In a OFPT_SET_ASYNC, only the properties that shall be changed > > need to be included, properties that are omitted from the message are > > unchanged. > > > > The OFPT_GET_ASYNC_CONFIG is used to query the asynchronous configuration > > of switch. In a OFPT_GET_ASYNC_REPLY message, all properties must be > > included. > > > > According to Openflow1.4 the initial configuration shall be: > > > > - In the “master” or “equal” role, enable all OFPT_PACKET_IN messages, > > except those with reason OFPR_INVALID_TTL, enable all > OFPT_PORT_STATUS > > and OFPT_FLOW_REMOVED messages, and disable all OFPT_ROLE_STATUS, > > OFPT_TABLE_STATUS and OFPT_REQUESTFORWARD messages. > > > > - In the “slave” role, enable all OFPT_PORT_STATUS messages and > disable > > all OFPT_PACKET_IN, OFPT_FLOW_REMOVED, OFPT_ROLE_STATUS, > > OFPT_TABLE_STATUS and OFPT_REQUESTFORWARD messages. > > > > Signed-off-by: Niti Rohilla <niti.rohi...@tcs.com> > > Thanks for the patch! > > NEWS says that this is an OpenFlow extension. I don't think that's > true--isn't everything in this patch standard OpenFlow 1.4? > > Please implement ofputil_*() functions for encoding and decoding > get_async_config replies. This will ultimately be cleaner than > implementing them directly in ofp-print.c and ofproto.c. > > What is the reason for the typedef in connmgr.h? We don't normally use > typedefs in OVS. It is only used in one place, so that use should > probably be replaced by a reference to the enum itself. > > Thanks, > > Ben. > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
