Re: [ovs-dev] [PATCH v6] db-ctl-base: add uuid specified method for create cmd
Hi Ben:
>>You added two copies of the following.
>>+ if (!uuid_from_string(uuid_from_cmd, row_uuid)) {
>>+ ctl_error(ctx, "row-uuid '%s' is not a valid UUID", row_uuid);
>>+ return;
>>+ }
This check block has been moved outside for v8. So there is only one uuid check
block now.
>>There is no test that uses an ID on a command *before* the "create">>command.
>>The documentation says this works. Please test it.
I see what you mean. Yes, if we want to use both --id and --row-uuid, and the
record is
referenced "before" the "create" cmd, it won't work. The reason is cmd "create"
executes
later, so the formal record won't take use of the symbol table entry with right
uuid.
I haven't come up with a solution yet, so we changed the db-ctl-base document
in v8
and add a restriction for --id and --row-uuid with create action.
Many thanks for your advice!
Best Regards
Timo
Re: Re: [ovs-dev] [PATCH v6] db-ctl-base: add uuid specified method for create
cmd> >This introduces duplicate code blocks into cmd_create(). Don't do that.
> We can't understand: do you mean that the row-uuid code block under if(id) is
> redundant ? Or we should add some additional code block ?
You added two copies of the following.
+if (!uuid_from_string(uuid_from_cmd, row_uuid)) {
+ctl_error(ctx, "row-uuid '%s' is not a valid UUID", row_uuid);
+return;
+}
> >Please add a test that using an id before creating the record works with
> >--row-uuid. I do not think that it currently works.
> In patch v6
> We have added a test in ovs-vsctl.at which is about --row-uuid after --id for
> create cmd.
There is no test that uses an ID on a command *before* the "create"
command. The documentation says this works. Please test it.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v8] db-ctl-base: add uuid specified method for create cmd
Commit a529e3cd1f (ovsdb-server: Allow OVSDB clients to specify the
UUID for inserted rows) solves ovsdb-client specifing the UUID for
insert operation. OVSDB now can support directly using uuid to identify
a row. But for xxxctl tool,specifying uuid when creating a row is not
yet supported . This patch tried to specify uuid when creating a row
by the ctl tools. A new parameter --row_uuid is added to setup row's UUID.
e.g. ovn-nbctl --row_uuid=3da0398b-a5a8-4bc9-808d-fa662865138f create
logical_switch name='abc'
Author: Liu Chang
Co-authored-by: Tao YunXiang
Co-authored-by: Rong Yin
Signed-off-by: Tao YunXiang
Signed-off-by: Liu Chang
Signed-off-by: Rong Yin
CC: Ben Pfaff
---
v7: db-ctl-base: add uuid specified method for create cmd
v6: db-ctl-base: add uuid specified method for create cmd
v5: db-ctl-base: add uuid specified method for create cmd
---
NEWS| 1 +
lib/db-ctl-base.c | 17 +++-
lib/db-ctl-base.xml | 11 ++-
lib/ovsdb-idl.c | 24 ++
lib/ovsdb-idl.h | 1 +
tests/ovs-vsctl.at | 57 +
6 files changed, 105 insertions(+), 6 deletions(-)
diff --git a/NEWS b/NEWS
index 32ca2e0c6..27fc3ae9d 100644
--- a/NEWS
+++ b/NEWS
@@ -54,6 +54,7 @@ v2.13.0 - 14 Feb 2020
replication server. This value is configurable with the unixctl
command - ovsdb-server/set-active-ovsdb-server-probe-interval.
* ovsdb-server: New OVSDB extension to allow clients to specify row UUIDs.
+ * db-ctl-base: New row_uuid parameter specified for create cmd
- 'ovs-appctl dpctl/dump-flows' can now show offloaded=partial for
partially offloaded flows, dp:dpdk for fully offloaded by dpdk, and
type filter supports new filters: "dpdk" and "partially-offloaded".
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index ab2af9eda..8035cdebf 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1718,16 +1718,24 @@ static void
cmd_create(struct ctl_context *ctx)
{
const char *id = shash_find_data(>options, "--id");
+const char *row_uuid = shash_find_data(>options, "--row-uuid");
const char *table_name = ctx->argv[1];
const struct ovsdb_idl_table_class *table;
const struct ovsdb_idl_row *row;
const struct uuid *uuid = NULL;
+struct uuid uuid_from_cmd;
int i;
ctx->error = get_table(table_name, );
if (ctx->error) {
return;
}
+if (row_uuid) {
+if (!uuid_from_string(_from_cmd, row_uuid)) {
+ctl_error(ctx, "row-uuid '%s' is not a valid UUID", row_uuid);
+return;
+}
+}
if (id) {
struct ovsdb_symbol *symbol = NULL;
@@ -1741,7 +1749,14 @@ cmd_create(struct ctl_context *ctx)
* warnings about that by pretending that there is a reference. */
symbol->strong_ref = true;
}
+if (row_uuid) {
+symbol->uuid = uuid_from_cmd;
+ovsdb_idl_txn_set_uuid_specified(ctx->txn);
+}
uuid = >uuid;
+} else if (row_uuid) {
+uuid = _from_cmd;
+ovsdb_idl_txn_set_uuid_specified(ctx->txn);
}
row = ovsdb_idl_txn_insert(ctx->txn, table, uuid);
@@ -2465,7 +2480,7 @@ static const struct ctl_command_syntax db_ctl_commands[]
= {
{"clear", 3, INT_MAX, "TABLE RECORD COLUMN...", pre_cmd_clear, cmd_clear,
NULL, "--if-exists", RW},
{"create", 2, INT_MAX, "TABLE COLUMN[:KEY]=VALUE...", pre_create,
- cmd_create, post_create, "--id=", RW},
+ cmd_create, post_create, "--id=,--row-uuid=", RW},
{"destroy", 1, INT_MAX, "TABLE [RECORD]...", pre_cmd_destroy, cmd_destroy,
NULL, "--if-exists,--all", RW},
{"wait-until", 2, INT_MAX, "TABLE RECORD [COLUMN[:KEY]=VALUE]...",
diff --git a/lib/db-ctl-base.xml b/lib/db-ctl-base.xml
index 10124c3ad..2804f0865 100644
--- a/lib/db-ctl-base.xml
+++ b/lib/db-ctl-base.xml
@@ -290,7 +290,7 @@
-[--id=@name] create table
column[:key]=value...
+[--id=@name]
[--row-uuid=uuid] create table
column[:key]=value...
Creates a new record in table and sets the initial values of
@@ -303,6 +303,15 @@
invocation in contexts where a UUID is expected. Such references may
precede or follow the create command.
+
+If uuid is supplied and is reasonable, then
the UUID for the
+new row may be specified as expected.
+
+
+If both @name and uuid
are specified,
+the new row with corresponding uuid may still be referred to by that
name, but this
+references can only follow the create command.
+
Caution (ovs-vsctl as example)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 190143f36..505ed00d9 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -281,6 +281,7 @@ struct ovsdb_idl_txn {
char *error;
bool dry_run;
struct ds comment;
+bool
[ovs-dev] Sua Net Fatura Por E-mail Chegou 03/2020 - E4O46OFG41
Seu servidor de e-mail não suporta mensagem HTML ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4] travis: Enable OvS Travis CI for arm
On 3/24/20 8:00 AM, Lance Yang wrote: > Enable part of travis jobs with gcc compiler for arm64 architecture > > 1. Add arm jobs into the matrix in .travis.yml configuration file > 2. To enable OVS-DPDK jobs, set the build target according to > different CPU architectures > 3. Temporarily disable sparse checker because of static code checking > failure on arm64 > > Considering the balance of the CI coverage and running time, some kernel > and DPDK jobs are removed from Arm CI. > > Successful travis build jobs report: > https://travis-ci.org/github/yzyuestc/ovs/builds/666129448 > > Reviewed-by: Yanqin Wei > Reviewed-by: Ruifeng Wang > Reviewed-by: JingZhao Ni > Reviewed-by: Gavin Hu > Signed-off-by: Lance Yang 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] [PATCHv2] netdev-afxdp: Add interrupt mode netdev class.
On 3/6/20 5:11 PM, Ilya Maximets wrote:
> On 3/4/20 7:09 PM, William Tu wrote:
>> On Fri, Feb 28, 2020 at 12:12 AM Ilya Maximets wrote:
>>>
>>> On 2/27/20 8:52 PM, William Tu wrote:
On Thu, Feb 27, 2020 at 11:10 AM William Tu wrote:
>
> On Thu, Feb 27, 2020 at 9:54 AM Ilya Maximets wrote:
>>
>> On 2/27/20 6:51 PM, William Tu wrote:
>>> On Thu, Feb 27, 2020 at 9:42 AM Ilya Maximets
>>> wrote:
On 2/27/20 6:30 PM, William Tu wrote:
> The patch adds a new netdev class 'afxdp-nonpmd' to enable afxdp
> interrupt mode. This is similar to 'type=afxdp', except that the
> is_pmd field is set to false. As a result, the packet processing
> is handled by main thread, not pmd thread. This avoids burning
> the CPU to always 100% when there is no traffic.
>
> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/655885506
> Signed-off-by: William Tu
> ---
> NEWS | 3 +++
> lib/netdev-afxdp.c| 20 +++-
> lib/netdev-linux.c| 20
> lib/netdev-provider.h | 1 +
> lib/netdev.c | 1 +
> tests/system-afxdp.at | 23 +++
> 6 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/NEWS b/NEWS
> index f62ef1f47ea8..594c55dc11d6 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -4,6 +4,9 @@ Post-v2.13.0
> - OpenFlow:
> * The OpenFlow ofp_desc/serial_num may now be configured by
> setting the
> value of other-config:dp-sn in the Bridge table.
> + - AF_XDP:
> + * New netdev class 'afxdp-nonpmd' for netdev-afxdp to save CPU
> cycles
> + by enabling interrupt mode.
>
>
> v2.13.0 - 14 Feb 2020
> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> index 482400d8d135..cd2c7c381139 100644
> --- a/lib/netdev-afxdp.c
> +++ b/lib/netdev-afxdp.c
> @@ -169,6 +169,12 @@ struct netdev_afxdp_tx_lock {
> );
> };
>
> +static int nonpmd_cnt; /* Number of afxdp netdevs in non-pmd mode.
> */
> +static bool
> +netdev_is_afxdp_nonpmd(struct netdev *netdev) {
> +return netdev_get_class(netdev) == _afxdp_nonpmd_class;
> +}
> +
> #ifdef HAVE_XDP_NEED_WAKEUP
> static inline void
> xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
> @@ -1115,7 +1121,10 @@ netdev_afxdp_batch_send(struct netdev *netdev,
> int qid,
> struct netdev_linux *dev;
> int ret;
>
> -if (concurrent_txq) {
> +/* Lock is required when mixing afxdp pmd and nonpmd mode.
> + * ex: one device is created 'afxdp', the other is
> 'afxdp-nonpmd'.
> + */
> +if (concurrent_txq || (nonpmd_cnt != 0)) {
I'm confused about this change. Are you expecting same device being
opened twice with different netdev type? Database should not allow
this.
>>>
>>> Not the same device.
>>>
>>> For example, dev1 opened as 'afxdp', and dev2 opened as 'afxdp-nonpmd'.
>>> When dev2 receives a packet and send to dev1, the main thread used by
>>> dev2
>>> is calling the send(), while it is possible that dev2's pmd thread is
>>> also calling
>>> send(). So need a lock here.
>>
>> But they will send to different tx queues.
>
> OK, I think you are talking about the XPS feature (dynamic txqs).
> I thought XPS can only work when all between pmd threads.
> So adding a lock here.
> The patch currently doesn't work without the lock. Let me investigate
> more.
>
More details.
On my test using veth (only have 1 rxq 1 txq)
Somehow the main thread is assigned to use queue id = 2, which causes
segfault.
>>>
>>> That is very strange...
>>> Could you, please, share your test setup? I'll try to reproduce.
>>>
>>> Best regards, Ilya Maximets.
>>
>> Sorry, somehow I miss your reply in my mailbox.
>> I simply tested it using two namespaces, one device as afxdp, the
>> other as afxdp-nonpmd.
>> ---
>> # start ovs-vswitchd and ovsdb-server
>>
>> ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
>> ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0xf
>>
>> ip netns add at_ns0
>> ip link add p0 type veth peer name afxdp-p0
>> ip link set p0 netns at_ns0
>> ip link set dev afxdp-p0 up
>> ovs-vsctl add-port br0 afxdp-p0
>> ovs-vsctl -- set interface afxdp-p0 options:n_rxq=1
>> type="afxdp-nonpmd" options:xdp-mode=generic
>>
>> ip netns exec at_ns0 sh << NS_EXEC_HEREDOC
>> ip addr add "10.1.1.1/24" dev p0
>> ip link set dev p0 up
>> NS_EXEC_HEREDOC
>>
>> ip netns add at_ns1
>> ip
[ovs-dev] [PATCH] dpif-netdev: Force port reconfiguration to change dynamic_txqs.
In case number of polling threads goes from exact number of Tx queues
in port to higher value while set_tx_multiq() not implemented or not
requesting reconfiguration, port will not be reconfigured and datapath
will continue using static Tx queue ids leading to crash.
Ex.:
Assuming that port p0 supports up to 4 Tx queues and doesn't support
set_tx_multiq() method. For example, netdev-afxdp could be the case,
because it could have multiple Tx queues, but doesn't have
set_tx_multiq() implementation because number of Tx queues always
equals to number of Rx queues.
1. Configuring pmd-cpu-mask to have 3 pmd threads.
2. Adding port p0 to OVS.
At this point wanted_txqs = 4 (3 for pmd threads + 1 for non-pmd).
Port reconfigured to have 4 Tx queues successfully.
dynamic_txqs = (4 < 4) = false;
3. Configuring pmd-cpu-mask to have 10 pmd threads.
At this point wanted_txqs = 11 (10 for pmd threads + 1 for non-pmd).
Since set_tx_multiq() is not implemented, netdev doesn't request
reconfiguration and 'dynamic_txqs' remains in 'false' state.
4. Since 'dynamic_txqs == false', dpif-netdev uses static Tx queue
ids that are in range [0, 10] while device only supports 4 leading
to unwanted behavior and crashes.
Fix that by marking for reconfiguration all the ports that will likely
change their 'dynamic_txqs' value.
It looks like the issue could be reproduced only with afxdp ports,
because all other non-dpdk ports ignores Tx queue ids and dpdk ports
requests for reconfiguration on set_tx_multiq().
Reported-by: William Tu
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2020-March/368364.html
Fixes: e32971b8ddb4 ("dpif-netdev: Centralized threads and queues handling
code.")
Signed-off-by: Ilya Maximets
---
Since the issue could be reproduced only with afxdp ports, this should
be backported only down to 2.12.
lib/dpif-netdev.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a798db45d..e456cc9be 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4941,9 +4941,17 @@ reconfigure_datapath(struct dp_netdev *dp)
/* Check for all the ports that need reconfiguration. We cache this in
* 'port->need_reconfigure', because netdev_is_reconf_required() can
- * change at any time. */
+ * change at any time.
+ * Also mark for reconfiguration all ports which will likely change their
+ * 'dynamic_txqs' parameter. It's required to stop using them before
+ * changing this setting and it's simpler to mark ports here and allow
+ * 'pmd_remove_stale_ports' to remove them from threads. There will be
+ * no actual reconfiguration in 'port_reconfigure' because it's
+ * unnecessary. */
HMAP_FOR_EACH (port, node, >ports) {
-if (netdev_is_reconf_required(port->netdev)) {
+if (netdev_is_reconf_required(port->netdev)
+|| (port->dynamic_txqs
+!= (netdev_n_txq(port->netdev) < wanted_txqs))) {
port->need_reconfigure = true;
}
}
--
2.25.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] I await your reply soonest.
Dear Friend, How are you and your entire family? I hope you and your entire family are doing everything possible to stay safe and protected from the coronavirus pandemic. I am your friend Mr. Ismael from the Arab Republic of Syrian. I am eagerly looking for an investment opportunity in your country, can you be of help? Please, it would be nice if I can do some investment in your care and if you are interested please reply me to my private email address: (gc44.271...@gmail.com) so that I could provide you with more details of the investment funds. Thanks for your kind attention. >From me - Ismael ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv9 1/2] userspace: Enable TSO support for non-DPDK.
This patch enables TSO support for non-DPDK use cases, and
also add check-system-tso testsuite. Before TSO, we have to
disable checksum offload, allowing the kernel to calculate the
TCP/UDP packet checsum. With TSO, we can skip the checksum
validation by enabling checksum offload, and with large packet
size, we see better performance.
Consider container to container use cases:
iperf3 -c (ns0) -> veth peer -> OVS -> veth peer -> iperf3 -s (ns1)
And I got around 6Gbps, similar to TSO with DPDK-enabled.
Signed-off-by: William Tu
Acked-by: Flavio Leitner
---
v9:
- make naming of flags more clear
- I couldn't think of any smart MACRO
- travis: https://travis-ci.org/github/williamtu/ovs-travis/builds/666513254
v8:
- make some namings more clear
v7: more refactoring on functions
- rss and flow mark related functions.
- dp_packet_clone_with_headroom
- fix definitions of DP_PACKET_OL_FLOW_MARK_MASK
- travis: https://travis-ci.org/github/williamtu/ovs-travis/builds/663658338
v6: fix indentation
v5: feedback from Flavio
- move some code around, break the long line
- travis is now working
https://travis-ci.org/github/williamtu/ovs-travis/builds/661607017
v4:
- Avoid duplications of DPDK and non-DPDK code by
refactoring the definition of DP_PACKET_OL flags
and relevant functions
- I got weird error in travis (I think this is unrelated)
https://travis-ci.org/github/williamtu/ovs-travis/builds/661313463
sindex.c:378:26: error: unknown type name ‘sqlite3_str’
static int query_appendf(sqlite3_str *query, const char *fmt, ...)
- test compile ok on dpdk and non-dpdk, make check-system-tso still
has a couple fails
v3:
- fix comments and some coding style
- add valgrind check
- travis: https://travis-ci.org/williamtu/ovs-travis/builds/660394007
v2:
- add make check-system-tso test
- combine logging for dpdk and non-dpdk
- I'm surprised that most of the test cases passed.
This is due to few tests using tcp/udp, so it does not trigger
TSO. I saw only geneve/vxlan fails randomly, maybe we can
check it later.
---
lib/dp-packet.c | 6 +-
lib/dp-packet.h | 572 +++---
lib/userspace-tso.c | 5 -
tests/.gitignore | 3 +
tests/automake.mk | 21 ++
tests/system-tso-macros.at| 31 +++
tests/system-tso-testsuite.at | 26 ++
7 files changed, 339 insertions(+), 325 deletions(-)
create mode 100644 tests/system-tso-macros.at
create mode 100644 tests/system-tso-testsuite.at
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index cd2623500e3d..72f6d09ac7f3 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -192,10 +192,8 @@ dp_packet_clone_with_headroom(const struct dp_packet
*buffer, size_t headroom)
sizeof(struct dp_packet) -
offsetof(struct dp_packet, l2_pad_size));
-#ifdef DPDK_NETDEV
-new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags;
-new_buffer->mbuf.ol_flags &= ~DPDK_MBUF_NON_OFFLOADING_FLAGS;
-#endif
+*dp_packet_ol_flags_ptr(new_buffer) = *dp_packet_ol_flags_ptr(buffer);
+*dp_packet_ol_flags_ptr(new_buffer) &= DP_PACKET_OL_SUPPORTED_MASK;
if (dp_packet_rss_valid(buffer)) {
dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer));
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 9f8991faad52..4c127e759e6d 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -47,19 +47,62 @@ enum OVS_PACKED_ENUM dp_packet_source {
};
#define DP_PACKET_CONTEXT_SIZE 64
+#ifdef DPDK_NETDEV
+#define DEF_OL_FLAG(NAME, DPDK_DEF, GENERIC_DEF) NAME = DPDK_DEF
+#else
+#define DEF_OL_FLAG(NAME, DPDK_DEF, GENERIC_DEF) NAME = GENERIC_DEF
+#endif
-#ifndef DPDK_NETDEV
/* Bit masks for the 'ol_flags' member of the 'dp_packet' structure. */
enum dp_packet_offload_mask {
-DP_PACKET_OL_RSS_HASH_MASK = 0x1, /* Is the 'rss_hash' valid? */
-DP_PACKET_OL_FLOW_MARK_MASK = 0x2, /* Is the 'flow_mark' valid? */
+/* Value 0 is not used. */
+/* Is the 'rss_hash' valid? */
+DEF_OL_FLAG(DP_PACKET_OL_RSS_HASH, PKT_RX_RSS_HASH, 0x1),
+/* Is the 'flow_mark' valid? (DPDK does not support) */
+DEF_OL_FLAG(DP_PACKET_OL_FLOW_MARK, PKT_RX_FDIR_ID, 0x2),
+/* Bad L4 checksum in the packet. */
+DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD, 0x4),
+/* Bad IP checksum in the packet. */
+DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_BAD, PKT_RX_IP_CKSUM_BAD, 0x8),
+/* Valid L4 checksum in the packet. */
+DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_GOOD, PKT_RX_L4_CKSUM_GOOD, 0x10),
+/* Valid IP checksum in the packet. */
+DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_GOOD, PKT_RX_IP_CKSUM_GOOD, 0x20),
+/* TCP Segmentation Offload. */
+DEF_OL_FLAG(DP_PACKET_OL_TX_TCP_SEG, PKT_TX_TCP_SEG, 0x40),
+/* Offloaded packet is IPv4. */
+DEF_OL_FLAG(DP_PACKET_OL_TX_IPV4, PKT_TX_IPV4, 0x80),
+/* Offloaded packet is IPv6. */
+
[ovs-dev] [PATCHv9 2/2] tests: Add tests using tap device.
Similar to using veth across namespaces, this patch creates tap devices, assigns to namespaces, and allows traffic to go through different test cases. Signed-off-by: William Tu Acked-by: Flavio Leitner --- tests/automake.mk | 1 + tests/system-tap.at | 34 ++ tests/system-tso-testsuite.at | 1 + 3 files changed, 36 insertions(+) create mode 100644 tests/system-tap.at diff --git a/tests/automake.mk b/tests/automake.mk index 66859d5377c3..cbba5b170427 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -158,6 +158,7 @@ SYSTEM_USERSPACE_TESTSUITE_AT = \ SYSTEM_TSO_TESTSUITE_AT = \ tests/system-tso-testsuite.at \ + tests/system-tap.at \ tests/system-tso-macros.at SYSTEM_AFXDP_TESTSUITE_AT = \ diff --git a/tests/system-tap.at b/tests/system-tap.at new file mode 100644 index ..871a3bda4fcc --- /dev/null +++ b/tests/system-tap.at @@ -0,0 +1,34 @@ +AT_SETUP([traffic between namespaces using tap]) +AT_KEYWORDS([http_tap]) +OVS_TRAFFIC_VSWITCHD_START() +AT_SKIP_IF([test $HAVE_TUNCTL = no]) + +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) + +ADD_NAMESPACES(at_ns0, at_ns1) + +AT_CHECK([ip tuntap add tap0 mode tap]) +on_exit 'ip tuntap del tap0 mode tap' +AT_CHECK([ip tuntap add tap1 mode tap]) +on_exit 'ip tuntap del tap1 mode tap' + +AT_CHECK([ovs-vsctl add-port br0 tap0 -- set int tap0 type=tap]) +AT_CHECK([ovs-vsctl add-port br0 tap1 -- set int tap1 type=tap]) +AT_CHECK([ip link set tap0 netns at_ns0]) +AT_CHECK([ip link set tap1 netns at_ns1]) + +AT_CHECK([ip netns exec at_ns0 ip link set dev tap0 up]) +AT_CHECK([ip netns exec at_ns1 ip link set dev tap1 up]) +AT_CHECK([ip netns exec at_ns0 ip addr add 10.1.1.1/24 dev tap0]) +AT_CHECK([ip netns exec at_ns1 ip addr add 10.1.1.2/24 dev tap1]) + +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl +3 packets transmitted, 3 received, 0% packet loss, time 0ms +]) + +OVS_START_L7([at_ns1], [http]) +NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o wget0.log]) + +OVS_TRAFFIC_VSWITCHD_STOP(["/.*ethtool command ETHTOOL_G.*/d"]) + +AT_CLEANUP diff --git a/tests/system-tso-testsuite.at b/tests/system-tso-testsuite.at index 99d748006a86..594d1a6fde85 100644 --- a/tests/system-tso-testsuite.at +++ b/tests/system-tso-testsuite.at @@ -23,4 +23,5 @@ m4_include([tests/system-common-macros.at]) m4_include([tests/system-userspace-macros.at]) m4_include([tests/system-tso-macros.at]) +m4_include([tests/system-tap.at]) m4_include([tests/system-traffic.at]) -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv8] userspace: Add GTP-U support.
GTP, GPRS Tunneling Protocol, is a group of IP-based communications protocols used to carry general packet radio service (GPRS) within GSM, UMTS and LTE networks. GTP protocol has two parts: Signalling (GTP-Control, GTP-C) and User data (GTP-User, GTP-U). GTP-C is used for setting up GTP-U protocol, which is an IP-in-UDP tunneling protocol. Usually GTP is used in connecting between base station for radio, Serving Gateway (S-GW), and PDN Gateway (P-GW). This patch implements GTP-U protocol for userspace datapath, supporting only required header fields and G-PDU message type. See spec in: https://tools.ietf.org/html/draft-hmm-dmm-5g-uplane-analysis-00 Signed-off-by: Feng Yang Co-authored-by: Feng Yang Signed-off-by: Yi Yang Co-authored-by: Yi Yang Signed-off-by: William Tu Acked-by: Ben Pfaff --- I tested it again and thinking about applying this patch if no more comments. v7 -> v8: - Add Feng Yang as co-authored - fix checkpatch error - rebase to master - https://travis-ci.org/github/williamtu/ovs-travis/builds/666518784 v6 -> v7: - address Ben's feedback - function name: use netdev_tnl_calc_udp_csum - remove unnecessary be32_to_be16(htonl - use SCNi8 instead of SCNx8 - Travis: https://travis-ci.org/williamtu/ovs-travis/builds/638683932 v5 -> v6: - rebase to master - travis: https://travis-ci.org/williamtu/ovs-travis/builds/638083655 v4 -> v5: - address Ben and Aaron comments 1) flow_get_metadata, format_flow_tunnel 2) use of ?: in MSVS 3) tun_key_to_attr 4) handling PT_GTPU_MSG packets 5) make gtpu_flags and gtpu_msgtype read-only - use be32_to_be64 and be64_to_be32 - make gtpu_flags as hexadicimal, gtpu_msgtype as decimal - remove PT_GTPU_MSG Address Roni's comments - for non-GPDU msgtype, don't pop header - add seq number flags parsing, push/pop header support - refactor netdev_tnl_push_udp_header() v3 -> v4: - applied Ben's doc revise - increment FLOW_WC_SEQ to 42 - minor fixes - travis: https://travis-ci.org/williamtu/ovs-travis/builds/621289678 v2 -> v3: - pick up the code from v2, rebase to master - many fixes in code, docs, and add more tests - travis: https://travis-ci.org/williamtu/ovs-travis/builds/619799361 v1 -> v2: - Add new packet_type PT_GTPU_MSG to handle GTP-U signaling message, GTP-U signaling message should be steered into a control plane handler by explicit openflow entry in flow table. - Fix gtpu_flags and gptu_msgtype set issue. - Verify communication with kernel GTP implementation from Jiannan Ouyang. - Fix unit test issue and make sure all the unit tests can pass. - This patch series add GTP-U tunnel support in DPDK userspace, GTP-U is used for carrying user data within the GPRS core network and between the radio access network and the core network. The user data transported can be packets in any of IPv4, IPv6, or PPP formats. --- Documentation/faq/configuration.rst | 13 ++ Documentation/faq/releases.rst| 1 + NEWS | 3 + datapath/linux/compat/include/linux/openvswitch.h | 2 + include/openvswitch/flow.h| 4 +- include/openvswitch/match.h | 6 + include/openvswitch/meta-flow.h | 28 include/openvswitch/packets.h | 4 +- lib/dpif-netlink-rtnl.c | 5 + lib/dpif-netlink.c| 5 + lib/flow.c| 28 ++-- lib/flow.h| 2 +- lib/match.c | 36 - lib/meta-flow.c | 38 + lib/meta-flow.xml | 79 ++- lib/netdev-native-tnl.c | 165 -- lib/netdev-native-tnl.h | 13 ++ lib/netdev-vport.c| 25 +++- lib/nx-match.c| 8 +- lib/odp-util.c| 123 +++- lib/odp-util.h| 2 +- lib/ofp-match.c | 2 +- lib/packets.h | 68 + lib/tnl-ports.c | 3 + ofproto/ofproto-dpif-rid.h| 2 +- ofproto/ofproto-dpif-xlate.c | 3 +- tests/ofproto.at | 2 +- tests/tunnel-push-pop.at | 22 +++ tests/tunnel.at | 76 ++ vswitchd/vswitch.xml | 24 30 files changed, 752 insertions(+), 40 deletions(-) diff --git a/Documentation/faq/configuration.rst b/Documentation/faq/configuration.rst index ff3b71a5d4ef..4a98740c5d4d 100644 ---
[ovs-dev] Pls read my message
My Name is Bahar Zanjani Siblings to 45 years old Babak Morteza Zanjani an Iranian billionaire and business magnate apprehended on 2013 sentenced to death penalty and partner to Jailed Iranian Diplomat Ali Reza Monfared. My siblings Babak Morteza Zanjani was the managing director of the UAE-based Sorinet Group, one of Iran's largest business conglomerates from Tehran, Iran currently on death penalty . Due to current sanctions and unrest in my country Iran , I am looking for a relationship with a business or individual in your country so that I can make some investments there. Please, reply me so we can discuss further as you keep this privy .Thanks for your cooperation Bahar Zanjani ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH branch-2.11 2/2] ovsdb-tool: fix memory leak while converting cluster into standalone database
Bleep bloop. Greetings aginwala aginwala, 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 , Aliasgar Ginwala Lines checked: 112, 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 branch-2.11 1/2] ovsdb-tool: Convert clustered db to standalone db.
Bleep bloop. Greetings aginwala aginwala, 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: 257, 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] [PATCHv2] github-ci: Enable github actions.
Bleep bloop. Greetings William Tu, 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 192 characters long (recommended limit is 79) #44 FILE: .github/workflows/ovs.yml:15: run: sudo apt install -y bison flex; git clone -b v5.0.0 git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git; cd iproute2; ./configure && make -j2 && sudo make install Lines checked: 70, 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] Conntrack with SCTP: +est is never reached.
The problem ended up being an OVN bug in the ordering of NAT operations in flows. These particular packets are SNAT'ed and DNAT'ed. OVN is committing them in an opposite order than it is handling subsequent packets in established state. In other words, subsequent packets are being sent to CT to do an SNAT, where they do not match the CT session, and then in table 14, ct matches this as invalid, because the packet did not match in the previous CT call. It's not an SCTP specific problem, it happens in TCP as well. For more info: https://bugzilla.redhat.com/show_bug.cgi?id=1718372#c18 https://bugzilla.redhat.com/show_bug.cgi?id=1815217#c12 Tim Rozet Red Hat CTO Networking Team On Sun, Mar 22, 2020 at 11:47 AM Aaron Conole wrote: > Ben Pfaff writes: > > > If I'm interpreting this correctly, then it sounds like OVS does > > work with SCTP. If so, and if OVN does not, then it seems > > likely that OVN is not using OVS conntrack correctly in some way > > for SCTP. (Or possibly that Tim and Aaron are using different > > versions of OVS or the kernel or the OVS kernel module, such > > that Tim is seeing an OVS kernel or conntrack bug that Aaron is > > not.) > > I tried with a couple different versions of OvS and kernel, so I > think it's probably not that. I'm hoping Tim can confirm on > Monday for us. I also hacked up my kernel to print out the state > flags to make sure I wasn't missing anything, and they're > displaying as expected (from an IRC conversation): 14:50 > I see the state flags as 0x22 and 0x2a > 14:50 with the initials as 0x21 > 14:52 hm > 14:53 so, 0x22 = OVS_CS_F_TRACKED | OVS_CS_F_ESTABLISHED > 14:53 0x2a = OVS_CS_F_TRACKED | OVS_CS_F_ESTABLISHED | > OVS_CS_F_REPLY_DIR > 14:54 0x21 = OVS_CS_F_TRACKED | OVS_CS_F_NEW > > First two packets (INIT and INIT_ACK) are 0x21, and after that it's > 0x22/0x2a (so it looks like even the COOKIE packets are flagged as +est > state packets). I hope I didn't miss anything. > > > I haven't looked at any details of OVS or OVN, so the above is just idle > > speculation. > > > > On Fri, Mar 20, 2020 at 03:22:10PM -0400, Aaron Conole wrote: > >> Aaron Conole writes: > >> > >> > Aaron Conole writes: > >> > > >> >> Tim Rozet writes: > >> >> > >> >>> I filed https://bugzilla.redhat.com/show_bug.cgi?id=1815217 to > track this issue. > >> >> > >> >> Thanks! > >> > > >> > I tested with the following setup (no modifications to kernel or ovs): > >> > >> Final update below > >> > >> > # using kernel 5.6.0-rc6+, ovs master built using make rpm-fedora and > installed > >> > > >> > ip netns add left > >> > ip netns add right > >> > ip link add center-left type veth peer name left0 > >> > ip link add center-right type veth peer name right0 > >> > ip link set center-left up > >> > ip link set center-right up > >> > ip link set left0 netns left > >> > ip link set right0 netns right > >> > ip netns exec left ip addr add 172.31.110.1/30 dev left0 > >> > ip netns exec right ip addr add 172.31.110.2/30 dev right0 > >> > ip netns exec left ip link set left0 up > >> > ip netns exec right ip link set right0 up > >> > > >> > # just to ignore any possible selinux issues... > >> > setenforce Permissive > >> > systemctl start openvswitch > >> > > >> > systemctl start openvswitch > >> > ovs-vsctl add-br br0 -- set Bridge br0 fail-mode=secure > >> > ovs-vsctl add-port br0 center-left > >> > ovs-vsctl add-port br0 center-right > >> > ovs-ofctl add-flow br0 table=0,arp,action=NORMAL > >> > > >> > ovs-ofctl add-flow br0 'table=0,sctp,actions=ct(table=1)' > >> > ovs-ofctl add-flow br0 \ > >> > > > 'table=1,sctp,in_port=center-left,ct_state=+trk+new,actions=ct(commit),center-right' > >> > >> > >> > ovs-ofctl add-flow br0 \ > >> > > 'table=1,sctp,in_port=center-right,ct_state=+rpl+trk,actions=center-left' > >> > ovs-ofctl add-flow br0 \ > >> > >> ^^ This flow isn't actually needed. > >> > >> Additionally, I tested with the rhel8 kernel, and FDP from rhel8 > >> (openvswitch2.12-2.12.0-22), and still was able to get an SCTP > >> connection. > >> > >> > > 'table=1,sctp,in_port=center-left,ct_state=+trk+est,actions=center-right' > >> > ovs-ofctl add-flow br0 \ > >> > > 'table=1,sctp,in_port=center-right,ct_state=+trk+est,actions=center-left' > >> > > >> > # ensure arp is following action normal > >> > ip netns exec left arping 172.31.110.2 -I left0 > >> > > >> > # in one terminal > >> > ip netns exec right ncat --listen --sctp -vv > >> > > >> > # in another terminal > >> > ip netns exec left ncat --sctp 172.31.110.2 31337 > >> > > >> > Result: > >> > > >> > [root@wsfd-netdev92 ~]# ip netns exec right ncat --listen --sctp -vv > >> > Ncat: Version 7.70 ( https://nmap.org/ncat ) > >> > Ncat: Listening on :::31337 > >> > Ncat: Listening on 0.0.0.0:31337 > >> > Ncat: Connection from 172.31.110.1. > >> > Ncat: Connection from 172.31.110.1:34461. > >> > asdf > >> > fdsa > >> > fasdfsadf > >> > asdfasdfasdfda > >> > > >> > > >> > Seems I have bidirectional
Re: [ovs-dev] [PATCH] vlog: Fix OVS_REQUIRES macro.
On Tue, Mar 24, 2020 at 12:31:33PM -0700, Ben Pfaff wrote: > On Tue, Mar 24, 2020 at 12:16:38PM -0700, William Tu wrote: > > Pass lock objects, not their addresses, to the annotation macros. > > > > Signed-off-by: William Tu > > Acked-by: Ben Pfaff Applied, thanks. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
Sounds good Ben. I sent https://patchwork.ozlabs.org/patch/1260937/ and https://patchwork.ozlabs.org/patch/1260935/ for 2.11. On Tue, Mar 24, 2020 at 9:53 AM Ben Pfaff wrote: > Probably best to post a rebased series. > > On Fri, Mar 20, 2020 at 11:43:41AM -0700, aginwala wrote: > > Hi Ben: > > > > I checked the history for 2.11 and this patch is failing to backport due > to > > this commit > > > https://github.com/openvswitch/ovs/commit/5832e6a4f103a1eddd62c9ba459ce669e3a5d132 > > as it updates OVSB fast re-sync in news and its ported 2.12 . Hence, > > backport to 2.12 worked. If we get > > > https://github.com/openvswitch/ovs/commit/5832e6a4f103a1eddd62c9ba459ce669e3a5d132 > > in > > too to 2.11 it will apply cleanly. Let me know else I will have to amend > > the news and it will fail backport for fast re-syc if we move that to > 2.11 > > in future. > > > > On Thu, Mar 19, 2020 at 4:22 PM aginwala wrote: > > > > > Ok sounds good. Will do that. Thanks a ton! > > > > > > On Thu, Mar 19, 2020 at 3:53 PM Ben Pfaff wrote: > > > > > >> I backported both to 2.12. > > >> > > >> They need a manual backport to 2.11, will you take care of it? > > >> > > >> On Thu, Mar 19, 2020 at 03:39:58PM -0700, aginwala wrote: > > >> > Oh I see it seems the previous patch needs to be backported too > > >> > > > >> > https://lists.linuxfoundation.org/pipermail/ovs-dev/2019-September/362925.html > > >> > . > > >> > Please see if you can get that too on 2.11 and 2.12 > > >> > > > >> > > > >> > > > >> > > > >> > On Thu, Mar 19, 2020 at 3:35 PM Ben Pfaff wrote: > > >> > > > >> > > It doesn't apply cleanly. > > >> > > > > >> > > On Thu, Mar 19, 2020 at 03:07:39PM -0700, aginwala wrote: > > >> > > > Hi Ben: > > >> > > > > > >> > > > Thanks for backporting previous patches. Please see if you can > back > > >> port > > >> > > > this one too to 2.11 and 2.12. > > >> > > > > > >> > > > On Thu, Mar 19, 2020 at 1:53 PM aginwala > wrote: > > >> > > > > > >> > > > > Hi Ben: > > >> > > > > Can you also backport this patch to 2.12 and 2.11 too? > > >> > > > > > > >> > > > > On Mon, Oct 7, 2019 at 11:22 AM Ben Pfaff > wrote: > > >> > > > > > > >> > > > >> On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc > wrote: > > >> > > > >> > memory leak is reported by valgrind while executing > functional > > >> test > > >> > > > >> > "ovsdb-tool convert-to-standalone" > > >> > > > >> > > > >> > > > >> > ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 > blocks > > >> are > > >> > > > >> definitely lost in loss record 20 of 20 > > >> > > > >> > ==13842==at 0x4C2DB8F: malloc (in > > >> > > > >> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > > >> > > > >> > ==13842==by 0x45EE2E: xmalloc (util.c:138) > > >> > > > >> > ==13842==by 0x43E386: json_create (json.c:1451) > > >> > > > >> > ==13842==by 0x43BDD2: json_object_create (json.c:254) > > >> > > > >> > ==13842==by 0x43DEE3: json_parser_push_object > (json.c:1273) > > >> > > > >> > ==13842==by 0x43E167: json_parser_input (json.c:1371) > > >> > > > >> > ==13842==by 0x43D6EA: json_lex_input (json.c:991) > > >> > > > >> > ==13842==by 0x43DAC1: json_parser_feed (json.c:1149) > > >> > > > >> > ==13842==by 0x40D108: parse_body (log.c:411) > > >> > > > >> > ==13842==by 0x40D386: ovsdb_log_read (log.c:476) > > >> > > > >> > ==13842==by 0x406A0B: do_convert_to_standalone > > >> > > (ovsdb-tool.c:1571) > > >> > > > >> > ==13842==by 0x406A0B: do_cluster_standalone > > >> (ovsdb-tool.c:1606) > > >> > > > >> > ==13842==by 0x438670: ovs_cmdl_run_command__ > > >> > > (command-line.c:223) > > >> > > > >> > ==13842==by 0x438720: ovs_cmdl_run_command > > >> (command-line.c:254) > > >> > > > >> > ==13842==by 0x405A4C: main (ovsdb-tool.c:79) > > >> > > > >> > > > >> > > > >> > The problem was in do_convert_to_standalone() function > which > > >> while > > >> > > > >> reading log file > > >> > > > >> > allocate json object which was not deallocated at the end. > > >> > > > >> > > > >> > > > >> > Signed-off-by: Damijan Skvarc > > >> > > > >> > > >> > > > >> Applied to master, thanks. > > >> > > > >> ___ > > >> > > > >> 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 branch-2.11 1/2] ovsdb-tool: Convert clustered db to standalone db.
From: Aliasgar Ginwala
Add support in ovsdb-tool for migrating clustered dbs to standalone dbs.
E.g. usage to migrate nb/sb db to standalone db from raft:
ovsdb-tool cluster-to-standalone ovnnb_db.db ovnnb_db_cluster.db
Acked-by: Han Zhou
Signed-off-by: Aliasgar Ginwala
Signed-off-by: Ben Pfaff
---
Documentation/ref/ovsdb.7.rst | 3 +
NEWS | 6 +-
ovsdb/ovsdb-tool.1.in | 8 +++
ovsdb/ovsdb-tool.c| 101 +-
tests/ovsdb-tool.at | 38 +
5 files changed, 154 insertions(+), 2 deletions(-)
diff --git a/Documentation/ref/ovsdb.7.rst b/Documentation/ref/ovsdb.7.rst
index c43fe1709..f6cbc537b 100644
--- a/Documentation/ref/ovsdb.7.rst
+++ b/Documentation/ref/ovsdb.7.rst
@@ -512,6 +512,9 @@ Use ``ovsdb-tool create-cluster`` to create a clustered
database from the
contents of a standalone database. Use ``ovsdb-tool backup`` to create a
standalone database from the contents of a clustered database.
+Use ``ovsdb-tool cluster-to-standalone`` to convert clustered database to
+standalone database when the cluster is down and cannot be revived.
+
Upgrading or Downgrading a Database
---
diff --git a/NEWS b/NEWS
index be1f38e23..dd62abfcc 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,10 @@ v2.11.2 - 03 Sep 2019
- Bug fixes
- DPDK
* OVS validated with DPDK 18.11.2 which is recommended to be used.
+ - OVSDB
+ * Support to convert from cluster database to standalone database is now
+ available when clustered is down and cannot be revived using ovsdb-tool
+ . Check "Database Migration Commands" in ovsdb-tool man section.
v2.11.1 - 30 Mar 2019
-
@@ -1386,4 +1390,4 @@ v0.90.6 - 6 Oct 2009
v0.90.5 - 21 Sep 2009
-
- Generalize in-band control to more diverse network setups
-- Bug fixes
+- Bug fixes
\ No newline at end of file
diff --git a/ovsdb/ovsdb-tool.1.in b/ovsdb/ovsdb-tool.1.in
index ec85e14c4..31a918d90 100644
--- a/ovsdb/ovsdb-tool.1.in
+++ b/ovsdb/ovsdb-tool.1.in
@@ -147,6 +147,14 @@ avoid this possibility, specify \fB\-\-cid=\fIuuid\fR,
where
\fIuuid\fR is the cluster ID of the cluster to join, as printed by
\fBovsdb\-tool get\-cid\fR.
.
+.SS "Database Migration Commands"
+This commands will convert cluster database to standalone database.
+.
+.IP "\fBcluster\-to\-standalone\fI db clusterdb"
+Use this command to convert to standalone database from clustered database
+when the cluster is down and cannot be revived. It creates new standalone
+\fIdb\fR file from the given cluster \fIdb\fR file.
+.
.SS "Version Management Commands"
.so ovsdb/ovsdb-schemas.man
.PP
diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
index 45e656d3d..ba78760ef 100644
--- a/ovsdb/ovsdb-tool.c
+++ b/ovsdb/ovsdb-tool.c
@@ -173,6 +173,9 @@ usage(void)
" compare-versions A OP B compare OVSDB schema version numbers\n"
" query [DB] TRNS execute read-only transaction on DB\n"
" transact [DB] TRNS execute read/write transaction on DB\n"
+ " cluster-to-standalone DB DBConvert clustered DB to\n"
+ " standalone DB when cluster is down and cannot be\n"
+ "revived\n"
" [-m]... show-log [DB] print DB's log entries\n"
"The default DB is %s.\n"
"The default SCHEMA is %s.\n",
@@ -944,6 +947,55 @@ print_raft_record(const struct raft_record *r,
}
}
+static void
+raft_header_to_standalone_log(const struct raft_header *h,
+ struct ovsdb_log *db_log_data)
+{
+if (h->snap_index) {
+if (!h->snap.data || json_array(h->snap.data)->n != 2) {
+ovs_fatal(0, "Incorrect raft header data array length");
+}
+
+struct json *schema_json = json_array(h->snap.data)->elems[0];
+if (schema_json->type != JSON_NULL) {
+struct ovsdb_schema *schema;
+check_ovsdb_error(ovsdb_schema_from_json(schema_json, ));
+ovsdb_schema_destroy(schema);
+check_ovsdb_error(ovsdb_log_write_and_free(db_log_data,
+ schema_json));
+}
+
+struct json *data_json = json_array(h->snap.data)->elems[1];
+if (!data_json || data_json->type != JSON_OBJECT) {
+ovs_fatal(0, "Invalid raft header data");
+}
+if (data_json->type != JSON_NULL) {
+check_ovsdb_error(ovsdb_log_write_and_free(db_log_data,
+ data_json));
+}
+}
+}
+
+static void
+raft_record_to_standalone_log(const struct raft_record *r,
+ struct ovsdb_log *db_log_data)
+{
+if (r->type == RAFT_REC_ENTRY) {
+if (!r->entry.data) {
+return;
+}
+if (json_array(r->entry.data)->n != 2) {
+ovs_fatal(0,
[ovs-dev] [PATCH branch-2.11 2/2] ovsdb-tool: fix memory leak while converting cluster into standalone database
From: Damijan Skvarc
memory leak is reported by valgrind while executing functional test
"ovsdb-tool convert-to-standalone"
==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks are definitely
lost in loss record 20 of 20
==13842==at 0x4C2DB8F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13842==by 0x45EE2E: xmalloc (util.c:138)
==13842==by 0x43E386: json_create (json.c:1451)
==13842==by 0x43BDD2: json_object_create (json.c:254)
==13842==by 0x43DEE3: json_parser_push_object (json.c:1273)
==13842==by 0x43E167: json_parser_input (json.c:1371)
==13842==by 0x43D6EA: json_lex_input (json.c:991)
==13842==by 0x43DAC1: json_parser_feed (json.c:1149)
==13842==by 0x40D108: parse_body (log.c:411)
==13842==by 0x40D386: ovsdb_log_read (log.c:476)
==13842==by 0x406A0B: do_convert_to_standalone (ovsdb-tool.c:1571)
==13842==by 0x406A0B: do_cluster_standalone (ovsdb-tool.c:1606)
==13842==by 0x438670: ovs_cmdl_run_command__ (command-line.c:223)
==13842==by 0x438720: ovs_cmdl_run_command (command-line.c:254)
==13842==by 0x405A4C: main (ovsdb-tool.c:79)
The problem was in do_convert_to_standalone() function which while reading log
file
allocate json object which was not deallocated at the end.
Signed-off-by: Damijan Skvarc
Signed-off-by: Ben Pfaff
Signed-off-by: Aliasgar Ginwala
---
ovsdb/ovsdb-tool.c | 37 +
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
index ba78760ef..91662cab8 100644
--- a/ovsdb/ovsdb-tool.c
+++ b/ovsdb/ovsdb-tool.c
@@ -953,26 +953,30 @@ raft_header_to_standalone_log(const struct raft_header *h,
{
if (h->snap_index) {
if (!h->snap.data || json_array(h->snap.data)->n != 2) {
-ovs_fatal(0, "Incorrect raft header data array length");
+ovs_fatal(0, "Incorrect raft header data array length");
}
-struct json *schema_json = json_array(h->snap.data)->elems[0];
+struct json_array *pa = json_array(h->snap.data);
+struct json *schema_json = pa->elems[0];
+struct ovsdb_error *error = NULL;
+
if (schema_json->type != JSON_NULL) {
struct ovsdb_schema *schema;
check_ovsdb_error(ovsdb_schema_from_json(schema_json, ));
ovsdb_schema_destroy(schema);
-check_ovsdb_error(ovsdb_log_write_and_free(db_log_data,
- schema_json));
+error = ovsdb_log_write(db_log_data, schema_json);
}
-struct json *data_json = json_array(h->snap.data)->elems[1];
-if (!data_json || data_json->type != JSON_OBJECT) {
-ovs_fatal(0, "Invalid raft header data");
-}
-if (data_json->type != JSON_NULL) {
-check_ovsdb_error(ovsdb_log_write_and_free(db_log_data,
- data_json));
+if (!error) {
+struct json *data_json = pa->elems[1];
+if (!data_json || data_json->type != JSON_OBJECT) {
+ovs_fatal(0, "Invalid raft header data");
+}
+if (data_json->type != JSON_NULL) {
+error = ovsdb_log_write(db_log_data, data_json);
+}
}
+check_ovsdb_error(error);
}
}
@@ -984,14 +988,14 @@ raft_record_to_standalone_log(const struct raft_record *r,
if (!r->entry.data) {
return;
}
-if (json_array(r->entry.data)->n != 2) {
+struct json_array *pa = json_array(r->entry.data);
+
+if (pa->n != 2) {
ovs_fatal(0, "Incorrect raft record array length");
}
-
-struct json *data_json = json_array(r->entry.data)->elems[1];
+struct json *data_json = pa->elems[1];
if (data_json->type != JSON_NULL) {
-check_ovsdb_error(ovsdb_log_write_and_free(db_log_data,
- data_json));
+check_ovsdb_error(ovsdb_log_write(db_log_data, data_json));
}
}
}
@@ -1586,6 +1590,7 @@ do_convert_to_standalone(struct ovsdb_log *log, struct
ovsdb_log *db_log_data)
raft_record_to_standalone_log(, db_log_data);
raft_record_uninit();
}
+json_destroy(json);
}
}
--
2.25.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] controller: use LLA IPv6 address as NS source address
Use router LLA IPv6 address as IPv6 source address for Neighbor
Solicitation packets
Fixes: c0bf32d72 ("Manage ARP process locally in a DVR scenario")
Signed-off-by: Lorenzo Bianconi
---
controller/pinctrl.c | 4 +++-
tests/ovn.at | 15 +--
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 8bf19776c..3fa8923e7 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -4563,9 +4563,11 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const struct
flow *ip_flow,
uint64_t packet_stub[128 / 8];
struct dp_packet packet;
+struct in6_addr ipv6_src;
dp_packet_use_stub(, packet_stub, sizeof packet_stub);
-compose_nd_ns(, ip_flow->dl_src, _flow->ipv6_src,
+in6_generate_lla(ip_flow->dl_src, _src);
+compose_nd_ns(, ip_flow->dl_src, _src,
_flow->ipv6_dst);
/* Reload previous packet metadata and set actions from userdata. */
diff --git a/tests/ovn.at b/tests/ovn.at
index 9a44f0a6f..9e907155b 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -11280,13 +11280,13 @@ options:rxq_pcap=${pcap_file}-rx.pcap
# This function sends ipv6 packet
test_ipv6() {
local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5
-local dst_mcast_mac=$6 mcast_node_ip=$7 nd_target=$8
+local dst_mcast_mac=$6 mcast_node_ip=$7 nd_target=$8 nd_src_ip=$9
local packet=${dst_mac}${src_mac}86dd60083aff${src_ip}${dst_ip}
packet=${packet}8000
src_mac=02010204
-expected_packet=${dst_mcast_mac}${src_mac}86dd60203aff${src_ip}
+expected_packet=${dst_mcast_mac}${src_mac}86dd60203aff${nd_src_ip}
expected_packet=${expected_packet}${mcast_node_ip}8700
expected_packet=${expected_packet}${nd_target}0101${src_mac}
@@ -11298,6 +11298,7 @@ test_ipv6() {
src_mac=50640002
dst_mac=af01
src_ip=aef0526400fffe02
+nd_src_ip=fe80020002fffe010204
dst_ip=20010db80001020002fffe010205
dst_mcast_mac=ff010205
mcast_node_ip=ff020001ff010205
@@ -11305,7 +11306,7 @@ nd_target=20010db80001020002fffe010205
# Send an IPv6 packet. Generated IPv6 Neighbor solicitation packet
# should be received by the ports attached to br-phys.
test_ipv6 1 $src_mac $dst_mac $src_ip $dst_ip $dst_mcast_mac \
-$mcast_node_ip $nd_target
+$mcast_node_ip $nd_target $nd_src_ip
OVS_WAIT_WHILE([test 24 = $(wc -c hv1/br-phys_n1-tx.pcap | cut -d " " -f1)])
OVS_WAIT_WHILE([test 24 = $(wc -c hv1/br-phys-tx.pcap | cut -d " " -f1)])
@@ -11338,7 +11339,7 @@ dst_mcast_mac=ff011305
mcast_node_ip=ff020001ff011305
nd_target=20010db80001020002fffe011305
test_ipv6 1 $src_mac $dst_mac $src_ip $dst_ip $dst_mcast_mac \
-$mcast_node_ip $nd_target
+$mcast_node_ip $nd_target $nd_src_ip
OVS_WAIT_WHILE([test 24 = $(wc -c hv1/br-phys_n1-tx.pcap | cut -d " " -f1)])
OVS_WAIT_WHILE([test 24 = $(wc -c hv1/br-phys-tx.pcap | cut -d " " -f1)])
@@ -14262,7 +14263,7 @@ send_na() {
get_nd() {
local eth_src=$1 src_ip=$2 dst_ip=$3 ta=$4
local ip6_hdr=60203aff${src_ip}${dst_ip}
-
request=ff10${eth_src}86dd${ip6_hdr}87003576${ta}0101${eth_src}
+
request=ff10${eth_src}86dd${ip6_hdr}870051f4${ta}0101${eth_src}
echo $request
}
@@ -14325,6 +14326,8 @@ router_mac1=02010203
router_ip=$(ip_to_hex 172 16 1 1)
router_ip6=20020001
+nd_src_ip6=fe80020002fffe010203
+
dst_mac=001122334455
dst_ip=$(ip_to_hex 172 16 1 10)
dst_ip6=20020010
@@ -14342,7 +14345,7 @@ nd_ip=ff020001ff10
ip6_hdr=60083afe${src_ip6}${dst_ip6}
send_icmp6_packet 1 1 $src_mac $router_mac0 $src_ip6 $dst_ip6
-echo $(get_nd $router_mac1 $src_ip6 $nd_ip $dst_ip6) >> expected
+echo $(get_nd $router_mac1 $nd_src_ip6 $nd_ip $dst_ip6) >> expected
echo "${dst_mac}${router_mac1}86dd${ip6_hdr}8000dcb662f1" >> expected
send_na 2 1 $dst_mac $router_mac1 $dst_ip6 $router_ip6
--
2.25.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] vlog: Fix OVS_REQUIRES macro.
On Tue, Mar 24, 2020 at 12:16:38PM -0700, William Tu wrote: > Pass lock objects, not their addresses, to the annotation macros. > > Signed-off-by: William Tu Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] vlog: Fix OVS_REQUIRES macro.
Pass lock objects, not their addresses, to the annotation macros.
Signed-off-by: William Tu
---
lib/vlog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/vlog.c b/lib/vlog.c
index 6d17d4837e9c..ee6b0d3a620a 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -257,7 +257,7 @@ vlog_get_level(const struct vlog_module *module,
}
static void
-update_min_level(struct vlog_module *module) OVS_REQUIRES(_file_mutex)
+update_min_level(struct vlog_module *module) OVS_REQUIRES(log_file_mutex)
{
enum vlog_destination destination;
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 2/2] Documentation: Add extra repo info for RHEL 8
On Tue, Mar 24, 2020 at 09:33:13AM -0700, Yi-Hung Wei wrote: > On Tue, Mar 24, 2020 at 8:42 AM Greg Rose wrote: > > > > The extra development repo for RHEL 8 has changed. Document it. > > > > Signed-off-by: Greg Rose > > > > --- > > V2 - Break long line > > --- > > Documentation/intro/install/fedora.rst | 4 > > 1 file changed, 4 insertions(+) > > LGTM. > > Acked-by: Yi-Hung Wei Applied to master, thanks ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 1/2] compat: Fix nf_ip_hook parameters for RHEL 8
On Tue, Mar 24, 2020 at 09:31:23AM -0700, Yi-Hung Wei wrote: > On Tue, Mar 24, 2020 at 8:42 AM Greg Rose wrote: > > > > From: Greg Rose > > > > A RHEL release version check was only checking for RHEL releases > > greater than 7.0 so that ended up including a compat fixup that > > is not needed for 8.0. Fix up the version check. > > > > Signed-off-by: Greg Rose > > --- > LGTM. > > Acked-by: Yi-Hung Wei Applied to master, thanks ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv2] github-ci: Enable github actions.
This enables per-commit check for 'make check-system-userspace' test using github action[1]. Currently this runs on ubuntu-1804 runner with everything PASS: 93 tests were successful. 43 tests were skipped. Example: https://github.com/williamtu/ovs-travis/runs/531402137?check_suite_focus=true [1] https://github.com/features/actions Signed-off-by: William Tu --- .github/workflows/ovs.yml | 26 ++ Makefile.am | 1 + 2 files changed, 27 insertions(+) create mode 100644 .github/workflows/ovs.yml diff --git a/.github/workflows/ovs.yml b/.github/workflows/ovs.yml new file mode 100644 index ..ccc95641b43c --- /dev/null +++ b/.github/workflows/ovs.yml @@ -0,0 +1,26 @@ +name: OVS system-userspace Test + +on: + push: +branches: [ master ] + pull_request: +branches: [ master ] + +jobs: + build: +runs-on: [ubuntu-18.04] +steps: +- uses: actions/checkout@v2 +- name: build iproute2-5.0.0 + run: sudo apt install -y bison flex; git clone -b v5.0.0 git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git; cd iproute2; ./configure && make -j2 && sudo make install +- name: configure + run: ./boot.sh; ./configure +- name: make + run: make -j2 +- name: check-system-userspace + run: sudo make check-system-userspace RECHECK=yes +- name: Upload artifact + uses: actions/upload-artifact@v1.0.0 + with: +name: system-userspace +path: tests/system-userspace-testsuite.log diff --git a/Makefile.am b/Makefile.am index b279303d186c..80448d0c31c1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -92,6 +92,7 @@ EXTRA_DIST = \ $(MAN_ROOTS) \ Vagrantfile \ Vagrantfile-FreeBSD \ + .github/workflows/ovs.yml \ .mailmap bin_PROGRAMS = sbin_PROGRAMS = -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Innova Learn
Miércoles 22 de Abril | Horario de 10:00 a 14:00 hrs. | (hora del centro de México) - Reparto de utilidades 2020: Guía práctica - Nuestro curso en línea interactivo le brindará herramientas que le ayudarán al cálculo del proyecto anual de Participación de los Trabajadores en las Utilidades (PTU), sus aspectos administrativos, legales y fechas de cumplimiento. Además de incluir recomendaciones para la dinámica de comunicación interna al personal y sindicato. ¿Qué aprenderás?: - Conocerá los lineamientos legales del proyecto de reparto de PTU. - Administrar los tiempos y estrategias del proyecto. - Conocerá la forma correcta del cálculo de los conceptos del proyecto de PTU. - Documental necesario en caso de las inspecciones de la S.T.P.S. para revisión de PTU. Solicita información respondiendo a este correo con la palabra Utilidadesjunto con los siguientes datos: Nombre: Correo electrónico: Número telefónico: Correo Alterno: Dirigido a: Jefes, Gerentes y responsables de RRHH. Responsables y personas en general responsables o con personal a su cargo. Números de Atención: (045) 55 15 54 66 30 - (045) 55 85567293 - (045) 5530167085 En caso de que haya recibido este correo sin haberlo solicitado o si desea dejar de recibir nuestra promoción favor de responder con la palabra baja o enviar un correo a bajas@ innovalearn.net ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
Probably best to post a rebased series. On Fri, Mar 20, 2020 at 11:43:41AM -0700, aginwala wrote: > Hi Ben: > > I checked the history for 2.11 and this patch is failing to backport due to > this commit > https://github.com/openvswitch/ovs/commit/5832e6a4f103a1eddd62c9ba459ce669e3a5d132 > as it updates OVSB fast re-sync in news and its ported 2.12 . Hence, > backport to 2.12 worked. If we get > https://github.com/openvswitch/ovs/commit/5832e6a4f103a1eddd62c9ba459ce669e3a5d132 > in > too to 2.11 it will apply cleanly. Let me know else I will have to amend > the news and it will fail backport for fast re-syc if we move that to 2.11 > in future. > > On Thu, Mar 19, 2020 at 4:22 PM aginwala wrote: > > > Ok sounds good. Will do that. Thanks a ton! > > > > On Thu, Mar 19, 2020 at 3:53 PM Ben Pfaff wrote: > > > >> I backported both to 2.12. > >> > >> They need a manual backport to 2.11, will you take care of it? > >> > >> On Thu, Mar 19, 2020 at 03:39:58PM -0700, aginwala wrote: > >> > Oh I see it seems the previous patch needs to be backported too > >> > > >> https://lists.linuxfoundation.org/pipermail/ovs-dev/2019-September/362925.html > >> > . > >> > Please see if you can get that too on 2.11 and 2.12 > >> > > >> > > >> > > >> > > >> > On Thu, Mar 19, 2020 at 3:35 PM Ben Pfaff wrote: > >> > > >> > > It doesn't apply cleanly. > >> > > > >> > > On Thu, Mar 19, 2020 at 03:07:39PM -0700, aginwala wrote: > >> > > > Hi Ben: > >> > > > > >> > > > Thanks for backporting previous patches. Please see if you can back > >> port > >> > > > this one too to 2.11 and 2.12. > >> > > > > >> > > > On Thu, Mar 19, 2020 at 1:53 PM aginwala wrote: > >> > > > > >> > > > > Hi Ben: > >> > > > > Can you also backport this patch to 2.12 and 2.11 too? > >> > > > > > >> > > > > On Mon, Oct 7, 2019 at 11:22 AM Ben Pfaff wrote: > >> > > > > > >> > > > >> On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc wrote: > >> > > > >> > memory leak is reported by valgrind while executing functional > >> test > >> > > > >> > "ovsdb-tool convert-to-standalone" > >> > > > >> > > >> > > > >> > ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks > >> are > >> > > > >> definitely lost in loss record 20 of 20 > >> > > > >> > ==13842==at 0x4C2DB8F: malloc (in > >> > > > >> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > >> > > > >> > ==13842==by 0x45EE2E: xmalloc (util.c:138) > >> > > > >> > ==13842==by 0x43E386: json_create (json.c:1451) > >> > > > >> > ==13842==by 0x43BDD2: json_object_create (json.c:254) > >> > > > >> > ==13842==by 0x43DEE3: json_parser_push_object (json.c:1273) > >> > > > >> > ==13842==by 0x43E167: json_parser_input (json.c:1371) > >> > > > >> > ==13842==by 0x43D6EA: json_lex_input (json.c:991) > >> > > > >> > ==13842==by 0x43DAC1: json_parser_feed (json.c:1149) > >> > > > >> > ==13842==by 0x40D108: parse_body (log.c:411) > >> > > > >> > ==13842==by 0x40D386: ovsdb_log_read (log.c:476) > >> > > > >> > ==13842==by 0x406A0B: do_convert_to_standalone > >> > > (ovsdb-tool.c:1571) > >> > > > >> > ==13842==by 0x406A0B: do_cluster_standalone > >> (ovsdb-tool.c:1606) > >> > > > >> > ==13842==by 0x438670: ovs_cmdl_run_command__ > >> > > (command-line.c:223) > >> > > > >> > ==13842==by 0x438720: ovs_cmdl_run_command > >> (command-line.c:254) > >> > > > >> > ==13842==by 0x405A4C: main (ovsdb-tool.c:79) > >> > > > >> > > >> > > > >> > The problem was in do_convert_to_standalone() function which > >> while > >> > > > >> reading log file > >> > > > >> > allocate json object which was not deallocated at the end. > >> > > > >> > > >> > > > >> > Signed-off-by: Damijan Skvarc > >> > > > >> > >> > > > >> Applied to master, thanks. > >> > > > >> ___ > >> > > > >> 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 V2 2/2] Documentation: Add extra repo info for RHEL 8
On Tue, Mar 24, 2020 at 8:42 AM Greg Rose wrote: > > The extra development repo for RHEL 8 has changed. Document it. > > Signed-off-by: Greg Rose > > --- > V2 - Break long line > --- > Documentation/intro/install/fedora.rst | 4 > 1 file changed, 4 insertions(+) LGTM. Acked-by: Yi-Hung Wei ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 1/2] compat: Fix nf_ip_hook parameters for RHEL 8
On Tue, Mar 24, 2020 at 8:42 AM Greg Rose wrote: > > From: Greg Rose > > A RHEL release version check was only checking for RHEL releases > greater than 7.0 so that ended up including a compat fixup that > is not needed for 8.0. Fix up the version check. > > Signed-off-by: Greg Rose > --- LGTM. Acked-by: Yi-Hung Wei ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V2 1/2] compat: Fix nf_ip_hook parameters for RHEL 8
From: Greg Rose A RHEL release version check was only checking for RHEL releases greater than 7.0 so that ended up including a compat fixup that is not needed for 8.0. Fix up the version check. Signed-off-by: Greg Rose --- datapath/linux/compat/stt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c index 7b46d1a..8a5853f 100644 --- a/datapath/linux/compat/stt.c +++ b/datapath/linux/compat/stt.c @@ -1559,7 +1559,7 @@ static void clean_percpu(struct work_struct *work) #endif #ifdef HAVE_NF_HOOK_STATE -#if RHEL_RELEASE_CODE > RHEL_RELEASE_VERSION(7,0) +#if RHEL_RELEASE_CODE > RHEL_RELEASE_VERSION(7,0) && RHEL_RELEASE_CODE < RHEL_RELEASE_VERSION(8,0) /* RHEL nfhook hacks. */ #ifndef __GENKSYMS__ #define LAST_PARAM const struct net_device *in, const struct net_device *out, \ -- 1.8.3.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V2 2/2] Documentation: Add extra repo info for RHEL 8
The extra development repo for RHEL 8 has changed. Document it. Signed-off-by: Greg Rose --- V2 - Break long line --- Documentation/intro/install/fedora.rst | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/intro/install/fedora.rst b/Documentation/intro/install/fedora.rst index 6fe1fb5..e5324e1 100644 --- a/Documentation/intro/install/fedora.rst +++ b/Documentation/intro/install/fedora.rst @@ -69,6 +69,10 @@ repositories to help yum-builddep, e.g.:: $ subscription-manager repos --enable=rhel-7-server-extras-rpms $ subscription-manager repos --enable=rhel-7-server-optional-rpms +or for RHEL 8:: +$ subscription-manager repos \ + --enable=codeready-builder-for-rhel-8-x86_64-rpms + DNF:: $ dnf builddep /tmp/ovs.spec -- 1.8.3.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH RFC ovn] Add VXLAN support for non-VTEP datapath bindings
On Tue, Mar 24, 2020 at 8:58 PM Ihar Hrachyshka wrote: > > On Tue, Mar 24, 2020 at 9:18 AM Numan Siddique wrote: > > > > (Sorry, top posting it) > > > > Hi Ihar, > > > > I tested out your patch. All the tests pass. > > > > I then tested out using the ovn-fake-multinode [1]. > > > > After the deployment, I changed the encap to vxlan. East/West traffic > > worked fine > > without any issues.. But I was not able to ping to the gateway router port > > IP > > from outside (172.16.0.100). The moment I changed the encap back to > > geneve, the ping > > works. I didn't look into the issue. So not sure what's the reason. > > You may want to try out > > once with ovn-fake-multinode > > > > Thanks a lot Numan. I used fake-multinode but as you said, E-W works > and I haven't considered gateway case (not an excuse for missing it, > just an observation). I will take a look. I guess this suggests my > patch will have to expand the test suite a bit more than what I have > in my (local) version of the patch where I also test E-W only. No worries. When you submit the formal patch, please also add necessary documentation in ovn-architecture. Thanks Numan > > Thanks again, > Ihar > > ___ > 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 RFC ovn] Add VXLAN support for non-VTEP datapath bindings
On Tue, Mar 24, 2020 at 9:18 AM Numan Siddique wrote: > > (Sorry, top posting it) > > Hi Ihar, > > I tested out your patch. All the tests pass. > > I then tested out using the ovn-fake-multinode [1]. > > After the deployment, I changed the encap to vxlan. East/West traffic > worked fine > without any issues.. But I was not able to ping to the gateway router port IP > from outside (172.16.0.100). The moment I changed the encap back to > geneve, the ping > works. I didn't look into the issue. So not sure what's the reason. > You may want to try out > once with ovn-fake-multinode > Thanks a lot Numan. I used fake-multinode but as you said, E-W works and I haven't considered gateway case (not an excuse for missing it, just an observation). I will take a look. I guess this suggests my patch will have to expand the test suite a bit more than what I have in my (local) version of the patch where I also test E-W only. Thanks again, Ihar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn.at: Fix ARP test that fails due to timing.
On 3/24/20 3:45 PM, Numan Siddique wrote:
> On Tue, Mar 24, 2020 at 7:06 PM Dumitru Ceara wrote:
>>
>> The test for "ARP/ND request broadcast limiting" checks that injected
>> ARP packets are not flooded using the MC_FLOOD multicast group. However,
>> this introduces a race condition in the test because GARPs generated by
>> OVN would also hit the same openflow rules.
>>
>> Remove the checks that use the MC_FLOOD group. They are also redundant
>> as the rest of the test checks that packets are forwarded according to
>> the newly added, higher priority rules.
>>
>> Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever
>> possible.")
>> Signed-off-by: Dumitru Ceara
>
> Thanks Dumitru. I applied this patch to master and branch-20.03
>
> Numan
>
Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv8 1/2] userspace: Enable TSO support for non-DPDK.
On Mon, Mar 23, 2020 at 06:54:29PM +0100, Ilya Maximets wrote: > On 3/20/20 9:52 PM, William Tu wrote: > > On Fri, Mar 20, 2020 at 7:32 AM Ilya Maximets wrote: > >> > >> On 3/20/20 12:56 AM, William Tu wrote: > >>> This patch enables TSO support for non-DPDK use cases, and > >>> also add check-system-tso testsuite. Before TSO, we have to > >>> disable checksum offload, allowing the kernel to calculate the > >>> TCP/UDP packet checsum. With TSO, we can skip the checksum > >>> validation by enabling checksum offload, and with large packet > >>> size, we see better performance. > >>> > >>> Consider container to container use cases: > >>> iperf3 -c (ns0) -> veth peer -> OVS -> veth peer -> iperf3 -s (ns1) > >>> And I got around 6Gbps, similar to TSO with DPDK-enabled. > >>> > >>> Signed-off-by: William Tu > >>> --- > >>> v8: > >>> - make some namings more clear > >>> > >>> v7: more refactoring on functions > >>> - rss and flow mark related functions. > >>> - dp_packet_clone_with_headroom > >>> - fix definitions of DP_PACKET_OL_FLOW_MARK_MASK > >>> - travis: > >>> https://travis-ci.org/github/williamtu/ovs-travis/builds/663658338 > >>> > >>> v6: fix indentation > >>> > >>> v5: feedback from Flavio > >>> - move some code around, break the long line > >>> - travis is now working > >>> https://travis-ci.org/github/williamtu/ovs-travis/builds/661607017 > >>> > >>> v4: > >>> - Avoid duplications of DPDK and non-DPDK code by > >>> refactoring the definition of DP_PACKET_OL flags > >>> and relevant functions > >>> - I got weird error in travis (I think this is unrelated) > >>> https://travis-ci.org/github/williamtu/ovs-travis/builds/661313463 > >>> sindex.c:378:26: error: unknown type name ‘sqlite3_str’ > >>> static int query_appendf(sqlite3_str *query, const char *fmt, ...) > >>> - test compile ok on dpdk and non-dpdk, make check-system-tso still > >>> has a couple fails > >>> > >>> v3: > >>> - fix comments and some coding style > >>> - add valgrind check > >>> - travis: https://travis-ci.org/williamtu/ovs-travis/builds/660394007 > >>> v2: > >>> - add make check-system-tso test > >>> - combine logging for dpdk and non-dpdk > >>> - I'm surprised that most of the test cases passed. > >>> This is due to few tests using tcp/udp, so it does not trigger > >>> TSO. I saw only geneve/vxlan fails randomly, maybe we can > >>> check it later. > >>> --- > >>> lib/dp-packet.c | 6 +- > >>> lib/dp-packet.h | 566 > >>> +++--- > >>> lib/userspace-tso.c | 5 - > >>> tests/.gitignore | 3 + > >>> tests/automake.mk | 21 ++ > >>> tests/system-tso-macros.at| 31 +++ > >>> tests/system-tso-testsuite.at | 26 ++ > >>> 7 files changed, 333 insertions(+), 325 deletions(-) > >>> create mode 100644 tests/system-tso-macros.at > >>> create mode 100644 tests/system-tso-testsuite.at > >>> > >>> diff --git a/lib/dp-packet.c b/lib/dp-packet.c > >>> index cd2623500e3d..e154ac13e4f2 100644 > >>> --- a/lib/dp-packet.c > >>> +++ b/lib/dp-packet.c > >>> @@ -192,10 +192,8 @@ dp_packet_clone_with_headroom(const struct dp_packet > >>> *buffer, size_t headroom) > >>> sizeof(struct dp_packet) - > >>> offsetof(struct dp_packet, l2_pad_size)); > >>> > >>> -#ifdef DPDK_NETDEV > >>> -new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags; > >>> -new_buffer->mbuf.ol_flags &= ~DPDK_MBUF_NON_OFFLOADING_FLAGS; > >>> -#endif > >>> +*dp_packet_ol_flags_ptr(new_buffer) = > >>> *dp_packet_ol_flags_ptr(buffer); > >>> +*dp_packet_ol_flags_ptr(new_buffer) &= ~DP_PACKET_OL_NONE_MASK; > >> > >> > >> I didn't finish reading the patch yet, but the name DP_PACKET_OL_NONE_MASK > >> sounds > >> confusing to me. What I mean: > >> > >> OL_NONE_MASK sounds like 'offloading none' --> 'no any ofloading' > >> wile > >> NON_OFFLOADING_FLAGS --> flags that are present, but not related to > >> offloading. > >> > >> The negative name clearly states that there are some flags that we don't > >> want > >> to clear. OL_NONE says that we're clearing all the offloading flags > >> without > >> paying any attention to the fact that there are things that we don't want > >> to clear. > >> > >> It's not clear from the name that we should do ~ and & to this mask and > >> not just > >> assign it. > >> > >> I don't think that my explanation makes any sense, though. :) > >> > >> Anyway, since you've started to change not really related things, it might > >> make sense to create a positive mask instead of negative one, i.e. > >> DP_PACKET_OL_SUPPORTED/KNOWN/USED_MASK and clear only these flags on > >> offloading reset. > >> This is possible right now since we removed support for dpdk ring (dpdkr) > >> ports > >> and we don't need to clear flags that we do not use. > >> We need to do this anyway in this or in the follow up patch. > >> Relevant discussion:
Re: [ovs-dev] [PATCH v3] conntrack: Reset ct_state when entering a new zone.
On 3/24/20 3:55 PM, Ilya Maximets wrote:
> On 3/20/20 9:44 PM, Aaron Conole wrote:
>> Dumitru Ceara writes:
>>
>>> When a new conntrack zone is entered, the ct_state field is zeroed in
>>> order to avoid using state information from different zones.
>>>
>>> One such scenario is when a packet is double NATed. Assuming two zones
>>> and 3 flows performing the following actions in order on the packet:
>>> 1. ct(zone=5,nat), recirc
>>> 2. ct(zone=1), recirc
>>> 3. ct(zone=1,nat)
>>>
>>> If at step #1 the packet matches an existing NAT entry, it will get
>>> translated and pkt->md.ct_state is set to CS_DST_NAT or CS_SRC_NAT.
>>> At step #2 the new tuple might match an existing connection and
>>> pkt->md.ct_zone is set to 1.
>>> If at step #3 the packet matches an existing NAT entry in zone 1,
>>> handle_nat() will be called to perform the translation but it will
>>> return early because the packet's zone matches the conntrack zone and
>>> the ct_state field still contains CS_DST_NAT or CS_SRC_NAT from the
>>> translations in zone 5.
>>>
>>> In order to reliably detect when a packet enters a new conntrack zone
>>> we also need to make sure that the pkt->md.ct_zone is properly
>>> initialized if pkt->md.ct_state is non-zero. This already happens for
>>> most cases. The only exception is when matched conntrack connection is
>>> of type CT_CONN_TYPE_UN_NAT and the master connection is missing. To
>>> cover this path we now call write_ct_md() in that case too. Remove
>>> setting the CS_TRACKED flag as in this case as it will be done by the
>>> new call to write_ct_md().
>>>
>>> CC: Darrell Ball
>>> Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
>>> Acked-by: Ilya Maximets
>>> Signed-off-by: Dumitru Ceara
>>>
>>> ---
>>> V3:
>>> - Add Ilya's ack and fix "Fixes" tag.
>>> - Remove NULL pointer dereference fix as there's already a patch for it:
>>> https://patchwork.ozlabs.org/patch/1257010/
>>> V2:
>>> - Address Ilya's comments:
>>> - revert changes to pkt_metadata_init().
>>> - update ct_state in process_one() only if ct_state is already
>>> non-zero.
>>> - Make sure pkt->md.ct_zone is always initialized when pkt->md.ct_state
>>> is non-zero.
>>> - Fix NULL pointer dereference in process_one() if conn_type is
>>> CT_CONN_TYPE_UN_NAT and master conn is not found.
>>> ---
>>
>> Acked-by: Aaron Conole
>>
>
>
> Thanks, Dumitru and Aaron!
> Applied to master and backported down to 2.8.
>
> Best regards, Ilya Maximets.
>
Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] conntrack: Reset ct_state when entering a new zone.
On 3/20/20 9:44 PM, Aaron Conole wrote:
> Dumitru Ceara writes:
>
>> When a new conntrack zone is entered, the ct_state field is zeroed in
>> order to avoid using state information from different zones.
>>
>> One such scenario is when a packet is double NATed. Assuming two zones
>> and 3 flows performing the following actions in order on the packet:
>> 1. ct(zone=5,nat), recirc
>> 2. ct(zone=1), recirc
>> 3. ct(zone=1,nat)
>>
>> If at step #1 the packet matches an existing NAT entry, it will get
>> translated and pkt->md.ct_state is set to CS_DST_NAT or CS_SRC_NAT.
>> At step #2 the new tuple might match an existing connection and
>> pkt->md.ct_zone is set to 1.
>> If at step #3 the packet matches an existing NAT entry in zone 1,
>> handle_nat() will be called to perform the translation but it will
>> return early because the packet's zone matches the conntrack zone and
>> the ct_state field still contains CS_DST_NAT or CS_SRC_NAT from the
>> translations in zone 5.
>>
>> In order to reliably detect when a packet enters a new conntrack zone
>> we also need to make sure that the pkt->md.ct_zone is properly
>> initialized if pkt->md.ct_state is non-zero. This already happens for
>> most cases. The only exception is when matched conntrack connection is
>> of type CT_CONN_TYPE_UN_NAT and the master connection is missing. To
>> cover this path we now call write_ct_md() in that case too. Remove
>> setting the CS_TRACKED flag as in this case as it will be done by the
>> new call to write_ct_md().
>>
>> CC: Darrell Ball
>> Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
>> Acked-by: Ilya Maximets
>> Signed-off-by: Dumitru Ceara
>>
>> ---
>> V3:
>> - Add Ilya's ack and fix "Fixes" tag.
>> - Remove NULL pointer dereference fix as there's already a patch for it:
>> https://patchwork.ozlabs.org/patch/1257010/
>> V2:
>> - Address Ilya's comments:
>> - revert changes to pkt_metadata_init().
>> - update ct_state in process_one() only if ct_state is already
>> non-zero.
>> - Make sure pkt->md.ct_zone is always initialized when pkt->md.ct_state
>> is non-zero.
>> - Fix NULL pointer dereference in process_one() if conn_type is
>> CT_CONN_TYPE_UN_NAT and master conn is not found.
>> ---
>
> Acked-by: Aaron Conole
>
Thanks, Dumitru and Aaron!
Applied to master and backported down to 2.8.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv2] fatal-signal: Fix clang error due to lock.
On Tue, Mar 24, 2020 at 03:38:31PM +0100, Ilya Maximets wrote:
> On 3/24/20 3:27 PM, Ilya Maximets wrote:
> > On 3/24/20 3:17 PM, William Tu wrote:
> >> Due to not acquiring lock, clang reports:
> >> lib/vlog.c:618:12: error: reading variable 'log_fd' requires holding
> >> mutex
> >> 'log_file_mutex' [-Werror,-Wthread-safety-analysis]
> >> return log_fd;
> >>
> >> The patch fixes it by creating a function in vlog.c to write
> >> directly to log file unsafely.
> >>
> >> Tested-at:
> >> https://travis-ci.org/github/williamtu/ovs-travis/builds/666165883
> >> Fixes: ecd4a8fcdff2 ("fatal-signal: Log backtrace when no monitor daemon.")
> >> Suggested-by: Ilya Maximets
> >> Signed-off-by: William Tu
> >> ---
>
> I didn't test, but the code looks good to me.
>
> Acked-by: Ilya Maximets
Thanks, applied to master.
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn.at: Fix ARP test that fails due to timing.
On Tue, Mar 24, 2020 at 7:06 PM Dumitru Ceara wrote:
>
> The test for "ARP/ND request broadcast limiting" checks that injected
> ARP packets are not flooded using the MC_FLOOD multicast group. However,
> this introduces a race condition in the test because GARPs generated by
> OVN would also hit the same openflow rules.
>
> Remove the checks that use the MC_FLOOD group. They are also redundant
> as the rest of the test checks that packets are forwarded according to
> the newly added, higher priority rules.
>
> Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever
> possible.")
> Signed-off-by: Dumitru Ceara
Thanks Dumitru. I applied this patch to master and branch-20.03
Numan
> ---
> tests/ovn.at | 33 -
> 1 file changed, 33 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 4baf2e9..9a44f0a 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -18015,9 +18015,6 @@ r1_dp_key=$(ovn-sbctl --bare --columns tunnel_key
> list datapath_binding rtr1)
> r1_tnl_key=$(ovn-sbctl --bare --columns tunnel_key list port_binding sw-rtr1)
> r2_tnl_key=$(ovn-sbctl --bare --columns tunnel_key list port_binding sw-rtr2)
>
> -mc_key=$(ovn-sbctl --bare --columns tunnel_key find multicast_group
> datapath=${sw_dp_uuid} name="_MC_flood")
> -mc_key=$(printf "%04x" $mc_key)
> -
> match_sw_metadata="metadata=0x${sw_dp_key}"
> match_r1_metadata="metadata=0x${r1_dp_key}"
>
> @@ -18042,11 +18039,6 @@ OVS_WAIT_UNTIL([
> grep n_packets=1 -c)
> test "0" = "${pkts_to_rtr2}"
> ])
> -OVS_WAIT_UNTIL([
> -pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -test "0" = "${pkts_flooded}"
> -])
>
> # Inject ND_NS for ofirst router owned IP address.
> src_ipv6=00100254
> @@ -18069,11 +18061,6 @@ OVS_WAIT_UNTIL([
> grep n_packets=1 -c)
> test "0" = "${pkts_to_rtr2}"
> ])
> -OVS_WAIT_UNTIL([
> -pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -test "0" = "${pkts_flooded}"
> -])
>
> # Configure load balancing on both routers.
> ovn-nbctl lb-add lb1-v4 10.0.0.11 42.42.42.1
> @@ -18108,11 +18095,6 @@ OVS_WAIT_UNTIL([
> grep n_packets=1 -c)
> test "0" = "${pkts_to_rtr2}"
> ])
> -OVS_WAIT_UNTIL([
> -pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -test "0" = "${pkts_flooded}"
> -])
>
> # Inject ND_NS for first router owned VIP address.
> src_ipv6=00100254
> @@ -18135,11 +18117,6 @@ OVS_WAIT_UNTIL([
> grep n_packets=1 -c)
> test "0" = "${pkts_to_rtr2}"
> ])
> -OVS_WAIT_UNTIL([
> -pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -test "0" = "${pkts_flooded}"
> -])
>
> # Configure NAT on both routers.
> ovn-nbctl lr-nat-add rtr1 dnat_and_snat 10.0.0.111 42.42.42.1
> @@ -18175,11 +18152,6 @@ OVS_WAIT_UNTIL([
> grep n_packets=1 -c)
> test "0" = "${pkts_to_rtr2}"
> ])
> -OVS_WAIT_UNTIL([
> -pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -test "0" = "${pkts_flooded}"
> -])
>
> # Inject ARP request for FIP1.
> send_arp_request 1 0 ${src_mac} $(ip_to_hex 10 0 0 254) $(ip_to_hex 10 0 0
> 121)
> @@ -18242,11 +18214,6 @@ OVS_WAIT_UNTIL([
> grep n_packets=1 -c)
> test "0" = "${pkts_to_rtr2}"
> ])
> -OVS_WAIT_UNTIL([
> -pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -test "0" = "${pkts_flooded}"
> -])
>
> # Inject ND_NS for FIP1.
> src_ipv6=00100254
> --
> 1.8.3.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] [PATCHv2] fatal-signal: Fix clang error due to lock.
On 3/24/20 3:27 PM, Ilya Maximets wrote:
> On 3/24/20 3:17 PM, William Tu wrote:
>> Due to not acquiring lock, clang reports:
>> lib/vlog.c:618:12: error: reading variable 'log_fd' requires holding mutex
>> 'log_file_mutex' [-Werror,-Wthread-safety-analysis]
>> return log_fd;
>>
>> The patch fixes it by creating a function in vlog.c to write
>> directly to log file unsafely.
>>
>> Tested-at: https://travis-ci.org/github/williamtu/ovs-travis/builds/666165883
>> Fixes: ecd4a8fcdff2 ("fatal-signal: Log backtrace when no monitor daemon.")
>> Suggested-by: Ilya Maximets
>> Signed-off-by: William Tu
>> ---
I didn't test, but the code looks good to me.
Acked-by: Ilya Maximets
>> include/openvswitch/vlog.h | 4 ++--
>> lib/fatal-signal.c | 8 ++--
>> lib/vlog.c | 15 ---
>> 3 files changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
>> index 476bf3d6d5b4..886fce5e0fd5 100644
>> --- a/include/openvswitch/vlog.h
>> +++ b/include/openvswitch/vlog.h
>> @@ -143,8 +143,8 @@ void vlog_set_syslog_method(const char *method);
>> /* Configure syslog target. */
>> void vlog_set_syslog_target(const char *target);
>>
>> -/* Return the log_fd. */
>> -int vlog_get_fd(void);
>> +/* Write directly to log file. */
>> +void vlog_direct_write_to_log_file_unsafe(const char *s);
>>
>> /* Initialization. */
>> void vlog_init(void);
>> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
>> index 4965c1ae82c0..51cf628d994e 100644
>> --- a/lib/fatal-signal.c
>> +++ b/lib/fatal-signal.c
>> @@ -197,11 +197,7 @@ send_backtrace_to_monitor(void) {
>> */
>> char str[] = "SIGSEGV detected, backtrace:\n";
>>
>> -if (vlog_get_fd() < 0) {
>> -return;
>> -}
>> -
>> -ignore(write(vlog_get_fd(), str, strlen(str)));
>> +vlog_direct_write_to_log_file_unsafe(str);
>>
>> for (int i = 0; i < dep; i++) {
>> char line[64];
>> @@ -210,7 +206,7 @@ send_backtrace_to_monitor(void) {
>> unw_bt[i].ip,
>> unw_bt[i].func,
>> unw_bt[i].offset);
>> -ignore(write(vlog_get_fd(), line, strlen(line)));
>> +vlog_direct_write_to_log_file_unsafe(line);
>> }
>> }
>> }
>> diff --git a/lib/vlog.c b/lib/vlog.c
>> index 502b33fc831e..6d17d4837e9c 100644
>> --- a/lib/vlog.c
>> +++ b/lib/vlog.c
>> @@ -612,10 +612,19 @@ vlog_set_syslog_target(const char *target)
>> ovs_rwlock_unlock(_rwlock);
>> }
>>
>> -int
>> -vlog_get_fd(void)
>> +/*
>> + * This function writes directly to log file without using async writer or
>> + * taking a lock. Caller must hold 'log_file_mutex' or be sure that it's
>> + * not necessary. Could be used in exceptional cases like dumping of
>> backtrace
>> + * on fatal signals.
>> + */
>> +void
>> +vlog_direct_write_to_log_file_unsafe(const char *s)
>> +OVS_NO_THREAD_SAFETY_ANALYSIS
>> {
>> -return log_fd;
>> +if (log_fd >= 0) {
>> +ignore(write(log_fd, s, strlen(s)));
>> +}
>> }
>>
>> /* Returns 'false' if 'facility' is not a valid string. If 'facility'
>>
>
> So, does this work?
> I mean, last time you said that this exact version of code doesn't print
> anything.
Never mind. I saw the reply in a previous thread.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] lockfile: Fix OVS_REQUIRES macro.
On Mon, Mar 23, 2020 at 04:48:42PM -0700, Ben Pfaff wrote:
> On Mon, Mar 23, 2020 at 04:34:37PM -0700, William Tu wrote:
> > Pass lock objects, not their addresses, to the annotation macros.
> >
> > Fixes: f21fa45f3085 ("lockfile: Minor code cleanup.")
> > Tested-at:
> > https://travis-ci.org/github/williamtu/ovs-travis/builds/666098338
> > Signed-off-by: William Tu
>
> Assuming it compiles, this is good.
>
> Acked-by: Ben Pfaff
Applied to master, thanks.
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] fatal-signal: Skip acquiring log fd lock.
On 3/24/20 3:30 PM, William Tu wrote:
> On Mon, Mar 23, 2020 at 9:26 PM William Tu wrote:
>>
>> On Mon, Mar 23, 2020 at 4:13 PM Ilya Maximets wrote:
>>>
>>> On 3/23/20 11:34 PM, William Tu wrote:
On Mon, Mar 23, 2020 at 2:58 PM William Tu wrote:
>
> On Mon, Mar 23, 2020 at 2:34 PM Ben Pfaff wrote:
>>
>> On Mon, Mar 23, 2020 at 02:22:17PM -0700, William Tu wrote:
>>> Due to not acquiring lock, clang reports:
>>> lib/vlog.c:618:12: error: reading variable 'log_fd' requires holding
>>> mutex
>>> 'log_file_mutex' [-Werror,-Wthread-safety-analysis]
>>> return log_fd;
>>> The patch fixes by skipping it using OVS_NO_THREAD_SAFETY_ANALYSIS.
>>>
>>> Tested-at:
>>> https://travis-ci.org/github/williamtu/ovs-travis/builds/666051210
>>> Fixex: ecd4a8fcdff2 ("fatal-signal: Log backtrace when no monitor
>>> daemon.")
>>> Signed-off-by: William Tu
>>
>> You can't just mark the existing send_backtrace_to_monitor()
>> OVS_NO_THREAD_SAFETY_ANALYSIS? It seems like the easiest change.
> OK
> that will also work.
>>
>> Also, the OVS_REQUIRES should also go in vlog.h.
Adding 'OVS_REQUIRES(log_file_mutext)' to vlog.h has error like this:
./include/openvswitch/vlog.h:147:36: error: use of undeclared
identifier 'log_file_mutex'
int vlog_get_fd(void) OVS_REQUIRES(log_file_mutex);
I will have to move log_file_mutex and pattern_rwlock to vlog.h.
Do we want this?
Does it make difference if OVS_REQUIRES is only used in C file
but not in header file?
>>>
>>> To avoid exposure of the vlog internals, maybe we could create a new
>>> function like:
>>>
>>> /*
>>> * This function writes directly to log file without using async writer or
>>> * taking a lock. Caller must hold 'log_file_mutex' or be sure that it's
>>> * not necessary. Could be used in exceptional cases like dumping of
>>> backtrace
>>> * on fatal signals.
>>> */
>>> void
>>> vlog_direct_write_to_log_file_unsafe(const char *s)
>>> OVS_NO_THREAD_SAFETY_ANALYSIS
>>> {
>>> if (log_fd >= 0) {
>>> ignore(write(log_fd, s, strlen(s)));
>>> }
>>> }
>>>
>>
>> Hi Ilya,
>>
>> Do you know any restrictions when calling functions in signal handler?
>> When use this approach, the backtrace does not print anything.
>> I'm still debugging it with the below patch:
>>
>> diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
>> index 476bf3d6d5b4..886fce5e0fd5 100644
>> --- a/include/openvswitch/vlog.h
>> +++ b/include/openvswitch/vlog.h
>> @@ -143,8 +143,8 @@ void vlog_set_syslog_method(const char *method);
>> /* Configure syslog target. */
>> void vlog_set_syslog_target(const char *target);
>>
>> -/* Return the log_fd. */
>> -int vlog_get_fd(void);
>> +/* Write directly to log file. */
>> +void vlog_direct_write_to_log_file_unsafe(const char *s);
>>
>> /* Initialization. */
>> void vlog_init(void);
>> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
>> index 4965c1ae82c0..51cf628d994e 100644
>> --- a/lib/fatal-signal.c
>> +++ b/lib/fatal-signal.c
>> @@ -197,11 +197,7 @@ send_backtrace_to_monitor(void) {
>> */
>> char str[] = "SIGSEGV detected, backtrace:\n";
>>
>> -if (vlog_get_fd() < 0) {
>> -return;
>> -}
>> -
>> -ignore(write(vlog_get_fd(), str, strlen(str)));
>> +vlog_direct_write_to_log_file_unsafe(str);
>>
>> for (int i = 0; i < dep; i++) {
>> char line[64];
>> @@ -210,7 +206,7 @@ send_backtrace_to_monitor(void) {
>> unw_bt[i].ip,
>> unw_bt[i].func,
>> unw_bt[i].offset);
>> -ignore(write(vlog_get_fd(), line, strlen(line)));
>> +vlog_direct_write_to_log_file_unsafe(line);
>> }
>> }
>> }
>> diff --git a/lib/vlog.c b/lib/vlog.c
>> index 502b33fc831e..6d17d4837e9c 100644
>> --- a/lib/vlog.c
>> +++ b/lib/vlog.c
>> @@ -612,10 +612,19 @@ vlog_set_syslog_target(const char *target)
>> ovs_rwlock_unlock(_rwlock);
>> }
>>
>> -int
>> -vlog_get_fd(void)
>> +/*
>> + * This function writes directly to log file without using async writer or
>> + * taking a lock. Caller must hold 'log_file_mutex' or be sure that it's
>> + * not necessary. Could be used in exceptional cases like dumping of
>> backtrace
>> + * on fatal signals.
>> + */
>> +void
>> +vlog_direct_write_to_log_file_unsafe(const char *s)
>> +OVS_NO_THREAD_SAFETY_ANALYSIS
>> {
>> -return log_fd;
>> +if (log_fd >= 0) {
>> +ignore(write(log_fd, s, strlen(s)));
>> +}
>> }
>>
>> /* Returns 'false' if 'facility' is not a valid string. If 'facility'
>
> I have the issue fixed and sent v2 patch
> https://mail.openvswitch.org/pipermail/ovs-dev/2020-March/369070.html
>
> The issue was that I compile the code with address sanitizer below:
> ./configure CC=clang --enable-Werror CFLAGS="-g -O2 -fsanitize=address"
> Then no
Re: [ovs-dev] [PATCH] fatal-signal: Skip acquiring log fd lock.
On Mon, Mar 23, 2020 at 9:26 PM William Tu wrote:
>
> On Mon, Mar 23, 2020 at 4:13 PM Ilya Maximets wrote:
> >
> > On 3/23/20 11:34 PM, William Tu wrote:
> > > On Mon, Mar 23, 2020 at 2:58 PM William Tu wrote:
> > >>
> > >> On Mon, Mar 23, 2020 at 2:34 PM Ben Pfaff wrote:
> > >>>
> > >>> On Mon, Mar 23, 2020 at 02:22:17PM -0700, William Tu wrote:
> > Due to not acquiring lock, clang reports:
> > lib/vlog.c:618:12: error: reading variable 'log_fd' requires holding
> > mutex
> > 'log_file_mutex' [-Werror,-Wthread-safety-analysis]
> > return log_fd;
> > The patch fixes by skipping it using OVS_NO_THREAD_SAFETY_ANALYSIS.
> >
> > Tested-at:
> > https://travis-ci.org/github/williamtu/ovs-travis/builds/666051210
> > Fixex: ecd4a8fcdff2 ("fatal-signal: Log backtrace when no monitor
> > daemon.")
> > Signed-off-by: William Tu
> > >>>
> > >>> You can't just mark the existing send_backtrace_to_monitor()
> > >>> OVS_NO_THREAD_SAFETY_ANALYSIS? It seems like the easiest change.
> > >> OK
> > >> that will also work.
> > >>>
> > >>> Also, the OVS_REQUIRES should also go in vlog.h.
> > >
> > > Adding 'OVS_REQUIRES(log_file_mutext)' to vlog.h has error like this:
> > > ./include/openvswitch/vlog.h:147:36: error: use of undeclared
> > > identifier 'log_file_mutex'
> > > int vlog_get_fd(void) OVS_REQUIRES(log_file_mutex);
> > >
> > > I will have to move log_file_mutex and pattern_rwlock to vlog.h.
> > > Do we want this?
> > > Does it make difference if OVS_REQUIRES is only used in C file
> > > but not in header file?
> >
> > To avoid exposure of the vlog internals, maybe we could create a new
> > function like:
> >
> > /*
> > * This function writes directly to log file without using async writer or
> > * taking a lock. Caller must hold 'log_file_mutex' or be sure that it's
> > * not necessary. Could be used in exceptional cases like dumping of
> > backtrace
> > * on fatal signals.
> > */
> > void
> > vlog_direct_write_to_log_file_unsafe(const char *s)
> > OVS_NO_THREAD_SAFETY_ANALYSIS
> > {
> > if (log_fd >= 0) {
> > ignore(write(log_fd, s, strlen(s)));
> > }
> > }
> >
>
> Hi Ilya,
>
> Do you know any restrictions when calling functions in signal handler?
> When use this approach, the backtrace does not print anything.
> I'm still debugging it with the below patch:
>
> diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
> index 476bf3d6d5b4..886fce5e0fd5 100644
> --- a/include/openvswitch/vlog.h
> +++ b/include/openvswitch/vlog.h
> @@ -143,8 +143,8 @@ void vlog_set_syslog_method(const char *method);
> /* Configure syslog target. */
> void vlog_set_syslog_target(const char *target);
>
> -/* Return the log_fd. */
> -int vlog_get_fd(void);
> +/* Write directly to log file. */
> +void vlog_direct_write_to_log_file_unsafe(const char *s);
>
> /* Initialization. */
> void vlog_init(void);
> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
> index 4965c1ae82c0..51cf628d994e 100644
> --- a/lib/fatal-signal.c
> +++ b/lib/fatal-signal.c
> @@ -197,11 +197,7 @@ send_backtrace_to_monitor(void) {
> */
> char str[] = "SIGSEGV detected, backtrace:\n";
>
> -if (vlog_get_fd() < 0) {
> -return;
> -}
> -
> -ignore(write(vlog_get_fd(), str, strlen(str)));
> +vlog_direct_write_to_log_file_unsafe(str);
>
> for (int i = 0; i < dep; i++) {
> char line[64];
> @@ -210,7 +206,7 @@ send_backtrace_to_monitor(void) {
> unw_bt[i].ip,
> unw_bt[i].func,
> unw_bt[i].offset);
> -ignore(write(vlog_get_fd(), line, strlen(line)));
> +vlog_direct_write_to_log_file_unsafe(line);
> }
> }
> }
> diff --git a/lib/vlog.c b/lib/vlog.c
> index 502b33fc831e..6d17d4837e9c 100644
> --- a/lib/vlog.c
> +++ b/lib/vlog.c
> @@ -612,10 +612,19 @@ vlog_set_syslog_target(const char *target)
> ovs_rwlock_unlock(_rwlock);
> }
>
> -int
> -vlog_get_fd(void)
> +/*
> + * This function writes directly to log file without using async writer or
> + * taking a lock. Caller must hold 'log_file_mutex' or be sure that it's
> + * not necessary. Could be used in exceptional cases like dumping of
> backtrace
> + * on fatal signals.
> + */
> +void
> +vlog_direct_write_to_log_file_unsafe(const char *s)
> +OVS_NO_THREAD_SAFETY_ANALYSIS
> {
> -return log_fd;
> +if (log_fd >= 0) {
> +ignore(write(log_fd, s, strlen(s)));
> +}
> }
>
> /* Returns 'false' if 'facility' is not a valid string. If 'facility'
I have the issue fixed and sent v2 patch
https://mail.openvswitch.org/pipermail/ovs-dev/2020-March/369070.html
The issue was that I compile the code with address sanitizer below:
./configure CC=clang --enable-Werror CFLAGS="-g -O2 -fsanitize=address"
Then no backtrace is shown.
Remove '-fsanitize=address' or use GCC works OK.
Thanks
William
Re: [ovs-dev] [PATCHv2] fatal-signal: Fix clang error due to lock.
On 3/24/20 3:17 PM, William Tu wrote:
> Due to not acquiring lock, clang reports:
> lib/vlog.c:618:12: error: reading variable 'log_fd' requires holding mutex
> 'log_file_mutex' [-Werror,-Wthread-safety-analysis]
> return log_fd;
>
> The patch fixes it by creating a function in vlog.c to write
> directly to log file unsafely.
>
> Tested-at: https://travis-ci.org/github/williamtu/ovs-travis/builds/666165883
> Fixes: ecd4a8fcdff2 ("fatal-signal: Log backtrace when no monitor daemon.")
> Suggested-by: Ilya Maximets
> Signed-off-by: William Tu
> ---
> include/openvswitch/vlog.h | 4 ++--
> lib/fatal-signal.c | 8 ++--
> lib/vlog.c | 15 ---
> 3 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
> index 476bf3d6d5b4..886fce5e0fd5 100644
> --- a/include/openvswitch/vlog.h
> +++ b/include/openvswitch/vlog.h
> @@ -143,8 +143,8 @@ void vlog_set_syslog_method(const char *method);
> /* Configure syslog target. */
> void vlog_set_syslog_target(const char *target);
>
> -/* Return the log_fd. */
> -int vlog_get_fd(void);
> +/* Write directly to log file. */
> +void vlog_direct_write_to_log_file_unsafe(const char *s);
>
> /* Initialization. */
> void vlog_init(void);
> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
> index 4965c1ae82c0..51cf628d994e 100644
> --- a/lib/fatal-signal.c
> +++ b/lib/fatal-signal.c
> @@ -197,11 +197,7 @@ send_backtrace_to_monitor(void) {
> */
> char str[] = "SIGSEGV detected, backtrace:\n";
>
> -if (vlog_get_fd() < 0) {
> -return;
> -}
> -
> -ignore(write(vlog_get_fd(), str, strlen(str)));
> +vlog_direct_write_to_log_file_unsafe(str);
>
> for (int i = 0; i < dep; i++) {
> char line[64];
> @@ -210,7 +206,7 @@ send_backtrace_to_monitor(void) {
> unw_bt[i].ip,
> unw_bt[i].func,
> unw_bt[i].offset);
> -ignore(write(vlog_get_fd(), line, strlen(line)));
> +vlog_direct_write_to_log_file_unsafe(line);
> }
> }
> }
> diff --git a/lib/vlog.c b/lib/vlog.c
> index 502b33fc831e..6d17d4837e9c 100644
> --- a/lib/vlog.c
> +++ b/lib/vlog.c
> @@ -612,10 +612,19 @@ vlog_set_syslog_target(const char *target)
> ovs_rwlock_unlock(_rwlock);
> }
>
> -int
> -vlog_get_fd(void)
> +/*
> + * This function writes directly to log file without using async writer or
> + * taking a lock. Caller must hold 'log_file_mutex' or be sure that it's
> + * not necessary. Could be used in exceptional cases like dumping of
> backtrace
> + * on fatal signals.
> + */
> +void
> +vlog_direct_write_to_log_file_unsafe(const char *s)
> +OVS_NO_THREAD_SAFETY_ANALYSIS
> {
> -return log_fd;
> +if (log_fd >= 0) {
> +ignore(write(log_fd, s, strlen(s)));
> +}
> }
>
> /* Returns 'false' if 'facility' is not a valid string. If 'facility'
>
So, does this work?
I mean, last time you said that this exact version of code doesn't print
anything.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv2] fatal-signal: Fix clang error due to lock.
Due to not acquiring lock, clang reports:
lib/vlog.c:618:12: error: reading variable 'log_fd' requires holding mutex
'log_file_mutex' [-Werror,-Wthread-safety-analysis]
return log_fd;
The patch fixes it by creating a function in vlog.c to write
directly to log file unsafely.
Tested-at: https://travis-ci.org/github/williamtu/ovs-travis/builds/666165883
Fixes: ecd4a8fcdff2 ("fatal-signal: Log backtrace when no monitor daemon.")
Suggested-by: Ilya Maximets
Signed-off-by: William Tu
---
include/openvswitch/vlog.h | 4 ++--
lib/fatal-signal.c | 8 ++--
lib/vlog.c | 15 ---
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
index 476bf3d6d5b4..886fce5e0fd5 100644
--- a/include/openvswitch/vlog.h
+++ b/include/openvswitch/vlog.h
@@ -143,8 +143,8 @@ void vlog_set_syslog_method(const char *method);
/* Configure syslog target. */
void vlog_set_syslog_target(const char *target);
-/* Return the log_fd. */
-int vlog_get_fd(void);
+/* Write directly to log file. */
+void vlog_direct_write_to_log_file_unsafe(const char *s);
/* Initialization. */
void vlog_init(void);
diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index 4965c1ae82c0..51cf628d994e 100644
--- a/lib/fatal-signal.c
+++ b/lib/fatal-signal.c
@@ -197,11 +197,7 @@ send_backtrace_to_monitor(void) {
*/
char str[] = "SIGSEGV detected, backtrace:\n";
-if (vlog_get_fd() < 0) {
-return;
-}
-
-ignore(write(vlog_get_fd(), str, strlen(str)));
+vlog_direct_write_to_log_file_unsafe(str);
for (int i = 0; i < dep; i++) {
char line[64];
@@ -210,7 +206,7 @@ send_backtrace_to_monitor(void) {
unw_bt[i].ip,
unw_bt[i].func,
unw_bt[i].offset);
-ignore(write(vlog_get_fd(), line, strlen(line)));
+vlog_direct_write_to_log_file_unsafe(line);
}
}
}
diff --git a/lib/vlog.c b/lib/vlog.c
index 502b33fc831e..6d17d4837e9c 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -612,10 +612,19 @@ vlog_set_syslog_target(const char *target)
ovs_rwlock_unlock(_rwlock);
}
-int
-vlog_get_fd(void)
+/*
+ * This function writes directly to log file without using async writer or
+ * taking a lock. Caller must hold 'log_file_mutex' or be sure that it's
+ * not necessary. Could be used in exceptional cases like dumping of backtrace
+ * on fatal signals.
+ */
+void
+vlog_direct_write_to_log_file_unsafe(const char *s)
+OVS_NO_THREAD_SAFETY_ANALYSIS
{
-return log_fd;
+if (log_fd >= 0) {
+ignore(write(log_fd, s, strlen(s)));
+}
}
/* Returns 'false' if 'facility' is not a valid string. If 'facility'
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ovn.at: Fix ARP test that fails due to timing.
The test for "ARP/ND request broadcast limiting" checks that injected
ARP packets are not flooded using the MC_FLOOD multicast group. However,
this introduces a race condition in the test because GARPs generated by
OVN would also hit the same openflow rules.
Remove the checks that use the MC_FLOOD group. They are also redundant
as the rest of the test checks that packets are forwarded according to
the newly added, higher priority rules.
Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever
possible.")
Signed-off-by: Dumitru Ceara
---
tests/ovn.at | 33 -
1 file changed, 33 deletions(-)
diff --git a/tests/ovn.at b/tests/ovn.at
index 4baf2e9..9a44f0a 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -18015,9 +18015,6 @@ r1_dp_key=$(ovn-sbctl --bare --columns tunnel_key list
datapath_binding rtr1)
r1_tnl_key=$(ovn-sbctl --bare --columns tunnel_key list port_binding sw-rtr1)
r2_tnl_key=$(ovn-sbctl --bare --columns tunnel_key list port_binding sw-rtr2)
-mc_key=$(ovn-sbctl --bare --columns tunnel_key find multicast_group
datapath=${sw_dp_uuid} name="_MC_flood")
-mc_key=$(printf "%04x" $mc_key)
-
match_sw_metadata="metadata=0x${sw_dp_key}"
match_r1_metadata="metadata=0x${r1_dp_key}"
@@ -18042,11 +18039,6 @@ OVS_WAIT_UNTIL([
grep n_packets=1 -c)
test "0" = "${pkts_to_rtr2}"
])
-OVS_WAIT_UNTIL([
-pkts_flooded=$(ovs-ofctl dump-flows br-int | \
-grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
-test "0" = "${pkts_flooded}"
-])
# Inject ND_NS for ofirst router owned IP address.
src_ipv6=00100254
@@ -18069,11 +18061,6 @@ OVS_WAIT_UNTIL([
grep n_packets=1 -c)
test "0" = "${pkts_to_rtr2}"
])
-OVS_WAIT_UNTIL([
-pkts_flooded=$(ovs-ofctl dump-flows br-int | \
-grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
-test "0" = "${pkts_flooded}"
-])
# Configure load balancing on both routers.
ovn-nbctl lb-add lb1-v4 10.0.0.11 42.42.42.1
@@ -18108,11 +18095,6 @@ OVS_WAIT_UNTIL([
grep n_packets=1 -c)
test "0" = "${pkts_to_rtr2}"
])
-OVS_WAIT_UNTIL([
-pkts_flooded=$(ovs-ofctl dump-flows br-int | \
-grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
-test "0" = "${pkts_flooded}"
-])
# Inject ND_NS for first router owned VIP address.
src_ipv6=00100254
@@ -18135,11 +18117,6 @@ OVS_WAIT_UNTIL([
grep n_packets=1 -c)
test "0" = "${pkts_to_rtr2}"
])
-OVS_WAIT_UNTIL([
-pkts_flooded=$(ovs-ofctl dump-flows br-int | \
-grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
-test "0" = "${pkts_flooded}"
-])
# Configure NAT on both routers.
ovn-nbctl lr-nat-add rtr1 dnat_and_snat 10.0.0.111 42.42.42.1
@@ -18175,11 +18152,6 @@ OVS_WAIT_UNTIL([
grep n_packets=1 -c)
test "0" = "${pkts_to_rtr2}"
])
-OVS_WAIT_UNTIL([
-pkts_flooded=$(ovs-ofctl dump-flows br-int | \
-grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
-test "0" = "${pkts_flooded}"
-])
# Inject ARP request for FIP1.
send_arp_request 1 0 ${src_mac} $(ip_to_hex 10 0 0 254) $(ip_to_hex 10 0 0 121)
@@ -18242,11 +18214,6 @@ OVS_WAIT_UNTIL([
grep n_packets=1 -c)
test "0" = "${pkts_to_rtr2}"
])
-OVS_WAIT_UNTIL([
-pkts_flooded=$(ovs-ofctl dump-flows br-int | \
-grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
-test "0" = "${pkts_flooded}"
-])
# Inject ND_NS for FIP1.
src_ipv6=00100254
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH RFC ovn] Add VXLAN support for non-VTEP datapath bindings
(Sorry, top posting it) Hi Ihar, I tested out your patch. All the tests pass. I then tested out using the ovn-fake-multinode [1]. After the deployment, I changed the encap to vxlan. East/West traffic worked fine without any issues.. But I was not able to ping to the gateway router port IP from outside (172.16.0.100). The moment I changed the encap back to geneve, the ping works. I didn't look into the issue. So not sure what's the reason. You may want to try out once with ovn-fake-multinode Thanks Numan [1] - https://github.com/ovn-org/ovn-fake-multinode/ On Tue, Mar 24, 2020 at 1:37 PM Numan Siddique wrote: > > On Tue, Mar 24, 2020 at 5:17 AM Ben Pfaff wrote: > > > > On Mon, Mar 23, 2020 at 06:39:14PM -0400, Ihar Hrachyshka wrote: > > > First, some questions as to implementation (or feasibility) of several > > > todo items in my list for the patch. > > > > > > 1) I initially thought that, because VXLAN would have limited space > > > for both networks and ports in its VNI, the encap type would not be > > > able to support as many of both as Geneve / STT, and so we would need > > > to enforce the limit programmatically somehow. But in OVN context, is > > > it even doable? North DB resources may be created before any chassis > > > are registered; once a chassis that is VXLAN only joins, it's too late > > > to forbid the spilling resources from existence (though it may be a > > > good time to detect this condition and perhaps fail to register the > > > chassis / configure flow tables). How do we want to handle this case? > > > Do we fail to start VXLAN configured ovn-controller when too many > > > networks / ports per network created? Do we forbid creating too many > > > resources when a chassis is registered that is VXLAN only? Both? Or do > > > we leave it up to the deployment / CMS to control the chassis / north > > > DB configuration? > > > > > > 2) Similar to the issue above, I originally planned to forbid using > > > ACLs relying on ingress port when a VXLAN chassis is involved (because > > > the VNI won't carry the information). I believe the approach should be > > > similar to how we choose to handle the issue with the maximum number > > > of resources, described above. > > > > > > I am new to OVN so maybe there are existing examples for such > > > situations already that I could get inspiration from. Let me know what > > > you think. > > > > I don't have good solutions for the above resource limit problems. We > > designed OVN so that this kind of resource limit wouldn't be a problem > > in practice, so we didn't think through what would happen if the limits > > suddenly became more stringent. > > > > I think that it falls upon the CMS by default. > > I agree. I think It should be handled by CMS. > > Thanks > Numan > > > > > > > > Assuming we pick a term to use to describe these out-of-cluster > > > > > switches, we should consider the impact of the rename. Renaming > > > > > internal symbols / functions is trivial. But "vtep" is used in OVN > > > > > schema (for example, for port binding 'type' attribute). Do we want to > > > > > rename those too? If so, what considerations should we apply when > > > > > doing it? Any guidance as to maintaining backwards compatibility? > > > > > > > > > > Also, is such a rename something that should happen at the same moment > > > > > when we add support for VXLAN for in-cluster communication? Or should > > > > > it be a separate work item? (If so, do we expect it to land before or > > > > > after the core VXLAN implementation lands?) > > > > > > > > We can't (or at any rate should not) change the terms in the schema, but > > > > we can change other places and point out to people in a few places that > > > > a "ramp switch" is sometimes, confusingly, called a "vtep". > > > > > > Gotcha. Any preferences as to whether to consider it a preparatory > > > work item; a follow-up; or a part of the VXLAN implementation? (I lean > > > towards handling the ramp term introduction as an independent > > > preparatory step.) > > > > I sent out a patch for people to look at. > > ___ > > 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 ovs v1 2/2] netdev-dpdk: Allow setting MAC on DPDK interfaces
On 3/5/20 3:15 PM, Noa Ezra wrote:
> Adding a command for setting MAC of DPDK interfaces using:
> ovs-appctl netdev-dpdk/set-mac
>
> Signed-off-by: Noa Ezra
> Acked-by: Roni Bar Yanai
> ---
> lib/netdev-dpdk.c | 36
> 1 file changed, 36 insertions(+)
I'm still not 100% sure that it is the way we need to go since
this configuration will not survive OVS restarts, i.e. it's difficult
to use/maintain in something like OpenStack environment.
(Also, DPDK currently doesn't have any protection from MAC changing
on VFs from the inside of VM.)
But anyway. Comments inline.
Best regards, Ilya Maximets.
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index e375b3d..2b8adac 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -3917,6 +3917,38 @@ out:
> netdev_close(netdev);
> }
>
> +static void
> +netdev_dpdk_set_mac(struct unixctl_conn *conn, int argc OVS_UNUSED,
> +const char *argv[], void *aux OVS_UNUSED)
> +{
> +struct netdev *netdev = NULL;
> +char *response = NULL;
> +struct eth_addr mac;
> +int error;
> +
> +netdev = netdev_from_name(argv[1]);
> +if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
> +unixctl_command_reply_error(conn, "Not a DPDK Interface");
netdev needs to be closed.
> +return;
> +}
> +
> +if (!argv[2] || !eth_addr_from_string(argv[2], )) {
need to check that argc > 2 for consistency.
> +response = xasprintf("No MAC address to set.");
Why this is not treated as error? i.e. _reply_error().
> +goto out;
> +}
> +
> +error = netdev_dpdk_set_etheraddr(netdev, mac);
> +if (error) {
> +response = xasprintf("interface %s: setting MAC failed (%s)",
> + argv[1], ovs_strerror(error));
Why this is not treated as error?
> +}
> +response = xasprintf("set-mac done.");
> +
> +out:
> +unixctl_command_reply(conn, response);
response leaked.
> +netdev_close(netdev);
> +}
> +
> /*
> * Set virtqueue flags so that we do not receive interrupts.
> */
> @@ -4256,6 +4288,10 @@ netdev_dpdk_class_init(void)
> "[netdev]", 0, 1,
> netdev_dpdk_get_mempool_info, NULL);
>
> +unixctl_command_register("netdev-dpdk/set-mac",
> + "[netdev] [mac]", 2, 2,
> + netdev_dpdk_set_mac, NULL);
This command needs to be documented in man pages and NEWS.
> +
> ret = rte_eth_dev_callback_register(RTE_ETH_ALL,
> RTE_ETH_EVENT_INTR_RESET,
> dpdk_eth_event_callback, NULL);
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-offload-tc: Flush rules on ingress block when init tc flow api
On Tue, Mar 24, 2020 at 11:42:13AM +0200, Dmytro Linkin wrote:
> On Tue, Mar 03, 2020 at 07:37:45PM +0200, Dmytro Linkin wrote:
> > On Tue, Mar 03, 2020 at 04:42:12PM +0100, Ilya Maximets wrote:
> > > On 3/3/20 4:29 PM, Dmytro Linkin wrote:
> > > > On Tue, Mar 03, 2020 at 02:15:21PM +0100, Ilya Maximets wrote:
> > > >> On 3/3/20 8:58 AM, Roi Dayan wrote:
> > > >>>
> > > >>>
> > > >>> On 2020-02-27 5:22 PM, Roi Dayan wrote:
> > > From: Dmytro Linkin
> > >
> > > OVS can fail to attach ingress block on iface when init tc flow api,
> > > if block already exist with rules on it and is shared with other
> > > iface.
> > > Fix by flush all existing rules on the ingress block prior to
> > > deleting
> > > it.
> > >
> > > Fixes: 093c9458fb02 ("tc: allow offloading of block ids")
> > > Signed-off-by: Dmytro Linkin
> > > Acked-by: Raed Salem
> > > Acked-by: Roi Dayan
...
> Ping
Sorry for the delay.
This seems reasonable. I am running it through Travis CI and if
that succeeds I plan to push it to master, branch-2.13 and branch-2.12.
It did not seem to apply cleanly to branches for older releases
so please consider submitting backports if appropriate.
https://travis-ci.org/github/horms2/ovs/builds/666306585
https://travis-ci.org/github/horms2/ovs/builds/666306504
https://travis-ci.org/github/horms2/ovs/builds/666305789
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 1/2] dpif-netdev: Expand the meter capacity using cmap
On Tue, Mar 24, 2020 at 1:11 PM Ben Pfaff wrote: > > On Tue, Mar 24, 2020 at 09:25:41AM +0800, Tonghao Zhang wrote: > > On Mon, Mar 16, 2020 at 12:06 PM Tonghao Zhang > > wrote: > > > > > > On Mon, Mar 16, 2020 at 11:07 AM Yanqin Wei wrote: > > > > > > > > Hi Xiangxia, > > > > > > > > The meter id is allocated by id_pool, which can always return the > > > > lowest available id. > > > Yes, I added 7+ meters, and it works fine. > > > > There is a light scalable data struct which supports direct address > > > > lookup. It can achieve several times performance improvement than > > > > cmap_find. And it has not copy memory when expanding. > > > > https://patchwork.ozlabs.org/patch/1253447/ > > > > > > > > I suggest using this light data struct for >65535 meter instance. Would > > > > you like to take a look at it? > > > Yes, and I also wait Ilya to review, and offer a suggestion. > > Hi maintainers, > > Will you have a plan to help me to review those patches, thanks. > > I know that I skipped over this because I was waiting for v3 using > idpool. Hi Ben, The meter id is allocated by id_pool in ofproto-dpif layer(meter_set function use the id_pool_alloc_id to allocate it). I guess what Yanqin mean is that I should use the patches[1] he sent, to compare the performance with cmap. The patches of Yanqin are not in upstream, but I use the meter cache to address the issue. [1] - https://patchwork.ozlabs.org/patch/1253447/ -- Best regards, Tonghao ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3] ovn-northd: Forward ARP requests on localnet ports.
On 3/24/20 12:13 PM, Numan Siddique wrote:
> On Tue, Mar 24, 2020 at 3:34 PM Dumitru Ceara wrote:
>>
>> Commit 32f5ebb06226 limited the ARP/ND broadcast domain but in scenarios
>> where ARP responder flows are installed only on chassis that own the
>> associated logical ports ARP requests should still be forwarded on
>> localnet ports because the router pipeline should be executed on the
>> chassis that owns the logical port. Only that chassis will reply to the
>> ARP/ND request.
>>
>> Reported-by: Michael Plato
>> Reported-at:
>> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-March/049856.html
>> Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever
>> possible.")
>> Signed-off-by: Dumitru Ceara
>
> Thanks. I applied this patch to master and branch-20.03
>
> Numan
>
Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3] ovn-northd: Forward ARP requests on localnet ports.
On Tue, Mar 24, 2020 at 3:34 PM Dumitru Ceara wrote:
>
> Commit 32f5ebb06226 limited the ARP/ND broadcast domain but in scenarios
> where ARP responder flows are installed only on chassis that own the
> associated logical ports ARP requests should still be forwarded on
> localnet ports because the router pipeline should be executed on the
> chassis that owns the logical port. Only that chassis will reply to the
> ARP/ND request.
>
> Reported-by: Michael Plato
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-March/049856.html
> Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever
> possible.")
> Signed-off-by: Dumitru Ceara
Thanks. I applied this patch to master and branch-20.03
Numan
>
> ---
> V3:
> - Address Numan's comment and add unit test.
> V2:
> - Address Numan's comment and update ovn-northd documentation.
> ---
> northd/ovn-northd.8.xml | 3 +-
> northd/ovn-northd.c | 6 +-
> tests/ovn.at| 169
>
> 3 files changed, 162 insertions(+), 16 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 7d03cbc..1e0993e 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1194,7 +1194,8 @@ output;
> Priority-75 flows for each IP address/VIP/NAT address owned by a
> router port connected to the switch. These flows match ARP requests
> and ND packets for the specific IP addresses. Matched packets are
> -forwarded only to the router that owns the IP address.
> +forwarded only to the router that owns the IP address and, if
> +present, to the localnet port of the logical switch.
>
>
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index f648d2e..b76df05 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5903,8 +5903,12 @@ build_lswitch_rport_arp_req_flow_for_ip(struct sset
> *ips,
> ds_put_cstr(, "}");
>
> /* Send a the packet only to the router pipeline and skip flooding it
> - * in the broadcast domain.
> + * in the broadcast domain (except for the localnet port).
> */
> +if (od->localnet_port) {
> +ds_put_format(, "clone { outport = %s; output; }; ",
> + od->localnet_port->json_key);
> +}
> ds_put_format(, "outport = %s; output;", patch_op->json_key);
> ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_L2_LKUP, priority,
> ds_cstr(), ds_cstr(), stage_hint);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 1b6073f..4baf2e9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -17928,13 +17928,14 @@ net_add n1
> sim_add hv1
> as hv1
> ovs-vsctl add-br br-phys
> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
> ovn_attach n1 br-phys 192.168.0.1
>
> -ovs-vsctl -- add-port br-int hv1-vif1 -- \
> -set interface hv1-vif1 external-ids:iface-id=sw-agg-ext \
> -options:tx_pcap=hv1/vif1-tx.pcap \
> -options:rxq_pcap=hv1/vif1-rx.pcap \
> -ofport-request=1
> +sim_add hv2
> +as hv2
> +ovs-vsctl add-br br-phys
> +ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
> +ovn_attach n1 br-phys 192.168.0.2
>
> # One Aggregation Switch connected to two Logical networks (routers).
> ovn-nbctl ls-add sw-agg
> @@ -17950,18 +17951,66 @@ ovn-nbctl lsp-add sw-agg sw-rtr2 \
> -- lsp-set-addresses sw-rtr2 00:00:00:00:02:00 \
> -- lsp-set-options sw-rtr2 router-port=rtr2-sw
>
> -# Configure L3 interface IPv4 & IPv6 on both routers
> +# Localnet port on the Aggregation Switch.
> +ovn-nbctl lsp-add sw-agg sw-agg-ln
> +ovn-nbctl lsp-set-addresses sw-agg-ln unknown
> +ovn-nbctl lsp-set-type sw-agg-ln localnet
> +ovn-nbctl lsp-set-options sw-agg-ln network_name=phys
> +
> +# Configure L3 interface IPv4 & IPv6 on both routers.
> ovn-nbctl lr-add rtr1
> ovn-nbctl lrp-add rtr1 rtr1-sw 00:00:00:00:01:00 10.0.0.1/24 10::1/64
>
> +ovn-nbctl lrp-add rtr1 rtr1-sw1 00:00:01:00:00:00 20.0.0.1/24 20::1/64
> +
> ovn-nbctl lr-add rtr2
> ovn-nbctl lrp-add rtr2 rtr2-sw 00:00:00:00:02:00 10.0.0.2/24 10::2/64
>
> +# Configure router gateway ports.
> +ovn-nbctl lrp-set-gateway-chassis rtr1-sw hv1 20
> +ovn-nbctl lrp-set-gateway-chassis rtr2-sw hv1 20
> +
> +# One private network behind rtr1 with two VMs.
> +ovn-nbctl ls-add sw1
> +ovn-nbctl lsp-add sw1 sw1-p1 \
> +-- lsp-set-addresses sw1-p1 00:00:00:01:00:00
> +ovn-nbctl lsp-add sw1 sw1-p2 \
> +-- lsp-set-addresses sw1-p2 00:00:00:02:00:00
> +ovn-nbctl lsp-add sw1 sw1-rtr1 \
> +-- lsp-set-type sw1-rtr1 router \
> +-- lsp-set-addresses sw1-rtr1 00:00:01:00:00:00 \
> +-- lsp-set-options sw1-rtr1 router-port=rtr1-sw1
> +
> +# Bind a "VM" connected to sw-agg on hv1.
> +as hv1
> +ovs-vsctl -- add-port br-int hv1-vif0 -- \
> +set interface hv1-vif0 external-ids:iface-id=sw-agg-ext \
> +
Re: [ovs-dev] [PATCH ovn v2] ovn-northd: Forward ARP requests on localnet ports.
On 3/23/20 1:45 PM, Numan Siddique wrote:
> On Mon, Mar 23, 2020 at 6:05 PM Dumitru Ceara wrote:
>>
>> Commit 32f5ebb06226 limited the ARP/ND broadcast domain but in scenarios
>> where ARP responder flows are installed only on chassis that own the
>> associated logical ports ARP requests should still be forwarded on
>> localnet ports because the router pipeline should be executed on the
>> chassis that owns the logical port. Only that chassis will reply to the
>> ARP/ND request.
>>
>> Reported-by: Michael Plato
>> Reported-at:
>> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-March/049856.html
>> Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever
>> possible.")
>> Signed-off-by: Dumitru Ceara
>
> Thanks for the v2. The patch LGTM.
> How about adding a test case which replicates the scenario reported by
> Michael ? so that
> we don't regress later ?
>
You're right, thanks for reviewing the patch. I sent a V3 which now
fixes the tests to cover this scenario too:
https://patchwork.ozlabs.org/patch/1260545/
Thanks,
Dumitru
> Thanks
> Numan
>
>>
>> ---
>> V2:
>> - Address Numan's comment and update ovn-northd documentation.
>> ---
>> northd/ovn-northd.8.xml | 3 ++-
>> northd/ovn-northd.c | 6 +-
>> 2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
>> index 7d03cbc..1e0993e 100644
>> --- a/northd/ovn-northd.8.xml
>> +++ b/northd/ovn-northd.8.xml
>> @@ -1194,7 +1194,8 @@ output;
>> Priority-75 flows for each IP address/VIP/NAT address owned by a
>> router port connected to the switch. These flows match ARP requests
>> and ND packets for the specific IP addresses. Matched packets are
>> -forwarded only to the router that owns the IP address.
>> +forwarded only to the router that owns the IP address and, if
>> +present, to the localnet port of the logical switch.
>>
>>
>>
>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>> index f648d2e..b76df05 100644
>> --- a/northd/ovn-northd.c
>> +++ b/northd/ovn-northd.c
>> @@ -5903,8 +5903,12 @@ build_lswitch_rport_arp_req_flow_for_ip(struct sset
>> *ips,
>> ds_put_cstr(, "}");
>>
>> /* Send a the packet only to the router pipeline and skip flooding it
>> - * in the broadcast domain.
>> + * in the broadcast domain (except for the localnet port).
>> */
>> +if (od->localnet_port) {
>> +ds_put_format(, "clone { outport = %s; output; }; ",
>> + od->localnet_port->json_key);
>> +}
>> ds_put_format(, "outport = %s; output;", patch_op->json_key);
>> ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_L2_LKUP, priority,
>> ds_cstr(), ds_cstr(), stage_hint);
>> --
>> 1.8.3.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
[ovs-dev] [PATCH ovn v3] ovn-northd: Forward ARP requests on localnet ports.
Commit 32f5ebb06226 limited the ARP/ND broadcast domain but in scenarios
where ARP responder flows are installed only on chassis that own the
associated logical ports ARP requests should still be forwarded on
localnet ports because the router pipeline should be executed on the
chassis that owns the logical port. Only that chassis will reply to the
ARP/ND request.
Reported-by: Michael Plato
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-March/049856.html
Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever
possible.")
Signed-off-by: Dumitru Ceara
---
V3:
- Address Numan's comment and add unit test.
V2:
- Address Numan's comment and update ovn-northd documentation.
---
northd/ovn-northd.8.xml | 3 +-
northd/ovn-northd.c | 6 +-
tests/ovn.at| 169
3 files changed, 162 insertions(+), 16 deletions(-)
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 7d03cbc..1e0993e 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1194,7 +1194,8 @@ output;
Priority-75 flows for each IP address/VIP/NAT address owned by a
router port connected to the switch. These flows match ARP requests
and ND packets for the specific IP addresses. Matched packets are
-forwarded only to the router that owns the IP address.
+forwarded only to the router that owns the IP address and, if
+present, to the localnet port of the logical switch.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index f648d2e..b76df05 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -5903,8 +5903,12 @@ build_lswitch_rport_arp_req_flow_for_ip(struct sset *ips,
ds_put_cstr(, "}");
/* Send a the packet only to the router pipeline and skip flooding it
- * in the broadcast domain.
+ * in the broadcast domain (except for the localnet port).
*/
+if (od->localnet_port) {
+ds_put_format(, "clone { outport = %s; output; }; ",
+ od->localnet_port->json_key);
+}
ds_put_format(, "outport = %s; output;", patch_op->json_key);
ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_L2_LKUP, priority,
ds_cstr(), ds_cstr(), stage_hint);
diff --git a/tests/ovn.at b/tests/ovn.at
index 1b6073f..4baf2e9 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -17928,13 +17928,14 @@ net_add n1
sim_add hv1
as hv1
ovs-vsctl add-br br-phys
+ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
ovn_attach n1 br-phys 192.168.0.1
-ovs-vsctl -- add-port br-int hv1-vif1 -- \
-set interface hv1-vif1 external-ids:iface-id=sw-agg-ext \
-options:tx_pcap=hv1/vif1-tx.pcap \
-options:rxq_pcap=hv1/vif1-rx.pcap \
-ofport-request=1
+sim_add hv2
+as hv2
+ovs-vsctl add-br br-phys
+ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+ovn_attach n1 br-phys 192.168.0.2
# One Aggregation Switch connected to two Logical networks (routers).
ovn-nbctl ls-add sw-agg
@@ -17950,18 +17951,66 @@ ovn-nbctl lsp-add sw-agg sw-rtr2 \
-- lsp-set-addresses sw-rtr2 00:00:00:00:02:00 \
-- lsp-set-options sw-rtr2 router-port=rtr2-sw
-# Configure L3 interface IPv4 & IPv6 on both routers
+# Localnet port on the Aggregation Switch.
+ovn-nbctl lsp-add sw-agg sw-agg-ln
+ovn-nbctl lsp-set-addresses sw-agg-ln unknown
+ovn-nbctl lsp-set-type sw-agg-ln localnet
+ovn-nbctl lsp-set-options sw-agg-ln network_name=phys
+
+# Configure L3 interface IPv4 & IPv6 on both routers.
ovn-nbctl lr-add rtr1
ovn-nbctl lrp-add rtr1 rtr1-sw 00:00:00:00:01:00 10.0.0.1/24 10::1/64
+ovn-nbctl lrp-add rtr1 rtr1-sw1 00:00:01:00:00:00 20.0.0.1/24 20::1/64
+
ovn-nbctl lr-add rtr2
ovn-nbctl lrp-add rtr2 rtr2-sw 00:00:00:00:02:00 10.0.0.2/24 10::2/64
+# Configure router gateway ports.
+ovn-nbctl lrp-set-gateway-chassis rtr1-sw hv1 20
+ovn-nbctl lrp-set-gateway-chassis rtr2-sw hv1 20
+
+# One private network behind rtr1 with two VMs.
+ovn-nbctl ls-add sw1
+ovn-nbctl lsp-add sw1 sw1-p1 \
+-- lsp-set-addresses sw1-p1 00:00:00:01:00:00
+ovn-nbctl lsp-add sw1 sw1-p2 \
+-- lsp-set-addresses sw1-p2 00:00:00:02:00:00
+ovn-nbctl lsp-add sw1 sw1-rtr1 \
+-- lsp-set-type sw1-rtr1 router \
+-- lsp-set-addresses sw1-rtr1 00:00:01:00:00:00 \
+-- lsp-set-options sw1-rtr1 router-port=rtr1-sw1
+
+# Bind a "VM" connected to sw-agg on hv1.
+as hv1
+ovs-vsctl -- add-port br-int hv1-vif0 -- \
+set interface hv1-vif0 external-ids:iface-id=sw-agg-ext \
+options:tx_pcap=hv1/vif0-tx.pcap \
+options:rxq_pcap=hv1/vif0-rx.pcap \
+ofport-request=1
+
+# Bind a "VM" connected to sw1 on hv1.
+as hv1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+set interface hv1-vif1 external-ids:iface-id=sw1-p1 \
+options:tx_pcap=hv1/vif1-tx.pcap \
+options:rxq_pcap=hv1/vif1-rx.pcap \
+ofport-request=2
+
+#
Re: [ovs-dev] [PATCH ovn v2] Avoid nb_cfg update notification flooding
On Mon, Mar 23, 2020 at 8:49 PM wrote:
>
> From: Lucas Alvares Gomes
>
> nb_cfg as a mechanism to "ping" OVN control plane is very useful
> in many ways. However, the current implementation will trigger
> update notifications flooding in the whole control plane. Each
> HV updates to SB the nb_cfg number and all these updates are
> notified to all the other HVs, which is O(n^2). Although updates
> are batched in fewers notifications than n^2, it still generates
> significant load on SB DB and also on ovn-controllers.
>
> To solve this problem and make the mechanism more useful in large
> scale producation deployment, this patch separates the per HV
> *private* data (write only by the owning chassis and not
> interesting to any other HVs) from the Chassis table to a separate
> table, so that each HV can conditionally monitor and get updates
> only for its own record.
>
> Test result shows great improvement:
> In a test environment with 1K sandbox HVs, and 10K ports created
> on 40 lswitches and 8 lrouters, do the sync test when the system
> is idle, with command:
>
> time ovn-nbctl --wait=hv sync
>
> Original result:
> real4m52.926s
> user0m0.328s
> sys 0m0.004s
>
> With this patch:
> real0m11.405s
> user0m0.316s
> sys 0m0.016s
>
> Also, regarding backwards compatibility note that the nb_cfg from the
> Chassis table is no longer updated. If any system is relying on this
> mechanism they should start using the nb_cfg from the Chassis_Private
> table from now on.
>
> Co-authored-by: Han Zhou
> Signed-off-by: Han Zhou
> Signed-off-by: Lucas Alvares Gomes
Thanks Lucas for v2.
I did some testing and I'm seeing a couple of issues
- when I start a sandbox, ovn-controller never creates a chassis row and I see
below issues in the log
***
2020-03-24T09:38:53.376Z|65637|jsonrpc|DBG|ssl:127.0.0.1:6642:
received reply,
result=[{"uuid":["uuid","cf141b1a-f601-4035-b444-b856481a110f"]},{"uuid":["uuid","20c335d8-cb7f-4544-a23a-fb53547438e7"]},{"details":"RBAC
rules for client \"chassis-1\" role \"ovn-controller\" prohibit row
insertion into table \"Chassis_Private\".","error":"permission
error"},null], id=56156
2020-03-24T09:38:53.376Z|65638|jsonrpc|DBG|ssl:127.0.0.1:6642:
received reply, result={}, id=56157
2020-03-24T09:38:53.376Z|65639|chassis|DBG|Could not find Chassis,
will create it: stored (chassis-1) ovs (chassis-1)
- When I run the test suite with - make check TESTSUITEFLAGS="-j5",
90% of the time I see 2 tests failing. I don't see this
behavior with master.
In one run I see below 2 tests failing and looks like the test
fails because it didn't receive the expected packet. If I run the same
test again individually it passes. Seems like something is wrong.
39: ovn -- 2 HVs, 2 LS, 1 lport/LS, 2 peer LRs FAILED
(ovs-macros.at:220)
73: ovn -- nested containersFAILED (ovs-macros.at:220)
Below is the tail of testsuite.log of test case 39
actions: output enqueue set_vlan_vid set_vlan_pcp strip_vlan
mod_dl_src mod_dl_dst mod_nw_src mod_nw_dst mod_nw_tos mod_tp_src
mod_tp_dst
1(hv2-vif1): addr:aa:55:aa:55:00:1f
config: 0
state: 0
speed: 0 Mbps now, 0 Mbps max
2(ovn-hv1-0): addr:ee:bb:1f:6b:1e:ae
config: 0
state: 0
speed: 0 Mbps now, 0 Mbps max
LOCAL(br-int): addr:1e:36:c5:d5:84:41
config: 0
state: 0
speed: 0 Mbps now, 0 Mbps max
OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0
NXST_FLOW reply (xid=0x4):
checking packets in hv2/vif1-tx.pcap against expected:
ovn.at:12: waiting until $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in"
$rcv_pcap > $rcv_text
rcv_n=`wc -l < "$rcv_text"`
echo "rcv_n=$rcv_n exp_n=$exp_n"
test $rcv_n -ge $exp_n...
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
rcv_n=0 exp_n=1
ovn.at:12: wait failed after 10 seconds
../../tests/ovs-macros.at:220: hard failure
39. ovn.at:3781: 39. ovn -- 2 HVs, 2 LS, 1 lport/LS, 2 peer LRs
(ovn.at:3781): FAILED (ovs-macros.at:220)
**
Sometimes I see similar failures even with ovn master. Running those
tests individually always passes.
I'm not really sure if this patch is the cause of these failures. But
this patch is making these tests fail more often.
Thanks
Numan
> ---
>
> v1 - v2 -> Addressed Dumitru's comments: ovn-northd no longer monitoring
> the external_ids column from Chassis and Chassis_Private tables, added an
> item to the NEWS file regarding the backward compatibility of this patch
> and added an item to the TODO file regarding conditionally monitoring
> a set of columns.
>
> NEWS| 6 +-
> TODO.rst| 3 +++
> controller/chassis.c| 19 +--
> controller/chassis.h| 8 ++--
> controller/ovn-controller.c | 37
Re: [ovs-dev] [PATCH] netdev-offload-tc: Flush rules on ingress block when init tc flow api
On Tue, Mar 03, 2020 at 07:37:45PM +0200, Dmytro Linkin wrote:
> On Tue, Mar 03, 2020 at 04:42:12PM +0100, Ilya Maximets wrote:
> > On 3/3/20 4:29 PM, Dmytro Linkin wrote:
> > > On Tue, Mar 03, 2020 at 02:15:21PM +0100, Ilya Maximets wrote:
> > >> On 3/3/20 8:58 AM, Roi Dayan wrote:
> > >>>
> > >>>
> > >>> On 2020-02-27 5:22 PM, Roi Dayan wrote:
> > From: Dmytro Linkin
> >
> > OVS can fail to attach ingress block on iface when init tc flow api,
> > if block already exist with rules on it and is shared with other iface.
> > Fix by flush all existing rules on the ingress block prior to deleting
> > it.
> >
> > Fixes: 093c9458fb02 ("tc: allow offloading of block ids")
> > Signed-off-by: Dmytro Linkin
> > Acked-by: Raed Salem
> > Acked-by: Roi Dayan
> > ---
> > lib/netdev-offload-tc.c | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> > index 550e440b3a45..b5ff6ccca55e 100644
> > --- a/lib/netdev-offload-tc.c
> > +++ b/lib/netdev-offload-tc.c
> > @@ -1907,6 +1907,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
> > static struct ovsthread_once block_once =
> > OVSTHREAD_ONCE_INITIALIZER;
> > enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
> > uint32_t block_id = 0;
> > +struct tcf_id id;
> > int ifindex;
> > int error;
> >
> > @@ -1917,6 +1918,14 @@ netdev_tc_init_flow_api(struct netdev *netdev)
> > return -ifindex;
> > }
> >
> > +block_id = get_block_id_from_netdev(netdev);
> > +
> > +/* Flush rules explicitly needed when we work with ingress_block,
> > + * so we will not fail with reattaching block to bond iface, for
> > ex.
> > + */
> > +id = tc_make_tcf_id(ifindex, block_id, 0, hook);
> > +tc_del_filter();
> > +
> > /* make sure there is no ingress/egress qdisc */
> > tc_add_del_qdisc(ifindex, false, 0, hook);
> >
> > @@ -1930,7 +1939,6 @@ netdev_tc_init_flow_api(struct netdev *netdev)
> > ovsthread_once_done(_mask_once);
> > }
> >
> > -block_id = get_block_id_from_netdev(netdev);
> > error = tc_add_del_qdisc(ifindex, true, block_id, hook);
> >
> > if (error && error != EEXIST) {
> >
> > >>>
> > >>> +ilya
> > >>>
> > >>> Hi Ilya,
> > >>>
> > >>> can you help review/ack this?
> > >>
> > >> Hi. I'm not an expert in linux tc, but since you're asking...
> > >>
> > >> IIUC, the issue is that tc_add_del_qdisc(ifindex, true, block_id, hook)
> > >> fails on bond iface. Is it correct?
> > >
> > > At first tc_add_del_qdisc() fails on deletion, because qdisc, which is
> > > added to block, is in use (rules are exist). We just don't care about
> > > any error returned. Then tc_add_del_qdisc() fail to add it, because it
> > > wasn't deleted and in use.
> > >
> > >> In this case I see one strange thing. We're clearing ingress qdisk
> > >> for block_id == 0, but after that trying to create new one with
> > >> block_id == ifindex (for LAG interface). Will it help if we delete
> > >> ingress qdisk providing correct block_id? This sounds like something
> > >> sane to do.
> > >
> > > Deleting block_id != 0 will fail, due to existing rules. But actually,
> > > deleting qdisc with block_id == 0 still delete correct block.
> > >
> > > Anyway, the point here is to flush rules on specified ingress block.
> >
> > Hmm. OK.
> >
> > Shouldn't we unconditionally flush rules from qdisk inside the
> > tc_add_del_qdisc() if deletion is requested? From your explanation
> > it seems like a prerequisite for that and qdisk will not be deleted
> > if we will not flush rules.
>
> Flushing rules needed only for shared block, 'cause like in our case,
> there are ifaces not attached to the bridge, so OVS don't ask kernel to
> delete qdisc. In other known cases kernel flush rules by itself on qdisc
> deletion, so flush inside tc_add_del_qdisc() mostly not needed.
> Mb, better do it for shared blocks only.
>
> > BTW, one thing in this patch that doesn't look good is that you're
> > using block_id before probing the support.
>
> Probably yes. What You suggest here? By case, it work. Probing triggers
> on internal OVS ifaces at first, so later calls to get block_id return
> valid id.
>
> Regards, Dmytro
Ping
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovs v1 0/2] Allow setting MAC on DPDK interfaces
Ping again. > -Original Message- > From: Noa Levy > Sent: Monday, March 16, 2020 10:20 AM > To: Tonghao Zhang > Cc: ovs-dev@openvswitch.org; Ameer Mahagneh > ; i.maxim...@ovn.org; Eli Britstein > ; Roni Bar Yanai > Subject: RE: [ovs-dev] [PATCH ovs v1 0/2] Allow setting MAC on DPDK > interfaces > > > > > -Original Message- > > From: Tonghao Zhang > > Sent: Monday, March 16, 2020 9:07 AM > > To: Noa Levy > > Cc: ovs-dev@openvswitch.org; Ameer Mahagneh > ; > > i.maxim...@ovn.org > > Subject: Re: [ovs-dev] [PATCH ovs v1 0/2] Allow setting MAC on DPDK > > interfaces > > > > On Sun, Mar 15, 2020 at 5:49 PM Noa Levy wrote: > > > > > > ping > > > > > > > -Original Message- > > > > From: Noa Ezra > > > > Sent: Thursday, March 5, 2020 4:15 PM > > > > To: ovs-dev@openvswitch.org > > > > Cc: i.maxim...@ovn.org; Ameer Mahagneh > ; > > Noa > > > > Levy ; Roni Bar Yanai ; > > > > e...@labmailer.mlnx; Majd Dibbiny > > > > Subject: [PATCH ovs v1 0/2] Allow setting MAC on DPDK interfaces > > > > > > > > In cloud topology, when SR-IOV with port representors is in use > > > > and VM is not trusted, the orchestration should set the VF mac address. > > > > When using DPDK there is an architecture limitation to set the VF > > > > mac address from host (Linux tooling). > > > > According to previous discussion > > > > (https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2 > > > > Fp > > > > atc > > > > > > > hwork.ozlabs.org%2Fpatch%2F1215075%2Fdata=02%7C01%7Cnoae%4 > > > > > > > 0mellanox.com%7C334e05153e934601f50308d7c10fb987%7Ca652971c7d2e4d > > > > > > > 9ba6a4d149256f461b%7C0%7C0%7C637190145598418235sdata=kEiOav > > > > > > > MVwc5zf4GCLVBeenDFAfk9Sur%2FHM21X%2FIRbA0%3Dreserved=0), > > > > it was agreed to add a new API in ovs-appctl for setting MAC > > > > address on port representors. > > > > > > > > ovs-appctl netdev-dpdk/set-mac > > I guess we can use "ip" tool to set MAC? > For some NIC vendors using "ifconfig" or "ip" commands - is not an option (if > the NIC is not bifurcated). > Therefore OpenStack should use the OVS API to set the MAC address for > dpdk interfaces. > > > > Ilya Maximets (1): > > > > netdev-dpdk: Add ability to set MAC address. > > > > > > > > Noa Ezra (1): > > > > netdev-dpdk: Allow setting MAC on DPDK interfaces > > > > > > > > lib/netdev-dpdk.c | 55 > > > > --- > > > > 1 file changed, 52 insertions(+), 3 deletions(-) > > > > > > > > -- > > > > 1.8.3.1 > > > > > > ___ > > > dev mailing list > > > d...@openvswitch.org > > > > > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail > > > .openvswitch.org%2Fmailman%2Flistinfo%2Fovs- > > devdata=02%7C01%7Cnoa > > > > > > e%40mellanox.com%7C3263fe845dc64809650a08d7c978b473%7Ca652971c7d > > 2e4d9b > > > > > > a6a4d149256f461b%7C0%7C1%7C637199392568994096sdata=t357ROa > > uVrjFZL > > > KSBc4zqYfMjdbGngE2fPNbcItnCI0%3Dreserved=0 > > > > > > > > -- > > Thanks, > > Tonghao ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 OVN] ovn-nbctl.c: Add an optional way to delete QoS by uuid
On Thu, Mar 19, 2020 at 11:23 AM Tao YunXiang
wrote:
>
> We can delete qos by specify ls and more parameters.
> If CMS want to delete it exactly, it must specify detailed "match" field.
> It's not an easy way, also maybe deleted by mistake.
> This change adds a way to specify ls and uuid, which is optional.
> You can still use the previous method to delete.
>
> usage:
> ovn-nbctl qos-del ls0 [UUID0]
>
> Author: Tao YunXiang
> Co-authored-by: Liu Chang
> Co-authored-by: Rong Yin
> Signed-off-by: Tao YunXiang
> Signed-off-by: Liu Chang
> Signed-off-by: Rong Yin
Can you please add a few tests in the tests/ovn-nbctl.at which would help
in regressions ?
Thanks
Numan
>
> ---
> v4: Add a way to delete QoS by its name or uuid
> v3: ovn-nbctl.c: Add a way to delete QoS by its name or uuid
>
> ---
> utilities/ovn-nbctl.c | 39 ---
> 1 file changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index e80058e61..5b2fa6084 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -604,7 +604,7 @@ ACL commands:\n\
> QoS commands:\n\
>qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]]
> [dscp=DSCP]\n\
> add an QoS rule to SWITCH\n\
> - qos-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
> + qos-del SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]\n\
> remove QoS rules from SWITCH\n\
>qos-list SWITCH print QoS rules for SWITCH\n\
> \n\
> @@ -2521,22 +2521,39 @@ nbctl_qos_del(struct ctl_context *ctx)
> }
>
> const char *direction;
> -error = parse_direction(ctx->argv[2], );
> -if (error) {
> -ctx->error = error;
> -return;
> +const struct uuid *qos_rule_uuid = NULL;
> +struct uuid uuid_from_cmd;
> +if (uuid_from_string(_from_cmd, ctx->argv[2])) {
> +qos_rule_uuid = _from_cmd;
> +} else {
> +error = parse_direction(ctx->argv[2], );
> +if (error) {
> +ctx->error = error;
> +return;
> +}
> }
>
> -/* If priority and match are not specified, delete all qos_rules with the
> - * specified direction. */
> +/* If uuid was specified, delete qos_rule with the
> + * specified uuid. */
> if (ctx->argc == 3) {
> struct nbrec_qos **new_qos_rules
> = xmalloc(sizeof *new_qos_rules * ls->n_qos_rules);
>
> int n_qos_rules = 0;
> -for (size_t i = 0; i < ls->n_qos_rules; i++) {
> -if (strcmp(direction, ls->qos_rules[i]->direction)) {
> -new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
> +if (qos_rule_uuid) {
> +for (size_t i = 0; i < ls->n_qos_rules; i++) {
> +if (!uuid_equals(qos_rule_uuid,
> + &(ls->qos_rules[i]->header_.uuid))) {
> +new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
> +}
> +}
> +/* If priority and match are not specified, delete all qos_rules
> + * with the specified direction. */
> +} else {
> +for (size_t i = 0; i < ls->n_qos_rules; i++) {
> +if (strcmp(direction, ls->qos_rules[i]->direction)) {
> +new_qos_rules[n_qos_rules++] = ls->qos_rules[i];
> +}
> }
> }
>
> @@ -6030,7 +6047,7 @@ static const struct ctl_command_syntax nbctl_commands[]
> = {
> { "qos-add", 5, 7,
>"SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]]
> [dscp=DSCP]",
>NULL, nbctl_qos_add, NULL, "--may-exist", RW },
> -{ "qos-del", 1, 4, "SWITCH [DIRECTION [PRIORITY MATCH]]", NULL,
> +{ "qos-del", 1, 4, "SWITCH [{DIRECTION | UUID} [PRIORITY MATCH]]", NULL,
>nbctl_qos_del, NULL, "", RW },
> { "qos-list", 1, 1, "SWITCH", NULL, nbctl_qos_list, NULL, "", RO },
>
> --
> 2.17.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 OVN v2] ovn-nbctl.c: Add an optional way to delete router policy by uuid
On Tue, Mar 24, 2020 at 7:20 AM Tao YunXiang
wrote:
>
> We can delete router policy by specify lr and more parameters.
> If CMS want to delete it exactly, it must specify detailed "match" field.
> It's not an easy way, also maybe deleted by mistake.
> This change adds a way to specify lr and uuid, which is optional.
> You can still use the previous method to delete.
>
> usage:
> ovn-nbctl lr-policy-del lr0 [UUID0]
>
> Author: Tao YunXiang
> Co-authored-by: Liu Chang
> Co-authored-by: Rong Yin
> Signed-off-by: Tao YunXiang
> Signed-off-by: Liu Chang
> Signed-off-by: Rong Yin
Can you please add a few tests in the tests/ovn-nbctl.at which would help
in regressions ?
Thanks
Numan
>
> ---
> utilities/ovn-nbctl.8.xml | 51 +++
> utilities/ovn-nbctl.c | 41 ++-
> 2 files changed, 81 insertions(+), 11 deletions(-)
>
> diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> index d973be259..b91e39042 100644
> --- a/utilities/ovn-nbctl.8.xml
> +++ b/utilities/ovn-nbctl.8.xml
> @@ -710,6 +710,57 @@
>
>
>
> +Logical Router Policy Commands
> +
> +
> + lr-policy-add router priority
> + match action [nexthop]
> +
> +
> + Add Policy to router which provides a way to configure
> + permit/deny and reroute policies on the router. Permit/deny
> policies
> + are similar to OVN ACLs, but exist on the logical-router. Reroute
> + policies are needed for service-insertion and service-chaining.
> + nexthop is an optional parameter. It needs to be
> provided
> + only when action is reroute. A policy is
> + uniquely identified by priority and match.
> + Multiple policies can have the same priority.
> +
> +
> +
> + The following example shows a policy to lr1, which will drop
> packets
> + from192.168.100.0/24.
> +
> +
> +
> + lr-policy-add lr1 100 ip4.src == 192.168.100.0/24
> drop.
> +
> +
> +
> +
> + lr-policy-del router [{priority |
> uuid}
> + [match]]
> +
> +
> + Deletes polices from router. If only router
> + is supplied, all the polices from the logical router are deleted.
> If
> + priority and/or match are also specified,
> then
> + all the polices that match the conditions will be deleted from the
> + logical router.
> +
> +
> +
> + If router and uuid are supplied, then the
> + policy with sepcified uuid is deleted.
> +
> +
> +
> + lr-policy-list router
> +
> +Lists the polices on router.
> +
> +
> +
> NAT Commands
>
>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index e80058e61..1983b5f95 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -696,7 +696,7 @@ Route commands:\n\
> Policy commands:\n\
>lr-policy-add ROUTER PRIORITY MATCH ACTION [NEXTHOP]\n\
> add a policy to router\n\
> - lr-policy-del ROUTER [PRIORITY [MATCH]]\n\
> + lr-policy-del ROUTER [{PRIORITY | UUID} [MATCH]]\n\
> remove policies from ROUTER\n\
>lr-policy-list ROUTER print policies for ROUTER\n\
> \n\
> @@ -3587,21 +3587,40 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
> return;
> }
>
> -error = parse_priority(ctx->argv[2], );
> -if (error) {
> -ctx->error = error;
> -return;
> +const struct uuid *lr_policy_uuid = NULL;
> +struct uuid uuid_from_cmd;
> +if (uuid_from_string(_from_cmd, ctx->argv[2])) {
> +lr_policy_uuid = _from_cmd;
> +} else {
> +error = parse_priority(ctx->argv[2], );
> +if (error) {
> +ctx->error = error;
> +return;
> +}
> +
> }
> -/* If match is not specified, delete all routing policies with the
> - * specified priority. */
> +/* If uuid was specified, delete routing policy with the
> + * specified uuid. */
> if (ctx->argc == 3) {
> struct nbrec_logical_router_policy **new_policies
> = xmemdup(lr->policies,
>sizeof *new_policies * lr->n_policies);
> int n_policies = 0;
> -for (int i = 0; i < lr->n_policies; i++) {
> -if (priority != lr->policies[i]->priority) {
> -new_policies[n_policies++] = lr->policies[i];
> +
> +if (lr_policy_uuid) {
> +for (size_t i = 0; i < lr->n_policies; i++) {
> +if (!uuid_equals(lr_policy_uuid,
> + &(lr->policies[i]->header_.uuid))) {
> +new_policies[n_policies++] = lr->policies[i];
> +}
> +}
> +/* If match is not specified, delete all routing policies with the
> + * specified priority. */
> +
Re: [ovs-dev] [PATCH RFC ovn] Add VXLAN support for non-VTEP datapath bindings
On Tue, Mar 24, 2020 at 5:17 AM Ben Pfaff wrote: > > On Mon, Mar 23, 2020 at 06:39:14PM -0400, Ihar Hrachyshka wrote: > > First, some questions as to implementation (or feasibility) of several > > todo items in my list for the patch. > > > > 1) I initially thought that, because VXLAN would have limited space > > for both networks and ports in its VNI, the encap type would not be > > able to support as many of both as Geneve / STT, and so we would need > > to enforce the limit programmatically somehow. But in OVN context, is > > it even doable? North DB resources may be created before any chassis > > are registered; once a chassis that is VXLAN only joins, it's too late > > to forbid the spilling resources from existence (though it may be a > > good time to detect this condition and perhaps fail to register the > > chassis / configure flow tables). How do we want to handle this case? > > Do we fail to start VXLAN configured ovn-controller when too many > > networks / ports per network created? Do we forbid creating too many > > resources when a chassis is registered that is VXLAN only? Both? Or do > > we leave it up to the deployment / CMS to control the chassis / north > > DB configuration? > > > > 2) Similar to the issue above, I originally planned to forbid using > > ACLs relying on ingress port when a VXLAN chassis is involved (because > > the VNI won't carry the information). I believe the approach should be > > similar to how we choose to handle the issue with the maximum number > > of resources, described above. > > > > I am new to OVN so maybe there are existing examples for such > > situations already that I could get inspiration from. Let me know what > > you think. > > I don't have good solutions for the above resource limit problems. We > designed OVN so that this kind of resource limit wouldn't be a problem > in practice, so we didn't think through what would happen if the limits > suddenly became more stringent. > > I think that it falls upon the CMS by default. I agree. I think It should be handled by CMS. Thanks Numan > > > > > Assuming we pick a term to use to describe these out-of-cluster > > > > switches, we should consider the impact of the rename. Renaming > > > > internal symbols / functions is trivial. But "vtep" is used in OVN > > > > schema (for example, for port binding 'type' attribute). Do we want to > > > > rename those too? If so, what considerations should we apply when > > > > doing it? Any guidance as to maintaining backwards compatibility? > > > > > > > > Also, is such a rename something that should happen at the same moment > > > > when we add support for VXLAN for in-cluster communication? Or should > > > > it be a separate work item? (If so, do we expect it to land before or > > > > after the core VXLAN implementation lands?) > > > > > > We can't (or at any rate should not) change the terms in the schema, but > > > we can change other places and point out to people in a few places that > > > a "ramp switch" is sometimes, confusingly, called a "vtep". > > > > Gotcha. Any preferences as to whether to consider it a preparatory > > work item; a follow-up; or a part of the VXLAN implementation? (I lean > > towards handling the ramp term introduction as an independent > > preparatory step.) > > I sent out a patch for people to look at. > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3]travis: Enable OvS Travis CI for arm.
Hi Ilya,
Thanks for your comments. We applied your suggestions and sent patch v4. Please
see the new patch.
Thanks for your time.
Best regards,
Lance Yang
> -Original Message-
> From: Ilya Maximets
> Sent: Tuesday, March 24, 2020 1:25 AM
> To: Lance Yang ; ovs-dev@openvswitch.org
> Cc: nd ; i.maxim...@ovn.org; Yanqin Wei ;
> Ruifeng
> Wang ; Gavin Hu ; i.maxim...@ovn.org
> Subject: Re: [ovs-dev] [PATCH v3]travis: Enable OvS Travis CI for arm.
>
> On 3/19/20 8:51 AM, Lance Yang wrote:
> > Enable part of travis jobs with gcc compiler for arm64 architecture
> >
> > 1. Add arm jobs into the matrix in .travis.yml configuration file 2.
> > To enable OVS-DPDK jobs, set the build target according to different
> > CPU architectures 3. Temporarily disable sparse checker because of
> > static code checking failure on arm64
> >
> > Considering the balance of the CI coverage and running time, some
> > kernel and DPDK jobs are removed from Arm CI. The running time
> > increases around by 7 minutes to 47 minutes in all.
> >
> > Successful travis build jobs report:
> > https://travis-ci.org/github/yzyuestc/ovs/builds/663833478
> >
> > Reviewed-by: Yanqin Wei
> > Reviewed-by: Ruifeng Wang
> > Reviewed-by: JingZhao Ni
> > Reviewed-by: Gavin Hu
> > Signed-off-by: Lance Yang
> > ---
> > v3:
> >- Remove some kernel jobs: 4.18, 4.17, 4.16, 4.15, 4.14, and 4.3.
> >- Remove one OvS-DPDK shared library job.
> > ---
> > .travis.yml| 15 +++
> > .travis/linux-build.sh | 16 ++--
> > 2 files changed, 29 insertions(+), 2 deletions(-)
>
> This version seems OK. We can give it a try.
> Couple of minor comments inline.
>
> >
> > diff --git a/.travis.yml b/.travis.yml index ef9f867..1149758 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -51,6 +51,21 @@ matrix:
> > - os: osx
> >compiler: clang
> >env: OPTS="--disable-ssl"
> > +- arch: arm64
> > + compiler: gcc
> > + env: OPTS="--disable-ssl"
> > +- arch: arm64
> > + compiler: gcc
> > + env: KERNEL_LIST="5.5 4.19"
> > +- arch: arm64
> > + compiler: gcc
> > + env: KERNEL_LIST="4.9 3.16"
> > +- arch: arm64
> > + compiler: gcc
> > + env: DPDK=1 OPTS="--enable-shared"
> > +- arch: arm64
> > + compiler: gcc
> > + env: DPDK_SHARED=1
> >
> > script: ./.travis/${TRAVIS_OS_NAME}-build.sh $OPTS
> >
> > diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index
> > 359f777..3140ec9 100755
> > --- a/.travis/linux-build.sh
> > +++ b/.travis/linux-build.sh
> > @@ -6,7 +6,7 @@ set -x
> > CFLAGS_FOR_OVS="-g -O2"
> > SPARSE_FLAGS=""
> > EXTRA_OPTS="--enable-Werror"
> > -TARGET="x86_64-native-linuxapp-gcc"
> > +TARGET=""
>
> We could just remove this line. We're not using it anywhere beside
> install_dpdk().
>
> >
> > function install_kernel()
> > {
> > @@ -87,6 +87,16 @@ function install_dpdk()
> > local DPDK_VER=$1
> > local VERSION_FILE="dpdk-dir/travis-dpdk-cache-version"
> >
> > +if [ -z "$TRAVIS_ARCH" ] ||
> > + [ "$TRAVIS_ARCH" == "amd64" ]; then
> > +TARGET="x86_64-native-linuxapp-gcc"
> > +elif [ "$TRAVIS_ARCH" == "aarch64" ]; then
> > +TARGET="arm64-armv8a-linuxapp-gcc"
> > +else
> > +echo "Target is unknown"
> > +exit 1
> > +fi
> > +
> > if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
> > # Avoid using cache for git tree build.
> > rm -rf dpdk-dir
> > @@ -178,7 +188,9 @@ elif [ "$M32" ]; then
> > # difference on 'configure' and 'make' stages.
> > export CC="$CC -m32"
> > else
> > -OPTS="--enable-sparse"
> > +if [ "$TRAVIS_ARCH" != "aarch64" ]; then
> > +OPTS="--enable-sparse"
> > +fi
>
> This should be:
>
> elif [ "$TRAVIS_ARCH" != "aarch64" ]; then
> OPTS="--enable-sparse"
> if [ "$AFXDP" ]; then
> ...
>
> As we don't need to add SPARSE_FLAGS to CFLAGS_FOR_OVS if sparse is not
> enabled.
>
> Best regards, Ilya Maximets.
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4] travis: Enable OvS Travis CI for arm
Enable part of travis jobs with gcc compiler for arm64 architecture
1. Add arm jobs into the matrix in .travis.yml configuration file
2. To enable OVS-DPDK jobs, set the build target according to
different CPU architectures
3. Temporarily disable sparse checker because of static code checking
failure on arm64
Considering the balance of the CI coverage and running time, some kernel
and DPDK jobs are removed from Arm CI.
Successful travis build jobs report:
https://travis-ci.org/github/yzyuestc/ovs/builds/666129448
Reviewed-by: Yanqin Wei
Reviewed-by: Ruifeng Wang
Reviewed-by: JingZhao Ni
Reviewed-by: Gavin Hu
Signed-off-by: Lance Yang
---
v4:
- Move TARGET variable into install_dpdk function.
- Avoid to add SPARSE_FLAGS to CFLAGS_FOR_OVS when sparse is not
enabled.
---
v3:
- Remove some kernel jobs: 4.18, 4.17, 4.16, 4.15, 4.14, and 4.3.
- Remove one OvS-DPDK shared library job.
---
.travis.yml| 15 +++
.travis/linux-build.sh | 13 +++--
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index ef9f867..1149758 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -51,6 +51,21 @@ matrix:
- os: osx
compiler: clang
env: OPTS="--disable-ssl"
+- arch: arm64
+ compiler: gcc
+ env: OPTS="--disable-ssl"
+- arch: arm64
+ compiler: gcc
+ env: KERNEL_LIST="5.5 4.19"
+- arch: arm64
+ compiler: gcc
+ env: KERNEL_LIST="4.9 3.16"
+- arch: arm64
+ compiler: gcc
+ env: DPDK=1 OPTS="--enable-shared"
+- arch: arm64
+ compiler: gcc
+ env: DPDK_SHARED=1
script: ./.travis/${TRAVIS_OS_NAME}-build.sh $OPTS
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 359f777..02615a8 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -6,7 +6,6 @@ set -x
CFLAGS_FOR_OVS="-g -O2"
SPARSE_FLAGS=""
EXTRA_OPTS="--enable-Werror"
-TARGET="x86_64-native-linuxapp-gcc"
function install_kernel()
{
@@ -87,6 +86,16 @@ function install_dpdk()
local DPDK_VER=$1
local VERSION_FILE="dpdk-dir/travis-dpdk-cache-version"
+if [ -z "$TRAVIS_ARCH" ] ||
+ [ "$TRAVIS_ARCH" == "amd64" ]; then
+TARGET="x86_64-native-linuxapp-gcc"
+elif [ "$TRAVIS_ARCH" == "aarch64" ]; then
+TARGET="arm64-armv8a-linuxapp-gcc"
+else
+echo "Target is unknown"
+exit 1
+fi
+
if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
# Avoid using cache for git tree build.
rm -rf dpdk-dir
@@ -177,7 +186,7 @@ elif [ "$M32" ]; then
# Adding m32 flag directly to CC to avoid any posiible issues with API/ABI
# difference on 'configure' and 'make' stages.
export CC="$CC -m32"
-else
+elif [ "$TRAVIS_ARCH" != "aarch64" ]; then
OPTS="--enable-sparse"
if [ "$AFXDP" ]; then
# netdev-afxdp uses memset for 64M for umem initialization.
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
