Re: [ovs-dev] [PATCH] ovs-tcpdump: Bugfix-of-ovs-tcpdump

[Simon Jones]

Ok, I will fix these.

Simon Jones


Ilya Maximets  于2023年7月4日周二 21:35写道：

> On 7/3/23 05:46, 冮晔维 wrote:
> > From: gangyewei 
>
> Hi.  Thanks for the update!  Please, add a version number while
> sending new versions though, e.g. [PATCH v2].  Otherwise, it's
> hard to track which version is the most recent one.
>
> > Fix bug of ovs-tcpdump, which will cause megaflow action wrong.
> > As use ovs-tcpdump will add mipxxx NIC, and this NIC has IPv6 address by
> default.
> > For vxlan topology, mipxxx will be treated as tunnel port, and will got
> error actions.
> > For detail discuss, refer email of ovs-discuss titled in "[BUG]
> [ovs-tcpdump] Got duplicate ...".
>
> If you want to cite the discussion, you need to provide a link
> to a public web archive.  It'll be hard to find this discussion
> for someone who didn't participate in it.  In your case the
> issue was reported in the mail thread, so you can use the tag
> instead:
>
>   Reported-at: 
>
> And there should be an empty line between the commit message
> and the tags.
>
> Also, please remove dashes from the subject line.  And make it
> a bit more specific to the problem you're trying to solve.
>
> There are some guidelines on patch formatting here:
>
> https://docs.openvswitch.org/en/latest/internals/contributing/submitting-patches/
>
> > Signed-off-by: gangyewei 
> > ---
> >  utilities/ovs-tcpdump.in | 4 
> >  1 file changed, 4 insertions(+)
> > diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> > index 420c11eb8..4cbd9a5d3 100755
> > --- a/utilities/ovs-tcpdump.in
> > +++ b/utilities/ovs-tcpdump.in
> > @@ -96,6 +96,10 @@ def _install_dst_if_linux(tap_name, mtu_value=None):
> >  *(['ip', 'link', 'set', 'dev', str(tap_name), 'up']))
> >  pipe.wait()
> > + pipe = _doexec(
> > + *(['ip', '-6', 'addr', 'flush', 'dev', str(tap_name)]))
> > + pipe.wait()
> > +
> >  def _remove_dst_if_linux(tap_name):
> >  _doexec(
>
> This patch is not correctly formatted.
> Looks like your mail client mangled the spaces and now the patch
> is not applicable:
>
>  Applying: ovs-tcpdump: Bugfix-of-ovs-tcpdump.
>  error: corrupt patch at line 20
>  error: could not build fake ancestor
>  Patch failed at 0001 ovs-tcpdump: Bugfix-of-ovs-tcpdump.
>
> If you can't configure your mail client to not do that, you may
> want to try a different client, e.g. "git send-email".
>
> Best regards, Ilya Maximets.
> ___
> 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 v3 2/2] userspace: Add Generic Segmentation Offloading.

[Ilya Maximets]

On 6/21/23 22:36, Mike Pattrick wrote:
> From: Flavio Leitner 
> 
> This provides a software implementation in the case
> the egress netdev doesn't support segmentation in hardware.
> 
> The challenge here is to guarantee packet ordering in the
> original batch that may be full of TSO packets. Each TSO
> packet can go up to ~64kB, so with segment size of 1440
> that means about 44 packets for each TSO. Each batch has
> 32 packets, so the total batch amounts to 1408 normal
> packets.
> 
> The segmentation estimates the total number of packets
> and then the total number of batches. Then allocate
> enough memory and finally do the work.
> 
> Finally each batch is sent in order to the netdev.
> 
> Signed-off-by: Flavio Leitner 
> Co-authored-by: Mike Pattrick 
> Signed-off-by: Mike Pattrick 
> ---
>  lib/automake.mk |   2 +
>  lib/dp-packet-gso.c | 172 
>  lib/dp-packet-gso.h |  24 +++
>  lib/dp-packet.h |  11 +++
>  lib/netdev-dpdk.c   |  46 
>  lib/netdev-linux.c  |  58 ---
>  lib/netdev.c| 120 ++-
>  lib/packets.c   |   4 +-
>  8 files changed, 314 insertions(+), 123 deletions(-)
>  create mode 100644 lib/dp-packet-gso.c
>  create mode 100644 lib/dp-packet-gso.h
> 
> diff --git a/lib/automake.mk b/lib/automake.mk
> index e64ee76ce..49a92958d 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -118,6 +118,8 @@ lib_libopenvswitch_la_SOURCES = \
>   lib/dpctl.h \
>   lib/dp-packet.h \
>   lib/dp-packet.c \
> + lib/dp-packet-gso.c \
> + lib/dp-packet-gso.h \
>   lib/dpdk.h \
>   lib/dpif-netdev-extract-study.c \
>   lib/dpif-netdev-lookup.h \
> diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c
> new file mode 100644
> index 0..bc72e2f90
> --- /dev/null
> +++ b/lib/dp-packet-gso.c
> @@ -0,0 +1,172 @@
> +/*
> + * Copyright (c) 2021 Red Hat, Inc.

Need to adjust the year. :)

> + *
> + * 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 "coverage.h"
> +#include "dp-packet.h"
> +#include "dp-packet-gso.h"
> +#include "netdev-provider.h"
> +#include "openvswitch/vlog.h"
> +
> +VLOG_DEFINE_THIS_MODULE(dp_packet_gso);
> +
> +COVERAGE_DEFINE(soft_seg_good);
> +
> +/* Retuns a new packet that is a segment of packet 'p'.
> + *
> + * The new packet is initialized with 'hdr_len' bytes from the
> + * start of packet 'p' and then appended with 'data_len' bytes
> + * from the 'data' buffer.
> + *
> + * Note: The packet headers are not updated. */
> +static struct dp_packet *
> +dp_packet_gso_seg_new(const struct dp_packet *p, size_t hdr_len,
> +  const char *data, size_t data_len)
> +{
> +struct dp_packet *seg = dp_packet_new_with_headroom(hdr_len + data_len,
> +
> dp_packet_headroom(p));
> +
> +/* Append the original packet headers and then the payload. */
> +dp_packet_put(seg, dp_packet_data(p), hdr_len);
> +dp_packet_put(seg, data, data_len);
> +
> +/* The new segment should have the same offsets. */
> +seg->l2_5_ofs = p->l2_5_ofs;
> +seg->l3_ofs = p->l3_ofs;
> +seg->l4_ofs = p->l4_ofs;
> +
> +/* The protocol headers remain the same, so preserve hash and mark. */
> +*dp_packet_rss_ptr(seg) = dp_packet_get_rss_hash(p);
> +*dp_packet_flow_mark_ptr(seg) = *dp_packet_flow_mark_ptr(p);
> +
> +/* The segment should inherit all the offloading flags from the
> + * original packet, except for the TCP segmentation, external
> + * buffer and indirect buffer flags. */
> +*dp_packet_ol_flags_ptr(seg) = *dp_packet_ol_flags_ptr(p)
> +& ~(DP_PACKET_OL_TX_TCP_SEG | DP_PACKET_OL_EXTERNAL
> +| DP_PACKET_OL_INDIRECT);

We should inherit DP_PACKET_OL_SUPPORTED_MASK & ~DP_PACKET_OL_TX_TCP_SEG.
So,

*dp_packet_ol_flags_ptr(seg) &= DP_PACKET_OL_SUPPORTED_MASK
& ~DP_PACKET_OL_TX_TCP_SEG;

Is that right?

> +
> +dp_packet_hwol_reset_tcp_seg(seg);

Also, this function just resets the DP_PACKET_OL_TX_TCP_SEG.
So, above should  be just:

*dp_packet_ol_flags_ptr(seg) &= DP_PACKET_OL_SUPPORTED_MASK;

The same as in the clone function.  Either way one of these operations
is redundant.

> +
> +return seg;
> +}
> +
> +/* Returns the calculated number of TCP segments in packet 'p'. */
> +int
> 

[ovs-dev] [PATCH v3] ovs-vsctl: Exit with error if postdb checks report errors.

[Flavio Leitner]

Today the exit code refers to the execution of the change
in the database. However, when not using parameter --no-wait
(default), the ovs-vsctl also checks if OVSDB transactions
are successfully recorded and reload by ovs-vswitchd. In this
case, an error message is printed if there is a problem during
the reload, like for example the one below:

 # ovs-vsctl add-port br0 gre0 -- \
set interface gre0 type=ip6gre options:key=100 \
options:remote_ip=2001::2
ovs-vsctl: Error detected while setting up 'gre0': could not \
add network device gre0 to ofproto (Address family not supported\
by protocol).  See ovs-vswitchd log for details.
ovs-vsctl: The default log directory is "/var/log/openvswitch".
 # echo $?
0

This patch changes to exit with specific error code 160
(ERROR_BAD_ARGUMENTS) on Windows and 65 (EX_DATAERR) on
Linux or BSD if errors were reported during the reload.

This change may break existing scripts because ovs-vsctl will
start to fail when before it was succeeding. However, if an
error is printed, then it is likely that the change was not
functional anyway.

Reported-at: https://bugzilla.redhat.com/1731553
Signed-off-by: Flavio Leitner 
---

v3: Fixed the Windows build issue reported by Ilya.
Return ERROR_BAD_ARGUMENTS on Windows.
v2:
Followed Aaron's suggestion to return EX_DATAERR.

 NEWS |  5 +
 tests/ovs-vsctl.at   | 30 --
 tests/ovs-vswitchd.at|  6 +-
 tests/tunnel.at  |  8 +++-
 utilities/ovs-vsctl.8.in |  4 
 utilities/ovs-vsctl.c| 35 +--
 6 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index 6a990c921..8c733e417 100644
--- a/NEWS
+++ b/NEWS
@@ -30,6 +30,11 @@ Post-v3.1.0
  * Added new options --[ovsdb-server|ovs-vswitchd]-umask=MODE to set umask
value when starting OVS daemons.  E.g., use --ovsdb-server-umask=0002
in order to create OVSDB sockets with access mode of 0770.
+   - ovs-vsctl:
+ * Exit with error code 160 (ERROR_BAD_ARGUMENTS) on Windows or
+   65 (EX_DATAERR) on other platforms if errors were reported while
+   checking if OVSDB transactions are successfully recorded and reload
+   by ovs-vswitchd.
- QoS:
  * Added new configuration option 'jitter' for a linux-netem QoS type.
- DPDK:
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index a368bff6e..a8274734f 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1522,7 +1522,11 @@ cat >experr experr <
+#define EXIT_POSTDB_ERROR ERROR_BAD_ARGUMENTS
+#else
+#include 
+#define EXIT_POSTDB_ERROR EX_DATAERR
+#endif
+
 struct vsctl_context;
 
 /* --db: The database server to contact. */
@@ -115,7 +124,7 @@ static void parse_options(int argc, char *argv[], struct 
shash *local_options);
 static void run_prerequisites(struct ctl_command[], size_t n_commands,
   struct ovsdb_idl *);
 static bool do_vsctl(const char *args, struct ctl_command *, size_t n,
- struct ovsdb_idl *);
+ struct ovsdb_idl *, bool *);
 
 /* post_db_reload_check frame work is to allow ovs-vsctl to do additional
  * checks after OVSDB transactions are successfully recorded and reload by
@@ -134,11 +143,13 @@ static bool do_vsctl(const char *args, struct ctl_command 
*, size_t n,
  * Current implementation only check for Post OVSDB reload failures on new
  * interface additions with 'add-br' and 'add-port' commands.
  *
+ * post_db_reload_check returns 'true' if a failure is reported.
+ *
  * post_db_reload_expect_iface()
  *
  * keep track of interfaces to be checked post OVSDB reload. */
 static void post_db_reload_check_init(void);
-static void post_db_reload_do_checks(const struct vsctl_context *);
+static bool post_db_reload_do_checks(const struct vsctl_context *);
 static void post_db_reload_expect_iface(const struct ovsrec_interface *);
 
 static struct uuid *neoteric_ifaces;
@@ -200,9 +211,15 @@ main(int argc, char *argv[])
 }
 
 if (seqno != 

[ovs-dev] [PATCH v14] netdev-dpdk: Add custom rx-steering configuration.

[Robin Jarry]

Some control protocols are used to maintain link status between
forwarding engines (e.g. LACP). When the system is not sized properly,
the PMD threads may not be able to process all incoming traffic from the
configured Rx queues. When a signaling packet of such protocols is
dropped, it can cause link flapping, worsening the situation.

Use the rte_flow API to redirect these protocols into a dedicated Rx
queue. The assumption is made that the ratio between control protocol
traffic and user data traffic is very low and thus this dedicated Rx
queue will never get full. Re-program the RSS redirection table to only
use the other Rx queues.

The additional Rx queue will be assigned a PMD core like any other Rx
queue. Polling that extra queue may introduce increased latency and
a slight performance penalty at the benefit of preventing link flapping.

This feature must be enabled per port on specific protocols via the
rx-steering option. This option takes "rss" followed by a "+" separated
list of protocol names. It is only supported on ethernet ports. This
feature is experimental.

If the user has already configured multiple Rx queues on the port, an
additional one will be allocated for control packets. If the hardware
cannot satisfy the number of requested Rx queues, the last Rx queue will
be assigned for control plane. If only one Rx queue is available, the
rx-steering feature will be disabled. If the hardware does not support
the rte_flow matchers/actions, the rx-steering feature will be
completely disabled on the port and regular rss will be performed
instead.

It cannot be enabled when other-config:hw-offload=true as it may
conflict with the offloaded flows. Similarly, if hw-offload is enabled,
custom rx-steering will be forcibly disabled on all ports and replaced
by regular rss.

Example use:

 ovs-vsctl add-bond br-phy bond0 phy0 phy1 -- \
   set interface phy0 type=dpdk options:dpdk-devargs=:ca:00.0 -- \
   set interface phy0 options:rx-steering=rss+lacp -- \
   set interface phy1 type=dpdk options:dpdk-devargs=:ca:00.1 -- \
   set interface phy1 options:rx-steering=rss+lacp

As a starting point, only one protocol is supported: LACP. Other
protocols can be added in the future. NIC compatibility should be
checked.

To validate that this works as intended, I used a traffic generator to
generate random traffic slightly above the machine capacity at line rate
on a two ports bond interface. OVS is configured to receive traffic on
two VLANs and pop/push them in a br-int bridge based on tags set on
patch ports.

   +--+
   | DUT  |
   |++|
   ||   br-int   || 
in_port=patch10,actions=mod_dl_src:$patch11,mod_dl_dst:$tgen1,output:patch11
   |||| 
in_port=patch11,actions=mod_dl_src:$patch10,mod_dl_dst:$tgen0,output:patch10
   || patch10patch11 ||
   |+---|---|+|
   ||   | |
   |+---|---|+|
   || patch00patch01 ||
   ||  tag:10tag:20  ||
   ||||
   ||   br-phy   || default flow, action=NORMAL
   ||||
   ||   bond0|| balance-slb, lacp=passive, lacp-time=fast
   ||phy0   phy1 ||
   |+--|-|---+|
   +---|-|+
   | |
   +---|-|+
   | port0  port1 | balance L3/L4, lacp=active, lacp-time=fast
   | lag  | mode trunk VLANs 10, 20
   |  |
   |switch|
   |  |
   |  vlan 10vlan 20  |  mode access
   |   port2  port3   |
   +-|--|-+
 |  |
   +-|--|-+
   |   tgen0  tgen1   |  Random traffic that is properly balanced
   |  |  across the bond ports in both directions.
   |  traffic generator   |
   +--+

Without rx-steering, the bond0 links are randomly switching to
"defaulted" when one of the LACP packets sent by the switch is dropped
because the RX queues are full and the PMD threads did not process them
fast enough. When that happens, all traffic must go through a single
link which causes above line rate traffic to be dropped.

 ~# ovs-appctl lacp/show-stats bond0
  bond0 statistics 
 member: phy0:
   TX PDUs: 347246
   RX PDUs: 14865
   RX Bad PDUs: 0
   RX Marker Request PDUs: 0
   Link Expired: 168
   Link Defaulted: 0
   Carrier Status Changed: 0
 member: phy1:
   TX PDUs: 347245
   RX PDUs: 14919
   RX Bad PDUs: 0
   RX Marker Request PDUs: 0
   Link Expired: 147
   Link Defaulted: 1
   Carrier Status Changed: 0

When rx-steering is enabled, no LACP packet is dropped and the bond
links remain enabled at all times, maximizing the throughput. Neither
the "Link Expired" nor the "Link Defaulted" counters are incremented
anymore.

This feature may be considered as "QoS". However, it does not work by
limiting the rate of traffic explicitly. It only guarantees that some
protocols have 

Re: [ovs-dev] [PATCH v13] netdev-dpdk: Add custom rx-steering configuration.

[Ilya Maximets]

On 7/1/23 18:21, Robin Jarry wrote:

Thanks for the update.  Looks fine in general, see some small
comments inline.

Best rgeards, Ilya Maximets.

> Some control protocols are used to maintain link status between
> forwarding engines (e.g. LACP). When the system is not sized properly,
> the PMD threads may not be able to process all incoming traffic from the
> configured Rx queues. When a signaling packet of such protocols is
> dropped, it can cause link flapping, worsening the situation.
> 
> Use the RTE flow API to redirect these protocols into a dedicated Rx

There is no such term as 'RTE flow'.  It's either 'DPDK's generic flow API'
that nobody is using or 'rte_flow'/'rte_flow API'.

> queue. The assumption is made that the ratio between control protocol
> traffic and user data traffic is very low and thus this dedicated Rx
> queue will never get full. Re-program the RSS redirection table to only
> use the other Rx queues.
> 
> The additional Rx queue will be assigned a PMD core like any other Rx
> queue. Polling that extra queue may introduce increased latency and
> a slight performance penalty at the benefit of preventing link flapping.
> 
> This feature must be enabled per port on specific protocols via the
> rx-steering option. This option takes "rss" followed by a "+" separated
> list of protocol names. It is only supported on ethernet ports. This
> feature is experimental.
> 
> If the user has already configured multiple Rx queues on the port, an
> additional one will be allocated for control packets. If the hardware
> cannot satisfy the number of requested Rx queues, the last Rx queue will
> be assigned for control plane. If only one Rx queue is available, the
> rx-steering feature will be disabled. If the hardware does not support
> the RTE flow matchers/actions, the rx-steering feature will be

s/RTE flow/rte_flow/

> completely disabled on the port.
> 
> It cannot be enabled when other_config:hw-offload=true as it may
> conflict with the offloaded RTE flows. Similarly, if hw-offload is

s/RTE //

> enabled, custom rx-steering will be forcibly disabled on all ports.
> 
> Example use:
> 
>  ovs-vsctl add-bond br-phy bond0 phy0 phy1 -- \
>set interface phy0 type=dpdk options:dpdk-devargs=:ca:00.0 -- \
>set interface phy0 options:rx-steering=rss+lacp -- \
>set interface phy1 type=dpdk options:dpdk-devargs=:ca:00.1 -- \
>set interface phy1 options:rx-steering=rss+lacp
> 
> As a starting point, only one protocol is supported: LACP. Other
> protocols can be added in the future. NIC compatibility should be
> checked.
> 
> To validate that this works as intended, I used a traffic generator to
> generate random traffic slightly above the machine capacity at line rate
> on a two ports bond interface. OVS is configured to receive traffic on
> two VLANs and pop/push them in a br-int bridge based on tags set on
> patch ports.
> 
>+--+
>| DUT  |
>|++|
>||   br-int   || 
> in_port=patch10,actions=mod_dl_src:$patch11,mod_dl_dst:$tgen1,output:patch11
>|||| 
> in_port=patch11,actions=mod_dl_src:$patch10,mod_dl_dst:$tgen0,output:patch10
>|| patch10patch11 ||
>|+---|---|+|
>||   | |
>|+---|---|+|
>|| patch00patch01 ||
>||  tag:10tag:20  ||
>||||
>||   br-phy   || default flow, action=NORMAL
>||||
>||   bond0|| balance-slb, lacp=passive, lacp-time=fast
>||phy0   phy1 ||
>|+--|-|---+|
>+---|-|+
>| |
>+---|-|+
>| port0  port1 | balance L3/L4, lacp=active, lacp-time=fast
>| lag  | mode trunk VLANs 10, 20
>|  |
>|switch|
>|  |
>|  vlan 10vlan 20  |  mode access
>|   port2  port3   |
>+-|--|-+
>  |  |
>+-|--|-+
>|   tgen0  tgen1   |  Random traffic that is properly balanced
>|  |  across the bond ports in both directions.
>|  traffic generator   |
>+--+
> 
> Without rx-steering, the bond0 links are randomly switching to
> "defaulted" when one of the LACP packets sent by the switch is dropped
> because the RX queues are full and the PMD threads did not process them
> fast enough. When that happens, all traffic must go through a single
> link which causes above line rate traffic to be dropped.
> 
>  ~# ovs-appctl lacp/show-stats bond0
>   bond0 statistics 
>  member: phy0:
>TX PDUs: 347246
>RX PDUs: 14865
>RX Bad PDUs: 0
>RX Marker Request PDUs: 0
>Link Expired: 168
>Link Defaulted: 0
>Carrier Status Changed: 0
>  member: phy1:
>TX PDUs: 347245
>RX PDUs: 14919
>RX Bad PDUs: 0
>RX 

Re: [ovs-dev] Scale testing OVN with ovn-heater for OpenStack use cases

[Roberto Bartzen Acosta via dev]

I'm interested in attending this meet, Frode. Please include me in the
invite list.

Thanks

Em ter., 4 de jul. de 2023 às 13:06, Numan Siddique 
escreveu:

> On Tue, Jul 4, 2023, 8:00 PM Frode Nordahl 
> wrote:
>
> > On Tue, Jul 4, 2023 at 4:16 PM Dumitru Ceara  wrote:
> > >
> > > On 6/30/23 23:07, Terry Wilson wrote:
> > > > On Fri, Jun 30, 2023 at 2:26 AM Frode Nordahl
> > > >  wrote:
> > > >>
> > > >> Hello all,
> > > >>
> > > >> On Tue, May 30, 2023 at 5:16 PM Felix Huettner
> > > >>  wrote:
> > > >>>
> > > >>> Hi Dumitru,
> > > >>>
> > > >>> On Fri, May 26, 2023 at 01:30:54PM +0200, Dumitru Ceara wrote:
> > >  On 5/24/23 09:37, Felix Huettner wrote:
> > > > Hi everyone,
> > > 
> > >  Hi Felix,
> > > 
> > > >
> > > > Ilya mentioned to me that you will want to bring openstack
> > examples to
> > > > ovn-heater.
> > > >
> > > 
> > >  Yes, we're discussing that.
> > > 
> > > > I wanted to ask how to best join this effort. It would be great
> > for us
> > > 
> > >  Everyone is welcome to contribute! :)
> > > 
> > > >>>
> > > >>> Great, we will try that out.
> > > >>
> > > >> Thank you for your interest in this effort, as promised I'm in the
> > > >> process of organizing a community meeting to get this going.
> > > >>
> > > >> Below are some proposals for dates and times, please respond with a
> > > >> prioritized list of what works best for you and we'll try to land
> on a
> > > >> single date/time for a first meeting:
> > > >> Wednesday July 5th 13:00 UTC
> > > >> Tuesday July 11th 13:30 UTC
> > > >> Wednesday July 12th 8:30 UTC
> > > >
> > > > I'd be interested in attending. Of these, Tuesday July 11th @ 13:30
> > > > works best for me. 08:30 would be 03:30 for me, which is a little
> > > > early. I could do Wed July 5th as well. It's the day after the the
> > > > July 4 holiday here, and a lot of people are making it a long
> weekend.
> > > > So there will be the inevitable "catch up" associated with that, but
> > > > it is doable.
> > > >
> > > > Terry (otherwiseguy)
> > > >
> > >
> > > Given that July 5th is tomorrow I'm assuming there won't be a community
> > > meeting as it seems quite short notice.
> > >
> > > Frode, is Tuesday July 11th 13:30 UTC still an option for you?  If I'm
> > > not wrong this was the slot that worked for everyone that replied.
> >
> > Yes, Tuesday July 11th 13:30 UTC is indeed sailing up as the consensus,
> > so let's conclude that's the date/time of our first meeting.
> >
> > Thanks for swift replies everyone! I'll send an meeting invite with
> > proposed
> > agenda to those who responded to this thread soon, and I'll also pop a
> > video link into this thread as we get closer for the benefit of anyone
> else
> > interested.
> >
>
> Please include me in the invite list.
> (nusid...@redhat.com)
>
> Numan
>
>
> > --
> > Frode Nordahl
> >
> > >
> > > Thanks,
> > > Dumitru
> > >
> > >
> > ___
> > 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
>

-- 




_‘Esta mensagem é direcionada apenas para os endereços constantes no 
cabeçalho inicial. Se você não está listado nos endereços constantes no 
cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa 
mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão 
imediatamente anuladas e proibidas’._


* **‘Apesar do Magazine Luiza tomar 
todas as precauções razoáveis para assegurar que nenhum vírus esteja 
presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por 
quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.*



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

Re: [ovs-dev] Scale testing OVN with ovn-heater for OpenStack use cases

[Numan Siddique]

On Tue, Jul 4, 2023, 8:00 PM Frode Nordahl 
wrote:

> On Tue, Jul 4, 2023 at 4:16 PM Dumitru Ceara  wrote:
> >
> > On 6/30/23 23:07, Terry Wilson wrote:
> > > On Fri, Jun 30, 2023 at 2:26 AM Frode Nordahl
> > >  wrote:
> > >>
> > >> Hello all,
> > >>
> > >> On Tue, May 30, 2023 at 5:16 PM Felix Huettner
> > >>  wrote:
> > >>>
> > >>> Hi Dumitru,
> > >>>
> > >>> On Fri, May 26, 2023 at 01:30:54PM +0200, Dumitru Ceara wrote:
> >  On 5/24/23 09:37, Felix Huettner wrote:
> > > Hi everyone,
> > 
> >  Hi Felix,
> > 
> > >
> > > Ilya mentioned to me that you will want to bring openstack
> examples to
> > > ovn-heater.
> > >
> > 
> >  Yes, we're discussing that.
> > 
> > > I wanted to ask how to best join this effort. It would be great
> for us
> > 
> >  Everyone is welcome to contribute! :)
> > 
> > >>>
> > >>> Great, we will try that out.
> > >>
> > >> Thank you for your interest in this effort, as promised I'm in the
> > >> process of organizing a community meeting to get this going.
> > >>
> > >> Below are some proposals for dates and times, please respond with a
> > >> prioritized list of what works best for you and we'll try to land on a
> > >> single date/time for a first meeting:
> > >> Wednesday July 5th 13:00 UTC
> > >> Tuesday July 11th 13:30 UTC
> > >> Wednesday July 12th 8:30 UTC
> > >
> > > I'd be interested in attending. Of these, Tuesday July 11th @ 13:30
> > > works best for me. 08:30 would be 03:30 for me, which is a little
> > > early. I could do Wed July 5th as well. It's the day after the the
> > > July 4 holiday here, and a lot of people are making it a long weekend.
> > > So there will be the inevitable "catch up" associated with that, but
> > > it is doable.
> > >
> > > Terry (otherwiseguy)
> > >
> >
> > Given that July 5th is tomorrow I'm assuming there won't be a community
> > meeting as it seems quite short notice.
> >
> > Frode, is Tuesday July 11th 13:30 UTC still an option for you?  If I'm
> > not wrong this was the slot that worked for everyone that replied.
>
> Yes, Tuesday July 11th 13:30 UTC is indeed sailing up as the consensus,
> so let's conclude that's the date/time of our first meeting.
>
> Thanks for swift replies everyone! I'll send an meeting invite with
> proposed
> agenda to those who responded to this thread soon, and I'll also pop a
> video link into this thread as we get closer for the benefit of anyone else
> interested.
>

Please include me in the invite list.
(nusid...@redhat.com)

Numan


> --
> Frode Nordahl
>
> >
> > Thanks,
> > Dumitru
> >
> >
> ___
> 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] Scale testing OVN with ovn-heater for OpenStack use cases

[Frode Nordahl]

On Tue, Jul 4, 2023 at 4:16 PM Dumitru Ceara  wrote:
>
> On 6/30/23 23:07, Terry Wilson wrote:
> > On Fri, Jun 30, 2023 at 2:26 AM Frode Nordahl
> >  wrote:
> >>
> >> Hello all,
> >>
> >> On Tue, May 30, 2023 at 5:16 PM Felix Huettner
> >>  wrote:
> >>>
> >>> Hi Dumitru,
> >>>
> >>> On Fri, May 26, 2023 at 01:30:54PM +0200, Dumitru Ceara wrote:
>  On 5/24/23 09:37, Felix Huettner wrote:
> > Hi everyone,
> 
>  Hi Felix,
> 
> >
> > Ilya mentioned to me that you will want to bring openstack examples to
> > ovn-heater.
> >
> 
>  Yes, we're discussing that.
> 
> > I wanted to ask how to best join this effort. It would be great for us
> 
>  Everyone is welcome to contribute! :)
> 
> >>>
> >>> Great, we will try that out.
> >>
> >> Thank you for your interest in this effort, as promised I'm in the
> >> process of organizing a community meeting to get this going.
> >>
> >> Below are some proposals for dates and times, please respond with a
> >> prioritized list of what works best for you and we'll try to land on a
> >> single date/time for a first meeting:
> >> Wednesday July 5th 13:00 UTC
> >> Tuesday July 11th 13:30 UTC
> >> Wednesday July 12th 8:30 UTC
> >
> > I'd be interested in attending. Of these, Tuesday July 11th @ 13:30
> > works best for me. 08:30 would be 03:30 for me, which is a little
> > early. I could do Wed July 5th as well. It's the day after the the
> > July 4 holiday here, and a lot of people are making it a long weekend.
> > So there will be the inevitable "catch up" associated with that, but
> > it is doable.
> >
> > Terry (otherwiseguy)
> >
>
> Given that July 5th is tomorrow I'm assuming there won't be a community
> meeting as it seems quite short notice.
>
> Frode, is Tuesday July 11th 13:30 UTC still an option for you?  If I'm
> not wrong this was the slot that worked for everyone that replied.

Yes, Tuesday July 11th 13:30 UTC is indeed sailing up as the consensus,
so let's conclude that's the date/time of our first meeting.

Thanks for swift replies everyone! I'll send an meeting invite with proposed
agenda to those who responded to this thread soon, and I'll also pop a
video link into this thread as we get closer for the benefit of anyone else
interested.

-- 
Frode Nordahl

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

Re: [ovs-dev] Scale testing OVN with ovn-heater for OpenStack use cases

[Robin Jarry]

Dumitru Ceara, Jul 04, 2023 at 16:16:
> Given that July 5th is tomorrow I'm assuming there won't be a community
> meeting as it seems quite short notice.
>
> Frode, is Tuesday July 11th 13:30 UTC still an option for you?  If I'm
> not wrong this was the slot that worked for everyone that replied.

July 11th 13:30 UTC works for me if that matters :)

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

Re: [ovs-dev] Scale testing OVN with ovn-heater for OpenStack use cases

[Dumitru Ceara]

On 6/30/23 23:07, Terry Wilson wrote:
> On Fri, Jun 30, 2023 at 2:26 AM Frode Nordahl
>  wrote:
>>
>> Hello all,
>>
>> On Tue, May 30, 2023 at 5:16 PM Felix Huettner
>>  wrote:
>>>
>>> Hi Dumitru,
>>>
>>> On Fri, May 26, 2023 at 01:30:54PM +0200, Dumitru Ceara wrote:
 On 5/24/23 09:37, Felix Huettner wrote:
> Hi everyone,

 Hi Felix,

>
> Ilya mentioned to me that you will want to bring openstack examples to
> ovn-heater.
>

 Yes, we're discussing that.

> I wanted to ask how to best join this effort. It would be great for us

 Everyone is welcome to contribute! :)

>>>
>>> Great, we will try that out.
>>
>> Thank you for your interest in this effort, as promised I'm in the
>> process of organizing a community meeting to get this going.
>>
>> Below are some proposals for dates and times, please respond with a
>> prioritized list of what works best for you and we'll try to land on a
>> single date/time for a first meeting:
>> Wednesday July 5th 13:00 UTC
>> Tuesday July 11th 13:30 UTC
>> Wednesday July 12th 8:30 UTC
> 
> I'd be interested in attending. Of these, Tuesday July 11th @ 13:30
> works best for me. 08:30 would be 03:30 for me, which is a little
> early. I could do Wed July 5th as well. It's the day after the the
> July 4 holiday here, and a lot of people are making it a long weekend.
> So there will be the inevitable "catch up" associated with that, but
> it is doable.
> 
> Terry (otherwiseguy)
> 

Given that July 5th is tomorrow I'm assuming there won't be a community
meeting as it seems quite short notice.

Frode, is Tuesday July 11th 13:30 UTC still an option for you?  If I'm
not wrong this was the slot that worked for everyone that replied.

Thanks,
Dumitru


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

Re: [ovs-dev] [PATCH v2] conntrack: Fix icmp_id conflicts in snat

[Ilya Maximets]

On 7/2/23 17:19, wushao...@chinatelecom.cn wrote:
> From: Shaohua Wu 
> 
> The icmp_id maybe conflicts in snat
> Description:
> If multiple devices send icmp packets with the same icmp_id,
> the sip of the packets changes to the same source ip address after the snat 
> operation,
> and the packets are sent to the same destination ip address.
> Only one ct entry is created.The data packets sent by other devices cannot be 
> sent to the peer device

Hi.  Thanks for the new version!
I didn't review the code, but see some comments inline.

> 
> ovs-appctl dpctl/dump-conntrack
> icmp,orig=(src=10.0.0.4,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=500,type=0,code=0)
> 
> After fixing the problem:
> ovs-appctl dpctl/dump-conntrack
> icmp,orig=(src=10.0.0.3,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=501,type=0,code=0)
> icmp,orig=(src=10.0.0.1,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=500,type=0,code=0)
> icmp,orig=(src=10.0.0.7,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=505,type=0,code=0)
> icmp,orig=(src=10.0.0.4,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=502,type=0,code=0)
> icmp,orig=(src=10.0.0.5,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=503,type=0,code=0)
> icmp,orig=(src=10.0.0.6,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=504,type=0,code=0)

There should be an empty line here.

> Signed-off-by: Shaohua Wu 
> ---

Please, add here the version log, i.e. what changed between
versions of the patch.

>  lib/conntrack.c | 39 +++
>  lib/packets.c   | 22 ++
>  lib/packets.h   |  2 ++
>  tests/system-traffic.at | 40 
>  4 files changed, 99 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 4375c03e2..07c6da6be 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -698,6 +698,25 @@ get_ip_proto(const struct dp_packet *pkt)
>  return ip_proto;
>  }
>  
> +static bool
> +packet_is_icmpv4_info_message(const struct dp_packet *pkt)
> +{
> +uint8_t ip_proto,icmp_type;
> +
> +ip_proto = get_ip_proto(pkt);
> +if (ip_proto == IPPROTO_ICMP) {
> +icmp_type = packet_get_icmp_type(pkt);
> +if (icmp_type == ICMP4_ECHO_REQUEST ||
> +icmp_type == ICMP4_ECHO_REPLY ||
> +icmp_type == ICMP4_TIMESTAMP ||
> +icmp_type == ICMP4_TIMESTAMPREPLY ||
> +icmp_type == ICMP4_INFOREQUEST ||
> +icmp_type == ICMP4_INFOREPLY)
> +return true;
> +}
> +return false;
> +}
> +
>  static bool
>  is_ftp_ctl(const enum ct_alg_ctl_type ct_alg_ctl)
>  {
> @@ -771,6 +790,9 @@ pat_packet(struct dp_packet *pkt, const struct conn_key 
> *key)
>  packet_set_tcp_port(pkt, key->dst.port, key->src.port);
>  } else if (key->nw_proto == IPPROTO_UDP) {
>  packet_set_udp_port(pkt, key->dst.port, key->src.port);
> +} else if (packet_is_icmpv4_info_message(pkt) &&
> +   key->nw_proto == IPPROTO_ICMP) {
> +  packet_set_icmp_id(pkt, key->src.icmp_id);
>  }
>  }
>  
> @@ -2306,8 +2328,8 @@ store_addr_to_key(union ct_addr *addr, struct conn_key 
> *key,
>  
>  static bool
>  nat_get_unique_l4(struct conntrack *ct, struct conn *nat_conn,
> -  ovs_be16 *port, uint16_t curr, uint16_t min,
> -  uint16_t max)
> +  ovs_be16 *port, ovs_be16 *peer_port,
> +  uint16_t curr, uint16_t min, uint16_t max)
>  {
>  static const unsigned int max_attempts = 128;
>  uint16_t range = max - min + 1;
> @@ -2328,6 +2350,9 @@ another_round:
>  }
>  
>  *port = htons(curr);
> +if (peer_port) {
> +*peer_port = htons(curr);
> +}
>  if (!conn_lookup(ct, _conn->rev_key,
>   time_msec(), NULL, NULL)) {
>  return true;
> @@ -2341,6 +2366,9 @@ another_round:
>  }
>  
>  *port = htons(orig);
> +if (peer_port) {
> +*peer_port = htons(orig);
> +}
>  
>  return false;
>  }
> @@ -2374,7 +2402,8 @@ nat_get_unique_tuple(struct conntrack *ct, const struct 
> conn *conn,
>  uint32_t hash = nat_range_hash(conn, ct->hash_basis, nat_info);
>  union ct_addr min_addr = {0}, max_addr = {0}, addr = {0};
>  bool pat_proto = conn->key.nw_proto == IPPROTO_TCP ||
> - conn->key.nw_proto == IPPROTO_UDP;
> + conn->key.nw_proto == IPPROTO_UDP ||
> + conn->key.nw_proto == IPPROTO_ICMP;
>  uint16_t min_dport, max_dport, curr_dport;
>  uint16_t min_sport, max_sport, curr_sport;
>  
> @@ -2409,11 +2438,13 @@ nat_get_unique_tuple(struct conntrack *ct, const 
> struct conn *conn,
>  bool found = false;
>  if (nat_info->nat_action & 

Re: [ovs-dev] [PATCH] ovs-tcpdump: Bugfix-of-ovs-tcpdump

[Ilya Maximets]

On 7/3/23 05:46, 冮晔维 wrote:
> From: gangyewei 

Hi.  Thanks for the update!  Please, add a version number while
sending new versions though, e.g. [PATCH v2].  Otherwise, it's
hard to track which version is the most recent one.

> Fix bug of ovs-tcpdump, which will cause megaflow action wrong.
> As use ovs-tcpdump will add mipxxx NIC, and this NIC has IPv6 address by 
> default.
> For vxlan topology, mipxxx will be treated as tunnel port, and will got error 
> actions.
> For detail discuss, refer email of ovs-discuss titled in "[BUG] [ovs-tcpdump] 
> Got duplicate ...".

If you want to cite the discussion, you need to provide a link
to a public web archive.  It'll be hard to find this discussion
for someone who didn't participate in it.  In your case the
issue was reported in the mail thread, so you can use the tag
instead:

  Reported-at: 

And there should be an empty line between the commit message
and the tags.

Also, please remove dashes from the subject line.  And make it
a bit more specific to the problem you're trying to solve.

There are some guidelines on patch formatting here:
  
https://docs.openvswitch.org/en/latest/internals/contributing/submitting-patches/

> Signed-off-by: gangyewei 
> ---
>  utilities/ovs-tcpdump.in | 4 
>  1 file changed, 4 insertions(+)
> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> index 420c11eb8..4cbd9a5d3 100755
> --- a/utilities/ovs-tcpdump.in
> +++ b/utilities/ovs-tcpdump.in
> @@ -96,6 +96,10 @@ def _install_dst_if_linux(tap_name, mtu_value=None):
>  *(['ip', 'link', 'set', 'dev', str(tap_name), 'up']))
>  pipe.wait()
> + pipe = _doexec(
> + *(['ip', '-6', 'addr', 'flush', 'dev', str(tap_name)]))
> + pipe.wait()
> +
>  def _remove_dst_if_linux(tap_name):
>  _doexec(

This patch is not correctly formatted.
Looks like your mail client mangled the spaces and now the patch
is not applicable:

 Applying: ovs-tcpdump: Bugfix-of-ovs-tcpdump.
 error: corrupt patch at line 20
 error: could not build fake ancestor
 Patch failed at 0001 ovs-tcpdump: Bugfix-of-ovs-tcpdump.

If you can't configure your mail client to not do that, you may
want to try a different client, e.g. "git send-email".

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v6 2/2] netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

[Simon Horman]

On Mon, Jul 03, 2023 at 01:28:06PM +0200, Ilya Maximets wrote:
> On 7/3/23 10:45, Simon Horman wrote:
> > On Thu, Jun 29, 2023 at 04:55:25PM +0200, Simon Horman wrote:
> >> On Thu, Jun 29, 2023 at 04:15:09PM +0200, Ilya Maximets wrote:
> >>> On 6/6/23 13:35, Ivan Malov wrote:
>  Action PORT_ID has been deprecated. Use REPRESENTED_PORT instead.
> >>>
> >>> AFAICT, not all drivers moved to a REPRESENTED_PORT action.
> >>> I don't see support in NFP driver, for example.
> >>>
> >>> Are we OK with dropping hardware offload support for NFP ?
> >>
> >> With my Corigine hat on:
> >>
> >> I would think the answer is no.
> >>
> >>> Or are there plans to support this new action in that driver?
> >>>
> >>> Simon, maybe you have some visibility on that?
> >>
> >> Let me check with the team.
> > 
> > Hi Ilya, Ivan, all,
> > 
> > I checked with the team at Corigine.
> > 
> > They don't see any problem with updating the NFP PMD to use
> > REPRESENTED_PORT instead of PORT_ID. And I think we can assume that work
> > will happen in a timely manner. But there is a question about how to make
> > the transition.
> > 
> > My understanding is that from a Corigine perspective, it would be best if
> > OvS could add support for REPRESENTED_PORT and then later remove support
> > for PORT_ID.
> 
> We could, in theory, probe the driver by trying to create dummy rte_flow
> rules.  But that sounds like a lot of unnecessary work.
> 
> If Corigine team can commit to update the driver before 23.11 release,
> we can switch OVS to REPRESENTED_PORT while moving to 23.11.
> 
> All the current DPDK drivers that support REPRESENTED_PORT do support
> PORT_ID as well, AFAICT, except for ICE driver.  But nobody complained
> about non-working offload with ICE, so there is no point trading it
> for NFP right now.
> 
> If we make the change while moving to 23.11 we'll hopefully not need to
> drop support for any driver.  And we'll gain support for ICE then.
> 
> Thoughts?

Thanks Ilya,

Sounds good.
I believe Corigine can work with that plan.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] vswitchd: Wait for a bridge exit before replying to exit unixctl.

[Ilya Maximets]

Before the cleanup option, the bridge_exit() call was fairly fast,
because it didn't include any particularly long operations.  However,
with the cleanup flag, this function destroys a lot of datapath
resources freeing a lot of memory, waiting on RCU and talking to
the kernel.  That may take a noticeable amount of time, especially
on a busy system or under profilers/sanitizers.  However, the unixctl
'exit' command replies instantly without waiting for any work to
actually be done.  This may cause system test failures or other
issues where scripts expect ovs-vswitchd to exit or destroy all the
datapath resources shortly after appctl call.

Fix that by waiting for the bridge_exit() before replying to the user.
At least, all the datapath resources will actually be destroyed by
the time ovs-appctl exits.

Also moving a structure from stack to global.  Seems cleaner this way.

Fixes: fe13ccdca6a2 ("vswitchd: Add --cleanup option to the 'appctl exit' 
command")
Signed-off-by: Ilya Maximets 
---
 vswitchd/ovs-vswitchd.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
index a244d2f70..55b437871 100644
--- a/vswitchd/ovs-vswitchd.c
+++ b/vswitchd/ovs-vswitchd.c
@@ -68,19 +68,18 @@ static unixctl_cb_func ovs_vswitchd_exit;
 static char *parse_options(int argc, char *argv[], char **unixctl_path);
 OVS_NO_RETURN static void usage(void);
 
-struct ovs_vswitchd_exit_args {
-bool *exiting;
-bool *cleanup;
-};
+static struct ovs_vswitchd_exit_args {
+struct unixctl_conn *conn;
+bool exiting;
+bool cleanup;
+} exit_args;
 
 int
 main(int argc, char *argv[])
 {
-char *unixctl_path = NULL;
 struct unixctl_server *unixctl;
+char *unixctl_path = NULL;
 char *remote;
-bool exiting, cleanup;
-struct ovs_vswitchd_exit_args exit_args = {, };
 int retval;
 
 set_program_name(argv[0]);
@@ -111,14 +110,12 @@ main(int argc, char *argv[])
 exit(EXIT_FAILURE);
 }
 unixctl_command_register("exit", "[--cleanup]", 0, 1,
- ovs_vswitchd_exit, _args);
+ ovs_vswitchd_exit, NULL);
 
 bridge_init(remote);
 free(remote);
 
-exiting = false;
-cleanup = false;
-while (!exiting) {
+while (!exit_args.exiting) {
 OVS_USDT_PROBE(main, run_start);
 memory_run();
 if (memory_should_report()) {
@@ -137,16 +134,20 @@ main(int argc, char *argv[])
 bridge_wait();
 unixctl_server_wait(unixctl);
 netdev_wait();
-if (exiting) {
+if (exit_args.exiting) {
 poll_immediate_wake();
 }
 OVS_USDT_PROBE(main, poll_block);
 poll_block();
 if (should_service_stop()) {
-exiting = true;
+exit_args.exiting = true;
 }
 }
-bridge_exit(cleanup);
+bridge_exit(exit_args.cleanup);
+
+if (exit_args.conn) {
+unixctl_command_reply(exit_args.conn, NULL);
+}
 unixctl_server_destroy(unixctl);
 service_stop();
 vlog_disable_async();
@@ -304,10 +305,9 @@ usage(void)
 
 static void
 ovs_vswitchd_exit(struct unixctl_conn *conn, int argc,
-  const char *argv[], void *exit_args_)
+  const char *argv[], void *args OVS_UNUSED)
 {
-struct ovs_vswitchd_exit_args *exit_args = exit_args_;
-*exit_args->exiting = true;
-*exit_args->cleanup = argc == 2 && !strcmp(argv[1], "--cleanup");
-unixctl_command_reply(conn, NULL);
+exit_args.conn = conn;
+exit_args.exiting = true;
+exit_args.cleanup = argc == 2 && !strcmp(argv[1], "--cleanup");
 }
-- 
2.40.1

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

Re: [ovs-dev] [PATCH ovn] Expose distributed gateway port information in NB DB

[Dumitru Ceara]

On 7/1/23 02:19, Han Zhou wrote:
> On Fri, Jun 30, 2023 at 6:35 AM Lucas Martins  wrote:
>>
>>  Hi all,
>>
>> On Thu, Apr 27, 2023 at 7:08 PM Han Zhou  wrote:
>>>
>>>
>>>
>>> On Thu, Apr 27, 2023 at 10:15 AM Ihar Hrachyshka 
> wrote:

 On Wed, Apr 19, 2023 at 5:17 AM Lucas Martins 
> wrote:
>
> Hi Han, all
>
> On Mon, Apr 17, 2023 at 8:02 PM Han Zhou  wrote:
>>
>>
>>
>> On Mon, Apr 17, 2023 at 7:18 AM Lucas Martins 
> wrote:
>>>
>>> Thanks all for the discussion and all the ideas here.
>>>
>>> After reading the emails, I think it boils down to two proposed
> approaches:
>>>
>>> 1) CMS to continue to connect to the Southbound database if
> they need
>>> information about the physical location of the resources. That
> would
>>> avoid the inefficiency of having to copy data back-and-forth
> from the
>>> Northbound and Southbound database.
>>>
>>> I guess the downside of this approach is that CMS will have to
>>> maintain a connection with both databases (which is already the
> case
>>> today).
>>>
>>> If we go with this approach, it would be good to have consensus
> from
>>> the core OVN team where some tables in the Southbound must be
> kept
>>> stable with backward compatibility in case of changes. Tables
> such
>>> Chassis, Chassis_Private and Port_Binding at least will need
> that. I
>>> guess that makes part of the Southbound database to not be
> considered
>>> OVN internal only.
>>
>> It doesn't look very clean to expose the SB DB to CMS, but in
> practice I think it is not a real problem for doing so for keeping tables
> backward compatible, because even without considering CMS, OVN itself needs
> to keep the compatibility for upgrading.
>> I am still open to the idea of keeping things in NB DB only, but
> at least one issue not addressed so far in this discussion (even with the
> status DB) is how to manage the orphan chassis if SB is not exposed to CMS.
> It seems still more practical to me to let CMS connect to SB directly
> instead of introducing a copy of Chassis table in NB and ovn-northd doing
> the back-and-forth data sync. So I am not sure if it should be a goal to
> remove all the SB access from CMS.
>>
>
> Yeah the Chassis situation in OVN is a strange one, indeed. Given
> that
> ovn-controller is the one creating it, not the CMS, shouldn't it be
> up
> to OVN to actually manage that table ? Perhaps we need to introduce
> some health check where ovn-controller could ping his own entry in
> the
> Chassis table from time to time to indicate that it's alive. And,
> ovn-northd could clean up the orphan entries if they are not updated
> in X amount of time.
>>>
>>> Thanks for the idea. In fact OVN has the "control-plane ping" mechanism
> using the nb_cfg which has similar idea, but it is mostly used for testing
> purposes and never recommended to be used in large scale production,
> because this would add a lot of write operations from all ovn-controllers
> to SB DB, which may increase the load significantly (depending on the
> number of nodes and frequency of this ping) to the SB which is already a
> bottle-neck.
>>> There can also be false-positives when ovn-controller is busy and slow
> (due to scenarios of recomputing within a large scale environment), which
> would make the things worse.
>>> On the other hand, an interval too long would also result in a cleanup
> too late, and then a new chassis (sometimes because of reimaging of the
> same node) would conflict with the old chassis.
>>> In fact, the CMS is the one that has the best knowledge when a chassis
> should be cleaned.
>>>

 (Random thought) NB could have an API (ChassisDeprovisionRequest?)
 that would be used by CMS to request cleanup for a chassis by name.
 Northd could then update the object with the status of the request, or
 delete it once it's processed.

>>>
>>> This may work, but I'd avoid any imperative approach if possible. NB
> (and also SB) should just maintain the desired states and let the
> controllers/daemons to converge.
>>> A "request" may be failed/timeout/invalid/out-of-date, which needs to
> be transactional and will be relatively complex to handle.
>>>
>>> If the direct connection to SB is really the only concern, I think the
> most viable approach is still to add a chassis table in NB just for CMS to
> specify the valid chassis that should exist, and ovn-northd (probably with
> a new thread) will take care of the cleanup. This is simple and scalable.
> The only question is whether it's worth making this change.
>>>
>
> Or piggybacking on the idea of the Status DB, maybe those health
> checks could be done in that DB instead ?
>
>> On the other hand, giving another thought, it doesn't sound too
> much extra cost for propagating the "hosting-chassis" information for
> chassis-redirect ports back to NB, 

Re: [ovs-dev] [PATCH ovn] tests: fixed userspace-system tests not properly cleaned up

[Ales Musil]

On Tue, Jul 4, 2023 at 12:08 PM Xavier Simonart  wrote:

> Usually, when cleaning up a system test we dexecute (for the ovn-vswitchd
> process):
> - OVS_TRAFFIC_VSWITCHD_STOP (i.e. ovs-appctl -t $1 exit --cleanup, wait up
> to 30 seconds
>   and report failure if not properly cleaned up).
> - kill_ovs_vswitchd (i.e. if ovs-vsitwhd still running,
>   ovs-appctl -t ovs-vswitchd exit --cleanup, wait 1 to 10 seconds and kill
> ovs-vswitvchd
>if not properly cleaned up).
>
> If we do not execute OVS_TRAFFIC_VSWITCHD_STOP, wait time is much smaller
> which might result in
> - clean up not done properly, ovs-vswitchd killed, ovs-netdev not deleted,
> and no error reported
> - next tests will fail
>
> Using this patch, we properly execute OVS_TRAFFIC_VSWITCHD_STOP, so the
> delay waiting for
> ovs-vswitchd to exit is longer, and, if by any chance this delay is still
> too short,
> the proper test will fail (i.e. the first test failing will be the one
> unable to clean).
>
> Signed-off-by: Xavier Simonart 
> ---
>  tests/system-ovn.at | 90 +
>  1 file changed, 90 insertions(+)
>
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 44a8072d6..8db08da70 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -5819,6 +5819,21 @@
> tcp,orig=(src=42.42.42.3,dst=42.42.42.2,sport=,dport=4242),reply=(s
>
>  
> tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
>  ])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> @@ -5915,6 +5930,21 @@
> tcp,orig=(src=4242::3,dst=4242::2,sport=,dport=4242),reply=(src=424
>
>  
> tcp,orig=(src=4242::3,dst=::1,sport=,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
>  ])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> @@ -7136,6 +7166,21 @@ as
>  OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
>  /.*terminating with signal 15.*/d"])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>

The cleanup was already in this test (DNAT LR hairpin IPv4) the second one
makes it fail constantly.
I have also noticed one thing. Some tests perform only "kill $(pidof
ovn-controller)" instead of
"OVS_APP_EXIT_AND_WAIT([ovn-controller])". Would you mind replacing those
in v2 as well?


>
> @@ -7238,6 +7283,21 @@ NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2
> 172.18.2.12 | FORMAT_PING], \
>  3 packets transmitted, 3 received, 0% packet loss, time 0ms
>  ])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> @@ -9168,6 +9228,21 @@ OVS_WAIT_UNTIL([
>  test "${requests}" -ge "6"
>  ])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> @@ -9307,6 +9382,21 @@ OVS_WAIT_UNTIL([
>  test "${requests}" -ge "6"
>  ])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> --
> 2.31.1
>
>
Thanks,
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.comIM: amusil

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

[ovs-dev] [PATCH ovn] tests: fixed "ovn-controller port security OF flows"

[Xavier Simonart]

The order of Port Binding key generation is not guaranteed, and
it might happen that sw0p1 and sw0p2 get different value than
the hardcoded 1 and 2 (e.g. 2 and 1).
Get the value from DB instead.

In addition, move "check_port_sec_offlows" definition
before it is being used the first time.

Fixes: 8cab00bdb581 ("ovn-controller: Add OF rules for port security.")
Signed-off-by: Xavier Simonart 
---
 tests/ovn.at | 216 +--
 1 file changed, 108 insertions(+), 108 deletions(-)

diff --git a/tests/ovn.at b/tests/ovn.at
index 53fd1c495..bd7eb928f 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -34036,6 +34036,14 @@ sw0p2_key=$(printf "%x" $(fetch_column Port_Binding 
tunnel_key logical_port=sw0p
 > hv2_t74_flows.expected
 > hv2_t75_flows.expected
 
+check_port_sec_offlows() {
+hv=$1
+t=$2
+
+as $hv ovs-ofctl dump-flows br-int table=${t} | ofctl_strip_all | sort | 
grep -v NXST_FLOW > ${hv}_t${t}_flows.actual
+AT_CHECK([diff -u ${hv}_t${t}_flows.actual ${hv}_t${t}_flows.expected])
+}
+
 check_port_sec_offlows hv1 73
 check_port_sec_offlows hv1 74
 check_port_sec_offlows hv1 75
@@ -34047,33 +34055,25 @@ check_port_sec_offlows hv2 75
 # Set port security for sw0p1
 check ovn-nbctl --wait=hv lsp-set-port-security sw0p1 "00:00:00:00:00:03"
 
-check_port_sec_offlows() {
-hv=$1
-t=$2
-
-as $hv ovs-ofctl dump-flows br-int table=${t} | ofctl_strip_all | sort | 
grep -v NXST_FLOW > ${hv}_t${t}_flows.actual
-AT_CHECK([diff -u ${hv}_t${t}_flows.actual ${hv}_t${t}_flows.expected])
-}
-
 echo " table=73, priority=80,reg14=0x$sw0p1_key,metadata=0x$sw0_dp_key 
actions=load:0x1->NXM_NX_REG10[[12]]
  table=73, 
priority=90,reg14=0x$sw0p1_key,metadata=0x$sw0_dp_key,dl_src=00:00:00:00:00:03 
actions=resubmit(,74)
- table=73, priority=95,arp,reg14=0x1,metadata=0x$sw0_dp_key 
actions=resubmit(,74)" > hv1_t73_flows.expected
+ table=73, priority=95,arp,reg14=0x$sw0p1_key,metadata=0x$sw0_dp_key 
actions=resubmit(,74)" > hv1_t73_flows.expected
 
 check_port_sec_offlows hv1 73
 
-echo " table=74, priority=80,arp,reg14=0x1,metadata=0x1 
actions=load:0x1->NXM_NX_REG10[[12]]
- table=74, priority=80,icmp6,reg14=0x1,metadata=0x1,nw_ttl=255,icmp_type=135 
actions=load:0->NXM_NX_REG10[[12]]
- table=74, priority=80,icmp6,reg14=0x1,metadata=0x1,nw_ttl=255,icmp_type=136 
actions=load:0x1->NXM_NX_REG10[[12]]
- table=74, 
priority=90,arp,reg14=0x1,metadata=0x1,dl_src=00:00:00:00:00:03,arp_sha=00:00:00:00:00:03
 actions=load:0->NXM_NX_REG10[[12]]
- table=74, 
priority=90,icmp6,reg14=0x1,metadata=0x1,nw_ttl=225,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:00
 actions=load:0->NXM_NX_REG10[[12]]
- table=74, 
priority=90,icmp6,reg14=0x1,metadata=0x1,nw_ttl=225,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:03
 actions=load:0->NXM_NX_REG10[[12]]
- table=74, 
priority=90,icmp6,reg14=0x1,metadata=0x1,nw_ttl=225,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:00
 actions=load:0->NXM_NX_REG10[[12]]
- table=74, 
priority=90,icmp6,reg14=0x1,metadata=0x1,nw_ttl=225,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:03
 actions=load:0->NXM_NX_REG10[[12]]" > hv1_t74_flows.expected
+echo " table=74, priority=80,arp,reg14=0x$sw0p1_key,metadata=0x1 
actions=load:0x1->NXM_NX_REG10[[12]]
+ table=74, 
priority=80,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=255,icmp_type=135 
actions=load:0->NXM_NX_REG10[[12]]
+ table=74, 
priority=80,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=255,icmp_type=136 
actions=load:0x1->NXM_NX_REG10[[12]]
+ table=74, 
priority=90,arp,reg14=0x$sw0p1_key,metadata=0x1,dl_src=00:00:00:00:00:03,arp_sha=00:00:00:00:00:03
 actions=load:0->NXM_NX_REG10[[12]]
+ table=74, 
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:00
 actions=load:0->NXM_NX_REG10[[12]]
+ table=74, 
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=135,icmp_code=0,nd_sll=00:00:00:00:00:03
 actions=load:0->NXM_NX_REG10[[12]]
+ table=74, 
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:00
 actions=load:0->NXM_NX_REG10[[12]]
+ table=74, 
priority=90,icmp6,reg14=0x$sw0p1_key,metadata=0x1,nw_ttl=225,icmp_type=136,icmp_code=0,nd_tll=00:00:00:00:00:03
 actions=load:0->NXM_NX_REG10[[12]]" > hv1_t74_flows.expected
 
 check_port_sec_offlows hv1 74
 
-echo " table=75, priority=80,reg15=0x1,metadata=0x1 
actions=load:0x1->NXM_NX_REG10[[12]]
- table=75, priority=85,reg15=0x1,metadata=0x1,dl_dst=00:00:00:00:00:03 
actions=load:0->NXM_NX_REG10[[12]]" > hv1_t75_flows.expected
+echo " table=75, priority=80,reg15=0x$sw0p1_key,metadata=0x1 
actions=load:0x1->NXM_NX_REG10[[12]]
+ table=75, 
priority=85,reg15=0x$sw0p1_key,metadata=0x1,dl_dst=00:00:00:00:00:03 
actions=load:0->NXM_NX_REG10[[12]]" > hv1_t75_flows.expected
 
 check_port_sec_offlows hv1 75
 
@@ -34088,42 +34088,42 @@ check_port_sec_offlows hv2 75
 # Add IPv4 addresses to sw0p1
 check ovn-nbctl --wait=hv 

Re: [ovs-dev] [PATCH ovn] system-tests: Do not hardcode DP key for the flows

[Xavier Simonart]

Looks good to me, thanks.
Acked-by: Xavier Simonart 


On Tue, Jul 4, 2023 at 12:33 PM Ales Musil  wrote:

> The order of DP key generation is not guaranteed, and
> it might happen that the R2 router gets different value than
> the hardcoded 2. Get the value from DB instead.
>
> Signed-off-by: Ales Musil 
> ---
>  tests/system-ovn-kmod.at | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tests/system-ovn-kmod.at b/tests/system-ovn-kmod.at
> index ebfaf26a0..b29e6b55a 100644
> --- a/tests/system-ovn-kmod.at
> +++ b/tests/system-ovn-kmod.at
> @@ -364,8 +364,9 @@ sed -e 's/zone=[[0-9]]*/zone=/;
> s/src=192.168.[[0-9]].2/src=192.168.
>  
> tcp,orig=(src=172.16.1.2,dst=172.16.1.100,sport=,dport=),reply=(src=192.168..2,dst=172.16.1.2,sport=,dport=),zone=,mark=2,protoinfo=(state=)
>  ])
>
> -AT_CHECK([ovs-ofctl dump-flows br-int table=78 |grep cookie |sed -e
> 's/duration=[[0-9]]*.[[0-9]]*s/duration=/;
> s/load:0xc0a80[[0-9]]02/load:0xc0a8002/;
> s/n_packets=[[0-9]]*/n_packets=/;
> s/n_bytes=[[0-9]]*/n_bytes=/;
> s/idle_age=[[0-9]]*/idle_age=/; s/hard_age=[[0-9]]*, //'], [0],
> [dnl
> - cookie=0x0, duration=, table=78, n_packets=,
> n_bytes=, idle_timeout=60, idle_age=,
> tcp,metadata=0x2,nw_src=172.16.1.2,nw_dst=172.16.1.100,tp_dst=8080
> actions=load:0x1->NXM_NX_REG10[[14]],load:0xc0a8002->NXM_NX_REG4[[]],load:0x50->NXM_NX_REG8[[0..15]]
> +dp_key=$(printf "0x%x" $(fetch_column datapath tunnel_key
> external_ids:name=R2))
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=78 --no-stats | sed
> -e 's/load:0xc0a80[[0-9]]02/load:0xc0a8002/'], [0], [dnl
> + table=78, idle_timeout=60,
> tcp,metadata=$dp_key,nw_src=172.16.1.2,nw_dst=172.16.1.100,tp_dst=8080
> actions=load:0x1->NXM_NX_REG10[[14]],load:0xc0a8002->NXM_NX_REG4[[]],load:0x50->NXM_NX_REG8[[0..15]]
>  ])
>
>  check_affinity_flows () {
> @@ -664,8 +665,9 @@ sed -e 's/zone=[[0-9]]*/zone=/;
> s/src=fd1[[0-9]]::2/src=fd1::2
>
>  
> tcp,orig=(src=fd72::2,dst=fd30::1,sport=,dport=),reply=(src=fd1::2,dst=fd72::2,sport=,dport=),zone=,mark=2,protoinfo=(state=)
>  ])
>
> -AT_CHECK([ovs-ofctl dump-flows br-int table=78 |grep cookie |sed -e
> 's/duration=[[0-9]]*.[[0-9]]*s/duration=/;
> s/load:0xfd1[[0-9]]/load:0xfd1/;
> s/n_packets=[[0-9]]*/n_packets=/;
> s/n_bytes=[[0-9]]*/n_bytes=/;
> s/idle_age=[[0-9]]*/idle_age=/; s/hard_age=[[0-9]]*, //'], [0],
> [dnl
> - cookie=0x0, duration=, table=78, n_packets=,
> n_bytes=, idle_timeout=60, idle_age=,
> tcp6,metadata=0x2,ipv6_src=fd72::2,ipv6_dst=fd30::1,tp_dst=8080
> actions=load:0x1->NXM_NX_REG10[[14]],load:0x2->NXM_NX_XXREG1[[0..63]],load:0xfd1->NXM_NX_XXREG1[[64..127]],load:0x50->NXM_NX_REG8[[0..15]]
> +dp_key=$(printf "0x%x" $(fetch_column datapath tunnel_key
> external_ids:name=R2))
> +AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=78 --no-stats | sed
> -e 's/load:0xfd1[[0-9]]/load:0xfd1/'],
> [0], [dnl
> + table=78, idle_timeout=60,
> tcp6,metadata=$dp_key,ipv6_src=fd72::2,ipv6_dst=fd30::1,tp_dst=8080
> actions=load:0x1->NXM_NX_REG10[[14]],load:0x2->NXM_NX_XXREG1[[0..63]],load:0xfd1->NXM_NX_XXREG1[[64..127]],load:0x50->NXM_NX_REG8[[0..15]]
>  ])
>
>  check_affinity_flows () {
> --
> 2.40.1
>
> ___
> 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] tests: fixed userspace-system tests not properly cleaned up

[Ales Musil]

On Tue, Jul 4, 2023 at 12:08 PM Xavier Simonart  wrote:

> Usually, when cleaning up a system test we dexecute (for the ovn-vswitchd
> process):
> - OVS_TRAFFIC_VSWITCHD_STOP (i.e. ovs-appctl -t $1 exit --cleanup, wait up
> to 30 seconds
>   and report failure if not properly cleaned up).
> - kill_ovs_vswitchd (i.e. if ovs-vsitwhd still running,
>   ovs-appctl -t ovs-vswitchd exit --cleanup, wait 1 to 10 seconds and kill
> ovs-vswitvchd
>if not properly cleaned up).
>
> If we do not execute OVS_TRAFFIC_VSWITCHD_STOP, wait time is much smaller
> which might result in
> - clean up not done properly, ovs-vswitchd killed, ovs-netdev not deleted,
> and no error reported
> - next tests will fail
>
> Using this patch, we properly execute OVS_TRAFFIC_VSWITCHD_STOP, so the
> delay waiting for
> ovs-vswitchd to exit is longer, and, if by any chance this delay is still
> too short,
> the proper test will fail (i.e. the first test failing will be the one
> unable to clean).
>
> Signed-off-by: Xavier Simonart 
> ---
>  tests/system-ovn.at | 90 +
>  1 file changed, 90 insertions(+)
>
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 44a8072d6..8db08da70 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -5819,6 +5819,21 @@
> tcp,orig=(src=42.42.42.3,dst=42.42.42.2,sport=,dport=4242),reply=(s
>
>  
> tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
>  ])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> @@ -5915,6 +5930,21 @@
> tcp,orig=(src=4242::3,dst=4242::2,sport=,dport=4242),reply=(src=424
>
>  
> tcp,orig=(src=4242::3,dst=::1,sport=,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
>  ])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> @@ -7136,6 +7166,21 @@ as
>  OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
>  /.*terminating with signal 15.*/d"])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> @@ -7238,6 +7283,21 @@ NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2
> 172.18.2.12 | FORMAT_PING], \
>  3 packets transmitted, 3 received, 0% packet loss, time 0ms
>  ])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> @@ -9168,6 +9228,21 @@ OVS_WAIT_UNTIL([
>  test "${requests}" -ge "6"
>  ])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> @@ -9307,6 +9382,21 @@ OVS_WAIT_UNTIL([
>  test "${requests}" -ge "6"
>  ])
>
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
>  AT_CLEANUP
>  ])
>
> --
> 2.31.1
>
>
Looks good to me, thanks!

Acked-by: Ales Musil 

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.comIM: amusil

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

Re: [ovs-dev] [PATCH ovn v3 2/2] ci: Run the new check-system-dpdk tests as part of the ci.

[Ales Musil]

On Tue, Jul 4, 2023 at 12:53 PM Eelco Chaudron  wrote:

>
>
> On 4 Jul 2023, at 11:00, Ales Musil wrote:
>
> > On Mon, Jun 19, 2023 at 11:32 AM Eelco Chaudron 
> wrote:
> >
> >> This patch includes changes made earlier by David in the
> >> ovs branch to cache the dpdk builds.
> >>
> >> Co-authored-by: David Marchand 
> >> Signed-off-by: David Marchand 
> >> Signed-off-by: Eelco Chaudron 
> >> ---
> >>
> >> v2: Replaced 'sleep 1' with '' after consulting with Dumitru.
> >> v3: No changes
> >>
> >> Note that I ran the full GitHub ci 20x on the v1 patchset.
> >> For v2, I did 10 runs of only the changed/fixed test. For v3,
> >> I only did 1 run.
> >>
> >
> > Hi Eelco,
> >
> > I have one comment down below.
> >
> >
> >>
> >>  .ci/ci.sh  |   12 ++-
> >>  .ci/dpdk-build.sh  |   54 ++
> >>  .ci/dpdk-prepare.sh|   11 ++
> >>  .ci/linux-build.sh |   49 ++-
> >>  .github/workflows/test.yml |   80
> >> 
> >>  Makefile.am|2 +
> >>  tests/system-ovn.at|2 +
> >>  7 files changed, 207 insertions(+), 3 deletions(-)
> >>  create mode 100755 .ci/dpdk-build.sh
> >>  create mode 100755 .ci/dpdk-prepare.sh
> >>
> >> diff --git a/.ci/ci.sh b/.ci/ci.sh
> >> index 90942bab6..10f11939c 100755
> >> --- a/.ci/ci.sh
> >> +++ b/.ci/ci.sh
> >> @@ -16,6 +16,7 @@
> >>
> >>  OVN_PATH=${OVN_PATH:-$PWD}
> >>  OVS_PATH=${OVS_PATH:-$OVN_PATH/ovs}
> >> +DPDK_PATH=${DPDK_PATH:-$OVN_PATH/dpdk-dir}
> >>  CONTAINER_CMD=${CONTAINER_CMD:-podman}
> >>  CONTAINER_WORKSPACE="/workspace"
> >>  CONTAINER_WORKDIR="/workspace/ovn-tmp"
> >> @@ -80,6 +81,10 @@ function copy_sources_to_workdir() {
> >>  && \
> >>  cp -a $CONTAINER_WORKSPACE/ovs/. $CONTAINER_WORKDIR/ovs \
> >>  && \
> >> +rm -rf $CONTAINER_WORKDIR/dpdk-dir \
> >> +&& \
> >> +cp -a $CONTAINER_WORKSPACE/dpdk-dir/.
> $CONTAINER_WORKDIR/dpdk-dir
> >> \
> >> +&& \
> >>  git config --global --add safe.directory $CONTAINER_WORKDIR
> >>  "
> >>  }
> >> @@ -95,7 +100,7 @@ function run_tests() {
> >>  cd $CONTAINER_WORKDIR \
> >>  && \
> >>  ARCH=$ARCH CC=$CC LIBS=$LIBS OPTS=$OPTS TESTSUITE=$TESTSUITE \
> >> -TEST_RANGE=$TEST_RANGE SANITIZERS=$SANITIZERS \
> >> +TEST_RANGE=$TEST_RANGE SANITIZERS=$SANITIZERS DPDK=$DPDK \
> >>  ./.ci/linux-build.sh
> >>  "
> >>  }
> >> @@ -148,12 +153,17 @@ if [ "$ARCH" = "aarch64" ]; then
> >>  ASAN_OPTIONS="detect_leaks=0"
> >>  fi
> >>
> >> +if [ -z "$DPDK" ]; then
> >> +   mkdir -p "$DPDK_PATH"
> >> +fi
> >> +
> >>  CONTAINER_ID="$($CONTAINER_CMD run --privileged -d \
> >>  --pids-limit=-1 \
> >>  --env ASAN_OPTIONS=$ASAN_OPTIONS \
> >>  -v /lib/modules/$(uname -r):/lib/modules/$(uname -r):ro \
> >>  -v $OVN_PATH:$CONTAINER_WORKSPACE/ovn:Z \
> >>  -v $OVS_PATH:$CONTAINER_WORKSPACE/ovs:Z \
> >> +-v $DPDK_PATH:$CONTAINER_WORKSPACE/dpdk-dir:Z \
> >>  $IMAGE_NAME)"
> >>  trap remove_container EXIT
> >>
> >> diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh
> >> new file mode 100755
> >> index 0..f44ac15b0
> >> --- /dev/null
> >> +++ b/.ci/dpdk-build.sh
> >> @@ -0,0 +1,54 @@
> >> +#!/bin/bash
> >> +
> >> +set -o errexit
> >> +set -x
> >> +
> >> +function build_dpdk()
> >> +{
> >> +local VERSION_FILE="dpdk-dir/cached-version"
> >> +local DPDK_VER=$1
> >> +local DPDK_OPTS=""
> >> +
> >> +rm -rf dpdk-dir
> >> +
> >> +if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
> >> +git clone --single-branch $DPDK_GIT dpdk-dir -b
> >> "${DPDK_VER##refs/*/}"
> >> +pushd dpdk-dir
> >> +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
> >> +fi
> >> +
> >> +# Switching to 'default' machine to make dpdk-dir cache usable on
> >> +# different CPUs. We can't be sure that all CI machines are exactly
> >> same.
> >> +DPDK_OPTS="$DPDK_OPTS -Dmachine=default"
> >> +
> >> +# Disable building DPDK unit tests. Not needed for OVS build or
> tests.
> >> +DPDK_OPTS="$DPDK_OPTS -Dtests=false"
> >> +
> >> +# Disable DPDK developer mode, this results in less build checks
> and
> >> less
> >> +# meson verbose outputs.
> >> +DPDK_OPTS="$DPDK_OPTS -Ddeveloper_mode=disabled"
> >> +
> >> +# OVS compilation and the "ovn-system-dpdk" unit tests (run in the
> CI)
> >> +# 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"
> >> +
> >> +# Install DPDK using prefix.
> >> +DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build"
> >> +

Re: [ovs-dev] [PATCH ovn v3 2/2] ci: Run the new check-system-dpdk tests as part of the ci.

[Eelco Chaudron]

On 4 Jul 2023, at 11:00, Ales Musil wrote:

> On Mon, Jun 19, 2023 at 11:32 AM Eelco Chaudron  wrote:
>
>> This patch includes changes made earlier by David in the
>> ovs branch to cache the dpdk builds.
>>
>> Co-authored-by: David Marchand 
>> Signed-off-by: David Marchand 
>> Signed-off-by: Eelco Chaudron 
>> ---
>>
>> v2: Replaced 'sleep 1' with '' after consulting with Dumitru.
>> v3: No changes
>>
>> Note that I ran the full GitHub ci 20x on the v1 patchset.
>> For v2, I did 10 runs of only the changed/fixed test. For v3,
>> I only did 1 run.
>>
>
> Hi Eelco,
>
> I have one comment down below.
>
>
>>
>>  .ci/ci.sh  |   12 ++-
>>  .ci/dpdk-build.sh  |   54 ++
>>  .ci/dpdk-prepare.sh|   11 ++
>>  .ci/linux-build.sh |   49 ++-
>>  .github/workflows/test.yml |   80
>> 
>>  Makefile.am|2 +
>>  tests/system-ovn.at|2 +
>>  7 files changed, 207 insertions(+), 3 deletions(-)
>>  create mode 100755 .ci/dpdk-build.sh
>>  create mode 100755 .ci/dpdk-prepare.sh
>>
>> diff --git a/.ci/ci.sh b/.ci/ci.sh
>> index 90942bab6..10f11939c 100755
>> --- a/.ci/ci.sh
>> +++ b/.ci/ci.sh
>> @@ -16,6 +16,7 @@
>>
>>  OVN_PATH=${OVN_PATH:-$PWD}
>>  OVS_PATH=${OVS_PATH:-$OVN_PATH/ovs}
>> +DPDK_PATH=${DPDK_PATH:-$OVN_PATH/dpdk-dir}
>>  CONTAINER_CMD=${CONTAINER_CMD:-podman}
>>  CONTAINER_WORKSPACE="/workspace"
>>  CONTAINER_WORKDIR="/workspace/ovn-tmp"
>> @@ -80,6 +81,10 @@ function copy_sources_to_workdir() {
>>  && \
>>  cp -a $CONTAINER_WORKSPACE/ovs/. $CONTAINER_WORKDIR/ovs \
>>  && \
>> +rm -rf $CONTAINER_WORKDIR/dpdk-dir \
>> +&& \
>> +cp -a $CONTAINER_WORKSPACE/dpdk-dir/. $CONTAINER_WORKDIR/dpdk-dir
>> \
>> +&& \
>>  git config --global --add safe.directory $CONTAINER_WORKDIR
>>  "
>>  }
>> @@ -95,7 +100,7 @@ function run_tests() {
>>  cd $CONTAINER_WORKDIR \
>>  && \
>>  ARCH=$ARCH CC=$CC LIBS=$LIBS OPTS=$OPTS TESTSUITE=$TESTSUITE \
>> -TEST_RANGE=$TEST_RANGE SANITIZERS=$SANITIZERS \
>> +TEST_RANGE=$TEST_RANGE SANITIZERS=$SANITIZERS DPDK=$DPDK \
>>  ./.ci/linux-build.sh
>>  "
>>  }
>> @@ -148,12 +153,17 @@ if [ "$ARCH" = "aarch64" ]; then
>>  ASAN_OPTIONS="detect_leaks=0"
>>  fi
>>
>> +if [ -z "$DPDK" ]; then
>> +   mkdir -p "$DPDK_PATH"
>> +fi
>> +
>>  CONTAINER_ID="$($CONTAINER_CMD run --privileged -d \
>>  --pids-limit=-1 \
>>  --env ASAN_OPTIONS=$ASAN_OPTIONS \
>>  -v /lib/modules/$(uname -r):/lib/modules/$(uname -r):ro \
>>  -v $OVN_PATH:$CONTAINER_WORKSPACE/ovn:Z \
>>  -v $OVS_PATH:$CONTAINER_WORKSPACE/ovs:Z \
>> +-v $DPDK_PATH:$CONTAINER_WORKSPACE/dpdk-dir:Z \
>>  $IMAGE_NAME)"
>>  trap remove_container EXIT
>>
>> diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh
>> new file mode 100755
>> index 0..f44ac15b0
>> --- /dev/null
>> +++ b/.ci/dpdk-build.sh
>> @@ -0,0 +1,54 @@
>> +#!/bin/bash
>> +
>> +set -o errexit
>> +set -x
>> +
>> +function build_dpdk()
>> +{
>> +local VERSION_FILE="dpdk-dir/cached-version"
>> +local DPDK_VER=$1
>> +local DPDK_OPTS=""
>> +
>> +rm -rf dpdk-dir
>> +
>> +if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
>> +git clone --single-branch $DPDK_GIT dpdk-dir -b
>> "${DPDK_VER##refs/*/}"
>> +pushd dpdk-dir
>> +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
>> +fi
>> +
>> +# Switching to 'default' machine to make dpdk-dir cache usable on
>> +# different CPUs. We can't be sure that all CI machines are exactly
>> same.
>> +DPDK_OPTS="$DPDK_OPTS -Dmachine=default"
>> +
>> +# Disable building DPDK unit tests. Not needed for OVS build or tests.
>> +DPDK_OPTS="$DPDK_OPTS -Dtests=false"
>> +
>> +# Disable DPDK developer mode, this results in less build checks and
>> less
>> +# meson verbose outputs.
>> +DPDK_OPTS="$DPDK_OPTS -Ddeveloper_mode=disabled"
>> +
>> +# OVS compilation and the "ovn-system-dpdk" unit tests (run in the CI)
>> +# 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"
>> +
>> +# Install DPDK using prefix.
>> +DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build"
>> +
>> +meson $DPDK_OPTS build
>> +ninja -C build
>> +ninja -C build install
>> +
>> +echo "Installed DPDK in $(pwd)"
>> +popd
>> +echo "${DPDK_VER}" > ${VERSION_FILE}
>> +}
>> +
>> +build_dpdk $DPDK_VER
>> diff --git a/.ci/dpdk-prepare.sh b/.ci/dpdk-prepare.sh
>> new file mode 100755
>> index 0..f7e6215dd
>> --- 

Re: [ovs-dev] [PATCH] ovsdb-idl: Reparse reference src before dst is deleted

[Tao Liu]

Hi, Ilya. Thanks for your comment.

On 7/4/23 4:10 AM, Ilya Maximets wrote:

On 7/2/23 11:59, Tao Liu wrote:

Commit 02f31a1262fc has fixed the row references when insertion and
deletion execute in one IDL run. However, we can still hit ovn-controller
crash in pressure test where pb->datapath == NULL. It is triggered by
reference dst deletion in same IDL run:

   idl run:
 update1:
   insert port_binding
   insert datapath_binding
 update2:
   delete datapath_binding
   delete port_binding

Fix it by reparse reference src before dst deletion.

Hi.  Thanks for the patch!

There seems to be an issue indeed.  Removed rows should have valid
references to other removed rows.

However, I'm not sure this solution solves the issue in a general case
as it depends on the order of events.  If events in the update2 will
be in opposite order, the issue will still be there.


The opposite order of update2 has been fixed in commit 02f31a1262fc, 
which now moved to


first reparse_row().


In a more general case we may also have two rows reference each other
and we need to preserve both references.

We probably could:

  1. Postpone all the deletions, execute insertions and updates.

  2. Call ovsdb_idl_reparse_refs_to_inserted() to re-parse every row
 that needs re-parsing due to newly added rows.

  3. Delete the rows that need to be deleted.

  4. Call ovsdb_idl_reparse_deleted() to get rid of references to
 deleted rows from non-deleted ones.


Sounds like a good idea, that makes things more clear.


What do you think?
Dumitru, maybe you have some thoughts on this?

And we will need need a unit test for this issue.


Will try to add one.


Best regards, Ilya Maximets.


Fixes: 02f31a1262fc ("ovsdb-idl: Preserve references for rows deleted in same IDL 
run as their insertion.")
Signed-off-by: Tao Liu 
---
  lib/ovsdb-idl.c | 18 --
  1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 634fbb56d..89393b52b 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2425,11 +2425,25 @@ ovsdb_idl_insert_row(struct ovsdb_idl_row *row, const 
struct shash *data)
  }
  
  static void

-ovsdb_idl_delete_row(struct ovsdb_idl_row *row)
+reparse_row(struct ovsdb_idl_row *row)
  {
-/* If row has to be reparsed, reparse it before it's deleted. */
  if (!ovs_list_is_empty(>reparse_node)) {
  ovsdb_idl_row_parse(row);
+ovs_list_remove(>reparse_node);
+ovs_list_init(>reparse_node);
+}
+}
+
+static void
+ovsdb_idl_delete_row(struct ovsdb_idl_row *row)
+{
+struct ovsdb_idl_arc *arc;
+
+/* If row has to be reparsed, reparse it before it's deleted. */
+reparse_row(row);
+/* Reparse src before dst is deleted */
+LIST_FOR_EACH (arc, dst_node, >dst_arcs) {
+reparse_row(arc->src);
  }
  ovsdb_idl_remove_from_indexes(row);
  ovsdb_idl_row_clear_arcs(row, true);


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

Re: [ovs-dev] [PATCH v6 0/9] Add vxlan gbp offload with TC

[Eelco Chaudron]

On 27 Jun 2023, at 12:48, Roi Dayan wrote:

> Hi,
>
> This series adds TC offload support for filtering vxlan tunnels with gbp 
> option.
> First 4 patches do some refactoring and the later patches adds the feature.
>
> Thanks,
> Roi

Thanks for following up on the series!

I’ve applied the changes discussed and applied the series to the master branch!

Cheers,

Eelco


> changelog
>
> v6:
> - fix nits in patch #7.
> - fix test in patch #9 to run test on supported kernel.
>
> v5:
> - Fix byte order issue in probe.
>
> v4:
> - probe TC kernel for vxlan gbp support.
> - add test.
> - style fix in patch 3.
> - log warn instead of err in patch 5.
>
> v3:
> - Add function nl_msg_start_nested_with_flag() to be used
>   with TC fiedls that require the nested flag. currently
>   only vxlan gbp tun opts.
> - Split put flower tunnel opts to sub functions for geneve
>   and vxlan tun opts.
>
> v2:
> - Fix incorrect compat modification in
>   patch "tc: Add vxlan gbp option flower match offload".
>
> Gavin Li (9):
>   tc: Pass tunnel entirely to tunnel option parse and put functions
>   odp-util: Extract vxlan gbp option decoding to a function
>   odp-util: Extract vxlan gbp option encoding to a function
>   netlink: Add new function to add NLA_F_NESTED to nested netlink
> messages
>   tc: Add vxlan gbp option flower match offload
>   tc: Pass encap entirely to nl_msg_put_act_tunnel_key_set
>   tc: Add vxlan encap action with gbp option offload
>   netdev-tc-offloads: Probe for allowing vxlan gbp support
>   system-offloads-traffic.at: Add vxlan gbp offload test
>
>  acinclude.m4 |   7 +
>  include/linux/pkt_cls.h  |  13 ++
>  include/linux/tc_act/tc_tunnel_key.h |  17 ++-
>  lib/netdev-offload-tc.c  | 105 -
>  lib/netlink.c|   9 ++
>  lib/netlink.h|   1 +
>  lib/odp-util.c   |  54 ---
>  lib/odp-util.h   |  16 ++
>  lib/tc.c | 218 ---
>  lib/tc.h |  80 +-
>  tests/system-offloads-traffic.at |  50 ++
>  11 files changed, 461 insertions(+), 109 deletions(-)
>
> -- 
> 2.38.0

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

[ovs-dev] [PATCH ovn] system-tests: Do not hardcode DP key for the flows

[Ales Musil]

The order of DP key generation is not guaranteed, and
it might happen that the R2 router gets different value than
the hardcoded 2. Get the value from DB instead.

Signed-off-by: Ales Musil 
---
 tests/system-ovn-kmod.at | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/system-ovn-kmod.at b/tests/system-ovn-kmod.at
index ebfaf26a0..b29e6b55a 100644
--- a/tests/system-ovn-kmod.at
+++ b/tests/system-ovn-kmod.at
@@ -364,8 +364,9 @@ sed -e 's/zone=[[0-9]]*/zone=/; 
s/src=192.168.[[0-9]].2/src=192.168.,dport=),reply=(src=192.168..2,dst=172.16.1.2,sport=,dport=),zone=,mark=2,protoinfo=(state=)
 ])
 
-AT_CHECK([ovs-ofctl dump-flows br-int table=78 |grep cookie |sed -e 
's/duration=[[0-9]]*.[[0-9]]*s/duration=/; 
s/load:0xc0a80[[0-9]]02/load:0xc0a8002/; 
s/n_packets=[[0-9]]*/n_packets=/; 
s/n_bytes=[[0-9]]*/n_bytes=/; s/idle_age=[[0-9]]*/idle_age=/; 
s/hard_age=[[0-9]]*, //'], [0], [dnl
- cookie=0x0, duration=, table=78, n_packets=, 
n_bytes=, idle_timeout=60, idle_age=, 
tcp,metadata=0x2,nw_src=172.16.1.2,nw_dst=172.16.1.100,tp_dst=8080 
actions=load:0x1->NXM_NX_REG10[[14]],load:0xc0a8002->NXM_NX_REG4[[]],load:0x50->NXM_NX_REG8[[0..15]]
+dp_key=$(printf "0x%x" $(fetch_column datapath tunnel_key 
external_ids:name=R2))
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=78 --no-stats | sed -e 
's/load:0xc0a80[[0-9]]02/load:0xc0a8002/'], [0], [dnl
+ table=78, idle_timeout=60, 
tcp,metadata=$dp_key,nw_src=172.16.1.2,nw_dst=172.16.1.100,tp_dst=8080 
actions=load:0x1->NXM_NX_REG10[[14]],load:0xc0a8002->NXM_NX_REG4[[]],load:0x50->NXM_NX_REG8[[0..15]]
 ])
 
 check_affinity_flows () {
@@ -664,8 +665,9 @@ sed -e 's/zone=[[0-9]]*/zone=/; 
s/src=fd1[[0-9]]::2/src=fd1::2
 
tcp,orig=(src=fd72::2,dst=fd30::1,sport=,dport=),reply=(src=fd1::2,dst=fd72::2,sport=,dport=),zone=,mark=2,protoinfo=(state=)
 ])
 
-AT_CHECK([ovs-ofctl dump-flows br-int table=78 |grep cookie |sed -e 
's/duration=[[0-9]]*.[[0-9]]*s/duration=/; 
s/load:0xfd1[[0-9]]/load:0xfd1/; 
s/n_packets=[[0-9]]*/n_packets=/; 
s/n_bytes=[[0-9]]*/n_bytes=/; s/idle_age=[[0-9]]*/idle_age=/; 
s/hard_age=[[0-9]]*, //'], [0], [dnl
- cookie=0x0, duration=, table=78, n_packets=, 
n_bytes=, idle_timeout=60, idle_age=, 
tcp6,metadata=0x2,ipv6_src=fd72::2,ipv6_dst=fd30::1,tp_dst=8080 
actions=load:0x1->NXM_NX_REG10[[14]],load:0x2->NXM_NX_XXREG1[[0..63]],load:0xfd1->NXM_NX_XXREG1[[64..127]],load:0x50->NXM_NX_REG8[[0..15]]
+dp_key=$(printf "0x%x" $(fetch_column datapath tunnel_key 
external_ids:name=R2))
+AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int table=78 --no-stats | sed -e 
's/load:0xfd1[[0-9]]/load:0xfd1/'], [0], [dnl
+ table=78, idle_timeout=60, 
tcp6,metadata=$dp_key,ipv6_src=fd72::2,ipv6_dst=fd30::1,tp_dst=8080 
actions=load:0x1->NXM_NX_REG10[[14]],load:0x2->NXM_NX_XXREG1[[0..63]],load:0xfd1->NXM_NX_XXREG1[[64..127]],load:0x50->NXM_NX_REG8[[0..15]]
 ])
 
 check_affinity_flows () {
-- 
2.40.1

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

[ovs-dev] [PATCH ovn] tests: fixed userspace-system tests not properly cleaned up

[Xavier Simonart]

Usually, when cleaning up a system test we dexecute (for the ovn-vswitchd 
process):
- OVS_TRAFFIC_VSWITCHD_STOP (i.e. ovs-appctl -t $1 exit --cleanup, wait up to 
30 seconds
  and report failure if not properly cleaned up).
- kill_ovs_vswitchd (i.e. if ovs-vsitwhd still running,
  ovs-appctl -t ovs-vswitchd exit --cleanup, wait 1 to 10 seconds and kill 
ovs-vswitvchd
   if not properly cleaned up).

If we do not execute OVS_TRAFFIC_VSWITCHD_STOP, wait time is much smaller which 
might result in
- clean up not done properly, ovs-vswitchd killed, ovs-netdev not deleted, and 
no error reported
- next tests will fail

Using this patch, we properly execute OVS_TRAFFIC_VSWITCHD_STOP, so the delay 
waiting for
ovs-vswitchd to exit is longer, and, if by any chance this delay is still too 
short,
the proper test will fail (i.e. the first test failing will be the one unable 
to clean).

Signed-off-by: Xavier Simonart 
---
 tests/system-ovn.at | 90 +
 1 file changed, 90 insertions(+)

diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 44a8072d6..8db08da70 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -5819,6 +5819,21 @@ 
tcp,orig=(src=42.42.42.3,dst=42.42.42.2,sport=,dport=4242),reply=(s
 
tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
 ])
 
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
 AT_CLEANUP
 ])
 
@@ -5915,6 +5930,21 @@ 
tcp,orig=(src=4242::3,dst=4242::2,sport=,dport=4242),reply=(src=424
 
tcp,orig=(src=4242::3,dst=::1,sport=,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
 ])
 
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
 AT_CLEANUP
 ])
 
@@ -7136,6 +7166,21 @@ as
 OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
 /.*terminating with signal 15.*/d"])
 
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
 AT_CLEANUP
 ])
 
@@ -7238,6 +7283,21 @@ NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 
172.18.2.12 | FORMAT_PING], \
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
 ])
 
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
 AT_CLEANUP
 ])
 
@@ -9168,6 +9228,21 @@ OVS_WAIT_UNTIL([
 test "${requests}" -ge "6"
 ])
 
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
 AT_CLEANUP
 ])
 
@@ -9307,6 +9382,21 @@ OVS_WAIT_UNTIL([
 test "${requests}" -ge "6"
 ])
 
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
 AT_CLEANUP
 ])
 
-- 
2.31.1

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

Re: [ovs-dev] [PATCH ovn v3 2/2] ci: Run the new check-system-dpdk tests as part of the ci.

[Ales Musil]

On Mon, Jun 19, 2023 at 11:32 AM Eelco Chaudron  wrote:

> This patch includes changes made earlier by David in the
> ovs branch to cache the dpdk builds.
>
> Co-authored-by: David Marchand 
> Signed-off-by: David Marchand 
> Signed-off-by: Eelco Chaudron 
> ---
>
> v2: Replaced 'sleep 1' with '' after consulting with Dumitru.
> v3: No changes
>
> Note that I ran the full GitHub ci 20x on the v1 patchset.
> For v2, I did 10 runs of only the changed/fixed test. For v3,
> I only did 1 run.
>

Hi Eelco,

I have one comment down below.


>
>  .ci/ci.sh  |   12 ++-
>  .ci/dpdk-build.sh  |   54 ++
>  .ci/dpdk-prepare.sh|   11 ++
>  .ci/linux-build.sh |   49 ++-
>  .github/workflows/test.yml |   80
> 
>  Makefile.am|2 +
>  tests/system-ovn.at|2 +
>  7 files changed, 207 insertions(+), 3 deletions(-)
>  create mode 100755 .ci/dpdk-build.sh
>  create mode 100755 .ci/dpdk-prepare.sh
>
> diff --git a/.ci/ci.sh b/.ci/ci.sh
> index 90942bab6..10f11939c 100755
> --- a/.ci/ci.sh
> +++ b/.ci/ci.sh
> @@ -16,6 +16,7 @@
>
>  OVN_PATH=${OVN_PATH:-$PWD}
>  OVS_PATH=${OVS_PATH:-$OVN_PATH/ovs}
> +DPDK_PATH=${DPDK_PATH:-$OVN_PATH/dpdk-dir}
>  CONTAINER_CMD=${CONTAINER_CMD:-podman}
>  CONTAINER_WORKSPACE="/workspace"
>  CONTAINER_WORKDIR="/workspace/ovn-tmp"
> @@ -80,6 +81,10 @@ function copy_sources_to_workdir() {
>  && \
>  cp -a $CONTAINER_WORKSPACE/ovs/. $CONTAINER_WORKDIR/ovs \
>  && \
> +rm -rf $CONTAINER_WORKDIR/dpdk-dir \
> +&& \
> +cp -a $CONTAINER_WORKSPACE/dpdk-dir/. $CONTAINER_WORKDIR/dpdk-dir
> \
> +&& \
>  git config --global --add safe.directory $CONTAINER_WORKDIR
>  "
>  }
> @@ -95,7 +100,7 @@ function run_tests() {
>  cd $CONTAINER_WORKDIR \
>  && \
>  ARCH=$ARCH CC=$CC LIBS=$LIBS OPTS=$OPTS TESTSUITE=$TESTSUITE \
> -TEST_RANGE=$TEST_RANGE SANITIZERS=$SANITIZERS \
> +TEST_RANGE=$TEST_RANGE SANITIZERS=$SANITIZERS DPDK=$DPDK \
>  ./.ci/linux-build.sh
>  "
>  }
> @@ -148,12 +153,17 @@ if [ "$ARCH" = "aarch64" ]; then
>  ASAN_OPTIONS="detect_leaks=0"
>  fi
>
> +if [ -z "$DPDK" ]; then
> +   mkdir -p "$DPDK_PATH"
> +fi
> +
>  CONTAINER_ID="$($CONTAINER_CMD run --privileged -d \
>  --pids-limit=-1 \
>  --env ASAN_OPTIONS=$ASAN_OPTIONS \
>  -v /lib/modules/$(uname -r):/lib/modules/$(uname -r):ro \
>  -v $OVN_PATH:$CONTAINER_WORKSPACE/ovn:Z \
>  -v $OVS_PATH:$CONTAINER_WORKSPACE/ovs:Z \
> +-v $DPDK_PATH:$CONTAINER_WORKSPACE/dpdk-dir:Z \
>  $IMAGE_NAME)"
>  trap remove_container EXIT
>
> diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh
> new file mode 100755
> index 0..f44ac15b0
> --- /dev/null
> +++ b/.ci/dpdk-build.sh
> @@ -0,0 +1,54 @@
> +#!/bin/bash
> +
> +set -o errexit
> +set -x
> +
> +function build_dpdk()
> +{
> +local VERSION_FILE="dpdk-dir/cached-version"
> +local DPDK_VER=$1
> +local DPDK_OPTS=""
> +
> +rm -rf dpdk-dir
> +
> +if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
> +git clone --single-branch $DPDK_GIT dpdk-dir -b
> "${DPDK_VER##refs/*/}"
> +pushd dpdk-dir
> +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
> +fi
> +
> +# Switching to 'default' machine to make dpdk-dir cache usable on
> +# different CPUs. We can't be sure that all CI machines are exactly
> same.
> +DPDK_OPTS="$DPDK_OPTS -Dmachine=default"
> +
> +# Disable building DPDK unit tests. Not needed for OVS build or tests.
> +DPDK_OPTS="$DPDK_OPTS -Dtests=false"
> +
> +# Disable DPDK developer mode, this results in less build checks and
> less
> +# meson verbose outputs.
> +DPDK_OPTS="$DPDK_OPTS -Ddeveloper_mode=disabled"
> +
> +# OVS compilation and the "ovn-system-dpdk" unit tests (run in the CI)
> +# 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"
> +
> +# Install DPDK using prefix.
> +DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build"
> +
> +meson $DPDK_OPTS build
> +ninja -C build
> +ninja -C build install
> +
> +echo "Installed DPDK in $(pwd)"
> +popd
> +echo "${DPDK_VER}" > ${VERSION_FILE}
> +}
> +
> +build_dpdk $DPDK_VER
> diff --git a/.ci/dpdk-prepare.sh b/.ci/dpdk-prepare.sh
> new file mode 100755
> index 0..f7e6215dd
> --- /dev/null
> +++ b/.ci/dpdk-prepare.sh
> @@ -0,0 +1,11 @@
> +#!/bin/bash
> +
> +set -ev
> +
> +# Installing wheel separately because it may be needed to build some
> +# of the packages during dependency 

Re: [ovs-dev] [PATCH ovn v3 1/2] tests: add make check-system-dpdk to test suite.

[Ales Musil]

On Tue, Jun 27, 2023 at 7:12 PM Dumitru Ceara  wrote:

> On 6/19/23 11:30, Eelco Chaudron wrote:
> > Allow the ovn-system tests to run on the OVS-DPDK infrastructure,
> > i.e., DPDK ports and mbuf memory.
> >
> > Co-authored-by: David Marchand 
> > Signed-off-by: David Marchand 
> > Signed-off-by: Eelco Chaudron 
> > ---
>
> Hi Eelco,
>
> Acked-by: Dumitru Ceara 
>
> Thanks,
> Dumitru
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks.

Acked-by: Ales Musil 

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.comIM: amusil

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

Re: [ovs-dev] [PATCH ovn 00/13] Fixes Multiple Unit Tests

[Ales Musil]

On Tue, Jun 27, 2023 at 11:31 AM Xavier Simonart 
wrote:

> Xavier Simonart (13):
>   tests: fixed "policy-based routing" and "route tables IPv6 --
> overlapping subnets"
>   tests: fixed "dhcpv6 : 1 HV, 2 LS, 5 LSPs" and "external logical port"
>   tests: fixed "send gratuitous ARP for NAT rules on HA distributed
> router"
>   tests: fixed "Load balancer health checks - IPv4 and IPv6"
>   tests: decreased failure rate of "tug-of-war between two chassis for
> the same port"
>   tests: fixed "IPv6 periodic RA"
>   tests: fixed "Encaps tunnel cleanup does not interfere with multiple
> controller on the same host"
>   tests: fixed "check meters update"
>   tests: fixed "send gratuitous arp for nat ips in localnet"
>   tests: fixed "nb_cfg timestamp"
>   tests: fixed "ACL Reject ping pong"
>   tests: fixed "MAC binding aging" and "IGMP external querier"
>   tests: fixed "vtep: 3 HVs, 1 VIFs/HV, 1 GW, 1 LS"
>
>  tests/ovn.at | 94 +++-
>  1 file changed, 56 insertions(+), 38 deletions(-)
>
> --
> 2.31.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
The whole series looks good to me, thanks.

Acked-by: Ales Musil 

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.comIM: amusil

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

Re: [ovs-dev] [PATCH v1] bridge ovs-vsctl Bridge IPFIX enable_input_sampling, enable_ouput_sampling fix unexpected values

[Eelco Chaudron]

On 4 Jul 2023, at 8:55, Sayali Naval (sanaval) via dev wrote:

> As per the Open vSwitch Manual 
> (http://www.openvswitch.org/support/dist-docs/ovs-vsctl.8.txt) the Bridge 
> IPFIX parameters can be passed as follows:
>
> ovs-vsctl -- set Bridge br0 ipfix=@i \
>   --  --id=@i  create  IPFIX targets=\"192.168.0.34:4739\" obs_do‐
>   main_id=123   obs_point_id=456   cache_active_timeout=60
>   cache_max_flows=13 \
>   other_config:enable-input-sampling=false \
>   other_config:enable-output-sampling=false
>
> where the default values are:
> enable_input_sampling: true
> enable_output_sampling: true
>
> But in the existing code 
> https://github.com/openvswitch/ovs/blob/master/vswitchd/bridge.c#L1563-L1567, 
> these 2 parameters take up unexpected values in some scenarios.
>
> be_opts.enable_input_sampling = !smap_get_bool(_cfg->other_config,
>   "enable-input-sampling", false);
>
> be_opts.enable_output_sampling = !smap_get_bool(_cfg->other_config,
>   "enable-output-sampling", 
> false);
>
> Here, the function smap_get_bool is being used with a negation.
>
> smap_get_bool is defined as below:
> (https://github.com/openvswitch/ovs/blob/master/lib/smap.c#L220-L232)
>
> /* Gets the value associated with 'key' in 'smap' and converts it to a 
> boolean.
>  * If 'key' is not in 'smap', or its value is neither "true" nor "false",
>  * returns 'def'. */
> bool
> smap_get_bool(const struct smap *smap, const char *key, bool def)
> {
> const char *value = smap_get_def(smap, key, "");
> if (def) {
> return strcasecmp("false", value) != 0;
> } else {
> return !strcasecmp("true", value);
> }
> }
>
> This returns expected values for the default case (since the above code will 
> negate “false” we get from smap_get bool function and return the value 
> “true”) but unexpected values for the case where the sampling value is passed 
> through the CLI.
> For example, if we pass "true" for other_config:enable-input-sampling in the 
> CLI, the above code will negate the “true” value we get from the smap_bool 
> function and return the value “false”. Same would be the case for 
> enable_output_sampling

Hi once again,

This is even more confusing, you have sent a v1 than a v2 (which had some 
formatting problems) and now you sent a v1 again. Maybe you can send a v3, so 
it’s clear to everyone which version is the latest?

Thanks,

Eelco

> Signed-off-by: Sayali Naval \(sanaval\) 
> ---
> [bridge-ipfix-fix 277b3baae] Fix values for enable_input_sampling & 
> enable_ouput_sampling
>  Date: Thu May 25 10:32:43 2023 -0700
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index f5dc59ad0..b972d55d0 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1560,11 +1560,11 @@ bridge_configure_ipfix(struct bridge *br)
>  be_opts.enable_tunnel_sampling = smap_get_bool(_cfg->other_config,
>   "enable-tunnel-sampling", true);
>
> -be_opts.enable_input_sampling = !smap_get_bool(_cfg->other_config,
> -  "enable-input-sampling", 
> false);
> +be_opts.enable_input_sampling = smap_get_bool(_cfg->other_config,
> +  "enable-input-sampling", true);
>
> -be_opts.enable_output_sampling = 
> !smap_get_bool(_cfg->other_config,
> -  "enable-output-sampling", 
> false);
> +be_opts.enable_output_sampling = smap_get_bool(_cfg->other_config,
> +  "enable-output-sampling", 
> true);
>
>  virtual_obs_id = smap_get(_cfg->other_config, "virtual_obs_id");
>  be_opts.virtual_obs_id = nullable_xstrdup(virtual_obs_id);
> --
> ___
> 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

[ovs-dev] [PATCH v1] bridge ovs-vsctl Bridge IPFIX enable_input_sampling, enable_ouput_sampling fix unexpected values

[Sayali Naval (sanaval) via dev]

As per the Open vSwitch Manual 
(http://www.openvswitch.org/support/dist-docs/ovs-vsctl.8.txt) the Bridge IPFIX 
parameters can be passed as follows:

ovs-vsctl -- set Bridge br0 ipfix=@i \
  --  --id=@i  create  IPFIX targets=\"192.168.0.34:4739\" obs_do‐
  main_id=123   obs_point_id=456   cache_active_timeout=60
  cache_max_flows=13 \
  other_config:enable-input-sampling=false \
  other_config:enable-output-sampling=false

where the default values are:
enable_input_sampling: true
enable_output_sampling: true

But in the existing code 
https://github.com/openvswitch/ovs/blob/master/vswitchd/bridge.c#L1563-L1567, 
these 2 parameters take up unexpected values in some scenarios.

be_opts.enable_input_sampling = !smap_get_bool(_cfg->other_config,
  "enable-input-sampling", false);

be_opts.enable_output_sampling = !smap_get_bool(_cfg->other_config,
  "enable-output-sampling", false);

Here, the function smap_get_bool is being used with a negation.

smap_get_bool is defined as below:
(https://github.com/openvswitch/ovs/blob/master/lib/smap.c#L220-L232)

/* Gets the value associated with 'key' in 'smap' and converts it to a boolean.
 * If 'key' is not in 'smap', or its value is neither "true" nor "false",
 * returns 'def'. */
bool
smap_get_bool(const struct smap *smap, const char *key, bool def)
{
const char *value = smap_get_def(smap, key, "");
if (def) {
return strcasecmp("false", value) != 0;
} else {
return !strcasecmp("true", value);
}
}

This returns expected values for the default case (since the above code will 
negate “false” we get from smap_get bool function and return the value “true”) 
but unexpected values for the case where the sampling value is passed through 
the CLI.
For example, if we pass "true" for other_config:enable-input-sampling in the 
CLI, the above code will negate the “true” value we get from the smap_bool 
function and return the value “false”. Same would be the case for 
enable_output_sampling.

Signed-off-by: Sayali Naval \(sanaval\) 
---
[bridge-ipfix-fix 277b3baae] Fix values for enable_input_sampling & 
enable_ouput_sampling
 Date: Thu May 25 10:32:43 2023 -0700
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index f5dc59ad0..b972d55d0 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1560,11 +1560,11 @@ bridge_configure_ipfix(struct bridge *br)
 be_opts.enable_tunnel_sampling = smap_get_bool(_cfg->other_config,
  "enable-tunnel-sampling", true);

-be_opts.enable_input_sampling = !smap_get_bool(_cfg->other_config,
-  "enable-input-sampling", false);
+be_opts.enable_input_sampling = smap_get_bool(_cfg->other_config,
+  "enable-input-sampling", true);

-be_opts.enable_output_sampling = !smap_get_bool(_cfg->other_config,
-  "enable-output-sampling", false);
+be_opts.enable_output_sampling = smap_get_bool(_cfg->other_config,
+  "enable-output-sampling", true);

 virtual_obs_id = smap_get(_cfg->other_config, "virtual_obs_id");
 be_opts.virtual_obs_id = nullable_xstrdup(virtual_obs_id);
--
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] bridge ovs-vsctl Bridge IPFIX enable_input_sampling, enable_ouput_sampling fix unexpected values

[Eelco Chaudron]

On 4 Jul 2023, at 3:39, Sayali Naval (sanaval) via dev wrote:

> As per the Open vSwitch Manual 
> (http://www.openvswitch.org/support/dist-docs/ovs-vsctl.8.txt) the Bridge 
> IPFIX parameters can be passed as follows:
>
> ovs-vsctl -- set Bridge br0 ipfix=@i \
>   --  --id=@i  create  IPFIX targets=\"192.168.0.34:4739\" obs_do‐
>   main_id=123   obs_point_id=456   cache_active_timeout=60
>   cache_max_flows=13 \
>   other_config:enable-input-sampling=false \
>   other_config:enable-output-sampling=false
>
> where the default values are:
> enable_input_sampling: true
> enable_output_sampling: true
>
> But in the existing code 
> https://github.com/openvswitch/ovs/blob/master/vswitchd/bridge.c#L1563-L1567, 
> these 2 parameters take up unexpected values in some scenarios.
>
> be_opts.enable_input_sampling = !smap_get_bool(_cfg->other_config,
>   "enable-input-sampling", false);
>
> be_opts.enable_output_sampling = !smap_get_bool(_cfg->other_config,
>   "enable-output-sampling", 
> false);
>
> Here, the function smap_get_bool is being used with a negation.
>
> smap_get_bool is defined as below:
> (https://github.com/openvswitch/ovs/blob/master/lib/smap.c#L220-L232)
>
> /* Gets the value associated with 'key' in 'smap' and converts it to a 
> boolean.
>  * If 'key' is not in 'smap', or its value is neither "true" nor "false",
>  * returns 'def'. */
> bool
> smap_get_bool(const struct smap *smap, const char *key, bool def)
> {
> const char *value = smap_get_def(smap, key, "");
> if (def) {
> return strcasecmp("false", value) != 0;
> } else {
> return !strcasecmp("true", value);
> }
> }
>
> This returns expected values for the default case (since the above code will 
> negate “false” we get from smap_get bool function and return the value 
> “true”) but unexpected values for the case where the sampling value is passed 
> through the CLI.
> For example, if we pass "true" for other_config:enable-input-sampling in the 
> CLI, the above code will negate the “true” value we get from the smap_bool 
> function and return the value “false”. Same would be the case for 
> enable_output_sampling.
>
> Signed-off-by: Sayali Naval \(sanaval\) 

Hi,

Thanks for your patch, however, your V2, has two patches as I guess you replied 
manually to your previous patch.

In addition, you should also include the version history of your patches. This 
is an example of a patch including version information (notice the location); 
https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/405204.html

Can you resent the patch as a new thread (not a reply to a previous version), 
with only the content of the actual patch?

Thanks,

Eelco


> ---
> [bridge-ipfix-fix 277b3baae] Fix values for enable_input_sampling & 
> enable_ouput_sampling
>  Date: Thu May 25 10:32:43 2023 -0700
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index f5dc59ad0..b972d55d0 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1560,11 +1560,11 @@ bridge_configure_ipfix(struct bridge *br)
>  be_opts.enable_tunnel_sampling = smap_get_bool(_cfg->other_config,
>   "enable-tunnel-sampling", true);
>
> -be_opts.enable_input_sampling = !smap_get_bool(_cfg->other_config,
> -  "enable-input-sampling", 
> false);
> +be_opts.enable_input_sampling = smap_get_bool(_cfg->other_config,
> +  "enable-input-sampling", true);
>
> -be_opts.enable_output_sampling = 
> !smap_get_bool(_cfg->other_config,
> -  "enable-output-sampling", 
> false);
> +be_opts.enable_output_sampling = smap_get_bool(_cfg->other_config,
> +  "enable-output-sampling", 
> true);
>
>  virtual_obs_id = smap_get(_cfg->other_config, "virtual_obs_id");
>  be_opts.virtual_obs_id = nullable_xstrdup(virtual_obs_id);
> --
> 
> From: dev  on behalf of Sayali Naval 
> (sanaval) via dev 
> Sent: Monday, July 3, 2023 4:51 PM
> To: d...@openvswitch.org 
> Subject: [ovs-dev] [PATCH v1] bridge ovs-vsctl Bridge IPFIX 
> enable_input_sampling, enable_ouput_sampling fix unexpected values
>
> As per the Open vSwitch Manual 
> (http://www.openvswitch.org/support/dist-docs/ovs-vsctl.8.txt) the Bridge 
> IPFIX parameters can be passed as follows:
>
> ovs-vsctl -- set Bridge br0 ipfix=@i \
>   --  --id=@i  create  IPFIX targets=\"192.168.0.34:4739\" obs_do‐
>   main_id=123   obs_point_id=456   cache_active_timeout=60
>   cache_max_flows=13 \
>   other_config:enable-input-sampling=false \
>   

Re: [ovs-dev] [PATCH v6 9/9] system-offloads-traffic.at: Add vxlan gbp offload test

[Roi Dayan via dev]

On 29/06/2023 11:50, Eelco Chaudron wrote:
> 
> 
> On 27 Jun 2023, at 12:48, Roi Dayan wrote:
> 
>> From: Gavin Li 
>>
>> Add a vxlan gbp offload test case:
>>
>>   vxlan offloads with gbp extention - ping between two ports - offloads
>> enabled ok
>>
>> Signed-off-by: Gavin Li 
>> Reviewed-by: Roi Dayan 
>> Reviewed-by: Simon Horman 
> 
> Some small comments below, let me know if you agree with the suggested 
> changes. If so, I can roll them in when applying the patch.
> 
> Cheers,
> 
> Eelco
> 
> 
>> ---
>>  tests/system-offloads-traffic.at | 50 
>>  1 file changed, 50 insertions(+)
>>
>> diff --git a/tests/system-offloads-traffic.at 
>> b/tests/system-offloads-traffic.at
>> index ae302a29499a..8829389c0414 100644
>> --- a/tests/system-offloads-traffic.at
>> +++ b/tests/system-offloads-traffic.at
>> @@ -805,3 +805,53 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/could not open network 
>> device ovs-p0/d
>>  /failed to offload flow/d
>>  "])
>>  AT_CLEANUP
>> +
>> +AT_SETUP([offloads - ping over vxlan tunnel with gbp - offloads enabled])
>> +OVS_CHECK_TUNNEL_TSO()
>> +OVS_CHECK_VXLAN()
>> +OVS_CHECK_MIN_KERNEL(5, 5)
> 
> I’m not a big fan of kernel version checks, as kernels from various 
> distributions might vary.
> How about the following change, i.e. checking for the probe log message:
> 
> -OVS_CHECK_MIN_KERNEL(5, 5)
> 
>  OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
> other_config:hw-offload=true])
> +AT_SKIP_IF([! grep -q "probe tc: vxlan gbp is supported." ovs-vswitchd.log])
> 

sure this is better. we just wanted to skip the test on older kernels. thanks.

>> +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
>> other_config:hw-offload=true])
>> +ADD_BR([br-underlay])
>> +
>> +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
>> +
>> +ADD_NAMESPACES(at_ns0)
>> +
>> +dnl Set up underlay link from host into the namespace using veth pair.
>> +ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
>> +AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
>> +AT_CHECK([ip link set dev br-underlay up])
>> +
>> +dnl Set up tunnel endpoints on OVS outside the namespace and with a native.
> 
> You added a dot here in v6, but the sentence continues on the next line, so 
> we should remove the dot here.

ack

> 
>> +dnl linux device inside the namespace.
>> +ADD_OVS_TUNNEL([vxlan], [br0], [at_vxlan0], [172.31.1.1], [10.1.1.100/24], 
>> [options:exts=gbp])
>> +AT_CHECK([ovs-ofctl add-flow br0 "in_port=br0 
>> actions=load:0x200->NXM_NX_TUN_GBP_ID[], output:at_vxlan0]")
>> +AT_CHECK([ovs-ofctl add-flow br0 "in_port=at_vxlan0, tun_gbp_id=512 
>> actions=output:br0"])
>> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>> +
>> +ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.100], 
>> [10.1.1.1/24],
>> +  [id 0 dstport 4789 gbp])
>> +NS_CHECK_EXEC([at_ns0], [iptables -I OUTPUT -p ip -j MARK --set-mark 512 
>> 2>/dev/null], [0])
>> +NS_CHECK_EXEC([at_ns0], [iptables -I INPUT -m mark --mark 512 -j ACCEPT 
>> 2>/dev/null], [0], [ignore])
>> +
>> +dnl First, check the underlay.
>> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | 
>> FORMAT_PING], [0], [dnl
>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> +])
>> +
>> +dnl Okay, now check the overlay.
>> +NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 10.1.1.100 | FORMAT_PING], 
>> [0], [dnl
>> +1000 packets transmitted, 1000 received, 0% packet loss, time 0ms
>> +])
>> +
>> +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep 
>> "eth_type(0x0800)" | grep "tp_dst=4789,vxlan(gbp(id=512))" | wc -l], [0], 
>> [dnl
>> +1
>> +])
>> +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep 
>> "eth_type(0x0800)" | grep "tp_dst=4789,vxlan(gbp(id=512,flags=0))" | wc -l], 
>> [0], [dnl
>> +1
>> +])
>> +
>> +OVS_TRAFFIC_VSWITCHD_STOP
>> +AT_CLEANUP
>> +
>> -- 
>> 2.38.0
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v6 7/9] tc: Add vxlan encap action with gbp option offload

[Roi Dayan via dev]

On 29/06/2023 11:50, Eelco Chaudron wrote:
> 
> 
> On 27 Jun 2023, at 12:48, Roi Dayan wrote:
> 
>> From: Gavin Li 
>>
>> Add TC offload support for vxlan encap with gbp option
>>
>> Signed-off-by: Gavin Li 
>> Reviewed-by: Gavi Teitz 
>> Reviewed-by: Roi Dayan 
>> Reviewed-by: Simon Horman 
>> Acked-by: Eelco Chaudron 
>> ---
> 
> I already ACKed the v5, but Ilya mentioned we should probably add a NEWS 
> section.
> 
> What about I add the following when merging?
> 
> diff --git a/NEWS b/NEWS
> index 66d5a4ea3..bd21f3aaf 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -40,6 +40,8 @@ Post-v3.1.0
>   * IP and L4 checksum offload support is now enabled by default for
> interfaces that support it.  See the 'status' column in the 
> 'interface'
> table to check the status.
> +   - Linux TC offload:
> + * Add support for offloading VXLAN tunnels with the GBP extensions.
> 
> 

skipped it somehow. you can add it. thanks!

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