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

Reply via email to