[ovs-dev] [PATCH v3] ovs-ofctl: replace-flows and diff-flows support for multiple tables

Shashank Shanbhag shashank.shanbhag at gmail.com
Thu Jun 18 21:39:07 UTC 2015


Hi Ben,

Please review when you get a chance. We would really like this to be in the
2.4 release. :-)

This patch has the following changes suggested by you in the last review:
1. ovs-ofctl replace-flow and diff-flow queries the switch for visible
tables

You also suggested that we move to using an array rather than a hash.
http://openvswitch.org/pipermail/dev/2015-June/055998.html

An array of 255 pointers to classifiers would lead to 2040 bytes (64-bit
system). I'd like to avoid that especially when there are a handful of
tables.


Please let me know,
Thanks,

---------------------------------------------------------
Shashank Shanbhag,
---------------------------------------------------------

On Thu, Jun 18, 2015 at 2:31 PM, Shashank Shanbhag <
shashank.shanbhag at oracle.com> wrote:

> From: Shashank Shanbhag <shashank.shanbhag at gmail.com>
>
> Fix replace-flows and diff-flows to modify/diff flows in multiple tables.
> Add a --tables(-T) option that allows the user to specify a comma-separated
> list of table indexes to replace/diff.
> Fix replace-flows to support OF1.3 and above.
> Add/change ovs-ofctl replace-flows and diff-flows tests.
>
> Signed-off-by: Shashank Shanbhag <shashank.shanbhag at gmail.com>
> Acked-by: Romain Lenglet <romain.lenglet at oracle.com>
> ---
>  AUTHORS                  |   1 +
>  NEWS                     |   7 +-
>  tests/ofproto-dpif.at    |   6 +-
>  tests/ofproto.at         |   2 +-
>  tests/ovs-ofctl.at       | 141 ++++++++++----
>  utilities/ovs-ofctl.8.in |  20 +-
>  utilities/ovs-ofctl.c    | 465
> ++++++++++++++++++++++++++++++++++++++++-------
>  7 files changed, 533 insertions(+), 109 deletions(-)
>
> diff --git a/AUTHORS b/AUTHORS
> index cebab48..96a00f1 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -155,6 +155,7 @@ Scott Lowe              scott.lowe at scottlowe.org
>  Scott Mann              sdmnix at gmail.com
>  Selvamuthukumar         smkumar at merunetworks.com
>  Shan Wei                davidshan at tencent.com
> +Shashank Shanbhag       shashank.shanbhag at gmail.com
>  Shih-Hao Li             shli at nicira.com
>  Shu Shen                shu.shen at radisys.com
>  Simon Horman            horms at verge.net.au
> diff --git a/NEWS b/NEWS
> index 831bf5f..99545ad 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -108,7 +108,12 @@ v2.4.0 - xx xxx xxxx
>       openvswitch.ko but built and loaded automatically as individual
> kernel
>       modules (vport-*.ko).
>     - Support for STT tunneling.
> -
> +   - ovs-ofctl: replace-flows and diff-flows now operate on multiple
> tables.
> +   - ovs-ofctl: --tables option added to replace-flows and diff-flows to
> allow
> +     specific tables to be replaced or diffed.
> +   - ovs-ofctl: replace-flows and diff-flows now query the switch for
> visible
> +     tables. Hidden tables are ignored.
> +   - ovs-ofctl works for OF version 1.3 and above.
>
>  v2.3.0 - 14 Aug 2014
>  ---------------------
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index bd1b21c..0567dab 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -3693,7 +3693,7 @@ priority=50 tcp ip_frag=no
> actions=move:OXM_OF_TCP_DST[[]]->OXM_OF_
>  priority=50 tcp ip_frag=first
>  actions=move:OXM_OF_TCP_DST[[]]->OXM_OF_TCP_SRC[[]],output:5
>  priority=50 tcp ip_frag=later           actions=output:6
>  ])
> -AT_CHECK([ovs-ofctl replace-flows br0 flows.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 flows.txt])
>
>
>  base_flow="in_port(90),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=128"
>  no_flow="$base_flow,frag=no),tcp(src=12345,dst=80)"
> @@ -3742,7 +3742,7 @@ priority=50 tcp ip_frag=no
> actions=set_field:81->tcp_dst,output:4
>  priority=50 tcp ip_frag=first
>  actions=set_field:81->tcp_dst,output:5
>  priority=50 tcp ip_frag=later           actions=output:6
>  ])
> -AT_CHECK([ovs-ofctl replace-flows br0 flows.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 flows.txt])
>
>
>  base_flow="in_port(90),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=128"
>  no_flow="$base_flow,frag=no),tcp(src=12345,dst=80)"
> @@ -3856,7 +3856,7 @@ ovs-ofctl: actions are invalid with specified match
> (OFPBAC_MATCH_INCONSISTENT)
>  AT_DATA([flows.txt], [dnl
>  priority=75 tcp actions=load:42->OXM_OF_TCP_SRC[[0..7]],output:1
>  ])
> -AT_CHECK([ovs-ofctl -O OpenFlow12 replace-flows br0 flows.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 flows.txt])
>
>  AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
>
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 9c5f0bb..16bbf78 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -3066,7 +3066,7 @@ ${PERL} -e '
>  ') > flows.txt
>  AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
>  # Check that multipart flow dumps work properly:
> -AT_CHECK([ovs-ofctl diff-flows br0 flows.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 diff-flows br0 flows.txt])
>  AT_CHECK([ovs-ofctl add-flow br0 in_port=1,cookie=3,actions=drop])
>  AT_CHECK([ovs-ofctl mod-flows br0 in_port=2,cookie=2,actions=output:2])
>  AT_CHECK([ovs-ofctl del-flows br0 cookie=1/-1])
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index 6c48569..8bb42e0 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -2622,15 +2622,15 @@ AT_CHECK([ovs-ofctl add-flows br0 add-flows.txt])
>  for i in `seq 0 1023`; do echo " dl_vlan=$i actions=drop"; done | sort >
> expout
>  AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sed '/NXST_FLOW/d' |
> sort],
>    [0], [expout])
> -AT_CHECK([ovs-ofctl diff-flows br0 add-flows.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 diff-flows br0 add-flows.txt])
>
>  # Remove even-numbered flows, compare again.
>  for i in `seq 0 1023 2`; do echo "dl_vlan=$i"; done > del-flows.txt
>  AT_CHECK([ovs-ofctl del-flows br0 - < del-flows.txt])
>  for i in `seq 0 1023 2`; do echo "+dl_vlan=$i actions=drop"; done | sort
> > expout
> -AT_CHECK([ovs-ofctl diff-flows br0 add-flows.txt | sort], [0], [expout])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 diff-flows br0 add-flows.txt | sort],
> [0], [expout])
>  for i in `seq 0 1023 2`; do echo "-dl_vlan=$i actions=drop"; done | sort
> > expout
> -AT_CHECK([ovs-ofctl diff-flows add-flows.txt br0 | sort], [0], [expout])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 diff-flows add-flows.txt br0 | sort],
> [0], [expout])
>
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> @@ -2648,13 +2648,13 @@ OVS_VSWITCHD_START
>  AT_DATA([flows.txt], [actions=resubmit(,1)
>  ])
>  AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> -AT_CHECK([ovs-ofctl diff-flows br0 flows.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 diff-flows br0 flows.txt])
>  AT_CHECK([ovs-ofctl add-flow br0
> idle_timeout=60,dl_vlan=9,actions=output:1])
> -AT_CHECK([ovs-ofctl diff-flows br0 flows.txt], [2], [dnl
> +AT_CHECK([ovs-ofctl -O OpenFlow14 diff-flows br0 flows.txt], [2], [dnl
>  -dl_vlan=9 idle_timeout=60 actions=output:1
>  ])
>  AT_CHECK([ovs-ofctl add-flow br0
> hard_timeout=120,cookie=1234,dl_vlan=9,actions=output:1])
> -AT_CHECK([ovs-ofctl diff-flows flows.txt br0], [2], [dnl
> +AT_CHECK([ovs-ofctl -O OpenFlow14 diff-flows flows.txt br0], [2], [dnl
>  +dl_vlan=9 cookie=0x4d2 hard_timeout=120 actions=output:1
>  ])
>  OVS_VSWITCHD_STOP
> @@ -2789,27 +2789,82 @@ AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip |
> sed '/ST_FLOW reply/d' | sort
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
>
> -
>  dnl Importance parameter added in OF1.4.
>  dnl This validates whether flows with importance
>  dnl parameter are getting replaced with "replace-flows" or
>  dnl not by validating dump-flows output after replace with the expected
> output.
> +dnl MOD_FLOW does not modify importance field - ONF EXT-496.
>
>  AT_SETUP([ovs-ofctl replace-flows with importance])
>  OVS_VSWITCHD_START
>
>  dnl Add flows to br0 with importance via OF1.4+. For more details refer
> "ovs-ofctl rule with importance" test case.
> -for i in 1 2 3 4 5 6 7 8; do echo
> "dl_vlan=$i,importance=$i,actions=drop"; done > add-flows.txt
> +for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i
> actions=drop"; done | sort > add-flows.txt
>  AT_CHECK([ovs-ofctl -O OpenFlow14 add-flows br0 add-flows.txt])
>
> -dnl Replace some flows in the bridge.
> -for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i +
> 10`,actions=drop"; done > replace-flows.txt
> +dnl Modify odd numbered flows. Leave even numbered ones alone.
> +for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then
> importance=`expr $i + 10`; else importance=$i; fi; echo "
> importance=$importance, dl_vlan=$i actions=drop"; done | sort >
> replace-flows.txt
> +AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 replace-flows.txt])
> +
> +dnl Dump the flows and compare them against expected output.
> +
> +for i in 1 2 3 4 5 6 7 8; do echo " importance=$i, dl_vlan=$i
> actions=drop"; done | sort > expout
> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed
> '/OFPST_FLOW/d' | sort], [0], [expout])
> +
> +dnl Replace some flows in the bridge. Delete flows that are not present
> in replace-flows.txt
> +dnl for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i + 10`
> actions=drop"; done | sort > replace-flows.txt
> +
> +for i in 1 3 5 7; do echo "dl_vlan=$i,importance=$i actions=drop"; done |
> sort > replace-flows.txt
>  AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 replace-flows.txt])
>
>  dnl Dump them and compare the dump flows output against the expected
> output.
> -for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then
> importance=`expr $i + 10`; else importance=$i; fi; echo "
> importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout
> -AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed
> '/OFPST_FLOW/d' | sort],
> -  [0], [expout])
> +for i in 1 3 5 7; do echo " importance=$i, dl_vlan=$i actions=drop"; done
> | sort > expout
> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed
> '/OFPST_FLOW/d' | sort], [0], [expout])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> +dnl --tables option added to ovs-ofctl "replace-flows" and "diff-flows".
> +dnl This validates whether flows in table IDs specified with --tables
> +dnl are getting replaced with "replace-flows" or not by validating
> +dnl dump-flows output after replace with the expected output.
> +
> +AT_SETUP([ovs-ofctl replace-flows with --tables])
> +OVS_VSWITCHD_START
> +
> +dnl Add flows to br0 for tables 1 to 8.
> +for i in 1 2 3 4 5 6 7 8; do echo "table=$i,dl_vlan=$i,actions=drop";
> done > add-flows.txt
> +AT_CHECK([ovs-ofctl -O OpenFlow14 add-flows br0 add-flows.txt])
> +
> +dnl Replace flows from tables 1,3,5,7 in the bridge.
> +for i in 1 3 5 7; do echo
> "table=$i,ip,nw_dst=192.168.1.$i,dl_vlan=$i,actions=drop"; done >
> replace-flows.txt
> +AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 replace-flows.txt
> --tables 1,5,7,3])
> +
> +dnl Generate the expout.
> +for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then echo "
> table=$i, ip,dl_vlan=$i,nw_dst=192.168.1.$i actions=drop"; else echo "
> table=$i, dl_vlan=$i actions=drop"; fi; done | sort > expout
> +dnl Dump the flows and check.
> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed
> '/OFPST_FLOW/d' | sort], [0], [expout])
> +
> +AT_DATA([oneflow.txt], [[
> +table=4,ip,nw_dst=10.0.0.4,actions=output:100
> +]])
> +
> +dnl Check that all flows except the ones in the replacement file are
> present after
> +dnl replace-flows is executed.
> +AT_CHECK([ovs-ofctl -O OpenFlow14 del-flows br0])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 add-flows br0 add-flows.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 oneflow.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed
> '/OFPST_FLOW/d' | sort], [0], [dnl
> + table=4, ip,nw_dst=10.0.0.4 actions=output:100
> +])
> +
> +dnl Check that tables that do not have flows are not replaced.
> +dnl Check that existing flows in other tables are not replaced as well.
> +AT_CHECK([ovs-ofctl -O OpenFlow14 del-flows br0])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 add-flows br0 add-flows.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 oneflow.txt --tables
> 100])
> +for i in 1 2 3 4 5 6 7 8; do echo " table=$i, dl_vlan=$i actions=drop";
> done | sort > expout
> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed
> '/OFPST_FLOW/d' | sort], [0], [expout])
>
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> @@ -2821,14 +2876,14 @@ AT_CHECK([ovs-appctl vlog/set vconn:dbg])
>
>  dnl Add flows to br0 with importance via OF1.4+, using an OF1.4+ bundle.
> For more details refer "ovs-ofctl rule with importance" test case.
>  for i in 1 2 3 4 5 6 7 8; do echo
> "dl_vlan=$i,importance=$i,actions=drop"; done > add-flows.txt
> -AT_CHECK([ovs-ofctl --bundle add-flows br0 add-flows.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 --bundle add-flows br0 add-flows.txt])
>
>  dnl Replace some flows in the bridge.
>  for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i +
> 10`,actions=drop"; done > replace-flows.txt
> -AT_CHECK([ovs-ofctl --bundle replace-flows br0 replace-flows.txt])
> +AT_CHECK([ovs-ofctl -O OpenFlow14 --bundle replace-flows br0
> replace-flows.txt])
>
>  dnl Dump them and compare the dump flows output against the expected
> output.
> -for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then
> importance=`expr $i + 10`; else importance=$i; fi; echo "
> importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout
> +for i in 1 3 5 7; do echo " importance=$i, dl_vlan=$i actions=drop"; done
> | sort > expout
>  AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed
> '/OFPST_FLOW/d' | sort],
>    [0], [expout])
>
> @@ -2836,12 +2891,10 @@ dnl Check logs for OpenFlow trace
>  # Prevent race.
>  OVS_WAIT_UNTIL([test `grep -- "|vconn|DBG|unix: sent (Success):
> OFPST_FLOW reply" ovs-vswitchd.log | wc -l` -ge 2])
>  # AT_CHECK([sed -n "s/^.*\(|vconn|DBG|.*xid=.*:\).*$/\1/p"
> ovs-vswitchd.log], [0], [dnl
> -AT_CHECK([print_vconn_debug | ofctl_strip], [0], [dnl
> +AT_CHECK([print_vconn_debug | ofctl_strip | awk 'NF &&
> $1!~/^(table|active|version)/'], [0], [dnl
>  vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
> - version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
>  vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
> - version bitmap: 0x01, 0x05
> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06
> and earlier, peer supports versions 0x01, 0x05)
> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06
> and earlier, peer supports version 0x05)
>  vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
>   bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
>  vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
> @@ -2875,47 +2928,61 @@ vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL
> (OF1.4):
>  vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
>   bundle_id=0 type=COMMIT_REPLY flags=0
>  vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
> - version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
>  vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
> - version bitmap: 0x01, 0x05
> -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06
> and earlier, peer supports versions 0x01, 0x05)
> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06
> and earlier, peer supports version 0x05)
> +vconn|DBG|unix: received: OFPST_TABLE request (OF1.4):
> +vconn|DBG|unix: sent (Success): OFPST_TABLE reply (OF1.4):
>  vconn|DBG|unix: received: OFPST_FLOW request (OF1.4):
>  vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4):
> + importance=1, dl_vlan=1 actions=drop
> + importance=2, dl_vlan=2 actions=drop
> + importance=3, dl_vlan=3 actions=drop
> + importance=4, dl_vlan=4 actions=drop
> + importance=5, dl_vlan=5 actions=drop
> + importance=6, dl_vlan=6 actions=drop
> + importance=7, dl_vlan=7 actions=drop
> + importance=8, dl_vlan=8 actions=drop
>  vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
>   bundle_id=0 type=OPEN_REQUEST flags=atomic ordered
>  vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
>   bundle_id=0 type=OPEN_REPLY flags=0
>  vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
>   bundle_id=0 flags=atomic ordered
> -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:11 actions=drop
> +OFPT_FLOW_MOD (OF1.4): MOD_STRICT dl_vlan=1 actions=drop
>  vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
>   bundle_id=0 flags=atomic ordered
> -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=3 importance:13 actions=drop
> +OFPT_FLOW_MOD (OF1.4): MOD_STRICT dl_vlan=3 actions=drop
>  vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
>   bundle_id=0 flags=atomic ordered
> -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=5 importance:15 actions=drop
> +OFPT_FLOW_MOD (OF1.4): MOD_STRICT dl_vlan=5 actions=drop
>  vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
>   bundle_id=0 flags=atomic ordered
> -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=7 importance:17 actions=drop
> +OFPT_FLOW_MOD (OF1.4): MOD_STRICT dl_vlan=7 actions=drop
> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
> + bundle_id=0 flags=atomic ordered
> +OFPT_FLOW_MOD (OF1.4): DEL_STRICT dl_vlan=2 actions=drop
> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
> + bundle_id=0 flags=atomic ordered
> +OFPT_FLOW_MOD (OF1.4): DEL_STRICT dl_vlan=4 actions=drop
> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
> + bundle_id=0 flags=atomic ordered
> +OFPT_FLOW_MOD (OF1.4): DEL_STRICT dl_vlan=6 actions=drop
> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4):
> + bundle_id=0 flags=atomic ordered
> +OFPT_FLOW_MOD (OF1.4): DEL_STRICT dl_vlan=8 actions=drop
>  vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4):
>   bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered
>  vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4):
>   bundle_id=0 type=COMMIT_REPLY flags=0
>  vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5):
> - version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06
>  vconn|DBG|unix: received: OFPT_HELLO (OF1.4):
> - version bitmap: 0x05
>  vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06
> and earlier, peer supports version 0x05)
>  vconn|DBG|unix: received: OFPST_FLOW request (OF1.4):
>  vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4):
> - importance=2, dl_vlan=2 actions=drop
> - importance=4, dl_vlan=4 actions=drop
> - importance=6, dl_vlan=6 actions=drop
> - importance=8, dl_vlan=8 actions=drop
> - importance=11, dl_vlan=1 actions=drop
> - importance=13, dl_vlan=3 actions=drop
> - importance=15, dl_vlan=5 actions=drop
> - importance=17, dl_vlan=7 actions=drop
> + importance=1, dl_vlan=1 actions=drop
> + importance=3, dl_vlan=3 actions=drop
> + importance=5, dl_vlan=5 actions=drop
> + importance=7, dl_vlan=7 actions=drop
>  ])
>
>  OVS_VSWITCHD_STOP
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index 63c2ecc..1bf5041 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -328,7 +328,7 @@ Deletes entries from \fIswitch\fR's flow table.  With
> only a
>  entries that match the specified flows.  With \fB\-\-strict\fR,
>  wildcards are not treated as active for matching purposes.
>  .
> -.IP "[\fB\-\-bundle\fR] [\fB\-\-readd\fR] \fBreplace\-flows \fIswitch
> file\fR"
> +.IP "[\fB\-\-bundle\fR] [\fB\-\-readd\fR] [\fB\-\-tables \fltable-ids\fR]
> \fBreplace\-flows \fIswitch file\fR"
>  Reads flow entries from \fIfile\fR (or \fBstdin\fR if \fIfile\fR is
>  \fB\-\fR) and queries the flow table from \fIswitch\fR.  Then it fixes
>  up any differences, adding flows from \fIflow\fR that are missing on
> @@ -337,12 +337,21 @@ up any differences, adding flows from \fIflow\fR
> that are missing on
>  or timeouts differ in \fIfile\fR.
>  .
>  .IP
> +With \fB\-\-tables\fR, \fBovs\-ofctl\fR only considers flows from the
> +comma-separated table IDs specified. The tables are replaced
> +in the specified order, e.g. (\fB\-\-tables 1,3,4,2\fR). The valid
> +range of table IDs is 0 to 254. If a table ID is not specified,
> +the table with that ID is ignored and no flows are replaced in that
> +table in the switch. If \fB\-\-tables\fR is not specified, all tables,
> +0 - 254, will be replaced.
> +.
> +.IP
>  With \fB\-\-readd\fR, \fBovs\-ofctl\fR adds all the flows from
>  \fIfile\fR, even those that exist with the same actions, cookie, and
>  timeout in \fIswitch\fR.  This resets all the flow packet and byte
>  counters to 0, which can be useful for debugging.
>  .
> -.IP "\fBdiff\-flows \fIsource1 source2\fR"
> +.IP "[\fB\-\-tables \fltable-ids\fR] \fBdiff\-flows \fIsource1 source2\fR"
>  Reads flow entries from \fIsource1\fR and \fIsource2\fR and prints the
>  differences.  A flow that is in \fIsource1\fR but not in \fIsource2\fR
>  is printed preceded by a \fB\-\fR, and a flow that is in \fIsource2\fR
> @@ -357,6 +366,13 @@ file name.  A name that contains \fB:\fR is
> considered to be a switch.
>  Otherwise, it is a file if a file by that name exists, a switch if
>  not.
>  .IP
> +With \fB\-\-tables\fR, \fBovs\-ofctl\fR only considers flows from the
> +comma-separated table IDs specified, e.g. (\fB\-\-tables 1,3,4,2\fR).
> +The valid range of table IDs is 0 to 254. Unlike \fBreplace\-flows\fR,
> +the order of table IDs is ignored. If an ID is not specified, the table
> +with that ID is ignored and is diffed. If \fB\-\-tables\fR is not
> +specified, all tables are diffed.
> +.IP
>  For this command, an exit status of 0 means that no differences were
>  found, 1 means that an error occurred, and 2 means that some
>  differences were found.
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 8df79b8..f0c05b9 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -13,7 +13,6 @@
>   * See the License for the specific language governing permissions and
>   * limitations under the License.
>   */
> -
>  #include <config.h>
>  #include <ctype.h>
>  #include <errno.h>
> @@ -29,6 +28,7 @@
>  #include <sys/stat.h>
>  #include <sys/time.h>
>
> +#include "bitmap.h"
>  #include "byte-order.h"
>  #include "classifier.h"
>  #include "command-line.h"
> @@ -37,6 +37,8 @@
>  #include "dirs.h"
>  #include "dynamic-string.h"
>  #include "fatal-signal.h"
> +#include "hmap.h"
> +#include "list.h"
>  #include "nx-match.h"
>  #include "odp-util.h"
>  #include "ofp-actions.h"
> @@ -50,6 +52,7 @@
>  #include "ofproto/ofproto.h"
>  #include "openflow/nicira-ext.h"
>  #include "openflow/openflow.h"
> +#include "openflow/openflow-common.h"
>  #include "dp-packet.h"
>  #include "packets.h"
>  #include "pcap-file.h"
> @@ -67,6 +70,8 @@
>
>  VLOG_DEFINE_THIS_MODULE(ofctl);
>
> +#define TABLE_IDS_BITMAP_LEN (OFPTT_MAX + 1)
> +
>  /* --bundle: Use OpenFlow 1.4 bundle for making the flow table change
> atomic.
>   * NOTE: Also the flow mod will use OpenFlow 1.4, so the semantics may be
>   * different (see the comment in parse_options() for details).
> @@ -102,6 +107,21 @@ static bool timestamp;
>       commands. */
>  static char *unixctl_path;
>
> +/* -T, --tables: Comma-separated list of OF flow table IDs to replace
> flows in.
> + *  tables is a ovs_list of table IDs specified in --tables. */
> +static struct ovs_list tables;
> +
> +/* bmtables is a bitmap where an offset is set if the corresponding table
> + * ID needs to be operated on by replace-flows or diff-flows command.
> + * If a table ID is hidden, the corresponding offset is unset even if
> specified
> + * in -T, --tables. */
> +static unsigned long *bmtables;
> +
> +/*
> + * visible_tables is a bitmap where an offset is set if the corresponding
> table
> + * is not a hidden table. */
> +static unsigned long *visible_tables;
> +
>  /* --sort, --rsort: Sort order. */
>  enum sort_order { SORT_ASC, SORT_DESC };
>  struct sort_criterion {
> @@ -120,6 +140,154 @@ static bool recv_flow_stats_reply(struct vconn *,
> ovs_be32 send_xid,
>                                    struct ofpbuf **replyp,
>                                    struct ofputil_flow_stats *,
>                                    struct ofpbuf *ofpacts);
> +
> +static void perform_stats_transaction(struct vconn *, struct ofpbuf
> *request, bool dump);
> +
> +static int table_ids_list_and_bitmap_from_string(struct ovs_list *tables,
> +                                       unsigned long **bmtables,
> +                                       const char *s);
> +
> +static void update_bitmaps_with_supported_table_ids(struct vconn *vconn);
> +
> +static void populate_visible_tables_bitmap(struct vconn *vconn,
> +                                    const struct ofp_header *oh);
> +
> +bool print_diff_flows(struct classifier *cls);
> +
> + /*
> + * An element in the list of table_ids. The list has table ids in the
> order
> + * specified in --tables passed to replace-flows.
> + */
> +struct tables_node {
> +    struct ovs_list lnode;
> +    uint8_t table_id;
> +};
> +
> +/* Holds the hmap_node with hash and the actual classifier. */
> +struct hmap_table_classifier_node {
> +    struct hmap_node hnode;
> +    struct classifier cls;
> +    uint8_t table_id;
> +};
> +
> +/*
> + * Creates a classifier for table_id and adds it into the hmap. The
> table_id
> + * is used as the hash.
> + */
> +static struct classifier *
> +insert_new_table_classifier(struct hmap *ht_cls, uint8_t table_id)
> +{
> +    struct hmap_table_classifier_node *ht_cls_node;
> +
> +    ht_cls_node = xzalloc(sizeof(struct hmap_table_classifier_node));
> +    ht_cls_node->table_id = table_id;
> +    classifier_init(&ht_cls_node->cls, NULL);
> +    /* Use the table_id as the hash. */
> +    hmap_insert(ht_cls, &ht_cls_node->hnode, table_id);
> +    return &ht_cls_node->cls;
> +}
> +
> +/*
> + * Get the classifier for table_id. The table_id is used as the hash.
> + */
> +static struct classifier *
> +get_table_classifier(struct hmap *ht_cls, uint8_t table_id)
> +{
> +    struct hmap_table_classifier_node *ht_cls_node;
> +    HMAP_FOR_EACH_IN_BUCKET (ht_cls_node, hnode, table_id, ht_cls) {
> +        if (ht_cls_node->table_id == table_id) {
> +            return &ht_cls_node->cls;
> +        }
> +    }
> +    return NULL;
> +}
> +
> +/*
> + * Takes a string of comma-separated table IDs, creates a list
> + * containing the table_ids in the order specified in the --tables
> argument.
> + * Also creates a bitmap of specified table IDs.
> + */
> +static int
> +table_ids_list_and_bitmap_from_string(struct ovs_list *tables,
> +                                       unsigned long **bmtables,
> +                                       const char *s)
> +{
> +    char *stemp = xstrdup(s);
> +    char *tok, *temp;
> +    unsigned int table_id;
> +    struct tables_node *tnode;
> +
> +    *bmtables = bitmap_allocate(TABLE_IDS_BITMAP_LEN);
> +
> +    temp = stemp;
> +    while(stemp) {
> +        tok = strsep(&stemp, ",");
> +        if(!str_to_uint(tok, 10, &table_id)
> +                || table_id > TABLE_IDS_BITMAP_LEN-1) {
> +            LIST_FOR_EACH (tnode, lnode, tables) {
> +               free(tnode);
> +            }
> +            bitmap_free(*bmtables);
> +            free(temp);
> +            return -1;
> +        }
> +        tnode = xzalloc(sizeof(struct tables_node));
> +        tnode->table_id = table_id;
> +        list_push_back(tables, &tnode->lnode);
> +
> +        bitmap_set(*bmtables, table_id, true);
> +    }
> +    free(temp);
> +    return 0;
> +}
> +
> +/*
> + * Make sure a bmtables offset is set only if the corresponding
> + * table ID is supported (not hidden) in the switch. */
> +static void
> +update_bitmaps_with_supported_table_ids(struct vconn *vconn)
> +{
> +    struct ofpbuf *request;
> +
> +    visible_tables = bitmap_allocate(TABLE_IDS_BITMAP_LEN);
> +
> +    if(list_is_empty(&tables)) {
> +        /* Initialize the bitmap bmtables and set all bits. */
> +        bmtables = bitmap_allocate1(TABLE_IDS_BITMAP_LEN);
> +    }
> +
> +    request = ofpraw_alloc(OFPRAW_OFPST_TABLE_REQUEST,
> vconn_get_version(vconn), 0);
> +    perform_stats_transaction(vconn, request, false);
> +    bitmap_and(bmtables, visible_tables, TABLE_IDS_BITMAP_LEN);
> +}
> +
> +/*
> + * Get all visible table IDs and set offsets in visible_tables bitmap that
> + * correspond to the table IDs. */
> +static void
> +populate_visible_tables_bitmap(struct vconn *vconn, const struct
> ofp_header *oh)
> +{
> +    struct ofpbuf msg;
> +    ofpbuf_use_const(&msg, oh, ntohs(oh->length));
> +    ofpraw_pull_assert(&msg);
> +
> +    for(;;) {
> +        struct ofputil_table_features features;
> +        struct ofputil_table_stats stats;
> +        int retval;
> +
> +        retval = ofputil_decode_table_stats_reply(&msg, &stats,
> &features);
> +        if (retval) {
> +            if (retval != EOF) {
> +                ovs_fatal(0, "%s: Error while decoding table features:
> %s",
> +                          vconn_get_name(vconn), ofperr_get_name(retval));
> +            }
> +            return;
> +        }
> +        bitmap_set(visible_tables, features.table_id, true);
> +    }
> +}
> +
>  int
>  main(int argc, char *argv[])
>  {
> @@ -171,6 +339,7 @@ parse_options(int argc, char *argv[])
>          VLOG_OPTION_ENUMS
>      };
>      static const struct option long_options[] = {
> +        {"tables", required_argument, NULL, 'T'},
>          {"timeout", required_argument, NULL, 't'},
>          {"strict", no_argument, NULL, OPT_STRICT},
>          {"readd", no_argument, NULL, OPT_READD},
> @@ -211,9 +380,13 @@ parse_options(int argc, char *argv[])
>       */
>      set_allowed_ofp_versions("OpenFlow10");
>
> +    /* Initialize ovs_list tables. */
> +    list_init(&tables);
> +
>      for (;;) {
>          unsigned long int timeout;
>          int c;
> +        int retval = 0;
>
>          c = getopt_long(argc, argv, short_options, long_options, NULL);
>          if (c == -1) {
> @@ -231,6 +404,17 @@ parse_options(int argc, char *argv[])
>              }
>              break;
>
> +        case 'T':
> +            retval = table_ids_list_and_bitmap_from_string(&tables,
> +                                                           &bmtables,
> +                                                           optarg);
> +            if (retval < 0) {
> +                ovs_fatal(0, "one of the passed table IDs %s for "
> +                        "-T or --tables is invalid", optarg);
> +            }
> +            break;
> +
> +
>          case 'F':
>              allowed_protocols = ofputil_protocols_from_string(optarg);
>              if (!allowed_protocols) {
> @@ -392,6 +576,7 @@ usage(void)
>      ofp_version_usage();
>      vlog_usage();
>      printf("\nOther options:\n"
> +           "  -T, --tables=TABLES         replace or diff flows in
> specified TABLES\n"
>             "  --strict                    use strict match for flow
> commands\n"
>             "  --readd                     replace flows that haven't
> changed\n"
>             "  -F, --flow-format=FORMAT    force particular flow format\n"
> @@ -541,7 +726,7 @@ dump_trivial_transaction(const char *vconn_name, enum
> ofpraw raw)
>  }
>
>  static void
> -dump_stats_transaction(struct vconn *vconn, struct ofpbuf *request)
> +perform_stats_transaction(struct vconn *vconn, struct ofpbuf *request,
> bool dump)
>  {
>      const struct ofp_header *request_oh = request->data;
>      ovs_be32 send_xid = request_oh->xid;
> @@ -562,9 +747,13 @@ dump_stats_transaction(struct vconn *vconn, struct
> ofpbuf *request)
>          recv_xid = ((struct ofp_header *) reply->data)->xid;
>          if (send_xid == recv_xid) {
>              enum ofpraw raw;
> -
> -            ofp_print(stdout, reply->data, reply->size, verbosity + 1);
> -
> +            if (dump) {
> +                ofp_print(stdout, reply->data, reply->size, verbosity +
> 1);
> +            }
> +            else {
> +                /* Update bmtables - set offset = table ID. */
> +                populate_visible_tables_bitmap(vconn, (struct ofp_header
> *) reply->data);
> +            }
>              ofpraw_decode(&raw, reply->data);
>              if (ofptype_from_ofpraw(raw) == OFPTYPE_ERROR) {
>                  done = true;
> @@ -584,6 +773,12 @@ dump_stats_transaction(struct vconn *vconn, struct
> ofpbuf *request)
>  }
>
>  static void
> +dump_stats_transaction(struct vconn *vconn, struct ofpbuf *request)
> +{
> +    perform_stats_transaction(vconn, request, true);
> +}
> +
> +static void
>  dump_trivial_stats_transaction(const char *vconn_name, enum ofpraw raw)
>  {
>      struct ofpbuf *request;
> @@ -2455,6 +2650,7 @@ fte_insert(struct classifier *cls, const struct
> match *match,
>             int priority, struct fte_version *version, int index)
>  {
>      struct fte *old, *fte;
> +    classifier_defer(cls);
>
>      fte = xzalloc(sizeof *fte);
>      cls_rule_init(&fte->rule, match, priority, CLS_MIN_VERSION);
> @@ -2473,12 +2669,13 @@ fte_insert(struct classifier *cls, const struct
> match *match,
>   * with the specified 'index'.  Returns the flow formats able to
> represent the
>   * flows that were read. */
>  static enum ofputil_protocol
> -read_flows_from_file(const char *filename, struct classifier *cls, int
> index)
> +read_flows_from_file(const char *filename, struct hmap *ht_cls, int index)
>  {
>      enum ofputil_protocol usable_protocols;
>      int line_number;
>      struct ds s;
>      FILE *file;
> +    struct classifier *cls;
>
>      file = !strcmp(filename, "-") ? stdin : fopen(filename, "r");
>      if (file == NULL) {
> @@ -2488,7 +2685,7 @@ read_flows_from_file(const char *filename, struct
> classifier *cls, int index)
>      ds_init(&s);
>      usable_protocols = OFPUTIL_P_ANY;
>      line_number = 0;
> -    classifier_defer(cls);
> +
>      while (!ds_get_preprocessed_line(&s, file, &line_number)) {
>          struct fte_version *version;
>          struct ofputil_flow_mod fm;
> @@ -2510,10 +2707,27 @@ read_flows_from_file(const char *filename, struct
> classifier *cls, int index)
>                                       | OFPUTIL_FF_EMERG);
>          version->ofpacts = fm.ofpacts;
>          version->ofpacts_len = fm.ofpacts_len;
> +        if (fm.table_id == 0xff) {
> +            fm.table_id = 0x0;
> +        }
> +
> +        if (!list_is_empty(&tables)
> +                && bmtables
> +                && !bitmap_is_set(bmtables, fm.table_id)) {
> +            /*
> +             * Ignore this flow if --tables is specified but does
> +             * not contain the table_id.
> +             */
> +            continue;
> +        }
>
> +        cls = get_table_classifier(ht_cls, fm.table_id);
> +        if (cls == NULL) {
> +            cls = insert_new_table_classifier(ht_cls, fm.table_id);
> +        }
>          fte_insert(cls, &fm.match, fm.priority, version, index);
>      }
> -    classifier_publish(cls);
> +
>      ds_destroy(&s);
>
>      if (file != stdin) {
> @@ -2582,8 +2796,9 @@ recv_flow_stats_reply(struct vconn *vconn, ovs_be32
> send_xid,
>  static void
>  read_flows_from_switch(struct vconn *vconn,
>                         enum ofputil_protocol protocol,
> -                       struct classifier *cls, int index)
> +                       struct hmap *ht_cls, int index)
>  {
> +    struct classifier *cls;
>      struct ofputil_flow_stats_request fsr;
>      struct ofputil_flow_stats fs;
>      struct ofpbuf *request;
> @@ -2593,16 +2808,29 @@ read_flows_from_switch(struct vconn *vconn,
>
>      fsr.aggregate = false;
>      match_init_catchall(&fsr.match);
> -    fsr.out_port = OFPP_ANY;
> -    fsr.table_id = 0xff;
> +
> +    /* Only get tables that are not hidden. */
> +    update_bitmaps_with_supported_table_ids(vconn);
> +
> +    if (list_size(&tables) == 1) {
> +        /* Only retrieve flows from the specified table. */
> +        struct tables_node * tnode;
> +        tnode  = CONTAINER_OF(list_front(&tables), struct tables_node,
> lnode);
> +        fsr.table_id = tnode->table_id;
> +    }
> +    else {
> +        fsr.table_id = 0xff;
> +    }
>      fsr.cookie = fsr.cookie_mask = htonll(0);
> +    fsr.out_group = OFPG11_ANY;
> +    fsr.out_port = OFPP_ANY;
> +
>      request = ofputil_encode_flow_stats_request(&fsr, protocol);
>      send_xid = ((struct ofp_header *) request->data)->xid;
>      send_openflow_buffer(vconn, request);
>
>      reply = NULL;
>      ofpbuf_init(&ofpacts, 0);
> -    classifier_defer(cls);
>      while (recv_flow_stats_reply(vconn, send_xid, &reply, &fs, &ofpacts))
> {
>          struct fte_version *version;
>
> @@ -2614,35 +2842,48 @@ read_flows_from_switch(struct vconn *vconn,
>          version->flags = 0;
>          version->ofpacts_len = fs.ofpacts_len;
>          version->ofpacts = xmemdup(fs.ofpacts, fs.ofpacts_len);
> -
> +        if (!list_is_empty(&tables)
> +                && bmtables
> +                && !bitmap_is_set(bmtables, fs.table_id)) {
> +            /*
> +             * Ignore this flow if --tables is specified but does
> +             * not contain the table_id.
> +             */
> +            continue;
> +        }
> +        cls = get_table_classifier(ht_cls, fs.table_id);
> +        if (cls == NULL) {
> +            cls = insert_new_table_classifier(ht_cls, fs.table_id);
> +        }
>          fte_insert(cls, &fs.match, fs.priority, version, index);
>      }
> -    classifier_publish(cls);
>      ofpbuf_uninit(&ofpacts);
>  }
>
>  static void
> -fte_make_flow_mod(const struct fte *fte, int index, uint16_t command,
> -                  enum ofputil_protocol protocol, struct ovs_list
> *packets)
> +fte_make_flow_mod(const struct fte *fte,
> +                  int index, uint16_t command,
> +                  enum ofputil_protocol protocol,
> +                  struct ovs_list *packets, uint8_t table_id)
>  {
>      const struct fte_version *version = fte->versions[index];
>      struct ofputil_flow_mod fm;
>      struct ofpbuf *ofm;
> -
>      minimatch_expand(&fte->rule.match, &fm.match);
>      fm.priority = fte->rule.priority;
>      fm.cookie = htonll(0);
>      fm.cookie_mask = htonll(0);
>      fm.new_cookie = version->cookie;
>      fm.modify_cookie = true;
> -    fm.table_id = 0xff;
> +    fm.table_id = table_id;
>      fm.command = command;
>      fm.idle_timeout = version->idle_timeout;
>      fm.hard_timeout = version->hard_timeout;
> -    fm.importance = version->importance;
>      fm.buffer_id = UINT32_MAX;
>      fm.out_port = OFPP_ANY;
> +    fm.out_group = OFPG_ANY;
>      fm.flags = version->flags;
> +    fm.importance = version->importance;
>      if (command == OFPFC_ADD || command == OFPFC_MODIFY ||
>          command == OFPFC_MODIFY_STRICT) {
>          fm.ofpacts = version->ofpacts;
> @@ -2657,93 +2898,153 @@ fte_make_flow_mod(const struct fte *fte, int
> index, uint16_t command,
>      list_push_back(packets, &ofm->list_node);
>  }
>
> +
> +static void
> +construct_flow_mods(struct ovs_list *requests,
> +                    enum ofputil_protocol protocol,
> +                    struct fte *fte,
> +                    uint8_t table_id)
> +{
> +    enum { FILE_IDX = 0, SWITCH_IDX = 1 };
> +    struct fte_version *file_ver = fte->versions[FILE_IDX];
> +    struct fte_version *sw_ver = fte->versions[SWITCH_IDX];
> +
> +    if (sw_ver && !file_ver) {
> +        fte_make_flow_mod(fte, SWITCH_IDX,
> +                          OFPFC_DELETE_STRICT,
> +                          protocol, requests,
> +                          table_id);
> +    }
> +    else if (file_ver
> +                && (readd || !sw_ver)) {
> +            fte_make_flow_mod(fte, FILE_IDX, OFPFC_ADD,
> +                              protocol, requests,
> +                              table_id);
> +    }
> +    else if (file_ver
> +                && sw_ver && !fte_version_equals(sw_ver, file_ver)) {
> +            fte_make_flow_mod(fte, FILE_IDX,
> +                              OFPFC_MODIFY_STRICT,
> +                              protocol, requests,
> +                              table_id);
> +    }
> +}
> +
> +static void
> +add_update_delete_switch_flows(struct ovs_list *requests,
> +                               struct hmap *ht_cls,
> +                               enum ofputil_protocol protocol)
> +{
> +    struct classifier *cls;
> +    struct fte *fte;
> +
> +    if (!list_is_empty(&tables)) {
> +        struct tables_node *tnode;
> +        LIST_FOR_EACH (tnode, lnode, &tables) {
> +            cls = get_table_classifier(ht_cls, tnode->table_id);
> +            if (cls == NULL) {
> +                /* Trying to get a flow that does not exist. */
> +                continue;
> +            }
> +            classifier_publish(cls);
> +            CLS_FOR_EACH (fte, rule, cls) {
> +                construct_flow_mods(requests, protocol, fte,
> tnode->table_id);
> +            }
> +            fte_free_all(cls);
> +        }
> +    }
> +    else {
> +        struct hmap_table_classifier_node *ht_cls_node;
> +        HMAP_FOR_EACH (ht_cls_node, hnode, ht_cls) {
> +            cls = get_table_classifier(ht_cls, ht_cls_node->table_id);
> +            if (cls == NULL) {
> +                /* Trying to get a flow that does not exist. */
> +                continue;
> +            }
> +            classifier_publish(cls);
> +            CLS_FOR_EACH (fte, rule, cls) {
> +                construct_flow_mods(requests,
> +                                    protocol,
> +                                    fte,
> +                                    ht_cls_node->table_id);
> +            }
> +            fte_free_all(cls);
> +        }
> +    }
> +}
> +
>  static void
>  ofctl_replace_flows(struct ovs_cmdl_context *ctx)
>  {
>      enum { FILE_IDX = 0, SWITCH_IDX = 1 };
>      enum ofputil_protocol usable_protocols, protocol;
> -    struct classifier cls;
> +
> +    struct hmap ht_cls;
>      struct ovs_list requests;
>      struct vconn *vconn;
> -    struct fte *fte;
>
> -    classifier_init(&cls, NULL);
> -    usable_protocols = read_flows_from_file(ctx->argv[2], &cls, FILE_IDX);
>
> -    protocol = open_vconn(ctx->argv[1], &vconn);
> -    protocol = set_protocol_for_flow_dump(vconn, protocol,
> usable_protocols);
> +    /* Initialize the hash map. */
> +    hmap_init(&ht_cls);
>
> -    read_flows_from_switch(vconn, protocol, &cls, SWITCH_IDX);
> +    /* Read flows from the file. */
> +    usable_protocols = read_flows_from_file(ctx->argv[2], &ht_cls,
> FILE_IDX);
>
> -    list_init(&requests);
> +    /* Setup connection to the switch. */
> +    protocol = open_vconn_for_flow_mod(ctx->argv[1], &vconn,
> usable_protocols);
> +    protocol = set_protocol_for_flow_dump(vconn, protocol,
> usable_protocols);
>
> -    /* Delete flows that exist on the switch but not in the file. */
> -    CLS_FOR_EACH (fte, rule, &cls) {
> -        struct fte_version *file_ver = fte->versions[FILE_IDX];
> -        struct fte_version *sw_ver = fte->versions[SWITCH_IDX];
> +    /* Generate switch dump request. */
> +    read_flows_from_switch(vconn, protocol, &ht_cls, SWITCH_IDX);
>
> -        if (sw_ver && !file_ver) {
> -            fte_make_flow_mod(fte, SWITCH_IDX, OFPFC_DELETE_STRICT,
> -                              protocol, &requests);
> -        }
> -    }
> +    list_init(&requests);
>
> -    /* Add flows that exist in the file but not on the switch.
> -     * Update flows that exist in both places but differ. */
> -    CLS_FOR_EACH (fte, rule, &cls) {
> -        struct fte_version *file_ver = fte->versions[FILE_IDX];
> -        struct fte_version *sw_ver = fte->versions[SWITCH_IDX];
> +    /* Delete flows that exist on the switch but not in the file.
> +     * Add flows that exist in the file but not on the switch.
> +     * Update the flows that exist in both places but differ.
> +     * If --tables is specified, iterate through tables,
> +     * perform deletion in order. */
> +    add_update_delete_switch_flows(&requests, &ht_cls, protocol);
>
> -        if (file_ver
> -            && (readd || !sw_ver || !fte_version_equals(sw_ver,
> file_ver))) {
> -            fte_make_flow_mod(fte, FILE_IDX, OFPFC_ADD, protocol,
> &requests);
> -        }
> -    }
>      if (bundle) {
>          bundle_transact(vconn, &requests, OFPBF_ORDERED | OFPBF_ATOMIC);
>      } else {
>          transact_multiple_noreply(vconn, &requests);
>      }
> +    hmap_destroy(&ht_cls);
>      vconn_close(vconn);
> -
> -    fte_free_all(&cls);
>  }
>
>  static void
> -read_flows_from_source(const char *source, struct classifier *cls, int
> index)
> +read_flows_from_source(const char *source, struct hmap *ht_cls, int index)
>  {
>      struct stat s;
> -
>      if (source[0] == '/' || source[0] == '.'
>          || (!strchr(source, ':') && !stat(source, &s))) {
> -        read_flows_from_file(source, cls, index);
> +        read_flows_from_file(source, ht_cls, index);
>      } else {
>          enum ofputil_protocol protocol;
>          struct vconn *vconn;
>
> +        /* Setup connection to switch and generate dump request. */
>          protocol = open_vconn(source, &vconn);
>          protocol = set_protocol_for_flow_dump(vconn, protocol,
> OFPUTIL_P_ANY);
> -        read_flows_from_switch(vconn, protocol, cls, index);
> +        read_flows_from_switch(vconn, protocol, ht_cls, index);
>          vconn_close(vconn);
>      }
>  }
>
> -static void
> -ofctl_diff_flows(struct ovs_cmdl_context *ctx)
> +bool
> +print_diff_flows(struct classifier *cls)
>  {
> -    bool differences = false;
> -    struct classifier cls;
>      struct ds a_s, b_s;
>      struct fte *fte;
> -
> -    classifier_init(&cls, NULL);
> -    read_flows_from_source(ctx->argv[1], &cls, 0);
> -    read_flows_from_source(ctx->argv[2], &cls, 1);
> +    bool differences = false;
>
>      ds_init(&a_s);
>      ds_init(&b_s);
> -
> -    CLS_FOR_EACH (fte, rule, &cls) {
> +    classifier_publish(cls);
> +    CLS_FOR_EACH (fte, rule, cls) {
>          struct fte_version *a = fte->versions[0];
>          struct fte_version *b = fte->versions[1];
>
> @@ -2761,12 +3062,46 @@ ofctl_diff_flows(struct ovs_cmdl_context *ctx)
>              }
>          }
>      }
> -
> +    fte_free_all(cls);
>      ds_destroy(&a_s);
>      ds_destroy(&b_s);
> +    return differences;
> +}
>
> -    fte_free_all(&cls);
> -
> +static void
> +ofctl_diff_flows(struct ovs_cmdl_context *ctx)
> +{
> +    bool differences = false;
> +    struct classifier *cls;
> +    struct hmap ht_cls;
> +
> +    hmap_init(&ht_cls);
> +    read_flows_from_source(ctx->argv[1], &ht_cls, 0);
> +    read_flows_from_source(ctx->argv[2], &ht_cls, 1);
> +
> +    if (!list_is_empty(&tables)) {
> +        struct tables_node *tnode;
> +        LIST_FOR_EACH (tnode, lnode, &tables) {
> +            cls = get_table_classifier(&ht_cls, tnode->table_id);
> +            if (cls == NULL) {
> +                /* Trying to diff a flow that does not exist. */
> +                continue;
> +            }
> +            differences = print_diff_flows(cls);
> +        }
> +    }
> +    else {
> +        struct hmap_table_classifier_node *ht_cls_node;
> +        HMAP_FOR_EACH (ht_cls_node, hnode, &ht_cls) {
> +            cls = get_table_classifier(&ht_cls, ht_cls_node->table_id);
> +            if (cls == NULL) {
> +                /* Trying to diff a flow that does not exist. */
> +                continue;
> +            }
> +            differences = print_diff_flows(cls);
> +        }
> +    }
> +    hmap_destroy(&ht_cls);
>      if (differences) {
>          exit(2);
>      }
> --
> 2.3.5
>
>



More information about the dev mailing list