[ovs-dev] [PATCH v1 ] Make 100 percents packets sampled when sampling rate is 1.
When sampling rate is 1, the sampling probability is UINT32_MAX. The packet
should be sampled even the prandom32() generate the number of UINT32_MAX.
And no packet need be sampled when the probability is 0.
Signed-off-by: Wenyu Zhang
---
datapath/actions.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index 4941ca0..456f56b 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -675,12 +675,14 @@ static int sample(struct datapath *dp, struct sk_buff
*skb,
const struct nlattr *acts_list = NULL;
const struct nlattr *a;
int rem;
+ uint32_t probability;
for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
a = nla_next(a, &rem)) {
switch (nla_type(a)) {
case OVS_SAMPLE_ATTR_PROBABILITY:
- if (prandom_u32() >= nla_get_u32(a))
+ probability = nla_get_u32(a);
+ if (!probability || (prandom_u32() > probability))
return 0;
break;
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: Retry tx/rx queue setup until we don't get any failure.
So ideally this will be fixed in a future release of DPDK. We have flagged this. However that solution will not be in place until the DPDK 2.2 release in December at the earliest (DPDK 2.1 is currently in release candidate mode at the moment so it won't make it to that). When this has been changed in DPDK we can revisit the OVS code. Technically DPDK is doing what it is supposed to with the current implementation i.e. it is returning the max number of queues it supports. From the OVS side I think we need to understand that this has a different connotation to what it had with previously with NICS in terms of how many of those queues are usable. Unfortunately I don’t see another way to negotiate the tx queue initialization without something like the patch below. Not until we have more explicit configuration details available for the HW device from DPDK. Thanks Ian > -Original Message- > From: Ethan Jackson [mailto:et...@nicira.com] > Sent: Wednesday, July 29, 2015 10:13 PM > To: Stokes, Ian > Cc: Traynor, Kevin; Daniele Di Proietto; dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: Retry tx/rx queue setup > until we don't get any failure. > > Sorry for taking so long to get to this. The one question I have is: > Is OVS the right layer to be fixing this? Isn't this really an issue > of DPDK reporting a number of available queues that for practical > purposes is wrong? I.E. Shouldn't this be fixed by the DPDK driver of > this system? This patch feels like a hack to me . . . > > Ethan > > On Tue, Jul 28, 2015 at 2:36 AM, Stokes, Ian > wrote: > > Hi all, > > > > Just wondering what the status of this patch is? Is there any feedback > > or queries we can answer to help? > > > > Thanks > > Ian > > > >> -Original Message- > >> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Traynor, > >> Kevin > >> Sent: Thursday, July 23, 2015 11:28 AM > >> To: Daniele Di Proietto; dev@openvswitch.org > >> Subject: Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: Retry tx/rx queue > setup > >> until we don't get any failure. > >> > >> > >> > -Original Message- > >> > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele > Di > >> > Proietto > >> > Sent: Thursday, July 16, 2015 7:48 PM > >> > To: dev@openvswitch.org > >> > Subject: [ovs-dev] [PATCH 2/2] netdev-dpdk: Retry tx/rx queue setup > >> until we > >> > don't get any failure. > >> > > >> > It has been observed that some DPDK device (e.g intel xl710) report > an > >> > high number of queues but make some of them available only for > special > >> > functions (SRIOV). Therefore the queues will be counted in > >> > rte_eth_dev_info_get(), but rte_eth_tx_queue_setup() will fail. > >> > > >> > This commit works around the issue by retrying the device > >> initialization > >> > with a smaller number of queues, if a queue fails to setup. > >> > > >> > Reported-by: Ian Stokes > >> > Signed-off-by: Daniele Di Proietto > >> > --- > >> > lib/netdev-dpdk.c | 100 +++--- > --- > >> --- > >> > -- > >> > 1 file changed, 73 insertions(+), 27 deletions(-) > >> > >> > >> Acked-by: Kevin Traynor > >> ___ > >> dev mailing list > >> dev@openvswitch.org > >> http://openvswitch.org/mailman/listinfo/dev > > ___ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Openflow 1.5 - Egress Tables [EXT-306]
Hi, I want to implement and contribute Egress Tables [EXT-306], Openflow 1.5 feature to OpenVSwitch. If anybody has already started working on it then please let us know. Thanks and Regards, Saloni Jain Tata Consultancy Services Mailto: saloni.j...@tcs.com Website: http://www.tcs.com Experience certainty. IT Services Business Solutions Consulting =-=-= Notice: The information contained in this e-mail message and/or attachments to it may contain confidential or privileged information. If you are not the intended recipient, any dissemination, use, review, distribution, printing or copying of the information contained in this e-mail message and/or attachments to it are strictly prohibited. If you have received this communication in error, please notify us by reply e-mail or telephone and immediately and permanently delete the message and any attachments. Thank you ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] OpenStack -- Add a Layer2 Device between two VMs
Hi, I'm looking for an external Layer2 Device integration within the OpenStack. This Layer2 device can have only 2 physical cable 1 for input and 1 for output to enable a network interception and then blocking the traffic based on the analysis. The whole of the OpenStack setup is comprised of 3 VMs itself. Controller + Compute1 + Network. so far the instances are able to do what I'm looking for but without network interception by my physical board. Here is the pic. VM --> Layer2 Device --> VM2 +VM3 This Layer2 device has to be physically connected which I have achieved by exposing the two port net work card to compute node and then adding it a new bridge so that the traffic should pass through it solely. PS. I can't assign IP addresses to my Layer2 Device's network ports. its a dumb device. Any idea how I can create a new bridge and then embed the external device on it . OR Is it possible to send the Layer2 over IP to a Cisco Switch's 2 ports so that I can send and recieve data from there in a mode which should enable network blocking by an external device. PS. I can't assign IP addresses to my Layer2 Device's network ports. its a dumb device. Regards, -- Nasir Mahmood ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] dpif: allow adding ukeys for same flow by different pmds
In multiqueue mode several pmd threads may process one
port, but different queues. Flow doesn't depend on queue.
So, while miss upcall processing, all threads (except first
for that port) will receive error = ENOSPC due to
ukey_install failure. Therefore they will not add the flow
to flow_table and will not insert it to exact match cache.
As a result all threads (except first for that port) will
always execute a miss.
Fix that by comparing ukeys not only by ufids but also
by pmd_ids.
Signed-off-by: Ilya Maximets
---
ofproto/ofproto-dpif-upcall.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 440f9e9..38e03c5 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -286,7 +286,8 @@ static bool ukey_install_start(struct udpif *, struct
udpif_key *ukey);
static bool ukey_install_finish(struct udpif_key *ukey, int error);
static bool ukey_install(struct udpif *udpif, struct udpif_key *ukey);
static struct udpif_key *ukey_lookup(struct udpif *udpif,
- const ovs_u128 *ufid);
+ const ovs_u128 *ufid,
+ const unsigned pmd_id);
static int ukey_acquire(struct udpif *, const struct dpif_flow *,
struct udpif_key **result, int *error);
static void ukey_delete__(struct udpif_key *);
@@ -1141,7 +1142,8 @@ process_upcall(struct udpif *udpif, struct upcall *upcall,
}
if (actions_len == 0) {
/* Lookup actions in userspace cache. */
-struct udpif_key *ukey = ukey_lookup(udpif, upcall->ufid);
+struct udpif_key *ukey = ukey_lookup(udpif, upcall->ufid,
+ upcall->pmd_id);
if (ukey) {
actions = ukey->actions->data;
actions_len = ukey->actions->size;
@@ -1324,14 +1326,14 @@ get_ufid_hash(const ovs_u128 *ufid)
}
static struct udpif_key *
-ukey_lookup(struct udpif *udpif, const ovs_u128 *ufid)
+ukey_lookup(struct udpif *udpif, const ovs_u128 *ufid, const unsigned pmd_id)
{
struct udpif_key *ukey;
int idx = get_ufid_hash(ufid) % N_UMAPS;
struct cmap *cmap = &udpif->ukeys[idx].cmap;
CMAP_FOR_EACH_WITH_HASH (ukey, cmap_node, get_ufid_hash(ufid), cmap) {
-if (ovs_u128_equals(&ukey->ufid, ufid)) {
+if (ovs_u128_equals(&ukey->ufid, ufid) && ukey->pmd_id == pmd_id) {
return ukey;
}
}
@@ -1488,7 +1490,7 @@ ukey_install_start(struct udpif *udpif, struct udpif_key
*new_ukey)
idx = new_ukey->hash % N_UMAPS;
umap = &udpif->ukeys[idx];
ovs_mutex_lock(&umap->mutex);
-old_ukey = ukey_lookup(udpif, &new_ukey->ufid);
+old_ukey = ukey_lookup(udpif, &new_ukey->ufid, new_ukey->pmd_id);
if (old_ukey) {
/* Uncommon case: A ukey is already installed with the same UFID. */
if (old_ukey->key_len == new_ukey->key_len
@@ -1570,7 +1572,7 @@ ukey_acquire(struct udpif *udpif, const struct dpif_flow
*flow,
struct udpif_key *ukey;
int retval;
-ukey = ukey_lookup(udpif, &flow->ufid);
+ukey = ukey_lookup(udpif, &flow->ufid, flow->pmd_id);
if (ukey) {
retval = ovs_mutex_trylock(&ukey->mutex);
} else {
--
2.1.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] Start fixing some Python 3 compatibility issues
This patch fixes just the Python 3 problems found by running
python3 setup.py install
There are still many other issues to be fixed, but this is a start.
Signed-off-by: Terry Wilson
---
python/ovs/daemon.py | 31 ---
python/ovs/db/idl.py | 10 +-
python/ovs/fatal_signal.py | 2 +-
python/ovs/json.py | 2 +-
python/ovs/ovsuuid.py | 2 +-
python/ovs/poller.py | 2 +-
python/ovs/socket_util.py | 38 +++---
python/ovs/stream.py | 12 +++-
python/setup.py| 2 +-
9 files changed, 52 insertions(+), 49 deletions(-)
diff --git a/python/ovs/daemon.py b/python/ovs/daemon.py
index 4a704c3..1e84dd2 100644
--- a/python/ovs/daemon.py
+++ b/python/ovs/daemon.py
@@ -133,30 +133,30 @@ def _make_pidfile():
global file_handle
file_handle = open(tmpfile, "w")
-except IOError, e:
+except IOError as e:
_fatal("%s: create failed (%s)" % (tmpfile, e.strerror))
try:
s = os.fstat(file_handle.fileno())
-except IOError, e:
+except IOError as e:
_fatal("%s: fstat failed (%s)" % (tmpfile, e.strerror))
try:
file_handle.write("%s\n" % pid)
file_handle.flush()
-except OSError, e:
+except OSError as e:
_fatal("%s: write failed: %s" % (tmpfile, e.strerror))
try:
fcntl.lockf(file_handle, fcntl.LOCK_EX | fcntl.LOCK_NB)
-except IOError, e:
+except IOError as e:
_fatal("%s: fcntl failed: %s" % (tmpfile, e.strerror))
# Rename or link it to the correct name.
if _overwrite_pidfile:
try:
os.rename(tmpfile, _pidfile)
-except OSError, e:
+except OSError as e:
_fatal("failed to rename \"%s\" to \"%s\" (%s)"
% (tmpfile, _pidfile, e.strerror))
else:
@@ -164,7 +164,7 @@ def _make_pidfile():
try:
os.link(tmpfile, _pidfile)
error = 0
-except OSError, e:
+except OSError as e:
error = e.errno
if error == errno.EEXIST:
_check_already_running()
@@ -200,7 +200,7 @@ def _waitpid(pid, options):
while True:
try:
return os.waitpid(pid, options)
-except OSError, e:
+except OSError as e:
if e.errno == errno.EINTR:
pass
return -e.errno, 0
@@ -209,13 +209,13 @@ def _waitpid(pid, options):
def _fork_and_wait_for_startup():
try:
rfd, wfd = os.pipe()
-except OSError, e:
+except OSError as e:
sys.stderr.write("pipe failed: %s\n" % os.strerror(e.errno))
sys.exit(1)
try:
pid = os.fork()
-except OSError, e:
+except OSError as e:
sys.stderr.write("could not fork: %s\n" % os.strerror(e.errno))
sys.exit(1)
@@ -227,7 +227,7 @@ def _fork_and_wait_for_startup():
try:
s = os.read(rfd, 1)
error = 0
-except OSError, e:
+except OSError as e:
s = ""
error = e.errno
if error != errno.EINTR:
@@ -315,7 +315,8 @@ def _monitor_daemon(daemon_pid):
wakeup = last_restart + 1
if now > wakeup:
break
-print "sleep %f" % ((wakeup - now) / 1000.0)
+sys.stdout.write("sleep %f\n" % (
+(wakeup - now) / 1000.0))
time.sleep((wakeup - now) / 1000.0)
last_restart = ovs.timeval.msec()
@@ -406,7 +407,7 @@ def __read_pidfile(pidfile, delete_if_stale):
try:
file_handle = open(pidfile, "r+")
-except IOError, e:
+except IOError as e:
if e.errno == errno.ENOENT and delete_if_stale:
return 0
vlog.warn("%s: open: %s" % (pidfile, e.strerror))
@@ -439,7 +440,7 @@ def __read_pidfile(pidfile, delete_if_stale):
# We won the right to delete the stale pidfile.
try:
os.unlink(pidfile)
-except IOError, e:
+except IOError as e:
vlog.warn("%s: failed to delete stale pidfile (%s)"
% (pidfile, e.strerror))
return -e.errno
@@ -447,7 +448,7 @@ def __read_pidfile(pidfile, delete_if_stale):
vlog.dbg("%s: deleted stale pidfile" % pidfile)
file_handle.close()
return 0
-except IOError, e:
+except IOError as e:
if e.errno not in [errno.EACCES, errno.EAGAIN]:
vlog.warn("%s: fcntl: %s" % (pidfile, e.strerror))
return -e.errno
@@ -456,7 +457,7 @@ def __read_pidfile(pidfile, delete_if_stale):
try:
try:
error = int(file_handle.readline())
-except IOError, e:
+except IOError as e:
vlog.warn
Re: [ovs-dev] [PATCH] datapath-windows: Solved BSOD when adding OVS ports
hi Sorin, OvsInitExternalNBLContext() is called from OvsStartNBLIngress() which is the function that hooks into NDIS to get hold of packets in the ingress path. Typically these are packets that are generated by VMs, or it could be generated by an layered driver above OVS. Under what conditions would a NBL generated by OVS, get processed in OvsStartNBLIngress()? -- Nithin > On Jul 1, 2015, at 7:01 AM, Sorin Vinturis > wrote: > > This BSOD occurred in the context of a packet (NBL) with multiple > NET_BUFFER(s) (NBs). The reason for the BSOD is due to the marking > of NBLs created by OVS as being external and wrongly completing them. > > This patch should be applied both on master and branch 2.4. > > Signed-off-by: Sorin Vinturis > Reported-by: Sorin Vinturis > Reported-at: > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_82&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=fhvbX5XtKdTcw6NAB-9ZhpeR8OCoGr9hIPCT5N4ZDtA&s=wozkLP_z4adCLJ55PdqIHRBkdlEmOrQ8PPlDVcoOqII&e= > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath-windows: Failed initialization for datapath tunnel ports
I know this is already checked in, but just wanted to say “good catch”. > On Jul 1, 2015, at 12:52 AM, Sorin Vinturis > wrote: > > Tunnel ports are not initialized with the corresponding default port. > The newly allocated vport is not yet initialized and the ovsType > member does not reflect the correct tunnel port type, thus the > transport port destination won't be correctly initialized. > > Signed-off-by: Sorin Vinturis > Reported-by: Sorin Vinturis > Reported-at: > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_88&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=d5rMVml9ongf3R-nSM1_TtFBhnUtMvclFT--CUXLpsI&s=YJAyoVyLPkeWucAgwBR2X0bg-7O4L8tdqtTGfRc-xmE&e= > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 16/21] ovn: Rename Binding table to Port_Binding.
I ran this:
$ git grep "Binding table" | grep -v Port_
ovn/controller/pipeline.c: * Binding table within the logical datapath. */
ovn/controller/pipeline.c:/* Iterates through all of the records in the
Binding table, updating the
ovn/ovn-architecture.7.xml:Binding tables, whereas
ovn-northd(8) populates the LN
ovn/ovn-sb.xml:The Binding tables contain the current placement of
logical components
We may need to also clarify these?
Thanks,
Alex Wang,
On Tue, Jul 28, 2015 at 8:44 AM, Ben Pfaff wrote:
> An upcoming patch will add a Datapath_Binding table, so clarifying the
> name seems useful.
>
> Signed-off-by: Ben Pfaff
> ---
> ovn/controller/binding.c | 28 ++---
> ovn/controller/physical.c | 4 +-
> ovn/controller/pipeline.c | 4 +-
> ovn/northd/ovn-northd.c | 105
> --
> ovn/ovn-sb.ovsschema | 2 +-
> ovn/ovn-sb.xml| 8 ++--
> 6 files changed, 79 insertions(+), 72 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index c83b044..1b137bb 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -76,7 +76,7 @@ binding_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
> const char *chassis_id)
> {
> const struct sbrec_chassis *chassis_rec;
> -const struct sbrec_binding *binding_rec;
> +const struct sbrec_port_binding *binding_rec;
> struct sset lports, all_lports;
> const char *name;
>
> @@ -96,11 +96,11 @@ binding_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
> }
> sset_clone(&all_lports, &lports);
>
> -ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
> - "ovn-controller: updating bindings for
> '%s'",
> - chassis_id);
> +ovsdb_idl_txn_add_comment(
> +ctx->ovnsb_idl_txn,"ovn-controller: updating port bindings for
> '%s'",
> +chassis_id);
>
> -SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> +SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> if (sset_find_and_delete(&lports, binding_rec->logical_port) ||
> (binding_rec->parent_port && binding_rec->parent_port[0]
> &&
> sset_contains(&all_lports, binding_rec->parent_port))) {
> @@ -113,14 +113,14 @@ binding_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
>binding_rec->chassis->name,
>chassis_rec->name);
> }
> -sbrec_binding_set_chassis(binding_rec, chassis_rec);
> +sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
> } else if (binding_rec->chassis == chassis_rec) {
> -sbrec_binding_set_chassis(binding_rec, NULL);
> +sbrec_port_binding_set_chassis(binding_rec, NULL);
> }
> }
>
> SSET_FOR_EACH (name, &lports) {
> -VLOG_DBG("No binding record for lport %s", name);
> +VLOG_DBG("No port binding record for lport %s", name);
> }
> sset_destroy(&lports);
> sset_destroy(&all_lports);
> @@ -144,15 +144,15 @@ binding_cleanup(struct controller_ctx *ctx, const
> char *chassis_id)
> return true;
> }
>
> -ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
> - "ovn-controller: removing all bindings for
> '%s'",
> - chassis_id);
> +ovsdb_idl_txn_add_comment(
> +ctx->ovnsb_idl_txn,
> +"ovn-controller: removing all port bindings for '%s'",
> chassis_id);
>
> -const struct sbrec_binding *binding_rec;
> +const struct sbrec_port_binding *binding_rec;
> bool any_changes = false;
> -SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> +SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> if (binding_rec->chassis == chassis_rec) {
> -sbrec_binding_set_chassis(binding_rec, NULL);
> +sbrec_port_binding_set_chassis(binding_rec, NULL);
> any_changes = true;
> }
> }
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 11f88cb..55d6107 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -91,8 +91,8 @@ physical_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
>
> /* Set up flows in table 0 for physical-to-logical translation and in
> table
> * 64 for logical-to-physical translation. */
> -const struct sbrec_binding *binding;
> -SBREC_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> +const struct sbrec_port_binding *binding;
> +SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> /* Find the OpenFlow port for the logical port, as 'ofport'. If
> it's
> * on a remote chassis, this is the OpenFlow port for the tunnel
> to
> * that chassis (and set 'local' to false). Otherwise, if it's
> on the
>
Re: [ovs-dev] [PATCH 1/6] datapath: Whitespace fixes.
On Wed, Jul 29, 2015 at 12:53 PM, Joe Stringer wrote: > Signed-off-by: Joe Stringer > --- > datapath/actions.c | 6 +- > datapath/datapath.c | 2 ++ > datapath/flow.h | 1 - > datapath/flow_netlink.c | 7 --- > datapath/vport.c| 3 +-- > datapath/vport.h| 1 - > 6 files changed, 8 insertions(+), 12 deletions(-) Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 06/21] ovn-controller: Factor encapsulation code out of chassis code.
> On Jul 29, 2015, at 8:46 AM, Ben Pfaff wrote: > > On Tue, Jul 28, 2015 at 04:53:24PM -0700, Justin Pettit wrote: > >> If you agree, there's no reason to repost these patches. > > I think you should take another look, just to be sure. I agree that you're correct and that the patch is fine. Acked-by: Justin Pettit Thanks, --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 11/21] ovn-controller: Honor external-ids:ovn-bridge changing.
> On Jul 28, 2015, at 8:44 AM, Ben Pfaff wrote: > > @@ -309,7 +295,7 @@ main(int argc, char *argv[]) > ctx.ovnsb_idl_txn = idl_loop_run(&ovnsb_idl_loop); > ctx.ovs_idl_txn = idl_loop_run(&ovs_idl_loop); > > -const struct ovsrec_bridge *br_int = get_bridge(&ctx, br_int_name); > +const struct ovsrec_bridge *br_int = get_br_int(ctx.ovs_idl); This behavior already existed, but "br_int" may be null, and that could cause a problem when encaps_cleanup() is called a bit further down. Otherwise: Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] Start fixing some Python 3 compatibility issues
On 07/29/2015 10:12 PM, Terry Wilson wrote: > This patch fixes just the Python 3 problems found by running > > python3 setup.py install > > There are still many other issues to be fixed, but this is a start. > > Signed-off-by: Terry Wilson This is just a rebase, right? All of the changes still look good to me. This should be fine now that the min Python version was increased to 2.7. Acked-by: Russell Bryant -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/6] datapath: Constify netlink structs.
On Wed, Jul 29, 2015 at 12:53 PM, Joe Stringer wrote: > Signed-off-by: Joe Stringer > --- > datapath/datapath.c | 25 + > datapath/datapath.h | 2 +- > 2 files changed, 14 insertions(+), 13 deletions(-) If I remember correctly the reason why these weren't marked const is because older kernels had functions that took these parameters as non-const and it provoked warnings. However, it seems to me that we have mostly redefined the places where these are passed, so perhaps it's not an issue anymore. Can you double check to see if that's the case? If so, this looks good to me. Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/6] datapath: Use skb_postpull_rcsum().
On Wed, Jul 29, 2015 at 12:53 PM, Joe Stringer wrote: > Signed-off-by: Joe Stringer > --- > datapath/actions.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 11/21] ovn-controller: Honor external-ids:ovn-bridge changing.
On Thu, Jul 30, 2015 at 10:03 AM, Justin Pettit wrote: > >> On Jul 28, 2015, at 8:44 AM, Ben Pfaff wrote: >> >> @@ -309,7 +295,7 @@ main(int argc, char *argv[]) >> ctx.ovnsb_idl_txn = idl_loop_run(&ovnsb_idl_loop); >> ctx.ovs_idl_txn = idl_loop_run(&ovs_idl_loop); >> >> -const struct ovsrec_bridge *br_int = get_bridge(&ctx, br_int_name); >> +const struct ovsrec_bridge *br_int = get_br_int(ctx.ovs_idl); > > This behavior already existed, but "br_int" may be null, and that could cause > a problem when encaps_cleanup() is called a bit further down. Thanks. I fixed this in the earlier patch that introduced it (Alex pointed it out, I think). > Acked-by: Justin Pettit Thanks! -- "I don't normally do acked-by's. I think it's my way of avoiding getting blamed when it all blows up." Andrew Morton ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 4/6] datapath: Backport eth_proto_is_802_3().
On Wed, Jul 29, 2015 at 12:53 PM, Joe Stringer wrote: > Backport of upstream commit 2c7a88c252bf3381958cf716f31b6b2e0f2f3fa7: > "etherdev: Fix sparse error, make test usable by other functions" > > Signed-off-by: Joe Stringer Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] Start fixing some Python 3 compatibility issues
On Thu, Jul 30, 2015 at 12:06 PM, Russell Bryant wrote: > On 07/29/2015 10:12 PM, Terry Wilson wrote: > > This patch fixes just the Python 3 problems found by running > > > > python3 setup.py install > > > > There are still many other issues to be fixed, but this is a start. > > > > Signed-off-by: Terry Wilson > > This is just a rebase, right? All of the changes still look good to me. > This should be fine now that the min Python version was increased to 2.7. > > Acked-by: Russell Bryant > > Looks good to me too. Thanks for picking this up again Terry! Acked-by: Kyle Mestery > -- > Russell Bryant > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 5/6] datapath: Use eth_proto_is_802_3.
On Wed, Jul 29, 2015 at 12:53 PM, Joe Stringer wrote: > From: Alexander Duyck > > Replace "ntohs(proto) >= ETH_P_802_3_MIN" w/ eth_proto_is_802_3(proto). > > Backport of upstream commit 6713fc9b8fa33444aa000f0f31076f6a859ccb34: > "openvswitch: Use eth_proto_is_802_3" > > Signed-off-by: Alexander Duyck > Signed-off-by: David S. Miller > Signed-off-by: Joe Stringer Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 11/21] ovn-controller: Honor external-ids:ovn-bridge changing.
I've pushed this series up to this patch to master now. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 16/21] ovn: Rename Binding table to Port_Binding.
Thanks, I fixed those up too.
On Thu, Jul 30, 2015 at 09:32:05AM -0700, Alex Wang wrote:
> I ran this:
>
> $ git grep "Binding table" | grep -v Port_
> ovn/controller/pipeline.c: * Binding table within the logical datapath. */
> ovn/controller/pipeline.c:/* Iterates through all of the records in the
> Binding table, updating the
> ovn/ovn-architecture.7.xml:Binding tables, whereas
> ovn-northd(8) populates the LN
> ovn/ovn-sb.xml:The Binding tables contain the current placement of
> logical components
>
> We may need to also clarify these?
>
> Thanks,
> Alex Wang,
>
> On Tue, Jul 28, 2015 at 8:44 AM, Ben Pfaff wrote:
>
> > An upcoming patch will add a Datapath_Binding table, so clarifying the
> > name seems useful.
> >
> > Signed-off-by: Ben Pfaff
> > ---
> > ovn/controller/binding.c | 28 ++---
> > ovn/controller/physical.c | 4 +-
> > ovn/controller/pipeline.c | 4 +-
> > ovn/northd/ovn-northd.c | 105
> > --
> > ovn/ovn-sb.ovsschema | 2 +-
> > ovn/ovn-sb.xml| 8 ++--
> > 6 files changed, 79 insertions(+), 72 deletions(-)
> >
> > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> > index c83b044..1b137bb 100644
> > --- a/ovn/controller/binding.c
> > +++ b/ovn/controller/binding.c
> > @@ -76,7 +76,7 @@ binding_run(struct controller_ctx *ctx, const struct
> > ovsrec_bridge *br_int,
> > const char *chassis_id)
> > {
> > const struct sbrec_chassis *chassis_rec;
> > -const struct sbrec_binding *binding_rec;
> > +const struct sbrec_port_binding *binding_rec;
> > struct sset lports, all_lports;
> > const char *name;
> >
> > @@ -96,11 +96,11 @@ binding_run(struct controller_ctx *ctx, const struct
> > ovsrec_bridge *br_int,
> > }
> > sset_clone(&all_lports, &lports);
> >
> > -ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
> > - "ovn-controller: updating bindings for
> > '%s'",
> > - chassis_id);
> > +ovsdb_idl_txn_add_comment(
> > +ctx->ovnsb_idl_txn,"ovn-controller: updating port bindings for
> > '%s'",
> > +chassis_id);
> >
> > -SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> > +SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> > if (sset_find_and_delete(&lports, binding_rec->logical_port) ||
> > (binding_rec->parent_port && binding_rec->parent_port[0]
> > &&
> > sset_contains(&all_lports, binding_rec->parent_port))) {
> > @@ -113,14 +113,14 @@ binding_run(struct controller_ctx *ctx, const struct
> > ovsrec_bridge *br_int,
> >binding_rec->chassis->name,
> >chassis_rec->name);
> > }
> > -sbrec_binding_set_chassis(binding_rec, chassis_rec);
> > +sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
> > } else if (binding_rec->chassis == chassis_rec) {
> > -sbrec_binding_set_chassis(binding_rec, NULL);
> > +sbrec_port_binding_set_chassis(binding_rec, NULL);
> > }
> > }
> >
> > SSET_FOR_EACH (name, &lports) {
> > -VLOG_DBG("No binding record for lport %s", name);
> > +VLOG_DBG("No port binding record for lport %s", name);
> > }
> > sset_destroy(&lports);
> > sset_destroy(&all_lports);
> > @@ -144,15 +144,15 @@ binding_cleanup(struct controller_ctx *ctx, const
> > char *chassis_id)
> > return true;
> > }
> >
> > -ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
> > - "ovn-controller: removing all bindings for
> > '%s'",
> > - chassis_id);
> > +ovsdb_idl_txn_add_comment(
> > +ctx->ovnsb_idl_txn,
> > +"ovn-controller: removing all port bindings for '%s'",
> > chassis_id);
> >
> > -const struct sbrec_binding *binding_rec;
> > +const struct sbrec_port_binding *binding_rec;
> > bool any_changes = false;
> > -SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> > +SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> > if (binding_rec->chassis == chassis_rec) {
> > -sbrec_binding_set_chassis(binding_rec, NULL);
> > +sbrec_port_binding_set_chassis(binding_rec, NULL);
> > any_changes = true;
> > }
> > }
> > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> > index 11f88cb..55d6107 100644
> > --- a/ovn/controller/physical.c
> > +++ b/ovn/controller/physical.c
> > @@ -91,8 +91,8 @@ physical_run(struct controller_ctx *ctx, const struct
> > ovsrec_bridge *br_int,
> >
> > /* Set up flows in table 0 for physical-to-logical translation and in
> > table
> > * 64 for logical-to-physical translation. */
> > -const struct sbrec_binding *binding;
> > -SBREC_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> > +const struct sbrec_
Re: [ovs-dev] [PATCH] datapath-windows: Solved BSOD when adding OVS ports
Nithin, When a packet (NBL) with multiple NB arrives, it is splitted into a list of multiple packets (NBLs) with single NB by the OvsCreateNewNBLsFromMultipleNBs() function. The new NBL list is inserted into the head of the existing NBL list and the execution continues with the processing of the first packet from the new NBL list. The OvsInitExternalNBLContext() is called for the second packet from the new NBL list and hence the need to change it. Thanks, Sorin -Original Message- From: Nithin Raju [mailto:nit...@vmware.com] Sent: Thursday, 30 July, 2015 19:19 To: Sorin Vinturis Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH] datapath-windows: Solved BSOD when adding OVS ports hi Sorin, OvsInitExternalNBLContext() is called from OvsStartNBLIngress() which is the function that hooks into NDIS to get hold of packets in the ingress path. Typically these are packets that are generated by VMs, or it could be generated by an layered driver above OVS. Under what conditions would a NBL generated by OVS, get processed in OvsStartNBLIngress()? -- Nithin > On Jul 1, 2015, at 7:01 AM, Sorin Vinturis > wrote: > > This BSOD occurred in the context of a packet (NBL) with multiple > NET_BUFFER(s) (NBs). The reason for the BSOD is due to the marking of > NBLs created by OVS as being external and wrongly completing them. > > This patch should be applied both on master and branch 2.4. > > Signed-off-by: Sorin Vinturis > Reported-by: Sorin Vinturis > Reported-at: > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvs > witch_ovs-2Dissues_issues_82&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw- > YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=fhvbX5Xt > KdTcw6NAB-9ZhpeR8OCoGr9hIPCT5N4ZDtA&s=wozkLP_z4adCLJ55PdqIHRBkdlEmOrQ8 > PPlDVcoOqII&e= ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN - L3 Gap between NB schema and Neutron
[adding Aaron Rosen] On Wed, Jul 29, 2015 at 12:20:30PM +0300, Gal Sagie wrote: > Currently Neutron support defining few subnets (IP cidrs) on a network > (logical switch) > and connecting them to the same router (or different routers). > Currently in the NB schema, the logical switch can be connected only to one > logical > router port. > > This needs to be extended so a logical switch can have more then one > logical router > port reference to support the above use case. Limiting a logical switch to a single router port is an intentional design decision. It means that a packet traverses at most two logical switches (one at ingress, one at egress), which simplifies some of the logical switch design, and it prevents loops. VMware's NVP controller uses the same design, for those reasons and others. The NVP paper from NSDI 2014 (see http://benpfaff.org/papers/net-virt.pdf) puts it this way: As an optimization, we constrain the logical topology such that logical L2 destinations can only be present at its edge[6]. This restriction means that the OVS flow table of a sending hypervisor needs only to have flows for logical datapaths to which its local VMs are attached as well as those of the L3 routers of the logical topology; the receiving hypervisor is determined by the logical IP destination address, leaving the last logical L2 hop to be executed at the receiving hypervisor. [6] We have found little value in supporting logical routers interconnected through logical switches without tenant VMs. Are you sure that Neutron supports multiple router ports per switch? Russell Bryant (in IRC) and Aaron Rosen (in a quick in-person chat) seemed doubtful. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN - L3 Gap between NB schema and Neutron
Yes, i checked this on my setup. For example, you can have both IPv6 and IPv4 subnets per the same network (which maps to a logical switch) and connect both as two different router ports (to the same router) You can also connect the same network to two different routers, i am not sure if you need the extra route extension for that or not, i think you could configure it as default gateway with out this extension, but with the extension you can define routing between the two routers. On Thu, Jul 30, 2015 at 9:03 PM, Ben Pfaff wrote: > [adding Aaron Rosen] > > On Wed, Jul 29, 2015 at 12:20:30PM +0300, Gal Sagie wrote: > > Currently Neutron support defining few subnets (IP cidrs) on a network > > (logical switch) > > and connecting them to the same router (or different routers). > > Currently in the NB schema, the logical switch can be connected only to > one > > logical > > router port. > > > > This needs to be extended so a logical switch can have more then one > > logical router > > port reference to support the above use case. > > Limiting a logical switch to a single router port is an intentional > design decision. It means that a packet traverses at most two logical > switches (one at ingress, one at egress), which simplifies some of the > logical switch design, and it prevents loops. > > VMware's NVP controller uses the same design, for those reasons and > others. The NVP paper from NSDI 2014 (see > http://benpfaff.org/papers/net-virt.pdf) puts it this way: > > As an optimization, we constrain the logical topology such that > logical L2 destinations can only be present at its edge[6]. This > restriction means that the OVS flow table of a sending hypervisor > needs only to have flows for logical datapaths to which its local > VMs are attached as well as those of the L3 routers of the logical > topology; the receiving hypervisor is determined by the logical IP > destination address, leaving the last logical L2 hop to be executed > at the receiving hypervisor. > > [6] We have found little value in supporting logical routers > interconnected through logical switches without tenant VMs. > > Are you sure that Neutron supports multiple router ports per switch? > Russell Bryant (in IRC) and Aaron Rosen (in a quick in-person chat) > seemed doubtful. > -- Best Regards , The G. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN - L3 Gap between NB schema and Neutron
Also adding Salvatore. On Thu, Jul 30, 2015 at 9:12 PM, Gal Sagie wrote: > Yes, i checked this on my setup. > For example, you can have both IPv6 and IPv4 subnets per the same network > (which maps to a logical switch) > and connect both as two different router ports (to the same router) > > You can also connect the same network to two different routers, i am not > sure if you need the extra route extension for that or not, i think you > could > configure it as default gateway with out this extension, but with the > extension you > can define routing between the two routers. > > > > > > On Thu, Jul 30, 2015 at 9:03 PM, Ben Pfaff wrote: > >> [adding Aaron Rosen] >> >> On Wed, Jul 29, 2015 at 12:20:30PM +0300, Gal Sagie wrote: >> > Currently Neutron support defining few subnets (IP cidrs) on a network >> > (logical switch) >> > and connecting them to the same router (or different routers). >> > Currently in the NB schema, the logical switch can be connected only to >> one >> > logical >> > router port. >> > >> > This needs to be extended so a logical switch can have more then one >> > logical router >> > port reference to support the above use case. >> >> Limiting a logical switch to a single router port is an intentional >> design decision. It means that a packet traverses at most two logical >> switches (one at ingress, one at egress), which simplifies some of the >> logical switch design, and it prevents loops. >> >> VMware's NVP controller uses the same design, for those reasons and >> others. The NVP paper from NSDI 2014 (see >> http://benpfaff.org/papers/net-virt.pdf) puts it this way: >> >> As an optimization, we constrain the logical topology such that >> logical L2 destinations can only be present at its edge[6]. This >> restriction means that the OVS flow table of a sending hypervisor >> needs only to have flows for logical datapaths to which its local >> VMs are attached as well as those of the L3 routers of the logical >> topology; the receiving hypervisor is determined by the logical IP >> destination address, leaving the last logical L2 hop to be executed >> at the receiving hypervisor. >> >> [6] We have found little value in supporting logical routers >> interconnected through logical switches without tenant VMs. >> >> Are you sure that Neutron supports multiple router ports per switch? >> Russell Bryant (in IRC) and Aaron Rosen (in a quick in-person chat) >> seemed doubtful. >> > > > > -- > Best Regards , > > The G. > -- Best Regards , The G. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] rhel: add builrequires for libtool and automake
Those two packages are needed to build but they might not
be present in the system.
Signed-off-by: Flavio Leitner
---
rhel/openvswitch-fedora.spec.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index f44da3c..48c34de 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -29,7 +29,7 @@ License: ASL 2.0 and LGPLv2+ and SISSL
Release: 1%{?dist}
Source: http://openvswitch.org/releases/%{name}-%{version}.tar.gz
-BuildRequires: autoconf
+BuildRequires: autoconf automake libtool
BuildRequires: systemd-units openssl openssl-devel
BuildRequires: python python-twisted-core python-zope-interface PyQt4
BuildRequires: desktop-file-utils
--
2.1.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/3] tests: Expand kernel sanity tests.
Acked-by: Daniele Di Proietto On 30/07/2015 00:52, "Joe Stringer" wrote: >The initial sanity test only checked IPv4 without IP fragments. This patch >adds additional tests using IPv6 and VLANs with IP fragments and expands >the existing test to be more strict. > >Signed-off-by: Joe Stringer >CC: Daniele Di Proietto >--- > >Daniele, these tests are basic sanity checks to ensure that the tree >works correctly today in particular when we start getting things like IP >fragments and VLANs involved. For each of these tests, I also have some >forthcoming tests that test the same environments but with >conntrack/firewall flows. Would you mind looking at how we can make >these tests generic across both datapaths, just like the other tests you >have in your conntrack branch? It might be worthwhile to start >upstreaming those changes, too. It doesn't appear too complicated to port these tests to the userspace datapath (they actually do not require any change at all). I have some patches almost ready on top of this series to do that. I'll post them tomorrow, hoping that this gets merged Thanks! >--- > tests/kmod-macros.at | 11 ++ > tests/kmod-traffic.at | 93 >++- > 2 files changed, 103 insertions(+), 1 deletion(-) > >diff --git a/tests/kmod-macros.at b/tests/kmod-macros.at >index 5824940..eef1263 100644 >--- a/tests/kmod-macros.at >+++ b/tests/kmod-macros.at >@@ -77,3 +77,14 @@ m4_define([ADD_VETH], > AT_CHECK([ip netns exec $2 ip link set dev $1 up]) > ] > ) >+ >+# ADD_VLAN([port], [namespace], [vlan-id], [ip-addr]) >+# >+# Add a VLAN device named 'port' within 'namespace'. It will be >configured >+# with the ID 'vlan-id' and the address 'ip-addr'. >+m4_define([ADD_VLAN], >+[ AT_CHECK([ip netns exec $2 ip link add link $1 name $1.$3 type >vlan id $3]) >+ AT_CHECK([ip netns exec $2 ip link set dev $1.$3 up]) >+ AT_CHECK([ip netns exec $2 ip addr add dev $1.$3 $4]) >+] >+) >diff --git a/tests/kmod-traffic.at b/tests/kmod-traffic.at >index f7783be..9df8b62 100644 >--- a/tests/kmod-traffic.at >+++ b/tests/kmod-traffic.at >@@ -11,6 +11,97 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") > > AT_CAPTURE_FILE([ping.output]) > AT_CHECK([ip netns exec at_ns0 bash -c "ping -q -c 3 -i 0.3 -w 2 >10.1.1.2 > ping.output"]) >+AT_CHECK([ip netns exec at_ns0 bash -c "ping -s 1600 -q -c 3 -i 0.3 -w 2 >10.1.1.2 >> ping.output"]) >+AT_CHECK([ip netns exec at_ns0 bash -c "ping -s 3200 -q -c 3 -i 0.3 -w 2 >10.1.1.2 >> ping.output"]) > >-OVS_KMOD_VSWITCHD_STOP([], DEL_NAMESPACES(at_ns0, at_ns1)) >+AT_CHECK([cat ping.output | grep "transmitted" | sed 's/time.*ms$/time >0ms/'], [0], [dnl >+3 packets transmitted, 3 received, 0% packet loss, time 0ms >+3 packets transmitted, 3 received, 0% packet loss, time 0ms >+3 packets transmitted, 3 received, 0% packet loss, time 0ms >+]) >+ >+OVS_KMOD_VSWITCHD_STOP >+AT_CLEANUP >+ >+AT_SETUP([kmod - ping between two ports on vlan]) >+OVS_KMOD_VSWITCHD_START( >+ [set-fail-mode br0 standalone -- ]) >+ >+ADD_NAMESPACES(at_ns0, at_ns1) >+ >+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") >+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") >+ >+ADD_VLAN(p0, at_ns0, 100, "10.2.2.1/24") >+ADD_VLAN(p1, at_ns1, 100, "10.2.2.2/24") >+ >+AT_CAPTURE_FILE([ping.output]) >+AT_CHECK([ip netns exec at_ns0 bash -c "ping -q -c 3 -i 0.3 -w 2 >10.2.2.2 > ping.output"]) >+AT_CHECK([ip netns exec at_ns0 bash -c "ping -s 1600 -q -c 3 -i 0.3 -w 2 >10.2.2.2 >> ping.output"]) >+AT_CHECK([ip netns exec at_ns0 bash -c "ping -s 3200 -q -c 3 -i 0.3 -w 2 >10.2.2.2 >> ping.output"]) >+ >+AT_CHECK([cat ping.output | grep "transmitted" | sed 's/time.*ms$/time >0ms/'], [0], [dnl >+3 packets transmitted, 3 received, 0% packet loss, time 0ms >+3 packets transmitted, 3 received, 0% packet loss, time 0ms >+3 packets transmitted, 3 received, 0% packet loss, time 0ms >+]) >+ >+OVS_KMOD_VSWITCHD_STOP >+AT_CLEANUP >+ >+AT_SETUP([kmod - ping6 between two ports]) >+OVS_KMOD_VSWITCHD_START( >+ [set-fail-mode br0 standalone -- ]) >+ >+ADD_NAMESPACES(at_ns0, at_ns1) >+ >+ADD_VETH(p0, at_ns0, br0, "fc00::1/96") >+ADD_VETH(p1, at_ns1, br0, "fc00::2/96") >+ >+dnl Without this sleep, we get occasional failures due to the following >error: >+dnl "connect: Cannot assign requested address" >+sleep 2; >+ >+AT_CAPTURE_FILE([ping.output]) >+AT_CHECK([ip netns exec at_ns0 bash -c "ping6 -q -c 3 -i 0.3 -w 2 >fc00::2 > ping.output"]) >+AT_CHECK([ip netns exec at_ns0 bash -c "ping6 -s 1600 -q -c 3 -i 0.3 -w >2 fc00::2 >> ping.output"]) >+AT_CHECK([ip netns exec at_ns0 bash -c "ping6 -s 3200 -q -c 3 -i 0.3 -w >2 fc00::2 >> ping.output"]) >+ >+AT_CHECK([cat ping.output | grep "transmitted" | sed 's/time.*ms$/time >0ms/'], [0], [dnl >+3 packets transmitted, 3 received, 0% packet loss, time 0ms >+3 packets transmitted, 3 received, 0% packet loss, time 0ms >+3 packets transmitted, 3 received, 0% packet loss, time 0ms >+]) >+ >+OVS_KMOD_VSWITCHD_STOP >+AT_CLEANUP >+ >+AT_SETUP([kmod - ping6 between
Re: [ovs-dev] [PATCH 2/3] kmod-macros: Don't unload kmod in VSWITCHD_STOP.
Acked-by: Daniele Di Proietto On 30/07/2015 00:52, "Joe Stringer" wrote: >We already queue the removal of the kernel module in OVS_VSWITCHD_START, >via an ON_EXIT() call. This redundant removal also doesn't interact very >well with usage of vports: If the datapath still exists in the kernel, >we cannot remove a vport module; if we cannot remove a vport module, we >cannot remove the openvswitch module. Having the call here prevents >tunnel tests from manually cleaning up their datapath on exit, so this >patch removes it. > >Signed-off-by: Joe Stringer >--- > tests/kmod-macros.at | 1 - > 1 file changed, 1 deletion(-) > >diff --git a/tests/kmod-macros.at b/tests/kmod-macros.at >index eef1263..3487c67 100644 >--- a/tests/kmod-macros.at >+++ b/tests/kmod-macros.at >@@ -32,7 +32,6 @@ m4_define([OVS_KMOD_VSWITCHD_STOP], > [AT_CHECK([ovs-vsctl del-br br0]) >OVS_VSWITCHD_STOP([$1]) >AT_CHECK([:; $2]) >- AT_CHECK([modprobe -r openvswitch]) > ]) > > # DEL_NAMESPACES(ns [, ns ... ]) >-- >2.1.4 > >___ >dev mailing list >dev@openvswitch.org >https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma >n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Sm >B5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=_qv5YP4RXejZW_70J74jm47DAhQepw >jSINEvbx7wGDA&s=0lJUFLfpRnvfKS5BkXUSgZKOpw3oCx3U1ISPaaCh1zA&e= ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] rhel: add builrequires for libtool and automake
On 07/30/2015 02:16 PM, Flavio Leitner wrote: > Those two packages are needed to build but they might not > be present in the system. > > Signed-off-by: Flavio Leitner Acked-by: Russell Bryant -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN - L3 Gap between NB schema and Neutron
If both the router ports point to the same router, then I am not sure why this would need to be two ports. Maybe the schema is not sufficient to report both IPv4 and IPv6 addresses on a single router port; if so, then I would support enhancing the schema to fix that. I suspect that for connecting to two different routers, it is possible to instead connect one router and then connect that router to others in a way that accomplishes an equivalent goal. I haven't thought it through though. On Thu, Jul 30, 2015 at 09:12:14PM +0300, Gal Sagie wrote: > Yes, i checked this on my setup. > For example, you can have both IPv6 and IPv4 subnets per the same network > (which maps to a logical switch) > and connect both as two different router ports (to the same router) > > You can also connect the same network to two different routers, i am not > sure if you need the extra route extension for that or not, i think you > could > configure it as default gateway with out this extension, but with the > extension you > can define routing between the two routers. > > > > > > On Thu, Jul 30, 2015 at 9:03 PM, Ben Pfaff wrote: > > > [adding Aaron Rosen] > > > > On Wed, Jul 29, 2015 at 12:20:30PM +0300, Gal Sagie wrote: > > > Currently Neutron support defining few subnets (IP cidrs) on a network > > > (logical switch) > > > and connecting them to the same router (or different routers). > > > Currently in the NB schema, the logical switch can be connected only to > > one > > > logical > > > router port. > > > > > > This needs to be extended so a logical switch can have more then one > > > logical router > > > port reference to support the above use case. > > > > Limiting a logical switch to a single router port is an intentional > > design decision. It means that a packet traverses at most two logical > > switches (one at ingress, one at egress), which simplifies some of the > > logical switch design, and it prevents loops. > > > > VMware's NVP controller uses the same design, for those reasons and > > others. The NVP paper from NSDI 2014 (see > > http://benpfaff.org/papers/net-virt.pdf) puts it this way: > > > > As an optimization, we constrain the logical topology such that > > logical L2 destinations can only be present at its edge[6]. This > > restriction means that the OVS flow table of a sending hypervisor > > needs only to have flows for logical datapaths to which its local > > VMs are attached as well as those of the L3 routers of the logical > > topology; the receiving hypervisor is determined by the logical IP > > destination address, leaving the last logical L2 hop to be executed > > at the receiving hypervisor. > > > > [6] We have found little value in supporting logical routers > > interconnected through logical switches without tenant VMs. > > > > Are you sure that Neutron supports multiple router ports per switch? > > Russell Bryant (in IRC) and Aaron Rosen (in a quick in-person chat) > > seemed doubtful. > > > > > > -- > Best Regards , > > The G. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN - L3 Gap between NB schema and Neutron
[also adding Salvatore] On Thu, Jul 30, 2015 at 11:27:57AM -0700, Ben Pfaff wrote: > If both the router ports point to the same router, then I am not sure > why this would need to be two ports. Maybe the schema is not sufficient > to report both IPv4 and IPv6 addresses on a single router port; if so, > then I would support enhancing the schema to fix that. > > I suspect that for connecting to two different routers, it is possible > to instead connect one router and then connect that router to others in > a way that accomplishes an equivalent goal. I haven't thought it > through though. > > On Thu, Jul 30, 2015 at 09:12:14PM +0300, Gal Sagie wrote: > > Yes, i checked this on my setup. > > For example, you can have both IPv6 and IPv4 subnets per the same network > > (which maps to a logical switch) > > and connect both as two different router ports (to the same router) > > > > You can also connect the same network to two different routers, i am not > > sure if you need the extra route extension for that or not, i think you > > could > > configure it as default gateway with out this extension, but with the > > extension you > > can define routing between the two routers. > > > > > > > > > > > > On Thu, Jul 30, 2015 at 9:03 PM, Ben Pfaff wrote: > > > > > [adding Aaron Rosen] > > > > > > On Wed, Jul 29, 2015 at 12:20:30PM +0300, Gal Sagie wrote: > > > > Currently Neutron support defining few subnets (IP cidrs) on a network > > > > (logical switch) > > > > and connecting them to the same router (or different routers). > > > > Currently in the NB schema, the logical switch can be connected only to > > > one > > > > logical > > > > router port. > > > > > > > > This needs to be extended so a logical switch can have more then one > > > > logical router > > > > port reference to support the above use case. > > > > > > Limiting a logical switch to a single router port is an intentional > > > design decision. It means that a packet traverses at most two logical > > > switches (one at ingress, one at egress), which simplifies some of the > > > logical switch design, and it prevents loops. > > > > > > VMware's NVP controller uses the same design, for those reasons and > > > others. The NVP paper from NSDI 2014 (see > > > http://benpfaff.org/papers/net-virt.pdf) puts it this way: > > > > > > As an optimization, we constrain the logical topology such that > > > logical L2 destinations can only be present at its edge[6]. This > > > restriction means that the OVS flow table of a sending hypervisor > > > needs only to have flows for logical datapaths to which its local > > > VMs are attached as well as those of the L3 routers of the logical > > > topology; the receiving hypervisor is determined by the logical IP > > > destination address, leaving the last logical L2 hop to be executed > > > at the receiving hypervisor. > > > > > > [6] We have found little value in supporting logical routers > > > interconnected through logical switches without tenant VMs. > > > > > > Are you sure that Neutron supports multiple router ports per switch? > > > Russell Bryant (in IRC) and Aaron Rosen (in a quick in-person chat) > > > seemed doubtful. > > > > > > > > > > > -- > > Best Regards , > > > > The G. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] rhel: add builrequires for libtool and automake
On Thu, Jul 30, 2015 at 03:16:30PM -0300, Flavio Leitner wrote: > Those two packages are needed to build but they might not > be present in the system. > > Signed-off-by: Flavio Leitner Thanks, applied to master, branch-2.4, and branch-2.3. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/3] tests: Add basic vxlan tunnel sanity test.
I get a warning in the OVS log that causes this test to fail. It appears that when br0 is removed (in OVS_KMOD_VSWITCHD_STOP) OVS gets a rtnetlink message (because br0 had an address in the routing table), but route_table_parse() fails, because if_indextoname() returns an error (the device is not there anymore). Adding AT_CHECK([ip addr del dev br0 "10.1.1.100/24"]) before OVS_KMOD_VSWITCHD_STOP solves the problem for me. Could you add that? What do you think? Also, I think that instead of creating the veth pair for the tunnel underlay manually, we could use another OVS bridge. This has two advantages: * The code is shorter * Tunnelling in userspace requires an OVS bridge as underlay. This is a summary of the changes I'm proposing: diff --git a/tests/kmod-traffic.at b/tests/kmod-traffic.at index 169078d..346d464 100644 --- a/tests/kmod-traffic.at +++ b/tests/kmod-traffic.at @@ -110,20 +110,18 @@ AT_SETUP([kmod - ping over vxlan tunnel]) AT_SKIP_IF([! ip link help 2>&1 | grep vxlan >/dev/null]) OVS_KMOD_VSWITCHD_START( - [set-fail-mode br0 standalone --]) + [set-fail-mode br0 standalone --dnl +add-br br-ovs-p0]) dnl Ensure that vport_* can be removed on exit. ON_EXIT([modprobe -r vport_vxlan]) ON_EXIT([ovs-dpctl del-dp ovs-system]) ADD_NAMESPACES(at_ns0) +ADD_VETH(p0, at_ns0, br-ovs-p0, "172.31.1.1/24") dnl Set up underlay link from host into the namespace using veth pair. -AT_CHECK([ip link add p0 type veth peer name host-p0]) -AT_CHECK([ip addr add dev host-p0 "172.31.1.100/24"]) -AT_CHECK([ip link set dev host-p0 up]) -AT_CHECK([ip link set p0 netns at_ns0]) -AT_CHECK([ip netns exec at_ns0 ip addr add dev p0 "172.31.1.1/24"]) -AT_CHECK([ip netns exec at_ns0 ip link set dev p0 up]) +AT_CHECK([ip addr add dev br-ovs-p0 "172.31.1.100/24"]) +AT_CHECK([ip link set dev br-ovs-p0 up]) dnl Set up remote end of tunnel inside the namespace. ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.1], [172.31.1.100], @@ -152,5 +150,8 @@ AT_CHECK([cat ping.output | grep "transmitted" | sed 's/time.*ms$/time 0ms/'], [ 3 packets transmitted, 3 received, 0% packet loss, time 0ms ]) +AT_CHECK([ip addr del dev br0 "10.1.1.100/24"]) +AT_CHECK([ip addr del dev br-ovs-p0 "172.31.1.100/24"]) + OVS_KMOD_VSWITCHD_STOP AT_CLEANUP I'll leave it up to you, I can also make this change in my next series. Acked-by: Daniele Di Proietto On 30/07/2015 00:52, "Joe Stringer" wrote: >Signed-off-by: Joe Stringer >--- > tests/kmod-macros.at | 20 > tests/kmod-traffic.at | 49 >+ > 2 files changed, 69 insertions(+) > >diff --git a/tests/kmod-macros.at b/tests/kmod-macros.at >index 3487c67..be3c123 100644 >--- a/tests/kmod-macros.at >+++ b/tests/kmod-macros.at >@@ -87,3 +87,23 @@ m4_define([ADD_VLAN], > AT_CHECK([ip netns exec $2 ip addr add dev $1.$3 $4]) > ] > ) >+ >+# ADD_NATIVE_TUNNEL([type], [port], [namespace], [local-addr], >[remote-addr], >+# [overlay-addr], [link-args]) >+# >+# Add a native tunnel device within 'namespace', with name 'port' and >type >+# 'type'. The tunnel device will be configured as point-to-point with the >+# 'local-addr' address configured as the underlay address within >'namespace', >+# and 'remote-addr' as the underlay address of the remote destination (as >+# viewed from the perspective of that namespace). >+# >+# 'port' will be configured with the address 'overlay-addr'. 'link-args' >is >+# made available so that additional arguments can be passed to "ip link", >+# for instance to configure the vxlan destination port. >+m4_define([ADD_NATIVE_TUNNEL], >+[ AT_CHECK([ip netns exec $3 ip link add dev $2 type $1 local $4 >remote $5 $7]) >+ AT_CHECK([ip netns exec $3 ip addr add dev $2 $6]) >+ AT_CHECK([ip netns exec $3 ip link set dev $2 up]) >+ AT_CHECK([ip netns exec $3 ip link set dev $2 mtu 1450]) >+] >+) >diff --git a/tests/kmod-traffic.at b/tests/kmod-traffic.at >index 9df8b62..169078d 100644 >--- a/tests/kmod-traffic.at >+++ b/tests/kmod-traffic.at >@@ -105,3 +105,52 @@ AT_CHECK([cat ping.output | grep "transmitted" | sed >'s/time.*ms$/time 0ms/'], [ > > OVS_KMOD_VSWITCHD_STOP > AT_CLEANUP >+ >+AT_SETUP([kmod - ping over vxlan tunnel]) >+AT_SKIP_IF([! ip link help 2>&1 | grep vxlan >/dev/null]) >+ >+OVS_KMOD_VSWITCHD_START( >+ [set-fail-mode br0 standalone --]) >+dnl Ensure that vport_* can be removed on exit. >+ON_EXIT([modprobe -r vport_vxlan]) >+ON_EXIT([ovs-dpctl del-dp ovs-system]) >+ >+ADD_NAMESPACES(at_ns0) >+ >+dnl Set up underlay link from host into the namespace using veth pair. >+AT_CHECK([ip link add p0 type veth peer name host-p0]) >+AT_CHECK([ip addr add dev host-p0 "172.31.1.100/24"]) >+AT_CHECK([ip link set dev host-p0 up]) >+AT_CHECK([ip link set p0 netns at_ns0]) >+AT_CHECK([ip netns exec at_ns0 ip addr add dev p0 "172.31.1.1/24"]) >+AT_CHECK([ip netns exec at_ns0 ip link set dev p0 up]) >+ >+dnl Set up remote end of tunn
Re: [ovs-dev] [PATCH 2/3] kmod-macros: Don't unload kmod in VSWITCHD_STOP.
On Wed, Jul 29, 2015 at 4:52 PM, Joe Stringer wrote: > We already queue the removal of the kernel module in OVS_VSWITCHD_START, > via an ON_EXIT() call. This redundant removal also doesn't interact very > well with usage of vports: If the datapath still exists in the kernel, > we cannot remove a vport module; if we cannot remove a vport module, we > cannot remove the openvswitch module. Having the call here prevents > tunnel tests from manually cleaning up their datapath on exit, so this > patch removes it. > I don't understand this. If we consider each test as an independent sessions, should we be able to be add and remove kernel modules in between? Some kernel bugs only shows up during module clean ups. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/3] tests: Add basic vxlan tunnel sanity test.
On Wed, Jul 29, 2015 at 4:52 PM, Joe Stringer wrote: > Signed-off-by: Joe Stringer > --- > tests/kmod-macros.at | 20 > tests/kmod-traffic.at | 49 + > 2 files changed, 69 insertions(+) > > diff --git a/tests/kmod-macros.at b/tests/kmod-macros.at > index 3487c67..be3c123 100644 > --- a/tests/kmod-macros.at > +++ b/tests/kmod-macros.at > @@ -87,3 +87,23 @@ m4_define([ADD_VLAN], >AT_CHECK([ip netns exec $2 ip addr add dev $1.$3 $4]) > ] > ) > + > +# ADD_NATIVE_TUNNEL([type], [port], [namespace], [local-addr], [remote-addr], > +# [overlay-addr], [link-args]) > +# > +# Add a native tunnel device within 'namespace', with name 'port' and type > +# 'type'. The tunnel device will be configured as point-to-point with the > +# 'local-addr' address configured as the underlay address within 'namespace', > +# and 'remote-addr' as the underlay address of the remote destination (as > +# viewed from the perspective of that namespace). > +# > +# 'port' will be configured with the address 'overlay-addr'. 'link-args' is > +# made available so that additional arguments can be passed to "ip link", > +# for instance to configure the vxlan destination port. > +m4_define([ADD_NATIVE_TUNNEL], > +[ AT_CHECK([ip netns exec $3 ip link add dev $2 type $1 local $4 remote > $5 $7]) > + AT_CHECK([ip netns exec $3 ip addr add dev $2 $6]) > + AT_CHECK([ip netns exec $3 ip link set dev $2 up]) > + AT_CHECK([ip netns exec $3 ip link set dev $2 mtu 1450]) > +] > +) > diff --git a/tests/kmod-traffic.at b/tests/kmod-traffic.at > index 9df8b62..169078d 100644 > --- a/tests/kmod-traffic.at > +++ b/tests/kmod-traffic.at > @@ -105,3 +105,52 @@ AT_CHECK([cat ping.output | grep "transmitted" | sed > 's/time.*ms$/time 0ms/'], [ > > OVS_KMOD_VSWITCHD_STOP > AT_CLEANUP > + > +AT_SETUP([kmod - ping over vxlan tunnel]) > +AT_SKIP_IF([! ip link help 2>&1 | grep vxlan >/dev/null]) > + > +OVS_KMOD_VSWITCHD_START( > + [set-fail-mode br0 standalone --]) > +dnl Ensure that vport_* can be removed on exit. > +ON_EXIT([modprobe -r vport_vxlan]) > +ON_EXIT([ovs-dpctl del-dp ovs-system]) > + > +ADD_NAMESPACES(at_ns0) > + > +dnl Set up underlay link from host into the namespace using veth pair. > +AT_CHECK([ip link add p0 type veth peer name host-p0]) > +AT_CHECK([ip addr add dev host-p0 "172.31.1.100/24"]) > +AT_CHECK([ip link set dev host-p0 up]) > +AT_CHECK([ip link set p0 netns at_ns0]) > +AT_CHECK([ip netns exec at_ns0 ip addr add dev p0 "172.31.1.1/24"]) > +AT_CHECK([ip netns exec at_ns0 ip link set dev p0 up]) > + > +dnl Set up remote end of tunnel inside the namespace. > +ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.1], > [172.31.1.100], > +[10.1.1.1/24], [id 0 dstport 4789]) > + > +dnl Set up local end of tunnel in default namespace. > +AT_CHECK([ovs-vsctl add-port br0 vxlan0 -- \ > +set int vxlan0 type=vxlan options:remote_ip=172.31.1.1]) > +AT_CHECK([ip addr add dev br0 10.1.1.100/24]) > +AT_CHECK([ip link set dev br0 up]) > +AT_CHECK([ip link set dev br0 mtu 1450]) > + > +AT_CAPTURE_FILE([ping.output]) > +dnl First, check the underlay > +AT_CHECK([ip netns exec at_ns0 bash -c "ping -q -c 3 -i 0.3 -w 2 > 172.31.1.100 > ping.output"]) > + > +dnl Okay, now check the overlay with different packet sizes > +AT_CHECK([ip netns exec at_ns0 bash -c "ping -q -c 3 -i 0.3 -w 2 10.1.1.100 > >> ping.output"]) > +AT_CHECK([ip netns exec at_ns0 bash -c "ping -s 1600 -q -c 3 -i 0.3 -w 2 > 10.1.1.100 >> ping.output"]) > +AT_CHECK([ip netns exec at_ns0 bash -c "ping -s 3200 -q -c 3 -i 0.3 -w 2 > 10.1.1.100 >> ping.output"]) > + May be it is better to check ping results after each ping test? > +AT_CHECK([cat ping.output | grep "transmitted" | sed 's/time.*ms$/time > 0ms/'], [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > +OVS_KMOD_VSWITCHD_STOP > +AT_CLEANUP > -- > 2.1.4 > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 6/6] datapath: Add support for 4.1 kernel.
On Wed, Jul 29, 2015 at 12:53 PM, Joe Stringer wrote:
> diff --git a/.travis.yml b/.travis.yml
> index 70cc14b..f3236db 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -12,6 +12,7 @@ env:
>- TESTSUITE=1 KERNEL=3.18.1
>- TESTSUITE=1 OPTS="--enable-shared"
>- BUILD_ENV="-m32" OPTS="--disable-ssl"
> + - KERNEL=4.1.3
>- KERNEL=4.0.2
I wonder if we should just replace 4.0.2? I don't know if we need to
check every kernel version and it will increase build times.
> diff --git a/acinclude.m4 b/acinclude.m4
> index 45cfaf6..42866de 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -134,10 +134,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [
> AC_MSG_RESULT([$kversion])
>
> if test "$version" -ge 4; then
> - if test "$version" = 4 && test "$patchlevel" -le 0; then
> + if test "$version" = 4 && test "$patchlevel" -le 1; then
>: # Linux 4.x
> else
> - AC_ERROR([Linux kernel in $KBUILD is version $kversion, but
> version newer than 4.0.x is not supported (please refer to the FAQ for
> advice)])
> + AC_ERROR([Linux kernel in $KBUILD is version $kversion, but
> version newer than 4.1.x is not supported (please refer to the FAQ for
> advice)])
Can you update the FAQ as well?
> diff --git a/datapath/datapath.h b/datapath/datapath.h
> index 152d70c..4d1f5af 100644
> --- a/datapath/datapath.h
> +++ b/datapath/datapath.h
> static inline struct net *ovs_dp_get_net(const struct datapath *dp)
> {
> - return read_pnet(&dp->net);
> + return rpl_read_pnet(&dp->net);
> }
>
> static inline void ovs_dp_set_net(struct datapath *dp, struct net *net)
> {
> - write_pnet(&dp->net, net);
> + rpl_write_pnet(&dp->net, net);
> }
Can you define macros to handle the rpl_ prefix without needing to
have it be explicit here?
> diff --git a/datapath/linux/compat/include/net/net_namespace.h
> b/datapath/linux/compat/include/net/net_namespace.h
> index b7dbfe3..05c48f4 100644
> --- a/datapath/linux/compat/include/net/net_namespace.h
> +++ b/datapath/linux/compat/include/net/net_namespace.h
> @@ -51,4 +51,33 @@ static void rpl_unregister_pernet_gen_##TYPE(struct
> rpl_pernet_operations *rpl_p
> #define DEFINE_COMPAT_PNET_REG_FUNC(TYPE)
> #endif /* 2.6.33 */
>
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,1,0)
> +typedef struct {
> +#ifdef CONFIG_NET_NS
> + struct net *net;
> +#endif
> +} possible_net_t;
> +
> +static inline void rpl_write_pnet(possible_net_t *pnet, struct net *net)
> +{
> +#ifdef CONFIG_NET_NS
> + pnet->net = net;
> +#endif
> +}
> +
> +static inline struct net *rpl_read_pnet(const possible_net_t *pnet)
> +{
> +#ifdef CONFIG_NET_NS
> + return pnet->net;
> +#else
> + return &init_net;
> +#endif
> +}
> +#else /* Linux >= 4.1 */
> +#define hold_net(x) (x)
> +#define release_net(x) (x)
> +#define rpl_read_pnet read_pnet
> +#define rpl_write_pnet write_pnet
> +#endif /* Linux >= 4.1 */
> +
> #endif /* net/net_namespace.h wrapper */
Can we just drop use of hold_net()/release_net()? The commit message
says that they were disabled even on old kernels.
> diff --git a/datapath/linux/compat/include/net/udp_tunnel.h
> b/datapath/linux/compat/include/net/udp_tunnel.h
> index 81cb3df..221639a 100644
> --- a/datapath/linux/compat/include/net/udp_tunnel.h
> +++ b/datapath/linux/compat/include/net/udp_tunnel.h
> @@ -4,7 +4,7 @@
> #include
> #include
>
> -#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,0,0)
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,1,0)
> #include_next
>
> static inline struct sk_buff *
> @@ -71,7 +71,8 @@ void rpl_setup_udp_tunnel_sock(struct net *net, struct
> socket *sock,
>
> /* Transmit the skb using UDP encapsulation. */
> #define udp_tunnel_xmit_skb rpl_udp_tunnel_xmit_skb
> -int rpl_udp_tunnel_xmit_skb(struct rtable *rt, struct sk_buff *skb,
> +int rpl_udp_tunnel_xmit_skb(struct rtable *rt,
> + struct sock *sk, struct sk_buff *skb,
> __be32 src, __be32 dst, __u8 tos, __u8 ttl,
> __be16 df, __be16 src_port, __be16 dst_port,
> bool xnet, bool nocheck);
I think it's probably not a great idea to keep on bumping up the
version number for which we use our own backports since it would be
preferable to use the kernel implementation where we can. Can we make
the version check a little more granular?
Looking at the list of commits to 4.1, I see a couple that weren't included:
netlink: implement nla_put_in_addr and nla_put_in6_addr
netlink: implement nla_get_in_addr and nla_get_in6_addr
openvswitch: disable LRO
Can you check these and also see if there are any others?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v1 ] Make 100 percents packets sampled when sampling rate is 1.
On Thu, Jul 30, 2015 at 12:02 AM, Wenyu Zhang wrote: > When sampling rate is 1, the sampling probability is UINT32_MAX. The packet > should be sampled even the prandom32() generate the number of UINT32_MAX. > And no packet need be sampled when the probability is 0. > > Signed-off-by: Wenyu Zhang Two things: * Can you also fix the implementation of sampling in userspace in lib/odp-execute.c? * Can you send a patch against the upstream Linux kernel to the netdev mailing list? That is the place that kernel code is landing first now. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/3] kmod-macros: Don't unload kmod in VSWITCHD_STOP.
On 30 July 2015 at 11:57, Andy Zhou wrote: > On Wed, Jul 29, 2015 at 4:52 PM, Joe Stringer wrote: >> We already queue the removal of the kernel module in OVS_VSWITCHD_START, >> via an ON_EXIT() call. This redundant removal also doesn't interact very >> well with usage of vports: If the datapath still exists in the kernel, >> we cannot remove a vport module; if we cannot remove a vport module, we >> cannot remove the openvswitch module. Having the call here prevents >> tunnel tests from manually cleaning up their datapath on exit, so this >> patch removes it. >> > > I don't understand this. If we consider each test as an independent > sessions, should we be able > to be add and remove kernel modules in between? Some kernel bugs only > shows up during module > clean ups. The issue is that as long as the datapath has a tunnel port configured, you cannot remove the vport module (and you need to remove the vport module before removing openvswitch module). The solution I use here is to queue up three commands using ON_EXIT(): - During OVS_VSWITCHD_START, ON_EXIT(modprobe -r openvswitch) - At the beginning of vport tests, ON_EXIT(modprobe -r vport_vxlan) - Following this, ON_EXIT(ovs-dpctl del-dp ovs-system). These should be executed in reverse order to remove tunnel ports, remove tunnel modules, and remove the ovs module. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 12/21] ovn-controller: Slightly adjust pipeline init and destroy for consistency.
Acked-by: Justin Pettit
--Justin
> On Jul 28, 2015, at 8:44 AM, Ben Pfaff wrote:
>
> This drops an unused parameter and groups the calls to these functions
> with ofctrl_destroy() in each case.
>
> Signed-off-by: Ben Pfaff
> ---
> ovn/controller/ovn-controller.c | 5 ++---
> ovn/controller/pipeline.c | 2 +-
> ovn/controller/pipeline.h | 2 +-
> 3 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index 303a722..59feaf9 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -225,6 +225,7 @@ main(int argc, char *argv[])
> sbrec_init();
>
> ofctrl_init();
> +pipeline_init();
>
> /* Connect to OVS OVSDB instance. We do not monitor all tables by
> * default, so modules must register their interest explicitly. */
> @@ -236,8 +237,6 @@ main(int argc, char *argv[])
> binding_register_ovs_idl(ctx.ovs_idl);
> physical_register_ovs_idl(ctx.ovs_idl);
>
> -pipeline_init();
> -
> get_initial_snapshot(ctx.ovs_idl);
>
> char *ovnsb_remote = get_ovnsb_remote(ctx.ovs_idl);
> @@ -313,7 +312,7 @@ main(int argc, char *argv[])
> }
>
> unixctl_server_destroy(unixctl);
> -pipeline_destroy(&ctx);
> +pipeline_destroy();
> ofctrl_destroy();
>
> idl_loop_destroy(&ovs_idl_loop);
> diff --git a/ovn/controller/pipeline.c b/ovn/controller/pipeline.c
> index 151b9d5..4c0ffd3 100644
> --- a/ovn/controller/pipeline.c
> +++ b/ovn/controller/pipeline.c
> @@ -357,7 +357,7 @@ pipeline_run(struct controller_ctx *ctx, struct hmap
> *flow_table)
> }
>
> void
> -pipeline_destroy(struct controller_ctx *ctx OVS_UNUSED)
> +pipeline_destroy(void)
> {
> expr_symtab_destroy(&symtab);
> ldp_destroy();
> diff --git a/ovn/controller/pipeline.h b/ovn/controller/pipeline.h
> index 889fef9..7d33341 100644
> --- a/ovn/controller/pipeline.h
> +++ b/ovn/controller/pipeline.h
> @@ -43,7 +43,7 @@ struct uuid;
>
> void pipeline_init(void);
> void pipeline_run(struct controller_ctx *, struct hmap *flow_table);
> -void pipeline_destroy(struct controller_ctx *);
> +void pipeline_destroy(void);
>
> uint32_t ldp_to_integer(const struct uuid *logical_datapath);
>
> --
> 2.1.3
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 13/21] ovn-controller: Use controller_ctx just to pass around data.
Acked-by: Justin Pettit
--Justin
> On Jul 28, 2015, at 8:44 AM, Ben Pfaff wrote:
>
> Until now, controller_ctx has been a store of common state (although
> the amount of data stored in it has declined to just database state).
> I think it's clearer if we just use it as a way to pass data to
> functions. This commit makes that change.
>
> Signed-off-by: Ben Pfaff
> ---
> ovn/controller/ovn-controller.c | 50 +++--
> 1 file changed, 28 insertions(+), 22 deletions(-)
>
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index 59feaf9..12515c3 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -202,7 +202,6 @@ int
> main(int argc, char *argv[])
> {
> struct unixctl_server *unixctl;
> -struct controller_ctx ctx = { .ovs_idl = NULL };
> bool exiting;
> int retval;
>
> @@ -229,29 +228,32 @@ main(int argc, char *argv[])
>
> /* Connect to OVS OVSDB instance. We do not monitor all tables by
> * default, so modules must register their interest explicitly. */
> -ctx.ovs_idl = ovsdb_idl_create(ovs_remote, &ovsrec_idl_class, false,
> true);
> -ovsdb_idl_add_table(ctx.ovs_idl, &ovsrec_table_open_vswitch);
> -ovsdb_idl_add_column(ctx.ovs_idl, &ovsrec_open_vswitch_col_external_ids);
> -chassis_register_ovs_idl(ctx.ovs_idl);
> -encaps_register_ovs_idl(ctx.ovs_idl);
> -binding_register_ovs_idl(ctx.ovs_idl);
> -physical_register_ovs_idl(ctx.ovs_idl);
> -
> -get_initial_snapshot(ctx.ovs_idl);
> -
> -char *ovnsb_remote = get_ovnsb_remote(ctx.ovs_idl);
> -ctx.ovnsb_idl = ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class,
> - true, true);
> -get_initial_snapshot(ctx.ovnsb_idl);
> -
> -struct idl_loop ovnsb_idl_loop = IDL_LOOP_INITIALIZER(ctx.ovnsb_idl);
> -struct idl_loop ovs_idl_loop = IDL_LOOP_INITIALIZER(ctx.ovs_idl);
> +struct idl_loop ovs_idl_loop = IDL_LOOP_INITIALIZER(
> +ovsdb_idl_create(ovs_remote, &ovsrec_idl_class, false, true));
> +ovsdb_idl_add_table(ovs_idl_loop.idl, &ovsrec_table_open_vswitch);
> +ovsdb_idl_add_column(ovs_idl_loop.idl,
> + &ovsrec_open_vswitch_col_external_ids);
> +chassis_register_ovs_idl(ovs_idl_loop.idl);
> +encaps_register_ovs_idl(ovs_idl_loop.idl);
> +binding_register_ovs_idl(ovs_idl_loop.idl);
> +physical_register_ovs_idl(ovs_idl_loop.idl);
> +get_initial_snapshot(ovs_idl_loop.idl);
> +
> +/* Connect to OVN SB database. */
> +char *ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
> +struct idl_loop ovnsb_idl_loop = IDL_LOOP_INITIALIZER(
> +ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true));
> +get_initial_snapshot(ovnsb_idl_loop.idl);
>
> /* Main loop. */
> exiting = false;
> while (!exiting) {
> -ctx.ovnsb_idl_txn = idl_loop_run(&ovnsb_idl_loop);
> -ctx.ovs_idl_txn = idl_loop_run(&ovs_idl_loop);
> +struct controller_ctx ctx = {
> +.ovs_idl = ovs_idl_loop.idl,
> +.ovs_idl_txn = idl_loop_run(&ovs_idl_loop),
> +.ovnsb_idl = ovnsb_idl_loop.idl,
> +.ovnsb_idl_txn = idl_loop_run(&ovnsb_idl_loop),
> +};
>
> const struct ovsrec_bridge *br_int = get_br_int(ctx.ovs_idl);
> const char *chassis_id = get_chassis_id(ctx.ovs_idl);
> @@ -291,8 +293,12 @@ main(int argc, char *argv[])
> /* It's time to exit. Clean up the databases. */
> bool done = false;
> while (!done) {
> -ctx.ovnsb_idl_txn = idl_loop_run(&ovnsb_idl_loop);
> -ctx.ovs_idl_txn = idl_loop_run(&ovs_idl_loop);
> +struct controller_ctx ctx = {
> +.ovs_idl = ovs_idl_loop.idl,
> +.ovs_idl_txn = idl_loop_run(&ovs_idl_loop),
> +.ovnsb_idl = ovnsb_idl_loop.idl,
> +.ovnsb_idl_txn = idl_loop_run(&ovnsb_idl_loop),
> +};
>
> const struct ovsrec_bridge *br_int = get_br_int(ctx.ovs_idl);
> const char *chassis_id = get_chassis_id(ctx.ovs_idl);
> --
> 2.1.3
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 14/21] smap: New function smap_get_uuid().
Acked-by: Justin Pettit
--Justin
> On Jul 28, 2015, at 8:44 AM, Ben Pfaff wrote:
>
> To be used in an upcoming commit.
>
> Signed-off-by: Ben Pfaff
> Acked-by: Russell Bryant
> ---
> lib/smap.c | 13 -
> lib/smap.h | 4 +++-
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/lib/smap.c b/lib/smap.c
> index 7fe3ce4..9badedd 100644
> --- a/lib/smap.c
> +++ b/lib/smap.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2012, 2014 Nicira, Inc.
> +/* Copyright (c) 2012, 2014, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -19,6 +19,7 @@
>
> #include "hash.h"
> #include "json.h"
> +#include "uuid.h"
>
> static struct smap_node *smap_add__(struct smap *, char *, void *,
> size_t hash);
> @@ -215,6 +216,16 @@ smap_get_int(const struct smap *smap, const char *key,
> int def)
> return value ? atoi(value) : def;
> }
>
> +/* Gets the value associated with 'key' in 'smap' and converts it to a UUID
> + * using uuid_from_string(). Returns true if successful, false if 'key' is
> not
> + * in 'smap' or if 'key' does not have the correct syntax for a UUID. */
> +bool
> +smap_get_uuid(const struct smap *smap, const char *key, struct uuid *uuid)
> +{
> +const char *value = smap_get(smap, key);
> +return value && uuid_from_string(uuid, value);
> +}
> +
> /* Returns true of there are no elements in 'smap'. */
> bool
> smap_is_empty(const struct smap *smap)
> diff --git a/lib/smap.h b/lib/smap.h
> index caf3efc..24598a8 100644
> --- a/lib/smap.h
> +++ b/lib/smap.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2012, 2014 Nicira, Inc.
> +/* Copyright (c) 2012, 2014, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -18,6 +18,7 @@
> #include "hmap.h"
>
> struct json;
> +struct uuid;
>
> /* A map from string to string. */
> struct smap {
> @@ -57,6 +58,7 @@ const char *smap_get(const struct smap *, const char *);
> struct smap_node *smap_get_node(const struct smap *, const char *);
> bool smap_get_bool(const struct smap *smap, const char *key, bool def);
> int smap_get_int(const struct smap *smap, const char *key, int def);
> +bool smap_get_uuid(const struct smap *, const char *key, struct uuid *);
>
> bool smap_is_empty(const struct smap *);
> size_t smap_count(const struct smap *);
> --
> 2.1.3
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 15/21] nroff: Add support for 'diagram' XML element for protocol headers.
> On Jul 29, 2015, at 9:05 AM, Ben Pfaff wrote: > > On Tue, Jul 28, 2015 at 11:31:48PM -0700, Alex Wang wrote: >> On Tue, Jul 28, 2015 at 8:44 AM, Ben Pfaff wrote: >> >>> This will be used in documentation for an upcoming change, to document >>> how Geneve OVN options are encoded. >>> >>> The code in this change is from a series (not yet submitted) that makes >>> much more extensive use of it for documenting protocol headers. >>> >>> Signed-off-by: Ben Pfaff > > ... > >> Really curious what's the protocol (reference) of writing this >> (xml->manpage)? > > I've been making it up as I go along, inspired by HTML and Texinfo > mostly. > >> Also, is the 'return s' redundant? > > Yes, thanks, I removed it. I gave it a quick look-over, and it seems reasonable. --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 16/21] ovn: Rename Binding table to Port_Binding.
Acked-by: Justin Pettit
--Justin
> On Jul 28, 2015, at 8:44 AM, Ben Pfaff wrote:
>
> An upcoming patch will add a Datapath_Binding table, so clarifying the
> name seems useful.
>
> Signed-off-by: Ben Pfaff
> ---
> ovn/controller/binding.c | 28 ++---
> ovn/controller/physical.c | 4 +-
> ovn/controller/pipeline.c | 4 +-
> ovn/northd/ovn-northd.c | 105 --
> ovn/ovn-sb.ovsschema | 2 +-
> ovn/ovn-sb.xml| 8 ++--
> 6 files changed, 79 insertions(+), 72 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index c83b044..1b137bb 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -76,7 +76,7 @@ binding_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
> const char *chassis_id)
> {
> const struct sbrec_chassis *chassis_rec;
> -const struct sbrec_binding *binding_rec;
> +const struct sbrec_port_binding *binding_rec;
> struct sset lports, all_lports;
> const char *name;
>
> @@ -96,11 +96,11 @@ binding_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
> }
> sset_clone(&all_lports, &lports);
>
> -ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
> - "ovn-controller: updating bindings for '%s'",
> - chassis_id);
> +ovsdb_idl_txn_add_comment(
> +ctx->ovnsb_idl_txn,"ovn-controller: updating port bindings for '%s'",
> +chassis_id);
>
> -SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> +SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> if (sset_find_and_delete(&lports, binding_rec->logical_port) ||
> (binding_rec->parent_port && binding_rec->parent_port[0] &&
> sset_contains(&all_lports, binding_rec->parent_port))) {
> @@ -113,14 +113,14 @@ binding_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
> binding_rec->chassis->name,
> chassis_rec->name);
> }
> -sbrec_binding_set_chassis(binding_rec, chassis_rec);
> +sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
> } else if (binding_rec->chassis == chassis_rec) {
> -sbrec_binding_set_chassis(binding_rec, NULL);
> +sbrec_port_binding_set_chassis(binding_rec, NULL);
> }
> }
>
> SSET_FOR_EACH (name, &lports) {
> -VLOG_DBG("No binding record for lport %s", name);
> +VLOG_DBG("No port binding record for lport %s", name);
> }
> sset_destroy(&lports);
> sset_destroy(&all_lports);
> @@ -144,15 +144,15 @@ binding_cleanup(struct controller_ctx *ctx, const char
> *chassis_id)
> return true;
> }
>
> -ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
> - "ovn-controller: removing all bindings for
> '%s'",
> - chassis_id);
> +ovsdb_idl_txn_add_comment(
> +ctx->ovnsb_idl_txn,
> +"ovn-controller: removing all port bindings for '%s'", chassis_id);
>
> -const struct sbrec_binding *binding_rec;
> +const struct sbrec_port_binding *binding_rec;
> bool any_changes = false;
> -SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> +SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> if (binding_rec->chassis == chassis_rec) {
> -sbrec_binding_set_chassis(binding_rec, NULL);
> +sbrec_port_binding_set_chassis(binding_rec, NULL);
> any_changes = true;
> }
> }
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 11f88cb..55d6107 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -91,8 +91,8 @@ physical_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
>
> /* Set up flows in table 0 for physical-to-logical translation and in
> table
> * 64 for logical-to-physical translation. */
> -const struct sbrec_binding *binding;
> -SBREC_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> +const struct sbrec_port_binding *binding;
> +SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> /* Find the OpenFlow port for the logical port, as 'ofport'. If it's
> * on a remote chassis, this is the OpenFlow port for the tunnel to
> * that chassis (and set 'local' to false). Otherwise, if it's on the
> diff --git a/ovn/controller/pipeline.c b/ovn/controller/pipeline.c
> index 4c0ffd3..1927ce4 100644
> --- a/ovn/controller/pipeline.c
> +++ b/ovn/controller/pipeline.c
> @@ -214,8 +214,8 @@ ldp_run(struct controller_ctx *ctx)
> simap_clear(&ldp->ports);
> }
>
> -const struct sbrec_binding *binding;
> -SBREC_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
> +const struct sbrec_port_binding *binding;
> +SBREC_PORT_BINDING_FOR_EACH (binding, ct
Re: [ovs-dev] [PATCH 2/6] datapath: Constify netlink structs.
On 30 July 2015 at 10:08, Jesse Gross wrote: > On Wed, Jul 29, 2015 at 12:53 PM, Joe Stringer wrote: >> Signed-off-by: Joe Stringer >> --- >> datapath/datapath.c | 25 + >> datapath/datapath.h | 2 +- >> 2 files changed, 14 insertions(+), 13 deletions(-) > > If I remember correctly the reason why these weren't marked const is > because older kernels had functions that took these parameters as > non-const and it provoked warnings. However, it seems to me that we > have mostly redefined the places where these are passed, so perhaps > it's not an issue anymore. Can you double check to see if that's the > case? If so, this looks good to me. > > Acked-by: Jesse Gross I took a quick scan through my travis build of this, and I don't see any warnings, eg v2.6.32.65: https://travis-ci.org/joestringer/openvswitch/jobs/73239310 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 6/6] datapath: Add support for 4.1 kernel.
On 30 July 2015 at 12:24, Jesse Gross wrote:
> On Wed, Jul 29, 2015 at 12:53 PM, Joe Stringer wrote:
>> diff --git a/.travis.yml b/.travis.yml
>> index 70cc14b..f3236db 100644
>> --- a/.travis.yml
>> +++ b/.travis.yml
>> @@ -12,6 +12,7 @@ env:
>>- TESTSUITE=1 KERNEL=3.18.1
>>- TESTSUITE=1 OPTS="--enable-shared"
>>- BUILD_ENV="-m32" OPTS="--disable-ssl"
>> + - KERNEL=4.1.3
>>- KERNEL=4.0.2
>
> I wonder if we should just replace 4.0.2? I don't know if we need to
> check every kernel version and it will increase build times.
Sure, this was on my mind too. Do we have some heuristic to figure out
which kernels to test?
The ones that make the most sense to me would be LTS or those included
in major releases of popular distros. Perhaps the existing selection
of versions is more intentional than I can see, but it looks like a
random scattering to me.
> Can you update the FAQ as well?
Sure.
>> diff --git a/datapath/datapath.h b/datapath/datapath.h
>> index 152d70c..4d1f5af 100644
>> --- a/datapath/datapath.h
>> +++ b/datapath/datapath.h
>> static inline struct net *ovs_dp_get_net(const struct datapath *dp)
>> {
>> - return read_pnet(&dp->net);
>> + return rpl_read_pnet(&dp->net);
>> }
>>
>> static inline void ovs_dp_set_net(struct datapath *dp, struct net *net)
>> {
>> - write_pnet(&dp->net, net);
>> + rpl_write_pnet(&dp->net, net);
>> }
>
> Can you define macros to handle the rpl_ prefix without needing to
> have it be explicit here?
I'll have a play around with this again; I had some trouble with this
originally.
>> diff --git a/datapath/linux/compat/include/net/net_namespace.h
>> b/datapath/linux/compat/include/net/net_namespace.h
>> index b7dbfe3..05c48f4 100644
>> --- a/datapath/linux/compat/include/net/net_namespace.h
>> +++ b/datapath/linux/compat/include/net/net_namespace.h
>> @@ -51,4 +51,33 @@ static void rpl_unregister_pernet_gen_##TYPE(struct
>> rpl_pernet_operations *rpl_p
>> #define DEFINE_COMPAT_PNET_REG_FUNC(TYPE)
>> #endif /* 2.6.33 */
>>
>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,1,0)
>> +typedef struct {
>> +#ifdef CONFIG_NET_NS
>> + struct net *net;
>> +#endif
>> +} possible_net_t;
>> +
>> +static inline void rpl_write_pnet(possible_net_t *pnet, struct net *net)
>> +{
>> +#ifdef CONFIG_NET_NS
>> + pnet->net = net;
>> +#endif
>> +}
>> +
>> +static inline struct net *rpl_read_pnet(const possible_net_t *pnet)
>> +{
>> +#ifdef CONFIG_NET_NS
>> + return pnet->net;
>> +#else
>> + return &init_net;
>> +#endif
>> +}
>> +#else /* Linux >= 4.1 */
>> +#define hold_net(x) (x)
>> +#define release_net(x) (x)
>> +#define rpl_read_pnet read_pnet
>> +#define rpl_write_pnet write_pnet
>> +#endif /* Linux >= 4.1 */
>> +
>> #endif /* net/net_namespace.h wrapper */
>
> Can we just drop use of hold_net()/release_net()? The commit message
> says that they were disabled even on old kernels.
OK, sure.
>> diff --git a/datapath/linux/compat/include/net/udp_tunnel.h
>> b/datapath/linux/compat/include/net/udp_tunnel.h
>> index 81cb3df..221639a 100644
>> --- a/datapath/linux/compat/include/net/udp_tunnel.h
>> +++ b/datapath/linux/compat/include/net/udp_tunnel.h
>> @@ -4,7 +4,7 @@
>> #include
>> #include
>>
>> -#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,0,0)
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,1,0)
>> #include_next
>>
>> static inline struct sk_buff *
>> @@ -71,7 +71,8 @@ void rpl_setup_udp_tunnel_sock(struct net *net, struct
>> socket *sock,
>>
>> /* Transmit the skb using UDP encapsulation. */
>> #define udp_tunnel_xmit_skb rpl_udp_tunnel_xmit_skb
>> -int rpl_udp_tunnel_xmit_skb(struct rtable *rt, struct sk_buff *skb,
>> +int rpl_udp_tunnel_xmit_skb(struct rtable *rt,
>> + struct sock *sk, struct sk_buff *skb,
>> __be32 src, __be32 dst, __u8 tos, __u8 ttl,
>> __be16 df, __be16 src_port, __be16 dst_port,
>> bool xnet, bool nocheck);
>
> I think it's probably not a great idea to keep on bumping up the
> version number for which we use our own backports since it would be
> preferable to use the kernel implementation where we can. Can we make
> the version check a little more granular?
OK.
> Looking at the list of commits to 4.1, I see a couple that weren't included:
> netlink: implement nla_put_in_addr and nla_put_in6_addr
> netlink: implement nla_get_in_addr and nla_get_in6_addr
> openvswitch: disable LRO
>
> Can you check these and also see if there are any others?
Sure thing. I'll comb back over and look for others I've missed.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 17/21] ovn: Rename Pipeline table to Rule table.
I think Pipeline is more descriptive about what it actually is. I also find it
confusing since we use the term "rule" in the classifier. I think Flow (or
Logical_Flow) would be clearer than Rule, since we really are talking about
flows, and people may look for a distinction that isn't there. That, and the
fact that we use "rule" for a different purpose in other parts of the tree, I
think will make it more confusing.
All that said, I haven't looked ahead at the other patches yet, so maybe this
is the right choice. I'll defer to you.
Acked-by: Justin Pettit
--Justin
> On Jul 28, 2015, at 8:44 AM, Ben Pfaff wrote:
>
> The OVN pipeline is being split into two phases, which are most naturally
> called "pipelines". I kept getting very confused trying to call them
> anything else, and in the end it seems to make more sense to just rename
> the Pipeline table.
>
> It would be even better to call this table Flow or Logical_Flow, but I
> am worried that we already have far too many uses of the word "flow".
> "Rule" is slightly less overloaded in OVS.
>
> Signed-off-by: Ben Pfaff
> ---
> ovn/TODO | 2 +-
> ovn/controller/automake.mk| 6 +-
> ovn/controller/ovn-controller.c | 8 +-
> ovn/controller/physical.c | 2 +-
> ovn/controller/{pipeline.c => rule.c} | 50 +-
> ovn/controller/{pipeline.h => rule.h} | 18 ++--
> ovn/lib/actions.c | 4 +-
> ovn/northd/ovn-northd.c | 182 +-
> ovn/ovn-architecture.7.xml| 20 ++--
> ovn/ovn-nb.xml| 4 +-
> ovn/ovn-sb.ovsschema | 2 +-
> ovn/ovn-sb.xml| 6 +-
> 12 files changed, 152 insertions(+), 152 deletions(-)
> rename ovn/controller/{pipeline.c => rule.c} (89%)
> rename ovn/controller/{pipeline.h => rule.h} (79%)
>
> diff --git a/ovn/TODO b/ovn/TODO
> index 07d66da..19c95ca 100644
> --- a/ovn/TODO
> +++ b/ovn/TODO
> @@ -48,7 +48,7 @@
> Currently, clients monitor the entire contents of a table. It
> might make sense to allow clients to monitor only rows that
> satisfy specific criteria, e.g. to allow an ovn-controller to
> -receive only Pipeline rows for logical networks on its hypervisor.
> +receive only Rule rows for logical networks on its hypervisor.
>
> *** Reducing redundant data and code within ovsdb-server.
>
> diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
> index 9ed6bec..55134a3 100644
> --- a/ovn/controller/automake.mk
> +++ b/ovn/controller/automake.mk
> @@ -10,10 +10,10 @@ ovn_controller_ovn_controller_SOURCES = \
> ovn/controller/ofctrl.h \
> ovn/controller/ovn-controller.c \
> ovn/controller/ovn-controller.h \
> - ovn/controller/pipeline.c \
> - ovn/controller/pipeline.h \
> ovn/controller/physical.c \
> - ovn/controller/physical.h
> + ovn/controller/physical.h \
> + ovn/controller/rule.c \
> + ovn/controller/rule.h
> ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la lib/libopenvswitch.la
> man_MANS += ovn/controller/ovn-controller.8
> EXTRA_DIST += ovn/controller/ovn-controller.8.xml
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index 12515c3..cfd6eb9 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -44,7 +44,7 @@
> #include "chassis.h"
> #include "encaps.h"
> #include "physical.h"
> -#include "pipeline.h"
> +#include "rule.h"
>
> VLOG_DEFINE_THIS_MODULE(main);
>
> @@ -224,7 +224,7 @@ main(int argc, char *argv[])
> sbrec_init();
>
> ofctrl_init();
> -pipeline_init();
> +rule_init();
>
> /* Connect to OVS OVSDB instance. We do not monitor all tables by
> * default, so modules must register their interest explicitly. */
> @@ -266,7 +266,7 @@ main(int argc, char *argv[])
>
> if (br_int) {
> struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
> -pipeline_run(&ctx, &flow_table);
> +rule_run(&ctx, &flow_table);
> if (chassis_id) {
> physical_run(&ctx, br_int, chassis_id, &flow_table);
> }
> @@ -318,7 +318,7 @@ main(int argc, char *argv[])
> }
>
> unixctl_server_destroy(unixctl);
> -pipeline_destroy();
> +rule_destroy();
> ofctrl_destroy();
>
> idl_loop_destroy(&ovs_idl_loop);
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 55d6107..2dc96ab 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -21,7 +21,7 @@
> #include "ofpbuf.h"
> #include "ovn-controller.h"
> #include "ovn/lib/ovn-sb-idl.h"
> -#include "pipeline.h"
> +#include "rule.h"
> #include "simap.h"
> #include "vswitch-idl.h"
>
> diff --git a/ovn/controller/pipeline.c b/ovn/controller/rule.c
> similarity index 89%
> rename from ovn/controller/pipeline.c
> rename to ovn/controller/rule.c
> index 1927ce4
Re: [ovs-dev] [PATCH 0/6] datapath: Support Linux-4.1.
Thanks Jesse, applied patches 1-5 to master. I'll address your comments on patch #6 in a new series. On 29 July 2015 at 12:53, Joe Stringer wrote: > This series addresses a few low-hanging fruit of stylistic differences between > the ovs tree datapath module and the upstream linux module, and adds support > for Linux 4.1. > > Alexander Duyck (1): > datapath: Use eth_proto_is_802_3. > > Joe Stringer (5): > datapath: Whitespace fixes. > datapath: Constify netlink structs. > datapath: Use skb_postpull_rcsum(). > datapath: Backport eth_proto_is_802_3(). > datapath: Add support for 4.1 kernel. > > .travis.yml | 1 + > acinclude.m4 | 4 ++-- > datapath/actions.c| 11 ++--- > datapath/datapath.c | 29 > +-- > datapath/datapath.h | 11 - > datapath/flow.c | 8 --- > datapath/flow.h | 1 - > datapath/flow_netlink.c | 9 +++ > datapath/linux/compat/geneve.c| 2 +- > datapath/linux/compat/include/linux/etherdevice.h | 13 ++ > datapath/linux/compat/include/net/net_namespace.h | 29 > +++ > datapath/linux/compat/include/net/udp_tunnel.h| 5 ++-- > datapath/linux/compat/include/net/vxlan.h | 14 ++- > datapath/linux/compat/stt.c | 16 +++-- > datapath/linux/compat/udp_tunnel.c| 10 > datapath/linux/compat/vxlan.c | 7 +++--- > datapath/vport-lisp.c | 3 ++- > datapath/vport-vxlan.c| 2 +- > datapath/vport.c | 3 +-- > datapath/vport.h | 1 - > 20 files changed, 111 insertions(+), 68 deletions(-) > > -- > 2.1.4 > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 18/21] actions: Allow caller to specify output table.
> On Jul 28, 2015, at 8:44 AM, Ben Pfaff wrote: > > When an upcoming commit divides the pipeline up into ingress and egress > pipeline, it will become necessary to resubmit to different tables from > each of those pipelines to implement output. This commit makes that > possible. > > Signed-off-by: Ben Pfaff > --- > ovn/controller/rule.c | 2 +- > ovn/lib/actions.c | 16 +++- > ovn/lib/actions.h | 6 -- > tests/test-ovn.c | 2 +- > 4 files changed, 17 insertions(+), 9 deletions(-) > > diff --git a/ovn/controller/rule.c b/ovn/controller/rule.c > index 0f5971b..c7281a0 100644 > --- a/ovn/controller/rule.c > +++ b/ovn/controller/rule.c > @@ -283,7 +283,7 @@ rule_run(struct controller_ctx *ctx, struct hmap > *flow_table) > ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub); > next_table_id = rule->table_id < 31 ? rule->table_id + 17 : 0; > error = actions_parse_string(rule->actions, &symtab, &ldp->ports, > - next_table_id, &ofpacts, &prereqs); > + next_table_id, 64, &ofpacts, &prereqs); Do you think we should start using #defines or enums for these tables instead of magic numbers? It's hard to keep track of their values, and if we need to shuffle things around it's going to be tough to not miss some. Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 19/21] rule: Introduce MFF_LOG_DATAPATH macro for consistency.
Acked-by: Justin Pettit
--Justin
> On Jul 28, 2015, at 8:44 AM, Ben Pfaff wrote:
>
> The other logical fields have their own macros, so the logical datapath
> field might as well have one.
>
> Signed-off-by: Ben Pfaff
> ---
> ovn/controller/physical.c | 10 +-
> ovn/controller/rule.h | 7 ---
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 2dc96ab..e284a6a 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -127,7 +127,7 @@ physical_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
> }
>
> /* Translate the logical datapath into the form we use in
> - * MFF_METADATA. */
> + * MFF_LOG_DATAPATH. */
> uint32_t ldp = ldp_to_integer(&binding->logical_datapath);
> if (!ldp) {
> continue;
> @@ -147,8 +147,8 @@ physical_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
> * traffic, match on the tags and then strip the tag.
> * Priority 100 is for traffic belonging to VMs.
> *
> - * For both types of traffic: set MFF_LOG_INPORT to the
> - * logical input port, MFF_METADATA to the logical datapath, and
> + * For both types of traffic: set MFF_LOG_INPORT to the logical
> + * input port, MFF_LOG_DATAPATH to the logical datapath, and
> * resubmit into the logical pipeline starting at table 16. */
> match_init_catchall(&match);
> ofpbuf_clear(&ofpacts);
> @@ -157,9 +157,9 @@ physical_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
> match_set_dl_vlan(&match, htons(tag));
> }
>
> -/* Set MFF_METADATA. */
> +/* Set MFF_LOG_DATAPATH. */
> struct ofpact_set_field *sf = ofpact_put_SET_FIELD(&ofpacts);
> -sf->field = mf_from_id(MFF_METADATA);
> +sf->field = mf_from_id(MFF_LOG_DATAPATH);
> sf->value.be64 = htonll(ldp);
> sf->mask.be64 = OVS_BE64_MAX;
>
> diff --git a/ovn/controller/rule.h b/ovn/controller/rule.h
> index 3998994..a7bd71f 100644
> --- a/ovn/controller/rule.h
> +++ b/ovn/controller/rule.h
> @@ -37,9 +37,10 @@ struct controller_ctx;
> struct hmap;
> struct uuid;
>
> -/* Logical ports. */
> -#define MFF_LOG_INPORT MFF_REG6 /* Logical input port. */
> -#define MFF_LOG_OUTPORT MFF_REG7 /* Logical output port. */
> +/* Logical fields. */
> +#define MFF_LOG_DATAPATH MFF_METADATA /* Logical datapath (64 bits). */
> +#define MFF_LOG_INPORT MFF_REG6 /* Logical input port (32 bits). */
> +#define MFF_LOG_OUTPORT MFF_REG7 /* Logical output port (32 bits). */
>
> void rule_init(void);
> void rule_run(struct controller_ctx *, struct hmap *flow_table);
> --
> 2.1.3
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/3] tests: Add basic vxlan tunnel sanity test.
On 30 July 2015 at 11:37, Daniele Di Proietto wrote: > I get a warning in the OVS log that causes this test to fail. > > It appears that when br0 is removed (in OVS_KMOD_VSWITCHD_STOP) > OVS gets a rtnetlink message (because br0 had an address in the > routing table), but route_table_parse() fails, because > if_indextoname() returns an error (the device is not there anymore). > > Adding > > AT_CHECK([ip addr del dev br0 "10.1.1.100/24"]) > > before OVS_KMOD_VSWITCHD_STOP solves the problem for me. > Could you add that? What do you think? I guess that just deleting the datapath before cleaning up the OVS side is a bit aggressive. We could probably do this, or we should remove the bridge from ovs via ovs-vsctl first. I can roll that into the next version. > Also, I think that instead of creating the veth pair for > the tunnel underlay manually, we could use another OVS > bridge. This has two advantages: > > * The code is shorter > * Tunnelling in userspace requires an OVS bridge as > underlay. > > This is a summary of the changes I'm proposing: > > diff --git a/tests/kmod-traffic.at b/tests/kmod-traffic.at > index 169078d..346d464 100644 > --- a/tests/kmod-traffic.at > +++ b/tests/kmod-traffic.at > @@ -110,20 +110,18 @@ AT_SETUP([kmod - ping over vxlan tunnel]) > AT_SKIP_IF([! ip link help 2>&1 | grep vxlan >/dev/null]) > > OVS_KMOD_VSWITCHD_START( > - [set-fail-mode br0 standalone --]) > + [set-fail-mode br0 standalone --dnl > +add-br br-ovs-p0]) > dnl Ensure that vport_* can be removed on exit. > ON_EXIT([modprobe -r vport_vxlan]) > ON_EXIT([ovs-dpctl del-dp ovs-system]) > > ADD_NAMESPACES(at_ns0) > > +ADD_VETH(p0, at_ns0, br-ovs-p0, "172.31.1.1/24") > dnl Set up underlay link from host into the namespace using veth pair. > -AT_CHECK([ip link add p0 type veth peer name host-p0]) > -AT_CHECK([ip addr add dev host-p0 "172.31.1.100/24"]) > -AT_CHECK([ip link set dev host-p0 up]) > -AT_CHECK([ip link set p0 netns at_ns0]) > -AT_CHECK([ip netns exec at_ns0 ip addr add dev p0 "172.31.1.1/24"]) > -AT_CHECK([ip netns exec at_ns0 ip link set dev p0 up]) > +AT_CHECK([ip addr add dev br-ovs-p0 "172.31.1.100/24"]) > +AT_CHECK([ip link set dev br-ovs-p0 up]) > > dnl Set up remote end of tunnel inside the namespace. > ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.1], > [172.31.1.100], > @@ -152,5 +150,8 @@ AT_CHECK([cat ping.output | grep "transmitted" | sed > 's/time.*ms$/time 0ms/'], [ > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > ]) > > +AT_CHECK([ip addr del dev br0 "10.1.1.100/24"]) > +AT_CHECK([ip addr del dev br-ovs-p0 "172.31.1.100/24"]) > + > OVS_KMOD_VSWITCHD_STOP > AT_CLEANUP > > > I'll leave it up to you, I can also make this change in my next series. > > Acked-by: Daniele Di Proietto This looks like a reasonable change. I'll test it out and roll it in with a v2, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/3] tests: Add basic vxlan tunnel sanity test.
On 30 July 2015 at 12:02, Andy Zhou wrote: > On Wed, Jul 29, 2015 at 4:52 PM, Joe Stringer wrote: >> Signed-off-by: Joe Stringer >> --- >> tests/kmod-macros.at | 20 >> tests/kmod-traffic.at | 49 + >> 2 files changed, 69 insertions(+) >> >> diff --git a/tests/kmod-macros.at b/tests/kmod-macros.at >> index 3487c67..be3c123 100644 >> --- a/tests/kmod-macros.at >> +++ b/tests/kmod-macros.at >> @@ -87,3 +87,23 @@ m4_define([ADD_VLAN], >>AT_CHECK([ip netns exec $2 ip addr add dev $1.$3 $4]) >> ] >> ) >> + >> +# ADD_NATIVE_TUNNEL([type], [port], [namespace], [local-addr], >> [remote-addr], >> +# [overlay-addr], [link-args]) >> +# >> +# Add a native tunnel device within 'namespace', with name 'port' and type >> +# 'type'. The tunnel device will be configured as point-to-point with the >> +# 'local-addr' address configured as the underlay address within >> 'namespace', >> +# and 'remote-addr' as the underlay address of the remote destination (as >> +# viewed from the perspective of that namespace). >> +# >> +# 'port' will be configured with the address 'overlay-addr'. 'link-args' is >> +# made available so that additional arguments can be passed to "ip link", >> +# for instance to configure the vxlan destination port. >> +m4_define([ADD_NATIVE_TUNNEL], >> +[ AT_CHECK([ip netns exec $3 ip link add dev $2 type $1 local $4 remote >> $5 $7]) >> + AT_CHECK([ip netns exec $3 ip addr add dev $2 $6]) >> + AT_CHECK([ip netns exec $3 ip link set dev $2 up]) >> + AT_CHECK([ip netns exec $3 ip link set dev $2 mtu 1450]) >> +] >> +) >> diff --git a/tests/kmod-traffic.at b/tests/kmod-traffic.at >> index 9df8b62..169078d 100644 >> --- a/tests/kmod-traffic.at >> +++ b/tests/kmod-traffic.at >> @@ -105,3 +105,52 @@ AT_CHECK([cat ping.output | grep "transmitted" | sed >> 's/time.*ms$/time 0ms/'], [ >> >> OVS_KMOD_VSWITCHD_STOP >> AT_CLEANUP >> + >> +AT_SETUP([kmod - ping over vxlan tunnel]) >> +AT_SKIP_IF([! ip link help 2>&1 | grep vxlan >/dev/null]) >> + >> +OVS_KMOD_VSWITCHD_START( >> + [set-fail-mode br0 standalone --]) >> +dnl Ensure that vport_* can be removed on exit. >> +ON_EXIT([modprobe -r vport_vxlan]) >> +ON_EXIT([ovs-dpctl del-dp ovs-system]) >> + >> +ADD_NAMESPACES(at_ns0) >> + >> +dnl Set up underlay link from host into the namespace using veth pair. >> +AT_CHECK([ip link add p0 type veth peer name host-p0]) >> +AT_CHECK([ip addr add dev host-p0 "172.31.1.100/24"]) >> +AT_CHECK([ip link set dev host-p0 up]) >> +AT_CHECK([ip link set p0 netns at_ns0]) >> +AT_CHECK([ip netns exec at_ns0 ip addr add dev p0 "172.31.1.1/24"]) >> +AT_CHECK([ip netns exec at_ns0 ip link set dev p0 up]) >> + >> +dnl Set up remote end of tunnel inside the namespace. >> +ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.1], >> [172.31.1.100], >> +[10.1.1.1/24], [id 0 dstport 4789]) >> + >> +dnl Set up local end of tunnel in default namespace. >> +AT_CHECK([ovs-vsctl add-port br0 vxlan0 -- \ >> +set int vxlan0 type=vxlan options:remote_ip=172.31.1.1]) >> +AT_CHECK([ip addr add dev br0 10.1.1.100/24]) >> +AT_CHECK([ip link set dev br0 up]) >> +AT_CHECK([ip link set dev br0 mtu 1450]) >> + >> +AT_CAPTURE_FILE([ping.output]) >> +dnl First, check the underlay >> +AT_CHECK([ip netns exec at_ns0 bash -c "ping -q -c 3 -i 0.3 -w 2 >> 172.31.1.100 > ping.output"]) >> + >> +dnl Okay, now check the overlay with different packet sizes >> +AT_CHECK([ip netns exec at_ns0 bash -c "ping -q -c 3 -i 0.3 -w 2 10.1.1.100 >> >> ping.output"]) >> +AT_CHECK([ip netns exec at_ns0 bash -c "ping -s 1600 -q -c 3 -i 0.3 -w 2 >> 10.1.1.100 >> ping.output"]) >> +AT_CHECK([ip netns exec at_ns0 bash -c "ping -s 3200 -q -c 3 -i 0.3 -w 2 >> 10.1.1.100 >> ping.output"]) >> + > May be it is better to check ping results after each ping test? Sure, the output from failed tests would be more clear if I do this. I'll roll this into the other patches too. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] type-props: Suppress warnings in newer Clang and GCC.
Until now, Clang 3.7+ and sufficiently new versions of GCC complained about TYPE_MAXIMUM(int), etc., because it shifts a negative value. This commit fixes the problem. This commit also gives these macros sensible definitions for _Bool, and documents all of them. Reported-by: Joe Stringer Signed-off-by: Ben Pfaff --- lib/type-props.h| 32 tests/test-type-props.c | 6 +- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/lib/type-props.h b/lib/type-props.h index 3c908a7..e5f4fdc 100644 --- a/lib/type-props.h +++ b/lib/type-props.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2011 Nicira, Inc. + * Copyright (c) 2008, 2011, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,15 +19,31 @@ #include +/* True if TYPE is _Bool, false otherwise. */ +#define TYPE_IS_BOOL(TYPE) ((TYPE) 1 == (TYPE) 2) + +/* True if TYPE is an integer type (including _Bool), false if it is a + * floating-point type. */ #define TYPE_IS_INTEGER(TYPE) ((TYPE) 1.5 == (TYPE) 1) + +/* True if TYPE is a signed integer or floating point type, otherwise false. */ #define TYPE_IS_SIGNED(TYPE) ((TYPE) 1 > (TYPE) -1) -#define TYPE_VALUE_BITS(TYPE) (sizeof(TYPE) * CHAR_BIT - TYPE_IS_SIGNED(TYPE)) -#define TYPE_MINIMUM(TYPE) (TYPE_IS_SIGNED(TYPE) \ -? ~(TYPE)0 << (sizeof(TYPE) * 8 - 1) \ -: 0) -#define TYPE_MAXIMUM(TYPE) (TYPE_IS_SIGNED(TYPE) \ -? ~(~(TYPE)0 << (sizeof(TYPE) * 8 - 1)) \ -: (TYPE)-1) + +/* The number of value bits in an signed or unsigned integer TYPE: + * + *- _Bool has 1 value bit. + * + *- An N-bit unsigned integer type has N value bits. + * + *- An N-bit signed integer type has N-1 value bits. + */ +#define TYPE_VALUE_BITS(TYPE) \ +(TYPE_IS_BOOL(TYPE) ? 1 : sizeof(TYPE) * CHAR_BIT - TYPE_IS_SIGNED(TYPE)) + +/* The minimum or maximum value of a signed or unsigned integer TYPE. */ +#define TYPE_MINIMUM(TYPE) (TYPE_IS_SIGNED(TYPE) ? -TYPE_MAXIMUM(TYPE) - 1 : 0) +#define TYPE_MAXIMUM(TYPE) \ +TYPE)1 << (TYPE_VALUE_BITS(TYPE) - 1)) - 1) * 2 + 1) /* Number of decimal digits required to format an integer of the given TYPE. * Includes space for a sign, if TYPE is signed, but not for a null diff --git a/tests/test-type-props.c b/tests/test-type-props.c index 3f24725..3c351eb 100644 --- a/tests/test-type-props.c +++ b/tests/test-type-props.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2011 Nicira, Inc. + * Copyright (c) 2008, 2009, 2011, 2015 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -44,6 +44,10 @@ main (void) char max_s[128]; char min_s[128]; +#ifndef __CHECKER__ /* sparse hates sizeof(bool). */ +TEST_TYPE(_Bool, 0, 1, 0); +#endif + TEST_TYPE(char, CHAR_MIN, CHAR_MAX, (CHAR_MIN < 0)); TEST_TYPE(signed char, SCHAR_MIN, SCHAR_MAX, 1); -- 2.1.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] type-props: Suppress warnings in newer Clang and GCC.
On 30 July 2015 at 17:46, Ben Pfaff wrote: > Until now, Clang 3.7+ and sufficiently new versions of GCC complained about > TYPE_MAXIMUM(int), etc., because it shifts a negative value. This commit > fixes the problem. > > This commit also gives these macros sensible definitions for _Bool, and > documents all of them. > > Reported-by: Joe Stringer > Signed-off-by: Ben Pfaff This fixes the build for me. I assume we have a test to determine this is performing the correct behaviour? Acked-by: Joe Stringer ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] type-props: Suppress warnings in newer Clang and GCC.
On Thu, Jul 30, 2015 at 05:57:13PM -0700, Joe Stringer wrote: > On 30 July 2015 at 17:46, Ben Pfaff wrote: > > Until now, Clang 3.7+ and sufficiently new versions of GCC complained about > > TYPE_MAXIMUM(int), etc., because it shifts a negative value. This commit > > fixes the problem. > > > > This commit also gives these macros sensible definitions for _Bool, and > > documents all of them. > > > > Reported-by: Joe Stringer > > Signed-off-by: Ben Pfaff > > This fixes the build for me. I assume we have a test to determine this > is performing the correct behaviour? Yes, the "type properties" test run by make check. This commit modifies the .c file that implements that test, and it does pass. > Acked-by: Joe Stringer Thanks, I'll apply this to master and branch-2.4 in a minute. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN - L3 Gap between NB schema and Neutron
Hi Gal, So you're saying that ml2 allows you to configure a topology like this? VM (10.0.0.2) Logical_Switch(10.0.0.2)LogicalRouter | | +--(10.0.0.3)--Logical-Router--WAN And then the vm would be responsible for having specific routes to each gw ip? I think you're right that this will work with the current L3 agent. That said, I wondering if it's even worth supporting this topology if it's complex to implement and there are not many use cases for it (or being requested by users). I haven't heard anyone asking for this before (and nvp doesn't implement this either fwiw). As an alternative to accomplishing the same thing one could use a VM with two ports. Aaron On Thu, Jul 30, 2015 at 11:28 AM, Ben Pfaff wrote: > [also adding Salvatore] > > On Thu, Jul 30, 2015 at 11:27:57AM -0700, Ben Pfaff wrote: > > If both the router ports point to the same router, then I am not sure > > why this would need to be two ports. Maybe the schema is not sufficient > > to report both IPv4 and IPv6 addresses on a single router port; if so, > > then I would support enhancing the schema to fix that. > > > > I suspect that for connecting to two different routers, it is possible > > to instead connect one router and then connect that router to others in > > a way that accomplishes an equivalent goal. I haven't thought it > > through though. > > > > On Thu, Jul 30, 2015 at 09:12:14PM +0300, Gal Sagie wrote: > > > Yes, i checked this on my setup. > > > For example, you can have both IPv6 and IPv4 subnets per the same > network > > > (which maps to a logical switch) > > > and connect both as two different router ports (to the same router) > > > > > > You can also connect the same network to two different routers, i am > not > > > sure if you need the extra route extension for that or not, i think you > > > could > > > configure it as default gateway with out this extension, but with the > > > extension you > > > can define routing between the two routers. > > > > > > > > > > > > > > > > > > On Thu, Jul 30, 2015 at 9:03 PM, Ben Pfaff wrote: > > > > > > > [adding Aaron Rosen] > > > > > > > > On Wed, Jul 29, 2015 at 12:20:30PM +0300, Gal Sagie wrote: > > > > > Currently Neutron support defining few subnets (IP cidrs) on a > network > > > > > (logical switch) > > > > > and connecting them to the same router (or different routers). > > > > > Currently in the NB schema, the logical switch can be connected > only to > > > > one > > > > > logical > > > > > router port. > > > > > > > > > > This needs to be extended so a logical switch can have more then > one > > > > > logical router > > > > > port reference to support the above use case. > > > > > > > > Limiting a logical switch to a single router port is an intentional > > > > design decision. It means that a packet traverses at most two > logical > > > > switches (one at ingress, one at egress), which simplifies some of > the > > > > logical switch design, and it prevents loops. > > > > > > > > VMware's NVP controller uses the same design, for those reasons and > > > > others. The NVP paper from NSDI 2014 (see > > > > http://benpfaff.org/papers/net-virt.pdf) puts it this way: > > > > > > > > As an optimization, we constrain the logical topology such that > > > > logical L2 destinations can only be present at its edge[6]. This > > > > restriction means that the OVS flow table of a sending hypervisor > > > > needs only to have flows for logical datapaths to which its local > > > > VMs are attached as well as those of the L3 routers of the > logical > > > > topology; the receiving hypervisor is determined by the logical > IP > > > > destination address, leaving the last logical L2 hop to be > executed > > > > at the receiving hypervisor. > > > > > > > > [6] We have found little value in supporting logical routers > > > > interconnected through logical switches without tenant VMs. > > > > > > > > Are you sure that Neutron supports multiple router ports per switch? > > > > Russell Bryant (in IRC) and Aaron Rosen (in a quick in-person chat) > > > > seemed doubtful. > > > > > > > > > > > > > > > > -- > > > Best Regards , > > > > > > The G. > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] datapath: Fix STT protocol field for sampling packet.
Fixes typo in STT sampling code.
Signed-off-by: Pravin B Shelar
---
datapath/vport-stt.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/datapath/vport-stt.c b/datapath/vport-stt.c
index 9a1c8a6..4eb0282 100644
--- a/datapath/vport-stt.c
+++ b/datapath/vport-stt.c
@@ -189,7 +189,7 @@ static int stt_get_egress_tun_info(struct vport *vport,
struct sk_buff *skb,
return ovs_tunnel_get_egress_info(egress_tun_info,
ovs_dp_get_net(vport->dp),
OVS_CB(skb)->egress_tun_info,
- IPPROTO_UDP, skb->mark, sport, dport);
+ IPPROTO_TCP, skb->mark, sport, dport);
}
static struct vport_ops ovs_stt_vport_ops = {
--
1.7.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
