summaryrefslogtreecommitdiff
path: root/ofproto
Commit message (Collapse)AuthorAgeFilesLines
* ofproto-dpif-upcall: Fix using uninitialized fitness.Ilya Maximets2018-02-261-0/+1
| | | | | | | | | | | | | | | | | | | | 'upcall_xlate()' makes a decision to compose slow path actions by checking the 'upcall->fitness', which is not initialized in case of calling from the 'upcall_cb()'. 'upcall_cb()' receives the real flow, so the fitness should be initialized as perfect. Fixes following tests on travis: ofproto-dpif.at: ofproto-dpif megaflow - disabled - pmd ofproto-dpif.at: ofproto-dpif - conntrack - output action CC: Ben Pfaff <blp@ovn.org> Fixes: 687bafbb8a79 ("ofproto-dpif-upcall: Slow path flows that datapath can't fully match.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Make ofproto_port_open_type() faster.Ben Pfaff2018-02-263-22/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ofproto_port_open_type() was surprisingly slow because it called the function ofproto_class_find__(), which itself was surprisingly slow because it actually creates a set of strings and enumerates all of the available classes. This patch improves performance by eliminating the call to ofproto_class_find__() from ofproto_port_open_type(). In turn that required changing a parameter type and updating all the callers. Possibly it would be worth making ofproto_class_find__() itself faster, but it doesn't look like any of its other callers would be used in inner loops. For more background, see also https://mail.openvswitch.org/pipermail/ovs-discuss/2018-February/046140.html This patch arises as a result of testing done by Ali Ginwala and Han Zhou. Their test showed that commit 2d4beba resulted in slower performance of ovs-vswitchd than was seen in previous versions of OVS. With this patch, Ali retested and reported that performance drastically improved by ~60%. The test for 10k lports, 40 LSs and 8 LRs and 1k HVs just got completed in 3 hours 39 min vs 8+ hours for branch-2.9. Cpu utilization graph of a farm comparing Ben's ofproto patch vs branch-2.9 is available @ https://raw.githubusercontent.com/noah8713/ovn-scale-test/scale_results/results/ovs_2.9_vs_ben_ofproto.png Reported-by: Mark Michelson <mmichels@redhat.com> Acked-by: Mark Michelson <mmichels@redhat.com> Tested-by: aginwala <aginwala@asu.edu> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-ipfix: Fix an issue in flow key partBenli Ye2018-02-151-3/+21
| | | | | | | | | | As struct ipfix_data_record_flow_key_iface didn't calculate its length in flow key part, it may cause problem when flow key part length is not enough. Use MAX_IF_LEN and MAX_IF_DESCR to pre-allocate memory for ipfix_data_record_flow_key_iface. Signed-off-by: Daniel Benli Ye <daniely@vmware.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofp-util, ofp-parse: Break up into many separate modules.Ben Pfaff2018-02-1312-11/+17
| | | | | | | | | | | | 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-dpif-upcall: Slow path flows that datapath can't fully match.Ben Pfaff2018-02-061-3/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the OVS architecture, when a datapath doesn't have a match for a packet, it sends the packet and the flow that it extracted from it to userspace. Userspace then examines the packet and the flow and compares them. Commonly, the flow is the same as what userspace expects, given the packet, but there are two other possibilities: - The flow lacks one or more fields that userspace expects to be there, that is, the datapath doesn't understand or parse them but userspace does. This is, for example, what would happen if current OVS userspace, which understands and extracts TCP flags, were to be paired with an older OVS kernel module, which does not. Internally OVS uses the name ODP_FIT_TOO_LITTLE for this situation. - The flow includes fields that userspace does not know about, that is, the datapath understands and parses them but userspace does not. This is, for example, what would happen if an old OVS userspace that does not understand or extract TCP flags, were to be paired with a recent OVS kernel module that does. Internally, OVS uses the name ODP_FIT_TOO_MUCH for this situation. The latter is not a big deal and OVS doesn't have to do much to cope with it. The former is more of a problem. When the datapath can't match on all the fields that OVS supports, it means that OVS can't safely install a flow at all, other than one that directs packets to the slow path. Otherwise, if OVS did install a flow, it could match a packet that does not match the flow that OVS intended to match and could cause the wrong behavior. Somehow, this nuance was lost a long time. From about 2013 until today, it seems that OVS has ignored ODP_FIT_TOO_LITTLE. Instead, it happily installs a flow regardless of whether the datapath can actually fully match it. I imagine that this is rarely a problem because most of the time the datapath and userspace are well matched, but it is still an important problem to fix. This commit fixes it, by forcing flows into the slow path when the datapath cannot match specifically enough. CC: Ethan Jackson <ejj@eecs.berkeley.edu> Fixes: e79a6c833e0d ("ofproto: Handle flow installation and eviction in upcall.") Reported-by: Huanle Han <hanxueluo@gmail.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-January/343665.html Signed-off-by: Ben Pfaff <blp@ovn.org>
* Remove last mentions of 'facet' from comments.Ben Pfaff2018-02-061-6/+5
| | | | | | How did these survive so long?! OVS hasn't had facets since 2013. Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Delete system tunnel interface when remove ovs bridgejuyan@redhat.com2018-02-011-0/+5
| | | | | | | | | | | | | | | | | When a user adds the first tunnel of a given type (e.g. the first VXLAN tunnel) to an OVS bridge, OVS adds a vport of the same type to the kernel datapath that backs the bridge. There is the corresponding expectation that, when the last tunnel of that type is removed from the OVS bridges, OVS would remove the vport that represents it from the backing kernel datapath, but OVS was not doing that. This commit fixes the problem. There is not any major concern about the lingering tunnel interface, but it's cleaner to delete it. Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides used interface") Signed-off-by: JunhanYan <juyan@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* xlate: fix packets loopback caused by duplicate read of xcfgp.Huanle Han2018-02-011-29/+14
| | | | | | | | | | | | | Some functions, such as xlate_normal_mcast_send_mrouters, test xbundle pointers equality to avoid sending packet back to in bundle. However, xbundle pointers port from different xcfgp for same port are inequal. This may lead to the packet loopback. This commit stores xcfgp on ctx at first and always uses the same xcfgp during one packet process period. Signed-off-by: Huanle Han <hanxueluo@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* util: Document and rely on ovs_assert() always evaluating its argument.Ben Pfaff2018-02-011-4/+3
| | | | | | | | | | The ovs_assert() macro always evaluates its argument, even when NDEBUG is defined so that failure is ignored. This behavior wasn't documented, and thus a lot of code didn't rely on it. This commit documents the behavior and simplifies bits of code that heretofore didn't rely on it. Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* Support accepting and displaying table names in OVS tools.Ben Pfaff2018-02-012-4/+2
| | | | | | | | | | | | | | | | | | | | | | | | OpenFlow has little-known support for naming tables. Open vSwitch has supported table names for ages, but it has never used or displayed them outside of commands dedicated to table manipulation. This commit adds support for table names in ovs-ofctl. When a table has a name, it displays that name in flows and actions, so that, for example, the following: table=1, arp, actions=resubmit(,2) might become: table=ingress_acl, arp, actions=resubmit(,mac_learning) given appropriately named tables. For backward compatibility, only interactive ovs-ofctl commands by default display table names; to display them in scripts, use the new --names option. This feature was inspired by a talk that Kei Nohguchi presented at Open vSwitch 2017 Fall Conference. CC: Kei Nohguchi <kei@nohguchi.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* ofp-actions: Make formatting and parsing functions take a struct argument.Ben Pfaff2018-01-313-7/+16
| | | | | | | | | | An upcoming commit will add another parameter for parsing and formatting actions. It is much easier to add these parameters if they are encapsulated in a struct, so this commit first makes that change. Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Acked-by: Mark Michelson <mmichels@redhat.com>
* classifier: Refactor interface for classifier_remove().Ben Pfaff2018-01-311-10/+4
| | | | | | | | | | | | | | Until now, classifier_remove() returned either null or the classifier rule passed to it, which is an unusual interface. This commit changes it to return true if it succeeds or false on failure. In addition, most of classifier_remove()'s callers know ahead of time that it must succeed, even though most of them didn't bother with an assertion, so this commit adds a classifier_remove_assert() function as a helper. Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* flow: Add some L7 payload data to most L4 protocols that accept it.Ben Pfaff2018-01-273-2/+44
| | | | | | | | | | | | | | This makes traffic generated by flow_compose() look slightly more realistic. It requires lots of updates to tests, but at least the tests themselves should be slightly more realistic too. At the same time, add --l7 and --l7-len options to ofproto/trace to allow users to specify the amount or contents of payloads that they want. Suggested-by: Brad Cowie <brad@cowie.nz> Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* ofproto-dpif-trace: Generalize syntax for ofproto/trace.Ben Pfaff2018-01-262-117/+119
| | | | | | | | | | | | | | ofproto/trace takes a bunch of options that have weird placement and syntax. This commit changes the syntax so that the options can be placed anywhere and consistently use a double-dash option prefix. For compatibility, the previous syntax is also supported. An upcoming commit will add new options and this change allows that upcoming commit to be less confusing. Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* ofproto: Avoid use-after-free on error path in ofproto_flow_mod_learn().Ben Pfaff2018-01-261-6/+4
| | | | | | | | | | | | | In the case where the learned flow limit has been reached (below_limit == false), ofproto_flow_mod_uninit() would unref ofm->temp_rule (which is also in the 'rule' local variable) before dereferencing rule->flow_cookie for the log message. This fixes the problem. (The greatest likely consequence of this bug was logging the wrong cookie value.) Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: William Tu <u9012063@gmail.com>
* ofproto: Fix double-unref of temporary rule when learning.Ben Pfaff2018-01-262-10/+8
| | | | | | | | | | 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>
* Fix incorrect handling of return value.Lili Huang2018-01-251-4/+5
| | | | | | | The value cookie_offset should be 'size_t' type. Signed-off-by: Lili Huang <huanglili.huang@huawei.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-upcall: Fix typo in comment.Ben Pfaff2018-01-241-1/+1
| | | | | Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
* xlate: fix xport lookup for recircZoltan Balogh2018-01-234-1/+61
| | | | | | | | | | | | | | | | | | | | Xlate_lookup and xlate_lookup_ofproto_() provides in_port and ofproto based on xport determined using flow, which is extracted from packet. The lookup can happen due to recirculation as well. It can happen, that packet_type has been modified during xlate before recirculation is triggered, so the lookup fails or delivers wrong xport. This can be worked around by propagating xport to ctx->xin after the very first lookup and store it in frozen state of the recirculation. So, when lookup is performed due to recirculation, the xport can be retrieved from the frozen state. The packet-type-aware unit tests are updated with a new one to verify this behavior. Signed-off-by: Zoltan Balogh <zoltan.balogh@ericsson.com> CC: Jan Scheurich <jan.scheurich@ericsson.com> Fixes: beb75a40fdc2 ("userspace: Switching of L3 packets in L2 pipeline") Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-xlate: add uuid to xportsZoltan Balogh2018-01-231-0/+12
| | | | | | | | This should make possible to look up xport by UUID and will be used by a later commit. Signed-off-by: Zoltan Balogh <zoltan.balogh@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-sflow: Recursively examine actions inside clone.Zoltan Balogh2018-01-234-7/+20
| | | | | | | | | | | | | | | | | | | | | Until now, dpif_sflow_read_actions() has ignored actions inside clone. This means that sflow missed tnl_push actions inside clone, which OVS now uses to avoid tx recirculation. This commit fixes the problem by making dpif_sflow_read_actions() recursively process actions inside clone. In addition, some sflow data needs to be stored and restored in ofproto-dpif-xlate when native_tunnel_output() is invoked. Otherwise the output action of underlay bridge is getting counted too when sFlow is set on the overlay bridge. Both bugs are connected to sflows and were introduced by the commit in the "Fixes:" tag below. Signed-off-by: Zoltan Balogh <zoltan.balogh@ericsson.com> CC: Sugesh Chandran <sugesh.chandran@intel.com> Fixes: 7c12dfc527a5 ("tunneling: Avoid datapath-recirc by combining recirc actions at xlate.") Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Fix wrong datapath flow with same in_port and output port.Lilijun (Jerry)2018-01-221-2/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In my test, the new datapath flow which has the same in_port and actions output port was found using ovs-appctl dpctl/dump-flows. Then the mac address will move from one port to another and back it again in the physical switch. This problem result in the VM's traffic become abnormal. My test key steps: 1) There are three VM using ovs bridge and intel 82599 nics as uplink port, deployed in different hosts connecting to the same physical switch. They can be named using VM-A, VM-B and VM-C, Host-A, Host-B, Host-C. 2) VM-A send many unicast packets to VM-B, and VM-B also send unicast packets to VM-A. 3) VM-C ping VM-A continuously, and do ovs port add/delete testing in Host-C ovs bridge. 4) In some abormal scence, the physical switch clear all the mac-entry on each ports. Then Host-C ovs bridge's uplink port will receive two direction packets(VM-A to VM-B, and VM-B to VM-A). The expected result is that this two direction packets should be droppd in the uplink port. Because the dst port of this packets is the uplink port which is also the src port by looking ovs bridge's mac-entry table learned by ovs NORMAL rules. But the truth is some packets being sent back to uplink port and physical switch. And then VM-A's mac was moved to the physical switch port of Host-C from the port of Host-A, as a reulst, VM-C ping VM-A failed at this time. When this problem occurs, the abnormal ovs datapath's flow "in_port(2) actions:2" was found by executing the command "ovs-appctl dpctl/dump-flows". Currently, xlate_normal() uses xbundle pointer compare to verify the packet's dst port whether is same with its input port. This implemention may be wrong while calling xlate_txn_start/xlate_txn_commit in type_run() at the same time, because xcfg/xbridge/xbundle object was reallocated and copied just before we lookup the dst mac_port and mac_xbundle. Then mac_xbundle and in_xbundle are same related with the uplink port but not same object pointer. And we can fix this bug by adding ofbundle check conditions shown in my patch. Signed-off-by: Lilijun <jerry.lilijun@huawei.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* dpif: Add support for OVS_ACTION_ATTR_CT_CLEAREric Garver2018-01-205-2/+52
| | | | | | | | | | | | This supports using the ct_clear action in the kernel datapath. To preserve compatibility with current ct_clear behavior on old kernels, we only pass this action down to the datapath if a probe reveals the datapath actually supports it. Signed-off-by: Eric Garver <e@erig.me> Acked-by: William Tu <u9012063@gmail.com> Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Justin Pettit <jpettit@ovn.org>
* nsh: add dec_nsh_ttl actionYi Yang2018-01-111-0/+31
| | | | | | | | | | NSH ttl is a 6-bit field ranged from 0 to 63, it should be decremented by 1 every hop, if it is 0 or it is so after decremented, the packet should be dropped and a packet-in message is sent to main controller. Signed-off-by: Yi Yang <yi.y.yang@intel.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* nsh: add new flow key 'ttl'Yi Yang2018-01-111-3/+4
| | | | | | | | IETF NSH draft added a new filed ttl in NSH header, this patch is to add new nsh key 'ttl' for it. Signed-off-by: Yi Yang <yi.y.yang@intel.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Don't slow-path controller actions with pause.Justin Pettit2018-01-105-116/+81
| | | | | | | | | A previous patch removed slow-pathing for controller actions with the exception of ones that specified "pause". This commit removes that restriction so that no controller actions are slow-pathed. Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Don't slow-path controller actions.Justin Pettit2018-01-105-236/+176
| | | | | | | | | | | | | | | | | | | | Controller actions have become more commonly used for purposes other than just making forwarding decisions (e.g., packet logging). A packet that needs to be copied to the controller and forwarded would always be sent to ovs-vswitchd to be handled, which could negatively affect performance and cause heavier CPU utilization in ovs-vswitchd. This commit changes the behavior so that OpenFlow controller actions become userspace datapath actions while continuing to let packet forwarding and manipulation continue to be handled by the datapath directly. This patch still slow-paths controller actions with the "pause" flag set. A future patch will stop slow-pathing these pause actions as well. Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Add 'ofproto_uuid' and 'ofp_in_port' to user action cookie.Justin Pettit2018-01-102-18/+44
| | | | | | | | | | | Previously, the ofproto instance and OpenFlow port have been derived based on the datapath port number. This change explicitly declares them both, which will be helpful in future commits that no longer can depend on having a unique datapath port (e.g., a source port that represents the controller). Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofp-actions: Add action "debug_slow" for testing slow-path.Justin Pettit2018-01-101-0/+7
| | | | | | | | | | It isn't otherwise useful and in fact hurts performance so it's disabled without --enable-dummy. An upcoming commit will make use of this. Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Modify process_upcall() to remove some redundant code.Justin Pettit2018-01-101-28/+17
| | | | | Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Reorganize upcall handling.Justin Pettit2018-01-101-45/+36
| | | | | | | | | | - This reduces the number of times upcall cookies are processed. - It separate true miss calls from slow-path actions. The reorganization will also be useful for a future commit. Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Add ability to look up an ofproto by UUID.Justin Pettit2018-01-103-32/+76
| | | | | | | This will have callers in the future. Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Use a fixed size userspace cookie.Justin Pettit2018-01-106-68/+62
| | | | | | | This simplifies the cookie handling a bit. Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* netdev: Custom statistics.Michal Weglicki2018-01-101-0/+3
| | | | | | | | | | | | | | | | | | | | | - New get_custom_stats interface function is added to netdev. It allows particular netdev implementation to expose custom counters in dictionary format (counter name/counter value). - New statistics are retrieved using experimenter code and are printed as a result to ofctl dump-ports. - New counters are available for OpenFlow 1.4+. - New statistics are printed to output via ofctl only if those are present in reply message. - New statistics definition is added to include/openflow/intel-ext.h. - Custom statistics are implemented only for dpdk-physical port type. - DPDK-physical implementation uses xstats to collect statistics. Only dropped and error counters are exposed. Co-authored-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-ipfix: add interface Information Elements to flow keyMichal Weglicki2018-01-104-110/+277
| | | | | | | | | | | | | | | | | | | | | | | Extend flow key part of data record to include following Information Elements: - ingressInterface - ingressInterfaceType - egressInterface - egressInterfaceType - interfaceName - interfaceDescription In case of input sampling we don't have information about egress port. Define templates depending not only on protocol types, but also on flow direction. Only egress flow will include egress information elements. With this change, dpif_ipfix_exporter stores every port in hmap rather than only tunnel ports. It allows to easily retrieve required information about interfaces during sampling upcalls. Co-authored-by: Michal Weglicki <michalx.weglicki@intel.com> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com> Signed-off-by: Przemyslaw Szczerbik <przemyslawx.szczerbik@intel.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Issue clear error messages for unsupported CT features.Ben Pfaff2018-01-101-12/+15
| | | | | | | | | | I spent way too much time this last week tracking down errors due to a VM with an antique kernel module that didn't support connection tracking. This commit adds clear error messages that would have made the problem obvious. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Darrell Ball <dlu998@gmail.com>
* tunnel: fix tunnel flags set/clear.William Tu2018-01-101-0/+1
| | | | | | | | | | | | | | | | Existing code only set these tunnel flags (df, csum, and key) when the flag is set in the output tunnel port, but did not clear when the flag is unset. The patch fixes it by setting and clearing it accordingly. Two existing testcases need to fix: 'tunnel - Geneve option present' has no key set up, so we should match 'flags(df)' instead of 'flags(df|key)'. The second case 'tunnel - concomitant IPv6 and IPv4 tunnels' follows the same pattern. One additional test case 'tunnel - Mix Geneve/GRE options' is added. Signed-off-by: William Tu <u9012063@gmail.com> VMWare-BZ: #2019012 Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-xlate: Incorrect handling of errors in group action processingVishal Deep Ajmera2018-01-101-0/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As per OpenFlow v1.3 specification, when an action list contains a group action a copy of the packet is passed to the group for processing by the group. This means that if there is an error encountered during group processing, only the copy of packet should be dropped, but subsequent actions in the action list should be executed on the original packet. Additionally, if the group type is "ALL", each action bucket of the group should process a copy of the packet. If there is an error while processing one bucket other buckets should still be processed. Example 1: table=0,in_port=tap0 actions=output:tap1,group:10,output:tap2 Even if any error is encountered while processing the group action, the packet should still be forwarded to ports tap1 and tap2. Example 2: group_id=1,type=all,bucket=actions=output:tap1,bucket=actions=encap(eth) Even if processing the action in the second bucket fails because the packet already has an Ethernet header, the other copy of the packet should still be processed by the first bucket and output to port tap1. Currently the error handling in OVS does not comply with those rules. When any group bucket execution fails the error is recorded in the so-called "translation context" which is global for the processing of the original packet. Once an error is recorded, OVS skips processing subsequent buckets and installs a drop action in the datapath even if parts of the action list were previously processed successfully. This patch clears the error flag after any bucket of a group is executed. This way the error encountered in processing any bucket of the group will not impact other actions of action-list or other buckets of the group. Errors which are system limits to protect translation from taking too long time or too much space are not cleared. Instead drop action is installed for them. Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Signed-off-by: Keshav Gupta <keshav.gupta@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Remove variable length userdata probe.Justin Pettit2018-01-093-98/+0
| | | | | | | | | | | | Commit e995e3df57 ("Allow OVS_USERSPACE_ATTR_USERDATA to be variable length.") changed userspace action userdata from eight bytes to variable length. OVS only supports Linux kernels greater than or equal to 3.10, which include support for variable length userdata. Other datapaths are more modern and should have support, so it's no longer necessary to probe. Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofp-errors: Send as much of a message as possible in an error reply.Ben Pfaff2018-01-095-50/+18
| | | | | | | | | | | | | | | When an OpenFlow switch sends an error message in reply to a request from a controller, it includes an excerpt from the original request. The OpenFlow specification only requires the switch to send at least the first 64 bytes of the request. Until now, Open vSwitch has only sent that much. This commit changes Open vSwitch to instead send the entire message, only trimming it if necessary to keep the error message within the 64 kB limit for an OpenFlow message. This should simplify debugging for controller authors. CC: Suneel Bomminayuni <sbomminayuni@vmware.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
* nsh: rework NSH netlink keys and actionsYi Yang2018-01-083-16/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch changes OVS_KEY_ATTR_NSH to nested attribute and adds three new NSH sub attribute keys: OVS_NSH_KEY_ATTR_BASE: for length-fixed NSH base header OVS_NSH_KEY_ATTR_MD1: for length-fixed MD type 1 context OVS_NSH_KEY_ATTR_MD2: for length-variable MD type 2 metadata Its intention is to align to NSH kernel implementation. NSH match fields, set and PUSH_NSH action all use the below nested attribute format: OVS_KEY_ATTR_NSH begin OVS_NSH_KEY_ATTR_BASE OVS_NSH_KEY_ATTR_MD1 OVS_KEY_ATTR_NSH end or OVS_KEY_ATTR_NSH begin OVS_NSH_KEY_ATTR_BASE OVS_NSH_KEY_ATTR_MD2 OVS_KEY_ATTR_NSH end In addition, NSH encap and decap actions are renamed as push_nsh and pop_nsh to meet action naming convention. Signed-off-by: Yi Yang <yi.y.yang@intel.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Delete all groups and meters when (un)configuring a controller.Ben Pfaff2018-01-081-9/+22
| | | | | | | | | | | | | | Open vSwitch has always deleted all flows from the flow table whenever a controller is configured or whenever all the controllers are unconfigured. After this commit, OVS additionally deletes all OpenFlow groups and meters. Suggested-by: Periyasamy Palanisamy <periyasamy.palanisamy@ericsson.com> Suggested-by: Jan Scheurich <jan.scheurich@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Jan Scheurich <jan.scheurich@ericsson.com> Tested-by: Jan Scheurich <jan.scheurich@ericsson.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com>
* tunnel: Add ofproto/list-tunnels command for troubleshooting.Ben Pfaff2018-01-041-19/+55
| | | | | | | | I've recently had to debug some issues related to tunnel implementation. This command would make it easier to have some confidence in how tunnels are actually set up inside OVS. Signed-off-by: Ben Pfaff <blp@ovn.org>
* tunnel: Log sanely in tnl_port_receive().Ben Pfaff2018-01-041-13/+3
| | | | | | | | | | | | | | | When this function was introduced in 2012, it modified its 'flow' argument and logged the changes (at debug level). However, since 2013 it has no longer modified its 'flow' argument, but the logging was still oriented around the idea that it did. This commit fixes up the logging to make sense again. This doesn't fix an actual bug that causes problems, but it does fix a conceptual error. Fixes: 2301f7ebc15e ("tunnel: Make tnl_port_receive() parameter 'const'.") Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* tunnel: Avoid flow_to_string() call when rate-limited.Ben Pfaff2018-01-041-4/+5
| | | | | | | | | flow_to_string() is relatively expensive. It is better to avoid it if the string is not actually going to be used. Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Acked-by: Ansis Atteka <aatteka@ovn.org>
* sparse: Add guards to prevent FreeBSD-incompatible #include order.Ben Pfaff2017-12-226-2/+10
| | | | | | | | | | FreeBSD insists that <sys/types.h> be included before <netinet/in.h> and that <netinet/in.h> be included before <arpa/inet.h>. This adds guards to the "sparse" headers to yield a warning if this order is violated. This commit also adjusts the order of many #includes to suit this requirement. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
* ofproto-dpif: Fix a couple minor issues in comments.Justin Pettit2017-12-212-2/+2
| | | | | | Signed-off-by: Justin Pettit <jpettit@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Acked-by: Ben Pfaff <blp@ovn.org>
* xcache: Handle null argument for xlate_cache_uninit().Justin Pettit2017-12-211-0/+3
| | | | | | | | | Most other OVS libraries' delete and uninitialization functions allow a null argument, but this one would cause a segfault. Signed-off-by: Justin Pettit <jpettit@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Acked-by: Ben Pfaff <blp@ovn.org>
* bond: Fix bug that writes to freed memoryYifeng Sun2017-12-201-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pr_op->pr_rule is pointing to memory in bond->hash. It shouldn't be written if bond->hash is already freed. This bug is reported by running kernel path testsuite under valgrind: Invalid write of size 8 at 0x413D16: update_recirc_rules__ (bond.c:392) by 0x414CA0: bond_unref (bond.c:290) by 0x427E3C: bundle_destroy (ofproto-dpif.c:3002) by 0x429EF4: bundle_set (ofproto-dpif.c:3023) by 0x40858B: port_destroy (bridge.c:4087) by 0x40BD04: bridge_destroy (bridge.c:3266) by 0x410528: bridge_exit (bridge.c:506) by 0x4072EE: main (ovs-vswitchd.c:135) Address 0xb5a85f0 is 5,360 bytes inside a block of size 12,288 free'd at 0x4C2EDEB: free (/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x414C8D: bond_unref (bond.c:288) by 0x427E3C: bundle_destroy (ofproto-dpif.c:3002) by 0x429EF4: bundle_set (ofproto-dpif.c:3023) by 0x40858B: port_destroy (bridge.c:4087) by 0x40BD04: bridge_destroy (bridge.c:3266) by 0x410528: bridge_exit (bridge.c:506) by 0x4072EE: main (ovs-vswitchd.c:135) Block was alloc'd at at 0x4C2DB8F: malloc (/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x516C04: xmalloc (util.c:120) by 0x414FD1: bond_entry_reset (bond.c:1651) by 0x414FD1: bond_reconfigure (bond.c:470) by 0x41507D: bond_create (bond.c:245) by 0x429D5D: bundle_set (ofproto-dpif.c:3194) by 0x408AC8: port_configure (bridge.c:1052) by 0x40CD87: bridge_reconfigure (bridge.c:682) by 0x410775: bridge_run (bridge.c:2998) by 0x407244: main (ovs-vswitchd.c:119) Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
* ofproto: Keep inserting buckets into a group from changing group type.Ben Pfaff2017-12-111-0/+17
| | | | | | | | | | | The "insert buckets" and "delete buckets" operations on a group should not change the group's type or properties, but the implementation did this by mistake. This fixes the problem. Reported-by: shivani dommeti <shivani.dommeti@gmail.com> Tested-by: shivani dommeti <shivani.dommeti@gmail.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2017-December/045830.html Signed-off-by: Ben Pfaff <blp@ovn.org>
View Lorry Controller Status