Re: [ovs-dev] [PATCH ovn v2 06/18] northd: Add a new engine 'lr-lb-nat-data' to manage routers' lb data.

2023-11-23 Thread Dumitru Ceara
On 11/23/23 21:45, Dumitru Ceara wrote:
> On 10/26/23 20:15, num...@ovn.org wrote:
>> From: Numan Siddique 
>>
>> This new engine now maintains the load balancer and NAT data of a
>> logical router which was earlier part of northd engine node data.
>> The main inputs to this engine are:
>>- northd node
>>- lr-nat node
>>
>> A record for each logical router is maintained in the 'lr_lb_nats'
>> hmap table and this record
>>- stores the lb related data
>>- embeds the 'lr-nat' record.
>>
>> This engine node becomes an input to 'lflow' node.
>>
>> Signed-off-by: Numan Siddique 
>> ---
>>  lib/stopwatch-names.h  |   1 +
>>  northd/automake.mk |   2 +
>>  northd/en-lflow.c  |   4 +
>>  northd/en-lr-lb-nat-data.c | 654 +
>>  northd/en-lr-lb-nat-data.h |  93 ++
>>  northd/en-lr-nat.h |   3 +
>>  northd/en-sync-sb.c|  50 +--
>>  northd/inc-proc-northd.c   |  13 +-
>>  northd/northd.c| 640 
>>  northd/northd.h| 137 +++-
>>  tests/ovn-northd.at| 110 +--
>>  11 files changed, 1212 insertions(+), 495 deletions(-)
>>  create mode 100644 northd/en-lr-lb-nat-data.c
>>  create mode 100644 northd/en-lr-lb-nat-data.h
>>
>> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
>> index 0a16da211e..7d85acdaea 100644
>> --- a/lib/stopwatch-names.h
>> +++ b/lib/stopwatch-names.h
>> @@ -33,5 +33,6 @@
>>  #define PORT_GROUP_RUN_STOPWATCH_NAME "port_group_run"
>>  #define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
>>  #define LR_NAT_RUN_STOPWATCH_NAME "lr_nat_run"
>> +#define LR_LB_NAT_DATA_RUN_STOPWATCH_NAME "lr_lb_nat_data"
>>  
>>  #endif
>> diff --git a/northd/automake.mk b/northd/automake.mk
>> index ae367a2a8b..4116c487df 100644
>> --- a/northd/automake.mk
>> +++ b/northd/automake.mk
>> @@ -26,6 +26,8 @@ northd_ovn_northd_SOURCES = \
>>  northd/en-lb-data.h \
>>  northd/en-lr-nat.c \
>>  northd/en-lr-nat.h \
>> +northd/en-lr-lb-nat-data.c \
>> +northd/en-lr-lb-nat-data.h \
>>  northd/inc-proc-northd.c \
>>  northd/inc-proc-northd.h \
>>  northd/ipam.c \
>> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
>> index 22f398d419..9cb0ead3f0 100644
>> --- a/northd/en-lflow.c
>> +++ b/northd/en-lflow.c
>> @@ -20,6 +20,7 @@
>>  
>>  #include "en-lflow.h"
>>  #include "en-lr-nat.h"
>> +#include "en-lr-lb-nat-data.h"
>>  #include "en-northd.h"
>>  #include "en-meters.h"
>>  
>> @@ -43,6 +44,8 @@ lflow_get_input_data(struct engine_node *node,
>>  engine_get_input_data("sync_meters", node);
>>  struct ed_type_lr_nat_data *lr_nat_data =
>>  engine_get_input_data("lr_nat", node);
>> +struct ed_type_lr_lb_nat_data *lr_lb_nat_data =
>> +engine_get_input_data("lr_lb_nat_data", node);
>>  
>>  lflow_input->nbrec_bfd_table =
>>  EN_OVSDB_GET(engine_get_input("NB_bfd", node));
>> @@ -66,6 +69,7 @@ lflow_get_input_data(struct engine_node *node,
>>  lflow_input->lr_ports = _data->lr_ports;
>>  lflow_input->ls_port_groups = _data->ls_port_groups;
>>  lflow_input->lr_nats = _nat_data->lr_nats;
>> +lflow_input->lr_lbnats = _lb_nat_data->lr_lbnats;
>>  lflow_input->meter_groups = _meters_data->meter_groups;
>>  lflow_input->lb_datapaths_map = _data->lb_datapaths_map;
>>  lflow_input->svc_monitor_map = _data->svc_monitor_map;
>> diff --git a/northd/en-lr-lb-nat-data.c b/northd/en-lr-lb-nat-data.c
>> new file mode 100644
>> index 00..19b638ce0b
>> --- /dev/null
>> +++ b/northd/en-lr-lb-nat-data.c
>> @@ -0,0 +1,654 @@
>> +/*
>> + * Copyright (c) 2023, Red Hat, Inc.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at:
>> + *
>> + * http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/* OVS includes */
>> +#include "include/openvswitch/hmap.h"
>> +#include "lib/bitmap.h"
>> +#include "lib/socket-util.h"
>> +#include "lib/uuidset.h"
>> +#include "openvswitch/util.h"
>> +#include "openvswitch/vlog.h"
>> +#include "stopwatch.h"
>> +
>> +/* OVN includes */
>> +#include "en-lb-data.h"
>> +#include "en-lr-lb-nat-data.h"
>> +#include "en-lr-nat.h"
>> +#include "lib/inc-proc-eng.h"
>> +#include "lib/lb.h"
>> +#include "lib/ovn-nb-idl.h"
>> +#include "lib/ovn-sb-idl.h"
>> +#include "lib/ovn-util.h"
>> +#include "lib/stopwatch-names.h"
>> +#include "northd.h"
>> +
>> +VLOG_DEFINE_THIS_MODULE(en_lr_lb_nat_data);
>> +
>> 

Re: [ovs-dev] [PATCH ovn v2 08/18] northd: Don't commit dhcp response flows in the conntrack.

2023-11-23 Thread Dumitru Ceara
On 11/15/23 07:45, Han Zhou wrote:
> On Thu, Oct 26, 2023 at 11:16 AM  wrote:
>>
>> From: Numan Siddique 
>>
>> This is not required.
>>
> Thanks Numan for the fix. Could you provide a little more detail why this
> is not required:
> - Is it a bug fix? What's the impact?
> - Shall we update the ovn-northd documentation for the related lflow
> changes?

I agree with Han, the commit log could be improved.  I think the
ovn-northd documentation was already behind wrt this flow.  There was no
explicit mention of it anywhere.

I'm pretty sure it's safe to not commit dhcp responses in conntrack, we
bypass conntrack for the requests anyway:

https://github.com/ovn-org/ovn/blob/5ef2a08ade01d698f84e197987ea679d8978d2b9/northd/northd.c#L7215-L7224

> - Is there a reason this is in the I-P patch series?

Guessing again, I'm still reviewing the rest of the series, but probably
to avoid an unnecessary dependency on logical switch data (if switches
have stateful ACLs applied).

Regards,
Dumitru

> 
> Thanks,
> Han
> 
>> Signed-off-by: Numan Siddique 
>> ---
>>  northd/northd.c | 8 ++--
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 1877cbc7df..c8a224d3cd 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -9223,9 +9223,7 @@ build_dhcpv4_options_flows(struct ovn_port *op,
>>  >nbsp->dhcpv4_options->options, "lease_time");
>>  ovs_assert(server_id && server_mac && lease_time);
>>  const char *dhcp_actions =
>> -(op->od->has_stateful_acl || op->od->has_lb_vip)
>> - ? REGBIT_ACL_VERDICT_ALLOW" = 1; ct_commit; next;"
>> - : REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
>> +REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
>>  ds_clear();
>>  ds_put_format(, "outport == %s && eth.src == %s "
>>"&& ip4.src == %s && udp && udp.src == 67 "
>> @@ -9308,9 +9306,7 @@ build_dhcpv6_options_flows(struct ovn_port *op,
>>  ipv6_string_mapped(server_ip, );
>>
>>  const char *dhcp6_actions =
>> -(op->od->has_stateful_acl || op->od->has_lb_vip)
>> -? REGBIT_ACL_VERDICT_ALLOW" = 1; ct_commit;
> next;"
>> -: REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
>> +REGBIT_ACL_VERDICT_ALLOW" = 1; next;";
>>  ds_clear();
>>  ds_put_format(, "outport == %s && eth.src == %s "
>>"&& ip6.src == %s && udp && udp.src == 547
> "
>> --
>> 2.41.0
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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


Re: [ovs-dev] [PATCH ovn v2 07/18] northd: Generate logical router's LB and NAT flows using lr_lbnat_data.

2023-11-23 Thread Dumitru Ceara
On 10/26/23 20:15, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> Previous commits added new engine nodes to store logical router's lb
> and NAT data.  Make use of the data stored by these engine nodes
> to generate logical flows related to router's LBs and NATs.
> 
> Signed-off-by: Numan Siddique 
> ---
>  northd/en-lflow.c  |   3 -
>  northd/en-lr-lb-nat-data.h |   4 +
>  northd/inc-proc-northd.c   |   1 -
>  northd/northd.c| 752 -
>  northd/northd.h|   1 -
>  5 files changed, 496 insertions(+), 265 deletions(-)
> 
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 9cb0ead3f0..229f4be1d0 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -42,8 +42,6 @@ lflow_get_input_data(struct engine_node *node,
>  engine_get_input_data("port_group", node);
>  struct sync_meters_data *sync_meters_data =
>  engine_get_input_data("sync_meters", node);
> -struct ed_type_lr_nat_data *lr_nat_data =
> -engine_get_input_data("lr_nat", node);
>  struct ed_type_lr_lb_nat_data *lr_lb_nat_data =
>  engine_get_input_data("lr_lb_nat_data", node);
>  
> @@ -68,7 +66,6 @@ lflow_get_input_data(struct engine_node *node,
>  lflow_input->ls_ports = _data->ls_ports;
>  lflow_input->lr_ports = _data->lr_ports;
>  lflow_input->ls_port_groups = _data->ls_port_groups;
> -lflow_input->lr_nats = _nat_data->lr_nats;
>  lflow_input->lr_lbnats = _lb_nat_data->lr_lbnats;
>  lflow_input->meter_groups = _meters_data->meter_groups;
>  lflow_input->lb_datapaths_map = _data->lb_datapaths_map;
> diff --git a/northd/en-lr-lb-nat-data.h b/northd/en-lr-lb-nat-data.h
> index 9029aee339..ffe41cad73 100644
> --- a/northd/en-lr-lb-nat-data.h
> +++ b/northd/en-lr-lb-nat-data.h
> @@ -56,6 +56,10 @@ struct lr_lb_nat_data_table {
>  #define LR_LB_NAT_DATA_TABLE_FOR_EACH(LR_LB_NAT_REC, TABLE) \
>  HMAP_FOR_EACH (LR_LB_NAT_REC, key_node, &(TABLE)->entries)
>  
> +#define LR_LB_NAT_DATA_TABLE_FOR_EACH_IN_P(LR_LB_NAT_REC, JOBID, TABLE) \
> +HMAP_FOR_EACH_IN_PARALLEL (LR_LB_NAT_REC, key_node, JOBID, \
> +   &(TABLE)->entries)
> +
>  struct lr_lb_nat_data_tracked_data {
>  /* Created or updated logical router with LB data. */
>  struct hmapx crupdated; /* Stores 'struct lr_lb_nat_data_record'. */
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index 369a151fa3..84627070a8 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -228,7 +228,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>  engine_add_input(_lflow, _sb_igmp_group, NULL);
>  engine_add_input(_lflow, _northd, lflow_northd_handler);
>  engine_add_input(_lflow, _port_group, lflow_port_group_handler);
> -engine_add_input(_lflow, _lr_nat, NULL);

Was this supposed to go in the previous patch, the one that adds
en_lr_lb_nat_data?

>  engine_add_input(_lflow, _lr_lb_nat_data, NULL);
>  
>  engine_add_input(_sync_to_sb_addr_set, _nb_address_set,
> diff --git a/northd/northd.c b/northd/northd.c
> index 24df14c0de..1877cbc7df 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -8854,18 +8854,14 @@ build_lrouter_groups(struct hmap *lr_ports, struct 
> ovs_list *lr_list)
>   */
>  static void
>  build_lswitch_rport_arp_req_self_orig_flow(struct ovn_port *op,
> -   uint32_t priority,
> -   struct ovn_datapath *od,
> -   const struct lr_nat_table 
> *lr_nats,
> -   struct hmap *lflows)
> +uint32_t priority,
> +const struct ovn_datapath *od,
> +const struct lr_nat_record 
> *lrnat_rec,
> +struct hmap *lflows)


Nit: indentation.

>  {
>  struct ds eth_src = DS_EMPTY_INITIALIZER;
>  struct ds match = DS_EMPTY_INITIALIZER;
>  
> -const struct lr_nat_record *lrnat_rec = lr_nat_table_find_by_index(
> -lr_nats, op->od->index);
> -ovs_assert(lrnat_rec);
> -
>  /* Self originated ARP requests/RARP/ND need to be flooded to the L2 
> domain
>   * (except on router ports).  Determine that packets are self originated
>   * by also matching on source MAC. Matching on ingress port is not
> @@ -8952,7 +8948,8 @@ lrouter_port_ipv6_reachable(const struct ovn_port *op,
>   */
>  static void
>  build_lswitch_rport_arp_req_flow(const char *ips,
> -int addr_family, struct ovn_port *patch_op, struct ovn_datapath *od,
> +int addr_family, struct ovn_port *patch_op,
> +const struct ovn_datapath *od,
>  uint32_t priority, struct hmap *lflows,
>  const struct ovsdb_idl_row *stage_hint)
>  {
> @@ -8993,8 +8990,6 @@ static void
>  build_lswitch_rport_arp_req_flows(struct ovn_port *op,

Re: [ovs-dev] [PATCH ovn v2 06/18] northd: Add a new engine 'lr-lb-nat-data' to manage routers' lb data.

2023-11-23 Thread Dumitru Ceara
On 10/26/23 20:15, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> This new engine now maintains the load balancer and NAT data of a
> logical router which was earlier part of northd engine node data.
> The main inputs to this engine are:
>- northd node
>- lr-nat node
> 
> A record for each logical router is maintained in the 'lr_lb_nats'
> hmap table and this record
>- stores the lb related data
>- embeds the 'lr-nat' record.
> 
> This engine node becomes an input to 'lflow' node.
> 
> Signed-off-by: Numan Siddique 
> ---
>  lib/stopwatch-names.h  |   1 +
>  northd/automake.mk |   2 +
>  northd/en-lflow.c  |   4 +
>  northd/en-lr-lb-nat-data.c | 654 +
>  northd/en-lr-lb-nat-data.h |  93 ++
>  northd/en-lr-nat.h |   3 +
>  northd/en-sync-sb.c|  50 +--
>  northd/inc-proc-northd.c   |  13 +-
>  northd/northd.c| 640 
>  northd/northd.h| 137 +++-
>  tests/ovn-northd.at| 110 +--
>  11 files changed, 1212 insertions(+), 495 deletions(-)
>  create mode 100644 northd/en-lr-lb-nat-data.c
>  create mode 100644 northd/en-lr-lb-nat-data.h
> 
> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
> index 0a16da211e..7d85acdaea 100644
> --- a/lib/stopwatch-names.h
> +++ b/lib/stopwatch-names.h
> @@ -33,5 +33,6 @@
>  #define PORT_GROUP_RUN_STOPWATCH_NAME "port_group_run"
>  #define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
>  #define LR_NAT_RUN_STOPWATCH_NAME "lr_nat_run"
> +#define LR_LB_NAT_DATA_RUN_STOPWATCH_NAME "lr_lb_nat_data"
>  
>  #endif
> diff --git a/northd/automake.mk b/northd/automake.mk
> index ae367a2a8b..4116c487df 100644
> --- a/northd/automake.mk
> +++ b/northd/automake.mk
> @@ -26,6 +26,8 @@ northd_ovn_northd_SOURCES = \
>   northd/en-lb-data.h \
>   northd/en-lr-nat.c \
>   northd/en-lr-nat.h \
> + northd/en-lr-lb-nat-data.c \
> + northd/en-lr-lb-nat-data.h \
>   northd/inc-proc-northd.c \
>   northd/inc-proc-northd.h \
>   northd/ipam.c \
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 22f398d419..9cb0ead3f0 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -20,6 +20,7 @@
>  
>  #include "en-lflow.h"
>  #include "en-lr-nat.h"
> +#include "en-lr-lb-nat-data.h"
>  #include "en-northd.h"
>  #include "en-meters.h"
>  
> @@ -43,6 +44,8 @@ lflow_get_input_data(struct engine_node *node,
>  engine_get_input_data("sync_meters", node);
>  struct ed_type_lr_nat_data *lr_nat_data =
>  engine_get_input_data("lr_nat", node);
> +struct ed_type_lr_lb_nat_data *lr_lb_nat_data =
> +engine_get_input_data("lr_lb_nat_data", node);
>  
>  lflow_input->nbrec_bfd_table =
>  EN_OVSDB_GET(engine_get_input("NB_bfd", node));
> @@ -66,6 +69,7 @@ lflow_get_input_data(struct engine_node *node,
>  lflow_input->lr_ports = _data->lr_ports;
>  lflow_input->ls_port_groups = _data->ls_port_groups;
>  lflow_input->lr_nats = _nat_data->lr_nats;
> +lflow_input->lr_lbnats = _lb_nat_data->lr_lbnats;
>  lflow_input->meter_groups = _meters_data->meter_groups;
>  lflow_input->lb_datapaths_map = _data->lb_datapaths_map;
>  lflow_input->svc_monitor_map = _data->svc_monitor_map;
> diff --git a/northd/en-lr-lb-nat-data.c b/northd/en-lr-lb-nat-data.c
> new file mode 100644
> index 00..19b638ce0b
> --- /dev/null
> +++ b/northd/en-lr-lb-nat-data.c
> @@ -0,0 +1,654 @@
> +/*
> + * Copyright (c) 2023, Red Hat, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +/* OVS includes */
> +#include "include/openvswitch/hmap.h"
> +#include "lib/bitmap.h"
> +#include "lib/socket-util.h"
> +#include "lib/uuidset.h"
> +#include "openvswitch/util.h"
> +#include "openvswitch/vlog.h"
> +#include "stopwatch.h"
> +
> +/* OVN includes */
> +#include "en-lb-data.h"
> +#include "en-lr-lb-nat-data.h"
> +#include "en-lr-nat.h"
> +#include "lib/inc-proc-eng.h"
> +#include "lib/lb.h"
> +#include "lib/ovn-nb-idl.h"
> +#include "lib/ovn-sb-idl.h"
> +#include "lib/ovn-util.h"
> +#include "lib/stopwatch-names.h"
> +#include "northd.h"
> +
> +VLOG_DEFINE_THIS_MODULE(en_lr_lb_nat_data);
> +
> +/* Static function declarations. */
> +static void lr_lb_nat_data_table_init(struct lr_lb_nat_data_table *);
> +static void lr_lb_nat_data_table_clear(struct 

Re: [ovs-dev] [PATCH ovn] pinctrl: Fix up comments about sending RST packets for healthcheck.

2023-11-23 Thread Ales Musil
On Mon, Nov 20, 2023 at 8:45 PM Dumitru Ceara  wrote:

> We're sending TCP RST not RST-ACK as the comment suggests.
>
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2023-November/409288.html
> Reported-by: Mark Michelson 
> Fixes: a35725a7a24b ("pinctrl: send RST instead of RST_ACK bit for lb hc")
> Signed-off-by: Dumitru Ceara 
> ---
>  controller/pinctrl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 62cf4da324..1b176490ea 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -7887,7 +7887,7 @@ pinctrl_handle_tcp_svc_check(struct rconn *swconn,
>  svc_mon->n_success++;
>  svc_mon->state = SVC_MON_S_ONLINE;
>
> -/* Send RST-ACK packet. */
> +/* Send RST packet. */
>  svc_monitor_send_tcp_health_check__(swconn, svc_mon, TCP_RST,
>  htonl(tcp_ack),
>  htonl(0), th->tcp_dst);
> --
> 2.39.3
>
> ___
> 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.com

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


Re: [ovs-dev] [PATCH ovn v2 05/18] northd: Add a new engine 'lr-nat' to manage lr NAT data.

2023-11-23 Thread Dumitru Ceara
On 11/17/23 23:05, Numan Siddique wrote:
> On Wed, Nov 15, 2023 at 1:27 AM Han Zhou  wrote:
>>
>> On Thu, Oct 26, 2023 at 11:15 AM  wrote:
>>>
>>> From: Numan Siddique 
>>>
>>> This new engine now maintains the NAT related data for each
>>> logical router which was earlier maintained by the northd
>>> engine node in the 'struct ovn_datapath'.  Main inputs to
>>> this engine node are:
>>>- northd
>>>- NB logical router
>>>
>>> A record for each logical router is maintained in the 'lr_nats'
>>> hmap table and this record
>>>   - stores the ovn_nat's
>>
>> It seems the sentence is incomplete.
>>
>>>
>>> Handlers are also added to handle the changes to both these
>>> inputs.  This engine node becomes an input to 'lflow' node.
>>> This essentially decouples the lr NAT data from the northd
>>> engine node.
>>>
>>> Signed-off-by: Numan Siddique 
>>> ---
>>>  lib/ovn-util.c   |   6 +-
>>>  lib/ovn-util.h   |   2 +-
>>>  lib/stopwatch-names.h|   1 +
>>>  northd/automake.mk   |   2 +
>>>  northd/en-lflow.c|   5 +
>>>  northd/en-lr-nat.c   | 498 +
>>>  northd/en-lr-nat.h   | 134 ++
>>>  northd/en-sync-sb.c  |  11 +-
>>>  northd/inc-proc-northd.c |   9 +
>>>  northd/northd.c  | 514 ++-
>>>  northd/northd.h  |  32 ++-
>>>  tests/ovn-northd.at  |  18 ++
>>>  12 files changed, 877 insertions(+), 355 deletions(-)
>>>  create mode 100644 northd/en-lr-nat.c
>>>  create mode 100644 northd/en-lr-nat.h

I would call these en-nat.{hc}, I think.  There's no NAT for switches.

>>>
>>> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
>>> index 33105202f2..05e635a6b4 100644
>>> --- a/lib/ovn-util.c
>>> +++ b/lib/ovn-util.c
>>> @@ -395,7 +395,7 @@ extract_sbrec_binding_first_mac(const struct
>> sbrec_port_binding *binding,
>>>  }
>>>
>>>  bool
>>> -lport_addresses_is_empty(struct lport_addresses *laddrs)
>>> +lport_addresses_is_empty(const struct lport_addresses *laddrs)
>>>  {
>>>  return !laddrs->n_ipv4_addrs && !laddrs->n_ipv6_addrs;
>>>  }
>>> @@ -405,6 +405,10 @@ destroy_lport_addresses(struct lport_addresses
>> *laddrs)
>>>  {
>>>  free(laddrs->ipv4_addrs);
>>>  free(laddrs->ipv6_addrs);
>>> +laddrs->ipv4_addrs = NULL;
>>> +laddrs->ipv6_addrs = NULL;
>>> +laddrs->n_ipv4_addrs = 0;
>>> +laddrs->n_ipv6_addrs = 0;
>>>  }
>>>
>>>  /* Returns a string of the IP address of 'laddrs' that overlaps with
>> 'ip_s'.
>>> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
>>> index bff50dbde9..5805415885 100644
>>> --- a/lib/ovn-util.h
>>> +++ b/lib/ovn-util.h
>>> @@ -112,7 +112,7 @@ bool extract_sbrec_binding_first_mac(const struct
>> sbrec_port_binding *binding,
>>>  bool extract_lrp_networks__(char *mac, char **networks, size_t
>> n_networks,
>>>  struct lport_addresses *laddrs);
>>>
>>> -bool lport_addresses_is_empty(struct lport_addresses *);
>>> +bool lport_addresses_is_empty(const struct lport_addresses *);
>>>  void destroy_lport_addresses(struct lport_addresses *);
>>>  const char *find_lport_address(const struct lport_addresses *laddrs,
>>> const char *ip_s);
>>> diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
>>> index 3452cc71cf..0a16da211e 100644
>>> --- a/lib/stopwatch-names.h
>>> +++ b/lib/stopwatch-names.h
>>> @@ -32,5 +32,6 @@
>>>  #define LFLOWS_TO_SB_STOPWATCH_NAME "lflows_to_sb"
>>>  #define PORT_GROUP_RUN_STOPWATCH_NAME "port_group_run"
>>>  #define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
>>> +#define LR_NAT_RUN_STOPWATCH_NAME "lr_nat_run"
>>>
>>>  #endif
>>> diff --git a/northd/automake.mk b/northd/automake.mk
>>> index cf622fc3c9..ae367a2a8b 100644
>>> --- a/northd/automake.mk
>>> +++ b/northd/automake.mk
>>> @@ -24,6 +24,8 @@ northd_ovn_northd_SOURCES = \
>>> northd/en-sync-from-sb.h \
>>> northd/en-lb-data.c \
>>> northd/en-lb-data.h \
>>> +   northd/en-lr-nat.c \
>>> +   northd/en-lr-nat.h \
>>> northd/inc-proc-northd.c \
>>> northd/inc-proc-northd.h \
>>> northd/ipam.c \
>>> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
>>> index 96d03b7ada..22f398d419 100644
>>> --- a/northd/en-lflow.c
>>> +++ b/northd/en-lflow.c
>>> @@ -19,6 +19,7 @@
>>>  #include 
>>>
>>>  #include "en-lflow.h"
>>> +#include "en-lr-nat.h"
>>>  #include "en-northd.h"
>>>  #include "en-meters.h"
>>>
>>> @@ -40,6 +41,9 @@ lflow_get_input_data(struct engine_node *node,
>>>  engine_get_input_data("port_group", node);
>>>  struct sync_meters_data *sync_meters_data =
>>>  engine_get_input_data("sync_meters", node);
>>> +struct ed_type_lr_nat_data *lr_nat_data =
>>> +engine_get_input_data("lr_nat", node);
>>> +
>>>  lflow_input->nbrec_bfd_table =
>>>  EN_OVSDB_GET(engine_get_input("NB_bfd", node));
>>>  lflow_input->sbrec_bfd_table =
>>> @@ -61,6 +65,7 @@ lflow_get_input_data(struct engine_node 

Re: [ovs-dev] [PATCH ovn v2 04/18] northd: Move router ports SB PB options sync to sync_to_sb_pb node.

2023-11-23 Thread Dumitru Ceara
On 10/26/23 20:14, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> It also moves the logical router port IPv6 prefix delegation
> updates to "sync-from-sb" engine node.
> 
> Signed-off-by: Numan Siddique 
> ---

I think I agree in general with the patch; I'd like to see the new
revision however because the check that Han also commented on [0] in
northd_handle_sb_port_binding_changes() makes me wonder too if we're not
missing cases (or if we should just relax the LSP check as well).

[0] https://mail.openvswitch.org/pipermail/ovs-dev/2023-November/409451.html

Aside from that I left a few minor comments below.

Thanks,
Dumitru

>  northd/en-northd.c  |   2 +-
>  northd/en-sync-sb.c |   3 +-
>  northd/northd.c | 283 ++--
>  northd/northd.h |   6 +-
>  tests/ovn-northd.at |  31 -
>  5 files changed, 198 insertions(+), 127 deletions(-)
> 
> diff --git a/northd/en-northd.c b/northd/en-northd.c
> index 96c2ce9f69..13e731cad9 100644
> --- a/northd/en-northd.c
> +++ b/northd/en-northd.c
> @@ -189,7 +189,7 @@ northd_sb_port_binding_handler(struct engine_node *node,
>  northd_get_input_data(node, _data);
>  
>  if (!northd_handle_sb_port_binding_changes(
> -input_data.sbrec_port_binding_table, >ls_ports)) {
> +input_data.sbrec_port_binding_table, >ls_ports, >lr_ports)) {
>  return false;
>  }
>  
> diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
> index 2540fcfb97..a14c609acd 100644
> --- a/northd/en-sync-sb.c
> +++ b/northd/en-sync-sb.c
> @@ -288,7 +288,8 @@ en_sync_to_sb_pb_run(struct engine_node *node, void *data 
> OVS_UNUSED)
>  const struct engine_context *eng_ctx = engine_get_context();
>  struct northd_data *northd_data = engine_get_input_data("northd", node);
>  
> -sync_pbs(eng_ctx->ovnsb_idl_txn, _data->ls_ports);
> +sync_pbs(eng_ctx->ovnsb_idl_txn, _data->ls_ports,
> + _data->lr_ports);
>  engine_set_node_state(node, EN_UPDATED);
>  }
>  
> diff --git a/northd/northd.c b/northd/northd.c
> index 9ce1b2cb5a..c9c7045755 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -3419,6 +3419,9 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
>  {
>  sbrec_port_binding_set_datapath(op->sb, op->od->sb);
>  if (op->nbrp) {
> +/* Note: SB port binding options for router ports are set in
> + * sync_pbs(). */
> +
>  /* If the router is for l3 gateway, it resides on a chassis
>   * and its port type is "l3gateway". */
>  const char *chassis_name = smap_get(>od->nbr->options, 
> "chassis");
> @@ -3430,15 +3433,11 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
>  sbrec_port_binding_set_type(op->sb, "patch");
>  }
>  
> -struct smap new;
> -smap_init();
>  if (is_cr_port(op)) {
>  ovs_assert(sbrec_chassis_by_name);
>  ovs_assert(sbrec_chassis_by_hostname);
>  ovs_assert(sbrec_ha_chassis_grp_by_name);
>  ovs_assert(active_ha_chassis_grps);
> -const char *redirect_type = smap_get(>nbrp->options,
> - "redirect-type");
>  
>  if (op->nbrp->ha_chassis_group) {
>  if (op->nbrp->n_gateway_chassis) {
> @@ -3480,49 +3479,8 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
>  /* Delete the legacy gateway_chassis from the pb. */
>  sbrec_port_binding_set_gateway_chassis(op->sb, NULL, 0);
>  }
> -smap_add(, "distributed-port", op->nbrp->name);
> -
> -bool always_redirect =
> -!op->od->has_distributed_nat &&
> -!l3dgw_port_has_associated_vtep_lports(op->l3dgw_port);
> -
> -if (redirect_type) {
> -smap_add(, "redirect-type", redirect_type);
> -/* XXX Why can't we enable always-redirect when redirect-type
> - * is bridged? */
> -if (!strcmp(redirect_type, "bridged")) {
> -always_redirect = false;
> -}
> -}
> -
> -if (always_redirect) {
> -smap_add(, "always-redirect", "true");
> -}
> -} else {
> -if (op->peer) {
> -smap_add(, "peer", op->peer->key);
> -if (op->nbrp->ha_chassis_group ||
> -op->nbrp->n_gateway_chassis) {
> -char *redirect_name =
> -ovn_chassis_redirect_name(op->nbrp->name);
> -smap_add(, "chassis-redirect-port", redirect_name);
> -free(redirect_name);
> -}
> -}
> -if (chassis_name) {
> -smap_add(, "l3gateway-chassis", chassis_name);
> -}
> -}
> -
> -const char *ipv6_pd_list = smap_get(>sb->options,
> -  

Re: [ovs-dev] [PATCH ovn v2 03/18] tests: Add a couple of tests in ovn-northd for I-P.

2023-11-23 Thread Dumitru Ceara
On 10/26/23 20:14, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> These tests cover scenarios for load balancers and NATs
> and check for the 'northd' and 'lflow' engine node
> recompute and compute stats.
> 
> Signed-off-by: Numan Siddique 
> ---
>  tests/ovn-northd.at | 274 
>  1 file changed, 274 insertions(+)
> 
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 28c293473c..699f6cfdce 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -10893,3 +10893,277 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>  
>  AT_CLEANUP
>  ])
> +
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([Load balancer incremental processing with stateless ACLs])
> +ovn_start
> +
> +check_engine_stats() {
> +  node=$1
> +  recompute=$2
> +  compute=$3
> +
> +  echo "__file__:__line__: Checking engine stats for node $node : recompute 
> - \
> +$recompute : compute - $compute"
> +
> +  node_stat=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats 
> $node)
> +  # node_stat will be of this format :
> +  # - Node: lflow - recompute: 3 - compute: 0 - abort: 0
> +  node_recompute_ct=$(echo $node_stat | cut -d '-' -f2 | cut -d ':' -f2)
> +  node_compute_ct=$(echo $node_stat | cut -d '-' -f3 | cut -d ':' -f2)
> +
> +  if [[ "$recompute" == "norecompute" ]]; then
> +# node should not be recomputed
> +echo "Expecting $node recompute count - $node_recompute_ct to be 0"
> +check test "$node_recompute_ct" -eq "0"
> +  else
> +echo "Expecting $node recompute count - $node_recompute_ct not to be 0"
> +check test "$node_recompute_ct" -ne "0"
> +  fi
> +
> +  if [[ "$compute" == "nocompute" ]]; then
> +# node should not be computed
> +echo "Expecting $node compute count - $node_compute_ct to be 0"
> +check test "$node_compute_ct" -eq "0"
> +  else
> +echo "Expecting $node compute count - $node_compute_ct not to be 0"
> +check test "$node_compute_ct" -ne "0"
> +  fi
> +}

I think there's no difference between the 3 definitions of
check_engine_stats() (this patch adds 2 of them, one already existed).

It's probably best to factor it out and have a single definition (maybe
in ovn-macros.at?).

> +
> +# Test I-P for load balancers.
> +# Presently ovn-northd handles I-P for NB LBs in northd_lb_data engine node
> +# only.
> +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lb-add lb1 10.0.0.10:80 10.0.0.3:80
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl acl-add sw0 from-lport 1 1 allow-stateless
> +check ovn-nbctl --wait=sb acl-add sw0 to-lport 1 1 allow-stateless
> +
> +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> +check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
> +check_engine_stats lb_data norecompute compute
> +check_engine_stats northd norecompute compute
> +check_engine_stats lflow recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
> +# Clear the VIPs of lb1
> +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> +check ovn-nbctl --wait=sb clear load_balancer . vips
> +check_engine_stats lb_data norecompute compute
> +check_engine_stats northd norecompute compute
> +check_engine_stats lflow recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
> +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lb-del lb1
> +check_engine_stats lb_data norecompute compute
> +check_engine_stats northd recompute nocompute
> +check_engine_stats lflow recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
> +AT_CLEANUP
> +])
> +
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([Logical router incremental processing for NAT])
> +
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.11
> +
> +check_engine_stats() {
> +  node=$1
> +  recompute=$2
> +  compute=$3
> +
> +  echo "__file__:__line__: Checking engine stats for node $node : recompute 
> - \
> +$recompute : compute - $compute"
> +
> +  node_stat=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats 
> $node)
> +  # node_stat will be of this format :
> +  # - Node: lflow - recompute: 3 - compute: 0 - abort: 0
> +  node_recompute_ct=$(echo $node_stat | cut -d '-' -f2 | cut -d ':' -f2)
> +  node_compute_ct=$(echo $node_stat | cut -d '-' -f3 | cut -d ':' -f2)
> +
> +  if [[ "$recompute" == "norecompute" ]]; then
> +# node should not be recomputed
> +echo "Expecting $node recompute count - $node_recompute_ct to be 0"
> +check test "$node_recompute_ct" -eq "0"
> +  else
> +echo "Expecting $node recompute count - $node_recompute_ct not to be 0"
> +check test "$node_recompute_ct" -ne "0"
> +  fi
> +
> +  if [[ "$compute" == "nocompute" ]]; then
> +# node should not be computed
> +echo "Expecting $node compute count - $node_compute_ct to be 0"
> +check test "$node_compute_ct" -eq "0"
> +  else
> +echo "Expecting $node compute count - $node_compute_ct not to be 0"

Re: [ovs-dev] [PATCH ovn v2 1/3] ci: Switch Cirrus CI to the Ubuntu image

2023-11-23 Thread Ales Musil
On Thu, Nov 23, 2023 at 11:11 AM Dumitru Ceara  wrote:

> On 11/22/23 14:28, Ales Musil wrote:
> > Switch the Cirrus CI to use the Ubuntu image,
> > so it is unified with the GH actions. That is
> > useful for various dependency pinnings.
> >
> > Signed-off-by: Ales Musil 
> > ---
>
> Hi Ales,


Hi Dumitru,

thank you for the review.


> I think I'd just remove this patch.  We don't use the container image in
> the same way starting with patch 2/3.
>

Done in v3.


> Regards,
> Dumitru
>
> >  .cirrus.yml | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/.cirrus.yml b/.cirrus.yml
> > index 2d3824972..71e9afbb6 100644
> > --- a/.cirrus.yml
> > +++ b/.cirrus.yml
> > @@ -1,7 +1,7 @@
> >  arm_unit_tests_task:
> >
> >arm_container:
> > -image: ghcr.io/ovn-org/ovn-tests:fedora
> > +image: ghcr.io/ovn-org/ovn-tests:ubuntu
> >  memory: 4G
> >  cpu: 2
> >
> > @@ -10,6 +10,7 @@ arm_unit_tests_task:
> >  CIRRUS_CLONE_SUBMODULES: true
> >  PATH: ${HOME}/bin:${HOME}/.local/bin:${PATH}
> >  RECHECK: yes
> > +LANG: C
> >  matrix:
> >- CC: gcc
> >  TESTSUITE: test
>
>
Thanks,
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.com

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


Re: [ovs-dev] [PATCH ovn v2 2/3] ci: Build container image before very job

2023-11-23 Thread Ales Musil
On Thu, Nov 23, 2023 at 11:13 AM Dumitru Ceara  wrote:

> On 11/22/23 14:28, Ales Musil wrote:
> > Build the image before every job to allow more
> > fine-grained dependency pinning. This is especially
> > useful for stable branches as they might need to stay
> > on specific distribution or Python version.
> >
> > Signed-off-by: Ales Musil 
> > ---
> >  .cirrus.yml| 40 -
> >  .github/workflows/test.yml | 46 +++---
> >  2 files changed, 77 insertions(+), 9 deletions(-)
> >
> > diff --git a/.cirrus.yml b/.cirrus.yml
> > index 71e9afbb6..894f953f3 100644
> > --- a/.cirrus.yml
> > +++ b/.cirrus.yml
> > @@ -1,9 +1,25 @@
> > -arm_unit_tests_task:
> > +compute_engine_instance:
> > +  image_project: ubuntu-os-cloud
> > +  image: family/ubuntu-2304-amd64
> > +  platform: linux
> > +  memory: 4G
> > +
> > +build_image_task:
> > +  install_dependencies_script:
> > +- sudo apt update
> > +- sudo apt install -y podman make
> > +
> > +  build_container_script:
> > +- cd utilities/containers
> > +- make ubuntu
> > +- podman save -o /tmp/image.tar ovn-org/ovn-tests:ubuntu
> > +
> > +  upload_image_script:
> > +- curl -s -X POST -T /tmp/image.tar http://
> $CIRRUS_HTTP_CACHE_HOST/${CIRRUS_CHANGE_IN_REPO}
>
> This seemed a bit strange initially but I understand now (thanks for the
> offline clarification) that it's because we want to avoid building for
> each instance of the matrix in the unit test task.
>
> I think a comment explaining that here would be good as others might
> also wonder why we need the cache.
>


Thank you for the review. I have added a comment in v3.


>
> Thanks,
> Dumitru
>

> >
> > -  arm_container:
> > -image: ghcr.io/ovn-org/ovn-tests:ubuntu
> > -memory: 4G
> > -cpu: 2
> > +arm_unit_tests_task:
> > +  depends_on:
> > +- build_image
> >
> >env:
> >  ARCH: aarch64
> > @@ -11,6 +27,7 @@ arm_unit_tests_task:
> >  PATH: ${HOME}/bin:${HOME}/.local/bin:${PATH}
> >  RECHECK: yes
> >  LANG: C
> > +IMAGE_NAME: ovn-org/ovn-tests:ubuntu
> >  matrix:
> >- CC: gcc
> >  TESTSUITE: test
> > @@ -35,5 +52,16 @@ arm_unit_tests_task:
> >
> >name: ARM64 ${CC} ${TESTSUITE} ${TEST_RANGE}
> >
> > +  install_dependencies_script:
> > +- sudo apt update
> > +- sudo apt install -y podman
> > +
> > +  download_cache_script:
> > +- curl http://$CIRRUS_HTTP_CACHE_HOST/${CIRRUS_CHANGE_IN_REPO} -o
> /tmp/image.tar
> > +
> > +  load_image_script:
> > +- podman load -i /tmp/image.tar
> > +- rm -rf /tmp/image.tar
> > +
> >build_script:
> > -- ./.ci/linux-build.sh
> > +- ./.ci/ci.sh --archive-logs
> > diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
> > index b3fcb57fc..fdbc8f5f5 100644
> > --- a/.github/workflows/test.yml
> > +++ b/.github/workflows/test.yml
> > @@ -80,10 +80,38 @@ jobs:
> >if: steps.dpdk_cache.outputs.cache-hit != 'true'
> >run: ./.ci/dpdk-build.sh
> >
> > +  prepare-container:
> > +env:
> > +  DEPENDENCIES: podman
> > +name: Prepare container
> > +runs-on: ubuntu-22.04
> > +
> > +steps:
> > +  - uses: actions/checkout@v3
> > +
> > +  - name: Update APT cache
> > +run: sudo apt update
> > +
> > +  - name: Install dependencies
> > +run: sudo apt install -y ${{ env.DEPENDENCIES }}
> > +
> > +  - name: Build container
> > +run: make ubuntu
> > +working-directory: utilities/containers
> > +
> > +  - name: Export image
> > +run: podman save -o /tmp/image.tar ovn-org/ovn-tests
> > +
> > +  - name: Cache image
> > +id: image_cache
> > +uses: actions/cache@v3
> > +with:
> > +  path: /tmp/image.tar
> > +  key: ${{ github.sha }}
> > +
> >build-linux:
> > -needs: build-dpdk
> > +needs: [build-dpdk, prepare-container]
> >  env:
> > -  IMAGE_NAME:  ghcr.io/ovn-org/ovn-tests:ubuntu
> >ARCH:${{ matrix.cfg.arch }}
> >CC:  ${{ matrix.cfg.compiler }}
> >DPDK:${{ matrix.cfg.dpdk }}
> > @@ -152,13 +180,25 @@ jobs:
> >  sort -V | tail -1)
> >working-directory: ovs
> >
> > -- name: cache
> > +- name: cache dpdk
> >if: matrix.cfg.dpdk != ''
> >uses: actions/cache@v3
> >with:
> >  path: dpdk-dir
> >  key: ${{ needs.build-dpdk.outputs.dpdk_key }}
> >
> > +- name: image cache
> > +  uses: actions/cache@v3
> > +  with:
> > +path: /tmp/image.tar
> > +key: ${{ github.sha }}
> > +
> > +- name: load image
> > +  run: |
> > +sudo podman load -i /tmp/image.tar
> > +podman load -i /tmp/image.tar
> > +rm -rf /tmp/image.tar
> > +
> >  - name: build
> >if: ${{ startsWith(matrix.cfg.testsuite, 'system-test') }}
> >run: sudo -E ./.ci/ci.sh --archive-log


Thanks,
Ales

-- 

Ales Musil


Re: [ovs-dev] [PATCH ovn v2 3/3] ci: Cover more container posibilities

2023-11-23 Thread Ales Musil
On Thu, Nov 23, 2023 at 11:15 AM Dumitru Ceara  wrote:

> On 11/22/23 14:28, Ales Musil wrote:
> > Add more conditions to the image prepare process.
> > This allows us to test the prebuilt images for both
> > Fedora and Ubuntu.
> >
> > Run the weekly image on main branch with the
> > use of Fedora for the weekly runs.
> >
> > Reported-at: https://issues.redhat.com/browse/FDP-149
> > Signed-off-by: Ales Musil 
> > ---
> >  .github/workflows/test.yml | 17 -
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
> > index fdbc8f5f5..60afcba77 100644
> > --- a/.github/workflows/test.yml
> > +++ b/.github/workflows/test.yml
> > @@ -95,10 +95,25 @@ jobs:
> >- name: Install dependencies
> >  run: sudo apt install -y ${{ env.DEPENDENCIES }}
> >
>
> Can we add a comment at the top of the workflow file explaining what
> runs where and triggered by what event?  It could also be a separate
> patch but now that we're changing things it might make sense to properly
> document it.
>
> Otherwise, looks good to me, thanks!
>

Thank you for the review, I've added a comment in v3.



>
> > +  - name: Choose image distro
> > +if: github.event_name == 'push' || github.event_name ==
> 'pull_request'
> > +run: |
> > +  echo "IMAGE_DISTRO=ubuntu" >> $GITHUB_ENV
> > +
> > +  - name: Choose image distro
> > +if: github.event_name == 'schedule'
> > +run: |
> > +  echo "IMAGE_DISTRO=fedora" >> $GITHUB_ENV
> > +
> >- name: Build container
> > -run: make ubuntu
> > +if: github.ref_name != 'main'
> > +run: make ${{ env.IMAGE_DISTRO }}
> >  working-directory: utilities/containers
> >
> > +  - name: Download container
> > +if: github.ref_name == 'main'
> > +run: podman pull ghcr.io/ovn-org/ovn-tests:${{
>  env.IMAGE_DISTRO }}
> > +
> >- name: Export image
> >  run: podman save -o /tmp/image.tar ovn-org/ovn-tests
> >
>
>
Thanks,
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.com

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


[ovs-dev] [PATCH ovn v3 1/3] ci: Build container image before very job

2023-11-23 Thread Ales Musil
Build the image before every job to allow more
fine-grained dependency pinning. This is especially
useful for stable branches as they might need to stay
on specific distribution or Python version.

This also allows us to switch to Ubuntu instead
of Fedora in the Cirrus CI for consistency reasons.

Signed-off-by: Ales Musil 
---
v3: Fix the Cirrus CI architecture to ensure it is really running on ARM64.
Add comment why do we need separate task in the Cirrus CI.
---
 .cirrus.yml| 44 ++--
 .github/workflows/test.yml | 46 +++---
 2 files changed, 80 insertions(+), 10 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 2d3824972..5453bf93d 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -1,15 +1,34 @@
-arm_unit_tests_task:
+compute_engine_instance:
+  image_project: ubuntu-os-cloud
+  image: family/ubuntu-2304-arm64
+  architecture: arm64
+  platform: linux
+  memory: 4G
+
+# Run separate task for the image build, so it's running only once outside
+# the test matrix.
+build_image_task:
+  install_dependencies_script:
+- sudo apt update
+- sudo apt install -y podman make
+
+  build_container_script:
+- cd utilities/containers
+- make ubuntu
+- podman save -o /tmp/image.tar ovn-org/ovn-tests:ubuntu
+
+  upload_image_script:
+- curl -s -X POST -T /tmp/image.tar 
http://$CIRRUS_HTTP_CACHE_HOST/${CIRRUS_CHANGE_IN_REPO}
 
-  arm_container:
-image: ghcr.io/ovn-org/ovn-tests:fedora
-memory: 4G
-cpu: 2
+arm_unit_tests_task:
+  depends_on:
+- build_image
 
   env:
-ARCH: aarch64
 CIRRUS_CLONE_SUBMODULES: true
 PATH: ${HOME}/bin:${HOME}/.local/bin:${PATH}
 RECHECK: yes
+IMAGE_NAME: ovn-org/ovn-tests:ubuntu
 matrix:
   - CC: gcc
 TESTSUITE: test
@@ -34,5 +53,16 @@ arm_unit_tests_task:
 
   name: ARM64 ${CC} ${TESTSUITE} ${TEST_RANGE}
 
+  install_dependencies_script:
+- sudo apt update
+- sudo apt install -y podman
+
+  download_cache_script:
+- curl http://$CIRRUS_HTTP_CACHE_HOST/${CIRRUS_CHANGE_IN_REPO} -o 
/tmp/image.tar
+
+  load_image_script:
+- podman load -i /tmp/image.tar
+- rm -rf /tmp/image.tar
+
   build_script:
-- ./.ci/linux-build.sh
+- ./.ci/ci.sh --archive-logs
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index b3fcb57fc..fdbc8f5f5 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -80,10 +80,38 @@ jobs:
   if: steps.dpdk_cache.outputs.cache-hit != 'true'
   run: ./.ci/dpdk-build.sh
 
+  prepare-container:
+env:
+  DEPENDENCIES: podman
+name: Prepare container
+runs-on: ubuntu-22.04
+
+steps:
+  - uses: actions/checkout@v3
+
+  - name: Update APT cache
+run: sudo apt update
+
+  - name: Install dependencies
+run: sudo apt install -y ${{ env.DEPENDENCIES }}
+
+  - name: Build container
+run: make ubuntu
+working-directory: utilities/containers
+
+  - name: Export image
+run: podman save -o /tmp/image.tar ovn-org/ovn-tests
+
+  - name: Cache image
+id: image_cache
+uses: actions/cache@v3
+with:
+  path: /tmp/image.tar
+  key: ${{ github.sha }}
+
   build-linux:
-needs: build-dpdk
+needs: [build-dpdk, prepare-container]
 env:
-  IMAGE_NAME:  ghcr.io/ovn-org/ovn-tests:ubuntu
   ARCH:${{ matrix.cfg.arch }}
   CC:  ${{ matrix.cfg.compiler }}
   DPDK:${{ matrix.cfg.dpdk }}
@@ -152,13 +180,25 @@ jobs:
 sort -V | tail -1)
   working-directory: ovs
 
-- name: cache
+- name: cache dpdk
   if: matrix.cfg.dpdk != ''
   uses: actions/cache@v3
   with:
 path: dpdk-dir
 key: ${{ needs.build-dpdk.outputs.dpdk_key }}
 
+- name: image cache
+  uses: actions/cache@v3
+  with:
+path: /tmp/image.tar
+key: ${{ github.sha }}
+
+- name: load image
+  run: |
+sudo podman load -i /tmp/image.tar
+podman load -i /tmp/image.tar
+rm -rf /tmp/image.tar
+
 - name: build
   if: ${{ startsWith(matrix.cfg.testsuite, 'system-test') }}
   run: sudo -E ./.ci/ci.sh --archive-logs
-- 
2.42.0

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


[ovs-dev] [PATCH ovn v3 3/3] Documentation: Add note about pinning the container after release

2023-11-23 Thread Ales Musil
Add note that one the patches after branching should
pin the container to the current LTS version so the
CI on that branch remains stable as long as possible.

Signed-off-by: Ales Musil 
---
 Documentation/internals/release-process.rst | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/internals/release-process.rst 
b/Documentation/internals/release-process.rst
index ec79fe48c..26d3f8d4d 100644
--- a/Documentation/internals/release-process.rst
+++ b/Documentation/internals/release-process.rst
@@ -64,6 +64,10 @@ Scheduling`_ for the timing of each stage:
branch.  Features to be added to release branches should be limited in scope
and risk and discussed on ovs-dev before creating the branch.
 
+   In order to keep the CI stable on the new release branch, the Ubuntu
+   container should be pinned to the current LTS version in the Dockerfile
+   e.g. registry.hub.docker.com/library/ubuntu:22.04.
+
 3. When committers come to rough consensus that the release is ready, they
release the .0 release on its branch, e.g. 25.09.0 for branch-25.09.  To
make the actual release, a committer pushes a signed tag named, e.g.
-- 
2.42.0

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


[ovs-dev] [PATCH ovn v3 2/3] ci: Cover more container posibilities

2023-11-23 Thread Ales Musil
Add more conditions to the image prepare process.
This allows us to test the prebuilt images for both
Fedora and Ubuntu.

Run the weekly image on main branch with the
use of Fedora for the weekly runs.

Reported-at: https://issues.redhat.com/browse/FDP-149
Signed-off-by: Ales Musil 
---
v3: Add comment that explains the matrix that we have for the prepare-image job.
---
 .github/workflows/test.yml | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index fdbc8f5f5..02865b32f 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -81,6 +81,15 @@ jobs:
   run: ./.ci/dpdk-build.sh
 
   prepare-container:
+# This job has the following matrix, x: Job trigger, y: Branch
+# (scheduled jobs run only on main):
+# +---+---+---+
+# |   | Push/Pull request | Scheduled |
+# +---+---+---+
+# |  main |  ghcr.io - Ubuntu | ghcr.io - Fedora  |
+# +---+---+---+
+# | !main |  Builds - Ubuntu  | x |
+# +---+---+---+
 env:
   DEPENDENCIES: podman
 name: Prepare container
@@ -95,10 +104,25 @@ jobs:
   - name: Install dependencies
 run: sudo apt install -y ${{ env.DEPENDENCIES }}
 
+  - name: Choose image distro
+if: github.event_name == 'push' || github.event_name == 'pull_request'
+run: |
+  echo "IMAGE_DISTRO=ubuntu" >> $GITHUB_ENV
+
+  - name: Choose image distro
+if: github.event_name == 'schedule'
+run: |
+  echo "IMAGE_DISTRO=fedora" >> $GITHUB_ENV
+
   - name: Build container
-run: make ubuntu
+if: github.ref_name != 'main'
+run: make ${{ env.IMAGE_DISTRO }}
 working-directory: utilities/containers
 
+  - name: Download container
+if: github.ref_name == 'main'
+run: podman pull ghcr.io/ovn-org/ovn-tests:${{ env.IMAGE_DISTRO }}
+
   - name: Export image
 run: podman save -o /tmp/image.tar ovn-org/ovn-tests
 
-- 
2.42.0

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


[ovs-dev] [PATCH ovn v3 0/3] Allow to use different container images per branch

2023-11-23 Thread Ales Musil
This series allows different branches to use different images.
This is needed for stability reasons as it is hard to keep up
with new Ubuntu/Fedora releases for all stable branches.

We will still use the prebuilt images for main branch which
is completely fine.

Ales Musil (3):
  ci: Build container image before very job
  ci: Cover more container posibilities
  Documentation: Add note about pinning the container after release

 .cirrus.yml | 44 ++---
 .github/workflows/test.yml  | 70 -
 Documentation/internals/release-process.rst |  4 ++
 3 files changed, 108 insertions(+), 10 deletions(-)

-- 
2.42.0

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


[ovs-dev] [PATCH branch-3.2] dpdk: Use DPDK 22.11.3 release for OVS 3.2.

2023-11-23 Thread Kevin Traynor
Update the CI and docs to use DPDK 22.11.3.

Signed-off-by: Kevin Traynor 
---
 .github/workflows/build-and-test.yml | 2 +-
 Documentation/faq/releases.rst   | 8 
 Documentation/intro/install/dpdk.rst | 8 
 NEWS | 3 +++
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index bc5494e86..fd911c110 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -9,5 +9,5 @@ jobs:
   CC: gcc
   DPDK_GIT: https://dpdk.org/git/dpdk-stable
-  DPDK_VER: 22.11.1
+  DPDK_VER: 22.11.3
 name: dpdk gcc
 outputs:
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index e6bda14e7..362bf4ec7 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -216,8 +216,8 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 2.15.x   20.11.6
 2.16.x   20.11.6
-2.17.x   21.11.2
-3.0.x21.11.2
-3.1.x22.11.1
-3.2.x22.11.1
+2.17.x   21.11.5
+3.0.x21.11.5
+3.1.x22.11.3
+3.2.x22.11.3
  
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index 63a0ebb23..02eaf8b10 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, 
building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 22.11.1
+- DPDK 22.11.3
 
 - A `DPDK supported NIC`_
@@ -74,7 +74,7 @@ Install DPDK
 
$ cd /usr/src/
-   $ wget https://fast.dpdk.org/rel/dpdk-22.11.1.tar.xz
-   $ tar xf dpdk-22.11.1.tar.xz
-   $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.1
+   $ wget https://fast.dpdk.org/rel/dpdk-22.11.3.tar.xz
+   $ tar xf dpdk-22.11.3.tar.xz
+   $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.3
$ cd $DPDK_DIR
 
diff --git a/NEWS b/NEWS
index eb7a9b1ba..bd5acb153 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,7 @@
 v3.2.2 - xx xxx 
 
+   - Bug fixes
+   - DPDK:
+ * OVS validated with DPDK 22.11.3
 
 v3.2.1 - 17 Oct 2023
-- 
2.41.0

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


[ovs-dev] [PATCH branch-3.1] dpdk: Use DPDK 22.11.3 release for OVS 3.1.

2023-11-23 Thread Kevin Traynor
Update the CI and docs to use DPDK 22.11.3.

Signed-off-by: Kevin Traynor 
---
 .github/workflows/build-and-test.yml | 2 +-
 Documentation/faq/releases.rst   | 6 +++---
 Documentation/intro/install/dpdk.rst | 8 
 NEWS | 3 +++
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 80c449336..2a001f9ed 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -9,5 +9,5 @@ jobs:
   CC: gcc
   DPDK_GIT: https://dpdk.org/git/dpdk-stable
-  DPDK_VER: 22.11.1
+  DPDK_VER: 22.11.3
 name: dpdk gcc
 outputs:
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 9e1b42262..f956c7e10 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -215,7 +215,7 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 2.15.x   20.11.6
 2.16.x   20.11.6
-2.17.x   21.11.2
-3.0.x21.11.2
-3.1.x22.11.1
+2.17.x   21.11.5
+3.0.x21.11.5
+3.1.x22.11.3
  
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index 63a0ebb23..02eaf8b10 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, 
building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 22.11.1
+- DPDK 22.11.3
 
 - A `DPDK supported NIC`_
@@ -74,7 +74,7 @@ Install DPDK
 
$ cd /usr/src/
-   $ wget https://fast.dpdk.org/rel/dpdk-22.11.1.tar.xz
-   $ tar xf dpdk-22.11.1.tar.xz
-   $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.1
+   $ wget https://fast.dpdk.org/rel/dpdk-22.11.3.tar.xz
+   $ tar xf dpdk-22.11.3.tar.xz
+   $ export DPDK_DIR=/usr/src/dpdk-stable-22.11.3
$ cd $DPDK_DIR
 
diff --git a/NEWS b/NEWS
index 13229cb3a..b78da3f9d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,7 @@
 v3.1.4 - xx xxx 
 
+   - Bug fixes
+   - DPDK:
+ * OVS validated with DPDK 22.11.3
 
 v3.1.3 - 17 Oct 2023
-- 
2.41.0

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


[ovs-dev] [PATCH branch-3.0] dpdk: Use DPDK 21.11.5 release for OVS 3.0.

2023-11-23 Thread Kevin Traynor
Update the CI and docs to use DPDK 21.11.5.

Signed-off-by: Kevin Traynor 
---
 .ci/linux-build.sh   | 2 +-
 Documentation/faq/releases.rst   | 4 ++--
 Documentation/intro/install/dpdk.rst | 8 
 NEWS | 3 +++
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index a0d780101..2df466b0c 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -229,5 +229,5 @@ fi
 if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="21.11.2"
+DPDK_VER="21.11.5"
 fi
 install_dpdk $DPDK_VER
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index b19d9f556..3825e2695 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -214,6 +214,6 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 2.15.x   20.11.6
 2.16.x   20.11.6
-2.17.x   21.11.2
-3.0.x21.11.2
+2.17.x   21.11.5
+3.0.x21.11.5
  
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index a284e6851..559e8eb1f 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, 
building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 21.11.2
+- DPDK 21.11.5
 
 - A `DPDK supported NIC`_
@@ -74,7 +74,7 @@ Install DPDK
 
$ cd /usr/src/
-   $ wget https://fast.dpdk.org/rel/dpdk-21.11.2.tar.xz
-   $ tar xf dpdk-21.11.2.tar.xz
-   $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.2
+   $ wget https://fast.dpdk.org/rel/dpdk-21.11.5.tar.xz
+   $ tar xf dpdk-21.11.5.tar.xz
+   $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.5
$ cd $DPDK_DIR
 
diff --git a/NEWS b/NEWS
index 33b982886..d59645b04 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,7 @@
 v3.0.6 - xx xxx 
 
+   - Bug fixes
+   - DPDK:
+ * OVS validated with DPDK 21.11.5
 
 v3.0.5 - 17 Oct 2023
-- 
2.41.0

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


[ovs-dev] [PATCH branch-2.17] dpdk: Use DPDK 21.11.5 release for OVS 2.17.

2023-11-23 Thread Kevin Traynor
Update the CI and docs to use DPDK 21.11.5.

Signed-off-by: Kevin Traynor 
---
 .ci/linux-build.sh   | 2 +-
 Documentation/faq/releases.rst   | 2 +-
 Documentation/intro/install/dpdk.rst | 8 
 NEWS | 3 +++
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index f5021e1a8..9464ea49c 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -221,5 +221,5 @@ fi
 if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
 if [ -z "$DPDK_VER" ]; then
-DPDK_VER="21.11.2"
+DPDK_VER="21.11.5"
 fi
 install_dpdk $DPDK_VER
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 49895c595..0e0c589a3 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -211,5 +211,5 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 2.15.x   20.11.6
 2.16.x   20.11.6
-2.17.x   21.11.2
+2.17.x   21.11.5
  
 
diff --git a/Documentation/intro/install/dpdk.rst 
b/Documentation/intro/install/dpdk.rst
index a284e6851..559e8eb1f 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -43,5 +43,5 @@ In addition to the requirements described in :doc:`general`, 
building Open
 vSwitch with DPDK will require the following:
 
-- DPDK 21.11.2
+- DPDK 21.11.5
 
 - A `DPDK supported NIC`_
@@ -74,7 +74,7 @@ Install DPDK
 
$ cd /usr/src/
-   $ wget https://fast.dpdk.org/rel/dpdk-21.11.2.tar.xz
-   $ tar xf dpdk-21.11.2.tar.xz
-   $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.2
+   $ wget https://fast.dpdk.org/rel/dpdk-21.11.5.tar.xz
+   $ tar xf dpdk-21.11.5.tar.xz
+   $ export DPDK_DIR=/usr/src/dpdk-stable-21.11.5
$ cd $DPDK_DIR
 
diff --git a/NEWS b/NEWS
index 7d4a8c081..642beb45b 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,7 @@
 v2.17.9 - xx xxx 
 -
+   - Bug fixes
+   - DPDK:
+ * OVS validated with DPDK 21.11.5
 
 v2.17.8 - 17 Oct 2023
-- 
2.41.0

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


Re: [ovs-dev] [PATCH ovn v2 02/18] northd: Track ovn_datapaths in northd engine track data.

2023-11-23 Thread Dumitru Ceara
On 11/15/23 06:30, Han Zhou wrote:
> On Thu, Oct 26, 2023 at 11:14 AM  wrote:
>>
>> From: Numan Siddique 
>>
>> northd engine tracked data now also stores the logical switches
>> and logical routers that got updated due to the changed load balancers.
>>
>> Eg 1.  For this command 'ovn-nbctl ls-lb-add sw0 lb1 -- lr-lb-add lr0
>> lb1', northd engine tracking data will store 'sw0' and 'lr0'.
>>
>> Eg 2.  If load balancer lb1 is already associated with 'sw0' and 'lr0'
>> then for this command 'ovn-nbctl set load_balancer 
>> vips:10.0.0.10=20.0.0.20', northd engine tracking data will store
>> 'sw0' and 'lr0'.
>>
>> An upcoming commit will make use of this tracked data.
>>
>> Signed-off-by: Numan Siddique 
>> ---
>>  northd/northd.c | 34 +-
>>  northd/northd.h | 12 
>>  2 files changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/northd/northd.c b/northd/northd.c
>> index df22a9c658..9ce1b2cb5a 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -5146,6 +5146,8 @@ destroy_northd_data_tracked_changes(struct
> northd_data *nd)
>>  struct northd_tracked_data *trk_changes = >trk_northd_changes;
>>  destroy_tracked_ovn_ports(_changes->trk_ovn_ports);
>>  destroy_tracked_lbs(_changes->trk_lbs);
>> +hmapx_clear(_changes->ls_with_changed_lbs.crupdated);
>> +hmapx_clear(_changes->lr_with_changed_lbs.crupdated);
>>  nd->change_tracked = false;
>>  }
>>
>> @@ -5158,6 +5160,8 @@ init_northd_tracked_data(struct northd_data *nd)
>>  hmapx_init(_changes->trk_ovn_ports.deleted);
>>  hmapx_init(_changes->trk_lbs.crupdated);
>>  hmapx_init(_changes->trk_lbs.deleted);
>> +hmapx_init(_changes->ls_with_changed_lbs.crupdated);
>> +hmapx_init(_changes->lr_with_changed_lbs.crupdated);
>>  }
>>
>>  static void
>> @@ -5169,6 +5173,8 @@ destroy_northd_tracked_data(struct northd_data *nd)
>>  hmapx_destroy(_changes->trk_ovn_ports.deleted);
>>  hmapx_destroy(_changes->trk_lbs.crupdated);
>>  hmapx_destroy(_changes->trk_lbs.deleted);
>> +hmapx_destroy(_changes->ls_with_changed_lbs.crupdated);
>> +hmapx_destroy(_changes->lr_with_changed_lbs.crupdated);
>>  }
>>
>>
>> @@ -5179,7 +5185,10 @@ northd_has_tracked_data(struct northd_tracked_data
> *trk_nd_changes)
>>  || !hmapx_is_empty(_nd_changes->trk_ovn_ports.updated)
>>  || !hmapx_is_empty(_nd_changes->trk_ovn_ports.deleted)
>>  || !hmapx_is_empty(_nd_changes->trk_lbs.crupdated)
>> -|| !hmapx_is_empty(_nd_changes->trk_lbs.deleted));
>> +|| !hmapx_is_empty(_nd_changes->trk_lbs.deleted)
>> +||
> !hmapx_is_empty(_nd_changes->ls_with_changed_lbs.crupdated)
>> +||
> !hmapx_is_empty(_nd_changes->lr_with_changed_lbs.crupdated)
>> +);
>>  }
>>
>>  bool
>> @@ -5188,6 +5197,8 @@ northd_has_only_ports_in_tracked_data(
>>  {
>>  return (hmapx_is_empty(_nd_changes->trk_lbs.crupdated)
>>  && hmapx_is_empty(_nd_changes->trk_lbs.deleted)
>> +&&
> hmapx_is_empty(_nd_changes->ls_with_changed_lbs.crupdated)
>> +&&
> hmapx_is_empty(_nd_changes->lr_with_changed_lbs.crupdated)
>>  && (!hmapx_is_empty(_nd_changes->trk_ovn_ports.created)
>>  || !hmapx_is_empty(_nd_changes->trk_ovn_ports.updated)
>>  || !hmapx_is_empty(_nd_changes->trk_ovn_ports.deleted)));
>> @@ -5828,6 +5839,9 @@ northd_handle_lb_data_changes(struct
> tracked_lb_data *trk_lb_data,
>> lb_dps->nb_ls_map) {
>>  od = ls_datapaths->array[index];
>>  init_lb_for_datapath(od);
>> +
>> +/* Add the ls datapath to the northd tracked data. */
>> +hmapx_add(_changes->ls_with_changed_lbs.crupdated, od);
>>  }
>>
>>  hmap_remove(lb_datapaths_map, _dps->hmap_node);
>> @@ -5909,6 +5923,9 @@ northd_handle_lb_data_changes(struct
> tracked_lb_data *trk_lb_data,
>>
>>  /* Re-evaluate 'od->has_lb_vip' */
>>  init_lb_for_datapath(od);
>> +
>> +/* Add the ls datapath to the northd tracked data. */
>> +hmapx_add(_changes->ls_with_changed_lbs.crupdated, od);
>>  }
>>
>>  LIST_FOR_EACH (codlb, list_node, _lb_data->crupdated_lr_lbs) {
>> @@ -5954,6 +5971,9 @@ northd_handle_lb_data_changes(struct
> tracked_lb_data *trk_lb_data,
>>
>>  /* Re-evaluate 'od->has_lb_vip' */
>>  init_lb_for_datapath(od);
>> +
>> +/* Add the lr datapath to the northd tracked data. */
>> +hmapx_add(_changes->lr_with_changed_lbs.crupdated, od);
>>  }
>>
>>  HMAP_FOR_EACH (clb, hmap_node, _lb_data->crupdated_lbs) {
>> @@ -5968,6 +5988,9 @@ northd_handle_lb_data_changes(struct
> tracked_lb_data *trk_lb_data,
>>  od = ls_datapaths->array[index];
>>  /* Re-evaluate 'od->has_lb_vip' */
>>  init_lb_for_datapath(od);
>> +
>> +/* Add the ls datapath to the northd tracked data. */
>> +

Re: [ovs-dev] [PATCH ovn v2 01/18] northd: Refactor the northd change tracking.

2023-11-23 Thread Dumitru Ceara
On 11/15/23 19:22, Numan Siddique wrote:
> On Wed, Nov 15, 2023 at 12:12 AM Han Zhou  wrote:
>>
>> On Thu, Oct 26, 2023 at 11:14 AM  wrote:
>>>
>>> From: Numan Siddique 
>>>
>>> northd engine tracking data now has the following tracking data
>>>   - changed ovn_ports (right now only changed logical switch ports are
>>> tracked.)
>>>   - changed load balancers.
>>>
>>> This separation becomes easier to add lflow handling for these
>>> changes in lflow northd engine handler.  This patch doesn't
>>> handle the load balancer changes in lflow handler.  It will
>>> be handled in upcoming commits.
>>>
>>> Signed-off-by: Numan Siddique 
>>> ---

Hi Numan,

I have a couple of comments too below, nothing fundamental but
hopefully something that would simplify the code a bit.

Regards,
Dumitru

>>>  northd/en-lflow.c   |  11 +-
>>>  northd/en-northd.c  |  13 +-
>>>  northd/en-sync-sb.c |  10 +-
>>>  northd/northd.c | 446 
>>>  northd/northd.h |  63 ---
>>>  tests/ovn-northd.at |  10 +-
>>>  6 files changed, 313 insertions(+), 240 deletions(-)
>>>
>>> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
>>> index 2b84fef0ef..96d03b7ada 100644
>>> --- a/northd/en-lflow.c
>>> +++ b/northd/en-lflow.c
>>> @@ -108,8 +108,8 @@ lflow_northd_handler(struct engine_node *node,
>>>  return false;
>>>  }
>>>
>>> -/* Fall back to recompute if lb related data has changed. */
>>> -if (northd_data->lb_changed) {
>>> +/* Fall back to recompute if load balancers have changed. */
>>> +if
>> (northd_has_lbs_in_tracked_data(_data->trk_northd_changes)) {
>>>  return false;
>>>  }
>>>
>>> @@ -119,13 +119,14 @@ lflow_northd_handler(struct engine_node *node,
>>>  struct lflow_input lflow_input;
>>>  lflow_get_input_data(node, _input);
>>>
>>> -if (!lflow_handle_northd_ls_changes(eng_ctx->ovnsb_idl_txn,
>>> -_data->tracked_ls_changes,
>>> -_input,
>> _data->lflows)) {
>>> +if (!lflow_handle_northd_port_changes(eng_ctx->ovnsb_idl_txn,
>>> +
>>  _data->trk_northd_changes.trk_ovn_ports,
>>> +_input, _data->lflows)) {
>>>  return false;
>>>  }
>>>
>>>  engine_set_node_state(node, EN_UPDATED);
>>> +

Nit: unrelated?

>>>  return true;
>>>  }
>>>
>>> diff --git a/northd/en-northd.c b/northd/en-northd.c
>>> index aa0f20f0c2..96c2ce9f69 100644
>>> --- a/northd/en-northd.c
>>> +++ b/northd/en-northd.c
>>> @@ -230,15 +230,16 @@ northd_lb_data_handler(struct engine_node *node,
>> void *data)
>>> >ls_datapaths,
>>> >lr_datapaths,
>>> >lb_datapaths_map,
>>> -   >lb_group_datapaths_map)) {
>>> +   >lb_group_datapaths_map,
>>> +   >trk_northd_changes)) {
>>>  return false;
>>>  }
>>>
>>> -/* Indicate the depedendant engine nodes that load balancer/group
>>> - * related data has changed (including association to logical
>>> - * switch/router). */
>>> -nd->lb_changed = true;
>>> -engine_set_node_state(node, EN_UPDATED);
>>> +if (northd_has_lbs_in_tracked_data(>trk_northd_changes)) {
>>> +nd->change_tracked = true;
>>> +engine_set_node_state(node, EN_UPDATED);
>>> +}
>>> +
>>>  return true;
>>>  }
>>>
>>> diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
>>> index 2ec3bf54f8..2540fcfb97 100644
>>> --- a/northd/en-sync-sb.c
>>> +++ b/northd/en-sync-sb.c
>>> @@ -236,7 +236,8 @@ sync_to_sb_lb_northd_handler(struct engine_node
>> *node, void *data OVS_UNUSED)
>>>  {
>>>  struct northd_data *nd = engine_get_input_data("northd", node);
>>>
>>> -if (!nd->change_tracked || nd->lb_changed) {
>>> +if (!nd->change_tracked ||
>>> +northd_has_lbs_in_tracked_data(>trk_northd_changes)) {
>>>  /* Return false if no tracking data or if lbs changed. */
>>>  return false;
>>>  }
>>> @@ -306,11 +307,14 @@ sync_to_sb_pb_northd_handler(struct engine_node
>> *node, void *data OVS_UNUSED)
>>>  }
>>>
>>>  struct northd_data *nd = engine_get_input_data("northd", node);
>>> -if (!nd->change_tracked) {
>>> +if (!nd->change_tracked ||
>>> +northd_has_lbs_in_tracked_data(>trk_northd_changes)) {
>>> +/* Return false if no tracking data or if lbs changed. */
>>>  return false;
>>>  }
>>>
>>> -if (!sync_pbs_for_northd_ls_changes(>tracked_ls_changes)) {
>>> +if (!sync_pbs_for_northd_changed_ovn_ports(
>>> +>trk_northd_changes.trk_ovn_ports)) {
>>>  return false;
>>>  }
>>>
>>> diff --git a/northd/northd.c b/northd/northd.c
>>> index f8b046d83e..df22a9c658 100644
>>> --- a/northd/northd.c
>>> +++ b/northd/northd.c
>>> @@ -4894,21 +4894,20 @@ 

Re: [ovs-dev] [dpdk-latest 2/2] ci: Reduce optional libraries in DPDK.

2023-11-23 Thread Simon Horman
On Wed, Nov 22, 2023 at 01:16:12PM +0100, David Marchand wrote:
> Since DPDK v23.11, it is possible to select more easily which optional
> library is enabled.
> 
> OVS needs the vhost library (and its dependencies).
> The net/tap DPDK driver needs the gso library.
> Other optional library can be disabled.
> 
> This reduces the cache entry for DPDK from ~7MB to ~4MB.

Nice resource reduction.

> Signed-off-by: David Marchand 

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


Re: [ovs-dev] [dpdk-latest 1/2] ci: Cache DPDK installed libraries only.

2023-11-23 Thread Simon Horman
On Wed, Nov 22, 2023 at 01:16:11PM +0100, David Marchand wrote:
> Rather than save the whole DPDK sources and build artefacts, checkout
> sources in a separate directory and build DPDK there.
> Only the installed artefacts are then going to the cache.
> Example sources in the share/dpdk installed directory can be pruned too.
> 
> This makes a (v23.11-rc3) DPDK cache entry size go from ~120MB to ~7MB.

Very nice.

> Signed-off-by: David Marchand 

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


[ovs-dev] [PATCH 8/8] ci: Exclude tests that show random failures through GitHub actions.

2023-11-23 Thread Eelco Chaudron
I ran 80 series of full tests, and the following tests showed failures:

 802.1ad - vlan_limit
   +2023-11-20T10:32:11.245Z|1|dpif_netdev(revalidator5)|ERR|internal
 error parsing flow key recirc_id(0),dp_hash(0),skb_priority(0),
 in_port(2),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),
 packet_type(ns=0,id=0),eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),
 eth_type(0x88a8),vlan(vid=4094,pcp=0),encap(eth_type(0x8100),
 vlan(vid=100,pcp=0),encap(eth_type(0x86dd),ipv6(
 src=::,dst=ff02::1:ff46:681b,label=0,proto=58,tclass=0,hlimit=255,
 frag=no),icmpv6(type=135,code=0),nd(target=fe80::407e:4bff:fe46:681b,
 sll=00:00:00:00:00:00,tll=00:00:00:00:00:00)))
   +2023-11-20T10:32:11.245Z|2|dpif(revalidator5)|WARN|netdev@ovs-netdev:
 failed to put[modify] (Invalid argument)
 ufid:ef1ca90c-dbd0-4ca7-9869-411bdffd1ece recirc_id(0),dp_hash(0/0),
 skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),
 ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),
 eth(src=42:7e:4b:46:68:1b,dst=33:33:ff:46:68:1b),eth_type(0x88a8),
 vlan(vid=4094,pcp=0/0x0),encap(eth_type(0x8100),
 vlan(vid=100/0x0,pcp=0/0x0),encap(eth_type(0x86dd),
 ipv6(src=::/::,dst=ff02::1:ff46:681b/::,label=0/0,proto=58/0,
 tclass=0/0,hlimit=255/0,frag=no),icmpv6(type=135/0,code=0/0),
 nd(target=fe80::407e:4bff:fe46:681b/::,
 sll=00:00:00:00:00:00/00:00:00:00:00:00,
 tll=00:00:00:00:00:00/00:00:00:00:00:00))), actions:drop

  conntrack - zones from other field, more tests
+2023-11-20T10:45:43.015Z|1|dpif(handler5)|WARN|system@ovs-system:
  execute ct(commit),3 failed (Invalid argument) on packet tcp,
  vlan_tci=0x,dl_src=42:7e:4b:46:68:1b,dl_dst=ba:72:4c:a5:31:6b,
  nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,
  nw_frag=no,tp_src=53738,tp_dst=80,tcp_flags=psh|ack tcp_csum:e4a

  conntrack - limit by zone
./system-traffic.at:5154: ovs-appctl dpctl/ct-get-limits zone=0,1,2,3,4,5
--- -   2023-11-20 10:51:09.965375141 +
+++ /home/runner/work/ovs/ovs/tests/system-kmod-testsuite.dir/at-groups/
  114/stdout2023-11-20 10:51:09.956723756 +
@@ -1,5 +1,5 @@
 default limit=10
-zone=0,limit=5,count=5
+zone=0,limit=5,count=6

  conntrack - Multiple ICMP traverse
./system-traffic.at:7571: ovs-appctl dpctl/dump-conntrack | grep 
"dst=10.1.1" | sed -e 's/port=[0-9]*/port=/g' -e 
's/id=[0-9]*/id=/g'
  -e 's/state=[0-9_A-Z]*/state=/g' | sort | uniq
--- -   2023-11-20 15:36:02.591051192 +
+++ 
/home/runner/work/ovs/ovs/tests/system-kmod-testsuite.dir/at-groups/156/stdout  
2023-11-20 15:36:02.585722099 +
@@ -1,2 +1,9 @@
+tcp,orig=(src=10.1.1.7,dst=13.107.43.16,sport=,
  dport=),reply=(src=13.107.43.16,dst=10.1.1.7,sport=,
  dport=),protoinfo=(state=)
+tcp,orig=(src=10.1.1.7,dst=168.63.129.16,sport=,
  dport=),reply=(src=168.63.129.16,dst=10.1.1.7,sport=,
  dport=),protoinfo=(state=)
...
+tcp,orig=(src=20.22.98.201,dst=10.1.1.7,sport=,dport=),
  reply=(src=10.1.1.7,dst=20.22.98.201,sport=,dport=),
  protoinfo=(state=)

  conntrack - ct flush
+++ 
/home/runner/work/ovs/ovs/tests/system-kmod-testsuite.dir/at-groups/57/stdout   
2023-11-22 13:51:04.234496131 +
@@ -1,3 +1,5 @@
+tcp,orig=(src=10.1.1.154,dst=13.107.42.16,sport=45300,dport=443),
  reply=(src=13.107.42.16,dst=10.1.1.154,sport=443,dport=45300),
  protoinfo=(state=ESTABLISHED)
+tcp,orig=(src=10.1.1.154,dst=20.72.125.48,sport=45572,dport=443),
  reply=(src=20.72.125.48,dst=10.1.1.154,sport=443,dport=45572),
  protoinfo=(state=ESTABLISHED)

As I do not see those failures when running these stand alone on the
same Ubuntu distribution, I've disabled first three tests for now.

For the other tests we narrowed the result to not include any of the local
IP addresses in the test results.

Signed-off-by: Eelco Chaudron 
---
 tests/system-traffic.at |   45 -
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index a7d4ed83b..459e6e07b 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2584,133 +2584,133 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 
"in_port=1 packet=5054000a5
 AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 
packet=5054000a505400090800451c0011a4cd0a0101020a010101000200010008
 actions=resubmit(,0)"])
 
 
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sort], [0], 
[dnl
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1," | sort], [0], 
[dnl
 
udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
 
udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
 AT_CHECK([FLUSH_CMD 

[ovs-dev] [PATCH 7/8] ci: Add make check-afxdp to GitHub actions ci.

2023-11-23 Thread Eelco Chaudron
Signed-off-by: Eelco Chaudron 
---
 .github/workflows/build-and-test.yml |4 
 1 file changed, 4 insertions(+)

diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 7cbb8d956..92282edb4 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -191,6 +191,10 @@ jobs:
 dpdk: dpdk
 testsuite:check-system-tso
 
+  - compiler: gcc
+dpdk: dpdk
+testsuite:check-afxdp
+
 steps:
 - name: checkout
   uses: actions/checkout@v3

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


[ovs-dev] [PATCH 6/8] ci: Allow make check-dpdk to run more tests.

2023-11-23 Thread Eelco Chaudron
Install additional packages and drivers required by
make check-dpdk.

Signed-off-by: Eelco Chaudron 
---
 .ci/dpdk-build.sh|2 +-
 .github/workflows/build-and-test.yml |2 +-
 python/test_requirements.txt |1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh
index aa83e4464..d4c178ee0 100755
--- a/.ci/dpdk-build.sh
+++ b/.ci/dpdk-build.sh
@@ -38,7 +38,7 @@ function build_dpdk()
 # any DPDK driver.
 # check-dpdk unit tests requires testpmd and some net/ driver.
 DPDK_OPTS="$DPDK_OPTS -Denable_apps=test-pmd"
-enable_drivers="net/null,net/af_xdp,net/tap,net/virtio"
+enable_drivers="net/null,net/af_xdp,net/tap,net/virtio,net/pcap"
 DPDK_OPTS="$DPDK_OPTS -Denable_drivers=$enable_drivers"
 
 # Install DPDK using prefix.
diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index d74668f61..7cbb8d956 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -5,7 +5,7 @@ on: [push, pull_request]
 jobs:
   build-dpdk:
 env:
-  dependencies: gcc libbpf-dev libnuma-dev ninja-build pkgconf
+  dependencies: gcc libbpf-dev libnuma-dev libpcap-dev ninja-build pkgconf
   CC: gcc
   DPDK_GIT: https://dpdk.org/git/dpdk-stable
   DPDK_VER: 22.11.1
diff --git a/python/test_requirements.txt b/python/test_requirements.txt
index c85ce41ad..5043c71e2 100644
--- a/python/test_requirements.txt
+++ b/python/test_requirements.txt
@@ -2,4 +2,5 @@ netaddr
 pyftpdlib
 pyparsing
 pytest
+scapy
 tftpy

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


[ovs-dev] [PATCH 5/8] ci: Add make check-system-tso to GitHub actions ci.

2023-11-23 Thread Eelco Chaudron
Signed-off-by: Eelco Chaudron 
---
 .github/workflows/build-and-test.yml |4 
 1 file changed, 4 insertions(+)

diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index db0a1ac39..d74668f61 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -187,6 +187,10 @@ jobs:
 dpdk: dpdk
 testsuite:check-system-userspace
 
+  - compiler: gcc
+dpdk: dpdk
+testsuite:check-system-tso
+
 steps:
 - name: checkout
   uses: actions/checkout@v3

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


[ovs-dev] [PATCH 4/8] ci: Add make check-system-userspace to GitHub actions ci.

2023-11-23 Thread Eelco Chaudron
Signed-off-by: Eelco Chaudron 
---
 .github/workflows/build-and-test.yml |4 
 1 file changed, 4 insertions(+)

diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 586b0cdd9..db0a1ac39 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -183,6 +183,10 @@ jobs:
 testsuite:check-offloads
 test_range:   "100-"
 
+  - compiler: gcc
+dpdk: dpdk
+testsuite:check-system-userspace
+
 steps:
 - name: checkout
   uses: actions/checkout@v3

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


[ovs-dev] [PATCH 3/8] ci: Add make check-offloads to GitHub actions ci.

2023-11-23 Thread Eelco Chaudron
This patch also adds the 'CHECK_GITHUB_ACTION' macro to skip
tests that won't execute successfully through GitHub actions.
We could not use the -k !keyword option, as it can not be
combined with a range of tests.

Signed-off-by: Eelco Chaudron 
---
 .ci/linux-build.sh   |2 +-
 .github/workflows/build-and-test.yml |7 +++
 tests/system-common-macros.at|4 
 tests/system-offloads-traffic.at |2 ++
 4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 4f2e36610..85788748f 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -139,7 +139,7 @@ else
 export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
 fi
 $run_as_root make $testsuite TESTSUITEFLAGS="$JOBS $TEST_RANGE" \
- RECHECK=yes
+ RECHECK=yes GITHUB_ACTIONS=$GITHUB_ACTIONS
 done
 fi
 
diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 0b881ca91..586b0cdd9 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -176,6 +176,13 @@ jobs:
 testsuite:check-kernel
 test_range:   "100-"
 
+  - compiler: gcc
+testsuite:check-offloads
+test_range:   "-100"
+  - compiler: gcc
+testsuite:check-offloads
+test_range:   "100-"
+
 steps:
 - name: checkout
   uses: actions/checkout@v3
diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 0113aae8b..0620be0c7 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -365,3 +365,7 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP],
 # OVS_CHECK_CT_CLEAR()
 m4_define([OVS_CHECK_CT_CLEAR],
 [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" 
ovs-vswitchd.log])])
+
+# OVS_CHECK_GITHUB_ACTION
+m4_define([OVS_CHECK_GITHUB_ACTION],
+[AT_SKIP_IF([test "$GITHUB_ACTIONS" = "true"])])
diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at
index 0bedee753..6bd49a3ee 100644
--- a/tests/system-offloads-traffic.at
+++ b/tests/system-offloads-traffic.at
@@ -192,6 +192,7 @@ AT_CLEANUP
 AT_SETUP([offloads - check interface meter offloading -  offloads disabled])
 AT_KEYWORDS([dp-meter])
 AT_SKIP_IF([test $HAVE_NC = "no"])
+OVS_CHECK_GITHUB_ACTION()
 OVS_TRAFFIC_VSWITCHD_START()
 
 AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=1 pktps bands=type=drop 
rate=1'])
@@ -240,6 +241,7 @@ AT_CLEANUP
 
 AT_SETUP([offloads - check interface meter offloading -  offloads enabled])
 AT_KEYWORDS([offload-meter])
+OVS_CHECK_GITHUB_ACTION()
 CHECK_TC_INGRESS_PPS()
 AT_SKIP_IF([test $HAVE_NC = "no"])
 OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . 
other_config:hw-offload=true])

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


[ovs-dev] [PATCH 2/8] ci: Add make check-kernel to GitHub actions ci.

2023-11-23 Thread Eelco Chaudron
Signed-off-by: Eelco Chaudron 
---
 .ci/linux-build.sh   |8 ++--
 .github/workflows/build-and-test.yml |   15 ---
 python/test_requirements.txt |4 +++-
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index e9e1e24b5..4f2e36610 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -129,13 +129,17 @@ else
 build_ovs
 for testsuite in $TESTSUITE; do
 run_as_root=
+if [ "$testsuite" != "check" ] && \
+   [ "$testsuite" != "check-ovsdb-cluster" ] ; then
+run_as_root="sudo -E PATH=$PATH"
+fi
 if [ "${testsuite##*dpdk}" != "$testsuite" ]; then
 sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages' || true
 [ "$(cat /proc/sys/vm/nr_hugepages)" = '1024' ]
 export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
-run_as_root="sudo -E PATH=$PATH"
 fi
-$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
+$run_as_root make $testsuite TESTSUITEFLAGS="$JOBS $TEST_RANGE" \
+ RECHECK=yes
 done
 fi
 
diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 5d441157c..0b881ca91 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -12,7 +12,7 @@ jobs:
 name: dpdk gcc
 outputs:
   dpdk_key: ${{ steps.gen_dpdk_key.outputs.key }}
-runs-on: ubuntu-20.04
+runs-on: ubuntu-22.04
 timeout-minutes: 30
 
 steps:
@@ -76,7 +76,8 @@ jobs:
 env:
   dependencies: |
 automake libtool gcc bc libjemalloc2 libjemalloc-dev libssl-dev \
-llvm-dev libnuma-dev libpcap-dev selinux-policy-dev libbpf-dev
+llvm-dev libnuma-dev libpcap-dev selinux-policy-dev libbpf-dev \
+lftp libreswan
   ASAN:${{ matrix.asan }}
   UBSAN:   ${{ matrix.ubsan }}
   CC:  ${{ matrix.compiler }}
@@ -87,9 +88,10 @@ jobs:
   OPTS:${{ matrix.opts }}
   STD: ${{ matrix.std }}
   TESTSUITE:   ${{ matrix.testsuite }}
+  TEST_RANGE:  ${{ matrix.test_range }}
 
 name: linux ${{ join(matrix.*, ' ') }}
-runs-on: ubuntu-20.04
+runs-on: ubuntu-22.04
 timeout-minutes: 30
 
 strategy:
@@ -167,6 +169,13 @@ jobs:
   - compiler: gcc
 testsuite:check-ovsdb-cluster
 
+  - compiler: gcc
+testsuite:check-kernel
+test_range:   "-100"
+  - compiler: gcc
+testsuite:check-kernel
+test_range:   "100-"
+
 steps:
 - name: checkout
   uses: actions/checkout@v3
diff --git a/python/test_requirements.txt b/python/test_requirements.txt
index 6aaee13e3..c85ce41ad 100644
--- a/python/test_requirements.txt
+++ b/python/test_requirements.txt
@@ -1,3 +1,5 @@
-pytest
 netaddr
+pyftpdlib
 pyparsing
+pytest
+tftpy

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


[ovs-dev] [PATCH 1/8] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-11-23 Thread Eelco Chaudron
Signed-off-by: Eelco Chaudron 
---
 .ci/linux-build.sh   |9 +
 .github/workflows/build-and-test.yml |3 +++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index aa2ecc505..e9e1e24b5 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -6,6 +6,7 @@ set -x
 CFLAGS_FOR_OVS="-g -O2"
 SPARSE_FLAGS=""
 EXTRA_OPTS="--enable-Werror"
+JOBS=${JOBS:-"-j4"}
 
 function install_dpdk()
 {
@@ -46,7 +47,7 @@ function build_ovs()
 configure_ovs $OPTS
 make selinux-policy
 
-make -j4
+make $JOBS
 }
 
 if [ "$DEB_PACKAGE" ]; then
@@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
 configure_ovs
 
 export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
-make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
-TESTSUITEFLAGS=-j4 RECHECK=yes
+make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
+TESTSUITEFLAGS=$JOBS RECHECK=yes
 else
 build_ovs
 for testsuite in $TESTSUITE; do
@@ -134,7 +135,7 @@ else
 export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
 run_as_root="sudo -E PATH=$PATH"
 fi
-$run_as_root make $testsuite TESTSUITEFLAGS=-j4 RECHECK=yes
+$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
 done
 fi
 
diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 09654205e..5d441157c 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -164,6 +164,9 @@ jobs:
 m32:  m32
 opts: --disable-ssl
 
+  - compiler: gcc
+testsuite:check-ovsdb-cluster
+
 steps:
 - name: checkout
   uses: actions/checkout@v3

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


[ovs-dev] [PATCH 0/8] ci: Add remaining check tests to GitHub actions.

2023-11-23 Thread Eelco Chaudron
This set of patches incorporates the remaining make check tests into
the GitHub action run. Notably, this inclusion does not appear to
extend the overall run time, as the newly added tests complete prior
to the existing ones.

Eelco Chaudron (8):
  ci: Add make check-ovsdb-cluster tests to GitHub action ci.
  ci: Add make check-kernel to GitHub actions ci.
  ci: Add make check-offloads to GitHub actions ci.
  ci: Add make check-system-userspace to GitHub actions ci.
  ci: Add make check-system-tso to GitHub actions ci.
  ci: Allow make check-dpdk to run more tests.
  ci: Add make check-afxdp to GitHub actions ci.
  ci: Exclude tests that show random failures through GitHub actions.


 .ci/dpdk-build.sh|  2 +-
 .ci/linux-build.sh   | 15 ++
 .github/workflows/build-and-test.yml | 39 +---
 python/test_requirements.txt |  5 +++-
 tests/system-common-macros.at|  4 +++
 tests/system-offloads-traffic.at |  2 ++
 tests/system-traffic.at  | 45 +++-
 7 files changed, 80 insertions(+), 32 deletions(-)


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


Re: [ovs-dev] [PATCH ovn v2 3/3] ci: Cover more container posibilities

2023-11-23 Thread Dumitru Ceara
On 11/22/23 14:28, Ales Musil wrote:
> Add more conditions to the image prepare process.
> This allows us to test the prebuilt images for both
> Fedora and Ubuntu.
> 
> Run the weekly image on main branch with the
> use of Fedora for the weekly runs.
> 
> Reported-at: https://issues.redhat.com/browse/FDP-149
> Signed-off-by: Ales Musil 
> ---
>  .github/workflows/test.yml | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
> index fdbc8f5f5..60afcba77 100644
> --- a/.github/workflows/test.yml
> +++ b/.github/workflows/test.yml
> @@ -95,10 +95,25 @@ jobs:
>- name: Install dependencies
>  run: sudo apt install -y ${{ env.DEPENDENCIES }}
>  

Can we add a comment at the top of the workflow file explaining what
runs where and triggered by what event?  It could also be a separate
patch but now that we're changing things it might make sense to properly
document it.

Otherwise, looks good to me, thanks!

> +  - name: Choose image distro
> +if: github.event_name == 'push' || github.event_name == 
> 'pull_request'
> +run: |
> +  echo "IMAGE_DISTRO=ubuntu" >> $GITHUB_ENV
> +
> +  - name: Choose image distro
> +if: github.event_name == 'schedule'
> +run: |
> +  echo "IMAGE_DISTRO=fedora" >> $GITHUB_ENV
> +
>- name: Build container
> -run: make ubuntu
> +if: github.ref_name != 'main'
> +run: make ${{ env.IMAGE_DISTRO }}
>  working-directory: utilities/containers
>  
> +  - name: Download container
> +if: github.ref_name == 'main'
> +run: podman pull ghcr.io/ovn-org/ovn-tests:${{ env.IMAGE_DISTRO }}
> +
>- name: Export image
>  run: podman save -o /tmp/image.tar ovn-org/ovn-tests
>  

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


Re: [ovs-dev] [PATCH ovn v2 2/3] ci: Build container image before very job

2023-11-23 Thread Dumitru Ceara
On 11/22/23 14:28, Ales Musil wrote:
> Build the image before every job to allow more
> fine-grained dependency pinning. This is especially
> useful for stable branches as they might need to stay
> on specific distribution or Python version.
> 
> Signed-off-by: Ales Musil 
> ---
>  .cirrus.yml| 40 -
>  .github/workflows/test.yml | 46 +++---
>  2 files changed, 77 insertions(+), 9 deletions(-)
> 
> diff --git a/.cirrus.yml b/.cirrus.yml
> index 71e9afbb6..894f953f3 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -1,9 +1,25 @@
> -arm_unit_tests_task:
> +compute_engine_instance:
> +  image_project: ubuntu-os-cloud
> +  image: family/ubuntu-2304-amd64
> +  platform: linux
> +  memory: 4G
> +
> +build_image_task:
> +  install_dependencies_script:
> +- sudo apt update
> +- sudo apt install -y podman make
> +
> +  build_container_script:
> +- cd utilities/containers
> +- make ubuntu
> +- podman save -o /tmp/image.tar ovn-org/ovn-tests:ubuntu
> +
> +  upload_image_script:
> +- curl -s -X POST -T /tmp/image.tar 
> http://$CIRRUS_HTTP_CACHE_HOST/${CIRRUS_CHANGE_IN_REPO}

This seemed a bit strange initially but I understand now (thanks for the
offline clarification) that it's because we want to avoid building for
each instance of the matrix in the unit test task.

I think a comment explaining that here would be good as others might
also wonder why we need the cache.

Thanks,
Dumitru

>  
> -  arm_container:
> -image: ghcr.io/ovn-org/ovn-tests:ubuntu
> -memory: 4G
> -cpu: 2
> +arm_unit_tests_task:
> +  depends_on:
> +- build_image
>  
>env:
>  ARCH: aarch64
> @@ -11,6 +27,7 @@ arm_unit_tests_task:
>  PATH: ${HOME}/bin:${HOME}/.local/bin:${PATH}
>  RECHECK: yes
>  LANG: C
> +IMAGE_NAME: ovn-org/ovn-tests:ubuntu
>  matrix:
>- CC: gcc
>  TESTSUITE: test
> @@ -35,5 +52,16 @@ arm_unit_tests_task:
>  
>name: ARM64 ${CC} ${TESTSUITE} ${TEST_RANGE}
>  
> +  install_dependencies_script:
> +- sudo apt update
> +- sudo apt install -y podman
> +
> +  download_cache_script:
> +- curl http://$CIRRUS_HTTP_CACHE_HOST/${CIRRUS_CHANGE_IN_REPO} -o 
> /tmp/image.tar
> +
> +  load_image_script:
> +- podman load -i /tmp/image.tar
> +- rm -rf /tmp/image.tar
> +
>build_script:
> -- ./.ci/linux-build.sh
> +- ./.ci/ci.sh --archive-logs
> diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
> index b3fcb57fc..fdbc8f5f5 100644
> --- a/.github/workflows/test.yml
> +++ b/.github/workflows/test.yml
> @@ -80,10 +80,38 @@ jobs:
>if: steps.dpdk_cache.outputs.cache-hit != 'true'
>run: ./.ci/dpdk-build.sh
>  
> +  prepare-container:
> +env:
> +  DEPENDENCIES: podman
> +name: Prepare container
> +runs-on: ubuntu-22.04
> +
> +steps:
> +  - uses: actions/checkout@v3
> +
> +  - name: Update APT cache
> +run: sudo apt update
> +
> +  - name: Install dependencies
> +run: sudo apt install -y ${{ env.DEPENDENCIES }}
> +
> +  - name: Build container
> +run: make ubuntu
> +working-directory: utilities/containers
> +
> +  - name: Export image
> +run: podman save -o /tmp/image.tar ovn-org/ovn-tests
> +
> +  - name: Cache image
> +id: image_cache
> +uses: actions/cache@v3
> +with:
> +  path: /tmp/image.tar
> +  key: ${{ github.sha }}
> +
>build-linux:
> -needs: build-dpdk
> +needs: [build-dpdk, prepare-container]
>  env:
> -  IMAGE_NAME:  ghcr.io/ovn-org/ovn-tests:ubuntu
>ARCH:${{ matrix.cfg.arch }}
>CC:  ${{ matrix.cfg.compiler }}
>DPDK:${{ matrix.cfg.dpdk }}
> @@ -152,13 +180,25 @@ jobs:
>  sort -V | tail -1)
>working-directory: ovs
>  
> -- name: cache
> +- name: cache dpdk
>if: matrix.cfg.dpdk != ''
>uses: actions/cache@v3
>with:
>  path: dpdk-dir
>  key: ${{ needs.build-dpdk.outputs.dpdk_key }}
>  
> +- name: image cache
> +  uses: actions/cache@v3
> +  with:
> +path: /tmp/image.tar
> +key: ${{ github.sha }}
> +
> +- name: load image
> +  run: |
> +sudo podman load -i /tmp/image.tar
> +podman load -i /tmp/image.tar
> +rm -rf /tmp/image.tar
> +
>  - name: build
>if: ${{ startsWith(matrix.cfg.testsuite, 'system-test') }}
>run: sudo -E ./.ci/ci.sh --archive-logs

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


Re: [ovs-dev] [PATCH ovn v2 1/3] ci: Switch Cirrus CI to the Ubuntu image

2023-11-23 Thread Dumitru Ceara
On 11/22/23 14:28, Ales Musil wrote:
> Switch the Cirrus CI to use the Ubuntu image,
> so it is unified with the GH actions. That is
> useful for various dependency pinnings.
> 
> Signed-off-by: Ales Musil 
> ---

Hi Ales,

I think I'd just remove this patch.  We don't use the container image in
the same way starting with patch 2/3.

Regards,
Dumitru

>  .cirrus.yml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/.cirrus.yml b/.cirrus.yml
> index 2d3824972..71e9afbb6 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -1,7 +1,7 @@
>  arm_unit_tests_task:
>  
>arm_container:
> -image: ghcr.io/ovn-org/ovn-tests:fedora
> +image: ghcr.io/ovn-org/ovn-tests:ubuntu
>  memory: 4G
>  cpu: 2
>  
> @@ -10,6 +10,7 @@ arm_unit_tests_task:
>  CIRRUS_CLONE_SUBMODULES: true
>  PATH: ${HOME}/bin:${HOME}/.local/bin:${PATH}
>  RECHECK: yes
> +LANG: C
>  matrix:
>- CC: gcc
>  TESTSUITE: test

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