On Mon, May 21, 2012 at 10:58:14PM +0900, FUJITA Tomonori wrote: > On Mon, 21 May 2012 22:50:18 +0900 > Isaku Yamahata <[email protected]> wrote: > > > On Mon, May 21, 2012 at 09:11:48PM +0900, FUJITA Tomonori wrote: > >> On Mon, 21 May 2012 12:48:12 +0900 > >> Isaku Yamahata <[email protected]> wrote: > >> > >> > On Sun, May 20, 2012 at 10:42:12AM +0900, FUJITA Tomonori wrote: > >> > > This enabls a vendor to register vendor's specific parser to handle > >> > > Switch to Controller Vendor messages. > >> > > > >> > > An object that vendor's parser returns is placed at msg.data. > >> > > > >> > > Signed-off-by: FUJITA Tomonori <[email protected]> > >> > > --- > >> > > ryu/ofproto/ofproto_v1_0_parser.py | 17 ++++++++++++++++- > >> > > 1 files changed, 16 insertions(+), 1 deletions(-) > >> > > > >> > > diff --git a/ryu/ofproto/ofproto_v1_0_parser.py > >> > > b/ryu/ofproto/ofproto_v1_0_parser.py > >> > > index b3a7cb4..efff7e5 100644 > >> > > --- a/ryu/ofproto/ofproto_v1_0_parser.py > >> > > +++ b/ryu/ofproto/ofproto_v1_0_parser.py > >> > > @@ -966,6 +966,15 @@ class OFPEchoReply(MsgBase): > >> > > @_register_parser > >> > > @_set_msg_type(ofproto_v1_0.OFPT_VENDOR) > >> > > class OFPVendor(MsgBase): > >> > > + _VENDORS = {} > >> > > + > >> > > + @staticmethod > >> > > + def register_vendor(_id): > >> > > + def _register_vendor(cls): > >> > > + OFPVendor._VENDORS[_id] = cls > >> > > + return cls > >> > > + return _register_vendor > >> > > + > >> > > def __init__(self, datapath): > >> > > super(OFPVendor, self).__init__(datapath) > >> > > self.data = None > >> > > >> > id_ instead of _id. > >> > Prefixing '_' means unused argument in python and pylint understands so. > >> > To avoid name conflict, append '_' to the name. > >> > > >> > > >> > > @@ -978,7 +987,13 @@ class OFPVendor(MsgBase): > >> > > (msg.vendor,) = struct.unpack_from( > >> > > ofproto_v1_0.OFP_VENDOR_HEADER_PACK_STR, msg.buf, > >> > > ofproto_v1_0.OFP_HEADER_SIZE) > >> > > - msg.data = msg.buf[ofproto_v1_0.OFP_VENDOR_HEADER_SIZE:] > >> > > + > >> > > + cls_ = cls._VENDORS.get(msg.vendor) > >> > > + if cls_: > >> > > + msg.data = cls_.parser(datapath, msg.buf, 0) > >> > > + else: > >> > > + msg.data = msg.buf[ofproto_v1_0.OFP_VENDOR_HEADER_SIZE:] > >> > > + > >> > > return msg > >> > > > >> > > def serialize_header(self): > >> > > >> > > >> > How about something like this? The cost is parsing vendor header twice. > >> > > >> > class OFPVendor(MsgBase) > >> > ... > >> > def parser(...): > >> > (vendor, ) = struct.unpack_from( > >> > ofproto_v1_0.OFP_VENDOR_HEADER_PACK_STR, buf, > >> > ofproto_v1_0.OFP_HEADER_SIZE) > >> > cls_ = cls._VENDORS.get(vendor) > >> > if cls_: > >> > msg = cls_.parser(datapath, version, msg_type, msg_len, xid, > >> > buf) > >> > else: > >> > msg = super(OFPVendor, cls).parser(datapath, version, > >> > msg_type, > >> > msg_len, xid, buf) > >> > msg.vendor = vendor > >> > msg.data = msg.buf[ofproto_v1_0.OFP_VENDOR_HEADER_SIZE:] > >> > reutrn msg > >> > > >> > class NiciraHeader(object): > > > > Oops. This should be NiciraHeader(OFPVendor). > > > > > >> > ... > >> > @classmethod > >> > def parser(cls, datapath, version, msg_type, msg_len, xid, buf): > >> > (vendor, subtype) = unpack... > >> > cls_ = cls._NX_SUBTYPES[subtype] > >> > msg = cls_.parser(datapath, version, msg_type, msg_len, xid, buf) > >> > msg.subtype = subtype > >> > return msg > >> > > >> > class NXRoleRequest(NiciraHeader) > >> > ... > >> > @classmethod > >> > def parser(cls, datapath, version, msg_type, msg_len, xid, buf): > >> > msg = MsgBase.parser(cls, datapath, version, msg_type, msg_len, > >> > xid, buf) > >> > >> We need some tricks here. msg needs to be an OFPVendor > >> object. Otherwise, as I wrote, we need some changes to the event code > >> (and I'm not convinced that it's the right thing to do). > > > > NiciraHeader should be a subclass of OPFVendor. > > So NXRoleRequest is a subclass of OFPVendor. Thus msg is OFPVendor object. > > I'm not sure we are on the same page. I'm talking about the breakage > of ofp_msg_to_ev().
Ahh, I see. So I should have talked about _set_msg_type() It should have been @_register_parser @_set_msg_type(ofproto_v1_0.OFPT_VENDOR) class OFPVendor(MsgBase): ... # no decorator class NiciraHeader(OFPVendor): ... @_set_msg_type(ofproto_v1_0.OFPT_VENDOR) class NXRoleRequest(MsgBase): ... Then, EventOFPVendor and EventNxRoleRequest are created. The point is that _OFP_MSG_EVENTS is keyed by class name, not by OF type number. > > > If you have a ofpvendor hander which is something like > > @set_ev_cls(EventOFPVendor, MAIN_DISPATCHER) > > def ofp_vendor_hander(): > > if subtype == ...: > > subtype_handler() > > ... > > > > It can be split into nx_role_request_handler and ofp_vendor_handler. > > If such split isn't undesirable, set_ev_cls can be enhanced to accept > > a list of events. something like > > @set_ev_cls([EventOFPVendor, EventNXRoleRequest], MAIN_DISPATCHER) > > def ofp_vendor_or_nx_role_request_handler(): > > ... > > > > Does the above satisfy your requirement? Or can you elaborate on what > > you need? > > As I always say, I want less code in the code for stopgap vendor stuff. I see. If only nx role parser is needed, the above I'm proposing might be overkill. -- yamahata ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ _______________________________________________ Ryu-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ryu-devel
