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.

>>
>> What do you think?
>>
> 
> Way better.
> 
>>
>>> +      </p>
>>>      </field>
>>>      <!-- XXX series -->
>>>      <field id="MFF_REG1" title="Register 1" hidden="yes"/>
>>> @@ -2952,6 +2960,11 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
>>>          2.6 and later) or 16 (in version 3.7 and later).
>>>        </p>
>>>  
>>> +      <p>
>>> +        The registers are defined using OXM the <code>OFPXMC_NXM_1</code>
>> This should be OFPXMC_PACKET_REGS instead.  But anyways, the comment
>> above applies here as well.  Maybe something like:
>>
>> "Code points for all of the extended registers are defined within
>> OXM_OF_PKT_REG code point prefix."
>>
>>> +        class.
>>> +      </p>
>>> +
>>>        <p>
>>>          Each of the 64-bit extended registers overlays two of the 32-bit
>>>          registers: <code>xreg0</code> overlays <code>reg0</code> and
>>> @@ -2999,6 +3012,12 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
>>>          vSwitch 4 128-bit registers (in versions 2.6 and later) and 8
>>>          (in version 3.7 and later).
>>>        </p>
>>> +
>>> +      <p>
>>> +        Registers 0 through 3 are defined using OXM the
>>> +        <code>OFPXMC_NXM_1</code> class while registers 4 through 7 are
>>> +        defined using the OXM <code>OFPXMC_EXPERIMENTER</code> class.
>>> +      </p>
>> "Code points for double-extended registers 0 through 3 ..."
> 
> Cool, I'll update the wording in v2.
> 
> Thanks,
> Dumitru
> 

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

Reply via email to