Re: [ovs-dev] [PATCH ovn] ci: ovn-kubernetes: Align CI jobs with recent ovn-kubernetes upstream.
Hi Dumitru,
this is a great improvement, thank you.
Acked-by: Ales Musil
On Tue, Jul 19, 2022 at 10:20 PM Dumitru Ceara wrote:
> This has the benefit that we now use names that have more meaning
> instead of 'true/false'. The change is inspired from the current
> contents of the upstream ovn-kubernetes e2e GHA test specification:
>
> https://github.com/ovn-org/ovn-kubernetes/blob/69d8a995d79b280c4bb46f079fbe26f4d5550b16/.github/workflows/test.yml#L349
>
> E.g., a new job will be called:
> e2e (control-plane, HA, shared, ipv4, noSnatGW)
>
> Instead of:
> e2e (control-plane, true, true, true, ipv4, IPv4, true, false)
>
> Signed-off-by: Dumitru Ceara
> ---
> Note: one of the jobs is still failing but that's a test issue and is
> being tracked upstream by:
> https://github.com/ovn-org/ovn-kubernetes/issues/3078
> ---
> .github/workflows/ovn-kubernetes.yml | 59 +++-
> 1 file changed, 23 insertions(+), 36 deletions(-)
>
> diff --git a/.github/workflows/ovn-kubernetes.yml
> b/.github/workflows/ovn-kubernetes.yml
> index 431e47660..03f35d7a3 100644
> --- a/.github/workflows/ovn-kubernetes.yml
> +++ b/.github/workflows/ovn-kubernetes.yml
> @@ -55,44 +55,31 @@ jobs:
> strategy:
>fail-fast: false
>matrix:
> -target:
> - - shard: shard-conformance
> -hybrid-overlay: false
> -multicast-enable: false
> -emptylb-enable: false
> - - shard: control-plane
> -hybrid-overlay: true
> -multicast-enable: true
> -emptylb-enable: true
> -ipfamily:
> - - ip: ipv4
> -name: "IPv4"
> -ipv4: true
> -ipv6: false
> - - ip: ipv6
> -name: "IPv6"
> -ipv4: false
> -ipv6: true
> - - ip: dualstack
> -name: "Dualstack"
> -ipv4: true
> -ipv6: true
> -# Example of how to exclude a fully qualified test:
> -# - {"ipfamily": {"ip": ipv4}, "ha": {"enabled": "false"},
> "gateway-mode": shared, "target": {"shard": shard-n-other}}
> -exclude:
> - # Not currently supported but needs to be.
> - - {"ipfamily": {"ip": dualstack}, "target": {"shard":
> control-plane}}
> - - {"ipfamily": {"ip": ipv6}, "target": {"shard": control-plane}}
> +# Valid options are:
> +# target: ["shard-conformance", "control-plane" ]
> +# shard-conformance: hybrid-overlay = multicast-enable =
> emptylb-enable = false
> +# control-plane: hybrid-overlay = multicast-enable =
> emptylb-enable = true
> +# gateway-mode: ["local", "shared"]
> +# ipfamily: ["ipv4", "ipv6", "dualstack"]
> +# disable-snat-multiple-gws: ["noSnatGW", "snatGW"]
> +include:
> + - {"target": "shard-conformance", "ha": "HA", "gateway-mode":
> "local", "ipfamily": "ipv6", "disable-snat-multiple-gws": "snatGW"}
> + - {"target": "shard-conformance", "ha": "HA", "gateway-mode":
> "local", "ipfamily": "dualstack", "disable-snat-multiple-gws": "snatGW"}
> + - {"target": "shard-conformance", "ha": "HA", "gateway-mode":
> "shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "snatGW"}
> + - {"target": "shard-conformance", "ha": "HA", "gateway-mode":
> "shared", "ipfamily": "ipv6", "disable-snat-multiple-gws": "snatGW"}
> + - {"target": "control-plane", "ha": "HA", "gateway-mode":
> "shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "noSnatGW"}
> + - {"target": "control-plane", "ha": "HA", "gateway-mode":
> "shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "snatGW"}
> needs: [build]
> env:
> - JOB_NAME: "${{ matrix.target.shard }}-${{ matrix.ipfamily.name }}"
> + JOB_NAME: "${{ matrix.target }}-${{ matrix.ha }}-${{
> matrix.gateway-mode }}-${{ matrix.ipfamily }}-${{
> matrix.disable-snat-multiple-gws }}-${{ matrix.second-bridge }}"
> + OVN_HYBRID_OVERLAY_ENABLE: "${{ matrix.target == 'control-plane' }}"
> + OVN_MULTICAST_ENABLE: "${{ matrix.target == 'control-plane' }}"
> + OVN_EMPTY_LB_EVENTS: "${{ matrix.target == 'control-plane' }}"
>OVN_HA: "true"
> - KIND_IPV4_SUPPORT: "${{ matrix.ipfamily.ipv4 }}"
> - KIND_IPV6_SUPPORT: "${{ matrix.ipfamily.ipv6 }}"
> - OVN_HYBRID_OVERLAY_ENABLE: "${{ matrix.target.hybrid-overlay }}"
> - OVN_GATEWAY_MODE: "shared"
> - OVN_MULTICAST_ENABLE: "${{ matrix.target.multicast-enable }}"
> - OVN_EMPTY_LB_EVENTS: "${{ matrix.target.emptylb-enable }}"
> + OVN_DISABLE_SNAT_MULTIPLE_GWS: "${{
> matrix.disable-snat-multiple-gws == 'noSnatGW' }}"
> + OVN_GATEWAY_MODE: "${{ matrix.gateway-mode }}"
> + KIND_IPV4_SUPPORT: "${{ matrix.ipfamily == 'IPv4' ||
> matrix.ipfamily == 'dualstack' }}"
> + KIND_IPV6_SUPPORT: "${{ matrix.ipfamily == 'IPv6' ||
> matrix.ipfamily == 'd
Re: [ovs-dev] [RFC ovn] controller: send GARPs for LSPs in vlan-backed network scenario
On 7/19/22 17:02, Numan Siddique wrote: > On Thu, Jul 14, 2022 at 1:30 PM Dumitru Ceara wrote: >> >> On 7/14/22 17:35, Numan Siddique wrote: >>> On Thu, Jul 14, 2022 at 3:36 AM Lorenzo Bianconi >>> wrote: > On 7/13/22 18:07, Lorenzo Bianconi wrote: >>> On 6/17/22 00:31, Lorenzo Bianconi wrote: When using VLAN backed networks and OVN routers leveraging the 'ovn-chassis-mac-mappings' option, the eth.src field is replaced by the chassis mac address in order to not expose the router mac address from different nodes and confuse the TOR switch. However doing so the TOR switch is not able to learn the port/mac bindings for routed E/W traffic and it is force to always flood it. Fix this issue adding the capability to send GARP traffic for logical switch ports if the corresponding logical switch has the ovn-lsp-garp parameter set to true in the option column. More into about the issue can be found here [0]. [0] https://mail.openvswitch.org/pipermail/ovs-discuss/2020-September/050678.html Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2087779 Signed-off-by: Lorenzo Bianconi --- >>> >>> Hi Lorenzo, >> >> Hi Dumitru, >> >> Thanks for reviewing it :) >> >>> >>> I have a few concerns with this approach: >>> >>> a. The CMS will have to set this option for all VMs on all logical >>> switches which will enable periodic GARPs for all of them all the time. >>> That seems like quite a lot of broadcast traffic in the fabric. >>> >>> b. There's no guarantee that the GARPs are sent in time to prevent the >>> FDB timeouts on the ToR switches. At best we could make the interval >>> configurable but I don't think this is way better either. >>> >>> c. This is not really introduced by your patch but we will be causing >>> this more often now. With the topology: >>> >>> (HV1) VM1 -- LS1 --- LR -- LS2 -- VM2 (HV2) >>> (VLAN-backed network) >>> >>> HV1 configured with chassis mac mapping HV1-MAC >>> HV2 configured with chassis mac mapping HV2-MAC >>> >>> We're leaking MAC addresses from LS1's broadcast domain (VM1-MAC) and >>> from LS2's broadcast domain (VM2-MAC) into the fabric. I'm not sure >>> that's OK. >>> >>> I'm not sure I understood what you mean here. >>> Let's say we have 2 VMs - VM1 and VM3 connected to the same VLAN >>> logical switch LS1 >>> and if VM1 is on chassis HV1 and VM3 is on chassis HV3, the fabric >>> will learn VM1's and VM3's mac >>> when they both communicate right ? Please note that logical switch >>> LS1 is a vlan tenant network with >>> a localnet port (so no geneve tunnels are used for the traffic from VM1 to >>> VM3). >>> >>> >> >> You're right, I misread the logical/physical topologies. The MACs will >> only be visible in the VLANs corresponding to the localnet port of each >> of the logical switches. Please ignore this part. >> >>> >>> I think a proper solution is to change how we run the logical pipelines >>> in case of vlan-backed networks. We currently have an assymetry: >>> >>> For packets flowing from VM1 to VM2 we execute: >>> - on HV1: LS1-ingress, LS1-egress, LR-ingress, LR-egress, LS2-ingress >>> - on HV2: LS2-egress >>> >>> For packets flowing from VM2 to VM1 we execute: >>> - on HV2: LS2-ingress, LS2-egress, LR-ingress, LR-egress, LS1-ingress >>> - on HV1: LS1-egress >>> >>> What if we change this to: >>> VM1 -> VM2: >>> - on HV1: LS1-ingress, LS1-egress, LR-ingress >>> - on HV2: LR-egress, LS2-ingress, LS2-egress >>> >>> VM2 -> VM1: >>> - on HV2: LS2-ingress, LS2-egress, LR-ingress >>> - on HV2: LR-egress, LS1-ingress, LS1-egress >> >>> >>> I'm not sure how easy it would be to run the pipeline as you suggested >>> when ovn-chassis-mac-mappings is configured. >> >> Indeed it doesn't seem possible as we have no knowledge about what >> happened prior to the packet being received on the vlan, it could've >> been already processed by an ovn node. >> >>> However OVN does support centralized routing if a logical router has a >>> distributed gateway port (connecting to the provider network) >>> and the other logical router ports connecting to the tenant VLAN >>> networks have the option - reside-on-redirect-chassis configured. >>> >>> So in a way CMS can use the centralized routing to avoid this issue. >>> However if we want to fix this issue for distributed routing, looks >>> to me >>> the proposed RFC patch is the only way to fix it. And with the proper >>> documentation CMS can know the benefits and drawbacks of both the >>> options. >>> >> >> I still think that's potentially a lot of broadcast traffic. > > If you compare this GARP broadcast traffic with the present flooding
Re: [ovs-dev] [PATCH ovn v2 2/2] multicast: Properly flood IGMP queries and reports.
On 7/19/22 22:40, Dumitru Ceara wrote: > RFC 4541 "Considerations for Internet Group Management Protocol (IGMP) > and Multicast Listener Discovery (MLD) Snooping Switches" [0] states: > > For IGMP packets: > 2.1.1. IGMP Forwarding Rules >1) A snooping switch should forward IGMP Membership Reports only to > those ports where multicast routers are attached. > > Alternatively stated: a snooping switch should not forward IGMP > Membership Reports to ports on which only hosts are attached. An > administrative control may be provided to override this > restriction, allowing the report messages to be flooded to other > ports. > > This is the main IGMP snooping functionality for the control path. > > Sending membership reports to other hosts can result, for IGMPv1 > and IGMPv2, in unintentionally preventing a host from joining a > specific multicast group. > > When an IGMPv1 or IGMPv2 host receives a membership report for a > group address that it intends to join, the host will suppress its > own membership report for the same group. This join or message > suppression is a requirement for IGMPv1 and IGMPv2 hosts. > However, if a switch does not receive a membership report from the > host it will not forward multicast data to it. > > In OVN this translates to forwarding reports only on: > a. ports where mrouters have been learned (IGMP queries were received on >those ports) > b. ports connecting a multicast enabled logical switch to a multicast >relay enabled logical router (OVN mrouter) > c. ports configured with mcast_flood_reports=true > > 2.1.2. Data Forwarding Rules > >1) Packets with a destination IP address outside 224.0.0.X which are > not IGMP should be forwarded according to group-based port > membership tables and must also be forwarded on router ports. > > In OVN this translates to forwarding traffic for a group G to: > a. all ports where G was learned > b. all ports where an (external or OVN) mrouter was learned. > c. all ports configured with mcast_flood=true > > IGMP queries are already snooped by ovn-controller. Just propagate the > information about where mrouters are to the Southbound DB through means > of a custom IGMP_Group (name="mrouters"). > > Snooped queries are then re-injected in the OVS pipeline with outport > set to MC_FLOOD_L2 (only flood them in the L2 domain). > > Snooped reports are re-injected in the OVS pipeline with outport set to > MC_MROUTER_FLOOD (flood them to snooped mrouters and OVN mrouters). > > The same behavior applies to IPv6 too (MLD). > > [0] https://datatracker.ietf.org/doc/html/rfc4541 > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1933990 > Reported-at: https://github.com/ovn-org/ovn/issues/126 > Acked-by: Numan Siddique > Signed-off-by: Dumitru Ceara Lucas, sorry, I dropped your "Tested-by" by accident. Would you have some time by any chance to do a quick sanity test, just in case? There was no code change between v1 and v2 of this patch. Thanks, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 4/4] multicast: Properly flood IGMP queries and reports.
On 7/19/22 19:19, Numan Siddique wrote: > On Thu, Jul 7, 2022 at 6:27 AM Lucas Alvares Gomes > wrote: >> >> On Thu, Jul 7, 2022 at 12:24 PM Lucas Alvares Gomes >> wrote: >>> >>> Hi, >>> >>> I tested this series of patches with OpenStack upstream [0] since we >>> have some automated tests for IGMP and it seems good: >>> >>> test_multicast_between_vms_on_same_network[id-113486fc-24c9-4be4-8361-03b1c9892867] >>> pass (link: >>> https://98794ab76263ee253bc7-6b6bf6aa2495d781bdba3c8c61916451.ssl.cf1.rackcdn.com/848934/2/check/neutron-tempest-plugin-ovn/b3191bf/testr_results.html, >>> this will expire at some point) >>> >>> What I did was to change ML2/OVN to set mcast_flood to False on the >>> LSPs, apply the series of patches in OVN and then run the tests. >>> >>> Tested-By: >>> https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/848934/ >>> >> >> Tested-By: Lucas Alvares Gomes > > Acked-by: Numan Siddique Thanks, I included your ack in v2 (no change to this patch): https://patchwork.ozlabs.org/project/ovn/list/?series=310294&state=* > > It would be great if someone else also reviews this patch. > I agree, the more eyes on this change the better! > Numan Thanks, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 3/4] tests: Factor out reset_pcap_file() helper.
On 7/19/22 19:17, Numan Siddique wrote: > On Tue, Jul 5, 2022 at 7:11 AM Dumitru Ceara wrote: >> >> Signed-off-by: Dumitru Ceara > > > Thanks for this patch. > Thanks for reviewing this patch set! > The below test cases is failing with this patch > > 347. ovn.at:14447: 347. localnet connectivity with multiple > requested-chassis -- ovn-northd -- parallelization=yes -- > ovn_monitor_all=yes (ovn.at:14447): FAILED (--) > 348. ovn.at:14447: 348. localnet connectivity with multiple > requested-chassis -- ovn-northd -- parallelization=yes -- > ovn_monitor_all=no (ovn.at:14447): FAILED (--) > 349. ovn.at:14447: 349. localnet connectivity with multiple > requested-chassis -- ovn-northd -- parallelization=no -- > ovn_monitor_all=yes (ovn.at:14447): FAILED (--) > > +ovs-vsctl: no row "hv1" in table Interface > ../../tests/ovn-macros.at:384: exit code was 1, expected 0 > 350. ovn.at:14447: 350. localnet connectivity with multiple > requested-chassis -- ovn-northd -- parallelization=no -- > ovn_monitor_all=no (ovn.at:14447): FAILED (--) > > Can you please take a look. > It just needed a rebase and fixing up a new call to reset_pcap_file(). I posted v2: https://patchwork.ozlabs.org/project/ovn/list/?series=310294&state=* Thanks, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2 2/2] multicast: Properly flood IGMP queries and reports.
RFC 4541 "Considerations for Internet Group Management Protocol (IGMP)
and Multicast Listener Discovery (MLD) Snooping Switches" [0] states:
For IGMP packets:
2.1.1. IGMP Forwarding Rules
1) A snooping switch should forward IGMP Membership Reports only to
those ports where multicast routers are attached.
Alternatively stated: a snooping switch should not forward IGMP
Membership Reports to ports on which only hosts are attached. An
administrative control may be provided to override this
restriction, allowing the report messages to be flooded to other
ports.
This is the main IGMP snooping functionality for the control path.
Sending membership reports to other hosts can result, for IGMPv1
and IGMPv2, in unintentionally preventing a host from joining a
specific multicast group.
When an IGMPv1 or IGMPv2 host receives a membership report for a
group address that it intends to join, the host will suppress its
own membership report for the same group. This join or message
suppression is a requirement for IGMPv1 and IGMPv2 hosts.
However, if a switch does not receive a membership report from the
host it will not forward multicast data to it.
In OVN this translates to forwarding reports only on:
a. ports where mrouters have been learned (IGMP queries were received on
those ports)
b. ports connecting a multicast enabled logical switch to a multicast
relay enabled logical router (OVN mrouter)
c. ports configured with mcast_flood_reports=true
2.1.2. Data Forwarding Rules
1) Packets with a destination IP address outside 224.0.0.X which are
not IGMP should be forwarded according to group-based port
membership tables and must also be forwarded on router ports.
In OVN this translates to forwarding traffic for a group G to:
a. all ports where G was learned
b. all ports where an (external or OVN) mrouter was learned.
c. all ports configured with mcast_flood=true
IGMP queries are already snooped by ovn-controller. Just propagate the
information about where mrouters are to the Southbound DB through means
of a custom IGMP_Group (name="mrouters").
Snooped queries are then re-injected in the OVS pipeline with outport
set to MC_FLOOD_L2 (only flood them in the L2 domain).
Snooped reports are re-injected in the OVS pipeline with outport set to
MC_MROUTER_FLOOD (flood them to snooped mrouters and OVN mrouters).
The same behavior applies to IPv6 too (MLD).
[0] https://datatracker.ietf.org/doc/html/rfc4541
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1933990
Reported-at: https://github.com/ovn-org/ovn/issues/126
Acked-by: Numan Siddique
Signed-off-by: Dumitru Ceara
---
V2:
- Added Numan's ack.
---
controller/ip-mcast.c | 129 -
controller/ip-mcast.h | 14
controller/pinctrl.c| 129 +
lib/ip-mcast-index.h|5 +
lib/mcast-group-index.h |4 -
northd/northd.c | 54 +++
northd/ovn-northd.8.xml |6 --
tests/ovn-macros.at | 25 +++
tests/ovn.at| 164 ++-
9 files changed, 472 insertions(+), 58 deletions(-)
diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c
index 9b0b4465a..a870fb29e 100644
--- a/controller/ip-mcast.c
+++ b/controller/ip-mcast.c
@@ -16,6 +16,7 @@
#include
#include "ip-mcast.h"
+#include "ip-mcast-index.h"
#include "lport.h"
#include "lib/ovn-sb-idl.h"
@@ -27,6 +28,18 @@ struct igmp_group_port {
const struct sbrec_port_binding *port;
};
+static const struct sbrec_igmp_group *
+igmp_group_lookup_(struct ovsdb_idl_index *igmp_groups,
+ const char *addr_str,
+ const struct sbrec_datapath_binding *datapath,
+ const struct sbrec_chassis *chassis);
+
+static struct sbrec_igmp_group *
+igmp_group_create_(struct ovsdb_idl_txn *idl_txn,
+ const char *addr_str,
+ const struct sbrec_datapath_binding *datapath,
+ const struct sbrec_chassis *chassis);
+
struct ovsdb_idl_index *
igmp_group_index_create(struct ovsdb_idl *idl)
{
@@ -54,17 +67,16 @@ igmp_group_lookup(struct ovsdb_idl_index *igmp_groups,
return NULL;
}
-struct sbrec_igmp_group *target =
-sbrec_igmp_group_index_init_row(igmp_groups);
-
-sbrec_igmp_group_index_set_address(target, addr_str);
-sbrec_igmp_group_index_set_datapath(target, datapath);
-sbrec_igmp_group_index_set_chassis(target, chassis);
+return igmp_group_lookup_(igmp_groups, addr_str, datapath, chassis);
+}
-const struct sbrec_igmp_group *g =
-sbrec_igmp_group_index_find(igmp_groups, target);
-sbrec_igmp_group_index_destroy_row(target);
-return g;
+const struct sbrec_igmp_group *
+igmp_mrout
[ovs-dev] [PATCH ovn v2 1/2] tests: Factor out reset_pcap_file() helper.
Signed-off-by: Dumitru Ceara
---
V2: Rebase and fix up new calls in test "localnet connectivity with
multiple requested-chassis"
---
tests/ovn-macros.at | 20 +++
tests/ovn.at| 364 ---
2 files changed, 46 insertions(+), 338 deletions(-)
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 427b7b669..0ad6b58c4 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -511,9 +511,13 @@ wait_for_ports_up() {
fi
}
-# reset_pcap_file iface pcap_file
+# reset_iface_pcap_file iface pcap_file
# Resets the pcap file associates with OVS interface. should be used
# with dummy datapath.
+#
+# XXX: This should actually replace reset_pcap_file() as they do almost
+# exactly the same thing but the "wait while the pcap file has the size
+# of the PCAP header" check causes tests to fail.
reset_iface_pcap_file() {
local iface=$1
local pcap_file=$2
@@ -528,6 +532,20 @@ options:rxq_pcap=${pcap_file}-rx.pcap
OVS_WAIT_WHILE([test 24 = $(wc -c ${pcap_file}-tx.pcap | cut -d " " -f1)])
}
+# reset_pcap_file iface pcap_file
+# Resets the pcap file associates with OVS interface. should be used
+# with dummy datapath.
+reset_pcap_file() {
+local iface=$1
+local pcap_file=$2
+check rm -f dummy-*.pcap
+check ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
+options:rxq_pcap=dummy-rx.pcap
+check rm -f ${pcap_file}*.pcap
+check ovs-vsctl -- set Interface $iface
options:tx_pcap=${pcap_file}-tx.pcap \
+options:rxq_pcap=${pcap_file}-rx.pcap
+}
+
# Receive a packet on a dummy netdev interface. If we expect packets to be
# recorded, then wait until the pcap file reflects the change.
netdev_dummy_receive() {
diff --git a/tests/ovn.at b/tests/ovn.at
index 7d10610fd..a3e8a7758 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -6487,16 +6487,6 @@ compare_dhcp_packets() {
fi
}
-reset_pcap_file() {
-local iface=$1
-local pcap_file=$2
-check ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
-options:rxq_pcap=dummy-rx.pcap
-rm -f ${pcap_file}*.pcap
-check ovs-vsctl -- set Interface $iface
options:tx_pcap=${pcap_file}-tx.pcap \
-options:rxq_pcap=${pcap_file}-rx.pcap
-}
-
AT_CAPTURE_FILE([sbflows])
ovn-sbctl dump-flows > sbflows
@@ -7082,16 +7072,6 @@ test_dhcpv6() {
as hv1 ovs-appctl netdev-dummy/receive hv1-vif$inport $request
}
-reset_pcap_file() {
-local iface=$1
-local pcap_file=$2
-ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
-options:rxq_pcap=dummy-rx.pcap
-rm -f ${pcap_file}*.pcap
-ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
-options:rxq_pcap=${pcap_file}-rx.pcap
-}
-
AT_CAPTURE_FILE([ofctl_monitor0.log])
as hv1 ovs-ofctl monitor br-int resume --detach --no-chdir \
--pidfile=ovs-ofctl0.pid 2> ofctl_monitor0.log
@@ -8789,16 +8769,6 @@ OVS_WAIT_UNTIL([
])
ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0
nat-addresses="f0:00:00:00:00:03 192.168.0.3"
-reset_pcap_file() {
-local iface=$1
-local pcap_file=$2
-ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
-options:rxq_pcap=dummy-rx.pcap
-rm -f ${pcap_file}*.pcap
-ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
-options:rxq_pcap=${pcap_file}-rx.pcap
-}
-
reset_pcap_file snoopvif hv1/snoopvif
OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 140])
@@ -8873,20 +8843,8 @@ OVN_WAIT_PATCH_PORT_FLOWS(["ln_port"], ["hv2"])
# Otherwise a garp might be sent after pcap have been reset but before chassis
is removed
AT_CHECK([ovn-nbctl --wait=hv remove logical_router lr0 options chassis])
-reset_pcap_file() {
-local hv=$1
-local iface=$2
-local pcap_file=$3
-as $hv
-ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
-options:rxq_pcap=dummy-rx.pcap
-rm -f ${pcap_file}*.pcap
-ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
-options:rxq_pcap=${pcap_file}-rx.pcap
-}
-
-reset_pcap_file hv1 snoopvif hv1/snoopvif
-reset_pcap_file hv2 snoopvif hv2/snoopvif
+as hv1 reset_pcap_file snoopvif hv1/snoopvif
+as hv2 reset_pcap_file snoopvif hv2/snoopvif
hv1_uuid=$(ovn-sbctl --bare --columns _uuid list chassis hv1)
AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
@@ -8902,8 +8860,8 @@ OVN_CHECK_PACKETS([hv2/snoopvif-tx.pcap],
[empty_expected])
# Temporarily remove lr0 chassis
AT_CHECK([ovn-nbctl remove logical_router lr0 options chassis])
-reset_pcap_file hv1 snoopvif hv1/snoopvif
-reset_pcap_file hv2 snoopvif hv2/snoopvif
+as hv1 reset_pcap_file snoopvif hv1/snoopvif
+as hv2 reset_pcap_file snoopvif hv2/snoopvif
hv2_uuid=$(ovn-sbctl --bare --columns _uuid list chassis hv2)
AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv2])
@@ -8988,16 +8946,6 @@ AT_CHECK([sort packets], [0], [expout])
# due to GARP backoff
ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 n
[ovs-dev] [PATCH v2 ovn 0/2] Properly forward packets to IP mrouters.
The first patch of the series is not necessarily related to the issue being fixed by patch 2/2 but it's good to have as it improves code documentation and simplifies (a bit) the multicast related tests. It can also be applied independently of patch 2/2. The second patch of the series changes the way OVN floods IP multicast control packets (IGMP/MLD queries and reports) making it more RFC compliant. V2: - Dropped first 2 patches (already applied to main). - Rebased and fixed up changed test in patch 1/2 (former 3/4). - Added Numan's ack to the last patch. Dumitru Ceara (2): tests: Factor out reset_pcap_file() helper. multicast: Properly flood IGMP queries and reports. controller/ip-mcast.c | 129 +++ controller/ip-mcast.h | 14 controller/pinctrl.c| 129 +++ lib/ip-mcast-index.h| 5 ++ lib/mcast-group-index.h | 4 - northd/northd.c | 54 + northd/ovn-northd.8.xml | 6 +- tests/ovn-macros.at | 25 ++ tests/ovn.at| 164 ++-- 9 files changed, 472 insertions(+), 58 deletions(-) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-offload-dpdk: setting RSS hash types in RSS action
On 6/24/22 17:17, Eli Britstein wrote: > +Ori > > -- > *From:* Finn, Emma > *Sent:* Friday, June 24, 2022 5:41 PM > *To:* Ilya Maximets ; ovs-dev@openvswitch.org > ; Stokes, Ian > *Cc:* Eli Britstein ; Slava Ovsiienko > ; Flavio Leitner ; Matan Azrad > > *Subject:* RE: [ovs-dev] [PATCH] netdev-offload-dpdk: setting RSS hash types > in RSS action > > External email: Use caution opening links or attachments > > >> -Original Message- >> From: dev On Behalf Of Ilya >> Maximets >> Sent: Monday 20 June 2022 19:00 >> To: ovs-dev@openvswitch.org; Stokes, Ian >> Cc: Eli Britstein ; viachesl...@nvidia.com; Flavio Leitner >> ; i.maxim...@ovn.org; ma...@nvidia.com >> Subject: Re: [ovs-dev] [PATCH] netdev-offload-dpdk: setting RSS hash types >> in RSS action >> >> On 3/22/22 03:08, Harold Huang wrote: >> > Hello, >> > Is there any opinion from the OVS or DPDK MLX5 driver maintainers? >> > This is a serious issue we've found when we use MLX5 PMD driver to >> > offload OVS-DPDK. >> >> It looks like DPDK is very inconsistent and drivers do not really put any >> effort >> in setting up a "best effort" hashing mechanism. >> mlx5 driver seems to use just RTE_ETH_RSS_IP. >> >> I suppose, there is no harm in using the same set of hashing fields as we do >> for RSS configuration in netdev-dpdk. >> >> Ian, Eli, what do you think? >> > Hi, > > I took this patch and tested on both Intel E810 and i40e. Flows are being > offloaded correctly and this won't break MARK and RSS action for Intel NICs. Thanks, Emma, Harold and Tonghao. Since there is no further progress on this topic and the patch seems to work fine, I applied it. Also backported down to 2.17 since this behavior may cause significant performance issues. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ci: ovn-kubernetes: Align CI jobs with recent ovn-kubernetes upstream.
This has the benefit that we now use names that have more meaning
instead of 'true/false'. The change is inspired from the current
contents of the upstream ovn-kubernetes e2e GHA test specification:
https://github.com/ovn-org/ovn-kubernetes/blob/69d8a995d79b280c4bb46f079fbe26f4d5550b16/.github/workflows/test.yml#L349
E.g., a new job will be called:
e2e (control-plane, HA, shared, ipv4, noSnatGW)
Instead of:
e2e (control-plane, true, true, true, ipv4, IPv4, true, false)
Signed-off-by: Dumitru Ceara
---
Note: one of the jobs is still failing but that's a test issue and is
being tracked upstream by:
https://github.com/ovn-org/ovn-kubernetes/issues/3078
---
.github/workflows/ovn-kubernetes.yml | 59 +++-
1 file changed, 23 insertions(+), 36 deletions(-)
diff --git a/.github/workflows/ovn-kubernetes.yml
b/.github/workflows/ovn-kubernetes.yml
index 431e47660..03f35d7a3 100644
--- a/.github/workflows/ovn-kubernetes.yml
+++ b/.github/workflows/ovn-kubernetes.yml
@@ -55,44 +55,31 @@ jobs:
strategy:
fail-fast: false
matrix:
-target:
- - shard: shard-conformance
-hybrid-overlay: false
-multicast-enable: false
-emptylb-enable: false
- - shard: control-plane
-hybrid-overlay: true
-multicast-enable: true
-emptylb-enable: true
-ipfamily:
- - ip: ipv4
-name: "IPv4"
-ipv4: true
-ipv6: false
- - ip: ipv6
-name: "IPv6"
-ipv4: false
-ipv6: true
- - ip: dualstack
-name: "Dualstack"
-ipv4: true
-ipv6: true
-# Example of how to exclude a fully qualified test:
-# - {"ipfamily": {"ip": ipv4}, "ha": {"enabled": "false"},
"gateway-mode": shared, "target": {"shard": shard-n-other}}
-exclude:
- # Not currently supported but needs to be.
- - {"ipfamily": {"ip": dualstack}, "target": {"shard": control-plane}}
- - {"ipfamily": {"ip": ipv6}, "target": {"shard": control-plane}}
+# Valid options are:
+# target: ["shard-conformance", "control-plane" ]
+# shard-conformance: hybrid-overlay = multicast-enable =
emptylb-enable = false
+# control-plane: hybrid-overlay = multicast-enable =
emptylb-enable = true
+# gateway-mode: ["local", "shared"]
+# ipfamily: ["ipv4", "ipv6", "dualstack"]
+# disable-snat-multiple-gws: ["noSnatGW", "snatGW"]
+include:
+ - {"target": "shard-conformance", "ha": "HA", "gateway-mode":
"local", "ipfamily": "ipv6", "disable-snat-multiple-gws": "snatGW"}
+ - {"target": "shard-conformance", "ha": "HA", "gateway-mode":
"local", "ipfamily": "dualstack", "disable-snat-multiple-gws": "snatGW"}
+ - {"target": "shard-conformance", "ha": "HA", "gateway-mode":
"shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "snatGW"}
+ - {"target": "shard-conformance", "ha": "HA", "gateway-mode":
"shared", "ipfamily": "ipv6", "disable-snat-multiple-gws": "snatGW"}
+ - {"target": "control-plane", "ha": "HA", "gateway-mode":
"shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "noSnatGW"}
+ - {"target": "control-plane", "ha": "HA", "gateway-mode":
"shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "snatGW"}
needs: [build]
env:
- JOB_NAME: "${{ matrix.target.shard }}-${{ matrix.ipfamily.name }}"
+ JOB_NAME: "${{ matrix.target }}-${{ matrix.ha }}-${{ matrix.gateway-mode
}}-${{ matrix.ipfamily }}-${{ matrix.disable-snat-multiple-gws }}-${{
matrix.second-bridge }}"
+ OVN_HYBRID_OVERLAY_ENABLE: "${{ matrix.target == 'control-plane' }}"
+ OVN_MULTICAST_ENABLE: "${{ matrix.target == 'control-plane' }}"
+ OVN_EMPTY_LB_EVENTS: "${{ matrix.target == 'control-plane' }}"
OVN_HA: "true"
- KIND_IPV4_SUPPORT: "${{ matrix.ipfamily.ipv4 }}"
- KIND_IPV6_SUPPORT: "${{ matrix.ipfamily.ipv6 }}"
- OVN_HYBRID_OVERLAY_ENABLE: "${{ matrix.target.hybrid-overlay }}"
- OVN_GATEWAY_MODE: "shared"
- OVN_MULTICAST_ENABLE: "${{ matrix.target.multicast-enable }}"
- OVN_EMPTY_LB_EVENTS: "${{ matrix.target.emptylb-enable }}"
+ OVN_DISABLE_SNAT_MULTIPLE_GWS: "${{ matrix.disable-snat-multiple-gws ==
'noSnatGW' }}"
+ OVN_GATEWAY_MODE: "${{ matrix.gateway-mode }}"
+ KIND_IPV4_SUPPORT: "${{ matrix.ipfamily == 'IPv4' || matrix.ipfamily ==
'dualstack' }}"
+ KIND_IPV6_SUPPORT: "${{ matrix.ipfamily == 'IPv6' || matrix.ipfamily ==
'dualstack' }}"
steps:
- name: Free up disk space
@@ -138,7 +125,7 @@ jobs:
- name: Run Tests
run: |
-make -C test ${{ matrix.target.shard }}
+make -C test ${{ matrix.target }}
working-directory: src/github.com/ovn-org/ovn-kubernetes
- name: Upload Junit Reports
--
2.3
Re: [ovs-dev] [PATCH ovn v2] tests: fixed multiple flaky tests (not waiting for patch flows)
On Mon, Jun 27, 2022 at 4:30 AM Xavier Simonart wrote:
>
> The following test cases were sometimes failing, (mainly) for the same reason
> i.e. packet lost as being sent before patch ports were installed.
> - 2 HVs, 2 LS, switching between multiple localnet ports with same tags
> - VLAN transparency, passthru=true, ARP responder disabled
> - send gratuitous arp for l3gateway only on selected chassis
> - 4 HV, 3 LS, 2 LR, packet test with HA distributed router gateway port
> - 1 LR with distributed router gateway port
> - send gratuitous arp for NAT rules on distributed router
> - send gratuitous ARP for NAT rules on HA distributed router
> - localnet connectivity with multiple requested-chassis
> - 2 HVs, 2 lports/HV, localnet ports, DVR chassis mac
> - 2 HVs, 4 lports/HV, localnet ports
> - 2 HVs, 2 LS, routing works for multiple collocated segments attached to
> different switches
> - 2 HVs, 1 LS, no switching between multiple localnet ports with different
> tags
>
> Signed-off-by: Xavier Simonart
Thanks for fixing these flakes. I applied to the main branch.
Numan
>
> ---
> v2: - handled Mark's comments.
>rebased on origin/main
> ---
> tests/ovn.at | 81 ++--
> 1 file changed, 54 insertions(+), 27 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index bfaa41962..b855653ac 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -76,19 +76,22 @@ m4_divert_text([PREPARE_TESTS],
> }
>
> ovn_wait_patch_port_flows () {
> - patch_port=$1
> - hv=$2
> - echo "$3: waiting for flows for $patch_port on $hv"
> - # Patch port might be created after ports are reported up
> - OVS_WAIT_UNTIL([
> - test 1 = $(as $hv ovs-vsctl show | grep "Port $patch_port" | wc -l)
> - ])
> - # Wait for a flow outputing to patch port
> - OVS_WAIT_UNTIL([
> - hv_patch_ofport=$(as $hv ovs-vsctl --bare --columns ofport find
> Interface name=$patch_port)
> - echo "$patch_port=$hv_patch_ofport"
> - test 1 = $(as $hv ovs-ofctl dump-flows br-int | grep -c
> "output:$hv_patch_ofport")
> - ])
> + for localnet in $1; do
> + patch_port="patch-br-int-to-$localnet"
> + for hv in $2; do
> + echo "$3: waiting for flows for $patch_port on $hv"
> + # Patch port might be created after ports are reported up
> + OVS_WAIT_UNTIL([
> + test 1 = $(as $hv ovs-vsctl show | grep "Port \b$patch_port\b"
> | wc -l)
> + ])
> + # Wait for a flow outputing to patch port
> + OVS_WAIT_UNTIL([
> + hv_patch_ofport=$(as $hv ovs-vsctl --bare --columns ofport find
> Interface name=$patch_port)
> + echo "$patch_port=$hv_patch_ofport"
> + test 1 -le $(as $hv ovs-ofctl dump-flows br-int | grep -c
> "output:\b$hv_patch_ofport\b")
> + ])
> + done
> + done
> }
>
> ovn_wait_remote_output_flows () {
> @@ -2952,6 +2955,8 @@ for i in 1 2; do
> done
> done
> wait_for_ports_up
> +OVN_WAIT_PATCH_PORT_FLOWS(["ln1" "ln2" "ln3"], ["hv1"] ["hv2"])
> +
> ovn-nbctl --wait=sb sync
> ovn-sbctl dump-flows > sbflows
> AT_CAPTURE_FILE([sbflows])
> @@ -3122,6 +3127,7 @@ done
>
> wait_for_ports_up
> ovn-nbctl --wait=sb sync
> +OVN_WAIT_PATCH_PORT_FLOWS(["ln-11" "ln-21"], ["hv-1" "hv-2"])
>
> ovn-sbctl dump-flows > sbflows
> AT_CAPTURE_FILE([sbflows])
> @@ -3394,6 +3400,7 @@ for i in 1 2; do
> ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:00:0$i
>
> OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup])
> +OVN_WAIT_PATCH_PORT_FLOWS(["ln-$i-20"], ["hv-$i"])
> done
>
> wait_for_ports_up
> @@ -3501,7 +3508,7 @@ done
>
> wait_for_ports_up
>
> -# Remote output flows are setup whe pb of remote is received
> +# Remote output flows are setup when pb of remote is received
> # Hence they can be setup after both ports have been reported up.
> OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv-1"],["hv-2"])
> OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv-2"],["hv-1"])
> @@ -3641,7 +3648,7 @@ for i in 1 2; do
>
> # Patch port might be created after ports are reported up
> # Wait for a flow outputing to patch port
> -OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-100"], ["hv-$i"])
> +OVN_WAIT_PATCH_PORT_FLOWS(["ln-100"], ["hv-$i"])
> done
>
> test_packet() {
> @@ -3710,7 +3717,7 @@ for i in 1 2; do
>
> # Patch port might be created after ports are reported up
> # Wait for a flow outputing to patch port
> -OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-100"], ["hv-$i"])
> +OVN_WAIT_PATCH_PORT_FLOWS(["ln-100"], ["hv-$i"])
> done
> # create taps on fabric to check vlan encapsulation there
> for i in 1 2; do
> @@ -3796,7 +3803,7 @@ for i in 1 2; do
>
> # Patch port might be created after ports are reported up
> # Wait for a flow outputing to patch port
> -OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln"], ["hv-$i"])
> +OVN_WAIT_PATCH_PORT_FLOWS(["ln"], ["h
Re: [ovs-dev] [PATCH] ovsdb: Remove extra make target dependency for local-config.5.
On 7/18/22 13:07, Dumitru Ceara wrote:
> On 7/15/22 18:04, Ilya Maximets wrote:
>> ovsdb/ directory should not be a dependency, otherwise the man
>> page is getting re-built every time unrelated files are changed.
>>
>> Fixes: 6f24c2bc769a ("ovsdb: Add Local_Config schema.")
>> Signed-off-by: Ilya Maximets
>> ---
>
> Acked-by: Dumitru Ceara
Thanks! Applied and backported down to 2.17.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ci: Prefer pip3 to install unit test dependencies
On 7/18/22 13:30, David Marchand wrote:
> While it looks like the right python3 versions of those dependencies
> seems to be installed in the CI, prefer calling this via pip3 like the
> rest of the script.
>
> Fixes: 445dceb88461 ("python: Introduce unit tests.")
> Signed-off-by: David Marchand
> ---
> .ci/linux-prepare.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
> index 1698a0713b..16a7aec0b5 100755
> --- a/.ci/linux-prepare.sh
> +++ b/.ci/linux-prepare.sh
> @@ -43,7 +43,7 @@ if [ "$M32" ]; then
> fi
>
> # Install python test dependencies
> -pip install -r python/test_requirements.txt
> +pip3 install -r python/test_requirements.txt
>
> # IPv6 is supported by kernel but disabled in TravisCI images:
> # https://github.com/travis-ci/travis-ci/issues/8891
Thanks! Applied to master and branch-3.0.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v5] controller: Add delay after multicast ARP packet
On Thu, Jul 14, 2022 at 6:42 AM Dumitru Ceara wrote:
>
> On 7/14/22 12:37, Ales Musil wrote:
> > The ovn-controller had a race condition over MAC
> > binding table with other controllers. When multiple
> > controllers received GARP from single source usually
> > the one who was able to win the race put it into SB.
> > The others got transaction error which triggered
> > full recompute even if it's not needed.
> >
> > In order to reduce the chance of multiple controllers
> > trying to insert same row at the same time add slight
> > delay to the MAC binding processing. This delay
> > is random interval between 1-50 in ms. This greatly
> > reduces the chance that multiple controllers will try
> > to add MAC binding at exactly the same time. This applies
> > only to multicast ARP request which applies only to GARP
> > that is sent to broadcast address.
> >
> > Local testing with this delay vs without show significantly
> > reduced chance of hitting the transaction error.
> >
> > During the testing 10 GARPs was sent to two controllers
> > at the same time. Without proposed fix at least one controller
> > had multiple transaction errors present every test iteration.
> >
> > With the proposed fix the transaction error was reduced to a
> > single one when it happened which was usually in less than
> > 10% of the iterations.
> >
> > As was mentioned before the race condition can still happen,
> > but the chance is greatly reduced.
> >
> > Suggested-by: Daniel Alvarez Sanchez
> > Signed-off-by: Ales Musil
> > ---
> > v3: Rebase on top of current main.
> > Change the delay to be per ARP request instead of
> > single delay for everything. This allows the possibility
> > to delay only multicast/broadcast AP as suggested by Han.
> > v4,v5: Address comments from Dumitru.
> > ---
>
> Thanks for this new revision!
>
> I have two very small comments but I don't think you need to send a v6
> for this, let's see if the maintainers require that or if the
> suggestions can be incorporated when the patch is accepted.
>
> Acked-by: Dumitru Ceara
Thanks. I applied this patch to the main branch. Unfortunately I
missed addressing
Dumitru's comments before applying. If you can send a follow up patch
that would be great.
Numan
>
> > controller/mac-learn.c | 41 +
> > controller/mac-learn.h | 9 +++--
> > controller/pinctrl.c | 27 ++-
> > tests/ovn.at | 6 +++---
> > 4 files changed, 61 insertions(+), 22 deletions(-)
> >
> > diff --git a/controller/mac-learn.c b/controller/mac-learn.c
> > index 27634dca8..a27607016 100644
> > --- a/controller/mac-learn.c
> > +++ b/controller/mac-learn.c
> > @@ -18,14 +18,18 @@
> > #include "mac-learn.h"
> >
> > /* OpenvSwitch lib includes. */
> > +#include "openvswitch/poll-loop.h"
> > #include "openvswitch/vlog.h"
> > #include "lib/packets.h"
> > +#include "lib/random.h"
> > #include "lib/smap.h"
> > +#include "lib/timeval.h"
> >
> > VLOG_DEFINE_THIS_MODULE(mac_learn);
> >
> > #define MAX_MAC_BINDINGS 1000
> > #define MAX_FDB_ENTRIES 1000
> > +#define MAX_MAC_BINDING_DELAY_MSEC 50
> >
> > static size_t mac_binding_hash(uint32_t dp_key, uint32_t port_key,
> > struct in6_addr *);
> > @@ -46,25 +50,19 @@ ovn_mac_bindings_init(struct hmap *mac_bindings)
> > }
> >
> > void
> > -ovn_mac_bindings_flush(struct hmap *mac_bindings)
> > +ovn_mac_bindings_destroy(struct hmap *mac_bindings)
> > {
> > struct mac_binding *mb;
> > HMAP_FOR_EACH_POP (mb, hmap_node, mac_bindings) {
> > free(mb);
> > }
> > -}
> > -
> > -void
> > -ovn_mac_bindings_destroy(struct hmap *mac_bindings)
> > -{
> > -ovn_mac_bindings_flush(mac_bindings);
> > hmap_destroy(mac_bindings);
> > }
> >
> > struct mac_binding *
> > ovn_mac_binding_add(struct hmap *mac_bindings, uint32_t dp_key,
> > uint32_t port_key, struct in6_addr *ip,
> > -struct eth_addr mac)
> > +struct eth_addr mac, bool is_unicast)
> > {
> > uint32_t hash = mac_binding_hash(dp_key, port_key, ip);
> >
> > @@ -75,10 +73,13 @@ ovn_mac_binding_add(struct hmap *mac_bindings, uint32_t
> > dp_key,
> > return NULL;
> > }
> >
> > +uint32_t delay = is_unicast
> > +? 0 : random_range(MAX_MAC_BINDING_DELAY_MSEC) + 1;
>
> Nit: the coding-style doesn't seem to be extremely specific but the way
> I read it this should be indented a bit differently:
>
> uint32_t delay = is_unicast
> ? 0 : random_range(MAX_MAC_BINDING_DELAY_MSEC) + 1;
>
> Or maybe cleaner:
>
> uint32_t delay = !is_unicast
> ? random_range(MAX_MAC_BINDING_DELAY_MSEC) + 1
> : 0;
>
> > mb = xmalloc(sizeof *mb);
> > mb->dp_key = dp_key;
> > mb->port_key = port_key;
> > mb->ip = *ip;
> > +mb->commit_at_ms = time_msec() + delay;
> > hmap_ins
Re: [ovs-dev] [PATCH ovn] pinctrl: fix ovn-controller abort when service start.
On Fri, Jul 15, 2022 at 8:35 AM wangchuanlei wrote: > > Hi, > when start service ovn-controller, the br-int may be not connected, in > case, swconn->version = -1, > on my enviroment, i have loadlalancer, in the process of sending health > check packet, ofp_proto is 0, which will leads controller serivice to abort > in ofputil_encode_packet_out. > So this patch is to avoid this problem. > > Signed-off-by: wangchuanlei Thanks for the patch. I applied this patch to the main branch and backported to branch-22.06. I will be backporting to branch-22.03 soon (running tests). Before applying I modified the commit message a bit and also added your name to the AUTHORS.rst file. Num > --- > controller/pinctrl.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index f954362b7..2fcf91bc9 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -3454,11 +3454,11 @@ pinctrl_handler(void *arg_) > > ip_mcast_querier_run(swconn, &send_mcast_query_time); > } > -} > > -ovs_mutex_lock(&pinctrl_mutex); > -svc_monitors_run(swconn, &svc_monitors_next_run_time); > -ovs_mutex_unlock(&pinctrl_mutex); > +ovs_mutex_lock(&pinctrl_mutex); > +svc_monitors_run(swconn, &svc_monitors_next_run_time); > +ovs_mutex_unlock(&pinctrl_mutex); > +} > > rconn_run_wait(swconn); > rconn_recv_wait(swconn); > -- > 2.27.0 > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] pinctrl: del redundant row.
On Fri, Jul 15, 2022 at 8:33 AM wangchuanlei wrote:
>
> The row of code is useless, need to be deleted.
>
> Signed-off-by: wangchuanlei
Thanks. I applied this patch to the main branch.
Numan
> ---
> controller/pinctrl.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index f954362b7..80ae9f01d 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -7527,7 +7527,6 @@ svc_monitors_run(struct rconn *swconn,
> svc_mon->next_send_time = current_time + svc_mon->interval;
> next_run_time = svc_mon->next_send_time;
> } else {
> -next_run_time = svc_mon->wait_time - current_time;
> next_run_time = svc_mon->wait_time;
> }
> break;
> --
> 2.27.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 ovn] lflow: fix possible use-after-free in add_lb_vip_hairpin_reply_action
On Mon, Jul 18, 2022 at 6:48 AM Dumitru Ceara wrote:
>
> On 7/16/22 08:45, Lorenzo Bianconi wrote:
> > ofpbuf_put_zeros routine can rellocate the buffer if the requested size
> > is bigger than buffer tailroom. Reload ol pointer before running
> > ofpact_finish_LEARN in order to avoid a possible use-after-free in
> > add_lb_vip_hairpin_reply_action routine.
> >
> > Fixes: 022ea339c8 ("lflow: Use learn() action to generate LB hairpin reply
> > flows.")
> > Signed-off-by: Lorenzo Bianconi
> > ---
> > Changes since v1:
> > - rely on ofpbuf_at_assert in order to not overwrite possible former actions
> > ---
>
> Looks good to me, thanks!
>
> Acked-by: Dumitru Ceara
>
Thanks. I applied this patch to the main branch and backported to
branch-22.06. I will be backporting to branch-22.03 soon (running
tests).
Numan
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-trace: implement chk_lb_hairpin_reply and ct_snat_to_vip actions
On Fri, Jul 1, 2022 at 1:41 PM Mark Michelson wrote:
>
> Thanks for this Lorenzo,
>
> Acked-by: Mark Michelson
Thanks. I applied this patch to the main branch.
Numan
>
> On 6/30/22 09:22, Lorenzo Bianconi wrote:
> > Add chk_lb_hairpin_reply and ct_snat_to_vip implementations for ovn-trace
> > utility in order to simulate hairpin traffic
> >
> > Rreported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2099311
> > Signed-off-by: Lorenzo Bianconi
> > ---
> > tests/ovn-northd.at | 8 ---
> > utilities/ovn-trace.c | 126 --
> > 2 files changed, 97 insertions(+), 37 deletions(-)
> >
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index fe97bedad..2f293bfc8 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -2936,7 +2936,6 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new
> > --minimal ls "${flow}"], [0], [dn
> > ct_lb_mark {
> > ct_lb_mark {
> > reg0[[6]] = 0;
> > -*** chk_lb_hairpin_reply action not implemented;
> > reg0[[12]] = 0;
> > ct_lb_mark /* default (use --ct to customize) */ {
> > output("lsp2");
> > @@ -2952,7 +2951,6 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new
> > --minimal ls "${flow}"], [0], [dn
> > ct_lb_mark {
> > ct_lb_mark {
> > reg0[[6]] = 0;
> > -*** chk_lb_hairpin_reply action not implemented;
> > reg0[[12]] = 0;
> > ct_lb_mark /* default (use --ct to customize) */ {
> > output("lsp2");
> > @@ -2974,7 +2972,6 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new
> > --minimal ls "${flow}"], [0], [dn
> > ct_lb_mark {
> > ct_lb_mark {
> > reg0[[6]] = 0;
> > -*** chk_lb_hairpin_reply action not implemented;
> > reg0[[12]] = 0;
> > ct_lb_mark /* default (use --ct to customize) */ {
> > output("lsp2");
> > @@ -2990,7 +2987,6 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new
> > --minimal ls "${flow}"], [0], [dn
> > ct_lb_mark {
> > ct_lb_mark {
> > reg0[[6]] = 0;
> > -*** chk_lb_hairpin_reply action not implemented;
> > reg0[[12]] = 0;
> > ct_lb_mark /* default (use --ct to customize) */ {
> > output("lsp2");
> > @@ -3092,7 +3088,6 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new
> > --minimal ls "${flow}"], [0], [dn
> > ct_lb_mark {
> > ct_lb_mark {
> > reg0[[6]] = 0;
> > -*** chk_lb_hairpin_reply action not implemented;
> > reg0[[12]] = 0;
> > ct_lb_mark /* default (use --ct to customize) */ {
> > output("lsp2");
> > @@ -3108,7 +3103,6 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new
> > --minimal ls "${flow}"], [0], [dn
> > ct_lb_mark {
> > ct_lb_mark {
> > reg0[[6]] = 0;
> > -*** chk_lb_hairpin_reply action not implemented;
> > reg0[[12]] = 0;
> > ct_lb_mark /* default (use --ct to customize) */ {
> > output("lsp2");
> > @@ -3130,7 +3124,6 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new
> > --minimal ls "${flow}"], [0], [dn
> > ct_lb_mark {
> > ct_lb_mark {
> > reg0[[6]] = 0;
> > -*** chk_lb_hairpin_reply action not implemented;
> > reg0[[12]] = 0;
> > ct_lb_mark /* default (use --ct to customize) */ {
> > output("lsp2");
> > @@ -3146,7 +3139,6 @@ AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new
> > --minimal ls "${flow}"], [0], [dn
> > ct_lb_mark {
> > ct_lb_mark {
> > reg0[[6]] = 0;
> > -*** chk_lb_hairpin_reply action not implemented;
> > reg0[[12]] = 0;
> > ct_lb_mark /* default (use --ct to customize) */ {
> > output("lsp2");
> > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> > index c4110de0a..baf489202 100644
> > --- a/utilities/ovn-trace.c
> > +++ b/utilities/ovn-trace.c
> > @@ -2488,10 +2488,13 @@ execute_ct_lb(const struct ovnact_ct_lb *ct_lb,
> >
> > if (dst) {
> > if (family == AF_INET6) {
> > +ct_lb_flow.ct_ipv6_dst = ct_lb_flow.ipv6_dst;
> > ct_lb_flow.ipv6_dst = dst->ipv6;
> > } else {
> > +ct_lb_flow.ct_nw_dst = ct_lb_flow.nw_dst;
> > ct_lb_flow.nw_dst = dst->ipv4;
> > }
> > +ct_lb_flow.ct_tp_dst = ct_lb_flow.tp_dst;
> > if (dst->port) {
> > ct_lb_flow.tp_dst = htons(dst->port);
> > }
> > @@ -2588,15 +2591,65 @@ execute_ovnfield_load(const struct ovnact_load
> > *load,
> > }
> > }
> >
> > +static bool
> > +get_lb_vip_data(struct flow *uflow, struct in6_addr *vip,
> > +char **vip_str, uint16_t *port)
> > +{
> > +int family;
> > +
> > +const struct sbrec_load_balancer *sbdb;
> > +SBREC_LOAD_BALANCER_FOR_EACH (sbdb, ovnsb_idl) {
> > +struct smap_node *node;
> > +SM
Re: [ovs-dev] [PATCH ovn] ovn-ic: do not learn routes with link-local next-hops
On Tue, Jul 12, 2022 at 2:25 PM Mark Michelson wrote:
>
> Acked-by: Mark Michelson
Thanks. I applied this patch to the main branch and backported to
branch-22.06. I will be backporting to branch-22.03 soon (running
tests).
Numan
>
> On 6/24/22 05:41, Lorenzo Bianconi wrote:
> > Do not learn IPv6 routes with link-local nex-thop. This issue occurs
> > when the lrp connected to the transit switch has no IPv6 addresses and
> > the internal logical router port has a valid IPv6 one.
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2100355
> > Signed-off-by: Lorenzo Bianconi
> > ---
> > ic/ovn-ic.c | 7 ++-
> > tests/ovn-ic.at | 50 +
> > 2 files changed, 56 insertions(+), 1 deletion(-)
> >
> > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> > index 8511cb9ac..95a5ff0de 100644
> > --- a/ic/ovn-ic.c
> > +++ b/ic/ovn-ic.c
> > @@ -925,7 +925,12 @@ parse_route(const char *s_prefix, const char
> > *s_nexthop,
> > }
> >
> > unsigned int nlen;
> > -return ip46_parse_cidr(s_nexthop, nexthop, &nlen);
> > +if (!ip46_parse_cidr(s_nexthop, nexthop, &nlen)) {
> > +return false;
> > +}
> > +
> > +/* Do not learn routes with link-local next hop. */
> > +return !in6_is_lla(nexthop);
> > }
> >
> > /* Return false if can't be added due to bad format. */
> > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> > index 05bd3e9a6..89f223562 100644
> > --- a/tests/ovn-ic.at
> > +++ b/tests/ovn-ic.at
> > @@ -492,6 +492,56 @@ OVN_CLEANUP_IC([az1], [az2])
> > AT_CLEANUP
> > ])
> >
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([ovn-ic -- route sync -- IPv6 route tables])
> > +AT_KEYWORDS([IPv6-route-sync])
> > +
> > +ovn_init_ic_db
> > +ovn-ic-nbctl ts-add ts1
> > +
> > +for i in 1 2; do
> > +ovn_start az$i
> > +ovn_as az$i
> > +
> > +# Enable route learning at AZ level
> > +ovn-nbctl set nb_global . options:ic-route-learn=true
> > +# Enable route advertising at AZ level
> > +ovn-nbctl set nb_global . options:ic-route-adv=true
> > +
> > +# Create LRP and connect to TS
> > +ovn-nbctl lr-add lr$i
> > +ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i
> > 2001:db8:1::$i/64
> > +ovn-nbctl lsp-add ts1 lsp-ts1-lr$i \
> > +-- lsp-set-addresses lsp-ts1-lr$i router \
> > +-- lsp-set-type lsp-ts1-lr$i router \
> > +-- lsp-set-options lsp-ts1-lr$i router-port=lrp-lr$i-ts1
> > +
> > +ovn-nbctl lrp-add lr$i lrp-lr$i-p$i 00:00:00:00:00:0$i
> > 2002:db8:1::$i/64
> > +done
> > +
> > +for i in 1 2; do
> > +OVS_WAIT_UNTIL([ovn_as az$i ovn-nbctl lr-route-list lr$i | grep
> > learned])
> > +done
> > +
> > +AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1 | awk '/learned/{print
> > $1, $2}'], [0], [dnl
> > +2002:db8:1::/64 2001:db8:1::2
> > +])
> > +
> > +# Do not learn routes from link-local nexthops
> > +for i in 1 2; do
> > +ovn_as az$i
> > +ovn-nbctl lrp-del lrp-lr$i-ts1
> > +ovn-nbctl lrp-add lr$i lrp-lr$i-ts1 aa:aa:aa:aa:aa:0$i
> > 169.254.100.$i/24
> > +done
> > +
> > +OVS_WAIT_WHILE([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep learned])
> > +AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep -q learned], [1])
> > +
> > +OVN_CLEANUP_IC([az1], [az2])
> > +
> > +AT_CLEANUP
> > +])
> > +
> > OVN_FOR_EACH_NORTHD([
> > AT_SETUP([ovn-ic -- route sync -- route tables])
> >
> >
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 4/4] multicast: Properly flood IGMP queries and reports.
On Thu, Jul 7, 2022 at 6:27 AM Lucas Alvares Gomes wrote: > > On Thu, Jul 7, 2022 at 12:24 PM Lucas Alvares Gomes > wrote: > > > > Hi, > > > > I tested this series of patches with OpenStack upstream [0] since we > > have some automated tests for IGMP and it seems good: > > > > test_multicast_between_vms_on_same_network[id-113486fc-24c9-4be4-8361-03b1c9892867] > > pass (link: > > https://98794ab76263ee253bc7-6b6bf6aa2495d781bdba3c8c61916451.ssl.cf1.rackcdn.com/848934/2/check/neutron-tempest-plugin-ovn/b3191bf/testr_results.html, > > this will expire at some point) > > > > What I did was to change ML2/OVN to set mcast_flood to False on the > > LSPs, apply the series of patches in OVN and then run the tests. > > > > Tested-By: > > https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/848934/ > > > > Tested-By: Lucas Alvares Gomes Acked-by: Numan Siddique It would be great if someone else also reviews this patch. Numan > > > [0] https://review.opendev.org/q/topic:test-igmp > > > > Cheers, > > Lucas > > > > On Tue, Jul 5, 2022 at 1:13 PM Dumitru Ceara wrote: > > > > > > RFC 4541 "Considerations for Internet Group Management Protocol (IGMP) > > > and Multicast Listener Discovery (MLD) Snooping Switches" [0] states: > > > > > > For IGMP packets: > > > 2.1.1. IGMP Forwarding Rules > > >1) A snooping switch should forward IGMP Membership Reports only to > > > those ports where multicast routers are attached. > > > > > > Alternatively stated: a snooping switch should not forward IGMP > > > Membership Reports to ports on which only hosts are attached. > > > An > > > administrative control may be provided to override this > > > restriction, allowing the report messages to be flooded to other > > > ports. > > > > > > This is the main IGMP snooping functionality for the control > > > path. > > > > > > Sending membership reports to other hosts can result, for IGMPv1 > > > and IGMPv2, in unintentionally preventing a host from joining a > > > specific multicast group. > > > > > > When an IGMPv1 or IGMPv2 host receives a membership report for a > > > group address that it intends to join, the host will suppress > > > its > > > own membership report for the same group. This join or message > > > suppression is a requirement for IGMPv1 and IGMPv2 hosts. > > > However, if a switch does not receive a membership report from > > > the > > > host it will not forward multicast data to it. > > > > > > In OVN this translates to forwarding reports only on: > > > a. ports where mrouters have been learned (IGMP queries were received on > > >those ports) > > > b. ports connecting a multicast enabled logical switch to a multicast > > >relay enabled logical router (OVN mrouter) > > > c. ports configured with mcast_flood_reports=true > > > > > > 2.1.2. Data Forwarding Rules > > > > > >1) Packets with a destination IP address outside 224.0.0.X which > > > are > > > not IGMP should be forwarded according to group-based port > > > membership tables and must also be forwarded on router ports. > > > > > > In OVN this translates to forwarding traffic for a group G to: > > > a. all ports where G was learned > > > b. all ports where an (external or OVN) mrouter was learned. > > > c. all ports configured with mcast_flood=true > > > > > > IGMP queries are already snooped by ovn-controller. Just propagate the > > > information about where mrouters are to the Southbound DB through means > > > of a custom IGMP_Group (name="mrouters"). > > > > > > Snooped queries are then re-injected in the OVS pipeline with outport > > > set to MC_FLOOD_L2 (only flood them in the L2 domain). > > > > > > Snooped reports are re-injected in the OVS pipeline with outport set to > > > MC_MROUTER_FLOOD (flood them to snooped mrouters and OVN mrouters). > > > > > > The same behavior applies to IPv6 too (MLD). > > > > > > [0] https://datatracker.ietf.org/doc/html/rfc4541 > > > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1933990 > > > Reported-at: https://github.com/ovn-org/ovn/issues/126 > > > Signed-off-by: Dumitru Ceara > > > --- > > > controller/ip-mcast.c | 129 - > > > controller/ip-mcast.h | 14 > > > controller/pinctrl.c| 129 + > > > lib/ip-mcast-index.h|5 + > > > lib/mcast-group-index.h |4 - > > > northd/northd.c | 54 +++ > > > northd/ovn-northd.8.xml |6 -- > > > tests/ovn-macros.at | 25 +++ > > > tests/ovn.at| 164 > > > ++- > > > 9 files changed, 472 insertions(+), 58 deletions(-) > > > > > > diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c > > > index 9b0b4465a..a870fb29e 100644 > > > --- a/controller
Re: [ovs-dev] [PATCH ovn 3/4] tests: Factor out reset_pcap_file() helper.
On Tue, Jul 5, 2022 at 7:11 AM Dumitru Ceara wrote:
>
> Signed-off-by: Dumitru Ceara
Thanks for this patch.
The below test cases is failing with this patch
347. ovn.at:14447: 347. localnet connectivity with multiple
requested-chassis -- ovn-northd -- parallelization=yes --
ovn_monitor_all=yes (ovn.at:14447): FAILED (--)
348. ovn.at:14447: 348. localnet connectivity with multiple
requested-chassis -- ovn-northd -- parallelization=yes --
ovn_monitor_all=no (ovn.at:14447): FAILED (--)
349. ovn.at:14447: 349. localnet connectivity with multiple
requested-chassis -- ovn-northd -- parallelization=no --
ovn_monitor_all=yes (ovn.at:14447): FAILED (--)
+ovs-vsctl: no row "hv1" in table Interface
../../tests/ovn-macros.at:384: exit code was 1, expected 0
350. ovn.at:14447: 350. localnet connectivity with multiple
requested-chassis -- ovn-northd -- parallelization=no --
ovn_monitor_all=no (ovn.at:14447): FAILED (--)
Can you please take a look.
Numan
> ---
> tests/ovn-macros.at | 20 +++
> tests/ovn.at| 358
> +++
> 2 files changed, 43 insertions(+), 335 deletions(-)
>
> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
> index 335f9158c..77e89f6b4 100644
> --- a/tests/ovn-macros.at
> +++ b/tests/ovn-macros.at
> @@ -508,9 +508,13 @@ wait_for_ports_up() {
> fi
> }
>
> -# reset_pcap_file iface pcap_file
> +# reset_iface_pcap_file iface pcap_file
> # Resets the pcap file associates with OVS interface. should be used
> # with dummy datapath.
> +#
> +# XXX: This should actually replace reset_pcap_file() as they do almost
> +# exactly the same thing but the "wait while the pcap file has the size
> +# of the PCAP header" check causes tests to fail.
> reset_iface_pcap_file() {
> local iface=$1
> local pcap_file=$2
> @@ -525,6 +529,20 @@ options:rxq_pcap=${pcap_file}-rx.pcap
> OVS_WAIT_WHILE([test 24 = $(wc -c ${pcap_file}-tx.pcap | cut -d " "
> -f1)])
> }
>
> +# reset_pcap_file iface pcap_file
> +# Resets the pcap file associates with OVS interface. should be used
> +# with dummy datapath.
> +reset_pcap_file() {
> +local iface=$1
> +local pcap_file=$2
> +check rm -f dummy-*.pcap
> +check ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
> +options:rxq_pcap=dummy-rx.pcap
> +check rm -f ${pcap_file}*.pcap
> +check ovs-vsctl -- set Interface $iface
> options:tx_pcap=${pcap_file}-tx.pcap \
> +options:rxq_pcap=${pcap_file}-rx.pcap
> +}
> +
> # Receive a packet on a dummy netdev interface. If we expect packets to be
> # recorded, then wait until the pcap file reflects the change.
> netdev_dummy_receive() {
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 0e868ae66..9512fd033 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -6479,16 +6479,6 @@ compare_dhcp_packets() {
> fi
> }
>
> -reset_pcap_file() {
> -local iface=$1
> -local pcap_file=$2
> -check ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
> -options:rxq_pcap=dummy-rx.pcap
> -rm -f ${pcap_file}*.pcap
> -check ovs-vsctl -- set Interface $iface
> options:tx_pcap=${pcap_file}-tx.pcap \
> -options:rxq_pcap=${pcap_file}-rx.pcap
> -}
> -
> AT_CAPTURE_FILE([sbflows])
> ovn-sbctl dump-flows > sbflows
>
> @@ -7074,16 +7064,6 @@ test_dhcpv6() {
> as hv1 ovs-appctl netdev-dummy/receive hv1-vif$inport $request
> }
>
> -reset_pcap_file() {
> -local iface=$1
> -local pcap_file=$2
> -ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
> -options:rxq_pcap=dummy-rx.pcap
> -rm -f ${pcap_file}*.pcap
> -ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
> -options:rxq_pcap=${pcap_file}-rx.pcap
> -}
> -
> AT_CAPTURE_FILE([ofctl_monitor0.log])
> as hv1 ovs-ofctl monitor br-int resume --detach --no-chdir \
> --pidfile=ovs-ofctl0.pid 2> ofctl_monitor0.log
> @@ -8781,16 +8761,6 @@ OVS_WAIT_UNTIL([
> ])
> ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0
> nat-addresses="f0:00:00:00:00:03 192.168.0.3"
>
> -reset_pcap_file() {
> -local iface=$1
> -local pcap_file=$2
> -ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
> -options:rxq_pcap=dummy-rx.pcap
> -rm -f ${pcap_file}*.pcap
> -ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
> -options:rxq_pcap=${pcap_file}-rx.pcap
> -}
> -
> reset_pcap_file snoopvif hv1/snoopvif
> OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 140])
>
> @@ -8865,20 +8835,8 @@ grep "Port patch-br-int-to-ln_port" | wc -l`])
> # Temporarily remove lr0 chassis
> AT_CHECK([ovn-nbctl remove logical_router lr0 options chassis])
>
> -reset_pcap_file() {
> -local hv=$1
> -local iface=$2
> -local pcap_file=$3
> -as $hv
> -ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
> -options:rxq_pcap=dummy-rx.pcap
> -rm -f ${pcap_file}*.pcap
> -ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
Re: [ovs-dev] [PATCH ovn 2/4] tests: Add 'IP-multicast' keyword to all relevant tests.
On Tue, Jul 5, 2022 at 7:09 AM Dumitru Ceara wrote: > > Signed-off-by: Dumitru Ceara Thanks. I applied this patch to the main branch. Numan > --- > tests/ovn.at|7 --- > tests/system-ovn.at |2 +- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/tests/ovn.at b/tests/ovn.at > index 6e35bb5b8..0e868ae66 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -20931,7 +20931,7 @@ AT_CLEANUP > > OVN_FOR_EACH_NORTHD([ > AT_SETUP([IGMP snoop/querier/relay]) > -AT_KEYWORDS([snoop query querier relay]) > +AT_KEYWORDS([IP-multicast snoop query querier relay]) > > ovn_start > > @@ -21524,7 +21524,7 @@ AT_CLEANUP > > OVN_FOR_EACH_NORTHD([ > AT_SETUP([IGMP relay - distributed gateway port]) > -AT_KEYWORDS([snoop relay]) > +AT_KEYWORDS([IP-multicast snoop relay]) > > ovn_start > > @@ -21673,7 +21673,7 @@ AT_CLEANUP > > OVN_FOR_EACH_NORTHD([ > AT_SETUP([MLD snoop/querier/relay]) > -AT_KEYWORDS([snoop query querier relay]) > +AT_KEYWORDS([IP-multicast snoop query querier relay]) > > ovn_start > > @@ -24664,6 +24664,7 @@ AT_CLEANUP > > OVN_FOR_EACH_NORTHD([ > AT_SETUP([interconnection - IGMP/MLD multicast]) > +AT_KEYWORDS([IP-multicast]) > > # Logical network: > # > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index df2da3408..8df26 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -4147,7 +4147,7 @@ AT_CLEANUP > OVN_FOR_EACH_NORTHD([ > AT_SETUP([2 LSs IGMP and MLD]) > AT_SKIP_IF([test $HAVE_TCPDUMP = no]) > -AT_KEYWORDS([ovnigmp]) > +AT_KEYWORDS([ovnigmp IP-multicast]) > > ovn_start > > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 1/4] multicast: Document reserved multicast groups.
On Tue, Jul 5, 2022 at 7:09 AM Dumitru Ceara wrote:
>
> Signed-off-by: Dumitru Ceara
Thanks. I applied this patch to the main branch.
Numan
> ---
> lib/mcast-group-index.h | 26 +-
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/lib/mcast-group-index.h b/lib/mcast-group-index.h
> index 5bc725451..e8d82b126 100644
> --- a/lib/mcast-group-index.h
> +++ b/lib/mcast-group-index.h
> @@ -28,11 +28,27 @@ struct sbrec_datapath_binding;
> enum ovn_mcast_tunnel_keys {
>
> OVN_MCAST_FLOOD_TUNNEL_KEY = OVN_MIN_MULTICAST,
> -OVN_MCAST_UNKNOWN_TUNNEL_KEY,
> -OVN_MCAST_MROUTER_FLOOD_TUNNEL_KEY,
> -OVN_MCAST_MROUTER_STATIC_TUNNEL_KEY,
> -OVN_MCAST_STATIC_TUNNEL_KEY,
> -OVN_MCAST_FLOOD_L2_TUNNEL_KEY,
> +OVN_MCAST_UNKNOWN_TUNNEL_KEY,/* For L2 unknown dest traffic. */
> +OVN_MCAST_MROUTER_FLOOD_TUNNEL_KEY, /* For L3 multicast traffic that
> must
> + * be relayed (multicast routed).
> + */
> +OVN_MCAST_MROUTER_STATIC_TUNNEL_KEY, /* For multicast reports that need
> to
> + * be forwarded statically towards
> + * mrouters.
> + */
> +OVN_MCAST_STATIC_TUNNEL_KEY, /* For switches:
> + * - for L3 multicast traffic that
> + * needs to be forwarded
> + * statically.
> + * For routers:
> + * - for L3 multicast traffic AND
> + * reports that need to be
> + * forwarded statically.
> + */
> +OVN_MCAST_FLOOD_L2_TUNNEL_KEY, /* Logical switch broadcast domain
> + * excluding ports towards logical
> + * routers.
> + */
> OVN_MIN_IP_MULTICAST,
> OVN_MAX_IP_MULTICAST = OVN_MAX_MULTICAST,
> };
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [OVN v3] OVN - Add Support for Remote Port Mirroring
On 6/22/22 20:36, Abhiram R N wrote: > Added changes in ovn-nbctl, ovn-sbctl, northd and in ovn-controller. > While Mirror creation just creates the mirror, the lsp-attach-mirror > triggers the sequence to create Mirror in OVS DB on compute node. > OVS already supports Port Mirroring. > > Note: This is targeted to mirror to destinations anywhere outside the > cluster where the analyser resides and it need not be an OVN node. > > Example commands are as below: > > Mirror creation > ovn-nbctl mirror-add mirror1 gre 0 from-lport 10.10.10.2 > > Attach a logical port to the mirror. > ovn-nbctl lsp-attach-mirror sw0-port1 mirror1 > > Detach a source from Mirror > ovn-nbctl lsp-detach-mirror sw0-port1 mirror1 > > Mirror deletion > ovn-nbctl mirror-del mirror1 > > Co-authored-by: Veda Barrenkala > Signed-off-by: Veda Barrenkala > Signed-off-by: Abhiram R N > --- Not a full review, I just wanted to highlight one issue with the patch. IIUC, after this change OVN will own all the mirrors configured for OVS. This will mean that ovn-controller will remove, for example, mirrors created by ovs-tcpdump or any other manually created mirrors. I don't think that's a good thing to do. The solution should probably mark mirrors created by OVN with some external-id and not touch mirrors that doesn't have it. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] net: openvswitch: Fix comment typo
The double `is' is duplicated in the comment, remove one. Signed-off-by: Jason Wang --- net/openvswitch/flow_netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c index 4c09cf8a0ab2..4a07ab094a84 100644 --- a/net/openvswitch/flow_netlink.c +++ b/net/openvswitch/flow_netlink.c @@ -3304,7 +3304,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr, /* Disallow subsequent L2.5+ set actions and mpls_pop * actions once the last MPLS label in the packet is -* is popped as there is no check here to ensure that +* popped as there is no check here to ensure that * the new eth type is valid and thus set actions could * write off the end of the packet or otherwise corrupt * it. -- 2.35.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] question on thread_pmd_main, locking, unguarded access to poll_list (sporadic ovs-ovswitchd segfault)
Hi all,
the context here is a sporadic ovs-ovswitchd segfault I am researching,
but I'd like to learn a bit about the locking of resources in ovs.
By studying the git history and reading the code,
including commits like:
commit eef85380810ab428a193f4874477a6dde5999ab7
Author: Ilya Maximets
Date: Mon Feb 11 20:34:06 2019 +0300
dpif-netdev: Fix unsafe access to pmd polling lists.
All accesses to 'pmd->poll_list' should be guarded by
'pmd->port_mutex'. Additionally fixed inappropriate usage
of 'HMAP_FOR_EACH_SAFE' (hmap doesn't change in a loop)
and dropped not needed local variable 'proc_cycles'.
CC: Nitin Katiyar
Fixes: 5bf84282482a ("Adding support for PMD auto load balancing")
Signed-off-by: Ilya Maximets
Acked-by: Kevin Traynor
Acked-by: Aaron Conole
And looking at the datastructure struct dp_netdev_pmd_thread,
It would seem to me that it is necessary to acquire port_mutex when operating
on the member of
struct dp_netdev_pmd_thread {
struct hmap poll_list OVS_GUARDED;
};
Instead in dpif-netdev.c I see that the pmd_thread_main() only acquires the
pmd->perf_states.stats_mutex before accessing the poll_list, and then does:
for (i = 0; i < poll_cnt; i++) {
if (!poll_list[i].rxq_enabled) {
continue;
}
if (poll_list[i].emc_enabled) {
atomic_read_relaxed(&pmd->dp->emc_insert_min,
&pmd->ctx.emc_insert_min);
} else {
pmd->ctx.emc_insert_min = 0;
}
process_packets =
dp_netdev_process_rxq_port(pmd, poll_list[i].rxq,
poll_list[i].port_no);
rx_packets += process_packets;
}
The issue I seem to be seeing in my case is that poll_list[i].rxq is actually
invalid, ie
poll_list[i].rxq->rx->netdev == NULL.
We encounter this by running the testpmd application.
I suspect I should be wrong, so could you give me an idea how the locking is
supposed to happen,
why is the port_mutex not acquired ?
Thanks,
Claudio
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] controller: fix typo in get_lport_type_str()
Signed-off-by: Vladislav Odintsov --- controller/binding.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/binding.c b/controller/binding.c index 9faef02dd..ba803ae3e 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -881,7 +881,7 @@ get_lport_type_str(enum en_lport_type lport_type) case LP_CHASSISREDIRECT: return "CHASSISREDIRECT"; case LP_L3GATEWAY: -return "L3GATEWAT"; +return "L3GATEWAY"; case LP_LOCALNET: return "PATCH"; case LP_LOCALPORT: -- 2.26.3 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC ovn] controller: send GARPs for LSPs in vlan-backed network scenario
On Thu, Jul 14, 2022 at 1:30 PM Dumitru Ceara wrote: > > On 7/14/22 17:35, Numan Siddique wrote: > > On Thu, Jul 14, 2022 at 3:36 AM Lorenzo Bianconi > > wrote: > >> > >>> On 7/13/22 18:07, Lorenzo Bianconi wrote: > > On 6/17/22 00:31, Lorenzo Bianconi wrote: > >> When using VLAN backed networks and OVN routers leveraging the > >> 'ovn-chassis-mac-mappings' option, the eth.src field is replaced by the > >> chassis mac address in order to not expose the router mac address from > >> different nodes and confuse the TOR switch. However doing so the TOR > >> switch is not able to learn the port/mac bindings for routed E/W > >> traffic > >> and it is force to always flood it. Fix this issue adding the > >> capability > >> to send GARP traffic for logical switch ports if the corresponding > >> logical > >> switch has the ovn-lsp-garp parameter set to true in the option column. > >> More into about the issue can be found here [0]. > >> > >> [0] > >> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-September/050678.html > >> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2087779 > >> > >> Signed-off-by: Lorenzo Bianconi > >> --- > > > > Hi Lorenzo, > > Hi Dumitru, > > Thanks for reviewing it :) > > > > > I have a few concerns with this approach: > > > > a. The CMS will have to set this option for all VMs on all logical > > switches which will enable periodic GARPs for all of them all the time. > > That seems like quite a lot of broadcast traffic in the fabric. > > > > b. There's no guarantee that the GARPs are sent in time to prevent the > > FDB timeouts on the ToR switches. At best we could make the interval > > configurable but I don't think this is way better either. > > > > c. This is not really introduced by your patch but we will be causing > > this more often now. With the topology: > > > > (HV1) VM1 -- LS1 --- LR -- LS2 -- VM2 (HV2) > > (VLAN-backed network) > > > > HV1 configured with chassis mac mapping HV1-MAC > > HV2 configured with chassis mac mapping HV2-MAC > > > > We're leaking MAC addresses from LS1's broadcast domain (VM1-MAC) and > > from LS2's broadcast domain (VM2-MAC) into the fabric. I'm not sure > > that's OK. > > > > I'm not sure I understood what you mean here. > > Let's say we have 2 VMs - VM1 and VM3 connected to the same VLAN > > logical switch LS1 > > and if VM1 is on chassis HV1 and VM3 is on chassis HV3, the fabric > > will learn VM1's and VM3's mac > > when they both communicate right ? Please note that logical switch > > LS1 is a vlan tenant network with > > a localnet port (so no geneve tunnels are used for the traffic from VM1 to > > VM3). > > > > > > You're right, I misread the logical/physical topologies. The MACs will > only be visible in the VLANs corresponding to the localnet port of each > of the logical switches. Please ignore this part. > > > > > I think a proper solution is to change how we run the logical pipelines > > in case of vlan-backed networks. We currently have an assymetry: > > > > For packets flowing from VM1 to VM2 we execute: > > - on HV1: LS1-ingress, LS1-egress, LR-ingress, LR-egress, LS2-ingress > > - on HV2: LS2-egress > > > > For packets flowing from VM2 to VM1 we execute: > > - on HV2: LS2-ingress, LS2-egress, LR-ingress, LR-egress, LS1-ingress > > - on HV1: LS1-egress > > > > What if we change this to: > > VM1 -> VM2: > > - on HV1: LS1-ingress, LS1-egress, LR-ingress > > - on HV2: LR-egress, LS2-ingress, LS2-egress > > > > VM2 -> VM1: > > - on HV2: LS2-ingress, LS2-egress, LR-ingress > > - on HV2: LR-egress, LS1-ingress, LS1-egress > > > > > I'm not sure how easy it would be to run the pipeline as you suggested > > when ovn-chassis-mac-mappings is configured. > > Indeed it doesn't seem possible as we have no knowledge about what > happened prior to the packet being received on the vlan, it could've > been already processed by an ovn node. > > > However OVN does support centralized routing if a logical router has a > > distributed gateway port (connecting to the provider network) > > and the other logical router ports connecting to the tenant VLAN > > networks have the option - reside-on-redirect-chassis configured. > > > > So in a way CMS can use the centralized routing to avoid this issue. > > However if we want to fix this issue for distributed routing, looks > > to me > > the proposed RFC patch is the only way to fix it. And with the proper > > documentation CMS can know the benefits and drawbacks of both the > > options. > > > > I still think that's potentially a lot of broadcast traffic. If you compare this GARP broadcast traffic with the present flooding done by the upstream switch for every traffic, that's still
[ovs-dev] [PATCH ovn] northd: add support to make l3dgw ports fully distributed
This is used when traffic from HW VTEP goes to
routable networks and logical switch to which VTEP
logical port is attached also needs to support
distributed routing features such as NAT and others.
Signed-off-by: Vladislav Odintsov
---
northd/northd.c | 8 +++-
ovn-nb.xml | 10 ++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/northd/northd.c b/northd/northd.c
index d31cb1688..0be45e22a 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -1674,6 +1674,12 @@ is_cr_port(const struct ovn_port *op)
return op->l3dgw_port;
}
+static bool
+is_distributed(const struct ovn_port *op)
+{
+return smap_get_bool(&op->nbrp->options, "distributed", false);
+}
+
static void
destroy_routable_addresses(struct ovn_port_routable_addresses *ra)
{
@@ -10920,7 +10926,7 @@ build_adm_ctrl_flows_for_lrouter_port(
ds_clear(match);
ds_put_format(match, "eth.dst == %s && inport == %s",
op->lrp_networks.ea_s, op->json_key);
-if (is_l3dgw_port(op)) {
+if (is_l3dgw_port(op) && !is_distributed(op)) {
/* Traffic with eth.dst = l3dgw_port->lrp_networks.ea_s
* should only be received on the gateway chassis. */
ds_put_format(match, " && is_chassis_resident(%s)",
diff --git a/ovn-nb.xml b/ovn-nb.xml
index e26afd83c..d4b454791 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -2976,6 +2976,16 @@
option.
+
+
+If true, indicates, that flow for current LRP in
+lr_in_admission must be installed on all chassis, even if it has
+associated chassisredirect port.
+Usable when traffic from HW VTEP goes to routable networks and logical
+switch to which VTEP logical port is attached also needs to support
+distributed routing features such as NAT, Load Balancing and others.
+Empty value (default) means false.
+
--
2.26.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] northd: handle virtual lport type update
ovn-northd re-create sbrec row for lport of type
virtual when the type explicitly updated in NBDB.
This approach was applied to handle container lport
type updates and avoid handling issues in the ovn-controller,
this patch expanded that approach to handle lport of type
virtual in the same way.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2099288
Fixes: cd3b685043fa (northd: handle container lport type update)
Signed-off-by: Mohammad Heib
---
northd/northd.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index d31cb1688..d587bc98d 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -1856,9 +1856,9 @@ localnet_can_learn_mac(const struct
nbrec_logical_switch_port *nbsp)
static bool
lsp_is_type_changed(const struct sbrec_port_binding *sb,
const struct nbrec_logical_switch_port *nbsp,
-bool *is_old_container_lport)
+bool *update_sbrec)
{
-*is_old_container_lport = false;
+*update_sbrec = false;
if (!sb || !nbsp) {
return false;
}
@@ -1870,13 +1870,19 @@ lsp_is_type_changed(const struct sbrec_port_binding *sb,
*/
if ((!sb->parent_port && nbsp->parent_name) ||
(sb->parent_port && !nbsp->parent_name)) {
-*is_old_container_lport = true;
+*update_sbrec = true;
return true;
} else {
return false;
}
}
+/* Cover cases where port changed to/from virtual port */
+if ((sb->type[0] && !strcmp(sb->type, "virtual"))||
+(nbsp->type[0] && !strcmp(nbsp->type, "virtual"))) {
+*update_sbrec = true;
+}
+
/* Both lports are not "VIF's" it is safe to use strcmp. */
if (sb->type[0] && nbsp->type[0]) {
return strcmp(sb->type, nbsp->type);
@@ -2598,19 +2604,18 @@ join_logical_ports(struct northd_input *input_data,
*created one and recompute everything that is needed
*for this lport.
*
- * This change will affect container lport type changes
- * only for now, this change is needed in container
- * lport cases to avoid port type conflicts in the
- * ovn-controller when the user clears the parent_port
- * field in the container lport.
+ * This change will affect container/virtual lport type
+ * changes only for now, this change is needed in
+ * contaier/virtual lport cases to avoid port type
+ * conflicts in the ovn-controller when the user clears
+ * the parent_port field in the container lport or updated
+ * the lport type.
*
- * This approach can be applied to all other lport types
- * changes by removing the is_old_container_lport.
*/
-bool is_old_container_lport = false;
+bool update_sbrec = false;
if (op->sb && lsp_is_type_changed(op->sb, nbsp,
- &is_old_container_lport)
- && is_old_container_lport) {
+ &update_sbrec)
+ && update_sbrec) {
ovs_list_remove(&op->list);
sbrec_port_binding_delete(op->sb);
ovn_port_destroy(ports, op);
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 1/2] handlers: Create additional handler threads when using CPU isolation
What would be the memory impact of this patch?
Context: we have been exploring the use of ovn in SoCs for edge
environments where memory is a strong constraint and performance is not our
biggest concern in all cases. One of the optimizations we had to make
(thanks +Zenghui Shi for this investigation) was to pin ovs-vswitchd to a
single core to reduce the memory impact (along with disabling mlock-all ),
we were able to reduce ovs-vswitchd footprint from 100MB+ down to 16MB
Is it possible to make this a configurable parameter defaulting to what you
explain here?
El El mar, 19 jul 2022 a las 9:02, Michael Santana
escribió:
> Additional threads are required to service upcalls when we have CPU
> isolation (in per-cpu dispatch mode). The reason additional threads
> are required is because it creates a more fair distribution. With more
> threads we decrease the load of each thread as more threads would
> decrease the number of cores each threads is assigned.
>
> Adding additional threads also increases the chance OVS utilizes all
> cores available to use. Some RPS schemas might make some handler
> threads get all the workload while others get no workload. This tends
> to happen when the handler thread count is low.
>
> An example would be an RPS that sends traffic on all even cores on a
> system with only the lower half of the cores available for OVS to use.
> In this example we have as many handlers threads as there are
> available cores. In this case 50% of the handler threads get all the
> workload while the other 50% get no workload. Not only that, but OVS
> is only utilizing half of the cores that it can use. This is the worst
> case scenario.
>
> The ideal scenario is to have as many threads as there are cores - in
> this case we guarantee that all cores OVS can use are utilized
>
> But, adding as many threads are there are cores could have a performance
> hit when the number of active cores (which all threads have to share) is
> very low. For this reason we avoid creating as many threads as there
> are cores and instead meet somewhere in the middle.
>
> The formula used to calculate the number of handler threads to create
> is as follows:
>
> handlers_n = min(next_prime(active_cores+1), total_cores)
>
> Where next_prime is a function that returns the next numeric prime
> number that is strictly greater than active_cores
>
> Assume default behavior when total_cores <= 2, that is do not create
> additional threads when we have less than 2 total cores on the system
>
> Fixes: b1e517bd2f81 ("dpif-netlink: Introduce per-cpu upcall dispatch.")
> Signed-off-by: Michael Santana
> ---
> lib/dpif-netlink.c | 83 --
> lib/ovs-thread.c | 17 ++
> lib/ovs-thread.h | 1 +
> 3 files changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 06e1e8ca0..21f149867 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -2506,6 +2506,85 @@ dpif_netlink_handler_uninit(struct dpif_handler
> *handler)
> }
> #endif
>
> +/*
> + * Returns true if num is a prime number,
> + * Otherwise return false
> + */
> +static bool
> +is_prime(uint32_t num)
> +{
> +if (num == 2) {
> +return true;
> +}
> +
> +if (num < 2) {
> +return false;
> +}
> +
> +if (num % 2 == 0) {
> +return false;
> +}
> +
> +for (uint64_t i = 3; i * i <= num; i += 2) {
> +if (num % i == 0) {
> +return false;
> +}
> +}
> +
> +return true;
> +}
> +
> +/*
> + * Returns num if num is a prime number. Otherwise returns the next
> + * prime greater than num. Search is limited by UINT32_MAX.
> + *
> + * Returns 0 if no prime has been found between num and UINT32_MAX
> + */
> +static uint32_t
> +next_prime(uint32_t num)
> +{
> +if (num <= 2) {
> +return 2;
> +}
> +
> +for (uint32_t i = num; i < UINT32_MAX; i++) {
> +if (is_prime(i)) {
> +return i;
> +}
> +}
> +
> +return 0;
> +}
> +
> +/*
> + * Calculates and returns the number of handler threads needed based
> + * the following formula:
> + *
> + * handlers_n = min(next_prime(active_cores+1), total_cores)
> + *
> + * Where next_prime is a function that returns the next numeric prime
> + * number that is strictly greater than active_cores
> + */
> +static uint32_t
> +dpif_netlink_calculate_n_handlers(void)
> +{
> +uint32_t total_cores = count_total_cores();
> +uint32_t n_handlers = count_cpu_cores();
> +uint32_t next_prime_num;
> +
> +/*
> + * If we have isolated cores, add additional handler threads to
> + * minimize the number of cores assigned to individual handler threads
> + */
> +if (n_handlers < total_cores && total_cores > 2) {
> +next_prime_num = next_prime(n_handlers + 1);
> +n_handlers = next_prime_num >= total_cores ?
> +total_cores : next_prime_num;
> +}
Re: [ovs-dev] [ovs-build] |fail| pw1656877 [ovs-dev, 2/2] Prepare for post-3.0.0 (3.0.90).
On 7/19/22 11:14, Stokes, Ian wrote: >> On 7/15/22 20:33, Ilya Maximets wrote: >>> On 7/15/22 20:24, Stokes, Ian wrote: > Looks like new MTU tests are unstable. > Ian, Michael, could you, please, check? > > Best regards, Ilya Maximets. Hey Ilya, Let me take a look at this, it passed our internal CI so not sure why it's >> failed externally, worst case I can revert this as it was really just a nice >> to >> have, if it's holding up the branching would you prefer this? >>> >>> No, it's not holding up branching. Just wanted to highlight the >>> issue. Would be great to fix it in a near future, but it's not >>> critical for right now. Thanks. >> >> I think, the tests are lacking the 'Clean up the testpmd now' part >> at least. If testpmd is still running while we're tearing down OVS, >> there is a fair chance that some packets will be received while the >> OpenFlow port is already removed. That migth explain some of the >> failures. Not sure if that what happened in this particular case >> thouhg. >> >> Also, looksing at other failure reports I see some weird things like: >> >> Unreasonably long 1140ms poll interval (408ms user, 263ms system) >> >> 408 + 263 != 1140. The main thread got preempted for half a second >> in this case. Are you running something else on the same cores on >> that system? Or maybe some processes are not getting stopped properly >> after the test? > > Thanks Ilya, I'll look at the unit tests today (apologies for the delay, few > folks were out of office yesterday). > With regards the system, Michael will be back in office tomorrow and I can > ask him to check the system then to see if there are any processes acting > incorrectly? Sure. Thanks for the update. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-build] |fail| pw1656877 [ovs-dev, 2/2] Prepare for post-3.0.0 (3.0.90).
> On 7/15/22 20:33, Ilya Maximets wrote: > > On 7/15/22 20:24, Stokes, Ian wrote: > >>> Looks like new MTU tests are unstable. > >>> Ian, Michael, could you, please, check? > >>> > >>> Best regards, Ilya Maximets. > >> > >> Hey Ilya, > >> > >> Let me take a look at this, it passed our internal CI so not sure why it's > failed externally, worst case I can revert this as it was really just a nice > to > have, if it's holding up the branching would you prefer this? > > > > No, it's not holding up branching. Just wanted to highlight the > > issue. Would be great to fix it in a near future, but it's not > > critical for right now. Thanks. > > I think, the tests are lacking the 'Clean up the testpmd now' part > at least. If testpmd is still running while we're tearing down OVS, > there is a fair chance that some packets will be received while the > OpenFlow port is already removed. That migth explain some of the > failures. Not sure if that what happened in this particular case > thouhg. > > Also, looksing at other failure reports I see some weird things like: > > Unreasonably long 1140ms poll interval (408ms user, 263ms system) > > 408 + 263 != 1140. The main thread got preempted for half a second > in this case. Are you running something else on the same cores on > that system? Or maybe some processes are not getting stopped properly > after the test? Thanks Ilya, I'll look at the unit tests today (apologies for the delay, few folks were out of office yesterday). With regards the system, Michael will be back in office tomorrow and I can ask him to check the system then to see if there are any processes acting incorrectly? Thanks Ian > > > > > Best regards, Ilya Maximets. > > > >> > >> Thanks > >> Ian > >>> > >>> On 7/15/22 18:28, ovs_jenk...@intel.com wrote: > Test-Label: intel-ovs-compilation > Test-Status: fail > http://patchwork.ozlabs.org/api/patches/1656877/ > > AVX-512_compilation: failed > DPLCS Test: success > DPIF Test: success > MFEX Test: fail > Errors in DPCLS test: > None > > Errors in DPIF test: > None > > Errors in MFEX test: > > 2022-07-15T16:27:27.850Z|00046|coverage|INFO|stream_open > >>> 0.0/sec 0.000/sec0./sec total: 1 > > 2022-07-15T16:27:27.850Z|00047|coverage|INFO|util_xalloc > >>> 0.0/sec 0.000/sec0./sec total: 8198 > > 2022-07- > 15T16:27:27.850Z|00048|coverage|INFO|netlink_received > >>> 0.0/sec 0.000/sec0./sec total: 29 > > 2022-07- > 15T16:27:27.850Z|00049|coverage|INFO|netlink_recv_jumbo > >>> 0.0/sec 0.000/sec0./sec total: 3 > > 2022-07-15T16:27:27.850Z|00050|coverage|INFO|netlink_sent > >>> 0.0/sec 0.000/sec0./sec total: 27 > > 2022-07-15T16:27:27.850Z|00051|coverage|INFO|134 events > never > >>> hit > > 2022-07-15T16:27:27.851Z|00052|bridge|INFO|ovs-vswitchd > (Open > >>> vSwitch) 3.0.90 > > 2022-07-15T16:27:27.957Z|00053|pmd_perf|INFO|DPDK provided > TSC > >>> frequency: 240 KHz > > 2022-07- > 15T16:27:27.958Z|00054|dpif_netdev_extract|INFO|Default > >>> MFEX Extract implementation is autovalidator. > > 2022-07-15T16:27:27.969Z|00055|dpif_netdev_impl|INFO|Default > >>> DPIF implementation is dpif_scalar. > > 2022-07- > 15T16:27:27.982Z|00056|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Datapath supports recirculation > > 2022-07- > 15T16:27:27.982Z|00057|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: VLAN header stack length probed as 1 > > 2022-07- > 15T16:27:27.982Z|00058|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: MPLS label stack length probed as 3 > > 2022-07- > 15T16:27:27.982Z|00059|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Datapath supports truncate action > > 2022-07- > 15T16:27:27.982Z|00060|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Datapath supports unique flow ids > > 2022-07- > 15T16:27:27.982Z|00061|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Datapath supports clone action > > 2022-07- > 15T16:27:27.982Z|00062|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Max sample nesting level probed as 10 > > 2022-07- > 15T16:27:27.982Z|00063|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Datapath supports eventmask in conntrack action > > 2022-07- > 15T16:27:27.982Z|00064|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Datapath supports ct_clear action > > 2022-07- > 15T16:27:27.982Z|00065|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Max dp_hash algorithm probed to be 1 > > 2022-07- > 15T16:27:27.982Z|00066|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Datapath supports check_pkt_len action > > 2022-07- > 15T16:27:27.982Z|00067|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Datapath supports timeout policy in conntrack action > > 2022-07- > 15T16:27:27.982Z|00068|ofproto_dpif|INFO|netdev@ovs- > >>> netdev: Datapath supports ct_zero_snat > > 2022-07- > 15T16:27:27.983Z|00069|ofproto_dpif|INFO|netdev@ovs- >
Re: [ovs-dev] [PATCH v5 00/17] python: add flow parsing library
On 7/15/22 21:19, Ilya Maximets wrote:
On 7/8/22 20:02, Adrian Moreno wrote:
While troubleshooting or developing new features in OVS, a considerable
amount of time is spent analyzing flows (whether that's Openflow flows
or datapath flows). Currently, OVS has tools to dump flows with
different levels of verbosity as well as to filter flows prior to
dumping them, e.g: 'ovs-ofctl dump-flows', 'ovs-appctl
dpctl/dump-flows', etc.
The output of these commands is considered stable so it should be
possible to write more layered tools that enable advanced flow analysis.
However, the way flows are formatted into strings is not trivial to
parse.
This series introduces the a flow parsing library capable of parsing
both Openflow and DPIF flows.
The library is based on generic key-value and list parsers and a number
of common decoders. Based on that, an Openflow flow parser and a DPIF
flow parser are introduced by defining a way to decode each possible
field and action they might contain.
The library has the following features:
- Parsed key-value pairs keep some metadata that refer to the original
strings they were extracted from. That way the flows can be printed
and formatted in flexible ways.
- It includes a basic flow filtering mechanism. A filter can be defined
combining logical (||, &&, !), arithmetical (<, >, =) or mask (~=)
operations
- It supports IPAddress and Ethernet masking (based on netaddr)
- The decoder to use for each key (match or action) is set explicitly to
avoid expensive runtime type-guessing.
- The decoders to use for Openflow fields is automatically generated
based on meta-flow.h
- Additional dependencies:
- netaddr: For IP and Ethernet Address management.
- pyparsing: For filtering syntax.
- pytest: For unit tests.
One key goal challenge of including this library is avoiding diversion
between the C code that prints/parses the flows and the python parsing
code. To that effect, the series introduces the following mechanisms:
- Decoding information of openflow fields is automatically generated
based on meta-flow.h
- The calls to ovs-ofctl made from tests/ofp-actions.at are wrapped by a
python script that also runs the python parsers. If an exception is
raised by the python code (meaning it was not capable of parsing the
flow string), the test will fail
- The calls to the test-odp made from tests/odp.at are wrapped by a
python script that also runs the python parsers. If an exception is
raised by the python code (meaning it was not capable of parsing the
flow string), the test will fail.
- A python unit test testsuite ensures python code works and it's easy
to add more flow examples to it
- A dependency check is introduced. The python parsing code mainly
depends on lib/ofp-actions.c and lib/odp-util.c. This series stores
the md5sum of such files and adds a build target that ensures the
files have not been changed. That way, anyone who modifies those files
will get a warning the fist time they build the project. Dependency
digests are easily updated using a string so hopefully this warning
would not be too inconvenient.
Library usage
-
from ovs.flows.ofp import OFPFlow
flow = OFPFlow("cookie=0x2b32ab4d, table=41, n_packets=11, n_bytes=462,
priority=33,ip,reg15=0x2/0x2,nw_src=10.128.0.2/24
actions=move:NXM_OF_TCP_DST[]->NXM_NX_XXREG0[32..47],ct(table=16,zone=NXM_NX_REG13[0..15],nat)")
flow.info
{'cookie': 724740941, 'table': 41, 'n_packets': 11, 'n_bytes': 462}
flow.match
{'priority': 33, 'ip': True, 'reg15': Mask32('0x2/0x2'), 'nw_src':
IPMask('10.128.0.2/24')}
flow.actions
[{'move': {'src': {'field': 'NXM_OF_TCP_DST'}, 'dst': {'field':
'NXM_NX_XXREG0', 'start': 32, 'end': 47}}}, {'ct': {'table': 16, 'zone':
{'field': 'NXM_NX_REG13', 'start': 0, 'end': 15}, 'nat': True}}]
from ovs.flows.filter import OFFilter
filt = OFFilter("nw_src ~= 10.128.0.10 and (table = 42 or n_packets > 0)")
filt.evaluate(flow)
True
V4 -> V5:
- rename tests/test-ovs-{dp,of}parse.py to tests/test-{dp,of}parse.py
- add dependencies to netaddr and pyparsing.
In pip: add a new extras_requirement called "flow"
In packages: add them in the Suggests section
In CI: install them in linux-prepare.sh
V3 -> V4:
- renamed the library ovs/flows -> ovs/flow. Note I've kept Acked-by
tags for patches where only this change was done.
- unit tests: instead of wrapping ovs-ofctl/test-odp tools, just add
another test to the correspondent *.at files as per Ilya's suggestion.
- Dropped patch 13.
- Addressed Ilya's comments on unit test checks (old patch 14/18, new
patch 13/17).
V2 -> V3:
- Simplified KV and list decoding code (Mark Michelson's suggestion)
- Fixed typos
- Added missing files to FLAKE8_PYFILES
- Go back to a simplified ipv4/6 regexp for ip-port range extraction.
Also added specific unit test for ip-port range decoding.
- Adapt ofp encap() action decoder to support new header types: mpls and mpls_mc
(the
