Hi Iwamoto-San
On 2016年07月26日 13:55, IWAMOTO Toshihiro wrote: > Thanks for cleaning up API incoherency. > > At Fri, 22 Jul 2016 13:59:49 +0900, > Shinpei Muraoka wrote: >> >> Since zone_src in NXActionCT was not possible to specify string, >> update zone_src of NXActionCT for the uniformity. >> Therefore, you will be able to specify string of OXM/NXM fields for zone_src. > > dragonflow is using number for zone_src now. > Please consider allowing number for one release. > >> If you want to set the immediate value for zone, >> zone_src set the None or empty character string. > > I don't think allowing an empty string here is a good idea. > > Using the predicate > > if zone_src is None: > > or > > if zone_src is None or zone_src == 0: > > (for backward compatibility) should be fine. > I changed zone_src to string to keep to_jsondict method simple. If you do not use jsondict, there is no problem by specifying a zero to zone_src. However, if you want to use parser, zone_src will be returned as empty string. dragonflow does not seem to use to_jsondict and parser. For this reason, I think that there is no problem with the current implementation. The current zone_src can not allow a number other than zero. How about correcting the following constant of dragonflow? https://github.com/openstack/dragonflow/blob/master/dragonflow/controller/common/constants.py#L57-L59 $ git diff diff --git a/dragonflow/controller/common/constants.py b/dragonflow/controller/common/constants.py index 106a1bf..796bf4b 100644 --- a/dragonflow/controller/common/constants.py +++ b/dragonflow/controller/common/constants.py @@ -55,8 +55,8 @@ CT_STATE_TRK = 0x20 CT_FLAG_COMMIT = 1 # Register match -METADATA_REG = 0x80000408 -CT_ZONE_REG = 0x1d402 +METADATA_REG = 'metadata' +CT_ZONE_REG = 'ct_zone' Thanks, > >> Signed-off-by: Shinpei Muraoka <shinpei.mura...@gmail.com> >> --- >> ryu/ofproto/nx_actions.py | 34 >> +++++++++++++++++---- >> .../of13/ovs-ofctl-of13-action_ct.packet | Bin 104 -> 104 bytes >> ryu/tests/packet_data_generator3/gen.py | 2 +- >> .../json/of13/ovs-ofctl-of13-action_ct.packet.json | 4 +-- >> .../of13/ovs-ofctl-of13-action_ct_exec.packet.json | 2 +- >> .../of13/ovs-ofctl-of13-action_ct_nat.packet.json | 2 +- >> .../ovs-ofctl-of13-action_ct_nat_v6.packet.json | 2 +- >> 7 files changed, 34 insertions(+), 12 deletions(-) >> >> diff --git a/ryu/ofproto/nx_actions.py b/ryu/ofproto/nx_actions.py >> index 9c97123..dbbdbf7 100644 >> --- a/ryu/ofproto/nx_actions.py >> +++ b/ryu/ofproto/nx_actions.py >> @@ -2658,7 +2658,7 @@ def generate(ofp_name, ofpp_name): >> zone_ofs_nbits Start and End for the OXM/NXM field. >> Setting method refer to the >> ``nicira_ext.ofs_nbits``. >> If you need set the Immediate value for zone, >> - zone_src must be set to zero. >> + zone_src must be set to None or empty character >> string. >> recirc_table Recirculate to a specific table >> alg Well-known port number for the protocol >> actions Zero or more actions may immediately follow this >> @@ -2670,8 +2670,8 @@ def generate(ofp_name, ofpp_name): >> match = parser.OFPMatch(eth_type=0x0800, ct_state=(0,32)) >> actions += [parser.NXActionCT( >> flags = 1, >> - zone_src = 0, >> - zone_ofs_nbits = 0, >> + zone_src = "reg0", >> + zone_ofs_nbits = nicira_ext.ofs_nbits(4, 31), >> recirc_table = 4, >> alg = 0, >> actions = [])] >> @@ -2680,7 +2680,13 @@ def generate(ofp_name, ofpp_name): >> >> # flags, zone_src, zone_ofs_nbits, recirc_table, >> # pad, alg >> - _fmt_str = '!HIHB3xH' >> + _fmt_str = '!H4sHB3xH' >> + _TYPE = { >> + 'ascii': [ >> + 'zone_src', >> + ] >> + } >> + >> # Followed by actions >> >> def __init__(self, >> @@ -2702,12 +2708,20 @@ def generate(ofp_name, ofpp_name): >> @classmethod >> def parser(cls, buf): >> (flags, >> - zone_src, >> + oxm_data, >> zone_ofs_nbits, >> recirc_table, >> alg,) = struct.unpack_from( >> cls._fmt_str, buf, 0) >> rest = buf[struct.calcsize(cls._fmt_str):] >> + >> + # OXM/NXM field >> + if oxm_data == b'\x00' * 4: >> + zone_src = "" >> + else: >> + (n, len_) = ofp.oxm_parse_header(oxm_data, 0) >> + zone_src = ofp.oxm_to_user_header(n) >> + >> # actions >> actions = [] >> while len(rest) > 0: >> @@ -2720,9 +2734,17 @@ def generate(ofp_name, ofpp_name): >> >> def serialize_body(self): >> data = bytearray() >> + # If zone_src is zero, zone_ofs_nbits is zone_imm >> + if not self.zone_src: >> + zone_src = b'\x00' * 4 >> + else: >> + zone_src = bytearray() >> + oxm = ofp.oxm_from_user_header(self.zone_src) >> + ofp.oxm_serialize_header(oxm, zone_src, 0) >> + >> msg_pack_into(self._fmt_str, data, 0, >> self.flags, >> - self.zone_src, >> + six.binary_type(zone_src), >> self.zone_ofs_nbits, >> self.recirc_table, >> self.alg) >> diff --git a/ryu/tests/packet_data/of13/ovs-ofctl-of13-action_ct.packet >> b/ryu/tests/packet_data/of13/ovs-ofctl-of13-action_ct.packet >> index >> 3aff2bdc3e516a200a34604ce6236b10b247cd03..5eb733f969fbabe7d5d88ec279422b3d5fd286d0 >> 100644 >> GIT binary patch >> delta 16 >> Ucmd1Em=Md&$iTuV&B6cz02<K&?f?J) >> >> delta 16 >> Rcmd1Em=Md&00Jyv5&#+90nGpa >> >> diff --git a/ryu/tests/packet_data_generator3/gen.py >> b/ryu/tests/packet_data_generator3/gen.py >> index 65f91a7..92676e4 100644 >> --- a/ryu/tests/packet_data_generator3/gen.py >> +++ b/ryu/tests/packet_data_generator3/gen.py >> @@ -108,7 +108,7 @@ MESSAGES = [ >> 'args': (['table=3,', >> 'importance=39032'] + >> ['dl_type=0x0800,ct_state=-trk'] + >> - ['actions=ct(table=4)'])}, >> + ['actions=ct(table=4,zone=NXM_NX_REG0[4..31])'])}, >> {'name': 'action_ct_exec', >> 'versions': [4], >> 'cmd': 'add-flow', >> diff --git >> a/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct.packet.json >> b/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct.packet.json >> index 0cd1e20..b1157e9 100644 >> --- a/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct.packet.json >> +++ b/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct.packet.json >> @@ -21,8 +21,8 @@ >> "recirc_table": 4, >> "subtype": 35, >> "type": 65535, >> - "zone_ofs_nbits": 0, >> - "zone_src": 0 >> + "zone_ofs_nbits": 283, >> + "zone_src": "reg0" >> } >> } >> ], >> diff --git >> a/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_exec.packet.json >> b/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_exec.packet.json >> index a163aba..e3fcd3b 100644 >> --- >> a/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_exec.packet.json >> +++ >> b/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_exec.packet.json >> @@ -36,7 +36,7 @@ >> "subtype": 35, >> "type": 65535, >> "zone_ofs_nbits": 0, >> - "zone_src": 0 >> + "zone_src": "" >> } >> } >> ], >> diff --git >> a/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_nat.packet.json >> b/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_nat.packet.json >> index ef6a7d9..5a38d8f 100644 >> --- >> a/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_nat.packet.json >> +++ >> b/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_nat.packet.json >> @@ -38,7 +38,7 @@ >> "subtype": 35, >> "type": 65535, >> "zone_ofs_nbits": 0, >> - "zone_src": 0 >> + "zone_src": "" >> } >> } >> ], >> diff --git >> a/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_nat_v6.packet.json >> >> b/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_nat_v6.packet.json >> index cca4625..5c1c8c0 100644 >> --- >> a/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_nat_v6.packet.json >> +++ >> b/ryu/tests/unit/ofproto/json/of13/ovs-ofctl-of13-action_ct_nat_v6.packet.json >> @@ -38,7 +38,7 @@ >> "subtype": 35, >> "type": 65535, >> "zone_ofs_nbits": 0, >> - "zone_src": 0 >> + "zone_src": "" >> } >> } >> ], >> -- >> 1.9.1 >> >> >> ------------------------------------------------------------------------------ >> What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic >> patterns at an interface-level. Reveals which users, apps, and protocols are >> consuming the most bandwidth. Provides multi-vendor support for NetFlow, >> J-Flow, sFlow and other flows. Make informed decisions using capacity >> planning >> reports.http://sdm.link/zohodev2dev >> _______________________________________________ >> Ryu-devel mailing list >> Ryu-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/ryu-devel >> ------------------------------------------------------------------------------ What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports.http://sdm.link/zohodev2dev _______________________________________________ Ryu-devel mailing list Ryu-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ryu-devel
