[ovs-dev] [PATCH] ofp-monitor: Extend Flow Monitoring support for OF 1.0+ with Nicira Extensions

[Vasu Dasari]

Hi Ben,

Quick comment on test case extension. I have added wrappers as you
suggested that code looks good. But I ran into a version specific problem
for testcase, "ofproto - flow monitoring". It uses is a ofctl/send
<https://github.com/openvswitch/ovs/blob/7100c220e669443aa646513ce6cb241ccf2caf5c/tests/ofproto.at#L4695>
command
to perform flow-delete operation, which works well for 1.0 as the byte
stream is handcrafted for version 1. But for any other version, we need to
construct th byte stream and call ofctl/send. So, I left following as it is
   4579:AT_SETUP([ofproto - flow monitoring])
   4716:AT_SETUP([ofproto - flow monitoring with !own])

but enhanced these cases to handle all supported versions.
  4757:AT_SETUP([ofproto - flow monitoring with out_port])
   4809:AT_SETUP([ofproto - flow monitoring pause and resume])

Here is the latest latest patch
<http://patchwork.ozlabs.org/project/openvswitch/patch/20210514193917.81656-1-vdasari@gmail.com/>.
Pardon me, I forgot to add v1 to the subject line but I have added the v1
changes in the patch.

Thanks
-Vasu

*Vasu Dasari*


On Mon, May 10, 2021 at 8:22 PM Vasu Dasari <vdasari at gmail.com> wrote:

> Ok. Thanks Ben. Will take care of this.
>
> -Vasu
>
> *Vasu Dasari*
>
>
> On Mon, May 10, 2021 at 7:57 PM Ben Pfaff <blp at ovn.org> wrote:
>
>> On Mon, May 10, 2021 at 07:38:08PM -0400, Vasu Dasari wrote:
>> > 1. Adding Nicira extensions for Flow Monitoring was pretty
>> straightforward
>> > with the existing code base. As you might have seen, I just had to tweak
>> > some functions to get this to work. I do not have any hard opinions
>> against
>> > your point that we should not duplicate functionality with Nicira
>> > extensions and OpenFlow 1.4,1.5. But my only suggestion is to make the
>> > support for OF 1.4, 1.5 flow monitoring messages to be implemented as a
>> > separate commit, as I think it is going to add new functions to make
>> that
>> > happen.
>>
>> I agree.  For this commit, though, I think that it should only add
>> support up to 1.3, that is, ofp-msgs.h should say 1.0-1.3, not 1.0+.
>>
>> Thanks,
>>
>> Ben.
>>
>