summaryrefslogtreecommitdiff
path: root/ofproto
Commit message (Collapse)AuthorAgeFilesLines
* ofproto: Handle flow monitor requests with multiple parts.Ben Pfaff2019-01-151-43/+46
| | | | | Acked-by: Justin Pettit <jpettit@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ovs-vswitchd: Implement OFPT_TABLE_FEATURES table modification request.Ben Pfaff2019-01-153-25/+279
| | | | | | | | | 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: Handle multipart requests with multiple parts.Ben Pfaff2019-01-103-28/+50
| | | | | | | | | | | | | | | OpenFlow has a concept of multipart messages, that is, messages that can be broken into multiple pieces that are sent separately. Before OpenFlow 1.3, only replies could actually have multiple pieces. OpenFlow 1.3 introduced the idea that requests could have multiple pieces. This is only useful for multipart requests that take an array as part of the request, which amounts to only flow monitoring requests and table features requests. So far, OVS hasn't implemented the multipart versions of these (it just reports an error). This commit introduces the necessary infastructure to implement them properly. Acked-by: Justin Pettit <jpettit@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Return correct error codes from meter_set.Tony van der Peet2018-12-121-2/+2
| | | | | | | | | This routine should return enum ofperr, but in a couple of places doesn't. When adding one more meter when the meter table is full, this results in an incorrect error message. Signed-off-by: Tony van der Peet <tony.vanderpeet@alliedtelesis.co.nz> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofp-actions: Make all actions a multiple of OFPACT_ALIGNTO bytes.Ben Pfaff2018-11-192-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | The functions to put ofpacts into ofpbufs have always padded them to OFPACT_ALIGNTO boundaries, but the underlying structures weren't necessarily padded out. That led to difficulties in a few places where structures were allocated on the stack instead in an ofpbuf, because functions like ofpact_init_*() would access beyond the end of the actual structure. This is true, for example, in test_multipath_main() in tests/test-multipath.c, which allocates a struct ofpact_multipath on the stack, and in lswitch_handshake() in learning-switch.c, which allocates a struct ofpact_output on the stack. It's possible to fix these individual cases, but it's possible that there are others that haven't been identified. This commit addresses the issue another way, by padding all of the ofpact structures to a full multiple of OFPACT_ALIGNTO and adding assertions to ensure that it can't be screwed up in the future. This commit removes the OFPACT_*_SIZE enums, because they are now equivalent to sizeof(struct ofpact_*) in every case. Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* bond: Remove executable bit from bond.cTimothy Redaelli2018-11-101-0/+0
| | | | | | | | | | | | | In commit 90061ea7d1dd ("bond: Fix LACP fallback to active-backup when recirc is enabled.") the file mode of bond.c accidentaly changed from 0644 to 0755. This commit restores the previous file mode (0644) on bond.c. CC: Ben Pfaff <blp@ovn.org> Fixes: 90061ea7d1dd ("bond: Fix LACP fallback to active-backup when recirc is enabled.") Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto.c: Handle the situation when ofp_port number exhausted.Han Zhou2018-11-091-13/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | When ofp_port number is exhausted, OFPP_NONE (65535) will be returned by alloc_ofp_port(). In this case we should error out instead of continue using 65535 as port number. Using the invalid number causes unpredictable consequences: 2018-11-06T01:29:10.042Z|142103|dpif(ovs-vswitchd)|WARN|system@ovs-system: failed to add ovn-aded97-0 as port: Device or resource busy 2018-11-06T01:29:10.045Z|142104|bridge(ovs-vswitchd)|INFO|bridge br-int: added interface ovn-aded97-0 on port 65535 2018-11-06T01:29:11.479Z|142108|ofproto(ovs-vswitchd)|WARN|br-int: cannot configure bfd on nonexistent port 65535 2018-11-06T01:29:11.479Z|142109|ofproto(ovs-vswitchd)|WARN|br-int: cannot configure LLDP on nonexistent port 65535 2018-11-06T01:29:11.479Z|142110|ofproto(ovs-vswitchd)|WARN|br-int: cannot configure datapath on nonexistent port 65535 ... 2018-11-06T01:29:18.783Z|142117|bfd(ovs-vswitchd)|INFO|ovn-aded97-0: BFD state change: admin_down->down "No Diagnostic"->"No Diagnostic". 2018-11-06T01:29:18.785Z|00061|bfd(monitor82)|INFO|Interface ovn-aded97-0 remote mult value 0 changed to 3 2018-11-06T01:29:18.785Z|00062|bfd(monitor82)|INFO|ovn-aded97-0: New remote min_rx. ... 2018-11-06T01:29:18.773Z|142111|bridge(ovs-vswitchd)|INFO|bridge br-int: deleted interface ovn-aded97-0 on port 65535 ... 2018-11-06T01:29:18.779Z|142115|dpif(ovs-vswitchd)|WARN|system@ovs-system: failed to add ovn-aded97-0 as port: Device or resource busy 2018-11-06T01:29:18.782Z|142116|bridge(ovs-vswitchd)|INFO|bridge br-int: added interface ovn-aded97-0 on port 65535 ... 2018-11-06T01:29:18.785Z|00064|bfd(monitor82)|WARN|ovn-aded97-0: Incorrect your_disc. ... Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto.c: Fix port number leaking.Han Zhou2018-11-091-1/+1
| | | | | | | | | | | | When there is an error in ofport_install(), the ofp port number is not deallocated, which leads to port number leak. For example, when there is an redundant tunnel port added in an OVS bridge, ovs-vswitchd will try to add the port to ofproto whenever OVSDB changes, which would trigger the port number leak, and over the time there won't be any port available for valid requests. Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-upcall: Don't purge ukeys while in a quiescent state.Ben Pfaff2018-11-061-22/+16
| | | | | | | | | | | | | revalidator_purge() iterates and modifies umap->cmap. This should not happen in quiescent state, because cmap implementation based on rcu protected variables. Let's narrow the quiescent period to avoid possible wrong memory accesses. CC: Joe Stringer <joe@ovn.org> Fixes: 9fce0584a643 ("revalidator: Use 'cmap' for storing ukeys.") Reported-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* bond: Fix LACP fallback to active-backup when recirc is enabled.Ben Pfaff2018-11-051-3/+22
| | | | | | Reported-by: Arun Navasivasakthivelsamy <arunkum.navasiv@nutanix.com> Tested-by: Arun Navasivasakthivelsamy <arunkum.navasiv@nutanix.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm buildNuman Siddique2018-11-051-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When 'make check' is called by the mock rpm build (which disables networking), the test "ovn-nbctl: LBs - daemon" fails when it runs the command "ovn-nbctl lb-add lb0 30.0.0.1a 192.168.10.10:80,192.168.10.20:80". ovn-nbctl extracts the vip by calling the socket util function 'inet_parse_active()', and this function blocks when libunbound function ub_resolve() is called further down. ub_resolve() is a blocking function without timeout and all the ovs/ovn utilities use this function. As reported by Timothy Redaelli, the issue can also be reproduced by running the below commands $ sudo unshare -mn -- sh -c 'ip addr add dev lo 127.0.0.1 && \ mount --bind /dev/null /etc/resolv.conf && runuser $SUDO_USER' $ make sandbox SANDBOXFLAGS="--ovn" $ ovn-nbctl -vsocket_util:off lb-add lb0 30.0.0.1a \ 192.168.10.10:80,192.168.10.20:80 To address this issue, this patch adds a new bool argument 'resolve_host' to the function inet_parse_active() to resolve the host only if it is 'true'. ovn-nbctl/ovn-northd will pass 'false' when it calls this function to parse the load balancer values. Reported-by: Timothy Redaelli <tredaelli@redhat.com> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1641672 Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* connmgr: Improve interface for setting controllers.Ben Pfaff2018-10-314-42/+28
| | | | | | | | | | | Using an shash instead of an array simplifies the code for both the caller and the callee. Putting the set of allowed OpenFlow versions into the ofproto_controller data structure also simplifies the overall function interface slightly. Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* connmgr: Modernize coding style.Ben Pfaff2018-10-311-168/+112
| | | | | | | | | | This moves declarations closer to first use and merges them with initialization when possible, moves "for" loop variable declarations into the "for" statements where possible, and otherwise makes this code look like it was written a little more recently than it was. Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* dpif: Restore a few lines with form feed charactersSriharsha Basavapatna2018-10-311-1/+1
| | | | | | | | | | A few lines with form feed characters (ASCII: ^L) were accidentally deleted by a recent commit to support rebalancing of offloaded flows. This patch reverts those lines. Fixes: 57924fc91c ("revalidator: Rebalance offloaded flows") Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofp-msgs: Added ONF_ and NXT_REQUESTFORWARD for OF1.0-1.3Zak Whittington2018-10-261-1/+6
| | | | | | | | | | | | | | | | Backported OFPT14_REQUESTFORWARD to OF1.0-1.3. OF 1.0-1.2 use an NXT Nicira extension while OF 1.3 uses an ONF extension (ONF version is specified in a previously published ONF spec sheet). Includes ofp-print tests for multiple inner message types, and multiple OF versions including the NXT and ONF. Also includes more end-to-end ofproto tests for both NXT OF1.0 and also ONF OF1.3. VMware-BZ: 2136594 Signed-off-by: Zak Whittington <zwhitt.vmware@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* connmgr: Fix vswitchd abort when a port is added and the controller is downNuman Siddique2018-10-231-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We see the below trace when a port is added to a bridge and the configured controller is down 0x00007fb002f8b207 in raise () from /lib64/libc.so.6 0x00007fb002f8c8f8 in abort () from /lib64/libc.so.6 0x00007fb004953026 in ofputil_protocol_to_ofp_version () from /lib64/libopenvswitch-2.10.so.0 0x00007fb00494e38e in ofputil_encode_port_status () from /lib64/libopenvswitch-2.10.so.0 0x00007fb004ef1c5b in connmgr_send_port_status () from /lib64/libofproto-2.10.so.0 0x00007fb004efa9f4 in ofport_install () from /lib64/libofproto-2.10.so.0 0x00007fb004efbfb2 in update_port () from /lib64/libofproto-2.10.so.0 0x00007fb004efc7f9 in ofproto_port_add () from /lib64/libofproto-2.10.so.0 0x0000556d540a3f95 in bridge_add_ports__ () 0x0000556d540a5a47 in bridge_reconfigure () 0x0000556d540a9199 in bridge_run () 0x0000556d540a02a5 in main () The abort is because of ofputil_protocol_to_ofp_version() is called with invalid protocol - OFPUTIL_P_NONE. Please see [1] for more details. Similar aborts are seen as reported in [2]. The commit [3] changed the behavior of the function rconn_get_version(). Before the commit [3], the function ofconn_receives_async_msg() would always return false if the connection to the controller was down, since rconn_get_version() used to return -1. This patch now checks the rconn connection status in ofconn_receives_async_msg() and returns false if not connected. This would avoid the aborts seen in the above stack trace. The issue can be reproduced by running the test added in this patch without the fix. [1] - https://bugzilla.redhat.com/show_bug.cgi?id=1640045 [2] - https://bugzilla.redhat.com/show_bug.cgi?id=1637926 [3] - 476d2551ab ("rconn: Introduce new invariant to fix assertion failure in corner case.") Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Eelco Chaudron <echaudro@redhat.com>
* revalidator: Rebalance offloaded flows based on the pps rateSriharsha Basavapatna via dev2018-10-191-6/+440
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is the third patch in the patch-set to support dynamic rebalancing of offloaded flows. The dynamic rebalancing functionality is implemented in this patch. The ukeys that are not scheduled for deletion are obtained and passed as input to the rebalancing routine. The rebalancing is done in the context of revalidation leader thread, after all other revalidator threads are done with gathering rebalancing data for flows. For each netdev that is in OOR state, a list of flows - both offloaded and non-offloaded (pending) - is obtained using the ukeys. For each netdev that is in OOR state, the flows are grouped and sorted into offloaded and pending flows. The offloaded flows are sorted in descending order of pps-rate, while pending flows are sorted in ascending order of pps-rate. The rebalancing is done in two phases. In the first phase, we try to offload all pending flows and if that succeeds, the OOR state on the device is cleared. If some (or none) of the pending flows could not be offloaded, then we start replacing an offloaded flow that has a lower pps-rate than a pending flow, until there are no more pending flows with a higher rate than an offloaded flow. The flows that are replaced from the device are added into kernel datapath. A new OVS configuration parameter "offload-rebalance", is added to ovsdb. The default value of this is "false". To enable this feature, set the value of this parameter to "true", which provides packets-per-second rate based policy to dynamically offload and un-offload flows. Note: This option can be enabled only when 'hw-offload' policy is enabled. It also requires 'tc-policy' to be set to 'skip_sw'; otherwise, flow offload errors (specifically ENOSPC error this feature depends on) reported by an offloaded device are supressed by TC-Flower kernel module. Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Co-authored-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com> Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com> Reviewed-by: Sathya Perla <sathya.perla@broadcom.com> Reviewed-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Simon Horman <simon.horman@netronome.com>
* revalidator: Gather packets-per-second rate of flowsSriharsha Basavapatna via dev2018-10-191-0/+51
| | | | | | | | | | | | | | | | | | | This is the second patch in the patch-set to support dynamic rebalancing of offloaded flows. The packets-per-second (pps) rate for each flow is computed in the context of revalidator threads when the flow stats are retrieved. The pps-rate is computed only after a flow is revalidated and is not scheduled for deletion. The parameters used to compute pps and the pps itself are saved in udpif_key since they need to be persisted across iterations of rebalancing. Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> Co-authored-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com> Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com> Reviewed-by: Sathya Perla <sathya.perla@broadcom.com> Reviewed-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Simon Horman <simon.horman@netronome.com>
* ofproto: Consistently force off OFPPS_LIVE if port or link is down.Ben Pfaff2018-10-183-34/+36
| | | | | | | | 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-dpif: Refactor port_run().Ben Pfaff2018-10-181-30/+32
| | | | | | | | This makes port_run() easier to understand but should not change its behavior. 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-183-8/+8
| | | | | | | | | | | 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: Refactor update_port().Ben Pfaff2018-10-181-25/+9
| | | | | | | | | update_port() worked a little too hard to avoid copying and comparing some bits in the ofputil_phy_port. This seems like a simpler approach all around. It should behave the same way. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Numan Siddique <nusididq@redhat.com>
* connmgr: Suppress duplicate port status notifications.Ben Pfaff2018-10-183-19/+41
| | | | | | | | | | | | | When the status of a port changes, ofproto calls into connmgr to notify controllers. Sometimes, particular changes are only visible to controllers running specific versions of OpenFlow. Until now, OVS would send those controllers duplicate port status notifications. This is unnecessary and somewhat confusing. This commit eliminates it. This commit updates one of the tests not to expect duplicate notifications. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Numan Siddique <nusididq@redhat.com>
* bond: Honor updelay and downdelay when LACP is in use.Ben Pfaff2018-10-171-2/+1
| | | | | | | | | | | | Since OVS added LACP support back in 2011, bonds have ignored the updelay and downdelay values for bonds with configured LACP. The reason is not clear, but at least one user needs support in this case, so this commit enables it. Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-October/047490.html Reported-by: Daniel Leaberry <dleaberry@purestorage.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
* ofproto-dpif-xlate: Avoid deadlock on multicast snooping recursion.Ben Pfaff2018-10-151-18/+84
| | | | | | | | | | | | | | Until now, OVS did multicast snooping outputs holding the read-lock on the mcast_snooping object. This could recurse via a patch port to try to take the write-lock on the same object, which deadlocked. This patch fixes the problem, by releasing the read-lock before doing any outputs. It would probably be better to use RCU for mcast_snooping. That would be a bigger patch and less suitable for backporting. Reported-by: Sameh Elsharkawy Reported-at: https://github.com/openvswitch/ovs-issues/issues/153 Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-xlate.c: Fix uninitialized variable warning.Justin Pettit2018-09-271-1/+1
| | | | | | | | | | With gcc 7.3.0 a warning is given about two variables possibly being uninitialized in compose_sample_action(). The code path only allows the variables to be used if they've been initialized, so this warning is incorrect. However, this change allows a clean build. Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Flavio Leitner <fbl@sysclose.org>
* sflow: Set agent address properly based on collector address.Justin Pettit2018-09-271-2/+3
| | | | | | | | | | | | If an agent address is not provided, OVS tries to choose a source address based on the source IP that would be used to connect to the sFlow collector. The code previously set the agent address to the collector's address instead of using the calculated source address. This patch properly uses the source address. Reported-by: Neil McKee <neil.mckee@inmon.com> Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto: Fix build with some GCC versions.Ben Pfaff2018-09-271-7/+6
| | | | | | | | | | GCC 4.8.x and possibly other versions don't like a designated initializer for an anonymous struct, see e.g. https://travis-ci.org/openvswitch/ovs/jobs/433747674 Fixes: f836888d28ec ("ofproto: Handle OpenFlow version mismatch for requestforward with groups.") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
* ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUMEYi-Hung Wei2018-09-261-0/+1
| | | | | | | | | | | | | | | | | | | | | This patch addresses the issue that the conntrack fields associated with a packet are missing after a packet is resumed by NXT_RESUME. For example, the last rule in the following OpenFlow pipeline is not working without this patch. table=0, arp,in_port=1 action=2 table=0, arp,in_port=2 action=1 table=0, in_port=2 icmp action=output:1 table=0, in_port=1 icmp action=ct(table=1) table=1, icmp action=controller(pause) resubmit(,2) table=2, in_port=1 icmp ct_state=+trk+new action=output:2 A unit test is added to prevent regression. Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in "continuations".") VMware-BZ: #2202764 Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* dpif: Remove support for multiple queues per port.Ben Pfaff2018-09-262-8/+5
| | | | | | | | | | | | Commit 69c51582ff78 ("dpif-netlink: don't allocate per thread netlink sockets") removed dpif-netlink support for multiple queues per port. No remaining dpif provider supports multiple queues per port, so remove infrastructure for the feature. CC: Matteo Croce <mcroce@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* ofproto-dpif-xlate: Fix load balancing for select groups with MPLS.Anju Thomas2018-09-261-0/+1
| | | | | | | | | | | | | Before this commit, OVS did not do load balancing for select group buckets in case of mpls tagged packets. After an MPLS pop action, the expectation is to trigger recirculation. This recirculation will ensure an RSS re-computation which will ensure load balancing in case of select group bucket. Due to a missing return statement before bucket selection, the bucket selection in case of select group happens before the recirculation and hence no load balancing is achieved. Signed-off-by: Anju Thomas <anju.thomas@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Handle OpenFlow version mismatch for requestforward with groups.Ben Pfaff2018-09-261-4/+7
| | | | | | | | | | | | | | | OpenFlow 1.4+ supports a feature called requestforward. When a controller enables this feature, the switch sends that controller a copy of other controllers' group and meter modification requests. OpenFlow 1.5 supports some group features not in OpenFlow 1.4. When OVS attempted to forward such requests to an OpenFlow 1.4 controller, it reported an error and exited. This commit fixes the problem by making OVS properly translate the messages to OpenFlow 1.4 format. Reported-by: Pierre Cregut <pierre.cregut@orange.com> Tested-by: Pierre Cregut <pierre.cregut@orange.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-September/047453.html Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Fix NXT_RESUME flow statsYi-Hung Wei2018-09-213-7/+20
| | | | | | | | | | | Currently, OVS does not update the flow stats after a packet is restarted by NXT_RESUME message. This patch fixes the aforementioned issue and adds an unit test to prevent regression. Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in "continuations".") VMware-BZ: #2198435 Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-xlate: Fix translation of groups with no buckets.Ben Pfaff2018-09-171-1/+1
| | | | | | | | | | | | | A group can have no buckets, in which case ovs_list_back() assert-fails. This fixes the problem. Found by OFTest. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1626488 Tested-by: Eelco Chaudron <echaudro@redhat.com> Fixes: a04e58881e25 ("ofproto-dpif-xlate: Simplify translation for groups.") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Eelco Chaudron <echaudro@redhat.com>
* ofproto-dpif-trace: Make -generate send packets to controller again.Ben Pfaff2018-08-272-17/+82
| | | | | | | | | | | | | | | | | | | Prior to the OVS 2.9 development cycle, any flow that sent a packet to a controller required that the flow be slow-pathed. In some cases this led to poor performance, so OVS 2.9 made controller actions fast-pathable. As a side effect of the change, "ovs-appctl ofproto/trace -generate" no longer sent packets to the controller. This usually didn't matter but it broke the Faucet tutorial, which relied on this behavior. This commit reintroduces the original behavior and thus should fix the tutorial. CC: Justin Pettit <jpettit@ovn.org> Fixes: d39ec23de384 ("ofproto-dpif: Don't slow-path controller actions.") Reported-by: macman31 <https://github.com/macman31> Reported-at: https://github.com/openvswitch/ovs-issues/issues/145 Reported-by: Brad Cowie <brad@cowie.nz> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047234.html Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org>
* ofproto-dpif: Check for EBUSY as wellGreg Rose2018-08-271-1/+1
| | | | | | | | | | | | Guru reported that we can't create more than one geneve tunnel. Sometimes a driver will return EBUSY as well as EEXIST for some duplicate configurations. Check for EBUSY too. Fixes: 7521e0cf9e ("ofproto-dpif: Let the dpif report when a ...") Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047214.html Reported-by: Guru Shetty <guru@ovn.org> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* dpif: Don't pass in '*meter_id' to meter_set commands.Justin Pettit2018-08-161-1/+1
| | | | | | | | | | | | The original intent of the API appears to be that the underlying DPIF implementaion would choose a local meter id. However, neither of the existing datapath meter implementations (userspace or Linux) implemented that; they expected a valid meter id to be passed in, otherwise they returned an error. This commit follows the existing implementations and makes the API somewhat cleaner. Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-upcall: Fix for flow limit issue in revalidatorVishal Deep Ajmera2018-08-151-2/+2
| | | | | | | | | | | | | | | | | | | | When the revalidator thread takes a long time to dump data path flows (e.g. due to busy CPU), it reduces the maximum limit for new flows that can be added. This results in more upcalls for packets which do not find data path flows and temporarily reduces overall throughput. When the situation improves and the revalidator gets enough CPU cycles, it should increase the flow limit allowing more flows to get inserted. Currently the flow limit does not increase if the existing number of flows is less than 2000 and does not allow any new flows due to incorrect condition check. This results in a permanent drop in performance in OVS with no automatic recovery. This patch fixes the conditional check for increasing flow limit. Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Fix coredump in ofproto_destroy__().liucheng (J)2018-08-151-0/+2
| | | | | | | | | | | | | | | | | | | | | | | There is a coredump when I add and delete bridges. When the rcu thread call ofproto_destroy__, the main thread may call ofproto_create. But the ofproto_destroy__ fun doesn't have the ofproto_mutex when access the all_ofprotos. #0 0x00007f824aa0d197 in raise () from /usr/lib64/libc.so.6 #1 0x00007f824aa0e888 in abort () from /usr/lib64/libc.so.6 #2 0x0000000000658249 in PAT_abort () #3 0x000000000065538d in patchIllInsHandler () #4 <signal handler called> #5 0x0000000000478a5b in hmap_remove (node=0x3320150, hmap=0x95fc40 <all_ofprotos>) at include/openvswitch/hmap.h:287 #6 ofproto_destroy__ (ofproto=0x3320150) at ofproto/ofproto.c:1642 #7 0x0000000000535e46 in ovsrcu_call_postponed () at lib/ovs_rcu.c:323 #8 0x0000000000536014 in ovsrcu_postpone_thread (arg=<optimized out>) at lib/ovs_rcu.c:338 #9 0x0000000000538488 in ovsthread_wrapper (aux_=<optimized out>) at lib/ovs_thread.c:682 #10 0x00007f824c130dc5 in start_thread () from /usr/lib64/libpthread.so.0 #11 0x00007f824aacf7bd in clone () from /usr/lib64/libc.so.6 Signed-off-by: Cheng Liu <liucheng11@huawei.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-xlate: Improve log message.Ben Pfaff2018-08-091-2/+2
| | | | | | | | | Until now, the bridge name was at the end of the log message, after the flow, which made it easy to miss. This commit moves it before the flow where it is easier to spot. Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Flavio Leitner <fbl@sysclose.org>
* ofproto-dpif-xlate: use new info-level logging helper when sending out an ↵Zak Whittington2018-08-071-2/+30
| | | | | | | | | | | | | in_port Added new helper function similar to xlate_report_error called xlate_report_info that logs info-level messages, and used that function to add an extra log message when attempting to send out an in-port. VMware-BZ: 2158607 Signed-off-by: Zak Whittington <zwhitt.vmware@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Fix typo in registered commandAlin Gabriel Serdean2018-08-011-1/+2
| | | | | | | | | | Also split line at 79 characters. Found by inspection. Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Acked-by: Ben Pfaff <blp@ovn.org>
* ofp-actions: Split ofpacts_check__() into many functions.Ben Pfaff2018-07-312-14/+18
| | | | | | | | | | | ofpacts_check__() was a huge switch statement with special cases for many different kinds of actions. This made it unwieldy and put the special cases far away from the rest of the code related to a given action. This commit refactors the code to avoid the problem. Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Yifeng Sun <pkusunyifeng@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
* ofproto: Add support for specifying a meter in controller actions.Justin Pettit2018-07-303-5/+64
| | | | | Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-xlate: Check the right IPv6 address in is_nd_dst_correct().Ben Pfaff2018-07-111-1/+1
| | | | | | | | | Fixes test 815 "tunnel_push_pop_ipv6 - action". CC: Aaron Conole <aconole@redhat.com> Fixes: 6f231f7c3a9e ("xlate: use const struct in6_addr in linklocal check") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Aaron Conole <aconole@redhat.com>
* xlate: use const struct in6_addr in linklocal checkAaron Conole2018-07-101-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | Commit 83c2757bd16e ("xlate: Move tnl_neigh_snoop() to terminate_native_tunnel()") introduced a call to IN6_IS_ADDR_MC_LINKLOCAL() when checking neighbor discovery. The call to this assumes that the argument may be a const uint8_t *. According to The Open Group Base Specifications Issue 7, 2018: macro is of type int and takes a single argument of type const struct in6_addr * The GNU implementation allows a bit of flexibility, by internally casting the argument. However, other implementations (such as OS X) more rigidly implement the standard and fail with errors like: error: member reference base type 'const uint8_t' (aka 'const unsigned char') is not a structure or union Fixes: 83c2757bd16e ("xlate: Move tnl_neigh_snoop() to terminate_native_tunnel()") Cc: Zoltan Balogh <zoltan.balogh.eth@gmail.com> Cc: Jan Scheurich <jan.scheurich@ericsson.com> Signed-off-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Add CLI commands to show and clear mac_learning statisticsEelco Chaudron2018-07-061-0/+67
| | | | | | | | | | | | | | | | | | | Add two new commands, fdb/stats-show and fdb/stats-clear, to ovs-appctl to show and clear the new mac_learning statistics. $ ovs-appctl fdb/stats-show ovs_pvp_br0 Statistics for bridge "ovs_pvp_br0": Current/maximum MAC entries in the table: 4/2048 Total number of learned MAC entries : 4 Total number of expired MAC entries : 1 Total number of evicted MAC entries : 0 Total number of port moved MAC entries : 32 $ ovs-appctl fdb/stats-clear ovs_pvp_br0 statistics successfully cleared Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif-xlate: Fix packet_in reason for Table-miss ruleKeshav Gupta2018-07-061-36/+46
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently in OvS if we hit "Table-miss" rules (associated with Controller action) then we send PACKET_IN message to controller with reason as OFPR_NO_MATCH. “Table-miss” rule is one whose priority is 0 and its catch all rule. But if we hit same "Table-miss" rule after executing group entry we will send the reason as OFPR_ACTION (for OF1.3 and below) and OFPR_GROUP (for OF1.4 and above). This is because once we execute group entry we set ctx->in_group and later when we hit the "Table-miss" rule, Since ctx->in_group is set we send reason as OFPR_ACTION (for OF1.3) and OFPR_GROUP (for OF1.4 and above). For eg: for the following pipeline, we will send the reason as OFPR_ACTION even if we hit The “Table-miss” rule. cookie=0x8000000, duration=761.189s, table=0, n_packets=1401, n_bytes=67954, priority=4,in_port=9,vlan_tci=0x0000/0x1fff actions=write_metadata:0x67870000000000/0xffffff0000000001,goto_table:17 cookie=0x6800001, duration=768.848s, table=17, n_packets=1418, n_bytes=68776, priority=10,metadata=0x67870000000000/0xffffff0000000000 actions=write_metadata:0xe067870000000000/0xfffffffffffffffe,goto_table:60 cookie=0x6800000, duration=24944.312s, table=60, n_packets=58244, n_bytes=2519520, priority=0 actions=resubmit(,17) cookie=0x8040000, duration=785.733s, table=17, n_packets=1450, n_bytes=69724, priority=10,metadata=0xe067870000000000/0xffffff0000000000 actions=write_metadata:0x67871d4d000000/0xfffffffffffffffe,goto_table:43 cookie=0x822002d, duration=24960.795s, table=43, n_packets=53097, n_bytes=2230074, priority=100,arp,arp_op=1 actions=group:6000 group_id=6000,type=all,bucket=actions=CONTROLLER:65535, bucket=actions=resubmit(,48), bucket=actions=resubmit(,81) cookie=0x8500000, duration=24977.323s, table=48, n_packets=58309, n_bytes=2522634, priority=0 actions=resubmit(,49),resubmit(,50) cookie=0x8050000, duration=24984.679s, table=50, n_packets=6, n_bytes=264, priority=0 actions=CONTROLLER:65535 Currently we are sending table_id as 50 and packet_in reason as OFPR_ACTION. Instead of sending packet_in reason as OFPR_NO_MATCH. Signed-off-by: Keshav Gupta <keshav.gupta@ericsson.com> Co-authored-by: Rohith Basavaraja <rohith.basavaraja@gmail.com> Signed-off-by: Rohith Basavaraja <rohith.basavaraja@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto-dpif: Let the dpif report when a port is a duplicate.Ben Pfaff2018-07-051-4/+3
| | | | | | | | | | | | | | | The port_add() function checks whether the port about to be added to the dpif is already present and adds it only if it is not. This duplicates a check also present (and necessary) in each dpif and races with it as well. When a dpif has a large number of ports, the check can be expensive (it is not efficiently implemented). It would be nice to made the check cheaper, but it also seems reasonable to do as done in this patch and just let the dpif report the duplication. Reported-by: Haifeng Lin <haifeng.lin@huawei.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
* ofproto: Fix OVS crash when reverting old flows in bundle commitVishal Deep Ajmera2018-06-181-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | During bundle commit flows which are added in bundle are applied to ofproto in-order. In case if a flow cannot be added (e.g. flow action is go-to group id which does not exist), OVS tries to revert back all previous flows which were successfully applied from the same bundle. This is possible since OVS maintains list of old flows which were replaced by flows from the bundle. While reinserting old flows ovs asserts due to check on rule state != RULE_INITIALIZED. This will work only for new flows, but for old flow the rule state will be RULE_REMOVED. This is causing an assert and OVS crash. The ovs assert check should be modified to != RULE_INSERTED to prevent any existing rule being re-inserted and allow new rules and old rules (in case of revert) to get inserted. Here is an example to trigger the assert: $ ovs-vsctl add-br br-test -- set Bridge br-test datapath_type=netdev $ cat flows.txt flow add table=1,priority=0,in_port=2,actions=NORMAL flow add table=1,priority=0,in_port=3,actions=NORMAL $ ovs-ofctl dump-flows -OOpenflow13 br-test cookie=0x0, duration=2.465s, table=1, n_packets=0, n_bytes=0, priority=0,in_port=2 actions=NORMAL cookie=0x0, duration=2.465s, table=1, n_packets=0, n_bytes=0, priority=0,in_port=3 actions=NORMAL $ cat flow-modify.txt flow modify table=1,priority=0,in_port=2,actions=drop flow modify table=1,priority=0,in_port=3,actions=group:10 $ ovs-ofctl bundle br-test flow-modify.txt -OOpenflow13 First flow rule will be modified since it is a valid rule. However second rule is invalid since no group with id 10 exists. Bundle commit tries to revert (insert) the first rule to old flow which results in ovs_assert at ofproto_rule_insert__() since old rule->state = RULE_REMOVED. Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>