On Tue, Mar 06, 2012 at 08:42:16AM +0900, FUJITA Tomonori wrote: > On Mon, 5 Mar 2012 09:34:06 +0700 > Simon Horman <[email protected]> wrote: > > > OFPAction.parser() should call the parser of one of the classes registered > > in _ACTION_TYPES() rather than calling itself until a buffer underflow > > occurs. > > > > Also, it is not necessary to increment offset as the parser method of the > > registered classes re-parse the type and length and thus do not expect it > > to be incremented. > > > > Signed-off-by: Simon Horman <[email protected]> > > > > --- > > > > I am unsure of the merit of using assert on error in this code. > Yeah, a malicious switch can cause this assertion. It's pointless. We > should handle about this error properly. At lease, please put a > "FIXME" comment. Assertion is only useful to find our bugs not > other components' bugs (switches, etc).
I have to admit some assert I put are pointless. Needs clean up. OFPParserError (or something like that) needs to be introduced instead of assert. As for malicious packet, the exception is raised up to gevent StreamServer. And then the connection is disconnected. This is intentional. (It should be explicitly commented in the code. Okay I'll send a patch) If we'd like to be a bit more smarter like skip to the next message, it can be easily done by catching exception explicitly. thanks, > > --- > > ryu/ofproto/ofproto_v1_0_parser.py | 5 +++-- > > 1 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/ryu/ofproto/ofproto_v1_0_parser.py > > b/ryu/ofproto/ofproto_v1_0_parser.py > > index 5ee4e3d..b0d0e89 100644 > > --- a/ryu/ofproto/ofproto_v1_0_parser.py > > +++ b/ryu/ofproto/ofproto_v1_0_parser.py > > @@ -144,8 +144,9 @@ class OFPAction(OFPActionHeader): > > def parser(cls, buf, offset): > > type_, len_ = struct.unpack_from( > > ofproto_v1_0.OFP_ACTION_HEADER_PACK_STR, buf, offset) > > - offset += ofproto_v1_0.OFP_ACTION_HEADER_SIZE > > - return cls.parser(buf, offset) > > + cls_ = cls._ACTION_TYPES.get(type_) > > + assert cls_ is not None > > + return cls_.parser(buf, offset) > > > > > > @OFPAction.register_action_type(ofproto_v1_0.OFPAT_OUTPUT, > > -- > > 1.7.6.3 > > > > > > ------------------------------------------------------------------------------ > > Try before you buy = See our experts in action! > > The most comprehensive online learning library for Microsoft developers > > is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, > > Metro Style Apps, more. Free future releases when you subscribe now! > > http://p.sf.net/sfu/learndevnow-dev2 > > _______________________________________________ > > Ryu-devel mailing list > > [email protected] > > https://lists.sourceforge.net/lists/listinfo/ryu-devel > > ------------------------------------------------------------------------------ > Try before you buy = See our experts in action! > The most comprehensive online learning library for Microsoft developers > is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, > Metro Style Apps, more. Free future releases when you subscribe now! > http://p.sf.net/sfu/learndevnow-dev2 > _______________________________________________ > Ryu-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/ryu-devel > -- yamahata ------------------------------------------------------------------------------ Virtualization & Cloud Management Using Capacity Planning Cloud computing makes use of virtualization - but cloud computing also focuses on allowing computing to be delivered as a service. http://www.accelacomm.com/jaw/sfnl/114/51521223/ _______________________________________________ Ryu-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ryu-devel
