Re: [ovs-dev] [PATCH 3/3] ovn.at: Fix tab/space issues.
> On Oct 14, 2015, at 10:40 PM, Ben Pfaff wrote: > > On Wed, Oct 14, 2015 at 10:38:06PM -0700, Justin Pettit wrote: >> Signed-off-by: Justin Pettit > > Acked-by: Ben Pfaff I pushed this patch and the first. It looks like the second one got delayed, but you should have it now. --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/5] ovn-controller.8: Correct reference to "ovn-encap-type".
Signed-off-by: Justin Pettit --- ovn/controller/ovn-controller.8.xml |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ovn/controller/ovn-controller.8.xml b/ovn/controller/ovn-controller.8.xml index aadec15..9c041ec 100644 --- a/ovn/controller/ovn-controller.8.xml +++ b/ovn/controller/ovn-controller.8.xml @@ -110,8 +110,9 @@ external_ids:ovn-encap-ip -The IP address that a chassis should use to connect to this node using -encapsulation type specified by external_ids:ovn-encap-ip. +The IP address that a chassis should use to connect to this node +using encapsulation type specified by +external_ids:ovn-encap-type. external_ids:ovn-bridge-mappings -- 1.7.5.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/5] ovn-sbctl: Add multiple encaps with "chassis-add".
Signed-off-by: Justin Pettit
---
ovn/utilities/ovn-sbctl.8.in |9 ---
ovn/utilities/ovn-sbctl.c| 44 ++---
tests/ovn-sbctl.at | 35 -
3 files changed, 71 insertions(+), 17 deletions(-)
diff --git a/ovn/utilities/ovn-sbctl.8.in b/ovn/utilities/ovn-sbctl.8.in
index 2f783e9..e4e4431 100644
--- a/ovn/utilities/ovn-sbctl.8.in
+++ b/ovn/utilities/ovn-sbctl.8.in
@@ -114,10 +114,11 @@ Prints a brief overview of the database contents.
.SS "Chassis Commands"
These commands manipulate \fBOVN_Southbound\fR chassis.
.
-.IP "[\fB\-\-may\-exist\fR] \fBchassis\-add \fIchassis\fR \fIencap-type\fR
\fIencap-ip\fR"
-Creates a new chassis named \fIchassis\fR. The chassis will have
-one encap entry with \fIencap-type\fR as tunnel type and \fIencap-ip\fR
-as destination ip.
+.IP "[\fB\-\-may\-exist\fR] \fBchassis\-add \fIchassis\fR \fIencap\-type\fR
\fIencap-ip\fR"
+Creates a new chassis named \fIchassis\fR. \fIencap\-type\fR is a
+comma-separated list of tunnel types. The chassis will have
+one encap entry for each specified tunnel type with \fIencap-ip\fR
+as the destination IP for each.
.IP
Without \fB\-\-may\-exist\fR, attempting to create a chassis that
exists is an error. With \fB\-\-may\-exist\fR, this command does
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index 29aaf47..7861fe7 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -308,9 +308,9 @@ General commands:\n\
\n\
Chassis commands:\n\
chassis-add CHASSIS ENCAP-TYPE ENCAP-IP create a new chassis named\n\
- CHASSIS with one encapsulation\n\
- entry of ENCAP-TYPE and ENCAP-IP\n\
- chassis-del CHASSIS delete CHASSIS and all of its encaps,\n\
+ CHASSIS with ENCAP-TYPE tunnels\n\
+ and ENCAP-IP\n\
+ chassis-del CHASSIS delete CHASSIS and all of its encaps\n\
and gateway_ports\n\
\n\
Port binding commands:\n\
@@ -526,13 +526,11 @@ static void
cmd_chassis_add(struct ctl_context *ctx)
{
struct sbctl_context *sbctl_ctx = sbctl_context_cast(ctx);
-struct sbrec_chassis *ch;
-struct sbrec_encap *encap;
bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
-const char *ch_name, *encap_type, *encap_ip;
+const char *ch_name, *encap_types, *encap_ip;
ch_name = ctx->argv[1];
-encap_type = ctx->argv[2];
+encap_types = ctx->argv[2];
encap_ip = ctx->argv[3];
sbctl_context_populate_cache(ctx);
@@ -546,12 +544,34 @@ cmd_chassis_add(struct ctl_context *ctx)
}
check_conflicts(sbctl_ctx, ch_name,
xasprintf("cannot create a chassis named %s", ch_name));
-ch = sbrec_chassis_insert(ctx->txn);
+
+char *tokstr = xstrdup(encap_types);
+char *token, *save_ptr = NULL;
+struct sset encap_set = SSET_INITIALIZER(&encap_set);
+for (token = strtok_r(tokstr, ",", &save_ptr); token != NULL;
+ token = strtok_r(NULL, ",", &save_ptr)) {
+sset_add(&encap_set, token);
+}
+free(tokstr);
+
+size_t n_encaps = sset_count(&encap_set);
+struct sbrec_encap **encaps = xmalloc(n_encaps * sizeof *encaps);
+const char *encap_type;
+int i = 0;
+SSET_FOR_EACH (encap_type, &encap_set){
+encaps[i] = sbrec_encap_insert(ctx->txn);
+
+sbrec_encap_set_type(encaps[i], encap_type);
+sbrec_encap_set_ip(encaps[i], encap_ip);
+i++;
+}
+sset_destroy(&encap_set);
+
+struct sbrec_chassis *ch = sbrec_chassis_insert(ctx->txn);
sbrec_chassis_set_name(ch, ch_name);
-encap = sbrec_encap_insert(ctx->txn);
-sbrec_encap_set_type(encap, encap_type);
-sbrec_encap_set_ip(encap, encap_ip);
-sbrec_chassis_set_encaps(ch, &encap, 1);
+sbrec_chassis_set_encaps(ch, encaps, n_encaps);
+free(encaps);
+
sbctl_context_invalidate_cache(ctx);
}
diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at
index 674e1e8..d02e00f 100644
--- a/tests/ovn-sbctl.at
+++ b/tests/ovn-sbctl.at
@@ -29,7 +29,40 @@ m4_define([OVN_SBCTL_TEST_STOP],
AT_CHECK([ovs-appctl -t ovn-northd exit])
AT_CHECK([ovs-appctl -t ovsdb-server exit])])
-# ovn-sbctl test.
+dnl -
+
+AT_SETUP([ovn-sbctl - chassis commands])
+OVN_SBCTL_TEST_START
+ovn_init_db ovn-sb
+
+AT_CHECK([ovn-sbctl chassis-add ch0 geneve 1.2.3.4])
+AT_CHECK([ovn-sbctl -f csv -d bare --no-headings --columns ip,type list encap
| sort],
+ [0], [dnl
+1.2.3.4,geneve
+])
+
+AT_CHECK([ovn-sbctl chassis-add ch1 stt,geneve,vxlan 1.2.3.5])
+AT_CHECK([ovn-sbctl -f csv -d bare --no-headings --columns ip,type list encap
| sort],
+ [0], [dnl
+1.2.3.4,geneve
+1.2.3.5,geneve
+1.2.3.5,stt
+1.2.3.5,vxlan
+])
+
+AT_CHECK([ovn-sbctl chassis-d
[ovs-dev] [PATCH 3/5] ovn-controller: Support VXLAN enapsulation.
Signed-off-by: Justin Pettit
---
ovn/controller/encaps.c |5 ++-
ovn/controller/ovn-controller.8.xml | 17 --
ovn/controller/physical.c | 58 +++---
3 files changed, 68 insertions(+), 12 deletions(-)
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index c914e2a..de1aef3 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -214,10 +214,11 @@ preferred_encap(const struct sbrec_chassis *chassis_rec)
/* For hypervisors, we only support Geneve and STT encapsulations.
* Sets are returned alphabetically, so "geneve" will be preferred
- * over "stt". */
+ * over "stt". For gateways, we only support VXLAN encapsulation. */
for (i = 0; i < chassis_rec->n_encaps; i++) {
if (!strcmp(chassis_rec->encaps[i]->type, "geneve")
-|| !strcmp(chassis_rec->encaps[i]->type, "stt")) {
+|| !strcmp(chassis_rec->encaps[i]->type, "stt")
+|| !strcmp(chassis_rec->encaps[i]->type, "vxlan")) {
return chassis_rec->encaps[i];
}
}
diff --git a/ovn/controller/ovn-controller.8.xml
b/ovn/controller/ovn-controller.8.xml
index 9c041ec..b0aee10 100644
--- a/ovn/controller/ovn-controller.8.xml
+++ b/ovn/controller/ovn-controller.8.xml
@@ -102,10 +102,19 @@
external_ids:ovn-encap-type
-The encapsulation type that a chassis should use to connect to this
-node. Supported tunnel types for connecting hypervisors are
-geneve and stt. Gateways may use
-geneve, vxlan, or stt.
+
+ The encapsulation type that a chassis should use to connect to
+ this node. Supported tunnel types for connecting hypervisors
+ are geneve and stt. Gateways may
+ use geneve, vxlan, or
+ stt.
+
+
+
+ Due to the limited amount of metadata in vxlan,
+ the capabilities and performance of connected gateways will be
+ reduced versus other tunnel formats.
+
external_ids:ovn-encap-ip
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 0c239df..b26db63 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -54,7 +54,7 @@ struct chassis_tunnel {
struct hmap_node hmap_node;
const char *chassis_id;
ofp_port_t ofport;
-enum chassis_tunnel_type { GENEVE, STT } type;
+enum chassis_tunnel_type { GENEVE, STT, VXLAN } type;
};
static struct chassis_tunnel *
@@ -120,6 +120,8 @@ put_encapsulation(enum mf_field_id mff_ovn_geneve,
put_load(datapath->tunnel_key | (outport << 24), MFF_TUN_ID, 0, 64,
ofpacts);
put_move(MFF_LOG_INPORT, 0, MFF_TUN_ID, 40, 15, ofpacts);
+} else if (tun->type == VXLAN) {
+put_load(datapath->tunnel_key, MFF_TUN_ID, 0, 24, ofpacts);
} else {
OVS_NOT_REACHED();
}
@@ -182,6 +184,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
}
} else if (!strcmp(iface_rec->type, "stt")) {
tunnel_type = STT;
+} else if (!strcmp(iface_rec->type, "vxlan")) {
+tunnel_type = VXLAN;
} else {
continue;
}
@@ -574,11 +578,14 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
/* Table 0, priority 100.
* ==
*
- * For packets that arrive from a remote hypervisor (by matching a tunnel
- * in_port), set MFF_LOG_DATAPATH, MFF_LOG_INPORT, and MFF_LOG_OUTPORT from
- * the tunnel key data, then resubmit to table 33 to handle packets to the
- * local hypervisor. */
-
+ * Process packets that arrive from a remote hypervisor (by matching
+ * on tunnel in_port). */
+
+/* Add flows for Geneve and STT encapsulations. These
+ * encapsulations have metadata about the ingress and egress logical
+ * ports. We set MFF_LOG_DATAPATH, MFF_LOG_INPORT, and
+ * MFF_LOG_OUTPORT from the tunnel key data, then resubmit to table
+ * 33 to handle packets to the local hypervisor. */
struct chassis_tunnel *tun;
HMAP_FOR_EACH (tun, hmap_node, &tunnels) {
struct match match = MATCH_CATCHALL_INITIALIZER;
@@ -595,14 +602,53 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
put_move(MFF_TUN_ID, 40, MFF_LOG_INPORT, 0, 15, &ofpacts);
put_move(MFF_TUN_ID, 24, MFF_LOG_OUTPORT, 0, 16, &ofpacts);
put_move(MFF_TUN_ID, 0, MFF_LOG_DATAPATH, 0, 24, &ofpacts);
+} else if (tun->type == VXLAN) {
+/* We'll handle VXLAN later. */
+continue;
} else {
OVS_NOT_REACHED();
}
+
put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100, &match, &ofpacts);
[ovs-dev] [PATCH 5/5] ovn.at: Add test for gateway.
This test exposed a problem that ovn-controller-vtep doesn't properly
set up the "Mcast_Macs_Remote" table, which prevents broadcasts from
being sourced from the physical side of the VTEP. That issue needs to
be resolved, and then the full set of gateway traffic patterns can run.
Signed-off-by: Justin Pettit
---
tests/ofproto-macros.at |2 +-
tests/ovn.at| 157 +++
2 files changed, 158 insertions(+), 1 deletions(-)
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index bc3933c..fe99186 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -202,7 +202,7 @@ ovn_attach() {
ovs-vsctl \
-- set Open_vSwitch . external-ids:system-id=$sandbox \
-- set Open_vSwitch .
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
--- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+-- set Open_vSwitch . external-ids:ovn-encap-type=geneve,vxlan \
-- set Open_vSwitch . external-ids:ovn-encap-ip=$ip \
-- add-br br-int \
-- set bridge br-int fail-mode=secure
other-config:disable-in-band=true \
diff --git a/tests/ovn.at b/tests/ovn.at
index b8b9e36..1b43a23 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -666,3 +666,160 @@ for i in 1 2 3; do
done
done
AT_CLEANUP
+
+
+AT_SETUP([ovn -- 3 HVs, 1 VIFs/HV, 1 gateway, 1 logical switch])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+
+# Configure the Northbound database
+ovn-nbctl lswitch-add lsw0
+
+ovn-nbctl lport-add lsw0 lp1
+ovn-nbctl lport-set-macs lp1 f0:00:00:00:00:01
+
+ovn-nbctl lport-add lsw0 lp2
+ovn-nbctl lport-set-macs lp2 f0:00:00:00:00:02
+
+ovn-nbctl lport-add lsw0 lp-vtep
+ovn-nbctl lport-set-type lp-vtep vtep
+ovn-nbctl lport-set-options lp-vtep vtep-physical-switch=br-vtep
vtep-logical-switch=lsw0
+ovn-nbctl lport-set-macs lp-vtep unknown
+
+net_add n1 # Network to connect hv1, hv2, and vtep
+net_add n2 # Network to connect vtep and hv3
+
+# Create hypervisor hv1 connected to n1
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl add-port br-int vif1 -- set Interface vif1 external-ids:iface-id=lp1
options:tx_pcap=hv1/vif1-tx.pcap options:rxq_pcap=hv1/vif1-rx.pcap
ofport-request=1
+
+# Create hypervisor hv2 connected to n1
+sim_add hv2
+as hv2
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.2
+ovs-vsctl add-port br-int vif2 -- set Interface vif2 external-ids:iface-id=lp2
options:tx_pcap=hv2/vif2-tx.pcap options:rxq_pcap=hv2/vif2-rx.pcap
ofport-request=1
+
+
+# Start the vtep emulator with a leg in both networks
+sim_add vtep
+as vtep
+
+ovsdb-tool create "$ovs_base"/vtep/vtep.db
"$abs_top_srcdir"/vtep/vtep.ovsschema || return 1
+ovs-appctl -t ovsdb-server ovsdb-server/add-db "$ovs_base"/vtep/vtep.db
+
+ovs-vsctl add-br br-phys
+net_attach n1 br-phys
+
+mac=`ovs-vsctl get Interface br-phys mac_in_use | sed s/\"//g`
+arp_table="$arp_table $sandbox,br-phys,192.168.0.3,$mac"
+ovs-appctl netdev-dummy/ip4addr br-phys 192.168.0.3/24 >/dev/null || return 1
+ovs-appctl ovs/route/add 192.168.0.3/24 br-phys >/dev/null || return 1
+
+ovs-vsctl add-br br-vtep
+net_attach n2 br-vtep
+
+vtep-ctl add-ps br-vtep
+vtep-ctl set Physical_Switch br-vtep tunnel_ips=192.168.0.3
+vtep-ctl add-ls lsw0
+
+start_daemon ovs-vtep br-vtep
+start_daemon ovn-controller-vtep --vtep-db=unix:"$ovs_base"/vtep/db.sock
--ovnsb-db=unix:"$ovs_base"/ovn-sb/ovn-sb.sock
+
+sleep 1
+
+vtep-ctl bind-ls br-vtep br-vtep_n2 0 lsw0
+
+sleep 1
+
+# Add hv3 on the other side of the vtep
+sim_add hv3
+as hv3
+ovs-vsctl add-br br-phys
+net_attach n2 br-phys
+
+ovs-vsctl add-port br-phys vif3 -- set Interface vif3
options:tx_pcap=hv3/vif3-tx.pcap options:rxq_pcap=hv3/vif3-rx.pcap
ofport-request=1
+
+# Pre-populate the hypervisors' ARP tables so that we don't lose any
+# packets for ARP resolution (native tunneling doesn't queue packets
+# for ARP resolution).
+ovn_populate_arp
+
+# Allow some time for ovn-northd and ovn-controller to catch up.
+# XXX This should be more systematic.
+sleep 1
+ovn-sbctl show
+
+# test_packet INPORT DST SRC ETHTYPE OUTPORT...
+#
+# This shell function causes a packet to be received on INPORT. The packet's
+# content has Ethernet destination DST and source SRC (each exactly 12 hex
+# digits) and Ethernet type ETHTYPE (4 hex digits). The OUTPORTs (zero or
+# more) list the VIFs on which the packet should be received. INPORT and the
+# OUTPORTs are specified as lport numbers, e.g. 1 for vif1.
+trim_zeros() {
+sed 's/\(00\)\{1,\}$//'
+}
+for i in 1 2 3; do
+: > $i.expected
+done
+test_packet() {
+local inport=$1 packet=$2$3$4; shift; shift; shift; shift
+#hv=hv`echo $inport | sed 's/^\(.\).*/\1/'`
+hv=hv$inport
+vif=vif$inport
+as $hv ovs-appctl netdev-dummy/receive $vif $packet
+for outport; do
+echo $packet | trim_zeros >> $outport.expected
+done
+}
+
+# Send packets betwee
[ovs-dev] [PATCH 4/5] ovn-controller: Support multiple encaps simultaneously.
Signed-off-by: Justin Pettit
---
ovn/controller/chassis.c| 88 ++-
ovn/controller/encaps.c | 20
ovn/controller/ovn-controller.8.xml | 10 +++-
ovn/controller/ovn-controller.c | 14 ++
ovn/controller/ovn-controller.h | 11
ovn/controller/physical.c |2 +-
6 files changed, 109 insertions(+), 36 deletions(-)
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 9d6a77a..894000d 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -16,6 +16,7 @@
#include
#include "chassis.h"
+#include "lib/dynamic-string.h"
#include "lib/vswitch-idl.h"
#include "openvswitch/vlog.h"
#include "ovn/lib/ovn-sb-idl.h"
@@ -30,6 +31,23 @@ chassis_register_ovs_idl(struct ovsdb_idl *ovs_idl)
ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_external_ids);
}
+static const char *
+pop_tunnel_name(uint32_t *type)
+{
+if (*type & GENEVE) {
+*type &= ~GENEVE;
+return "geneve";
+} else if (*type & STT) {
+*type &= ~STT;
+return "stt";
+} else if (*type & VXLAN) {
+*type &= ~VXLAN;
+return "vxlan";
+}
+
+OVS_NOT_REACHED();
+}
+
void
chassis_run(struct controller_ctx *ctx, const char *chassis_id)
{
@@ -40,13 +58,10 @@ chassis_run(struct controller_ctx *ctx, const char
*chassis_id)
const struct sbrec_chassis *chassis_rec;
const struct ovsrec_open_vswitch *cfg;
const char *encap_type, *encap_ip;
-struct sbrec_encap *encap_rec;
static bool inited = false;
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. */
cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
if (!cfg) {
VLOG_INFO("No Open_vSwitch row defined.");
@@ -60,23 +75,45 @@ chassis_run(struct controller_ctx *ctx, const char
*chassis_id)
return;
}
+char *tokstr = xstrdup(encap_type);
+char *save_ptr = NULL;
+char *token;
+uint32_t req_tunnels = 0;
+for (token = strtok_r(tokstr, ",", &save_ptr); token != NULL;
+ token = strtok_r(NULL, ",", &save_ptr)) {
+uint32_t type = get_tunnel_type(token);
+if (!type) {
+VLOG_INFO("Unknown tunnel type: %s", token);
+}
+req_tunnels |= type;
+}
+free(tokstr);
+
if (chassis_rec) {
-int i;
-
-for (i = 0; i < chassis_rec->n_encaps; i++) {
-if (!strcmp(chassis_rec->encaps[i]->type, encap_type)
-&& !strcmp(chassis_rec->encaps[i]->ip, encap_ip)) {
-/* Nothing changed. */
-inited = true;
-return;
-} else if (!inited) {
-VLOG_WARN("Chassis config changing on startup, make sure "
- "multiple chassis are not configured : %s/%s->%s/%s",
- chassis_rec->encaps[i]->type,
- chassis_rec->encaps[i]->ip,
- encap_type, encap_ip);
-}
+uint32_t cur_tunnels = 0;
+for (int i = 0; i < chassis_rec->n_encaps; i++) {
+cur_tunnels |= get_tunnel_type(chassis_rec->encaps[i]->type);
+}
+if (req_tunnels == cur_tunnels
+&& !strcmp(chassis_rec->encaps[0]->ip, encap_ip)) {
+/* Nothing changed. */
+inited = true;
+return;
+} else if (!inited) {
+struct ds cur_encaps = DS_EMPTY_INITIALIZER;
+for (int i = 0; i < chassis_rec->n_encaps; i++) {
+ds_put_format(&cur_encaps, "%s,",
+ chassis_rec->encaps[i]->type);
+}
+ds_chomp(&cur_encaps, ',');
+
+VLOG_WARN("Chassis config changing on startup, make sure "
+ "multiple chassis are not configured : %s/%s->%s/%s",
+ ds_cstr(&cur_encaps),
+ chassis_rec->encaps[0]->ip,
+ encap_type, encap_ip);
+ds_destroy(&cur_encaps);
}
}
@@ -89,12 +126,19 @@ chassis_run(struct controller_ctx *ctx, const char
*chassis_id)
sbrec_chassis_set_name(chassis_rec, chassis_id);
}
-encap_rec = sbrec_encap_insert(ctx->ovnsb_idl_txn);
+int n_encaps = count_1bits(req_tunnels);
+struct sbrec_encap **encaps = xmalloc(n_encaps * sizeof *encaps);
+for (int i = 0; i < n_encaps; i++) {
+const char *type = pop_tunnel_name(&req_tunnels);
+
+encaps[i] = sbrec_encap_insert(ctx->ovnsb_idl_txn);
-sbrec_encap_set_type(encap_rec, encap_type);
-sbrec_encap_set_ip(encap_rec, encap_ip);
+sbrec_encap_set_type(encaps[i], type);
+sbrec_encap_set_ip(encaps[i], encap_ip);
+}
-sbrec_chassis_set_encaps(chassis_rec, &encap_rec, 1);
+sbrec_chassis_set_encaps(chassis_rec, encaps, n_encaps);
Re: [ovs-dev] [PATCH 3/3] ovn.at: Fix tab/space issues.
> On Oct 15, 2015, at 1:42 AM, Justin Pettit wrote: > > >> On Oct 14, 2015, at 10:40 PM, Ben Pfaff wrote: >> >> On Wed, Oct 14, 2015 at 10:38:06PM -0700, Justin Pettit wrote: >>> Signed-off-by: Justin Pettit >> >> Acked-by: Ben Pfaff > > I pushed this patch and the first. It looks like the second one got delayed, > but you should have it now. Scratch reviewing that. My vtep series depended on that patch, so I just resent it as part of the series. --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] datapath-windows: Support attribute OVS_KEY_ATTR_TCP_FLAGS
This patch adds OVS_KEY_ATTR_TCP_FLAGS to our flow mechanism.
Also clean unecesarry parts of code.
Signed-off-by: Alin Gabriel Serdean
Signed-off-by: Sorin Vinturis
---
This patch is intended for branch-2.4 as well.
---
datapath-windows/ovsext/DpInternal.h | 7 +++---
datapath-windows/ovsext/Flow.c | 40 ++
datapath-windows/ovsext/Flow.h | 2 +-
datapath-windows/ovsext/PacketParser.c | 1 +
datapath-windows/ovsext/PacketParser.h | 3 ---
5 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/datapath-windows/ovsext/DpInternal.h
b/datapath-windows/ovsext/DpInternal.h
index 8de48a2..0405b8e 100644
--- a/datapath-windows/ovsext/DpInternal.h
+++ b/datapath-windows/ovsext/DpInternal.h
@@ -65,6 +65,8 @@ typedef struct _OVS_VPORT_EXT_INFO {
typedef struct L4Key {
ovs_be16 tpSrc; /* TCP/UDP/SCTP source port. */
ovs_be16 tpDst; /* TCP/UDP/SCTP destination port. */
+ovs_be16 flags; /* TCP flags */
+uint8_t pad[2];
} L4Key;
typedef struct Ipkey {
@@ -75,7 +77,7 @@ typedef struct Ipkey {
uint8_t nwTtl; /* IP TTL/Hop Limit. */
uint8_t nwFrag; /* FLOW_FRAG_* flags. */
L4Key l4;
-} IpKey; /* Size of 16 byte. */
+} IpKey; /* Size of 20 byte. */
typedef struct ArpKey {
ovs_be32 nwSrc; /* IPv4 source address. */
@@ -95,7 +97,6 @@ typedef struct Ipv6Key {
uint8_t nwTtl; /* IP TTL/Hop Limit. */
uint8_t nwFrag; /* FLOW_FRAG_* flags. */
L4Key l4;
-uint32_t pad;
} Ipv6Key; /* Size of 48 byte. */
typedef struct Icmp6Key {
@@ -110,7 +111,7 @@ typedef struct Icmp6Key {
uint8_t arpSha[6]; /* ARP/ND source hardware address. */
uint8_t arpTha[6]; /* ARP/ND target hardware address. */
struct in6_addr ndTarget;/* IPv6 neighbor discovery (ND) target. */
-} Icmp6Key; /* Size of 72 byte. */
+} Icmp6Key; /* Size of 76 byte. */
typedef struct L2Key {
uint32_t inPort; /* Port number of input port. */
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index b629c93..ca6a353 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -980,6 +980,7 @@ _MapFlowIpv4KeyToNlKey(PNL_BUFFER nlBuf, IpKey
*ipv4FlowPutKey)
switch (ipv4Key.ipv4_proto) {
case IPPROTO_TCP: {
struct ovs_key_tcp tcpKey;
+UINT16 tcpFlags = 0;
tcpKey.tcp_src = ipv4FlowPutKey->l4.tpSrc;
tcpKey.tcp_dst = ipv4FlowPutKey->l4.tpDst;
if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP,
@@ -988,6 +989,13 @@ _MapFlowIpv4KeyToNlKey(PNL_BUFFER nlBuf, IpKey
*ipv4FlowPutKey)
rc = STATUS_UNSUCCESSFUL;
goto done;
}
+tcpFlags = ipv4FlowPutKey->l4.flags;
+if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP_FLAGS,
+ (PCHAR)(&tcpFlags),
+sizeof(tcpFlags))) {
+rc = STATUS_UNSUCCESSFUL;
+goto done;
+}
break;
}
@@ -1074,6 +1082,7 @@ _MapFlowIpv6KeyToNlKey(PNL_BUFFER nlBuf, Ipv6Key
*ipv6FlowPutKey,
switch (ipv6Key.ipv6_proto) {
case IPPROTO_TCP: {
struct ovs_key_tcp tcpKey;
+UINT16 tcpFlags = 0;
tcpKey.tcp_src = ipv6FlowPutKey->l4.tpSrc;
tcpKey.tcp_dst = ipv6FlowPutKey->l4.tpDst;
if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP,
@@ -1082,6 +1091,13 @@ _MapFlowIpv6KeyToNlKey(PNL_BUFFER nlBuf, Ipv6Key
*ipv6FlowPutKey,
rc = STATUS_UNSUCCESSFUL;
goto done;
}
+tcpFlags = ipv6FlowPutKey->l4.flags;
+if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP_FLAGS,
+ (PCHAR)(&tcpFlags),
+ sizeof(tcpFlags))) {
+rc = STATUS_UNSUCCESSFUL;
+goto done;
+}
break;
}
@@ -1357,6 +1373,12 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
ipv4FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
}
+if (keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]) {
+const UINT16 *flags;
+flags = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]);
+ipv4FlowPutKey->l4.flags = *flags;
+}
+
if (keyAttrs[OVS_KEY_ATTR_UDP]) {
const struct ovs_key_udp *udpKey;
udpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_UDP]);
@@ -1401,6 +1423,12 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
ipv6FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
}
+if (keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]) {
+const UINT16 *flags;
+flags = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]);
+ipv6FlowP
[ovs-dev] [PATCH v2] datapath-windows: Support attribute OVS_KEY_ATTR_TCP_FLAGS
This patch adds OVS_KEY_ATTR_TCP_FLAGS to our flow mechanism.
Also clean unecesarry parts of code.
Signed-off-by: Alin Gabriel Serdean
Signed-off-by: Sorin Vinturis
---
This patch is intended for branch-2.4 as well.
---
datapath-windows/ovsext/DpInternal.h | 7 +++---
datapath-windows/ovsext/Flow.c | 40 ++
datapath-windows/ovsext/Flow.h | 2 +-
datapath-windows/ovsext/PacketParser.c | 1 +
datapath-windows/ovsext/PacketParser.h | 3 ---
5 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/datapath-windows/ovsext/DpInternal.h
b/datapath-windows/ovsext/DpInternal.h
index 8de48a2..0405b8e 100644
--- a/datapath-windows/ovsext/DpInternal.h
+++ b/datapath-windows/ovsext/DpInternal.h
@@ -65,6 +65,8 @@ typedef struct _OVS_VPORT_EXT_INFO {
typedef struct L4Key {
ovs_be16 tpSrc; /* TCP/UDP/SCTP source port. */
ovs_be16 tpDst; /* TCP/UDP/SCTP destination port. */
+ovs_be16 flags; /* TCP flags */
+uint8_t pad[2];
} L4Key;
typedef struct Ipkey {
@@ -75,7 +77,7 @@ typedef struct Ipkey {
uint8_t nwTtl; /* IP TTL/Hop Limit. */
uint8_t nwFrag; /* FLOW_FRAG_* flags. */
L4Key l4;
-} IpKey; /* Size of 16 byte. */
+} IpKey; /* Size of 20 byte. */
typedef struct ArpKey {
ovs_be32 nwSrc; /* IPv4 source address. */
@@ -95,7 +97,6 @@ typedef struct Ipv6Key {
uint8_t nwTtl; /* IP TTL/Hop Limit. */
uint8_t nwFrag; /* FLOW_FRAG_* flags. */
L4Key l4;
-uint32_t pad;
} Ipv6Key; /* Size of 48 byte. */
typedef struct Icmp6Key {
@@ -110,7 +111,7 @@ typedef struct Icmp6Key {
uint8_t arpSha[6]; /* ARP/ND source hardware address. */
uint8_t arpTha[6]; /* ARP/ND target hardware address. */
struct in6_addr ndTarget;/* IPv6 neighbor discovery (ND) target. */
-} Icmp6Key; /* Size of 72 byte. */
+} Icmp6Key; /* Size of 76 byte. */
typedef struct L2Key {
uint32_t inPort; /* Port number of input port. */
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index b629c93..ca6a353 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -980,6 +980,7 @@ _MapFlowIpv4KeyToNlKey(PNL_BUFFER nlBuf, IpKey
*ipv4FlowPutKey)
switch (ipv4Key.ipv4_proto) {
case IPPROTO_TCP: {
struct ovs_key_tcp tcpKey;
+UINT16 tcpFlags = 0;
tcpKey.tcp_src = ipv4FlowPutKey->l4.tpSrc;
tcpKey.tcp_dst = ipv4FlowPutKey->l4.tpDst;
if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP,
@@ -988,6 +989,13 @@ _MapFlowIpv4KeyToNlKey(PNL_BUFFER nlBuf, IpKey
*ipv4FlowPutKey)
rc = STATUS_UNSUCCESSFUL;
goto done;
}
+tcpFlags = ipv4FlowPutKey->l4.flags;
+if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP_FLAGS,
+ (PCHAR)(&tcpFlags),
+sizeof(tcpFlags))) {
+rc = STATUS_UNSUCCESSFUL;
+goto done;
+}
break;
}
@@ -1074,6 +1082,7 @@ _MapFlowIpv6KeyToNlKey(PNL_BUFFER nlBuf, Ipv6Key
*ipv6FlowPutKey,
switch (ipv6Key.ipv6_proto) {
case IPPROTO_TCP: {
struct ovs_key_tcp tcpKey;
+UINT16 tcpFlags = 0;
tcpKey.tcp_src = ipv6FlowPutKey->l4.tpSrc;
tcpKey.tcp_dst = ipv6FlowPutKey->l4.tpDst;
if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP,
@@ -1082,6 +1091,13 @@ _MapFlowIpv6KeyToNlKey(PNL_BUFFER nlBuf, Ipv6Key
*ipv6FlowPutKey,
rc = STATUS_UNSUCCESSFUL;
goto done;
}
+tcpFlags = ipv6FlowPutKey->l4.flags;
+if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP_FLAGS,
+ (PCHAR)(&tcpFlags),
+ sizeof(tcpFlags))) {
+rc = STATUS_UNSUCCESSFUL;
+goto done;
+}
break;
}
@@ -1357,6 +1373,12 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
ipv4FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
}
+if (keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]) {
+const UINT16 *flags;
+flags = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]);
+ipv4FlowPutKey->l4.flags = *flags;
+}
+
if (keyAttrs[OVS_KEY_ATTR_UDP]) {
const struct ovs_key_udp *udpKey;
udpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_UDP]);
@@ -1401,6 +1423,12 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
ipv6FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
}
+if (keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]) {
+const UINT16 *flags;
+flags = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]);
+ipv6FlowP
Re: [ovs-dev] [PATCH net-next V15 3/3] 802.1AD: Flow handling, actions, vlan parsing and netlink attributes
On 10/13/15 2:14 PM, Pravin Shelar wrote:
On Tue, Oct 13, 2015 at 10:39 AM, Thomas F Herbert
wrote:
Pravin,
Thanks for the review.
On 10/13/15 7:47 AM, Pravin Shelar wrote:
On Sat, Oct 10, 2015 at 4:40 PM, Thomas F Herbert
wrote:
Add support for 802.1ad including the ability to push and pop double
tagged vlans. Add support for 802.1ad to netlink parsing and flow
conversion. Uses double nested encap attributes to represent double
tagged vlan. Inner TPID encoded along with ctci in nested attributes.
Signed-off-by: Thomas F Herbert
---
net/openvswitch/actions.c | 6 +-
net/openvswitch/flow.c | 92 +++
net/openvswitch/flow.h | 11 ++-
net/openvswitch/flow_netlink.c | 166
+
net/openvswitch/vport-netdev.c | 4 +-
5 files changed, 245 insertions(+), 34 deletions(-)
...
...
I see lot of duplicate code here. How about code below:
struct qtag_prefix {
__be16 eth_type; /* ETH_P_8021Q or ETH_P_8021AD */
__be16 tci;
};
/* Return < 0 on memory error
* Return == 0 on non vlan or incomplete packet packet
* Return > 0 on successfully parsing vlan tag.
*/
static int parse_vlan_tag(__be16 vlan_proto, struct sk_buff *skb,
struct vlan_tag *cvlan)
{
if (likely(!eth_type_vlan(skb->vlan_proto)))
return 0;
if (unlikely(skb->len < sizeof(struct qtag_prefix) +
sizeof(__be16))) {
vlan->tci = 0;
return 0;
}
if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
sizeof(__be16
return -ENOMEM;
qp = (struct qtag_prefix *)skb->data;
key->eth.cvlan.tci = qp->tci | htons(VLAN_TAG_PRESENT);
key->eth.cvlan.tpid = qp->eth_type;
__skb_pull(skb, sizeof(struct qtag_prefix));
return 1;
}
This makes for cleaner code and certainly better for maintainability so I
have just implemented it for this next revision. However, note that with
this change, we incur the overhead of an additional function call for single
tagged vlan packets.
If there is any performance issue we can fix the code later.
static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
{
struct qtag_prefix *qp = (struct qtag_prefix *)skb->data;
...
+ u64 mask_v_attrs = 0;
+
+ err = parse_flow_mask_nlattrs(*nla, a, &mask_v_attrs,
log);
+ if (err)
+ return err;
+
+ if (mask_v_attrs & 1 << OVS_KEY_ATTR_ENCAP) {
+ if (!*ie_valid) {
+ OVS_NLERR(log, "Encap mask attribute is
set for non-CVLAN frame.");
+ err = -EINVAL;
+ return err;
+ }
+ encap = a[OVS_KEY_ATTR_ENCAP];
+
+ err = cust_vlan_from_nlattrs(match, a, is_mask,
log);
+ if (err)
+ return err;
+ *nla = encap;
+
There is no checking for ATTR_VLAN or ATTR_ETHERTYPE. This can result
in null pointer deference in cust_vlan_from_nlattrs().
The original vlan code does not check for these attribs in the masked case.
It does check for them in the non-masked case and then sets a boolean and
checks it in the masked case. I do the same thing for the inner vlan. I
check for the attributes in the non-masked case and set a boolean and check
the boolean in the masked case. Why is this not sufficient?
Original code is checking for attributes before referencing them. For
example in function ovs_nla_get_match() before extracting eth_type,
it does check a[OVS_KEY_ATTR_ETHERTYPE]. But If you spot bug in
current code please send fix for net tree.
Regarding the Boolean, it is for presence of inner vlan for key
attribute, mask attribute still could be missing vlan attribute.
For vlan mask, we can keep check sanity check as outer vlan. It means
eth_type must be specified and should be 0x, and tci mask is
optional and by default initialized to 0x.
You are correct. I was thinking of something else. I had this but must
have lost it one of the patch revisions. Fixed it in V16.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net-next V16 1/3] openvswitch: 802.1ad uapi changes.
openvswitch: Add support for 8021.AD
Change the description of the VLAN tpid field.
Signed-off-by: Thomas F Herbert
---
include/uapi/linux/openvswitch.h | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 32e07d8..b0c959c 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -585,13 +585,13 @@ struct ovs_action_push_mpls {
* @vlan_tci: Tag control identifier (TCI) to push. The CFI bit must be set
* (but it will not be set in the 802.1Q header that is pushed).
*
- * The @vlan_tpid value is typically %ETH_P_8021Q. The only acceptable TPID
- * values are those that the kernel module also parses as 802.1Q headers, to
- * prevent %OVS_ACTION_ATTR_PUSH_VLAN followed by %OVS_ACTION_ATTR_POP_VLAN
- * from having surprising results.
+ * The @vlan_tpid value is typically %ETH_P_8021Q or %ETH_P_8021AD.
+ * The only acceptable TPID values are those that the kernel module also parses
+ * as 802.1Q or 802.1AD headers, to prevent %OVS_ACTION_ATTR_PUSH_VLAN followed
+ * by %OVS_ACTION_ATTR_POP_VLAN from having surprising results.
*/
struct ovs_action_push_vlan {
- __be16 vlan_tpid; /* 802.1Q TPID. */
+ __be16 vlan_tpid; /* 802.1Q or 802.1ad TPID. */
__be16 vlan_tci;/* 802.1Q TCI (VLAN ID and priority). */
};
@@ -664,9 +664,10 @@ enum ovs_ct_attr {
* is copied from the value to the packet header field, rest of the bits are
* left unchanged. The non-masked value bits must be passed in as zeroes.
* Masking is not supported for the %OVS_KEY_ATTR_TUNNEL attribute.
- * @OVS_ACTION_ATTR_PUSH_VLAN: Push a new outermost 802.1Q header onto the
- * packet.
- * @OVS_ACTION_ATTR_POP_VLAN: Pop the outermost 802.1Q header off the packet.
+ * @OVS_ACTION_ATTR_PUSH_VLAN: Push a new outermost 802.1Q or 802.1ad header
+ * onto the packet.
+ * @OVS_ACTION_ATTR_POP_VLAN: Pop the outermost 802.1Q or 802.1ad header
+ * from the packet.
* @OVS_ACTION_ATTR_SAMPLE: Probabilitically executes actions, as specified in
* the nested %OVS_SAMPLE_ATTR_* attributes.
* @OVS_ACTION_ATTR_PUSH_MPLS: Push a new MPLS label stack entry onto the
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net-next V16 3/3] openvswitch: 802.1AD: Flow handling, actions, vlan parsing and netlink attributes
Add support for 802.1ad including the ability to push and pop double
tagged vlans. Add support for 802.1ad to netlink parsing and flow
conversion. Uses double nested encap attributes to represent double
tagged vlan. Inner TPID encoded along with ctci in nested attributes.
Signed-off-by: Thomas F Herbert
---
net/openvswitch/actions.c | 6 +-
net/openvswitch/flow.c | 75 ++
net/openvswitch/flow.h | 8 +-
net/openvswitch/flow_netlink.c | 169 +
net/openvswitch/vport-netdev.c | 4 +-
5 files changed, 228 insertions(+), 34 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 315f533..09cc1c9 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -236,7 +236,8 @@ static int pop_vlan(struct sk_buff *skb, struct sw_flow_key
*key)
if (skb_vlan_tag_present(skb))
invalidate_flow_key(key);
else
- key->eth.tci = 0;
+ key->eth.vlan.tci = 0;
+ key->eth.vlan.tpid = 0;
return err;
}
@@ -246,7 +247,8 @@ static int push_vlan(struct sk_buff *skb, struct
sw_flow_key *key,
if (skb_vlan_tag_present(skb))
invalidate_flow_key(key);
else
- key->eth.tci = vlan->vlan_tci;
+ key->eth.vlan.tci = vlan->vlan_tci;
+ key->eth.vlan.tpid = vlan->vlan_tpid;
return skb_vlan_push(skb, vlan->vlan_tpid,
ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
}
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index c8db44a..8a4e298 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -302,24 +302,69 @@ static bool icmp6hdr_ok(struct sk_buff *skb)
sizeof(struct icmp6hdr));
}
-static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
+struct qtag_prefix {
+ __be16 eth_type; /* ETH_P_8021Q or ETH_P_8021AD */
+ __be16 tci;
+};
+
+/* Parse vlan tag from vlan header.
+ * Returns ERROR on memory error.
+ * Returns 0 if it encounters a non-vlan or incomplete packet.
+ * Returns 1 after successfully parsing vlan tag.
+ */
+
+static int parse_vlan_tag(struct sk_buff *skb, __be16 vlan_proto,
+ __be16 vlan_tci, struct vlan_head *vlan)
{
- struct qtag_prefix {
- __be16 eth_type; /* ETH_P_8021Q */
- __be16 tci;
- };
- struct qtag_prefix *qp;
+ if (likely(!eth_type_vlan(vlan_proto)))
+ return 0;
if (unlikely(skb->len < sizeof(struct qtag_prefix) + sizeof(__be16)))
return 0;
if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
-sizeof(__be16
+sizeof(__be16
return -ENOMEM;
- qp = (struct qtag_prefix *) skb->data;
- key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
+ vlan->tci = vlan_tci | htons(VLAN_TAG_PRESENT);
+ vlan->tpid = vlan_proto;
+
__skb_pull(skb, sizeof(struct qtag_prefix));
+ return 1;
+}
+
+static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ struct qtag_prefix *qp = (struct qtag_prefix *)skb->data;
+ int res;
+
+ if (likely(skb_vlan_tag_present(skb))) {
+ key->eth.vlan.tci = htons(skb->vlan_tci);
+ key->eth.vlan.tpid = skb->vlan_proto;
+
+ /* Case where ingress processing has already stripped
+* the outer vlan tag.
+*/
+ res = parse_vlan_tag(skb, qp->eth_type, qp->tci,
+&key->eth.cvlan);
+ if (res < 0)
+ return res;
+ /* For inner tag, return 0 because neither
+* non-existant nor partial inner tag is an error.
+*/
+ return 0;
+ }
+ res = parse_vlan_tag(skb, qp->eth_type, qp->tci, &key->eth.vlan);
+ if (res <= 0)
+ /* This is an outer tag in the non-accelerated VLAN
+* case. Return error unless it is a complete vlan tag.
+*/
+ return res;
+
+ /* Parse inner vlan tag if present for non-accelerated case. */
+ res = parse_vlan_tag(skb, qp->eth_type, qp->tci, &key->eth.cvlan);
+ if (res <= 0)
+ return res;
return 0;
}
@@ -480,12 +525,12 @@ static int key_extract(struct sk_buff *skb, struct
sw_flow_key *key)
* update skb->csum here.
*/
- key->eth.tci = 0;
- if (skb_vlan_tag_present(skb))
- key->eth.tci = htons(skb->vlan_tci);
- else if (eth->h_proto == htons(ETH_P_8021Q))
- if (unlikely(parse_vlan(skb, key)))
- return -ENOMEM;
+ key->eth.vlan.tci = 0;
+ key->eth.vlan.tpid = 0;
+ key->eth.cvlan.tci = 0;
+
Re: [ovs-dev] [PATCH net-next V16 2/3] Check for vlan ethernet types for 8021.q or 802.1ad
On 10/15/2015 5:01 PM, Thomas F Herbert wrote:
Hm, I'm seeing no sign-off on this patch.
---
include/linux/if_vlan.h | 16
1 file changed, 16 insertions(+)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 67ce5bd..ac23521 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -627,6 +627,22 @@ static inline netdev_features_t vlan_features_check(const
struct sk_buff *skb,
return features;
}
+/**
+ * eth_type_vlan - check for valid vlan ether type.
+ * @ethertype: ether type to check
+ *
+ * Returns true if the ether type is a vlan ether type.
+ */
+static inline bool eth_type_vlan(__be16 ethertype)
+{
+ switch (ethertype) {
+ case (htons(ETH_P_8021Q)):
+ case (htons(ETH_P_8021AD)):
Hm, I thought you had a request to delete the outer parens already...
[...]
MBR, Sergei
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net-next V16 0/3] openvswitch: Add support for 802.1ad
This patch accompanies version 14 of the user level openvswitch patch submitted to openvswitch dev list. V16,15: Implement reviewer comments. V14: Add outer tpid to flow_key V13: Fix incorrect encoding and decoding of netlink to/from key attributes. V12: Fix some problems and issues pointed out by reviewers. When parsing netlink attributes Ether types other then 0x88a8 as outer tpid. V11: Add inner tpid to flow key. Fix separate inner encap attribute when parsing netlink attributes. Merge 2 patches to consolidate qinq changes. V10: Implement reviewer comments: Consolidate vlan parsing functions. Splits netlink parsing and flow conversion into a separate patch. Uses double encap attribute encapsulation for 802.1ad. Netlink attributes now look like this: eth_type(0x88a8),vlan(vid=100),encap(eth_type(0x8100), vlan(vid=200), encap(eth_type(0x0800), ...)) The double encap atributes in this version of the patch is incompatible with old versions of the user level 802.1ad patch. A new user level patch which is also being submitted simultaneously to openvswitch dev mailing list. V9: Includes changes suggested by reviewers V8: Includes changes suggested by reviewers V7: Includes changes suggested by reviewers V6: Rebased to net-next V5: Use encapsulated attributes For discussion, history and previous versions of the kernel module patch and the user code patch see the OVS dev mailing list, openvswitch.org/pipermail/dev/.. Thomas F Herbert (3): openvswitch: 802.1ad uapi changes. Check for vlan ethernet types for 8021.q or 802.1ad 802.1AD: Flow handling, actions, vlan parsing and netlink attributes include/linux/if_vlan.h | 16 include/uapi/linux/openvswitch.h | 17 ++-- net/openvswitch/actions.c| 6 +- net/openvswitch/flow.c | 75 + net/openvswitch/flow.h | 8 +- net/openvswitch/flow_netlink.c | 169 +++ net/openvswitch/vport-netdev.c | 4 +- 7 files changed, 253 insertions(+), 42 deletions(-) -- 2.4.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net-next V16 2/3] Check for vlan ethernet types for 8021.q or 802.1ad
---
include/linux/if_vlan.h | 16
1 file changed, 16 insertions(+)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 67ce5bd..ac23521 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -627,6 +627,22 @@ static inline netdev_features_t vlan_features_check(const
struct sk_buff *skb,
return features;
}
+/**
+ * eth_type_vlan - check for valid vlan ether type.
+ * @ethertype: ether type to check
+ *
+ * Returns true if the ether type is a vlan ether type.
+ */
+static inline bool eth_type_vlan(__be16 ethertype)
+{
+ switch (ethertype) {
+ case (htons(ETH_P_8021Q)):
+ case (htons(ETH_P_8021AD)):
+ return true;
+ default:
+ return false;
+ }
+}
/**
* compare_vlan_header - Compare two vlan headers
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/3] ovn-controller.8: Correct reference to "ovn-encap-type".
On Wed, Oct 14, 2015 at 10:38:05PM -0700, Justin Pettit wrote: > Signed-off-by: Justin Pettit Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovs-vswitchd.conf.db.5: Update docs for max-idle.
On 14 October 2015 at 21:55, Justin Pettit wrote: > >> On Oct 14, 2015, at 8:28 PM, Joe Stringer wrote: >> >>> Do you want to make that change too? >> >> Sure. Is it something we want to backport or just apply to master? > > I'd just apply it to master. I don't think we've had any issues with it. OK, I folded the incremental into the patch and pushed it to master. I also pushed a similar incremental as a separate patch for flow-limit. Thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/3] ovn.at: Fix tab/space issues.
On Thu, Oct 15, 2015 at 01:59:18AM -0700, Justin Pettit wrote: > > > On Oct 15, 2015, at 1:42 AM, Justin Pettit wrote: > > > > > >> On Oct 14, 2015, at 10:40 PM, Ben Pfaff wrote: > >> > >> On Wed, Oct 14, 2015 at 10:38:06PM -0700, Justin Pettit wrote: > >>> Signed-off-by: Justin Pettit > >> > >> Acked-by: Ben Pfaff > > > > I pushed this patch and the first. It looks like the second one got > > delayed, but you should have it now. > > Scratch reviewing that. My vtep series depended on that patch, so I > just resent it as part of the series. I try not to resent patches. I just review them. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/3] ovn.at: Fix tab/space issues.
> On Oct 15, 2015, at 9:51 AM, Ben Pfaff wrote: > > On Thu, Oct 15, 2015 at 01:59:18AM -0700, Justin Pettit wrote: >> >> Scratch reviewing that. My vtep series depended on that patch, so I >> just resent it as part of the series. > > I try not to resent patches. I just review them. After about four hours of working on that gateway test, I was feeling pretty resentful. Luckily, the process brought out a couple of issues. --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] openvswitch: Allocate memory for ovs internal device stats.
On Thu, Oct 15, 2015 at 7:54 AM, James Morse wrote:
> "openvswitch: Remove vport stats" removed the per-vport statistics, in
> order to use the netdev's statistics fields.
> "openvswitch: Fix ovs_vport_get_stats()" fixed the export of these stats
> to user-space, by using the provided netdev_ops to collate them - but ovs
> internal devices still use an unallocated dev->tstats field to count
> packets, which are not exported by this api.
>
> Allocate the dev->tstats field for ovs internal devices, and wire up
> ndo_get_stats64 with the original implementation of
> ovs_vport_get_stats().
>
> On its own, "openvswitch: Fix ovs_vport_get_stats()" fixes the OOPs,
> unmasking a full-on panic on arm64:
> =%<==
> [] internal_dev_recv+0xa8/0x170 [openvswitch]
> [] do_output.isra.31+0x60/0x19c [openvswitch]
> [] do_execute_actions+0x208/0x11c0 [openvswitch]
> [] ovs_execute_actions+0xc8/0x238 [openvswitch]
> [] ovs_packet_cmd_execute+0x21c/0x288 [openvswitch]
> [] genl_family_rcv_msg+0x1b0/0x310
> [] genl_rcv_msg+0xa4/0xe4
> [] netlink_rcv_skb+0xb0/0xdc
> [] genl_rcv+0x38/0x50
> [] netlink_unicast+0x164/0x210
> [] netlink_sendmsg+0x304/0x368
> [] sock_sendmsg+0x30/0x4c
> [SNIP]
> Kernel panic - not syncing: Fatal exception in interrupt
> =%<==
>
> Signed-off-by: James Morse
> Fixes: 8c876639c985 ("openvswitch: Remove vport stats.")
> ---
> Hi netdev,
>
> "openvswitch: Fix ovs_vport_get_stats()" - already applied according to
> [0] (where?), is an incomplete fix for the issue in "openvswitch: Remove
> vport stats.". Use of an unallocated dev->tstats remains for ovs internal
> devices, which causes a panic on arm64. Could this patch and "openvswitch:
> Fix ovs_vport_get_stats()" be considered as fixes for v4.3?
>
You need to target this patch for net branch. So that the fix will get
into v4.3.
> Thanks!
>
> James Morse
>
>
> [0] https://patchwork.ozlabs.org/patch/525827/
>
> net/openvswitch/vport-internal_dev.c | 43
> +++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/vport-internal_dev.c
> b/net/openvswitch/vport-internal_dev.c
> index 388b8a6bf112..758e53cb7a81 100644
> --- a/net/openvswitch/vport-internal_dev.c
> +++ b/net/openvswitch/vport-internal_dev.c
> @@ -106,12 +106,48 @@ static void internal_dev_destructor(struct net_device
> *dev)
> free_netdev(dev);
> }
>
> +static struct rtnl_link_stats64 *
> +internal_get_stats(struct net_device *dev, struct rtnl_link_stats64 *stats)
> +{
> + int i;
> +
> + memset(stats, 0, sizeof(*stats));
> + stats->rx_errors = dev->stats.rx_errors;
> + stats->tx_errors = dev->stats.tx_errors;
> + stats->tx_dropped = dev->stats.tx_dropped;
> + stats->rx_dropped = dev->stats.rx_dropped;
> +
> + stats->rx_dropped += atomic_long_read(&dev->rx_dropped);
> + stats->tx_dropped += atomic_long_read(&dev->tx_dropped);
> +
dev_get_stats() already account for dev->rx and tx dropped packets. So
there is no need to add it here.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/5] ovn-controller.8: Correct reference to "ovn-encap-type".
On Thu, Oct 15, 2015 at 01:54:56AM -0700, Justin Pettit wrote: > Signed-off-by: Justin Pettit Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/5] ovn-sbctl: Add multiple encaps with "chassis-add".
On Thu, Oct 15, 2015 at 01:54:57AM -0700, Justin Pettit wrote: > Signed-off-by: Justin Pettit Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovn: Add stateful ACL support.
Add support for the "allow-related" ACL action. This is dependent on
the OVS conntrack functionality, which is not available on all platforms
or kernel versions.
Here is a sample policy that will allow all tenants in logical switch
"ls0" to SSH to each other. Anyone can make an HTTP request to "lp0".
All other IP traffic is dropped:
ovn-nbctl acl-add ls0 from-lport 100 ip allow-related
ovn-nbctl acl-add ls0 to-lport 100 tcp.dst==22 allow-related
ovn-nbctl acl-add ls0 to-lport 100 "outport == \"lp0\" \
&& tcp.dst==80" allow-related
ovn-nbctl acl-add ls0 to-lport 1 ip drop
Note: Kernel conntrack support is checked into the mainline Linux
kernel, but hasn't been backported to the main OVS repo yet.
---
I've pushed this patch on a partial backport of conntrack here:
https://github.com/justinpettit/ovs/tree/ovn-acl
---
ovn/TODO|8 ++
ovn/controller/binding.c| 51 +-
ovn/controller/binding.h|4 +-
ovn/controller/lflow.c | 16 +++-
ovn/controller/lflow.h |8 +-
ovn/controller/ovn-controller.8.xml |5 +
ovn/controller/ovn-controller.c | 33 ++-
ovn/controller/ovn-controller.h |4 +
ovn/controller/physical.c | 16 +++-
ovn/controller/physical.h |3 +-
ovn/lib/actions.c | 48 -
ovn/lib/actions.h | 13 ++-
ovn/northd/ovn-northd.c | 196 ++-
ovn/ovn-architecture.7.xml |8 ++
ovn/ovn-sb.xml | 60 ++--
tests/ovn.at|2 +
tests/test-ovn.c|8 +-
17 files changed, 400 insertions(+), 83 deletions(-)
diff --git a/ovn/TODO b/ovn/TODO
index a48251f..8d22c2c 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -137,3 +137,11 @@
Both ovn-controller and ovn-contorller-vtep should use BFD to
monitor the tunnel liveness. Both ovs-vswitchd schema and
VTEP schema supports BFD.
+
+* ACL
+
+** Support FTP ALGs.
+
+** Support reject action.
+
+** Support log option.
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index fca2430..9759312 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -16,6 +16,7 @@
#include
#include "binding.h"
+#include "lib/bitmap.h"
#include "lib/sset.h"
#include "lib/util.h"
#include "lib/vswitch-idl.h"
@@ -71,9 +72,55 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
struct sset *lports)
}
}
+static void
+update_ct_zones(struct sset *lports, struct simap *ct_zones,
+unsigned long *ct_zone_bitmap)
+{
+struct simap_node *ct_zone, *ct_zone_next;
+const char *iface_id;
+int scan_start = 1;
+
+/* xxx This is wasteful to assign a zone to each port--even if no
+ * xxx security policy is applied. */
+
+/* Delete any zones that are associated with removed ports. */
+SIMAP_FOR_EACH_SAFE(ct_zone, ct_zone_next, ct_zones) {
+if (!sset_contains(lports, ct_zone->name)) {
+bitmap_set0(ct_zone_bitmap, ct_zone->data);
+simap_delete(ct_zones, ct_zone);
+}
+}
+
+/* Assign a unique zone id for each logical port. */
+SSET_FOR_EACH(iface_id, lports) {
+size_t zone;
+
+if (simap_contains(ct_zones, iface_id)) {
+continue;
+}
+
+/* We assume that there are 64K zones and that we own them all. */
+zone = bitmap_scan(ct_zone_bitmap, 0, scan_start, MAX_CT_ZONES + 1);
+if (zone == MAX_CT_ZONES + 1) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+VLOG_WARN_RL(&rl, "exhausted all ct zones");
+return;
+}
+scan_start = zone + 1;
+
+bitmap_set1(ct_zone_bitmap, zone);
+simap_put(ct_zones, iface_id, zone);
+
+/* xxx We should erase any old entries for this
+ * xxx zone, but we need a generic interface to the conntrack
+ * xxx table. */
+}
+}
+
void
binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
-const char *chassis_id)
+const char *chassis_id, struct simap *ct_zones,
+unsigned long *ct_zone_bitmap)
{
const struct sbrec_chassis *chassis_rec;
const struct sbrec_port_binding *binding_rec;
@@ -97,6 +144,7 @@ binding_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
/* We have no integration bridge, therefore no local logical ports.
* We'll remove our chassis from all port binding records below. */
}
+update_ct_zones(&lports, ct_zones, ct_zone_bitmap);
sset_clone(&all_lports, &lports);
ovsdb_idl_txn_add_comment(
@@ -141,6 +189,7 @@ binding_cleanup(struct controller_ctx *ctx, const char
*chassis_id)
if (!chassis_id) {
return true;
}
+
const struct sbrec_chassis *chassis_rec
= get_chassis_by_name(c
Re: [ovs-dev] OVN: V2 RFC add a new JSON-RPC selective monitoring method
Ben Pfaff wrote on 15/10/2015 02:44:44 AM: > On Wed, Sep 23, 2015 at 08:57:38PM +0300, Liran Schour wrote: > > Here is an update for the proposed OVSDB protocol specification (RFC > > 7047)change suggested for overcoming OVN scalability issues by allowing > > clients to monitor only rows that meet specific criteria > > (http://openvswitch.org/pipermail/dev/2015-August/059149.html) > > > > Original proposal (v1): > > http://openvswitch.org/pipermail/dev/2015-August/059441.html > > Discussion: > > http://openvswitch.org/pipermail/dev/2015-September/059681.html > > > > Changes v1 -> v2: > >* Add "columns" member to request object to specify > > which columns should be monitored > >* Clarify behavior when a row modification matches monitored conditions > >* Add "modified" member to object to specify sending > > only changed columns as part of update notification > > Hi, sorry it took so long to review this. I think it's fine on the > basis of what I was thinking about this before. > > It's probably best to coordinate with Andy on implementation, since I'd > rather end up with two versions of monitor rather than three. I think > that Andy is already writing code, so maybe it would make sense to wait > for him to post his changes before you start on yours (unless you've > already started!). > OK. Andy, tell me when you post your changes so I will be able to base mine on top of yours. > However, another issue occurs to me now, and I'd like your opinion on it > before we proceed. Once monitor supports a "where" clause, it seems > likely to me that from time to time a client will want to change that > clause on some of the tables it monitors. For example, in OVN a "where" > clause might match rows that are relevant to the logical switches that > are present on a given chassis. If a new VIF comes up, then another > logical switch could then be relevant and need to be added to the > "where" clause. > > One way to implement this kind of a change is for the client to cancel > its previous monitor and create a new one. But that is inefficient > because the server will have to send and the client will have to receive > and parse all of the rows that match the new monitor, instead of just > the rows newly matched by the modified clause. So it would be better to > allow a client to modify a monitor rather than just to create and cancel > them. > > Have you thought about this? > My initial thoughts were that clients (ovn-controllers) can have multiple monitor sessions (e.g. one per logical switch) with a single monitor condition each. Now I agree we should allow the clients to monitor a set of conditions in one session and to be able to modify this set. Therefore I propose to add another request method to the OVSDB protocol called "monitor_cond_change" that will specify the changes on the set of conditions. A simple usage of this new method will be to start a monitor_cond session with empty conditions array by the ovn-controller. When a new VM will be attached a monitor_cond_change will be sent with a condition matching the new logical switch to be monitored and vice versa when detaching VMs from the host. Here is the spec: --- Monitor_cond_change: The "monitor_cond_change" request enables a client to change an existing conditional replication of the database by specifying a new set of conditions for each replicated table. The request object has the following members: * "method": "monitor_cond_change" * "params": [, , ] * "id": The parameter should have a value of an existing conditional monitoring session from this client. The object maps the name of the table to an array of . Each is an object with the following members: * "added": [*] new set of conditions to be added to the existing set of conditions for this table. * "removed": [*] existing conditions to be removed from replication. is specified in Section 5.1 in the RFC. The response object has the following members: * "result": * "error": null * "id": same "id" as request The object is described in detail in Section 4.1.6. If initial contents are requested by origin monitor_cond request, will contain any newly matched rows by "added" conditions. If deleted contents are requested by origin monitor request, will contain any matched rows by "removed" conditions. Changes according to the new conditions are automatically sent to the client using the "update" monitor notification (see Section 4.1.6). In case that newly set of conditions is empty, no update notifications will be sent. --- Thanks, - Liran ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v4 02/12] ovs-ofctl: Add bundle support and unit testing.
> On Oct 14, 2015, at 9:35 PM, Takashi Yamamoto wrote: > > hi, > > On Wed, Jun 10, 2015 at 9:24 AM, Jarno Rajahalme > wrote: >> >> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at >> index 1e12827..b7db9bb 100644 >> --- a/tests/ovs-ofctl.at >> +++ b/tests/ovs-ofctl.at >> @@ -2813,3 +2813,110 @@ AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | >> ofctl_strip | sed '/OFPST_FLO >> >> OVS_VSWITCHD_STOP >> AT_CLEANUP >> + >> +AT_SETUP([ovs-ofctl replace-flows with --bundle]) >> +OVS_VSWITCHD_START >> + >> +AT_CHECK([ovs-appctl vlog/set vconn:dbg]) >> + >> +dnl Add flows to br0 with importance via OF1.4+, using an OF1.4+ bundle. >> For more details refer "ovs-ofctl rule with importance" test case. >> +for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i,actions=drop"; >> done > add-flows.txt >> +AT_CHECK([ovs-ofctl --bundle add-flows br0 add-flows.txt]) >> + >> +dnl Replace some flows in the bridge. >> +for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i + >> 10`,actions=drop"; done > replace-flows.txt >> +AT_CHECK([ovs-ofctl --bundle replace-flows br0 replace-flows.txt]) >> + >> +dnl Dump them and compare the dump flows output against the expected output. >> +for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then >> importance=`expr $i + 10`; else importance=$i; fi; echo " >> importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout >> +AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed >> '/OFPST_FLOW/d' | sort], >> + [0], [expout]) >> + >> +dnl Check logs for OpenFlow trace >> +# Prevent race. >> +OVS_WAIT_UNTIL([test `grep -- "|vconn|DBG|unix: sent (Success): OFPST_FLOW >> reply" ovs-vswitchd.log | wc -l` -ge 2]) >> +# AT_CHECK([sed -n "s/^.*\(|vconn|DBG|.*xid=.*:\).*$/\1/p" >> ovs-vswitchd.log], [0], [dnl >> +AT_CHECK([print_vconn_debug | ofctl_strip], [0], [dnl >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> + version bitmap: 0x01, 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 >> and earlier, peer supports versions 0x01, 0x05) >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=OPEN_REPLY flags=0 >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:1 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=2 importance:2 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=3 importance:3 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=4 importance:4 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=5 importance:5 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=6 importance:6 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=7 importance:7 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): >> + bundle_id=0 flags=ordered >> +OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=8 importance:8 actions=drop >> +vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REQUEST flags=ordered >> +vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): >> + bundle_id=0 type=COMMIT_REPLY flags=0 >> +vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): >> + version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 >> +vconn|DBG|unix: received: OFPT_HELLO (OF1.4): >> + version bitmap: 0x01, 0x05 >> +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 >> and earlier, peer supports versions 0x01, 0x05) >> +vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): >> +vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): > > can you explain why this flow-stats reply is expected to be empty? > i'm seeing a test failure where it containing flows added in the > previous add-flows > on my environment. do you have any idea? > It appears that ovs-ofctl replace-flows had not been properly updated when the out_group member was added to struct ofputil_flow_stats_request. I’ll send a patch to fix this in a moment. Thanks for reporting this! Jarno ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/5] ovn-controller: Support VXLAN enapsulation.
On Thu, Oct 15, 2015 at 01:54:58AM -0700, Justin Pettit wrote: > Signed-off-by: Justin Pettit It looks like physical_run() will now iterate through every row in the Port_Binding table for each VXLAN tunnel. That seems expensive. Here's a suggested documentation update. diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml index 47dfc2a..7898f21 100644 --- a/ovn/ovn-architecture.7.xml +++ b/ovn/ovn-architecture.7.xml @@ -634,24 +634,39 @@ logical input port field - A field that denotes the logical port from which the packet - entered the logical datapath. - - OVN stores this in Nicira extension register number 6. (This field is - passed across tunnels as part of the tunnel key.) + +A field that denotes the logical port from which the packet +entered the logical datapath. + +OVN stores this in Nicira extension register number 6. + + + +Geneve and STT tunnel pass this field as part of the tunnel key. +Although VXLAN tunnels do not explicitly carry a logical input port, +OVN only uses VXLAN to communicate with gateways that from OVN's +perspective consist of only a single logical port, so that OVN can set +the logical input port field to this one on ingress to the OVN logical +pipeline. + logical output port field - A field that denotes the logical port from which the packet will - leave the logical datapath. This is initialized to 0 at the - beginning of the logical ingress pipeline. - - OVN stores this in - Nicira extension register number 7. (This field is passed across - tunnels as part of the tunnel key.) + +A field that denotes the logical port from which the packet will +leave the logical datapath. This is initialized to 0 at the +beginning of the logical ingress pipeline. + +OVN stores this in Nicira extension register number 7. + + + +Geneve and STT tunnels pass this field as part of the tunnel key. +VXLAN tunnels do not transmit the logical output port field. + VLAN ID Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 4/5] ovn-controller: Support multiple encaps simultaneously.
On Thu, Oct 15, 2015 at 01:54:59AM -0700, Justin Pettit wrote:
> Signed-off-by: Justin Pettit
The chassis.c code only verifies the IP address for one of the
encapsulations, maybe it should do it for all of them, something like
this:
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 894000d..84791c0 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -90,13 +90,16 @@ chassis_run(struct controller_ctx *ctx, const char
*chassis_id)
free(tokstr);
if (chassis_rec) {
+/* Compare desired tunnels against those currently in the database. */
uint32_t cur_tunnels = 0;
+bool same = true;
for (int i = 0; i < chassis_rec->n_encaps; i++) {
cur_tunnels |= get_tunnel_type(chassis_rec->encaps[i]->type);
+same = same && strcmp(chassis_rec->encaps[i]->ip, encap_ip);
}
+same = same && req_tunnels == cur_tunnels;
-if (req_tunnels == cur_tunnels
-&& !strcmp(chassis_rec->encaps[0]->ip, encap_ip)) {
+if (same) {
/* Nothing changed. */
inited = true;
return;
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 5/5] ovn.at: Add test for gateway.
On Thu, Oct 15, 2015 at 01:55:00AM -0700, Justin Pettit wrote: > This test exposed a problem that ovn-controller-vtep doesn't properly > set up the "Mcast_Macs_Remote" table, which prevents broadcasts from > being sourced from the physical side of the VTEP. That issue needs to > be resolved, and then the full set of gateway traffic patterns can run. > > Signed-off-by: Justin Pettit It looks like our test code is very promising. Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 01/23] ovn: Extend logical "next" action to jump to arbitrary flow tables.
> On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote:
>
> @@ -30,8 +30,10 @@ struct action_context {
> /* Input. */
> 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.
> */
> +uint8_t first_table;/* First OpenFlow table. */
> +uint8_t n_tables; /* Number of OpenFlow tables. */
I think these two descriptions could be a bit misleading, since they define the
range of tables that a "next" action can jump to, but it sounds like it's the
full range of supported OpenFlow tables.
> +uint8_t cur_table; /* 0 <= cur_table < n_tables. */
This member name seems a little confusing, since all the other "_table" members
refer to an OpenFlow port, but this one is an offset. What about something
like "cur_table_offset"? Another option would be to relabel them things like
"*_of_table" (or "*_phy_table") and "*_log_table".
> +static void
> +parse_next_action(struct action_context *ctx)
> +{
> +if (!ctx->n_tables) {
> +action_error(ctx, "\"next\" action not allowed here.");
> +} else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
> +int table;
Similar to above, this is actually a logical table or an offset to an OpenFlow
table. It might be nice to clarify that for later readers.
This is a nice addition, but I think some of the older phrasing related to
logical and physical tables was a bit clearer than exposing that they're
offsets.
Acked-by: Justin Pettit
--Justin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH ] debian: place kernel module to satisfy depmod search.
On 10/14/2015 04:58 PM, Ansis Atteka wrote: On Wed, Oct 14, 2015 at 4:08 PM, Ben Pfaff wrote: On Wed, Oct 14, 2015 at 03:28:24PM -0700, Joe Stringer wrote: On 14 October 2015 at 15:21, Ben Pfaff wrote: On Tue, Oct 06, 2015 at 04:35:32PM -0700, Saurabh Mohan wrote: On Ubuntu depmod's search priority is configured in /etc/depmod to be updates and then the kernel built-in directory. $ cat /etc/depmod.d/ubuntu.conf search updates ubuntu built-in Thus change the placement of openvswitch.ko under updates/ not kernel/updates. Signed-off-by: Saurabh Mohan This appears to be correct, but I'm confused about how this could have not been noticed for years. Did something change recently? We recently changed it from kernel/ to kernel/updates (prior to v2.4 release), and the commit message suggests it was previously nondeterministic: commit b519432205c36bda5c7331f77a49eaaa919967ad Author: Ansis Atteka Date: Tue May 26 16:49:49 2015 -0700 debian: install openvswitch kernel module under "updates" directory This patch fixes a bug where "modprobe openvswitch" command on Ubuntu distribution would have sometimes tried to load OVS kernel module that shipped together with Linux Kernel, even though one had also installed OVS datapath debian package created with module-assistant. Because of this issue force-reload-kmod command occasionally malfunctioned and failed to load the right kernel module. This bug happened *occasionally* because the default Ubuntu depmod configuration in /etc/depmod.d/ubuntu.conf is set to look for kernel modules first in "updates" directory, then in "ubuntu" directory and then in other directories. If there were two openvswitch.ko modules in "other directories", then modprobe would have loaded kernel module that was nondeterministically listed first by file system. OK, I understand why it was nondeterministic before, but where does kernel/updates come in then, since it seems to be different from and not as high-priority as "updates"? Does anyone know? I am still trying to find the answer in my email history why I ended up using "kernel/updates" over "updates". Saurabh, did you encounter an issue where the wrong kernel module was loaded or is this to achieve conformance? (resending it again as previously i did not include everyone. Sorry) Ansis, we tried using this patch but still noticed that the wrong kernel module was getting selected. The only way to fix it was to put the module outside kernel/ directory. Thanks, Ben. ___ 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 02/23] ovn: Implement logical action to decrement IPv4 TTL.
> On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > This is necessary for IPv4 routing. > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 03/23] ovn: Change h1 titles to title case in documentation.
> On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > Manpage section titles are traditionally all-uppercase, but OVS's > XML-to-nroff translator takes care of that and there's no need to actually > provide them in all-caps (and it looks ugly). > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH ] debian: place kernel module to satisfy depmod search.
On Wed, Oct 14, 2015 at 5:33 PM, Saurabh Mohan wrote: > On 10/14/2015 04:58 PM, Ansis Atteka wrote: >> >> On Wed, Oct 14, 2015 at 4:08 PM, Ben Pfaff wrote: >>> >>> On Wed, Oct 14, 2015 at 03:28:24PM -0700, Joe Stringer wrote: On 14 October 2015 at 15:21, Ben Pfaff wrote: > > On Tue, Oct 06, 2015 at 04:35:32PM -0700, Saurabh Mohan wrote: >> >> On Ubuntu depmod's search priority is configured in /etc/depmod to be >> updates and then the kernel built-in directory. >> $ cat /etc/depmod.d/ubuntu.conf >> search updates ubuntu built-in >> >> Thus change the placement of openvswitch.ko under updates/ not >> kernel/updates. >> >> Signed-off-by: Saurabh Mohan > > > This appears to be correct, but I'm confused about how this could have > not been noticed for years. Did something change recently? We recently changed it from kernel/ to kernel/updates (prior to v2.4 release), and the commit message suggests it was previously nondeterministic: commit b519432205c36bda5c7331f77a49eaaa919967ad Author: Ansis Atteka Date: Tue May 26 16:49:49 2015 -0700 debian: install openvswitch kernel module under "updates" directory This patch fixes a bug where "modprobe openvswitch" command on Ubuntu distribution would have sometimes tried to load OVS kernel module that shipped together with Linux Kernel, even though one had also installed OVS datapath debian package created with module-assistant. Because of this issue force-reload-kmod command occasionally malfunctioned and failed to load the right kernel module. This bug happened *occasionally* because the default Ubuntu depmod configuration in /etc/depmod.d/ubuntu.conf is set to look for kernel modules first in "updates" directory, then in "ubuntu" directory and then in other directories. If there were two openvswitch.ko modules in "other directories", then modprobe would have loaded kernel module that was nondeterministically listed first by file system. >>> >>> >>> OK, I understand why it was nondeterministic before, but where does >>> kernel/updates come in then, since it seems to be different from and not >>> as high-priority as "updates"? Does anyone know? >> >> >> I am still trying to find the answer in my email history why I ended >> up using "kernel/updates" over "updates". >> >> Saurabh, did you encounter an issue where the wrong kernel module was >> loaded or is this to achieve conformance? >> > > Anis, we tried using this patch but still noticed that the wrong kernel > module was getting selected. The only way to fix it was > to put the module outside kernel/ directory. Can you give me locations of all ovs kernel modules (dkms, module-assistant and the one that came with linux) present in /lib/modules// on your system that was having trouble? I am just wondering why my patch prioritized module-assistant created kernel module (in kernel/updates) over the one that comes with linux kernel itself. I guess this is the problem you are seeing here again, right? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 04/23] ovn-nbctl: Remove external-ids commands.
> On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > Users are served just as well through the general-purpose "set" and "get" > database commands, so avoid the additional code and documentation. > > (ovs-vsctl does have special external-ids commands for bridges, but those > exist because of special handling for "fake bridges".) > > Signed-off-by: Ben Pfaff Every line of code I write (or copy and paste from ovs-vsctl) is like a child to me. Good bye, sweet peas. Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 05/23] nx-match: Serialize match on IP TTL even when outputting OXM.
> On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > The 'oxm' parameter to nxm_put_ip() indicates whether NXM or OXM code > points should be used in cases where both exist. It shouldn't cause > matches to be dropped entirely, since that changes the meaning, but that's > what was done here for matches on the IP (v4 or v6) TTL. This commit > fixes the problem. > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 07/23] ovn-controller: Un-inline get_chassis_by_name().
> On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > I don't know of any reason to inline this. Well, obviously because it's on the critical path. It has to do with registers and CPU caches and stuff--you wouldn't understand. > Also rename for consistency with get_bridge(). > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 08/23] ovn-controller: Factor patch port management into new "patch" module.
> On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > This is a proposed plan for logical L3 in OVN. It is not entirely > complete but it includes many important details and I believe that it moves > planning forward. > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 09/23] patch: Bail out earlier if OVS IDL transactions cannot be executed.
> On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > The whole point of this module is side effects on the Open vSwitch > database, so the whole thing can be skipped if those are impossible. > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/4] Summary: From: Jarno Rajahalme
ovs-ofctl: Fix replace-flows. The replace-flows test cases tested for incorrect behavior due to the missing initialization of the out_group member of struct ofputil_flow_stats_request. This patch fixes this by properly initializing out_group to OFPG_ANY. Note that replace-flows still does not support multiple tables, but that will be fixed in a later patch in the series. Reported-by: Takashi Yamamoto Signed-off-by: Jarno Rajahalme --- tests/ovs-ofctl.at| 34 +- utilities/ovs-ofctl.c | 2 ++ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 33e67ed..6f03adb 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -2851,12 +2851,12 @@ dnl Add flows to br0 with importance via OF1.4+. For more details refer "ovs-ofc for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i,actions=drop"; done > add-flows.txt AT_CHECK([ovs-ofctl -O OpenFlow14 add-flows br0 add-flows.txt]) -dnl Replace some flows in the bridge. -for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i + 10`,actions=drop"; done > replace-flows.txt +dnl Replace the flows in the bridge. +for i in 1 3 5 7; do echo " importance=`expr $i + 10`, dl_vlan=$i actions=drop"; done > replace-flows.txt AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 replace-flows.txt]) dnl Dump them and compare the dump flows output against the expected output. -for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then importance=`expr $i + 10`; else importance=$i; fi; echo " importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout +cat replace-flows.txt > expout AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed '/OFPST_FLOW/d' | sort], [0], [expout]) @@ -2873,11 +2873,11 @@ for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i,actions=drop"; done AT_CHECK([ovs-ofctl --bundle add-flows br0 add-flows.txt]) dnl Replace some flows in the bridge. -for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i + 10`,actions=drop"; done > replace-flows.txt +for i in 1 3 5 7; do echo " importance=`expr $i + 10`, dl_vlan=$i actions=drop"; done > replace-flows.txt AT_CHECK([ovs-ofctl --bundle replace-flows br0 replace-flows.txt]) dnl Dump them and compare the dump flows output against the expected output. -for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then importance=`expr $i + 10`; else importance=$i; fi; echo " importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout +cat replace-flows.txt > expout AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed '/OFPST_FLOW/d' | sort], [0], [expout]) @@ -2930,12 +2930,32 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4): vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): + importance=1, dl_vlan=1 actions=drop + importance=2, dl_vlan=2 actions=drop + importance=3, dl_vlan=3 actions=drop + importance=4, dl_vlan=4 actions=drop + importance=5, dl_vlan=5 actions=drop + importance=6, dl_vlan=6 actions=drop + importance=7, dl_vlan=7 actions=drop + importance=8, dl_vlan=8 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REPLY flags=0 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=2 actions=drop +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): + bundle_id=0 flags=atomic ordered +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=4 actions=drop +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): + bundle_id=0 flags=atomic ordered +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=6 actions=drop +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): + bundle_id=0 flags=atomic ordered +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=8 actions=drop +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): + bundle_id=0 flags=atomic ordered OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:11 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered @@ -2957,10 +2977,6 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4): vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): - importance=2, dl_vlan=2 actions=drop - importance=4, dl_vlan=4 actions=drop - importance=6, dl_vlan=6 actions=drop - importance=8, dl_vlan=8 actions=drop importance=11, dl_vlan=1 actions=drop importance=13, dl_vlan=3 actions=drop importance=15, dl_vlan=5 actio
[ovs-dev] [PATCH 2/4] ovs-ofctl: Fix OpenFlow versions with '--bundle'
While the presence of the '--bundle' option implicitly added the OpenFlow 1.4 to the allowed protocols, it failed to remove OpenFlow 1.0 from the allowed protocols. This is changed so that '--bundle' option now also implicitly removes versions lesser than 1.4 from the allowed protocols. This has no behavioral difference when ovs-ofctl is paired with OVS that supports OpenFlow 1.4, as the greatest common version is negotiated, but prevents negotiation of OpenFlow 1.0 when OVS does not support OpenFlow 1.4. Found by inspection. Signed-off-by: Jarno Rajahalme --- tests/ofproto.at | 12 ++-- tests/ovs-ofctl.at| 8 utilities/ovs-ofctl.c | 3 +++ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/tests/ofproto.at b/tests/ofproto.at index 5e4441c..4c6dd29 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -3962,8 +3962,8 @@ vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY: vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 vconn|DBG|unix: received: OFPT_HELLO (OF1.4): - version bitmap: 0x01, 0x05 -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) + version bitmap: 0x05 +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): @@ -4014,8 +4014,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply: vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 vconn|DBG|unix: received: OFPT_HELLO (OF1.4): - version bitmap: 0x01, 0x05 -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) + version bitmap: 0x05 +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): @@ -4045,8 +4045,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply: vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 vconn|DBG|unix: received: OFPT_HELLO (OF1.4): - version bitmap: 0x01, 0x05 -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) + version bitmap: 0x05 +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 6f03adb..7375cad 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -2889,8 +2889,8 @@ AT_CHECK([print_vconn_debug | vconn_windows_sub | ofctl_strip], [0], [dnl vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 vconn|DBG|unix: received: OFPT_HELLO (OF1.4): - version bitmap: 0x01, 0x05 -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) + version bitmap: 0x05 +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): @@ -2926,8 +2926,8 @@ vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 vconn|DBG|unix: received: OFPT_HELLO (OF1.4): - version bitmap: 0x01, 0x05 -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports versions 0x01, 0x05) + version bitmap: 0x05 +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): importance=1, dl_vlan=1 actions=drop diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index fbc9da4..ee15e1a 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -312,6 +312,9 @@ parse_options(int argc, char *argv[]) /* Add implicit allowance for OpenFlow 1.4. */ add_allowed_ofp_versions(ofputil_protocols_to_version_bitmap( OFPUTIL_P_OF14_OXM)); +/* Remove all prior versions. */ +mask_allowed_ofp_versions(ofputil_protocols_to_version_bitmap( +
[ovs-dev] [PATCH 3/4] ovs-ofctl: Support multiple tables in replace-flows and diff-flows.
Signed-off-by: Jarno Rajahalme --- tests/ovs-ofctl.at| 60 - utilities/ovs-ofctl.c | 174 +++--- 2 files changed, 140 insertions(+), 94 deletions(-) diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 7375cad..e52cbf9 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -2869,11 +2869,11 @@ OVS_VSWITCHD_START AT_CHECK([ovs-appctl vlog/set vconn:dbg]) dnl Add flows to br0 with importance via OF1.4+, using an OF1.4+ bundle. For more details refer "ovs-ofctl rule with importance" test case. -for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i,actions=drop"; done > add-flows.txt +for i in 1 2 3 4 5 6 7 8; do echo "table=$i,dl_vlan=$i,importance=$i,actions=drop"; done > add-flows.txt AT_CHECK([ovs-ofctl --bundle add-flows br0 add-flows.txt]) dnl Replace some flows in the bridge. -for i in 1 3 5 7; do echo " importance=`expr $i + 10`, dl_vlan=$i actions=drop"; done > replace-flows.txt +for i in 1 3 5 7; do echo " table=$i, importance=`expr $i + 10`, dl_vlan=$i actions=drop"; done > replace-flows.txt AT_CHECK([ovs-ofctl --bundle replace-flows br0 replace-flows.txt]) dnl Dump them and compare the dump flows output against the expected output. @@ -2897,28 +2897,28 @@ vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REPLY flags=0 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:1 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:1 dl_vlan=1 importance:1 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=2 importance:2 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:2 dl_vlan=2 importance:2 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=3 importance:3 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:3 dl_vlan=3 importance:3 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=4 importance:4 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:4 dl_vlan=4 importance:4 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=5 importance:5 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:5 dl_vlan=5 importance:5 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=6 importance:6 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:6 dl_vlan=6 importance:6 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=7 importance:7 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:7 dl_vlan=7 importance:7 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=8 importance:8 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:8 dl_vlan=8 importance:8 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=COMMIT_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): @@ -2930,42 +2930,42 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4): vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 and earlier, peer supports version 0x05) vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): - importance=1, dl_vlan=1 actions=drop - importance=2, dl_vlan=2 actions=drop - importance=3, dl_vlan=3 actions=drop - importance=4, dl_vlan=4 actions=drop - importance=5, dl_vlan=5 actions=drop - importance=6, dl_vlan=6 actions=drop - importance=7, dl_vlan=7 actions=drop - importance=8, dl_vlan=8 actions=drop + table=1, importance=1, dl_vlan=1 actions=drop + table=2, importance=2, dl_vlan=2 actions=drop + table=3, importance=3, dl_vlan=3 actions=drop + table=4, importance=4, dl_vlan=4 actions=drop + table=5, importance=5, dl_vlan=5 actions=drop + table=6, importance=6, dl_vlan=6 actions=drop + table=7, importance=7, dl_vlan=7 actions=drop + table=8, importance=8, dl_vlan=8 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REQUEST flags=atomic ordered vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): bundle_id=0 type=OPEN_REPLY flags=0 vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=2 actions=drop +OFPT_FLOW_MOD (OF1.4): ADD table:1 dl_vlan=1 importance:11 actions=drop vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): bundle_id=0 flags=atomic ordered -OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=4 ac
[ovs-dev] [PATCH 4/4] openflow: Remove OFPG11_*
Protocol-independent symbols OFPG_* were already defined in
openflow-common.h, so remove the protocol version dependent symbols.
Found by inspection.
Signed-off-by: Jarno Rajahalme
---
include/openflow/openflow-1.1.h | 18 ++
lib/ofp-parse.c | 4 ++--
lib/ofp-print.c | 2 +-
lib/ofp-util.c | 22 +++---
ofproto/ofproto.c | 4 ++--
utilities/ovs-ofctl.c | 2 +-
6 files changed, 19 insertions(+), 33 deletions(-)
diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index 5b4ba2b..361150a 100644
--- a/include/openflow/openflow-1.1.h
+++ b/include/openflow/openflow-1.1.h
@@ -336,7 +336,7 @@ struct ofp11_flow_mod {
indicates no restriction. */
ovs_be32 out_group; /* For OFPFC_DELETE* commands, require
matching entries to include this as an
-output group. A value of OFPG11_ANY
+output group. A value of OFPG_ANY
indicates no restriction. */
ovs_be16 flags; /* One of OFPFF_*. */
ovs_be16 importance; /* Eviction precedence (OF1.4+). */
@@ -354,20 +354,6 @@ enum ofp11_group_type {
OFPGT11_FF/* Fast failover group. */
};
-/* Group numbering. Groups can use any number up to OFPG_MAX. */
-enum ofp11_group {
-/* Last usable group number. */
-OFPG11_MAX= 0xff00,
-
-/* Fake groups. */
-OFPG11_ALL= 0xfffc, /* Represents all groups for group delete
-commands. */
-OFPG11_ANY= 0x /* Wildcard group used only for flow stats
-requests. Selects all flows regardless
-of group (including flows with no
-group). */
-};
-
/* Bucket for use in groups. */
struct ofp11_bucket {
ovs_be16 len;/* Length the bucket in bytes, including
@@ -425,7 +411,7 @@ struct ofp11_flow_stats_request {
as an output port. A value of OFPP_ANY
indicates no restriction. */
ovs_be32 out_group; /* Require matching entries to include this
- as an output group. A value of OFPG11_ANY
+ as an output group. A value of OFPG_ANY
indicates no restriction. */
uint8_t pad2[4]; /* Align to 64 bits. */
ovs_be64 cookie; /* Require matching entries to contain this
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 8437656..ec05567 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -351,7 +351,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command,
char *string,
fm->out_port = OFPP_ANY;
fm->flags = 0;
fm->importance = 0;
-fm->out_group = OFPG11_ANY;
+fm->out_group = OFPG_ANY;
fm->delete_reason = OFPRR_DELETE;
if (fields & F_ACTIONS) {
act_str = extract_actions(string);
@@ -1165,7 +1165,7 @@ parse_bucket_str(struct ofputil_bucket *bucket, char
*str_, uint8_t group_type,
bucket->weight = group_type == OFPGT11_SELECT ? 1 : 0;
bucket->bucket_id = OFPG15_BUCKET_ALL;
bucket->watch_port = OFPP_ANY;
-bucket->watch_group = OFPG11_ANY;
+bucket->watch_group = OFPG_ANY;
ds_init(&actions);
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index d0c94ce..db60505 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2387,7 +2387,7 @@ ofp_print_group(struct ds *s, uint32_t group_id, uint8_t
type,
if (bucket->watch_port != OFPP_NONE) {
ds_put_format(s, "watch_port:%"PRIu32",", bucket->watch_port);
}
-if (bucket->watch_group != OFPG11_ANY) {
+if (bucket->watch_group != OFPG_ANY) {
ds_put_format(s, "watch_group:%"PRIu32",", bucket->watch_group);
}
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 97b5449..a148ef1 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1713,7 +1713,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
fm->out_group = (ofm->command == OFPFC_DELETE ||
ofm->command == OFPFC_DELETE_STRICT
? ntohl(ofm->out_group)
- : OFPG11_ANY);
+ : OFPG_ANY);
raw_flags = ofm->flags;
} else {
uint16_t command;
@@ -1745,7 +1745,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
fm->importance = 0;
fm->buffer_id = ntohl(ofm->buffer_id);
fm->out_port = u16_to_ofp(ntohs(ofm->out_port));
-fm->out_group = OFPG11_ANY;
+fm->out_group = OFPG_ANY;
raw_flags = ofm->flags;
} else if (raw == OFPRAW_
Re: [ovs-dev] [PATCH 1/4] Summary: From: Jarno Rajahalme
> On Oct 15, 2015, at 2:28 PM, Jarno Rajahalme wrote: > > ovs-ofctl: Fix replace-flows. Sorry about the botched title, will fix for the commit, Jarno ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH net-next V16 3/3] openvswitch: 802.1AD: Flow handling, actions, vlan parsing and netlink attributes
On Thu, Oct 15, 2015 at 7:01 AM, Thomas F Herbert
wrote:
> Add support for 802.1ad including the ability to push and pop double
> tagged vlans. Add support for 802.1ad to netlink parsing and flow
> conversion. Uses double nested encap attributes to represent double
> tagged vlan. Inner TPID encoded along with ctci in nested attributes.
>
> Signed-off-by: Thomas F Herbert
> ---
> net/openvswitch/actions.c | 6 +-
> net/openvswitch/flow.c | 75 ++
> net/openvswitch/flow.h | 8 +-
> net/openvswitch/flow_netlink.c | 169
> +
> net/openvswitch/vport-netdev.c | 4 +-
> 5 files changed, 228 insertions(+), 34 deletions(-)
>
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 315f533..09cc1c9 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -236,7 +236,8 @@ static int pop_vlan(struct sk_buff *skb, struct
> sw_flow_key *key)
> if (skb_vlan_tag_present(skb))
> invalidate_flow_key(key);
> else
> - key->eth.tci = 0;
> + key->eth.vlan.tci = 0;
> + key->eth.vlan.tpid = 0;
> return err;
> }
>
> @@ -246,7 +247,8 @@ static int push_vlan(struct sk_buff *skb, struct
> sw_flow_key *key,
> if (skb_vlan_tag_present(skb))
> invalidate_flow_key(key);
> else
> - key->eth.tci = vlan->vlan_tci;
> + key->eth.vlan.tci = vlan->vlan_tci;
> + key->eth.vlan.tpid = vlan->vlan_tpid;
> return skb_vlan_push(skb, vlan->vlan_tpid,
> ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
> }
> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index c8db44a..8a4e298 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -302,24 +302,69 @@ static bool icmp6hdr_ok(struct sk_buff *skb)
> sizeof(struct icmp6hdr));
> }
>
> -static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
> +struct qtag_prefix {
> + __be16 eth_type; /* ETH_P_8021Q or ETH_P_8021AD */
> + __be16 tci;
> +};
> +
Now we can just use newly defined struct vlan_header here.
> +/* Parse vlan tag from vlan header.
> + * Returns ERROR on memory error.
> + * Returns 0 if it encounters a non-vlan or incomplete packet.
> + * Returns 1 after successfully parsing vlan tag.
> + */
> +
> +static int parse_vlan_tag(struct sk_buff *skb, __be16 vlan_proto,
> + __be16 vlan_tci, struct vlan_head *vlan)
> {
> - struct qtag_prefix {
> - __be16 eth_type; /* ETH_P_8021Q */
> - __be16 tci;
> - };
> - struct qtag_prefix *qp;
> + if (likely(!eth_type_vlan(vlan_proto)))
> + return 0;
>
> if (unlikely(skb->len < sizeof(struct qtag_prefix) + sizeof(__be16)))
> return 0;
>
> if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
> -sizeof(__be16
> +sizeof(__be16
> return -ENOMEM;
>
> - qp = (struct qtag_prefix *) skb->data;
> - key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
> + vlan->tci = vlan_tci | htons(VLAN_TAG_PRESENT);
> + vlan->tpid = vlan_proto;
> +
> __skb_pull(skb, sizeof(struct qtag_prefix));
> + return 1;
> +}
> +
> +static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
> +{
> + struct qtag_prefix *qp = (struct qtag_prefix *)skb->data;
> + int res;
> +
> + if (likely(skb_vlan_tag_present(skb))) {
> + key->eth.vlan.tci = htons(skb->vlan_tci);
> + key->eth.vlan.tpid = skb->vlan_proto;
> +
> + /* Case where ingress processing has already stripped
> +* the outer vlan tag.
> +*/
> + res = parse_vlan_tag(skb, qp->eth_type, qp->tci,
> +&key->eth.cvlan);
> + if (res < 0)
> + return res;
> + /* For inner tag, return 0 because neither
> +* non-existant nor partial inner tag is an error.
> +*/
> + return 0;
> + }
> + res = parse_vlan_tag(skb, qp->eth_type, qp->tci, &key->eth.vlan);
> + if (res <= 0)
> + /* This is an outer tag in the non-accelerated VLAN
> +* case. Return error unless it is a complete vlan tag.
> +*/
> + return res;
> +
> + /* Parse inner vlan tag if present for non-accelerated case. */
> + res = parse_vlan_tag(skb, qp->eth_type, qp->tci, &key->eth.cvlan);
> + if (res <= 0)
> + return res;
>
same qp pointer is passed for inner and outer vlan parameters here. It
is better to just pass skb and keep qp inside parse_vlan_tag()
function.
> return 0;
> }
> @@
Re: [ovs-dev] [PATCH 08/23] ovn-controller: Factor patch port management into new "patch" module.
After reviewing patch 10, I have a few more thoughts on this patch. This
doesn't change my original ACK, though.
> +static void
> +create_patch_port(struct controller_ctx *ctx,
> + const char *network,
> + const struct ovsrec_bridge *b1,
> + const struct ovsrec_bridge *b2)
> +{
This could use a comment and maybe a renaming of some arguments. In
particular, it isn't obvious the importance between "b1" and "b2". Also, it
might be worth noting that it only creates one side of the patch port.
> +static void
> +create_patch_ports(struct controller_ctx *ctx,
> + const char *network,
> + struct shash *existing_ports,
> + const struct ovsrec_bridge *b1,
> + const struct ovsrec_bridge *b2)
This could use a comment, since many of the arguments are non-obvious. In
particular, how "existing_ports" will be modified and the difference between
"b1" and "b2".
> +static void
> +parse_bridge_mappings(struct controller_ctx *ctx,
> + const struct ovsrec_bridge *br_int,
> + const char *mappings_cfg)
This function does a lot more than just parse the bridge mappings. It could
probably use a better name, but, at the very least, I think a comment would be
helpful--especially for the non-obvious things like what will happen to
"mappings_cfg".
--Justin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 10/23] patch: Refactor to better support new kinds of patches.
> On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > +/* Add any patch ports that should exist but don't. */ > +parse_bridge_mappings(ctx, br_int, &existing_ports); This comment could probably use a bit more information. > +/* Delete any patch ports that do exist but shouldn't. (Any that both > + * should and do exist were removed above.) */ I think it might be a bit clearer if you replaced "above" with 'from "existing_ports"' or something similar. Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] physical: Improve treatment of localnet non-VLAN logical ports.
Until now, the flow table treated localnet logical ports that have a VLAN
quite differently from those that don't. The ones without a VLAN were
essentially trunk ports: any packets that came in, that weren't picked off
by a localnet port with a VLAN, were passed to the ones without a VLAN.
This wasn't the intended behavior.
This commit changes behavior to the intended behavior. Now, localnet ports
without a specific VLAN only receive packets without a VLAN header or those
with VLAN ID 0 (with that header stripped off).
Found by inspection.
Signed-off-by: Ben Pfaff
---
ovn/controller/physical.c | 30 --
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 0c239df..04b869c 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -637,15 +637,12 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
}
hmap_destroy(&tunnels);
-/* Table 0, Priority 150 and 100.
- * ==
+/* Table 0, Priority 100.
+ * ==
*
* We have now determined the full set of port bindings associated with
* each "localnet" network. Only create flows for datapaths that have
* another local binding. Otherwise, we know it would just be dropped.
- *
- * Use priority 150 for inputs that match both the network and a VLAN tag.
- * Use priority 100 for matching untagged traffic from the local network.
*/
struct shash_node *ln_bindings_node, *ln_bindings_node_next;
SHASH_FOR_EACH_SAFE (ln_bindings_node, ln_bindings_node_next,
@@ -658,14 +655,19 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
match_set_in_port(&match, ln_bindings->ofport);
if (ln_vlan->tag) {
match_set_dl_vlan(&match, htons(ln_vlan->tag));
+} else {
+/* Match priority-tagged frames, e.g. VLAN ID 0.
+ *
+ * We'll add a second flow for frames that lack any 802.1Q
+ * header later. */
+match_set_dl_tci_masked(&match, htons(VLAN_CFI),
+htons(VLAN_VID_MASK | VLAN_CFI));
}
struct ofpbuf ofpacts;
ofpbuf_init(&ofpacts, 0);
-if (ln_vlan->tag) {
-ofpact_put_STRIP_VLAN(&ofpacts);
-}
+ofpact_put_STRIP_VLAN(&ofpacts);
uint32_t ofpacts_orig_size = ofpacts.size;
struct binding_elem *b;
@@ -686,8 +688,16 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
}
if (ofpacts.size > ofpacts_orig_size) {
-ofctrl_add_flow(flow_table, 0, ln_vlan->tag ? 150 : 100,
-&match, &ofpacts);
+ofctrl_add_flow(flow_table, 0, 100, &match, &ofpacts);
+
+if (!ln_vlan->tag) {
+/* Add a second flow for frames that lack any 802.1Q
+ * header. For these, drop the OFPACT_STRIP_VLAN
+ * action. */
+ofpbuf_pull(&ofpacts, ofpacts_orig_size);
+match_set_dl_tci_masked(&match, 0, htons(VLAN_CFI));
+ofctrl_add_flow(flow_table, 0, 100, &match, &ofpacts);
+}
}
ofpbuf_uninit(&ofpacts);
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH net-next V16 3/3] openvswitch: 802.1AD: Flow handling, actions, vlan parsing and netlink attributes
On 10/15/15 7:02 PM, Pravin Shelar wrote:
Thanks for the review. See my comment below.
--TFH
On Thu, Oct 15, 2015 at 7:01 AM, Thomas F Herbert
wrote:
Add support for 802.1ad including the ability to push and pop double
tagged vlans. Add support for 802.1ad to netlink parsing and flow
conversion. Uses double nested encap attributes to represent double
tagged vlan. Inner TPID encoded along with ctci in nested attributes.
Signed-off-by: Thomas F Herbert
---
net/openvswitch/actions.c | 6 +-
net/openvswitch/flow.c | 75 ++
net/openvswitch/flow.h | 8 +-
net/openvswitch/flow_netlink.c | 169 +
net/openvswitch/vport-netdev.c | 4 +-
5 files changed, 228 insertions(+), 34 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 315f533..09cc1c9 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -236,7 +236,8 @@ static int pop_vlan(struct sk_buff *skb, struct sw_flow_key
*key)
if (skb_vlan_tag_present(skb))
invalidate_flow_key(key);
else
- key->eth.tci = 0;
+ key->eth.vlan.tci = 0;
+ key->eth.vlan.tpid = 0;
return err;
}
@@ -246,7 +247,8 @@ static int push_vlan(struct sk_buff *skb, struct
sw_flow_key *key,
if (skb_vlan_tag_present(skb))
invalidate_flow_key(key);
else
- key->eth.tci = vlan->vlan_tci;
+ key->eth.vlan.tci = vlan->vlan_tci;
+ key->eth.vlan.tpid = vlan->vlan_tpid;
return skb_vlan_push(skb, vlan->vlan_tpid,
ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
}
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index c8db44a..8a4e298 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -302,24 +302,69 @@ static bool icmp6hdr_ok(struct sk_buff *skb)
sizeof(struct icmp6hdr));
}
-static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
+struct qtag_prefix {
+ __be16 eth_type; /* ETH_P_8021Q or ETH_P_8021AD */
+ __be16 tci;
+};
+
Now we can just use newly defined struct vlan_header here.
+/* Parse vlan tag from vlan header.
+ * Returns ERROR on memory error.
+ * Returns 0 if it encounters a non-vlan or incomplete packet.
+ * Returns 1 after successfully parsing vlan tag.
+ */
+
+static int parse_vlan_tag(struct sk_buff *skb, __be16 vlan_proto,
+ __be16 vlan_tci, struct vlan_head *vlan)
{
- struct qtag_prefix {
- __be16 eth_type; /* ETH_P_8021Q */
- __be16 tci;
- };
- struct qtag_prefix *qp;
+ if (likely(!eth_type_vlan(vlan_proto)))
+ return 0;
if (unlikely(skb->len < sizeof(struct qtag_prefix) + sizeof(__be16)))
return 0;
if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
-sizeof(__be16
+sizeof(__be16
return -ENOMEM;
- qp = (struct qtag_prefix *) skb->data;
- key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
+ vlan->tci = vlan_tci | htons(VLAN_TAG_PRESENT);
+ vlan->tpid = vlan_proto;
+
__skb_pull(skb, sizeof(struct qtag_prefix));
+ return 1;
+}
+
+static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ struct qtag_prefix *qp = (struct qtag_prefix *)skb->data;
+ int res;
+
+ if (likely(skb_vlan_tag_present(skb))) {
+ key->eth.vlan.tci = htons(skb->vlan_tci);
+ key->eth.vlan.tpid = skb->vlan_proto;
+
+ /* Case where ingress processing has already stripped
+* the outer vlan tag.
+*/
+ res = parse_vlan_tag(skb, qp->eth_type, qp->tci,
+&key->eth.cvlan);
+ if (res < 0)
+ return res;
+ /* For inner tag, return 0 because neither
+* non-existant nor partial inner tag is an error.
+*/
+ return 0;
+ }
+ res = parse_vlan_tag(skb, qp->eth_type, qp->tci, &key->eth.vlan);
+ if (res <= 0)
+ /* This is an outer tag in the non-accelerated VLAN
+* case. Return error unless it is a complete vlan tag.
+*/
+ return res;
+
+ /* Parse inner vlan tag if present for non-accelerated case. */
+ res = parse_vlan_tag(skb, qp->eth_type, qp->tci, &key->eth.cvlan);
+ if (res <= 0)
+ return res;
same qp pointer is passed for inner and outer vlan parameters here. It
is better to just pass skb and keep qp inside parse_vlan_tag()
function.
return 0;
}
@@ -480,12 +525,12 @@ static int key_extract(struct sk_buff *skb, struct
sw_flow_key *key)
* update skb->
Re: [ovs-dev] [PATCH ] debian: place kernel module to satisfy depmod search.
On 10/15/2015 01:55 PM, Ansis Atteka wrote: On Wed, Oct 14, 2015 at 5:33 PM, Saurabh Mohan wrote: On 10/14/2015 04:58 PM, Ansis Atteka wrote: On Wed, Oct 14, 2015 at 4:08 PM, Ben Pfaff wrote: On Wed, Oct 14, 2015 at 03:28:24PM -0700, Joe Stringer wrote: On 14 October 2015 at 15:21, Ben Pfaff wrote: On Tue, Oct 06, 2015 at 04:35:32PM -0700, Saurabh Mohan wrote: On Ubuntu depmod's search priority is configured in /etc/depmod to be updates and then the kernel built-in directory. $ cat /etc/depmod.d/ubuntu.conf search updates ubuntu built-in Thus change the placement of openvswitch.ko under updates/ not kernel/updates. Signed-off-by: Saurabh Mohan This appears to be correct, but I'm confused about how this could have not been noticed for years. Did something change recently? We recently changed it from kernel/ to kernel/updates (prior to v2.4 release), and the commit message suggests it was previously nondeterministic: commit b519432205c36bda5c7331f77a49eaaa919967ad Author: Ansis Atteka Date: Tue May 26 16:49:49 2015 -0700 debian: install openvswitch kernel module under "updates" directory This patch fixes a bug where "modprobe openvswitch" command on Ubuntu distribution would have sometimes tried to load OVS kernel module that shipped together with Linux Kernel, even though one had also installed OVS datapath debian package created with module-assistant. Because of this issue force-reload-kmod command occasionally malfunctioned and failed to load the right kernel module. This bug happened *occasionally* because the default Ubuntu depmod configuration in /etc/depmod.d/ubuntu.conf is set to look for kernel modules first in "updates" directory, then in "ubuntu" directory and then in other directories. If there were two openvswitch.ko modules in "other directories", then modprobe would have loaded kernel module that was nondeterministically listed first by file system. OK, I understand why it was nondeterministic before, but where does kernel/updates come in then, since it seems to be different from and not as high-priority as "updates"? Does anyone know? I am still trying to find the answer in my email history why I ended up using "kernel/updates" over "updates". Saurabh, did you encounter an issue where the wrong kernel module was loaded or is this to achieve conformance? Anis, we tried using this patch but still noticed that the wrong kernel module was getting selected. The only way to fix it was to put the module outside kernel/ directory. Can you give me locations of all ovs kernel modules (dkms, module-assistant and the one that came with linux) present in /lib/modules// on your system that was having trouble? root@test01-1:/lib/modules# find . -name openvswitch.ko -print ./3.13.0-32-generic/kernel/net/openvswitch/openvswitch.ko ./3.13.0-32-generic/updates/openvswitch.ko I am just wondering why my patch prioritized module-assistant created kernel module (in kernel/updates) over the one that comes with linux kernel itself. I guess this is the problem you are seeing here again, right? yes, we were seeing the same problem. my observation was that if we put the module under directory /updates then depmod would still select /net/openvswitch/openvswitch.ko ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 11/23] patch: Allow client to determine port names.
> On Oct 9, 2015, at 9:20 PM, Ben Pfaff wrote: > > -create_patch_ports(ctx, network, existing_ports, br_int, ovs_bridge); > -create_patch_ports(ctx, network, existing_ports, ovs_bridge, br_int); > +char *br_int_name = patch_port_name(br_int, ovs_bridge); > +char *ovs_bridge_name = patch_port_name(ovs_bridge, br_int); > +create_patch_ports(ctx, network, > + br_int, br_int_name, > + ovs_bridge, ovs_bridge_name, > + existing_ports); Is there a reason to create the patch port names here instead of in create_patch_ports()? It seems like it complicates the interface for create_patch_ports() for not much benefit. Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 12/23] ovn-controller: Rename "ovn-patch-port" to "ovn-localnet-port".
> On Oct 9, 2015, at 9:20 PM, Ben Pfaff wrote: > > An upcoming patch will introduce a different use for patch ports, so > ovn-patch-port would become an ambiguous name. > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn: Add stateful ACL support.
On Thu, Oct 15, 2015 at 10:32:51AM -0700, Justin Pettit wrote: > Add support for the "allow-related" ACL action. This is dependent on > the OVS conntrack functionality, which is not available on all platforms > or kernel versions. > > Here is a sample policy that will allow all tenants in logical switch > "ls0" to SSH to each other. Anyone can make an HTTP request to "lp0". > All other IP traffic is dropped: > > ovn-nbctl acl-add ls0 from-lport 100 ip allow-related > ovn-nbctl acl-add ls0 to-lport 100 tcp.dst==22 allow-related > ovn-nbctl acl-add ls0 to-lport 100 "outport == \"lp0\" \ > && tcp.dst==80" allow-related > ovn-nbctl acl-add ls0 to-lport 1 ip drop > > Note: Kernel conntrack support is checked into the mainline Linux > kernel, but hasn't been backported to the main OVS repo yet. > --- > I've pushed this patch on a partial backport of conntrack here: > > https://github.com/justinpettit/ovs/tree/ovn-acl Thanks! This is going to be awesome. This lacks a Signed-off-by. ovn-northd.xml needs an update to explain all the new flows and renumbered flow tables. I get one "sparse" warning: ../ovn/lib/actions.c:151:13: warning: incorrect type in assignment (different base types) ../ovn/lib/actions.c:151:13:expected unsigned short [unsigned] [usertype] alg ../ovn/lib/actions.c:151:13:got restricted ovs_be16 In symtab_init() in ovn/controller/lflow.c, I think it would be a little better to define ct.trk as a subfield, instead of a predicate, since subfields are a little more general-purpose. Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH net-next V16 3/3] openvswitch: 802.1AD: Flow handling, actions, vlan parsing and netlink attributes
On Thu, Oct 15, 2015 at 4:48 PM, Thomas F Herbert
wrote:
> On 10/15/15 7:02 PM, Pravin Shelar wrote:
> Thanks for the review. See my comment below.
>
> --TFH
>
>
>> On Thu, Oct 15, 2015 at 7:01 AM, Thomas F Herbert
>> wrote:
>>>
>>> Add support for 802.1ad including the ability to push and pop double
>>> tagged vlans. Add support for 802.1ad to netlink parsing and flow
>>> conversion. Uses double nested encap attributes to represent double
>>> tagged vlan. Inner TPID encoded along with ctci in nested attributes.
>>>
>>> Signed-off-by: Thomas F Herbert
>>> ---
>>> net/openvswitch/actions.c | 6 +-
>>> net/openvswitch/flow.c | 75 ++
>>> net/openvswitch/flow.h | 8 +-
>>> net/openvswitch/flow_netlink.c | 169
>>> +
>>> net/openvswitch/vport-netdev.c | 4 +-
>>> 5 files changed, 228 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>>> index 315f533..09cc1c9 100644
>>> --- a/net/openvswitch/actions.c
>>> +++ b/net/openvswitch/actions.c
>>> @@ -236,7 +236,8 @@ static int pop_vlan(struct sk_buff *skb, struct
>>> sw_flow_key *key)
>>> if (skb_vlan_tag_present(skb))
>>> invalidate_flow_key(key);
>>> else
>>> - key->eth.tci = 0;
>>> + key->eth.vlan.tci = 0;
>>> + key->eth.vlan.tpid = 0;
>>> return err;
>>> }
>>>
>>> @@ -246,7 +247,8 @@ static int push_vlan(struct sk_buff *skb, struct
>>> sw_flow_key *key,
>>> if (skb_vlan_tag_present(skb))
>>> invalidate_flow_key(key);
>>> else
>>> - key->eth.tci = vlan->vlan_tci;
>>> + key->eth.vlan.tci = vlan->vlan_tci;
>>> + key->eth.vlan.tpid = vlan->vlan_tpid;
>>> return skb_vlan_push(skb, vlan->vlan_tpid,
>>> ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
>>> }
>>> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
>>> index c8db44a..8a4e298 100644
>>> --- a/net/openvswitch/flow.c
>>> +++ b/net/openvswitch/flow.c
>>> @@ -302,24 +302,69 @@ static bool icmp6hdr_ok(struct sk_buff *skb)
>>>sizeof(struct icmp6hdr));
>>> }
>>>
>>> -static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
>>> +struct qtag_prefix {
>>> + __be16 eth_type; /* ETH_P_8021Q or ETH_P_8021AD */
>>> + __be16 tci;
>>> +};
>>> +
>>
>> Now we can just use newly defined struct vlan_header here.
>>
>>> +/* Parse vlan tag from vlan header.
>>> + * Returns ERROR on memory error.
>>> + * Returns 0 if it encounters a non-vlan or incomplete packet.
>>> + * Returns 1 after successfully parsing vlan tag.
>>> + */
>>> +
>>> +static int parse_vlan_tag(struct sk_buff *skb, __be16 vlan_proto,
>>> + __be16 vlan_tci, struct vlan_head *vlan)
>>> {
>>> - struct qtag_prefix {
>>> - __be16 eth_type; /* ETH_P_8021Q */
>>> - __be16 tci;
>>> - };
>>> - struct qtag_prefix *qp;
>>> + if (likely(!eth_type_vlan(vlan_proto)))
>>> + return 0;
>>>
>>> if (unlikely(skb->len < sizeof(struct qtag_prefix) +
>>> sizeof(__be16)))
>>> return 0;
>>>
>>> if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
>>> -sizeof(__be16
>>> +sizeof(__be16
>>> return -ENOMEM;
>>>
>>> - qp = (struct qtag_prefix *) skb->data;
>>> - key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
>>> + vlan->tci = vlan_tci | htons(VLAN_TAG_PRESENT);
>>> + vlan->tpid = vlan_proto;
>>> +
>>> __skb_pull(skb, sizeof(struct qtag_prefix));
>>> + return 1;
>>> +}
>>> +
>>> +static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
>>> +{
>>> + struct qtag_prefix *qp = (struct qtag_prefix *)skb->data;
>>> + int res;
>>> +
>>> + if (likely(skb_vlan_tag_present(skb))) {
>>> + key->eth.vlan.tci = htons(skb->vlan_tci);
>>> + key->eth.vlan.tpid = skb->vlan_proto;
>>> +
>>> + /* Case where ingress processing has already stripped
>>> +* the outer vlan tag.
>>> +*/
>>> + res = parse_vlan_tag(skb, qp->eth_type, qp->tci,
>>> +&key->eth.cvlan);
>>> + if (res < 0)
>>> + return res;
>>> + /* For inner tag, return 0 because neither
>>> +* non-existant nor partial inner tag is an error.
>>> +*/
>>> + return 0;
>>> + }
>>> + res = parse_vlan_tag(skb, qp->eth_type, qp->tci, &key->eth.vlan);
>>> + if (res <= 0)
>>> + /* This is an outer tag in the non-accelerated VLAN
>>> +* case. Return error unless it is a complete vlan tag.
>>
Re: [ovs-dev] [PATCH 13/23] ovn: Implement logical patch ports.
> On Oct 9, 2015, at 9:20 PM, Ben Pfaff wrote:
>
> diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
> index 90c72ff..f25709c 100644
> --- a/ovn/controller/patch.c
> +++ b/ovn/controller/patch.c
> @@ -50,7 +50,7 @@ match_patch_port(const struct ovsrec_port *port, const char
> *peer)
>
> static void
> create_patch_port(struct controller_ctx *ctx,
> - const char *network,
> + const char *key, const char *value,
> const struct ovsrec_bridge *src, const char *src_name,
> const struct ovsrec_bridge *dst, const char *dst_name,
> struct shash *existing_ports)
The names "key" and "value" seem awfully generic.
> static void
> create_patch_ports(struct controller_ctx *ctx,
> - const char *network,
> + const char *key, const char *value,
Same here about "key" and "value".
I wonder if this function is really needed. This only has one caller and it
doesn't do much. The other function that creates patch ports just calls
create_patch_port() directly.
> +char *src_name = xasprintf("patch-%s-to-%s", local, peer);
> +char *dst_name = xasprintf("patch-%s-to-%s", peer, local);
In the previous patch, you defined patch_port_name(), which could be used for
this.
> +create_patch_port(ctx, "ovn-logical-patch-port", local,
> + br_int, src_name, br_int, dst_name,
> + existing_ports);
A description of "ovn-localnet-port" is provided in the ovn-controller man
page. It might be nice to do the same for "ovn-logical-patch-port".
> /* Add any patch ports that should exist but don't. */
> parse_bridge_mappings(ctx, br_int, &existing_ports);
> +add_logical_patch_ports(ctx, br_int, &existing_ports);
Since these are doing very similar things, do you think it's worth using more
similar names?
> @@ -252,6 +262,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
> mff_ovn_geneve,
> *
> * The "localnet" port may be configured with a VLAN ID. If so,
> * 'tag' will be set to that VLAN ID; otherwise 'tag' is 0.
> + *
Did you add this on purpose?
Acked-by: Justin Pettit
--Justin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 14/23] ovn: Add new predicates for matching broadcast and multicast packets.
> On Oct 9, 2015, at 9:20 PM, Ben Pfaff wrote: > > In my opinion, "eth.mcast" is a bit more readable than "eth.dst[40]", and > so on. > > Signed-off-by: Ben Pfaff I think there are a couple of spots in "ovn-northd.8.xml" that could use these new definitions. Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 15/23] ovn-nb: Extend schema to support networks of routers.
> On Oct 9, 2015, at 9:20 PM, Ben Pfaff wrote: > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 16/23] ovn-nb: Rename Port_Bindings 'macs' column to 'addresses'.
> On Oct 9, 2015, at 9:20 PM, Ben Pfaff wrote: > > In an upcoming commit this column will also support IP+MAC pairs. > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 17/23] ovn-nb: Add support for IP+MAC binding pairs in Port_Binding 'address'.
> On Oct 9, 2015, at 9:20 PM, Ben Pfaff wrote: > > When a logical router can statically obtain the IP+MAC pairs for its > attached logical switches, it can avoid expensive ARP resolution. > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 18/23] ovn-nb.xml: Reorganize documentation for Logical_Port table.
> On Oct 9, 2015, at 9:20 PM, Ben Pfaff wrote: > > This uses the column grouping feature and the ability to document an > individual key within a column to better, in my opinion, organize the > documentation for the Logical_Port table. > > This will make it easier to document a new port type that a future commit > will add. > > Signed-off-by: Ben Pfaff Agreed it looks better. Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 19/23] ovn-nb: Change how router ports work.
> On Oct 9, 2015, at 9:20 PM, Ben Pfaff wrote: > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 20/23] ovn-nb: Add "enabled" column to Logical_Router_Port.
> On Oct 9, 2015, at 9:21 PM, Ben Pfaff wrote: > > Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 21/23] packets: New function ip_parse_masked().
> On Oct 9, 2015, at 9:21 PM, Ben Pfaff wrote: > > Signed-off-by: Ben Pfaff > > +char * OVS_WARN_UNUSED_RESULT > +ip_parse_masked(const char *s, ovs_be32 *ip, ovs_be32 *mask) It might be nice to provide a comment describing this function. Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 22/23] ovn-northd: Add stages for logical routers.
> On Oct 9, 2015, at 9:21 PM, Ben Pfaff wrote: > > Until now, ovn-northd has only set up flows for logical switches. With the > arrival of logical routers, it needs to set up flows for them too. The > stages within logical routers are completely different from those for > logical switches, so this prepares for that by adding logic for identifying > those stages. > > Signed-off-by: Ben Pfaff The next patch is the big one! http://i.imgur.com/7drHiqr.gif Unfortunately, I gotta take a break to get dinner. (Plus, merge my changes so I don't have to deal with the conflicts. ;-) ) Acked-by: Justin Pettit --Justin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH net-next V16 3/3] openvswitch: 802.1AD: Flow handling, actions, vlan parsing and netlink attributes
On 10/15/15 8:37 PM, Pravin Shelar wrote:
On Thu, Oct 15, 2015 at 4:48 PM, Thomas F Herbert
wrote:
On 10/15/15 7:02 PM, Pravin Shelar wrote:
Thanks for the review. See my comment below.
--TFH
On Thu, Oct 15, 2015 at 7:01 AM, Thomas F Herbert
wrote:
Add support for 802.1ad including the ability to push and pop double
tagged vlans. Add support for 802.1ad to netlink parsing and flow
conversion. Uses double nested encap attributes to represent double
tagged vlan. Inner TPID encoded along with ctci in nested attributes.
Signed-off-by: Thomas F Herbert
---
net/openvswitch/actions.c | 6 +-
net/openvswitch/flow.c | 75 ++
net/openvswitch/flow.h | 8 +-
net/openvswitch/flow_netlink.c | 169
+
net/openvswitch/vport-netdev.c | 4 +-
5 files changed, 228 insertions(+), 34 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 315f533..09cc1c9 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -236,7 +236,8 @@ static int pop_vlan(struct sk_buff *skb, struct
sw_flow_key *key)
if (skb_vlan_tag_present(skb))
invalidate_flow_key(key);
else
- key->eth.tci = 0;
+ key->eth.vlan.tci = 0;
+ key->eth.vlan.tpid = 0;
return err;
}
@@ -246,7 +247,8 @@ static int push_vlan(struct sk_buff *skb, struct
sw_flow_key *key,
if (skb_vlan_tag_present(skb))
invalidate_flow_key(key);
else
- key->eth.tci = vlan->vlan_tci;
+ key->eth.vlan.tci = vlan->vlan_tci;
+ key->eth.vlan.tpid = vlan->vlan_tpid;
return skb_vlan_push(skb, vlan->vlan_tpid,
ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
}
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index c8db44a..8a4e298 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -302,24 +302,69 @@ static bool icmp6hdr_ok(struct sk_buff *skb)
sizeof(struct icmp6hdr));
}
-static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
+struct qtag_prefix {
+ __be16 eth_type; /* ETH_P_8021Q or ETH_P_8021AD */
+ __be16 tci;
+};
+
Now we can just use newly defined struct vlan_header here.
+/* Parse vlan tag from vlan header.
+ * Returns ERROR on memory error.
+ * Returns 0 if it encounters a non-vlan or incomplete packet.
+ * Returns 1 after successfully parsing vlan tag.
+ */
+
+static int parse_vlan_tag(struct sk_buff *skb, __be16 vlan_proto,
+ __be16 vlan_tci, struct vlan_head *vlan)
{
- struct qtag_prefix {
- __be16 eth_type; /* ETH_P_8021Q */
- __be16 tci;
- };
- struct qtag_prefix *qp;
+ if (likely(!eth_type_vlan(vlan_proto)))
+ return 0;
if (unlikely(skb->len < sizeof(struct qtag_prefix) +
sizeof(__be16)))
return 0;
if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
-sizeof(__be16
+sizeof(__be16
return -ENOMEM;
- qp = (struct qtag_prefix *) skb->data;
- key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
+ vlan->tci = vlan_tci | htons(VLAN_TAG_PRESENT);
+ vlan->tpid = vlan_proto;
+
__skb_pull(skb, sizeof(struct qtag_prefix));
+ return 1;
+}
+
+static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ struct qtag_prefix *qp = (struct qtag_prefix *)skb->data;
+ int res;
+
+ if (likely(skb_vlan_tag_present(skb))) {
+ key->eth.vlan.tci = htons(skb->vlan_tci);
+ key->eth.vlan.tpid = skb->vlan_proto;
+
+ /* Case where ingress processing has already stripped
+* the outer vlan tag.
+*/
+ res = parse_vlan_tag(skb, qp->eth_type, qp->tci,
+&key->eth.cvlan);
+ if (res < 0)
+ return res;
+ /* For inner tag, return 0 because neither
+* non-existant nor partial inner tag is an error.
+*/
+ return 0;
+ }
+ res = parse_vlan_tag(skb, qp->eth_type, qp->tci, &key->eth.vlan);
+ if (res <= 0)
+ /* This is an outer tag in the non-accelerated VLAN
+* case. Return error unless it is a complete vlan tag.
+*/
+ return res;
+
+ /* Parse inner vlan tag if present for non-accelerated case. */
+ res = parse_vlan_tag(skb, qp->eth_type, qp->tci,
&key->eth.cvlan);
+ if (res <= 0)
+ return res;
same qp pointer is passed for inner and outer vlan parameters here. It
is better to just pass skb and keep qp inside parse_vlan_tag()
function.
ret
[ovs-dev] Delivery failed
This message was undeliverable due to the following reason: Your message could not be delivered because the destination server was not reachable within the allowed queue period. The amount of time a message is queued before it is returned depends on local configura- tion parameters. Most likely there is a network problem that prevented delivery, but it is also possible that the computer is turned off, or does not have a mail system running right now. Your message could not be delivered within 8 days: Host 19.220.125.167 is not responding. The following recipients did not receive this message: Please reply to postmas...@openvswitch.org if you feel this message to be in error. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 01/23] ovn: Extend logical "next" action to jump to arbitrary flow tables.
On Thu, Oct 15, 2015 at 01:46:44PM -0700, Justin Pettit wrote:
>
> > On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote:
> >
> > @@ -30,8 +30,10 @@ struct action_context {
> > /* Input. */
> > 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. */
> > +uint8_t first_table;/* First OpenFlow table. */
> > +uint8_t n_tables; /* Number of OpenFlow tables. */
>
> I think these two descriptions could be a bit misleading, since they
> define the range of tables that a "next" action can jump to, but it
> sounds like it's the full range of supported OpenFlow tables.
OK, that's a good point, thanks.
> > +uint8_t cur_table; /* 0 <= cur_table < n_tables. */
>
> This member name seems a little confusing, since all the other
> "_table" members refer to an OpenFlow port, but this one is an offset.
> What about something like "cur_table_offset"? Another option would be
> to relabel them things like "*_of_table" (or "*_phy_table") and
> "*_log_table".
How about ptable and ltable?
> > +static void
> > +parse_next_action(struct action_context *ctx)
> > +{
> > +if (!ctx->n_tables) {
> > +action_error(ctx, "\"next\" action not allowed here.");
> > +} else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
> > +int table;
>
> Similar to above, this is actually a logical table or an offset to an
> OpenFlow table. It might be nice to clarify that for later readers.
I renamed it "ltable".
> This is a nice addition, but I think some of the older phrasing
> related to logical and physical tables was a bit clearer than exposing
> that they're offsets.
>
> Acked-by: Justin Pettit
Thanks for the review. I guess given the ack you're more or less happy,
so here's an incremental that I folded in, and I'll apply this to master
in a minute.
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 2bc495c..8afddee 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -27,19 +27,24 @@
/* Context maintained during actions_parse(). */
struct action_context {
-/* Input. */
+/* Input. */
+
struct lexer *lexer;/* Lexer for pulling more tokens. */
-const struct shash *symtab; /* Symbol table. */
-uint8_t first_table;/* First OpenFlow table. */
-uint8_t n_tables; /* Number of OpenFlow tables. */
-uint8_t cur_table; /* 0 <= cur_table < n_tables. */
-uint8_t output_table; /* OpenFlow table for 'output' to resubmit. */
const struct simap *ports; /* Map from port name to number. */
+const struct shash *symtab; /* Symbol table. */
-/* State. */
+/* OVN maps each logical flow table (ltable), one-to-one, onto a physical
+ * OpenFlow flow table (ptable). These members describe the mapping and
+ * data related to flow tables. */
+uint8_t n_tables; /* Number of flow tables. */
+uint8_t first_ptable; /* First OpenFlow table. */
+uint8_t cur_ltable; /* 0 <= cur_ltable < n_tables. */
+uint8_t output_ptable; /* OpenFlow table for 'output' to resubmit. */
+
+/* State. */
char *error;/* Error, if any, otherwise NULL. */
-/* Output. */
+/* Output. */
struct ofpbuf *ofpacts; /* Actions. */
struct expr *prereqs; /* Prerequisites to apply to match. */
};
@@ -149,9 +154,9 @@ parse_next_action(struct action_context *ctx)
if (!ctx->n_tables) {
action_error(ctx, "\"next\" action not allowed here.");
} else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
-int table;
+int ltable;
-if (!action_get_int(ctx, &table)) {
+if (!action_get_int(ctx, lexer, LEX_T_RPAREN)) {
@@ -159,16 +164,16 @@ parse_next_action(struct action_context *ctx)
return;
}
-if (table >= ctx->n_tables) {
+if (ltable >= ctx->n_tables) {
action_error(ctx, "\"next\" argument must be in range 0 to %d.",
ctx->n_tables - 1);
return;
}
-emit_resubmit(ctx, ctx->first_table + table);
+emit_resubmit(ctx, ctx->first_ptable + ltable);
} else {
-if (ctx->cur_table < ctx->n_tables) {
-emit_resubmit(ctx, ctx->first_table + ctx->cur_table + 1);
+if (ctx->cur_ltable < ctx->n_tables) {
+emit_resubmit(ctx, ctx->first_ptable + ctx->cur_ltable + 1);
} else {
action_error(ctx, "\"next\" action not allowed in last table.");
}
@@ -204,7 +209,7 @@ parse_actions(struct action_context *ctx)
} else if (lexer_match_id(ctx->lexer, "next")) {
parse_next_action(ctx);
} else if (lexer_matc
Re: [ovs-dev] [PATCH 2/4] ovs-ofctl: Fix OpenFlow versions with '--bundle'
On Fri, Oct 16, 2015 at 6:28 AM, Jarno Rajahalme wrote: > While the presence of the '--bundle' option implicitly added the > OpenFlow 1.4 to the allowed protocols, it failed to remove OpenFlow > 1.0 from the allowed protocols. This is changed so that '--bundle' > option now also implicitly removes versions lesser than 1.4 from the > allowed protocols. This has no behavioral difference when ovs-ofctl > is paired with OVS that supports OpenFlow 1.4, as the greatest common > version is negotiated, but prevents negotiation of OpenFlow 1.0 when > OVS does not support OpenFlow 1.4. > > Found by inspection. > > Signed-off-by: Jarno Rajahalme Acked-by: YAMAMOTO Takashi > --- > tests/ofproto.at | 12 ++-- > tests/ovs-ofctl.at| 8 > utilities/ovs-ofctl.c | 3 +++ > 3 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/tests/ofproto.at b/tests/ofproto.at > index 5e4441c..4c6dd29 100644 > --- a/tests/ofproto.at > +++ b/tests/ofproto.at > @@ -3962,8 +3962,8 @@ vconn|DBG|unix: sent (Success): OFPT_BARRIER_REPLY: > vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): > version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 > vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > - version bitmap: 0x01, 0x05 > -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports versions 0x01, 0x05) > + version bitmap: 0x05 > +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): > bundle_id=0 type=OPEN_REQUEST flags=atomic ordered > vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): > @@ -4014,8 +4014,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply: > vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): > version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 > vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > - version bitmap: 0x01, 0x05 > -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports versions 0x01, 0x05) > + version bitmap: 0x05 > +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): > bundle_id=0 type=OPEN_REQUEST flags=atomic ordered > vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): > @@ -4045,8 +4045,8 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply: > vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): > version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 > vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > - version bitmap: 0x01, 0x05 > -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports versions 0x01, 0x05) > + version bitmap: 0x05 > +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): > bundle_id=0 type=OPEN_REQUEST flags=atomic ordered > vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at > index 6f03adb..7375cad 100644 > --- a/tests/ovs-ofctl.at > +++ b/tests/ovs-ofctl.at > @@ -2889,8 +2889,8 @@ AT_CHECK([print_vconn_debug | vconn_windows_sub | > ofctl_strip], [0], [dnl > vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): > version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 > vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > - version bitmap: 0x01, 0x05 > -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports versions 0x01, 0x05) > + version bitmap: 0x05 > +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): > bundle_id=0 type=OPEN_REQUEST flags=atomic ordered > vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): > @@ -2926,8 +2926,8 @@ vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL > (OF1.4): > vconn|DBG|unix: sent (Success): OFPT_HELLO (OF1.5): > version bitmap: 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 > vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > - version bitmap: 0x01, 0x05 > -vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports versions 0x01, 0x05) > + version bitmap: 0x05 > +vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): > vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): > importance=1, dl_vlan=1 actions=drop > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c > index fbc9da4..ee15e1a 100644 > --- a/utilities/ovs-ofctl.c > +++ b/utilities/ovs-ofctl.c > @@ -312,6 +312,9 @@ parse_options(int argc, char *argv[]) > /* Add implicit allowance for OpenFlow 1.4. */ >
Re: [ovs-dev] [PATCH 1/4] Summary: From: Jarno Rajahalme
On Fri, Oct 16, 2015 at 6:28 AM, Jarno Rajahalme wrote: > ovs-ofctl: Fix replace-flows. > > The replace-flows test cases tested for incorrect > behavior due to the missing initialization of the out_group member of > struct ofputil_flow_stats_request. This patch fixes this by properly > initializing out_group to OFPG_ANY. > > Note that replace-flows still does not support multiple tables, but > that will be fixed in a later patch in the series. > > Reported-by: Takashi Yamamoto > Signed-off-by: Jarno Rajahalme thank you for quick fix. assuming the title fixed, Acked-by: YAMAMOTO Takashi > --- > tests/ovs-ofctl.at| 34 +- > utilities/ovs-ofctl.c | 2 ++ > 2 files changed, 27 insertions(+), 9 deletions(-) > > diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at > index 33e67ed..6f03adb 100644 > --- a/tests/ovs-ofctl.at > +++ b/tests/ovs-ofctl.at > @@ -2851,12 +2851,12 @@ dnl Add flows to br0 with importance via OF1.4+. For > more details refer "ovs-ofc > for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i,actions=drop"; > done > add-flows.txt > AT_CHECK([ovs-ofctl -O OpenFlow14 add-flows br0 add-flows.txt]) > > -dnl Replace some flows in the bridge. > -for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i + > 10`,actions=drop"; done > replace-flows.txt > +dnl Replace the flows in the bridge. > +for i in 1 3 5 7; do echo " importance=`expr $i + 10`, dl_vlan=$i > actions=drop"; done > replace-flows.txt > AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 replace-flows.txt]) > > dnl Dump them and compare the dump flows output against the expected output. > -for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then > importance=`expr $i + 10`; else importance=$i; fi; echo " > importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout > +cat replace-flows.txt > expout > AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed > '/OFPST_FLOW/d' | sort], >[0], [expout]) > > @@ -2873,11 +2873,11 @@ for i in 1 2 3 4 5 6 7 8; do echo > "dl_vlan=$i,importance=$i,actions=drop"; done > AT_CHECK([ovs-ofctl --bundle add-flows br0 add-flows.txt]) > > dnl Replace some flows in the bridge. > -for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i + > 10`,actions=drop"; done > replace-flows.txt > +for i in 1 3 5 7; do echo " importance=`expr $i + 10`, dl_vlan=$i > actions=drop"; done > replace-flows.txt > AT_CHECK([ovs-ofctl --bundle replace-flows br0 replace-flows.txt]) > > dnl Dump them and compare the dump flows output against the expected output. > -for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then > importance=`expr $i + 10`; else importance=$i; fi; echo " > importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout > +cat replace-flows.txt > expout > AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed > '/OFPST_FLOW/d' | sort], >[0], [expout]) > > @@ -2930,12 +2930,32 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports versions 0x01, 0x05) > vconn|DBG|unix: received: OFPST_FLOW request (OF1.4): > vconn|DBG|unix: sent (Success): OFPST_FLOW reply (OF1.4): > + importance=1, dl_vlan=1 actions=drop > + importance=2, dl_vlan=2 actions=drop > + importance=3, dl_vlan=3 actions=drop > + importance=4, dl_vlan=4 actions=drop > + importance=5, dl_vlan=5 actions=drop > + importance=6, dl_vlan=6 actions=drop > + importance=7, dl_vlan=7 actions=drop > + importance=8, dl_vlan=8 actions=drop > vconn|DBG|unix: received: OFPT_BUNDLE_CONTROL (OF1.4): > bundle_id=0 type=OPEN_REQUEST flags=atomic ordered > vconn|DBG|unix: sent (Success): OFPT_BUNDLE_CONTROL (OF1.4): > bundle_id=0 type=OPEN_REPLY flags=0 > vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): > bundle_id=0 flags=atomic ordered > +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=2 actions=drop > +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): > + bundle_id=0 flags=atomic ordered > +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=4 actions=drop > +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): > + bundle_id=0 flags=atomic ordered > +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=6 actions=drop > +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): > + bundle_id=0 flags=atomic ordered > +OFPT_FLOW_MOD (OF1.4): DEL_STRICT table:255 dl_vlan=8 actions=drop > +vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): > + bundle_id=0 flags=atomic ordered > OFPT_FLOW_MOD (OF1.4): ADD dl_vlan=1 importance:11 actions=drop > vconn|DBG|unix: received: OFPT_BUNDLE_ADD_MESSAGE (OF1.4): > bundle_id=0 flags=atomic ordered > @@ -2957,10 +2977,6 @@ vconn|DBG|unix: received: OFPT_HELLO (OF1.4): > vconn|DBG|unix: negotiated OpenFlow version 0x05 (we support version 0x06 > and earlier, peer supports version 0x05) > vconn|DBG|unix: received: OFPST_FLOW request (OF1.4):
Re: [ovs-dev] [PATCH 02/23] ovn: Implement logical action to decrement IPv4 TTL.
On Thu, Oct 15, 2015 at 01:53:23PM -0700, Justin Pettit wrote: > > > On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > > > This is necessary for IPv4 routing. > > > > Signed-off-by: Ben Pfaff > > Acked-by: Justin Pettit Thanks, applied. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 03/23] ovn: Change h1 titles to title case in documentation.
On Thu, Oct 15, 2015 at 01:54:26PM -0700, Justin Pettit wrote: > > > On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > > > Manpage section titles are traditionally all-uppercase, but OVS's > > XML-to-nroff translator takes care of that and there's no need to actually > > provide them in all-caps (and it looks ugly). > > > > Signed-off-by: Ben Pfaff > > Acked-by: Justin Pettit Thanks, applied. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 04/23] ovn-nbctl: Remove external-ids commands.
On Thu, Oct 15, 2015 at 01:58:08PM -0700, Justin Pettit wrote: > > > On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > > > Users are served just as well through the general-purpose "set" and "get" > > database commands, so avoid the additional code and documentation. > > > > (ovs-vsctl does have special external-ids commands for bridges, but those > > exist because of special handling for "fake bridges".) > > > > Signed-off-by: Ben Pfaff > > Every line of code I write (or copy and paste from ovs-vsctl) is like > a child to me. Good bye, sweet peas. Like sands through the hourglass, so are the days of our lives. > Acked-by: Justin Pettit Thanks, applied to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 05/23] nx-match: Serialize match on IP TTL even when outputting OXM.
On Thu, Oct 15, 2015 at 01:59:35PM -0700, Justin Pettit wrote: > > On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > > > The 'oxm' parameter to nxm_put_ip() indicates whether NXM or OXM code > > points should be used in cases where both exist. It shouldn't cause > > matches to be dropped entirely, since that changes the meaning, but that's > > what was done here for matches on the IP (v4 or v6) TTL. This commit > > fixes the problem. > > > > Signed-off-by: Ben Pfaff > > Acked-by: Justin Pettit Thanks, applied to master, branch-2.4, and branch-2.3. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 07/23] ovn-controller: Un-inline get_chassis_by_name().
On Thu, Oct 15, 2015 at 02:04:26PM -0700, Justin Pettit wrote: > > > On Oct 9, 2015, at 9:15 PM, Ben Pfaff wrote: > > > > I don't know of any reason to inline this. > > Well, obviously because it's on the critical path. It has to do with > registers and CPU caches and stuff--you wouldn't understand. Dude, don't get all TLB on me. > > Also rename for consistency with get_bridge(). > > > > Signed-off-by: Ben Pfaff > > Acked-by: Justin Pettit Thanks, applied to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 08/23] ovn-controller: Factor patch port management into new "patch" module.
On Thu, Oct 15, 2015 at 11:38:34PM +, Justin Pettit wrote:
> After reviewing patch 10, I have a few more thoughts on this patch. This
> doesn't change my original ACK, though.
This patch is supposed to be mostly moving code around. I'm happy to
make the improvements you mention but I'd like to separate them from
just moving code from ovn-controller.c into a new file.
> > +static void
> > +create_patch_port(struct controller_ctx *ctx,
> > + const char *network,
> > + const struct ovsrec_bridge *b1,
> > + const struct ovsrec_bridge *b2)
> > +{
>
> This could use a comment and maybe a renaming of some arguments. In
> particular, it isn't obvious the importance between "b1" and "b2".
> Also, it might be worth noting that it only creates one side of the
> patch port.
The later commit "patch: Allow client to determine port names." renames
b1 and b2 to src and dst. It doesn't add a comment; I'll take care of
that though.
> > +static void
> > +create_patch_ports(struct controller_ctx *ctx,
> > + const char *network,
> > + struct shash *existing_ports,
> > + const struct ovsrec_bridge *b1,
> > + const struct ovsrec_bridge *b2)
>
> This could use a comment, since many of the arguments are non-obvious.
> In particular, how "existing_ports" will be modified and the
> difference between "b1" and "b2".
This gets a little more obvious later, in the same commit I mentioned
above, since it gets reduced to a trivial two-statement function.
> > +static void
> > +parse_bridge_mappings(struct controller_ctx *ctx,
> > + const struct ovsrec_bridge *br_int,
> > + const char *mappings_cfg)
>
> This function does a lot more than just parse the bridge mappings. It
> could probably use a better name, but, at the very least, I think a
> comment would be helpful--especially for the non-obvious things like
> what will happen to "mappings_cfg".
OK, I'll add a comment.
But what's nonobvious about mappings_cfg?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Delivery failed
The original message was received at Fri, 16 Oct 2015 13:24:57 +0700 from austininvestments.net [78.2.227.188] - The following addresses had permanent fatal errors - - Transcript of the session follows - ... while talking to host 164.192.90.139: 554 Service unavailable; [139.223.205.10] blocked using relays.osirusoft.com, reason: Blocked Session aborted ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 23/23] ovn: Implement basic logical L3 routing.
Hi Ben,
On Fri, Oct 9, 2015 at 9:21 PM, Ben Pfaff wrote:
> +/* Connect logical router ports, and logical switch ports of type
> "router",
> + * to their peers. */
> +struct ovn_port *op;
> +HMAP_FOR_EACH (op, key_node, ports) {
This seems not efficient. There are far more lswitch ports than router
ports and patch ports. Would it be better to have a separate index to
iterate routers ports and patch ports?
> +/* ARP reply. These flows reply to ARP requests for the router's own
> + * IP address. */
> +match = xasprintf(
> +"inport == %s && arp.tha == "ETH_ADDR_FMT" && arp.op == 1",
> +op->json_key, ETH_ADDR_ARGS(op->mac));
Should this be arp.tpa == "IP_FMT" ... ? Because I think we should
match router's IP instead of MAC here.
After all this is amazing code. Thanks Ben!
Han
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
