[ovs-dev] Panready Dorade Denton

2017-01-12 Thread Bonesca - Jona 
    [ View in browser ]( http://r.newsletter.bonescamail.nl/nru6qw66oatrf.html 
)    
 
 
 
NEW PRODUCTION PANREADY DORADE DENTON
 
ORIGIN: MAROCCO
PACKING: 5 KILO
SPECIFICATION: GUTTED AND SCALED
SIZE: +/- 4 PIECES PER KILO
PRICES:
1 BOX € 2,89
10 BOX € 2,69
PALET(145 BOXES) €2,59 = € 12,95 PER BOX!! 



 
This email was sent to d...@openvswitch.org
You received this email because you are registered with Bonesca Import en 
Export BV
 
[ Unsubscribe here ]( http://r.newsletter.bonescamail.nl/nru6qw66oatrg.html )   
 

Sent by
[  ]( http://r.newsletter.bonescamail.nl/2n3cqzkodaoatrd.html )      
© 2017 Bonesca Import en Export BV   

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

[ovs-dev] [PATCH net] openvswitch: maintain correct checksum state in conntrack actions

2017-01-12 Thread Lance Richardson 
When executing conntrack actions on skbuffs with checksum mode
CHECKSUM_COMPLETE, the checksum must be updated to account for
header pushes and pulls. Otherwise we get "hw csum failure"
logs similar to this (ICMP packet received on geneve tunnel
via ixgbe NIC):

[  405.740065] genev_sys_6081: hw csum failure
[  405.740106] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G  I 
4.10.0-rc3+ #1
[  405.740108] Call Trace:
[  405.740110]  
[  405.740113]  dump_stack+0x63/0x87
[  405.740116]  netdev_rx_csum_fault+0x3a/0x40
[  405.740118]  __skb_checksum_complete+0xcf/0xe0
[  405.740120]  nf_ip_checksum+0xc8/0xf0
[  405.740124]  icmp_error+0x1de/0x351 [nf_conntrack_ipv4]
[  405.740132]  nf_conntrack_in+0xe1/0x550 [nf_conntrack]
[  405.740137]  ? find_bucket.isra.2+0x62/0x70 [openvswitch]
[  405.740143]  __ovs_ct_lookup+0x95/0x980 [openvswitch]
[  405.740145]  ? netif_rx_internal+0x44/0x110
[  405.740149]  ovs_ct_execute+0x147/0x4b0 [openvswitch]
[  405.740153]  do_execute_actions+0x22e/0xa70 [openvswitch]
[  405.740157]  ovs_execute_actions+0x40/0x120 [openvswitch]
[  405.740161]  ovs_dp_process_packet+0x84/0x120 [openvswitch]
[  405.740166]  ovs_vport_receive+0x73/0xd0 [openvswitch]
[  405.740168]  ? udp_rcv+0x1a/0x20
[  405.740170]  ? ip_local_deliver_finish+0x93/0x1e0
[  405.740172]  ? ip_local_deliver+0x6f/0xe0
[  405.740174]  ? ip_rcv_finish+0x3a0/0x3a0
[  405.740176]  ? ip_rcv_finish+0xdb/0x3a0
[  405.740177]  ? ip_rcv+0x2a7/0x400
[  405.740180]  ? __netif_receive_skb_core+0x970/0xa00
[  405.740185]  netdev_frame_hook+0xd3/0x160 [openvswitch]
[  405.740187]  __netif_receive_skb_core+0x1dc/0xa00
[  405.740194]  ? ixgbe_clean_rx_irq+0x46d/0xa20 [ixgbe]
[  405.740197]  __netif_receive_skb+0x18/0x60
[  405.740199]  netif_receive_skb_internal+0x40/0xb0
[  405.740201]  napi_gro_receive+0xcd/0x120
[  405.740204]  gro_cell_poll+0x57/0x80 [geneve]
[  405.740206]  net_rx_action+0x260/0x3c0
[  405.740209]  __do_softirq+0xc9/0x28c
[  405.740211]  irq_exit+0xd9/0xf0
[  405.740213]  do_IRQ+0x51/0xd0
[  405.740215]  common_interrupt+0x93/0x93

Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
Signed-off-by: Lance Richardson 
---
 net/openvswitch/conntrack.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 6b78bab..54253ea 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -514,7 +514,7 @@ static int ovs_ct_nat_execute(struct sk_buff *skb, struct 
nf_conn *ct,
int hooknum, nh_off, err = NF_ACCEPT;
 
nh_off = skb_network_offset(skb);
-   skb_pull(skb, nh_off);
+   skb_pull_rcsum(skb, nh_off);
 
/* See HOOK2MANIP(). */
if (maniptype == NF_NAT_MANIP_SRC)
@@ -579,6 +579,7 @@ static int ovs_ct_nat_execute(struct sk_buff *skb, struct 
nf_conn *ct,
err = nf_nat_packet(ct, ctinfo, hooknum, skb);
 push:
skb_push(skb, nh_off);
+   skb_postpush_rcsum(skb, skb->data, nh_off);
 
return err;
 }
@@ -886,7 +887,7 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
 
/* The conntrack module expects to be working at L3. */
nh_ofs = skb_network_offset(skb);
-   skb_pull(skb, nh_ofs);
+   skb_pull_rcsum(skb, nh_ofs);
 
if (key->ip.frag != OVS_FRAG_TYPE_NONE) {
err = handle_fragments(net, key, info->zone.id, skb);
@@ -900,6 +901,7 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
err = ovs_ct_lookup(net, key, info, skb);
 
skb_push(skb, nh_ofs);
+   skb_postpush_rcsum(skb, skb->data, nh_ofs);
if (err)
kfree_skb(skb);
return err;
-- 
1.8.3.1

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

Re: [ovs-dev] [PATCH] travis: Update build list email address.

2017-01-12 Thread Ben Pfaff 
On Thu, Jan 12, 2017 at 12:25:27PM -0800, Andy Zhou wrote:
> On Thu, Jan 12, 2017 at 9:16 AM, Ben Pfaff  wrote:
> 
> > The lists these days prefer an ovs- prefix.  Currently all of the build
> > emails are being dropped because it is missing.
> >
> > Signed-off-by: Ben Pfaff 
> >
> 
>  Acked-by: Andy Zhou 

Thanks, applied to master and backported as far as branch-2.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] ofproto-dpif: Make ofproto/trace output easier to read.

2017-01-12 Thread Justin Pettit 

> On Jan 12, 2017, at 9:23 AM, Ben Pfaff  wrote:
> 
> On Mon, Jan 09, 2017 at 08:46:28PM -0800, Justin Pettit wrote:
>> I think it would be helpful to have a comment describing this
>> function.  Also mentioning that the caller maintains ownership of
>> 'text'.
> 
> There was a lot of missing comments and documentation.  I added a bunch.
> 
>> I may be missing something, but is there anything that frees these
>> "oftrace_node"s either here or ofproto-dpif-xlate.c?
> 
> No.  Oops.  I fixed this.

You'd indicated off-list that ovn-trace probably has similar issues.  These 
weren't originally a problem, but it could be now that it can be run in daemon 
mode.  Are you planning to address that?

--Justin


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

[ovs-dev] [PATCH v2 5/5] datapath-windows: Fragment NBL based on MRU size

2017-01-12 Thread Anand Kumar 
MRU value is updated only for the Ipv4 fragments. If it is non zero,
then fragment the NBL based on MRU value and send out the new NBL to
the vnic.
---
 datapath-windows/ovsext/Actions.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 9cc79dc..3156fb6 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -34,6 +34,7 @@
 #include "Vport.h"
 #include "Vxlan.h"
 #include "Geneve.h"
+#include "IpFragment.h"
 
 #ifdef OVS_DBG_MOD
 #undef OVS_DBG_MOD
@@ -873,6 +874,7 @@ OvsOutputForwardingCtx(OvsForwardingContext *ovsFwdCtx)
 NDIS_STATUS status = STATUS_SUCCESS;
 POVS_SWITCH_CONTEXT switchContext = ovsFwdCtx->switchContext;
 PCWSTR dropReason;
+PNET_BUFFER_LIST fragNbl = NULL;
 
 /*
  * Handle the case where the some of the destination ports are tunneled
@@ -918,6 +920,30 @@ OvsOutputForwardingCtx(OvsForwardingContext *ovsFwdCtx)
 goto dropit;
 }
 
+if (ovsFwdCtx->mru != 0) {
+/* fragment nbl based on mru */
+fragNbl = 
OvsFragmentNBL(ovsFwdCtx->switchContext,ovsFwdCtx->curNbl, &(ovsFwdCtx->layers),
+ ovsFwdCtx->mru, 0, TRUE);
+if (fragNbl != NULL) {
+OvsCompleteNBLForwardingCtx(ovsFwdCtx,
+L"Dropped, failed to create 
fragments");
+ovsFwdCtx->sendFlags |= 
NDIS_SEND_FLAGS_SWITCH_DESTINATION_GROUP;
+status = OvsInitForwardingCtx(ovsFwdCtx,
+  ovsFwdCtx->switchContext,
+  fragNbl,
+  ovsFwdCtx->srcVportNo,
+  ovsFwdCtx->sendFlags,
+  
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(fragNbl),
+  ovsFwdCtx->mru,
+  ovsFwdCtx->completionList,
+  >layers, FALSE);
+if (status != NDIS_STATUS_SUCCESS) {
+dropReason = L"Dropped due to resouces";
+goto dropit;
+}
+}
+}
+
 OvsSendNBLIngress(ovsFwdCtx->switchContext, ovsFwdCtx->curNbl,
   ovsFwdCtx->sendFlags);
 /* End this pipeline by resetting the corresponding context. */
-- 
2.9.3.windows.1

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

[ovs-dev] [PATCH v2 1/5] datapath-windows: Added a new file to support Ipv4 fragments.

2017-01-12 Thread Anand Kumar 
This patch adds functionalities to handle IPv4 fragments, which will be
used by Conntrack module.

Added a new structure to hold the Ipv4 fragments and a hash table to
hold Ipv4 datagram entries. Also added a clean up thread that runs
every minute to delete the expired IPv4 datagram entries.

The individual fragments are ignored by the conntrack. Once all the
fragments are recieved, a new NBL is created out of the reassembled
fragments and conntrack executes actions on the new NBL.

Created new APIs OvsProcessIpv4Fragment() to process individual fragments,
OvsIpv4Reassemble() to reassemble Ipv4 fragments.
---
 datapath-windows/automake.mk   |   2 +
 datapath-windows/ovsext/Debug.h|   3 +-
 datapath-windows/ovsext/IpFragment.c   | 506 +
 datapath-windows/ovsext/IpFragment.h   |  74 +
 datapath-windows/ovsext/Switch.c   |   9 +
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 6 files changed, 595 insertions(+), 1 deletion(-)
 create mode 100644 datapath-windows/ovsext/IpFragment.c
 create mode 100644 datapath-windows/ovsext/IpFragment.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index 53983ae..4f7b55a 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -32,6 +32,8 @@ EXTRA_DIST += \
datapath-windows/ovsext/Flow.h \
datapath-windows/ovsext/Gre.h \
datapath-windows/ovsext/Gre.c \
+   datapath-windows/ovsext/IpFragment.c \
+   datapath-windows/ovsext/IpFragment.h \
datapath-windows/ovsext/IpHelper.c \
datapath-windows/ovsext/IpHelper.h \
datapath-windows/ovsext/Jhash.c \
diff --git a/datapath-windows/ovsext/Debug.h b/datapath-windows/ovsext/Debug.h
index cae6ac9..6de1812 100644
--- a/datapath-windows/ovsext/Debug.h
+++ b/datapath-windows/ovsext/Debug.h
@@ -42,8 +42,9 @@
 #define OVS_DBG_STT  BIT32(22)
 #define OVS_DBG_CONTRK   BIT32(23)
 #define OVS_DBG_GENEVE   BIT32(24)
+#define OVS_DBG_IPFRAG   BIT32(25)
 
-#define OVS_DBG_LAST 24  /* Set this to the last defined module number. */
+#define OVS_DBG_LAST 25  /* Set this to the last defined module number. */
 /* Please add above OVS_DBG_LAST. */
 
 #define OVS_DBG_ERRORDPFLTR_ERROR_LEVEL
diff --git a/datapath-windows/ovsext/IpFragment.c 
b/datapath-windows/ovsext/IpFragment.c
new file mode 100644
index 000..2ce3932
--- /dev/null
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -0,0 +1,506 @@
+/*
+ * Copyright (c) 2017 VMware, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "Conntrack.h"
+#include "Debug.h"
+#include "IpFragment.h"
+#include "Jhash.h"
+#include "Offload.h"
+#include "PacketParser.h"
+
+#ifdef OVS_DBG_MOD
+#undef OVS_DBG_MOD
+#endif
+#define OVS_DBG_MOD OVS_DBG_IPFRAG
+
+/* Function declarations */
+static VOID OvsIpFragmentEntryCleaner(PVOID data);
+static VOID OvsIpFragmentEntryDelete(POVS_IPFRAG_ENTRY entry);
+
+/* Global and static variables */
+static OVS_IPFRAG_THREAD_CTX ipFragThreadCtx;
+static PNDIS_RW_LOCK_EX ovsIpFragmentHashLockObj;
+static UINT64 ipTotalEntries;
+static PLIST_ENTRY OvsIpFragTable;
+
+NDIS_STATUS
+OvsInitIpFragment(POVS_SWITCH_CONTEXT context)
+{
+
+NDIS_STATUS status;
+HANDLE threadHandle = NULL;
+
+/* Init the sync-lock */
+ovsIpFragmentHashLockObj = NdisAllocateRWLock(context->NdisFilterHandle);
+if (ovsIpFragmentHashLockObj == NULL) {
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+/* Init the Hash Buffer */
+OvsIpFragTable = OvsAllocateMemoryWithTag(sizeof(LIST_ENTRY)
+  * IP_FRAG_HASH_TABLE_SIZE,
+  OVS_MEMORY_TAG);
+if (OvsIpFragTable == NULL) {
+NdisFreeRWLock(ovsIpFragmentHashLockObj);
+ovsIpFragmentHashLockObj = NULL;
+return STATUS_INSUFFICIENT_RESOURCES;
+}
+
+for (int i = 0; i < IP_FRAG_HASH_TABLE_SIZE; i++) {
+InitializeListHead([i]);
+}
+
+/* Init Cleaner Thread */
+KeInitializeEvent(, NotificationEvent, FALSE);
+status = PsCreateSystemThread(, SYNCHRONIZE, NULL, NULL,
+  NULL, OvsIpFragmentEntryCleaner,
+  );
+
+if (status != STATUS_SUCCESS) {
+OvsFreeMemoryWithTag(OvsIpFragTable, OVS_MEMORY_TAG);
+OvsIpFragTable = NULL;
+NdisFreeRWLock(ovsIpFragmentHashLockObj);
+ovsIpFragmentHashLockObj = NULL;
+

[ovs-dev] [PATCH v2 4/5] datapath-windows: Updated OvsTcpSegmentNBL to handle IP fragments.

2017-01-12 Thread Anand Kumar 
With this patch, OvsTcpSegmentNBL not only supports tcp segments but also the
IP fragments.

To reflect the new changes, renamed function name from OvsTcpSegmentNBL
to OvsFragmentNBL and created a wrapper for OvsTcpSegmentNBL.
---
 datapath-windows/ovsext/BufferMgmt.c | 189 +--
 datapath-windows/ovsext/BufferMgmt.h |  10 +-
 datapath-windows/ovsext/Geneve.c |   2 +-
 datapath-windows/ovsext/Gre.c|   2 +-
 datapath-windows/ovsext/Stt.c|   2 +-
 datapath-windows/ovsext/User.c   |   2 +-
 datapath-windows/ovsext/Vxlan.c  |   2 +-
 7 files changed, 150 insertions(+), 59 deletions(-)

diff --git a/datapath-windows/ovsext/BufferMgmt.c 
b/datapath-windows/ovsext/BufferMgmt.c
index 6027c35..3f7c0cd 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -1083,6 +1083,31 @@ nblcopy_error:
 return NULL;
 }
 
+NDIS_STATUS
+GetIpHeaderInfo(PNET_BUFFER_LIST curNbl,
+UINT32 *hdrSize)
+{
+CHAR *ethBuf[sizeof(EthHdr)];
+EthHdr *eth;
+IPHdr *ipHdr;
+PNET_BUFFER curNb;
+
+curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
+ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL);
+
+eth = (EthHdr *)NdisGetDataBuffer(curNb, ETH_HEADER_LENGTH,
+  (PVOID), 1, 0);
+if (eth == NULL) {
+return NDIS_STATUS_INVALID_PACKET;
+}
+ipHdr = (IPHdr *)((PCHAR)eth + ETH_HEADER_LENGTH);
+if (ipHdr == NULL) {
+return NDIS_STATUS_INVALID_PACKET;
+}
+*hdrSize = (UINT32)(ETH_HEADER_LENGTH + (ipHdr->ihl * 4));
+return NDIS_STATUS_SUCCESS;
+}
+
 /*
  * --
  * GetSegmentHeaderInfo
@@ -1112,15 +1137,16 @@ GetSegmentHeaderInfo(PNET_BUFFER_LIST nbl,
 
 /*
  * --
- * FixSegmentHeader
+ * FixPacketHeader
  *
- *Fix IP length, IP checksum, TCP sequence number and TCP checksum
- *in the segment.
+ *Fix IP length, Offset, IP checksum, TCP sequence number and TCP checksum
+ *in the segment as applicable.
  * --
  */
 static NDIS_STATUS
-FixSegmentHeader(PNET_BUFFER nb, UINT16 segmentSize, UINT32 seqNumber,
- BOOLEAN lastPacket, UINT16 packetCounter)
+FixPacketHeader(PNET_BUFFER nb, UINT16 segmentSize, UINT32 seqNumber,
+BOOLEAN lastPacket, UINT16 packetCounter, UINT16 offset,
+BOOLEAN isFragment)
 {
 EthHdr *dstEth = NULL;
 TCPHdr *dstTCP = NULL;
@@ -1139,41 +1165,55 @@ FixSegmentHeader(PNET_BUFFER nb, UINT16 segmentSize, 
UINT32 seqNumber,
 case ETH_TYPE_IPV4_NBO:
 {
 IPHdr *dstIP = NULL;
-
-ASSERT((INT)MmGetMdlByteCount(mdl) - NET_BUFFER_CURRENT_MDL_OFFSET(nb)
+if (!isFragment) {
+ASSERT((INT)MmGetMdlByteCount(mdl) - 
NET_BUFFER_CURRENT_MDL_OFFSET(nb)
 >= sizeof(EthHdr) + sizeof(IPHdr) + sizeof(TCPHdr));
-dstIP = (IPHdr *)((PCHAR)dstEth + sizeof(*dstEth));
-dstTCP = (TCPHdr *)((PCHAR)dstIP + dstIP->ihl * 4);
-ASSERT((INT)MmGetMdlByteCount(mdl) - NET_BUFFER_CURRENT_MDL_OFFSET(nb)
+dstIP = (IPHdr *)((PCHAR)dstEth + sizeof(*dstEth));
+dstTCP = (TCPHdr *)((PCHAR)dstIP + dstIP->ihl * 4);
+ASSERT((INT)MmGetMdlByteCount(mdl) - 
NET_BUFFER_CURRENT_MDL_OFFSET(nb)
 >= sizeof(EthHdr) + dstIP->ihl * 4 + TCP_HDR_LEN(dstTCP));
 
-/* Fix IP length and checksum */
-ASSERT(dstIP->protocol == IPPROTO_TCP);
-dstIP->tot_len = htons(segmentSize + dstIP->ihl * 4 + 
TCP_HDR_LEN(dstTCP));
-dstIP->id += packetCounter;
+/* Fix IP length and checksum */
+ASSERT(dstIP->protocol == IPPROTO_TCP);
+dstIP->tot_len = htons(segmentSize + dstIP->ihl * 4 + 
TCP_HDR_LEN(dstTCP));
+dstIP->id += packetCounter;
+dstTCP->seq = htonl(seqNumber);
+
+/*
+ * Set the TCP FIN and PSH bit only for the last packet
+ * More information can be found under:
+ * 
https://msdn.microsoft.com/en-us/library/windows/hardware/ff568840%28v=vs.85%29.aspx
+ */
+if (dstTCP->fin) {
+dstTCP->fin = lastPacket;
+}
+if (dstTCP->psh) {
+dstTCP->psh = lastPacket;
+}
+UINT16 csumLength = segmentSize + TCP_HDR_LEN(dstTCP);
+dstTCP->check = IPPseudoChecksum(>saddr,
+ >daddr,
+ IPPROTO_TCP,
+ csumLength);
+dstTCP->check = CalculateChecksumNB(nb,
+csumLength,
+sizeof(*dstEth) + dstIP->ihl * 
4);
+} else {
+

[ovs-dev] [PATCH v2 3/5] datapath-windows: Retain MRU value in the OvsForwardingContext.

2017-01-12 Thread Anand Kumar 
This patch retains the MRU value for the reassembled IP datagram in the
OvsForwardingContext when the packet is forwarded to userspace and/or
retrived from userspace.

Also retain the MRU value when there are any deferred actions for the
current NBL.
---
 datapath-windows/ovsext/Actions.c| 48 +++-
 datapath-windows/ovsext/Actions.h|  3 +++
 datapath-windows/ovsext/DpInternal.h |  2 +-
 datapath-windows/ovsext/PacketIO.c   |  5 ++--
 datapath-windows/ovsext/Recirc.c |  6 -
 datapath-windows/ovsext/Recirc.h |  6 +++--
 datapath-windows/ovsext/Tunnel.c |  4 +--
 datapath-windows/ovsext/User.c   | 26 +--
 datapath-windows/ovsext/User.h   |  6 +++--
 9 files changed, 72 insertions(+), 34 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index f378619..9cc79dc 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -150,6 +150,7 @@ OvsInitForwardingCtx(OvsForwardingContext *ovsFwdCtx,
  UINT32 srcVportNo,
  ULONG sendFlags,
  PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO 
fwdDetail,
+ UINT16 mru,
  OvsCompletionList *completionList,
  OVS_PACKET_HDR_INFO *layers,
  BOOLEAN resetTunnelInfo)
@@ -167,7 +168,7 @@ OvsInitForwardingCtx(OvsForwardingContext *ovsFwdCtx,
 ovsFwdCtx->switchContext = switchContext;
 ovsFwdCtx->completionList = completionList;
 ovsFwdCtx->fwdDetail = fwdDetail;
-
+ovsFwdCtx->mru = mru;
 if (fwdDetail->NumAvailableDestinations > 0) {
 /*
  * XXX: even though MSDN says GetNetBufferListDestinations() returns
@@ -615,6 +616,7 @@ OvsDoFlowLookupOutput(OvsForwardingContext *ovsFwdCtx)
  ovsFwdCtx->srcVportNo,
  ovsFwdCtx->sendFlags,
  , , >layers,
+ ovsFwdCtx->mru,
  flow->actions, flow->actionsLen);
 ovsFwdCtx->curNbl = NULL;
 } else {
@@ -623,9 +625,11 @@ OvsDoFlowLookupOutput(OvsForwardingContext *ovsFwdCtx)
 ovsFwdCtx->switchContext->datapath.misses++;
 InitializeListHead();
 status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS, vport,
-  ,ovsFwdCtx->curNbl,
-  FALSE, >layers,
-  ovsFwdCtx->switchContext, , );
+,ovsFwdCtx->curNbl,
+FALSE, >layers,
+ovsFwdCtx->switchContext,
+ovsFwdCtx->mru,
+, );
 if (num) {
 OvsQueuePackets(, num);
 }
@@ -722,6 +726,7 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
 status = OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
   newNbl, srcVportNo, 0,
   
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
+  ovsFwdCtx->mru,
   ovsFwdCtx->completionList,
   >layers, FALSE);
 ovsFwdCtx->curNbl = newNbl;
@@ -822,6 +827,7 @@ OvsTunnelPortRx(OvsForwardingContext *ovsFwdCtx)
 OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
  newNbl, tunnelRxVport->portNo, 0,
  NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
+ ovsFwdCtx->mru,
  ovsFwdCtx->completionList,
  >layers, FALSE);
 
@@ -921,6 +927,7 @@ OvsOutputForwardingCtx(OvsForwardingContext *ovsFwdCtx)
 status = OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
   newNbl, ovsFwdCtx->srcVportNo, 0,
   
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
+  ovsFwdCtx->mru,
   ovsFwdCtx->completionList,
   >layers, FALSE);
 if (status != NDIS_STATUS_SUCCESS) {
@@ -986,7 +993,7 @@ OvsLookupFlowOutput(POVS_SWITCH_CONTEXT switchContext,
 status = OvsInitForwardingCtx(, switchContext, curNbl,
   internalVport->portNo, 0,
   
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(curNbl),
-  completionList, NULL, TRUE);
+  0, completionList, NULL, TRUE);
 if (status != NDIS_STATUS_SUCCESS) {
 OvsCompleteNBLForwardingCtx(,

[ovs-dev] [PATCH v2 2/5] datapath-windows: Added Ipv4 fragments support in Conntrack

2017-01-12 Thread Anand Kumar 
This patch adds support for Ipv4 fragments in conntrack module.
Individual fragments are not tracked, the reassembled Ipv4 datagram is treated
as a single ct entry.

Added MRU field in OvsForwardingContext, to keep track of Maximum recieved unit
from all the recieved IPv4 fragments.
---
 datapath-windows/ovsext/Actions.c   | 15 +++
 datapath-windows/ovsext/Conntrack.c | 31 +--
 datapath-windows/ovsext/Conntrack.h |  7 ++-
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index b4a286b..f378619 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -125,6 +125,9 @@ typedef struct OvsForwardingContext {
 
 /* header information */
 OVS_PACKET_HDR_INFO layers;
+
+/* Maximum Recieving Unit */
+UINT16 mru;
 } OvsForwardingContext;
 
 /*
@@ -1910,11 +1913,15 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
 }
 }
 
-status = OvsExecuteConntrackAction(ovsFwdCtx.curNbl, layers,
-   key, (const PNL_ATTR)a);
+status = OvsExecuteConntrackAction(switchContext, 
, &(ovsFwdCtx.mru),
+   ovsFwdCtx.completionList,
+   
ovsFwdCtx.fwdDetail->SourcePortId,
+   layers, key, (const PNL_ATTR)a);
 if (status != NDIS_STATUS_SUCCESS) {
-OVS_LOG_ERROR("CT Action failed");
-dropReason = L"OVS-conntrack action failed";
+if (status != NDIS_STATUS_PENDING) {
+OVS_LOG_ERROR("CT Action failed");
+dropReason = L"OVS-conntrack action failed";
+}
 goto dropit;
 }
 break;
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index d1be480..219680e 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -15,6 +15,7 @@
  */
 
 #include "Conntrack.h"
+#include "IpFragment.h"
 #include "Jhash.h"
 #include "PacketParser.h"
 #include "Event.h"
@@ -312,13 +313,25 @@ OvsCtEntryExpired(POVS_CT_ENTRY entry)
 }
 
 static __inline NDIS_STATUS
-OvsDetectCtPacket(OvsFlowKey *key)
+OvsDetectCtPacket(POVS_SWITCH_CONTEXT switchContext,
+  PNET_BUFFER_LIST *curNbl,
+  OvsCompletionList *completionList,
+  NDIS_SWITCH_PORT_ID sourcePort,
+  OvsFlowKey *key,
+  UINT16 *mru,
+  PNET_BUFFER_LIST *newNbl)
 {
 /* Currently we support only Unfragmented TCP packets */
 switch (ntohs(key->l2.dlType)) {
 case ETH_TYPE_IPV4:
 if (key->ipKey.nwFrag != OVS_FRAG_TYPE_NONE) {
-return NDIS_STATUS_NOT_SUPPORTED;
+return OvsProcessIpv4Fragment(switchContext,
+  curNbl,
+  completionList,
+  sourcePort,
+  mru,
+  key->tunKey.tunnelId,
+  newNbl);
 }
 if (key->ipKey.nwProto == IPPROTO_TCP
 || key->ipKey.nwProto == IPPROTO_UDP
@@ -688,7 +701,11 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
  *---
  */
 NDIS_STATUS
-OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
+OvsExecuteConntrackAction(POVS_SWITCH_CONTEXT switchContext,
+  PNET_BUFFER_LIST *curNbl,
+  UINT16 *mru,
+  OvsCompletionList *completionList,
+  NDIS_SWITCH_PORT_ID sourcePort,
   OVS_PACKET_HDR_INFO *layers,
   OvsFlowKey *key,
   const PNL_ATTR a)
@@ -699,10 +716,11 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
 MD_MARK *mark = NULL;
 MD_LABELS *labels = NULL;
 PCHAR helper = NULL;
-
+PNET_BUFFER_LIST newNbl = NULL;
 NDIS_STATUS status;
 
-status = OvsDetectCtPacket(key);
+status = OvsDetectCtPacket(switchContext, curNbl, completionList,
+   sourcePort, key, mru, );
 if (status != NDIS_STATUS_SUCCESS) {
 return status;
 }
@@ -735,7 +753,8 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
 }
 }
 
-status = OvsCtExecute_(curNbl, key, layers,
+/* If newNbl is not allocated, use the current Nbl*/
+status = OvsCtExecute_(newNbl != NULL ? newNbl : *curNbl, key, layers,
commit, zone, mark, labels, helper);
 return status;
 }
diff --git a/datapath-windows/ovsext/Conntrack.h 
b/datapath-windows/ovsext/Conntrack.h

[ovs-dev] [BUG] ovs-ofctl version 2.5.0 will crash with OFPFMFC_BAD_COMMAND

2017-01-12 Thread Samuel Jean via dev 
Hi,

It seems that shelling out to ovs-ofctl very quickly can lead to bug where
it reports an OFPT_ERROR.

We were able to constantly reproduce within minutes of running the above
flow modifications on Unbutu.

Any help, hints or guidance would be appreciated.  I'd be happy to pursue
some debugging that would be required to nail down the issue here.

Best regards,
Sam Jean

# cat ./ofpcrash.sh
#!/bin/sh
ovs-ofctl add-flow br0
'priority=100,table=25,idle_timeout=0,actions=resubmit(,35)' || exit 1
ovs-ofctl add-flow br0
'priority=100,table=35,idle_timeout=0,actions=resubmit(,45)' || exit 1
ovs-ofctl add-flow br0
'priority=100,table=45,idle_timeout=0,actions=resubmit(,50)' || exit 1
ovs-ofctl add-flow br0
'priority=100,table=50,idle_timeout=0,actions=resubmit(,65)' || exit 1
ovs-ofctl add-flow br0
'priority=100,table=65,idle_timeout=0,actions=output:1' || exit 1
ovs-ofctl add-flow br0
'priority=3000,ip,dl_dst=01:00:00:00:00:00/01:00:00:00:00:00,table=0,idle_timeout=0,actions=drop'
|| exit 1
ovs-ofctl add-flow br0
'priority=1000,ip,in_port=1,dl_dst=0c:01:00:12:cf:01,table=0,idle_timeout=0,cookie=20250774994944,actions=resubmit(,25)'
|| exit 1

# while true; do ./ofpcrash.sh || break; done
OFPT_ERROR (xid=0x4): OFPFMFC_BAD_COMMAND
OFPT_FLOW_MOD (xid=0x4):
(***truncated to 64 bytes from 88***)
  01 0e 00 58 00 00 00 04-00 38 20 ff 00 00 00 00 |...X.8 .|
0010  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ||
0020  00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 ||
0030  00 00 00 00 00 00 00 00-32 00 00 00 00 00 00 64 |2..d|

# ovs-ofctl --version
ovs-ofctl (Open vSwitch) 2.5.0
Compiled Sep 15 2016 12:55:18
OpenFlow versions 0x1:0x4

# uname -srmvp
Linux 3.13.0-100-generic #147-Ubuntu SMP Tue Oct 18 16:48:51 UTC 2016
x86_64 x86_64
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif: Simplify dpif_execute_helper_cb()

2017-01-12 Thread Andy Zhou 
On Wed, Jan 11, 2017 at 4:39 PM, Jarno Rajahalme  wrote:

> Looks good to me:
>
> Acked-by: Jarno Rajahalme 
>
Thanks, pushed.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] travis: Update build list email address.

2017-01-12 Thread Andy Zhou 
On Thu, Jan 12, 2017 at 9:16 AM, Ben Pfaff  wrote:

> The lists these days prefer an ovs- prefix.  Currently all of the build
> emails are being dropped because it is missing.
>
> Signed-off-by: Ben Pfaff 
>

 Acked-by: Andy Zhou 

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

[ovs-dev] [PATCH] ofproto-dpif: Use acquire/release barriers with 'tables_version'.

2017-01-12 Thread Jarno Rajahalme 
Use memory_order_release when updating the tables version number to
make sure no memory accesses before the atomic_store (possibly
relating to setting up the new version) are reordered to take place
after the atomic_store, which makes the new version available to other
threads.

Correspondingly, use memory_order_acquire when reading the
current tables_version to make sure no later memory accesses (possibly
relating to the current version) are reordered to take place before
the atomic_read to ensure that those memory accesses can not relate to
an older version than returned by the atomic_read.

Suggested-by: Daniele Di Proietto 
Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-dpif.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 3b274ee..0e6842d 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1548,7 +1548,15 @@ set_tables_version(struct ofproto *ofproto_, 
ovs_version_t version)
 {
 struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
 
-atomic_store_relaxed(>tables_version, version);
+/* Use memory_order_release to signify that any prior memory accesses can
+ * not be reordered to happen after this atomic store.  This makes sure the
+ * new version is properly set up when the readers can read this 'version'
+ * value. */
+atomic_store_explicit(>tables_version, version,
+  memory_order_release);
+/* 'need_revalidate' can be reordered to happen before the atomic_store
+ * above, but it does not matter as this variable is not accessed by other
+ * threads. */
 ofproto->backer->need_revalidate = REV_FLOW_TABLE;
 }
 
@@ -3723,8 +3731,12 @@ ofproto_dpif_get_tables_version(struct ofproto_dpif 
*ofproto)
 {
 ovs_version_t version;
 
-atomic_read_relaxed(>tables_version, );
-
+/* Use memory_order_acquire to signify that any following memory accesses
+ * can not be reordered to happen before this atomic read.  This makes sure
+ * all following reads relate to this or a newer version, but never to an
+ * older version. */
+atomic_read_explicit(>tables_version, ,
+ memory_order_acquire);
 return version;
 }
 
-- 
2.1.4

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

Re: [ovs-dev] [PATCH] netdev-vport: Do not log empty warnings on success.

2017-01-12 Thread Daniele Di Proietto 





On 12/01/2017 09:33, "Ben Pfaff"  wrote:

>On Thu, Jan 12, 2017 at 12:23:55AM -0800, Daniele Di Proietto wrote:
>> set_tunnel_config() always logs a warning, even on success. This
>> shouldn't happen.
>> 
>> Without this, some unit tests fail.
>> 
>> Fixes: 9fff138ec3a6("netdev: Add 'errp' to set_config().")
>> Signed-off-by: Daniele Di Proietto 
>> ---
>>  lib/netdev-vport.c | 10 ++
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>> index ad5ffcc81..2db51df72 100644
>> --- a/lib/netdev-vport.c
>> +++ b/lib/netdev-vport.c
>> @@ -561,10 +561,12 @@ set_tunnel_config(struct netdev *dev_, const struct 
>> smap *args, char **errp)
>>  err = 0;
>>  
>>  out:
>> -ds_chomp(, '\n');
>> -VLOG_WARN("%s", ds_cstr());
>> -if (err) {
>> -*errp = ds_steal_cstr();
>> +if (*ds_cstr()) {
>
>How about "if (errors.length)" instead?

Ok

>
>> +ds_chomp(, '\n');
>> +VLOG_WARN("%s", ds_cstr());
>> +if (err) {
>> +*errp = ds_steal_cstr();
>> +}
>>  }
>>  
>>  ds_destroy();
>
>Acked-by: Ben Pfaff 

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

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

2017-01-12 Thread Shashank Ram 
Hi Alin, I plan to replace all occurrences of "portId == 
switchContext->virtualExternalPortId" by calling into the wrapper. Will send 
out a subsequent patch soon.


Thanks,

Shashank


From: Alin Serdean 
Sent: Thursday, January 12, 2017 8:27:22 AM
To: Shashank Ram; d...@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Add a wrapper to check for 
external vport

Hi,

Thanks for the patch.

Is this part of a series or is it a standalone patch?

I don't see the added value to add the wrapper just from this patch.

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Shashank Ram
> Sent: Thursday, January 12, 2017 12:45 AM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH] datapath-windows: Add a wrapper to check for
> external vport
>
> Signed-off-by: Shashank Ram 
> ---
>  datapath-windows/ovsext/Vport.c | 6 ++  datapath-
> windows/ovsext/Vport.h | 2 ++
>  2 files changed, 8 insertions(+)
>
> diff --git a/datapath-windows/ovsext/Vport.c b/datapath-
> windows/ovsext/Vport.c index e9e22aa..9142937 100644
> --- a/datapath-windows/ovsext/Vport.c
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-dpdk: Use intermediate queue during packet transmission.

2017-01-12 Thread Fischetti, Antonio 
Hi Ilya,
thanks for your detailed feedback. I've added a couple of replies 
inline about when the instant send happens and the may_steal param.


Regards,
Antonio

> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Tuesday, December 20, 2016 12:47 PM
> To: Bodireddy, Bhanuprakash ; Aaron
> Conole 
> Cc: d...@openvswitch.org; Daniele Di Proietto ;
> Thadeu Lima de Souza Cascardo ; Fischetti, Antonio
> ; Markus Magnusson
> 
> Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Use intermediate queue during
> packet transmission.
> 
> On 20.12.2016 15:19, Bodireddy, Bhanuprakash wrote:
> >> -Original Message-
> >> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> >> Sent: Tuesday, December 20, 2016 8:09 AM
> >> To: Bodireddy, Bhanuprakash ; Aaron
> >> Conole 
> >> Cc: d...@openvswitch.org; Daniele Di Proietto ;
> >> Thadeu Lima de Souza Cascardo ; Fischetti, Antonio
> >> ; Markus Magnusson
> >> 
> >> Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Use intermediate queue
> during
> >> packet transmission.
> >>
> >> On 19.12.2016 21:05, Bodireddy, Bhanuprakash wrote:
> >>> Thanks Ilya and Aaron for reviewing this patch and providing your
> >> comments, my reply inline.
> >>>
>  -Original Message-
>  From: Ilya Maximets [mailto:i.maxim...@samsung.com]
>  Sent: Monday, December 19, 2016 8:41 AM
>  To: Aaron Conole ; Bodireddy, Bhanuprakash
>  
>  Cc: d...@openvswitch.org; Daniele Di Proietto
>  ; Thadeu Lima de Souza Cascardo
>  ; Fischetti, Antonio
>  ; Markus Magnusson
>  
>  Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Use intermediate queue
>  during packet transmission.
> 
>  Hi,
> 
>  I didn't test this patch yet. Bhanu, could you please describe your
>  test scenario and performance results in more details.
> >>>
> >>> During the recent performance analysis improvements for classifier, we
> >> found that bottleneck was also observed at flow batching.
> >>> This was due to fewer packets in batch. To reproduce this, a simple
> P2P test
> >> case can be used with 30 IXIA streams and matching IP flow rules.
> >>> Eg:  For an IXIA stream with src Ip: 2.2.2.1, dst tip: 5.5.5.1
> >>> ovs-ofctl add-flow br0
> >>> dl_type=0x0800,nw_src=2.2.2.1,actions=output:2
> >>>
> >>> For an IXIA stream with src Ip: 4.4.4.1, dst tip: 15.15.15.1
> >>> ovs-ofctl add-flow br0
> >>> dl_type=0x0800,nw_src=4.4.4.1,actions=output:2
> >>>
> >>> This leaves fewer packets in batches and
> packet_batch_per_flow_execute()
> >> shall be invoked for every batch.
> >>> With 30 flows, I see the throughput drops to ~7.0 Mpps in PHY2PHY case
> for
> >> 64 byte udp packets.
> >>>
>  It'll be nice if you provide throughput and latency measurement
>  results for different scenarios and packet sizes. Latency is
> important here.
> >>> We are yet to do latency measurements in this case. With 30 IXIA
> >>> streams comprising of 64 byte udp packets there was an throughput
> >>> improvement of 30% in P2P case and 13-15% in PVP case(single queue).
> we
> >> will try to get the latency stats with and without this patch.
> >>>
> 
>  About the patch itself:
> 
>   1. 'drain' called only for PMD threads. This will lead to
>  broken connection with non-PMD ports.
> >>> I see that non-PMD ports are handled with vswitchd thread.
> >>> Tested PVP loopback case with tap ports and found to be working as
> >>> expected. Can you let me know the specific case you are referring here
> so
> >> that I can verify if the patch breaks it.
> >>
> >> I meant something like this:
> >>
> >>
> >> *---HOST-1(br-int)-*   *-HOST-2(br-
> int)--*
> >> |  |   |
> |
> >> |   internal_port <--> dpdk0 <---> dpdk0 <-->
> internal_port  |
> >> |   192.168.0.1/24 |   |
> 192.168.0.2/24 |
> >> *--*   *---
> --*
> >>
> >> (HOST-1)# ping 192.168.0.2
> >>
> >> In this case I'm expecting that first (NETDEV_MAX_BURST - 1) icmp
> packets
> >> will stuck in TX queue. The next packet will cause sending the whole
> batch of
> >> NETDEV_MAX_BURST icmp packets. That is not good behaviour.
> >
> > Thanks for test case. I tested this and found to be working with the
> patch.
> > The reason being, PMD threads uses intermediate queue implementation by
> invoking 'netdev_dpdk_eth_tx_queue()',

Re: [ovs-dev] broken tests

2017-01-12 Thread Daniele Di Proietto 





On 12/01/2017 09:24, "Ben Pfaff"  wrote:

>Commit 9fff138ec3a6dbe75073d16cba7fbe86ac273c36 "netdev: Add 'errp' to
>set_config()." breaks the unit tests because netdev-vport now logs lots
>of blank lines.  I am unsure of the right fix--is it to just drop the
>new VLOG_WARN call?

Hi Ben,

Sorry about that, I posted a patch that should fix this:

https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327564.html

If you're fine with it, I'll merge it shortly.

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

[ovs-dev] broken tests

2017-01-12 Thread Ben Pfaff 
Commit 9fff138ec3a6dbe75073d16cba7fbe86ac273c36 "netdev: Add 'errp' to
set_config()." breaks the unit tests because netdev-vport now logs lots
of blank lines.  I am unsure of the right fix--is it to just drop the
new VLOG_WARN call?

Thanks,

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

Re: [ovs-dev] [PATCH v4] ofproto-dpif: Make ofproto/trace output easier to read.

2017-01-12 Thread Ben Pfaff 
On Mon, Jan 09, 2017 at 08:46:28PM -0800, Justin Pettit wrote:
> I think it would be helpful to have a comment describing this
> function.  Also mentioning that the caller maintains ownership of
> 'text'.

There was a lot of missing comments and documentation.  I added a bunch.

> I may be missing something, but is there anything that frees these
> "oftrace_node"s either here or ofproto-dpif-xlate.c?

No.  Oops.  I fixed this.

> > On Jan 5, 2017, at 5:04 PM, Ben Pfaff  wrote:
>
> > static void
> > +oftrace_node_print_details(struct ds *output,
> > +const struct ovs_list *nodes, int level)
> > {
> > +const struct oftrace_node *sub;
> > +LIST_FOR_EACH (sub, node, nodes) {
> > +bool more = sub->node.next != nodes || sub->always_indent || 
> > oftrace_node_type_is_terminal(sub->type);
> 
> This line is much longer than 80 characters.

'always_indent' wasn't useful anyhow.  I removed this clause and split
the line.

> > +bool title = sub->type == OFT_BRIDGE;
> > 
> > ...
> > +if (title) {
> > +ds_put_char(output, '\n');
> > +}
> > +ds_put_char_multiple(output, ' ', (level + more) * 4);
> 
> It's clever, but, 'more' being a bool looks kind of weird to me seeing
> it used for math.

I spent a few minutes thinking of other ways to express this, but (level
+ (more ? 1 : 0)) isn't better, and using a separate statement with ++
doesn't seem better either.

After talking to you for a minute in-person, I decided to leave this as
is.

> > +switch (sub->type) {
> > +case OFT_DETAIL:
> > +ds_put_format(output, " -> %s\n", sub->name);
> > +break;
> > +case OFT_WARN:
> > +ds_put_format(output, " >> %s\n", sub->name);
> > +break;
> > +case OFT_ERROR:
> > +ds_put_format(output, "  %s \n", sub->name);
> > +break;
> > +case OFT_BRIDGE:
> > +ds_put_format(output, "%s\n", sub->name);
> > +ds_put_char_multiple(output, ' ', (level + more) * 4);
> > +ds_put_char_multiple(output, '-', strlen(sub->name));
> > +ds_put_char(output, '\n');
> > +break;
> > +case OFT_TABLE:
> > +case OFT_THAW:
> > +case OFT_ACTION:
> > +ds_put_format(output, "%s\n", sub->name);
> > +break;
> > }
> 
> The actions printing at a shallower depth than the rule looks a little
> odd to me.  I suppose that helps if there are a lot of actions.
> Regardless, I'm sure I'll get used to it.

It only seems at first glance that it's indented shallower, but in fact
it comes out in a natural way because of the nesting, e.g.:

bridge("br0")
-
 0. priority 0
resubmit(,1)
 1. in_port=2,vlan_tci=0x, priority 99
mod_vlan_vid:20
resubmit(,2)
 2. No match.
drop

> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index 371ced4..f96468d 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > ...
> > +static struct ovs_list * OVS_PRINTF_FORMAT(3, 4)
> > +xlate_report(const struct xlate_ctx *ctx, enum oftrace_node_type type,
> > + const char *format, ...)
> > {
> > -if (OVS_UNLIKELY(ctx->xin->report_hook)) {
> > +struct ovs_list *subtrace = NULL;
> > +if (OVS_UNLIKELY(ctx->xin->trace)) {
> > va_list args;
> > -
> > va_start(args, format);
> > -ctx->xin->report_hook(ctx->xin, ctx->indentation, format, args);
> > +char *text = xvasprintf(format, args);
> > +subtrace = _report(ctx->xin->trace, type, text)->subs;
> 
> I think you need to free "text", since oftrace_report() makes a copy.

Thanks, fixed.

> > +/* This is like xlate_report() for errors that are serious enough that we
> > + * should log them even if we are not tracing. */
> > +static void OVS_PRINTF_FORMAT(2, 3)
> > +xlate_report_error(const struct xlate_ctx *ctx, const char *format, ...)
> 
> The next two functions in the source file have comments pointing out
> that they're similar to xlate_report(), but xlate_report() doesn't
> have a comment describing it.  It might be nice to do that.

I added better comments everywhere.

> The difference in behavior between xlate_report_actions() and
> xlate_report_action_set() seem to be more than just that one has to do
> with actions and the other with action sets.  It might be nice to
> document the differences.

Including here.

> > +ctx->xin->trace = old_trace;
> > if (independent_mirrors) {
> > ctx->mirrors = old_mirrors;
> > }
> 
> There are a few of these "old_trace" logic blocks, and I think the
> memory management and linked list processing may not be handled
> properly.

This works out OK in practice.  Memory management is not a problem, once
I added some; it's just a tree, you free it from root down.

> > @@

[ovs-dev] [PATCH 0/1] dpif-netdev: Conditional EMC insert

2017-01-12 Thread Ciara Loftus 
This patch is part of the OVS-DPDK performance optimizations presented
on the OVS fall conference
(http://openvswitch.org/support/ovscon2016/8/1400-gray.pdf)

The Exact Match Cache does not perform well in use cases with a high
numbers of parallel packet flows. When the flow count exceeds 8k, which 
is the size of the EMC, 'thrashing' occurs. Subsequent packets incur
EMC misses and lead to the insertion of new entries, which are likely 
already overwritten by the time the next packet of a flow arrives.

The extra cost of useless EMC insertions and failed EMC lookups degrades
the performance of the netdev-dpdk datapath up to 30% compared to the
pure performance of the dpcls classifier with EMC disabled. Profiling 
has shown that the bulk of the degradation is caused by the EMC 
insertions.

To avoid this degradation we apply 'probabilistic' EMC insertions, whereby
an EMC miss only results in an EMC insertion with a random probability
of P=1/N (N integer). With this model, the insertion overhead of the EMC
can be reduced by a factor N, while the EMC speedup is maintained for a
small to medium number of flows. Probabilistic insertion can be
implemented with minimal run-time cost and naturally favors long-lived
flows. 

Different values for P from 1/100 to 1/4000 were validated and benchmarked
when creating this patch. Not much variance between different probabilities 
was observed. 

We chose P=1/100 because it gives almost the same improvement as lower
probabilities while reaching the EMC capacity and thus optimal performance
quicker than the lower probabilities.

For P=1/100, the EMC reached full capacity after 843ms when subjecting the 
datapath with 10 long-lived flows at 14.8 Mpps. This is much quicker 
compared to P=1/500 and P=1/1000, which took 3792ms and 7370ms 
respectively.

 lib/dpif-netdev.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

-- 
2.4.3

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

Re: [ovs-dev] [PATCH v1 0/5] datapath-windows: Add support for Ipv4 fragments

2017-01-12 Thread Alin Serdean 
Hi,

Thanks a lot for the series, it will be a great addition to the Windows 
datapath!

One small nit it does not compile under release because of `ovspool` is defined 
only in debug mode.

I also tried to set up an environment and for some reason the fragments were 
stalling. I tried icmp and tcp with no luck.

I will take a better look over the code and come back with more comments.

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Anand Kumar
> Sent: Tuesday, January 10, 2017 2:59 AM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH v1 0/5] datapath-windows: Add support for Ipv4
> fragments
> 
> Add support for maintaining and tracking IPv4 fragments.
> This patch adds a new file IpFragment.c and IpFragment.h which includes
> Ipv4 fragment related API's.
> 
> Anand Kumar (5):
>   datapath-windows: Added a new file to support Ipv4 fragments.
>   datapath-windows: Added Ipv4 fragments support in Conntrack
>   datapath-windows: Retain MRU value in the OvsForwardingContext.
>   datapath-windows: Updated OvsTcpSegmentNBL to handle IP fragments.
>   datapath-windows: Fragment NBL based on MRU size
> 
>  datapath-windows/automake.mk   |   2 +
>  datapath-windows/ovsext/Actions.c  |  89 --
>  datapath-windows/ovsext/Actions.h  |   3 +
>  datapath-windows/ovsext/BufferMgmt.c   | 189 
>  datapath-windows/ovsext/BufferMgmt.h   |  10 +-
>  datapath-windows/ovsext/Conntrack.c|  31 +-
>  datapath-windows/ovsext/Conntrack.h|   7 +-
>  datapath-windows/ovsext/Debug.h|   3 +-
>  datapath-windows/ovsext/DpInternal.h   |   2 +-
>  datapath-windows/ovsext/Geneve.c   |   2 +-
>  datapath-windows/ovsext/Gre.c  |   2 +-
>  datapath-windows/ovsext/IpFragment.c   | 506
> +
>  datapath-windows/ovsext/IpFragment.h   |  74 +
>  datapath-windows/ovsext/PacketIO.c |   5 +-
>  datapath-windows/ovsext/Recirc.c   |   6 +-
>  datapath-windows/ovsext/Recirc.h   |   6 +-
>  datapath-windows/ovsext/Stt.c  |   2 +-
>  datapath-windows/ovsext/Switch.c   |   9 +
>  datapath-windows/ovsext/Tunnel.c   |   4 +-
>  datapath-windows/ovsext/User.c |  28 +-
>  datapath-windows/ovsext/User.h |   6 +-
>  datapath-windows/ovsext/Vxlan.c|   2 +-
>  datapath-windows/ovsext/ovsext.vcxproj |   2 +
>  23 files changed, 885 insertions(+), 105 deletions(-)  create mode 100644
> datapath-windows/ovsext/IpFragment.c
>  create mode 100644 datapath-windows/ovsext/IpFragment.h
> 
> --
> 2.9.3.windows.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] datapath-windows: Add a wrapper to check for external vport

2017-01-12 Thread Alin Serdean 
Hi,

Thanks for the patch.

Is this part of a series or is it a standalone patch?

I don't see the added value to add the wrapper just from this patch.

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Shashank Ram
> Sent: Thursday, January 12, 2017 12:45 AM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH] datapath-windows: Add a wrapper to check for
> external vport
> 
> Signed-off-by: Shashank Ram 
> ---
>  datapath-windows/ovsext/Vport.c | 6 ++  datapath-
> windows/ovsext/Vport.h | 2 ++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/datapath-windows/ovsext/Vport.c b/datapath-
> windows/ovsext/Vport.c index e9e22aa..9142937 100644
> --- a/datapath-windows/ovsext/Vport.c
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] revalidator: Prevent double-delete of ukey.

2017-01-12 Thread Numan Siddique 
On Thu, Jan 12, 2017 at 4:05 AM, Joe Stringer  wrote:

> On 10 January 2017 at 16:00, Jarno Rajahalme  wrote:
> > Acked-by: Jarno Rajahalme 
>
> Thanks, applied to master.
>
> Numan, I believe that this is an issue regardless of whether it fixes
> your issue. Let us know if you have any further trouble.
>

​Sure. I will do. Thanks for the fix.

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

Re: [ovs-dev] [PATCH v3 00/18] DPDK/pmd reconfiguration refactor and bugfixes

2017-01-12 Thread Ilya Maximets 
Hi, Daniele.
Thanks for v3.

Acked-by: Ilya Maximets 

On 09.01.2017 06:14, Daniele Di Proietto wrote:
> The first two commits of the series are trivial bugfixes for dpif-netdev.
> 
> Then the series fixes a long standing bug that caused a crash when the
> admin link state of a port is changed while traffic is flowing.
> 
> The next part makes use of reconfiguration for port add: this makes
> the operation twice as fast and reduce some code duplication.  This part
> conflicts with the port naming change, so I'm willing to postpone it, unless
> we find it to be useful for the port naming change.
> 
> The rest of the series refactors a lot of code if dpif-netdev:
> 
> * We no longer start pmd threads on demand for each numa node.  This made
>   the code very complicated and introduced a lot of bugs.
> * The pmd threads state is now internal to dpif-netdev and it's not stored in
>   ovs-numa.
> * There's now a single function that handles pmd threads/ports changes: this
>   reduces code duplication and makes port reconfiguration faster, as we don't
>   have to bring down the whole datapath.
> 
> v3->v2:
> 
> * Rebased:
>   * Rebased against dpdk arbitrary name change.
>   * Dropped unsigned 'core_id' commit because a similar fix is already
> on master
> * Put space between *FOR_EACH* and (
> * Actually use new FOR_EACH_NUMA_ON_DUMP
> * Use hmap_contains() instead of dp_netdev_lookup_port() in a couple of
>   places
> * Restore spaces in log messages, lost while wrapping the string.
> 
> v1->v2:
> 
> * Postpone cls deletion in dp_netdev_destroy_pmd()
> * Allow ports to be in tnl_port_cache and send_port_cache at the same time
> * Set counter to 1025 when reloading pmd without queues to be polled
> * Rebased:
>   * Allow 0x in pmd-cpu-mask
>   * ...
> * Don't duplicate get_core_by_core_id() in get_cpu_core()
> * New commit for ovs-numa: don't use hmap_first_with_hash()
> * Keep per numa count of cores in ovs_numa_dump
> * Print queue id and port name in warning if there's no pmd thread
> * Extract pmd_remove_stale_ports() from reconfigure_datapath()
> * s/reload_all_pmds()/reload_affected_pmds()/
> * Declare variables at the beginning of the block in rxq_scheduling()
> * Use 'q' instead of 'port->rxqs[qid]' in a couple of places
> * Unref pmd in rxq_scheduling()
> * Simplify check for changed pmd threads
> * Properly reset queues to unassigned in reconfigure_datapath()
> * Optimize tx port insertion in pmd cache
> 
> 
> Daniele Di Proietto (18):
>   dpif-netdev: Fix memory leak.
>   dpif-netdev: Take non_pmd_mutex to access tx cached ports.
>   dpif-netdev: Don't try to output on a device without txqs.
>   netdev-dpdk: Don't call rte_dev_stop() in update_flags().
>   netdev-dpdk: Start also dpdkr devices only once on port-add.
>   netdev-dpdk: Refactor construct and destruct.
>   dpif-netdev: Use a boolean instead of pmd->port_seq.
>   dpif-netdev: Block pmd threads if there are no ports.
>   dpif-netdev: Create pmd threads for every numa node.
>   dpif-netdev: Make 'static_tx_qid' const.
>   dpctl: Avoid making assumptions on pmd threads.
>   ovs-numa: New ovs_numa_dump_contains_core() function.
>   ovs-numa: Add new dump types.
>   ovs-numa: Don't use hmap_first_with_hash().
>   ovs-numa: Add per numa and global counts in dump.
>   dpif-netdev: Use hmap for poll_list in pmd threads.
>   dpif-netdev: Centralized threads and queues handling code.
>   ovs-numa: Remove unused functions.
> 
>  lib/dpctl.c   |  107 +---
>  lib/dpif-netdev.c | 1427 
> -
>  lib/dpif.c|6 +-
>  lib/dpif.h|   12 +-
>  lib/netdev-dpdk.c |  170 +++
>  lib/netdev.c  |   41 +-
>  lib/netdev.h  |1 +
>  lib/ovs-numa.c|  284 +--
>  lib/ovs-numa.h|   35 +-
>  tests/pmd.at  |   49 +-
>  vswitchd/bridge.c |2 +
>  11 files changed, 1079 insertions(+), 1055 deletions(-)
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] dpif-netdev: add EMC entry count and %full figure to pmd-stats-show

2017-01-12 Thread Ciara Loftus 
'pmd-stats-show' now reports the number of entries in the EMC as well as
the percentage full it is.
Eg. For 2048 entries the EMC is reported as 25% full as the maximum
capacity is 8192 entries.

Signed-off-by: Ciara Loftus 
Signed-off-by: Georg Schmuecking 
Co-authored-by: Georg Schmuecking 
---
 lib/dpif-netdev.c | 27 +++
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 546a1e9..0b0ca54 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -152,6 +152,7 @@ struct emc_entry {
 struct emc_cache {
 struct emc_entry entries[EM_FLOW_HASH_ENTRIES];
 int sweep_idx;/* For emc_cache_slow_sweep(). */
+int n_entries;/* Number of EMC entries */
 };
 
 /* Iterate in the exact match cache through every entry that might contain a
@@ -601,7 +602,7 @@ static int dpif_netdev_xps_get_tx_qid(const struct 
dp_netdev_pmd_thread *pmd,
   struct tx_port *tx, long long now);
 
 static inline bool emc_entry_alive(struct emc_entry *ce);
-static void emc_clear_entry(struct emc_entry *ce);
+static void emc_clear_entry(struct emc_cache *cache, struct emc_entry *ce);
 
 static void
 emc_cache_init(struct emc_cache *flow_cache)
@@ -609,6 +610,7 @@ emc_cache_init(struct emc_cache *flow_cache)
 int i;
 
 flow_cache->sweep_idx = 0;
+flow_cache->n_entries = 0;
 for (i = 0; i < ARRAY_SIZE(flow_cache->entries); i++) {
 flow_cache->entries[i].flow = NULL;
 flow_cache->entries[i].key.hash = 0;
@@ -623,8 +625,9 @@ emc_cache_uninit(struct emc_cache *flow_cache)
 int i;
 
 for (i = 0; i < ARRAY_SIZE(flow_cache->entries); i++) {
-emc_clear_entry(_cache->entries[i]);
+emc_clear_entry(flow_cache, _cache->entries[i]);
 }
+flow_cache->n_entries = 0;
 }
 
 /* Check and clear dead flow references slowly (one entry at each
@@ -635,7 +638,7 @@ emc_cache_slow_sweep(struct emc_cache *flow_cache)
 struct emc_entry *entry = _cache->entries[flow_cache->sweep_idx];
 
 if (!emc_entry_alive(entry)) {
-emc_clear_entry(entry);
+emc_clear_entry(flow_cache, entry);
 }
 flow_cache->sweep_idx = (flow_cache->sweep_idx + 1) & EM_FLOW_HASH_MASK;
 }
@@ -717,9 +720,13 @@ pmd_info_show_stats(struct ds *reply,
 ds_put_cstr(reply, ":\n");
 
 ds_put_format(reply,
-  "\temc hits:%llu\n\tmegaflow hits:%llu\n"
+  "\temc entries:%i (%.2f%% full)\n\temc hits:%llu\n"
+  "\tmegaflow hits:%llu\n"
   "\tavg. subtable lookups per hit:%.2f\n"
   "\tmiss:%llu\n\tlost:%llu\n",
+  pmd->flow_cache.n_entries,
+  ((double)pmd->flow_cache.n_entries /
+  (double)EM_FLOW_HASH_ENTRIES) * 100,
   stats[DP_STAT_EXACT_HIT], stats[DP_STAT_MASKED_HIT],
   stats[DP_STAT_MASKED_HIT] > 0
   ? (1.0*stats[DP_STAT_LOOKUP_HIT])/stats[DP_STAT_MASKED_HIT]
@@ -1937,21 +1944,25 @@ emc_entry_alive(struct emc_entry *ce)
 }
 
 static void
-emc_clear_entry(struct emc_entry *ce)
+emc_clear_entry(struct emc_cache *flow_cache, struct emc_entry *ce)
 {
 if (ce->flow) {
 dp_netdev_flow_unref(ce->flow);
 ce->flow = NULL;
+flow_cache->n_entries--;
 }
 }
 
 static inline void
-emc_change_entry(struct emc_entry *ce, struct dp_netdev_flow *flow,
+emc_change_entry(struct emc_cache *flow_cache, struct emc_entry *ce,
+ struct dp_netdev_flow *flow,
  const struct netdev_flow_key *key)
 {
 if (ce->flow != flow) {
 if (ce->flow) {
 dp_netdev_flow_unref(ce->flow);
+} else {
+flow_cache->n_entries++;
 }
 
 if (dp_netdev_flow_ref(flow)) {
@@ -1975,7 +1986,7 @@ emc_insert(struct emc_cache *cache, const struct 
netdev_flow_key *key,
 EMC_FOR_EACH_POS_WITH_HASH(cache, current_entry, key->hash) {
 if (netdev_flow_key_equal(_entry->key, key)) {
 /* We found the entry with the 'mf' miniflow */
-emc_change_entry(current_entry, flow, NULL);
+emc_change_entry(cache, current_entry, flow, NULL);
 return;
 }
 
@@ -1991,7 +2002,7 @@ emc_insert(struct emc_cache *cache, const struct 
netdev_flow_key *key,
 /* We didn't find the miniflow in the cache.
  * The 'to_be_replaced' entry is where the new flow will be stored */
 
-emc_change_entry(to_be_replaced, flow, key);
+emc_change_entry(cache, to_be_replaced, flow, key);
 }
 
 static inline struct dp_netdev_flow *
-- 
2.4.3

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

[ovs-dev] [PATCH] dpif-netdev: Change definitions of 'idle' & 'processing' cycles

2017-01-12 Thread Ciara Loftus 
Instead of counting all polling cycles as processing cycles, only count
the cycles where packets were received from the polling.

Signed-off-by: Georg Schmuecking 
Signed-off-by: Ciara Loftus 
Co-authored-by: Ciara Loftus 
---
 lib/dpif-netdev.c | 55 +++
 1 file changed, 43 insertions(+), 12 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 546a1e9..da6f8fe 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -272,7 +272,10 @@ enum dp_stat_type {
 
 enum pmd_cycles_counter_type {
 PMD_CYCLES_POLLING, /* Cycles spent polling NICs. */
-PMD_CYCLES_PROCESSING,  /* Cycles spent processing packets */
+PMD_CYCLES_IDLE,/* Cycles spent idle or unsuccessful polling */
+PMD_CYCLES_PROCESSING,  /* Cycles spent successfully polling and
+ * processing polled packets */
+
 PMD_N_CYCLES
 };
 
@@ -731,10 +734,10 @@ pmd_info_show_stats(struct ds *reply,
 }
 
 ds_put_format(reply,
-  "\tpolling cycles:%"PRIu64" (%.02f%%)\n"
+  "\tidle cycles:%"PRIu64" (%.02f%%)\n"
   "\tprocessing cycles:%"PRIu64" (%.02f%%)\n",
-  cycles[PMD_CYCLES_POLLING],
-  cycles[PMD_CYCLES_POLLING] / (double)total_cycles * 100,
+  cycles[PMD_CYCLES_IDLE],
+  cycles[PMD_CYCLES_IDLE] / (double)total_cycles * 100,
   cycles[PMD_CYCLES_PROCESSING],
   cycles[PMD_CYCLES_PROCESSING] / (double)total_cycles * 100);
 
@@ -2873,30 +2876,43 @@ cycles_count_end(struct dp_netdev_pmd_thread *pmd,
 non_atomic_ullong_add(>cycles.n[type], interval);
 }
 
-static void
+/* Calculate the intermediate cycle result and add to the counter 'type' */
+static inline void
+cycles_count_intermediate(struct dp_netdev_pmd_thread *pmd,
+  enum pmd_cycles_counter_type type)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+unsigned long long new_cycles = cycles_counter();
+unsigned long long interval = new_cycles - pmd->last_cycles;
+pmd->last_cycles = new_cycles;
+
+non_atomic_ullong_add(>cycles.n[type], interval);
+}
+
+static int
 dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread *pmd,
struct dp_netdev_port *port,
struct netdev_rxq *rxq)
 {
 struct dp_packet_batch batch;
 int error;
+int batch_cnt = 0;
 
 dp_packet_batch_init();
-cycles_count_start(pmd);
 error = netdev_rxq_recv(rxq, );
-cycles_count_end(pmd, PMD_CYCLES_POLLING);
 if (!error) {
 *recirc_depth_get() = 0;
 
-cycles_count_start(pmd);
+batch_cnt = batch.count;
 dp_netdev_input(pmd, , port->port_no);
-cycles_count_end(pmd, PMD_CYCLES_PROCESSING);
 } else if (error != EAGAIN && error != EOPNOTSUPP) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 
 VLOG_ERR_RL(, "error receiving data from %s: %s",
 netdev_get_name(port->netdev), ovs_strerror(error));
 }
+
+return batch_cnt;
 }
 
 static int
@@ -3006,21 +3022,26 @@ dpif_netdev_run(struct dpif *dpif)
 struct dp_netdev *dp = get_dp_netdev(dpif);
 struct dp_netdev_pmd_thread *non_pmd;
 uint64_t new_tnl_seq;
+int process_packets = 0;
 
 ovs_mutex_lock(>port_mutex);
 non_pmd = dp_netdev_get_pmd(dp, NON_PMD_CORE_ID);
 if (non_pmd) {
 ovs_mutex_lock(>non_pmd_mutex);
+cycles_count_start(non_pmd);
 HMAP_FOR_EACH (port, node, >ports) {
 if (!netdev_is_pmd(port->netdev)) {
 int i;
 
 for (i = 0; i < port->n_rxq; i++) {
-dp_netdev_process_rxq_port(non_pmd, port,
-   port->rxqs[i].rxq);
+process_packets +=
+dp_netdev_process_rxq_port(non_pmd, port,
+   port->rxqs[i].rxq);
 }
 }
 }
+cycles_count_end(non_pmd, process_packets ? PMD_CYCLES_PROCESSING
+  : PMD_CYCLES_IDLE);
 dpif_netdev_xps_revalidate_pmd(non_pmd, time_msec(), false);
 ovs_mutex_unlock(>non_pmd_mutex);
 
@@ -3131,6 +3152,7 @@ pmd_thread_main(void *f_)
 bool exiting;
 int poll_cnt;
 int i;
+int process_packets = 0;
 
 poll_list = NULL;
 
@@ -3149,9 +3171,12 @@ reload:
 netdev_rxq_get_queue_id(poll_list[i].rx));
 }
 
+cycles_count_start(pmd);
 for (;;) {
 for (i = 0; i < poll_cnt; i++) {
-dp_netdev_process_rxq_port(pmd, poll_list[i].port, 
poll_list[i].rx);
+   process_packets += dp_netdev_process_rxq_port(pmd,
+

Re: [ovs-dev] [PATCH] netdev-vport: Do not log empty warnings on success.

2017-01-12 Thread Fischetti, Antonio 
Hi Daniele,
I've checked that without this patch I was getting
ERROR: 2316 tests were run,
75 failed unexpectedly.
2 tests were skipped.

Instead after applying this patch I get
ERROR: 2316 tests were run,
42 failed unexpectedly.
2 tests were skipped.


In particular, after I apply this patch the following tunnel tests are not 
failing anymore.

===
tunnel

749: tunnel - input  ok
750: tunnel - ECN decapsulation  ok
751: tunnel - output ok
752: tunnel - unencrypted tunnel and not setting skb_mark ok
753: tunnel - unencrypted tunnel and setting skb_mark to 1 ok
754: tunnel - unencrypted tunnel and setting skb_mark to 2 ok
755: tunnel - ToS and TTL inheritanceok
756: tunnel - set_tunnel ok
757: tunnel - keyok
758: tunnel - key match  ok
759: tunnel - Geneve ok
760: tunnel - VXLAN  ok
761: tunnel - LISP   ok
762: tunnel - different VXLAN UDP port   ok
763: ofproto-dpif - set_field - tun_src/tun_dst/tun_id ok
764: tunnel - Geneve metadataok
765: tunnel - Geneve option present  ok
766: tunnel - concomitant IPv6 and IPv4 tunnels  ok

tunnel_push_pop

767: tunnel_push_pop - actionok
768: tunnel_push_pop - packet_outok

tunnel_push_pop_ipv6

769: tunnel_push_pop_ipv6 - action   ok

1093: ofproto-dpif - truncate and output to gre tunnel ok

1097: ofproto-dpif - sFlow packet sampling - tunnel set ok
1098: ofproto-dpif - sFlow packet sampling - tunnel push ok

1107: ofproto-dpif - Flow IPFIX sanity check - tunnel set ok

1147: ofproto-dpif megaflow - tunnels ok

1154: ofproto-dpif - ofproto-dpif-monitor 1   ok
1155: ofproto-dpif - ofproto-dpif-monitor 2   ok

===

One further comment, this patch fails with ./utilities/checkpatch.py.
E: No signatures found.
Warnings: 0, Errors: 1

Besides this, it looks ok to me.

Acked-by: Antonio Fischetti 


> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Daniele Di Proietto
> Sent: Thursday, January 12, 2017 8:24 AM
> To: d...@openvswitch.org
> Cc: Daniele Di Proietto 
> Subject: [ovs-dev] [PATCH] netdev-vport: Do not log empty warnings on
> success.
> 
> set_tunnel_config() always logs a warning, even on success. This
> shouldn't happen.
> 
> Without this, some unit tests fail.
> 
> Fixes: 9fff138ec3a6("netdev: Add 'errp' to set_config().")
> Signed-off-by: Daniele Di Proietto 
> ---
>  lib/netdev-vport.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index ad5ffcc81..2db51df72 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -561,10 +561,12 @@ set_tunnel_config(struct netdev *dev_, const struct
> smap *args, char **errp)
>  err = 0;
> 
>  out:
> -ds_chomp(, '\n');
> -VLOG_WARN("%s", ds_cstr());
> -if (err) {
> -*errp = ds_steal_cstr();
> +if (*ds_cstr()) {
> +ds_chomp(, '\n');
> +VLOG_WARN("%s", ds_cstr());
> +if (err) {
> +*errp = ds_steal_cstr();
> +}
>  }
> 
>  ds_destroy();
> --
> 2.11.0
> 
> ___
> 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] [PATCH] netdev-vport: Do not log empty warnings on success.

2017-01-12 Thread Daniele Di Proietto 
set_tunnel_config() always logs a warning, even on success. This
shouldn't happen.

Without this, some unit tests fail.

Fixes: 9fff138ec3a6("netdev: Add 'errp' to set_config().")
Signed-off-by: Daniele Di Proietto 
---
 lib/netdev-vport.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index ad5ffcc81..2db51df72 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -561,10 +561,12 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args, char **errp)
 err = 0;
 
 out:
-ds_chomp(, '\n');
-VLOG_WARN("%s", ds_cstr());
-if (err) {
-*errp = ds_steal_cstr();
+if (*ds_cstr()) {
+ds_chomp(, '\n');
+VLOG_WARN("%s", ds_cstr());
+if (err) {
+*errp = ds_steal_cstr();
+}
 }
 
 ds_destroy();
-- 
2.11.0

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