On Tue, 26 Jul 2016 17:05:52 +0900 IWAMOTO Toshihiro <iwam...@valinux.co.jp> wrote:
> 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. The whole point is that we can't ask others to change their code for us randomly. So we have to preserve the previous API. With the current path, still Dragonflow developers need to change their code? If so, Muraoka-san, please fix the patch. If not, I'll push this patch. ------------------------------------------------------------------------------ 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
