Re: [ovs-dev] [PATCH ovn] tests: Fix flaky test 'SB Port binding incremental processing'

2024-01-31 Thread 0-day Robot
Bleep bloop.  Greetings Numan Siddique, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: The subject summary should end with a dot.
Subject: tests: Fix flaky test 'SB Port binding incremental processing'
ERROR: "Fixes" tag is malformed.
Use the following format:
  git log -1 --pretty=format:"Fixes: %h (\"%s\")" --abbrev=12 COMMIT_REF

5: Fixes: 121661678317 ("northd: Move router ports SB PB options sync to

Lines checked: 30, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH ovn v2] tests: Fix flaky test 'SB Port binding incremental processing'.

2024-01-31 Thread numans
From: Numan Siddique 

Fixes: 121661678317 ("northd: Move router ports SB PB options sync to 
sync_to_sb_pb node.")

Signed-off-by: Numan Siddique 
---

v1 -> v2
-
   * Fix the checkpatch errors.

 tests/ovn-northd.at | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 67e81ddba3..fbd0d9ef18 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -10404,6 +10404,8 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
 # Make lrp a gateway port
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb lrp-set-gateway-chassis lrp hv1
+wait_column "hosting-chassis=hv1" nb:Logical_Router_Port status name=lrp
+
 # There will be 3 recomputes of northd engine node
 #   1. missing handler for input NB_logical_router
 #   2. missing handler for input SB_ha_chassis_group
-- 
2.43.0

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


[ovs-dev] [PATCH ovn] tests: Fix flaky test 'SB Port binding incremental processing'

2024-01-31 Thread numans
From: Numan Siddique 

Fixes: 121661678317 ("northd: Move router ports SB PB options sync to
sync_to_sb_pb node.")

Signed-off-by: Numan Siddique 
---
 tests/ovn-northd.at | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 67e81ddba3..fbd0d9ef18 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -10404,6 +10404,8 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
 # Make lrp a gateway port
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb lrp-set-gateway-chassis lrp hv1
+wait_column "hosting-chassis=hv1" nb:Logical_Router_Port status name=lrp
+
 # There will be 3 recomputes of northd engine node
 #   1. missing handler for input NB_logical_router
 #   2. missing handler for input SB_ha_chassis_group
-- 
2.43.0

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


Re: [ovs-dev] [PATCH ovn] northd: Added lb_vip_mac config option in Logical_Switch.

2024-01-31 Thread Numan Siddique
On Mon, Jan 8, 2024 at 8:13 AM Priyankar Jain
 wrote:
>
> Currently load balancer applied to a logical switch has the
> following restriction:
> - VIP of the load balancer cannot reside in the subnet prefix as the
>   clients as OVN does not install ARP responder flows for the LB VIP.
>

Hi Priyankar,

Sorry for the late reviews.

The above statement is actually not correct.  OVN does allow the VIP
of the load balancer
to be from the same subnet prefix.   But in order for this to work,
this logical switch
has to be associated with a logical router.

Eg.
--
ovn-nbctl ls-add sw0

ovn-nbctl lsp-add sw0 sw0-p1
ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3"
ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3"

ovn-nbctl lr-add lr0
ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
ovn-nbctl lsp-add sw0 sw0-lr0
ovn-nbctl lsp-set-type sw0-lr0 router
ovn-nbctl lsp-set-addresses sw0-lr0 router
ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0

ovn-nbctl lb-add lb1 10.0.0.10:80 10.0.0.3:80,20.0.0.3:80
ovn-nbctl --wait=sb ls-lb-add sw0 lb1

# This will work once you do
ovn-nbctl lr-lb-add lr0 lb1
---

Do you have a use case where you attach a load balancer to a logical switch
and this logical switch doesn't connect to any logical router ?   If
so, then perhaps we can consider
this patch.  However if you always attach a logical router to a
logical switch (like how its
done in the test case added in this patch),  then just attaching the
lb to the router would do.

Can you please confirm if this is sufficient for your use case ?
Perhaps we should document it in OVN :)

Thanks
Numan


> This change adds a new config option "lb_vip_mac" in the logical_switch
> table which is expected to be a MAC address. If the logical_switch has
> this option configured, northd will program an ARP responder flow for
> all the LB VIPs of the logical_switch with this MAC address.
>
> Usecase: With this change, CMS can set the lb_vip_mac value to same as
> the default gateway MAC. This allows CMS to allocate VIP of the Load
> balancer from any subnet prefix.
>
> Signed-off-by: Priyankar Jain 




> ---
>  northd/northd.c |  71 ++
>  northd/northd.h |   2 +
>  northd/ovn-northd.8.xml |  49 ++
>  tests/ovn.at| 109 
>  4 files changed, 231 insertions(+)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index db3cd272e..ebca2c073 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -790,8 +790,11 @@ init_lb_for_datapath(struct ovn_datapath *od)
>  {
>  if (od->nbs) {
>  od->has_lb_vip = ls_has_lb_vip(od);
> +od->lb_vip_mac = nullable_xstrdup(
> +smap_get(>nbs->other_config, "lb_vip_mac"));
>  } else {
>  od->has_lb_vip = lr_has_lb_vip(od);
> +od->lb_vip_mac = NULL;
>  }
>  }
>
> @@ -800,6 +803,9 @@ destroy_lb_for_datapath(struct ovn_datapath *od)
>  {
>  ovn_lb_ip_set_destroy(od->lb_ips);
>  od->lb_ips = NULL;
> +
> +free(od->lb_vip_mac);
> +od->lb_vip_mac = NULL;
>  }
>
>  /* A group of logical router datapaths which are connected - either
> @@ -12204,6 +12210,70 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
> *lb_vip,
>  }
>  }
>
> +static void
> +build_lb_rules_arp_nd_rsp(struct hmap *lflows, struct ovn_lb_datapaths 
> *lb_dps,
> +  const struct ovn_datapaths *ls_datapaths,
> +  struct ds *match, struct ds *actions)
> +{
> +if (!lb_dps->n_nb_ls) {
> +return;
> +}
> +
> +const struct ovn_northd_lb *lb = lb_dps->lb;
> +for (size_t i = 0; i < lb->n_vips; i++) {
> +struct ovn_lb_vip *lb_vip = >vips[i];
> +
> +size_t index;
> +BITMAP_FOR_EACH_1 (index, ods_size(ls_datapaths), lb_dps->nb_ls_map) 
> {
> +struct ovn_datapath *od = ls_datapaths->array[index];
> +if (!od->lb_vip_mac) {
> +  continue;
> +}
> +ds_clear(match);
> +ds_clear(actions);
> +if (IN6_IS_ADDR_V4MAPPED(_vip->vip)) {
> +ds_put_format(match, "arp.tpa == %s && arp.op == 1",
> +  lb_vip->vip_str);
> +ds_put_format(actions,
> +"eth.dst = eth.src; "
> +"eth.src = %s; "
> +"arp.op = 2; /* ARP reply */ "
> +"arp.tha = arp.sha; "
> +"arp.sha = %s; "
> +"arp.tpa = arp.spa; "
> +"arp.spa = %s; "
> +"outport = inport; "
> +"flags.loopback = 1; "
> +"output;",
> +od->lb_vip_mac, od->lb_vip_mac,
> +lb_vip->vip_str);
> +} else {
> +ds_put_format(match, "nd_ns && nd.target == %s",
> + 

Re: [ovs-dev] [PATCH] github: Bump Fedora version to 39.

2024-01-31 Thread Ilya Maximets
On 1/30/24 10:55, Simon Horman wrote:
> On Mon, Jan 29, 2024 at 11:33:56PM +0100, Ilya Maximets wrote:
>> Fedora 37 reached EOL in November.  Switch to the most recent version
>> to avoid potential CI failures in the future.
>>
>> Signed-off-by: Ilya Maximets 
> 
> Acked-by: Simon Horman 
> 

Thanks, Simon and Eelco!

Applied to branches down to 3.2.  We do not have fedora builds in CI
in older branches.

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


Re: [ovs-dev] [PATCH v8 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

2024-01-31 Thread Aaron Conole
Eelco Chaudron  writes:

> On 25 Jan 2024, at 21:55, Aaron Conole wrote:
>
>> From: Kevin Sprague 
>>
>> During normal operations, it is useful to understand when a particular flow
>> gets removed from the system. This can be useful when debugging performance
>> issues tied to ofproto flow changes, trying to determine deployed traffic
>> patterns, or while debugging dynamic systems where ports come and go.
>>
>> Prior to this change, there was a lack of visibility around flow expiration.
>> The existing debugging infrastructure could tell us when a flow was added to
>> the datapath, but not when it was removed or why.
>>
>> This change introduces a USDT probe at the point where the revalidator
>> determines that the flow should be removed.  Additionally, we track the
>> reason for the flow eviction and provide that information as well.  With
>> this change, we can track the complete flow lifecycle for the netlink
>> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and
>> the revaldiator USDT, letting us watch as flows are added and removed from
>> the kernel datapath.
>>
>> This change only enables this information via USDT probe, so it won't be
>> possible to access this information any other way (see:
>> Documentation/topics/usdt-probes.rst).
>>
>> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
>> which serves as a demonstration of how the new USDT probe might be used
>> going forward.
>>
>> Signed-off-by: Kevin Sprague 
>> Co-authored-by: Aaron Conole 
>> Signed-off-by: Aaron Conole 
>
> Thanks for following this up Aaron! See comments on this patch below. I have 
> no additional comments on patch 2.
>
> Cheers,
>
> Eelco
>
>
>> ---
>>  Documentation/topics/usdt-probes.rst |   1 +
>>  ofproto/ofproto-dpif-upcall.c|  42 +-
>>  utilities/automake.mk|   3 +
>>  utilities/usdt-scripts/flow_reval_monitor.py | 653 +++
>>  4 files changed, 693 insertions(+), 6 deletions(-)
>>  create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py
>>
>> diff --git a/Documentation/topics/usdt-probes.rst 
>> b/Documentation/topics/usdt-probes.rst
>> index e527f43bab..a8da9bb1f7 100644
>> --- a/Documentation/topics/usdt-probes.rst
>> +++ b/Documentation/topics/usdt-probes.rst
>> @@ -214,6 +214,7 @@ Available probes in ``ovs_vswitchd``:
>>  - dpif_recv:recv_upcall
>>  - main:poll_block
>>  - main:run_start
>> +- revalidate:flow_result
>>  - revalidate_ukey\_\_:entry
>>  - revalidate_ukey\_\_:exit
>>  - udpif_revalidator:start_dump
>
> You are missing the specific flow_result result section. This is from the 
> previous patch:

D'oh!  Thanks for catching it.  I'll re-add it.

> @@ -358,6 +360,27 @@  See also the ``main:run_start`` probe above.
>  - ``utilities/usdt-scripts/bridge_loop.bt``
>
>
> +probe revalidate:flow_result
> +~
> +
> +**Description**:
> +This probe is triggered when the revalidator decides whether or not to
> +revalidate a flow. ``reason`` is an enum that denotes that either the flow
> +is being kept, or the reason why the flow is being deleted. The
> +``flow_reval_monitor.py`` script uses this probe to notify users when flows
> +matching user-provided criteria are deleted.
> +
> +**Arguments**:
> +
> +- *arg0*: ``(enum flow_del_reason) reason``
> +- *arg1*: ``(struct udpif *) udpif``
> +- *arg2*: ``(struct udpif_key *) ukey``
> +
> +**Script references**:
> +
> +- ``utilities/usdt-scripts/flow_reval_monitor.py``
> +
> +
>  Adding your own probes
>  --
>
>> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>> index b5cbeed878..97d75833f7 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -269,6 +269,18 @@ enum ukey_state {
>>  };
>>  #define N_UKEY_STATES (UKEY_DELETED + 1)
>>
>> +enum flow_del_reason {
>> +FDR_REVALIDATE = 0, /* The flow was revalidated. */
>
> It was called FDR_FLOW_LIVE before, which might make more sense. As the flow 
> is just NOT deleted. It might or might not have been revalidated. Thoughts?

I think it had to have been revalidated if we emit the reason, because
we only emit the reason code after revalidation.  IE: there are many
places where we skip revalidation but the flow stays live - and we don't
emit reasons in those cases.

So at least for this patch, it MUST have been revalidated.  But maybe in
the future, we would want to catch cases where the flow hasn't been.  In
that case, it makes sense to add the FDR_FLOW_LIVE at that time - I
think.

Maybe you disagree?

>> +FDR_FLOW_IDLE,  /* The flow went unused and was deleted. */
>> +FDR_TOO_EXPENSIVE,  /* The flow was too expensive to revalidate. */
>> +FDR_FLOW_WILDCARDED,/* The flow needed a narrower wildcard mask. */
>> +FDR_BAD_ODP_FIT,/* The flow had a bad ODP flow fit. */
>> +FDR_NO_OFPROTO, /* The flow didn't have an associated ofproto. 

Re: [ovs-dev] [PATCH ovn] github: Update versions of action dependencies (Node.js 20).

2024-01-31 Thread Ilya Maximets
On 1/31/24 14:27, Ilya Maximets wrote:
> On 1/31/24 13:13, Ilya Maximets wrote:
>> On 1/30/24 15:18, Ilya Maximets wrote:
>>> On 1/30/24 13:07, Ilya Maximets wrote:
 checkout@v3, cache@v3, setup-python@v4 and setup-go@v3 are using
 outdated Node.js 16 which is now deprecated in GHA [1], so these
 actions may stop working soon.

 Updating to most recent major versions with Node.js 20.  This stops
 GHA from throwing warnings in every build.

 [1] 
 https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/

 While at it also updating upload-artifact and download-artifact to
 the latest versions.

 Removing versions from the upload-artifact comment, since the
 behavior doesn't seem to change much between versions.

 New setup-go@v5 attempts to cache dependencies by default.  However,
 the default path it uses is go.sum in the root directory.  This
 triggers a warning, since the file doesn't exist:

   Restore cache failed: Dependencies file is not found in
   /home/runner/work/ovn-kubernetes/ovn-kubernetes.
   Supported file pattern: go.sum

 Specify a path to all .sum files we have in the repository to make
 the setup-go happy.  This should in theory make the builds a touch
 faster.  This change is in line with recent changes in ovn-kubernetes
 itself.

 Signed-off-by: Ilya Maximets 
 ---
  .github/workflows/containers.yml  |  2 +-
  .../workflows/ovn-fake-multinode-tests.yml| 26 +++---
  .github/workflows/ovn-kubernetes.yml  | 19 +-
  .github/workflows/test.yml| 36 +--
  4 files changed, 42 insertions(+), 41 deletions(-)
>>>
>>> Recheck-request: github-robot
>>
>> Recheck-request: github-robot
> 
> Let's try specific re-checks.  The global one doesn't actually re-check the
> workflow that I wanted to re-check.
> 
> Recheck-request: github-robot-_Build_and_Test

And the other one that failed after 2 successful re-runs. :)

Recheck-request: github-robot-_ovn-kubernetes
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net-next] selftests: openvswitch: Test ICMP related matches work with SNAT

2024-01-31 Thread Aaron Conole
Brad Cowie  writes:

> Add a test case for regression in openvswitch nat that was fixed by
> commit e6345d2824a3 ("netfilter: nf_nat: fix action not being set for
> all ct states").
>
> Link: https://lore.kernel.org/netdev/20231221224311.130319-1-b...@faucet.nz/
> Link: https://mail.openvswitch.org/pipermail/ovs-dev/2024-January/410476.html
> Suggested-by: Aaron Conole 
> Signed-off-by: Brad Cowie 
> ---

I tested this on a patched kernel and as well as an unpatched kernel and
got the following:

6.5.5-200: TEST: ip4-nat-related: ICMP related matches work with SNAT  
[FAIL]
6.7.0: TEST: ip4-nat-related: ICMP related matches work with SNAT  
[ OK ]

Thanks for adding the test case!

Tested-by: Aaron Conole 
Acked-by: Aaron Conole 

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


Re: [ovs-dev] [PATCH v2 3/4] dp-packet: Include inner offsets in adjustments and checks.

2024-01-31 Thread Mike Pattrick
On Wed, Jan 31, 2024 at 10:04 AM David Marchand
 wrote:
>
> On Tue, Jan 30, 2024 at 11:15 PM Mike Pattrick  wrote:
> >
> > Include inner offsets in functions where l3 and l4 offsets are either
> > modified or checked.
> >
> > Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
> > Signed-off-by: Mike Pattrick 
> > ---
> > v2:
> >
> >  - Prints out new offsets in autovalidator
> >  - Extends resize_l2 change to avx512
> >
> > Signed-off-by: Mike Pattrick 
> > ---
> >  lib/dp-packet.c  | 18 +-
> >  lib/odp-execute-avx512.c | 19 ++-
> >  2 files changed, 27 insertions(+), 10 deletions(-)
> >
> > diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> > index 0e23c766e..640b1dfeb 100644
> > --- a/lib/dp-packet.c
> > +++ b/lib/dp-packet.c
> > @@ -507,6 +507,8 @@ dp_packet_resize_l2_5(struct dp_packet *b, int 
> > increment)
> >  /* Adjust layer offsets after l2_5. */
> >  dp_packet_adjust_layer_offset(>l3_ofs, increment);
> >  dp_packet_adjust_layer_offset(>l4_ofs, increment);
> > +dp_packet_adjust_layer_offset(>inner_l3_ofs, increment);
> > +dp_packet_adjust_layer_offset(>inner_l4_ofs, increment);
> >
> >  return dp_packet_data(b);
> >  }
> > @@ -529,17 +531,23 @@ dp_packet_compare_offsets(struct dp_packet *b1, 
> > struct dp_packet *b2,
> >  if ((b1->l2_pad_size != b2->l2_pad_size) ||
> >  (b1->l2_5_ofs != b2->l2_5_ofs) ||
> >  (b1->l3_ofs != b2->l3_ofs) ||
> > -(b1->l4_ofs != b2->l4_ofs)) {
> > +(b1->l4_ofs != b2->l4_ofs) ||
> > +(b1->inner_l3_ofs != b2->inner_l3_ofs) ||
> > +(b1->inner_l4_ofs != b2->inner_l4_ofs)) {
> >  if (err_str) {
> >  ds_put_format(err_str, "Packet offset comparison failed\n");
> >  ds_put_format(err_str, "Buffer 1 offsets: l2_pad_size %u,"
> > -  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
> > +  " l2_5_ofs : %u l3_ofs %u, inner_l3_ofs %u,"
> > +  " l4_ofs %u, inner_l4_ofs %u\n",
> >b1->l2_pad_size, b1->l2_5_ofs,
> > -  b1->l3_ofs, b1->l4_ofs);
> > +  b1->l3_ofs, b1->inner_l3_ofs,
> > +  b1->l4_ofs, b1->inner_l4_ofs);
> >  ds_put_format(err_str, "Buffer 2 offsets: l2_pad_size %u,"
> > -  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
> > +  " l2_5_ofs : %u l3_ofs %u, inner_l3_ofs %u,"
> > +  " l4_ofs %u, inner_l4_ofs %u\n",
> >b2->l2_pad_size, b2->l2_5_ofs,
> > -  b2->l3_ofs, b2->l4_ofs);
> > +  b2->l3_ofs, b2->inner_l3_ofs,
> > +  b2->l4_ofs, b2->inner_l4_ofs);
> >  }
> >  return false;
> >  }
>
> Not a strong opinion, but I prefer keeping those offsets in the same
> order than a real packet layout, rather than mix l3 / l4 outer/inner
> offsets.
> IOW:
> -  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
> +  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u,"
> +  " inner_l3_ofs : %u, inner_l4_ofs %u\n",
>
>
> > diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
> > index 747e04014..7f9870669 100644
> > --- a/lib/odp-execute-avx512.c
> > +++ b/lib/odp-execute-avx512.c
> > @@ -35,10 +35,11 @@
> >
> >  VLOG_DEFINE_THIS_MODULE(odp_execute_avx512);
> >
> > -/* The below three build asserts make sure that l2_5_ofs, l3_ofs, and 
> > l4_ofs
> > - * fields remain in the same order and offset to l2_padd_size. This is 
> > needed
> > - * as the avx512_dp_packet_resize_l2() function will manipulate those 
> > fields at
> > - * a fixed memory index based on the l2_padd_size offset. */
> > +/* The below three build asserts make sure that l2_5_ofs, l3_ofs, l4_ofs,
>
> Counting build asserts is useless in a comment.. and here it gets
> wrong after the change.
> I suggest a simple: "The below build asserts".
>
>
> > + * inner_l3_ofs, and inner_l4_ofs fields remain in the same order and 
> > offset to
> > + * l2_padd_size. This is needed as the avx512_dp_packet_resize_l2() 
> > function
>
> l2_pad_size*
>
>
> > + * will manipulate those fields at a fixed memory index based on the
> > + * l2_padd_size offset. */
>
> Idem.
>
>
> >  BUILD_ASSERT_DECL(offsetof(struct dp_packet, l2_pad_size) +
> >MEMBER_SIZEOF(struct dp_packet, l2_pad_size) ==
> >offsetof(struct dp_packet, l2_5_ofs));
> > @@ -51,6 +52,14 @@ BUILD_ASSERT_DECL(offsetof(struct dp_packet, l3_ofs) +
> > MEMBER_SIZEOF(struct dp_packet, l3_ofs) ==
> > offsetof(struct dp_packet, l4_ofs));
> >
> > +BUILD_ASSERT_DECL(offsetof(struct dp_packet, l4_ofs) +
> > +   MEMBER_SIZEOF(struct dp_packet, l4_ofs) ==
> > +   offsetof(struct 

Re: [ovs-dev] [PATCH v8 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

2024-01-31 Thread Eelco Chaudron


On 25 Jan 2024, at 21:55, Aaron Conole wrote:

> From: Kevin Sprague 
>
> During normal operations, it is useful to understand when a particular flow
> gets removed from the system. This can be useful when debugging performance
> issues tied to ofproto flow changes, trying to determine deployed traffic
> patterns, or while debugging dynamic systems where ports come and go.
>
> Prior to this change, there was a lack of visibility around flow expiration.
> The existing debugging infrastructure could tell us when a flow was added to
> the datapath, but not when it was removed or why.
>
> This change introduces a USDT probe at the point where the revalidator
> determines that the flow should be removed.  Additionally, we track the
> reason for the flow eviction and provide that information as well.  With
> this change, we can track the complete flow lifecycle for the netlink
> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and
> the revaldiator USDT, letting us watch as flows are added and removed from
> the kernel datapath.
>
> This change only enables this information via USDT probe, so it won't be
> possible to access this information any other way (see:
> Documentation/topics/usdt-probes.rst).
>
> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
> which serves as a demonstration of how the new USDT probe might be used
> going forward.
>
> Signed-off-by: Kevin Sprague 
> Co-authored-by: Aaron Conole 
> Signed-off-by: Aaron Conole 

Thanks for following this up Aaron! See comments on this patch below. I have no 
additional comments on patch 2.

Cheers,

Eelco


> ---
>  Documentation/topics/usdt-probes.rst |   1 +
>  ofproto/ofproto-dpif-upcall.c|  42 +-
>  utilities/automake.mk|   3 +
>  utilities/usdt-scripts/flow_reval_monitor.py | 653 +++
>  4 files changed, 693 insertions(+), 6 deletions(-)
>  create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py
>
> diff --git a/Documentation/topics/usdt-probes.rst 
> b/Documentation/topics/usdt-probes.rst
> index e527f43bab..a8da9bb1f7 100644
> --- a/Documentation/topics/usdt-probes.rst
> +++ b/Documentation/topics/usdt-probes.rst
> @@ -214,6 +214,7 @@ Available probes in ``ovs_vswitchd``:
>  - dpif_recv:recv_upcall
>  - main:poll_block
>  - main:run_start
> +- revalidate:flow_result
>  - revalidate_ukey\_\_:entry
>  - revalidate_ukey\_\_:exit
>  - udpif_revalidator:start_dump

You are missing the specific flow_result result section. This is from the 
previous patch:

@@ -358,6 +360,27 @@  See also the ``main:run_start`` probe above.
 - ``utilities/usdt-scripts/bridge_loop.bt``


+probe revalidate:flow_result
+~
+
+**Description**:
+This probe is triggered when the revalidator decides whether or not to
+revalidate a flow. ``reason`` is an enum that denotes that either the flow
+is being kept, or the reason why the flow is being deleted. The
+``flow_reval_monitor.py`` script uses this probe to notify users when flows
+matching user-provided criteria are deleted.
+
+**Arguments**:
+
+- *arg0*: ``(enum flow_del_reason) reason``
+- *arg1*: ``(struct udpif *) udpif``
+- *arg2*: ``(struct udpif_key *) ukey``
+
+**Script references**:
+
+- ``utilities/usdt-scripts/flow_reval_monitor.py``
+
+
 Adding your own probes
 --

> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index b5cbeed878..97d75833f7 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -269,6 +269,18 @@ enum ukey_state {
>  };
>  #define N_UKEY_STATES (UKEY_DELETED + 1)
>
> +enum flow_del_reason {
> +FDR_REVALIDATE = 0, /* The flow was revalidated. */

It was called FDR_FLOW_LIVE before, which might make more sense. As the flow is 
just NOT deleted. It might or might not have been revalidated. Thoughts?

> +FDR_FLOW_IDLE,  /* The flow went unused and was deleted. */
> +FDR_TOO_EXPENSIVE,  /* The flow was too expensive to revalidate. */
> +FDR_FLOW_WILDCARDED,/* The flow needed a narrower wildcard mask. */
> +FDR_BAD_ODP_FIT,/* The flow had a bad ODP flow fit. */
> +FDR_NO_OFPROTO, /* The flow didn't have an associated ofproto. */
> +FDR_XLATION_ERROR,  /* There was an error translating the flow. */
> +FDR_AVOID_CACHING,  /* Flow deleted to avoid caching. */
> +FDR_FLOW_LIMIT, /* All flows being killed. */

Looking at the comment from Han on FDR_PURGE, and this patch needing another 
spin, we should probably add it.

> +};
> +
>  /* 'udpif_key's are responsible for tracking the little bit of state udpif
>   * needs to do flow expiration which can't be pulled directly from the
>   * datapath.  They may be created by any handler or revalidator thread at any
> @@ -2272,7 +2284,8 @@ populate_xcache(struct udpif *udpif, struct udpif_key 
> *ukey,
>  static enum reval_result
>  

Re: [ovs-dev] [PATCH v2 4/4] ofproto-dpif-monitor: Remove unneeded calls to clear packets.

2024-01-31 Thread David Marchand
On Tue, Jan 30, 2024 at 11:15 PM Mike Pattrick  wrote:
>
> Currently the monitor will call dp_packet_clear() on the dp_packet that
> is shared amongst BFD, LLDP, and CFM. However, all of these packets are
> created with eth_compose(), which already calls dp_packet_clear().
>
> Signed-off-by: Mike Pattrick 

Reviewed-by: David Marchand 


-- 
David Marchand

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


Re: [ovs-dev] [PATCH v2 3/4] dp-packet: Include inner offsets in adjustments and checks.

2024-01-31 Thread David Marchand
On Tue, Jan 30, 2024 at 11:15 PM Mike Pattrick  wrote:
>
> Include inner offsets in functions where l3 and l4 offsets are either
> modified or checked.
>
> Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
> Signed-off-by: Mike Pattrick 
> ---
> v2:
>
>  - Prints out new offsets in autovalidator
>  - Extends resize_l2 change to avx512
>
> Signed-off-by: Mike Pattrick 
> ---
>  lib/dp-packet.c  | 18 +-
>  lib/odp-execute-avx512.c | 19 ++-
>  2 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index 0e23c766e..640b1dfeb 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -507,6 +507,8 @@ dp_packet_resize_l2_5(struct dp_packet *b, int increment)
>  /* Adjust layer offsets after l2_5. */
>  dp_packet_adjust_layer_offset(>l3_ofs, increment);
>  dp_packet_adjust_layer_offset(>l4_ofs, increment);
> +dp_packet_adjust_layer_offset(>inner_l3_ofs, increment);
> +dp_packet_adjust_layer_offset(>inner_l4_ofs, increment);
>
>  return dp_packet_data(b);
>  }
> @@ -529,17 +531,23 @@ dp_packet_compare_offsets(struct dp_packet *b1, struct 
> dp_packet *b2,
>  if ((b1->l2_pad_size != b2->l2_pad_size) ||
>  (b1->l2_5_ofs != b2->l2_5_ofs) ||
>  (b1->l3_ofs != b2->l3_ofs) ||
> -(b1->l4_ofs != b2->l4_ofs)) {
> +(b1->l4_ofs != b2->l4_ofs) ||
> +(b1->inner_l3_ofs != b2->inner_l3_ofs) ||
> +(b1->inner_l4_ofs != b2->inner_l4_ofs)) {
>  if (err_str) {
>  ds_put_format(err_str, "Packet offset comparison failed\n");
>  ds_put_format(err_str, "Buffer 1 offsets: l2_pad_size %u,"
> -  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
> +  " l2_5_ofs : %u l3_ofs %u, inner_l3_ofs %u,"
> +  " l4_ofs %u, inner_l4_ofs %u\n",
>b1->l2_pad_size, b1->l2_5_ofs,
> -  b1->l3_ofs, b1->l4_ofs);
> +  b1->l3_ofs, b1->inner_l3_ofs,
> +  b1->l4_ofs, b1->inner_l4_ofs);
>  ds_put_format(err_str, "Buffer 2 offsets: l2_pad_size %u,"
> -  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
> +  " l2_5_ofs : %u l3_ofs %u, inner_l3_ofs %u,"
> +  " l4_ofs %u, inner_l4_ofs %u\n",
>b2->l2_pad_size, b2->l2_5_ofs,
> -  b2->l3_ofs, b2->l4_ofs);
> +  b2->l3_ofs, b2->inner_l3_ofs,
> +  b2->l4_ofs, b2->inner_l4_ofs);
>  }
>  return false;
>  }

Not a strong opinion, but I prefer keeping those offsets in the same
order than a real packet layout, rather than mix l3 / l4 outer/inner
offsets.
IOW:
-  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
+  " l2_5_ofs : %u l3_ofs %u, l4_ofs %u,"
+  " inner_l3_ofs : %u, inner_l4_ofs %u\n",


> diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
> index 747e04014..7f9870669 100644
> --- a/lib/odp-execute-avx512.c
> +++ b/lib/odp-execute-avx512.c
> @@ -35,10 +35,11 @@
>
>  VLOG_DEFINE_THIS_MODULE(odp_execute_avx512);
>
> -/* The below three build asserts make sure that l2_5_ofs, l3_ofs, and l4_ofs
> - * fields remain in the same order and offset to l2_padd_size. This is needed
> - * as the avx512_dp_packet_resize_l2() function will manipulate those fields 
> at
> - * a fixed memory index based on the l2_padd_size offset. */
> +/* The below three build asserts make sure that l2_5_ofs, l3_ofs, l4_ofs,

Counting build asserts is useless in a comment.. and here it gets
wrong after the change.
I suggest a simple: "The below build asserts".


> + * inner_l3_ofs, and inner_l4_ofs fields remain in the same order and offset 
> to
> + * l2_padd_size. This is needed as the avx512_dp_packet_resize_l2() function

l2_pad_size*


> + * will manipulate those fields at a fixed memory index based on the
> + * l2_padd_size offset. */

Idem.


>  BUILD_ASSERT_DECL(offsetof(struct dp_packet, l2_pad_size) +
>MEMBER_SIZEOF(struct dp_packet, l2_pad_size) ==
>offsetof(struct dp_packet, l2_5_ofs));
> @@ -51,6 +52,14 @@ BUILD_ASSERT_DECL(offsetof(struct dp_packet, l3_ofs) +
> MEMBER_SIZEOF(struct dp_packet, l3_ofs) ==
> offsetof(struct dp_packet, l4_ofs));
>
> +BUILD_ASSERT_DECL(offsetof(struct dp_packet, l4_ofs) +
> +   MEMBER_SIZEOF(struct dp_packet, l4_ofs) ==
> +   offsetof(struct dp_packet, inner_l3_ofs));
> +
> +BUILD_ASSERT_DECL(offsetof(struct dp_packet, inner_l3_ofs) +
> +   MEMBER_SIZEOF(struct dp_packet, inner_l3_ofs) ==
> +   offsetof(struct dp_packet, inner_l4_ofs));
> +
>  /* The below build assert makes sure it's 

Re: [ovs-dev] [PATCH v2 2/4] bfd: Set proper offsets and flags in BFD packets.

2024-01-31 Thread David Marchand
On Tue, Jan 30, 2024 at 11:15 PM Mike Pattrick  wrote:
>
> Previously the BFD packet creation code did not appropriately set
> offsets or flags. This contributed to issues involving encapsulation and
> the TSO code.

I noted that apart from fixing the offsets / flags used to checksum
offloading, this patch also fixes the packet_type used by other
dp_packet helpers.
I see nothing fixed on that later topic though.


>
> Fixes: ccc096898c46 ("bfd: Implement Bidirectional Forwarding Detection.")
> Signed-off-by: Mike Pattrick 

Reviewed-by: David Marchand 


-- 
David Marchand

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


Re: [ovs-dev] [PATCH v2 1/4] dp-packet: Validate correct offset for L4 inner size.

2024-01-31 Thread David Marchand
On Tue, Jan 30, 2024 at 11:15 PM Mike Pattrick  wrote:
>
> This patch fixes the correctness of dp_packet_inner_l4_size() when
> checking for the existence of an inner L4 header. Previously it checked
> for the outer L4 header.
>
> This function is currently only used when a packet is already flagged
> for tunneling, so an incorrect determination isn't possible as long as
> the flags of the packet are correct.
>
> Fixes: 85bcbbed839a ("userspace: Enable tunnel tests with TSO.")
> Signed-off-by: Mike Pattrick 

Reviewed-by: David Marchand 


-- 
David Marchand

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


Re: [ovs-dev] [PATCH ovn] github: Update versions of action dependencies (Node.js 20).

2024-01-31 Thread Ilya Maximets
On 1/31/24 13:13, Ilya Maximets wrote:
> On 1/30/24 15:18, Ilya Maximets wrote:
>> On 1/30/24 13:07, Ilya Maximets wrote:
>>> checkout@v3, cache@v3, setup-python@v4 and setup-go@v3 are using
>>> outdated Node.js 16 which is now deprecated in GHA [1], so these
>>> actions may stop working soon.
>>>
>>> Updating to most recent major versions with Node.js 20.  This stops
>>> GHA from throwing warnings in every build.
>>>
>>> [1] 
>>> https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/
>>>
>>> While at it also updating upload-artifact and download-artifact to
>>> the latest versions.
>>>
>>> Removing versions from the upload-artifact comment, since the
>>> behavior doesn't seem to change much between versions.
>>>
>>> New setup-go@v5 attempts to cache dependencies by default.  However,
>>> the default path it uses is go.sum in the root directory.  This
>>> triggers a warning, since the file doesn't exist:
>>>
>>>   Restore cache failed: Dependencies file is not found in
>>>   /home/runner/work/ovn-kubernetes/ovn-kubernetes.
>>>   Supported file pattern: go.sum
>>>
>>> Specify a path to all .sum files we have in the repository to make
>>> the setup-go happy.  This should in theory make the builds a touch
>>> faster.  This change is in line with recent changes in ovn-kubernetes
>>> itself.
>>>
>>> Signed-off-by: Ilya Maximets 
>>> ---
>>>  .github/workflows/containers.yml  |  2 +-
>>>  .../workflows/ovn-fake-multinode-tests.yml| 26 +++---
>>>  .github/workflows/ovn-kubernetes.yml  | 19 +-
>>>  .github/workflows/test.yml| 36 +--
>>>  4 files changed, 42 insertions(+), 41 deletions(-)
>>
>> Recheck-request: github-robot
> 
> Recheck-request: github-robot

Let's try specific re-checks.  The global one doesn't actually re-check the
workflow that I wanted to re-check.

Recheck-request: github-robot-_Build_and_Test
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH ovn v2] OVN-SB: Exposes igmp group protocol version through IGMP table.

2024-01-31 Thread Mohammad Heib
Expose the igmp/mld group protocol version through the
IGMP_GROUP table in SBDB.

This patch can be used by ovn consumer for debuggability purposes, user
now can  match between the protocol version used in the OVN logical
switches and the uplink ports.

Rreported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2160476
Signed-off-by: Mohammad Heib 
---
 NEWS  |  2 ++
 controller/ip-mcast.c | 10 --
 controller/ip-mcast.h |  5 +++--
 controller/pinctrl.c  | 28 
 northd/ovn-northd.c   |  2 +-
 ovn-sb.ovsschema  |  5 +++--
 ovn-sb.xml|  4 
 tests/ovn.at  |  3 +++
 8 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/NEWS b/NEWS
index 6553bd078..6505ef22b 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,8 @@ Post v23.09.0
   - Support selecting encapsulation IP based on the source/destination VIF's
 settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more
 details.
+  - IGMP_Group has new "protocol" column that displays the the group
+protocol version.
 
 OVN v23.09.0 - 15 Sep 2023
 --
diff --git a/controller/ip-mcast.c b/controller/ip-mcast.c
index a870fb29e..18a3e1fc2 100644
--- a/controller/ip-mcast.c
+++ b/controller/ip-mcast.c
@@ -107,11 +107,12 @@ igmp_mrouter_create(struct ovsdb_idl_txn *idl_txn,
 }
 
 void
-igmp_group_update_ports(const struct sbrec_igmp_group *g,
+igmp_group_update(const struct sbrec_igmp_group *g,
 struct ovsdb_idl_index *datapaths,
 struct ovsdb_idl_index *port_bindings,
 const struct mcast_snooping *ms OVS_UNUSED,
-const struct mcast_group *mc_group)
+const struct mcast_group *mc_group,
+const char *protocol)
 OVS_REQ_RDLOCK(ms->rwlock)
 {
 struct igmp_group_port *old_ports_storage =
@@ -151,6 +152,11 @@ igmp_group_update_ports(const struct sbrec_igmp_group *g,
 sbrec_igmp_group_update_ports_delvalue(g, igmp_port->port);
 }
 
+/* set Group protocol */
+if (protocol) {
+ sbrec_igmp_group_set_protocol(g, protocol);
+}
+
 free(old_ports_storage);
 hmap_destroy(_ports);
 }
diff --git a/controller/ip-mcast.h b/controller/ip-mcast.h
index 326f39db1..2a8921976 100644
--- a/controller/ip-mcast.h
+++ b/controller/ip-mcast.h
@@ -45,11 +45,12 @@ struct sbrec_igmp_group *igmp_mrouter_create(
 const struct sbrec_datapath_binding *datapath,
 const struct sbrec_chassis *chassis);
 
-void igmp_group_update_ports(const struct sbrec_igmp_group *g,
+void igmp_group_update(const struct sbrec_igmp_group *g,
  struct ovsdb_idl_index *datapaths,
  struct ovsdb_idl_index *port_bindings,
  const struct mcast_snooping *ms,
- const struct mcast_group *mc_group)
+ const struct mcast_group *mc_group,
+ const char *protocol)
 OVS_REQ_RDLOCK(ms->rwlock);
 void
 igmp_mrouter_update_ports(const struct sbrec_igmp_group *g,
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index bd3bd3d81..f3597b489 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -180,6 +180,7 @@ struct pinctrl {
 bool mac_binding_can_timestamp;
 bool fdb_can_timestamp;
 bool dns_supports_ovn_owned;
+bool igmp_support_protocol;
 };
 
 static struct pinctrl pinctrl;
@@ -3586,11 +3587,21 @@ pinctrl_update(const struct ovsdb_idl *idl, const char 
*br_int_name)
 if (dns_supports_ovn_owned != pinctrl.dns_supports_ovn_owned) {
 pinctrl.dns_supports_ovn_owned = dns_supports_ovn_owned;
 
-/* Notify pinctrl_handler that fdb timestamp column
+/* Notify pinctrl_handler that dns ovn_owned column
* availability has changed. */
 notify_pinctrl_handler();
 }
 
+bool igmp_support_proto =
+sbrec_server_has_igmp_group_table_col_protocol(idl);
+if (igmp_support_proto != pinctrl.igmp_support_protocol) {
+pinctrl.igmp_support_protocol = igmp_support_proto;
+
+/* Notify pinctrl_handler that igmp protocol column
+ * availability has changed. */
+notify_pinctrl_handler();
+}
+
 ovs_mutex_unlock(_mutex);
 }
 
@@ -5400,9 +5411,18 @@ ip_mcast_sync(struct ovsdb_idl_txn *ovnsb_idl_txn,
local_dp->datapath, chassis);
 }
 
-igmp_group_update_ports(sbrec_igmp, sbrec_datapath_binding_by_key,
-sbrec_port_binding_by_key, ip_ms->ms,
-mc_group);
+/* Set Group protocol if supported */
+if (pinctrl.igmp_support_protocol) {
+igmp_group_update(sbrec_igmp, sbrec_datapath_binding_by_key,
+  sbrec_port_binding_by_key, ip_ms->ms,
+  

Re: [ovs-dev] [PATCH ovn] github: Update versions of action dependencies (Node.js 20).

2024-01-31 Thread Ilya Maximets
On 1/30/24 15:18, Ilya Maximets wrote:
> On 1/30/24 13:07, Ilya Maximets wrote:
>> checkout@v3, cache@v3, setup-python@v4 and setup-go@v3 are using
>> outdated Node.js 16 which is now deprecated in GHA [1], so these
>> actions may stop working soon.
>>
>> Updating to most recent major versions with Node.js 20.  This stops
>> GHA from throwing warnings in every build.
>>
>> [1] 
>> https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/
>>
>> While at it also updating upload-artifact and download-artifact to
>> the latest versions.
>>
>> Removing versions from the upload-artifact comment, since the
>> behavior doesn't seem to change much between versions.
>>
>> New setup-go@v5 attempts to cache dependencies by default.  However,
>> the default path it uses is go.sum in the root directory.  This
>> triggers a warning, since the file doesn't exist:
>>
>>   Restore cache failed: Dependencies file is not found in
>>   /home/runner/work/ovn-kubernetes/ovn-kubernetes.
>>   Supported file pattern: go.sum
>>
>> Specify a path to all .sum files we have in the repository to make
>> the setup-go happy.  This should in theory make the builds a touch
>> faster.  This change is in line with recent changes in ovn-kubernetes
>> itself.
>>
>> Signed-off-by: Ilya Maximets 
>> ---
>>  .github/workflows/containers.yml  |  2 +-
>>  .../workflows/ovn-fake-multinode-tests.yml| 26 +++---
>>  .github/workflows/ovn-kubernetes.yml  | 19 +-
>>  .github/workflows/test.yml| 36 +--
>>  4 files changed, 42 insertions(+), 41 deletions(-)
> 
> Recheck-request: github-robot

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


Re: [ovs-dev] [PATCH ovs v2] Documentation: Adding note about using the jemalloc library.

2024-01-31 Thread Frode Nordahl
On Mon, Jan 29, 2024 at 12:33 PM Roberto Bartzen Acosta
 wrote:
>
> Updating the reference documentation with the inclusion of possible building 
> problems with libjemalloc and solution suggestions.

nit: the above line is very long and does not look well in `git show`

> Reported-at: 
> https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/2015748
> Signed-off-by: Roberto Bartzen Acosta 
> ---
>  Documentation/intro/install/general.rst | 16 
>  1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/intro/install/general.rst 
> b/Documentation/intro/install/general.rst
> index 19e360d47..e2eb19510 100644
> --- a/Documentation/intro/install/general.rst
> +++ b/Documentation/intro/install/general.rst
> @@ -344,6 +344,22 @@ you wish to link with jemalloc add it to LIBS::
>
>  $ ./configure LIBS=-ljemalloc
>
> +.. note::
> +  Linking Open vSwitch with the jemalloc shared library may not work as
> +  expected in certain operating system development environments. You can
> +  override the automatic compiler decision to avoid possible linker issues by
> +  passing ``-fno-lto`` or disabling ``-fno-builtin`` flag since the jemalloc

nit: Using the word `disabling` here creates a slightly confusing
double negative because of the flag already having a ``no-`` in its
name, in the current form I think we could just drop `disabling`,
'passing A or B flag' works fine without it.

> +  override standard built-in memory allocation functions such as malloc,
> +  calloc, etc. Both options can solve possible jemalloc linker issues with 
> pros
> +  and cons for each case, feel free to choose the path that appears best to
> +  you. Disabling LTO flag example::
> +
> +  $ ./configure LIBS=-ljemalloc CFLAGS=-fno-lto
> +
> +  Disabling built-in flag example::
> +
> +  ./configure LIBS=-ljemalloc CFLAGS=-fno-builtin
> +
>  .. _general-building:
>
>  Building
> --
> 2.25.1

Thanks for updating the docs with this information, I had a couple of
nits above, let's hear from one of the maintainers if they agree and
if any update could be incorporated as part of a merge before deciding
if any iterations are required.

Reviewed-by: Frode Nordahl 

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


Re: [ovs-dev] OVN 24.03 Soft Freeze 19 January

2024-01-31 Thread Dumitru Ceara
On 1/18/24 18:25, Han Zhou wrote:
> On Tue, Jan 16, 2024 at 5:45 AM Mark Michelson  wrote:
>>
>> Hi everyone,
>>
>> The soft freeze for OVN 24.03 is this Friday, 19 January, 2024. Please
>> ensure that any patches that introduce new features are posted to the
>> mailing list by that date if you wish to have them included in OVN 24.03.
>>
>> If you have a feature patch that you wish to be included in 24.03,
>> please post a link to the series in a reply to this mail. The OVN team
>> will do our best to review the desired changes.
>>
>> After soft freeze, we will create the OVN 24.03 branch on 2 February. At
>> that point, no new feature patches will be merged into the 24.03 branch.
>> However, bug fixes will still be eligible for inclusion.
>>

As mentioned during the last IRC meeting, we realized we're missing a
critical feature for ovn-kubernetes.  Lorenzo posted a patch yesterday
to add support for it:

https://patchwork.ozlabs.org/project/ovn/patch/83230852f6430353c4ee1fd49f422d0e08a78f13.1706626208.git.lorenzo.bianc...@redhat.com/

It includes quite a lot of changes to existing test cases (due to some
tables being inserted) but aside from that the feature is self contained
and doesn't impact anything else.  With that in mind I'd like to request
an exception to allow this to be included in 24.03.

Thanks,
Dumitru

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


Re: [ovs-dev] [PATCH ovn] northd: Add qos packet marking.

2024-01-31 Thread Dumitru Ceara
Recheck-request: github-robot

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