On Thu, 21 Jul 2016 15:56:07 +0900 Shinpei Muraoka <shinpei.mura...@gmail.com> wrote:
> Hi Fujita-San > >> The patch can't be cleanly reverted. So let's forget this point. > > I got it. > I will include it in the patch. > > >> Keeping the jsondict outputs is nice but it's optional for me. > > I think the simple source is good. > I modify the format to be set the zone_src a string. > If you want to set the immediate value for zone, > zone_src set the None or empty character string. > > >> As Iwamoto-san pointed out, this patch doesn't recover the old API: >> >> https://github.com/osrg/ryu/blob/d090b291bee5a8e1883cb4a75b9045b2703cdba8/ryu/ofproto/nx_actions.py#L215 >> >> Can you please fix that? > > NXActionRegLoad was present in the nx_actions.py and > ofproto_v1_0_parser.py originally. > The nx_actions.py had ofs and nbits in the argument. > The ofproto_v1_0_parser.py had ofs_nbits in the argument. > It is common to set the values directly to ofs_nbits in nx_actions.py. > Now it is in the form of ofproto_v1_0_parser.py. > I think the current form is correct, > what you do think? I think that the point is we keep the exact same API. So strictly speaking, we have to have ofs_nbits argument for RegLoad with OF1.0 and ofs and nbits with other OF versions. Right? The safer option is adding a hack for accepting both ofs_nbits/ofs and nbits. But after checking the dragonflow code, RegLoad is not used. So we might be able to have ofs_nbits argument for both if it's more consistent with the rests. Iwamoto-san, what's your opinion? > > Thanks, > > On 2016年07月21日 11:15, FUJITA Tomonori wrote: >> On Thu, 21 Jul 2016 10:19:07 +0900 >> Shinpei Muraoka <shinpei.mura...@gmail.com> wrote: >> >>> > I think it's better to separate this into the pure revert patch and >>> > the other changes in order to make git history more readable. >>> >>> Separate this into the revert patch and the other changes. >>> However, revert patch has an impact on the document. >>> Therefore, document fixes and method additions include in revert >>> patch. >> >> The patch can't be cleanly reverted. So let's forget this point. >> >> >>> > Sorry for the crappy API, but I think we need a nice warning at least >>> > if we change jsondict outputs of NXActionCT.parser. >>> >>> Write the following as caution. >>> If the value of zone_src is other than zero, >>> there is case that value of the zone_src is set by the parser as a >>> string. >> >> Keeping the jsondict outputs is nice but it's optional for me. >> >> >>>>> diff --git a/ryu/ofproto/nx_actions.py b/ryu/ofproto/nx_actions.py >>>>> index 94c2213..6bd9b55 100644 >>>>> --- a/ryu/ofproto/nx_actions.py >>>>> +++ b/ryu/ofproto/nx_actions.py >>>>> @@ -15,6 +15,7 @@ >>>>> # limitations under the License. >>>>> >>>>> import six >>>>> +import base64 >>>>> >>>>> import struct >>>>> >>>>> @@ -356,18 +357,18 @@ def generate(ofp_name, ofpp_name): >>>>> ================ >>>>> ====================================================== >>>>> Attribute Description >>>>> ================ >>>>> ====================================================== >>>>> - start Start bit for destination field >>>>> - end End bit for destination field >>>>> + ofs_nbits Start and End for the OXM/NXM field. >>>>> + Setting method refer to the ``nicira_ext.ofs_nbits`` >>>>> dst OXM/NXM header for destination field >>>>> value OXM/NXM value to be loaded >>>>> ================ >>>>> ====================================================== >>>>> >>>>> Example:: >>>>> >>>>> - actions += [parser.NXActionRegLoad(start=4, >>>>> - end=31, >>>>> - dst="eth_dst", >>>>> - value=0x112233)] >>>>> + actions += [parser.NXActionRegLoad( >>>>> + ofs_nbits=nicira_ext.ofs_nbits(4, 31), >>>>> + dst="eth_dst", >>>>> + value=0x112233)] >>>>> """ >>>>> _subtype = nicira_ext.NXAST_REG_LOAD >>>>> _fmt_str = '!HIQ' # ofs_nbits, dst, value >>>>> @@ -377,11 +378,10 @@ def generate(ofp_name, ofpp_name): >>>>> ] >>>>> } >>>>> >>>>> - def __init__(self, start, end, dst, value, >>>>> + def __init__(self, ofs_nbits, dst, value, >>>> >>>> For some reason, it was ofs and nbits for NXActionRegLoad. >>>> Please retain the old API. >> >> As Iwamoto-san pointed out, this patch doesn't recover the old API: >> >> https://github.com/osrg/ryu/blob/d090b291bee5a8e1883cb4a75b9045b2703cdba8/ryu/ofproto/nx_actions.py#L215 >> >> Can you please fix that? >> >> Thanks! >> ------------------------------------------------------------------------------ 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
