[ovs-dev] [PATCH v2] tests: Fix compatibility issue with Python 3.13 in vlog.at.

[Frode Nordahl]

The vlog - Python3 test makes use of output from Python
Tracebacks in its test assertion.

In Python 3.13 a line with tophat (``^``) markers is added below
Tracebacks from calls to assert [0], which makes the test fail.
This change of behavior is also backported to the Python 3.12 and
3.11 stable branches [1].

Strip lines containing one or more occurence of the ``^``
character from the output before performing the test assertions.

0: https://github.com/python/cpython/pull/105935
1: https://github.com/python/cpython/issues/116034
Reported-at: https://launchpad.net/bugs/2060434
Signed-off-by: Frode Nordahl 
---
 tests/vlog.at | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/vlog.at b/tests/vlog.at
index 785014956..efe91479a 100644
--- a/tests/vlog.at
+++ b/tests/vlog.at
@@ -8,6 +8,7 @@ AT_CHECK([$PYTHON3 $srcdir/test-vlog.py --log-file log_file \
 
 AT_CHECK([sed -e 's/.*-.*-.*T..:..:..Z |//' \
 -e 's/File ".*", line [[0-9]][[0-9]]*,/File , line ,/' \
+-e '/\^\+/d' \
 stderr_log], [0], [dnl
   0  | module_0 | EMER | emergency
   1  | module_0 | ERR | error
-- 
2.43.0

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

Re: [ovs-dev] [PATCH] tests: Fix compatibility issue with Python 3.12 in vlog.at.

[Ilya Maximets]

On 4/8/24 22:10, Frode Nordahl wrote:
> On Mon, Apr 8, 2024 at 9:18 PM Frode Nordahl  wrote:
>>
>> On Mon, Apr 8, 2024 at 9:14 PM Ilya Maximets  wrote:
>>>
>>> On 4/8/24 16:48, Frode Nordahl wrote:
 On Mon, Apr 8, 2024 at 4:44 PM Ilya Maximets  wrote:
>
> On 4/8/24 16:39, Frode Nordahl wrote:
>> The vlog - Python3 test makes use of output from Python
>> Tracebacks in its assertion.
>>
>> In Python 3.12 a line with tophat (``^``) markers is added below
>> the assert line, which makes the test fail.
>
> Hmm.  Are you sure it's 3.12?
>
> I believe I did run tests with 3.12 a few times at some point
> and didn't have this issue.

 I guess I should have spelled out the specific point release in use,
 we see it in Ubuntu with Python 3.12.2 [0].

 0: https://launchpad.net/ubuntu/+source/python3.12/3.12.2-5ubuntu3

>>>
>>> Even 3.12.2 doesn't seem to explain the situation.
>>>
>>> 3.12.2 in Fedora 41 doesn't have this issue.  Moreover, these is something
>>> fishy here.  On Fedora I don't have any '^' markers in the output, but on
>>> Ubuntu I see them.  AFAICT, recent changes in 3.12 branch should have 
>>> changed
>>> the underlining from a single '^' to multiple.  But not from having none at
>>> all to multiple.  Also, the same should be true for python starting from 
>>> 3.10:
>>>   https://github.com/python/cpython/issues/116563
>>> That's strange.
>>>
>>> To be clear, I'm not opposed to a change, I'm just a little puzzled on what
>>> is going on here, as it doesn't seem to be related much to the python 
>>> version.
>>
>> Thanks for checking, and no issue at all, I was also a bit confused as
>> it appeared to change between two package patch versions, and I landed
>> on something Python 3.12.2 to be most likely.
>>
>> I'll do an extra round of due diligence so that we are sure we
>> understand what's going on.
> 
> I've tracked the change of behavior down to this upstream fix:
> https://github.com/python/cpython/issues/116034
> 
> Which is included in the Debian/Ubuntu packages.
> 
> AFAICT from the discussion on that issue, they are essentially making
> Python 3.11 and 3.12 behave like Python 3.13.
> 
> So, I guess for OVS, we could make the commit subject address Python
> 3.13 compatibility with a note about those backports for 3.12 and 3.11
> in the message?
> 
> WDYT?


Ack.  Sounds good to me.  Thanks for finding the root cause!

For the patch itself, I'm not sure if '/[[[:space:]]]\+^\+/d'
sufficiently protects us from false positives, i.e. if we're
not matching on a line start ($), then there is probably not
much sense in matching on spaces either.
I'd suggest to add a '$' or just match on any line containing
the '^'.  We do not expect any lines like that in the test.

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

Re: [ovs-dev] [PATCH] tests: Fix compatibility issue with Python 3.12 in vlog.at.

[Frode Nordahl]

On Mon, Apr 8, 2024 at 9:18 PM Frode Nordahl  wrote:
>
> On Mon, Apr 8, 2024 at 9:14 PM Ilya Maximets  wrote:
> >
> > On 4/8/24 16:48, Frode Nordahl wrote:
> > > On Mon, Apr 8, 2024 at 4:44 PM Ilya Maximets  wrote:
> > >>
> > >> On 4/8/24 16:39, Frode Nordahl wrote:
> > >>> The vlog - Python3 test makes use of output from Python
> > >>> Tracebacks in its assertion.
> > >>>
> > >>> In Python 3.12 a line with tophat (``^``) markers is added below
> > >>> the assert line, which makes the test fail.
> > >>
> > >> Hmm.  Are you sure it's 3.12?
> > >>
> > >> I believe I did run tests with 3.12 a few times at some point
> > >> and didn't have this issue.
> > >
> > > I guess I should have spelled out the specific point release in use,
> > > we see it in Ubuntu with Python 3.12.2 [0].
> > >
> > > 0: https://launchpad.net/ubuntu/+source/python3.12/3.12.2-5ubuntu3
> > >
> >
> > Even 3.12.2 doesn't seem to explain the situation.
> >
> > 3.12.2 in Fedora 41 doesn't have this issue.  Moreover, these is something
> > fishy here.  On Fedora I don't have any '^' markers in the output, but on
> > Ubuntu I see them.  AFAICT, recent changes in 3.12 branch should have 
> > changed
> > the underlining from a single '^' to multiple.  But not from having none at
> > all to multiple.  Also, the same should be true for python starting from 
> > 3.10:
> >   https://github.com/python/cpython/issues/116563
> > That's strange.
> >
> > To be clear, I'm not opposed to a change, I'm just a little puzzled on what
> > is going on here, as it doesn't seem to be related much to the python 
> > version.
>
> Thanks for checking, and no issue at all, I was also a bit confused as
> it appeared to change between two package patch versions, and I landed
> on something Python 3.12.2 to be most likely.
>
> I'll do an extra round of due diligence so that we are sure we
> understand what's going on.

I've tracked the change of behavior down to this upstream fix:
https://github.com/python/cpython/issues/116034

Which is included in the Debian/Ubuntu packages.

AFAICT from the discussion on that issue, they are essentially making
Python 3.11 and 3.12 behave like Python 3.13.

So, I guess for OVS, we could make the commit subject address Python
3.13 compatibility with a note about those backports for 3.12 and 3.11
in the message?

WDYT?

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

Re: [ovs-dev] [PATCH] tests: Fix compatibility issue with Python 3.12 in vlog.at.

[Frode Nordahl]

On Mon, Apr 8, 2024 at 9:14 PM Ilya Maximets  wrote:
>
> On 4/8/24 16:48, Frode Nordahl wrote:
> > On Mon, Apr 8, 2024 at 4:44 PM Ilya Maximets  wrote:
> >>
> >> On 4/8/24 16:39, Frode Nordahl wrote:
> >>> The vlog - Python3 test makes use of output from Python
> >>> Tracebacks in its assertion.
> >>>
> >>> In Python 3.12 a line with tophat (``^``) markers is added below
> >>> the assert line, which makes the test fail.
> >>
> >> Hmm.  Are you sure it's 3.12?
> >>
> >> I believe I did run tests with 3.12 a few times at some point
> >> and didn't have this issue.
> >
> > I guess I should have spelled out the specific point release in use,
> > we see it in Ubuntu with Python 3.12.2 [0].
> >
> > 0: https://launchpad.net/ubuntu/+source/python3.12/3.12.2-5ubuntu3
> >
>
> Even 3.12.2 doesn't seem to explain the situation.
>
> 3.12.2 in Fedora 41 doesn't have this issue.  Moreover, these is something
> fishy here.  On Fedora I don't have any '^' markers in the output, but on
> Ubuntu I see them.  AFAICT, recent changes in 3.12 branch should have changed
> the underlining from a single '^' to multiple.  But not from having none at
> all to multiple.  Also, the same should be true for python starting from 3.10:
>   https://github.com/python/cpython/issues/116563
> That's strange.
>
> To be clear, I'm not opposed to a change, I'm just a little puzzled on what
> is going on here, as it doesn't seem to be related much to the python version.

Thanks for checking, and no issue at all, I was also a bit confused as
it appeared to change between two package patch versions, and I landed
on something Python 3.12.2 to be most likely.

I'll do an extra round of due diligence so that we are sure we
understand what's going on.

--
Frode Nordahl

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

Re: [ovs-dev] [PATCH] tests: Fix compatibility issue with Python 3.12 in vlog.at.

[Ilya Maximets]

On 4/8/24 16:48, Frode Nordahl wrote:
> On Mon, Apr 8, 2024 at 4:44 PM Ilya Maximets  wrote:
>>
>> On 4/8/24 16:39, Frode Nordahl wrote:
>>> The vlog - Python3 test makes use of output from Python
>>> Tracebacks in its assertion.
>>>
>>> In Python 3.12 a line with tophat (``^``) markers is added below
>>> the assert line, which makes the test fail.
>>
>> Hmm.  Are you sure it's 3.12?
>>
>> I believe I did run tests with 3.12 a few times at some point
>> and didn't have this issue.
> 
> I guess I should have spelled out the specific point release in use,
> we see it in Ubuntu with Python 3.12.2 [0].
> 
> 0: https://launchpad.net/ubuntu/+source/python3.12/3.12.2-5ubuntu3
> 

Even 3.12.2 doesn't seem to explain the situation.

3.12.2 in Fedora 41 doesn't have this issue.  Moreover, these is something
fishy here.  On Fedora I don't have any '^' markers in the output, but on
Ubuntu I see them.  AFAICT, recent changes in 3.12 branch should have changed
the underlining from a single '^' to multiple.  But not from having none at
all to multiple.  Also, the same should be true for python starting from 3.10:
  https://github.com/python/cpython/issues/116563
That's strange.

To be clear, I'm not opposed to a change, I'm just a little puzzled on what
is going on here, as it doesn't seem to be related much to the python version.

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

Re: [ovs-dev] [PATCH] tests: Fix compatibility issue with Python 3.12 in vlog.at.

[Frode Nordahl]

On Mon, Apr 8, 2024 at 4:44 PM Ilya Maximets  wrote:
>
> On 4/8/24 16:39, Frode Nordahl wrote:
> > The vlog - Python3 test makes use of output from Python
> > Tracebacks in its assertion.
> >
> > In Python 3.12 a line with tophat (``^``) markers is added below
> > the assert line, which makes the test fail.
>
> Hmm.  Are you sure it's 3.12?
>
> I believe I did run tests with 3.12 a few times at some point
> and didn't have this issue.

I guess I should have spelled out the specific point release in use,
we see it in Ubuntu with Python 3.12.2 [0].

0: https://launchpad.net/ubuntu/+source/python3.12/3.12.2-5ubuntu3

-- 
Frode Nordahl

> Bets regards, Ilya Maximets.
>
> >
> > Strip lines starting whith whitespace and otherwise only
> > containing one or more occurence of the ``^`` character from the
> > output before performing the test assertions.
> >
> > Note that the extra pair of brackets (``[]``) is required to make
> > m4 pass the expected string (``[[:space:]]``) to sed.
> >
> > Reported-at: https://launchpad.net/bugs/2060434
> > Signed-off-by: Frode Nordahl 
> > ---
> >  tests/vlog.at | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/tests/vlog.at b/tests/vlog.at
> > index 785014956..9a0c7d787 100644
> > --- a/tests/vlog.at
> > +++ b/tests/vlog.at
> > @@ -8,6 +8,7 @@ AT_CHECK([$PYTHON3 $srcdir/test-vlog.py --log-file log_file 
> > \
> >
> >  AT_CHECK([sed -e 's/.*-.*-.*T..:..:..Z |//' \
> >  -e 's/File ".*", line [[0-9]][[0-9]]*,/File , line ,/' \
> > +-e '/[[[:space:]]]\+^\+/d' \
> >  stderr_log], [0], [dnl
> >0  | module_0 | EMER | emergency
> >1  | module_0 | ERR | error
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests: Fix compatibility issue with Python 3.12 in vlog.at.

[Ilya Maximets]

On 4/8/24 16:39, Frode Nordahl wrote:
> The vlog - Python3 test makes use of output from Python
> Tracebacks in its assertion.
> 
> In Python 3.12 a line with tophat (``^``) markers is added below
> the assert line, which makes the test fail.

Hmm.  Are you sure it's 3.12?

I believe I did run tests with 3.12 a few times at some point
and didn't have this issue.

Bets regards, Ilya Maximets.

> 
> Strip lines starting whith whitespace and otherwise only
> containing one or more occurence of the ``^`` character from the
> output before performing the test assertions.
> 
> Note that the extra pair of brackets (``[]``) is required to make
> m4 pass the expected string (``[[:space:]]``) to sed.
> 
> Reported-at: https://launchpad.net/bugs/2060434
> Signed-off-by: Frode Nordahl 
> ---
>  tests/vlog.at | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/vlog.at b/tests/vlog.at
> index 785014956..9a0c7d787 100644
> --- a/tests/vlog.at
> +++ b/tests/vlog.at
> @@ -8,6 +8,7 @@ AT_CHECK([$PYTHON3 $srcdir/test-vlog.py --log-file log_file \
>  
>  AT_CHECK([sed -e 's/.*-.*-.*T..:..:..Z |//' \
>  -e 's/File ".*", line [[0-9]][[0-9]]*,/File , line ,/' \
> +-e '/[[[:space:]]]\+^\+/d' \
>  stderr_log], [0], [dnl
>0  | module_0 | EMER | emergency
>1  | module_0 | ERR | error

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

[ovs-dev] [PATCH] tests: Fix compatibility issue with Python 3.12 in vlog.at.

[Frode Nordahl]

The vlog - Python3 test makes use of output from Python
Tracebacks in its assertion.

In Python 3.12 a line with tophat (``^``) markers is added below
the assert line, which makes the test fail.

Strip lines starting whith whitespace and otherwise only
containing one or more occurence of the ``^`` character from the
output before performing the test assertions.

Note that the extra pair of brackets (``[]``) is required to make
m4 pass the expected string (``[[:space:]]``) to sed.

Reported-at: https://launchpad.net/bugs/2060434
Signed-off-by: Frode Nordahl 
---
 tests/vlog.at | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/vlog.at b/tests/vlog.at
index 785014956..9a0c7d787 100644
--- a/tests/vlog.at
+++ b/tests/vlog.at
@@ -8,6 +8,7 @@ AT_CHECK([$PYTHON3 $srcdir/test-vlog.py --log-file log_file \
 
 AT_CHECK([sed -e 's/.*-.*-.*T..:..:..Z |//' \
 -e 's/File ".*", line [[0-9]][[0-9]]*,/File , line ,/' \
+-e '/[[[:space:]]]\+^\+/d' \
 stderr_log], [0], [dnl
   0  | module_0 | EMER | emergency
   1  | module_0 | ERR | error
-- 
2.43.0

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

Re: [ovs-dev] [PATCH v2] netdev-dpdk: Fallback to non tunnel offloading API.

[David Marchand]

On Fri, Apr 5, 2024 at 3:00 PM Ilya Maximets  wrote:
> >>
> >>> Basically, resolving a neighbor with ARP inside tunnels is broken on
> >>> Intel nics (even without re-enabling outer udp checksum).
> >>> This can be observed with the following debug patch at the end of
> >>> netdev_dpdk_prep_hwol_packet():
> >>>
> >>> +char buf[256];
> >>> +if (rte_get_tx_ol_flag_list(mbuf->ol_flags, buf, sizeof(buf)) != 0)
> >>> +buf[0] = '\0';
> >>> +VLOG_WARN("len=%u, ol_flags=%s, outer_l2_len=%u, outer_l3_len=%u,
> >>> l2_len=%u, l3_len=%u, l4_len=%u, tso_segsz=%u", mbuf->pkt_len, buf,
> >>> mbuf->outer_l2_len, mbuf->outer_l3_len, mbuf->l2_len, mbuf->l3_len,
> >>> mbuf->l4_len, mbuf->tso_segsz);
> >>>
> >>> Then doing a "arping" inside the tunnel triggers:
> >>> 2024-04-03T16:05:40.920Z|00014|netdev_dpdk(pmd-c03/id:8)|WARN|len=96,
> >>> ol_flags=RTE_MBUF_F_TX_L4_NO_CKSUM RTE_MBUF_F_TX_OUTER_IP_CKSUM
> >>> RTE_MBUF_F_TX_OUTER_IPV4 RTE_MBUF_F_TX_TUNNEL_VXLAN , outer_l2_len=18,
> >>> outer_l3_len=20, l2_len=0, l3_len=0, l4_len=0, tso_segsz=0
>
> The fact that l2_len and l3_len are not set here looks like an OVS
> bug though, as AFAIU, these should always be set if any Tx offload
> is requested.

The commit that introduces such Tx offloads requests is:
f81d782c19 - netdev-native-tnl: Mark all vxlan/geneve packets as
tunneled. (7 weeks ago) 


-- 
David Marchand

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

Re: [ovs-dev] [PATCH ovn v4] Make tunnel ids exhaustion test trigger the problem.

[Ihar Hrachyshka]

Note to reviewers: looks like the port tunnel id test case revealed a
number of undefined behaviors and leaks (?) in northd; I am working on
addressing these. Before that, we should not merge this patch as-is. (Or at
least we should not merge the part with the port case; I believe the
network case is fine.)

On Sat, Apr 6, 2024 at 2:30 AM Vladislav Odintsov  wrote:

> Hi Ihar,
>
> Thanks for cooperation and enhancements in the testcases!
> The patch looks good to me.
>
> On 5 Apr 2024, at 19:14, Ihar Hrachyshka  wrote:
>
> The original version of the scenario passed with or without the fix.
> This is because all LSs were processed in one go, so the allocate
> function was never entered with *hint==0.
>
> Also, added another scenario that will check behavior when *hint is out
> of [min;max] bounds but > max (this happens in an obscure scenario where
> a vxlan chassis is added to the cluster mid-light, forcing northd to
> reduce its effective max value for tunnel ids; which may become lower
> than the current *hint for ports.)
>
> Fixes: a1f165a7b807 ("northd: fix infinite loop in ovn_allocate_tnlid()")
> Co-Authored-By: Vladislav Odintsov 
> Signed-off-by: Vladislav Odintsov 
> Signed-off-by: Ihar Hrachyshka 
> ---
> v1: initial version.
> v2: cover both cases of hint = 0 and hint > max.
> v3: reduce the number of ports to create in the hint > max scenario needed
> to trigger the problem.
> v4: remove spurious lib/ovn-util.c change.
> ---
> tests/ovn-northd.at | 43 ---
> 1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index be006fb32..1a4e7274d 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2823,7 +2823,7 @@ AT_CLEANUP
> ])
>
> OVN_FOR_EACH_NORTHD_NO_HV([
> -AT_SETUP([check tunnel ids exhaustion])
> +AT_SETUP([check datapath tunnel ids exhaustion])
> ovn_start
>
> # Create a fake chassis with vxlan encap to lower MAX DP tunnel key to 2^12
> @@ -2833,13 +2833,18 @@ ovn-sbctl \
>
> cmd="ovn-nbctl --wait=sb"
>
> -for i in {1..4097}; do
> +for i in {1..4095}; do
> cmd="${cmd} -- ls-add lsw-${i}"
> done
>
> eval $cmd
>
> -check_row_count nb:Logical_Switch 4097
> +check_row_count nb:Logical_Switch 4095
> +wait_row_count sb:Datapath_Binding 4095
> +
> +ovn-nbctl ls-add lsw-exhausted
> +
> +check_row_count nb:Logical_Switch 4096
> wait_row_count sb:Datapath_Binding 4095
>
> OVS_WAIT_UNTIL([grep "all datapath tunnel ids exhausted"
> northd/ovn-northd.log])
> @@ -2847,6 +2852,38 @@ OVS_WAIT_UNTIL([grep "all datapath tunnel ids
> exhausted" northd/ovn-northd.log])
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([check port tunnel ids exhaustion; vxlan chassis pops up
> midflight])
> +ovn_start
> +
> +cmd="ovn-nbctl --wait=sb"
> +
> +cmd="${cmd} -- ls-add lsw"
> +for i in {1..2048}; do
> +cmd="${cmd} -- lsp-add lsw lsp-${i}"
> +done
> +
> +eval $cmd
> +
> +check_row_count nb:Logical_Switch_Port 2048
> +wait_row_count sb:Port_Binding 2048
> +
> +# Now create a fake chassis with vxlan encap to lower MAX port tunnel key
> to 2^11
> +ovn-sbctl \
> +--id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="vxlan" \
> +-- --id=@c create chassis name=hv1 encaps=@e
> +
> +ovn-nbctl lsp-add lsw lsp-exhausted
> +
> +check_row_count nb:Logical_Switch_Port 2049
> +wait_row_count sb:Port_Binding 2048
> +
> +OVS_WAIT_UNTIL([grep "all port tunnel ids exhausted"
> northd/ovn-northd.log])
> +
> +AT_CLEANUP
> +])
> +
> +
>
> OVN_FOR_EACH_NORTHD_NO_HV([
> AT_SETUP([Logical Flow Datapath Groups])
> --
> 2.41.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
>
> Regards,
> Vladislav Odintsov
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] treewide: Remove remaining XenServer references.

[Alin Serdean]

Ooops , my bad :)
Sent from phone 

> On 8 Apr 2024, at 15:39, Ilya Maximets  wrote:
> 
> On 4/8/24 15:29, Alin Serdean wrote:
>> 
>> The spellchecker one: 
>> https://github.com/openvswitch/ovs/blob/master/utilities/checkpatch.py#L115 
>> 
>> 
>> I don’t assume so but wanted to verify.
> 
> Ah, this one was just added today by Eelco. :)
> And it's also in OVS repo, not in OVN.
> 
>> 
>> 
>> 
 On 8 Apr 2024, at 13:30, Ilya Maximets  wrote:
>>> 
>>> On 4/8/24 13:26, Alin Serdean wrote:
 
 Just one question: do we have to remove the xenserver reference in the 
 checkpatch.py?
>>> 
>>> Hmm.  Which one?
>>> 
 
 Otherwise looks good.
 
 Acked-by: Alin Gabriel Serdean 
 
> On 8 Apr 2024, at 13:06, Ilya Maximets  wrote:
> 
> Remaining bits from the OVS/OVN split.
> 
> Fixes: 1af37d11be73 ("Remove XenServer Code")
> Signed-off-by: Ilya Maximets 
> ---
> Documentation/tutorials/index.rst |  6 --
> README.rst|  3 ---
> acinclude.m4  | 26 ---
> build-aux/initial-tab-whitelist   |  1 -
> debian/copyright.in   | 34 ---
> 5 files changed, 70 deletions(-)
> 
> diff --git a/Documentation/tutorials/index.rst 
> b/Documentation/tutorials/index.rst
> index 0b7f52d0b..865a64018 100644
> --- a/Documentation/tutorials/index.rst
> +++ b/Documentation/tutorials/index.rst
> @@ -30,12 +30,6 @@ Tutorials
> Getting started with Open vSwitch (OVS) and Open Virtual Network (OVN) 
> for Open
> vSwitch.
> 
> -.. TODO(stephenfin): We could really do with a few basic tutorials, 
> along with
> -   some more specialized ones (DPDK, XenServer, Windows). The latter 
> could
> -   probably be formed from the install guides, but the former will need 
> to be
> -   produced from scratch or reproduced from blogs (with permission of the
> -   author)
> -
> .. toctree::
>   :maxdepth: 2
> 
> diff --git a/README.rst b/README.rst
> index 428cd8ee8..dce8c5bc8 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -91,9 +91,6 @@ license applies to the file in question.
> 
> File build-aux/cccl is licensed under the GNU General Public License, 
> version 2.
> 
> -Files under the xenserver directory are licensed on a file-by-file basis.
> -Refer to each file for details.
> -
> Contact
> ---
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index 1a80dfaa7..73a2a51be 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -177,32 +177,6 @@ AC_DEFUN([OVS_CONDITIONAL_CC_OPTION],
>   AM_CONDITIONAL([$2], [test $ovs_have_cc_option = yes])])
> dnl --
> 
> -dnl Check for too-old XenServer.
> -AC_DEFUN([OVS_CHECK_XENSERVER_VERSION],
> -  [AC_CACHE_CHECK([XenServer release], [ovs_cv_xsversion],
> -[if test -e /etc/redhat-release; then
> -   ovs_cv_xsversion=`sed -n 's/^XenServer DDK release 
> \([[^-]]*\)-.*/\1/p' /etc/redhat-release`
> - fi
> - if test -z "$ovs_cv_xsversion"; then
> -   ovs_cv_xsversion=none
> - fi])
> -  case $ovs_cv_xsversion in
> -none)
> -  ;;
> -
> -[[1-9]][[0-9]]* |dnl XenServer 10 or later
> -[[6-9]]* |   dnl XenServer 6 or later
> -5.[[7-9]]* | dnl XenServer 5.7 or later
> -5.6.[[1-9]][[0-9]][[0-9]][[0-9]]* |  dnl XenServer 5.6.1000 or later
> -5.6.[[2-9]][[0-9]][[0-9]]* | dnl XenServer 5.6.200 or later
> -5.6.1[[0-9]][[0-9]]) dnl Xenserver 5.6.100 or later
> -  ;;
> -
> -*)
> -  AC_MSG_ERROR([This appears to be XenServer $ovs_cv_xsversion, but 
> only XenServer 5.6.100 or later is supported.  (If you are really using a 
> supported version of XenServer, you may override this error message by 
> specifying 'ovs_cv_xsversion=5.6.100' on the "configure" command line.)])
> -  ;;
> -  esac])
> -
> dnl OVS_CHECK_SPARSE_TARGET
> dnl
> dnl The "cgcc" script from "sparse" isn't very good at detecting the
> diff --git a/build-aux/initial-tab-whitelist 
> b/build-aux/initial-tab-whitelist
> index 8ad43d616..eb5c1a23a 100644
> --- a/build-aux/initial-tab-whitelist
> +++ b/build-aux/initial-tab-whitelist
> @@ -5,7 +5,6 @@
> \.sln$
> ^ovs/
> ^third-party/
> -^xenserver/
> ^debian/rules.modules$
> ^debian/rules$
> ^\.gitmodules$
> diff --git a/debian/copyright.in b/debian/copyright.in
> index 9ad00340f..335fd6af8 100644
> --- a/debian/copyright.in
> +++ b/debian/copyright.in
> 

Re: [ovs-dev] [PATCH ovn] treewide: Remove remaining XenServer references.

[Ilya Maximets]

On 4/8/24 15:29, Alin Serdean wrote:
> 
> The spellchecker one: 
> https://github.com/openvswitch/ovs/blob/master/utilities/checkpatch.py#L115 
> 
> 
> I don’t assume so but wanted to verify.

Ah, this one was just added today by Eelco. :)
And it's also in OVS repo, not in OVN.

> 
> 
> 
>> On 8 Apr 2024, at 13:30, Ilya Maximets  wrote:
>>
>> On 4/8/24 13:26, Alin Serdean wrote:
>>>
>>> Just one question: do we have to remove the xenserver reference in the 
>>> checkpatch.py?
>>
>> Hmm.  Which one?
>>
>>>
>>> Otherwise looks good.
>>>
>>> Acked-by: Alin Gabriel Serdean 
>>>
 On 8 Apr 2024, at 13:06, Ilya Maximets  wrote:

 Remaining bits from the OVS/OVN split.

 Fixes: 1af37d11be73 ("Remove XenServer Code")
 Signed-off-by: Ilya Maximets 
 ---
 Documentation/tutorials/index.rst |  6 --
 README.rst    |  3 ---
 acinclude.m4  | 26 ---
 build-aux/initial-tab-whitelist   |  1 -
 debian/copyright.in   | 34 ---
 5 files changed, 70 deletions(-)

 diff --git a/Documentation/tutorials/index.rst 
 b/Documentation/tutorials/index.rst
 index 0b7f52d0b..865a64018 100644
 --- a/Documentation/tutorials/index.rst
 +++ b/Documentation/tutorials/index.rst
 @@ -30,12 +30,6 @@ Tutorials
 Getting started with Open vSwitch (OVS) and Open Virtual Network (OVN) for 
 Open
 vSwitch.

 -.. TODO(stephenfin): We could really do with a few basic tutorials, along 
 with
 -   some more specialized ones (DPDK, XenServer, Windows). The latter could
 -   probably be formed from the install guides, but the former will need 
 to be
 -   produced from scratch or reproduced from blogs (with permission of the
 -   author)
 -
 .. toctree::
   :maxdepth: 2

 diff --git a/README.rst b/README.rst
 index 428cd8ee8..dce8c5bc8 100644
 --- a/README.rst
 +++ b/README.rst
 @@ -91,9 +91,6 @@ license applies to the file in question.

 File build-aux/cccl is licensed under the GNU General Public License, 
 version 2.

 -Files under the xenserver directory are licensed on a file-by-file basis.
 -Refer to each file for details.
 -
 Contact
 ---

 diff --git a/acinclude.m4 b/acinclude.m4
 index 1a80dfaa7..73a2a51be 100644
 --- a/acinclude.m4
 +++ b/acinclude.m4
 @@ -177,32 +177,6 @@ AC_DEFUN([OVS_CONDITIONAL_CC_OPTION],
   AM_CONDITIONAL([$2], [test $ovs_have_cc_option = yes])])
 dnl --

 -dnl Check for too-old XenServer.
 -AC_DEFUN([OVS_CHECK_XENSERVER_VERSION],
 -  [AC_CACHE_CHECK([XenServer release], [ovs_cv_xsversion],
 -    [if test -e /etc/redhat-release; then
 -   ovs_cv_xsversion=`sed -n 's/^XenServer DDK release 
 \([[^-]]*\)-.*/\1/p' /etc/redhat-release`
 - fi
 - if test -z "$ovs_cv_xsversion"; then
 -   ovs_cv_xsversion=none
 - fi])
 -  case $ovs_cv_xsversion in
 -    none)
 -  ;;
 -
 -    [[1-9]][[0-9]]* |    dnl XenServer 10 or later
 -    [[6-9]]* |   dnl XenServer 6 or later
 -    5.[[7-9]]* | dnl XenServer 5.7 or later
 -    5.6.[[1-9]][[0-9]][[0-9]][[0-9]]* |  dnl XenServer 5.6.1000 or later
 -    5.6.[[2-9]][[0-9]][[0-9]]* | dnl XenServer 5.6.200 or later
 -    5.6.1[[0-9]][[0-9]]) dnl Xenserver 5.6.100 or later
 -  ;;
 -
 -    *)
 -  AC_MSG_ERROR([This appears to be XenServer $ovs_cv_xsversion, but 
 only XenServer 5.6.100 or later is supported.  (If you are really using a 
 supported version of XenServer, you may override this error message by 
 specifying 'ovs_cv_xsversion=5.6.100' on the "configure" command line.)])
 -  ;;
 -  esac])
 -
 dnl OVS_CHECK_SPARSE_TARGET
 dnl
 dnl The "cgcc" script from "sparse" isn't very good at detecting the
 diff --git a/build-aux/initial-tab-whitelist 
 b/build-aux/initial-tab-whitelist
 index 8ad43d616..eb5c1a23a 100644
 --- a/build-aux/initial-tab-whitelist
 +++ b/build-aux/initial-tab-whitelist
 @@ -5,7 +5,6 @@
 \.sln$
 ^ovs/
 ^third-party/
 -^xenserver/
 ^debian/rules.modules$
 ^debian/rules$
 ^\.gitmodules$
 diff --git a/debian/copyright.in b/debian/copyright.in
 index 9ad00340f..335fd6af8 100644
 --- a/debian/copyright.in
 +++ b/debian/copyright.in
 @@ -27,40 +27,6 @@ Upstream Copyright Holders:

 License:

 -* The following components are licensed under the
 -  GNU Lesser General Public License version 2.1 only
 -  with the exception clause below as a pre-amble.
 -
 

[ovs-dev] ovsdb-idl: potential issues with the persistent UUID implementation

[Terry Wilson]

There was a patch in OVS 3.1 that added support to the IDL code for
specifying the permanent UUID of a row when inserting [1]. There are
both C and Python implementations. Initially, I was adding support to
ovsdbapp for this feature and noticed that the Python tests for this
feature passed `new_uuid` to `insert()` as a string:

elif name == "insert_uuid":
...
s = txn.insert(idl.tables["simple"], new_uuid=args[0],
   persist_uuid=True)

when the existing code in `insert()` treats `new_uuid` as a `uuid.UUID` or None.

def insert(self, table, new_uuid=None, persist_uuid=False):
...

if new_uuid is None:
new_uuid = uuid.uuid4()
row = Row(self.idl, table, new_uuid, None, persist_uuid=persist_uuid)
table.rows[row.uuid] = row
self._txn_rows[row.uuid] = row
return row

It creates a `Row` with `row.uuid` of type string in this case instead
of a `uuid.UUID` and then stores it in `table.rows` under a string
key. This `Row` is fairly short-lived in that it gets deleted once we
send the request to the ovsdb-server and call
`Transaction.__disassemble()`. Then, when we get the OVSDB update
notification, the `Row` is created normally and stored in
`table.rows`. So at this point there's no real problem except that
from an interface perspective, new_uuid is the wrong type. In client
code, this can probably be problem having the returned `Row` having a
string for `uuid`.

This starts to get interesting when you try to simply fix this type
issue. For the case where `new_uuid` already exists in the DB, If you
pass `new_uuid=uuid.uuid4()` and fix the code in commit

op["op"] = "insert"
if row._persist_uuid:
op["uuid"] = row.uuid

to convert `row.uuid` to a string for the JSONRPC request, the code in
insert that does `table.rows[row.uuid] = row` will then *overwrite*
the existing row, then when the transaction is disassembled, that row
will be deleted. Since the row exists in the server, the txn will
fail. So your insert() call ends up deleting the row instead of adding
a new one. The existing test case will catch this issue. The tests
currently depend on getting a response from ovsdb-server regarding the
duplicate UUID. So fixing it just in the Python implementation by
checking locally `insert()` if the UUID was already stored in
`table.rows` would be a behavior difference between the two. Trying to
just pass the old row to the transaction, since `Row._data` would no
longer be `None` would convert the insert request to an update.

Knowing that the C IDL and Python IDL are very similar, I went to
compare what they were doing. It was essentially the same. The C code
does take a `const struct uuid *` as an argument to
`ovsdb_idl_txn_insert()` like one would expect, and insert the new row
into `table->rows`. The difference is, that `table->rows` is an hmap
and hmaps allow duplicate keys. So the C IDL version is storing two
copies of the same row in the DB after `ovsdb_idl_txn_insert()` and
the one that is inserted, having been inserted into
`txn->txn_rows` is looked up in `ovsdb_idl_txn_disassemble` and
deleted directly with `hmap_remove()`. So things, perhaps
accidentally, end up working fine in practice there. One caveat would
be that any operation that tried to find the row while the transaction
was active with something like `HMAP_FOR_EACH_WITH_HASH()` which
`ovsdb_idl_get_row()`, will get the first one that is iterated over
based on the hmap implementation.

So my questions are:

Do we really mean to be storing two identical rows in the C version?

If not, should we also do local checks for the row client-side in both
versions of the IDL code and fail early? It would be an API change to
either assert or return NULL/None in the insert methods. One possible
issue with this is that it would enable a race condition: Imagine two
requests for deleting and then recreating an object with a persistent
UUID and they are routed to different worker IDLs. The recreate could
fail if it doesn't receive the update notification from ovsdb-server
with the delete in time, and one of the reasons we want to use this
feature is to avoid this kind of issue.

If we actually intend to store multiple rows with the same UUID
temporarily in the in-memory tables, then how do we want to do that in
the Python code? table.rows technically isn't a dict, it's a custom
class that subclasses dict, so if we absolutely needed to we could
override `__setitem__`, `__delitem__`, and `__getitem__` to handle
duplicates by storing/retrieving them from a list.

Ultimately, it seems like we need to a) always send the insert
transactions for persistent uuids (as the current code does) and b) do
that without passing the wrong type in the Python code or
inadvertently causing issues with blindly storing multiple rows with
the same key in the table.rows hmap in the C code. Ideas?

[1] 
https://github.com/openvswitch/ovs/commit/55b9507e6824b935ffa0205fc7c7bebfe4e54279

Re: [ovs-dev] [PATCH ovn] treewide: Remove remaining XenServer references.

[Alin Serdean]

The spellchecker one: 
https://github.com/openvswitch/ovs/blob/master/utilities/checkpatch.py#L115

I don’t assume so but wanted to verify.



> On 8 Apr 2024, at 13:30, Ilya Maximets  wrote:
> 
> On 4/8/24 13:26, Alin Serdean wrote:
>> 
>> Just one question: do we have to remove the xenserver reference in the 
>> checkpatch.py?
> 
> Hmm.  Which one?
> 
>> 
>> Otherwise looks good.
>> 
>> Acked-by: Alin Gabriel Serdean 
>> 
 On 8 Apr 2024, at 13:06, Ilya Maximets  wrote:
>>> 
>>> Remaining bits from the OVS/OVN split.
>>> 
>>> Fixes: 1af37d11be73 ("Remove XenServer Code")
>>> Signed-off-by: Ilya Maximets 
>>> ---
>>> Documentation/tutorials/index.rst |  6 --
>>> README.rst|  3 ---
>>> acinclude.m4  | 26 ---
>>> build-aux/initial-tab-whitelist   |  1 -
>>> debian/copyright.in   | 34 ---
>>> 5 files changed, 70 deletions(-)
>>> 
>>> diff --git a/Documentation/tutorials/index.rst 
>>> b/Documentation/tutorials/index.rst
>>> index 0b7f52d0b..865a64018 100644
>>> --- a/Documentation/tutorials/index.rst
>>> +++ b/Documentation/tutorials/index.rst
>>> @@ -30,12 +30,6 @@ Tutorials
>>> Getting started with Open vSwitch (OVS) and Open Virtual Network (OVN) for 
>>> Open
>>> vSwitch.
>>> 
>>> -.. TODO(stephenfin): We could really do with a few basic tutorials, along 
>>> with
>>> -   some more specialized ones (DPDK, XenServer, Windows). The latter could
>>> -   probably be formed from the install guides, but the former will need to 
>>> be
>>> -   produced from scratch or reproduced from blogs (with permission of the
>>> -   author)
>>> -
>>> .. toctree::
>>>   :maxdepth: 2
>>> 
>>> diff --git a/README.rst b/README.rst
>>> index 428cd8ee8..dce8c5bc8 100644
>>> --- a/README.rst
>>> +++ b/README.rst
>>> @@ -91,9 +91,6 @@ license applies to the file in question.
>>> 
>>> File build-aux/cccl is licensed under the GNU General Public License, 
>>> version 2.
>>> 
>>> -Files under the xenserver directory are licensed on a file-by-file basis.
>>> -Refer to each file for details.
>>> -
>>> Contact
>>> ---
>>> 
>>> diff --git a/acinclude.m4 b/acinclude.m4
>>> index 1a80dfaa7..73a2a51be 100644
>>> --- a/acinclude.m4
>>> +++ b/acinclude.m4
>>> @@ -177,32 +177,6 @@ AC_DEFUN([OVS_CONDITIONAL_CC_OPTION],
>>>   AM_CONDITIONAL([$2], [test $ovs_have_cc_option = yes])])
>>> dnl --
>>> 
>>> -dnl Check for too-old XenServer.
>>> -AC_DEFUN([OVS_CHECK_XENSERVER_VERSION],
>>> -  [AC_CACHE_CHECK([XenServer release], [ovs_cv_xsversion],
>>> -[if test -e /etc/redhat-release; then
>>> -   ovs_cv_xsversion=`sed -n 's/^XenServer DDK release 
>>> \([[^-]]*\)-.*/\1/p' /etc/redhat-release`
>>> - fi
>>> - if test -z "$ovs_cv_xsversion"; then
>>> -   ovs_cv_xsversion=none
>>> - fi])
>>> -  case $ovs_cv_xsversion in
>>> -none)
>>> -  ;;
>>> -
>>> -[[1-9]][[0-9]]* |dnl XenServer 10 or later
>>> -[[6-9]]* |   dnl XenServer 6 or later
>>> -5.[[7-9]]* | dnl XenServer 5.7 or later
>>> -5.6.[[1-9]][[0-9]][[0-9]][[0-9]]* |  dnl XenServer 5.6.1000 or later
>>> -5.6.[[2-9]][[0-9]][[0-9]]* | dnl XenServer 5.6.200 or later
>>> -5.6.1[[0-9]][[0-9]]) dnl Xenserver 5.6.100 or later
>>> -  ;;
>>> -
>>> -*)
>>> -  AC_MSG_ERROR([This appears to be XenServer $ovs_cv_xsversion, but 
>>> only XenServer 5.6.100 or later is supported.  (If you are really using a 
>>> supported version of XenServer, you may override this error message by 
>>> specifying 'ovs_cv_xsversion=5.6.100' on the "configure" command line.)])
>>> -  ;;
>>> -  esac])
>>> -
>>> dnl OVS_CHECK_SPARSE_TARGET
>>> dnl
>>> dnl The "cgcc" script from "sparse" isn't very good at detecting the
>>> diff --git a/build-aux/initial-tab-whitelist 
>>> b/build-aux/initial-tab-whitelist
>>> index 8ad43d616..eb5c1a23a 100644
>>> --- a/build-aux/initial-tab-whitelist
>>> +++ b/build-aux/initial-tab-whitelist
>>> @@ -5,7 +5,6 @@
>>> \.sln$
>>> ^ovs/
>>> ^third-party/
>>> -^xenserver/
>>> ^debian/rules.modules$
>>> ^debian/rules$
>>> ^\.gitmodules$
>>> diff --git a/debian/copyright.in b/debian/copyright.in
>>> index 9ad00340f..335fd6af8 100644
>>> --- a/debian/copyright.in
>>> +++ b/debian/copyright.in
>>> @@ -27,40 +27,6 @@ Upstream Copyright Holders:
>>> 
>>> License:
>>> 
>>> -* The following components are licensed under the
>>> -  GNU Lesser General Public License version 2.1 only
>>> -  with the exception clause below as a pre-amble.
>>> -
>>> -xenserver/etc_xensource_scripts_vif
>>> -xenserver/opt_xensource_libexec_InterfaceReconfigure.py
>>> -xenserver/opt_xensource_libexec_InterfaceReconfigureBridge.py
>>> -xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
>>> -xenserver/opt_xensource_libexec_interface-reconfigure

Re: [ovs-dev] [PATCH ovn] treewide: Remove remaining XenServer references.

[Ilya Maximets]

On 4/8/24 13:26, Alin Serdean wrote:
> 
> Just one question: do we have to remove the xenserver reference in the 
> checkpatch.py?

Hmm.  Which one?

> 
> Otherwise looks good.
> 
> Acked-by: Alin Gabriel Serdean 
> 
>> On 8 Apr 2024, at 13:06, Ilya Maximets  wrote:
>>
>> Remaining bits from the OVS/OVN split.
>>
>> Fixes: 1af37d11be73 ("Remove XenServer Code")
>> Signed-off-by: Ilya Maximets 
>> ---
>> Documentation/tutorials/index.rst |  6 --
>> README.rst|  3 ---
>> acinclude.m4  | 26 ---
>> build-aux/initial-tab-whitelist   |  1 -
>> debian/copyright.in   | 34 ---
>> 5 files changed, 70 deletions(-)
>>
>> diff --git a/Documentation/tutorials/index.rst 
>> b/Documentation/tutorials/index.rst
>> index 0b7f52d0b..865a64018 100644
>> --- a/Documentation/tutorials/index.rst
>> +++ b/Documentation/tutorials/index.rst
>> @@ -30,12 +30,6 @@ Tutorials
>> Getting started with Open vSwitch (OVS) and Open Virtual Network (OVN) for 
>> Open
>> vSwitch.
>>
>> -.. TODO(stephenfin): We could really do with a few basic tutorials, along 
>> with
>> -   some more specialized ones (DPDK, XenServer, Windows). The latter could
>> -   probably be formed from the install guides, but the former will need to 
>> be
>> -   produced from scratch or reproduced from blogs (with permission of the
>> -   author)
>> -
>> .. toctree::
>>:maxdepth: 2
>>
>> diff --git a/README.rst b/README.rst
>> index 428cd8ee8..dce8c5bc8 100644
>> --- a/README.rst
>> +++ b/README.rst
>> @@ -91,9 +91,6 @@ license applies to the file in question.
>>
>> File build-aux/cccl is licensed under the GNU General Public License, 
>> version 2.
>>
>> -Files under the xenserver directory are licensed on a file-by-file basis.
>> -Refer to each file for details.
>> -
>> Contact
>> ---
>>
>> diff --git a/acinclude.m4 b/acinclude.m4
>> index 1a80dfaa7..73a2a51be 100644
>> --- a/acinclude.m4
>> +++ b/acinclude.m4
>> @@ -177,32 +177,6 @@ AC_DEFUN([OVS_CONDITIONAL_CC_OPTION],
>>AM_CONDITIONAL([$2], [test $ovs_have_cc_option = yes])])
>> dnl --
>>
>> -dnl Check for too-old XenServer.
>> -AC_DEFUN([OVS_CHECK_XENSERVER_VERSION],
>> -  [AC_CACHE_CHECK([XenServer release], [ovs_cv_xsversion],
>> -[if test -e /etc/redhat-release; then
>> -   ovs_cv_xsversion=`sed -n 's/^XenServer DDK release 
>> \([[^-]]*\)-.*/\1/p' /etc/redhat-release`
>> - fi
>> - if test -z "$ovs_cv_xsversion"; then
>> -   ovs_cv_xsversion=none
>> - fi])
>> -  case $ovs_cv_xsversion in
>> -none)
>> -  ;;
>> -
>> -[[1-9]][[0-9]]* |dnl XenServer 10 or later
>> -[[6-9]]* |   dnl XenServer 6 or later
>> -5.[[7-9]]* | dnl XenServer 5.7 or later
>> -5.6.[[1-9]][[0-9]][[0-9]][[0-9]]* |  dnl XenServer 5.6.1000 or later
>> -5.6.[[2-9]][[0-9]][[0-9]]* | dnl XenServer 5.6.200 or later
>> -5.6.1[[0-9]][[0-9]]) dnl Xenserver 5.6.100 or later
>> -  ;;
>> -
>> -*)
>> -  AC_MSG_ERROR([This appears to be XenServer $ovs_cv_xsversion, but 
>> only XenServer 5.6.100 or later is supported.  (If you are really using a 
>> supported version of XenServer, you may override this error message by 
>> specifying 'ovs_cv_xsversion=5.6.100' on the "configure" command line.)])
>> -  ;;
>> -  esac])
>> -
>> dnl OVS_CHECK_SPARSE_TARGET
>> dnl
>> dnl The "cgcc" script from "sparse" isn't very good at detecting the
>> diff --git a/build-aux/initial-tab-whitelist 
>> b/build-aux/initial-tab-whitelist
>> index 8ad43d616..eb5c1a23a 100644
>> --- a/build-aux/initial-tab-whitelist
>> +++ b/build-aux/initial-tab-whitelist
>> @@ -5,7 +5,6 @@
>> \.sln$
>> ^ovs/
>> ^third-party/
>> -^xenserver/
>> ^debian/rules.modules$
>> ^debian/rules$
>> ^\.gitmodules$
>> diff --git a/debian/copyright.in b/debian/copyright.in
>> index 9ad00340f..335fd6af8 100644
>> --- a/debian/copyright.in
>> +++ b/debian/copyright.in
>> @@ -27,40 +27,6 @@ Upstream Copyright Holders:
>>
>> License:
>>
>> -* The following components are licensed under the
>> -  GNU Lesser General Public License version 2.1 only
>> -  with the exception clause below as a pre-amble.
>> -
>> -xenserver/etc_xensource_scripts_vif
>> -xenserver/opt_xensource_libexec_InterfaceReconfigure.py
>> -xenserver/opt_xensource_libexec_InterfaceReconfigureBridge.py
>> -xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
>> -xenserver/opt_xensource_libexec_interface-reconfigure
>> -xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py
>> -
>> -* These components are only distributed in the source package.
>> -  They do not appear in any binary packages.
>> -
>> -  On Debian systems, the complete text of the
>> -  GNU Lesser General Public License version 2.1 can be found in
>> -  

Re: [ovs-dev] [PATCH ovn] treewide: Remove remaining XenServer references.

[Alin Serdean]

Just one question: do we have to remove the xenserver reference in the 
checkpatch.py?

Otherwise looks good.

Acked-by: Alin Gabriel Serdean 

> On 8 Apr 2024, at 13:06, Ilya Maximets  wrote:
> 
> Remaining bits from the OVS/OVN split.
> 
> Fixes: 1af37d11be73 ("Remove XenServer Code")
> Signed-off-by: Ilya Maximets 
> ---
> Documentation/tutorials/index.rst |  6 --
> README.rst|  3 ---
> acinclude.m4  | 26 ---
> build-aux/initial-tab-whitelist   |  1 -
> debian/copyright.in   | 34 ---
> 5 files changed, 70 deletions(-)
> 
> diff --git a/Documentation/tutorials/index.rst 
> b/Documentation/tutorials/index.rst
> index 0b7f52d0b..865a64018 100644
> --- a/Documentation/tutorials/index.rst
> +++ b/Documentation/tutorials/index.rst
> @@ -30,12 +30,6 @@ Tutorials
> Getting started with Open vSwitch (OVS) and Open Virtual Network (OVN) for 
> Open
> vSwitch.
> 
> -.. TODO(stephenfin): We could really do with a few basic tutorials, along 
> with
> -   some more specialized ones (DPDK, XenServer, Windows). The latter could
> -   probably be formed from the install guides, but the former will need to be
> -   produced from scratch or reproduced from blogs (with permission of the
> -   author)
> -
> .. toctree::
>:maxdepth: 2
> 
> diff --git a/README.rst b/README.rst
> index 428cd8ee8..dce8c5bc8 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -91,9 +91,6 @@ license applies to the file in question.
> 
> File build-aux/cccl is licensed under the GNU General Public License, version 
> 2.
> 
> -Files under the xenserver directory are licensed on a file-by-file basis.
> -Refer to each file for details.
> -
> Contact
> ---
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index 1a80dfaa7..73a2a51be 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -177,32 +177,6 @@ AC_DEFUN([OVS_CONDITIONAL_CC_OPTION],
>AM_CONDITIONAL([$2], [test $ovs_have_cc_option = yes])])
> dnl --
> 
> -dnl Check for too-old XenServer.
> -AC_DEFUN([OVS_CHECK_XENSERVER_VERSION],
> -  [AC_CACHE_CHECK([XenServer release], [ovs_cv_xsversion],
> -[if test -e /etc/redhat-release; then
> -   ovs_cv_xsversion=`sed -n 's/^XenServer DDK release 
> \([[^-]]*\)-.*/\1/p' /etc/redhat-release`
> - fi
> - if test -z "$ovs_cv_xsversion"; then
> -   ovs_cv_xsversion=none
> - fi])
> -  case $ovs_cv_xsversion in
> -none)
> -  ;;
> -
> -[[1-9]][[0-9]]* |dnl XenServer 10 or later
> -[[6-9]]* |   dnl XenServer 6 or later
> -5.[[7-9]]* | dnl XenServer 5.7 or later
> -5.6.[[1-9]][[0-9]][[0-9]][[0-9]]* |  dnl XenServer 5.6.1000 or later
> -5.6.[[2-9]][[0-9]][[0-9]]* | dnl XenServer 5.6.200 or later
> -5.6.1[[0-9]][[0-9]]) dnl Xenserver 5.6.100 or later
> -  ;;
> -
> -*)
> -  AC_MSG_ERROR([This appears to be XenServer $ovs_cv_xsversion, but only 
> XenServer 5.6.100 or later is supported.  (If you are really using a 
> supported version of XenServer, you may override this error message by 
> specifying 'ovs_cv_xsversion=5.6.100' on the "configure" command line.)])
> -  ;;
> -  esac])
> -
> dnl OVS_CHECK_SPARSE_TARGET
> dnl
> dnl The "cgcc" script from "sparse" isn't very good at detecting the
> diff --git a/build-aux/initial-tab-whitelist b/build-aux/initial-tab-whitelist
> index 8ad43d616..eb5c1a23a 100644
> --- a/build-aux/initial-tab-whitelist
> +++ b/build-aux/initial-tab-whitelist
> @@ -5,7 +5,6 @@
> \.sln$
> ^ovs/
> ^third-party/
> -^xenserver/
> ^debian/rules.modules$
> ^debian/rules$
> ^\.gitmodules$
> diff --git a/debian/copyright.in b/debian/copyright.in
> index 9ad00340f..335fd6af8 100644
> --- a/debian/copyright.in
> +++ b/debian/copyright.in
> @@ -27,40 +27,6 @@ Upstream Copyright Holders:
> 
> License:
> 
> -* The following components are licensed under the
> -  GNU Lesser General Public License version 2.1 only
> -  with the exception clause below as a pre-amble.
> -
> -xenserver/etc_xensource_scripts_vif
> -xenserver/opt_xensource_libexec_InterfaceReconfigure.py
> -xenserver/opt_xensource_libexec_InterfaceReconfigureBridge.py
> -xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
> -xenserver/opt_xensource_libexec_interface-reconfigure
> -xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py
> -
> -* These components are only distributed in the source package.
> -  They do not appear in any binary packages.
> -
> -  On Debian systems, the complete text of the
> -  GNU Lesser General Public License version 2.1 can be found in
> -  `/usr/share/common-licenses/LGPL-2.1'
> -
> -  The exception clause pre-amble reads:
> -
> -  As a special exception to the GNU Lesser General Public License, you
> -  may link, statically or 

[ovs-dev] [PATCH ovn] treewide: Remove remaining XenServer references.

[Ilya Maximets]

Remaining bits from the OVS/OVN split.

Fixes: 1af37d11be73 ("Remove XenServer Code")
Signed-off-by: Ilya Maximets 
---
 Documentation/tutorials/index.rst |  6 --
 README.rst|  3 ---
 acinclude.m4  | 26 ---
 build-aux/initial-tab-whitelist   |  1 -
 debian/copyright.in   | 34 ---
 5 files changed, 70 deletions(-)

diff --git a/Documentation/tutorials/index.rst 
b/Documentation/tutorials/index.rst
index 0b7f52d0b..865a64018 100644
--- a/Documentation/tutorials/index.rst
+++ b/Documentation/tutorials/index.rst
@@ -30,12 +30,6 @@ Tutorials
 Getting started with Open vSwitch (OVS) and Open Virtual Network (OVN) for Open
 vSwitch.
 
-.. TODO(stephenfin): We could really do with a few basic tutorials, along with
-   some more specialized ones (DPDK, XenServer, Windows). The latter could
-   probably be formed from the install guides, but the former will need to be
-   produced from scratch or reproduced from blogs (with permission of the
-   author)
-
 .. toctree::
:maxdepth: 2
 
diff --git a/README.rst b/README.rst
index 428cd8ee8..dce8c5bc8 100644
--- a/README.rst
+++ b/README.rst
@@ -91,9 +91,6 @@ license applies to the file in question.
 
 File build-aux/cccl is licensed under the GNU General Public License, version 
2.
 
-Files under the xenserver directory are licensed on a file-by-file basis.
-Refer to each file for details.
-
 Contact
 ---
 
diff --git a/acinclude.m4 b/acinclude.m4
index 1a80dfaa7..73a2a51be 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -177,32 +177,6 @@ AC_DEFUN([OVS_CONDITIONAL_CC_OPTION],
AM_CONDITIONAL([$2], [test $ovs_have_cc_option = yes])])
 dnl --
 
-dnl Check for too-old XenServer.
-AC_DEFUN([OVS_CHECK_XENSERVER_VERSION],
-  [AC_CACHE_CHECK([XenServer release], [ovs_cv_xsversion],
-[if test -e /etc/redhat-release; then
-   ovs_cv_xsversion=`sed -n 's/^XenServer DDK release \([[^-]]*\)-.*/\1/p' 
/etc/redhat-release`
- fi
- if test -z "$ovs_cv_xsversion"; then
-   ovs_cv_xsversion=none
- fi])
-  case $ovs_cv_xsversion in
-none)
-  ;;
-
-[[1-9]][[0-9]]* |dnl XenServer 10 or later
-[[6-9]]* |   dnl XenServer 6 or later
-5.[[7-9]]* | dnl XenServer 5.7 or later
-5.6.[[1-9]][[0-9]][[0-9]][[0-9]]* |  dnl XenServer 5.6.1000 or later
-5.6.[[2-9]][[0-9]][[0-9]]* | dnl XenServer 5.6.200 or later
-5.6.1[[0-9]][[0-9]]) dnl Xenserver 5.6.100 or later
-  ;;
-
-*)
-  AC_MSG_ERROR([This appears to be XenServer $ovs_cv_xsversion, but only 
XenServer 5.6.100 or later is supported.  (If you are really using a supported 
version of XenServer, you may override this error message by specifying 
'ovs_cv_xsversion=5.6.100' on the "configure" command line.)])
-  ;;
-  esac])
-
 dnl OVS_CHECK_SPARSE_TARGET
 dnl
 dnl The "cgcc" script from "sparse" isn't very good at detecting the
diff --git a/build-aux/initial-tab-whitelist b/build-aux/initial-tab-whitelist
index 8ad43d616..eb5c1a23a 100644
--- a/build-aux/initial-tab-whitelist
+++ b/build-aux/initial-tab-whitelist
@@ -5,7 +5,6 @@
 \.sln$
 ^ovs/
 ^third-party/
-^xenserver/
 ^debian/rules.modules$
 ^debian/rules$
 ^\.gitmodules$
diff --git a/debian/copyright.in b/debian/copyright.in
index 9ad00340f..335fd6af8 100644
--- a/debian/copyright.in
+++ b/debian/copyright.in
@@ -27,40 +27,6 @@ Upstream Copyright Holders:
 
 License:
 
-* The following components are licensed under the
-  GNU Lesser General Public License version 2.1 only
-  with the exception clause below as a pre-amble.
-
-xenserver/etc_xensource_scripts_vif
-xenserver/opt_xensource_libexec_InterfaceReconfigure.py
-xenserver/opt_xensource_libexec_InterfaceReconfigureBridge.py
-xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
-xenserver/opt_xensource_libexec_interface-reconfigure
-xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py
-
-* These components are only distributed in the source package.
-  They do not appear in any binary packages.
-
-  On Debian systems, the complete text of the
-  GNU Lesser General Public License version 2.1 can be found in
-  `/usr/share/common-licenses/LGPL-2.1'
-
-  The exception clause pre-amble reads:
-
-  As a special exception to the GNU Lesser General Public License, you
-  may link, statically or dynamically, a "work that uses the Library"
-  with a publicly distributed version of the Library to produce an
-  executable file containing portions of the Library, and distribute
-  that executable file under terms of your choice, without any of the
-  additional requirements listed in clause 6 of the GNU Lesser General
-  Public License.  By "a publicly distributed version of the Library",
-  we mean either the unmodified Library 

Re: [ovs-dev] [PATCH] checkpatch: Add additional words to extra_keywords.

[Eelco Chaudron]

On 2 Apr 2024, at 19:46, Simon Horman wrote:

> On Tue, Apr 02, 2024 at 03:21:16PM +0200, Eelco Chaudron wrote:
>> This patch add another set of keywords based on the results
>> of the last thousand committed patches.
>>
>> Signed-off-by: Eelco Chaudron 
>
> Acked-by: Simon Horman 

Thanks Simon for the review, applied to the main branch.

Cheers,

Eelco

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

Re: [ovs-dev] [PATCH v3] ofproto-dpif-upcall: Try lock for udpif_key map during sweep.

[Eelco Chaudron]

On 7 Apr 2024, at 3:57, LIU Yulong wrote:

> Hi Eelco,
>
> Sorry for the late reply. And thank you very much for your code examination.
>
> Today, I removed that  "ovsrcu_quiesce();" at  line 3066 [1], and
> started to run the
> test in my local environment. Let's see if such change can overcome the issue.

Lets see…

> Regarding the long time lock or quiesce state, Ilya's feedback seemed
> to be true during some
> service restart or large amount flow installations (no matter with or
> without the lock move fix).

This looks worrisome anyway and might be worth investigating (without your 
patch).

> I noticed logs like this:
>
> 2024-04-07T01:33:53.025Z|1|ovs_rcu(urcu2)|WARN|blocked 1001 ms
> waiting for main to quiesce
> 2024-04-07T01:33:54.025Z|2|ovs_rcu(urcu2)|WARN|blocked 2000 ms
> waiting for main to quiesce
> 2024-04-07T01:33:56.025Z|3|ovs_rcu(urcu2)|WARN|blocked 4000 ms
> waiting for main to quiesce
> 2024-04-07T01:34:00.025Z|4|ovs_rcu(urcu2)|WARN|blocked 8000 ms
> waiting for main to quiesce
> 2024-04-07T01:34:08.025Z|5|ovs_rcu(urcu2)|WARN|blocked 16001 ms
> waiting for main to quiesce
>
> And
>
> 2024-04-03T03:38:20.817Z|00136|ovs_rcu(urcu2)|WARN|blocked 2640 ms
> waiting for dpdk_watchdog1 to quiesce
> 2024-04-03T03:38:20.817Z|00137|ovs_rcu(urcu2)|WARN|blocked 2640 ms
> waiting for dpdk_watchdog1 to quiesce
> 2024-04-03T03:38:53.624Z|00138|ovs_rcu(urcu2)|WARN|blocked 1066 ms
> waiting for revalidator6 to quiesce
> 2024-04-03T03:38:57.606Z|00139|ovs_rcu(urcu2)|WARN|blocked 5048 ms
> waiting for revalidator6 to quiesce
> 2024-04-03T03:38:57.606Z|00140|ovs_rcu(urcu2)|WARN|blocked 5048 ms
> waiting for revalidator6 to quiesce
> 2024-04-03T03:39:47.105Z|00141|ovs_rcu(urcu2)|WARN|blocked 54547 ms
> waiting for revalidator6 to quiesce
> 2024-04-03T03:39:47.105Z|00142|ovs_rcu(urcu2)|WARN|blocked 54548 ms
> waiting for revalidator6 to quiesce
> 2024-04-03T03:39:47.105Z|00143|ovs_rcu(urcu2)|WARN|blocked 54548 ms
> waiting for revalidator6 to quiesce
> 2024-04-03T03:42:30.702Z|00144|ovs_rcu(urcu2)|WARN|blocked 10937 ms
> waiting for pmd-c03/id:8 to quiesce
> 2024-04-03T03:42:30.703Z|00145|ovs_rcu(urcu2)|WARN|blocked 10939 ms
> waiting for pmd-c03/id:8 to quiesce
> 2024-04-03T03:42:30.704Z|00146|ovs_rcu(urcu2)|WARN|blocked 10939 ms
> waiting for pmd-c03/id:8 to quiesce
> 2024-04-03T03:42:30.704Z|00147|ovs_rcu(urcu2)|WARN|blocked 10939 ms
> waiting for pmd-c03/id:8 to quiesce
> 2024-04-03T03:42:39.083Z|00151|ovs_rcu(urcu2)|WARN|blocked 19318 ms
> waiting for pmd-c03/id:8 to quiesce
> 2024-04-03T03:42:52.928Z|00155|ovs_rcu(urcu2)|WARN|blocked 33164 ms
> waiting for pmd-c03/id:8 to quiesce
> 2024-04-03T03:45:13.305Z|00156|ovs_rcu(urcu2)|WARN|blocked 173540 ms
> waiting for pmd-c03/id:8 to quiesce
> 2024-04-03T03:45:13.305Z|00157|ovs_rcu(urcu2)|WARN|blocked 173540 ms
> waiting for pmd-c03/id:8 to quiesce
> 2024-04-03T03:47:58.186Z|00158|ovs_rcu(urcu2)|WARN|blocked 338421 ms
> waiting for pmd-c03/id:8 to quiesce
>
>
> Not sure if this is related to the core issue.

I’ll guess it’s the opposite. If we do not go through the quiesce state the 
entries will not be deleted :)

>
> [1] 
> https://github.com/openvswitch/ovs/blob/master/ofproto/ofproto-dpif-upcall.c#L3066
>
>
> LIU Yulong
>
>
> On Fri, Mar 22, 2024 at 8:56 PM Eelco Chaudron  wrote:
>>
>>
>>
>> On 15 Mar 2024, at 11:04, LIU Yulong wrote:
>>
>>> A potential race condition happened with the following 3 threads:
>>> * PMD thread replaced the old_ukey and transitioned the state to
>>>   UKEY_DELETED.
>>> * RCU thread is freeing the old_ukey mutex.
>>> * While the revalidator thread is trying to lock the old_ukey mutex.
>>>
>>> We added some timestamp to udpif_key state transition and
>>> the udpif_key mutex free action, as well as the sweep try lock for
>>> the same udpif_key. When the ovs-vswitchd goes core, the udpif_key
>>> try_lock mutex had always a bit later than the udpif_key mutex free
>>> action. For instance [3]:
>>> ukey_destroy_time = 13217289156490
>>> sweep_now = 13217289156568
>>>
>>> The second time is 78us behind the first time.
>>>
>>> Then vswitchd process aborts at the revalidator thread try_lock of
>>> ukey->mutex because of the NULL pointer.
>>>
>>> This patch adds the try_lock for the ukeys' basket udpif_key map to
>>> avoid the PMD and revalidator access to the same map for replacing the
>>> udpif_key and transitioning the udpif_key state.
>>>
>>> More details can be found at:
>>> [1] 
>>> https://mail.openvswitch.org/pipermail/ovs-discuss/2024-March/052973.html
>>> [2] 
>>> https://mail.openvswitch.org/pipermail/ovs-discuss/2024-February/052949.html
>>> [3] 
>>> https://mail.openvswitch.org/pipermail/ovs-discuss/2024-March/052993.html
>>>
>>> Signed-off-by: LIU Yulong 
>>
>> Hi LIU,
>>
>> I've examined a lot of code, but I can't seem to figure out what event could 
>> be causing your problem. I also don't understand why your fix would prevent 
>> the problem from occurring (aside from possibly avoiding some 

Re: [ovs-dev] [PATCH ovn v2] northd, controller: Use paused controller action for packet buffering.

[Ales Musil]

On Fri, Apr 5, 2024 at 10:35 PM Numan Siddique  wrote:

>
>
> On Tue, Apr 2, 2024 at 2:28 AM Ales Musil  wrote:
>
>> The current packet injection loses ct_state in the process. When
>> the ct_state is lost we might commit to DNAT zone and perform
>> zero SNAT after the packet injection. This causes the first session
>> to be wrong as the reply packets are not unDNATted.
>>
>> Instead of re-injecting the packet back into the pipeline when
>> we get the MAC binding, use paused controller action. The paused
>> controller action stores ct_state, which is important for the behavior
>> of the resumed packet.
>>
>> At the same time bump the OvS submodule latest branch-3.3. This is
>> mainly for [0], which fixes metering for paused controller actions.
>>
>> [0] c560f6ca3257 ("ofproto-dpif-xlate: Fix continuations with associated
>> metering.")
>>
>> Reported-at: https://issues.redhat.com/browse/FDP-439
>> Signed-off-by: Ales Musil 
>> Acked-by: Mark Michelson 
>> ---
>> v2: Fix the Jira link and add ack from Mark.
>>
>
> Hi Ales,
>
> I see one problem during upgrades.  The packet buffering functionality
> would be broken when ovn-controllers are updated to the version
> which  has your patch  but ovn-northd is still yet to.
>
> In this case, the below logical flow will be present  (without the outer
> "output" action)
>
> ---
>   table=21(lr_in_arp_request  ), priority=100  , match=(eth.dst ==
> 00:00:00:00:00:00 && ip4), action=(arp { eth.dst = ff:ff:ff:ff:ff:ff;
> arp.spa = reg1; arp.tpa = reg0; arp.op = 1; output; };)
> ---
>
> The updated ovn-controller will now resume the packet after learning the
> nexthop.  But this resumed packet will be dropped because there is no
> "next" or "output" action following arp {}.
>
> Is this behavior acceptable until ovn-northd is also updated/upgraded ?
>

> Thanks
> Numan
>
>
Hi Numan,

this is a very good point, thank you for raising this. It probably depends
how long the upgrade takes, for ovn-kubernetes with ic it is all done per
node so that should be fine. However it might be more concerning for
OpenStack. In the end we would be losing only a small amount of packets (in
most cases only 1) for connections without MAC binding, on the other hand
the scenario it tries to fix is causing the first connection (whole
connection) to be completely broken.

I've tried to think about a possible solution to avoid this problem and I
see only one way out of this. That would involve keeping the old arp
unchanged, adding new action that would result in the pause instead. I'm
not sure if that's worth the effort, it's hard to estimate how big of an
impact it might have during the upgrade. One unfortunate thing is that we
cannot modify the actions of the continuation, if we could there wouldn't
be any change required in northd.

Let me know if you have other ideas on how to avoid this issue, maybe the
solution might be easier.


> ---
>
>>  controller/mac-learn.c | 30 +
>>  controller/mac-learn.h |  9 ++--
>>  controller/pinctrl.c   | 64 ++--
>>  lib/actions.c  | 41 +-
>>  northd/northd.c|  6 +--
>>  ovs|  2 +-
>>  tests/ovn-northd.at|  3 ++
>>  tests/ovn.at   |  8 ++--
>>  tests/system-ovn.at| 95 ++
>>  9 files changed, 195 insertions(+), 63 deletions(-)
>>
>> diff --git a/controller/mac-learn.c b/controller/mac-learn.c
>> index 071f01b4f..0c3b60c23 100644
>> --- a/controller/mac-learn.c
>> +++ b/controller/mac-learn.c
>> @@ -199,15 +199,24 @@ ovn_fdb_add(struct hmap *fdbs, uint32_t dp_key,
>> struct eth_addr mac,
>>  /* packet buffering functions */
>>
>>  struct packet_data *
>> -ovn_packet_data_create(struct ofpbuf ofpacts,
>> -   const struct dp_packet *original_packet)
>> +ovn_packet_data_create(const struct ofputil_packet_in *pin,
>> +   const struct ofpbuf *continuation)
>>  {
>>  struct packet_data *pd = xmalloc(sizeof *pd);
>>
>> -pd->ofpacts = ofpacts;
>> -/* clone the packet to send it later with correct L2 address */
>> -pd->p = dp_packet_clone_data(dp_packet_data(original_packet),
>> - dp_packet_size(original_packet));
>> +pd->pin = (struct ofputil_packet_in) {
>> +.packet = xmemdup(pin->packet, pin->packet_len),
>> +.packet_len = pin->packet_len,
>> +.flow_metadata = pin->flow_metadata,
>> +.reason = pin->reason,
>> +.table_id = pin->table_id,
>> +.cookie = pin->cookie,
>> +/* Userdata are empty on purpose,
>> + * it is not needed for the continuation. */
>> +.userdata = NULL,
>> +.userdata_len = 0,
>> +};
>> +pd->continuation = ofpbuf_clone(continuation);
>>
>>  return pd;
>>  }
>> @@ -216,8 +225,8 @@ ovn_packet_data_create(struct ofpbuf ofpacts,
>>  void
>>  ovn_packet_data_destroy(struct packet_data *pd)
>>  {
>> -dp_packet_delete(pd->p);
>> -