Re: [ovs-dev] [RFC PATCHv2] ofp-actions: Add clone action.

[William Tu]

On Thu, Dec 1, 2016 at 6:25 PM, Gao Zhenyu  wrote:
> Thanks for working on it!
>
> Can we have nested clone?
>
> actions=clone(push_vlan(1), resubmit(1,2), clone(mod_dl_src:,
> output:5) )
>

I tested it for a couple of simple cases, it seems ok. But it might
complicate things a lot.

> BTW, I believe the port 3 will get a 104byte size packet with this action,
> right?
> actions= clone(truncate(100), push_vlan, output:3)
>

Yes.

Regards,
William

>
> Thanks
> Zhenyu Gao
>
> 2016-12-01 5:35 GMT+08:00 William Tu :
>>
>> This patch adds OpenFlow clone action with syntax as below:
>> "clone([action][,action...])".  The clone() action makes a copy of the
>> current packet and executes the list of actions against the packet,
>> without affecting the packet after the "clone(...)" action.  In other
>> word, the packet before the clone() and after the clone() is the same,
>> no matter what actions executed inside the clone().
>>
>> The patch reuses the dapatah SAMPLE action, with probability of 100%.
>> The kernel datapath 'sample()' clones the skb and its flow key under this
>> circumstance, and actions specified in the clone() are executed against
>> the cloned skb.  Note that there is no performance overhead of clone,
>> since
>> if eventually the packet is output, it will also clone the packet.  Here
>> we
>> simply move the copy at the beginning.
>>
>> The complexity of adding clone might outweight its use cases.  I'm looking
>> for comments as well as listing some cases below:
>> Use case 1:
>> Set different fields and output to different ports without unset
>> actions=
>>   clone(mod_dl_src:, output:1), clone(mod_dl_dst:, output:2),
>> output:3
>> Since each clone() has independent packet, output:1 has only dl_src
>> modified,
>> output:2 has only dl_dst modified, output:3 has original packet.
>>
>> Similar to case1
>> actions=
>>   push_vlan(...), output:2, pop_vlan, push_vlan(...), output:3
>> can be changed to
>> actions=
>>   clone(push_vlan(...), output:2),clone(push_vlan(...), output:3)
>> without having to add pop_vlan.
>>
>> case 2: resubmit to another table without worrying packet being modified
>>   actions=clone(resubmit(1,2)), ...
>>
>> case 3: truncate in the clone action
>> Currently OVS can truncate packet by 'output(port=1,max_len=100)', which
>> ties truncate and output together.  However, sometimes the layer decides
>> to truncate is separate from the layer to output.  One proposal is to
>> introduce actions s.t like truncate_set() and truncate_unset(), where
>> only the action in between sees truncated packet.  Another approach is
>> to use clone() as below:
>> actions=
>>   clone(truncate(100), push_vlan, resubmit, ...)
>> where we don't need to worry about missing the truncate_unset() because
>> truncated packet is not visible outside the clone().
>>
>> We definitely should put some limit on the action types available inside
>> clone(). For this patch, there is no restriction.
>>
>> Signed-off-by: William Tu 
>> ---
>> v1->v2
>> - rebase and change NX1.0+(41) to 42
>> ---
>>  include/openvswitch/ofp-actions.h |  23 +
>>  lib/ofp-actions.c | 101
>> +-
>>  ofproto/ofproto-dpif-xlate.c  |  35 +
>>  tests/ofproto-dpif.at |  19 +++
>>  tests/system-traffic.at   |  29 +++
>>  5 files changed, 206 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/openvswitch/ofp-actions.h
>> b/include/openvswitch/ofp-actions.h
>> index 2999261..be4a991 100644
>> --- a/include/openvswitch/ofp-actions.h
>> +++ b/include/openvswitch/ofp-actions.h
>> @@ -109,6 +109,7 @@
>>  OFPACT(CT,  ofpact_conntrack,   ofpact, "ct")   \
>>  OFPACT(NAT, ofpact_nat, ofpact, "nat")  \
>>  OFPACT(OUTPUT_TRUNC,ofpact_output_trunc,ofpact, "output_trunc") \
>> +OFPACT(CLONE,   ofpact_clone,   ofpact, "clone")\
>>  \
>>  /* Debugging actions.   \
>>   *  \
>> @@ -868,6 +869,28 @@ struct ofpact_sample {
>>  enum nx_action_sample_direction direction;
>>  };
>>
>> +/* OFPACT_CLONE.
>> + *
>> + * Used for cloning the packet and execute actions
>> + * in the clone envelope. */
>> +struct ofpact_clone {
>> +OFPACT_PADDED_MEMBERS(
>> +struct ofpact ofpact;
>> +);
>> +struct ofpact actions[0];
>> +};
>> +
>> +BUILD_ASSERT_DECL(offsetof(struct ofpact_clone, actions)
>> +  % OFPACT_ALIGNTO == 0);
>> +BUILD_ASSERT_DECL(offsetof(struct ofpact_clone, actions)
>> +  == sizeof(struct ofpact_clone));
>> +
>> +static inline size_t
>> +ofpact_clone_get_action_len(const struct ofpact_clone *oc)
>> +{
>> +return oc->ofpact.len - offsetof(struct 

Re: [ovs-dev] [patch_v8] ovn: Add datapaths of interest filtering.

[Darrell Ball]

On 11/30/16, 3:07 AM, "ovs-dev-boun...@openvswitch.org on behalf of Liran 
Schour"  wrote:

Darrell Ball  wrote on 30/11/2016 02:29:01 AM:

> On Tue, Nov 29, 2016 at 4:10 AM, Liran Schour  wrote:
> Ben Pfaff  wrote on 29/11/2016 12:51:51 AM:
> 
> > I hope that this version is ready to go in.  Liran, are you happy with
> > this version?
> > 
> 
> I did some short evaluation and got the following results:
> Simulate 50 hosts, each host serves 20 logical ports, each logical 
> network is spread over 6 of the 50 hosts.
> 
> Thanks for testing Liran
> 
> Can a few questions be asked so the differences are understood.
> There is expectation that not monitoring logical ports will make a 
> difference, but
> this large is not expected.
> 
> 1) This topology seems to only use logical switches in a flat 
> topology for each tenant;
> is that correct ?

Right. We do not have any logical routers.

> 2)  What is the distribution of logical switches per HV ?

You have 20 logical ports from 20 different logical switches per HV.

Without some incremental processing, port processing which typically should not 
change
that often becomes more important.
Without port conditional monitoring, each HV will monitor 1000 ports vs 20 in 
this test.
I reinstated part of my V3 patch for ports earlier than later since we are in 
between 
incremental processing versions and port conditional monitoring becomes more
important to do earlier, afais.

> 3) What is the total number of logical switches we end up with ?

In that specific test: 167 logical switches.

> 
>  
> 
> 
> Darrell patch:
> --
> Host 1 # flows 1504
> Host 2 # flows 1626
> Host 3 # flows 1443
> ...
> Logical flows = 8016
> Elapsed time 207,990ms
> total of 1002 (in 208ms per port)
> Ports created and bound in 5,648,773,788,428 cycles
> 1% south 69,936,025,644
> 1% north 77,863,484,163
> 97% clients 5,500,974,278,621
> 
> Conditional monitor V16 patch:
> --
> Host 1 # flows 1358
> Host 2 # flows 1482
> Host 3 # flows 1296
> ...
> Logical flows = 8016
> Elapsed time 207,998ms
> total of 1002 (in 208ms per port)
> Ports created and bound in 1,216,848,309,516 cycles
> 
>  
> 
> 4) This is only the CPU time to initially create and bind logical 
> ports to each HV ?
>  How exactly is this time measured ?

You can ignore the time parameter, it is a trace from the past of the 
evaluation system.

My main question was - what is the defined start and end points ?
i.e. when do we start measuring and when do we stop measuring ?

Some of these numbers are really hard to explain.
Hence, I added a separate simple unit test to simply track number of events 
(port and flow) seen by the HVs with conditional monitoring. This is also
easier to interpret and straightforward.


> 5) This total cycles is in the trillions 
> The previous tests from V10 and V15 below seem consistent and in the 2 
digit
> billions, which is at least 30 times less than now.
> Is this same test as before using the 50 hosts configuration  ?
> 

The number that is shown in the tables below is total number of cycles of 
the SB ovsdb-server not overall cycles of all the DC. And as you can see 
(2% south 30,653,838,360) is still in the 2 digit billions.

I see, ok.
Most of the proportional benefit will be on the HV clients anyways.

> Evaluation on simulated environment of 50 hosts and 1000 logical ports 
shows
> the following results (cycles #):
> LN spread over # hosts|master| patch| change
> -
> 1 | 24597200127  | 24339235374  |  1.0%
> 6 | 23788521572  | 19145229352  | 19.5%
>12 | 23886405758  | 17913143176  | 25.0%
>18 | 25812686279  | 23675094540  |  8.2%
>24 | 28414671499  | 24770202308  | 12.8%
>30 | 31487218890  | 28397543436  |  9.8%
>36 | 36116993930  | 34105388739  |  5.5%
>42 | 37898342465  | 38647139083  | -1.9%
>48 | 41637996229  | 41846616306  | -0.5%
>50 | 41679995357  | 43455565977  | -4.2%
> 
> and from V15
> 
> Evaluation on simulated environment of 50 hosts and 1000 logical ports 
shows
> the following results (cycles #):
> 
> LN spread over # hosts|master| patch| change
> 

Re: [ovs-dev] [RFC PATCHv2] ofp-actions: Add clone action.

[Gao Zhenyu]

Thanks for working on it!

Can we have nested clone?

actions=clone(push_vlan(1), resubmit(1,2), clone(mod_dl_src:,
output:5) )

BTW, I believe the port 3 will get a 104byte size packet with this action,
right?
actions= clone(truncate(100), push_vlan, output:3)


Thanks
Zhenyu Gao

2016-12-01 5:35 GMT+08:00 William Tu :

> This patch adds OpenFlow clone action with syntax as below:
> "clone([action][,action...])".  The clone() action makes a copy of the
> current packet and executes the list of actions against the packet,
> without affecting the packet after the "clone(...)" action.  In other
> word, the packet before the clone() and after the clone() is the same,
> no matter what actions executed inside the clone().
>
> The patch reuses the dapatah SAMPLE action, with probability of 100%.
> The kernel datapath 'sample()' clones the skb and its flow key under this
> circumstance, and actions specified in the clone() are executed against
> the cloned skb.  Note that there is no performance overhead of clone, since
> if eventually the packet is output, it will also clone the packet.  Here we
> simply move the copy at the beginning.
>
> The complexity of adding clone might outweight its use cases.  I'm looking
> for comments as well as listing some cases below:
> Use case 1:
> Set different fields and output to different ports without unset
> actions=
>   clone(mod_dl_src:, output:1), clone(mod_dl_dst:, output:2),
> output:3
> Since each clone() has independent packet, output:1 has only dl_src
> modified,
> output:2 has only dl_dst modified, output:3 has original packet.
>
> Similar to case1
> actions=
>   push_vlan(...), output:2, pop_vlan, push_vlan(...), output:3
> can be changed to
> actions=
>   clone(push_vlan(...), output:2),clone(push_vlan(...), output:3)
> without having to add pop_vlan.
>
> case 2: resubmit to another table without worrying packet being modified
>   actions=clone(resubmit(1,2)), ...
>
> case 3: truncate in the clone action
> Currently OVS can truncate packet by 'output(port=1,max_len=100)', which
> ties truncate and output together.  However, sometimes the layer decides
> to truncate is separate from the layer to output.  One proposal is to
> introduce actions s.t like truncate_set() and truncate_unset(), where
> only the action in between sees truncated packet.  Another approach is
> to use clone() as below:
> actions=
>   clone(truncate(100), push_vlan, resubmit, ...)
> where we don't need to worry about missing the truncate_unset() because
> truncated packet is not visible outside the clone().
>
> We definitely should put some limit on the action types available inside
> clone(). For this patch, there is no restriction.
>
> Signed-off-by: William Tu 
> ---
> v1->v2
> - rebase and change NX1.0+(41) to 42
> ---
>  include/openvswitch/ofp-actions.h |  23 +
>  lib/ofp-actions.c | 101 ++
> +++-
>  ofproto/ofproto-dpif-xlate.c  |  35 +
>  tests/ofproto-dpif.at |  19 +++
>  tests/system-traffic.at   |  29 +++
>  5 files changed, 206 insertions(+), 1 deletion(-)
>
> diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-
> actions.h
> index 2999261..be4a991 100644
> --- a/include/openvswitch/ofp-actions.h
> +++ b/include/openvswitch/ofp-actions.h
> @@ -109,6 +109,7 @@
>  OFPACT(CT,  ofpact_conntrack,   ofpact, "ct")   \
>  OFPACT(NAT, ofpact_nat, ofpact, "nat")  \
>  OFPACT(OUTPUT_TRUNC,ofpact_output_trunc,ofpact, "output_trunc") \
> +OFPACT(CLONE,   ofpact_clone,   ofpact, "clone")\
>  \
>  /* Debugging actions.   \
>   *  \
> @@ -868,6 +869,28 @@ struct ofpact_sample {
>  enum nx_action_sample_direction direction;
>  };
>
> +/* OFPACT_CLONE.
> + *
> + * Used for cloning the packet and execute actions
> + * in the clone envelope. */
> +struct ofpact_clone {
> +OFPACT_PADDED_MEMBERS(
> +struct ofpact ofpact;
> +);
> +struct ofpact actions[0];
> +};
> +
> +BUILD_ASSERT_DECL(offsetof(struct ofpact_clone, actions)
> +  % OFPACT_ALIGNTO == 0);
> +BUILD_ASSERT_DECL(offsetof(struct ofpact_clone, actions)
> +  == sizeof(struct ofpact_clone));
> +
> +static inline size_t
> +ofpact_clone_get_action_len(const struct ofpact_clone *oc)
> +{
> +return oc->ofpact.len - offsetof(struct ofpact_clone, actions);
> +}
> +
>  /* OFPACT_DEC_TTL.
>   *
>   * Used for OFPAT11_DEC_NW_TTL, NXAST_DEC_TTL and NXAST_DEC_TTL_CNT_IDS.
> */
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index 7507558..227ecb2 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -306,6 +306,9 @@ enum ofp_raw_action_type {
> 

Re: [ovs-dev] assertion failing in commit_set_ipv4_action(), flow->nw_proto != base_flow->nw_proto

[Jarno Rajahalme]

> On Nov 30, 2016, at 8:50 PM, Ben Pfaff  wrote:
> 
> On Wed, Nov 30, 2016 at 06:58:57PM -0800, Jarno Rajahalme wrote:
>> 
>>> On Nov 30, 2016, at 8:41 AM, Ben Pfaff  wrote:
>>> 
>>> On Wed, Nov 30, 2016 at 12:05:46PM +0100, Thomas Morin wrote:
 Hi Ben,
 
 2016-11-30, Ben Pfaff:
> Do you have any idea what in your OpenFlow pipeline might do that,
> i.e. is there anything especially tricky in the OpenFlow flows?
> 
> Are you willing to show us your OpenFlow flow table?
 
 The setup involves three OVS bridges connected with patch-ports: br-int --
 br-tun -- br-mpls, with the traffic that triggers the assert being 
 processed
 by br-int with a NORMAL action (ie. MAC learning).
 
 The flows in this setup aren't particularly tricky, I think, although I'm
 not sure what qualifies as tricky or non-tricky :)
 
 Anyway, since yesterday I managed to identify the event that trigger the
 assert, by adding more logging before the assert and displaying the actions
 taken:
 
 2016-11-29T14:44:40.126Z|1|odp_util(revalidator45)|WARN|commit_set_ipv4_action
 assert would fail
 2016-11-29T14:44:40.126Z|2|odp_util(revalidator45)|WARN|  base_flow: 
 ip,in_port=5,dl_vlan=3,dl_vlan_pcp=0,dl_src=fa:16:3e:33:f7:fe,dl_dst=00:00:5e:00:43:64,nw_src=0.0.0.0,nw_dst=0.0.0.0,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0
 2016-11-29T14:44:40.126Z|3|odp_util(revalidator45)|WARN|  flow: 
 tcp,in_port=5,dl_vlan=3,dl_vlan_pcp=0,dl_src=fa:16:3e:33:f7:fe,dl_dst=00:00:5e:00:43:64,nw_src=10.0.1.22,nw_dst=10.0.0.3,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=53295,tp_dst=8080,tcp_flags=psh|ack
 2016-11-29T14:44:40.126Z|4|odp_util(revalidator45)|WARN|  masks: 
 recirc_id=0x,reg0=0x,in_port=4294967295,dl_vlan=4095,dl_vlan_pcp=7,dl_src=ff:ff:ff:ff:ff:ff,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x
 2016-11-29T14:44:40.126Z|5|odp_util(revalidator45)|WARN|  actions: 
 set(ipv4(src=10.0.1.22,dst=10.0.0.3,ttl=63)),set(eth(src=b8:2a:72:de:1b:e3,dst=00:17:cb:79:2c:01)),push_mpls(label=410384,tc=0,ttl=63,bos=1,eth_type=0x8847),9,set(eth(src=fa:16:3e:33:f7:fe,dst=00:00:5e:00:43:64)),pop_mpls(eth_type=0x800),push_vlan(vid=3,pcp=0),1
>> 
>> push_mpls clears L3/L4, while pop_mpls re-populates them, and then 
>> processing the output to port 1 hits the assert?
> 
> That's what I'm thinking too.
> 
> Jarno, is this something you have time to look into?  It'd be great, if
> you do.  I'm way behind.

I’m looking at this.

Based on the trace given it seems that:
1. Packet is received on br-int port 32, which outputs it via NORMAL action 
over a patch port to another bridge. The only patch-port on br-int is 2 
(patch-tun). The NORMAL action adds dl_vlan=1.
2. br-tun receives the packet on in_port 1 (patch-int), and outputs it on it’s 
port 2 (patch-to-mpls)
3. br-mpls receives the packet on it’s in_port 2 (patch-to-tun), does pop_vlan, 
and outputs to it’s port 21 (ipvpn-pp-out), which is also an patch port.
4. br-mpls (?) receives the packet on it’s in_port 20 (ipvpn-pp-in), does 
dec_ttl,push_mpls:0x8847,load:0x644c0->OXM_OF_MPLS_LABEL[],set_field:b8:2a:72:de:1b:e3->eth_src,set_field:00:17:cb:79:2c:01->eth_dst,output:1

All this generates a megaflow: 
set(ipv4(src=10.0.1.23,dst=10.0.0.3,ttl=63)),set(eth(src=b8:2a:72:de:1b:e3,dst=00:17:cb:79:2c:01)),push_mpls(label=410816,tc=0,ttl=63,bos=1,eth_type=0x8847),9

This is only the beginning part of the trouble-some megaflow, in which br-int 
sends the packet also to another port (vlan 3), and as part of that pops the 
MPLS and restores the original ethernet addresses. Maybe this would happen with 
the trace too, if you flushed MACs before the trace?

The patch ports 21 and 20 appear to be in the same bridge and patched to each 
other. Is this the case?

The crashing megaflow has in_port=5,dl_vlan=3. Is this also on br-int?

Also, OVS 2.6 is a little bit less aggressive about avoiding recirculation 
after mpls operations, and I’d be interested to know if your case fails the 
same way with OVS 2.6?

Thanks,

  Jarno

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

Re: [ovs-dev] [PATCH v2] ovn-controller: Remove unused members from local_datapath.

[Mickey Spiegel]

On Thu, Dec 1, 2016 at 4:34 PM, Ben Pfaff  wrote:

> Nothing used these, except to initialize and free them.  'logical_port'
> wasn't meaningful in any case, it was just the name of the first logical
> port encountered from a particular logical datapath when traversing the
> database's Port_Binding table, which isn't in a meaningful order.
>

I have long considered the 'uuid' in local_datapath dangerous, due to
the first logical port encountered issue that you mention. Since
incremental processing disappeared, it has not been doing any harm.
I agree that it is safer to remove it.

Acked-by: Mickey Spiegel 


>
> Signed-off-by: Ben Pfaff 
> ---
> v1->v2: I found some more members that were unused.
>
>  ovn/controller/binding.c| 2 --
>  ovn/controller/ovn-controller.c | 1 -
>  ovn/controller/ovn-controller.h | 3 ---
>  3 files changed, 6 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index d7b175e..fb76032 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -114,8 +114,6 @@ add_local_datapath(struct hmap *local_datapaths,
>  }
>
>  struct local_datapath *ld = xzalloc(sizeof *ld);
> -ld->logical_port = xstrdup(binding_rec->logical_port);
> -memcpy(>uuid, _rec->header_.uuid, sizeof ld->uuid);
>  hmap_insert(local_datapaths, >hmap_node,
>  binding_rec->datapath->tunnel_key);
>  }
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-
> controller.c
> index 52eb491..45537de 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -566,7 +566,6 @@ main(int argc, char *argv[])
>  struct local_datapath *cur_node, *next_node;
>  HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
> _datapaths) {
>  hmap_remove(_datapaths, _node->hmap_node);
> -free(cur_node->logical_port);
>  free(cur_node);
>  }
>  hmap_destroy(_datapaths);
> diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-
> controller.h
> index 78c8b08..8a3e855 100644
> --- a/ovn/controller/ovn-controller.h
> +++ b/ovn/controller/ovn-controller.h
> @@ -53,9 +53,6 @@ struct ct_zone_pending_entry {
>   * the localnet port */
>  struct local_datapath {
>  struct hmap_node hmap_node;
> -struct hmap_node uuid_hmap_node;
> -struct uuid uuid;
> -char *logical_port;
>  const struct sbrec_port_binding *localnet_port;
>  };
>
> --
> 2.10.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] What is the minimum Linux kernel version ovs 2.6.1 build requires?

[Yang, Yi Y]

Joe, it is my bad. Our nsh patches added two pointer variables to " struct 
ovs_skb_cb", that increased its size by 16 bytes, but sk_buff.cb only has 48 
bytes, when I build ovs with "--with-linux=..." option, sizeof(struct 
ovs_gso_cb) will be over 48 bytes, that is the root cause of this build issue. 
After I manage to remove those two pointer variables, it can be built 
successfully. But sizeof(struct ovs_gso_cb) is still 48 bytes, obviously it 
can't be extended any more in the future unless Linux kernel changes sk_buff.cb 
to bigger size.

-Original Message-
From: Joe Stringer [mailto:j...@ovn.org] 
Sent: Friday, December 2, 2016 2:21 AM
To: Yang, Yi Y 
Cc: d...@openvswitch.org; ovs-disc...@openvswitch.org
Subject: Re: [ovs-dev] What is the minimum Linux kernel version ovs 2.6.1 build 
requires?

On 30 November 2016 at 04:58, Yang, Yi Y  wrote:
> Hi, folks
>
> I tried to build ovs 2.6.1 in Ubuntu trusty 64 with the below configuration 
> options, but it failed, it is ok when I use the same way to build in Ubuntu 
> 16.04 which has Linux kernel 4.4. What is the minimum Linux kernel version 
> ovs 2.6.1 build requires?

I'm surprised to see this; I regularly build master and branch-2.6 on Ubuntu 
14.04 with linux 3.13 and 16.04 with linux 4.4.

Which kernel version are you seeing this with?

Can you also pastebin the $BUILD/datapath/linux/kcompat.h ?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] ovn-controller: Remove unused members from local_datapath.

[Ben Pfaff]

Nothing used these, except to initialize and free them.  'logical_port'
wasn't meaningful in any case, it was just the name of the first logical
port encountered from a particular logical datapath when traversing the
database's Port_Binding table, which isn't in a meaningful order.

Signed-off-by: Ben Pfaff 
---
v1->v2: I found some more members that were unused.

 ovn/controller/binding.c| 2 --
 ovn/controller/ovn-controller.c | 1 -
 ovn/controller/ovn-controller.h | 3 ---
 3 files changed, 6 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index d7b175e..fb76032 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -114,8 +114,6 @@ add_local_datapath(struct hmap *local_datapaths,
 }
 
 struct local_datapath *ld = xzalloc(sizeof *ld);
-ld->logical_port = xstrdup(binding_rec->logical_port);
-memcpy(>uuid, _rec->header_.uuid, sizeof ld->uuid);
 hmap_insert(local_datapaths, >hmap_node,
 binding_rec->datapath->tunnel_key);
 }
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 52eb491..45537de 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -566,7 +566,6 @@ main(int argc, char *argv[])
 struct local_datapath *cur_node, *next_node;
 HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, _datapaths) {
 hmap_remove(_datapaths, _node->hmap_node);
-free(cur_node->logical_port);
 free(cur_node);
 }
 hmap_destroy(_datapaths);
diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 78c8b08..8a3e855 100644
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -53,9 +53,6 @@ struct ct_zone_pending_entry {
  * the localnet port */
 struct local_datapath {
 struct hmap_node hmap_node;
-struct hmap_node uuid_hmap_node;
-struct uuid uuid;
-char *logical_port;
 const struct sbrec_port_binding *localnet_port;
 };
 
-- 
2.10.2

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

[ovs-dev] [PATCH] ovn-controller: Remove unused member 'logical_port' from local_datapath.

[Ben Pfaff]

Nothing used this, except to initialize and free it.  It wasn't
meaningful in any case, it was just the name of the first logical port
encountered from a particular logical datapath when traversing the
database's Port_Binding table, which isn't in a meaningful order.

Also, use assignment to copy uuids; no need for memcpy here.

Signed-off-by: Ben Pfaff 
---
 ovn/controller/binding.c| 3 +--
 ovn/controller/ovn-controller.c | 1 -
 ovn/controller/ovn-controller.h | 1 -
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index d7b175e..db28cc8 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -114,8 +114,7 @@ add_local_datapath(struct hmap *local_datapaths,
 }
 
 struct local_datapath *ld = xzalloc(sizeof *ld);
-ld->logical_port = xstrdup(binding_rec->logical_port);
-memcpy(>uuid, _rec->header_.uuid, sizeof ld->uuid);
+ld->uuid = binding_rec->header_.uuid;
 hmap_insert(local_datapaths, >hmap_node,
 binding_rec->datapath->tunnel_key);
 }
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 52eb491..45537de 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -566,7 +566,6 @@ main(int argc, char *argv[])
 struct local_datapath *cur_node, *next_node;
 HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, _datapaths) {
 hmap_remove(_datapaths, _node->hmap_node);
-free(cur_node->logical_port);
 free(cur_node);
 }
 hmap_destroy(_datapaths);
diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 78c8b08..46c381b 100644
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -55,7 +55,6 @@ struct local_datapath {
 struct hmap_node hmap_node;
 struct hmap_node uuid_hmap_node;
 struct uuid uuid;
-char *logical_port;
 const struct sbrec_port_binding *localnet_port;
 };
 
-- 
2.10.2

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

Re: [ovs-dev] [PATCH] CONTRIBUTING.rst: Update patch summary and description style guidelines.

[Ben Pfaff]

On Thu, Dec 01, 2016 at 01:16:14PM +, Stephen Finucane wrote:
> On Wed, 2016-11-30 at 12:40 -0800, Ben Pfaff wrote:
> > Suggested-by: Joe Stringer 
> > Suggested-at: https://mail.openvswitch.org/pipermail/ovs-dev/2016-Nov
> > ember/325513.html
> > Signed-off-by: Ben Pfaff 
> > ---
> >  CONTRIBUTING.rst | 12 ++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
> > index 867562e..721371e 100644
> > --- a/CONTRIBUTING.rst
> > +++ b/CONTRIBUTING.rst
> > @@ -94,7 +94,11 @@ Where:
> >    multiple distinct pieces of code.
> >  
> >  :
> > -  briefly describes the change.
> > +
> > +  briefly describes the change.  Use the the imperative form,
> > +  e.g. "Force SNAT for multiple gateway routers." or "Fix daemon
> > exit
> > +  for bad datapaths or flows."  This turns the summary into a
> > +  sentence, so please end it with a period.
> 
> This is super nitty (sorry), but the full stop takes up 1/50th of the
> available summary line characters [1] without adding any value :) I'd
> argue that its use certainly should be, at best, optional. If we're
> adding anything extra, I'd recommend that summary lines should be
> constrained to 50/51 characters where possible (vim does this itself).

Perhaps the period is more of a personal preference.  I'll drop it.

I find 50 characters to be almost impossible in many cases, myself, but
we can at least recommend it as something to try for.

> >  The subject, minus the ``[PATCH /]`` prefix, becomes the first
> > line of
> >  the commit's change log message.
> > @@ -106,7 +110,9 @@ The body of the email should start with a more
> > thorough description of the
> >  change.  This becomes the body of the commit message, following the
> > subject.
> >  There is no need to duplicate the summary given in the subject.
> >  
> > -Please limit lines in the description to 79 characters in width.
> > +Please limit lines in the description to 75 characters in
> > width.  That
> > +allows the description to format properly even when indented (e.g.
> > by
> > +"git log" or in email quotations).
> 
> Another nit (such is the way with anything coding standards'y): isn't
> 72 characters the usual recommendation? [1] Less importantly, we don't
> need to be so needlessly polite - "Limit lines..." is fine.

The default limit for "git citool" is 75.  Linus recommends 74 in the
subsurface tree.  I think 75 is a reasonable limit since "git log"
indents by 4 spaces and so 75-character lines won't wrap on an 80-column
terminal.

I don't think being polite costs us anything here.

Here's an incremental.

--8<--cut here-->8--

diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index 721371e..2b262ca 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -97,8 +97,8 @@ Where:
 
   briefly describes the change.  Use the the imperative form,
   e.g. "Force SNAT for multiple gateway routers." or "Fix daemon exit
-  for bad datapaths or flows."  This turns the summary into a
-  sentence, so please end it with a period.
+  for bad datapaths or flows."  Try to keep the summary short, about
+  50 characters wide.
 
 The subject, minus the ``[PATCH /]`` prefix, becomes the first line of
 the commit's change log message.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v2] ofproto: Honor OFPFF_RESET_COUNTS flag in flow modify message.

[Ben Pfaff]

On Thu, Dec 01, 2016 at 09:37:19PM +, Tony van der Peet wrote:
> I will foreshadow another problem I found while implementing test
> suite 140. This is where flow is created without SEND_FLOW_REM flag,
> but flow delete does set this bit. The flow removed message is not
> sent in this case. I can see a similar solution to the current patch
> working equally effectively. I'll review Jarno's test cases and try to
> come up with a similarly comprehensive set for this new case.

This is some kind of new interpretation of how flags work.
Historically, flags are set only when a flow is added.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Tome el Control de su Empresa

[Normas de Información Financiera (NIF)]

En línea y en Vivo / Para todo su Equipo con una sola Conexión 



Normas de Información Financiera (NIF) CONTROLES INTERNOS



16 de diciembre - Online en Vivo - 10:00 a 13:00 y de 15:00 a 18:00 Hrs   



Tome el verdadero control de su empresa. Aprenda la manera óptima de regular, 
registrar y administrar sus estados financieros. Con este Seminario ONLINE en 
Vivo usted conocerá las ventajas de implementar en su empresa las NIF para un 
estricto control interno y conocer a fondo su situación financiera, los efectos 
de su actividad operativa, flujos de efectivo y así, poder obtener 
financiamientos y determinar la viabilidad del negocio. 



"Pregunte por nuestra Promoción Navideña" 





TEMARIO:



1. Postulados básicos de las NIF.





2. NIF vigentes en México y Latinoamérica.



3. Conceptos específicos financieros y de resultados.



4. Estado de flujo de efectivo.









...¡Y mucho más!






¿Requiere la información a la Brevedad?

responda este email con la palabra: 

Normas.

Junto con los Siguientes Datos: Nombre, Teléfono, Empresa.

centro telefónico: 018002129393




Lic. Ángel Hernández

Coordinador de Evento






¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo 
desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la 
suscripción, solicite su BAJA.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v2] ofproto: Honor OFPFF_RESET_COUNTS flag in flow modify message.

[Ben Pfaff]

On Thu, Dec 01, 2016 at 01:09:12PM -0800, Jarno Rajahalme wrote:
> > If I may be permitted to nit-pick, the name "modify_forward_counts" took
> > me a bit of thinking to properly understand.  Maybe "modify_keep_stats"
> > would be easier for me to understand at first glance.
> 
> “stats” include the last used timestamp, which is treated
> independently of the byte and packet counts. How about
> “modify_keep_counts”?

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

Re: [ovs-dev] [PATCH net-next v2] ofproto: Honor OFPFF_RESET_COUNTS flag in flow modify message.

[Tony van der Peet]

Hi Ben, Jarno

> Tony, does this solve the problem for you?

Yes it does. I like the solution presented by Jarno a bit more than mine too, I 
had a feeling that adding a new flag would cause problems.

I will foreshadow another problem I found while implementing test suite 140. 
This is where flow is created without SEND_FLOW_REM flag, but flow delete does 
set this bit. The flow removed message is not sent in this case. I can see a 
similar solution to the current patch working equally effectively. I'll review 
Jarno's test cases and try to come up with a similarly comprehensive set for 
this new case.

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

[ovs-dev] [PATCH 3/4] datapath-windows: Conntrack - Introduce support for tracking related connections

[Sairam Venugopal]

Introduce a new table to track related connections. This table will be
used to track FTP data connections based on the control connection. There
is a new Conntrack-ftp.c to parse incoming FTP messages to determine the
related data ports. It creates a new entry in the related connections
tracker table. If there is a matching FTP data connection, then the state
for that connection is marked as RELATED.

Signed-off-by: Sairam Venugopal 
---
 datapath-windows/automake.mk|   2 +
 datapath-windows/ovsext/Conntrack-ftp.c | 234 ++
 datapath-windows/ovsext/Conntrack-related.c | 300 
 datapath-windows/ovsext/ovsext.vcxproj  |   2 +
 4 files changed, 538 insertions(+)
 create mode 100644 datapath-windows/ovsext/Conntrack-ftp.c
 create mode 100644 datapath-windows/ovsext/Conntrack-related.c

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index faa1fc6..c53de9f 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -14,8 +14,10 @@ EXTRA_DIST += \
datapath-windows/ovsext/Atomic.h \
datapath-windows/ovsext/BufferMgmt.c \
datapath-windows/ovsext/BufferMgmt.h \
+   datapath-windows/ovsext/Conntrack-ftp.c \
datapath-windows/ovsext/Conntrack-icmp.c \
datapath-windows/ovsext/Conntrack-other.c \
+   datapath-windows/ovsext/Conntrack-related.c \
datapath-windows/ovsext/Conntrack-tcp.c \
datapath-windows/ovsext/Conntrack.c \
datapath-windows/ovsext/Conntrack.h \
diff --git a/datapath-windows/ovsext/Conntrack-ftp.c 
b/datapath-windows/ovsext/Conntrack-ftp.c
new file mode 100644
index 000..f6b3358
--- /dev/null
+++ b/datapath-windows/ovsext/Conntrack-ftp.c
@@ -0,0 +1,234 @@
+/*
+ * Copyright (c) 2016 VMware, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "Conntrack.h"
+#include "PacketParser.h"
+
+typedef enum FTP_TYPE {
+FTP_TYPE_PASV = 1,
+FTP_TYPE_ACTIVE
+} FTP_TYPE;
+
+static __inline UINT32
+OvsStrncmp(const char *s1, const char *s2, size_t n)
+{
+if (!s1 || !s2) {
+return 0;
+}
+
+const char *s2end = s2 + n;
+while (s2 < s2end && *s2 != '\0' && toupper(*s1) == toupper(*s2)) {
+s1++, s2++;
+}
+
+if (s2end == s2) {
+return 0;
+}
+
+return (UINT32)(toupper(*s1) - toupper(*s2));
+}
+
+static __inline VOID
+OvsStrlcpy(char *dest, const char *src, size_t size)
+{
+/* XXX Replace ret with strlen(src) instead. */
+size_t ret = size;
+if (size) {
+   size_t len = (ret >= size) ? size - 1 : ret;
+   memcpy(dest, src, len);
+   dest[len] = '\0';
+   }
+}
+
+/*
+ *---
+ * OvsCtExtractNumbers
+ * Returns an array of numbers after parsing the string.
+ *Eg: PASV: 192,168,0,1,5,6 -> {192,168,0,1,5,6}
+ *EPRT: 192.168.0.1 -> {192,168,0,1}
+ *
+ *---
+ */
+static __inline NDIS_STATUS
+OvsCtExtractNumbers(char *buf,
+UINT32 bufLen,
+UINT32 arr[],
+UINT32 arrLen,
+char delimiter)
+{
+if (!buf) {
+return  NDIS_STATUS_INVALID_PACKET;
+}
+
+UINT32 i = 0;
+
+while (*buf != '\0') {
+if (i >= bufLen || i >= arrLen) {
+/* Non-standard FTP command */
+return NDIS_STATUS_INVALID_PARAMETER;
+}
+
+/* Parse the number */
+if (*buf >= '0' && *buf <= '9') {
+arr[i] = arr[i] * 10 + *buf - '0';
+} else if (*buf == delimiter) {
+i++;
+} else {
+/* End of FTP response is either ) or \r\n */
+if (*buf == ')' || *buf == '\r') {
+return NDIS_STATUS_SUCCESS;
+}
+/* Could be non-numerals or space */
+}
+buf++;
+}
+
+/* Parsing ended without the correct format */
+return NDIS_STATUS_INVALID_PARAMETER;
+}
+
+/*
+ *
+ * OvsCtHandleFtp
+ * Extract the FTP control data from the packet and created a related
+ * entry if it's a valid connection. This method doesn't support extended
+ * FTP yet. Supports PORT and PASV commands.
+ * Eg:
+ * 'PORT 192,168,137,103,192,22\r\n' -> '192.168.137.103' and 49174

[ovs-dev] [PATCH 0/4] datapath-windows: Enable support for tracking FTP connections

[Sairam Venugopal]

Add support for maintaining and tracking related connections. This patch 
introduces 
the concept of related-connections table. There is an FTP parser in place to 
parse 
FTP PASV and PORT commands. Support for traking extended FTP commands will be 
added 
in subsequently.


Sairam Venugopal (4):
  datapath-windows: Conntrack - Fix OvsGetTcpPayloadLength()
  datapath-windows: Cleanup Conntrack definitions and introduce related
entries
  datapath-windows: Conntrack - Introduce support for tracking related
connections
  datapath-windows: Conntrack - Enable FTP support

 datapath-windows/automake.mk|   2 +
 datapath-windows/ovsext/Conntrack-ftp.c | 234 ++
 datapath-windows/ovsext/Conntrack-related.c | 300 
 datapath-windows/ovsext/Conntrack-tcp.c |  15 --
 datapath-windows/ovsext/Conntrack.c |  74 +--
 datapath-windows/ovsext/Conntrack.h |  67 +++
 datapath-windows/ovsext/Switch.c|   8 +
 datapath-windows/ovsext/ovsext.vcxproj  |   2 +
 8 files changed, 673 insertions(+), 29 deletions(-)
 create mode 100644 datapath-windows/ovsext/Conntrack-ftp.c
 create mode 100644 datapath-windows/ovsext/Conntrack-related.c

-- 
2.9.0.windows.1

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

[ovs-dev] [PATCH 4/4] datapath-windows: Conntrack - Enable FTP support

[Sairam Venugopal]

Enable the support for tracking FTP connections in the Connection tracker.
This checks an incoming ftp control connection to extract the related data
connection. When a matching data connection arrives, it would then update
the connection entry to point to the original control connection.

Signed-off-by: Sairam Venugopal 
---
 datapath-windows/ovsext/Conntrack.c | 62 +++--
 datapath-windows/ovsext/Conntrack.h |  8 +
 datapath-windows/ovsext/Switch.c|  8 +
 3 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index d240c29..114c7b7 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -192,11 +192,21 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
 }
 
 state |= OVS_CS_F_NEW;
+POVS_CT_ENTRY parentEntry = NULL;
+parentEntry = OvsCtRelatedLookup(ctx->key, currentTime);
+if (parentEntry != NULL) {
+state |= OVS_CS_F_RELATED;
+}
+
 if (commit) {
 entry = OvsConntrackCreateTcpEntry(tcp, curNbl, currentTime);
 if (!entry) {
 return NULL;
 }
+/* If this is related entry, then update parent */
+if (parentEntry != NULL) {
+entry->parent = parentEntry;
+}
 OvsCtAddEntry(entry, ctx, currentTime);
 }
 
@@ -492,6 +502,13 @@ OvsCtSetupLookupCtx(OvsFlowKey *flowKey,
 return NDIS_STATUS_SUCCESS;
 }
 
+static __inline BOOLEAN
+OvsDetectFtpPacket(OvsFlowKey *key) {
+return (key->ipKey.nwProto == IPPROTO_TCP &&
+(ntohs(key->ipKey.l4.tpDst) == IPPORT_FTP ||
+ntohs(key->ipKey.l4.tpSrc) == IPPORT_FTP));
+}
+
 /*
  *
  * OvsProcessConntrackEntry
@@ -542,6 +559,21 @@ OvsProcessConntrackEntry(PNET_BUFFER_LIST curNbl,
 break;
 }
 }
+
+if (key->ipKey.nwProto == IPPROTO_TCP && entry) {
+/* Update the related bit if there is a parent */
+if (entry->parent) {
+state |= OVS_CS_F_RELATED;
+} else {
+POVS_CT_ENTRY parentEntry;
+parentEntry = OvsCtRelatedLookup(ctx->key, currentTime);
+if (parentEntry != NULL) {
+entry->parent = parentEntry;
+state |= OVS_CS_F_RELATED;
+}
+}
+}
+
 /* Copy mark and label from entry into flowKey. If actions specify
different mark and label, update the flowKey. */
 if (entry != NULL) {
@@ -592,7 +624,8 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
   BOOLEAN commit,
   UINT16 zone,
   MD_MARK *mark,
-  MD_LABELS *labels)
+  MD_LABELS *labels,
+  char *helper)
 {
 NDIS_STATUS status = NDIS_STATUS_SUCCESS;
 POVS_CT_ENTRY entry = NULL;
@@ -629,6 +662,17 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
 OvsConntrackSetLabels(key, entry, >value, >mask);
 }
 
+if (entry && OvsDetectFtpPacket(key)) {
+/* FTP parser will always be loaded */
+UNREFERENCED_PARAMETER(helper);
+
+status = OvsCtHandleFtp(curNbl, key, layers, currentTime, entry,
+(ntohs(key->ipKey.l4.tpDst) == IPPORT_FTP));
+if (status != NDIS_STATUS_SUCCESS) {
+OVS_LOG_ERROR("Error while parsing the FTP packet");
+}
+}
+
 NdisReleaseRWLock(ovsConntrackLockObj, );
 
 return status;
@@ -651,6 +695,8 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
 UINT16 zone = 0;
 MD_MARK *mark = NULL;
 MD_LABELS *labels = NULL;
+char *helper = NULL;
+
 NDIS_STATUS status;
 
 status = OvsDetectCtPacket(key);
@@ -674,9 +720,21 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
 if (ctAttr) {
 labels = NlAttrGet(ctAttr);
 }
+ctAttr = NlAttrFindNested(a, OVS_CT_ATTR_HELPER);
+if (ctAttr) {
+helper = NlAttrGet(ctAttr);
+if (!memchr(helper, '\0', 16)) {
+OVS_LOG_ERROR("Invalid CT_ATTR_HELPER:%s", helper);
+return NDIS_STATUS_INVALID_PARAMETER;
+}
+if (strcmp("ftp", helper) != 0) {
+/* Only support FTP */
+return NDIS_STATUS_NOT_SUPPORTED;
+}
+}
 
 status = OvsCtExecute_(curNbl, key, layers,
-   commit, zone, mark, labels);
+   commit, zone, mark, labels, helper);
 return status;
 }
 
diff --git a/datapath-windows/ovsext/Conntrack.h 
b/datapath-windows/ovsext/Conntrack.h
index 44fdfa0..d22b0c4 100644
--- a/datapath-windows/ovsext/Conntrack.h
+++ b/datapath-windows/ovsext/Conntrack.h
@@ -86,6 +86,7 @@ typedef struct OVS_CT_ENTRY {
 UINT32  mark;
 UINT64  

[ovs-dev] [PATCH 2/4] datapath-windows: Cleanup Conntrack definitions and introduce related entries

[Sairam Venugopal]

Consolidate the reusable structs and includes. Introduce the new
OVS_CT_REL_ENTRY to track related connections.

Signed-off-by: Sairam Venugopal 
---
 datapath-windows/ovsext/Conntrack.c | 12 
 datapath-windows/ovsext/Conntrack.h | 37 +
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index f482783..d240c29 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -14,27 +14,15 @@
  * limitations under the License.
  */
 
-#ifdef OVS_DBG_MOD
-#undef OVS_DBG_MOD
-#endif
-#define OVS_DBG_MOD OVS_DBG_CONTRK
-
 #include "Conntrack.h"
 #include "Jhash.h"
 #include "PacketParser.h"
-#include "Debug.h"
 #include "Event.h"
 
 #define WINDOWS_TICK 1000
 #define SEC_TO_UNIX_EPOCH 11644473600LL
 #define SEC_TO_NANOSEC 10LL
 
-typedef struct _OVS_CT_THREAD_CTX {
-KEVENT  event;
-PVOID   threadObject;
-UINT32  exit;
-} OVS_CT_THREAD_CTX, *POVS_CT_THREAD_CTX;
-
 KSTART_ROUTINE ovsConntrackEntryCleaner;
 static PLIST_ENTRY ovsConntrackTable;
 static OVS_CT_THREAD_CTX ctThreadCtx;
diff --git a/datapath-windows/ovsext/Conntrack.h 
b/datapath-windows/ovsext/Conntrack.h
index 428b12e..44fdfa0 100644
--- a/datapath-windows/ovsext/Conntrack.h
+++ b/datapath-windows/ovsext/Conntrack.h
@@ -19,6 +19,14 @@
 
 #include "precomp.h"
 #include "Flow.h"
+#include "Debug.h"
+#include "NetProto.h"
+#include 
+
+#ifdef OVS_DBG_MOD
+#undef OVS_DBG_MOD
+#endif
+#define OVS_DBG_MOD OVS_DBG_CONTRK
 
 struct ct_addr {
 union {
@@ -80,6 +88,19 @@ typedef struct OVS_CT_ENTRY {
 struct ovs_key_ct_labels labels;
 } OVS_CT_ENTRY, *POVS_CT_ENTRY;
 
+typedef struct OVS_CT_REL_ENTRY {
+OVS_CT_KEY  key;
+POVS_CT_ENTRY   parent;
+UINT64  expiration;
+LIST_ENTRY  link;
+} OVS_CT_REL_ENTRY, *POVS_CT_REL_ENTRY;
+
+typedef struct _OVS_CT_THREAD_CTX {
+KEVENT  event;
+PVOID   threadObject;
+UINT32  exit;
+} OVS_CT_THREAD_CTX, *POVS_CT_THREAD_CTX;
+
 typedef struct OvsConntrackKeyLookupCtx {
 OVS_CT_KEY  key;
 POVS_CT_ENTRY   entry;
@@ -167,4 +188,20 @@ OvsCreateNlMsgFromCtEntry(POVS_CT_ENTRY entry,
   UINT32 nlmsgPid,
   UINT8 nfGenVersion,
   UINT32 dpIfIndex);
+
+/* Tracking related connections */
+NTSTATUS OvsInitCtRelated(POVS_SWITCH_CONTEXT context);
+VOID OvsCleanupCtRelated(VOID);
+NDIS_STATUS
+OvsCtRelatedEntryCreate(UINT8 ipProto,
+UINT16 dl_type,
+UINT32 serverIp,
+UINT32 clientIp,
+UINT16 serverPort,
+UINT16 clientPort,
+UINT64 currentTime,
+POVS_CT_ENTRY parent);
+POVS_CT_ENTRY
+OvsCtRelatedLookup(OVS_CT_KEY key, UINT64 currentTime);
+
 #endif /* __OVS_CONNTRACK_H_ */
-- 
2.9.0.windows.1

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

Re: [ovs-dev] [PATCH] ofproto: Support reset_counts on flow mod message.

[Jarno Rajahalme]

> On Nov 29, 2016, at 3:46 PM, Tony van der Peet 
>  wrote:
> 
> If the reset_counts flag is set on a flow modification message, the
> flow counters must be cleared, even if the flow does not already have
> the reset_counts flag set. And the flow modification message is not
> able to set the reset_counts flag in the flow, so the flag must be
> tracked separately and not saved in the flow state.

Thanks for pointing this out!

Looking into this I noticed that the behavior was even more broken, most likely 
due to refactoring done by me earlier:

- OpenFlow 1.0 and 1.1 mod-flows did reset the counts, while only add-flow 
should
- With OpenFlow 1.2 and later, if the old flow had the reset_counts flag set, 
the counts would be reset by mod-flows, even if the mod-flows message does not 
have the reset_counts flag set.
- And as you noted, With OpenFlow 1.2 and later, mod-flows with a reset_count 
did not reset the counts, if the old flow did not have the reset_counts flag 
set.

So, even though the prevailing interpretation seems to be that the reset_counts 
flag in the flow-mod message should be stored as part of the flow state (and 
reported back in flow dumps with OpenFlow >= 1.3), we should always just look 
at the reset_counts flag in the current flow-mod and ignore the reset_counts 
flag stored in the flow. For OpenFlow 1.0 and 1.1 we implicitly add the 
reset_counts flag for add-flow messages (only) to maintain the expected 
behavior.

> ---
> include/openvswitch/ofp-util.h | 2 ++
> ofproto/ofproto.c  | 5 -
> 2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
> index 8703d2a..b7a32dc 100644
> --- a/include/openvswitch/ofp-util.h
> +++ b/include/openvswitch/ofp-util.h
> @@ -277,6 +277,8 @@ enum ofputil_flow_mod_flags {
>   set or modified. */
> OFPUTIL_FF_NO_READONLY   = 1 << 7, /* Allow rules within read only tables
>   to be modified */
> +OFPUTIL_FF_MOD_RESET_COU = 1 << 8, /* Reset counters for a flow 
> modification
> +  or delete message */
> };
> 

Instead of adding more confusion to the stored flow state, it is better to 
store the reset_counts flag in the (decoded) flow-mod message to the run-time 
context in ofproto.

I posted a patch yesterday with the fix and a test case verifying the behavior. 
I’d be happy if you could test and review it!

  Jarno

> /* Protocol-independent flow_mod.
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 53b7226..5af1828 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -5094,6 +5094,9 @@ replace_rule_start(struct ofproto *ofproto, struct 
> ofproto_flow_mod *ofm,
> new_rule->idle_timeout = old_rule->idle_timeout;
> new_rule->hard_timeout = old_rule->hard_timeout;
> *CONST_CAST(uint16_t *, _rule->importance) = 
> old_rule->importance;
> +if (new_rule->flags & OFPUTIL_FF_RESET_COUNTS) {
> +old_rule->flags |= OFPUTIL_FF_MOD_RESET_COU;
> +}
> new_rule->flags = old_rule->flags;
> new_rule->created = old_rule->created;
> }
> @@ -5160,7 +5163,7 @@ replace_rule_finish(struct ofproto *ofproto, struct 
> ofproto_flow_mod *ofm,
> struct ovs_list *dead_cookies)
> OVS_REQUIRES(ofproto_mutex)
> {
> -bool forward_counts = !(new_rule->flags & OFPUTIL_FF_RESET_COUNTS);
> +bool forward_counts = !(new_rule->flags & (OFPUTIL_FF_RESET_COUNTS | 
> OFPUTIL_FF_MOD_RESET_COU));
> struct rule *replaced_rule;
> 
> replaced_rule = (old_rule && old_rule->removed_reason != OFPRR_EVICTION)
> -- 
> 2.10.2
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH net-next v2] ofproto: Honor OFPFF_RESET_COUNTS flag in flow modify message.

[Jarno Rajahalme]

> On Nov 30, 2016, at 9:06 PM, Ben Pfaff  wrote:
> 
> On Wed, Nov 30, 2016 at 05:51:12PM -0800, Jarno Rajahalme wrote:
>> While a flow modify must keep the original flow's flags, it must reset
>> counts if (and only if) the reset_counts flag is present in the flow
>> mod message.
>> 
>> Behavior prior to this patch is broken in a few ways:
>> 
>> - OpenFlow 1.0 and 1.1 mod-flows did reset the counts, if the flow had
>>  reset_counts flag set.  Only add-flow should reset counts.
>> - With OpenFlow 1.2 and later, if the old flow had the reset_counts
>>  flag set, the counts would be reset by mod-flows, even if the
>>  flow-mod message does not have the reset_counts flag set.
>> - With OpenFlow 1.2 and later, mod-flows with a reset_count did not
>>  reset the counts, if the old flow did not have the reset_counts flag
>>  set.
>> 
>> Even though the prevailing interpretation seems to be that the
>> reset_counts flag in the flow-mod message should be stored as part of
>> the flow state (and reported back in flow dumps with OpenFlow >= 1.3),
>> we should always just look at the reset_counts flag in the current
>> flow-mod and ignore the reset_counts flag stored in the flow when
>> processing a flow mod.
>> 
>> For OpenFlow 1.0 and 1.1 we already implicitly add the reset_counts
>> flag for add-flow messages (only) to maintain the expected behavior.
>> 
>> This patch adds a comprehensive test case to prevent future regressions.
>> 
>> Suggested-by: Tony van der Peet 
>> Fixes: 748eb2f5b1 ("ofproto-dpif: Always forward 'used' from the old_rule.")
>> Signed-off-by: Jarno Rajahalme 
> 
> This is tagged "net-next", but it's an OVS patch.
> 

Oops, my bad :-)

> This is very nice.  One rarely sees such a thorough test case.  It had
> never occurred to me to try to match exact values for duration,
> idle_age, and hard_age in output (at least down to a number of seconds),
> but if it works that's very nice too.
> 

Seems to work fine with time/stop + time/warp!

> If I may be permitted to nit-pick, the name "modify_forward_counts" took
> me a bit of thinking to properly understand.  Maybe "modify_keep_stats"
> would be easier for me to understand at first glance.
> 

“stats” include the last used timestamp, which is treated independently of the 
byte and packet counts. How about “modify_keep_counts”?

> Tony, does this solve the problem for you?
> 
> Acked-by: Ben Pfaff >

Thanks for the review!

  Jarno

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

[ovs-dev] [PATCH] ovsdb-tool: Document database numbering scheme.

[Ben Pfaff]

Prompted by an IRC discussion.

Signed-off-by: Ben Pfaff 
---
 ovsdb/ovsdb-tool.1.in | 25 +
 vswitchd/vswitch.xml  | 11 ++-
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/ovsdb/ovsdb-tool.1.in b/ovsdb/ovsdb-tool.1.in
index a2f1f22..d01531e 100644
--- a/ovsdb/ovsdb-tool.1.in
+++ b/ovsdb/ovsdb-tool.1.in
@@ -62,6 +62,29 @@ specified, the compacted version is written as a new file 
named
 omitted, then the compacted version of the database replaces \fIdb\fR
 in-place.
 .
+.SS "Version Management Commands"
+.PP
+An OVSDB schema has a schema version number, and an OVSDB database
+embeds a particular version of an OVSDB schema.  These version numbers
+take the form \fIx\fB.\fIy\fB.\fIz\fR, e.g. \fB1.2.3\fR.  The OVSDB
+implementation does not enforce a particular version numbering scheme,
+but schemas managed within the Open vSwitch project use the following
+approach.  Whenever the database schema is changed in a non-backward
+compatible way (e.g. deleting a column or a table), \fIx\fR is
+incremented (and \fIy\fR and \fIz\fR are reset to 0).  When the
+database schema is changed in a backward compatible way (e.g. adding a
+new column), \fIy\fR is incremented (and \fIz\fR is reset to 0).  When
+the database schema is changed cosmetically (e.g. reindenting its
+syntax), \fIz\fR is incremented.
+.
+.PP
+Some OVSDB databases and schemas, especially very old ones, do not
+have a version number.
+.
+.PP
+These commands work with different versions of OVSDB schemas and
+databases.
+.
 .IP "\fBconvert\fI db schema \fR[\fItarget\fR]"
 Reads \fIdb\fR, translating it into to the schema specified in
 \fIschema\fR, and writes out the new interpretation.  If \fItarget\fR
@@ -106,6 +129,8 @@ If \fIschema\fR or \fIdb\fR was created before schema 
checksums were
 introduced, then it will not have a checksum and this command
 will print a blank line.
 .
+.SS "Other Commands"
+.
 .IP "\fBquery\fI db transaction\fR"
 Opens \fIdb\fR, executes \fItransaction\fR on it, and prints the
 results.  The \fItransaction\fR must be a JSON array in the format of
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index b92b7c0..d10fc1d 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -501,15 +501,8 @@
 
   
 
-  The database schema version number in the form
-  major.minor.tweak,
-  e.g. 1.2.3.  Whenever the database schema is changed in
-  a non-backward compatible way (e.g. deleting a column or a table),
-  major is incremented.  When the database schema is changed
-  in a backward compatible way (e.g. adding a new column),
-  minor is incremented.  When the database schema is changed
-  cosmetically (e.g. reindenting its syntax), tweak is
-  incremented.
+  The database schema version number, e.g. 1.2.3.  See
+  ovsdb-tool(1) for an explanation of the numbering scheme.
 
 
 
-- 
2.10.2

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

[ovs-dev] [RFC PATCH] Pipeline packet processing in OVS using FVL flow director.

[Sugesh Chandran]

The patch uses a pipeline model of packet processing in dpif-netdev.
Packets are processed by either normal or hardware pipeline based on flow-
director ID on the packet. The extendable model allows to enable any type
of partial offloads(Packet types, flow director) in a need basis.
The patch improved VxLAN decapsulation performance by ~62% .As a caveat, the
default software path performance is reduced by ~6-7% due to the
pipeline handling overhead.

Following major changes are introduced by this commit.

1) Added a netdev specific pipeline selection logic on packet reception.
2) Every packet carries a 64 bit pipeline metadata over the lifetime of packet
in OVS.
3) Selective miniflow extract logic is introduced for pipeline based miniflow
extract.
4) Flow insert logic is modified to replace the default s/w rules of vxlan
tunneling with (h/w + s/w) rules.
5) Action logic is modified to perform the light-weight tunnel pop for the
hardware pipeline.

TODO::
1) Flow insertions are handled by the PMD threads at the moment. Its necessary
to define a handler thread to install hw+s/w flows to avoid blocking PMD
threads for longer period.
2) Hardware flow deletion is not implemented in this patch. Can add when the
flow insert approach is finalized.
3) hardware specific rules are populated only at emc level. dp_classifier also
need to be populated accordingly.

Signed-off-by: Sugesh Chandran 
---
 include/openvswitch/flow.h|   9 +-
 include/openvswitch/packets.h |  15 ++-
 lib/automake.mk   |   8 +-
 lib/dpdk-i40e-ofld.c  | 257 ++
 lib/dpdk-i40e-ofld.h  |  72 
 lib/dpif-netdev.c | 222 +---
 lib/flow.c|  71 ++--
 lib/hw-pipeline.c |  75 
 lib/hw-pipeline.h |  52 +
 lib/match.c   |   2 +-
 lib/netdev-bsd.c  |   1 +
 lib/netdev-dpdk.c |  34 +-
 lib/netdev-dummy.c|   1 +
 lib/netdev-linux.c|   1 +
 lib/netdev-native-tnl.c   |  42 +++
 lib/netdev-native-tnl.h   |   2 +-
 lib/netdev-provider.h |   6 +
 lib/netdev-vport.c|   3 +-
 lib/nx-match.c|   2 +-
 lib/odp-util.h|   2 +-
 lib/ofp-util.c|   2 +-
 lib/packets.h |   8 ++
 ofproto/ofproto-dpif-rid.h|   2 +-
 ofproto/ofproto-dpif-xlate.c  |   2 +-
 24 files changed, 842 insertions(+), 49 deletions(-)
 create mode 100644 lib/dpdk-i40e-ofld.c
 create mode 100644 lib/dpdk-i40e-ofld.h
 create mode 100644 lib/hw-pipeline.c
 create mode 100644 lib/hw-pipeline.h

diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
index df80dfe..3639fc0 100644
--- a/include/openvswitch/flow.h
+++ b/include/openvswitch/flow.h
@@ -23,7 +23,7 @@
 /* This sequence number should be incremented whenever anything involving flows
  * or the wildcarding of flows changes.  This will cause build assertion
  * failures in places which likely need to be updated. */
-#define FLOW_WC_SEQ 36
+#define FLOW_WC_SEQ 37
 
 /* Number of Open vSwitch extension 32-bit registers. */
 #define FLOW_N_REGS 16
@@ -99,6 +99,9 @@ struct flow {
 uint32_t conj_id;   /* Conjunction ID. */
 ofp_port_t actset_output;   /* Output port in action set. */
 
+uint16_t pipeline_id;
+uint16_t pipeline_state;
+uint8_t pad0[4]; /* Pad to make pipeline 64 bit */
 /* L2, Order the same as in the Ethernet header! (64-bit aligned) */
 struct eth_addr dl_dst; /* Ethernet destination address. */
 struct eth_addr dl_src; /* Ethernet source address. */
@@ -135,8 +138,8 @@ BUILD_ASSERT_DECL(sizeof(struct flow_tnl) % 
sizeof(uint64_t) == 0);
 
 /* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
 BUILD_ASSERT_DECL(offsetof(struct flow, igmp_group_ip4) + sizeof(uint32_t)
-  == sizeof(struct flow_tnl) + 248
-  && FLOW_WC_SEQ == 36);
+  == sizeof(struct flow_tnl) + 256
+  && FLOW_WC_SEQ == 37);
 
 /* Incremental points at which flow classification may be performed in
  * segments.
diff --git a/include/openvswitch/packets.h b/include/openvswitch/packets.h
index 5d97309..26fbc87 100644
--- a/include/openvswitch/packets.h
+++ b/include/openvswitch/packets.h
@@ -19,6 +19,13 @@
 
 #include 
 #include "openvswitch/tun-metadata.h"
+/* Unfortunately, a "struct flow" sometimes has to handle OpenFlow port
+ * numbers and other times datapath (dpif) port numbers.  This union allows
+ * access to both. */
+union flow_in_port {
+odp_port_t odp_port;
+ofp_port_t ofp_port;
+};
 
 /* Tunnel information used in flow key and metadata. */
 struct flow_tnl {
@@ -53,12 +60,4 @@ struct flow_tnl {
 
 #define FLOW_TNL_F_MASK ((1 << 4) - 1)
 
-/* Unfortunately, a "struct flow" sometimes has to handle OpenFlow port
- * numbers and other 

[ovs-dev] [RFC PATCH] Pipeline packet processing in OVS using FVL flow director.

[Sugesh Chandran]

The RFC patch that uses the metadata information from the NIC to avoid/reduce 
the
packet processing overhead in OVS.  
It uses the flow director feature in XL710 NICs to identify and report the  
VxLAN tunneled packets. OVS-DPDK uses the reported information to bypass the
VxLAN tunnel processing in software.
 

The concept of pipeline packet processing in OVS is explained as below, 

Every incoming packets go through following stages in OVS data-path 

PKT-IN --> MF-EXTRACT --> LOOKUP --> ACTION --> PKT-OUT 

The pipeline proposal defines separate MF-EXTRACT, ACTION stages per pipeline   
than a common function for all the packets. A NIC pre-processed packets are 
handled
by a specific HW-MF-EXTRACT and HW-ACTION than the  
default set of functions. It improves the performance   
significantly, because of  the light-weight extract and action functions.   
However there is a slight overhead introduced to the default software path  
due to the introduction of new pipeline handling.   
Similar to the data-path changes, The FLOW-INSERT/DELETE stages are also
 
modified to perform a pipeline aware flow insert and delete.

The main advantage of the proposal are, 
1) Very customizable and can define different pipelines based on different  
NIC and port features.  
2) Very little impact on existing packet processing stages. 
3) Can work with P4 


Sugesh Chandran (1):
  Pipeline packet processing in OVS using FVL flow director.

 include/openvswitch/flow.h|   9 +-
 include/openvswitch/packets.h |  15 ++-
 lib/automake.mk   |   8 +-
 lib/dpdk-i40e-ofld.c  | 257 ++
 lib/dpdk-i40e-ofld.h  |  72 
 lib/dpif-netdev.c | 222 +---
 lib/flow.c|  71 ++--
 lib/hw-pipeline.c |  75 
 lib/hw-pipeline.h |  52 +
 lib/match.c   |   2 +-
 lib/netdev-bsd.c  |   1 +
 lib/netdev-dpdk.c |  34 +-
 lib/netdev-dummy.c|   1 +
 lib/netdev-linux.c|   1 +
 lib/netdev-native-tnl.c   |  42 +++
 lib/netdev-native-tnl.h   |   2 +-
 lib/netdev-provider.h |   6 +
 lib/netdev-vport.c|   3 +-
 lib/nx-match.c|   2 +-
 lib/odp-util.h|   2 +-
 lib/ofp-util.c|   2 +-
 lib/packets.h |   8 ++
 ofproto/ofproto-dpif-rid.h|   2 +-
 ofproto/ofproto-dpif-xlate.c  |   2 +-
 24 files changed, 842 insertions(+), 49 deletions(-)
 create mode 100644 lib/dpdk-i40e-ofld.c
 create mode 100644 lib/dpdk-i40e-ofld.h
 create mode 100644 lib/hw-pipeline.c
 create mode 100644 lib/hw-pipeline.h

-- 
2.5.0

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

Re: [ovs-dev] [PATCH] build: fix rpm-fedora target breakage

[Ben Pfaff]

On Thu, Dec 01, 2016 at 09:58:10AM +, Stephen Finucane wrote:
> On Wed, 2016-11-30 at 20:56 -0800, Ben Pfaff wrote:
> > On Wed, Nov 30, 2016 at 10:57:25PM -0500, Lance Richardson wrote:
> > > Since commit 3deca69b08f2 ("doc: Convert AUTHORS to rST"), the rpm-
> > > fedora
> > > target fails to build with:
> > > 
> > >   *** No rule to make target `AUTHORS.rst', needed by
> > > `debian/copyright'.
> > > 
> > > Fix by adding AUTHORS.rst to the docs list to ensure that it is
> > > included in the dist archive.
> > > 
> > > Fixes: 3deca69b08f2 ("doc: Convert AUTHORS to rST")
> > > Signed-off-by: Lance Richardson 
> > 
> > Not just that target, the regular old build actually fails with:
> > 
> > The following files are in git but not the distribution:
> > AUTHORS.rst
> > 
> > as long OVS was checked out of Git, anyway.
> > 
> > Either way, I applied this to master, and thanks for the fix.
> 
> Sorry for missing this - I can't run 'make distcheck' on my machine as
> it throws obscure "Permission denied" errors that I haven't been able
> to suss out yet. 

This particular error comes from plain "make", not "make check" or "make
distcheck".  It's emitted by this chunk of Makefile.am.  You don't see
it?  Maybe the code in the makefile needs some improvement.

# If we're checked out from a Git repository, make sure that every
# file that is in Git is distributed.
#
# We only enable this check when GNU make is in use because the
# Makefile in datapath/linux, needed to get the list of files to
# distribute, requires GNU make extensions.
if GNU_MAKE
ALL_LOCAL += dist-hook-git
dist-hook-git: distfiles
@if test -e $(srcdir)/.git && (git --version) >/dev/null 2>&1; then 
\
  (cd datapath && $(MAKE) distfiles);   
\
  (cat distfiles; sed 's|^|datapath/|' datapath/distfiles) |
\
LC_ALL=C sort -u > all-distfiles;   
\
  (cd $(srcdir) && git ls-files) | grep -v '\.gitignore$$' |
\
LC_ALL=C sort -u > all-gitfiles;
\
  LC_ALL=C comm -1 -3 all-distfiles all-gitfiles > 
missing-distfiles; \
  if test -s missing-distfiles; then
\
echo "The following files are in git but not the 
distribution:"; \
cat missing-distfiles;  \
exit 1; \
  fi;   
\
fi
CLEANFILES += all-distfiles all-gitfiles missing-distfiles
# The following is based on commands for the Automake "distdir" target.
distfiles: Makefile
@srcdirstrip=`echo "$(srcdir)" | sed 's/[].[^$$\\*]/&/g'`; \
topsrcdirstrip=`echo "$(top_srcdir)" | sed 
's/[].[^$$\\*]/&/g'`; \
list='$(DISTFILES)'; \
for file in $$list; do echo $$file; done | \
  sed -e "s|^$$srcdirstrip/||;t" \
  -e "s|^$$topsrcdirstrip/|$(top_builddir)/|;t" | \
  LC_ALL=C sort -u > $@
CLEANFILES += distfiles
endif
.PHONY: dist-hook-git

> Out of curiosity though, why did 'AUTHORS' (sans 'rst' suffix) not
> need to be tracked? Some form of Autotools magic?

Automake always distributes AUTHORS because the GNU standards say that
it should exist.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller: Remove obsolete concept of "stale" patched datapaths.

[Ben Pfaff]

On Thu, Dec 01, 2016 at 09:02:57AM -0500, Russell Bryant wrote:
> On Thu, Dec 1, 2016 at 2:11 AM, Ben Pfaff  wrote:
> 
> > When ovn-controller implemented incremental processing, the set of
> > patched datapaths was revised on each trip through the main loop, so it
> > was necessary to notice datapaths that shouldn't exist anymore and remove
> > them.
> >
> > With incremental processing gone, the set of patched datapaths is
> > built and torn down on every trip through the main loop, so there will
> > never be any stale datapaths.  This commit retires the concept.
> >
> > Signed-off-by: Ben Pfaff 
> >
> 
> Acked-by: Russell Bryant 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 1/3] ovn-nb: remote connection management in nb db

[Lance Richardson]

Add support for managing remote connections, including
SSL configuration, to northbound db schema, and add necessary
commands to ovn-nbctl.

Signed-off-by: Lance Richardson 
---
 NEWS  |   2 +
 ovn/ovn-nb.ovsschema  |  53 +++-
 ovn/ovn-nb.xml| 288 ++
 ovn/utilities/ovn-nbctl.8.xml |  36 ++
 ovn/utilities/ovn-nbctl.c | 208 ++
 tests/ovn.at  |  52 
 6 files changed, 634 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 5c5628c..2ec3dbb 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,8 @@ Post-v2.6.0
  * DSCP marking is now supported, via the new northbound QoS table.
  * IPAM now supports fixed MAC addresses.
  * Support for source IP address based routing.
+ * Support for managing SSL and remote connection configuration in
+   northbound database.
- Fixed regression in table stats maintenance introduced in OVS
  2.3.0, wherein the number of OpenFlow table hits and misses was
  not accurate.
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 65f2d7c..39c7f99 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
 "name": "OVN_Northbound",
 "version": "5.4.1",
-"cksum": "3773248894 11490",
+"cksum": "3485560318 13777",
 "tables": {
 "NB_Global": {
 "columns": {
@@ -10,7 +10,16 @@
 "hv_cfg": {"type": {"key": "integer"}},
 "external_ids": {
 "type": {"key": "string", "value": "string",
- "min": 0, "max": "unlimited"}}},
+ "min": 0, "max": "unlimited"}},
+"connections": {
+"type": {"key": {"type": "uuid",
+ "refTable": "Connection"},
+ "min": 0,
+ "max": "unlimited"}},
+"ssl": {
+"type": {"key": {"type": "uuid",
+ "refTable": "SSL"},
+ "min": 0, "max": 1}}},
 "maxRows": 1,
 "isRoot": true},
 "Logical_Switch": {
@@ -221,6 +230,40 @@
 "external_ids": {
 "type": {"key": "string", "value": "string",
  "min": 0, "max": "unlimited"}}},
-"isRoot": true}
-}
-}
+"isRoot": true},
+"Connection": {
+"columns": {
+"target": {"type": "string"},
+"max_backoff": {"type": {"key": {"type": "integer",
+ "minInteger": 1000},
+ "min": 0,
+ "max": 1}},
+"inactivity_probe": {"type": {"key": "integer",
+  "min": 0,
+  "max": 1}},
+"other_config": {"type": {"key": "string",
+  "value": "string",
+  "min": 0,
+  "max": "unlimited"}},
+"external_ids": {"type": {"key": "string",
+ "value": "string",
+ "min": 0,
+ "max": "unlimited"}},
+"is_connected": {"type": "boolean", "ephemeral": true},
+"status": {"type": {"key": "string",
+"value": "string",
+"min": 0,
+"max": "unlimited"},
+"ephemeral": true}},
+"indexes": [["target"]]},
+"SSL": {
+"columns": {
+"private_key": {"type": "string"},
+"certificate": {"type": "string"},
+"ca_cert": {"type": "string"},
+"bootstrap_ca_cert": {"type": "boolean"},
+"external_ids": {"type": {"key": "string",
+  "value": "string",
+  "min": 0,
+  "max": "unlimited"}}},
+"maxRows": 1}}}
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 3e40881..a3dc916 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -69,6 +69,17 @@
 See External IDs at the beginning of this document.
   
 
+
+  
+Database clients to which the Open vSwitch database server should
+connect or on which it should listen, along with options for how these
+connections should be configured.  See the 
+table for more information.
+  
+  
+Global SSL configuration.
+  
+
   
 
   
@@ 

Re: [ovs-dev] [PATCH] physical: Remove obsolete comments.

[Russell Bryant]

On Thu, Dec 1, 2016 at 2:11 AM, Ben Pfaff  wrote:

> These comments were added above code that removed flows from the flow
> table.  Now that that code was deleted, the comments no longer make sense.
>
> Signed-off-by: Ben Pfaff 
>

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

Re: [ovs-dev] [PATCH] CONTRIBUTING.rst: Update patch summary and description style guidelines.

[Stephen Finucane]

On Wed, 2016-11-30 at 12:40 -0800, Ben Pfaff wrote:
> Suggested-by: Joe Stringer 
> Suggested-at: https://mail.openvswitch.org/pipermail/ovs-dev/2016-Nov
> ember/325513.html
> Signed-off-by: Ben Pfaff 
> ---
>  CONTRIBUTING.rst | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
> index 867562e..721371e 100644
> --- a/CONTRIBUTING.rst
> +++ b/CONTRIBUTING.rst
> @@ -94,7 +94,11 @@ Where:
>    multiple distinct pieces of code.
>  
>  :
> -  briefly describes the change.
> +
> +  briefly describes the change.  Use the the imperative form,
> +  e.g. "Force SNAT for multiple gateway routers." or "Fix daemon
> exit
> +  for bad datapaths or flows."  This turns the summary into a
> +  sentence, so please end it with a period.

This is super nitty (sorry), but the full stop takes up 1/50th of the
available summary line characters [1] without adding any value :) I'd
argue that its use certainly should be, at best, optional. If we're
adding anything extra, I'd recommend that summary lines should be
constrained to 50/51 characters where possible (vim does this itself).

>  The subject, minus the ``[PATCH /]`` prefix, becomes the first
> line of
>  the commit's change log message.
> @@ -106,7 +110,9 @@ The body of the email should start with a more
> thorough description of the
>  change.  This becomes the body of the commit message, following the
> subject.
>  There is no need to duplicate the summary given in the subject.
>  
> -Please limit lines in the description to 79 characters in width.
> +Please limit lines in the description to 75 characters in
> width.  That
> +allows the description to format properly even when indented (e.g.
> by
> +"git log" or in email quotations).

Another nit (such is the way with anything coding standards'y): isn't
72 characters the usual recommendation? [1] Less importantly, we don't
need to be so needlessly polite - "Limit lines..." is fine.

Stephen

[1] https://medium.com/@preslavrachev/what-s-with-the-50-72-rule-8a906f
61f09c#.1maspcaci
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Packet type-aware pipeline, L3 tunneling, EXT-382 and NSH: Minutes call 2016-11-29

[Jiri Benc]

On Wed, 30 Nov 2016 22:52:38 +, Jan Scheurich wrote:
> The corresponding section in the document is already updated with the
> results of the discussion. So please have look. There's also a short
> summary of the main results at the end of the mail.

Thanks for writing that down. I see several shortcomings in the
proposal and several things make zero sense to me as proposed (probably
EXT-382 fault). I commented on that in the document.

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

Re: [ovs-dev] [PATCH] build: fix rpm-fedora target breakage

[Stephen Finucane]

On Wed, 2016-11-30 at 20:56 -0800, Ben Pfaff wrote:
> On Wed, Nov 30, 2016 at 10:57:25PM -0500, Lance Richardson wrote:
> > Since commit 3deca69b08f2 ("doc: Convert AUTHORS to rST"), the rpm-
> > fedora
> > target fails to build with:
> > 
> >   *** No rule to make target `AUTHORS.rst', needed by
> > `debian/copyright'.
> > 
> > Fix by adding AUTHORS.rst to the docs list to ensure that it is
> > included in the dist archive.
> > 
> > Fixes: 3deca69b08f2 ("doc: Convert AUTHORS to rST")
> > Signed-off-by: Lance Richardson 
> 
> Not just that target, the regular old build actually fails with:
> 
> The following files are in git but not the distribution:
> AUTHORS.rst
> 
> as long OVS was checked out of Git, anyway.
> 
> Either way, I applied this to master, and thanks for the fix.

Sorry for missing this - I can't run 'make distcheck' on my machine as
it throws obscure "Permission denied" errors that I haven't been able
to suss out yet. Out of curiosity though, why did 'AUTHORS' (sans 'rst'
suffix) not need to be tracked? Some form of Autotools magic?

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