Hi Ben, Thanks for the review.
All the comments have been incorporated in Version 3 of the patch. I am not having any issues with "make check". All the test cases in testsuite are executing successfully on the latest master code. Thanks Niti Rohilla On Wed, Jul 15, 2015 at 9:49 PM, Ben Pfaff <b...@nicira.com> wrote: > On Fri, Jul 10, 2015 at 05:42:50PM +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 revision! > > This commit causes many failures in the testsuite. Please run "make > check" and figure out the problems. > > This code combines decoding "set async config" and "get async config > reply" into a single function. That's mostly OK--although it should be > mentioned in comments--but there is an important distinction that I > believe it misses. For decoding "set async config", it is important to > return an error when an unknown attribute type is present, because the > switch can't implement an unknown type. But for decoding "get async > config reply", there is no such problem (it just means that the switch > supports some feature that the controller doesn't understand, which is > not a problem) so the unknown attribute type should be ignored. Please > implement this distinction. > > In NEWS, s/asynchoronous/asynchronous/. > > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
