> Hi Yamamoto-san,
>
> thanks for your feedback and sorry for not addressing it in v4,
> it slipped my mind.
>
> On Thu, Mar 13, 2014 at 02:46:41PM +0900, YAMAMOTO Takashi wrote:
>> > ---
>> > v2 - v3
>> > * No change
>> > ---
>> > ryu/ofproto/ofproto_v1_4.py | 5 +++-
>> > ryu/ofproto/ofproto_v1_4_parser.py | 60
>> > ++++++++++++++++++++++++++++++++++++++
>> > 2 files changed, 64 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/ryu/ofproto/ofproto_v1_4.py b/ryu/ofproto/ofproto_v1_4.py
>> > index 8b9431e..0688491 100644
>> > --- a/ryu/ofproto/ofproto_v1_4.py
>> > +++ b/ryu/ofproto/ofproto_v1_4.py
>> > @@ -1451,7 +1451,10 @@ assert (calcsize(OFP_BUNDLE_CTRL_MSG_PACK_STR) +
>> > OFP_HEADER_SIZE ==
>> > OFP_BUNDLE_CTRL_MSG_SIZE)
>> >
>> > # struct ofp_bundle_add_msg
>> > -OFP_BUNDLE_ADD_MSG_PACK_STR = '!IHH' + _OFP_HEADER_PACK_STR
>> > +_OFP_BUNDLE_ADD_MSG_0_PACK_STR = 'I2xH'
>> > +OFP_BUNDLE_ADD_MSG_0_PACK_STR = '!' + _OFP_BUNDLE_ADD_MSG_0_PACK_STR
>> > +OFP_BUNDLE_ADD_MSG_PACK_STR = (OFP_BUNDLE_ADD_MSG_0_PACK_STR +
>> > + _OFP_HEADER_PACK_STR)
>> > OFP_BUNDLE_ADD_MSG_SIZE = 24
>> > assert (calcsize(OFP_BUNDLE_ADD_MSG_PACK_STR) + OFP_HEADER_SIZE ==
>> > OFP_BUNDLE_ADD_MSG_SIZE)
>> > diff --git a/ryu/ofproto/ofproto_v1_4_parser.py
>> > b/ryu/ofproto/ofproto_v1_4_parser.py
>> > index 186684a..c4d2431 100644
>> > --- a/ryu/ofproto/ofproto_v1_4_parser.py
>> > +++ b/ryu/ofproto/ofproto_v1_4_parser.py
>> > @@ -5850,3 +5850,63 @@ class OFPBundleCtrlMsg(MsgBase):
>> > self.buf, ofproto.OFP_HEADER_SIZE, self.bundle_id,
>> > self.type, self.flags)
>> > self.buf += bin_props
>> > +
>> > +
>> > +@_set_msg_type(ofproto.OFPT_BUNDLE_ADD_MESSAGE)
>> > +class OFPBundleAddMsg(MsgBase):
>> > + """
>> > + Bundle control message
>> > +
>> > + The controller uses this message to create, destroy and commit bundles
>> > +
>> > + ================
>> > ======================================================
>> > + Attribute Description
>> > + ================
>> > ======================================================
>> > + bundle_id Id of the bundle
>> > + flags Bitmap of the following flags.
>> > + OFPBF_ATOMIC
>> > + OFPBF_ORDERED
>> > + message ``MsgBase`` subclass instance
>> > + properties List of ``OFPBundleProp`` subclass instance
>> > + ================
>> > ======================================================
>> > +
>> > + Example::
>> > +
>> > + def send_bundle_add_message(self, datapath):
>> > + ofp = datapath.ofproto
>> > + ofp_parser = datapath.ofproto_parser
>> > +
>> > + msg = ofp_parser.OFPRoleRequest(datapath,
>> > ofp.OFPCR_ROLE_EQUAL, 0)
>> > +
>> > + req = ofp_parser.OFPBundleAddMsg(datapath, 7,
>> > [ofp.OFPBF_ATOMIC],
>> > + msg, [])
>> > + datapath.send_msg(req)
>> > + """
>> > + def __init__(self, datapath, bundle_id, flags, message, properties):
>> > + super(OFPBundleAddMsg, self).__init__(datapath)
>> > + self.bundle_id = bundle_id
>> > + self.flags = flags
>> > + self.message = message
>> > + self.properties = properties
>> > +
>> > + def _serialize_body(self):
>> > + # Message
>>
>> the xid field of inner header should be the same as the one in outer.
>> (OF1.4.0 7.3.9.2)
>
> Do you think this should be handled using an assertion
> or by forcibly setting one of the headers to match the other?
IMO the latter is more appropriate.
given that set_xid for the inner message is likely a bug in the caller,
assert msg.xid is None might be a good idea too.
i.e.
assert self.message.xid is None
self.message.xid = self.xid
YAMAMOTO Takashi
>
>> > + tail_buf = self.message.serialize()
>>
>> this assignment of tail_buf seems unnecessary.
>
> Thanks, I'll remove the assignment.
>
>> > + tail_buf = self.message.buf
>> > +
>> > + # Pad
>> > + message_len = len(tail_buf)
>> > + pad_len = utils.round_up(message_len, 8) - message_len
>> > + ofproto_parser.msg_pack_into("%dx" % pad_len, tail_buf,
>> > message_len)
>>
>> my reading of the spec is that padding is unnecessary unless
>> there's one or more properties.
>
> Thanks, I'll guard the padding code with if len(p) > 0
>
>> YAMAMOTO Takashi
>>
>> > +
>> > + # Properties
>> > + for p in self.properties:
>> > + tail_buf += p.serialize()
>> > +
>> > + # Head
>> > + msg_pack_into(ofproto.OFP_BUNDLE_ADD_MSG_0_PACK_STR,
>> > + self.buf, ofproto.OFP_HEADER_SIZE, self.bundle_id,
>> > + self.flags)
>> > +
>> > + # Finish
>> > + self.buf += tail_buf
>> > --
>> > 1.8.5.2
>> >
>> >
>> > ------------------------------------------------------------------------------
>> > Learn Graph Databases - Download FREE O'Reilly Book
>> > "Graph Databases" is the definitive new guide to graph databases and their
>> > applications. Written by three acclaimed leaders in the field,
>> > this first edition is now available. Download your free book today!
>> > http://p.sf.net/sfu/13534_NeoTech
>> > _______________________________________________
>> > Ryu-devel mailing list
>> > [email protected]
>> > https://lists.sourceforge.net/lists/listinfo/ryu-devel
>>
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel
