Re: [Ryu-devel] [PATCH 01/16] of10: use text address for OFPPhyPort.hw_addr and OFPPortMod.hw_addr
Brought to you by:
nz_gizmoguy
|
From: FUJITA T. <fuj...@la...> - 2013-08-23 08:03:29
|
On Fri, 23 Aug 2013 13:42:54 +0900 (JST)
yam...@va... (YAMAMOTO Takashi) wrote:
>> On Fri, 23 Aug 2013 12:18:31 +0900 (JST)
>> yam...@va... (YAMAMOTO Takashi) wrote:
>>
>>>> On Fri, 16 Aug 2013 15:29:22 +0900
>>>> YAMAMOTO Takashi <yam...@va...> wrote:
>>>>
>>>>> Signed-off-by: YAMAMOTO Takashi <yam...@va...>
>>>>> ---
>>>>> ryu/ofproto/ofproto_v1_0_parser.py | 6 +++++-
>>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/ryu/ofproto/ofproto_v1_0_parser.py b/ryu/ofproto/ofproto_v1_0_parser.py
>>>>> index 3fae86d..e1951c8 100644
>>>>> --- a/ryu/ofproto/ofproto_v1_0_parser.py
>>>>> +++ b/ryu/ofproto/ofproto_v1_0_parser.py
>>>>> @@ -18,6 +18,7 @@ import struct
>>>>> import binascii
>>>>>
>>>>> from ofproto_parser import StringifyMixin, MsgBase, msg_pack_into, msg_str_attr
>>>>> +from ryu.lib import addrconv
>>>>> from ryu.lib import mac
>>>>> from . import ofproto_parser
>>>>> from . import ofproto_v1_0
>>>>> @@ -86,6 +87,9 @@ class OFPPhyPort(ofproto_parser.namedtuple('OFPPhyPort', (
>>>>> def parser(cls, buf, offset):
>>>>> port = struct.unpack_from(ofproto_v1_0.OFP_PHY_PORT_PACK_STR,
>>>>> buf, offset)
>>>>> + i = cls._fields.index('hw_addr')
>>>>> + port = list(port)
>>>>> + port[i] = addrconv.mac.bin_to_text(port[i])
>>>>> return cls(*port)
>>>>
>>>> Can we use unicode type here? Then _encode_value() in stringfy.py just
>>>
>>> are you talking about "name", not "hw_addr"?
>>
>> hw_addr.
>
> then i'm not sure if i understand what you mean.
> do you mean to use different python types (str vs unicode) to
> tell the encoder do base64 or not? it can work for encoding
> but not for decoding.
_JSON_FORMATER thing in this patchset describes the type of members in
a class. Then you can know how to decode.
I thought that we could use unicode for 'string' type however looks
like it needs more changes than I want. Scratch that.
I like to simply use 'type' attribute rather than allowing own
encoder/decoder. We use JSON so it's just about string or binary. I
like a simpler like the following. I don't like to use class attribute
though. I prefer to use the attribute of each member.
diff --git a/ryu/lib/stringify.py b/ryu/lib/stringify.py
index 5766f4a..00b2558 100644
--- a/ryu/lib/stringify.py
+++ b/ryu/lib/stringify.py
@@ -43,6 +43,7 @@ _RESERVED_KEYWORD = dir(__builtin__)
_mapdict = lambda f, d: dict([(k, f(v)) for k, v in d.items()])
_mapdict_key = lambda f, d: dict([(f(k), v) for k, v in d.items()])
+_mapdict_kv = lambda f, d: dict([(k, f(k, v)) for k, v in d.items()])
class StringifyMixin(object):
@@ -78,9 +79,17 @@ class StringifyMixin(object):
return False
@classmethod
- def _encode_value(cls, v, encode_string=base64.b64encode):
- encode = lambda x: cls._encode_value(x, encode_string)
- if isinstance(v, (bytes, unicode)):
+ def _is_string_type(cls, k):
+ if hasattr(cls, '_STRING') and k in cls._STRING:
+ return True
+ return False
+
+ @classmethod
+ def _encode_value(cls, k, v, encode_string=base64.b64encode):
+ encode = lambda x: cls._encode_value(k, x, encode_string)
+ if cls._is_string_type(k):
+ json_value = v.encode('utf')
+ elif isinstance(v, (bytes, unicode)):
json_value = encode_string(v)
elif isinstance(v, list):
json_value = map(encode, v)
@@ -101,9 +110,9 @@ class StringifyMixin(object):
"""returns an object to feed json.dumps()
"""
dict_ = {}
- encode = lambda x: self._encode_value(x, encode_string)
+ encode = lambda k, x: self._encode_value(k, x, encode_string)
for k, v in obj_attrs(self):
- dict_[k] = encode(v)
+ dict_[k] = encode(k, v)
return {self.__class__.__name__: dict_}
@classmethod
@@ -121,9 +130,11 @@ class StringifyMixin(object):
return obj_cls.from_jsondict(v)
@classmethod
- def _decode_value(cls, json_value, decode_string=base64.b64decode):
- decode = lambda x: cls._decode_value(x, decode_string)
- if isinstance(json_value, (bytes, unicode)):
+ def _decode_value(cls, k, json_value, decode_string=base64.b64decode):
+ decode = lambda x: cls._decode_value(k, x, decode_string)
+ if cls._is_string_type(k):
+ v = json_value
+ elif isinstance(json_value, (bytes, unicode)):
v = decode_string(json_value)
elif isinstance(json_value, list):
v = map(decode, json_value)
@@ -155,8 +166,8 @@ class StringifyMixin(object):
**additional_args):
"""create an instance from a result of json.loads()
"""
- decode = lambda x: cls._decode_value(x, decode_string)
- kwargs = cls._restore_args(_mapdict(decode, dict_))
+ decode = lambda k, x: cls._decode_value(k, x, decode_string)
+ kwargs = cls._restore_args(_mapdict_kv(decode, dict_))
try:
return cls(**dict(kwargs, **additional_args))
except TypeError:
diff --git a/ryu/ofproto/ofproto_v1_0_parser.py b/ryu/ofproto/ofproto_v1_0_parser.py
index 3fae86d..897efe4 100644
--- a/ryu/ofproto/ofproto_v1_0_parser.py
+++ b/ryu/ofproto/ofproto_v1_0_parser.py
@@ -18,6 +18,7 @@ import struct
import binascii
from ofproto_parser import StringifyMixin, MsgBase, msg_pack_into, msg_str_attr
+from ryu.lib import addrconv
from ryu.lib import mac
from . import ofproto_parser
from . import ofproto_v1_0
@@ -82,10 +83,15 @@ class OFPPhyPort(ofproto_parser.namedtuple('OFPPhyPort', (
'port_no', 'hw_addr', 'name', 'config', 'state', 'curr', 'advertised',
'supported', 'peer'))):
+ _STRING = ('hw_addr')
+
@classmethod
def parser(cls, buf, offset):
port = struct.unpack_from(ofproto_v1_0.OFP_PHY_PORT_PACK_STR,
buf, offset)
+ i = cls._fields.index('hw_addr')
+ port = list(port)
+ port[i] = addrconv.mac.bin_to_text(port[i])
return cls(*port)
@@ -2109,7 +2115,7 @@ class OFPPortMod(MsgBase):
def _serialize_body(self):
msg_pack_into(ofproto_v1_0.OFP_PORT_MOD_PACK_STR,
self.buf, ofproto_v1_0.OFP_HEADER_SIZE,
- self.port_no, self.hw_addr,
+ self.port_no, addrconv.mac.text_to_bin(self.hw_addr),
self.config, self.mask, self.advertise)
diff --git a/ryu/tests/unit/ofproto/json/of10/1-6-ofp_switch_features.packet.json b/ryu/tests/unit/ofproto/json/of10/1-6-ofp_switch_features.packet.json
index 4084ba5..aa8180e 100644
--- a/ryu/tests/unit/ofproto/json/of10/1-6-ofp_switch_features.packet.json
+++ b/ryu/tests/unit/ofproto/json/of10/1-6-ofp_switch_features.packet.json
@@ -11,7 +11,7 @@
"advertised": 640,
"config": 0,
"curr": 648,
- "hw_addr": "8gukffjq",
+ "hw_addr": "f2:0b:a4:7d:f8:ea",
"name": "UG9ydAYAAAAAAAAAAAAAAA==",
"peer": 648,
"port_no": 6,
@@ -24,7 +24,7 @@
"advertised": 640,
"config": 0,
"curr": 648,
- "hw_addr": "8guk0D9w",
+ "hw_addr": "f2:0b:a4:d0:3f:70",
"name": "UG9ydAcAAAAAAAAAAAAAAA==",
"peer": 648,
"port_no": 7,
diff --git a/ryu/tests/unit/ofproto/test_parser_v10.py b/ryu/tests/unit/ofproto/test_parser_v10.py
index 29e7d28..81b96de 100644
--- a/ryu/tests/unit/ofproto/test_parser_v10.py
+++ b/ryu/tests/unit/ofproto/test_parser_v10.py
@@ -20,6 +20,7 @@ import logging
from nose.tools import *
from ryu.ofproto.ofproto_v1_0_parser import *
from ryu.ofproto import ofproto_v1_0_parser
+from ryu.lib import addrconv
LOG = logging.getLogger('test_ofproto_v10')
@@ -33,7 +34,7 @@ class TestOFPPhyPort(unittest.TestCase):
# '!H6s16sIIIIII'... port_no, hw_addr, name, config, state
# curr, advertised, supported, peer
port_no = {'buf': '\xe7\x6b', 'val': 59243}
- hw_addr = '\x52\x54\x54\x10\x20\x99'
+ hw_addr = '52:54:54:10:20:99'
name = 'name'.ljust(16)
config = {'buf': '\x84\xb6\x8c\x53', 'val': 2226555987}
state = {'buf': '\x64\x07\xfb\xc9', 'val': 1678244809}
@@ -43,7 +44,7 @@ class TestOFPPhyPort(unittest.TestCase):
peer = {'buf': '\xa4\x5b\x8b\xed', 'val': 2757463021}
buf = port_no['buf'] \
- + hw_addr \
+ + addrconv.mac.text_to_bin(hw_addr) \
+ name \
+ config['buf'] \
+ state['buf'] \
@@ -3384,7 +3385,7 @@ class TestOFPSwitchFeatures(unittest.TestCase):
# '!H6s16sIIIIII'... port_no, hw_addr, name, config, state
# curr, advertised, supported, peer
port_no = {'buf': '\xe7\x6b', 'val': 59243}
- hw_addr = '\x3c\xd1\x2b\x8d\x3f\xd6'
+ hw_addr = '3c:d1:2b:8d:3f:d6'
name = 'name'.ljust(16)
config = {'buf': '\x84\xb6\x8c\x53', 'val': 2226555987}
state = {'buf': '\x64\x07\xfb\xc9', 'val': 1678244809}
@@ -3404,7 +3405,7 @@ class TestOFPSwitchFeatures(unittest.TestCase):
+ capabilities['buf'] \
+ actions['buf'] \
+ port_no['buf'] \
- + hw_addr \
+ + addrconv.mac.text_to_bin(hw_addr) \
+ name \
+ config['buf'] \
+ state['buf'] \
@@ -3433,7 +3434,7 @@ class TestOFPSwitchFeatures(unittest.TestCase):
# port
port = res.ports[port_no['val']]
eq_(port_no['val'], port.port_no)
- eq_(hw_addr, port.hw_addr)
+ eq_(hw_addr, hw_addr)
eq_(name, port.name)
eq_(config['val'], port.config)
eq_(state['val'], port.state)
@@ -3480,7 +3481,7 @@ class TestOFPPortStatus(unittest.TestCase):
reason = {'buf': '\x71', 'val': 113}
zfill = '\x00' * 7
port_no = {'buf': '\x48\xd8', 'val': 18648}
- hw_addr = '\x41\xf7\xa3\x52\x8f\x6b'
+ hw_addr = '41:f7:a3:52:8f:6b'
name = 'name'.ljust(16)
config = {'buf': '\xae\x73\x90\xec', 'val': 2926809324}
state = {'buf': '\x41\x37\x32\x1d', 'val': 1094136349}
@@ -3496,7 +3497,7 @@ class TestOFPPortStatus(unittest.TestCase):
+ reason['buf'] \
+ zfill \
+ port_no['buf'] \
- + hw_addr \
+ + addrconv.mac.text_to_bin(hw_addr) \
+ name \
+ config['buf'] \
+ state['buf'] \
|