[ovs-dev] [RFC PATCH] xlate: Use dp_hash for select groups.

[Jarno Rajahalme]

Add a new select group selection method "dp_hash", which uses minimal
number of bits from the datapath calculated packet hash to inform the
select group bucket selection.  This makes the datapath flows more
generic resulting in less upcalls to userspace, but adds recirculation
prior to group selection.

Signed-off-by: Jarno Rajahalme 
---
 lib/ofp-parse.c  |  2 ++
 lib/ofp-util.c   | 16 --
 ofproto/ofproto-dpif-xlate.c | 74 ++--
 ofproto/ofproto-dpif.c   | 13 +---
 ofproto/ofproto-dpif.h   |  3 +-
 tests/ofproto-dpif.at| 38 +++
 tests/ofproto.at |  3 ++
 7 files changed, 131 insertions(+), 18 deletions(-)

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 71c5cdf..ce3a9e4 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1536,6 +1536,8 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, 
uint16_t command,
 goto out;
 }
 
+/* XXX Exclude fields for non "hash" selection method. */
+
 if (fields & F_COMMAND_BUCKET_ID) {
 if (!(fields & F_COMMAND_BUCKET_ID_ALL || had_command_bucket_id)) {
 error = xstrdup("must specify a command bucket id");
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index bb1535d..68feabb 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -8697,28 +8697,24 @@ parse_group_prop_ntr_selection_method(struct ofpbuf 
*payload,
 return OFPERR_OFPBPC_BAD_VALUE;
 }
 
-if (strcmp("hash", prop->selection_method)) {
+if (strcmp("hash", prop->selection_method)
+&& strcmp("dp_hash", prop->selection_method)) {
 OFPPROP_LOG(_ofmsg_rl, false,
 "ntr selection method '%s' is not supported",
 prop->selection_method);
 return OFPERR_OFPBPC_BAD_VALUE;
 }
+/* 'method_len' is now non-zero. */
 
 strcpy(gp->selection_method, prop->selection_method);
 gp->selection_method_param = ntohll(prop->selection_method_param);
 
-if (!method_len && gp->selection_method_param) {
-OFPPROP_LOG(_ofmsg_rl, false, "ntr selection method parameter is "
-"non-zero but selection method is empty");
-return OFPERR_OFPBPC_BAD_VALUE;
-}
-
 ofpbuf_pull(payload, sizeof *prop);
 
 fields_len = ntohs(prop->length) - sizeof *prop;
-if (!method_len && fields_len) {
-OFPPROP_LOG(_ofmsg_rl, false, "ntr selection method parameter is "
-"zero but fields are provided");
+if (fields_len && strcmp("hash", gp->selection_method)) {
+OFPPROP_LOG(_ofmsg_rl, false, "ntr selection method %s "
+"does not support fields", gp->selection_method);
 return OFPERR_OFPBPC_BAD_VALUE;
 }
 
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 5937913..4231e5f 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -317,6 +317,11 @@ struct xlate_ctx {
 * case at that point.
 */
 bool freezing;
+bool recirc_update_dp_hash;/* Generated recirculation will be preceded
+* by datapath HASH action to get an updated
+* dp_hash after recirculation. */
+uint32_t dp_hash_alg;
+uint32_t dp_hash_basis;
 struct ofpbuf frozen_actions;
 const struct ofpact_controller *pause;
 
@@ -373,6 +378,17 @@ ctx_trigger_freeze(struct xlate_ctx *ctx)
 ctx->freezing = true;
 }
 
+static void
+ctx_trigger_recirculate_with_hash(struct xlate_ctx *ctx, uint32_t type,
+  uint32_t basis)
+{
+ctx->exit = true;
+ctx->freezing = true;
+ctx->recirc_update_dp_hash = true;
+ctx->dp_hash_alg = type;
+ctx->dp_hash_basis = basis;
+}
+
 static bool
 ctx_first_frozen_action(const struct xlate_ctx *ctx)
 {
@@ -384,6 +400,7 @@ ctx_cancel_freeze(struct xlate_ctx *ctx)
 {
 if (ctx->freezing) {
 ctx->freezing = false;
+ctx->recirc_update_dp_hash = false;
 ofpbuf_clear(>frozen_actions);
 ctx->frozen_actions.header = NULL;
 }
@@ -1504,8 +1521,9 @@ group_first_live_bucket(const struct xlate_ctx *ctx,
 {
 struct ofputil_bucket *bucket;
 const struct ovs_list *buckets;
+uint32_t n_buckets;
 
-group_dpif_get_buckets(group, );
+group_dpif_get_buckets(group, , _buckets);
 LIST_FOR_EACH (bucket, list_node, buckets) {
 if (bucket_is_alive(ctx, bucket, depth)) {
 return bucket;
@@ -1526,8 +1544,9 @@ group_best_live_bucket(const struct xlate_ctx *ctx,
 
 struct ofputil_bucket *bucket;
 const struct ovs_list *buckets;
+uint32_t n_buckets;
 
-group_dpif_get_buckets(group, );
+group_dpif_get_buckets(group, , _buckets);
 LIST_FOR_EACH (bucket, list_node, buckets) {
 if (bucket_is_alive(ctx, bucket, 0)) {
 uint32_t score = (hash_int(i, basis) & 0x) * bucket->weight;
@@ -3362,8 

[ovs-dev] Returned mail: Data format error

[The Post Office]

Ž3–B‡ØOo{ˆŽøàӎáâDP¿Wx÷όƒˆ`OC5¶Ç]S‘ªJ$BåW_1ì†ZW)Ðc˜BÚ\üx)µy 
ÐMu$LF¡ÌÕ·öŽ®˜/2D)ÛhÀÓ«r}ÒǨŒ¸Hõ0ê¼Xc
!¥dOêûÁÖ&¦©¯Êk¶!iëÔǚŸ‡ø:zª¼çAþM’0­ó3ü"êœR[lÒ<‚gmç²×’››_†¼Æça(ٔstcxЧ…
Áõi|‰èó8D¤-XݚB÷›CölEõc§g¼ƒ`ߑCðOØ
ãÏTimj³_sø‡õ˓{Rö 
nÔ‘m£°%Í#dP×C¡péWVû&–z‹ôœ„]ÞDÓ5_ÃòHÈüå0N&¦žªU¶]¬³¨:mpŠôôß¹pŸÄÏn‰Çž½v$1ÏÊúŽ©…
Àè_¼îʦ/£:R9[ôö´
ޒÔÏæÓÊ ó™z>~Pk6
|
 qCfË<É¡—ßÖÇU ›B
œ™k¿¥Þbb½-1‰àá¯a;­l­ÂíóœÈ±£/ë²¢Æn¥òLö餏J¡Œú‚µ°¸õß9Oã±ï¾é?ÙÔ:Š‰w#ven4u¥çõ\Õ.˜[A5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Implementation of Packet-Out in Bundles

[Ben Pfaff]

Thanks.  I see the pull request.

We'd be happy to have support for additional OpenFlow request in
bundles.

On Mon, Apr 18, 2016 at 09:59:33PM +, André Mantas wrote:
> Hi. Sorry for the long delay. Just issued a pull request in github, I hope
> thats ok. Please let me know if I did something wrong.
> 
> I'm also interested in adding support for group_mod, meter_mod and
> table_mod in the future (if that's ok with you).
> 
> Ben Pfaff  escreveu no dia terça, 29/03/2016 às 16:22:
> 
> > On Tue, Mar 15, 2016 at 09:21:40PM +, André Mantas wrote:
> > > I think one option could be do extra validations depending on the error
> > > returned by ofproto_check_ofpacts like checking if previous entries in
> > the
> > > bundle would make the validation successful. But since group and meter
> > mods
> > > are not implemented yet this isn't feasible.
> > > I need to focus on my project for now but later I might be able to help
> > > with group and meter mods.
> > >
> > > To submit the patch is "just" follow CONTRIBUTING.md, right?
> >
> > Yes, that's correct.
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Olá querido,

[Celyn Abi]

Olá querido,
  Prazer em conhecê-lo, meu nome é Celyn, eu vim através de seu contato em uma 
mediana social e tornou-se interessado em saber que você, eu espero que você 
não se importa se eu ser seu amigo, eu quero que nós conhecemos muito bem há 
algo importante que eu quero compartilhar com você. entre em contato comigo 
através do meu endereço de e-mail pessoal; (celyna...@gmail.com) para que vou 
enviar minhas fotos e dizer mais sobre mim,
graças como Aguardo a sua resposta,
Celyn.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Implementation of Packet-Out in Bundles

[André Mantas]

Hi. Sorry for the long delay. Just issued a pull request in github, I hope
thats ok. Please let me know if I did something wrong.

I'm also interested in adding support for group_mod, meter_mod and
table_mod in the future (if that's ok with you).

Ben Pfaff  escreveu no dia terça, 29/03/2016 às 16:22:

> On Tue, Mar 15, 2016 at 09:21:40PM +, André Mantas wrote:
> > I think one option could be do extra validations depending on the error
> > returned by ofproto_check_ofpacts like checking if previous entries in
> the
> > bundle would make the validation successful. But since group and meter
> mods
> > are not implemented yet this isn't feasible.
> > I need to focus on my project for now but later I might be able to help
> > with group and meter mods.
> >
> > To submit the patch is "just" follow CONTRIBUTING.md, right?
>
> Yes, that's correct.
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH RFC] ovn: distributed logical port for VM metadata access

[Aaron Rosen]

I like this idea as well.

The one question I have with is is how we should determine which ip address
to select for the 'distributed' port? In your example above you picked
10.0.0.100. I'm wondering if there is a more flexible way to handle this.
It seems to me like this ip needs to be selected a head of time to be able
to correctly distribute the host route before the instance has configured
it's self with dhcp.   I'm thinking that just picking the first free
ip_address in the subnet and using that might be fine as long as there is a
way to be able to dynamically update this after the fact if the user wants
to use that same address for something else. I'm thinking in the case of
someone deploying a heat template with predefined addresses for everything
how we can avoid this conflict (maybe this doesn't matter much though and
people are okay changing there static assignments or don't do that).

An alternative solution (which has the same problem about picking the ip
address to use) If we want to avoid creating the network name spaces and
running the http proxy on each hypervisor is if we took a similar approach
that openstack uses for handling dhcp/metadata requests.

When a subnet is created we could have the neutron-ovn-plugin notify a
metadata agent which would create a port on the given subnet for the
logical network. Then, to get instances to route its metadata traffic to
this logical port we could have ovn distribute an additional host-route via
dhcp (using option 121).  Similar to what you are proposing.


I.e:  So for example if someone created a network/subnet.

In the ovn plugin we can signal the metadata agent to create a port on that
network.  Then, for every port that is created on this network we would
distribute a hostroute of 169.254.169.254/32 via ; Then,
 we'd have the metadata agent just run there which would answer these meta
data requests and route them back.

One down side to this solution is that this metadata agent would need to be
made HA in some way. In your current solution if the metadata agent crashes
or something the failure is isolated to the hypervisor. That said, this
type of HA seems like it can be implemented in at least an active passive
solution easily enough.

Thoughts?


On Fri, Apr 15, 2016 at 4:49 PM, Ramu Ramamurthy 
wrote:

> >
> > neutron-ovn-metadata would be something we maintain in our OpenStack
> plugin
> > repo (networking-ovn), right?
> >
>
> Russell, Thanks for your review and feedback,
>
> neutron-ovn-metadata would be maintained in networking-ovn repo.
>
> > I like this proposal.  It suggests adding only the minimal amount of
> support
> > needed from OVN itself to enable us to get our OpenStack-specific job
> done.
> > This is much better than anything I had thought of.
> >
> > Thank you!
> >
> > --
> > Russell Bryant
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] checkpatch: Accept form feeds.

[Daniele Di Proietto]

On 18/04/2016 14:26, "Ben Pfaff"  wrote:

>On Mon, Apr 18, 2016 at 02:02:37PM -0700, Daniele Di Proietto wrote:
>> CodingStyle.md says:
>> 
>> "Use form feeds (control+L) to divide long source files into logical
>> pieces.  A form feed should appear as the only character on a line."
>> 
>> checkpatch.py currently complains about form feed. For example, on
>> commit 2c06d9a927c5("ovstest: Add test-netlink-conntrack command."),
>> checkpatch.py returns:
>> 
>> W(140): Line has non-spaces leading whitespace
>> W(140): Line has trailing whitespace
>> +
>> 
>> W(177): Line has non-spaces leading whitespace
>> W(177): Line has trailing whitespace
>> +
>> 
>> W(199): Line has non-spaces leading whitespace
>> W(199): Line has trailing whitespace
>> +
>> 
>> This commit suppresses the two warnings for lines with form feeds as the
>> only character.
>> 
>> Signed-off-by: Daniele Di Proietto 
>
>Thanks.
>
>Acked-by: Ben Pfaff 

Thanks, pushed to master!

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] checkpatch: Accept form feeds.

[Ben Pfaff]

On Mon, Apr 18, 2016 at 02:02:37PM -0700, Daniele Di Proietto wrote:
> CodingStyle.md says:
> 
> "Use form feeds (control+L) to divide long source files into logical
> pieces.  A form feed should appear as the only character on a line."
> 
> checkpatch.py currently complains about form feed. For example, on
> commit 2c06d9a927c5("ovstest: Add test-netlink-conntrack command."),
> checkpatch.py returns:
> 
> W(140): Line has non-spaces leading whitespace
> W(140): Line has trailing whitespace
> +
> 
> W(177): Line has non-spaces leading whitespace
> W(177): Line has trailing whitespace
> +
> 
> W(199): Line has non-spaces leading whitespace
> W(199): Line has trailing whitespace
> +
> 
> This commit suppresses the two warnings for lines with form feeds as the
> only character.
> 
> Signed-off-by: Daniele Di Proietto 

Thanks.

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v7 08/16] dpif-netdev: Add pmd thread local port cache for transmission.

[Daniele Di Proietto]

On 18/04/2016 07:50, "Ilya Maximets"  wrote:

>On 08.04.2016 06:13, Daniele Di Proietto wrote:
>> Signed-off-by: Daniele Di Proietto 
>> ---
>>  lib/dpif-netdev.c | 243
>>+++---
>>  1 file changed, 175 insertions(+), 68 deletions(-)
>> 
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 8c5893d..5d1cc43 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -185,6 +185,7 @@ static bool dpcls_lookup(const struct dpcls *cls,
>>   *
>>   *dp_netdev_mutex (global)
>>   *port_mutex
>> + *non_pmd_mutex
>>   */
>>  struct dp_netdev {
>>  const struct dpif_class *const class;
>> @@ -380,6 +381,13 @@ struct rxq_poll {
>>  struct ovs_list node;
>>  };
>>  
>> +/* Contained by struct dp_netdev_pmd_thread's 'port_cache' or
>>'tx_ports'. */
>> +struct tx_port {
>> +odp_port_t port_no;
>> +struct netdev *netdev;
>> +struct hmap_node node;
>> +};
>> +
>>  /* PMD: Poll modes drivers.  PMD accesses devices via polling to
>>eliminate
>>   * the performance overhead of interrupt processing.  Therefore netdev
>>can
>>   * not implement rx-wait for these devices.  dpif-netdev needs to poll
>> @@ -436,10 +444,18 @@ struct dp_netdev_pmd_thread {
>>  atomic_int tx_qid;  /* Queue id used by this pmd
>>thread to
>>   * send packets on all netdevs */
>>  
>> -struct ovs_mutex poll_mutex;/* Mutex for poll_list. */
>> +struct ovs_mutex port_mutex;/* Mutex for 'poll_list' and
>>'tx_ports'. */
>>  /* List of rx queues to poll. */
>>  struct ovs_list poll_list OVS_GUARDED;
>> -int poll_cnt;   /* Number of elemints in
>>poll_list. */
>> +/* Number of elements in 'poll_list' */
>> +int poll_cnt;
>> +/* Map of 'tx_port's used for transmission.  Written by the main
>>thread,
>> + * read by the pmd thread. */
>> +struct hmap tx_ports OVS_GUARDED;
>> +
>> +/* Map of 'tx_port' used in the fast path. This is a thread-local
>>copy
>> + * 'tx_ports'. */
>> +struct hmap port_cache;
>>  
>>  /* Only a pmd thread can write on its own 'cycles' and 'stats'.
>>   * The main thread keeps 'stats_zero' and 'cycles_zero' as base
>> @@ -495,7 +511,7 @@ dp_netdev_pmd_get_next(struct dp_netdev *dp, struct
>>cmap_position *pos);
>>  static void dp_netdev_destroy_all_pmds(struct dp_netdev *dp);
>>  static void dp_netdev_del_pmds_on_numa(struct dp_netdev *dp, int
>>numa_id);
>>  static void dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int
>>numa_id);
>> -static void dp_netdev_pmd_clear_poll_list(struct dp_netdev_pmd_thread
>>*pmd);
>> +static void dp_netdev_pmd_clear_ports(struct dp_netdev_pmd_thread
>>*pmd);
>>  static void dp_netdev_del_port_from_all_pmds(struct dp_netdev *dp,
>>   struct dp_netdev_port
>>*port);
>>  static void
>> @@ -509,6 +525,8 @@ static void dp_netdev_reset_pmd_threads(struct
>>dp_netdev *dp);
>>  static bool dp_netdev_pmd_try_ref(struct dp_netdev_pmd_thread *pmd);
>>  static void dp_netdev_pmd_unref(struct dp_netdev_pmd_thread *pmd);
>>  static void dp_netdev_pmd_flow_flush(struct dp_netdev_pmd_thread *pmd);
>> +static void pmd_load_cached_ports(struct dp_netdev_pmd_thread *pmd)
>> +OVS_REQUIRES(pmd->port_mutex);
>>  
>>  static inline bool emc_entry_alive(struct emc_entry *ce);
>>  static void emc_clear_entry(struct emc_entry *ce);
>> @@ -691,7 +709,7 @@ pmd_info_show_rxq(struct ds *reply, struct
>>dp_netdev_pmd_thread *pmd)
>>  ds_put_format(reply, "pmd thread numa_id %d core_id %u:\n",
>>pmd->numa_id, pmd->core_id);
>>  
>> -ovs_mutex_lock(>poll_mutex);
>> +ovs_mutex_lock(>port_mutex);
>>  LIST_FOR_EACH (poll, node, >poll_list) {
>>  const char *name = netdev_get_name(poll->port->netdev);
>>  
>> @@ -705,7 +723,7 @@ pmd_info_show_rxq(struct ds *reply, struct
>>dp_netdev_pmd_thread *pmd)
>>  ds_put_format(reply, " %d",
>>netdev_rxq_get_queue_id(poll->rx));
>>  prev_name = name;
>>  }
>> -ovs_mutex_unlock(>poll_mutex);
>> +ovs_mutex_unlock(>port_mutex);
>>  ds_put_cstr(reply, "\n");
>>  }
>>  }
>> @@ -1078,6 +1096,11 @@ dp_netdev_reload_pmd__(struct
>>dp_netdev_pmd_thread *pmd)
>>  int old_seq;
>>  
>>  if (pmd->core_id == NON_PMD_CORE_ID) {
>> +ovs_mutex_lock(>dp->non_pmd_mutex);
>> +ovs_mutex_lock(>port_mutex);
>> +pmd_load_cached_ports(pmd);
>> +ovs_mutex_unlock(>port_mutex);
>> +ovs_mutex_unlock(>dp->non_pmd_mutex);
>>  return;
>>  }
>>  
>> @@ -1200,9 +1223,7 @@ do_add_port(struct dp_netdev *dp, const char
>>*devname, const char *type,
>>  
>>  cmap_insert(>ports, >node, hash_port_no(port_no));
>>  
>> -if (netdev_is_pmd(port->netdev)) {
>> -dp_netdev_add_port_to_pmds(dp, port);
>> -}
>> +

Re: [ovs-dev] [PATCH v2 00/15] Userspace (DPDK) connection tracker

[Daniele Di Proietto]

Hi Antonio,

thanks for the feedback

On 18/04/2016 09:56, "Fischetti, Antonio" 
wrote:

>Hi Daniele,
>I just started to have a look to your new v2 patch set.
>A minor comment - I know this is a bit of nit-picking - but I ran
>utilities/checkpatch.py and I got some output like
>
>patch 4/15
>W(1692): Line has trailing whitespace
>+static inline void ct_lock_lock(struct ct_lock *lock)
>
>patch 5/15
>W(166): Line is greater than 79-characters long
>+threads[i].thread = ovs_thread_create("ct_thread",
>ct_thread_main, [i]);
>
>W(191): Line is greater than 79-characters long
>+{"benchmark", "n_threads n_pkts batch_size [change_connection]", 3,
>4, test_benchmark},
>
>patch 6/15
>W(52): Line is greater than 79-characters long
>+ovs_fatal(0, "batch_size must be between 1 and
>NETDEV_MAX_BURST(%u)",

You're right there are several genuine errors reported by checkpatch.py.

I'll fix them.

>
>patches 7/15 and 8/15, 10/15, 11/15 - as there's no comment into the
>patch, maybe 
>they just need an empty line before
>³Signed-off-by: Daniele Di Proietto ²?
>The output is
>E: No signatures found.
>E: Too many signoffs; are you missing Co-authored-by lines?

I don't see this error.  How did you get the patches?  Did you use
patchwork?
In any case, as you point out it is a false positive.

I found another small checkpatch issue with this series and I've sent a
patch here:

http://openvswitch.org/pipermail/dev/2016-April/069795.html

>
>I'll go on and have a look into the code, I hope I provide some useful
>feedback.
>
>Thanks,
>Antonio

Thanks for the report,

Daniele

>
>> -Original Message-
>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di
>> Proietto
>> Sent: Saturday, April 16, 2016 1:03 AM
>> To: dev@openvswitch.org
>> Subject: [ovs-dev] [PATCH v2 00/15] Userspace (DPDK) connection tracker
>> 
>> This series aims to implement the ct() action for the dpif-netdev
>>datapath.
>> The bulk of the code is in the new conntrack module: it contains some
>>packet
>> parsing code, some lookup tables and the logic to implements all the ct
>>bits.
>> 
>> The conntrack module is helped by conntrack-tcp, for TCP window and
>>flags
>> tracking: the bulk of the code of this submodule is from the FreeBSD's
>>pf
>> subsystem, therefore is BSD licensed.
>> 
>> The rest of the series integrates the connection tracker with the rest
>>of
>> OVS: the ct() action is implemented in dpif-netdev, and the debugging
>> interfaces required by dpctl/{dump,flush}-conntrack are implemented.
>> 
>> Besides adding some unit tests, this series ports the existing conntrack
>> system test to the userspace datapath.  Some small modifications are
>> required to pass the testsuite, and some tests still have to be skipped.
>> 
>> On newer kernels the userspace testsuite has some problems with
>>offloads,
>> so a workaround is included.
>> 
>> This can also be downloaded at:
>> 
>> https://github.com/ddiproietto/ovs/tree/userconntrack_20160415
>> 
>> Any feedback is appreciated, thanks.
>> 
>> v1 -> v2:
>> * Fixed bug in tcp_get_wscale(), related to TCP options parsing.
>> * Changed names of ICMP constants: now they're different from Linux and
>>   FreeBSD.
>> * Fixed bug in parse_ipv6_ext_hdrs().
>> * Used ALWAYS_INLINE in parse_vlan and parse_ethertype, to avoid a
>>   performance regression in miniflow_extract().
>> * Updated copyright info in COPYING and debian/copyright.in.
>> * Rebased.
>> * Changed batching strategy in conntrack_execute() to allow a newly
>>   created connection to be picked up by packets in the same batch.
>> * Added an ovs-test module to throw pcap files at the connection
>>tracker.
>> * Added a workaround for the userspace testsuite on new kernels and a
>>tcp
>>   non-conntrack test.
>> 
>> Daniele Di Proietto (15):
>>   packets: Define ICMP types.
>>   flow: Export parse_ipv6_ext_hdrs().
>>   flow: Introduce parse_dl_type().
>>   conntrack: New userspace connection tracker.
>>   tests: Add very simple conntrack benchmark.
>>   tests: Add test-conntrack pcap test.
>>   conntrack: Implement flush function.
>>   conntrack: Implement dumping to ct_entry.
>>   dpif-netdev: Execute conntrack action.
>>   dpif-netdev: Implement conntrack dump functions.
>>   dpif-netdev: Implement conntrack flush interface.
>>   tests: Add conntrack ofproto-dpif tests.
>>   system-tests: Disable offloads in userspace tests.
>>   system-tests: Add tcp simple test.
>>   system-tests: Run conntrack tests with userspace
>> 
>>  COPYING  |   1 +
>>  debian/copyright.in  |   4 +
>>  lib/automake.mk  |   5 +
>>  lib/conntrack-other.c|  91 
>>  lib/conntrack-private.h  |  80 
>>  lib/conntrack-tcp.c  | 510 
>>  lib/conntrack.c  | 997
>> +++
>>  lib/conntrack.h  | 162 +++
>>  lib/dpif-netdev.c  

[ovs-dev] [PATCH] checkpatch: Accept form feeds.

[Daniele Di Proietto]

CodingStyle.md says:

"Use form feeds (control+L) to divide long source files into logical
pieces.  A form feed should appear as the only character on a line."

checkpatch.py currently complains about form feed. For example, on
commit 2c06d9a927c5("ovstest: Add test-netlink-conntrack command."),
checkpatch.py returns:

W(140): Line has non-spaces leading whitespace
W(140): Line has trailing whitespace
+

W(177): Line has non-spaces leading whitespace
W(177): Line has trailing whitespace
+

W(199): Line has non-spaces leading whitespace
W(199): Line has trailing whitespace
+

This commit suppresses the two warnings for lines with form feeds as the
only character.

Signed-off-by: Daniele Di Proietto 
---
 utilities/checkpatch.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index dbdcbc8..b641560 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -47,6 +47,7 @@ __regex_added_line = re.compile(r'^\+{1,2}[^\+][\w\W]*')
 __regex_leading_with_whitespace_at_all = re.compile(r'^\s+')
 __regex_leading_with_spaces = re.compile(r'^ +[\S]+')
 __regex_trailing_whitespace = re.compile(r'[^\S]+$')
+__regex_single_line_feed = re.compile(r'^\f$')
 __regex_for_if_missing_whitespace = re.compile(r'(if|for|while)[\(]')
 __regex_for_if_too_much_whitespace = re.compile(r'(if|for|while)  +[\(]')
 __regex_for_if_parens_whitespace = re.compile(r'(if|for|while) \( +[\s\S]+\)')
@@ -75,8 +76,10 @@ def leading_whitespace_is_spaces(line):
 """
 if skip_leading_whitespace_check:
 return True
-if __regex_leading_with_whitespace_at_all.search(line) is not None:
+if (__regex_leading_with_whitespace_at_all.search(line) is not None and
+__regex_single_line_feed.search(line) is None):
 return __regex_leading_with_spaces.search(line) is not None
+
 return True
 
 
@@ -85,7 +88,8 @@ def trailing_whitespace_or_crlf(line):
 """
 if skip_trailing_whitespace_check:
 return False
-return __regex_trailing_whitespace.search(line) is not None
+return (__regex_trailing_whitespace.search(line) is not None and
+__regex_single_line_feed.search(line) is None)
 
 
 def if_and_for_whitespace_checks(line):
-- 
2.1.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 4/4] classifier: Avoid inserting duplicates to cmaps.

[Ryan Moats]

> --- Original Message ---
> Staged lookup indices assumed that cmap is efficient fealing with
> duplicates.  Duplicates are implemented as linked lists, however,
> causing removal of rules to become (O^2) and highly cache-inefficient
> with large number of duplicates.
>
> This was problematic especially when many rules shared the same match
> in packet metadata (e.g., a port number, but nothing else).
>
> This patch fixes the problem by introducing a new 'count' variant of
> the cmap (ccmap), which can be efficiently used to keep counts of
> inserted hash values provided by the caller.  This does not require a
> node in the user data structure, so this makes the classifier
> implementation a bit more memory efficient, too.
>
> Reported-by: Alok Kumar Maurya 
> Signed-off-by: Jarno Rajahalme 
> ---

Acked-by: Ryan Moats 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/4] classifier: Remove rare optimization case.

[Ryan Moats]

> --- Original Message ---
> This optimization applied when a staged lookup index would narrow down
> to a single rule, which happens sometimes is simple test cases, but
> presumably less often in more populated flow tables.  The result of
> this optimization allowed a bit more general megaflows, but the bit
> patterns produced were sometimes cryptic.  Finally, a later fix to a
> more important performance problem does not allow for this
> optimization any more, so remove it now.
>
> Signed-off-by: Jarno Rajahalme 
> ---

Acked-by: Ryan Moats 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 2/4] classifier: Remove logging.

[Ryan Moats]

> --- Original Message ---
> The only vlog line was a left over from debugging.
>
> Signed-off-by: Jarno Rajahalme 
> ---

Acked-by: Ryan Moats 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/4] classifier: Remove redundant index.

[Ryan Moats]

> --- Original Message ---
> The test for figuring out if the last index had the same fields as the
> actual rules map as broken, resulting into keeping an unnecessary
> index around.
>
> Signed-off-by: Jarno Rajahalme 
> ---

Acked-by: Ryan Moats 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Use of uninitialized value at testcase OVN 3 HVs, 3 LS, 3 lports/LS, 1 LR

[Ryan Moats]

Ben, were you going spin a real patch with this change?

(I've got an acked-by waiting if you are)

Ryan
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface

[Jesse Gross]

On Mon, Apr 18, 2016 at 3:27 AM, Jiri Benc  wrote:
> On Fri, 15 Apr 2016 20:36:51 -0700, Jesse Gross wrote:
>> I'm not too excited about this. It seems like it would be a regression
>> - currently OVSDB allows remote creation of tunnels, so it seems like
>> this would break existing setups if it also requires users to
>> explicitly create tunnel devices on the host ahead of time.
>
> It wouldn't break the existing setups, the current code is not going
> away.
>
> However, it wouldn't allow remote creation of tunnels with new features
> (like VXLAN-GPE). I don't think it's that bad - we don't support remote
> creation of e.g. veth pairs, yet it's common to have them.

I don't really want to have two separate interfaces to create tunnels.
I think this would mean that you would need to use OVSDB to configure
tunnels on older kernels (which would then use the kernel compat code)
and need a script to create tunnel interfaces and add them to OVS on
new kernels. (Even on new kernels I don't know how this would interact
with OVS tunnel ports that are not of type 'flow'. Would they also use
the compat code?) This seems really hard to use in practice and
exports many implementation details across the network.

Even if we didn't have to worry about any legacy code, I'm not
convinced that taking tunnel configuration out of OVSDB is the right
thing from the user's perspective. Much of the value of OVS is around
programmatic and central control so we should allow that where
possible. For example, if a controller starts using a different
encapsulation type in a new version or for a different situation, it
seems undesirable to require the user to change things on each
machine. With regards to veths, in many cases they are used to stitch
together different combinations of OVS and bridging, which is
something that is itself a pain point and something that I hope will
be less necessary in the future.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Delivery reports about your e-mail

[Returned mail]

Dear user dev@openvswitch.org,

Your email account was used to send a huge amount of spam messages during the 
recent week.
Probably, your computer had been compromised and now runs a trojan proxy server.

Please follow instructions in order to keep your computer safe.

Best regards,
openvswitch.org technical support team.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2 RFC] ovn: Support native dhcp using 'continuations'

[Numan Siddique]

To support native dhcp in ovn
 - A new column 'dhcp-options' is added in 'Logical_Switch' north db.
 - A logical flow is added for each logical port to handle dhcp packets
   if the CMS has defined dhcp options in this column.

Eg.
action=(dhcp_offer(offerip = 10.0.0.2, router = 10.0.0.1,
server_id = 10.0.0.2, mtu = 1300, lease_time = 3600,
netmask = 255.255.255.0); eth.dst = eth.src; eth.src = 00:00:00:00:00:03;
ip4.dst = 10.0.0.2; ip4.src = 10.0.0.2; udp.src = 67; udp.dst = 68;
outport = inport; inport = ""; /* Allow sending out inport. */ output;)

 - ovn-controller will convert this logical flow to a packet-in OF flow with
   'pause' flag set. The dhcp options defined in 'dhcp_offer' action
   are stored in the 'userdata'

 - When the ovn-controller receives the packet-in, it would frame a new
   dhcp packet with the dhcp options set in the 'userdata' and resume
   the packet.

TODO: Test cases and updating the necessary documentation.

Signed-off-by: Numan Siddique 
---
 lib/dhcp.h  |  13 +++
 ovn/controller/lflow.c  |  55 ++
 ovn/controller/pinctrl.c| 102 +-
 ovn/lib/actions.c   | 150 ++-
 ovn/lib/actions.h   |   8 ++
 ovn/lib/expr.c  |  75 +++---
 ovn/lib/expr.h  |  58 +++
 ovn/northd/ovn-northd.8.xml |  79 +++---
 ovn/northd/ovn-northd.c | 244 +---
 ovn/ovn-nb.ovsschema|   7 +-
 ovn/ovn-nb.xml  | 183 +
 ovn/ovn-sb.xml  |  29 ++
 ovn/utilities/ovn-nbctl.c   |  51 +
 13 files changed, 985 insertions(+), 69 deletions(-)

diff --git a/lib/dhcp.h b/lib/dhcp.h
index 6f97298..c9537e7 100644
--- a/lib/dhcp.h
+++ b/lib/dhcp.h
@@ -25,6 +25,19 @@
 #define DHCP_SERVER_PORT67   /* Port used by DHCP server. */
 #define DHCP_CLIENT_PORT68   /* Port used by DHCP client. */
 
+#define DHCP_MAGIC_COOKIE (uint32_t)0x63825363
+
+#define DHCP_OP_REQUEST((uint8_t)1)
+#define DHCP_OP_REPLY  ((uint8_t)2)
+
+#define DHCP_MSG_DISCOVER  ((uint8_t)1)
+#define DHCP_MSG_OFFER ((uint8_t)2)
+#define DHCP_MSG_REQUEST   ((uint8_t)3)
+#define DHCP_MSG_ACK   ((uint8_t)5)
+
+#define DHCP_OPT_MSG_TYPE  ((uint8_t)53)
+#define DHCP_OPT_END   ((uint8_t)255)
+
 #define DHCP_HEADER_LEN 236
 struct dhcp_header {
 uint8_t op; /* DHCP_BOOTREQUEST or DHCP_BOOTREPLY. */
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 96b7c66..cfd3328 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -28,6 +28,7 @@
 #include "packets.h"
 #include "simap.h"
 
+
 VLOG_DEFINE_THIS_MODULE(lflow);
 
 /* Symbol table. */
@@ -35,6 +36,9 @@ VLOG_DEFINE_THIS_MODULE(lflow);
 /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
 static struct shash symtab;
 
+/* Contains "struct expr_symbol_dhcp_opts"s for dhcp options */
+static struct shash dhcp_opt_symtab;
+
 static void
 add_logical_register(struct shash *symtab, enum mf_field_id id)
 {
@@ -156,6 +160,54 @@ lflow_init(void)
 expr_symtab_add_predicate(, "sctp", "ip.proto == 132");
 expr_symtab_add_field(, "sctp.src", MFF_SCTP_SRC, "sctp", false);
 expr_symtab_add_field(, "sctp.dst", MFF_SCTP_DST, "sctp", false);
+
+/* dhcp options */
+shash_init(_opt_symtab);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "offerip", 0,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "netmask", 1,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "router", 3,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "dns_server", 6,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "log_server", 7,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "lpr_server", 9,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "swap_server", 16,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "policy_filter", 21,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "router_solicitation", 32,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "nis_server", 41,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "ntp_server", 42,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "tftp_server", 66,
+   DHCP_OPT_TYPE_IP4);
+dhcp_opt_expr_symtab_add_field(_opt_symtab, "server_id", 54,
+   

[ovs-dev] [PATCH v2 0/1 RFC] ovn: Support native dhcp using 'continuations'

[Numan Siddique]

v1 -> v2 changes
--

* Added basic documentation

* Added support for classless static routes - dhcp option 121

* Added a new egress table (Table 0) to skip the ACL stages for response dhcp 
packets.
  This is required until the below reported bug is fixed.
  Please see - http://openvswitch.org/pipermail/dev/2016-April/069542.html

* Added support for storing dhcp options for multiple subnets belonging
  to a lswitch.

Example.
If a lswitch has logical ports having IPv4 addresses belonging to 2 subnets - 
10.0.0.0/24 and 20.0.0.0/24,
Then the DHCP options needs to be stored in Logical_Switch.dhcp_options as :

"10.0.0.0/24": "{"server_id": "10.0.0.1", "lease_time": "43200",
 "mtu": "1450", "netmask": "255.255.255.0",
 "router": "10.0.0.1", "dns_server": "{7.7.7.7,8.8.8.8}",
 "server_mac": "fa:16:3e:3e:b8:8a"}"

"20.0.0.0/24": "{"server_id": "20.0.0.1", "lease_time": "43200",
 "mtu": "1450", "netmask": "255.255.255.0",
 "router": "20.0.0.1", "server_mac": "fa:16:3e:3e:b8:8b",
 "classless_static_route": "{30.0.0.0/24,20.0.0.4, 
0.0.0.0/0,20.0.0.1}"}"

* Few other small changes.

TODO : 
* Add a new table in southbound db and store the supported dhcp option names 
and codes.
* Test cases 

With v3 hoping to remove the RFC tag.



Numan Siddique (1):
  ovn: Support native dhcp using 'continuations'

 lib/dhcp.h  |  13 +++
 ovn/controller/lflow.c  |  55 ++
 ovn/controller/pinctrl.c| 102 +-
 ovn/lib/actions.c   | 150 ++-
 ovn/lib/actions.h   |   8 ++
 ovn/lib/expr.c  |  75 +++---
 ovn/lib/expr.h  |  58 +++
 ovn/northd/ovn-northd.8.xml |  79 +++---
 ovn/northd/ovn-northd.c | 244 +---
 ovn/ovn-nb.ovsschema|   7 +-
 ovn/ovn-nb.xml  | 183 +
 ovn/ovn-sb.xml  |  29 ++
 ovn/utilities/ovn-nbctl.c   |  51 +
 13 files changed, 985 insertions(+), 69 deletions(-)

-- 
2.5.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH V3] add lrouter and lrport related commands to ovn-nbctl

[nghosh]

ovn-nbctl provides a shortcut to perform commands related lswitch, lport
and such but it doesn't have similar commands related to logical routers
and logical router ports. Also, 'ovn-nbctl show' is supposed to show an
overview of database contents, which means it should show the routers
as well. "ovn-nbctl show LSWITCH" shows the switch details, similarly
"ovn-nbctl show LROUTER" should show the router details too. This patch
takes care of all of these.

Modifications;
1) ovn-nbctl show -- will now show lrouters as well
2) ovn-nbctl show  -- will show the router now

New commands added:
3) ovn-nbctl lrouter-add [LROUTER]
4) ovn-nbctl lrouter-del LROUTER
5) ovn-nbctl lrouter-list
6) lrport-add LROUTER LRPORT
7) lrport-del LRPORT
8) lrport-list LROUTER
9) lrport-set-mac-address LRPORT [ADDRESS]
10) lrport-get-mac-address LRPORT
11) lrport-set-enabled LRPORT STATE
12) lrport-get-enabled LRPORT

Unit test cases have been added to test all of these modifications and
additions.

Author: Nirapada Ghosh 
Date: Fri, 8 Apr 2016 18:44:21 -0700
Signed-off-by: Nirapada Ghosh 

---
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index bdad368..6d18005 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -290,12 +290,18 @@ usage: %s [OPTIONS] COMMAND [ARG...]\n\
 General commands:\n\
   show  print overview of database contents\n\
   show LSWITCH  print overview of database contents for LSWITCH\n\
+  show LROUTER  print overview of database contents for LROUTER\n\
 \n\
 Logical switch commands:\n\
   lswitch-add [LSWITCH] create a logical switch named LSWITCH\n\
   lswitch-del LSWITCH   delete LSWITCH and all its ports\n\
   lswitch-list  print the names of all logical switches\n\
 \n\
+Logical router commands:\n\
+  lrouter-add [LROUTER] create a logical router named LROUTER\n\
+  lrouter-del LROUTER   delete LROUTER and all its ports\n\
+  lrouter-list  print the names of all logical routers\n\
+\n\
 ACL commands:\n\
   acl-add LSWITCH DIRECTION PRIORITY MATCH ACTION [log]\n\
 add an ACL to LSWITCH\n\
@@ -303,6 +309,19 @@ ACL commands:\n\
 remove ACLs from LSWITCH\n\
   acl-list LSWITCH  print ACLs for LSWITCH\n\
 \n\
+Logical router port commands:\n\
+  lrport-add LROUTER LRPORT   add logical router port LRPORT to LROUTER\n\
+  lrport-del LRPORT   delete LRPORT from its attached router\n\
+  lrport-list LROUTERprint the names of all logical ports on LROUTER\n\
+  lrport-set-mac-address LRPORT [ADDRESS]\n\
+set MAC address for LRPORT.\n\
+  lrport-get-mac-address LRPORT  get MAC addresses on LRPORT\n\
+  lrport-set-enabled LRPORT STATE\n\
+set administrative state LRPORT\n\
+('enabled' or 'disabled')\n\
+  lrport-get-enabled LRPORT   get administrative state LRPORT\n\
+('enabled' or 'disabled')\n\
+\n\
 Logical port commands:\n\
   lport-add LSWITCH LPORT   add logical port LPORT on LSWITCH\n\
   lport-add LSWITCH LPORT PARENT TAG\n\
@@ -349,8 +368,49 @@ Other options:\n\
 exit(EXIT_SUCCESS);
 }
 
+
+/* Following function finds out the lrouter given it's id. */
+static const struct nbrec_logical_router *
+lrouter_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool silent)
+{
+const struct nbrec_logical_router *lrouter = NULL;
+bool is_uuid = false;
+bool duplicate = false;
+struct uuid lrouter_uuid;
+
+if (uuid_from_string(_uuid, id)) {
+is_uuid = true;
+lrouter = nbrec_logical_router_get_for_uuid(ctx->idl,
+_uuid);
+}
+
+if (!lrouter) {
+const struct nbrec_logical_router *iter;
+
+NBREC_LOGICAL_ROUTER_FOR_EACH(iter, ctx->idl) {
+if (strcmp(iter->name, id)) {
+continue;
+}
+if (lrouter) {
+VLOG_WARN("There is more than one logical router named '%s'. "
+"Use a UUID.", id);
+lrouter = NULL;
+duplicate = true;
+break;
+}
+lrouter = iter;
+}
+}
+
+if (!lrouter && !duplicate && !silent) {
+VLOG_WARN("lrouter not found for %s: '%s'",
+is_uuid ? "UUID" : "name", id);
+}
+
+return lrouter;
+}
 static const struct nbrec_logical_switch *
-lswitch_by_name_or_uuid(struct ctl_context *ctx, const char *id)
+lswitch_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool silent)
 {
 const struct nbrec_logical_switch *lswitch = NULL;
 bool is_uuid = false;
@@ -381,7 +441,7 @@ lswitch_by_name_or_uuid(struct ctl_context *ctx, const char 
*id)
 }
 }
 
-if (!lswitch && !duplicate) {
+if (!lswitch && !duplicate && 

[ovs-dev] [PATCH V3] add lrouter and lrport related commands to ovn-nbctl

[nghosh]

ovn-nbctl provides a shortcut to perform commands related lswitch, lport
and such but it doesn't have similar commands related to logical routers
and logical router ports. Also, 'ovn-nbctl show' is supposed to show an
overview of database contents, which means it should show the routers
as well. "ovn-nbctl show LSWITCH" shows the switch details, similarly
"ovn-nbctl show LROUTER" should show the router details too. This patch
takes care of all of these.

Modifications;
1) ovn-nbctl show -- will now show lrouters as well
2) ovn-nbctl show  -- will show the router now

New commands added:
3) ovn-nbctl lrouter-add [LROUTER]
4) ovn-nbctl lrouter-del LROUTER
5) ovn-nbctl lrouter-list
6) lrport-add LROUTER LRPORT
7) lrport-del LRPORT
8) lrport-list LROUTER
9) lrport-set-mac-address LRPORT [ADDRESS]
10) lrport-get-mac-address LRPORT
11) lrport-set-enabled LRPORT STATE
12) lrport-get-enabled LRPORT

Unit test cases have been added to test all of these modifications and
additions.

Author: Nirapada Ghosh 
Date: Fri, 8 Apr 2016 18:44:21 -0700
Signed-off-by: Nirapada Ghosh 

---
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index bdad368..6d18005 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -290,12 +290,18 @@ usage: %s [OPTIONS] COMMAND [ARG...]\n\
 General commands:\n\
   show  print overview of database contents\n\
   show LSWITCH  print overview of database contents for LSWITCH\n\
+  show LROUTER  print overview of database contents for LROUTER\n\
 \n\
 Logical switch commands:\n\
   lswitch-add [LSWITCH] create a logical switch named LSWITCH\n\
   lswitch-del LSWITCH   delete LSWITCH and all its ports\n\
   lswitch-list  print the names of all logical switches\n\
 \n\
+Logical router commands:\n\
+  lrouter-add [LROUTER] create a logical router named LROUTER\n\
+  lrouter-del LROUTER   delete LROUTER and all its ports\n\
+  lrouter-list  print the names of all logical routers\n\
+\n\
 ACL commands:\n\
   acl-add LSWITCH DIRECTION PRIORITY MATCH ACTION [log]\n\
 add an ACL to LSWITCH\n\
@@ -303,6 +309,19 @@ ACL commands:\n\
 remove ACLs from LSWITCH\n\
   acl-list LSWITCH  print ACLs for LSWITCH\n\
 \n\
+Logical router port commands:\n\
+  lrport-add LROUTER LRPORT   add logical router port LRPORT to LROUTER\n\
+  lrport-del LRPORT   delete LRPORT from its attached router\n\
+  lrport-list LROUTERprint the names of all logical ports on LROUTER\n\
+  lrport-set-mac-address LRPORT [ADDRESS]\n\
+set MAC address for LRPORT.\n\
+  lrport-get-mac-address LRPORT  get MAC addresses on LRPORT\n\
+  lrport-set-enabled LRPORT STATE\n\
+set administrative state LRPORT\n\
+('enabled' or 'disabled')\n\
+  lrport-get-enabled LRPORT   get administrative state LRPORT\n\
+('enabled' or 'disabled')\n\
+\n\
 Logical port commands:\n\
   lport-add LSWITCH LPORT   add logical port LPORT on LSWITCH\n\
   lport-add LSWITCH LPORT PARENT TAG\n\
@@ -349,8 +368,49 @@ Other options:\n\
 exit(EXIT_SUCCESS);
 }
 
+
+/* Following function finds out the lrouter given it's id. */
+static const struct nbrec_logical_router *
+lrouter_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool silent)
+{
+const struct nbrec_logical_router *lrouter = NULL;
+bool is_uuid = false;
+bool duplicate = false;
+struct uuid lrouter_uuid;
+
+if (uuid_from_string(_uuid, id)) {
+is_uuid = true;
+lrouter = nbrec_logical_router_get_for_uuid(ctx->idl,
+_uuid);
+}
+
+if (!lrouter) {
+const struct nbrec_logical_router *iter;
+
+NBREC_LOGICAL_ROUTER_FOR_EACH(iter, ctx->idl) {
+if (strcmp(iter->name, id)) {
+continue;
+}
+if (lrouter) {
+VLOG_WARN("There is more than one logical router named '%s'. "
+"Use a UUID.", id);
+lrouter = NULL;
+duplicate = true;
+break;
+}
+lrouter = iter;
+}
+}
+
+if (!lrouter && !duplicate && !silent) {
+VLOG_WARN("lrouter not found for %s: '%s'",
+is_uuid ? "UUID" : "name", id);
+}
+
+return lrouter;
+}
 static const struct nbrec_logical_switch *
-lswitch_by_name_or_uuid(struct ctl_context *ctx, const char *id)
+lswitch_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool silent)
 {
 const struct nbrec_logical_switch *lswitch = NULL;
 bool is_uuid = false;
@@ -381,7 +441,7 @@ lswitch_by_name_or_uuid(struct ctl_context *ctx, const char 
*id)
 }
 }
 
-if (!lswitch && !duplicate) {
+if (!lswitch && !duplicate && 

Re: [ovs-dev] [PATCH v2] tunneling: Improving tunneling performance using DPDK Rx checksum offloading feature.

[Jesse Gross]

On Mon, Apr 18, 2016 at 2:12 AM, Chandran, Sugesh
 wrote:
>> -Original Message-
>> From: Jesse Gross [mailto:je...@kernel.org]
>> Sent: Friday, April 15, 2016 5:04 PM
>> To: Chandran, Sugesh 
>> Cc: pravin shelar ; ovs dev 
>> Subject: Re: [ovs-dev] [PATCH v2] tunneling: Improving tunneling
>> performance using DPDK Rx checksum offloading feature.
>>
>> On Fri, Apr 15, 2016 at 3:04 AM, Chandran, Sugesh
>>  wrote:
>> >> -Original Message-
>> >> From: pravin shelar [mailto:pshe...@ovn.org]
>> >> Sent: Thursday, April 14, 2016 5:59 PM
>> >> To: Chandran, Sugesh 
>> >> Cc: ovs dev 
>> >> Subject: Re: [ovs-dev] [PATCH v2] tunneling: Improving tunneling
>> >> performance using DPDK Rx checksum offloading feature.
>> >> On Wed, Apr 13, 2016 at 7:42 AM, Sugesh Chandran
>> >>  wrote:
>> >> > +static inline bool
>> >> > +is_checksum_valid(struct dp_packet *packet) { #ifdef DPDK_NETDEV
>> >> > +if (packet->mbuf.ol_flags & (PKT_RX_IP_CKSUM_BAD |
>> >> > + PKT_RX_L4_CKSUM_BAD)) {
>> >> > +return 0;
>> >> > +}
>> >> > +packet->md.ol_flags = NETDEV_RX_CHECKSUM_OFFLOAD;
>> >> There is no need to define redundant flags for same information in
>> >> dp_packet. We can just access packet->mbuf members to check the
>> >> checksum flag.
>> > [Sugesh] mbuf doesn’t have a flag for checksum. However  the checksum
>> > Invalid flags in mbuf get set when a packet received with invalid
>> > checksum on a checksum offloaded port. So a packet with a valid
>> > checksum cannot say if the checksum is already validated in the
>> > NIC/not. We need this information in the packet to bypass checksum
>> validation in tunneling code.
>>
>> What do you think the chances are of fixing this in DPDK? The current design
>> doesn't make a lot of sense to me, so it would be better to address it at the
>> source rather than papering over it in OVS.
> [Sugesh] I feel DPDK may not have to fix this behavior due to the fact that
> 1) The checksum offloading can be enabled only on supported DPDK ports. The 
> port
> Initialization reports error otherwise.
> 2) Any packet received on checksum offload port, always validated by NIC when
> its enabled.
> 3) DPDK reports error on packets having invalid checksum. Why should DPDK
> reports a packet is validated/valid also , provided its implicit as its 
> received on the offloaded
> port.

From a driver perspective, I don't think there is much difference in
reporting checksum valid vs. checksum invalid. However, reporting only
invalid state requires additional bookkeeping to track which ports
support checksum offload, which in this case also results in a
performance impact as you have reported. This just seems like
unnecessary complexity for no gain.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 00/15] Userspace (DPDK) connection tracker

[Fischetti, Antonio]

Hi Daniele,
I just started to have a look to your new v2 patch set.
A minor comment - I know this is a bit of nit-picking - but I ran
utilities/checkpatch.py and I got some output like

patch 4/15
W(1692): Line has trailing whitespace
+static inline void ct_lock_lock(struct ct_lock *lock)

patch 5/15
W(166): Line is greater than 79-characters long
+threads[i].thread = ovs_thread_create("ct_thread", ct_thread_main, 
[i]);

W(191): Line is greater than 79-characters long
+{"benchmark", "n_threads n_pkts batch_size [change_connection]", 3, 4, 
test_benchmark},

patch 6/15
W(52): Line is greater than 79-characters long
+ovs_fatal(0, "batch_size must be between 1 and 
NETDEV_MAX_BURST(%u)",

patches 7/15 and 8/15, 10/15, 11/15 - as there's no comment into the patch, 
maybe 
they just need an empty line before 
“Signed-off-by: Daniele Di Proietto ”?
The output is
E: No signatures found.
E: Too many signoffs; are you missing Co-authored-by lines?

I'll go on and have a look into the code, I hope I provide some useful feedback.

Thanks,
Antonio

> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di
> Proietto
> Sent: Saturday, April 16, 2016 1:03 AM
> To: dev@openvswitch.org
> Subject: [ovs-dev] [PATCH v2 00/15] Userspace (DPDK) connection tracker
> 
> This series aims to implement the ct() action for the dpif-netdev datapath.
> The bulk of the code is in the new conntrack module: it contains some packet
> parsing code, some lookup tables and the logic to implements all the ct bits.
> 
> The conntrack module is helped by conntrack-tcp, for TCP window and flags
> tracking: the bulk of the code of this submodule is from the FreeBSD's pf
> subsystem, therefore is BSD licensed.
> 
> The rest of the series integrates the connection tracker with the rest of
> OVS: the ct() action is implemented in dpif-netdev, and the debugging
> interfaces required by dpctl/{dump,flush}-conntrack are implemented.
> 
> Besides adding some unit tests, this series ports the existing conntrack
> system test to the userspace datapath.  Some small modifications are
> required to pass the testsuite, and some tests still have to be skipped.
> 
> On newer kernels the userspace testsuite has some problems with offloads,
> so a workaround is included.
> 
> This can also be downloaded at:
> 
> https://github.com/ddiproietto/ovs/tree/userconntrack_20160415
> 
> Any feedback is appreciated, thanks.
> 
> v1 -> v2:
> * Fixed bug in tcp_get_wscale(), related to TCP options parsing.
> * Changed names of ICMP constants: now they're different from Linux and
>   FreeBSD.
> * Fixed bug in parse_ipv6_ext_hdrs().
> * Used ALWAYS_INLINE in parse_vlan and parse_ethertype, to avoid a
>   performance regression in miniflow_extract().
> * Updated copyright info in COPYING and debian/copyright.in.
> * Rebased.
> * Changed batching strategy in conntrack_execute() to allow a newly
>   created connection to be picked up by packets in the same batch.
> * Added an ovs-test module to throw pcap files at the connection tracker.
> * Added a workaround for the userspace testsuite on new kernels and a tcp
>   non-conntrack test.
> 
> Daniele Di Proietto (15):
>   packets: Define ICMP types.
>   flow: Export parse_ipv6_ext_hdrs().
>   flow: Introduce parse_dl_type().
>   conntrack: New userspace connection tracker.
>   tests: Add very simple conntrack benchmark.
>   tests: Add test-conntrack pcap test.
>   conntrack: Implement flush function.
>   conntrack: Implement dumping to ct_entry.
>   dpif-netdev: Execute conntrack action.
>   dpif-netdev: Implement conntrack dump functions.
>   dpif-netdev: Implement conntrack flush interface.
>   tests: Add conntrack ofproto-dpif tests.
>   system-tests: Disable offloads in userspace tests.
>   system-tests: Add tcp simple test.
>   system-tests: Run conntrack tests with userspace
> 
>  COPYING  |   1 +
>  debian/copyright.in  |   4 +
>  lib/automake.mk  |   5 +
>  lib/conntrack-other.c|  91 
>  lib/conntrack-private.h  |  80 
>  lib/conntrack-tcp.c  | 510 
>  lib/conntrack.c  | 997
> +++
>  lib/conntrack.h  | 162 +++
>  lib/dpif-netdev.c| 138 +-
>  lib/flow.c   | 154 +++---
>  lib/flow.h   |   4 +
>  lib/packets.h|  14 +-
>  tests/automake.mk|   1 +
>  tests/dpif-netdev.at |  14 +-
>  tests/ofproto-dpif.at| 698 ++-
>  tests/system-common-macros.at|   1 +
>  tests/system-kmod-macros.at  |  35 ++
>  tests/system-traffic.at  |  69 ++-
>  tests/system-userspace-macros.at |  63 ++-
>  tests/test-conntrack.c   | 232 +
>  20 files changed, 3164 insertions(+), 109 deletions(-)
>  create mode 100644 

[ovs-dev] [PATCH V2] add lrouter and lrport related commands to ovn-nbctl

[nghosh]

ovn-nbctl provides a shortcut to perform commands related lswitch, lport
and such but it doesn't have similar commands related to logical router
and logical router port. Also, 'nbctl showi' is supposed to show an
overview of database contents, which means it should show the routers
as well. "show LSWITCH" shows the switch details, similarly "show LROUTER"
should show the router details too. This patch takes care of all of these.
Unit test cases have been added to test all of these modifications and 
additions.

Modifications;
1) ovn-nbctl show -- will now show lrouters as well
2) ovn-nbctl show  -- will show the router now

New commands added:
3) ovn-nbctl lrouter-add [LROUTER]
4) ovn-nbctl lrouter-del LROUTER
5) ovn-nbctl lrouter-list
6) lrport-add LROUTER LRPORT
7) lrport-del LRPORT
8) lrport-list LROUTER
9) lrport-set-mac-address LRPORT [ADDRESS]
10) lrport-get-mac-address LRPORT
11) lrport-set-enabled LRPORT STATE
12) lrport-get-enabled LRPORT

Author: Nirapada Ghosh 
Date: Fri, 8 Apr 2016 18:44:21 -0700
Signed-off-by: Nirapada Ghosh 

---
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index bdad368..6d18005 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -290,12 +290,18 @@ usage: %s [OPTIONS] COMMAND [ARG...]\n\
 General commands:\n\
   show  print overview of database contents\n\
   show LSWITCH  print overview of database contents for LSWITCH\n\
+  show LROUTER  print overview of database contents for LROUTER\n\
 \n\
 Logical switch commands:\n\
   lswitch-add [LSWITCH] create a logical switch named LSWITCH\n\
   lswitch-del LSWITCH   delete LSWITCH and all its ports\n\
   lswitch-list  print the names of all logical switches\n\
 \n\
+Logical router commands:\n\
+  lrouter-add [LROUTER] create a logical router named LROUTER\n\
+  lrouter-del LROUTER   delete LROUTER and all its ports\n\
+  lrouter-list  print the names of all logical routers\n\
+\n\
 ACL commands:\n\
   acl-add LSWITCH DIRECTION PRIORITY MATCH ACTION [log]\n\
 add an ACL to LSWITCH\n\
@@ -303,6 +309,19 @@ ACL commands:\n\
 remove ACLs from LSWITCH\n\
   acl-list LSWITCH  print ACLs for LSWITCH\n\
 \n\
+Logical router port commands:\n\
+  lrport-add LROUTER LRPORT   add logical router port LRPORT to LROUTER\n\
+  lrport-del LRPORT   delete LRPORT from its attached router\n\
+  lrport-list LROUTERprint the names of all logical ports on LROUTER\n\
+  lrport-set-mac-address LRPORT [ADDRESS]\n\
+set MAC address for LRPORT.\n\
+  lrport-get-mac-address LRPORT  get MAC addresses on LRPORT\n\
+  lrport-set-enabled LRPORT STATE\n\
+set administrative state LRPORT\n\
+('enabled' or 'disabled')\n\
+  lrport-get-enabled LRPORT   get administrative state LRPORT\n\
+('enabled' or 'disabled')\n\
+\n\
 Logical port commands:\n\
   lport-add LSWITCH LPORT   add logical port LPORT on LSWITCH\n\
   lport-add LSWITCH LPORT PARENT TAG\n\
@@ -349,8 +368,49 @@ Other options:\n\
 exit(EXIT_SUCCESS);
 }
 
+
+/* Following function finds out the lrouter given it's id. */
+static const struct nbrec_logical_router *
+lrouter_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool silent)
+{
+const struct nbrec_logical_router *lrouter = NULL;
+bool is_uuid = false;
+bool duplicate = false;
+struct uuid lrouter_uuid;
+
+if (uuid_from_string(_uuid, id)) {
+is_uuid = true;
+lrouter = nbrec_logical_router_get_for_uuid(ctx->idl,
+_uuid);
+}
+
+if (!lrouter) {
+const struct nbrec_logical_router *iter;
+
+NBREC_LOGICAL_ROUTER_FOR_EACH(iter, ctx->idl) {
+if (strcmp(iter->name, id)) {
+continue;
+}
+if (lrouter) {
+VLOG_WARN("There is more than one logical router named '%s'. "
+"Use a UUID.", id);
+lrouter = NULL;
+duplicate = true;
+break;
+}
+lrouter = iter;
+}
+}
+
+if (!lrouter && !duplicate && !silent) {
+VLOG_WARN("lrouter not found for %s: '%s'",
+is_uuid ? "UUID" : "name", id);
+}
+
+return lrouter;
+}
 static const struct nbrec_logical_switch *
-lswitch_by_name_or_uuid(struct ctl_context *ctx, const char *id)
+lswitch_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool silent)
 {
 const struct nbrec_logical_switch *lswitch = NULL;
 bool is_uuid = false;
@@ -381,7 +441,7 @@ lswitch_by_name_or_uuid(struct ctl_context *ctx, const char 
*id)
 }
 }
 
-if (!lswitch && !duplicate) {
+if (!lswitch && !duplicate && !silent) {
 

Re: [ovs-dev] [PATCH 1/4] docs: OVSDB replication design document

[Marcelo E. Magallon]

Hi,

 sorry about the delay in responding. I was actually catching up with
 emails on the mailing list to try to gauge if we are indeed trying to
 accomplish the same thing or not.

On Mon, Apr 11, 2016 at 03:44:09PM -0700, Ben Pfaff wrote:
> On Fri, Apr 01, 2016 at 10:52:26AM -0700, Ben Pfaff wrote:
> > I don't think it makes sense to stack replication and Raft-based HA.
> > 
> > Thinking about OpenSwitch, I guess that your use case is something
> > like this: an OpenSwitch instance maintains, on-box, an
> > authoritative database instance, and then the replication feature
> > allows that database's content to be backed up somewhere else.  I
> > see how that differs from the use case for Raft, where there is no
> > preference for a particular database server to be authoritative.
> > What I don't see yet is how or why that's useful.  What is the use
> > case?
> 
> In case it wasn't clear, I didn't mean my message above to sound like
> a "no, we won't take this".  Instead, I'm trying to understand the use
> case better.  Perhaps there is room for both replication and HA in
> OVSDB, but before I say "yes" to that, I want to understand both
> cases.

 Yes, that's totally fair.

 We do not have a need for only 1+1 redundancy. We have a need in which
 we have to remain operational with less than a quantum of instances in
 operation, which raft can’t do unless you introduce modifications to
 the algorithm (e.g. etcd or consul, I can't remember which one
 exactly).

 Also, raft assumes that everybody's vote is equal. If you’re treating
 multiple instances of OVS as one large virtual switch, you are not
 running a separate version of OSPF on each instance, each feeding its
 own version of the routing table into the database.  You have one OSPF
 instance on a "stack commander" feeding the entire routing table into
 the database. This is the "correct" state, no matter how many raft
 members have voted on it. We grow to more than 2 members by setting up
 multiple one way replications, all originating from the "commander". In
 future patches, we will also implement two way replication so that the
 member can write to his local database to reflect state that the
 commander cannot know about (like port state) ... until that happens
 daemons on a "member" can connect directly to the commander's OVSDB
 instance and update the commander's state directly.

 This work is done in the conetxt of OpenSwitch (http://openswitch.net/,
 probaly http://openswitch.net/documents/user/architecture is more
 relevant to this discussion).  With the proposed patch we can have two
 OVSDB instances each running on a TOR switch. One of the switches is
 active and the other is a stand-by. The stand-by instance is constantly
 replicating the active one. In case of a failure in the active, the
 stand-by can take over and the control plane can be rebuilt from the
 state stored in the database.

 I don't think the two approaches are in conflict with each other,
 actually the complement each other. What I'm trying to figure out is
 where they overlap (from a code point of view).

 Marcelo
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovs-dev,v2,2/3] Add skeleton for OF1.6 support.

[Ben Pfaff]

On Sun, Apr 17, 2016 at 05:21:53PM -0500, Ryan Moats wrote:
> 
> > --- Original Message ---
> > Signed-off-by: Ben Pfaff 
> > ---
> >  FAQ.md | 33 +--
> >  build-aux/extract-ofp-errors   |  3 +-
> >  build-aux/extract-ofp-msgs |  3 +-
> >  include/openflow/openflow-common.h |  5 ++-
> >  lib/ofp-actions.c  |  1 +
> >  lib/ofp-errors.c   |  4 +-
> >  lib/ofp-msgs.c |  5 +++
> >  lib/ofp-print.c|  4 ++
> >  lib/ofp-util.c | 82 ++
> +--
> >  lib/ofp-util.h |  9 ++--
> 
> I believe lib/ofp-util.h needs to be changed to include/openflow/ofp-util.h

Thanks.  ("git rebase" is so freakishly smart that it figured this out
on its own, though.)

> >  tests/ofp-errors.at|  2 +
> >  tests/ofproto-macros.at|  3 +-
> >  tests/ofproto.at   | 84 ++
> +---
> >  tests/ovs-ofctl.at | 18 
> >  vswitchd/vswitch.ovsschema |  7 ++--
> >  vswitchd/vswitch.xml   |  5 ++-
> >  16 files changed, 177 insertions(+), 91 deletions(-)
> 
> Given the above modification,
> 
> Acked-by: Ryan Moats 

Thanks for the reviews.  I'll apply these in a minute.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] smap,shash: add numeric and flexible sort

[Worth, Kevin]

>> The default implementation of shash_sort and smap_sort provide only
>> lexigraphic sorting. This patch allows both a numeric sort and a flexible
>> version where the caller provides a comparison function
>> ("compar" being the name used in other standard functions e.g. qsort(3))
>> 
>> Change-Id: I12fd3f8eef3f627fc1482bda47ba8b2329123121
>> Signed-off-by: Kevin Worth 

> Thanks for the patch.
> 
> This is whitespace damaged (it looks like spaces at beginnings of lines
> were deleted) and I couldn't apply it.  Please re-send, or submit a pull
> request.

Thanks Ben, think my email client mangled things... submitted pull request 
https://github.com/openvswitch/ovs/pull/116

-Kevin W.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Is it possible to create and manage a custom table in openvswitch?

[Ben Pfaff]

No.  No.

On Mon, Apr 18, 2016 at 02:37:56PM +, Pynbiang Hadem wrote:
> Thanks Ben,        It seems the "learn" action will manipulate the flow 
> tables only. Can it be used for creating a new customized table?. Is there 
> any working sample on this?. Pls share if any.ThanksHadem
>  
> 
> On Friday, 15 April 2016 10:18 PM, Ben Pfaff  wrote:
>  
> 
>  On Fri, Apr 15, 2016 at 09:37:19AM +, Pynbiang Hadem wrote:
> > Hi,I need to store some information in the switch which maybe tabular
> > in nature consisting of 5 to 6 fields. The table will be populated
> > dynamically during flow processing. Is it possible to create and
> > manage such a custom table in openvswitch?. Pls suggest/advice.
> 
> You might be able to use the "learn" action.
> 
> 
>   
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v7 08/16] dpif-netdev: Add pmd thread local port cache for transmission.

[Ilya Maximets]

On 08.04.2016 06:13, Daniele Di Proietto wrote:
> Signed-off-by: Daniele Di Proietto 
> ---
>  lib/dpif-netdev.c | 243 
> +++---
>  1 file changed, 175 insertions(+), 68 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 8c5893d..5d1cc43 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -185,6 +185,7 @@ static bool dpcls_lookup(const struct dpcls *cls,
>   *
>   *dp_netdev_mutex (global)
>   *port_mutex
> + *non_pmd_mutex
>   */
>  struct dp_netdev {
>  const struct dpif_class *const class;
> @@ -380,6 +381,13 @@ struct rxq_poll {
>  struct ovs_list node;
>  };
>  
> +/* Contained by struct dp_netdev_pmd_thread's 'port_cache' or 'tx_ports'. */
> +struct tx_port {
> +odp_port_t port_no;
> +struct netdev *netdev;
> +struct hmap_node node;
> +};
> +
>  /* PMD: Poll modes drivers.  PMD accesses devices via polling to eliminate
>   * the performance overhead of interrupt processing.  Therefore netdev can
>   * not implement rx-wait for these devices.  dpif-netdev needs to poll
> @@ -436,10 +444,18 @@ struct dp_netdev_pmd_thread {
>  atomic_int tx_qid;  /* Queue id used by this pmd thread to
>   * send packets on all netdevs */
>  
> -struct ovs_mutex poll_mutex;/* Mutex for poll_list. */
> +struct ovs_mutex port_mutex;/* Mutex for 'poll_list' and 'tx_ports'. 
> */
>  /* List of rx queues to poll. */
>  struct ovs_list poll_list OVS_GUARDED;
> -int poll_cnt;   /* Number of elemints in poll_list. */
> +/* Number of elements in 'poll_list' */
> +int poll_cnt;
> +/* Map of 'tx_port's used for transmission.  Written by the main thread,
> + * read by the pmd thread. */
> +struct hmap tx_ports OVS_GUARDED;
> +
> +/* Map of 'tx_port' used in the fast path. This is a thread-local copy
> + * 'tx_ports'. */
> +struct hmap port_cache;
>  
>  /* Only a pmd thread can write on its own 'cycles' and 'stats'.
>   * The main thread keeps 'stats_zero' and 'cycles_zero' as base
> @@ -495,7 +511,7 @@ dp_netdev_pmd_get_next(struct dp_netdev *dp, struct 
> cmap_position *pos);
>  static void dp_netdev_destroy_all_pmds(struct dp_netdev *dp);
>  static void dp_netdev_del_pmds_on_numa(struct dp_netdev *dp, int numa_id);
>  static void dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int numa_id);
> -static void dp_netdev_pmd_clear_poll_list(struct dp_netdev_pmd_thread *pmd);
> +static void dp_netdev_pmd_clear_ports(struct dp_netdev_pmd_thread *pmd);
>  static void dp_netdev_del_port_from_all_pmds(struct dp_netdev *dp,
>   struct dp_netdev_port *port);
>  static void
> @@ -509,6 +525,8 @@ static void dp_netdev_reset_pmd_threads(struct dp_netdev 
> *dp);
>  static bool dp_netdev_pmd_try_ref(struct dp_netdev_pmd_thread *pmd);
>  static void dp_netdev_pmd_unref(struct dp_netdev_pmd_thread *pmd);
>  static void dp_netdev_pmd_flow_flush(struct dp_netdev_pmd_thread *pmd);
> +static void pmd_load_cached_ports(struct dp_netdev_pmd_thread *pmd)
> +OVS_REQUIRES(pmd->port_mutex);
>  
>  static inline bool emc_entry_alive(struct emc_entry *ce);
>  static void emc_clear_entry(struct emc_entry *ce);
> @@ -691,7 +709,7 @@ pmd_info_show_rxq(struct ds *reply, struct 
> dp_netdev_pmd_thread *pmd)
>  ds_put_format(reply, "pmd thread numa_id %d core_id %u:\n",
>pmd->numa_id, pmd->core_id);
>  
> -ovs_mutex_lock(>poll_mutex);
> +ovs_mutex_lock(>port_mutex);
>  LIST_FOR_EACH (poll, node, >poll_list) {
>  const char *name = netdev_get_name(poll->port->netdev);
>  
> @@ -705,7 +723,7 @@ pmd_info_show_rxq(struct ds *reply, struct 
> dp_netdev_pmd_thread *pmd)
>  ds_put_format(reply, " %d", netdev_rxq_get_queue_id(poll->rx));
>  prev_name = name;
>  }
> -ovs_mutex_unlock(>poll_mutex);
> +ovs_mutex_unlock(>port_mutex);
>  ds_put_cstr(reply, "\n");
>  }
>  }
> @@ -1078,6 +1096,11 @@ dp_netdev_reload_pmd__(struct dp_netdev_pmd_thread 
> *pmd)
>  int old_seq;
>  
>  if (pmd->core_id == NON_PMD_CORE_ID) {
> +ovs_mutex_lock(>dp->non_pmd_mutex);
> +ovs_mutex_lock(>port_mutex);
> +pmd_load_cached_ports(pmd);
> +ovs_mutex_unlock(>port_mutex);
> +ovs_mutex_unlock(>dp->non_pmd_mutex);
>  return;
>  }
>  
> @@ -1200,9 +1223,7 @@ do_add_port(struct dp_netdev *dp, const char *devname, 
> const char *type,
>  
>  cmap_insert(>ports, >node, hash_port_no(port_no));
>  
> -if (netdev_is_pmd(port->netdev)) {
> -dp_netdev_add_port_to_pmds(dp, port);
> -}
> +dp_netdev_add_port_to_pmds(dp, port);
>  seq_change(dp->port_seq);
>  
>  return 0;
> @@ -1371,6 +1392,9 @@ do_del_port(struct dp_netdev *dp, struct dp_netdev_port 
> *port)
>  {
>  

Re: [ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface

[Thadeu Lima de Souza Cascardo]

On Mon, Apr 18, 2016 at 12:36:31PM +0200, Jiri Benc wrote:
> On Mon, 18 Apr 2016 06:57:22 -0300, Thadeu Lima de Souza Cascardo wrote:
> > But... wait a minute. We don't support adding devices name as vxlan_sys*. 
> > Such
> > names are reserved. I think that means we could probably rely on the names.
> 
> They are not. You can easily create an interface named vxlan_sys_4789
> by 'ip link add'.
> 

I mean by using ovs-vsctl add-port. At vswitchd/bridge.c:iface_do_create,

if (netdev_is_reserved_name(iface_cfg->name)) {
VLOG_WARN("could not create interface %s, name is reserved",
  iface_cfg->name);
error = EINVAL;
goto error;
}

netdev_is_reserved_name checks for names starting with the dpif_port prefix from
vports. That includes vxlan_sys.

[root@devconf2 ~]# ovs-vsctl add-port br0 vxlan_sys_
ovs-vsctl: Error detected while setting up 'vxlan_sys_'.  See ovs-vswitchd 
log for details.
[root@devconf2 ~]# tail /var/log/openvswitch/ovs-vswitchd.log
2016-04-18T07:34:01.422Z|00032|vlog|INFO|opened log file 
/var/log/openvswitch/ovs-vswitchd.log
2016-04-18T09:30:34.142Z|00033|bridge|WARN|could not create interface 
vxlan_sys_, name is reserved

> > That should be taken care of. We would get EINVAL, and that's why I return
> > EOPNOTSUPP if that's the case. Then, the code falls back to the compat mode.
> 
> We would not get EINVAL. The interface will be created, netlink will
> return success but the interface won't be in the metadata mode.
> 
> The same applies to all other configuration, e.g. VXLAN-GPE. The
> netlink command will succeed, the interface will be created but it
> won't be GPE.
> 
> It sucks completely, it's a big gap in netlink design but all my
> attempts to solve this were rejected upstream.

I will test that and add the verification code.

Thanks.
Cascardo.

> 
>  Jiri
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Returned mail: Data format error

[Post Office]

The original message was received at Mon, 18 Apr 2016 16:55:10 +0530
from 26.65.250.5

- The following addresses had permanent fatal errors -
dev@openvswitch.org



___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] Add configurable OpenFlow port name.

[Xiao Liang]

On Mon, Apr 18, 2016 at 5:46 PM, Takashi YAMAMOTO  wrote:
> for some reasons you want to change of_name without re-creating a port?
> why? (just curious)
>

I don't have special use cases, just for convenience.

>
> On Mon, Apr 18, 2016 at 4:19 PM, Xiao Liang  wrote:
>>
>> By introducing of_name, ovs_name serves as a key of ports which is
>> shared by ofproto and netdev. It's easier to find and convert ports
>> back and forth. of_name and kernel_name could be configured (if
>> supported) independently of each other.
>>
>> On Mon, Apr 18, 2016 at 11:43 AM, Takashi YAMAMOTO 
>> wrote:
>> > let me explain what netdev-bsd does first.
>> > on some platform "tap" interfaces are always named automatically by
>> > kernel
>> > itself
>> > and there's no way to rename them.  say, they always will have names
>> > like
>> > "tap0".
>> > so if you does "ovs-vsctl add-port br0 foo",
>> >   ovs_name = "foo"
>> >   kernel_name = "tap0"
>> >
>> > now, you are going to add another name for openflow. let's call it
>> > of_name.
>> > eg. "ovs-vsctl add-port br0 foo -- set int foo ofname=wan",
>> >   of_name = "wan"
>> >   ovs_name = "foo"
>> >   kernel_name = "tap0"
>> >
>> > while i don't have strong opinions either ways,
>> > i'm not sure why you want to use different names for of_name and
>> > ovs_name
>> > in the first place.  eg. what's wrong with "ovs-vsctl add-port br0 wan".
>> > can you explain a little?
>> >
>> > On Mon, Apr 18, 2016 at 10:37 AM, Xiao Liang 
>> > wrote:
>> >>
>> >> Hi Ben, Yamamoto-san,
>> >>
>> >> Kindly remind you of this thread. Would like to hear your preference
>> >> on the way to implement this feature.
>> >>
>> >> On Mon, Apr 11, 2016 at 11:18 PM, Ben Pfaff  wrote:
>> >> > On Mon, Apr 11, 2016 at 04:30:04PM +0800, Xiao Liang wrote:
>> >> >> On Mon, Apr 11, 2016 at 3:42 PM, Takashi Yamamoto
>> >> >>  wrote:
>> >> >> > hi,
>> >> >> >
>> >> >> > have you considered the opposite way?
>> >> >> > ie. have an ability to specify the device name.
>> >> >> >
>> >> >> > netdev-bsd already has a distinction between "kernel name" and
>> >> >> > "ovs
>> >> >> > name".
>> >> >> >
>> >> >>
>> >> >> Hi,
>> >> >>
>> >> >> I'm not familiar with netdev-bsd code, but I think this approach
>> >> >> will
>> >> >> make ports more difficult to manage and need much more effort.
>> >> >
>> >> > Yamamoto-san: thanks for bringing this up.  I'm going to wait for you
>> >> > and Xiao to talk this through a bit before continuing review.
>> >> ___
>> >> dev mailing list
>> >> dev@openvswitch.org
>> >> http://openvswitch.org/mailman/listinfo/dev
>> >
>> >
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface

[Jiri Benc]

On Mon, 18 Apr 2016 06:57:22 -0300, Thadeu Lima de Souza Cascardo wrote:
> I don't see how this address the case when the user adds a vxlan interface
> created by the system. Like:
> 
> ip link add vxlan_sys_4789 type vxlan id 10 remote 192.168.123.1 dstport 4789
> ovs-vsctl add-port br0 vxlan_sys_4789
> 
> Its driver would be vxlan. That goes to vxlan0 too.

Yes but that's a fundamental problem that cannot be solved. There's no
way an interface created by the user can be *reliably* distinguished
from an interface automatically created by ovs, whatever approach you
choose. Anything that ovs does with the interface, the user can do,
too.

The only exception is adding the interface to ovs bridge - that can't be
done with 'ip'. Thus, if you disallow adding of vxlan interfaces (i.e.
'ovs-vsctl add-port br0 iface' where 'iface' is an already created
interface that is of vxlan type), then and only then you can be sure
that the interfaces connected to the kernel datapath were automatically
created by ovs.

> But... wait a minute. We don't support adding devices name as vxlan_sys*. Such
> names are reserved. I think that means we could probably rely on the names.

They are not. You can easily create an interface named vxlan_sys_4789
by 'ip link add'.

> That should be taken care of. We would get EINVAL, and that's why I return
> EOPNOTSUPP if that's the case. Then, the code falls back to the compat mode.

We would not get EINVAL. The interface will be created, netlink will
return success but the interface won't be in the metadata mode.

The same applies to all other configuration, e.g. VXLAN-GPE. The
netlink command will succeed, the interface will be created but it
won't be GPE.

It sucks completely, it's a big gap in netlink design but all my
attempts to solve this were rejected upstream.

 Jiri
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface

[Jiri Benc]

On Fri, 15 Apr 2016 20:36:51 -0700, Jesse Gross wrote:
> What about using the driver name exposed through ethtool? I believe
> that all of the tunnel and internal devices expose this in a
> consistent way - i.e. a VXLAN device can be queried and get back the
> string "vxlan". Any driver other than the ones that we recognize can
> be assumed to be OVS_VPORT_TYPE_NETDEV.

Or netlink. Though ethtool is more generic in this case, as with
netlink, every interface type would need to be queried differently.

> I'm not too excited about this. It seems like it would be a regression
> - currently OVSDB allows remote creation of tunnels, so it seems like
> this would break existing setups if it also requires users to
> explicitly create tunnel devices on the host ahead of time.

It wouldn't break the existing setups, the current code is not going
away.

However, it wouldn't allow remote creation of tunnels with new features
(like VXLAN-GPE). I don't think it's that bad - we don't support remote
creation of e.g. veth pairs, yet it's common to have them.

> One comment on the patch itself - it's possible that the device that
> is being created might not support all of the necessary options that
> we are passing to it. For example, the original VXLAN driver as merged
> into the kernel didn't support COLLECT_METADATA. We'll need to check
> that the device that was created supports what we need and fallback to
> the old model otherwise.

Yes. Unfortunately, the only way to find out is to fetch back the
config from the created interface and if it doesn't match, delete the
interface. And fall back to the compat code but *only if* there are no
advanced features specified. What the "advanced features" are will need
to be hardcoded in ovs.

What I'm proposing makes this much simpler. No need for verifying the
interface was created with all the wanted flags, no need for knowing
what features are supported by the compat code, etc. Just take whatever
was given to ovs by the user and use it.

 Jiri
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface

[Jiri Benc]

On Fri, 15 Apr 2016 18:30:59 -0300, Thadeu Lima de Souza Cascardo wrote:
> Jiri has suggested that we require users to create the interfaces themselves, 
> by
> using whatever method their OS has, and add them as netdev devices. That would
> still require fetching some of the configuration from the device in order to
> make it properly work with flow-based tunnels. In fact, if we set the remote 
> or
> local IP addresses on those devices, this would require multiple interfaces
> instead of only one just to be able to specify the same level of configuration
> as ovsdb allows us to.

No need for multiple interfaces. We have 'external' parameter for 'ip
link add' that will create the interface in metada mode.

The reason I'm suggesting this is it solves the problem with cleanup in
unexpected conditions (What happens with the interface automatically
created by ovs when ovs crashes? What happens if user creates an
interface named vxlan_sys_*? Relaying on a particular interface name
doesn't look like a good design to me.) and it allows easy support of
future vxlan features without modifying ovs.

This would require querying the kernel whether the interface is in
metadata mode on vport addition, sure. But that's very easy to do and
I think it's the only configuration parameter that we need to know in
ovs.

The existing code that allows adding the ports through the compat
genetlink interface would stay, of course, for backwards compatibility.
To use new vxlan feature, the new way of setup would be needed.

 Jiri
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC PATCH] create vxlan device using rtnetlink interface

[Thadeu Lima de Souza Cascardo]

On Fri, Apr 15, 2016 at 08:36:51PM -0700, Jesse Gross wrote:
> On Fri, Apr 15, 2016 at 2:30 PM, Thadeu Lima de Souza Cascardo
>  wrote:
> > Hi, this is an RFC patch (could probably be 2 patches - more below), that
> > creates VXLAN devices in userspace and adds them as netdev vports, instead 
> > of
> > using the vxlan vport code.
> >
> > The reason for this change is that it has been mentioned in the past that 
> > some
> > of the vport code should be considered compat code, that is, it won't 
> > receive
> > new features or flags.
> 
> Thanks for looking at this. I agree that this is definitely a
> necessary direction.
> 
> > First is the creation of the device under a switch/case. I tried to make 
> > this a
> > netdev_class function, overriding the netdev class by dpif_port_open_type. 
> > That
> > required exporting a lot of the vport functions. I can still do it that 
> > way, if
> > that's preferrable, but this seems more simple.
> 
> Can you give some examples of what would need to be exported? It would
> be nice to just set the open function for each type but if it is
> really horrible we can live with the switch statement.
> 

netdev_vport_{alloc, construct, destruct, dealloc}, get_tunnel_config,
set_tunnel_config, get_netdev_tunnel_config.

We could also do it the other way around, write this code in netdev-vport.c and
export one or two functions from netdev-linux or dpif-netlink, and the create
function per tunnel type.

> > The other problem is during port_del, that since we don't have a netdev, the
> > dpif_port type would not be vxlan, and deciding whether to remove it or not
> > could not be made. In fact, this is one of the main reasons why this is an 
> > RFC.
> >
> > The solution here is to decide on the type by the device's name, and there 
> > is
> > even a function for this, though it looks up for the netdev's already 
> > created,
> > those that probably come from the database. However, when OVS starts, it 
> > removes
> > ports from the datapath, and that information is not available, and may not 
> > even
> > be. In fact, since the dpif_port has a different name because it's a vport, 
> > it
> > won't match any netdev at all. So, I did the opposite of getting the type 
> > from
> > the dpif_port name, ie, if it contains vxlan_sys, it's of vxlan type. Even 
> > if we
> > make this more strict and use the port, we could still be in conflict with a
> > vxlan device created by the user with such name. This should have been one 
> > patch
> > by itself.
> 
> What about using the driver name exposed through ethtool? I believe
> that all of the tunnel and internal devices expose this in a
> consistent way - i.e. a VXLAN device can be queried and get back the
> string "vxlan". Any driver other than the ones that we recognize can
> be assumed to be OVS_VPORT_TYPE_NETDEV.
> 

I don't see how this address the case when the user adds a vxlan interface
created by the system. Like:

ip link add vxlan_sys_4789 type vxlan id 10 remote 192.168.123.1 dstport 4789
ovs-vsctl add-port br0 vxlan_sys_4789

Its driver would be vxlan. That goes to vxlan0 too.

But... wait a minute. We don't support adding devices name as vxlan_sys*. Such
names are reserved. I think that means we could probably rely on the names.

> > Jiri has suggested that we require users to create the interfaces 
> > themselves, by
> > using whatever method their OS has, and add them as netdev devices. That 
> > would
> > still require fetching some of the configuration from the device in order to
> > make it properly work with flow-based tunnels. In fact, if we set the 
> > remote or
> > local IP addresses on those devices, this would require multiple interfaces
> > instead of only one just to be able to specify the same level of 
> > configuration
> > as ovsdb allows us to. And that still requires some proper changes in order 
> > to
> > support flow-based tunnels (calling tnl_port_add/tnl_port_del with proper
> > configuration, for example).
> 
> I'm not too excited about this. It seems like it would be a regression
> - currently OVSDB allows remote creation of tunnels, so it seems like
> this would break existing setups if it also requires users to
> explicitly create tunnel devices on the host ahead of time.

Agreed. That's a very good reason not to go this path. And realizing we already
reserve the names, I am more comfortable relying on only that.

> 
> One comment on the patch itself - it's possible that the device that
> is being created might not support all of the necessary options that
> we are passing to it. For example, the original VXLAN driver as merged
> into the kernel didn't support COLLECT_METADATA. We'll need to check
> that the device that was created supports what we need and fallback to
> the old model otherwise.
> 

That should be taken care of. We would get EINVAL, and that's why I return
EOPNOTSUPP if that's the case. Then, the code falls back to the compat mode.

> I presume that you 

[ovs-dev] [PATCH V3, 2/3] datapath-windows: Removed double initialization on local variables

[Paul Boca]

Signed-off-by: Paul-Daniel Boca 
Acked-by: Sorin Vinturis 
---
v3: fixed a minor compilation issue.
---
 datapath-windows/ovsext/Actions.c | 10 +-
 datapath-windows/ovsext/Flow.c|  2 +-
 datapath-windows/ovsext/IpHelper.c|  4 ++--
 datapath-windows/ovsext/Netlink/Netlink.c |  2 +-
 datapath-windows/ovsext/Oid.c |  2 +-
 datapath-windows/ovsext/PacketParser.c|  1 -
 datapath-windows/ovsext/Tunnel.c  |  5 ++---
 datapath-windows/ovsext/TunnelFilter.c| 12 ++--
 8 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index cf54ae2..5dae6b4 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -983,7 +983,7 @@ static __inline NDIS_STATUS
 OvsOutputBeforeSetAction(OvsForwardingContext *ovsFwdCtx)
 {
 PNET_BUFFER_LIST newNbl;
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+NDIS_STATUS status;
 
 /*
  * Create a copy and work on the copy after this point. The original NBL is
@@ -1142,7 +1142,7 @@ static __inline NDIS_STATUS
 OvsActionMplsPop(OvsForwardingContext *ovsFwdCtx,
  ovs_be16 ethertype)
 {
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+NDIS_STATUS status;
 OVS_PACKET_HDR_INFO *layers = >layers;
 EthHdr *ethHdr = NULL;
 
@@ -1945,7 +1945,7 @@ OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext,
   const PNL_ATTR actions,
   INT actionsLen)
 {
-NDIS_STATUS status = STATUS_SUCCESS;
+NDIS_STATUS status;
 
 status = OvsDoExecuteActions(switchContext, completionList, curNbl,
  portNo, sendFlags, key, hash, layers,
@@ -1974,8 +1974,8 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
 UINT32 srcPortNo,
 OVS_PACKET_HDR_INFO *layers)
 {
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
-OvsFlow *flow = NULL;
+NDIS_STATUS status;
+OvsFlow *flow;
 OvsForwardingContext ovsFwdCtx = { 0 };
 UINT64 hash = 0;
 ASSERT(layers);
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index a7e9bd2..1f23625 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -723,7 +723,7 @@ done:
 static NTSTATUS
 _MapFlowInfoToNl(PNL_BUFFER nlBuf, OvsFlowInfo *flowInfo)
 {
-NTSTATUS rc = STATUS_SUCCESS;
+NTSTATUS rc;
 
 rc = MapFlowKeyToNlKey(nlBuf, &(flowInfo->key), OVS_FLOW_ATTR_KEY,
OVS_KEY_ATTR_TUNNEL);
diff --git a/datapath-windows/ovsext/IpHelper.c 
b/datapath-windows/ovsext/IpHelper.c
index 8126222..d747e8c 100644
--- a/datapath-windows/ovsext/IpHelper.c
+++ b/datapath-windows/ovsext/IpHelper.c
@@ -231,7 +231,7 @@ OvsGetIPEntry(NET_LUID interfaceLuid,
 NTSTATUS status;
 UINT32 i;
 
-if (ipEntry == NULL || ipEntry == NULL) {
+if (ipEntry == NULL) {
 return STATUS_INVALID_PARAMETER;
 }
 
@@ -1209,7 +1209,7 @@ static VOID
 OvsHandleFwdRequest(POVS_IP_HELPER_REQUEST request)
 {
 SOCKADDR_INET dst, src;
-NTSTATUS status = STATUS_SUCCESS;
+NTSTATUS status;
 MIB_IPFORWARD_ROW2 ipRoute;
 MIB_IPNET_ROW2 ipNeigh;
 OVS_FWD_INFO fwdInfo;
diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index e2312da..27dcd4f 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -565,7 +565,7 @@ NlMsgPutNested(PNL_BUFFER buf, UINT16 type,
const PVOID data, UINT32 size)
 {
 UINT32 offset = NlMsgStartNested(buf, type);
-BOOLEAN ret = FALSE;
+BOOLEAN ret;
 
 ASSERT(offset);
 
diff --git a/datapath-windows/ovsext/Oid.c b/datapath-windows/ovsext/Oid.c
index 93894ae..7c7ffe7 100644
--- a/datapath-windows/ovsext/Oid.c
+++ b/datapath-windows/ovsext/Oid.c
@@ -366,7 +366,7 @@ OvsExtOidRequest(NDIS_HANDLE filterModuleContext,
  PNDIS_OID_REQUEST oidRequest)
 {
 POVS_SWITCH_CONTEXT switchObject = 
(POVS_SWITCH_CONTEXT)filterModuleContext;
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+NDIS_STATUS status;
 PNDIS_OID_REQUEST clonedOidRequest = NULL;
 struct _METHOD *methodInfo = &(oidRequest->DATA.METHOD_INFORMATION);
 BOOLEAN completeOid = FALSE;
diff --git a/datapath-windows/ovsext/PacketParser.c 
b/datapath-windows/ovsext/PacketParser.c
index bd6910c..93df342 100644
--- a/datapath-windows/ovsext/PacketParser.c
+++ b/datapath-windows/ovsext/PacketParser.c
@@ -93,7 +93,6 @@ OvsParseIPv6(const NET_BUFFER_LIST *packet,
 UINT32 nextHdr;
 Ipv6Key *flow= >ipv6Key;
 
-ofs = layers->l3Offset;
 nh = OvsGetPacketBytes(packet, sizeof *nh, ofs, );
 if (!nh) {
 return NDIS_STATUS_FAILURE;
diff --git a/datapath-windows/ovsext/Tunnel.c b/datapath-windows/ovsext/Tunnel.c
index e957aaf..97d2020 100644
--- a/datapath-windows/ovsext/Tunnel.c

Re: [ovs-dev] [PATCH v2] Add configurable OpenFlow port name.

[Takashi YAMAMOTO]

for some reasons you want to change of_name without re-creating a port?
why? (just curious)

On Mon, Apr 18, 2016 at 4:19 PM, Xiao Liang  wrote:

> By introducing of_name, ovs_name serves as a key of ports which is
> shared by ofproto and netdev. It's easier to find and convert ports
> back and forth. of_name and kernel_name could be configured (if
> supported) independently of each other.
>
> On Mon, Apr 18, 2016 at 11:43 AM, Takashi YAMAMOTO 
> wrote:
> > let me explain what netdev-bsd does first.
> > on some platform "tap" interfaces are always named automatically by
> kernel
> > itself
> > and there's no way to rename them.  say, they always will have names like
> > "tap0".
> > so if you does "ovs-vsctl add-port br0 foo",
> >   ovs_name = "foo"
> >   kernel_name = "tap0"
> >
> > now, you are going to add another name for openflow. let's call it
> of_name.
> > eg. "ovs-vsctl add-port br0 foo -- set int foo ofname=wan",
> >   of_name = "wan"
> >   ovs_name = "foo"
> >   kernel_name = "tap0"
> >
> > while i don't have strong opinions either ways,
> > i'm not sure why you want to use different names for of_name and ovs_name
> > in the first place.  eg. what's wrong with "ovs-vsctl add-port br0 wan".
> > can you explain a little?
> >
> > On Mon, Apr 18, 2016 at 10:37 AM, Xiao Liang 
> wrote:
> >>
> >> Hi Ben, Yamamoto-san,
> >>
> >> Kindly remind you of this thread. Would like to hear your preference
> >> on the way to implement this feature.
> >>
> >> On Mon, Apr 11, 2016 at 11:18 PM, Ben Pfaff  wrote:
> >> > On Mon, Apr 11, 2016 at 04:30:04PM +0800, Xiao Liang wrote:
> >> >> On Mon, Apr 11, 2016 at 3:42 PM, Takashi Yamamoto
> >> >>  wrote:
> >> >> > hi,
> >> >> >
> >> >> > have you considered the opposite way?
> >> >> > ie. have an ability to specify the device name.
> >> >> >
> >> >> > netdev-bsd already has a distinction between "kernel name" and "ovs
> >> >> > name".
> >> >> >
> >> >>
> >> >> Hi,
> >> >>
> >> >> I'm not familiar with netdev-bsd code, but I think this approach will
> >> >> make ports more difficult to manage and need much more effort.
> >> >
> >> > Yamamoto-san: thanks for bringing this up.  I'm going to wait for you
> >> > and Xiao to talk this through a bit before continuing review.
> >> ___
> >> dev mailing list
> >> dev@openvswitch.org
> >> http://openvswitch.org/mailman/listinfo/dev
> >
> >
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] tunneling: Improving tunneling performance using DPDK Rx checksum offloading feature.

[Chandran, Sugesh]

Regards
_Sugesh

> -Original Message-
> From: Jesse Gross [mailto:je...@kernel.org]
> Sent: Friday, April 15, 2016 5:04 PM
> To: Chandran, Sugesh 
> Cc: pravin shelar ; ovs dev 
> Subject: Re: [ovs-dev] [PATCH v2] tunneling: Improving tunneling
> performance using DPDK Rx checksum offloading feature.
> 
> On Fri, Apr 15, 2016 at 3:04 AM, Chandran, Sugesh
>  wrote:
> >> -Original Message-
> >> From: pravin shelar [mailto:pshe...@ovn.org]
> >> Sent: Thursday, April 14, 2016 5:59 PM
> >> To: Chandran, Sugesh 
> >> Cc: ovs dev 
> >> Subject: Re: [ovs-dev] [PATCH v2] tunneling: Improving tunneling
> >> performance using DPDK Rx checksum offloading feature.
> >> On Wed, Apr 13, 2016 at 7:42 AM, Sugesh Chandran
> >>  wrote:
> >> > +static inline bool
> >> > +is_checksum_valid(struct dp_packet *packet) { #ifdef DPDK_NETDEV
> >> > +if (packet->mbuf.ol_flags & (PKT_RX_IP_CKSUM_BAD |
> >> > + PKT_RX_L4_CKSUM_BAD)) {
> >> > +return 0;
> >> > +}
> >> > +packet->md.ol_flags = NETDEV_RX_CHECKSUM_OFFLOAD;
> >> There is no need to define redundant flags for same information in
> >> dp_packet. We can just access packet->mbuf members to check the
> >> checksum flag.
> > [Sugesh] mbuf doesn’t have a flag for checksum. However  the checksum
> > Invalid flags in mbuf get set when a packet received with invalid
> > checksum on a checksum offloaded port. So a packet with a valid
> > checksum cannot say if the checksum is already validated in the
> > NIC/not. We need this information in the packet to bypass checksum
> validation in tunneling code.
> 
> What do you think the chances are of fixing this in DPDK? The current design
> doesn't make a lot of sense to me, so it would be better to address it at the
> source rather than papering over it in OVS.
[Sugesh] I feel DPDK may not have to fix this behavior due to the fact that
1) The checksum offloading can be enabled only on supported DPDK ports. The 
port 
Initialization reports error otherwise.
2) Any packet received on checksum offload port, always validated by NIC when
its enabled. 
3) DPDK reports error on packets having invalid checksum. Why should DPDK
reports a packet is validated/valid also , provided its implicit as its 
received on the offloaded
port.

Anyway I will discuss this with DPDK folks and keep you posted.



___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V4] ovn-controller: reload configured SB probe timer

[Lei Huang]

On 4/14/16, 3:24 PM, "dev on behalf of ngh...@us.ibm.com" <
dev-boun...@openvswitch.org on behalf of ngh...@us.ibm.com> wrote:

There are four sessions established from ovn-controller to the following:
OVN Southbound — JSONRPC based
Local ovsdb — JSONRPC based
Local vswitchd — openflow based from ofctrl
Local vswitchd — openflow based from pinctrl

All of these sessions have their own probe_interval, and currently one
[SB] of them can be configured using ovn-vsctl command, but that is not
effective on the fly —in other words, ovn-controller has to be restarted to
use that probe_timer value, this patch takes care of that.
For the local ovsdb connection, probe timer is already disabled. For the
last
two connections, they do not need probe_timer as they are over unix domain
socket. This patch takes care of that as well.

This change has been tested putting logs in several places like in
ovn-controller.c, lib/rconn.c to make sure the right probe_timer
values are in effect. Also, by making sure from ovn-controller's
log file that there is no more reconnect happening due to probe
under heavy load.

Author: Nirapada Ghosh 
Date:   Wed Mar 30 19:03:10 2016 -0700
Signed-off-by: Nirapada Ghosh 

diff --git a/lib/rconn.c b/lib/rconn.c
index 6de4c63..54f3745 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -339,6 +339,9 @@ rconn_connect(struct rconn *rc, const char *target,
const char *name)
 rconn_disconnect__(rc);
 rconn_set_target__(rc, target, name);
 rc->reliable = true;
+if (!stream_or_pstream_needs_probes()) {

stream_or_pstream_needs_probes() is called without argument ‘target’,
compile gives a warning:

warning: implicit declaration of function ` stream_or_stream_needs_probs’ …
Header file stream.h should be included.

+rc->probe_interval =0;

Need a whitespace at right side of ‘=‘.

+}
 reconnect(rc);
 ovs_mutex_unlock(>mutex);
}
diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
index 7c68c9d..92e9543 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -56,6 +56,13 @@ static unixctl_cb_func ovn_controller_exit;
static unixctl_cb_func ct_zone_list;
#define DEFAULT_BRIDGE_NAME "br-int"
+#define DEFAULT_PROBE_INTERVAL 5
+
+static void set_probe_timer_if_changed(const struct ovsrec_open_vswitch
*cfg,
+   const struct ovsdb_idl *sb_idl);
+static bool extract_probe_timer(const struct ovsrec_open_vswitch *cfg,
+char *key_name,
+int *ret_value);
static void parse_options(int argc, char *argv[]);
OVS_NO_RETURN static void usage(void);
@@ -217,32 +224,6 @@ get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
 }
}
-/* Retrieves the OVN Southbound remote's json session probe interval from
the
- * "external-ids:ovn-remote-probe-interval" key in 'ovs_idl' and returns
it.
- *
- * This function must be called after get_ovnsb_remote(). */
-static bool
-get_ovnsb_remote_probe_interval(struct ovsdb_idl *ovs_idl, int *value)
-{
-const struct ovsrec_open_vswitch *cfg =
ovsrec_open_vswitch_first(ovs_idl);
-if (!cfg) {
-return false;
-}
-
-const char *probe_interval =
-smap_get(>external_ids, "ovn-remote-probe-interval");
-if (probe_interval) {
-if (str_to_int(probe_interval, 10, value)) {
-return true;
-}
-
-VLOG_WARN("Invalid value for OVN remote probe interval: %s",
-  probe_interval);
-}
-
-return false;
-}
-
int
main(int argc, char *argv[])
{
@@ -306,10 +287,12 @@ main(int argc, char *argv[])
 ovsdb_idl_create(ovnsb_remote, _idl_class, true, true));
 ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop.idl);
-int probe_interval = 0;
-if (get_ovnsb_remote_probe_interval(ovs_idl_loop.idl,
_interval)) {
-ovsdb_idl_set_probe_interval(ovnsb_idl_loop.idl, probe_interval);
-}
+const struct ovsrec_open_vswitch *cfg =
+ovsrec_open_vswitch_first(ovs_idl_loop.idl);
+if (!cfg) {
+return false;
+ }
+set_probe_timer_if_changed(cfg,ovnsb_idl_loop.idl);
 /* Initialize connection tracking zones. */
 struct simap ct_zones = SIMAP_INITIALIZER(_zones);
@@ -327,6 +310,7 @@ main(int argc, char *argv[])
 .ovnsb_idl = ovnsb_idl_loop.idl,
 .ovnsb_idl_txn = ovsdb_idl_loop_run(_idl_loop),
 };
+set_probe_timer_if_changed(cfg,ovnsb_idl_loop.idl);

Need a whitespace after comma.

 /* Contains "struct local_datpath" nodes whose hash values are the
  * tunnel_key of datapaths with at least one local port binding. */
@@ -561,3 +545,55 @@ ct_zone_list(struct unixctl_conn *conn, int argc
OVS_UNUSED,
 unixctl_command_reply(conn, ds_cstr());
 ds_destroy();
}
+
+/* If SB probe timer is changed using ovs-vsctl command, this function
+ * will set that probe timer value for the session.
+ * cfg: Holding the external-id 

Re: [ovs-dev] [PATCH V4] ovn-controller: reload configured SB probe timer

[Huang, Lei]

On 4/14/16, 3:24 PM, "dev on behalf of ngh...@us.ibm.com" 
 wrote:

>There are four sessions established from ovn-controller to the following:
>OVN Southbound — JSONRPC based
>Local ovsdb — JSONRPC based
>Local vswitchd — openflow based from ofctrl
>Local vswitchd — openflow based from pinctrl
>
>All of these sessions have their own probe_interval, and currently one
>[SB] of them can be configured using ovn-vsctl command, but that is not
>effective on the fly —in other words, ovn-controller has to be restarted to
>use that probe_timer value, this patch takes care of that.
>For the local ovsdb connection, probe timer is already disabled. For the last
>two connections, they do not need probe_timer as they are over unix domain
>socket. This patch takes care of that as well.
>
>This change has been tested putting logs in several places like in
>ovn-controller.c, lib/rconn.c to make sure the right probe_timer
>values are in effect. Also, by making sure from ovn-controller's
>log file that there is no more reconnect happening due to probe
>under heavy load.
>
>Author: Nirapada Ghosh 
>Date:   Wed Mar 30 19:03:10 2016 -0700
>Signed-off-by: Nirapada Ghosh 
>
>diff --git a/lib/rconn.c b/lib/rconn.c
>index 6de4c63..54f3745 100644
>--- a/lib/rconn.c
>+++ b/lib/rconn.c
>@@ -339,6 +339,9 @@ rconn_connect(struct rconn *rc, const char *target, const 
>char *name)
> rconn_disconnect__(rc);
> rconn_set_target__(rc, target, name);
> rc->reliable = true;
>+if (!stream_or_pstream_needs_probes()) {
stream_or_pstream_needs_probes() is called without argument ‘target’, compile 
gives a warning:

warning: implicit declaration of function ` stream_or_stream_needs_probs’ …
Header file stream.h should be included.

>+rc->probe_interval =0;
Need a whitespace at right side of ‘=‘.
>+}
> reconnect(rc);
> ovs_mutex_unlock(>mutex);
> }
>diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
>index 7c68c9d..92e9543 100644
>--- a/ovn/controller/ovn-controller.c
>+++ b/ovn/controller/ovn-controller.c
>@@ -56,6 +56,13 @@ static unixctl_cb_func ovn_controller_exit;
> static unixctl_cb_func ct_zone_list;
> 
> #define DEFAULT_BRIDGE_NAME "br-int"
>+#define DEFAULT_PROBE_INTERVAL 5
>+
>+static void set_probe_timer_if_changed(const struct ovsrec_open_vswitch *cfg,
>+   const struct ovsdb_idl *sb_idl);
>+static bool extract_probe_timer(const struct ovsrec_open_vswitch *cfg,
>+char *key_name,
>+int *ret_value);
> 
> static void parse_options(int argc, char *argv[]);
> OVS_NO_RETURN static void usage(void);
>@@ -217,32 +224,6 @@ get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
> }
> }
> 
>-/* Retrieves the OVN Southbound remote's json session probe interval from the
>- * "external-ids:ovn-remote-probe-interval" key in 'ovs_idl' and returns it.
>- *
>- * This function must be called after get_ovnsb_remote(). */
>-static bool
>-get_ovnsb_remote_probe_interval(struct ovsdb_idl *ovs_idl, int *value)
>-{
>-const struct ovsrec_open_vswitch *cfg = 
>ovsrec_open_vswitch_first(ovs_idl);
>-if (!cfg) {
>-return false;
>-}
>-
>-const char *probe_interval =
>-smap_get(>external_ids, "ovn-remote-probe-interval");
>-if (probe_interval) {
>-if (str_to_int(probe_interval, 10, value)) {
>-return true;
>-}
>-
>-VLOG_WARN("Invalid value for OVN remote probe interval: %s",
>-  probe_interval);
>-}
>-
>-return false;
>-}
>-
> int
> main(int argc, char *argv[])
> {
>@@ -306,10 +287,12 @@ main(int argc, char *argv[])
> ovsdb_idl_create(ovnsb_remote, _idl_class, true, true));
> ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop.idl);
> 
>-int probe_interval = 0;
>-if (get_ovnsb_remote_probe_interval(ovs_idl_loop.idl, _interval)) {
>-ovsdb_idl_set_probe_interval(ovnsb_idl_loop.idl, probe_interval);
>-}
>+const struct ovsrec_open_vswitch *cfg =
>+ovsrec_open_vswitch_first(ovs_idl_loop.idl);
>+if (!cfg) {
>+return false;
>+ }
>+set_probe_timer_if_changed(cfg,ovnsb_idl_loop.idl);
> 
> /* Initialize connection tracking zones. */
> struct simap ct_zones = SIMAP_INITIALIZER(_zones);
>@@ -327,6 +310,7 @@ main(int argc, char *argv[])
> .ovnsb_idl = ovnsb_idl_loop.idl,
> .ovnsb_idl_txn = ovsdb_idl_loop_run(_idl_loop),
> };
>+set_probe_timer_if_changed(cfg,ovnsb_idl_loop.idl);
Need a whitespace after comma.
> 
> /* Contains "struct local_datpath" nodes whose hash values are the
>  * tunnel_key of datapaths with at least one local port binding. */
>@@ -561,3 +545,55 @@ ct_zone_list(struct unixctl_conn *conn, int argc 
>OVS_UNUSED,
> unixctl_command_reply(conn, ds_cstr());
> ds_destroy();
> }
>+
>+/* If SB 

Re: [ovs-dev] [PATCH v7 16/16] netdev-dpdk: Use ->reconfigure() call to change rx/tx queues.

[Kavanagh, Mark B]

>
>
>
>On 14/04/2016 05:37, "Kavanagh, Mark B"  wrote:
>
>>Hi Daniele,
>>
>>One comment inline.
>>
>>Thanks,
>>Mark
>>
>>>
>>>This introduces in dpif-netdev and netdev-dpdk the first use for the
>>>newly introduce reconfigure netdev call.
>>>
>>>When a request to change the number of queues comes, netdev-dpdk will
>>>remember this and notify the upper layer via
>>>netdev_request_reconfigure().
>>>
>>>The datapath, instead of periodically calling netdev_set_multiq(), can
>>>detect this and call reconfigure().
>>>
>>>This mechanism can also be used to:
>>>* Automatically match the number of rxq with the one provided by qemu
>>>  via the new_device callback.
>>>* Provide a way to change the MTU of dpdk devices at runtime.
>>>* Move a DPDK vhost device to the proper NUMA socket.
>>>
>>>Signed-off-by: Daniele Di Proietto 
>>>---
>>> lib/dpif-netdev.c |  69 +-
>>> lib/netdev-dpdk.c | 195
>>>++
>>> lib/netdev-provider.h |  23 +++---
>>> lib/netdev.c  |  34 +++--
>>> lib/netdev.h  |   3 +-
>>> 5 files changed, 155 insertions(+), 169 deletions(-)
>
>[...]
>
>>>@@ -312,12 +305,12 @@ struct netdev_class {
>>>  * making sure that these concurrent calls do not create a race
>>>condition
>>>  * by using multiple hw queues or locking.
>>>  *
>>>- * On error, the tx queue and rx queue configuration is
>>>indeterminant.
>>>- * Caller should make decision on whether to restore the previous or
>>>- * the default configuration.  Also, caller must make sure there is
>>>no
>>>- * other thread accessing the queues at the same time. */
>>>-int (*set_multiq)(struct netdev *netdev, unsigned int n_txq,
>>>-  unsigned int n_rxq);
>>>+ * The caller will call netdev_reconfigure() (if necessary) before
>>>using
>>>+ * netdev_send() on any of the newly configured queues, giving the
>>>provider
>>>+ * a chance to adjust its settings.
>>>+ *
>>>+ * On error, the tx queue configuration is unchanged. */
>>>+int (*set_multiq)(struct netdev *netdev, unsigned int n_txq);
>>
>>Since this function now deals only with TX queues, an identifier along
>>the lines of 'set_tx_multiq' might more accurately describe its
>>functionality. Specific netdev classes would need to modify the names of
>>their own specific 'set_multiq' functions accordingly.
>
>You're right, 'set_tx_multiq()' is definitely a better name for it. I
>updated it.
>
>Thanks for all your feedback

No problem - thanks for the patchset!

Cheers,
Mark
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v7 09/16] hmap: Use struct for hmap_at_position().

[Kavanagh, Mark B]

>
>Hi Mark,
>
>On 14/04/2016 05:36, "Kavanagh, Mark B"  wrote:
>
>>Hi Daniele,
>>
>>One minor comment inline.
>>
>>Cheers,
>>Mark
>>
>>>
>>>The interface will be more similar to the cmap.
>>>
>>>Signed-off-by: Daniele Di Proietto 
>>>---
>>> lib/hmap.c | 26 --
>>> lib/hmap.h |  7 ++-
>>> lib/sset.c | 12 +---
>>> lib/sset.h |  7 ++-
>>> ofproto/ofproto-dpif.c |  8 +++-
>>> 5 files changed, 32 insertions(+), 28 deletions(-)
>>>
>>>diff --git a/lib/hmap.c b/lib/hmap.c
>>>index b70ce51..9462c5e 100644
>>>--- a/lib/hmap.c
>>>+++ b/lib/hmap.c
>>>@@ -236,24 +236,22 @@ hmap_random_node(const struct hmap *hmap)
>>> }
>>>
>>> /* Returns the next node in 'hmap' in hash order, or NULL if no nodes
>>>remain in
>>>- * 'hmap'.  Uses '*bucketp' and '*offsetp' to determine where to begin
>>>- * iteration, and stores new values to pass on the next iteration into
>>>them
>>>- * before returning.
>>>+ * 'hmap'.  Uses '*pos' to determine where to begin iteration, and
>>>updates
>>>+ * '*pos' to pass on the next iteration into them before returning.
>>>  *
>>>  * It's better to use plain HMAP_FOR_EACH and related functions, since
>>>they are
>>>  * faster and better at dealing with hmaps that change during iteration.
>>>  *
>>>- * Before beginning iteration, store 0 into '*bucketp' and '*offsetp'.
>>>- */
>>>+ * Before beginning iteration, set '*pos' to all zeros. */
>>> struct hmap_node *
>>> hmap_at_position(const struct hmap *hmap,
>>>- uint32_t *bucketp, uint32_t *offsetp)
>>>+ struct hmap_position *pos)
>>> {
>>> size_t offset;
>>> size_t b_idx;
>>>
>>>-offset = *offsetp;
>>>-for (b_idx = *bucketp; b_idx <= hmap->mask; b_idx++) {
>>>+offset = pos->offset;
>>>+for (b_idx = pos->bucket; b_idx <= hmap->mask; b_idx++) {
>>> struct hmap_node *node;
>>> size_t n_idx;
>>>
>>>@@ -261,11 +259,11 @@ hmap_at_position(const struct hmap *hmap,
>>>  n_idx++, node = node->next) {
>>> if (n_idx == offset) {
>>> if (node->next) {
>>>-*bucketp = node->hash & hmap->mask;
>>>-*offsetp = offset + 1;
>>>+pos->bucket = node->hash & hmap->mask;
>>>+pos->offset = offset + 1;
>>> } else {
>>>-*bucketp = (node->hash & hmap->mask) + 1;
>>>-*offsetp = 0;
>>>+pos->bucket = (node->hash & hmap->mask) + 1;
>>>+pos->offset = 0;
>>> }
>>> return node;
>>> }
>>>@@ -273,8 +271,8 @@ hmap_at_position(const struct hmap *hmap,
>>> offset = 0;
>>> }
>>>
>>>-*bucketp = 0;
>>>-*offsetp = 0;
>>>+pos->bucket = 0;
>>>+pos->offset = 0;
>>> return NULL;
>>> }
>>>
>>>diff --git a/lib/hmap.h b/lib/hmap.h
>>>index 08c4719..9a96c5f 100644
>>>--- a/lib/hmap.h
>>>+++ b/lib/hmap.h
>>>@@ -201,8 +201,13 @@ static inline struct hmap_node *hmap_first(const
>>>struct hmap *);
>>> static inline struct hmap_node *hmap_next(const struct hmap *,
>>>   const struct hmap_node *);
>>>
>>>+struct hmap_position {
>>>+unsigned int bucket;
>>>+unsigned int offset;
>>>+};
>>>+
>>> struct hmap_node *hmap_at_position(const struct hmap *,
>>>-   uint32_t *bucket, uint32_t *offset);
>>>+   struct hmap_position *);
>>>
>>> /* Returns the number of nodes currently in 'hmap'. */
>>> static inline size_t
>>>diff --git a/lib/sset.c b/lib/sset.c
>>>index f9d4fc0..4fd3fae 100644
>>>--- a/lib/sset.c
>>>+++ b/lib/sset.c
>>>@@ -251,21 +251,19 @@ sset_equals(const struct sset *a, const struct
>>>sset *b)
>>> }
>>>
>>> /* Returns the next node in 'set' in hash order, or NULL if no nodes
>>>remain in
>>>- * 'set'.  Uses '*bucketp' and '*offsetp' to determine where to begin
>>>- * iteration, and stores new values to pass on the next iteration into
>>>them
>>>- * before returning.
>>>+ * 'set'.  Uses '*pos' to determine where to begin iteration, and
>>>updates
>>>+ * '*pos' to pass on the next iteration into them before returning.
>>>  *
>>>  * It's better to use plain SSET_FOR_EACH and related functions, since
>>>they are
>>>  * faster and better at dealing with ssets that change during iteration.
>>>  *
>>>- * Before beginning iteration, store 0 into '*bucketp' and '*offsetp'.
>>>- */
>>>+ * Before beginning iteration, set '*pos' to all zeros. */
>>> struct sset_node *
>>>-sset_at_position(const struct sset *set, uint32_t *bucketp, uint32_t
>>>*offsetp)
>>>+sset_at_position(const struct sset *set, struct sset_position *pos)
>>> {
>>> struct hmap_node *hmap_node;
>>>
>>>-hmap_node = hmap_at_position(>map, bucketp, offsetp);
>>>+hmap_node = hmap_at_position(>map, >pos);
>>> return 

[ovs-dev] [PATCH v4 1/1] ovn: Add column enabled to table Logical_Router

[JunoZhu]

This patch add column "enabled" to table Logical_Router for
 setting router administrative state.

The type of "enabled" is bool.

If the administrative state is false, delete all the flows
relevant to the logical router from table Logical_Flow.

Signed-off-by: Na Zhu 
---
 ovn/northd/ovn-northd.8.xml |   7 +++
 ovn/northd/ovn-northd.c |  10 +++
 ovn/ovn-nb.ovsschema|   5 +-
 ovn/ovn-nb.xml  |   7 +++
 tests/ovn.at| 144 
 5 files changed, 171 insertions(+), 2 deletions(-)

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index da776e1..a7ebda2 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -397,6 +397,13 @@ output;
 
 Logical Router Datapaths
 
+
+Logical router datapaths will only exist for  rows in the  database that 
do 
+not have  set 
+to false
+
+
 Ingress Table 0: L2 Admission Control
 
 
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 260c02f..e3436da 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -317,6 +317,12 @@ ovn_datapath_from_sbrec(struct hmap *datapaths,
 return ovn_datapath_find(datapaths, );
 }
 
+static bool
+lrouter_is_enabled(const struct nbrec_logical_router *lrouter)
+{
+return !lrouter->enabled || *lrouter->enabled;
+}
+
 static void
 join_datapaths(struct northd_context *ctx, struct hmap *datapaths,
struct ovs_list *sb_only, struct ovs_list *nb_only,
@@ -374,6 +380,10 @@ join_datapaths(struct northd_context *ctx, struct hmap 
*datapaths,
 
 const struct nbrec_logical_router *nbr;
 NBREC_LOGICAL_ROUTER_FOR_EACH (nbr, ctx->ovnnb_idl) {
+if (!lrouter_is_enabled(nbr)) {
+continue;
+}
+
 struct ovn_datapath *od = ovn_datapath_find(datapaths,
 >header_.uuid);
 if (od) {
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 40a7a97..e3e41e3 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
 "name": "OVN_Northbound",
-"version": "2.0.2",
-"cksum": "4289495412 4436",
+"version": "2.1.0",
+"cksum": "2201582413 4513",
 "tables": {
 "Logical_Switch": {
 "columns": {
@@ -72,6 +72,7 @@
"min": 0,
"max": "unlimited"}},
 "default_gw": {"type": {"key": "string", "min": 0, "max": 1}},
+"enabled": {"type": {"key": "boolean", "min": 0, "max": 1}},
 "external_ids": {
 "type": {"key": "string", "value": "string",
  "min": 0, "max": "unlimited"}}},
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index e65bc3a..843ae4c 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -627,6 +627,13 @@
   IP address to use as default gateway, if any.
 
 
+
+  This column is used to administratively set router state.  If this column
+  is empty or is set to true, the router is enabled.  If this
+  column is set to false, the router is disabled.  A disabled
+  router has all ingress and egress traffic dropped.
+
+
 
   
 See External IDs at the beginning of this document.
diff --git a/tests/ovn.at b/tests/ovn.at
index 6fea4e0..e5f50e8 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -2192,3 +2192,147 @@ OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 
 AT_CLEANUP
+
+
+AT_SETUP([ovn -- 1 HVs, 2 LSs, 1 lport/LS, 1 LR])
+AT_KEYWORDS([router-admin-state])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+
+# Logical network:
+# One LR - R1 has switch ls1 (191.168.1.0/24) connected to it,
+# and has switch ls2 (172.16.1.0/24) connected to it.
+
+ovn-nbctl create Logical_Router name=R1
+
+ovn-nbctl lswitch-add ls1
+ovn-nbctl lswitch-add ls2
+
+# Connect ls1 to R1
+ovn-nbctl -- --id=@lrp create Logical_Router_port name=ls1 \
+network=192.168.1.1/24 mac=\"00:00:00:01:02:03\" -- add Logical_Router R1 \
+ports @lrp -- lport-add ls1 rp-ls1
+
+ovn-nbctl set Logical_port rp-ls1 type=router options:router-port=ls1 \
+addresses=\"00:00:00:01:02:03\"
+
+# Connect ls2 to R1
+ovn-nbctl -- --id=@lrp create Logical_Router_port name=ls2 \
+network=172.16.1.1/24 mac=\"00:00:00:01:02:04\" -- add Logical_Router R1 \
+ports @lrp -- lport-add ls2 rp-ls2
+
+ovn-nbctl set Logical_port rp-ls2 type=router options:router-port=ls2 \
+addresses=\"00:00:00:01:02:04\"
+
+# Create logical port ls1-lp1 in ls1
+ovn-nbctl lport-add ls1 ls1-lp1 \
+-- lport-set-addresses ls1-lp1 "f0:00:00:01:02:03 192.168.1.2"
+
+# Create logical port ls2-lp1 in ls2
+ovn-nbctl lport-add ls2 ls2-lp1 \
+-- lport-set-addresses ls2-lp1 "f0:00:00:01:02:04 172.16.1.2"
+
+# Create one hypervisor and create OVS ports corresponding to logical ports.
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys

[ovs-dev] [PATCH V2, 3/3] datapath-windows: Removed always true condition in VXLAN

[Paul Boca]

Instance ID flag must be set to 1 in case of valid VXLAN id

Signed-off-by: Paul-Daniel Boca 
Acked-by: Sorin Vinturis 
---
 datapath-windows/ovsext/Vxlan.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c
index b89c032..20214cb 100644
--- a/datapath-windows/ovsext/Vxlan.c
+++ b/datapath-windows/ovsext/Vxlan.c
@@ -304,10 +304,8 @@ OvsDoEncapVxlan(POVS_VPORT_ENTRY vport,
 vxlanHdr->locallyReplicate = 0;
 vxlanHdr->flags2 = 0;
 vxlanHdr->reserved1 = 0;
-if (tunKey->flags | OVS_TNL_F_KEY) {
-vxlanHdr->vxlanID = VXLAN_TUNNELID_TO_VNI(tunKey->tunnelId);
-vxlanHdr->instanceID = 1;
-}
+vxlanHdr->vxlanID = VXLAN_TUNNELID_TO_VNI(tunKey->tunnelId);
+vxlanHdr->instanceID = 1;
 vxlanHdr->reserved2 = 0;
 }
 return STATUS_SUCCESS;
-- 
2.7.2.windows.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH V2, 2/3] datapath-windows: Removed double initialization on local variables

[Paul Boca]

Signed-off-by: Paul-Daniel Boca 
Acked-by: Sorin Vinturis 
---
 datapath-windows/ovsext/Actions.c | 10 +-
 datapath-windows/ovsext/Flow.c|  2 +-
 datapath-windows/ovsext/IpHelper.c| 10 +-
 datapath-windows/ovsext/Netlink/Netlink.c |  2 +-
 datapath-windows/ovsext/Oid.c |  2 +-
 datapath-windows/ovsext/PacketParser.c|  1 -
 datapath-windows/ovsext/Tunnel.c  |  5 ++---
 datapath-windows/ovsext/TunnelFilter.c| 12 ++--
 8 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index cf54ae2..5dae6b4 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -983,7 +983,7 @@ static __inline NDIS_STATUS
 OvsOutputBeforeSetAction(OvsForwardingContext *ovsFwdCtx)
 {
 PNET_BUFFER_LIST newNbl;
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+NDIS_STATUS status;
 
 /*
  * Create a copy and work on the copy after this point. The original NBL is
@@ -1142,7 +1142,7 @@ static __inline NDIS_STATUS
 OvsActionMplsPop(OvsForwardingContext *ovsFwdCtx,
  ovs_be16 ethertype)
 {
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+NDIS_STATUS status;
 OVS_PACKET_HDR_INFO *layers = >layers;
 EthHdr *ethHdr = NULL;
 
@@ -1945,7 +1945,7 @@ OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext,
   const PNL_ATTR actions,
   INT actionsLen)
 {
-NDIS_STATUS status = STATUS_SUCCESS;
+NDIS_STATUS status;
 
 status = OvsDoExecuteActions(switchContext, completionList, curNbl,
  portNo, sendFlags, key, hash, layers,
@@ -1974,8 +1974,8 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
 UINT32 srcPortNo,
 OVS_PACKET_HDR_INFO *layers)
 {
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
-OvsFlow *flow = NULL;
+NDIS_STATUS status;
+OvsFlow *flow;
 OvsForwardingContext ovsFwdCtx = { 0 };
 UINT64 hash = 0;
 ASSERT(layers);
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index a7e9bd2..1f23625 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -723,7 +723,7 @@ done:
 static NTSTATUS
 _MapFlowInfoToNl(PNL_BUFFER nlBuf, OvsFlowInfo *flowInfo)
 {
-NTSTATUS rc = STATUS_SUCCESS;
+NTSTATUS rc;
 
 rc = MapFlowKeyToNlKey(nlBuf, &(flowInfo->key), OVS_FLOW_ATTR_KEY,
OVS_KEY_ATTR_TUNNEL);
diff --git a/datapath-windows/ovsext/IpHelper.c 
b/datapath-windows/ovsext/IpHelper.c
index 8126222..3b42610 100644
--- a/datapath-windows/ovsext/IpHelper.c
+++ b/datapath-windows/ovsext/IpHelper.c
@@ -231,7 +231,7 @@ OvsGetIPEntry(NET_LUID interfaceLuid,
 NTSTATUS status;
 UINT32 i;
 
-if (ipEntry == NULL || ipEntry == NULL) {
+if (ipEntry == NULL) {
 return STATUS_INVALID_PARAMETER;
 }
 
@@ -1209,15 +1209,15 @@ static VOID
 OvsHandleFwdRequest(POVS_IP_HELPER_REQUEST request)
 {
 SOCKADDR_INET dst, src;
-NTSTATUS status = STATUS_SUCCESS;
+NTSTATUS status;
 MIB_IPFORWARD_ROW2 ipRoute;
 MIB_IPNET_ROW2 ipNeigh;
 OVS_FWD_INFO fwdInfo;
 UINT32 ipAddr;
 UINT32 srcAddr;
-POVS_FWD_ENTRY fwdEntry = NULL;
-POVS_IPFORWARD_ENTRY ipf = NULL;
-POVS_IPNEIGH_ENTRY ipn = NULL;
+POVS_FWD_ENTRY fwdEntry;
+POVS_IPFORWARD_ENTRY ipf;
+POVS_IPNEIGH_ENTRY ipn;
 LOCK_STATE_EX lockState;
 BOOLEAN  newIPF = FALSE;
 BOOLEAN  newIPN = FALSE;
diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index e2312da..27dcd4f 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -565,7 +565,7 @@ NlMsgPutNested(PNL_BUFFER buf, UINT16 type,
const PVOID data, UINT32 size)
 {
 UINT32 offset = NlMsgStartNested(buf, type);
-BOOLEAN ret = FALSE;
+BOOLEAN ret;
 
 ASSERT(offset);
 
diff --git a/datapath-windows/ovsext/Oid.c b/datapath-windows/ovsext/Oid.c
index 93894ae..7c7ffe7 100644
--- a/datapath-windows/ovsext/Oid.c
+++ b/datapath-windows/ovsext/Oid.c
@@ -366,7 +366,7 @@ OvsExtOidRequest(NDIS_HANDLE filterModuleContext,
  PNDIS_OID_REQUEST oidRequest)
 {
 POVS_SWITCH_CONTEXT switchObject = 
(POVS_SWITCH_CONTEXT)filterModuleContext;
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+NDIS_STATUS status;
 PNDIS_OID_REQUEST clonedOidRequest = NULL;
 struct _METHOD *methodInfo = &(oidRequest->DATA.METHOD_INFORMATION);
 BOOLEAN completeOid = FALSE;
diff --git a/datapath-windows/ovsext/PacketParser.c 
b/datapath-windows/ovsext/PacketParser.c
index bd6910c..93df342 100644
--- a/datapath-windows/ovsext/PacketParser.c
+++ b/datapath-windows/ovsext/PacketParser.c
@@ -93,7 +93,6 @@ OvsParseIPv6(const NET_BUFFER_LIST *packet,
 UINT32 nextHdr;
 Ipv6Key *flow= 

[ovs-dev] [PATCH V2, 1/3] datapath-windows: Avoid using uninitialized gOvsExtDriverHandle

[Paul Boca]

Ensure gOvsExtDriverHandle is not used if initialization fails
Added PAGED_CODE() where needed

Signed-off-by: Paul-Daniel Boca 
Acked-by: Sorin Vinturis 
---
 datapath-windows/ovsext/Datapath.c | 4 +++-
 datapath-windows/ovsext/Driver.c   | 9 +
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 0a25af0..06f99b3 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -616,6 +616,7 @@ OvsOpenCloseDevice(PDEVICE_OBJECT deviceObject,
 POVS_DEVICE_EXTENSION ovsExt =
 (POVS_DEVICE_EXTENSION)NdisGetDeviceReservedExtension(deviceObject);
 
+PAGED_CODE();
 ASSERT(deviceObject == gOvsDeviceObject);
 ASSERT(ovsExt != NULL);
 
@@ -648,7 +649,7 @@ NTSTATUS
 OvsCleanupDevice(PDEVICE_OBJECT deviceObject,
  PIRP irp)
 {
-
+PAGED_CODE();
 PIO_STACK_LOCATION irpSp;
 PFILE_OBJECT fileObject;
 
@@ -696,6 +697,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
 NETLINK_FAMILY *nlFamilyOps;
 OVS_USER_PARAMS_CONTEXT usrParamsCtx;
 
+PAGED_CODE();
 #ifdef DBG
 POVS_DEVICE_EXTENSION ovsExt =
 (POVS_DEVICE_EXTENSION)NdisGetDeviceReservedExtension(deviceObject);
diff --git a/datapath-windows/ovsext/Driver.c b/datapath-windows/ovsext/Driver.c
index 80979ea..50c9614 100644
--- a/datapath-windows/ovsext/Driver.c
+++ b/datapath-windows/ovsext/Driver.c
@@ -60,6 +60,8 @@ static const GUID ovsExtGuid = {
   {0x8b, 0x47, 0x57, 0x82, 0x97, 0xad, 0x76, 0x23}
 };
 
+DRIVER_INITIALIZE DriverEntry;
+
 /* Declarations of callback functions for the filter driver. */
 DRIVER_UNLOAD OvsExtUnload;
 FILTER_NET_PNP_EVENT OvsExtNetPnPEvent;
@@ -141,6 +143,7 @@ DriverEntry(PDRIVER_OBJECT driverObject,
 
 driverObject->DriverUnload = OvsExtUnload;
 
+gOvsExtDriverHandle = NULL;
 status = NdisFRegisterFilterDriver(driverObject,
(NDIS_HANDLE)gOvsExtDriverObject,
,
@@ -152,16 +155,14 @@ DriverEntry(PDRIVER_OBJECT driverObject,
 /* Create the communication channel for userspace. */
 status = OvsCreateDeviceObject(gOvsExtDriverHandle);
 if (status != NDIS_STATUS_SUCCESS) {
+NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
+gOvsExtDriverHandle = NULL;
 goto cleanup;
 }
 
 cleanup:
 if (status != NDIS_STATUS_SUCCESS){
 OvsCleanup();
-if (gOvsExtDriverHandle) {
-NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
-gOvsExtDriverHandle = NULL;
-}
 }
 
 return status;
-- 
2.7.2.windows.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] datapath-windows: Fixed buffer overflow in OvsInitVportWithNicParam

[Paul Boca]

nicParam->PermanentMacAddress is 32 bytes and vport->permMacAddress is 6 bytes

Signed-off-by: Paul-Daniel Boca 
---
 datapath-windows/ovsext/DpInternal.h | 6 +++---
 datapath-windows/ovsext/Vport.h  | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/datapath-windows/ovsext/DpInternal.h 
b/datapath-windows/ovsext/DpInternal.h
index a3ce311..760552d 100644
--- a/datapath-windows/ovsext/DpInternal.h
+++ b/datapath-windows/ovsext/DpInternal.h
@@ -41,9 +41,9 @@ typedef struct _OVS_VPORT_GET {
 typedef struct _OVS_VPORT_EXT_INFO {
 uint32_t dpNo;
 uint32_t portNo;
-uint8_t macAddress[ETH_ADDR_LEN];
-uint8_t permMACAddress[ETH_ADDR_LEN];
-uint8_t vmMACAddress[ETH_ADDR_LEN];
+uint8_t macAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
+uint8_t permMACAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
+uint8_t vmMACAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
 uint16_t nicIndex;
 uint32_t portId;
 uint32_t type;
diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h
index 373896d..3f18eb1 100644
--- a/datapath-windows/ovsext/Vport.h
+++ b/datapath-windows/ovsext/Vport.h
@@ -102,9 +102,9 @@ typedef struct _OVS_VPORT_ENTRY {
 NDIS_SWITCH_NIC_STATE  nicState;
 NDIS_SWITCH_PORT_TYPE  portType;
 
-UINT8  permMacAddress[ETH_ADDR_LEN];
-UINT8  currMacAddress[ETH_ADDR_LEN];
-UINT8  vmMacAddress[ETH_ADDR_LEN];
+UINT8  permMacAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
+UINT8  currMacAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
+UINT8  vmMacAddress[NDIS_MAX_PHYS_ADDRESS_LENGTH];
 
 NDIS_SWITCH_PORT_NAME  hvPortName;
 IF_COUNTED_STRING  portFriendlyName;
-- 
2.7.2.windows.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always true condition in VXLAN

[Sorin Vinturis]

Acked-by: Sorin Vinturis 

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Paul Boca
Sent: Friday, 15 April, 2016 18:05
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH 3/3] datapath-windows: Removed always true condition 
in VXLAN

Instance ID flag must be set to 1 in case of valid VXLAN id

Signed-off-by: Paul-Daniel Boca 
---
 datapath-windows/ovsext/Vxlan.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c 
index b89c032..20214cb 100644
--- a/datapath-windows/ovsext/Vxlan.c
+++ b/datapath-windows/ovsext/Vxlan.c
@@ -304,10 +304,8 @@ OvsDoEncapVxlan(POVS_VPORT_ENTRY vport,
 vxlanHdr->locallyReplicate = 0;
 vxlanHdr->flags2 = 0;
 vxlanHdr->reserved1 = 0;
-if (tunKey->flags | OVS_TNL_F_KEY) {
-vxlanHdr->vxlanID = VXLAN_TUNNELID_TO_VNI(tunKey->tunnelId);
-vxlanHdr->instanceID = 1;
-}
+vxlanHdr->vxlanID = VXLAN_TUNNELID_TO_VNI(tunKey->tunnelId);
+vxlanHdr->instanceID = 1;
 vxlanHdr->reserved2 = 0;
 }
 return STATUS_SUCCESS;
--
2.7.2.windows.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/3] datapath-windows: Removed double initialization on local variables

[Sorin Vinturis]

I had a few minor comments inline, but the patch looks good.

Acked-by: Sorin Vinturis 

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Paul Boca
Sent: Friday, 15 April, 2016 18:05
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH 2/3] datapath-windows: Removed double initialization 
on local variables

Signed-off-by: Paul-Daniel Boca 
---
 datapath-windows/ovsext/Actions.c |  8 
 datapath-windows/ovsext/Flow.c|  2 +-
 datapath-windows/ovsext/IpHelper.c|  4 ++--
 datapath-windows/ovsext/Netlink/Netlink.c |  2 +-
 datapath-windows/ovsext/Oid.c |  2 +-
 datapath-windows/ovsext/PacketParser.c|  1 -
 datapath-windows/ovsext/Tunnel.c  |  2 +-
 datapath-windows/ovsext/TunnelFilter.c| 12 ++--
 8 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index cf54ae2..7e45db7 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -983,7 +983,7 @@ static __inline NDIS_STATUS  
OvsOutputBeforeSetAction(OvsForwardingContext *ovsFwdCtx)  {
 PNET_BUFFER_LIST newNbl;
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+NDIS_STATUS status;
 
 /*
  * Create a copy and work on the copy after this point. The original NBL 
is @@ -1142,7 +1142,7 @@ static __inline NDIS_STATUS  
OvsActionMplsPop(OvsForwardingContext *ovsFwdCtx,
  ovs_be16 ethertype)
 {
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+NDIS_STATUS status;
 OVS_PACKET_HDR_INFO *layers = >layers;
 EthHdr *ethHdr = NULL;
 
@@ -1945,7 +1945,7 @@ OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext,
   const PNL_ATTR actions,
   INT actionsLen)
 {
-NDIS_STATUS status = STATUS_SUCCESS;
+NDIS_STATUS status;
 
 status = OvsDoExecuteActions(switchContext, completionList, curNbl,
  portNo, sendFlags, key, hash, layers, @@ 
-1974,7 +1974,7 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
 UINT32 srcPortNo,
 OVS_PACKET_HDR_INFO *layers)  {
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+NDIS_STATUS status;
 OvsFlow *flow = NULL;

SV: You could do the same with the 'flow' pointer.

 OvsForwardingContext ovsFwdCtx = { 0 };
 UINT64 hash = 0;
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c 
index a7e9bd2..1f23625 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -723,7 +723,7 @@ done:
 static NTSTATUS
 _MapFlowInfoToNl(PNL_BUFFER nlBuf, OvsFlowInfo *flowInfo)  {
-NTSTATUS rc = STATUS_SUCCESS;
+NTSTATUS rc;
 
 rc = MapFlowKeyToNlKey(nlBuf, &(flowInfo->key), OVS_FLOW_ATTR_KEY,
OVS_KEY_ATTR_TUNNEL); diff --git 
a/datapath-windows/ovsext/IpHelper.c b/datapath-windows/ovsext/IpHelper.c
index 8126222..d747e8c 100644
--- a/datapath-windows/ovsext/IpHelper.c
+++ b/datapath-windows/ovsext/IpHelper.c
@@ -231,7 +231,7 @@ OvsGetIPEntry(NET_LUID interfaceLuid,
 NTSTATUS status;
 UINT32 i;
 
-if (ipEntry == NULL || ipEntry == NULL) {
+if (ipEntry == NULL) {
 return STATUS_INVALID_PARAMETER;
 }
 
@@ -1209,7 +1209,7 @@ static VOID
 OvsHandleFwdRequest(POVS_IP_HELPER_REQUEST request)  {
 SOCKADDR_INET dst, src;
-NTSTATUS status = STATUS_SUCCESS;
+NTSTATUS status;

SV: The same applies for 'fwdEntry', 'ipf' and 'ipn'.

 MIB_IPFORWARD_ROW2 ipRoute;
 MIB_IPNET_ROW2 ipNeigh;
 OVS_FWD_INFO fwdInfo;
diff --git a/datapath-windows/ovsext/Netlink/Netlink.c 
b/datapath-windows/ovsext/Netlink/Netlink.c
index e2312da..27dcd4f 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -565,7 +565,7 @@ NlMsgPutNested(PNL_BUFFER buf, UINT16 type,
const PVOID data, UINT32 size)  {
 UINT32 offset = NlMsgStartNested(buf, type);
-BOOLEAN ret = FALSE;
+BOOLEAN ret;
 
 ASSERT(offset);
 
diff --git a/datapath-windows/ovsext/Oid.c b/datapath-windows/ovsext/Oid.c 
index 93894ae..7c7ffe7 100644
--- a/datapath-windows/ovsext/Oid.c
+++ b/datapath-windows/ovsext/Oid.c
@@ -366,7 +366,7 @@ OvsExtOidRequest(NDIS_HANDLE filterModuleContext,
  PNDIS_OID_REQUEST oidRequest)  {
 POVS_SWITCH_CONTEXT switchObject = 
(POVS_SWITCH_CONTEXT)filterModuleContext;
-NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+NDIS_STATUS status;
 PNDIS_OID_REQUEST clonedOidRequest = NULL;
 struct _METHOD *methodInfo = &(oidRequest->DATA.METHOD_INFORMATION);
 BOOLEAN completeOid = FALSE;
diff --git a/datapath-windows/ovsext/PacketParser.c 
b/datapath-windows/ovsext/PacketParser.c
index bd6910c..93df342 100644
--- a/datapath-windows/ovsext/PacketParser.c
+++ b/datapath-windows/ovsext/PacketParser.c
@@ -93,7 +93,6 @@ OvsParseIPv6(const NET_BUFFER_LIST 

Re: [ovs-dev] [PATCH 1/3] datapath-windows: Avoid using uninitialized gOvsExtDriverHandle

[Sorin Vinturis]

Acked-by: Sorin Vinturis 

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Paul Boca
Sent: Saturday, 16 April, 2016 00:59
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH 1/3] datapath-windows: Avoid using uninitialized 
gOvsExtDriverHandle

Ensure gOvsExtDriverHandle is not used if initialization fails Added 
PAGED_CODE() where needed

Signed-off-by: Paul-Daniel Boca 
---
 datapath-windows/ovsext/Datapath.c | 4 +++-
 datapath-windows/ovsext/Driver.c   | 9 +
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 0a25af0..06f99b3 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -616,6 +616,7 @@ OvsOpenCloseDevice(PDEVICE_OBJECT deviceObject,
 POVS_DEVICE_EXTENSION ovsExt =
 (POVS_DEVICE_EXTENSION)NdisGetDeviceReservedExtension(deviceObject);
 
+PAGED_CODE();
 ASSERT(deviceObject == gOvsDeviceObject);
 ASSERT(ovsExt != NULL);
 
@@ -648,7 +649,7 @@ NTSTATUS
 OvsCleanupDevice(PDEVICE_OBJECT deviceObject,
  PIRP irp)
 {
-
+PAGED_CODE();
 PIO_STACK_LOCATION irpSp;
 PFILE_OBJECT fileObject;
 
@@ -696,6 +697,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
 NETLINK_FAMILY *nlFamilyOps;
 OVS_USER_PARAMS_CONTEXT usrParamsCtx;
 
+PAGED_CODE();
 #ifdef DBG
 POVS_DEVICE_EXTENSION ovsExt =
 (POVS_DEVICE_EXTENSION)NdisGetDeviceReservedExtension(deviceObject);
diff --git a/datapath-windows/ovsext/Driver.c b/datapath-windows/ovsext/Driver.c
index 80979ea..50c9614 100644
--- a/datapath-windows/ovsext/Driver.c
+++ b/datapath-windows/ovsext/Driver.c
@@ -60,6 +60,8 @@ static const GUID ovsExtGuid = {
   {0x8b, 0x47, 0x57, 0x82, 0x97, 0xad, 0x76, 0x23}  };
 
+DRIVER_INITIALIZE DriverEntry;
+
 /* Declarations of callback functions for the filter driver. */  DRIVER_UNLOAD 
OvsExtUnload;  FILTER_NET_PNP_EVENT OvsExtNetPnPEvent; @@ -141,6 +143,7 @@ 
DriverEntry(PDRIVER_OBJECT driverObject,
 
 driverObject->DriverUnload = OvsExtUnload;
 
+gOvsExtDriverHandle = NULL;
 status = NdisFRegisterFilterDriver(driverObject,
(NDIS_HANDLE)gOvsExtDriverObject,
, @@ -152,16 +155,14 @@ 
DriverEntry(PDRIVER_OBJECT driverObject,
 /* Create the communication channel for userspace. */
 status = OvsCreateDeviceObject(gOvsExtDriverHandle);
 if (status != NDIS_STATUS_SUCCESS) {
+NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
+gOvsExtDriverHandle = NULL;
 goto cleanup;
 }
 
 cleanup:
 if (status != NDIS_STATUS_SUCCESS){
 OvsCleanup();
-if (gOvsExtDriverHandle) {
-NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
-gOvsExtDriverHandle = NULL;
-}
 }
 
 return status;
--
2.7.2.windows.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] Add configurable OpenFlow port name.

[Xiao Liang]

By introducing of_name, ovs_name serves as a key of ports which is
shared by ofproto and netdev. It's easier to find and convert ports
back and forth. of_name and kernel_name could be configured (if
supported) independently of each other.

On Mon, Apr 18, 2016 at 11:43 AM, Takashi YAMAMOTO  wrote:
> let me explain what netdev-bsd does first.
> on some platform "tap" interfaces are always named automatically by kernel
> itself
> and there's no way to rename them.  say, they always will have names like
> "tap0".
> so if you does "ovs-vsctl add-port br0 foo",
>   ovs_name = "foo"
>   kernel_name = "tap0"
>
> now, you are going to add another name for openflow. let's call it of_name.
> eg. "ovs-vsctl add-port br0 foo -- set int foo ofname=wan",
>   of_name = "wan"
>   ovs_name = "foo"
>   kernel_name = "tap0"
>
> while i don't have strong opinions either ways,
> i'm not sure why you want to use different names for of_name and ovs_name
> in the first place.  eg. what's wrong with "ovs-vsctl add-port br0 wan".
> can you explain a little?
>
> On Mon, Apr 18, 2016 at 10:37 AM, Xiao Liang  wrote:
>>
>> Hi Ben, Yamamoto-san,
>>
>> Kindly remind you of this thread. Would like to hear your preference
>> on the way to implement this feature.
>>
>> On Mon, Apr 11, 2016 at 11:18 PM, Ben Pfaff  wrote:
>> > On Mon, Apr 11, 2016 at 04:30:04PM +0800, Xiao Liang wrote:
>> >> On Mon, Apr 11, 2016 at 3:42 PM, Takashi Yamamoto
>> >>  wrote:
>> >> > hi,
>> >> >
>> >> > have you considered the opposite way?
>> >> > ie. have an ability to specify the device name.
>> >> >
>> >> > netdev-bsd already has a distinction between "kernel name" and "ovs
>> >> > name".
>> >> >
>> >>
>> >> Hi,
>> >>
>> >> I'm not familiar with netdev-bsd code, but I think this approach will
>> >> make ports more difficult to manage and need much more effort.
>> >
>> > Yamamoto-san: thanks for bringing this up.  I'm going to wait for you
>> > and Xiao to talk this through a bit before continuing review.
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Returned mail: see transcript for details

[Automatic Email Delivery Software]

Dear user of openvswitch.org,

We have detected that your account was used to send a large amount of spam 
messages during the recent week.
Most likely your computer had been infected and now runs a trojaned proxy 
server.

We recommend that you follow the instructions in order to keep your computer 
safe.

Best wishes,
openvswitch.org user support team.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev