Re: [ovs-dev] [PATCH v2] dpif-netdev: fix race for queues between pmd threads
I agree. It made sense with my first fix. Fixed and sent again.
Thanks.
On 27.07.2015 19:58, Daniele Di Proietto wrote:
> Thanks for the updated patch.
>
> One comment below
>
> On 27/07/2015 13:19, "Ilya Maximets" wrote:
>
>> Currently pmd threads select queues in pmd_load_queues() according to
>> get_n_pmd_threads_on_numa(). This behavior leads to race between pmds,
>> beacause dp_netdev_set_pmds_on_numa() starts them one by one and
>> current number of threads changes incrementally.
>>
>> As a result we may have the following situation with 2 pmd threads:
>>
>> * dp_netdev_set_pmds_on_numa()
>> * pmd12 thread started. Currently only 1 pmd thread exists.
>> dpif_netdev(pmd12)|INFO|Core 1 processing port 'port_1'
>> dpif_netdev(pmd12)|INFO|Core 1 processing port 'port_2'
>> * pmd14 thread started. 2 pmd threads exists.
>> dpif_netdev|INFO|Created 2 pmd threads on numa node 0
>> dpif_netdev(pmd14)|INFO|Core 2 processing port 'port_2'
>>
>> We have:
>> core 1 --> port 1, port 2
>> core 2 --> port 2
>>
>> Fix this by starting pmd threads only after all of them have
>> been configured.
>>
>> Cc: Daniele Di Proietto
>> Cc: Dyasly Sergey
>> Signed-off-by: Ilya Maximets
>> ---
>> lib/dpif-netdev.c | 21 ++---
>> 1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 79c4612..4fca7b7 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -1123,10 +1123,9 @@ do_add_port(struct dp_netdev *dp, const char
>> *devname, const char *type,
>> ovs_refcount_init(&port->ref_cnt);
>> cmap_insert(&dp->ports, &port->node, hash_port_no(port_no));
>>
>> -if (netdev_is_pmd(netdev)) {
>> +if (netdev_is_pmd(netdev))
>> dp_netdev_set_pmds_on_numa(dp, netdev_get_numa_id(netdev));
>> -dp_netdev_reload_pmds(dp);
>> -}
>> +
>
> I think we should still call 'dp_netdev_reload_pmds()' when adding a
> new port.
>
>> seq_change(dp->port_seq);
>>
>> return 0;
>> @@ -2961,6 +2960,7 @@ dp_netdev_set_pmds_on_numa(struct dp_netdev *dp,
>> int numa_id)
>> * pmd threads for the numa node. */
>> if (!n_pmds) {
>> int can_have, n_unpinned, i;
>> +struct dp_netdev_pmd_thread **pmds;
>>
>> n_unpinned = ovs_numa_get_n_unpinned_cores_on_numa(numa_id);
>> if (!n_unpinned) {
>> @@ -2972,15 +2972,22 @@ dp_netdev_set_pmds_on_numa(struct dp_netdev *dp,
>> int numa_id)
>> /* If cpu mask is specified, uses all unpinned cores, otherwise
>> * tries creating NR_PMD_THREADS pmd threads. */
>> can_have = dp->pmd_cmask ? n_unpinned : MIN(n_unpinned,
>> NR_PMD_THREADS);
>> +pmds = xzalloc(can_have * sizeof *pmds);
>> for (i = 0; i < can_have; i++) {
>> -struct dp_netdev_pmd_thread *pmd = xzalloc(sizeof *pmd);
>> unsigned core_id =
>> ovs_numa_get_unpinned_core_on_numa(numa_id);
>> -
>> -dp_netdev_configure_pmd(pmd, dp, i, core_id, numa_id);
>> +pmds[i] = xzalloc(sizeof **pmds);
>> +dp_netdev_configure_pmd(pmds[i], dp, i, core_id, numa_id);
>> +}
>> +/* The pmd thread code needs to see all the others configured pmd
>> + * threads on the same numa node. That's why we call
>> + * 'dp_netdev_configure_pmd()' on all the threads and then we
>> actually
>> + * start them. */
>> +for (i = 0; i < can_have; i++) {
>> /* Each thread will distribute all devices rx-queues among
>> * themselves. */
>> -pmd->thread = ovs_thread_create("pmd", pmd_thread_main, pmd);
>> +pmds[i]->thread = ovs_thread_create("pmd", pmd_thread_main,
>> pmds[i]);
>> }
>> +free(pmds);
>> VLOG_INFO("Created %d pmd threads on numa node %d", can_have,
>> numa_id);
>> }
>> }
>> --
>> 2.1.4
>>
>
>
___
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.
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
[ovs-dev] [PATCH v2] datapath-windows: Avoid BSOD when cleaning up a tunnel vport
If an error appears when creating a tunnel vport the cleanup is performed
twice, which causes the tunnel vport to be released also twice and
generate a BSOD.
This patch modifies the tunnel filter cleanup logic to avoid this issue.
Signed-off-by: Sorin Vinturis
Reported-by: Sorin Vinturis
Reported-at: https://github.com/openvswitch/ovs-issues/issues/97
Acked-by: Nithin Raju
---
v2: added acked.
---
datapath-windows/ovsext/TunnelFilter.c | 1 -
datapath-windows/ovsext/Vport.c| 7 ++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/datapath-windows/ovsext/TunnelFilter.c
b/datapath-windows/ovsext/TunnelFilter.c
index d736c19..7865451 100644
--- a/datapath-windows/ovsext/TunnelFilter.c
+++ b/datapath-windows/ovsext/TunnelFilter.c
@@ -1419,7 +1419,6 @@ OvsTunnelFilterQueueRequest(PIRP irp,
} while (error);
if (error) {
-OvsTunnelFilterCompleteRequest(irp, callback, tunnelContext, status);
if (request) {
OvsFreeMemory(request);
request = NULL;
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index d9f489f..b2ff308 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -920,7 +920,8 @@ OvsInitTunnelVport(PVOID userContext,
{
POVS_TUNFLT_INIT_CONTEXT tunnelContext = NULL;
-tunnelContext = OvsAllocateMemory(sizeof(*tunnelContext));
+tunnelContext = OvsAllocateMemoryWithTag(sizeof(*tunnelContext),
+ OVS_VPORT_POOL_TAG);
if (tunnelContext == NULL) {
status = STATUS_INSUFFICIENT_RESOURCES;
break;
@@ -935,6 +936,10 @@ OvsInitTunnelVport(PVOID userContext,
dstPort,
OvsTunnelVportPendingInit,
(PVOID)tunnelContext);
+if (status != STATUS_PENDING) {
+OvsFreeMemoryWithTag(tunnelContext, OVS_VPORT_POOL_TAG);
+tunnelContext = NULL;
+}
break;
}
case OVS_VPORT_TYPE_STT:
--
1.9.0.msysgit.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] kernel module testing
> > > For what it's worth, I also think that something like Gerrit would be > > > useful given the number of platforms that OVS is running on. Right > > > now, it's seems like we're doing the human-powered version, which is > > > Guru, Daniele, or Ben complain when something breaks Windows, DPDK, > > > 32-bit respectively. It also effectively provides the features of > > > Patchwork in a way that is more maintainable. > > > > > > I agree that the Gerrit UI sucks (I haven't tried the OpenStack > > > interface) and maybe there are alternatives, like Github's set of > > > tools. But I think the status quo that we have isn't all that great > > > either and I also would like to avoid having a collection of > > > independent tools that fall apart over time. > > > > I'm happy to encourage people to submit changes via Github, as an > > experiment. > > I think this is a reasonable place to start, especially since we > already have the infrastructure set up. I think the main thing to try > out is whether we are OK with this type of workflow for reviewing > patches as opposed to the mailing list, so at some level it doesn't > matter where we start. > > I don't think that Travis CI is good enough to handle all of the > pre-checkin tests that we would like to do (kernel, Windows, > performance, etc.) but it looks like there are other tools that have > integration with Github that minimally should be the same as what > Gerrit can do. I'm obliged to point out the work that's ongoing to get some level of automated testing going in patchwork (for use with DPDK): https://lists.ozlabs.org/pipermail/patchwork/2015-July/001363.html However, I still think Gerrit is the more mature, feature-filled solution if mailing list-based development isn't an absolute necessity. OpenStack really have the Gerrit process nailed. Someone really needs to fix that web UI though... Stephen PS: Third party CI support (a lá OpenStack) would be an incredibly useful feature to offload some level of testing (like performance testing of the DPDK element of OVS). It's probably not an option with GerritHub (I haven't checked), but if you're deciding to go all in on Gerrit or not then this is one for the PROS column. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v1 2/2] ofproto: Implement OF1.4 Group & Meter change notification messages
Hi Ben, Thanks for the review. All the comments have been incorporated in Version 2 of the patch. Thanks Niti Rohilla On Tue, Jul 28, 2015 at 1:23 AM, Ben Pfaff wrote: > On Thu, Jul 23, 2015 at 05:06:36PM +0530, niti1...@gmail.com wrote: > > From: Niti Rohilla > > > > This patch adds support for Openflow1.4 Group & meter change notification > > messages. In a multi controller environment, when a controller modifies > the > > state of group and meter table, the request that successfully modifies > this > > state is forwarded to other controllers. Other controllers are informed > with > > the OFPT_REQUESTFORWARD message. Request forwarding is enabled on a per > > controller channel basis using the Set Asynchronous Configuration > Message. > > > > Signed-off-by: Niti Rohilla > > Thank you for working on this. I have some comments. > > ofp_print_requestforward() will abort the program if a received > OFPT_REQUESTFORWARD message contains any message other than a group mod > or a meter mod. That's not acceptable. > > In ofputil_encode_requestforward(), for efficiency, the final argument > to ofpraw_alloc_xid() should be the length of 'rf'. > > The message inside OFPT_REQUESTFORWARD needs to have the same OpenFlow > version as the wrapper. ofputil_decode_requestforward() should check > that, and report an error if it differs. It also means > ofputil_encode_requestforward() doesn't need a protocol version > parameter (since it can be inferred from 'rf') and that > connmgr_send_requestforward() needs to somehow make the inner message > compatible with 'ofconn'. I think that probably has to be done by > re-encoding the inner message for the protocol in use on each 'ofconn'. > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 2/2] ofproto: Implement OF1.4 Group & Meter change notification messages
From: Niti Rohilla
This patch adds support for Openflow1.4 Group & meter change notification
messages. In a multi controller environment, when a controller modifies the
state of group and meter table, the request that successfully modifies this
state is forwarded to other controllers. Other controllers are informed with
the OFPT_REQUESTFORWARD message. Request forwarding is enabled on a per
controller channel basis using the Set Asynchronous Configuration Message.
---
Comments recevied on 28th July,2015 have been incorporated in this version.
Following are the changes between v1 and v2:
1. Modified ofp_print_requestforward(), now the program will not abort if a
received OFPT_REQUESTFORWARD message contains any message other than a group
mod or a meter mod.
2. Modified ofputil_encode_requestforward(), the final argument to
ofpraw_alloc_xid() is the length of 'rf->request'.
3. Added a function ofputil_re_encode_requestforward() to re-encode the inner
message for the protocol in use on each 'ofconn'. Modified
ofputil_decode_requestforward() to match the versions of OFPT_REQUESTFORWARD
and encapsulated "GROUP_MOD" or "METER_MOD" request and reports an error if
it differs.
include/openflow/openflow-1.4.h | 6
lib/learning-switch.c | 1 +
lib/ofp-msgs.h | 6
lib/ofp-print.c | 36 +++
lib/ofp-util.c | 68
lib/ofp-util.h | 14
lib/rconn.c | 1 +
ofproto/connmgr.c | 37
ofproto/connmgr.h | 3 ++
ofproto/ofproto.c | 25 +++---
ovn/controller/ofctrl.c | 1 +
tests/ofp-print.at | 46 +
tests/ofproto.at| 76 +
13 files changed, 315 insertions(+), 5 deletions(-)
diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
index 37eef4a..5202fe1 100644
--- a/include/openflow/openflow-1.4.h
+++ b/include/openflow/openflow-1.4.h
@@ -355,6 +355,12 @@ struct ofp14_role_prop_experimenter {
};
OFP_ASSERT(sizeof(struct ofp14_role_prop_experimenter) == 12);
+/* Group/Meter request forwarding. */
+struct ofp14_requestforward {
+struct ofp_header request; /* Request being forwarded. */
+};
+OFP_ASSERT(sizeof(struct ofp14_requestforward) == 8);
+
/* Bundle control message types */
enum ofp14_bundle_ctrl_type {
OFPBCT_OPEN_REQUEST= 0,
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 1753cea..7ddf69b 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -419,6 +419,7 @@ lswitch_process_packet(struct lswitch *sw, const struct
ofpbuf *msg)
case OFPTYPE_ROLE_REQUEST:
case OFPTYPE_ROLE_REPLY:
case OFPTYPE_ROLE_STATUS:
+case OFPTYPE_REQUESTFORWARD:
case OFPTYPE_SET_FLOW_FORMAT:
case OFPTYPE_FLOW_MOD_TABLE_ID:
case OFPTYPE_SET_PACKET_IN_FORMAT:
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 1358b21..52a74d1 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -247,6 +247,9 @@ enum ofpraw {
/* OFPT 1.4+ (30): struct ofp14_role_status, uint8_t[8][]. */
OFPRAW_OFPT14_ROLE_STATUS,
+/* OFPT 1.4+ (32): struct ofp14_requestforward, uint8_t[8][]. */
+OFPRAW_OFPT14_REQUESTFORWARD,
+
/* OFPT 1.4+ (33): struct ofp14_bundle_ctrl_msg, uint8_t[8][]. */
OFPRAW_OFPT14_BUNDLE_CONTROL,
@@ -559,6 +562,9 @@ enum ofptype {
/* Controller role change event messages. */
OFPTYPE_ROLE_STATUS, /* OFPRAW_OFPT14_ROLE_STATUS. */
+/* Request forwarding by the switch. */
+OFPTYPE_REQUESTFORWARD, /* OFPRAW_OFPT14_REQUESTFORWARD. */
+
OFPTYPE_BUNDLE_CONTROL, /* OFPRAW_OFPT14_BUNDLE_CONTROL. */
OFPTYPE_BUNDLE_ADD_MESSAGE, /* OFPRAW_OFPT14_BUNDLE_ADD_MESSAGE. */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 6db32d1..f6e665d 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2960,6 +2960,38 @@ ofp_print_geneve_table_reply(struct ds *s, const struct
ofp_header *oh)
ofputil_uninit_geneve_table(>r.mappings);
}
+/* This function will print the request forward message. The reason for
+ * request forward is taken from rf.request.type */
+static void
+ofp_print_requestforward(struct ds *string, const struct ofp_header *oh)
+{
+struct ofputil_requestforward rf;
+enum ofperr error;
+enum ofptype type;
+
+error = ofputil_decode_requestforward(oh, &rf);
+if (error) {
+ofp_print_error(string, error);
+return;
+}
+
+error = ofptype_decode(&type, rf.request);
+if (error) {
+ofp_print_error(string, error);
+return;
+}
+
+ds_put_cstr(string, " reason=");
+
+if (type == OFPTYPE_GROUP_MOD) {
+ds_put_cstr(string, "group_mod");
+ofp_print_group_mod(string, rf.request);
+} else if (type == OFPTYPE_METER_MOD) {
+ds_put_cstr(string, "meter_mo
[ovs-dev] [PATCH v3 2/2] ofproto: Implement OF1.4 Group & Meter change notification messages
From: Niti Rohilla
This patch adds support for Openflow1.4 Group & meter change notification
messages. In a multi controller environment, when a controller modifies the
state of group and meter table, the request that successfully modifies this
state is forwarded to other controllers. Other controllers are informed with
the OFPT_REQUESTFORWARD message. Request forwarding is enabled on a per
controller channel basis using the Set Asynchronous Configuration Message.
Signed-off-by: Niti Rohilla
---
Following are the changes between v2 and v3:
Added Signed-off
include/openflow/openflow-1.4.h | 6
lib/learning-switch.c | 1 +
lib/ofp-msgs.h | 6
lib/ofp-print.c | 36 +++
lib/ofp-util.c | 68
lib/ofp-util.h | 14
lib/rconn.c | 1 +
ofproto/connmgr.c | 37
ofproto/connmgr.h | 3 ++
ofproto/ofproto.c | 25 +++---
ovn/controller/ofctrl.c | 1 +
tests/ofp-print.at | 46 +
tests/ofproto.at| 76 +
13 files changed, 315 insertions(+), 5 deletions(-)
diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
index 37eef4a..5202fe1 100644
--- a/include/openflow/openflow-1.4.h
+++ b/include/openflow/openflow-1.4.h
@@ -355,6 +355,12 @@ struct ofp14_role_prop_experimenter {
};
OFP_ASSERT(sizeof(struct ofp14_role_prop_experimenter) == 12);
+/* Group/Meter request forwarding. */
+struct ofp14_requestforward {
+struct ofp_header request; /* Request being forwarded. */
+};
+OFP_ASSERT(sizeof(struct ofp14_requestforward) == 8);
+
/* Bundle control message types */
enum ofp14_bundle_ctrl_type {
OFPBCT_OPEN_REQUEST= 0,
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 1753cea..7ddf69b 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -419,6 +419,7 @@ lswitch_process_packet(struct lswitch *sw, const struct
ofpbuf *msg)
case OFPTYPE_ROLE_REQUEST:
case OFPTYPE_ROLE_REPLY:
case OFPTYPE_ROLE_STATUS:
+case OFPTYPE_REQUESTFORWARD:
case OFPTYPE_SET_FLOW_FORMAT:
case OFPTYPE_FLOW_MOD_TABLE_ID:
case OFPTYPE_SET_PACKET_IN_FORMAT:
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 1358b21..52a74d1 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -247,6 +247,9 @@ enum ofpraw {
/* OFPT 1.4+ (30): struct ofp14_role_status, uint8_t[8][]. */
OFPRAW_OFPT14_ROLE_STATUS,
+/* OFPT 1.4+ (32): struct ofp14_requestforward, uint8_t[8][]. */
+OFPRAW_OFPT14_REQUESTFORWARD,
+
/* OFPT 1.4+ (33): struct ofp14_bundle_ctrl_msg, uint8_t[8][]. */
OFPRAW_OFPT14_BUNDLE_CONTROL,
@@ -559,6 +562,9 @@ enum ofptype {
/* Controller role change event messages. */
OFPTYPE_ROLE_STATUS, /* OFPRAW_OFPT14_ROLE_STATUS. */
+/* Request forwarding by the switch. */
+OFPTYPE_REQUESTFORWARD, /* OFPRAW_OFPT14_REQUESTFORWARD. */
+
OFPTYPE_BUNDLE_CONTROL, /* OFPRAW_OFPT14_BUNDLE_CONTROL. */
OFPTYPE_BUNDLE_ADD_MESSAGE, /* OFPRAW_OFPT14_BUNDLE_ADD_MESSAGE. */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 6db32d1..f6e665d 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2960,6 +2960,38 @@ ofp_print_geneve_table_reply(struct ds *s, const struct
ofp_header *oh)
ofputil_uninit_geneve_table(>r.mappings);
}
+/* This function will print the request forward message. The reason for
+ * request forward is taken from rf.request.type */
+static void
+ofp_print_requestforward(struct ds *string, const struct ofp_header *oh)
+{
+struct ofputil_requestforward rf;
+enum ofperr error;
+enum ofptype type;
+
+error = ofputil_decode_requestforward(oh, &rf);
+if (error) {
+ofp_print_error(string, error);
+return;
+}
+
+error = ofptype_decode(&type, rf.request);
+if (error) {
+ofp_print_error(string, error);
+return;
+}
+
+ds_put_cstr(string, " reason=");
+
+if (type == OFPTYPE_GROUP_MOD) {
+ds_put_cstr(string, "group_mod");
+ofp_print_group_mod(string, rf.request);
+} else if (type == OFPTYPE_METER_MOD) {
+ds_put_cstr(string, "meter_mod");
+ofp_print_meter_mod(string, rf.request);
+}
+}
+
static void
ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw,
struct ds *string, int verbosity)
@@ -3089,6 +3121,10 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw
raw,
ofp_print_role_status_message(string, oh);
break;
+case OFPTYPE_REQUESTFORWARD:
+ofp_print_requestforward(string, oh);
+break;
+
case OFPTYPE_METER_STATS_REQUEST:
case OFPTYPE_METER_CONFIG_STATS_REQUEST:
ofp_print_stats(string, oh);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index
[ovs-dev] [Patch v1 0/3]Implementation of OpenFlow 1.4 Vacancy Evevnts
Hi Ben, This series of patch contains Implementation of OF1.4 [EXT-192] Vacancy Events. Following are the patches: Implement Openflow 1.4 Vacancy Events for OFPT_TABLE_MOD. Implement Vacancy Events for OFPMP_TABLE_DESC. Implement OFPT_TABLE_STATUS Message. lib/ofp-parse.c | 65 ++-- lib/ofp-parse.h | 3 + lib/ofp-print.c | 22 ++ lib/ofp-util.c | 87 ++ lib/ofp-util.h | 39 + ofproto/ofproto-provider.h | 11 + ofproto/ofproto.c | 24 +++--- tests/ofp-print.at | 2 - utilities/ovs-ofctl.c | 90 ++- lib/ofp-print.c | 11 + lib/ofp-util.c | 31 + ofproto/ofproto.c | 26 +-- tests/ofproto.at | 10 include/openflow/openflow-1.4.h | 8 +++ lib/learning-switch.c | 1 lib/ofp-msgs.h | 6 +++ lib/ofp-print.c | 26 +++ lib/ofp-util.c | 82 lib/ofp-util.h | 12 + lib/rconn.c | 1 ofproto/connmgr.c | 31 + ofproto/connmgr.h | 3 + ofproto/ofproto.c | 31 + ovn/controller/ofctrl.c | 1 tests/ofproto.at | 20 +++-- 25 files changed, 610 insertions(+), 33 deletions(-) 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] [PATCH v1 1/3] Implement Openflow 1.4 Vacancy Events for OFPT_TABLE_MOD.
From: Saloni Jain
OpenFlow 1.4 introduces the ability to turn on vacancy events with an
OFPT_TABLE_MOD message specifying OFPTC_VACANCY_EVENTS. This commit adds
support for the new feature in ovs-ofctl mod-table.
As per the openflow specification-1.4, vacancy event adds a mechanism
enabling the controller to get an early warning based on capacity
threshold chosen by the controller.
With this commit, vacancy events can be configured as:
ovs-ofctl -O OpenFlow14 mod-table vacancy-
The syntax of as .
specify vacancy threshold values, vacancy down and vacancy up.
To disable vacancy events, following command should be given:
ovs-ofctl -O OpenFlow14 mod-table novacancy
Signed-off-by: Saloni Jain
---
lib/ofp-parse.c| 65 +---
lib/ofp-parse.h|3 ++
lib/ofp-print.c| 22 +++
lib/ofp-util.c | 87 ++
lib/ofp-util.h | 39 +++
ofproto/ofproto-provider.h | 11 ++
ofproto/ofproto.c | 24
tests/ofp-print.at |2 +-
utilities/ovs-ofctl.c | 90 +++-
9 files changed, 319 insertions(+), 24 deletions(-)
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 8cdda50..9239bb2 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -869,6 +869,49 @@ parse_ofp_flow_mod_str(struct ofputil_flow_mod *fm, const
char *string,
return error;
}
+/* Convert 'setting' (as described for the "mod-table" command
+ * in ovs-ofctl man page) into 'tm->table_vacancy->vacancy_up' and
+ * 'tm->table_vacancy->vacancy_down' threshold values.
+ * For the two threshold values, value of vacancy_up is always greater
+ * than value of vacancy_down.
+ *
+ * Returns NULL if successful, otherwise a malloc()'d string describing the
+ * error. The caller is responsible for freeing the returned string. */
+char * OVS_WARN_UNUSED_RESULT
+parse_ofp_table_vacancy(struct ofputil_table_mod *tm, const char *setting)
+{
+char *save_ptr = NULL;
+char *vac_up, *vac_down;
+char *value = strdup(setting);
+int vacancy_up, vacancy_down;
+
+strtok_r(value, "-", &save_ptr);
+vac_down = strtok_r(NULL, "..", &save_ptr);
+if (!vac_down) {
+return xasprintf("Vacancy down value missing");
+}
+if (!str_to_int(vac_down, 0, &vacancy_down) ||
+vacancy_down < 0 || vacancy_down > 100) {
+return xasprintf("Invalid vacancy down value \"%s\"", vac_down);
+}
+vac_up = strtok_r(NULL, "..", &save_ptr);
+if (!vac_up) {
+return xasprintf("Vacancy up value missing");
+}
+if (!str_to_int(vac_up, 0, &vacancy_up) ||
+vacancy_up < 0 || vacancy_up > 100) {
+return xasprintf("Invalid vacancy up value \"%s\"", vac_up);
+}
+if (vacancy_down > vacancy_up) {
+return xasprintf("Invalid vacancy range, vacancy up should be greater"
+ " than vacancy down ""(%s)",
+ ofperr_to_string(OFPERR_OFPBPC_BAD_VALUE));
+}
+tm->table_vacancy.vacancy_down = vacancy_down;
+tm->table_vacancy.vacancy_up = vacancy_up;
+return NULL;
+}
+
/* Convert 'table_id' and 'setting' (as described for the "mod-table" command
* in the ovs-ofctl man page) into 'tm' for sending a table_mod command to a
* switch.
@@ -895,13 +938,13 @@ parse_ofp_table_mod(struct ofputil_table_mod *tm, const
char *table_id,
tm->miss = OFPUTIL_TABLE_MISS_DEFAULT;
tm->eviction = OFPUTIL_TABLE_EVICTION_DEFAULT;
tm->eviction_flags = UINT32_MAX;
-
+tm->vacancy = OFPUTIL_TABLE_VACANCY_DEFAULT;
+tm->table_vacancy.vacancy_down = 0;
+tm->table_vacancy.vacancy_up = 0;
+tm->table_vacancy.vacancy = 0;
/* Only OpenFlow 1.1 and 1.2 can configure table-miss via table_mod.
- * Only OpenFlow 1.4+ can configure eviction via table_mod.
- *
- * (OpenFlow 1.4+ can also configure vacancy events via table_mod, but OVS
- * doesn't support those yet and they're also logically a per-OpenFlow
- * session setting so it wouldn't make sense to support them here anyway.)
+ * Only OpenFlow 1.4+ can configure eviction and vacancy events
+ * via table_mod.
*/
if (!strcmp(setting, "controller")) {
tm->miss = OFPUTIL_TABLE_MISS_CONTROLLER;
@@ -918,6 +961,16 @@ parse_ofp_table_mod(struct ofputil_table_mod *tm, const
char *table_id,
} else if (!strcmp(setting, "noevict")) {
tm->eviction = OFPUTIL_TABLE_EVICTION_OFF;
*usable_versions = (1 << OFP14_VERSION) | (1u << OFP15_VERSION);
+} else if (!strncmp(setting, "vacancy", strcspn(setting, "-"))) {
+tm->vacancy = OFPUTIL_TABLE_VACANCY_ON;
+*usable_versions = (1 << OFP14_VERSION) | (1u << OFP15_VERSION);
+char *error = parse_ofp_table_vacancy(tm, setting);
+if (error) {
+return error;
+}
+} else if (!strcmp(setting, "novacancy")) {
+
[ovs-dev] [PATCH v1 2/3] Implement Vacancy Events for OFPMP_TABLE_DESC.
From: Saloni Jain
This patch adds support for vacancy events in table-desc.
ovs-ofctl -O OpenFlow14 dump-tables-desc
-This command is enhanced to display the Vacancy Event configuration
of the tables on a , which is set using the mod-table command.
Signed-off-by: Saloni Jain
---
lib/ofp-print.c | 11 +++
lib/ofp-util.c| 31 +++
ofproto/ofproto.c | 26 +-
tests/ofproto.at | 10 +-
4 files changed, 72 insertions(+), 6 deletions(-)
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index cf35aa6..0c3b8a9 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1057,6 +1057,17 @@ ofp_print_table_desc(struct ds *string, const struct
ofputil_table_desc *td)
ofputil_table_eviction_to_string(td->eviction));
ofputil_put_eviction_flags(string, td->eviction_flags);
ds_put_char(string, '\n');
+ds_put_format(string, " vacancy=%s",
+ ofputil_table_vacancy_to_string(td->vacancy));
+if (td->vacancy == OFPUTIL_TABLE_VACANCY_ON) {
+ds_put_format(string, " vacancy_down=%"PRIu8"%%",
+ td->table_vacancy.vacancy_down);
+ds_put_format(string, " vacancy_up=%"PRIu8"%%",
+ td->table_vacancy.vacancy_up);
+ds_put_format(string, " vacancy=%"PRIu8"%%",
+ td->table_vacancy.vacancy);
+}
+ds_put_char(string, '\n');
}
static void
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index e2e90ff..6446d33 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -4903,6 +4903,22 @@ parse_table_desc_eviction_property(struct ofpbuf
*property,
return 0;
}
+static enum ofperr
+parse_table_desc_vacancy_property(struct ofpbuf *property,
+ struct ofputil_table_desc *td)
+{
+struct ofp14_table_mod_prop_vacancy *otv = property->data;
+
+if (property->size != sizeof *otv) {
+return OFPERR_OFPBPC_BAD_LEN;
+}
+
+td->table_vacancy.vacancy_down = otv->vacancy_down;
+td->table_vacancy.vacancy_up = otv->vacancy_up;
+td->table_vacancy.vacancy = otv->vacancy;
+return 0;
+}
+
/* Decodes the next OpenFlow "table desc" message (of possibly several) from
* 'msg' into an abstract form in '*td'. Returns 0 if successful, EOF if the
* last "table desc" in 'msg' was already decoded, otherwise an OFPERR_*
@@ -4944,6 +4960,7 @@ ofputil_decode_table_desc(struct ofpbuf *msg,
ofpbuf_use_const(&properties, ofpbuf_pull(msg, length), length);
td->eviction = ofputil_decode_table_eviction(otd->config, version);
+td->vacancy = ofputil_decode_table_vacancy(otd->config, version);
td->eviction_flags = UINT32_MAX;
while (properties.size > 0) {
@@ -4961,6 +4978,10 @@ ofputil_decode_table_desc(struct ofpbuf *msg,
error = parse_table_desc_eviction_property(&payload, td);
break;
+case OFPTMPT14_VACANCY:
+error = parse_table_desc_vacancy_property(&payload, td);
+break;
+
default:
log_property(true, "unknown table_desc property %"PRIu16, type);
error = 0;
@@ -5013,6 +5034,16 @@ ofputil_append_table_desc_reply(const struct
ofputil_table_desc *td,
ote->length = htons(sizeof *ote);
ote->flags = htonl(td->eviction_flags);
}
+if (td->vacancy == OFPUTIL_TABLE_VACANCY_ON) {
+struct ofp14_table_mod_prop_vacancy *otv;
+
+otv = ofpbuf_put_zeros(reply, sizeof *otv);
+otv->type = htons(OFPTMPT14_VACANCY);
+otv->length = htons(sizeof *otv);
+otv->vacancy_down = td->table_vacancy.vacancy_down;
+otv->vacancy_up = td->table_vacancy.vacancy_up;
+otv->vacancy = td->table_vacancy.vacancy;
+}
otd = ofpbuf_at_assert(reply, start_otd, sizeof *otd);
otd->length = htons(reply->size - start_otd);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 176ff10..86e21e2 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3576,6 +3576,26 @@ handle_table_features_request(struct ofconn *ofconn,
return 0;
}
+static void
+query_table_desc__(struct ofputil_table_desc *td,
+ struct ofproto *ofproto, uint8_t table_id)
+{
+unsigned int count = ofproto->tables[table_id].n_flows;
+unsigned int max_flows = ofproto->tables[table_id].max_flows;
+
+td->table_id = table_id;
+td->eviction = (ofproto->tables[table_id].eviction & EVICTION_OPENFLOW
+? OFPUTIL_TABLE_EVICTION_ON
+: OFPUTIL_TABLE_EVICTION_OFF);
+td->eviction_flags = OFPROTO_EVICTION_FLAGS;
+td->vacancy = (ofproto->tables[table_id].vacancy & OFPTC14_VACANCY_EVENTS
+ ? OFPUTIL_TABLE_VACANCY_ON
+ : OFPUTIL_TABLE_VACANCY_OFF);
+td->table_vacancy.vacancy_down = ofproto->tables[table_id].vacancy_down;
+td->table_vacancy.vacancy_up = ofproto->tables[table_id].vacancy_up;
+td->table_vacancy.vacancy = (count *
[ovs-dev] [PATCH v1 3/3] Implement OFPT_TABLE_STATUS Message.
From: Saloni Jain
On change in a table state, the controller needs to be informed with
the OFPT_TABLE_STATUS message. The message is sent with reason
OFPTR_VACANCY_DOWN or OFPTR_VACANCY_UP in case of change in remaining
space eventually crossing any one of the threshold.
Signed-off-by: Saloni Jain
---
include/openflow/openflow-1.4.h |8
lib/learning-switch.c |1 +
lib/ofp-msgs.h |6 +++
lib/ofp-print.c | 26 +
lib/ofp-util.c | 82 +++
lib/ofp-util.h | 12 ++
lib/rconn.c |1 +
ofproto/connmgr.c | 31 +++
ofproto/connmgr.h |3 ++
ofproto/ofproto.c | 31 +++
ovn/controller/ofctrl.c |1 +
tests/ofproto.at| 20 --
12 files changed, 219 insertions(+), 3 deletions(-)
diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
index 37eef4a..1c996f7 100644
--- a/include/openflow/openflow-1.4.h
+++ b/include/openflow/openflow-1.4.h
@@ -172,6 +172,14 @@ struct ofp14_table_desc {
};
OFP_ASSERT(sizeof(struct ofp14_table_desc) == 8);
+/* A table config has changed in the datapath */
+struct ofp14_table_status {
+uint8_t reason;/* One of OFPTR_*. */
+uint8_t pad[7];/* Pad to 64 bits */
+/* Followed by struct ofp14_table_desc */
+};
+OFP_ASSERT(sizeof(struct ofp14_table_status) == 8);
+
/* ## ## */
/* ## ofp14_port_stats ## */
/* ## ## */
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 1753cea..a78c3f9 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -419,6 +419,7 @@ lswitch_process_packet(struct lswitch *sw, const struct
ofpbuf *msg)
case OFPTYPE_ROLE_REQUEST:
case OFPTYPE_ROLE_REPLY:
case OFPTYPE_ROLE_STATUS:
+case OFPTYPE_TABLE_STATUS:
case OFPTYPE_SET_FLOW_FORMAT:
case OFPTYPE_FLOW_MOD_TABLE_ID:
case OFPTYPE_SET_PACKET_IN_FORMAT:
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 1358b21..ba72a62 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -247,6 +247,9 @@ enum ofpraw {
/* OFPT 1.4+ (30): struct ofp14_role_status, uint8_t[8][]. */
OFPRAW_OFPT14_ROLE_STATUS,
+/* OFPT 1.4+ (31): struct ofp14_table_status, uint8_t[8][]. */
+OFPRAW_OFPT14_TABLE_STATUS,
+
/* OFPT 1.4+ (33): struct ofp14_bundle_ctrl_msg, uint8_t[8][]. */
OFPRAW_OFPT14_BUNDLE_CONTROL,
@@ -559,6 +562,9 @@ enum ofptype {
/* Controller role change event messages. */
OFPTYPE_ROLE_STATUS, /* OFPRAW_OFPT14_ROLE_STATUS. */
+/* Asynchronous messages. */
+OFPTYPE_TABLE_STATUS, /* OFPRAW_OFPT14_TABLE_STATUS. */
+
OFPTYPE_BUNDLE_CONTROL, /* OFPRAW_OFPT14_BUNDLE_CONTROL. */
OFPTYPE_BUNDLE_ADD_MESSAGE, /* OFPRAW_OFPT14_BUNDLE_ADD_MESSAGE. */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 0c3b8a9..70bd8af 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1071,6 +1071,28 @@ ofp_print_table_desc(struct ds *string, const struct
ofputil_table_desc *td)
}
static void
+ofp_print_table_status_message(struct ds *string, const struct ofp_header *oh)
+{
+struct ofputil_table_status ts;
+enum ofperr error;
+
+error = ofputil_decode_table_status(oh, &ts);
+if (error) {
+ofp_print_error(string, error);
+return;
+}
+
+if (ts.reason == OFPTR_VACANCY_DOWN) {
+ds_put_format(string, " reason=VACANCY_DOWN");
+} else if (ts.reason == OFPTR_VACANCY_UP) {
+ds_put_format(string, " reason=VACANCY_UP");
+}
+
+ds_put_format(string, "\ntable_desc:-");
+ofp_print_table_desc(string, &ts.desc);
+}
+
+static void
ofp_print_queue_get_config_request(struct ds *string,
const struct ofp_header *oh)
{
@@ -3122,6 +3144,10 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw
raw,
ofp_print_role_status_message(string, oh);
break;
+case OFPTYPE_TABLE_STATUS:
+ofp_print_table_status_message(string, oh);
+break;
+
case OFPTYPE_METER_STATS_REQUEST:
case OFPTYPE_METER_CONFIG_STATS_REQUEST:
ofp_print_stats(string, oh);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 6446d33..9e6e438 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -9169,6 +9169,7 @@ ofputil_is_bundlable(enum ofptype type)
case OFPTYPE_TABLE_FEATURES_STATS_REPLY:
case OFPTYPE_TABLE_DESC_REPLY:
case OFPTYPE_ROLE_STATUS:
+case OFPTYPE_TABLE_STATUS:
case OFPTYPE_NXT_GENEVE_TABLE_REQUEST:
case OFPTYPE_NXT_GENEVE_TABLE_REPLY:
break;
@@ -9598,3 +9599,84 @@ ofputil_encode_get_async_config(const struct ofp_header
*oh,
return buf;
}
+
+static void
+ofputil_put_ofp14_table_desc(const struct ofputil_table_desc *td,
+ struct ofpbuf *b, enum ofp_version version)
Re: [ovs-dev] [PATCH 3/3] bridge: reconfigure when system interfaces change
On Mon, Jul 27, 2015 at 03:59:18PM -0700, Ben Pfaff wrote: > On Mon, Jul 27, 2015 at 03:55:52PM -0700, Gurucharan Shetty wrote: > > I wonder what happens for Windows here. > > > I guess we need an if-notifier-windows.c also. > Sorry for that, I forgot to include a stub for the other cases. That would include ESX, right? I am just following how the route-table works. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/3] rtbsd: support RTM_IFANNOUNCE messages
On Mon, Jul 27, 2015 at 03:46:28PM -0700, Ben Pfaff wrote: > On Mon, Jul 27, 2015 at 02:24:19PM -0300, Thadeu Lima de Souza Cascardo wrote: > > When devices are created, they are announced using RTM_IFANNOUNCE messages > > using > > PF_ROUTE. > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > Do you know anyone who might be qualified to properly review this? It > seems reasonable but I don't know this part of BSD at all. Maybe one of the guys who contributed some of the specific BSD code: Ed Maste contributed lib/route-table-bsd.c, and Giuseppe Lettieri contributed lib/rtbsd.c. I am cc'ing them on this message, and bouncing the patch to them as well. What about the style? Is it OK to read using struct if_msghdr, then cast to struct if_announcemsghdr? From my tests, this works fine, the RAW socket seems to work like a DGRAM socket in this particular case. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] kernel module testing
On 7/28/15 3:19 AM, Finucane, Stephen wrote: For what it's worth, I also think that something like Gerrit would be useful given the number of platforms that OVS is running on. Right now, it's seems like we're doing the human-powered version, which is Guru, Daniele, or Ben complain when something breaks Windows, DPDK, 32-bit respectively. It also effectively provides the features of Patchwork in a way that is more maintainable. I agree that the Gerrit UI sucks (I haven't tried the OpenStack interface) and maybe there are alternatives, like Github's set of tools. But I think the status quo that we have isn't all that great either and I also would like to avoid having a collection of independent tools that fall apart over time. I'm happy to encourage people to submit changes via Github, as an experiment. I think this is a reasonable place to start, especially since we already have the infrastructure set up. I think the main thing to try out is whether we are OK with this type of workflow for reviewing patches as opposed to the mailing list, so at some level it doesn't matter where we start. I don't think that Travis CI is good enough to handle all of the pre-checkin tests that we would like to do (kernel, Windows, performance, etc.) but it looks like there are other tools that have integration with Github that minimally should be the same as what Gerrit can do. I'm obliged to point out the work that's ongoing to get some level of automated testing going in patchwork (for use with DPDK): https://lists.ozlabs.org/pipermail/patchwork/2015-July/001363.html Stephen, thanks for bringing this up. However, I still think Gerrit is the more mature, feature-filled solution if mailing list-based development isn't an absolute necessity. OpenStack really have the Gerrit process nailed. My question would be how to handle multi-architecture support required for testing kernel module. It seems like a mechanism that we are working on for DPDK would work well because it is decentralized. The testing for each arch and target environment, Linux revision etc. is "farmed out" to jenkins or some other CI tool that only have to report the result via email which is in turn is summarized by patchwork. Someone really needs to fix that web UI though... Stephen PS: Third party CI support (a lá OpenStack) would be an incredibly useful feature to offload some level of testing (like performance testing of the DPDK element of OVS). It's probably not an option with GerritHub (I haven't checked), but if you're deciding to go all in on Gerrit or not then this is one for the PROS column. ___ 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] bridge: reconfigure when system interfaces change
> > Sorry for that, I forgot to include a stub for the other cases. That would > include ESX, right? I am just following how the route-table works. You can ignore ESX. But, yeah, we will need something for Windows such that we atleast don't have compilation and run time issues on Windows. A stub should be okay here. > > Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Does OVS support TTP ?
No. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] kernel module testing
> On 7/28/15 3:19 AM, Finucane, Stephen wrote: > For what it's worth, I also think that something like Gerrit would be > useful given the number of platforms that OVS is running on. Right > now, it's seems like we're doing the human-powered version, which is > Guru, Daniele, or Ben complain when something breaks Windows, DPDK, > 32-bit respectively. It also effectively provides the features of > Patchwork in a way that is more maintainable. > > I agree that the Gerrit UI sucks (I haven't tried the OpenStack > interface) and maybe there are alternatives, like Github's set of > tools. But I think the status quo that we have isn't all that great > either and I also would like to avoid having a collection of > independent tools that fall apart over time. > >>> > >>> I'm happy to encourage people to submit changes via Github, as an > >>> experiment. > >> > >> I think this is a reasonable place to start, especially since we > >> already have the infrastructure set up. I think the main thing to try > >> out is whether we are OK with this type of workflow for reviewing > >> patches as opposed to the mailing list, so at some level it doesn't > >> matter where we start. > >> > >> I don't think that Travis CI is good enough to handle all of the > >> pre-checkin tests that we would like to do (kernel, Windows, > >> performance, etc.) but it looks like there are other tools that have > >> integration with Github that minimally should be the same as what > >> Gerrit can do. > > > > I'm obliged to point out the work that's ongoing to get some level of > automated testing going in patchwork (for use with DPDK): > > > > https://lists.ozlabs.org/pipermail/patchwork/2015-July/001363.html > Stephen, thanks for bringing this up. > > > > However, I still think Gerrit is the more mature, feature-filled solution > if mailing list-based development isn't an absolute necessity. OpenStack > really have the Gerrit process nailed. > > > My question would be how to handle multi-architecture support required > for testing kernel module. It seems like a mechanism that we are working > on for DPDK would work well because it is decentralized. The testing for > each arch and target environment, Linux revision etc. is "farmed out" to > jenkins or some other CI tool that only have to report the result via > email which is in turn is summarized by patchwork. See the 'PS:' below. Third party CIs, maintained by someone with said architecture, could be used for this. Have a look at the "Jenkins check" table for this review in OpenStack Gerrit: https://review.openstack.org/#/c/202655/ You'll see there's at least the third party CIs commenting on the change: * IBM PowerKVM CI * Intel PCI CI check * Microsoft Hyper-V CI check * Virtuozzo Storage CI check These CIs mostly check different hypervisor support, but architecture support for OVS would be virtually the same thing. My own team here in Intel, for example, have a CI job validating NUMA topology features of OpenStack Nova. We don't report these results yet, but we could! Stephen > > Someone really needs to fix that web UI though... > > > > Stephen > > > > PS: Third party CI support (a lá OpenStack) would be an incredibly useful > feature to offload some level of testing (like performance testing of the > DPDK element of OVS). It's probably not an option with GerritHub (I haven't > checked), but if you're deciding to go all in on Gerrit or not then this is > one for the PROS column. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH V2 1/4] rtnetlink: Extend rtnetlink to support RTNLGRP_IPV4_IFADDR and RTNLGRP_IPV6_IFADDR.
On Mon, Jul 27, 2015 at 11:56:27PM -0700, Alex Wang wrote:
> This commit renames the rtnetlink-link.{c,h} to rtnetlink.{c,h}
> and extends the module to support RTNLGRP_IPV4_IFADDR and
> RTNLGRP_IPV4_IFADDR multicast groups. A later patch will start
> using this module to react to interface address changes.
>
> Signed-off-by: Alex Wang
>
> ---
> PATCH->V2:
> - new patch, since parsing RTNLGRP_IPV4_IFADDR/RTNLGRP_IPV6_IFADDR
> is different from parsing RTNLGRP_LINK, we need to extend to
> module.
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH V2 2/4] netdev-linux: Make netdev_linux_notify_sock join RTNLGRP_IPV4_IFADDR and RTNLGRP_IPV6_IFADDR.
On Mon, Jul 27, 2015 at 11:56:28PM -0700, Alex Wang wrote:
> Currently the netdev_linux_notify_sock only joins multicast group
> RTNLGRP_LINK for link status change notification. Some ovs features
> also require the detection of ip addresses changes and update of the
> netdev-linux's cache. To achieve this, we need to make
> netdev_linux_notify_sock join the multicast group RTNLGRP_IPV4_IFADDR
> and RTNLGRP_IPV6_IFADDR.
>
> Signed-off-by: Alex Wang
> ---
> PATCH->V2:
> - rebase on top of rtnetlink module extension.
...
> +} else if (rtnetlink_type_is_rtnlgrp_addr(change->nlmsg_type)) {
> +/* Invalidates in4, in6. */
> +netdev_linux_changed(dev, dev->ifi_flags,
> + ~VALID_IN4 ^ VALID_IN6);
This would be more conventionally written ~(VALID_IN4 | VALID_IN6).
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] datapath-windows: Avoid BSOD when cleaning up a tunnel vport
On Tue, Jul 28, 2015 at 10:04:07AM +, Sorin Vinturis wrote: > If an error appears when creating a tunnel vport the cleanup is performed > twice, which causes the tunnel vport to be released also twice and > generate a BSOD. > > This patch modifies the tunnel filter cleanup logic to avoid this issue. > > Signed-off-by: Sorin Vinturis > Reported-by: Sorin Vinturis > Reported-at: https://github.com/openvswitch/ovs-issues/issues/97 > Acked-by: Nithin Raju > --- > v2: added acked. Applied to master and branch-2.4. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Inter-op openvswitch and openfcoe
On 27/07/15 22:53, Jesse Gross wrote: On Mon, Jul 27, 2015 at 9:59 AM, Anoob Soman wrote: Hi All, I am aware there had been some discussion on running lldpad and open-fcoe utils (like fcoemon) on top of ovs, but couldn’t figure out a way to successfully run lldpad and fcoemon on top of openvswitch. I did some experimentation with openvswitch and open-fcoe and now I am trying to get opinion from you guys about what would be the best approach to make fcoeuilts run on top of openvswitch. I used openvswitch v2.3.2 for my testing. My experimentation was carried out using XenServer and in my experimentation I could see FIP and LLDP don’t reach lldpad and fcoemon daemons. Bit of background what these daemon are. You can skip this paragraph, if you know about these daemons. FCoE’s lossless ethernet can be achieved using dcb (dcbx protocol). dcb, in open-fcoe, is enabled using lldpad (LLDP agent daemon) which handles dbcx options and much more. dcb needs to be enabled and available on a interface for fcoemon to start fcoe operations. I have figured out why lldpad and fcoemon don’t receive ETH_P_LLDP and ETH_P_FIP packets. lldpad and fcoemon open ETH_P_LLDP and ETH_P_FIP raw sockets respectively. Since the processing of raw socket packets (ptype_base) happens after openvswitch (rx_handler), when running in XenServer any raw socket listening on ethX interface, will not receive any packets, as openvswitch always return RX_HANDLER_CONSUMED. rx_handlers are only set for ethX and xenbrX (bridge interface) doesn’t have any rx_handler set. Ideally, if raw socket is listening on xenbrX interface it should receive the packets. But in case of lldpad it is not possible to make this daemon receive packet on xenbrX interface. lldpad daemon opens PF_LLDP raw socket and it listens on all ports. If you consider XenServer, then it listens on xenbrX (bridge interface) and ethX interface. llpdad also queries for the dcb state, from the netdevice, using dcbnl_ops. As of now xenbrX (bridge interface) doesn’t have any dcbnl_ops, which mean lldpad will only be able to configure, listen, send, receive lldp pdu on ethX interfaces. Without dcb support on xenbrX interface, fcoemon will not start fcoe handling on xenbrX interface. This rules out having these daemons run on top of xenbrX (bridge interface). So I am looking for an interface, similar to linux bridge, where setting an action of drop doesn’t drop the packet instead it returns RX_HANDLER_PASS and ptype_base handler (including packet_handler) can pickup the packet. I did a proof of concept, by having a hack, in netdev_frame_hook() datapath/vport-netdev.c if (skb->protocol == cpu_to_be16(ETH_P_FIP) || skb->protocol == cpu_to_be16(0x88CC)) return RX_HANDLER_PASS; and this makes fcoemon and lldpad daemon work as expected. If returning RX_HANDLER_PASS is the right approach to go forward, how do we design this into ovs. Can we have an action called “ignore" which will ignore certain flows from being processed by openvswitch. Is there any other approach that I can take. I think there's roughly three possible approaches: #1: Replicate support for these protocols in OVS, which would be able to install the appropriate flows directly in the datapath to get these packets itself. This is certainly possible and cleaner as well. But I think its a huge amount to work to replicate the support in ovs. #2: Add some mechanism where these daemons can talk to OVS and have them cooperate to get the appropriate packets, while still keeping the logic in the original daemon. Does it mean that daemon would be like openflow controllers and can use packet_[in/out] to receive/send daemon specific traffic ? #3: The method that you are describing, where OVS has a mechanism to allow the existing daemons to work unmodified. There is already a little LLDP code in OVS to support certain functionality (auto-attach), so I guess that means that we have roughly started down path #1. However, in that case, I think that was new functionality that did not also exist in lldpad. I haven't really thought about how approach #3 would interact with existing lldp code. Let me think about this at bit more. I think the main issue with #3 is finding a way to make things work together cleanly. If OVS is managing the switch configuration, how does it interact with these external daemons? What happens if they are used together with the OVS LLDP code? Can this be handled automatically without making the user understand the details of the kernel implementation? Thanks, Anoob. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 2/2] ofproto: Implement OF1.4 Group & Meter change notification messages
On Tue, Jul 28, 2015 at 05:22:25PM +0530, niti1...@gmail.com wrote: > From: Niti Rohilla > > This patch adds support for Openflow1.4 Group & meter change notification > messages. In a multi controller environment, when a controller modifies the > state of group and meter table, the request that successfully modifies this > state is forwarded to other controllers. Other controllers are informed with > the OFPT_REQUESTFORWARD message. Request forwarding is enabled on a per > controller channel basis using the Set Asynchronous Configuration Message. > > Signed-off-by: Niti Rohilla Your function for "re-encoding" requests doesn't actually do it! It only changes the version number. That won't properly account for differences between OF1.4 and later versions of the protocol, for example in changes to numbering of OXM fields. The code needs to use ofputil_*() functions to encode the group_mod and meter_mod requests properly for the protocol in use. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 01/22] ovn-controller: Fix potential use-after-free in get_core_config().
On Sat, Jul 25, 2015 at 11:04:13PM -0700, Alex Wang wrote:
> On Sun, Jul 19, 2015 at 3:44 PM, Ben Pfaff wrote:
>
> > It's unsafe to hold a pointer to a row in the IDL across calls to
> > ovsdb_idl_run() for that IDL.
> >
> > Signed-off-by: Ben Pfaff
> > ---
> > ovn/controller/ovn-controller.c | 17 -
> > 1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/ovn/controller/ovn-controller.c
> > b/ovn/controller/ovn-controller.c
> > index fda1534..e571bb5 100644
> > --- a/ovn/controller/ovn-controller.c
> > +++ b/ovn/controller/ovn-controller.c
> > @@ -95,16 +95,15 @@ get_bridge(struct controller_ctx *ctx, const char
> > *name)
> > static void
> > get_core_config(struct controller_ctx *ctx)
> > {
> > -const struct ovsrec_open_vswitch *cfg;
> > -
> > -cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
> > -if (!cfg) {
> > -VLOG_ERR("No Open_vSwitch row defined.");
> > -ovsdb_idl_destroy(ctx->ovs_idl);
> > -exit(EXIT_FAILURE);
> > -}
> > -
> > while (1) {
> > +const struct ovsrec_open_vswitch *cfg;
> > +cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
> > +if (!cfg) {
> > +VLOG_ERR("No Open_vSwitch row defined.");
> > +ovsdb_idl_destroy(ctx->ovs_idl);
> > +exit(EXIT_FAILURE);
> > +}
> > +
> >
>
> Curious, why don't you call ovsdb_idl_run(ctx->ovs_idl) first in the while
> loop? seems to me, theoretically, cfg could still get changed after
> ovsdb_idl_run(ctx->ovs_idl).
You're right, that was a dumb "fix".
Here's an incremental that fixes that problem.
I'll fix for v2.
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index e571bb5..bd3ef0d 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -96,6 +96,8 @@ static void
get_core_config(struct controller_ctx *ctx)
{
while (1) {
+ovsdb_idl_run(ctx->ovs_idl);
+
const struct ovsrec_open_vswitch *cfg;
cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
if (!cfg) {
@@ -107,8 +109,6 @@ get_core_config(struct controller_ctx *ctx)
const struct ovsrec_bridge *br_int;
const char *remote, *system_id, *br_int_name;
-ovsdb_idl_run(ctx->ovs_idl);
-
br_int_name = smap_get(&cfg->external_ids, "ovn-bridge");
if (!br_int_name) {
br_int_name = DEFAULT_BRIDGE_NAME;
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 03/22] ovn-controller: Avoid blocking to commit OVSDB transactions.
On Wed, Jul 22, 2015 at 04:26:48PM -0400, Russell Bryant wrote: > On 07/19/2015 06:44 PM, Ben Pfaff wrote: > > Until now, ovn-controller has been full of loops that commit a transaction > > to the OVS or OVN Southbound database. These blocking loops delay other > > work within ovn-controller. They also make it unsafe to keep pointers to > > database records within a single ovn-controller main loop, since calls > > to ovsdb_idl_run() can cause IDL records to be destroyed. This commit > > drops all of the blocking calls, instead doing a single commit to the > > databases at the end of each main loop. > > > > Signed-off-by: Ben Pfaff > > This is great! This is something I've wanted to do for a while but > never got around to. Thanks! The changes look good. I also did some > basic testing of this with ovs-sandbox and did not encounter any errors. > > Alex, I think it would be great to follow this same pattern for > ovn-controller-vtep. The idl_loop struct and related functions could be > shared between the two, at least. > > Acked-by: Russell Bryant Thanks for the ack. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 03/22] ovn-controller: Avoid blocking to commit OVSDB transactions.
On Sat, Jul 25, 2015 at 11:31:48PM -0700, Alex Wang wrote: > Also, could we add 'commit' to function name like > idl_loop_commit_and_wait()? I just assumed that the idl_loop_wait() just > waits... but it actually commits as well, OK, I'll make that change for v2. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 22/22] ovn: Change strategy for tunnel keys.
On Wed, Jul 22, 2015 at 05:17:22PM +, Chandran, Sugesh wrote: > I am just trying out this new patches in my OVN setup. > This setup has two logical network span across two nodes/Chassis. > I found two issues with the new patch set as below > 1) The controller cannot start properly due to the outdated Southbound DB. Do > we have any provision in OVN to update the DB and its contents for schema > changes such this? > If not how does this impact the OVN upgrade/downgrade functionality in the > future?? Of course we'll add support for upgrading from one OVN version to another. It's not there yet. (I'll note that OVSDB has pretty good support for upgrading databases. We'll probably use that.) > 2) As a workaround, I started the OVN with empty DB and populated everything > from scratch, which works fine. But the OVS forwarding rules are not > populated properly in the datapath. > I noticed that the traffic between VMs in same node dropped because there is > no rule created to process it, at the same time the traffic between VMs in > different node(using geneve tunnel) flows perfectly. > Please note, The rules are programmed correctly when I tested the same two > node OVN setup without these patches before. I can believe that there are bugs right now. I'm working on writing end-to-end tests for OVN; I suspect that those will fix many bugs, and prevent many more new bugs from ever getting into OVN. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 01/21] ovn-controller: Fix potential use-after-free in get_core_config().
It's unsafe to hold a pointer to a row in the IDL across calls to
ovsdb_idl_run() for that IDL.
Signed-off-by: Ben Pfaff
Acked-by: Russell Bryant
---
v1->v2: Call ovsdb_idl_run() before instead of after accessing IDL.
---
ovn/controller/ovn-controller.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index fda1534..bd3ef0d 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -95,21 +95,20 @@ get_bridge(struct controller_ctx *ctx, const char *name)
static void
get_core_config(struct controller_ctx *ctx)
{
-const struct ovsrec_open_vswitch *cfg;
+while (1) {
+ovsdb_idl_run(ctx->ovs_idl);
-cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
-if (!cfg) {
-VLOG_ERR("No Open_vSwitch row defined.");
-ovsdb_idl_destroy(ctx->ovs_idl);
-exit(EXIT_FAILURE);
-}
+const struct ovsrec_open_vswitch *cfg;
+cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
+if (!cfg) {
+VLOG_ERR("No Open_vSwitch row defined.");
+ovsdb_idl_destroy(ctx->ovs_idl);
+exit(EXIT_FAILURE);
+}
-while (1) {
const struct ovsrec_bridge *br_int;
const char *remote, *system_id, *br_int_name;
-ovsdb_idl_run(ctx->ovs_idl);
-
br_int_name = smap_get(&cfg->external_ids, "ovn-bridge");
if (!br_int_name) {
br_int_name = DEFAULT_BRIDGE_NAME;
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 00/21] Change strategy for tunnel keys
This is mostly a rebase versus v1. A few patches have been acked,
and patch 1 includes a bug fix.
Ben Pfaff (21):
ovn-controller: Fix potential use-after-free in get_core_config().
ovn-controller: Drop unnecessary checks for ovsdb_idl_is_alive().
ovn-controller: Avoid blocking to commit OVSDB transactions.
ovn-controller: Explicitly pass the flow table from function to
function.
ovn-controller: Pass 'br_int' explicitly to functions that need it.
ovn-controller: Factor encapsulation code out of chassis code.
ovn-controller: Pass 'chassis_id' explicitly to functions that need
it.
ovn-controller: Tolerate missing integration bridge.
ovn-controller: Tolerate missing 'chassis_id'.
ovn-controller: Rename init functions that just register IDL columns.
ovn-controller: Honor external-ids:ovn-bridge changing.
ovn-controller: Slightly adjust pipeline init and destroy for
consistency.
ovn-controller: Use controller_ctx just to pass around data.
smap: New function smap_get_uuid().
nroff: Add support for 'diagram' XML element for protocol headers.
ovn: Rename Binding table to Port_Binding.
ovn: Rename Pipeline table to Rule table.
actions: Allow caller to specify output table.
rule: Introduce MFF_LOG_DATAPATH macro for consistency.
ofctrl: Negotiate OVN Geneve option.
ovn: Change strategy for tunnel keys.
lib/smap.c | 13 +-
lib/smap.h |4 +-
ovn/TODO | 21 +-
ovn/controller/automake.mk |8 +-
ovn/controller/binding.c | 123 ++--
ovn/controller/binding.h | 11 +-
ovn/controller/chassis.c | 382 +-
ovn/controller/chassis.h | 10 +-
ovn/controller/{chassis.c => encaps.c} | 211 ++
ovn/controller/{binding.h => encaps.h} | 17 +-
ovn/controller/ofctrl.c| 516 +-
ovn/controller/ofctrl.h|9 +-
ovn/controller/ovn-controller.c| 299
ovn/controller/ovn-controller.h|9 +-
ovn/controller/physical.c | 400 ---
ovn/controller/physical.h | 23 +-
ovn/controller/{pipeline.c => rule.c} | 129 ++--
ovn/controller/{pipeline.h => rule.h} | 34 +-
ovn/lib/actions.c | 20 +-
ovn/lib/actions.h |6 +-
ovn/northd/ovn-northd.c| 1186
ovn/ovn-architecture.7.xml | 369 --
ovn/ovn-nb.xml | 17 +-
ovn/ovn-sb.ovsschema | 46 +-
ovn/ovn-sb.xml | 334 +++--
python/build/nroff.py | 98 +++
tests/test-ovn.c |2 +-
27 files changed, 2618 insertions(+), 1679 deletions(-)
copy ovn/controller/{chassis.c => encaps.c} (58%)
copy ovn/controller/{binding.h => encaps.h} (60%)
rename ovn/controller/{pipeline.c => rule.c} (77%)
rename ovn/controller/{pipeline.h => rule.h} (53%)
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 04/21] ovn-controller: Explicitly pass the flow table from function to function.
As I was working in ovn-controller, I found it hard to tell what code
produced and what code consumed the OpenFlow flow table, because it was
all implicit. This commit makes the data structure an explicit variable
in the main loop, which makes it easier to see.
Signed-off-by: Ben Pfaff
---
ovn/controller/ofctrl.c | 73 -
ovn/controller/ofctrl.h | 5 +--
ovn/controller/ovn-controller.c | 12 ---
ovn/controller/physical.c | 10 +++---
ovn/controller/physical.h | 3 +-
ovn/controller/pipeline.c | 12 +++
ovn/controller/pipeline.h | 3 +-
7 files changed, 60 insertions(+), 58 deletions(-)
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 843e1a1..e3192c1 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -35,7 +35,7 @@ VLOG_DEFINE_THIS_MODULE(ofctrl);
/* An OpenFlow flow. */
struct ovn_flow {
/* Key. */
-struct hmap_node hmap_node; /* In 'desired_flows' or 'installed_flows'. */
+struct hmap_node hmap_node;
uint8_t table_id;
uint16_t priority;
struct match match;
@@ -64,17 +64,14 @@ static unsigned int seqno;
* zero, to avoid unbounded buffering. */
static struct rconn_packet_counter *tx_counter;
-/* Flow tables. Each holds "struct ovn_flow"s.
- *
- * 'desired_flows' is the flow table that we want the switch to have.
- * 'installed_flows' is the flow table currently installed in the switch. */
-static struct hmap desired_flows;
+/* Flow table of "struct ovn_flow"s, that holds the flow table currently
+ * installed in the switch. */
static struct hmap installed_flows;
static void ovn_flow_table_clear(struct hmap *flow_table);
static void ovn_flow_table_destroy(struct hmap *flow_table);
-static void ofctrl_update_flows(void);
+static void ofctrl_update_flows(struct hmap *desired_flows);
static void ofctrl_recv(const struct ofpbuf *msg);
void
@@ -82,19 +79,22 @@ ofctrl_init(void)
{
swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP13_VERSION);
tx_counter = rconn_packet_counter_create();
-hmap_init(&desired_flows);
hmap_init(&installed_flows);
}
-/* This function should be called in the main loop after anything that updates
- * the flow table (e.g. after calls to ofctrl_clear_flows() and
- * ofctrl_add_flow()). */
+/* Attempts to update the OpenFlow flows in bridge 'br_int_name' to those in
+ * 'flow_table'. Removes all of the flows from 'flow_table' and frees them.
+ *
+ * The flow table will only be updated if we've got an OpenFlow connection to
+ * 'br_int_name' and it's not backlogged. Otherwise, it'll have to wait until
+ * the next iteration. */
void
-ofctrl_run(struct controller_ctx *ctx)
+ofctrl_run(struct controller_ctx *ctx, struct hmap *flow_table)
{
char *target;
target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), ctx->br_int_name);
if (strcmp(target, rconn_get_target(swconn))) {
+VLOG_INFO("%s: connecting to switch", target);
rconn_connect(swconn, target, target);
}
free(target);
@@ -102,10 +102,10 @@ ofctrl_run(struct controller_ctx *ctx)
rconn_run(swconn);
if (!rconn_is_connected(swconn)) {
-return;
+goto exit;
}
if (!rconn_packet_counter_n_packets(tx_counter)) {
-ofctrl_update_flows();
+ofctrl_update_flows(flow_table);
}
for (int i = 0; i < 50; i++) {
@@ -117,6 +117,9 @@ ofctrl_run(struct controller_ctx *ctx)
ofctrl_recv(msg);
ofpbuf_delete(msg);
}
+
+exit:
+ovn_flow_table_clear(flow_table);
}
void
@@ -131,7 +134,6 @@ ofctrl_destroy(void)
{
rconn_destroy(swconn);
ovn_flow_table_destroy(&installed_flows);
-ovn_flow_table_destroy(&desired_flows);
rconn_packet_counter_destroy(tx_counter);
}
@@ -246,22 +248,17 @@ ofctrl_recv(const struct ofpbuf *msg)
/* Flow table interface to the rest of ovn-controller. */
-/* Clears the table of flows desired to be in the switch. Call this before
- * adding the desired flows (with ofctrl_add_flow()). */
-void
-ofctrl_clear_flows(void)
-{
-ovn_flow_table_clear(&desired_flows);
-}
-
-/* Adds a flow with the specified 'match' and 'actions' to the OpenFlow table
- * numbered 'table_id' with the given 'priority'. The caller retains ownership
- * of 'match' and 'actions'.
+/* Adds a flow to 'desired_flows' with the specified 'match' and 'actions' to
+ * the OpenFlow table numbered 'table_id' with the given 'priority'. The
+ * caller retains ownership of 'match' and 'actions'.
*
* This just assembles the desired flow table in memory. Nothing is actually
- * sent to the switch until a later call to ofctrl_run(). */
+ * sent to the switch until a later call to ofctrl_run().
+ *
+ * The caller should initialize its own hmap to hold the flows. */
void
-ofctrl_add_flow(uint8_t table_id, uint16_t priority,
+ofctrl_add_flow(struct hmap *desired_flows,
+uint8_t table_id, uint16_t prio
[ovs-dev] [PATCH v2 03/21] ovn-controller: Avoid blocking to commit OVSDB transactions.
Until now, ovn-controller has been full of loops that commit a transaction
to the OVS or OVN Southbound database. These blocking loops delay other
work within ovn-controller. They also make it unsafe to keep pointers to
database records within a single ovn-controller main loop, since calls
to ovsdb_idl_run() can cause IDL records to be destroyed. This commit
drops all of the blocking calls, instead doing a single commit to the
databases at the end of each main loop.
Signed-off-by: Ben Pfaff
Acked-by: Russell Bryant
---
v1->v2: Rename idl_loop_wait() to idl_loop_commit_and_wait().
---
ovn/controller/binding.c| 61
ovn/controller/binding.h| 4 +-
ovn/controller/chassis.c| 117 ++
ovn/controller/chassis.h| 4 +-
ovn/controller/ovn-controller.c | 123 +---
ovn/controller/ovn-controller.h | 4 ++
6 files changed, 188 insertions(+), 125 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 0a4a39e..6af216c 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -76,10 +76,12 @@ binding_run(struct controller_ctx *ctx)
{
const struct sbrec_chassis *chassis_rec;
const struct sbrec_binding *binding_rec;
-struct ovsdb_idl_txn *txn;
struct sset lports, all_lports;
const char *name;
-int retval;
+
+if (!ctx->ovnsb_idl_txn) {
+return;
+}
chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
if (!chassis_rec) {
@@ -91,8 +93,7 @@ binding_run(struct controller_ctx *ctx)
get_local_iface_ids(ctx, &lports);
sset_clone(&all_lports, &lports);
-txn = ovsdb_idl_txn_create(ctx->ovnsb_idl);
-ovsdb_idl_txn_add_comment(txn,
+ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: updating bindings for '%s'",
ctx->chassis_id);
@@ -115,14 +116,6 @@ binding_run(struct controller_ctx *ctx)
}
}
-retval = ovsdb_idl_txn_commit_block(txn);
-if (retval == TXN_ERROR) {
-VLOG_INFO("Problem committing binding information: %s",
- ovsdb_idl_txn_status_to_string(retval));
-}
-
-ovsdb_idl_txn_destroy(txn);
-
SSET_FOR_EACH (name, &lports) {
VLOG_DBG("No binding record for lport %s", name);
}
@@ -130,40 +123,32 @@ binding_run(struct controller_ctx *ctx)
sset_destroy(&all_lports);
}
-void
-binding_destroy(struct controller_ctx *ctx)
+/* Returns true if the database is all cleaned up, false if more work is
+ * required. */
+bool
+binding_cleanup(struct controller_ctx *ctx)
{
-const struct sbrec_chassis *chassis_rec;
-int retval = TXN_TRY_AGAIN;
-
-ovs_assert(ctx->ovnsb_idl);
+if (!ctx->ovnsb_idl_txn) {
+return false;
+}
-chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
+const struct sbrec_chassis *chassis_rec
+= get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
if (!chassis_rec) {
-return;
+return true;
}
-while (retval != TXN_SUCCESS && retval != TXN_UNCHANGED) {
-const struct sbrec_binding *binding_rec;
-struct ovsdb_idl_txn *txn;
-
-txn = ovsdb_idl_txn_create(ctx->ovnsb_idl);
-ovsdb_idl_txn_add_comment(txn,
+ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: removing all bindings for '%s'",
ctx->chassis_id);
-SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
-if (binding_rec->chassis == chassis_rec) {
-sbrec_binding_set_chassis(binding_rec, NULL);
-}
-}
-
-retval = ovsdb_idl_txn_commit_block(txn);
-if (retval == TXN_ERROR) {
-VLOG_INFO("Problem removing bindings: %s",
- ovsdb_idl_txn_status_to_string(retval));
+const struct sbrec_binding *binding_rec;
+bool any_changes = false;
+SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
+if (binding_rec->chassis == chassis_rec) {
+sbrec_binding_set_chassis(binding_rec, NULL);
+any_changes = true;
}
-
-ovsdb_idl_txn_destroy(txn);
}
+return !any_changes;
}
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index 2611173..e73c1d1 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -17,10 +17,12 @@
#ifndef OVN_BINDING_H
#define OVN_BINDING_H 1
+#include
+
struct controller_ctx;
void binding_init(struct controller_ctx *);
void binding_run(struct controller_ctx *);
-void binding_destroy(struct controller_ctx *);
+bool binding_cleanup(struct controller_ctx *);
#endif /* ovn/binding.h */
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index cf18dd0..9d2959a 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -52,8 +5
[ovs-dev] [PATCH v2 06/21] ovn-controller: Factor encapsulation code out of chassis code.
These two pieces of code have different requirements and hardly anything in
common, and I found it easier to understand and reasonably modify the
overall structure of the program when they were separated.
Signed-off-by: Ben Pfaff
---
ovn/controller/automake.mk | 2 +
ovn/controller/chassis.c | 315 ++---
ovn/controller/chassis.h | 6 +-
ovn/controller/{chassis.c => encaps.c} | 129 ++
ovn/controller/{chassis.h => encaps.h} | 12 +-
ovn/controller/ovn-controller.c| 12 +-
6 files changed, 49 insertions(+), 427 deletions(-)
copy ovn/controller/{chassis.c => encaps.c} (72%)
copy ovn/controller/{chassis.h => encaps.h} (79%)
diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index 25b5f8b..9ed6bec 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -4,6 +4,8 @@ ovn_controller_ovn_controller_SOURCES = \
ovn/controller/binding.h \
ovn/controller/chassis.c \
ovn/controller/chassis.h \
+ ovn/controller/encaps.c \
+ ovn/controller/encaps.h \
ovn/controller/ofctrl.c \
ovn/controller/ofctrl.h \
ovn/controller/ovn-controller.c \
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 4bd39e7..5f1c194 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -16,10 +16,6 @@
#include
#include "chassis.h"
-#include "lib/hash.h"
-#include "lib/poll-loop.h"
-#include "lib/sset.h"
-#include "lib/util.h"
#include "lib/vswitch-idl.h"
#include "openvswitch/vlog.h"
#include "ovn/lib/ovn-sb-idl.h"
@@ -32,21 +28,15 @@ chassis_init(struct controller_ctx *ctx)
{
ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_open_vswitch);
ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_open_vswitch_col_external_ids);
-ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_bridge);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_bridge_col_ports);
-ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_port);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_port_col_name);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_port_col_interfaces);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_port_col_external_ids);
-ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_interface);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_interface_col_name);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_interface_col_type);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_interface_col_options);
}
-static void
-register_chassis(struct controller_ctx *ctx)
+void
+chassis_run(struct controller_ctx *ctx)
{
+if (!ctx->ovnsb_idl_txn) {
+return;
+}
+
const struct sbrec_chassis *chassis_rec;
const struct ovsrec_open_vswitch *cfg;
const char *encap_type, *encap_ip;
@@ -109,303 +99,22 @@ register_chassis(struct controller_ctx *ctx)
inited = true;
}
-/* Enough context to create a new tunnel, using tunnel_add(). */
-struct tunnel_ctx {
-/* Contains "struct port_hash_node"s. Used to figure out what
- * existing tunnels should be deleted: we index all of the OVN encap
- * rows into this data structure, then as existing rows are
- * generated we remove them. After generating all the rows, any
- * remaining in 'tunnel_hmap' must be deleted from the database. */
-struct hmap tunnel_hmap;
-
-/* Names of all ports in the bridge, to allow checking uniqueness when
- * adding a new tunnel. */
-struct sset port_names;
-
-struct ovsdb_idl_txn *ovs_txn;
-const struct ovsrec_bridge *br_int;
-};
-
-struct port_hash_node {
-struct hmap_node node;
-const struct ovsrec_port *port;
-const struct ovsrec_bridge *bridge;
-};
-
-static size_t
-port_hash(const char *chassis_id, const char *type, const char *ip)
-{
-size_t hash = hash_string(chassis_id, 0);
-hash = hash_string(type, hash);
-return hash_string(ip, hash);
-}
-
-static size_t
-port_hash_rec(const struct ovsrec_port *port)
-{
-const char *chassis_id, *ip;
-const struct ovsrec_interface *iface;
-
-chassis_id = smap_get(&port->external_ids, "ovn-chassis-id");
-
-if (!chassis_id || !port->n_interfaces) {
-/* This should not happen for an OVN-created port. */
-return 0;
-}
-
-iface = port->interfaces[0];
-ip = smap_get(&iface->options, "remote_ip");
-
-return port_hash(chassis_id, iface->type, ip);
-}
-
-static char *
-tunnel_create_name(struct tunnel_ctx *tc, const char *chassis_id)
-{
-int i;
-
-for (i = 0; i < UINT16_MAX; i++) {
-char *port_name;
-port_name = xasprintf("ovn-%.6s-%x", chassis_id, i);
-
-if (!sset_contains(&tc->port_names, port_name)) {
-return port_name;
-}
-
-free(port_name);
-}
-
-return NULL;
-}
-
-
-static void
-tunnel_add(struct tunnel_ctx *tc, const char *new_chassis_id,
- const struct sbrec_encap *encap)
-{
-struct port_hash_node *hash_node;
[ovs-dev] [PATCH v2 02/21] ovn-controller: Drop unnecessary checks for ovsdb_idl_is_alive().
The IDLs as created by ovn-controller always retry failed connections,
which means that ovsdb_idl_is_alive() will always report that they are
alive.
Signed-off-by: Ben Pfaff
Acked-by: Russell Bryant
---
ovn/controller/ovn-controller.c | 16
1 file changed, 16 deletions(-)
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index bd3ef0d..6f94187 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -210,22 +210,6 @@ main(int argc, char *argv[])
break;
}
-if (!ovsdb_idl_is_alive(ctx.ovnsb_idl)) {
-int retval = ovsdb_idl_get_last_error(ctx.ovnsb_idl);
-VLOG_ERR("%s: database connection failed (%s)",
- ovnsb_remote, ovs_retval_to_string(retval));
-retval = EXIT_FAILURE;
-break;
-}
-
-if (!ovsdb_idl_is_alive(ctx.ovs_idl)) {
-int retval = ovsdb_idl_get_last_error(ctx.ovs_idl);
-VLOG_ERR("%s: database connection failed (%s)",
- ovs_remote, ovs_retval_to_string(retval));
-retval = EXIT_FAILURE;
-break;
-}
-
ofctrl_clear_flows();
chassis_run(&ctx);
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 05/21] ovn-controller: Pass 'br_int' explicitly to functions that need it.
I found it hard otherwise to see what code depended on this.
Signed-off-by: Ben Pfaff
Acked-by: Russell Bryant
---
ovn/controller/binding.c| 12 ++--
ovn/controller/binding.h| 3 ++-
ovn/controller/chassis.c| 22 +++---
ovn/controller/chassis.h| 7 +--
ovn/controller/ofctrl.c | 11 ++-
ovn/controller/ofctrl.h | 3 ++-
ovn/controller/ovn-controller.c | 37 ++---
ovn/controller/ovn-controller.h | 5 +
ovn/controller/physical.c | 9 +
ovn/controller/physical.h | 4 +++-
10 files changed, 59 insertions(+), 54 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 6af216c..f3b1e16 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -45,16 +45,16 @@ binding_init(struct controller_ctx *ctx)
}
static void
-get_local_iface_ids(struct controller_ctx *ctx, struct sset *lports)
+get_local_iface_ids(const struct ovsrec_bridge *br_int, struct sset *lports)
{
int i;
-for (i = 0; i < ctx->br_int->n_ports; i++) {
-const struct ovsrec_port *port_rec = ctx->br_int->ports[i];
+for (i = 0; i < br_int->n_ports; i++) {
+const struct ovsrec_port *port_rec = br_int->ports[i];
const char *iface_id;
int j;
-if (!strcmp(port_rec->name, ctx->br_int_name)) {
+if (!strcmp(port_rec->name, br_int->name)) {
continue;
}
@@ -72,7 +72,7 @@ get_local_iface_ids(struct controller_ctx *ctx, struct sset
*lports)
}
void
-binding_run(struct controller_ctx *ctx)
+binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
{
const struct sbrec_chassis *chassis_rec;
const struct sbrec_binding *binding_rec;
@@ -90,7 +90,7 @@ binding_run(struct controller_ctx *ctx)
sset_init(&lports);
sset_init(&all_lports);
-get_local_iface_ids(ctx, &lports);
+get_local_iface_ids(br_int, &lports);
sset_clone(&all_lports, &lports);
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index e73c1d1..dbcc6fb 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -20,9 +20,10 @@
#include
struct controller_ctx;
+struct ovsrec_bridge;
void binding_init(struct controller_ctx *);
-void binding_run(struct controller_ctx *);
+void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int);
bool binding_cleanup(struct controller_ctx *);
#endif /* ovn/binding.h */
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 9d2959a..4bd39e7 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -296,7 +296,7 @@ preferred_encap(const struct sbrec_chassis *chassis_rec)
}
static void
-update_encaps(struct controller_ctx *ctx)
+update_encaps(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
{
const struct sbrec_chassis *chassis_rec;
const struct ovsrec_bridge *br;
@@ -304,7 +304,7 @@ update_encaps(struct controller_ctx *ctx)
struct tunnel_ctx tc = {
.tunnel_hmap = HMAP_INITIALIZER(&tc.tunnel_hmap),
.port_names = SSET_INITIALIZER(&tc.port_names),
-.br_int = ctx->br_int
+.br_int = br_int
};
tc.ovs_txn = ctx->ovs_idl_txn;
@@ -357,21 +357,21 @@ update_encaps(struct controller_ctx *ctx)
}
void
-chassis_run(struct controller_ctx *ctx)
+chassis_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
{
if (ctx->ovnsb_idl_txn) {
register_chassis(ctx);
}
if (ctx->ovs_idl_txn) {
-update_encaps(ctx);
+update_encaps(ctx, br_int);
}
}
/* Returns true if the database is all cleaned up, false if more work is
* required. */
bool
-chassis_cleanup(struct controller_ctx *ctx)
+chassis_cleanup(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
{
if (!ctx->ovnsb_idl_txn || !ctx->ovs_idl_txn) {
return false;
@@ -394,17 +394,17 @@ chassis_cleanup(struct controller_ctx *ctx)
ovsdb_idl_txn_add_comment(ctx->ovs_idl_txn,
"ovn-controller: destroying tunnels");
struct ovsrec_port **ports
-= xmalloc(sizeof *ctx->br_int->ports * ctx->br_int->n_ports);
+= xmalloc(sizeof *br_int->ports * br_int->n_ports);
size_t n = 0;
-for (size_t i = 0; i < ctx->br_int->n_ports; i++) {
-if (!smap_get(&ctx->br_int->ports[i]->external_ids,
+for (size_t i = 0; i < br_int->n_ports; i++) {
+if (!smap_get(&br_int->ports[i]->external_ids,
"ovn-chassis-id")) {
-ports[n++] = ctx->br_int->ports[i];
+ports[n++] = br_int->ports[i];
any_changes = true;
}
}
-ovsrec_bridge_verify_ports(ctx->br_int);
-ovsrec_bridge_set_ports(ctx->br_int, ports, n);
+ovsrec_bridge_verify_ports(br_int);
+ovsrec_bridge_set_ports(br_int, ports
[ovs-dev] [PATCH v2 07/21] ovn-controller: Pass 'chassis_id' explicitly to functions that need it.
I found it otherwise difficult to see what code depended on this.
Signed-off-by: Ben Pfaff
---
ovn/controller/binding.c| 13 +++--
ovn/controller/binding.h| 5 +++--
ovn/controller/chassis.c| 14 +++---
ovn/controller/chassis.h| 4 ++--
ovn/controller/encaps.c | 7 ---
ovn/controller/encaps.h | 4 ++--
ovn/controller/ovn-controller.c | 25 +
ovn/controller/ovn-controller.h | 2 --
ovn/controller/physical.c | 4 ++--
ovn/controller/physical.h | 1 +
10 files changed, 41 insertions(+), 38 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index f3b1e16..2cb0b42 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -72,7 +72,8 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
struct sset *lports)
}
void
-binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
+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;
@@ -83,7 +84,7 @@ binding_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int)
return;
}
-chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
+chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, chassis_id);
if (!chassis_rec) {
return;
}
@@ -95,7 +96,7 @@ binding_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int)
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: updating bindings for '%s'",
- ctx->chassis_id);
+ chassis_id);
SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
if (sset_find_and_delete(&lports, binding_rec->logical_port) ||
@@ -126,21 +127,21 @@ binding_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int)
/* Returns true if the database is all cleaned up, false if more work is
* required. */
bool
-binding_cleanup(struct controller_ctx *ctx)
+binding_cleanup(struct controller_ctx *ctx, const char *chassis_id)
{
if (!ctx->ovnsb_idl_txn) {
return false;
}
const struct sbrec_chassis *chassis_rec
-= get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
+= get_chassis_by_name(ctx->ovnsb_idl, chassis_id);
if (!chassis_rec) {
return true;
}
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: removing all bindings for '%s'",
- ctx->chassis_id);
+ chassis_id);
const struct sbrec_binding *binding_rec;
bool any_changes = false;
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index dbcc6fb..c60235f 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -23,7 +23,8 @@ struct controller_ctx;
struct ovsrec_bridge;
void binding_init(struct controller_ctx *);
-void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int);
-bool binding_cleanup(struct controller_ctx *);
+void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
+ const char *chassis_id);
+bool binding_cleanup(struct controller_ctx *, const char *chassis_id);
#endif /* ovn/binding.h */
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 5f1c194..511d5c9 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -31,7 +31,7 @@ chassis_init(struct controller_ctx *ctx)
}
void
-chassis_run(struct controller_ctx *ctx)
+chassis_run(struct controller_ctx *ctx, const char *chassis_id)
{
if (!ctx->ovnsb_idl_txn) {
return;
@@ -43,7 +43,7 @@ chassis_run(struct controller_ctx *ctx)
struct sbrec_encap *encap_rec;
static bool inited = false;
-chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
+chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, chassis_id);
/* xxx Need to support more than one encap. Also need to support
* xxx encap options. */
@@ -82,11 +82,11 @@ chassis_run(struct controller_ctx *ctx)
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
"ovn-controller: registering chassis '%s'",
- ctx->chassis_id);
+ chassis_id);
if (!chassis_rec) {
chassis_rec = sbrec_chassis_insert(ctx->ovnsb_idl_txn);
-sbrec_chassis_set_name(chassis_rec, ctx->chassis_id);
+sbrec_chassis_set_name(chassis_rec, chassis_id);
}
encap_rec = sbrec_encap_insert(ctx->ovnsb_idl_txn);
@@ -102,18 +102,18 @@ chassis_run(struct controller_ctx *ctx)
/* Returns true if the database is all cleaned up, false if more work is
* required. */
bool
-chassis_cleanup(struct controller_ctx *ctx)
+chassis_cleanup(
[ovs-dev] [PATCH v2 10/21] ovn-controller: Rename init functions that just register IDL columns.
The generic *_init() names for these functions made it sounds like they
do something more than just register IDL columns, even though that's all
they do. Also, the controller_ctx that was passed into each of them was
only used to get the IDL handle. This commit renames them and changes
their parameter type to simplify and make all of this clearer.
Signed-off-by: Ben Pfaff
---
ovn/controller/binding.c| 24
ovn/controller/binding.h| 3 ++-
ovn/controller/chassis.c| 6 +++---
ovn/controller/chassis.h| 3 ++-
ovn/controller/encaps.c | 22 +++---
ovn/controller/encaps.h | 3 ++-
ovn/controller/ovn-controller.c | 11 ---
ovn/controller/physical.c | 26 +-
ovn/controller/physical.h | 3 ++-
9 files changed, 51 insertions(+), 50 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 4cf8636..c83b044 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -26,22 +26,22 @@
VLOG_DEFINE_THIS_MODULE(binding);
void
-binding_init(struct controller_ctx *ctx)
+binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
{
-ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_open_vswitch);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_open_vswitch_col_bridges);
+ovsdb_idl_add_table(ovs_idl, &ovsrec_table_open_vswitch);
+ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_bridges);
-ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_bridge);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_bridge_col_name);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_bridge_col_ports);
+ovsdb_idl_add_table(ovs_idl, &ovsrec_table_bridge);
+ovsdb_idl_add_column(ovs_idl, &ovsrec_bridge_col_name);
+ovsdb_idl_add_column(ovs_idl, &ovsrec_bridge_col_ports);
-ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_port);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_port_col_name);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_port_col_interfaces);
+ovsdb_idl_add_table(ovs_idl, &ovsrec_table_port);
+ovsdb_idl_add_column(ovs_idl, &ovsrec_port_col_name);
+ovsdb_idl_add_column(ovs_idl, &ovsrec_port_col_interfaces);
-ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_interface);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_interface_col_name);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_interface_col_external_ids);
+ovsdb_idl_add_table(ovs_idl, &ovsrec_table_interface);
+ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_name);
+ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_external_ids);
}
static void
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index c60235f..708a6e6 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -20,9 +20,10 @@
#include
struct controller_ctx;
+struct ovsdb_idl;
struct ovsrec_bridge;
-void binding_init(struct controller_ctx *);
+void binding_register_ovs_idl(struct ovsdb_idl *);
void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
const char *chassis_id);
bool binding_cleanup(struct controller_ctx *, const char *chassis_id);
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 2dfce19..9d6a77a 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -24,10 +24,10 @@
VLOG_DEFINE_THIS_MODULE(chassis);
void
-chassis_init(struct controller_ctx *ctx)
+chassis_register_ovs_idl(struct ovsdb_idl *ovs_idl)
{
-ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_open_vswitch);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_open_vswitch_col_external_ids);
+ovsdb_idl_add_table(ovs_idl, &ovsrec_table_open_vswitch);
+ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_external_ids);
}
void
diff --git a/ovn/controller/chassis.h b/ovn/controller/chassis.h
index 24648b2..26017d0 100644
--- a/ovn/controller/chassis.h
+++ b/ovn/controller/chassis.h
@@ -19,9 +19,10 @@
#include
struct controller_ctx;
+struct ovsdb_idl;
struct ovsrec_bridge;
-void chassis_init(struct controller_ctx *);
+void chassis_register_ovs_idl(struct ovsdb_idl *);
void chassis_run(struct controller_ctx *, const char *chassis_id);
bool chassis_cleanup(struct controller_ctx *, const char *chassis_id);
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index f150246..0872bf4 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -27,18 +27,18 @@
VLOG_DEFINE_THIS_MODULE(encaps);
void
-encaps_init(struct controller_ctx *ctx)
+encaps_register_ovs_idl(struct ovsdb_idl *ovs_idl)
{
-ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_bridge);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_bridge_col_ports);
-ovsdb_idl_add_table(ctx->ovs_idl, &ovsrec_table_port);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_port_col_name);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsrec_port_col_interfaces);
-ovsdb_idl_add_column(ctx->ovs_idl, &ovsr
[ovs-dev] [PATCH v2 11/21] ovn-controller: Honor external-ids:ovn-bridge changing.
Until now, if external-ids:ovn-bridge changed, ovn-controller ignored
the change. With this commit, ovn-controller uses the new bridge.
Signed-off-by: Ben Pfaff
---
ovn/controller/ovn-controller.c | 81 +
1 file changed, 33 insertions(+), 48 deletions(-)
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 49d23d7..303a722 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -56,8 +56,6 @@ static void parse_options(int argc, char *argv[]);
OVS_NO_RETURN static void usage(void);
static char *ovs_remote;
-static char *ovnsb_remote;
-
static void
get_initial_snapshot(struct ovsdb_idl *idl)
@@ -73,12 +71,21 @@ get_initial_snapshot(struct ovsdb_idl *idl)
}
static const struct ovsrec_bridge *
-get_bridge(struct controller_ctx *ctx, const char *name)
+get_br_int(struct ovsdb_idl *ovs_idl)
{
-const struct ovsrec_bridge *br;
+const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
+if (!cfg) {
+return NULL;
+}
+
+const char *br_int_name = smap_get(&cfg->external_ids, "ovn-bridge");
+if (!br_int_name) {
+br_int_name = DEFAULT_BRIDGE_NAME;
+}
-OVSREC_BRIDGE_FOR_EACH(br, ctx->ovs_idl) {
-if (!strcmp(br->name, name)) {
+const struct ovsrec_bridge *br;
+OVSREC_BRIDGE_FOR_EACH (br, ovs_idl) {
+if (!strcmp(br->name, br_int_name)) {
return br;
}
}
@@ -93,49 +100,30 @@ get_chassis_id(const struct ovsdb_idl *ovs_idl)
return cfg ? smap_get(&cfg->external_ids, "system-id") : NULL;
}
-/* Retrieve the OVN integration bridge from the "external-ids:ovn-bridge"
- * key, the remote location from the "external-ids:ovn-remote" key, and
- * the chassis name from the "external-ids:system-id" key in the
- * Open_vSwitch table of the OVS database instance.
+/* Retrieves the OVN Southbound remote location from the
+ * "external-ids:ovn-remote" key in 'ovs_idl' and returns a copy of it.
*
- * xxx ovn-controller does not support changing any of these mid-run,
- * xxx but that should be addressed later. */
-static void
-get_core_config(struct controller_ctx *ctx, char **br_int_namep)
+ * XXX ovn-controller does not support this changing mid-run, but that should
+ * be addressed later. */
+static char *
+get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
{
while (1) {
-ovsdb_idl_run(ctx->ovs_idl);
-
-const struct ovsrec_open_vswitch *cfg;
-cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
-if (!cfg) {
-VLOG_ERR("No Open_vSwitch row defined.");
-ovsdb_idl_destroy(ctx->ovs_idl);
-exit(EXIT_FAILURE);
-}
-
-const char *remote, *br_int_name;
-
-br_int_name = smap_get(&cfg->external_ids, "ovn-bridge");
-if (!br_int_name) {
-br_int_name = DEFAULT_BRIDGE_NAME;
-}
-
-remote = smap_get(&cfg->external_ids, "ovn-remote");
-if (!remote) {
-VLOG_INFO("OVN OVSDB remote not specified. Waiting...");
-goto try_again;
+ovsdb_idl_run(ovs_idl);
+
+const struct ovsrec_open_vswitch *cfg
+= ovsrec_open_vswitch_first(ovs_idl);
+if (cfg) {
+const char *remote = smap_get(&cfg->external_ids, "ovn-remote");
+if (remote) {
+return xstrdup(remote);
+}
}
-ovnsb_remote = xstrdup(remote);
-*br_int_namep = xstrdup(br_int_name);
-return;
-
-try_again:
-ovsdb_idl_wait(ctx->ovs_idl);
+VLOG_INFO("OVN OVSDB remote not specified. Waiting...");
+ovsdb_idl_wait(ovs_idl);
poll_block();
}
-
}
struct idl_loop {
@@ -252,9 +240,7 @@ main(int argc, char *argv[])
get_initial_snapshot(ctx.ovs_idl);
-char *br_int_name;
-get_core_config(&ctx, &br_int_name);
-
+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);
@@ -268,7 +254,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);
const char *chassis_id = get_chassis_id(ctx.ovs_idl);
if (chassis_id) {
@@ -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);
const char *chassis_id = get_chassis_id(ctx.ovs_idl);
/* Run all of the cleanup functions, even if one of them r
[ovs-dev] [PATCH v2 09/21] ovn-controller: Tolerate missing 'chassis_id'.
Until now, if the chassis id was missing, ovn-controller exited. This
commit makes ovn-controller wait for it to return.
Signed-off-by: Ben Pfaff
---
ovn/controller/binding.c| 3 +++
ovn/controller/chassis.c| 4
ovn/controller/ovn-controller.c | 38 +-
3 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index b6b345e..4cf8636 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -135,6 +135,9 @@ binding_cleanup(struct controller_ctx *ctx, const char
*chassis_id)
return false;
}
+if (!chassis_id) {
+return true;
+}
const struct sbrec_chassis *chassis_rec
= get_chassis_by_name(ctx->ovnsb_idl, chassis_id);
if (!chassis_rec) {
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 511d5c9..2dfce19 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -104,6 +104,10 @@ chassis_run(struct controller_ctx *ctx, const char
*chassis_id)
bool
chassis_cleanup(struct controller_ctx *ctx, const char *chassis_id)
{
+if (!chassis_id) {
+return true;
+}
+
/* Delete Chassis row. */
const struct sbrec_chassis *chassis_rec
= get_chassis_by_name(ctx->ovnsb_idl, chassis_id);
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index d7ab7a3..2746407 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -86,6 +86,13 @@ get_bridge(struct controller_ctx *ctx, const char *name)
return NULL;
}
+static const char *
+get_chassis_id(const struct ovsdb_idl *ovs_idl)
+{
+const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
+return cfg ? smap_get(&cfg->external_ids, "system-id") : NULL;
+}
+
/* Retrieve the OVN integration bridge from the "external-ids:ovn-bridge"
* key, the remote location from the "external-ids:ovn-remote" key, and
* the chassis name from the "external-ids:system-id" key in the
@@ -94,8 +101,7 @@ get_bridge(struct controller_ctx *ctx, const char *name)
* xxx ovn-controller does not support changing any of these mid-run,
* xxx but that should be addressed later. */
static void
-get_core_config(struct controller_ctx *ctx, char **br_int_namep,
-char **chassis_idp)
+get_core_config(struct controller_ctx *ctx, char **br_int_namep)
{
while (1) {
ovsdb_idl_run(ctx->ovs_idl);
@@ -108,7 +114,7 @@ get_core_config(struct controller_ctx *ctx, char
**br_int_namep,
exit(EXIT_FAILURE);
}
-const char *remote, *system_id, *br_int_name;
+const char *remote, *br_int_name;
br_int_name = smap_get(&cfg->external_ids, "ovn-bridge");
if (!br_int_name) {
@@ -121,14 +127,7 @@ get_core_config(struct controller_ctx *ctx, char
**br_int_namep,
goto try_again;
}
-system_id = smap_get(&cfg->external_ids, "system-id");
-if (!system_id) {
-VLOG_INFO("system-id not specified. Waiting...");
-goto try_again;
-}
-
ovnsb_remote = xstrdup(remote);
-*chassis_idp = xstrdup(system_id);
*br_int_namep = xstrdup(br_int_name);
return;
@@ -256,8 +255,8 @@ main(int argc, char *argv[])
get_initial_snapshot(ctx.ovs_idl);
-char *br_int_name, *chassis_id;
-get_core_config(&ctx, &br_int_name, &chassis_id);
+char *br_int_name;
+get_core_config(&ctx, &br_int_name);
ctx.ovnsb_idl = ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class,
true, true);
@@ -273,15 +272,20 @@ main(int argc, char *argv[])
ctx.ovs_idl_txn = idl_loop_run(&ovs_idl_loop);
const struct ovsrec_bridge *br_int = get_bridge(&ctx, br_int_name);
+const char *chassis_id = get_chassis_id(ctx.ovs_idl);
-chassis_run(&ctx, chassis_id);
-encaps_run(&ctx, br_int, chassis_id);
-binding_run(&ctx, br_int, chassis_id);
+if (chassis_id) {
+chassis_run(&ctx, chassis_id);
+encaps_run(&ctx, br_int, chassis_id);
+binding_run(&ctx, br_int, chassis_id);
+}
if (br_int) {
struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
pipeline_run(&ctx, &flow_table);
-physical_run(&ctx, br_int, chassis_id, &flow_table);
+if (chassis_id) {
+physical_run(&ctx, br_int, chassis_id, &flow_table);
+}
ofctrl_run(br_int, &flow_table);
hmap_destroy(&flow_table);
}
@@ -309,6 +313,7 @@ main(int argc, char *argv[])
ctx.ovs_idl_txn = idl_loop_run(&ovs_idl_loop);
const struct ovsrec_bridge *br_int = get_bridge(&ctx, br_int_name);
+const char *chassis_id = get_chassis_id(ctx.ovs_idl);
/* Run all of the cleanup functions, even if one of them returns false.
[ovs-dev] [PATCH v2 13/21] ovn-controller: Use controller_ctx just to pass around data.
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
[ovs-dev] [PATCH v2 15/21] nroff: Add support for 'diagram' XML element for protocol headers.
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
---
python/build/nroff.py | 98 +++
1 file changed, 98 insertions(+)
diff --git a/python/build/nroff.py b/python/build/nroff.py
index 6d22d46..778ce0f 100644
--- a/python/build/nroff.py
+++ b/python/build/nroff.py
@@ -105,6 +105,102 @@ def pre_to_nroff(nodes, para, font):
s += '.fi\n'
return s
+def diagram_header_to_nroff(header_node):
+header_fields = []
+i = 0
+for node in header_node.childNodes:
+if node.nodeType == node.ELEMENT_NODE and node.tagName == 'bits':
+name = node.attributes['name'].nodeValue
+width = node.attributes['width'].nodeValue
+above = node.getAttribute('above')
+below = node.getAttribute('below')
+fill = node.getAttribute('fill')
+header_fields += [{"name": name,
+ "tag": "B%d" % i,
+ "width": width,
+ "above": above,
+ "below": below,
+ "fill": fill}]
+i += 1
+elif node.nodeType == node.COMMENT_NODE:
+pass
+elif node.nodeType == node.TEXT_NODE and node.data.isspace():
+pass
+else:
+fatal("unknown node %s in diagram element" % node)
+
+pic_s = ""
+for f in header_fields:
+pic_s += " %s: box \"%s\" width %s" % (f['tag'], f['name'],
f['width'])
+if f['fill'] == 'yes':
+pic_s += " fill"
+pic_s += '\n'
+for f in header_fields:
+pic_s += " \"%s\" at %s.n above\n" % (f['above'], f['tag'])
+pic_s += " \"%s\" at %s.s below\n" % (f['below'], f['tag'])
+name = header_node.getAttribute('name')
+if name == "":
+visible = " invis"
+else:
+visible = ""
+pic_s += "line <->%s \"%s\" above " % (visible, name)
+pic_s += "from %s.nw + (0,textht) " % header_fields[0]['tag']
+pic_s += "to %s.ne + (0,textht)\n" % header_fields[-1]['tag']
+
+text_s = ""
+for f in header_fields:
+text_s += """.IP \\(bu
+%s bits""" % (f['above'])
+if f['name']:
+text_s += ": %s" % f['name']
+if f['below']:
+text_s += " (%s)" % f['below']
+text_s += "\n"
+return pic_s, text_s
+
+def diagram_to_nroff(nodes, para):
+pic_s = ''
+text_s = ''
+move = False
+for node in nodes:
+if node.nodeType == node.ELEMENT_NODE and node.tagName == 'header':
+if move:
+pic_s += "move .1\n"
+text_s += ".sp\n"
+pic_header, text_header = diagram_header_to_nroff(node)
+pic_s += "[\n" + pic_header + "]\n"
+text_s += text_header
+move = True
+elif node.nodeType == node.ELEMENT_NODE and node.tagName == 'nospace':
+move = False
+elif node.nodeType == node.ELEMENT_NODE and node.tagName == 'dots':
+pic_s += "move .1\n"
+pic_s += '". . ." ljust\n'
+text_s += ".sp\n"
+elif node.nodeType == node.COMMENT_NODE:
+pass
+elif node.nodeType == node.TEXT_NODE and node.data.isspace():
+pass
+else:
+fatal("unknown node %s in diagram element" % node)
+return para + """
+.\\" check if in troff mode (TTY)
+.if t \{
+.PS
+boxht = .2
+textht = 1/6
+fillval = .2
+""" + pic_s + """\
+.PE
+\\}
+.\\" check if in nroff mode:
+.if n \{
+.RS
+""" + text_s + """\
+.RE
+\\}"""
+return s
+
def block_xml_to_nroff(nodes, para='.PP'):
s = ''
for node in nodes:
@@ -176,6 +272,8 @@ def block_xml_to_nroff(nodes, para='.PP'):
else:
font = r'\fB'
s += pre_to_nroff(node.childNodes, para, font)
+elif node.tagName == 'diagram':
+s += diagram_to_nroff(node.childNodes, para)
else:
s += inline_xml_to_nroff(node, r'\fR')
elif node.nodeType == node.COMMENT_NODE:
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 14/21] smap: New function smap_get_uuid().
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
[ovs-dev] [PATCH v2 12/21] ovn-controller: Slightly adjust pipeline init and destroy for consistency.
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
[ovs-dev] [PATCH v2 16/21] ovn: Rename Binding table to Port_Binding.
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, ctx->ovnsb_idl) {
struct logical_datapath *ldp;
ldp = ldp_lookup(&binding->logical_datapath);
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 70d868a..2ad727c 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -406,7
[ovs-dev] [PATCH v2 18/21] actions: Allow caller to specify output table.
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);
if (error) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s",
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 0aabdcf..e15c7f8 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -31,6 +31,7 @@ struct action_context {
struct lexer *lexer;/* Lexer for pulling more tokens. */
const struct shash *symtab; /* Symbol table. */
uint8_t next_table_id; /* OpenFlow table for 'next' to resubmit. */
+uint8_t output_table_id;/* OpenFlow table for 'output' to resubmit. */
const struct simap *ports; /* Map from port name to number. */
/* State. */
@@ -161,8 +162,7 @@ parse_actions(struct action_context *ctx)
action_error(ctx, "\"next\" action not allowed here.");
}
} else if (lexer_match_id(ctx->lexer, "output")) {
-/* Table 64 does logical-to-physical translation. */
-emit_resubmit(ctx, 64);
+emit_resubmit(ctx, ctx->output_table_id);
} else {
action_syntax_error(ctx, "expecting action");
}
@@ -189,6 +189,9 @@ parse_actions(struct action_context *ctx)
* 'next_table_id' should be the OpenFlow table to which the "next" action will
* resubmit, or 0 to disable "next".
*
+ * 'output_table_id' should be the OpenFlow table to which the "output" action
+ * will resubmit
+ *
* Some actions add extra requirements (prerequisites) to the flow's match. If
* so, this function sets '*prereqsp' to the actions' prerequisites; otherwise,
* it sets '*prereqsp' to NULL. The caller owns '*prereqsp' and must
@@ -202,7 +205,8 @@ parse_actions(struct action_context *ctx)
char * OVS_WARN_UNUSED_RESULT
actions_parse(struct lexer *lexer, const struct shash *symtab,
const struct simap *ports, uint8_t next_table_id,
- struct ofpbuf *ofpacts, struct expr **prereqsp)
+ uint8_t output_table_id, struct ofpbuf *ofpacts,
+ struct expr **prereqsp)
{
size_t ofpacts_start = ofpacts->size;
@@ -211,6 +215,7 @@ actions_parse(struct lexer *lexer, const struct shash
*symtab,
ctx.symtab = symtab;
ctx.ports = ports;
ctx.next_table_id = next_table_id;
+ctx.output_table_id = output_table_id;
ctx.error = NULL;
ctx.ofpacts = ofpacts;
ctx.prereqs = NULL;
@@ -232,7 +237,8 @@ actions_parse(struct lexer *lexer, const struct shash
*symtab,
char * OVS_WARN_UNUSED_RESULT
actions_parse_string(const char *s, const struct shash *symtab,
const struct simap *ports, uint8_t next_table_id,
- struct ofpbuf *ofpacts, struct expr **prereqsp)
+ uint8_t output_table_id, struct ofpbuf *ofpacts,
+ struct expr **prereqsp)
{
struct lexer lexer;
char *error;
@@ -240,7 +246,7 @@ actions_parse_string(const char *s, const struct shash
*symtab,
lexer_init(&lexer, s);
lexer_get(&lexer);
error = actions_parse(&lexer, symtab, ports, next_table_id,
- ofpacts, prereqsp);
+ output_table_id, ofpacts, prereqsp);
lexer_destroy(&lexer);
return error;
diff --git a/ovn/lib/actions.h b/ovn/lib/actions.h
index b442061..74cd185 100644
--- a/ovn/lib/actions.h
+++ b/ovn/lib/actions.h
@@ -28,11 +28,13 @@ struct simap;
char *actions_parse(struct lexer *, const struct shash *symtab,
const struct simap *ports, uint8_t next_table_id,
-struct ofpbuf *ofpacts, struct expr **prereqsp)
+uint8_t output_table_id, struct ofpbuf *ofpacts,
+struct expr **prereqsp)
OVS_WARN_UNUSED_RESULT;
char *actions_parse_string(const char *s, const struct shash *symtab,
const struct simap *ports, uint8_t next_table_id,
- str
[ovs-dev] [PATCH v2 20/21] ofctrl: Negotiate OVN Geneve option.
This won't really get used until the next commit.
Signed-off-by: Ben Pfaff
---
ovn/controller/ofctrl.c | 470
ovn/controller/ofctrl.h | 5 +-
ovn/controller/ovn-controller.c | 6 +-
ovn/controller/physical.h | 7 +
4 files changed, 348 insertions(+), 140 deletions(-)
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index b1c421c..d8a0573 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -27,9 +27,10 @@
#include "openflow/openflow.h"
#include "openvswitch/vlog.h"
#include "ovn-controller.h"
-#include "vswitch-idl.h"
+#include "physical.h"
#include "rconn.h"
#include "socket-util.h"
+#include "vswitch-idl.h"
VLOG_DEFINE_THIS_MODULE(ofctrl);
@@ -53,6 +54,9 @@ static char *ovn_flow_to_string(const struct ovn_flow *);
static void ovn_flow_log(const struct ovn_flow *, const char *action);
static void ovn_flow_destroy(struct ovn_flow *);
+static ovs_be32 queue_msg(struct ofpbuf *);
+static void queue_flow_mod(struct ofputil_flow_mod *);
+
/* OpenFlow connection to the switch. */
static struct rconn *swconn;
@@ -60,6 +64,25 @@ static struct rconn *swconn;
* rconn_get_connection_seqno(rconn), 'swconn' has reconnected. */
static unsigned int seqno;
+/* Connection state machine. */
+#define STATES \
+STATE(S_NEW)\
+STATE(S_GENEVE_TABLE_REQUESTED) \
+STATE(S_GENEVE_TABLE_MOD_SENT) \
+STATE(S_CLEAR_FLOWS)\
+STATE(S_UPDATE_FLOWS)
+enum ofctrl_state {
+#define STATE(NAME) NAME,
+STATES
+#undef STATE
+};
+
+/* Current state. */
+static enum ofctrl_state state;
+
+/* Transaction IDs for messages in flight to the switch. */
+static ovs_be32 xid, xid2;
+
/* Counter for in-flight OpenFlow messages on 'swconn'. We only send a new
* round of flow table modifications to the switch when the counter falls to
* zero, to avoid unbounded buffering. */
@@ -69,11 +92,15 @@ static struct rconn_packet_counter *tx_counter;
* installed in the switch. */
static struct hmap installed_flows;
+/* MFF_* field ID for our Geneve option. In S_GENEVE_TABLE_MOD_SENT, this is
+ * the option we requested (we don't know whether we obtained it yet). In
+ * S_CLEAR_FLOWS or S_UPDATE_FLOWS, this is really the option we have. */
+static enum mf_field_id mff_ovn_geneve;
+
static void ovn_flow_table_clear(struct hmap *flow_table);
static void ovn_flow_table_destroy(struct hmap *flow_table);
-static void ofctrl_update_flows(struct hmap *desired_flows);
-static void ofctrl_recv(const struct ofpbuf *msg);
+static void ofctrl_recv(const struct ofp_header *, enum ofptype);
void
ofctrl_init(void)
@@ -82,15 +109,244 @@ ofctrl_init(void)
tx_counter = rconn_packet_counter_create();
hmap_init(&installed_flows);
}
+
+/* S_NEW, for a new connection.
+ *
+ * Sends NXT_GENEVE_TABLE_REQUEST and transitions to
+ * S_GENEVE_TABLE_REQUESTED. */
-/* Attempts to update the OpenFlow flows in bridge 'br_int' to those in
- * 'flow_table'. Removes all of the flows from 'flow_table' and frees them.
+static void
+run_S_NEW(void)
+{
+struct ofpbuf *buf = ofpraw_alloc(OFPRAW_NXT_GENEVE_TABLE_REQUEST,
+ rconn_get_version(swconn), 0);
+xid = queue_msg(buf);
+state = S_GENEVE_TABLE_REQUESTED;
+}
+
+static void
+recv_S_NEW(const struct ofp_header *oh OVS_UNUSED,
+ enum ofptype type OVS_UNUSED)
+{
+OVS_NOT_REACHED();
+}
+
+/* S_GENEVE_TABLE_REQUESTED, when NXT_GENEVE_TABLE_REQUEST has been sent
+ * and we're waiting for a reply.
*
- * The flow table will only be updated if we've got an OpenFlow connection to
- * 'br_int' and it's not backlogged. Otherwise, it'll have to wait until the
- * next iteration. */
-void
-ofctrl_run(const struct ovsrec_bridge *br_int, struct hmap *flow_table)
+ * If we receive an NXT_GENEVE_TABLE_REPLY:
+ *
+ * - If it contains our tunnel metadata option, assign its field ID to
+ * mff_ovn_geneve and transition to S_CLEAR_FLOWS.
+ *
+ * - Otherwise, if there is an unused tunnel metadata field ID, send
+ * NXT_GENEVE_TABLE_MOD and OFPT_BARRIER_REQUEST, and transition to
+ * S_GENEVE_TABLE_MOD_SENT.
+ *
+ * - Otherwise, log an error, disable Geneve, and transition to
+ * S_CLEAR_FLOWS.
+ *
+ * If we receive an OFPT_ERROR:
+ *
+ * - Log an error, disable Geneve, and transition to S_CLEAR_FLOWS. */
+
+static void
+run_S_GENEVE_TABLE_REQUESTED(void)
+{
+}
+
+static void
+recv_S_GENEVE_TABLE_REQUESTED(const struct ofp_header *oh, enum ofptype type)
+{
+if (oh->xid != xid) {
+ofctrl_recv(oh, type);
+} else if (type == OFPTYPE_NXT_GENEVE_TABLE_REPLY) {
+struct ofputil_geneve_table_reply reply;
+enum ofperr error = ofputil_decode_geneve_table_reply(oh, &reply);
+if (error) {
+VLOG_ERR("failed to decode Geneve table requ
[ovs-dev] [PATCH v2 17/21] ovn: Rename Pipeline table to Rule table.
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..0f5971b 100644
--- a/ovn/controller/pipeline.c
+++ b/ovn/controller/rule.c
@@ -14,7 +14,7 @@
*/
#include
-#include "pipeline.h"
+#include "rule.h"
#include "dynamic-string.h"
#include "ofctrl.h"
#include "ofp-actions.h"
@@ -26,11 +26,11 @@
#include "ovn/lib/ovn-sb-idl.h"
#include "simap.h"
-VLOG_DEFINE_THIS_MODULE(pipeline);
+VLOG_DEFINE_THIS_MODULE(rule);
/* Symbol table. */
-/* Contains "struct expr_symbol"s for fields supported by OVN pipeline. */
+/* Contains "struct expr_symbol"s for fields supported by OVN rules. */
static struct shash symtab;
static void
@@ -244,31 +244,31 @@ ldp_destroy(void)
}
void
-pipeline_init(void)
+rule_init(void)
{
symtab_init();
}
-/* Translates logical flows in the Pipeline table in the OVN_SB database
- * into Ope
[ovs-dev] [PATCH v2 19/21] rule: Introduce MFF_LOG_DATAPATH macro for consistency.
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
Re: [ovs-dev] [PATCH v2] doc: Document proposed OVN Gateway HA design.
On Tue, Jul 21, 2015 at 12:04:03PM -0700, Ethan Jackson wrote: > High availability for gateways in network virtualization deployments > is fairly difficult to get right. There are a ton of options, most of > which are too complicated or perform badly. To help solve this > problem, this patch proposes an HA design based on some of the lessons > learned building similar systems. The hope is that it can be used as > a starting point for design discussions and an eventual > implementation. > > Signed-off-by: Ethan Jackson Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2 08/21] ovn-controller: Tolerate missing integration bridge.
Until now, if the integration bridge was missing, ovn-controller exited.
This commit makes it wait until the integration bridge is created.
Signed-off-by: Ben Pfaff
---
ovn/controller/binding.c| 4 +++-
ovn/controller/encaps.c | 2 +-
ovn/controller/ofctrl.c | 16 ++--
ovn/controller/ovn-controller.c | 39 ++-
4 files changed, 24 insertions(+), 37 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 2cb0b42..b6b345e 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -91,7 +91,9 @@ binding_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
sset_init(&lports);
sset_init(&all_lports);
-get_local_iface_ids(br_int, &lports);
+if (br_int) {
+get_local_iface_ids(br_int, &lports);
+}
sset_clone(&all_lports, &lports);
ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index 7aa523f..f150246 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -231,7 +231,7 @@ void
encaps_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
const char *chassis_id)
{
-if (!ctx->ovs_idl_txn) {
+if (!ctx->ovs_idl_txn || !br_int) {
return;
}
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index c548645..b1c421c 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -92,13 +92,17 @@ ofctrl_init(void)
void
ofctrl_run(const struct ovsrec_bridge *br_int, struct hmap *flow_table)
{
-char *target;
-target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name);
-if (strcmp(target, rconn_get_target(swconn))) {
-VLOG_INFO("%s: connecting to switch", target);
-rconn_connect(swconn, target, target);
+if (br_int) {
+char *target;
+target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name);
+if (strcmp(target, rconn_get_target(swconn))) {
+VLOG_INFO("%s: connecting to switch", target);
+rconn_connect(swconn, target, target);
+}
+free(target);
+} else {
+rconn_disconnect(swconn);
}
-free(target);
rconn_run(swconn);
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index e3c745a..d7ab7a3 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -108,7 +108,6 @@ get_core_config(struct controller_ctx *ctx, char
**br_int_namep,
exit(EXIT_FAILURE);
}
-const struct ovsrec_bridge *br_int;
const char *remote, *system_id, *br_int_name;
br_int_name = smap_get(&cfg->external_ids, "ovn-bridge");
@@ -116,13 +115,6 @@ get_core_config(struct controller_ctx *ctx, char
**br_int_namep,
br_int_name = DEFAULT_BRIDGE_NAME;
}
-br_int = get_bridge(ctx, br_int_name);
-if (!br_int) {
-VLOG_INFO("Integration bridge '%s' does not exist. Waiting...",
- br_int_name);
-goto try_again;
-}
-
remote = smap_get(&cfg->external_ids, "ovn-remote");
if (!remote) {
VLOG_INFO("OVN OVSDB remote not specified. Waiting...");
@@ -280,24 +272,19 @@ 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);
-/* xxx If run into any surprising changes, we exit. We should
- * xxx handle this more gracefully. */
const struct ovsrec_bridge *br_int = get_bridge(&ctx, br_int_name);
-if (!br_int) {
-VLOG_ERR("Integration bridge '%s' disappeared", br_int_name);
-retval = EXIT_FAILURE;
-goto exit;
-}
chassis_run(&ctx, chassis_id);
encaps_run(&ctx, br_int, chassis_id);
binding_run(&ctx, br_int, chassis_id);
-struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
-pipeline_run(&ctx, &flow_table);
-physical_run(&ctx, br_int, chassis_id, &flow_table);
-ofctrl_run(br_int, &flow_table);
-hmap_destroy(&flow_table);
+if (br_int) {
+struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
+pipeline_run(&ctx, &flow_table);
+physical_run(&ctx, br_int, chassis_id, &flow_table);
+ofctrl_run(br_int, &flow_table);
+hmap_destroy(&flow_table);
+}
unixctl_server_run(unixctl);
@@ -309,7 +296,9 @@ main(int argc, char *argv[])
idl_loop_commit_and_wait(&ovnsb_idl_loop);
idl_loop_commit_and_wait(&ovs_idl_loop);
-ofctrl_wait();
+if (br_int) {
+ofctrl_wait();
+}
poll_block();
}
@@ -319,14 +308,7 @@ main(int argc, char *argv[])
ctx.ovnsb_idl_txn = idl_loop_run(&ovnsb_idl_loop);
ctx.ovs_idl_txn = idl_loop_run(&
Re: [ovs-dev] [PATCH v2 04/21] ovn-controller: Explicitly pass the flow table from function to function.
On 07/28/2015 11:44 AM, Ben Pfaff wrote: > As I was working in ovn-controller, I found it hard to tell what code > produced and what code consumed the OpenFlow flow table, because it was > all implicit. This commit makes the data structure an explicit variable > in the main loop, which makes it easier to see. > > Signed-off-by: Ben Pfaff I acked v1 of this, but it may have gotten lost. Acked-by: Russell Bryant -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Inter-op openvswitch and openfcoe
On Tue, Jul 28, 2015 at 8:26 AM, Anoob Soman wrote: > On 27/07/15 22:53, Jesse Gross wrote: >> On Mon, Jul 27, 2015 at 9:59 AM, Anoob Soman >> wrote: >> #2: Add some mechanism where these daemons can talk to OVS and have >> them cooperate to get the appropriate packets, while still keeping the >> logic in the original daemon. > > Does it mean that daemon would be like openflow controllers and can use > packet_[in/out] to receive/send daemon specific traffic ? That would certainly be the least invasive to OVS although it might still have the integration problems that I listed for #3. I suppose a new interface is possible for this type of operation but I'm not sure that it would be all that desirable. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH V2 2/4] netdev-linux: Make netdev_linux_notify_sock join RTNLGRP_IPV4_IFADDR and RTNLGRP_IPV6_IFADDR.
Thx for the review, adopt the suggestion,
applied to branch2.3, 2.4, master~
On Tue, Jul 28, 2015 at 8:06 AM, Ben Pfaff wrote:
> On Mon, Jul 27, 2015 at 11:56:28PM -0700, Alex Wang wrote:
> > Currently the netdev_linux_notify_sock only joins multicast group
> > RTNLGRP_LINK for link status change notification. Some ovs features
> > also require the detection of ip addresses changes and update of the
> > netdev-linux's cache. To achieve this, we need to make
> > netdev_linux_notify_sock join the multicast group RTNLGRP_IPV4_IFADDR
> > and RTNLGRP_IPV6_IFADDR.
> >
> > Signed-off-by: Alex Wang
> > ---
> > PATCH->V2:
> > - rebase on top of rtnetlink module extension.
>
> ...
>
> > +} else if (rtnetlink_type_is_rtnlgrp_addr(change->nlmsg_type)) {
> > +/* Invalidates in4, in6. */
> > +netdev_linux_changed(dev, dev->ifi_flags,
> > + ~VALID_IN4 ^ VALID_IN6);
>
> This would be more conventionally written ~(VALID_IN4 | VALID_IN6).
>
> Acked-by: Ben Pfaff
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] datapath-windows: Fixed spelling errors in OVS
Ben, could you also apply this patch to the 2.4 branch? Thanks, Sorin -Original Message- From: Ben Pfaff [mailto:b...@nicira.com] Sent: Thursday, 16 July, 2015 21:43 To: Sorin Vinturis Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v2] datapath-windows: Fixed spelling errors in OVS On Wed, Jul 15, 2015 at 08:30:09AM +, Sorin Vinturis wrote: > Solved some spelling errors observed in the datapath code. > > Signed-off-by: Sorin Vinturis > --- > v2: rebased the patch. Applied, thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] datapath-windows: Fixed spelling errors in OVS
OK, done. On Tue, Jul 28, 2015 at 04:58:23PM +, Sorin Vinturis wrote: > Ben, could you also apply this patch to the 2.4 branch? > > Thanks, > Sorin > > -Original Message- > From: Ben Pfaff [mailto:b...@nicira.com] > Sent: Thursday, 16 July, 2015 21:43 > To: Sorin Vinturis > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v2] datapath-windows: Fixed spelling errors in > OVS > > On Wed, Jul 15, 2015 at 08:30:09AM +, Sorin Vinturis wrote: > > Solved some spelling errors observed in the datapath code. > > > > Signed-off-by: Sorin Vinturis > > --- > > v2: rebased the patch. > > Applied, thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH V2 2/4] netdev-linux: Make netdev_linux_notify_sock join RTNLGRP_IPV4_IFADDR and RTNLGRP_IPV6_IFADDR.
On Tue, Jul 28, 2015 at 9:30 AM, Alex Wang wrote:
> Thx for the review, adopt the suggestion,
>
> applied to branch2.3, 2.4, master~
>
When backporting to branch-2.3, found few conflicts... also, considering
that, get_in4,
is only used for vlan-splinter check... we should fix it the simple way...
will send out another
simple fix for branch-2.3,
>
> On Tue, Jul 28, 2015 at 8:06 AM, Ben Pfaff wrote:
>
>> On Mon, Jul 27, 2015 at 11:56:28PM -0700, Alex Wang wrote:
>> > Currently the netdev_linux_notify_sock only joins multicast group
>> > RTNLGRP_LINK for link status change notification. Some ovs features
>> > also require the detection of ip addresses changes and update of the
>> > netdev-linux's cache. To achieve this, we need to make
>> > netdev_linux_notify_sock join the multicast group RTNLGRP_IPV4_IFADDR
>> > and RTNLGRP_IPV6_IFADDR.
>> >
>> > Signed-off-by: Alex Wang
>> > ---
>> > PATCH->V2:
>> > - rebase on top of rtnetlink module extension.
>>
>> ...
>>
>> > +} else if (rtnetlink_type_is_rtnlgrp_addr(change->nlmsg_type)) {
>> > +/* Invalidates in4, in6. */
>> > +netdev_linux_changed(dev, dev->ifi_flags,
>> > + ~VALID_IN4 ^ VALID_IN6);
>>
>> This would be more conventionally written ~(VALID_IN4 | VALID_IN6).
>>
>> Acked-by: Ben Pfaff
>>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 04/21] ovn-controller: Explicitly pass the flow table from function to function.
On Tue, Jul 28, 2015 at 11:50:07AM -0400, Russell Bryant wrote: > On 07/28/2015 11:44 AM, Ben Pfaff wrote: > > As I was working in ovn-controller, I found it hard to tell what code > > produced and what code consumed the OpenFlow flow table, because it was > > all implicit. This commit makes the data structure an explicit variable > > in the main loop, which makes it easier to see. > > > > Signed-off-by: Ben Pfaff > > I acked v1 of this, but it may have gotten lost. > > Acked-by: Russell Bryant Sorry I missed it, it's there now. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/3] rtbsd: support RTM_IFANNOUNCE messages
On Tue, Jul 28, 2015 at 09:48:56AM -0300, Thadeu Lima de Souza Cascardo wrote: > On Mon, Jul 27, 2015 at 03:46:28PM -0700, Ben Pfaff wrote: > > On Mon, Jul 27, 2015 at 02:24:19PM -0300, Thadeu Lima de Souza Cascardo > > wrote: > > > When devices are created, they are announced using RTM_IFANNOUNCE > > > messages using > > > PF_ROUTE. > > > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > > > Do you know anyone who might be qualified to properly review this? It > > seems reasonable but I don't know this part of BSD at all. > > Maybe one of the guys who contributed some of the specific BSD code: > > Ed Maste contributed lib/route-table-bsd.c, and > Giuseppe Lettieri contributed lib/rtbsd.c. > > I am cc'ing them on this message, and bouncing the patch to them as well. > > What about the style? Is it OK to read using struct if_msghdr, then cast to > struct if_announcemsghdr? From my tests, this works fine, the RAW socket seems > to work like a DGRAM socket in this particular case. Doesn't the compiler complain about that without a cast? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [bracnh-2.3] vlan-splinter: Do not check for ipv6 address.
When ovs cleans up unused vlan sub interfaces created by vlan splinter
feature, it will not touch interface that has ip address assigned.
However, the get_in6() function implemented in netdev-linux module
is buggy and always returns 0 (found ipv6 address). This causes the
vlan splinter created sub interfaces never being removed.
Since vlan splinter check is the only user of get_in6 function and
there is no known use case of vlan interface with ipv6 address, we can
simply remove the call of get_in6() function in vlan splinter check.
VMware-BZ: #1485521
Reported-by: Ronald Lee
Signed-off-by: Alex Wang
---
vswitchd/bridge.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 6cd30b8..5b13679 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -4155,8 +4155,7 @@ collect_splinter_vlans(const struct ovsrec_open_vswitch
*ovs_cfg)
struct netdev *netdev;
if (!netdev_open(vlan_dev->name, "system", &netdev)) {
-if (!netdev_get_in4(netdev, NULL, NULL) ||
-!netdev_get_in6(netdev, NULL)) {
+if (!netdev_get_in4(netdev, NULL, NULL)) {
/* It has an IP address configured, so we don't own
* it. Don't delete it. */
} else {
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [bracnh-2.3] vlan-splinter: Do not check for ipv6 address.
On Tue, Jul 28, 2015 at 10:32:27AM -0700, Alex Wang wrote: > When ovs cleans up unused vlan sub interfaces created by vlan splinter > feature, it will not touch interface that has ip address assigned. > However, the get_in6() function implemented in netdev-linux module > is buggy and always returns 0 (found ipv6 address). This causes the > vlan splinter created sub interfaces never being removed. > > Since vlan splinter check is the only user of get_in6 function and > there is no known use case of vlan interface with ipv6 address, we can > simply remove the call of get_in6() function in vlan splinter check. > > VMware-BZ: #1485521 > Reported-by: Ronald Lee > Signed-off-by: Alex Wang Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/3] rtbsd: support RTM_IFANNOUNCE messages
On Tue, Jul 28, 2015 at 10:22:47AM -0700, Ben Pfaff wrote: > On Tue, Jul 28, 2015 at 09:48:56AM -0300, Thadeu Lima de Souza Cascardo wrote: > > On Mon, Jul 27, 2015 at 03:46:28PM -0700, Ben Pfaff wrote: > > > On Mon, Jul 27, 2015 at 02:24:19PM -0300, Thadeu Lima de Souza Cascardo > > > wrote: > > > > When devices are created, they are announced using RTM_IFANNOUNCE > > > > messages using > > > > PF_ROUTE. > > > > > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > > > > > Do you know anyone who might be qualified to properly review this? It > > > seems reasonable but I don't know this part of BSD at all. > > > > Maybe one of the guys who contributed some of the specific BSD code: > > > > Ed Maste contributed lib/route-table-bsd.c, and > > Giuseppe Lettieri contributed lib/rtbsd.c. > > > > I am cc'ing them on this message, and bouncing the patch to them as well. > > > > What about the style? Is it OK to read using struct if_msghdr, then cast to > > struct if_announcemsghdr? From my tests, this works fine, the RAW socket > > seems > > to work like a DGRAM socket in this particular case. > > Doesn't the compiler complain about that without a cast? Good point. I thought there was a cast already there. Fixed. I was pointing out to the assumption that one message is larger than the other. I don't think this is a problem in this particular case. if_msghdr has a struct at the end, which stores a lot of stats, and all the other earlier members of both if_msghdr and if_announcemsghdr are of types that don't have different sizes on different supported architectures, AFAIK. I know almost nothing about *BSD. Cascardo. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/3] rtbsd: support RTM_IFANNOUNCE messages
On Tue, Jul 28, 2015 at 02:43:52PM -0300, Thadeu Lima de Souza Cascardo wrote: > On Tue, Jul 28, 2015 at 10:22:47AM -0700, Ben Pfaff wrote: > > On Tue, Jul 28, 2015 at 09:48:56AM -0300, Thadeu Lima de Souza Cascardo > > wrote: > > > On Mon, Jul 27, 2015 at 03:46:28PM -0700, Ben Pfaff wrote: > > > > On Mon, Jul 27, 2015 at 02:24:19PM -0300, Thadeu Lima de Souza Cascardo > > > > wrote: > > > > > When devices are created, they are announced using RTM_IFANNOUNCE > > > > > messages using > > > > > PF_ROUTE. > > > > > > > > > > Signed-off-by: Thadeu Lima de Souza Cascardo > > > > > > > > Do you know anyone who might be qualified to properly review this? It > > > > seems reasonable but I don't know this part of BSD at all. > > > > > > Maybe one of the guys who contributed some of the specific BSD code: > > > > > > Ed Maste contributed lib/route-table-bsd.c, and > > > Giuseppe Lettieri contributed lib/rtbsd.c. > > > > > > I am cc'ing them on this message, and bouncing the patch to them as well. > > > > > > What about the style? Is it OK to read using struct if_msghdr, then cast > > > to > > > struct if_announcemsghdr? From my tests, this works fine, the RAW socket > > > seems > > > to work like a DGRAM socket in this particular case. > > > > Doesn't the compiler complain about that without a cast? > > Good point. I thought there was a cast already there. Fixed. I was pointing > out > to the assumption that one message is larger than the other. I don't think > this > is a problem in this particular case. if_msghdr has a struct at the end, which > stores a lot of stats, and all the other earlier members of both if_msghdr and > if_announcemsghdr are of types that don't have different sizes on different > supported architectures, AFAIK. I know almost nothing about *BSD. This is a kind of undefined behavior that one can normally get away with. Let's give Ed and Giuseppe a day or two to take a look, and if we don't hear back from them then I'll consider this good enough. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 01/21] ovn-controller: Fix potential use-after-free in get_core_config().
On Tue, Jul 28, 2015 at 8:44 AM, Ben Pfaff wrote: > > Signed-off-by: Ben Pfaff > Acked-by: Russell Bryant > --- > v1->v2: Call ovsdb_idl_run() before instead of after accessing IDL. > --- > ovn/controller/ovn-controller.c | 19 +-- > 1 file changed, 9 insertions(+), 10 deletions(-) > > diff --git a/ovn/controller/ovn-controller.c > b/ovn/controller/ovn-controller.c > index fda1534..bd3ef0d 100644 > --- a/ovn/controller/ovn-controller.c > +++ b/ovn/controller/ovn-controller.c > @@ -95,21 +95,20 @@ get_bridge(struct controller_ctx *ctx, const char > *name) > static void > get_core_config(struct controller_ctx *ctx) Thx, looks good to me~ ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 01/21] ovn-controller: Fix potential use-after-free in get_core_config().
On Tue, Jul 28, 2015 at 10:54:30AM -0700, Alex Wang wrote: > On Tue, Jul 28, 2015 at 8:44 AM, Ben Pfaff wrote: > > > > > Signed-off-by: Ben Pfaff > > Acked-by: Russell Bryant > > --- > > v1->v2: Call ovsdb_idl_run() before instead of after accessing IDL. > > --- > > ovn/controller/ovn-controller.c | 19 +-- > > 1 file changed, 9 insertions(+), 10 deletions(-) > > > > diff --git a/ovn/controller/ovn-controller.c > > b/ovn/controller/ovn-controller.c > > index fda1534..bd3ef0d 100644 > > --- a/ovn/controller/ovn-controller.c > > +++ b/ovn/controller/ovn-controller.c > > @@ -95,21 +95,20 @@ get_bridge(struct controller_ctx *ctx, const char > > *name) > > static void > > get_core_config(struct controller_ctx *ctx) > > > Thx, looks good to me~ Thanks. I applied patches 1-5 to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] dpif-netdev: fix race for queues between pmd threads
Acked-by: Daniele Di Proietto
Thanks!
On 28/07/2015 07:55, "Ilya Maximets" wrote:
>Currently pmd threads select queues in pmd_load_queues() according to
>get_n_pmd_threads_on_numa(). This behavior leads to race between pmds,
>beacause dp_netdev_set_pmds_on_numa() starts them one by one and
>current number of threads changes incrementally.
>
>As a result we may have the following situation with 2 pmd threads:
>
>* dp_netdev_set_pmds_on_numa()
>* pmd12 thread started. Currently only 1 pmd thread exists.
>dpif_netdev(pmd12)|INFO|Core 1 processing port 'port_1'
>dpif_netdev(pmd12)|INFO|Core 1 processing port 'port_2'
>* pmd14 thread started. 2 pmd threads exists.
>dpif_netdev|INFO|Created 2 pmd threads on numa node 0
>dpif_netdev(pmd14)|INFO|Core 2 processing port 'port_2'
>
>We have:
>core 1 --> port 1, port 2
>core 2 --> port 2
>
>Fix this by starting pmd threads only after all of them have
>been configured.
>
>Cc: Daniele Di Proietto
>Cc: Dyasly Sergey
>Signed-off-by: Ilya Maximets
>---
> lib/dpif-netdev.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index 79c4612..83e55e7 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -2961,6 +2961,7 @@ dp_netdev_set_pmds_on_numa(struct dp_netdev *dp,
>int numa_id)
> * pmd threads for the numa node. */
> if (!n_pmds) {
> int can_have, n_unpinned, i;
>+struct dp_netdev_pmd_thread **pmds;
>
> n_unpinned = ovs_numa_get_n_unpinned_cores_on_numa(numa_id);
> if (!n_unpinned) {
>@@ -2972,15 +2973,22 @@ dp_netdev_set_pmds_on_numa(struct dp_netdev *dp,
>int numa_id)
> /* If cpu mask is specified, uses all unpinned cores, otherwise
> * tries creating NR_PMD_THREADS pmd threads. */
> can_have = dp->pmd_cmask ? n_unpinned : MIN(n_unpinned,
>NR_PMD_THREADS);
>+pmds = xzalloc(can_have * sizeof *pmds);
> for (i = 0; i < can_have; i++) {
>-struct dp_netdev_pmd_thread *pmd = xzalloc(sizeof *pmd);
> unsigned core_id =
>ovs_numa_get_unpinned_core_on_numa(numa_id);
>-
>-dp_netdev_configure_pmd(pmd, dp, i, core_id, numa_id);
>+pmds[i] = xzalloc(sizeof **pmds);
>+dp_netdev_configure_pmd(pmds[i], dp, i, core_id, numa_id);
>+}
>+/* The pmd thread code needs to see all the others configured pmd
>+ * threads on the same numa node. That's why we call
>+ * 'dp_netdev_configure_pmd()' on all the threads and then we
>actually
>+ * start them. */
>+for (i = 0; i < can_have; i++) {
> /* Each thread will distribute all devices rx-queues among
> * themselves. */
>-pmd->thread = ovs_thread_create("pmd", pmd_thread_main, pmd);
>+pmds[i]->thread = ovs_thread_create("pmd", pmd_thread_main,
>pmds[i]);
> }
>+free(pmds);
> VLOG_INFO("Created %d pmd threads on numa node %d", can_have,
>numa_id);
> }
> }
>--
>2.1.4
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 01/10] lib: Add smap_equal().
On Tue, Jul 21, 2015 at 12:59:11PM -0400, Russell Bryant wrote:
> Add a method to determine of two smaps are equal (have the exact same
> set of key-value pairs).
>
> Signed-off-by: Russell Bryant
Can't this be done in O(n) instead of O(n log n)?
bool
smap_equal(const struct smap *smap1, const struct smap *smap2)
{
if (smap_count(smap1) != smap_count(smap2)) {
return false;
}
const struct smap_node *node;
SMAP_FOR_EACH (node, smap1) {
const char *value2 = smap_get(smap2, node->key);
if (!value2 || strcmp(node->value, value2)) {
return false;
}
}
return true;
}
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ofctrl: Fix use of uninitialized hash value in ofctrl_add_flow().
When ofctrl_add_flow() called ovn_flow_lookup(), the latter used the hash
from the flow's hmap_node, but the former hadn't initialized it at that
point. This commit fixes the problem.
Reported-by: Russell Bryant
Reported-at: http://openvswitch.org/pipermail/dev/2015-July/057851.html
Signed-off-by: Ben Pfaff
---
ovn/controller/ofctrl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index c548645..e701f8b 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -268,6 +268,7 @@ ofctrl_add_flow(struct hmap *desired_flows,
f->match = *match;
f->ofpacts = xmemdup(actions->data, actions->size);
f->ofpacts_len = actions->size;
+f->hmap_node.hash = ovn_flow_hash(f);
if (ovn_flow_lookup(desired_flows, f)) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
@@ -281,7 +282,7 @@ ofctrl_add_flow(struct hmap *desired_flows,
return;
}
-hmap_insert(desired_flows, &f->hmap_node, ovn_flow_hash(f));
+hmap_insert(desired_flows, &f->hmap_node, f->hmap_node.hash);
}
/* ovn_flow. */
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 02/10] ovn: Fix uninit access warning from valgrind.
On Tue, Jul 21, 2015 at 12:59:12PM -0400, Russell Bryant wrote: > While running ovn-controller under valgrind, I noticed several > variations of the same warning. Switching this allocation from malloc > to zalloc resolves it. > > For reference, some examples from valgrind are: > > ==8487== Conditional jump or move depends on uninitialised value(s) > ==8487==at 0x408935: hmap_next_with_hash__ (hmap.h:275) > ==8487==by 0x4088BF: hmap_first_with_hash (hmap.h:286) > ==8487==by 0x408357: ovn_flow_lookup (ofctrl.c:307) > ==8487==by 0x408259: ofctrl_add_flow (ofctrl.c:274) > ==8487==by 0x40B00F: pipeline_run (pipeline.c:330) > ==8487==by 0x408F65: main (ovn-controller.c:397) > ==8487== > ==8487== Use of uninitialised value of size 8 > ==8487==at 0x4088B3: hmap_first_with_hash (hmap.h:286) > ==8487==by 0x408357: ovn_flow_lookup (ofctrl.c:307) > ==8487==by 0x408259: ofctrl_add_flow (ofctrl.c:274) > ==8487==by 0x40B00F: pipeline_run (pipeline.c:330) > ==8487==by 0x408F65: main (ovn-controller.c:397) > > Signed-off-by: Russell Bryant I fixed this in my tunnel key series but I forgot to properly note it or to break it out as a separate patch. I've now posted it independently: http://openvswitch.org/pipermail/dev/2015-July/058172.html ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-dpdk: add support for rings in secondary processes in IVSHMEM setups
This is interesting, thanks for the patch.
I'm definitely not a IVSHMEM expert, but I have a concern: what
happens if the secondary OVS process allocates or frees some
mbufs? (e.g because a packet is sent to multiple destinations or is
dropped)
I found this in the DPDK documentation:
http://dpdk.org/doc/guides/prog_guide/ivshmem_lib.html#best-practices-for-w
riting-ivshmem-applications\
I'd appreciate feedback from someone with more IVSHMEM experience
than me (Maryam, perhaps?)
Thanks,
Daniele
On 22/07/2015 21:51, "Melvin Walls" wrote:
>In order for OVS running inside a VM using IVSHMEM to recognize ports
>created
>on the host, you have to start vswitchd with the --proc-type=secondary EAL
>option.
>
>When creating rings in secondary processes functions like
>rte_eth_dev_configure() fail with the error code E_RTE_SECONDARY, i.e.,
>the
>operations are not allowed in secondary processes. Avoiding this requires
>some
>changes to the way secondary processes handle dpdk rings.
>
>This patch changes dpdk_ring_create() to use rte_ring_lookup() instead of
>rte_ring_create() when called from a secondary process. It also
>introduces two
>functions: netdev_dpdk_ring_rxq_recv() and netdev_dpdk_ring_send__() to
>handle
>tx/rx on dpdk rings in secondary processes.
>
>Signed-off-by: Melvin Walls
>Signed-off-by: Ethan Jackson
>---
> lib/netdev-dpdk.c | 158
>+-
> 1 file changed, 122 insertions(+), 36 deletions(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 5ae805e..5abe90f 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -227,6 +227,10 @@ struct netdev_dpdk {
> /* Identifier used to distinguish vhost devices from each other */
> char vhost_id[PATH_MAX];
>
>+/* Rings for secondary processes in IVSHMEM setups, NULL otherwise */
>+struct rte_ring *rx_ring;
>+struct rte_ring *tx_ring;
>+
> /* In dpdk_list. */
> struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
> };
>@@ -340,12 +344,16 @@ dpdk_mp_get(int socket_id, int mtu)
>OVS_REQUIRES(dpdk_mutex)
> return NULL;
> }
>
>-dmp->mp = rte_mempool_create(mp_name, mp_size, MBUF_SIZE(mtu),
>- MP_CACHE_SZ,
>- sizeof(struct
>rte_pktmbuf_pool_private),
>- rte_pktmbuf_pool_init, NULL,
>- ovs_rte_pktmbuf_init, NULL,
>- socket_id, 0);
>+if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
>+dmp->mp = rte_mempool_create(mp_name, mp_size,
>MBUF_SIZE(mtu),
>+ MP_CACHE_SZ,
>+ sizeof(struct
>rte_pktmbuf_pool_private),
>+ rte_pktmbuf_pool_init, NULL,
>+ ovs_rte_pktmbuf_init, NULL,
>+ socket_id, 0);
>+} else {
>+dmp->mp = rte_mempool_lookup(mp_name);
>+}
> } while (!dmp->mp && rte_errno == ENOMEM && (mp_size /= 2) >=
>MIN_NB_MBUF);
>
> if (dmp->mp == NULL) {
>@@ -439,39 +447,41 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
>OVS_REQUIRES(dpdk_mutex)
> dev->up.n_rxq = MIN(info.max_rx_queues, dev->up.n_rxq);
> dev->real_n_txq = MIN(info.max_tx_queues, dev->up.n_txq);
>
>-diag = rte_eth_dev_configure(dev->port_id, dev->up.n_rxq,
>dev->real_n_txq,
>- &port_conf);
>-if (diag) {
>-VLOG_ERR("eth dev config error %d. rxq:%d txq:%d", diag,
>dev->up.n_rxq,
>- dev->real_n_txq);
>-return -diag;
>-}
>-
>-for (i = 0; i < dev->real_n_txq; i++) {
>-diag = rte_eth_tx_queue_setup(dev->port_id, i,
>NIC_PORT_TX_Q_SIZE,
>- dev->socket_id, NULL);
>+if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
>+diag = rte_eth_dev_configure(dev->port_id, dev->up.n_rxq,
>dev->real_n_txq,
>+ &port_conf);
> if (diag) {
>-VLOG_ERR("eth dev tx queue setup error %d",diag);
>+VLOG_ERR("eth dev config error %d. rxq:%d txq:%d", diag,
>dev->up.n_rxq,
>+ dev->real_n_txq);
> return -diag;
> }
>-}
>
>-for (i = 0; i < dev->up.n_rxq; i++) {
>-diag = rte_eth_rx_queue_setup(dev->port_id, i,
>NIC_PORT_RX_Q_SIZE,
>- dev->socket_id,
>- NULL, dev->dpdk_mp->mp);
>+for (i = 0; i < dev->real_n_txq; i++) {
>+diag = rte_eth_tx_queue_setup(dev->port_id, i,
>NIC_PORT_TX_Q_SIZE,
>+ dev->socket_id, NULL);
>+if (diag) {
>+VLOG_ERR("eth dev tx queue setup error %d",diag);
>+return -diag;
>+}
>+}
>+
>+for (i = 0; i < de
[ovs-dev] [PATCH] netdev-dpdk: Increase limit for dpdk ring names
Hi,
Currently, the ovs netdev-dpdk ring type dpdkr, restricts dpdk ring name
size to 10 bytes (includes the _rx/tx suffix).
This only gives us space to accommodate 10 rings as truncation happens
with dpdkr10_rx/tx.
The enclosed patch below ensures that we respect the ring name size
constraints imposed by the rte dpdk ring apis by allowing for a greater
size instead of using a hard-coded ring name size in ovs netdev-dpdk layer.
Regards,
-Karthick
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 5ae805e..00229c2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -92,7 +92,7 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
#define NIC_PORT_RX_Q_SIZE 2048 /* Size of Physical NIC RX Queue, Max
(n+32<=4096)*/
#define NIC_PORT_TX_Q_SIZE 2048 /* Size of Physical NIC TX Queue, Max
(n+32<=4096)*/
-
+#define RING_NAME_MAX (RTE_MEMZONE_NAMESIZE - sizeof(RTE_RING_MZ_PREFIX))
static char *cuse_dev_name = NULL;/* Character device cuse_dev_name. */
static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets */
@@ -1797,7 +1797,7 @@ dpdk_ring_create(const char dev_name[], unsigned int
port_no,
unsigned int *eth_port_id)
{
struct dpdk_ring *ivshmem;
-char ring_name[10];
+char ring_name[RING_NAME_MAX];
int err;
ivshmem = dpdk_rte_mzalloc(sizeof *ivshmem);
@@ -1806,7 +1806,7 @@ dpdk_ring_create(const char dev_name[], unsigned int
port_no,
}
/* XXX: Add support for multiquque ring. */
-err = snprintf(ring_name, 10, "%s_tx", dev_name);
+err = snprintf(ring_name, sizeof(ring_name), "%s_tx", dev_name);
if (err < 0) {
return -err;
}
@@ -1819,7 +1819,7 @@ dpdk_ring_create(const char dev_name[], unsigned int
port_no,
return ENOMEM;
}
-err = snprintf(ring_name, 10, "%s_rx", dev_name);
+err = snprintf(ring_name, sizeof(ring_name), "%s_rx", dev_name);
if (err < 0) {
return -err;
}
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 03/21] ovn-controller: Avoid blocking to commit OVSDB transactions.
On Tue, Jul 28, 2015 at 8:44 AM, Ben Pfaff wrote:
> Until now, ovn-controller has been full of loops that commit a transaction
> to the OVS or OVN Southbound database. These blocking loops delay other
> work within ovn-controller. They also make it unsafe to keep pointers to
> database records within a single ovn-controller main loop, since calls
> to ovsdb_idl_run() can cause IDL records to be destroyed. This commit
> drops all of the blocking calls, instead doing a single commit to the
> databases at the end of each main loop.
>
> Signed-off-by: Ben Pfaff
> Acked-by: Russell Bryant
> ---
> v1->v2: Rename idl_loop_wait() to idl_loop_commit_and_wait().
> ---
> ovn/controller/binding.c| 61
> ovn/controller/binding.h| 4 +-
> ovn/controller/chassis.c| 117
> ++
> ovn/controller/chassis.h| 4 +-
> ovn/controller/ovn-controller.c | 123
> +---
> ovn/controller/ovn-controller.h | 4 ++
> 6 files changed, 188 insertions(+), 125 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index 0a4a39e..6af216c 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -76,10 +76,12 @@ binding_run(struct controller_ctx *ctx)
> {
> const struct sbrec_chassis *chassis_rec;
> const struct sbrec_binding *binding_rec;
> -struct ovsdb_idl_txn *txn;
> struct sset lports, all_lports;
> const char *name;
> -int retval;
> +
> +if (!ctx->ovnsb_idl_txn) {
> +return;
> +}
>
> chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
> if (!chassis_rec) {
> @@ -91,8 +93,7 @@ binding_run(struct controller_ctx *ctx)
> get_local_iface_ids(ctx, &lports);
> sset_clone(&all_lports, &lports);
>
> -txn = ovsdb_idl_txn_create(ctx->ovnsb_idl);
> -ovsdb_idl_txn_add_comment(txn,
> +ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
>"ovn-controller: updating bindings for
> '%s'",
>ctx->chassis_id);
>
> @@ -115,14 +116,6 @@ binding_run(struct controller_ctx *ctx)
> }
> }
>
> -retval = ovsdb_idl_txn_commit_block(txn);
> -if (retval == TXN_ERROR) {
> -VLOG_INFO("Problem committing binding information: %s",
> - ovsdb_idl_txn_status_to_string(retval));
> -}
> -
> -ovsdb_idl_txn_destroy(txn);
> -
> SSET_FOR_EACH (name, &lports) {
> VLOG_DBG("No binding record for lport %s", name);
> }
> @@ -130,40 +123,32 @@ binding_run(struct controller_ctx *ctx)
> sset_destroy(&all_lports);
> }
>
> -void
> -binding_destroy(struct controller_ctx *ctx)
> +/* Returns true if the database is all cleaned up, false if more work is
> + * required. */
> +bool
> +binding_cleanup(struct controller_ctx *ctx)
> {
> -const struct sbrec_chassis *chassis_rec;
> -int retval = TXN_TRY_AGAIN;
> -
> -ovs_assert(ctx->ovnsb_idl);
> +if (!ctx->ovnsb_idl_txn) {
> +return false;
> +}
>
> -chassis_rec = get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
> +const struct sbrec_chassis *chassis_rec
> += get_chassis_by_name(ctx->ovnsb_idl, ctx->chassis_id);
> if (!chassis_rec) {
> -return;
> +return true;
> }
>
> -while (retval != TXN_SUCCESS && retval != TXN_UNCHANGED) {
> -const struct sbrec_binding *binding_rec;
> -struct ovsdb_idl_txn *txn;
> -
> -txn = ovsdb_idl_txn_create(ctx->ovnsb_idl);
> -ovsdb_idl_txn_add_comment(txn,
> +ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
>"ovn-controller: removing all bindings for
> '%s'",
>ctx->chassis_id);
>
> -SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> -if (binding_rec->chassis == chassis_rec) {
> -sbrec_binding_set_chassis(binding_rec, NULL);
> -}
> -}
> -
> -retval = ovsdb_idl_txn_commit_block(txn);
> -if (retval == TXN_ERROR) {
> -VLOG_INFO("Problem removing bindings: %s",
> - ovsdb_idl_txn_status_to_string(retval));
> +const struct sbrec_binding *binding_rec;
> +bool any_changes = false;
> +SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
> +if (binding_rec->chassis == chassis_rec) {
> +sbrec_binding_set_chassis(binding_rec, NULL);
> +any_changes = true;
> }
> -
> -ovsdb_idl_txn_destroy(txn);
> }
> +return !any_changes;
> }
> diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
> index 2611173..e73c1d1 100644
> --- a/ovn/controller/binding.h
> +++ b/ovn/controller/binding.h
> @@ -17,10 +17,12 @@
> #ifndef OVN_BINDING_H
> #define OVN_BINDING_H 1
>
> +#include
> +
> struct controller_ctx;
>
> void binding_init(struct controller_ctx *);
> void binding_run(struct
Re: [ovs-dev] [PATCH] ofctrl: Fix use of uninitialized hash value in ofctrl_add_flow().
On 07/28/2015 02:26 PM, Ben Pfaff wrote: > When ofctrl_add_flow() called ovn_flow_lookup(), the latter used the hash > from the flow's hmap_node, but the former hadn't initialized it at that > point. This commit fixes the problem. > > Reported-by: Russell Bryant > Reported-at: http://openvswitch.org/pipermail/dev/2015-July/057851.html > Signed-off-by: Ben Pfaff Acked-by: Russell Bryant -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 01/10] lib: Add smap_equal().
On 07/28/2015 02:13 PM, Ben Pfaff wrote:
> On Tue, Jul 21, 2015 at 12:59:11PM -0400, Russell Bryant wrote:
>> Add a method to determine of two smaps are equal (have the exact same
>> set of key-value pairs).
>>
>> Signed-off-by: Russell Bryant
>
> Can't this be done in O(n) instead of O(n log n)?
>
> bool
> smap_equal(const struct smap *smap1, const struct smap *smap2)
> {
> if (smap_count(smap1) != smap_count(smap2)) {
> return false;
> }
>
> const struct smap_node *node;
> SMAP_FOR_EACH (node, smap1) {
> const char *value2 = smap_get(smap2, node->key);
> if (!value2 || strcmp(node->value, value2)) {
> return false;
> }
> }
> return true;
> }
>
Yeah, that's better, thanks. I'll revise.
--
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 03/10] ovn: Make column comparisons more generic.
On Tue, Jul 21, 2015 at 12:59:13PM -0400, Russell Bryant wrote: > The logic in ovn-northd's parents_equal() and tags_equal() is useful > for other columns, so convert them into more generic functions that > can be reused. > > Signed-off-by: Russell Bryant Applied to master, thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofctrl: Fix use of uninitialized hash value in ofctrl_add_flow().
On Tue, Jul 28, 2015 at 12:24 PM, Russell Bryant wrote: > On 07/28/2015 02:26 PM, Ben Pfaff wrote: >> When ofctrl_add_flow() called ovn_flow_lookup(), the latter used the hash >> from the flow's hmap_node, but the former hadn't initialized it at that >> point. This commit fixes the problem. >> >> Reported-by: Russell Bryant >> Reported-at: http://openvswitch.org/pipermail/dev/2015-July/057851.html >> Signed-off-by: Ben Pfaff > > Acked-by: Russell Bryant Thanks, applied to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 04/10] ovn: Drop unnecessary br_int local variable.
On Tue, Jul 21, 2015 at 12:59:14PM -0400, Russell Bryant wrote: > When getting the initial configuration, go ahead and store br_int in > the controller context. This is already done in the main loop, but a > future patch wants to use br_int from the controller context sooner in > the code. > > Signed-off-by: Russell Bryant It seems that this one is no longer necessary given the changes from the tunnel-key series. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] (no subject)
___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 04/10] ovn: Drop unnecessary br_int local variable.
On 07/28/2015 03:55 PM, Ben Pfaff wrote: > On Tue, Jul 21, 2015 at 12:59:14PM -0400, Russell Bryant wrote: >> When getting the initial configuration, go ahead and store br_int in >> the controller context. This is already done in the main loop, but a >> future patch wants to use br_int from the controller context sooner in >> the code. >> >> Signed-off-by: Russell Bryant > > It seems that this one is no longer necessary given the changes from the > tunnel-key series. > Thanks, I'll drop it in the next rebase. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 06/10] ovn: Add patch ports for ovn bridge mappings.
On Tue, Jul 21, 2015 at 12:59:16PM -0400, Russell Bryant wrote: > While parsing the OVN bridge mapping configuration, ensure that patch > ports exist between the OVN integration bridge and the physical > network bridge. If they do not exist, create them automatically. > > Signed-off-by: Russell Bryant This looks like a good idea but I have a couple of comments. First, probably this should be adapted so that it's not doing a blocking commit (since we've eliminated the others now). Second, besides adding patch ports, I'd expect that it should remove patch ports (that it created) when the configuration changes so that they are no longer needed. (Probably it should distinguish the ones that ovn-controller owns by adding an external-id.) ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 05/10] ovn: Add bridge mappings to ovn-controller.
On 07/28/2015 04:02 PM, Ben Pfaff wrote: > On Tue, Jul 21, 2015 at 12:59:15PM -0400, Russell Bryant wrote: >> Add a new OVN configuration entry in the Open_vSwitch database called >> "ovn-bridge-mappings". This allows the configuration of mappings >> between a physical network name and an OVS bridge that provides >> connectivity to that network. >> >> For example, if you wanted to configure "physnet1" to map to "br-eth0" >> and "physnet2" to map to "br-eth1", the configuration would be: >> >> $ ovs-vsctl set open . \ >> > external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1 >> >> In this patch, the configuration option is only parsed and validated >> to make sure the referenced bridges actually exist. Later patches >> will make use of the bridge mappings. >> >> Documentation for this configuration value is introduced in a later >> patch that makes use of this by introducing a "localnet" logical port >> type. >> >> Signed-off-by: Russell Bryant > > This seems reasonable but one of the things I'm trying to do with my > tunnel key series is to eliminate the multiple phases of startup in > ovn-controller, where first it gets the configuration and then later > uses the configuration. Do you think it would be possible to use that > approach here too? > Thanks, that's good feedback. I did this before I saw your series. I'll rework this on top of the stuff you've done. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 06/10] ovn: Add patch ports for ovn bridge mappings.
On 07/28/2015 04:06 PM, Ben Pfaff wrote: > On Tue, Jul 21, 2015 at 12:59:16PM -0400, Russell Bryant wrote: >> While parsing the OVN bridge mapping configuration, ensure that patch >> ports exist between the OVN integration bridge and the physical >> network bridge. If they do not exist, create them automatically. >> >> Signed-off-by: Russell Bryant > > This looks like a good idea but I have a couple of comments. First, > probably this should be adapted so that it's not doing a blocking commit > (since we've eliminated the others now). Second, besides adding patch > ports, I'd expect that it should remove patch ports (that it created) > when the configuration changes so that they are no longer needed. > (Probably it should distinguish the ones that ovn-controller owns by > adding an external-id.) > Both suggestions make sense to me. I'll incorporate them. Thanks! -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 08/10] ovn: Add type and options to logical port.
On Tue, Jul 21, 2015 at 12:59:18PM -0400, Russell Bryant wrote: > We have started discussing the use of the logical port abstraction in > OVN to represent special types of connections into an OVN logical > switch. This patch proposes some schema updates to reflect these > special types of logical ports. A logical port can have a "type" and > a set of options specific to that type. > > Some examples of logical port types would be "vtep" for connectivity > to a VTEP gateway or "local" for a connection to a locally accessible > network via an ovs bridge. Actualy support for these (or other) types > will come in later patches. > > Signed-off-by: Russell Bryant Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 05/10] ovn: Add bridge mappings to ovn-controller.
On Tue, Jul 21, 2015 at 12:59:15PM -0400, Russell Bryant wrote: > Add a new OVN configuration entry in the Open_vSwitch database called > "ovn-bridge-mappings". This allows the configuration of mappings > between a physical network name and an OVS bridge that provides > connectivity to that network. > > For example, if you wanted to configure "physnet1" to map to "br-eth0" > and "physnet2" to map to "br-eth1", the configuration would be: > > $ ovs-vsctl set open . \ > > external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1 > > In this patch, the configuration option is only parsed and validated > to make sure the referenced bridges actually exist. Later patches > will make use of the bridge mappings. > > Documentation for this configuration value is introduced in a later > patch that makes use of this by introducing a "localnet" logical port > type. > > Signed-off-by: Russell Bryant This seems reasonable but one of the things I'm trying to do with my tunnel key series is to eliminate the multiple phases of startup in ovn-controller, where first it gets the configuration and then later uses the configuration. Do you think it would be possible to use that approach here too? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 09/10] ovn: Get/set lport type and options in ovn-nbctl.
On Tue, Jul 21, 2015 at 12:59:19PM -0400, Russell Bryant wrote: > A recent patch added "type" and "options" columns to the Logical_Port > table in OVN_Northbound. This patch allows you to get and set those > columns with ovn-nbctl. > > Signed-off-by: Russell Bryant This seems reasonable, although we could also just add support for the "set" database command (I think Alex has a patch to do that?), e.g. ovn-nbctl set Logical_Port portname type=asdf options=... ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 10/10] ovn: Add "localnet" logical port type.
On Tue, Jul 21, 2015 at 12:59:20PM -0400, Russell Bryant wrote: > Introduce a new logical port type called "localnet". A logical port > with this type also has an option called "network_name". A "localnet" > logical port represents a connection to a locally accessible network. > ovn-controller will use the ovn-bridge-mappings configuration to > figure out which patch port on br-int should be used for this port. > > Signed-off-by: Russell Bryant I'm glad to see this. Thanks a lot! I'll do a detailed review when v4 comes around. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 09/10] ovn: Get/set lport type and options in ovn-nbctl.
On 07/28/2015 04:12 PM, Ben Pfaff wrote: > On Tue, Jul 21, 2015 at 12:59:19PM -0400, Russell Bryant wrote: >> A recent patch added "type" and "options" columns to the Logical_Port >> table in OVN_Northbound. This patch allows you to get and set those >> columns with ovn-nbctl. >> >> Signed-off-by: Russell Bryant > > This seems reasonable, although we could also just add support for the > "set" database command (I think Alex has a patch to do that?), e.g. > ovn-nbctl set Logical_Port portname type=asdf options=... > Ah yes, that'd be fine too. Alex, have you already looked at doing this in ovn-nbctl? If not I'll take a look. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovn-controller: Avoid overlooking changes that occur during commit.
A commit to the database takes multiple trips through the main loop. In
that time, the database could change, but ovn-controller can't properly
react to the changes until the commit has succeeded or failed. Since
commit f1fd765733 (ovn-controller: Avoid blocking to commit OVSDB
transactions), Open vSwitch has failed to properly re-check the contents
of the database following a successful commit. That meant that it was
possible for ovn-controller to fail to react to a database change until
much later, if nothing else happened for some time.
Reported-by; Alex Wang
Reported-at: http://openvswitch.org/pipermail/dev/2015-July/058176.html
Signed-off-by: Ben Pfaff
---
ovn/controller/ovn-controller.c | 48 +
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 0657140..affc14b 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -189,28 +189,40 @@ idl_loop_commit_and_wait(struct idl_loop *loop)
struct ovsdb_idl_txn *txn = loop->committing_txn;
if (txn) {
enum ovsdb_idl_txn_status status = ovsdb_idl_txn_commit(txn);
-switch (status) {
-case TXN_INCOMPLETE:
-break;
+if (status != TXN_INCOMPLETE) {
+switch (status) {
+case TXN_TRY_AGAIN:
+/* We want to re-evaluate the database when it's changed from
+ * the contents that it had when we started the commit. (That
+ * might have already happened.) */
+loop->skip_seqno = loop->precommit_seqno;
+if (ovsdb_idl_get_seqno(loop->idl) != loop->skip_seqno) {
+poll_immediate_wake();
+}
+break;
+
+case TXN_SUCCESS:
+/* If the database has already changed since we started the
+ * commit, re-evaluate it immediately to avoid missing a change
+ * for a while. */
+if (ovsdb_idl_get_seqno(loop->idl) != loop->precommit_seqno) {
+poll_immediate_wake();
+}
+break;
+
+case TXN_UNCHANGED:
+case TXN_ABORTED:
+case TXN_NOT_LOCKED:
+case TXN_ERROR:
+break;
+
+case TXN_UNCOMMITTED:
+case TXN_INCOMPLETE:
+OVS_NOT_REACHED();
-case TXN_TRY_AGAIN:
-loop->skip_seqno = loop->precommit_seqno;
-if (ovsdb_idl_get_seqno(loop->idl) != loop->skip_seqno) {
-poll_immediate_wake();
}
-/* Fall through. */
-case TXN_UNCHANGED:
-case TXN_ABORTED:
-case TXN_SUCCESS:
-case TXN_NOT_LOCKED:
-case TXN_ERROR:
ovsdb_idl_txn_destroy(txn);
loop->committing_txn = NULL;
-break;
-
-case TXN_UNCOMMITTED:
-OVS_NOT_REACHED();
-
}
}
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 03/21] ovn-controller: Avoid blocking to commit OVSDB transactions.
On Tue, Jul 28, 2015 at 11:56:05AM -0700, Alex Wang wrote: > Curious here, if we could run into the following race (in main function): > > 1. first run of while loop: we have some change from ovnsb~ > 2. ovn-controller execute all *_run() functions to update~ > 3. the txn commit to ovsdb is INCOMPLETE~ and at the same time, there is > another > change from ovnsb~ > 4. second run of while loop: the next call to > idl_loop_run()->ovsdb_idl_run() will update the > local idl. however, since the 'idl_loop' has 'committing_txn', the > *_run() will be no-op. > 5. assume this time, the commit to ovsdb finishes. since the local idl has > already been > updated, ovn-controller will call poll_block() with no immediate > wakeup... > 6. so, the second change to ovnsb will only be checked the next time there > is a new update > from ovnsb. You are right. Here is a proposed fix: http://openvswitch.org/pipermail/dev/2015-July/058192.html ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] ovn-controller: Fix flow generation for container traffic.
On Fri, Jul 24, 2015 at 04:31:03PM -0700, Gurucharan Shetty wrote: > In table 64, when a vlan tag is set on a packet > destined to a container running inside a VM, we currently > don't revert it. This has an unintended consequence for > broadcast traffic when one endpoint of the braodcast > traffic is a plain VM (without containers running inside) where > the previously set tag would remain in the packets sent to the VM. > > This commit fixes the above problem by popping the VLAN > and resetting the input port after outputting the packet > with a vlan tag to a container logical port. > > Signed-off-by: Gurucharan Shetty Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] dpif-netdev: fix race for queues between pmd threads
On Tue, Jul 28, 2015 at 09:55:52AM +0300, Ilya Maximets wrote: > Currently pmd threads select queues in pmd_load_queues() according to > get_n_pmd_threads_on_numa(). This behavior leads to race between pmds, > beacause dp_netdev_set_pmds_on_numa() starts them one by one and > current number of threads changes incrementally. > > As a result we may have the following situation with 2 pmd threads: > > * dp_netdev_set_pmds_on_numa() > * pmd12 thread started. Currently only 1 pmd thread exists. > dpif_netdev(pmd12)|INFO|Core 1 processing port 'port_1' > dpif_netdev(pmd12)|INFO|Core 1 processing port 'port_2' > * pmd14 thread started. 2 pmd threads exists. > dpif_netdev|INFO|Created 2 pmd threads on numa node 0 > dpif_netdev(pmd14)|INFO|Core 2 processing port 'port_2' > > We have: > core 1 --> port 1, port 2 > core 2 --> port 2 > > Fix this by starting pmd threads only after all of them have > been configured. > > Cc: Daniele Di Proietto > Cc: Dyasly Sergey > Signed-off-by: Ilya Maximets > --- Looks good to me. Acked-by: Flavio Leitner ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH V3 1/1] Fix detection of vhost_cuse in dpdk rte_config.h
On Thu, Jul 23, 2015 at 10:48 AM, Gary Mussar wrote: > Dpdk allows users to create a config that includes other config files and > then override values. > > Eg. > defconfig_x86_64-native_vhost_cuse-linuxapp-gcc: > > CONFIG_RTE_BUILD_COMBINE_LIBS=y > CONFIG_RTE_BUILD_SHARED_LIB=n > CONFIG_RTE_LIBRTE_VHOST=y > CONFIG_RTE_LIBRTE_VHOST_USER=n > > This allows you to have both a vhostuser and vhostcuse config in the same > source tree without the need to replicate everything in those config files > just to change a couple of settings. The resultant .config file has all of > the settings from the included files with the updated settings at the end. > The resultant rte_config.h contains multiple undefs and defines for the > overridden settings. > > Eg. > > grep RTE_LIBRTE_VHOST_USER > x86_64-native_vhost_cuse-linuxapp-gcc/include/rte_config.h > #undef RTE_LIBRTE_VHOST_USER > #define RTE_LIBRTE_VHOST_USER 1 > #undef RTE_LIBRTE_VHOST_USER > > The current mechanism to detect the RTE_LIBRTE_VHOST_USER setting merely > greps the rte_config.h file for the string "define RTE_LIBRTE_VHOST_USER 1" > rather than the final setting of RTE_LIBRTE_VHOST_USER. The following patch > changes this test to detect the final setting of RTE_LIBRTE_VHOST_USER. > > Signed-off-by: Gary Mussar > --- I pushed this patch to master and branch-2.4. Thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH branch-2.4 v2] tunnel: Mark GRE64 tunnel protocol deprecated.
On Mon, Jul 27, 2015 at 11:25 AM, Jesse Gross wrote:
> On Tue, Jul 21, 2015 at 9:52 PM, Pravin B Shelar wrote:
>> v2.3.0 - 14 Aug 2014
>> diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
>> index 7bbcf57..8a47ab2 100644
>> --- a/datapath/vport-gre.c
>> +++ b/datapath/vport-gre.c
>> @@ -330,6 +330,7 @@ static struct vport *gre64_create(const struct
>> vport_parms *parms)
>> struct vport *vport;
>> int err;
>>
>> + pr_warn("GRE64 tunnel protocol is deprecated.");
> [...]
>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>> index ea9abf9..d2bd129 100644
>> --- a/lib/netdev-vport.c
>> +++ b/lib/netdev-vport.c
>> @@ -242,6 +242,13 @@ netdev_vport_alloc(void)
>> return &netdev->up;
>> }
>>
>> +static struct netdev *
>> +netdev_gre64_vport_alloc(void)
>> +{
>> +VLOG_WARN("GRE64 tunnel protocol is deprecated.");
>> +return netdev_vport_alloc();
>> +}
>
> For both of these two log messages, I think I would include the
> version number where it will be removed and also make them only warn
> once overall.
>
I updated the patch and pushed it to branch-2.4.
Thanks.
> Otherwise looks good though:
> Acked-by: Jesse Gross
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] ofp-print: Abbreviate duplicated table features.
On Mon, Jul 27, 2015 at 1:58 PM, Andy Zhou wrote: > Ben, thanks for rebasing. I will review it soon. > > On Mon, Jul 27, 2015 at 1:56 PM, Ben Pfaff wrote: >> I spent some time recently looking at the results of "ovs-ofctl >> dump-table-features". It was really distressing because of the volume of >> information. Every table yielded well over 100 lines of output and for 253 >> (visible) tables that meant over 25,300 lines of output total, which is >> basically unusable. >> >> This commit cuts the volume of output greatly by eliminating most of the >> duplication from one table to the next. The command now prints the full >> output only for table 0, and for each subsequent table prints only the >> parts that differ. That reduces the output volume for tables after the >> first to only 9 lines each (one of which is blank), for a total of more >> like 2,400 lines, which is still not short but reasonably manageable. >> >> Signed-off-by: Ben Pfaff >> --- >> v1->v2: Rebase. Nice patch! It is much simpler and cleaner than I imagined. Acked-by: Andy Zhou ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] ofp-print: Abbreviate duplicated table features.
On Tue, Jul 28, 2015 at 02:33:11PM -0700, Andy Zhou wrote: > On Mon, Jul 27, 2015 at 1:58 PM, Andy Zhou wrote: > > Ben, thanks for rebasing. I will review it soon. > > > > On Mon, Jul 27, 2015 at 1:56 PM, Ben Pfaff wrote: > >> I spent some time recently looking at the results of "ovs-ofctl > >> dump-table-features". It was really distressing because of the volume of > >> information. Every table yielded well over 100 lines of output and for 253 > >> (visible) tables that meant over 25,300 lines of output total, which is > >> basically unusable. > >> > >> This commit cuts the volume of output greatly by eliminating most of the > >> duplication from one table to the next. The command now prints the full > >> output only for table 0, and for each subsequent table prints only the > >> parts that differ. That reduces the output volume for tables after the > >> first to only 9 lines each (one of which is blank), for a total of more > >> like 2,400 lines, which is still not short but reasonably manageable. > >> > >> Signed-off-by: Ben Pfaff > >> --- > >> v1->v2: Rebase. > > > Nice patch! It is much simpler and cleaner than I imagined. > > Acked-by: Andy Zhou Thanks! Applied to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] openflow: Add additional reserved space in struct nx_geneve_table_reply.
It's possible to imagine that a switch might want to report additional
capabilities related to Geneve beyond just the number of options and
how much space they can consume. Some examples include additional
restrictions on parsing (if this command is used for non-OVS implementations
or OVS changes how it works) and per-packet actions that can't be done
generically (such as checksums or encryption). It's not yet clear if
these will be necessary or if OpenFlow is the right place to expose
them. However, it's easy to do now and there is very little cost so
it seems like a good idea to leave some additional reserved space.
Signed-off-by: Jesse Gross
---
include/openflow/nicira-ext.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 465b19d..b56edb3 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -986,12 +986,12 @@ OFP_ASSERT(sizeof(struct nx_geneve_table_mod) == 8);
struct nx_geneve_table_reply {
ovs_be32 max_option_space; /* Maximum total of option sizes supported. */
ovs_be16 max_fields; /* Maximum number of match fields supported. */
-uint8_t pad[2];
+uint8_t pad[10];
/* struct nx_geneve_map[0]; Array of maps between indicies and Geneve
options. The number of elements is
inferred from the length field in the
header. */
};
-OFP_ASSERT(sizeof(struct nx_geneve_table_reply) == 8);
+OFP_ASSERT(sizeof(struct nx_geneve_table_reply) == 16);
#endif /* openflow/nicira-ext.h */
--
2.1.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ofp-util: Report error for weighted bucket in non-select group.
This complies with OpenFlow 1.1 and later.
Found by OFTest.
Signed-off-by: Ben Pfaff
---
lib/ofp-util.c | 4
1 file changed, 4 insertions(+)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 9996e84..28f6d8f 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -8592,6 +8592,10 @@ ofputil_decode_group_mod(const struct ofp_header *oh,
}
LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
+if (bucket->weight && gm->type != OFPGT11_SELECT) {
+return OFPERR_OFPGMFC_INVALID_GROUP;
+}
+
switch (gm->type) {
case OFPGT11_ALL:
case OFPGT11_INDIRECT:
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ofproto: Ignore generation ID for role change to "equal".
The OpenFlow specification says that only role changes to slave or master
check the generation ID, so this is a bug fix.
Found by OFTest.
Signed-off-by: Ben Pfaff
---
ofproto/ofproto.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index a724071..e9b1472 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5315,7 +5315,8 @@ handle_role_request(struct ofconn *ofconn, const struct
ofp_header *oh)
}
if (request.role != OFPCR12_ROLE_NOCHANGE) {
-if (request.have_generation_id
+if (request.role != OFPCR12_ROLE_EQUAL
+&& request.have_generation_id
&& !ofconn_set_master_election_id(ofconn, request.generation_id)) {
return OFPERR_OFPRRFC_STALE;
}
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ofp-actions: OFPP_ANY (aka OFPP_NONE) is not a valid output port.
This is implied by the list of ports that are valid for output in the
various versions of the OpenFlow specification.
Found by OFTest.
Signed-off-by: Ben Pfaff
---
lib/ofp-actions.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index eef3389..14a2802 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -5396,10 +5396,12 @@ ofpact_check_output_port(ofp_port_t port, ofp_port_t
max_ports)
case OFPP_FLOOD:
case OFPP_ALL:
case OFPP_CONTROLLER:
-case OFPP_NONE:
case OFPP_LOCAL:
return 0;
+case OFPP_NONE:
+return OFPERR_OFPBAC_BAD_OUT_PORT;
+
default:
if (ofp_to_u16(port) < ofp_to_u16(max_ports)) {
return 0;
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] openflow: Add additional reserved space in struct nx_geneve_table_reply.
On Tue, Jul 28, 2015 at 03:20:52PM -0700, Jesse Gross wrote: > It's possible to imagine that a switch might want to report additional > capabilities related to Geneve beyond just the number of options and > how much space they can consume. Some examples include additional > restrictions on parsing (if this command is used for non-OVS implementations > or OVS changes how it works) and per-packet actions that can't be done > generically (such as checksums or encryption). It's not yet clear if > these will be necessary or if OpenFlow is the right place to expose > them. However, it's easy to do now and there is very little cost so > it seems like a good idea to leave some additional reserved space. > > Signed-off-by: Jesse Gross I'd consider s/pad/zero/ to make it easier for receivers to be sure (later) that they're interpreting non-garbage. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] openflow: Add additional reserved space in struct nx_geneve_table_reply.
On Tue, Jul 28, 2015 at 03:30:05PM -0700, Ben Pfaff wrote: > On Tue, Jul 28, 2015 at 03:20:52PM -0700, Jesse Gross wrote: > > It's possible to imagine that a switch might want to report additional > > capabilities related to Geneve beyond just the number of options and > > how much space they can consume. Some examples include additional > > restrictions on parsing (if this command is used for non-OVS implementations > > or OVS changes how it works) and per-packet actions that can't be done > > generically (such as checksums or encryption). It's not yet clear if > > these will be necessary or if OpenFlow is the right place to expose > > them. However, it's easy to do now and there is very little cost so > > it seems like a good idea to leave some additional reserved space. > > > > Signed-off-by: Jesse Gross > > I'd consider s/pad/zero/ to make it easier for receivers to be sure > (later) that they're interpreting non-garbage. Oh, but it's fine either way: Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] openflow: Add additional reserved space in struct nx_geneve_table_reply.
On Tue, Jul 28, 2015 at 3:30 PM, Ben Pfaff wrote: > On Tue, Jul 28, 2015 at 03:30:05PM -0700, Ben Pfaff wrote: >> On Tue, Jul 28, 2015 at 03:20:52PM -0700, Jesse Gross wrote: >> > It's possible to imagine that a switch might want to report additional >> > capabilities related to Geneve beyond just the number of options and >> > how much space they can consume. Some examples include additional >> > restrictions on parsing (if this command is used for non-OVS >> > implementations >> > or OVS changes how it works) and per-packet actions that can't be done >> > generically (such as checksums or encryption). It's not yet clear if >> > these will be necessary or if OpenFlow is the right place to expose >> > them. However, it's easy to do now and there is very little cost so >> > it seems like a good idea to leave some additional reserved space. >> > >> > Signed-off-by: Jesse Gross >> >> I'd consider s/pad/zero/ to make it easier for receivers to be sure >> (later) that they're interpreting non-garbage. > > Oh, but it's fine either way: > Acked-by: Ben Pfaff I changed it to 'reserved' since I think that's more canonical and applied this to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] ovn-controller: Fix flow generation for container traffic.
On Tue, Jul 28, 2015 at 01:44:42PM -0700, Ben Pfaff wrote:
> On Fri, Jul 24, 2015 at 04:31:03PM -0700, Gurucharan Shetty wrote:
> > In table 64, when a vlan tag is set on a packet
> > destined to a container running inside a VM, we currently
> > don't revert it. This has an unintended consequence for
> > broadcast traffic when one endpoint of the braodcast
> > traffic is a plain VM (without containers running inside) where
> > the previously set tag would remain in the packets sent to the VM.
> >
> > This commit fixes the above problem by popping the VLAN
> > and resetting the input port after outputting the packet
> > with a vlan tag to a container logical port.
> >
> > Signed-off-by: Gurucharan Shetty
>
> Acked-by: Ben Pfaff
The same bug is in my tunnel-key series, I'm folding the following into
the final patch in that series.
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 09b7a99..4c81bb2 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -123,6 +123,14 @@ put_encapsulation(enum mf_field_id mff_ovn_geneve,
}
}
+static void
+put_stack(enum mf_field_id field, struct ofpact_stack *stack)
+{
+stack->subfield.field = mf_from_id(field);
+stack->subfield.ofs = 0;
+stack->subfield.n_bits = stack->subfield.field->n_bits;
+}
+
void
physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
const struct ovsrec_bridge *br_int, const char *this_chassis_id,
@@ -301,9 +309,19 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
/* A packet might need to hair-pin back into its ingress
* OpenFlow port (to a different logical port, which we already
* checked back in table 34), so set the in_port to zero. */
+put_stack(MFF_IN_PORT, ofpact_put_STACK_PUSH(&ofpacts));
put_load(0, MFF_IN_PORT, 0, 16, &ofpacts);
}
ofpact_put_OUTPUT(&ofpacts)->port = ofport;
+if (tag) {
+/* Revert the tag added to the packets headed to containers
+ * in the previous step. If we don't do this, the packets
+ * that are to be broadcasted to a VM in the same logical
+ * switch will also contain the tag. Also revert the zero'd
+ * in_port. */
+ofpact_put_STRIP_VLAN(&ofpacts);
+put_stack(MFF_IN_PORT, ofpact_put_STACK_POP(&ofpacts));
+}
ofctrl_add_flow(flow_table, 64, 100, &match, &ofpacts);
} else {
/* Table 32, priority 100.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
