summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* Prepare for 2.14.10.branch-2.14Ilya Maximets2023-04-063-1/+10
| | | | | Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* Set release date for 2.14.9.v2.14.9Ilya Maximets2023-04-062-2/+5
| | | | | Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* ofproto-dpif-xlate: Always mask ip proto field.Aaron Conole2023-04-066-10/+229
| | | | | | | | | | | | | | | | | | | | | | | | | The ofproto layer currently treats nw_proto field as overloaded to mean both that a proper nw layer exists, as well as the value contained in the header for the nw proto. However, this is incorrect behavior as relevant standards permit that any value, including '0' should be treated as a valid value. Because of this overload, when the ofproto layer builds action list for a packet with nw_proto of 0, it won't build the complete action list that we expect to be built for the packet. That will cause a bad behavior where all packets passing the datapath will fall into an incomplete action set. The fix here is to unwildcard nw_proto, allowing us to preserve setting actions for protocols which we know have support for the actions we program. This means that a traffic which contains nw_proto == 0 cannot cause connectivity breakage with other traffic on the link. Reported-by: David Marchand <dmarchand@redhat.com> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2134873 Acked-by: Ilya Maximets <i.maximets@ovn.org> Signed-off-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* netdev-windows: Add checking when creating netdev with system type on WindowsWilson Peng2023-03-061-0/+11
| | | | | | | | | | | | | | | | | | | | In the recent Antrea project testing, some port could not be created on Windows. When doing debug, our team found there is one case happening when multiple ports are waiting for be created with correct port number. Some system type port will be created netdev successfully and it will cause conflict as in the dpif side it will be internal type. So finally the port will be created failed and it could not be easily recovered. With the patch, on Windows the netdev creating will be blocked for system type when the ovs_tyep got on dpif is internal. More detailed case description is in the reported issue No.262 with link below. Reported-at:https://github.com/openvswitch/ovs-issues/issues/262 Signed-off-by: Wilson Peng <pweisong@vmware.com> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
* classifier: Fix missing masks on a final stage with ports trie.Ilya Maximets2023-02-282-5/+108
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Flow lookup doesn't include masks of the final stage in a resulting flow wildcards in case that stage had L4 ports match. Only the result of ports trie lookup is added to the mask. It might be sufficient in many cases, but it's not correct, because ports trie is not how we decided that the packet didn't match in this subtable. In fact, we used a full subtable mask in order to determine that, so all the subtable mask bits has to be added. Ports trie can still be used to adjust ports' mask, but it is not sufficient to determine that the packet didn't match. Assuming we have following 2 OpenFlow rules on the bridge: table=0, priority=10,tcp,tp_dst=80,tcp_flags=+psh actions=drop table=0, priority=0 actions=output(1) The first high priority rule supposed to drop all the TCP data traffic sent on port 80. The handshake, however, is allowed for forwarding. Both 'tcp_flags' and 'tp_dst' are on the final stage in the flow. Since the stage mask from that stage is not incorporated into the flow wildcards and only ports mask is getting updated, we have the following megaflow for the SYN packet that has no match on 'tcp_flags': $ ovs-appctl ofproto/trace br0 "in_port=br0,tcp,tp_dst=80,tcp_flags=syn" Megaflow: recirc_id=0,eth,tcp,in_port=LOCAL,nw_frag=no,tp_dst=80 Datapath actions: 1 If this flow is getting installed into datapath flow table, all the packets for port 80, regardless of TCP flags, will be forwarded. Incorporating all the looked at bits from the final stage into the stages map in order to get all the necessary wildcards. Ports mask has to be updated as a last step, because it doesn't cover the full 64-bit slot in the flowmap. With this change, in the example above, OVS is producing correct flow wildcards including match on TCP flags: Megaflow: recirc_id=0,eth,tcp,in_port=LOCAL,nw_frag=no,tp_dst=80,tcp_flags=-psh Datapath actions: 1 This way only -psh packets will be forwarded, as expected. This issue affects all other fields on stage 4, not only TCP flags. Tests included to cover tcp_flags, nd_target and ct_tp_src/dst. First two are frequently used, ct ones are sharing the same flowmap slot with L4 ports, so important to test. Before the pre-computation of stage masks, flow wildcards were updated during lookup, so there was no issue. The bits of the final stage was lost with introduction of 'stages_map'. Recent adjustment of segment boundaries exposed 'tcp_flags' to the issue. Reported-at: https://github.com/openvswitch/ovs-issues/issues/272 Fixes: ca44218515f0 ("classifier: Adjust segment boundary to execute prerequisite processing.") Fixes: fa2fdbf8d0c1 ("classifier: Pre-compute stage masks.") Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* cirrus: Update to use FreeBSD 12.4.Ilya Maximets2023-01-091-1/+1
| | | | | | | | 12.4 was released in December. That means that 12.3 will become unavailable in a near future. Updating. Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* dpif: Fix tunnel key set for IPv6 tunnels with SLOW_ACTION.Eelco Chaudron2023-01-062-1/+49
| | | | | | | | | | | | | | The dpif_execute_helper_cb() function is supposed to add the OVS_ACTION_ATTR_SET(OVS_KEY_ATTR_TUNNEL()) action to the list of actions when passing it down to the kernel. This function was only checking if the IPv4 destination address was set, not both. This patch fixes this, including a datapath testcase. Fixes: 076caa2fb077 ("ofproto: Meter translation.") Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* ci: Fix overriding OPTS provided from the yml.Ilya Maximets2023-01-031-1/+1
| | | | | | | | | | | For GCC builds we're overriding --disable-ssl or --enable-shared options set up in the GHA yml file. Fix that by adding to EXTRA_OPTS instead. Fixes: 2581b0ad1159 ("travis: Combine kernel builds.") Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* Prepare for 2.14.9.Ilya Maximets2022-12-203-1/+10
| | | | | Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* Set release date for 2.14.8.v2.14.8Ilya Maximets2022-12-202-2/+7
| | | | | Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* hash: Fix compilation error on Fedora 34 with GCC 11 and -O0.Guzowski Adrian2022-12-202-11/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | With newest version of GCC, OVS fails to compile with -O0 due to false-positive overread detection in hash-related functions. Those function declares "const uint32_t p[]" as argument and it throws off the compiler into thinking that it reads from memory region of size 0. To fix that behavior, a change in argument declaration needs to be made: instead of using "[]" notation for a pointer, simply use "*". The reported error in question: lib/conntrack.c: In function ‘conn_key_hash’: lib/conntrack.c:2154:12: error: ‘hash_words’ reading 4 bytes \ from a region of size 0 [-Werror=stringop-overread] 2154| return hash_words((uint32_t *) (&key->dst + 1), | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2155| (uint32_t *) (key + 1) - (uint32_t *) (&key->dst + 1), | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2156| hash); | ~~~~~ lib/conntrack.c:2154:12: note: referencing argument 1 \ of type ‘const uint32_t *’ {aka ‘const unsigned int *’} In file included from lib/packets.h:31, from lib/ct-dpif.h:21, from lib/conntrack.h:23, from lib/conntrack.c:26: lib/hash.h:294:1: note: in a call to function ‘hash_words’ 294 | hash_words(const uint32_t p[], size_t n_words, uint32_t basis) | ^~~~~~~~~~ Signed-off-by: Guzowski Adrian <adrian.guzowski@exatel.pl> Tested-by: Numan Siddique <numans@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* lldp: Fix bugs when parsing malformed AutoAttach.Qian Chen2022-12-202-0/+21
| | | | | | | | | | | | | | | | | | | | The OVS LLDP implementation includes support for AutoAttach standard, which the 'upstream' lldpd project does not include. As part of adding this support, the message parsing for these TLVs did not include proper length checks for the LLDP_TLV_AA_ELEMENT_SUBTYPE and the LLDP_TLV_AA_ISID_VLAN_ASGNS_SUBTYPE elements. The result is that a message without a proper boundary will cause an overread of memory, and lead to undefined results, including crashes or other unidentified behavior. The fix is to introduce proper bounds checking for these elements. Introduce a unit test to ensure that we have some proper rejection in this code base in the future. Fixes: be53a5c447c3 ("auto-attach: Initial support for Auto-Attach standard") Signed-off-by: Qian Chen <cq674350529@163.com> Co-authored-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* rculist: Use rculist_back_protected to access prev.Adrian Moreno2022-12-061-3/+5
| | | | | | | | | | | | | | | | | The .prev member of a rculist should not be used directly by users because it's not rcu-safe. A convenient fake mutex (rculist_fake_mutex) helps ensuring that in conjunction with clang's thread safety extensions. Only writers with exclusive access to the rculist should access .prev via some of the provided *_protected() accessors. Use rculist_back_protected() in REVERSE_PROTECTED iterators to avoid clang's compilation warning. Acked-by: Mike Pattrick <mkp@redhat.com> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* rculist: Fix iteration macros.Ilya Maximets2022-11-241-9/+9
| | | | | | | | | | | | | | | | Some macros for rculist have no users and there are no unit tests specific to that library as well, so broken code wasn't spotted while updating to multi-variable iterators. Fixing multiple problems like missing commas, parenthesis, incorrect variable and macro names. Fixes: d293965d7b06 ("rculist: use multi-variable helpers for loop macros.") Reported-by: Subrata Nath <subrata.nath@nokia.com> Co-authored-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Dumitru Ceara <dceara@redhat.com> Acked-by: Alin-Gabriel Serdean <aserdean@ovn.org> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* learn: Fix parsing immediate value for a field match.Ilya Maximets2022-11-242-13/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The value is right-justified after the string parsing with parse_int_string(), i.e. it is in BE byte order and aligned to the right side of the array. For example, the 0x10011 value in a 4-byte field will look like 0x00 0x01 0x00 0x11. However, value copy to the resulted ofpact is performed from the start of the memory. So, in case the destination size is smaller than the original field size, incorrect part of the value will be copied. In the 0x00 0x01 0x00 0x11 example above, if the copy is performed to a 3-byte field, the first 3 bytes will be copied, which are 0x00 0x01 0x00 instead of 0x01 0x00 0x11. This leads to a problem where NXM_NX_REG3[0..16]=0x10011 turns into NXM_NX_REG3[0..16]=0x100 after the parsing. Fix that by offsetting the starting position to the size difference in bytes similarly to how it is done in learn_parse_load_immediate(). While at it, changing &imm to imm.b in function calls that expect byte arrays as an argument. The old way is technically correct, but more error prone. The mf_write_subfield_value() call was also incorrect. However, the 'match' variable is actually not used for anything since checking removal in commit: dd43a558597b ("Do not perform validation in learn_parse();") So, just removing the call and the 'match' variable entirely instead of fixing it. Fixes: 21b2fa617126 ("ofp-parse: Allow match field names in actions and brackets in matches.") Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-November/052100.html Reported-by: Thomas Lee <newsforthomas@engineer.com> Reviewed-by: Simon Horman <simon.horman@corigine.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* datapath-windows: Check the condition to reset pseudo header checksum on Rx sideWilson Peng2022-11-241-4/+23
| | | | | | | | | | | | | | | If ovs node running on Windows is processing NAT action on the RX side, it will reset pseudo header checksum only if the L4 checksum is same as the calculated pseudo header checksum before NAT action. Without the fix, if the L4 header checksum is filled with a pseudo header checksum (sourceip, dstip, protocol, tcppayloadlen+tcpheaderlen) OVS will still do the checksum update(replace some IP and port and recalculate the checksum). It will lead to incorrect L4 header checksum. Reported-at:https://github.com/openvswitch/ovs-issues/issues/265 Signed-off-by: Wilson Peng <pweisong@vmware.com> Signed-off-by: Alin-Gabriel Serdean <aserdean@ovn.org>
* netdev-linux: Fix inability to apply QoS on ports with custom qdiscs.Ilya Maximets2022-11-022-4/+45
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | tc_del_qdisc() function only removes qdiscs with handle '1:0'. If for some reason the interface has a qdisc with non-zero handle attached, tc_del_qdisc() will not delete it and subsequent tc_install() will fail to install a new qdisc. The problem is that Libvirt by default is setting noqueue qdisc for all tap interfaces it creates. This is done for performance reasons to ensure lockless xmit. The issue is causing non-working QoS in OpenStack setups since new versions of Libvirt started to use OVS to configure it. In the past, Libvirt configured TC on its own, bypassing OVS. Removing the handle value from the deletion request, so any qdisc can be removed. Changing the error checking to also pass ENOENT, since that is the error reported if only default qdisc is present. Alternative solution might be to use NLM_F_REPLACE, but that will be a larger change with a potential need of refactoring. Potential side effect of the change is that OVS may start removing qdiscs that it didn't remove before. Though it's not a new issue and 'linux-noop' QoS type should be used for ports that OVS should not touch. Otherwise, OVS owns qdiscs on all interfaces attached to it. While at it, adding more logs as errors are not logged in any way at the moment making the issue hard to debug. Reported-at: https://bugzilla.redhat.com/2138339 Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-October/052088.html Reported-at: https://github.com/openvswitch/ovs-issues/issues/268 Suggested-by: Slawek Kaplonski <skaplons@redhat.com> Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* vswitch.xml: Fix the name of rstp-path-cost option.Ilya Maximets2022-11-021-1/+1
| | | | | | | | | For some reason it is documented as 'rstp-port-path-cost', while the code and some other bits of documentation use 'rstp-path-cost'. Fixes: 9efd308e957c ("Rapid Spanning Tree Protocol (IEEE 802.1D).") Reviewed-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* bond: Fix crash while logging not yet enabled member.yangchang2022-11-021-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The log should be printed with the member name, not the active member name, and the active member does not judge whether it is NULL. If null, OVS will crash with the following backtrace: (gdb) bt 0 bond_check_admissibility (ofproto/bond.c:877) 1 is_admissible (ofproto/ofproto-dpif-xlate.c:2574) 2 xlate_normal (ofproto/ofproto-dpif-xlate.c:3027) 3 xlate_output_action (ofproto/ofproto-dpif-xlate.c:5284) 4 do_xlate_actions (ofproto/ofproto-dpif-xlate.c:6960) 5 xlate_actions (ofproto/ofproto-dpif-xlate.c:7924) 6 upcall_xlate (ofproto/ofproto-dpif-upcall.c:1237) 7 process_upcall (ofproto/ofproto-dpif-upcall.c:1456) 8 upcall_cb (ofproto/ofproto-dpif-upcall.c:1358) 9 dp_netdev_upcall (lib/dpif-netdev.c:7793) 10 handle_packet_upcall (lib/dpif-netdev.c:8255) 11 fast_path_processing (lib/dpif-netdev.c:8374) 12 dp_netdev_input__ (lib/dpif-netdev.c:8463) 13 dp_netdev_input (lib/dpif-netdev.c:8501) 14 dp_netdev_process_rxq_port (lib/dpif-netdev.c:5337) 15 pmd_thread_main (lib/dpif-netdev.c:6944) 16 ovsthread_wrapper (lib/ovs-thread.c:422) 17 ?? (/lib64/libpthread.so.0) 18 clone (/lib64/libc.so.6) Fixes: 423416f58749 ("lacp: report desync in ovs threads enabling slave") Signed-off-by: yangchang <yangchang@chinatelecom.cn> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* unaligned: Correct the stats of packet_count and byte_count on Windows.Wilson Peng2022-10-251-6/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The stats(byte_count) is got via function call ofputil_decode_flow_stats_reply() and for OpenFlow15 it will also call oxs_pull_entry__(). Currently we found on Windows the byte_count counter is incorrect. It will get the byte_count on OpenFlow15 handling via ntohll(get_unaligned_be64(payload)) Quote the comments below from Ilya Maximets (thanks for the given soluton and explanation): static inline uint64_t get_unaligned_u64__(const uint64_t *p_) ... return ntohll(((uint64_t) p[0] << 56) | ((uint64_t) p[1] << 48) | ((uint64_t) p[2] << 40) | ((uint64_t) p[3] << 32) | (p[4] << 24) | (p[5] << 16) | (p[6] << 8) | p[7]); And indeed the expression above has an issue with data types. The problem is the (p[4] << 24) part. The p[4] itself has a type 'uint8_t' which is unsigned 8bit value. It is not enough to hold the result of a left shift, so compiler automatically promotes it to the 'int' by default. But it is *signed* 32bit value. In your original report p[4] was equal to 0x81. After the left shift it became 0x81000000. Looks correct, but the type is 'int'. The next operation that we do is '|' with the previous shifted bytes that were explicitly converted to uint64_t before the left shift. So we have uint64_t | int. In this case compiler needs to extend the 'int' to 'unit64_t' before performing the operation. And since the 'int' is signed and the sign bit happens to be set in the 0x81000000, the sign extension is performed in order to preserve the value. The result is 0xffffffff81000000. And that is breaking everything else. From the new test below, it is incorrect for the n_bytes counter via OpenFlow15 on CMD: ovs-ofctl dump-flows. With the patch, get_unaligned_u64__() will return correct value to caller on Windows. In the output (Got via original CMD without fix) below n_bytes 2177130813 will be incorrectly changed to 18446744071591715133 when processing OpenFlow15 which is equal to 0xFFFFFFFF81C4613D and here the p[4] on Windows is 0x81. With the fix, new compiled ovs-ofctl1025.exe could dump the correct n_bytes counter Via OpenFlow15. ovs-ofctl.exe -O OpenFlow15 dump-flows nsx-managed | findstr 1516011 cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=18446744071591715133, cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=18446744071591715133, ovs-ofctl.exe -O OpenFlow10 dump-flows nsx-managed | findstr 1516011 cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813, cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813, ovs-ofctl.exe dump-flows nsx-managed | findstr 1516011 cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813, cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813, With the fix, new compiled ovs-ofctl1025.exe could dump the correct n_bytes counter Via OpenFlow15. ovs-ofctl1025.exe -O OpenFlow15 dump-flows nsx-managed | findstr 1516011 cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813, cookie=<>, duration=<>s, table=4, n_packets=1516011, n_bytes=2177130813, Fixes: afa3a93165f1 ("Add header for access to potentially unaligned data.") Signed-off-by: Wilson Peng <pweisong@vmware.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* tests: Fix filtering of whole-second durations.Ilya Maximets2022-10-252-6/+6
| | | | | | | | | | | | | | | | | | | | Current macros are unable to filter whole seconds, e.g. 'duration:6s'. This is causing random test failures, most frequently in CirrusCI: ./dpif-netdev.at:370: ovs-ofctl -O OpenFlow13 meter-stats br0 | strip_timers --- - +++ /tmp/cirrus-ci-build/tests/testsuite.dir/at-groups/990/stdout @@ -1,5 +1,5 @@ OFPST_METER reply (OF1.3) (xid=0x2): -meter:1 flow_count:1 packet_in_count:10 byte_in_count:600 duration:0.0s bands: +meter:1 flow_count:1 packet_in_count:10 byte_in_count:600 duration:6s bands: Fix sed matches to correctly handle that scenario. Repeating the [0-9\.] twice because it is hard to write a shorter portable version with sed. Acked-by: Mike Pattrick <mkp@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* github: Update versions of action dependencies.Ilya Maximets2022-10-121-6/+6
| | | | | | | | | | | | | | | | checkout@v2, cache@v2 and setup-python@v2 are using outdated Node.js 12 which is now deprecated in GHA [1], so these actions will stop working soon. Updating to most recent major versions with Node.js 16. This stops GHA from throwing warnings in every build. While at it, also updating upload-artifacts to more recent version. [1] https://github.blog/changelog/2022-09-22-github-actions-all-actions-will-begin-running-on-node16-instead-of-node12/ Acked-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* Prepare for 2.14.8.Ilya Maximets2022-10-073-1/+10
| | | | | Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* Set release date for 2.14.7.v2.14.7Ilya Maximets2022-10-072-2/+3
| | | | | Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* daemon-unix: Fix file descriptor leak when monitor restarts child.Fengqi Li2022-10-061-0/+2
| | | | | | | | | | | | | | | | When segmentation fault occurred in ovn-northd, monitor will try to restart the ovn-northd daemon process every 10s. Assume the following scenarios: There is a segmentation fault and the ovn-northd daemon process does not restart properly every time. New fds are created each time the ovn-northd daemon process is restarted by the monitor process, but old fds(fd[0]) owned by the monitor process was not closed properly. One pipe leak for each restart of the ovn-northd daemon process. After a long time file descriptors were exhausted. Fixes: e2ed6fbeb18c ("fatal-signal: Catch SIGSEGV and print backtrace.") Signed-off-by: Fengqi Li <lifengqi@inspur.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* vconn: Allow ECONNREFUSED in refuse connection test.Mike Pattrick2022-10-061-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | The "tcp vconn - refuse connection" test may fail due to a Connection Refused error. The network stack returns ECONNREFUSED on a reset connection in SYN_SENT state and EPIPE or ECONNRESET in all other cases. 2022-09-19T17:45:48Z|00001|socket_util|INFO|0:127.0.0.1: listening on port 34189 2022-09-19T17:45:48Z|00002|poll_loop|DBG|wakeup due to [POLLOUT][ POLLERR][POLLHUP] on fd 4 (127.0.0.1:47140<->) at ../lib/stream-fd. c:153 test-vconn: unexpected vconn_connect() return value 111 (Connection refused) ../../tests/vconn.at:21: exit code was 1, expected 0 530. vconn.at:21: 530. tcp vconn - refuse connection (vconn.at:21): FAILED (vconn.at:21) This was observed from a CI system, and isn't a common case. Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Mike Pattrick <mkp@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* dpdk: Use DPDK 19.11.13 release.Michael Phelan2022-10-034-7/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Update OVS CLI and relevant documentation to use DPDK 19.11.13. DPDK 19.11.13 contains fixes for the CVEs listed below: CVE-2022-28199 [1] CVE-2022-2132 [2] A bug was introduced in DPDK 19.11.12 by the commit 1e68fe334ff0 ("vhost: fix unsafe vring addresses modifications"). This bug can cause a deadlock when vIOMMU is enabled and NUMA reallocation of the virtqueues happen. A fix [3] has been posted and is due to be included in the DPDK 19.11.14 release. If a user wishes to avoid the issue then it is recommended to use DPDK 19.11.11 until the release of DPDK 19.11.14. It should be noted that DPDK 19.11.11 does not benefit from the numerous bug and CVE fixes addressed since its release. If a user wishes to benefit from these fixes it is recommended to use DPDK 19.11.13. [1] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-28199 [2] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-2132 [3] https://patches.dpdk.org/project/dpdk/patch/20220725203206.427083-2-david.marchand@redhat.com/ Signed-off-by: Michael Phelan <michael.phelan@intel.com> Acked-by: Kevin Traynor <ktraynor@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
* python: idl: Fix idl.Row.__str__ method.Christopher Aubut2022-09-191-1/+2
| | | | | | | | | | | | | Fixes idl.Row's __str__ method to only print if the column exists on the object. The Row object passed to the 'updates' argument of Idl.notify only contains a subset of columns. Printing that argument causes an AttributeError. Fixes: 6a1c98461b46 ("Add a __str__ method to idl.Row") Submitted-at: https://github.com/openvswitch/ovs/pull/392 Acked-by: Terry Wilson <twilson@redhat.com> Signed-off-by: Christopher Aubut <christopher@aubut.me> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* bond: Avoid deadlock while updating post recirculation rules.Ilya Maximets2022-09-164-4/+128
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the PACKET_OUT from controller ends up with sending packet to a bond interface, the main thread will take locks in the following order: handle_openflow --> take ofproto_mutex handle_packet_out packet_xlate output_normal bond_update_post_recirc_rules --> take rwlock in bond.c If at the same time revalidator thread is processing other packet with the output to the same bond: xlate_actions output_normal bond_update_post_recirc_rules --> take rwlock in bond.c update_recirc_rules ofproto_dpif_add_internal_flow ofproto_flow_mod --> take ofproto_mutex So, it is possible for these 2 threads to lock each other by taking one lock and waiting for another thread to release the second lock. It is also possible for the main thread to lock itself up by trying to acquire ofproto_mutex for the second time, if it will actually proceed with update_recirc_rules() after taking the bond rwlock. The problem appears to be that bond_update_post_recirc_rules() is called during the flow translation even if side effects are prohibited, which is the case for openflow PACKET_OUT handling. Skipping actual flow updates during the flow translation if side effects are disabled to avoid the deadlock. Since flows are not installed now when actions translated for very first packet, installing initial flows in bond_reconfigure(). This will cover the case of allocating a new recirc_id. Also checking if we need to update flows in bond_run() to cover link state changes. Regression test is added to catch the double lock case. Reported-at: https://github.com/openvswitch/ovs-issues/issues/259 Reported-by: Daniel Ding <zhihui.ding@easystack.cn> Fixes: adcf00ba35a0 ("ofproto/bond: Implement bond megaflow using recirculation") Acked-by: Mike Pattrick <mkp@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* ofproto-dpif-upcall: Add debug commands to pause/resume revalidators.Ilya Maximets2022-09-161-0/+33
| | | | | | | | | | | New commands 'revalidator/pause' and 'revalidator/resume'. Not documented, since these should not be used in production environments. Will be used for unit tests in the next commit. Acked-by: Mike Pattrick <mkp@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* test-list: Fix false-positive build failure with GCC 12.Ilya Maximets2022-09-161-0/+2
| | | | | | | | | | | | | | | | | | | GCC 12.2.1 on Fedora 36 generates the following false-positive warning that is treated as error with -Werror: tests/test-list.c: In function 'test_list_construction': tests/test-list.c:110:9: error: 'values' may be used uninitialized 110 | check_list(&list, values, n); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ For some reason it fails to recognize that array will not be used if 'n' equals zero. Fix that by just initializing arrays in full before using, since it's just a test code. Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* cirrus: Upgrade to FreeBSD 13.1 image.Ilya Maximets2022-09-121-1/+1
| | | | | | | | | | 13.1 got released in May and now we have problems updating some packages in 13.0 and CI is failing. Update to 13.1 to unblock the CI. Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* raft: Fix unnecessary periodic compactions.Ilya Maximets2022-08-301-1/+1
| | | | | | | | | | | | | | | | | | | While creating a new database file and storing a new snapshot into it, raft module by mistake updates the base offset for the old file. So, the base offset of a new file remains zero. Then the old file is getting replaced with the new one, copying new offsets as well. In the end, after a full compaction, base offset is always zero. And any offset is twice as large as zero. That triggers a new compaction again at the earliest scheduled time. In practice this issue triggers compaction every 10-20 minutes regardless of the database load, after the first one is triggered by the actual file growth or by the 24h maximum limit. Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for clustered databases.") Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-August/051977.html Reported-by: Oleksandr Mykhalskyi <oleksandr.mykhalskyi@netcracker.com> Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* github: Move CI to ubuntu 20.04 base image.Ilya Maximets2022-08-123-6/+5
| | | | | | | | | | | | | | | | | | 18.04 image is deprecated and will disappear soon. Also some slowdowns and brownouts are planned to push users away from this deprecated version: https://github.com/actions/virtual-environments/issues/6002 Moving to 20.04. Can't move to 22.04 at the moment because of deprecation warnings from openssl 3.0. Added missing dh-python dependency for debian. And disabled cast-align warnings also for GCC, since newer versions are complaining about DPDK headers in the same way as Clang does. Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* github: Remove kernels that fail to build with newer compilers.Ilya Maximets2022-08-121-7/+2
| | | | | | | | | | Kernels 5.0, 4.20, 4.18, 4.17, 4.16 are not longterm kernels and didn't receive updates for a long time. They are failing build with compilers provided with ubuntu 20.04. Removing them from the test matrix since we need to move CI from deprecated 18.04 image. Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* netdev-offload-tc: Disable offload of IPv6 fragments.Ilya Maximets2022-08-081-1/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | OVS kernel datapath and TC are parsing IPv6 fragments differently. For IPv6 later fragments, according to the original design [1], OVS always sets nw_proto=44 (IPPROTO_FRAGMENT), regardless of the type of the L4 protocol. This leads to situation where flow for nw_proto=44 gets installed to TC, but packets can not match on it, causing all IPv6 later fragments to go to userspace significantly degrading performance. Disabling offload for such packets, so the flow can be installed to the OVS kernel datapath instead. Disabling for all IPv6 fragments including the first one, because it doesn't make a lot of sense to handle them separately. It may also cause potential problems with conntrack trying to re-assemble a packet from fragments handled by different datapaths (first in HW, later in OVS kernel). Checking both 'nw_proto' and 'nw_frag' as classifier might decide to match only on one of them and also nw_proto will not be 44 for the first fragment. The issue was hidden for some time due to incorrect behavior of the OVS kernel datapath that was recently fixed in kernel commit: 12378a5a75e3 ("net: openvswitch: fix parsing of nw_proto for IPv6 fragments") To allow offloading in the future either flow dissector in TC should be changed to parse packets in the same way as OVS does, or parsing in OVS kernel and userspace should be made configurable, so users can opt-in to the behavior change. Silent change of the behavior (change by default) is not an option, because existing OpenFlow pipelines may depend on a certain behavior. [1] https://docs.openvswitch.org/en/latest/topics/design/#fragments Fixes: 83e866067ea6 ("netdev-tc-offloads: Add support for IP fragmentation") Reviewed-by: Roi Dayan <roid@nvidia.com> Reviewed-by: Marcelo Ricardo Leitner <mleitner@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* system-traffic: Fix IPv4 fragmentation test sequence for check-kernel.Paolo Valerio2022-08-081-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The following test sequence: conntrack - IPv4 fragmentation incomplete reassembled packet conntrack - IPv4 fragmentation with fragments specified leads to a systematic failure of the latter test on the kernel datapath (linux). Multiple executions of the former may also lead to multiple failures. This is due to the fact that fragments not yet reassembled are kept in a queue for /proc/sys/net/ipv4/ipfrag_time seconds, and if the kernel receives a fragment already present in the queue, it returns -EINVAL. Below the related log message: |00058|dpif|WARN|system@ovs-system: execute ct(commit) failed (Invalid argument) on packet udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a, nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=0,nw_frag=first,tp_src=1, tp_dst=2 udp_csum:0 Fix the sequence by sending the second fragment in "conntrack - IPv4 fragmentation incomplete reassembled packet", once the checks are done. IPv6 tests are not affected as the defrag kernel code path pretends to add the duplicate fragment to the queue returning -EINPROGRESS, when a duplicate is detected. Signed-off-by: Paolo Valerio <pvalerio@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* system-traffic: Fix incorrect neigh entry in ipv6 header modification test.Ilya Maximets2022-08-081-5/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | The permanent neighbor entry for fc00::1 is added into a wrong namespace, so in order to reply to a ping from at_ns1, the address of fc00::1 has to be discovered. Interfaces are attached to OVS and we're removing flows that can forward ND requests after initial setup. In case ND request wasn't sent and replied before that, at_ns1 will not be able to discover fc00:1 and won't reply to pings. It's hard to catch this condition while running tests locally, but for some reason our CI is failing consistently. Fix the issue by removing all the unnecessary permanent entries and just allowing all the normal traffic to flow through the low priority OVS flow, so all addresses can be discovered. Also adding one more wait to avoid occasional drops of the very first packet. Fixes: 2ff43c78c685 ("packets: Re-calculate IPv6 checksum only for first frag upon modify.") Acked-by: Salem Sol <salems@nvidia.com> Acked-by: Michael Phelan <michael.phelan@intel.com> Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* system-traffic: Don't run IPv6 header modification test on kernels < 5.19.Ilya Maximets2022-08-083-0/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | OVS kernel module is incorrectly updating checksums while changing IPv6 fields of later fragments that doesn't really have L4 headers. This makes the 'ping6 between two ports with header modify' test fail on most of the distribution kernels. The issue got indirectly fixed in latest 5.19 with commit: 12378a5a75e3 ("net: openvswitch: fix parsing of nw_proto for IPv6 fragments") The reason is that set_ipv6() function in net/openvswitch/actions.c is using the protocol number from the parsed flow key and not from the packet itself, and nw_proto=44 is not a protocol where we can update the checksum. It was backported to all supported upstream stable trees, but didn't find its way to most of the distributions yet. Restricting the test to 5.19+ kernels to avoid failures on distro kernels. Additionally allowing the previous test for later fragments to be executed in userspace testsuite. Fixes: 2ff43c78c685 ("packets: Re-calculate IPv6 checksum only for first frag upon modify.") Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* python: Fix E275 missing whitespace after keyword.Ilya Maximets2022-08-043-4/+4
| | | | | | | | | | | | | | | | | | | | With just released flake8 5.0 we're getting a bunch of E275 errors: utilities/bugtool/ovs-bugtool.in:959:23: E275 missing whitespace after keyword tests/test-ovsdb.py:623:11: E275 missing whitespace after keyword python/setup.py:105:8: E275 missing whitespace after keyword python/setup.py:106:8: E275 missing whitespace after keyword python/ovs/db/idl.py:145:15: E275 missing whitespace after keyword python/ovs/db/idl.py:167:15: E275 missing whitespace after keyword make[2]: *** [flake8-check] Error 1 This breaks CI on branches below 2.16. We don't see a problem right now on newer branches because we're installing extra dependencies that backtrack flake8 down to 4.1 or even 3.9. Acked-by: Mike Pattrick <mkp@redhat.com> Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* dpif-netlink: Fix incorrect bit shift in compat mode.Ilya Maximets2022-08-041-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior in lib/dpif-netlink.c:1077:40: runtime error: left shift of 1 by 31 places cannot be represented in type 'int' #0 0x73fc31 in dpif_netlink_port_add_compat lib/dpif-netlink.c:1077:40 #1 0x73fc31 in dpif_netlink_port_add lib/dpif-netlink.c:1132:17 #2 0x2c1745 in dpif_port_add lib/dpif.c:597:13 #3 0x07b279 in port_add ofproto/ofproto-dpif.c:3957:17 #4 0x01b209 in ofproto_port_add ofproto/ofproto.c:2124:13 #5 0xfdbfce in iface_do_create vswitchd/bridge.c:2066:13 #6 0xfdbfce in iface_create vswitchd/bridge.c:2109:13 #7 0xfdbfce in bridge_add_ports__ vswitchd/bridge.c:1173:21 #8 0xfb5319 in bridge_add_ports vswitchd/bridge.c:1189:5 #9 0xfb5319 in bridge_reconfigure vswitchd/bridge.c:901:9 #10 0xfae0f9 in bridge_run vswitchd/bridge.c:3334:9 #11 0xfe67dd in main vswitchd/ovs-vswitchd.c:129:9 #12 0x4b6d8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) #13 0x4b6e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) #14 0x562594eed024 in _start (vswitchd/ovs-vswitchd+0x787024) Fixes: 526df7d8543f ("tunnel: Provide framework for tunnel extensions for VXLAN-GBP and others") Acked-by: Mike Pattrick <mkp@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* packets: Re-calculate IPv6 checksum only for first frag upon modify.Salem Sol2022-08-042-4/+43
| | | | | | | | | | | | | In case of modifying an IPv6 packet src/dst address the L4 checksum should be recalculated only for the first frag. Currently it's done for all frags, leading to incorrect reassembled packet checksum. Fix it by adding a new flag to recalculate the checksum only for the first frag. Fixes: bc7a5acdff08 ("datapath: add ipv6 'set' action") Signed-off-by: Salem Sol <salems@nvidia.com> Acked-by: Mike Pattrick <mkp@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* libopenvswitch.pc: Add missing libs for a static build.Ilya Maximets2022-07-291-1/+1
| | | | | | | | | | | | | | | SSL, BPF, lcap-ng and other libraries are in use by a static library, so they has to be linked while building applications with that static library, i.e. 'pkg-config --libs --static libopenvswitch' must return -lssl, -lcap-ng, etc. in the output for a successful build. For dynamic library (non-private Libs) all these libraries will be dynamically linked to libopenvswitch.so, so the application will pick them up without having a direct dependency. Acked-by: Frode Nordahl <frode.nordahl@canonical.com> Reviewed-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* rhel: Stop installing internal headers.Ilya Maximets2022-07-296-36/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, openvswitch-devel installs following header tree: /usr/include /openflow/*.h /openvswitch /*.h /openflow/*.h /openvswitch/*.h /sparse/*.h /lib/*.h Few issues with that: 1. openflow and openvswitch headers are installed twice. Once in the main /usr/include and second time in the /usr/include/openvswitch/. 2. For some reason internal headers such as lib/*.h and fairly useless headers such as sparse/*.h are installed as well. One more issue is that current pkg-config files doesn't work with builds installed with 'make install', because 'make install' doesn't create this weird header tree. While double install of same headers is not a huge problem, it doesn't seem right. Installation of the internal headers is a bigger issue. They are not part of API/ABI and we do not provide any stability guarantees for them. We are making incompatible changes constantly in minor updates, so users should not rely on these headers. If it's necessary for some external application to use them, this external application should not link with libopenvswitch dynamically and also it can't expect the static library to not break these API/ABI, hence there is no real point installing them. Application should use OVS as a submodule like OVN does or compile itself by obtaining required version of OVS sources otherwise. Another option is to properly export and install required headers. pkg-config configuration files updated as necessary. Fixes: 4886d4d2495b ("debian, rhel: Ship ovs shared libraries and header files") Reviewed-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* netdev-linux: Do not touch LAG members if master is not attached to OVS.Tao Liu2022-07-261-1/+4
| | | | | | | | | | | | | | | | | | | Bond master netdev may be created without a classification type, due to routing or tunneling code. If bond master is not attached to ovs, the ingress block on LAG members should not be updated. Simple reproducer: tc q ls dev net3 ingress ip a add 10.1.1.1/30 dev bond0 ip l set net3 master bond0 tc q ls dev net3 ingress Fixes: d22f8927c3c9 ("netdev-linux: monitor and offload LAG slaves to TC") Signed-off-by: Tao Liu <thomas.liu@ucloud.cn> Acked-by: Roi Dayan <roid@nvidia.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* netdev: Clear auto_classified if netdev reopened with the type specified.Tao Liu2022-07-261-18/+23
| | | | | | | | | | | | | When netdev first opened by netdev_open(..., NULL, ...), netdev_class sets to system by default, and auto_classified sets to true. If netdev reopens by netdev_open(..., "system", ...), auto_classified should be cleared. This will be used in next patch to fix lag issue. Fixes: 8c2c225e481d ("netdev: Fix netdev_open() to track and recreate classless interfaces") Signed-off-by: Tao Liu <thomas.liu@ucloud.cn> Acked-by: Roi Dayan <roid@nvidia.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* system-traffic: Properly stop dangling ping after geneve test.Ilya Maximets2022-07-251-1/+1
| | | | | | | | | Ping process remains in the system after the test. Using a proper macro that will correctly register it for stopping at cleanup stage. Fixes: 134e6831acca ("system-traffic: Check frozen state handling with TLV map change") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Reviewed-by: David Marchand <david.marchand@redhat.com>
* conntrack: Fix conntrack multiple new state.Eli Britstein2022-07-252-3/+13
| | | | | | | | | | | | A connection is established if we see packets from both directions. The cited commit fixed the issue of sending twice in one direction, but still an issue if more than that. Fix it. Fixes: a867c010ee91 ("conntrack: Fix conntrack new state") Signed-off-by: Eli Britstein <elibr@nvidia.com> Acked-by: Paolo Valerio <pvalerio@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
* tc: Fix misaligned access while creating pedit actions.Ilya Maximets2022-07-141-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | calc_offsets() function returns 'data' and 'mask' pointers, which are pointers somewhere inside struct tc_flower_key, and they are not aligned, causing misaligned memory access. For example: ipv6.rewrite_hlimit is at 148 byte offset inside the struct tc_flower_key. While the actual field is in the 7th byte of the IPv6 header in the actual packet. So, pedit will need to write the last byte of the [4-7] range to the actual packet. So, data pointer is positioned to 145th byte inside the tc_flower_key with the 000000FF mask. Obviously, 145th byte inside the structure is not 4-byte aligned. lib/tc.c:2879:34: runtime error: load of misaligned address 0x7f2802eaa321 for type 'ovs_be32' (aka 'unsigned int'), which requires 4 byte alignment 0x7f2802eaa321: note: pointer points here 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ... ^ 0 0xd7f2fb in nl_msg_put_flower_rewrite_pedits lib/tc.c:2879:34 1 0xd7f2fb in nl_msg_put_flower_acts lib/tc.c:3141:25 2 0xd6ae5a in nl_msg_put_flower_options lib/tc.c:3445:12 3 0xd6a2be in tc_replace_flower lib/tc.c:3712:17 4 0xd2bf25 in netdev_tc_flow_put lib/netdev-offload-tc.c:2224:11 5 0x94f6b7 in netdev_flow_put lib/netdev-offload.c:316:14 6 0xcbd19e in parse_flow_put lib/dpif-netlink.c:2289:11 7 0xcbd19e in try_send_to_netdev lib/dpif-netlink.c:2376:15 8 0xcbd19e in dpif_netlink_operate lib/dpif-netlink.c:2447:23 9 0x86536e in dpif_operate lib/dpif.c:1372:13 10 0x6bc289 in handle_upcalls ofproto/ofproto-dpif-upcall.c:1654:5 11 0x6bc289 in recv_upcalls ofproto/ofproto-dpif-upcall.c:892:9 12 0x6b766a in udpif_upcall_handler ofproto/ofproto-dpif-upcall.c:792:13 13 0xb5015a in ovsthread_wrapper lib/ovs-thread.c:422:12 14 0x7f280b2081ce in start_thread (/lib64/libpthread.so.0+0x81ce) 15 0x7f2809e39dd2 in clone (/lib64/libc.so.6+0x39dd2) Fix misaligned read by using appropriate functions. Fixes: 8ada482bbe19 ("tc: Add header rewrite using tc pedit action") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Simon Horman <simon.horman@corigine.com>
* dpif-netdev: Refactor AVX512 runtime checks.David Marchand2022-06-293-8/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | [ original commit fe171e4f109f001f07b867756a261d898f0d2cfc ] As described in the bugzilla below, cpu_has_isa code may be compiled with some AVX512 instructions in it, because cpu.c is built as part of the libopenvswitchavx512. This is a problem when this function (supposed to probe for AVX512 instructions availability) is invoked from generic OVS code, on older CPUs that don't support them. For the same reason, dpcls_subtable_avx512_gather_probe, dp_netdev_input_outer_avx512_probe, mfex_avx512_probe and mfex_avx512_vbmi_probe are potential runtime bombs and can't either be built as part of libopenvswitchavx512. Move cpu.c to be part of the "normal" libopenvswitch. And move other helpers in generic OVS code. Note: - dpcls_subtable_avx512_gather_probe is split in two, because it also needs to do its own magic, - while moving those helpers, prefer direct calls to cpu_has_isa and avoid cast to intermediate integer variables when a simple boolean is enough, Fixes: 352b6c7116cd ("dpif-lookup: add avx512 gather implementation.") Fixes: abb807e27dd4 ("dpif-netdev: Add command to switch dpif implementation.") Fixes: 250ceddcc2d0 ("dpif-netdev/mfex: Add AVX512 based optimized miniflow extract") Fixes: b366fa2f4947 ("dpif-netdev: Call cpuid for x86 isa availability.") Reported-at: https://bugzilla.redhat.com/2100393 Reported-by: Ales Musil <amusil@redhat.com> Co-authored-by: Ales Musil <amusil@redhat.com> Signed-off-by: Ales Musil <amusil@redhat.com> Signed-off-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>