Re: [ovs-dev] [PATCH ovn] ovn.at: Make some of the tests more predictable.
On 11/10/20 1:47 AM, Ben Pfaff wrote:
> On Mon, Nov 09, 2020 at 02:43:43PM +0100, Dumitru Ceara wrote:
>> Fix some of the race conditions that are present in the current OVN test
>> suite.
>>
>> Signed-off-by: Dumitru Ceara
>
> Thanks!
>
> I think I see some problems (I did not test this). First, I'm surprised
> this works, since I would expect m4 to drop the [], so that they'd need
> to be doubled into [[]]:
>
> sed 's/conjunction([0-9]\+,[0-9]\+\/[0-9]\+)/conjunction()/g'
>
> Second, \+ is a GNU sed extension. Usually we avoid GNU extensions
> since sometimes people want to run OVS (and maybe OVN?) with BSD. The
> portable way to write it is \{1,\}.
>
You're right, sorry, I didn't test this properly. sed wasn't matching
anything with my change.
> Maybe it would be easier to write s/conjunction([^)]*)/conjunction()/g,
> doubling the [] if necessary.
>
This is way better indeed. I'll use it in v2.
Thanks,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [net-next] netfiler: conntrack: Add the option to set ct tcp flag - BE_LIBERAL per-ct basis.
On Tue, Nov 10, 2020 at 1:25 AM Jakub Kicinski wrote: > > On Mon, 9 Nov 2020 12:59:30 +0530 nusid...@redhat.com wrote: > > From: Numan Siddique > > > > Before calling nf_conntrack_in(), caller can set this flag in the > > connection template for a tcp packet and any errors in the > > tcp_in_window() will be ignored. > > > > A helper function - nf_ct_set_tcp_be_liberal(nf_conn) is added which > > sets this flag for both the directions of the nf_conn. > > > > openvswitch makes use of this feature so that any out of window tcp > > packets are not marked invalid. Prior to this there was no easy way > > to distinguish if conntracked packet is marked invalid because of > > tcp_in_window() check error or because it doesn't belong to an > > existing connection. > > > > An earlier attempt (see the link) tried to solve this problem for > > openvswitch in a different way. Florian Westphal instead suggested > > to be liberal in openvswitch for tcp packets. > > > > Link: > > https://patchwork.ozlabs.org/project/netdev/patch/20201006083355.121018-1-nusid...@redhat.com/ > > > > Suggested-by: Florian Westphal > > Signed-off-by: Numan Siddique > > Please repost Ccing Pablo & netfilter-devel. Thanks. I will repost. Numan > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3 4/6] tests: Eliminate most "sleep" calls.
On 11/10/20 1:26 AM, Ben Pfaff wrote: > On Mon, Nov 09, 2020 at 02:44:27PM +0100, Dumitru Ceara wrote: >> On 11/6/20 4:36 AM, Ben Pfaff wrote: >>> Many of these could be replaced by "ovn-nbctl sync". Some weren't >>> really needed at all because they were adjacent to something that itself >>> called sync or otherwise used --wait. Some were more appropriately >>> done with explicit waits for what was really needed. >>> >>> I left some "sleep"s. Some were because they were "negative" sleeps: >>> they were giving time for something to happen that should *not* happen >>> (in other words, if you wait for it to happen, you'll wait forever, >>> unless there's a bug). Some were because I didn't know how to properly >>> wait for what they were waiting for, or because I didn't understand >>> the circumstances deeply enough. >>> >>> Signed-off-by: Ben Pfaff >>> --- >>> tests/ovn-northd.at | 7 ++ >>> tests/ovn.at| 167 >>> 2 files changed, 52 insertions(+), 122 deletions(-) >>> >>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >>> index 949a8eee054e..5d670233561e 100644 >>> --- a/tests/ovn-northd.at >>> +++ b/tests/ovn-northd.at >>> @@ -1120,6 +1120,7 @@ ovn-nbctl --wait=sb -- --id=@hc create \ >>> Load_Balancer_Health_Check vip="10.0.0.10\:80" -- add Load_Balancer . \ >>> health_check @hc >>> wait_row_count Service_Monitor 2 >>> +check ovn-nbctl --wait=hv sync >> >> This should be "--wait=sb". ovn-controller is not started for tests in >> ovn-northd.at and "--wait=hv" would return immediately. This applies to >> all "ovn-nbctl --wait=hv sync" in ovn-northd.at. > > I see what you mean. > > This is surprising to me. When I defined this stuff years ago, my > intent was that --wait=hv would be stronger than --wait=sb. That is, > --wait=sb would cause ovn-nbctl to wait for the southbound database to > catch up with the northbound changes, and --wait=hb would cause it to > wait for the southbound database AND the hypervisors to catch up. > > That's even how it's documented for ovn-nbctl: > > > With --wait=sb, before ovn-nbctl exits, it > waits for ovn-northd to bring the southbound database > up-to-date with the northbound database updates. > > > > With --wait=hv, before ovn-nbctl exits, it > additionally waits for all OVN chassis (hypervisors and gateways) to > become up-to-date with the northbound database updates. (This can > become an indefinite wait if any chassis is malfunctioning.) > > > That's not what actually happens, though. As you point out, if there > are no hypervisors, then the hypervisors are always up-to-date, even if > the database is not. > > I think this is a bug in ovn-nbctl. Arguably, it's a bug in the > database definition (perhaps hv_cfg shouldn't even be filled in but left > empty if there are no chassis) but I think it is too late to fix it > there. It is, however, easy enough to fix it in ovn-nbctl. > > (And at the same time, I'll change these in ovn-northd to just say > --wait=sb. You have a point.) > > How about this as an additional patch? > > -8<--cut here-->8-- > > From 7b373c22dbda2f808f3d5f3b8ae6eb67612dbe87 Mon Sep 17 00:00:00 2001 > From: Ben Pfaff > Date: Mon, 9 Nov 2020 16:12:50 -0800 > Subject: [PATCH ovn] ovn-nbctl: Make --wait=hv wait for southbound database in > corner case. > > In the corner case where there are no chassis, --wait=hv didn't wait at > all, instead of waiting for the southbound database. > > Reported-by: Dumitru Ceara > Signed-off-by: Ben Pfaff > --- > ovn-nb.xml| 27 --- > utilities/ovn-nbctl.c | 2 +- > 2 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 51b186b84419..d3e51f3e5e87 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -84,13 +84,26 @@ > > > > -Sequence number that ovn-northd sets to the smallest > -sequence number of all the chassis in the system, as reported in the > -Chassis_Private table in the southbound database. Thus, > - equals if all chassis > are > -caught up with the northbound configuration (which may never happen, > if > -any chassis is down). This value can regress, if a chassis was > removed > -from the system and rejoins before catching up. > + > + Sequence number that ovn-northd sets to the smallest > + sequence number of all the chassis in the system, as reported in > the > + Chassis_Private table in the southbound database. > Thus, > + equals if all > chassis are > + caught up with the northbound configuration (which may never > happen, if > + any chassis is down). This value can regress, if a chassis was > removed > + from the system and rejoi
Re: [ovs-dev] [net-next] netfiler: conntrack: Add the option to set ct tcp flag - BE_LIBERAL per-ct basis.
On Tue, Nov 10, 2020 at 3:06 AM Florian Westphal wrote:
>
> nusid...@redhat.com wrote:
> > From: Numan Siddique
> >
> > Before calling nf_conntrack_in(), caller can set this flag in the
> > connection template for a tcp packet and any errors in the
> > tcp_in_window() will be ignored.
> >
> > A helper function - nf_ct_set_tcp_be_liberal(nf_conn) is added which
> > sets this flag for both the directions of the nf_conn.
> >
> > openvswitch makes use of this feature so that any out of window tcp
> > packets are not marked invalid. Prior to this there was no easy way
> > to distinguish if conntracked packet is marked invalid because of
> > tcp_in_window() check error or because it doesn't belong to an
> > existing connection.
> >
> > An earlier attempt (see the link) tried to solve this problem for
> > openvswitch in a different way. Florian Westphal instead suggested
> > to be liberal in openvswitch for tcp packets.
> >
> > Link:
> > https://patchwork.ozlabs.org/project/netdev/patch/20201006083355.121018-1-nusid...@redhat.com/
> >
> > Suggested-by: Florian Westphal
> > Signed-off-by: Numan Siddique
> > ---
> > include/net/netfilter/nf_conntrack_l4proto.h | 6 ++
> > net/netfilter/nf_conntrack_core.c| 13 +++--
> > net/openvswitch/conntrack.c | 1 +
> > 3 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/net/netfilter/nf_conntrack_l4proto.h
> > b/include/net/netfilter/nf_conntrack_l4proto.h
> > index 88186b95b3c2..572ae8d2a622 100644
> > --- a/include/net/netfilter/nf_conntrack_l4proto.h
> > +++ b/include/net/netfilter/nf_conntrack_l4proto.h
> > @@ -203,6 +203,12 @@ static inline struct nf_icmp_net
> > *nf_icmpv6_pernet(struct net *net)
> > {
> > return &net->ct.nf_ct_proto.icmpv6;
> > }
> > +
> > +static inline void nf_ct_set_tcp_be_liberal(struct nf_conn *ct)
> > +{
> > + ct->proto.tcp.seen[0].flags |= IP_CT_TCP_FLAG_BE_LIBERAL;
> > + ct->proto.tcp.seen[1].flags |= IP_CT_TCP_FLAG_BE_LIBERAL;
> > +}
> > #endif
> >
> > #ifdef CONFIG_NF_CT_PROTO_DCCP
> > diff --git a/net/netfilter/nf_conntrack_core.c
> > b/net/netfilter/nf_conntrack_core.c
> > index 234b7cab37c3..8290c5b04e88 100644
> > --- a/net/netfilter/nf_conntrack_core.c
> > +++ b/net/netfilter/nf_conntrack_core.c
> > @@ -1748,10 +1748,18 @@ static int nf_conntrack_handle_packet(struct
> > nf_conn *ct,
> > struct sk_buff *skb,
> > unsigned int dataoff,
> > enum ip_conntrack_info ctinfo,
> > - const struct nf_hook_state *state)
> > + const struct nf_hook_state *state,
> > + union nf_conntrack_proto *tmpl_proto)
>
> I would prefer if we could avoid adding yet another function argument.
>
> AFAICS its enough to call the new nf_ct_set_tcp_be_liberal() helper
> before nf_conntrack_confirm() in ovs_ct_commit(), e.g.:
Thanks for the comments. I actually tried this approach first, but it
doesn't seem to work.
I noticed that for the committed connections, the ct tcp flag -
IP_CT_TCP_FLAG_BE_LIBERAL is
not set when nf_conntrack_in() calls resolve_normal_ct().
Would you expect that the tcp ct flags should have been preserved once
the connection is committed ?
Thanks
Numan
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -1235,10 +1235,13 @@ static int ovs_ct_commit(struct net *net, struct
> sw_flow_key *key,
> if (!nf_ct_is_confirmed(ct)) {
> err = ovs_ct_init_labels(ct, key, &info->labels.value,
> &info->labels.mask);
> if (err)
> return err;
> +
> + if (nf_ct_protonum(ct) == IPPROTO_TCP)
> + nf_ct_set_tcp_be_liberal(ct);
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] AUTHORS: Update Roi Dayan
On 11/9/20 6:35 PM, Roi Dayan wrote: > Signed-off-by: Roi Dayan > --- > .mailmap| 1 + > AUTHORS.rst | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/.mailmap b/.mailmap > index 85373d11387f..ea7ccbdb4d16 100644 > --- a/.mailmap > +++ b/.mailmap > @@ -67,6 +67,7 @@ Ralf Spenneberg > Rami Rosen > Ramu Ramamurthy > Robert Åkerblom-Andersson > > +Roi Dayan Maybe you want to remap mellanox address to nvidia one in format: Name ? Format 'Name ' is to specify Name that should be used for email. For exmaple, if you have your name spelled differently in different commits. > Romain Lenglet > Romain Lenglet > Russell Bryant > diff --git a/AUTHORS.rst b/AUTHORS.rst > index 9e9d210a2024..10f0f272e3a9 100644 > --- a/AUTHORS.rst > +++ b/AUTHORS.rst > @@ -331,7 +331,7 @@ Robert Åkerblom-Andersson robert@gmail.com > Robert Wojciechowicz robertx.wojciechow...@intel.com > Rob Hoes rob.h...@citrix.com > Rohith Basavaraja rohith.basavar...@gmail.com > -Roi Dayan r...@mellanox.com > +Roi Dayan r...@nvidia.com > Róbert Mulik robert.mu...@ericsson.com > Romain Lenglet romain.leng...@berabera.info > Roni Bar Yanai ron...@mellanox.com > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idl.at: Return stream open_block python tests.
On 11/9/20 2:45 AM, Gaëtan Rivet wrote:
> Hi Ilya,
>
> One nit below,
>
> On 04/09/20 13:51 +0200, Ilya Maximets wrote:
>> Invocations of CHECK_STREAM_OPEN_BLOCK_PY was accidentally removed
>> during python2 to python3 conversion. So, these tests was not
>> checked since that time.
>>
>> This change returns tests back. CHECK_STREAM_OPEN_BLOCK_PY needed
>> updates, so instead I refactored function for C tests to be able to
>> perform python tests too.
>>
>> Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
>> Signed-off-by: Ilya Maximets
>> ---
>> tests/ovsdb-idl.at | 36 ++--
>> 1 file changed, 14 insertions(+), 22 deletions(-)
>>
>> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
>> index 789ae23a9..96be361fc 100644
>> --- a/tests/ovsdb-idl.at
>> +++ b/tests/ovsdb-idl.at
>> @@ -1778,33 +1778,25 @@ OVSDB_CHECK_IDL_COMPOUND_INDEX_WITH_REF([set,
>> simple3 idl-compound-index-with-re
>> ]])
>>
>> m4_define([CHECK_STREAM_OPEN_BLOCK],
>> - [AT_SETUP([Check Stream open block - C - $1])
>> - AT_SKIP_IF([test "$1" = "tcp6" && test "$IS_WIN32" = "yes"])
>> - AT_SKIP_IF([test "$1" = "tcp6" && test "$HAVE_IPV6" = "no"])
>> - AT_KEYWORDS([Check Stream open block $1])
>> - AT_CHECK([ovsdb_start_idltest "ptcp:0:$2"])
>> + [AT_SETUP([Check stream open block - $1 - $3])
>> + AT_SKIP_IF([test "$3" = "tcp6" && test "$IS_WIN32" = "yes"])
>> + AT_SKIP_IF([test "$3" = "tcp6" && test "$HAVE_IPV6" = "no"])
>> + AT_KEYWORDS([Check stream open block open_block $3])
>
> The keywords seems to copy the title. Is 'Check' a relevant keyword for
> this test? I only see it used there. Using TESTSUITEFLAGS='-k Check'
> seems less intuitive than '-k open_block' for example.
>
> Also, reading the keywords in ovsdb-idl.at, 'ovsdb server' is
> always used except for those two tests. Would it be relevant there as well?
I don't like these keywords too. I kept them because they were in the
original test. But yes, we could make them better.
Something like:
AT_KEYWORDS([ovsdb server stream open_block $1 $3])
What do you think?
>
>> + AT_CHECK([ovsdb_start_idltest "ptcp:0:$4"])
>> PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>> WRONG_PORT=$(($TCP_PORT + 101))
>> - AT_CHECK([test-stream tcp:$2:$TCP_PORT], [0], [ignore])
>> - AT_CHECK([test-stream tcp:$2:$WRONG_PORT], [1], [ignore], [ignore])
>> + AT_CHECK([$2 tcp:$4:$TCP_PORT], [0], [ignore])
>> + AT_CHECK([$2 tcp:$4:$WRONG_PORT], [1], [ignore], [ignore])
>> OVSDB_SERVER_SHUTDOWN
>> - AT_CHECK([test-stream tcp:$2:$TCP_PORT], [1], [ignore], [ignore])
>> + AT_CHECK([$2 tcp:$4:$TCP_PORT], [1], [ignore], [ignore])
>> AT_CLEANUP])
>>
>> -CHECK_STREAM_OPEN_BLOCK([tcp], [127.0.0.1])
>> -CHECK_STREAM_OPEN_BLOCK([tcp6], [[[::1]]])
>> -
>> -m4_define([CHECK_STREAM_OPEN_BLOCK_PY],
>> - [AT_SETUP([$1 - Python3])
>> - AT_KEYWORDS([Check PY Stream open block - $3])
>> - AT_CHECK([ovsdb_start_idltest "ptcp:0:127.0.0.1"])
>> - PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>> - WRONG_PORT=$(($TCP_PORT + 101))
>> - AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$TCP_PORT], [0],
>> [ignore])
>> - AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$WRONG_PORT], [1],
>> [ignore])
>> - OVSDB_SERVER_SHUTDOWN
>> - AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$TCP_PORT], [1],
>> [ignore])
>> - AT_CLEANUP])
>> +CHECK_STREAM_OPEN_BLOCK([C], [test-stream], [tcp], [127.0.0.1])
>> +CHECK_STREAM_OPEN_BLOCK([C], [test-stream], [tcp6], [[[::1]]])
>> +CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py],
>> +[tcp], [127.0.0.1])
>> +CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py],
>> +[tcp6], [[[::1]]])
>
> This fix expands the coverage for python test to IPv6 (beyond
> re-enabling the test). It seems fine, should it be said explicitly in
> the commit log?
It's not very important, but sure I could add this info to the commit message.
Thanks!
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 0/2] Mitigate RAFT memory consumption issues.
On 11/3/20 4:53 PM, Ilya Maximets wrote: > Version 2: > - Skipping 3 patches that already applied. > - Fixed use of uninitialized backlog limits in jsonrpc_session, >i.e. added initialization on each jsorpc_session_open*() > - Added Acked-by from Dumitru to the last patch. > > Original cover letter: > > Under a heavy load or in a long run memory consumption if ovsdb-server > process that is part of a RAFT cluster could reach very high values. > From my experience it could be up to 60-100 GB. In these conditions > it's likely that ovsdb-server will be killed by OOM-killer or just > will not be able to work properly wasting time on processing outdated > or unneeded data. There are 3 main parts that consumes most of the > memory: > > 1. Backlog on RAFT connections between servers. > 2. Local RAFT log. > 3. Libc doesn't return memory back to system. > > Backlog could start growing if one of remote servers doesn't doing > well and is not able to process requests in time. This sending > backlog could contain snapshots or even just big number of big > append requests. It could grow to tens of GBs really fast and > most of this data might be even unnecessary if it becomes obsolete > by one of the previous requests or if current 'term' changes and > all the old messages should be dropped. Solution for this is > to monitor the size of the current backlog and disconnect if it grows > too big since it will be easier to just reconnect and send one new > snapshot. > > Local RAFT log contains all the DB changes that are not part of a > snapshot yet. Since snapshots are taken at most once in 10 minutes, > log could grow pretty big. Up to tens of thousands of entries and > each of these entries could be fairly big by themselves. That being > said RAFT log could grow up to tens of GBs too. > > One extra point for memory consumption is that memory likely doesn't > go away even after calling free() due to implementation of a C memory > allocators. And this happens a lot. ovsdb-server process usually > holds a lot of system memory even if the database is almost empty. > This heap memory might be returned back to OS by using malloc_trim(). > > -- > All of these issues was found on branch-2.13, but it always hard to > distinguish new features from the bug fix when we're talking about > scaling issues. Anyway, I think, it'll be good to have these > patches (if they are any good) backorted to 2.13, especially because > it's going to be our next LTS. Thoughts? > > > Ilya Maximets (2): > raft: Set threshold on backlog for raft connections. > raft: Make backlog thresholds configurable. > > NEWS| 3 +++ > lib/jsonrpc.c | 60 - > lib/jsonrpc.h | 6 + > ovsdb/ovsdb-server.1.in | 5 > ovsdb/raft.c| 50 ++ > 5 files changed, 123 insertions(+), 1 deletion(-) > Dumitru, thanks for review! Applied to master and backported down to 2.13. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 1/1] netdev-offload-dpdk: Preserve HW statistics for modified flows
On 10/15/20 1:46 PM, Finn, Emma wrote:
>> -Original Message-
>> From: dev On Behalf Of Sriharsha
>> Basavapatna via dev
>> Sent: Wednesday 14 October 2020 12:17
>> To: Eli Britstein
>> Cc: ovs dev ; Gaetan Rivet ; Ilya
>> Maximets
>> Subject: Re: [ovs-dev] [PATCH V2 1/1] netdev-offload-dpdk: Preserve HW
>> statistics for modified flows
>>
>> On Mon, Oct 12, 2020 at 7:57 PM Eli Britstein wrote:
>>>
>>> In case of a flow modification, preserve the HW statistics of the old
>>> HW flow to the new one.
>>>
>>> Fixes: 3c7330ebf036 ("netdev-offload-dpdk: Support offload of output
>>> action.")
>>> Signed-off-by: Eli Britstein
>>> Reviewed-by: Gaetan Rivet
>>
>> Acked-by: Sriharsha Basavapatna
>>
>
> Tested with X710 devices.
> Will do further testing with E810 devices when the patches become available
> in DPDK.
> Tested-by: Emma Finn
Thanks!
Applied to master and backported down to 2.13.
While looking at the patch I noticed that we always return zero stats while
removing the flow. That doesn't look right. We should, probably, fix that too.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovs v4] dpctl-netdev: Add the option 'pmd' for dump-flows
On 10/15/20 12:53 PM, Gaëtan Rivet wrote: > On 15/10/20 11:33 +0800, xiangxia.m@gmail.com wrote: >> From: Tonghao Zhang >> >> "ovs-appctl dpctl/dump-flows" added the option >> "pmd" which allow user to dump pmd specified. >> >> That option is useful to dump rules of pmd >> when we have a large number of rules in dp. >> >> Signed-off-by: Tonghao Zhang > > Thanks for the changes! > > Acked-by: Gaetan Rivet Thanks! Applied to master. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb: Remove read permission of *.db from others
On 10/14/20 2:44 PM, Mark Gray wrote:
> On 23/09/2020 21:48, Yi-Hung Wei wrote:
>> Currently, when ovsdb *.db is created by ovsdb-tool it grants read
>> permission to others. This may incur security concerns, for example,
>> IPsec Pre-shared keys are stored in ovs-vsitchd.conf.db.
>> This patch addresses the concerns by removing permission for others.
>>
>> Reported-by: Antonin Bas
>> Signed-off-by: Yi-Hung Wei
>> ---
>> ovsdb/log.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ovsdb/log.c b/ovsdb/log.c
>> index 41af77679178..4a28fa3db6da 100644
>> --- a/ovsdb/log.c
>> +++ b/ovsdb/log.c
>> @@ -212,7 +212,7 @@ ovsdb_log_open(const char *name, const char *magic,
>> if (!strcmp(name, "/dev/stdin") && open_mode == OVSDB_LOG_READ_ONLY) {
>> fd = dup(STDIN_FILENO);
>> } else {
>> -fd = open(name, flags, 0666);
>> +fd = open(name, flags, 0660);
>> }
>> if (fd < 0) {
>> const char *op = (open_mode == OVSDB_LOG_CREATE_EXCL ? "create"
>>
>
> I tested this and it works as expected. It also passes all the OVS
> tests. It's definitely an issue as PSKs could indeed be in the the DB.
> There may be unintended side-effects of this but I can't think of any
> that would be legitimately need user RW permissions.
>
> Acked-by: Mark Gray
Thanks!
Applied to master and backported down to 2.13 as it's our new LTS.
If you think that this should be backported further, please, let me know.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2] ovn.at: Make some of the tests more predictable.
Fix some of the race conditions that are present in the current OVN test
suite.
Signed-off-by: Dumitru Ceara
---
v2:
- Fix the sed expression as suggested by Ben.
---
tests/ovn.at | 48 +++-
1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/tests/ovn.at b/tests/ovn.at
index f154e3d..9c23ae7 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -4114,6 +4114,7 @@ for i in 1 2 3; do
ovn-nbctl lsp-set-addresses lp$i$j "f0:00:00:00:00:$i$j
192.168.0.$i$j" "$extra_addr"
ovn-nbctl lsp-set-port-security lp$i$j "f0:00:00:00:00:$i$j
192.168.0.$i$j" "$extra_addr"
fi
+OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp$i$j` = xup])
done
done
@@ -13579,14 +13580,15 @@ ovn-nbctl --wait=hv sync
# Check OVS flows, the less restrictive flows should have been installed.
AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
-grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
+grep "priority=1003" | awk '{print $7 " " $8}' | \
+sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl
priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
actions=conjunction(2,1/2),conjunction(3,1/2)
-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
actions=conjunction(2,1/2),conjunction(3,1/2)
+priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
actions=conjunction(),conjunction()
+priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
actions=conjunction(),conjunction()
priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46)
-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2)
-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2)
+priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction()
+priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction()
])
# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed.
@@ -13624,14 +13626,15 @@ ovn-nbctl --wait=hv sync
# Check OVS flows, the second less restrictive allow ACL should have been
installed.
AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
-grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
+grep "priority=1003" | awk '{print $7 " " $8}' | \
+sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl
priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
actions=conjunction(2,1/2),conjunction(3,1/2)
-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
actions=conjunction(2,1/2),conjunction(3,1/2)
+priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
actions=conjunction(),conjunction()
+priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
actions=conjunction(),conjunction()
priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46)
-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2)
-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2)
+priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction()
+priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction()
])
# Remove the less restrictive allow ACL.
@@ -13640,14 +13643,15 @@ ovn-nbctl --wait=hv sync
# Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled.
AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
-grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
+grep "priority=1003" | awk '{print $7 " " $8}' | \
+sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl
priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
actions=conjunction(2,1/2),conjunction(3,1/2)
-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
actions=conjunction(2,1/2),conjunction(3,1/2)
-priority=1003,ip,metadata=0x1,nw_src=10.0.0.1
actions=conjunction(2,2/2),conjunction(3,2/2)
-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2)
-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2)
+priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
actions=conjunction(),conjunction()
+priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
actions=conjunction(),conjunction()
+priority=1003,ip,metadata=0x1,nw_src=10.0.0.1
actions=conjunction(),conjunction()
+priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction()
+priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction()
])
# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed.
@@ -13678,14 +13682,15 @@ ovn-nbctl --wait=hv sync
# Check OVS flows, the less restrictive flows should have been installed.
AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
- grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
+ grep "p
Re: [ovs-dev] [PATCH v2 0/4] New LTS and updted release policy.
On 9/28/20 4:34 AM, Ilya Maximets wrote: > Version 2: > * Added 'Ack's to patch #1. > * Second patch split in two to simplify review and acceptance, text > partially re-worded to avoid unclear ambiguous sentences. > * Added new patch to standardize LTS process. > > Ilya Maximets (4): > releases: Mark 2.13 as a new LTS release. > release-process: Add transition period for LTS releases. > release-process: Standardize designation of new LTS releases. > release-process: Policy for unmaintained branches. > > Documentation/faq/releases.rst| 2 +- > .../contributing/backporting-patches.rst | 3 +- > Documentation/internals/release-process.rst | 52 +++ > 3 files changed, 45 insertions(+), 12 deletions(-) > This change was mentioned many times in different meetings and conversations, so, I think, everyone who had an opinion already posted it here. Thanks to everyone for reviews and discussions! Applied to master. I will update our website to reflect that 2.13 is our new LTS. 2.5 is in a transitional state now. It will be supported until 2.15 released. Maybe a few months longer since we moved LTS a few months after release of 2.14. We will also need to release new stable versions including stable release of a new LTS branch soon. Stay tuned! Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 2/2] netdev-dpdk: Add option to configure VF MAC address.
On 09/11/2020 16:49, Kevin Traynor wrote: > On 08/11/2020 20:09, Gaetan Rivet wrote: >> In some cloud topologies, using DPDK VF representors in guest requires >> configuring a VF before it is assigned to the guest. >> >> A first basic option for such configuration is setting the VF MAC >> address. Add a key 'dpdk-vf-mac' to the 'options' column of the Interface >> table. >> >> This option can be used as such: >> >>$ ovs-vsctl add-port br0 dpdk-rep0 -- set Interface dpdk-rep0 type=dpdk \ >> options:dpdk-vf-mac=00:11:22:33:44:55 >> >> Issue: 1981388 >> Change-Id: Iaec052592fe0a66a4a2d8ed34ccafe105423d16a >> Signed-off-by: Gaetan Rivet >> Suggested-by: Ilya Maximets >> Acked-by: Eli Britstein >> --- > > Thanks for adding to the documentation Gaetan. > > Acked-by: Kevin Traynor > Wait. Travis build is failing with clang - I presume the ovs robot will report this also. https://travis-ci.org/github/kevuzaj/ovs/builds/742494330 > ___ > 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] ovsdb-idl: Add ovsdb_idl_monitor_condition_pending().
On 11/9/20 12:09 PM, Dumitru Ceara wrote:
> IDL clients had no way of checking whether monitor_cond_change requests
> were pending (either local or in flight). This commit introduces a new
> API to check for the state of the conditional monitoring clauses.
>
> Signed-off-by: Dumitru Ceara
> ---
> lib/ovsdb-idl.c | 40
> lib/ovsdb-idl.h | 1 +
> 2 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> index a1a5776..3f19d40 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -421,6 +421,7 @@ unsigned int ovsdb_idl_set_condition(struct ovsdb_idl *,
> const struct ovsdb_idl_condition *);
>
> unsigned int ovsdb_idl_get_condition_seqno(const struct ovsdb_idl *);
> +bool ovsdb_idl_monitor_condition_pending(struct ovsdb_idl *);
I don't think that we need a new api for this. Condition sequence number,
I believe, exists for exactly this case. There is an issue, however, that
db bindings doesn't return result of ovsdb_idl_set_condition() throwing it
away without giving a chance for an idl user to actually use it. That could
be fixed by something like this:
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 698fe25f3..d319adb03 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -1416,10 +1416,10 @@ struct %(s)s *
print("\nstruct ovsdb_idl_column %s_columns[%s_N_COLUMNS];" % (
structName, structName.upper()))
print("""
-void
+unsigned int
%(s)s_set_condition(struct ovsdb_idl *idl, struct ovsdb_idl_condition
*condition)
{
-ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
+return ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
}""" % {'p': prefix,
's': structName,
'tl': tableName.lower()})
---
I think, I had this patch somewhere already, but it seems that I never
actually send it.
With this change ovn-controller will need to store the returned value
and wait until current condition seqno != expected to be sure that
condition was successfully set.
What do you think?
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 2/2] netdev-dpdk: Add option to configure VF MAC address.
On 10/11/20 09:42 +, Kevin Traynor wrote: > On 09/11/2020 16:49, Kevin Traynor wrote: > > On 08/11/2020 20:09, Gaetan Rivet wrote: > >> In some cloud topologies, using DPDK VF representors in guest requires > >> configuring a VF before it is assigned to the guest. > >> > >> A first basic option for such configuration is setting the VF MAC > >> address. Add a key 'dpdk-vf-mac' to the 'options' column of the Interface > >> table. > >> > >> This option can be used as such: > >> > >>$ ovs-vsctl add-port br0 dpdk-rep0 -- set Interface dpdk-rep0 type=dpdk > >> \ > >> options:dpdk-vf-mac=00:11:22:33:44:55 > >> > >> Issue: 1981388 > >> Change-Id: Iaec052592fe0a66a4a2d8ed34ccafe105423d16a > >> Signed-off-by: Gaetan Rivet > >> Suggested-by: Ilya Maximets > >> Acked-by: Eli Britstein > >> --- > > > > Thanks for adding to the documentation Gaetan. > > > > Acked-by: Kevin Traynor > > > > Wait. Travis build is failing with clang - I presume the ovs robot will > report this also. > https://travis-ci.org/github/kevuzaj/ovs/builds/742494330 > Ah yes sorry, good catch. I did not retry with clang in the latest versions. Will fix and resubmit. -- Gaëtan ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idl: Add ovsdb_idl_monitor_condition_pending().
On 11/10/20 11:07 AM, Ilya Maximets wrote:
> On 11/9/20 12:09 PM, Dumitru Ceara wrote:
>> IDL clients had no way of checking whether monitor_cond_change requests
>> were pending (either local or in flight). This commit introduces a new
>> API to check for the state of the conditional monitoring clauses.
>>
>> Signed-off-by: Dumitru Ceara
>> ---
>> lib/ovsdb-idl.c | 40
>> lib/ovsdb-idl.h | 1 +
>> 2 files changed, 33 insertions(+), 8 deletions(-)
>>
>
>
>
>> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
>> index a1a5776..3f19d40 100644
>> --- a/lib/ovsdb-idl.h
>> +++ b/lib/ovsdb-idl.h
>> @@ -421,6 +421,7 @@ unsigned int ovsdb_idl_set_condition(struct ovsdb_idl *,
>> const struct ovsdb_idl_condition *);
>>
>> unsigned int ovsdb_idl_get_condition_seqno(const struct ovsdb_idl *);
>> +bool ovsdb_idl_monitor_condition_pending(struct ovsdb_idl *);
>
> I don't think that we need a new api for this. Condition sequence number,
> I believe, exists for exactly this case. There is an issue, however, that
> db bindings doesn't return result of ovsdb_idl_set_condition() throwing it
> away without giving a chance for an idl user to actually use it. That could
> be fixed by something like this:
>
> diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
> index 698fe25f3..d319adb03 100755
> --- a/ovsdb/ovsdb-idlc.in
> +++ b/ovsdb/ovsdb-idlc.in
> @@ -1416,10 +1416,10 @@ struct %(s)s *
> print("\nstruct ovsdb_idl_column %s_columns[%s_N_COLUMNS];" % (
> structName, structName.upper()))
> print("""
> -void
> +unsigned int
> %(s)s_set_condition(struct ovsdb_idl *idl, struct ovsdb_idl_condition
> *condition)
> {
> -ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
> +return ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
> }""" % {'p': prefix,
> 's': structName,
> 'tl': tableName.lower()})
> ---
>
> I think, I had this patch somewhere already, but it seems that I never
> actually send it.
Thanks for looking into this, Ilya! Yes, returning the seqno would
probably be nice to have in *_set_condition(..).
>
> With this change ovn-controller will need to store the returned value
> and wait until current condition seqno != expected to be sure that
> condition was successfully set.
>
> What do you think?
>
I thought about this too before proposing the new API but it seemed to
me that in that case the code that sets the conditions in ovn-controller
might get unnecessarily complicated:
https://github.com/ovn-org/ovn/blob/c108f23e1c10910031f9409b79001d001aae0c8f/controller/ovn-controller.c#L240
I guess it can be rewritten as something like this:
expected_cond_seqno = 0;
expected_cond_seqno =
MAX(sbrec_port_binding_set_condition(ovnsb_idl, &pb),
expected_cond_seqno);
[...]
expected_cond_seqno =
MAX(sbrec_chassis_private_set_condition(ovnsb_idl, &chprv),
expected_cond_seqno);
[...]
return expected_cond_seqno;
I know it's not really OVS's problem but the set_condition() API doesn't
seem to make it easy to write simple code to use it properly.
However, if you think a new API would be redundant, we'll have to just
find a way to properly explain (comments I guess) why we need to do
things like this in ovn-controller.
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idl.at: Return stream open_block python tests.
On 10/11/20 10:15 +0100, Ilya Maximets wrote:
> On 11/9/20 2:45 AM, Gaëtan Rivet wrote:
> > Hi Ilya,
> >
> > One nit below,
> >
> > On 04/09/20 13:51 +0200, Ilya Maximets wrote:
> >> Invocations of CHECK_STREAM_OPEN_BLOCK_PY was accidentally removed
> >> during python2 to python3 conversion. So, these tests was not
> >> checked since that time.
> >>
> >> This change returns tests back. CHECK_STREAM_OPEN_BLOCK_PY needed
> >> updates, so instead I refactored function for C tests to be able to
> >> perform python tests too.
> >>
> >> Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
> >> Signed-off-by: Ilya Maximets
> >> ---
> >> tests/ovsdb-idl.at | 36 ++--
> >> 1 file changed, 14 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> >> index 789ae23a9..96be361fc 100644
> >> --- a/tests/ovsdb-idl.at
> >> +++ b/tests/ovsdb-idl.at
> >> @@ -1778,33 +1778,25 @@ OVSDB_CHECK_IDL_COMPOUND_INDEX_WITH_REF([set,
> >> simple3 idl-compound-index-with-re
> >> ]])
> >>
> >> m4_define([CHECK_STREAM_OPEN_BLOCK],
> >> - [AT_SETUP([Check Stream open block - C - $1])
> >> - AT_SKIP_IF([test "$1" = "tcp6" && test "$IS_WIN32" = "yes"])
> >> - AT_SKIP_IF([test "$1" = "tcp6" && test "$HAVE_IPV6" = "no"])
> >> - AT_KEYWORDS([Check Stream open block $1])
> >> - AT_CHECK([ovsdb_start_idltest "ptcp:0:$2"])
> >> + [AT_SETUP([Check stream open block - $1 - $3])
> >> + AT_SKIP_IF([test "$3" = "tcp6" && test "$IS_WIN32" = "yes"])
> >> + AT_SKIP_IF([test "$3" = "tcp6" && test "$HAVE_IPV6" = "no"])
> >> + AT_KEYWORDS([Check stream open block open_block $3])
> >
> > The keywords seems to copy the title. Is 'Check' a relevant keyword for
> > this test? I only see it used there. Using TESTSUITEFLAGS='-k Check'
> > seems less intuitive than '-k open_block' for example.
> >
> > Also, reading the keywords in ovsdb-idl.at, 'ovsdb server' is
> > always used except for those two tests. Would it be relevant there as well?
>
>
> I don't like these keywords too. I kept them because they were in the
> original test. But yes, we could make them better.
>
> Something like:
> AT_KEYWORDS([ovsdb server stream open_block $1 $3])
>
> What do you think?
>
I think those keywords are better, one small nit for $1 -- in this case
either 'C' or 'Python3'. Other tests with python are using 'Python'
instead.
Invoking the macro with 'Python' instead would align with others, and it
seems fine as python2 support was dropped.
Best regards,
--
Gaëtan
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idl.at: Return stream open_block python tests.
On 11/10/20 11:39 AM, Gaëtan Rivet wrote:
> On 10/11/20 10:15 +0100, Ilya Maximets wrote:
>> On 11/9/20 2:45 AM, Gaëtan Rivet wrote:
>>> Hi Ilya,
>>>
>>> One nit below,
>>>
>>> On 04/09/20 13:51 +0200, Ilya Maximets wrote:
Invocations of CHECK_STREAM_OPEN_BLOCK_PY was accidentally removed
during python2 to python3 conversion. So, these tests was not
checked since that time.
This change returns tests back. CHECK_STREAM_OPEN_BLOCK_PY needed
updates, so instead I refactored function for C tests to be able to
perform python tests too.
Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
Signed-off-by: Ilya Maximets
---
tests/ovsdb-idl.at | 36 ++--
1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 789ae23a9..96be361fc 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -1778,33 +1778,25 @@ OVSDB_CHECK_IDL_COMPOUND_INDEX_WITH_REF([set,
simple3 idl-compound-index-with-re
]])
m4_define([CHECK_STREAM_OPEN_BLOCK],
- [AT_SETUP([Check Stream open block - C - $1])
- AT_SKIP_IF([test "$1" = "tcp6" && test "$IS_WIN32" = "yes"])
- AT_SKIP_IF([test "$1" = "tcp6" && test "$HAVE_IPV6" = "no"])
- AT_KEYWORDS([Check Stream open block $1])
- AT_CHECK([ovsdb_start_idltest "ptcp:0:$2"])
+ [AT_SETUP([Check stream open block - $1 - $3])
+ AT_SKIP_IF([test "$3" = "tcp6" && test "$IS_WIN32" = "yes"])
+ AT_SKIP_IF([test "$3" = "tcp6" && test "$HAVE_IPV6" = "no"])
+ AT_KEYWORDS([Check stream open block open_block $3])
>>>
>>> The keywords seems to copy the title. Is 'Check' a relevant keyword for
>>> this test? I only see it used there. Using TESTSUITEFLAGS='-k Check'
>>> seems less intuitive than '-k open_block' for example.
>>>
>>> Also, reading the keywords in ovsdb-idl.at, 'ovsdb server' is
>>> always used except for those two tests. Would it be relevant there as well?
>>
>>
>> I don't like these keywords too. I kept them because they were in the
>> original test. But yes, we could make them better.
>>
>> Something like:
>> AT_KEYWORDS([ovsdb server stream open_block $1 $3])
>>
>> What do you think?
>>
>
> I think those keywords are better, one small nit for $1 -- in this case
> either 'C' or 'Python3'. Other tests with python are using 'Python'
> instead.
>
> Invoking the macro with 'Python' instead would align with others, and it
> seems fine as python2 support was dropped.
All other tests has Python3 in the test name. We could actually just make
it conditional, if it's important:
AT_KEYWORDS([ovsdb server stream open_block
m4_if([$1], [Python3], [Python], [$1]) $3])
Bes regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idl: Add ovsdb_idl_monitor_condition_pending().
On 11/10/20 11:31 AM, Dumitru Ceara wrote:
> On 11/10/20 11:07 AM, Ilya Maximets wrote:
>> On 11/9/20 12:09 PM, Dumitru Ceara wrote:
>>> IDL clients had no way of checking whether monitor_cond_change requests
>>> were pending (either local or in flight). This commit introduces a new
>>> API to check for the state of the conditional monitoring clauses.
>>>
>>> Signed-off-by: Dumitru Ceara
>>> ---
>>> lib/ovsdb-idl.c | 40
>>> lib/ovsdb-idl.h | 1 +
>>> 2 files changed, 33 insertions(+), 8 deletions(-)
>>>
>>
>>
>>
>>> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
>>> index a1a5776..3f19d40 100644
>>> --- a/lib/ovsdb-idl.h
>>> +++ b/lib/ovsdb-idl.h
>>> @@ -421,6 +421,7 @@ unsigned int ovsdb_idl_set_condition(struct ovsdb_idl *,
>>> const struct ovsdb_idl_condition *);
>>>
>>> unsigned int ovsdb_idl_get_condition_seqno(const struct ovsdb_idl *);
>>> +bool ovsdb_idl_monitor_condition_pending(struct ovsdb_idl *);
>>
>> I don't think that we need a new api for this. Condition sequence number,
>> I believe, exists for exactly this case. There is an issue, however, that
>> db bindings doesn't return result of ovsdb_idl_set_condition() throwing it
>> away without giving a chance for an idl user to actually use it. That could
>> be fixed by something like this:
>>
>> diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
>> index 698fe25f3..d319adb03 100755
>> --- a/ovsdb/ovsdb-idlc.in
>> +++ b/ovsdb/ovsdb-idlc.in
>> @@ -1416,10 +1416,10 @@ struct %(s)s *
>> print("\nstruct ovsdb_idl_column %s_columns[%s_N_COLUMNS];" % (
>> structName, structName.upper()))
>> print("""
>> -void
>> +unsigned int
>> %(s)s_set_condition(struct ovsdb_idl *idl, struct ovsdb_idl_condition
>> *condition)
>> {
>> -ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
>> +return ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
>> }""" % {'p': prefix,
>> 's': structName,
>> 'tl': tableName.lower()})
>> ---
>>
>> I think, I had this patch somewhere already, but it seems that I never
>> actually send it.
>
> Thanks for looking into this, Ilya! Yes, returning the seqno would
> probably be nice to have in *_set_condition(..).
>
>>
>> With this change ovn-controller will need to store the returned value
>> and wait until current condition seqno != expected to be sure that
>> condition was successfully set.
>>
>> What do you think?
>>
>
> I thought about this too before proposing the new API but it seemed to
> me that in that case the code that sets the conditions in ovn-controller
> might get unnecessarily complicated:
>
> https://github.com/ovn-org/ovn/blob/c108f23e1c10910031f9409b79001d001aae0c8f/controller/ovn-controller.c#L240
>
> I guess it can be rewritten as something like this:
>
> expected_cond_seqno = 0;
> expected_cond_seqno =
> MAX(sbrec_port_binding_set_condition(ovnsb_idl, &pb),
> expected_cond_seqno);
> [...]
> expected_cond_seqno =
> MAX(sbrec_chassis_private_set_condition(ovnsb_idl, &chprv),
> expected_cond_seqno);
> [...]
> return expected_cond_seqno;
>
> I know it's not really OVS's problem but the set_condition() API doesn't
> seem to make it easy to write simple code to use it properly.
>
> However, if you think a new API would be redundant, we'll have to just
> find a way to properly explain (comments I guess) why we need to do
> things like this in ovn-controller.
Maybe a little pinch of magic will help? :)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index a06cae3cc..8bc331e95 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -236,16 +236,24 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
}
}
-out:
-sbrec_port_binding_set_condition(ovnsb_idl, &pb);
-sbrec_logical_flow_set_condition(ovnsb_idl, &lf);
-sbrec_mac_binding_set_condition(ovnsb_idl, &mb);
-sbrec_multicast_group_set_condition(ovnsb_idl, &mg);
-sbrec_dns_set_condition(ovnsb_idl, &dns);
-sbrec_controller_event_set_condition(ovnsb_idl, &ce);
-sbrec_ip_multicast_set_condition(ovnsb_idl, &ip_mcast);
-sbrec_igmp_group_set_condition(ovnsb_idl, &igmp);
-sbrec_chassis_private_set_condition(ovnsb_idl, &chprv);
+out:;
+unsigned int cond_seqnos[] = {
+sbrec_port_binding_set_condition(ovnsb_idl, &pb),
+sbrec_logical_flow_set_condition(ovnsb_idl, &lf),
+sbrec_mac_binding_set_condition(ovnsb_idl, &mb),
+sbrec_multicast_group_set_condition(ovnsb_idl, &mg),
+sbrec_dns_set_condition(ovnsb_idl, &dns),
+sbrec_controller_event_set_condition(ovnsb_idl, &ce),
+sbrec_ip_multicast_set_condition(ovnsb_idl, &ip_mcast),
+sbrec_igmp_group_set_condition(ovnsb_idl, &igmp),
+};
+/* We'll need to wait for a maximum expected sequence number to be sure
+ * that all conditions accepted by the server. */
+unsigned
[ovs-dev] [PATCH v6 0/2] netdev-dpdk: support changing VF MAC
v6: Fix dev->mutex lock missing on get_config(). Rebased on master (trivial conflict in NEWS). Reduce when possible line length in doc. Otherwise I migrated to travis-ci.com and tried to trigger a build with clang + DPDK, but to no avail, job is forever in queue. Local build with clang was without issue. v5: Improve doc following Kevin's comments. v4: fix Kevin's and Ilya's comments. v3: fix Ilya's comments. v2: fix 0-day bot issues in 2/2. Hello Ilya, Following your suggestion, here is a small patch adding the ability to configure the MAC address of DPDK VF representors. As said off-ML, I've used the options column instead of other_configs, as it allows avoid having the DPDK representor concept bleed out of netdev-dpdk. There is only a small compilation fix on your first patch (rte_eth_addr type instead of eth_addr), otherwise it is the same. Thank you for your help and reading, Gaetan Rivet (1): netdev-dpdk: Add option to configure VF MAC address. Ilya Maximets (1): netdev-dpdk: Add ability to set MAC address. Documentation/topics/dpdk/phy.rst | 51 +++ NEWS | 2 + lib/netdev-dpdk.c | 101 +- vswitchd/vswitch.xml | 18 ++ 4 files changed, 169 insertions(+), 3 deletions(-) -- 2.29.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 00/12] Add offload support for sFlow
Hi Ilya, Any comments for this patch set? Thanks, Chris On Thu, 2020-10-29 at 19:23 +0800, Chris Mi wrote: > This patch set adds offload support for sFlow. > > Psample is a genetlink channel for packet sampling. TC action > act_sample > uses psample to send sampled packets to userspace. > > When offloading sample action to TC, userspace creates a unique ID to > map sFlow action and tunnel info and passes this ID to kernel instead > of the sFlow info. psample will send this ID and sampled packet to > userspace. Using the ID, userspace can recover the sFlow info and > send > sampled packet to the right sFlow monitoring host. > > Travis: > v5: > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftravis-ci.com%2Fgithub%2Fmishuang2017%2Fovs%2Fbuilds%2F195422006&data=02%7C01%7Ccmi%40nvidia.com%7C8048d7a70bcf43b69c5108d87bfd1df6%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637395674367205649&sdata=Uz6So8Kw1VBw2MYBVIUAWMhGJkv1uverdI5fFLLPAtk%3D&reserved=0 > > v2-v1: > - Fix robot errors. > v3-v2: > - Remove Gerrit Change-Id. > - Add patch #9 to fix older kernels build issue. > - Add travis test result. > v4-v3: > - Fix offload issue when sampling rate is 1. > v5-v4: > - Move polling thread from ofproto to netdev-offload-tc. > > Chris Mi (12): > compat: Add psample and tc sample action defines for older kernels > ovs-kmod-ctl: Load kernel module psample > dpif: Introduce register sFlow upcall callback API > ofproto: Add upcall callback to process sFlow packet > netdev-offload: Introduce register sFlow upcall callback API > netdev-offload-tc: Implement register sFlow upcall callback API > dpif-netlink: Implement register sFlow upcall callback API > netdev-offload-tc: Introduce group ID management API > netdev-offload-tc: Remove redundant ovsthread once > netdev-offload-tc: Create psample netlink socket > netdev-offload-tc: Add psample receive handler > netdev-offload-tc: Add offload support for sFlow > > include/linux/automake.mk| 4 +- > include/linux/psample.h | 58 +++ > include/linux/tc_act/tc_sample.h | 25 ++ > lib/dpif-netdev.c| 1 + > lib/dpif-netlink.c | 27 ++ > lib/dpif-netlink.h | 4 + > lib/dpif-provider.h | 10 + > lib/dpif.c | 8 + > lib/dpif.h | 26 ++ > lib/netdev-offload-provider.h| 3 + > lib/netdev-offload-tc.c | 594 > ++- > lib/netdev-offload.c | 30 ++ > lib/netdev-offload.h | 4 + > lib/tc.c | 61 +++- > lib/tc.h | 10 +- > ofproto/ofproto-dpif-upcall.c| 42 +++ > utilities/ovs-kmod-ctl.in| 14 + > 17 files changed, 902 insertions(+), 19 deletions(-) > create mode 100644 include/linux/psample.h > create mode 100644 include/linux/tc_act/tc_sample.h > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v6 2/2] netdev-dpdk: Add option to configure VF MAC address.
In some cloud topologies, using DPDK VF representors in guest requires
configuring a VF before it is assigned to the guest.
A first basic option for such configuration is setting the VF MAC
address. Add a key 'dpdk-vf-mac' to the 'options' column of the Interface
table.
This option can be used as such:
$ ovs-vsctl add-port br0 dpdk-rep0 -- set Interface dpdk-rep0 type=dpdk \
options:dpdk-vf-mac=00:11:22:33:44:55
Signed-off-by: Gaetan Rivet
Suggested-by: Ilya Maximets
Acked-by: Eli Britstein
Acked-by: Kevin Traynor
---
Documentation/topics/dpdk/phy.rst | 51 +++
NEWS | 2 +
lib/netdev-dpdk.c | 69 +++
vswitchd/vswitch.xml | 18
4 files changed, 140 insertions(+)
diff --git a/Documentation/topics/dpdk/phy.rst
b/Documentation/topics/dpdk/phy.rst
index 55a98e2b0..7ee3eacff 100644
--- a/Documentation/topics/dpdk/phy.rst
+++ b/Documentation/topics/dpdk/phy.rst
@@ -379,6 +379,57 @@ an eth device whose mac address is ``00:11:22:33:44:55``::
$ ovs-vsctl add-port br0 dpdk-mac -- set Interface dpdk-mac type=dpdk \
options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
+Representor specific configuration
+~~
+
+In some topologies, a VF must be configured before being assigned to a
+guest (VM) machine. This configuration is done through VF-specific fields
+in the ``options`` column of the ``Interface`` table.
+
+.. important::
+
+ Some DPDK port use `bifurcated drivers `__,
+ which means that a kernel netdevice remains when Open vSwitch is stopped.
+
+ In such case, any configuration applied to a VF would remain set on the
+ kernel netdevice, and be inherited from it when Open vSwitch is restarted,
+ even if the options described in this section are unset from Open vSwitch.
+
+.. _bifurcated-drivers:
http://doc.dpdk.org/guides/linux_gsg/linux_drivers.html#bifurcated-driver
+
+- Configure the VF MAC address::
+
+$ ovs-vsctl set Interface dpdk-rep0 options:dpdk-vf-mac=00:11:22:33:44:55
+
+The requested MAC address is assigned to the port and is listed as part of
+its options::
+
+$ ovs-appctl dpctl/show
+[...]
+ port 3: dpdk-rep0 (dpdk: configured_rx_queues=1, ...,
dpdk-vf-mac=00:11:22:33:44:55, ...)
+
+$ ovs-vsctl show
+[...]
+Port dpdk-rep0
+Interface dpdk-rep0
+type: dpdk
+options: {dpdk-devargs="",
dpdk-vf-mac="00:11:22:33:44:55"}
+
+$ ovs-vsctl get Interface dpdk-rep0 status
+{dpdk-vf-mac="00:11:22:33:44:55", ...}
+
+$ ovs-vsctl list Interface dpdk-rep0 | grep 'mac_in_use\|options'
+mac_in_use : "00:11:22:33:44:55"
+options : {dpdk-devargs="",
dpdk-vf-mac="00:11:22:33:44:55"}
+
+The value listed as ``dpdk-vf-mac`` is only a request from the user and is
+possibly not yet applied.
+
+When the requested configuration is successfully applied to the port,
+this MAC address is then also shown in the column ``mac_in_use`` of
+the ``Interface`` table. On failure however, ``mac_in_use`` will keep its
+previous value, which will thus differ from ``dpdk-vf-mac``.
+
Jumbo Frames
diff --git a/NEWS b/NEWS
index a542c68ca..18848 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,8 @@ Post-v2.14.0
- Userspace datapath:
* Add the 'pmd' option to "ovs-appctl dpctl/dump-flows", which
restricts a flow dump to a single PMD thread if set.
+ * New 'options:dpdk-vf-mac' field for DPDK interface of VF ports,
+ that allows configuring the MAC address of a VF representor.
- The environment variable OVS_UNBOUND_CONF, if set, is now used
as the DNS resolver's (unbound) configuration file.
- Linux datapath:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 084f97807..75dffefb8 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -522,6 +522,9 @@ struct netdev_dpdk {
* otherwise interrupt mode is used. */
bool requested_lsc_interrupt_mode;
bool lsc_interrupt_mode;
+
+/* VF configuration. */
+struct eth_addr requested_hwaddr;
);
PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1692,6 +1695,16 @@ out:
return ret;
}
+static bool
+dpdk_port_is_representor(struct netdev_dpdk *dev)
+OVS_REQUIRES(dev->mutex)
+{
+struct rte_eth_dev_info dev_info;
+
+rte_eth_dev_info_get(dev->port_id, &dev_info);
+return (*dev_info.dev_flags) & RTE_ETH_DEV_REPRESENTOR;
+}
+
static int
netdev_dpdk_get_config(const struct netdev *netdev, struct smap *args)
{
@@ -1726,6 +1739,11 @@ netdev_dpdk_get_config(const struct netdev *netdev,
struct smap *args)
}
smap_add(args, "lsc_interrupt_mode",
dev->lsc_interrupt_mode ? "true" : "false");
+
+if (dpdk_port_is_representor(dev)) {
+smap_add_format(args, "dpdk-vf-mac", ETH_ADDR_FMT,
+ETH_AD
Re: [ovs-dev] [PATCH] ovsdb-idl.at: Return stream open_block python tests.
On 10/11/20 11:56 +0100, Ilya Maximets wrote:
> On 11/10/20 11:39 AM, Gaëtan Rivet wrote:
> > On 10/11/20 10:15 +0100, Ilya Maximets wrote:
> >> On 11/9/20 2:45 AM, Gaëtan Rivet wrote:
> >>> Hi Ilya,
> >>>
> >>> One nit below,
> >>>
> >>> On 04/09/20 13:51 +0200, Ilya Maximets wrote:
> Invocations of CHECK_STREAM_OPEN_BLOCK_PY was accidentally removed
> during python2 to python3 conversion. So, these tests was not
> checked since that time.
>
> This change returns tests back. CHECK_STREAM_OPEN_BLOCK_PY needed
> updates, so instead I refactored function for C tests to be able to
> perform python tests too.
>
> Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
> Signed-off-by: Ilya Maximets
> ---
> tests/ovsdb-idl.at | 36 ++--
> 1 file changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> index 789ae23a9..96be361fc 100644
> --- a/tests/ovsdb-idl.at
> +++ b/tests/ovsdb-idl.at
> @@ -1778,33 +1778,25 @@ OVSDB_CHECK_IDL_COMPOUND_INDEX_WITH_REF([set,
> simple3 idl-compound-index-with-re
> ]])
>
> m4_define([CHECK_STREAM_OPEN_BLOCK],
> - [AT_SETUP([Check Stream open block - C - $1])
> - AT_SKIP_IF([test "$1" = "tcp6" && test "$IS_WIN32" = "yes"])
> - AT_SKIP_IF([test "$1" = "tcp6" && test "$HAVE_IPV6" = "no"])
> - AT_KEYWORDS([Check Stream open block $1])
> - AT_CHECK([ovsdb_start_idltest "ptcp:0:$2"])
> + [AT_SETUP([Check stream open block - $1 - $3])
> + AT_SKIP_IF([test "$3" = "tcp6" && test "$IS_WIN32" = "yes"])
> + AT_SKIP_IF([test "$3" = "tcp6" && test "$HAVE_IPV6" = "no"])
> + AT_KEYWORDS([Check stream open block open_block $3])
> >>>
> >>> The keywords seems to copy the title. Is 'Check' a relevant keyword for
> >>> this test? I only see it used there. Using TESTSUITEFLAGS='-k Check'
> >>> seems less intuitive than '-k open_block' for example.
> >>>
> >>> Also, reading the keywords in ovsdb-idl.at, 'ovsdb server' is
> >>> always used except for those two tests. Would it be relevant there as
> >>> well?
> >>
> >>
> >> I don't like these keywords too. I kept them because they were in the
> >> original test. But yes, we could make them better.
> >>
> >> Something like:
> >> AT_KEYWORDS([ovsdb server stream open_block $1 $3])
> >>
> >> What do you think?
> >>
> >
> > I think those keywords are better, one small nit for $1 -- in this case
> > either 'C' or 'Python3'. Other tests with python are using 'Python'
> > instead.
> >
> > Invoking the macro with 'Python' instead would align with others, and it
> > seems fine as python2 support was dropped.
>
> All other tests has Python3 in the test name. We could actually just make
> it conditional, if it's important:
>
>AT_KEYWORDS([ovsdb server stream open_block
> m4_if([$1], [Python3], [Python], [$1]) $3])
>
> Bes regards, Ilya Maximets.
IMO this makes reading the code harder for no real gain, I would drop $1 from
the
keywords, but that's just me :).
Whichever way you prefer to go,
Acked-by: Gaetan Rivet
Best,
--
Gaëtan
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idl: Add ovsdb_idl_monitor_condition_pending().
On 11/10/20 12:50 PM, Ilya Maximets wrote:
> On 11/10/20 11:31 AM, Dumitru Ceara wrote:
>> On 11/10/20 11:07 AM, Ilya Maximets wrote:
>>> On 11/9/20 12:09 PM, Dumitru Ceara wrote:
IDL clients had no way of checking whether monitor_cond_change requests
were pending (either local or in flight). This commit introduces a new
API to check for the state of the conditional monitoring clauses.
Signed-off-by: Dumitru Ceara
---
lib/ovsdb-idl.c | 40
lib/ovsdb-idl.h | 1 +
2 files changed, 33 insertions(+), 8 deletions(-)
>>>
>>>
>>>
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index a1a5776..3f19d40 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -421,6 +421,7 @@ unsigned int ovsdb_idl_set_condition(struct ovsdb_idl
*,
const struct ovsdb_idl_condition *);
unsigned int ovsdb_idl_get_condition_seqno(const struct ovsdb_idl *);
+bool ovsdb_idl_monitor_condition_pending(struct ovsdb_idl *);
>>>
>>> I don't think that we need a new api for this. Condition sequence number,
>>> I believe, exists for exactly this case. There is an issue, however, that
>>> db bindings doesn't return result of ovsdb_idl_set_condition() throwing it
>>> away without giving a chance for an idl user to actually use it. That could
>>> be fixed by something like this:
>>>
>>> diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
>>> index 698fe25f3..d319adb03 100755
>>> --- a/ovsdb/ovsdb-idlc.in
>>> +++ b/ovsdb/ovsdb-idlc.in
>>> @@ -1416,10 +1416,10 @@ struct %(s)s *
>>> print("\nstruct ovsdb_idl_column %s_columns[%s_N_COLUMNS];" % (
>>> structName, structName.upper()))
>>> print("""
>>> -void
>>> +unsigned int
>>> %(s)s_set_condition(struct ovsdb_idl *idl, struct ovsdb_idl_condition
>>> *condition)
>>> {
>>> -ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
>>> +return ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
>>> }""" % {'p': prefix,
>>> 's': structName,
>>> 'tl': tableName.lower()})
>>> ---
>>>
>>> I think, I had this patch somewhere already, but it seems that I never
>>> actually send it.
>>
>> Thanks for looking into this, Ilya! Yes, returning the seqno would
>> probably be nice to have in *_set_condition(..).
>>
>>>
>>> With this change ovn-controller will need to store the returned value
>>> and wait until current condition seqno != expected to be sure that
>>> condition was successfully set.
>>>
>>> What do you think?
>>>
>>
>> I thought about this too before proposing the new API but it seemed to
>> me that in that case the code that sets the conditions in ovn-controller
>> might get unnecessarily complicated:
>>
>> https://github.com/ovn-org/ovn/blob/c108f23e1c10910031f9409b79001d001aae0c8f/controller/ovn-controller.c#L240
>>
>> I guess it can be rewritten as something like this:
>>
>> expected_cond_seqno = 0;
>> expected_cond_seqno =
>> MAX(sbrec_port_binding_set_condition(ovnsb_idl, &pb),
>> expected_cond_seqno);
>> [...]
>> expected_cond_seqno =
>> MAX(sbrec_chassis_private_set_condition(ovnsb_idl, &chprv),
>> expected_cond_seqno);
>> [...]
>> return expected_cond_seqno;
>>
>> I know it's not really OVS's problem but the set_condition() API doesn't
>> seem to make it easy to write simple code to use it properly.
>>
>> However, if you think a new API would be redundant, we'll have to just
>> find a way to properly explain (comments I guess) why we need to do
>> things like this in ovn-controller.
>
> Maybe a little pinch of magic will help? :)
>
I think it does. :)
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index a06cae3cc..8bc331e95 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -236,16 +236,24 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
> }
> }
>
> -out:
> -sbrec_port_binding_set_condition(ovnsb_idl, &pb);
> -sbrec_logical_flow_set_condition(ovnsb_idl, &lf);
> -sbrec_mac_binding_set_condition(ovnsb_idl, &mb);
> -sbrec_multicast_group_set_condition(ovnsb_idl, &mg);
> -sbrec_dns_set_condition(ovnsb_idl, &dns);
> -sbrec_controller_event_set_condition(ovnsb_idl, &ce);
> -sbrec_ip_multicast_set_condition(ovnsb_idl, &ip_mcast);
> -sbrec_igmp_group_set_condition(ovnsb_idl, &igmp);
> -sbrec_chassis_private_set_condition(ovnsb_idl, &chprv);
> +out:;
I'll see if I can get rid of this label all together.
> +unsigned int cond_seqnos[] = {
> +sbrec_port_binding_set_condition(ovnsb_idl, &pb),
> +sbrec_logical_flow_set_condition(ovnsb_idl, &lf),
> +sbrec_mac_binding_set_condition(ovnsb_idl, &mb),
> +sbrec_multicast_group_set_condition(ovnsb_idl, &mg),
> +sbrec_dns_set_condition(ovnsb_idl, &dns),
> +sbrec_controller_event_set_condition(ovnsb_idl, &ce),
> +
Re: [ovs-dev] [PATCH v6 1/2] netdev-dpdk: Add ability to set MAC address.
Bleep bloop. Greetings Gaetan Rivet, 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: Gaetan Rivet Lines checked: 72, 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
[ovs-dev] [PATCH v6 1/2] netdev-dpdk: Add ability to set MAC address.
From: Ilya Maximets
It is possible to set the MAC address of DPDK ports by calling
rte_eth_dev_default_mac_addr_set(). OvS does not actually call
this function for non-internal ports, but the implementation is
exposed to be used in a later commit.
Signed-off-by: Ilya Maximets
Signed-off-by: Gaetan Rivet
---
lib/netdev-dpdk.c | 32 +---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 0b830be78..084f97807 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2910,19 +2910,45 @@ netdev_dpdk_eth_send(struct netdev *netdev, int qid,
return 0;
}
+static int
+netdev_dpdk_set_etheraddr__(struct netdev_dpdk *dev, const struct eth_addr mac)
+OVS_REQUIRES(dev->mutex)
+{
+int err = 0;
+
+if (dev->type == DPDK_DEV_ETH) {
+struct rte_ether_addr ea;
+
+memcpy(ea.addr_bytes, mac.ea, ETH_ADDR_LEN);
+err = -rte_eth_dev_default_mac_addr_set(dev->port_id, &ea);
+}
+if (!err) {
+dev->hwaddr = mac;
+} else {
+VLOG_WARN("%s: Failed to set requested mac("ETH_ADDR_FMT"): %s",
+ netdev_get_name(&dev->up), ETH_ADDR_ARGS(mac),
+ rte_strerror(err));
+}
+
+return err;
+}
+
static int
netdev_dpdk_set_etheraddr(struct netdev *netdev, const struct eth_addr mac)
{
struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+int err = 0;
ovs_mutex_lock(&dev->mutex);
if (!eth_addr_equals(dev->hwaddr, mac)) {
-dev->hwaddr = mac;
-netdev_change_seq_changed(netdev);
+err = netdev_dpdk_set_etheraddr__(dev, mac);
+if (!err) {
+netdev_change_seq_changed(netdev);
+}
}
ovs_mutex_unlock(&dev->mutex);
-return 0;
+return err;
}
static int
--
2.29.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 2/2] netdev-dpdk: Add option to configure VF MAC address.
Bleep bloop. Greetings Gaetan Rivet, 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 97 characters long (recommended limit is 79)
#53 FILE: Documentation/topics/dpdk/phy.rst:398:
.. _bifurcated-drivers:
http://doc.dpdk.org/guides/linux_gsg/linux_drivers.html#bifurcated-driver
WARNING: Line is 95 characters long (recommended limit is 79)
#64 FILE: Documentation/topics/dpdk/phy.rst:409:
port 3: dpdk-rep0 (dpdk: configured_rx_queues=1, ...,
dpdk-vf-mac=00:11:22:33:44:55, ...)
WARNING: Line is 100 characters long (recommended limit is 79)
#71 FILE: Documentation/topics/dpdk/phy.rst:416:
options: {dpdk-devargs="",
dpdk-vf-mac="00:11:22:33:44:55"}
WARNING: Line is 97 characters long (recommended limit is 79)
#78 FILE: Documentation/topics/dpdk/phy.rst:423:
options : {dpdk-devargs="",
dpdk-vf-mac="00:11:22:33:44:55"}
Lines checked: 285, Warnings: 4, 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
[ovs-dev] [PATCH ovn v2] northd: Don't poll ovsdb before the connection is fully established
Set initial SB and NB DBs probe interval to 0 to avoid connection flapping. Before configured in northd_probe_interval value is actually applied to southbound and northbound database connections, both connections must be fully established, otherwise ovnnb_db_run() will return without retrieving configuration data from northbound DB. In cases when southbound database is big enough, default interval of 5 seconds will kill and retry the connection before it is fully established, no matter what is set in northd_probe_interval. Client reconnect will cause even more load to ovsdb-server and cause cascade effect, so northd can never stabilise. We have more than 2000 ports in our lab, and northd could not start before this patch, holding at 100% CPU utilisation both itself and ovsdb-server. After connections are established, any value in northd_probe_interval, or default DEFAULT_PROBE_INTERVAL_MSEC is applied correctly. Signed-off-by: Renat Nurgaliyev --- v2: - Resubmitting patch because git am failed last time --- northd/ovn-northd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 684c2bd47..073c6a0f3 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -100,7 +100,7 @@ static struct eth_addr svc_monitor_mac_ea; /* Default probe interval for NB and SB DB connections. */ #define DEFAULT_PROBE_INTERVAL_MSEC 5000 -static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC; -static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC; +static int northd_probe_interval_nb = 0; +static int northd_probe_interval_sb = 0; #define MAX_OVN_TAGS 4096 -- 2.29.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ovsdb-idlc: Return expected sequence number while setting conditions.
ovsdb_idl_set_condition() returns a sequence number that can be used to
check if the requested conditions are acknowledged by the server.
However, database bindings do not return this value to the user, making
it impossible to check if the conditions are accepted.
Signed-off-by: Ilya Maximets
---
ovsdb/ovsdb-idlc.in | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 698fe25f3..b13195606 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -389,7 +389,7 @@ bool %(s)s_is_updated(const struct %(s)s *, enum
%(s)s_column_id);
args = ['%(type)s%(name)s' % member for member in members]
print('%s);' % ', '.join(args))
-print('void %(s)s_set_condition(struct ovsdb_idl *, struct
ovsdb_idl_condition *);' % {'s': structName})
+print('unsigned int %(s)s_set_condition(struct ovsdb_idl *, struct
ovsdb_idl_condition *);' % {'s': structName})
print("")
@@ -1416,10 +1416,10 @@ struct %(s)s *
print("\nstruct ovsdb_idl_column %s_columns[%s_N_COLUMNS];" % (
structName, structName.upper()))
print("""
-void
+unsigned int
%(s)s_set_condition(struct ovsdb_idl *idl, struct ovsdb_idl_condition
*condition)
{
-ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
+return ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
}""" % {'p': prefix,
's': structName,
'tl': tableName.lower()})
--
2.25.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idl: Add ovsdb_idl_monitor_condition_pending().
On 11/10/20 12:59 PM, Dumitru Ceara wrote:
> On 11/10/20 12:50 PM, Ilya Maximets wrote:
>> On 11/10/20 11:31 AM, Dumitru Ceara wrote:
>>> On 11/10/20 11:07 AM, Ilya Maximets wrote:
On 11/9/20 12:09 PM, Dumitru Ceara wrote:
> IDL clients had no way of checking whether monitor_cond_change requests
> were pending (either local or in flight). This commit introduces a new
> API to check for the state of the conditional monitoring clauses.
>
> Signed-off-by: Dumitru Ceara
> ---
> lib/ovsdb-idl.c | 40
> lib/ovsdb-idl.h | 1 +
> 2 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> index a1a5776..3f19d40 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -421,6 +421,7 @@ unsigned int ovsdb_idl_set_condition(struct ovsdb_idl
> *,
> const struct ovsdb_idl_condition *);
>
> unsigned int ovsdb_idl_get_condition_seqno(const struct ovsdb_idl *);
> +bool ovsdb_idl_monitor_condition_pending(struct ovsdb_idl *);
I don't think that we need a new api for this. Condition sequence number,
I believe, exists for exactly this case. There is an issue, however, that
db bindings doesn't return result of ovsdb_idl_set_condition() throwing it
away without giving a chance for an idl user to actually use it. That
could
be fixed by something like this:
diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index 698fe25f3..d319adb03 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -1416,10 +1416,10 @@ struct %(s)s *
print("\nstruct ovsdb_idl_column %s_columns[%s_N_COLUMNS];" % (
structName, structName.upper()))
print("""
-void
+unsigned int
%(s)s_set_condition(struct ovsdb_idl *idl, struct ovsdb_idl_condition
*condition)
{
-ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
+return ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
}""" % {'p': prefix,
's': structName,
'tl': tableName.lower()})
---
I think, I had this patch somewhere already, but it seems that I never
actually send it.
>>>
>>> Thanks for looking into this, Ilya! Yes, returning the seqno would
>>> probably be nice to have in *_set_condition(..).
>>>
With this change ovn-controller will need to store the returned value
and wait until current condition seqno != expected to be sure that
condition was successfully set.
What do you think?
>>>
>>> I thought about this too before proposing the new API but it seemed to
>>> me that in that case the code that sets the conditions in ovn-controller
>>> might get unnecessarily complicated:
>>>
>>> https://github.com/ovn-org/ovn/blob/c108f23e1c10910031f9409b79001d001aae0c8f/controller/ovn-controller.c#L240
>>>
>>> I guess it can be rewritten as something like this:
>>>
>>> expected_cond_seqno = 0;
>>> expected_cond_seqno =
>>> MAX(sbrec_port_binding_set_condition(ovnsb_idl, &pb),
>>> expected_cond_seqno);
>>> [...]
>>> expected_cond_seqno =
>>> MAX(sbrec_chassis_private_set_condition(ovnsb_idl, &chprv),
>>> expected_cond_seqno);
>>> [...]
>>> return expected_cond_seqno;
>>>
>>> I know it's not really OVS's problem but the set_condition() API doesn't
>>> seem to make it easy to write simple code to use it properly.
>>>
>>> However, if you think a new API would be redundant, we'll have to just
>>> find a way to properly explain (comments I guess) why we need to do
>>> things like this in ovn-controller.
>>
>> Maybe a little pinch of magic will help? :)
>>
>
> I think it does. :)
>
>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> index a06cae3cc..8bc331e95 100644
>> --- a/controller/ovn-controller.c
>> +++ b/controller/ovn-controller.c
>> @@ -236,16 +236,24 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
>> }
>> }
>>
>> -out:
>> -sbrec_port_binding_set_condition(ovnsb_idl, &pb);
>> -sbrec_logical_flow_set_condition(ovnsb_idl, &lf);
>> -sbrec_mac_binding_set_condition(ovnsb_idl, &mb);
>> -sbrec_multicast_group_set_condition(ovnsb_idl, &mg);
>> -sbrec_dns_set_condition(ovnsb_idl, &dns);
>> -sbrec_controller_event_set_condition(ovnsb_idl, &ce);
>> -sbrec_ip_multicast_set_condition(ovnsb_idl, &ip_mcast);
>> -sbrec_igmp_group_set_condition(ovnsb_idl, &igmp);
>> -sbrec_chassis_private_set_condition(ovnsb_idl, &chprv);
>> +out:;
>
> I'll see if I can get rid of this label all together.
>
>> +unsigned int cond_seqnos[] = {
>> +sbrec_port_binding_set_condition(ovnsb_idl, &pb),
>> +sbrec_logical_flow_set_condition(ovnsb_idl, &lf),
>> +sbrec_mac_binding_set_condition(ovnsb_idl, &mb),
>> +
[ovs-dev] [PATCH ovn v3] northd: Don't poll ovsdb before the connection is fully established
Set initial SB and NB DBs probe interval to 0 to avoid connection flapping. Before configured in northd_probe_interval value is actually applied to southbound and northbound database connections, both connections must be fully established, otherwise ovnnb_db_run() will return without retrieving configuration data from northbound DB. In cases when southbound database is big enough, default interval of 5 seconds will kill and retry the connection before it is fully established, no matter what is set in northd_probe_interval. Client reconnect will cause even more load to ovsdb-server and cause cascade effect, so northd can never stabilise. We have more than 2000 ports in our lab, and northd could not start before this patch, holding at 100% CPU utilisation both itself and ovsdb-server. After connections are established, any value in northd_probe_interval, or default DEFAULT_PROBE_INTERVAL_MSEC is applied correctly. Signed-off-by: Renat Nurgaliyev --- v2: - Resubmitting patch because git am failed last time v3: - Resubmitting patch because mail client broke formatting --- northd/ovn-northd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 684c2bd47..073c6a0f3 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -100,7 +100,7 @@ static struct eth_addr svc_monitor_mac_ea; /* Default probe interval for NB and SB DB connections. */ #define DEFAULT_PROBE_INTERVAL_MSEC 5000 -static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC; -static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC; +static int northd_probe_interval_nb = 0; +static int northd_probe_interval_sb = 0; #define MAX_OVN_TAGS 4096 -- 2.29.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [net-next] netfiler: conntrack: Add the option to set ct tcp flag - BE_LIBERAL per-ct basis.
Numan Siddique wrote: > On Tue, Nov 10, 2020 at 3:06 AM Florian Westphal wrote: > Thanks for the comments. I actually tried this approach first, but it > doesn't seem to work. > I noticed that for the committed connections, the ct tcp flag - > IP_CT_TCP_FLAG_BE_LIBERAL is > not set when nf_conntrack_in() calls resolve_normal_ct(). Yes, it won't be set during nf_conntrack_in, thats why I suggested to do it before confirming the connection. > Would you expect that the tcp ct flags should have been preserved once > the connection is committed ? Yes, they are preserved when you set them after nf_conntrack_in(), else we would already have trouble with hw flow offloading which needs to turn off window checks as well. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-offload-tc: Use single thread for probing tc features
On 2020-11-02 2:07 PM, Roi Dayan wrote:
There is no need for a thread start per probe.
Signed-off-by: Roi Dayan
Reviewed-by: Paul Blakey
---
lib/netdev-offload-tc.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index e828a8683910..53662ef3f0e6 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1988,8 +1988,7 @@ probe_tc_block_support(int ifindex)
static int
netdev_tc_init_flow_api(struct netdev *netdev)
{
-static struct ovsthread_once multi_mask_once = OVSTHREAD_ONCE_INITIALIZER;
-static struct ovsthread_once block_once = OVSTHREAD_ONCE_INITIALIZER;
+static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
uint32_t block_id = 0;
struct tcf_id id;
@@ -2014,16 +2013,12 @@ netdev_tc_init_flow_api(struct netdev *netdev)
/* make sure there is no ingress/egress qdisc */
tc_add_del_qdisc(ifindex, false, 0, hook);
-if (ovsthread_once_start(&block_once)) {
+if (ovsthread_once_start(&once)) {
probe_tc_block_support(ifindex);
/* Need to re-fetch block id as it depends on feature availability. */
block_id = get_block_id_from_netdev(netdev);
-ovsthread_once_done(&block_once);
-}
-
-if (ovsthread_once_start(&multi_mask_once)) {
probe_multi_mask_per_prio(ifindex);
-ovsthread_once_done(&multi_mask_once);
+ovsthread_once_done(&once);
}
error = tc_add_del_qdisc(ifindex, true, block_id, hook);
Hi,
Any comments, can we merge this small change?
Thanks,
Roi
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-offload-tc: Use single thread for probing tc features
On 11/10/20 1:39 PM, Roi Dayan wrote:
>
>
> On 2020-11-02 2:07 PM, Roi Dayan wrote:
>> There is no need for a thread start per probe.
>>
>> Signed-off-by: Roi Dayan
>> Reviewed-by: Paul Blakey
>> ---
>> lib/netdev-offload-tc.c | 11 +++
>> 1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
>> index e828a8683910..53662ef3f0e6 100644
>> --- a/lib/netdev-offload-tc.c
>> +++ b/lib/netdev-offload-tc.c
>> @@ -1988,8 +1988,7 @@ probe_tc_block_support(int ifindex)
>> static int
>> netdev_tc_init_flow_api(struct netdev *netdev)
>> {
>> - static struct ovsthread_once multi_mask_once =
>> OVSTHREAD_ONCE_INITIALIZER;
>> - static struct ovsthread_once block_once = OVSTHREAD_ONCE_INITIALIZER;
>> + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>> enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
>> uint32_t block_id = 0;
>> struct tcf_id id;
>> @@ -2014,16 +2013,12 @@ netdev_tc_init_flow_api(struct netdev *netdev)
>> /* make sure there is no ingress/egress qdisc */
>> tc_add_del_qdisc(ifindex, false, 0, hook);
>> - if (ovsthread_once_start(&block_once)) {
>> + if (ovsthread_once_start(&once)) {
>> probe_tc_block_support(ifindex);
>> /* Need to re-fetch block id as it depends on feature
>> availability. */
>> block_id = get_block_id_from_netdev(netdev);
>> - ovsthread_once_done(&block_once);
>> - }
>> -
>> - if (ovsthread_once_start(&multi_mask_once)) {
>> probe_multi_mask_per_prio(ifindex);
>> - ovsthread_once_done(&multi_mask_once);
>> + ovsthread_once_done(&once);
>> }
>> error = tc_add_del_qdisc(ifindex, true, block_id, hook);
>>
>
>
> Hi,
>
> Any comments, can we merge this small change?
Have you missed previous replies from me and Simon?
They are available on mail list and in patchwork:
https://patchwork.ozlabs.org/project/openvswitch/patch/20201102120702.173016-1-r...@nvidia.com/
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [net-next] netfiler: conntrack: Add the option to set ct tcp flag - BE_LIBERAL per-ct basis.
On Tue, Nov 10, 2020 at 5:55 PM Florian Westphal wrote: > > Numan Siddique wrote: > > On Tue, Nov 10, 2020 at 3:06 AM Florian Westphal wrote: > > Thanks for the comments. I actually tried this approach first, but it > > doesn't seem to work. > > I noticed that for the committed connections, the ct tcp flag - > > IP_CT_TCP_FLAG_BE_LIBERAL is > > not set when nf_conntrack_in() calls resolve_normal_ct(). > > Yes, it won't be set during nf_conntrack_in, thats why I suggested > to do it before confirming the connection. Sorry for the confusion. What I mean is - I tested your suggestion - i.e called nf_ct_set_tcp_be_liberal() before calling nf_conntrack_confirm(). Once the connection is established, for subsequent packets, openvswitch calls nf_conntrack_in() [1] to see if the packet is part of the existing connection or not (i.e ct.new or ct.est ) and if the packet happens to be out-of-window then skb->_nfct is set to NULL. And the tcp be flags set during confirmation are not reflected when nf_conntrack_in() calls resolve_normal_ct(). > > > Would you expect that the tcp ct flags should have been preserved once > > the connection is committed ? > > Yes, they are preserved when you set them after nf_conntrack_in(), else > we would already have trouble with hw flow offloading which needs to > turn off window checks as well. Looks they are not preserved for the openvswitch case. Probably openvswitch is doing something wrong. I will debug further and see what is going on. Please let me know if you have any further comments. Thanks Numan > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v2] northd: Don't poll ovsdb before the connection is fully established
Bleep bloop. Greetings Renat Nurgaliyev, 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. git-am: error: corrupt patch at line 16 error: could not build fake ancestor hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0001 northd: Don't poll ovsdb before the connection is fully established When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v4] northd: Don't poll ovsdb before the connection is fully established
Set initial SB and NB DBs probe interval to 0 to avoid connection flapping. Before configured in northd_probe_interval value is actually applied to southbound and northbound database connections, both connections must be fully established, otherwise ovnnb_db_run() will return without retrieving configuration data from northbound DB. In cases when southbound database is big enough, default interval of 5 seconds will kill and retry the connection before it is fully established, no matter what is set in northd_probe_interval. Client reconnect will cause even more load to ovsdb-server and cause cascade effect, so northd can never stabilise. We have more than 2000 ports in our lab, and northd could not start before this patch, holding at 100% CPU utilisation both itself and ovsdb-server. After connections are established, any value in northd_probe_interval, or default DEFAULT_PROBE_INTERVAL_MSEC is applied correctly. Signed-off-by: Renat Nurgaliyev --- v2: - Resubmitting patch because git am failed last time v3: - Resubmitting patch because mail client broke formatting v4: - Resubmitting patch because mail client broke formatting once again. I switched to mutt and it won't happen again, I promise guys. --- northd/ovn-northd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 684c2bd47..073c6a0f3 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -100,7 +100,7 @@ static struct eth_addr svc_monitor_mac_ea; /* Default probe interval for NB and SB DB connections. */ #define DEFAULT_PROBE_INTERVAL_MSEC 5000 -static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC; -static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC; +static int northd_probe_interval_nb = 0; +static int northd_probe_interval_sb = 0; #define MAX_OVN_TAGS 4096 -- 2.29.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3] northd: Don't poll ovsdb before the connection is fully established
Bleep bloop. Greetings Renat Nurgaliyev, 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. git-am: error: patch failed: northd/ovn-northd.c:100 error: northd/ovn-northd.c: patch does not apply error: Did you hand edit your patch? It does not apply to blobs recorded in its index. hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0001 northd: Don't poll ovsdb before the connection is fully established When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". 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] [net-next] netfiler: conntrack: Add the option to set ct tcp flag - BE_LIBERAL per-ct basis.
Numan Siddique wrote: > On Tue, Nov 10, 2020 at 5:55 PM Florian Westphal wrote: > > > > Numan Siddique wrote: > > > On Tue, Nov 10, 2020 at 3:06 AM Florian Westphal wrote: > > > Thanks for the comments. I actually tried this approach first, but it > > > doesn't seem to work. > > > I noticed that for the committed connections, the ct tcp flag - > > > IP_CT_TCP_FLAG_BE_LIBERAL is > > > not set when nf_conntrack_in() calls resolve_normal_ct(). > > > > Yes, it won't be set during nf_conntrack_in, thats why I suggested > > to do it before confirming the connection. > > Sorry for the confusion. What I mean is - I tested your suggestion - i.e > called > nf_ct_set_tcp_be_liberal() before calling nf_conntrack_confirm(). > > Once the connection is established, for subsequent packets, openvswitch > calls nf_conntrack_in() [1] to see if the packet is part of the > existing connection or not (i.e ct.new or ct.est ) > and if the packet happens to be out-of-window then skb->_nfct is set > to NULL. And the tcp > be flags set during confirmation are not reflected when > nf_conntrack_in() calls resolve_normal_ct(). Can you debug where this happens? This looks very very wrong. resolve_normal_ct() has no business to check any of those flags (and I don't see where it uses them, it should only deal with the tuples). The flags come into play when nf_conntrack_handle_packet() gets called after resolve_normal_ct has found an entry, since that will end up calling the tcp conntrack part. The entry found/returned by resolve_normal_ct should be the same nf_conn entry that was confirmed earlier, i.e. it should be in "liberal" mode. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idlc: Return expected sequence number while setting conditions.
On 11/10/20 1:18 PM, Ilya Maximets wrote:
> ovsdb_idl_set_condition() returns a sequence number that can be used to
> check if the requested conditions are acknowledged by the server.
> However, database bindings do not return this value to the user, making
> it impossible to check if the conditions are accepted.
>
> Signed-off-by: Ilya Maximets
Hi Ilya,
This change looks OK to me:
Acked-by: Dumitru Ceara
It did uncover another bug in ovsdb_idl_db_condition_set() but I'll send
a separate patch for that.
Thanks,
Dumitru
> ---
> ovsdb/ovsdb-idlc.in | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
> index 698fe25f3..b13195606 100755
> --- a/ovsdb/ovsdb-idlc.in
> +++ b/ovsdb/ovsdb-idlc.in
> @@ -389,7 +389,7 @@ bool %(s)s_is_updated(const struct %(s)s *, enum
> %(s)s_column_id);
> args = ['%(type)s%(name)s' % member for member in members]
> print('%s);' % ', '.join(args))
>
> -print('void %(s)s_set_condition(struct ovsdb_idl *, struct
> ovsdb_idl_condition *);' % {'s': structName})
> +print('unsigned int %(s)s_set_condition(struct ovsdb_idl *,
> struct ovsdb_idl_condition *);' % {'s': structName})
>
> print("")
>
> @@ -1416,10 +1416,10 @@ struct %(s)s *
> print("\nstruct ovsdb_idl_column %s_columns[%s_N_COLUMNS];" % (
> structName, structName.upper()))
> print("""
> -void
> +unsigned int
> %(s)s_set_condition(struct ovsdb_idl *idl, struct ovsdb_idl_condition
> *condition)
> {
> -ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
> +return ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition);
> }""" % {'p': prefix,
> 's': structName,
> 'tl': tableName.lower()})
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4] northd: Don't poll ovsdb before the connection is fully established
Bleep bloop. Greetings Renat Nurgaliyev, 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: ERROR: Author Renat Nurgaliyev needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Renat Nurgaliyev Lines checked: 47, Warnings: 1, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v5] northd: Don't poll ovsdb before the connection is fully established
Set initial SB and NB DBs probe interval to 0 to avoid connection flapping. Before configured in northd_probe_interval value is actually applied to southbound and northbound database connections, both connections must be fully established, otherwise ovnnb_db_run() will return without retrieving configuration data from northbound DB. In cases when southbound database is big enough, default interval of 5 seconds will kill and retry the connection before it is fully established, no matter what is set in northd_probe_interval. Client reconnect will cause even more load to ovsdb-server and cause cascade effect, so northd can never stabilise. We have more than 2000 ports in our lab, and northd could not start before this patch, holding at 100% CPU utilisation both itself and ovsdb-server. After connections are established, any value in northd_probe_interval, or default DEFAULT_PROBE_INTERVAL_MSEC is applied correctly. Signed-off-by: Renat Nurgaliyev --- v2: - Resubmitting patch because git am failed last time v3: - Resubmitting patch because mail client broke formatting v4: - Resubmitting patch because mail client broke formatting once again. I switched to mutt and it won't happen again, I promise guys. v5: - Fixed signoff. --- northd/ovn-northd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 684c2bd47..073c6a0f3 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -100,7 +100,7 @@ static struct eth_addr svc_monitor_mac_ea; /* Default probe interval for NB and SB DB connections. */ #define DEFAULT_PROBE_INTERVAL_MSEC 5000 -static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC; -static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC; +static int northd_probe_interval_nb = 0; +static int northd_probe_interval_sb = 0; #define MAX_OVN_TAGS 4096 -- 2.29.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ovsdb-idl: Return correct seqno from ovsdb_idl_db_set_condition().
If an IDL client sets the same monitor condition twice, the expected
seqno when the IDL contents are updated should be the same for both
calls.
In the following scenario:
1. Client calls ovsdb_idl_db_set_condition(db, table, cond1)
2. ovsdb_idl sends monitor_cond_change(cond1) but the server doesn't yet
reply.
3. Client calls ovsdb_idl_db_set_condition(db, table, cond1)
At step 3 the returned expected seqno should be the same as at step 1.
Similarly, if step 2 is skipped, i.e., the client calls sets
the condition twice in the same iteration, then both
ovsdb_idl_db_set_condition() calls should return the same value.
Fixes: 46437c5232bd ("ovsdb-idl: Enhance conditional monitoring API")
Signed-off-by: Dumitru Ceara
---
lib/ovsdb-idl.c| 9 ++---
tests/test-ovsdb.c | 4
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index fdb9d85..d1fa6ab 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1564,7 +1564,6 @@ ovsdb_idl_db_set_condition(struct ovsdb_idl_db *db,
{
struct ovsdb_idl_condition *table_cond;
struct ovsdb_idl_table *table = ovsdb_idl_db_table_from_class(db, tc);
-unsigned int seqno = db->cond_seqno;
/* Compare the new condition to the last known condition which can be
* either "new" (not sent yet), "requested" or "acked", in this order.
@@ -1582,10 +1581,14 @@ ovsdb_idl_db_set_condition(struct ovsdb_idl_db *db,
ovsdb_idl_condition_clone(&table->new_cond, condition);
db->cond_changed = true;
poll_immediate_wake();
-return seqno + 1;
+return db->cond_seqno + 1;
+} else if (table_cond != table->ack_cond) {
+/* 'condition' was already set but has not been "acked" yet. The IDL
+ * will be up to date when db->cond_seqno gets incremented. */
+return db->cond_seqno + 1;
}
-return seqno;
+return db->cond_seqno;
}
/* Sets the replication condition for 'tc' in 'idl' to 'condition' and
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index b1a4be3..bb9deba 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -2391,6 +2391,10 @@ update_conditions(struct ovsdb_idl *idl, char *commands)
if (seqno == next_seqno ) {
ovs_fatal(0, "condition unchanged");
}
+unsigned int new_next_seqno = ovsdb_idl_set_condition(idl, tc, &cond);
+if (next_seqno != new_next_seqno) {
+ovs_fatal(0, "condition expected seqno changed");
+}
ovsdb_idl_condition_destroy(&cond);
json_destroy(json);
}
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Operaciones e implicaciones legales
Webinar en Vivo: Contabilidad Forense. El control interno como herramienta para evitar fraudes Jueves 26 de Noviembre - Horario de 10:00 a 17:00 Hrs. Identificaremos y conoceremos los puntos finos sobre la contabilidad forense, el control interno y la auditoría como elementos fundamentales para la detección de fraudes, identificación de riesgos y sus Implicaciones en la información financiera, así como sus alcances en la materialidad de las operaciones e implicaciones legales. Objetivos Específicos: - Identificará la importancia del control interno, sus elementos y alcances. - Analizará la importancia de la auditoria del control interno como herramienta fundamental en la detección de fraudes. - Identificará las herramientas de prevención de fraudes. - Estudiará y analizará la contabilidad forense. Para mayor información, responder sobre este correo con la palabra Contabilidad + los siguientes datos: NOMBRE: TELÉFONO: EMPRESA: CORREO ALTERNO: Para información inmediata llamar al: (+52) 55 15 54 66 30 - (+52) 55 30 16 70 85 O puede enviarnos un mensaje vía whatsapp Innova Learn México - innovalearn. mx - Mérida, Yucatán, México ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v11 ovn] Allow to run multiple controllers on the same machine
"
On Tue, Nov 3, 2020 at 4:45 AM Ihar Hrachyshka wrote:
>
> User stories:
> 1) NFV: an admin wants to run two separate instances of OVN controller
>using the same database but configuring ports on different bridges.
>Some of these bridges may use DPDK while others may not.
>
> 2) Parallel OVN instances: an admin wants to run two separate
>instances of OVN controller using different databases. The
>instances are completely independent and serve different consumers.
>For example, the same machine runs both OpenStack and OpenShift
>stacks, each running its own separate OVN stack.
>
> To serve these use cases, several features should be added to
> ovn-controller:
>
> - use different database configuration for multiple controllers;
> - customize chassis name used by controller.
>
> =
>
> For each of the following database configuration options, their
> extended chassis specific counterparts are introduced:
>
> external_ids:hostname
> external_ids:ovn-bridge
> external_ids:ovn-bridge-datapath-type
> external_ids:ovn-bridge-mappings
> external_ids:ovn-chassis-mac-mappings
> external_ids:ovn-cms-options
> external_ids:ovn-encap-csum
> external_ids:ovn-encap-ip
> external_ids:ovn-encap-type
> external_ids:ovn-is-interconn
> external_ids:ovn-monitor-all
> external_ids:ovn-openflow-probe-interval
> external_ids:ovn-remote
> external_ids:ovn-remote-probe-interval
>
> For example,
>
> external_ids:ovn-bridge -> external_ids:ovn-bridge-=
> external_ids:ovn-encap-ip -> external_ids:ovn-encap-ip-=
> external_ids:ovn-remote -> external_ids:ovn-remote-=
>
> Priority wise, specific options take precedence.
>
> =
>
> For system-id,
>
> You can now pass intended chassis name via CLI argument:
>
> $ ovn-controller ... -n
>
> Alternatively, you can configure a chassis name by putting it into the
> ${ovn_sysconfdir}/system-id-override file before running the
> controller.
>
> The latter option may be more useful in container environment where
> the same image may be reused for multiple controller instances, where
> ovs_sysconfigdir/ovn/system-id-override is a volume mounted into this
> generic image. The override file is read once on startup. If you want
> to apply a new chassis name to a controller instance, restart it to
> reread the file.
>
> Priority wise, this is the order in which different means to configure
> the chassis name are used:
>
> - ovn-controller ... -n CLI argument.
> - ${ovs_sysconfdir}/ovn/system-id-override file;
> - external_ids:system-id= ovsdb option;
>
> =
>
> Concurrent chassis running on the same host may inadvertantly remove
> patch ports that belong to their peer chassis. To avoid that, patch
> ports are now tagged in external-ids:ovn-chassis-id with the
> appropriate chassis name, and only patch ports that belong to the
> chassis are touched when cleaning up. Also, now only tunnels on the
> active integration bridge are being cleaned up.
>
> Note that external-ids:ovn-chassis-id key is already used for tunnel
> ports to identify the remote tunnel endpoint. We can reuse the same
> key for patch ports because the key usage is not overlapping.
>
> Alternatively, we could introduce a new key with a similar but
> different name. This would simplify code changes needed but would
> arguably introduce even more confusion. Since the key name is not
> entirely self-descriptive for tunnel ports (a better name would be
> e.g. ovn-remote-chassis or ovn-peer-chassis), the ideal scenario would
> be to rename the key for tunnel endpoints but reuse it for patch
> ports. This would involve additional migration steps and is probably
> not worth the hassle.
>
> =
>
> This patch also expands tunnel port name to allow for long similar
> chassis names used on the same host.
>
> =
>
> Note: this patch assumes that each chassis has its own unique IP.
> Future work may consider adding support to specify custom port numbers
> for tunneling that would allow to reuse the same IP address for
> multiple chassis running on the same host. This work is out of scope
> for this patch.
>
> Signed-off-by: Ihar Hrachyshka
>
> ---
>
> v1: initial implementation.
> v2: fixed test case to check ports are claimed by proper chassis.
> v2: added NEWS entry.
> v2: fixed some compiler warnings.
> v2: moved file_system_id declaration inside a function that uses it.
> v2: removed unneeded binding.h #include.
> v2: docs: better explanation of alternatives to select chassis name.
> v3: reverted priority order for chassis configuration: first CLI, then
> system-id file, then ovsdb.
> v4: introduce helpers to extract external-ids (per-chassis or global).
> v4: introduce per-chassis config options for all keys.
> v4: introduce -M (--concurrent) CLI argument to avoid patch ports
> removed by concurrent chassis.
> v5: rebased.
> v6: switched from -M (--concurrent) to external-ids:ovn-is-concurrent.
> v6: with ovn-is-concurrent=true, also avoid removing unknown tunnel
> endpoints.
> v7: don't
Re: [ovs-dev] OVN-OVS build compatibility, take 2
On 11/9/20 11:06 PM, Mark Michelson wrote: > On 11/9/20 8:03 AM, Ilya Maximets wrote: >> On 10/23/20 8:56 PM, Mark Michelson wrote: >>> On 10/23/20 8:42 AM, Brian Haley wrote: On 10/22/20 3:31 PM, Mark Michelson wrote: > Hi, > > In today's OVN meeting [1], Numan brought up that he had proposed an OVN > patch [2] that deals with a compilation error that occurred after > updating to the latest OVS master. This sparked a discussion about the > process behind OVN/OVS build compatibility. > > After OVN was split from OVS last year, the attitude with regard to > build compatibility was that > > 1) Only devs are likely to be building OVN, so building against the > latest and greatest OVS should be acceptable. > 2) Since OVN links to OVS's libraries statically, it's fine if the > version of OVS used to build OVS is different from the version of OVS > that OVN runs against. > > After nearly a year of having OVN separated, we've come to the > realization that this may not be the best way to do things. Reasons why > include > > 1) The "latest and greatest" OVS could actually be a very unstable > mid-version build of OVS. Since OVN is released more often than OVS, > this necessitates OVN releases being built against an unstable version > of OVS. > 2) Debugging OVN problems that are rooted in OVSDB or OVS libraries can > be tremendously difficult. Bisecting OVN commits likely requires > changing the OVS commit to build against. This effectively gives two > moving targets for tracking the issue. > 3) OVN includes "non-public" headers in OVS. > > Based on the meeting today, proposed ideas for fixing this are > > 1) Move headers from ovs's lib/ folder to include/ since they are > consumed by OVN, an external program. > 2) Anchor OVN builds on a specific release of OVS rather than just using > the latest OVS release. This is what we do in Openstack Neutron. For example, we have two gate jobs - one using released code, the other using the tip of the master branch. It can take either a branch, tag, or commit, but it's currently: OVN_BRANCH: v20.06.1 # TODO(jlibosva): v2.13.1 is incompatible with kernel 4.15.0-118, sticking to commit hash until new v2.13 tag is created OVS_BRANCH: 0047ca3a0290f1ef954f2c76b31477cf4b9755f5 and master: OVN_BRANCH: master OVS_BRANCH: master Doing something like this would let you pick a point in time, and allow the user to override if they wish, like if the OS/kernel in question had an issue (as shown above). It also keeps OVN out of keeping a copy if the OVS tree. My $.02 -Brian >>> >>> Thanks Brian, >>> >>> During our meeting we discussed something similar. If I understand your >>> proposal correctly, the problem is that we won't have as much control over >>> which changes in OVS we consume. For instance, if we start by anchoring OVN >>> development to OVS at commit A, we may find a month later that there is a >>> bug we need to fix. In the meantime, commits B, C, D, and E have gone into >>> OVS. So when we merge our bug fix, we are doing so as commit F. If we then >>> point OVN to commit F of OVS, then we are also consuming commits B, C, D, >>> and E, which we don't want. By cherry-picking commit F to a separate >>> branch, we know that we are only getting commit F on top of commit A. >> >> There is one big problem with this way of handling things. >> Assuming that all commits (B, C, D, E and F) requires code changes in OVN >> for it to be compiled successfully. Assuming that we cloned OVS at commit >> A to a special branch. Now we're porting commit F without commits B-E. >> At this point this special branch is the only version of OVS that could >> be used to build OVN, i.e. there is no any upstream OVS version that could >> be used to build OVN. >> In this case, we will have to maintain a special OVS branch for each OVN >> release or store somewhere the commit hash of OVS (from this special branch) >> per OVN release. That is exactly same problem that we have right now except >> that we also need to maintain additional OVS branches. >> >> I think, that having OVS as a git submodule should make life easier, as there >> will be only one version of OVS used to build OVN for every moment in time >> and users (people, who tries to build OVN) will not need to think about which >> version of OVS to build with. Submodule version shift might be considered >> while implementing new features or before releasing a new version (or new >> stable version) of OVN to get bug fixes. > > I'm not that familiar with git submodules, but does a submodule allow for > easily building different versions of OVN against specific version of OVS? As > an easy example, let's say that with OVN 20.12 we want to u
Re: [ovs-dev] HIGH cost performance casters and accessories factory and OEM provider
Dear Customer, I am Robin of Shangxin Caster China Manufacturer. At the moment, medical casters, medical carts, air cargo casters and leveling casters are hotly sold which we hope they can do really help those who need in the special period. Are you interested to know more? You can also find us in google by click our company name below signature.Let's talk details. Best regards, Robin Cheu Marketing Manager Ningbo Shangxin Caster Wheels Co.,Ltd No.187, Langxia street, Yuyao city, Ningbo, Zhejiang, China. Tel. +86 0574-62188161 Mob. +86 152 5836 8162 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v2] ovn.at: Make some of the tests more predictable.
On Tue, Nov 10, 2020 at 10:37:34AM +0100, Dumitru Ceara wrote: > Fix some of the race conditions that are present in the current OVN test > suite. > > Signed-off-by: Dumitru Ceara > --- > v2: > - Fix the sed expression as suggested by Ben. Thanks a lot! I applied this to OVN master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3 4/6] tests: Eliminate most "sleep" calls.
On Tue, Nov 10, 2020 at 09:47:56AM +0100, Dumitru Ceara wrote: > On 11/10/20 1:26 AM, Ben Pfaff wrote: > > On Mon, Nov 09, 2020 at 02:44:27PM +0100, Dumitru Ceara wrote: > >> On 11/6/20 4:36 AM, Ben Pfaff wrote: > >>> Many of these could be replaced by "ovn-nbctl sync". Some weren't > >>> really needed at all because they were adjacent to something that itself > >>> called sync or otherwise used --wait. Some were more appropriately > >>> done with explicit waits for what was really needed. > >>> > >>> I left some "sleep"s. Some were because they were "negative" sleeps: > >>> they were giving time for something to happen that should *not* happen > >>> (in other words, if you wait for it to happen, you'll wait forever, > >>> unless there's a bug). Some were because I didn't know how to properly > >>> wait for what they were waiting for, or because I didn't understand > >>> the circumstances deeply enough. > >>> > >>> Signed-off-by: Ben Pfaff > >>> --- > >>> tests/ovn-northd.at | 7 ++ > >>> tests/ovn.at| 167 > >>> 2 files changed, 52 insertions(+), 122 deletions(-) > >>> > >>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > >>> index 949a8eee054e..5d670233561e 100644 > >>> --- a/tests/ovn-northd.at > >>> +++ b/tests/ovn-northd.at > >>> @@ -1120,6 +1120,7 @@ ovn-nbctl --wait=sb -- --id=@hc create \ > >>> Load_Balancer_Health_Check vip="10.0.0.10\:80" -- add Load_Balancer . \ > >>> health_check @hc > >>> wait_row_count Service_Monitor 2 > >>> +check ovn-nbctl --wait=hv sync > >> > >> This should be "--wait=sb". ovn-controller is not started for tests in > >> ovn-northd.at and "--wait=hv" would return immediately. This applies to > >> all "ovn-nbctl --wait=hv sync" in ovn-northd.at. > > > > I see what you mean. > > > > This is surprising to me. When I defined this stuff years ago, my > > intent was that --wait=hv would be stronger than --wait=sb. That is, > > --wait=sb would cause ovn-nbctl to wait for the southbound database to > > catch up with the northbound changes, and --wait=hb would cause it to > > wait for the southbound database AND the hypervisors to catch up. > > > > That's even how it's documented for ovn-nbctl: > > > > > > With --wait=sb, before ovn-nbctl exits, > > it > > waits for ovn-northd to bring the southbound database > > up-to-date with the northbound database updates. > > > > > > > > With --wait=hv, before ovn-nbctl exits, > > it > > additionally waits for all OVN chassis (hypervisors and gateways) > > to > > become up-to-date with the northbound database updates. (This can > > become an indefinite wait if any chassis is malfunctioning.) > > > > > > That's not what actually happens, though. As you point out, if there > > are no hypervisors, then the hypervisors are always up-to-date, even if > > the database is not. > > > > I think this is a bug in ovn-nbctl. Arguably, it's a bug in the > > database definition (perhaps hv_cfg shouldn't even be filled in but left > > empty if there are no chassis) but I think it is too late to fix it > > there. It is, however, easy enough to fix it in ovn-nbctl. > > > > (And at the same time, I'll change these in ovn-northd to just say > > --wait=sb. You have a point.) > > > > How about this as an additional patch? > > > > -8<--cut here-->8-- > > > > From 7b373c22dbda2f808f3d5f3b8ae6eb67612dbe87 Mon Sep 17 00:00:00 2001 > > From: Ben Pfaff > > Date: Mon, 9 Nov 2020 16:12:50 -0800 > > Subject: [PATCH ovn] ovn-nbctl: Make --wait=hv wait for southbound database > > in > > corner case. > > > > In the corner case where there are no chassis, --wait=hv didn't wait at > > all, instead of waiting for the southbound database. > > > > Reported-by: Dumitru Ceara > > Signed-off-by: Ben Pfaff > > --- > > ovn-nb.xml| 27 --- > > utilities/ovn-nbctl.c | 2 +- > > 2 files changed, 21 insertions(+), 8 deletions(-) > > > > diff --git a/ovn-nb.xml b/ovn-nb.xml > > index 51b186b84419..d3e51f3e5e87 100644 > > --- a/ovn-nb.xml > > +++ b/ovn-nb.xml > > @@ -84,13 +84,26 @@ > > > > > > > > -Sequence number that ovn-northd sets to the smallest > > -sequence number of all the chassis in the system, as reported in > > the > > -Chassis_Private table in the southbound database. > > Thus, > > - equals if all > > chassis are > > -caught up with the northbound configuration (which may never > > happen, if > > -any chassis is down). This value can regress, if a chassis was > > removed > > -from the system and rejoins before catching up. > > + > > + Sequence number that ovn-northd sets to the smallest > > + sequence number of all the chassis in the system, as reported in > > the > > +
Re: [ovs-dev] ddlog for ACL log meters (was: [PATCH v3 ovn 1/1] northd: Enhance the implementation of ACL log meters.)
On Mon, Nov 09, 2020 at 09:53:16AM -0500, Flavio Fernandes wrote: > > On Nov 7, 2020, at 4:39 PM, Ben Pfaff wrote: > > On Tue, Nov 03, 2020 at 10:18:34PM +, Flavio Fernandes wrote: > >> When multiple ACL rows use the same Meter for log rate-limiting, the > >> 'noisiest' ACL matches may completely consume the Meter and shadow other > >> ACL logs. This patch introduces an option in NB_Global that allows for a > >> Meter to rate-limit each ACL log separately. > > > > I'm not sure I like the overall design here. This option isn't going to > > scale very well to many of these meters, since they'd all need to be > > shoved into a single comma-separated list. Another way might be to add > > a column to the ACL table to mark a meter as shared or private. Or > > there could be a name convention, e.g. prefix with "shared_" to make it > > shared. > > Understood. I think I tried "too hard" to avoid making changes to the > northbound schema, but you bring a valid concern about > scalability. Adding a "bool" to the ACL row, or to the Meter row would > definitely make this more scalable. I would like to ask for opinion > from you on the following choices (not listed in any order). You == > Ben and the rest of the ovn core developers and users. > > A) add a bool column to ACL row called "fair_log_meter" (please help me with > the name); > B) add a bool column to Meter row called ^same as above^; > C) change parsing of the value of the global to allow for up to one Meter name > D) change parsing of the value of the global to allow for up to a constant > Meter names > E) have an implict behavior based on the name of the Meter "shared_", so that > multiple meters are created in the SB as needed. > F) same as 'E', but use a different prefix str > G) any other good approach? I think that A and B are reasonable approaches. I don't understand the issue well enough to know whether this property has more to do with the ACL or with the Meter, so I'm not sure where it better to put it. I don't think C or D makes sense, because I don't know a reason why there would be only one of these meter or only a small number of these meters. I think that E or F would only make sense if there was some reason A or B would not work. OK, let's step back here and consider G. Why do we need a new southbound Meter row for each instance? That's a bit of a waste, isn't it? Why can't we just add an indicator to the southbound Meter that says "each reference to me is unique"? Or a new argument to the southbound log() action that distinguishes references to a given meter, so that identical values get the same one and different values get distinct ones? > > I don't really understand the vocabulary here, either. I would have > > guessed that "shared" would mean that all the ACLs with a given meter > > name share a single southbound Meter, but it's the opposite: a "shared" > > meter has a different southbound Meter for each ACL. > > Yes, good point. I was seeing this change mostly from the Northbound > perspective, but you are right that this is a confusing name looking > at what happens at the SB. How about we use the word "fair" instead? > So, something like "acl_fair_log_meters". Any suggestions on a better > name are very welcome. "fair" seems OK to me. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ovn-sb: Fix speling error in documentation.
Signed-off-by: Ben Pfaff --- ovn-sb.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ovn-sb.xml b/ovn-sb.xml index b1480f218635..9a49501cb0b9 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -2068,7 +2068,7 @@ The string should reference a entry from the table. The only meter - that is appriopriate + that is appropriate is drop. -- 2.26.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] ddlog for ACL log meters (was: [PATCH v3 ovn 1/1] northd: Enhance the implementation of ACL log meters.)
[cc Dimitru, Numan, MarkM] > On Nov 10, 2020, at 2:15 PM, Ben Pfaff wrote: > > On Mon, Nov 09, 2020 at 09:53:16AM -0500, Flavio Fernandes wrote: >>> On Nov 7, 2020, at 4:39 PM, Ben Pfaff wrote: >>> On Tue, Nov 03, 2020 at 10:18:34PM +, Flavio Fernandes wrote: When multiple ACL rows use the same Meter for log rate-limiting, the 'noisiest' ACL matches may completely consume the Meter and shadow other ACL logs. This patch introduces an option in NB_Global that allows for a Meter to rate-limit each ACL log separately. >>> >>> I'm not sure I like the overall design here. This option isn't going to >>> scale very well to many of these meters, since they'd all need to be >>> shoved into a single comma-separated list. Another way might be to add >>> a column to the ACL table to mark a meter as shared or private. Or >>> there could be a name convention, e.g. prefix with "shared_" to make it >>> shared. >> >> Understood. I think I tried "too hard" to avoid making changes to the >> northbound schema, but you bring a valid concern about >> scalability. Adding a "bool" to the ACL row, or to the Meter row would >> definitely make this more scalable. I would like to ask for opinion >> from you on the following choices (not listed in any order). You == >> Ben and the rest of the ovn core developers and users. >> >> A) add a bool column to ACL row called "fair_log_meter" (please help me with >> the name); >> B) add a bool column to Meter row called ^same as above^; >> C) change parsing of the value of the global to allow for up to one Meter >> name >> D) change parsing of the value of the global to allow for up to a constant >> Meter names >> E) have an implict behavior based on the name of the Meter "shared_", so >> that multiple meters are created in the SB as needed. >> F) same as 'E', but use a different prefix str >> G) any other good approach? > > I think that A and B are reasonable approaches. I don't understand the > issue well enough to know whether this property has more to do with the > ACL or with the Meter, so I'm not sure where it better to put it. Let's pursue the schema change, then! Funny thing is that my very initial idea [1] was 'A'. I can see how 'B' could benefit non-ACL-log related functionality in the NB that refers to Meters, but I think that is overkill. I ended up hesitant in changing the schema after talking with my OVN guru Dumitru. I will follow up with him on this idea to see how strongly he feels about it. > I don't think C or D makes sense, because I don't know a reason why > there would be only one of these meter or only a small number of these > meters. Option C (not the language) is likely too restrictive and D would still feel like a leash. A small number of 'fair' Meters would be all that ACL logs needed. If many ACL logs end up having different Meter needs, then having a 1:1 mapping between ACLs and Meter rows would be the answer. And that is what we already have the ability to do today. So, I cannot imagine a usage case where we would have more than a couple of values for the NB_global. The potential for scaling troubles does not give me a good felling, so I'm back to 'A'. ;) > > I think that E or F would only make sense if there was some reason A or > B would not work. ++ > > OK, let's step back here and consider G. Why do we need a new > southbound Meter row for each instance? That's a bit of a waste, isn't > it? Why can't we just add an indicator to the southbound Meter that > says "each reference to me is unique"? Or a new argument to the > southbound log() action that distinguishes references to a given meter, > so that identical values get the same one and different values get > distinct ones? Hmm That sounds really good, actually. We would still need 'A' as a way to propagate that desire from the CMS' perspective, agree? I must confess that the wasteful approach of rows in the SB comes from my limited knowledge on how to efficiently use the log action to do what you described. Also, because I was hoping to solve the whole problem within northd. If I understand you correctly, option 'G' you mention here would require changes in the SB schema as well as in ovn-controller? I will definitely need some pointers to make that happen. Wanna be my partner in crime? :) No pressure. > >>> I don't really understand the vocabulary here, either. I would have >>> guessed that "shared" would mean that all the ACLs with a given meter >>> name share a single southbound Meter, but it's the opposite: a "shared" >>> meter has a different southbound Meter for each ACL. >> >> Yes, good point. I was seeing this change mostly from the Northbound >> perspective, but you are right that this is a confusing name looking >> at what happens at the SB. How about we use the word "fair" instead? >> So, something like "acl_fair_log_meters". Any suggestions on a better >> name are very welcome. > > "fair" seems OK to me. > ++ Let me ta
Re: [ovs-dev] [PATCH ovn v3 6/6] ovn-northd-ddlog: New implementation of ovn-northd based on ddlog.
On Mon, Nov 09, 2020 at 02:44:53PM +0100, Dumitru Ceara wrote: > There are a few others that are ddlog-related: > > 145: ovn -- /32 router IP address -- ovn-northd-ddlog FAILED > (ovs-macros.at:231) > > This is because the following logical flows are not added to the SB by > ovn-northd-ddlog: > table=14(ls_in_arp_rsp ), priority=100 , match=(arp.tpa == > 10.0.0.2 && arp.op == 1 && inport == "alice1"), action=(next;) > table=14(ls_in_arp_rsp ), priority=50 , match=(arp.tpa == > 10.0.0.2 && arp.op == 1), action=(eth.dst = eth.src; eth.src = > f0:00:00:01:02:04; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; > arp.sha = f0:00:00:01:02:04; arp.tpa = arp.spa; arp.spa = 10.0.0.2; > outport = inport; flags.loopback = 1; output;) Hmm, I don't see this failure, and the diff I see between the flow tables is empty if I filter uuids and drop the inconsistent use of /32 suffixes, e.g.: diff -u <(uuidfilt tests/testsuite.dir/144/sbflows | sed 's,/32,,g') <(uuidfilt tests/testsuite.dir/145/sbflows | sed 's,/32,,g') I wonder why we see something different, is this racy for you? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] I noticed that you are still paying those high 2.5% Credit Card Processing Fees every month! Not anymore. You have options.
Ons rekening fransche rivieren allerlei verbrand tot behouden. Gezocht goa wolfram den vreemde grooter. Anderhalf in bezwarend er in nutteloos vernielen prachtige. Holte japan dik dagen wijze ook zware hun. Bouw dit ton een dekt mont. Wellesley wegwerpen dus stoompomp aangelegd van. De ze omdat zulke gayah nu te onder perak zeker. Weg gestookt engeland atjehers had boringen zou voorzorg een. Valorem ons wat stijgen dollars krijgen. Dit deed dier het liet open meer jaar. Vorming en meestal de stroeve er na gebruik plantte invloed. Van goud hun ziet goed kern kuil duim. Onder maken werkt te dagen af waren tegen. Ingewanden nu in op al kooplieden caoutchouc. Dragen spuwen wascht gezond in en de na. Aangewend verdiende opgericht opgevoerd of in ik schippers. Elk hij inboorling georgetown ingewanden als districten ton. Kinta anson kreeg wijze heb zes groei. Weg bijgang als gemengd gezocht. Nog lot francs nieuwe kedona. Dieper tonnen zij beslag gif. Zoo beschikken mee wetenschap ondernemer kilometers een. Eenig ieder ouder ad bezit allen te er op. Die ons ten zaken wegen negri nog. Per zijn dit doet iets. Gold dank en al heen. Huwelijken nu al verwachten verbazende. Hen bezig wij leven uit aan japan. Een tengka zij jammer vinden pusing zes. Zaken stuit welks in is bezet. Ter eveneens gezegend afkoopen welvaart wij meesters weg met veteraan. Omwonden in staatjes gesloten onnoodig verschil er de. Ijzererts ze evenwicht hoofdzaak af producten bovenkant. Lange de beste te in raakt. Zoo had wilde des heeft langs. Af verdiende alluviale arabische arbeiders om. Parasiet bestuurd lot tusschen dan deeltjes. Vruchtbaar ingewanden om kooplieden na ontwouding. Nemen nu bezet en assam. Zeker steun ze rijst en en zijde perak op. Zilver leener in de slecht ziekte. Aanleiding inlandsche wantrouwen of af ik feestdagen uitgevoerd te. Na en om ongunstig nu honderden chineesch arabieren voorschot. Van rug gebracht hectaren eromheen grootste ten vestigen schatten. Talrijk planter zoo gewicht nam stroeve zwijnen brengen. Lage heen zoo van gold. Enkele zou aaneen tonnen vloeit dik bakjes vooral wel. Af en planten er chinees duizend zwijnen tapioca enkelen. Negritos ook dag plantage verbruik voorziet verhoogd een den strooien. Bezet op holen de anson geest holte bevel er. Engelsche is ze behandeld vereenigd ze om. Ad nacht geest geval en dient in. Dik aldus wegen wezen naast telde den tot zoo. Handelaars ten gomsoorten ondernomen agentschap aanleiding dal heb natuurlijk. Genomen van tot gropeng opweegt. Ingenieur resideert ten die stichting tot wat. Thee laag goa over was maal tot den. Na de buitendien ingenieurs gewoonlijk er. Ook gambir dik met gronds wij woords jammer buizen. Kaal wat diep zout hier oog alle. De gedaan na af staten openen. Eischen geoogst heuvels haalden markten zal was aan. Voorzien overgaat atjehers te in. Waren mag ieder naast ficus liput rijke heb. Vordert gebruik daartoe zit zal zin systeem. Met invoer schuld pijlen ver vierde. Wel zit maar vier rang deed over. Dergelijke dik tembunmijn agentschap belangrijk plotseling het. In open nu en al zich jaar. Nam tooverslag met als bevaarbaar mogendheid ondernomen. Luister javanen om ad leveren krijgen. Dieper afzien aan sterke are gezift herten gif. Kongostaat verwachten middellijn gas ongebruikt feestdagen buitendien mee. Koopman genomen javanen ik nu ontdekt op menigte. Bakken europa tunnel invoer zullen dat kamper zoo. Leveren zwijnen hoogere duivels sombere bak van dik lot. Uit stam hier aan acre zin. Schuld forten zeggen elk bij minste jungle leggen een. Dragen aan bouwde lappen loopen rubben ceylon nog. De afkoopen ad vreemden er af scheppen gewonnen. Ik bouw werd kant te er geur. Ik arabische in belasting al chineesch in belovende. Nu deel arme thee in land weer ze doet al. Koopman laatste ormoezd cultuur bontste is scholen nu om ad. Een per bekoeld bersawa tijdens bronnen ver woonden voordat wat. Bezet zelve de erbij ze in werkt meest of. Op gomsoorten uitgevoerd is bescheiden geruineerd. Reden dus ook cenis nam geven. Regentijd ingenieur wassching zit scheidden als gesmolten weerstand. Nam een bescheiden dan gunstigste dus archimedes verlichten huwelijken ontginning. Mier naar doet vaak ader ook het zulk zoo. Dreigt weg kunnen hij tonnen openen wie alleen. De ze bakje omdat zware nu assam naast. Hielden goa men wel geplant wij ploegen. Bij lot het doorzoeken rug kwartslaag ondernemer. Scheidden mineralen hij kapitalen dat oog. Rente geest ik roode na staan. Ontsnappen en ze en dweepzieke woekeraars nu europeesch. Af nadering en tusschen dichtbij sembilan. Ormoezd van vreemde ten opnieuw regelen een. Gomboomen opgericht bijgeloof wie hij zes. Talrijke te doodende tusschen al gesloten en er getracht gestookt. Toeneming resideert is binnenste in te terreinen ze vervangen. Slikban
[ovs-dev] [PATCH ovn] actions: Fix issues with fwd_group action.
The example for the fwd_group action was obviously wrong (it needed a
closing parenthesis and semicolon) and then when I looked closer I saw
other issues:
- It's weird to insist on "true" or "false" in quotation marks (why?)
and the example didn't show that.
- The example didn't show that port names need to be quoted.
- The implementation didn't allow specifying liveness=false (why?)
or liveness="false" and the test even checked for that.
This fixes all that stuff. For backward compatibility, it still accepts
"true" (and "false") in quotation marks, but now accepts without as well
for future use.
Signed-off-by: Ben Pfaff
---
include/ovn/lex.h | 1 +
lib/actions.c | 14 --
lib/lex.c | 13 +
ovn-sb.xml| 21 -
tests/ovn.at | 14 +++---
5 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/include/ovn/lex.h b/include/ovn/lex.h
index 1da6ccc8c021..ecb7ace24326 100644
--- a/include/ovn/lex.h
+++ b/include/ovn/lex.h
@@ -137,6 +137,7 @@ enum lex_type lexer_lookahead(const struct lexer *);
bool lexer_match(struct lexer *, enum lex_type);
bool lexer_force_match(struct lexer *, enum lex_type);
bool lexer_match_id(struct lexer *, const char *id);
+bool lexer_match_string(struct lexer *, const char *s);
bool lexer_is_int(const struct lexer *);
bool lexer_get_int(struct lexer *, int *value);
bool lexer_force_int(struct lexer *, int *value);
diff --git a/lib/actions.c b/lib/actions.c
index 23e54ef2a6c4..6300fef2c48e 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -3344,15 +3344,17 @@ parse_fwd_group_action(struct action_context *ctx)
if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
return;
}
-if (ctx->lexer->token.type != LEX_T_STRING) {
+if (lexer_match_string(ctx->lexer, "true") ||
+lexer_match_id(ctx->lexer, "true")) {
+liveness = true;
+} else if (lexer_match_string(ctx->lexer, "false") ||
+ lexer_match_id(ctx->lexer, "false")) {
+liveness = false;
+} else {
lexer_syntax_error(ctx->lexer,
- "expecting true/false");
+ "expecting true or false");
return;
}
-if (!strcmp(ctx->lexer->token.s, "true")) {
-liveness = true;
-lexer_get(ctx->lexer);
-}
lexer_force_match(ctx->lexer, LEX_T_COMMA);
}
if (lexer_match_id(ctx->lexer, "childports")) {
diff --git a/lib/lex.c b/lib/lex.c
index 4d921998877e..c84d52aa8d1d 100644
--- a/lib/lex.c
+++ b/lib/lex.c
@@ -906,6 +906,19 @@ lexer_match_id(struct lexer *lexer, const char *id)
}
}
+/* If 'lexer''s current token is the string 's', advances 'lexer' to the next
+ * token and returns true. Otherwise returns false. */
+bool
+lexer_match_string(struct lexer *lexer, const char *s)
+{
+if (lexer->token.type == LEX_T_STRING && !strcmp(lexer->token.s, s)) {
+lexer_get(lexer);
+return true;
+} else {
+return false;
+}
+}
+
bool
lexer_is_int(const struct lexer *lexer)
{
diff --git a/ovn-sb.xml b/ovn-sb.xml
index b1480f218635..482df00777f3 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -2074,23 +2074,26 @@
-fwd_group(P);
+fwd_group(liveness=bool,
childports=port, ...);
-Parameters: liveness, list of child ports P.
+Parameters: optional liveness, either
+true or false, defaulting to false;
+childports, a comma-delimited list of strings denoting
+logical ports to load balance across.
-It load balances traffic to one or more child ports in a
-logical switch. ovn-controller translates the
-fwd_group into openflow group with one bucket
-for each child port. If liveness is set to true, it also
-integrates the bucket selection with BFD status on the tunnel
+Load balance traffic to one or more child ports in a logical
+switch. ovn-controller translates the
+fwd_group into an OpenFlow group with one bucket for
+each child port. If liveness=true is specified, it
+also integrates the bucket selection with BFD status on the tunnel
interface corresponding to child port.
- Example: fwd_group(liveness=true, childports=p1,p2
-
+ Example: fwd_group(liveness=true, childports="p1",
+ "p2");
diff --git a/tests/ovn.at b/tests/ovn.at
index 9c23ae73ad86..bc731fefc47a 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1695,7 +1695,8 @@ ip.proto = select(1, 2, 3);
reg0[0..14] = select(1, 2, 3);
cannot use 15-bit field reg0[0..14] for "select", whic
[ovs-dev] [PATCH ovn v4 1/8] Export `VLOG_WARN` and `VLOG_ERR` from libovn for use in ddlog
From: Leonid Ryzhyk
Export `ddlog_warn` and `ddlog_err` functions that are just wrappers
around `VLOG_WARN` and `VLOG_ERR`. This is not ideal because the
functions are exported by `ovn_util.c` and the resulting log messages use
`ovn_util` as module name. More importantly, these functions do not do
log rate limiting.
Signed-off-by: Leonid Ryzhyk
Signed-off-by: Ben Pfaff
---
lib/ovn-util.c | 17 +
lib/ovn-util.h | 6 ++
2 files changed, 23 insertions(+)
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index abe6b04a7701..eb4f14efffa6 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -722,3 +722,20 @@ ip_address_and_port_from_lb_key(const char *key, char
**ip_address,
*addr_family = ss.ss_family;
return true;
}
+
+#ifdef DDLOG
+
+/* Callbacks used by the ddlog northd code to print warnings and errors.
+ */
+void
+ddlog_warn(const char *msg)
+{
+VLOG_WARN("%s", msg);
+}
+
+void
+ddlog_err(const char *msg)
+{
+VLOG_ERR("%s", msg);
+}
+#endif
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index a39cbef5a47e..77d0936a5fbc 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -230,4 +230,10 @@ char *str_tolower(const char *orig);
bool ip_address_and_port_from_lb_key(const char *key, char **ip_address,
uint16_t *port, int *addr_family);
+#ifdef DDLOG
+void ddlog_warn(const char *msg);
+void ddlog_err(const char *msg);
+#endif
+
+
#endif
--
2.26.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v4 0/8] Add DDlog implementation of ovn-northd
v1->v2 (thanks Numan!):
- Applied several patches.
- New commit to add documentation for the system-userspace testsuite.
- New patches to improve the testsuite a couple of ways.
- Revised "Prepare for multiple northd types" to fix system-userspace
testsuite.
- Updated DDlog implementation of northd to match latest master.
- Updated copyright notices in DDlog implementation.
v2->v3:
- Applied documentation patch.
- Added some test improvements to fix reported problems.
- Really updated copyright notices (didn't commit, last time).
- Correctly skip ddlog tests when ddlog not compiled.
v3->v4:
- Fix dependencies for parallel build.
- Fix spelling error in documentation.
- Use --wait=sb, not --wait=hv, when no chassis are running.
- Fixed IGMP and MLD tests by porting commit 9d2e8d32fb98
("ofctrl.c: Fix duplicated flow handling in I-P while merging
opposite changes."), which I had missed before.
Ben Pfaff (5):
tests: Prepare for multiple northd types.
tests: Use portable "test a = b", not "test a == b".
tests: Eliminate most "sleep" calls.
tests: Improve debuggability of tests.
Fix IGMP and MLD tests.
Flavio Fernandes (1):
northd: Enhance the implementation of ACL log meters.
Leonid Ryzhyk (2):
Export `VLOG_WARN` and `VLOG_ERR` from libovn for use in ddlog
ovn-northd-ddlog: New implementation of ovn-northd based on ddlog.
Documentation/automake.mk |2 +
Documentation/intro/install/general.rst | 31 +-
Documentation/topics/debugging-ddlog.rst | 280 +
Documentation/topics/index.rst|1 +
Documentation/tutorials/ddlog-new-feature.rst | 362 +
Documentation/tutorials/index.rst |1 +
NEWS |6 +
acinclude.m4 | 43 +
configure.ac |5 +
lib/ovn-util.c| 17 +
lib/ovn-util.h|6 +
m4/ovn.m4 | 16 +
northd/.gitignore |4 +
northd/automake.mk| 104 +
northd/helpers.dl | 128 +
northd/ipam.dl| 506 ++
northd/lrouter.dl | 715 ++
northd/lswitch.dl | 638 ++
northd/multicast.dl | 263 +
northd/ovn-nb.dlopts | 13 +
northd/ovn-northd-ddlog.c | 1752
northd/ovn-northd.c | 201 +-
northd/ovn-sb.dlopts | 28 +
northd/ovn.dl | 387 +
northd/ovn.rs | 857 ++
northd/ovn.toml |2 +
northd/ovn_northd.dl | 7528 +
northd/ovsdb2ddlog2c | 127 +
ovn-nb.xml| 14 +
tests/atlocal.in |7 +
tests/ovn-controller-vtep.at |6 +-
tests/ovn-ic.at | 11 +-
tests/ovn-macros.at | 99 +-
tests/ovn-northd.at | 485 +-
tests/ovn.at | 819 +-
tests/ovs-macros.at | 43 +-
tests/system-ovn.at | 124 +-
tutorial/ovs-sandbox | 24 +-
utilities/checkpatch.py |2 +-
utilities/ovn-ctl | 20 +-
40 files changed, 15046 insertions(+), 631 deletions(-)
create mode 100644 Documentation/topics/debugging-ddlog.rst
create mode 100644 Documentation/tutorials/ddlog-new-feature.rst
create mode 100644 northd/helpers.dl
create mode 100644 northd/ipam.dl
create mode 100644 northd/lrouter.dl
create mode 100644 northd/lswitch.dl
create mode 100644 northd/multicast.dl
create mode 100644 northd/ovn-nb.dlopts
create mode 100644 northd/ovn-northd-ddlog.c
create mode 100644 northd/ovn-sb.dlopts
create mode 100644 northd/ovn.dl
create mode 100644 northd/ovn.rs
create mode 100644 northd/ovn.toml
create mode 100644 northd/ovn_northd.dl
create mode 100755 northd/ovsdb2ddlog2c
--
2.26.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v4 2/8] tests: Prepare for multiple northd types.
The idea is to run each test twice, once with ovn-northd, once
with ovn-northd-ddlog. To do that, we add a macro OVN_FOR_EACH_NORTHD
and bracket each test that uses ovn-northd in it.
Signed-off-by: Ben Pfaff
---
tests/ovn-ic.at | 11 +-
tests/ovn-macros.at | 96 ++--
tests/ovn-northd.at | 161 ++-
tests/ovn.at| 264 +---
tests/ovs-macros.at | 20 ++--
tests/system-ovn.at | 122 +++-
6 files changed, 533 insertions(+), 141 deletions(-)
diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
index 0638af401295..2a4fba031f36 100644
--- a/tests/ovn-ic.at
+++ b/tests/ovn-ic.at
@@ -1,4 +1,5 @@
AT_BANNER([OVN Interconnection Controller])
+OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- AZ register])
ovn_init_ic_db
@@ -29,7 +30,9 @@ availability-zone az3
OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
+])
+OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- transit switch handling])
ovn_init_ic_db
@@ -59,7 +62,9 @@ check_column ts2 nb:Logical_Switch name
OVN_CLEANUP_IC([az1])
AT_CLEANUP
+])
+OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- gateway sync])
ovn_init_ic_db
@@ -120,8 +125,9 @@ OVN_CLEANUP_SBOX(gw2)
OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
+])
-
+OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- port sync])
ovn_init_ic_db
@@ -185,7 +191,9 @@ OVN_CLEANUP_SBOX(gw1)
OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
+])
+OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- route sync])
ovn_init_ic_db
@@ -310,3 +318,4 @@ OVS_WAIT_WHILE([ovn_as az1 ovn-nbctl lr-route-list lr1 |
grep learned | grep 10.
OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
+])
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 5b9c2dee6812..105ad0ab7f7f 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -47,10 +47,12 @@ m4_define([OVN_CLEANUP],[
OVS_APP_EXIT_AND_WAIT([ovsdb-server])
as northd
-OVS_APP_EXIT_AND_WAIT([ovn-northd])
+OVS_APP_EXIT_AND_WAIT([[$NORTHD_TYPE]])
-as northd-backup
-OVS_APP_EXIT_AND_WAIT([ovn-northd])
+if test -d northd-backup; then
+as northd-backup
+OVS_APP_EXIT_AND_WAIT([[$NORTHD_TYPE]])
+fi
OVN_CLEANUP_VSWITCH([main])
])
@@ -69,10 +71,12 @@ m4_define([OVN_CLEANUP_AZ],[
OVS_APP_EXIT_AND_WAIT([ovsdb-server])
as $1/northd
-OVS_APP_EXIT_AND_WAIT([ovn-northd])
+OVS_APP_EXIT_AND_WAIT([[$NORTHD_TYPE]])
-as $1/northd-backup
-OVS_APP_EXIT_AND_WAIT([ovn-northd])
+if test -d $1/northd-backup; then
+as $1/northd-backup
+OVS_APP_EXIT_AND_WAIT([[$NORTHD_TYPE]])
+fi
as $1/ic
OVS_APP_EXIT_AND_WAIT([ovn-ic])
@@ -134,15 +138,48 @@ ovn_init_ic_db () {
ovn_init_db ovn-ic-sb
}
-# ovn_start [AZ]
+# ovn_start_northd (primary|backup) [AZ]
+ovn_start_northd() {
+local priority=$1
+local AZ=$2
+local msg_prefix=${AZ:+$AZ: }
+local d_prefix=${AZ:+$AZ/}
+
+local suffix=
+case $priority in
+backup) suffix=-backup ;;
+esac
+
+local northd_args=
+case ${NORTHD_TYPE:=ovn-northd} in
+ovn-northd) ;;
+ovn-northd-ddlog)
northd_args="--ddlog-record=${AZ:+$AZ/}replay$suffix.dat -v" ;;
+esac
+
+local name=${d_prefix}northd${suffix}
+echo "${prefix}starting $name"
+test -d "$ovs_base/$name" || mkdir "$ovs_base/$name"
+as $name start_daemon $NORTHD_TYPE $northd_args -vjsonrpc \
+ --ovnnb-db=$OVN_NB_DB --ovnsb-db=$OVN_SB_DB
+}
+
+# ovn_start [--no-backup-northd] [AZ]
#
# Creates and initializes ovn-sb and ovn-nb databases and starts their
# ovsdb-server instance, sets appropriate environment variables so that
# ovn-sbctl and ovn-nbctl use them by default, and starts ovn-northd running
# against them.
ovn_start () {
-if test -n "$1"; then
-mkdir "$ovs_base"/$1
+local backup_northd=:
+case $1 in
+--no-backup-northd) backup_northd=false; shift ;;
+esac
+local AZ=$1
+local msg_prefix=${AZ:+$AZ: }
+local d_prefix=${AZ:+$AZ/}
+
+if test -n "$AZ"; then
+mkdir "$ovs_base"/$AZ
fi
ovn_init_db ovn-sb $1; ovn-sbctl init
@@ -150,36 +187,19 @@ ovn_start () {
if test -n "$1"; then
ovn-nbctl set NB_Global . name=$1
fi
-local ovn_sb_db=$OVN_SB_DB
-local ovn_nb_db=$OVN_NB_DB
-local as_d=northd
-if test -n "$1"; then
-as_d=$1/$as_d
+ovn_start_northd primary $AZ
+if $backup_northd; then
+ovn_start_northd backup $AZ
fi
-echo "starting ovn-northd"
-mkdir "$ovs_base"/$as_d
-as $as_d start_daemon ovn-northd -v \
- --ovnnb-db=$ovn_nb_db \
- --ovnsb-db=$ovn_sb_db
-as_d=northd-backup
-if test -n "$1"; then
-as_d=$1/$as_d
-fi
-echo "starting backup ovn-northd"
-mkdir "$ovs_base"/$as_d
-as $as_d start_daemon ovn-northd -v \
- --ovnnb-db=$ovn_nb_db \
- --ovnsb-db=$ovn_sb_db
+if test -n "$AZ"; then
+o
[ovs-dev] [PATCH ovn v4.1 0/7] Add DDlog implementation of ovn-northd
v1->v2:
- Applied several patches.
- New commit to add documentation for the system-userspace testsuite.
- New patches to improve the testsuite a couple of ways.
- Revised "Prepare for multiple northd types" to fix system-userspace
testsuite.
- Updated DDlog implementation of northd to match latest master.
- Updated copyright notices in DDlog implementation.
v2->v3:
- Applied documentation patch.
- Added some test improvements to fix reported problems.
- Really updated copyright notices (didn't commit, last time).
- Correctly skip ddlog tests when ddlog not compiled.
v3->v4:
- Fix dependencies for parallel build.
- Fix spelling error in documentation.
- Use --wait=sb, not --wait=hv, when no chassis are running.
- Fixed IGMP and MLD tests by porting commit 9d2e8d32fb98
("ofctrl.c: Fix duplicated flow handling in I-P while merging
opposite changes."), which I had missed before.
v4->v4.1:
- Squash last two patches as I'd intended.
Ben Pfaff (4):
tests: Prepare for multiple northd types.
tests: Use portable "test a = b", not "test a == b".
tests: Eliminate most "sleep" calls.
tests: Improve debuggability of tests.
Flavio Fernandes (1):
northd: Enhance the implementation of ACL log meters.
Leonid Ryzhyk (2):
Export `VLOG_WARN` and `VLOG_ERR` from libovn for use in ddlog
ovn-northd-ddlog: New implementation of ovn-northd based on ddlog.
Documentation/automake.mk |2 +
Documentation/intro/install/general.rst | 31 +-
Documentation/topics/debugging-ddlog.rst | 280 +
Documentation/topics/index.rst|1 +
Documentation/tutorials/ddlog-new-feature.rst | 362 +
Documentation/tutorials/index.rst |1 +
NEWS |6 +
acinclude.m4 | 43 +
configure.ac |5 +
lib/ovn-util.c| 17 +
lib/ovn-util.h|6 +
m4/ovn.m4 | 16 +
northd/.gitignore |4 +
northd/automake.mk| 104 +
northd/helpers.dl | 128 +
northd/ipam.dl| 506 ++
northd/lrouter.dl | 715 ++
northd/lswitch.dl | 638 ++
northd/multicast.dl | 263 +
northd/ovn-nb.dlopts | 13 +
northd/ovn-northd-ddlog.c | 1752
northd/ovn-northd.c | 201 +-
northd/ovn-sb.dlopts | 28 +
northd/ovn.dl | 387 +
northd/ovn.rs | 857 ++
northd/ovn.toml |2 +
northd/ovn_northd.dl | 7528 +
northd/ovsdb2ddlog2c | 127 +
ovn-nb.xml| 14 +
tests/atlocal.in |7 +
tests/ovn-controller-vtep.at |6 +-
tests/ovn-ic.at | 11 +-
tests/ovn-macros.at | 99 +-
tests/ovn-northd.at | 485 +-
tests/ovn.at | 819 +-
tests/ovs-macros.at | 43 +-
tests/system-ovn.at | 124 +-
tutorial/ovs-sandbox | 24 +-
utilities/checkpatch.py |2 +-
utilities/ovn-ctl | 20 +-
40 files changed, 15046 insertions(+), 631 deletions(-)
create mode 100644 Documentation/topics/debugging-ddlog.rst
create mode 100644 Documentation/tutorials/ddlog-new-feature.rst
create mode 100644 northd/helpers.dl
create mode 100644 northd/ipam.dl
create mode 100644 northd/lrouter.dl
create mode 100644 northd/lswitch.dl
create mode 100644 northd/multicast.dl
create mode 100644 northd/ovn-nb.dlopts
create mode 100644 northd/ovn-northd-ddlog.c
create mode 100644 northd/ovn-sb.dlopts
create mode 100644 northd/ovn.dl
create mode 100644 northd/ovn.rs
create mode 100644 northd/ovn.toml
create mode 100644 northd/ovn_northd.dl
create mode 100755 northd/ovsdb2ddlog2c
--
2.26.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v4.1 1/7] Export `VLOG_WARN` and `VLOG_ERR` from libovn for use in ddlog
From: Leonid Ryzhyk
Export `ddlog_warn` and `ddlog_err` functions that are just wrappers
around `VLOG_WARN` and `VLOG_ERR`. This is not ideal because the
functions are exported by `ovn_util.c` and the resulting log messages use
`ovn_util` as module name. More importantly, these functions do not do
log rate limiting.
Signed-off-by: Leonid Ryzhyk
Signed-off-by: Ben Pfaff
---
lib/ovn-util.c | 17 +
lib/ovn-util.h | 6 ++
2 files changed, 23 insertions(+)
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index abe6b04a7701..eb4f14efffa6 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -722,3 +722,20 @@ ip_address_and_port_from_lb_key(const char *key, char
**ip_address,
*addr_family = ss.ss_family;
return true;
}
+
+#ifdef DDLOG
+
+/* Callbacks used by the ddlog northd code to print warnings and errors.
+ */
+void
+ddlog_warn(const char *msg)
+{
+VLOG_WARN("%s", msg);
+}
+
+void
+ddlog_err(const char *msg)
+{
+VLOG_ERR("%s", msg);
+}
+#endif
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index a39cbef5a47e..77d0936a5fbc 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -230,4 +230,10 @@ char *str_tolower(const char *orig);
bool ip_address_and_port_from_lb_key(const char *key, char **ip_address,
uint16_t *port, int *addr_family);
+#ifdef DDLOG
+void ddlog_warn(const char *msg);
+void ddlog_err(const char *msg);
+#endif
+
+
#endif
--
2.26.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v4.1 2/7] tests: Prepare for multiple northd types.
The idea is to run each test twice, once with ovn-northd, once
with ovn-northd-ddlog. To do that, we add a macro OVN_FOR_EACH_NORTHD
and bracket each test that uses ovn-northd in it.
Signed-off-by: Ben Pfaff
---
tests/ovn-ic.at | 11 +-
tests/ovn-macros.at | 96 ++--
tests/ovn-northd.at | 161 ++-
tests/ovn.at| 264 +---
tests/ovs-macros.at | 20 ++--
tests/system-ovn.at | 122 +++-
6 files changed, 533 insertions(+), 141 deletions(-)
diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
index 0638af401295..2a4fba031f36 100644
--- a/tests/ovn-ic.at
+++ b/tests/ovn-ic.at
@@ -1,4 +1,5 @@
AT_BANNER([OVN Interconnection Controller])
+OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- AZ register])
ovn_init_ic_db
@@ -29,7 +30,9 @@ availability-zone az3
OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
+])
+OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- transit switch handling])
ovn_init_ic_db
@@ -59,7 +62,9 @@ check_column ts2 nb:Logical_Switch name
OVN_CLEANUP_IC([az1])
AT_CLEANUP
+])
+OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- gateway sync])
ovn_init_ic_db
@@ -120,8 +125,9 @@ OVN_CLEANUP_SBOX(gw2)
OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
+])
-
+OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- port sync])
ovn_init_ic_db
@@ -185,7 +191,9 @@ OVN_CLEANUP_SBOX(gw1)
OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
+])
+OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- route sync])
ovn_init_ic_db
@@ -310,3 +318,4 @@ OVS_WAIT_WHILE([ovn_as az1 ovn-nbctl lr-route-list lr1 |
grep learned | grep 10.
OVN_CLEANUP_IC([az1], [az2])
AT_CLEANUP
+])
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
index 5b9c2dee6812..105ad0ab7f7f 100644
--- a/tests/ovn-macros.at
+++ b/tests/ovn-macros.at
@@ -47,10 +47,12 @@ m4_define([OVN_CLEANUP],[
OVS_APP_EXIT_AND_WAIT([ovsdb-server])
as northd
-OVS_APP_EXIT_AND_WAIT([ovn-northd])
+OVS_APP_EXIT_AND_WAIT([[$NORTHD_TYPE]])
-as northd-backup
-OVS_APP_EXIT_AND_WAIT([ovn-northd])
+if test -d northd-backup; then
+as northd-backup
+OVS_APP_EXIT_AND_WAIT([[$NORTHD_TYPE]])
+fi
OVN_CLEANUP_VSWITCH([main])
])
@@ -69,10 +71,12 @@ m4_define([OVN_CLEANUP_AZ],[
OVS_APP_EXIT_AND_WAIT([ovsdb-server])
as $1/northd
-OVS_APP_EXIT_AND_WAIT([ovn-northd])
+OVS_APP_EXIT_AND_WAIT([[$NORTHD_TYPE]])
-as $1/northd-backup
-OVS_APP_EXIT_AND_WAIT([ovn-northd])
+if test -d $1/northd-backup; then
+as $1/northd-backup
+OVS_APP_EXIT_AND_WAIT([[$NORTHD_TYPE]])
+fi
as $1/ic
OVS_APP_EXIT_AND_WAIT([ovn-ic])
@@ -134,15 +138,48 @@ ovn_init_ic_db () {
ovn_init_db ovn-ic-sb
}
-# ovn_start [AZ]
+# ovn_start_northd (primary|backup) [AZ]
+ovn_start_northd() {
+local priority=$1
+local AZ=$2
+local msg_prefix=${AZ:+$AZ: }
+local d_prefix=${AZ:+$AZ/}
+
+local suffix=
+case $priority in
+backup) suffix=-backup ;;
+esac
+
+local northd_args=
+case ${NORTHD_TYPE:=ovn-northd} in
+ovn-northd) ;;
+ovn-northd-ddlog)
northd_args="--ddlog-record=${AZ:+$AZ/}replay$suffix.dat -v" ;;
+esac
+
+local name=${d_prefix}northd${suffix}
+echo "${prefix}starting $name"
+test -d "$ovs_base/$name" || mkdir "$ovs_base/$name"
+as $name start_daemon $NORTHD_TYPE $northd_args -vjsonrpc \
+ --ovnnb-db=$OVN_NB_DB --ovnsb-db=$OVN_SB_DB
+}
+
+# ovn_start [--no-backup-northd] [AZ]
#
# Creates and initializes ovn-sb and ovn-nb databases and starts their
# ovsdb-server instance, sets appropriate environment variables so that
# ovn-sbctl and ovn-nbctl use them by default, and starts ovn-northd running
# against them.
ovn_start () {
-if test -n "$1"; then
-mkdir "$ovs_base"/$1
+local backup_northd=:
+case $1 in
+--no-backup-northd) backup_northd=false; shift ;;
+esac
+local AZ=$1
+local msg_prefix=${AZ:+$AZ: }
+local d_prefix=${AZ:+$AZ/}
+
+if test -n "$AZ"; then
+mkdir "$ovs_base"/$AZ
fi
ovn_init_db ovn-sb $1; ovn-sbctl init
@@ -150,36 +187,19 @@ ovn_start () {
if test -n "$1"; then
ovn-nbctl set NB_Global . name=$1
fi
-local ovn_sb_db=$OVN_SB_DB
-local ovn_nb_db=$OVN_NB_DB
-local as_d=northd
-if test -n "$1"; then
-as_d=$1/$as_d
+ovn_start_northd primary $AZ
+if $backup_northd; then
+ovn_start_northd backup $AZ
fi
-echo "starting ovn-northd"
-mkdir "$ovs_base"/$as_d
-as $as_d start_daemon ovn-northd -v \
- --ovnnb-db=$ovn_nb_db \
- --ovnsb-db=$ovn_sb_db
-as_d=northd-backup
-if test -n "$1"; then
-as_d=$1/$as_d
-fi
-echo "starting backup ovn-northd"
-mkdir "$ovs_base"/$as_d
-as $as_d start_daemon ovn-northd -v \
- --ovnnb-db=$ovn_nb_db \
- --ovnsb-db=$ovn_sb_db
+if test -n "$AZ"; then
+o
[ovs-dev] [PATCH ovn v4.1 4/7] tests: Eliminate most "sleep" calls.
Many of these could be replaced by "ovn-nbctl sync". Some weren't really needed at all because they were adjacent to something that itself called sync or otherwise used --wait. Some were more appropriately done with explicit waits for what was really needed. I left some "sleep"s. Some were because they were "negative" sleeps: they were giving time for something to happen that should *not* happen (in other words, if you wait for it to happen, you'll wait forever, unless there's a bug). Some were because I didn't know how to properly wait for what they were waiting for, or because I didn't understand the circumstances deeply enough. Signed-off-by: Ben Pfaff --- tests/ovn-northd.at | 11 ++- tests/ovn.at| 167 2 files changed, 54 insertions(+), 124 deletions(-) diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 949a8eee054e..78b1ff728af3 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -878,7 +878,7 @@ uuid=$(fetch_column Port_Binding _uuid logical_port=cr-DR-S1) echo "CR-LRP UUID is: " $uuid check ovn-nbctl set Logical_Router $cr_uuid options:chassis=gw1 -check ovn-nbctl --wait=hv sync +check ovn-nbctl --wait=sb sync ovn-nbctl create Address_Set name=allowed_range addresses=\"1.1.1.1\" ovn-nbctl create Address_Set name=disallowed_range addresses=\"2.2.2.2\" @@ -1120,6 +1120,7 @@ ovn-nbctl --wait=sb -- --id=@hc create \ Load_Balancer_Health_Check vip="10.0.0.10\:80" -- add Load_Balancer . \ health_check @hc wait_row_count Service_Monitor 2 +check ovn-nbctl --wait=sb sync ovn-sbctl dump-flows sw0 | grep ct_lb | grep priority=120 > lflows.txt AT_CHECK([cat lflows.txt | sed 's/table=..//'], [0], [dnl @@ -1139,6 +1140,7 @@ OVS_WAIT_FOR_OUTPUT( # Set the service monitor for sw1-p1 to offline check ovn-sbctl set service_monitor sw1-p1 status=offline wait_row_count Service_Monitor 1 logical_port=sw1-p1 status=offline +check ovn-nbctl --wait=sb sync AT_CAPTURE_FILE([sbflows4]) OVS_WAIT_FOR_OUTPUT( @@ -1150,6 +1152,7 @@ OVS_WAIT_FOR_OUTPUT( ovn-sbctl set service_monitor $sm_sw0_p1 status=offline wait_row_count Service_Monitor 1 logical_port=sw0-p1 status=offline +check ovn-nbctl --wait=sb sync AT_CAPTURE_FILE([sbflows5]) OVS_WAIT_FOR_OUTPUT( @@ -1166,6 +1169,7 @@ ovn-sbctl set service_monitor $sm_sw0_p1 status=online ovn-sbctl set service_monitor $sm_sw1_p1 status=online wait_row_count Service_Monitor 1 logical_port=sw1-p1 status=online +check ovn-nbctl --wait=sb sync AT_CAPTURE_FILE([sbflows7]) OVS_WAIT_FOR_OUTPUT( @@ -1176,6 +1180,7 @@ OVS_WAIT_FOR_OUTPUT( # Set the service monitor for sw1-p1 to error ovn-sbctl set service_monitor $sm_sw1_p1 status=error wait_row_count Service_Monitor 1 logical_port=sw1-p1 status=error +check ovn-nbctl --wait=sb sync ovn-sbctl dump-flows sw0 | grep "ip4.dst == 10.0.0.10 && tcp.dst == 80" \ | grep priority=120 > lflows.txt @@ -1214,6 +1219,7 @@ OVS_WAIT_FOR_OUTPUT( check ovn-sbctl set service_monitor sw1-p1 status=online wait_row_count Service_Monitor 1 logical_port=sw1-p1 status=online +check ovn-nbctl --wait=sb sync AT_CAPTURE_FILE([sbflows10]) OVS_WAIT_FOR_OUTPUT( @@ -1244,6 +1250,7 @@ AT_CHECK([ovn-nbctl -- --id=@hc create Load_Balancer_Health_Check vip="10.0.0.10 check ovn-nbctl ls-lb-add sw0 lb2 check ovn-nbctl ls-lb-add sw1 lb2 +check ovn-nbctl --wait=sb sync wait_row_count Service_Monitor 5 @@ -1756,7 +1763,7 @@ check ovn-nbctl pg-add pg0 sw0-p1 sw1-p1 check ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4 && tcp && tcp.dst == 80" reject check ovn-nbctl acl-add pg0 to-lport 1003 "outport == @pg0 && ip6 && udp" reject -check ovn-nbctl --wait=hv sync +check ovn-nbctl --wait=sb sync AS_BOX([1]) diff --git a/tests/ovn.at b/tests/ovn.at index 2648cdd6cd07..5d9b3b1a4fa9 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -1798,10 +1798,6 @@ check ovn-nbctl --wait=hv sync # for ARP resolution). OVN_POPULATE_ARP -# Allow some time for ovn-northd and ovn-controller to catch up. -# XXX This should be more systematic. -sleep 1 - # Make sure there is no attempt to adding duplicated flows by ovn-controller AT_FAIL_IF([test -n "`grep duplicate hv1/ovn-controller.log`"]) AT_FAIL_IF([test -n "`grep duplicate hv2/ovn-controller.log`"]) @@ -1996,11 +1992,7 @@ done # set address for lp13 with invalid characters. # lp13 should be configured with only 192.168.0.13. -ovn-nbctl lsp-set-addresses lp13 "f0:00:00:00:00:13 192.168.0.13 invalid 192.169.0.13" - -# Allow some time for ovn-northd and ovn-controller to catch up. -# XXX This should be more systematic. -sleep 1 +ovn-nbctl --wait=hv lsp-set-addresses lp13 "f0:00:00:00:00:13 192.168.0.13 invalid 192.169.0.13" sip=`ip_to_hex 192 168 0 11` tip=`ip_to_hex 192 168 0 13` @@ -2075,7 +2067,11 @@ for i in 1 2; do done done -sleep 1 +# Wait for bindings to take effect. +wait_row_count Port_Binding 1 logical_port=lp11 'encap!=[[]]' +wait_row_count Port_Binding 1 l
[ovs-dev] [PATCH ovn v4.1 3/7] tests: Use portable "test a = b", not "test a == b".
"==" is a GNU extension to "test". A pox upon it.
Signed-off-by: Ben Pfaff
---
tests/ovn.at| 26 +-
tests/system-ovn.at | 2 +-
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/tests/ovn.at b/tests/ovn.at
index 75c8725911ff..2648cdd6cd07 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5410,7 +5410,7 @@ test_dhcp() {
reply_dst_ip=${offer_ip}
fi
-if test "$dhcp_type" == "04"; then
+if test "$dhcp_type" = "04"; then
ciaddr=$offer_ip
fi
@@ -12857,11 +12857,11 @@ for i in 1 2 3; do
-- lsp-set-addresses lp$i$j$k \
"f0:00:00:00:0$i:$j$k 192.168.$i$j.$k"
# logical ports lp[12]?1 belongs to port group pg1
-if test $i != 3 && test $k == 1; then
+if test $i != 3 && test $k = 1; then
pg1_ports="$pg1_ports `get_lsp_uuid $i$j$k`"
fi
# logical ports lp[23]?2 belongs to port group pg2
-if test $i != 1 && test $k == 2; then
+if test $i != 1 && test $k = 2; then
pg2_ports="$pg2_ports `get_lsp_uuid $i$j$k`"
fi
done
@@ -13012,8 +13012,8 @@ for is in 1 2 3; do
if test $d != $s; then unicast=$d; else unicast=; fi
# packets matches ACL should be dropped
-if test $id != 3 && test $kd == 1; then
-if test $is != 1 && test $ks == 2; then
+if test $id != 3 && test $kd = 1; then
+if test $is != 1 && test $ks = 2; then
unicast=
fi
fi
@@ -13081,11 +13081,11 @@ for i in 1 2 3; do
-- lsp-set-addresses lp$i$j$k \
"f0:00:00:00:0$i:$j$k 192.168.$i$j.$k"
# logical ports lp[12]?1 belongs to port group pg1
-if test $i != 3 && test $k == 1; then
+if test $i != 3 && test $k = 1; then
pg1_ports="$pg1_ports `get_lsp_uuid $i$j$k`"
fi
# logical ports lp[23]?2 belongs to port group pg2
-if test $i != 1 && test $k == 2; then
+if test $i != 1 && test $k = 2; then
pg2_ports="$pg2_ports `get_lsp_uuid $i$j$k`"
fi
done
@@ -13254,8 +13254,8 @@ for is in 1 2 3; do
if test $d != $s; then unicast=$d; else unicast=; fi
# packets matches ACL1 but not ACL2 should be dropped
-if test $id != 3 && test $kd == 1; then
-if test $is == 1 || test $ks != 2; then
+if test $id != 3 && test $kd = 1; then
+if test $is = 1 || test $ks != 2; then
unicast=
fi
fi
@@ -14482,7 +14482,7 @@ hv3_uuid=$(ovn-sbctl list chassis hv3 | grep uuid | awk
'{print $3}')
chassis=`ovn-sbctl --bare --columns chassis find port_binding \
logical_port=ls1-lp_ext1`
-AT_CHECK([test x$chassis == x], [0], [])
+AT_CHECK([test x$chassis = x], [0], [])
# Associate hagrp1 ha-chassis-group to ls1-lp_ext1
ovn-nbctl --wait=hv set Logical_Switch_Port ls1-lp_ext1 \
@@ -22187,8 +22187,8 @@ echo hv3_ts=$hv3_ts
hv_cfg_ts=$(ovn-nbctl get nb_global . hv_cfg_timestamp)
check test "$hv3_ts" = "$hvt4"
-AT_CHECK([test x$(ovn-nbctl --print-wait-time --wait=sb sync | grep ms | wc
-l) == x2])
-AT_CHECK([test x$(ovn-nbctl --print-wait-time --wait=hv sync | grep ms | wc
-l) == x3])
+AT_CHECK([test x$(ovn-nbctl --print-wait-time --wait=sb sync | grep ms | wc
-l) = x2])
+AT_CHECK([test x$(ovn-nbctl --print-wait-time --wait=hv sync | grep ms | wc
-l) = x3])
for i in $(seq 2 $n); do
OVN_CLEANUP_SBOX([hv$i])
@@ -22815,7 +22815,7 @@ sleep 2
# Check if the encap_rec changed (it should not have)
encap_rec_mvtep1=$(ovn-sbctl --data=bare --no-heading --column encaps list
chassis hv1)
-AT_CHECK([test "$encap_rec_mvtep" == "$encap_rec_mvtep1"], [0], [])
+AT_CHECK([test "$encap_rec_mvtep" = "$encap_rec_mvtep1"], [0], [])
OVN_CLEANUP([hv1])
AT_CLEANUP
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 8bb8c439f4d6..ee9ce332c4a7 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -,7 +,7 @@ AT_CHECK([ovs-vsctl set interface ovs-public
external-ids:ovn-egress-iface=true]
OVS_WAIT_UNTIL([tc qdisc show | grep -q 'htb 1: dev ovs-public'])
AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options qos_burst=1000])
-OVS_WAIT_UNTIL([test "$(tc qdisc show | grep 'htb 1: dev ovs-public')" == ""])
+OVS_WAIT_UNTIL([test "$(tc qdisc show | grep 'htb 1: dev ovs-public')" = ""])
kill $(pidof ovn-controller)
--
2.26.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v4.1 5/7] tests: Improve debuggability of tests.
These changes should make it easier to debug various tests.
Signed-off-by: Ben Pfaff
---
tests/ovn-controller-vtep.at | 6 +-
tests/ovn-northd.at | 129 -
tests/ovn.at | 356 +++
tests/ovs-macros.at | 20 +-
4 files changed, 217 insertions(+), 294 deletions(-)
diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
index 8b4c180b1669..0d1693682616 100644
--- a/tests/ovn-controller-vtep.at
+++ b/tests/ovn-controller-vtep.at
@@ -181,7 +181,7 @@ OVN_CONTROLLER_VTEP_START
AT_CHECK([vtep-ctl add-ls lswitch0 -- bind-ls br-vtep p0 100 lswitch0 --
bind-ls br-vtep p1 300 lswitch0])
# adds logical switch port in ovn-nb database, and sets the type and options.
OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep_lswitch0], [br-vtep], [lswitch0])
-check ovn-sbctl wait-until Port_Binding br-vtep_lswitch0 chassis!='[[]]'
+wait_row_count Port_Binding 1 logical_port=br-vtep_lswitch0 chassis!='[[]]'
# should see one binding, associated to chassis of 'br-vtep'.
chassis_uuid=$(ovn-sbctl --columns=_uuid list Chassis br-vtep | cut -d ':' -f2
| tr -d ' ')
AT_CHECK_UNQUOTED([ovn-sbctl --columns=chassis list Port_Binding
br-vtep_lswitch0 | cut -d ':' -f2 | tr -d ' '], [0], [dnl
@@ -192,7 +192,7 @@ ${chassis_uuid}
AT_CHECK([vtep-ctl add-ls lswitch1 -- bind-ls br-vtep p0 200 lswitch1])
# adds logical switch port in ovn-nb database for lswitch1.
OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep_lswitch1], [br-vtep], [lswitch1])
-check ovn-sbctl wait-until Port_Binding br-vtep_lswitch1 chassis!='[[]]'
+wait_row_count Port_Binding 1 logical_port=br-vtep_lswitch1 chassis!='[[]]'
# This is allowed, but not recommended, to have two vlan_bindings (to
different vtep logical switches)
# from one vtep gateway physical port in one ovn-nb logical swithch.
AT_CHECK_UNQUOTED([ovn-sbctl --columns=chassis list Port_Binding | cut -d ':'
-f2 | tr -d ' ' | sort], [0], [dnl
@@ -203,7 +203,7 @@ ${chassis_uuid}
# adds another logical switch port in ovn-nb database for lswitch0.
OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep_lswitch0_dup], [br-vtep], [lswitch0])
-check ovn-sbctl wait-until Port_Binding br-vtep_lswitch0_dup chassis!='[[]]'
+wait_row_count Port_Binding 1 logical_port=br-vtep_lswitch0_dup chassis!='[[]]'
# it is not allowed to have more than one ovn-nb logical port for the same
# vtep logical switch on a vtep gateway chassis, so should still see only
# two port_binding entries bound.
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 78b1ff728af3..972ff5c626a3 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1078,7 +1078,7 @@ health_check @hc | uuidfilt], [0], [<0>
wait_row_count Service_Monitor 0
-# create logical switches and ports
+AS_BOX([create logical switches and ports])
ovn-nbctl ls-add sw0
ovn-nbctl --wait=sb lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 \
"00:00:00:00:00:03 10.0.0.3"
@@ -1105,7 +1105,7 @@ OVS_WAIT_FOR_OUTPUT(
(ls_in_stateful ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10
&& tcp.dst == 80), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80);)
])
-# Delete the Load_Balancer_Health_Check
+AS_BOX([Delete the Load_Balancer_Health_Check])
ovn-nbctl --wait=sb clear load_balancer . health_check
wait_row_count Service_Monitor 0
@@ -1115,7 +1115,7 @@ OVS_WAIT_FOR_OUTPUT(
[ (ls_in_stateful ), priority=120 , match=(ct.new && ip4.dst ==
10.0.0.10 && tcp.dst == 80), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80);)
])
-# Create the Load_Balancer_Health_Check again.
+AS_BOX([Create the Load_Balancer_Health_Check again.])
ovn-nbctl --wait=sb -- --id=@hc create \
Load_Balancer_Health_Check vip="10.0.0.10\:80" -- add Load_Balancer . \
health_check @hc
@@ -1127,7 +1127,7 @@ AT_CHECK([cat lflows.txt | sed 's/table=..//'], [0], [dnl
(ls_in_stateful ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10
&& tcp.dst == 80), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80);)
])
-# Get the uuid of both the service_monitor
+AS_BOX([Get the uuid of both the service_monitor])
sm_sw0_p1=$(fetch_column Service_Monitor _uuid logical_port=sw0-p1)
sm_sw1_p1=$(fetch_column Service_Monitor _uuid logical_port=sw1-p1)
@@ -1137,7 +1137,7 @@ OVS_WAIT_FOR_OUTPUT(
[ (ls_in_stateful ), priority=120 , match=(ct.new && ip4.dst ==
10.0.0.10 && tcp.dst == 80), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80);)
])
-# Set the service monitor for sw1-p1 to offline
+AS_BOX([Set the service monitor for sw1-p1 to offline])
check ovn-sbctl set service_monitor sw1-p1 status=offline
wait_row_count Service_Monitor 1 logical_port=sw1-p1 status=offline
check ovn-nbctl --wait=sb sync
@@ -1148,7 +1148,7 @@ OVS_WAIT_FOR_OUTPUT(
[ (ls_in_stateful ), priority=120 , match=(ct.new && ip4.dst ==
10.0.0.10 && tcp.dst == 80), action=(ct_lb(backends=10.0.0.3:80);)
])
-# Set the service monitor for sw0-p1 to offline
+AS_BOX([Set the service monitor for sw0-p1 to offline])
ovn-sb
[ovs-dev] [PATCH ovn v4.1 7/7] northd: Enhance the implementation of ACL log meters.
From: Flavio Fernandes
When multiple ACL rows use the same Meter for log rate-limiting, the
'noisiest' ACL matches may completely consume the Meter and shadow other
ACL logs. This patch introduces an option in NB_Global that allows for a
Meter to rate-limit each ACL log separately.
The change is backward compatible. Based on a well known option, northd
will turn a single Meter in the NB into multiple Meters in the SB by
appending the ACL uuid to its name. It will also make action in logical
flow use the unique Meter name for each affected ACL log. In order to
make the Meter behave as described, add this option:
ovn-nbctl set nb_global . options:acl_shared_log_meters="${meter_name}"
If more than one Meter is wanted, simply use a comma to separate each name.
Reported-by: Flavio Fernandes
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-October/050769.html
Signed-off-by: Flavio Fernandes
Signed-off-by: Ben Pfaff
---
Documentation/topics/debugging-ddlog.rst | 2 +-
northd/automake.mk | 4 +-
northd/lswitch.dl| 20 +-
northd/multicast.dl | 4 +
northd/ovn-northd.c | 201 +++
northd/ovn_northd.dl | 305 +--
ovn-nb.xml | 14 ++
tests/ovn-northd.at | 99
tests/ovn.at | 6 -
9 files changed, 449 insertions(+), 206 deletions(-)
diff --git a/Documentation/topics/debugging-ddlog.rst
b/Documentation/topics/debugging-ddlog.rst
index 046419b995f1..5403de73db41 100644
--- a/Documentation/topics/debugging-ddlog.rst
+++ b/Documentation/topics/debugging-ddlog.rst
@@ -26,7 +26,7 @@ Debugging the DDlog version of ovn-northd
=
This document gives some tips for debugging correctness issues in the
-DDlog implementation of ``ovn-northd``. To keep things conrete, we
+DDlog implementation of ``ovn-northd``. To keep things concrete, we
assume here that a failure occurred in one of the test cases in
``ovn-e2e.at``, but the same methodology applies in any other
environment. If none of these methods helps, ask for assistance or
diff --git a/northd/automake.mk b/northd/automake.mk
index 2717f59c5f3a..157b5d0df487 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -22,8 +22,7 @@ bin_PROGRAMS += northd/ovn-northd-ddlog
northd_ovn_northd_ddlog_SOURCES = \
northd/ovn-northd-ddlog.c \
northd/ovn-northd-ddlog-sb.inc \
- northd/ovn-northd-ddlog-nb.inc \
- northd/ovn_northd_ddlog/ddlog.h
+ northd/ovn-northd-ddlog-nb.inc
northd_ovn_northd_ddlog_LDADD = \
northd/ovn_northd_ddlog/target/release/libovn_northd_ddlog.la \
lib/libovn.la \
@@ -46,6 +45,7 @@ BUILT_SOURCES += \
northd/ovn-northd-ddlog-sb.inc \
northd/ovn-northd-ddlog-nb.inc
+northd/ovn-northd-ddlog.$(OBJEXT): northd/ovn_northd_ddlog/ddlog.h
northd/ovn_northd_ddlog/ddlog.h: northd/ddlog.stamp
CARGO_VERBOSE = $(cargo_verbose_$(V))
diff --git a/northd/lswitch.dl b/northd/lswitch.dl
index 4ba3f6a376d8..80fd8977f6c5 100644
--- a/northd/lswitch.dl
+++ b/northd/lswitch.dl
@@ -101,18 +101,16 @@ LogicalSwitchHasStatefulACL(ls, false) :-
nb::Logical_Switch(._uuid = ls),
not LogicalSwitchStatefulACL(ls, _).
-relation LogicalSwitchLocalnetPort0(ls_uuid: uuid, lsp_name: string)
-LogicalSwitchLocalnetPort0(ls_uuid, lsp_name) :-
+relation LogicalSwitchLocalnetPort0(ls_uuid: uuid, lsp:
nb::Logical_Switch_Port)
+LogicalSwitchLocalnetPort0(ls_uuid, lsp) :-
ls in nb::Logical_Switch(._uuid = ls_uuid),
var lsp_uuid = FlatMap(ls.ports),
-lsp in nb::Logical_Switch_Port(._uuid = lsp_uuid),
-lsp.__type == "localnet",
-var lsp_name = lsp.name.
+lsp in nb::Logical_Switch_Port(._uuid = lsp_uuid, .__type = "localnet").
-relation LogicalSwitchLocalnetPorts(ls_uuid: uuid, localnet_port_names:
Vec)
-LogicalSwitchLocalnetPorts(ls_uuid, localnet_port_names) :-
+relation LogicalSwitchLocalnetPorts(ls_uuid: uuid, localnet_ports:
Vec)
+LogicalSwitchLocalnetPorts(ls_uuid, localnet_ports) :-
LogicalSwitchLocalnetPort0(ls_uuid, lsp_name),
-var localnet_port_names = lsp_name.group_by(ls_uuid).to_vec().
+var localnet_ports = lsp_name.group_by(ls_uuid).to_vec().
LogicalSwitchLocalnetPorts(ls_uuid, vec_empty()) :-
ls in nb::Logical_Switch(),
var ls_uuid = ls._uuid,
@@ -163,7 +161,7 @@ relation &Switch(
has_stateful_acl: bool,
has_lb_vip:bool,
has_dns_records: bool,
-localnet_port_names: Vec,
+localnet_ports:Vec,
subnet:Option<(in_addr/*subnet*/, in_addr/*mask*/,
bit<32>/*start_ipv4*/, bit<32>/*total_ipv4s*/)>,
ipv6_prefix: Option,
mcast_cfg: Ref,
@@ -187,7 +185,7 @@ function ipv6_parse_prefix(s: string): Option {
.has_stateful_acl = has_stateful_acl,
.has_lb_vip
Re: [ovs-dev] [PATCH ovn v4 0/8] Add DDlog implementation of ovn-northd
Oops, I goofed a bit, please see v4.1 instead. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] ddlog for ACL log meters (was: [PATCH v3 ovn 1/1] northd: Enhance the implementation of ACL log meters.)
On Tue, Nov 10, 2020 at 03:43:17PM -0500, Flavio Fernandes wrote:
> [cc Dimitru, Numan, MarkM]
>
>
> > On Nov 10, 2020, at 2:15 PM, Ben Pfaff wrote:
> >
> > On Mon, Nov 09, 2020 at 09:53:16AM -0500, Flavio Fernandes wrote:
> >>> On Nov 7, 2020, at 4:39 PM, Ben Pfaff wrote:
> >>> On Tue, Nov 03, 2020 at 10:18:34PM +, Flavio Fernandes wrote:
> When multiple ACL rows use the same Meter for log rate-limiting, the
> 'noisiest' ACL matches may completely consume the Meter and shadow other
> ACL logs. This patch introduces an option in NB_Global that allows for a
> Meter to rate-limit each ACL log separately.
> >>>
> >>> I'm not sure I like the overall design here. This option isn't going to
> >>> scale very well to many of these meters, since they'd all need to be
> >>> shoved into a single comma-separated list. Another way might be to add
> >>> a column to the ACL table to mark a meter as shared or private. Or
> >>> there could be a name convention, e.g. prefix with "shared_" to make it
> >>> shared.
> > OK, let's step back here and consider G. Why do we need a new
> > southbound Meter row for each instance? That's a bit of a waste, isn't
> > it? Why can't we just add an indicator to the southbound Meter that
> > says "each reference to me is unique"? Or a new argument to the
> > southbound log() action that distinguishes references to a given meter,
> > so that identical values get the same one and different values get
> > distinct ones?
>
> Hmm That sounds really good, actually. We would still need 'A' as a way
> to propagate that desire from the CMS' perspective, agree?
>
> I must confess that the wasteful approach of rows in the SB comes from my
> limited knowledge on how to efficiently use the log action to do what you
> described. Also, because I was hoping to solve the whole problem within
> northd.
>
> If I understand you correctly, option 'G' you mention here would require
> changes in the SB schema as well as in ovn-controller? I will definitely
> need some pointers to make that happen. Wanna be my partner in
> crime? :) No pressure.
It would require ovn-controller changes. However, maybe it's not worth
doing them yet? It would only yield a small efficiency improvement, and
it's always possible to get that later.
> > Finally, we need to create new southbound Meter records for the "shared"
> > meters. I'm actually a bit confused about this. I would have thought
> > that "shared" meters wouldn't need the Meter records whose names are
> > un-suffixed by __, but the C implementation appears to always
> > create them. So I left in place the existing DDlog code that just
> > copies from nb::Meter to sb::Out_Meter:
>
> I did not want to alienate any other users of the Meter name. So yes,
> I intentionally kept the creation of the unmodified meter in place to
> cover potential cases where something unrelated to acl log decided
> to use the same meter name. Maybe a bit too paranoid?!? ;)
I don't know. This kind of thing can always be changed later.
> > The above might need some explanation for those relatively new to
> > Datalog or DDlog. First, the style. This uses Datalog-style syntax
> > (output :- input) instead of the FLWOR-style syntax (for (input) {
> > output }). One reason for that is because the FLWOR-style syntax
> > currently doesn't support FlatMap, which we need in that case.
>
> It will be a while longer until I manage to acclimate to the syntax in
> ovn_northd.dl. But I can see how powerful it can be. Reminds me of
> the few months it took me to learn Erlang, which remains as one of
> my favorite languages.
It's kind of cool. Some of the things that are relatively easy in C are
somewhat difficult and obscure in DDlog (ID allocation is the best
example that occurs to me off-hand), but some things that are tricky in
C are really easy in DDlog. The syntax is the biggest barrier,
honestly.
> Lastly, it goes w/out saying but just to be clear: Given how fast you move
> and how far along ddlog patches are, my intentions are for not getting any
> of the 'fair' meters code merged before ddlog's.
Thanks a lot!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] actions: Fix issues with fwd_group action.
Bleep bloop. Greetings Ben Pfaff, 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 99 characters long (recommended limit is 79) #104 FILE: ovn-sb.xml:2077: fwd_group(liveness=bool, childports=port, ...); Lines checked: 169, 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 v4.1 1/7] Export `VLOG_WARN` and `VLOG_ERR` from libovn for use in ddlog
Bleep bloop. Greetings Ben Pfaff, 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: Ben Pfaff Lines checked: 61, 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 v4.1 6/7] ovn-northd-ddlog: New implementation of ovn-northd based on ddlog.
Bleep bloop. Greetings Ben Pfaff, 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: Comment with 'xxx' marker #4653 FILE: northd/ovn-northd-ddlog.c:1273: * XXX If the transaction we're sending to the database fails, then WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator #4857 FILE: northd/ovn-northd-ddlog.c:1477: --ovnnb-db=DATABASE connect to ovn-nb database at DATABASE\n\ WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator #4859 FILE: northd/ovn-northd-ddlog.c:1479: --ovnsb-db=DATABASE connect to ovn-sb database at DATABASE\n\ WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator #4861 FILE: northd/ovn-northd-ddlog.c:1481: --unixctl=SOCKET override default control socket name\n\ Lines checked: 14400, Warnings: 9, 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 v4.1 7/7] northd: Enhance the implementation of ACL log meters.
Bleep bloop. Greetings Ben Pfaff, 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: Ben Pfaff Lines checked: 1102, 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 v11 ovn] Allow to run multiple controllers on the same machine
On Tue, Nov 10, 2020 at 10:25 AM Numan Siddique wrote:
>
> "
>
> On Tue, Nov 3, 2020 at 4:45 AM Ihar Hrachyshka wrote:
> >
> > User stories:
> > 1) NFV: an admin wants to run two separate instances of OVN controller
> >using the same database but configuring ports on different bridges.
> >Some of these bridges may use DPDK while others may not.
> >
> > 2) Parallel OVN instances: an admin wants to run two separate
> >instances of OVN controller using different databases. The
> >instances are completely independent and serve different consumers.
> >For example, the same machine runs both OpenStack and OpenShift
> >stacks, each running its own separate OVN stack.
> >
> > To serve these use cases, several features should be added to
> > ovn-controller:
> >
> > - use different database configuration for multiple controllers;
> > - customize chassis name used by controller.
> >
> > =
> >
> > For each of the following database configuration options, their
> > extended chassis specific counterparts are introduced:
> >
> > external_ids:hostname
> > external_ids:ovn-bridge
> > external_ids:ovn-bridge-datapath-type
> > external_ids:ovn-bridge-mappings
> > external_ids:ovn-chassis-mac-mappings
> > external_ids:ovn-cms-options
> > external_ids:ovn-encap-csum
> > external_ids:ovn-encap-ip
> > external_ids:ovn-encap-type
> > external_ids:ovn-is-interconn
> > external_ids:ovn-monitor-all
> > external_ids:ovn-openflow-probe-interval
> > external_ids:ovn-remote
> > external_ids:ovn-remote-probe-interval
> >
> > For example,
> >
> > external_ids:ovn-bridge -> external_ids:ovn-bridge-=
> > external_ids:ovn-encap-ip -> external_ids:ovn-encap-ip-=
> > external_ids:ovn-remote -> external_ids:ovn-remote-=
> >
> > Priority wise, specific options take precedence.
> >
> > =
> >
> > For system-id,
> >
> > You can now pass intended chassis name via CLI argument:
> >
> > $ ovn-controller ... -n
> >
> > Alternatively, you can configure a chassis name by putting it into the
> > ${ovn_sysconfdir}/system-id-override file before running the
> > controller.
> >
> > The latter option may be more useful in container environment where
> > the same image may be reused for multiple controller instances, where
> > ovs_sysconfigdir/ovn/system-id-override is a volume mounted into this
> > generic image. The override file is read once on startup. If you want
> > to apply a new chassis name to a controller instance, restart it to
> > reread the file.
> >
> > Priority wise, this is the order in which different means to configure
> > the chassis name are used:
> >
> > - ovn-controller ... -n CLI argument.
> > - ${ovs_sysconfdir}/ovn/system-id-override file;
> > - external_ids:system-id= ovsdb option;
> >
> > =
> >
> > Concurrent chassis running on the same host may inadvertantly remove
> > patch ports that belong to their peer chassis. To avoid that, patch
> > ports are now tagged in external-ids:ovn-chassis-id with the
> > appropriate chassis name, and only patch ports that belong to the
> > chassis are touched when cleaning up. Also, now only tunnels on the
> > active integration bridge are being cleaned up.
> >
> > Note that external-ids:ovn-chassis-id key is already used for tunnel
> > ports to identify the remote tunnel endpoint. We can reuse the same
> > key for patch ports because the key usage is not overlapping.
> >
> > Alternatively, we could introduce a new key with a similar but
> > different name. This would simplify code changes needed but would
> > arguably introduce even more confusion. Since the key name is not
> > entirely self-descriptive for tunnel ports (a better name would be
> > e.g. ovn-remote-chassis or ovn-peer-chassis), the ideal scenario would
> > be to rename the key for tunnel endpoints but reuse it for patch
> > ports. This would involve additional migration steps and is probably
> > not worth the hassle.
> >
> > =
> >
> > This patch also expands tunnel port name to allow for long similar
> > chassis names used on the same host.
> >
> > =
> >
> > Note: this patch assumes that each chassis has its own unique IP.
> > Future work may consider adding support to specify custom port numbers
> > for tunneling that would allow to reuse the same IP address for
> > multiple chassis running on the same host. This work is out of scope
> > for this patch.
> >
> > Signed-off-by: Ihar Hrachyshka
> >
> > ---
> >
> > v1: initial implementation.
> > v2: fixed test case to check ports are claimed by proper chassis.
> > v2: added NEWS entry.
> > v2: fixed some compiler warnings.
> > v2: moved file_system_id declaration inside a function that uses it.
> > v2: removed unneeded binding.h #include.
> > v2: docs: better explanation of alternatives to select chassis name.
> > v3: reverted priority order for chassis configuration: first CLI, then
> > system-id file, then ovsdb.
> > v4: introduce helpers to extract external-ids (per-chassis or global).
> > v4: introduce per-chassis config option
[ovs-dev] Visa Raised Interchange & Processing Fees. Cash Discount for $24.99/mo Flat-Fee.
Beide alais oog dit heele echte assam. Op er nu producten bereiding te schepping japansche verbonden. Misschien stoompomp gelukkige kan tot die gevestigd voorschot gas dringende. Hadden zilver varens te tronoh in is. Heb der noodlottig herkenbaar middelpunt bevaarbaar dus wel monopolies smeltovens. Wij bewegende lot singapore mengeling wat vochtigen gedurende aan federatie. Weerstand bepaalden de nu gesmolten krachtige vreedzame gevestigd. Er vrij tien maar apen bron ze zijn. Kwam zake open ze nu af. Initiatief is of getaxeerde dergelijke in nu. Deele groot nu er assam. Slotte van minder rijker zoo jammer duiken. In mogelijk te ze af stroomen plantage menschen vijftien. Mag stoompomp plaatsing ingericht daaronder der brandhout overvloed zit. Op op te ormoezd en belooft cultuur aangaan meester. Opgebracht archimedes wetenschap ver middelpunt doorzoeken zin sap regelmatig. Vruchtbaar feestdagen dat die per aangeplant zal buitendien. Kriang lappen langen tweede met sap dieper mag kregen. Tegen juist holte is grond beide ze te. Perzische far voorschot aangewend elk ten. Natuurlijk ontwouding onvermoeid sap aanleiding verscholen dit uit zes. Dag juist zal bij wordt eigen. Loodrecht olifanten ton mei arabische. Lot gold toen maar laag goed zoo. Kunnen kisten en lossen op zuiger kegels pusing er of. Ugong zesde is spijt eerst nu of kwala. Brengen den tunnels zes geoogst inkomen aan met. Tijgers ook zwijnen planter jungles gewicht had met een. Aankoopen er de gedurende nabijheid op. Heb alais assam omdat per tot. Kan zij gebergten nutteloos ook financien sap opgevoerd wonderwel. Mongolen bak zoo plaatsen wij verlaten. Voorraad atjehers dikwijls opmeting voordeel was zee het rivieren. En ontwikkeld opgebracht voertuigen de districten. Goudmijnen is er ingewanden getaxeerde tinwinning insnijding kilometers de. Rug der twee thee mont. Kisten ruimte nieuwe bij streng overal het. Zekeren brokken zij far percent dal element hun talrijk mantras. Gevonden eilandje en verdeeld centraal schijnen zuiniger ze. Na bakken eerste gebied de voeden vormen meeste al. Laag jaar ad iets ik af meer op. Maar over dan kuil bak zit toen was veel. Gebouwen naburige ook zin ongeveer rug maleiers algemeen. Eischen gif gebeurt simplon wat ons opnieuw. Had kan ver elk velden streek drukke. Mineralen perzische rug zes maleische elk bepaalden entrepots. Plaats kracht wat spuwen wel lot ons winnen. Bereiding overwaard hanteeren maleische versteend bepaalden dag zij. Troepjes wasschen bladeren dat dan vestigen rug scheppen mogelijk. Goed op dien over ik en ze dure door vele. Als zuiger ook zwavel velden buizen zeggen sultan. Aan meest wel roode gas niets zou langs. Woekeraars tooverslag om europeesch in ontwouding en. En eischen krijgen of tienden. Haar men dal want wild naar werd den. Kwarts ons dan dit ter verren ziekte bezoek rubben hoogte. Nabijheid terreinen opgericht aan wat britschen der. Dal naast perak halen werkt verre zoo. Spijt alles de ad beide nu eerst er. En nu monopolie uitrollen siameezen nutteloos. Van far kamarat dat terrein vliegen gezocht dik. Ingenieurs de intusschen verkochten ad er om wonderbare. Laatsten nu kapitaal en arbeiden district ze maleiers werkzaam. Wij tot zoo gemengd genomen fortuin bersawa bewerkt geleden zin. Was zuidgrens verzameld wat bak bepaalden per. Voor al bouw de pomp ad door stad nu. Te krachtige degelijke ad brandstof op ik. Bewijs die beslag gezond leenen ook sunger boomen ons. In upasboom en atjehers gewonnen de uithoudt kolonien toekomst er. Toeneming afstanden resultaat per gas van vreedzame beteekent visschers wij. Hun agentschap mag voorloopig met zij belangrijk. Ploeg om dient nu ze ze jonge sinds varen. Behalen planten evenals gif citadel stukken proeven weg tin. Kegels eerder hoogen en nu enorme ze waarde winnen. Are maleiers wie mineraal onzuiver oog zandlaag. Nu is of binnenste en vochtigen onderling regeering uitgaande. Aanleiding europeesch nu spoorwegen sagopalmen is. En nu mijnschool monopolies getaxeerde om en weelderige. Gedaald behalen ik meester de kleeren voldoen te ze. Mee menschen oog inkrimpt verleden resident. Geschikt verwoede men toekomst ons omgeving den bestuurd resident. Van buitendien ingesneden agentschap gunstigste lot dal archimedes. Prachtige mee hellingen chineezen lot toe resultaat. Al in of jaar vier zout ze. Meesters ze gebruikt uitgaven allerlei en. Bezetten meenemen zandlaag in nu om verhoogd af. Te ad dergelijke aanmerking ik vergrooten ze weggevoerd. Zij weg bezig geluk aldus naast naast tot mag zendt. Alle over are dit het waar pomp dit sago. Dienen in ad op zoodat jammer. Uitvoer scholen mee die per geplant. Men puin arme dat dik oog drie apen neen. Het groeit mijnen herten elders noodig zij ton oorlog. Ormoezd ad amboina aardige in geweest tweeden. Al en buitendien verzamelen onderne
[ovs-dev] re
I have a donation for you. Contact me for more details Redlands Community College provides a learner-centered environment committed to academic excellence strengthened through service and civic engagement. **CONFIDENTIALITY** - This e-mail (including any attachments) may contain confidential, proprietary, and/or privileged information and is intended only for the individual(s) addressed in the message. If you are not the named addressee, you should not disseminate, distribute, or copy this e-mail. If you are not the intended recipient, please notify the sender immediately by return e-mail, and delete this message and any attachments from your system. Any unauthorized disclosure or use of this information is prohibited. Information contained herein may be subject to the Privacy Act of 1974, Family Educational Rights and Privacy Act of 1974 (FERPA), and/or the Health Insurance Portability and Accountability Act of 1996 (HIPAA). ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] Allow VLAN traffic when LS:vlan-passthru=true
On Tue, Nov 10, 2020 at 8:06 AM Ihar Hrachyshka wrote:
>
> A new other_config:vlan-passthru knob is added to Logical-Switches. When
> true, VLAN tagged incoming traffic is allowed. This option can be used
> to implement OpenStack Network VLAN transparency API extension [1].
>
> [1]
> https://docs.openstack.org/api-ref/network/v2/index.html#vlan-transparency-extension
>
> Signed-off-by: Ihar Hrachyshka
Thanks for the patch.
I applied this patch to master with the changes below in the test file
which adds "check" before
the ovn-nbctl commands.
diff --git a/tests/ovn.at b/tests/ovn.at
index 1d11ba812..ce62fa231 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -2987,12 +2987,11 @@ AT_CLEANUP
AT_SETUP([ovn -- VLAN transparency, passthru=true])
ovn_start
-ovn-nbctl ls-add ls
-ovn-nbctl --wait=sb add Logical-Switch ls other_config vlan-passthru=true
+check ovn-nbctl ls-add ls
+check ovn-nbctl --wait=sb add Logical-Switch ls other_config vlan-passthru=true
for i in 1 2; do
-ovn-nbctl lsp-add ls lsp$i
-ovn-nbctl lsp-set-addresses lsp$i f0:00:00:00:00:0$i
-#ovn-nbctl lsp-set-port-security lsp$i f0:00:00:00:00:0$i
+check ovn-nbctl lsp-add ls lsp$i
+check ovn-nbctl lsp-set-addresses lsp$i f0:00:00:00:00:0$i
done
net_add physnet
@@ -3035,11 +3034,11 @@ AT_CLEANUP
AT_SETUP([ovn -- VLAN transparency, passthru=false])
ovn_start
-ovn-nbctl ls-add ls
-ovn-nbctl --wait=sb add Logical-Switch ls other_config vlan-passthru=false
+check ovn-nbctl ls-add ls
+check ovn-nbctl --wait=sb add Logical-Switch ls other_config
vlan-passthru=false
for i in 1 2; do
-ovn-nbctl lsp-add ls lsp$i
-ovn-nbctl lsp-set-addresses lsp$i f0:00:00:00:00:0$i
+check ovn-nbctl lsp-add ls lsp$i
+check ovn-nbctl lsp-set-addresses lsp$i f0:00:00:00:00:0$i
done
net_add physnet
Numan
> ---
> NEWS| 2 +
> northd/ovn-northd.c | 14 +--
> ovn-nb.xml | 7
> tests/ovn.at| 93 +
> 4 files changed, 113 insertions(+), 3 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 47ffc27b8..601023067 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -6,6 +6,8 @@ Post-v20.09.0
> removed. See ovn-nb(5) for advice on how to update your config if
> needed.
> - Add IPv4 iPXE support introducing "bootfile_name_alt" option to ovn dhcp
> server.
> + - Support other_config:vlan-passthru=true to allow VLAN tagged incoming
> + traffic.
>
> OVN v20.09.0 - 28 Sep 2020
> --
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index dbe5fa676..8f134a048 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -6803,6 +6803,12 @@
> build_drop_arp_nd_flows_for_unbound_router_ports(struct ovn_port *op,
> ds_destroy(&match);
> }
>
> +static bool
> +is_vlan_transparent(const struct ovn_datapath *od)
> +{
> +return smap_get_bool(&od->nbs->other_config, "vlan-passthru", false);
> +}
> +
> static void
> build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
> struct hmap *port_groups, struct hmap *lflows,
> @@ -6850,9 +6856,11 @@ build_lswitch_flows(struct hmap *datapaths, struct
> hmap *ports,
> continue;
> }
>
> -/* Logical VLANs not supported. */
> -ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_L2, 100,
> "vlan.present",
> - "drop;");
> +if (!is_vlan_transparent(od)) {
> +/* Block logical VLANs. */
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_L2, 100,
> + "vlan.present", "drop;");
> +}
>
> /* Broadcast/multicast source address is invalid. */
> ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_L2, 100,
> "eth.src[40]",
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 5e8635992..5704eabea 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -525,6 +525,13 @@
>
>
>
> +
> + + type='{"type": "boolean"}'>
> +Determines whether VLAN tagged incoming traffic should be allowed.
> +
> +
> +
>
>
> See External IDs at the beginning of this document.
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 1c29cdf26..3d3811888 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -2984,6 +2984,99 @@ OVN_CLEANUP([hv-1],[hv-2])
>
> AT_CLEANUP
>
> +AT_SETUP([ovn -- VLAN transparency, passthru=true])
> +ovn_start
> +
> +ovn-nbctl ls-add ls
> +ovn-nbctl --wait=sb add Logical-Switch ls other_config vlan-passthru=true
> +for i in 1 2; do
> +ovn-nbctl lsp-add ls lsp$i
> +ovn-nbctl lsp-set-addresses lsp$i f0:00:00:00:00:0$i
> +#ovn-nbctl lsp-set-port-security lsp$i f0:00:00:00:00:0$i
> +done
> +
> +net_add physnet
> +ovs-vsctl add-br br-phys
> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=physnet:br-phys
> +ovn_attach physnet br-phys 192.168.0.1
> +
> +for i in 1 2; do
> +ovs-vsctl add-port br-int vif$i -- set Interface vif$i
> external-i
Re: [ovs-dev] [PATCH ovn v5] northd: Don't poll ovsdb before the connection is fully established
On Tue, Nov 10, 2020 at 7:54 PM Renat Nurgaliyev wrote: > > Set initial SB and NB DBs probe interval to 0 to avoid connection > flapping. > > Before configured in northd_probe_interval value is actually applied > to southbound and northbound database connections, both connections > must be fully established, otherwise ovnnb_db_run() will return > without retrieving configuration data from northbound DB. In cases > when southbound database is big enough, default interval of 5 seconds > will kill and retry the connection before it is fully established, no > matter what is set in northd_probe_interval. Client reconnect will > cause even more load to ovsdb-server and cause cascade effect, so > northd can never stabilise. We have more than 2000 ports in our lab, > and northd could not start before this patch, holding at 100% CPU > utilisation both itself and ovsdb-server. > > After connections are established, any value in northd_probe_interval, > or default DEFAULT_PROBE_INTERVAL_MSEC is applied correctly. > > Signed-off-by: Renat Nurgaliyev > --- > v2: > - Resubmitting patch because git am failed last time > v3: > - Resubmitting patch because mail client broke formatting > v4: > - Resubmitting patch because mail client broke formatting once again. > I switched to mutt and it won't happen again, I promise guys. > v5: > - Fixed signoff. > --- Thanks. I applied this patch to master. Numan > northd/ovn-northd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 684c2bd47..073c6a0f3 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -100,7 +100,7 @@ static struct eth_addr svc_monitor_mac_ea; > > /* Default probe interval for NB and SB DB connections. */ > #define DEFAULT_PROBE_INTERVAL_MSEC 5000 > -static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC; > -static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC; > +static int northd_probe_interval_nb = 0; > +static int northd_probe_interval_sb = 0; > > #define MAX_OVN_TAGS 4096 > -- > 2.29.2 > > ___ > 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] northd: fix lb_action when there are no active backends for lb health_check
On Wed, Nov 4, 2020 at 4:42 PM Flavio Fernandes wrote:
>
> Tested-by: Flavio Fernandes mailto:fla...@flaviof.com>>
>
>
> > On Nov 3, 2020, at 11:43 AM, Lorenzo Bianconi
> > wrote:
> >
> > Fix the following warning reported by ovn-controller when there are no
> > active backends for lb health_check and selection_fields have been
> > configured for hash computation
> >
> > flow|WARN|error parsing actions "drop;
> > hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");":
> > Syntax error at `hash_fields' expecting end of input.
> >
> > Fixes: 5af304e747 ("Support selection fields in load balancer.")
> > Signed-off-by: Lorenzo Bianconi
Hi Lorenzo,
Would you mind adding a test case for this in ovn-northd.at ?
This would help in future regressions and adding the corresponding
code in northd-ddlog.
Thanks
Numan
> > ---
> > northd/ovn-northd.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 8800a3d3c..88d3e3ed2 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -3598,6 +3598,8 @@ static void build_lb_vip_ct_lb_actions(struct lb_vip
> > *lb_vip,
> >struct ds *action,
> >char *selection_fields)
> > {
> > +bool skip_hash_fields = false;
> > +
> > if (lb_vip->health_check) {
> > ds_put_cstr(action, "ct_lb(backends=");
> >
> > @@ -3616,6 +3618,7 @@ static void build_lb_vip_ct_lb_actions(struct lb_vip
> > *lb_vip,
> > }
> >
> > if (!n_active_backends) {
> > +skip_hash_fields = true;
> > ds_clear(action);
> > ds_put_cstr(action, "drop;");
> > } else {
> > @@ -3626,7 +3629,7 @@ static void build_lb_vip_ct_lb_actions(struct lb_vip
> > *lb_vip,
> > ds_put_format(action, "ct_lb(backends=%s);", lb_vip->backend_ips);
> > }
> >
> > -if (selection_fields && selection_fields[0]) {
> > +if (!skip_hash_fields && selection_fields && selection_fields[0]) {
> > ds_chomp(action, ';');
> > ds_chomp(action, ')');
> > ds_put_format(action, "; hash_fields=\"%s\");", selection_fields);
> > --
> > 2.26.2
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-util: Remove redundant struct v46_ip.
On Thu, Oct 22, 2020 at 7:16 PM Dumitru Ceara wrote:
>
> Use struct in6_addr instead and IPv4-mapped IPs.
>
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2020-October/376160.html
> Fixes: 719f586e4d38 ("northd: Add lflows for IPv6 NAT.")
> Suggested-by: Ilya Maximets
> Signed-off-by: Dumitru Ceara
Thanks Dumitru. I applied this patch to master.
Numan
> ---
> ic/ovn-ic.c | 112
> --
> lib/ovn-util.c| 31 +-
> lib/ovn-util.h| 15 ++-
> northd/ovn-northd.c | 38 -
> utilities/ovn-nbctl.c | 4 +-
> 5 files changed, 99 insertions(+), 101 deletions(-)
>
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 923969f..ba28bc9 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -817,9 +817,9 @@ struct ic_router_info {
> /* Represents an interconnection route entry. */
> struct ic_route_info {
> struct hmap_node node;
> -struct v46_ip prefix;
> +struct in6_addr prefix;
> unsigned int plen;
> -struct v46_ip nexthop;
> +struct in6_addr nexthop;
>
> /* Either nb_route or nb_lrp is set and the other one must be NULL.
> * - For a route that is learned from IC-SB, or a static route that is
> @@ -832,23 +832,23 @@ struct ic_route_info {
> };
>
> static uint32_t
> -ic_route_hash(const struct v46_ip *prefix, unsigned int plen,
> - const struct v46_ip *nexthop)
> +ic_route_hash(const struct in6_addr *prefix, unsigned int plen,
> + const struct in6_addr *nexthop)
> {
> uint32_t basis = hash_bytes(prefix, sizeof *prefix, (uint32_t)plen);
> return hash_bytes(nexthop, sizeof *nexthop, basis);
> }
>
> static struct ic_route_info *
> -ic_route_find(struct hmap *routes, const struct v46_ip *prefix,
> - unsigned int plen, const struct v46_ip *nexthop)
> +ic_route_find(struct hmap *routes, const struct in6_addr *prefix,
> + unsigned int plen, const struct in6_addr *nexthop)
> {
> struct ic_route_info *r;
> uint32_t hash = ic_route_hash(prefix, plen, nexthop);
> HMAP_FOR_EACH_WITH_HASH (r, node, hash, routes) {
> -if (ip46_equals(&r->prefix, prefix) &&
> +if (ipv6_addr_equals(&r->prefix, prefix) &&
> r->plen == plen &&
> -ip46_equals(&r->nexthop, nexthop)) {
> +ipv6_addr_equals(&r->nexthop, nexthop)) {
> return r;
> }
> }
> @@ -870,8 +870,8 @@ ic_router_find(struct hmap *ic_lrs, const struct
> nbrec_logical_router *lr)
>
> static bool
> parse_route(const char *s_prefix, const char *s_nexthop,
> -struct v46_ip *prefix, unsigned int *plen,
> -struct v46_ip *nexthop)
> +struct in6_addr *prefix, unsigned int *plen,
> +struct in6_addr *nexthop)
> {
> if (!ip46_parse_cidr(s_prefix, prefix, plen)) {
> return false;
> @@ -886,7 +886,7 @@ static bool
> add_to_routes_learned(struct hmap *routes_learned,
>const struct nbrec_logical_router_static_route
> *nb_route)
> {
> -struct v46_ip prefix, nexthop;
> +struct in6_addr prefix, nexthop;
> unsigned int plen;
> if (!parse_route(nb_route->ip_prefix, nb_route->nexthop,
> &prefix, &plen, &nexthop)) {
> @@ -903,62 +903,60 @@ add_to_routes_learned(struct hmap *routes_learned,
> }
>
> static bool
> -get_nexthop_from_lport_addresses(int family,
> +get_nexthop_from_lport_addresses(bool is_v4,
> const struct lport_addresses *laddr,
> - struct v46_ip *nexthop)
> + struct in6_addr *nexthop)
> {
> -memset(nexthop, 0, sizeof *nexthop);
> -nexthop->family = family;
> -if (family == AF_INET) {
> +if (is_v4) {
> if (!laddr->n_ipv4_addrs) {
> return false;
> }
> -nexthop->ipv4 = laddr->ipv4_addrs[0].addr;
> +in6_addr_set_mapped_ipv4(nexthop, laddr->ipv4_addrs[0].addr);
> return true;
> }
>
> /* ipv6 */
> if (laddr->n_ipv6_addrs) {
> -nexthop->ipv6 = laddr->ipv6_addrs[0].addr;
> +*nexthop = laddr->ipv6_addrs[0].addr;
> return true;
> }
>
> /* ipv6 link local */
> -in6_generate_lla(laddr->ea, &nexthop->ipv6);
> +in6_generate_lla(laddr->ea, nexthop);
> return true;
> }
>
> static bool
> -prefix_is_link_local(struct v46_ip *prefix, unsigned int plen)
> +prefix_is_link_local(struct in6_addr *prefix, unsigned int plen)
> {
> -if (prefix->family == AF_INET) {
> +if (IN6_IS_ADDR_V4MAPPED(prefix)) {
> /* Link local range is "169.254.0.0/16". */
> if (plen < 16) {
> return false;
> }
> ovs_be32 lla;
> inet_pton(AF_INET, "169.254.0.0", &lla);
> -return ((prefix->ipv4 & htonl(0x)) == lla);
> +return ((in6_addr_get_mapped_ipv4(prefix) & htonl(0x
Re: [ovs-dev] [PATCH ovn] actions: Fix issues with fwd_group action.
On Wed, Nov 11, 2020 at 6:49 AM Ben Pfaff wrote:
>
> The example for the fwd_group action was obviously wrong (it needed a
> closing parenthesis and semicolon) and then when I looked closer I saw
> other issues:
>
> - It's weird to insist on "true" or "false" in quotation marks (why?)
> and the example didn't show that.
>
> - The example didn't show that port names need to be quoted.
>
> - The implementation didn't allow specifying liveness=false (why?)
> or liveness="false" and the test even checked for that.
>
> This fixes all that stuff. For backward compatibility, it still accepts
> "true" (and "false") in quotation marks, but now accepts without as well
> for future use.
>
> Signed-off-by: Ben Pfaff
Oops. Thanks for fixing this.
Acked-by: Numan Siddique
Numan
> ---
> include/ovn/lex.h | 1 +
> lib/actions.c | 14 --
> lib/lex.c | 13 +
> ovn-sb.xml| 21 -
> tests/ovn.at | 14 +++---
> 5 files changed, 45 insertions(+), 18 deletions(-)
>
> diff --git a/include/ovn/lex.h b/include/ovn/lex.h
> index 1da6ccc8c021..ecb7ace24326 100644
> --- a/include/ovn/lex.h
> +++ b/include/ovn/lex.h
> @@ -137,6 +137,7 @@ enum lex_type lexer_lookahead(const struct lexer *);
> bool lexer_match(struct lexer *, enum lex_type);
> bool lexer_force_match(struct lexer *, enum lex_type);
> bool lexer_match_id(struct lexer *, const char *id);
> +bool lexer_match_string(struct lexer *, const char *s);
> bool lexer_is_int(const struct lexer *);
> bool lexer_get_int(struct lexer *, int *value);
> bool lexer_force_int(struct lexer *, int *value);
> diff --git a/lib/actions.c b/lib/actions.c
> index 23e54ef2a6c4..6300fef2c48e 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -3344,15 +3344,17 @@ parse_fwd_group_action(struct action_context *ctx)
> if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
> return;
> }
> -if (ctx->lexer->token.type != LEX_T_STRING) {
> +if (lexer_match_string(ctx->lexer, "true") ||
> +lexer_match_id(ctx->lexer, "true")) {
> +liveness = true;
> +} else if (lexer_match_string(ctx->lexer, "false") ||
> + lexer_match_id(ctx->lexer, "false")) {
> +liveness = false;
> +} else {
> lexer_syntax_error(ctx->lexer,
> - "expecting true/false");
> + "expecting true or false");
> return;
> }
> -if (!strcmp(ctx->lexer->token.s, "true")) {
> -liveness = true;
> -lexer_get(ctx->lexer);
> -}
> lexer_force_match(ctx->lexer, LEX_T_COMMA);
> }
> if (lexer_match_id(ctx->lexer, "childports")) {
> diff --git a/lib/lex.c b/lib/lex.c
> index 4d921998877e..c84d52aa8d1d 100644
> --- a/lib/lex.c
> +++ b/lib/lex.c
> @@ -906,6 +906,19 @@ lexer_match_id(struct lexer *lexer, const char *id)
> }
> }
>
> +/* If 'lexer''s current token is the string 's', advances 'lexer' to the next
> + * token and returns true. Otherwise returns false. */
> +bool
> +lexer_match_string(struct lexer *lexer, const char *s)
> +{
> +if (lexer->token.type == LEX_T_STRING && !strcmp(lexer->token.s, s)) {
> +lexer_get(lexer);
> +return true;
> +} else {
> +return false;
> +}
> +}
> +
> bool
> lexer_is_int(const struct lexer *lexer)
> {
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index b1480f218635..482df00777f3 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -2074,23 +2074,26 @@
>
>
>
> -fwd_group(P);
> +fwd_group(liveness=bool,
> childports=port, ...);
>
>
> -Parameters: liveness, list of child ports P.
> +Parameters: optional liveness, either
> +true or false, defaulting to false;
> +childports, a comma-delimited list of strings
> denoting
> +logical ports to load balance across.
>
>
>
> -It load balances traffic to one or more child ports in a
> -logical switch. ovn-controller translates the
> -fwd_group into openflow group with one bucket
> -for each child port. If liveness is set to true, it also
> -integrates the bucket selection with BFD status on the tunnel
> +Load balance traffic to one or more child ports in a logical
> +switch. ovn-controller translates the
> +fwd_group into an OpenFlow group with one bucket for
> +each child port. If liveness=true is specified, it
> +also integrates the bucket selection with BFD status on the
> tunnel
> interface corresponding to child port.
>
>
> - Example: fwd_group(liveness=true, childports=p1,p2
> -
Re: [ovs-dev] [PATCH ovn] ovn-sb: Fix speling error in documentation.
On Wed, Nov 11, 2020 at 12:47 AM Ben Pfaff wrote: > > Signed-off-by: Ben Pfaff In the commit message should it be - s/speling/spelling ? (or is it correct in U.S english ?) Acked-by: Numan Siddique Thanks Numan > --- > ovn-sb.xml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ovn-sb.xml b/ovn-sb.xml > index b1480f218635..9a49501cb0b9 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -2068,7 +2068,7 @@ >The string should reference a > entry from the > table. The only meter > - that is appriopriate > + that is appropriate >is drop. > > > -- > 2.26.2 > > ___ > 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] Add support for DHCP options 28 (Broadcast Address)
On Wed, Oct 28, 2020 at 7:30 PM wrote:
>
> From: Lucas Alvares Gomes
>
> This patch adds support for the DHCP option 28 (Broadcast Address). This
> options is part of the RFC 2132: https://tools.ietf.org/html/rfc2132
> and can be specified by OpenStack users.
>
> Signed-off-by: Lucas Alvares Gomes
Thanks Lucas. I applied this patch to master.
Numan
> ---
> lib/ovn-l7.h| 3 +++
> northd/ovn-northd.c | 1 +
> ovn-nb.xml | 7 +++
> tests/ovn.at| 6 +++---
> tests/test-ovn.c| 1 +
> 5 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
> index c3e8fd660..aa13fa57a 100644
> --- a/lib/ovn-l7.h
> +++ b/lib/ovn-l7.h
> @@ -93,6 +93,9 @@ struct gen_opts_map {
> #define DHCP_OPT_DOMAIN_SEARCH_LIST \
> DHCP_OPTION("domain_search_list", 119, "domains")
>
> +#define DHCP_OPT_BROADCAST_ADDRESS \
> +DHCP_OPTION("broadcast_address", 28, "ipv4")
> +
> #define DHCP_OPT_BOOTFILE_CODE 67
>
> /* Use unused 254 option for iPXE bootfile_name_alt userdata DHCP option.
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 8800a3d3c..16d19714a 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -12483,6 +12483,7 @@ static struct gen_opts_map supported_dhcp_opts[] = {
> DHCP_OPT_TCP_KEEPALIVE_INTERVAL,
> DHCP_OPT_DOMAIN_SEARCH_LIST,
> DHCP_OPT_BOOTFILE_ALT,
> +DHCP_OPT_BROADCAST_ADDRESS,
> };
>
> static struct gen_opts_map supported_dhcpv6_opts[] = {
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 5e8635992..d0d64d857 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -3127,6 +3127,13 @@
> dhcp request contains etherboot option (175), otherwise
> "bootfile_name_alt" will be used.
>
> +
> +
> +
> +The DHCPv4 option code for this option is 28. This option
> +specifies the IP address used as a broadcast address.
> +
> +
>
>
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 04b7a3df7..db4cfcd4d 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1284,9 +1284,9 @@ reg2[5] =
> put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.254.0,m
> reg0[15] =
> put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.255.0,mtu=1400,ip_forward_enable=1,default_ttl=121,dns_server={8.8.8.8,7.7.7.7},classless_static_route={30.0.0.0/24,10.0.0.4,40.0.0.0/16,10.0.0.6,0.0.0.0/0,10.0.0.1},ethernet_encap=1,router_discovery=0,tftp_server_address={10.0.0.4,10.0.0.5},arp_cache_timeout=10,tcp_keepalive_interval=10);
> formats as reg0[15] = put_dhcp_opts(offerip = 10.0.0.4, router =
> 10.0.0.1, netmask = 255.255.255.0, mtu = 1400, ip_forward_enable = 1,
> default_ttl = 121, dns_server = {8.8.8.8, 7.7.7.7}, classless_static_route =
> {30.0.0.0/24, 10.0.0.4, 40.0.0.0/16, 10.0.0.6, 0.0.0.0/0, 10.0.0.1},
> ethernet_encap = 1, router_discovery = 0, tftp_server_address = {10.0.0.4,
> 10.0.0.5}, arp_cache_timeout = 10, tcp_keepalive_interval = 10);
> encodes as
> controller(userdata=00.00.00.02.00.00.00.00.00.01.de.10.00.00.00.6f.0a.00.00.04.03.04.0a.00.00.01.01.04.ff.ff.ff.00.1a.02.05.78.13.01.01.17.01.79.06.08.08.08.08.08.07.07.07.07.79.14.18.1e.00.00.0a.00.00.04.10.28.00.0a.00.00.06.00.0a.00.00.01.24.01.01.1f.01.00.96.08.0a.00.00.04.0a.00.00.05.23.04.00.00.00.0a.26.04.00.00.00.0a,pause)
> -reg0[15] =
> put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.255.0,mtu=1400,ip_forward_enable=1,default_ttl=121,dns_server={8.8.8.8,7.7.7.7},classless_static_route={30.0.0.0/24,10.0.0.4,40.0.0.0/16,10.0.0.6,0.0.0.0/0,10.0.0.1},ethernet_encap=1,router_discovery=0,tftp_server=10.0.0.10);
> -formats as reg0[15] = put_dhcp_opts(offerip = 10.0.0.4, router =
> 10.0.0.1, netmask = 255.255.255.0, mtu = 1400, ip_forward_enable = 1,
> default_ttl = 121, dns_server = {8.8.8.8, 7.7.7.7}, classless_static_route =
> {30.0.0.0/24, 10.0.0.4, 40.0.0.0/16, 10.0.0.6, 0.0.0.0/0, 10.0.0.1},
> ethernet_encap = 1, router_discovery = 0, tftp_server = 10.0.0.10);
> -encodes as
> controller(userdata=00.00.00.02.00.00.00.00.00.01.de.10.00.00.00.6f.0a.00.00.04.03.04.0a.00.00.01.01.04.ff.ff.ff.00.1a.02.05.78.13.01.01.17.01.79.06.08.08.08.08.08.07.07.07.07.79.14.18.1e.00.00.0a.00.00.04.10.28.00.0a.00.00.06.00.0a.00.00.01.24.01.01.1f.01.00.42.04.0a.00.00.0a,pause)
> +reg0[15] =
> put_dhcp_opts(offerip=10.0.0.4,router=10.0.0.1,netmask=255.255.255.0,mtu=1400,ip_forward_enable=1,default_ttl=121,dns_server={8.8.8.8,7.7.7.7},classless_static_route={30.0.0.0/24,10.0.0.4,40.0.0.0/16,10.0.0.6,0.0.0.0/0,10.0.0.1},ethernet_encap=1,router_discovery=0,tftp_server=10.0.0.10,broadcast_address=255.255.255.255);
> +formats as reg0[15] = put_dhcp_opts(offerip = 10.0.0.4, router =
> 10.0.0.1, netmask = 255.255.255.0, mtu = 1400, ip_forward_enable = 1,
> default_ttl = 121, dns_server = {8.8.8.8, 7.7.7.7}, classless_static_route =
> {30.0.0.0/24, 10.0.0.4, 40.0.0.0/16, 10.0.0.6, 0.0.0.0/0, 10.0.0.1},
> ethern
