[ovs-dev] [PATCH] datapath-windows: Add software checksums for nbl which contain multiple nb

2017-04-20 Thread Alin Serdean
Until now we only needed to compute software checksums on net buffer lists
containing a single net buffer.

This patch allows the software checksums to be applied on a net buffer list
with multiple net buffers. The hard assumption for this, is the net buffers are
part of the same connection. The position of the offsets is pointed by the
layers parameter.

This will be useful for introducing support ip fragments in conntrack.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Offload.c | 105 --
 1 file changed, 54 insertions(+), 51 deletions(-)

diff --git a/datapath-windows/ovsext/Offload.c 
b/datapath-windows/ovsext/Offload.c
index f3ab0e1..65d3b67 100644
--- a/datapath-windows/ovsext/Offload.c
+++ b/datapath-windows/ovsext/Offload.c
@@ -647,7 +647,9 @@ OvsCalculateUDPChecksum(PNET_BUFFER_LIST curNbl,
  * OvsApplySWChecksumOnNB --
  *
  * This function calculates and sets the required software offloads given by
- * csumInfo for a given NBL(nbl) with a single NB.
+ * csumInfo for a given NBL(nbl). If the given net buffer list 'nbl'
+ * has multiple net buffers, we assume that they are part of the same
+ * connection with the same offsets defined in 'layers'.
  *
  */
 NDIS_STATUS
@@ -661,60 +663,61 @@ OvsApplySWChecksumOnNB(POVS_PACKET_HDR_INFO layers,
 UINT32 packetLength = 0;
 ASSERT(nbl != NULL);
 
-curNb = NET_BUFFER_LIST_FIRST_NB(nbl);
-ASSERT(curNb->Next == NULL);
-packetLength = NET_BUFFER_DATA_LENGTH(curNb);
-curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
-   LowPagePriority);
-if (!bufferStart) {
-return NDIS_STATUS_RESOURCES;
-}
+for (curNb = NET_BUFFER_LIST_FIRST_NB(nbl); curNb != NULL;
+ curNb = curNb->Next) {
+packetLength = NET_BUFFER_DATA_LENGTH(curNb);
+curMdl = NET_BUFFER_CURRENT_MDL(curNb);
+bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
+   LowPagePriority);
+if (!bufferStart) {
+return NDIS_STATUS_RESOURCES;
+}
 
-bufferStart += NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
+bufferStart += NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
 
-if (layers->isIPv4) {
-IPHdr *ip = (IPHdr *)(bufferStart + layers->l3Offset);
+if (layers->isIPv4) {
+IPHdr *ip = (IPHdr *)(bufferStart + layers->l3Offset);
 
-if (csumInfo->Transmit.IpHeaderChecksum) {
-ip->check = 0;
-ip->check = IPChecksum((UINT8 *)ip, 4 * ip->ihl, 0);
-}
+if (csumInfo->Transmit.IpHeaderChecksum) {
+ip->check = 0;
+ip->check = IPChecksum((UINT8 *)ip, 4 * ip->ihl, 0);
+}
 
-if (layers->isTcp && csumInfo->Transmit.TcpChecksum) {
-UINT16 csumLength = (UINT16)(packetLength - layers->l4Offset);
-TCPHdr *tcp = (TCPHdr *)(bufferStart + layers->l4Offset);
-tcp->check = IPPseudoChecksum(>saddr, >daddr,
-  IPPROTO_TCP, csumLength);
-tcp->check = CalculateChecksumNB(curNb, csumLength,
- (UINT32)(layers->l4Offset));
-} else if (layers->isUdp && csumInfo->Transmit.UdpChecksum) {
-UINT16 csumLength = (UINT16)(packetLength - layers->l4Offset);
-UDPHdr *udp = (UDPHdr *)((PCHAR)ip + sizeof *ip);
-udp->check = IPPseudoChecksum(>saddr, >daddr,
-  IPPROTO_UDP, csumLength);
-udp->check = CalculateChecksumNB(curNb, csumLength,
- (UINT32)(layers->l4Offset));
-}
-} else if (layers->isIPv6) {
-IPv6Hdr *ip = (IPv6Hdr *)(bufferStart + layers->l3Offset);
-
-if (layers->isTcp && csumInfo->Transmit.TcpChecksum) {
-UINT16 csumLength = (UINT16)(packetLength - layers->l4Offset);
-TCPHdr *tcp = (TCPHdr *)(bufferStart + layers->l4Offset);
-tcp->check = IPv6PseudoChecksum((UINT32 *) >saddr,
-(UINT32 *) >daddr,
-IPPROTO_TCP, csumLength);
-tcp->check = CalculateChecksumNB(curNb, csumLength,
- (UINT32)(layers->l4Offset));
-} else if (layers->isUdp && csumInfo->Transmit.UdpChecksum) {
-UINT16 csumLength = (UINT16)(packetLength - layers->l4Offset);
-UDPHdr *udp = (UDPHdr *)((PCHAR)ip + sizeof *ip);
-udp->check = IPv6PseudoChecksum((UINT32 *) >saddr,
-(UINT32 *) >daddr,
-IPPROTO_UDP, csumLength);
-udp->check = CalculateChecksumNB(curNb, csumLength,
- 

Re: [ovs-dev] [branch-2.7 0/4] Backport of variable length metaflow field fixes.

2017-04-20 Thread Joe Stringer
On 17 March 2017 at 14:30, Joe Stringer  wrote:
> On 15 March 2017 at 16:01, Joe Stringer  wrote:
>> Commit 04f48a68c428 ("ofp-actions: Fix variable length meta-flow OXMs."), on
>> branch-2.7 as 9554b03d6ab7, attempted to address incorrect encode and decode 
>> of
>> variable length metaflow fields where the OXM/NXM encoding of the variable
>> length fields would incorrectly serialize the length. The patch addresses 
>> this
>> by introducing a new per-bridge structure that adds additional metaflow 
>> fields
>> for the variable-length fields on demand when the TLVs are configured by a
>> controller.
>>
>> Unfortunately, in the original patch there was nothing ensuring that flows
>> referring to variable length fields would retain valid field references when
>> controllers reconfigure the TLVs. In practice, this could lead to a crash of
>> ovs-vswitchd by configuring a TLV field, adding a flow which refers to it,
>> removing the TLV field, then running some traffic that hit the configured 
>> flow.
>>
>> This series looks to remedy the situation by reference counting the variable
>> length fields and preventing a controller from reconfiguring TLV fields when
>> there are active flows whose match or actions refer to the field.
>>
>> This series was applied to master, but given the size of the change and the
>> minor changes necessary to apply to branch-2.7, I would feel more confident 
>> in
>> backporting it if there was an extra round of review to ensure that nothing 
>> was
>> missed when this series was first applied to master.
>
> One further concern I have with this series is that while it allows us
> to fix bugs in OVS 2.7, it would change some files in
> include/openvswitch/, which I believe indirectly implies that it could
> break the libopenvswitch ABI, which we try not to do within a release
> series:
>
> http://docs.openvswitch.org/en/latest/internals/contributing/libopenvswitch-abi/

Reporting back, using abipkgdiff from
libabigail[https://sourceware.org/libabigail], I was able to identify
the following ABI breakages from v2.7.0 to branch-2.7 with these
patches applied, details below.

A bunch of these are libraries only exported through headers in lib/,
which I believe is not considered 'stable' ABI.

However, there are several that are exported in include/openvswitch:

ofpacts_pull_openflow_instructions
ofperr_encode_hello
ofputil_decode_flow_stats_request
ofputil_decode_packet_in
ofputil_decode_packet_in_private
ofputil_encode_bundle_msgs
ofputil_pull_ofp11_match

Now, the shared library is currently something like
'libopenvswitch-2.so.7.0.0'  (or, when we prepare 2.7.1,
'libopenvswitch-2.so.7.0.1' unless other changes are made). The
libtool ABI numbering does not appear to have any influence on the
naming of the shared library. It is (1,0,0) for current, revision and
age. I'm suspecting that the right answer to this is to bump current
and age as per 
[https://lists.gnu.org/archive/html/libtool/2009-08/msg00034.html].
When I do this, the ABI appears completely different according to
abipkgdiff due to the different 'current' number.

I'm not sure if we are supposed to make the ABI versioning number
consistent with our shared library naming, or if it is reasonable for
each OVS release series (eg, 2.7.x) to have independent libtool
versioning numbers.

Output from abipkgdiff with this series applied:

 changes of 'libopenvswitch-2.so.7.0.0'===
  Functions changes summary: 0 Removed, 14 Changed (84 filtered out),
7 Added functions
  Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

  7 Added functions:

'function void mcast_snooping_flush_bundle(mcast_snooping*,
void*)'{mcast_snooping_flush_bundle@@libopenvswitch_1}
'function ofperr mf_vl_mff_mf_from_nxm_header(uint32_t, const
vl_mff_map*, const mf_field**, uint64_t*)'
{mf_vl_mff_mf_from_nxm_header@@libopenvswitch_1}
'function ofperr mf_vl_mff_nx_pull_entry(ofpbuf*, const
vl_mff_map*, const mf_field**, uint64_t*)'
{mf_vl_mff_nx_pull_entry@@libopenvswitch_1}
'function ofperr mf_vl_mff_nx_pull_header(ofpbuf*, const
vl_mff_map*, const mf_field**, _Bool*, uint64_t*)'
{mf_vl_mff_nx_pull_header@@libopenvswitch_1}
'function void mf_vl_mff_ref(const vl_mff_map*, uint64_t)'
{mf_vl_mff_ref@@libopenvswitch_1}
'function void mf_vl_mff_set_tlv_bitmap(const mf_field*,
uint64_t*)'{mf_vl_mff_set_tlv_bitmap@@libopenvswitch_1}
'function void mf_vl_mff_unref(const vl_mff_map*, uint64_t)'
{mf_vl_mff_unref@@libopenvswitch_1}

  14 functions with some indirect sub-type change:

[C]'function ofperr bundle_check(const ofpact_bundle*, ofp_port_t,
const flow*)' at bundle.c:107:1 has some indirect sub-type changes:
  return type changed:
1 enumerator insertion:
  'ofperr::OFPERR_NXTTMFC_INVALID_TLV_DEL' value '1073741993'

3 enumerator changes:
  'ofperr::OFPERR_NXR_NOT_SUPPORTED' from value '1073741993'
to '1073741994'
 

[ovs-dev] [PATCH] ovn.at: Fix "ovn -- 1 LR with distributed router gateway port" test

2017-04-20 Thread YAMAMOTO Takashi
NetBSD implementation of wc command outputs extra whitespaces
like the following.  Tweak the test to success on such environments.

% echo hoge|wc -l|hexdump -C
  20 20 20 20 20 20 20 31  0a   |   1.|
0009
%

The failing test was introduced by
commit 41a15b71ed1ef35aa612a1128082219fbfc3f327
(ovn: Introduce distributed gateway port and "chassisredirect" port binding)

Signed-off-by: YAMAMOTO Takashi 
---
 tests/ovn.at | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/tests/ovn.at b/tests/ovn.at
index af77c19..1bffc4c 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -6567,20 +6567,14 @@ as hv3 ovs-ofctl dump-flows br-int
 echo "--"
 
 # Check that redirect mapping is programmed only on hv2
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=33 | grep =0x3,metadata=0x1 
| wc -l], [0], [0
-])
-AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=33 | grep =0x3,metadata=0x1 
| grep load:0x2- | wc -l], [0], [1
-])
+AT_CHECK([test `as hv1 ovs-ofctl dump-flows br-int table=33 | grep 
=0x3,metadata=0x1 | wc -l` -eq 0])
+AT_CHECK([test `as hv2 ovs-ofctl dump-flows br-int table=33 | grep 
=0x3,metadata=0x1 | grep load:0x2- | wc -l` -eq 1])
 # Check that hv1 sends chassisredirect port traffic to hv2
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=32 | grep =0x3,metadata=0x1 
| grep output | wc -l], [0], [1
-])
-AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=32 | grep =0x3,metadata=0x1 
| wc -l], [0], [0
-])
+AT_CHECK([test `as hv1 ovs-ofctl dump-flows br-int table=32 | grep 
=0x3,metadata=0x1 | grep output | wc -l` -eq 1])
+AT_CHECK([test `as hv2 ovs-ofctl dump-flows br-int table=32 | grep 
=0x3,metadata=0x1 | wc -l` -eq 0])
 # Check that arp reply on distributed gateway port is only programmed on hv2
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep arp | grep 
=0x2,metadata=0x1 | wc -l], [0], [0
-])
-AT_CHECK([as hv2 ovs-ofctl dump-flows br-int | grep arp | grep 
=0x2,metadata=0x1 | wc -l], [0], [1
-])
+AT_CHECK([test `as hv1 ovs-ofctl dump-flows br-int | grep arp | grep 
=0x2,metadata=0x1 | wc -l` -eq 0])
+AT_CHECK([test `as hv2 ovs-ofctl dump-flows br-int | grep arp | grep 
=0x2,metadata=0x1 | wc -l` -eq 1])
 
 
 ip_to_hex() {
-- 
2.5.4 (Apple Git-61)

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


Re: [ovs-dev] [PATCH] bridge: Log interface deletion

2017-04-20 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme 

> On Apr 20, 2017, at 5:39 PM, Andy Zhou  wrote:
> 
> Currently, interface additions are logged but not deletion. This
> makes system debugging, such as confirming OVSDB transaction are
> timely replicated harder than necessary.
> 
> Signed-off-by: Andy Zhou 
> ---
> vswitchd/bridge.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 867a26d8de19..81bd8074e593 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -4317,6 +4317,9 @@ iface_destroy__(struct iface *iface)
> struct port *port = iface->port;
> struct bridge *br = port->bridge;
> 
> +VLOG_INFO("bridge %s: deleted interface %s on port %d",
> +  br->name, iface->name, iface->ofp_port);
> +
> if (br->ofproto && iface->ofp_port != OFPP_NONE) {
> ofproto_port_unregister(br->ofproto, iface->ofp_port);
> }
> -- 
> 1.8.3.1
> 
> ___
> 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 v2 2/2] datapath: pass extended ACK struct to parsing functions

2017-04-20 Thread Jarno Rajahalme

> On Apr 20, 2017, at 4:36 PM, Joe Stringer  wrote:
> 
> On 19 April 2017 at 16:54, Jarno Rajahalme  > wrote:
>> From: Johannes Berg 
>> 
>> Upstream commit:
>> 
>>commit fceb6435e85298f747fee938415057af837f5a8a
>>Author: Johannes Berg 
>>Date:   Wed Apr 12 14:34:07 2017 +0200
>> 
>>netlink: pass extended ACK struct to parsing functions
>> 
>>Pass the new extended ACK reporting struct to all of the generic
>>netlink parsing functions. For now, pass NULL in almost all callers
>>(except for some in the core.)
>> 
>>Signed-off-by: Johannes Berg 
>>Signed-off-by: David S. Miller 
>> 
>> Signed-off-by: Jarno Rajahalme 
>> ---
> 
> Acked-by: Joe Stringer >

Thanks for the review, pushed to master,

  Jarno

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


Re: [ovs-dev] [PATCH 6/6] rstp: Add the 'ovs-appctl rstp/show' command.

2017-04-20 Thread Jarno Rajahalme
I’d like to see one of the existing RSTP test cases modified to use this new 
feature.

One more comment below,

  Jarno


> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao  
> wrote:
> 
> The rstp/show command will help users and developers to
> get more details about rstp. This patch works together with
> the previous patches.
> 
> Signed-off-by: nickcooper-zhangtonghao 
> ---
> NEWS   |   4 +-
> lib/rstp.c | 113 +++--
> lib/rstp.h |   2 +-
> vswitchd/ovs-vswitchd.8.in |  11 -
> 4 files changed, 123 insertions(+), 7 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 00c9106..a28b8da 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -15,7 +15,9 @@ Post-v2.7.0
>  "dot1q-tunnel" port VLAN mode.
>- OVN:
>  * Make the DHCPv4 router setting optional.
> -   - Add the command 'ovs-appctl stp/show' (see ovs-vswitchd(8)).
> +   - STP/RSTP
> + * Add the command 'ovs-appctl stp/show' and 'ovs-appctl rstp/show'
> +   (see ovs-vswitchd(8)).
> 
> v2.7.0 - 21 Feb 2017
> -
> diff --git a/lib/rstp.c b/lib/rstp.c
> index b942f6e..7a4f1ea 100644
> --- a/lib/rstp.c
> +++ b/lib/rstp.c
> @@ -120,6 +120,10 @@ static void rstp_port_set_mcheck__(struct rstp_port *, 
> bool mcheck)
> OVS_REQUIRES(rstp_mutex);
> static void reinitialize_port__(struct rstp_port *p)
> OVS_REQUIRES(rstp_mutex);
> +static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
> + const char *argv[], void *aux);
> +static void rstp_unixctl_show(struct unixctl_conn *, int argc,
> +  const char *argv[], void *aux);
> 
> const char *
> rstp_state_name(enum rstp_state state)
> @@ -208,9 +212,6 @@ rstp_port_get_number(const struct rstp_port *p)
> return number;
> }
> 
> -static void rstp_unixctl_tcn(struct unixctl_conn *, int argc,
> - const char *argv[], void *aux);
> -
> /* Decrements the State Machines' timers. */
> void
> rstp_tick_timers(struct rstp *rstp)
> @@ -246,6 +247,8 @@ rstp_init(void)
> 
> unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
> rstp_unixctl_tcn,
>  NULL);
> +unixctl_command_register("rstp/show", "[bridge]", 0, 1,
> + rstp_unixctl_show, NULL);
> ovsthread_once_done();
> }
> }
> @@ -1398,7 +1401,7 @@ rstp_get_designated_root(const struct rstp *rstp)
>  * there is no such port.
>  */
> struct rstp_port *
> -rstp_get_root_port(struct rstp *rstp)
> +rstp_get_root_port(const struct rstp *rstp)
> OVS_EXCLUDED(rstp_mutex)
> {
> struct rstp_port *p;
> @@ -1545,3 +1548,105 @@ rstp_unixctl_tcn(struct unixctl_conn *conn, int argc,
> out:
> ovs_mutex_unlock(_mutex);
> }
> +
> +static void
> +rstp_bridge_id_details(struct ds *ds, const rstp_identifier bridge_id,
> +   const uint16_t hello_time, const uint16_t max_age,
> +   const uint16_t forward_delay)
> +OVS_REQUIRES(rstp_mutex)
> +{
> +uint16_t priority = bridge_id >> 48;
> +ds_put_format(ds, "\tstp-priority\t%"PRIu16"\n", priority);
> +
> +struct eth_addr mac;
> +const uint64_t mac_bits = (UINT64_C(1) << 48) - 1;
> +eth_addr_from_uint64(bridge_id & mac_bits, );
> +ds_put_format(ds, "\tstp-system-id\t"ETH_ADDR_FMT"\n", 
> ETH_ADDR_ARGS(mac));
> +ds_put_format(ds, "\tstp-hello-time\t%"PRIu16"s\n", hello_time);
> +ds_put_format(ds, "\tstp-max-age\t%"PRIu16"s\n", max_age);
> +ds_put_format(ds, "\tstp-fwd-delay\t%"PRIu16"s\n", forward_delay);
> +}
> +
> +static void
> +rstp_print_details(struct ds *ds, const struct rstp *rstp)
> +OVS_REQUIRES(rstp_mutex)
> +{
> +ds_put_format(ds, " %s \n", rstp->name);
> +ds_put_cstr(ds, "Root ID:\n");
> +
> +bool is_root = rstp_is_root_bridge(rstp);
> +struct rstp_port *p = rstp_get_root_port(rstp);
> +
> +rstp_identifier bridge_id =
> +is_root ? rstp->bridge_identifier : rstp_get_root_id(rstp);
> +uint16_t hello_time =
> +is_root ? rstp->bridge_hello_time : p->designated_times.hello_time;
> +uint16_t max_age =
> +is_root ? rstp->bridge_max_age : p->designated_times.max_age;
> +uint16_t forward_delay =
> +is_root ? rstp->bridge_forward_delay : 
> p->designated_times.forward_delay;
> +
> +rstp_bridge_id_details(ds, bridge_id, hello_time, max_age, 
> forward_delay);
> +if (is_root) {
> +ds_put_cstr(ds, "\tThis bridge is the root\n");
> +} else {
> +ds_put_format(ds, "\troot-port\t%s\n", p->port_name);
> +ds_put_format(ds, "\troot-path-cost\t%u\n",
> +  rstp_get_root_path_cost(rstp));
> +}
> +ds_put_cstr(ds, "\n");
> +
> +ds_put_cstr(ds, "Bridge ID:\n");
> +rstp_bridge_id_details(ds, rstp->bridge_identifier,
> +   rstp->bridge_hello_time,
> +  

Re: [ovs-dev] [PATCH 5/6] rstp: Add rstp port name for human reading.

2017-04-20 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme 

> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao  
> wrote:
> 
> This patch is useful to debug rstp subsystem and log the
> port name instead of port number. This patch will also
> be used to display rstp info for next patches.
> 
> Signed-off-by: nickcooper-zhangtonghao 
> ---
> lib/rstp-common.h  |  1 +
> lib/rstp.c | 14 +-
> lib/rstp.h |  3 ++-
> ofproto/ofproto-dpif.c |  2 +-
> 4 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/rstp-common.h b/lib/rstp-common.h
> index 27e8079..c108232 100644
> --- a/lib/rstp-common.h
> +++ b/lib/rstp-common.h
> @@ -262,6 +262,7 @@ struct rstp_port {
> struct rstp *rstp OVS_GUARDED_BY(rstp_mutex);
> struct hmap_node node OVS_GUARDED_BY(rstp_mutex); /* In rstp->ports. */
> void *aux OVS_GUARDED_BY(rstp_mutex);
> +char *port_name;
> struct rstp_bpdu received_bpdu_buffer OVS_GUARDED_BY(rstp_mutex);
> /*
>  * MAC status parameters
> diff --git a/lib/rstp.c b/lib/rstp.c
> index 6f1c1e3..b942f6e 100644
> --- a/lib/rstp.c
> +++ b/lib/rstp.c
> @@ -760,6 +760,14 @@ rstp_port_set_port_number__(struct rstp_port *port, 
> uint16_t port_number)
> }
> }
> 
> +static void
> +rstp_port_set_port_name__(struct rstp_port *port, const char *name)
> +OVS_REQUIRES(rstp_mutex)
> +{
> +free(port->port_name);
> +port->port_name = xstrdup(name);
> +}
> +
> /* Converts the link speed to a port path cost [Table 17-3]. */
> uint32_t
> rstp_convert_speed_to_cost(unsigned int speed)
> @@ -1173,6 +1181,7 @@ rstp_add_port(struct rstp *rstp)
> rstp_port_set_priority__(p, RSTP_DEFAULT_PORT_PRIORITY);
> rstp_port_set_port_number__(p, 0);
> p->aux = NULL;
> +p->port_name = NULL;
> rstp_initialize_port_defaults__(p);
> VLOG_DBG("%s: RSTP port "RSTP_PORT_ID_FMT" initialized.", rstp->name,
>  p->port_id);
> @@ -1210,6 +1219,7 @@ rstp_port_unref(struct rstp_port *rp)
> ovs_mutex_lock(_mutex);
> rstp = rp->rstp;
> rstp_port_set_state__(rp, RSTP_DISABLED);
> +free(rp->port_name);
> hmap_remove(>ports, >node);
> VLOG_DBG("%s: removed port "RSTP_PORT_ID_FMT"", rstp->name,
>  rp->port_id);
> @@ -1448,13 +1458,15 @@ void
> rstp_port_set(struct rstp_port *port, uint16_t port_num, int priority,
>   uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
>   enum rstp_admin_point_to_point_mac_state admin_p2p_mac_state,
> -  bool admin_port_state, bool do_mcheck, void *aux)
> +  bool admin_port_state, bool do_mcheck, void *aux,
> +  const char *name)
> OVS_EXCLUDED(rstp_mutex)
> {
> ovs_mutex_lock(_mutex);
> port->aux = aux;
> rstp_port_set_priority__(port, priority);
> rstp_port_set_port_number__(port, port_num);
> +rstp_port_set_port_name__(port, name);
> rstp_port_set_path_cost__(port, path_cost);
> rstp_port_set_admin_edge__(port, is_admin_edge);
> rstp_port_set_auto_edge__(port, is_auto_edge);
> diff --git a/lib/rstp.h b/lib/rstp.h
> index 78e07fb..fa67e3c 100644
> --- a/lib/rstp.h
> +++ b/lib/rstp.h
> @@ -221,7 +221,8 @@ uint32_t rstp_convert_speed_to_cost(unsigned int speed);
> void rstp_port_set(struct rstp_port *, uint16_t port_num, int priority,
>uint32_t path_cost, bool is_admin_edge, bool is_auto_edge,
>enum rstp_admin_point_to_point_mac_state 
> admin_p2p_mac_state,
> -   bool admin_port_state, bool do_mcheck, void *aux)
> +   bool admin_port_state, bool do_mcheck, void *aux,
> +   const char *name)
> OVS_EXCLUDED(rstp_mutex);
> 
> enum rstp_state rstp_port_get_state(const struct rstp_port *)
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index f015131..d41d90f 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -2675,7 +2675,7 @@ set_rstp_port(struct ofport *ofport_,
> rstp_port_set(rp, s->port_num, s->priority, s->path_cost,
>   s->admin_edge_port, s->auto_edge,
>   s->admin_p2p_mac_state, s->admin_port_state, s->mcheck,
> -  ofport);
> +  ofport, netdev_get_name(ofport->up.netdev));
> update_rstp_port_state(ofport);
> /* Synchronize operational status. */
> rstp_port_set_mac_operational(rp, ofport->may_enable);
> -- 
> 1.8.3.1
> 
> 
> 
> ___
> 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 4/6] rstp: Init a recursive mutex for rstp.

2017-04-20 Thread Jarno Rajahalme

> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao  
> wrote:
> 
> This patch will be used for next patch.

I don’t see exactly what in the following patch(es) need this. Could you 
elaborate?

> 
> Signed-off-by: nickcooper-zhangtonghao 
> ---
> lib/rstp.c | 15 ---
> lib/rstp.h |  6 --
> 2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/rstp.c b/lib/rstp.c
> index 907a907..6f1c1e3 100644
> --- a/lib/rstp.c
> +++ b/lib/rstp.c
> @@ -50,7 +50,7 @@
> 
> VLOG_DEFINE_THIS_MODULE(rstp);
> 
> -struct ovs_mutex rstp_mutex = OVS_MUTEX_INITIALIZER;
> +static struct ovs_mutex rstp_mutex;
> 
> static struct ovs_list all_rstps__ = OVS_LIST_INITIALIZER(_rstps__);
> static struct ovs_list *const all_rstps OVS_GUARDED_BY(rstp_mutex) = 
> _rstps__;
> @@ -239,8 +239,15 @@ void
> rstp_init(void)
> OVS_EXCLUDED(rstp_mutex)
> {
> -unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
> - NULL);
> +static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> +
> +if (ovsthread_once_start()) {
> +ovs_mutex_init_recursive(_mutex);
> +
> +unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
> rstp_unixctl_tcn,
> + NULL);
> +ovsthread_once_done();
> +}
> }
> 
> /* Creates and returns a new RSTP instance that initially has no ports. */
> @@ -255,6 +262,8 @@ rstp_create(const char *name, rstp_identifier 
> bridge_address,
> 
> VLOG_DBG("Creating RSTP instance");
> 
> +rstp_init();
> +

rstp_init() is already called earlier from the bridge_init(), so I see little 
point calling it from here. Not having multiple call sites would also remove 
the need for most of the changes above.

> rstp = xzalloc(sizeof *rstp);
> rstp->name = xstrdup(name);
> 
> diff --git a/lib/rstp.h b/lib/rstp.h
> index 4942d59..78e07fb 100644
> --- a/lib/rstp.h
> +++ b/lib/rstp.h
> @@ -36,12 +36,6 @@
> #include "compiler.h"
> #include "util.h"
> 
> -/* Thread Safety: Callers passing in RSTP and RSTP port object
> - * pointers must hold a reference to the passed object to ensure that
> - * the object does not become stale while it is being accessed. */
> -
> -extern struct ovs_mutex rstp_mutex;
> -

This change, if needed, should be in a separate patch with it’s own commit 
message.

  Jarno

> #define RSTP_MAX_PORTS 4095
> 
> struct dp_packet;
> -- 
> 1.8.3.1
> 
> 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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


[ovs-dev] Greetings

2017-04-20 Thread Microsoft Corporation SweepstakesPromotion(c)2017

-- 
Attention:Lucky Winner

We are happy to notify you that your e-mail address has been awarded
£750.000GBP in our MICROSOFT E-MAIL PROMO AWARD (c)cash credited to file
REFNO:MSW-L/2017-28793 BATCH NO.43615M.Your secret pin code is 
ML075798522 for claims please contact::Mr Peter Williams claim officer
Email:mrpeter.willi...@yandex.com Tel: +44) 7031996393

Yours Faithfully,
Hon.Dr.Satya Nadella
Microsoft Corporation/CEO

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


Re: [ovs-dev] [PATCH 3/6] stp: Add link-state checking support for stp ports.

2017-04-20 Thread Jarno Rajahalme

> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao  
> wrote:
> 
> When bridge stp enabled, we enable the stp ports despite
> ports are down. When initializing, this patch checks
> link-state of ports and enable or disable them according
> to their link-state. This patch also allow user to enable
> and disable a port when bridge stp is running.
> 

This describes what the patch does but gives little help for understanding why 
this change is needed. STP would notice that the link is down as it is not able 
to exchange BPDUs over that link. Also, a link that is in STP_DISABLED state 
forwards all traffic, so that when the link comes up, but before stp_run() 
manages to enable STP there would be a loop in the network. To prevent this it 
seems to me that we should leave STP enabled also when the link goes down, so 
that STP would have the chance to initially block to port when it comes back up.

  Jarno

> Signed-off-by: nickcooper-zhangtonghao 
> ---
> ofproto/ofproto-dpif.c | 41 +++-
> tests/stp.at   | 73 ++
> 2 files changed, 113 insertions(+), 1 deletion(-)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 4beacda..f015131 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -2488,6 +2488,37 @@ update_stp_port_state(struct ofport_dpif *ofport)
> }
> }
> 
> +static void
> +stp_check_and_update_link_state(struct ofproto_dpif *ofproto)
> +{
> +struct ofport *ofport_;
> +struct ofport_dpif *ofport;
> +bool up;
> +
> +HMAP_FOR_EACH (ofport_, hmap_node, >up.ports) {
> +ofport = ofport_dpif_cast(ofport_);
> +up = netdev_get_carrier(ofport_->netdev);
> +
> +if (ofport->stp_port &&
> +up != (stp_port_get_state(ofport->stp_port) != STP_DISABLED)) {
> +
> +VLOG_DBG("bridge: %s, port: %s is %s, %s it", ofproto->up.name,
> + netdev_get_name(ofport->up.netdev),
> + up ? "up" : "down",
> + up ? "enabling" : "disabling");
> +
> +if (up) {
> +stp_port_enable(ofport->stp_port);
> +stp_port_set_aux(ofport->stp_port, ofport);
> +} else {
> +stp_port_disable(ofport->stp_port);
> +}
> +
> +update_stp_port_state(ofport);
> +}
> +}
> +}
> +
> /* Configures STP on 'ofport_' using the settings defined in 's'.  The
>  * caller is responsible for assigning STP port numbers and ensuring
>  * there are no duplicates. */
> @@ -2518,7 +2549,12 @@ set_stp_port(struct ofport *ofport_,
> /* Set name before enabling the port so that debugging messages can print
>  * the name. */
> stp_port_set_name(sp, netdev_get_name(ofport->up.netdev));
> -stp_port_enable(sp);
> +
> +if (netdev_get_carrier(ofport_->netdev)) {
> +stp_port_enable(sp);
> +} else {
> +stp_port_disable(sp);
> +}
> 
> stp_port_set_aux(sp, ofport);
> stp_port_set_priority(sp, s->priority);
> @@ -2580,6 +2616,9 @@ stp_run(struct ofproto_dpif *ofproto)
> stp_tick(ofproto->stp, MIN(INT_MAX, elapsed));
> ofproto->stp_last_tick = now;
> }
> +
> +stp_check_and_update_link_state(ofproto);
> +
> while (stp_get_changed_port(ofproto->stp, )) {
> struct ofport_dpif *ofport = stp_port_get_aux(sp);
> 
> diff --git a/tests/stp.at b/tests/stp.at
> index 98632a8..de8f971 100644
> --- a/tests/stp.at
> +++ b/tests/stp.at
> @@ -420,6 +420,8 @@ AT_CHECK([ovs-vsctl add-port br1 p8 -- \
>set port p8 other_config:stp-enable=false -- \
> ])
> 
> +ovs-appctl netdev-dummy/set-admin-state up
> +
> ovs-appctl time/stop
> 
> AT_CHECK([ovs-ofctl add-flow br0 "in_port=7 icmp actions=1"])
> @@ -519,6 +521,7 @@ AT_CHECK([
> set interface p6 type=dummy options:pstream=punix:$OVS_RUNDIR/p6.sock 
> ofport_request=6
> ], [0])
> 
> +ovs-appctl netdev-dummy/set-admin-state up
> 
> ovs-appctl time/stop
> 
> @@ -633,6 +636,8 @@ AT_CHECK([
> set interface p2 type=dummy ofport_request=2
> ], [0])
> 
> +ovs-appctl netdev-dummy/set-admin-state up
> +
> ovs-appctl time/stop
> 
> # give time for STP to move initially
> @@ -653,6 +658,8 @@ AT_CHECK([
> set interface p3 type=dummy ofport_request=3
> ], [0])
> 
> +ovs-appctl netdev-dummy/set-admin-state p3 up
> +
> # The new stp port should be a listening state and other
> # stp ports keep forwarding.
> AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
> @@ -676,5 +683,71 @@ AT_CHECK([ovs-appctl stp/show br0 | grep p3], [0], [dnl
>   p3 designated listening  19128.3
> ])
> 
> +AT_CLEANUP
> +
> +AT_SETUP([STP - check link-state when stp is running])
> +OVS_VSWITCHD_START([])
> +
> +AT_CHECK([
> +ovs-vsctl -- \
> +set port br0 other_config:stp-enable=false -- \
> +set bridge br0 datapath-type=dummy 

[ovs-dev] [PATCH] bridge: Log interface deletion

2017-04-20 Thread Andy Zhou
Currently, interface additions are logged but not deletion. This
makes system debugging, such as confirming OVSDB transaction are
timely replicated harder than necessary.

Signed-off-by: Andy Zhou 
---
 vswitchd/bridge.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 867a26d8de19..81bd8074e593 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -4317,6 +4317,9 @@ iface_destroy__(struct iface *iface)
 struct port *port = iface->port;
 struct bridge *br = port->bridge;
 
+VLOG_INFO("bridge %s: deleted interface %s on port %d",
+  br->name, iface->name, iface->ofp_port);
+
 if (br->ofproto && iface->ofp_port != OFPP_NONE) {
 ofproto_port_unregister(br->ofproto, iface->ofp_port);
 }
-- 
1.8.3.1

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


Re: [ovs-dev] [PATCH 1/6] rstp/stp: Unref the rstp/stp when bridges destroyed.

2017-04-20 Thread Jarno Rajahalme
Looks correct to me.

Acked-by: Jarno Rajahalme 

> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao  
> wrote:
> 
> When bridges destroyed, which stp enabled, you can
> still get stp info via the command 'ovs-appctl stp/show'.
> And the rstp is also in the same case. We should unref
> them. The rstp/stp ports have been unregistered via
> 'ofproto_port_unregister' function when ports destroyed.
> We will unref rstp/stp struct in the 'destruct' of
> ofproto-dpif provider.
> 
> Signed-off-by: nickcooper-zhangtonghao 
> ---
> ofproto/ofproto-dpif.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 523adad..4beacda 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1494,6 +1494,8 @@ destruct(struct ofproto *ofproto_)
> hmap_destroy(>bundles);
> mac_learning_unref(ofproto->ml);
> mcast_snooping_unref(ofproto->ms);
> +stp_unref(ofproto->stp);
> +rstp_unref(ofproto->rstp);
> 
> sset_destroy(>ports);
> sset_destroy(>ghost_ports);
> -- 
> 1.8.3.1
> 
> 
> 
> 
> ___
> 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 v2 2/2] datapath: pass extended ACK struct to parsing functions

2017-04-20 Thread Joe Stringer
On 19 April 2017 at 16:54, Jarno Rajahalme  wrote:
> From: Johannes Berg 
>
> Upstream commit:
>
> commit fceb6435e85298f747fee938415057af837f5a8a
> Author: Johannes Berg 
> Date:   Wed Apr 12 14:34:07 2017 +0200
>
> netlink: pass extended ACK struct to parsing functions
>
> Pass the new extended ACK reporting struct to all of the generic
> netlink parsing functions. For now, pass NULL in almost all callers
> (except for some in the core.)
>
> Signed-off-by: Johannes Berg 
> Signed-off-by: David S. Miller 
>
> Signed-off-by: Jarno Rajahalme 
> ---

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


[ovs-dev] Potential Clients

2017-04-20 Thread Andrea Grey
Hi, Trust this email discovers you well!

 

Would you be keen to obtain Telecom Users information for your sales and
marketing campaign?

 

Cisco, Verizon, Shoretel, Mitel, Brocade, Juniper Networks, Video
Conferencing Software, Audio Conferencing Software, IT resellers/VARS, Call
Center/Data Center, Telecom Administrators and many more...

 

If you are interested, please answer back by filling off the void spots
underneath: 

Directed Industry or Technology: 

Work Titles: 

Geography:

 

Data fields we provide: Name, Title, Email, Phone, Company Name, Phone, Fax,
Website, Physical Address, Revenue Size, Employee Size, LinkedIn, SIC/NAICS
codes and numerous more details 

 

We provide Decision Makers (C-Level, VP-Level, Directors and managers)
contacts from all industries across the globe.

 

Appreciate your time and look forward to your response.

 

Kind regards,

Andrea Grey

Lead Generation Executive

 

To Opt-Out please answer "Overlook" in the headline

 

  

 

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


[ovs-dev] [PATCH v2] datapath-windows: Pass fwdCtx to conntrack

2017-04-20 Thread Yin Lin
There dependencies in Contrack module such as NAT and fragmentation on
OvsForwardingContext. This patch will make OvsForwardingContext public
in order to implement these functionalities.

Signed-off-by: Yin Lin 
Acked-by: Alin Serdean 
---
 datapath-windows/ovsext/Actions.c   | 61 ++---
 datapath-windows/ovsext/Actions.h   | 58 +++
 datapath-windows/ovsext/Conntrack.c |  5 +--
 datapath-windows/ovsext/Conntrack.h |  4 +--
 4 files changed, 65 insertions(+), 63 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 46f84bc..3bd00a7 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -71,63 +71,6 @@ typedef struct _OVS_ACTION_STATS {
 OVS_ACTION_STATS ovsActionStats;
 
 /*
- * There a lot of data that needs to be maintained while executing the pipeline
- * as dictated by the actions of a flow, across different functions at 
different
- * levels. Such data is put together in a 'context' structure. Care should be
- * exercised while adding new members to the structure - only add ones that get
- * used across multiple stages in the pipeline/get used in multiple functions.
- */
-typedef struct OvsForwardingContext {
-POVS_SWITCH_CONTEXT switchContext;
-/* The NBL currently used in the pipeline. */
-PNET_BUFFER_LIST curNbl;
-/* NDIS forwarding detail for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
-/* Array of destination ports for 'curNbl'. */
-PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
-/* send flags while sending 'curNbl' into NDIS. */
-ULONG sendFlags;
-/* Total number of output ports, used + unused, in 'curNbl'. */
-UINT32 destPortsSizeIn;
-/* Total number of used output ports in 'curNbl'. */
-UINT32 destPortsSizeOut;
-/*
- * If 'curNbl' is not owned by OVS, they need to be tracked, if they need 
to
- * be freed/completed.
- */
-OvsCompletionList *completionList;
-/*
- * vport number of 'curNbl' when it is passed from the PIF bridge to the 
INT
- * bridge. ie. during tunneling on the Rx side.
- */
-UINT32 srcVportNo;
-
-/*
- * Tunnel key:
- * - specified in actions during tunneling Tx
- * - extracted from an NBL during tunneling Rx
- */
-OvsIPv4TunnelKey tunKey;
-
-/*
- * Tunneling - Tx:
- * To store the output port, when it is a tunneled port. We don't foresee
- * multiple tunneled ports as outport for any given NBL.
- */
-POVS_VPORT_ENTRY tunnelTxNic;
-
-/*
- * Tunneling - Rx:
- * Points to the Internal port on the PIF Bridge, if the packet needs to be
- * de-tunneled.
- */
-POVS_VPORT_ENTRY tunnelRxNic;
-
-/* header information */
-OVS_PACKET_HDR_INFO layers;
-} OvsForwardingContext;
-
-/*
  * --
  * OvsInitForwardingCtx --
  * Function to init/re-init the 'ovsFwdCtx' context as the actions pipeline
@@ -2032,8 +1975,8 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
 }
 }
 
-status = OvsExecuteConntrackAction(ovsFwdCtx.curNbl, layers,
-   key, (const PNL_ATTR)a);
+status = OvsExecuteConntrackAction(, key,
+   (const PNL_ATTR)a);
 if (status != NDIS_STATUS_SUCCESS) {
 OVS_LOG_ERROR("CT Action failed");
 dropReason = L"OVS-conntrack action failed";
diff --git a/datapath-windows/ovsext/Actions.h 
b/datapath-windows/ovsext/Actions.h
index c56c260..1ce6c20 100644
--- a/datapath-windows/ovsext/Actions.h
+++ b/datapath-windows/ovsext/Actions.h
@@ -20,6 +20,64 @@
 #include "Switch.h"
 #include "PacketIO.h"
 
+
+/*
+ * There a lot of data that needs to be maintained while executing the pipeline
+ * as dictated by the actions of a flow, across different functions at 
different
+ * levels. Such data is put together in a 'context' structure. Care should be
+ * exercised while adding new members to the structure - only add ones that get
+ * used across multiple stages in the pipeline/get used in multiple functions.
+ */
+typedef struct OvsForwardingContext {
+POVS_SWITCH_CONTEXT switchContext;
+/* The NBL currently used in the pipeline. */
+PNET_BUFFER_LIST curNbl;
+/* NDIS forwarding detail for 'curNbl'. */
+PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
+/* Array of destination ports for 'curNbl'. */
+PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
+/* send flags while sending 'curNbl' into NDIS. */
+ULONG sendFlags;
+/* Total number of output ports, used + unused, in 'curNbl'. */
+UINT32 destPortsSizeIn;
+/* Total number of used output ports in 'curNbl'. */
+  

Re: [ovs-dev] [PATCH v5] [RFC] ovn-controller: add quiet mode

2017-04-20 Thread Han Zhou
>
> [1] http://openvswitch.org/pipermail/dev/2016-August/078272.html

This link is old, should be:
https://mail.openvswitch.org/pipermail/ovs-dev/2016-August/078272.html


> Notes:
> v4->v5: Based on the old patch from Ryan Moats and rebased to current
> master. Current version is RFC only because a problem is found in
> testing. Sometimes latest SB DB change is not seen by ovn-controller:
> cur_seqno in ovn-controller stays behind the real seqno seen by
> ovsdb-tool,

Please ignore this ovsdb-tool part, which is misleading. ovsdb-tool would
never see seqno of an idl :o)
The problem seems to be related to ctx.ovs_idl_txn. I am still debugging.

>   but ovn-controller does not wakeup until a new
change
> happens in SB DB, which can be triggered by, e.g.:
> ovn-nbctl --wait=hv sync.

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


[ovs-dev] Linkedin Para su Empresa - En Línea

2017-04-20 Thread Excel - Fórmulas y Atajos
En línea y en Vivo / Para todo su Equipo con una sola Conexión

Cómo user Exitosamente la red Linkedin para su Empresa

27 de abril - Online en Vivo - 10:00 a 13:00 Hrs   Fórmulas y Atajos para el 
Manejo Óptimo de Excel

05 de abril - Online en Vivo - 10:00 a 13:00 Hrs  
Facebook y Twitter son una excelente herramienta para conectarse con sus 
clientes; sin embargo, su estrategia de redes sociales no está completa si no 
considera también el aspecto profesional, el posicionamiento y las 
oportunidades de networking que únicamente Linkedin puede proveerle. Es 
importante conocer las fórmulas y los atajos que tiene Excel y su fácil 
aplicabilidad, ya que todas las empresas terminan reportando en Excel. Hoy en 
día compatible con smartphones, tabletas, laptops, en la nube. ¡Todos 
utilizamos Excel, es flexible y cuenta con fórmulas y funciones para todo!
 
¿Requiere la información a la Brevedad?
responda este email con la palabra: 
Info - Linkedin.
centro telefónico: 018002129393
 

Lic. Arturo López
Coordinador de Evento
 ¿Requiere la información a la Brevedad?
responda este email con la palabra: 
Info - Excel.
centro telefónico: 018002129393
 

Lic. Karla Borjas
Coordinador de Evento
 



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


Re: [ovs-dev] [PATCH v2 1/2] datapath: Fix refcount leak on force commit.

2017-04-20 Thread Joe Stringer
On 19 April 2017 at 16:54, Jarno Rajahalme  wrote:
> Upstream commit:
>
> commit b768b16de58d5e0b1d7c3f936825b25327ced20c
> Author: Jarno Rajahalme 
> Date:   Tue Mar 28 11:25:26 2017 -0700
>
> openvswitch: Fix refcount leak on force commit.
>
> The reference count held for skb needs to be released when the skb's
> nfct pointer is cleared regardless of if nf_ct_delete() is called or
> not.
>
> Failing to release the skb's reference cound led to deferred conntrack
> cleanup spinning forever within nf_conntrack_cleanup_net_list() when
> cleaning up a network namespace:
>
>kworker/u16:0-19025 [004] 45981067.173642: sched_switch: 
> kworker/u16:0:19025 [120] R ==> rcu_preempt:7 [120]
>kworker/u16:0-19025 [004] 45981067.173651: kernel_stack: 
> => ___preempt_schedule (a001ed36)
> => _raw_spin_unlock_bh (a0713290)
> => nf_ct_iterate_cleanup (c00a4454)
> => nf_conntrack_cleanup_net_list (c00a5e1e)
> => nf_conntrack_pernet_exit (c00a63dd)
> => ops_exit_list.isra.1 (a06075f3)
> => cleanup_net (a0607df0)
> => process_one_work (a0084c31)
> => worker_thread (a008592b)
> => kthread (a008bee2)
> => ret_from_fork (a071b67c)
>
> Fixes: dd41d33f0b03 ("openvswitch: Add force commit.")
> Reported-by: Yang Song 
> Signed-off-by: Jarno Rajahalme 
> Acked-by: Joe Stringer 
> Signed-off-by: David S. Miller 
>
> Signed-off-by: Jarno Rajahalme 

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


Re: [ovs-dev] [PATCH ovs V7 19/24] netdev-vport: Use common offloads interface

2017-04-20 Thread Joe Stringer
On 20 April 2017 at 03:57, Roi Dayan  wrote:
>
>
> On 14/04/2017 04:19, Joe Stringer wrote:
>>
>> On 7 April 2017 at 06:13, Roi Dayan  wrote:
>>>
>>> From: Paul Blakey 
>>>
>>> netdev vports are backed by actualy netdev at the kernel
>>> level, so they can use the common netdev-tc offloads interface
>>> for flow offloading (if enabled).
>>>
>>> Signed-off-by: Paul Blakey 
>>> Signed-off-by: Simon Horman 
>>> Reviewed-by: Roi Dayan 
>>> ---
>>
>>
>> 
>>
>>> @@ -779,11 +783,37 @@ get_stats(const struct netdev *netdev, struct
>>> netdev_stats *stats)
>>>  return 0;
>>>  }
>>>
>>> -
>>
>>
>> Please don't drop the ^L, they're used to separate the file into
>> logically separate sections.
>
>
> ok.
>
>
>>
>>> +#ifdef __linux__
>>> +static int
>>> +netdev_vport_get_ifindex__(const struct netdev *netdev_)
>>> +{
>>> +char buf[NETDEV_VPORT_NAME_BUFSIZE];
>>> +const char *name = netdev_vport_get_dpif_port(netdev_, buf,
>>> sizeof(buf));
>>> +
>>> +return linux_get_ifindex(name);
>>
>>
>> Why link directly against linux implementation, ignoring that netdev
>> provides netdev_get_ifindex() ?
>>
>
> netdev_get_ifindex() is the interface that got us into the vport_class
> callback which we set for vxlan interface, netdev_vport_get_ifindex__().
> by default vxlan vport class have NULL for the get_ifindex callback
> and we added that if we are in Linux we call the linux implementation.
> if we will call netdev_get_ifindex() here we will be in a loop.

OK, thanks for the explanation.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovs V7 15/24] netdev-tc-offloads: Implement netdev flow del using tc interface

2017-04-20 Thread Joe Stringer
On 19 April 2017 at 22:07, Roi Dayan  wrote:
>
>
> On 14/04/2017 04:15, Joe Stringer wrote:
>>
>> On 7 April 2017 at 06:13, Roi Dayan  wrote:
>>>
>>> From: Paul Blakey 
>>>
>>> Signed-off-by: Paul Blakey 
>>> Reviewed-by: Roi Dayan 
>>> Reviewed-by: Simon Horman 
>>> ---
>>>  lib/netdev-tc-offloads.c | 33 ++---
>>>  1 file changed, 30 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
>>> index 4ef29d1..d5961e2 100644
>>> --- a/lib/netdev-tc-offloads.c
>>> +++ b/lib/netdev-tc-offloads.c
>>> @@ -874,10 +874,37 @@ netdev_tc_flow_get(struct netdev *netdev
>>> OVS_UNUSED,
>>>
>>>  int
>>>  netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
>>> -   const ovs_u128 *ufid OVS_UNUSED,
>>> -   struct dpif_flow_stats *stats OVS_UNUSED)
>>> +   const ovs_u128 *ufid,
>>> +   struct dpif_flow_stats *stats)
>>>  {
>>> -return EOPNOTSUPP;
>>> +struct netdev *dev;
>>> +int prio = 0;
>>> +int ifindex;
>>> +int handle;
>>> +int error;
>>> +
>>> +handle = get_ufid_tc_mapping(ufid, , );
>>> +if (!handle) {
>>> +return ENOENT;
>>> +}
>>> +
>>> +ifindex = netdev_get_ifindex(dev);
>>> +if (ifindex < 0) {
>>> +VLOG_ERR_RL(_err, "failed to get ifindex for %s: %s",
>>> +netdev_get_name(dev), ovs_strerror(-ifindex));
>>> +netdev_close(dev);
>>
>>
>> Whose reference is this?
>>
>> If the ifindex can't be gotten, was it ever inserted into the
>> ufid_tc_mapping in the first place?
>>
>> If I follow, there's one reference on the netdev associated with the
>> flow, then two associated with the ufid_tc mapping.. is the caller
>> also likely to have a reference?
>
>
> the reference is from calling get_ufid_tc_mapping() in the start of the
> function.

OK. Might also be worth mentioning in the comment for
get_ufid_tc_mapping() that the caller is responsible for closing
'netdev' if a handle is successfully returned.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] system-ovn.at: Add test for ping other router's port on distributed router

2017-04-20 Thread Mickey Spiegel
I forgot one other comment.

On Thu, Apr 20, 2017 at 11:05 AM, Mickey Spiegel 
wrote:

>
> On Tue, Apr 18, 2017 at 4:49 AM, Guoshuai Li  wrote:
>
>> Signed-off-by: Guoshuai Li 
>> ---
>>  tests/system-ovn.at | 101 ++
>> ++
>>  tests/system-traffic.at |  20 ++
>>  2 files changed, 121 insertions(+)
>>
>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
>> index dd62bd1..68da38a 100644
>> --- a/tests/system-ovn.at
>> +++ b/tests/system-ovn.at
>
>
>  ... I have not looked at the system-ovn test yet.
>
>>
>>
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index c042773..295e606 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -3678,3 +3678,23 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 1 -w 3
>> 10.4.2.2], [1], [ignore])
>>
>>  OVS_TRAFFIC_VSWITCHD_STOP(["/dropping VLAN \(0\|300\) packet received
>> on dot1q-tunnel port/d"])
>>  AT_CLEANUP
>> +
>> +AT_SETUP([datapath - SNAT and UNSNAT])
>>
>
The name should be more specific. This does not just test SNAT and UNSNAT
in the datapath, it includes an action in between that forces processing to
userspace. Something like "datapath - SNAT, userspace action, UNSNAT"?

Mickey


> +OVS_TRAFFIC_VSWITCHD_START()
>> +
>> +AT_CHECK([ovs-ofctl add-flow br0 "table=0, 
>> priority=100,in_port=1,ip,nw_dst=20.0.0.2
>> actions=dec_ttl(),mod_dl_src:00:00:02:01:02:01,mod_dl_dst:00
>> :00:02:01:02:02,resubmit(,1)"])
>> +AT_CHECK([ovs-ofctl add-flow br0 "table=1, 
>> priority=100,ip,nw_src=192.168.1.2
>> actions=ct(commit,table=2,zone=6,nat(src=20.0.0.1))"])
>>
>
> There should be another table added here with a flow that does the clone
> with nested ct_clear actions. The use of ct_clear changes how the unsnat in
> table 3 is processed.
>
> Mickey
>
>
>> +AT_CHECK([ovs-ofctl add-flow br0 "table=2, 
>> priority=100,icmp,nw_dst=20.0.0.2,icmp_type=8,icmp_code=0
>> actions=push:NXM_OF_IP_SRC[],push:NXM_OF_IP_DST[],pop:NXM_OF
>> _IP_SRC[],pop:NXM_OF_IP_DST[],load:0xff->NXM_NX_IP_TTL[],loa
>> d:0->NXM_OF_ICMP_TYPE[],dec_ttl(),mod_dl_src:00:00:02:01:
>> 02:02,mod_dl_dst:00:00:02:01:02:01,resubmit(,3)"])
>> +AT_CHECK([ovs-ofctl add-flow br0 "table=3, priority=100,ip,nw_dst=20.0.0.1
>> actions=ct(table=4,zone=6,nat)"])
>> +AT_CHECK([ovs-ofctl add-flow br0 "table=4, 
>> priority=100,ip,nw_dst=192.168.1.2
>> actions=dec_ttl(),mod_dl_src:00:00:01:01:02:01,mod_dl_dst:f0
>> :00:00:01:02:01,load:0->NXM_OF_IN_PORT[],output:1"])
>> +
>> +ADD_NAMESPACES(foo1)
>> +ADD_VETH(foo1, foo1, br0, "192.168.1.2/24", "f0:00:00:01:02:01",
>> "192.168.1.1")
>> +NS_CHECK_EXEC([foo1], [arp -s 192.168.1.1 00:00:01:01:02:01])
>> +
>> +NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 20.0.0.2 | FORMAT_PING],
>> [0], [dnl
>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> +])
>> +
>> +OVS_TRAFFIC_VSWITCHD_STOP
>> +AT_CLEANUP
>> --
>> 2.10.1.windows.1
>>
>> This patch is used to analyze "ovn: unsnat handling error for Distributed
>> Gateway" problems:
>>
>> https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331033.html
>>
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] system-ovn.at: Add test for ping other router's port on distributed router

2017-04-20 Thread Mickey Spiegel
On Tue, Apr 18, 2017 at 4:49 AM, Guoshuai Li  wrote:

> Signed-off-by: Guoshuai Li 
> ---
>  tests/system-ovn.at | 101 ++
> ++
>  tests/system-traffic.at |  20 ++
>  2 files changed, 121 insertions(+)
>
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index dd62bd1..68da38a 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at


 ... I have not looked at the system-ovn test yet.

>
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index c042773..295e606 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -3678,3 +3678,23 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 1 -w 3
> 10.4.2.2], [1], [ignore])
>
>  OVS_TRAFFIC_VSWITCHD_STOP(["/dropping VLAN \(0\|300\) packet received on
> dot1q-tunnel port/d"])
>  AT_CLEANUP
> +
> +AT_SETUP([datapath - SNAT and UNSNAT])
> +OVS_TRAFFIC_VSWITCHD_START()
> +
> +AT_CHECK([ovs-ofctl add-flow br0 "table=0, 
> priority=100,in_port=1,ip,nw_dst=20.0.0.2
> actions=dec_ttl(),mod_dl_src:00:00:02:01:02:01,mod_dl_dst:
> 00:00:02:01:02:02,resubmit(,1)"])
> +AT_CHECK([ovs-ofctl add-flow br0 "table=1, priority=100,ip,nw_src=192.168.1.2
> actions=ct(commit,table=2,zone=6,nat(src=20.0.0.1))"])
>

There should be another table added here with a flow that does the clone
with nested ct_clear actions. The use of ct_clear changes how the unsnat in
table 3 is processed.

Mickey


> +AT_CHECK([ovs-ofctl add-flow br0 "table=2, 
> priority=100,icmp,nw_dst=20.0.0.2,icmp_type=8,icmp_code=0
> actions=push:NXM_OF_IP_SRC[],push:NXM_OF_IP_DST[],pop:NXM_
> OF_IP_SRC[],pop:NXM_OF_IP_DST[],load:0xff->NXM_NX_IP_TTL[],
> load:0->NXM_OF_ICMP_TYPE[],dec_ttl(),mod_dl_src:00:00:02:
> 01:02:02,mod_dl_dst:00:00:02:01:02:01,resubmit(,3)"])
> +AT_CHECK([ovs-ofctl add-flow br0 "table=3, priority=100,ip,nw_dst=20.0.0.1
> actions=ct(table=4,zone=6,nat)"])
> +AT_CHECK([ovs-ofctl add-flow br0 "table=4, priority=100,ip,nw_dst=192.168.1.2
> actions=dec_ttl(),mod_dl_src:00:00:01:01:02:01,mod_dl_dst:
> f0:00:00:01:02:01,load:0->NXM_OF_IN_PORT[],output:1"])
> +
> +ADD_NAMESPACES(foo1)
> +ADD_VETH(foo1, foo1, br0, "192.168.1.2/24", "f0:00:00:01:02:01",
> "192.168.1.1")
> +NS_CHECK_EXEC([foo1], [arp -s 192.168.1.1 00:00:01:01:02:01])
> +
> +NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 20.0.0.2 | FORMAT_PING],
> [0], [dnl
> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> +])
> +
> +OVS_TRAFFIC_VSWITCHD_STOP
> +AT_CLEANUP
> --
> 2.10.1.windows.1
>
> This patch is used to analyze "ovn: unsnat handling error for Distributed
> Gateway" problems:
>
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331033.html
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovs V7 11/24] dpif-netlink: Use netdev flow put api to insert a flow

2017-04-20 Thread Joe Stringer
On 19 April 2017 at 23:13, Roi Dayan  wrote:
>
>
> On 14/04/2017 04:08, Joe Stringer wrote:
>>
>> On 7 April 2017 at 06:12, Roi Dayan  wrote:
>>>
>>> From: Paul Blakey 
>>>
>>> Using the new netdev flow api operate will now try and
>>> offload flows to the relevant netdev of the input port.
>>> Other operate methods flows will come in later patches.
>>>
>>> Signed-off-by: Paul Blakey 
>>> Reviewed-by: Roi Dayan 
>>> Reviewed-by: Simon Horman 
>>> ---
>>
>>
>> 
>>
>>>  static void
>>> -dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t
>>> n_ops)
>>> +dbg_print_flow(const struct nlattr *key, size_t key_len,
>>> +   const struct nlattr *mask, size_t mask_len,
>>> +   const struct nlattr *actions, size_t actions_len,
>>> +   const ovs_u128 *ufid,
>>> +   const char *op)
>>
>>
>> I wonder if we could refactor log_flow_message() into somewhere
>> neutral and share the output format for these flows from here and
>> there.
>
>
> we'll check it. can also use maybe the other logging functions as well
> (put/del/get).
> Need to modify to use the actual current module for logging instead of the
> static this_module else everything will be logged as the dpif module.

True, good point. Feel free to do this as a follow-on patch for the series.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v5] [RFC] ovn-controller: add quiet mode

2017-04-20 Thread Han Zhou
As discussed in [1], what the incremental processing code
actually accomplished was that the ovn-controller would
be "quiet" and not burn CPU when things weren't changing.
This patch set recreates this state by calculating whether
changes have occured that would require a full calculation
to be performed.  It does this by persisting a copy of
the localvif_to_ofport and tunnel information in the
controller module, rather than in the physical.c module
as was the case with previous commits.

Performance improved extremely in below test scenario:

- 1 lswitch with 10 lports bound locally
- Each lport has an ingress ACL, referencing the same address-set
- The address-set has 10,000 IPv4 addresses

For each IP address in the address-set, there will be 3
OpenFlow rules generated for each ACL. So the total number
of rules is 300k+.

Without the patch, it takes 50+ minutes to install all the
rules to ovs-vswitchd.

With the patch, it takes 20 seconds to install all the rules
to ovs-vswitchd.

The reason is that the large number of rules are sent to
ovs-vswitchd gradually in many iterations of ovn-controller
main loop. Without the patch, cpu cycles are wasted in
lflow_run to re-processing the large address set in every
main loop iteration. With the patch, lflow_run is not
executed in most iterations because there is no change of
input.

[1] http://openvswitch.org/pipermail/dev/2016-August/078272.html

Signed-off-by: Ryan Moats 
Signed-off-by: Han Zhou 
---

Notes:
v4->v5: Based on the old patch from Ryan Moats and rebased to current
master. Current version is RFC only because a problem is found in
testing. Sometimes latest SB DB change is not seen by ovn-controller:
cur_seqno in ovn-controller stays behind the real seqno seen by
ovsdb-tool, but ovn-controller does not wakeup until a new change
happens in SB DB, which can be triggered by, e.g.:
ovn-nbctl --wait=hv sync.

 ovn/controller/ofctrl.c |   2 +
 ovn/controller/ovn-controller.c |  59 ---
 ovn/controller/ovn-controller.h |   1 +
 ovn/controller/patch.c  |   2 +
 ovn/controller/physical.c   | 102 +---
 ovn/controller/physical.h   |   5 ++
 6 files changed, 116 insertions(+), 55 deletions(-)

diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 10c8105..20e4ab6 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -383,6 +383,8 @@ run_S_CLEAR_FLOWS(void)
 queue_msg(encode_group_mod());
 ofputil_uninit_group_mod();
 
+force_full_process();
+
 /* Clear existing groups, to match the state of the switch. */
 if (groups) {
 ovn_group_table_clear(groups, true);
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index e00f57a..2822eb7 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -484,6 +484,23 @@ get_nb_cfg(struct ovsdb_idl *idl)
 return sb ? sb->nb_cfg : 0;
 }
 
+/* Contains mapping of localvifs to OF ports for detecting if the physical
+ * configuration of the switch has changed. */
+static struct simap localvif_to_ofport =
+SIMAP_INITIALIZER(_to_ofport);
+
+/* Contains the list of known tunnels. */
+static struct hmap tunnels = HMAP_INITIALIZER();
+
+/* The last sequence number seen from the southbound IDL. */
+static unsigned int seqno = 0;
+
+void
+force_full_process(void)
+{
+seqno = 0;
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -613,10 +630,8 @@ main(int argc, char *argv[])
 
 struct ldatapath_index ldatapaths;
 struct lport_index lports;
-struct mcgroup_index mcgroups;
 ldatapath_index_init(, ctx.ovnsb_idl);
 lport_index_init(, ctx.ovnsb_idl);
-mcgroup_index_init(, ctx.ovnsb_idl);
 
 const struct sbrec_chassis *chassis = NULL;
 if (chassis_id) {
@@ -626,36 +641,47 @@ main(int argc, char *argv[])
 _datapaths, _lports);
 }
 
+bool physical_change = true;
 if (br_int && chassis) {
+enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int,
+ _ct_zones);
+physical_change = detect_and_save_physical_changes(
+_to_ofport, , mff_ovn_geneve, br_int,
+chassis);
+unsigned int cur_seqno = ovsdb_idl_get_seqno(ovnsb_idl_loop.idl);
+
 struct shash addr_sets = SHASH_INITIALIZER(_sets);
 addr_sets_init(, _sets);
 
 patch_run(, br_int, chassis, _datapaths);
 
-enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int,
- _ct_zones);
-
 pinctrl_run(, , br_int, chassis, _datapaths);
 update_ct_zones(_lports, _datapaths, _zones,
 ct_zone_bitmap, _ct_zones);
 if (ctx.ovs_idl_txn) {
+if 

Re: [ovs-dev] datapath-windows: Pass fwdCtx to conntrack

2017-04-20 Thread Guru Shetty
On 19 April 2017 at 13:18, Yin Lin  wrote:

> There dependencies in Contrack module such as NAT and fragmentation on
> OvsForwardingContext. This patch will make OvsForwardingContext public
> in order to implement these functionalities.
>

Can you please respin this with a Signed-off-by and Acked-by?


> ---
>  datapath-windows/ovsext/Actions.c   | 61 ++
> ---
>  datapath-windows/ovsext/Actions.h   | 58 ++
> +
>  datapath-windows/ovsext/Conntrack.c |  5 +--
>  datapath-windows/ovsext/Conntrack.h |  4 +--
>  4 files changed, 65 insertions(+), 63 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/
> Actions.c
> index 46f84bc..3bd00a7 100644
> --- a/datapath-windows/ovsext/Actions.c
> +++ b/datapath-windows/ovsext/Actions.c
> @@ -71,63 +71,6 @@ typedef struct _OVS_ACTION_STATS {
>  OVS_ACTION_STATS ovsActionStats;
>
>  /*
> - * There a lot of data that needs to be maintained while executing the
> pipeline
> - * as dictated by the actions of a flow, across different functions at
> different
> - * levels. Such data is put together in a 'context' structure. Care
> should be
> - * exercised while adding new members to the structure - only add ones
> that get
> - * used across multiple stages in the pipeline/get used in multiple
> functions.
> - */
> -typedef struct OvsForwardingContext {
> -POVS_SWITCH_CONTEXT switchContext;
> -/* The NBL currently used in the pipeline. */
> -PNET_BUFFER_LIST curNbl;
> -/* NDIS forwarding detail for 'curNbl'. */
> -PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
> -/* Array of destination ports for 'curNbl'. */
> -PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts;
> -/* send flags while sending 'curNbl' into NDIS. */
> -ULONG sendFlags;
> -/* Total number of output ports, used + unused, in 'curNbl'. */
> -UINT32 destPortsSizeIn;
> -/* Total number of used output ports in 'curNbl'. */
> -UINT32 destPortsSizeOut;
> -/*
> - * If 'curNbl' is not owned by OVS, they need to be tracked, if they
> need to
> - * be freed/completed.
> - */
> -OvsCompletionList *completionList;
> -/*
> - * vport number of 'curNbl' when it is passed from the PIF bridge to
> the INT
> - * bridge. ie. during tunneling on the Rx side.
> - */
> -UINT32 srcVportNo;
> -
> -/*
> - * Tunnel key:
> - * - specified in actions during tunneling Tx
> - * - extracted from an NBL during tunneling Rx
> - */
> -OvsIPv4TunnelKey tunKey;
> -
> -/*
> - * Tunneling - Tx:
> - * To store the output port, when it is a tunneled port. We don't
> foresee
> - * multiple tunneled ports as outport for any given NBL.
> - */
> -POVS_VPORT_ENTRY tunnelTxNic;
> -
> -/*
> - * Tunneling - Rx:
> - * Points to the Internal port on the PIF Bridge, if the packet needs
> to be
> - * de-tunneled.
> - */
> -POVS_VPORT_ENTRY tunnelRxNic;
> -
> -/* header information */
> -OVS_PACKET_HDR_INFO layers;
> -} OvsForwardingContext;
> -
> -/*
>   * 
> --
>   * OvsInitForwardingCtx --
>   * Function to init/re-init the 'ovsFwdCtx' context as the actions
> pipeline
> @@ -2032,8 +1975,8 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT
> switchContext,
>  }
>  }
>
> -status = OvsExecuteConntrackAction(ovsFwdCtx.curNbl, layers,
> -   key, (const PNL_ATTR)a);
> +status = OvsExecuteConntrackAction(, key,
> +   (const PNL_ATTR)a);
>  if (status != NDIS_STATUS_SUCCESS) {
>  OVS_LOG_ERROR("CT Action failed");
>  dropReason = L"OVS-conntrack action failed";
> diff --git a/datapath-windows/ovsext/Actions.h b/datapath-windows/ovsext/
> Actions.h
> index c56c260..1ce6c20 100644
> --- a/datapath-windows/ovsext/Actions.h
> +++ b/datapath-windows/ovsext/Actions.h
> @@ -20,6 +20,64 @@
>  #include "Switch.h"
>  #include "PacketIO.h"
>
> +
> +/*
> + * There a lot of data that needs to be maintained while executing the
> pipeline
> + * as dictated by the actions of a flow, across different functions at
> different
> + * levels. Such data is put together in a 'context' structure. Care
> should be
> + * exercised while adding new members to the structure - only add ones
> that get
> + * used across multiple stages in the pipeline/get used in multiple
> functions.
> + */
> +typedef struct OvsForwardingContext {
> +POVS_SWITCH_CONTEXT switchContext;
> +/* The NBL currently used in the pipeline. */
> +PNET_BUFFER_LIST curNbl;
> +/* NDIS forwarding detail for 'curNbl'. */
> +PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
> +/* Array of destination ports for 'curNbl'. */
> +

Re: [ovs-dev] [PATCH] ofproto-dpif : propagate may_enable flag as link aliveness

2017-04-20 Thread László Sürü
Thanks for the review and comments.
All unit tests were passing for me before with 'make check', I haven't seen 
failing unit tests.
It might have been a temporary misbehavior.

I've applied the proposed changes and successfully rerun the tests 
also with 'testsuite' this time based on your attached log (see attachment).
Also, I've adapted the tests to the new OF 1.6 test as well.

Please find the modified patched below in this mail.

Thanks and regards
Laszlo

-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org] 
Sent: Saturday, April 15, 2017 6:44 AM
To: László Sürü 
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] ofproto-dpif : propagate may_enable flag as link 
aliveness

Thanks a lot for the revised patch!  It will be good to get this done properly.

I'm appending some changes that I suggest folding in to better match the usual 
OVS coding style.

However, when I run the testsuite, I get the failures below.  Do you see these 
too?  Can you take a look at them?  I'm also attaching the full testsuite.log 
in case it helps.

bfd

 22: bfd - liveness propagation - OF1.3  FAILED (bfd.at:847)
 23: bfd - liveness propagation - OF1.4  FAILED (bfd.at:920)
 24: bfd - liveness propagation - OF1.5  FAILED (bfd.at:993)

ofproto

890: ofproto - mod-port (OpenFlow 1.6)   FAILED (ofproto.at:1308)

--8<--cut here-->8--
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 218e8eb..c82640d 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1894,6 +1894,16 @@ port_modified(struct ofport *port_)
 bfd_set_netdev(port->bfd, netdev);
 }
 
+/* Set liveness, unless the link is administratively or
+ * operationally down or link monitoring false */
+if (!(port->up.pp.config & OFPUTIL_PC_PORT_DOWN) &&
+!(port->up.pp.state & OFPUTIL_PS_LINK_DOWN) &&
+port->may_enable) {
+port->up.pp.state |= OFPUTIL_PS_LIVE;
+} else {
+port->up.pp.state &= ~OFPUTIL_PS_LIVE;
+}
+
 ofproto_dpif_monitor_port_update(port, port->bfd, port->cfm,
  port->lldp, >up.pp.hw_addr);
 
@@ -3457,6 +3467,19 @@ port_run(struct ofport_dpif *ofport)
 if (ofport->rstp_port) {
 rstp_port_set_mac_operational(ofport->rstp_port, enable);
 }
+
+/* Propagate liveness, unless the link is administratively or
+ * operationally down. */
+if (!(ofport->up.pp.config & OFPUTIL_PC_PORT_DOWN) &&
+!(ofport->up.pp.state & OFPUTIL_PS_LINK_DOWN)) {
+enum ofputil_port_state of_state = ofport->up.pp.state;
+if (enable) {
+of_state |= OFPUTIL_PS_LIVE;
+} else {
+of_state &= ~OFPUTIL_PS_LIVE;
+}
+ofproto_port_set_state(>up, of_state);
+}
 }
 
 ofport->may_enable = enable;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 7440d5b..d41e35a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2473,9 +2473,6 @@ ofport_modified(struct ofport *port, struct 
ofputil_phy_port *pp)
 port->pp.peer = pp->peer;
 port->pp.curr_speed = pp->curr_speed;
 port->pp.max_speed = pp->max_speed;
-
-connmgr_send_port_status(port->ofproto->connmgr, NULL,
- >pp, OFPPR_MODIFY);
 }
 
 /* Update OpenFlow 'state' in 'port' and notify controller. */
@@ -2633,7 +2630,8 @@ update_port(struct ofproto *ofproto, const char *name)
 struct netdev *old_netdev = port->netdev;
 
 /* 'name' hasn't changed location.  Any properties changed? */
-if (!ofport_equal(>pp, )) {
+bool port_changed = !ofport_equal(>pp, );
+if (port_changed) {
 ofport_modified(port, );
 }
 
@@ -2649,6 +2647,12 @@ update_port(struct ofproto *ofproto, const char *name)
 port->ofproto->ofproto_class->port_modified(port);
 }
 
+/* Send status update, if any port property changed */
+if (port_changed) {
+connmgr_send_port_status(port->ofproto->connmgr, NULL,
+ >pp, OFPPR_MODIFY);
+}
+
 netdev_close(old_netdev);
 } else {
 /* If 'port' is nonnull then its name differs from 'name' and thus
diff --git a/tests/bfd.at b/tests/bfd.at
index dee8124..81c7db2 100644
--- a/tests/bfd.at
+++ b/tests/bfd.at
@@ -830,3 +830,222 @@ BFD_CHECK([p1], [false], [false], [none], [down], 
[Control Detection Time Expire
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+# test bfd: liveness propagation - OF1.3.
+AT_SETUP([bfd - liveness propagation - OF1.3])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl -O OpenFlow13 -P standard monitor br0 --detach --no-chdir 
--pidfile])
+check_liveness () {
+printf '\n\n--- check_liveness %d ---\n\n\n' $1
+shift
+
+   

[ovs-dev] [PATCH 3/3] ovn-controller: document command-line options in man page

2017-04-20 Thread Lance Richardson
Signed-off-by: Lance Richardson 
---
 ovn/controller/ovn-controller.8.xml | 64 +
 1 file changed, 23 insertions(+), 41 deletions(-)

diff --git a/ovn/controller/ovn-controller.8.xml 
b/ovn/controller/ovn-controller.8.xml
index f9cbbfe..dcca716 100644
--- a/ovn/controller/ovn-controller.8.xml
+++ b/ovn/controller/ovn-controller.8.xml
@@ -20,6 +20,26 @@
   machine-local and do not run over a physical network.
 
 
+Options
+  
+Daemon Options
+http://www.w3.org/2003/XInclude"/>
+
+Logging Options
+http://www.w3.org/2003/XInclude"/>
+
+PKI Options
+
+  PKI configuration is required in order to use SSL for the connections to
+  the Northbound and Southbound databases.
+
+http://www.w3.org/2003/XInclude"/>
+
+Other Options
+
+http://www.w3.org/2003/XInclude"/>
+
+
 Configuration
 
   ovn-controller retrieves most of its configuration
@@ -28,47 +48,9 @@
   vSwitch's "run" directory.  It may be overridden by specifying the
   ovs-database argument in one of the following forms:
 
-
-  
-
-  ssl:ip:port
-
-
-  The specified SSL port on the host at the given
-  ip, which must be expressed as an IP address (not a DNS
-  name) in IPv4 or IPv6 address format.  If ip is an IPv6
-  address, then wrap ip with square brackets, e.g.:
-  ssl:[::1]:6640.  The --private-key,
-  --certificate and either of --ca-cert
-  or --bootstrap-ca-cert options are mandatory when this
-  form is used.
-
-  
-  
-
-  tcp:ip:port
-
-
-  Connect to the given TCP port on ip, where
-  ip can be IPv4 or IPv6 address. If ip is an
-  IPv6 address, then wrap ip with square brackets, e.g.:
-  tcp:[::1]:6640.
-
-  
-  
-
-  unix:file
-
-
-  On POSIX, connect to the Unix domain server socket named
-  file.
-
-
-  On Windows, connect to a localhost TCP port whose value is written
-  in file.
-
-  
-
+http://www.w3.org/2003/XInclude"/>
+http://www.w3.org/2003/XInclude"/>
+
 
   ovn-controller assumes it gets configuration
   information from the following keys in the Open_vSwitch
-- 
2.7.4

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


[ovs-dev] [PATCH 1/3] ovsdb: add xml equivalents of remote man page fragments

2017-04-20 Thread Lance Richardson
Add XML equivalents for remote-active.man and remote-passive.man
for inclusion by man pages using XML format.

Signed-off-by: Lance Richardson 
---
 ovsdb/remote-active.xml  | 30 ++
 ovsdb/remote-passive.xml | 36 
 2 files changed, 66 insertions(+)
 create mode 100644 ovsdb/remote-active.xml
 create mode 100644 ovsdb/remote-passive.xml

diff --git a/ovsdb/remote-active.xml b/ovsdb/remote-active.xml
new file mode 100644
index 000..fe4801c
--- /dev/null
+++ b/ovsdb/remote-active.xml
@@ -0,0 +1,30 @@
+
+
+  ssl:ip:port
+  
+The specified SSL port on the host at the given ip,
+which must be expressed as an IP address (not a DNS name) in IPv4 or IPv6
+address format.  If ip is an IPv6 address, then wrap
+ip with square brackets, e.g.: ssl:[::1]:6640.
+The --private-key, --certificate, and
+--ca-cert options are mandatory when this form is used.
+  
+  tcp:ip:port
+  
+Connect to the given TCP port on ip, where
+ip can be an IPv4 or IPv6 address.  If ip is an
+IPv6 address, then wrap ip with square brackets, e.g.:
+tcp:[::1]:6640.
+  
+  unix:file
+  
+
+On POSIX, connect to the Unix domain server socket named file.
+
+
+On Windows, connect to a local named pipe that is represented by a file
+created in the path file to mimic the behavior of a Unix domain
+socket.
+
+  
+
diff --git a/ovsdb/remote-passive.xml b/ovsdb/remote-passive.xml
new file mode 100644
index 000..6056b1e
--- /dev/null
+++ b/ovsdb/remote-passive.xml
@@ -0,0 +1,36 @@
+
+
+  pssl:port:ip
+  
+Listen on the given SSL port for a connection.  By default,
+connections are not bound to a particular local IP address and
+it listens only on IPv4 (but not IPv6) addresses, but
+specifying ip limits connections to those from the given
+ip, either IPv4 or IPv6 address.  If ip is
+an IPv6 address, then wrap ip with square brackets, e.g.:
+pssl:6640:[::1]. The --private-key,
+--certificate, and --ca-cert options are
+mandatory when this form is used.
+  
+  ptcp:port:ip
+  
+Listen on the given TCP port for a connection.  By default,
+connections are not bound to a particular local IP address and
+it listens only on IPv4 (but not IPv6) addresses, but
+ip may be specified to listen only for connections to the given
+ip, either IPv4 or IPv6 address.  If ip is
+an IPv6 address, then wrap ip with square brackets, e.g.:
+ptcp:6640:[::1].
+  
+  punix:file
+  
+
+On POSIX, listen on the Unix domain server socket named \fIfile\fR for a
+connection.
+
+
+On Windows, listen on a local named pipe.  A file is created in the
+path file to mimic the behavior of a Unix domain socket.
+
+  
+
-- 
2.7.4

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


[ovs-dev] [PATCH 0/3] document command-line options in ovn man pages

2017-04-20 Thread Lance Richardson
Add command-line option documentation to ovn-northd and ovn-controller
man pages.

Lance Richardson (3):
  ovsdb: add xml equivalents of remote man page fragments
  northd: document command-line options in man page
  ovn-controller: document command-line options in man page

 ovn/controller/ovn-controller.8.xml |  64 ---
 ovn/northd/ovn-northd.8.xml | 101 +---
 ovsdb/remote-active.xml |  30 +++
 ovsdb/remote-passive.xml|  36 +
 4 files changed, 125 insertions(+), 106 deletions(-)
 create mode 100644 ovsdb/remote-active.xml
 create mode 100644 ovsdb/remote-passive.xml

-- 
2.7.4

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


[ovs-dev] [PATCH v2] ofproto-dpif-ipfix: total counters.

2017-04-20 Thread mweglicx
Implementation of IPFix counters which hold
total values measured since metering process startup.

v2: Patch is reapplied for a review because
it hasn't been correctly marked in patchwork.

Signed-off-by: Michal Weglicki 
---
 ofproto/ofproto-dpif-ipfix.c | 110 +--
 1 file changed, 85 insertions(+), 25 deletions(-)

diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index 3f90f21..3de81a9 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -84,6 +84,13 @@ enum dpif_ipfix_tunnel_type {
 
 typedef struct ofputil_ipfix_stats ofproto_ipfix_stats;
 
+struct dpif_ipfix_global_stats {
+uint64_t packet_total_count;
+uint64_t octet_total_count;
+uint64_t octet_total_sum_of_squares;
+uint64_t layer2_octet_total_count;
+};
+
 struct dpif_ipfix_port {
 struct hmap_node hmap_node; /* In struct dpif_ipfix's "tunnel_ports" hmap. 
*/
 struct ofport *ofport;  /* To retrieve port stats. */
@@ -103,7 +110,8 @@ struct dpif_ipfix_exporter {
 char *virtual_obs_id;
 uint8_t virtual_obs_len;
 
-ofproto_ipfix_stats stats;
+ofproto_ipfix_stats ofproto_stats;
+struct dpif_ipfix_global_stats ipfix_global_stats;
 };
 
 struct dpif_ipfix_bridge_exporter {
@@ -348,20 +356,24 @@ struct ipfix_data_record_aggregated_common {
 ovs_be32 flow_start_delta_microseconds; /* FLOW_START_DELTA_MICROSECONDS */
 ovs_be32 flow_end_delta_microseconds; /* FLOW_END_DELTA_MICROSECONDS */
 ovs_be64 packet_delta_count;  /* PACKET_DELTA_COUNT */
+ovs_be64 packet_total_count;  /* PACKET_DELTA_COUNT */
 ovs_be64 layer2_octet_delta_count;  /* LAYER2_OCTET_DELTA_COUNT */
+ovs_be64 layer2_octet_total_count;  /* LAYER2_OCTET_DELTA_COUNT */
 uint8_t flow_end_reason;  /* FLOW_END_REASON */
 });
-BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_common) == 25);
+BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_common) == 41);
 
 /* Part of data record for IP aggregated elements. */
 OVS_PACKED(
 struct ipfix_data_record_aggregated_ip {
 ovs_be64 octet_delta_count;  /* OCTET_DELTA_COUNT */
+ovs_be64 octet_total_count;  /* OCTET_TOTAL_COUNT */
 ovs_be64 octet_delta_sum_of_squares;  /* OCTET_DELTA_SUM_OF_SQUARES */
+ovs_be64 octet_total_sum_of_squares;  /* OCTET_TOTAL_SUM_OF_SQUARES */
 ovs_be64 minimum_ip_total_length;  /* MINIMUM_IP_TOTAL_LENGTH */
 ovs_be64 maximum_ip_total_length;  /* MAXIMUM_IP_TOTAL_LENGTH */
 });
-BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_ip) == 32);
+BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_ip) == 48);
 
 /*
  * Refer to RFC 7011, the length of Variable length element is 0~65535:
@@ -444,9 +456,13 @@ struct ipfix_flow_cache_entry {
 uint64_t flow_start_timestamp_usec;
 uint64_t flow_end_timestamp_usec;
 uint64_t packet_delta_count;
+uint64_t packet_total_count;
 uint64_t layer2_octet_delta_count;
+uint64_t layer2_octet_total_count;
 uint64_t octet_delta_count;
+uint64_t octet_total_count;
 uint64_t octet_delta_sum_of_squares;  /* 0 if not IP. */
+uint64_t octet_total_sum_of_squares;  /* 0 if not IP. */
 uint16_t minimum_ip_total_length;  /* 0 if not IP. */
 uint16_t maximum_ip_total_length;  /* 0 if not IP. */
 };
@@ -544,6 +560,9 @@ dpif_ipfix_exporter_init(struct dpif_ipfix_exporter 
*exporter)
 exporter->cache_max_flows = 0;
 exporter->virtual_obs_id = NULL;
 exporter->virtual_obs_len = 0;
+
+memset(>ipfix_global_stats, 0,
+   sizeof(struct dpif_ipfix_global_stats));
 }
 
 static void
@@ -561,6 +580,9 @@ dpif_ipfix_exporter_clear(struct dpif_ipfix_exporter 
*exporter)
 free(exporter->virtual_obs_id);
 exporter->virtual_obs_id = NULL;
 exporter->virtual_obs_len = 0;
+
+memset(>ipfix_global_stats, 0,
+   sizeof(struct dpif_ipfix_global_stats));
 }
 
 static void
@@ -1199,12 +1221,16 @@ ipfix_define_template_fields(enum ipfix_proto_l2 l2, 
enum ipfix_proto_l3 l3,
 DEF(FLOW_START_DELTA_MICROSECONDS);
 DEF(FLOW_END_DELTA_MICROSECONDS);
 DEF(PACKET_DELTA_COUNT);
+DEF(PACKET_TOTAL_COUNT);
 DEF(LAYER2_OCTET_DELTA_COUNT);
+DEF(LAYER2_OCTET_TOTAL_COUNT);
 DEF(FLOW_END_REASON);
 
 if (l3 != IPFIX_PROTO_L3_UNKNOWN) {
 DEF(OCTET_DELTA_COUNT);
+DEF(OCTET_TOTAL_COUNT);
 DEF(OCTET_DELTA_SUM_OF_SQUARES);
+DEF(OCTET_TOTAL_SUM_OF_SQUARES);
 DEF(MINIMUM_IP_TOTAL_LENGTH);
 DEF(MAXIMUM_IP_TOTAL_LENGTH);
 }
@@ -1316,8 +1342,8 @@ ipfix_send_template_msgs(struct dpif_ipfix_exporter 
*exporter,
 tx_errors += error_pkts;
 tx_packets += collectors_count(exporter->collectors) - error_pkts;
 
-exporter->stats.tx_pkts += tx_packets;
-exporter->stats.tx_errors += tx_errors;
+exporter->ofproto_stats.tx_pkts += tx_packets;
+exporter->ofproto_stats.tx_errors += tx_errors;
 
 /* XXX: Add Options 

Re: [ovs-dev] [PATCH] ofproto-dpif: Send QinQ related sFlow counters

2017-04-20 Thread Rzasik, LukaszX
Hi Neil,

Thanks for the comments. I'll try to explain what is my purpose and why I 
implemented it that way.

I'm aiming at implementing QinQ related sFlow counters and it seemed that 
extended_vlantunnel is the right place.
The only other place that I can find where VLAN tags can be reported is 
extended_switch but there is place only for a single src_vlan and dst_vlan. 

>From the description of extended_vlantunnel it fits QinQ scenario ideally. I 
>tried to keep my implementation as close as possible with the description. It 
>does not report all VLAN tags. It reports only stripped VLAN tags when there 
>are multiple tags in the packet. 

Also TODO that you left in the code made me confident that it is the right 
approach:
/* TODO: 802.1AD(QinQ) is not supported by OVS (yet), so do not
  * construct a VLAN-stack. The sFlow user-action cookie already
  * captures the egress VLAN ID so there is nothing more to do here.
  */ 

After your comment I looked for some more information about using 
extended_vlantunnel for reporting non-standard 802.1Q headers but I cannot find 
any information about that in the sFlow spec. 

As you mentioned, the spec states only these three conditions:
 1. The packet has nested vlan tags, AND
 2. The reporting device is VLAN aware, AND
 3. One or more VLAN tags have been stripped, either
because they represent proprietary encapsulations, or
because switch hardware automatically strips the outer VLAN
encapsulation.

And I think they are satisfied in the implementation. 

Could you explain in more details why you think this is not the right place to 
report stripped VLAN tags in QinQ scenario?

The implementation does not consider 0x9100 ethernet type as I could not find 
it anywhere used in OVS code.
I could extend the implementation to take it into account as the alternative 
for 0x88a8. 

Thanks!

-Original Message-
From: Neil McKee [mailto:neil.mc...@inmon.com] 
Sent: Tuesday, April 18, 2017 7:17 PM
To: Eric Garver ; Rzasik, LukaszX ; 
d...@openvswitch.org; Thomas F Herbert 
Subject: Re: [ovs-dev] [PATCH] ofproto-dpif: Send QinQ related sFlow counters

This patch seems like it may be reporting a VLAN stack that can already be 
decoded from the (ingress-sampled) packet header?

The TODO I left in the code may have been misleading...  this structure is 
intended for the case where the sampled packet-header includes non-standard 
802.1Q headers -- which then have to be stripped out of the sampled header to 
make it decodable,  or where the 802.1Q headers have already been stripped out 
of the packet before you sampled it (e.g. by hardware).

 Here is the wording from the sFlow spec at
http://sflow.org/sflow_version_5.txt:

/* Extended VLAN tunnel information
   Record outer VLAN encapsulations that have
   been stripped. extended_vlantunnel information
   should only be reported if all the following conditions are satisfied:
 1. The packet has nested vlan tags, AND
 2. The reporting device is VLAN aware, AND
 3. One or more VLAN tags have been stripped, either
because they represent proprietary encapsulations, or
because switch hardware automatically strips the outer VLAN
encapsulation.
   Reporting extended_vlantunnel information is not a substitute for
   reporting extended_switch information. extended_switch data must
   always be reported to describe the ingress/egress VLAN information
   for the packet. The extended_vlantunnel information only applies to
   nested VLAN tags, and then only when one or more tags has been
   stripped. */
/* opaque = flow_data; enterprise = 0; format = 1012 */ extended_vlantunnel {
  unsigned int stack<>;  /* List of stripped 802.1Q TPID/TCI layers. Each
TPID,TCI pair is represented as a single 32 bit
integer. Layers listed from outermost to
innermost. */ }

My impression is that non-standard 802.1Q headers are now rare (particularly 
where OVS is concerned).  i.e. everyone uses the
standard ethernet types 0x8100, 0x9100 etc.   So it's unlikely that
the OVS sFlow agent will need to doctor the sampled header to remove 
non-standard 802.1Q headers.

[Quite separately,  if the OVS flow actions included instructions to
*push* vlans onto the VLAN stack before forwarding the packet,  then it would 
make sense to report that in the sFlow export just as we do when packets are 
pushed onto other kinds of tunnel (MPLS, VXLAN etc.).
  Unfortunately there doesn't seem to be an sFlow structure that represents 
this today (e.g. in http://sflow.org/sflow_tunnels.txt)]
--
Neil McKee
InMon Corp.
http://www.inmon.com


On Tue, Apr 18, 2017 at 8:24 AM, Eric Garver  wrote:
> Hi Lukasz,
>
> Thanks for expanding the 802.1ad support.
> I'm not familiar with sFlow, so I'll provide what feedback I can.
>
> On Tue, Apr 18, 

Re: [ovs-dev] [PATCH] checkpatch: fix pointer declaration

2017-04-20 Thread Aaron Conole
Ben Pfaff  writes:

> On Wed, Apr 19, 2017 at 05:42:58PM -0400, Aaron Conole wrote:
>> Lance pointed to a problem where scripts were incorrectly being flagged as
>> pointer spacing warnings.  As an example, the text:
>> 
>>   --u*|-u)
>> 
>> would flag the warning.
>> 
>> Since the type name should always have a space in front of it, change the
>> regex to require some spacing.  Additionally, ** is a common notation in
>> comments to mean 'raise to the power of', so ensure that it is not
>> accidentally flagged as well by adding a not-group populated with *.
>> 
>> Reported-by: Lance Richardson 
>> Signed-off-by: Aaron Conole 
>
> Should checkpatch limit C code warnings to C source files and header
> files?

That's a good suggestion.  I'll cook up a patch to whitelist the files
that are definitely C-code: .c, .h, .c.in, .h.in as suffixes.

You can hold off on applying this if you want to wait for that change as
well - just let me know and I'll submit a series.

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


[ovs-dev] [patch_v0 3/3] OF1.5-EXT-334: Make Check Modifications for OF1.5/EXT-334-OXS Support

2017-04-20 Thread Harivelam Lavanya
[patch_v0 3/3] OF1.5-EXT-334: Make Check Modifications for OF1.5/EXT-334-OXS 
Support

Changes in ofproto.at and ofproto-dpif.at :-  

For 1.5 version "flags"  variable has been removed from flow_stats_reply 
structure.So inorder to avoid make check failures for OF1.5 version add-flow 
with flags,below test cases has been modified for 1.5 version in respective .at 
files

Files  testcase  
ofproto.at   0884
ofproto-dpif1126

Signed-off-by: Harivelam Lavanya
Co-authored-by: Satya Valli 

>From 42f9e61e07ff5fee02831049f48b2a8facb1f369 Mon Sep 17 00:00:00 2001
From: Harivelam Lavanya 
Date: Thu, 20 Apr 2017 16:35:35 +0530
Subject: [PATCH 4/4] Make Check Modifications for OF1.5/EXT-334-OXS Support

---
 tests/ofproto-dpif.at | 1 - 
 tests/ofproto.at  | 4 
 2 files changed, 5 deletions(-)

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 0c2ea384b..3ee98cfbd 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -6944,7 +6944,6 @@ flow_mods_reset_counts () {
 # OpenFlow versions >= 1.3 should behave the same way 
 flow_mods_reset_counts 13
 flow_mods_reset_counts 14
-flow_mods_reset_counts 15
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 5c0d07623..6a222ce68 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1392,10 +1392,6 @@ AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | 
ofctl_strip], [0], [dnl
 OFPST_FLOW reply (OF1.4):
  check_overlap reset_counts in_port=1 actions=drop
 ])
-AT_CHECK([ovs-ofctl -O OpenFlow15 dump-flows br0 | ofctl_strip], [0], [dnl
-OFPST_FLOW reply (OF1.5):
- check_overlap reset_counts in_port=1 actions=drop
-])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-- 
2.11.0


=-=-=
Notice: The information contained in this e-mail
message and/or attachments to it may contain 
confidential or privileged information. If you are 
not the intended recipient, any dissemination, use, 
review, distribution, printing or copying of the 
information contained in this e-mail message 
and/or attachments to it are strictly prohibited. If 
you have received this communication in error, 
please notify us by reply e-mail or telephone and 
immediately and permanently delete the message 
and any attachments. Thank you


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


[ovs-dev] [patch_v0 2/3] OF1.5-EXT-334: Extensible Flow Entry Statistics Implementation --lib/ofp-util.c lib/ox_stat.c

2017-04-20 Thread Harivelam Lavanya
[patch_v0 2/3] OF1.5-EXT-334: Extensible Flow Entry Statistics Implementation 
--lib/ofp-util.c lib/ox_stat.c

Signed-off-by: Harivelam Lavanya
Co-authored-by: Satya Valli 


>From f2e03a1fdcad61fd9221d6f81ff708b017a119f4 Mon Sep 17 00:00:00 2001
From: Harivelam Lavanya 
Date: Wed, 19 Apr 2017 19:44:27 +0530
Subject: [PATCH 3/4] OF1.5/EXT-334-OXS Individal Flow Entry Statistics
 --lib/ofp-util.c lib/ox_stat.c

---
 lib/ofp-util.c | 125 ++-
 lib/ox-stat.c  | 310 +
 2 files changed, 430 insertions(+), 5 deletions(-)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1f038c61e..a261d981a 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -50,9 +50,12 @@
 #include "unaligned.h"
 #include "util.h"
 #include "uuid.h"
+#include "ox-stat.h"

 VLOG_DEFINE_THIS_MODULE(ofp_util);

+extern uint8_t oxs_field_set;
+
 /* Rate limit for OpenFlow message parse errors.  These always indicate a bug
  * in the peer and so there's not much point in showing a lot of them. */
 static struct vlog_rate_limit bad_ofmsg_rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -2313,6 +2316,39 @@ ofputil_decode_ofpst11_flow_request(struct 
ofputil_flow_stats_request *fsr,
 return 0;
 }

+ofputil_decode_ofpst15_flow_request(struct ofputil_flow_stats_request *fsr,
+struct ofpbuf *b, bool aggregate,
+const struct tun_table *tun_table,
+const struct vl_mff_map *vl_mff_map)
+{
+   const struct ofp15_oxs_flow_stats_request *ofsr;
+   enum ofperr error,stat_error;
+   uint16_t statlen;
+
+   ofsr = ofpbuf_pull(b, sizeof *ofsr);
+   fsr->aggregate = aggregate;
+   fsr->table_id = ofsr->table_id;
+
+   error = ofputil_port_from_ofp11(ofsr->out_port, >out_port);
+   if (error) {
+   return error;
+   }
+
+   fsr->out_group = ntohl(ofsr->out_group);
+   fsr->cookie = ofsr->cookie;
+   fsr->cookie_mask = ofsr->cookie_mask;
+
+   error = ofputil_pull_ofp11_match(b, tun_table, vl_mff_map, >match,
+NULL);
+   stat_error = oxs_pull_stat(b, NULL, );
+
+   if (error || stat_error) {
+   return error;
+   }
+
+   return 0;
+}
+
 static enum ofperr
 ofputil_decode_nxst_flow_request(struct ofputil_flow_stats_request *fsr,
  struct ofpbuf *b, bool aggregate,
@@ -2762,6 +2798,10 @@ ofputil_decode_flow_stats_request(struct 
ofputil_flow_stats_request *fsr,
 case OFPRAW_OFPST11_AGGREGATE_REQUEST:
 return ofputil_decode_ofpst11_flow_request(fsr, , true, tun_table,
vl_mff_map);
+case OFPRAW_OFPST15_OXS_FLOW_REQUEST:
+oxs_field_set = 0;
+return ofputil_decode_ofpst15_flow_request(fsr, , false, tun_table,
+   vl_mff_map);

 case OFPRAW_NXST_FLOW_REQUEST:
 return ofputil_decode_nxst_flow_request(fsr, , false, tun_table,
@@ -2791,9 +2831,7 @@ ofputil_encode_flow_stats_request(const struct 
ofputil_flow_stats_request *fsr,
 case OFPUTIL_P_OF11_STD:
 case OFPUTIL_P_OF12_OXM:
 case OFPUTIL_P_OF13_OXM:
-case OFPUTIL_P_OF14_OXM:
-case OFPUTIL_P_OF15_OXM:
-case OFPUTIL_P_OF16_OXM: {
+case OFPUTIL_P_OF14_OXM: {
 struct ofp11_flow_stats_request *ofsr;

 raw = (fsr->aggregate
@@ -2811,6 +2849,25 @@ ofputil_encode_flow_stats_request(const struct 
ofputil_flow_stats_request *fsr,
 break;
 }

+case OFPUTIL_P_OF15_OXM:
+case OFPUTIL_P_OF16_OXM: {
+struct ofp15_oxs_flow_stats_request *ofsr;
+raw = (fsr->aggregate
+   ? OFPRAW_OFPST11_AGGREGATE_REQUEST
+   : OFPRAW_OFPST15_OXS_FLOW_REQUEST);
+msg = ofpraw_alloc(raw, ofputil_protocol_to_ofp_version(protocol),
+   ofputil_match_typical_len(protocol));
+ofsr = ofpbuf_put_zeros(msg, sizeof *ofsr);
+ofsr->table_id = fsr->table_id;
+ofsr->out_port = ofputil_port_to_ofp11(fsr->out_port);
+ofsr->out_group = htonl(fsr->out_group);
+ofsr->cookie = fsr->cookie;
+ofsr->cookie_mask = fsr->cookie_mask;
+ofputil_put_ofp11_match(msg, >match, protocol);
+oxs_put_stat(msg, NULL, ofputil_protocol_to_ofp_version(protocol));
+break;
+}
+
 case OFPUTIL_P_OF10_STD:
 case OFPUTIL_P_OF10_STD_TID: {
 struct ofp10_flow_stats_request *ofsr;
@@ -2893,7 +2950,48 @@ ofputil_decode_flow_stats_reply(struct 
ofputil_flow_stats *fs,

 if (!msg->size) {
 return EOF;
-} else if (raw == OFPRAW_OFPST11_FLOW_REPLY
+} else if (raw == OFPRAW_OFPST15_OXS_FLOW_REPLY) {
+const struct ofp15_oxs_flow_stats_reply *ofs;
+size_t length;
+uint16_t padded_match_len;
+uint16_t stat_len;
+
+ofs = ofpbuf_try_pull(msg, sizeof *ofs);
+

[ovs-dev] [patch_v0 0/3] OF1.5-EXT-334: Extensible Flow Entry Statistics Implementation

2017-04-20 Thread Satyavalli Rama
OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining the 
existing flow entry statistics with OXS Fields. 
This Patch provide implementation for OXS fields encoding in TLV format.

To support this implementation below two messages are newly added 

OFPST_OXS_FLOW_STATS_REQUEST 
OFPTYPE_OXS_FLOW_STATS_REPLY 

As per the openflow specification-1.5, this enhancement should take place on 
the  existing flow entry statistics
with the OXS fields on all the messages that carries flow entry statistics.

The current commit adds support for the new feature in flow statistics 
multipart messages only.Aggegate Multipart and Flow Removal messages are not as 
part of this commit.

Some more fields are added to ovs-ofctl dump-flows command to support 
OpenFlow15 OXS stats.Below are Commands to display OXS stats field wise
ovs-ofctl dump-flows -O OpenFlow15  oxs-duration
ovs-ofctl dump-flows -O OpenFlow15  oxs-idle_time
ovs-ofctl dump-flows -O OpenFlow15  oxs-packet_count 
ovs-ofctl dump-flows -O OpenFlow15  oxs-byte_count


Signed-off-by: Satya Valli 
Co-authored-by: Lavanya Harivelam 

>From bfec9bd0cd687a2a212c6ac971020a89a7e6b5d4 Mon Sep 17 00:00:00 2001
From: SatyaValli 
Date: Wed, 19 Apr 2017 18:54:56 +0530
Subject: [PATCH 1/4] OF1.5/EXT-334 - OXS Individal Flow Entry Statistics

---
 include/openflow/openflow-1.5.h | 48 +
 include/openvswitch/ofp-msgs.h  |  6 ++
 lib/automake.mk |  2 ++
 lib/ofp-parse.c | 45 +-
 lib/ofp-print.c | 10 +
 lib/rconn.c |  2 ++
 ofproto/ofproto.c   |  4 
 7 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 3649e6c29..ff5fa13fa 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -150,4 +150,52 @@ struct ofp15_group_desc_stats {
 };
 OFP_ASSERT(sizeof(struct ofp15_group_desc_stats) == 16);

+struct ofp_oxs_stat {
+ovs_be16 reserved;  /* One of OFPST_* */
+ovs_be16 length;/* Stats Length */
+};
+OFP_ASSERT(sizeof(struct ofp_oxs_stat) == 4);
+
+/*Body for ofp_multipart_request of type
+  OFPMP_FLOW_DESC & OFPMP_FLOW_STATS.*/
+struct ofp15_oxs_flow_stats_request {
+uint8_t table_id; /* ID of table to read
+ (from ofp_table_desc),
+ OFPTT_ALL for all tables. */
+uint8_t pad[3];   /* Align to 32 bits. */
+ovs_be32 out_port;/* Require matching entries to include
+ this as an output port. A value of
+ OFP_ANY indicates no restriction. */
+ovs_be32 out_group;   /* Require matching entries to include
+ this as an output group. A value of
+ OFPG_ANY indicates no restriction. */
+uint8_t pad2[4];  /* Align to 64 bits. */
+ovs_be64 cookie;  /* Require matching entries to contain
+ this cookie value */
+ovs_be64 cookie_mask; /* Mask used to restrict the cookie bits
+ that must match. A value of 0
+ indicates no restriction. */
+};
+OFP_ASSERT(sizeof(struct ofp15_oxs_flow_stats_request) == 32);
+
+/* Body of reply to OFPMP_FLOW_STATS request
+* and body for OFPIT_STAT_TRIGGER generated status. */
+struct ofp15_oxs_flow_stats_reply {
+ovs_be16 length; /* Length of this entry.   */
+uint8_t pad2[2]; /* Align to 64-bits.   */
+uint8_t table_id;/* ID of table flow came from. */
+uint8_t reason;  /* One of OFPFSR_*.*/
+ovs_be16 priority;   /* Priority of the entry.  */
+};
+OFP_ASSERT(sizeof(struct ofp15_oxs_flow_stats_reply) == 8);
+
+/* OXS flow stat field types for OpenFlow basic class. */
+enum oxs_ofb_stat_fields {
+OFPXST_OFB_DURATION  = 0,   /* Time flow entry has been alive.*/
+OFPXST_OFB_IDLE_TIME = 1,   /* Time flow entry has been idle. */
+OFPXST_OFB_FLOW_COUNT= 2,   /* Number of aggregated flow entries. */
+OFPXST_OFB_PACKET_COUNT  = 3,   /* Number of packets in flow entry.   */
+OFPXST_OFB_BYTE_COUNT= 4,   /* Number of bytes in flow entry. */
+};
+
 #endif /* openflow/openflow-1.5.h */
diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index 34708f3bd..057f28656 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -287,6 +287,8 @@ enum ofpraw {
 OFPRAW_OFPST10_FLOW_REQUEST,
 /* OFPST 1.1+ (1): struct ofp11_flow_stats_request, 

[ovs-dev] [patch_v0 1/3] OF1.5-EXT-334: Extensible Flow Entry Statistics Implementation --lib/ox-stat.h lib/ox-stat.c

2017-04-20 Thread Satyavalli Rama
>From 670ca34412b659989e6bd9d1139892dd1717910b Mon Sep 17 00:00:00 2001
From: SatyaValli 
Date: Wed, 19 Apr 2017 19:22:02 +0530
Subject: [PATCH 2/4] OF1.5/EXT-334-OXS Individal Flow Entry Statistics
 --lib/ox_stat.h lib/ox_stat.c

---
 lib/ox-stat.c | 229 ++
 lib/ox-stat.h |  30 
 2 files changed, 259 insertions(+)
 create mode 100644 lib/ox-stat.c
 create mode 100644 lib/ox-stat.h

diff --git a/lib/ox-stat.c b/lib/ox-stat.c
new file mode 100644
index 0..b48ef3aca
--- /dev/null
+++ b/lib/ox-stat.c
@@ -0,0 +1,229 @@
+/*
+ * Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, 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 
+#include "nx-match.h"
+#include "ox-stat.h"
+#include 
+#include "classifier.h"
+#include "colors.h"
+#include "openvswitch/hmap.h"
+#include "openflow/nicira-ext.h"
+#include "openvswitch/dynamic-string.h"
+#include "openvswitch/meta-flow.h"
+#include "openvswitch/ofp-actions.h"
+#include "openvswitch/ofp-errors.h"
+#include "openvswitch/ofp-util.h"
+#include "openvswitch/ofpbuf.h"
+#include "openvswitch/vlog.h"
+#include "packets.h"
+#include "openvswitch/shash.h"
+#include "tun-metadata.h"
+#include "unaligned.h"
+#include "util.h"
+
+VLOG_DEFINE_THIS_MODULE(ox_stat);
+
+/* ## -- ## */
+/* ## OpenFlow Extensible Stats. ## */
+/* ## -- ## */
+
+/* Components of a OXS TLV header. */
+
+static struct ovs_list oxs_ox_map[OFPXST_OFB_BYTE_COUNT + 1];
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+static uint32_t oxs_header_no_len(uint32_t header) {
+return header & 0xff80; }
+
+#define OXS_CLASS(HEADER) ((HEADER) >> 16)
+#define OXS_FIELD(HEADER) (((HEADER) >> 9) & 0x7f)
+#define OXS_TYPE(HEADER) (((HEADER) >> 9) & 0x7f)
+#define OXS_RESERVED(HEADER) (((HEADER) >> 8) & 1)
+#define OXS_LENGTH(HEADER) ((HEADER) & 0xff)
+
+/* Components of a OXS TLV header. */
+#define OXS_HEADER__(CLASS, FIELD, RESERVED, LENGTH) \
+(((CLASS) << 16) | ((FIELD) << 9) | ((RESERVED) << 8) | (LENGTH))
+
+
+#define OXS_HEADER(CLASS, FIELD, LENGTH) \
+OXS_HEADER__(CLASS, FIELD, 0, LENGTH)
+
+
+
+/*  OXS Class IDs.
+ *  The high order bit differentiate reserved classes from member classes.
+ *  Classes 0x to 0x7FFF are member classes, allocated by ONF.
+ *  Classes 0x8000 to 0xFFFE are reserved classes, reserved for
+ *  standardisation.
+ */
+
+enum ofp_oxs_class {
+  OFPXSC_OPENFLOW_BASIC = 0x8002,   /* Basic stats class for OpenFlow */
+  OFPXSC_EXPERIMENTER   = 0x,   /* Experimenter class */
+};
+
+
+#define OFPXST_OFB_ALL ((UINT64_C(1) << 6) - 1)
+#define OXS_OX_COOKIEOXS_HEADER  (0x8002, 5 , 8)
+
+struct oxs_field {
+uint32_t header;
+enum ofp_version version;
+const char *name;
+enum oxs_ofb_stat_fields id;
+};
+
+struct oxs_field_index {
+struct hmap_node header_node;
+struct hmap_node name_node;
+struct ovs_list ox_node;
+const struct oxs_field fs;
+};
+
+#define OXS_STATS_DURATION_LEN 8
+#define OXS_STATS_IDLE_TIME_LEN8
+#define OXS_STATS_FLOW_COUNT_LEN   4
+#define OXS_STATS_PACKET_COUNT_LEN 8
+#define OXS_STATS_BYTE_COUNT_LEN   8
+
+#define OXS_OF_DURATION OXS_HEADER (0x8002, OFPXST_OFB_DURATION, \
+OXS_STATS_DURATION_LEN)
+#define OXS_OF_IDLE_TIMEOXS_HEADER (0x8002, OFPXST_OFB_IDLE_TIME, \
+OXS_STATS_IDLE_TIME_LEN)
+#define OXS_OF_FLOW_COUNT   OXS_HEADER (0x8002, OFPXST_OFB_FLOW_COUNT, \
+OXS_STATS_FLOW_COUNT_LEN)
+#define OXS_OF_PACKET_COUNT OXS_HEADER (0x8002, OFPXST_OFB_PACKET_COUNT, \
+OXS_STATS_PACKET_COUNT_LEN)
+#define OXS_OF_BYTE_COUNT   OXS_HEADER (0x8002, OFPXST_OFB_BYTE_COUNT, \
+OXS_STATS_BYTE_COUNT_LEN)
+
+static struct oxs_field_index all_oxs_fields[] = {
+{.fs = { OXS_OF_DURATION, OFP15_VERSION, "OFPXST_OFB_DURATION",
+ OFPXST_OFB_DURATION } },
+{.fs = { OXS_OF_IDLE_TIME, OFP15_VERSION, "OFPXST_OFB_IDLE_TIME",
+ OFPXST_OFB_IDLE_TIME } },
+{.fs = { OXS_OF_FLOW_COUNT, OFP15_VERSION, "OFPXST_OFB_FLOW_COUNT",
+ OFPXST_OFB_FLOW_COUNT } },
+{.fs = { OXS_OF_PACKET_COUNT, OFP15_VERSION, "OFPXST_OFB_PACKET_COUNT",
+ OFPXST_OFB_PACKET_COUNT } },
+{.fs = { OXS_OF_BYTE_COUNT, 

Re: [ovs-dev] datapath-windows: Pass fwdCtx to conntrack

2017-04-20 Thread Alin Serdean
Acked-by: Alin Gabriel Serdean 


> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Yin Lin
> Sent: Wednesday, April 19, 2017 11:19 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] datapath-windows: Pass fwdCtx to conntrack
> 
> There dependencies in Contrack module such as NAT and fragmentation on
> OvsForwardingContext. This patch will make OvsForwardingContext public in
> order to implement these functionalities.
> ---
>  datapath-windows/ovsext/Actions.c   | 61 
> ++---
>  datapath-windows/ovsext/Actions.h   | 58
> +++
>  datapath-windows/ovsext/Conntrack.c |  5 +--  datapath-
> windows/ovsext/Conntrack.h |  4 +--
>  4 files changed, 65 insertions(+), 63 deletions(-)
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH branch-2.6] xenserver: Add install files to RPM file list

2017-04-20 Thread Greg Rose
On Wed, 2017-04-19 at 20:34 -0700, Ben Pfaff wrote:
> On Wed, Apr 19, 2017 at 10:25:11AM -0700, Greg Rose wrote:
> > On Mon, 2017-04-17 at 14:08 -0700, Greg Rose wrote:
> > > Add missing install files to RPM file list to prevent file check error
> > > so that RPM files will be built.
> > > 
> > > Signed-off-by: Greg Rose 
> > 
> > I have not seen this patch committed to the 2.6 branch.  Nor did I see
> > commit 6d02480dc2 cherry-picked to the 2.7 branch.
> 
> I've taken care of it now.  Thanks again!
> 
> (We're often behind on these things, but they rarely get lost because of
> the tracker at https://patchwork.ozlabs.org/project/openvswitch/list/.)

OK, thanks Ben.  That's good to know.

- Greg

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


Re: [ovs-dev] [PATCH 5/5] ovn-trace: Implement ct_next and ct_clear actions.

2017-04-20 Thread Miguel Angel Ajo Pelayo
Just a very nit opinion/comment. nice work.

On Tue, Apr 18, 2017 at 9:47 PM, Ben Pfaff  wrote:

> Signed-off-by: Ben Pfaff 
> ---
>  NEWS  |  1 +
>  ovn/utilities/ovn-trace.8.xml | 58 +
>  ovn/utilities/ovn-trace.c | 99 ++
> -
>  3 files changed, 156 insertions(+), 2 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index eb825ac72161..f0ada38b4019 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -19,6 +19,7 @@ Post-v2.7.0
>   * Gratuitous ARP for NAT addresses on a distributed logical router.
>   * Allow ovn-controller SSL configuration to be obtained from vswitchd
> database.
> + * ovn-trace now has basic support for tracing distributed firewalls.
> - Add the command 'ovs-appctl stp/show' (see ovs-vswitchd(8)).
> - OpenFlow:
>   * Increased support for OpenFlow 1.6 (draft).
> diff --git a/ovn/utilities/ovn-trace.8.xml b/ovn/utilities/ovn-trace.8.xml
> index 78914240d88c..8bb329bfbd71 100644
> --- a/ovn/utilities/ovn-trace.8.xml
> +++ b/ovn/utilities/ovn-trace.8.xml
> @@ -307,6 +307,64 @@
>  
>
>  
> +
> +--ct=flags
> +
> +  
> +This option sets the ct_state flags that a
> +ct_next logical action will report.  The
> flags
> +must be a comma- or space-separated list of the following
> connection
> +tracking flags:
> +  
> +
> +  
> +
> +  trk: Include to indicate connection tracking has
> taken
> +  place.  (This bit is set automatically even if not listed in
> +  flags.
> +
> +new: Include to indicate a new flow.
> +est: Include to indicate an established
> flow.
> +rel: Include to indicate a related flow.
> +rpl: Include to indicate a reply flow.
> +inv: Include to indicate a connection entry in a
> +bad state.
> +dnat: Include to indicate a packet whose
> +destination IP address has been changed.
> +snat: Include to indicate a packet whose source
> IP
> +address has been changed.
> +  
> +
> +  
> +The ct_next action is used to implement the OVN
> +distributed firewall.  For testing, useful flag combinations
> include:
> +  
> +
> +  
> +trk,new: A packet in a flow in either direction
> +through a firewall that has not yet been committed (with
> +ct_commit).
> +trk,est: A packet in an established flow going
> out
> +through a firewall.
> +trk,rpl: A packet coming in through a firewall in
> +reply to an established flow.
> +trk,inv: An invalid packet in either
> direction.
> +  
> +
> +  
> +A packet might pass through the connection tracker twice in one
> trip
> +through OVN: once following egress from a VM as it passes outward
> +through a firewall, and once preceding ingress to a second VM as
> it
> +passes inward through a firewall.  Use multiple --ct
> +options to specify the flags for multiple ct_next
> actions.
> +  
> +
> +  
> +When --ct is unspecified, or when there are fewer
> +--ct options than ct_next actions, the
> +flags default to trk,est.
> +  
> +
>
>
>Daemon Options
> diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
> index 66844b11ac1d..e9463f02a70f 100644
> --- a/ovn/utilities/ovn-trace.c
> +++ b/ovn/utilities/ovn-trace.c
> @@ -69,6 +69,11 @@ static bool minimal;
>  static const char *ovs;
>  static struct vconn *vconn;
>
> +/* --ct: Connection tracking state to use for ct_next() actions. */
> +static uint32_t *ct_states;
> +static size_t n_ct_states;
> +static size_t ct_state_idx;
> +
>  OVS_NO_RETURN static void usage(void);
>  static void parse_options(int argc, char *argv[]);
>  static char *trace(const char *datapath, const char *flow);
> @@ -156,6 +161,42 @@ default_ovs(void)
>  }
>
>  static void
> +parse_ct_option(const char *state_s_)
> +{
> +uint32_t state = CS_TRACKED;
> +
> +char *state_s = xstrdup(state_s_);
> +char *save_ptr = NULL;
> +for (char *cs = strtok_r(state_s, ", ", _ptr); cs;
>

why do we enforce the use of spaces? ", ", would it
make sense to accept just "," as delimiter and then
strip/ignore spaces for usability reasons?


> + cs = strtok_r(NULL, ", ", _ptr)) {
> +uint32_t bit = ct_state_from_string(cs);
> +if (!bit) {
> +ovs_fatal(0, "%s: unknown connection tracking state flag",
> cs);
> +}
> +state |= bit;
> +}
> +free(state_s);
> +
> +/* Check constraints listed in ovs-fields(7). */
> +if (state & CS_INVALID && state & ~(CS_TRACKED | CS_INVALID)) {
> +VLOG_WARN("%s: invalid connection state: "
> +  "when \"inv\" is set, only \"trk\" may also be set",
> +  state_s_);
> +}
> +if (state & CS_NEW && 

Re: [ovs-dev] [PATCH 1/6] rstp/stp: Unref the rstp/stp when bridges destroyed.

2017-04-20 Thread nickcooper-zhangtonghao
who can help me to review patches ? Thanks very much.

stp:
http://patchwork.ozlabs.org/patch/745880/ 

http://patchwork.ozlabs.org/patch/745881/ 

http://patchwork.ozlabs.org/patch/745884/ 

http://patchwork.ozlabs.org/patch/745882/ 

http://patchwork.ozlabs.org/patch/745883/ 

http://patchwork.ozlabs.org/patch/745885/ 


ofproto:
http://patchwork.ozlabs.org/patch/743155/ 



Thanks.
Nick

> On Apr 1, 2017, at 11:11 AM, nickcooper-zhangtonghao  
> wrote:
> 
> When bridges destroyed, which stp enabled, you can
> still get stp info via the command 'ovs-appctl stp/show'.
> And the rstp is also in the same case. We should unref
> them. The rstp/stp ports have been unregistered via
> 'ofproto_port_unregister' function when ports destroyed.
> We will unref rstp/stp struct in the 'destruct' of
> ofproto-dpif provider.
> 
> Signed-off-by: nickcooper-zhangtonghao  >
> ---
> ofproto/ofproto-dpif.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 523adad..4beacda 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1494,6 +1494,8 @@ destruct(struct ofproto *ofproto_)
> hmap_destroy(>bundles);
> mac_learning_unref(ofproto->ml);
> mcast_snooping_unref(ofproto->ms);
> +stp_unref(ofproto->stp);
> +rstp_unref(ofproto->rstp);
> 
> sset_destroy(>ports);
> sset_destroy(>ghost_ports);
> -- 
> 1.8.3.1

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


Re: [ovs-dev] [PATCH 4/5] flow: New function flow_clear_conntrack().

2017-04-20 Thread Miguel Angel Ajo Pelayo
Acked-By: Miguel Angel Ajo 

On Tue, Apr 18, 2017 at 9:47 PM, Ben Pfaff  wrote:

> This will have a new user in an upcoming commit.
>
> Signed-off-by: Ben Pfaff 
> ---
>  lib/flow.c   | 21 +
>  lib/flow.h   |  1 +
>  ofproto/ofproto-dpif-xlate.c | 18 +-
>  3 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/lib/flow.c b/lib/flow.c
> index 7552cd72a173..2ba51214ef80 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -1024,6 +1024,27 @@ ct_state_from_string(const char *s)
>  return 0;
>  }
>
> +/* Clears the fields in 'flow' associated with connection tracking. */
> +void
> +flow_clear_conntrack(struct flow *flow)
> +{
> +flow->ct_state = 0;
> +flow->ct_zone = 0;
> +flow->ct_mark = 0;
> +flow->ct_label = OVS_U128_ZERO;
> +
> +flow->ct_nw_proto = 0;
> +flow->ct_tp_src = 0;
> +flow->ct_tp_dst = 0;
> +if (flow->dl_type == htons(ETH_TYPE_IP)) {
> +flow->ct_nw_src = 0;
> +flow->ct_nw_dst = 0;
> +} else if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
> +memset(>ct_ipv6_src, 0, sizeof flow->ct_ipv6_src);
> +memset(>ct_ipv6_dst, 0, sizeof flow->ct_ipv6_dst);
> +}
> +}
> +
>  char *
>  flow_to_string(const struct flow *flow)
>  {
> diff --git a/lib/flow.h b/lib/flow.h
> index 9d4ae49ccfbd..2957108a68db 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -74,6 +74,7 @@ void flow_get_metadata(const struct flow *, struct match
> *flow_metadata);
>
>  const char *ct_state_to_string(uint32_t state);
>  uint32_t ct_state_from_string(const char *);
> +void flow_clear_conntrack(struct flow *);
>
>  char *flow_to_string(const struct flow *);
>  void format_flags(struct ds *ds, const char *(*bit_to_string)(uint32_t),
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index a24aef9a43a1..76b8a3aa729a 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3222,23 +3222,7 @@ static void
>  clear_conntrack(struct xlate_ctx *ctx)
>  {
>  ctx->conntracked = false;
> -
> -struct flow *flow = >xin->flow;
> -flow->ct_state = 0;
> -flow->ct_zone = 0;
> -flow->ct_mark = 0;
> -flow->ct_label = OVS_U128_ZERO;
> -
> -flow->ct_nw_proto = 0;
> -flow->ct_tp_src = 0;
> -flow->ct_tp_dst = 0;
> -if (flow->dl_type == htons(ETH_TYPE_IP)) {
> -flow->ct_nw_src = 0;
> -flow->ct_nw_dst = 0;
> -} if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
> -memset(>ct_ipv6_src, 0, sizeof flow->ct_ipv6_src);
> -memset(>ct_ipv6_dst, 0, sizeof flow->ct_ipv6_dst);
> -}
> +flow_clear_conntrack(>xin->flow);
>  }
>
>  static bool
> --
> 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] ofproto: Report only un-deleted groups in group stats replies.

2017-04-20 Thread Timothy M. Redaelli
On 04/19/2017 08:29 PM, Ben Pfaff wrote:
> Deleted groups hang around in the group table until the next grace period,
> so it's important for the group stats code to pretend that they're gone
> until they really get deleted.

[...]
Thank you for the patch, I tested it in all the scenarios and it works.

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


Re: [ovs-dev] [PATCH ovs V7 11/24] dpif-netlink: Use netdev flow put api to insert a flow

2017-04-20 Thread Roi Dayan



On 14/04/2017 04:08, Joe Stringer wrote:

On 7 April 2017 at 06:12, Roi Dayan  wrote:

From: Paul Blakey 

Using the new netdev flow api operate will now try and
offload flows to the relevant netdev of the input port.
Other operate methods flows will come in later patches.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
Reviewed-by: Simon Horman 
---





 static void
-dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
+dbg_print_flow(const struct nlattr *key, size_t key_len,
+   const struct nlattr *mask, size_t mask_len,
+   const struct nlattr *actions, size_t actions_len,
+   const ovs_u128 *ufid,
+   const char *op)


I wonder if we could refactor log_flow_message() into somewhere
neutral and share the output format for these flows from here and
there.


we'll check it. can also use maybe the other logging functions as well 
(put/del/get).
Need to modify to use the actual current module for logging instead of 
the static this_module else everything will be logged as the dpif module.






+{
+struct ds s;
+
+ds_init();
+ds_put_cstr(, op);
+ds_put_cstr(, " (");
+odp_format_ufid(ufid, );
+ds_put_cstr(, ")");
+if (key_len) {
+ds_put_cstr(, "\nflow (verbose): ");
+odp_flow_format(key, key_len, mask, mask_len, NULL, , true);
+ds_put_cstr(, "\nflow: ");
+odp_flow_format(key, key_len, mask, mask_len, NULL, , false);
+}
+ds_put_cstr(, "\nactions: ");
+format_odp_actions(, actions, actions_len);
+VLOG_DBG("\n%s", ds_cstr());
+ds_destroy();
+}
+
+static int
+try_send_to_netdev(struct dpif_netlink *dpif, struct dpif_op *op)
 {
-struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
+switch (op->type) {
+case DPIF_OP_FLOW_PUT: {
+struct dpif_flow_put *put = >u.flow_put;

+if (!put->ufid) {
+break;
+}
+dbg_print_flow(put->key, put->key_len, put->mask, put->mask_len,
+   put->actions, put->actions_len, put->ufid,
+   (put->flags & DPIF_FP_MODIFY ? "PUT(MODIFY)" : "PUT"));
+return parse_flow_put(dpif, put);
+}
+case DPIF_OP_FLOW_DEL:
+case DPIF_OP_FLOW_GET:
+case DPIF_OP_EXECUTE:
+default:
+break;
+}
+return EOPNOTSUPP;
+}
+
+static void
+dpif_netlink_operate_chunks(struct dpif_netlink *dpif, struct dpif_op **ops,
+size_t n_ops)
+{
 while (n_ops > 0) {
 size_t chunk = dpif_netlink_operate__(dpif, ops, n_ops);
+
 ops += chunk;
 n_ops -= chunk;
 }
 }

+static void
+dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
+{
+struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
+struct dpif_op *new_ops[OPERATE_MAX_OPS];
+int count = 0;
+int i = 0;
+int err = 0;
+
+if (netdev_flow_api_enabled) {
+while (n_ops > 0) {
+count = 0;
+
+while (n_ops > 0 && count < OPERATE_MAX_OPS) {
+struct dpif_op *op = ops[i++];
+
+err = try_send_to_netdev(dpif, op);
+if (err && err != EEXIST) {
+new_ops[count++] = op;
+} else {
+op->error = err;
+}
+
+n_ops--;
+}
+
+dpif_netlink_operate_chunks(dpif, new_ops, count);
+}
+
+return;
+}
+
+dpif_netlink_operate_chunks(dpif, ops, n_ops);
+}
+
 #if _WIN32
 static void
 dpif_netlink_handler_uninit(struct dpif_handler *handler)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 8747778..349425e 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -41,6 +41,7 @@
 #include "util.h"
 #include "uuid.h"
 #include "openvswitch/vlog.h"
+#include "openvswitch/match.h"

 VLOG_DEFINE_THIS_MODULE(odp_util);

@@ -5497,6 +5498,58 @@ odp_flow_key_to_mask(const struct nlattr *mask_key, 
size_t mask_key_len,
 }
 }

+/* Converts the netlink formated key/mask to match.
+ * Fails if odp_flow_key_from_key/mask and odp_flow_key_key/mask
+ * disagree on the acceptable form of flow */
+int
+parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len,
+const struct nlattr *mask, size_t mask_len,
+struct match *match)
+{
+enum odp_key_fitness fitness;
+
+fitness = odp_flow_key_to_flow(key, key_len, >flow);
+if (fitness) {
+/* This should not happen: it indicates that odp_flow_key_from_flow()
+ * and odp_flow_key_to_flow() disagree on the acceptable form of a
+ * flow.  Log the problem as an error, with enough details to enable
+ * debugging. */
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
+if 

[ovs-dev] Bug#860786: README.Debian: include IPv6 in examples for /etc/network/interfaces

2017-04-20 Thread Daniel Pocock
Package: openvswitch-switch
Version: 2.3.0+git20140819-3+deb8u1
Severity: wishlist


Looking at the examples for /etc/network/interfaces, none of them
include IPv6.

I've tried configuring it using the example below and it appears to be
working fine in a dual stack DHCP environment.

In another environment, I used an "up ip addr add [ipv6 addr] ..."
command in the inet stanza

Please confirm if the inet6 stanza is fully supported and if this is the
right way to use it and consider adding it to README.Debian:




# This file describes the network interfaces available on your system
# and how to activate them. For more information, see interfaces(5).

# The loopback network interface
auto lo
iface lo inet loopback

auto ovsbr0
iface ovsbr0 inet dhcp
ovs_type OVSBridge
ovs_ports eth0

iface ovsbr0 inet6 dhcp
ovs_type OVSBridge
ovs_ports eth0

allow-ovsbr0 eth0
iface eth0 inet manual
ovs_bridge ovsbr0
ovs_type OVSPort
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev