date:20170124

Re: [ovs-dev] [PATCH] ovn-northd: Add flows in DHCP_OPTIONS pipeline to support renew requests

2017-01-24 Thread Numan Siddique

On Wed, Jan 25, 2017 at 1:40 AM, Russell Bryant  wrote:

>
>
> On Tue, Jan 24, 2017 at 1:58 PM, Numan Siddique 
> wrote:
>
>> ovn-northd adds the flows to send the DHCPv4 packets to ovn-controller
>> only with the match ip4.src = 0.0.0.0 and ip4.dst = 255.255.255.255.
>>
>> When a DHCPv4 lease is about to expire, before sending a DHCPDISCOVER
>> packet, the client can send a DHCPREQUEST packet to renew its ip
>> with ip4.src set to its offered ip and ip4.dst set to the DHCP server
>> ip or broadcast ip.
>>
>> This patch supports this missing scenario by adding the necessary
>> flows in DHCP_OPTIONS ingress pipeline.
>>
>> Signed-off-by: Numan Siddique 
>
>
> Thanks for working on this, Numan!
>
> I see some DHCP test cases in tests/ovn.at.  Could you look at extending
> those to cover this case as well?
>

Thanks for the review. Done.

I just posted v2

Numan



> Thanks,
>
> --
> Russell Bryant
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ovn-northd: Add flows in DHCP_OPTIONS pipeline to support renew requests

2017-01-24 Thread Numan Siddique

ovn-northd adds the flows to send the DHCPv4 packets to ovn-controller
only with the match ip4.src = 0.0.0.0 and ip4.dst = 255.255.255.255.

When a DHCPv4 lease is about to expire, before sending a DHCPDISCOVER
packet, the client can send a DHCPREQUEST packet to renew its ip
with ip4.src set to its offered ip and ip4.dst set to the DHCP server
ip or broadcast ip.

This patch supports this missing scenario by adding the necessary
flows in DHCP_OPTIONS ingress pipeline.

Signed-off-by: Numan Siddique 
---
 ovn/northd/ovn-northd.c | 37 +++---
 tests/ovn.at| 84 +++--
 2 files changed, 108 insertions(+), 13 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 87c80d1..3b05470 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2115,7 +2115,8 @@ lsp_is_up(const struct nbrec_logical_switch_port *lsp)
 
 static bool
 build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,
-struct ds *options_action, struct ds *response_action)
+struct ds *options_action, struct ds *response_action,
+struct ds *ipv4_addr_match)
 {
 if (!op->nbsp->dhcpv4_options) {
 /* CMS has disabled native DHCPv4 for this lport. */
@@ -2185,6 +2186,9 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
   "output;",
   server_mac, IP_ARGS(offer_ip), server_ip);
 
+ds_put_format(ipv4_addr_match,
+  "ip4.src == "IP_FMT" && ip4.dst == {%s, 255.255.255.255}",
+  IP_ARGS(offer_ip), server_ip);
 smap_destroy(_options);
 return true;
 }
@@ -3085,9 +3089,10 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) {
 struct ds options_action = DS_EMPTY_INITIALIZER;
 struct ds response_action = DS_EMPTY_INITIALIZER;
+struct ds ipv4_addr_match = DS_EMPTY_INITIALIZER;
 if (build_dhcpv4_action(
 op, op->lsp_addrs[i].ipv4_addrs[j].addr,
-_action, _action)) {
+_action, _action, _addr_match)) {
 struct ds match = DS_EMPTY_INITIALIZER;
 ds_put_format(
 , "inport == %s && eth.src == %s && "
@@ -3098,15 +3103,39 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_OPTIONS,
   100, ds_cstr(),
   ds_cstr(_action));
+ds_clear();
+/* Allow ip4.src = OFFER_IP and
+ * ip4.dst = {SERVER_IP, 255.255.255.255} for the below
+ * cases
+ *  -  When the client wants to renew the IP by sending
+ * the DHCPREQUEST to the server ip.
+ *  -  When the client wants to renew the IP by
+ * broadcasting the DHCPREQUEST.
+ */
+ds_put_format(
+, "inport == %s && eth.src == %s && "
+"%s && udp.src == 68 && udp.dst == 67", op->json_key,
+op->lsp_addrs[i].ea_s, ds_cstr(_addr_match));
+
+ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_OPTIONS,
+  100, ds_cstr(),
+  ds_cstr(_action));
+ds_clear();
+
 /* If REGBIT_DHCP_OPTS_RESULT is set, it means the
- * put_dhcp_opts action  is successful */
-ds_put_cstr(, " && "REGBIT_DHCP_OPTS_RESULT);
+ * put_dhcp_opts action  is successful. */
+ds_put_format(
+, "inport == %s && eth.src == %s && "
+"ip4 && udp.src == 68 && udp.dst == 67"
+" && "REGBIT_DHCP_OPTS_RESULT, op->json_key,
+op->lsp_addrs[i].ea_s);
 ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_RESPONSE,
   100, ds_cstr(),
   ds_cstr(_action));
 ds_destroy();
 ds_destroy(_action);
 ds_destroy(_action);
+ds_destroy(_addr_match);
 break;
 }
 }
diff --git a/tests/ovn.at b/tests/ovn.at
index 126574c..7ace715 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -3705,8 +3705,17 @@ as hv1 ovs-vsctl show
 # This shell function sends a DHCP request packet
 # test_dhcp INPORT SRC_MAC DHCP_TYPE OFFER_IP ...
 test_dhcp() {
-local inport=$1 src_mac=$2 dhcp_type=$3 offer_ip=$4
-local

[ovs-dev] OVS - ODL Sync on OF Bundle in 1.3

2017-01-24 Thread Jan Scheurich

We'll use below webex instead of Lync/SkypeFB:
https://meetings.webex.com/collabs/#/meetings/detail?uuid=MBFUHFYE8TRG3NPX76F4PLRS2N-3OWH=231184.55753

Rescheduled to Wednesday due to unavailability of key participants.
@Jarno: As you are the OVS brain behind bundles. Do you have the chance to join 
for a short time to discuss some  details regarding OF 1.3 bundle 
implementation?

Hi Jarno,

OpenDaylight folks are finally starting to implement support of OpenFlow 
bundles as a basis for the bundle-based hitless recync procedure we discussed 
earlier. As ODL does not yet have protocol support for OpenFlow versions 1.4 or 
1.5, they intend to implement the bundle extension to OF 1.3 as specified in 
EXT-230 in
https://www.opennetworking.org/images/stories/downloads/sdn-resources/onf-specifications/openflow/openflow-extensions-1.3.x-pack2.zip

Would you have time for a short meeting on early Monday to discuss and confirm 
whether the OVS implementation of bundles in OF 1.3 is compliant with EXT-230 
and has everything they need?

Thanks, Jan


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

[ovs-dev] [userspace meter v3 5/5] dpif-netdev: Simple DROP meter implementation.

2017-01-24 Thread Andy Zhou

From: Jarno Rajahalme 

Meters may be used by any flow, so some kind of locking must be used.
In this version we have an adaptive mutex for each meter, which may
not be optimal for DPDK.  However, this should serve as a basis for
further improvement.

A batch of packets is first tried as a whole, and only if some of the
meter bands are hit, we need to process the packets individually.

Signed-off-by: Jarno Rajahalme 
Signed-off-by: Andy Zhou 
---
 lib/dpif-netdev.c| 362 ---
 tests/dpif-netdev.at | 106 +++
 2 files changed, 450 insertions(+), 18 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index f81b464..96c283a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -86,6 +86,8 @@ DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
 
 /* Configuration parameters. */
 enum { MAX_FLOWS = 65536 }; /* Maximum number of flows in flow table. */
+enum { MAX_METERS = 65536 };/* Maximum number of meters. */
+enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
 
 /* Protects against changes to 'dp_netdevs'. */
 static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -193,6 +195,31 @@ static bool dpcls_lookup(struct dpcls *cls,
  struct dpcls_rule **rules, size_t cnt,
  int *num_lookups_p);
 
+/* Set of supported meter flags */
+#define DP_SUPPORTED_METER_FLAGS_MASK \
+(OFPMF13_STATS | OFPMF13_PKTPS | OFPMF13_KBPS | OFPMF13_BURST)
+
+/* Set of supported meter band types */
+#define DP_SUPPORTED_METER_BAND_TYPES   \
+( 1 << OFPMBT13_DROP )
+
+struct dp_meter_band {
+struct ofputil_meter_band up; /* type, prec_level, pad, rate, burst_size */
+uint32_t bucket; /* In 1/1000 packets (for PKTPS), or in bits (for KBPS) */
+uint64_t packet_count;
+uint64_t byte_count;
+};
+
+struct dp_meter {
+uint16_t flags;
+uint16_t n_bands;
+uint32_t max_delta_t;
+uint64_t used;
+uint64_t packet_count;
+uint64_t byte_count;
+struct dp_meter_band bands[];
+};
+
 /* Datapath based on the network device interface from netdev.h.
  *
  *
@@ -223,6 +250,11 @@ struct dp_netdev {
 struct hmap ports;
 struct seq *port_seq;   /* Incremented whenever a port changes. */
 
+/* Meters. */
+struct ovs_mutex meter_locks[MAX_METERS];
+struct dp_meter *meters[MAX_METERS]; /* Meter bands. */
+uint32_t meter_free; /* Next free meter. */
+
 /* Protects access to ofproto-dpif-upcall interface during revalidator
  * thread synchronization. */
 struct fat_rwlock upcall_rwlock;
@@ -1059,6 +1091,10 @@ create_dp_netdev(const char *name, const struct 
dpif_class *class,
 dp->reconfigure_seq = seq_create();
 dp->last_reconfigure_seq = seq_read(dp->reconfigure_seq);
 
+for (int i = 0; i < MAX_METERS; ++i) {
+ovs_mutex_init_adaptive(>meter_locks[i]);
+}
+
 /* Disable upcalls by default. */
 dp_netdev_disable_upcall(dp);
 dp->upcall_aux = NULL;
@@ -1136,6 +1172,16 @@ dp_netdev_destroy_upcall_lock(struct dp_netdev *dp)
 fat_rwlock_destroy(>upcall_rwlock);
 }
 
+static void
+dp_delete_meter(struct dp_netdev *dp, uint32_t meter_id)
+OVS_REQUIRES(dp->meter_locks[meter_id])
+{
+if (dp->meters[meter_id]) {
+free(dp->meters[meter_id]);
+dp->meters[meter_id] = NULL;
+}
+}
+
 /* Requires dp_netdev_mutex so that we can't get a new reference to 'dp'
  * through the 'dp_netdevs' shash while freeing 'dp'. */
 static void
@@ -1151,6 +1197,7 @@ dp_netdev_free(struct dp_netdev *dp)
 do_del_port(dp, port);
 }
 ovs_mutex_unlock(>port_mutex);
+
 dp_netdev_destroy_all_pmds(dp, true);
 cmap_destroy(>poll_threads);
 
@@ -1169,6 +1216,13 @@ dp_netdev_free(struct dp_netdev *dp)
 /* Upcalls must be disabled at this point */
 dp_netdev_destroy_upcall_lock(dp);
 
+for (int i = 0; i < MAX_METERS; ++i) {
+ovs_mutex_lock(>meter_locks[i]);
+dp_delete_meter(dp, i);
+ovs_mutex_unlock(>meter_locks[i]);
+ovs_mutex_destroy(>meter_locks[i]);
+}
+
 free(dp->pmd_cmask);
 free(CONST_CAST(char *, dp->name));
 free(dp);
@@ -3603,37 +3657,304 @@ static void
 dpif_netdev_meter_get_features(const struct dpif * dpif OVS_UNUSED,
struct ofputil_meter_features *features)
 {
-features->max_meters = 0;
-features->band_types = 0;
-features->capabilities = 0;
-features->max_bands = 0;
+features->max_meters = MAX_METERS;
+features->band_types = DP_SUPPORTED_METER_BAND_TYPES;
+features->capabilities = DP_SUPPORTED_METER_FLAGS_MASK;
+features->max_bands = MAX_BANDS;
 features->max_color = 0;
 }
 
+/* Returns false when packet needs to be dropped. */
+static void
+dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_,
+uint32_t

[ovs-dev] [userspace meter v3 4/5] ofproto: Meter translation.

2017-01-24 Thread Andy Zhou

From: Jarno Rajahalme 

Translate OpenFlow METER instructions to datapath meter actions.

Signed-off-by: Jarno Rajahalme 
Signed-off-by: Andy Zhou 
---
 include/openvswitch/ofp-actions.h |  1 +
 lib/dpif.c| 40 +---
 lib/ofp-actions.c |  1 +
 ofproto/ofproto-dpif-xlate.c  | 11 -
 ofproto/ofproto.c | 49 ++-
 5 files changed, 72 insertions(+), 30 deletions(-)

diff --git a/include/openvswitch/ofp-actions.h 
b/include/openvswitch/ofp-actions.h
index 8ca787a..e269901 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -532,6 +532,7 @@ struct ofpact_metadata {
 struct ofpact_meter {
 struct ofpact ofpact;
 uint32_t meter_id;
+uint32_t provider_meter_id;
 };
 
 /* OFPACT_WRITE_ACTIONS, OFPACT_CLONE.
diff --git a/lib/dpif.c b/lib/dpif.c
index 4e9476c..cc49d94 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1104,6 +1104,7 @@ struct dpif_execute_helper_aux {
 struct dpif *dpif;
 const struct flow *flow;
 int error;
+const struct nlattr *meter_action; /* Non-NULL, if have a meter action. */
 };
 
 /* This is called for actions that need the context of the datapath to be
@@ -1119,6 +1120,13 @@ dpif_execute_helper_cb(void *aux_, struct 
dp_packet_batch *packets_,
 ovs_assert(packets_->count == 1);
 
 switch ((enum ovs_action_attr)type) {
+case OVS_ACTION_ATTR_METER:
+/* Maintain a pointer to the first meter action seen. */
+if (!aux->meter_action) {
+aux->meter_action = action;
+}
+   break;
+
 case OVS_ACTION_ATTR_CT:
 case OVS_ACTION_ATTR_OUTPUT:
 case OVS_ACTION_ATTR_TUNNEL_PUSH:
@@ -1129,15 +1137,29 @@ dpif_execute_helper_cb(void *aux_, struct 
dp_packet_batch *packets_,
 struct ofpbuf execute_actions;
 uint64_t stub[256 / 8];
 struct pkt_metadata *md = >md;
-bool dst_set;
 
-dst_set = flow_tnl_dst_is_set(>tunnel);
-if (dst_set) {
+if (flow_tnl_dst_is_set(>tunnel) || aux->meter_action) {
+ofpbuf_use_stub(_actions, stub, sizeof stub);
+
+if (aux->meter_action) {
+const struct nlattr *a = aux->meter_action;
+
+do {
+ofpbuf_put(_actions, a, NLA_ALIGN(a->nla_len));
+/* Find next meter action before 'action', if any. */
+do {
+a = nl_attr_next(a);
+} while (a != action &&
+ nl_attr_type(a) != OVS_ACTION_ATTR_METER);
+} while (a != action);
+}
+
 /* The Linux kernel datapath throws away the tunnel information
  * that we supply as metadata.  We have to use a "set" action to
  * supply it. */
-ofpbuf_use_stub(_actions, stub, sizeof stub);
-odp_put_tunnel_action(>tunnel, _actions);
+if (md->tunnel.ip_dst) {
+odp_put_tunnel_action(>tunnel, _actions);
+}
 ofpbuf_put(_actions, action, NLA_ALIGN(action->nla_len));
 
 execute.actions = execute_actions.data;
@@ -1170,14 +1192,16 @@ dpif_execute_helper_cb(void *aux_, struct 
dp_packet_batch *packets_,
 
 dp_packet_delete(clone);
 
-if (dst_set) {
+if (flow_tnl_dst_is_set(>tunnel) || aux->meter_action) {
 ofpbuf_uninit(_actions);
+
+/* Do not re-use the same meters for later output actions. */
+aux->meter_action = NULL;
 }
 break;
 }
 
 case OVS_ACTION_ATTR_HASH:
-case OVS_ACTION_ATTR_METER:
 case OVS_ACTION_ATTR_PUSH_VLAN:
 case OVS_ACTION_ATTR_POP_VLAN:
 case OVS_ACTION_ATTR_PUSH_MPLS:
@@ -1201,7 +1225,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch 
*packets_,
 static int
 dpif_execute_with_help(struct dpif *dpif, struct dpif_execute *execute)
 {
-struct dpif_execute_helper_aux aux = {dpif, execute->flow, 0};
+struct dpif_execute_helper_aux aux = {dpif, execute->flow, 0, NULL};
 struct dp_packet_batch pb;
 
 COVERAGE_INC(dpif_execute_with_help);
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index cf1ad0f..733b2c5 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -6868,6 +6868,7 @@ ofpacts_pull_openflow_instructions(struct ofpbuf 
*openflow,
 
 om = ofpact_put_METER(ofpacts);
 om->meter_id = ntohl(oim->meter_id);
+om->provider_meter_id = UINT32_MAX; /* No provider meter ID. */
 }
 if (insts[OVSINST_OFPIT11_APPLY_ACTIONS]) {
 const struct ofp_action_header *actions;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 48b27a6..166e236 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4579,6 +4579,15 @@ xlate_clone(struct xlate_ctx *ctx, const struct

[ovs-dev] [userspace meter v3 3/5] dpif: Meter framework.

2017-01-24 Thread Andy Zhou

From: Jarno Rajahalme 

Add DPIF-level infrastructure for meters.  Allow meter_set to modify
the meter configuration (e.g. set the burst size if unspecified).

Signed-off-by: Jarno Rajahalme 
Signed-off-by: Andy Zhou 
---
 datapath/linux/compat/include/linux/openvswitch.h |  4 +-
 lib/dpif-netdev.c | 45 
 lib/dpif-netlink.c| 46 +++-
 lib/dpif-provider.h   | 29 
 lib/dpif.c| 88 +++
 lib/dpif.h| 13 +++-
 lib/odp-execute.c |  3 +
 lib/odp-util.c| 14 
 ofproto/ofproto-dpif-sflow.c  |  1 +
 ofproto/ofproto-dpif.c| 60 ++--
 ofproto/ofproto-provider.h| 13 ++--
 ofproto/ofproto.c |  2 +-
 12 files changed, 304 insertions(+), 14 deletions(-)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 425d3a4..b121391 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -787,13 +787,14 @@ enum ovs_nat_attr {
  * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
  * type may not be changed.
  *
- *
  * @OVS_ACTION_ATTR_SET_TO_MASKED: Kernel internal masked set action translated
  * from the @OVS_ACTION_ATTR_SET.
  * @OVS_ACTION_ATTR_TUNNEL_PUSH: Push tunnel header described by struct
  * ovs_action_push_tnl.
  * @OVS_ACTION_ATTR_TUNNEL_POP: Lookup tunnel port by port-no passed and pop
  * tunnel header.
+ * @OVS_ACTION_ATTR_METER: Run packet through a meter, which may drop the
+ * packet, or modify the packet (e.g., change the DSCP field).
  */
 
 enum ovs_action_attr {
@@ -819,6 +820,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
OVS_ACTION_ATTR_CLONE, /* Nested OVS_CLONE_ATTR_*.  */
+   OVS_ACTION_ATTR_METER, /* u32 meter number. */
 #endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 719a518..f81b464 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3597,6 +3597,46 @@ dp_netdev_disable_upcall(struct dp_netdev *dp)
 fat_rwlock_wrlock(>upcall_rwlock);
 }
 
+
+/* Meters */
+static void
+dpif_netdev_meter_get_features(const struct dpif * dpif OVS_UNUSED,
+   struct ofputil_meter_features *features)
+{
+features->max_meters = 0;
+features->band_types = 0;
+features->capabilities = 0;
+features->max_bands = 0;
+features->max_color = 0;
+}
+
+static int
+dpif_netdev_meter_set(struct dpif *dpif OVS_UNUSED,
+  ofproto_meter_id *meter_id OVS_UNUSED,
+  struct ofputil_meter_config *config OVS_UNUSED)
+{
+return EFBIG; /* meter_id out of range */
+}
+
+static int
+dpif_netdev_meter_get(const struct dpif *dpif OVS_UNUSED,
+  ofproto_meter_id meter_id OVS_UNUSED,
+  struct ofputil_meter_stats *stats OVS_UNUSED,
+  uint16_t n_bands OVS_UNUSED)
+{
+return EFBIG; /* meter_id out of range */
+}
+
+static int
+dpif_netdev_meter_del(struct dpif *dpif OVS_UNUSED,
+  ofproto_meter_id meter_id OVS_UNUSED,
+  struct ofputil_meter_stats *stats OVS_UNUSED,
+  uint16_t n_bands OVS_UNUSED)
+{
+return EFBIG; /* meter_id out of range */
+}
+
+
 static void
 dpif_netdev_disable_upcall(struct dpif *dpif)
 OVS_NO_THREAD_SAFETY_ANALYSIS
@@ -4721,6 +4761,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 break;
 }
 
+case OVS_ACTION_ATTR_METER:
 case OVS_ACTION_ATTR_PUSH_VLAN:
 case OVS_ACTION_ATTR_POP_VLAN:
 case OVS_ACTION_ATTR_PUSH_MPLS:
@@ -4858,6 +4899,10 @@ const struct dpif_class dpif_netdev_class = {
 dpif_netdev_ct_dump_next,
 dpif_netdev_ct_dump_done,
 dpif_netdev_ct_flush,
+dpif_netdev_meter_get_features,
+dpif_netdev_meter_set,
+dpif_netdev_meter_get,
+dpif_netdev_meter_del,
 };
 
 static void
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index c8b0e37..8a48227 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2356,6 +2356,46 @@ dpif_netlink_ct_flush(struct dpif *dpif OVS_UNUSED, 
const uint16_t *zone)
 }
 }
 
+
+/* Meters */
+static void
+dpif_netlink_meter_get_features(const struct dpif * dpif OVS_UNUSED,
+struct ofputil_meter_features *features)
+{
+features->max_meters = 0;
+

[ovs-dev] [userspace meter v3 2/5] dp-packet: Enhance packet batch APIs.

2017-01-24 Thread Andy Zhou

One common use case of 'struct dp_packet_batch' is to process all
packets in the batch in order. Add an iterator for this use case
to simplify the logic of calling sites,

Another common use case is to drop packets in the batch, by reading
all packets, but writing back pointers of fewer packets. Add macros
to support this use case.

Signed-off-by: Andy Zhou 
---
 lib/dp-packet.h  | 140 +++
 lib/dpif-netdev.c|  62 +--
 lib/dpif.c   |   2 +-
 lib/netdev-dummy.c   |  10 ++--
 lib/netdev-linux.c   |   3 +-
 lib/netdev.c |  24 
 lib/odp-execute.c|  55 -
 ofproto/ofproto-dpif-xlate.c |   2 +-
 tests/test-conntrack.c   |  56 +++--
 9 files changed, 201 insertions(+), 153 deletions(-)

diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index cf7d247..d0c14a5 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -632,79 +632,143 @@ reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
 enum { NETDEV_MAX_BURST = 32 }; /* Maximum number packets in a batch. */
 
 struct dp_packet_batch {
-int count;
+size_t count;
 bool trunc; /* true if the batch needs truncate. */
 struct dp_packet *packets[NETDEV_MAX_BURST];
 };
 
-static inline void dp_packet_batch_init(struct dp_packet_batch *b)
+static inline void
+dp_packet_batch_init(struct dp_packet_batch *batch)
 {
-b->count = 0;
-b->trunc = false;
+batch->count = 0;
+batch->trunc = false;
 }
 
 static inline void
-dp_packet_batch_clone(struct dp_packet_batch *dst,
-  struct dp_packet_batch *src)
+dp_packet_batch_add__(struct dp_packet_batch *batch,
+  struct dp_packet *packet, size_t limit)
 {
-int i;
-
-for (i = 0; i < src->count; i++) {
-dst->packets[i] = dp_packet_clone(src->packets[i]);
+if (batch->count < limit) {
+batch->packets[batch->count++] = packet;
+} else {
+dp_packet_delete(packet);
 }
-dst->count = src->count;
-dst->trunc = src->trunc;
 }
 
 static inline void
-packet_batch_init_packet(struct dp_packet_batch *b, struct dp_packet *p)
+dp_packet_batch_add(struct dp_packet_batch *batch, struct dp_packet *packet)
+{
+dp_packet_batch_add__(batch, packet, NETDEV_MAX_BURST);
+}
+
+static inline size_t
+dp_packet_batch_size(const struct dp_packet_batch *batch)
+{
+return batch->count;
+}
+
+/*
+ * Clear 'batch' for refill. Use dp_packet_batch_refill() to add
+ * packets back into the 'batch'.
+ *
+ * Return the original size of the 'batch'.  */
+static inline void
+dp_packet_batch_refill_init(struct dp_packet_batch *batch)
 {
-b->count = 1;
-b->trunc = false;
-b->packets[0] = p;
+batch->count = 0;
+};
+
+static inline void
+dp_packet_batch_refill(struct dp_packet_batch *batch,
+   struct dp_packet *packet, size_t idx)
+{
+dp_packet_batch_add__(batch, packet, MIN(NETDEV_MAX_BURST, idx + 1));
+}
+
+static inline void
+dp_packet_batch_init_packet(struct dp_packet_batch *batch, struct dp_packet *p)
+{
+dp_packet_batch_init(batch);
+batch->count = 1;
+batch->packets[0] = p;
+}
+
+static inline bool
+dp_packet_batch_is_empty(const struct dp_packet_batch *batch)
+{
+return !dp_packet_batch_size(batch);
+}
+
+#define DP_PACKET_BATCH_FOR_EACH(PACKET, BATCH)\
+for (size_t i = 0; i < dp_packet_batch_size(BATCH); i++)  \
+if ((PACKET = BATCH->packets[i]) != NULL)
+
+/* Use this macro for cases where some packets in the 'BATCH' may be
+ * dropped after going through each packet in the 'BATCH'.
+ *
+ * For packets to stay in the 'BATCH', they need to be refilled back
+ * into the 'BATCH' by calling dp_packet_batch_refill(). Caller owns
+ * the packets that are not refilled.
+ *
+ * Caller needs to supply 'SIZE', that stores the current number of
+ * packets in 'BATCH'. It is best to declare this variable with
+ * the 'const' modifier since it should not be modified by
+ * the iterator.  */
+#define DP_PACKET_BATCH_REFILL_FOR_EACH(IDX, SIZE, PACKET, BATCH)   \
+for (dp_packet_batch_refill_init(BATCH), IDX=0; IDX < SIZE; IDX++)  \
+ if ((PACKET = BATCH->packets[IDX]) != NULL)
+
+static inline void
+dp_packet_batch_clone(struct dp_packet_batch *dst,
+  struct dp_packet_batch *src)
+{
+struct dp_packet *packet;
+
+dp_packet_batch_init(dst);
+DP_PACKET_BATCH_FOR_EACH (packet, src) {
+dp_packet_batch_add(dst, dp_packet_clone(packet));
+}
 }
 
 static inline void
 dp_packet_delete_batch(struct dp_packet_batch *batch, bool may_steal)
 {
 if (may_steal) {
-int i;
+struct dp_packet *packet;
 
-for (i = 0; i < batch->count; i++) {
-dp_packet_delete(batch->packets[i]);
+DP_PACKET_BATCH_FOR_EACH (packet, batch) {
+dp_packet_delete(packet);
 }
+

[ovs-dev] [userspace meter v3 0/5] Userspace meter

2017-01-24 Thread Andy Zhou

Repost user space meter support. This is based Jarno's original work
at: https://mail.openvswitch.org/pipermail/ovs-dev/2015-November/306304.html.
With some enhancements, and rebased to current master.

---
v1-v2: rebase and repost.
v2-v3: simplify patch 2/5. 

Andy Zhou (2):
  netdev-dummy: Add --len option for netdev-dummy/receive command
  dp-packet: Enhance packet batch APIs.

Jarno Rajahalme (3):
  dpif: Meter framework.
  ofproto: Meter translation.
  dpif-netdev: Simple DROP meter implementation.

 datapath/linux/compat/include/linux/openvswitch.h |   4 +-
 include/openvswitch/ofp-actions.h |   1 +
 lib/dp-packet.h   | 140 +--
 lib/dpif-netdev.c | 433 --
 lib/dpif-netlink.c|  46 ++-
 lib/dpif-provider.h   |  29 ++
 lib/dpif.c| 128 ++-
 lib/dpif.h|  13 +-
 lib/netdev-dummy.c|  48 ++-
 lib/netdev-linux.c|   3 +-
 lib/netdev.c  |  24 +-
 lib/odp-execute.c |  58 +--
 lib/odp-util.c|  14 +
 lib/ofp-actions.c |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  |  13 +-
 ofproto/ofproto-dpif.c|  60 ++-
 ofproto/ofproto-provider.h|  13 +-
 ofproto/ofproto.c |  51 +--
 tests/dpif-netdev.at  | 106 ++
 tests/test-conntrack.c|  56 ++-
 21 files changed, 1036 insertions(+), 206 deletions(-)

-- 
1.9.1

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

[ovs-dev] [PATCH v2 2/3] routing-table: parse skb-mark from RTNETLINK msg

2017-01-24 Thread Pravin B Shelar

Keep track of skb-mark of given RTNL routing notification.
This will be used by next commit.

Signed-off-by: Pravin B Shelar 
Acked-by: Jarno Rajahalme 
---
 lib/route-table.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/route-table.c b/lib/route-table.c
index 00f95e3..61c8cd8 100644
--- a/lib/route-table.c
+++ b/lib/route-table.c
@@ -45,6 +45,7 @@ struct route_data {
 struct in6_addr rta_dst; /* 0 if missing. */
 struct in6_addr rta_gw;
 char ifname[IFNAMSIZ]; /* Interface name. */
+uint32_t mark;
 };
 
 /* A digested version of a route message sent down by the kernel to indicate
@@ -190,11 +191,13 @@ route_table_parse(struct ofpbuf *buf, struct 
route_table_msg *change)
 [RTA_DST] = { .type = NL_A_U32, .optional = true  },
 [RTA_OIF] = { .type = NL_A_U32, .optional = true },
 [RTA_GATEWAY] = { .type = NL_A_U32, .optional = true },
+[RTA_MARK] = { .type = NL_A_U32, .optional = true },
 };
 
 static const struct nl_policy policy6[] = {
 [RTA_DST] = { .type = NL_A_IPV6, .optional = true },
 [RTA_OIF] = { .type = NL_A_U32, .optional = true },
+[RTA_MARK] = { .type = NL_A_U32, .optional = true },
 [RTA_GATEWAY] = { .type = NL_A_IPV6, .optional = true },
 };
 
@@ -270,6 +273,9 @@ route_table_parse(struct ofpbuf *buf, struct 
route_table_msg *change)
 change->rd.rta_gw = nl_attr_get_in6_addr(attrs[RTA_GATEWAY]);
 }
 }
+if (attrs[RTA_MARK]) {
+change->rd.mark = nl_attr_get_u32(attrs[RTA_MARK]);
+}
 } else {
 VLOG_DBG_RL(, "received unparseable rtnetlink route message");
 return 0;
-- 
2.9.3

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

[ovs-dev] [PATCH v2 3/3] ovs-router: introduce pkt-mark.

2017-01-24 Thread Pravin B Shelar

OVS router is basically partial copy of linux kernel FIB.
kernel routing table uses skb-mark along with usual routing
parameters. Following patch brings in support for skb-mark
to ovs-router so that we can lookup route for given skb-mark.

Signed-off-by: Pravin B Shelar 
---
v1-v2:
Removed ovs/route2/add command
reverted change to plen variable type.
---
 lib/netdev-vport.c   |   4 +-
 lib/ovs-router.c | 125 ++-
 lib/ovs-router.h |   6 ++-
 lib/route-table.c|   2 +-
 ofproto/ofproto-dpif-sflow.c |   5 +-
 ofproto/ofproto-dpif-xlate.c |   2 +-
 tests/ovs-router.at  |  42 +++
 tests/tunnel-push-pop.at |   3 ++
 8 files changed, 147 insertions(+), 42 deletions(-)

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 88b0bcf..2d0aa43 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -260,10 +260,12 @@ tunnel_check_status_change__(struct netdev_vport *netdev)
 bool status = false;
 struct in6_addr *route;
 struct in6_addr gw;
+uint32_t mark;
 
 iface[0] = '\0';
 route = >tnl_cfg.ipv6_dst;
-if (ovs_router_lookup(route, iface, NULL, )) {
+mark = netdev->tnl_cfg.egress_pkt_mark;
+if (ovs_router_lookup(mark, route, iface, NULL, )) {
 struct netdev *egress_netdev;
 
 if (!netdev_open(iface, NULL, _netdev)) {
diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index 935b60a..d30eb3c 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -45,6 +45,11 @@
 #include "unaligned.h"
 #include "unixctl.h"
 #include "util.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(ovs_router);
+
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 
 static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
 static struct classifier cls;
@@ -57,6 +62,7 @@ struct ovs_router_entry {
 struct in6_addr src_addr;
 uint8_t plen;
 uint8_t priority;
+uint32_t mark;
 };
 
 static struct ovs_router_entry *
@@ -88,11 +94,12 @@ ovs_router_lookup_fallback(const struct in6_addr *ip6_dst, 
char output_bridge[],
 }
 
 bool
-ovs_router_lookup(const struct in6_addr *ip6_dst, char output_bridge[],
+ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst,
+  char output_bridge[],
   struct in6_addr *src, struct in6_addr *gw)
 {
 const struct cls_rule *cr;
-struct flow flow = {.ipv6_dst = *ip6_dst};
+struct flow flow = {.ipv6_dst = *ip6_dst, .pkt_mark = mark};
 
 cr = classifier_lookup(, OVS_VERSION_MAX, , NULL);
 if (cr) {
@@ -115,7 +122,8 @@ rt_entry_free(struct ovs_router_entry *p)
 free(p);
 }
 
-static void rt_init_match(struct match *match, const struct in6_addr *ip6_dst,
+static void rt_init_match(struct match *match, uint32_t mark,
+  const struct in6_addr *ip6_dst,
   uint8_t plen)
 {
 struct in6_addr dst;
@@ -127,6 +135,8 @@ static void rt_init_match(struct match *match, const struct 
in6_addr *ip6_dst,
 memset(match, 0, sizeof *match);
 match->flow.ipv6_dst = dst;
 match->wc.masks.ipv6_dst = mask;
+match->wc.masks.pkt_mark = UINT32_MAX;
+match->flow.pkt_mark = mark;
 }
 
 static int
@@ -178,7 +188,8 @@ out:
 }
 
 static int
-ovs_router_insert__(uint8_t priority, const struct in6_addr *ip6_dst,
+ovs_router_insert__(uint32_t mark, uint8_t priority,
+const struct in6_addr *ip6_dst,
 uint8_t plen, const char output_bridge[],
 const struct in6_addr *gw)
 {
@@ -187,13 +198,14 @@ ovs_router_insert__(uint8_t priority, const struct 
in6_addr *ip6_dst,
 struct match match;
 int err;
 
-rt_init_match(, ip6_dst, plen);
+rt_init_match(, mark, ip6_dst, plen);
 
 p = xzalloc(sizeof *p);
 ovs_strlcpy(p->output_bridge, output_bridge, sizeof p->output_bridge);
 if (ipv6_addr_is_set(gw)) {
 p->gw = *gw;
 }
+p->mark = mark;
 p->nw_addr = match.flow.ipv6_dst;
 p->plen = plen;
 p->priority = priority;
@@ -202,7 +214,12 @@ ovs_router_insert__(uint8_t priority, const struct 
in6_addr *ip6_dst,
 err = get_src_addr(gw, output_bridge, >src_addr);
 }
 if (err) {
+struct ds ds = DS_EMPTY_INITIALIZER;
+
+ipv6_format_mapped(ip6_dst, );
+VLOG_DBG_RL(, "src addr not available for route %s", ds_cstr());
 free(p);
+ds_destroy();
 return err;
 }
 /* Longest prefix matches first. */
@@ -222,13 +239,12 @@ ovs_router_insert__(uint8_t priority, const struct 
in6_addr *ip6_dst,
 }
 
 void
-ovs_router_insert(const struct in6_addr *ip_dst, uint8_t plen,
+ovs_router_insert(uint32_t mark, const struct in6_addr *ip_dst, uint8_t plen,
   const char output_bridge[], const struct in6_addr *gw)
 {
-ovs_router_insert__(plen, ip_dst, plen, output_bridge, gw);
+ovs_router_insert__(mark, plen, ip_dst, plen, output_bridge, gw);
 }
 
-

[ovs-dev] [PATCH v2 1/3] tunnel: Add support to configure ptk_mark

2017-01-24 Thread Pravin B Shelar

Today packet mark action is broken for Tunnel ports with
tunnel monitoring. User can write a flow to set pkt-mark for
any tunnel traffic, but there is no way to set the packet
mark for corresponding BFD trffic.

Following patch introduces new option in OVSDB tunnel
configuration so that user can set skb-mark for given
tunnel endpoint. OVS would set the mark according to the
skb-mark option for all tunnel traffic including packets
generated by vSwitchd like tunnel monitoring BFD packet.

Signed-off-by: Pravin B Shelar 
---
v1-v2:
Handle zero egress_pkt_mark
Added more documentation.
---
 NEWS |  2 ++
 lib/netdev-vport.c   |  7 +++
 lib/netdev.h |  2 ++
 ofproto/tunnel.c |  5 +
 tests/tunnel-push-pop.at | 16 
 vswitchd/vswitch.xml |  6 ++
 6 files changed, 38 insertions(+)

diff --git a/NEWS b/NEWS
index 0a9551c..6838649 100644
--- a/NEWS
+++ b/NEWS
@@ -51,6 +51,8 @@ Post-v2.6.0
a per-OpenFlow bridge basis rather than globally. (The interface
has not changed.)
  * Removed support for IPsec tunnels.
+ * Added support to set packet mark for tunnel endpoint using
+   `egress_pkt_mark` OVSDB option.
- DPDK:
  * New option 'n_rxq_desc' and 'n_txq_desc' fields for DPDK interfaces
which set the number of rx and tx descriptors to use for the given port.
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 4c2ced5..88b0bcf 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -509,6 +509,9 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
 }
 
 free(str);
+} else if (!strcmp(node->key, "egress_pkt_mark")) {
+tnl_cfg.egress_pkt_mark = strtoul(node->value, NULL, 10);
+tnl_cfg.set_egress_pkt_mark = true;
 } else {
 ds_put_format(, "%s: unknown %s argument '%s'\n",
   name, type, node->key);
@@ -649,6 +652,10 @@ get_tunnel_config(const struct netdev *dev, struct smap 
*args)
 smap_add(args, "df_default", "false");
 }
 
+if (tnl_cfg.set_egress_pkt_mark) {
+smap_add_format(args, "egress_pkt_mark",
+"%"PRIu32, tnl_cfg.egress_pkt_mark);
+}
 return 0;
 }
 
diff --git a/lib/netdev.h b/lib/netdev.h
index bef9cdd..d6c07c1 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -89,6 +89,8 @@ struct netdev_tunnel_config {
 struct in6_addr ipv6_dst;
 
 uint32_t exts;
+bool set_egress_pkt_mark;
+uint32_t egress_pkt_mark;
 
 uint8_t ttl;
 bool ttl_inherit;
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index ce727f4..e285d54 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -461,6 +461,11 @@ tnl_port_send(const struct ofport_dpif *ofport, struct 
flow *flow,
 | (cfg->csum ? FLOW_TNL_F_CSUM : 0)
 | (cfg->out_key_present ? FLOW_TNL_F_KEY : 0);
 
+if (cfg->set_egress_pkt_mark) {
+flow->pkt_mark = cfg->egress_pkt_mark;
+wc->masks.pkt_mark = UINT32_MAX;
+}
+
 if (pre_flow_str) {
 char *post_flow_str = flow_to_string(flow);
 char *tnl_str = tnl_port_fmt(tnl_port);
diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
index 700ef55..4aaa669 100644
--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -12,6 +12,8 @@ AT_CHECK([ovs-vsctl add-port int-br t2 -- set Interface t2 
type=vxlan \
options:remote_ip=1.1.2.93 options:out_key=flow 
options:csum=true ofport_request=4\
 -- add-port int-br t4 -- set Interface t4 type=geneve \
options:remote_ip=flow options:key=123 ofport_request=5\
+-- add-port int-br t5 -- set Interface t5 type=geneve \
+   options:remote_ip=1.1.2.93 options:out_key=flow 
options:egress_pkt_mark=1234 ofport_request=6\
], [0])
 
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
@@ -25,6 +27,7 @@ dummy@ovs-dummy: hit:0 missed:0
t2 2/4789: (vxlan: key=123, remote_ip=1.1.2.92)
t3 4/4789: (vxlan: csum=true, out_key=flow, remote_ip=1.1.2.93)
t4 5/6081: (geneve: key=123, remote_ip=flow)
+   t5 6/6081: (geneve: egress_pkt_mark=1234, out_key=flow, 
remote_ip=1.1.2.93)
 ])
 
 dnl First setup dummy interface IP address, then add the route
@@ -91,6 +94,12 @@ AT_CHECK([tail -1 stdout], [0],
   [Datapath actions: tnl_pop(6081)
 ])
 
+dnl Check Geneve tunnel (t6) pop
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0800),ipv4(src=1.1.2.96,dst=1.1.2.88,proto=17,tos=0,ttl=64,frag=no),udp(src=51283,dst=6081)'],
 [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: tnl_pop(6081)
+])
+
 dnl Check VXLAN tunnel push
 AT_CHECK([ovs-ofctl add-flow int-br action=2])
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy

[ovs-dev] [PATCH v2 0/3] Tunnel: add support for packet marking

2017-01-24 Thread Pravin B Shelar

Following patch series adds support for setting packet
mark for tunnel traffic. This allows better integration
with linux networking stack.

v1-v2:
Fixed patch 1 an 3 according to comments from Jarno.

Pravin B Shelar (3):
  tunnel: Add support to configure ptk_mark
  routing-table: parse skb-mark from RTNETLINK msg
  ovs-router: introduce pkt-mark.

 NEWS |   2 +
 lib/netdev-vport.c   |  11 +++-
 lib/netdev.h |   2 +
 lib/ovs-router.c | 125 ++-
 lib/ovs-router.h |   6 ++-
 lib/route-table.c|   8 ++-
 ofproto/ofproto-dpif-sflow.c |   5 +-
 ofproto/ofproto-dpif-xlate.c |   2 +-
 ofproto/tunnel.c |   5 ++
 tests/ovs-router.at  |  42 +++
 tests/tunnel-push-pop.at |  19 +++
 vswitchd/vswitch.xml |   6 +++
 12 files changed, 191 insertions(+), 42 deletions(-)

-- 
2.9.3

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

Re: [ovs-dev] [PATCH ovs V2 03/21] other-config: Add hw-offload switch to control netdev flow offloading

2017-01-24 Thread Roi Dayan




On 1/24/2017 9:12 PM, Joe Stringer wrote:

On 24 January 2017 at 00:45, Roi Dayan  wrote:


On 1/23/2017 8:41 PM, Joe Stringer wrote:

On 22 January 2017 at 08:13, Paul Blakey  wrote:


On 05/01/2017 03:26, Joe Stringer wrote:

On 25 December 2016 at 03:39, Paul Blakey  wrote:

Add a new configuration option - hw-offload that enables netdev
flow api. Enabling this option will allow offloading flows
using netdev implementation instead of the kernel datapath.
This configuration option defaults to false - disabled.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
   lib/netdev.c | 18 ++
   lib/netdev.h |  2 ++
   vswitchd/bridge.c|  2 ++
   vswitchd/vswitch.xml | 11 +++
   4 files changed, 33 insertions(+)

diff --git a/lib/netdev.c b/lib/netdev.c
index 3ac3c48..b289166 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2071,7 +2071,25 @@ netdev_init_flow_api(struct netdev *netdev)
   {
   const struct netdev_class *class = netdev->netdev_class;

+if (!netdev_flow_api_enabled) {
+return EOPNOTSUPP;
+}
+
   return (class->init_flow_api
   ? class->init_flow_api(netdev)
   : EOPNOTSUPP);
   }
+
+bool netdev_flow_api_enabled = false;
+
+void
+netdev_set_flow_api_enabled(bool enabled)
+{
+static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+
+if (ovsthread_once_start()) {
+netdev_flow_api_enabled = enabled;
+VLOG_INFO("netdev: Flow API %s", enabled ? "Enabled" :
"Disabled");
+ovsthread_once_done();
+}
+}


Requiring restart to apply this option seems a bit arbitrary, why not
allow it to be changed at runtime?


Because it isn't something that is supposed to change often,
It might cause problems if its enabled later on with different (netdev)
implementations. We can try and test changing it to runtime at a later
time.

Forcing the user to restart OVS to make a database setting take effect
is an unusual requirement. Even uncommon configurations that require
datapath-level changes (eg, reconfiguring queues) is applied in OVS
without requiring the user to intervene and restart. I understand that
even DPDK init doesn't require restart any more.

Here's another question: Let's say that hardware offloads is enabled
and traffic runs. User turns it off, restarts OVS. What happens to the
hardware flows? Presumably OVS doesn't manage them any more because
hw-offloads is off, but they would continue to forward traffic. If OVS
then gets different OpenFlow rules then the forwarding could be wrong.


Hi Joe,

We understand that forcing the user to restart OVS is not something common
we want to do.
We can make this option changeable at runtime and flush all the rules when
hw-offload changes from true to false.
If something will block us we'll let you know and if a future requirement
will need this then it can be changed later.

To answer your next question then this is also what happens now when the
user restarts OVS. All rules are flushed so there shoudn't be HW forwarding
rules.

How are they flushed?

I don't think ovs-vswitchd flushes flows when it stops.

Typically if you restart OVS, then on startup the flow table will be
empty so before you get a chance to install your OpenFlow flows again,
OVS will be running, dumping datapath flows, and deleting them because
they don't match the current OpenFlow table. However, now that you've
disabled hw flows, I suspect that OVS won't try to manage them. So the
hw flows will continue to forward according to policy prior to
ovs-vswitchd shutdown.

There is also a way to start OVS where it will wait for you to finish
initializing the OpenFlow tables before it starts revalidating the
flows (ovs_vsctl set open_vswitch .
other_config:flow-restore-wait="true") - see utilities/ovs-ctl.in for
how this may be used.

These are all only considerations where the datapath has its own life
beyond the OVS binary. They don't apply when running with userspace
datapath, clearly, because the datapath and ovs-vswitchd are the same
in that case.


They are flushed on next start. In netdev_linux_set_policing we added a 
call to flush all rules without checking the hw-offload config.
Also before it in master branch there is a call to remove ingress and 
readd it. removing the ingress cause flush of all the rules.

So on restart we were always in a situation were all rules are flushed.


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

[ovs-dev] [patch_v4 6/6] Enhance V6 NAT test.

2017-01-24 Thread Darrell Ball

Signed-off-by: Darrell Ball 
---
 tests/system-traffic.at | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 29dd6d6..a15e059 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2777,15 +2777,17 @@ ADD_VETH(p0, at_ns0, br0, "fc00::1/96")
 NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address 80:88:88:88:88:88])
 ADD_VETH(p1, at_ns1, br0, "fc00::2/96")
 NS_CHECK_EXEC([at_ns1], [ip -6 neigh add fc00::240 lladdr 80:88:88:88:88:88 
dev p1])
+NS_CHECK_EXEC([at_ns1], [ip -6 neigh add fc00::241 lladdr 80:88:88:88:88:88 
dev p1])
 
 dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from 
ns1->ns0.
 AT_DATA([flows.txt], [dnl
 priority=1,action=drop
 priority=10,icmp6,action=normal
-priority=100,in_port=1,ip6,action=ct(commit,nat(src=fc00::240)),2
+priority=100,in_port=1,ip6,action=ct(commit,nat(src=fc00::240-fc00::241)),2
 priority=100,in_port=2,ct_state=-trk,ip6,action=ct(nat,table=0)
 priority=100,in_port=2,ct_state=+trk+est,ip6,action=1
 
priority=200,in_port=2,ct_state=+trk+new,icmp6,icmpv6_code=0,icmpv6_type=135,nd_target=fc00::240,action=ct(commit,nat(dst=fc00::1)),1
+priority=200,in_port=2,ct_state=+trk+new,icmp6,icmpv6_code=0,icmpv6_type=135,nd_target=fc00::241,action=ct(commit,nat(dst=fc00::1)),1
 ])
 
 AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
-- 
1.9.1

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

[ovs-dev] [patch_v4 3/6] Userspace Datapath: Introduce NAT support.

2017-01-24 Thread Darrell Ball

This patch introduces NAT support for the userspace datapath.
The conntrack module changes are in this patch.

The per packet scope of lookups for NAT and un_NAT is at
the bucket level rather than global. One hash table is
introduced to support create/delete handling. The create/delete
events may be further optimized, if the need becomes clear.

Some NAT options with limited utility (persistent, random) are
not supported yet, but will be supported in a later patch.

Signed-off-by: Darrell Ball 
---
 lib/conntrack-private.h |  16 +-
 lib/conntrack.c | 740 ++--
 lib/conntrack.h |  44 +++
 3 files changed, 717 insertions(+), 83 deletions(-)

diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 493865f..b71af37 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -51,14 +51,23 @@ struct conn_key {
 uint16_t zone;
 };
 
+struct nat_conn_key_node {
+struct hmap_node node;
+struct conn_key key;
+struct conn_key value;
+};
+
 struct conn {
 struct conn_key key;
 struct conn_key rev_key;
 long long expiration;
 struct ovs_list exp_node;
 struct hmap_node node;
-uint32_t mark;
 ovs_u128 label;
+/* XXX: consider flattening. */
+struct nat_action_info_t *nat_info;
+uint32_t mark;
+uint8_t conn_type;
 };
 
 enum ct_update_res {
@@ -67,6 +76,11 @@ enum ct_update_res {
 CT_UPDATE_NEW,
 };
 
+enum ct_conn_type {
+CT_CONN_TYPE_DEFAULT,
+   CT_CONN_TYPE_UN_NAT,
+};
+
 struct ct_l4_proto {
 struct conn *(*new_conn)(struct conntrack_bucket *, struct dp_packet *pkt,
  long long now);
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 0a611a2..34728a6 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -76,6 +76,20 @@ static void set_label(struct dp_packet *, struct conn *,
   const struct ovs_key_ct_labels *mask);
 static void *clean_thread_main(void *f_);
 
+static struct nat_conn_key_node *
+nat_conn_keys_lookup(struct hmap *nat_conn_keys,
+ const struct conn_key *key,
+ uint32_t basis);
+
+static void
+nat_conn_keys_remove(struct hmap *nat_conn_keys,
+const struct conn_key *key,
+uint32_t basis);
+
+static bool
+nat_select_range_tuple(struct conntrack *ct, const struct conn *conn,
+  struct conn *nat_conn);
+
 static struct ct_l4_proto *l4_protos[] = {
 [IPPROTO_TCP] = _proto_tcp,
 [IPPROTO_UDP] = _proto_other,
@@ -90,9 +104,11 @@ long long ct_timeout_val[] = {
 };
 
 /* If the total number of connections goes above this value, no new connections
- * are accepted */
+ * are accepted; this is for CT_CONN_TYPE_DEFAULT connections. */
 #define DEFAULT_N_CONN_LIMIT 300
 
+#define DT
+
 /* Initializes the connection tracker 'ct'.  The caller is responsible for
  * calling 'conntrack_destroy()', when the instance is not needed anymore */
 void
@@ -101,6 +117,11 @@ conntrack_init(struct conntrack *ct)
 unsigned i, j;
 long long now = time_msec();
 
+ct_rwlock_init(>nat_resources_lock);
+ct_rwlock_wrlock(>nat_resources_lock);
+hmap_init(>nat_conn_keys);
+ct_rwlock_unlock(>nat_resources_lock);
+
 for (i = 0; i < CONNTRACK_BUCKETS; i++) {
 struct conntrack_bucket *ctb = >buckets[i];
 
@@ -139,13 +160,24 @@ conntrack_destroy(struct conntrack *ct)
 ovs_mutex_destroy(>cleanup_mutex);
 ct_lock_lock(>lock);
 HMAP_FOR_EACH_POP(conn, node, >connections) {
-atomic_count_dec(>n_conn);
+if (conn->conn_type == CT_CONN_TYPE_DEFAULT) {
+atomic_count_dec(>n_conn);
+}
 delete_conn(conn);
 }
 hmap_destroy(>connections);
 ct_lock_unlock(>lock);
 ct_lock_destroy(>lock);
 }
+ct_rwlock_wrlock(>nat_resources_lock);
+struct nat_conn_key_node *nat_conn_key_node;
+HMAP_FOR_EACH_POP(nat_conn_key_node, node, >nat_conn_keys) {
+free(nat_conn_key_node);
+}
+hmap_destroy(>nat_conn_keys);
+ct_rwlock_unlock(>nat_resources_lock);
+ct_rwlock_destroy(>nat_resources_lock);
+
 }
 
 static unsigned hash_to_bucket(uint32_t hash)
@@ -167,10 +199,175 @@ write_ct_md(struct dp_packet *pkt, uint16_t state, 
uint16_t zone,
 pkt->md.ct_label = label;
 }
 
+static void
+nat_packet(struct dp_packet *pkt, const struct conn *conn, uint16_t *state)
+{
+if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+   *state |= CS_SRC_NAT;
+if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
+struct ip_header *nh = dp_packet_l3(pkt);
+packet_set_ipv4_addr(pkt, >ip_src,
+conn->rev_key.dst.addr.ipv4_aligned);
+} else if (conn->key.dl_type == htons(ETH_TYPE_IPV6)) {
+struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt);
+struct in6_addr ipv6_addr;
+

[ovs-dev] [patch_v4 1/6] Export packet_set_ipv6_addr()fordpdkdatapath.

2017-01-24 Thread Darrell Ball

Signed-off-by: Darrell Ball 
---
 lib/packets.c | 2 +-
 lib/packets.h | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/packets.c b/lib/packets.c
index fa70df6..94e7d87 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -986,7 +986,7 @@ packet_update_csum128(struct dp_packet *packet, uint8_t 
proto,
 }
 }
 
-static void
+void
 packet_set_ipv6_addr(struct dp_packet *packet, uint8_t proto,
  ovs_16aligned_be32 addr[4],
  const struct in6_addr *new_addr,
diff --git a/lib/packets.h b/lib/packets.h
index c4d3799..850f192 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1100,6 +1100,10 @@ void packet_set_ipv4_addr(struct dp_packet *packet, 
ovs_16aligned_be32 *addr,
 void packet_set_ipv6(struct dp_packet *, const struct in6_addr *src,
  const struct in6_addr *dst, uint8_t tc,
  ovs_be32 fl, uint8_t hlmit);
+void packet_set_ipv6_addr(struct dp_packet *packet, uint8_t proto,
+  ovs_16aligned_be32 addr[4],
+  const struct in6_addr *new_addr,
+  bool recalculate_csum);
 void packet_set_tcp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
 void packet_set_udp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
 void packet_set_sctp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
-- 
1.9.1

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

[ovs-dev] [patch_v4 4/6] Unset CS_NEW for established connections.

2017-01-24 Thread Darrell Ball

Signed-off-by: Darrell Ball 
---
 lib/conntrack.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 34728a6..aaecb00 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -443,6 +443,7 @@ conn_update_state(struct conntrack *ct, struct dp_packet 
*pkt,
 switch (res) {
 case CT_UPDATE_VALID:
 *state |= CS_ESTABLISHED;
+*state &= ~CS_NEW;
 if (ctx->reply) {
 *state |= CS_REPLY_DIR;
 }
-- 
1.9.1

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

[ovs-dev] [patch_v4 0/6] Userspace Datapath: Introduce NAT support.

2017-01-24 Thread Darrell Ball

This patch series introduces NAT support for the userspace datapath.

The per packet scope of lookups for NAT and un_NAT is at
the bucket level rather than global. One hash table is 
introduced to support create/delete handling. The create/delete
events may be further optimized, if the need becomes clear.

The existing NAT tests are enabled for the dpdk datapath,
with an added enhancement to the V6 NAT test.

Some NAT options with limited utility (persistent, random) are
not supported yet, but will be supported in a later patch.

One V6 api is exported to facilitate selective editing the V6
header - packet_set_ipv6_addr().

alg and fragmentation support are not included here but are
being worked on.

NEWS is not updated in this series yet, until confirmation of
release.

I realize patch 3 is big. It may be clearer and easier to keep
as a single patch, so I have done that after some discussion.

v3->v4: Fix rev_key vs key for nat_conn_keys access in a couple
places; this would have affected cleanup; at same time
rename some variables and change nat_conn_keys APIs to
use conn key, rather than conn.

Fix conntrack_flush() CT_CONN_TYPE_DEFAULT flag placement;
the intention was that it be the same as in sweep_bucket().

Fix nat_ipv6_addrs_delta() max boundary checking logic. I
also enhanced the conntrack - IPv6 HTTP with NAT test to
give it more coverage as partial penance.

Rebase

v2->v3: Fix a theoretical resend for closed connection restart.
Parse out a function to help and also limit
conn_state_update() to one.

I decided to cap V6 address range delta at 4 billion using
internal adjustment (user visibility not required).

Some cleanup of deprecated code path.

Parse out some more changes as separate patches.
 
v1->v2: Updates/fixes that were missed in v1 patches.

Darrell Ball (6):
  Export packet_set_ipv6_addr()fordpdkdatapath.
  Parse NAT netlink for userspace datapath.
  Userspace Datapath: Introduce NAT support.
  Unset CS_NEW for established connections.
  Enable NAT tests for userspace datapath.
  Enhance V6 NAT test.

 lib/conntrack-private.h  |  25 +-
 lib/conntrack.c  | 742 ++-
 lib/conntrack.h  |  73 +++-
 lib/dpif-netdev.c|  85 -
 lib/packets.c|   2 +-
 lib/packets.h|   4 +
 tests/system-traffic.at  |   4 +-
 tests/system-userspace-macros.at |   7 +-
 tests/test-conntrack.c   |   8 +-
 9 files changed, 843 insertions(+), 107 deletions(-)

-- 
1.9.1

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

[ovs-dev] [patch_v4 2/6] Parse NAT netlink for userspace datapath.

2017-01-24 Thread Darrell Ball

Signed-off-by: Darrell Ball 
---
 lib/conntrack-private.h |  9 --
 lib/conntrack.c |  3 +-
 lib/conntrack.h | 29 -
 lib/dpif-netdev.c   | 85 ++---
 tests/test-conntrack.c  |  8 +++--
 5 files changed, 116 insertions(+), 18 deletions(-)

diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 013f19f..493865f 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -29,15 +29,6 @@
 #include "packets.h"
 #include "unaligned.h"
 
-struct ct_addr {
-union {
-ovs_16aligned_be32 ipv4;
-union ovs_16aligned_in6_addr ipv6;
-ovs_be32 ipv4_aligned;
-struct in6_addr ipv6_aligned;
-};
-};
-
 struct ct_endpoint {
 struct ct_addr addr;
 union {
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 9bea3d9..0a611a2 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -273,7 +273,8 @@ conntrack_execute(struct conntrack *ct, struct 
dp_packet_batch *pkt_batch,
   ovs_be16 dl_type, bool commit, uint16_t zone,
   const uint32_t *setmark,
   const struct ovs_key_ct_labels *setlabel,
-  const char *helper)
+  const char *helper,
+  const struct nat_action_info_t *nat_action_info OVS_UNUSED)
 {
 struct dp_packet **pkts = pkt_batch->packets;
 size_t cnt = pkt_batch->count;
diff --git a/lib/conntrack.h b/lib/conntrack.h
index 254f61c..471e529 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -26,6 +26,7 @@
 #include "openvswitch/thread.h"
 #include "openvswitch/types.h"
 #include "ovs-atomic.h"
+#include "packets.h"
 
 /* Userspace connection tracker
  * 
@@ -61,6 +62,31 @@ struct dp_packet_batch;
 
 struct conntrack;
 
+struct ct_addr {
+union {
+ovs_16aligned_be32 ipv4;
+union ovs_16aligned_in6_addr ipv6;
+ovs_be32 ipv4_aligned;
+struct in6_addr ipv6_aligned;
+};
+};
+
+enum nat_action_e {
+   NAT_ACTION = 1 << 0,
+   NAT_ACTION_SRC = 1 << 1,
+   NAT_ACTION_SRC_PORT = 1 << 2,
+   NAT_ACTION_DST = 1 << 3,
+   NAT_ACTION_DST_PORT = 1 << 4,
+};
+
+struct nat_action_info_t {
+   struct ct_addr min_addr;
+   struct ct_addr max_addr;
+   uint16_t min_port;
+   uint16_t max_port;
+uint16_t nat_action;
+};
+
 void conntrack_init(struct conntrack *);
 void conntrack_destroy(struct conntrack *);
 
@@ -68,7 +94,8 @@ int conntrack_execute(struct conntrack *, struct 
dp_packet_batch *,
   ovs_be16 dl_type, bool commit,
   uint16_t zone, const uint32_t *setmark,
   const struct ovs_key_ct_labels *setlabel,
-  const char *helper);
+  const char *helper,
+  const struct nat_action_info_t *nat_action_info);
 
 struct conntrack_dump {
 struct conntrack *ct;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 42631bc..0ca4291 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -97,7 +97,8 @@ static struct shash dp_netdevs OVS_GUARDED_BY(dp_netdev_mutex)
 static struct vlog_rate_limit upcall_rl = VLOG_RATE_LIMIT_INIT(600, 600);
 
 #define DP_NETDEV_CS_SUPPORTED_MASK (CS_NEW | CS_ESTABLISHED | CS_RELATED \
- | CS_INVALID | CS_REPLY_DIR | CS_TRACKED)
+ | CS_INVALID | CS_REPLY_DIR | CS_TRACKED \
+ | CS_SRC_NAT | CS_DST_NAT)
 #define DP_NETDEV_CS_UNSUPPORTED_MASK (~(uint32_t)DP_NETDEV_CS_SUPPORTED_MASK)
 
 static struct odp_support dp_netdev_support = {
@@ -4688,7 +4689,9 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 const char *helper = NULL;
 const uint32_t *setmark = NULL;
 const struct ovs_key_ct_labels *setlabel = NULL;
-
+struct nat_action_info_t nat_action_info;
+bool nat = false;
+memset(_action_info, 0, sizeof nat_action_info);
 NL_ATTR_FOR_EACH_UNSAFE (b, left, nl_attr_get(a),
  nl_attr_get_size(a)) {
 enum ovs_ct_attr sub_type = nl_attr_type(b);
@@ -4709,15 +4712,89 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 case OVS_CT_ATTR_LABELS:
 setlabel = nl_attr_get(b);
 break;
-case OVS_CT_ATTR_NAT:
+case OVS_CT_ATTR_NAT: {
+const struct nlattr *b_nest;
+unsigned int left_nest;
+bool ip_min_specified = false;
+bool proto_num_min_specified = false;
+bool ip_max_specified = false;
+bool proto_num_max_specified = false;
+
+NL_NESTED_FOR_EACH_UNSAFE (b_nest, left_nest, b) {
+enum ovs_nat_attr sub_type_nest = nl_attr_type(b_nest);
+
+switch(sub_type_nest) {
+

[ovs-dev] [PATCH] selinux: Allow creating tap devices.

2017-01-24 Thread Daniele Di Proietto

Current SELinux policy in RHEL and Fedora doesn't allow the creation of
TAP devices.

A tap device is used by dpif-netdev to create internal devices.

Without this patch, adding any bridge backed by the userspace datapath
would fail.

This doesn't mean that we can run Open vSwitch with DPDK under SELinux
yet, but at least we can use the userspace datapath.

Signed-off-by: Daniele Di Proietto 
---
 selinux/openvswitch-custom.te | 5 +
 1 file changed, 5 insertions(+)

diff --git a/selinux/openvswitch-custom.te b/selinux/openvswitch-custom.te
index 47ddb562c..98de89c98 100644
--- a/selinux/openvswitch-custom.te
+++ b/selinux/openvswitch-custom.te
@@ -5,8 +5,11 @@ require {
 type openvswitch_tmp_t;
 type ifconfig_exec_t;
 type hostname_exec_t;
+type tun_tap_device_t;
 class netlink_socket { setopt getopt create connect getattr write read 
};
 class file { write getattr read open execute execute_no_trans };
+class chr_file { ioctl open read write };
+class tun_socket { create };
 }
 
 #= openvswitch_t ==
@@ -14,3 +17,5 @@ allow openvswitch_t self:netlink_socket { setopt getopt 
create connect getattr w
 allow openvswitch_t hostname_exec_t:file { read getattr open execute 
execute_no_trans };
 allow openvswitch_t ifconfig_exec_t:file { read getattr open execute 
execute_no_trans };
 allow openvswitch_t openvswitch_tmp_t:file { execute execute_no_trans };
+allow openvswitch_t self:tun_socket { create };
+allow openvswitch_t tun_tap_device_t:chr_file { ioctl open read write };
-- 
2.11.0

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

[ovs-dev] [PATCH 3/3] rhel: Fix ifup and ifdown after DPDK naming change.

2017-01-24 Thread Daniele Di Proietto

Names like dpdk0 and dpdk1 are not enough to identify a DPDK interface.
We could update README.RHEL.rst and add

OVS_EXTRA='set Interface ${DEVICE} options:dpdk-devargs=:01:00.0'

but a better solution is to add new parameters in the configuration file
to explicitly specify the dpdk-devargs.

Fixes: 55e075e65ef9("netdev-dpdk: Arbitrary 'dpdk' port naming")
Signed-off-by: Daniele Di Proietto 
---
 rhel/README.RHEL.rst| 13 +
 rhel/etc_sysconfig_network-scripts_ifup-ovs |  6 --
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/rhel/README.RHEL.rst b/rhel/README.RHEL.rst
index afccf1703..af4589325 100644
--- a/rhel/README.RHEL.rst
+++ b/rhel/README.RHEL.rst
@@ -266,14 +266,16 @@ DPDK NIC port:
 
 ::
 
-==> ifcfg-dpdk0 <==
-DPDK vhost-user port:
-DEVICE=dpdk0
+==> ifcfg-mydpdk0 <==
+DEVICE=mydpdk0
+DPDK_DEVARGS=":01:00.0"
 ONBOOT=yes
 DEVICETYPE=ovs
 TYPE=OVSDPDKPort
 OVS_BRIDGE=obr0
 
+DPDK vhost-user port:
+
 ::
 
 ==> ifcfg-vhu0 <==
@@ -283,6 +285,8 @@ DPDK NIC port:
 TYPE=OVSDPDKVhostUserPort
 OVS_BRIDGE=obr0
 
+DPDK bond:
+
 ::
 
 ==> ifcfg-bond0 <==
@@ -292,7 +296,8 @@ DPDK NIC port:
 TYPE=OVSDPDKBond
 OVS_BRIDGE=ovsbridge0
 BOOTPROTO=none
-BOND_IFACES="dpdk0 dpdk1"
+BOND_IFACES="mydpdk0 mydpdk1"
+BOND_DPDK_DEVARGS=":01:00.0 :06:00.0"
 OVS_OPTIONS="bond_mode=active-backup"
 HOTPLUG=no
 
diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs 
b/rhel/etc_sysconfig_network-scripts_ifup-ovs
index e49e6fe71..8fe60fcb1 100755
--- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
+++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
@@ -170,7 +170,7 @@ case "$TYPE" in
ovs-vsctl -t ${TIMEOUT} \
-- --if-exists del-port "$OVS_BRIDGE" "$DEVICE" \
-- add-port "$OVS_BRIDGE" "$DEVICE" $OVS_OPTIONS \
-   -- set Interface "$DEVICE" type=dpdk ${OVS_EXTRA+-- 
$OVS_EXTRA}
+   -- set Interface "$DEVICE" type=dpdk 
options:dpdk-devargs="${DPDK_DEVARGS}" ${OVS_EXTRA+-- $OVS_EXTRA}
;;
OVSDPDKRPort)
ifup_ovs_bridge
@@ -188,8 +188,10 @@ case "$TYPE" in
;;
OVSDPDKBond)
ifup_ovs_bridge
+   set -- ${BOND_DPDK_DEVARGS}
for _iface in $BOND_IFACES; do
-   IFACE_TYPES="${IFACE_TYPES} -- set interface ${_iface} 
type=dpdk"
+   IFACE_TYPES="${IFACE_TYPES} -- set interface ${_iface} 
type=dpdk options:dpdk-devargs=$1"
+   shift
done
ovs-vsctl -t ${TIMEOUT} \
-- --if-exists del-port "$OVS_BRIDGE" "$DEVICE" \
-- 
2.11.0

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

[ovs-dev] [PATCH 2/3] rhel: Remove obsolete OVSDPDKVhostPort from ifdown script.

2017-01-24 Thread Daniele Di Proietto

The support for vhost cuse port has been removed long ago.

Fixes:419876444357("netdev-dpdk: Remove dpdkvhostcuse ports")
Signed-off-by: Daniele Di Proietto 
---
 rhel/etc_sysconfig_network-scripts_ifdown-ovs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rhel/etc_sysconfig_network-scripts_ifdown-ovs 
b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
index 39884016c..8c9f3694c 100755
--- a/rhel/etc_sysconfig_network-scripts_ifdown-ovs
+++ b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
@@ -59,7 +59,7 @@ case "$TYPE" in
OVSPatchPort|OVSTunnel)
ovs-vsctl -t ${TIMEOUT} -- --if-exists del-port "$OVS_BRIDGE" 
"$DEVICE"
;;
-   
OVSDPDKPort|OVSDPDKRPort|OVSDPDKVhostPort|OVSDPDKVhostUserPort|OVSDPDKBond)
+   OVSDPDKPort|OVSDPDKRPort|OVSDPDKVhostUserPort|OVSDPDKBond)
ovs-vsctl -t ${TIMEOUT} -- --if-exists del-port "$OVS_BRIDGE" 
"$DEVICE"
;;
*)
-- 
2.11.0

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

Re: [ovs-dev] [PATCH 2/2] ovs-fields: New manpage to document Open vSwitch and OpenFlow fields.

2017-01-24 Thread Justin Pettit


> On Jan 24, 2017, at 5:11 PM, Justin Pettit  wrote:
> 
> This was a lot of work.  Thanks for doing it!
> 
> --Justin

Whoops!

Acked-by: Justin Pettit 

--Justin


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

Re: [ovs-dev] [PATCH 2/2] ovs-fields: New manpage to document Open vSwitch and OpenFlow fields.

2017-01-24 Thread Justin Pettit


> On Dec 28, 2016, at 1:10 PM, Ben Pfaff  wrote:
> 
> From: Ben Pfaff 
> 
> There is still plenty of opportunity for improvement, but this new
> ovs-fields(7) manpage is much more comprehensive than ovs-ofctl(8)
> could be.
> 
> Signed-off-by: Ben Pfaff 
> ---
> Documentation/ref/index.rst |4 +
> build-aux/extract-ofp-fields|  359 +++-
> include/openvswitch/meta-flow.h |  205 +-
> lib/automake.mk |   17 +-
> lib/meta-flow.xml   | 4163 +++
> ovn/ovn-architecture.7.xml  |2 +-
> ovn/utilities/ovn-trace.8.xml   |2 +-
> python/build/nroff.py   |   62 +-
> utilities/ovs-ofctl.8.in|  993 +-
> vswitchd/vswitch.xml|6 +-
> 10 files changed, 4573 insertions(+), 1240 deletions(-)
> create mode 100644 lib/meta-flow.xml

Since the documentation formatted well, I'm going to assume extract-ofp-fields 
and nroff.py are written perfectly and without error.

> diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
> new file mode 100644
> index 000..1b646f5
> --- /dev/null
> +++ b/lib/meta-flow.xml

For everyone else's benefit, I printed out the ovs-fields man page and directly 
marked the page with feedback, which I handed to Ben.  I'm sure he'd be happy 
to scan all 57 pages for anyone interested.

> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index af1eb2b..a3a29b1 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> 
> ...
> +\fBovs\-fields\fR(7) describes the supported fields and how to match
> +them.  In addition to match fields, commands that operate on flow
> +accept a few addition key-value pairs:

s/flow/flows
s/addition/additional

> +.IP "\fBduration=\fR..."
> +.IQ "\fBn_packet=\fR..."
> +.IQ "\fBn_bytes=\fR..."
> +These ``fields'' are ignored to allow output from the \fBdump\-flows\fR
> command to be used as input for other commands that parse flows.

I actually found the previous way this was written clearer.

This was a lot of work.  Thanks for doing it!

--Justin


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

Re: [ovs-dev] [PATCH v11 2/5] ovn: avoid snat recirc only on gateway routers

2017-01-24 Thread Guru Shetty

On 21 January 2017 at 16:52, Mickey Spiegel  wrote:

> Currently, for performance reasons on gateway routers, ct_snat
> that does not specify an IP address does not immediately trigger
> recirculation.  On gateway routers, ct_snat that does not specify
> an IP address happens in the UNSNAT pipeline stage, which is
> followed by the DNAT pipeline stage that triggers recirculation
> for all packets.  This DNAT pipeline stage recirculation takes
> care of the recirculation needs of UNSNAT as well as other cases
> such as UNDNAT.
>
> On distributed routers, UNDNAT is handled in the egress pipeline
> stage, separately from DNAT in the ingress pipeline stages.  The
> DNAT pipeline stage only triggers recirculation for some packets.
> Due to this difference in design, UNSNAT needs to trigger its own
> recirculation.
>
> This patch restricts the logic that avoids recirculation for
> ct_snat, so that it only applies to datapaths representing
> gateway routers.
>
> Signed-off-by: Mickey Spiegel 
>
Acked-by: Gurucharan Shetty 

> ---
>  include/ovn/actions.h  |  3 +++
>  ovn/controller/lflow.c | 10 ++
>  ovn/lib/actions.c  | 15 +--
>  ovn/ovn-sb.xml | 23 +++
>  tests/ovn.at   |  2 +-
>  5 files changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 1d7bd69..d2510fd 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -445,6 +445,9 @@ struct ovnact_encode_params {
>  /* 'true' if the flow is for a switch. */
>  bool is_switch;
>
> +/* 'true' if the flow is for a gateway router. */
> +bool is_gateway_router;
> +
>  /* A map from a port name to its connection tracking zone. */
>  const struct simap *ct_zones;
>
> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index 2d9213b..fa00db2 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -107,6 +107,15 @@ is_switch(const struct sbrec_datapath_binding *ldp)
>
>  }
>
> +static bool
> +is_gateway_router(const struct sbrec_datapath_binding *ldp,
> +  const struct hmap *local_datapaths)
> +{
> +struct local_datapath *ld =
> +get_local_datapath(local_datapaths, ldp->tunnel_key);
> +return ld ? ld->has_local_l3gateway : false;
> +}
> +
>  /* Adds the logical flows from the Logical_Flow table to flow tables. */
>  static void
>  add_logical_flows(struct controller_ctx *ctx, const struct lport_index
> *lports,
> @@ -221,6 +230,7 @@ consider_logical_flow(const struct lport_index *lports,
>  .lookup_port = lookup_port_cb,
>  .aux = ,
>  .is_switch = is_switch(ldp),
> +.is_gateway_router = is_gateway_router(ldp, local_datapaths),
>  .ct_zones = ct_zones,
>  .group_table = group_table,
>
> diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> index 90a2add..fff838b 100644
> --- a/ovn/lib/actions.c
> +++ b/ovn/lib/actions.c
> @@ -829,12 +829,15 @@ encode_ct_nat(const struct ovnact_ct_nat *cn,
>  ct = ofpacts->header;
>  if (cn->ip) {
>  ct->flags |= NX_CT_F_COMMIT;
> -} else if (snat) {
> -/* XXX: For performance reasons, we try to prevent additional
> - * recirculations.  So far, ct_snat which is used in a gateway
> router
> - * does not need a recirculation. ct_snat(IP) does need a
> - * recirculation.  Should we consider a method to let the actions
> - * specify whether an action needs recirculation if there more use
> +} else if (snat && ep->is_gateway_router) {
> +/* For performance reasons, we try to prevent additional
> + * recirculations.  ct_snat which is used in a gateway router
> + * does not need a recirculation.  ct_snat(IP) does need a
> + * recirculation.  ct_snat in a distributed router needs
> + * recirculation regardless of whether an IP address is
> + * specified.
> + * XXX Should we consider a method to let the actions specify
> + * whether an action needs recirculation if there are more use
>   * cases?. */
>  ct->recirc_table = NX_CT_RECIRC_NONE;
>  }
> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> index f806af7..b33afd3 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -1128,11 +1128,26 @@
>  
>
>  ct_snat sends the packet through the SNAT zone to
> -unSNAT any packet that was SNATed in the opposite direction.
> If
> -the packet needs to be sent to the next tables, then it
> should be
> -followed by a next; action.  The next tables
> will not
> -see the changes in the packet caused by the connection
> tracker.
> +unSNAT any packet that was SNATed in the opposite direction.
> The
> +behavior on gateway routers differs from the behavior on a
> +distributed router:
>

Re: [ovs-dev] [PATCH v3] datapath-windows: Add a wrapper to retreive external vport

2017-01-24 Thread Sairam Venugopal

Acked-by: Sairam Venugopal 






On 1/24/17, 12:37 PM, "ovs-dev-boun...@openvswitch.org on behalf of Shashank 
Ram"  wrote:

>This wrapper is to simplify readability.
>
>Signed-off-by: Shashank Ram 
>---
> datapath-windows/ovsext/Actions.c  | 8 
> datapath-windows/ovsext/PacketIO.c | 6 +++---
> datapath-windows/ovsext/Vport.c| 6 ++
> datapath-windows/ovsext/Vport.h| 2 ++
> 4 files changed, 15 insertions(+), 7 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Actions.c 
>b/datapath-windows/ovsext/Actions.c
>index b4a286b..df1280d 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -290,8 +290,8 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx,
>  * default port.
>  */
> BOOLEAN validSrcPort =
>-(ovsFwdCtx->fwdDetail->SourcePortId ==
>- ovsFwdCtx->switchContext->virtualExternalPortId) ||
>+(OvsIsExternalVportByPortId(ovsFwdCtx->switchContext,
>+ ovsFwdCtx->fwdDetail->SourcePortId)) ||
> (ovsFwdCtx->fwdDetail->SourcePortId ==
>  NDIS_SWITCH_DEFAULT_PORT_ID);
>
>@@ -2130,8 +2130,8 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
> }
> status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS,
> vport, key, ovsFwdCtx.curNbl,
>-vport->portId ==
>-switchContext->virtualExternalPortId,
>+
>OvsIsExternalVportByPortId(switchContext,
>+vport->portId),
> ,
> ovsFwdCtx.switchContext,
> , );
>diff --git a/datapath-windows/ovsext/PacketIO.c 
>b/datapath-windows/ovsext/PacketIO.c
>index 4005589..a90b556 100644
>--- a/datapath-windows/ovsext/PacketIO.c
>+++ b/datapath-windows/ovsext/PacketIO.c
>@@ -259,13 +259,13 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext,
> curNbl,
> nextNativeForwardedNbl,
> sendCompleteFlags,
>-sourcePort == switchContext->virtualExternalPortId);
>+OvsIsExternalVportByPortId(switchContext, sourcePort));
> continue;
> }
> #endif /* NDIS_SUPPORT_NDIS640 */
>
> ctx = OvsInitExternalNBLContext(switchContext, curNbl,
>-  sourcePort == switchContext->virtualExternalPortId);
>+  OvsIsExternalVportByPortId(switchContext, sourcePort));
> if (ctx == NULL) {
> RtlInitUnicodeString(,
> L"Cannot allocate external NBL context.");
>@@ -345,7 +345,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext,
> datapath->misses++;
> status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS,
>  vport, , curNbl,
>- sourcePort == 
>switchContext->virtualExternalPortId,
>+ OvsIsExternalVportByPortId(switchContext, 
>sourcePort),
>  , switchContext, , );
> if (status == NDIS_STATUS_SUCCESS) {
> /* Complete the packet since it was copied to user
>diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
>index e9e22aa..9142937 100644
>--- a/datapath-windows/ovsext/Vport.c
>+++ b/datapath-windows/ovsext/Vport.c
>@@ -909,6 +909,12 @@ OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT 
>switchContext,
> }
> }
>
>+BOOLEAN OvsIsExternalVportByPortId(POVS_SWITCH_CONTEXT switchContext,
>+   NDIS_SWITCH_PORT_ID portId)
>+{
>+return (portId == switchContext->virtualExternalPortId);
>+}
>+
> POVS_VPORT_ENTRY
> OvsAllocateVport(VOID)
> {
>diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h
>index 4dc4e00..7d88f86 100644
>--- a/datapath-windows/ovsext/Vport.h
>+++ b/datapath-windows/ovsext/Vport.h
>@@ -134,6 +134,8 @@ POVS_VPORT_ENTRY OvsFindVportByHvNameW(POVS_SWITCH_CONTEXT 
>switchContext,
> POVS_VPORT_ENTRY OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT 
> switchContext,
>  NDIS_SWITCH_PORT_ID portId,
>  NDIS_SWITCH_NIC_INDEX index);
>+BOOLEAN OvsIsExternalVportByPortId(POVS_SWITCH_CONTEXT switchContext,
>+   NDIS_SWITCH_PORT_ID portId);
> POVS_VPORT_ENTRY OvsFindTunnelVportByDstPortAndType(POVS_SWITCH_CONTEXT 
> switchContext,
> UINT16 dstPort,
> OVS_VPORT_TYPE 
> ovsPortType);
>--
>2.6.2
>
>___

Re: [ovs-dev] OVS - ODL Sync on OF Bundle in 1.3

2017-01-24 Thread Jarno Rajahalme


> On Jan 24, 2017, at 11:30 AM, Abhijit Kumbhare 
>  wrote:
> 
> Thanks Jan! Good info. So looks like we would need to support the messages on 
> both the OvS defined message numbers/error codes as well as the ONF defined 
> ones.

OVS uses only ONF defined message numbers and error codes for the bundle 
feature. They just happen to be different between 1.3 (extension) and 1.4+ 
(“native”).

  Jarno

> 
> From: Jan Scheurich  >
> Date: Monday, January 23, 2017 at 9:01 AM
> To: Abhijit Kumbhare  >, Zoltán Balogh 
> >, László Sürü 
> >, Miklós Pelyva 
> >, "Jozef 
> Bacigal -X (jbacigal - PANTHEON TECHNOLOGIES at Cisco)" 
> >, Tomáš 
> Slušný >, 
> Prasanna Huddar  >, Shuva Jyoti Kar 
> >, Sharath 
> Kumar V >, 
> Kanagasundaram K  >, Sunil Kumar G 
> >, D 
> Arunprakash >, 
> Muthukumaran K  >, "Jarno Rajahalme (ja...@ovn.org 
> )" >, 
> "d...@openvswitch.org "  >
> Subject: RE: OVS - ODL Sync on OF Bundle in 1.3 
> 
> Hi,
>  
> Here are the main differences I have found between the ONF Bundle extension 
> (EXT-230) to OF 1.3 and the OF Bundle specified in OF 1.5.1:
>  
> No OFPC_BUNDLES bit in ofp_capabilities to indicate switch support for 
> bundles.
> Bundle Features MP message not supported
> No support for scheduled bundles
> ONF Experimenter messages 2300 (ONF_ET_BUNDLE_CONTROL) and 2301 
> (ONF_ET_BUNDLE_ADD_MESSAGE) instead of standard OFP messages 33 
> (OFP_BUNDLE_CONTROL) and 34 (OFP_ BUNDLE_ADD_MESSAGE). 
> Other than that the message syntax and semantics seem identical
> ONF experimenter error codes for bundles errors in the range 2300-2315 
> instead of standard error codes 0-15 for error type OFPET_BUNDLE_FAILED. 
> Otherwise same meaning.
>  
> Inside OVS it seems that standard OF 1.5 and OF 1.3 extension messages are 
> internally mapped to a common message type. Errors are mapped back to 
> standard OFP error (type, code) for OF 1.5 and experimenter error type in OF 
> 1.3.
>  
> So, the OVS bundle implementation for OF 1.3 should be faithful to EXT-230.
>  
> BR, Jan
>  
> -Original Appointment-
> From: Jan Scheurich 
> Sent: Wednesday, 18 January, 2017 16:23
> To: Jan Scheurich; Abhijit Kumbhare; Zoltán Balogh; László Sürü; Miklós 
> Pelyva; Jozef Bacigál; Tomáš Slušný; Prasanna Huddar; Shuva Jyoti Kar; 
> Sharath Kumar V; Kanagasundaram K; Sunil Kumar G; D Arunprakash; Muthukumaran 
> K; Jarno Rajahalme (ja...@ovn.org ); 
> d...@openvswitch.org 
> Subject: OVS - ODL Sync on OF Bundle in 1.3 
> When: Monday, 23 January, 2017 17:00-18:00 (UTC+01:00) Amsterdam, Berlin, 
> Bern, Rome, Stockholm, Vienna.
> Where: Skype Meeting
>  
>  
>  <>Hi Jarno,
>  
> OpenDaylight folks are finally starting to implement support of OpenFlow 
> bundles as a basis for the bundle-based hitless recync procedure we discussed 
> earlier. As ODL does not yet have protocol support for OpenFlow versions 1.4 
> or 1.5, they intend to implement the bundle extension to OF 1.3 as specified 
> in EXT-230 in
> https://www.opennetworking.org/images/stories/downloads/sdn-resources/onf-specifications/openflow/openflow-extensions-1.3.x-pack2.zip
>  
> 
>  
> Would you have time for a short meeting on early Monday to discuss and 
> confirm whether the OVS implementation of bundles in OF 1.3 is compliant with 
> EXT-230 and has everything they need?
>  
> Thanks, Jan
>  
>  
> .
>  <>à Join Skype Meeting    
> <><>
> This is an online meeting for Skype for Business, the professional meetings 
> and communications app formerly known as Lync.
> Join by phone
>  
> +492115343925

Re: [ovs-dev] Sync on PTAP, EXT-382 and NSH

2017-01-24 Thread Jarno Rajahalme

I also missed the meeting, did not have it my calendar, sorry.

  Jarno

> On Jan 24, 2017, at 5:52 AM, Jiri Benc  wrote:
> 
> On Wed, 18 Jan 2017 09:53:18 +, Jan Scheurich wrote:
>> Please be invited to the next sync meeting.
> 
> Sorry, won't make the meeting today. Too busy with DevConf.cz
> preparations.
> 
>> Actions Points:
>> AP-1 (Jarno): Coordinate review of Yi's backported net-next patches
>> AP-2 (Jiri) Check the ability of the kernel datapath to match on the
>> presence of Ethernet header w/o matching on Ethernet addresses.
>> Formating of such DP flows?
> 
> The OVS_KEY_ATTR_ETHERNET netlink attribute contains source and
> destination MAC address. It can be masked.
> 
> If the OVS_KEY_ATTR_ETHERNET attribute is present, only Ethernet frames
> will match. The MAC addresses may be masked (wildcarded). The
> OVS_KEY_ATTR_ETHERTYPE attribute indicates the ethertype to match and
> cannot be masked (it's always an exact match). If it's not present,
> ETH_P_802_2 is assumed.
> 
> If the OVS_KEY_ATTR_ETHERNET in not present, the OVS_KEY_ATTR_ETHERTYPE
> attribute is mandatory and indicates the ethertype to match. Again, it
> cannot be masked.
> 
> To answer the question, it's currently possible to match on Ethernet
> header without matching on Ethernet addresses but not without matching
> on ethertype. I think it can be relaxed with some work if needed.
> 
>> AP-3 (Jiri) Provide an update on the status and ETA of the RTNETLINK API 
>> patches
> 
> RFC patchset posted last week:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327749.html
> 
> Jiri

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

Re: [ovs-dev] [PATCH 3/3] ovs-router: introduce pkt-mark.

2017-01-24 Thread Pravin Shelar

On Mon, Jan 23, 2017 at 9:04 AM, Jarno Rajahalme  wrote:
>
>> On Dec 28, 2016, at 1:44 AM, Pravin B Shelar  wrote:
>>
>> OVS router is basically partial copy of linux kernel FIB.
>> kernel routing table use skb-mark along with usual routing
>
> “use”->”uses”
>
> Could you elaborate a bit here, is the kernel doing a simple exact match of 
> the whole mark? Is the value 0 special in any way?
>
no, zero is not special value. kernel always exact match the skb mark.

>> parameters. Following patch brings in support for skb-mark
>> to ovs-router so that we can lookup route according to
>> flow skb-mark.
>>
>
> So by default we use the flow’s skb_mark, but if the ‘egress_pkt_mark’ is 
> set, then we use that?
>
not really.
ovs-router and kernel router would use flow-mark. This makes the route
lookup consistent with kernel routing lookup.

>> Signed-off-by: Pravin B Shelar 
>> ---
>> lib/netdev-vport.c   |   4 +-
>> lib/ovs-router.c | 192 
>> ++-
>> lib/ovs-router.h |   6 +-
>> lib/route-table.c|   2 +-
>> ofproto/ofproto-dpif-sflow.c |   2 +-
>> ofproto/ofproto-dpif-xlate.c |   2 +-
>> tests/ovs-router.at  |  22 +
>> tests/tunnel-push-pop.at |   3 +
>> 8 files changed, 188 insertions(+), 45 deletions(-)
>>
>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>> index 25dd0e8..54db559 100644
>> --- a/lib/netdev-vport.c
>> +++ b/lib/netdev-vport.c
>> @@ -259,10 +259,12 @@ tunnel_check_status_change__(struct netdev_vport 
>> *netdev)
>> bool status = false;
>> struct in6_addr *route;
>> struct in6_addr gw;
>> +uint32_t mark;
>>
>> iface[0] = '\0';
>> route = >tnl_cfg.ipv6_dst;
>> -if (ovs_router_lookup(route, iface, NULL, )) {
>> +mark = netdev->tnl_cfg.egress_pkt_mark;
>> +if (ovs_router_lookup(mark, route, iface, NULL, )) {
>> struct netdev *egress_netdev;
>>
>> if (!netdev_open(iface, NULL, _netdev)) {
>> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
>> index 935b60a..7300b36 100644
>> --- a/lib/ovs-router.c
>> +++ b/lib/ovs-router.c
>> @@ -45,6 +45,11 @@
>> #include "unaligned.h"
>> #include "unixctl.h"
>> #include "util.h"
>> +#include "openvswitch/vlog.h"
>> +
>> +VLOG_DEFINE_THIS_MODULE(ovs_route);
>> +
>
> This should be ‘ovs_router’ instead.
>
ok.

>> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
>>
>> static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
>> static struct classifier cls;
>> @@ -57,6 +62,7 @@ struct ovs_router_entry {
>> struct in6_addr src_addr;
>> uint8_t plen;
>> uint8_t priority;
>> +uint32_t mark;
>> };
>>
>> static struct ovs_router_entry *
>> @@ -88,11 +94,12 @@ ovs_router_lookup_fallback(const struct in6_addr 
>> *ip6_dst, char output_bridge[],
>> }
>>
>> bool
>> -ovs_router_lookup(const struct in6_addr *ip6_dst, char output_bridge[],
>> +ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst,
>> +  char output_bridge[],
>>   struct in6_addr *src, struct in6_addr *gw)
>> {
>> const struct cls_rule *cr;
>> -struct flow flow = {.ipv6_dst = *ip6_dst};
>> +struct flow flow = {.ipv6_dst = *ip6_dst, .pkt_mark = mark};
>>
>> cr = classifier_lookup(, OVS_VERSION_MAX, , NULL);
>> if (cr) {
>> @@ -115,7 +122,8 @@ rt_entry_free(struct ovs_router_entry *p)
>> free(p);
>> }
>>
>> -static void rt_init_match(struct match *match, const struct in6_addr 
>> *ip6_dst,
>> +static void rt_init_match(struct match *match, uint32_t mark,
>> +  const struct in6_addr *ip6_dst,
>>   uint8_t plen)
>> {
>> struct in6_addr dst;
>> @@ -127,6 +135,8 @@ static void rt_init_match(struct match *match, const 
>> struct in6_addr *ip6_dst,
>> memset(match, 0, sizeof *match);
>> match->flow.ipv6_dst = dst;
>> match->wc.masks.ipv6_dst = mask;
>> +match->wc.masks.pkt_mark = UINT32_MAX;
>> +match->flow.pkt_mark = mark;
>
> So by default, when the route entry does not specify a mark, an exact match 
> on zero is made?
>
Yes, In kernel skb-mark is always used for route lookup. So I would
like to do same here.

>> }
>>
>> static int
>> @@ -178,7 +188,8 @@ out:
>> }
>>
>> static int
>> -ovs_router_insert__(uint8_t priority, const struct in6_addr *ip6_dst,
>> +ovs_router_insert__(uint32_t mark, uint8_t priority,
>> +const struct in6_addr *ip6_dst,
>> uint8_t plen, const char output_bridge[],
>> const struct in6_addr *gw)
>> {
>> @@ -187,13 +198,14 @@ ovs_router_insert__(uint8_t priority, const struct 
>> in6_addr *ip6_dst,
>> struct match match;
>> int err;
>>
>> -rt_init_match(, ip6_dst, plen);
>> +rt_init_match(, mark, ip6_dst, plen);
>>
>> p = xzalloc(sizeof *p);
>> ovs_strlcpy(p->output_bridge, output_bridge, sizeof p->output_bridge);
>> if (ipv6_addr_is_set(gw)) {

Re: [ovs-dev] [PATCH v2 1/2] tunneling: Avoid recirculation on datapath by computing the recirculate actions at translate time.

2017-01-24 Thread Jarno Rajahalme

The userspace CLONE action is now merged to OVS master, so you can rebase the 
patches to use that.

  Jarno

> On Jan 24, 2017, at 3:49 AM, Zoltán Balogh  wrote:
> 
> Hi,
> 
> All right, we will rebase the code once the patch is merged.
> 
> Could you have a look at the code and review it, please?
> So we could receive feedback in time. 
> 
> Furthermore it would be nice to get some help regarding the failing test 
> cases.
> 
> 768: tunnel_push_pop - packet_outFAILED 
> (tunnel-push-pop.at:203)
> 2261: ovn -- 3 HVs, 1 LS, 3 lports/HV FAILED (ovn.at:1295)
> 2266: ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR   FAILED (ovn.at:2468)
> 2268: ovn -- 2 HVs, 2 LS, 1 lport/LS, 2 peer LRs  FAILED (ovn.at:2979)
> 2298: ovn -- 1 LR with distributed router gateway port FAILED (ovn.at:6480)
> 
> The patches are available here:
> https://patchwork.ozlabs.org/patch/717683/
> https://patchwork.ozlabs.org/patch/717681/
> 
> 
> Best regards,
> Zoltan
> 
> 
> 
>> On Jan 22, 2017, at 5:50 AM, Jan Scheurich  wrote:
>> =20
>> How does the dpif-netdev CLONE action introduced here relate to the simil=
> ar action already introduced in the context of the OpenFLow CLONE action (s=
> ee for example https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/=
> 327808.html).
>> =20
> 
> OpenFlow level clone action was added a bit earlier, the email you refer to=
> is a dpif-netdev level clone action.
> 
>> I assume these do comparable things and the native tunnel implementation =
> should re-use the new CLONE action.
>> =20
> 
> This is true. There were other needs for the datapath level clone action, a=
> nd once that is merged this patch series should rebase to that.
> 
>  Jarno
> 
>> /Jan
>> =20
>> On 2017-01-20 14:40, Zolt=E1n Balogh wrote:
>>> From: Sugesh Chandran 
>>> =20
>>> Openvswitch datapath recirculates packets for tunneling, i.e.
>>> the incoming packets are encapsulated at first pass. Further actions are
>>> applied on encapsulated packets on the second pass after recirculating.
>>> The proposed patch compute and append the post tunnel actions at the tim=
> e of
>>> translation itself instead of recirculating at datapath. These actions a=
> re solely
>>> depends on tunnel attributes so there is no need of datapath recirculati=
> on.
>>> By avoiding the recirculation at datapath, the patch offers upto 30%
>>> performance improvement for VxLAN tunneling in our testing.
>>> The action execution logic is also extended with new CLONE action to def=
> ine
>>> the packet cloning when the actions are combined. The lenght in the CLON=
> E
>>> action specifies the size of nested action set.
>>> =20
>>> Signed-off-by: Sugesh Chandran 
>>> Signed-off-by: Zolt=E1n Balogh 
>>> Co-authored-by: Zolt=E1n Balogh 
>>> ---
>>> datapath/linux/compat/include/linux/openvswitch.h |   1 +
>>> lib/dpif-netdev.c |  20 +-
>>> lib/dpif.c|   1 +
>>> lib/odp-execute.c |  57 -
>>> lib/odp-util.c|  96 +++-
>>> lib/odp-util.h|   5 +
>>> ofproto/ofproto-dpif-sflow.c  |   1 +
>>> ofproto/ofproto-dpif-xlate.c  | 267 +++=
> ---
>>> 8 files changed, 288 insertions(+), 160 deletions(-)
>>> =20
>>> diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapat=
> h/linux/compat/include/linux/openvswitch.h
>>> index 12260d8..91849d6 100644
>>> --- a/datapath/linux/compat/include/linux/openvswitch.h
>>> +++ b/datapath/linux/compat/include/linux/openvswitch.h
>>> @@ -818,6 +818,7 @@ enum ovs_action_attr {
>>> #ifndef __KERNEL__
>>> =09OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
>>> =09OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
>>> +=09OVS_ACTION_ATTR_CLONE,
>>> #endif
>>> =09__OVS_ACTION_ATTR_MAX,=09  /* Nothing past this will be accepted
>>> =09=09=09=09   * from userspace. */
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>> index 3901129..0b85fe4 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -4547,24 +4547,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch =
> *packets_,
>>>   case OVS_ACTION_ATTR_TUNNEL_PUSH:
>>> if (*depth < MAX_RECIRC_DEPTH) {
>>> -struct dp_packet_batch tnl_pkt;
>>> -struct dp_packet_batch *orig_packets_ =3D packets_;
>>> -int err;
>>> -
>>> -if (!may_steal) {
>>> -dp_packet_batch_clone(_pkt, packets_);
>>> -packets_ =3D _pkt;
>>> -dp_packet_batch_reset_cutlen(orig_packets_);
>>> -}
>>> -
>>> dp_packet_batch_apply_cutlen(packets_);
>>> -
>>> -err =3D push_tnl_action(pmd, a,

[ovs-dev] [PATCH v3] datapath-windows: Add a wrapper to retreive external vport

2017-01-24 Thread Shashank Ram

This wrapper is to simplify readability.

Signed-off-by: Shashank Ram 
---
 datapath-windows/ovsext/Actions.c  | 8 
 datapath-windows/ovsext/PacketIO.c | 6 +++---
 datapath-windows/ovsext/Vport.c| 6 ++
 datapath-windows/ovsext/Vport.h| 2 ++
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index b4a286b..df1280d 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -290,8 +290,8 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx,
  * default port.
  */
 BOOLEAN validSrcPort =
-(ovsFwdCtx->fwdDetail->SourcePortId ==
- ovsFwdCtx->switchContext->virtualExternalPortId) ||
+(OvsIsExternalVportByPortId(ovsFwdCtx->switchContext,
+ ovsFwdCtx->fwdDetail->SourcePortId)) ||
 (ovsFwdCtx->fwdDetail->SourcePortId ==
  NDIS_SWITCH_DEFAULT_PORT_ID);

@@ -2130,8 +2130,8 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
 }
 status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS,
 vport, key, ovsFwdCtx.curNbl,
-vport->portId ==
-switchContext->virtualExternalPortId,
+
OvsIsExternalVportByPortId(switchContext,
+vport->portId),
 ,
 ovsFwdCtx.switchContext,
 , );
diff --git a/datapath-windows/ovsext/PacketIO.c 
b/datapath-windows/ovsext/PacketIO.c
index 4005589..a90b556 100644
--- a/datapath-windows/ovsext/PacketIO.c
+++ b/datapath-windows/ovsext/PacketIO.c
@@ -259,13 +259,13 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext,
 curNbl,
 nextNativeForwardedNbl,
 sendCompleteFlags,
-sourcePort == switchContext->virtualExternalPortId);
+OvsIsExternalVportByPortId(switchContext, sourcePort));
 continue;
 }
 #endif /* NDIS_SUPPORT_NDIS640 */

 ctx = OvsInitExternalNBLContext(switchContext, curNbl,
-  sourcePort == switchContext->virtualExternalPortId);
+  OvsIsExternalVportByPortId(switchContext, sourcePort));
 if (ctx == NULL) {
 RtlInitUnicodeString(,
 L"Cannot allocate external NBL context.");
@@ -345,7 +345,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext,
 datapath->misses++;
 status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS,
  vport, , curNbl,
- sourcePort == 
switchContext->virtualExternalPortId,
+ OvsIsExternalVportByPortId(switchContext, 
sourcePort),
  , switchContext, , );
 if (status == NDIS_STATUS_SUCCESS) {
 /* Complete the packet since it was copied to user
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index e9e22aa..9142937 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -909,6 +909,12 @@ OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT 
switchContext,
 }
 }

+BOOLEAN OvsIsExternalVportByPortId(POVS_SWITCH_CONTEXT switchContext,
+   NDIS_SWITCH_PORT_ID portId)
+{
+return (portId == switchContext->virtualExternalPortId);
+}
+
 POVS_VPORT_ENTRY
 OvsAllocateVport(VOID)
 {
diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h
index 4dc4e00..7d88f86 100644
--- a/datapath-windows/ovsext/Vport.h
+++ b/datapath-windows/ovsext/Vport.h
@@ -134,6 +134,8 @@ POVS_VPORT_ENTRY OvsFindVportByHvNameW(POVS_SWITCH_CONTEXT 
switchContext,
 POVS_VPORT_ENTRY OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT 
switchContext,
  NDIS_SWITCH_PORT_ID portId,
  NDIS_SWITCH_NIC_INDEX index);
+BOOLEAN OvsIsExternalVportByPortId(POVS_SWITCH_CONTEXT switchContext,
+   NDIS_SWITCH_PORT_ID portId);
 POVS_VPORT_ENTRY OvsFindTunnelVportByDstPortAndType(POVS_SWITCH_CONTEXT 
switchContext,
 UINT16 dstPort,
 OVS_VPORT_TYPE 
ovsPortType);
--
2.6.2

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

Re: [ovs-dev] [PATCH v2] datapath-windows: Add a wrapper to retreive external vport

2017-01-24 Thread Sairam Venugopal

Hi Shashank,

Please find my comments inline.

Thanks,
Sairam



On 1/24/17, 12:25 PM, "ovs-dev-boun...@openvswitch.org on behalf of Shashank 
Ram"  wrote:

>This wrapper is to simplify readability.
>
>Signed-off-by: Shashank Ram 
>---
> datapath-windows/ovsext/Actions.c  | 8 
> datapath-windows/ovsext/PacketIO.c | 6 +++---
> datapath-windows/ovsext/Vport.c| 6 ++
> datapath-windows/ovsext/Vport.h| 2 ++
> 4 files changed, 15 insertions(+), 7 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Actions.c 
>b/datapath-windows/ovsext/Actions.c
>index b4a286b..2331c15 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -290,8 +290,8 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx,
>  * default port.
>  */
> BOOLEAN validSrcPort =
>-(ovsFwdCtx->fwdDetail->SourcePortId ==
>- ovsFwdCtx->switchContext->virtualExternalPortId) ||

Sai: You are comparing ovsFwdCtx->switchContext->virtualExternalPortId with 
itself. It should be
ovsFwdCtx->fwdDetail->SourcePortId


>+(OvsIsExternalVportByPortId(ovsFwdCtx->switchContext,
>+ ovsFwdCtx->switchContext->virtualExternalPortId)) ||
> (ovsFwdCtx->fwdDetail->SourcePortId ==
>  NDIS_SWITCH_DEFAULT_PORT_ID);
>
>@@ -2130,8 +2130,8 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
> }
> status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS,
> vport, key, ovsFwdCtx.curNbl,
>-vport->portId ==
>-switchContext->virtualExternalPortId,
>+
>OvsIsExternalVportByPortId(switchContext,
>+vport->portId),
> ,
> ovsFwdCtx.switchContext,
> , );
>diff --git a/datapath-windows/ovsext/PacketIO.c 
>b/datapath-windows/ovsext/PacketIO.c
>index 4005589..a90b556 100644
>--- a/datapath-windows/ovsext/PacketIO.c
>+++ b/datapath-windows/ovsext/PacketIO.c
>@@ -259,13 +259,13 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext,
> curNbl,
> nextNativeForwardedNbl,
> sendCompleteFlags,
>-sourcePort == switchContext->virtualExternalPortId);
>+OvsIsExternalVportByPortId(switchContext, sourcePort));
> continue;
> }
> #endif /* NDIS_SUPPORT_NDIS640 */
>
> ctx = OvsInitExternalNBLContext(switchContext, curNbl,
>-  sourcePort == switchContext->virtualExternalPortId);
>+  OvsIsExternalVportByPortId(switchContext, sourcePort));
> if (ctx == NULL) {
> RtlInitUnicodeString(,
> L"Cannot allocate external NBL context.");
>@@ -345,7 +345,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext,
> datapath->misses++;
> status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS,
>  vport, , curNbl,
>- sourcePort == 
>switchContext->virtualExternalPortId,
>+ OvsIsExternalVportByPortId(switchContext, 
>sourcePort),
>  , switchContext, , );
> if (status == NDIS_STATUS_SUCCESS) {
> /* Complete the packet since it was copied to user
>diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
>index e9e22aa..9142937 100644
>--- a/datapath-windows/ovsext/Vport.c
>+++ b/datapath-windows/ovsext/Vport.c
>@@ -909,6 +909,12 @@ OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT 
>switchContext,
> }
> }
>
>+BOOLEAN OvsIsExternalVportByPortId(POVS_SWITCH_CONTEXT switchContext,
>+   NDIS_SWITCH_PORT_ID portId)
>+{
>+return (portId == switchContext->virtualExternalPortId);
>+}
>+
> POVS_VPORT_ENTRY
> OvsAllocateVport(VOID)
> {
>diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h
>index 4dc4e00..7d88f86 100644
>--- a/datapath-windows/ovsext/Vport.h
>+++ b/datapath-windows/ovsext/Vport.h
>@@ -134,6 +134,8 @@ POVS_VPORT_ENTRY OvsFindVportByHvNameW(POVS_SWITCH_CONTEXT 
>switchContext,
> POVS_VPORT_ENTRY OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT 
> switchContext,
>  NDIS_SWITCH_PORT_ID portId,
>  NDIS_SWITCH_NIC_INDEX index);
>+BOOLEAN OvsIsExternalVportByPortId(POVS_SWITCH_CONTEXT switchContext,
>+   NDIS_SWITCH_PORT_ID portId);
> POVS_VPORT_ENTRY OvsFindTunnelVportByDstPortAndType(POVS_SWITCH_CONTEXT 
> switchContext,
> UINT16

[ovs-dev] [PATCH v2] datapath-windows: Add a wrapper to retreive external vport

2017-01-24 Thread Shashank Ram

This wrapper is to simplify readability.

Signed-off-by: Shashank Ram 
---
 datapath-windows/ovsext/Actions.c  | 8 
 datapath-windows/ovsext/PacketIO.c | 6 +++---
 datapath-windows/ovsext/Vport.c| 6 ++
 datapath-windows/ovsext/Vport.h| 2 ++
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index b4a286b..2331c15 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -290,8 +290,8 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx,
  * default port.
  */
 BOOLEAN validSrcPort =
-(ovsFwdCtx->fwdDetail->SourcePortId ==
- ovsFwdCtx->switchContext->virtualExternalPortId) ||
+(OvsIsExternalVportByPortId(ovsFwdCtx->switchContext,
+ ovsFwdCtx->switchContext->virtualExternalPortId)) ||
 (ovsFwdCtx->fwdDetail->SourcePortId ==
  NDIS_SWITCH_DEFAULT_PORT_ID);

@@ -2130,8 +2130,8 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
 }
 status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS,
 vport, key, ovsFwdCtx.curNbl,
-vport->portId ==
-switchContext->virtualExternalPortId,
+
OvsIsExternalVportByPortId(switchContext,
+vport->portId),
 ,
 ovsFwdCtx.switchContext,
 , );
diff --git a/datapath-windows/ovsext/PacketIO.c 
b/datapath-windows/ovsext/PacketIO.c
index 4005589..a90b556 100644
--- a/datapath-windows/ovsext/PacketIO.c
+++ b/datapath-windows/ovsext/PacketIO.c
@@ -259,13 +259,13 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext,
 curNbl,
 nextNativeForwardedNbl,
 sendCompleteFlags,
-sourcePort == switchContext->virtualExternalPortId);
+OvsIsExternalVportByPortId(switchContext, sourcePort));
 continue;
 }
 #endif /* NDIS_SUPPORT_NDIS640 */

 ctx = OvsInitExternalNBLContext(switchContext, curNbl,
-  sourcePort == switchContext->virtualExternalPortId);
+  OvsIsExternalVportByPortId(switchContext, sourcePort));
 if (ctx == NULL) {
 RtlInitUnicodeString(,
 L"Cannot allocate external NBL context.");
@@ -345,7 +345,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext,
 datapath->misses++;
 status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS,
  vport, , curNbl,
- sourcePort == 
switchContext->virtualExternalPortId,
+ OvsIsExternalVportByPortId(switchContext, 
sourcePort),
  , switchContext, , );
 if (status == NDIS_STATUS_SUCCESS) {
 /* Complete the packet since it was copied to user
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index e9e22aa..9142937 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -909,6 +909,12 @@ OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT 
switchContext,
 }
 }

+BOOLEAN OvsIsExternalVportByPortId(POVS_SWITCH_CONTEXT switchContext,
+   NDIS_SWITCH_PORT_ID portId)
+{
+return (portId == switchContext->virtualExternalPortId);
+}
+
 POVS_VPORT_ENTRY
 OvsAllocateVport(VOID)
 {
diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h
index 4dc4e00..7d88f86 100644
--- a/datapath-windows/ovsext/Vport.h
+++ b/datapath-windows/ovsext/Vport.h
@@ -134,6 +134,8 @@ POVS_VPORT_ENTRY OvsFindVportByHvNameW(POVS_SWITCH_CONTEXT 
switchContext,
 POVS_VPORT_ENTRY OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT 
switchContext,
  NDIS_SWITCH_PORT_ID portId,
  NDIS_SWITCH_NIC_INDEX index);
+BOOLEAN OvsIsExternalVportByPortId(POVS_SWITCH_CONTEXT switchContext,
+   NDIS_SWITCH_PORT_ID portId);
 POVS_VPORT_ENTRY OvsFindTunnelVportByDstPortAndType(POVS_SWITCH_CONTEXT 
switchContext,
 UINT16 dstPort,
 OVS_VPORT_TYPE 
ovsPortType);
--
2.6.2

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

Re: [ovs-dev] [PATCH 1/3] tunnel: Add support to configure ptk_mark

2017-01-24 Thread Pravin Shelar

On Mon, Jan 23, 2017 at 9:04 AM, Jarno Rajahalme  wrote:
> Pravin,
>
> Sorry for the delay, I hope you find time to respond.
>
>> On Dec 28, 2016, at 1:44 AM, Pravin B Shelar  wrote:
>>
>> Today packet mark action is broaken for Tunnel ports with
>> tunnel monitoring.
>
> “broaken”->”broken”
>
ok.

> Could you be more specific here. Setting packet mark works, but apparently 
> not when sending the packet to a tunnel, or possibly when sending packet to a 
> tunnel when tunnel monitoring (you mean BFD?). What exactly is the problem?
>
>> User can write a flow to set pkt-mark for
>> any tunnel packet generated by vswitchd itself, like BFD packets.
>>
>
> So the problem is that if a flow sets the pkt_mark, it will be also used for 
> BFD packets? Or is it that it will not be used for BFD packets? What is the 
> desired behavior?
>
Problem is for tunnels with tunnel monitoring. Using OVS flow table we
can set pkt-mark for tunnel packets in datapath, but not for tunnel
monitoring packets which are originated from vswitchd.

>> Following patch introduces new option in OVSDB tunnel
>> configuration so that user can set skb-mark for given
>> tunnel endpoint. OVS would set the mark according to the
>> skb-mark option for all tunnel traffic including packets
>> generated by vSwitchd like tunnel monitoring BFD packet.
>>
>
> What happens to the tunnel packet mark when the new option is not specified? 
> Is that behavior changed?
>
Nope, it remains the same. As mentioned in the man page it is optional
parameter.

>> Signed-off-by: Pravin B Shelar 
>> ---
>> lib/netdev-vport.c   |  8 
>> lib/netdev.h |  1 +
>> ofproto/tunnel.c |  5 +
>> tests/tunnel-push-pop.at | 16 
>> vswitchd/vswitch.xml |  4 
>> 5 files changed, 34 insertions(+)
>>
>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>> index 02a246a..25dd0e8 100644
>> --- a/lib/netdev-vport.c
>> +++ b/lib/netdev-vport.c
>> @@ -505,6 +505,10 @@ set_tunnel_config(struct netdev *dev_, const struct 
>> smap *args)
>> }
>>
>> free(str);
>> +} else if (!strcmp(node->key, "egress_pkt_mark")) {
>> +char *ptr = NULL;
>> +
>> +tnl_cfg.egress_pkt_mark = strtoul(node->value, , 10);
>
> Could skip the “ptr” and use NULL instead.
>
ok.

> Should document somewhere what happens when the option is not given, which 
> seems to be the same as setting the egress_pkt_mark to zero.
>
>> } else {
>> VLOG_WARN("%s: unknown %s argument '%s'", name, type, node->key);
>> }
>> @@ -623,6 +627,10 @@ get_tunnel_config(const struct netdev *dev, struct smap 
>> *args)
>> smap_add(args, "df_default", "false");
>> }
>>
>> +if (tnl_cfg.egress_pkt_mark) {
>> +smap_add_format(args, "egress_pkt_mark",
>> +"%"PRIu32, tnl_cfg.egress_pkt_mark);
>> +}
>
>
> Does a zero mark mean that the mark is not used? If the zero value is still 
> used is leaving it out misleading?
>
This is good point. If user explicitly sets the mark then we need to
reset it, I will update the patch.

>> return 0;
>> }
>>
>> diff --git a/lib/netdev.h b/lib/netdev.h
>> index a667fe3..5ec1274 100644
>> --- a/lib/netdev.h
>> +++ b/lib/netdev.h
>> @@ -89,6 +89,7 @@ struct netdev_tunnel_config {
>> struct in6_addr ipv6_dst;
>>
>> uint32_t exts;
>> +uint32_t egress_pkt_mark;
>>
>> uint8_t ttl;
>> bool ttl_inherit;
>> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
>> index ce727f4..6205948 100644
>> --- a/ofproto/tunnel.c
>> +++ b/ofproto/tunnel.c
>> @@ -461,6 +461,11 @@ tnl_port_send(const struct ofport_dpif *ofport, struct 
>> flow *flow,
>> | (cfg->csum ? FLOW_TNL_F_CSUM : 0)
>> | (cfg->out_key_present ? FLOW_TNL_F_KEY : 0);
>>
>> +if (cfg->egress_pkt_mark) {
>> +flow->pkt_mark = cfg->egress_pkt_mark;
>> +wc->masks.pkt_mark = UINT32_MAX;
>
> Here the pkt_mark from the flow is not used or inherited, so why are we 
> setting the mask (which causes an exact match on the incoming pkt_mark value 
> (which is likely zero))?
>
The problem is in commit odp action. If I do not set the mask then the
odp action uses zero mask and key to generate skb-mark set action.
Result is pkt-mark is always zero in set pkt-mark action.


>> +}
>> +
>
> If egress_pkt_mark is zero, do we further on in the tunnel code (maybe due to 
> later patches) use the pkt_mask value from the flow?
>
If egress_pkt_mark is set then it would overwrite the flow packet mark.

>> if (pre_flow_str) {
>> char *post_flow_str = flow_to_string(flow);
>> char *tnl_str = tnl_port_fmt(tnl_port);
>> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
>> index 700ef55..4aaa669 100644
>> --- a/tests/tunnel-push-pop.at
>> +++ b/tests/tunnel-push-pop.at
>> @@ -12,6 +12,8 @@ AT_CHECK([ovs-vsctl add-port int-br t2 -- set Interface t2 
>> type=vxlan \
>>

Re: [ovs-dev] [PATCH] ovn-northd: Add flows in DHCP_OPTIONS pipeline to support renew requests

2017-01-24 Thread Russell Bryant

On Tue, Jan 24, 2017 at 1:58 PM, Numan Siddique  wrote:

> ovn-northd adds the flows to send the DHCPv4 packets to ovn-controller
> only with the match ip4.src = 0.0.0.0 and ip4.dst = 255.255.255.255.
>
> When a DHCPv4 lease is about to expire, before sending a DHCPDISCOVER
> packet, the client can send a DHCPREQUEST packet to renew its ip
> with ip4.src set to its offered ip and ip4.dst set to the DHCP server
> ip or broadcast ip.
>
> This patch supports this missing scenario by adding the necessary
> flows in DHCP_OPTIONS ingress pipeline.
>
> Signed-off-by: Numan Siddique 


Thanks for working on this, Numan!

I see some DHCP test cases in tests/ovn.at.  Could you look at extending
those to cover this case as well?

Thanks,

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

Re: [ovs-dev] OVS - ODL Sync on OF Bundle in 1.3

2017-01-24 Thread Abhijit Kumbhare

Thanks Jan! Good info. So looks like we would need to support the messages on 
both the OvS defined message numbers/error codes as well as the ONF defined 
ones.

From: Jan Scheurich 
>
Date: Monday, January 23, 2017 at 9:01 AM
To: Abhijit Kumbhare 
>, Zoltán 
Balogh >, László 
Sürü >, Miklós Pelyva 
>, "Jozef Bacigal 
-X (jbacigal - PANTHEON TECHNOLOGIES at Cisco)" 
>, Tomáš Slušný 
>, Prasanna 
Huddar >, 
Shuva Jyoti Kar 
>, Sharath 
Kumar V >, 
Kanagasundaram K 
>, Sunil 
Kumar G >, D 
Arunprakash >, 
Muthukumaran K 
>, "Jarno 
Rajahalme (ja...@ovn.org)" 
>, 
"d...@openvswitch.org" 
>
Subject: RE: OVS - ODL Sync on OF Bundle in 1.3

Hi,

Here are the main differences I have found between the ONF Bundle extension 
(EXT-230) to OF 1.3 and the OF Bundle specified in OF 1.5.1:

  *   No OFPC_BUNDLES bit in ofp_capabilities to indicate switch support for 
bundles.
  *   Bundle Features MP message not supported
  *   No support for scheduled bundles
  *   ONF Experimenter messages 2300 (ONF_ET_BUNDLE_CONTROL) and 2301 
(ONF_ET_BUNDLE_ADD_MESSAGE) instead of standard OFP messages 33 
(OFP_BUNDLE_CONTROL) and 34 (OFP_ BUNDLE_ADD_MESSAGE).
Other than that the message syntax and semantics seem identical
  *   ONF experimenter error codes for bundles errors in the range 2300-2315 
instead of standard error codes 0-15 for error type OFPET_BUNDLE_FAILED. 
Otherwise same meaning.

Inside OVS it seems that standard OF 1.5 and OF 1.3 extension messages are 
internally mapped to a common message type. Errors are mapped back to standard 
OFP error (type, code) for OF 1.5 and experimenter error type in OF 1.3.

So, the OVS bundle implementation for OF 1.3 should be faithful to EXT-230.

BR, Jan

-Original Appointment-
From: Jan Scheurich
Sent: Wednesday, 18 January, 2017 16:23
To: Jan Scheurich; Abhijit Kumbhare; Zoltán Balogh; László Sürü; Miklós Pelyva; 
Jozef Bacigál; Tomáš Slušný; Prasanna Huddar; Shuva Jyoti Kar; Sharath Kumar V; 
Kanagasundaram K; Sunil Kumar G; D Arunprakash; Muthukumaran K; Jarno Rajahalme 
(ja...@ovn.org); 
d...@openvswitch.org
Subject: OVS - ODL Sync on OF Bundle in 1.3
When: Monday, 23 January, 2017 17:00-18:00 (UTC+01:00) Amsterdam, Berlin, Bern, 
Rome, Stockholm, Vienna.
Where: Skype Meeting

Hi Jarno,

OpenDaylight folks are finally starting to implement support of OpenFlow 
bundles as a basis for the bundle-based hitless recync procedure we discussed 
earlier. As ODL does not yet have protocol support for OpenFlow versions 1.4 or 
1.5, they intend to implement the bundle extension to OF 1.3 as specified in 
EXT-230 in
https://www.opennetworking.org/images/stories/downloads/sdn-resources/onf-specifications/openflow/openflow-extensions-1.3.x-pack2.zip

Would you have time for a short meeting on early Monday to discuss and confirm 
whether the OVS implementation of bundles in OF 1.3 is compliant with EXT-230 
and has everything they need?

Thanks, Jan

.
--> Join Skype Meeting
This is an online meeting for Skype for Business, the professional meetings and 
communications app formerly known as Lync.
Join by phone

+492115343925 (Germany)  English (United 
States)
89925 (Germany)  English (United States)

Find a local number

Conference ID: 64134840
Forgot your dial-in PIN? 
|Help

To join a Lync / Skype for Business meeting from an Ericsson standard video 
room, add 77 before the Conference ID (e.g. 771234567 where 1234567 is the 
conference ID).To join from a video room outside of Ericsson add one of the 
domains after 77 and

Re: [ovs-dev] [PATCH ovs V2 03/21] other-config: Add hw-offload switch to control netdev flow offloading

2017-01-24 Thread Joe Stringer

On 24 January 2017 at 00:45, Roi Dayan  wrote:
>
>
> On 1/23/2017 8:41 PM, Joe Stringer wrote:
>>
>> On 22 January 2017 at 08:13, Paul Blakey  wrote:
>>>
>>>
>>> On 05/01/2017 03:26, Joe Stringer wrote:

 On 25 December 2016 at 03:39, Paul Blakey  wrote:
>
> Add a new configuration option - hw-offload that enables netdev
> flow api. Enabling this option will allow offloading flows
> using netdev implementation instead of the kernel datapath.
> This configuration option defaults to false - disabled.
>
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> ---
>   lib/netdev.c | 18 ++
>   lib/netdev.h |  2 ++
>   vswitchd/bridge.c|  2 ++
>   vswitchd/vswitch.xml | 11 +++
>   4 files changed, 33 insertions(+)
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 3ac3c48..b289166 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -2071,7 +2071,25 @@ netdev_init_flow_api(struct netdev *netdev)
>   {
>   const struct netdev_class *class = netdev->netdev_class;
>
> +if (!netdev_flow_api_enabled) {
> +return EOPNOTSUPP;
> +}
> +
>   return (class->init_flow_api
>   ? class->init_flow_api(netdev)
>   : EOPNOTSUPP);
>   }
> +
> +bool netdev_flow_api_enabled = false;
> +
> +void
> +netdev_set_flow_api_enabled(bool enabled)
> +{
> +static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> +
> +if (ovsthread_once_start()) {
> +netdev_flow_api_enabled = enabled;
> +VLOG_INFO("netdev: Flow API %s", enabled ? "Enabled" :
> "Disabled");
> +ovsthread_once_done();
> +}
> +}


 Requiring restart to apply this option seems a bit arbitrary, why not
 allow it to be changed at runtime?

>>> Because it isn't something that is supposed to change often,
>>> It might cause problems if its enabled later on with different (netdev)
>>> implementations. We can try and test changing it to runtime at a later
>>> time.
>>
>> Forcing the user to restart OVS to make a database setting take effect
>> is an unusual requirement. Even uncommon configurations that require
>> datapath-level changes (eg, reconfiguring queues) is applied in OVS
>> without requiring the user to intervene and restart. I understand that
>> even DPDK init doesn't require restart any more.
>>
>> Here's another question: Let's say that hardware offloads is enabled
>> and traffic runs. User turns it off, restarts OVS. What happens to the
>> hardware flows? Presumably OVS doesn't manage them any more because
>> hw-offloads is off, but they would continue to forward traffic. If OVS
>> then gets different OpenFlow rules then the forwarding could be wrong.
>
>
> Hi Joe,
>
> We understand that forcing the user to restart OVS is not something common
> we want to do.
> We can make this option changeable at runtime and flush all the rules when
> hw-offload changes from true to false.
> If something will block us we'll let you know and if a future requirement
> will need this then it can be changed later.
>
> To answer your next question then this is also what happens now when the
> user restarts OVS. All rules are flushed so there shoudn't be HW forwarding
> rules.

How are they flushed?

I don't think ovs-vswitchd flushes flows when it stops.

Typically if you restart OVS, then on startup the flow table will be
empty so before you get a chance to install your OpenFlow flows again,
OVS will be running, dumping datapath flows, and deleting them because
they don't match the current OpenFlow table. However, now that you've
disabled hw flows, I suspect that OVS won't try to manage them. So the
hw flows will continue to forward according to policy prior to
ovs-vswitchd shutdown.

There is also a way to start OVS where it will wait for you to finish
initializing the OpenFlow tables before it starts revalidating the
flows (ovs_vsctl set open_vswitch .
other_config:flow-restore-wait="true") - see utilities/ovs-ctl.in for
how this may be used.

These are all only considerations where the datapath has its own life
beyond the OVS binary. They don't apply when running with userspace
datapath, clearly, because the datapath and ovs-vswitchd are the same
in that case.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovn-northd: Add flows in DHCP_OPTIONS pipeline to support renew requests

2017-01-24 Thread Numan Siddique

ovn-northd adds the flows to send the DHCPv4 packets to ovn-controller
only with the match ip4.src = 0.0.0.0 and ip4.dst = 255.255.255.255.

When a DHCPv4 lease is about to expire, before sending a DHCPDISCOVER
packet, the client can send a DHCPREQUEST packet to renew its ip
with ip4.src set to its offered ip and ip4.dst set to the DHCP server
ip or broadcast ip.

This patch supports this missing scenario by adding the necessary
flows in DHCP_OPTIONS ingress pipeline.

Signed-off-by: Numan Siddique 
---
 ovn/northd/ovn-northd.c | 37 +
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 87c80d1..3b05470 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2115,7 +2115,8 @@ lsp_is_up(const struct nbrec_logical_switch_port *lsp)
 
 static bool
 build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,
-struct ds *options_action, struct ds *response_action)
+struct ds *options_action, struct ds *response_action,
+struct ds *ipv4_addr_match)
 {
 if (!op->nbsp->dhcpv4_options) {
 /* CMS has disabled native DHCPv4 for this lport. */
@@ -2185,6 +2186,9 @@ build_dhcpv4_action(struct ovn_port *op, ovs_be32 
offer_ip,
   "output;",
   server_mac, IP_ARGS(offer_ip), server_ip);
 
+ds_put_format(ipv4_addr_match,
+  "ip4.src == "IP_FMT" && ip4.dst == {%s, 255.255.255.255}",
+  IP_ARGS(offer_ip), server_ip);
 smap_destroy(_options);
 return true;
 }
@@ -3085,9 +3089,10 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) {
 struct ds options_action = DS_EMPTY_INITIALIZER;
 struct ds response_action = DS_EMPTY_INITIALIZER;
+struct ds ipv4_addr_match = DS_EMPTY_INITIALIZER;
 if (build_dhcpv4_action(
 op, op->lsp_addrs[i].ipv4_addrs[j].addr,
-_action, _action)) {
+_action, _action, _addr_match)) {
 struct ds match = DS_EMPTY_INITIALIZER;
 ds_put_format(
 , "inport == %s && eth.src == %s && "
@@ -3098,15 +3103,39 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_OPTIONS,
   100, ds_cstr(),
   ds_cstr(_action));
+ds_clear();
+/* Allow ip4.src = OFFER_IP and
+ * ip4.dst = {SERVER_IP, 255.255.255.255} for the below
+ * cases
+ *  -  When the client wants to renew the IP by sending
+ * the DHCPREQUEST to the server ip.
+ *  -  When the client wants to renew the IP by
+ * broadcasting the DHCPREQUEST.
+ */
+ds_put_format(
+, "inport == %s && eth.src == %s && "
+"%s && udp.src == 68 && udp.dst == 67", op->json_key,
+op->lsp_addrs[i].ea_s, ds_cstr(_addr_match));
+
+ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_OPTIONS,
+  100, ds_cstr(),
+  ds_cstr(_action));
+ds_clear();
+
 /* If REGBIT_DHCP_OPTS_RESULT is set, it means the
- * put_dhcp_opts action  is successful */
-ds_put_cstr(, " && "REGBIT_DHCP_OPTS_RESULT);
+ * put_dhcp_opts action  is successful. */
+ds_put_format(
+, "inport == %s && eth.src == %s && "
+"ip4 && udp.src == 68 && udp.dst == 67"
+" && "REGBIT_DHCP_OPTS_RESULT, op->json_key,
+op->lsp_addrs[i].ea_s);
 ovn_lflow_add(lflows, op->od, S_SWITCH_IN_DHCP_RESPONSE,
   100, ds_cstr(),
   ds_cstr(_action));
 ds_destroy();
 ds_destroy(_action);
 ds_destroy(_action);
+ds_destroy(_addr_match);
 break;
 }
 }
-- 
2.9.3

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

[ovs-dev] [patch_v3 5/5] Enable NAT tests for userspace datapath.

2017-01-24 Thread Darrell Ball

Signed-off-by: Darrell Ball 
---
 tests/system-userspace-macros.at | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at
index 631f71a..6e3d468 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -99,9 +99,6 @@ m4_define([CHECK_CONNTRACK_LOCAL_STACK],
 # CHECK_CONNTRACK_NAT()
 #
 # Perform requirements checks for running conntrack NAT tests. The userspace
-# doesn't support NATs yet, so skip the tests
+# datapath supports NAT.
 #
-m4_define([CHECK_CONNTRACK_NAT],
-[
-AT_SKIP_IF([:])
-])
+m4_define([CHECK_CONNTRACK_NAT])
-- 
1.9.1

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

[ovs-dev] [patch_v3 4/5] unset CS_NEW for established connections

2017-01-24 Thread Darrell Ball

Signed-off-by: Darrell Ball 
---
 lib/conntrack.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index c629541..d618f24 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -441,6 +441,7 @@ conn_update_state(struct conntrack *ct, struct dp_packet 
*pkt,
 switch (res) {
 case CT_UPDATE_VALID:
 *state |= CS_ESTABLISHED;
+*state &= ~CS_NEW;
 if (ctx->reply) {
 *state |= CS_REPLY_DIR;
 }
-- 
1.9.1

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

[ovs-dev] [patch_v3 2/5] Parse NAT netlink for userspace datapath.

2017-01-24 Thread Darrell Ball

Signed-off-by: Darrell Ball 
---
 lib/conntrack-private.h |  9 --
 lib/conntrack.c |  3 +-
 lib/conntrack.h | 31 +-
 lib/dpif-netdev.c   | 85 ++---
 tests/test-conntrack.c  |  8 +++--
 5 files changed, 118 insertions(+), 18 deletions(-)

diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 013f19f..493865f 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -29,15 +29,6 @@
 #include "packets.h"
 #include "unaligned.h"
 
-struct ct_addr {
-union {
-ovs_16aligned_be32 ipv4;
-union ovs_16aligned_in6_addr ipv6;
-ovs_be32 ipv4_aligned;
-struct in6_addr ipv6_aligned;
-};
-};
-
 struct ct_endpoint {
 struct ct_addr addr;
 union {
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 9bea3d9..bae42a3 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -273,7 +273,8 @@ conntrack_execute(struct conntrack *ct, struct 
dp_packet_batch *pkt_batch,
   ovs_be16 dl_type, bool commit, uint16_t zone,
   const uint32_t *setmark,
   const struct ovs_key_ct_labels *setlabel,
-  const char *helper)
+  const char *helper,
+ const struct nat_action_info_t 
*nat_action_info OVS_UNUSED)
 {
 struct dp_packet **pkts = pkt_batch->packets;
 size_t cnt = pkt_batch->count;
diff --git a/lib/conntrack.h b/lib/conntrack.h
index 254f61c..cbdfb91 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -26,6 +26,8 @@
 #include "openvswitch/thread.h"
 #include "openvswitch/types.h"
 #include "ovs-atomic.h"
+#include "ovs-thread.h"
+#include "packets.h"
 
 /* Userspace connection tracker
  * 
@@ -61,6 +63,32 @@ struct dp_packet_batch;
 
 struct conntrack;
 
+struct ct_addr {
+union {
+ovs_16aligned_be32 ipv4;
+union ovs_16aligned_in6_addr ipv6;
+ovs_be32 ipv4_aligned;
+struct in6_addr ipv6_aligned;
+};
+};
+
+// Both NAT_ACTION_* and NAT_ACTION_*_PORT can be set
+enum nat_action_e {
+   NAT_ACTION = 1 << 0,
+   NAT_ACTION_SRC = 1 << 1,
+   NAT_ACTION_SRC_PORT = 1 << 2,
+   NAT_ACTION_DST = 1 << 3,
+   NAT_ACTION_DST_PORT = 1 << 4,
+};
+
+struct nat_action_info_t {
+   struct ct_addr min_addr;
+   struct ct_addr max_addr;
+   uint16_t min_port;
+   uint16_t max_port;
+uint16_t nat_action;
+};
+
 void conntrack_init(struct conntrack *);
 void conntrack_destroy(struct conntrack *);
 
@@ -68,7 +96,8 @@ int conntrack_execute(struct conntrack *, struct 
dp_packet_batch *,
   ovs_be16 dl_type, bool commit,
   uint16_t zone, const uint32_t *setmark,
   const struct ovs_key_ct_labels *setlabel,
-  const char *helper);
+  const char *helper,
+  const struct nat_action_info_t *nat_action_info);
 
 struct conntrack_dump {
 struct conntrack *ct;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3901129..a71c766 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -97,7 +97,8 @@ static struct shash dp_netdevs OVS_GUARDED_BY(dp_netdev_mutex)
 static struct vlog_rate_limit upcall_rl = VLOG_RATE_LIMIT_INIT(600, 600);
 
 #define DP_NETDEV_CS_SUPPORTED_MASK (CS_NEW | CS_ESTABLISHED | CS_RELATED \
- | CS_INVALID | CS_REPLY_DIR | CS_TRACKED)
+ | CS_INVALID | CS_REPLY_DIR | CS_TRACKED \
+ | CS_SRC_NAT | CS_DST_NAT)
 #define DP_NETDEV_CS_UNSUPPORTED_MASK (~(uint32_t)DP_NETDEV_CS_SUPPORTED_MASK)
 
 static struct odp_support dp_netdev_support = {
@@ -4681,7 +4682,9 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 const char *helper = NULL;
 const uint32_t *setmark = NULL;
 const struct ovs_key_ct_labels *setlabel = NULL;
-
+struct nat_action_info_t nat_action_info;
+bool nat = false;
+memset(_action_info, 0, sizeof nat_action_info);
 NL_ATTR_FOR_EACH_UNSAFE (b, left, nl_attr_get(a),
  nl_attr_get_size(a)) {
 enum ovs_ct_attr sub_type = nl_attr_type(b);
@@ -4702,15 +4705,89 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 case OVS_CT_ATTR_LABELS:
 setlabel = nl_attr_get(b);
 break;
-case OVS_CT_ATTR_NAT:
+case OVS_CT_ATTR_NAT: {
+const struct nlattr *b_nest;
+unsigned int left_nest;
+bool ip_min_specified = false;
+bool proto_num_min_specified = false;
+bool ip_max_specified = false;
+bool proto_num_max_specified = false;
+
+NL_NESTED_FOR_EACH_UNSAFE (b_nest, left_nest, b) {
+enum ovs_nat_attr

[ovs-dev] [patch_v3 1/5] Export packet_set_ipv6_addr() fordpdkdatapath.

2017-01-24 Thread Darrell Ball

Signed-off-by: Darrell Ball 
---
 lib/packets.c | 2 +-
 lib/packets.h | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/packets.c b/lib/packets.c
index fa70df6..94e7d87 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -986,7 +986,7 @@ packet_update_csum128(struct dp_packet *packet, uint8_t 
proto,
 }
 }
 
-static void
+void
 packet_set_ipv6_addr(struct dp_packet *packet, uint8_t proto,
  ovs_16aligned_be32 addr[4],
  const struct in6_addr *new_addr,
diff --git a/lib/packets.h b/lib/packets.h
index c4d3799..850f192 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1100,6 +1100,10 @@ void packet_set_ipv4_addr(struct dp_packet *packet, 
ovs_16aligned_be32 *addr,
 void packet_set_ipv6(struct dp_packet *, const struct in6_addr *src,
  const struct in6_addr *dst, uint8_t tc,
  ovs_be32 fl, uint8_t hlmit);
+void packet_set_ipv6_addr(struct dp_packet *packet, uint8_t proto,
+  ovs_16aligned_be32 addr[4],
+  const struct in6_addr *new_addr,
+  bool recalculate_csum);
 void packet_set_tcp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
 void packet_set_udp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
 void packet_set_sctp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
-- 
1.9.1

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

[ovs-dev] [patch_v3 0/5] Userspace Datapath: introduce NAT support.

2017-01-24 Thread Darrell Ball

This patch series introduces NAT support for the userspace datapath.

The per packet scope of lookups for NAT and un_NAT is at
the bucket level rather than global. One hash table is 
introduced to support create/delete handling. The create/delete
events may be further optimized, if the need becomes clear.

The existing NAT tests are enabled for the dpdk datapath,
although additional tests will be done.

Some NAT options with limited utility (persistent, random) are
not supported yet, but will be supported in a later patch.

One V6 api is exported to facilitate selective editing the V6
header - packet_set_ipv6_addr().

alg and fragmentation support are not included here but are
being worked on.

NEWS is not updated in this series yet, until confirmation of
release.

I realize patch 3 is big. It may be clearer and easier to keep
as a single patch, so I have done that after some discussion.

v2->v3: Fix a theoretical resend for closed connection restart.
Parse out a function to help and also limit
conn_state_update() to one.

I decided to cap V6 address range delta at 4 billion using
internal adjustment (user visibility not required).

Some cleanup of deprecated code path.

Parse out some more changes as separate patches.
 
v1->v2: Updates/fixes that were missed in v1 patches.

Darrell Ball (5):
  Export packet_set_ipv6_addr() fordpdkdatapath.
  Parse NAT netlink for userspace datapath.
  Userspace Datapath Conntrack: Introduce NAT support.
  unset CS_NEW for established connections
  Enable NAT tests for userspace datapath.

 lib/conntrack-private.h  |  25 +-
 lib/conntrack.c  | 742 ++-
 lib/conntrack.h  |  73 +++-
 lib/dpif-netdev.c|  85 -
 lib/packets.c|   2 +-
 lib/packets.h|   4 +
 tests/system-userspace-macros.at |   7 +-
 tests/test-conntrack.c   |   8 +-
 8 files changed, 839 insertions(+), 107 deletions(-)

-- 
1.9.1

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

Re: [ovs-dev] [PATCH] datapath: internal-dev: Disable rtnl_link_ops register under linux < 3.17

2017-01-24 Thread Pravin Shelar

On Mon, Jan 23, 2017 at 10:04 AM, Jarno Rajahalme  wrote:
> Thomas & Pravin,
>
> Apparently the commit 5282e284ac57 (“datapath: introduce rtnl ops stub”)
> applied to OVS two years ago is not compatible with Linux 3.16 or earlier,
> as the corresponding upstream change was only applied in Linux 3.17. For
> Linux 3.16 and earlier, in your opinion, is the right solution to
> effectively revert this commit, or would there be a way to make the ops stub
> compatible with these Linux versions by somehow amending the stub (.setup,
> .dellink, etc.)?
>
Sorry for the delay. the patch fell off my radar. I have couple of
comments below.


>   Jarno
>
> Begin forwarded message:
>
> From: Markos Chandras 
> Subject: Re: [ovs-dev] [PATCH] datapath: internal-dev: Disable rtnl_link_ops
> register under linux < 3.17
> Date: January 23, 2017 at 7:13:03 AM PST
> To: ovs-dev@openvswitch.org
>
> Hi,
>
> On 12/14/2016 03:44 AM, Zhang Dongya wrote:
>
> From: "fortitude.zhang" 
>
> Under linux < 3.17, b0ab2fabb5b91da99c189db02e91ae10bc8355c5 is not
> introduced,
> which allow internal-dev created by openvswitch datapath deleted by using
> rtnl
> interface, this causes data related to internal-dev not freed and stops
> datapath
> working correctly.
>

What about backports? can we detect if the upstream commit is
backported by distribution kernel?
if it is not possible to detect it, have you tried setting
unregister_netdevice_queue() to vport rtnl dellink operation or
something similar?

Thanks,
Pravin.

> Signed-off-by: fortitude.zhang 
>
>
> I recently encountered this bug and this commit fixed it for me. So...
>
> Tested-by: Markos Chandras 
>
> It might also worth backporting it to stable branches as well.
>
> --
> markos
>
> SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] openvswitch 2.6.2~pre+git20161223-3 MIGRATED to testing

2017-01-24 Thread Debian testing watch

FYI: The status of the openvswitch source package
in Debian's testing distribution has changed.

  Previous version: 2.6.2~pre+git20161223-2
  Current version:  2.6.2~pre+git20161223-3

-- 
This email is automatically generated once a day.  As the installation of
new packages into testing happens multiple times a day you will receive
later changes on the next day.
See https://release.debian.org/testing-watch/ for more information.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Puppet User List

2017-01-24 Thread lois . collins




Hi,



Would you be interested in all Puppet  users  information for your sales
and marketing campaigns?



If you looking for particular titles from your target geography please let
me know and I will get back to you with more information regarding the same.



We also have other technology users like



1. Jenkins users

2. Codeship users

3. Travis CI users

4. Ansible users

5. CircleCI users

6. TeamCity users

7. Distelli list

8. AppVeyor list

9. AWS Cloud Formation list

10. Microsoft Team Foundation Server List



Please review and let me know your selection.



Await your response!



Thanks,

Lois collins

Data Specialist



 “To opt out please
response Remove”
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Sync on PTAP, EXT-382 and NSH

2017-01-24 Thread Jiri Benc

On Wed, 18 Jan 2017 09:53:18 +, Jan Scheurich wrote:
> Please be invited to the next sync meeting.

Sorry, won't make the meeting today. Too busy with DevConf.cz
preparations.

> Actions Points:
> AP-1 (Jarno): Coordinate review of Yi's backported net-next patches
> AP-2 (Jiri) Check the ability of the kernel datapath to match on the
> presence of Ethernet header w/o matching on Ethernet addresses.
> Formating of such DP flows?

The OVS_KEY_ATTR_ETHERNET netlink attribute contains source and
destination MAC address. It can be masked.

If the OVS_KEY_ATTR_ETHERNET attribute is present, only Ethernet frames
will match. The MAC addresses may be masked (wildcarded). The
OVS_KEY_ATTR_ETHERTYPE attribute indicates the ethertype to match and
cannot be masked (it's always an exact match). If it's not present,
ETH_P_802_2 is assumed.

If the OVS_KEY_ATTR_ETHERNET in not present, the OVS_KEY_ATTR_ETHERTYPE
attribute is mandatory and indicates the ethertype to match. Again, it
cannot be masked.

To answer the question, it's currently possible to match on Ethernet
header without matching on Ethernet addresses but not without matching
on ethertype. I think it can be relaxed with some work if needed.

> AP-3 (Jiri) Provide an update on the status and ETA of the RTNETLINK API 
> patches

RFC patchset posted last week:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327749.html

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

Re: [ovs-dev] [PATCH ovs V2 03/21] other-config: Add hw-offload switch to control netdev flow offloading

2017-01-24 Thread Roi Dayan




On 1/23/2017 8:41 PM, Joe Stringer wrote:

On 22 January 2017 at 08:13, Paul Blakey  wrote:


On 05/01/2017 03:26, Joe Stringer wrote:

On 25 December 2016 at 03:39, Paul Blakey  wrote:

Add a new configuration option - hw-offload that enables netdev
flow api. Enabling this option will allow offloading flows
using netdev implementation instead of the kernel datapath.
This configuration option defaults to false - disabled.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
  lib/netdev.c | 18 ++
  lib/netdev.h |  2 ++
  vswitchd/bridge.c|  2 ++
  vswitchd/vswitch.xml | 11 +++
  4 files changed, 33 insertions(+)

diff --git a/lib/netdev.c b/lib/netdev.c
index 3ac3c48..b289166 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2071,7 +2071,25 @@ netdev_init_flow_api(struct netdev *netdev)
  {
  const struct netdev_class *class = netdev->netdev_class;

+if (!netdev_flow_api_enabled) {
+return EOPNOTSUPP;
+}
+
  return (class->init_flow_api
  ? class->init_flow_api(netdev)
  : EOPNOTSUPP);
  }
+
+bool netdev_flow_api_enabled = false;
+
+void
+netdev_set_flow_api_enabled(bool enabled)
+{
+static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+
+if (ovsthread_once_start()) {
+netdev_flow_api_enabled = enabled;
+VLOG_INFO("netdev: Flow API %s", enabled ? "Enabled" :
"Disabled");
+ovsthread_once_done();
+}
+}


Requiring restart to apply this option seems a bit arbitrary, why not
allow it to be changed at runtime?


Because it isn't something that is supposed to change often,
It might cause problems if its enabled later on with different (netdev)
implementations. We can try and test changing it to runtime at a later time.

Forcing the user to restart OVS to make a database setting take effect
is an unusual requirement. Even uncommon configurations that require
datapath-level changes (eg, reconfiguring queues) is applied in OVS
without requiring the user to intervene and restart. I understand that
even DPDK init doesn't require restart any more.

Here's another question: Let's say that hardware offloads is enabled
and traffic runs. User turns it off, restarts OVS. What happens to the
hardware flows? Presumably OVS doesn't manage them any more because
hw-offloads is off, but they would continue to forward traffic. If OVS
then gets different OpenFlow rules then the forwarding could be wrong.


Hi Joe,

We understand that forcing the user to restart OVS is not something 
common we want to do.
We can make this option changeable at runtime and flush all the rules 
when hw-offload changes from true to false.
If something will block us we'll let you know and if a future 
requirement will need this then it can be changed later.


To answer your next question then this is also what happens now when the 
user restarts OVS. All rules are flushed so there shoudn't be HW 
forwarding rules.


Thanks,
Roi


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

[ovs-dev] [userspace meter v2 3/5] dpif: Meter framework.

2017-01-24 Thread Andy Zhou

From: Jarno Rajahalme 

Add DPIF-level infrastructure for meters.  Allow meter_set to modify
the meter configuration (e.g. set the burst size if unspecified).

Signed-off-by: Jarno Rajahalme 
Signed-off-by: Andy Zhou 
---
 datapath/linux/compat/include/linux/openvswitch.h |  4 +-
 lib/dpif-netdev.c | 45 
 lib/dpif-netlink.c| 46 +++-
 lib/dpif-provider.h   | 29 
 lib/dpif.c| 88 +++
 lib/dpif.h| 13 +++-
 lib/odp-execute.c |  9 ++-
 lib/odp-util.c| 14 
 ofproto/ofproto-dpif-sflow.c  |  1 +
 ofproto/ofproto-dpif.c| 60 ++--
 ofproto/ofproto-provider.h| 13 ++--
 ofproto/ofproto.c |  2 +-
 12 files changed, 307 insertions(+), 17 deletions(-)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 425d3a4..b121391 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -787,13 +787,14 @@ enum ovs_nat_attr {
  * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
  * type may not be changed.
  *
- *
  * @OVS_ACTION_ATTR_SET_TO_MASKED: Kernel internal masked set action translated
  * from the @OVS_ACTION_ATTR_SET.
  * @OVS_ACTION_ATTR_TUNNEL_PUSH: Push tunnel header described by struct
  * ovs_action_push_tnl.
  * @OVS_ACTION_ATTR_TUNNEL_POP: Lookup tunnel port by port-no passed and pop
  * tunnel header.
+ * @OVS_ACTION_ATTR_METER: Run packet through a meter, which may drop the
+ * packet, or modify the packet (e.g., change the DSCP field).
  */
 
 enum ovs_action_attr {
@@ -819,6 +820,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
OVS_ACTION_ATTR_CLONE, /* Nested OVS_CLONE_ATTR_*.  */
+   OVS_ACTION_ATTR_METER, /* u32 meter number. */
 #endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4757d2c..0eb23e8 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3597,6 +3597,46 @@ dp_netdev_disable_upcall(struct dp_netdev *dp)
 fat_rwlock_wrlock(>upcall_rwlock);
 }
 
+
+/* Meters */
+static void
+dpif_netdev_meter_get_features(const struct dpif * dpif OVS_UNUSED,
+   struct ofputil_meter_features *features)
+{
+features->max_meters = 0;
+features->band_types = 0;
+features->capabilities = 0;
+features->max_bands = 0;
+features->max_color = 0;
+}
+
+static int
+dpif_netdev_meter_set(struct dpif *dpif OVS_UNUSED,
+  ofproto_meter_id *meter_id OVS_UNUSED,
+  struct ofputil_meter_config *config OVS_UNUSED)
+{
+return EFBIG; /* meter_id out of range */
+}
+
+static int
+dpif_netdev_meter_get(const struct dpif *dpif OVS_UNUSED,
+  ofproto_meter_id meter_id OVS_UNUSED,
+  struct ofputil_meter_stats *stats OVS_UNUSED,
+  uint16_t n_bands OVS_UNUSED)
+{
+return EFBIG; /* meter_id out of range */
+}
+
+static int
+dpif_netdev_meter_del(struct dpif *dpif OVS_UNUSED,
+  ofproto_meter_id meter_id OVS_UNUSED,
+  struct ofputil_meter_stats *stats OVS_UNUSED,
+  uint16_t n_bands OVS_UNUSED)
+{
+return EFBIG; /* meter_id out of range */
+}
+
+
 static void
 dpif_netdev_disable_upcall(struct dpif *dpif)
 OVS_NO_THREAD_SAFETY_ANALYSIS
@@ -4723,6 +4763,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch 
*packets_,
 break;
 }
 
+case OVS_ACTION_ATTR_METER:
 case OVS_ACTION_ATTR_PUSH_VLAN:
 case OVS_ACTION_ATTR_POP_VLAN:
 case OVS_ACTION_ATTR_PUSH_MPLS:
@@ -4860,6 +4901,10 @@ const struct dpif_class dpif_netdev_class = {
 dpif_netdev_ct_dump_next,
 dpif_netdev_ct_dump_done,
 dpif_netdev_ct_flush,
+dpif_netdev_meter_get_features,
+dpif_netdev_meter_set,
+dpif_netdev_meter_get,
+dpif_netdev_meter_del,
 };
 
 static void
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index c8b0e37..8a48227 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2356,6 +2356,46 @@ dpif_netlink_ct_flush(struct dpif *dpif OVS_UNUSED, 
const uint16_t *zone)
 }
 }
 
+
+/* Meters */
+static void
+dpif_netlink_meter_get_features(const struct dpif * dpif OVS_UNUSED,
+struct ofputil_meter_features *features)
+{
+features->max_meters = 0;
+

[ovs-dev] [userspace meter v2 4/5] ofproto: Meter translation.

2017-01-24 Thread Andy Zhou

From: Jarno Rajahalme 

Translate OpenFlow METER instructions to datapath meter actions.

Signed-off-by: Jarno Rajahalme 
Signed-off-by: Andy Zhou 
---
 include/openvswitch/ofp-actions.h |  1 +
 lib/dpif.c| 40 +---
 lib/ofp-actions.c |  1 +
 ofproto/ofproto-dpif-xlate.c  | 11 -
 ofproto/ofproto.c | 49 ++-
 5 files changed, 72 insertions(+), 30 deletions(-)

diff --git a/include/openvswitch/ofp-actions.h 
b/include/openvswitch/ofp-actions.h
index 8ca787a..e269901 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -532,6 +532,7 @@ struct ofpact_metadata {
 struct ofpact_meter {
 struct ofpact ofpact;
 uint32_t meter_id;
+uint32_t provider_meter_id;
 };
 
 /* OFPACT_WRITE_ACTIONS, OFPACT_CLONE.
diff --git a/lib/dpif.c b/lib/dpif.c
index 4e9476c..cc49d94 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1104,6 +1104,7 @@ struct dpif_execute_helper_aux {
 struct dpif *dpif;
 const struct flow *flow;
 int error;
+const struct nlattr *meter_action; /* Non-NULL, if have a meter action. */
 };
 
 /* This is called for actions that need the context of the datapath to be
@@ -1119,6 +1120,13 @@ dpif_execute_helper_cb(void *aux_, struct 
dp_packet_batch *packets_,
 ovs_assert(packets_->count == 1);
 
 switch ((enum ovs_action_attr)type) {
+case OVS_ACTION_ATTR_METER:
+/* Maintain a pointer to the first meter action seen. */
+if (!aux->meter_action) {
+aux->meter_action = action;
+}
+   break;
+
 case OVS_ACTION_ATTR_CT:
 case OVS_ACTION_ATTR_OUTPUT:
 case OVS_ACTION_ATTR_TUNNEL_PUSH:
@@ -1129,15 +1137,29 @@ dpif_execute_helper_cb(void *aux_, struct 
dp_packet_batch *packets_,
 struct ofpbuf execute_actions;
 uint64_t stub[256 / 8];
 struct pkt_metadata *md = >md;
-bool dst_set;
 
-dst_set = flow_tnl_dst_is_set(>tunnel);
-if (dst_set) {
+if (flow_tnl_dst_is_set(>tunnel) || aux->meter_action) {
+ofpbuf_use_stub(_actions, stub, sizeof stub);
+
+if (aux->meter_action) {
+const struct nlattr *a = aux->meter_action;
+
+do {
+ofpbuf_put(_actions, a, NLA_ALIGN(a->nla_len));
+/* Find next meter action before 'action', if any. */
+do {
+a = nl_attr_next(a);
+} while (a != action &&
+ nl_attr_type(a) != OVS_ACTION_ATTR_METER);
+} while (a != action);
+}
+
 /* The Linux kernel datapath throws away the tunnel information
  * that we supply as metadata.  We have to use a "set" action to
  * supply it. */
-ofpbuf_use_stub(_actions, stub, sizeof stub);
-odp_put_tunnel_action(>tunnel, _actions);
+if (md->tunnel.ip_dst) {
+odp_put_tunnel_action(>tunnel, _actions);
+}
 ofpbuf_put(_actions, action, NLA_ALIGN(action->nla_len));
 
 execute.actions = execute_actions.data;
@@ -1170,14 +1192,16 @@ dpif_execute_helper_cb(void *aux_, struct 
dp_packet_batch *packets_,
 
 dp_packet_delete(clone);
 
-if (dst_set) {
+if (flow_tnl_dst_is_set(>tunnel) || aux->meter_action) {
 ofpbuf_uninit(_actions);
+
+/* Do not re-use the same meters for later output actions. */
+aux->meter_action = NULL;
 }
 break;
 }
 
 case OVS_ACTION_ATTR_HASH:
-case OVS_ACTION_ATTR_METER:
 case OVS_ACTION_ATTR_PUSH_VLAN:
 case OVS_ACTION_ATTR_POP_VLAN:
 case OVS_ACTION_ATTR_PUSH_MPLS:
@@ -1201,7 +1225,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch 
*packets_,
 static int
 dpif_execute_with_help(struct dpif *dpif, struct dpif_execute *execute)
 {
-struct dpif_execute_helper_aux aux = {dpif, execute->flow, 0};
+struct dpif_execute_helper_aux aux = {dpif, execute->flow, 0, NULL};
 struct dp_packet_batch pb;
 
 COVERAGE_INC(dpif_execute_with_help);
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index cf1ad0f..733b2c5 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -6868,6 +6868,7 @@ ofpacts_pull_openflow_instructions(struct ofpbuf 
*openflow,
 
 om = ofpact_put_METER(ofpacts);
 om->meter_id = ntohl(oim->meter_id);
+om->provider_meter_id = UINT32_MAX; /* No provider meter ID. */
 }
 if (insts[OVSINST_OFPIT11_APPLY_ACTIONS]) {
 const struct ofp_action_header *actions;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 48b27a6..166e236 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4579,6 +4579,15 @@ xlate_clone(struct xlate_ctx *ctx, const struct

[ovs-dev] [userspace meter v2 2/5] dp-packet: Enhance packet batch APIs.

2017-01-24 Thread Andy Zhou

One common use case of 'struct dp_packet_batch' is to process all
packets in the batch in order. Add an iterator for this use case
to simplify the logic of calling sites,

Another common use case is to drop packets in the batch, by reading
all packets, but writing back pointers of fewer packets. Add macros
to support this use case.

Signed-off-by: Andy Zhou 
---
 lib/dp-packet.h  | 140 ---
 lib/dpif-netdev.c|  56 -
 lib/dpif.c   |   2 +-
 lib/netdev-dummy.c   |  10 ++--
 lib/netdev-linux.c   |   3 +-
 lib/netdev.c |  26 
 lib/odp-execute.c|  49 +++
 ofproto/ofproto-dpif-xlate.c |   2 +-
 tests/test-conntrack.c   |  56 -
 9 files changed, 204 insertions(+), 140 deletions(-)

diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index cf7d247..20ae00c 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -630,81 +630,151 @@ reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
 #endif
 
 enum { NETDEV_MAX_BURST = 32 }; /* Maximum number packets in a batch. */
+enum packet_batch_write_index {
+/* 0 .. NETDEV_MAX_BURST -1, valid write index */
+PACKET_BATCH_READ_ONLY = NETDEV_MAX_BURST };
 
 struct dp_packet_batch {
-int count;
+size_t count;
+size_t windex; /* enum patcket_batch_write_index.  */
 bool trunc; /* true if the batch needs truncate. */
 struct dp_packet *packets[NETDEV_MAX_BURST];
 };
 
-static inline void dp_packet_batch_init(struct dp_packet_batch *b)
+/* Iterator for all packets in 'BATCH' in order. 'IDX' is the 'PACKET' index
+ * in 'BATCH'->packets.
+ *
+ * Note, 'IDX' is Declared within the Iterator, it is only valid within
+ * the scope the iterator. In case 'IDX' is not required,
+ * use 'DP_PACKET_BATCH_FOR_EACH' instead.  */
+#define DP_PACKET_BATCH_FOR_EACH_IDX(IDX, PACKET, BATCH) \
+ovs_assert(!BATCH->windex || BATCH->windex == PACKET_BATCH_READ_ONLY); \
+for (int IDX = 0; IDX < BATCH->count; IDX++)  \
+if ((PACKET = BATCH->packets[IDX]) != NULL) \
+
+#define DP_PACKET_BATCH_FOR_EACH(PACKET, BATCH)\
+ DP_PACKET_BATCH_FOR_EACH_IDX(i, PACKET, BATCH) \
+
+/* 'BATCH' rewrite APIs.
+ *
+ * This macro is useful for cases where some the packets in 'BATCH'
+ * may be dropped. Once the macro is called, 'dp_packet_batch_write()'
+ * should be used to add packets that are dropped back into the 'BATCH',
+ * or the 'BATCH' will become empty after DP_PACKET_BATCH_WRITE_CLOSE().
+ *
+ * A typical code block will look something like the follows:
+ *
+ *  DP_PACKET_BATCH_WRITE_OPEN(batch);
+ *
+ *  DP_PACKET_BATCH_FOR_EACH(packet, batch) {
+ *...
+ *// Write back the packets that are not dropped.
+ *dp_packet_batch_write(batch, packet);
+ *  }
+ *  DP_PACKET_BATCH_WRITE_CLOSE(batch);
+ *
+ * DP_PACKET_BATCH_WRITE_OPEN and DP_PACKET_BATCH_WRITE_CLOSE
+ * must appear in the same code block in pair.
+ *
+ * For packets dropped out of 'batch' the client needs to call
+ * dp_packet_delete() to free the packet.
+ *
+ * It is only safe to rewrite the 'batch' if writes are slower than
+ * read, otherwise, an unread packet can be overwritten. When implementing
+ * packet drop, this is always true.  */
+#define DP_PACKET_BATCH_WRITE_OPEN(BATCH) \
+{ BATCH->windex = 0; \
+
+#define DP_PACKET_BATCH_WRITE_CLOSE(BATCH) \
+  ovs_assert(BATCH->windex != PACKET_BATCH_READ_ONLY);   \
+  BATCH->count = BATCH->windex; \
+  BATCH->windex = PACKET_BATCH_READ_ONLY; };
+
+static inline void dp_packet_batch_init(struct dp_packet_batch *batch)
+{
+batch->count = 0;
+batch->windex = PACKET_BATCH_READ_ONLY;
+batch->trunc = false;
+}
+
+static inline void dp_packet_batch_write(struct dp_packet_batch *batch,
+ struct dp_packet *packet)
 {
-b->count = 0;
-b->trunc = false;
+ovs_assert(batch->windex < NETDEV_MAX_BURST);
+batch->packets[batch->windex++] = packet;
 }
 
 static inline void
 dp_packet_batch_clone(struct dp_packet_batch *dst,
   struct dp_packet_batch *src)
 {
-int i;
+struct dp_packet *packet;
+
+dp_packet_batch_init(dst);
+DP_PACKET_BATCH_WRITE_OPEN(dst);
 
-for (i = 0; i < src->count; i++) {
-dst->packets[i] = dp_packet_clone(src->packets[i]);
+DP_PACKET_BATCH_FOR_EACH (packet, src) {
+dp_packet_batch_write(dst, dp_packet_clone(packet));
 }
-dst->count = src->count;
-dst->trunc = src->trunc;
+
+DP_PACKET_BATCH_WRITE_CLOSE(dst);
 }
 
 static inline void
-packet_batch_init_packet(struct dp_packet_batch *b, struct dp_packet *p)
+dp_packet_batch_init_packet(struct dp_packet_batch *batch, struct dp_packet *p)
+{
+dp_packet_batch_init(batch);
+batch->count = 1;
+batch->packets[0] = p;
+}
+
+static inline bool
+dp_packet_batch_is_empty(const

[ovs-dev] [userspace meter v2 1/5] netdev-dummy: Add --len option for netdev-dummy/receive command

2017-01-24 Thread Andy Zhou

Currently, there is no way to specify the packet size when injecting
a packet via "netdev-dummy/receive" with a flow specification. Thus
far, packet size is not important for testing OVS features, but it
becomes useful in writing unit tests for the meter implementation
in a later patch.

Signed-off-by: Andy Zhou 
---
 lib/netdev-dummy.c | 38 --
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index e6e36cd..10df0a7 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2011, 2012, 2013, 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2010, 2011, 2012, 2013, 2015, 2016, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1433,7 +1433,15 @@ pkt_list_delete(struct ovs_list *l)
 }
 
 static struct dp_packet *
-eth_from_packet_or_flow(const char *s)
+eth_from_packet(const char *s)
+{
+struct dp_packet *packet;
+eth_from_hex(s, );
+return packet;
+}
+
+static struct dp_packet *
+eth_from_flow(const char *s)
 {
 enum odp_key_fitness fitness;
 struct dp_packet *packet;
@@ -1441,10 +1449,6 @@ eth_from_packet_or_flow(const char *s)
 struct flow flow;
 int error;
 
-if (!eth_from_hex(s, )) {
-return packet;
-}
-
 /* Convert string to datapath key.
  *
  * It would actually be nicer to parse an OpenFlow-like flow key here, but
@@ -1540,10 +1544,24 @@ netdev_dummy_receive(struct unixctl_conn *conn,
 for (i = k; i < argc; i++) {
 struct dp_packet *packet;
 
-packet = eth_from_packet_or_flow(argv[i]);
+/* Try to parse 'argv[i]' as packet in hex. */
+packet = eth_from_packet(argv[i]);
+
 if (!packet) {
-unixctl_command_reply_error(conn, "bad packet syntax");
-goto exit;
+/* Try parse 'argv[i]' as odp flow. */
+packet = eth_from_flow(argv[i]);
+
+if (!packet) {
+unixctl_command_reply_error(conn, "bad packet or flow syntax");
+goto exit;
+}
+
+/* Parse optional --len argument immediately follow a 'flow'.  */
+if (argc >= i + 2 && !strcmp(argv[i + 1], "--len")) {
+int packet_size = strtol(argv[i + 2], NULL, 10);
+dp_packet_set_size(packet, packet_size);
+i+=2;
+}
 }
 
 netdev_dummy_queue_packet(dummy_dev, packet, rx_qid);
@@ -1757,7 +1775,7 @@ void
 netdev_dummy_register(enum dummy_level level)
 {
 unixctl_command_register("netdev-dummy/receive",
- "name [--qid queue_id] packet|flow...",
+ "name [--qid queue_id] packet|flow [--len packet 
len]..",
  2, INT_MAX, netdev_dummy_receive, NULL);
 unixctl_command_register("netdev-dummy/set-admin-state",
  "[netdev] up|down", 1, 2,
-- 
1.9.1

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

[ovs-dev] [userspace meter v2 0/5] userspace meter

2017-01-24 Thread Andy Zhou

Repost user space meter support. This is based Jarno's original work
at: https://mail.openvswitch.org/pipermail/ovs-dev/2015-November/306304.html.
With some enhancements, and rebased to current master.

---
v1-v2: rebase and repost.

Andy Zhou (2):
  netdev-dummy: Add --len option for netdev-dummy/receive command
  dp-packet: Enhance packet batch APIs.

Jarno Rajahalme (3):
  dpif: Meter framework.
  ofproto: Meter translation.
  dpif-netdev: Simple DROP meter implementation.

 datapath/linux/compat/include/linux/openvswitch.h |   4 +-
 include/openvswitch/ofp-actions.h |   1 +
 lib/dp-packet.h   | 140 +--
 lib/dpif-netdev.c | 428 --
 lib/dpif-netlink.c|  46 ++-
 lib/dpif-provider.h   |  29 ++
 lib/dpif.c| 128 ++-
 lib/dpif.h|  13 +-
 lib/netdev-dummy.c|  48 ++-
 lib/netdev-linux.c|   3 +-
 lib/netdev.c  |  26 +-
 lib/odp-execute.c |  58 +--
 lib/odp-util.c|  14 +
 lib/ofp-actions.c |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  |  13 +-
 ofproto/ofproto-dpif.c|  60 ++-
 ofproto/ofproto-provider.h|  13 +-
 ofproto/ofproto.c |  51 +--
 tests/dpif-netdev.at  | 106 ++
 tests/test-conntrack.c|  56 ++-
 21 files changed, 1043 insertions(+), 196 deletions(-)

-- 
1.9.1

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

53 matches

Mail list logo