At Tue, 26 Jul 2016 16:25:15 +0900, Shinpei Muraoka wrote: > > 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.
Ok. I see your point. > > 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' > This kind of change will be eventually needed, but if ryu suddenly stops to allow numeric zone_src, we'll need to coordinate with dragonflow people and schedule global-requirements change to avoid their bad experience. > 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
