Hi Iwamoto-San,

On 2016年08月23日 11:41, IWAMOTO Toshihiro wrote:
> At Tue, 23 Aug 2016 09:45:30 +0900,
> Iwase Yusuke wrote:
>>
>> Hi Iwamoto-San,
>>
>>
>> On 2016年08月22日 17:31, IWAMOTO Toshihiro wrote:
>>> At Mon, 22 Aug 2016 09:40:25 +0900,
>>> IWASE Yusuke wrote:
>>>>
>>>> Signed-off-by: IWASE Yusuke <[email protected]>
>>>> ---
>>>>  ryu/lib/packet/afi.py  |   1 +
>>>>  ryu/lib/packet/bgp.py  | 806 
>>>> +++++++++++++++++++++++++++++++++++++++++++++++--
>>>>  ryu/lib/packet/safi.py |   1 +
>>>>  3 files changed, 777 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/ryu/lib/packet/afi.py b/ryu/lib/packet/afi.py
>>>> index c84bf47..7076042 100644
>>>> --- a/ryu/lib/packet/afi.py
>>>> +++ b/ryu/lib/packet/afi.py
>>>> @@ -22,3 +22,4 @@ address-family-numbers.xhtml
>>>>
>>>>  IP = 1
>>>>  IP6 = 2
>>>> +L2VPN = 25
>>>> diff --git a/ryu/lib/packet/bgp.py b/ryu/lib/packet/bgp.py
>>>> index 99da867..697eff3 100644
>>>> --- a/ryu/lib/packet/bgp.py
>>>> +++ b/ryu/lib/packet/bgp.py
>>>> @@ -37,6 +37,7 @@ from ryu.lib.packet import safi as subaddr_family
>>>>  from ryu.lib.packet import packet_base
>>>>  from ryu.lib.packet import stream_parser
>>>>  from ryu.lib import addrconv
>>>> +from ryu.lib import type_desc
>>>>  from ryu.lib.pack_utils import msg_pack_into
>>>>
>>>>  reduce = six.moves.reduce
>>>> @@ -180,9 +181,7 @@ class _Value(object):
>>>>          args = []
>>>>          for f in self._VALUE_FIELDS:
>>>>              args.append(getattr(self, f))
>>>> -        buf = bytearray()
>>>> -        msg_pack_into(self._VALUE_PACK_STR, buf, 0, *args)
>>>> -        return buf
>>>> +        return struct.pack(self._VALUE_PACK_STR, *args)
>>>>
>>>>
>>>>  class _TypeDisp(object):
>>>
>>>> @@ -661,7 +662,7 @@ class _RouteDistinguisher(StringifyMixin, _TypeDisp, 
>>>> _Value):
>>>>          value = self.serialize_value()
>>>>          buf = bytearray()
>>>>          msg_pack_into(self._PACK_STR, buf, 0, self.type)
>>>> -        return buf + value
>>>> +        return six.binary_type(buf + value)
>>>>
>>>>      @property
>>>>      def formatted_str(self):
>>>> @@ -820,7 +821,7 @@ class _LabelledAddrPrefix(_AddrPrefix):
>>>>                        (label & 0xff0000) >> 16,
>>>>                        (label & 0x00ff00) >> 8,
>>>>                        (label & 0x0000ff) >> 0)
>>>> -        return buf
>>>> +        return six.binary_type(buf)
>>>>
>>>>      @classmethod
>>>>      def _label_from_bin(cls, label):
>>>> @@ -1030,6 +1031,651 @@ class 
>>>> LabelledVPNIP6AddrPrefix(_LabelledAddrPrefix, _VPNAdd
>>>
>>> IIRC, ryu packet library returns bytearray as serialize() outputs.
>>> Are we changing that?
>>
>> Sorry, I didn't know about it enough.
>> Do we have any document which describes the policy of that implementation?
>> Are there still more rule or policy I should consider?
>> The following just says that packet library return "the raw data to send" or
>> "sequence of bytes", NOT bytearray instance.
>>   http://ryu.readthedocs.io/en/latest/library_packet.html#building-packet
>>   
>> http://osrg.github.io/ryu-book/en/html/packet_lib.html#generation-of-packets-serialization
>
> I thought str.binary_type conversion was done when calling struct.pack
> but it turned out to be a false memory. Sorry.
>
> A bit unrelated, but ryu/lib/packet/icmpv6.py is an example where
> serialize returns binary_type.

Thanks.

Personally, I think...
packet.Packet.data should be a bytearray type after serialize(),
but it is not mentioned for the members of the other packet header class.
   
https://github.com/iwaseyusuke/ryu/blob/master/ryu/lib/packet/packet.py#L24-L36
   
https://github.com/iwaseyusuke/ryu/blob/master/ryu/lib/packet/packet_base.py#L22-L23

And, in most case for Ryu users, packet header classes (e.g. icmpv6.icmpv6,
bgp.BGPUpdate, and so on) are not directly serialized by users.
These are serialized through serializing process of packet.Packet class.
In some case, for example, subclasses of bgp.BGPMessage are serialize not
by packet.Packet class, but this is handled by BGPSpeaker's framework.

>
>
>> OTOH, I think the above is the "internal" API of packet library,
>> Ryu users does not need to use them directly.
>> And also, a bytearray instance cannot be an argument of struct.pack(),
>> so I fix to convert them into six.binary_type for the convenience.
>
> Yes. That was an internal API. I wasn't very careful when reading the
> patch.  Please disregard this comment.

OK, thank you for reviewing my patch.
The latest version of my patch is v2 which I posted yesterday.
   [PATCH v2 00/20] BGPSpeaker: Support MPLS-Based Ethernet VPN (RFC7432)

Thanks,
Iwase

>
>> OK, I will update my patch.
>>
>> Thank,
>> Iwase
>>
>>>
>>> --
>>> IWAMOTO Toshihiro
>>>
>>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Ryu-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/ryu-devel
>

------------------------------------------------------------------------------
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to