[ovs-dev] [CudaMailTagged] [PATCH] geneve-map-rename: rename geneve-map to tlv-map.
This patch renames the command name related with geneve-map to a more generic
name as following:
add-geneve-map -> add-tlv-map
del-geneve-map -> del-tlv-map
dump-geneve-map -> dump-tlv-map
It also renames the Geneve_table to tlv_table.
By doing this renaming, the NSH variable context header(the same TLV format as
Geneve) or other protocol can reuse the field tun_metadata in the future.
Signed-off-by: Mengke Liu
Signed-off-by: Ricky Li
---
include/openflow/nicira-ext.h | 44 +-
lib/dpif-netdev.c | 2 +-
lib/geneve.h | 4 +--
lib/learning-switch.c | 6 ++--
lib/meta-flow.h | 2 +-
lib/ofp-errors.h | 8 ++---
lib/ofp-msgs.h| 16 +-
lib/ofp-parse.c | 8 ++---
lib/ofp-parse.h | 4 +--
lib/ofp-print.c | 36 +++---
lib/ofp-util.c| 72 +--
lib/ofp-util.h| 28 -
lib/rconn.c | 6 ++--
lib/tun-metadata.c| 12
lib/tun-metadata.h| 10 +++---
ofproto/ofproto.c | 26
ovn/controller/ofctrl.c | 52 +++
tests/tunnel-push-pop-ipv6.at | 2 +-
tests/tunnel-push-pop.at | 2 +-
tests/tunnel.at | 20 ++--
utilities/ovs-ofctl.8.in | 22 ++---
utilities/ovs-ofctl.c | 40
22 files changed, 211 insertions(+), 211 deletions(-)
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index efb40fa..2fd94b4 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -924,9 +924,9 @@ struct nx_flow_monitor_cancel {
};
OFP_ASSERT(sizeof(struct nx_flow_monitor_cancel) == 4);
-/* Geneve option table maintenance commands.
+/* TLV option table maintenance commands.
*
- * In order to work with Geneve options, we need to maintain a mapping
+ * In order to work with TLV options, we need to maintain a mapping
* table between an option (defined by ) and
* an NXM field that can be operated on for the purposes of matches,
* actions, etc. This mapping must be explicitly specified by the
@@ -940,8 +940,8 @@ OFP_ASSERT(sizeof(struct nx_flow_monitor_cancel) == 4);
* Note that mappings should not be changed while they are in active use by
* a flow. The result of doing so is undefined. */
-/* Geneve table commands */
-enum nx_geneve_table_mod_command {
+/* TLV table commands */
+enum nx_tlv_table_mod_command {
NXGTMC_ADD, /* New mappings (fails if an option is already
mapped). */
NXGTMC_DELETE, /* Delete mappings, identified by index
@@ -950,48 +950,48 @@ enum nx_geneve_table_mod_command {
in this command is ignored. */
};
-/* Map between a Geneve option and an NXM field. */
-struct nx_geneve_map {
-ovs_be16 option_class; /* Geneve option class. */
-uint8_t option_type; /* Geneve option type. */
-uint8_t option_len; /* Geneve option length (multiple of 4). */
+/* Map between a TLV option and an NXM field. */
+struct nx_tlv_map {
+ovs_be16 option_class; /* TLV class. */
+uint8_t option_type; /* TLV type. */
+uint8_t option_len; /* TLV length (multiple of 4). */
ovs_be16 index;/* NXM_NX_TUN_METADATA index */
uint8_t pad[2];
};
-OFP_ASSERT(sizeof(struct nx_geneve_map) == 8);
+OFP_ASSERT(sizeof(struct nx_tlv_map) == 8);
-/* NXT_GENEVE_TABLE_MOD.
+/* NXT_TLV_TABLE_MOD.
*
- * Use to configure a mapping between Geneve options (class, type, length)
+ * Use to configure a mapping between TLV options (class, type, length)
* and NXM fields (NXM_NX_TUN_METADATA where 'index' is ).
*
* This command is atomic: all operations on different options will
* either succeed or fail. */
-struct nx_geneve_table_mod {
+struct nx_tlv_table_mod {
ovs_be16 command; /* One of NTGTMC_* */
uint8_t pad[6];
-/* struct nx_geneve_map[0]; Array of maps between indicies and Geneve
+/* struct nx_tlv_map[0]; Array of maps between indicies and TLV
options. The number of elements is
inferred from the length field in the
header. */
};
-OFP_ASSERT(sizeof(struct nx_geneve_table_mod) == 8);
+OFP_ASSERT(sizeof(struct nx_tlv_table_mod) == 8);
-/* NXT_GENEVE_TABLE_REPLY.
+/* NXT_TLV_TABLE_REPLY.
*
- * Issued in reponse to an NXT_GENEVE_TABLE_REQUEST to give information
- * about the current status of the Geneve table in the switch. Provides
+ * Issued in reponse to an NXT_TLV_TABLE_REQUEST to give information
+ * about the current status of the TLV table in the switch. Provides
* both static information about the switch's capabilities as well as
- * the configured Geneve option table. */
-struct nx_geneve_table
Re: [ovs-dev] [PATCH 0/7] Enable NSH based Service Function Chaining support in OVS
> -Original Message-
> From: Jesse Gross [mailto:je...@kernel.org]
> Sent: Saturday, December 12, 2015 1:41 AM
> To: Liu, Mengke
> Cc: dev@openvswitch.org; Pritesh Kothari (pritkoth) ;
> Zhou, Danny ; Li, Ricky ;
> pa...@cisco.com
> Subject: Re: [ovs-dev] [PATCH 0/7] Enable NSH based Service Function
> Chaining support in OVS
>
> On Thu, Dec 10, 2015 at 6:55 PM, Liu, Mengke
> wrote:
> >> -Original Message-
> >> From: jgr...@nicira.com [mailto:jgr...@nicira.com] On Behalf Of Jesse
> >> Gross
> >> Sent: Tuesday, December 8, 2015 12:32 AM
> >> To: Liu, Mengke
> >> Cc: dev@openvswitch.org; Pritesh Kothari (pritkoth)
> >> ; Zhou, Danny ; Li, Ricky
> >> ; pa...@cisco.com
> >> Subject: Re: [ovs-dev] [PATCH 0/7] Enable NSH based Service Function
> >> Chaining support in OVS
> >>
> >> On Fri, Nov 6, 2015 at 4:23 PM, Jesse Gross wrote:
> >> > On Tue, Nov 3, 2015 at 10:20 AM, Liu, Mengke
> >> wrote:
> >> >>> > So for NSH header, we need to add the TLV support. For the
> >> >>> > fixed fields of NSH header like NSI, NSP, we’d like to add
> >> >>> > specific meta-flow fields for them, for example:
> >> >>> >
> >> >>> > MFF_NSPType: NXM_NX_NSP (105), Length:4 bytes
> >> >>> >
> >> >>> > MFF_NSI Type: NXM_NX_NSI (106), Length:1 bytes
> >> >>> >
> >> >>> > But for the variable length context headers of NSH, we’d like
> >> >>> > to use fields like “tun_metadata” (tnl->metadata) to support
> >> >>> > it. We also have two
> >> >>> options:
> >> >>> >
> >> >>> > 1) Reuse the “tun_metadata” for NSH variable context header,
> it’s
> >> >>> > similar to current Geneve TLV support. But it’s a little wield
> >> >>> > because the NSH header is already an independent protocol layer
> >> >>> > but not belong to the tunnel layer.
> >> >>> >
> >> >>> > 2) Define a new “nsh_metadata” fields for NSH variable context
> >> header.
> >> >>> >
> >> >>> > Which one do you prefer? Please tell us for you inputs on our
> >> >>> > modification plan.
> >> >>>
> >> >>> I would definitely like to reuse the same set of fields as were
> >> >>> used for Geneve since there are a large number of them and have a
> >> >>> second set seems wasteful. I don't think there is anything that
> >> >>> inherently ties them to tunneling, so if you have a different
> >> >>> name that is more generic we can still change them as long as it
> >> >>> is before OVS 2.5 is
> >> released.
> >> >>>
> >> >>> By the way, there are several OpenFlow commands that were added
> >> >>> support mapping TLVs to fields. These are currently specific to
> >> >>> Geneve because they validate some protocol specific aspects. NSH
> >> >>> actually uses the same TLV format as Geneve, so in theory they
> >> >>> could be shared (and it would be nice to avoid duplicating
> >> >>> these). The main thing that concerns me about this is the
> >> >>> possibility that the protocols will diverge in the future or some
> >> >>> other protocol that does not have the same format will want to
> >> >>> use the same thing. In any case, it would be nice to think about
> >> >>> how this could be made useable
> >> by everybody before OVS 2.5.
> >> >>
> >> >> For NSH TLV implementation, We agree that we should reuse the
> >> >> same
> >> set of fields as were used for Geneve. As NSH in MD-Type 2 use same
> >> TLV format as Geneve, they can share the pool of 64 NXMs which can be
> >> mapped on Geneve TLVs or NSH TLVs. It may be better to make the
> >> command name more generic. For example, we can rename
> >> “add-geneve-map” to “add-tlv- map”.
> >> >> An example in our initial design for NSH MD-type 2 support:
> >> >> ovs-ofctl add-tlv-map br0
> >> >> {class=0x,type=0,len=4}->tun_metadata0
> >> >> ovs-ofctl add-flow br0 in_port=LOCAL, actions=push_nsh,
> >> >> set_field:221->nsp, set_field:3->nsi, set_field:2-
> >> >nsh_md_type,set_field:111->tun_metadata0, 1 What do you think
> about
> >> this proposal for NSH TLV support?
> >> >
> >> > I think it's basically fine although I worry a bit that "add-tlv-map"
> >> > isn't overly descriptive - it isn't obvious that it is referring to
> >> > this type of metadata or there could be TLVs in other formats.
> >>
> >> I just wanted to point out that OVS 2.5 has branched at this point
> >> and contains the current (Geneve-centric) names of these
> commands/fields.
> >> If you want to change them, I would still apply a patch to help with
> >> forward compatibility. However, the window of time to do that is
> >> rapidly closing, so speak now or forever hold your peace.
> >
> > We strongly agree that the command "add-geneve-map" should be
> renamed to "add-tlv-map" or the other generic name for NSH TLV support
> and other protocol in future. We plan to add NSH MD-Type 2 support in the
> first quarter of 2016.
> >
> > About this patch for renaming the command name, if you are busy, we are
> pleased to submit it for review. Thanks.
>
> Please submit a patch to do the renaming ASAP so that it can b
[ovs-dev] Returned mail: see transcript for details
The original message was received at Mon, 14 Dec 2015 11:47:57 +0300 from openvswitch.org [86.233.57.199] - The following addresses had permanent fatal errors - - Transcript of the session follows - ... while talking to 57.122.35.213: >>> RCPT To: <<< 550 MAILBOX NOT FOUND ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] jsonrpc-server: Use prototype style for ovsdb_jsonrpc_disable_monitor2().
Without "void", this is a pre-ANSI style function definition that has
subtly different semantics.
Found by sparse.
Signed-off-by: Ben Pfaff
---
ovsdb/jsonrpc-server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 1dcd394..2065702 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -1374,7 +1374,7 @@ ovsdb_jsonrpc_monitor_flush_all(struct
ovsdb_jsonrpc_session *s)
}
void
-ovsdb_jsonrpc_disable_monitor2()
+ovsdb_jsonrpc_disable_monitor2(void)
{
/* Once disabled, it is not possible to re-enable it. */
monitor2_enable__ = false;
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] match: Add support for matching IGMP fields.
On Thu, Dec 10, 2015 at 01:42:41PM -0800, Jarno Rajahalme wrote: > Complete the IGMP protocol support by making IGMP fields (type, code, > and group) matchable via OpenFlow by the way of new Nicira extensions. > > The new fields are: 8-bit NXM_NX_IGMP_TYPE (111), 8-bit > NXM_NX_IGMP_CODE (112), and 32-bit NXM_NX_IGMP_GROUP (113). > > VMware-BZ: #1558992 > Signed-off-by: Jarno Rajahalme Is this something we're targeting to backport to OVS 2.5? If not, then the meta-flow.h headers should say "since v2.6" instead of "since v2.5". This needs to add an item to NEWS and documentation to ovs-ofctl(8). What's here looks good, and I trust you to do a good job on the above, so: Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [ovs-discuss] [announce] driverctl: utility for persistent alternative driver binding
> -Original Message- > From: Panu Matilainen [mailto:pmati...@redhat.com] > Sent: Tuesday, December 8, 2015 12:05 PM > To: Gray, Mark D; d...@dpdk.org; us...@dpdk.org; dev@openvswitch.org; > disc...@openvswitch.org > Subject: Re: [ovs-discuss] [announce] driverctl: utility for persistent > alternative driver binding > > On 12/04/2015 05:44 PM, Gray, Mark D wrote: > > I welcome this initiative, one question below: > > > >> -Original Message- > >> From: discuss [mailto:discuss-boun...@openvswitch.org] On Behalf Of > >> Panu Matilainen > >> Sent: Friday, December 4, 2015 10:54 AM > >> To: d...@dpdk.org; us...@dpdk.org; dev@openvswitch.org; > >> disc...@openvswitch.org > >> Subject: [ovs-discuss] [announce] driverctl: utility for persistent > >> alternative driver binding > >> > >> Hi all, > >> > >> While this is not directly related to DPDK or OVS, it is potentially > >> useful for users of both, so excuse me for cross-posting. > >> > >> Quoting from the project README (for the full text see > >> http://laiskiainen.org/git/?p=driverctl.git;a=blob_plain;f=README) > >> > >> > driverctl is a tool for manipulating and inspecting the system > >> > device driver choices. > >> > > >> > Devices are normally assigned to their sole designated kernel driver > >> > by default. However in some situations it may be desireable to > >> > override that default, for example to try an older driver to > >> > work around a regression in a driver or to try an experimental > >> > alternative driver. Another common use-case is pass-through > >> > drivers and driver stubs to allow userspace to drive the device, > >> > such as in case of virtualization. > >> > > >> > driverctl integrates with udev to support overriding > >> > driver selection for both cold- and hotplugged devices from the > >> > moment of discovery, but can also change already assigned drivers, > >> > assuming they are not in use by the system. The driver overrides > >> > created by driverctl are persistent across system reboots > >> > by default. > >> > > >> > Usage > >> > - > >> > > >> > Find devices currently driven by ixgbe driver: > >> > > >> > # driverctl -v list-devices | grep ixgbe > >> > :01:00.0 ixgbe (Ethernet 10G 4P X520/I350 rNDC) > >> > :01:00.1 ixgbe (Ethernet 10G 4P X520/I350 rNDC) > >> > > >> > Change them to use the vfio-pci driver: > >> > # driverctl set-override :01:00.0 vfio-pci > >> > # driverctl set-override :01:00.1 vfio-pci > >> > > >> > Find devices with driver overrides: > >> > # driverctl -v list-devices|grep \\* > >> > :01:00.0 vfio-pci [*] (Ethernet 10G 4P X520/I350 rNDC) > >> > :01:00.1 vfio-pci [*] (Ethernet 10G 4P X520/I350 rNDC) > >> > > >> > Remove the override from slot :01:00.1: > >> > # driverctl unset-override :01:00.1 > >> > >> DPDK of course has its own dpdk_nic_bind(.py) tool for this purpose, > >> the main differences to driverctl are: > >> - driverctl bindings are persistent across system boots > > > > [Gray, Mark D] This is great! > > > > Will this integrate with, for example in Red Hat-based systems, > > /etc/sysconfig/network-scripts/ifcfg-X? In DPDK, could we then potentially > reference devices by that (arbitrary) name? > > driverctl is not specific to NICs so network-scripts integration is out of > scope. > I forgot that point. It makes sense. > That aside, maybe I'm missing something but I'm not sure what there is to > integrate with since DPDK ports are ultimately application specific. > For OVS I've sent a patch to support managing OVS DPDK ports via > network-scripts: > http://openvswitch.org/pipermail/dev/2015-December/062850.html Thanks, I wasn’t familiar with this work but this is a good approach. > > . Panu - > What I am thinking would be an ideal end state for OVS would be the possibility to add a port using ovs-vsctl add-port referencing the device name ovs-vsctl add-port p1p2 If dpdk was enabled, then OVS could query something(?) to determine which physical interface this name actually referred to and then from that figure out what dpdk port number it referred to. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [monitor2 v2 7/9] ovsdb-client: support monitor2
On Mon, Dec 14, 2015 at 08:55:01AM +0200, Liran Schour wrote:
> Andy Zhou wrote on 12/12/2015 12:37:32 AM:
>
> > On Tue, Dec 1, 2015 at 6:20 AM, Liran Schour wrote:
> > "dev" wrote on 25/11/2015 12:16:01 AM:
> >
> > > From: Andy Zhou
> > > @@ -617,6 +621,101 @@ monitor_print(struct json *table_updates,
> > > }
> > >
> > > static void
> > > +monitor2_print_row(struct json *row, const char *type, const char
> > *uuid,
> > > + const struct ovsdb_column_set *columns, struct
> table
> > *t)
> > > +{
> > > +if (!strcmp(type, "delete")) {
> > > +if (row->type != JSON_NULL) {
> > > +ovs_error(0, "delete method does not expect ");
> > > +return;
> > > +}
> > > +
> > > +table_add_row(t);
> > > +table_add_cell(t)->text = xstrdup(uuid);
> > > +table_add_cell(t)->text = xstrdup(type);
> > > +} else {
> > > +if (!row || row->type != JSON_OBJECT) {
> > > +ovs_error(0, " is not object");
> > > +return;
> > > +}
> > > +monitor_print_row(row, type, uuid, columns, t);
> > > +}
> > > +}
> >
> > Update2 does not send default values, therefore we will miss these
> values
> > on ovsdb-client.
> > I can fix it on monitor_cond or you can send an updated patch.
> >
> > What changes do you have in mind?
>
> > The goal of "monitor" command is to show what's being transmitted
> > per ovsdb remote protocol. Since
> > the default values are not being transmitted, I'd think it is O.K.
> > not to show them. No?
> >
>
> OK. I thought the user expects to see the real outcome of the updates. If
> not and "monitor" command should show only what is being transmitted it is
> OK like it is now.
I guess that the purpose could be argued either way, but I think it's
reasonable to have it work the way Andy did it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath-windows: Cleanup unused variables
On Fri, Dec 11, 2015 at 05:32:41PM +, Alin Serdean wrote: > This patch removes unused variables defined in stt and vxlan ports. > > Signed-off-by: Alin Gabriel Serdean Applied to master, thank you! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Fix broken sed
On Thu, Dec 10, 2015 at 10:18:51PM +, Alin Serdean wrote: > Patch 43000bc introduced a portability improvement. > > This patch adds the command for $SED 's' and also changes to x86 for 32 bit > instead of x64. > > Signed-off-by: Alin Gabriel Serdean This is partly my fault because I reviewed that commit without seeing the bugs it introduced. My apologies. I applied this to master and branch-2.5. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] AUTHORS: Add Carlo Andreotti
On Fri, Dec 11, 2015 at 01:59:00PM +0100, Daniele Venturino wrote: > Carlo was involved in the testing and validation processes of the Rapid > Spanning Tree Implementation. > > I also updated the Copyright string in some files. > > Signed-off by: Daniele Venturino Thanks, I applied this to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [CudaMailTagged] [PATCH] geneve-map-rename: rename geneve-map to tlv-map.
On Tue, Dec 15, 2015 at 12:52:26AM +0800, Mengke Liu wrote: > This patch renames the command name related with geneve-map to a more generic > name as following: > add-geneve-map -> add-tlv-map > del-geneve-map -> del-tlv-map > dump-geneve-map -> dump-tlv-map > > It also renames the Geneve_table to tlv_table. > > By doing this renaming, the NSH variable context header(the same TLV format as > Geneve) or other protocol can reuse the field tun_metadata in the future. > > Signed-off-by: Mengke Liu > Signed-off-by: Ricky Li Jesse, I'm going to assume you'll review this? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] missing Hyper-V and OVN meetings this week
On 12/14/2015 12:21 AM, Ben Pfaff wrote: > I'm going to have to miss the Hyper-V and OVN meetings in #openvswitch > this week. I'm in Pune, India this week, and they're at inconvenient > times for the time zone there. Have a good trip! > I should be able to make the Hyper-V meeting next week, after I'm back. > (The OVN meeting that week is on Christmas Eve; we might want to cancel > it.) Yes, let's cancel the one next week. The week after is New Year's Eve, so I suggest we postpone OVN meetings until Thursday, January 7. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net] openvswitch: fix trivial comment typo
The commit 33db4125ec74 ("openvswitch: Rename LABEL->LABELS") left
over an old OVS_CT_ATTR_LABEL instance, fix it.
Fixes: 33db4125ec74 ("openvswitch: Rename LABEL->LABELS")
Signed-off-by: Paolo Abeni
---
include/uapi/linux/openvswitch.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 28ccedd..a27222d 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -628,7 +628,7 @@ struct ovs_action_hash {
* @OVS_CT_ATTR_MARK: u32 value followed by u32 mask. For each bit set in the
* mask, the corresponding bit in the value is copied to the connection
* tracking mark field in the connection.
- * @OVS_CT_ATTR_LABEL: %OVS_CT_LABELS_LEN value followed by %OVS_CT_LABELS_LEN
+ * @OVS_CT_ATTR_LABELS: %OVS_CT_LABELS_LEN value followed by %OVS_CT_LABELS_LEN
* mask. For each bit set in the mask, the corresponding bit in the value is
* copied to the connection tracking label field in the connection.
* @OVS_CT_ATTR_HELPER: variable length string defining conntrack ALG.
--
1.8.3.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v7] dpif-netdev: proper tx queue id
Currently tx_qid is equal to pmd->core_id. This leads to unexpected
behavior if pmd-cpu-mask different from '/(0*)(1|3|7)?(f*)/',
e.g. if core_ids are not sequential, or doesn't start from 0, or both.
Example:
starting 2 pmd threads with 1 port, 2 rxqs per port,
pmd-cpu-mask = 0014 and let dev->real_n_txq = 2
It that case pmd_1->tx_qid = 2, pmd_2->tx_qid = 4 and
txq_needs_locking = true (if device hasn't ovs_numa_get_n_cores()+1
queues).
In that case, after truncating in netdev_dpdk_send__():
'qid = qid % dev->real_n_txq;'
pmd_1: qid = 2 % 2 = 0
pmd_2: qid = 4 % 2 = 0
So, both threads will call dpdk_queue_pkts() with same qid = 0.
This is unexpected behavior if there is 2 tx queues in device.
Queue #1 will not be used and both threads will lock queue #0
on each send.
Fix that by introducing per pmd thread hash map 'tx_queues', where will
be stored all available tx queues for that pmd thread with
port_no as a key(hash). All tx_qid-s will be unique per port and
sequential to prevent described unexpected mapping after truncating.
Implemented infrastructure can be used in the future to choose
between all tx queues available for that pmd thread.
Signed-off-by: Ilya Maximets
---
version 7:
* rebased on current master
* My comment for v6 that there is a race for total number of
threads was wrong. So, patch actually the same as v6.
version 6:
* fixed assigning same txq to multiple pmd threads on different
NUMA sockets.
* Changed logic of txq distribution in pmd_load_queues().
version 5:
* txqs 0 from ports of non-pmd netdevs added to all pmd threads
version 4:
* fixed distribution of tx queues if multiqueue is not supported
version 3:
* fixed failing of unit tests by adding tx queues of non
pmd devices to non pmd thread. (they haven't been used by any thread)
* pmd_flush_tx_queues --> dp_netdev_pmd_detach_tx_queues
* function names changed to dp_netdev_*
* dp_netdev_pmd_lookup_txq now looks by port_no.
* removed unnecessary dp_netdev_lookup_port in dp_execute_cb
for OVS_ACTION_ATTR_OUTPUT.
* refactoring
lib/dpif-netdev.c | 189 ++
1 file changed, 163 insertions(+), 26 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8794ca0..1586852 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -372,6 +372,13 @@ struct dp_netdev_pmd_cycles {
atomic_ullong n[PMD_N_CYCLES];
};
+struct dp_netdev_pmd_txq {
+struct cmap_node node;/* In owning dp_netdev_pmd_thread's */
+ /* 'tx_queues'. */
+struct dp_netdev_port *port;
+int tx_qid;
+};
+
/* PMD: Poll modes drivers. PMD accesses devices via polling to eliminate
* the performance overhead of interrupt processing. Therefore netdev can
* not implement rx-wait for these devices. dpif-netdev needs to poll
@@ -425,10 +432,12 @@ struct dp_netdev_pmd_thread {
pthread_t thread;
int index; /* Idx of this pmd thread among pmd*/
/* threads on same numa node. */
+int global_index; /* Idx of this pmd thread among all*/
+/* pmd threads in dp. */
unsigned core_id; /* CPU core id of this pmd thread. */
int numa_id;/* numa node id of this pmd thread. */
-int tx_qid; /* Queue id used by this pmd thread to
- * send packets on all netdevs */
+struct cmap tx_queues; /* Queue ids used by this pmd thread to
+ * send packets to ports */
/* Only a pmd thread can write on its own 'cycles' and 'stats'.
* The main thread keeps 'stats_zero' and 'cycles_zero' as base
@@ -470,6 +479,15 @@ static void dp_netdev_input(struct dp_netdev_pmd_thread *,
static void dp_netdev_disable_upcall(struct dp_netdev *);
void dp_netdev_pmd_reload_done(struct dp_netdev_pmd_thread *pmd);
+static void dp_netdev_configure_non_pmd_txqs(struct dp_netdev_pmd_thread *pmd);
+static void dp_netdev_pmd_add_txq(struct dp_netdev_pmd_thread *pmd,
+ struct dp_netdev_port *port, int queue_id);
+static void dp_netdev_pmd_del_txq(struct dp_netdev_pmd_thread *pmd,
+ struct dp_netdev_pmd_txq *txq);
+static void dp_netdev_pmd_detach_tx_queues(struct dp_netdev_pmd_thread *pmd);
+static struct dp_netdev_pmd_txq *
+dp_netdev_pmd_lookup_txq(const struct dp_netdev_pmd_thread *pmd,
+ odp_port_t port_no);
static void dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd,
struct dp_netdev *dp, int index,
Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
Just some more activity on this topic, after spending time going through
the code a bit.
Zoltan Kiss writes:
> On 08/12/15 17:31, Gray, Mark D wrote:
>>
>>
>>> -Original Message-
>>> From: Ben Pfaff [mailto:b...@ovn.org]
>>> Sent: Tuesday, December 8, 2015 4:50 PM
>>> To: Gray, Mark D
>>> Cc: Traynor, Kevin; dev@openvswitch.org
>>> Subject: Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
>>>
>>> On Tue, Dec 08, 2015 at 04:41:37PM +, Gray, Mark D wrote:
> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Kevin
> Traynor
> Sent: Monday, December 7, 2015 5:58 PM
> To: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
>
> Add some information about the DPDK -c and -n parameters.
>
> Signed-off-by: Kevin Traynor
> Reported-by: Zoltan Kiss
> ---
> INSTALL.DPDK.md | 14 --
> 1 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index
> 96b686c..ee016da
> 100644
> --- a/INSTALL.DPDK.md
> +++ b/INSTALL.DPDK.md
> @@ -145,8 +145,18 @@ Using the DPDK with ovs-vswitchd:
>
> DPDK configuration arguments can be passed to vswitchd via `--dpdk`
> argument. This needs to be first argument passed to vswitchd process.
> - dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
> - for dpdk initialization.
> + The DPDK configuration arguments are passed to DPDK during DPDK
> + initialization.
> +
> + The DPDK -c coremask is a required argument. To avoid wasted
>>> resources
> + only one core should be set. The standard OVS threads (e.g. main
> + process, handler, revalidator) will run on the core that is specified.
Might be worth mentioning that then there is a corresponding potential
decrease in performance of revalidation and flow handling.
>>>
>>> With the kernel datapath, OVS sets up flows and revalidates them on
>>> multiple cores. You're saying that with DPDK it only uses one core?
>>> Why?
>>
>> In my opinion, the core mask should define the affinities of the other
>> threads (main, process, handler, revalidator) and the pmd-cpu-mask
>
> That would sound good, but I'm afraid you can't really influence how
> DPDK interprets the -c parameter. Maybe you can delegate the tasks of
> the non-pmd threads to the lcore threads (via
> rte_eal_[mp_]remote_launch() functions), but I'm not sure if its
> viable.
I'm not sure how good that sounds, really. From a reading of the code,
the coremask here is used _solely_ for the EAL 'lcore' thread(s) which
process EAL 'rpc' messages. There'd be a lot of intrusive conversion to
integrate with DPDK in that space.
I think it may be a good idea to rename it here from a generic
"coremask," but as you indicate, we can't impose any additional
semantics without getting everyone really confused.
>> should define the affinities of the packet processing threads. However,
>> I don't know if this was the intended behavior because the name is a little
>> too generic ("core mask"). If this was the intended behavior, Kevin and I
>> just did some tests, and it is not behaving like this.
I think the behavior intention was to leave it as 'dpdk lcore only.'
Maybe there's a better way of doing it that I don't see, though;
otherwise you've kindof overloaded the dpdk argument with ovs argument
as well. Sorry if that doesn't make sense.
-Aaron
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
Hi,
On 14/12/15 16:02, Aaron Conole wrote:
Just some more activity on this topic, after spending time going through
the code a bit.
Zoltan Kiss writes:
On 08/12/15 17:31, Gray, Mark D wrote:
-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org]
Sent: Tuesday, December 8, 2015 4:50 PM
To: Gray, Mark D
Cc: Traynor, Kevin; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
On Tue, Dec 08, 2015 at 04:41:37PM +, Gray, Mark D wrote:
-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Kevin
Traynor
Sent: Monday, December 7, 2015 5:58 PM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
Add some information about the DPDK -c and -n parameters.
Signed-off-by: Kevin Traynor
Reported-by: Zoltan Kiss
---
INSTALL.DPDK.md | 14 --
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index
96b686c..ee016da
100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -145,8 +145,18 @@ Using the DPDK with ovs-vswitchd:
DPDK configuration arguments can be passed to vswitchd via `--dpdk`
argument. This needs to be first argument passed to vswitchd process.
- dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
- for dpdk initialization.
+ The DPDK configuration arguments are passed to DPDK during DPDK
+ initialization.
+
+ The DPDK -c coremask is a required argument. To avoid wasted
resources
+ only one core should be set. The standard OVS threads (e.g. main
+ process, handler, revalidator) will run on the core that is specified.
Might be worth mentioning that then there is a corresponding potential
decrease in performance of revalidation and flow handling.
With the kernel datapath, OVS sets up flows and revalidates them on
multiple cores. You're saying that with DPDK it only uses one core?
Why?
In my opinion, the core mask should define the affinities of the other
threads (main, process, handler, revalidator) and the pmd-cpu-mask
That would sound good, but I'm afraid you can't really influence how
DPDK interprets the -c parameter. Maybe you can delegate the tasks of
the non-pmd threads to the lcore threads (via
rte_eal_[mp_]remote_launch() functions), but I'm not sure if its
viable.
I'm not sure how good that sounds, really. From a reading of the code,
the coremask here is used _solely_ for the EAL 'lcore' thread(s) which
process EAL 'rpc' messages. There'd be a lot of intrusive conversion to
integrate with DPDK in that space.
I think the current way of handling this almost OK:
- OVS creates the threads by itself, based on pmd-cpu-mask
- the DPDK code makes sure rte_lcore_id() is set in
pmd_thread_setaffinity_cpu(). So other DPDK libraries will see these OVS
PMD threads as perfectly fine DPDK lcore threads
- the only thing I would change is how the "-c coremask" parameter is
handled. Currently the user passes is, but it really should have one bit
set in there, for the master DPDK thread. On every other core in that
list DPDK will create an lcore thread, which won't do any work at all.
It's not a big issue, but they still waste some resources.
I think it may be a good idea to rename it here from a generic
"coremask," but as you indicate, we can't impose any additional
semantics without getting everyone really confused.
should define the affinities of the packet processing threads. However,
I don't know if this was the intended behavior because the name is a little
too generic ("core mask"). If this was the intended behavior, Kevin and I
just did some tests, and it is not behaving like this.
I think the behavior intention was to leave it as 'dpdk lcore only.'
Maybe there's a better way of doing it that I don't see, though;
otherwise you've kindof overloaded the dpdk argument with ovs argument
as well. Sorry if that doesn't make sense.
-Aaron
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Reference Number #38235141, Last Payment Notice
Dear Customer, We regret to inform you that due to your unpaid debt amount of $745.47 to SandorInc., from November 31, 2015 we have passed your case to the court. Your prompt attention is required to resolve this issue. Attached you can findyour invoice and case information to review. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Zoltan Kiss
> Sent: Monday, December 14, 2015 5:21 PM
> To: Aaron Conole
> Cc: dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
>
> Hi,
>
> On 14/12/15 16:02, Aaron Conole wrote:
> > Just some more activity on this topic, after spending time going through
> > the code a bit.
> >
> > Zoltan Kiss writes:
> >> On 08/12/15 17:31, Gray, Mark D wrote:
> >>>
> >>>
> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, December 8, 2015 4:50 PM
> To: Gray, Mark D
> Cc: Traynor, Kevin; dev@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
>
> On Tue, Dec 08, 2015 at 04:41:37PM +, Gray, Mark D wrote:
> >> -Original Message-
> >> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Kevin
> >> Traynor
> >> Sent: Monday, December 7, 2015 5:58 PM
> >> To: dev@openvswitch.org
> >> Subject: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
> >>
> >> Add some information about the DPDK -c and -n parameters.
> >>
> >> Signed-off-by: Kevin Traynor
> >> Reported-by: Zoltan Kiss
> >> ---
> >>INSTALL.DPDK.md | 14 --
> >>1 files changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index
> >> 96b686c..ee016da
> >> 100644
> >> --- a/INSTALL.DPDK.md
> >> +++ b/INSTALL.DPDK.md
> >> @@ -145,8 +145,18 @@ Using the DPDK with ovs-vswitchd:
> >>
> >> DPDK configuration arguments can be passed to vswitchd via `--
> dpdk`
> >> argument. This needs to be first argument passed to vswitchd
> process.
> >> - dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
> >> - for dpdk initialization.
> >> + The DPDK configuration arguments are passed to DPDK during DPDK
> >> + initialization.
> >> +
> >> + The DPDK -c coremask is a required argument. To avoid wasted
> resources
> >> + only one core should be set. The standard OVS threads (e.g. main
> >> + process, handler, revalidator) will run on the core that is
> specified.
> >
> > Might be worth mentioning that then there is a corresponding potential
> > decrease in performance of revalidation and flow handling.
>
> With the kernel datapath, OVS sets up flows and revalidates them on
> multiple cores. You're saying that with DPDK it only uses one core?
> Why?
> >>>
> >>> In my opinion, the core mask should define the affinities of the other
> >>> threads (main, process, handler, revalidator) and the pmd-cpu-mask
> >>
> >> That would sound good, but I'm afraid you can't really influence how
> >> DPDK interprets the -c parameter. Maybe you can delegate the tasks of
> >> the non-pmd threads to the lcore threads (via
> >> rte_eal_[mp_]remote_launch() functions), but I'm not sure if its
> >> viable.
> >
> > I'm not sure how good that sounds, really. From a reading of the code,
> > the coremask here is used _solely_ for the EAL 'lcore' thread(s) which
> > process EAL 'rpc' messages. There'd be a lot of intrusive conversion to
> > integrate with DPDK in that space.
>
> I think the current way of handling this almost OK:
>
> - OVS creates the threads by itself, based on pmd-cpu-mask
> - the DPDK code makes sure rte_lcore_id() is set in
> pmd_thread_setaffinity_cpu(). So other DPDK libraries will see these OVS
> PMD threads as perfectly fine DPDK lcore threads
> - the only thing I would change is how the "-c coremask" parameter is
> handled. Currently the user passes is, but it really should have one bit
> set in there, for the master DPDK thread. On every other core in that
> list DPDK will create an lcore thread, which won't do any work at all.
> It's not a big issue, but they still waste some resources.
>
>
> >
> > I think it may be a good idea to rename it here from a generic
> > "coremask," but as you indicate, we can't impose any additional
> > semantics without getting everyone really confused.
> >
> >>> should define the affinities of the packet processing threads. However,
> >>> I don't know if this was the intended behavior because the name is a
> little
> >>> too generic ("core mask"). If this was the intended behavior, Kevin and
> I
> >>> just did some tests, and it is not behaving like this.
> >
> > I think the behavior intention was to leave it as 'dpdk lcore only.'
> > Maybe there's a better way of doing it that I don't see, though;
> > otherwise you've kindof overloaded the dpdk argument with ovs argument
> > as well. Sorry if that doesn't make sense.
> >
> > -Aaron
How about letting the control threads just float on the non-isolcpu'd cores.
We could then potentially remove the -c argument which would simplify
setup as the user would only nee
Re: [ovs-dev] [PATCH net] openvswitch: fix trivial comment typo
On 14 December 2015 at 05:29, Paolo Abeni wrote:
> The commit 33db4125ec74 ("openvswitch: Rename LABEL->LABELS") left
> over an old OVS_CT_ATTR_LABEL instance, fix it.
>
> Fixes: 33db4125ec74 ("openvswitch: Rename LABEL->LABELS")
> Signed-off-by: Paolo Abeni
Thanks for the fix.
Acked-by: Joe Stringer
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Use constants for conntrack state bits.
On 12/11/2015 04:29 PM, Joe Stringer wrote:
> On 10 December 2015 at 09:12, Russell Bryant wrote:
>> A previous commit fixed this code to match changes to the conntrack
>> state bit assignments. This patch further updates the code to use
>> the defined constants to ensure this code adapts automatically to any
>> possible future changes.
>>
>> Signed-off-by: Russell Bryant
>> Requested-by: Joe Stringer
>
> Thanks, minor comment below.
>
>> lib/packets.h | 25 +
>> ovn/controller/lflow.c | 22 +++---
>> 2 files changed, 32 insertions(+), 15 deletions(-)
>>
>> diff --git a/lib/packets.h b/lib/packets.h
>> index edf140b..c50e316 100644
>> --- a/lib/packets.h
>> +++ b/lib/packets.h
>> @@ -733,14 +733,23 @@ struct tcp_header {
>> BUILD_ASSERT_DECL(TCP_HEADER_LEN == sizeof(struct tcp_header));
>>
>> /* Connection states */
>> -#define CS_NEW 0x01
>> -#define CS_ESTABLISHED 0x02
>> -#define CS_RELATED 0x04
>> -#define CS_REPLY_DIR 0x08
>> -#define CS_INVALID 0x10
>> -#define CS_TRACKED 0x20
>> -#define CS_SRC_NAT 0x40
>> -#define CS_DST_NAT 0x80
>> +#define CS_NEW_BIT 0
>> +#define CS_ESTABLISHED_BIT 1
>> +#define CS_RELATED_BIT 2
>> +#define CS_REPLY_DIR_BIT 3
>> +#define CS_INVALID_BIT 4
>> +#define CS_TRACKED_BIT 5
>> +#define CS_SRC_NAT_BIT 6
>> +#define CS_DST_NAT_BIT 7
>
> Would these be appropriate to be an enum instead?
Sure, I'll post a v2 in a minute. Here's the incremetal diff:
> diff --git a/lib/packets.h b/lib/packets.h
> index c50e316..36bd759 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -733,23 +733,27 @@ struct tcp_header {
> BUILD_ASSERT_DECL(TCP_HEADER_LEN == sizeof(struct tcp_header));
>
> /* Connection states */
> -#define CS_NEW_BIT 0
> -#define CS_ESTABLISHED_BIT 1
> -#define CS_RELATED_BIT 2
> -#define CS_REPLY_DIR_BIT 3
> -#define CS_INVALID_BIT 4
> -#define CS_TRACKED_BIT 5
> -#define CS_SRC_NAT_BIT 6
> -#define CS_DST_NAT_BIT 7
> -
> -#define CS_NEW (1 << CS_NEW_BIT)
> -#define CS_ESTABLISHED (1 << CS_ESTABLISHED_BIT)
> -#define CS_RELATED (1 << CS_RELATED_BIT)
> -#define CS_REPLY_DIR (1 << CS_REPLY_DIR_BIT)
> -#define CS_INVALID (1 << CS_INVALID_BIT)
> -#define CS_TRACKED (1 << CS_TRACKED_BIT)
> -#define CS_SRC_NAT (1 << CS_SRC_NAT_BIT)
> -#define CS_DST_NAT (1 << CS_DST_NAT_BIT)
> +enum {
> +CS_NEW_BIT = 0,
> +CS_ESTABLISHED_BIT = 1,
> +CS_RELATED_BIT = 2,
> +CS_REPLY_DIR_BIT = 3,
> +CS_INVALID_BIT = 4,
> +CS_TRACKED_BIT = 5,
> +CS_SRC_NAT_BIT = 6,
> +CS_DST_NAT_BIT = 7,
> +};
I know the integers aren't necessary above, but I kind of like
specifying them anyway since the specific values are important.
> +
> +enum {
> +CS_NEW = (1 << CS_NEW_BIT),
> +CS_ESTABLISHED = (1 << CS_ESTABLISHED_BIT),
> +CS_RELATED = (1 << CS_RELATED_BIT),
> +CS_REPLY_DIR = (1 << CS_REPLY_DIR_BIT),
> +CS_INVALID = (1 << CS_INVALID_BIT),
> +CS_TRACKED = (1 << CS_TRACKED_BIT),
> +CS_SRC_NAT = (1 << CS_SRC_NAT_BIT),
> +CS_DST_NAT = (1 << CS_DST_NAT_BIT),
> +};
and I went ahead and used an enum here, as well.
--
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovn: Use constants for conntrack state bits.
A previous commit fixed this code to match changes to the conntrack
state bit assignments. This patch further updates the code to use
the defined constants to ensure this code adapts automatically to any
possible future changes.
Signed-off-by: Russell Bryant
Requested-by: Joe Stringer
---
lib/packets.h | 29 +
ovn/controller/lflow.c | 22 +++---
2 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/lib/packets.h b/lib/packets.h
index edf140b..36bd759 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -733,14 +733,27 @@ struct tcp_header {
BUILD_ASSERT_DECL(TCP_HEADER_LEN == sizeof(struct tcp_header));
/* Connection states */
-#define CS_NEW 0x01
-#define CS_ESTABLISHED 0x02
-#define CS_RELATED 0x04
-#define CS_REPLY_DIR 0x08
-#define CS_INVALID 0x10
-#define CS_TRACKED 0x20
-#define CS_SRC_NAT 0x40
-#define CS_DST_NAT 0x80
+enum {
+CS_NEW_BIT = 0,
+CS_ESTABLISHED_BIT = 1,
+CS_RELATED_BIT = 2,
+CS_REPLY_DIR_BIT = 3,
+CS_INVALID_BIT = 4,
+CS_TRACKED_BIT = 5,
+CS_SRC_NAT_BIT = 6,
+CS_DST_NAT_BIT = 7,
+};
+
+enum {
+CS_NEW = (1 << CS_NEW_BIT),
+CS_ESTABLISHED = (1 << CS_ESTABLISHED_BIT),
+CS_RELATED = (1 << CS_RELATED_BIT),
+CS_REPLY_DIR = (1 << CS_REPLY_DIR_BIT),
+CS_INVALID = (1 << CS_INVALID_BIT),
+CS_TRACKED = (1 << CS_TRACKED_BIT),
+CS_SRC_NAT = (1 << CS_SRC_NAT_BIT),
+CS_DST_NAT = (1 << CS_DST_NAT_BIT),
+};
/* Undefined connection state bits. */
#define CS_SUPPORTED_MASK(CS_NEW | CS_ESTABLISHED | CS_RELATED \
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 764a147..91ad5db 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -24,6 +24,7 @@
#include "ovn/lib/actions.h"
#include "ovn/lib/expr.h"
#include "ovn/lib/ovn-sb-idl.h"
+#include "packets.h"
#include "simap.h"
VLOG_DEFINE_THIS_MODULE(lflow);
@@ -58,14 +59,21 @@ symtab_init(void)
MFF_LOG_REGS;
#undef MFF_LOG_REG
-/* Connection tracking state. See CS_* in lib/packets.h. */
+/* Connection tracking state. */
expr_symtab_add_field(&symtab, "ct_state", MFF_CT_STATE, NULL, false);
-expr_symtab_add_predicate(&symtab, "ct.trk", "ct_state[5]");
-expr_symtab_add_subfield(&symtab, "ct.new", "ct.trk", "ct_state[0]");
-expr_symtab_add_subfield(&symtab, "ct.est", "ct.trk", "ct_state[1]");
-expr_symtab_add_subfield(&symtab, "ct.rel", "ct.trk", "ct_state[2]");
-expr_symtab_add_subfield(&symtab, "ct.rpl", "ct.trk", "ct_state[3]");
-expr_symtab_add_subfield(&symtab, "ct.inv", "ct.trk", "ct_state[4]");
+char ct_state_str[16];
+snprintf(ct_state_str, sizeof ct_state_str, "ct_state[%d]",
CS_TRACKED_BIT);
+expr_symtab_add_predicate(&symtab, "ct.trk", ct_state_str);
+snprintf(ct_state_str, sizeof ct_state_str, "ct_state[%d]", CS_NEW_BIT);
+expr_symtab_add_subfield(&symtab, "ct.new", "ct.trk", ct_state_str);
+snprintf(ct_state_str, sizeof ct_state_str, "ct_state[%d]",
CS_ESTABLISHED_BIT);
+expr_symtab_add_subfield(&symtab, "ct.est", "ct.trk", ct_state_str);
+snprintf(ct_state_str, sizeof ct_state_str, "ct_state[%d]",
CS_RELATED_BIT);
+expr_symtab_add_subfield(&symtab, "ct.rel", "ct.trk", ct_state_str);
+snprintf(ct_state_str, sizeof ct_state_str, "ct_state[%d]",
CS_REPLY_DIR_BIT);
+expr_symtab_add_subfield(&symtab, "ct.rpl", "ct.trk", ct_state_str);
+snprintf(ct_state_str, sizeof ct_state_str, "ct_state[%d]",
CS_INVALID_BIT);
+expr_symtab_add_subfield(&symtab, "ct.inv", "ct.trk", ct_state_str);
/* Data fields. */
expr_symtab_add_field(&symtab, "eth.src", MFF_ETH_SRC, NULL, false);
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Use constants for conntrack state bits.
On 12/14/2015 12:54 PM, Russell Bryant wrote: > A previous commit fixed this code to match changes to the conntrack > state bit assignments. This patch further updates the code to use > the defined constants to ensure this code adapts automatically to any > possible future changes. > > Signed-off-by: Russell Bryant > Requested-by: Joe Stringer > --- > lib/packets.h | 29 + > ovn/controller/lflow.c | 22 +++--- > 2 files changed, 36 insertions(+), 15 deletions(-) I forgot to update the subject, but this is a v2. v1->v2: - Change list of defines to enums, as suggested by Joe. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Returned mail: Data format error
The original message was received at Mon, 14 Dec 2015 21:31:23 +0300 from openvswitch.org [136.19.70.143] - The following addresses had permanent fatal errors - dev@openvswitch.org - Transcript of session follows - ... while talking to server 94.104.125.86: 554 Service unavailable; [57.55.75.233] blocked using bl.spamcop.net, reason: Blocked Session aborted, reason: lost connection ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.
On 14/12/15 17:35, Traynor, Kevin wrote:
How about letting the control threads just float on the non-isolcpu'd cores.
We could then potentially remove the -c argument which would simplify
setup as the user would only need to think about one mask - pmd-cpu-mask
(which of course we could also default).
Strawman for it would be something like this...
Seems good, assuming that the thread affinity at the time dpdk_init()
was called reflects what cores are allowed for non-PMD threads. And what
if the user wants to change that later?
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e3a0771..031f405 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2135,12 +2135,15 @@ process_vhost_flags(char *flag, char *default_val, int
size,
int
dpdk_init(int argc, char **argv)
{
int result;
int base = 0;
char *pragram_name = argv[0];
+int err;
+int isset;
+cpu_set_t cpuset;
if (argc < 2 || strcmp(argv[1], "--dpdk"))
return 0;
/* Remove the --dpdk argument from arg list.*/
argc--;
@@ -2176,23 +2179,51 @@ dpdk_init(int argc, char **argv)
*/
argc -= 2;
argv += 2;/* Increment by two to bypass the vhost flag arguments
*/
base = 2;
}
+/*NOTE: Assumes -c option removed from cmdline/db etc. */
+
+/* Get the main thread affinity */
+CPU_ZERO(&cpuset);
+err = pthread_getaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset);
+if (err) {
+VLOG_ERR("Thread getaffinity error %d",err);
+return err;
+}
+
+/* Extract lowest core affinity and set the -c */
+for (i = 0; i < CPU_SETSIZE; i++) {
+isset = CPU_ISSET(i, &cpuset);
+if (isset) {
+/* TODO: check for any numa inconsistencies with memory and
embedded -c
+ * option in argv for rte_eal_init() */
+break;
+}
+}
+
/* Keep the program name argument as this is needed for call to
* rte_eal_init()
*/
argv[0] = pragram_name;
/* Make sure things are initialized ... */
result = rte_eal_init(argc, argv);
if (result < 0) {
ovs_abort(result, "Cannot init EAL");
}
+/* Set the main thread affinity back to pre rte_eal_init() value */
+err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset);
+if (err) {
+VLOG_ERR("Thread setaffinity error %d",err);
+return err;
+}
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] missing Hyper-V and OVN meetings this week
> On Dec 14, 2015, at 5:26 AM, Russell Bryant wrote: > >> I should be able to make the Hyper-V meeting next week, after I'm back. >> (The OVN meeting that week is on Christmas Eve; we might want to cancel >> it.) > > Yes, let's cancel the one next week. The week after is New Year's Eve, > so I suggest we postpone OVN meetings until Thursday, January 7. Agreed. Talk to you this coming Thursday, and then on January 7. --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH net] openvswitch: fix trivial comment typo
From: Paolo Abeni
Date: Mon, 14 Dec 2015 14:29:58 +0100
> The commit 33db4125ec74 ("openvswitch: Rename LABEL->LABELS") left
> over an old OVS_CT_ATTR_LABEL instance, fix it.
>
> Fixes: 33db4125ec74 ("openvswitch: Rename LABEL->LABELS")
> Signed-off-by: Paolo Abeni
Applied, thanks.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Agri Basics invoice #40642514 and 40642515
Please find attached invoice #40642514. Have a nice day Cedric Ray Accounts Receivable 320 Golden Shore, Suite 350 Long Beach, CA 90802 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [RFC] ipv6 tunneling: Fix for performance drop introduced by ipv6 tunnel support.
Adding a new flag to validate if the tunnel metadata is valid/not. This flag
avoids
the need of resetting and validating the entire ipv4/ipv6 tunnel destination
address which caused a serious performance drop.
Signed-off-by: Sugesh Chandran
---
lib/match.c| 2 ++
lib/netdev-vport.c | 3 ++-
lib/packets.h | 8
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/lib/match.c b/lib/match.c
index 95d34bc..79a561d 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -188,6 +188,7 @@ match_set_tun_dst_masked(struct match *match, ovs_be32 dst,
ovs_be32 mask)
{
match->wc.masks.tunnel.ip_dst = mask;
match->flow.tunnel.ip_dst = dst & mask;
+match->flow.tunnel.is_tnl_valid = 1;
}
void
@@ -210,6 +211,7 @@ match_set_tun_ipv6_dst(struct match *match, const struct
in6_addr *dst)
{
match->flow.tunnel.ipv6_dst = *dst;
match->wc.masks.tunnel.ipv6_dst = in6addr_exact;
+match->flow.tunnel.is_tnl_valid = 1;
}
void
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 88f5022..1cf37d7 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -920,6 +920,7 @@ ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl
*tnl,
tnl->ip_src = ip_src;
tnl->ip_dst = ip_dst;
+tnl->is_tnl_valid = 1;
tnl->ip_tos = ip->ip_tos;
tnl->ip_ttl = ip->ip_ttl;
@@ -931,7 +932,7 @@ ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl
*tnl,
memcpy(tnl->ipv6_dst.s6_addr, ip6->ip6_dst.be16, sizeof ip6->ip6_dst);
tnl->ip_tos = 0;
tnl->ip_ttl = ip6->ip6_hlim;
-
+tnl->is_tnl_valid = 1;
*hlen += IPV6_HEADER_LEN;
} else {
diff --git a/lib/packets.h b/lib/packets.h
index edf140b..06c1e19 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -37,6 +37,7 @@ struct ds;
/* Tunnel information used in flow key and metadata. */
struct flow_tnl {
+bool is_tnl_valid;
ovs_be32 ip_dst;
struct in6_addr ipv6_dst;
ovs_be32 ip_src;
@@ -49,7 +50,7 @@ struct flow_tnl {
ovs_be16 tp_dst;
ovs_be16 gbp_id;
uint8_t gbp_flags;
-uint8_t pad1[5];/* Pad to 64 bits. */
+uint8_t pad1[1];/* Pad to 64 bits. */
struct tun_metadata metadata;
};
@@ -79,7 +80,7 @@ static inline bool ipv6_addr_is_set(const struct in6_addr
*addr);
static inline bool
flow_tnl_dst_is_set(const struct flow_tnl *tnl)
{
-return tnl->ip_dst || ipv6_addr_is_set(&tnl->ipv6_dst);
+return tnl->is_tnl_valid;
}
struct in6_addr flow_tnl_dst(const struct flow_tnl *tnl);
@@ -157,8 +158,7 @@ pkt_metadata_init(struct pkt_metadata *md, odp_port_t port)
* we can just zero out ip_dst and the rest of the data will never be
* looked at. */
memset(md, 0, offsetof(struct pkt_metadata, in_port));
-md->tunnel.ip_dst = 0;
-md->tunnel.ipv6_dst = in6addr_any;
+md->tunnel.is_tnl_valid = 0;
md->in_port.odp_port = port;
}
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] lib: fix sparse warnings
Fixes the following sparse warning messages:
lib/ovsdb-idl.c:146:12: error: symbol 'table_updates_names' was not
declared. Should it be static?
lib/ovsdb-idl.c:147:12: error: symbol 'table_update_names' was not
declared. Should it be static?
lib/ovsdb-idl.c:148:12: error: symbol 'row_update_names' was not
declared. Should it be static?
Reported-by: Joe Perches
See: https://travis-ci.org/openvswitch/ovs/jobs/96737772#L870
Signed-off-by: Andy Zhou
---
lib/ovsdb-idl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 0d02ae8..6086935 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -143,9 +143,9 @@ enum ovsdb_update_version {
};
/* Name arrays indexed by 'enum ovsdb_update_version'. */
-const char *table_updates_names[] = {"table_updates", "table_updates2"};
-const char *table_update_names[] = {"table_update", "table_update2"};
-const char *row_update_names[] = {"row_update", "row_update2"};
+static const char *table_updates_names[] = {"table_updates", "table_updates2"};
+static const char *table_update_names[] = {"table_update", "table_update2"};
+static const char *row_update_names[] = {"row_update", "row_update2"};
static struct vlog_rate_limit syntax_rl = VLOG_RATE_LIMIT_INIT(1, 5);
static struct vlog_rate_limit semantic_rl = VLOG_RATE_LIMIT_INIT(1, 5);
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] jsonrpc-server: Use prototype style for ovsdb_jsonrpc_disable_monitor2().
On Mon, Dec 14, 2015 at 3:09 AM, Ben Pfaff wrote: > Without "void", this is a pre-ANSI style function definition that has > subtly different semantics. > > Found by sparse. > > Signed-off-by: Ben Pfaff > --- > Thanks for the fix. Acked-by: Andy Zhou ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] geneve-map-rename: rename geneve-map to tlv-map.
On Mon, Dec 14, 2015 at 8:52 AM, Mengke Liu wrote: > This patch renames the command name related with geneve-map to a more generic > name as following: > add-geneve-map -> add-tlv-map > del-geneve-map -> del-tlv-map > dump-geneve-map -> dump-tlv-map > > It also renames the Geneve_table to tlv_table. > > By doing this renaming, the NSH variable context header(the same TLV format as > Geneve) or other protocol can reuse the field tun_metadata in the future. > > Signed-off-by: Mengke Liu > Signed-off-by: Ricky Li I have just a couple of comments: * The G in the various commands and error codes actually stands for Geneve. i.e. NXGTMC_ADD means Nicira eXtension Geneve Table Mod Command. Presumably these should be updated as well. * It looks like you did a global find and replace of "Geneve" with "TLV", which is good for making sure that you got everything, however, it sometimes results in comments that are not very helpful because TLV is very generic whereas Geneve at least gives some context. In places where there is descriptive text such as in nicira-ext.h or ovs-ofctl.8.in, I think we need to have a little more description of what TLV is referring to (perhaps giving Geneve as an example). In other places, we should make sure that the comments still make sense grammatically. So please go over the substitutions and make improvements as necessary. > diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c > index e55e524..ffac397 100644 > --- a/ovn/controller/ofctrl.c > +++ b/ovn/controller/ofctrl.c > @@ -92,7 +92,7 @@ static struct rconn_packet_counter *tx_counter; > * installed in the switch. */ > static struct hmap installed_flows; > > -/* MFF_* field ID for our Geneve option. In S_GENEVE_TABLE_MOD_SENT, this is > +/* MFF_* field ID for our TLV option. In S_TLV_TABLE_MOD_SENT, this is I think that the first substitution should remain Geneve here instead of TLV since OVN actually is using Geneve specifically. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] match: Add support for matching IGMP fields.
> On Dec 14, 2015, at 3:12 AM, Ben Pfaff wrote: > > On Thu, Dec 10, 2015 at 01:42:41PM -0800, Jarno Rajahalme wrote: >> Complete the IGMP protocol support by making IGMP fields (type, code, >> and group) matchable via OpenFlow by the way of new Nicira extensions. >> >> The new fields are: 8-bit NXM_NX_IGMP_TYPE (111), 8-bit >> NXM_NX_IGMP_CODE (112), and 32-bit NXM_NX_IGMP_GROUP (113). >> >> VMware-BZ: #1558992 >> Signed-off-by: Jarno Rajahalme > > Is this something we're targeting to backport to OVS 2.5? If not, then > the meta-flow.h headers should say "since v2.6" instead of "since > v2.5". > > This needs to add an item to NEWS and documentation to ovs-ofctl(8). > > What's here looks good, and I trust you to do a good job on the above, > so: > Acked-by: Ben Pfaff Ben, While doing this I just realized that this may require further work, as the kernel module does not seem to parse the IGMP fields! Jarno ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH branch-2.3] odp-util: Return exact mask if netlink mask attribute is missing.
In the ODP context an empty mask netlink attribute usually means that
the flow should be an exact match.
odp_flow_key_to_mask() instead returns a struct flow_wildcards
with matches only on recirc_id and vlan_tci.
A more appropriate behavior is to handle a missing (zero length) netlink
mask specially (like we do in userspace and Linux datapath) and create
an exact match flow_wildcards from the original flow.
This fixes a bug in revalidate_ukey(): every flow created with
megaflows disabled would be revalidated away, because the mask would
seem too generic. (Another possible fix would be to handle the special
case of a missing mask in revalidate_ukey(), but this seems a more
generic solution).
Signed-off-by: Daniele Di Proietto
Acked-by: Jarno Rajahalme
---
lib/dpif-netdev.c | 65 +++
lib/odp-util.c| 26 +++--
lib/odp-util.h| 2 +-
ofproto/ofproto-dpif-upcall.c | 5 ++--
tests/test-odp.c | 4 +--
utilities/ovs-dpctl.c | 2 +-
6 files changed, 54 insertions(+), 50 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 78f8636..5fe4430 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1100,48 +1100,29 @@ static int
dpif_netdev_mask_from_nlattrs(const struct nlattr *key, uint32_t key_len,
const struct nlattr *mask_key,
uint32_t mask_key_len, const struct flow *flow,
- struct flow *mask)
-{
-if (mask_key_len) {
-enum odp_key_fitness fitness;
-
-fitness = odp_flow_key_to_mask(mask_key, mask_key_len, mask, flow);
-if (fitness) {
-/* This should not happen: it indicates that
- * odp_flow_key_from_mask() and odp_flow_key_to_mask()
- * disagree on the acceptable form of a mask. Log the problem
- * as an error, with enough details to enable debugging. */
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-
-if (!VLOG_DROP_ERR(&rl)) {
-struct ds s;
-
-ds_init(&s);
-odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s,
-true);
-VLOG_ERR("internal error parsing flow mask %s (%s)",
- ds_cstr(&s), odp_key_fitness_to_string(fitness));
-ds_destroy(&s);
-}
+ struct flow_wildcards *mask)
+{
+enum odp_key_fitness fitness;
-return EINVAL;
-}
-} else {
-enum mf_field_id id;
-/* No mask key, unwildcard everything except fields whose
- * prerequisities are not met. */
-memset(mask, 0x0, sizeof *mask);
-
-for (id = 0; id < MFF_N_IDS; ++id) {
-/* Skip registers and metadata. */
-if (!(id >= MFF_REG0 && id < MFF_REG0 + FLOW_N_REGS)
-&& id != MFF_METADATA) {
-const struct mf_field *mf = mf_from_id(id);
-if (mf_are_prereqs_ok(mf, flow)) {
-mf_mask_field(mf, mask);
-}
-}
+fitness = odp_flow_key_to_mask(mask_key, mask_key_len, mask, flow);
+if (fitness) {
+/* This should not happen: it indicates that
+ * odp_flow_key_from_mask() and odp_flow_key_to_mask()
+ * disagree on the acceptable form of a mask. Log the problem
+ * as an error, with enough details to enable debugging. */
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
+if (!VLOG_DROP_ERR(&rl)) {
+struct ds s;
+
+ds_init(&s);
+odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s,
+true);
+VLOG_ERR("internal error parsing flow mask %s (%s)",
+ ds_cstr(&s), odp_key_fitness_to_string(fitness));
+ds_destroy(&s);
}
+return EINVAL;
}
/* Force unwildcard the in_port.
@@ -1150,7 +1131,7 @@ dpif_netdev_mask_from_nlattrs(const struct nlattr *key,
uint32_t key_len,
* above because "everything" only includes the 16-bit OpenFlow port number
* mask->in_port.ofp_port, which only covers half of the 32-bit datapath
* port number mask->in_port.odp_port. */
-mask->in_port.odp_port = u32_to_odp(UINT32_MAX);
+mask->masks.in_port.odp_port = u32_to_odp(UINT32_MAX);
return 0;
}
@@ -1313,7 +1294,7 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct
dpif_flow_put *put)
}
error = dpif_netdev_mask_from_nlattrs(put->key, put->key_len,
put->mask, put->mask_len,
- &flow, &wc.masks);
+ &flow, &wc);
if (error) {
return error;
}
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 3ef13b4..
Re: [ovs-dev] [PATCH 0/9] Translation fixes for revalidation
You're right, thanks for the suggestion. I backported the applicable fixes up to branch-2.3. I've send one back port for branch-2.3 for review, as the code is quite different from master (we didn't have flow_wildcards_init_for_packet): http://openvswitch.org/pipermail/dev/2015-December/063402.html On 11/12/2015 09:32, "Jarno Rajahalme" wrote: >IMO this series should be backported to 2.4 and 2.3 as well, where >applicable. > > Jarno > > >> On Dec 10, 2015, at 18:17, Daniele Di Proietto >>wrote: >> >> >> >>> On 10/12/2015 16:27, "Jesse Gross" wrote: >>> On Thu, Dec 10, 2015 at 4:11 PM, Jarno Rajahalme wrote: > On Dec 10, 2015, at 3:20 PM, Jesse Gross wrote: > > On Wed, Dec 9, 2015 at 6:27 PM, Daniele Di Proietto > wrote: >> Sometimes the ofproto layer creates a flow which is not liked by the >> revalidation for various reasons. This behavior, while not critical >> might impact the performance. This series aims to fix a lot of >>these >> bugs. >> >> The detection has been done by modifying OVS to revalidate a flow as >> soon as it is installed (this is not included in the series, I'd be >> happy to discuss strategies to merge something like that upstream). >> If the revalidation complains there's a bug. This series fixes all >>the >> bugs found in the testsuite. >> >> The first commits are trivial fixes to various components in OVS. >>The >> last three commits address more complicated problems and I'd be >>happy >> to discuss alternative (maybe simpler) solutions. > > I just wanted to say that this is really great work. Thanks a lot for > tracking all of these corner cases down! +2 >> >> Thanks! >> >> I forgot to add my signoffs and your acks, Jarno, sorry about that. >> > I think it would definitely be worthwhile to upstream your detection > code when you have a chance. Maybe add ovs-appctl commands to turn on/off the immediate revalidation and related error reporting. I have a customer bug case were this would be immediately useful. >>> >>> I think it would be useful to both have a command to turn on immediate >>> revalidation and also automatically do it for a low percentage of >>> flows that are installed (which is what we used to have and is nice >>> since it catches errors that nobody knows about yet). We can also turn >>> this on for unit tests and fail them if it detects an inconsistency. >> >> An appctl is definitely a good idea. Maybe we can make the percentage >> configurable and set it to 100% in the testcases. I'll work on that. >> ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Hi
The original message was received at Tue, 15 Dec 2015 07:27:18 +0300 from 64.238.63.138 - The following addresses had permanent fatal errors - dev@openvswitch.org - Transcript of the session follows - ... while talking to openvswitch.org.: >>> RCPT To: <<< 550 5.1.1 ... Invalid recipient ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] lib: Use proper type cast to poison lists.
'struct ovs_list' comprises of two pointers to 'struct ovs_list'.
Use these in the cast rather than void*.
VMware-BZ: #1571356
Signed-off-by: Joe Stringer
---
lib/list.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/list.h b/lib/list.h
index db4a61365cf8..f9c9d850717e 100644
--- a/lib/list.h
+++ b/lib/list.h
@@ -26,8 +26,8 @@
/* "struct ovs_list" with pointers that will (probably) cause segfaults if
* dereferenced and, better yet, show up clearly in a debugger. */
#define OVS_LIST_POISON \
-(struct ovs_list) { (void *) (uintptr_t) 0xULL, \
-(void *) (uintptr_t) 0xULL }
+(struct ovs_list) { (struct ovs_list *) (uintptr_t) 0xULL, \
+(struct ovs_list *) (uintptr_t) 0xULL }
static inline void list_init(struct ovs_list *);
static inline void list_poison(struct ovs_list *);
--
2.1.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Bug: ovsdb monitor2 - includes unknown object
Hi,
With latest ovs code, there comes below kind of error when ovn-northd is
receiving updates from ovsdb-server for north-bound db changes.
2015-12-14T18:22:02Z|2|ovsdb_idl|WARN|syntax
"{"new":{"acls":["set",[]],"external_ids":["map",[["neutron:network_name","private"]]],"name":"neutron-fb9b829d-131a-4540-9dcd-3e6e2efe2378","ports":["set",[]]}}":
syntax error: includes unknown object
It seems for some reason ovsdb-server is sending result as v1 monitor
format, although client is using method monitor2.
Workaround is: ovs-appctl -t ovsdb-server ovsdb-server/disable-monitor2
--
Best regards,
Han
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 00/12] Conntrack debugging appctl/dpctl
On 2 December 2015 at 11:46, Daniele Di Proietto wrote: > The goal of this series is to introduce two dpctl command to interact > with the Linux kernel connection tracker. The same infrastructure > will be used by the userspace connection tracker. > > First, it defines some structures and some formatting routines (ct-dpif). > > Then, it adds some code to transform the netlink conntrack format > into the OVS specific structure (netlink-conntrack) > > Some function pointers are added into dpif-provider to implement conntrack > flushing and dumping. > > dpif-netlink implements the new dpif-provider interface using > netlink-conntrack. > > New functions are added in ct-dpif to implement dumping and flushing. > > The dpctl commands are finally added to the dpctl module, and they're > used by the system testsuite. > > Finally, a test module (test-netlink-conntrack) is added to allow the use > of the netlink-conntrack API without a datapath. Apologies for the long round-trip time. Thanks a lot for this work! Overall it looks pretty good to me. The last patch doesn't apply any more, but it's pretty trivial and looks fine. I've assembled some various small pieces of (very minor) feedback in a branch here: https://github.com/joestringer/openvswitch/tree/review/appctl_v3 When I run ovs-dpctl --help, I don't see the new commands listed. I briefly poked around, but didn't see where it is these are picked up from. Could you take a look? Do you think that these changes will build fine on Windows and *BSD? You could use Appveyor to at least check the build on Windows. I guess you may already have some kind of BSD setup. Maybe it doesn't make a difference. I'd like to see this also backported to branch-2.5, given that it would be an invaluable debugging tool for the new conntrack functionality. Acked-by: Joe Stringer Justin, did you have any other comments? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
