Re: [ovs-dev] [PATCH net-next v9 00/10] net: openvswitch: Add sample multicasting.

2024-07-05 Thread patchwork-bot+netdevbpf
Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski :

On Thu,  4 Jul 2024 10:56:51 +0200 you wrote:
> ** Background **
> Currently, OVS supports several packet sampling mechanisms (sFlow,
> per-bridge IPFIX, per-flow IPFIX). These end up being translated into a
> userspace action that needs to be handled by ovs-vswitchd's handler
> threads only to be forwarded to some third party application that
> will somehow process the sample and provide observability on the
> datapath.
> 
> [...]

Here is the summary with links:
  - [net-next,v9,01/10] net: psample: add user cookie
https://git.kernel.org/netdev/net-next/c/093b0f366567
  - [net-next,v9,02/10] net: sched: act_sample: add action cookie to sample
https://git.kernel.org/netdev/net-next/c/03448444ae5c
  - [net-next,v9,03/10] net: psample: skip packet copy if no listeners
https://git.kernel.org/netdev/net-next/c/c35d86a23029
  - [net-next,v9,04/10] net: psample: allow using rate as probability
https://git.kernel.org/netdev/net-next/c/7b1b2b60c63f
  - [net-next,v9,05/10] net: openvswitch: add psample action
https://git.kernel.org/netdev/net-next/c/aae0b82b46cb
  - [net-next,v9,06/10] net: openvswitch: store sampling probability in cb.
https://git.kernel.org/netdev/net-next/c/71763d8a8203
  - [net-next,v9,07/10] selftests: openvswitch: add psample action
https://git.kernel.org/netdev/net-next/c/60ccf62d3ceb
  - [net-next,v9,08/10] selftests: openvswitch: add userspace parsing
https://git.kernel.org/netdev/net-next/c/c7815abbea45
  - [net-next,v9,09/10] selftests: openvswitch: parse trunc action
https://git.kernel.org/netdev/net-next/c/b192bf12dbb0
  - [net-next,v9,10/10] selftests: openvswitch: add psample test
https://git.kernel.org/netdev/net-next/c/30d772a03582

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn v5 2/2] pinctrl: Handle arp/nd for other address families.

2024-07-05 Thread Numan Siddique
On Fri, Jun 28, 2024 at 5:54 PM Numan Siddique  wrote:
>
> On Wed, Jun 12, 2024 at 2:50 AM Felix Huettner via dev
>  wrote:
> >
> > Previously we could only generate ARP requests from IPv4 packets
> > and NS requests from IPv6 packets. This was the case because we rely on
> > information in the packet to generate the ARP/NS requests.
> >
> > However in case of ARP/NS requests originating from the Logical_Router
> > pipeline for nexthop lookups we overwrite the affected fields
> > afterwards. This overwrite is done by the userdata openflow actions.
> > Because of this we actually do not rely on any information of the IPv4/6
> > packets in these cases.
> >
> > Unfortunately we can not easily determine if we are actually later
> > overwriting the affected fields. The approach now is to use the fields
> > from the IP header if we have a matching IP version and default to some
> > values otherwise. In case we overwrite this data afterwards we are
> > generally good. If we do not overwrite this data because of some bug we
> > will send out invalid ARP/NS requests. They will hopefully be dropped by
> > the rest of the network.
> >
> > The alternative would have been to introduce new arp/nd_ns actions where
> > we guarantee this overwrite. This would not suffer from the above
> > limitations, but would require a coordination on upgrades between all
> > ovn-controllers and northd.
> >
> > Signed-off-by: Felix Huettner 
>
> Hi Felix,
>
> Thanks for the patch series and sorry for the delay in reviews.
>
> Looks like this patch series requires another rebase.
>
> I tested this patch series using the ovn-fake-mutlinode - [1] .
>
> After starting it,  I added the below static route
>
> ovn-nbctl lr-route-add lr1 172.15.0.0/24 3001::b
>
> And in the 'ovn-chassis-1' container I ran this command
>
> [root@ovn-chassis-1 ~]# ip netns exec sw01p1 ping 172.15.0.50
>
> Since we don't know the next hop,  the below logical flow is hit
> (which is as expected)
>
>   table=24(lr_in_arp_request  ), priority=200  , match=(eth.dst ==
> 00:00:00:00:00:00 && reg9[9] == 0 && xxreg0 == 3001::4), action=(nd_ns
> { eth.dst = 33:33:ff:00:00:04; ip6.dst = ff02::1:ff00:4; nd.target =
> 3001::b; output; }; output;)
>
> And If I look into the ovn-controller logs (after enabling vconn:dbg)
> I see that ovn-controller receives the icmp ping packet and it
> generates IPv6 NS packet which is also as expected
>
> 
> 2024-06-28T21:39:06.303Z|00020|vconn(ovn_pinctrl0)|DBG|unix:/var/run/openvswitch/br-int.mgmt:
> received: NXT_PACKET_IN2 (OF1.5) (xid=0x0): cookie=0x77e2d8b
> total_len=98 
> reg0=0x3001,reg1=0xac10016e,reg3=0xb,reg4=0x3001,reg7=0xa,reg9=0x4,reg10=0x1,reg11=0x1,reg12=0x3,reg14=0x1,reg15=0x3,metadata=0x3,in_port=5
> (via action) data_len=98 (unbuffered)
>  
> userdata=00.00.00.09.00.00.00.00.00.1c.00.18.00.80.00.00.00.00.00.00.00.01.de.10.80.00.3e.10.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.25.00.00.00
>  continuation.bridge=b72b4e65-0756-4215-bfbf-c3db3652e8ee
>  continuation.actions=unroll_xlate(table=0, cookie=0),resubmit(,37)
>  continuation.odp_port=5
> icmp,vlan_tci=0x,dl_src=30:51:00:00:00:03,dl_dst=00:00:00:00:00:00,nw_src=11.0.0.3,nw_dst=172.15.0.50,nw_tos=0,nw_ecn=0,nw_ttl=63,nw_frag=no,icmp_type=8,icmp_code=0
> icmp_csum:cf9b
> 2024-06-28T21:39:06.304Z|00021|vconn(ovn_pinctrl0)|DBG|unix:/var/run/openvswitch/br-int.mgmt:
> sent (Success): OFPT_PACKET_OUT (OF1.5) (xid=0x413):
> in_port=CONTROLLER
> actions=set_field:0x3001->reg0,set_field:0xac10016e->reg1,set_field:0xb->reg3,set_field:0x3001->reg4,set_field:0xa->reg7,set_field:0x4->reg9,set_field:0x1->reg10,set_field:0x1->reg11,set_field:0x3->reg12,set_field:0x1->reg14,set_field:0x3->reg15,set_field:0x3->metadata,move:NXM_NX_XXREG0[]->NXM_NX_ND_TARGET[],resubmit(,37)
> data_len=86
> icmp6,vlan_tci=0x,dl_src=30:51:00:00:00:03,dl_dst=33:33:ff:ff:ff:ff,ipv6_src=fe80::3251:ff:fe00:3,ipv6_dst=ff02::1::,ipv6_label=0x0,nw_tos=0,nw_ecn=0,nw_ttl=255,nw_frag=no,icmp_type=135,icmp_code=0,nd_target=:::::::,nd_sll=30:51:00:00:00:03,nd_tll=00:00:00:00:00:00
> icmp6_csum:1877
> --
>
> But strangely when this packet leaves the physical interface 'eth2' on
> ovn-chassis-1,  the nd_target is wrongly populated
>
> tcpdump on the physical interface (connected to br-ex)
> ---
> 17:37:40.289881 30:51:00:00:00:03 > 33:33:ff:ff:ff:ff, ethertype IPv6
> (0x86dd), length 86: (hlim 255, next-header ICMPv6 (58) payload
> length: 32) fe80::3251:ff:fe00:3 > ff02::1::: [icmp6 sum ok]
> ICMP6, neighbor solicitation, length 32, who has 3001:0:ac10:16e::b
>   source link-address option (1), length 8 (1): 30:51:00:00:00:03
>
> ---
>
> You can see that the nd_target is '3001:0:ac10:16e::b  and not
> '3001::b', which is strange.  The inner action of "nd_ns" clearly sets
> nd.target = 3001::b.
>
> Can you please verify in your end if that is the case ?  You can
> perhaps use ovn-fake-multinode if you can't  reproduce in your setu

[ovs-dev] [PATCH v6] tunnel: Allow UDP zero checksum with IPv6 tunnels.

2024-07-05 Thread Mike Pattrick
This patch adopts the proposed RFC 6935 by allowing null UDP checksums
even if the tunnel protocol is IPv6. This is already supported by Linux
through the udp6zerocsumtx tunnel option. It is disabled by default and
IPv6 tunnels are flagged as requiring a checksum, but this patch enables
the user to set csum=false on IPv6 tunnels.

Acked-by: Simon Horman 
Signed-off-by: Mike Pattrick 
---
 NEWS  | 11 +++
 lib/netdev-native-tnl.c   |  2 +-
 lib/netdev-vport.c| 17 +++--
 lib/netdev.h  | 18 +-
 ofproto/tunnel.c  | 10 --
 tests/tunnel-push-pop-ipv6.at |  9 +
 tests/tunnel-push-pop.at  |  7 +++
 tests/tunnel.at   |  2 +-
 vswitchd/vswitch.xml  | 12 +---
 9 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index e0359b759..bbf489c3c 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,10 @@ Post-v3.3.0
  * Conntrack now supports 'random' flag for selecting ports in a range
while natting and 'persistent' flag for selection of the IP address
from a range.
+ * IPv6 UDP tunnel encapsulation including Geneve and VXLAN will now
+   honour the csum option.  Configuring the interface with
+   "options:csum=false" now has the same effect as the udp6zerocsumtx
+   option has with Linux kernel UDP tunnels.
- The primary development branch has been renamed from 'master' to 'main'.
  The OVS tree remains hosted on GitHub.
  https://github.com/openvswitch/ovs.git
@@ -16,6 +20,13 @@ Post-v3.3.0
per interface 'options:dpdk-lsc-interrupt' to 'false'.
- Python:
  * Added custom transaction support to the Idl via add_op().
+   - Kernel datapath:
+ * Previously the kernel datapath did not enable UDP checksums by default
+   in IPv6 tunnels.  This behaviour is non-standard, differs from the
+   Linux kernel, and as also different than the userspace datapath.  Now
+   these tunnels will calculate checksums by default and that behaviour can
+   be changed with "options:csum=false" just as with the userspace
+   datapath.
 
 
 v3.3.0 - 16 Feb 2024
diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 0f9f07f44..16c56608d 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -384,7 +384,7 @@ udp_build_header(const struct netdev_tunnel_config *tnl_cfg,
 udp = netdev_tnl_ip_build_header(data, params, IPPROTO_UDP, 0);
 udp->udp_dst = tnl_cfg->dst_port;
 
-if (params->is_ipv6 || params->flow->tunnel.flags & FLOW_TNL_F_CSUM) {
+if (params->flow->tunnel.flags & FLOW_TNL_F_CSUM) {
 /* Write a value in now to mark that we should compute the checksum
  * later. 0x is handy because it is transparent to the
  * calculation. */
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 60caa02fb..234a4ebe1 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -702,7 +702,9 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
 tnl_cfg.dst_port = htons(atoi(node->value));
 } else if (!strcmp(node->key, "csum") && has_csum) {
 if (!strcmp(node->value, "true")) {
-tnl_cfg.csum = true;
+tnl_cfg.csum = NETDEV_TNL_CSUM_ENABLED;
+} else if (!strcmp(node->value, "false")) {
+tnl_cfg.csum = NETDEV_TNL_CSUM_DISABLED;
 }
 } else if (!strcmp(node->key, "seq") && has_seq) {
 if (!strcmp(node->value, "true")) {
@@ -850,6 +852,15 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
 }
 }
 
+/* The default csum state for GRE is special as it does have an optional
+ * checksum but the default configuration isn't correlated with IP version
+ * like UDP tunnels are.  Likewise, tunnels with no checksum at all must be
+ * in this state. */
+if (tnl_cfg.csum == NETDEV_TNL_CSUM_DEFAULT &&
+(!has_csum || strstr(type, "gre"))) {
+tnl_cfg.csum = NETDEV_TNL_DEFAULT_NO_CSUM;
+}
+
 enum tunnel_layers layers = tunnel_supported_layers(type, &tnl_cfg);
 const char *full_type = (strcmp(type, "vxlan") ? type
  : (tnl_cfg.exts & (1 << OVS_VXLAN_EXT_GPE)
@@ -1026,8 +1037,10 @@ get_tunnel_config(const struct netdev *dev, struct smap 
*args)
 }
 }
 
-if (tnl_cfg->csum) {
+if (tnl_cfg->csum == NETDEV_TNL_CSUM_ENABLED) {
 smap_add(args, "csum", "true");
+} else if (tnl_cfg->csum == NETDEV_TNL_CSUM_DISABLED) {
+smap_add(args, "csum", "false");
 }
 
 if (tnl_cfg->set_seq) {
diff --git a/lib/netdev.h b/lib/netdev.h
index 67a8486bd..63e03d72d 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -111,6 +111,22 @@ enum netdev_srv6_flowlabel {
 SRV6_FLOWLABEL_COMPUTE,
 };
 
+enum netdev_tnl_csum {
+/* Default value for UDP tunnels if no configurations i

[ovs-dev] [PATCH v4 3/3] netdev-dpdk: Re-enable VXLAN/Geneve offload for Intel cards.

2024-07-05 Thread Mike Pattrick
Previously the OVS support for checksum/TSO offloading didn't work well
with some network cards that supported VXLAN/Geneve tunnel TSO but not
outer UDP checksums. Now support for this configuration is improved and
we no longer need to disable the VXLAN/Geneve TSO flags from intel
hardware support flags.

The modification to outer UDP offload is still required pending a future
DPDK release.

Suggested-by: David Marchand 
Signed-off-by: Mike Pattrick 
---
 lib/netdev-dpdk.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 02cef6e45..95a78241d 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1363,8 +1363,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
   "net/ice, net/i40e or net/iavf port.",
   netdev_get_name(&dev->up));
 info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_OUTER_UDP_CKSUM;
-info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_VXLAN_TNL_TSO;
-info.tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_GENEVE_TNL_TSO;
 }
 
 if (info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM) {
-- 
2.39.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v4 2/3] Userspace: Software fallback for UDP encapsulated TCP segmentation.

2024-07-05 Thread Mike Pattrick
When sending packets that are flagged as requiring segmentation to an
interface that does not support this feature, send the packet to the TSO
software fallback instead of dropping it.

Signed-off-by: Mike Pattrick 
---
v2:
 - Fixed udp tunnel length
 - Added test that UDP headers are correct
 - Split inner and outer ip_id into different counters
 - Set tunnel flags in reset_tcp_seg

v3:
 - Changed logic of netdev_send() to account for NICs that support
 tunnel offload but not checksum offload
 - Adjusted udp tunnel header length during software tso

v4:
 - Moved a bugfix into its own patch
 - Fixed an indentation issue
 - Changed the scope of ip_hdr
Signed-off-by: Mike Pattrick 
---
 lib/dp-packet-gso.c | 90 -
 lib/dp-packet.h | 34 
 lib/netdev.c| 44 ++--
 tests/system-traffic.at | 87 +++
 4 files changed, 214 insertions(+), 41 deletions(-)

diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c
index 847685ad9..04ebb19da 100644
--- a/lib/dp-packet-gso.c
+++ b/lib/dp-packet-gso.c
@@ -47,6 +47,8 @@ dp_packet_gso_seg_new(const struct dp_packet *p, size_t 
hdr_len,
 seg->l2_5_ofs = p->l2_5_ofs;
 seg->l3_ofs = p->l3_ofs;
 seg->l4_ofs = p->l4_ofs;
+seg->inner_l3_ofs = p->inner_l3_ofs;
+seg->inner_l4_ofs = p->inner_l4_ofs;
 
 /* The protocol headers remain the same, so preserve hash and mark. */
 *dp_packet_rss_ptr(seg) = *dp_packet_rss_ptr(p);
@@ -71,7 +73,12 @@ dp_packet_gso_nr_segs(struct dp_packet *p)
 const char *data_tail;
 const char *data_pos;
 
-data_pos = dp_packet_get_tcp_payload(p);
+if (dp_packet_hwol_is_tunnel_vxlan(p) ||
+dp_packet_hwol_is_tunnel_geneve(p)) {
+data_pos = dp_packet_get_inner_tcp_payload(p);
+} else {
+data_pos = dp_packet_get_tcp_payload(p);
+}
 data_tail = (char *) dp_packet_tail(p) - dp_packet_l2_pad_size(p);
 
 return DIV_ROUND_UP(data_tail - data_pos, segsz);
@@ -89,14 +96,16 @@ dp_packet_gso(struct dp_packet *p, struct dp_packet_batch 
**batches)
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 struct dp_packet_batch *curr_batch = *batches;
 struct tcp_header *tcp_hdr;
-struct ip_header *ip_hdr;
+uint16_t inner_ip_id = 0;
+uint16_t outer_ip_id = 0;
 struct dp_packet *seg;
 uint16_t tcp_offset;
 uint16_t tso_segsz;
 uint32_t tcp_seq;
-uint16_t ip_id;
+bool outer_ipv4;
 int hdr_len;
 int seg_len;
+bool tnl;
 
 tso_segsz = dp_packet_get_tso_segsz(p);
 if (!tso_segsz) {
@@ -105,20 +114,37 @@ dp_packet_gso(struct dp_packet *p, struct dp_packet_batch 
**batches)
 return false;
 }
 
-tcp_hdr = dp_packet_l4(p);
+if (dp_packet_hwol_is_tunnel_vxlan(p) ||
+dp_packet_hwol_is_tunnel_geneve(p)) {
+outer_ipv4 = dp_packet_hwol_is_outer_ipv4(p);
+tcp_hdr = dp_packet_inner_l4(p);
+tnl = true;
+
+if (outer_ipv4) {
+outer_ip_id = ntohs(((struct ip_header *) dp_packet_l3(p))->ip_id);
+}
+if (dp_packet_hwol_is_ipv4(p)) {
+struct ip_header *ip_hdr = dp_packet_inner_l3(p);
+inner_ip_id = ntohs(ip_hdr->ip_id);
+}
+} else {
+outer_ipv4 = dp_packet_hwol_is_ipv4(p);
+tcp_hdr = dp_packet_l4(p);
+tnl = false;
+
+if (outer_ipv4) {
+struct ip_header *ip_hdr = dp_packet_l3(p);
+outer_ip_id = ntohs(ip_hdr->ip_id);
+}
+}
+
 tcp_offset = TCP_OFFSET(tcp_hdr->tcp_ctl);
 tcp_seq = ntohl(get_16aligned_be32(&tcp_hdr->tcp_seq));
-hdr_len = ((char *) dp_packet_l4(p) - (char *) dp_packet_eth(p))
+hdr_len = ((char *) tcp_hdr - (char *) dp_packet_eth(p))
   + tcp_offset * 4;
-ip_id = 0;
-if (dp_packet_hwol_is_ipv4(p)) {
-ip_hdr = dp_packet_l3(p);
-ip_id = ntohs(ip_hdr->ip_id);
-}
-
 const char *data_tail = (char *) dp_packet_tail(p)
 - dp_packet_l2_pad_size(p);
-const char *data_pos = dp_packet_get_tcp_payload(p);
+const char *data_pos = (char *) tcp_hdr + tcp_offset * 4;
 int n_segs = dp_packet_gso_nr_segs(p);
 
 for (int i = 0; i < n_segs; i++) {
@@ -130,14 +156,36 @@ dp_packet_gso(struct dp_packet *p, struct dp_packet_batch 
**batches)
 seg = dp_packet_gso_seg_new(p, hdr_len, data_pos, seg_len);
 data_pos += seg_len;
 
+if (tnl) {
+/* Update tunnel UDP header length. */
+struct udp_header *tnl_hdr;
+
+tnl_hdr = dp_packet_l4(seg);
+tnl_hdr->udp_len = htons(dp_packet_l4_size(seg));
+
+/* Update tunnel inner L3 header. */
+if (dp_packet_hwol_is_ipv4(seg)) {
+struct ip_header *ip_hdr = dp_packet_inner_l3(seg);
+ip_hdr->ip_tot_len = htons(dp_packet_inner_l3_size(seg));
+ip_hdr->ip_id = htons(inner

[ovs-dev] [PATCH v4 1/3] userspace: Adjust segment size on encapsulation.

2024-07-05 Thread Mike Pattrick
When prepending a tunnel header to a packet marked for segmentation, we
need to adjust the segment size. Failure to do so can result in packets
that are larger than the intended MTU post segmentation.

Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
Signed-off-by: Mike Pattrick 
---
 lib/netdev-native-tnl.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 0f9f07f44..da0bb6fda 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -161,6 +161,17 @@ netdev_tnl_push_ip_header(struct dp_packet *packet, const 
void *header,
 *ip_tot_size = dp_packet_size(packet) - sizeof (struct eth_header);
 
 memcpy(eth, header, size);
+
+/* The prepended header may cause TSO marked packets to exceed the intended
+ * MTU on segmentation. */
+if (dp_packet_hwol_is_tso(packet)) {
+uint16_t tso_segsz = dp_packet_get_tso_segsz(packet);
+if (tso_segsz > size) {
+tso_segsz -= size;
+dp_packet_set_tso_segsz(packet, tso_segsz);
+}
+}
+
 /* The encapsulated packet has type Ethernet. Adjust dp_packet. */
 packet->packet_type = htonl(PT_ETH);
 dp_packet_reset_offsets(packet);
-- 
2.39.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v5 ovn 1/3] northd: Introduce ECMP_Nexthop table in SB db.

2024-07-05 Thread 0-day Robot
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.


git-am:
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 northd: Introduce ECMP_Nexthop table in SB db.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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 v5 ovn 3/3] ofctrl: Introduce ecmp_nexthop_monitor.

2024-07-05 Thread Lorenzo Bianconi
Introduce ecmp_nexthop_monitor in ovn-controller in order to track and
flush ecmp-symmetric reply ct entires when requested by the CMS (e.g
removing the related static routes).

Signed-off-by: Lorenzo Bianconi 
---
 controller/ofctrl.c | 54 +
 controller/ofctrl.h |  2 ++
 controller/ovn-controller.c |  2 ++
 tests/system-ovn.at |  8 ++
 4 files changed, 66 insertions(+)

diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 9d181a782..2c891c049 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -388,9 +388,17 @@ struct meter_band_entry {
 
 static struct shash meter_bands;
 
+#define ECMP_NEXTHOP_IDS_LEN65535
+static unsigned long *ecmp_nexthop_ids;
+
 static void ofctrl_meter_bands_destroy(void);
 static void ofctrl_meter_bands_clear(void);
 
+static void ecmp_nexthop_monitor_run(
+const struct sbrec_ecmp_nexthop_table *enh_table,
+struct ovs_list *msgs);
+
+
 /* MFF_* field ID for our Geneve option.  In S_TLV_TABLE_MOD_SENT, this is
  * the option we requested (we don't know whether we obtained it yet).  In
  * S_CLEAR_FLOWS or S_UPDATE_FLOWS, this is really the option we have. */
@@ -429,6 +437,7 @@ ofctrl_init(struct ovn_extend_table *group_table,
 groups = group_table;
 meters = meter_table;
 shash_init(&meter_bands);
+ecmp_nexthop_ids = bitmap_allocate(ECMP_NEXTHOP_IDS_LEN);
 }
 
 /* S_NEW, for a new connection.
@@ -876,6 +885,7 @@ ofctrl_destroy(void)
 expr_symtab_destroy(&symtab);
 shash_destroy(&symtab);
 ofctrl_meter_bands_destroy();
+bitmap_free(ecmp_nexthop_ids);
 }
 
 uint64_t
@@ -2305,6 +2315,47 @@ add_meter(struct ovn_extend_table_info *m_desired,
 ofctrl_meter_bands_alloc(sb_meter, m_desired, msgs);
 }
 
+static void
+ecmp_nexthop_monitor_flush_ct_entry(uint64_t id, struct ovs_list *msgs)
+{
+ovs_u128 mask = {
+/* ct_labels.label BITS[96-127] */
+.u64.hi = 0x,
+};
+ovs_u128 nexthop = {
+.u64.hi = id << 32,
+};
+struct ofp_ct_match match = {
+.labels = nexthop,
+.labels_mask = mask,
+};
+struct ofpbuf *msg = ofp_ct_match_encode(&match, NULL,
+ rconn_get_version(swconn));
+ovs_list_push_back(msgs, &msg->list_node);
+}
+
+static void
+ecmp_nexthop_monitor_run(const struct sbrec_ecmp_nexthop_table *enh_table,
+ struct ovs_list *msgs)
+{
+unsigned long *ids = bitmap_allocate(ECMP_NEXTHOP_IDS_LEN);
+
+const struct sbrec_ecmp_nexthop *sbrec_ecmp_nexthop;
+SBREC_ECMP_NEXTHOP_TABLE_FOR_EACH (sbrec_ecmp_nexthop, enh_table) {
+bitmap_set1(ids, sbrec_ecmp_nexthop->id);
+}
+
+int id;
+BITMAP_FOR_EACH_1 (id, ECMP_NEXTHOP_IDS_LEN, ecmp_nexthop_ids) {
+if (!bitmap_is_set(ids, id)) {
+ecmp_nexthop_monitor_flush_ct_entry(id, msgs);
+}
+}
+
+bitmap_free(ecmp_nexthop_ids);
+ecmp_nexthop_ids = ids;
+}
+
 static void
 installed_flow_add(struct ovn_flow *d,
struct ofputil_bundle_ctrl_msg *bc,
@@ -2663,6 +2714,7 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table,
struct shash *pending_ct_zones,
struct hmap *pending_lb_tuples,
struct ovsdb_idl_index *sbrec_meter_by_name,
+   const struct sbrec_ecmp_nexthop_table *enh_table,
uint64_t req_cfg,
bool lflows_changed,
bool pflows_changed)
@@ -2703,6 +2755,8 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table,
 /* OpenFlow messages to send to the switch to bring it up-to-date. */
 struct ovs_list msgs = OVS_LIST_INITIALIZER(&msgs);
 
+ecmp_nexthop_monitor_run(enh_table, &msgs);
+
 /* Iterate through ct zones that need to be flushed. */
 struct shash_node *iter;
 SHASH_FOR_EACH(iter, pending_ct_zones) {
diff --git a/controller/ofctrl.h b/controller/ofctrl.h
index 129e3b6ad..33953a8a4 100644
--- a/controller/ofctrl.h
+++ b/controller/ofctrl.h
@@ -31,6 +31,7 @@ struct ofpbuf;
 struct ovsrec_bridge;
 struct ovsrec_open_vswitch_table;
 struct sbrec_meter_table;
+struct sbrec_ecmp_nexthop_table;
 struct shash;
 
 struct ovn_desired_flow_table {
@@ -59,6 +60,7 @@ void ofctrl_put(struct ovn_desired_flow_table *lflow_table,
 struct shash *pending_ct_zones,
 struct hmap *pending_lb_tuples,
 struct ovsdb_idl_index *sbrec_meter_by_name,
+const struct sbrec_ecmp_nexthop_table *enh_table,
 uint64_t nb_cfg,
 bool lflow_changed,
 bool pflow_changed);
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d4523a25a..e707f03c7 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -6076,6 +6076,8 @@ main(int argc, char *argv[])
&ct_zones_data->pending,
&lb_data->removed_tu

[ovs-dev] [PATCH v5 ovn 2/3] northd: Add nexhop id in ct_label.label.

2024-07-05 Thread Lorenzo Bianconi
Introduce the nexthop identifier in the ct_label.label field for
ecmp-symmetric replies connections. This field will be used by
ovn-controller to track ct entries and to flush them if requested by the
CMS (e.g. removing the related static routes).

Signed-off-by: Lorenzo Bianconi 
---
 northd/en-lflow.c|  3 +++
 northd/inc-proc-northd.c |  1 +
 northd/northd.c  | 34 ---
 northd/northd.h  |  1 +
 tests/ovn.at |  4 +--
 tests/system-ovn.at  | 58 +++-
 6 files changed, 65 insertions(+), 36 deletions(-)

diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 3dba5034b..b4df49076 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -54,6 +54,8 @@ lflow_get_input_data(struct engine_node *node,
 engine_get_input_data("lr_stateful", node);
 struct ed_type_ls_stateful *ls_stateful_data =
 engine_get_input_data("ls_stateful", node);
+struct ecmp_nexthop_data *nexthop_data =
+engine_get_input_data("ecmp_nexthop", node);
 
 lflow_input->sbrec_logical_flow_table =
 EN_OVSDB_GET(engine_get_input("SB_logical_flow", node));
@@ -83,6 +85,7 @@ lflow_get_input_data(struct engine_node *node,
 lflow_input->parsed_routes = &static_routes_data->parsed_routes;
 lflow_input->route_tables = &static_routes_data->route_tables;
 lflow_input->route_policies = &route_policies_data->route_policies;
+lflow_input->nexthops_table = &nexthop_data->nexthops;
 
 struct ed_type_global_config *global_config =
 engine_get_input_data("global_config", node);
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index c768c29fe..8df9c7f51 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -285,6 +285,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
 engine_add_input(&en_lflow, &en_bfd_nb_sync, NULL);
 engine_add_input(&en_lflow, &en_route_policies, NULL);
 engine_add_input(&en_lflow, &en_static_routes, NULL);
+engine_add_input(&en_lflow, &en_ecmp_nexthop, NULL);
 engine_add_input(&en_lflow, &en_global_config,
  node_global_config_handler);
 engine_add_input(&en_lflow, &en_northd, lflow_northd_handler);
diff --git a/northd/northd.c b/northd/northd.c
index c2c5cf964..0803f4626 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -10815,7 +10815,8 @@ add_ecmp_symmetric_reply_flows(struct lflow_table 
*lflows,
struct ovn_port *out_port,
const struct parsed_route *route,
struct ds *route_match,
-   struct lflow_ref *lflow_ref)
+   struct lflow_ref *lflow_ref,
+   struct simap *nexthops_table)
 {
 const struct nbrec_logical_router_static_route *st_route = route->route;
 struct ds match = DS_EMPTY_INITIALIZER;
@@ -10853,9 +10854,15 @@ add_ecmp_symmetric_reply_flows(struct lflow_table 
*lflows,
 ds_put_cstr(&match, " && !ct.rpl && (ct.new || ct.est)");
 ds_put_format(&actions,
 "ct_commit { ct_label.ecmp_reply_eth = eth.src; "
-" %s = %" PRId64 ";}; "
-"next;",
+" %s = %" PRId64 ";",
 ct_ecmp_reply_port_match, out_port->sb->tunnel_key);
+
+struct simap_node *n = simap_find(nexthops_table, st_route->nexthop);
+if (n) {
+ds_put_format(&actions, " ct_label.label = %d;", n->data);
+}
+ds_put_cstr(&actions, " }; next;");
+
 ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ECMP_STATEFUL, 100,
 ds_cstr(&match), ds_cstr(&actions),
 &st_route->header_,
@@ -10913,7 +10920,8 @@ static void
 build_ecmp_route_flow(struct lflow_table *lflows, struct ovn_datapath *od,
   bool ct_masked_mark, const struct hmap *lr_ports,
   struct ecmp_groups_node *eg,
-  struct lflow_ref *lflow_ref)
+  struct lflow_ref *lflow_ref,
+  struct simap *nexthops_table)
 
 {
 bool is_ipv4 = IN6_IS_ADDR_V4MAPPED(&eg->prefix);
@@ -10971,7 +10979,7 @@ build_ecmp_route_flow(struct lflow_table *lflows, 
struct ovn_datapath *od,
 add_ecmp_symmetric_reply_flows(lflows, od, ct_masked_mark,
lrp_addr_s, out_port,
route_, &route_match,
-   lflow_ref);
+   lflow_ref, nexthops_table);
 }
 ds_clear(&match);
 ds_put_format(&match, REG_ECMP_GROUP_ID" == %"PRIu16" && "
@@ -12865,7 +12873,8 @@ build_static_route_flows_for_lrouter(
 struct lflow_table *lflows, const struct hmap *lr_ports,
 struct hmap *parsed_routes,
 struct simap *route_tables,
-struct lflow_ref *lflow_ref)
+struct l

[ovs-dev] [PATCH v5 ovn 1/3] northd: Introduce ECMP_Nexthop table in SB db.

2024-07-05 Thread Lorenzo Bianconi
Introduce ECMP_Nexthop table in the SB db in order to track active
ecmp-symmetric-reply connections and flush stale ones.

Signed-off-by: Lorenzo Bianconi 
---
 northd/en-northd.c   | 33 +++
 northd/en-northd.h   |  4 ++
 northd/inc-proc-northd.c |  7 +++-
 northd/northd.c  | 89 
 northd/northd.h  | 10 +
 ovn-sb.ovsschema | 18 +++-
 ovn-sb.xml   | 31 ++
 tests/ovn-northd.at  |  4 ++
 8 files changed, 193 insertions(+), 3 deletions(-)

diff --git a/northd/en-northd.c b/northd/en-northd.c
index 7595c6f5f..f90d981f5 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -392,6 +392,23 @@ en_bfd_nb_sync_run(struct engine_node *node OVS_UNUSED,
 &route_policies_data->bfd_active_connections);
 }
 
+void
+en_ecmp_nexthop_run(struct engine_node *node, void *data)
+{
+const struct engine_context *eng_ctx = engine_get_context();
+struct static_routes_data *static_routes_data =
+engine_get_input_data("static_routes", node);
+struct ecmp_nexthop_data *enh_data = data;
+const struct sbrec_ecmp_nexthop_table *sbrec_ecmp_nexthop_table =
+EN_OVSDB_GET(engine_get_input("SB_ecmp_nexthop", node));
+
+build_ecmp_nexthop_table(eng_ctx->ovnsb_idl_txn,
+ &static_routes_data->parsed_routes,
+ &enh_data->nexthops,
+ sbrec_ecmp_nexthop_table);
+engine_set_node_state(node, EN_UPDATED);
+}
+
 void
 *en_northd_init(struct engine_node *node OVS_UNUSED,
 struct engine_arg *arg OVS_UNUSED)
@@ -439,6 +456,16 @@ void *en_bfd_nb_sync_init(struct engine_node *node 
OVS_UNUSED,
 return NULL;
 }
 
+void
+*en_ecmp_nexthop_init(struct engine_node *node OVS_UNUSED,
+  struct engine_arg *arg OVS_UNUSED)
+{
+struct ecmp_nexthop_data *data = xzalloc(sizeof *data);
+
+ecmp_nexthop_init(data);
+return data;
+}
+
 void
 en_northd_cleanup(void *data)
 {
@@ -474,3 +501,9 @@ void
 en_bfd_nb_sync_cleanup(void *data OVS_UNUSED)
 {
 }
+
+void
+en_ecmp_nexthop_cleanup(void *data)
+{
+ecmp_nexthop_destroy(data);
+}
diff --git a/northd/en-northd.h b/northd/en-northd.h
index 53b671331..692779559 100644
--- a/northd/en-northd.h
+++ b/northd/en-northd.h
@@ -42,5 +42,9 @@ void *en_bfd_nb_sync_init(struct engine_node *node OVS_UNUSED,
 void en_bfd_nb_sync_run(struct engine_node *node OVS_UNUSED,
 void *data OVS_UNUSED);
 void en_bfd_nb_sync_cleanup(void *data OVS_UNUSED);
+void en_ecmp_nexthop_run(struct engine_node *node, void *data);
+void *en_ecmp_nexthop_init(struct engine_node *node OVS_UNUSED,
+   struct engine_arg *arg OVS_UNUSED);
+void en_ecmp_nexthop_cleanup(void *data);
 
 #endif /* EN_NORTHD_H */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 9d4d928d1..c768c29fe 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -103,7 +103,8 @@ static unixctl_cb_func chassis_features_list;
 SB_NODE(fdb, "fdb") \
 SB_NODE(static_mac_binding, "static_mac_binding") \
 SB_NODE(chassis_template_var, "chassis_template_var") \
-SB_NODE(logical_dp_group, "logical_dp_group")
+SB_NODE(logical_dp_group, "logical_dp_group") \
+SB_NODE(ecmp_nexthop, "ecmp_nexthop")
 
 enum sb_engine_node {
 #define SB_NODE(NAME, NAME_STR) SB_##NAME,
@@ -161,6 +162,7 @@ static ENGINE_NODE(route_policies, "route_policies");
 static ENGINE_NODE(static_routes, "static_routes");
 static ENGINE_NODE(bfd, "bfd");
 static ENGINE_NODE(bfd_nb_sync, "bfd_nb_sync");
+static ENGINE_NODE(ecmp_nexthop, "ecmp_nexthop");
 
 void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
   struct ovsdb_idl_loop *sb)
@@ -266,6 +268,9 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
 engine_add_input(&en_bfd_nb_sync, &en_static_routes, NULL);
 engine_add_input(&en_bfd_nb_sync, &en_route_policies, NULL);
 
+engine_add_input(&en_ecmp_nexthop, &en_sb_ecmp_nexthop, NULL);
+engine_add_input(&en_ecmp_nexthop, &en_static_routes, NULL);
+
 engine_add_input(&en_sync_meters, &en_nb_acl, NULL);
 engine_add_input(&en_sync_meters, &en_nb_meter, NULL);
 engine_add_input(&en_sync_meters, &en_sb_meter, NULL);
diff --git a/northd/northd.c b/northd/northd.c
index 876705053..c2c5cf964 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -9964,6 +9964,83 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
 bitmap_free(bfd_src_ports);
 }
 
+#define NEXTHOP_IDS_LEN65535
+bool
+build_ecmp_nexthop_table(
+struct ovsdb_idl_txn *ovnsb_txn,
+struct hmap *routes,
+struct simap *nexthops,
+const struct sbrec_ecmp_nexthop_table *sbrec_ecmp_nexthop_table)
+{
+unsigned long *nexthop_ids = bitmap_allocate(NEXTHOP_IDS_LEN);
+bool ret = false;
+
+if (!ovnsb_txn) {
+return false;
+}
+
+const 

[ovs-dev] [PATCH v5 ovn 0/3] Introduce ECMP_nexthop monitor in ovn-controller

2024-07-05 Thread Lorenzo Bianconi
Reported-at: https://issues.redhat.com/browse/FDP-56

Changes since v4:
- rebase on top of https://patchwork.ozlabs.org/project/ovn/list/?series=414052
Changes since v3:
- rebase on top of https://patchwork.ozlabs.org/project/ovn/list/?series=409660
- use bitmap for id tracking in ofctrl.c
- use simap for id tracking in northd.c
- cosmetics
Changes since v2:
- rebase on top of https://patchwork.ozlabs.org/project/ovn/list/?series=409660
  in order to use new bfd and static_routes maps
- add IP northd node for ecmp_nexthop processing
Changes since v1:
- add ID column in ECMP_Nexthop table in SB db
- remove nexthop-id in logical_router_static_route option column

Lorenzo Bianconi (3):
  northd: Introduce ECMP_Nexthop table in SB db.
  northd: Add nexhop id in ct_label.label.
  ofctrl: Introduce ecmp_nexthop_monitor.

 controller/ofctrl.c |  54 
 controller/ofctrl.h |   2 +
 controller/ovn-controller.c |   2 +
 northd/en-lflow.c   |   3 +
 northd/en-northd.c  |  33 ++
 northd/en-northd.h  |   4 ++
 northd/inc-proc-northd.c|   8 ++-
 northd/northd.c | 123 +---
 northd/northd.h |  11 
 ovn-sb.ovsschema|  18 +-
 ovn-sb.xml  |  31 +
 tests/ovn-northd.at |   4 ++
 tests/ovn.at|   4 +-
 tests/system-ovn.at |  66 ---
 14 files changed, 324 insertions(+), 39 deletions(-)

-- 
2.45.2

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn v4 1/4] controller: Move CT zone handling into separate module.

2024-07-05 Thread Numan Siddique
On Wed, Jun 26, 2024 at 1:56 AM Ales Musil  wrote:
>
> Move the CT zone handling specific bits into its own module. This
> allows for easier changes done within the module and separates the
> logic that is unrelated from ovn-controller.
>
> Signed-off-by: Ales Musil 
> Acked-by: Lorenzo Bianconi 

Thanks.  I applied the first 2 patches of this series to the main branch

Numan

> ---
> v4: Rebase on top of latest main.
> Added ack from Lorenzo.
> v3: Rebase on top of latest main.
> ---
>  controller/automake.mk  |   4 +-
>  controller/ct-zone.c| 378 ++
>  controller/ct-zone.h|  74 +++
>  controller/ofctrl.c |   3 +-
>  controller/ovn-controller.c | 393 +++-
>  controller/ovn-controller.h |  21 +-
>  controller/pinctrl.c|   2 +-
>  tests/ovn.at|   4 +-
>  8 files changed, 486 insertions(+), 393 deletions(-)
>  create mode 100644 controller/ct-zone.c
>  create mode 100644 controller/ct-zone.h
>
> diff --git a/controller/automake.mk b/controller/automake.mk
> index 1b1b3aeb1..ed93cfb3c 100644
> --- a/controller/automake.mk
> +++ b/controller/automake.mk
> @@ -47,7 +47,9 @@ controller_ovn_controller_SOURCES = \
> controller/mac-cache.h \
> controller/mac-cache.c \
> controller/statctrl.h \
> -   controller/statctrl.c
> +   controller/statctrl.c \
> +   controller/ct-zone.h \
> +   controller/ct-zone.c
>
>  controller_ovn_controller_LDADD = lib/libovn.la 
> $(OVS_LIBDIR)/libopenvswitch.la
>  man_MANS += controller/ovn-controller.8
> diff --git a/controller/ct-zone.c b/controller/ct-zone.c
> new file mode 100644
> index 0..3e37fedb6
> --- /dev/null
> +++ b/controller/ct-zone.c
> @@ -0,0 +1,378 @@
> +/* 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 "ct-zone.h"
> +#include "local_data.h"
> +#include "openvswitch/vlog.h"
> +
> +VLOG_DEFINE_THIS_MODULE(ct_zone);
> +
> +static void
> +ct_zone_restore(const struct sbrec_datapath_binding_table *dp_table,
> +struct ct_zone_ctx *ctx, const char *name, int zone);
> +static void ct_zone_add_pending(struct shash *pending_ct_zones,
> +enum ct_zone_pending_state state,
> +int zone, bool add, const char *name);
> +
> +void
> +ct_zones_restore(struct ct_zone_ctx *ctx,
> + const struct ovsrec_open_vswitch_table *ovs_table,
> + const struct sbrec_datapath_binding_table *dp_table,
> + const struct ovsrec_bridge *br_int)
> +{
> +memset(ctx->bitmap, 0, sizeof ctx->bitmap);
> +bitmap_set1(ctx->bitmap, 0); /* Zone 0 is reserved. */
> +
> +struct shash_node *pending_node;
> +SHASH_FOR_EACH (pending_node, &ctx->pending) {
> +struct ct_zone_pending_entry *ctpe = pending_node->data;
> +
> +if (ctpe->add) {
> +ct_zone_restore(dp_table, ctx, pending_node->name, ctpe->zone);
> +}
> +}
> +
> +const struct ovsrec_open_vswitch *cfg;
> +cfg = ovsrec_open_vswitch_table_first(ovs_table);
> +if (!cfg) {
> +return;
> +}
> +
> +if (!br_int) {
> +/* If the integration bridge hasn't been defined, assume that
> + * any existing ct-zone definitions aren't valid. */
> +return;
> +}
> +
> +struct smap_node *node;
> +SMAP_FOR_EACH (node, &br_int->external_ids) {
> +if (strncmp(node->key, "ct-zone-", 8)) {
> +continue;
> +}
> +
> +const char *user = node->key + 8;
> +if (!user[0]) {
> +continue;
> +}
> +
> +if (shash_find(&ctx->pending, user)) {
> +continue;
> +}
> +
> +unsigned int zone;
> +if (!str_to_uint(node->value, 10, &zone)) {
> +continue;
> +}
> +
> +ct_zone_restore(dp_table, ctx, user, zone);
> +}
> +}
> +
> +bool
> +ct_zone_assign_unused(struct ct_zone_ctx *ctx, const char *zone_name,
> +  int *scan_start)
> +{
> +/* We assume that there are 64K zones and that we own them all. */
> +int zone = bitmap_scan(ctx->bitmap, 0, *scan_start, MAX_CT_ZONES + 1);
> +if (zone == MAX_CT_ZONES + 1) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> +VLOG_WARN_RL(&rl, "exhausted all ct z

[ovs-dev] [PATCH v5 ovn] northd: Add bfd, static_routes and route_policies I-P nodes.

2024-07-05 Thread Lorenzo Bianconi
Introduce bfd, static_routes, route_policies and bfd_nb_sync nodes to northd I-P
engine to track bfd connections and northd static_route/policy_route
changes.

Reported-at: https://issues.redhat.com/browse/FDP-600
Signed-off-by: Lorenzo Bianconi 
---
Changes since v4:
- introduce bfd_nb_sync node and ger rid of bfd_consumer routine
Changes since v3:
- fix bfd_northd_change_handler logic
- fix route_policies_northd_change_handler logic
- fix static_routes_northd_change_handler logic
- add unit-tests
- cosmetics
Changes since v2:
- add incremental processing routines
- split bfd_consumer node in static_routes and route_policies nodes
Changes since v1:
- add incremental processing logic for bfd_consumer node to avoid a full
  recompute
---
 northd/en-lflow.c|  25 +-
 northd/en-northd.c   | 215 ++
 northd/en-northd.h   |  23 ++
 northd/inc-proc-northd.c |  37 ++-
 northd/northd.c  | 587 +++
 northd/northd.h  |  63 -
 tests/ovn-northd.at  |  56 
 7 files changed, 810 insertions(+), 196 deletions(-)

diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index c4b927fb8..3dba5034b 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -41,6 +41,11 @@ lflow_get_input_data(struct engine_node *node,
  struct lflow_input *lflow_input)
 {
 struct northd_data *northd_data = engine_get_input_data("northd", node);
+struct bfd_data *bfd_data = engine_get_input_data("bfd", node);
+struct static_routes_data *static_routes_data =
+engine_get_input_data("static_routes", node);
+struct route_policies_data *route_policies_data =
+engine_get_input_data("route_policies", node);
 struct port_group_data *pg_data =
 engine_get_input_data("port_group", node);
 struct sync_meters_data *sync_meters_data =
@@ -50,10 +55,6 @@ lflow_get_input_data(struct engine_node *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));
-lflow_input->sbrec_bfd_table =
-EN_OVSDB_GET(engine_get_input("SB_bfd", node));
 lflow_input->sbrec_logical_flow_table =
 EN_OVSDB_GET(engine_get_input("SB_logical_flow", node));
 lflow_input->sbrec_multicast_group_table =
@@ -78,7 +79,10 @@ lflow_get_input_data(struct engine_node *node,
 lflow_input->meter_groups = &sync_meters_data->meter_groups;
 lflow_input->lb_datapaths_map = &northd_data->lb_datapaths_map;
 lflow_input->svc_monitor_map = &northd_data->svc_monitor_map;
-lflow_input->bfd_connections = NULL;
+lflow_input->bfd_connections = &bfd_data->bfd_connections;
+lflow_input->parsed_routes = &static_routes_data->parsed_routes;
+lflow_input->route_tables = &static_routes_data->route_tables;
+lflow_input->route_policies = &route_policies_data->route_policies;
 
 struct ed_type_global_config *global_config =
 engine_get_input_data("global_config", node);
@@ -95,25 +99,14 @@ void en_lflow_run(struct engine_node *node, void *data)
 struct lflow_input lflow_input;
 lflow_get_input_data(node, &lflow_input);
 
-struct hmap bfd_connections = HMAP_INITIALIZER(&bfd_connections);
-lflow_input.bfd_connections = &bfd_connections;
-
 stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
 
 struct lflow_data *lflow_data = data;
 lflow_table_clear(lflow_data->lflow_table);
 lflow_reset_northd_refs(&lflow_input);
 
-build_bfd_table(eng_ctx->ovnsb_idl_txn,
-lflow_input.nbrec_bfd_table,
-lflow_input.sbrec_bfd_table,
-lflow_input.lr_ports,
-&bfd_connections);
 build_lflows(eng_ctx->ovnsb_idl_txn, &lflow_input,
  lflow_data->lflow_table);
-bfd_cleanup_connections(lflow_input.nbrec_bfd_table,
-&bfd_connections);
-hmap_destroy(&bfd_connections);
 stopwatch_stop(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
 
 engine_set_node_state(node, EN_UPDATED);
diff --git a/northd/en-northd.c b/northd/en-northd.c
index 4479b4aff..7595c6f5f 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -236,6 +236,162 @@ northd_global_config_handler(struct engine_node *node, 
void *data OVS_UNUSED)
 return true;
 }
 
+bool
+route_policies_northd_change_handler(struct engine_node *node,
+ void *data OVS_UNUSED)
+{
+struct northd_data *northd_data = engine_get_input_data("northd", node);
+if (!northd_has_tracked_data(&northd_data->trk_data)) {
+return false;
+}
+
+/* This node uses the below data from the en_northd engine node.
+ * See (lr_stateful_get_input_data())
+ *   1. northd_data->lr_datapaths
+ *  This data gets updated when a logical router is created or deleted.
+ *  northd engine node presently falls ba

[ovs-dev] [PATCH v2 ovn] nbctl: Added local link ipv6 to nbctl show.

2024-07-05 Thread Mj Ponsonby
From: MJ Ponsonby 


This commit modifies a test to deal with the changed,
response from the command
This commit also modifies the function of ovn-nbctl show,
to also return the ipv6 link local address as mentioned

This information would be useful to have with the wider useage of
advertising IPv4 Prefixes over IPv6 next hops [0]

0: https://datatracker.ietf.org/doc/html/rfc5549

Reported-at: https://launchpad.net/bugs/2069804
Signed-off-by: MJ Ponsonby 
---
 tests/ovn-nbctl.at|  1 +
 utilities/ovn-nbctl.c | 10 ++
 2 files changed, 11 insertions(+)

diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 19c83a4a5..de014e1f9 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1670,6 +1670,7 @@ AT_CHECK([ovn-nbctl show lr0 | uuidfilt], [0], [dnl
 router <0> (lr0)
 port lrp0
 mac: "00:00:00:01:02:03"
+ipv6-lla: "fe80::200:ff:fe01:203"
 networks: [["192.168.1.1/24"]]
 ])
 
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 32ca4f750..04c123022 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -780,6 +780,16 @@ print_lr(const struct nbrec_logical_router *lr, struct ds 
*s)
 if (lrp->mac) {
 ds_put_cstr(s, "mac: ");
 ds_put_format(s, "\"%s\"\n", lrp->mac);
+
+/* Have the mac address in an array. */
+struct eth_addr ea;
+eth_addr_from_string(lrp->mac, &ea);
+struct in6_addr lla;
+in6_generate_lla(ea, &lla);
+
+ds_put_cstr(s, "ipv6-lla: \"");
+ipv6_format_addr(&lla, s);
+ds_put_cstr(s, "\"\n");
 }
 if (lrp->n_networks) {
 ds_put_cstr(s, "networks: [");
-- 
2.43.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v1] netdev-native-tnl: Fix inner L2 pad loss causing checksum errors.

2024-07-05 Thread Mike Pattrick
On Tue, Jul 2, 2024 at 10:56 PM Jun Wang  wrote:
>
> We encountered a scenario where, if the received packet contains
> padding bytes, and we then add Geneve tunnel encapsulation without
> carrying the padding bytes, it results in checksum errors when sending
> out. Therefore, adding an inner_l2_pad is necessary.
>
> For example, this type of packet format:
>    06 6c 6a 71 e1 d3 6c e2 d3 8b ea 24 81 00 03 e9
> 0010   08 00 45 00 00 28 9d e5 40 00 3b 06 6e 64 0a 6f
> 0020   05 14 0a fe 19 06 01 bb 22 ae 59 c0 8c 61 8e 26
> 0030   14 e3 50 10 00 7f ce 3a 00 00 00 00 9e 38 cf 64

Hello Jun,

Thank you for this submission. This is an interesting case and I don't
know that we have an appropriate test case for micrograms like this.
Was this the

One question I have is shouldn't we remove the padding while we
encapsulate, not keep it? The encapsulation should always push the
frame size past 64 bytes.


Thanks,
M

>
> Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
>
> Signed-off-by: Jun Wang 
> ---
>  lib/dp-packet.h | 21 -
>  lib/netdev-native-tnl.c |  3 +++
>  2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index a75b1c5..d583b28 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -176,6 +176,8 @@ struct dp_packet {
>  ovs_be32 packet_type;  /* Packet type as defined in OpenFlow */
>  uint16_t csum_start;   /* Position to start checksumming from. */
>  uint16_t csum_offset;  /* Offset to place checksum. */
> +uint16_t inner_l2_pad_size;/* Detected inner l2 padding size.
> +* Padding is non-pullable. */
>  union {
>  struct pkt_metadata md;
>  uint64_t data[DP_PACKET_CONTEXT_SIZE / 8];
> @@ -209,7 +211,10 @@ static inline void *dp_packet_eth(const struct dp_packet 
> *);
>  static inline void dp_packet_reset_offsets(struct dp_packet *);
>  static inline void dp_packet_reset_offload(struct dp_packet *);
>  static inline uint16_t dp_packet_l2_pad_size(const struct dp_packet *);
> +static inline uint16_t dp_packet_inner_l2_pad_size(const struct dp_packet *);
>  static inline void dp_packet_set_l2_pad_size(struct dp_packet *, uint16_t);
> +static inline void dp_packet_set_inner_l2_pad_size(struct dp_packet *,
> +   uint16_t);
>  static inline void *dp_packet_l2_5(const struct dp_packet *);
>  static inline void dp_packet_set_l2_5(struct dp_packet *, void *);
>  static inline void *dp_packet_l3(const struct dp_packet *);
> @@ -435,6 +440,7 @@ static inline void
>  dp_packet_reset_offsets(struct dp_packet *b)
>  {
>  b->l2_pad_size = 0;
> +b->inner_l2_pad_size = 0;
>  b->l2_5_ofs = UINT16_MAX;
>  b->l3_ofs = UINT16_MAX;
>  b->l4_ofs = UINT16_MAX;
> @@ -448,6 +454,12 @@ dp_packet_l2_pad_size(const struct dp_packet *b)
>  return b->l2_pad_size;
>  }
>
> +static inline uint16_t
> +dp_packet_inner_l2_pad_size(const struct dp_packet *b)
> +{
> +return b->inner_l2_pad_size;
> +}
> +
>  static inline void
>  dp_packet_set_l2_pad_size(struct dp_packet *b, uint16_t pad_size)
>  {
> @@ -455,6 +467,13 @@ dp_packet_set_l2_pad_size(struct dp_packet *b, uint16_t 
> pad_size)
>  b->l2_pad_size = pad_size;
>  }
>
> +static inline void
> +dp_packet_set_inner_l2_pad_size(struct dp_packet *b, uint16_t pad_size)
> +{
> +ovs_assert(pad_size <= dp_packet_size(b));
> +b->inner_l2_pad_size = pad_size;
> +}
> +
>  static inline void *
>  dp_packet_l2_5(const struct dp_packet *b)
>  {
> @@ -543,7 +562,7 @@ dp_packet_inner_l4_size(const struct dp_packet *b)
>  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)
> +   - dp_packet_inner_l2_pad_size(b)
> : 0;
>  }
>
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index 0f9f07f..96ffdc1 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -156,6 +156,7 @@ netdev_tnl_push_ip_header(struct dp_packet *packet, const 
> void *header,
>  struct eth_header *eth;
>  struct ip_header *ip;
>  struct ovs_16aligned_ip6_hdr *ip6;
> +uint16_t l2_pad_size;
>
>  eth = dp_packet_push_uninit(packet, size);
>  *ip_tot_size = dp_packet_size(packet) - sizeof (struct eth_header);
> @@ -163,7 +164,9 @@ netdev_tnl_push_ip_header(struct dp_packet *packet, const 
> void *header,
>  memcpy(eth, header, size);
>  /* The encapsulated packet has type Ethernet. Adjust dp_packet. */
>  packet->packet_type = htonl(PT_ETH);
> +l2_pad_size = dp_packet_l2_pad_size(packet);
>  dp_packet_reset_offsets(packet);
> +dp_packet_set_inner_l2_pad_size(packet, l2_pad_size);
>  packet->l3_ofs = sizeof (struct eth_header);
>
>  if (netdev_tnl_is_header_ipv6(header)) {
> --
> 1.8.3.1
>
>
>
> ___

Re: [ovs-dev] [PATCH net-next 0/3] selftests: openvswitch: Address some flakes in the CI environment

2024-07-05 Thread Adrián Moreno
On Fri, Jul 05, 2024 at 09:49:12AM GMT, Aaron Conole wrote:
> Jakub Kicinski  writes:
>
> > On Tue,  2 Jul 2024 09:28:27 -0400 Aaron Conole wrote:
> >> These patches aim to make using the openvswitch testsuite more reliable.
> >> These should address the major sources of flakiness in the openvswitch
> >> test suite allowing the CI infrastructure to exercise the openvswitch
> >> module for patch series.  There should be no change for users who simply
> >> run the tests (except that patch 3/3 does make some of the debugging a bit
> >> easier by making some output more verbose).
> >
> > Hi Aaron!
> >
> > The results look solid on normal builds now, but with a debug kernel
> > the test is failing consistently:
> >
> > https://netdev.bots.linux.dev/contest.html?executor=vmksft-net-dbg&test=openvswitch-sh
>
> Yes - it shows a test case issue with the upcall and psample tests.
>
> Adrian and I discussed the correct approach would be using a wait_for
> instead of just sleeping, because it seems the dbg environment might be
> too racy.  I think he is working on a follow up to submit after the
> psample work gets merged - we were hoping not to hold that patch series
> up with more potential conflicts or merge issues if that's okay.
>

Yes. I am working on a patch to solve the failures in slow systems.

Thanks.
Adrián

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net-next 0/3] selftests: openvswitch: Address some flakes in the CI environment

2024-07-05 Thread Jakub Kicinski
On Fri, 05 Jul 2024 09:49:12 -0400 Aaron Conole wrote:
> > The results look solid on normal builds now, but with a debug kernel
> > the test is failing consistently:
> >
> > https://netdev.bots.linux.dev/contest.html?executor=vmksft-net-dbg&test=openvswitch-sh
> >   
> 
> Yes - it shows a test case issue with the upcall and psample tests.
> 
> Adrian and I discussed the correct approach would be using a wait_for
> instead of just sleeping, because it seems the dbg environment might be
> too racy.  I think he is working on a follow up to submit after the
> psample work gets merged - we were hoping not to hold that patch series
> up with more potential conflicts or merge issues if that's okay.

Makes sense, thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net-next 0/3] selftests: openvswitch: Address some flakes in the CI environment

2024-07-05 Thread Aaron Conole
Jakub Kicinski  writes:

> On Tue,  2 Jul 2024 09:28:27 -0400 Aaron Conole wrote:
>> These patches aim to make using the openvswitch testsuite more reliable.
>> These should address the major sources of flakiness in the openvswitch
>> test suite allowing the CI infrastructure to exercise the openvswitch
>> module for patch series.  There should be no change for users who simply
>> run the tests (except that patch 3/3 does make some of the debugging a bit
>> easier by making some output more verbose).
>
> Hi Aaron!
>
> The results look solid on normal builds now, but with a debug kernel
> the test is failing consistently:
>
> https://netdev.bots.linux.dev/contest.html?executor=vmksft-net-dbg&test=openvswitch-sh

Yes - it shows a test case issue with the upcall and psample tests.

Adrian and I discussed the correct approach would be using a wait_for
instead of just sleeping, because it seems the dbg environment might be
too racy.  I think he is working on a follow up to submit after the
psample work gets merged - we were hoping not to hold that patch series
up with more potential conflicts or merge issues if that's okay.

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net-next 0/3] selftests: openvswitch: Address some flakes in the CI environment

2024-07-05 Thread Jakub Kicinski
On Tue,  2 Jul 2024 09:28:27 -0400 Aaron Conole wrote:
> These patches aim to make using the openvswitch testsuite more reliable.
> These should address the major sources of flakiness in the openvswitch
> test suite allowing the CI infrastructure to exercise the openvswitch
> module for patch series.  There should be no change for users who simply
> run the tests (except that patch 3/3 does make some of the debugging a bit
> easier by making some output more verbose).

Hi Aaron!

The results look solid on normal builds now, but with a debug kernel
the test is failing consistently:

https://netdev.bots.linux.dev/contest.html?executor=vmksft-net-dbg&test=openvswitch-sh
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn v6 0/2] Add support to disable VXLAN mode.

2024-07-05 Thread Aleksey Baulin via dev
That thread definitely answers my question.
Thanks, Vladislav!

On Fri, Jul 5, 2024 at 2:51 PM Vladislav Odintsov  wrote:
>
> Hi Alexey,
>
> The discussion for explicit configuration vs automatic determining VXLAN mode 
> can be found reading the next thread [1].
>
> 1: https://mail.openvswitch.org/pipermail/ovs-dev/2024-April/412986.html
>
> regards,
>
> Vladislav Odintsov

-- 
Aleksey Baulin
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn v6 0/2] Add support to disable VXLAN mode.

2024-07-05 Thread Vladislav Odintsov
Hi Alexey,

The discussion for explicit configuration vs automatic determining VXLAN mode 
can be found reading the next thread [1].

1: https://mail.openvswitch.org/pipermail/ovs-dev/2024-April/412986.html

regards,
 
Vladislav Odintsov

-Original Message-
From: dev  on behalf of Aleksey Baulin via dev 

Reply to: Aleksey Baulin 
Date: Friday, 5 July 2024 at 14:32
To: "ovs-dev@openvswitch.org" 
Subject: Re: [ovs-dev] [PATCH ovn v6 0/2] Add support to disable VXLAN mode.

I missed the discussion on this patch set. I can see that it's been
accepted. Still, I'd like to ask a question.

Originally there was the VTEP-only VxLAN scenario that worked great in
OVN. Then, the patch by Ihar Hrachyshka provided support for internal
VxLANs. The tunnel id space was severely limited which, in turn,
became the limitation for the VTEP-only VxLAN scenario as well. In
other words, the patch that introduced internal VxLANs broke the
scenario for VTEP-only VxLANs.

In essence, Vladislav Odintsov asserts that the patch by Ihar
Hrachyshka introduced an implicit configuration option the state of
which was determined in software (the function is_vxlan_mode()). The
new patch by Vladislav Odintsov makes that option explicit. That makes
the behavior of a configured cluster completely depend on its value -
as opposed to determining the behavior in software from configuration.

On the one hand, that makes it simple - one option to control whether
the cluster supports internal VxLANs or it is VTEP-only VxLANs. On the
other hand I can't help but wonder - why can't that be determined in
software - like in the original patch? I would think that all
necessary data is present in a configuration and there's no need for
an extra explicit option. With the new patch from Vladislav Odintsov
it can be done just once in one place.

So the question is: what are the pros and cons of each variant? Why is
the variant with a new option chosen?

Thanks!
___
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 v6 0/2] Add support to disable VXLAN mode.

2024-07-05 Thread Aleksey Baulin via dev
I missed the discussion on this patch set. I can see that it's been
accepted. Still, I'd like to ask a question.

Originally there was the VTEP-only VxLAN scenario that worked great in
OVN. Then, the patch by Ihar Hrachyshka provided support for internal
VxLANs. The tunnel id space was severely limited which, in turn,
became the limitation for the VTEP-only VxLAN scenario as well. In
other words, the patch that introduced internal VxLANs broke the
scenario for VTEP-only VxLANs.

In essence, Vladislav Odintsov asserts that the patch by Ihar
Hrachyshka introduced an implicit configuration option the state of
which was determined in software (the function is_vxlan_mode()). The
new patch by Vladislav Odintsov makes that option explicit. That makes
the behavior of a configured cluster completely depend on its value -
as opposed to determining the behavior in software from configuration.

On the one hand, that makes it simple - one option to control whether
the cluster supports internal VxLANs or it is VTEP-only VxLANs. On the
other hand I can't help but wonder - why can't that be determined in
software - like in the original patch? I would think that all
necessary data is present in a configuration and there's no need for
an extra explicit option. With the new patch from Vladislav Odintsov
it can be done just once in one place.

So the question is: what are the pros and cons of each variant? Why is
the variant with a new option chosen?

Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net-next] openvswitch: prepare for stolen verdict coming from conntrack and nat engine

2024-07-05 Thread patchwork-bot+netdevbpf
Hello:

This patch was applied to netdev/net-next.git (main)
by David S. Miller :

On Wed,  3 Jul 2024 12:46:34 +0200 you wrote:
> At this time, conntrack either returns NF_ACCEPT or NF_DROP.
> To improve debuging it would be nice to be able to replace NF_DROP
> verdict with NF_DROP_REASON() helper,
> 
> This helper releases the skb instantly (so drop_monitor can pinpoint
> precise location) and returns NF_STOLEN.
> 
> [...]

Here is the summary with links:
  - [net-next] openvswitch: prepare for stolen verdict coming from conntrack 
and nat engine
https://git.kernel.org/netdev/net-next/c/c7f79f2620b7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net-next v9 00/10] net: openvswitch: Add sample multicasting.

2024-07-05 Thread Adrián Moreno
On Thu, Jul 04, 2024 at 10:56:51AM GMT, Adrian Moreno wrote:
> ** Background **
> Currently, OVS supports several packet sampling mechanisms (sFlow,
> per-bridge IPFIX, per-flow IPFIX). These end up being translated into a
> userspace action that needs to be handled by ovs-vswitchd's handler
> threads only to be forwarded to some third party application that
> will somehow process the sample and provide observability on the
> datapath.
>
> A particularly interesting use-case is controller-driven
> per-flow IPFIX sampling where the OpenFlow controller can add metadata
> to samples (via two 32bit integers) and this metadata is then available
> to the sample-collecting system for correlation.
>
> ** Problem **
> The fact that sampled traffic share netlink sockets and handler thread
> time with upcalls, apart from being a performance bottleneck in the
> sample extraction itself, can severely compromise the datapath,
> yielding this solution unfit for highly loaded production systems.
>
> Users are left with little options other than guessing what sampling
> rate will be OK for their traffic pattern and system load and dealing
> with the lost accuracy.
>
> Looking at available infrastructure, an obvious candidated would be
> to use psample. However, it's current state does not help with the
> use-case at stake because sampled packets do not contain user-defined
> metadata.
>
> ** Proposal **
> This series is an attempt to fix this situation by extending the
> existing psample infrastructure to carry a variable length
> user-defined cookie.
>
> The main existing user of psample is tc's act_sample. It is also
> extended to forward the action's cookie to psample.
>
> Finally, a new OVS action (OVS_SAMPLE_ATTR_PSAMPLE) is created.
> It accepts a group and an optional cookie and uses psample to
> multicast the packet and the metadata.
>
> --
> v8 -> v9:
> - Rebased.
>
> v7 -> v8:
> - Rebased
> - Redirect flow insertion to /dev/null to avoid spat in test.
> - Removed inline keyword in stub execute_psample_action function.
>
> v6 -> v7:
> - Rebased
> - Fixed typo in comment.
>
> v5 -> v6:
> - Renamed emit_sample -> psample
> - Addressed unused variable and conditionally compilation of function.
>
> v4 -> v5:
> - Rebased.
> - Removed lefover enum value and wrapped some long lines in selftests.
>
> v3 -> v4:
> - Rebased.
> - Addressed Jakub's comment on private and unused nla attributes.
>
> v2 -> v3:
> - Addressed comments from Simon, Aaron and Ilya.
> - Dropped probability propagation in nested sample actions.
> - Dropped patch v2's 7/9 in favor of a userspace implementation and
> consume skb if emit_sample is the last action, same as we do with
> userspace.
> - Split ovs-dpctl.py features in independent patches.
>
> v1 -> v2:
> - Create a new action ("emit_sample") rather than reuse existing
>   "sample" one.
> - Add probability semantics to psample's sampling rate.
> - Store sampling probability in skb's cb area and use it in emit_sample.
> - Test combining "emit_sample" with "trunc"
> - Drop group_id filtering and tracepoint in psample.
>
> rfc_v2 -> v1:
> - Accommodate Ilya's comments.
> - Split OVS's attribute in two attributes and simplify internal
> handling of psample arguments.
> - Extend psample and tc with a user-defined cookie.
> - Add a tracepoint to psample to facilitate troubleshooting.
>
> rfc_v1 -> rfc_v2:
> - Use psample instead of a new OVS-only multicast group.
> - Extend psample and tc with a user-defined cookie.
>
> Adrian Moreno (10):
>   net: psample: add user cookie
>   net: sched: act_sample: add action cookie to sample
>   net: psample: skip packet copy if no listeners
>   net: psample: allow using rate as probability
>   net: openvswitch: add psample action
>   net: openvswitch: store sampling probability in cb.
>   selftests: openvswitch: add psample action
>   selftests: openvswitch: add userspace parsing
>   selftests: openvswitch: parse trunc action
>   selftests: openvswitch: add psample test
>
>  Documentation/netlink/specs/ovs_flow.yaml |  17 ++
>  include/net/psample.h |   5 +-
>  include/uapi/linux/openvswitch.h  |  31 +-
>  include/uapi/linux/psample.h  |  11 +-
>  net/openvswitch/Kconfig   |   1 +
>  net/openvswitch/actions.c |  66 -
>  net/openvswitch/datapath.h|   3 +
>  net/openvswitch/flow_netlink.c|  32 ++-
>  net/openvswitch/vport.c   |   1 +
>  net/psample/psample.c |  16 +-
>  net/sched/act_sample.c|  12 +
>  .../selftests/net/openvswitch/openvswitch.sh  | 115 +++-
>  .../selftests/net/openvswitch/ovs-dpctl.py| 272 +-
>  13 files changed, 566 insertions(+), 16 deletions(-)
>
> --
> 2.45.2
>

Hi,

Simon Horman has spotted that openvswitch.sh tests are failing in the
debug executor:

https://netdev.bots.linux.dev/contest.html?test=openvswitch-sh

The

Re: [ovs-dev] [PATCH ovn] nbctl: Added local link ipv6 to nbctl show.

2024-07-05 Thread Frode Nordahl
Hello, MJ,

Thank you for the patch!

On Thu, Jul 4, 2024 at 6:39 PM MJ Ponsonby  wrote:
>
> Reported-at: https://launchpad.net/bugs/2069804

The bug reference is usually colocated with the Signed-off-by at the
end of the commit message.

> This commit modifies a test to deal with the changed,
> response from the command
> This commit also modifies the function of ovn-nbctl show,
> to also return the ipv6 link local address as mentioned

Would it make sense to include a short sentence on why we want to do
this? There is some reasoning on the bug.

> Signed-off-by: MJ Ponsonby 
> ---
>  tests/ovn-nbctl.at|  1 +
>  utilities/ovn-nbctl.c | 10 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 19c83a4a5..54acaceb2 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -1670,6 +1670,7 @@ AT_CHECK([ovn-nbctl show lr0 | uuidfilt], [0], [dnl
>  router <0> (lr0)
>  port lrp0
>  mac: "00:00:00:01:02:03"
> +ipv6-lla: "fe80::0200:00ff:fe01:0203"

I wonder if we want to include the prefix length in this output, i.e.
"fe80::0200:00ff:fe01:0203/64", feels more consistent with the other
information provided for LRPs. WDYT?

>  networks: [["192.168.1.1/24"]]
>  ])
>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index 32ca4f750..51c3f5afd 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -780,6 +780,16 @@ print_lr(const struct nbrec_logical_router *lr, struct 
> ds *s)
>  if (lrp->mac) {
>  ds_put_cstr(s, "mac: ");
>  ds_put_format(s, "\"%s\"\n", lrp->mac);
> +
> +/* Have the mac address in an array. */

Not sure if this comment is necessary.

--
Frode Nordahl

> +struct eth_addr ea;
> +eth_addr_from_string(mac, ea);
> +struct in6_addr lla;
> +in6_generate_lla(ea, &lla);
> +
> +ds_put_cstr(s, "ipv6-lla: \"");
> +ipv6_format_addr(&lla, s);
> +ds_put_cstr(s, "\"\n");
>  }
>  if (lrp->n_networks) {
>  ds_put_cstr(s, "networks: [");
> --
> 2.43.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev