summaryrefslogtreecommitdiff
path: root/ofproto/ofproto-provider.h
Commit message (Collapse)AuthorAgeFilesLines
* ofproto-dpif-upcall: Wait for valid hw flow stats before applying ↵Eelco Chaudron2023-03-151-0/+5
| | | | | | | | | | | | | | | | | min-revalidate-pps. Depending on the driver implementation, it can take from 0.2 seconds up to 2 seconds before offloaded flow statistics are updated. This is true for both TC and rte_flow-based offloading. This is causing a problem with min-revalidate-pps, as old statistic values are used during this period. This fix will wait for at least 2 seconds, by default, before assuming no packets where received during this period. Reviewed-by: Simon Horman <simon.horman@corigine.com> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* openflow: Add extension to flush CT by generic match.Ales Musil2023-01-161-2/+5
| | | | | | | | | | | Add extension that allows to flush connections from CT by specifying fields that the connections should be matched against. This allows to match only some fields of the connection e.g. source address for orig direction. Reported-at: https://bugzilla.redhat.com/2120546 Signed-off-by: Ales Musil <amusil@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* ofp-monitor: Support flow monitoring for OpenFlow 1.3, 1.4+.Vasu Dasari2022-04-281-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 introduced 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 modules 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 <vdasari@gmail.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/383915.html Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* ofproto: Add refcount to ofproto to fix ofproto use-after-free.Peng He2022-03-071-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | From hepeng: https://patchwork.ozlabs.org/project/openvswitch/patch/20200717015041.82746-1-hepeng.0320@bytedance.com/#2487473 also from guohongzhi <guohongzhi1@huawei.com>: http://patchwork.ozlabs.org/project/openvswitch/patch/20200306130555.19884-1-guohongzhi1@huawei.com/ also from a discussion about the mixing use of RCU and refcount in the mail list with Ilya Maximets, William Tu, Ben Pfaf, and Gaëtan Rivet. A summary, as quoted from Ilya: " RCU for ofproto was introduced for one and only one reason - to avoid freeing ofproto while rules are still alive. This was done in commit f416c8d61601 ("ofproto: RCU postpone rule destruction."). The goal was to allow using rules without refcounting them within a single grace period. And that forced us to postpone destruction of the ofproto for a single grace period. Later commit 39c9459355b6 ("Use classifier versioning.") made it possible for rules to be alive for more than one grace period, so the commit made ofproto wait for 2 grace periods by double postponing. As we can see now, that wasn't enough and we have to wait for more than 2 grace periods in certain cases. " In a short, the ofproto should have a longer life time than rule, if the rule lasts for more than 2 grace periods, the ofproto should live longer to ensure rule->ofproto is valid. It's hard to predict how long a ofproto should live, thus we need to use refcount on ofproto to make things easy. The controversial part is that we have already used RCU postpone to delay ofproto destrution, if we have to add refcount, is it simpler to use just refcount without RCU postpone? IMO, I think going back to the pure refcount solution is more complicated than mixing using both. Gaëtan Rive asks some questions on guohongzhi's v2 patch: during ofproto_rule_create, should we use ofproto_ref or ofproto_try_ref? how can we make sure the ofproto is alive? By using RCU, ofproto has three states: state 1: alive, with refcount >= 1 state 2: dying, with refcount == 0, however pointer is valid state 3: died, memory freed, pointer might be dangling. Without using RCU, there is no state 2, thus, we have to be very careful every time we see a ofproto pointer. In contrast, with RCU, we can be sure that it's alive at least in this grace peroid, so we can just check if it is dying by ofproto_try_ref. This shows that by mixing use of RCU and refcount we can save a lot of work worrying if ofproto is dangling. In short, the RCU part makes sure the ofproto is alive when we use it, and the refcount part makes sure it lives longer enough. In this patch, I have merged guohongzhi's patch and mine, and fixes accoring to the previous comments. Acked-by: Adrian Moreno <amorenoz@redhat.com> Acked-by: Gaetan Rivet <grive@u256.net> Acked-by: Mike Pattrick <mkp@redhat.com> Acked-by: Alin-Gabriel Serdean <aserdean@ovn.org> Tested-by: Alin-Gabriel Serdean <aserdean@ovn.org> Signed-off-by: Peng He <hepeng.0320@bytedance.com> Co-authored-by: Hongzhi Guo <guohongzhi1@huawei.com> Signed-off-by: Hongzhi Guo <guohongzhi1@huawei.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* ofproto: Fix resource usage explosion due to removal of large number of flows.Ilya Maximets2021-11-301-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While removing flows, removal itself is deferred, so classifier changes performed already from the RCU thread. This way every deferred removal triggers classifier change and reallocation of a pvector. Freeing of old version of a pvector is postponed. Since all this is happening from an RCU thread, all these copies of the same pvector will be freed only after the next grace period. Below is the example output of the 'valgrind --tool=massif' from an OVN deployment, where copies of that pvector took 5 GB of memory while processing a bundled flow removal: ------------------------------------------------------------------- n time(i) total(B) useful-heap(B) extra-heap(B) ------------------------------------------------------------------- 89 176,257,987,954 5,329,763,160 5,318,171,607 11,591,553 99.78% (5,318,171,607B) (heap allocation functions) malloc/new/new[] ->98.45% (5,247,008,392B) xmalloc__ (util.c:137) |->98.17% (5,232,137,408B) pvector_impl_dup (pvector.c:48) ||->98.16% (5,231,472,896B) pvector_remove (pvector.c:159) |||->98.16% (5,231,472,800B) destroy_subtable (classifier.c:1558) ||||->98.16% (5,231,472,800B) classifier_remove (classifier.c:792) |||| ->98.16% (5,231,472,800B) classifier_remove_assert (classifier.c:832) |||| ->98.16% (5,231,472,800B) remove_rule_rcu__ (ofproto.c:2978) |||| ->98.16% (5,231,472,800B) remove_rule_rcu (ofproto.c:2990) |||| ->98.16% (5,231,472,800B) ovsrcu_call_postponed (ovs-rcu.c:346) |||| ->98.16% (5,231,472,800B) ovsrcu_postpone_thread (ovs-rcu.c:362) |||| ->98.16% (5,231,472,800B) ovsthread_wrapper |||| ->98.16% (5,231,472,800B) start_thread |||| ->98.16% (5,231,472,800B) clone Collecting all the flows to be removed and postponing removal for all of them together to avoid the problem. This way all removals will trigger only a single pvector re-allocation greatly reducing the CPU and memory usage. Reported-by: Vladislav Odintsov <odivlad@gmail.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-November/389538.html Tested-by: Vladislav Odintsov <odivlad@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* ofproto: Fix resource usage explosion while processing bundled FLOW_MOD.Ilya Maximets2021-11-301-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While processing a bundle, OVS will add all new and modified rules to classifiers. Classifiers are using RCU-protected pvector to store subtables. Addition of a new subtable or removal of the old one leads to re-allocation and memory copy of the pvector array. Old version of that array is given to RCU thread to free it later. The problem is that bundle is processed under the mutex without entering the quiescent state. Therefore, memory can not be freed until the whole bundle is processed. So, if a few thousands of flows added to the same table in a bundle, pvector will be re-allocated while adding each of them. So, we'll have a few thousands of copies of the same array waiting to be freed. In case of OVN deployments, there could be hundreds of thousands of flows in the same table leading to a fast consumption of a huge amount of memory and wasting a lot of CPU cycles on allocations and copies. Below snippet of the 'valgrind --tool=massif' output shows ovs-vswitchd consuming 3.5GB of RAM while processing a bundle with 65K FLOW_MODs in the OVN deployment. 3.4GB of that memory are copies of the same pvector. ------------------------------------------------------------------- n time(i) total(B) useful-heap(B) extra-heap(B) ------------------------------------------------------------------- 64 109,907,465,404 3,559,987,568 3,546,879,748 13,107,820 99.63% (3,546,879,748B) (heap allocation functions) malloc/new/new[] ->97.61% (3,474,750,333B) xmalloc__ (util.c:137) |->97.61% (3,474,750,333B) xmalloc (util.c:172) | ->96.38% (3,431,068,352B) pvector_impl_dup (pvector.c:48) || ->96.38% (3,431,067,840B) pvector_insert (pvector.c:138) || |->96.38% (3,431,067,840B) classifier_replace (classifier.c:664) || | ->96.38% (3,431,067,840B) classifier_insert (classifier.c:695) || | ->96.38% (3,431,067,840B) replace_rule_start (ofproto.c:5563) || | ->96.38% (3,431,067,840B) add_flow_start (ofproto.c:5179) || | ->96.38% (3,431,067,840B) ofproto_flow_mod_start (ofproto.c:8017) || | ->96.38% (3,431,067,744B) do_bundle_commit (ofproto.c:8168) || | |->96.38% (3,431,067,744B) handle_bundle_control (ofproto.c:8309) || | | ->96.38% (3,431,067,744B) handle_single_part_openflow (ofproto.c:8593) || | | ->96.38% (3,431,067,744B) handle_openflow (ofproto.c:8674) || | | ->96.38% (3,431,067,744B) ofconn_run (connmgr.c:1329) || | | ->96.38% (3,431,067,744B) connmgr_run (connmgr.c:356) || | | ->96.38% (3,431,067,744B) ofproto_run (ofproto.c:1879) || | | ->96.38% (3,431,067,744B) bridge_run__ (bridge.c:3251) || | | ->96.38% (3,431,067,744B) bridge_run (bridge.c:3310) || | | ->96.38% (3,431,067,744B) main (ovs-vswitchd.c:127) Fixing that by postponing the publishing of classifier updates, so each flow modification can work with the same version of pvector. Unfortunately, bundled PACKET_OUT messages requires all previous changes to be published before processing, otherwise the packet will use wrong version of OF tables. Publishing all changes before processing PACKET_OUT messages to avoid this issue. Hopefully, mixup of a big number of FLOW_MOD and PACKET_OUT messages is not a common usecase. Reported-by: Vladislav Odintsov <odivlad@gmail.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-November/389503.html Tested-by: Vladislav Odintsov <odivlad@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* ofproto: Change type of n_handlers and n_revalidators.Mark Gray2021-07-161-1/+1
| | | | | | | | | | | 'n_handlers' and 'n_revalidators' are declared as type 'size_t'. However, dpif_handlers_set() requires parameter 'n_handlers' as type 'uint32_t'. This patch fixes this type mismatch. Signed-off-by: Mark Gray <mark.d.gray@redhat.com> Acked-by: Flavio Leitner <fbl@sysclose.org> Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* Eliminate use of term "slave" in bond, LACP, and bundle contexts.Ben Pfaff2020-10-211-6/+6
| | | | | | | | | | | | | The new term is "member". Most of these changes should not change user-visible behavior. One place where they do is in "ovs-ofctl dump-flows", which will now output "members:..." inside "bundle" actions instead of "slaves:...". I don't expect this to cause real problems in most systems. The old syntax is still supported on input for backward compatibility. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
* Add offload packets statisticszhaozhanxu2019-12-061-2/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add argument '--offload-stats' for command ovs-appctl bridge/dump-flows to display the offloaded packets statistics. The commands display as below: orignal command: ovs-appctl bridge/dump-flows br0 duration=574s, n_packets=1152, n_bytes=110768, priority=0,actions=NORMAL table_id=254, duration=574s, n_packets=0, n_bytes=0, priority=2,recirc_id=0,actions=drop table_id=254, duration=574s, n_packets=0, n_bytes=0, priority=0,reg0=0x1,actions=controller(reason=) table_id=254, duration=574s, n_packets=0, n_bytes=0, priority=0,reg0=0x2,actions=drop table_id=254, duration=574s, n_packets=0, n_bytes=0, priority=0,reg0=0x3,actions=drop new command with argument '--offload-stats' Notice: 'n_offload_packets' are a subset of n_packets and 'n_offload_bytes' are a subset of n_bytes. ovs-appctl bridge/dump-flows --offload-stats br0 duration=582s, n_packets=1152, n_bytes=110768, n_offload_packets=1107, n_offload_bytes=107992, priority=0,actions=NORMAL table_id=254, duration=582s, n_packets=0, n_bytes=0, n_offload_packets=0, n_offload_bytes=0, priority=2,recirc_id=0,actions=drop table_id=254, duration=582s, n_packets=0, n_bytes=0, n_offload_packets=0, n_offload_bytes=0, priority=0,reg0=0x1,actions=controller(reason=) table_id=254, duration=582s, n_packets=0, n_bytes=0, n_offload_packets=0, n_offload_bytes=0, priority=0,reg0=0x2,actions=drop table_id=254, duration=582s, n_packets=0, n_bytes=0, n_offload_packets=0, n_offload_bytes=0, priority=0,reg0=0x3,actions=drop Signed-off-by: zhaozhanxu <zhaozhanxu@163.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
* ofproto-provider: Move datapath capabilities callback to correct section.Ilya Maximets2019-11-281-2/+3
| | | | | | | | | | | 'get_datapath_cap' callback was mistakenly placed in 'Connection tracking' section of the 'struct dpif_class' while belongs to the 'Datapath information'. CC: William Tu <u9012063@gmail.com> Fixes: 27501802d09f ("ofproto-dpif: Expose datapath capability to ovsdb.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: William Tu <u9012063@gmail.com>
* ofproto: Fix crash on PACKET_OUT due to recursive locking after upcall.Ilya Maximets2019-11-261-3/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Handling of OpenFlow PACKET_OUT implies pushing the packet through the datapath and packet processing inside the datapath could trigger an upcall. In case of system datapath, 'dpif_execute()' sends packet to the kernel module and returns. If any, upcall will be triggered inside the kernel and handled by a separate thread in userspace. But in case of userspace datapath full processing of the packet happens inside the 'dpif_execute()' in the same thread that handled PACKET_OUT. This causes an issue if upcall will lead to modification of flow rules. For example, it could happen while processing of 'learn' actions. Since whole handling of PACKET_OUT is protected by 'ofproto_mutex', OVS will assert on attempt to take it recursively while processing 'learn' actions: 0 __GI_raise (sig=sig@entry=6) 1 __GI_abort () 2 ovs_abort_valist () 3 ovs_abort () 4 ovs_mutex_lock_at (where=where@entry=0xad4199 "ofproto/ofproto.c:5391") <Trying to acquire ofproto_mutex again> 5 ofproto_flow_mod_learn () at ofproto/ofproto.c:5391 <Trying to modify flows according to 'learn' action> 6 xlate_learn_action () at ofproto/ofproto-dpif-xlate.c:5378 <'learn' action found> 7 do_xlate_actions () at ofproto/ofproto-dpif-xlate.c:6893 8 xlate_recursively () at ofproto/ofproto-dpif-xlate.c:4233 9 xlate_table_action () at ofproto/ofproto-dpif-xlate.c:4361 10 in xlate_ofpact_resubmit () at ofproto/ofproto-dpif-xlate.c:4672 11 do_xlate_actions () at ofproto/ofproto-dpif-xlate.c:6773 12 xlate_actions () at ofproto/ofproto-dpif-xlate.c:7570 <Translating actions> 13 upcall_xlate () at ofproto/ofproto-dpif-upcall.c:1197 14 process_upcall () at ofproto/ofproto-dpif-upcall.c:1413 15 upcall_cb () at ofproto/ofproto-dpif-upcall.c:1315 16 dp_netdev_upcall (DPIF_UC_MISS) at lib/dpif-netdev.c:6236 <Flow cache miss. Making upcall> 17 handle_packet_upcall () at lib/dpif-netdev.c:6591 18 fast_path_processing () at lib/dpif-netdev.c:6709 19 dp_netdev_input__ () at lib/dpif-netdev.c:6797 20 dp_netdev_recirculate () at lib/dpif-netdev.c:6842 21 dp_execute_cb () at lib/dpif-netdev.c:7158 22 odp_execute_actions () at lib/odp-execute.c:794 23 dp_netdev_execute_actions () at lib/dpif-netdev.c:7332 24 dpif_netdev_execute () at lib/dpif-netdev.c:3725 25 dpif_netdev_operate () at lib/dpif-netdev.c:3756 <Packet pushed to userspace datapath for processing> 26 dpif_operate () at lib/dpif.c:1367 27 dpif_execute () at lib/dpif.c:1321 28 packet_execute () at ofproto/ofproto-dpif.c:4760 29 ofproto_packet_out_finish () at ofproto/ofproto.c:3594 <Taking ofproto_mutex> 30 handle_packet_out () at ofproto/ofproto.c:3635 31 handle_single_part_openflow (OFPTYPE_PACKET_OUT) at ofproto/ofproto.c:8400 32 handle_openflow () at ofproto/ofproto.c:8587 33 ofconn_run () 34 connmgr_run () 35 ofproto_run () 36 bridge_run__ () 37 bridge_run () 38 main () Fix that by splitting the 'ofproto_packet_out_finish()' in two parts. First one that translates side-effects and requires holding 'ofproto_mutex' and the second that only pushes packet to datapath. The second part moved out of 'ofproto_mutex' because 'ofproto_mutex' is not required and actually should not be taken in order to avoid recursive locking. Reported-by: Anil Kumar Koli <anilkumar.k@altencalsoftlabs.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2019-April/048494.html Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Expose datapath capability to ovsdb.William Tu2019-11-211-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The patch adds support for fetching the datapath's capabilities from the result of 'check_support()', and write the supported capability to a new database column, called 'capabilities' under Datapath table. To see how it works, run: # ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev # ovs-vsctl -- --id=@m create Datapath datapath_version=0 \ 'ct_zones={}' 'capabilities={}' \ -- set Open_vSwitch . datapaths:"netdev"=@m # ovs-vsctl list-dp-cap netdev ufid=true sample_nesting=true clone=true tnl_push_pop=true \ ct_orig_tuple=true ct_eventmask=true ct_state=true \ ct_clear=true max_vlan_headers=1 recirc=true ct_label=true \ max_hash_alg=1 ct_state_nat=true ct_timeout=true \ ct_mark=true ct_orig_tuple6=true check_pkt_len=true \ masked_set_action=true max_mpls_depth=3 trunc=true ct_zone=true Signed-off-by: William Tu <u9012063@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> --- v5: Add improved documentation from Ben and fix checkpatch error (tab and line 79 char) v4: rebase to master v3: fix 32-bit build, reported by Greg travis: https://travis-ci.org/williamtu/ovs-travis/builds/599276267 v2: rebase to master
* ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy tablesYi-Hung Wei2019-09-261-0/+9
| | | | | | | | | | | | | | | | | | | | | | | This patch consumes the CT_Zone and CT_Timeout_Policy tables, maintains the zone-based configuration in the vswitchd. Whenever there is a database change, vswitchd will read the datapath, CT_Zone, and CT_Timeout_Policy tables from ovsdb, builds an internal snapshot of the database configuration in bridge.c, and pushes down the change into ofproto and dpif layer. If a new zone-based timeout policy is added, it updates the zone to timeout policy mapping in the per datapath type datapath structure in dpif-backer, and pushes down the timeout policy into the datapath via dpif interface. If a timeout policy is no longer used, for kernel datapath, vswitchd may not be able to remove it from datapath immediately since datapath flows can still reference the to-be-deleted timeout policies. Thus, we keep an timeout policy kill list, that vswitchd will go back to the list periodically and try to kill the unused timeout policies. Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
* upcall: Configure datapath min-revalidate-pps through ovs-vsctl.Vlad Buslov2019-08-211-0/+4
| | | | | | | | | | | This patch adds a new configuration option, "min-revalidate-pps" to the Open_vSwitch "other-config" column. This sets minimum pps that flow must have in order to be revalidated when revalidation duration exceeds half of max-revalidator config variable. Signed-off-by: Vlad Buslov <vladbu@mellanox.com> Acked-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* upcall: Configure datapath max-revalidator through ovs-vsctl.Vlad Buslov2019-08-211-0/+4
| | | | | | | | | | | This patch adds a new configuration option, "max-revalidator" to the Open_vSwitch "other-config" column. This sets maximum allowed ravalidator timeout. Actual timeout value is determined at runtime as minimum of "max-idle" and "max-revalidator". Signed-off-by: Vlad Buslov <vladbu@mellanox.com> Acked-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Return error codes for rule insertions.Aravind Prasad S2019-04-231-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | Currently, rule_insert() API does not have return value. There are some possible scenarios where rule insertions can fail at run-time even though the static checks during rule_construct() had passed previously. Some possible scenarios for failure of rule insertions: **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and Normal switch functioning coexist) where the CAM space could get suddenly filled up by Normal switch functioning and Openflow gets devoid of available space. **) Some deployments could have separate independent layers for HW rule insertions and application layer to interact with OVS. HW layer could face any dynamic issue during rule handling which application could not have predicted/captured in rule-construction phase. Rule-insert errors for bundles are handled too. Testing: Tested failures of rule insertions and also with bundles. Signed-off-by: Aravind Prasad S <aravind.sridharan at dell.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* bridge: Propagate patch port pairing errors to db.Ilya Maximets2019-03-261-0/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Virtual ports like 'patch' ports that almost fully implemented on 'ofproto' layer could have internal to 'ofproto' statuses that could not be retrieved from 'netdev' or other layers. For example, in current implementation there is no way to get the patch port pairing status (i.e. if it has usable peer?). New 'ofproto-provider' API function 'vport_get_status' introduced to cover this gap. It allowes 'bridge' layer to retrive current status of ofproto virtual ports and propagate it to DB. For now we're only interested in pairing errors of 'patch' ports. That are propagated to the 'error' column of the 'Interface' table. Ex.: $ ovs-vsctl show ... Bridge "br1" ... Port "patch1" Interface "patch1" type: patch options: {peer="patch0"} error: "No usable peer 'patch0' exists in 'system' datapath." Bridge "br0" datapath_type: netdev ... Port "patch0" Interface "patch0" type: patch options: {peer="patch1"} error: "No usable peer 'patch1' exists in 'netdev' datapath." Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* treewide: Get rid of // comments, even inside comments.Ben Pfaff2019-01-251-3/+3
| | | | | | | | | | | | | Just a style fix. With this patch, the following reports no hits: git ls-files | grep '\.[ch]$' | grep -vE 'datapath|sflow' \ | xargs grep -n // | grep -vE "http|s/|'|\"" Acked-by: Ilya Maximets <i.maximets@samsung.com> Reported-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ovs-vswitchd: Implement OFPT_TABLE_FEATURES table modification request.Ben Pfaff2019-01-151-0/+19
| | | | | | | | | This allows a controller to change the name of OpenFlow flow tables in the OVS software switch. CC: Brad Cowie <brad@cowie.nz> Acked-by: Justin Pettit <jpettit@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Consistently force off OFPPS_LIVE if port or link is down.Ben Pfaff2018-10-181-0/+1
| | | | | | | | It doesn't make sense for a port that is down to be "live" from OpenFlow's point of view, but this could happen in OVS. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Numan Siddique <nusididq@redhat.com>
* ofproto: Move may_enable from ofport_dpif to ofport.Ben Pfaff2018-10-181-0/+1
| | | | | | | | | | | This concept of whether a port is suitable to be "live" in the sense of the OpenFlow OFPPS_LIVE bit is a generic one that can be handled at the ofproto layer instead of needing to be part of ofproto-dpif. An upcoming commit will make more use of this at the ofproto layer. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Numan Siddique <nusididq@redhat.com>
* ofproto-dpif: Use dp_hash as default selection methodJan Scheurich2018-05-251-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The dp_hash selection method for select groups overcomes the scalability problems of the current default selection method which, due to L2-L4 hashing during xlation and un-wildcarding of the hashed fields, basically requires an upcall to the slow path to load-balance every L4 connection. The consequence are an explosion of datapath flows (megaflows degenerate to miniflows) and a limitation of connection setup rate OVS can handle. This commit changes the default selection method to dp_hash, provided the bucket configuration is such that the dp_hash method can accurately represent the bucket weights with up to 64 hash values. Otherwise we stick to original default hash method. We use the new dp_hash algorithm OVS_HASH_L4_SYMMETRIC to maintain the symmetry property of the old default hash method. A controller can explicitly request the old default hash selection method by specifying selection method "hash" with an empty list of fields in the Group properties of the OpenFlow 1.5 Group Mod message. Update the documentation about selection method in the ovs-ovctl man page. Revise and complete the ofproto-dpif unit tests cases for select groups. Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com> Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com> Co-authored-by: Nitin Katiyar <nitin.katiyar@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofp-flow: Reduce memory consumption for ofputil_flow_mod, using minimatch.Ben Pfaff2018-03-311-1/+1
| | | | | | | | | | | | | | | | | | | | | | | Until now, struct ofputil_flow_mod, which represents an OpenFlow flow table modification request, has incorporated a struct match, which made the overall ofputil_flow_mod about 2.5 kB. This is OK for a small number of flows, but absurdly inflates memory requirements when there are hundreds of thousands of flows. This commit fixes the problem by changing struct match to struct minimatch inside ofputil_flow_mod, which reduces its size to about 100 bytes plus the actual size of the flow match (usually a few dozen bytes). This affects memory usage of ovs-ofctl (when it adds a large number of flows) more than ovs-vswitchd. Reported-by: Michael Ben-Ami <mbenami@digitalocean.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Armando Migliaccio <armamig@gmail.com> Tested-by: Armando Migliaccio <armamig@gmail.com> Reviewed-by: Jan Scheurich <jan.scheurich@ericsson.com> Tested-by: Jan Scheurich <jan.scheurich@ericsson.com> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* ofp-util, ofp-parse: Break up into many separate modules.Ben Pfaff2018-02-131-1/+7
| | | | | | | | | | | | ofp-util had been far too large and monolithic for a long time. This commit breaks it up into units that make some logical sense. It also moves the pieces of ofp-parse that were specific to each unit into the relevant unit. Most of this commit is just moving code around. Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* ofproto: Fix double-unref of temporary rule when learning.Ben Pfaff2018-01-261-1/+4
| | | | | | | | | | When ofproto_flow_mod_init() accepts a rule, it takes ownership of it and either unrefs it on error or transfers ownership to the struct it initializes on success, but ofproto_flow_mod_init_for_learn() was unref-ing it a second time if it reported an error. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: William Tu <u9012063@gmail.com>
* vswitchd: Add --cleanup option to the 'appctl exit' commandAndy Zhou2017-05-031-1/+1
| | | | | | | | | | | | | | | | | | 'appctl exit' stops the running vswitchd daemon, without releasing the datapath resources (such as bridges and ports) that vswitchd has created. This is expected when vswitchd is to be relaunched, to reduce the perturbation of exiting traffic and connections. However, when vswitchd is intended to be shutdown permanently, it is desirable not to leak datapath resources. In theory, this can be achieved by removing the corresponding configurations from OVSDB before shutting down vswitchd. However it is not always possible in practice. Sometimes it is convenient and robust for vswitchd to release all datapath resources that it has configured. Add 'appctl exit --cleanup' option for this use case. Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
* ofproto: Support action upcall metersAndy Zhou2017-04-281-0/+4
| | | | | | | | | | | Allow action upcall meters, i.e. slowpath and controller meters, to be added and displayed. Keep track of datapath meter ID of those action upcall meters in ofproto to aid action translation. Later patches will make use of them. Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
* ofproto: Store meters using hmapAndy Zhou2017-04-281-5/+2
| | | | | | | | | | | | | | | | | Currently, meters are stored in a fixed pointer array. It is not very efficient since the controller, at least in theory, can pick any meter id (up to the limits to uint32_t), not necessarily within the lower end of a region, or in close range to each other. In particular, OFPM_SLOWPATH and OFPM_CONTROLLER meters are specified at the high region. Switching to using hmap. Ofproto layer does not restrict the number of meters that controller can add, nor does it care about the value of meter_id. Datapth limits the number of meters ofproto layer can support at run time. Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
* ofp-actions: Add limit to learn action.Daniele Di Proietto2017-03-161-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds a new feature to the learn actions: the possibility to limit the number of learned flows. To be compatible with users of the old learn action, a new structure is introduced as well as a new OpenFlow raw action number. There's a small corner case when we have to delete the ukey. This happens when: * The learned rule has expired (or has been deleted). * The ukey that learned the rule is still in the datapath. * No packets hit the datapath flow recently. In this case we cannot relearn the rule (because there are no new packets), and the actions might depend on the learn execution, so the only option is to delete the ukey. I don't think this has big performance implications since it's done only for ukey with no traffic. We could also slowpath it, but that will cause an action upcall and the correct datapath actions will be installed later by a revalidator. If we delete the ukey, the next upcall will be a miss upcall and that will immediatedly install the correct datapath flow. Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Add ref counting for variable length mf_fields.Yi-Hung Wei2017-03-151-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | Currently, a controller may potentially trigger a segmentation fault if it accidentally removes a TLV mapping that is still used by an active flow. To resolve this issue, in this patch, we maintain reference counting for each dynamically allocated variable length mf_fields, so that vswitchd can use this information to properly remove a TLV mapping, and to return an error if the controller tries to remove a TLV mapping that is still used by any active flow. To keep track of the usage of tun_metadata for each flow, two 'uint64_t' bitmaps are introduce for the flow match and flow action respectively. We use 'uint64_t' as a bitmap since the 64 geneve TLV tunnel metadata are the only available variable length mf_fields for now. We shall adopt general bitmap when more variable length mf_fields are introduced. The bitmaps are configured during the flow decoding process, and vswitchd use these bitmaps to increase or decrease the ref counting when the flow is created or deleted. VMWare-BZ: #1768370 Fixes: 04f48a68c428 ("ofp-actions: Fix variable length meta-flow OXMs.") Suggested-by: Jarno Rajahalme <jarno@ovn.org> Suggested-by: Joe Stringer <joe@ovn.org> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
* ofproto: Fix thread safety annotation.Jarno Rajahalme2017-03-081-1/+2
| | | | | | | | ofproto_check_ofpacts() requires ofproto_mutex, but the header did not tell that so the trace did not take the mutex. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Signed-off-by: Andy Zhou <azhou@ovn.org>
* dpif: Meter framework.Jarno Rajahalme2017-03-081-6/+7
| | | | | | | | Add DPIF-level infrastructure for meters. Allow meter_set to modify the meter configuration (e.g. set the burst size if unspecified). Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Signed-off-by: Andy Zhou <azhou@ovn.org>
* meta-flow: Remove cmap dependency.Yi-Hung Wei2017-02-211-1/+1
| | | | | | | | | | | | | | Previous patch 04f48a68 ("ofp-actions: Fix variable length meta-flow OXMs.") introduced dependency of an internal library (cmap.h) to ovs public interface (meta-flow.h) that may cause potential building problem. In this patch, we remove cmap from struct mf_field, and provide a wrapper struct vl_mff_map that resolve the dependency problem. Fixes: 04f48a68c428 ("ofp-actions: Fix variable length meta-flow OXMs.") Suggested-by: Joe Stringer <joe@ovn.org> Suggested-by: Daniele Di Proietto <diproiettod@vmware.com> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
* dpif-netdev: Pass Openvswitch other_config smap to dpif.Daniele Di Proietto2017-02-031-3/+8
| | | | | | | | | | | | | | | | | Currently we parse the 'other_config' column in Openvswitch table in bridge.c. We extract the values (just 'pmd-cpu-mask' for now) and we pass them down to the datapath, via different layers. If we want to pass other values to dpif-netdev.c (like we recently discussed) we would have to touch ofproto.c, ofproto-dpif.c and dpif.c. This patch sends the entire other_config column to dpif-netdev, so that dpif-netdev can extract the values it's interested in. No functional change. Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> Acked-by: Ben Pfaff <blp@ovn.org>
* ofp-actions: Fix variable length meta-flow OXMs.Yi-Hung Wei2017-02-011-1/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, if a flow action that involves a tunnel metadata meta-flow field is dumped from vswitchd, the replied field length in the OXM header is filled with the maximum possible field length, instead of the length configured in the tunnel TLV mapping table. To solve this issue, this patch introduces the following changes. In order to maintain the correct length of variable length mf_fields (i.e. tun_metadata), this patch creates a per-switch based map (struct vl_mff_map) that hosts the variable length mf_fields. This map is updated when a controller adds/deletes tlv-mapping entries to/from a switch. Although the per-swtch based vl_mff_map only hosts tun_metadata for now, it is able to support new variable length mf_fields in the future. With this commit, when a switch decodes a flow action with mf_field, the switch firstly looks up the global mf_fields map to identify the mf_field type. For the variable length mf_fields, the switch uses the vl_mff_map to get the configured mf_field entries. By lookig up vl_mff_map, the switch can check if the added flow action access beyond the configured size of a variable length mf_field, and the switch reports an ofperr if the controller adds a flow with unmapped variable length mf_field. Later on, when a controller request flows from the switch, with the per-switch based mf_fields, the switch will encode the OXM header with correct length for variable length mf_fields. To use the vl_mff_map for decoding flow actions, extract-ofp-actions is updated to pass the vl_mff_map to the required action decoding functions. Also, a new error code is introduced to identify a flow with an invalid variable length mf_field. Moreover, a testcase is added to prevent future regressions. Committer notes: - Factor out common code - Style fixups - Rename OFPERR_NXFMFC_INVALID_VL_MFF -> OFPERR_NXFMFC_INVALID_TLV_FIELD VMWare-BZ: #1768370 Reported-by: Harold Lim <haroldl@vmware.com> Suggested-by: Joe Stringer <joe@ovn.org> Suggested-by: Jarno Rajahalme <jarno@ovn.org> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Joe Stringer <joe@ovn.org>
* ofproto: Update access rules for 'flow_cookie'.Ben Pfaff2016-12-211-2/+2
| | | | | | | | | | | The 'flow_cookie' member of struct rule is read during flow translation without holding a mutex, breaking the documented OVS_GUARDED access rule. However, the flow_cookie member is actually never modified after a rule is initialized, so this commit changes the access rules to reflect that. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Lance Richardson <lrichard@redhat.com>
* ofproto: Honor OFPFF_RESET_COUNTS flag in flow modify message.Jarno Rajahalme2016-12-051-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While a flow modify must keep the original flow's flags, it must reset counts if (and only if) the reset_counts flag is present in the flow mod message. Behavior prior to this patch is broken in a few ways: - OpenFlow 1.0 and 1.1 mod-flows did reset the counts, if the flow had reset_counts flag set. Only add-flow should reset counts. - With OpenFlow 1.2 and later, if the old flow had the reset_counts flag set, the counts would be reset by mod-flows, even if the flow-mod message does not have the reset_counts flag set. - With OpenFlow 1.2 and later, mod-flows with a reset_count did not reset the counts, if the old flow did not have the reset_counts flag set. Even though the prevailing interpretation seems to be that the reset_counts flag in the flow-mod message should be stored as part of the flow state (and reported back in flow dumps with OpenFlow >= 1.3), we should always just look at the reset_counts flag in the current flow-mod and ignore the reset_counts flag stored in the flow when processing a flow mod. For OpenFlow 1.0 and 1.1 we already implicitly add the reset_counts flag for add-flow messages (only) to maintain the expected behavior. This patch adds a comprehensive test case to prevent future regressions. Suggested-by: Tony van der Peet <tony.vanderpeet@alliedtelesis.co.nz> Fixes: 748eb2f5b1 ("ofproto-dpif: Always forward 'used' from the old_rule.") Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* Add OpenFlow command to flush conntrack table entries.Justin Pettit2016-09-231-0/+7
| | | | | Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* tun-metadata: Manage tunnel TLV mapping table on a per-bridge basis.Jesse Gross2016-09-191-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When using tunnel TLVs (at the moment, this means Geneve options), a controller must first map the class and type onto an appropriate OXM field so that it can be used in OVS flow operations. This table is managed using OpenFlow extensions. The original code that added support for TLVs made the mapping table global as a simplification. However, this is not really logically correct as the OpenFlow management commands are operating on a per-bridge basis. This removes the original limitation to make the table per-bridge. One nice result of this change is that it is generally clearer whether the tunnel metadata is in datapath or OpenFlow format. Rather than allowing ad-hoc format changes and trying to handle both formats in the tunnel metadata functions, the format is more clearly separated by function. Datapaths (both kernel and userspace) use datapath format and it is not changed during the upcall process. At the beginning of action translation, tunnel metadata is converted to OpenFlow format and flows and wildcards are translated back at the end of the process. As an additional benefit, this change improves performance in some flow setup situations by keeping the tunnel metadata in the original packet format in more cases. This helps when copies need to be made as the amount of data touched is only what is present in the packet rather than the maximum amount of metadata supported. Co-authored-by: Madhu Challa <challa@noironetworks.com> Signed-off-by: Madhu Challa <challa@noironetworks.com> Signed-off-by: Jesse Gross <jesse@kernel.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Support packet_outs in bundles.Jarno Rajahalme2016-09-141-0/+7
| | | | | | | | | | | | | | | | | | | | | | | Add support for OFPT_PACKET_OUT messages in bundles. While ovs-ofctl already has a packet-out command, we did not have a string parser for it, as the parsing was done directly from command line arguments. This patch adds the string parser for packet-out messages, adds support for it into the 'ovs-ofctl packet-out' command, and adds a new ofctl/packet-out ovs-appctl command that can be used when ovs-ofctl is used as a flow monitor. The old 'ovs-ofctl packet-out syntax is deprecated' and will be removed in a later OVS release. The new packet-out parser is further supported with the ovs-ofctl bundle command, which allows bundles to mix flow mods, group mods and packet-out messages. Also the packet-outs in bundles are only executed if the whole bundle is successful. A failing packet-out translation may also make the whole bundle to fail. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Refactor packet_out handling.Jarno Rajahalme2016-09-141-43/+48
| | | | | | | | | | | | | | | | | | Refactor handle_packet_out() to prepare for bundle support for packet outs in a later patch. Two new callbacks are introduced in ofproto-provider class: ->packet_xlate() and ->packet_execute(). ->packet_xlate() translates the packet using the flow and actions provided by the caller, but defers all OpenFlow-visible side-effects (stats, learn actions, actual packet output, etc.) to be explicitly executed with the ->packet_execute() call. Adds a new ofproto_rule_reduce_timeouts__() that must be called with 'ofproto_mutex' held. This is used in the next patch. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Use ofproto_flow_mod for learn execution from xlate cache.Jarno Rajahalme2016-09-141-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | Use ofproto_flow_mod with a reference to an existing or new rule instead of ofputil_flow_mod for learn action execution from xlate cache Typically we would find that when a learn xlate cache entry is created, a preceding upcall has already created the learned flow. In this case the xlate cache entry takes a reference to that flow and keeps refreshing it without needing to perform any flow table lookups. Otherwise the creation of the xlate cache entry creates the new rule, which is then subsequently added to the classifier. In both cases this is both faster and shrinks the memory cost of each learn cache entry from ~3.5kb to about 0.3kb. If the learned rule does not yet exist, it is created and attached to the ofproto_flow_mod, from which it is then added. If the referred rule happens to expire, or is modified in any way and is thus removed from the classifier tables, we create a new rule using the old rule as a template, so that we can avoid storing the ofputil_flow_mod in all cases. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Change rule's 'removed' member to a tri-state 'state'.Jarno Rajahalme2016-09-131-2/+15
| | | | | | | | As a rule may not be re-inserted to ofproto data structures, it is cleaner to have three states for the rule, rather than just two. This will be useful for managing learned flows in later patches. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Don't use connmgr after destruction.Jarno Rajahalme2016-09-131-1/+1
| | | | | | | | | | | | | | Set ofproto's connmgr pointer to NULL after the connmgr has been destructed, and check for NULL when sending a flow removed notification. Verified by sending the flow removed message unconditionally and observing numerous core dumps in the test suite. Found by inspection. Fixes: f695ebfae5 ("ofproto: Postpone sending flow removed messages.") Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* lib: Retire packet buffering feature.Jarno Rajahalme2016-08-301-28/+0
| | | | | | | | | | | | | | | | | | | | | | | | | OVS implementation of buffering packets that are sent to the controller is not compliant with the OpenFlow specifications after OpenFlow 1.0, which is possibly true since OpenFlow 1.0 is not really specifying the packet buffering behavior. OVS implementation executes the buffered packet against the actions of the modified or added rule, whereas OpenFlow (since 1.1) specifies that the packet should be matched against the flow table 0 and processed accordingly. Rather than fix this behavior, and potentially break OVS users, the packet buffering feature is removed altogether. After all, such packet buffering is an optional OpenFlow feature, and as such any possible users should continue to work without this feature. This patch also makes OVS check the received 'buffer_id' values more rigorously, and fixes some internal users accordingly. Found by inspection. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Reduce bundle memory use.Jarno Rajahalme2016-08-151-2/+41
| | | | | | | | | | | | | | | Instead of storing the (big) struct ofputil_flow_mod, create the new rule and/or create the rule criteria for matching at bundle message insert time. This change reduces the size of a bundle flow mod from 3.5kb to 272 bytes, not counting the created rule, which was anyway created during bundle commit. In successful bundles this shifts work out of the ofproto_mutex critical section and should thus reduce the time the mutex is held during bundle commit. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Add 'command' to ofproto_flow_mod.Jarno Rajahalme2016-07-291-0/+1
| | | | | | | This helps releasing ofputil_flow_mod earlier in a later patch. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Add 'modify_cookie' to ofproto_flow_mod.Jarno Rajahalme2016-07-291-1/+2
| | | | | | | | | ofproto internally modifies 'modify_cookie' field, and adding a replica to ofproto_flow_mod allows the ofputil_flow_mod argument to be changed to a const. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Reduce dependency on ofputil_flow_mod after rule has been created.Jarno Rajahalme2016-07-291-0/+7
| | | | | | | | One step towards the goal of removing the ofputil_flow_mod from the bundle message. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Always forward 'used' from the old_rule.Jarno Rajahalme2016-07-291-3/+4
| | | | | | | | | | | | | | | | | Use new rule's flags to determine whether stats should be forwarded from the old, modified rule to the new rule. This captures the fact that prior to OpenFlow 1.2, which defines the reset counts flag, the reset counts semantics was assumed by default. However, in that case the reset counts flag is only present in the new flow, not on the corresponding flow mod. Having the above fixed revealed that the 'used' timestamp was not forwarded from the old rule to the new rule when counts were not being forwarded. Fix this by always forwarding the 'used' timestamp. Fixes: 39c9459355 ("Use classifier versioning.") Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>