Re: [ovs-dev] [PATCH v3 2/2] ci: Run tc offload tests in GitHub Actions.
On Thu, Mar 9, 2023 at 5:26 PM Ilya Maximets wrote: > > On 3/9/23 17:26, Ilya Maximets wrote: > > On 3/9/23 17:22, Eelco Chaudron wrote: > >> > >> > >> On 9 Mar 2023, at 15:42, Simon Horman wrote: > >> > >>> On Wed, Mar 08, 2023 at 04:18:47PM +0100, Eelco Chaudron wrote: > Run "make check-offloads" as part of the GitHub actions tests. > > This test was run 25 times using GitHub actions, and the > failing rerun test cases where excluded. There are quite some > first-run failures, but unfortunately, there is no other > more stable kernel available as a GitHub-hosted runner. > > Did not yet include sanitizers in the run, as it's causing > the test to run too long >30min and there seems to be (timing) > issues with some of the tests. > > Signed-off-by: Eelco Chaudron > >>> > >>> Hi Eelco, > >>> > >>> I like this patch a lot. > >>> But I am observing reliability problems when executing the new job. > >>> > >>> For 5 runs, on each occasion some tests failed the first time. > >>> And on 3 of those runs at least one test failed on the recheck, > >>> so the job failed. > >> > >> Damn :) I did 25 runs (I did not check for re-runs), and they were fine. I > >> also cleaned up my jobs recently, so I no longer have them. > >> > >> I can do this again and figure out wich tests are failing. Then analyze > >> the failures to see if we need to exclude them or can fine-tune them. > >> > >> Ilya any other approach you can think off? > > > > Not really. :/ > > GHA is kind of hard to debug. Cirrus has a way to give you a console > > access to the running job, but GHA doesn't have that. fwiw; There are third party actions available that would allow you to SSH into the runner [0], which could possibly be useful to temporarily enable while debugging. 0: https://github.com/marketplace/actions/debugging-with-ssh -- Frode Nordahl ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v7 1/1] userspace: Add SRv6 tunnel support.
On 2023/01/17 18:41, Nobuhiro MIKI wrote: > SRv6 (Segment Routing IPv6) tunnel vport is responsible > for encapsulation and decapsulation the inner packets with > IPv6 header and an extended header called SRH > (Segment Routing Header). See spec in: > > https://datatracker.ietf.org/doc/html/rfc8754 > > This patch implements SRv6 tunneling in userspace datapath. > It uses `remote_ip` and `local_ip` options as with existing > tunnel protocols. It also adds a dedicated `srv6_segs` option > to define a sequence of routers called segment list. > > Signed-off-by: Nobuhiro MIKI Hi Ilya and OVS Community, Please let me discuss with you how to promote the development and review of this huge patch. It has been 8 months since v1 was submitted and only a few reviews and improvements have been done. Perhaps it is because this patch is so huge that it affects about 27 files and 700 lines. Would the review process go smoother if we split it into several patches as follows? - Support two tunnel types (e.g. IPPROTO_IPIP and IPPRPTO_IPV6) - SRv6 header generation and removal - Parsing of extension headers for IPPRPTO_ROUTING in parse_ipv6_ext_hdrs__() - Parsing of Segment Lists - Add OVS_VPORT_TYPE_SRV6 Thank you for always helping me out. Any comments would be greatly appreciated. And I would be glad to have anyone who would like to help me in the development of this feature. Best Regards, Nobuhiro MIKI ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] tests: Extends tests for feature flag compatibility
Thanks Ales and Dumitru. I pushed this change to main. On 3/8/23 04:18, Dumitru Ceara wrote: On 3/8/23 08:30, Ales Musil wrote: Extend the tests for "ct-no-masked-label" and "ovn-ct-lb-related" with check for LB lflows when LB is configured with "skip_snat" or "force_snat". Signed-off-by: Ales Musil --- This patch should be backported only to 23.03. --- Looks good to me, thanks! Acked-by: Dumitru Ceara ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-22.12] northd: Fix missig "); " from LB flows
On Thu, Mar 9, 2023 at 6:27 PM Mark Michelson wrote:
> Thanks Ales and Dumitru. I pushed this change to branch-22.12.
>
Could you please also backport this one down to 21.12? All branches down
from 22.12 are affected.
Thanks,
Ales
>
> On 3/8/23 04:27, Dumitru Ceara wrote:
> > On 3/8/23 08:30, Ales Musil wrote:
> >> Fix missing enclose for LB lflows when both
> >> "ct-no-masked-label" and "ovn-ct-lb-related"
> >> features are set false and the LB is configured
> >> with either "skip_snat" or "force_snat".
> >> Add missing test case for those.
> >>
> >> Fixes: cd600de6 ("northd: Add flag for CT related.")
> >> Signed-off-by: Ales Musil
> >> ---
> >> This patch should be backported all the way down to 21.12.
> >> ---
> >
> > Looks good to me, thanks!
> >
> > Acked-by: Dumitru Ceara
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
>
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA
amu...@redhat.comIM: amusil
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-22.12] northd: Fix missig "); " from LB flows
Thanks Ales and Dumitru. I pushed this change to branch-22.12.
On 3/8/23 04:27, Dumitru Ceara wrote:
On 3/8/23 08:30, Ales Musil wrote:
Fix missing enclose for LB lflows when both
"ct-no-masked-label" and "ovn-ct-lb-related"
features are set false and the LB is configured
with either "skip_snat" or "force_snat".
Add missing test case for those.
Fixes: cd600de6 ("northd: Add flag for CT related.")
Signed-off-by: Ales Musil
---
This patch should be backported all the way down to 21.12.
---
Looks good to me, thanks!
Acked-by: Dumitru Ceara
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] AUTHORS: Add Ales Musil and Dumitru Ceara
Thanks Ales and Dumitru. I pushed this change to main. I know you both have been authors of OVN for earlier versions as well. Should I also add these changes to previous supported OVN branches? On 3/9/23 09:40, Dumitru Ceara wrote: On 3/9/23 15:38, Ales Musil wrote: Signed-off-by: Ales Musil --- AUTHORS.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AUTHORS.rst b/AUTHORS.rst index f214340e3..02a16e6c8 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -38,6 +38,7 @@ Aaron Rosenaro...@clemson.edu Abhiram R Nabhira...@gmail.com Aidan Shribman aidan.shrib...@gmail.com Alan Pevec alan.pe...@redhat.com +Ales Musil amu...@redhat.com Oops, sorry about missing this.. Alexander Duyckalexander.h.du...@redhat.com Alexandru Copotalex.miha...@gmail.com Alexei Starovoitov a...@plumgrid.com @@ -126,6 +127,7 @@ Dongdong dongdo...@huawei.com Dongjundo...@dtdream.com Duan Jiong djduanji...@gmail.com Duffie Cooley +Dumitru Ceara dce...@redhat.com Haha! Acked-by: Dumitru Ceara Dustin Lundquist dus...@null-ptr.net Ed Maste ema...@freebsd.org Ed Swierk eswi...@skyportsystems.com ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 2/2] ci: Run tc offload tests in GitHub Actions.
On 3/9/23 17:26, Ilya Maximets wrote: > On 3/9/23 17:22, Eelco Chaudron wrote: >> >> >> On 9 Mar 2023, at 15:42, Simon Horman wrote: >> >>> On Wed, Mar 08, 2023 at 04:18:47PM +0100, Eelco Chaudron wrote: Run "make check-offloads" as part of the GitHub actions tests. This test was run 25 times using GitHub actions, and the failing rerun test cases where excluded. There are quite some first-run failures, but unfortunately, there is no other more stable kernel available as a GitHub-hosted runner. Did not yet include sanitizers in the run, as it's causing the test to run too long >30min and there seems to be (timing) issues with some of the tests. Signed-off-by: Eelco Chaudron >>> >>> Hi Eelco, >>> >>> I like this patch a lot. >>> But I am observing reliability problems when executing the new job. >>> >>> For 5 runs, on each occasion some tests failed the first time. >>> And on 3 of those runs at least one test failed on the recheck, >>> so the job failed. >> >> Damn :) I did 25 runs (I did not check for re-runs), and they were fine. I >> also cleaned up my jobs recently, so I no longer have them. >> >> I can do this again and figure out wich tests are failing. Then analyze the >> failures to see if we need to exclude them or can fine-tune them. >> >> Ilya any other approach you can think off? > > Not really. :/ > GHA is kind of hard to debug. Cirrus has a way to give you a console > access to the running job, but GHA doesn't have that. > > BTW, didn't we fic the force commit test recently? I see it disabled * fix > in the patch. > >> >> >>> The job is here: >>> https://github.com/horms/ovs/actions/runs/4365805718/jobs/7654575559 >>> >>> And a summary of the checks run for recheck for each job run is as follows. >>> >>> Attempt #1 >>> 85: failed >>> 166: ok >>> >>> Attempt #2 >>> 50: ok >>> 64: failed >>> 86: ok >>> 135: failed >>> >>> Attempt #3 >>> 50: ok >>> >>> Attempt #4 >>> 50: failed >>> >>> Attempt #5 >>> 50: ok >>> 163: ok >>> 169: ok >> > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 2/2] ci: Run tc offload tests in GitHub Actions.
On 3/9/23 17:22, Eelco Chaudron wrote: > > > On 9 Mar 2023, at 15:42, Simon Horman wrote: > >> On Wed, Mar 08, 2023 at 04:18:47PM +0100, Eelco Chaudron wrote: >>> Run "make check-offloads" as part of the GitHub actions tests. >>> >>> This test was run 25 times using GitHub actions, and the >>> failing rerun test cases where excluded. There are quite some >>> first-run failures, but unfortunately, there is no other >>> more stable kernel available as a GitHub-hosted runner. >>> >>> Did not yet include sanitizers in the run, as it's causing >>> the test to run too long >30min and there seems to be (timing) >>> issues with some of the tests. >>> >>> Signed-off-by: Eelco Chaudron >> >> Hi Eelco, >> >> I like this patch a lot. >> But I am observing reliability problems when executing the new job. >> >> For 5 runs, on each occasion some tests failed the first time. >> And on 3 of those runs at least one test failed on the recheck, >> so the job failed. > > Damn :) I did 25 runs (I did not check for re-runs), and they were fine. I > also cleaned up my jobs recently, so I no longer have them. > > I can do this again and figure out wich tests are failing. Then analyze the > failures to see if we need to exclude them or can fine-tune them. > > Ilya any other approach you can think off? Not really. :/ GHA is kind of hard to debug. Cirrus has a way to give you a console access to the running job, but GHA doesn't have that. BTW, didn't we fic the force commit test recently? I see it disabled in the patch. > > >> The job is here: >> https://github.com/horms/ovs/actions/runs/4365805718/jobs/7654575559 >> >> And a summary of the checks run for recheck for each job run is as follows. >> >> Attempt #1 >> 85: failed >> 166: ok >> >> Attempt #2 >> 50: ok >> 64: failed >> 86: ok >> 135: failed >> >> Attempt #3 >> 50: ok >> >> Attempt #4 >> 50: failed >> >> Attempt #5 >> 50: ok >> 163: ok >> 169: ok > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 2/2] ci: Run tc offload tests in GitHub Actions.
On 9 Mar 2023, at 15:42, Simon Horman wrote: > On Wed, Mar 08, 2023 at 04:18:47PM +0100, Eelco Chaudron wrote: >> Run "make check-offloads" as part of the GitHub actions tests. >> >> This test was run 25 times using GitHub actions, and the >> failing rerun test cases where excluded. There are quite some >> first-run failures, but unfortunately, there is no other >> more stable kernel available as a GitHub-hosted runner. >> >> Did not yet include sanitizers in the run, as it's causing >> the test to run too long >30min and there seems to be (timing) >> issues with some of the tests. >> >> Signed-off-by: Eelco Chaudron > > Hi Eelco, > > I like this patch a lot. > But I am observing reliability problems when executing the new job. > > For 5 runs, on each occasion some tests failed the first time. > And on 3 of those runs at least one test failed on the recheck, > so the job failed. Damn :) I did 25 runs (I did not check for re-runs), and they were fine. I also cleaned up my jobs recently, so I no longer have them. I can do this again and figure out wich tests are failing. Then analyze the failures to see if we need to exclude them or can fine-tune them. Ilya any other approach you can think off? > The job is here: > https://github.com/horms/ovs/actions/runs/4365805718/jobs/7654575559 > > And a summary of the checks run for recheck for each job run is as follows. > > Attempt #1 > 85: failed > 166: ok > > Attempt #2 > 50: ok > 64: failed > 86: ok > 135: failed > > Attempt #3 > 50: ok > > Attempt #4 > 50: failed > > Attempt #5 > 50: ok > 163: ok > 169: ok ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 2/2] ci: Run tc offload tests in GitHub Actions.
On Wed, Mar 08, 2023 at 04:18:47PM +0100, Eelco Chaudron wrote: > Run "make check-offloads" as part of the GitHub actions tests. > > This test was run 25 times using GitHub actions, and the > failing rerun test cases where excluded. There are quite some > first-run failures, but unfortunately, there is no other > more stable kernel available as a GitHub-hosted runner. > > Did not yet include sanitizers in the run, as it's causing > the test to run too long >30min and there seems to be (timing) > issues with some of the tests. > > Signed-off-by: Eelco Chaudron Hi Eelco, I like this patch a lot. But I am observing reliability problems when executing the new job. For 5 runs, on each occasion some tests failed the first time. And on 3 of those runs at least one test failed on the recheck, so the job failed. The job is here: https://github.com/horms/ovs/actions/runs/4365805718/jobs/7654575559 And a summary of the checks run for recheck for each job run is as follows. Attempt #1 85: failed 166: ok Attempt #2 50: ok 64: failed 86: ok 135: failed Attempt #3 50: ok Attempt #4 50: failed Attempt #5 50: ok 163: ok 169: ok ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] AUTHORS: Add Ales Musil and Dumitru Ceara
On 3/9/23 15:38, Ales Musil wrote: > Signed-off-by: Ales Musil > --- > AUTHORS.rst | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/AUTHORS.rst b/AUTHORS.rst > index f214340e3..02a16e6c8 100644 > --- a/AUTHORS.rst > +++ b/AUTHORS.rst > @@ -38,6 +38,7 @@ Aaron Rosenaro...@clemson.edu > Abhiram R Nabhira...@gmail.com > Aidan Shribman aidan.shrib...@gmail.com > Alan Pevec alan.pe...@redhat.com > +Ales Musil amu...@redhat.com Oops, sorry about missing this.. > Alexander Duyckalexander.h.du...@redhat.com > Alexandru Copotalex.miha...@gmail.com > Alexei Starovoitov a...@plumgrid.com > @@ -126,6 +127,7 @@ Dongdong dongdo...@huawei.com > Dongjundo...@dtdream.com > Duan Jiong djduanji...@gmail.com > Duffie Cooley > +Dumitru Ceara dce...@redhat.com Haha! Acked-by: Dumitru Ceara > Dustin Lundquist dus...@null-ptr.net > Ed Maste ema...@freebsd.org > Ed Swierk eswi...@skyportsystems.com ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] AUTHORS: Add Ales Musil and Dumitru Ceara
Signed-off-by: Ales Musil --- AUTHORS.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AUTHORS.rst b/AUTHORS.rst index f214340e3..02a16e6c8 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -38,6 +38,7 @@ Aaron Rosenaro...@clemson.edu Abhiram R Nabhira...@gmail.com Aidan Shribman aidan.shrib...@gmail.com Alan Pevec alan.pe...@redhat.com +Ales Musil amu...@redhat.com Alexander Duyckalexander.h.du...@redhat.com Alexandru Copotalex.miha...@gmail.com Alexei Starovoitov a...@plumgrid.com @@ -126,6 +127,7 @@ Dongdong dongdo...@huawei.com Dongjundo...@dtdream.com Duan Jiong djduanji...@gmail.com Duffie Cooley +Dumitru Ceara dce...@redhat.com Dustin Lundquist dus...@null-ptr.net Ed Maste ema...@freebsd.org Ed Swierk eswi...@skyportsystems.com -- 2.39.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] dpdk: Allow retaining CAP_SYS_RAWIO privileges
0-day Robot writes:
> Bleep bloop. Greetings Aaron Conole, 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: Line is 98 characters long (recommended limit is 79)
> #113 FILE: lib/daemon-unix.c:839:
> VLOG_WARN("No driver requires Linux capability
> CAP_SYS_RAWIO, disabling it.");
>
> Lines checked: 415, Warnings: 1, Errors: 0
I chose to keep this string unsplit because it makes it easier when
'grep'ing. If you think I should respin to shorten the lines, I can do
that, too.
>
> 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
Re: [ovs-dev] [PATCH ovn v2] northd: Ignore remote chassis when computing the supported feature set.
On 3/8/23 15:48, Ales Musil wrote:
> On Wed, Mar 8, 2023 at 3:44 PM Dumitru Ceara wrote:
>
>> Chassis in remote AZs are not programmed by the local ovn-northd. So we
>> don't need to take into account their supported feature set when
>> building logical flows.
>>
>> This patch also introduces a new northd unix command,
>> "debug/chassis-features-list". This is used by the newly added self
>> test but might also be useful when debugging live issues.
>>
>> Suggested-by: Numan Siddique
>> Signed-off-by: Dumitru Ceara
>> ---
>> V2:
>> - Addressed Ales' comment:
>> - explicitly set remote chassis features as unsupported in the test.
>> ---
>> northd/inc-proc-northd.c | 26 ++
>> northd/northd.c | 7 +++
>> tests/ovn-northd.at | 36
>> 3 files changed, 69 insertions(+)
>>
>> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
>> index 1c8550df5d..b28f551895 100644
>> --- a/northd/inc-proc-northd.c
>> +++ b/northd/inc-proc-northd.c
>> @@ -34,10 +34,13 @@
>> #include "en-lflow.h"
>> #include "en-northd-output.h"
>> #include "en-sync-sb.h"
>> +#include "unixctl.h"
>> #include "util.h"
>>
>> VLOG_DEFINE_THIS_MODULE(inc_proc_northd);
>>
>> +static unixctl_cb_func chassis_features_list;
>> +
>> #define NB_NODES \
>> NB_NODE(nb_global, "nb_global") \
>> NB_NODE(copp, "copp") \
>> @@ -305,6 +308,12 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>> engine_ovsdb_node_add_index(_sb_address_set,
>> "sbrec_address_set_by_name",
>> sbrec_address_set_by_name);
>> +
>> +struct northd_data *northd_data =
>> +engine_get_internal_data(_northd);
>> +unixctl_command_register("debug/chassis-features-list", "", 0, 0,
>> + chassis_features_list,
>> + _data->features);
>> }
>>
>> /* Returns true if the incremental processing ended up updating nodes. */
>> @@ -355,3 +364,20 @@ void inc_proc_northd_cleanup(void)
>> engine_cleanup();
>> engine_set_context(NULL);
>> }
>> +
>> +static void
>> +chassis_features_list(struct unixctl_conn *conn, int argc OVS_UNUSED,
>> + const char *argv[] OVS_UNUSED, void *features_)
>> +{
>> +struct chassis_features *features = features_;
>> +struct ds ds = DS_EMPTY_INITIALIZER;
>> +
>> +ds_put_format(, "ct_no_masked_label:%s\n",
>> + features->ct_no_masked_label ? "true" : "false");
>> +ds_put_format(, "ct_lb_related: %s\n",
>> + features->ct_lb_related ? "true" : "false");
>> +ds_put_format(, "mac_binding_timestamp: %s\n",
>> + features->mac_binding_timestamp ? "true" : "false");
>> +unixctl_command_reply(conn, ds_cstr());
>> +ds_destroy();
>> +}
>> diff --git a/northd/northd.c b/northd/northd.c
>> index ab10fea6ab..661f91ff7e 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -432,6 +432,13 @@ build_chassis_features(const struct northd_input
>> *input_data,
>> const struct sbrec_chassis *chassis;
>>
>> SBREC_CHASSIS_TABLE_FOR_EACH (chassis, input_data->sbrec_chassis) {
>> +/* Only consider local AZ chassis. Remote ones don't install
>> + * flows generated by the local northd.
>> + */
>> +if (smap_get_bool(>other_config, "is-remote", false)) {
>> +continue;
>> +}
>> +
>> bool ct_no_masked_label =
>> smap_get_bool(>other_config,
>>OVN_FEATURE_CT_NO_MASKED_LABEL,
>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>> index d0f6893e94..6c901a202b 100644
>> --- a/tests/ovn-northd.at
>> +++ b/tests/ovn-northd.at
>> @@ -8650,3 +8650,39 @@ AT_CHECK([grep -e "ls_in_acl" -e "ls_out_acl"
>> lflows2 | grep "priority=65532"],
>>
>> AT_CLEANUP
>> ])
>> +
>> +OVN_FOR_EACH_NORTHD_NO_HV([
>> +AT_SETUP([Chassis-feature compatibitility - remote chassis])
>> +ovn_start
>> +
>> +AS_BOX([Local chassis])
>> +check ovn-sbctl chassis-add hv1 geneve 127.0.0.1 \
>> + -- set chassis hv1 other_config:ct-no-masked-label=true \
>> + -- set chassis hv1 other_config:ovn-ct-lb-related=true \
>> + -- set chassis hv1 other_config:mac-binding-timestamp=true
>> +
>> +check ovn-nbctl --wait=sb sync
>> +
>> +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE
>> debug/chassis-features-list], [0], [dnl
>> +ct_no_masked_label:true
>> +ct_lb_related: true
>> +mac_binding_timestamp: true
>> +])
>> +
>> +AS_BOX([Remote chassis])
>> +check ovn-sbctl chassis-add hv2 geneve 127.0.0.2 \
>> + -- set chassis hv2 other_config:is-remote=true \
>> + -- set chassis hv2 other_config:ct-no-masked-label=false \
>> + -- set chassis hv2 other_config:ovn-ct-lb-related=false \
>> + -- set chassis hv2 other_config:mac-binding-timestamp=false
>> +
>> +check ovn-nbctl --wait=sb sync
>> +
>> +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE
>>
Re: [ovs-dev] [PATCH ovn] tests: Properly wait for chassis to added
On 3/9/23 09:52, Dumitru Ceara wrote:
> On 3/9/23 09:50, Ales Musil wrote:
>> Properly wait for chassis to be added before
>> getting the id.
>>
>> Signed-off-by: Ales Musil
>> ---
>
> Thanks for the quick fix, Ales! Just so I don't forget, dropping this here:
>
> Fixes: a09852bfe631 ("chassis: Remove unsupported config option")
>
I applied this to the main branch.
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4] northd, lsp: Add additional arp proxy features
On 3/9/23 10:53, Enrique Llorente Pastora wrote:
> On Thu, Mar 9, 2023 at 10:37 AM Ales Musil wrote:
>
>>
>>
>> On Thu, Mar 9, 2023 at 10:24 AM Dumitru Ceara wrote:
>>
>>> On 3/9/23 08:45, Ales Musil wrote:
On Wed, Mar 8, 2023 at 3:26 PM Enrique Llorente
>>> wrote:
> Configure mac address
> The mac address returned by ARP/NDP can be configured similar to LSP
> addresses where the mac is the first entry on the list
>
> IPv6
> Support NDP IPv6 protocol
>
> Use CIDRs
> Allow to specify subnets for ipv4 and ipv6, they will match whatever
> address is received from ARP/NDP
>
> Signed-off-by: Enrique Llorente
>
Hi,
just two nits below, but there is probably no need to spin another
>>> version.
> ---
> NEWS | 2 +
> northd/northd.c | 129 +---
> northd/ovn-northd.8.xml | 9 ++
> ovn-nb.xml| 17 ++-
> tests/ovn.at | 179 ++
> tests/system-common-macros.at | 1 +
> tests/system-ovn.at | 270
>>> ++
> 7 files changed, 551 insertions(+), 56 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 900825b1e..d859ae19d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -8,6 +8,8 @@ OVN v23.03.0 - 03 Mar 2023
>- Add ovn-ctl commands for (re)starting/stopping OVSDB relay for
>>> OVN SB
> DB.
>- Add new ovn-db@.service systemd unit to run ovsdb-servers in
>>> separate
> systemd units.
> + - Enhance LSP.options:arp_proxy to support IPv6, configurable MAC
> +addresses and CIDRs.
>
This needs to be under "Post v23.03.0" instead, however that can be
>>> moved
during merge.
>
> OVN v22.12.0 - 16 Dec 2022
> --
> diff --git a/northd/northd.c b/northd/northd.c
> index 33025bb5c..c61b538e7 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -1547,6 +1547,8 @@ struct ovn_port {
> struct ovs_list list; /* In list of similar records. */
>
> struct ovs_list dp_node;
> +
> +struct lport_addresses proxy_arp_addrs;
> };
>
> static bool
> @@ -1628,6 +1630,7 @@ ovn_port_create(struct hmap *ports, const char
>>> *key,
> op->sb = sb;
> ovn_port_set_nb(op, nbsp, nbrp);
> op->l3dgw_port = op->cr_port = NULL;
> +op->proxy_arp_addrs.ea = eth_addr_zero;
> hmap_insert(ports, >key_node, hash_string(op->key, 0));
> return op;
> }
> @@ -1654,6 +1657,7 @@ ovn_port_destroy(struct hmap *ports, struct
>>> ovn_port
> *port)
> destroy_routable_addresses(>routables);
>
> destroy_lport_addresses(>lrp_networks);
> +destroy_lport_addresses(>proxy_arp_addrs);
> free(port->json_key);
> free(port->key);
> free(port);
> @@ -2704,6 +2708,20 @@ join_logical_ports(struct northd_input
>>> *input_data,
> if (peer->od && peer->od->mcast_info.rtr.relay) {
> op->od->mcast_info.sw.flood_relay = true;
> }
> +
> +/* For LSP of router type arp proxy can be activated so
> + * it needs to be parsed
> + * either takes "MAC IP1 IP2" or "IP1 IP2"
> + */
> +const char *arp_proxy =
> smap_get(>nbsp->options,"arp_proxy");
> +int ofs = 0;
> +if (arp_proxy &&
> +!extract_addresses(arp_proxy, >proxy_arp_addrs,
>>> )
> &&
> +!extract_ip_addresses(arp_proxy,
>>> >proxy_arp_addrs)) {
> +static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(1, 5);
> +VLOG_WARN_RL(, "Invalid arp_proxy option: '%s' at
>>> lsp
> '%s'",
> + arp_proxy, op->nbsp->name);
> +}
> } else if (op->nbrp && op->nbrp->peer && !op->l3dgw_port) {
> struct ovn_port *peer = ovn_port_find(ports,
>>> op->nbrp->peer);
> if (peer) {
> @@ -8625,29 +8643,33 @@ build_lswitch_arp_nd_responder_known_ips(struct
> ovn_port *op,
> }
> }
> }
> +if (op->proxy_arp_addrs.n_ipv4_addrs ||
> +op->proxy_arp_addrs.n_ipv6_addrs) {
> +/* Select the mac address to answer the proxy ARP/NDP */
> +char *ea_s = NULL;
> +if (!eth_addr_is_zero(op->proxy_arp_addrs.ea)) {
> +ea_s = op->proxy_arp_addrs.ea_s;
> +} else if (op->peer) {
> +ea_s = op->peer->lrp_networks.ea_s;
> +} else {
> +return;
> +}
>
> -if
Re: [ovs-dev] [PATCH ovn v4 0/7] Enable system tests over userspace datapath in CI
On 3/8/23 15:32, Ales Musil wrote: > In order to allow there are ome fixes done to the > tests itself. The LB affinity needs a fix from OvS > side so it is marked as skip for the time being. > > To save some on the CI the last patch replaces > unit tests with Clang and Jemalloc with the > userspace system tests, the Jemalloc still > run with GCC. > > Ales Musil (7): Thanks, Ales (for the series) and Ilya, Mark, Simon (for reviews)! I applied this series to the main branch. Regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] tests: Properly wait for chassis to added
On 3/9/23 09:50, Ales Musil wrote: > Properly wait for chassis to be added before > getting the id. > > Signed-off-by: Ales Musil > --- I guess you meant to title this "tests: Properly wait for chassis to be added". I changed the commit title and pushed it to the main branch. Thanks! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] controller: lflow: do not use tcp as default IP protocol for ct_snat_to_vip action
Fix non-tcp hairpin use-case if the load-balancer is configured without
ports.
Tested-by: Ying Xu
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2157846
Signed-off-by: Lorenzo Bianconi
---
controller/lflow.c | 19 ++--
tests/ovn.at | 115 +
2 files changed, 129 insertions(+), 5 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index 003195ae4..023a30860 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -1729,6 +1729,7 @@ add_lb_vip_hairpin_flows(const struct ovn_controller_lb
*lb,
static void
add_lb_ct_snat_hairpin_for_dp(const struct ovn_controller_lb *lb,
+ const struct ovn_lb_vip *lb_vip,
const struct sbrec_datapath_binding *datapath,
const struct hmap *local_datapaths,
struct match *dp_match,
@@ -1742,12 +1743,20 @@ add_lb_ct_snat_hairpin_for_dp(const struct
ovn_controller_lb *lb,
match_set_metadata(dp_match, htonll(datapath->tunnel_key));
}
+uint16_t priority = datapath ? 200 : 100;
+if (!lb_vip->vip_port) {
+/* If L4 ports are not specified for the current LB, we will decrease
+ * the flow priority in order to not collide with other LBs with more
+ * fine-grained configuration.
+ */
+priority -= 10;
+}
/* A flow added for the "hairpin_snat_ip" case will have an extra
* datapath match, but it will also match on the less restrictive
* general case. Therefore, we set the priority in the
* "hairpin_snat_ip" case to be higher than the general case. */
ofctrl_add_flow(flow_table, OFTABLE_CT_SNAT_HAIRPIN,
-datapath ? 200 : 100, lb->slb->header_.uuid.parts[0],
+priority, lb->slb->header_.uuid.parts[0],
dp_match, dp_acts, >slb->header_.uuid);
}
@@ -1834,8 +1843,8 @@ add_lb_ct_snat_hairpin_vip_flow(const struct
ovn_controller_lb *lb,
}
}
-match_set_nw_proto(, lb->proto);
if (lb_vip->vip_port) {
+match_set_nw_proto(, lb->proto);
if (!lb->hairpin_orig_tuple) {
match_set_ct_nw_proto(, lb->proto);
match_set_ct_tp_dst(, htons(lb_vip->vip_port));
@@ -1852,18 +1861,18 @@ add_lb_ct_snat_hairpin_vip_flow(const struct
ovn_controller_lb *lb,
}
if (!use_hairpin_snat_ip) {
-add_lb_ct_snat_hairpin_for_dp(lb, NULL, NULL,
+add_lb_ct_snat_hairpin_for_dp(lb, lb_vip, NULL, NULL,
, , flow_table);
} else {
for (size_t i = 0; i < lb->slb->n_datapaths; i++) {
-add_lb_ct_snat_hairpin_for_dp(lb, lb->slb->datapaths[i],
+add_lb_ct_snat_hairpin_for_dp(lb, lb_vip, lb->slb->datapaths[i],
local_datapaths,
, , flow_table);
}
if (lb->slb->datapath_group) {
for (size_t i = 0; i < lb->slb->datapath_group->n_datapaths; i++) {
add_lb_ct_snat_hairpin_for_dp(
-lb, lb->slb->datapath_group->datapaths[i],
+lb, lb_vip, lb->slb->datapath_group->datapaths[i],
local_datapaths, , , flow_table);
}
}
diff --git a/tests/ovn.at b/tests/ovn.at
index 40d701bb2..989c377eb 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -29500,7 +29500,9 @@ OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw1-p1) =
xup])
check ovn-nbctl lb-add lb-ipv4-tcp 88.88.88.88:8080 42.42.42.1:4041 tcp
check ovn-nbctl lb-add lb-ipv4-udp 88.88.88.88:4040 42.42.42.1:2021 udp
+check ovn-nbctl lb-add lb-ipv4 88.88.88.89 42.42.42.2
check ovn-nbctl lb-add lb-ipv6-tcp [[8800::0088]]:8080 [[4200::1]]:4041 tcp
+check ovn-nbctl lb-add lb-ipv6 8800::0089 4200::2
check ovn-nbctl --wait=hv lb-add lb-ipv6-udp [[8800::0088]]:4040
[[4200::1]]:2021 udp
AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=68 | grep -v NXST], [1],
[dnl
@@ -29791,6 +29793,119 @@ AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=70
| ofctl_strip_all | grep -
table=70,
priority=100,udp6,reg2=0xfc8/0x,reg4=0x8800,reg5=0,reg6=0,reg7=0x88
actions=ct(commit,zone=NXM_NX_REG12[[0..15]],nat(src=8800::88))
])
+check ovn-nbctl --wait=hv ls-lb-add sw0 lb-ipv4
+OVS_WAIT_UNTIL(
+[test $(as hv1 ovs-ofctl dump-flows br-int table=68 | grep -c -v NXST) -eq
7]
+)
+
+OVS_WAIT_UNTIL(
+[test $(as hv2 ovs-ofctl dump-flows br-int table=68 | grep -c -v NXST) -eq
7]
+)
+
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=68 | ofctl_strip_all | grep
-v NXST], [0], [dnl
+ table=68,
priority=100,ct_mark=0x2/0x2,ip,reg1=0x58585859,nw_src=42.42.42.2,nw_dst=42.42.42.2
actions=load:0x1->NXM_NX_REG10[[7]],learn(table=69,delete_learned,OXM_OF_METADATA[[]],eth_type=0x800,NXM_OF_IP_SRC[[]],ip_dst=88.88.88.89,NXM_OF_IP_PROTO[[]],load:0x1->NXM_NX_REG10[[7]])
+
Re: [ovs-dev] [PATCH v3 1/2] dpctl: Fix flush-conntrack with datapath as argument
On Thu, Mar 9, 2023 at 3:00 PM Ales Musil wrote:
> Specifying datapath with "dpctl/flush-conntrack" didn't
> work as expected and caused error:
> ovs-dpctl: field system@ovs-system missing value (Invalid argument)
>
> To prevent that check if we have datapath as first argument
> and use it accordingly.
>
> Fixes: a9ae73b916ba ("ofp, dpif: Allow CT flush based on partial match.")
> Signed-off-by: Ales Musil
> ---
> v2: Add test case.
> v3: Add test case also for flush-conntrack without arguments.
> ---
> lib/dpctl.c | 17 -
> tests/system-traffic.at | 16
> 2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index c501a0cd7..a7a4d8ee8 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -1717,10 +1717,16 @@ dpctl_flush_conntrack(int argc, const char *argv[],
> uint16_t zone, *pzone = NULL;
> int error;
> int args = argc - 1;
> +int zone_pos = 1;
> +
> +if (dp_arg_exists(argc, argv)) {
> +args--;
> +zone_pos = 2;
> +}
>
> /* Parse zone. */
> -if (args && !strncmp(argv[1], "zone=", 5)) {
> -if (!ovs_scan(argv[1], "zone=%"SCNu16, )) {
> +if (args && !strncmp(argv[zone_pos], "zone=", 5)) {
> +if (!ovs_scan(argv[zone_pos], "zone=%"SCNu16, )) {
> ds_put_cstr(, "failed to parse zone");
> error = EINVAL;
> goto error;
> @@ -1747,13 +1753,6 @@ dpctl_flush_conntrack(int argc, const char *argv[],
> args--;
> }
>
> -/* Report error if there is more than one unparsed argument. */
> -if (args > 1) {
> -ds_put_cstr(, "invalid arguments");
> -error = EINVAL;
> -goto error;
> -}
> -
> error = opt_dpif_open(argc, argv, dpctl_p, 5, );
> if (error) {
> dpctl_error(dpctl_p, error, "Cannot open dpif");
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 380372430..d5723685a 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -2360,8 +2360,10 @@
> priority=100,in_port=2,icmp,action=ct(zone=5,commit),1
> ])
>
> AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> +dp=$(ovs-appctl dpctl/dump-dps)
>
> m4_foreach([FLUSH_CMD], [[ovs-appctl dpctl/flush-conntrack],
> + [ovs-appctl dpctl/flush-conntrack $dp],
> [ovs-ofctl ct-flush br0]], [
> AS_BOX([Testing with FLUSH_CMD])
>
> @@ -2503,6 +2505,20 @@
> udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.
>
> AT_CHECK([FLUSH_CMD zone=5 '' 'ct_nw_src=10.1.1.1'])
>
> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
> +
> +dnl Test UDP from port 1 and 2, partial flush by src address in reply
> direction
> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1
> packet=5054000a505400090800451c0011a4cd0a0101010a010102000100020008
> actions=resubmit(,0)"])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2
> packet=5054000a505400090800451c0011a4cd0a0101020a010101000200010008
> actions=resubmit(,0)"])
> +
> +
> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sort],
> [0], [dnl
>
> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
>
> +udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
> +])
> +
> +AT_CHECK([FLUSH_CMD])
> +
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
> ])
>
> --
> 2.39.2
>
>
Ah I forgot to change the comment so this should be applied on top:
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index d5723685a..7115a8ef4 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2507,7 +2507,7 @@ AT_CHECK([FLUSH_CMD zone=5 '' 'ct_nw_src=10.1.1.1'])
AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
-dnl Test UDP from port 1 and 2, partial flush by src address in reply
direction
+dnl Test UDP from port 1 and 2, flush without arguments
AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1
packet=5054000a505400090800451c0011a4cd0a0101010a010102000100020008
actions=resubmit(,0)"])
AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2
packet=5054000a505400090800451c0011a4cd0a0101020a010101000200010008
actions=resubmit(,0)"])
Thanks,
Ales
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA
amu...@redhat.comIM: amusil
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 2/2] vswitch: Add missing documentation for "ct_flush" capability
Fixes: 08146bf7d9b4 ("openflow: Add extension to flush CT by generic match.")
Signed-off-by: Ales Musil
---
v3: Make the connection between the extension in the capability more explicit.
---
vswitchd/vswitch.xml | 6 ++
1 file changed, 6 insertions(+)
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 12708a313..4c0ba2187 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -6301,6 +6301,12 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch
options:peer=p1 \
translated to an ephemeral port. If there is no collision, no SNAT
is performed.
+
+True if the datapath supports CT flush OpenFlow Nicira extension
+called NXT_CT_FLUSH. The NXT_CT_FLUSH
+extensions allows to flush CT entries based on specified parameters.
+
--
2.39.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 1/2] dpctl: Fix flush-conntrack with datapath as argument
Specifying datapath with "dpctl/flush-conntrack" didn't
work as expected and caused error:
ovs-dpctl: field system@ovs-system missing value (Invalid argument)
To prevent that check if we have datapath as first argument
and use it accordingly.
Fixes: a9ae73b916ba ("ofp, dpif: Allow CT flush based on partial match.")
Signed-off-by: Ales Musil
---
v2: Add test case.
v3: Add test case also for flush-conntrack without arguments.
---
lib/dpctl.c | 17 -
tests/system-traffic.at | 16
2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/lib/dpctl.c b/lib/dpctl.c
index c501a0cd7..a7a4d8ee8 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -1717,10 +1717,16 @@ dpctl_flush_conntrack(int argc, const char *argv[],
uint16_t zone, *pzone = NULL;
int error;
int args = argc - 1;
+int zone_pos = 1;
+
+if (dp_arg_exists(argc, argv)) {
+args--;
+zone_pos = 2;
+}
/* Parse zone. */
-if (args && !strncmp(argv[1], "zone=", 5)) {
-if (!ovs_scan(argv[1], "zone=%"SCNu16, )) {
+if (args && !strncmp(argv[zone_pos], "zone=", 5)) {
+if (!ovs_scan(argv[zone_pos], "zone=%"SCNu16, )) {
ds_put_cstr(, "failed to parse zone");
error = EINVAL;
goto error;
@@ -1747,13 +1753,6 @@ dpctl_flush_conntrack(int argc, const char *argv[],
args--;
}
-/* Report error if there is more than one unparsed argument. */
-if (args > 1) {
-ds_put_cstr(, "invalid arguments");
-error = EINVAL;
-goto error;
-}
-
error = opt_dpif_open(argc, argv, dpctl_p, 5, );
if (error) {
dpctl_error(dpctl_p, error, "Cannot open dpif");
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 380372430..d5723685a 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2360,8 +2360,10 @@ priority=100,in_port=2,icmp,action=ct(zone=5,commit),1
])
AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+dp=$(ovs-appctl dpctl/dump-dps)
m4_foreach([FLUSH_CMD], [[ovs-appctl dpctl/flush-conntrack],
+ [ovs-appctl dpctl/flush-conntrack $dp],
[ovs-ofctl ct-flush br0]], [
AS_BOX([Testing with FLUSH_CMD])
@@ -2503,6 +2505,20 @@
udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.
AT_CHECK([FLUSH_CMD zone=5 '' 'ct_nw_src=10.1.1.1'])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+
+dnl Test UDP from port 1 and 2, partial flush by src address in reply direction
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1
packet=5054000a505400090800451c0011a4cd0a0101010a010102000100020008
actions=resubmit(,0)"])
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2
packet=5054000a505400090800451c0011a4cd0a0101020a010101000200010008
actions=resubmit(,0)"])
+
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sort], [0],
[dnl
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
+udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
+])
+
+AT_CHECK([FLUSH_CMD])
+
AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
])
--
2.39.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 2/2] vswitch: Add missing documentation for "ct_flush" capability
On Thu, Mar 9, 2023 at 1:15 PM Ilya Maximets wrote:
> On 3/6/23 14:37, Ales Musil wrote:
> > Signed-off-by: Ales Musil
>
> Needs a Fixes tag.
>
> > ---
> > vswitchd/vswitch.xml | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index 12708a313..15e4f97b7 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -6301,6 +6301,11 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
> type=patch options:peer=p1 \
> > translated to an ephemeral port. If there is no collision, no
> SNAT
> > is performed.
> >
> > + > + type='{"type": "boolean"}'>
> > +True if the datapath supports CT flush extension. The extensions
> > +allows to flush CT entries based on specified parameters.
>
> This doesn't really describe the capability. It was possible to flush
> conntrack based on the zone in the past. That matches the description,
> but doesn't match the intent behind the capability flag. You probably
> should mention how this capability relates to the new OpenFlow request.
>
> Best regards, Ilya Maximets.
>
>
I've made the relation with the extension more explicit in v3.
Thanks,
Ales
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA
amu...@redhat.comIM: amusil
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 1/2] dpctl: Fix flush-conntrack with datapath as argument
On Thu, Mar 9, 2023 at 1:11 PM Ilya Maximets wrote:
> On 3/6/23 14:37, Ales Musil wrote:
> > Specifying datapath with "dpctl/flush-conntrack" didn't
> > work as expected and caused error:
> > ovs-dpctl: field system@ovs-system missing value (Invalid argument)
> >
> > To prevent that check if we have datapath as first argument
> > and use it accordingly.
> >
> > Signed-off-by: Ales Musil
>
> This needs a Fixes tag.
>
> > ---
> > v2: Add test case.
> > ---
> > lib/dpctl.c | 17 -
> > tests/system-traffic.at | 2 ++
> > 2 files changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/dpctl.c b/lib/dpctl.c
> > index c501a0cd7..a7a4d8ee8 100644
> > --- a/lib/dpctl.c
> > +++ b/lib/dpctl.c
> > @@ -1717,10 +1717,16 @@ dpctl_flush_conntrack(int argc, const char
> *argv[],
> > uint16_t zone, *pzone = NULL;
> > int error;
> > int args = argc - 1;
> > +int zone_pos = 1;
> > +
> > +if (dp_arg_exists(argc, argv)) {
> > +args--;
> > +zone_pos = 2;
> > +}
> >
> > /* Parse zone. */
> > -if (args && !strncmp(argv[1], "zone=", 5)) {
> > -if (!ovs_scan(argv[1], "zone=%"SCNu16, )) {
> > +if (args && !strncmp(argv[zone_pos], "zone=", 5)) {
> > +if (!ovs_scan(argv[zone_pos], "zone=%"SCNu16, )) {
> > ds_put_cstr(, "failed to parse zone");
> > error = EINVAL;
> > goto error;
> > @@ -1747,13 +1753,6 @@ dpctl_flush_conntrack(int argc, const char
> *argv[],
> > args--;
> > }
> >
> > -/* Report error if there is more than one unparsed argument. */
> > -if (args > 1) {
> > -ds_put_cstr(, "invalid arguments");
> > -error = EINVAL;
> > -goto error;
> > -}
> > -
> > error = opt_dpif_open(argc, argv, dpctl_p, 5, );
> > if (error) {
> > dpctl_error(dpctl_p, error, "Cannot open dpif");
> > diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> > index 380372430..c6ec61dba 100644
> > --- a/tests/system-traffic.at
> > +++ b/tests/system-traffic.at
> > @@ -2360,8 +2360,10 @@
> priority=100,in_port=2,icmp,action=ct(zone=5,commit),1
> > ])
> >
> > AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> > +dp=$(ovs-appctl dpctl/dump-dps)
> >
> > m4_foreach([FLUSH_CMD], [[ovs-appctl dpctl/flush-conntrack],
> > + [ovs-appctl dpctl/flush-conntrack $dp],
> > [ovs-ofctl ct-flush br0]], [
> > AS_BOX([Testing with FLUSH_CMD])
> >
>
> It looks like we don't have tests for a case where FLUSH_CMD is called
> without any arguments while all of them are optional. Could you, please,
> add one?
>
> Best regards, Ilya Maximets.
>
>
Both added in v3.
Thanks,
Ales
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA
amu...@redhat.comIM: amusil
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 6/6] netdev-dpdk-offload: Add support for meter action
References: <20230309130252.356289-7-simon.hor...@corigine.com> Bleep bloop. Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or co-authors or committers: Jin Liu , Simon Horman Lines checked: 49, Warnings: 1, Errors: 0 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
Re: [ovs-dev] [PATCH v2 5/6] netdev-dpdk: add meter algorithms
References: <20230309130252.356289-6-simon.hor...@corigine.com> Bleep bloop. Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or co-authors or committers: Jin Liu , Simon Horman Lines checked: 98, Warnings: 1, Errors: 0 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
Re: [ovs-dev] [PATCH v2 4/6] netdev-offload-dpdk: Implement meter offload API for DPDK
References: <20230309130252.356289-5-simon.hor...@corigine.com> Bleep bloop. Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or co-authors or committers: Jin Liu , Simon Horman Lines checked: 410, Warnings: 1, Errors: 0 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
Re: [ovs-dev] [PATCH v3 1/2] github: Combine ASAN and UBSAN runs.
On Wed, Mar 08, 2023 at 04:18:02PM +0100, Eelco Chaudron wrote: > Combine the ASAN and UBSAN tests into a single sanitizer run. > > Signed-off-by: Eelco Chaudron Reviewed-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 3/6] dpif-netdev: Offloading meter with DPDK
References: <20230309130252.356289-4-simon.hor...@corigine.com> Bleep bloop. Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or co-authors or committers: Jin Liu , Simon Horman Lines checked: 69, Warnings: 1, Errors: 0 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
Re: [ovs-dev] [PATCH v2 2/6] netdev-offload: Add DPDK meter offload API
References: <20230309130252.356289-3-simon.hor...@corigine.com> Bleep bloop. Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or co-authors or committers: Jin Liu , Simon Horman Lines checked: 224, Warnings: 1, Errors: 0 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
Re: [ovs-dev] [PATCH v2 1/6] netdev-offload-dpdk: use flow transfer proxy
References: <20230309130252.356289-2-simon.hor...@corigine.com> Bleep bloop. Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or co-authors or committers: Jin Liu , Simon Horman Lines checked: 147, Warnings: 1, Errors: 0 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
Re: [ovs-dev] [PATCH ovn] northd: Use generic ct.est flows for LR LBs
On Thu, Mar 09, 2023 at 07:21:44AM +0100, Ales Musil wrote:
> Currently, there is one ct.est flow per LB VIP,
> that was required to keep track if we need to
> pass the "skip_snat" or "force_snat" flags.
> However since c1d6b8ac ("northd: Store skip_snat and force_snat in
> ct_label/mark")
> the flags are carried in the ct entry and
> we can use match on them the same way we do
> for related traffic.
>
> Simplify the logic for established
> traffic through load balancers, by removing
> the requirement for one ct.est flow per VIP
> and replacing them with three generic ct.est flows:
> match=(ct.est && !ct.rel && !ct.new && ct_mark.natted), action=(next;)
> match=(ct.est && !ct.rel && !ct.new && ct_mark.natted && ct_mark.force_snat
> == 1), action=(flags.force_snat_for_lb = 1; next;)
> match=(ct.est && !ct.rel && !ct.new && ct_mark.natted && ct_mark.skip_snat ==
> 1), action=(flags.skip_snat_for_lb = 1; next;)
>
> This allows us avoiding of matching on L4
> in defrag stage by not storing the L3 and L4
> destination in registers. Match directly on
> L3 and L4 destination for ct.new in DNAT stage.
>
> Populate the registers in LB affinity check stage
> as they are needed for LB affinity learn.
>
> Reported-at: https://bugzilla.redhat.com/2172048
> Reported-at: https://bugzilla.redhat.com/2170885
> Signed-off-by: Ales Musil
Reviewed-by: Simon Horman
> @@ -10475,10 +10485,21 @@ build_distr_lrouter_nat_flows_for_lb(struct
> lrouter_nat_lb_flows_ctx *ctx,
> enum lrouter_nat_lb_flow_type type,
> struct ovn_datapath *od)
> {
> -char *gw_action = od->is_gw_router ? "ct_dnat;" : "ct_dnat_in_czone;";
> +char *undnat_action;
nit: I think this could be const.
nit: Maybe a blank line here.
> +switch (type) {
> +case LROUTER_NAT_LB_FLOW_FORCE_SNAT:
> +undnat_action = "flags.force_snat_for_lb = 1; next;";
> +break;
> +case LROUTER_NAT_LB_FLOW_SKIP_SNAT:
> +undnat_action = "flags.skip_snat_for_lb = 1; next;";
> +break;
> +case LROUTER_NAT_LB_FLOW_NORMAL:
> +case LROUTER_NAT_LB_FLOW_MAX:
> +undnat_action = od->is_gw_router ? "ct_dnat;" : "ct_dnat_in_czone;";
> +break;
> +}
> /* Store the match lengths, so we can reuse the ds buffer. */
> size_t new_match_len = ctx->new_match->length;
> -size_t est_match_len = ctx->est_match->length;
> size_t undnat_match_len = ctx->undnat_match->length;
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4 7/7] ci: Replace clang jemalloc suite with system-test-userspace
On Wed, Mar 08, 2023 at 03:32:11PM +0100, Ales Musil wrote: > The test with jemalloc and clang is redundant as > we are already testing jemalloc with gcc. Replace > it with system-test-userspace which runs system tests > over userspace OvS datapath. > > Tested-by: Simon Horman > Signed-off-by: Ales Musil Reviewed-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4 6/7] system-tests: Move the LB affinity tests into system-ovn-kmod
On Wed, Mar 08, 2023 at 03:32:10PM +0100, Ales Musil wrote: > To be consistent moe the LB affinity tests into system-ovn-kmod > for now as they are waiting currently not working with > userspace datapath. > > Signed-off-by: Ales Musil Reviewed-by: Simon Horman Tested-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4 5/7] tests: Use revalidator/purge instead of dpctl/del-flows
On Wed, Mar 08, 2023 at 03:32:09PM +0100, Ales Musil wrote: > The dpctl/del-flows shouldn't be used on running > ofproto layer, use revalidator/purge instead. > > Signed-off-by: Ales Musil Reviewed-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4 4/7] system-tests: Replace use of ADD_INT with ADD_VETH
On Wed, Mar 08, 2023 at 03:32:08PM +0100, Ales Musil wrote: > The ADD_INT does not work very well with userspace datapath. > When we move the interface into separate namespace > the ovs-vswitch cannot get ethtool info from that interface > and reports errors for some calls: > > |00089|netdev_linux|WARN|ethtool command ETHTOOL_GSET on network device vm1 > failed: No such device > |00091|netdev_linux|WARN|ethtool command ETHTOOL_GDRVINFO on network device > vm1 failed: No such device > > To avoid any warnings that might fail the tests > use ADD_VETH instead. Also remove ADD_INT as it > is not used anywhere else. > > Signed-off-by: Ales Musil Reviewed-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4 3/7] system-tests: Do not use verbose output for ping6
On Wed, Mar 08, 2023 at 03:32:07PM +0100, Ales Musil wrote: > Newer versions of ping6 print some info to > stderr when specified with -v flag. This info > causes tests to fail. Do not use the verbose > output. > > It reports following info into stderr > which is then represtend as error by the > NS_CHECK_EXEC: > > ping6: sock4.fd: -1 (socktype: 0), sock6.fd: 3 (socktype: SOCK_RAW), > hints.ai_family: AF_INET6 > > There is also possibility to ignore > the stderr, but it doesn't bring any value > to the test. > > Signed-off-by: Ales Musil Reviewed-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4 1/7] ci: Add support for userspace system test
On Wed, Mar 08, 2023 at 03:32:05PM +0100, Ales Musil wrote: > Add support for running system tests with > OvS userspace netdev. > > Signed-off-by: Ales Musil Reviewed-by: Simon Horman Reviewed-by: Simon Horman ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 4/6] netdev-offload-dpdk: Implement meter offload API for DPDK
From: Peng Zhang
For dpif-netdev, meters are mapped by DPDK meter with one-to-one
relationship. Implement meter offload API to set/get/del the DPDK
meter with proxy port id.
Signed-off-by: Peng Zhang
Signed-off-by: Jin Liu
Signed-off-by: Simon Horman
---
lib/netdev-dpdk.c | 202 ++
lib/netdev-dpdk.h | 41
lib/netdev-offload-dpdk.c | 89 +
3 files changed, 332 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index d6f2f0517da6..cc2d0762226f 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -5330,8 +5331,209 @@ netdev_dpdk_rte_flow_query_count(struct netdev *netdev,
return ret;
}
+static int OVS_UNUSED
+netdev_dpdk_meter_profile_init(struct rte_mtr_meter_profile *profile,
+ struct rte_mtr_capabilities *cap,
+ const uint64_t rate,
+ const uint64_t burst,
+ const int flag)
+{
+if (!cap->meter_srtcm_rfc2697_n_max) {
+return EOPNOTSUPP;
+}
+
+profile->alg = RTE_MTR_SRTCM_RFC2697;
+profile->packet_mode = flag;
+profile->srtcm_rfc2697.cir = rate;
+profile->srtcm_rfc2697.cbs = burst;
+profile->srtcm_rfc2697.ebs = burst;
+
+return 0;
+}
+
#ifdef ALLOW_EXPERIMENTAL_API
+static int
+netdev_dpdk_rte_mtr_meter_add(struct rte_mtr_meter_profile *profile,
+ struct netdev *netdev,
+ uint32_t meter_id,
+ const uint32_t rate,
+ const uint32_t burst,
+ const int flag,
+ struct rte_mtr_error *error)
+{
+struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+uint32_t meter_profile_id = meter_id;
+uint32_t meter_policy_id = meter_id;
+struct rte_mtr_capabilities cap;
+struct rte_mtr_stats mtr_stats;
+struct rte_mtr_params params;
+uint64_t stats_mask = 0;
+int prox_port_id;
+int clear = 0;
+int mod;
+int ret;
+
+memset(_stats, 0, sizeof(struct rte_mtr_stats));
+memset(, 0, sizeof(cap));
+
+ovs_mutex_lock(>mutex);
+
+prox_port_id = dev->flow_transfer_proxy_port_id;
+ret = rte_mtr_capabilities_get(prox_port_id, , error);
+if (ret) {
+goto out;
+}
+
+ret = netdev_dpdk_meter_profile_init(profile, , rate, burst, flag);
+if (ret) {
+goto out;
+}
+
+/* If can get the meter stats, the meter is offload in the HW.
+ * So the operate is mod, just update the meter_profile.
+ *
+ * If can't get the meter stats, the meter is not offload in the HW.
+ * So the operate is add, need create the profile, policy, mtr. */
+mod = rte_mtr_stats_read(prox_port_id, meter_id, _stats, _mask,
+ clear, error);
+ret = rte_mtr_meter_profile_add(prox_port_id, meter_profile_id, profile,
+error);
+if (!mod || ret) {
+goto out;
+}
+
+rte_mtr_policy_drop_red(policy);
+ret = rte_mtr_meter_policy_add(prox_port_id, meter_policy_id, ,
+ error);
+
+if (ret) {
+goto out;
+}
+
+memset(, 0 , sizeof(struct rte_mtr_params));
+params.meter_profile_id = meter_profile_id;
+params.meter_policy_id = meter_policy_id;
+params.stats_mask = cap.stats_mask;
+params.meter_enable = 1;
+
+ret = rte_mtr_create(prox_port_id, meter_id, , 1, error);
+out:
+ovs_mutex_unlock(>mutex);
+return ret;
+}
+
+int
+netdev_dpdk_meter_create(struct netdev *netdev,
+ const uint32_t meter_profile_id,
+ const uint64_t rate,
+ const uint64_t burst,
+ const int flag)
+{
+struct rte_mtr_meter_profile profile;
+struct rte_mtr_error error;
+int ret;
+
+memset(, 0 , sizeof(struct rte_mtr_meter_profile));
+memset(, 0 , sizeof(struct rte_mtr_error));
+
+ret = netdev_dpdk_rte_mtr_meter_add(, netdev, meter_profile_id,
+rate, burst, flag, );
+if (!ret) {
+if (!VLOG_DROP_DBG()) {
+VLOG_DBG("%s: rte_meter_id %d port_id %d mtr create ",
+ netdev_get_name(netdev), meter_profile_id,
+ netdev_dpdk_get_prox_port_id(netdev));
+}
+} else {
+VLOG_DBG("%s: rte_mtr creation failed: %d (%s).",
+ netdev_get_name(netdev), error.type, error.message);
+}
+return ret;
+}
+
+int
+netdev_dpdk_meter_del(struct netdev *netdev,
+ const uint32_t meter_id,
+ const uint32_t meter_profile_id,
+ const uint32_t meter_policy_id)
+{
+struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+
[ovs-dev] [PATCH v2 5/6] netdev-dpdk: add meter algorithms
From: Peng Zhang
Add the meter algorithms. DPDK meter support three algorithms,
and OVS also can support these algorithms.
Signed-off-by: Peng Zhang
Signed-off-by: Jin Liu
Signed-off-by: Simon Horman
---
lib/netdev-dpdk.c | 58 +--
1 file changed, 51 insertions(+), 7 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index cc2d0762226f..2ce95aed2455 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -5331,6 +5331,50 @@ netdev_dpdk_rte_flow_query_count(struct netdev *netdev,
return ret;
}
+/* RTE_MTR_TRTCM_RFC2697 meter profile */
+static void
+netdev_dpdk_meter_profile_rfc2697_init(struct rte_mtr_meter_profile *profile,
+ const uint64_t rate,
+ const uint64_t burst,
+ const int flag)
+{
+ profile->alg = RTE_MTR_SRTCM_RFC2697;
+ profile->packet_mode = flag;
+ profile->srtcm_rfc2697.cir = rate;
+ profile->srtcm_rfc2697.cbs = burst;
+ profile->srtcm_rfc2697.ebs = burst;
+}
+
+/* RTE_MTR_TRTCM_RFC2698 meter profile */
+static void
+netdev_dpdk_meter_profile_rfc2698_init(struct rte_mtr_meter_profile *profile,
+ const uint64_t rate,
+ const uint64_t burst,
+ const int flag)
+{
+ profile->alg = RTE_MTR_TRTCM_RFC2698;
+ profile->packet_mode = flag;
+ profile->trtcm_rfc2698.cir = rate;
+ profile->trtcm_rfc2698.cbs = burst;
+ profile->trtcm_rfc2698.pir = rate;
+ profile->trtcm_rfc2698.pbs = burst;
+}
+
+/* RTE_MTR_TRTCM_RFC2698 meter profile */
+static void
+netdev_dpdk_meter_profile_rfc4115_init(struct rte_mtr_meter_profile *profile,
+ const uint64_t rate,
+ const uint64_t burst,
+ const int flag)
+{
+ profile->alg = RTE_MTR_TRTCM_RFC4115;
+ profile->packet_mode = flag;
+ profile->trtcm_rfc4115.cir = rate;
+ profile->trtcm_rfc4115.cbs = burst;
+ profile->trtcm_rfc4115.eir = rate;
+ profile->trtcm_rfc4115.ebs = burst;
+}
+
static int OVS_UNUSED
netdev_dpdk_meter_profile_init(struct rte_mtr_meter_profile *profile,
struct rte_mtr_capabilities *cap,
@@ -5338,16 +5382,16 @@ netdev_dpdk_meter_profile_init(struct
rte_mtr_meter_profile *profile,
const uint64_t burst,
const int flag)
{
-if (!cap->meter_srtcm_rfc2697_n_max) {
+if (cap->meter_srtcm_rfc2697_n_max) {
+netdev_dpdk_meter_profile_rfc2697_init(profile, rate, burst, flag);
+} else if (cap->meter_trtcm_rfc2698_n_max) {
+netdev_dpdk_meter_profile_rfc2698_init(profile, rate, burst, flag);
+} else if (cap->meter_trtcm_rfc4115_n_max) {
+netdev_dpdk_meter_profile_rfc4115_init(profile, rate, burst, flag);
+} else {
return EOPNOTSUPP;
}
-profile->alg = RTE_MTR_SRTCM_RFC2697;
-profile->packet_mode = flag;
-profile->srtcm_rfc2697.cir = rate;
-profile->srtcm_rfc2697.cbs = burst;
-profile->srtcm_rfc2697.ebs = burst;
-
return 0;
}
--
2.30.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 6/6] netdev-dpdk-offload: Add support for meter action
From: Peng Zhang
Add support of DPDK meter action logic.
Signed-off-by: Peng Zhang
Signed-off-by: Jin Liu
Signed-off-by: Simon Horman
---
lib/netdev-offload-dpdk.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index faa878cb200c..ac93c01cbac9 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -2114,6 +2114,16 @@ parse_clone_actions(struct netdev *netdev,
return 0;
}
+static void OVS_UNUSED
+parse_meter_action(struct flow_actions *actions, uint32_t meter_id)
+{
+struct rte_flow_action_meter *rte_meter;
+
+rte_meter = xzalloc(sizeof *rte_meter);
+rte_meter->mtr_id = meter_id;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_METER, rte_meter);
+}
+
static void
add_jump_action(struct flow_actions *actions, uint32_t group)
{
@@ -2220,6 +2230,9 @@ parse_flow_actions(struct netdev *netdev,
if (add_tnl_pop_action(netdev, actions, nla)) {
return -1;
}
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_METER) {
+uint32_t meter_id = nl_attr_get_u32(nla);
+parse_meter_action(actions, meter_id);
#endif
} else {
VLOG_DBG_RL(, "Unsupported action type %d", nl_attr_type(nla));
--
2.30.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 3/6] dpif-netdev: Offloading meter with DPDK
From: Peng Zhang
OVS-DPDK meters are created in advance and OpenFlow rules refer to them by
their unique ID. A new API is used to offload them. By calling the API,
meters are created and try to be offload by port in the bridge with the
proxy port id.
Signed-off-by: Peng Zhang
Signed-off-by: Jin Liu
Signed-off-by: Simon Horman
---
lib/dpif-netdev.c | 22 +-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 2c08a71c8db2..e8d0ca6606de 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7277,6 +7277,11 @@ dpif_netdev_meter_set(struct dpif *dpif,
ofproto_meter_id meter_id,
ovs_mutex_unlock(>meters_lock);
+if (netdev_is_flow_api_enabled()) {
+dpdk_meter_offload_set(dpif_normalize_type(dpif_type(dpif)),
+ meter_id, config);
+}
+
return 0;
}
@@ -7313,8 +7318,18 @@ dpif_netdev_meter_get(const struct dpif *dpif,
ovs_mutex_unlock(>lock);
stats->n_bands = i;
-}
+if (netdev_is_flow_api_enabled()) {
+dpdk_meter_offload_get(dpif_normalize_type(dpif_type(dpif)),
+ meter_id_, stats);
+
+/* nit: Meter offload currently only supports one band */
+if (meter->n_bands) {
+stats->bands[0].packet_count = stats->packet_in_count;
+stats->bands[0].byte_count = stats->byte_in_count;
+}
+}
+}
return 0;
}
@@ -7330,6 +7345,11 @@ dpif_netdev_meter_del(struct dpif *dpif,
if (!error) {
uint32_t meter_id = meter_id_.uint32;
+if (netdev_is_flow_api_enabled()) {
+dpdk_meter_offload_del(dpif_normalize_type(dpif_type(dpif)),
+ meter_id_, stats);
+}
+
ovs_mutex_lock(>meters_lock);
dp_meter_detach_free(>meters, meter_id);
ovs_mutex_unlock(>meters_lock);
--
2.30.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 2/6] netdev-offload: Add DPDK meter offload API
From: Peng Zhang
Add API to offload DPDK meter to HW, and the corresponding functions to call
the DPDK meter callbacks from all the registered flow API providers.
The interfaces are like those related to DPDK meter in dpif_class, in order
to pass necessary info to HW.
Signed-off-by: Peng Zhang
Signed-off-by: Jin Liu
Signed-off-by: Simon Horman
---
Documentation/howto/dpdk.rst | 5 +-
lib/netdev-offload-provider.h | 30 +++
lib/netdev-offload.c | 99 +++
lib/netdev-offload.h | 9
4 files changed, 141 insertions(+), 2 deletions(-)
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 04609b20bd21..02fc568770ee 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -401,10 +401,11 @@ Supported actions for hardware offload are:
- Modification of IPv6 (set_field:->ipv6_src/ipv6_dst/mod_nw_ttl).
- Clone/output (tnl_push and output) for encapsulating over a tunnel.
- Tunnel pop, for packets received on physical ports.
+- Meter.
.. note::
- Tunnel offloads are experimental APIs in DPDK. In order to enable it,
- compile with -DALLOW_EXPERIMENTAL_API.
+ Tunnel offloads and Meter offloads are experimental APIs in DPDK. To enable
+ these features, compile with -DALLOW_EXPERIMENTAL_API.
Multiprocess
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 9108856d18d1..7ecbb8d026f1 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -102,6 +102,16 @@ struct netdev_flow_api {
int (*meter_set)(ofproto_meter_id meter_id,
struct ofputil_meter_config *config);
+/* Offloads or modifies the offloaded meter on the netdev with the given
+ * 'meter_id' and the configuration in 'config'. On failure, a non-zero
+ * error code is returned.
+ *
+ * The meter id specified through 'config->meter_id' is converted as an
+ * internal meter id. */
+int (*dpdk_meter_set)(struct netdev *,
+ ofproto_meter_id meter_id,
+ struct ofputil_meter_config *);
+
/* Queries HW for meter stats with the given 'meter_id'. Store the stats
* of dropped packets to band 0. On failure, a non-zero error code is
* returned.
@@ -113,6 +123,18 @@ struct netdev_flow_api {
int (*meter_get)(ofproto_meter_id meter_id,
struct ofputil_meter_stats *stats);
+/* Queries netdev for meter stats with the given 'meter_id'. Store the
+ * stats of dropped packets to band 0. On failure, a non-zero error code
+ * is returned.
+ *
+ * Note that the 'stats' structure is already initialized, and only the
+ * available statistics should be incremented, not replaced. Those fields
+ * are packet_in_count, byte_in_count and band[]->byte_count and
+ * band[]->packet_count. */
+int (*dpdk_meter_get)(struct netdev *,
+ ofproto_meter_id meter_id,
+ struct ofputil_meter_stats *);
+
/* Removes meter 'meter_id' from HW. Store the stats of dropped packets to
* band 0. On failure, a non-zero error code is returned.
*
@@ -121,6 +143,14 @@ struct netdev_flow_api {
int (*meter_del)(ofproto_meter_id meter_id,
struct ofputil_meter_stats *stats);
+/* Removes meter 'meter_id' from netdev. Store the stats of dropped packets
+ * to band 0. On failure, a non-zero error code is returned.
+ *
+ * If del success, 'stats' will be set zero. */
+int (*dpdk_meter_del)(struct netdev *,
+ ofproto_meter_id meter_id,
+ struct ofputil_meter_stats *stats);
+
/* Initializies the netdev flow api.
* Return 0 if successful, otherwise returns a positive errno value. */
int (*init_flow_api)(struct netdev *);
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index 4592262bd34e..44cab0c4ee79 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -895,3 +895,102 @@ netdev_set_flow_api_enabled(const struct smap
*ovs_other_config)
}
}
}
+
+void
+dpdk_meter_offload_set(const char *dpif_type,
+ ofproto_meter_id meter_id,
+ struct ofputil_meter_config *config)
+{
+struct netdev_registered_flow_api *rfa;
+struct port_to_netdev_data *data;
+struct netdev *dev;
+int ret;
+
+ovs_rwlock_rdlock(_hmap_rwlock);
+HMAP_FOR_EACH (data, portno_node, _to_netdev) {
+dev = data->netdev;
+if (netdev_get_dpif_type(dev) == dpif_type) {
+/* Offload APIs could fail, for example, because the offload is not
+ * supported. This is fine, as the offload API should take care of
+ * this. */
+CMAP_FOR_EACH (rfa, cmap_node, _flow_apis) {
+if (rfa->flow_api->dpdk_meter_set) {
+ret =
[ovs-dev] [PATCH v2 0/6] Add support for DPDK meter HW offload
Hi, this series adds support for DPDK meter HW offload. Peng Zhang (6): netdev-offload-dpdk: use flow transfer proxy netdev-offload: Add DPDK meter offload API dpif-netdev: Offloading meter with DPDK netdev-offload-dpdk: Implement meter offload API for DPDK netdev-dpdk: add meter algorithms netdev-dpdk-offload: Add support for meter action Changes since v1: * Add the prox mechanism: add the meter by proxy id * Change the offload interface from netdev-dpdk to the netdev-offload * Changed base to dpdk-latest branch Workflow Runs - one per patch: [PATCH 1/6] https://github.com/shorman-corigine/ovs/actions/runs/4372584064 [PATCH 2/6] https://github.com/shorman-corigine/ovs/actions/runs/4372584769 [PATCH 3/6] https://github.com/shorman-corigine/ovs/actions/runs/4372586659 [PATCH 4/6] https://github.com/shorman-corigine/ovs/actions/runs/4372587084 [PATCH 5/6] https://github.com/shorman-corigine/ovs/actions/runs/4372587699 [PATCH 6/6] https://github.com/shorman-corigine/ovs/actions/runs/4372588350 Documentation/howto/dpdk.rst | 5 +- lib/dpif-netdev.c | 22 ++- lib/netdev-dpdk.c | 306 +- lib/netdev-dpdk.h | 43 + lib/netdev-offload-dpdk.c | 102 lib/netdev-offload-provider.h | 30 lib/netdev-offload.c | 99 +++ lib/netdev-offload.h | 9 + 8 files changed, 611 insertions(+), 5 deletions(-) -- 2.30.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 1/6] netdev-offload-dpdk: use flow transfer proxy
From: Peng Zhang
Manage "transfer" flows via the corresponding mechanism.
Doing so requires that the traffic source be specified
explicitly, via the corresponding pattern item.
Signed-off-by: Peng Zhang
Signed-off-by: Jin Liu
Signed-off-by: Simon Horman
---
lib/netdev-dpdk.c | 60 +--
lib/netdev-dpdk.h | 2 ++
2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index fff57f78279a..d6f2f0517da6 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -437,6 +437,7 @@ enum dpdk_hw_ol_features {
struct netdev_dpdk {
PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
dpdk_port_t port_id;
+dpdk_port_t flow_transfer_proxy_port_id;
/* If true, device was attached by rte_eth_dev_attach(). */
bool attached;
@@ -1155,6 +1156,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
uint32_t rx_chksm_offload_capa = RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+int ret;
+
+/* Managing "transfer" flows requires that the user communicate them
+ * via a port which has the privilege to control the embedded switch.
+ * For some vendors, all ports in a given switching domain have
+ * this privilege. For other vendors, it's only one port.
+ *
+ * Get the proxy port ID and remember it for later use.
+ */
+ret = rte_flow_pick_transfer_proxy(dev->port_id,
+ >flow_transfer_proxy_port_id,
+ NULL);
+if (ret != 0) {
+/* The PMD does not indicate the proxy port.
+ * Assume the proxy is unneeded.
+ */
+dev->flow_transfer_proxy_port_id = dev->port_id;
+}
rte_eth_dev_info_get(dev->port_id, );
@@ -3769,6 +3788,7 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
struct ds used_interfaces = DS_EMPTY_INITIALIZER;
struct rte_eth_dev_info dev_info;
dpdk_port_t sibling_port_id;
+struct netdev_dpdk *dev;
dpdk_port_t port_id;
bool used = false;
char *response;
@@ -3786,8 +3806,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
argv[1]);
RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
-struct netdev_dpdk *dev;
-
LIST_FOR_EACH (dev, list_node, _list) {
if (dev->port_id != sibling_port_id) {
continue;
@@ -3807,6 +3825,17 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
}
ds_destroy(_interfaces);
+/* The device being detached may happen to be a flow proxy port
+ * for another device (still attached). Update the flow proxy port id,
+ * indicate that the device being detached no longer needs a flow proxy.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+if (dev->port_id == port_id) {
+dev->flow_transfer_proxy_port_id = port_id;
+break;
+}
+}
+
rte_eth_dev_info_get(port_id, _info);
rte_eth_dev_close(port_id);
if (rte_dev_remove(dev_info.device) < 0) {
@@ -3817,6 +3846,16 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
response = xasprintf("All devices shared with device '%s' "
"have been detached", argv[1]);
+/* The device being detached may happen to be a flow proxy port.
+ * After the flow proxy port was detached, the related ports
+ * will reconfigure the device and update the proxy_port_id.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+ if (dev->flow_transfer_proxy_port_id == port_id) {
+netdev_request_reconfigure(>up);
+}
+}
+
ovs_mutex_unlock(_mutex);
unixctl_command_reply(conn, response);
free(response);
@@ -5192,6 +5231,23 @@ out:
return ret;
}
+int
+netdev_dpdk_get_prox_port_id(struct netdev *netdev)
+{
+struct netdev_dpdk *dev;
+int ret = -1;
+
+if (!is_dpdk_class(netdev->netdev_class)) {
+return ret;
+}
+
+dev = netdev_dpdk_cast(netdev);
+ovs_mutex_lock(>mutex);
+ret = dev->flow_transfer_proxy_port_id;
+ovs_mutex_unlock(>mutex);
+return ret;
+}
+
bool
netdev_dpdk_flow_api_supported(struct netdev *netdev)
{
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index 5cd95d00f5a5..277db156fe06 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -51,6 +51,8 @@ netdev_dpdk_rte_flow_query_count(struct netdev *netdev,
struct rte_flow_error *error);
int
netdev_dpdk_get_port_id(struct netdev *netdev);
+int
+netdev_dpdk_get_prox_port_id(struct netdev *netdev);
#ifdef ALLOW_EXPERIMENTAL_API
--
2.30.2
___
dev mailing list
d...@openvswitch.org
[ovs-dev] [PATCH v2] ofproto-dpif-upcall: Remove redundant time_msec() in revalidate().
Remove one of two consecutive time_msec() calls in the revalidate() function.
We take the time stamp after udpif_get_n_flows(), to avoid any potential
delays in getting the number of offloaded flows.
Signed-off-by: Eelco Chaudron
---
v2: Moved time_msec() after udpif_get_n_flows().
ofproto/ofproto-dpif-upcall.c |4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index db7570ee2..7d96a3300 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2712,8 +2712,6 @@ revalidate(struct revalidator *revalidator)
break;
}
-now = time_msec();
-
/* In normal operation we want to keep flows around until they have
* been idle for 'ofproto_max_idle' milliseconds. However:
*
@@ -2750,7 +2748,7 @@ revalidate(struct revalidator *revalidator)
max_idle = n_dp_flows > flow_limit ? 100 : ofproto_max_idle;
-udpif->dpif->current_ms = time_msec();
+udpif->dpif->current_ms = now = time_msec();
for (f = flows; f < [n_dumped]; f++) {
long long int used = f->stats.used;
struct recirc_refs recircs = RECIRC_REFS_EMPTY_INITIALIZER;
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 2/2] vswitch: Add missing documentation for "ct_flush" capability
On 3/6/23 14:37, Ales Musil wrote:
> Signed-off-by: Ales Musil
Needs a Fixes tag.
> ---
> vswitchd/vswitch.xml | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 12708a313..15e4f97b7 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -6301,6 +6301,11 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
> type=patch options:peer=p1 \
> translated to an ephemeral port. If there is no collision, no SNAT
> is performed.
>
> + + type='{"type": "boolean"}'>
> +True if the datapath supports CT flush extension. The extensions
> +allows to flush CT entries based on specified parameters.
This doesn't really describe the capability. It was possible to flush
conntrack based on the zone in the past. That matches the description,
but doesn't match the intent behind the capability flag. You probably
should mention how this capability relates to the new OpenFlow request.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 1/2] dpctl: Fix flush-conntrack with datapath as argument
On 3/6/23 14:37, Ales Musil wrote:
> Specifying datapath with "dpctl/flush-conntrack" didn't
> work as expected and caused error:
> ovs-dpctl: field system@ovs-system missing value (Invalid argument)
>
> To prevent that check if we have datapath as first argument
> and use it accordingly.
>
> Signed-off-by: Ales Musil
This needs a Fixes tag.
> ---
> v2: Add test case.
> ---
> lib/dpctl.c | 17 -
> tests/system-traffic.at | 2 ++
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index c501a0cd7..a7a4d8ee8 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -1717,10 +1717,16 @@ dpctl_flush_conntrack(int argc, const char *argv[],
> uint16_t zone, *pzone = NULL;
> int error;
> int args = argc - 1;
> +int zone_pos = 1;
> +
> +if (dp_arg_exists(argc, argv)) {
> +args--;
> +zone_pos = 2;
> +}
>
> /* Parse zone. */
> -if (args && !strncmp(argv[1], "zone=", 5)) {
> -if (!ovs_scan(argv[1], "zone=%"SCNu16, )) {
> +if (args && !strncmp(argv[zone_pos], "zone=", 5)) {
> +if (!ovs_scan(argv[zone_pos], "zone=%"SCNu16, )) {
> ds_put_cstr(, "failed to parse zone");
> error = EINVAL;
> goto error;
> @@ -1747,13 +1753,6 @@ dpctl_flush_conntrack(int argc, const char *argv[],
> args--;
> }
>
> -/* Report error if there is more than one unparsed argument. */
> -if (args > 1) {
> -ds_put_cstr(, "invalid arguments");
> -error = EINVAL;
> -goto error;
> -}
> -
> error = opt_dpif_open(argc, argv, dpctl_p, 5, );
> if (error) {
> dpctl_error(dpctl_p, error, "Cannot open dpif");
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 380372430..c6ec61dba 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -2360,8 +2360,10 @@ priority=100,in_port=2,icmp,action=ct(zone=5,commit),1
> ])
>
> AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> +dp=$(ovs-appctl dpctl/dump-dps)
>
> m4_foreach([FLUSH_CMD], [[ovs-appctl dpctl/flush-conntrack],
> + [ovs-appctl dpctl/flush-conntrack $dp],
> [ovs-ofctl ct-flush br0]], [
> AS_BOX([Testing with FLUSH_CMD])
>
It looks like we don't have tests for a case where FLUSH_CMD is called
without any arguments while all of them are optional. Could you, please,
add one?
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4] northd, lsp: Add additional arp proxy features
On Thu, Mar 9, 2023 at 10:37 AM Ales Musil wrote:
>
>
> On Thu, Mar 9, 2023 at 10:24 AM Dumitru Ceara wrote:
>
>> On 3/9/23 08:45, Ales Musil wrote:
>> > On Wed, Mar 8, 2023 at 3:26 PM Enrique Llorente
>> wrote:
>> >
>> >> Configure mac address
>> >> The mac address returned by ARP/NDP can be configured similar to LSP
>> >> addresses where the mac is the first entry on the list
>> >>
>> >> IPv6
>> >> Support NDP IPv6 protocol
>> >>
>> >> Use CIDRs
>> >> Allow to specify subnets for ipv4 and ipv6, they will match whatever
>> >> address is received from ARP/NDP
>> >>
>> >> Signed-off-by: Enrique Llorente
>> >>
>> >
>> > Hi,
>> >
>> > just two nits below, but there is probably no need to spin another
>> version.
>> >
>> >
>> >> ---
>> >> NEWS | 2 +
>> >> northd/northd.c | 129 +---
>> >> northd/ovn-northd.8.xml | 9 ++
>> >> ovn-nb.xml| 17 ++-
>> >> tests/ovn.at | 179 ++
>> >> tests/system-common-macros.at | 1 +
>> >> tests/system-ovn.at | 270
>> ++
>> >> 7 files changed, 551 insertions(+), 56 deletions(-)
>> >>
>> >> diff --git a/NEWS b/NEWS
>> >> index 900825b1e..d859ae19d 100644
>> >> --- a/NEWS
>> >> +++ b/NEWS
>> >> @@ -8,6 +8,8 @@ OVN v23.03.0 - 03 Mar 2023
>> >>- Add ovn-ctl commands for (re)starting/stopping OVSDB relay for
>> OVN SB
>> >> DB.
>> >>- Add new ovn-db@.service systemd unit to run ovsdb-servers in
>> separate
>> >> systemd units.
>> >> + - Enhance LSP.options:arp_proxy to support IPv6, configurable MAC
>> >> +addresses and CIDRs.
>> >>
>> >
>> > This needs to be under "Post v23.03.0" instead, however that can be
>> moved
>> > during merge.
>> >
>> >
>> >>
>> >> OVN v22.12.0 - 16 Dec 2022
>> >> --
>> >> diff --git a/northd/northd.c b/northd/northd.c
>> >> index 33025bb5c..c61b538e7 100644
>> >> --- a/northd/northd.c
>> >> +++ b/northd/northd.c
>> >> @@ -1547,6 +1547,8 @@ struct ovn_port {
>> >> struct ovs_list list; /* In list of similar records. */
>> >>
>> >> struct ovs_list dp_node;
>> >> +
>> >> +struct lport_addresses proxy_arp_addrs;
>> >> };
>> >>
>> >> static bool
>> >> @@ -1628,6 +1630,7 @@ ovn_port_create(struct hmap *ports, const char
>> *key,
>> >> op->sb = sb;
>> >> ovn_port_set_nb(op, nbsp, nbrp);
>> >> op->l3dgw_port = op->cr_port = NULL;
>> >> +op->proxy_arp_addrs.ea = eth_addr_zero;
>> >> hmap_insert(ports, >key_node, hash_string(op->key, 0));
>> >> return op;
>> >> }
>> >> @@ -1654,6 +1657,7 @@ ovn_port_destroy(struct hmap *ports, struct
>> ovn_port
>> >> *port)
>> >> destroy_routable_addresses(>routables);
>> >>
>> >> destroy_lport_addresses(>lrp_networks);
>> >> +destroy_lport_addresses(>proxy_arp_addrs);
>> >> free(port->json_key);
>> >> free(port->key);
>> >> free(port);
>> >> @@ -2704,6 +2708,20 @@ join_logical_ports(struct northd_input
>> *input_data,
>> >> if (peer->od && peer->od->mcast_info.rtr.relay) {
>> >> op->od->mcast_info.sw.flood_relay = true;
>> >> }
>> >> +
>> >> +/* For LSP of router type arp proxy can be activated so
>> >> + * it needs to be parsed
>> >> + * either takes "MAC IP1 IP2" or "IP1 IP2"
>> >> + */
>> >> +const char *arp_proxy =
>> >> smap_get(>nbsp->options,"arp_proxy");
>> >> +int ofs = 0;
>> >> +if (arp_proxy &&
>> >> +!extract_addresses(arp_proxy, >proxy_arp_addrs,
>> )
>> >> &&
>> >> +!extract_ip_addresses(arp_proxy,
>> >proxy_arp_addrs)) {
>> >> +static struct vlog_rate_limit rl =
>> >> VLOG_RATE_LIMIT_INIT(1, 5);
>> >> +VLOG_WARN_RL(, "Invalid arp_proxy option: '%s' at
>> lsp
>> >> '%s'",
>> >> + arp_proxy, op->nbsp->name);
>> >> +}
>> >> } else if (op->nbrp && op->nbrp->peer && !op->l3dgw_port) {
>> >> struct ovn_port *peer = ovn_port_find(ports,
>> op->nbrp->peer);
>> >> if (peer) {
>> >> @@ -8625,29 +8643,33 @@ build_lswitch_arp_nd_responder_known_ips(struct
>> >> ovn_port *op,
>> >> }
>> >> }
>> >> }
>> >> +if (op->proxy_arp_addrs.n_ipv4_addrs ||
>> >> +op->proxy_arp_addrs.n_ipv6_addrs) {
>> >> +/* Select the mac address to answer the proxy ARP/NDP */
>> >> +char *ea_s = NULL;
>> >> +if (!eth_addr_is_zero(op->proxy_arp_addrs.ea)) {
>> >> +ea_s = op->proxy_arp_addrs.ea_s;
>> >> +} else if (op->peer) {
>> >> +ea_s = op->peer->lrp_networks.ea_s;
>> >> +} else {
>> >> +return;
>> >> +}
>> >>
>> >> -if (op->peer) {
>> >> -const char *arp_proxy =
>> >>
Re: [ovs-dev] Proposal: Move dceara/ovn-heater to ovn-org/ovn-heater?
On 3/9/23 08:08, Frode Nordahl wrote: > On Wed, Mar 8, 2023 at 5:27 PM Dumitru Ceara wrote: >> >> On 3/8/23 16:07, Ilya Maximets wrote: >>> On 3/8/23 15:32, Dumitru Ceara wrote: On Wed, Mar 8, 2023 at 3:15 PM Dumitru Ceara wrote: > > Hi all, > > Frode brought up during last week's OVN IRC meeting the possibility of > moving ovn-heater [0] to the ovn-org [1] organization. > > For some context: today ovn-heater uses containers on multiple bare > metal machines to simulate OVN nodes provisioned by ovn-kubernetes > (already part of ovn-org [2]) in various scenarios. This is used to > benchmark OVN (and ovsdb-server and other OVS components) at scale in > these scenarios. > > We (at Red Hat) have been using ovn-heater consistently (weekly runs) > throughout the last year in order to benchmark OVN in OpenShift like > scenarios in clusters scaling up to 500 nodes. These runs have already > helped us pin-point a lot of the scale related issues (both OVN and > ovsdb-server) that have been fixed in the last months/year. > > The core of ovn-heater is (rather simple) python code so there should be > no reason that would block us from adding more scale test scenarios to > simulate clusters provisioned by other CMS (e.g., OpenStack) turning > ovn-heater into a generic OVN scale testing tool. > > I cc-ed all the OVN maintainers and all GitHub ovn-org owners but > feedback from the whole community is more than welcome! >>> >>> As a piece of feedback from the contributor to that project, >>> I think it needs a contribution guide and a documented >>> contribution policy such as adoption of DCO, for example. >>> AUTHORS file may need some updates. >>> >>> I'm pretty sure that all the contributors so far had a right >>> to contribute their code. But these policies might be good >>> to iron out before "going public". >> >> Many thanks for pointing this out! I opened an initial PR to take care >> of some of this: >> >> https://github.com/dceara/ovn-heater/pull/155 >> >>> >>> DCO bot is in use by some of the existing ovn-org projects, >>> so it may help enforcing policies after migration. >>> >> >> Sounds applicable indeed. >> >>> My 2c. >>> >>> Best regards, Ilya Maximets. >>> >> >> Thanks, >> Dumitru >> > > Thanks for bringing this up, Dumitru, and I wanted to reiterate our > interest in the project here. Parts of the repository are universally > applicable for OVN scale testing and could be useful for other CMSs > and deployment scenarios. > > We would like to deploy it in a slightly different way, so I'll be > working on a proposal for how we could decouple the deployment > specific and generic parts and hopefully put up a PR today. > Sure, looking forward to it! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4] northd, lsp: Add additional arp proxy features
On Thu, Mar 9, 2023 at 10:24 AM Dumitru Ceara wrote:
> On 3/9/23 08:45, Ales Musil wrote:
> > On Wed, Mar 8, 2023 at 3:26 PM Enrique Llorente
> wrote:
> >
> >> Configure mac address
> >> The mac address returned by ARP/NDP can be configured similar to LSP
> >> addresses where the mac is the first entry on the list
> >>
> >> IPv6
> >> Support NDP IPv6 protocol
> >>
> >> Use CIDRs
> >> Allow to specify subnets for ipv4 and ipv6, they will match whatever
> >> address is received from ARP/NDP
> >>
> >> Signed-off-by: Enrique Llorente
> >>
> >
> > Hi,
> >
> > just two nits below, but there is probably no need to spin another
> version.
> >
> >
> >> ---
> >> NEWS | 2 +
> >> northd/northd.c | 129 +---
> >> northd/ovn-northd.8.xml | 9 ++
> >> ovn-nb.xml| 17 ++-
> >> tests/ovn.at | 179 ++
> >> tests/system-common-macros.at | 1 +
> >> tests/system-ovn.at | 270 ++
> >> 7 files changed, 551 insertions(+), 56 deletions(-)
> >>
> >> diff --git a/NEWS b/NEWS
> >> index 900825b1e..d859ae19d 100644
> >> --- a/NEWS
> >> +++ b/NEWS
> >> @@ -8,6 +8,8 @@ OVN v23.03.0 - 03 Mar 2023
> >>- Add ovn-ctl commands for (re)starting/stopping OVSDB relay for OVN
> SB
> >> DB.
> >>- Add new ovn-db@.service systemd unit to run ovsdb-servers in
> separate
> >> systemd units.
> >> + - Enhance LSP.options:arp_proxy to support IPv6, configurable MAC
> >> +addresses and CIDRs.
> >>
> >
> > This needs to be under "Post v23.03.0" instead, however that can be
> moved
> > during merge.
> >
> >
> >>
> >> OVN v22.12.0 - 16 Dec 2022
> >> --
> >> diff --git a/northd/northd.c b/northd/northd.c
> >> index 33025bb5c..c61b538e7 100644
> >> --- a/northd/northd.c
> >> +++ b/northd/northd.c
> >> @@ -1547,6 +1547,8 @@ struct ovn_port {
> >> struct ovs_list list; /* In list of similar records. */
> >>
> >> struct ovs_list dp_node;
> >> +
> >> +struct lport_addresses proxy_arp_addrs;
> >> };
> >>
> >> static bool
> >> @@ -1628,6 +1630,7 @@ ovn_port_create(struct hmap *ports, const char
> *key,
> >> op->sb = sb;
> >> ovn_port_set_nb(op, nbsp, nbrp);
> >> op->l3dgw_port = op->cr_port = NULL;
> >> +op->proxy_arp_addrs.ea = eth_addr_zero;
> >> hmap_insert(ports, >key_node, hash_string(op->key, 0));
> >> return op;
> >> }
> >> @@ -1654,6 +1657,7 @@ ovn_port_destroy(struct hmap *ports, struct
> ovn_port
> >> *port)
> >> destroy_routable_addresses(>routables);
> >>
> >> destroy_lport_addresses(>lrp_networks);
> >> +destroy_lport_addresses(>proxy_arp_addrs);
> >> free(port->json_key);
> >> free(port->key);
> >> free(port);
> >> @@ -2704,6 +2708,20 @@ join_logical_ports(struct northd_input
> *input_data,
> >> if (peer->od && peer->od->mcast_info.rtr.relay) {
> >> op->od->mcast_info.sw.flood_relay = true;
> >> }
> >> +
> >> +/* For LSP of router type arp proxy can be activated so
> >> + * it needs to be parsed
> >> + * either takes "MAC IP1 IP2" or "IP1 IP2"
> >> + */
> >> +const char *arp_proxy =
> >> smap_get(>nbsp->options,"arp_proxy");
> >> +int ofs = 0;
> >> +if (arp_proxy &&
> >> +!extract_addresses(arp_proxy, >proxy_arp_addrs,
> )
> >> &&
> >> +!extract_ip_addresses(arp_proxy,
> >proxy_arp_addrs)) {
> >> +static struct vlog_rate_limit rl =
> >> VLOG_RATE_LIMIT_INIT(1, 5);
> >> +VLOG_WARN_RL(, "Invalid arp_proxy option: '%s' at
> lsp
> >> '%s'",
> >> + arp_proxy, op->nbsp->name);
> >> +}
> >> } else if (op->nbrp && op->nbrp->peer && !op->l3dgw_port) {
> >> struct ovn_port *peer = ovn_port_find(ports,
> op->nbrp->peer);
> >> if (peer) {
> >> @@ -8625,29 +8643,33 @@ build_lswitch_arp_nd_responder_known_ips(struct
> >> ovn_port *op,
> >> }
> >> }
> >> }
> >> +if (op->proxy_arp_addrs.n_ipv4_addrs ||
> >> +op->proxy_arp_addrs.n_ipv6_addrs) {
> >> +/* Select the mac address to answer the proxy ARP/NDP */
> >> +char *ea_s = NULL;
> >> +if (!eth_addr_is_zero(op->proxy_arp_addrs.ea)) {
> >> +ea_s = op->proxy_arp_addrs.ea_s;
> >> +} else if (op->peer) {
> >> +ea_s = op->peer->lrp_networks.ea_s;
> >> +} else {
> >> +return;
> >> +}
> >>
> >> -if (op->peer) {
> >> -const char *arp_proxy =
> >> smap_get(>nbsp->options,"arp_proxy");
> >> -
> >> -struct lport_addresses proxy_arp_addrs;
> >> int i = 0;
> >> -
> >> -/* Add responses for ARP proxies. */
> >> -
Re: [ovs-dev] [PATCH ovn v4] northd, lsp: Add additional arp proxy features
On 3/9/23 08:45, Ales Musil wrote:
> On Wed, Mar 8, 2023 at 3:26 PM Enrique Llorente wrote:
>
>> Configure mac address
>> The mac address returned by ARP/NDP can be configured similar to LSP
>> addresses where the mac is the first entry on the list
>>
>> IPv6
>> Support NDP IPv6 protocol
>>
>> Use CIDRs
>> Allow to specify subnets for ipv4 and ipv6, they will match whatever
>> address is received from ARP/NDP
>>
>> Signed-off-by: Enrique Llorente
>>
>
> Hi,
>
> just two nits below, but there is probably no need to spin another version.
>
>
>> ---
>> NEWS | 2 +
>> northd/northd.c | 129 +---
>> northd/ovn-northd.8.xml | 9 ++
>> ovn-nb.xml| 17 ++-
>> tests/ovn.at | 179 ++
>> tests/system-common-macros.at | 1 +
>> tests/system-ovn.at | 270 ++
>> 7 files changed, 551 insertions(+), 56 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 900825b1e..d859ae19d 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -8,6 +8,8 @@ OVN v23.03.0 - 03 Mar 2023
>>- Add ovn-ctl commands for (re)starting/stopping OVSDB relay for OVN SB
>> DB.
>>- Add new ovn-db@.service systemd unit to run ovsdb-servers in separate
>> systemd units.
>> + - Enhance LSP.options:arp_proxy to support IPv6, configurable MAC
>> +addresses and CIDRs.
>>
>
> This needs to be under "Post v23.03.0" instead, however that can be moved
> during merge.
>
>
>>
>> OVN v22.12.0 - 16 Dec 2022
>> --
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 33025bb5c..c61b538e7 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -1547,6 +1547,8 @@ struct ovn_port {
>> struct ovs_list list; /* In list of similar records. */
>>
>> struct ovs_list dp_node;
>> +
>> +struct lport_addresses proxy_arp_addrs;
>> };
>>
>> static bool
>> @@ -1628,6 +1630,7 @@ ovn_port_create(struct hmap *ports, const char *key,
>> op->sb = sb;
>> ovn_port_set_nb(op, nbsp, nbrp);
>> op->l3dgw_port = op->cr_port = NULL;
>> +op->proxy_arp_addrs.ea = eth_addr_zero;
>> hmap_insert(ports, >key_node, hash_string(op->key, 0));
>> return op;
>> }
>> @@ -1654,6 +1657,7 @@ ovn_port_destroy(struct hmap *ports, struct ovn_port
>> *port)
>> destroy_routable_addresses(>routables);
>>
>> destroy_lport_addresses(>lrp_networks);
>> +destroy_lport_addresses(>proxy_arp_addrs);
>> free(port->json_key);
>> free(port->key);
>> free(port);
>> @@ -2704,6 +2708,20 @@ join_logical_ports(struct northd_input *input_data,
>> if (peer->od && peer->od->mcast_info.rtr.relay) {
>> op->od->mcast_info.sw.flood_relay = true;
>> }
>> +
>> +/* For LSP of router type arp proxy can be activated so
>> + * it needs to be parsed
>> + * either takes "MAC IP1 IP2" or "IP1 IP2"
>> + */
>> +const char *arp_proxy =
>> smap_get(>nbsp->options,"arp_proxy");
>> +int ofs = 0;
>> +if (arp_proxy &&
>> +!extract_addresses(arp_proxy, >proxy_arp_addrs, )
>> &&
>> +!extract_ip_addresses(arp_proxy, >proxy_arp_addrs)) {
>> +static struct vlog_rate_limit rl =
>> VLOG_RATE_LIMIT_INIT(1, 5);
>> +VLOG_WARN_RL(, "Invalid arp_proxy option: '%s' at lsp
>> '%s'",
>> + arp_proxy, op->nbsp->name);
>> +}
>> } else if (op->nbrp && op->nbrp->peer && !op->l3dgw_port) {
>> struct ovn_port *peer = ovn_port_find(ports, op->nbrp->peer);
>> if (peer) {
>> @@ -8625,29 +8643,33 @@ build_lswitch_arp_nd_responder_known_ips(struct
>> ovn_port *op,
>> }
>> }
>> }
>> +if (op->proxy_arp_addrs.n_ipv4_addrs ||
>> +op->proxy_arp_addrs.n_ipv6_addrs) {
>> +/* Select the mac address to answer the proxy ARP/NDP */
>> +char *ea_s = NULL;
>> +if (!eth_addr_is_zero(op->proxy_arp_addrs.ea)) {
>> +ea_s = op->proxy_arp_addrs.ea_s;
>> +} else if (op->peer) {
>> +ea_s = op->peer->lrp_networks.ea_s;
>> +} else {
>> +return;
>> +}
>>
>> -if (op->peer) {
>> -const char *arp_proxy =
>> smap_get(>nbsp->options,"arp_proxy");
>> -
>> -struct lport_addresses proxy_arp_addrs;
>> int i = 0;
>> -
>> -/* Add responses for ARP proxies. */
>> -if (arp_proxy && extract_ip_addresses(arp_proxy,
>> - _arp_addrs) &&
>> -proxy_arp_addrs.n_ipv4_addrs) {
>> +/* Add IPv4 responses for ARP proxies. */
>> +if (op->proxy_arp_addrs.n_ipv4_addrs) {
>> /* Match rule on all proxy ARP
Re: [ovs-dev] [PATCH ovn] tests: Properly wait for chassis to added
On 3/9/23 09:50, Ales Musil wrote:
> Properly wait for chassis to be added before
> getting the id.
>
> Signed-off-by: Ales Musil
> ---
Thanks for the quick fix, Ales! Just so I don't forget, dropping this here:
Fixes: a09852bfe631 ("chassis: Remove unsupported config option")
Regards,
Dumtiru
> tests/ovn-controller.at | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 159e66461..597aed00b 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -2540,6 +2540,7 @@ ovs-vsctl add-br br-phys
> ovn_attach n1 br-phys 192.168.0.1
>
> ovn-nbctl --wait=sb sync
> +wait_row_count Chassis 1 name=hv1
>
> chassis_id=$(fetch_column Chassis _uuid name=hv1)
> check ovn-sbctl set chassis $chassis_id other_config:unsupported="value"
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] tests: Properly wait for chassis to added
Properly wait for chassis to be added before getting the id. Signed-off-by: Ales Musil --- tests/ovn-controller.at | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 159e66461..597aed00b 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -2540,6 +2540,7 @@ ovs-vsctl add-br br-phys ovn_attach n1 br-phys 192.168.0.1 ovn-nbctl --wait=sb sync +wait_row_count Chassis 1 name=hv1 chassis_id=$(fetch_column Chassis _uuid name=hv1) check ovn-sbctl set chassis $chassis_id other_config:unsupported="value" -- 2.39.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
