Re: [ovs-dev] Help to Modify OVS Source Code

[Ashish Varma]

You can look at the "handle_packet_out" function at "ofproto.c" which
handles the "PACKET_OUT" message from the controller. (where you can check
for the LLDP eth type in the packet)



On Tue, Sep 26, 2017 at 9:56 AM, Mohammed Kamel  wrote:

> Hello,
>
> I am new at mining and OVS. I need help with modifying the OVS source code
> to make the following: I want to print a message on the counsel each time
> the OVS switch receive a LLDP packet from the controller. and how to use
> this modified code in the mininet?? I really need your help with that ASAP.
>
> Thank you,
> Mohammed
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] netdev, dpif: fix the crash/assert on port delete

[Ashish Varma]

Tested the patch with the steps causing the crash.
No problem detected. Also ran make check with no issues.

On Mon, Nov 13, 2017 at 2:06 PM, Ben Pfaff <b...@ovn.org> wrote:

> I sent a patch series that should help a little.  Would you mind testing
> it against the case that was causing problems?  The series is posted
> here:
> https://patchwork.ozlabs.org/project/openvswitch/list/?series=13360
>
> On Mon, Nov 13, 2017 at 12:36:06PM -0800, Ashish Varma wrote:
> > Thanks Ben.
> >
> > On Mon, Nov 13, 2017 at 11:36 AM, Ben Pfaff <b...@ovn.org> wrote:
> >
> > > On Mon, Nov 06, 2017 at 12:17:45PM -0800, Ashish Varma wrote:
> > > > a crash is seen in "netdev_ports_remove" when an interface is deleted
> > > and added
> > > > back in the system and when the interface is part of a bridge
> > > configuration.
> > > > e.g. steps:
> > > >   create a tap0 interface using "ip tuntap add.."
> > > >   add the tap0 interface to br0 using "ovs-vsctl add-port.."
> > > >   delete the tap0 interface from system using "ip tuntap del.."
> > > >   add the tap0 interface back in system using "ip tuntap add.."
> > > >(this changes the ifindex of the interface)
> > > >   delete tap0 from br0 using "ovs-vsctl del-port.."
> > > >
> > > > In the function "netdev_ports_insert", two hmap entries were created
> for
> > > > mapping "portnum -> netdev" and "ifindex -> portnum".
> > > > When the interface is deleted from the system, the
> "netdev_ports_remove"
> > > > function is not getting called and the old ifindex entry is not
> getting
> > > > cleaned up from the "ifindex_to_port" hmap.
> > > >
> > > > As part of the fix, added function "dpif_port_remove" which will call
> > > > "netdev_ports_remove" in the path where the interface deletion from
> the
> > > system
> > > > is detected.
> > > > Also, in "netdev_ports_remove", added the code where the
> > > "ifindex_to_port_data"
> > > > (ifindex -> portnum map node) is getting freed when the ifindex is
> not
> > > > available any more. (as the interface is already deleted.)
> > > >
> > > > VMware-BZ: #1975788
> > > > Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
> > >
> > > Thanks.  I applied this to master and branch-2.8.
> > >
> > > I still think that the underlying design here is a bad one.  I'm going
> > > to look at a better fix.
> > >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] tests: add-del-add sequence for interface

[Ashish Varma]

Added a unit test case for testing the condition when a veth interface
is added to br0 and then the veth interface is deleted from the
system and added back with the same name.

Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
---
v1-v2
Incorporated the comments by Yi-Hung Wei:
 Added system-interface.at test to system-userspace-testsuite.
 Replaced space by tab in automake.mk.
 Added handling for conditions when the veth interface is not detected
 (as expected) by vswitchd, which caused test case failure. 
 (This was happening intermittently)
---
 tests/automake.mk   |  3 ++-
 tests/system-interface.at   | 27 +++
 tests/system-kmod-testsuite.at  |  1 +
 tests/system-userspace-testsuite.at |  1 +
 4 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 tests/system-interface.at

diff --git a/tests/automake.mk b/tests/automake.mk
index c1a2357..5af03d6 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -119,7 +119,8 @@ SYSTEM_TESTSUITE_AT = \
tests/system-common-macros.at \
tests/system-ovn.at \
tests/system-layer3-tunnels.at \
-   tests/system-traffic.at
+   tests/system-traffic.at \
+   tests/system-interface.at
 
 SYSTEM_OFFLOADS_TESTSUITE_AT = \
tests/system-common-macros.at \
diff --git a/tests/system-interface.at b/tests/system-interface.at
new file mode 100644
index 000..db790d5
--- /dev/null
+++ b/tests/system-interface.at
@@ -0,0 +1,27 @@
+AT_BANNER([system-inteface])
+
+dnl add a veth interface to br0, then delete and re-create
+dnl the veth interface with the same name in the system
+AT_SETUP([interface - add delete add same interface])
+
+OVS_TRAFFIC_VSWITCHD_START()
+
+AT_CHECK([ip link add ovs-veth0 type veth peer name ovs-veth1])
+on_exit 'ip link del ovs-veth0'
+
+AT_CHECK([ovs-vsctl add-port br0 ovs-veth0])
+
+AT_CHECK([ip link del ovs-veth0])
+AT_CHECK([ip link add ovs-veth0 type veth peer name ovs-veth1])
+
+AT_CHECK([ovs-vsctl del-port br0 ovs-veth0])
+
+OVS_TRAFFIC_VSWITCHD_STOP(["dnl
+/could not open network device ovs-veth0/d
+/cannot get .*STP status on nonexistent port/d
+/ethtool command .*on network device ovs-veth0 failed/d
+/error receiving .*ovs-veth0/d
+/ovs-veth0: removing policing failed/d"])
+
+AT_CLEANUP
+
diff --git a/tests/system-kmod-testsuite.at b/tests/system-kmod-testsuite.at
index 975b200..bda314a 100644
--- a/tests/system-kmod-testsuite.at
+++ b/tests/system-kmod-testsuite.at
@@ -25,3 +25,4 @@ m4_include([tests/system-kmod-macros.at])
 m4_include([tests/system-traffic.at])
 m4_include([tests/system-layer3-tunnels.at])
 m4_include([tests/system-ovn.at])
+m4_include([tests/system-interface.at])
diff --git a/tests/system-userspace-testsuite.at 
b/tests/system-userspace-testsuite.at
index a89536f..c0aaa60 100644
--- a/tests/system-userspace-testsuite.at
+++ b/tests/system-userspace-testsuite.at
@@ -25,4 +25,5 @@ m4_include([tests/system-common-macros.at])
 m4_include([tests/system-traffic.at])
 m4_include([tests/system-layer3-tunnels.at])
 m4_include([tests/system-ovn.at])
+m4_include([tests/system-interface.at])
 m4_include([tests/system-userspace-packet-type-aware.at])
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] netdev, dpif: fix the crash/assert on port delete

[Ashish Varma]

Thanks Ben.

On Mon, Nov 13, 2017 at 11:36 AM, Ben Pfaff <b...@ovn.org> wrote:

> On Mon, Nov 06, 2017 at 12:17:45PM -0800, Ashish Varma wrote:
> > a crash is seen in "netdev_ports_remove" when an interface is deleted
> and added
> > back in the system and when the interface is part of a bridge
> configuration.
> > e.g. steps:
> >   create a tap0 interface using "ip tuntap add.."
> >   add the tap0 interface to br0 using "ovs-vsctl add-port.."
> >   delete the tap0 interface from system using "ip tuntap del.."
> >   add the tap0 interface back in system using "ip tuntap add.."
> >(this changes the ifindex of the interface)
> >   delete tap0 from br0 using "ovs-vsctl del-port.."
> >
> > In the function "netdev_ports_insert", two hmap entries were created for
> > mapping "portnum -> netdev" and "ifindex -> portnum".
> > When the interface is deleted from the system, the "netdev_ports_remove"
> > function is not getting called and the old ifindex entry is not getting
> > cleaned up from the "ifindex_to_port" hmap.
> >
> > As part of the fix, added function "dpif_port_remove" which will call
> > "netdev_ports_remove" in the path where the interface deletion from the
> system
> > is detected.
> > Also, in "netdev_ports_remove", added the code where the
> "ifindex_to_port_data"
> > (ifindex -> portnum map node) is getting freed when the ifindex is not
> > available any more. (as the interface is already deleted.)
> >
> > VMware-BZ: #1975788
> > Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
>
> Thanks.  I applied this to master and branch-2.8.
>
> I still think that the underlying design here is a bad one.  I'm going
> to look at a better fix.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] netdev, dpif: fix the crash/assert on port delete

[Ashish Varma]

a crash is seen in "netdev_ports_remove" when an interface is deleted and added
back in the system and when the interface is part of a bridge configuration.
e.g. steps:
  create a tap0 interface using "ip tuntap add.."
  add the tap0 interface to br0 using "ovs-vsctl add-port.."
  delete the tap0 interface from system using "ip tuntap del.."
  add the tap0 interface back in system using "ip tuntap add.."
   (this changes the ifindex of the interface)
  delete tap0 from br0 using "ovs-vsctl del-port.."

In the function "netdev_ports_insert", two hmap entries were created for
mapping "portnum -> netdev" and "ifindex -> portnum".
When the interface is deleted from the system, the "netdev_ports_remove"
function is not getting called and the old ifindex entry is not getting
cleaned up from the "ifindex_to_port" hmap.

As part of the fix, added function "dpif_port_remove" which will call
"netdev_ports_remove" in the path where the interface deletion from the system
is detected.
Also, in "netdev_ports_remove", added the code where the "ifindex_to_port_data"
(ifindex -> portnum map node) is getting freed when the ifindex is not
available any more. (as the interface is already deleted.)

VMware-BZ: #1975788
Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
---
 AUTHORS.rst|  1 +
 lib/dpif.c |  6 ++
 lib/dpif.h |  1 +
 lib/netdev.c   | 22 +++---
 ofproto/ofproto-dpif.c |  6 ++
 5 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 3d61c04..7eecd54 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -62,6 +62,7 @@ Ariel Tubaltsev atubalt...@vmware.com
 Arnoldo Lutz        arnoldo.lutz.guev...@hpe.com
 Arun Sharma arun.sha...@calsoftinc.com
 Aryan TaheriMonfaredaryan.taherimonfa...@uis.no
+Ashish Varmaashishvarma@gmail.com
 Ashwin Swaminathan  ashwi...@arista.com
 Babu Shanmugam  bscha...@redhat.com
 Ben Pfaff   b...@ovn.org
diff --git a/lib/dpif.c b/lib/dpif.c
index 3233bc8..6b2734b 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -623,6 +623,12 @@ dpif_port_del(struct dpif *dpif, odp_port_t port_no)
 return error;
 }
 
+int
+dpif_port_remove(struct dpif *dpif, odp_port_t port_no)
+{
+return netdev_ports_remove(port_no, dpif->dpif_class);
+}
+
 /* Makes a deep copy of 'src' into 'dst'. */
 void
 dpif_port_clone(struct dpif_port *dst, const struct dpif_port *src)
diff --git a/lib/dpif.h b/lib/dpif.h
index d9ded8b..c2d1c61 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -452,6 +452,7 @@ const char *dpif_port_open_type(const char *datapath_type,
 const char *port_type);
 int dpif_port_add(struct dpif *, struct netdev *, odp_port_t *port_nop);
 int dpif_port_del(struct dpif *, odp_port_t port_no);
+int dpif_port_remove(struct dpif *, odp_port_t port_no);
 
 /* A port within a datapath.
  *
diff --git a/lib/netdev.c b/lib/netdev.c
index 704b38f..954da92 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2179,6 +2179,7 @@ struct ifindex_to_port_data {
 struct hmap_node node;
 int ifindex;
 odp_port_t port;
+const struct dpif_class *dpif_class;
 };
 
 #define NETDEV_PORTS_HASH_INT(port, dpif) \
@@ -2228,6 +2229,7 @@ netdev_ports_insert(struct netdev *netdev, const struct 
dpif_class *dpif_class,
 ifidx = xzalloc(sizeof *ifidx);
 ifidx->ifindex = ifindex;
 ifidx->port = dpif_port->port_no;
+ifidx->dpif_class = dpif_class;
 
 hmap_insert(_to_netdev, >node, hash);
 hmap_insert(_to_port, >node, ifidx->ifindex);
@@ -2266,10 +2268,9 @@ netdev_ports_remove(odp_port_t port_no, const struct 
dpif_class *dpif_class)
 
 if (data) {
 int ifindex = netdev_get_ifindex(data->netdev);
+struct ifindex_to_port_data *ifidx = NULL;
 
 if (ifindex > 0) {
-struct ifindex_to_port_data *ifidx = NULL;
-
 HMAP_FOR_EACH_WITH_HASH (ifidx, node, ifindex, _to_port) {
 if (ifidx->port == port_no) {
 hmap_remove(_to_port, >node);
@@ -2279,11 +2280,18 @@ netdev_ports_remove(odp_port_t port_no, const struct 
dpif_class *dpif_class)
 }
 ovs_assert(ifidx);
 } else {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-
-VLOG_WARN_RL(, "netdev ports map has dpif port %"PRIu32
- " but netdev has no ifindex: %s", port_no,
- ovs_strerror(ifindex));
+/* case where the interface is already deleted form the datapath
+ * (e.g. tunctl -d or ip tuntap del), then the ifindex is not
+  

Re: [ovs-dev] [PATCH] netdev, dpif: fix the crash/assert on port delete

[Ashish Varma]

Hi Ben,

I have refactored the function "dpif_port_del" and removed the new function
"dpif_port_remove" added in the v1 patch.
Removed the braces around the if compare.

Sending v2 patch now.

Thanks for the review.

-Ashish

On Fri, Nov 3, 2017 at 2:26 PM, Ben Pfaff <b...@ovn.org> wrote:

> On Fri, Nov 03, 2017 at 07:36:41AM -0700, Ashish Varma wrote:
> > a crash is seen in "netdev_ports_remove" when an interface is deleted
> and added
> > back in the system and when the interface is part of a bridge
> configuration.
> > e.g. steps:
> >   create a tap0 interface using "ip tuntap add.."
> >   add the tap0 interface to br0 using "ovs-vsctl add-port.."
> >   delete the tap0 interface from system using "ip tuntap del.."
> >   add the tap0 interface back in system using "ip tuntap add.."
> >(this changes the ifindex of the interface)
> >   delete tap0 from br0 using "ovs-vsctl del-port.."
> >
> > In the function "netdev_ports_insert", two hmap entries were created for
> > mapping "portnum -> netdev" and "ifindex -> portnum".
> > When the interface is deleted from the system, the "netdev_ports_remove"
> > function is not getting called and the old ifindex entry is not getting
> > cleaned up from the "ifindex_to_port" hmap.
> >
> > As part of the fix, added function "dpif_port_remove" which will call
> > "netdev_ports_remove" in the path where the interface deletion from the
> system
> > is detected.
> > Also, in "netdev_ports_remove", added the code where the
> "ifindex_to_port_data"
> > (ifindex -> portnum map node) is getting freed when the ifindex is not
> > available any more. (as the interface is already deleted.)
> >
> > VMware-BZ: #1975788
> > Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
>
> Thanks for the patch.  It's good to fix a bug and especially to fix a
> crash.  I'm still not entirely certain about the actual need for this
> hmap, but I guess that this fixes a real problem.
>
> This introduces new code in netdev_ports_remove() to handle the new
> case.  The new case duplicates some code that I believe should be
> possible to avoid duplicating.  Can you try to refactor slightly to
> avoid the code duplication?
>
> I see that this writes an expression like: (a == b) && (c == d).
> Usually, in Open vSwitch, we don't add the extra parentheses unless they
> are needed or clear up some genuine confusion, so that we would instead
> write a == b && c == d.
>
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] tests: add-del-add sequence for interface

[Ashish Varma]

added a unit test case for testing the condition when a tap interface
is added to br0 and then the tap interface is deleted from the
system and added back with the same name.

Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
---
 tests/automake.mk  |  3 ++-
 tests/system-interface.at  | 22 ++
 tests/system-kmod-testsuite.at |  1 +
 3 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 tests/system-interface.at

diff --git a/tests/automake.mk b/tests/automake.mk
index 7eed106..8c5f1b9 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -118,7 +118,8 @@ SYSTEM_TESTSUITE_AT = \
tests/system-common-macros.at \
tests/system-ovn.at \
tests/system-layer3-tunnels.at \
-   tests/system-traffic.at
+   tests/system-traffic.at \
+tests/system-interface.at
 
 SYSTEM_OFFLOADS_TESTSUITE_AT = \
tests/system-common-macros.at \
diff --git a/tests/system-interface.at b/tests/system-interface.at
new file mode 100644
index 000..a71e2aa
--- /dev/null
+++ b/tests/system-interface.at
@@ -0,0 +1,22 @@
+AT_BANNER([system-inteface])
+
+dnl add a tap interface to br0, then delete and re-create
+dnl the tap interface with the same name in the system
+AT_SETUP([interface - add delete add same interface])
+
+OVS_TRAFFIC_VSWITCHD_START()
+
+AT_CHECK([ip tuntap add dev ovs-tap0 mode tap])
+on_exit 'ip tuntap del dev ovs-tap0 mode tap'
+
+AT_CHECK([ovs-vsctl add-port br0 ovs-tap0])
+
+AT_CHECK([ip tuntap del dev ovs-tap0 mode tap])
+AT_CHECK([ip tuntap add dev ovs-tap0 mode tap])
+
+AT_CHECK([ovs-vsctl del-port br0 ovs-tap0])
+
+OVS_TRAFFIC_VSWITCHD_STOP(["/could not open network device ovs-tap0/d"])
+
+AT_CLEANUP
+
diff --git a/tests/system-kmod-testsuite.at b/tests/system-kmod-testsuite.at
index 975b200..bda314a 100644
--- a/tests/system-kmod-testsuite.at
+++ b/tests/system-kmod-testsuite.at
@@ -25,3 +25,4 @@ m4_include([tests/system-kmod-macros.at])
 m4_include([tests/system-traffic.at])
 m4_include([tests/system-layer3-tunnels.at])
 m4_include([tests/system-ovn.at])
+m4_include([tests/system-interface.at])
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/3] tests/sendpkt.py: Fix to work with Python3

[Ashish Varma]

Acked-by: Ashish Varma >

On Thu, May 31, 2018 at 7:52 AM, Timothy Redaelli 
wrote:

> CC: Ashish Varma 
> Fixes: 296251ca0c82 ("tests: Added NSH related unit test cases for
> datapath")
> Signed-off-by: Timothy Redaelli 
> ---
>  tests/sendpkt.py | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tests/sendpkt.py b/tests/sendpkt.py
> index 50a4795eb..328ae2bc9 100755
> --- a/tests/sendpkt.py
> +++ b/tests/sendpkt.py
> @@ -66,7 +66,10 @@ for a in args[1:]:
>
>  hex_list.append(temp)
>
> -pkt = "".join(map(chr, hex_list))
> +if sys.version_info < (3, 0):
> +pkt = "".join(map(chr, hex_list))
> +else:
> +pkt = bytes(hex_list)
>
>  try:
>  sockfd = socket.socket(socket.AF_PACKET, socket.SOCK_RAW)
> --
> 2.17.0
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1] doc: Added OVS Conntrack tutorial

[Ashish Varma]

OVS supports connection tracker related match fields and actions.
Added a tutorial to demonstrate the basic use cases for some of these
match fields and actions.

Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
---
 Documentation/automake.mk |   1 +
 Documentation/tutorials/index.rst |   1 +
 Documentation/tutorials/ovs-conntrack.rst | 572 ++
 3 files changed, 574 insertions(+)
 create mode 100644 Documentation/tutorials/ovs-conntrack.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 2b202cb..93cf3a1 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -27,6 +27,7 @@ DOC_SOURCE = \
Documentation/tutorials/ovs-advanced.rst \
Documentation/tutorials/ovn-openstack.rst \
Documentation/tutorials/ovn-sandbox.rst \
+   Documentation/tutorials/ovs-conntrack.rst \
Documentation/topics/index.rst \
Documentation/topics/bonding.rst \
Documentation/topics/idl-compound-indexes.rst \
diff --git a/Documentation/tutorials/index.rst 
b/Documentation/tutorials/index.rst
index c2d343b..ab90b7c 100644
--- a/Documentation/tutorials/index.rst
+++ b/Documentation/tutorials/index.rst
@@ -43,3 +43,4 @@ vSwitch.
ovs-advanced
ovn-sandbox
ovn-openstack
+   ovs-conntrack
diff --git a/Documentation/tutorials/ovs-conntrack.rst 
b/Documentation/tutorials/ovs-conntrack.rst
new file mode 100644
index 000..31cd1a8
--- /dev/null
+++ b/Documentation/tutorials/ovs-conntrack.rst
@@ -0,0 +1,572 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+==
+OVS Conntrack Tutorial
+==
+
+OVS can be used with the Connection tracking system
+where OpenFlow flow can be used to match on the state of a TCP, UDP, ICMP,
+etc., connections. (Connection tracking system supports tracking of both
+statefull and stateless protocols)
+
+This tutorial demonstrates how OVS can use the connection tracking system
+to match on the TCP segments from connection setup to connection teardown.
+It will use OVS with the linux kernel module as the datapath for this
+tutorial. (datapath which utilizes the openvswitch kernel module to do
+the packet processing in the linux kernel)
+It was tested with the “master” branch of Open vSwitch.
+
+Definitions
+---
+
+**conntrack**: is a connection tracking module for stateful packet
+inspection.
+
+**pipeline**: is the packet processing pipeline which is the path taken by
+the packet when traversing through the tables where the pakcet matches the
+match fields of a flow in the table and performs the actions present in
+the matched flow.
+
+**network namespace**: is a way to create virtual routing domains within
+a single instance of linux kernel.  Each network namespace has it's own
+instance of network tables (arp, routing) and certain interfaces attached
+to it.
+
+**flow**: used in this tutorial refers to the OpenFlow flow which can be
+programmed using an OpenFlow controller or OVS command line tools like
+ovs-ofctl which is used here.  A flow will have match fields and actions.
+
+Conntrack Related Fields
+
+
+Match Fields
+
+OVS supports following match fields related to conntrack:
+
+1. **ct_state**:
+The state of a connection matching the packet.
+Possible values:
+
+- *new*
+- *est*
+- *rel*
+- *rpl*
+- *inv*
+- *trk*
+- *snat*
+- *dnat*
+
+Each of these flags is preceded by either a "+" for a flag that
+must be set, or a "-" for a flag that must be unset.
+Multiple flags can also be specified e.g. ct_state=+trk+new
+We will see the usage of some these flags below. For a detailed
+description, please see the OVS fields documentation at:
+http://openvswitch.org/support/dist-docs/ovs-fields.7.txt
+
+2. **ct_zone**: A zone is an independent connection tracking context which can
+be set by a ct action.
+A 16-bit ct_zone set by the most recent ct action (by an OpenFlow
+flow on a conntrack entry) can be used as a match field in
+another flow entry.
+
+3. **ct_mark**

Re: [ovs-dev] [PATCH] tests: Test for ovs-ofctl snoop command

[Ashish Varma]

Thanks for the review. I will try to use OVS_WAIT_UNTIL (or something
similar)

On Thu, Aug 2, 2018 at 7:33 AM, Ilya Maximets 
wrote:

> Hello. Thanks for the patch.
> I'm not much familiar with that functionality thus someone
> else should review the sanity of this patch, but I have few
> comments about testing itself. See inline.
>
> Best regard, Ilya Maximets.
>
> > Added test for snoop command to check for the initial handshake messages
> > when a bridge connects to a controller via 'unix' connection method.
> >
> > Signed-off-by: Ashish Varma 
> > ---
> >  tests/ovs-ofctl.at | 28 
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> > index 06597d7..794277b 100644
> > --- a/tests/ovs-ofctl.at
> > +++ b/tests/ovs-ofctl.at
> > @@ -3184,3 +3184,31 @@ AT_CHECK([grep -q "ct_dpif|DBG|.*ct_flush: zone
> 123" ovs-vswitchd.log])
> >
> >  OVS_VSWITCHD_STOP
> >  AT_CLEANUP
> > +
> > +
> > +AT_SETUP([ovs-ofctl snoop-unix-connection])
> > +OVS_VSWITCHD_START
> > +
> > +dnl setup controller for br0 before starting the controller
> > +AT_CHECK([ovs-vsctl set-controller br0 unix:testcontroller])
> > +
> > +dnl then start listening on the '.snoop' connection
> > +AT_CHECK([ovs-ofctl --detach --pidfile=ovsofctl_snoop.pid snoop br0 1>
> snoopbr0.txt 2>&1])
> > +on_exit 'kill `cat ovsofctl_snoop.pid`'
> > +on_exit 'unlink snoopbr0.txt'
> > +
> > +dnl finally start the controller
> > +AT_CHECK([ovs-testcontroller --detach --pidfile punix:testcontroller],
> [0], [ignore])
>
> Could you, please, add '-vsyslog:off' to this command?
>
> > +on_exit 'kill `cat ovs-testcontroller.pid`'
> > +OVS_WAIT_UNTIL([test -e testcontroller])
> > +
> > +dnl wait for 2 seconds for snoop to collect the messages from the bridge
> > +sleep 2
>
> Waiting few seconds is a bad style, IMHO. For example this could
> lead to test failures in parallel testing if the thread will have
> no enough time to work.
>
> Is it possible to replace this by something like OVS_WAIT_UNTIL ?
>
> > +
> > +dnl check some of the initial openflow setup messages
> > +AT_CHECK([egrep "OFPT_FEATURES_REQUEST" snoopbr0.txt 1> /dev/null 2>&1])
> > +AT_CHECK([egrep "OFPT_FEATURES_REPLY" snoopbr0.txt 1> /dev/null 2>&1])
> > +AT_CHECK([egrep "OFPT_SET_CONFIG" snoopbr0.txt 1> /dev/null 2>&1])
> > +
> > +OVS_VSWITCHD_STOP(["/connection failed (No such file or directory)/d"])
> > +AT_CLEANUP
> > --
> > 2.7.4
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] tests: Test for ovs-ofctl snoop command

[Ashish Varma]

Added test for snoop command to check for the initial handshake messages
when a bridge connects to a controller via 'unix' connection method.

Signed-off-by: Ashish Varma 
---
 tests/ovs-ofctl.at | 28 
 1 file changed, 28 insertions(+)

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 06597d7..794277b 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -3184,3 +3184,31 @@ AT_CHECK([grep -q "ct_dpif|DBG|.*ct_flush: zone 123" 
ovs-vswitchd.log])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+
+AT_SETUP([ovs-ofctl snoop-unix-connection])
+OVS_VSWITCHD_START
+
+dnl setup controller for br0 before starting the controller
+AT_CHECK([ovs-vsctl set-controller br0 unix:testcontroller])
+
+dnl then start listening on the '.snoop' connection
+AT_CHECK([ovs-ofctl --detach --pidfile=ovsofctl_snoop.pid snoop br0 1> 
snoopbr0.txt 2>&1])
+on_exit 'kill `cat ovsofctl_snoop.pid`'
+on_exit 'unlink snoopbr0.txt'
+
+dnl finally start the controller
+AT_CHECK([ovs-testcontroller --detach --pidfile punix:testcontroller], [0], 
[ignore])
+on_exit 'kill `cat ovs-testcontroller.pid`'
+OVS_WAIT_UNTIL([test -e testcontroller])
+
+dnl wait for 2 seconds for snoop to collect the messages from the bridge
+sleep 2
+
+dnl check some of the initial openflow setup messages
+AT_CHECK([egrep "OFPT_FEATURES_REQUEST" snoopbr0.txt 1> /dev/null 2>&1])
+AT_CHECK([egrep "OFPT_FEATURES_REPLY" snoopbr0.txt 1> /dev/null 2>&1])
+AT_CHECK([egrep "OFPT_SET_CONFIG" snoopbr0.txt 1> /dev/null 2>&1])
+
+OVS_VSWITCHD_STOP(["/connection failed (No such file or directory)/d"])
+AT_CLEANUP
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1] ovs-testcontroller: Added section for runtime managment commands

[Ashish Varma]

Thanks for the review. I will send a v2 patch with your suggestions.

On Tue, Aug 7, 2018 at 4:06 PM, Ben Pfaff  wrote:

> On Mon, Aug 06, 2018 at 05:21:37PM -0700, Ashish Varma wrote:
> > Even though there are no runtime management commands supported by
> > ovs-testcontroller, the '--unixctl' section of the man page refers to
> > 'RUNTIME MANAGEMENT COMMANDS'. This message which refers to the runtime
> > management commands section is common for all '--unixctl' option.
> > (via 'lib/unixctl.man')
> >
> > Added an empty section in the man page for 'RUNTIME MANAGEMENT COMMANDS'
> to
> > avoid confusion to the reader of the man page.
> >
> > Signed-off-by: Ashish Varma 
>
> Thanks for improving the OVS documentation!
>
> However, unixctl is useless if there are no commands.  It's probably
> better to remove the support for it.
>
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] tests: Test for ovs-ofctl snoop command

[Ashish Varma]

Added test for snoop command to check for the initial handshake messages
when a bridge connects to a controller via 'unix' connection method.

Signed-off-by: Ashish Varma 
---
v1-v2:

Removed the sleep and added OVS_WAIT_UNTIL.
Added comment on why we are adding an exception for the 'connection failed'
WARN log message.
Added '-vsyslog:off'.
Minor change for pid file name change.
---
 tests/ovs-ofctl.at | 28 
 1 file changed, 28 insertions(+)

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 06597d7..a1eecad 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -3184,3 +3184,31 @@ AT_CHECK([grep -q "ct_dpif|DBG|.*ct_flush: zone 123" 
ovs-vswitchd.log])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+
+AT_SETUP([ovs-ofctl snoop-unix-connection])
+OVS_VSWITCHD_START
+
+dnl setup controller for br0 before starting the controller
+AT_CHECK([ovs-vsctl -vsyslog:off set-controller br0 unix:testcontroller])
+
+dnl then start listening on the '.snoop' connection
+AT_CHECK([ovs-ofctl -vsyslog:off --detach --pidfile=ovs-ofctl-snoop.pid snoop 
br0 1> snoopbr0.txt 2>&1])
+on_exit 'kill `cat ovs-ofctl-snoop.pid`'
+on_exit 'unlink snoopbr0.txt'
+
+dnl finally start the controller
+AT_CHECK([ovs-testcontroller -vsyslog:off --detach --pidfile 
punix:testcontroller], [0], [ignore])
+on_exit 'kill `cat ovs-testcontroller.pid`'
+OVS_WAIT_UNTIL([test -e testcontroller])
+
+dnl check for some of the initial handshake messages
+OVS_WAIT_UNTIL([egrep "OFPT_FEATURES_REQUEST" snoopbr0.txt 1> /dev/null 2>&1])
+OVS_WAIT_UNTIL([egrep "OFPT_FEATURES_REPLY" snoopbr0.txt 1> /dev/null 2>&1])
+OVS_WAIT_UNTIL([egrep "OFPT_SET_CONFIG" snoopbr0.txt 1> /dev/null 2>&1])
+
+dnl need to suppress the 'connection failed' WARN message in ovs-vswitchd
+dnl because we need ovs-vswitchd to have the controller config before starting
+dnl the controller to 'snoop' the OpenFlow messages from beginning
+OVS_VSWITCHD_STOP(["/connection failed (No such file or directory)/d"])
+AT_CLEANUP
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3] tests: Test for ovs-ofctl snoop command

[Ashish Varma]

Added test for snoop command to check for the initial handshake messages
when a bridge connects to a controller via 'unix' connection method.

Signed-off-by: Ashish Varma 
---
v2-v3:

Moved 'on_exit kill" command before the start of the command to avoid the race 
condition between start of the process and registering to have it killed on
interrupt.
Removed deletion of snoopbr0.txt to debug any failure.
Cleaner shell re-direct.

---
 tests/ovs-ofctl.at | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 06597d7..21a576a 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -3184,3 +3184,30 @@ AT_CHECK([grep -q "ct_dpif|DBG|.*ct_flush: zone 123" 
ovs-vswitchd.log])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+
+AT_SETUP([ovs-ofctl snoop-unix-connection])
+OVS_VSWITCHD_START
+
+dnl setup controller for br0 before starting the controller
+AT_CHECK([ovs-vsctl -vsyslog:off set-controller br0 unix:testcontroller])
+
+dnl then start listening on the '.snoop' connection
+on_exit 'kill `cat ovs-ofctl-snoop.pid`'
+AT_CHECK([ovs-ofctl -vsyslog:off --detach --pidfile=ovs-ofctl-snoop.pid snoop 
br0 > snoopbr0.txt 2>&1])
+
+dnl finally start the controller
+on_exit 'kill `cat ovs-testcontroller.pid`'
+AT_CHECK([ovs-testcontroller -vsyslog:off --detach --pidfile 
punix:testcontroller], [0], [ignore])
+OVS_WAIT_UNTIL([test -e testcontroller])
+
+dnl check for some of the initial handshake messages
+OVS_WAIT_UNTIL([egrep "OFPT_FEATURES_REQUEST" snoopbr0.txt 1> /dev/null 2>&1])
+OVS_WAIT_UNTIL([egrep "OFPT_FEATURES_REPLY" snoopbr0.txt 1> /dev/null 2>&1])
+OVS_WAIT_UNTIL([egrep "OFPT_SET_CONFIG" snoopbr0.txt 1> /dev/null 2>&1])
+
+dnl need to suppress the 'connection failed' WARN message in ovs-vswitchd
+dnl because we need ovs-vswitchd to have the controller config before starting
+dnl the controller to 'snoop' the OpenFlow messages from beginning
+OVS_VSWITCHD_STOP(["/connection failed (No such file or directory)/d"])
+AT_CLEANUP
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1] ovs-testcontroller: Added section for runtime managment commands

[Ashish Varma]

Even though there are no runtime management commands supported by
ovs-testcontroller, the '--unixctl' section of the man page refers to
'RUNTIME MANAGEMENT COMMANDS'. This message which refers to the runtime
management commands section is common for all '--unixctl' option.
(via 'lib/unixctl.man')

Added an empty section in the man page for 'RUNTIME MANAGEMENT COMMANDS' to
avoid confusion to the reader of the man page.

Signed-off-by: Ashish Varma 
---
 utilities/ovs-testcontroller.8.in | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/utilities/ovs-testcontroller.8.in 
b/utilities/ovs-testcontroller.8.in
index df3c35f..5ea325f 100644
--- a/utilities/ovs-testcontroller.8.in
+++ b/utilities/ovs-testcontroller.8.in
@@ -146,6 +146,9 @@ Use this option more than once to add flows from multiple 
files.
 .so lib/common.man
 .so lib/ofp-version.man
 .
+.SH "RUNTIME MANAGEMENT COMMANDS"
+There are no runtime management commands defined for \fBovs\-testcontroller\fR.
+.
 .SH EXAMPLES
 .PP
 To bind locally to port 6653 (the default) and wait for incoming
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev,v3] tests: Test for ovs-ofctl snoop command

[Ashish Varma]

Thanks for clarifying.

On Tue, Aug 7, 2018 at 12:07 PM, Aaron Conole  wrote:

> Ashish Varma  writes:
>
> > Hi Aaron,
> >
> > My patch is already committed by Ben. Not sure what is wrong here.
>
> Thanks for the heads up.
>
> This happens - usually if the patch is committed before the bot has run
> against it.  I have it on my list of TODOs, so as soon as my next 'work
> on the bot' time comes, I'll fix it.
>
> > Is there something that I need to do now? (or should have done correctly)
>
> Without looking too deeply, most likely a false positive.  Feel free to
> not worry about it :)  I'll ping if it's a real issue somewhere.
>
> > Thanks,
> > Ashish
> > VMware OVS team
> >
> > On Mon, Aug 6, 2018 at 7:15 PM, 0-day Robot  wrote:
> >
> >  Bleep bloop.  Greetings Ashish Varma, I am a robot and I have tried out
> your patch.
> >  Thanks for your contribution.
> >
> >  I encountered some error that I wasn't expecting.  See the details
> below.
> >
> >  git-am:
> >  Failed to merge in the changes.
> >  Patch failed at 0001 tests: Test for ovs-ofctl snoop command
> >  The copy of the patch that failed is found in:
> > /var/lib/jenkins/jobs/upstream_build_from_pw/
> workspace/.git/rebase-apply/patch
> >  When you have resolved this problem, run "git am --resolved".
> >  If you prefer to skip this patch, run "git am --skip" instead.
> >  To restore the original branch and stop patching, run "git am --abort".
> >
> >  Please check this out.  If you feel there has been an error, please
> email acon...@bytheb.org
> >
> >  Thanks,
> >  0-day Robot
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Problem in using ovs in raspberry pi - regd.

[Ashish Varma]

I understand that you have few Rasberry Pi's as switches and some as hosts.
A picture/some more explanation of the topology would help.
How many ports does the Rasberry Pi have. How many are connected to the OVS
bridge?  I understand you use the wireless interface of Rasberry Pi to
connet to RYU.
What are the flows installed on the OVS instances? (Can you give the output
of ovs-ofctl dump-flows  as well as ovs-vsctl show)


On Thu, Aug 2, 2018 at 7:34 PM, Viswanath Muthukumaraswamy Sathananth <
vs527...@dal.ca> wrote:

> Hi,
>
> I am trying to implement a real test bed to check the interaction between
> switches and controllers. Raspberry Pi 3b+ act as switches with OVS version
> 2.5.5 running on them. Two such  Raspberry Pis connected to a RYU
> controller running on Windows 10 Machine wirelessly. One Raspberry Pi which
> is a simple host is attached to every switch. When I try to ping from one
> host to another I get a few problems.
>
> 1. The host raspberry pi is treated as LOCAL. And the flow entries have
> in_port or out_port as LOCAL.
> 2. RYU controller is not able to indetify any links.
> 3. Ping request doesn't echo back.
>
> What can be the possible errors in this case. Kindly suggest us a solution.
>
> Thanks,
>
> Viz
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1] tests: Added NSH related unit test cases for datapath

[Ashish Varma]

Sure, no problem.

Thanks,
Ashish

On Tue, Apr 10, 2018 at 5:00 PM, Gregory Rose <gvrose8...@gmail.com> wrote:

> On 4/6/2018 7:35 AM, Gregory Rose wrote:
>
>>
>>
>> On 4/4/2018 10:23 AM, Ben Pfaff wrote:
>>
>>> On Thu, Mar 29, 2018 at 04:46:09PM -0700, Ashish Varma wrote:
>>>
>>>> Added test cases for encap, decap, replace and forwarding of NSH
>>>> packets.
>>>> Also added a python script 'sendpkt.py' to send hex ethernet frames.
>>>>
>>>> Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
>>>>
>>> Ashish, do you have a suggestion who should review this?  Greg, are you
>>> the right person?
>>>
>> I can have a look at them now that I'm back but need to get caught up
>> with some other issues first.
>>
>
> Ashish,
>
> I continue to be otherwise occupied so I'm sorry for the delay in
> reviewing your patch.  It is still on my radar!
>
> Thanks,
>
> - Greg
>
>
>> Thanks,
>>
>> - Greg
>>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3] ovs-ofctl: Fixed the "snoop" command of ovs-ofctl

[Ashish Varma]

In case where "use_names" is set (e.g. in an interactive session) to show
the port and table names when ovs-ofctl is run with snoop command,
ovs-ofctl would get stuck in an endless loop inside "table_iterator_next"
function's while loop checking for "while (ti->send_xid != recv_xid)".
This would happening because the "vconn" to ".snoop" socket would
not respond to TABLE_FEATURES_REQUEST sent by ovs-ofctl.

This commit disables showing port or table names in the snoop command.

Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
---
v2-v3

Updated the comment in the code to explain why 'use_names' is set to 0
for snoop command.
---
 utilities/ovs-ofctl.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 6708b07..92a5ebc 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2326,6 +2326,11 @@ ofctl_snoop(struct ovs_cmdl_context *ctx)
 {
 struct vconn *vconn;
 
+/* we can't use the snoop vconn to send table features request or
+ * port description request messages to show names. ovs-vswitchd
+ * will not respond to these messages on snoop vconn */
+use_names = 0;
+
 open_vconn__(ctx->argv[1], SNOOP, );
 monitor_vconn(vconn, false, false);
 }
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1] ovs-ofctl: Fixed the "snoop" command of ovs-ofctl

[Ashish Varma]

In case where "use_names" is set (e.g. in an interactive session) to show
the port and table names when ovs-ofctl is run with snoop command,
ovs-ofctl would get stuck in an endless loop inside "table_iterator_next"
function's while loop checking for "while (ti->send_xid != recv_xid)".
This would happening because the "vconn" to ".snoop" socket would
not respond to TABLE_FEATURES_REQUEST sent by ovs-ofctl.

This commit disables showing port or table names in the snoop command.

Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
---
 utilities/ovs-ofctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 6708b07..3023787 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2326,6 +2326,7 @@ ofctl_snoop(struct ovs_cmdl_context *ctx)
 {
 struct vconn *vconn;
 
+use_names = 0; /* don't show port and table names */
 open_vconn__(ctx->argv[1], SNOOP, );
 monitor_vconn(vconn, false, false);
 }
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ovs-ofctl: Fixed the "snoop" command of ovs-ofctl

[Ashish Varma]

In normal ovs-ofctl commands (e.g. add-flow), ovs-ofctl connects to
ovs-vswitchd process on “<ovs_rundir()>/.mgmt” unix socket.
In an output that contains a port or table, port name or table name can be
displayed, instead of their numbers, if the user turns on this feature.
(by -—names option) Also, this feature is enabled when interacting with a user
on console.  To fetch the names, ovs-ofctl sends TABLE FEATURE /
PORT DESCRIPTION request message to ovs-vswitchd process and expects a reply
on the connection.
When ovs-ofctl runs the snoop command, it connects to
“<ovs_rundir()>/.snoop” unix socket. ovs-vswitchd process would
not reply to the TABLE FEATURE / PORT DESCRIPTION request message on this
connection. It would only send any open flow message it receives from the
controller.
When using port/table name feature with snoop command, the print of open flow
message would call ‘tables_to_show()’/‘ports_to_show()’ which in turn would
send the request message on the snoop connection. ovs-vswitchd would not reply
back on this connection, but instead would keep sending the open flow messages
received from controller. ‘table_iterator_next()/port_iterator_next()’ function
would loop for ever waiting for response.
The fix for this is to turn off display of table/port names when running
snoop command.

Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
---
v1-v2

Added more description to the cause of the issue and reason to add the fix.
---
 utilities/ovs-ofctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 6708b07..3023787 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2326,6 +2326,7 @@ ofctl_snoop(struct ovs_cmdl_context *ctx)
 {
 struct vconn *vconn;
 
+use_names = 0; /* don't show port and table names */
 open_vconn__(ctx->argv[1], SNOOP, );
 monitor_vconn(vconn, false, false);
 }
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ovs-ofctl: Fixed the "snoop" command of ovs-ofctl

[Ashish Varma]

Sure, I will update the comment in the code to explain the effect of using:
 "use_names = 0".
Also, would change the comment in the commit back to the original message.
Thanks,

On Wed, Apr 18, 2018 at 4:29 PM, Ben Pfaff <b...@ovn.org> wrote:

> On Tue, Apr 17, 2018 at 06:40:46PM -0700, Ashish Varma wrote:
> > In normal ovs-ofctl commands (e.g. add-flow), ovs-ofctl connects to
> > ovs-vswitchd process on “<ovs_rundir()>/.mgmt” unix socket.
> > In an output that contains a port or table, port name or table name can
> be
> > displayed, instead of their numbers, if the user turns on this feature.
> > (by -—names option) Also, this feature is enabled when interacting with
> a user
> > on console.  To fetch the names, ovs-ofctl sends TABLE FEATURE /
> > PORT DESCRIPTION request message to ovs-vswitchd process and expects a
> reply
> > on the connection.
> > When ovs-ofctl runs the snoop command, it connects to
> > “<ovs_rundir()>/.snoop” unix socket. ovs-vswitchd process
> would
> > not reply to the TABLE FEATURE / PORT DESCRIPTION request message on this
> > connection. It would only send any open flow message it receives from the
> > controller.
> > When using port/table name feature with snoop command, the print of open
> flow
> > message would call ‘tables_to_show()’/‘ports_to_show()’ which in turn
> would
> > send the request message on the snoop connection. ovs-vswitchd would not
> reply
> > back on this connection, but instead would keep sending the open flow
> messages
> > received from controller. ‘table_iterator_next()/port_iterator_next()’
> function
> > would loop for ever waiting for response.
> > The fix for this is to turn off display of table/port names when running
> > snoop command.
> >
> > Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
> > ---
> > v1-v2
> >
> > Added more description to the cause of the issue and reason to add the
> fix.
>
> Thanks for the update.
>
> I think I didn't describe my request on v1 very well.  I thought that
> the commit message was fine; it described the problem and the solution
> well.  I was asking for the comment in the code to describe the problem
> that it solved.  As is, the following comment:
> > +use_names = 0; /* don't show port and table names */
> explains the immediate effect, but not why.  The "why" is more important
> than the immediate effect, because the effect is pretty easy to
> understand if the reader looks at the comments on the definition of
> "use_names", but the reason is nonobvious.
>
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1] tests: Added NSH related unit test cases for datapath

[Ashish Varma]

Added test cases for encap, decap, replace and forwarding of NSH packets.
Also added a python script 'sendpkt.py' to send hex ethernet frames.

Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
---
 tests/automake.mk   |   4 +-
 tests/sendpkt.py|  94 
 tests/system-traffic.at | 163 
 3 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100755 tests/sendpkt.py

diff --git a/tests/automake.mk b/tests/automake.mk
index 780d3b8..0796123 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -400,7 +400,9 @@ CHECK_PYFILES = \
tests/test-unix-socket.py \
tests/test-unixctl.py \
tests/test-vlog.py \
-   tests/uuidfilt.py
+   tests/uuidfilt.py \
+   tests/sendpkt.py
+
 EXTRA_DIST += $(CHECK_PYFILES)
 PYCOV_CLEAN_FILES += $(CHECK_PYFILES:.py=.py,cover) .coverage
 
diff --git a/tests/sendpkt.py b/tests/sendpkt.py
new file mode 100755
index 000..6d16d0f
--- /dev/null
+++ b/tests/sendpkt.py
@@ -0,0 +1,94 @@
+#! /usr/bin/env python
+
+# Copyright (c) 2018 VMware, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+# This program can be used send L2-L7 protocol messages using the hex bytes
+# of the packet, to test simple protocol scenarios. (e.g. generate simple
+# nsh packets to test nsh match fields/actions)
+#
+# Currently, the script supports sending the packets starting from the
+# Ethernet header. As a part of future enchancement, raw ip packet support
+# can also be added, and that's why there is "-t"/"--type" option
+#
+
+
+import socket
+import struct
+import sys
+from optparse import OptionParser
+
+
+usage = "usage: %prog [OPTIONS] OUT-INTERFACE HEX-BYTES \n \
+ bytes in HEX-BYTES must be separated by space(s)"
+parser = OptionParser(usage=usage)
+parser.add_option("-t", "--type", type="string", dest="packet_type",
+  help="packet type ('eth' is the default PACKET_TYPE)",
+  default="eth")
+
+(options, args) = parser.parse_args()
+
+# validate the arguments
+if len(args) < 2:
+parser.print_help()
+sys.exit(1)
+
+# validate the "-t" or "--type" option
+if options.packet_type != "eth":
+parser.error('invalid argument to "-t"/"--type". Allowed value is "eth".')
+
+# store the hex bytes with 0x appended at the beginning
+# if not present in the user input and validate the hex bytes
+hex_list = []
+for a in args[1:]:
+if a[:2] != "0x":
+hex_byte = "0x" + a
+else:
+hex_byte = a
+try:
+temp = int(hex_byte, 0)
+except:
+parser.error("invalid hex byte " + a)
+
+if temp > 0xff:
+parser.error("hex byte " + a + " cannot be greater than 0xff!")
+
+hex_list.append(temp)
+
+pkt = "".join(map(chr, hex_list))
+
+try:
+sockfd = socket.socket(socket.AF_PACKET, socket.SOCK_RAW)
+except socket.error as msg:
+print 'unable to create socket! error eode: ' + str(msg[0]) + ' : '\
++ msg[1]
+sys.exit(2)
+
+try:
+sockfd.bind((args[0], 0))
+except socket.error as msg:
+print 'unable to bind socket! error eode: ' + str(msg[0]) + ' : '\
++ msg[1]
+sys.exit(2)
+
+try:
+sockfd.send(pkt)
+except socket.error as msg:
+print 'unable to send packet! error eode: ' + str(msg[0]) + ' : '\
++ msg[1]
+sys.exit(2)
+
+print 'send success!'
+sys.exit(0)
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 2afadec..58db095 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -4478,3 +4478,166 @@ NS_CHECK_EXEC([at_ns0], [ping -s 1600 -q -c 3 -i 0.3 -w 
2 10.2.2.2 | FORMAT_PING
 
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
+
+
+AT_BANNER([nsh-datapath])
+
+AT_SETUP([nsh - encap header])
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "0.0.0.0")
+ADD_VETH(p1, at_ns1, br0, "0.0.0.0")
+
+dnl The flow will encap a nsh header to the TCP syn packet
+dnl eth/ip/tcp --> OVS --> eth/nsh/eth/ip/tcp
+AT_CHECK([ovs-ofctl -Oopenfl

[ovs-dev] [PATCH v1] ofproto-dpif-upcall: fix for segmentation fault

[Ashish Varma]

Added check for NULL pointer on return from xlate_lookup_ofproto
function. Access to "ofproto" variable when NULL was causing segmentation
fault.

VMware-BZ: #2061914
Signed-off-by: Ashish Varma <ashishvarma@gmail.com>
---
 ofproto/ofproto-dpif-upcall.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 526be77..0079674 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2129,6 +2129,11 @@ revalidate_ukey__(struct udpif *udpif, const struct 
udpif_key *ukey,
 ofproto = xlate_lookup_ofproto(udpif->backer, , _in_port);
 
 ofpbuf_clear(odp_actions);
+
+if (!ofproto) {
+goto exit;
+}
+
 compose_slow_path(udpif, xoutp, , ctx.flow.in_port.odp_port,
   ofp_in_port, odp_actions,
   ofproto->up.slowpath_meter_id, >uuid);
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Error at ovsdb-cluster.at:204

[Ashish Varma]

Not sure if this is reported before. I am seeing failure (not always) at
the following line when running the test ten times in the main branch:

i=10;while [ $i -gt 0 ]; do make check TESTSUITEFLAGS='2485' | grep "2485:
OVSDB 3-server torture test"; i=$((i-1)); done
2485: OVSDB 3-server torture test - remove/re-add follower 2 FAILED (
ovsdb-cluster.at:204)
ERROR: 1 test was run,
1 failed unexpectedly.
make[3]: *** [check-local] Error 1
make[2]: *** [check-am] Error 2
make[1]: *** [check-recursive] Error 1
make: *** [check] Error 2
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok

In another instance, the test got skipped once:
i=10;while [ $i -gt 0 ]; do make check TESTSUITEFLAGS='2485' | grep "2485:
OVSDB 3-server torture test"; i=$((i-1)); done
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 skipped (
ovsdb-cluster.at:197)
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
2485: OVSDB 3-server torture test - remove/re-add follower 2 ok
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1] Documentation: Fixing some minor spelling mistakes and consistent usage of certain keywords.

[Ashish Varma]

Signed-off-by: Ashish Varma 
---
 Documentation/tutorials/ovs-conntrack.rst | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/tutorials/ovs-conntrack.rst 
b/Documentation/tutorials/ovs-conntrack.rst
index 07ea998..a86288f 100644
--- a/Documentation/tutorials/ovs-conntrack.rst
+++ b/Documentation/tutorials/ovs-conntrack.rst
@@ -31,7 +31,7 @@ etc., connections. (Connection tracking system supports 
tracking of both
 statefull and stateless protocols)
 
 This tutorial demonstrates how OVS can use the connection tracking system
-to match on the TCP segments from connection setup to connection teardown.
+to match on the TCP segments from connection setup to connection tear down.
 It will use OVS with the Linux kernel module as the datapath for this
 tutorial. (The datapath that utilizes the openvswitch kernel module to do
 the packet processing in the Linux kernel)
@@ -77,10 +77,10 @@ Possible values:
 - *snat*
 - *dnat*
 
-Each of these flags is preceded by either a "+" for a flag that
+Each of these flags are preceded by either a "+" for a flag that
 must be set, or a "-" for a flag that must be unset.
 Multiple flags can also be specified e.g. ct_state=+trk+new.
-We will see the usage of some these flags below. For a detailed
+We will see the usage of some of these flags below. For a detailed
 description, please see the OVS fields documentation at:
 http://openvswitch.org/support/dist-docs/ovs-fields.7.txt
 
@@ -283,7 +283,7 @@ packets from "left" to "right" and from "right" to "left"::
 Instead of adding these two flows, we will add flows to match on the
 states of the TCP segments.
 
-We will send the TCP connections setup segments namely:
+We will send the TCP connection setup segments namely:
 syn, syn-ack and ack between hosts 192.168.0.2 in the "left" namespace and
 10.0.0.2 in the "right" namespace.
 
@@ -294,7 +294,7 @@ First, let's add a flow to start "tracking" a packet 
received at OVS.
 To start tracking a packet, it first needs to match a flow, which has action
 as "ct".  This action sends the packet through the connection tracker.  To
 identify that a packet is an "untracked" packet, the ct_state in the flow
-match filed must be set to "-trk", which means it is not a tracked packet.
+match field must be set to "-trk", which means it is not a tracked packet.
 Once the packet is sent to the connection tracker, then only we will know about
 its conntrack state.  (i.e. whether this packet represents start of a new
 connection or the packet belongs to an existing connection or it is
@@ -353,8 +353,8 @@ The conntrack module will now have an entry for this 
connection::
 Note: At this stage, if the TCP syn packet is re-transmitted, it will again
 match flow #1 (since a new packet is untracked) and it will match flow #2.
 The reason it will match flow #2 is that although conntrack has information
-about the connection, but it is not in established state, therefore it matches
-the "new" state again.
+about the connection, but it is not in "ESTABLISHED" state, therefore it
+matches the "new" state again.
 
 Next for the TCP syn-ack from the opposite/server direction, we need
 following flows at OVS::
@@ -418,7 +418,7 @@ conntrack entry::
  reply=(src=10.0.0.2,dst=192.168.0.2,sport=2048,dport=1024), \
  protoinfo=(state=ESTABLISHED)
 
-The conntrck state stays in "ESTABLISHED" state, but now since it has received
+The conntrack state stays in "ESTABLISHED" state, but now since it has received
 the ack from client, it will stay in this state for a longer time even without
 receiving any data on this connection.
 
@@ -445,7 +445,7 @@ The acknowledgement for the data would hit flow #3 and flow 
#4.
 
 TCP Connection Teardown
 ~~~
-There are different ways to teardown TCP connection. We will teardown the
+There are different ways to tear down TCP connection. We will tear down the
 connection by sending "fin" from client, "fin-ack" from server followed
 by the last "ack" by client.
 
@@ -507,7 +507,7 @@ conntrack entry::
 Summary
 ---
 
-Following table summarizes the TCP segments exhanged against the flow
+Following table summarizes the TCP segments exchanged against the flow
 match fields
 
   +---+---+
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1] Documentation: Fixing some minor spelling mistakes and consistent usage of certain keywords.

[Ashish Varma]

I think I got misled by the 's' in the flags. I will submit a v2 patch with
this sentence reverted back to it's original version.

Thanks for the review.

On Tue, Nov 13, 2018 at 10:03 AM Mark Michelson  wrote:

> Thanks for the patch. I have one finding down below.
>
> On 11/12/2018 04:55 PM, Ashish Varma wrote:
> > Signed-off-by: Ashish Varma 
> > ---
> >   Documentation/tutorials/ovs-conntrack.rst | 20 ++--
> >   1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/Documentation/tutorials/ovs-conntrack.rst
> b/Documentation/tutorials/ovs-conntrack.rst
> > index 07ea998..a86288f 100644
> > --- a/Documentation/tutorials/ovs-conntrack.rst
> > +++ b/Documentation/tutorials/ovs-conntrack.rst
> > @@ -31,7 +31,7 @@ etc., connections. (Connection tracking system
> supports tracking of both
> >   statefull and stateless protocols)
> >
> >   This tutorial demonstrates how OVS can use the connection tracking
> system
> > -to match on the TCP segments from connection setup to connection
> teardown.
> > +to match on the TCP segments from connection setup to connection tear
> down.
> >   It will use OVS with the Linux kernel module as the datapath for this
> >   tutorial. (The datapath that utilizes the openvswitch kernel module to
> do
> >   the packet processing in the Linux kernel)
> > @@ -77,10 +77,10 @@ Possible values:
> >   - *snat*
> >   - *dnat*
> >
> > -Each of these flags is preceded by either a "+" for a flag that
> > +Each of these flags are preceded by either a "+" for a flag that
>
> The original version of this sentence is correct. Or at least
> traditionally, the original version is correct:
> https://www.grammar.com/each-singular-or-plural/
>
> There's an argument to be made that your version could also be
> acceptable, but I think the original version of the sentence doesn't
> need to be changed.
>
> >   must be set, or a "-" for a flag that must be unset.
> >   Multiple flags can also be specified e.g. ct_state=+trk+new.
> > -We will see the usage of some these flags below. For a detailed
> > +We will see the usage of some of these flags below. For a detailed
> >   description, please see the OVS fields documentation at:
> >   http://openvswitch.org/support/dist-docs/ovs-fields.7.txt
> >
> > @@ -283,7 +283,7 @@ packets from "left" to "right" and from "right" to
> "left"::
> >   Instead of adding these two flows, we will add flows to match on the
> >   states of the TCP segments.
> >
> > -We will send the TCP connections setup segments namely:
> > +We will send the TCP connection setup segments namely:
> >   syn, syn-ack and ack between hosts 192.168.0.2 in the "left" namespace
> and
> >   10.0.0.2 in the "right" namespace.
> >
> > @@ -294,7 +294,7 @@ First, let's add a flow to start "tracking" a packet
> received at OVS.
> >   To start tracking a packet, it first needs to match a flow, which has
> action
> >   as "ct".  This action sends the packet through the connection
> tracker.  To
> >   identify that a packet is an "untracked" packet, the ct_state in the
> flow
> > -match filed must be set to "-trk", which means it is not a tracked
> packet.
> > +match field must be set to "-trk", which means it is not a tracked
> packet.
> >   Once the packet is sent to the connection tracker, then only we will
> know about
> >   its conntrack state.  (i.e. whether this packet represents start of a
> new
> >   connection or the packet belongs to an existing connection or it is
> > @@ -353,8 +353,8 @@ The conntrack module will now have an entry for this
> connection::
> >   Note: At this stage, if the TCP syn packet is re-transmitted, it will
> again
> >   match flow #1 (since a new packet is untracked) and it will match flow
> #2.
> >   The reason it will match flow #2 is that although conntrack has
> information
> > -about the connection, but it is not in established state, therefore it
> matches
> > -the "new" state again.
> > +about the connection, but it is not in "ESTABLISHED" state, therefore it
> > +matches the "new" state again.
> >
> >   Next for the TCP syn-ack from the opposite/server direction, we need
> >   following flows at OVS::
> > @@ -418,7 +418,7 @@ conntrack entry::
> >reply=(src=10.0.0.2,dst=192.168.0.2,sport=2048,dport=1024), \
> >protoin

Re: [ovs-dev] [PATCH v1] ofp-monitor: Added support for OpenFlow 1.4+ Flow Monitor

[Ashish Varma]

Thanks for the feedback. Would revert back shortly with replies and few
questions followed by the v2 patch.

On Mon, Dec 10, 2018 at 1:54 PM Ben Pfaff  wrote:

> On Wed, Nov 28, 2018 at 10:30:07AM -0800, Ashish Varma wrote:
> > OVS supports Nicira version of Flow Monitor feature which allows an
> OpenFlow
> > controller to keep track of any changes in the flow table. (The changes
> can
> > done by the controller itself or by any other controller connected to
> OVS.)
> > This patch adds support for the OpenFlow 1.4+ OFPMP_FLOW_MONITOR
> multipart
> > message.
> > Also added support in "ovs-ofctl monitor" to send OpenFlow 1.4+ messages
> to
> > OVS.
> > Added unit test cases to test the OpenFlow version of Flow Monitor
> messages.
> >
> > Signed-off-by: Ashish Varma 
>
> Thanks a lot for working on this.  The OF1.4+ flow monitor support was
> based on the OVS design for the same feature, so it's a shame that we
> didn't implement it early on.
>
> In openflow-1.4.h, please align the comments on members of structs and
> enumerations to some common column, as is the usual practice in OVS
> code, e.g. instead of this:
>
> /* struct ofp14_flow_update_full. */
> OFPFME_INITIAL = 0, /* Flow present when flow monitor created. */
> OFPFME_ADDED = 1, /* Flow was added. */
> OFPFME_REMOVED = 2, /* Flow was removed. */
> OFPFME_MODIFIED = 3, /* Flow instructions were changed. */
> /* struct ofp14_flow_update_abbrev. */
> OFPFME_ABBREV = 4, /* Abbreviated reply. */
> /* struct ofp14_flow_update_header. */
> OFPFME_PAUSED = 5, /* Monitoring paused (out of buffer space). */
> OFPFME_RESUMED = 6, /* Monitoring resumed. */
>
> more like this (also adding some blank lines for clarity):
>
> /* struct ofp14_flow_update_full. */
> OFPFME_INITIAL = 0, /* Flow present when flow monitor created.
> */
> OFPFME_ADDED = 1,   /* Flow was added. */
> OFPFME_REMOVED = 2, /* Flow was removed. */
> OFPFME_MODIFIED = 3,/* Flow instructions were changed. */
>
> /* struct ofp14_flow_update_abbrev. */
> OFPFME_ABBREV = 4,  /* Abbreviated reply. */
>
> /* struct ofp14_flow_update_header. */
> OFPFME_PAUSED = 5,  /* Monitoring paused (out of buffer
> space). */
> OFPFME_RESUMED = 6, /* Monitoring resumed. */
>
> Also please use the "14" infix for these enumeration members,
> e.g. OFPFME14_INITIAL.
>
> The OFPUTIL_FMF_* flags aren't really abstracted from NXFMF_* and
> OFPFMF14_*; they're just a copy of the OFPFMF14_* ones with the names
> changed.  I think it might be better to just use OFPFMF14_* with a note
> that NXFMF_* doesn't have NXFMF_ONLY_OWN.
>
> It's usually unnecessary to use a union of an ofp_port_t and an
> ofp11_port_t, because OVS can usually reject unsupported ports when it
> parses them from OpenFlow using ofputil_port_from_ofp11().  Maybe there
> is some exception here but that's not obvious to me yet.
>
> I don't see how a piece of code that looks at an
> ofputil_flow_monitor_request can tell whether to look at
> out_port.ofp_port or out_port.ofp11_port.
>
> Our usual style is to start a comment with a capital letter and end it
> with a period:
>
> /* there is one to one mapping between ofp14_flow_update_event and
>  * ofputil_flow_update_event */
>
> The name ofputil_get_util_flow_update_event_from_nx_event() is pretty
> long and I don't know what the _util_ part of it is there for.
>
> In new code, please try to declare (and initialize) variables at their
> first use, when possible, rather than the older style we used of
> declaring variables at the top of a block.
>
> I think that ofputil_start_flow_update() should always use
> NXST_FLOW_MONITOR_REPLY for OpenFlow 1.0, so it seems to me that it
> should check for OFPUTIL_P_OF10_ANY rather than OFPUTIL_P_OF10_STD_ANY.
> (Or it could accept an OpenFlow version instead of a protocol.)
>
> I don't understand why ofputil_append_flow_update() checks for
> OFPUTIL_P_OF10_STD in particular.  It seems like any OF1.0 protocol
> would make sense.  (Or it could accept an OpenFlow version instead of a
> protocol.)
>
> In general I'm not sure of the rationale here for how OpenFlow versions
> are handled.  Currently, it looks like OVS supports flow monitoring in
> OF1.0 only.  The OpenFlow specification defines flow monitoring for
> OF1.4 and later and for OF1.3 as an "official extension" in extensions
> pack 2 (which you can find at
>
> https://www.opennetworking.org/wp-content/uploads/2014/10/openflow-extensions-1.3.x-pack2.zip
> ).
> So we should use the official specs for 1.3 and later.  For 1.1 and 1.2,
> we have a choice of supporting our extension or the OpenFlow version.  I
> don't have a particular preference.
>
> Thanks!  I'll look forward to the next revision.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1] ofctl.man: Minor change to correct the "out_group" field usage text.

[Ashish Varma]

Right now the man page of ovs-ofctl has "out_group=port". Correcting the
output to group instead of port.

Signed-off-by: Ashish Varma 
---
 utilities/ovs-ofctl.8.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index ea55008..59b6e1a 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1952,7 +1952,7 @@ If set, a matching flow must include an output action to 
\fIport\fR,
 which must be an OpenFlow port number or name (e.g. \fBlocal\fR).
 .
 .TP
-\fBout_group=\fIport\fR
+\fBout_group=\fIgroup\fR
 If set, a matching flow must include an \fBgroup\fR action naming
 \fIgroup\fR, which must be an OpenFlow group number.  This field
 is supported in Open vSwitch 2.5 and later and requires OpenFlow 1.1
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] Documentation: Fixing some minor spelling mistakes and consistent usage of certain keywords.

[Ashish Varma]

Signed-off-by: Ashish Varma 
---
v1-v2

Reverted the sentence "Each of these flags are" back to 
"Each of these flags is".
---
 Documentation/tutorials/ovs-conntrack.rst | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/tutorials/ovs-conntrack.rst 
b/Documentation/tutorials/ovs-conntrack.rst
index 07ea998..ac5ca9a 100644
--- a/Documentation/tutorials/ovs-conntrack.rst
+++ b/Documentation/tutorials/ovs-conntrack.rst
@@ -31,7 +31,7 @@ etc., connections. (Connection tracking system supports 
tracking of both
 statefull and stateless protocols)
 
 This tutorial demonstrates how OVS can use the connection tracking system
-to match on the TCP segments from connection setup to connection teardown.
+to match on the TCP segments from connection setup to connection tear down.
 It will use OVS with the Linux kernel module as the datapath for this
 tutorial. (The datapath that utilizes the openvswitch kernel module to do
 the packet processing in the Linux kernel)
@@ -80,7 +80,7 @@ Possible values:
 Each of these flags is preceded by either a "+" for a flag that
 must be set, or a "-" for a flag that must be unset.
 Multiple flags can also be specified e.g. ct_state=+trk+new.
-We will see the usage of some these flags below. For a detailed
+We will see the usage of some of these flags below. For a detailed
 description, please see the OVS fields documentation at:
 http://openvswitch.org/support/dist-docs/ovs-fields.7.txt
 
@@ -283,7 +283,7 @@ packets from "left" to "right" and from "right" to "left"::
 Instead of adding these two flows, we will add flows to match on the
 states of the TCP segments.
 
-We will send the TCP connections setup segments namely:
+We will send the TCP connection setup segments namely:
 syn, syn-ack and ack between hosts 192.168.0.2 in the "left" namespace and
 10.0.0.2 in the "right" namespace.
 
@@ -294,7 +294,7 @@ First, let's add a flow to start "tracking" a packet 
received at OVS.
 To start tracking a packet, it first needs to match a flow, which has action
 as "ct".  This action sends the packet through the connection tracker.  To
 identify that a packet is an "untracked" packet, the ct_state in the flow
-match filed must be set to "-trk", which means it is not a tracked packet.
+match field must be set to "-trk", which means it is not a tracked packet.
 Once the packet is sent to the connection tracker, then only we will know about
 its conntrack state.  (i.e. whether this packet represents start of a new
 connection or the packet belongs to an existing connection or it is
@@ -353,8 +353,8 @@ The conntrack module will now have an entry for this 
connection::
 Note: At this stage, if the TCP syn packet is re-transmitted, it will again
 match flow #1 (since a new packet is untracked) and it will match flow #2.
 The reason it will match flow #2 is that although conntrack has information
-about the connection, but it is not in established state, therefore it matches
-the "new" state again.
+about the connection, but it is not in "ESTABLISHED" state, therefore it
+matches the "new" state again.
 
 Next for the TCP syn-ack from the opposite/server direction, we need
 following flows at OVS::
@@ -418,7 +418,7 @@ conntrack entry::
  reply=(src=10.0.0.2,dst=192.168.0.2,sport=2048,dport=1024), \
  protoinfo=(state=ESTABLISHED)
 
-The conntrck state stays in "ESTABLISHED" state, but now since it has received
+The conntrack state stays in "ESTABLISHED" state, but now since it has received
 the ack from client, it will stay in this state for a longer time even without
 receiving any data on this connection.
 
@@ -445,7 +445,7 @@ The acknowledgement for the data would hit flow #3 and flow 
#4.
 
 TCP Connection Teardown
 ~~~
-There are different ways to teardown TCP connection. We will teardown the
+There are different ways to tear down TCP connection. We will tear down the
 connection by sending "fin" from client, "fin-ack" from server followed
 by the last "ack" by client.
 
@@ -507,7 +507,7 @@ conntrack entry::
 Summary
 ---
 
-Following table summarizes the TCP segments exhanged against the flow
+Following table summarizes the TCP segments exchanged against the flow
 match fields
 
   +---+---+
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1] ofp-monitor: Added support for OpenFlow 1.4+ Flow Monitor

[Ashish Varma]

OVS supports Nicira version of Flow Monitor feature which allows an OpenFlow
controller to keep track of any changes in the flow table. (The changes can
done by the controller itself or by any other controller connected to OVS.)
This patch adds support for the OpenFlow 1.4+ OFPMP_FLOW_MONITOR multipart
message.
Also added support in "ovs-ofctl monitor" to send OpenFlow 1.4+ messages to
OVS.
Added unit test cases to test the OpenFlow version of Flow Monitor messages.

Signed-off-by: Ashish Varma 
---
 include/openflow/openflow-1.4.h   |  79 ++
 include/openvswitch/ofp-monitor.h |  73 -
 lib/ofp-monitor.c | 553 --
 lib/ofp-print.c   |  10 +-
 ofproto/connmgr.c | 100 ---
 ofproto/connmgr.h |  11 +-
 ofproto/ofproto-provider.h|   7 +-
 ofproto/ofproto.c |  95 ---
 tests/ofproto.at  | 381 ++
 utilities/ovs-ofctl.8.in  |   3 +
 utilities/ovs-ofctl.c |  20 +-
 11 files changed, 1099 insertions(+), 233 deletions(-)

diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
index 9399950..295decb 100644
--- a/include/openflow/openflow-1.4.h
+++ b/include/openflow/openflow-1.4.h
@@ -381,4 +381,83 @@ enum ofp14_flow_monitor_flags {
  */
 };
 
+/* OFPMP_FLOW_MONITOR reply header.
+ *
+ * The body of an OFPMP_FLOW_MONITOR reply is an array of variable-length
+ * structures, each of which begins with this header. The ’length’ member
+ * may be used to traverse the array, and the ’event’ member may be used to
+ * determine the particular structure.
+ *
+ * Every instance is a multiple of 8 bytes long. */
+struct ofp14_flow_update_header {
+ovs_be16 length; /* Length of this entry. */
+ovs_be16 event; /* One of OFPFME_*. */
+/* ...other data depending on ’event’... */
+};
+OFP_ASSERT(sizeof(struct ofp14_flow_update_header) == 4);
+
+/* ’event’ values in struct ofp14_flow_update_header. */
+enum ofp14_flow_update_event {
+/* struct ofp14_flow_update_full. */
+OFPFME_INITIAL = 0, /* Flow present when flow monitor created. */
+OFPFME_ADDED = 1, /* Flow was added. */
+OFPFME_REMOVED = 2, /* Flow was removed. */
+OFPFME_MODIFIED = 3, /* Flow instructions were changed. */
+/* struct ofp14_flow_update_abbrev. */
+OFPFME_ABBREV = 4, /* Abbreviated reply. */
+/* struct ofp14_flow_update_header. */
+OFPFME_PAUSED = 5, /* Monitoring paused (out of buffer space). */
+OFPFME_RESUMED = 6, /* Monitoring resumed. */
+};
+
+/* OFPMP_FLOW_MONITOR reply for OFPFME_INITIAL, OFPFME_ADDED, OFPFME_REMOVED,
+ * and OFPFME_MODIFIED. */
+struct ofp14_flow_update_full {
+ovs_be16 length; /* Length is 32 + match + instructions. */
+ovs_be16 event; /* One of OFPFME_*. */
+uint8_t table_id; /* ID of flow’s table. */
+uint8_t reason; /* OFPRR_* for OFPFME_REMOVED, else zero. */
+ovs_be16 idle_timeout; /* Number of seconds idle before expiration. */
+ovs_be16 hard_timeout; /* Number of seconds before expiration. */
+ovs_be16 priority; /* Priority of the entry. */
+uint8_t zeros[4]; /* Reserved, currently zeroed. */
+ovs_be64 cookie; /* Opaque controller-issued identifier. */
+/* Followed by:
+ * Fields to match. Variable size.
+ * //struct ofp11_match match;
+ * Instruction set.
+ * If OFPFMF_INSTRUCTIONS was not specified, or ’event’ is
+ * OFPFME_REMOVED, no instructions are included.
+ *
+ * //struct ofp11_instruction instructions[0];
+ */
+};
+OFP_ASSERT(sizeof(struct ofp14_flow_update_full) == 24);
+
+/* OFPMP_FLOW_MONITOR reply for OFPFME_ABBREV.
+ *
+ * When the controller does not specify OFPFMF_NO_ABBREV in a monitor request,
+ * any flow tables changes due to the controller’s own requests (on the same
+ * OpenFlow channel) will be abbreviated, when possible, to this form, which
+ * simply specifies the ’xid’ of the OpenFlow request
+ * (e.g. an OFPT_FLOW_MOD) that caused the change.
+ * Some changes cannot be abbreviated and will be sent in full.
+ */
+struct ofp14_flow_update_abbrev {
+ovs_be16 length; /* Length is 8. */
+ovs_be16 event; /* OFPFME_ABBREV. */
+ovs_be32 xid; /* Controller-specified xid from flow_mod. */
+};
+OFP_ASSERT(sizeof(struct ofp14_flow_update_abbrev) == 8);
+
+/* OFPMP_FLOW_MONITOR reply for OFPFME_PAUSED and OFPFME_RESUMED.
+ */
+struct ofp14_flow_update_paused {
+ovs_be16 length; /* Length is 8. */
+ovs_be16 event; /* One of OFPFME_*. */
+uint8_t zeros[4]; /* Reserved, currently zeroed. */
+};
+OFP_ASSERT(sizeof(struct ofp14_flow_update_paused) == 8);
+
+
 #endif /* openflow/openflow-1.4.h */
diff --git a/include/openvswitch/ofp-monitor.h 
b/include/openvswitch/ofp-monitor.h
index 5951260..1d4ec50 100644
--- a/include/openvswitch/ofp-monitor.h
+++ b/include/openvswitch/ofp-monitor.h
@@ -23,6 +23,9 @@

[ovs-dev] [PATCH v1] ofp-monitor: Fixed the usage of 'usable_protocols' variable in 'parse_flow_monitor_request' function.

[Ashish Varma]

'usable_protocols' is now getting set to OFPUTIL_P_OF10_ANY on return from
'parse_flow_monitor_request' function. The calling function now checks for the
value in this variable against the 'allowed_protocols' variable.
Also a check is added for a match field which is not supported in OpenFlow 1.0
and return an error.
Modified the man page of ovs-ofctl to reflect Flow Monitor support as
OpenFlow 1.0 Nicira extension only.

Signed-off-by: Ashish Varma 
---
 lib/ofp-monitor.c| 8 
 utilities/ovs-ofctl.8.in | 3 ++-
 utilities/ovs-ofctl.c| 8 
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/lib/ofp-monitor.c b/lib/ofp-monitor.c
index aeebd7e..d24ffe5 100644
--- a/lib/ofp-monitor.c
+++ b/lib/ofp-monitor.c
@@ -469,6 +469,8 @@ parse_flow_monitor_request__(struct 
ofputil_flow_monitor_request *fmr,
 fmr->table_id = 0xff;
 match_init_catchall(>match);
 
+/* A match field may reduce the usable protocols. */
+*usable_protocols = OFPUTIL_P_ANY;
 while (ofputil_parse_key_value(, , )) {
 const struct ofp_protocol *p;
 char *error = NULL;
@@ -493,6 +495,10 @@ parse_flow_monitor_request__(struct 
ofputil_flow_monitor_request *fmr,
 } else if (mf_from_name(name)) {
 error = ofp_parse_field(mf_from_name(name), value, port_map,
 >match, usable_protocols);
+if (!error && !(*usable_protocols & OFPUTIL_P_OF10_ANY)) {
+return xasprintf("%s: match field is not supported for "
+ "flow monitor", name);
+}
 } else {
 if (!*value) {
 return xasprintf("%s: field %s missing value", str_, name);
@@ -514,6 +520,8 @@ parse_flow_monitor_request__(struct 
ofputil_flow_monitor_request *fmr,
 return error;
 }
 }
+/* Flow Monitor is supported in OpenFlow 1.0 only. */
+*usable_protocols = OFPUTIL_P_OF10_ANY;
 return NULL;
 }
 
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 60b4a1c..cb5c612 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -578,7 +578,8 @@ monitoring will not show any traffic.
 .IP "\fBmonitor \fIswitch\fR [\fImiss-len\fR] [\fBinvalid_ttl\fR] 
[\fBwatch:\fR[\fIspec\fR...]]"
 Connects to \fIswitch\fR and prints to the console all OpenFlow
 messages received.  Usually, \fIswitch\fR should specify the name of a
-bridge in the \fBovs\-vswitchd\fR database.
+bridge in the \fBovs\-vswitchd\fR database. This is available only in
+OpenFlow 1.0 as Nicira extension.
 .IP
 If \fImiss-len\fR is provided, \fBovs\-ofctl\fR sends an OpenFlow ``set
 configuration'' message at connection setup time that requests
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 63620e4..926a4f6 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2286,6 +2286,14 @@ ofctl_monitor(struct ovs_cmdl_context *ctx)
 ovs_fatal(0, "%s", error);
 }
 
+if (!(usable_protocols & allowed_protocols)) {
+char *allowed_s =
+ofputil_protocols_to_string(allowed_protocols);
+char *usable_s = ofputil_protocols_to_string(usable_protocols);
+ovs_fatal(0, "none of the usable protocols (%s) are among "
+"the allowed protocols (%s)", usable_s, allowed_s);
+}
+
 msg = ofpbuf_new(0);
 ofputil_append_flow_monitor_request(, msg);
 dump_transaction(vconn, msg);
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ofp-monitor: Added support for OpenFlow 1.4+ Flow Monitor

[Ashish Varma]

On Tue, Feb 5, 2019 at 4:41 PM Ben Pfaff  wrote:

> On Tue, Feb 05, 2019 at 04:26:56PM -0800, Ashish Varma wrote:
> > In the parse_flow_monitor_request(), usable_protocols is an out
> parameter.
> > (and is set in parse_flow_monitor_request() only if a match field is
> > provided. )
> > allowed_versions on the other hand is used only in the ofctl_monitor
> > function. That is the reason for the check in ofctl_monitor.
>
> The usual way we handle this sort of version dependency is a bitmap like
> 'usable_protocols'.  It works well for flows, for example.  You can see
> lots of examples in ovs-ofctl.c if you search for the identifier
> usable_protocols.
>
> Looking closer at this instance, the existing code is buggy.  Currently,
> only OpenFlow 1.0 should be supported, because that's the only protocol
> where OVS actually supports flow monitoring.
> parse_flow_monitor_request() should be returning that consistently as
> the out parameter (as you noted).  Or it should be returning 0 if it's
> impossible to request a flow monitor at all (in the case where OF1.0
> can't support whatever field is specified when
> parse_flow_monitor_request__ parses a field).  But it's buggy and
> nothing ever properly initializes it.  That bug is masked by another
> bug: nothing ever checks it, either!  And, finally, there is a third
> bug: ovs-ofctl calls ofputil_append_flow_monitor_request() and blindly
> always uses OF1.0, which might not be the protocol actually in use on
> the vconn.
>
> The manpage for the ovs-ofctl monitor command doesn't say that "watch:"
> is OF1.0 only or that it is an Open vSwitch extension, either, although
> it should.
>
> It would be for the best if we can fix all these bugs before we add
> support for OF1.4+ flow monitor, and then backport the bug fixes.  Does
> my description of the problems make sense?  Can you tackle these
> problems too?
>
>
Sure. I will try and fix this before the support for OF1.4+ flow monitor.
Thanks for explaining the issue.



> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ofp-monitor: Added support for OpenFlow 1.4+ Flow Monitor

[Ashish Varma]

Thanks for the review!

In the parse_flow_monitor_request(), usable_protocols is an out parameter.
(and is set in parse_flow_monitor_request() only if a match field is
provided. )
allowed_versions on the other hand is used only in the ofctl_monitor
function. That is the reason for the check in ofctl_monitor.

Would add the examples in tests/ofp-print.at

I will add the monitoring support for OpenFlow 1.1, 1.2 and 1.3 as a
separate patch.

Thanks,
Ashish



On Mon, Feb 4, 2019 at 3:48 PM Ben Pfaff  wrote:

> On Thu, Jan 31, 2019 at 03:49:39PM -0800, Ashish Varma wrote:
> > OVS supports Nicira version of Flow Monitor feature which allows an
> OpenFlow
> > controller to keep track of any changes in the flow table. (The changes
> can
> > done by the controller itself or by any other controller connected to
> OVS.)
> > This patch adds support for the OpenFlow 1.4+ OFPMP_FLOW_MONITOR
> multipart
> > message.
> > Also added support in "ovs-ofctl monitor" to send OpenFlow 1.4+ messages
> to
> > OVS.
> > Added unit test cases to test the OpenFlow version of Flow Monitor
> messages.
> >
> > Signed-off-by: Ashish Varma 
>
> Thanks for the revision!  It will be nice to finally have this feature!
>
> Since parse_flow_monitor_request() already has a 'usable_protocols'
> parameter, I would suggest using that to limit the support for
> out_group, rather than putting a special case into ofctl_monitor().
>
> I would add an example of the new message to tests/ofp-print.at.
>
> I think we talked about supporting flow monitors in OF1.1, 1.2, and
> 1.3.  I seem to recall that you plan to add that afterward, in a
> separate patch.  Is that correct?  I hope that, if so, you can start
> work on it soon after this patch is accepted.
>
> I would add an item to NEWS.
>
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ofp-monitor: Added support for OpenFlow 1.4+ Flow Monitor

[Ashish Varma]

OVS supports Nicira version of Flow Monitor feature which allows an OpenFlow
controller to keep track of any changes in the flow table. (The changes can
done by the controller itself or by any other controller connected to OVS.)
This patch adds support for the OpenFlow 1.4+ OFPMP_FLOW_MONITOR multipart
message.
Also added support in "ovs-ofctl monitor" to send OpenFlow 1.4+ messages to
OVS.
Added unit test cases to test the OpenFlow version of Flow Monitor messages.

Signed-off-by: Ashish Varma 
---
v1-v2:  

 Aligned the comments on members of structs and enumerations to  
 a common column.

 Added "14" infix for enumeration members of OpenFlow 1.4.

 Removed the union of an ofp_port_t and an ofp11_port_t as out_port in
 "ofputil_flow_monitor_request". 

 Removed "ofputil_flow_monitor_flags" enum and use "ofp14_flow_monitor_flags" 
 flags directly which is a superset of "nx_flow_monitor_flags".

 Changed comments to start with a capital letter and end with periods.

 Renamed "ofputil_get_util_flow_update_event_from_nx_event()" function to
 "ofputil_get_flow_update_event_from_nx_event".

 Declared and initialized variables at their first use.

 Used OFPUTIL_P_OF10_ANY rather than OFPUTIL_P_OF10_STD_ANY or any specific
 OFPUTIL_P_OF10_* protocol.

 Fixed parsing of the flags in the "delete" command of the Flow Monitor 
 request.

 Fixed the validation of "out_group" in ovs-ofctl monitor command.

 In function "handle_flow_monitor_request", changed "b->data" to "b->head" in 
 "ofconn_send_error(ofconn, b->data, error)" as ofpbuf b would be pulled by 
 the "ofputil_decode_flow_monitor_request" function.

 The current patch adds functionality for Flow Monitoring in OpenFlow version
 1.4 and later. I will have a separate patch for adding support for OpenFlow
 version 1.3 based on the OpenFlow extension pack 2 and would apply the 
 extension to version 1.1 and 1.2 as well. The 1.3 OpenFlow extension pack 2
 is more aligned with the NSXT FLOW_MONITOR implementation.
---
 include/openflow/openflow-1.4.h   |  94 ++-
 include/openvswitch/ofp-monitor.h |  49 +++-
 lib/ofp-monitor.c | 534 +++---
 lib/ofp-print.c   |  10 +-
 ofproto/connmgr.c |  98 ---
 ofproto/connmgr.h |  11 +-
 ofproto/ofproto-provider.h|   7 +-
 ofproto/ofproto.c | 124 +
 tests/ofproto.at  | 381 +++
 utilities/ovs-ofctl.8.in  |   3 +
 utilities/ovs-ofctl.c |  19 +-
 11 files changed, 1079 insertions(+), 251 deletions(-)

diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
index 9399950..04d1290 100644
--- a/include/openflow/openflow-1.4.h
+++ b/include/openflow/openflow-1.4.h
@@ -358,7 +358,7 @@ OFP_ASSERT(sizeof(struct ofp14_flow_monitor_request) == 16);
 
 /* Flow monitor commands */
 enum ofp14_flow_monitor_command {
-OFPFMC14_ADD = 0, /* New flow monitor. */
+OFPFMC14_ADD = 0,/* New flow monitor. */
 OFPFMC14_MODIFY = 1, /* Modify existing flow monitor. */
 OFPFMC14_DELETE = 2, /* Delete/cancel existing flow monitor. */
 };
@@ -367,18 +367,100 @@ enum ofp14_flow_monitor_command {
 enum ofp14_flow_monitor_flags {
 /* When to send updates. */
 /* Common to NX and OpenFlow 1.4 */
-OFPFMF14_INITIAL = 1 << 0, /* Initially matching flows. */
-OFPFMF14_ADD = 1 << 1, /* New matching flows as they are added. */
-OFPFMF14_REMOVED = 1 << 2, /* Old matching flows as they are removed. 
*/
-OFPFMF14_MODIFY = 1 << 3,  /* Matching flows as they are changed. */
+OFPFMF14_INITIAL = 1 << 0,/* Initially matching flows. */
+OFPFMF14_ADD = 1 << 1,/* New matching flows as they are added. */
+OFPFMF14_REMOVED = 1 << 2,/* Old matching flows as they are removed. */
+OFPFMF14_MODIFY = 1 << 3, /* Matching flows as they are changed. */
 
 /* What to include in updates. */
 /* Common to NX and OpenFlow 1.4 */
 OFPFMF14_INSTRUCTIONS = 1 << 4, /* If set, instructions are included. */
 OFPFMF14_NO_ABBREV = 1 << 5,/* If set, include own changes in full. */
-/* OpenFlow 1.4 */
+
+/* Only in OpenFlow 1.4, NXFMF_* version does not have NXFMF_ONLY_OWN */
 OFPFMF14_ONLY_OWN = 1 << 6, /* If set, don't include other controllers.
  */
 };
 
+/* OFPMP_FLOW_MONITOR reply header.
+ *
+ * The body of an OFPMP_FLOW_MONITOR reply is an array of variable-length
+ * structures, each of which begins with this header. The ’length’ member
+ * may be used to traverse the array, and the ’event’ member may be used to
+ * determine the particular struct

[ovs-dev] [PATCH v1] ofproto-dpif-trace: Fix for the segmentation fault in ofproto_trace().

[Ashish Varma]

Added the check for NULL in "next_ct_states" argument passed to the
"ofproto_trace()" function. Under normal scenario, this is non-NULL. A NULL
"next_ct_states" argument is passed from the "upcall_xlate()" function on
encountering XLATE_RECURSION_TOO_DEEP or XLATE_TOO_MANY_RESUBMITS error.

VMware-BZ: #2282287
Signed-off-by: Ashish Varma 
---
 ofproto/ofproto-dpif-trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index eca61ce..4a981e1 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -740,7 +740,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct 
flow *flow,
 ds_put_format(output, "\nrecirc(%#"PRIx32")",
   recirc_node->recirc_id);
 
-if (recirc_node->type == OFT_RECIRC_CONNTRACK) {
+if (next_ct_states && recirc_node->type == OFT_RECIRC_CONNTRACK) {
 uint32_t ct_state;
 if (ovs_list_is_empty(next_ct_states)) {
 ct_state = CS_TRACKED | CS_NEW;
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] rhel: Fixed a bug for checking the correct major version and revision.

[Ashish Varma]

Fixed a bug where checking for major version 3.10 and major revision not
equal to 327 or 693 or 957 should have gone to the default else at the end.
In the current code, the default else condition will not get executed
for kernel with major version 3.10 and major revision not equal
to 327/693/957 resulting in failure to load the kernel module.

Signed-off-by: Ashish Varma 
---
 rhel/openvswitch-kmod-fedora.spec.in | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/rhel/openvswitch-kmod-fedora.spec.in 
b/rhel/openvswitch-kmod-fedora.spec.in
index 92d763f..27f443a 100644
--- a/rhel/openvswitch-kmod-fedora.spec.in
+++ b/rhel/openvswitch-kmod-fedora.spec.in
@@ -90,13 +90,12 @@ if grep -qs "suse" /etc/os-release; then
 if [ -x "%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh" ]; then
 %{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh
 fi
-elif [ "$mainline_major" = "3" ] && [ "$mainline_minor" = "10" ]; then
-if [ "$major_rev" = "327" ] || [ "$major_rev" = "693" ] || \
-   [ "$major_rev" = "957" ]; then
-# For RHEL 7.2, 7.4 and 7.6
-if [ -x "%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh" ]; then
-%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh
-fi
+elif [ "$mainline_major" = "3" ] && [ "$mainline_minor" = "10" ] &&
+ { [ "$major_rev" = "327" ] || [ "$major_rev" = "693" ] || \
+   [ "$major_rev" = "957" ]; }; then
+# For RHEL 7.2, 7.4 and 7.6
+if [ -x "%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh" ]; then
+%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh
 fi
 else
 # Ensure that modprobe will find our modules.
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] rhel: Fixed a bug for checking the correct major version and revision.

[Ashish Varma]

Fixed a bug where checking for major version 3.10 and major revision not
equal to 327 or 693 or 957 should have gone to the default else at the end.
In the current code, the default else condition will not get executed
for kernel with major version 3.10 and major revision not equal
to 327/693/957 resulting in failure to load the kernel module.

Fixes: 402efbe4e176 ("rhel: Add 4.12 kernel support in ovs-kmod-manage.sh")
Signed-off-by: Ashish Varma 
---
v1-v2:
Added the Fixes tag.
---
 rhel/openvswitch-kmod-fedora.spec.in | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/rhel/openvswitch-kmod-fedora.spec.in 
b/rhel/openvswitch-kmod-fedora.spec.in
index 92d763f..27f443a 100644
--- a/rhel/openvswitch-kmod-fedora.spec.in
+++ b/rhel/openvswitch-kmod-fedora.spec.in
@@ -90,13 +90,12 @@ if grep -qs "suse" /etc/os-release; then
 if [ -x "%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh" ]; then
 %{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh
 fi
-elif [ "$mainline_major" = "3" ] && [ "$mainline_minor" = "10" ]; then
-if [ "$major_rev" = "327" ] || [ "$major_rev" = "693" ] || \
-   [ "$major_rev" = "957" ]; then
-# For RHEL 7.2, 7.4 and 7.6
-if [ -x "%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh" ]; then
-%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh
-fi
+elif [ "$mainline_major" = "3" ] && [ "$mainline_minor" = "10" ] &&
+ { [ "$major_rev" = "327" ] || [ "$major_rev" = "693" ] || \
+   [ "$major_rev" = "957" ]; }; then
+# For RHEL 7.2, 7.4 and 7.6
+if [ -x "%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh" ]; then
+%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh
 fi
 else
 # Ensure that modprobe will find our modules.
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1] doc: Added OVS Nicira Extension document

[Ashish Varma]

OVS supports Nicira Extensions as various vendor messages or as vendor
types in stats or multipart messages. Added a document to describe the
Nicira extension as currently supported by OVS.

Signed-off-by: Ashish Varma 
---
 Documentation/automake.mk |   1 +
 Documentation/index.rst   |   3 +-
 Documentation/topics/index.rst|   1 +
 Documentation/topics/ovs-nicira-extension.rst | 274 ++
 4 files changed, 278 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/topics/ovs-nicira-extension.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 2a3214a..f54492e 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -64,6 +64,7 @@ DOC_SOURCE = \
Documentation/topics/porting.rst \
Documentation/topics/role-based-access-control.rst \
Documentation/topics/tracing.rst \
+   Documentation/topics/ovs-nicira-extension.rst \
Documentation/topics/windows.rst \
Documentation/howto/index.rst \
Documentation/howto/docker.rst \
diff --git a/Documentation/index.rst b/Documentation/index.rst
index bace34d..152e5b3 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -77,7 +77,8 @@ Deeper Dive
 - **Architecture** :doc:`topics/design` |
   :doc:`topics/openflow` |
   :doc:`topics/integration` |
-  :doc:`topics/porting`
+  :doc:`topics/porting` |
+  :doc:`topics/ovs-nicira-extension`
 
 - **DPDK** :doc:`howto/dpdk` |
   :doc:`topics/dpdk/vhost-user`
diff --git a/Documentation/topics/index.rst b/Documentation/topics/index.rst
index 057649d..b053a27 100644
--- a/Documentation/topics/index.rst
+++ b/Documentation/topics/index.rst
@@ -51,6 +51,7 @@ OVS
testing
tracing
idl-compound-indexes
+   ovs-nicira-extension
 
 OVN
 ---
diff --git a/Documentation/topics/ovs-nicira-extension.rst 
b/Documentation/topics/ovs-nicira-extension.rst
new file mode 100644
index 000..332f807
--- /dev/null
+++ b/Documentation/topics/ovs-nicira-extension.rst
@@ -0,0 +1,274 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+=
+OVS Nicira Extensions
+=
+
+Q: What are the Nicira vendor messages in OpenFlow and OVS?
+
+A: OpenFlow since version 1.0 allows 'vendor objects' to be added in the
+protocol and OVS has used this as 'Nicira extensions' in the
+implementation. It has been used to add additional functionality for
+the desired features not present in the standard OpenFlow protocol.
+
+There are two types of vendor or experimenter message types in OpenFlow:
+
+
+1. **OFPT_VENDOR (In OpenFlow 1.0) or
+   OFPT_EXPERIMENTER (In OpenFlow 1.1+)**
+
+This is a vendor message type with value the value of 4
+in the OpenFlow header 'type' field. After the header of this message,
+there is a vendor id field which identifies the vendor. This is followed
+by a subtype field which defines the vendor specific message types.
+Currently, following vendor ids are defined:
+
+(ovs/include/openflow/openflow-common.h)
+
+::
+
+   HPL_VENDOR_ID  0x04EA  HP Labs
+   NTR_VENDOR_ID  0x154d  Netronome
+   NTR_COMPAT_VENDOR_ID   0x1540  Incorrect value used in v2.4
+   NX_VENDOR_ID   0x2320  Nicira
+   ONF_VENDOR_ID  0x4f4e4600  Open Networking Foundation
+   INTEL_VENDOR_ID0xAA01  Intel
+
+OVS uses the Nicira vendor id 0x2320 for all the vendor extension
+messages. To see a list of all the Nicira vendor message subytes, we
+can refer to 'ovs/lib/ofp-msgs.inc' file which gets auto generated after
+compiling the ovs code. Here we can see all the structures of type
+'struct raw_instance' and check for lines containing the hex string
+0x2320. For e.g. in the following line inside
+'ofpraw_nxt_flow_mod_instances':
+
+::
+
+   { {0, NULL}, {1, 4, 0, 0x2320, 13}, OFPRAW_NXT_FLOW_MOD, 0 },
+
+   1  -  is the OpenFlow version 1
+   4  -  is the vendor/experimenter message

[ovs-dev] [PATCH v2] ofp-monitor: Fixed the usage of 'usable_protocols' variable in 'parse_flow_monitor_request' function.

[Ashish Varma]

'usable_protocols' is now getting set to OFPUTIL_P_OF10_ANY on return from
'parse_flow_monitor_request' function. The calling function now checks for the
value in this variable against the 'allowed_protocols' variable.
Also a check is added for a match field which is not supported in OpenFlow 1.0
and return an error.
Modified the man page of ovs-ofctl to reflect Flow Monitor support as
OpenFlow 1.0 Nicira extension only.

Signed-off-by: Ashish Varma 
---
v1-v2:
Added a test that triggers the error condition fixed in this patch.
Setting the usable_protocols variable to OFPUTIL_P_OF10_ANY or even a
further subset if the match field requires it to. e.g. 'sctp_dst' is supported
as only NXM protocol and not in STD protocol.
Consistent wording for the error message on encountering an unusable flow
format.
---
 lib/ofp-monitor.c|  8 
 tests/ofproto.at | 21 +
 utilities/ovs-ofctl.8.in |  3 ++-
 utilities/ovs-ofctl.c|  8 
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/lib/ofp-monitor.c b/lib/ofp-monitor.c
index aeebd7e..2fffb3b 100644
--- a/lib/ofp-monitor.c
+++ b/lib/ofp-monitor.c
@@ -469,6 +469,7 @@ parse_flow_monitor_request__(struct 
ofputil_flow_monitor_request *fmr,
 fmr->table_id = 0xff;
 match_init_catchall(>match);
 
+*usable_protocols = OFPUTIL_P_ANY;
 while (ofputil_parse_key_value(, , )) {
 const struct ofp_protocol *p;
 char *error = NULL;
@@ -493,6 +494,10 @@ parse_flow_monitor_request__(struct 
ofputil_flow_monitor_request *fmr,
 } else if (mf_from_name(name)) {
 error = ofp_parse_field(mf_from_name(name), value, port_map,
 >match, usable_protocols);
+if (!error && !(*usable_protocols & OFPUTIL_P_OF10_ANY)) {
+return xasprintf("%s: match field is not supported for "
+ "flow monitor", name);
+}
 } else {
 if (!*value) {
 return xasprintf("%s: field %s missing value", str_, name);
@@ -513,6 +518,9 @@ parse_flow_monitor_request__(struct 
ofputil_flow_monitor_request *fmr,
 if (error) {
 return error;
 }
+/* Flow Monitor is supported in OpenFlow 1.0 or can be further reduced
+ * to a few 1.0 flavors by a match field. */
+*usable_protocols &= OFPUTIL_P_OF10_ANY;
 }
 return NULL;
 }
diff --git a/tests/ofproto.at b/tests/ofproto.at
index a810dd6..298f932 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -5124,6 +5124,27 @@ NXT_FLOW_MONITOR_RESUMED:
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto - flow monitoring usable protocols])
+AT_KEYWORDS([monitor])
+
+OVS_VSWITCHD_START
+
+on_exit 'kill `cat ovs-ofctl.pid`'
+ovs-ofctl -OOpenFlow14 monitor br0 watch:udp,udp_dst=8 --detach --no-chdir 
--pidfile >monitor.log 2>&1
+AT_CAPTURE_FILE([monitor.log])
+
+# ovs-ofctl should exit because monitor is not supported in OpenFlow 1.4
+OVS_WAIT_UNTIL([grep "ovs-ofctl: none of the usable flow formats 
(OpenFlow10,NXM) is among the allowed flow formats (OXM-OpenFlow14)" 
monitor.log])
+
+# check that only NXM flag is returned as usable protocols for sctp_dst
+# and ovs-ofctl should exit since monitor is not supported in OpenFlow 1.4
+ovs-ofctl -OOpenFlow14 monitor br0 watch:sctp,sctp_dst=9 --detach --no-chdir 
--pidfile >monitor.log 2>&1
+OVS_WAIT_UNTIL([grep "ovs-ofctl: none of the usable flow formats (NXM) is 
among the allowed flow formats (OXM-OpenFlow14)" monitor.log])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
 AT_SETUP([ofproto - event filtering (OpenFlow 1.3)])
 AT_KEYWORDS([monitor])
 OVS_VSWITCHD_START
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 60b4a1c..cb5c612 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -578,7 +578,8 @@ monitoring will not show any traffic.
 .IP "\fBmonitor \fIswitch\fR [\fImiss-len\fR] [\fBinvalid_ttl\fR] 
[\fBwatch:\fR[\fIspec\fR...]]"
 Connects to \fIswitch\fR and prints to the console all OpenFlow
 messages received.  Usually, \fIswitch\fR should specify the name of a
-bridge in the \fBovs\-vswitchd\fR database.
+bridge in the \fBovs\-vswitchd\fR database. This is available only in
+OpenFlow 1.0 as Nicira extension.
 .IP
 If \fImiss-len\fR is provided, \fBovs\-ofctl\fR sends an OpenFlow ``set
 configuration'' message at connection setup time that requests
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 754629d..c6f8111 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2286,6 +2286,14 @@ ofctl_monitor(struct ovs_cmdl_context *ctx)
 ovs_fatal(0, "%s", error);
 }
 
+if (!(usable_protocols & allowed_protocols)) {
+char *allowed_s =
+ofputil_protocols_to_string(allowed_pr

[ovs-dev] [PATCH v2] doc: Added OVS Extensions document

[Ashish Varma]

OVS supports OVS Extensions as various vendor messages or as vendor
types in stats or multipart messages. Added a document to describe the
extensions as currently supported by OVS.

Signed-off-by: Ashish Varma 
---
v1-v2:
Fixed the indentation in the commit message.
Converted the document from FAQ format to regular document format.
Changed the multiple references to Nicira extension to Open vSwitch extensions.
Changed the file name from ovs-nicira-extension.rst to ovs-extensions.rst.
Removed references from 'objects' to 'extensions'.
Refer to ofp-msgs.h instead of ofp-msgs.inc.
NXM description now refers to ovs-fields(7).
The C structures are now being referred to respective files instead of copying
it in the document.

---
 Documentation/automake.mk   |   1 +
 Documentation/index.rst |   3 +-
 Documentation/topics/index.rst  |   1 +
 Documentation/topics/ovs-extensions.rst | 147 
 4 files changed, 151 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/topics/ovs-extensions.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 2a3214a..5aa494a 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -64,6 +64,7 @@ DOC_SOURCE = \
Documentation/topics/porting.rst \
Documentation/topics/role-based-access-control.rst \
Documentation/topics/tracing.rst \
+   Documentation/topics/ovs-extensions.rst \
Documentation/topics/windows.rst \
Documentation/howto/index.rst \
Documentation/howto/docker.rst \
diff --git a/Documentation/index.rst b/Documentation/index.rst
index bace34d..4e4bb61 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -77,7 +77,8 @@ Deeper Dive
 - **Architecture** :doc:`topics/design` |
   :doc:`topics/openflow` |
   :doc:`topics/integration` |
-  :doc:`topics/porting`
+  :doc:`topics/porting` |
+  :doc:`topics/ovs-extensions`
 
 - **DPDK** :doc:`howto/dpdk` |
   :doc:`topics/dpdk/vhost-user`
diff --git a/Documentation/topics/index.rst b/Documentation/topics/index.rst
index 057649d..6b514a5 100644
--- a/Documentation/topics/index.rst
+++ b/Documentation/topics/index.rst
@@ -51,6 +51,7 @@ OVS
testing
tracing
idl-compound-indexes
+   ovs-extensions
 
 OVN
 ---
diff --git a/Documentation/topics/ovs-extensions.rst 
b/Documentation/topics/ovs-extensions.rst
new file mode 100644
index 000..babc603
--- /dev/null
+++ b/Documentation/topics/ovs-extensions.rst
@@ -0,0 +1,147 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+===
+Open vSwitch Extensions
+===
+
+Introduction
+
+OpenFlow since version 1.0 allows vendor extensions to be added in the
+protocol and OVS has used these extensions in the implementation. (Initially
+also known as 'Nicira extensions'. This is the reason we see the prefix
+'NX' in the different vendor extension messages.)
+These extensions have been used to add additional functionality for the
+desired features not present in the standard OpenFlow protocol.
+
+
+OVS vendor extension messages in OpenFlow and OVS
+-
+
+1. **OFPT_VENDOR (In OpenFlow 1.0) or
+   OFPT_EXPERIMENTER (In OpenFlow 1.1+)**
+
+This is a vendor message type with value the value of OFPT_VENDOR
+or OFPT_EXPERIMENTER (refer to respective OpenFlow specifications)
+in the OpenFlow header 'type' field. After the header of this message,
+there is a vendor id field which identifies the vendor. This is followed
+by a subtype field which defines the vendor specific message types.
+The vendor ids are defined in: ovs/include/openflow/openflow-common.h
+
+To see a list of all the vendor message subtypes, we
+can refer to 'ovs/lib/ofp-msgs.h' file. We can see the instances
+of 'ofpraw' enum which has a comment containing the keyword NXT.
+For e.g. in the below mentioned line containing OFPRAW_NXT_FLOW_MOD:
+
+::
+
+   /* NXT 1.0+ (13): struct nx_flow_mod, uint8_t[8][]. */
+   OFPRAW_NXT_FLOW_MOD,
+
+   NXT  - stands for Nicira extension message.
+   nx_flow_mod

Re: [ovs-dev] [PATCH v1 2/2] ofp-monitor: Support flow monitoring for OpenFlow 1.3, 1.4+

[Ashish Varma]

I will take a look.

Thanks,
Ashish

On Tue, Aug 24, 2021 at 9:29 AM Vasu Dasari  wrote:

> Hi Ben/Ashish,
>
> When you get a chance, Can you please take a look at my code?
>
> -Vasu
>
> *Vasu Dasari*
>
>
> On Thu, Jul 29, 2021 at 10:36 PM Vasu Dasari  wrote:
>
>> Extended OpenFlow monitoring support
>> * OpenFlow 1.3 with ONF extensions
>> * OpenFlow 1.4+ as defined in OpenFlow specification 1.4+.
>>
>> ONF extensions are similar to Nicira extensions except for
>> onf_flow_monitor_request{}
>> where out_port is defined as 32-bit number OF(1.1) number, oxm match
>> formats are
>> used in update and request messages.
>>
>> Flow monitoring support in 1.4+ is slightly different from Nicira and ONF
>> extensions.
>>  * More flow monitoring flags are defined.
>>  * Monitor add/modify/delete command is intruduced in flow_monitor
>>request message.
>>  * Addition of out_group as part of flow_monitor request message
>>
>> Description of changes:
>> 1. Generate ofp-msgs.inc to be able to support 1.3, 1.4+ flow Monitoring
>> messages.
>> include/openvswitch/ofp-msgs.h
>>
>> 2. Modify openflow header files with protocol specific headers.
>> include/openflow/openflow-1.3.h
>> include/openflow/openflow-1.4.h
>>
>> 3. Modify OvS abstraction of openflow headers. ofp-monitor.h leverages
>> enums
>>from on nicira extensions for creating protocol abstraction headers.
>> OF(1.4+)
>>enums are superset of nicira extensions.
>> include/openvswitch/ofp-monitor.h
>>
>> 4. Changes to these files reflect encoding and decoding of new protocol
>> messages.
>> lib/ofp-monitor.c
>>
>> 5. Changes to mmodules using ofp-monitor APIs. Most of the changes here
>> are to
>>migrate enums from nicira to OF 1.4+ versions.
>> ofproto/connmgr.c
>> ofproto/connmgr.h
>> ofproto/ofproto-provider.h
>> ofproto/ofproto.c
>>
>> 6. Extended protocol decoding tests to verify all protocol versions
>> FLOW_MONITOR_CANCEL
>> FLOW_MONITOR_PAUSED
>> FLOW_MONITOR_RESUMED
>> FLOW_MONITOR request
>> FLOW_MONITOR reply
>> tests/ofp-print.at
>>
>> 7. Modify flow monitoring tests to be able executed by all protocol
>> versions.
>> tests/ofproto.at
>>
>> 7. Modified documentation highlighting the change
>> utilities/ovs-ofctl.8.in
>> NEWS
>>
>> Signed-off-by: Vasu Dasari 
>> Reported-at:
>> https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/383915.html
>> ---
>> v1:
>>  - Fixed 0-day Robot errors
>>
>> ---
>>  NEWS  |   6 +-
>>  include/openflow/openflow-1.3.h   |  89 
>>  include/openflow/openflow-1.4.h   |  93 +++-
>>  include/openvswitch/ofp-monitor.h |   9 +-
>>  include/openvswitch/ofp-msgs.h|  39 +-
>>  lib/ofp-monitor.c | 844 --
>>  lib/ofp-print.c   |  24 +-
>>  ofproto/connmgr.c |  47 +-
>>  ofproto/connmgr.h |   6 +-
>>  ofproto/ofproto-provider.h|   4 +-
>>  ofproto/ofproto.c |  89 +++-
>>  tests/ofp-print.at| 122 -
>>  tests/ofproto.at  | 176 +--
>>  utilities/ovs-ofctl.8.in  |   3 +
>>  utilities/ovs-ofctl.c |   6 +
>>  15 files changed, 1265 insertions(+), 292 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 02884b774..47ad9de2a 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -25,8 +25,10 @@ v2.16.0 - xx xxx 
>> - In ovs-vsctl and vtep-ctl, the "find" command now accept new
>>   operators {in} and {not-in}.
>> - OpenFlow:
>> - * Extend Flow Monitoring support for OpenFlow 1.0-1.2 with Nicira
>> -   Extensions
>> + * Extended Flow Monitoring support for all supported OpenFlow
>> versions
>> + OpenFlow versions 1.0-1.2 with Nicira Extensions
>> + OpenFlow versions 1.3 with Open Network Foundation extension
>> + OpenFlow versions 1.4+, as defined in the OpenFlow specification
>> - Userspace datapath:
>>   * Auto load balancing of PMDs now partially supports cross-NUMA
>> polling
>> cases, e.g if all PMD threads are running on the same NUMA node.
>> diff --git a/include/openflow/openflow-1.3.h
>> b/include/openflow/openflow-1.3.h
>> index c48a8ea7f..1a818dbb4 100644
>> --- a/include/openflow/openflow-1.3.h
>> +++ b/include/openflow/openflow-1.3.h
>> @@ -374,4 +374,93 @@ struct ofp13_async_config {
>>  };
>>  OFP_ASSERT(sizeof(struct ofp13_async_config) == 24);
>>
>> +struct onf_flow_monitor_request {
>> +ovs_be32   id;/* Controller-assigned ID for this
>> monitor. */
>> +ovs_be16   flags; /* ONFFMF_*. */
>> +ovs_be16   match_len; /* Length of oxm_fields. */
>> +ovs_be32   out_port;  /* Required output port, if not OFPP_NONE.
>> */
>> +uint8_ttable_id;  /* One table’s ID or 0xff for all tables.
>> */
>> +uint8_tzeros[3];  /* Align to 64 bits (must be zero). */
>> +/* Followed by an ofp11_match