Re: [ovs-dev] [PATCH OVN] ovndb-ctl:probe interval of ovndb-ctl daemon
Hi, Ilya,Numan
I fully understand. I did miss the "RECONNECT_DEFAULT_PROBE_INTERVAL"。
In my scale test，probe interval of ovn-dbctl is 60 seconds，default probe
interval（5 seconds）is not large enough。
I only want to specify the probe interval value of ovn-dbctl，"adding a command
line option" is a good way
Thanks for your reply
Best regards, Wentao Jia
>>
>> Hi, Ilya
>>
>> Initially, ovndb-ctl command is a program executed only once，probe interval
>> is not be set。
>> ovndb-ctl daemon mode is a longrun program，so we need to set probe interval。
>
>I understand that. But,
>"not set" should be equal to "set to default 5 seconds".
>I'm not sure why we need to call ovsdb_idl_set_probe_interval
>explicitly if the default 5 seconds should already be configured.
>That is my question.
>
>We should not need to call ovsdb_idl_set_probe_interval to
>have 5 second inactivity probe interval. So, I'm trying to
>understand why this is needed in your case.
>
>The only reason I can think of is the unix: connection method
>that may not have inactivity probe enabled by default. But
>unix sockets are typically good at detecting broken connections.
>
>Best regards, Ilya Maximets.
>
>>
>> Best regards, Wentao Jia
>>
>>
>
> ovndb-ctl deamon mode, the connection cannot be reconnected when
> connection is broken, set inactivity probe interval to make it
> reconnected
>>>
>>>Hi, Wentao, Numan.
>>>
>>>Could you, please, elaborate, why exactly connection can not be
>>>reconnected? AFAICT, ovsdb_idl_set_probe_interval() only updates
>>>the fsm->probe_interval that should already be equal to 5 seconds
>>>by default (RECONNECT_DEFAULT_PROBE_INTERVAL).
>>>So, why do we need to set it from the application code? Am I missing
>>>something?
>>>
>>>Best regards, Ilya Maximets.
>>>
>
>
> Signed-off-by: Wentao Jia
Thanks for the patch.
I see one issue with using the default probe interval value of 5
seconds. If the database is huge,
5 seconds may not be enough and the dbctl daemons/utilities could be
in infinite loop connecting/disconnecting
due to the probe interval time out.
Instead, I'd suggest adding a command line option so that users can
specify the probe interval value to use
(with the default value set to 0 if not specified).
Like ovn-nbctl --probe-interval=1 ...
Thanks
Numan
> ---
> utilities/ovn-dbctl.c | 4
> 1 file changed, 4 insertions(+)
>
>
> diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c
> index 791caabb2..2080e8ba0 100644
> --- a/utilities/ovn-dbctl.c
> +++ b/utilities/ovn-dbctl.c
> @@ -109,6 +109,9 @@ static void server_loop(const struct
> ovn_dbctl_options *dbctl_options,
> struct ovsdb_idl *idl, int argc, char *argv[]);
> static void ovn_dbctl_exit(int status);
>
> +/* Default probe interval for NB and SB DB connections. */
> +#define DEFAULT_PROBE_INTERVAL_MSEC 5000
> +
> int
> ovn_dbctl_main(int argc, char *argv[],
> const struct ovn_dbctl_options *dbctl_options)
> @@ -191,6 +194,7 @@ ovn_dbctl_main(int argc, char *argv[],
> /* "retry" is true iff in daemon mode. */
> ovsdb_idl_set_remote(idl, db, daemon_mode);
> ovsdb_idl_set_leader_only(idl, leader_only);
> +ovsdb_idl_set_probe_interval(idl, DEFAULT_PROBE_INTERVAL_MSEC);
>
> if (daemon_mode) {
> server_loop(dbctl_options, idl, argc, argv_);
> --
> 2.32.0
>>>
>>
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Undefined Behavior sanitizer findings in Open vSwitch 2.16.2
Hi Everyone, I am testing Open vSwitch 2.16.2. Undefined Behavior sanitizer is producing some findings. My question is, is the undefined behavior something the project would be interested in fixing? If so, I can send over the findings and a proposed patch. If not, I can carry the patch privately. Jeff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] RFC for adding P4 Support in OVS
Hi, We are working on adding P4 support to OVS and we have recently open-sourced the patches on the IPDK github - https://github.com/ipdk-io/ovs/tree/ovs-with-p4 The architecture document that explains the changes and the upcoming feature list is here - https://github.com/ipdk-io/ovs/blob/ovs-with-p4/OVS_WITH_P4_ARCH.rst Here is the link for the user-guide document to run OVS on host - https://github.com/ipdk-io/ipdk/blob/main/build/ovs-with-p4_howto We have also built a development container that ties in all necessary pieces and runs OVS on container. It also brings up 2 Vagrant VMs and switches traffic between them using a simple P4 and OVS - https://github.com/ipdk-io/ipdk/blob/main/build/README.md Here is the link to the code-walk video that we did last November - https://www.youtube.com/watch?v=vhRL5SQReQs We still have work to do to turn these patches into a minimal set that could be upstreamed into OVS. Once that is done, we would like to upstream these patches to openvswitch. Please let us know your comments. Thanks Namrata ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2] Add LTS section to release documentation.
OVN LTS releases have a lot of ambiguity, so this is intended to codify LTS support and cadence. Signed-off-by: Mark Michelson --- Documentation/internals/release-process.rst | 28 + 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/Documentation/internals/release-process.rst b/Documentation/internals/release-process.rst index f37c09e51..9db6e7494 100644 --- a/Documentation/internals/release-process.rst +++ b/Documentation/internals/release-process.rst @@ -75,16 +75,24 @@ Scheduling`_ for the timing of each stage: 2019.10.2, and so on. The process is the same for these additional release as for a .0 release. -At most two release branches are formally maintained at any given time: the -latest release and the latest release designed as LTS. An LTS release is one -that the OVN project has designated as being maintained for a longer period of -time. Currently, an LTS release is maintained until the next LTS is chosen. -There is not currently a strict guideline on how often a new LTS release is -chosen, but so far it has been about every 2 years. That could change based on -the current state of OVN development. For example, we do not want to designate -a new release as LTS that includes disruptive internal changes, as that may -make it harder to support for a longer period of time. Discussion about -choosing the next LTS release occurs on the OVS development mailing list. +Long-term Support Releases +-- + +The OVN project will periodically designate a release as "long-term support" or +LTS for short. An LTS release has the distinction of being maintained for +longer than a standard release. + +LTS releases will receive bug fixes until the point that another LTS is +released. At that point, the old LTS will receive an additional year of +critical and security fixes. Critical fixes are those that are required to +ensure basic operation (e.g. memory leak fixes, crash fixes). Security fixes +are those that address concerns about exploitable flaws in OVN and that have a +corresponding CVE report. + +LTS releases are scheduled to be released once every two years. This means +that any given LTS will receive bug fix support for two years, followed by +one year of critical bug fixes and security fixes. + Release Numbering - -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVN release proposals 3/3: Make 22.03 release series an LTS
On Mon, Nov 22, 2021 at 11:00 PM Mark Michelson wrote: > > Development in the 21.09 cycle of OVN saw an unprecedented push towards > performance and scalability improvements. Development in the 21.12 cycle > has seen stability enhancements and even more performance improvements. > It's reached the point where I think it would be safe (and a good idea) > to label 22.03 as an LTS release. > > As a reminder of what an LTS release is: > "An LTS release is one that the OVN project has designated as being > maintained for a longer period of time. Currently, an LTS release is > maintained until the next LTS is chosen." > > This means that we would continue to maintain 22.03 even after the > following release is made. It's unknown how long the support will go, > since it is unknown which release will be the next LTS. The next LTS > will likely be chosen when some milestone or milestones have been > reached, and those milestones have had time to stabilize. I suspect this > will require us to support 22.03 for multiple years. > > The rationale for this proposal is to give users a version they can feel > safe using for multiple years, receiving bug fixes during that whole time. > > Please respond with your thoughts on this proposal. We welcome the proposal of naming an OVN LTS release, and as it happens 22.03 is a very good fit for us as we're starting the Ubuntu LTS cycle at 22.04. So +1 for the proposal of naming 22.03 an LTS. I think it would be preferable to have new LTS's come on a cadence, but we are already discussing that in a separate thread. -- Frode Nordahl > ___ > 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
[ovs-dev] [PATCH ovn] ovn-parallel-hmap: Fix NUMA and core detection.
ovn-parallel-hmap attempted to determine the number of threads to
allocate for its pool based on the NUMA and core count reported by OVS.
The problem is that since ovn-northd never called ovs_numa_init(), OVS
would always return OVS_NUMA_UNSPEC for the number of NUMAs, and
OVS_CORE_UNSPEC for the number of cores. Parallelization still was
operational because we would fall back to a CPU core count. On a single
NUMA machine, this is identical to what was expected, so everything
seemed fine.
In 37ad427cfb78a9e59d7e7ef249b2f2b3ac68c65a, the --dumy-numa option was
added to ovn-northd to allow for parallel operation on machines with
fewer NUMAs and cores than the parallelization code otherwise would
require. However, because of the missing call to ovs_numa_init(), this
option did not function as intended.
This commit adds a call to ovs_numa_init() when setting up the parallel
hmap thread pool so that accurate NUMA and core counts are reported, and
dummy NUMA and core counts can be used.
The referenced bugzilla is not actually reporting this as an issue, but
this particular fix will enable the functionality requested.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1975345
Signed-off-by: Mark Michelson
---
lib/ovn-parallel-hmap.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/ovn-parallel-hmap.c b/lib/ovn-parallel-hmap.c
index 56ceed8e8..7edc4c0b6 100644
--- a/lib/ovn-parallel-hmap.c
+++ b/lib/ovn-parallel-hmap.c
@@ -404,6 +404,7 @@ static void
setup_worker_pools(bool force) {
int cores, nodes;
+ovs_numa_init();
nodes = ovs_numa_get_n_numas();
if (nodes == OVS_NUMA_UNSPEC || nodes <= 0) {
nodes = 1;
--
2.31.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVN release proposals 2/3: Monthly point releases
On Mon, Nov 22, 2021 at 10:59 PM Mark Michelson wrote: > > Currently, we tend to release a .0 version of OVN. It's rare to release > any follow-up point releases unless there is a major regression or if a > "critical" new feature barely missed the soft-freeze deadline. > > Since it is rare to have point releases, it sometimes seems > pointless(ha!) to backport changes to the current supported release > branch, because those changes never actually get released. > > My proposal is to have monthly point releases. In other words, if we > were to release 22.03.0 in March, then in April, we would create a > 22.03.1 release, in May we would create a 22.03.2 release, and in June > we would create a 22.03.3 release alongside the 22.06.0 release. > Assuming proposal 1 does not pass, this would be the end. But if > proposal 1 passes, then we would continue to release 22.03.4 in July, > 22.03.5 in August, and 22.03.6 alongside the 22.09.0 release in > September. For LTS releases (of which there are not any yet), the point > releases would continue until the end of their support is reached. > > The goal of this is to ensure that users of a specific release actually > receive updates instead of feeling compelled to update before they are > ready. It also may motivate developers to ensure that bug fixes get > backported to the current release version. > > Please respond with your thoughts on this proposal. With the software distribution hat on, this proposal speaks directly to some of our pain points and I think what you are proposing would help solve that. The rigor of such a schedule would help ensure the quality of what gets into point releases since all the proposed stable changes are tested together in the upstream gate, as well as in the downstream stable release update process which is triggered on availability of upstream point releases. It also becomes of increased importance if we go to semi-annual releases as proposed in your previous e-mail. +1 from me -- Frode Nordahl > ___ > 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] OVN release proposals 1/3: Semi-annual releases
Mark, Thank you for putting this series of thoughts on OVN release strategy forward, and apologies for the tardy response to them. On Mon, Nov 22, 2021 at 10:59 PM Mark Michelson wrote: > > For the past two years, OVN has had quarterly releases. This allowed for > new features to be released more rapidly than if OVN were released at a > slower pace. During 2021, there were two trends: It has indeed been two eventful years! > 1) "Small" new features were less frequent. Most features that added in > the past couple of years (e.g. incremental processing in ovn-controller, > ovn-northd-ddlog, pluggable architecture, etc.) are large features that > could benefit from longer development times. > 2) A lot of newer development focuses on scalability and performance > rather than adding bells and whistles. > > For this reason, I propose the following: Let's switch from quarterly > releases to semi-annual releases, like OVS does. The OVS release schedule does work quite well, we build and distribute these two projects together, so re-aligning their release schedules after this two year period with a flurry of development does make a lot of sense to me. > The upside is that releases will be easier to manage, and developers > will have more lead time to develop larger features. In doing so, we > would double our current soft-freeze and hard-freeze periods for releases. I can relate to that, having missed a couple of self imposed release targets. > The downside is that users will need to wait longer for new features to > be released. At this stage of development I do not think this will be a problem, if something should fall into this category, chances are it can also be labeled as a bug and be brought to the world through a backport and subsequent point release. > Please respond with your thoughts on this proposal. +1 from me -- Frode Nordahl > ___ > 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
[ovs-dev] [PATCH ovn] ovn-controller: Avoid reprocessing same lflows in the same I-P run.
For I-P node lflow_output, different change handlers can trigger
reprocessing lflows. However, it is a waste to reprocess the same lflow
multiple times in the same run, because each processing of the lflow is
against the same input data.
For example, if a lflow references an addresset A and a port-group P, it
is possible that both A and P are changed in the same run, the same
lflow reprocess would be triggered by both
lflow_output_addr_sets_handler() and lflow_output_port_groups_handler().
Another example may incur a even bigger waste is that when a NB port-group
include ports across a big number of datapaths, so the lflow is
referencing a big number of SB port-groups with DP group including all
those DPs. When there are multiple port changes in the NB port-group,
there can be multiple small SB port-group changes, and so the lflows can
be reprocessed multiple times.
This patch avoid reprocessing the same lflow in the same I-P run, which
should improve performance in above scenarios.
There is only one exception when a lflow may still be reprocessed more
than once: if a lflow A is processed, which results in a compound
conjunction added to an existed flow generated by an exited lflow B, and
then lflow B needs to be reprocessed in the same run, which would
cause flood-remove and reprocess lflow A. In this case lflow A is
processed twice.
Apart from the performance gains, there is also a potential problem of
DP group fixed by this patch. If there is addrset/pg change and at the
same run there is also a new local datapath monitored, then the existed
code would firstly handle the addrset/pg changes causing a lflow being
processed against the DP group including the new DP, which could have
conjunction flows, but later the same lflow is reprocessed by
lflow_output_runtime_data_handler()->lflow_add_flows_for_datapath() for
the new DP only. Because lflows adding conjunction flows will not be
checked against redundancy but only tries to combine the conjunction
actions, it would result in redundanct conjunction actions added to the
same flow, which is also linked to the same SB lflow twice. The
mechanism of this patch will avoid this problem.
Signed-off-by: Han Zhou
---
controller/lflow.c | 143 +++-
controller/lflow.h | 7 ++
controller/ovn-controller.c | 18 -
3 files changed, 147 insertions(+), 21 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index 933e2f3cc..b976c7d56 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -78,8 +78,16 @@ consider_logical_flow(const struct sbrec_logical_flow *lflow,
struct hmap *dhcp_opts, struct hmap *dhcpv6_opts,
struct hmap *nd_ra_opts,
struct controller_event_options *controller_event_opts,
+ bool is_recompute,
struct lflow_ctx_in *l_ctx_in,
struct lflow_ctx_out *l_ctx_out);
+static struct lflow_processed_node *
+lflows_processed_find(struct hmap *lflows_processed,
+ const struct uuid *lflow_uuid);
+static void lflows_processed_add(struct hmap *lflows_processed,
+ const struct uuid *lflow_uuid);
+static void lflows_processed_remove(struct hmap *lflows_processed,
+struct lflow_processed_node *node);
static void lflow_resource_add(struct lflow_resource_ref *, enum ref_type,
const char *ref_name, const struct uuid *);
static struct ref_lflow_node *ref_lflow_lookup(struct hmap *ref_lflow_table,
@@ -366,7 +374,7 @@ add_logical_flows(struct lflow_ctx_in *l_ctx_in,
SBREC_LOGICAL_FLOW_TABLE_FOR_EACH (lflow, l_ctx_in->logical_flow_table) {
consider_logical_flow(lflow, _opts, _opts,
- _ra_opts, _event_opts,
+ _ra_opts, _event_opts, true,
l_ctx_in, l_ctx_out);
}
@@ -413,6 +421,12 @@ lflow_handle_changed_flows(struct lflow_ctx_in *l_ctx_in,
struct ofctrl_flood_remove_node *ofrn, *next;
SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow,
l_ctx_in->logical_flow_table) {
+if (lflows_processed_find(l_ctx_out->lflows_processed,
+ >header_.uuid)) {
+VLOG_DBG("lflow "UUID_FMT"has been processed, skip.",
+ UUID_ARGS(>header_.uuid));
+continue;
+}
VLOG_DBG("delete lflow "UUID_FMT, UUID_ARGS(>header_.uuid));
ofrn = xmalloc(sizeof *ofrn);
ofrn->sb_uuid = lflow->header_.uuid;
@@ -437,8 +451,20 @@ lflow_handle_changed_flows(struct lflow_ctx_in *l_ctx_in,
if (lflow) {
VLOG_DBG("re-add lflow "UUID_FMT,
UUID_ARGS(>header_.uuid));
+
+/* For the extra lflows that need to be reprocessed because of the
+ * flood
[ovs-dev] [PATCH] ovs-monitor-ipsec: Add force-encapsulation option to force NAT-T
Both LibreSwan and OpenSwan allow administrators to unconditionally
force enable NAT-T for ESP. This may help to surmount restrictive
firewalls in scenarios where IP protocol number 50 is blocked, but where
NAT autodetection fails. Add a switch --force-encapsulation to expose
this feature to users of ovs-monitor-ipsec
Signed-off-by: Andreas Karis
---
ipsec/ovs-monitor-ipsec.in | 29 +
utilities/ovs-ctl.in | 7 +++
2 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in
index 89a36fe17..3421adcdb 100755
--- a/ipsec/ovs-monitor-ipsec.in
+++ b/ipsec/ovs-monitor-ipsec.in
@@ -171,8 +171,9 @@ conn %%default
auto=route
ike=aes256gcm16-sha256-modp2048
esp=aes256gcm16-modp2048
+%s
-""" % (FILE_HEADER)
+"""
CA_SECTION = """ca ca_auth
cacert=%s
@@ -219,13 +220,17 @@ conn prevent_unencrypted_vxlan
rightid=$remote_name
leftcert=$certificate""")}
-def __init__(self, root_prefix):
+def __init__(self, root_prefix, args):
self.CHARON_CONF = root_prefix + "/etc/strongswan.d/ovs.conf"
self.IPSEC = root_prefix + "/usr/sbin/ipsec"
self.IPSEC_CONF = root_prefix + "/etc/ipsec.conf"
self.IPSEC_SECRETS = root_prefix + "/etc/ipsec.secrets"
self.conf_file = None
self.secrets_file = None
+if args.force_encapsulation:
+self.extra_params = "forceencaps=yes"
+else:
+self.extra_params = ""
def restart_ike_daemon(self):
"""This function restarts StrongSwan."""
@@ -234,7 +239,7 @@ conn prevent_unencrypted_vxlan
f.close()
f = open(self.IPSEC_CONF, "w")
-f.write(self.CONF_HEADER)
+f.write(self.CONF_HEADER % (FILE_HEADER, self.extra_params))
f.close()
f = open(self.IPSEC_SECRETS, "w")
@@ -274,7 +279,7 @@ conn prevent_unencrypted_vxlan
def config_init(self):
self.conf_file = open(self.IPSEC_CONF, "w")
self.secrets_file = open(self.IPSEC_SECRETS, "w")
-self.conf_file.write(self.CONF_HEADER)
+self.conf_file.write(self.CONF_HEADER % (FILE_HEADER,
self.extra_params))
self.secrets_file.write(FILE_HEADER)
def config_global(self, monitor):
@@ -387,8 +392,9 @@ conn %%default
ike=aes_gcm256-sha2_256
esp=aes_gcm256
ikev2=insist
+%s
-""" % (FILE_HEADER)
+"""
SHUNT_POLICY = """conn prevent_unencrypted_gre
type=drop
@@ -452,6 +458,10 @@ conn prevent_unencrypted_vxlan
else "/etc/ipsec.secrets")
ipsec_ctl = (args.ipsec_ctl if args.ipsec_ctl
else "/run/pluto/pluto.ctl")
+if args.force_encapsulation:
+self.extra_params = "encapsulation=yes"
+else:
+self.extra_params = ""
self.IPSEC = libreswan_root_prefix + "/usr/sbin/ipsec"
self.IPSEC_CONF = libreswan_root_prefix + ipsec_conf
@@ -472,7 +482,7 @@ conn prevent_unencrypted_vxlan
self._nss_clear_database()
f = open(self.IPSEC_CONF, "w")
-f.write(self.CONF_HEADER)
+f.write(self.CONF_HEADER % (FILE_HEADER, self.extra_params))
f.close()
f = open(self.IPSEC_SECRETS, "w")
@@ -485,7 +495,7 @@ conn prevent_unencrypted_vxlan
def config_init(self):
self.conf_file = open(self.IPSEC_CONF, "w")
self.secrets_file = open(self.IPSEC_SECRETS, "w")
-self.conf_file.write(self.CONF_HEADER)
+self.conf_file.write(self.CONF_HEADER % (FILE_HEADER,
self.extra_params))
self.secrets_file.write(FILE_HEADER)
def config_global(self, monitor):
@@ -1012,7 +1022,7 @@ class IPsecMonitor(object):
# Choose to either use StrongSwan or LibreSwan as IKE daemon
if ike_daemon == "strongswan":
-self.ike_helper = StrongSwanHelper(root_prefix)
+self.ike_helper = StrongSwanHelper(root_prefix, args)
elif ike_daemon == "libreswan":
self.ike_helper = LibreSwanHelper(root_prefix, args)
else:
@@ -1284,6 +1294,9 @@ def main():
parser.add_argument("--ipsec-ctl", metavar="IPSEC-CTL",
help="Use DIR/IPSEC-CTL as location for "
" pluto ctl socket (libreswan only).")
+parser.add_argument("--force-encapsulation", action='store_true',
+help="Unconditionally enable ESP NAT-T encapsulation."
+" (either libreswan or strongswan).")
ovs.vlog.add_args(parser)
ovs.daemon.add_args(parser)
diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index e6e07f476..deb715ae5 100644
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -240,11 +240,15 @@ start_ovs_ipsec () {
if test X$RESTART_IKE_DAEMON = Xno; then
no_restart="--no-restart-ike-daemon"
fi
+if test X$FORCE_ENCAPSULATION = Xyes; then
+
[ovs-dev] [PATCH v2 ovn] inc-proc-eng: use VLOG_INFO_RL for recompute time over 500ms
Use VLOG_INFO_RL in engine_recompute and engine_compute routines
to log compute time over a given threshold (i.e. 500ms).
Make timeout threshold configurable through ovs-appctl command:
$ovn-appctl -t ovn-controller inc-engine/compute-log-timeout 100
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1988555
Signed-off-by: Lorenzo Bianconi
---
Changes since v1:
- add inc-engine/compute-log-timeout ovs-appctl command.
---
lib/inc-proc-eng.c | 44 ++--
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c
index 2958a55e3..7b4391700 100644
--- a/lib/inc-proc-eng.c
+++ b/lib/inc-proc-eng.c
@@ -47,6 +47,8 @@ static const char *engine_node_state_name[EN_STATE_MAX] = {
[EN_ABORTED] = "Aborted",
};
+static long long engine_compute_log_timeout_msec = 500;
+
static void
engine_recompute(struct engine_node *node, bool allowed,
const char *reason_fmt, ...) OVS_PRINTF_FORMAT(3, 4);
@@ -152,6 +154,20 @@ engine_trigger_recompute_cmd(struct unixctl_conn *conn,
int argc OVS_UNUSED,
unixctl_command_reply(conn, NULL);
}
+static void
+engine_set_log_timeout_cmd(struct unixctl_conn *conn, int argc OVS_UNUSED,
+ const char *argv[], void *arg OVS_UNUSED)
+{
+
+unsigned int timeout;
+if (!str_to_uint(argv[1], 10, )) {
+unixctl_command_reply_error(conn, "unsigned integer required");
+return;
+}
+engine_compute_log_timeout_msec = timeout;
+unixctl_command_reply(conn, NULL);
+}
+
void
engine_init(struct engine_node *node, struct engine_arg *arg)
{
@@ -172,6 +188,8 @@ engine_init(struct engine_node *node, struct engine_arg
*arg)
engine_clear_stats, NULL);
unixctl_command_register("inc-engine/recompute", "", 0, 0,
engine_trigger_recompute_cmd, NULL);
+unixctl_command_register("inc-engine/compute-log-timeout", "", 1, 1,
+ engine_set_log_timeout_cmd, NULL);
}
void
@@ -361,8 +379,15 @@ engine_recompute(struct engine_node *node, bool allowed,
long long int now = time_msec();
node->run(node, node->data);
node->stats.recompute++;
-VLOG_DBG("node: %s, recompute (%s) took %lldms", node->name, reason,
- time_msec() - now);
+long long int delta_time = time_msec() - now;
+if (delta_time > engine_compute_log_timeout_msec) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(20, 10);
+VLOG_INFO_RL(, "node: %s, recompute (%s) took %lldms", node->name,
+ reason, delta_time);
+} else {
+VLOG_DBG("node: %s, recompute (%s) took %lldms", node->name, reason,
+ delta_time);
+}
done:
free(reason);
}
@@ -379,10 +404,17 @@ engine_compute(struct engine_node *node, bool
recompute_allowed)
*/
long long int now = time_msec();
bool handled = node->inputs[i].change_handler(node, node->data);
-
-VLOG_DBG("node: %s, handler for input %s took %lldms",
- node->name, node->inputs[i].node->name,
- time_msec() - now);
+long long int delta_time = time_msec() - now;
+if (delta_time > engine_compute_log_timeout_msec) {
+static struct vlog_rate_limit rl =
+VLOG_RATE_LIMIT_INIT(20, 10);
+VLOG_INFO_RL(, "node: %s, handler for input %s took %lldms",
+ node->name, node->inputs[i].node->name,
+ delta_time);
+} else {
+VLOG_DBG("node: %s, handler for input %s took %lldms",
+ node->name, node->inputs[i].node->name, delta_time);
+}
if (!handled) {
engine_recompute(node, recompute_allowed,
"failed handler for input %s",
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] system-dpdk: Fix MFEX logs check.
> > -Original Message-
> > From: David Marchand
> > Sent: Wednesday 19 January 2022 17:26
> > To: d...@openvswitch.org
> > Cc: Stokes, Ian ; Phelan, Michael
> ; Ferriter, Cian
> > ; i.maxim...@ovn.org; ktray...@redhat.com
> > Subject: [PATCH] system-dpdk: Fix MFEX logs check.
> >
> > Some warning logs must be waived when using the net/pcap DPDK driver.
> > Those logs can affect different DPDK drivers (like mlx5) and the tests in
> > system-dpdk are not testing MTU and Rx checksum, we might as well ignore
> > those warnings from OVS.
> >
> > Fixes: d446dcb7e03f ("system-dpdk: Refactor common logs matching.")
> > Signed-off-by: David Marchand
>
> Reviewed and tested. This fixes the errors I see prior to applying the patch.
> The
> MFEX tests now pass:
>
> OVS-DPDK unit tests
>
> 6: OVS-DPDK - MFEX Autovalidator ok
> 7: OVS-DPDK - MFEX Autovalidator Fuzzy ok
> 8: OVS-DPDK - MFEX Configuration ok
>
> Acked-by: Cian Ferriter
Thanks David, Eelco & Cian, applied to master and 2.17.
Thanks
Ian
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 ovn] ovn-nbctl: report peer addresses running lsp-get-addresses for patch ports
Report logical router port addresses running lsp-get-addresses if the
logical switch port address is set to "router".
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2028040
Signed-off-by: Lorenzo Bianconi
---
Changes since v1:
- rely on lrp_by_name_or_uuid
---
tests/ovn-nbctl.at| 10 ++
utilities/ovn-nbctl.c | 29 +
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index a43a1ce8f..539a121c0 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -181,6 +181,16 @@ unknown
AT_CHECK([ovn-nbctl lsp-set-addresses lp0])
AT_CHECK([ovn-nbctl lsp-get-addresses lp0], [0], [dnl
+])
+
+AT_CHECK([ovn-nbctl lr-add lr0])
+AT_CHECK([ovn-nbctl lrp-add lr0 rp-ls0 aa:bb:bb:00:00:01 192.168.0.1/24])
+AT_CHECK([ovn-nbctl lsp-add ls0 ls0-rp])
+AT_CHECK([ovn-nbctl lsp-set-addresses ls0-rp router])
+AT_CHECK([ovn-nbctl lsp-set-type ls0-rp router])
+AT_CHECK([ovn-nbctl lsp-set-options ls0-rp router-port=rp-ls0])
+AT_CHECK([ovn-nbctl lsp-get-addresses ls0-rp], [0], [dnl
+aa:bb:bb:00:00:01 192.168.0.1/24
])])
dnl -
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 55b0f5124..f3209417a 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -1494,8 +1494,18 @@ nbctl_pre_lsp_get_addresses(struct ctl_context *ctx)
{
ovsdb_idl_add_column(ctx->idl, _logical_switch_port_col_name);
ovsdb_idl_add_column(ctx->idl, _logical_switch_port_col_addresses);
+ovsdb_idl_add_column(ctx->idl, _logical_switch_port_col_options);
+
+ovsdb_idl_add_column(ctx->idl, _logical_router_col_ports);
+ovsdb_idl_add_column(ctx->idl, _logical_router_port_col_name);
+ovsdb_idl_add_column(ctx->idl, _logical_router_port_col_mac);
+ovsdb_idl_add_column(ctx->idl, _logical_router_port_col_networks);
}
+static char * OVS_WARN_UNUSED_RESULT
+lrp_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist,
+const struct nbrec_logical_router_port **lrp_p);
+
static void
nbctl_lsp_get_addresses(struct ctl_context *ctx)
{
@@ -1511,6 +1521,21 @@ nbctl_lsp_get_addresses(struct ctl_context *ctx)
return;
}
+const char *router_port = smap_get(>options, "router-port");
+if (lsp->n_addresses == 1 && !strcmp(lsp->addresses[0], "router") &&
+router_port) {
+const struct nbrec_logical_router_port *lrp;
+error = lrp_by_name_or_uuid(ctx, router_port, false, );
+if (lrp) {
+ds_put_format(>output, "%s", lrp->mac);
+for (size_t j = 0; j < lrp->n_networks; j++) {
+ds_put_format(>output, " %s", lrp->networks[j]);
+}
+ds_put_cstr(>output, "\n");
+return;
+}
+}
+
svec_init();
for (i = 0; i < lsp->n_addresses; i++) {
svec_add(, lsp->addresses[i]);
@@ -4108,10 +4133,6 @@ nbctl_pre_lr_route_add(struct ctl_context *ctx)
_logical_router_static_route_col_external_ids);
}
-static char * OVS_WARN_UNUSED_RESULT
-lrp_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist,
-const struct nbrec_logical_router_port **lrp_p);
-
static void
nbctl_lr_route_add(struct ctl_context *ctx)
{
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH OVN] ovndb-ctl:probe interval of ovndb-ctl daemon
On Thu, Jan 20, 2022 at 8:54 AM Ilya Maximets wrote:
>
> On 1/20/22 14:13, Wentao Jia wrote:
> >
> >
> > Hi, Ilya
> >
> > Initially, ovndb-ctl command is a program executed only once，probe interval
> > is not be set。
> > ovndb-ctl daemon mode is a longrun program，so we need to set probe interval。
>
> I understand that. But,
> "not set" should be equal to "set to default 5 seconds".
> I'm not sure why we need to call ovsdb_idl_set_probe_interval
> explicitly if the default 5 seconds should already be configured.
> That is my question.
>
> We should not need to call ovsdb_idl_set_probe_interval to
> have 5 second inactivity probe interval. So, I'm trying to
> understand why this is needed in your case.
I missed this completely. Thanks Ilya for pointing this out.
I kind of assumed the probe interval was set to 0.
@Wentao Jia Can you please check what is the default value set in your case.
Numan
>
> The only reason I can think of is the unix: connection method
> that may not have inactivity probe enabled by default. But
> unix sockets are typically good at detecting broken connections.
>
> Best regards, Ilya Maximets.
>
> >
> > Best regards, Wentao Jia
> >
> >
>
> ovndb-ctl deamon mode, the connection cannot be reconnected when
> connection is broken, set inactivity probe interval to make it
> reconnected
> >>
> >>Hi, Wentao, Numan.
> >>
> >>Could you, please, elaborate, why exactly connection can not be
> >>reconnected? AFAICT, ovsdb_idl_set_probe_interval() only updates
> >>the fsm->probe_interval that should already be equal to 5 seconds
> >>by default (RECONNECT_DEFAULT_PROBE_INTERVAL).
> >>So, why do we need to set it from the application code? Am I missing
> >>something?
> >>
> >>Best regards, Ilya Maximets.
> >>
>
>
> Signed-off-by: Wentao Jia
> >>>
> >>> Thanks for the patch.
> >>>
> >>> I see one issue with using the default probe interval value of 5
> >>> seconds. If the database is huge,
> >>> 5 seconds may not be enough and the dbctl daemons/utilities could be
> >>> in infinite loop connecting/disconnecting
> >>> due to the probe interval time out.
> >>>
> >>> Instead, I'd suggest adding a command line option so that users can
> >>> specify the probe interval value to use
> >>> (with the default value set to 0 if not specified).
> >>>
> >>> Like ovn-nbctl --probe-interval=1 ...
> >>>
> >>>
> >>> Thanks
> >>> Numan
> >>>
> ---
> utilities/ovn-dbctl.c | 4
> 1 file changed, 4 insertions(+)
>
>
> diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c
> index 791caabb2..2080e8ba0 100644
> --- a/utilities/ovn-dbctl.c
> +++ b/utilities/ovn-dbctl.c
> @@ -109,6 +109,9 @@ static void server_loop(const struct
> ovn_dbctl_options *dbctl_options,
> struct ovsdb_idl *idl, int argc, char *argv[]);
> static void ovn_dbctl_exit(int status);
>
> +/* Default probe interval for NB and SB DB connections. */
> +#define DEFAULT_PROBE_INTERVAL_MSEC 5000
> +
> int
> ovn_dbctl_main(int argc, char *argv[],
> const struct ovn_dbctl_options *dbctl_options)
> @@ -191,6 +194,7 @@ ovn_dbctl_main(int argc, char *argv[],
> /* "retry" is true iff in daemon mode. */
> ovsdb_idl_set_remote(idl, db, daemon_mode);
> ovsdb_idl_set_leader_only(idl, leader_only);
> +ovsdb_idl_set_probe_interval(idl, DEFAULT_PROBE_INTERVAL_MSEC);
>
> if (daemon_mode) {
> server_loop(dbctl_options, idl, argc, argv_);
> --
> 2.32.0
> >>
> >
> >
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-nbctl: add the capability to specify CoPP UUID or CoPP name
On 1/20/22 15:16, Lorenzo Bianconi wrote:
>> On 1/20/22 11:38, Lorenzo Bianconi wrote:
>>> Introduce the capability to specify CoPP UUID or CoPP name in order to
>>> reuse the same CoPP reference on multiple datapaths.
>>> Introduce logical_router and logical_switches columns in CoPP table in
>>> order to specify datapaths where CoPP is installed.
>>>
>>> Reported-ad: https://bugzilla.redhat.com/show_bug.cgi?id=2040852
>>> Signed-off-by: Lorenzo Bianconi
>>> ---
>>
>> Hi Lorenzo,
>
> Hi Dumitru,
>
> thx for the review.
>
>>
>>> ovn-nb.ovsschema | 15 +-
>>> ovn-nb.xml| 9
>>> tests/ovn-northd.at | 27 ++
>>> utilities/ovn-nbctl.8.xml | 16 --
>>> utilities/ovn-nbctl.c | 103 --
>>> 5 files changed, 150 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
>>> index 55977339a..cf2947d93 100644
>>> --- a/ovn-nb.ovsschema
>>> +++ b/ovn-nb.ovsschema
>>> @@ -1,7 +1,7 @@
>>> {
>>> "name": "OVN_Northbound",
>>> -"version": "5.34.1",
>>> -"cksum": "2177334725 30782",
>>> +"version": "5.35.0",
>>> +"cksum": "2039436985 31434",
>>> "tables": {
>>> "NB_Global": {
>>> "columns": {
>>> @@ -32,6 +32,17 @@
>>> "isRoot": true},
>>> "Copp": {
>>> "columns": {
>>> +"name": {"type": "string"},
>>
>> I think name should be an index too. That would be a backwards
>> incompatible change but, AFAIK, there are no users of CoPP yet.
>>
>> What do you think?
>
> if name is used as index we will need to make it mandatory. One possible
> solution
> would be to create a random string when not provided. Any downside with this
> approach?
This still doesn't cover the case when entries already existed in the
database before the upgrade.
I don't think there's a backwards compatible way of doing this and I
don't really think it's useful to add a "name" field that's not used as
index.
However, for this feature in particular, because it's rather new and not
used (as far as I know) by any CMS, I think we can probably break
compatibility and add the missing index.
I'm curious about the maintainers' opinion on this too.
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-nbctl: add the capability to specify CoPP UUID or CoPP name
> On 1/20/22 11:38, Lorenzo Bianconi wrote:
> > Introduce the capability to specify CoPP UUID or CoPP name in order to
> > reuse the same CoPP reference on multiple datapaths.
> > Introduce logical_router and logical_switches columns in CoPP table in
> > order to specify datapaths where CoPP is installed.
> >
> > Reported-ad: https://bugzilla.redhat.com/show_bug.cgi?id=2040852
> > Signed-off-by: Lorenzo Bianconi
> > ---
>
> Hi Lorenzo,
Hi Dumitru,
thx for the review.
>
> > ovn-nb.ovsschema | 15 +-
> > ovn-nb.xml| 9
> > tests/ovn-northd.at | 27 ++
> > utilities/ovn-nbctl.8.xml | 16 --
> > utilities/ovn-nbctl.c | 103 --
> > 5 files changed, 150 insertions(+), 20 deletions(-)
> >
> > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> > index 55977339a..cf2947d93 100644
> > --- a/ovn-nb.ovsschema
> > +++ b/ovn-nb.ovsschema
> > @@ -1,7 +1,7 @@
> > {
> > "name": "OVN_Northbound",
> > -"version": "5.34.1",
> > -"cksum": "2177334725 30782",
> > +"version": "5.35.0",
> > +"cksum": "2039436985 31434",
> > "tables": {
> > "NB_Global": {
> > "columns": {
> > @@ -32,6 +32,17 @@
> > "isRoot": true},
> > "Copp": {
> > "columns": {
> > +"name": {"type": "string"},
>
> I think name should be an index too. That would be a backwards
> incompatible change but, AFAIK, there are no users of CoPP yet.
>
> What do you think?
if name is used as index we will need to make it mandatory. One possible
solution
would be to create a random string when not provided. Any downside with this
approach?
Regards,
Lorenzo
>
> Thanks,
> Dumitru
>
> > +"logical_switch": {"type": {"key": {"type": "uuid",
> > + "refTable": "Logical_Switch",
> > + "refType": "strong"},
> > + "min": 0,
> > + "max": "unlimited"}},
> > +"logical_router": {"type": {"key": {"type": "uuid",
> > + "refTable": "Logical_Router",
> > + "refType": "strong"},
> > + "min": 0,
> > + "max": "unlimited"}},
> > "meters": {
> > "type": {"key": "string",
> > "value": "string",
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 6a6972856..4d319267f 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -360,6 +360,15 @@
> >associate entries from table to control protocol
> >names.
> >
> > +
> > + CoPP name.
> > +
> > +
> > + Reference to where the CoPP is
> > installed.
> > +
> > +
> > + Reference to where the CoPP is
> > installed.
> > +
> >
> >Rate limiting meter for ARP packets (request/reply) used for learning
> >neighbors.
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 652903761..bd284c915 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -3403,6 +3403,33 @@ check ovn-nbctl lr-copp-del r0
> > AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
> > ])
> >
> > +check ovn-nbctl ls-copp-del sw1
> > +AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
> > +])
> > +
> > +check ovn-nbctl --wait=hv lr-copp-add copp0 r0 arp meter0
> > +AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
> > +arp: meter0
> > +])
> > +
> > +AT_CHECK([fetch_column nb:CoPP name], [0], [dnl
> > +copp0
> > +])
> > +
> > +lr_uuid=$(fetch_column nb:Logical_Router _uuid)
> > +copp_lr_uuid=$(fetch_column nb:CoPP logical_router)
> > +AT_CHECK([test "$lr_uuid" = "$copp_lr_uuid"])
> > +
> > +copp_uuid=$(fetch_column nb:CoPP _uuid)
> > +check ovn-nbctl --wait=hv ls-copp-add $copp_uuid sw1 arp meter0
> > +
> > +ls_uuid=$(fetch_column nb:Logical_Switch _uuid)
> > +copp_ls_uuid=$(fetch_column nb:CoPP logical_switch)
> > +AT_CHECK([test "$ls_uuid" = "$copp_ls_uuid"])
> > +
> > +ls_copp_uuid=$(fetch_column nb:Logical_Switch copp)
> > +AT_CHECK([test "$ls_copp_uuid" = "$copp_uuid"])
> > +
> > AT_CLEANUP
> > ])
> >
> > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> > index 80a564660..98326dcc2 100644
> > --- a/utilities/ovn-nbctl.8.xml
> > +++ b/utilities/ovn-nbctl.8.xml
> > @@ -1474,13 +1474,17 @@
> >
> >
> >
> > - ls-copp-add switch proto
> > - meter
> > + ls-copp-add [UUID|name]
> > + switch proto meter
> >
> > Adds the control proto to meter mapping
> > to the switch control plane protection policy. If no
> > policy exists yet, it creates one. If a mapping already existed for
> > proto, this will overwrite it.
> > +If UUID is provided, the already installed will be
> > reused
> > +
Re: [ovs-dev] [PATCH OVN] ovndb-ctl:probe interval of ovndb-ctl daemon
On 1/20/22 14:13, Wentao Jia wrote:
>
>
> Hi, Ilya
>
> Initially, ovndb-ctl command is a program executed only once，probe interval
> is not be set。
> ovndb-ctl daemon mode is a longrun program，so we need to set probe interval。
I understand that. But,
"not set" should be equal to "set to default 5 seconds".
I'm not sure why we need to call ovsdb_idl_set_probe_interval
explicitly if the default 5 seconds should already be configured.
That is my question.
We should not need to call ovsdb_idl_set_probe_interval to
have 5 second inactivity probe interval. So, I'm trying to
understand why this is needed in your case.
The only reason I can think of is the unix: connection method
that may not have inactivity probe enabled by default. But
unix sockets are typically good at detecting broken connections.
Best regards, Ilya Maximets.
>
> Best regards, Wentao Jia
>
>
ovndb-ctl deamon mode, the connection cannot be reconnected when
connection is broken, set inactivity probe interval to make it
reconnected
>>
>>Hi, Wentao, Numan.
>>
>>Could you, please, elaborate, why exactly connection can not be
>>reconnected? AFAICT, ovsdb_idl_set_probe_interval() only updates
>>the fsm->probe_interval that should already be equal to 5 seconds
>>by default (RECONNECT_DEFAULT_PROBE_INTERVAL).
>>So, why do we need to set it from the application code? Am I missing
>>something?
>>
>>Best regards, Ilya Maximets.
>>
Signed-off-by: Wentao Jia
>>>
>>> Thanks for the patch.
>>>
>>> I see one issue with using the default probe interval value of 5
>>> seconds. If the database is huge,
>>> 5 seconds may not be enough and the dbctl daemons/utilities could be
>>> in infinite loop connecting/disconnecting
>>> due to the probe interval time out.
>>>
>>> Instead, I'd suggest adding a command line option so that users can
>>> specify the probe interval value to use
>>> (with the default value set to 0 if not specified).
>>>
>>> Like ovn-nbctl --probe-interval=1 ...
>>>
>>>
>>> Thanks
>>> Numan
>>>
---
utilities/ovn-dbctl.c | 4
1 file changed, 4 insertions(+)
diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c
index 791caabb2..2080e8ba0 100644
--- a/utilities/ovn-dbctl.c
+++ b/utilities/ovn-dbctl.c
@@ -109,6 +109,9 @@ static void server_loop(const struct ovn_dbctl_options
*dbctl_options,
struct ovsdb_idl *idl, int argc, char *argv[]);
static void ovn_dbctl_exit(int status);
+/* Default probe interval for NB and SB DB connections. */
+#define DEFAULT_PROBE_INTERVAL_MSEC 5000
+
int
ovn_dbctl_main(int argc, char *argv[],
const struct ovn_dbctl_options *dbctl_options)
@@ -191,6 +194,7 @@ ovn_dbctl_main(int argc, char *argv[],
/* "retry" is true iff in daemon mode. */
ovsdb_idl_set_remote(idl, db, daemon_mode);
ovsdb_idl_set_leader_only(idl, leader_only);
+ovsdb_idl_set_probe_interval(idl, DEFAULT_PROBE_INTERVAL_MSEC);
if (daemon_mode) {
server_loop(dbctl_options, idl, argc, argv_);
--
2.32.0
>>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] vswitchd.xml: Add missing tx-steering PMD option.
On 20/01/2022 12:16, Ilya Maximets wrote:
On 1/18/22 11:51, Kevin Traynor wrote:
On 17/01/2022 22:25, Maxime Coquelin wrote:
This patch documents PMD's other_config:tx-steering option.
Signed-off-by: Maxime Coquelin
---
vswitchd/vswitch.xml | 22 ++
1 file changed, 22 insertions(+)
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 026b5e2ca..ef7f8f2c8 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -3375,6 +3375,28 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch
options:peer=p1 \
This option may only be used with dpdk VF representors.
+
+
+
+ Specifies the Tx steering mode for the interface.
+
+
+ thread enables static txq mapping when the number of txq
+ is greater or equal than the number of PMD threads, and XPS mode if
+ lower than the number of PMD threads.
I think it should be 'greater than PMD threads, and XPS mode if equal or lower
than the number of PMD threads.', because one txq is also needed for the OVS
main thread. I checked the code and this txq is accounted for in wanted_txqs.
I would also recommend to not use the 'XPS' as a name for the dynamic txq
mapping
in the 'thread' mode. Because all of this is a sort of XPS. The hash mode is
a form of {trans,x}mit packet steering too.
So, something like this maybe:
thread enables static (1:1) thread-to-txq mapping when the
number of Tx queues is greater than number of PMD threads, and dynamic (N:1)
mapping if equal or lower. In this mode a single thread can not use more
than 1 transmit queue of a given port.
And, I think, we need to fix the userspace-tx-steering.rst too, e.g.:
Depending on the port's number of Tx queues being greater or equal than the
number of PMD threads, static (1:1) or dynamic (N:1) thread-to-txq mapping
will be used.
Maxime, Kevin, what do you think?
+1. Only thing I'd add is to explicitly associate each mode with the
txq/PMD values in the rst, like it is done in the xml.
With this fix:
Acked-by: Kevin Traynor
+
+
+ hash enables hash-based Tx steering, which distributes
+ the packets on all the transmit queues based on their 5-tuples
+ hashes.
+
+
+ Defaults to thread.
+
+
+
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH OVN] ovndb-ctl:probe interval of ovndb-ctl daemon
Hi, Ilya
Initially, ovndb-ctl command is a program executed only once，probe interval is
not be set。
ovndb-ctl daemon mode is a longrun program，so we need to set probe interval。
Best regards, Wentao Jia >>> >>> ovndb-ctl deamon mode, the connection cannot
be reconnected when >>> connection is broken, set inactivity probe interval to
make it >>> reconnected > >Hi, Wentao, Numan. > >Could you, please, elaborate,
why exactly connection can not be >reconnected? AFAICT,
ovsdb_idl_set_probe_interval() only updates >the fsm->probe_interval that
should already be equal to 5 seconds >by default
(RECONNECT_DEFAULT_PROBE_INTERVAL). >So, why do we need to set it from the
application code? Am I missing >something? > >Best regards, Ilya Maximets. >
>>> >>> >>> Signed-off-by: Wentao Jia >> >> Thanks
for the patch. >> >> I see one issue with using the default probe interval
value of 5 >> seconds. If the database is huge, >> 5 seconds may not be enough
and the dbctl daemons/utilities could be >> in infinite loop
connecting/disconnecting >> due to the probe interval time out. >> >> Instead,
I'd suggest adding a command line option so that users can >> specify the probe
interval value to use >> (with the default value set to 0 if not specified). >>
>> Like ovn-nbctl --probe-interval=1 ... >> >> >> Thanks >> Numan >> >>>
--- >>> utilities/ovn-dbctl.c | 4 >>> 1 file changed, 4 insertions(+) >>>
>>> >>> diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c >>> index
791caabb2..2080e8ba0 100644 >>> --- a/utilities/ovn-dbctl.c >>> +++
b/utilities/ovn-dbctl.c >>> @@ -109,6 +109,9 @@ static void server_loop(const
struct ovn_dbctl_options *dbctl_options, >>> struct ovsdb_idl *idl, int argc,
char *argv[]); >>> static void ovn_dbctl_exit(int status); >>> >>> +/* Default
probe interval for NB and SB DB connections. */ >>> +#define
DEFAULT_PROBE_INTERVAL_MSEC 5000 >>> + >>> int >>> ovn_dbctl_main(int argc,
char *argv[], >>> const struct ovn_dbctl_options *dbctl_options) >>> @@ -191,6
+194,7 @@ ovn_dbctl_main(int argc, char *argv[], >>> /* "retry" is true iff in
daemon mode. */ >>> ovsdb_idl_set_remote(idl, db, daemon_mode); >>>
ovsdb_idl_set_leader_only(idl, leader_only); >>> +
ovsdb_idl_set_probe_interval(idl, DEFAULT_PROBE_INTERVAL_MSEC); >>> >>> if
(daemon_mode) { >>> server_loop(dbctl_options, idl, argc, argv_); >>> -- >>>
2.32.0 >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ovsdb-idl: ovsdb_idl_loop_destroy must also destroy the committing txn.
Found by AddressSanitizer when running OVN tests:
Direct leak of 64 byte(s) in 1 object(s) allocated from:
#0 0x498fb2 in calloc
(/home/runner/work/ovn/ovn/ovn-21.12.90/_build/sub/ic/ovn-ic+0x498fb2)
#1 0x5f681e in xcalloc__ /home/runner/work/ovn/ovn/ovs/lib/util.c:121:31
#2 0x5f681e in xzalloc__ /home/runner/work/ovn/ovn/ovs/lib/util.c:131:12
#3 0x5f681e in xzalloc /home/runner/work/ovn/ovn/ovs/lib/util.c:165:12
#4 0x5e3697 in ovsdb_idl_txn_add_map_op
/home/runner/work/ovn/ovn/ovs/lib/ovsdb-idl.c:4057:29
#5 0x4d3f25 in update_isb_pb_external_ids
/home/runner/work/ovn/ovn/ovn-21.12.90/_build/sub/../../ic/ovn-ic.c:576:5
#6 0x4cc4cc in create_isb_pb
/home/runner/work/ovn/ovn/ovn-21.12.90/_build/sub/../../ic/ovn-ic.c:716:5
#7 0x4cc4cc in port_binding_run
/home/runner/work/ovn/ovn/ovn-21.12.90/_build/sub/../../ic/ovn-ic.c:803:21
#8 0x4cc4cc in ovn_db_run
/home/runner/work/ovn/ovn/ovn-21.12.90/_build/sub/../../ic/ovn-ic.c:1700:5
#9 0x4c9c1c in main
/home/runner/work/ovn/ovn/ovn-21.12.90/_build/sub/../../ic/ovn-ic.c:1984:17
#10 0x7f9ad9f4a0b2 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
Signed-off-by: Dumitru Ceara
---
lib/ovsdb-idl.c | 4
1 file changed, 4 insertions(+)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index b7ba25eca6b5..46f51a527356 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -4242,6 +4242,10 @@ void
ovsdb_idl_loop_destroy(struct ovsdb_idl_loop *loop)
{
if (loop) {
+if (loop->committing_txn) {
+ovsdb_idl_txn_abort(loop->committing_txn);
+ovsdb_idl_txn_destroy(loop->committing_txn);
+}
ovsdb_idl_destroy(loop->idl);
}
}
--
2.27.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] vswitchd.xml: Add missing tx-steering PMD option.
Hi, Ilya, Kevin,
On 1/20/22 13:16, Ilya Maximets wrote:
On 1/18/22 11:51, Kevin Traynor wrote:
On 17/01/2022 22:25, Maxime Coquelin wrote:
This patch documents PMD's other_config:tx-steering option.
Signed-off-by: Maxime Coquelin
---
vswitchd/vswitch.xml | 22 ++
1 file changed, 22 insertions(+)
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 026b5e2ca..ef7f8f2c8 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -3375,6 +3375,28 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch
options:peer=p1 \
This option may only be used with dpdk VF representors.
+
+
+
+ Specifies the Tx steering mode for the interface.
+
+
+ thread enables static txq mapping when the number of txq
+ is greater or equal than the number of PMD threads, and XPS mode if
+ lower than the number of PMD threads.
I think it should be 'greater than PMD threads, and XPS mode if equal or lower
than the number of PMD threads.', because one txq is also needed for the OVS
main thread. I checked the code and this txq is accounted for in wanted_txqs.
I would also recommend to not use the 'XPS' as a name for the dynamic txq
mapping
in the 'thread' mode. Because all of this is a sort of XPS. The hash mode is
a form of {trans,x}mit packet steering too.
So, something like this maybe:
thread enables static (1:1) thread-to-txq mapping when the
number of Tx queues is greater than number of PMD threads, and dynamic (N:1)
mapping if equal or lower. In this mode a single thread can not use more
than 1 transmit queue of a given port.
And, I think, we need to fix the userspace-tx-steering.rst too, e.g.:
Depending on the port's number of Tx queues being greater or equal than the
number of PMD threads, static (1:1) or dynamic (N:1) thread-to-txq mapping
will be used.
Maxime, Kevin, what do you think?
I agree with your suggestions, I'll post a new revision tomorrow, that
will include userspace-tx-steering.rst changes too.
Thanks,
Maxime
With this fix:
Acked-by: Kevin Traynor
+
+
+ hash enables hash-based Tx steering, which distributes
+ the packets on all the transmit queues based on their 5-tuples
+ hashes.
+
+
+ Defaults to thread.
+
+
+
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] vswitchd.xml: Add missing tx-steering PMD option.
On 1/18/22 11:51, Kevin Traynor wrote:
> On 17/01/2022 22:25, Maxime Coquelin wrote:
>> This patch documents PMD's other_config:tx-steering option.
>>
>> Signed-off-by: Maxime Coquelin
>> ---
>> vswitchd/vswitch.xml | 22 ++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index 026b5e2ca..ef7f8f2c8 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -3375,6 +3375,28 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
>> type=patch options:peer=p1 \
>>
>> This option may only be used with dpdk VF representors.
>>
>> +
>> + > + type='{"type": "string",
>> + "enum": ["set", ["thread", "hash"]]}'>
>> +
>> + Specifies the Tx steering mode for the interface.
>> +
>> +
>> + thread enables static txq mapping when the number of
>> txq
>> + is greater or equal than the number of PMD threads, and XPS mode
>> if
>> + lower than the number of PMD threads.
>
> I think it should be 'greater than PMD threads, and XPS mode if equal or lower
> than the number of PMD threads.', because one txq is also needed for the OVS
> main thread. I checked the code and this txq is accounted for in wanted_txqs.
I would also recommend to not use the 'XPS' as a name for the dynamic txq
mapping
in the 'thread' mode. Because all of this is a sort of XPS. The hash mode is
a form of {trans,x}mit packet steering too.
So, something like this maybe:
thread enables static (1:1) thread-to-txq mapping when the
number of Tx queues is greater than number of PMD threads, and dynamic (N:1)
mapping if equal or lower. In this mode a single thread can not use more
than 1 transmit queue of a given port.
And, I think, we need to fix the userspace-tx-steering.rst too, e.g.:
Depending on the port's number of Tx queues being greater or equal than the
number of PMD threads, static (1:1) or dynamic (N:1) thread-to-txq mapping
will be used.
Maxime, Kevin, what do you think?
>
> With this fix:
> Acked-by: Kevin Traynor
>
>> +
>> +
>> + hash enables hash-based Tx steering, which
>> distributes
>> + the packets on all the transmit queues based on their 5-tuples
>> + hashes.
>> +
>> +
>> + Defaults to thread.
>> +
>> +
>> +
>>
>>
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-nbctl: add the capability to specify CoPP UUID or CoPP name
On 1/20/22 11:38, Lorenzo Bianconi wrote:
> Introduce the capability to specify CoPP UUID or CoPP name in order to
> reuse the same CoPP reference on multiple datapaths.
> Introduce logical_router and logical_switches columns in CoPP table in
> order to specify datapaths where CoPP is installed.
>
> Reported-ad: https://bugzilla.redhat.com/show_bug.cgi?id=2040852
> Signed-off-by: Lorenzo Bianconi
> ---
Hi Lorenzo,
> ovn-nb.ovsschema | 15 +-
> ovn-nb.xml| 9
> tests/ovn-northd.at | 27 ++
> utilities/ovn-nbctl.8.xml | 16 --
> utilities/ovn-nbctl.c | 103 --
> 5 files changed, 150 insertions(+), 20 deletions(-)
>
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index 55977339a..cf2947d93 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Northbound",
> -"version": "5.34.1",
> -"cksum": "2177334725 30782",
> +"version": "5.35.0",
> +"cksum": "2039436985 31434",
> "tables": {
> "NB_Global": {
> "columns": {
> @@ -32,6 +32,17 @@
> "isRoot": true},
> "Copp": {
> "columns": {
> +"name": {"type": "string"},
I think name should be an index too. That would be a backwards
incompatible change but, AFAIK, there are no users of CoPP yet.
What do you think?
Thanks,
Dumitru
> +"logical_switch": {"type": {"key": {"type": "uuid",
> + "refTable": "Logical_Switch",
> + "refType": "strong"},
> + "min": 0,
> + "max": "unlimited"}},
> +"logical_router": {"type": {"key": {"type": "uuid",
> + "refTable": "Logical_Router",
> + "refType": "strong"},
> + "min": 0,
> + "max": "unlimited"}},
> "meters": {
> "type": {"key": "string",
> "value": "string",
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 6a6972856..4d319267f 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -360,6 +360,15 @@
>associate entries from table to control protocol
>names.
>
> +
> + CoPP name.
> +
> +
> + Reference to where the CoPP is installed.
> +
> +
> + Reference to where the CoPP is installed.
> +
>
>Rate limiting meter for ARP packets (request/reply) used for learning
>neighbors.
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 652903761..bd284c915 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -3403,6 +3403,33 @@ check ovn-nbctl lr-copp-del r0
> AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
> ])
>
> +check ovn-nbctl ls-copp-del sw1
> +AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
> +])
> +
> +check ovn-nbctl --wait=hv lr-copp-add copp0 r0 arp meter0
> +AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
> +arp: meter0
> +])
> +
> +AT_CHECK([fetch_column nb:CoPP name], [0], [dnl
> +copp0
> +])
> +
> +lr_uuid=$(fetch_column nb:Logical_Router _uuid)
> +copp_lr_uuid=$(fetch_column nb:CoPP logical_router)
> +AT_CHECK([test "$lr_uuid" = "$copp_lr_uuid"])
> +
> +copp_uuid=$(fetch_column nb:CoPP _uuid)
> +check ovn-nbctl --wait=hv ls-copp-add $copp_uuid sw1 arp meter0
> +
> +ls_uuid=$(fetch_column nb:Logical_Switch _uuid)
> +copp_ls_uuid=$(fetch_column nb:CoPP logical_switch)
> +AT_CHECK([test "$ls_uuid" = "$copp_ls_uuid"])
> +
> +ls_copp_uuid=$(fetch_column nb:Logical_Switch copp)
> +AT_CHECK([test "$ls_copp_uuid" = "$copp_uuid"])
> +
> AT_CLEANUP
> ])
>
> diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> index 80a564660..98326dcc2 100644
> --- a/utilities/ovn-nbctl.8.xml
> +++ b/utilities/ovn-nbctl.8.xml
> @@ -1474,13 +1474,17 @@
>
>
>
> - ls-copp-add switch proto
> - meter
> + ls-copp-add [UUID|name]
> + switch proto meter
>
> Adds the control proto to meter mapping
> to the switch control plane protection policy. If no
> policy exists yet, it creates one. If a mapping already existed for
> proto, this will overwrite it.
> +If UUID is provided, the already installed will be reused
> +(if not found and error will be reported).
> +If name is provided, CoPP name can be used for CoPP
> +table lookup.
>
>
>ls-copp-del switch [proto]
> @@ -1497,13 +1501,17 @@
> switch.
>
>
> - lr-copp-add router proto
> - meter
> + lr-copp-add [UUID|name]
> + router proto meter
>
> Adds the control proto to meter mapping
> to the router control plane protection policy. If no
> policy exists yet,
Re: [ovs-dev] [PATCH OVN] ovndb-ctl:probe interval of ovndb-ctl daemon
On 1/19/22 17:43, Numan Siddique wrote:
> On Thu, Jan 6, 2022 at 11:02 PM Wentao Jia wrote:
>>
>>
>> ovndb-ctl deamon mode, the connection cannot be reconnected when
>> connection is broken, set inactivity probe interval to make it
>> reconnected
Hi, Wentao, Numan.
Could you, please, elaborate, why exactly connection can not be
reconnected? AFAICT, ovsdb_idl_set_probe_interval() only updates
the fsm->probe_interval that should already be equal to 5 seconds
by default (RECONNECT_DEFAULT_PROBE_INTERVAL).
So, why do we need to set it from the application code? Am I missing
something?
Best regards, Ilya Maximets.
>>
>>
>> Signed-off-by: Wentao Jia
>
> Thanks for the patch.
>
> I see one issue with using the default probe interval value of 5
> seconds. If the database is huge,
> 5 seconds may not be enough and the dbctl daemons/utilities could be
> in infinite loop connecting/disconnecting
> due to the probe interval time out.
>
> Instead, I'd suggest adding a command line option so that users can
> specify the probe interval value to use
> (with the default value set to 0 if not specified).
>
> Like ovn-nbctl --probe-interval=1 ...
>
>
> Thanks
> Numan
>
>> ---
>> utilities/ovn-dbctl.c | 4
>> 1 file changed, 4 insertions(+)
>>
>>
>> diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c
>> index 791caabb2..2080e8ba0 100644
>> --- a/utilities/ovn-dbctl.c
>> +++ b/utilities/ovn-dbctl.c
>> @@ -109,6 +109,9 @@ static void server_loop(const struct ovn_dbctl_options
>> *dbctl_options,
>> struct ovsdb_idl *idl, int argc, char *argv[]);
>> static void ovn_dbctl_exit(int status);
>>
>> +/* Default probe interval for NB and SB DB connections. */
>> +#define DEFAULT_PROBE_INTERVAL_MSEC 5000
>> +
>> int
>> ovn_dbctl_main(int argc, char *argv[],
>> const struct ovn_dbctl_options *dbctl_options)
>> @@ -191,6 +194,7 @@ ovn_dbctl_main(int argc, char *argv[],
>> /* "retry" is true iff in daemon mode. */
>> ovsdb_idl_set_remote(idl, db, daemon_mode);
>> ovsdb_idl_set_leader_only(idl, leader_only);
>> +ovsdb_idl_set_probe_interval(idl, DEFAULT_PROBE_INTERVAL_MSEC);
>>
>> if (daemon_mode) {
>> server_loop(dbctl_options, idl, argc, argv_);
>> --
>> 2.32.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 01/11] util: Avoid false positive UB when iterating collections.
On 1/20/22 11:11, Adrian Moreno wrote: > > > On 12/21/21 12:01, Dumitru Ceara wrote: >> UB Sanitizer was flagging OBJECT_CONTAINING() as undefined behavior >> due to the way it's used when iterating through collections, e.g., >> HMAP_FOR_EACH_INIT(). However, in such cases we don't actually >> dereference the pointer, we just use its value. Avoid the undefined >> behavior by casting to 'void *' first. >> >> Signed-off-by: Dumitru Ceara >> --- >> include/openvswitch/util.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h >> index 228b185c3a5f..cfbd5f1a3375 100644 >> --- a/include/openvswitch/util.h >> +++ b/include/openvswitch/util.h >> @@ -128,7 +128,7 @@ OVS_NO_RETURN void ovs_assert_failure(const char >> *, const char *, const char *); >> * from the type of '*OBJECT'. */ >> #define OBJECT_CONTAINING(POINTER, OBJECT, >> MEMBER) \ >> ((OVS_TYPEOF(OBJECT)) (void >> *) \ >> - ((char *) (POINTER) - OBJECT_OFFSETOF(OBJECT, MEMBER))) >> + ((uintptr_t)(void *)(POINTER) - OBJECT_OFFSETOF(OBJECT, MEMBER))) >> /* Given POINTER, the address of the given MEMBER within an object >> of the type >> * that that OBJECT points to, assigns the address of the outer >> object to >> >> ___ >> dev mailing list >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > > I've been looking into UB recently and I agree: moving from pointer to > integer arithmetics _can_ relax some restrictions that the compiler > enforces on pointers, so the patch looks good to me. > Thanks for reviewing this! > There are more pointer arithmetics that can be replaced with "uintptr_t" > in util.h. Do you plan to replace all of them? If not, I can do it in a > series I'm preparing that also addresses UB on this macros. > I'm ok to drop this patch if you have a more comprehensive one. As long as the outcome is the same. For example, if applying the rest of this series on top of your patches keeps UBSan happy, then I guess we're good. Regards, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-nbctl: add the capability to specify CoPP UUID or CoPP name
Bleep bloop. Greetings Lorenzo Bianconi, 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 lacks whitespace around operator WARNING: Line lacks whitespace around operator #164 FILE: utilities/ovn-nbctl.c:440: ls-copp-add [UUID|NAME] SWITCH PROTO METER\n\ WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator #173 FILE: utilities/ovn-nbctl.c:450: lr-copp-add [UUID|NAME] ROUTER PROTO METER\n\ Lines checked: 327, 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] ovn-nbctl: add the capability to specify CoPP UUID or CoPP name
Introduce the capability to specify CoPP UUID or CoPP name in order to
reuse the same CoPP reference on multiple datapaths.
Introduce logical_router and logical_switches columns in CoPP table in
order to specify datapaths where CoPP is installed.
Reported-ad: https://bugzilla.redhat.com/show_bug.cgi?id=2040852
Signed-off-by: Lorenzo Bianconi
---
ovn-nb.ovsschema | 15 +-
ovn-nb.xml| 9
tests/ovn-northd.at | 27 ++
utilities/ovn-nbctl.8.xml | 16 --
utilities/ovn-nbctl.c | 103 --
5 files changed, 150 insertions(+), 20 deletions(-)
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index 55977339a..cf2947d93 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Northbound",
-"version": "5.34.1",
-"cksum": "2177334725 30782",
+"version": "5.35.0",
+"cksum": "2039436985 31434",
"tables": {
"NB_Global": {
"columns": {
@@ -32,6 +32,17 @@
"isRoot": true},
"Copp": {
"columns": {
+"name": {"type": "string"},
+"logical_switch": {"type": {"key": {"type": "uuid",
+ "refTable": "Logical_Switch",
+ "refType": "strong"},
+ "min": 0,
+ "max": "unlimited"}},
+"logical_router": {"type": {"key": {"type": "uuid",
+ "refTable": "Logical_Router",
+ "refType": "strong"},
+ "min": 0,
+ "max": "unlimited"}},
"meters": {
"type": {"key": "string",
"value": "string",
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 6a6972856..4d319267f 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -360,6 +360,15 @@
associate entries from table to control protocol
names.
+
+ CoPP name.
+
+
+ Reference to where the CoPP is installed.
+
+
+ Reference to where the CoPP is installed.
+
Rate limiting meter for ARP packets (request/reply) used for learning
neighbors.
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 652903761..bd284c915 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3403,6 +3403,33 @@ check ovn-nbctl lr-copp-del r0
AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
])
+check ovn-nbctl ls-copp-del sw1
+AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
+])
+
+check ovn-nbctl --wait=hv lr-copp-add copp0 r0 arp meter0
+AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
+arp: meter0
+])
+
+AT_CHECK([fetch_column nb:CoPP name], [0], [dnl
+copp0
+])
+
+lr_uuid=$(fetch_column nb:Logical_Router _uuid)
+copp_lr_uuid=$(fetch_column nb:CoPP logical_router)
+AT_CHECK([test "$lr_uuid" = "$copp_lr_uuid"])
+
+copp_uuid=$(fetch_column nb:CoPP _uuid)
+check ovn-nbctl --wait=hv ls-copp-add $copp_uuid sw1 arp meter0
+
+ls_uuid=$(fetch_column nb:Logical_Switch _uuid)
+copp_ls_uuid=$(fetch_column nb:CoPP logical_switch)
+AT_CHECK([test "$ls_uuid" = "$copp_ls_uuid"])
+
+ls_copp_uuid=$(fetch_column nb:Logical_Switch copp)
+AT_CHECK([test "$ls_copp_uuid" = "$copp_uuid"])
+
AT_CLEANUP
])
diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
index 80a564660..98326dcc2 100644
--- a/utilities/ovn-nbctl.8.xml
+++ b/utilities/ovn-nbctl.8.xml
@@ -1474,13 +1474,17 @@
- ls-copp-add switch proto
- meter
+ ls-copp-add [UUID|name]
+ switch proto meter
Adds the control proto to meter mapping
to the switch control plane protection policy. If no
policy exists yet, it creates one. If a mapping already existed for
proto, this will overwrite it.
+If UUID is provided, the already installed will be reused
+(if not found and error will be reported).
+If name is provided, CoPP name can be used for CoPP
+table lookup.
ls-copp-del switch [proto]
@@ -1497,13 +1501,17 @@
switch.
- lr-copp-add router proto
- meter
+ lr-copp-add [UUID|name]
+ router proto meter
Adds the control proto to meter mapping
to the router control plane protection policy. If no
policy exists yet, it creates one. If a mapping already existed for
proto, this will overwrite it.
+If UUID is provided, the already installed will be reused
+(if not found and error will be reported).
+If name is provided, CoPP name can be used for CoPP
+table lookup.
lr-copp-del router [proto]
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index d67d2db65..8889f1c6b 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -437,7 +437,7 @@
Re: [ovs-dev] [PATCH] ofproto: fix ipfix not always sampling on egress
On 20 Jan 2022, at 11:06, Adrian Moreno wrote:
> On 1/20/22 10:13, Eelco Chaudron wrote:
>>
>>
>> On 17 Jan 2022, at 10:27, Adrian Moreno wrote:
>>
>>> We are currently requiring in_port to be a valid port number for ipfix
>>> sampling even if the sampling is done on the output port (egress).
>>>
>>> This restriction is not really needed and can affect pipelines that
>>> intentionally set the in_port to OFPP_NONE during flow processing. For
>>> instance, OVN does this, see:
>>>
>>> cfa547821 Fix ovn-controller generated packets from getting dropped for
>>> reject ACL action.
>>>
>>> This patch skips ipfix sampling only if both (ingress and egress) ports
>>> are invalid.
>>>
>>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2016346
>>> Signed-off-by: Adrian Moreno
>>
>> Adrian, the change looks good to me. Maybe you could add a test case for
>> this specific configuration, i.e., ingress and egress only?
>>
>
> I agree we need a test. I'm not very familiar with the unit test system so I
> was looking into how to verify what datapath flows are being programmed. Any
> suggestions/pointers?
My first idea would be to use “ovs-appctl ofproto/trace” which will give you
the megaflow and datapath actions, but the existing tests are probably better.
The “ofproto-dpif - Bridge IPFIX sanity check” (and the others) just use
dpctl/dump-flows, guess you can use those as a baseline.
>> Cheers,
>>
>> Eelco
>>
>>> ---
>>> ofproto/ofproto-dpif-xlate.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>>> index cafcd014a..189276bc1 100644
>>> --- a/ofproto/ofproto-dpif-xlate.c
>>> +++ b/ofproto/ofproto-dpif-xlate.c
>>> @@ -3272,7 +3272,9 @@ compose_ipfix_action(struct xlate_ctx *ctx,
>>> odp_port_t output_odp_port)
>>> struct dpif_ipfix *ipfix = ctx->xbridge->ipfix;
>>> odp_port_t tunnel_out_port = ODPP_NONE;
>>>
>>> -if (!ipfix || ctx->xin->flow.in_port.ofp_port == OFPP_NONE) {
>>> +if (!ipfix ||
>>> +(output_odp_port == ODPP_NONE &&
>>> + ctx->xin->flow.in_port.ofp_port == OFPP_NONE)) {
>>> return;
>>> }
>>>
>>> --
>>> 2.34.1
>>>
>>> ___
>>> dev mailing list
>>> d...@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
> --
> Adrián Moreno
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 01/11] util: Avoid false positive UB when iterating collections.
On 12/21/21 12:01, Dumitru Ceara wrote: UB Sanitizer was flagging OBJECT_CONTAINING() as undefined behavior due to the way it's used when iterating through collections, e.g., HMAP_FOR_EACH_INIT(). However, in such cases we don't actually dereference the pointer, we just use its value. Avoid the undefined behavior by casting to 'void *' first. Signed-off-by: Dumitru Ceara --- include/openvswitch/util.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h index 228b185c3a5f..cfbd5f1a3375 100644 --- a/include/openvswitch/util.h +++ b/include/openvswitch/util.h @@ -128,7 +128,7 @@ OVS_NO_RETURN void ovs_assert_failure(const char *, const char *, const char *); * from the type of '*OBJECT'. */ #define OBJECT_CONTAINING(POINTER, OBJECT, MEMBER) \ ((OVS_TYPEOF(OBJECT)) (void *) \ - ((char *) (POINTER) - OBJECT_OFFSETOF(OBJECT, MEMBER))) + ((uintptr_t)(void *)(POINTER) - OBJECT_OFFSETOF(OBJECT, MEMBER))) /* Given POINTER, the address of the given MEMBER within an object of the type * that that OBJECT points to, assigns the address of the outer object to ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev I've been looking into UB recently and I agree: moving from pointer to integer arithmetics _can_ relax some restrictions that the compiler enforces on pointers, so the patch looks good to me. There are more pointer arithmetics that can be replaced with "uintptr_t" in util.h. Do you plan to replace all of them? If not, I can do it in a series I'm preparing that also addresses UB on this macros. -- Adrián Moreno ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto: fix ipfix not always sampling on egress
On 1/20/22 10:13, Eelco Chaudron wrote:
On 17 Jan 2022, at 10:27, Adrian Moreno wrote:
We are currently requiring in_port to be a valid port number for ipfix
sampling even if the sampling is done on the output port (egress).
This restriction is not really needed and can affect pipelines that
intentionally set the in_port to OFPP_NONE during flow processing. For
instance, OVN does this, see:
cfa547821 Fix ovn-controller generated packets from getting dropped for
reject ACL action.
This patch skips ipfix sampling only if both (ingress and egress) ports
are invalid.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2016346
Signed-off-by: Adrian Moreno
Adrian, the change looks good to me. Maybe you could add a test case for this
specific configuration, i.e., ingress and egress only?
I agree we need a test. I'm not very familiar with the unit test system so I was
looking into how to verify what datapath flows are being programmed. Any
suggestions/pointers?
Cheers,
Eelco
---
ofproto/ofproto-dpif-xlate.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index cafcd014a..189276bc1 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3272,7 +3272,9 @@ compose_ipfix_action(struct xlate_ctx *ctx, odp_port_t
output_odp_port)
struct dpif_ipfix *ipfix = ctx->xbridge->ipfix;
odp_port_t tunnel_out_port = ODPP_NONE;
-if (!ipfix || ctx->xin->flow.in_port.ofp_port == OFPP_NONE) {
+if (!ipfix ||
+(output_odp_port == ODPP_NONE &&
+ ctx->xin->flow.in_port.ofp_port == OFPP_NONE)) {
return;
}
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
--
Adrián Moreno
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 0/9] Actions Infrastructure + Optimizations
> -Original Message- > From: Ilya Maximets > Sent: Friday, January 14, 2022 8:17 PM > To: Van Haaren, Harry ; Finn, Emma > ; d...@openvswitch.org; Amber, Kumar > > Cc: i.maxim...@ovn.org; Stokes, Ian ; Flavio Leitner > ; Kevin Traynor ; Mcnamara, John > > Subject: Re: [PATCH v4 0/9] Actions Infrastructure + Optimizations > > On 1/14/22 18:11, Van Haaren, Harry wrote: > >> -Original Message- > >> From: Ilya Maximets > >> Sent: Thursday, January 13, 2022 1:14 PM > >> To: Van Haaren, Harry ; Finn, Emma > >> ; d...@openvswitch.org; Amber, Kumar > >> > >> Cc: i.maxim...@ovn.org; Stokes, Ian ; Flavio Leitner > >> ; Kevin Traynor ; Mcnamara, John > >> > >> Subject: Re: [PATCH v4 0/9] Actions Infrastructure + Optimizations > >> > >> On 1/13/22 11:53, Van Haaren, Harry wrote: > -Original Message- > From: Ilya Maximets > Sent: Wednesday, January 12, 2022 6:01 PM > To: Van Haaren, Harry ; Finn, Emma > ; d...@openvswitch.org; Amber, Kumar > > Cc: i.maxim...@ovn.org; Stokes, Ian ; Flavio > Leitner > ; Kevin Traynor > Subject: Re: [PATCH v4 0/9] Actions Infrastructure + Optimizations > > On 1/6/22 14:11, Van Haaren, Harry wrote: > >> -Original Message- > >> From: Finn, Emma > >> Sent: Wednesday, January 5, 2022 4:54 PM > >> To: d...@openvswitch.org; Van Haaren, Harry > >> ; > >> Amber, Kumar > >> Cc: Finn, Emma > >> Subject: [PATCH v4 0/9] Actions Infrastructure + Optimizations > >> > >> --- > >> v4: > >> - Rebase to master > >> - Add ISA implementation of push_vlan action > > > > Thanks for the updated patchset Emma & Amber. > > > > Overall, this is working as expected and I've only had some minor > > comments throughout the patchset. I've added my Acked-by to most > > patches, some small open questions remain to be addressed in a v5. > > > > +CC Ian/Ilya , I'd like to see the v5 get merged, so let's continue to > > work > towards that. > > Hi, Harry, Ian, others. > >>> > >>> Hi Ilya, > >>> > Following up from a brief conversation during today's upstream meeting. > It was brought to my attention that you're expecting this series and > the 'hash' one to be accepted into 2.17. Though there are few issues > with that: > > 1. This review for v4 was actually very first review of the patch set. > The other one as of today doesn't have any reviews at all. > Looking at the change log for this patch set it doesn't seem that > internal reviews behind the closed doors (if there were any) requested > any significant changes. In any case, internal reviews is not the way > how open-source projects work. > >>> > >>> Actions & MFEX were developed internally yes, and hence internal reviews > and > >>> architecture was iterated on. Saying "not many large changes requested" is > not > >> relevant, > >>> it means that internally the architecture was well aligned. If anything, > >>> it > means > >> that > >>> reviewers did not have big concerns, hence we should have better > confidence > >> to merge. > >> > >> Me, who never seen these reviews and architecture discussions has exactly > >> zero confidence in these patch-set the same as the rest of the community. > >> Development behind closed doors is never taken into account while making > >> decisions in open-source projects, because this development and made > >> decisions are not visible and can not be peer-reviewed by anyone outside. > >> > >> On a quick glance over the code and reviews done in a last couple of days, > >> I'd also question the quality of these public reviews as the patch set > >> at least contains a few issues in a copy-pasted code that was already > >> fixed on master branch (left alone that these code duplications should, > >> likley, not exist in a first place). > >> > >>> > >>> > 2. The soft freeze for 2.17 began on Jan 3 in accordance with our > release schedule (even a bit later), and as you know, during the soft > freeze we're not normally accepting patches that wasn't already > reviewed > before the soft freeze begun. > https://mail.openvswitch.org/pipermail/ovs-dev/2022- > January/390487.html > >>> > >>> Actions and MFEX Hashing were discussed and reviewed in public at OVS > Conf; > >>> https://www.openvswitch.org/support/ovscon2021/#T32 > >>> https://www.openvswitch.org/support/ovscon2021/#T33 > >> > >> They were presented, but not discussed or reviewed. > >> > >>> > >>> The closing "call to action" slide clearly states 2.17 upstream is > >>> intended, > >>> and welcome community review & comments, no concerns were raised. > >>> > That's not the end of a world, but you need to request an exception in > reply to the email linked above. > >>> > >>> As both the patches and intent to merge for OVS 2.17 were clearly > >>> discussed >
Re: [ovs-dev] [PATCH] system-dpdk: Fix MFEX logs check.
> -Original Message-
> From: David Marchand
> Sent: Wednesday 19 January 2022 17:26
> To: d...@openvswitch.org
> Cc: Stokes, Ian ; Phelan, Michael
> ; Ferriter, Cian
> ; i.maxim...@ovn.org; ktray...@redhat.com
> Subject: [PATCH] system-dpdk: Fix MFEX logs check.
>
> Some warning logs must be waived when using the net/pcap DPDK driver.
> Those logs can affect different DPDK drivers (like mlx5) and the tests in
> system-dpdk are not testing MTU and Rx checksum, we might as well ignore
> those warnings from OVS.
>
> Fixes: d446dcb7e03f ("system-dpdk: Refactor common logs matching.")
> Signed-off-by: David Marchand
Reviewed and tested. This fixes the errors I see prior to applying the patch.
The MFEX tests now pass:
OVS-DPDK unit tests
6: OVS-DPDK - MFEX Autovalidator ok
7: OVS-DPDK - MFEX Autovalidator Fuzzy ok
8: OVS-DPDK - MFEX Configuration ok
Acked-by: Cian Ferriter
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto: fix ipfix not always sampling on egress
On 17 Jan 2022, at 10:27, Adrian Moreno wrote:
> We are currently requiring in_port to be a valid port number for ipfix
> sampling even if the sampling is done on the output port (egress).
>
> This restriction is not really needed and can affect pipelines that
> intentionally set the in_port to OFPP_NONE during flow processing. For
> instance, OVN does this, see:
>
> cfa547821 Fix ovn-controller generated packets from getting dropped for
> reject ACL action.
>
> This patch skips ipfix sampling only if both (ingress and egress) ports
> are invalid.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2016346
> Signed-off-by: Adrian Moreno
Adrian, the change looks good to me. Maybe you could add a test case for this
specific configuration, i.e., ingress and egress only?
Cheers,
Eelco
> ---
> ofproto/ofproto-dpif-xlate.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index cafcd014a..189276bc1 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3272,7 +3272,9 @@ compose_ipfix_action(struct xlate_ctx *ctx, odp_port_t
> output_odp_port)
> struct dpif_ipfix *ipfix = ctx->xbridge->ipfix;
> odp_port_t tunnel_out_port = ODPP_NONE;
>
> -if (!ipfix || ctx->xin->flow.in_port.ofp_port == OFPP_NONE) {
> +if (!ipfix ||
> +(output_odp_port == ODPP_NONE &&
> + ctx->xin->flow.in_port.ofp_port == OFPP_NONE)) {
> return;
> }
>
> --
> 2.34.1
>
> ___
> 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] system-dpdk: Fix MFEX logs check.
On 19 Jan 2022, at 18:25, David Marchand wrote:
> Some warning logs must be waived when using the net/pcap DPDK driver.
> Those logs can affect different DPDK drivers (like mlx5) and the tests in
> system-dpdk are not testing MTU and Rx checksum, we might as well ignore
> those warnings from OVS.
>
> Fixes: d446dcb7e03f ("system-dpdk: Refactor common logs matching.")
> Signed-off-by: David Marchand
Reviewed and verified in my setup (where I did not get these specific errors),
but the tests do pass.
Acked-by: Eelco Chaudron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] inc-proc-eng: use VLOG_INFO_RL for recompute time over 500ms
On 12/21/21 18:50, Lorenzo Bianconi wrote:
> Use VLOG_INFO_RL in engine_recompute and engine_compute routines
> to log compute time over a given threshold (i.e. 500ms).
> This change will be useful during controller debugging.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1988555
> Signed-off-by: Lorenzo Bianconi
> ---
Hi Lorenzo,
This works but I have a couple minor comments.
> lib/inc-proc-eng.c | 27 +--
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c
> index 2958a55e3..1681fc5ec 100644
> --- a/lib/inc-proc-eng.c
> +++ b/lib/inc-proc-eng.c
> @@ -328,6 +328,7 @@ engine_init_run(void)
> }
> }
>
> +#define ENGINE_COMPUTE_LOG_TO 500 /* ms */
Please move this to the beginning of the file where all declarations are
located.
Please add a prefix to make it explicit that this timeout is in msec, e.g.:
#define ENGINE_COMPUTE_LOG_TO_MS 500 /* ms */
> /* Do a full recompute (or at least try). If we're not allowed then
> * mark the node as "aborted".
> */
> @@ -361,8 +362,15 @@ engine_recompute(struct engine_node *node, bool allowed,
> long long int now = time_msec();
> node->run(node, node->data);
> node->stats.recompute++;
> -VLOG_DBG("node: %s, recompute (%s) took %lldms", node->name, reason,
> - time_msec() - now);
> +long long int delta_time = time_msec() - now;
> +if (delta_time > ENGINE_COMPUTE_LOG_TO) {
Can we make the timeout configurable? E.g., through an appctl like
"inc-engine/compute-log-timeout".
Thanks,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
