[ovs-dev] [PATCH 1/3] megaflow: Add mask attribute to netlink protocol

[Jesse Gross]

I mostly meant the kernel code since that's what I've been looking at
but would be good to be consistent throughout the code base. I think
there are just a few instances of the term in the userspace code and
if we think the term in not obvious then it might be better to just
use "wildcard" or some variation.

On Thu, Jun 6, 2013 at 4:50 PM, Andy Zhou <azhou at nicira.com> wrote:
> Thanks, I will address this point in V2 patch.
>
> I agree with you to remove it from the other locations, but want to make
> sure you meant within the scope of kernel code, right?
>
> Andy
>
>
> On Thu, Jun 6, 2013 at 4:41 PM, Jesse Gross <jesse at nicira.com> wrote:
>>
>> It looks good - I might also add that if the mask attribute is not
>> specified at all then the flow is exact match. I agree that it is
>> probably clearer to not use the term "mega flow" here but we should
>> then also remove it from the other locations in the code as well.
>>
>> On Thu, Jun 6, 2013 at 3:15 PM, Andy Zhou <azhou at nicira.com> wrote:
>> > How about the following change? I choose not to use the term "mega flow"
>> > here to avoid confusing outside people.
>> >
>> > diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
>> > index 6e71b4c..203c7eb 100644
>> > --- a/include/linux/openvswitch.h
>> > +++ b/include/linux/openvswitch.h
>> > @@ -410,9 +410,11 @@ struct ovs_key_nd {
>> >
>> >   * @OVS_FLOW_ATTR_CLEAR: If present in a %OVS_FLOW_CMD_SET request,
>> > clears
>> > the
>> >   * last-used time, accumulated TCP flags, and statistics for this flow.
>> >   * Otherwise ignored in requests.  Never present in notifications.
>> > - * @OVS_FLOW_ATTR_MASK: Optional Nested %OVS_KEY_ATTR_* attributes
>> > specifying
>> > - * the mega flow mask. OVS_KEY_ATTR_* attributes specified only in
>> > OVS_FLOW_ATT
>> > - * but not here will be treated as wildcard field.
>> > + * @OVS_FLOW_ATTR_MASK: Nested %OVS_KEY_ATTR_* attributes specifying
>> > the
>> > + * mask bits for wildcarded flow match. Mask bit value '1' specifies
>> > exact
>> > + * match with corresponding flow key bit, while mask bit value '0'
>> > specifies
>> > + * a wildcarded match. Omitting attribute is treated as wildcarding
>> > + * all corresponding fields. Optional for all requests.
>> >
>> >
>> > On Thu, Jun 6, 2013 at 10:46 AM, Jesse Gross <jesse at nicira.com> wrote:
>> >>
>> >> On Wed, Jun 5, 2013 at 10:46 PM, Andy Zhou <azhou at nicira.com> wrote:
>> >> > diff --git a/include/linux/openvswitch.h
>> >> > b/include/linux/openvswitch.h
>> >> > index e890fd8..6e71b4c 100644
>> >> > --- a/include/linux/openvswitch.h
>> >> > +++ b/include/linux/openvswitch.h
>> >> > @@ -410,6 +410,9 @@ struct ovs_key_nd {
>> >> >   * @OVS_FLOW_ATTR_CLEAR: If present in a %OVS_FLOW_CMD_SET request,
>> >> > clears the
>> >> >   * last-used time, accumulated TCP flags, and statistics for this
>> >> > flow.
>> >> >   * Otherwise ignored in requests.  Never present in notifications.
>> >> > + * @OVS_FLOW_ATTR_MASK: Optional Nested %OVS_KEY_ATTR_* attributes
>> >> > specifying
>> >> > + * the mega flow mask. OVS_KEY_ATTR_* attributes specified only in
>> >> > OVS_FLOW_ATTR_KEY
>> >> > + * but not here will be treated as wildcard field.
>> >>
>> >> Can you expand this comment a little bit? Outside of people that have
>> >> been working on it, I'm not sure that the term mega flow is really
>> >> self-descriptive.
>> >
>> >
>
>