[ovs-dev] [PATCH net-next] selftests: openvswitch: Test ICMP related matches work with SNAT
Add a test case for regression in openvswitch nat that was fixed by
commit e6345d2824a3 ("netfilter: nf_nat: fix action not being set for
all ct states").
Link: https://lore.kernel.org/netdev/20231221224311.130319-1-b...@faucet.nz/
Link: https://mail.openvswitch.org/pipermail/ovs-dev/2024-January/410476.html
Suggested-by: Aaron Conole
Signed-off-by: Brad Cowie
---
.../selftests/net/openvswitch/openvswitch.sh | 62 +++
1 file changed, 62 insertions(+)
diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh
b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index f8499d4c87f3..87b80bee6df4 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -17,6 +17,7 @@ tests="
ct_connect_v4 ip4-ct-xon: Basic ipv4 tcp
connection using ct
connect_v4 ip4-xon: Basic ipv4 ping
between two NS
nat_connect_v4 ip4-nat-xon: Basic ipv4 tcp
connection via NAT
+ nat_related_v4 ip4-nat-related: ICMP related
matches work with SNAT
netlink_checks ovsnl: validate netlink attrs
and settings
upcall_interfaces ovs: test the upcall interfaces
drop_reason drop: test drop reasons are
emitted"
@@ -473,6 +474,67 @@ test_nat_connect_v4 () {
return 0
}
+# nat_related_v4 test
+# - client->server ip packets go via SNAT
+# - client solicits ICMP destination unreachable packet from server
+# - undo NAT for ICMP reply and test dst ip has been updated
+test_nat_related_v4 () {
+ which nc >/dev/null 2>/dev/null || return $ksft_skip
+
+ sbx_add "test_nat_related_v4" || return $?
+
+ ovs_add_dp "test_nat_related_v4" natrelated4 || return 1
+ info "create namespaces"
+ for ns in client server; do
+ ovs_add_netns_and_veths "test_nat_related_v4" "natrelated4"
"$ns" \
+ "${ns:0:1}0" "${ns:0:1}1" || return 1
+ done
+
+ ip netns exec client ip addr add 172.31.110.10/24 dev c1
+ ip netns exec client ip link set c1 up
+ ip netns exec server ip addr add 172.31.110.20/24 dev s1
+ ip netns exec server ip link set s1 up
+
+ ip netns exec server ip route add 192.168.0.20/32 via 172.31.110.10
+
+ # Allow ARP
+ ovs_add_flow "test_nat_related_v4" natrelated4 \
+ "in_port(1),eth(),eth_type(0x0806),arp()" "2" || return 1
+ ovs_add_flow "test_nat_related_v4" natrelated4 \
+ "in_port(2),eth(),eth_type(0x0806),arp()" "1" || return 1
+
+ # Allow IP traffic from client->server, rewrite source IP with SNAT to
192.168.0.20
+ ovs_add_flow "test_nat_related_v4" natrelated4 \
+
"ct_state(-trk),in_port(1),eth(),eth_type(0x0800),ipv4(dst=172.31.110.20)" \
+ "ct(commit,nat(src=192.168.0.20)),recirc(0x1)" || return 1
+ ovs_add_flow "test_nat_related_v4" natrelated4 \
+
"recirc_id(0x1),ct_state(+trk-inv),in_port(1),eth(),eth_type(0x0800),ipv4()" \
+ "2" || return 1
+
+ # Allow related ICMP responses back from server and undo NAT to restore
original IP
+ # Drop any ICMP related packets where dst ip hasn't been restored back
to original IP
+ ovs_add_flow "test_nat_related_v4" natrelated4 \
+ "ct_state(-trk),in_port(2),eth(),eth_type(0x0800),ipv4()" \
+ "ct(commit,nat),recirc(0x2)" || return 1
+ ovs_add_flow "test_nat_related_v4" natrelated4 \
+
"recirc_id(0x2),ct_state(+rel+trk),in_port(2),eth(),eth_type(0x0800),ipv4(src=172.31.110.20,dst=172.31.110.10,proto=1),icmp()"
\
+ "1" || return 1
+ ovs_add_flow "test_nat_related_v4" natrelated4 \
+
"recirc_id(0x2),ct_state(+rel+trk),in_port(2),eth(),eth_type(0x0800),ipv4(dst=192.168.0.20,proto=1),icmp()"
\
+ "drop" || return 1
+
+ # Solicit destination unreachable response from server
+ ovs_sbx "test_nat_related_v4" ip netns exec client \
+ bash -c "echo a | nc -u -w 1 172.31.110.20 1"
+
+ # Check to make sure no packets matched the drop rule with incorrect
dst ip
+ python3 "$ovs_base/ovs-dpctl.py" dump-flows natrelated4 \
+ | grep "drop" | grep "packets:0" >/dev/null || return 1
+
+ info "done..."
+ return 0
+}
+
# netlink_validation
# - Create a dp
# - check no warning with "old version" simulation
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-23.09] ovs: Bump submodule to tip of OVS branch-3.2.
On Tue, Jan 30, 2024 at 10:52 AM Numan Siddique wrote: > > On Tue, Jan 30, 2024 at 7:38 AM Dumitru Ceara wrote: > > > > This picks up the following relevant commit: > > cd8ffc956c3c ovs-atomic: Fix inclusion of Clang header by GCC 14. > > > > Without this builds on Fedora 40 (rawhide) are broken due to failing to > > compile the submodule. > > > > Signed-off-by: Dumitru Ceara > > Acked-by: Numan Siddique I went ahead and applied this patch to branch-23.09. Thanks Numan > > Numan > > > --- > > ovs | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/ovs b/ovs > > index ec1d730163..49e64f13b2 16 > > --- a/ovs > > +++ b/ovs > > @@ -1 +1 @@ > > -Subproject commit ec1d730163d984934c467e050ebf6d39f8c09384 > > +Subproject commit 49e64f13b2c965f5b53a65eeab70ac2e3f0bf69a > > -- > > 2.39.3 > > > > ___ > > 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] Add "disable_arp_nd_rsp" option to LSP
> On 29-Jan-2024, at 9:11 PM, Ihar Hrachyshka wrote:
>
> On Mon, Jan 22, 2024 at 12:22 PM Naveen Yerramneni
> wrote:
> This option can be used to enable/disable arp/nd reply flows.
>
> Usecase:
> =
> It is useful to reduce packet loss when VM is being migrated to
>
> It may indeed be useful to be able to disable ARP responder for a LS/port.
>
> I am wondering if you have details about your packet loss issues when
> migrating a VM. Could you please confirm that we are talking about live
> migration (e.g. through libvirt) and that you already use multichassis port
> bindings to host the same port on multiple chassis (on source and
> destination)? In this case, OVN will set up flows that will clone (flood)
> traffic to both locations proactively, for the moment when your hypervisor
> switches running the VM from source to destination. You should not observe
> (significant) packet losses in this scenario.
VM migration is happening between two different logical switches (i.e., ports
are different) hence requested-chassis option is not helpful here. In this
case, same VLAN is stretched using VXLAN (external VTEP devices).
Packet loss is observed in 2 cases:
1. When port is configured on the destination but migration is still in
progress. Patch raised for this -
https://www.mail-archive.com/ovs-dev@openvswitch.org/msg82745.html
2. When VM sends GARP packet post migration and port is not yet deleted on
the source side then, source side logical switch responds to GARP. This makes
the intermediate VTEP devices to incorrectly learn the location of the port.
Skipping ARP/ND responder and letting the ARP/ND get flooded to learn the
location of the port properly.
> different AZ via VXLAN tunnel. Port is configured in both AZs
> on different logical switches which are sharing same IP subnet.
>
> This snippet above suggests to me that you migrate between different logical
> switch ports? Could you please elaborate on how you set up your overlay
> connectivity for the VM?
>
> The reason I ask is because live migration reuses the same LSP, only changing
> the chassis that host(s) the LSP.
VM migration is happening between two different logical switches (i.e., ports
are different). In this case, same VLAN is stretched using VXLAN (external
VTEP devices).
>
> In reality, the port is active on only one logical switch.
> Skipping ARP/ND responder and letting the ARP/ND get flooded to
> learn the location of the port.
>
> Signed-off-by: Naveen Yerramneni
> ---
> northd/northd.c | 10 +-
> tests/ovn-northd.at [ovn-northd.at] | 31 +++
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 952f8200d..4e070c0fe 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -1844,6 +1844,12 @@ localnet_can_learn_mac(const struct
> nbrec_logical_switch_port *nbsp)
> return smap_get_bool(>options, "localnet_learn_fdb", false);
> }
>
> +static bool
> +lsp_disable_arp_nd_rsp(const struct nbrec_logical_switch_port *nbsp)
> +{
> +return smap_get_bool(>options, "disable_arp_nd_rsp", false);
> +}
> +
> static bool
> lsp_is_type_changed(const struct sbrec_port_binding *sb,
> const struct nbrec_logical_switch_port *nbsp,
> @@ -9921,7 +9927,9 @@ build_lswitch_arp_nd_responder_known_ips(struct
> ovn_port *op,
> return;
> }
>
> -if (lsp_is_external(op->nbsp) || op->has_unknown) {
> +if (lsp_is_external(op->nbsp) || op->has_unknown ||
> + (!strcmp(op->nbsp->type, "") &&
> +lsp_disable_arp_nd_rsp(op->nbsp))) {
> return;
> }
>
> diff --git a/tests/ovn-northd.at [ovn-northd.at] b/tests/ovn-northd.at
> [ovn-northd.at]
> index 9a0d418e4..9a36ee810 100644
> --- a/tests/ovn-northd.at [ovn-northd.at]
> +++ b/tests/ovn-northd.at [ovn-northd.at]
> @@ -11094,5 +11094,36 @@ AT_CHECK([ovn-sbctl dump-flows S1 | grep pre_acl |
> sed 's/table=./table=?/'], [0
> ])
>
>
> +AT_CLEANUP
> +])
> +
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([check options:disable_arp_nd_rsp for LSP])
> +ovn_start NORTHD_TYPE
> +ovn-nbctl ls-add S1
> +ovn-nbctl --wait=sb lsp-add S1 S1-vm1
> +ovn-nbctl --wait=sb lsp-set-addresses S1-vm1 "50:54:00:00:00:010
> 192.168.0.10 fd00::10"
> +
> +ovn-sbctl dump-flows S1 > S1flows
> +AT_CAPTURE_FILE([S1flows])
> +
> +AT_CHECK([grep -e "ls_in_arp_rsp" S1flows | sed 's/table=../table=??/'],
> [0], [dnl
> + table=??(ls_in_arp_rsp ), priority=100 , match=(arp.tpa ==
> 192.168.0.10 && arp.op == 1 && inport == "S1-vm1"), action=(next;)
> + table=??(ls_in_arp_rsp ), priority=100 , match=(nd_ns && ip6.dst ==
> {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10 && inport == "S1-vm1"),
> action=(next;)
> + table=??(ls_in_arp_rsp ), priority=50 , match=(arp.tpa ==
> 192.168.0.10 && arp.op == 1), action=(eth.dst = eth.src; eth.src =
> 50:54:00:00:00:10;
[ovs-dev] [PATCH v2 3/4] dp-packet: Include inner offsets in adjustments and checks.
Include inner offsets in functions where l3 and l4 offsets are either
modified or checked.
Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
Signed-off-by: Mike Pattrick
---
v2:
- Prints out new offsets in autovalidator
- Extends resize_l2 change to avx512
Signed-off-by: Mike Pattrick
---
lib/dp-packet.c | 18 +-
lib/odp-execute-avx512.c | 19 ++-
2 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 0e23c766e..640b1dfeb 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -507,6 +507,8 @@ dp_packet_resize_l2_5(struct dp_packet *b, int increment)
/* Adjust layer offsets after l2_5. */
dp_packet_adjust_layer_offset(>l3_ofs, increment);
dp_packet_adjust_layer_offset(>l4_ofs, increment);
+dp_packet_adjust_layer_offset(>inner_l3_ofs, increment);
+dp_packet_adjust_layer_offset(>inner_l4_ofs, increment);
return dp_packet_data(b);
}
@@ -529,17 +531,23 @@ dp_packet_compare_offsets(struct dp_packet *b1, struct
dp_packet *b2,
if ((b1->l2_pad_size != b2->l2_pad_size) ||
(b1->l2_5_ofs != b2->l2_5_ofs) ||
(b1->l3_ofs != b2->l3_ofs) ||
-(b1->l4_ofs != b2->l4_ofs)) {
+(b1->l4_ofs != b2->l4_ofs) ||
+(b1->inner_l3_ofs != b2->inner_l3_ofs) ||
+(b1->inner_l4_ofs != b2->inner_l4_ofs)) {
if (err_str) {
ds_put_format(err_str, "Packet offset comparison failed\n");
ds_put_format(err_str, "Buffer 1 offsets: l2_pad_size %u,"
- " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
+ " l2_5_ofs : %u l3_ofs %u, inner_l3_ofs %u,"
+ " l4_ofs %u, inner_l4_ofs %u\n",
b1->l2_pad_size, b1->l2_5_ofs,
- b1->l3_ofs, b1->l4_ofs);
+ b1->l3_ofs, b1->inner_l3_ofs,
+ b1->l4_ofs, b1->inner_l4_ofs);
ds_put_format(err_str, "Buffer 2 offsets: l2_pad_size %u,"
- " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
+ " l2_5_ofs : %u l3_ofs %u, inner_l3_ofs %u,"
+ " l4_ofs %u, inner_l4_ofs %u\n",
b2->l2_pad_size, b2->l2_5_ofs,
- b2->l3_ofs, b2->l4_ofs);
+ b2->l3_ofs, b2->inner_l3_ofs,
+ b2->l4_ofs, b2->inner_l4_ofs);
}
return false;
}
diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
index 747e04014..7f9870669 100644
--- a/lib/odp-execute-avx512.c
+++ b/lib/odp-execute-avx512.c
@@ -35,10 +35,11 @@
VLOG_DEFINE_THIS_MODULE(odp_execute_avx512);
-/* The below three build asserts make sure that l2_5_ofs, l3_ofs, and l4_ofs
- * fields remain in the same order and offset to l2_padd_size. This is needed
- * as the avx512_dp_packet_resize_l2() function will manipulate those fields at
- * a fixed memory index based on the l2_padd_size offset. */
+/* The below three build asserts make sure that l2_5_ofs, l3_ofs, l4_ofs,
+ * inner_l3_ofs, and inner_l4_ofs fields remain in the same order and offset to
+ * l2_padd_size. This is needed as the avx512_dp_packet_resize_l2() function
+ * will manipulate those fields at a fixed memory index based on the
+ * l2_padd_size offset. */
BUILD_ASSERT_DECL(offsetof(struct dp_packet, l2_pad_size) +
MEMBER_SIZEOF(struct dp_packet, l2_pad_size) ==
offsetof(struct dp_packet, l2_5_ofs));
@@ -51,6 +52,14 @@ BUILD_ASSERT_DECL(offsetof(struct dp_packet, l3_ofs) +
MEMBER_SIZEOF(struct dp_packet, l3_ofs) ==
offsetof(struct dp_packet, l4_ofs));
+BUILD_ASSERT_DECL(offsetof(struct dp_packet, l4_ofs) +
+ MEMBER_SIZEOF(struct dp_packet, l4_ofs) ==
+ offsetof(struct dp_packet, inner_l3_ofs));
+
+BUILD_ASSERT_DECL(offsetof(struct dp_packet, inner_l3_ofs) +
+ MEMBER_SIZEOF(struct dp_packet, inner_l3_ofs) ==
+ offsetof(struct dp_packet, inner_l4_ofs));
+
/* The below build assert makes sure it's safe to read/write 128-bits starting
* at the l2_pad_size location. */
BUILD_ASSERT_DECL(sizeof(struct dp_packet) -
@@ -125,7 +134,7 @@ avx512_dp_packet_resize_l2(struct dp_packet *b, int
resize_by_bytes)
/* Each lane represents 16 bits in a 12-bit register. In this case the
* first three 16-bit values, which will map to the l2_5_ofs, l3_ofs and
* l4_ofs fields. */
-const uint8_t k_lanes = 0b1110;
+const uint8_t k_lanes = 0b10;
/* Set all 16-bit words in the 128-bits v_offset register to the value we
* need to add/substract from the l2_5_ofs, l3_ofs, and l4_ofs fields. */
--
2.39.3
___
dev mailing list
d...@openvswitch.org
[ovs-dev] [PATCH v2 4/4] ofproto-dpif-monitor: Remove unneeded calls to clear packets.
Currently the monitor will call dp_packet_clear() on the dp_packet that
is shared amongst BFD, LLDP, and CFM. However, all of these packets are
created with eth_compose(), which already calls dp_packet_clear().
Signed-off-by: Mike Pattrick
---
ofproto/ofproto-dpif-monitor.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/ofproto/ofproto-dpif-monitor.c b/ofproto/ofproto-dpif-monitor.c
index bb0e49091..5132f9c95 100644
--- a/ofproto/ofproto-dpif-monitor.c
+++ b/ofproto/ofproto-dpif-monitor.c
@@ -275,19 +275,16 @@ monitor_mport_run(struct mport *mport, struct dp_packet
*packet)
long long int lldp_wake_time = LLONG_MAX;
if (mport->cfm && cfm_should_send_ccm(mport->cfm)) {
-dp_packet_clear(packet);
cfm_compose_ccm(mport->cfm, packet, mport->hw_addr);
ofproto_dpif_send_packet(mport->ofport, false, packet);
}
if (mport->bfd && bfd_should_send_packet(mport->bfd)) {
bool oam;
-dp_packet_clear(packet);
bfd_put_packet(mport->bfd, packet, mport->hw_addr, );
ofproto_dpif_send_packet(mport->ofport, oam, packet);
}
if (mport->lldp && lldp_should_send_packet(mport->lldp)) {
-dp_packet_clear(packet);
lldp_put_packet(mport->lldp, packet, mport->hw_addr);
ofproto_dpif_send_packet(mport->ofport, false, packet);
}
--
2.39.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 2/4] bfd: Set proper offsets and flags in BFD packets.
Previously the BFD packet creation code did not appropriately set
offsets or flags. This contributed to issues involving encapsulation and
the TSO code.
Fixes: ccc096898c46 ("bfd: Implement Bidirectional Forwarding Detection.")
Signed-off-by: Mike Pattrick
---
v2: Corrected formatting, and just calculate checksum up front
---
lib/bfd.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/lib/bfd.c b/lib/bfd.c
index 9698576d0..9af258917 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -586,7 +586,6 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
{
long long int min_tx, min_rx;
struct udp_header *udp;
-struct eth_header *eth;
struct ip_header *ip;
struct msg *msg;
@@ -605,15 +604,13 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
* set. */
ovs_assert(!(bfd->flags & FLAG_POLL) || !(bfd->flags & FLAG_FINAL));
-dp_packet_reserve(p, 2); /* Properly align after the ethernet header. */
-eth = dp_packet_put_uninit(p, sizeof *eth);
-eth->eth_src = eth_addr_is_zero(bfd->local_eth_src)
-? eth_src : bfd->local_eth_src;
-eth->eth_dst = eth_addr_is_zero(bfd->local_eth_dst)
-? eth_addr_bfd : bfd->local_eth_dst;
-eth->eth_type = htons(ETH_TYPE_IP);
+ip = eth_compose(p,
+ eth_addr_is_zero(bfd->local_eth_dst)
+ ? eth_addr_bfd : bfd->local_eth_dst,
+ eth_addr_is_zero(bfd->local_eth_src)
+ ? eth_src : bfd->local_eth_src,
+ ETH_TYPE_IP, sizeof *ip + sizeof *udp + sizeof *msg);
-ip = dp_packet_put_zeros(p, sizeof *ip);
ip->ip_ihl_ver = IP_IHL_VER(5, 4);
ip->ip_tot_len = htons(sizeof *ip + sizeof *udp + sizeof *msg);
ip->ip_ttl = MAXTTL;
@@ -621,15 +618,17 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p,
ip->ip_proto = IPPROTO_UDP;
put_16aligned_be32(>ip_src, bfd->ip_src);
put_16aligned_be32(>ip_dst, bfd->ip_dst);
-/* Checksum has already been zeroed by put_zeros call. */
+/* Checksum has already been zeroed by eth_compose call. */
ip->ip_csum = csum(ip, sizeof *ip);
+dp_packet_set_l4(p, ip + 1);
-udp = dp_packet_put_zeros(p, sizeof *udp);
+udp = dp_packet_l4(p);
udp->udp_src = htons(bfd->udp_src);
udp->udp_dst = htons(BFD_DEST_PORT);
udp->udp_len = htons(sizeof *udp + sizeof *msg);
+/* Checksum already zero from eth_compose. */
-msg = dp_packet_put_uninit(p, sizeof *msg);
+msg = (struct msg *)(udp + 1);
msg->vers_diag = (BFD_VERSION << 5) | bfd->diag;
msg->flags = (bfd->state & STATE_MASK) | bfd->flags;
--
2.39.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 1/4] dp-packet: Validate correct offset for L4 inner size.
This patch fixes the correctness of dp_packet_inner_l4_size() when
checking for the existence of an inner L4 header. Previously it checked
for the outer L4 header.
This function is currently only used when a packet is already flagged
for tunneling, so an incorrect determination isn't possible as long as
the flags of the packet are correct.
Fixes: 85bcbbed839a ("userspace: Enable tunnel tests with TSO.")
Signed-off-by: Mike Pattrick
---
v2: Corrected patch subject
---
lib/dp-packet.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index dceb701e8..802d3f385 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -540,7 +540,7 @@ dp_packet_inner_l4(const struct dp_packet *b)
static inline size_t
dp_packet_inner_l4_size(const struct dp_packet *b)
{
-return OVS_LIKELY(b->l4_ofs != UINT16_MAX)
+return OVS_LIKELY(b->inner_l4_ofs != UINT16_MAX)
? (const char *) dp_packet_tail(b)
- (const char *) dp_packet_inner_l4(b)
- dp_packet_l2_pad_size(b)
--
2.39.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v6 05/13] northd: Refactor lflow management into a separate module.
Bleep bloop. Greetings Numan Siddique, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line has non-spaces leading whitespace WARNING: Line has trailing whitespace #4892 FILE: northd/northd.c:16073: Lines checked: 5918, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v6 04/13] northd: Add a new node 'ls_stateful'.
Bleep bloop. Greetings Numan Siddique, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Inappropriate bracing around statement #687 FILE: northd/en-ls-stateful.h:55: HMAP_FOR_EACH (LS_STATEFUL_REC, key_node, &(TABLE)->entries) ERROR: Inappropriate bracing around statement #742 FILE: northd/en-port-group.h:52: HMAP_FOR_EACH (LS_PG, key_node, &(TABLE)->entries) Lines checked: 1799, Warnings: 0, Errors: 2 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v2 2/3] rbac: Restrict IGMP_Group updates to relevant chassis.
On 1/30/24 16:08, Mark Michelson wrote:
RBAC did not restrict which chassis could update IGMP_Groups. With this
change, we add a new "chassis_name" column to IGMP_Group.
This may seem odd since there is already a "chassis" column in
IGMP_Group. But RBAC specifically works by string matching based on the
certificate common name. Therefore, we need to have a chassis_name
string column instead of a chassis UUID column.
Getting RBAC to function properly required me to fix an existing bug as
well. igmp_group_cleanup() did not ensure that only local IGMP group
records were deleted. This presumably meant that when one ovn-controller
in a cluster was shut down, it would delete ALL IGMP_Group records in
the southbound DB, not just the local ones.
Signed-off-by: Mark Michelson
---
v1 -> v2:
* Rebased on top of current main
* Fixed igmp_group_cleanup() to only delete local records.
---
controller/ip-mcast.c | 26 +++---
controller/ip-mcast.h | 9 ++---
controller/ovn-controller.c | 3 ++-
controller/pinctrl.c| 16 +---
northd/ovn-northd.c | 2 +-
ovn-sb.ovsschema| 7 ---
ovn-sb.xml | 5 +
tests/ovn.at| 2 +-
8 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c
index a870fb29e..b457c7e69 100644
--- a/controller/ip-mcast.c
+++ b/controller/ip-mcast.c
@@ -38,7 +38,8 @@ 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);
+ const struct sbrec_chassis *chassis,
+ bool igmp_group_has_chassis_name);
struct ovsdb_idl_index *
igmp_group_index_create(struct ovsdb_idl *idl)
@@ -86,7 +87,8 @@ struct sbrec_igmp_group *
igmp_group_create(struct ovsdb_idl_txn *idl_txn,
const struct in6_addr *address,
const struct sbrec_datapath_binding *datapath,
- const struct sbrec_chassis *chassis)
+ const struct sbrec_chassis *chassis,
+ bool igmp_group_has_chassis_name)
{
char addr_str[INET6_ADDRSTRLEN];
@@ -94,16 +96,18 @@ igmp_group_create(struct ovsdb_idl_txn *idl_txn,
return NULL;
}
-return igmp_group_create_(idl_txn, addr_str, datapath, chassis);
+return igmp_group_create_(idl_txn, addr_str, datapath, chassis,
+ igmp_group_has_chassis_name);
}
struct sbrec_igmp_group *
igmp_mrouter_create(struct ovsdb_idl_txn *idl_txn,
const struct sbrec_datapath_binding *datapath,
-const struct sbrec_chassis *chassis)
+const struct sbrec_chassis *chassis,
+bool igmp_group_has_chassis_name)
{
return igmp_group_create_(idl_txn, OVN_IGMP_GROUP_MROUTERS, datapath,
- chassis);
+ chassis, igmp_group_has_chassis_name);
}
void
@@ -211,7 +215,8 @@ igmp_group_delete(const struct sbrec_igmp_group *g)
bool
igmp_group_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
- struct ovsdb_idl_index *igmp_groups)
+ struct ovsdb_idl_index *igmp_groups,
+ const struct sbrec_chassis *chassis)
{
const struct sbrec_igmp_group *g;
@@ -220,6 +225,9 @@ igmp_group_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
}
SBREC_IGMP_GROUP_FOR_EACH_BYINDEX (g, igmp_groups) {
+if (chassis != g->chassis) {
+continue;
+}
igmp_group_delete(g);
}
@@ -249,13 +257,17 @@ 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)
+ const struct sbrec_chassis *chassis,
+ bool igmp_group_has_chassis_name)
{
struct sbrec_igmp_group *g = sbrec_igmp_group_insert(idl_txn);
sbrec_igmp_group_set_address(g, addr_str);
sbrec_igmp_group_set_datapath(g, datapath);
sbrec_igmp_group_set_chassis(g, chassis);
+if (igmp_group_has_chassis_name) {
+sbrec_igmp_group_set_chassis_name(g, chassis->name);
+}
return g;
}
diff --git a/controller/ip-mcast.h b/controller/ip-mcast.h
index 326f39db1..eebada968 100644
--- a/controller/ip-mcast.h
+++ b/controller/ip-mcast.h
@@ -39,11 +39,13 @@ struct sbrec_igmp_group *igmp_group_create(
struct ovsdb_idl_txn *idl_txn,
const struct in6_addr *address,
const struct sbrec_datapath_binding *datapath,
-const struct sbrec_chassis *chassis);
+const struct sbrec_chassis *chassis,
+bool
Re: [ovs-dev] [PATCH ovn v6 02/13] northd: Add a new engine 'lr_stateful' to manage lr's stateful data.
Bleep bloop. Greetings Numan Siddique, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Inappropriate bracing around statement #100 FILE: northd/en-lr-nat.h:95: HMAP_FOR_EACH (LR_NAT_REC, key_node, &(TABLE)->entries) ERROR: Inappropriate bracing around statement #849 FILE: northd/en-lr-stateful.h:74: HMAP_FOR_EACH (LR_LB_NAT_REC, key_node, &(TABLE)->entries) Lines checked: 3014, Warnings: 0, Errors: 2 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v6 13/13] northd: Add I-P for NB_Global and SB_Global.
From: Numan Siddique
A new engine node "global_config" is added which handles the changes
to NB_Global an SB_Global tables. It also creates these rows if
not present.
Without the I-P, any changes to the options column of these tables
result in recompute of 'northd' and 'lflow' engine nodes.
Acked-by: Dumitru Ceara
Acked-by: Han Zhou
Signed-off-by: Numan Siddique
---
northd/aging.c| 21 +-
northd/automake.mk| 2 +
northd/en-global-config.c | 576 ++
northd/en-global-config.h | 65 +
northd/en-lflow.c | 11 +-
northd/en-northd.c| 52 ++--
northd/en-northd.h| 2 +-
northd/en-sync-sb.c | 21 +-
northd/inc-proc-northd.c | 38 ++-
northd/northd.c | 230 +++
northd/northd.h | 24 +-
tests/ovn-northd.at | 256 +++--
12 files changed, 1001 insertions(+), 297 deletions(-)
create mode 100644 northd/en-global-config.c
create mode 100644 northd/en-global-config.h
diff --git a/northd/aging.c b/northd/aging.c
index cdf5f4464e..b76963a2dd 100644
--- a/northd/aging.c
+++ b/northd/aging.c
@@ -15,6 +15,7 @@
#include
+#include "en-global-config.h"
#include "lib/inc-proc-eng.h"
#include "lib/ovn-nb-idl.h"
#include "lib/ovn-sb-idl.h"
@@ -347,15 +348,10 @@ aging_context_handle_timestamp(struct aging_context *ctx,
int64_t timestamp,
static uint32_t
get_removal_limit(struct engine_node *node, const char *name)
{
-const struct nbrec_nb_global_table *nb_global_table =
-EN_OVSDB_GET(engine_get_input("NB_nb_global", node));
-const struct nbrec_nb_global *nb =
-nbrec_nb_global_table_first(nb_global_table);
-if (!nb) {
-return 0;
-}
+struct ed_type_global_config *global_config =
+engine_get_input_data("global_config", node);
-return smap_get_uint(>options, name, 0);
+return smap_get_uint(_config->nb_options, name, 0);
}
/* MAC binding aging */
@@ -394,11 +390,14 @@ en_mac_binding_aging_run(struct engine_node *node, void
*data OVS_UNUSED)
{
const struct engine_context *eng_ctx = engine_get_context();
struct northd_data *northd_data = engine_get_input_data("northd", node);
+struct ed_type_global_config *global_config =
+engine_get_input_data("global_config", node);
+
struct aging_waker *waker =
engine_get_input_data("mac_binding_aging_waker", node);
if (!eng_ctx->ovnsb_idl_txn ||
-!northd_data->features.mac_binding_timestamp ||
+!global_config->features.mac_binding_timestamp ||
time_msec() < waker->next_wake_msec) {
return;
}
@@ -530,9 +529,11 @@ en_fdb_aging_run(struct engine_node *node, void *data
OVS_UNUSED)
const struct engine_context *eng_ctx = engine_get_context();
struct northd_data *northd_data = engine_get_input_data("northd", node);
struct aging_waker *waker = engine_get_input_data("fdb_aging_waker", node);
+struct ed_type_global_config *global_config =
+engine_get_input_data("global_config", node);
if (!eng_ctx->ovnsb_idl_txn ||
-!northd_data->features.fdb_timestamp ||
+!global_config->features.fdb_timestamp ||
time_msec() < waker->next_wake_msec) {
return;
}
diff --git a/northd/automake.mk b/northd/automake.mk
index 19abb0dece..d491973a8b 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -8,6 +8,8 @@ northd_ovn_northd_SOURCES = \
northd/northd.c \
northd/northd.h \
northd/ovn-northd.c \
+ northd/en-global-config.c \
+ northd/en-global-config.h \
northd/en-northd.c \
northd/en-northd.h \
northd/en-lflow.c \
diff --git a/northd/en-global-config.c b/northd/en-global-config.c
new file mode 100644
index 00..9ac5faf995
--- /dev/null
+++ b/northd/en-global-config.c
@@ -0,0 +1,576 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include
+#include
+#include
+
+/* OVS includes */
+#include "openvswitch/vlog.h"
+
+/* OVN includes */
+#include "debug.h"
+#include "en-global-config.h"
+#include "include/ovn/features.h"
+#include "ipam.h"
+#include "lib/ovn-nb-idl.h"
+#include "lib/ovn-sb-idl.h"
+#include "northd.h"
+
+
+VLOG_DEFINE_THIS_MODULE(en_global_config);
+
+/* static function declarations. */
+static void northd_enable_all_features(struct ed_type_global_config *);
+static void build_chassis_features(const struct
[ovs-dev] [PATCH ovn v6 11/13] northd: Add a noop handler for northd SB mac binding.
From: Numan Siddique northd engine node uses the sb mac binding table to cleanup mac binding entries for deleted logical ports and datapaths. Any update to SB mac binding doesn't change the northd engine node state or data. Hence it is ok to add a noop_handler. Presently, mac_binding_aging node depends on SB mac binding too and it falls back to full recompute for any SB mac binding changes. It needs to be evaluated if mac_binding_aging really needs to handle SB mac binding updates. If not, we can omit the SB mac binding updates (ovsdb_idl_omit_alert()) and also remove the noop_handler this patch adds for northd node. Acked-by: Han Zhou Acked-by: Dumitru Ceara Signed-off-by: Numan Siddique --- northd/inc-proc-northd.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index f7c3d2bcf5..40a9e5e06c 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -177,7 +177,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(_northd, _sb_mirror, NULL); engine_add_input(_northd, _sb_meter, NULL); engine_add_input(_northd, _sb_datapath_binding, NULL); -engine_add_input(_northd, _sb_mac_binding, NULL); engine_add_input(_northd, _sb_dns, NULL); engine_add_input(_northd, _sb_ha_chassis_group, NULL); engine_add_input(_northd, _sb_ip_multicast, NULL); @@ -186,6 +185,17 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(_northd, _sb_static_mac_binding, NULL); engine_add_input(_northd, _sb_chassis_template_var, NULL); +/* northd engine node uses the sb mac binding table to + * cleanup mac binding entries for deleted logical ports + * and datapaths. Any update to SB mac binding doesn't + * change the northd engine node state or data. Hence + * it is ok to add a noop_handler here. + * Note: mac_binding_aging engine node depends on SB mac binding + * and it results in full recompute for any changes to it. + * */ +engine_add_input(_northd, _sb_mac_binding, + engine_noop_handler); + engine_add_input(_northd, _sb_port_binding, northd_sb_port_binding_handler); engine_add_input(_northd, _nb_nb_global, -- 2.43.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v6 12/13] northd: Add northd change handler for sync_to_sb_lb node.
From: Numan Siddique
Any changes to northd engine node due to load balancers
are now handled in 'sync_to_sb_lb' node to sync the changed
load balancers to SB load balancers.
The logic to sync the SB load balancers is changed a bit and it
now mimics the SB lflow sync.
Below are the scale testing results done with all the patches applied
in this series using ovn-heater. The test ran the scenario -
ocp-500-density-heavy.yml [1].
The resuts are:
---
Min (s) Median (s) 90%ile (s) 99%ile
(s) Max (s) Mean (s)Total (s) Count Failed
---
Iteration Total 0.1368831.1290161.192001
1.2041671.2127280.66501783.127099 125 0
Namespace.add_ports 0.0052160.0057360.007034
0.0154860.0189780.0062110.776373125 0
WorkerNode.bind_port0.0350300.0460820.052469
0.0582930.0603110.04597311.493259 250 0
WorkerNode.ping_port0.0050570.0067271.047692
1.0692531.0713360.26689666.724094 250 0
---
The results with the present main [2] are:
---
Min (s) Median (s) 90%ile (s) 99%ile
(s) Max (s) Mean (s)Total (s) Count Failed
---
Iteration Total 0.1354912.2238053.311270
3.3390783.3453461.729172216.146495 125 0
Namespace.add_ports 0.0053800.0057440.006819
0.0187730.0208000.0062920.786532125 0
WorkerNode.bind_port0.0341790.0460550.053488
0.0588010.0710430.04611711.529311 250 0
WorkerNode.ping_port0.0049560.0069523.086952
3.1917433.1928070.791544197.886026 250 0
---
[1] -
https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml
[2] - 2a12cda890a7("controller, northd: Wait for cleanup before replying to
exit")
Acked-by: Dumitru Ceara
Acked-by: Han Zhou
Signed-off-by: Numan Siddique
---
northd/en-sync-sb.c | 497 +--
northd/inc-proc-northd.c | 1 +
northd/lflow-mgr.c | 178 ++
northd/lflow-mgr.h | 23 +-
northd/northd.c | 238 ---
northd/northd.h | 6 -
tests/ovn-northd.at | 164 ++---
7 files changed, 697 insertions(+), 410 deletions(-)
diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
index 9ca59d4338..9002729b39 100644
--- a/northd/en-sync-sb.c
+++ b/northd/en-sync-sb.c
@@ -18,9 +18,11 @@
#include
#include
+/* OVS includes. */
#include "lib/svec.h"
#include "openvswitch/util.h"
+/* OVN includes. */
#include "en-lr-nat.h"
#include "en-lr-stateful.h"
#include "en-sync-sb.h"
@@ -30,6 +32,7 @@
#include "lib/ovn-nb-idl.h"
#include "lib/ovn-sb-idl.h"
#include "lib/ovn-util.h"
+#include "lflow-mgr.h"
#include "northd.h"
#include "openvswitch/vlog.h"
@@ -210,49 +213,127 @@ sync_to_sb_addr_set_nb_port_group_handler(struct
engine_node *node,
/* sync_to_sb_lb engine node functions.
* This engine node syncs the SB load balancers.
*/
+struct sb_lb_record {
+struct hmap_node key_node; /* Index on 'nblb->header_.uuid'. */
+
+struct ovn_lb_datapaths *lb_dps;
+const struct sbrec_load_balancer *sbrec_lb;
+struct ovn_dp_group *ls_dpg;
+struct ovn_dp_group *lr_dpg;
+struct uuid sb_uuid;
+};
+
+struct sb_lb_table {
+struct hmap entries; /* Stores struct sb_lb_record. */
+struct hmap ls_dp_groups;
+struct hmap lr_dp_groups;
+};
+
+struct ed_type_sync_to_sb_lb_data {
+struct sb_lb_table sb_lbs;
+};
+
+static void sb_lb_table_init(struct sb_lb_table *);
+static void sb_lb_table_clear(struct sb_lb_table *);
+static void sb_lb_table_destroy(struct sb_lb_table *);
+
+static struct sb_lb_record *sb_lb_table_find(struct hmap *sb_lbs,
[ovs-dev] [PATCH ovn v6 10/13] northd: Add ls_stateful handler for lflow engine node.
From: Numan Siddique
Acked-by: Han Zhou
Signed-off-by: Numan Siddique
---
northd/en-lflow.c| 26 +
northd/en-lflow.h| 1 +
northd/en-ls-stateful.c | 9 +++--
northd/en-ls-stateful.h | 26 +
northd/inc-proc-northd.c | 2 +-
northd/northd.c | 61 +++---
northd/northd.h | 5 +++
tests/ovn-northd.at | 81 ++--
8 files changed, 173 insertions(+), 38 deletions(-)
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 41b1779539..525453054b 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -190,6 +190,32 @@ lflow_lr_stateful_handler(struct engine_node *node, void
*data)
return true;
}
+bool
+lflow_ls_stateful_handler(struct engine_node *node, void *data)
+{
+struct ed_type_ls_stateful *ls_sful_data =
+engine_get_input_data("ls_stateful", node);
+
+if (!ls_stateful_has_tracked_data(_sful_data->trk_data)) {
+return false;
+}
+
+const struct engine_context *eng_ctx = engine_get_context();
+struct lflow_data *lflow_data = data;
+struct lflow_input lflow_input;
+
+lflow_get_input_data(node, _input);
+if (!lflow_handle_ls_stateful_changes(eng_ctx->ovnsb_idl_txn,
+ _sful_data->trk_data,
+ _input,
+ lflow_data->lflow_table)) {
+return false;
+}
+
+engine_set_node_state(node, EN_UPDATED);
+return true;
+}
+
void *en_lflow_init(struct engine_node *node OVS_UNUSED,
struct engine_arg *arg OVS_UNUSED)
{
diff --git a/northd/en-lflow.h b/northd/en-lflow.h
index 1d813a2a29..32cae61763 100644
--- a/northd/en-lflow.h
+++ b/northd/en-lflow.h
@@ -21,5 +21,6 @@ void en_lflow_cleanup(void *data);
bool lflow_northd_handler(struct engine_node *, void *data);
bool lflow_port_group_handler(struct engine_node *, void *data);
bool lflow_lr_stateful_handler(struct engine_node *, void *data);
+bool lflow_ls_stateful_handler(struct engine_node *node, void *data);
#endif /* EN_LFLOW_H */
diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c
index fae71c4c8f..44f74ea08e 100644
--- a/northd/en-ls-stateful.c
+++ b/northd/en-ls-stateful.c
@@ -39,6 +39,7 @@
#include "lib/ovn-sb-idl.h"
#include "lib/ovn-util.h"
#include "lib/stopwatch-names.h"
+#include "lflow-mgr.h"
#include "northd.h"
VLOG_DEFINE_THIS_MODULE(en_ls_stateful);
@@ -289,6 +290,7 @@ ls_stateful_record_create(struct ls_stateful_table *table,
ls_stateful_rec->ls_index = od->index;
ls_stateful_rec->nbs_uuid = od->nbs->header_.uuid;
ls_stateful_record_init(ls_stateful_rec, od, NULL, ls_pgs);
+ls_stateful_rec->lflow_ref = lflow_ref_create();
hmap_insert(>entries, _stateful_rec->key_node,
uuid_hash(>nbs->header_.uuid));
@@ -299,14 +301,15 @@ ls_stateful_record_create(struct ls_stateful_table *table,
static void
ls_stateful_record_destroy(struct ls_stateful_record *ls_stateful_rec)
{
+lflow_ref_destroy(ls_stateful_rec->lflow_ref);
free(ls_stateful_rec);
}
static void
ls_stateful_record_init(struct ls_stateful_record *ls_stateful_rec,
-const struct ovn_datapath *od,
-const struct ls_port_group *ls_pg,
-const struct ls_port_group_table *ls_pgs)
+ const struct ovn_datapath *od,
+ const struct ls_port_group *ls_pg,
+ const struct ls_port_group_table *ls_pgs)
{
ls_stateful_rec->has_lb_vip = ls_has_lb_vip(od);
ls_stateful_record_set_acl_flags(ls_stateful_rec, od, ls_pg, ls_pgs);
diff --git a/northd/en-ls-stateful.h b/northd/en-ls-stateful.h
index a31b1d154d..eae4b08e22 100644
--- a/northd/en-ls-stateful.h
+++ b/northd/en-ls-stateful.h
@@ -31,6 +31,8 @@
#include "lib/ovn-util.h"
#include "lib/stopwatch-names.h"
+struct lflow_ref;
+
struct ls_stateful_record {
struct hmap_node key_node;
@@ -45,6 +47,30 @@ struct ls_stateful_record {
bool has_lb_vip;
bool has_acls;
uint64_t max_acl_tier;
+
+/* 'lflow_ref' is used to reference logical flows generated for
+ * this ls_stateful record.
+ *
+ * This data is initialized and destroyed by the en_ls_stateful node,
+ * but populated and used only by the en_lflow node. Ideally this data
+ * should be maintained as part of en_lflow's data. However, it would
+ * be less efficient and more complex:
+ *
+ * 1. It would require an extra search (using the index) to find the
+ * lflows.
+ *
+ * 2. Building the index needs to be thread-safe, using either a global
+ * lock which is obviously less efficient, or hash-based lock array which
+ * is more complex.
+ *
+ * Adding the lflow_ref here is more straightforward. The drawback is that
+ * we need to keep in mind that this data belongs to
[ovs-dev] [PATCH ovn v6 09/13] northd: Add lr_stateful handler for lflow engine node.
From: Numan Siddique
Acked-by: Han Zhou
Signed-off-by: Numan Siddique
---
northd/en-lflow.c| 27 +++
northd/en-lflow.h| 1 +
northd/en-lr-stateful.c | 33 +++-
northd/en-lr-stateful.h | 31 +++-
northd/inc-proc-northd.c | 2 +-
northd/northd.c | 369 ++-
northd/northd.h | 11 +-
tests/ovn-northd.at | 48 ++---
8 files changed, 333 insertions(+), 189 deletions(-)
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index c49d24d54b..41b1779539 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -163,6 +163,33 @@ lflow_port_group_handler(struct engine_node *node, void
*data OVS_UNUSED)
return true;
}
+bool
+lflow_lr_stateful_handler(struct engine_node *node, void *data)
+{
+struct ed_type_lr_stateful *lr_sful_data =
+engine_get_input_data("lr_stateful", node);
+
+if (!lr_stateful_has_tracked_data(_sful_data->trk_data)
+|| lr_sful_data->trk_data.vip_nats_changed) {
+return false;
+}
+
+const struct engine_context *eng_ctx = engine_get_context();
+struct lflow_data *lflow_data = data;
+struct lflow_input lflow_input;
+
+lflow_get_input_data(node, _input);
+if (!lflow_handle_lr_stateful_changes(eng_ctx->ovnsb_idl_txn,
+ _sful_data->trk_data,
+ _input,
+ lflow_data->lflow_table)) {
+return false;
+}
+
+engine_set_node_state(node, EN_UPDATED);
+return true;
+}
+
void *en_lflow_init(struct engine_node *node OVS_UNUSED,
struct engine_arg *arg OVS_UNUSED)
{
diff --git a/northd/en-lflow.h b/northd/en-lflow.h
index f7325c56b1..1d813a2a29 100644
--- a/northd/en-lflow.h
+++ b/northd/en-lflow.h
@@ -20,5 +20,6 @@ void *en_lflow_init(struct engine_node *node, struct
engine_arg *arg);
void en_lflow_cleanup(void *data);
bool lflow_northd_handler(struct engine_node *, void *data);
bool lflow_port_group_handler(struct engine_node *, void *data);
+bool lflow_lr_stateful_handler(struct engine_node *, void *data);
#endif /* EN_LFLOW_H */
diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c
index 3e2a6c254e..5f3d677131 100644
--- a/northd/en-lr-stateful.c
+++ b/northd/en-lr-stateful.c
@@ -40,6 +40,7 @@
#include "lib/ovn-sb-idl.h"
#include "lib/ovn-util.h"
#include "lib/stopwatch-names.h"
+#include "lflow-mgr.h"
#include "northd.h"
VLOG_DEFINE_THIS_MODULE(en_lr_stateful);
@@ -81,7 +82,7 @@ static void remove_lrouter_lb_reachable_ips(struct
lr_stateful_record *,
enum lb_neighbor_responder_mode,
const struct sset *lb_ips_v4,
const struct sset *lb_ips_v6);
-static void lr_stateful_rebuild_vip_nats(struct lr_stateful_record *);
+static bool lr_stateful_rebuild_vip_nats(struct lr_stateful_record *);
/* 'lr_stateful' engine node manages the NB logical router LB data.
*/
@@ -109,6 +110,7 @@ en_lr_stateful_clear_tracked_data(void *data_)
struct ed_type_lr_stateful *data = data_;
hmapx_clear(>trk_data.crupdated);
+data->trk_data.vip_nats_changed = false;
}
void
@@ -196,6 +198,10 @@ lr_stateful_lb_data_handler(struct engine_node *node, void
*data_)
/* Add the lr_stateful_rec rec to the tracking data. */
hmapx_add(>trk_data.crupdated, lr_stateful_rec);
+
+if (!sset_is_empty(_stateful_rec->vip_nats)) {
+data->trk_data.vip_nats_changed = true;
+}
continue;
}
@@ -313,7 +319,9 @@ lr_stateful_lb_data_handler(struct engine_node *node, void
*data_)
HMAPX_FOR_EACH (hmapx_node, >trk_data.crupdated) {
struct lr_stateful_record *lr_stateful_rec = hmapx_node->data;
-lr_stateful_rebuild_vip_nats(lr_stateful_rec);
+if (lr_stateful_rebuild_vip_nats(lr_stateful_rec)) {
+data->trk_data.vip_nats_changed = true;
+}
const struct ovn_datapath *od =
ovn_datapaths_find_by_index(input_data.lr_datapaths,
lr_stateful_rec->lr_index);
@@ -352,8 +360,13 @@ lr_stateful_lr_nat_handler(struct engine_node *node, void
*data_)
lr_stateful_record_create(>table, lrnat_rec, od,
input_data.lb_datapaths_map,
input_data.lbgrp_datapaths_map);
+if (!sset_is_empty(_stateful_rec->vip_nats)) {
+data->trk_data.vip_nats_changed = true;
+}
} else {
-lr_stateful_rebuild_vip_nats(lr_stateful_rec);
+if (lr_stateful_rebuild_vip_nats(lr_stateful_rec)) {
+data->trk_data.vip_nats_changed = true;
+}
}
/* Add the lr_stateful_rec rec
[ovs-dev] [PATCH ovn v6 08/13] northd: Handle lb changes in lflow engine.
From: Numan Siddique
Since northd tracked data has the changed lb information,
the lflow change handler for northd inputs can now handle
lb updates incrementally. All the lflows generated for
each lb is stored in the ovn_lb_datapaths->lflow_ref and
this is used similar to how we handle ovn_port changes.
Acked-by: Han Zhou
Signed-off-by: Numan Siddique
---
northd/en-lflow.c | 11 +++--
northd/lb.c | 5 +-
northd/lb.h | 28 +++
northd/lflow-mgr.c | 51
northd/northd.c | 111 +---
northd/northd.h | 4 ++
tests/ovn-northd.at | 30
7 files changed, 198 insertions(+), 42 deletions(-)
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index fafdc24465..c49d24d54b 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -123,11 +123,6 @@ lflow_northd_handler(struct engine_node *node,
return false;
}
-/* Fall back to recompute if load balancers have changed. */
-if (northd_has_lbs_in_tracked_data(_data->trk_data)) {
-return false;
-}
-
const struct engine_context *eng_ctx = engine_get_context();
struct lflow_data *lflow_data = data;
@@ -141,6 +136,12 @@ lflow_northd_handler(struct engine_node *node,
return false;
}
+if (!lflow_handle_northd_lb_changes(
+eng_ctx->ovnsb_idl_txn, _data->trk_data.trk_lbs,
+_input, lflow_data->lflow_table)) {
+return false;
+}
+
engine_set_node_state(node, EN_UPDATED);
return true;
}
diff --git a/northd/lb.c b/northd/lb.c
index e35569cb70..af0c92954c 100644
--- a/northd/lb.c
+++ b/northd/lb.c
@@ -23,7 +23,8 @@
/* OVN includes */
#include "lb.h"
-#include "lib/ovn-nb-idl.h"
+#include "lflow-mgr.h"
+#include "lib/lb.h"
#include "northd.h"
#include "ovn/lex.h"
@@ -563,6 +564,7 @@ ovn_lb_datapaths_create(const struct ovn_northd_lb *lb,
size_t n_ls_datapaths,
lb_dps->lb = lb;
lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths);
lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths);
+lb_dps->lflow_ref = lflow_ref_create();
return lb_dps;
}
@@ -572,6 +574,7 @@ ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps)
{
bitmap_free(lb_dps->nb_lr_map);
bitmap_free(lb_dps->nb_ls_map);
+lflow_ref_destroy(lb_dps->lflow_ref);
free(lb_dps);
}
diff --git a/northd/lb.h b/northd/lb.h
index 00f81c3533..aa6616af41 100644
--- a/northd/lb.h
+++ b/northd/lb.h
@@ -128,6 +128,7 @@ void ovn_lb_group_reinit(
const struct nbrec_load_balancer_group *,
const struct hmap *lbs);
+struct lflow_ref;
struct ovn_lb_datapaths {
struct hmap_node hmap_node;
@@ -137,6 +138,33 @@ struct ovn_lb_datapaths {
size_t n_nb_lr;
unsigned long *nb_lr_map;
+
+/* Reference of lflows generated for this load balancer.
+ *
+ * This data is initialized and destroyed by the en_northd node, but
+ * populated and used only by the en_lflow node. Ideally this data should
+ * be maintained as part of en_lflow's data (struct lflow_data): a hash
+ * index from ovn_port key to lflows. However, it would be less efficient
+ * and more complex:
+ *
+ * 1. It would require an extra search (using the index) to find the
+ * lflows.
+ *
+ * 2. Building the index needs to be thread-safe, using either a global
+ * lock which is obviously less efficient, or hash-based lock array which
+ * is more complex.
+ *
+ * Maintaining the lflow_ref here is more straightforward. The drawback is
+ * that we need to keep in mind that this data belongs to en_lflow node,
+ * so never access it from any other nodes.
+ *
+ * 'lflow_ref' is used to reference logical flows generated for this
+ * load balancer.
+ *
+ * Note: lflow_ref is not thread safe. Only one thread should
+ * access ovn_lb_datapaths->lflow_ref at any given time.
+ */
+struct lflow_ref *lflow_ref;
};
struct ovn_lb_datapaths *ovn_lb_datapaths_create(const struct ovn_northd_lb *,
diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
index 3b423192bb..c45c124f64 100644
--- a/northd/lflow-mgr.c
+++ b/northd/lflow-mgr.c
@@ -535,7 +535,15 @@ struct lflow_ref_node {
/* The lflow. */
struct ovn_lflow *lflow;
-/* Index id of the datapath this lflow_ref_node belongs to. */
+/* Indicates whether the lflow was added with a dp_group using the
+ * ovn_lflow_add_with_dp_group() macro. */
+bool dpgrp_lflow;
+/* dpgrp bitmap and bitmap length. Valid only of dpgrp_lflow is true. */
+unsigned long *dpgrp_bitmap;
+size_t dpgrp_bitmap_len;
+
+/* Index id of the datapath this lflow_ref_node belongs to.
+ * Valid only if dpgrp_lflow is false. */
size_t dp_index;
/* Indicates if the lflow_ref_node for an lflow - L(M, A) is linked
@@ -573,7 +581,9 @@ lflow_ref_destroy(struct lflow_ref *lflow_ref)
/* Unlinks the lflows referenced by the
[ovs-dev] [PATCH ovn v6 07/13] northd: Move ovn_lb_datapaths from lib to northd module.
From: Numan Siddique
It also moves the ovn-controller specific code from lib/lb.c
and lib/lb.h to controller/lb.c and controller/lb.h.
Acked-by: Han Zhou
Co-authored-by: Dumitru Ceara
Signed-off-by: Dumitru Ceara
Signed-off-by: Numan Siddique
---
controller/automake.mk | 2 +
controller/lb.c | 146
controller/lb.h | 55 +++
controller/lflow.c | 1 +
lib/lb.c| 771 +---
lib/lb.h| 199 +--
northd/automake.mk | 4 +-
northd/en-lb-data.c | 1 +
northd/en-lr-stateful.c | 1 +
northd/en-sync-sb.c | 1 +
northd/lb.c | 651 +
northd/lb.h | 189 ++
northd/northd.c | 1 +
13 files changed, 1068 insertions(+), 954 deletions(-)
create mode 100644 controller/lb.c
create mode 100644 controller/lb.h
create mode 100644 northd/lb.c
create mode 100644 northd/lb.h
diff --git a/controller/automake.mk b/controller/automake.mk
index 0dbbd5d26b..a17ff0d60b 100644
--- a/controller/automake.mk
+++ b/controller/automake.mk
@@ -14,6 +14,8 @@ controller_ovn_controller_SOURCES = \
controller/if-status.h \
controller/ip-mcast.c \
controller/ip-mcast.h \
+ controller/lb.c \
+ controller/lb.h \
controller/lflow.c \
controller/lflow.h \
controller/lflow-cache.c \
diff --git a/controller/lb.c b/controller/lb.c
new file mode 100644
index 00..8f9f20ed54
--- /dev/null
+++ b/controller/lb.c
@@ -0,0 +1,146 @@
+/*
+ * Copyright (c) 2024, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+/* OpenvSwitch lib includes. */
+#include "openvswitch/vlog.h"
+#include "lib/smap.h"
+
+/* OVN includes */
+#include "lb.h"
+#include "lib/ovn-sb-idl.h"
+#include "ovn/lex.h"
+
+VLOG_DEFINE_THIS_MODULE(controller_lb);
+
+static void
+ovn_lb_get_hairpin_snat_ip(const struct uuid *lb_uuid,
+ const struct smap *lb_options,
+ struct lport_addresses *hairpin_addrs)
+{
+const char *addresses = smap_get(lb_options, "hairpin_snat_ip");
+
+if (!addresses) {
+return;
+}
+
+if (!extract_ip_address(addresses, hairpin_addrs)) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+VLOG_WARN_RL(, "bad hairpin_snat_ip %s in load balancer "UUID_FMT,
+ addresses, UUID_ARGS(lb_uuid));
+}
+}
+
+struct ovn_controller_lb *
+ovn_controller_lb_create(const struct sbrec_load_balancer *sbrec_lb,
+ const struct smap *template_vars,
+ struct sset *template_vars_ref)
+{
+struct ovn_controller_lb *lb = xzalloc(sizeof *lb);
+bool template = smap_get_bool(_lb->options, "template", false);
+
+lb->slb = sbrec_lb;
+lb->n_vips = smap_count(_lb->vips);
+lb->vips = xcalloc(lb->n_vips, sizeof *lb->vips);
+
+struct smap_node *node;
+size_t n_vips = 0;
+
+SMAP_FOR_EACH (node, _lb->vips) {
+struct ovn_lb_vip *lb_vip = >vips[n_vips];
+
+struct lex_str key_s = template
+ ? lexer_parse_template_string(node->key,
+ template_vars,
+ template_vars_ref)
+ : lex_str_use(node->key);
+struct lex_str value_s = template
+ ? lexer_parse_template_string(node->value,
+ template_vars,
+ template_vars_ref)
+ : lex_str_use(node->value);
+char *error = ovn_lb_vip_init_explicit(lb_vip,
+ lex_str_get(_s),
+ lex_str_get(_s));
+if (error) {
+free(error);
+} else {
+n_vips++;
+}
+lex_str_free(_s);
+lex_str_free(_s);
+}
+
+lb->proto = IPPROTO_TCP;
+if (sbrec_lb->protocol && sbrec_lb->protocol[0]) {
+if (!strcmp(sbrec_lb->protocol, "udp")) {
+lb->proto = IPPROTO_UDP;
+} else if (!strcmp(sbrec_lb->protocol, "sctp")) {
+lb->proto = IPPROTO_SCTP;
+}
+}
+
+/* It's possible that parsing VIPs
[ovs-dev] [PATCH ovn v6 04/13] northd: Add a new node 'ls_stateful'.
From: Numan Siddique
This new engine now maintains the load balancer and ACL data of a
logical switch which was earlier part of northd engine node data.
The main inputs to this engine are:
- northd node
- Port group node
A record for each logical switch is maintained in the 'ls_statefuls'
hmap table and this record stores the below data which was earlier
part of 'struct ovn_datapath'.
- bool has_stateful_acl;
- bool has_lb_vip;
- bool has_acls;
- uint64_t max_acl_tier;
This engine node becomes an input to 'lflow' node.
If a logical switch is configured with
- DHCP for all or some of its ports
- stateful ACLs and/or load balancers
ovn-northd was adding flows to commit the DHCP response generated by
ovn-controller to conntrack. With this patch we don't commit the
DHCP response to conntrack anymore for the following reasons:
1. There is no need to actually commit the response.
2. Since stateful information is moved to 'ls_stateful' node, it
becomes ineffecient to access ls_stateful's data
('has_lb_vip' and 'has_acls') in build_dhcpv4_options_flows().
Acked-by: Dumitru Ceara
Signed-off-by: Numan Siddique
---
lib/stopwatch-names.h| 2 +
northd/automake.mk | 2 +
northd/en-lflow.c| 4 +
northd/en-lr-stateful.c | 21 +-
northd/en-lr-stateful.h | 8 +
northd/en-ls-stateful.c | 437 +++
northd/en-ls-stateful.h | 87
northd/en-northd.c | 2 +-
northd/en-port-group.h | 3 +
northd/inc-proc-northd.c | 7 +
northd/northd.c | 363
northd/northd.h | 35 +++-
northd/ovn-northd.c | 2 +
13 files changed, 792 insertions(+), 181 deletions(-)
create mode 100644 northd/en-ls-stateful.c
create mode 100644 northd/en-ls-stateful.h
diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index 1e8a5b656f..660c653fb5 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -25,6 +25,7 @@
#define LFLOWS_PORTS_STOPWATCH_NAME "lflows_ports"
#define LFLOWS_LBS_STOPWATCH_NAME "lflows_lbs"
#define LFLOWS_LR_STATEFUL_STOPWATCH_NAME "lflows_lr_stateful"
+#define LFLOWS_LS_STATEFUL_STOPWATCH_NAME "lflows_ls_stateful"
#define LFLOWS_IGMP_STOPWATCH_NAME "lflows_igmp"
#define LFLOWS_DP_GROUPS_STOPWATCH_NAME "lflows_dp_groups"
#define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
@@ -32,5 +33,6 @@
#define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
#define LR_NAT_RUN_STOPWATCH_NAME "lr_nat_run"
#define LR_STATEFUL_RUN_STOPWATCH_NAME "lr_stateful"
+#define LS_STATEFUL_RUN_STOPWATCH_NAME "ls_stateful"
#endif
diff --git a/northd/automake.mk b/northd/automake.mk
index b886356c9c..a178541759 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -28,6 +28,8 @@ northd_ovn_northd_SOURCES = \
northd/en-lr-nat.h \
northd/en-lr-stateful.c \
northd/en-lr-stateful.h \
+ northd/en-ls-stateful.c \
+ northd/en-ls-stateful.h \
northd/inc-proc-northd.c \
northd/inc-proc-northd.h \
northd/ipam.c \
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index bd2296ac43..b0161b98d9 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -21,6 +21,7 @@
#include "en-lflow.h"
#include "en-lr-nat.h"
#include "en-lr-stateful.h"
+#include "en-ls-stateful.h"
#include "en-northd.h"
#include "en-meters.h"
@@ -44,6 +45,8 @@ lflow_get_input_data(struct engine_node *node,
engine_get_input_data("sync_meters", node);
struct ed_type_lr_stateful *lr_stateful_data =
engine_get_input_data("lr_stateful", node);
+struct ed_type_ls_stateful *ls_stateful_data =
+engine_get_input_data("ls_stateful", node);
lflow_input->nbrec_bfd_table =
EN_OVSDB_GET(engine_get_input("NB_bfd", node));
@@ -67,6 +70,7 @@ lflow_get_input_data(struct engine_node *node,
lflow_input->lr_ports = _data->lr_ports;
lflow_input->ls_port_groups = _data->ls_port_groups;
lflow_input->lr_stateful_table = _stateful_data->table;
+lflow_input->ls_stateful_table = _stateful_data->table;
lflow_input->meter_groups = _meters_data->meter_groups;
lflow_input->lb_datapaths_map = _data->lb_datapaths_map;
lflow_input->svc_monitor_map = _data->svc_monitor_map;
diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c
index 4404caaf93..8665b3c791 100644
--- a/northd/en-lr-stateful.c
+++ b/northd/en-lr-stateful.c
@@ -311,7 +311,12 @@ lr_stateful_lb_data_handler(struct engine_node *node, void
*data_)
struct hmapx_node *hmapx_node;
HMAPX_FOR_EACH (hmapx_node, >trk_data.crupdated) {
-lr_stateful_rebuild_vip_nats(hmapx_node->data);
+struct lr_stateful_record *lr_stateful_rec = hmapx_node->data;
+lr_stateful_rebuild_vip_nats(lr_stateful_rec);
+const struct ovn_datapath *od =
+ovn_datapaths_find_by_index(input_data.lr_datapaths,
+
Re: [ovs-dev] [PATCH ovn v2 2/3] rbac: Restrict IGMP_Group updates to relevant chassis.
Bleep bloop. Greetings Mark Michelson, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 88 characters long (recommended limit is 79) #204 FILE: controller/pinctrl.c:5425: pinctrl.igmp_group_has_chassis_name); Lines checked: 277, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v6 03/13] northd: Generate router's stateful flows using lr_stateful data.
From: Numan Siddique
Previous commits added new engine nodes to store logical router's
stateful (LB and NAT data). Make use of the data stored by these
engine nodes to generate logical flows related to router's LBs and NATs.
Acked-by: Dumitru Ceara
Signed-off-by: Numan Siddique
---
lib/stopwatch-names.h| 1 +
northd/en-lflow.c| 3 -
northd/en-lr-stateful.h | 4 +
northd/inc-proc-northd.c | 1 -
northd/northd.c | 816 +--
northd/northd.h | 1 -
northd/ovn-northd.c | 1 +
7 files changed, 531 insertions(+), 296 deletions(-)
diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index e5e41fbfd8..1e8a5b656f 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -24,6 +24,7 @@
#define LFLOWS_DATAPATHS_STOPWATCH_NAME "lflows_datapaths"
#define LFLOWS_PORTS_STOPWATCH_NAME "lflows_ports"
#define LFLOWS_LBS_STOPWATCH_NAME "lflows_lbs"
+#define LFLOWS_LR_STATEFUL_STOPWATCH_NAME "lflows_lr_stateful"
#define LFLOWS_IGMP_STOPWATCH_NAME "lflows_igmp"
#define LFLOWS_DP_GROUPS_STOPWATCH_NAME "lflows_dp_groups"
#define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 97c7f383cd..bd2296ac43 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -42,8 +42,6 @@ lflow_get_input_data(struct engine_node *node,
engine_get_input_data("port_group", node);
struct sync_meters_data *sync_meters_data =
engine_get_input_data("sync_meters", node);
-struct ed_type_lr_nat_data *lr_nat_data =
-engine_get_input_data("lr_nat", node);
struct ed_type_lr_stateful *lr_stateful_data =
engine_get_input_data("lr_stateful", node);
@@ -68,7 +66,6 @@ lflow_get_input_data(struct engine_node *node,
lflow_input->ls_ports = _data->ls_ports;
lflow_input->lr_ports = _data->lr_ports;
lflow_input->ls_port_groups = _data->ls_port_groups;
-lflow_input->lr_nats = _nat_data->lr_nats;
lflow_input->lr_stateful_table = _stateful_data->table;
lflow_input->meter_groups = _meters_data->meter_groups;
lflow_input->lb_datapaths_map = _data->lb_datapaths_map;
diff --git a/northd/en-lr-stateful.h b/northd/en-lr-stateful.h
index bc3265adac..c6d62cb835 100644
--- a/northd/en-lr-stateful.h
+++ b/northd/en-lr-stateful.h
@@ -73,6 +73,10 @@ struct lr_stateful_table {
#define LR_STATEFUL_TABLE_FOR_EACH(LR_LB_NAT_REC, TABLE) \
HMAP_FOR_EACH (LR_LB_NAT_REC, key_node, &(TABLE)->entries)
+#define LR_STATEFUL_TABLE_FOR_EACH_IN_P(LR_STATEFUL_REC, JOBID, TABLE) \
+HMAP_FOR_EACH_IN_PARALLEL (LR_STATEFUL_REC, key_node, JOBID, \
+ &(TABLE)->entries)
+
struct lr_stateful_tracked_data {
/* Created or updated logical router with LB and/or NAT data. */
struct hmapx crupdated; /* Stores 'struct lr_stateful_record'. */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 97bcce9655..adb38dde78 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -221,7 +221,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(_lflow, _sb_logical_flow, NULL);
engine_add_input(_lflow, _sb_multicast_group, NULL);
engine_add_input(_lflow, _sb_igmp_group, NULL);
-engine_add_input(_lflow, _lr_nat, NULL);
engine_add_input(_lflow, _lr_stateful, NULL);
engine_add_input(_lflow, _northd, lflow_northd_handler);
engine_add_input(_lflow, _port_group, lflow_port_group_handler);
diff --git a/northd/northd.c b/northd/northd.c
index a425a8d7ab..c4b4f7f886 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -8857,18 +8857,14 @@ build_lrouter_groups(struct hmap *lr_ports, struct
ovs_list *lr_list)
*/
static void
build_lswitch_rport_arp_req_self_orig_flow(struct ovn_port *op,
- uint32_t priority,
- struct ovn_datapath *od,
- const struct lr_nat_table *lr_nats,
- struct hmap *lflows)
+uint32_t priority,
+const struct ovn_datapath *od,
+const struct lr_nat_record *lrnat_rec,
+struct hmap *lflows)
{
struct ds eth_src = DS_EMPTY_INITIALIZER;
struct ds match = DS_EMPTY_INITIALIZER;
-const struct lr_nat_record *lrnat_rec = lr_nat_table_find_by_index(
-lr_nats, op->od->index);
-ovs_assert(lrnat_rec);
-
/* Self originated ARP requests/RARP/ND need to be flooded to the L2 domain
* (except on router ports). Determine that packets are self originated
* by also matching on source MAC. Matching on ingress port is not
@@ -8954,10 +8950,11 @@ lrouter_port_ipv6_reachable(const struct ovn_port *op,
* switching domain as regular broadcast.
*/
static void
[ovs-dev] [PATCH ovn v6 02/13] northd: Add a new engine 'lr_stateful' to manage lr's stateful data.
From: Numan Siddique
This new engine now maintains the load balancer and NAT data of a
logical router which was earlier part of northd engine node data.
The main inputs to this engine are:
- northd node
- lr_nat node
- lb_data node
A record for each logical router is maintained in the 'lr_stateful_table'
hmap table and this record
- stores the lb related data
- embeds the 'lr_nat' record.
This engine node becomes an input to 'lflow' node.
Signed-off-by: Numan Siddique
---
lib/stopwatch-names.h| 1 +
northd/automake.mk | 2 +
northd/en-lflow.c| 4 +
northd/en-lr-nat.h | 3 +
northd/en-lr-stateful.c | 659
northd/en-lr-stateful.h | 112 ++
northd/en-sync-sb.c | 54 +--
northd/inc-proc-northd.c | 13 +-
northd/northd.c | 710 ---
northd/northd.h | 146 +++-
northd/ovn-northd.c | 1 +
tests/ovn-northd.at | 62
12 files changed, 1250 insertions(+), 517 deletions(-)
create mode 100644 northd/en-lr-stateful.c
create mode 100644 northd/en-lr-stateful.h
diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index 782d64320a..e5e41fbfd8 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -30,5 +30,6 @@
#define PORT_GROUP_RUN_STOPWATCH_NAME "port_group_run"
#define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
#define LR_NAT_RUN_STOPWATCH_NAME "lr_nat_run"
+#define LR_STATEFUL_RUN_STOPWATCH_NAME "lr_stateful"
#endif
diff --git a/northd/automake.mk b/northd/automake.mk
index a477105470..b886356c9c 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -26,6 +26,8 @@ northd_ovn_northd_SOURCES = \
northd/en-lb-data.h \
northd/en-lr-nat.c \
northd/en-lr-nat.h \
+ northd/en-lr-stateful.c \
+ northd/en-lr-stateful.h \
northd/inc-proc-northd.c \
northd/inc-proc-northd.h \
northd/ipam.c \
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index f1fa92cdfb..97c7f383cd 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -20,6 +20,7 @@
#include "en-lflow.h"
#include "en-lr-nat.h"
+#include "en-lr-stateful.h"
#include "en-northd.h"
#include "en-meters.h"
@@ -43,6 +44,8 @@ lflow_get_input_data(struct engine_node *node,
engine_get_input_data("sync_meters", node);
struct ed_type_lr_nat_data *lr_nat_data =
engine_get_input_data("lr_nat", node);
+struct ed_type_lr_stateful *lr_stateful_data =
+engine_get_input_data("lr_stateful", node);
lflow_input->nbrec_bfd_table =
EN_OVSDB_GET(engine_get_input("NB_bfd", node));
@@ -66,6 +69,7 @@ lflow_get_input_data(struct engine_node *node,
lflow_input->lr_ports = _data->lr_ports;
lflow_input->ls_port_groups = _data->ls_port_groups;
lflow_input->lr_nats = _nat_data->lr_nats;
+lflow_input->lr_stateful_table = _stateful_data->table;
lflow_input->meter_groups = _meters_data->meter_groups;
lflow_input->lb_datapaths_map = _data->lb_datapaths_map;
lflow_input->svc_monitor_map = _data->svc_monitor_map;
diff --git a/northd/en-lr-nat.h b/northd/en-lr-nat.h
index 79390f8db8..16b166ee05 100644
--- a/northd/en-lr-nat.h
+++ b/northd/en-lr-nat.h
@@ -91,6 +91,9 @@ struct lr_nat_table {
const struct lr_nat_record * lr_nat_table_find_by_index(
const struct lr_nat_table *, size_t od_index);
+#define LR_NAT_TABLE_FOR_EACH(LR_NAT_REC, TABLE) \
+HMAP_FOR_EACH (LR_NAT_REC, key_node, &(TABLE)->entries)
+
struct ed_type_lr_nat_data {
struct lr_nat_table lr_nats;
diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c
new file mode 100644
index 00..4404caaf93
--- /dev/null
+++ b/northd/en-lr-stateful.c
@@ -0,0 +1,659 @@
+/*
+ * Copyright (c) 2024, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include
+#include
+#include
+
+/* OVS includes */
+#include "include/openvswitch/hmap.h"
+#include "lib/bitmap.h"
+#include "lib/socket-util.h"
+#include "lib/uuidset.h"
+#include "openvswitch/util.h"
+#include "openvswitch/vlog.h"
+#include "stopwatch.h"
+
+/* OVN includes */
+#include "en-lb-data.h"
+#include "en-lr-nat.h"
+#include "en-lr-stateful.h"
+#include "lib/inc-proc-eng.h"
+#include "lib/lb.h"
+#include "lib/ovn-nb-idl.h"
+#include "lib/ovn-sb-idl.h"
+#include "lib/ovn-util.h"
+#include "lib/stopwatch-names.h"
+#include "northd.h"
+
[ovs-dev] [PATCH ovn v6 01/13] northd: Add a new engine 'lr_nat' to manage lr NAT data.
From: Numan Siddique
This new engine now maintains the NAT related data for each
logical router which was earlier maintained by the northd
engine node in the 'struct ovn_datapath'. The input to
this engine node is 'northd'.
A record for each logical router (lr_nat_record) is maintained
in the 'lr_nats' hmap table which stores the lr's NAT dat.
'northd' engine now reports logical routers changed due to NATs
in its tracking data. 'lr_nat' engine node makes use of
this tracked data in its northd change handler to update the
NAT data.
This engine node becomes an input to 'lflow' node.
Acked-by: Dumitru Ceara
Signed-off-by: Numan Siddique
---
lib/ovn-util.c | 8 +-
lib/ovn-util.h | 3 +-
lib/stopwatch-names.h| 1 +
northd/automake.mk | 2 +
northd/en-lflow.c| 5 +
northd/en-lr-nat.c | 397 ++
northd/en-lr-nat.h | 132 +
northd/en-northd.c | 4 +
northd/en-sync-sb.c | 6 +-
northd/inc-proc-northd.c | 6 +
northd/northd.c | 587 ---
northd/northd.h | 46 +--
northd/ovn-northd.c | 1 +
tests/ovn-northd.at | 46 ++-
14 files changed, 866 insertions(+), 378 deletions(-)
create mode 100644 northd/en-lr-nat.c
create mode 100644 northd/en-lr-nat.h
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index 882e67f939..3e69a25347 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -385,11 +385,17 @@ extract_sbrec_binding_first_mac(const struct
sbrec_port_binding *binding,
}
bool
-lport_addresses_is_empty(struct lport_addresses *laddrs)
+lport_addresses_is_empty(const struct lport_addresses *laddrs)
{
return !laddrs->n_ipv4_addrs && !laddrs->n_ipv6_addrs;
}
+void
+init_lport_addresses(struct lport_addresses *laddrs)
+{
+*laddrs = (struct lport_addresses) { 0 };
+}
+
void
destroy_lport_addresses(struct lport_addresses *laddrs)
{
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 2afff0d074..16e054812c 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -112,7 +112,8 @@ bool extract_sbrec_binding_first_mac(const struct
sbrec_port_binding *binding,
bool extract_lrp_networks__(char *mac, char **networks, size_t n_networks,
struct lport_addresses *laddrs);
-bool lport_addresses_is_empty(struct lport_addresses *);
+bool lport_addresses_is_empty(const struct lport_addresses *);
+void init_lport_addresses(struct lport_addresses *);
void destroy_lport_addresses(struct lport_addresses *);
const char *find_lport_address(const struct lport_addresses *laddrs,
const char *ip_s);
diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index 4e93c1dc14..782d64320a 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -29,5 +29,6 @@
#define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
#define PORT_GROUP_RUN_STOPWATCH_NAME "port_group_run"
#define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
+#define LR_NAT_RUN_STOPWATCH_NAME "lr_nat_run"
#endif
diff --git a/northd/automake.mk b/northd/automake.mk
index 5d77ca67b7..a477105470 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -24,6 +24,8 @@ northd_ovn_northd_SOURCES = \
northd/en-sync-from-sb.h \
northd/en-lb-data.c \
northd/en-lb-data.h \
+ northd/en-lr-nat.c \
+ northd/en-lr-nat.h \
northd/inc-proc-northd.c \
northd/inc-proc-northd.h \
northd/ipam.c \
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 95ee558d53..f1fa92cdfb 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -19,6 +19,7 @@
#include
#include "en-lflow.h"
+#include "en-lr-nat.h"
#include "en-northd.h"
#include "en-meters.h"
@@ -40,6 +41,9 @@ lflow_get_input_data(struct engine_node *node,
engine_get_input_data("port_group", node);
struct sync_meters_data *sync_meters_data =
engine_get_input_data("sync_meters", node);
+struct ed_type_lr_nat_data *lr_nat_data =
+engine_get_input_data("lr_nat", node);
+
lflow_input->nbrec_bfd_table =
EN_OVSDB_GET(engine_get_input("NB_bfd", node));
lflow_input->sbrec_bfd_table =
@@ -61,6 +65,7 @@ lflow_get_input_data(struct engine_node *node,
lflow_input->ls_ports = _data->ls_ports;
lflow_input->lr_ports = _data->lr_ports;
lflow_input->ls_port_groups = _data->ls_port_groups;
+lflow_input->lr_nats = _nat_data->lr_nats;
lflow_input->meter_groups = _meters_data->meter_groups;
lflow_input->lb_datapaths_map = _data->lb_datapaths_map;
lflow_input->svc_monitor_map = _data->svc_monitor_map;
diff --git a/northd/en-lr-nat.c b/northd/en-lr-nat.c
new file mode 100644
index 00..7664ec0ca9
--- /dev/null
+++ b/northd/en-lr-nat.c
@@ -0,0 +1,397 @@
+/*
+ * Copyright (c) 2024, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may
[ovs-dev] [PATCH ovn v6 00/13] northd lflow incremental processing
From: Numan Siddique This patch series adds incremental processing in the lflow engine node to handle changes to northd and other engine nodes. Changed related to load balancers and NAT are mainly handled in this patch series. This patch series can also be found here - https://github.com/numansiddique/ovn/tree/northd_lbnatacl_lflow/v5 Prior to this patch series, most of the changes to northd engine resulted in full recomputation of logical flows. This series aims to improve the performance of ovn-northd by adding the I-P support. In order to add this support, some of the northd engine node data (from struct ovn_datapath) is split and moved over to new engine nodes - mainly related to load balancers, NAT and ACLs. Below are the scale testing results done with these patches applied using ovn-heater. The test ran the scenario - ocp-500-density-heavy.yml [1]. With all the lflow I-P patches applied, the resuts are: --- Min (s) Median (s) 90%ile (s) 99%ile (s) Max (s) Mean (s)Total (s) Count Failed --- Iteration Total 0.1368831.1290161.192001 1.2041671.2127280.66501783.127099 125 0 Namespace.add_ports 0.0052160.0057360.007034 0.0154860.0189780.0062110.776373125 0 WorkerNode.bind_port0.0350300.0460820.052469 0.0582930.0603110.04597311.493259 250 0 WorkerNode.ping_port0.0050570.0067271.047692 1.0692531.0713360.26689666.724094 250 0 --- The results with the present main are: --- Min (s) Median (s) 90%ile (s) 99%ile (s) Max (s) Mean (s)Total (s) Count Failed --- Iteration Total 0.1354912.2238053.311270 3.3390783.3453461.729172216.146495 125 0 Namespace.add_ports 0.0053800.0057440.006819 0.0187730.0208000.0062920.786532125 0 WorkerNode.bind_port0.0341790.0460550.053488 0.0588010.0710430.04611711.529311 250 0 WorkerNode.ping_port0.0049560.0069523.086952 3.1917433.1928070.791544197.886026 250 0 --- Please see the link [2] which has a high level description of the changes done in this patch series. [1] - https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml [2] - https://mail.openvswitch.org/pipermail/ovs-dev/2023-December/410053.html v5 -> v6 -- * Applied the first 3 patches of v5 after addressing all the review comments (and with the Acks) * Rebased to latest main and resolved the conflicts. * Addressed almost all of the review comments received for v5 from Han and Dumitru. - Added detailed documentation on 'struct lflow_ref' and life cycle of 'struct lflow_ref_node'. - Added documentation on the thread safety limitations when using 'struct lflow_ref'. v4 -> v5 --- * Rebased to latest main and resolved the conflicts. * Addressed the review comments from Han in patch 15 (and in p8). Removed the assert if SB dp group is missing and handled it by returning false so that lflow engine recomputes. Added test cases to cover this scenario for both lflows (p8) and SB load balancers (p15) . v3 -> v4 --- * Addressed most of the review comments from Dumitru and Han. * Found a couple of bugs in v3 patch 9 - "northd: Refactor lflow management into a separate module." and addressed them in v4. To brief the issue, if a logical flow L(M, A) is referenced by 2 lflow_ref's which belong to the same datapath, then the lflow was deleted even if one lflow_ref was cleared due to any changes. It is addressed now by maintaining a reference count in
[ovs-dev] [PATCH ovn v2 3/3] rbac: Only allow relevant chassis to update BFD.
This adds a new "chassis_name" column to the BFD table. ovn-northd sets
this to the logical port's chassis name when creating the BFD record.
RBAC has been updated so that chassis may only update their own records.
Signed-off-by: Mark Michelson
---
v1 -> v2:
* Rebased on current main
---
northd/northd.c | 9 -
northd/ovn-northd.c | 2 +-
ovn-sb.ovsschema| 7 ---
ovn-sb.xml | 4
4 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 2a2fab231..51622c302 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -10912,6 +10912,7 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
nbrec_bfd_set_status(nb_bt, "admin_down");
}
+struct ovn_port *op = ovn_port_find(lr_ports, nb_bt->logical_port);
bfd_e = bfd_port_lookup(_only, nb_bt->logical_port, nb_bt->dst_ip);
if (!bfd_e) {
int udp_src = bfd_get_unused_port(bfd_src_ports);
@@ -10925,6 +10926,9 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
sbrec_bfd_set_disc(sb_bt, 1 + random_uint32());
sbrec_bfd_set_src_port(sb_bt, udp_src);
sbrec_bfd_set_status(sb_bt, nb_bt->status);
+if (op && op->sb && op->sb->chassis) {
+sbrec_bfd_set_chassis_name(sb_bt, op->sb->chassis->name);
+}
int min_tx = nb_bt->n_min_tx ? nb_bt->min_tx[0] : BFD_DEF_MINTX;
sbrec_bfd_set_min_tx(sb_bt, min_tx);
@@ -10943,6 +10947,10 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
}
}
build_bfd_update_sb_conf(nb_bt, bfd_e->sb_bt);
+if (op && op->sb && op->sb->chassis &&
+strcmp(op->sb->chassis->name, sb_bt->chassis_name)) {
+sbrec_bfd_set_chassis_name(sb_bt, op->sb->chassis->name);
+}
hmap_remove(_only, _e->hmap_node);
bfd_e->ref = false;
@@ -10951,7 +10959,6 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
hmap_insert(bfd_connections, _e->hmap_node, hash);
}
-struct ovn_port *op = ovn_port_find(lr_ports, nb_bt->logical_port);
if (op) {
op->has_bfd = true;
}
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 90a6d62b1..fdd5939e5 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -122,7 +122,7 @@ static const char *rbac_igmp_group_auth[] =
static const char *rbac_igmp_group_update[] =
{"address", "chassis", "datapath", "ports"};
static const char *rbac_bfd_auth[] =
-{""};
+{"chassis_name"};
static const char *rbac_bfd_update[] =
{"status"};
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index b42f18b04..84ae09515 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Southbound",
-"version": "20.32.0",
-"cksum": "1262133774 31276",
+"version": "20.33.0",
+"cksum": "4076371179 31328",
"tables": {
"SB_Global": {
"columns": {
@@ -578,7 +578,8 @@
"min": 0, "max": "unlimited"}},
"options": {
"type": {"key": "string", "value": "string",
- "min": 0, "max": "unlimited"}}},
+ "min": 0, "max": "unlimited"}},
+"chassis_name": {"type": "string"}},
"indexes": [["logical_port", "dst_ip", "src_port", "disc"]],
"isRoot": true},
"FDB": {
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 2de7228e7..1b18a27a0 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -4989,6 +4989,10 @@ tcp.flags = RST;
receiving system in Asynchronous mode.
+
+The name of the chassis where the logical port is bound.
+
+
Reserved for future use.
--
2.43.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2 2/3] rbac: Restrict IGMP_Group updates to relevant chassis.
RBAC did not restrict which chassis could update IGMP_Groups. With this
change, we add a new "chassis_name" column to IGMP_Group.
This may seem odd since there is already a "chassis" column in
IGMP_Group. But RBAC specifically works by string matching based on the
certificate common name. Therefore, we need to have a chassis_name
string column instead of a chassis UUID column.
Getting RBAC to function properly required me to fix an existing bug as
well. igmp_group_cleanup() did not ensure that only local IGMP group
records were deleted. This presumably meant that when one ovn-controller
in a cluster was shut down, it would delete ALL IGMP_Group records in
the southbound DB, not just the local ones.
Signed-off-by: Mark Michelson
---
v1 -> v2:
* Rebased on top of current main
* Fixed igmp_group_cleanup() to only delete local records.
---
controller/ip-mcast.c | 26 +++---
controller/ip-mcast.h | 9 ++---
controller/ovn-controller.c | 3 ++-
controller/pinctrl.c| 16 +---
northd/ovn-northd.c | 2 +-
ovn-sb.ovsschema| 7 ---
ovn-sb.xml | 5 +
tests/ovn.at| 2 +-
8 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c
index a870fb29e..b457c7e69 100644
--- a/controller/ip-mcast.c
+++ b/controller/ip-mcast.c
@@ -38,7 +38,8 @@ 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);
+ const struct sbrec_chassis *chassis,
+ bool igmp_group_has_chassis_name);
struct ovsdb_idl_index *
igmp_group_index_create(struct ovsdb_idl *idl)
@@ -86,7 +87,8 @@ struct sbrec_igmp_group *
igmp_group_create(struct ovsdb_idl_txn *idl_txn,
const struct in6_addr *address,
const struct sbrec_datapath_binding *datapath,
- const struct sbrec_chassis *chassis)
+ const struct sbrec_chassis *chassis,
+ bool igmp_group_has_chassis_name)
{
char addr_str[INET6_ADDRSTRLEN];
@@ -94,16 +96,18 @@ igmp_group_create(struct ovsdb_idl_txn *idl_txn,
return NULL;
}
-return igmp_group_create_(idl_txn, addr_str, datapath, chassis);
+return igmp_group_create_(idl_txn, addr_str, datapath, chassis,
+ igmp_group_has_chassis_name);
}
struct sbrec_igmp_group *
igmp_mrouter_create(struct ovsdb_idl_txn *idl_txn,
const struct sbrec_datapath_binding *datapath,
-const struct sbrec_chassis *chassis)
+const struct sbrec_chassis *chassis,
+bool igmp_group_has_chassis_name)
{
return igmp_group_create_(idl_txn, OVN_IGMP_GROUP_MROUTERS, datapath,
- chassis);
+ chassis, igmp_group_has_chassis_name);
}
void
@@ -211,7 +215,8 @@ igmp_group_delete(const struct sbrec_igmp_group *g)
bool
igmp_group_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
- struct ovsdb_idl_index *igmp_groups)
+ struct ovsdb_idl_index *igmp_groups,
+ const struct sbrec_chassis *chassis)
{
const struct sbrec_igmp_group *g;
@@ -220,6 +225,9 @@ igmp_group_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
}
SBREC_IGMP_GROUP_FOR_EACH_BYINDEX (g, igmp_groups) {
+if (chassis != g->chassis) {
+continue;
+}
igmp_group_delete(g);
}
@@ -249,13 +257,17 @@ 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)
+ const struct sbrec_chassis *chassis,
+ bool igmp_group_has_chassis_name)
{
struct sbrec_igmp_group *g = sbrec_igmp_group_insert(idl_txn);
sbrec_igmp_group_set_address(g, addr_str);
sbrec_igmp_group_set_datapath(g, datapath);
sbrec_igmp_group_set_chassis(g, chassis);
+if (igmp_group_has_chassis_name) {
+sbrec_igmp_group_set_chassis_name(g, chassis->name);
+}
return g;
}
diff --git a/controller/ip-mcast.h b/controller/ip-mcast.h
index 326f39db1..eebada968 100644
--- a/controller/ip-mcast.h
+++ b/controller/ip-mcast.h
@@ -39,11 +39,13 @@ struct sbrec_igmp_group *igmp_group_create(
struct ovsdb_idl_txn *idl_txn,
const struct in6_addr *address,
const struct sbrec_datapath_binding *datapath,
-const struct sbrec_chassis *chassis);
+const struct sbrec_chassis *chassis,
+bool igmp_group_has_chassis_name);
struct sbrec_igmp_group *igmp_mrouter_create(
struct ovsdb_idl_txn *idl_txn,
[ovs-dev] [PATCH ovn v2 1/3] rbac: Only allow relevant chassis to update service monitors.
Service monitors already had the restriction that chassis could not
insert or delete records. However, there was nothing restricting chassis
from updating records for service monitors that are relevant to other
chassis.
This change adds a new "chassis_name" column to the Service_Monitor
table. ovn-northd will set this column to the chassis on which the
relevant logical port is bound. This way, only that particular chassis
can update the status of the service monitor.
Signed-off-by: Mark Michelson
---
v1 -> v2:
* Rebased on top of currrent main
---
northd/northd.c | 19 +--
northd/ovn-northd.c | 2 +-
ovn-sb.ovsschema| 5 +++--
ovn-sb.xml | 4
4 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index d2091d4bc..2a2fab231 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3799,13 +3799,19 @@ static struct service_monitor_info *
create_or_get_service_mon(struct ovsdb_idl_txn *ovnsb_txn,
struct hmap *monitor_map,
const char *ip, const char *logical_port,
- uint16_t service_port, const char *protocol)
+ uint16_t service_port, const char *protocol,
+ const char *chassis_name)
{
struct service_monitor_info *mon_info =
get_service_mon(monitor_map, ip, logical_port, service_port,
protocol);
if (mon_info) {
+if (chassis_name && strcmp(mon_info->sbrec_mon->chassis_name,
+ chassis_name)) {
+sbrec_service_monitor_set_chassis_name(mon_info->sbrec_mon,
+ chassis_name);
+}
return mon_info;
}
@@ -3820,6 +3826,9 @@ create_or_get_service_mon(struct ovsdb_idl_txn *ovnsb_txn,
sbrec_service_monitor_set_port(sbrec_mon, service_port);
sbrec_service_monitor_set_logical_port(sbrec_mon, logical_port);
sbrec_service_monitor_set_protocol(sbrec_mon, protocol);
+if (chassis_name) {
+sbrec_service_monitor_set_chassis_name(sbrec_mon, chassis_name);
+}
mon_info = xzalloc(sizeof *mon_info);
mon_info->sbrec_mon = sbrec_mon;
hmap_insert(monitor_map, _info->hmap_node, hash);
@@ -3862,12 +3871,18 @@ ovn_lb_svc_create(struct ovsdb_idl_txn *ovnsb_txn,
protocol = "tcp";
}
+const char *chassis_name = NULL;
+if (op->sb && op->sb->chassis) {
+chassis_name = op->sb->chassis->name;
+}
+
struct service_monitor_info *mon_info =
create_or_get_service_mon(ovnsb_txn, monitor_map,
backend->ip_str,
backend_nb->logical_port,
backend->port,
- protocol);
+ protocol,
+ chassis_name);
ovs_assert(mon_info);
sbrec_service_monitor_set_options(
mon_info->sbrec_mon, _vip_nb->lb_health_check->options);
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index dadc1af38..c32a11cbd 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -114,7 +114,7 @@ static const char *rbac_mac_binding_update[] =
{"logical_port", "ip", "mac", "datapath", "timestamp"};
static const char *rbac_svc_monitor_auth[] =
-{""};
+{"chassis_name"};
static const char *rbac_svc_monitor_auth_update[] =
{"status"};
static const char *rbac_igmp_group_auth[] =
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index 72e230b75..1d2b3028d 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Southbound",
-"version": "20.30.0",
-"cksum": "2972392849 31172",
+"version": "20.31.0",
+"cksum": "2473562445 31224",
"tables": {
"SB_Global": {
"columns": {
@@ -509,6 +509,7 @@
"logical_port": {"type": "string"},
"src_mac": {"type": "string"},
"src_ip": {"type": "string"},
+"chassis_name": {"type": "string"},
"status": {
"type": {"key": {"type": "string",
"enum": ["set", ["online", "offline", "error"]]},
diff --git a/ovn-sb.xml b/ovn-sb.xml
index e393f92b3..1f3b318e0 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -4815,6 +4815,10 @@ tcp.flags = RST;
Source IPv4 address to use in the service monitor packet.
+
+The name of the chassis where the logical port is bound.
+
+
The interval, in seconds, between service monitor checks.
--
2.43.0
___
dev mailing list
d...@openvswitch.org
Re: [ovs-dev] [PATCH ovn] tests: Fix grep warning
On Tue, Jan 30, 2024 at 3:35 AM Dumitru Ceara wrote:
>
> On 1/30/24 08:59, Ales Musil wrote:
> > On Tue, Jan 30, 2024 at 8:58 AM Ales Musil wrote:
> >
> >> The Fedora version of grep (grep (GNU grep) 3.11) complains
> >> about the syntax grep "output\:": grep: warning: stray \ before :
> >>
> >> Remove the \ which works also for Ubuntu grep version
> >> (grep (GNU grep) 3.7).
> >>
> >>
> > I forgot to add the Fixes tag.
> >
> > Fixes: 17b6a12fa286 ("ovn-controller: Support VIF-based local encap IPs
> > selection.")
> >
> >
> >> Signed-off-by: Ales Musil
> >> ---
>
> Thanks, I added the "fixes" tag and a dot at the end of the commit
> summary then pushed this to main.
>
> Regards,
> Dumitru
Thanks Ales and Dumitru for the fix.
Han
>
> >> tests/ovn.at | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tests/ovn.at b/tests/ovn.at
> >> index 62966752f..cf87b9ad4 100644
> >> --- a/tests/ovn.at
> >> +++ b/tests/ovn.at
> >> @@ -30435,7 +30435,7 @@ check_packet_tunnel() {
> >> as $hv
> >> echo "vif$src -> vif$dst should go through tunnel $local_encap_ip
->
> >> $remote_encap_ip"
> >> tunnel_ofport=$(ovs-vsctl --bare --column=ofport find interface
> >> options:local_ip=$local_encap_ip options:remote_ip=$remote_encap_ip)
> >> -AT_CHECK([test $(ovs-appctl ofproto/trace br-int in_port=vif$src
> >> $packet | grep "output\:" | awk -F ':' '{ print $2 }') ==
$tunnel_ofport])
> >> +AT_CHECK([test $(ovs-appctl ofproto/trace br-int in_port=vif$src
> >> $packet | grep "output:" | awk -F ':' '{ print $2 }') ==
$tunnel_ofport])
> >> }
> >>
> >> for i in 1 2; do
> >> --
> >> 2.43.0
> >>
> >>
> >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC PATCH 10/10] python: ovs: flowviz: Add datapath graph format.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote: > Graph view leverages the tree format (specially the tree-based > filtering) and uses graphviz library to build a visual graph of the > datapath in graphviz format. > > Conntrack zones are shown in random colors to help visualize connection > tracking interdependencies. > > An html flag builds an HTML page with both the html flows and the graph > (in svg) that enables navegation. The code looks good, maybe add an example? I tried the html output, and also something like; ovs-flowviz --style light -i ~/dpctl.txt datapath graph | dot -Grankdir=LR -Tpng -o graph.png Acked-by: Eelco Chaudron > Signed-off-by: Adrian Moreno > --- ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC PATCH 09/10] python: ovs: flowviz: Add datapath html format.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote: > Using the existing FlowTree and HTMLFormatter, create an HTML tree > visualization that also supports collapsing and expanding entire flow > trees and subtrees. This looks good to me, maybe add an example in the commit message. Acked-by: Eelco Chaudron > Signed-off-by: Adrian Moreno > --- ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC PATCH 08/10] python: ovs: flowviz: Add Openflow cookie format.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote:
> When anaylizing OVN issues, it might be useful to see what OpenFlow
> flows were generated from each logical flow. In order to make it simpler
> to visualize this, add a cookie format that simply sorts the flows first
> by cookie, then by table.
The code looks good to me, however, did not try with ovs-detrace. One comment
on code that needs to move to the previous patch.
Maybe add an example in the commit message.
Acked-by: Eelco Chaudron
> Signed-off-by: Adrian Moreno
> ---
> python/ovs/flowviz/ofp/cli.py | 57 -
> python/ovs/flowviz/ofp/logic.py | 63 -
> 2 files changed, 118 insertions(+), 2 deletions(-)
>
> diff --git a/python/ovs/flowviz/ofp/cli.py b/python/ovs/flowviz/ofp/cli.py
> index 6b1435ea1..9658d00d3 100644
> --- a/python/ovs/flowviz/ofp/cli.py
> +++ b/python/ovs/flowviz/ofp/cli.py
> @@ -18,7 +18,7 @@ import click
>
> from ovs.flowviz.main import maincli
> from ovs.flowviz.ofp.html import HTMLProcessor
> -from ovs.flowviz.ofp.logic import LogicFlowProcessor
> +from ovs.flowviz.ofp.logic import CookieProcessor, LogicFlowProcessor
> from ovs.flowviz.process import (
> OpenFlowFactory,
> JSONProcessor,
> @@ -182,6 +182,61 @@ def logic(
> processor.print(show_flows)
>
>
> +@openflow.command()
> +@click.option(
> +"-d",
> +"--ovn-detrace",
> +"ovn_detrace_flag",
> +is_flag=True,
> +show_default=True,
> +help="Use ovn-detrace to extract cookie information",
> +)
> +@click.option(
> +"--ovn-detrace-path",
> +default="/usr/bin",
> +type=click.Path(),
> +help="Use an alternative path to where ovn_detrace.py is located. "
> +"Instead of using this option you can just set PYTHONPATH accordingly",
> +show_default=True,
> +callback=ovn_detrace_callback,
> +)
> +@click.option(
> +"--ovnnb-db",
> +default=os.getenv("OVN_NB_DB") or "unix:/var/run/ovn/ovnnb_db.sock",
> +help="Specify the OVN NB database string (implies -d). "
> +"If the OVN_NB_DB environment variable is set, it's used as default. "
> +"Otherwise, the default is unix:/var/run/ovn/ovnnb_db.sock",
> +callback=ovn_detrace_callback,
> +)
> +@click.option(
> +"--ovnsb-db",
> +default=os.getenv("OVN_SB_DB") or "unix:/var/run/ovn/ovnsb_db.sock",
> +help="Specify the OVN NB database string (implies -d). "
> +"If the OVN_NB_DB environment variable is set, it's used as default. "
> +"Otherwise, the default is unix:/var/run/ovn/ovnnb_db.sock",
> +callback=ovn_detrace_callback,
> +)
> +@click.option(
> +"-o",
> +"--ovn-filter",
> +help="Specify a filter to be run on ovn-detrace information (implied
> -d). "
> +"Format: python regular expression "
> +"(see https://docs.python.org/3/library/re.html)",
> +callback=ovn_detrace_callback,
> +)
> +@click.pass_obj
> +def cookie(
> +opts, ovn_detrace_flag, ovn_detrace_path, ovnnb_db, ovnsb_db, ovn_filter
> +):
> +"""Print the flow tables sorted by cookie."""
> +if ovn_detrace_flag:
> +opts["ovn_detrace_flag"] = True
> +
> +processor = CookieProcessor(opts)
> +processor.process()
> +processor.print()
> +
> +
> @openflow.command()
> @click.pass_obj
> def html(opts):
> diff --git a/python/ovs/flowviz/ofp/logic.py b/python/ovs/flowviz/ofp/logic.py
> index cb4568cf1..9d244d137 100644
> --- a/python/ovs/flowviz/ofp/logic.py
> +++ b/python/ovs/flowviz/ofp/logic.py
> @@ -200,7 +200,7 @@ class LogicFlowProcessor(OpenFlowFactory, FileProcessor):
> if len(self.heat_map) > 0 and len(table.values()) > 0:
> for i, field in enumerate(self.heat_map):
> (min_val, max_val) = self.min_max[name][i]
> -self.console.style.set_value_style(
> +formatter.style.set_value_style(
This probably needs to move to patch 7.
> field, heat_pallete(min_val, max_val)
> )
>
> @@ -301,3 +301,64 @@ cookie_style_gen = hash_pallete(
> saturation=[0.5],
> value=[0.5 + x / 10 * (0.85 - 0.5) for x in range(0, 10)],
> )
> +
> +
> +class CookieProcessor(OpenFlowFactory, FileProcessor):
> +"""Processor that sorts flows into cookies and tables."""
> +
> +def __init__(self, opts):
> +super().__init__(opts)
> +self.data = dict()
> +self.ovn_detrace = (
> +OVNDetrace(opts) if opts.get("ovn_detrace_flag") else None
> +)
> +
> +def start_file(self, name, filename):
> +self.cookies = dict()
> +
> +def stop_file(self, name, filename):
> +self.data[name] = self.cookies
> +
> +def process_flow(self, flow, name):
> +"""Sort the flows by table and logical flow."""
> +cookie = flow.info.get("cookie") or 0
> +if not self.cookies.get(cookie):
> +self.cookies[cookie] = dict()
> +
> +table
Re: [ovs-dev] [RFC PATCH 07/10] python: ovs: flowviz: Add OpenFlow logical view.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote: > This view is interesting for debugging the logical pipeline. It arranges > the flows in "logical" groups (not to be confused with OVN's > Logical_Flows). A logical group of flows is a set of flows that: > - Have the same table number and priority > - Match on the same fields (regardless of the value they match against) > - Have the same actions, regardless of the arguments for tose actions, > except for output and recirc, for which arguments do care. > > Optionally, the cookie can also be force to be unique for the logical > group. By doing so, we can extend the information we show by querying an > external OVN database and running "ovn-detrace" on each cookie. The > result is a compact list of flow groups with interlieved OVN > information. > > Furthermore, if connected to an OVN database, we can apply an OVN > regexp filter. > > Examples: > $ ovs-ofctl dump-flows br-int | ovs-flowviz openflow logic > $ ovs-ofctl dump-flows br-int | ovs-flowviz openflow logic -s -h > $ export OVN_NB_DB=... > $ export OVN_SB_DB=... > $ ovs-ofctl dump-flows br-int | ovs-flowviz openflow logic -d > $ ovs-ofctl dump-flows br-int | ovs-flowviz openflow logic -d > --ovn-filter="acl.*icmp4" When trying some random trace I got the following: $ ovs-flowviz -i ~/ofctl.txt -l "drop" --style=dark openflow logic -h ╭─╮ │ Filename: /home/echaudron/ofctl.txt │ ╰─╯ Traceback (most recent call last): File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/bin/ovs-flowviz", line 20, in main.main() File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/ovs/flowviz/main.py", line 196, in main maincli() File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py", line 1157, in __call__ return self.main(*args, **kwargs) ^^ File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py", line 1078, in main rv = self.invoke(ctx) File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py", line 1688, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) ^^^ File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py", line 1688, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) ^^^ File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py", line 1434, in invoke return ctx.invoke(self.callback, **ctx.params) ^^^ File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/core.py", line 783, in invoke return __callback(*args, **kwargs) ^^^ File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/click/decorators.py", line 45, in new_func return f(get_current_context().obj, *args, **kwargs) ^ File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/ovs/flowviz/ofp/cli.py", line 182, in logic processor.print(show_flows) File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/ovs/flowviz/ofp/logic.py", line 203, in print self.console.style.set_value_style( AttributeError: 'LogicFlowProcessor' object has no attribute 'console' This gets solved if I apply the next patch, see the comment in the next patch. The code itself looks good, I did not test with the actual ovs-detrace tool. > Signed-off-by: Adrian Moreno > --- > python/automake.mk | 4 +- > python/ovs/flowviz/ofp/cli.py | 113 >
Re: [ovs-dev] [RFC PATCH 07/10] python: ovs: flowviz: Add OpenFlow logical view.
On 30 Jan 2024, at 16:52, Eelco Chaudron wrote: > On 1 Dec 2023, at 20:14, Adrian Moreno wrote: > >> Datapath flows can be arranged into a "tree"-like structure based on >> recirculation ids, e.g: >> >> recirc(0),eth(...),ipv4(...) actions=ct,recirc(0x42) >>\-> recirc(42),ct_state(0/0),eth(...),ipv4(...) actions=1 >>\-> recirc(42),ct_state(1/0),eth(...),ipv4(...) actions=userspace(...) >> >> This patch adds support for building such logical datapath trees in a >> format-agnostic way and adds support for console-based formatting >> supporting: >> - head-maps formatting of statistics >> - hash-based pallete of recirculation ids: each recirculation id is >> assigned a unique color to easily follow the sequence of related >> actions. >> - full-tree filtering: if a user specifies a filter, an entire subtree >> is filtered out if none of its branches satisfy it. >> >> Signed-off-by: Adrian Moreno > > This patch looks good to me. One small nit on a comment not ending with a dot. > > Acked-by: Eelco Chaudron > Ignore this email, it was supposed to be a reply to patch 6 :( ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC PATCH 06/10] python: ovs: flowviz: Add datapath tree format.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote: > Datapath flows can be arranged into a "tree"-like structure based on > recirculation ids, e.g: > > recirc(0),eth(...),ipv4(...) actions=ct,recirc(0x42) >\-> recirc(42),ct_state(0/0),eth(...),ipv4(...) actions=1 >\-> recirc(42),ct_state(1/0),eth(...),ipv4(...) actions=userspace(...) > > This patch adds support for building such logical datapath trees in a > format-agnostic way and adds support for console-based formatting > supporting: > - head-maps formatting of statistics > - hash-based pallete of recirculation ids: each recirculation id is > assigned a unique color to easily follow the sequence of related > actions. > - full-tree filtering: if a user specifies a filter, an entire subtree > is filtered out if none of its branches satisfy it. > > Signed-off-by: Adrian Moreno This patch looks good to me. One small nit on a comment not ending with a dot. Acked-by: Eelco Chaudron > --- > python/automake.mk | 1 + > python/ovs/flowviz/console.py | 22 +++ > python/ovs/flowviz/odp/cli.py | 21 ++- > python/ovs/flowviz/odp/tree.py | 290 + > 4 files changed, 332 insertions(+), 2 deletions(-) > create mode 100644 python/ovs/flowviz/odp/tree.py > > diff --git a/python/automake.mk b/python/automake.mk > index b4c1f84be..5050089e9 100644 > --- a/python/automake.mk > +++ b/python/automake.mk > @@ -71,6 +71,7 @@ ovs_flowviz = \ > python/ovs/flowviz/main.py \ > python/ovs/flowviz/odp/__init__.py \ > python/ovs/flowviz/odp/cli.py \ > + python/ovs/flowviz/odp/tree.py \ > python/ovs/flowviz/ofp/__init__.py \ > python/ovs/flowviz/ofp/cli.py \ > python/ovs/flowviz/ofp/html.py \ > diff --git a/python/ovs/flowviz/console.py b/python/ovs/flowviz/console.py > index 5b4b047c2..2d65f9bb6 100644 > --- a/python/ovs/flowviz/console.py > +++ b/python/ovs/flowviz/console.py > @@ -13,6 +13,9 @@ > # limitations under the License. > > import colorsys > +import itertools > +import zlib > + > from rich.console import Console > from rich.text import Text > from rich.style import Style > @@ -169,6 +172,25 @@ def heat_pallete(min_value, max_value): > return heat > > > +def hash_pallete(hue, saturation, value): > +"""Generates a color pallete with the cartesian product > +of the hsv values provided and returns a callable that assigns a color > for > +each value hash > +""" > +HSV_tuples = itertools.product(hue, saturation, value) > +RGB_tuples = map(lambda x: colorsys.hsv_to_rgb(*x), HSV_tuples) > +styles = [ > +Style(color=Color.from_rgb(r * 255, g * 255, b * 255)) > +for r, g, b in RGB_tuples > +] > + > +def get_style(string): > +hash_val = zlib.crc32(bytes(str(string), "utf-8")) > +return styles[hash_val % len(styles)] > + > +return get_style > + > + > def default_highlight(): > """Generates a default style for highlights.""" > return Style(underline=True) > diff --git a/python/ovs/flowviz/odp/cli.py b/python/ovs/flowviz/odp/cli.py > index 78f5cfff4..4740e753e 100644 > --- a/python/ovs/flowviz/odp/cli.py > +++ b/python/ovs/flowviz/odp/cli.py > @@ -13,12 +13,12 @@ > # limitations under the License. > > import click > - > from ovs.flowviz.main import maincli > +from ovs.flowviz.odp.tree import ConsoleTreeProcessor > from ovs.flowviz.process import ( > DatapathFactory, > -JSONProcessor, > ConsoleProcessor, > +JSONProcessor, > ) > > > @@ -65,3 +65,20 @@ def console(opts, heat_map): > ) > proc.process() > proc.print() > + > + > +@datapath.command() > +@click.option( > +"-h", > +"--heat-map", > +is_flag=True, > +default=False, > +show_default=True, > +help="Create heat-map with packet and byte counters", > +) > +@click.pass_obj > +def tree(opts, heat_map): > +"""Print the flows in a tree based on the 'recirc_id'.""" > +processor = ConsoleTreeProcessor(opts) > +processor.process() > +processor.print(heat_map) > diff --git a/python/ovs/flowviz/odp/tree.py b/python/ovs/flowviz/odp/tree.py > new file mode 100644 > index 0..cfddb162e > --- /dev/null > +++ b/python/ovs/flowviz/odp/tree.py > @@ -0,0 +1,290 @@ > +# Copyright (c) 2023 Red Hat, Inc. > +# > +# Licensed under the Apache License, Version 2.0 (the "License"); > +# you may not use this file except in compliance with the License. > +# You may obtain a copy of the License at: > +# > +# http://www.apache.org/licenses/LICENSE-2.0 > +# > +# Unless required by applicable law or agreed to in writing, software > +# distributed under the License is distributed on an "AS IS" BASIS, > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > +# See the License for the specific language governing permissions and > +# limitations under the License. > + > +from rich.style import Style > +from rich.text import Text > +from rich.tree import Tree > + >
Re: [ovs-dev] [RFC PATCH 07/10] python: ovs: flowviz: Add OpenFlow logical view.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote: > Datapath flows can be arranged into a "tree"-like structure based on > recirculation ids, e.g: > > recirc(0),eth(...),ipv4(...) actions=ct,recirc(0x42) >\-> recirc(42),ct_state(0/0),eth(...),ipv4(...) actions=1 >\-> recirc(42),ct_state(1/0),eth(...),ipv4(...) actions=userspace(...) > > This patch adds support for building such logical datapath trees in a > format-agnostic way and adds support for console-based formatting > supporting: > - head-maps formatting of statistics > - hash-based pallete of recirculation ids: each recirculation id is > assigned a unique color to easily follow the sequence of related > actions. > - full-tree filtering: if a user specifies a filter, an entire subtree > is filtered out if none of its branches satisfy it. > > Signed-off-by: Adrian Moreno This patch looks good to me. One small nit on a comment not ending with a dot. Acked-by: Eelco Chaudron > --- > python/automake.mk | 1 + > python/ovs/flowviz/console.py | 22 +++ > python/ovs/flowviz/odp/cli.py | 21 ++- > python/ovs/flowviz/odp/tree.py | 290 + > 4 files changed, 332 insertions(+), 2 deletions(-) > create mode 100644 python/ovs/flowviz/odp/tree.py > > diff --git a/python/automake.mk b/python/automake.mk > index b4c1f84be..5050089e9 100644 > --- a/python/automake.mk > +++ b/python/automake.mk > @@ -71,6 +71,7 @@ ovs_flowviz = \ > python/ovs/flowviz/main.py \ > python/ovs/flowviz/odp/__init__.py \ > python/ovs/flowviz/odp/cli.py \ > + python/ovs/flowviz/odp/tree.py \ > python/ovs/flowviz/ofp/__init__.py \ > python/ovs/flowviz/ofp/cli.py \ > python/ovs/flowviz/ofp/html.py \ > diff --git a/python/ovs/flowviz/console.py b/python/ovs/flowviz/console.py > index 5b4b047c2..2d65f9bb6 100644 > --- a/python/ovs/flowviz/console.py > +++ b/python/ovs/flowviz/console.py > @@ -13,6 +13,9 @@ > # limitations under the License. > > import colorsys > +import itertools > +import zlib > + > from rich.console import Console > from rich.text import Text > from rich.style import Style > @@ -169,6 +172,25 @@ def heat_pallete(min_value, max_value): > return heat > > > +def hash_pallete(hue, saturation, value): > +"""Generates a color pallete with the cartesian product > +of the hsv values provided and returns a callable that assigns a color > for > +each value hash > +""" > +HSV_tuples = itertools.product(hue, saturation, value) > +RGB_tuples = map(lambda x: colorsys.hsv_to_rgb(*x), HSV_tuples) > +styles = [ > +Style(color=Color.from_rgb(r * 255, g * 255, b * 255)) > +for r, g, b in RGB_tuples > +] > + > +def get_style(string): > +hash_val = zlib.crc32(bytes(str(string), "utf-8")) > +return styles[hash_val % len(styles)] > + > +return get_style > + > + > def default_highlight(): > """Generates a default style for highlights.""" > return Style(underline=True) > diff --git a/python/ovs/flowviz/odp/cli.py b/python/ovs/flowviz/odp/cli.py > index 78f5cfff4..4740e753e 100644 > --- a/python/ovs/flowviz/odp/cli.py > +++ b/python/ovs/flowviz/odp/cli.py > @@ -13,12 +13,12 @@ > # limitations under the License. > > import click > - > from ovs.flowviz.main import maincli > +from ovs.flowviz.odp.tree import ConsoleTreeProcessor > from ovs.flowviz.process import ( > DatapathFactory, > -JSONProcessor, > ConsoleProcessor, > +JSONProcessor, > ) > > > @@ -65,3 +65,20 @@ def console(opts, heat_map): > ) > proc.process() > proc.print() > + > + > +@datapath.command() > +@click.option( > +"-h", > +"--heat-map", > +is_flag=True, > +default=False, > +show_default=True, > +help="Create heat-map with packet and byte counters", > +) > +@click.pass_obj > +def tree(opts, heat_map): > +"""Print the flows in a tree based on the 'recirc_id'.""" > +processor = ConsoleTreeProcessor(opts) > +processor.process() > +processor.print(heat_map) > diff --git a/python/ovs/flowviz/odp/tree.py b/python/ovs/flowviz/odp/tree.py > new file mode 100644 > index 0..cfddb162e > --- /dev/null > +++ b/python/ovs/flowviz/odp/tree.py > @@ -0,0 +1,290 @@ > +# Copyright (c) 2023 Red Hat, Inc. > +# > +# Licensed under the Apache License, Version 2.0 (the "License"); > +# you may not use this file except in compliance with the License. > +# You may obtain a copy of the License at: > +# > +# http://www.apache.org/licenses/LICENSE-2.0 > +# > +# Unless required by applicable law or agreed to in writing, software > +# distributed under the License is distributed on an "AS IS" BASIS, > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > +# See the License for the specific language governing permissions and > +# limitations under the License. > + > +from rich.style import Style > +from rich.text import Text > +from rich.tree import Tree > + >
Re: [ovs-dev] [PATCH ovn branch-23.09] ovs: Bump submodule to tip of OVS branch-3.2.
On Tue, Jan 30, 2024 at 7:38 AM Dumitru Ceara wrote: > > This picks up the following relevant commit: > cd8ffc956c3c ovs-atomic: Fix inclusion of Clang header by GCC 14. > > Without this builds on Fedora 40 (rawhide) are broken due to failing to > compile the submodule. > > Signed-off-by: Dumitru Ceara Acked-by: Numan Siddique Numan > --- > ovs | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ovs b/ovs > index ec1d730163..49e64f13b2 16 > --- a/ovs > +++ b/ovs > @@ -1 +1 @@ > -Subproject commit ec1d730163d984934c467e050ebf6d39f8c09384 > +Subproject commit 49e64f13b2c965f5b53a65eeab70ac2e3f0bf69a > -- > 2.39.3 > > ___ > 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] [RFC PATCH 05/10] python: ovs: flowviz: Add html formatting.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote:
> Add a HTML Formatter and use it to print OpenFlow flows in an HTML list
> with table links.
>
> Signed-off-by: Adrian Moreno
No real comments from my side, this looks good! Maybe add an example to the
commit message.
One small potential addition request below?
Acked-by: Eelco Chaudron
> ---
> python/automake.mk | 3 +-
> python/ovs/flowviz/html_format.py | 136
> python/ovs/flowviz/ofp/cli.py | 10 ++
> python/ovs/flowviz/ofp/html.py | 80
> python/ovs/flowviz/ovs-flowviz.conf | 16 +++-
> 5 files changed, 243 insertions(+), 2 deletions(-)
> create mode 100644 python/ovs/flowviz/html_format.py
> create mode 100644 python/ovs/flowviz/ofp/html.py
>
> diff --git a/python/automake.mk b/python/automake.mk
> index cf8b71659..b4c1f84be 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -67,15 +67,16 @@ ovs_flowviz = \
> python/ovs/flowviz/__init__.py \
> python/ovs/flowviz/console.py \
> python/ovs/flowviz/format.py \
> + python/ovs/flowviz/html_format.py \
> python/ovs/flowviz/main.py \
> python/ovs/flowviz/odp/__init__.py \
> python/ovs/flowviz/odp/cli.py \
> python/ovs/flowviz/ofp/__init__.py \
> python/ovs/flowviz/ofp/cli.py \
> + python/ovs/flowviz/ofp/html.py \
> python/ovs/flowviz/ovs-flowviz \
> python/ovs/flowviz/process.py
>
> -
> # These python files are used at build time but not runtime,
> # so they are not installed.
> EXTRA_DIST += \
> diff --git a/python/ovs/flowviz/html_format.py
> b/python/ovs/flowviz/html_format.py
> new file mode 100644
> index 0..ebfa65c34
> --- /dev/null
> +++ b/python/ovs/flowviz/html_format.py
> @@ -0,0 +1,136 @@
> +# Copyright (c) 2023 Red Hat, Inc.
> +#
> +# Licensed under the Apache License, Version 2.0 (the "License");
> +# you may not use this file except in compliance with the License.
> +# You may obtain a copy of the License at:
> +#
> +# http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> +# See the License for the specific language governing permissions and
> +# limitations under the License.
> +
> +from ovs.flowviz.format import FlowFormatter, FlowBuffer, FlowStyle
> +
> +
> +class HTMLStyle:
> +"""HTMLStyle defines a style for html-formatted flows.
> +
> +Args:
> +color(str): Optional; a string representing the CSS color to use
> +anchor_gen(callable): Optional; a callable to be used to generate the
> +href
> +"""
> +
> +def __init__(self, color=None, anchor_gen=None):
> +self.color = color
> +self.anchor_gen = anchor_gen
> +
> +
> +class HTMLBuffer(FlowBuffer):
> +"""HTMLBuffer implementes FlowBuffer to provide html-based flow
> formatting.
> +
> +Each flow gets formatted as:
> +...
> +"""
> +
> +def __init__(self):
> +self._text = ""
> +
> +@property
> +def text(self):
> +return self._text
> +
> +def _append(self, string, color, href):
> +"""Append a key a string"""
> +style = ' style="color:{}"'.format(color) if color else ""
> +self._text += "".format(style)
> +if href:
> +self._text += "".format(href)
> +self._text += string
> +if href:
> +self._text += ""
> +self._text += ""
> +
> +def append_key(self, kv, style):
> +"""Append a key.
> +Args:
> +kv (KeyValue): the KeyValue instance to append
> +style (HTMLStyle): the style to use
> +"""
> +href = style.anchor_gen(kv) if (style and style.anchor_gen) else ""
> +return self._append(
> +kv.meta.kstring, style.color if style else "", href
> +)
> +
> +def append_delim(self, kv, style):
> +"""Append a delimiter.
> +Args:
> +kv (KeyValue): the KeyValue instance to append
> +style (HTMLStyle): the style to use
> +"""
> +href = style.anchor_gen(kv) if (style and style.anchor_gen) else ""
> +return self._append(kv.meta.delim, style.color if style else "",
> href)
> +
> +def append_end_delim(self, kv, style):
> +"""Append an end delimiter.
> +Args:
> +kv (KeyValue): the KeyValue instance to append
> +style (HTMLStyle): the style to use
> +"""
> +href = style.anchor_gen(kv) if (style and style.anchor_gen) else ""
> +return self._append(
> +kv.meta.end_delim, style.color if style else "", href
> +)
> +
> +def append_value(self, kv, style):
> +"""Append a value.
> +Args:
> +kv (KeyValue): the KeyValue
Re: [ovs-dev] [RFC PATCH 04/10] python: ovs: flowviz: Add default config file.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote: > It has two basic styles defined: "dark" and "light" intended for > dark and light terminals. This patch looks fine to me. Should we maybe add an example in the commit message on how to use the --style option? Cheers, Eelco > Signed-off-by: Adrian Moreno Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC PATCH 03/10] python: ovs: flowviz: Add console formatting.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote:
> Add a flow formatting framework and one implementation for console
> printing using rich.
>
> The flow formatting framework is a simple set of classes that can be
> used to write different flow formatting implementations. It supports
> styles to be described by any class, highlighting and config-file based
> style definition.
>
> The first flow formatting implementation is also introduced: the
> ConsoleFormatter. It uses the an advanced rich-text printing library
> [1].
>
> The console printing supports:
> - Heatmap: printing the packet/byte statistics of each flow in a color
> that represents its relative size: blue (low) -> red (high).
> - Printing a banner with the file name and alias.
> - Extensive style definition via config file.
>
> This console format is added to both OpenFlow and Datapath flows.
>
> Examples:
> - Highlight drops in datapath flows:
> $ ovs-flowviz -i flows.txt --highlight "drop" datapath console
> - Quickly detect where most packets are going using heatmap and
> paginated output:
> $ ovs-ofctl dump-flows br-int | ovs-flowviz -p openflow console -h
-p is not supported.
>
> [1] https://rich.readthedocs.io/en/stable/introduction.html
>
> Signed-off-by: Adrian Moreno
> ---
When trying to install this using the python setup, I get the following error:
running install_data
warning: install_data: setup script did not provide a directory for
'ovs/flowviz/ovs-flowviz.conf' -- installing right in
'/home/echaudron/Documents/review/ovs_adrian_tools/python-venv'
error: can't copy 'ovs/flowviz/ovs-flowviz.conf': doesn't exist or not a
regular file
[end of output]
Guess you need to move this file from patch 4 to 3.
Also, note that rich is not installed as a dependency.
Below are some small nits, but the code looks good.
Cheers,
Eelco
> python/automake.mk| 2 +
> python/ovs/flowviz/console.py | 174
> python/ovs/flowviz/format.py | 372 ++
> python/ovs/flowviz/main.py| 57 +-
> python/ovs/flowviz/odp/cli.py | 25 +++
> python/ovs/flowviz/ofp/cli.py | 26 +++
> python/ovs/flowviz/process.py | 87 +++-
> python/setup.py | 5 +-
> 8 files changed, 738 insertions(+), 10 deletions(-)
> create mode 100644 python/ovs/flowviz/console.py
> create mode 100644 python/ovs/flowviz/format.py
>
> diff --git a/python/automake.mk b/python/automake.mk
> index 4845565b8..8d82cb4a8 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -65,6 +65,8 @@ ovs_pytests = \
>
> ovs_flowviz = \
> python/ovs/flowviz/__init__.py \
> + python/ovs/flowviz/console.py \
> + python/ovs/flowviz/format.py \
> python/ovs/flowviz/main.py \
> python/ovs/flowviz/odp/__init__.py \
> python/ovs/flowviz/odp/cli.py \
> diff --git a/python/ovs/flowviz/console.py b/python/ovs/flowviz/console.py
> new file mode 100644
> index 0..5b4b047c2
> --- /dev/null
> +++ b/python/ovs/flowviz/console.py
> @@ -0,0 +1,174 @@
> +# Copyright (c) 2023 Red Hat, Inc.
> +#
> +# Licensed under the Apache License, Version 2.0 (the "License");
> +# you may not use this file except in compliance with the License.
> +# You may obtain a copy of the License at:
> +#
> +# http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> +# See the License for the specific language governing permissions and
> +# limitations under the License.
> +
> +import colorsys
In other files you have a cr/lf between import and from.
> +from rich.console import Console
> +from rich.text import Text
> +from rich.style import Style
> +from rich.color import Color
> +from rich.panel import Panel
> +from rich.emoji import Emoji
> +
> +from ovs.flowviz.format import FlowFormatter, FlowBuffer, FlowStyle
> +
> +
> +def file_header(name):
> +return Panel(
> +Text(
> +Emoji.replace(":scroll:")
> ++ " "
> ++ name
> ++ " "
> ++ Emoji.replace(":scroll:"),
> +style="bold",
> +justify="center",
> +)
> +)
> +
> +
> +class ConsoleBuffer(FlowBuffer):
> +"""ConsoleBuffer implements FlowBuffer to provide console-based text
> +formatting based on rich.Text.
> +
> +Append functions accept a rich.Style.
> +
> +Args:
> +rtext(rich.Text): Optional; text instance to reuse
> +"""
> +
> +def __init__(self, rtext):
> +self._text = rtext or Text()
> +
> +@property
> +def text(self):
> +return self._text
> +
> +def _append(self, string, style):
> +"""Append to internal text."""
> +return self._text.append(string, style)
> +
> +def append_key(self, kv, style):
> +"""Append a
Re: [ovs-dev] [RFC PATCH 02/10] python: ovs: flowviz: Add file processing infra.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote:
> process.py contains a useful base class that processes files
> odp.py and ofp.py: contain datapath and openflow subcommand definitions
> as well as the first formatting option: json.
>
> Also, this patch adds basic filtering support.
>
> Examples:
> $ ovs-ofctl dump-flows br-int | ovs-flowviz openflow json
> $ ovs-ofctl dump-flows br-int > flows.txt && ovs-flowviz -i flows.txt
> openflow json
> $ ovs-ofctl appctl dpctl/dump-flows | ovs-flowviz -f 'ct' datapath json
> $ ovs-ofctl appctl dpctl/dump-flows > flows.txt && ovs-flowviz -i flows.txt
> -f 'drop' datapath json
Some small comments below!
> Signed-off-by: Adrian Moreno
> ---
> python/automake.mk | 5 +-
> python/ovs/flowviz/__init__.py | 2 +
> python/ovs/flowviz/main.py | 103 +-
> python/ovs/flowviz/odp/cli.py | 42
> python/ovs/flowviz/ofp/cli.py | 42
> python/ovs/flowviz/process.py | 192 +
> 6 files changed, 384 insertions(+), 2 deletions(-)
> create mode 100644 python/ovs/flowviz/odp/cli.py
> create mode 100644 python/ovs/flowviz/ofp/cli.py
> create mode 100644 python/ovs/flowviz/process.py
>
> diff --git a/python/automake.mk b/python/automake.mk
> index 4302f0136..4845565b8 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -67,8 +67,11 @@ ovs_flowviz = \
> python/ovs/flowviz/__init__.py \
> python/ovs/flowviz/main.py \
> python/ovs/flowviz/odp/__init__.py \
> + python/ovs/flowviz/odp/cli.py \
> python/ovs/flowviz/ofp/__init__.py \
> - python/ovs/flowviz/ovs-flowviz
> + python/ovs/flowviz/ofp/cli.py \
> + python/ovs/flowviz/ovs-flowviz \
> + python/ovs/flowviz/process.py
>
>
> # These python files are used at build time but not runtime,
> diff --git a/python/ovs/flowviz/__init__.py b/python/ovs/flowviz/__init__.py
> index e69de29bb..898dba522 100644
> --- a/python/ovs/flowviz/__init__.py
> +++ b/python/ovs/flowviz/__init__.py
> @@ -0,0 +1,2 @@
> +import ovs.flowviz.ofp.cli # noqa: F401
> +import ovs.flowviz.odp.cli # noqa: F401
> diff --git a/python/ovs/flowviz/main.py b/python/ovs/flowviz/main.py
> index a2d5ca1fa..a45c06e48 100644
> --- a/python/ovs/flowviz/main.py
> +++ b/python/ovs/flowviz/main.py
> @@ -12,19 +12,67 @@
> # See the License for the specific language governing permissions and
> # limitations under the License.
>
> +import os
> +
> import click
Do we maybe want all imports together and sorted, i.e.:
import click
import os
> +from ovs.flow.filter import OFFilter
> +
>
> class Options(dict):
> """Options dictionary"""
>
>
> +def validate_input(ctx, param, value):
> +"""Validate the "-i" option"""
> +result = list()
> +for input_str in value:
> +parts = input_str.strip().split(",")
> +if len(parts) == 2:
> +file_parts = tuple(parts)
> +elif len(parts) == 1:
> +file_parts = tuple(["Filename: " + parts[0], parts[0]])
> +else:
> +raise click.BadParameter(
> +"input filename should have the following format: "
> +"[alias,]FILENAME"
> +)
> +
> +if not os.path.isfile(file_parts[1]):
> +raise click.BadParameter(
> +"input filename %s does not exist" % file_parts[1]
> +)
> +result.append(file_parts)
> +return result
> +
> +
> @click.group(
> subcommand_metavar="TYPE",
> context_settings=dict(help_option_names=["-h", "--help"]),
> )
> +@click.option(
> +"-i",
> +"--input",
> +"filename",
> +help="Read flows from specified filepath. If not provided, flows will be"
> +" read from stdin. This option can be specified multiple times."
> +" Format [alias,]FILENAME. Where alias is a name that shall be used to"
> +" refer to this FILENAME",
> +multiple=True,
> +type=click.Path(),
> +callback=validate_input,
> +)
> +@click.option(
> +"-f",
> +"--filter",
> +help="Filter flows that match the filter expression."
> +"Run 'ovs-flowviz filter' for a detailed description of the filtering "
> +"syntax",
> +type=str,
> +show_default=False,
> +)
> @click.pass_context
> -def maincli(ctx):
> +def maincli(ctx, filename, filter):
> """
> OpenvSwitch flow visualization utility.
>
> @@ -32,6 +80,59 @@ def maincli(ctx):
> (such as the output of ovs-ofctl dump-flows or ovs-appctl
> dpctl/dump-flows)
> and prints them in different formats.
> """
> +ctx.obj = Options()
> +ctx.obj["filename"] = filename or None
> +if filter:
> +try:
> +ctx.obj["filter"] = OFFilter(filter)
> +except Exception as e:
> +raise click.BadParameter("Wrong filter syntax: {}".format(e))
> +
> +
> +@maincli.command(hidden=True)
> +@click.pass_context
> +def filter(ctx):
> +"""
> +\b
> +Filter Syntax
> +*
> +
>
Re: [ovs-dev] [RFC PATCH 01/10] python: ovs: Add flowviz scheleton.
On 1 Dec 2023, at 20:14, Adrian Moreno wrote: > Add a new python package (just the scheleton for now) to hold a flow > visualization tool based on the flow parsing library. Thanks for this series, and sorry for the late review :( Maybe I'm doing something wrong, but doing 'the pip install .' in the python directory does not complain, or install the click dependency. (python-venv) [ebuild:~/..._adrian_tools/python]$ ovs-flowviz Traceback (most recent call last): File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/bin/ovs-flowviz", line 17, in from ovs.flowviz import main File "/home/echaudron/Documents/review/ovs_adrian_tools/python-venv/lib64/python3.11/site-packages/ovs/flowviz/main.py", line 15, in import click ModuleNotFoundError: No module named 'click' The same for netaddr/pyparsing needed by the flow library. In addition, two small style comments below. > Signed-off-by: Adrian Moreno > --- > python/automake.mk | 12 +++-- > python/ovs/flowviz/__init__.py | 0 > python/ovs/flowviz/main.py | 41 ++ > python/ovs/flowviz/odp/__init__.py | 0 > python/ovs/flowviz/ofp/__init__.py | 0 > python/ovs/flowviz/ovs-flowviz | 20 +++ > python/setup.py| 11 +--- > 7 files changed, 79 insertions(+), 5 deletions(-) > create mode 100644 python/ovs/flowviz/__init__.py > create mode 100644 python/ovs/flowviz/main.py > create mode 100644 python/ovs/flowviz/odp/__init__.py > create mode 100644 python/ovs/flowviz/ofp/__init__.py > create mode 100755 python/ovs/flowviz/ovs-flowviz > > diff --git a/python/automake.mk b/python/automake.mk > index 84cf2eab5..4302f0136 100644 > --- a/python/automake.mk > +++ b/python/automake.mk > @@ -63,6 +63,14 @@ ovs_pytests = \ > python/ovs/tests/test_odp.py \ > python/ovs/tests/test_ofp.py > > +ovs_flowviz = \ > + python/ovs/flowviz/__init__.py \ > + python/ovs/flowviz/main.py \ > + python/ovs/flowviz/odp/__init__.py \ > + python/ovs/flowviz/ofp/__init__.py \ > + python/ovs/flowviz/ovs-flowviz > + > + > # These python files are used at build time but not runtime, > # so they are not installed. > EXTRA_DIST += \ > @@ -81,7 +89,7 @@ EXTRA_DIST += \ > # C extension support. > EXTRA_DIST += python/ovs/_json.c > > -PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) $(ovs_pytests) > +PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) > $(ovs_pytests) $(ovs_flowviz) > > EXTRA_DIST += $(PYFILES) > PYCOV_CLEAN_FILES += $(PYFILES:.py=.py,cover) > @@ -95,7 +103,7 @@ FLAKE8_PYFILES += \ > python/ovs/dirs.py.template \ > python/setup.py > > -nobase_pkgdata_DATA = $(ovs_pyfiles) $(ovstest_pyfiles) > +nobase_pkgdata_DATA = $(ovs_pyfiles) $(ovstest_pyfiles) $(ovs_flowviz) > ovs-install-data-local: > $(MKDIR_P) python/ovs > sed \ > diff --git a/python/ovs/flowviz/__init__.py b/python/ovs/flowviz/__init__.py > new file mode 100644 > index 0..e69de29bb > diff --git a/python/ovs/flowviz/main.py b/python/ovs/flowviz/main.py > new file mode 100644 > index 0..a2d5ca1fa > --- /dev/null > +++ b/python/ovs/flowviz/main.py > @@ -0,0 +1,41 @@ > +# Copyright (c) 2023 Red Hat, Inc. > +# > +# Licensed under the Apache License, Version 2.0 (the "License"); > +# you may not use this file except in compliance with the License. > +# You may obtain a copy of the License at: > +# > +# http://www.apache.org/licenses/LICENSE-2.0 > +# > +# Unless required by applicable law or agreed to in writing, software > +# distributed under the License is distributed on an "AS IS" BASIS, > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > +# See the License for the specific language governing permissions and > +# limitations under the License. > + > +import click > + > + > +class Options(dict): > +"""Options dictionary""" > + > + > +@click.group( > +subcommand_metavar="TYPE", > +context_settings=dict(help_option_names=["-h", "--help"]), > +) > +@click.pass_context > +def maincli(ctx): > +""" > +OpenvSwitch flow visualization utility. > + > +It reads openflow and datapath flows > +(such as the output of ovs-ofctl dump-flows or ovs-appctl > dpctl/dump-flows) > +and prints them in different formats. > +""" > + > + > +def main(): > +""" > +Main Function > +""" > +maincli() > diff --git a/python/ovs/flowviz/odp/__init__.py > b/python/ovs/flowviz/odp/__init__.py > new file mode 100644 > index 0..e69de29bb > diff --git a/python/ovs/flowviz/ofp/__init__.py > b/python/ovs/flowviz/ofp/__init__.py > new file mode 100644 > index 0..e69de29bb > diff --git a/python/ovs/flowviz/ovs-flowviz b/python/ovs/flowviz/ovs-flowviz > new file mode 100755 > index 0..9d0959812 > --- /dev/null > +++ b/python/ovs/flowviz/ovs-flowviz > @@ -0,0 +1,20 @@ > +#!/usr/bin/env python3 > +# > +#
Re: [ovs-dev] [RFC PATCH 00/10] Add flow visualization utility
On 1 Dec 2023, at 20:14, Adrian Moreno wrote: > This series introduces a python utility called ovs-flowviz. > > The goal of this utility is to read both datapath and Openflow flows > (using the flow library already available) and print them in different > formats and styles to make it easier to understand them and troubleshoot > issues. > > The formats are quite opinionated and so are the colors chosen so I'm > eager to hear what is the impression caused to the community. > > Here is a summary of the formats and features supported: > > - Openflow >- Console: Prints flows back to the console allowing filtering and > extensive formatting. >- Cookie: Arranges flows based on cookie (instead of table) to ease > visualization of OVN-generated flows. >- HTML: Prints an HTML file that shows the flows arranged by tables. > resubmit actions have a hyperlinke to the target table to ease > navegation. >- Logic: Many times OVN generates many "logically-equivalent" flows. > These are flows that have the same structure: match on the same > values and have the same actions. The only thing that changes is > the actual value they match against and maybe the arguments of the > actions. This format collapses these flows so you can visualize the > logical pipeline easier. >- JSON: JSON format. > > More OpenFlow features: >- OVN metadata: Both the "cookie" and the "logic" format allow to > connect to a running OVN NB/SB databases and enrich the flows with > OVN context based on the flow's cookie. > > - Datapath: >- Console: Prints flows back to the console allowing filtering and > extensive formatting. >- Tree: Datapath flows use recirculation to further execute > additional actions. This format builds a tree of flows following > the recirculation identifiers and prints it in the console. >- HTML: Prints the datapath flow tree in HTML. It includes some > minimal JS to support expanding and collapsing of entire branches. >- Graph: Following the "tree" format, this one prints the tree in > graphviz format. >- JSON: JSON format. > > Additional datapath features: >- Many datapath formats are based on the tree flow hierarchy. An > interesting feature of this structure is that filtering is done at > the branch level. This means that when a flow satisfies the filter, > the entire sub-tree leading to that flow is shown. > > Additional common features: >- Styling: Styles for both console and HTML formats can be defined > using a configuration file. >- Heat maps: Some formats support heat-maps. A color pallete ranging > from blue (cold) to red (hot) is used to print the number of > packets and bytes of the flows. That way, the flows that are > handling more traffic are much easier to spot > > I know this is a long series, so it's OK if it takes time to review. I'm > specially interested to know what is found useful and what is worth > removing (if any). Thanks Adrian, for submitting the series and I think the quality of the code is good enough to submit it as a non-RFC. Most of the comments I have are style nits. The only main thing missing is some documentation/man pages for the tool. I’ll reply to the individual patches with comments. Cheers, Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] northd: Add qos packet marking.
Bleep bloop. Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 94 characters long (recommended limit is 79) WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator #2521 FILE: utilities/ovn-nbctl.c:286: qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP] [mark=MARK]\n\ Lines checked: 2601, Warnings: 4, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v8 2/2] rhel: Enable USDT scripts by default in Fedora builds.
Ilya Maximets writes:
> On 1/25/24 21:55, Aaron Conole wrote:
>> All supported versions of Fedora do package libbpf, so it
>> makes sense to enable USDT support.
>>
>> Signed-off-by: Aaron Conole
>> ---
>> v8: Include the correct devel package as a dependency
>>
>> rhel/openvswitch-fedora.spec.in | 8
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/rhel/openvswitch-fedora.spec.in
>> b/rhel/openvswitch-fedora.spec.in
>> index 5d24ebcda8..b97f509cae 100644
>> --- a/rhel/openvswitch-fedora.spec.in
>> +++ b/rhel/openvswitch-fedora.spec.in
>> @@ -28,6 +28,8 @@
>> %bcond_with dpdk
>> # To disable AF_XDP support, specify '--without afxdp' when building
>> %bcond_without afxdp
>> +# To control the USDT support
>> +%bcond_without usdt
>>
>> # If there is a need to automatically enable the package after installation,
>> # specify the "--with autoenable"
>> @@ -77,6 +79,9 @@ Provides: %{name}-dpdk = %{version}-%{release}
>> %if %{with afxdp}
>> BuildRequires: libxdp-devel libbpf-devel numactl-devel
>> %endif
>> +%if %{with afxdp}
>
> Did you mean usdt ?
Ugh... yes. I'll post the update. Thanks for catching it.
>> +BuildRequires: libbpf-devel systemtap-sdt-devel
>> +%endif
>> BuildRequires: unbound unbound-devel
>>
>> Requires: openssl hostname iproute module-init-tools unbound
>> @@ -173,6 +178,9 @@ This package provides IPsec tunneling support for OVS
>> tunnels.
>> --enable-afxdp \
>> %else
>> --disable-afxdp \
>> +%endif
>> +%if %{with usdt}
>> +--enable-usdt-probes \
>> %endif
>> --enable-ssl \
>> --disable-static \
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-controller: Stop dropping bind_vport requests immediately after handling.
Bleep bloop. Greetings Mohammad Heib, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: The subject, ': ', is over 70 characters, i.e., 77. Subject: ovn-controller: Stop dropping bind_vport requests immediately after handling. Lines checked: 148, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] github: Update versions of action dependencies (Node.js 20).
On 1/30/24 13:07, Ilya Maximets wrote: > checkout@v3, cache@v3, setup-python@v4 and setup-go@v3 are using > outdated Node.js 16 which is now deprecated in GHA [1], so these > actions may stop working soon. > > Updating to most recent major versions with Node.js 20. This stops > GHA from throwing warnings in every build. > > [1] > https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/ > > While at it also updating upload-artifact and download-artifact to > the latest versions. > > Removing versions from the upload-artifact comment, since the > behavior doesn't seem to change much between versions. > > New setup-go@v5 attempts to cache dependencies by default. However, > the default path it uses is go.sum in the root directory. This > triggers a warning, since the file doesn't exist: > > Restore cache failed: Dependencies file is not found in > /home/runner/work/ovn-kubernetes/ovn-kubernetes. > Supported file pattern: go.sum > > Specify a path to all .sum files we have in the repository to make > the setup-go happy. This should in theory make the builds a touch > faster. This change is in line with recent changes in ovn-kubernetes > itself. > > Signed-off-by: Ilya Maximets > --- > .github/workflows/containers.yml | 2 +- > .../workflows/ovn-fake-multinode-tests.yml| 26 +++--- > .github/workflows/ovn-kubernetes.yml | 19 +- > .github/workflows/test.yml| 36 +-- > 4 files changed, 42 insertions(+), 41 deletions(-) Recheck-request: github-robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v3] ovn-ic: Fix global blacklist filter for IPv6 addresses.
This commit fixes the prefix filter function as the return condition for IPv6
addresses is disabling the advertisement of all learned prefixes regardless of
the match with the blacklist or not.
Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2046804
Fixes: 57b347c55168 ("ovn-ic: Route advertisement.")
Signed-off-by: Roberto Bartzen Acosta
---
ic/ovn-ic.c | 22 ---
tests/ovn-ic.at | 100
2 files changed, 116 insertions(+), 6 deletions(-)
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 6f8f5734d..1c9c9ae2c 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -1024,6 +1024,20 @@ prefix_is_link_local(struct in6_addr *prefix, unsigned
int plen)
((prefix->s6_addr[1] & 0xc0) == 0x80));
}
+static bool
+compare_ipv6_prefixes(const struct in6_addr *s_prefix,
+ const struct in6_addr *d_prefix2, int plen)
+{
+struct in6_addr mask = ipv6_create_mask(plen);
+for (int i = 0; i <= (plen / 8); i++) {
+if ((s_prefix->s6_addr[i] & mask.s6_addr[i]) ^
+(d_prefix2->s6_addr[i] & mask.s6_addr[i])) {
+return false;
+}
+}
+return true;
+}
+
static bool
prefix_is_black_listed(const struct smap *nb_options,
struct in6_addr *prefix,
@@ -1064,12 +1078,8 @@ prefix_is_black_listed(const struct smap *nb_options,
continue;
}
} else {
-struct in6_addr mask = ipv6_create_mask(bl_plen);
-for (int i = 0; i < 16 && mask.s6_addr[i] != 0; i++) {
-if ((prefix->s6_addr[i] & mask.s6_addr[i])
-!= (bl_prefix.s6_addr[i] & mask.s6_addr[i])) {
-continue;
-}
+if (!compare_ipv6_prefixes(prefix, _prefix, bl_plen)) {
+continue;
}
}
matched = true;
diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
index d4c436f84..1f9df71e9 100644
--- a/tests/ovn-ic.at
+++ b/tests/ovn-ic.at
@@ -1274,3 +1274,103 @@ OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-ic -- route sync -- IPv6 blacklist filter])
+AT_KEYWORDS([IPv6-route-sync-blacklist])
+
+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
+# Enable blacklist single filter for IPv6
+ovn-nbctl set nb_global . options:ic-route-blacklist="2003:db8:1::/64,\
+2004:::/32,2005:1234::/21"
+
+OVS_WAIT_UNTIL([ovn-nbctl show | grep ts1])
+
+# 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
+
+# Create blacklisted LRPs and connect to TS
+ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext$i \
+11:11:11:11:11:1$i 2003:db8:1::$i/64
+
+ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \
+22:22:22:22:22:2$i 2004::bbb::$i/48
+
+# filtered by 2005:1234::/21 - (2005:1000: - 2005:17ff:)
+ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext3$i \
+33:33:33:33:33:3$i 2005:1734:5678::$i/50
+
+# additional not filtered prefix -> different subnet bits
+ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext4$i \
+33:33:33:33:33:3$i 2005:1834:5678::$i/50
+
+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
+2005:1834:5678::/50 2001:db8:1::2
+])
+
+for i in 1 2; do
+ovn_as az$i
+
+# Drop blacklist
+ovn-nbctl remove nb_global . options ic-route-blacklist
+
+done
+
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 |
+awk '/learned/{print $1, $2}' | sort ], [0], [dnl
+2002:db8:1::/64 2001:db8:1::2
+2003:db8:1::/64 2001:db8:1::2
+2004::bbb::/48 2001:db8:1::2
+2005:1734:5678::/50 2001:db8:1::2
+2005:1834:5678::/50 2001:db8:1::2
+])
+
+for i in 1 2; do
+ovn_as az$i
+
+ovn-nbctl set nb_global . \
+options:ic-route-blacklist="2003:db8:1::/64,2004:db8:1::/64"
+
+# Create an 'extra' blacklisted LRP and connect to TS
+ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \
+44:44:44:44:44:4$i 2004:db8:1::$i/64
+
+done
+
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 |
+awk '/learned/{print $1, $2}' | sort ], [0], [dnl
+2002:db8:1::/64 2001:db8:1::2
+2004::bbb::/48 2001:db8:1::2
Re: [ovs-dev] [PATCH ovn v2] ovn-ic: fix global blacklist filter for IPv6 addresses
Bleep bloop. Greetings Roberto Bartzen Acosta, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: The subject summary should start with a capital. WARNING: The subject summary should end with a dot. Subject: ovn-ic: fix global blacklist filter for IPv6 addresses ERROR: Author Roberto Bartzen Acosta needs to sign off. Lines checked: 166, Warnings: 2, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ovn-controller: Stop dropping bind_vport requests immediately after handling.
ovn-controller immediately removes the vport_bindings requests that were
generated by VIFs after handling them locally, this approach is intended
to avoid binding the vport to one VIF only and allocate the vport
between the different VIFs that exist in the vport:virtual-parents.
Although the behavior mentioned above is correct, in some cases when the
SB Database is busy the transaction that binds this vport to the desired
VIF/chassis can fail and the controller will not re-try to bind the
vport again because we deleted the bind_vport request in the previous
loop/TXN.
This patch aims to change the above behavior by storing the bind_vport
requests for a bit longer time and this is done by the following:
1. add relevancy_time for each new bind_vport request and
mark this request as new.
2. loop0: ovn-controller will try to handle this bind_vport request
for the first time as usual (no change).
3. loop0: ovn-controller will try to delete the already handled bind_vport
request as usual but first, it will check if this request is marked as
new and
if the relevancy_time is still valid if so the controller will mark this
request as an old request and keep it, otherwise remove it.
4.loop1: ovn-controller will try to commit the same change again for
the old request, if the previous commit in loop0 succeeded the
change will not have any effect on SB, otherwise we will try to
commit the same vport_bind request again.
5. loop1: delete the old bind_vport request.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1954659
Signed-off-by: Mohammad Heib
---
controller/pinctrl.c | 58 +++-
1 file changed, 52 insertions(+), 6 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index bd3bd3d81..152962448 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -6519,10 +6519,52 @@ struct put_vport_binding {
uint32_t vport_key;
uint32_t vport_parent_key;
+
+/* This vport record Only relevant if "relevancy_time"
+ * is earlier than the current_time, "new_record" is true.
+ */
+long long int relevancy_time;
+bool new_record;
};
/* Contains "struct put_vport_binding"s. */
static struct hmap put_vport_bindings;
+/* the relevance time in ms of vport record before deleteing. */
+#define VPORT_RELEVANCE_TIME 1500
+
+/*
+ * Validate if the vport_binding record that was added
+ * by the pinctrl thread is still relevant and needs
+ * to be updated in the SBDB or not.
+ *
+ * vport_binding record is only relevant and needs to be updated in SB if:
+ * 1. The put_vport_binding:relevancy_time still valid.
+ * 2. The put_vport_binding:new_record is true:
+ * The new_record will be set to "true" when this vport record is created
+ * by function "pinctrl_handle_bind_vport".
+ *
+ * After the first attempt to bind this vport to the chassis and
+ * virtual_parent by function "run_put_vport_bindings" we will set the
+ * value of vpb:new_record to "false" and keep it in "put_vport_bindings"
+ *
+ * After the second attempt of binding the vpb it will be removed by
+ * this function.
+ *
+ * The above guarantees that we will try to bind the vport twice in
+ * a certain amount of time.
+ *
+*/
+static bool
+is_vport_binding_relevant(struct put_vport_binding *vpb)
+{
+long long int cur_time = time_msec();
+
+if (vpb->new_record && vpb->relevancy_time > cur_time) {
+vpb->new_record = false;
+return true;
+}
+return false;
+}
static void
init_put_vport_bindings(void)
@@ -6531,18 +6573,21 @@ init_put_vport_bindings(void)
}
static void
-flush_put_vport_bindings(void)
+flush_put_vport_bindings(bool force_flush)
{
struct put_vport_binding *vport_b;
-HMAP_FOR_EACH_POP (vport_b, hmap_node, _vport_bindings) {
-free(vport_b);
+HMAP_FOR_EACH_SAFE (vport_b, hmap_node, _vport_bindings) {
+if (!is_vport_binding_relevant(vport_b) || force_flush) {
+hmap_remove(_vport_bindings, _b->hmap_node);
+free(vport_b);
+}
}
}
static void
destroy_put_vport_bindings(void)
{
-flush_put_vport_bindings();
+flush_put_vport_bindings(true);
hmap_destroy(_vport_bindings);
}
@@ -6620,7 +6665,7 @@ run_put_vport_bindings(struct ovsdb_idl_txn
*ovnsb_idl_txn,
sbrec_port_binding_by_key, chassis, vpb);
}
-flush_put_vport_bindings();
+flush_put_vport_bindings(false);
}
/* Called with in the pinctrl_handler thread context. */
@@ -6658,7 +6703,8 @@ pinctrl_handle_bind_vport(
vpb->dp_key = dp_key;
vpb->vport_key = vport_key;
vpb->vport_parent_key = vport_parent_key;
-
+vpb->new_record = true;
+vpb->relevancy_time = time_msec() + VPORT_RELEVANCE_TIME;
notify_pinctrl_main();
}
--
2.34.3
___
dev mailing list
[ovs-dev] [PATCH ovn v2] ovn-ic: fix global blacklist filter for IPv6 addresses
This commit fixes the prefix filter function as the return condition for IPv6
addresses is disabling the advertisement of all learned prefixes regardless of
the match with the blacklist or not.
Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2046804
Fixes: 57b347c55168 ("ovn-ic: Route advertisement.")
---
ic/ovn-ic.c | 22 ---
tests/ovn-ic.at | 100
2 files changed, 116 insertions(+), 6 deletions(-)
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 6f8f5734d..1c9c9ae2c 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -1024,6 +1024,20 @@ prefix_is_link_local(struct in6_addr *prefix, unsigned
int plen)
((prefix->s6_addr[1] & 0xc0) == 0x80));
}
+static bool
+compare_ipv6_prefixes(const struct in6_addr *s_prefix,
+ const struct in6_addr *d_prefix2, int plen)
+{
+struct in6_addr mask = ipv6_create_mask(plen);
+for (int i = 0; i <= (plen / 8); i++) {
+if ((s_prefix->s6_addr[i] & mask.s6_addr[i]) ^
+(d_prefix2->s6_addr[i] & mask.s6_addr[i])) {
+return false;
+}
+}
+return true;
+}
+
static bool
prefix_is_black_listed(const struct smap *nb_options,
struct in6_addr *prefix,
@@ -1064,12 +1078,8 @@ prefix_is_black_listed(const struct smap *nb_options,
continue;
}
} else {
-struct in6_addr mask = ipv6_create_mask(bl_plen);
-for (int i = 0; i < 16 && mask.s6_addr[i] != 0; i++) {
-if ((prefix->s6_addr[i] & mask.s6_addr[i])
-!= (bl_prefix.s6_addr[i] & mask.s6_addr[i])) {
-continue;
-}
+if (!compare_ipv6_prefixes(prefix, _prefix, bl_plen)) {
+continue;
}
}
matched = true;
diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
index d4c436f84..1f9df71e9 100644
--- a/tests/ovn-ic.at
+++ b/tests/ovn-ic.at
@@ -1274,3 +1274,103 @@ OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-ic -- route sync -- IPv6 blacklist filter])
+AT_KEYWORDS([IPv6-route-sync-blacklist])
+
+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
+# Enable blacklist single filter for IPv6
+ovn-nbctl set nb_global . options:ic-route-blacklist="2003:db8:1::/64,\
+2004:::/32,2005:1234::/21"
+
+OVS_WAIT_UNTIL([ovn-nbctl show | grep ts1])
+
+# 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
+
+# Create blacklisted LRPs and connect to TS
+ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext$i \
+11:11:11:11:11:1$i 2003:db8:1::$i/64
+
+ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \
+22:22:22:22:22:2$i 2004::bbb::$i/48
+
+# filtered by 2005:1234::/21 - (2005:1000: - 2005:17ff:)
+ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext3$i \
+33:33:33:33:33:3$i 2005:1734:5678::$i/50
+
+# additional not filtered prefix -> different subnet bits
+ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext4$i \
+33:33:33:33:33:3$i 2005:1834:5678::$i/50
+
+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
+2005:1834:5678::/50 2001:db8:1::2
+])
+
+for i in 1 2; do
+ovn_as az$i
+
+# Drop blacklist
+ovn-nbctl remove nb_global . options ic-route-blacklist
+
+done
+
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 |
+awk '/learned/{print $1, $2}' | sort ], [0], [dnl
+2002:db8:1::/64 2001:db8:1::2
+2003:db8:1::/64 2001:db8:1::2
+2004::bbb::/48 2001:db8:1::2
+2005:1734:5678::/50 2001:db8:1::2
+2005:1834:5678::/50 2001:db8:1::2
+])
+
+for i in 1 2; do
+ovn_as az$i
+
+ovn-nbctl set nb_global . \
+options:ic-route-blacklist="2003:db8:1::/64,2004:db8:1::/64"
+
+# Create an 'extra' blacklisted LRP and connect to TS
+ovn-nbctl lrp-add lr$i lrp-lr$i-p-ext2$i \
+44:44:44:44:44:4$i 2004:db8:1::$i/64
+
+done
+
+OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr1 |
+awk '/learned/{print $1, $2}' | sort ], [0], [dnl
+2002:db8:1::/64 2001:db8:1::2
+2004::bbb::/48 2001:db8:1::2
+2005:1734:5678::/50 2001:db8:1::2
[ovs-dev] [PATCH ovn branch-23.09] ovs: Bump submodule to tip of OVS branch-3.2.
This picks up the following relevant commit: cd8ffc956c3c ovs-atomic: Fix inclusion of Clang header by GCC 14. Without this builds on Fedora 40 (rawhide) are broken due to failing to compile the submodule. Signed-off-by: Dumitru Ceara --- ovs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ovs b/ovs index ec1d730163..49e64f13b2 16 --- a/ovs +++ b/ovs @@ -1 +1 @@ -Subproject commit ec1d730163d984934c467e050ebf6d39f8c09384 +Subproject commit 49e64f13b2c965f5b53a65eeab70ac2e3f0bf69a -- 2.39.3 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] github: Update versions of action dependencies (Node.js 20).
checkout@v3, cache@v3, setup-python@v4 and setup-go@v3 are using
outdated Node.js 16 which is now deprecated in GHA [1], so these
actions may stop working soon.
Updating to most recent major versions with Node.js 20. This stops
GHA from throwing warnings in every build.
[1]
https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/
While at it also updating upload-artifact and download-artifact to
the latest versions.
Removing versions from the upload-artifact comment, since the
behavior doesn't seem to change much between versions.
New setup-go@v5 attempts to cache dependencies by default. However,
the default path it uses is go.sum in the root directory. This
triggers a warning, since the file doesn't exist:
Restore cache failed: Dependencies file is not found in
/home/runner/work/ovn-kubernetes/ovn-kubernetes.
Supported file pattern: go.sum
Specify a path to all .sum files we have in the repository to make
the setup-go happy. This should in theory make the builds a touch
faster. This change is in line with recent changes in ovn-kubernetes
itself.
Signed-off-by: Ilya Maximets
---
.github/workflows/containers.yml | 2 +-
.../workflows/ovn-fake-multinode-tests.yml| 26 +++---
.github/workflows/ovn-kubernetes.yml | 19 +-
.github/workflows/test.yml| 36 +--
4 files changed, 42 insertions(+), 41 deletions(-)
diff --git a/.github/workflows/containers.yml b/.github/workflows/containers.yml
index bdd118087..87e28d645 100644
--- a/.github/workflows/containers.yml
+++ b/.github/workflows/containers.yml
@@ -20,7 +20,7 @@ jobs:
matrix:
distro: [ fedora, ubuntu ]
steps:
- - uses: actions/checkout@v3
+ - uses: actions/checkout@v4
- name: Update APT cache
run: sudo apt update
diff --git a/.github/workflows/ovn-fake-multinode-tests.yml
b/.github/workflows/ovn-fake-multinode-tests.yml
index b3ba82a30..179c1d662 100644
--- a/.github/workflows/ovn-fake-multinode-tests.yml
+++ b/.github/workflows/ovn-fake-multinode-tests.yml
@@ -26,7 +26,7 @@ jobs:
XDG_RUNTIME_DIR: ''
steps:
- name: Check out ovn-fake-multi-node
- uses: actions/checkout@v3
+ uses: actions/checkout@v4
with:
repository: 'ovn-org/ovn-fake-multinode'
path: 'ovn-fake-multinode'
@@ -36,14 +36,14 @@ jobs:
# ovn-fake-multinode builds and installs ovs from ovn-fake-multinode/ovs
# and it builds and installs ovn from ovn-fake-multinode/ovn. It uses the
ovs submodule for ovn compilation.
- name: Check out ovs master
- uses: actions/checkout@v3
+ uses: actions/checkout@v4
with:
path: 'ovn-fake-multinode/ovs'
repository: 'openvswitch/ovs'
ref: 'master'
- name: Check out ovn ${{ matrix.cfg.branch }}
- uses: actions/checkout@v3
+ uses: actions/checkout@v4
with:
path: 'ovn-fake-multinode/ovn'
submodules: recursive
@@ -63,7 +63,7 @@ jobs:
sudo podman save ovn/ovn-multi-node:${{ matrix.cfg.branch }} >
/tmp/_output/ovn_${{ matrix.cfg.branch }}_image.tar
working-directory: ovn-fake-multinode
-- uses: actions/upload-artifact@v3
+- uses: actions/upload-artifact@v4
with:
name: test-${{ matrix.cfg.branch }}-image
path: /tmp/_output/ovn_${{ matrix.cfg.branch }}_image.tar
@@ -100,7 +100,7 @@ jobs:
steps:
- name: Check out ovn
- uses: actions/checkout@v3
+ uses: actions/checkout@v4
- name: install required dependencies
run: |
@@ -112,11 +112,11 @@ jobs:
. .ci/linux-util.sh
free_up_disk_space_ubuntu
-- uses: actions/download-artifact@v3
+- uses: actions/download-artifact@v4
with:
name: test-main-image
-- uses: actions/download-artifact@v3
+- uses: actions/download-artifact@v4
with:
name: test-branch-22.03-image
@@ -126,7 +126,7 @@ jobs:
sudo podman load --input ovn_branch-22.03_image.tar
- name: Check out ovn-fake-multi-node
- uses: actions/checkout@v3
+ uses: actions/checkout@v4
with:
repository: 'ovn-org/ovn-fake-multinode'
path: 'ovn-fake-multinode'
@@ -156,12 +156,12 @@ jobs:
echo "$HOME/.local/bin" >> $GITHUB_PATH
- name: set up python
- uses: actions/setup-python@v4
+ uses: actions/setup-python@v5
with:
python-version: '3.12'
- name: Check out ovn
- uses: actions/checkout@v3
+ uses: actions/checkout@v4
with:
path: 'ovn'
submodules: recursive
@@ -190,9 +190,9 @@ jobs:
- name: copy logs on failure
if: failure() || cancelled()
run: |
-# upload-artifact@v3 throws exceptions if it tries to upload socket
+# upload-artifact throws exceptions if it tries to upload socket
# files and we could have some socket files
Re: [ovs-dev] [PATCH ovn v2] ovn-ic: Handle NB:name updates properly.
On 1/23/24 14:40, Mohammad Heib wrote: > When the user updates the NB_GLOBAL.name after registering > to IC Databases if the user already has defined chassis as a gateway > that will cause ovn-ic instance to run in an infinity loop trying > to update the gateways and insert the current gateway to the SB.chassis > tables as a remote chassis (we match on the new AZ ) which will fail since > we already have this chassis with is-interconn in local SB. > > This patch aims to fix the above issues by updating the AZ.name only > when the user updates the NB.name locally. > > Signed-off-by: Mohammad Heib > Acked-by: Dumitru Ceara > --- Thanks, Mohammad! I changed my ack to a signed-off-by and pushed this to main and backported it to all stable branches down to 22.03. Regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] actions: Use random port selection for SNAT with external_port_range.
On 1/29/24 15:33, Xavier Simonart wrote: > Hi Dumitru > > Thanks for the patch. > > On Tue, Jan 23, 2024 at 10:44 PM Mark Michelson wrote: > >> Hi Dumitru, >> >> Acked-by: Mark Michelson >> >> I think this change is good. I looked through the documentation for the >> "external_port_range" column in the NAT table. This change appears to >> actually make the documentation *more* accurate rather than to introduce >> any potentially undesirable behavior changes. >> >> What I find more telling is that the only tests you had to update were >> the actions tests. We apparently don't have any system tests that use >> NAT with an external port range. That's not great :( >> That's not great indeed, we should try to improve that in the future. >> On 1/22/24 09:54, Dumitru Ceara wrote: >>> This is to avoid unexpected behavior changes due to the underlying >>> datapath (e.g., kernel) changing defaults. If we don't explicitly >>> request a port selection algorithm, OVS leaves it up to the >>> datapath to decide how to do the port selection. Currently that means >>> that source port allocation is not random if the original source port >>> fits in the requested range. >> > > This looks good to me. > In most cases, a source port within range was not translated w/ existing > behavior > > ovn-nbctl --portrange lr-nat-add rtr snat 66.66.66.66 42.42.42.0/24 > 1-2 > > 43.43.43.2:15000 -> 42.42.42.2:8080 => 66.66.66.66:15000 -> > 42.42.42.2:80 # no PAT as within range > > But it was still translated in some cases: > > ovn-nbctl --portrange lr-nat-add rtr snat 66.66.66.66 42.42.42.0/24 > 1-2 > > 43.43.43.2:4 -> 42.42.42.2:8080 => 66.66.66.66:15000 -> > 42.42.42.2:80 # PAT. dst port happens to be 15000 > > 43.43.43.2:15000 -> 42.42.42.2:8080 => 66.66.66.66:XXX -> > 42.42.42.2:80 # PAT to XXX != 15000 as 15000 already used. > > In other words, I do not see how one could rely on existing (pre-patch) > behavior (no PAT if the source port is within the range) > as it is not guaranteed. > > That makes sense, thanks for trying this out! Thanks for the reviews, Mark, Ales and Xavier! I pushed this to main and backported it to all stable branches down to 22.03. Regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] util: Replace and remove ovn_smap_get_uint
On 1/18/24 09:06, Ales Musil wrote:
> Replace and remove the ovn_smap_get_uint as smap_get_uint is avaiable in OvS
> library since 7b2e999fd759 ("smap: Add smap_get_uint() helper function.").
>
> Signed-off-by: Ales Musil
> ---
Applied to main, thanks!
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovs: Bump submodule to latest OVS branch-3.3.
On 1/30/24 10:48, Mohammad Heib wrote:
> Hi,
> Thank you Dumitru.
>
> Acked-by: Mohammad Heib
>
> On Tue, Jan 30, 2024 at 11:16 AM Eelco Chaudron wrote:
>
>>
>>
>> On 30 Jan 2024, at 9:57, Dumitru Ceara wrote:
>>
>>> On 1/30/24 09:04, Eelco Chaudron wrote:
On 30 Jan 2024, at 0:36, Dumitru Ceara wrote:
> This picks up the following relevant OVS commits:
> 8893e24d9d dpdk: Update to use v23.11.
> ed738eca39 util: Annotate function that will never return NULL.
> 77d0bad04 mcast-snooping: Store IGMP/MLD protocol version.
> b222593bc6 mcast-snooping: Add group protocol to mdb/show output.
> a940a691e7 ovs-atomic: Fix inclusion of Clang header by GCC 14.
> b0cf73112d dp-packet: Reset offload/offsets when clearing a packet.
>
> This commit also ports the CI DPDK related changes from OVS commit
> 8893e24d9d09 ("dpdk: Update to use v23.11.") which are required as
>> 23.11
> is the supported DPDK branch on OVS 3.3. There's also a small change
> that had to be made when mangling the prefix path of the installed
> DPDK version in the container.
>
> As a side effect this will also address OVN Fedora 40 (rawhide) build
> failures on aarch64 and s390x. They are fixed by a940a691e7d9
> ("ovs-atomic: Fix inclusion of Clang header by GCC 14.").
>
> Suggested-by: Ilya Maximets
> Signed-off-by: Dumitru Ceara
There is also a change in controller/pinctrl.c, any reason why this is
>> part of this patch?
>>>
>>> Yes, b222593bc6 ("mcast-snooping: Add group protocol to mdb/show
>>> output") changed the OVS mcast library API. That's used by OVN. We
>>> need that pinctrl.c change otherwise compilation fails.
>>
>> Thanks Dumitru for the clarification, maybe you can add this to the commit
>> message when you apply? The rest looks good, and as the tests passed;
>>
>> Acked-by: Eelco Chaudron
>>
Thanks, Numan, Eelco and Mohammad for the reviews!
I updated the commit log to reflect the pinctrl.c change too and pushed
this to the main branch.
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] tests: Fix grep warning
On 1/30/24 08:59, Ales Musil wrote:
> On Tue, Jan 30, 2024 at 8:58 AM Ales Musil wrote:
>
>> The Fedora version of grep (grep (GNU grep) 3.11) complains
>> about the syntax grep "output\:": grep: warning: stray \ before :
>>
>> Remove the \ which works also for Ubuntu grep version
>> (grep (GNU grep) 3.7).
>>
>>
> I forgot to add the Fixes tag.
>
> Fixes: 17b6a12fa286 ("ovn-controller: Support VIF-based local encap IPs
> selection.")
>
>
>> Signed-off-by: Ales Musil
>> ---
Thanks, I added the "fixes" tag and a dot at the end of the commit
summary then pushed this to main.
Regards,
Dumitru
>> tests/ovn.at | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index 62966752f..cf87b9ad4 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -30435,7 +30435,7 @@ check_packet_tunnel() {
>> as $hv
>> echo "vif$src -> vif$dst should go through tunnel $local_encap_ip ->
>> $remote_encap_ip"
>> tunnel_ofport=$(ovs-vsctl --bare --column=ofport find interface
>> options:local_ip=$local_encap_ip options:remote_ip=$remote_encap_ip)
>> -AT_CHECK([test $(ovs-appctl ofproto/trace br-int in_port=vif$src
>> $packet | grep "output\:" | awk -F ':' '{ print $2 }') == $tunnel_ofport])
>> +AT_CHECK([test $(ovs-appctl ofproto/trace br-int in_port=vif$src
>> $packet | grep "output:" | awk -F ':' '{ print $2 }') == $tunnel_ofport])
>> }
>>
>> for i in 1 2; do
>> --
>> 2.43.0
>>
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] github: Bump Fedora version to 39.
On 1/30/24 09:54, Jakob Meng wrote: > On 29.01.24 23:33, Ilya Maximets wrote: >> Fedora 37 reached EOL in November. Switch to the most recent version >> to avoid potential CI failures in the future. >> >> Signed-off-by: Ilya Maximets >> --- >> .github/workflows/build-and-test.yml | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/.github/workflows/build-and-test.yml >> b/.github/workflows/build-and-test.yml >> index ddb425580..fc7558148 100644 >> --- a/.github/workflows/build-and-test.yml >> +++ b/.github/workflows/build-and-test.yml >> @@ -462,7 +462,7 @@ jobs: >>build-linux-rpm: >> name: linux rpm fedora >> runs-on: ubuntu-latest >> -container: fedora:37 >> +container: fedora:39 >> timeout-minutes: 30 >> >> strategy: > > Why not fedora:rawhide? > > We will not have to update this line anymore and we will catch breaking > changes early, like the GCC14 issue. > > I assume there has been a discussion previously, but I cannot find it in my > archives (which is not old)... The reason is that we don't want our CI to break whenever rawhide breaks. It is unstable, so it will break. We're not testing Fedora, we're testing OVS, so we need a stable environment to test with. If we use rawhide, whenever it breaks we can't merge anything into OVS until the build is fixed. That can be very inconvenient. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] github: Bump Fedora version to 39.
On Mon, Jan 29, 2024 at 11:33:56PM +0100, Ilya Maximets wrote: > Fedora 37 reached EOL in November. Switch to the most recent version > to avoid potential CI failures in the future. > > Signed-off-by: Ilya Maximets Acked-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] utilities: Add TASK_STOPPED accounting to the kernel_delay.py script.
On Mon, Jan 29, 2024 at 01:51:42PM +0100, Eelco Chaudron wrote: > This changes add statistics for when a thread is put into stop state. > For example with the following: > > kill -STOP $(pidof ovs-vswitchd); sleep 1; kill -CONT $(pidof ovs-vswitchd); > > Acked-by: Simon Horman > Signed-off-by: Eelco Chaudron > --- > v2: - Removed worst/max ns delay zero checks. Thanks for the update. I confirm that I'm (still) happy with this patch. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v8 2/2] rhel: Enable USDT scripts by default in Fedora builds.
On 1/25/24 21:55, Aaron Conole wrote:
> All supported versions of Fedora do package libbpf, so it
> makes sense to enable USDT support.
>
> Signed-off-by: Aaron Conole
> ---
> v8: Include the correct devel package as a dependency
>
> rhel/openvswitch-fedora.spec.in | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index 5d24ebcda8..b97f509cae 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -28,6 +28,8 @@
> %bcond_with dpdk
> # To disable AF_XDP support, specify '--without afxdp' when building
> %bcond_without afxdp
> +# To control the USDT support
> +%bcond_without usdt
>
> # If there is a need to automatically enable the package after installation,
> # specify the "--with autoenable"
> @@ -77,6 +79,9 @@ Provides: %{name}-dpdk = %{version}-%{release}
> %if %{with afxdp}
> BuildRequires: libxdp-devel libbpf-devel numactl-devel
> %endif
> +%if %{with afxdp}
Did you mean usdt ?
> +BuildRequires: libbpf-devel systemtap-sdt-devel
> +%endif
> BuildRequires: unbound unbound-devel
>
> Requires: openssl hostname iproute module-init-tools unbound
> @@ -173,6 +178,9 @@ This package provides IPsec tunneling support for OVS
> tunnels.
> --enable-afxdp \
> %else
> --disable-afxdp \
> +%endif
> +%if %{with usdt}
> +--enable-usdt-probes \
> %endif
> --enable-ssl \
> --disable-static \
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v8 2/2] rhel: Enable USDT scripts by default in Fedora builds.
On Thu, Jan 25, 2024 at 03:55:44PM -0500, Aaron Conole wrote: > All supported versions of Fedora do package libbpf, so it > makes sense to enable USDT support. > > Signed-off-by: Aaron Conole > --- > v8: Include the correct devel package as a dependency Thanks for the update, this one looks good. Acked-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v8 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.
On Fri, Jan 26, 2024 at 10:29:27AM -0800, Han Zhou wrote:
> On Fri, Jan 26, 2024 at 10:26 AM Han Zhou wrote:
> >
> >
> >
> > On Fri, Jan 26, 2024 at 7:53 AM Aaron Conole wrote:
> > >
> > > Han Zhou writes:
> > >
> > > > On Thu, Jan 25, 2024 at 12:55 PM Aaron Conole
> wrote:
> > > >>
> > > >> From: Kevin Sprague
> > > >>
> > > >> During normal operations, it is useful to understand when a
> particular flow
> > > >> gets removed from the system. This can be useful when debugging
> performance
> > > >> issues tied to ofproto flow changes, trying to determine deployed
> traffic
> > > >> patterns, or while debugging dynamic systems where ports come and go.
> > > >>
> > > >> Prior to this change, there was a lack of visibility around flow
> expiration.
> > > >> The existing debugging infrastructure could tell us when a flow was
> added to
> > > >> the datapath, but not when it was removed or why.
> > > >>
> > > >> This change introduces a USDT probe at the point where the
> revalidator
> > > >> determines that the flow should be removed. Additionally, we track
> the
> > > >> reason for the flow eviction and provide that information as well.
> With
> > > >> this change, we can track the complete flow lifecycle for the netlink
> > > >> datapath by hooking the upcall tracepoint in kernel, the flow put
> USDT, and
> > > >> the revaldiator USDT, letting us watch as flows are added and
> removed from
> > > >> the kernel datapath.
> > > >>
> > > >> This change only enables this information via USDT probe, so it
> won't be
> > > >> possible to access this information any other way (see:
> > > >> Documentation/topics/usdt-probes.rst).
> > > >>
> > > >> Also included is a script
> (utilities/usdt-scripts/flow_reval_monitor.py)
> > > >> which serves as a demonstration of how the new USDT probe might be
> used
> > > >> going forward.
> > > >>
> > > >> Signed-off-by: Kevin Sprague
> > > >> Co-authored-by: Aaron Conole
> > > >> Signed-off-by: Aaron Conole
> > > >
> > > > Thanks Aaron for taking care of this patch. I saw you resolved most
> of my comments for the v6 of the original patch:
> > > >
> https://mail.openvswitch.org/pipermail/ovs-dev/2023-January/401220.html
> > > >
> > > > Butit seems my last comment was missed:
> > > > ===
> > > >
> > > > I do notice a counter in my patch doesn't have a
> > > > counterpart in this patch. In revalidator_sweep__(), I have:
> > > > if (purge) {
> > > > result = UKEY_DELETE;
> > > > +COVERAGE_INC(upcall_flow_del_purge);
> > > >
> > > > Would it be good to add one (e.g. FDR_PURGE) here, too?
> > > >
> > > > ===
> > > > Could you check if this can be added?
> > > > If this is merged I can rebase my patch on top of this.
> > >
> > > Sorry I didn't reply to this.
> > >
> > > I'm not sure it makes sense to add the probe for purge, specifically as
> > > the purge is only done in two cases:
> > >
> > > 1. The threads are being stopped (which should never occur after
> > >initialization unless the vswitchd is being stopped / killed)
> > >
> > > 2. An admin runs a command to purge the revalidators (which isn't a
> > >recommended procedure as it can cause lots of really weird side
> > >effects and we only use it as a debug tool).
> > >
> > > Did I understand the case enough? I didn't reread the patch you're
> > > proposing, so I might be misunderstanding something.
> >
> > I believe your understanding is correct. However, I think it would be
> good to cover all the reasons of DP flow deletion, and "purge" seems to be
> the only case missing now.
> > Although purge is less likely to happen in production, it is still
> possible, probably in some weird scenarios. Would it be good to add for
> completeness, if there is no harm?
> >
> > Thanks,
> > Han
> >
> That being said, I don't have a strong opinion on this. So I give my ack in
> either case:
> Acked-by: Han Zhou
Perhaps we can treat this as a follow-up item?
Acked-by: Simon Horman
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovs: Bump submodule to latest OVS branch-3.3.
Hi,
Thank you Dumitru.
Acked-by: Mohammad Heib
On Tue, Jan 30, 2024 at 11:16 AM Eelco Chaudron wrote:
>
>
> On 30 Jan 2024, at 9:57, Dumitru Ceara wrote:
>
> > On 1/30/24 09:04, Eelco Chaudron wrote:
> >>
> >>
> >> On 30 Jan 2024, at 0:36, Dumitru Ceara wrote:
> >>
> >>> This picks up the following relevant OVS commits:
> >>> 8893e24d9d dpdk: Update to use v23.11.
> >>> ed738eca39 util: Annotate function that will never return NULL.
> >>> 77d0bad04 mcast-snooping: Store IGMP/MLD protocol version.
> >>> b222593bc6 mcast-snooping: Add group protocol to mdb/show output.
> >>> a940a691e7 ovs-atomic: Fix inclusion of Clang header by GCC 14.
> >>> b0cf73112d dp-packet: Reset offload/offsets when clearing a packet.
> >>>
> >>> This commit also ports the CI DPDK related changes from OVS commit
> >>> 8893e24d9d09 ("dpdk: Update to use v23.11.") which are required as
> 23.11
> >>> is the supported DPDK branch on OVS 3.3. There's also a small change
> >>> that had to be made when mangling the prefix path of the installed
> >>> DPDK version in the container.
> >>>
> >>> As a side effect this will also address OVN Fedora 40 (rawhide) build
> >>> failures on aarch64 and s390x. They are fixed by a940a691e7d9
> >>> ("ovs-atomic: Fix inclusion of Clang header by GCC 14.").
> >>>
> >>> Suggested-by: Ilya Maximets
> >>> Signed-off-by: Dumitru Ceara
> >>
> >> There is also a change in controller/pinctrl.c, any reason why this is
> part of this patch?
> >>
> >
> > Yes, b222593bc6 ("mcast-snooping: Add group protocol to mdb/show
> > output") changed the OVS mcast library API. That's used by OVN. We
> > need that pinctrl.c change otherwise compilation fails.
>
> Thanks Dumitru for the clarification, maybe you can add this to the commit
> message when you apply? The rest looks good, and as the tests passed;
>
> Acked-by: Eelco Chaudron
>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v7 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.
On Fri, Jan 26, 2024 at 10:55:48AM -0500, Aaron Conole wrote: > Simon Horman writes: > > > On Thu, Jan 25, 2024 at 12:05:29PM -0500, Aaron Conole wrote: > >> From: Kevin Sprague > >> > >> During normal operations, it is useful to understand when a particular flow > >> gets removed from the system. This can be useful when debugging performance > >> issues tied to ofproto flow changes, trying to determine deployed traffic > >> patterns, or while debugging dynamic systems where ports come and go. > >> > >> Prior to this change, there was a lack of visibility around flow > >> expiration. > >> The existing debugging infrastructure could tell us when a flow was added > >> to > >> the datapath, but not when it was removed or why. > >> > >> This change introduces a USDT probe at the point where the revalidator > >> determines that the flow should be removed. Additionally, we track the > >> reason for the flow eviction and provide that information as well. With > >> this change, we can track the complete flow lifecycle for the netlink > >> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and > >> the revaldiator USDT, letting us watch as flows are added and removed from > >> the kernel datapath. > >> > >> This change only enables this information via USDT probe, so it won't be > >> possible to access this information any other way (see: > >> Documentation/topics/usdt-probes.rst). > >> > >> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py) > >> which serves as a demonstration of how the new USDT probe might be used > >> going forward. > >> > >> Signed-off-by: Kevin Sprague > >> Co-authored-by: Aaron Conole > >> Signed-off-by: Aaron Conole > > > > Hi Aaron, > > > > has any consideration been given to adding tests for this feature? > > I don't think so. Actually, we don't have any tests for any of the USDT > probes / scripts. At the very least, we could probably update some of > the existing tests to take advantage of these tracepoints to verify the > information being captured. That's a much bigger task, though. Thanks Aaron, I understand your point that this is a bigger task. And I have no objections to treating it as a follow-up. > Maybe Adrian has opinions on it as well, since Retis may use this > information to gather insights. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] dpif-netdev: Increase MAX_RECIRC_DEPTH to 8.
On Fri, Jan 26, 2024 at 02:24:51PM +0100, jm...@redhat.com wrote: > From: Jakob Meng > > In a scenario where OVN does load balancing and then SNAT with a OVS > userspace datapath [0], the recirc_depth may be greater than 6. In > that case, ovs-vswitchd might drop packets and raise warnings: > > dpif_netdev|WARN|Packet dropped. Max recirculation depth exceeded. > > Increasing MAX_RECIRC_DEPTH to 8 solves this issue. > > [0] > https://github.com/ovn-org/ovn/blob/dd5cd73e3df1bfb1a215cb45d1e2e03eff1d049a/tests/system-ovn-kmod.at#L740 > > Reported-at: https://issues.redhat.com/browse/FDP-251 > Signed-off-by: Jakob Meng Hi Jakob, I'm unsure what the considerations were when setting this limit, but I do note that it was increased once before, from 5 to 6, for an OVN use-case [1]. So this approach seems reasonable to me. Acked-by: Simon Horman [1] dpif-netdev: Set MAX_RECIRC_DEPTH to 6. https://github.com/openvswitch/ovs/commit/3f9d3836d63a ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovs: Bump submodule to latest OVS branch-3.3.
On 30 Jan 2024, at 9:57, Dumitru Ceara wrote:
> On 1/30/24 09:04, Eelco Chaudron wrote:
>>
>>
>> On 30 Jan 2024, at 0:36, Dumitru Ceara wrote:
>>
>>> This picks up the following relevant OVS commits:
>>> 8893e24d9d dpdk: Update to use v23.11.
>>> ed738eca39 util: Annotate function that will never return NULL.
>>> 77d0bad04 mcast-snooping: Store IGMP/MLD protocol version.
>>> b222593bc6 mcast-snooping: Add group protocol to mdb/show output.
>>> a940a691e7 ovs-atomic: Fix inclusion of Clang header by GCC 14.
>>> b0cf73112d dp-packet: Reset offload/offsets when clearing a packet.
>>>
>>> This commit also ports the CI DPDK related changes from OVS commit
>>> 8893e24d9d09 ("dpdk: Update to use v23.11.") which are required as 23.11
>>> is the supported DPDK branch on OVS 3.3. There's also a small change
>>> that had to be made when mangling the prefix path of the installed
>>> DPDK version in the container.
>>>
>>> As a side effect this will also address OVN Fedora 40 (rawhide) build
>>> failures on aarch64 and s390x. They are fixed by a940a691e7d9
>>> ("ovs-atomic: Fix inclusion of Clang header by GCC 14.").
>>>
>>> Suggested-by: Ilya Maximets
>>> Signed-off-by: Dumitru Ceara
>>
>> There is also a change in controller/pinctrl.c, any reason why this is part
>> of this patch?
>>
>
> Yes, b222593bc6 ("mcast-snooping: Add group protocol to mdb/show
> output") changed the OVS mcast library API. That's used by OVN. We
> need that pinctrl.c change otherwise compilation fails.
Thanks Dumitru for the clarification, maybe you can add this to the commit
message when you apply? The rest looks good, and as the tests passed;
Acked-by: Eelco Chaudron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovs: Bump submodule to latest OVS branch-3.3.
On 1/30/24 09:04, Eelco Chaudron wrote:
>
>
> On 30 Jan 2024, at 0:36, Dumitru Ceara wrote:
>
>> This picks up the following relevant OVS commits:
>> 8893e24d9d dpdk: Update to use v23.11.
>> ed738eca39 util: Annotate function that will never return NULL.
>> 77d0bad04 mcast-snooping: Store IGMP/MLD protocol version.
>> b222593bc6 mcast-snooping: Add group protocol to mdb/show output.
>> a940a691e7 ovs-atomic: Fix inclusion of Clang header by GCC 14.
>> b0cf73112d dp-packet: Reset offload/offsets when clearing a packet.
>>
>> This commit also ports the CI DPDK related changes from OVS commit
>> 8893e24d9d09 ("dpdk: Update to use v23.11.") which are required as 23.11
>> is the supported DPDK branch on OVS 3.3. There's also a small change
>> that had to be made when mangling the prefix path of the installed
>> DPDK version in the container.
>>
>> As a side effect this will also address OVN Fedora 40 (rawhide) build
>> failures on aarch64 and s390x. They are fixed by a940a691e7d9
>> ("ovs-atomic: Fix inclusion of Clang header by GCC 14.").
>>
>> Suggested-by: Ilya Maximets
>> Signed-off-by: Dumitru Ceara
>
> There is also a change in controller/pinctrl.c, any reason why this is part
> of this patch?
>
Yes, b222593bc6 ("mcast-snooping: Add group protocol to mdb/show
output") changed the OVS mcast library API. That's used by OVN. We
need that pinctrl.c change otherwise compilation fails.
Regards,
Dumitru
> Cheers,
>
> Eelco
>
>> ---
>> .ci/dpdk-build.sh | 28 ++--
>> .ci/linux-build.sh | 11 ++-
>> .github/workflows/test.yml | 4 ++--
>> controller/pinctrl.c | 6 +-
>> ovs| 2 +-
>> 5 files changed, 32 insertions(+), 19 deletions(-)
>>
>> diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh
>> index f44ac15b07..0c13c98c98 100755
>> --- a/.ci/dpdk-build.sh
>> +++ b/.ci/dpdk-build.sh
>> @@ -5,25 +5,27 @@ set -x
>>
>> function build_dpdk()
>> {
>> -local VERSION_FILE="dpdk-dir/cached-version"
>> local DPDK_VER=$1
>> local DPDK_OPTS=""
>> +local DPDK_INSTALL_DIR="$(pwd)/dpdk-dir"
>> +local VERSION_FILE="$DPDK_INSTALL_DIR/cached-version"
>>
>> -rm -rf dpdk-dir
>> +rm -rf dpdk-src
>> +rm -rf $DPDK_INSTALL_DIR
>>
>> if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
>> -git clone --single-branch $DPDK_GIT dpdk-dir -b
>> "${DPDK_VER##refs/*/}"
>> -pushd dpdk-dir
>> +git clone --single-branch $DPDK_GIT dpdk-src -b
>> "${DPDK_VER##refs/*/}"
>> +pushd dpdk-src
>> git log -1 --oneline
>> else
>> wget https://fast.dpdk.org/rel/dpdk-$1.tar.xz
>> tar xvf dpdk-$1.tar.xz > /dev/null
>> DIR_NAME=$(tar -tf dpdk-$1.tar.xz | head -1 | cut -f1 -d"/")
>> -mv ${DIR_NAME} dpdk-dir
>> -pushd dpdk-dir
>> +mv ${DIR_NAME} dpdk-src
>> +pushd dpdk-src
>> fi
>>
>> -# Switching to 'default' machine to make dpdk-dir cache usable on
>> +# Switching to 'default' machine to make the dpdk cache usable on
>> # different CPUs. We can't be sure that all CI machines are exactly
>> same.
>> DPDK_OPTS="$DPDK_OPTS -Dmachine=default"
>>
>> @@ -38,16 +40,22 @@ function build_dpdk()
>> # only depend on virtio/tap drivers.
>> # We can disable all remaining drivers to save compilation time.
>> DPDK_OPTS="$DPDK_OPTS -Denable_drivers=net/null,net/tap,net/virtio"
>> +# OVS depends on the vhost library (and its dependencies).
>> +# net/tap depends on the gso library.
>> +DPDK_OPTS="$DPDK_OPTS -Denable_libs=cryptodev,dmadev,gso,vhost"
>>
>> # Install DPDK using prefix.
>> -DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build"
>> +DPDK_OPTS="$DPDK_OPTS --prefix=$DPDK_INSTALL_DIR"
>>
>> meson $DPDK_OPTS build
>> ninja -C build
>> ninja -C build install
>> -
>> -echo "Installed DPDK in $(pwd)"
>> popd
>> +
>> +# Remove examples sources.
>> +rm -rf $DPDK_INSTALL_DIR/share/dpdk/examples
>> +
>> +echo "Installed DPDK in $DPDK_INSTALL_DIR"
>> echo "${DPDK_VER}" > ${VERSION_FILE}
>> }
>>
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index b0c3c9252e..78f17f8bdb 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -13,24 +13,25 @@ RECHECK=${RECHECK:-"no"}
>>
>> function install_dpdk()
>> {
>> -local VERSION_FILE="dpdk-dir/cached-version"
>> -local DPDK_LIB=$(pwd)/dpdk-dir/build/lib/x86_64-linux-gnu
>> +local DPDK_INSTALL_DIR="$(pwd)/dpdk-dir"
>> +local VERSION_FILE="${DPDK_INSTALL_DIR}/cached-version"
>> +local DPDK_LIB=${DPDK_INSTALL_DIR}/lib/x86_64-linux-gnu
>>
>> # Export the following path for pkg-config to find the .pc file.
>> export PKG_CONFIG_PATH=$DPDK_LIB/pkgconfig/:$PKG_CONFIG_PATH
>>
>> if [ ! -f "${VERSION_FILE}" ]; then
>> -echo "Could not find DPDK in $(pwd)/dpdk-dir"
>> +echo "Could not find DPDK in $DPDK_INSTALL_DIR"
>>
Re: [ovs-dev] [PATCH] github: Bump Fedora version to 39.
On 29.01.24 23:33, Ilya Maximets wrote: > Fedora 37 reached EOL in November. Switch to the most recent version > to avoid potential CI failures in the future. > > Signed-off-by: Ilya Maximets > --- > .github/workflows/build-and-test.yml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/.github/workflows/build-and-test.yml > b/.github/workflows/build-and-test.yml > index ddb425580..fc7558148 100644 > --- a/.github/workflows/build-and-test.yml > +++ b/.github/workflows/build-and-test.yml > @@ -462,7 +462,7 @@ jobs: >build-linux-rpm: > name: linux rpm fedora > runs-on: ubuntu-latest > -container: fedora:37 > +container: fedora:39 > timeout-minutes: 30 > > strategy: Why not fedora:rawhide? We will not have to update this line anymore and we will catch breaking changes early, like the GCC14 issue. I assume there has been a discussion previously, but I cannot find it in my archives (which is not old)... ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] github: Bump Fedora version to 39.
On 29 Jan 2024, at 23:33, Ilya Maximets wrote: > Fedora 37 reached EOL in November. Switch to the most recent version > to avoid potential CI failures in the future. > > Signed-off-by: Ilya Maximets Thanks for looking forward! The change looks fine to me, only tested on the master branch. Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] tests: Fix grep warning
Bleep bloop. Greetings Ales Musil, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: The subject summary should end with a dot. Subject: tests: Fix grep warning Lines checked: 33, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovs: Bump submodule to latest OVS branch-3.3.
On 30 Jan 2024, at 0:36, Dumitru Ceara wrote:
> This picks up the following relevant OVS commits:
> 8893e24d9d dpdk: Update to use v23.11.
> ed738eca39 util: Annotate function that will never return NULL.
> 77d0bad04 mcast-snooping: Store IGMP/MLD protocol version.
> b222593bc6 mcast-snooping: Add group protocol to mdb/show output.
> a940a691e7 ovs-atomic: Fix inclusion of Clang header by GCC 14.
> b0cf73112d dp-packet: Reset offload/offsets when clearing a packet.
>
> This commit also ports the CI DPDK related changes from OVS commit
> 8893e24d9d09 ("dpdk: Update to use v23.11.") which are required as 23.11
> is the supported DPDK branch on OVS 3.3. There's also a small change
> that had to be made when mangling the prefix path of the installed
> DPDK version in the container.
>
> As a side effect this will also address OVN Fedora 40 (rawhide) build
> failures on aarch64 and s390x. They are fixed by a940a691e7d9
> ("ovs-atomic: Fix inclusion of Clang header by GCC 14.").
>
> Suggested-by: Ilya Maximets
> Signed-off-by: Dumitru Ceara
There is also a change in controller/pinctrl.c, any reason why this is part of
this patch?
Cheers,
Eelco
> ---
> .ci/dpdk-build.sh | 28 ++--
> .ci/linux-build.sh | 11 ++-
> .github/workflows/test.yml | 4 ++--
> controller/pinctrl.c | 6 +-
> ovs| 2 +-
> 5 files changed, 32 insertions(+), 19 deletions(-)
>
> diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh
> index f44ac15b07..0c13c98c98 100755
> --- a/.ci/dpdk-build.sh
> +++ b/.ci/dpdk-build.sh
> @@ -5,25 +5,27 @@ set -x
>
> function build_dpdk()
> {
> -local VERSION_FILE="dpdk-dir/cached-version"
> local DPDK_VER=$1
> local DPDK_OPTS=""
> +local DPDK_INSTALL_DIR="$(pwd)/dpdk-dir"
> +local VERSION_FILE="$DPDK_INSTALL_DIR/cached-version"
>
> -rm -rf dpdk-dir
> +rm -rf dpdk-src
> +rm -rf $DPDK_INSTALL_DIR
>
> if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
> -git clone --single-branch $DPDK_GIT dpdk-dir -b
> "${DPDK_VER##refs/*/}"
> -pushd dpdk-dir
> +git clone --single-branch $DPDK_GIT dpdk-src -b
> "${DPDK_VER##refs/*/}"
> +pushd dpdk-src
> git log -1 --oneline
> else
> wget https://fast.dpdk.org/rel/dpdk-$1.tar.xz
> tar xvf dpdk-$1.tar.xz > /dev/null
> DIR_NAME=$(tar -tf dpdk-$1.tar.xz | head -1 | cut -f1 -d"/")
> -mv ${DIR_NAME} dpdk-dir
> -pushd dpdk-dir
> +mv ${DIR_NAME} dpdk-src
> +pushd dpdk-src
> fi
>
> -# Switching to 'default' machine to make dpdk-dir cache usable on
> +# Switching to 'default' machine to make the dpdk cache usable on
> # different CPUs. We can't be sure that all CI machines are exactly same.
> DPDK_OPTS="$DPDK_OPTS -Dmachine=default"
>
> @@ -38,16 +40,22 @@ function build_dpdk()
> # only depend on virtio/tap drivers.
> # We can disable all remaining drivers to save compilation time.
> DPDK_OPTS="$DPDK_OPTS -Denable_drivers=net/null,net/tap,net/virtio"
> +# OVS depends on the vhost library (and its dependencies).
> +# net/tap depends on the gso library.
> +DPDK_OPTS="$DPDK_OPTS -Denable_libs=cryptodev,dmadev,gso,vhost"
>
> # Install DPDK using prefix.
> -DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build"
> +DPDK_OPTS="$DPDK_OPTS --prefix=$DPDK_INSTALL_DIR"
>
> meson $DPDK_OPTS build
> ninja -C build
> ninja -C build install
> -
> -echo "Installed DPDK in $(pwd)"
> popd
> +
> +# Remove examples sources.
> +rm -rf $DPDK_INSTALL_DIR/share/dpdk/examples
> +
> +echo "Installed DPDK in $DPDK_INSTALL_DIR"
> echo "${DPDK_VER}" > ${VERSION_FILE}
> }
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index b0c3c9252e..78f17f8bdb 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -13,24 +13,25 @@ RECHECK=${RECHECK:-"no"}
>
> function install_dpdk()
> {
> -local VERSION_FILE="dpdk-dir/cached-version"
> -local DPDK_LIB=$(pwd)/dpdk-dir/build/lib/x86_64-linux-gnu
> +local DPDK_INSTALL_DIR="$(pwd)/dpdk-dir"
> +local VERSION_FILE="${DPDK_INSTALL_DIR}/cached-version"
> +local DPDK_LIB=${DPDK_INSTALL_DIR}/lib/x86_64-linux-gnu
>
> # Export the following path for pkg-config to find the .pc file.
> export PKG_CONFIG_PATH=$DPDK_LIB/pkgconfig/:$PKG_CONFIG_PATH
>
> if [ ! -f "${VERSION_FILE}" ]; then
> -echo "Could not find DPDK in $(pwd)/dpdk-dir"
> +echo "Could not find DPDK in $DPDK_INSTALL_DIR"
> return 1
> fi
>
> # As we build inside a container we need to update the prefix.
> -sed -i -E "s|^prefix=.*|prefix=$(pwd)/dpdk-dir/build|" \
> +sed -i -E "s|^prefix=.*|prefix=${DPDK_INSTALL_DIR}|" \
> "$DPDK_LIB/pkgconfig/libdpdk-libs.pc"
>
> # Update the library paths.
> sudo ldconfig
> -echo "Found cached DPDK $(cat ${VERSION_FILE}) build in
Re: [ovs-dev] [PATCH ovn] tests: Fix grep warning
On Tue, Jan 30, 2024 at 8:58 AM Ales Musil wrote:
> The Fedora version of grep (grep (GNU grep) 3.11) complains
> about the syntax grep "output\:": grep: warning: stray \ before :
>
> Remove the \ which works also for Ubuntu grep version
> (grep (GNU grep) 3.7).
>
>
I forgot to add the Fixes tag.
Fixes: 17b6a12fa286 ("ovn-controller: Support VIF-based local encap IPs
selection.")
> Signed-off-by: Ales Musil
> ---
> tests/ovn.at | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 62966752f..cf87b9ad4 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -30435,7 +30435,7 @@ check_packet_tunnel() {
> as $hv
> echo "vif$src -> vif$dst should go through tunnel $local_encap_ip ->
> $remote_encap_ip"
> tunnel_ofport=$(ovs-vsctl --bare --column=ofport find interface
> options:local_ip=$local_encap_ip options:remote_ip=$remote_encap_ip)
> -AT_CHECK([test $(ovs-appctl ofproto/trace br-int in_port=vif$src
> $packet | grep "output\:" | awk -F ':' '{ print $2 }') == $tunnel_ofport])
> +AT_CHECK([test $(ovs-appctl ofproto/trace br-int in_port=vif$src
> $packet | grep "output:" | awk -F ':' '{ print $2 }') == $tunnel_ofport])
> }
>
> for i in 1 2; do
> --
> 2.43.0
>
>
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA
amu...@redhat.com
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
