date:20171019

Re: [ovs-dev] [PATCH net-next v12] openvswitch: enable NSH support

2017-10-19 Thread Yang, Yi

On Thu, Oct 19, 2017 at 03:41:18PM +0200, Jiri Benc wrote:
> On Thu, 19 Oct 2017 21:12:15 +0800, Yang, Yi wrote:
> > flow_key in set_nsh is got from netlink message which is set by
> > commit_nsh in user space, here is code.
> 
> Isn't this the 'key' local variable that you're talking about, while I'm
> referring to the 'flow_key' parameter?

Oh, my mistake, but it is possible not to polulate nsh key in flow_key
for push_nsh then set, as Jan and I explained before, we don't
recirculate the packet after push_nsh for performance, so parse function
isn't called for NSH header, mdtype can't be gotten from flow_key yet.
Only one case is true, i.e. an ingress NSH packet is parsed then set by 
changing si and ttl.

For push_nsh, my typical use scinario is push_nsh then set then output
to vxlangpe port.

> 
>  Jiri
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] datapath-windows: Update OvsIPv4TunnelKey flags in geneve decap.

2017-10-19 Thread Sairam Venugopal

Thanks for fixing this.

Alin/Guru - Can we back port this to 2.8 too? This causes flow misses when 
geneve options are present.

Acked-by: Sairam Venugopal 






On 10/19/17, 1:26 PM, "ovs-dev-boun...@openvswitch.org on behalf of Anand 
Kumar"  
wrote:

>Currently, the OvsLookupFlow fails for the decap packet,
>when the Geneve options are present in the packet as the OvsIPv4TunnelKey
>flags are not set in the Geneve decap.
>
>Set the OvsIPv4TunnelKey flags OVS_TNL_F_OAM and OVS_TNL_F_CRT_OPT
>in OvsDecapGeneve based on the geneve header. Also set OVS_TNL_F_GENEVE_OPT
>if the packet has geneve options.
>
>Signed-off-by: Anand Kumar 
>---
> datapath-windows/ovsext/Geneve.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Geneve.c 
>b/datapath-windows/ovsext/Geneve.c
>index 43374e2..6dca69b 100644
>--- a/datapath-windows/ovsext/Geneve.c
>+++ b/datapath-windows/ovsext/Geneve.c
>@@ -324,10 +324,10 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT 
>switchContext,
> status = STATUS_NDIS_INVALID_PACKET;
> goto dropNbl;
> }
>-tunKey->flags = OVS_TNL_F_KEY;
>-if (geneveHdr->oam) {
>-tunKey->flags |= OVS_TNL_F_OAM;
>-}
>+/* Update tunnelKey flags. */
>+tunKey->flags = OVS_TNL_F_KEY | (geneveHdr->oam ? OVS_TNL_F_OAM : 0) |
>+(geneveHdr->critical ? OVS_TNL_F_CRT_OPT : 0);
>+
> tunKey->tunnelId = GENEVE_VNI_TO_TUNNELID(geneveHdr->vni);
> tunKey->tunOptLen = (uint8)geneveHdr->optLen * 4;
> if (tunKey->tunOptLen > TUN_OPT_MAX_LEN ||
>@@ -349,6 +349,7 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT 
>switchContext,
> memcpy(TunnelKeyGetOptions(tunKey), optStart, tunKey->tunOptLen);
> }
> NdisAdvanceNetBufferDataStart(curNb, tunKey->tunOptLen, FALSE, NULL);
>+tunKey->flags |= OVS_TNL_F_GENEVE_OPT;
> }
> 
> return NDIS_STATUS_SUCCESS;
>-- 
>2.9.3.windows.1
>
>___
>dev mailing list
>d...@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=BRjHHHPFJJP1H7BWHjwF0OS4UFpTbBiTQNM1SPvh51w=JCowvVY7CQ4CxJWnx8dPPqUbvS346xU066Dd0DnMI8A=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Documentation: Add document describing RBAC

2017-10-19 Thread Mark Michelson

Role based access control is a relatively new addition to OVS/OVN, and
aside from the database documentation in ovn-sb(5), there is not much
explaining what RBAC is, how to use it, and the available roles. This
document remedies that situation.

It is hopeful that any new roles added will be added to this document in
the future.

Signed-off-by: Mark Michelson 
---
 Documentation/automake.mk  |  1 +
 Documentation/topics/index.rst |  1 +
 Documentation/topics/role-based-access-control.rst | 97 ++
 3 files changed, 99 insertions(+)
 create mode 100644 Documentation/topics/role-based-access-control.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 6f38912f2..5344cde74 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -41,6 +41,7 @@ DOC_SOURCE = \
Documentation/topics/openflow.rst \
Documentation/topics/ovsdb-replication.rst \
Documentation/topics/porting.rst \
+   Documentation/topics/role-based-access-control.rst \
Documentation/topics/tracing.rst \
Documentation/topics/windows.rst \
Documentation/howto/index.rst \
diff --git a/Documentation/topics/index.rst b/Documentation/topics/index.rst
index f68334b4b..00d6b0b83 100644
--- a/Documentation/topics/index.rst
+++ b/Documentation/topics/index.rst
@@ -57,6 +57,7 @@ OVN
:maxdepth: 2
 
high-availability
+   role-based-access-control
 
 .. list-table::
 
diff --git a/Documentation/topics/role-based-access-control.rst 
b/Documentation/topics/role-based-access-control.rst
new file mode 100644
index 0..52c1f0c58
--- /dev/null
+++ b/Documentation/topics/role-based-access-control.rst
@@ -0,0 +1,97 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+=
+Role Based Access Control
+=
+
+Where SSL provides authentication when connecting to an OVS database, role
+based access control (RBAC) provides authorization to operations performed
+by clients connecting to an OVS database. RBAC allows for administrators to
+restrict the database operations a client may perform and thus enhance the
+security already provided by SSL.
+
+In theory, any OVS database could define RBAC roles and permissions, but at
+present only the OVN southbound database has the appropriate tables defined to
+facilitate RBAC.
+
+Mechanics
+-
+RBAC is intended to supplement SSL. In order to enable RBAC, the connection to
+the database must use SSL. Some permissions in RBAC are granted based on the
+certificate common name (CN) of the connecting client.
+
+RBAC is controlled with two database tables, RBAC_Role and RBAC_Permissions.
+The RBAC_Permissions table contains records that describe a set of permissions
+for a given table in the database.
+
+The RBAC_Permission table contains the following columns:
+
+- table: The table in the database for which permissions are being described.
+- insert_delete: Describes whether insertion and deletion of records is
+  allowed.
+- update: A list of columns that are allowed to be updated.
+- authorization: A list of column names. One of the listed columns must match
+  the SSL certificate CN in order for the attempted operation on the table to
+  succeed. If a key-value pair is provided, then the key is the column name,
+  and the value is the name of a key in that column. An empty string gives
+  permission to all clients to perform operations.
+
+The RBAC_Role table contains the following columns:
+
+- name: The name of the role being defined
+- permissions: A list of key-value pairs. The key is the name of a table in the
+  database, and the value is a UUID of a record in the RBAC_Permission
+  table that describes the permissions the role has for that
+  table.
+
+.. note::
+
+   All tables not explicitly referenced in an RBAC_Role record are read-only
+
+In order to enable RBAC, specify the role name as an argument to the
+set-connection command for the database. As an example, to enable the
+"ovn-controller" role on the OVN southbound database, use the following

[ovs-dev] [PATCH v2] datapath-windows: Update OvsIPv4TunnelKey flags in geneve decap.

2017-10-19 Thread Anand Kumar

Currently, the OvsLookupFlow fails for the decap packet,
when the Geneve options are present in the packet as the OvsIPv4TunnelKey
flags are not set in the Geneve decap.

Set the OvsIPv4TunnelKey flags OVS_TNL_F_OAM and OVS_TNL_F_CRT_OPT
in OvsDecapGeneve based on the geneve header. Also set OVS_TNL_F_GENEVE_OPT
if the packet has geneve options.

Signed-off-by: Anand Kumar 
---
 datapath-windows/ovsext/Geneve.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Geneve.c b/datapath-windows/ovsext/Geneve.c
index 43374e2..6dca69b 100644
--- a/datapath-windows/ovsext/Geneve.c
+++ b/datapath-windows/ovsext/Geneve.c
@@ -324,10 +324,10 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT 
switchContext,
 status = STATUS_NDIS_INVALID_PACKET;
 goto dropNbl;
 }
-tunKey->flags = OVS_TNL_F_KEY;
-if (geneveHdr->oam) {
-tunKey->flags |= OVS_TNL_F_OAM;
-}
+/* Update tunnelKey flags. */
+tunKey->flags = OVS_TNL_F_KEY | (geneveHdr->oam ? OVS_TNL_F_OAM : 0) |
+(geneveHdr->critical ? OVS_TNL_F_CRT_OPT : 0);
+
 tunKey->tunnelId = GENEVE_VNI_TO_TUNNELID(geneveHdr->vni);
 tunKey->tunOptLen = (uint8)geneveHdr->optLen * 4;
 if (tunKey->tunOptLen > TUN_OPT_MAX_LEN ||
@@ -349,6 +349,7 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT 
switchContext,
 memcpy(TunnelKeyGetOptions(tunKey), optStart, tunKey->tunOptLen);
 }
 NdisAdvanceNetBufferDataStart(curNb, tunKey->tunOptLen, FALSE, NULL);
+tunKey->flags |= OVS_TNL_F_GENEVE_OPT;
 }
 
 return NDIS_STATUS_SUCCESS;
-- 
2.9.3.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH RFC v2] netdev-dpdk: Allow specification of index for PCI devices

2017-10-19 Thread Loftus, Ciara

> 
> On 10/17/2017 11:48 AM, Ciara Loftus wrote:
> > Some NICs have only one PCI address associated with multiple ports. This
> > patch extends the dpdk-devargs option's format to cater for such
> > devices. Whereas before only one of N ports associated with one PCI
> > address could be added, now N can.
> >
> > This patch allows for the following use of the dpdk-devargs option:
> >
> > ovs-vsctl set Interface myport options:dpdk-devargs=:06:00.0,X
> >
> > Where X is an unsigned integer representing one of multiple ports
> > associated with the PCI address provided.
> >
> > This patch has not been tested.
> >
> > Signed-off-by: Ciara Loftus 
> > ---
> > v2:
> > * Simplify function to find port ID of indexed device
> > * Hotplug compatibility
> > * Detach compatibility
> > * Documentation
> > * Use strtol instead of atoi
> >
> >  Documentation/howto/dpdk.rst |  9 +
> >  Documentation/intro/install/dpdk.rst |  9 +
> >  NEWS |  2 ++
> >  lib/netdev-dpdk.c| 67 ++---
> ---
> >  vswitchd/vswitch.xml | 11 --
> >  5 files changed, 85 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/howto/dpdk.rst
> b/Documentation/howto/dpdk.rst
> > index d123819..9447b71 100644
> > --- a/Documentation/howto/dpdk.rst
> > +++ b/Documentation/howto/dpdk.rst
> > @@ -48,6 +48,15 @@ number of dpdk devices found in the log file::
> >  $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \
> >  options:dpdk-devargs=:01:00.1
> >
> > +If your PCI device has multiple ports under the same PCI ID, you can use
> the
> > +following notation to indicate the specific device you wish to add::
> > +
> > +$ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
> > +options:dpdk-devargs=:01:00.0,0
> > +
> > +The above would attempt to use the first device (0) associated with that
> PCI
> > +ID. Use ,1 ,2 etc. to access the next.
> > +
> >  After the DPDK ports get added to switch, a polling thread continuously
> polls
> >  DPDK devices and consumes 100% of the core, as can be checked from
> ``top`` and
> >  ``ps`` commands::
> > diff --git a/Documentation/intro/install/dpdk.rst
> b/Documentation/intro/install/dpdk.rst
> > index bb69ae5..d0e87f5 100644
> > --- a/Documentation/intro/install/dpdk.rst
> > +++ b/Documentation/intro/install/dpdk.rst
> > @@ -271,6 +271,15 @@ ports. For example, to create a userspace bridge
> named ``br0`` and add two
> >  DPDK devices will not be available for use until a valid dpdk-devargs is
> >  specified.
> >
> > +If your PCI device has multiple ports under the same PCI ID, you can use
> the
> > +following notation to indicate the specific device you wish to add::
> > +
> > +$ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
> > +options:dpdk-devargs=:01:00.0,0
> > +
> > +The above would attempt to use the first device (0) associated with that
> PCI
> > +ID. Use ,1 ,2 etc. to access the next.
> > +
> >  Refer to ovs-vsctl(8) and :doc:`/howto/dpdk` for more details.
> >
> >  Performance Tuning
> > diff --git a/NEWS b/NEWS
> > index 75f5fa5..ca885a6 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -5,6 +5,8 @@ Post-v2.8.0
> > chassis "hostname" in addition to a chassis "name".
> > - Linux kernel 4.13
> >   * Add support for compiling OVS with the latest Linux 4.13 kernel
> > +   - DPDK:
> > + * Support for adding devices with multiple DPDK ports under one PCI
> ID.
> >
> >  v2.8.0 - 31 Aug 2017
> > - ovs-ofctl:
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index c60f46f..35e15da 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -1214,16 +1214,40 @@
> netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id)
> >  }
> >
> >  static dpdk_port_t
> > +dpdk_get_port_by_name_with_index(char *name, int idx, int base_id)
> > +{
> > +struct rte_eth_dev_info dev_info;
> > +char pci_addr[PCI_PRI_STR_SIZE];
> > +dpdk_port_t offset_port_id = base_id + idx;
> > +
> > +if (rte_eth_dev_is_valid_port(offset_port_id)) {
> > +rte_eth_dev_info_get(offset_port_id, _info);
> > +rte_pci_device_name(_info.pci_dev->addr, pci_addr,
> > +sizeof(pci_addr));
> > +if (!strcmp(pci_addr, name)) {
> > +return offset_port_id;
> > +}
> > +}
> > +
> > +VLOG_INFO("Invalid PCI offset %i for device %s", idx, name);
> > +
> > +return DPDK_ETH_PORT_ID_INVALID;
> > +}
> > +
> > +static dpdk_port_t
> >  netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
> >  const char *devargs, char **errp)
> >  {
> > -/* Get the name up to the first comma. */
> > -char *name = xmemdup0(devargs, strcspn(devargs, ","));
> > +char *devargs_copy = xmemdup0((devargs), strlen(devargs));
> > +char *name, *idx_str;
> > +unsigned idx;
> >

[ovs-dev] [PATCH] openvswitch: conntrack: mark expected switch fall-through

2017-10-19 Thread Gustavo A. R. Silva

In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Notice that in this particular case I placed a "fall through" comment on
its own line, which is what GCC is expecting to find.

Signed-off-by: Gustavo A. R. Silva 
---
 net/openvswitch/conntrack.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index fe861e2..b27c5c6 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -752,6 +752,7 @@ static int ovs_ct_nat_execute(struct sk_buff *skb, struct 
nf_conn *ct,
}
}
/* Non-ICMP, fall thru to initialize if needed. */
+   /* fall through */
case IP_CT_NEW:
/* Seen it before?  This can happen for loopback, retrans,
 * or local packets.
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.

2017-10-19 Thread Stokes, Ian

> > -Original Message-
> > From: Stokes, Ian
> > Sent: Thursday, October 19, 2017 6:23 PM
> > To: Fischetti, Antonio ;
> > d...@openvswitch.org
> > Subject: RE: [ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in
> > mempool name creation.
> >
> > > In case a mempool name could not be generated log a message and
> > > return a null mempool pointer to the caller.
> > >
> >
> > Now that I look at this, is it the case that the issues have been
> > resolved in patches 1 and 2 but this is a clean up patch for something
> that might happen?
> 
> [Antonio] Exactly. All the issues we saw:
>  - issue with vhostuserclient in a PVP test
>  - issue of new MTU not displayed when an existing mempool is returned.
>  - issue with the NUMA-Aware usecase
> get resolved by patches #1 and #2.
> 
> All remaining patches are small improvements or just cosmetics.
> 
> This patch would be a small improvement to manage the unlikely event of a
> name creation failure that 'might' happen.
> So this has nothing to do with the issues listed above. I added this patch
> to the series because  - after looking at the code - I thought it would be
> good to track and manage an empty mempool name, just to be extra-safe.
> 

Ok, thanks for clarifying Antonio, I'm going to start testing on this patch 
series tomorrow with a view to validation. 

I think most reviewers have acked at this point but I want to flag it again to 
those ccd, I'm anxious to get this patch upstreamed to fix master once it has 
passed validation.
> 
> >
> >
> > > CC: Mark B Kavanagh 
> > > CC: Darrell Ball 
> > > CC: Ciara Loftus 
> > > CC: Kevin Traynor 
> > > CC: Aaron Conole 
> > > Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for
> > > each
> > > port.")
> > > Signed-off-by: Antonio Fischetti 
> > > ---
> > >  lib/netdev-dpdk.c | 7 +++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > > dc1e9c3..6fc6e1b
> > > 100644
> > > --- a/lib/netdev-dpdk.c
> > > +++ b/lib/netdev-dpdk.c
> > > @@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
> > >  int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
> "ovs_%x_%d_%d_%u",
> > > h, dmp->socket_id, dmp->mtu, dmp->mp_size);
> > >  if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
> > > +VLOG_DBG("snprintf returned %d. Failed to generate a mempool
> "
> > > +"name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.",
> > > +ret, dmp->if_name, h, dmp->mtu, dmp->mp_size);
> > >  return NULL;
> > >  }
> > >  return mp_name;
> > > @@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int
> > > mtu, bool
> > > *mp_exists)
> > >
> > >  do {
> > >  char *mp_name = dpdk_mp_name(dmp);
> > > +if (!mp_name) {
> > > +rte_free(dmp);
> > > +return NULL;
> > > +}
> > >
> > >  VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
> > >"on socket %d for %d Rx and %d Tx queues.",
> > > --
> > > 2.4.11
> > >
> > > ___
> > > 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 v8 0/6] netdev-dpdk: Fix mempool management and other cleanup.

2017-10-19 Thread Fischetti, Antonio



> -Original Message-
> From: Stokes, Ian
> Sent: Thursday, October 19, 2017 6:22 PM
> To: Fischetti, Antonio ; d...@openvswitch.org
> Subject: RE: [ovs-dev] [PATCH v8 0/6] netdev-dpdk: Fix mempool management and
> other cleanup.
> 
> > Patch #1, #2 and #4 contain the fixes.

[Antonio] Further to previous line: patches #1 and #2 do fix the issues we saw: 
 - issue with vhostuserclient in a PVP test
 - issue of new MTU not displayed when an existing mempool is returned.
 - issue with the NUMA-Aware usecase 

All other patches #3, #4, #5 and #6 are small improvements or just clean-up 
that we 
could even skip at all.
Actually patch #4 is not just a clean-up, it's managing an unlikely event that 
'might' happen (I've never seen it), that's why I promoted it as a 'fix' in the
line above.

So just the first 2 patches are needed to fix the mempool issues.


> > All other patches in this series are a clean up for code readability or
> > small improvements.
> >
> 
> Hi Antonio, if the fixes are in patches 1 2 and 4 is there a reason they have
> not been grouped in a patchset and patch 1,2,3?
> 
> The other patches could be applied separately afterwards?

[Antonio] Patches #1 and #2 are the ones needed to fix the issues.
All other patches are clean-up/small improvement, that could be optionally
applied. I think it's better to apply them in the given order.


> 
> > List of versions:
> >  - v8:
> >- Debug message rephrased in patch #2.
> >- Reworked patch #4 for snprintf error code.
> >- Comments in patch #6 moved into patch #1.
> >
> >  - v7:
> >- Restored 2 separate patches for the 2 fixes.
> >- patch #1: detect when previous mempools must be released.
> >- patch #2: mempool name generation for NUMA-awareness test case.
> >- patch "netdev-dpdk: manage empty mempool names." renamed as
> >  "netdev-dpdk: manage failure in mempool name creation."
> >- Various rework based on comments.
> >
> >  - v6:
> >- patches #1 and #2 squashed into one.
> >- Reworked to consider the latest comments.
> >- tested the release of pre-existing mempools (reported by Ciara L.)
> >- tested the change of MTU when an existing mempool is returned
> >  (reported by Robert M.)
> >- tested the NUMA-Awareness usecase (reported by Ciara L.)
> >  - v5: manage new MTU value when a pre-existing mempool is returned.
> >  - v4: fix NUMA awareness usecase
> >  - v3: avoid deletion of pre-existing mempools
> >  - v2: rework to accomodate code changes for dpdk ports too
> >  - v1: 1st implementation.
> >
> > Fischetti, Antonio (6):
> >   netdev-dpdk: fix management of pre-existing mempools.
> >   Fix mempool names to reflect socket id.
> >   netdev-dpdk: skip init for existing mempools.
> >   netdev-dpdk: manage failure in mempool name creation.
> >   netdev-dpdk: Reword mp_size as n_mbufs.
> >   netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.
> >
> >  lib/netdev-dpdk.c | 96 ++
> > -
> >  1 file changed, 59 insertions(+), 37 deletions(-)
> >
> > --
> > 2.4.11
> >
> > ___
> > 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 v8 4/6] netdev-dpdk: manage failure in mempool name creation.

2017-10-19 Thread Fischetti, Antonio



> -Original Message-
> From: Stokes, Ian
> Sent: Thursday, October 19, 2017 6:23 PM
> To: Fischetti, Antonio ; d...@openvswitch.org
> Subject: RE: [ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in mempool
> name creation.
> 
> > In case a mempool name could not be generated log a message and return a
> > null mempool pointer to the caller.
> >
> 
> Now that I look at this, is it the case that the issues have been resolved in
> patches 1 and 2 but this is a clean up patch for something that might happen?

[Antonio] Exactly. All the issues we saw: 
 - issue with vhostuserclient in a PVP test
 - issue of new MTU not displayed when an existing mempool is returned.
 - issue with the NUMA-Aware usecase
get resolved by patches #1 and #2.

All remaining patches are small improvements or just cosmetics.

This patch would be a small improvement to manage the unlikely event of a name 
creation failure that 'might' happen. 
So this has nothing to do with the issues listed above. I added this patch to 
the series because  - after looking at the code - I thought it would be good to 
track and manage an empty mempool name, just to be extra-safe.


> 
> 
> > CC: Mark B Kavanagh 
> > CC: Darrell Ball 
> > CC: Ciara Loftus 
> > CC: Kevin Traynor 
> > CC: Aaron Conole 
> > Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
> > port.")
> > Signed-off-by: Antonio Fischetti 
> > ---
> >  lib/netdev-dpdk.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index dc1e9c3..6fc6e1b
> > 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
> >  int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
> > h, dmp->socket_id, dmp->mtu, dmp->mp_size);
> >  if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
> > +VLOG_DBG("snprintf returned %d. Failed to generate a mempool "
> > +"name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.",
> > +ret, dmp->if_name, h, dmp->mtu, dmp->mp_size);
> >  return NULL;
> >  }
> >  return mp_name;
> > @@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
> > *mp_exists)
> >
> >  do {
> >  char *mp_name = dpdk_mp_name(dmp);
> > +if (!mp_name) {
> > +rte_free(dmp);
> > +return NULL;
> > +}
> >
> >  VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
> >"on socket %d for %d Rx and %d Tx queues.",
> > --
> > 2.4.11
> >
> > ___
> > 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] datapath-windows: Update OvsIPv4TunnelKey flags in geneve decap.

2017-10-19 Thread Anand Kumar

Hi Sairam,

Sure, I will add the check to see if there are any geneve tunnel options and 
set OVS_TNL_F_GENEVE_OPT accordingly and send out v2 version of the patch.

Thanks,
Anand Kumar

On 10/19/17, 8:13 AM, "Sairam Venugopal"  wrote:

Anand,

Looks like Linux does make a check to see if there are any metadata present 
before setting these flags - 
https://github.com/openvswitch/ovs/blob/master/datapath/linux/compat/geneve.c#L239

Can you verify this and update the patch? (un-ack in meantime??)

Thanks,
Sairam




On 10/18/17, 3:45 PM, "ovs-dev-boun...@openvswitch.org on behalf of Sairam 
Venugopal"  
wrote:

>Thanks for fixing this. Should we check for geneveHdr->opts before setting 
OVS_TNL_F_GENEVE_OPT?
>
>I know the current code maintains consistency with Linux. But we need to 
understand if this is intended.
>
>Acked-by: Sairam Venugopal 
>
>
>
>
>
>On 10/17/17, 5:31 PM, "ovs-dev-boun...@openvswitch.org on behalf of Anand 
Kumar"  
wrote:
>
>>Set the geneve flags OVS_TNL_F_GENEVE_OPT and OVS_TNL_F_CRT_OPT
>>in OvsDecapGeneve, so that windows behavior is similiar to linux

>>https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs_blob_master_datapath_linux_compat_geneve.c-23L242=DwICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=ntDbsrs3dZtqbGfAIQnQA7qzLK8SlLouMDV36u0JsGQ=TtHsQ-7YSUu8XJHpn4iiQkhaothJamn3dU7_FKuRptE=
>>
>>Signed-off-by: Anand Kumar 
>>---
>> datapath-windows/ovsext/Geneve.c | 8 
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>>diff --git a/datapath-windows/ovsext/Geneve.c 
b/datapath-windows/ovsext/Geneve.c
>>index 43374e2..77244b1 100644
>>--- a/datapath-windows/ovsext/Geneve.c
>>+++ b/datapath-windows/ovsext/Geneve.c
>>@@ -324,10 +324,10 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT 
switchContext,
>> status = STATUS_NDIS_INVALID_PACKET;
>> goto dropNbl;
>> }
>>-tunKey->flags = OVS_TNL_F_KEY;
>>-if (geneveHdr->oam) {
>>-tunKey->flags |= OVS_TNL_F_OAM;
>>-}
>>+/* Update tunnelKey flags. */
>>+tunKey->flags = OVS_TNL_F_KEY | OVS_TNL_F_GENEVE_OPT |
>>+(geneveHdr->oam ? OVS_TNL_F_OAM : 0) |
>>+(geneveHdr->critical ? OVS_TNL_F_CRT_OPT : 0);
>> tunKey->tunnelId = GENEVE_VNI_TO_TUNNELID(geneveHdr->vni);
>> tunKey->tunOptLen = (uint8)geneveHdr->optLen * 4;
>> if (tunKey->tunOptLen > TUN_OPT_MAX_LEN ||
>>-- 
>>2.9.3.windows.1
>>
>>___
>>dev mailing list
>>d...@openvswitch.org

>>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=ntDbsrs3dZtqbGfAIQnQA7qzLK8SlLouMDV36u0JsGQ=weAfeEKoncNrwUrh3c5va-Efy3yApbOLqPd2LZleZkM=
>___
>dev mailing list
>d...@openvswitch.org

>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=eMSUuuWxH9h1UeKSVIcTwgE8ktXUUIht3ybSd8WphY8=Y6LNJpoXU2BdIsXqef34Gk0al6E4EaHcbISPLhClhVc=


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.

2017-10-19 Thread Stokes, Ian

> In case a mempool name could not be generated log a message and return a
> null mempool pointer to the caller.
> 

Now that I look at this, is it the case that the issues have been resolved in 
patches 1 and 2 but this is a clean up patch for something that might happen?


> CC: Mark B Kavanagh 
> CC: Darrell Ball 
> CC: Ciara Loftus 
> CC: Kevin Traynor 
> CC: Aaron Conole 
> Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
> port.")
> Signed-off-by: Antonio Fischetti 
> ---
>  lib/netdev-dpdk.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index dc1e9c3..6fc6e1b
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
>  int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
> h, dmp->socket_id, dmp->mtu, dmp->mp_size);
>  if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
> +VLOG_DBG("snprintf returned %d. Failed to generate a mempool "
> +"name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.",
> +ret, dmp->if_name, h, dmp->mtu, dmp->mp_size);
>  return NULL;
>  }
>  return mp_name;
> @@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
> *mp_exists)
> 
>  do {
>  char *mp_name = dpdk_mp_name(dmp);
> +if (!mp_name) {
> +rte_free(dmp);
> +return NULL;
> +}
> 
>  VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
>"on socket %d for %d Rx and %d Tx queues.",
> --
> 2.4.11
> 
> ___
> 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 v8 0/6] netdev-dpdk: Fix mempool management and other cleanup.

2017-10-19 Thread Stokes, Ian

> Patch #1, #2 and #4 contain the fixes.
> All other patches in this series are a clean up for code readability or
> small improvements.
> 

Hi Antonio, if the fixes are in patches 1 2 and 4 is there a reason they have 
not been grouped in a patchset and patch 1,2,3?

The other patches could be applied separately afterwards?

> List of versions:
>  - v8:
>- Debug message rephrased in patch #2.
>- Reworked patch #4 for snprintf error code.
>- Comments in patch #6 moved into patch #1.
> 
>  - v7:
>- Restored 2 separate patches for the 2 fixes.
>- patch #1: detect when previous mempools must be released.
>- patch #2: mempool name generation for NUMA-awareness test case.
>- patch "netdev-dpdk: manage empty mempool names." renamed as
>  "netdev-dpdk: manage failure in mempool name creation."
>- Various rework based on comments.
> 
>  - v6:
>- patches #1 and #2 squashed into one.
>- Reworked to consider the latest comments.
>- tested the release of pre-existing mempools (reported by Ciara L.)
>- tested the change of MTU when an existing mempool is returned
>  (reported by Robert M.)
>- tested the NUMA-Awareness usecase (reported by Ciara L.)
>  - v5: manage new MTU value when a pre-existing mempool is returned.
>  - v4: fix NUMA awareness usecase
>  - v3: avoid deletion of pre-existing mempools
>  - v2: rework to accomodate code changes for dpdk ports too
>  - v1: 1st implementation.
> 
> Fischetti, Antonio (6):
>   netdev-dpdk: fix management of pre-existing mempools.
>   Fix mempool names to reflect socket id.
>   netdev-dpdk: skip init for existing mempools.
>   netdev-dpdk: manage failure in mempool name creation.
>   netdev-dpdk: Reword mp_size as n_mbufs.
>   netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.
> 
>  lib/netdev-dpdk.c | 96 ++
> -
>  1 file changed, 59 insertions(+), 37 deletions(-)
> 
> --
> 2.4.11
> 
> ___
> 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 v8 6/6] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.

2017-10-19 Thread antonio . fischetti

For readability purposes dpdk_mp_put is renamed as dpdk_mp_free.

CC: Mark B Kavanagh 
CC: Darrell Ball 
CC: Ciara Loftus 
CC: Kevin Traynor 
CC: Aaron Conole 
Signed-off-by: Antonio Fischetti 
---
 lib/netdev-dpdk.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index bf143e0..82652f0 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -602,8 +602,9 @@ dpdk_mp_get(struct netdev_dpdk *dev, int mtu, bool 
*mp_exists)
 return dmp;
 }
 
+/* Release an existing mempool. */
 static void
-dpdk_mp_put(struct dpdk_mp *dmp)
+dpdk_mp_free(struct dpdk_mp *dmp)
 {
 char *mp_name;
 
@@ -649,7 +650,8 @@ netdev_dpdk_mempool_configure(struct netdev_dpdk *dev)
 dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
 return EEXIST;
 } else {
-dpdk_mp_put(dev->dpdk_mp);
+/* A new mempool was created, release the previous one. */
+dpdk_mp_free(dev->dpdk_mp);
 dev->dpdk_mp = mp;
 dev->mtu = dev->requested_mtu;
 dev->socket_id = dev->requested_socket_id;
@@ -1094,7 +1096,7 @@ common_destruct(struct netdev_dpdk *dev)
 OVS_EXCLUDED(dev->mutex)
 {
 rte_free(dev->tx_q);
-dpdk_mp_put(dev->dpdk_mp);
+dpdk_mp_free(dev->dpdk_mp);
 
 ovs_list_remove(>list_node);
 free(ovsrcu_get_protected(struct ingress_policer *,
-- 
2.4.11

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v8 1/6] netdev-dpdk: fix management of pre-existing mempools.

2017-10-19 Thread antonio . fischetti

Fix an issue on reconfiguration of pre-existing mempools.
This patch avoids to call dpdk_mp_put() - and erroneously
release the mempool - when it already exists.

CC: Mark B Kavanagh 
CC: Aaron Conole 
CC: Darrell Ball 
Acked-by: Kevin Traynor 
Reported-by: Ciara Loftus 
Tested-by: Ciara Loftus 
Reported-by: Róbert Mulik 
Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.")
Signed-off-by: Antonio Fischetti 
---
I've tested this patch by
  - changing at run-time the number of Rx queues:
  ovs-vsctl set Interface dpdk0 type=dpdk options:n_rxq=4

  - reducing the MTU of the dpdk ports of 1 byte to force
the configuration of an existing mempool:
  ovs-vsctl set Interface dpdk0 mtu_request=1499

This issue was observed in a PVP test topology with dpdkvhostuserclient
ports. It can happen also with dpdk type ports, eg by reducing the MTU
of 1 byte.

To replicate the bug scenario in the PVP case it's sufficient to
set 1 dpdkvhostuserclient port, and just boot the VM.

Below some more details on my own test setup.

 PVP test setup
 --
CLIENT_SOCK_DIR=/tmp
SOCK0=dpdkvhostuser0
SOCK1=dpdkvhostuser1

1 PMD
Add 2 dpdk ports, n_rxq=1
Add 2 vhu ports both of type dpdkvhostuserclient and specify vhost-server-path
 ovs-vsctl set Interface dpdkvhostuser0 
options:vhost-server-path="$CLIENT_SOCK_DIR/$SOCK0"
 ovs-vsctl set Interface dpdkvhostuser1 
options:vhost-server-path="$CLIENT_SOCK_DIR/$SOCK1"

Set port-based rules: dpdk0 <--> vhu0 and dpdk1 <--> vhu1
 add-flow br0 in_port=1,action=output:3
 add-flow br0 in_port=3,action=output:1
 add-flow br0 in_port=4,action=output:2
 add-flow br0 in_port=2,action=output:4

 Launch QEMU
 ---
As OvS vhu ports are acting as clients, we must specify 'server' in the next 
command.
VM_IMAGE=

 sudo -E taskset 0x3F00 $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -name 
us-vhost-vm1 -cpu host -enable-kvm -m 4096M -object 
memory-backend-file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on -numa 
node,memdev=mem -mem-prealloc -smp 4 -drive file=$VM_IMAGE -chardev 
socket,id=char0,path=$CLIENT_SOCK_DIR/$SOCK0,server -netdev 
type=vhost-user,id=mynet1,chardev=char0,vhostforce -device 
virtio-net-pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=off -chardev 
socket,id=char1,path=$CLIENT_SOCK_DIR/$SOCK1,server -netdev 
type=vhost-user,id=mynet2,chardev=char1,vhostforce -device 
virtio-net-pci,mac=00:00:00:00:00:02,netdev=mynet2,mrg_rxbuf=off --nographic

 Expected behavior
 -
With this fix OvS shouldn't crash.
---
 lib/netdev-dpdk.c | 44 
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c60f46f..45a81f2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -508,12 +508,13 @@ dpdk_mp_name(struct dpdk_mp *dmp)
 }
 
 static struct dpdk_mp *
-dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
+dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool *mp_exists)
 {
 struct dpdk_mp *dmp = dpdk_rte_mzalloc(sizeof *dmp);
 if (!dmp) {
 return NULL;
 }
+*mp_exists = false;
 dmp->socket_id = dev->requested_socket_id;
 dmp->mtu = mtu;
 ovs_strzcpy(dmp->if_name, dev->up.name, IFNAMSIZ);
@@ -530,8 +531,6 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
 + MIN(RTE_MAX_LCORE, dev->requested_n_rxq) * NETDEV_MAX_BURST
 + MIN_NB_MBUF;
 
-bool mp_exists = false;
-
 do {
 char *mp_name = dpdk_mp_name(dmp);
 
@@ -559,7 +558,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
 /* As the mempool create returned EEXIST we can expect the
  * lookup has returned a valid pointer.  If for some reason
  * that's not the case we keep track of it. */
-mp_exists = true;
+*mp_exists = true;
 } else {
 VLOG_ERR("Failed mempool \"%s\" create request of %u mbufs",
  mp_name, dmp->mp_size);
@@ -573,20 +572,23 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
 rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
 return dmp;
 }
-} while (!mp_exists &&
+} while (!(*mp_exists) &&
 (rte_errno == ENOMEM && (dmp->mp_size /= 2) >= MIN_NB_MBUF));
 
 rte_free(dmp);
 return NULL;
 }
 
+/* Returns a valid pointer when either of the following is true:
+ *  - a new mempool was just created;
+ *  - a matching mempool already exists. */
 static struct dpdk_mp *
-dpdk_mp_get(struct netdev_dpdk *dev, int mtu)
+dpdk_mp_get(struct netdev_dpdk *dev, int mtu, bool *mp_exists)
 {
 struct dpdk_mp *dmp;
 
 ovs_mutex_lock(_mp_mutex);
-dmp = dpdk_mp_create(dev, mtu);
+dmp = dpdk_mp_create(dev, mtu, mp_exists);

[ovs-dev] [PATCH v8 5/6] netdev-dpdk: Reword mp_size as n_mbufs.

2017-10-19 Thread antonio . fischetti

For code readability purposes mp_size is renamed as n_mbufs
in dpdk_mp structure.
This parameter is passed to rte mempool creation functions
and is meant to contain the number of elements inside
the requested mempool.

CC: Ciara Loftus 
CC: Aaron Conole 
Acked-by: Kevin Traynor 
Acked-by: Mark Kavanagh 
Signed-off-by: Antonio Fischetti 
---
 lib/netdev-dpdk.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 6fc6e1b..bf143e0 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -308,7 +308,7 @@ struct dpdk_mp {
 int mtu;
 int socket_id;
 char if_name[IFNAMSIZ];
-unsigned mp_size;
+unsigned n_mbufs;   /* Number of mbufs inside the mempool. */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mp_mutex);
 };
 
@@ -500,11 +500,11 @@ dpdk_mp_name(struct dpdk_mp *dmp)
 uint32_t h = hash_string(dmp->if_name, 0);
 char *mp_name = xcalloc(RTE_MEMPOOL_NAMESIZE, sizeof *mp_name);
 int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
-   h, dmp->socket_id, dmp->mtu, dmp->mp_size);
+   h, dmp->socket_id, dmp->mtu, dmp->n_mbufs);
 if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
 VLOG_DBG("snprintf returned %d. Failed to generate a mempool "
 "name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.",
-ret, dmp->if_name, h, dmp->mtu, dmp->mp_size);
+ret, dmp->if_name, h, dmp->mtu, dmp->n_mbufs);
 return NULL;
 }
 return mp_name;
@@ -523,13 +523,13 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
*mp_exists)
 ovs_strzcpy(dmp->if_name, dev->up.name, IFNAMSIZ);
 
 /*
- * XXX: rough estimation of memory required for port:
+ * XXX: rough estimation of number of mbufs required for this port:
  * 
  * + 
  * + 
  * + 
  */
-dmp->mp_size = dev->requested_n_rxq * dev->requested_rxq_size
+dmp->n_mbufs = dev->requested_n_rxq * dev->requested_rxq_size
 + dev->requested_n_txq * dev->requested_txq_size
 + MIN(RTE_MAX_LCORE, dev->requested_n_rxq) * NETDEV_MAX_BURST
 + MIN_NB_MBUF;
@@ -543,11 +543,11 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
*mp_exists)
 
 VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
   "on socket %d for %d Rx and %d Tx queues.",
-  dev->up.name, dmp->mp_size,
+  dev->up.name, dmp->n_mbufs,
   dev->requested_socket_id,
   dev->requested_n_rxq, dev->requested_n_txq);
 
-dmp->mp = rte_pktmbuf_pool_create(mp_name, dmp->mp_size,
+dmp->mp = rte_pktmbuf_pool_create(mp_name, dmp->n_mbufs,
   MP_CACHE_SZ,
   sizeof (struct dp_packet)
  - sizeof (struct rte_mbuf),
@@ -556,7 +556,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
*mp_exists)
   dmp->socket_id);
 if (dmp->mp) {
 VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,
- dmp->mp_size);
+ dmp->n_mbufs);
 /* rte_pktmbuf_pool_create has done some initialization of the
  * rte_mbuf part of each dp_packet. Some OvS specific fields
  * of the packet still need to be initialized by
@@ -574,14 +574,14 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
*mp_exists)
 *mp_exists = true;
 } else {
 VLOG_ERR("Failed mempool \"%s\" create request of %u mbufs",
- mp_name, dmp->mp_size);
+ mp_name, dmp->n_mbufs);
 }
 free(mp_name);
 if (dmp->mp) {
 return dmp;
 }
 } while (!(*mp_exists) &&
-(rte_errno == ENOMEM && (dmp->mp_size /= 2) >= MIN_NB_MBUF));
+(rte_errno == ENOMEM && (dmp->n_mbufs /= 2) >= MIN_NB_MBUF));
 
 rte_free(dmp);
 return NULL;
-- 
2.4.11

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v8 4/6] netdev-dpdk: manage failure in mempool name creation.

2017-10-19 Thread antonio . fischetti

In case a mempool name could not be generated log a message
and return a null mempool pointer to the caller.

CC: Mark B Kavanagh 
CC: Darrell Ball 
CC: Ciara Loftus 
CC: Kevin Traynor 
CC: Aaron Conole 
Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.")
Signed-off-by: Antonio Fischetti 
---
 lib/netdev-dpdk.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index dc1e9c3..6fc6e1b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
 int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
h, dmp->socket_id, dmp->mtu, dmp->mp_size);
 if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
+VLOG_DBG("snprintf returned %d. Failed to generate a mempool "
+"name for \"%s\". Hash:0x%x, mtu:%d, mbufs:%u.",
+ret, dmp->if_name, h, dmp->mtu, dmp->mp_size);
 return NULL;
 }
 return mp_name;
@@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
*mp_exists)
 
 do {
 char *mp_name = dpdk_mp_name(dmp);
+if (!mp_name) {
+rte_free(dmp);
+return NULL;
+}
 
 VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
   "on socket %d for %d Rx and %d Tx queues.",
-- 
2.4.11

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v8 2/6] Fix mempool names to reflect socket id.

2017-10-19 Thread antonio . fischetti

Create mempool names by considering also the NUMA socket number.
So a name reflects what socket the mempool is allocated on.
This change is needed for the NUMA-awareness feature.

CC: Mark B Kavanagh 
CC: Aaron Conole 
Acked-by: Kevin Traynor 
Reported-by: Ciara Loftus 
Tested-by: Ciara Loftus 
Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.")
Signed-off-by: Antonio Fischetti 
---
Mempool names now contains the requested socket id and become like:
"ovs_4adb057e_1_2030_20512".

Tested with DPDK 17.05.2 (from dpdk-stable branch).
NUMA-awareness feature enabled (DPDK/config/common_base).

Created 1 single dpdkvhostuser port type.
OvS pmd-cpu-mask=FF3 # enable cores on both numa nodes
QEMU core mask = 0xFC000 # cores for qemu on numa node 1 only

 Before launching the VM:
 
ovs-appctl dpif-netdev/pmd-rxq-show
shows core #1 is serving the vhu port.

pmd thread numa_id 0 core_id 1:
isolated : false
port: dpdkvhostuser0queue-id: 0

 After launching the VM:
 ---
the vhu port is now managed by core #27
pmd thread numa_id 1 core_id 27:
isolated : false
port: dpdkvhostuser0queue-id: 0

and the log shows a new mempool is allocated on NUMA node 1, while
the previous one is deleted:

2017-10-06T14:04:55Z|00105|netdev_dpdk|DBG|Allocated 
"ovs_4adb057e_1_2030_20512" mempool with 20512 mbufs
2017-10-06T14:04:55Z|00106|netdev_dpdk|DBG|Releasing 
"ovs_4adb057e_0_2030_20512" mempool
---
 lib/netdev-dpdk.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 45a81f2..7e95f36 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -499,8 +499,8 @@ dpdk_mp_name(struct dpdk_mp *dmp)
 {
 uint32_t h = hash_string(dmp->if_name, 0);
 char *mp_name = xcalloc(RTE_MEMPOOL_NAMESIZE, sizeof *mp_name);
-int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%u",
-   h, dmp->mtu, dmp->mp_size);
+int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
+   h, dmp->socket_id, dmp->mtu, dmp->mp_size);
 if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
 return NULL;
 }
@@ -534,10 +534,11 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
*mp_exists)
 do {
 char *mp_name = dpdk_mp_name(dmp);
 
-VLOG_DBG("Requesting a mempool of %u mbufs for netdev %s "
- "with %d Rx and %d Tx queues.",
- dmp->mp_size, dev->up.name,
- dev->requested_n_rxq, dev->requested_n_txq);
+VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
+  "on socket %d for %d Rx and %d Tx queues.",
+  dev->up.name, dmp->mp_size,
+  dev->requested_socket_id,
+  dev->requested_n_rxq, dev->requested_n_txq);
 
 dmp->mp = rte_pktmbuf_pool_create(mp_name, dmp->mp_size,
   MP_CACHE_SZ,
-- 
2.4.11

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v8 3/6] netdev-dpdk: skip init for existing mempools.

2017-10-19 Thread antonio . fischetti

Skip initialization of mempool packet areas if this was already
done in a previous call to dpdk_mp_create.

CC: Darrell Ball 
CC: Ciara Loftus 
CC: Aaron Conole 
Acked-by: Kevin Traynor 
Acked-by: Mark Kavanagh 
Signed-off-by: Antonio Fischetti 
---
 lib/netdev-dpdk.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 7e95f36..dc1e9c3 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -550,6 +550,11 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
*mp_exists)
 if (dmp->mp) {
 VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,
  dmp->mp_size);
+/* rte_pktmbuf_pool_create has done some initialization of the
+ * rte_mbuf part of each dp_packet. Some OvS specific fields
+ * of the packet still need to be initialized by
+ * ovs_rte_pktmbuf_init. */
+rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
 } else if (rte_errno == EEXIST) {
 /* A mempool with the same name already exists.  We just
  * retrieve its pointer to be returned to the caller. */
@@ -566,11 +571,6 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
*mp_exists)
 }
 free(mp_name);
 if (dmp->mp) {
-/* rte_pktmbuf_pool_create has done some initialization of the
- * rte_mbuf part of each dp_packet, while ovs_rte_pktmbuf_init
- * initializes some OVS specific fields of dp_packet.
- */
-rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
 return dmp;
 }
 } while (!(*mp_exists) &&
-- 
2.4.11

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v8 0/6] netdev-dpdk: Fix mempool management and other cleanup.

2017-10-19 Thread antonio . fischetti

Patch #1, #2 and #4 contain the fixes.
All other patches in this series are a clean up for code readability or small
improvements.

List of versions:
 - v8:
   - Debug message rephrased in patch #2.
   - Reworked patch #4 for snprintf error code.
   - Comments in patch #6 moved into patch #1.

 - v7:
   - Restored 2 separate patches for the 2 fixes.
   - patch #1: detect when previous mempools must be released.
   - patch #2: mempool name generation for NUMA-awareness test case.
   - patch "netdev-dpdk: manage empty mempool names." renamed as
 "netdev-dpdk: manage failure in mempool name creation."
   - Various rework based on comments.

 - v6:
   - patches #1 and #2 squashed into one.
   - Reworked to consider the latest comments.
   - tested the release of pre-existing mempools (reported by Ciara L.)
   - tested the change of MTU when an existing mempool is returned
 (reported by Robert M.)
   - tested the NUMA-Awareness usecase (reported by Ciara L.)
 - v5: manage new MTU value when a pre-existing mempool is returned.
 - v4: fix NUMA awareness usecase
 - v3: avoid deletion of pre-existing mempools
 - v2: rework to accomodate code changes for dpdk ports too
 - v1: 1st implementation.

Fischetti, Antonio (6):
  netdev-dpdk: fix management of pre-existing mempools.
  Fix mempool names to reflect socket id.
  netdev-dpdk: skip init for existing mempools.
  netdev-dpdk: manage failure in mempool name creation.
  netdev-dpdk: Reword mp_size as n_mbufs.
  netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.

 lib/netdev-dpdk.c | 96 ++-
 1 file changed, 59 insertions(+), 37 deletions(-)

-- 
2.4.11

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovs-save: Handle different 'ip addr show' output.

2017-10-19 Thread Gurucharan Shetty

On RHEL 7.4 (with iproute-3.10.0-87), a DHCP provided
ipv4 address has the "dynamic" keyword set. For e.g
"ip addr show breth0 | grep inet" shows:

inet 10.116.248.91/20 brd 10.116.255.255 scope global dynamic breth0
inet6 fe80::250:56ff:fea8:fdf0/64 scope link

The keyword "dynamic" (according to 'man ip-address') is only
used for ipv6, but in this case this is not true. Our current
code will skip the ipv4 address restoration because of this.

With this commit, we special case "dynamic" keyword to be valid
in case of ipv4.

VMware-BZ: #1982196
Signed-off-by: Gurucharan Shetty 
---
 utilities/ovs-lib.in | 8 
 1 file changed, 8 insertions(+)

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 8665698..ac2da85 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -329,6 +329,14 @@ move_ip_address () {
 while test $# != 0; do
 case $1 in
 dynamic)
+# XXX: According to 'man ip-address', "dynamic" is only
+# used for ipv6 addresses.  But, atleast on RHEL 7.4
+# (iproute-3.10.0-87), it is being used for ipv4
+# addresses assigned with dhcp.
+if [ "$family" = "inet" ]; then
+shift
+continue
+fi
 # Omit kernel-maintained route.
 continue 2
 ;;
-- 
1.9.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] datapath-windows: Update OvsIPv4TunnelKey flags in geneve decap.

2017-10-19 Thread Sairam Venugopal

Anand,

Looks like Linux does make a check to see if there are any metadata present 
before setting these flags - 
https://github.com/openvswitch/ovs/blob/master/datapath/linux/compat/geneve.c#L239

Can you verify this and update the patch? (un-ack in meantime??)

Thanks,
Sairam




On 10/18/17, 3:45 PM, "ovs-dev-boun...@openvswitch.org on behalf of Sairam 
Venugopal"  
wrote:

>Thanks for fixing this. Should we check for geneveHdr->opts before setting 
>OVS_TNL_F_GENEVE_OPT?
>
>I know the current code maintains consistency with Linux. But we need to 
>understand if this is intended.
>
>Acked-by: Sairam Venugopal 
>
>
>
>
>
>On 10/17/17, 5:31 PM, "ovs-dev-boun...@openvswitch.org on behalf of Anand 
>Kumar"  
>wrote:
>
>>Set the geneve flags OVS_TNL_F_GENEVE_OPT and OVS_TNL_F_CRT_OPT
>>in OvsDecapGeneve, so that windows behavior is similiar to linux
>>https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs_blob_master_datapath_linux_compat_geneve.c-23L242=DwICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=ntDbsrs3dZtqbGfAIQnQA7qzLK8SlLouMDV36u0JsGQ=TtHsQ-7YSUu8XJHpn4iiQkhaothJamn3dU7_FKuRptE=
>>
>>Signed-off-by: Anand Kumar 
>>---
>> datapath-windows/ovsext/Geneve.c | 8 
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>>diff --git a/datapath-windows/ovsext/Geneve.c 
>>b/datapath-windows/ovsext/Geneve.c
>>index 43374e2..77244b1 100644
>>--- a/datapath-windows/ovsext/Geneve.c
>>+++ b/datapath-windows/ovsext/Geneve.c
>>@@ -324,10 +324,10 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT 
>>switchContext,
>> status = STATUS_NDIS_INVALID_PACKET;
>> goto dropNbl;
>> }
>>-tunKey->flags = OVS_TNL_F_KEY;
>>-if (geneveHdr->oam) {
>>-tunKey->flags |= OVS_TNL_F_OAM;
>>-}
>>+/* Update tunnelKey flags. */
>>+tunKey->flags = OVS_TNL_F_KEY | OVS_TNL_F_GENEVE_OPT |
>>+(geneveHdr->oam ? OVS_TNL_F_OAM : 0) |
>>+(geneveHdr->critical ? OVS_TNL_F_CRT_OPT : 0);
>> tunKey->tunnelId = GENEVE_VNI_TO_TUNNELID(geneveHdr->vni);
>> tunKey->tunOptLen = (uint8)geneveHdr->optLen * 4;
>> if (tunKey->tunOptLen > TUN_OPT_MAX_LEN ||
>>-- 
>>2.9.3.windows.1
>>
>>___
>>dev mailing list
>>d...@openvswitch.org
>>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=ntDbsrs3dZtqbGfAIQnQA7qzLK8SlLouMDV36u0JsGQ=weAfeEKoncNrwUrh3c5va-Efy3yApbOLqPd2LZleZkM=
>___
>dev mailing list
>d...@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=eMSUuuWxH9h1UeKSVIcTwgE8ktXUUIht3ybSd8WphY8=Y6LNJpoXU2BdIsXqef34Gk0al6E4EaHcbISPLhClhVc=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH RFC v2] netdev-dpdk: Allow specification of index for PCI devices

2017-10-19 Thread Kevin Traynor

On 10/17/2017 11:48 AM, Ciara Loftus wrote:
> Some NICs have only one PCI address associated with multiple ports. This
> patch extends the dpdk-devargs option's format to cater for such
> devices. Whereas before only one of N ports associated with one PCI
> address could be added, now N can.
> 
> This patch allows for the following use of the dpdk-devargs option:
> 
> ovs-vsctl set Interface myport options:dpdk-devargs=:06:00.0,X
> 
> Where X is an unsigned integer representing one of multiple ports
> associated with the PCI address provided.
> 
> This patch has not been tested.
> 
> Signed-off-by: Ciara Loftus 
> ---
> v2:
> * Simplify function to find port ID of indexed device
> * Hotplug compatibility
> * Detach compatibility
> * Documentation
> * Use strtol instead of atoi
> 
>  Documentation/howto/dpdk.rst |  9 +
>  Documentation/intro/install/dpdk.rst |  9 +
>  NEWS |  2 ++
>  lib/netdev-dpdk.c| 67 
> ++--
>  vswitchd/vswitch.xml | 11 --
>  5 files changed, 85 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index d123819..9447b71 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -48,6 +48,15 @@ number of dpdk devices found in the log file::
>  $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \
>  options:dpdk-devargs=:01:00.1
>  
> +If your PCI device has multiple ports under the same PCI ID, you can use the
> +following notation to indicate the specific device you wish to add::
> +
> +$ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
> +options:dpdk-devargs=:01:00.0,0
> +
> +The above would attempt to use the first device (0) associated with that PCI
> +ID. Use ,1 ,2 etc. to access the next.
> +
>  After the DPDK ports get added to switch, a polling thread continuously polls
>  DPDK devices and consumes 100% of the core, as can be checked from ``top`` 
> and
>  ``ps`` commands::
> diff --git a/Documentation/intro/install/dpdk.rst 
> b/Documentation/intro/install/dpdk.rst
> index bb69ae5..d0e87f5 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -271,6 +271,15 @@ ports. For example, to create a userspace bridge named 
> ``br0`` and add two
>  DPDK devices will not be available for use until a valid dpdk-devargs is
>  specified.
>  
> +If your PCI device has multiple ports under the same PCI ID, you can use the
> +following notation to indicate the specific device you wish to add::
> +
> +$ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
> +options:dpdk-devargs=:01:00.0,0
> +
> +The above would attempt to use the first device (0) associated with that PCI
> +ID. Use ,1 ,2 etc. to access the next.
> +
>  Refer to ovs-vsctl(8) and :doc:`/howto/dpdk` for more details.
>  
>  Performance Tuning
> diff --git a/NEWS b/NEWS
> index 75f5fa5..ca885a6 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -5,6 +5,8 @@ Post-v2.8.0
> chassis "hostname" in addition to a chassis "name".
> - Linux kernel 4.13
>   * Add support for compiling OVS with the latest Linux 4.13 kernel
> +   - DPDK:
> + * Support for adding devices with multiple DPDK ports under one PCI ID.
>  
>  v2.8.0 - 31 Aug 2017
> - ovs-ofctl:
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index c60f46f..35e15da 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1214,16 +1214,40 @@ netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id)
>  }
>  
>  static dpdk_port_t
> +dpdk_get_port_by_name_with_index(char *name, int idx, int base_id)
> +{
> +struct rte_eth_dev_info dev_info;
> +char pci_addr[PCI_PRI_STR_SIZE];
> +dpdk_port_t offset_port_id = base_id + idx;
> +
> +if (rte_eth_dev_is_valid_port(offset_port_id)) {
> +rte_eth_dev_info_get(offset_port_id, _info);
> +rte_pci_device_name(_info.pci_dev->addr, pci_addr,
> +sizeof(pci_addr));
> +if (!strcmp(pci_addr, name)) {
> +return offset_port_id;
> +}
> +}
> +
> +VLOG_INFO("Invalid PCI offset %i for device %s", idx, name);
> +
> +return DPDK_ETH_PORT_ID_INVALID;
> +}
> +
> +static dpdk_port_t
>  netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
>  const char *devargs, char **errp)
>  {
> -/* Get the name up to the first comma. */
> -char *name = xmemdup0(devargs, strcspn(devargs, ","));
> +char *devargs_copy = xmemdup0((devargs), strlen(devargs));
> +char *name, *idx_str;
> +unsigned idx;
>  dpdk_port_t new_port_id = DPDK_ETH_PORT_ID_INVALID;
>  
> +name = strtok_r(devargs_copy, ",", _copy);
> +idx_str = strtok_r(devargs_copy, ",", _copy);
> +
>  if (!rte_eth_dev_count()
> -||

Re: [ovs-dev] [PATCH net-next v12] openvswitch: enable NSH support

2017-10-19 Thread Jiri Benc

On Thu, 19 Oct 2017 21:12:15 +0800, Yang, Yi wrote:
> flow_key in set_nsh is got from netlink message which is set by
> commit_nsh in user space, here is code.

Isn't this the 'key' local variable that you're talking about, while I'm
referring to the 'flow_key' parameter?

 Jiri
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v12] openvswitch: enable NSH support

2017-10-19 Thread Yang, Yi

On Thu, Oct 19, 2017 at 02:43:47PM +0200, Jiri Benc wrote:
> On Thu, 19 Oct 2017 19:40:53 +0800, Yang, Yi wrote:
> > Actually mdtype can't be set, only push_nsh can set mdtype, so set_nsh
> > won't have mdtype flow key, we can't get it from flow_key in set_nsh,
> > only ttl, flags and path_hdr can be set in set_nsh as you can see in code.
> > I understand your concern is calling skb_ensure_writable twice, so
> > changing the first one to pskb_may_pull is more appropriate for set_nsh.
> 
> Isn't set_nsh called only after the packet was already dissected (i.e.
> parse_nsh called)? The dissected fields should be available in flow_key.

flow_key in set_nsh is got from netlink message which is set by
commit_nsh in user space, here is code.

mask.mdtype = 0; /* Not writable. */
mask.np = 0; /* Not writable. */

if (commit_nsh(_flow->nsh, use_masked, , , ,
sizeof key, odp_actions)) {
put_nsh_key(, base_flow, false);
if (mask.mdtype != 0) { /* Mask was changed by commit(). */
put_nsh_key(, >masks, true);
}
}

Here set is set with mask, so key value is masked by mask.mdtype

/* OVS_KEY_ATTR_NSH keys */
nsh_key_ofs = nl_msg_start_nested(odp_actions,
OVS_KEY_ATTR_NSH);

/* put value and mask for OVS_NSH_KEY_ATTR_BASE */
char *data = nl_msg_put_unspec_uninit(odp_actions,
  OVS_NSH_KEY_ATTR_BASE,
  2 * sizeof(nsh_base));
const char *lkey = (char *)_base, *lmask = (char
*)_base_mask;
size_t lkey_size = sizeof(nsh_base);

while (lkey_size--) {
*data++ = *lkey++ & *lmask++;
}
lmask = (char *)_base_mask;
memcpy(data, lmask, sizeof(nsh_base_mask));

> 
>  Jiri
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v12] openvswitch: enable NSH support

2017-10-19 Thread Jiri Benc

On Thu, 19 Oct 2017 19:40:53 +0800, Yang, Yi wrote:
> Actually mdtype can't be set, only push_nsh can set mdtype, so set_nsh
> won't have mdtype flow key, we can't get it from flow_key in set_nsh,
> only ttl, flags and path_hdr can be set in set_nsh as you can see in code.
> I understand your concern is calling skb_ensure_writable twice, so
> changing the first one to pskb_may_pull is more appropriate for set_nsh.

Isn't set_nsh called only after the packet was already dissected (i.e.
parse_nsh called)? The dissected fields should be available in flow_key.

 Jiri
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack

2017-10-19 Thread Daniel Alvarez Sanchez

System information:
===

OS: CentOS Linux release 7.3.1611 (Core)
Kernel version: 3.10.0-693.2.2.el7.x86_64 #1 SMP
OVS version: v2.8.1  (git tag)
#ovs-vswitchd --version
ovs-vswitchd (Open vSwitch) 2.8.1

Bug description:


Right now, OVN doesn't work using OVS 2.8.1 on Centos 7.3 and conntrack.
Numan Siddique and I have been doing some research on this and we have come
up with the following conclusions:

When doing a DHCP request on the mentioned system above, the kernel throws
the following error (see Reproducer section below):

netlink: Key 26 has unexpected len 16 expected 0

Apparently, this commit [0], introduced that key (26
/OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4 and looks like the OVS modules in the above
kernel doesn't have that key. When ovs-vswitchd sends those extra bytes,
the kernel
module can't find the key and fails with the netlink error above:

2017-10-18T08:00:18Z|00444|netlink_socket|DBG|nl_sock_transact_multiple__
(Success): nl(len:496, type=30(ovs_packet), flags=1[REQUEST], seq=6d,
pid=5939,genl(cmd=3,version=1)

However, if we run OVS master, everything works ok and ovs-vswitchd sends
20 bytes less (4 bytes of the header + 16 bytes of data) so it looks like
it's adapting to the kernel datapath in some way:

2017-10-18T07:59:03Z|00391|netlink_socket|DBG|nl_sock_transact_multiple__
(Success): nl(len:476, type=30(ovs_packet), flags=1[REQUEST], seq=32,
pid=4294962064,genl(cmd=3,version=1)

Note lengths in both cases: 496 vs 476 (working case). In the first case
(496) the kernel throws the netlink error ("netlink: Key 26 has unexpected
len 16 expected 0").

I've checked that running an OVS version up to [1] fixes it but can't find
the exact commit which fixes the current bug.


[0]
https://github.com/openvswitch/ovs/commit/c30b4ceafa235d11a1a9ded5fed11fec86182ee0
[1]
https://github.com/openvswitch/ovs/commit/80cee1163e6301dd1c0bd01c5f0323fb1a45adf4


Reproducer:
=

ovn-nbctl ls-add sw0
ovn-nbctl lsp-add sw0 sw0-port1
ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 192.168.0.2"

ovn-nbctl --wait=hv acl-add sw0 from-lport 1001 'inport == "sw0-port1" &&
ip' allow-related
ovn-nbctl --wait=hv acl-add sw0 to-lport 1001 'outport == "sw0-port1" &&
ip' drop
ovn-nbctl acl-list sw0


add_phys_port() {
name=$1
mac=$2
ip=$3
mask=$4
gw=$5
iface_id=$6
ip netns add $name
ovs-vsctl add-port br-int $name -- set interface $name type=internal
ip link set $name netns $name
ip netns exec $name ip link set $name address $mac
ip netns exec $name ip addr add $ip/$mask dev $name
ip netns exec $name ip link set $name up
ip netns exec $name ip route add default via $gw
ovs-vsctl set Interface $name external_ids:iface-id=$iface_id
}

d1="$(ovn-nbctl create DHCP_Options cidr=192.168.0.0/24 \
options="\"server_id\"=\"192.168.0.1\" \"server_mac\"=\"ff:10:00:00:00:01\"
\
\"lease_time\"=\"3600\" \"router\"=\"192.168.0.1\"")"

ovn-nbctl lsp-set-dhcpv4-options sw0-port1 ${d1}

# when you run the below command it should list the dhcp options just added
ovn-nbctl list dhcp_options

add_phys_port vm1 50:54:00:00:00:01 192.168.0.2 24 192.168.0.1 sw0-port1

# the below command should get the ip address from the OVN
ip netns exec vm1 dhclient -d vm1

At this point, the DHCP request won't succeed and the error can be seen
using
'dmesg'.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 2/6] netdev-dpdk: Fix mempool names to reflect socket id.

2017-10-19 Thread Kevin Traynor

On 10/19/2017 12:45 PM, Fischetti, Antonio wrote:
> Thanks Kevin for your review.
> I could re-spin a new version to take into account your comments on the patch 
> #6.
> I was wondering a better way to rephrase the debug message in this patch.
> Would it be ok something like:
> 
>   VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
>   "on socket %d for %d Rx and %d Tx queues.",
>   dev->up.name, dmp->mp_size, 
>  dev->requested_socket_id,
>   dev->requested_n_rxq, dev->requested_n_txq);
> 

Yep, that reads better, thanks.

> Any suggestion, let me know.
> 
> -Antonio
> 
>> -Original Message-
>> From: Kevin Traynor [mailto:ktray...@redhat.com]
>> Sent: Thursday, October 19, 2017 11:33 AM
>> To: Fischetti, Antonio ; d...@openvswitch.org
>> Cc: Kavanagh, Mark B ; Aaron Conole
>> 
>> Subject: Re: [PATCH v7 2/6] netdev-dpdk: Fix mempool names to reflect socket
>> id.
>>
>> On 10/18/2017 05:01 PM, antonio.fische...@intel.com wrote:
>>> Create mempool names by considering also the NUMA socket number.
>>> So a name reflects what socket the mempool is allocated on.
>>> This change is needed for the NUMA-awareness feature.
>>>
>>> CC: Mark B Kavanagh 
>>> CC: Kevin Traynor 
>>> CC: Aaron Conole 
>>> Reported-by: Ciara Loftus 
>>> Tested-by: Ciara Loftus 
>>> Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
>> port.")
>>> Signed-off-by: Antonio Fischetti 
>>
>> If you have to re-spin anyway I'd make the debug sentence a little more
>> natural sounding, but it's ok as is and I don't think it's worth
>> re-spinning just for that.
>>
>> Otherwise LGTM.
>> Acked-by: Kevin Traynor 
>>
>>
>>> ---
>>> Mempool names now contains the requested socket id and become like:
>>> "ovs_4adb057e_1_2030_20512".
>>>
>>> Tested with DPDK 17.05.2 (from dpdk-stable branch).
>>> NUMA-awareness feature enabled (DPDK/config/common_base).
>>>
>>> Created 1 single dpdkvhostuser port type.
>>> OvS pmd-cpu-mask=FF3 # enable cores on both numa nodes
>>> QEMU core mask = 0xFC000 # cores for qemu on numa node 1 only
>>>
>>>  Before launching the VM:
>>>  
>>> ovs-appctl dpif-netdev/pmd-rxq-show
>>> shows core #1 is serving the vhu port.
>>>
>>> pmd thread numa_id 0 core_id 1:
>>> isolated : false
>>> port: dpdkvhostuser0queue-id: 0
>>>
>>>  After launching the VM:
>>>  ---
>>> the vhu port is now managed by core #27
>>> pmd thread numa_id 1 core_id 27:
>>> isolated : false
>>> port: dpdkvhostuser0queue-id: 0
>>>
>>> and the log shows a new mempool is allocated on NUMA node 1, while
>>> the previous one is deleted:
>>>
>>> 2017-10-06T14:04:55Z|00105|netdev_dpdk|DBG|Allocated
>> "ovs_4adb057e_1_2030_20512" mempool with 20512 mbufs
>>> 2017-10-06T14:04:55Z|00106|netdev_dpdk|DBG|Releasing
>> "ovs_4adb057e_0_2030_20512" mempool
>>> ---
>>> ---
>>>  lib/netdev-dpdk.c | 9 +
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>> index d49afd8..3155505 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -499,8 +499,8 @@ dpdk_mp_name(struct dpdk_mp *dmp)
>>>  {
>>>  uint32_t h = hash_string(dmp->if_name, 0);
>>>  char *mp_name = xcalloc(RTE_MEMPOOL_NAMESIZE, sizeof *mp_name);
>>> -int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%u",
>>> -   h, dmp->mtu, dmp->mp_size);
>>> +int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
>>> +   h, dmp->socket_id, dmp->mtu, dmp->mp_size);
>>>  if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
>>>  return NULL;
>>>  }
>>> @@ -535,9 +535,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
>> *mp_exists)
>>>  char *mp_name = dpdk_mp_name(dmp);
>>>
>>>  VLOG_DBG("Requesting a mempool of %u mbufs for netdev %s "
>>> - "with %d Rx and %d Tx queues.",
>>> + "with %d Rx and %d Tx queues, socket id:%d.",
>>>   dmp->mp_size, dev->up.name,
>>> - dev->requested_n_rxq, dev->requested_n_txq);
>>> + dev->requested_n_rxq, dev->requested_n_txq,
>>> + dev->requested_socket_id);
>>>
>>>  dmp->mp = rte_pktmbuf_pool_create(mp_name, dmp->mp_size,
>>>MP_CACHE_SZ,
>>>
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v12] openvswitch: enable NSH support

2017-10-19 Thread Yang, Yi

On Thu, Oct 19, 2017 at 05:19:55AM +0800, Jiri Benc wrote:
> On Mon, 16 Oct 2017 21:53:29 +0800, Yi Yang wrote:
> > +static int set_nsh(struct sk_buff *skb, struct sw_flow_key *flow_key,
> > +  const struct nlattr *a)
> > +{
> > +   struct nshhdr *nh;
> > +   size_t length;
> > +   int err;
> > +   u8 flags;
> > +   u8 ttl;
> > +   int i;
> > +
> > +   struct ovs_key_nsh key;
> > +   struct ovs_key_nsh mask;
> > +
> > +   err = nsh_key_from_nlattr(a, , );
> > +   if (err)
> > +   return err;
> > +
> > +   /* Make sure the NSH base header is there */
> > +   err = skb_ensure_writable(skb, skb_network_offset(skb) +
> > +  NSH_BASE_HDR_LEN);
> > +   if (unlikely(err))
> > +   return err;
> > +
> > +   nh = nsh_hdr(skb);
> > +   length = nsh_hdr_len(nh);
> > +
> > +   /* Make sure the whole NSH header is there */
> > +   err = skb_ensure_writable(skb, skb_network_offset(skb) +
> > +  length);
> 
> Calling skb_ensure_writable twice is an unnecessary waste in the fast
> path. If anything, the first call should be changed to pskb_may_pull.
> 
> But what we really should do here is to move the header only once. We
> know how much data we're going to write, we have everything stored in
> the key and can calculate it from there.

Actually mdtype can't be set, only push_nsh can set mdtype, so set_nsh
won't have mdtype flow key, we can't get it from flow_key in set_nsh,
only ttl, flags and path_hdr can be set in set_nsh as you can see in code.
I understand your concern is calling skb_ensure_writable twice, so
changing the first one to pskb_may_pull is more appropriate for set_nsh.

> 
>   length = NSH_BASE_HDR_LEN;
>   switch (flow_key->nsh.base.mdtype) {
>   case NSH_MD_TYPE1:
>   length += sizeof(struct ovs_nsh_key_md1);
>   break;
>   case NSH_MD_TYPE2:
>   length += whatever_way_we_store_the_tlvs_in_flow_key;
>   break;
>   }
>   err = skb_ensure_writable(skb, skb_network_offset(skb) + length);
> 
> However, set_nsh does not support MD type 2, thus the second case is a
> dead code. In both switches in this function. As such, it should be
> removed and added only when MD type 2 is introduced. I'd still keep the
> overall logic to ease the future addition, though. This boils down to:
> 
>   length = NSH_BASE_HDR_LEN;
>   /* Assume MD type 1. This function cannot be called for anything
>* else currently. When MD type 2 is added, the line below will
>* have to be turned into a switch on flow_key->nsh.base.mdtype.
>*/
>   length += sizeof(struct ovs_nsh_key_md1);
>   err = skb_ensure_writable(skb, skb_network_offset(skb) + length);
>   ...
>   flow_key->nsh.base.path_hdr = nh->path_hdr;
>   /* Only MD type 1, see the comment above. */
>   for (i = 0; i < NSH_MD1_CONTEXT_SIZE; i++) {
>   ...
> 
> Please verify I'm not missing something.
> 
> It seems we also rely on the user space checking first whether the
> packet is indeed compatible with the pushed key/mask. Most importantly,
> that it's of the same mdtype and (in the future) that the MD type 2
> TLVs being written actually fit. Seems this is done the same way in the
> other set_* actions, thus fine with me.
> 
>  Jiri
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 2/6] netdev-dpdk: Fix mempool names to reflect socket id.

2017-10-19 Thread Fischetti, Antonio

Thanks Kevin for your review.
I could re-spin a new version to take into account your comments on the patch 
#6.
I was wondering a better way to rephrase the debug message in this patch.
Would it be ok something like:

  VLOG_DBG("Port %s: Requesting a mempool of %u mbufs "
  "on socket %d for %d Rx and %d Tx queues.",
  dev->up.name, dmp->mp_size, 
   dev->requested_socket_id,
  dev->requested_n_rxq, dev->requested_n_txq);

Any suggestion, let me know.

-Antonio

> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Thursday, October 19, 2017 11:33 AM
> To: Fischetti, Antonio ; d...@openvswitch.org
> Cc: Kavanagh, Mark B ; Aaron Conole
> 
> Subject: Re: [PATCH v7 2/6] netdev-dpdk: Fix mempool names to reflect socket
> id.
> 
> On 10/18/2017 05:01 PM, antonio.fische...@intel.com wrote:
> > Create mempool names by considering also the NUMA socket number.
> > So a name reflects what socket the mempool is allocated on.
> > This change is needed for the NUMA-awareness feature.
> >
> > CC: Mark B Kavanagh 
> > CC: Kevin Traynor 
> > CC: Aaron Conole 
> > Reported-by: Ciara Loftus 
> > Tested-by: Ciara Loftus 
> > Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each
> port.")
> > Signed-off-by: Antonio Fischetti 
> 
> If you have to re-spin anyway I'd make the debug sentence a little more
> natural sounding, but it's ok as is and I don't think it's worth
> re-spinning just for that.
> 
> Otherwise LGTM.
> Acked-by: Kevin Traynor 
> 
> 
> > ---
> > Mempool names now contains the requested socket id and become like:
> > "ovs_4adb057e_1_2030_20512".
> >
> > Tested with DPDK 17.05.2 (from dpdk-stable branch).
> > NUMA-awareness feature enabled (DPDK/config/common_base).
> >
> > Created 1 single dpdkvhostuser port type.
> > OvS pmd-cpu-mask=FF3 # enable cores on both numa nodes
> > QEMU core mask = 0xFC000 # cores for qemu on numa node 1 only
> >
> >  Before launching the VM:
> >  
> > ovs-appctl dpif-netdev/pmd-rxq-show
> > shows core #1 is serving the vhu port.
> >
> > pmd thread numa_id 0 core_id 1:
> > isolated : false
> > port: dpdkvhostuser0queue-id: 0
> >
> >  After launching the VM:
> >  ---
> > the vhu port is now managed by core #27
> > pmd thread numa_id 1 core_id 27:
> > isolated : false
> > port: dpdkvhostuser0queue-id: 0
> >
> > and the log shows a new mempool is allocated on NUMA node 1, while
> > the previous one is deleted:
> >
> > 2017-10-06T14:04:55Z|00105|netdev_dpdk|DBG|Allocated
> "ovs_4adb057e_1_2030_20512" mempool with 20512 mbufs
> > 2017-10-06T14:04:55Z|00106|netdev_dpdk|DBG|Releasing
> "ovs_4adb057e_0_2030_20512" mempool
> > ---
> > ---
> >  lib/netdev-dpdk.c | 9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index d49afd8..3155505 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -499,8 +499,8 @@ dpdk_mp_name(struct dpdk_mp *dmp)
> >  {
> >  uint32_t h = hash_string(dmp->if_name, 0);
> >  char *mp_name = xcalloc(RTE_MEMPOOL_NAMESIZE, sizeof *mp_name);
> > -int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%u",
> > -   h, dmp->mtu, dmp->mp_size);
> > +int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
> > +   h, dmp->socket_id, dmp->mtu, dmp->mp_size);
> >  if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
> >  return NULL;
> >  }
> > @@ -535,9 +535,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool
> *mp_exists)
> >  char *mp_name = dpdk_mp_name(dmp);
> >
> >  VLOG_DBG("Requesting a mempool of %u mbufs for netdev %s "
> > - "with %d Rx and %d Tx queues.",
> > + "with %d Rx and %d Tx queues, socket id:%d.",
> >   dmp->mp_size, dev->up.name,
> > - dev->requested_n_rxq, dev->requested_n_txq);
> > + dev->requested_n_rxq, dev->requested_n_txq,
> > + dev->requested_socket_id);
> >
> >  dmp->mp = rte_pktmbuf_pool_create(mp_name, dmp->mp_size,
> >MP_CACHE_SZ,
> >

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 5/6] netdev-dpdk: Reword mp_size as n_mbufs.

2017-10-19 Thread Kevin Traynor

On 10/18/2017 05:01 PM, antonio.fische...@intel.com wrote:
> For code readability purposes mp_size is renamed as n_mbufs
> in dpdk_mp structure.
> This parameter is passed to rte mempool creation functions
> and is meant to contain the number of elements inside
> the requested mempool.
> 
> CC: Ciara Loftus 
> CC: Kevin Traynor 
> CC: Aaron Conole 
> Acked-by: Mark Kavanagh 
> Signed-off-by: Antonio Fischetti 
> ---

LGTM.
Acked-by: Kevin Traynor 

>  lib/netdev-dpdk.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index a3b4638..0057e6b 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -308,7 +308,7 @@ struct dpdk_mp {
>  int mtu;
>  int socket_id;
>  char if_name[IFNAMSIZ];
> -unsigned mp_size;
> +unsigned n_mbufs;   /* Number of mbufs inside the mempool. */
>  struct ovs_list list_node OVS_GUARDED_BY(dpdk_mp_mutex);
>  };
>  
> @@ -500,11 +500,11 @@ dpdk_mp_name(struct dpdk_mp *dmp)
>  uint32_t h = hash_string(dmp->if_name, 0);
>  char *mp_name = xcalloc(RTE_MEMPOOL_NAMESIZE, sizeof *mp_name);
>  int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
> -   h, dmp->socket_id, dmp->mtu, dmp->mp_size);
> +   h, dmp->socket_id, dmp->mtu, dmp->n_mbufs);
>  if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
>  VLOG_DBG("Failed to generate a mempool name for \"%s\". "
>  "Hash:0x%x, mtu:%d, mbufs:%u, %s",
> -dmp->if_name, h, dmp->mtu, dmp->mp_size, ovs_strerror(ret));
> +dmp->if_name, h, dmp->mtu, dmp->n_mbufs, ovs_strerror(ret));
>  return NULL;
>  }
>  return mp_name;
> @@ -523,13 +523,13 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>  ovs_strzcpy(dmp->if_name, dev->up.name, IFNAMSIZ);
>  
>  /*
> - * XXX: rough estimation of memory required for port:
> + * XXX: rough estimation of number of mbufs required for this port:
>   * 
>   * + 
>   * + 
>   * + 
>   */
> -dmp->mp_size = dev->requested_n_rxq * dev->requested_rxq_size
> +dmp->n_mbufs = dev->requested_n_rxq * dev->requested_rxq_size
>  + dev->requested_n_txq * dev->requested_txq_size
>  + MIN(RTE_MAX_LCORE, dev->requested_n_rxq) * NETDEV_MAX_BURST
>  + MIN_NB_MBUF;
> @@ -543,11 +543,11 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>  
>  VLOG_DBG("Requesting a mempool of %u mbufs for netdev %s "
>   "with %d Rx and %d Tx queues, socket id:%d.",
> - dmp->mp_size, dev->up.name,
> + dmp->n_mbufs, dev->up.name,
>   dev->requested_n_rxq, dev->requested_n_txq,
>   dev->requested_socket_id);
>  
> -dmp->mp = rte_pktmbuf_pool_create(mp_name, dmp->mp_size,
> +dmp->mp = rte_pktmbuf_pool_create(mp_name, dmp->n_mbufs,
>MP_CACHE_SZ,
>sizeof (struct dp_packet)
>   - sizeof (struct rte_mbuf),
> @@ -556,7 +556,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>dmp->socket_id);
>  if (dmp->mp) {
>  VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,
> - dmp->mp_size);
> + dmp->n_mbufs);
>  /* rte_pktmbuf_pool_create has done some initialization of the
>   * rte_mbuf part of each dp_packet. Some OvS specific fields
>   * of the packet still need to be initialized by
> @@ -574,14 +574,14 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>  *mp_exists = true;
>  } else {
>  VLOG_ERR("Failed mempool \"%s\" create request of %u mbufs",
> - mp_name, dmp->mp_size);
> + mp_name, dmp->n_mbufs);
>  }
>  free(mp_name);
>  if (dmp->mp) {
>  return dmp;
>  }
>  } while (!(*mp_exists) &&
> -(rte_errno == ENOMEM && (dmp->mp_size /= 2) >= MIN_NB_MBUF));
> +(rte_errno == ENOMEM && (dmp->n_mbufs /= 2) >= MIN_NB_MBUF));
>  
>  rte_free(dmp);
>  return NULL;
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 6/6] netdev-dpdk: Rename dpdk_mp_put as dpdk_mp_free.

2017-10-19 Thread Kevin Traynor

On 10/18/2017 05:01 PM, antonio.fische...@intel.com wrote:
> For readability purposes dpdk_mp_put is renamed as dpdk_mp_free.
> Some other comments are also added to mempool functions.
> 
> CC: Mark B Kavanagh 
> CC: Darrell Ball 
> CC: Ciara Loftus 
> CC: Kevin Traynor 
> CC: Aaron Conole 
> Signed-off-by: Antonio Fischetti 
> ---
>  lib/netdev-dpdk.c | 16 +++-
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 0057e6b..411cb06 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -587,6 +587,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>  return NULL;
>  }
>  
> +/* Returns a valid pointer when either of the following is true:
> + *  - a new mempool was just created
> + *  - a matching mempool already exists
> + */

This comment should have been updated when the code has changed, this is
an unrelated commit.

>  static struct dpdk_mp *
>  dpdk_mp_get(struct netdev_dpdk *dev, int mtu, bool *mp_exists)
>  {
> @@ -599,8 +603,9 @@ dpdk_mp_get(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>  return dmp;
>  }
>  
> +/* Release an existing mempool. */
>  static void
> -dpdk_mp_put(struct dpdk_mp *dmp)
> +dpdk_mp_free(struct dpdk_mp *dmp)
>  {
>  char *mp_name;
>  
> @@ -618,8 +623,8 @@ dpdk_mp_put(struct dpdk_mp *dmp)
>  }
>  
>  /* Tries to allocate a new mempool - or re-use an existing one where
> - * appropriate - on requested_socket_id with mbuf size corresponding to
> - * requested_mtu.
> + * appropriate - on requested_socket_id with a size determined by
> + * requested_mtu and requested Rx/Tx queues.

This is re-writing the comment you made in an earlier commit, and not
related to commit subject

>   * On success - or when re-using an existing mempool - the new configuration
>   * will be applied.
>   * On error, device will be left unchanged. */
> @@ -646,7 +651,8 @@ netdev_dpdk_mempool_configure(struct netdev_dpdk *dev)
>  dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
>  return EEXIST;
>  } else {
> -dpdk_mp_put(dev->dpdk_mp);
> +/* A new mempool was created, release the previous one. */
> +dpdk_mp_free(dev->dpdk_mp);
>  dev->dpdk_mp = mp;
>  dev->mtu = dev->requested_mtu;
>  dev->socket_id = dev->requested_socket_id;
> @@ -1091,7 +1097,7 @@ common_destruct(struct netdev_dpdk *dev)
>  OVS_EXCLUDED(dev->mutex)
>  {
>  rte_free(dev->tx_q);
> -dpdk_mp_put(dev->dpdk_mp);
> +dpdk_mp_free(dev->dpdk_mp);
>  
>  ovs_list_remove(>list_node);
>  free(ovsrcu_get_protected(struct ingress_policer *,
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 4/6] netdev-dpdk: manage failure in mempool name creation.

2017-10-19 Thread Kevin Traynor

On 10/18/2017 05:01 PM, antonio.fische...@intel.com wrote:
> In case a mempool name could not be generated log a message
> and return a null mempool pointer to the caller.
> 
> CC: Mark B Kavanagh 
> CC: Darrell Ball 
> CC: Ciara Loftus 
> CC: Kevin Traynor 
> CC: Aaron Conole 
> Signed-off-by: Antonio Fischetti 
> ---
>  lib/netdev-dpdk.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 7c550f2..a3b4638 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -502,6 +502,9 @@ dpdk_mp_name(struct dpdk_mp *dmp)
>  int ret = snprintf(mp_name, RTE_MEMPOOL_NAMESIZE, "ovs_%x_%d_%d_%u",
> h, dmp->socket_id, dmp->mtu, dmp->mp_size);
>  if (ret < 0 || ret >= RTE_MEMPOOL_NAMESIZE) {
> +VLOG_DBG("Failed to generate a mempool name for \"%s\". "
> +"Hash:0x%x, mtu:%d, mbufs:%u, %s",
> +dmp->if_name, h, dmp->mtu, dmp->mp_size, ovs_strerror(ret));

ovs_strerror(ret) look to be wrong

>  return NULL;
>  }
>  return mp_name;
> @@ -533,6 +536,10 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>  
>  do {
>  char *mp_name = dpdk_mp_name(dmp);
> +if (!mp_name) {
> +rte_free(dmp);
> +return NULL;
> +}

This looks like a fix, so should have Fixes: in commit message.

>  
>  VLOG_DBG("Requesting a mempool of %u mbufs for netdev %s "
>   "with %d Rx and %d Tx queues, socket id:%d.",
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 3/6] netdev-dpdk: skip init for existing mempools.

2017-10-19 Thread Kevin Traynor

On 10/18/2017 05:01 PM, antonio.fische...@intel.com wrote:
> Skip initialization of mempool packet areas if this was already
> done in a previous call to dpdk_mp_create.
> 
> CC: Darrell Ball 
> CC: Ciara Loftus 
> CC: Kevin Traynor 
> CC: Aaron Conole 
> Acked-by: Mark Kavanagh 
> Signed-off-by: Antonio Fischetti 
> ---

LGTM
Acked-by: Kevin Traynor 

>  lib/netdev-dpdk.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 3155505..7c550f2 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -550,6 +550,11 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>  if (dmp->mp) {
>  VLOG_DBG("Allocated \"%s\" mempool with %u mbufs", mp_name,
>   dmp->mp_size);
> +/* rte_pktmbuf_pool_create has done some initialization of the
> + * rte_mbuf part of each dp_packet. Some OvS specific fields
> + * of the packet still need to be initialized by
> + * ovs_rte_pktmbuf_init. */
> +rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
>  } else if (rte_errno == EEXIST) {
>  /* A mempool with the same name already exists.  We just
>   * retrieve its pointer to be returned to the caller. */
> @@ -566,11 +571,6 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool 
> *mp_exists)
>  }
>  free(mp_name);
>  if (dmp->mp) {
> -/* rte_pktmbuf_pool_create has done some initialization of the
> - * rte_mbuf part of each dp_packet, while ovs_rte_pktmbuf_init
> - * initializes some OVS specific fields of dp_packet.
> - */
> -rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
>  return dmp;
>  }
>  } while (!(*mp_exists) &&
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 1/6] netdev-dpdk: fix management of pre-existing mempools.

2017-10-19 Thread Kevin Traynor

On 10/18/2017 05:01 PM, antonio.fische...@intel.com wrote:
> Fix an issue on reconfiguration of pre-existing mempools.
> This patch avoids to call dpdk_mp_put() - and erroneously
> release the mempool - when it already exists.
> 
> CC: Mark B Kavanagh 
> CC: Kevin Traynor 
> CC: Aaron Conole 
> CC: Darrell Ball 
> Reported-by: Ciara Loftus 
> Tested-by: Ciara Loftus 
> Reported-by: Róbert Mulik 
> Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each 
> port.")
> Signed-off-by: Antonio Fischetti 
> ---

LGTM

Acked-by: Kevin Traynor 

> I've tested this patch by
>   - changing at run-time the number of Rx queues:
>   ovs-vsctl set Interface dpdk0 type=dpdk options:n_rxq=4
> 
>   - reducing the MTU of the dpdk ports of 1 byte to force
> the configuration of an existing mempool:
>   ovs-vsctl set Interface dpdk0 mtu_request=1499
> 
> This issue was observed in a PVP test topology with dpdkvhostuserclient
> ports. It can happen also with dpdk type ports, eg by reducing the MTU
> of 1 byte.
> 
> To replicate the bug scenario in the PVP case it's sufficient to
> set 1 dpdkvhostuserclient port, and just boot the VM.
> 
> Below some more details on my own test setup.
> 
>  PVP test setup
>  --
> CLIENT_SOCK_DIR=/tmp
> SOCK0=dpdkvhostuser0
> SOCK1=dpdkvhostuser1
> 
> 1 PMD
> Add 2 dpdk ports, n_rxq=1
> Add 2 vhu ports both of type dpdkvhostuserclient and specify vhost-server-path
>  ovs-vsctl set Interface dpdkvhostuser0 
> options:vhost-server-path="$CLIENT_SOCK_DIR/$SOCK0"
>  ovs-vsctl set Interface dpdkvhostuser1 
> options:vhost-server-path="$CLIENT_SOCK_DIR/$SOCK1"
> 
> Set port-based rules: dpdk0 <--> vhu0 and dpdk1 <--> vhu1
>  add-flow br0 in_port=1,action=output:3
>  add-flow br0 in_port=3,action=output:1
>  add-flow br0 in_port=4,action=output:2
>  add-flow br0 in_port=2,action=output:4
> 
>  Launch QEMU
>  ---
> As OvS vhu ports are acting as clients, we must specify 'server' in the next 
> command.
> VM_IMAGE=
> 
>  sudo -E taskset 0x3F00 $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 -name 
> us-vhost-vm1 -cpu host -enable-kvm -m 4096M -object 
> memory-backend-file,id=mem,size=4096M,mem-path=/dev/hugepages,share=on -numa 
> node,memdev=mem -mem-prealloc -smp 4 -drive file=$VM_IMAGE -chardev 
> socket,id=char0,path=$CLIENT_SOCK_DIR/$SOCK0,server -netdev 
> type=vhost-user,id=mynet1,chardev=char0,vhostforce -device 
> virtio-net-pci,mac=00:00:00:00:00:01,netdev=mynet1,mrg_rxbuf=off -chardev 
> socket,id=char1,path=$CLIENT_SOCK_DIR/$SOCK1,server -netdev 
> type=vhost-user,id=mynet2,chardev=char1,vhostforce -device 
> virtio-net-pci,mac=00:00:00:00:00:02,netdev=mynet2,mrg_rxbuf=off --nographic
> 
>  Expected behavior
>  -
> With this fix OvS shouldn't crash.
> ---
>  lib/netdev-dpdk.c | 41 +
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index c60f46f..d49afd8 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -508,12 +508,13 @@ dpdk_mp_name(struct dpdk_mp *dmp)
>  }
>  
>  static struct dpdk_mp *
> -dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
> +dpdk_mp_create(struct netdev_dpdk *dev, int mtu, bool *mp_exists)
>  {
>  struct dpdk_mp *dmp = dpdk_rte_mzalloc(sizeof *dmp);
>  if (!dmp) {
>  return NULL;
>  }
> +*mp_exists = false;
>  dmp->socket_id = dev->requested_socket_id;
>  dmp->mtu = mtu;
>  ovs_strzcpy(dmp->if_name, dev->up.name, IFNAMSIZ);
> @@ -530,8 +531,6 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
>  + MIN(RTE_MAX_LCORE, dev->requested_n_rxq) * NETDEV_MAX_BURST
>  + MIN_NB_MBUF;
>  
> -bool mp_exists = false;
> -
>  do {
>  char *mp_name = dpdk_mp_name(dmp);
>  
> @@ -559,7 +558,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
>  /* As the mempool create returned EEXIST we can expect the
>   * lookup has returned a valid pointer.  If for some reason
>   * that's not the case we keep track of it. */
> -mp_exists = true;
> +*mp_exists = true;
>  } else {
>  VLOG_ERR("Failed mempool \"%s\" create request of %u mbufs",
>   mp_name, dmp->mp_size);
> @@ -573,7 +572,7 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
>  rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
>  return dmp;
>  }
> -} while (!mp_exists &&
> +} while (!(*mp_exists) &&
>  (rte_errno == ENOMEM && (dmp->mp_size /= 2) >= MIN_NB_MBUF));
>  
>  rte_free(dmp);
> @@ -581,12 +580,12 @@ dpdk_mp_create(struct netdev_dpdk *dev, int mtu)
>  }
>  
>  static struct dpdk_mp *
> -dpdk_mp_get(struct

Re: [ovs-dev] [PATCH v3 5/5] doc: ConnTracker cfg parameters.

2017-10-19 Thread Stephen Finucane

On Fri, 2017-10-13 at 09:25 +0100, antonio.fische...@intel.com wrote:
> Update documentation with the new commands to Read/Write
> ConnTracker configuration parameters.
> 
> CC: Kevin Traynor 
> CC: Darrell Ball 
> Signed-off-by: Antonio Fischetti 

One nit below, but otherwise LGTM.

Acked-by: Stephen Finucane 

> ---
>  Documentation/intro/install/dpdk.rst | 25 +
>  lib/dpctl.man| 10 ++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/Documentation/intro/install/dpdk.rst
> b/Documentation/intro/install/dpdk.rst
> index bb69ae5..a1f259c 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -568,6 +568,31 @@ not needed i.e. jumbo frames are not needed, it can be
> forced off by adding
>  chains of descriptors it will make more individual virtio descriptors
> available
>  for rx to the guest using dpdkvhost ports and this can improve performance.
>  
> +Connection Tracker
> +~~
> +
> +When the Connection Tracker is enabled the overall performance can be deeply
> +affected, even with simple firewall rules and with stateless protocols like
> +UDP.  In order to find a better tuning, commands like
> +
> +::
> +
> +$ ovs-appctl dpctl/ct-get-glbl-cfg 
> +$ ovs-appctl dpctl/ct-set-glbl-cfg =
> +
> +allow respectively to read the current value, or set a new value to a
> +configuration parameter.
> +For example, to reduce the impact of the Connection Tracker load on the
> +system performance, the maximum number of tracked connections can be
> +reduced.
> +
> +The available configuration parameters are:
> +
> +- maxconn: Maximum number of connections managed by the Connection Tracker
> +  module. It's both readable and writeable.
> +- totconn: Total number of connections currently managed by the Connection
> +  Tracker module. Readable only.

nit: This section would probably read better as a definition list

  ``maxconn``
Maximum number of connections...

  ``totconn``
Total number of connections...

> +
>  Limitations
>  
>  
> diff --git a/lib/dpctl.man b/lib/dpctl.man
> index 675fe5a..64ad105 100644
> --- a/lib/dpctl.man
> +++ b/lib/dpctl.man
> @@ -235,3 +235,13 @@ For each ConnTracker bucket, displays the number of
> connections used
>  by \fIdp\fR.
>  If \fBgt=\fIThreshold\fR is specified, bucket numbers are displayed when
>  the number of connections in a bucket is greater than \fIThreshold\fR.
> +.
> +.TP
> +\*(DX\fBct\-get\-glbl\-cfg\fR [\fIdp\fR] \fBparam\fR
> +Read the current value of the specified ConnTracker parameter used
> +by \fIdp\fR.
> +.
> +.TP
> +\*(DX\fBct\-set\-glbl\-cfg\fR [\fIdp\fR] \fBparam=\fI..\fR
> +Set a value to the specified ConnTracker parameter used
> +by \fIdp\fR.

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Documentation: Add Faucet tutorial.

2017-10-19 Thread Stephen Finucane

On Wed, 2017-10-18 at 14:05 -0700, Ben Pfaff wrote:
> This is for a talk at the Faucet conference on Oct. 19:
> http://conference.faucet.nz/schedule/
> 
> Signed-off-by: Ben Pfaff 

Spotted a few small issues while skimming this. Did this build correctly and
did the output HTML look OK?

Stephen

> ---
>  Documentation/automake.mk  |1 +
>  Documentation/tutorials/faucet.rst | 1462
> 
>  Documentation/tutorials/index.rst  |1 +
>  3 files changed, 1464 insertions(+)
>  create mode 100644 Documentation/tutorials/faucet.rst
> 
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index 6f38912f264b..da482b419887 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -23,6 +23,7 @@ DOC_SOURCE = \
>   Documentation/intro/install/windows.rst \
>   Documentation/intro/install/xenserver.rst \
>   Documentation/tutorials/index.rst \
> + Documentation/tutorials/faucet.rst \
>   Documentation/tutorials/ovs-advanced.rst \
>   Documentation/tutorials/ovn-openstack.rst \
>   Documentation/tutorials/ovn-sandbox.rst \
> diff --git a/Documentation/tutorials/faucet.rst
> b/Documentation/tutorials/faucet.rst
> new file mode 100644
> index ..67c663649f72
> --- /dev/null
> +++ b/Documentation/tutorials/faucet.rst
> @@ -0,0 +1,1462 @@
> +..
> +  Licensed under the Apache License, Version 2.0 (the "License"); you
> may
> +  not use this file except in compliance with the License. You may
> obtain
> +  a copy of the License at
> +
> +  http://www.apache.org/licenses/LICENSE-2.0
> +
> +  Unless required by applicable law or agreed to in writing, software
> +  distributed under the License is distributed on an "AS IS" BASIS,
> WITHOUT
> +  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See
> the
> +  License for the specific language governing permissions and
> limitations
> +  under the License.
> +
> +  Convention for heading levels in Open vSwitch documentation:
> +
> +  ===  Heading 0 (reserved for the title in a document)
> +  ---  Heading 1
> +  ~~~  Heading 2
> +  +++  Heading 3
> +  '''  Heading 4
> +
> +  Avoid deeper levels because they do not render well.
> +
> +===
> +OVS Faucet Tutorial
> +===
> +
> +This tutorial demonstrates how Open vSwitch works with a controller, using
> the
> +Faucet controller as a simple way to get started.  It was tested with the
> +"master" branch of Open vSwitch and version 1.6.7 of Faucet in October 2017.
> +It does not use advanced or recently added features in OVS or Faucet, so
> other
> +versions of both pieces of software are likely to work equally well.
> +
> +The goal of the tutorial is to demonstrate Open vSwitch and Faucet in an
> +end-to-end way, that is, to show how it works from the Faucet controller
> +configuration at the top, through the OpenFlow flow table, to the datapath
> +processing.  Along the way, in addition to helping to understand the
> +architecture at each level, we discuss performance and troubleshooting
> issues.
> +We hope that this demonstration makes it easier for users and potential
> users
> +to understand how Open vSwitch works and how to debug and troubleshoot it.
> +
> +We provide enough details in the tutorial that you should be able to fully
> +follow along by following the instructions.
> +
> +Setting Up OVS
> +--
> +
> +This section explains to how to set up Open vSwitch for the purpose of using
> it
> +with Faucet for the tutorial.
> +
> +You might already have Open vSwitch installed on one or more computers or
> VMs,
> +perhaps set up to control a set of VMs or a physical network.  This is
> +admirable, but we will be using Open vSwitch in a different way to set up a
> +simulation environment called the OVS "sandbox".  The sandbox does not use
> +virtual machines or containers, which makes it more limited than setups
> based
> +on those kinds of setups, but on the other hand it is also (in this writer's
> +opinion) easier to set up.
> +
> +There are two ways to start a sandbox: one that uses the Open vSwitch that
> is
> +already installed on a system, and another that uses a copy of Open vSwitch
> +that has been built but not yet installed.  The latter is more often used
> and
> +thus better tested, but both should work.  The instructions below explain
> both
> +approaches:
> +
> +1. Get a copy of the Open vSwitch source repository using Git, then ``cd``
> into
> +   the new directory::
> +
> + $ git clone https://github.com/openvswitch/ovs.git
> + $ cd ovs
> +
> +   The default checkout is the master branch.  You can check out a tag
> +   (such as v2.8.0) or a branch (such as origin/branch-2.8), if you
> +   prefer.
> +
> +2. If you do not already have an installed copy of Open vSwitch on your
> system,
> +   or if you do not want to use it for the

[ovs-dev] 2.8.0: STP - flush the fdb and mdb when topology changed failure on s390x

2017-10-19 Thread Ilya Maximets

Hi James.
I had same issue while building with '-march=native' on x86 system.
Changing the repeated time warps to a single long warp almost solved
the issue for me, but I think that testcase is not completely right
and it still rarely fails.

That's the patch I prepared last month for that:
* https://mail.openvswitch.org/pipermail/ovs-dev/2017-September/338708.html

Maybe it'll help or my case will give you some hints about the root cause.
Unfortunately I had no much time to investigate the issue deeply.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Documentation: Add Faucet tutorial.

2017-10-19 Thread Han Zhou

On Wed, Oct 18, 2017 at 2:05 PM, Ben Pfaff  wrote:

> This is for a talk at the Faucet conference on Oct. 19:
> http://conference.faucet.nz/schedule/
>
> Signed-off-by: Ben Pfaff 
> ---
>  Documentation/automake.mk  |1 +
>  Documentation/tutorials/faucet.rst | 1462 ++
> ++
>  Documentation/tutorials/index.rst  |1 +
>  3 files changed, 1464 insertions(+)
>  create mode 100644 Documentation/tutorials/faucet.rst
>
> diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> index 6f38912f264b..da482b419887 100644
> --- a/Documentation/automake.mk
> +++ b/Documentation/automake.mk
> @@ -23,6 +23,7 @@ DOC_SOURCE = \
> Documentation/intro/install/windows.rst \
> Documentation/intro/install/xenserver.rst \
> Documentation/tutorials/index.rst \
> +   Documentation/tutorials/faucet.rst \
> Documentation/tutorials/ovs-advanced.rst \
> Documentation/tutorials/ovn-openstack.rst \
> Documentation/tutorials/ovn-sandbox.rst \
> diff --git a/Documentation/tutorials/faucet.rst b/Documentation/tutorials/
> faucet.rst
> new file mode 100644
> index ..67c663649f72
> --- /dev/null
> +++ b/Documentation/tutorials/faucet.rst
> @@ -0,0 +1,1462 @@
> +..
> +  Licensed under the Apache License, Version 2.0 (the "License"); you
> may
> +  not use this file except in compliance with the License. You may
> obtain
> +  a copy of the License at
> +
> +  http://www.apache.org/licenses/LICENSE-2.0
> +
> +  Unless required by applicable law or agreed to in writing, software
> +  distributed under the License is distributed on an "AS IS" BASIS,
> WITHOUT
> +  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> See the
> +  License for the specific language governing permissions and
> limitations
> +  under the License.
> +
> +  Convention for heading levels in Open vSwitch documentation:
> +
> +  ===  Heading 0 (reserved for the title in a document)
> +  ---  Heading 1
> +  ~~~  Heading 2
> +  +++  Heading 3
> +  '''  Heading 4
> +
> +  Avoid deeper levels because they do not render well.
> +
> +===
> +OVS Faucet Tutorial
> +===
> +
> +This tutorial demonstrates how Open vSwitch works with a controller,
> using the
>

s/controller/general purpose OpenFlow controller

Otherwise, people may get confused why wouldn't OVN demonstrate the same.


> +Faucet controller as a simple way to get started.  It was tested with the
>
+"master" branch of Open vSwitch and version 1.6.7 of Faucet in October
> 2017.
> +It does not use advanced or recently added features in OVS or Faucet, so
> other
> +versions of both pieces of software are likely to work equally well.
> +
> +The goal of the tutorial is to demonstrate Open vSwitch and Faucet in an
> +end-to-end way, that is, to show how it works from the Faucet controller
> +configuration at the top, through the OpenFlow flow table, to the datapath
> +processing.  Along the way, in addition to helping to understand the
> +architecture at each level, we discuss performance and troubleshooting
> issues.
> +We hope that this demonstration makes it easier for users and potential
> users
> +to understand how Open vSwitch works and how to debug and troubleshoot it.
> +
> +We provide enough details in the tutorial that you should be able to fully
> +follow along by following the instructions.
> +
> +Setting Up OVS
> +--
> +
> +This section explains to how to set up Open vSwitch for the purpose of
> using it
> +with Faucet for the tutorial.
> +
> +You might already have Open vSwitch installed on one or more computers or
> VMs,
> +perhaps set up to control a set of VMs or a physical network.  This is
> +admirable, but we will be using Open vSwitch in a different way to set up
> a
> +simulation environment called the OVS "sandbox".  The sandbox does not use
> +virtual machines or containers, which makes it more limited than setups
> based
> +on those kinds of setups, but on the other hand it is also (in this
> writer's
> +opinion) easier to set up.
> +
> +There are two ways to start a sandbox: one that uses the Open vSwitch
> that is
> +already installed on a system, and another that uses a copy of Open
> vSwitch
> +that has been built but not yet installed.  The latter is more often used
> and
> +thus better tested, but both should work.  The instructions below explain
> both
> +approaches:
> +
> +1. Get a copy of the Open vSwitch source repository using Git, then
> ``cd`` into
> +   the new directory::
> +
> + $ git clone https://github.com/openvswitch/ovs.git
> + $ cd ovs
> +
> +   The default checkout is the master branch.  You can check out a tag
> +   (such as v2.8.0) or a branch (such as origin/branch-2.8), if you
> +   prefer.
> +
> +2. If you do not already have an installed copy of Open vSwitch on your
>

38 matches

Mail list logo