On 1/6/26 12:47 PM, Ilya Maximets wrote:
> On 1/6/26 11:32 AM, Dumitru Ceara wrote:
>> On 1/5/26 4:51 PM, Ilya Maximets wrote:
>>>>>> diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
>>>>>> index 5c57ab08ff18..c996cddad341 100644
>>>>>> --- a/lib/meta-flow.xml
>>>>>> +++ b/lib/meta-flow.xml
>>>>>> @@ -2905,7 +2905,8 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
>>>>>>        This is the first of several Open vSwitch registers, all of which 
>>>>>> have
>>>>>>        the same properties.  Open vSwitch 1.1 introduced registers 0, 1, 
>>>>>> 2, and
>>>>>>        3, version 1.3 added register 4, version 1.7 added registers 5, 
>>>>>> 6, and 7,
>>>>>> -      and version 2.6 added registers 8 through 15.
>>>>>> +      version 2.6 added registers 8 through 15 and version 3.7 added 
>>>>>> registers
>>>>>> +      16 through 31.
>>>>>>      </field>
>>>>>>      <!-- XXX series -->
>>>>>>      <field id="MFF_REG1" title="Register 1" hidden="yes"/>
>>>>>> @@ -2923,6 +2924,22 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
>>>>>>      <field id="MFF_REG13" title="Register 13" hidden="yes"/>
>>>>>>      <field id="MFF_REG14" title="Register 14" hidden="yes"/>
>>>>>>      <field id="MFF_REG15" title="Register 15" hidden="yes"/>
>>>>>> +    <field id="MFF_REG16" title="Register 16" hidden="yes"/>
>>>>>> +    <field id="MFF_REG17" title="Register 17" hidden="yes"/>
>>>>>> +    <field id="MFF_REG18" title="Register 18" hidden="yes"/>
>>>>>> +    <field id="MFF_REG19" title="Register 19" hidden="yes"/>
>>>>>> +    <field id="MFF_REG20" title="Register 20" hidden="yes"/>
>>>>>> +    <field id="MFF_REG21" title="Register 21" hidden="yes"/>
>>>>>> +    <field id="MFF_REG22" title="Register 22" hidden="yes"/>
>>>>>> +    <field id="MFF_REG23" title="Register 23" hidden="yes"/>
>>>>>> +    <field id="MFF_REG24" title="Register 24" hidden="yes"/>
>>>>>> +    <field id="MFF_REG25" title="Register 25" hidden="yes"/>
>>>>>> +    <field id="MFF_REG26" title="Register 26" hidden="yes"/>
>>>>>> +    <field id="MFF_REG27" title="Register 27" hidden="yes"/>
>>>>>> +    <field id="MFF_REG28" title="Register 28" hidden="yes"/>
>>>>>> +    <field id="MFF_REG29" title="Register 29" hidden="yes"/>
>>>>>> +    <field id="MFF_REG30" title="Register 30" hidden="yes"/>
>>>>>> +    <field id="MFF_REG31" title="Register 31" hidden="yes"/>
>>>>> The ovs-fields man page shows the REG0 as an exmaple, we probably need to
>>>>> mention that new registers have a different class somehow, either by 
>>>>> showing
>>>>> another example, or at least just stating what is the other class and for
>>>>> which registers it is getting used.  Same for the xxregs.
>>>>>
>>>> Would something like this work for you (I added a line to each type of
>>>> register, including XREGs and XXREGs):
>>>>
>>>> diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
>>>> index c996cddad341..ca6394b0685f 100644
>>>> --- a/lib/meta-flow.xml
>>>> +++ b/lib/meta-flow.xml
>>>> @@ -2902,11 +2902,19 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
>>>>      </field>
>>>>  
>>>>      <field id="MFF_REG0" title="Register 0">
>>>> -      This is the first of several Open vSwitch registers, all of which 
>>>> have
>>>> -      the same properties.  Open vSwitch 1.1 introduced registers 0, 1, 
>>>> 2, and
>>>> -      3, version 1.3 added register 4, version 1.7 added registers 5, 6, 
>>>> and 7,
>>>> -      version 2.6 added registers 8 through 15 and version 3.7 added 
>>>> registers
>>>> -      16 through 31.
>>>> +      <p>
>>>> +        This is the first of several Open vSwitch registers, all of which 
>>>> have
>>>> +        the same properties.  Open vSwitch 1.1 introduced registers 0, 1, 
>>>> 2,
>>>> +        and 3, version 1.3 added register 4, version 1.7 added registers 
>>>> 5, 6,
>>>> +        and 7, version 2.6 added registers 8 through 15 and version 3.7 
>>>> added
>>> nit: Missed the last time, but the oxford comma before the final 'and' seems
>>> like something we should preserve.
>>>
>>
>> I didn't even notice the comma it before you mentioned it. :)
>>
>> I'll re-add it and I'll also use it in the extended registers section.
>>
>>>> +        registers 16 through 31.
>>>> +      </p>
>>>> +
>>>> +      <p>
>>>> +        Registers 0 through 15 are defined using OXM the
>>> "using OXM the" ?  Sounds strange.
>>>
>>>> +        <code>OFPXMC_NXM_1</code> class while registers 16 through 31 are
>>>> +        defined using the OXM <code>OFPXMC_EXPERIMENTER</code> class.
>>> I think, it may be less confusing if we talk here in terms of code point
>>> prefixes instead of OXM classes.  Since we document OXM/NXM code points
>>> here and 32-bit registers do not have OXM code points, mentioning the OXM
>>> class may be hard to follow.  Also, OFPXMC_EXPERIMENTER doesn't specify
>>> which experimenter class is being used.
>>>
>>> Maybe something like "Code points for registers 0 through 15 are defined
>>> within NXM_NX code point prefix, while regitsters 16 through 31 are
>>> defined within NXOXM_ET, due to limited space within the NXM_NX class."
> 
> One more thing here is maybe good to point out the first NXOXM_ET version
> including the number inside the class, since they do not start with a zero,
> e.g.: "... while registers 16 through 31 are defined within NXOXM_ET
> starting with NXOXM_ET_REG16 (17), due to ..."
> The docs seem a little more complete this way.  Not sure.
> 

Ack, I incorporated this suggestion too, took care of all the other
comments you had and posted v2:

https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/

I'll have to be honest and say that updating the ovs-ofctl.at tests was
not a very pleasant experience. :)  I'm not sure if I missed anything
there, hopefully now we cover all the things you had in mind.

Regards,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to