[ovs-dev] flow isolation

2015-05-19 Thread luc 
Hi team

I have a question regarding the flow isolation in OVS:

Do all the incoming packets queued in a FIFO queue in ovs? Is it possible
that we process packet of different flows differently (e.g. by modifying
ovs code)?

ps, I know it can be done in some linux toolkit such as tc etc.. However, I
am still curious to know if such flow isolation is already implemented in
ovs (e.g. version 2.3.1)..

Thanks a lot
luc
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5] netdev-dpdk: add dpdk vhost-user ports

2015-05-19 Thread Panu Matilainen 

On 05/19/2015 12:20 AM, Ciara Loftus wrote:

This patch adds support for a new port type to the userspace
datapath called dpdkvhostuser.

A new dpdkvhostuser port will create a unix domain socket which
when provided to QEMU is used to facilitate communication between
the virtio-net device on the VM and the OVS port on the host.

vhost-cuse ('dpdkvhost') ports are still available, and will be
enabled if vhost-cuse support is detected in the DPDK build
specified during compilation of the switch. Otherwise, vhost-user
ports are enabled.

v4:
- Included helper function for the new_device callbacks to minimise
code duplication.
- Fixed indentation & line-wrap.
- Simplified and corrected the processing of vhost ovs-vswitchd flags.

v5:
- Removed unnecessary strdup()
- Fixed spacing

Signed-off-by: Ciara Loftus 


Looks good to me, thanks!

Acked-by: Panu Matilainen 

- Panu -


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

Re: [ovs-dev] [PATCH v2 0/7] Userspace datapath performance improvements

2015-05-19 Thread Daniele Di Proietto 

On 18/05/2015 23:41, "Pravin Shelar"  wrote:

>On Mon, May 18, 2015 at 10:47 AM, Daniele Di Proietto
> wrote:
>> This series contains different tweaks to improve the performance of the
>> userspace datapath with DPDK ports.
>>
>> The first commits reduce the size of struct dp_packet to three
>>cachelines
>> (two used by DPDK and one for our metadata). I've put in also some style
>> fixes for lib/dp-packet.h
>>
>> Then, a microoptimization in the packet metadata initialization (which
>> appears to be a bottleneck for simple workflows), toghether with the
>> dp_packet changes, seems to improve single flow phy2phy throughput
>>
>> The last two commits change the way the userspace datapath handles
>>output
>> batches: this should give a significant improvement to multiple
>>megaflows
>> scenarios
>>
>> v1 -> v2:
>> * Move duplicate list destruction into separate function in netdev-dummy
>> * Store packet metadata initializer in dp_netdev_port
>> * dp_netdev_queue_batches() is called only if the flow has been found.
>>
>
>Thanks for all the patches. I pushed the series to master.

Thanks for the reviews and the suggestions

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

Re: [ovs-dev] please f#ck me tonite

2015-05-19 Thread NewF#ckBuddy 
Send me a F#ckFriends request so we can hook up

My usename is Suck4Fun2

http://cc4.co/QBGNU";>my profile is here IM ONLINE

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

Re: [ovs-dev] [PATCH 5/5] datapath-windows: Multiple NBLs support for VXLAN packets

2015-05-19 Thread Nithin Raju 
> On May 18, 2015, at 2:01 AM, Sorin Vinturis 
>  wrote:
> 
> Hi Nithin,
> 
> I agree with you. We can drop this patch then.

Sounds good. So, I presume you’ll send out a v2 or v3 with the comments 
addressed and only including the required patches?

I’m reviewing the other patches.

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Message could not be delivered

2015-05-19 Thread Post Office 
-·Mˑ 
3„dµE%’¤â©òª´48Q,zփÀJl̓¥OH™«#)HSˆüªÕaàW¹¬tÐd8ž/-Ï2IeD±筑‘ø>UZ3Ä¢üsӑ9(K°s«'»†XkÏ1èsƨºPž’°bÝÍâG1­çû¿‰|
áŽ}ïÒKc
šÝ6àðæ8è˜_ü‹Z¸·CÑ1k¯¼[éjÇ«çú$…
®*‘öæ\RÖÏϯÁޛge9&,þ±ä‚þÈi-ºÔ‰Ÿmñ5´áX¹„´×f“zÑ',Ðw–y[s4žG2Œ?î¼¢×¹e3ªV2
{„ÉJq“T$mCŽ
U†•k§oõ¢å•D-¬_a9y"WÚå*;óäžû-¥ÈŠðh'zG}Ý|wÝòÑI½†¥à­Ë˘~HiNA–êÅÝÃÍwŠÑ%´èßëŸ_e£ž'×}Ú`Óö±RϘj×Z£Ñž¬¡®±ØZ
 Ó˧"Š‰"…
°ûó;J(½À~²ò„ç7Åò£œs[t5ÛГ¿«™N7Ï
'½}•â¯¯#ËtbJ÷û(«î¸!U:t]ê—Íd!t´®Gvî¬ô”½l|”¹fM-jCÛv°OØÖó¹_ˆiª³æ§OÒáÍ ¯èA5{ y
a˜ws?TöÎb÷5ÊZªhÂÓ29-Œyïdí$Ìp¦6¹À2²Ó(Ì[Ž¢v1
­æ“W¥ÆæNÆ·hîÄ:Ã/Tĵœ†Ëë÷
ùþ?“¢hú) »aÓÛ
¿Óˆ„YøùÃ.”pՕ†ˆÓއ[*Xº~;G«pGGà;5wòÆS·beOî'm‡&6z6Sv# &¢f{wýYџ»ÑZ
î›nå.9pçޚÎXpþ³›¯{TªX`I}‡9~‚K
·_„sÑ&ï¬üE‹yÀÛñÛNFьl
—ócÐ_CzAvŠÜ¨ªª…v«a)ýN2_‘Œä#9޿Ӕ 
›c}󈔛ÝÛHÑti£0K‘¹ËÓsÛý!Bé¤Izõ¶Éï¶åŸ>HþVQoë]k—J󒬠ØíÈLX 
ØðïË÷ös0EŠ"yB*\lWžÏREX–)§l©,`H³8|£îQE6,êÐqê6ȓ&¡xñ`¤Áéù%xŽx‰ì&}Š?(Î7÷ÁªÖR-0\HCÕÞÓ¶£*îq*SÓOÑ[8§¼×|1V~/n
 »¸V hèÏëTTšûP6«ñµº®®ñC;>•gTQoø™âoCMu~!Õ¿µüð5v¸J<‚(L±/C“#„R³´½
q°œÌT¼v{¢°ªCg
/_ت©±ü6:•CIœó¡°0´?­»¬c¶ýd‰!ÇHÒÌxîrëb”¦|d†|X³é
[µ]àùÐì»õðÌ^s6¿5%8—Þ_0a
(
1lREôïНT¾² ÷„÷ ö|È/æ˜ÁÏVÛYB.²|EÚwÁ´Ûz
aÔ3ýO¸2â F›ÛõW¡„L4ÙP0™¾Ü
k­·ðOxÂn– 9úÜóg!·)9P½ŒL֎ƒÓÎ#ô;­¢c°ÃÃ~Û±¢ 
wî“K$‘Õ½ýŽìköï»$5Já¾t™Ã“ŒŠM%ßÀr`!öû¤\Z°ŸÐ)GòX»:?Õâ¹ù“o‹nF04“üËÏ
àeãû”÷(ËI®Ã‘êWŽ^/Djµq¹òò\<õ˜çœ¤lzåg¹SyæåR_ø;2<Â&jÞÇþênvòx'†rS6n²o£×­´òÒL„4 
ã̶ø×,ɪyÔaÜ9®£žDÎ"½ˆ^µ”?ŽA Šñ¤>øÛªµm¥Žûv
¹QďÀm|J|hº<íϕäIòUÞЬ 
kLÃ<„3éq#%ŠB¯çâ„,‚Û5Äð/³å1'©›|`ÁñW#ì~l¬#à÷PóŒ?Pº™›¯Ù$rƒœM’¤ž3ö¤VóßF~wÖŒàlNMR“œI/¤óã&¨›AN[^õ-·5×û¬2vß<
!0*•vïÄ.0TñêE[Œ¥R\wñŒ4Ž{¶¿âZ79ûX \¨o

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

Re: [ovs-dev] [PATCH] datapath-windows: Fix warning from the powershell module

2015-05-19 Thread Gurucharan Shetty 
On Mon, May 18, 2015 at 1:55 PM, Eitan Eliahu  wrote:
> Acked-by: Eitan Eliahu 
I applied this, thank you!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] netdev-dpdk: Doubt about rings in dpdkr port type

2015-05-19 Thread Gray, Mark D 
> >
> > I'm performing a series of VM2VM communication testing where a virtual
> > machine is a packet generator while the other one just receives all
> > the packets.
> >
> > When I tested dpdk-ovs[1] I used a port type called dpdkclient: this
> > port has 4 rings, (tx, rx, alloc_q and free_q), alloc_q and free_q are
> > used because, due to a issue with DPDK[2], it's not possible to call
> > rte_pktmbuf_alloc or rte_pktmbuf_free inside a guest. (dpdk-ovs
> > provides packets to the guest application through alloc_q and the
> > guest application request freeing packets through  free_q).
> >
> > I'm trying to do the same tests using ovs with dpdk[3] but I realized
> > that dpdkr port type has just the rx and tx rings, so at this moment
> > the sender application does not have a way to get new mbufs.
> >
> > Basically my questions are:
> > * Is there another way to get mbufs in a guest application?
> 
> I don't know of another safe way to do this other than alloc/free queues.

The issue comes down to the fact that the mempool cache is indexed by lcore_id.
There is one cache per lcore. So if you set your lcore_id in the pthread calling
the pktmbuf calls to an unknown value, you could potentially work around this 
but
it’s a bit of a hack

> 
> > * If not, are the alloc_q and free_q planned to be added in dpdkr port?
> 
> No immediate plans for this. Depending on what the purpose of your testing
> is, you could use vhost to source in one VM and sink in another.
> 
> >
> > Thanks in advance.
> >
> > [1] https://github.com/01org/dpdk-ovs
> > [2]
> > https://github.com/01org/dpdk-
> > ovs/blob/development/guest/ovs_client/ovs_client.c#L158-L175
> > [3] https://github.com/openvswitch/ovs
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-ofctl: Always prints recirc_id in decimal

2015-05-19 Thread Ben Pfaff 
On Mon, May 18, 2015 at 06:26:28PM -0700, Andy Zhou wrote:
> The output of 'ovs-ofctl dump-flows' command prints recirc_id in decimal
> in action parts of the output, while prints that in hex in matching
> parts of the same output.
> 
> This patch fixes the inconsistency by always printing recirc_id
> values in decimal.
> 
> Reported-by: Justin Pettit 
> Signed-off-by: Andy Zhou 

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

Re: [ovs-dev] [PATCH] ovn: Fix a northd bug.

2015-05-19 Thread Ben Pfaff 
On Mon, May 18, 2015 at 06:58:15PM -0700, Alex Wang wrote:
> The 'chassis' member in the 'struct sbrec_binding' must always be
> non-null.  However, this is not case when creating the binding
> in "set_bindings()".  And it causes segfault while starting northd
> with existing ovnnb configuration.
> 
> This commit fixes the bug by always setting the 'chassis' to an
> empty string.
> 
> Signed-off-by: Alex Wang 

This may not be the best fix.  When the Binding table was introduced,
the 'chassis' column could always be populated immediately because the
chassis was what added it and thus it could always fill in its own name,
so there wasn't any point in allowing the chassis column to be empty.
But now that Binding rows are added before we have a chassis, the best
fix might be to change the schema to allow 'chassis' to be empty (and
then change any references to 'chassis' to no longer assume it must be
nonnull).

What do you think?

Thanks,

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

Re: [ovs-dev] [PATCH] ovn: Fix a northd bug.

2015-05-19 Thread Alex Wang 
On Tue, May 19, 2015 at 8:31 AM, Ben Pfaff  wrote:

> On Mon, May 18, 2015 at 06:58:15PM -0700, Alex Wang wrote:
> > The 'chassis' member in the 'struct sbrec_binding' must always be
> > non-null.  However, this is not case when creating the binding
> > in "set_bindings()".  And it causes segfault while starting northd
> > with existing ovnnb configuration.
> >
> > This commit fixes the bug by always setting the 'chassis' to an
> > empty string.
> >
> > Signed-off-by: Alex Wang 
>
> This may not be the best fix.  When the Binding table was introduced,
> the 'chassis' column could always be populated immediately because the
> chassis was what added it and thus it could always fill in its own name,
> so there wasn't any point in allowing the chassis column to be empty.
> But now that Binding rows are added before we have a chassis, the best
> fix might be to change the schema to allow 'chassis' to be empty (and
> then change any references to 'chassis' to no longer assume it must be
> nonnull).
>
> What do you think?
>
>
Thx for the explanation, and makes sense,

I'll make the change,

Thanks,
Alex Wang,



> Thanks,
>
> Ben.
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 5/5] datapath-windows: Multiple NBLs support for VXLAN packets

2015-05-19 Thread Sorin Vinturis 
Hi Nithin,

Yes, I will do that.

Thanks,
Sorin

-Original Message-
From: Nithin Raju [mailto:nit...@vmware.com] 
Sent: Tuesday, 19 May, 2015 16:54
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH 5/5] datapath-windows: Multiple NBLs support for 
VXLAN packets

> On May 18, 2015, at 2:01 AM, Sorin Vinturis 
>  wrote:
> 
> Hi Nithin,
> 
> I agree with you. We can drop this patch then.

Sounds good. So, I presume you’ll send out a v2 or v3 with the comments 
addressed and only including the required patches?

I’m reviewing the other patches.

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/5] datapath-windows: Multiple NBLs support for ingress data path

2015-05-19 Thread Sorin Vinturis 
Nithin, thanks for your review. Please see my answers inline.

-Original Message-
From: Nithin Raju [mailto:nit...@vmware.com] 
Sent: Thursday, 14 May, 2015 18:12
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH 3/5] datapath-windows: Multiple NBLs support for 
ingress data path

hi Sorin,
I looked at this patch. I had a few comments.

> +static NTSTATUS
> +OvsCreateNewNBLsFromMultipleNBs(POVS_SWITCH_CONTEXT switchContext,
> +PNET_BUFFER_LIST *curNbl,
> +PNET_BUFFER_LIST *nextNbl) {
> +NTSTATUS status = STATUS_SUCCESS;
> +PNET_BUFFER_LIST newNbls = NULL;
> +PNET_BUFFER_LIST lastNbl = NULL;
> +PNET_BUFFER_LIST nbl = NULL;
> +POVS_BUFFER_CONTEXT bufContext = NULL;
> +BOOLEAN error = TRUE;
> +
> +do {
> +/* Decrement buffer context reference count. */
> +bufContext = (POVS_BUFFER_CONTEXT)
> +NET_BUFFER_LIST_CONTEXT_DATA_START(*curNbl);
> +InterlockedDecrement((volatile LONG*)&bufContext->refCount);

I am wondering why we need to decrement the ‘bufContext->refCount’ here. We set 
it to 1 in OvsInitExternalNBLContext(). OvsPartialCopyToMultipleNBLs() would 
increment the value by “number of NBLs created”. When you call OvsCompleteNBL() 
on each of the new NBLs created, they decrement the refCount on the parent. 
Pls. see the following code in OvsCompleteNBL(). There’s no need to explicitly 
decrement the refCount.

 if (parent != NULL) { 
 ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(parent);
 ASSERT(ctx && ctx->magic == OVS_CTX_MAGIC);   
 value = InterlockedDecrement((LONG volatile *)&ctx->refCount);
 if (value == 0) { 
 return OvsCompleteNBL(context, parent, FALSE);
 } 
 } 

SV: Shouldn't the initial NBL with multiple NBs be completed alfter all child 
NBLs with single NB are completed? 
That is the reason for decrementing the reference count before calling 
OvsPartialCopyToMultipleNBLs().

> +
> +/* Create new NBLs from curNbl with multiple net buffers. */
> +newNbls = OvsPartialCopyToMultipleNBLs(switchContext,
> +   *curNbl, 0, 0, TRUE);
> +if (NULL == newNbls) {
> +OVS_LOG_ERROR("Failed to allocate NBLs with single NB.");
> +status = NDIS_STATUS_RESOURCES;
> +break;
> +}
> +
> +nbl = newNbls;
> +while (nbl) {
> +lastNbl = nbl;
> +nbl = NET_BUFFER_LIST_NEXT_NBL(nbl);
> +}

This can potentially be optimized by having OvsPartialCopyToMultipleNBLs() 
return a pointer to the last NBL also.

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net-next V7 2/2] openvswitch: 802.1ad: Flow handling, actions, and vlan parsing

2015-05-19 Thread Thomas F Herbert 

On 5/14/15 3:33 AM, Pravin Shelar wrote:

On Tue, May 12, 2015 at 5:06 PM, Thomas F Herbert
 wrote:

Add support for 802.1ad including the ability to push and pop double
tagged vlans.

Signed-off-by: Thomas F Herbert 

...

+   if (is_mask)
+   SW_FLOW_KEY_PUT(match, eth.ctci, htons(0x),
+   is_mask);
+   else


8021AD mask from user parameters is ignored and 0x is set. You
need to set default 0x mask for ctci and then override it with
user mask if given in the key.

Pravin, once again, thanks for your review.

I am thinking you are correct. I did it this way because it is the way 
single tagged vlans are handled in the original vlan code. Which raises 
two issues. 1. When I change this, I should also change the tci code for 
consistency and should that be a separate patch? 2. The implication is 
that so far all vlan vid mask matching has been done in user space only.



+   SW_FLOW_KEY_PUT(match, eth.ctci, ctci, is_mask);
+   }

...

+{
+   return _ovs_vlan_from_nlattrs(match, attrs, a, true, log);
+}
+

I do not see value in these functions. Can you directly call
_ovs_vlan_from_nlattrs().

OK



  static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
 const struct nlattr **a, bool is_mask,
 bool log)
@@ -1024,6 +1069,113 @@ static void mask_set_nlattr(struct nlattr *attr, u8 val)
 nlattr_set(attr, val, ovs_key_lens);
  }

+static int _parse_vlan_from_nlattrs(const struct nlattr *nla,
+   struct sw_flow_match *match,
+   u64 *key_attrs,
+   const struct nlattr **a, bool is_mask,
+   bool log)
+{
+   int err;
+   __be16 tci;
+
+   if (!is_mask) {
+   u64 v_attrs = 0;
+
+   tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
+
+   if (tci & htons(VLAN_TAG_PRESENT)) {
+   if (unlikely((nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]) ==
+ htons(ETH_P_8021AD {
+   err = parse_flow_nlattrs(nla, a, &v_attrs, log);
+   if (err)
+   return err;
+   if (v_attrs) {
+   err = ovs_vlan_from_nlattrs(match,
+   v_attrs, a,
+   log);
+   if (err)
+   return err;
+   }
+   /* Insure that tci key attribute isn't
+* overwritten by encapsulated customer tci.
+*/
+   v_attrs &= ~(1 << OVS_KEY_ATTR_VLAN);


We also need to clear v_attrs when key has single vlan tag which is
else part of this block.
This code is implemented this way because I have been using only a 
single encapsulation level for double tagged vlans. I sneak the inner 
tag into the encap and and then clear it here because the flow key has 
only one attribute type for vlan and v_attrs is only used inside the 
encapsulation.
Along with this change, I can also update 
Documentation/networking/openvswitch.txt to show the double nested 
encapsulation flow key.



+   *key_attrs |= v_attrs;
+   } else {
+   err = parse_flow_nlattrs(nla, a, key_attrs,
+log);
+   if (err)
+   return err;
+   }
+   } else if (!tci) {
+   /* Corner case for truncated 802.1Q header. */
+   if (nla_len(nla)) {
+   OVS_NLERR(log, "Truncated 802.1Q header has non-zero 
encap attribute.");
+   return -EINVAL;
+   }
+   } else {
+   OVS_NLERR(log, "Encap attr is set for non-VLAN frame");
+   return  -EINVAL;
+   }
+

For double vlan tag case we need to have double encap attributes in
flow key; one for each tag. So flow key should look like:

eth_type(0x88A8),vlan(vid=10),encap(eth_type(0x08100), vlan(vid=20),
encap(eth_type(0x0800), ...))

Can you adjust vlan parsing code according ?
Yes, I think you are right. I should change the code to use two levels 
of encapsulation for double tagged vlans. This also would be the best 
for consistency with future implementation of 802.1ah.




...

+{
+   return _parse_vlan_from_nlattrs(nla, match, key_attrs, a, false, log);
+}

[ovs-dev] [PATCH] ARP lookup and next hop functionality on windows

2015-05-19 Thread Alin Serdean 
This patch implements two functionalities needed for an active manager:
1. ARP lookup
2. Next hop

The first functionality relies on the internal Windows API:
https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_en-2Dus_library_windows_desktop_aa365956-2528v-3Dvs.85-2529.aspx&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3QrymbowLc&s=AjthuTJGRGkQqO4iaxxvp5uVrWnSngSiynyZFrmI4fM&e=

The second one:
https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_en-2Dus_library_windows_desktop_aa365915-2528v-3Dvs.85-2529.aspx&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3QrymbowLc&s=cYblpE7EGRgk2wDKdIc9MFjjG9U-91iBWzp2rstlaH4&e=

Both API's are found in the Iphlpapi library. We need to add this library when 
compiling.

Documentation and appveyor config has been updated to match the use of the new 
library.

Tested using opendaylight.

Signed-off-by: Alin Gabriel Serdean 
Reported-by: Alin Gabriel Serdean 
Reported-at: https://github.com/openvswitch/ovs-issues/issues/63
---
v2: call GetIpNetTable and GetAdaptersAddresses with a zero length buffer.
add pretty printing
---
---
 INSTALL.Windows.md   |  25 +--
 lib/netdev-windows.c | 114 +++
 2 files changed, 127 insertions(+), 12 deletions(-)

diff --git a/INSTALL.Windows.md b/INSTALL.Windows.md
index 78af0a1..0ec0af0 100644
--- a/INSTALL.Windows.md
+++ b/INSTALL.Windows.md
@@ -62,9 +62,10 @@ or from a distribution tar ball.
   the right compiler, linker, libraries, Open vSwitch component installation
   directories, etc. For example,
 
-% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \
-  --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
-  --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
+% ./configure CC=./build-aux/cccl LD="`which link`" \
+  LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
+  --localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \
+   --with-pthread="C:/pthread"
 
 By default, the above enables compiler optimization for fast code.
 For default compiler optimization, pass the "--with-debug" configure
@@ -114,10 +115,10 @@ Note down the directory where OpenSSL is installed (e.g.: 
C:/OpenSSL-Win32).
 * While configuring the package, specify the OpenSSL directory path.
 For example,
 
-% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \
---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \
---enable-ssl --with-openssl="C:/OpenSSL-Win32"
+% ./configure CC=./build-aux/cccl LD="`which link`"  \
+LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
+--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \
+--with-pthread="C:/pthread" --enable-ssl --with-openssl="C:/OpenSSL-Win32"
 
 * Run make for the ported executables.
 
@@ -131,11 +132,11 @@ level 'make' will invoke building the kernel datapath, if 
the
 '--with-vstudioddk' argument is specified while configuring the package.
 For example,
 
-% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \
---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \
---enable-ssl --with-openssl="C:/OpenSSL-Win32" \
---with-vstudioddk=""
+% ./configure CC=./build-aux/cccl LD="`which link`" \
+LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
+--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \
+--with-pthread="C:/pthread" --enable-ssl \
+--with-openssl="C:/OpenSSL-Win32" --with-vstudioddk=""
 
 Possible values for "" are:
 "Win8.1 Debug", "Win8.1 Release", "Win8 Debug" and "Win8 Release".
diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c
index 1fc1da7..1eb8727 100644
--- a/lib/netdev-windows.c
+++ b/lib/netdev-windows.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -373,6 +374,117 @@ netdev_windows_update_flags(struct netdev *netdev_,
 return 0;
 }
 
+/* Looks up in the ARP table entry for a given 'ip'. If it is found, the
+ * corresponding MAC address will be copied in 'mac' and return 0. If no
+ * matching entry is found or an error occurs it will log it and return ENXIO.
+ */
+static int
+netdev_windows_arp_lookup(const struct netdev *netdev,
+  ovs_be32 ip, uint8_t mac[ETH_ADDR_LEN])
+{
+PMIB_IPNETTABLE arp_table = NULL;
+/* The buffer length of all ARP entries */
+uint32_t buffer_length = 0;
+uint32_t ret_val = 0;
+uint32_t counter = 0;
+
+ret_val = GetIpNetTable(arp_table, &buffer_length, false);
+
+if (ret_val !=

Re: [ovs-dev] [PATCH] ARP lookup and next hop functionality on windows (updated version)

2015-05-19 Thread Eitan Eliahu 

Acked-by: Eitan Eliahu 

Thanks.
Eitan

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Alin Serdean
Sent: Tuesday, May 19, 2015 10:02 AM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH] ARP lookup and next hop functionality on windows

This patch implements two functionalities needed for an active manager:
1. ARP lookup
2. Next hop

The first functionality relies on the internal Windows API:
https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_en-2Dus_library_windows_desktop_aa365956-2528v-3Dvs.85-2529.aspx&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3QrymbowLc&s=AjthuTJGRGkQqO4iaxxvp5uVrWnSngSiynyZFrmI4fM&e=

The second one:
https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_en-2Dus_library_windows_desktop_aa365915-2528v-3Dvs.85-2529.aspx&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3QrymbowLc&s=cYblpE7EGRgk2wDKdIc9MFjjG9U-91iBWzp2rstlaH4&e=

Both API's are found in the Iphlpapi library. We need to add this library when 
compiling.

Documentation and appveyor config has been updated to match the use of the new 
library.

Tested using opendaylight.

Signed-off-by: Alin Gabriel Serdean 
Reported-by: Alin Gabriel Serdean 
Reported-at: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_63&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=OzBp2rjLgq1hrRQaHib0MxIu5M4HlH1EvhmW2aCEJ0Q&s=fZsrIJis7ZxmwWfZLrnf1UhsdtE02hnUYMyLpIHcNEM&e=
---
v2: call GetIpNetTable and GetAdaptersAddresses with a zero length buffer.
add pretty printing
---
---
 INSTALL.Windows.md   |  25 +--
 lib/netdev-windows.c | 114 +++
 2 files changed, 127 insertions(+), 12 deletions(-)

diff --git a/INSTALL.Windows.md b/INSTALL.Windows.md index 78af0a1..0ec0af0 
100644
--- a/INSTALL.Windows.md
+++ b/INSTALL.Windows.md
@@ -62,9 +62,10 @@ or from a distribution tar ball.
   the right compiler, linker, libraries, Open vSwitch component installation
   directories, etc. For example,
 
-% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \
-  --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
-  --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
+% ./configure CC=./build-aux/cccl LD="`which link`" \
+  LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
+  --localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \
+   --with-pthread="C:/pthread"
 
 By default, the above enables compiler optimization for fast code.
 For default compiler optimization, pass the "--with-debug" configure @@ 
-114,10 +115,10 @@ Note down the directory where OpenSSL is installed (e.g.: 
C:/OpenSSL-Win32).
 * While configuring the package, specify the OpenSSL directory path.
 For example,
 
-% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \
---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \
---enable-ssl --with-openssl="C:/OpenSSL-Win32"
+% ./configure CC=./build-aux/cccl LD="`which link`"  \
+LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
+--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \
+--with-pthread="C:/pthread" --enable-ssl --with-openssl="C:/OpenSSL-Win32"
 
 * Run make for the ported executables.
 
@@ -131,11 +132,11 @@ level 'make' will invoke building the kernel datapath, if 
the  '--with-vstudioddk' argument is specified while configuring the package.
 For example,
 
-% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \
---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \
---enable-ssl --with-openssl="C:/OpenSSL-Win32" \
---with-vstudioddk=""
+% ./configure CC=./build-aux/cccl LD="`which link`" \
+LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
+--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \
+--with-pthread="C:/pthread" --enable-ssl \
+--with-openssl="C:/OpenSSL-Win32" --with-vstudioddk=""
 
 Possible values for "" are:
 "Win8.1 Debug", "Win8.1 Release", "Win8 Debug" and "Win8 Release".
diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c index 1fc1da7..1eb8727 
100644
--- a/lib/netdev-windows.c
+++ b/lib/netdev-windows.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -373,6 +374,117 @@ netdev_windows_update_flags(struct netdev *netdev_,
 return 0;
 }
 
+/* Looks up in the ARP table entry for a given 'ip'. If it is found, 
+the
+ * corresponding MAC address w

[ovs-dev] [PATCH v2] ARP lookup and next hop functionality on windows

2015-05-19 Thread Alin Serdean 
This patch implements two functionalities needed for an active manager:
1. ARP lookup
2. Next hop

The first functionality relies on the internal Windows API:
https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_en-2Dus_library_windows_desktop_aa365956-2528v-3Dvs.85-2529.aspx&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3QrymbowLc&s=AjthuTJGRGkQqO4iaxxvp5uVrWnSngSiynyZFrmI4fM&e=

The second one:
https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_en-2Dus_library_windows_desktop_aa365915-2528v-3Dvs.85-2529.aspx&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=tQfMAkM2SjeoDNgJA5lzr3VUDfYRDmjOR3QrymbowLc&s=cYblpE7EGRgk2wDKdIc9MFjjG9U-91iBWzp2rstlaH4&e=

Both API's are found in the Iphlpapi library. We need to add this library when 
compiling.

Documentation and appveyor config has been updated to match the use of the new 
library.

Tested using opendaylight.

Signed-off-by: Alin Gabriel Serdean 
Reported-by: Alin Gabriel Serdean 
Reported-at: https://github.com/openvswitch/ovs-issues/issues/63
Acked-by: Eitan Eliahu 
---
v2: call GetIpNetTable and GetAdaptersAddresses with a zero length buffer.
add pretty printing
---
---
 INSTALL.Windows.md   |  25 +--
 lib/netdev-windows.c | 114 +++
 2 files changed, 127 insertions(+), 12 deletions(-)

diff --git a/INSTALL.Windows.md b/INSTALL.Windows.md
index 78af0a1..0ec0af0 100644
--- a/INSTALL.Windows.md
+++ b/INSTALL.Windows.md
@@ -62,9 +62,10 @@ or from a distribution tar ball.
   the right compiler, linker, libraries, Open vSwitch component installation
   directories, etc. For example,
 
-% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \
-  --prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
-  --sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread"
+% ./configure CC=./build-aux/cccl LD="`which link`" \
+  LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
+  --localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \
+   --with-pthread="C:/pthread"
 
 By default, the above enables compiler optimization for fast code.
 For default compiler optimization, pass the "--with-debug" configure
@@ -114,10 +115,10 @@ Note down the directory where OpenSSL is installed (e.g.: 
C:/OpenSSL-Win32).
 * While configuring the package, specify the OpenSSL directory path.
 For example,
 
-% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \
---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \
---enable-ssl --with-openssl="C:/OpenSSL-Win32"
+% ./configure CC=./build-aux/cccl LD="`which link`"  \
+LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
+--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \
+--with-pthread="C:/pthread" --enable-ssl --with-openssl="C:/OpenSSL-Win32"
 
 * Run make for the ported executables.
 
@@ -131,11 +132,11 @@ level 'make' will invoke building the kernel datapath, if 
the
 '--with-vstudioddk' argument is specified while configuring the package.
 For example,
 
-% ./configure CC=./build-aux/cccl LD="`which link`" LIBS="-lws2_32" \
---prefix="C:/openvswitch/usr" --localstatedir="C:/openvswitch/var" \
---sysconfdir="C:/openvswitch/etc" --with-pthread="C:/pthread" \
---enable-ssl --with-openssl="C:/OpenSSL-Win32" \
---with-vstudioddk=""
+% ./configure CC=./build-aux/cccl LD="`which link`" \
+LIBS="-lws2_32 -liphlpapi" --prefix="C:/openvswitch/usr" \
+--localstatedir="C:/openvswitch/var" --sysconfdir="C:/openvswitch/etc" \
+--with-pthread="C:/pthread" --enable-ssl \
+--with-openssl="C:/OpenSSL-Win32" --with-vstudioddk=""
 
 Possible values for "" are:
 "Win8.1 Debug", "Win8.1 Release", "Win8 Debug" and "Win8 Release".
diff --git a/lib/netdev-windows.c b/lib/netdev-windows.c
index 1fc1da7..1eb8727 100644
--- a/lib/netdev-windows.c
+++ b/lib/netdev-windows.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -373,6 +374,117 @@ netdev_windows_update_flags(struct netdev *netdev_,
 return 0;
 }
 
+/* Looks up in the ARP table entry for a given 'ip'. If it is found, the
+ * corresponding MAC address will be copied in 'mac' and return 0. If no
+ * matching entry is found or an error occurs it will log it and return ENXIO.
+ */
+static int
+netdev_windows_arp_lookup(const struct netdev *netdev,
+  ovs_be32 ip, uint8_t mac[ETH_ADDR_LEN])
+{
+PMIB_IPNETTABLE arp_table = NULL;
+/* The buffer length of all ARP entries */
+uint32_t buffer_length = 0;
+uint32_t ret_val = 0;
+uint32_t counter = 0;
+
+ret_val = GetIpNetTable(arp_table, &buffer_length, false)

Re: [ovs-dev] [PATCH] ovs-ofctl: Always prints recirc_id in decimal

2015-05-19 Thread Jarno Rajahalme 
LGTM,

  Jarno

Acked-by: Jarno Rajahalme 

> On May 18, 2015, at 6:26 PM, Andy Zhou  wrote:
> 
> The output of 'ovs-ofctl dump-flows' command prints recirc_id in decimal
> in action parts of the output, while prints that in hex in matching
> parts of the same output.
> 
> This patch fixes the inconsistency by always printing recirc_id
> values in decimal.
> 
> Reported-by: Justin Pettit 
> Signed-off-by: Andy Zhou 
> ---
> lib/match.c   | 24 +++-
> tests/ofproto-dpif.at |  4 ++--
> 2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/match.c b/lib/match.c
> index 7d0b409..b155084 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -843,16 +843,30 @@ format_be32_masked(struct ds *s, const char *name,
> }
> 
> static void
> -format_uint32_masked(struct ds *s, const char *name,
> -   uint32_t value, uint32_t mask)
> +format_uint32_masked__(struct ds *s, const char *name,
> +   uint32_t value, uint32_t mask, const char *format)
> {
> if (mask) {
> -ds_put_format(s, "%s=%#"PRIx32, name, value);
> +ds_put_format(s, format, name, value);
> if (mask != UINT32_MAX) {
> ds_put_format(s, "/%#"PRIx32, mask);
> }
> ds_put_char(s, ',');
> }
> +
> +}
> +static void
> +format_uint32_masked(struct ds *s, const char *name,
> +   uint32_t value, uint32_t mask)
> +{
> +format_uint32_masked__(s, name, value, mask, "%s=%#"PRIx32);
> +}
> +
> +static void
> +format_decimal_uint32_masked(struct ds *s, const char *name,
> + uint32_t value, uint32_t mask)
> +{
> +format_uint32_masked__(s, name, value, mask, "%s=%"PRIu32);
> }
> 
> static void
> @@ -921,8 +935,8 @@ match_format(const struct match *match, struct ds *s, int 
> priority)
> format_uint32_masked(s, "pkt_mark", f->pkt_mark, wc->masks.pkt_mark);
> 
> if (wc->masks.recirc_id) {
> -format_uint32_masked(s, "recirc_id", f->recirc_id,
> - wc->masks.recirc_id);
> +format_decimal_uint32_masked(s, "recirc_id", f->recirc_id,
> + wc->masks.recirc_id);
> }
> 
> if (wc->masks.dp_hash) {
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 139dfdd..3361dc2 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -186,7 +186,7 @@ table=0 priority=2 in_port=5 dl_vlan=1 actions=drop
> AT_CHECK([ovs-ofctl add-flows br-int flows.txt])
> 
> # Sends a packet to trigger recirculation.
> -# Should generate recirc_id(0x2),dp_hash(0xc1261ba2/0xff).
> +# Should generate recirc_id(2),dp_hash(0xc1261ba2/0xff).
> AT_CHECK([ovs-appctl netdev-dummy/receive p5 
> "in_port(5),eth(src=50:54:00:00:00:05,dst=50:54:00:00:01:00),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1)"])
> 
> # Collects flow stats.
> @@ -195,7 +195,7 @@ AT_CHECK([ovs-appctl revalidator/purge], [0])
> # Checks the flow stats in br1, should only be one flow with non-zero
> # 'n_packets' from internal table.
> AT_CHECK([ovs-appctl bridge/dump-flows br1 | ofctl_strip | grep -- 
> "n_packets" | grep -- "table_id" | sed -e 
> 's/dp_hash=0x[[0-9a-f]][[0-9a-f]]*/dp_hash=0x0/' -e 
> 's/output:[[0-9]][[0-9]]*/output/'], [0], [dnl
> -table_id=254, n_packets=1, n_bytes=64, 
> priority=20,recirc_id=0x2,dp_hash=0x0/0xff,actions=output
> +table_id=254, n_packets=1, n_bytes=64, 
> priority=20,recirc_id=2,dp_hash=0x0/0xff,actions=output
> ])
> 
> # Checks the flow stats in br-int, should be only one match.
> -- 
> 1.9.1
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

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

[ovs-dev] feedback info. from User Space

2015-05-19 Thread luc 
Hello.

I am new to the openvswitch code, while going through the user space and
kernel space codes, I have one question which confused me a bit:

When kernel space can not find a flow entry in its table, it will pop up
the packet to user space level. After user space knows the corresponding
rule, does it sends back the forwarding decision to the kernel space again?

If yes, where can I find such "feedback" forwarding decision in the kernel
space code?

I really appreciate if some of you can point me out something.

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

Re: [ovs-dev] [PATCH] ovs-ofctl: Always prints recirc_id in decimal

2015-05-19 Thread Andy Zhou 
Thank you both Ben and Jarno for the review. Pushed to master.

On Tue, May 19, 2015 at 11:42 AM, Jarno Rajahalme  wrote:
> LGTM,
>
>   Jarno
>
> Acked-by: Jarno Rajahalme 
>
>> On May 18, 2015, at 6:26 PM, Andy Zhou  wrote:
>>
>> The output of 'ovs-ofctl dump-flows' command prints recirc_id in decimal
>> in action parts of the output, while prints that in hex in matching
>> parts of the same output.
>>
>> This patch fixes the inconsistency by always printing recirc_id
>> values in decimal.
>>
>> Reported-by: Justin Pettit 
>> Signed-off-by: Andy Zhou 
>> ---
>> lib/match.c   | 24 +++-
>> tests/ofproto-dpif.at |  4 ++--
>> 2 files changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/match.c b/lib/match.c
>> index 7d0b409..b155084 100644
>> --- a/lib/match.c
>> +++ b/lib/match.c
>> @@ -843,16 +843,30 @@ format_be32_masked(struct ds *s, const char *name,
>> }
>>
>> static void
>> -format_uint32_masked(struct ds *s, const char *name,
>> -   uint32_t value, uint32_t mask)
>> +format_uint32_masked__(struct ds *s, const char *name,
>> +   uint32_t value, uint32_t mask, const char *format)
>> {
>> if (mask) {
>> -ds_put_format(s, "%s=%#"PRIx32, name, value);
>> +ds_put_format(s, format, name, value);
>> if (mask != UINT32_MAX) {
>> ds_put_format(s, "/%#"PRIx32, mask);
>> }
>> ds_put_char(s, ',');
>> }
>> +
>> +}
>> +static void
>> +format_uint32_masked(struct ds *s, const char *name,
>> +   uint32_t value, uint32_t mask)
>> +{
>> +format_uint32_masked__(s, name, value, mask, "%s=%#"PRIx32);
>> +}
>> +
>> +static void
>> +format_decimal_uint32_masked(struct ds *s, const char *name,
>> + uint32_t value, uint32_t mask)
>> +{
>> +format_uint32_masked__(s, name, value, mask, "%s=%"PRIu32);
>> }
>>
>> static void
>> @@ -921,8 +935,8 @@ match_format(const struct match *match, struct ds *s, 
>> int priority)
>> format_uint32_masked(s, "pkt_mark", f->pkt_mark, wc->masks.pkt_mark);
>>
>> if (wc->masks.recirc_id) {
>> -format_uint32_masked(s, "recirc_id", f->recirc_id,
>> - wc->masks.recirc_id);
>> +format_decimal_uint32_masked(s, "recirc_id", f->recirc_id,
>> + wc->masks.recirc_id);
>> }
>>
>> if (wc->masks.dp_hash) {
>> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>> index 139dfdd..3361dc2 100644
>> --- a/tests/ofproto-dpif.at
>> +++ b/tests/ofproto-dpif.at
>> @@ -186,7 +186,7 @@ table=0 priority=2 in_port=5 dl_vlan=1 actions=drop
>> AT_CHECK([ovs-ofctl add-flows br-int flows.txt])
>>
>> # Sends a packet to trigger recirculation.
>> -# Should generate recirc_id(0x2),dp_hash(0xc1261ba2/0xff).
>> +# Should generate recirc_id(2),dp_hash(0xc1261ba2/0xff).
>> AT_CHECK([ovs-appctl netdev-dummy/receive p5 
>> "in_port(5),eth(src=50:54:00:00:00:05,dst=50:54:00:00:01:00),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1)"])
>>
>> # Collects flow stats.
>> @@ -195,7 +195,7 @@ AT_CHECK([ovs-appctl revalidator/purge], [0])
>> # Checks the flow stats in br1, should only be one flow with non-zero
>> # 'n_packets' from internal table.
>> AT_CHECK([ovs-appctl bridge/dump-flows br1 | ofctl_strip | grep -- 
>> "n_packets" | grep -- "table_id" | sed -e 
>> 's/dp_hash=0x[[0-9a-f]][[0-9a-f]]*/dp_hash=0x0/' -e 
>> 's/output:[[0-9]][[0-9]]*/output/'], [0], [dnl
>> -table_id=254, n_packets=1, n_bytes=64, 
>> priority=20,recirc_id=0x2,dp_hash=0x0/0xff,actions=output
>> +table_id=254, n_packets=1, n_bytes=64, 
>> priority=20,recirc_id=2,dp_hash=0x0/0xff,actions=output
>> ])
>>
>> # Checks the flow stats in br-int, should be only one match.
>> --
>> 1.9.1
>>
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] netdev-dpdk: Use default NIC configuration.

2015-05-19 Thread Kevin Traynor 
This patch simplifies Rx/Tx NIC configuration by removing
custom values and using the defaults provided by the DPDK
PMDs. This also enables Rx vectorisation which improves
performance.

Signed-off-by: Kevin Traynor 
---
 lib/netdev-dpdk.c |   32 ++--
 1 files changed, 2 insertions(+), 30 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 505ab75..685d998 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -90,15 +90,6 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
 #define NIC_PORT_RX_Q_SIZE 2048  /* Size of Physical NIC RX Queue, Max 
(n+32<=4096)*/
 #define NIC_PORT_TX_Q_SIZE 2048  /* Size of Physical NIC TX Queue, Max 
(n+32<=4096)*/
 
-/* XXX: Needs per NIC value for these constants. */
-#define RX_PTHRESH 32 /* Default values of RX prefetch threshold reg. */
-#define RX_HTHRESH 32 /* Default values of RX host threshold reg. */
-#define RX_WTHRESH 16 /* Default values of RX write-back threshold reg. */
-
-#define TX_PTHRESH 36 /* Default values of TX prefetch threshold reg. */
-#define TX_HTHRESH 0  /* Default values of TX host threshold reg. */
-#define TX_WTHRESH 0  /* Default values of TX write-back threshold reg. */
-
 #define MAX_PKT_BURST 32   /* Max burst size for RX/TX */
 
 /* Character device cuse_dev_name. */
@@ -130,25 +121,6 @@ static const struct rte_eth_conf port_conf = {
 },
 };
 
-static const struct rte_eth_rxconf rx_conf = {
-.rx_thresh = {
-.pthresh = RX_PTHRESH,
-.hthresh = RX_HTHRESH,
-.wthresh = RX_WTHRESH,
-},
-};
-
-static const struct rte_eth_txconf tx_conf = {
-.tx_thresh = {
-.pthresh = TX_PTHRESH,
-.hthresh = TX_HTHRESH,
-.wthresh = TX_WTHRESH,
-},
-.tx_free_thresh = 0,
-.tx_rs_thresh = 0,
-.txq_flags = ETH_TXQ_FLAGS_NOMULTSEGS|ETH_TXQ_FLAGS_NOOFFLOADS,
-};
-
 enum { MAX_TX_QUEUE_LEN = 384 };
 enum { DPDK_RING_SIZE = 256 };
 BUILD_ASSERT_DECL(IS_POW2(DPDK_RING_SIZE));
@@ -451,7 +423,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) 
OVS_REQUIRES(dpdk_mutex)
 
 for (i = 0; i < dev->up.n_txq; i++) {
 diag = rte_eth_tx_queue_setup(dev->port_id, i, NIC_PORT_TX_Q_SIZE,
-  dev->socket_id, &tx_conf);
+  dev->socket_id, NULL);
 if (diag) {
 VLOG_ERR("eth dev tx queue setup error %d",diag);
 return -diag;
@@ -461,7 +433,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) 
OVS_REQUIRES(dpdk_mutex)
 for (i = 0; i < dev->up.n_rxq; i++) {
 diag = rte_eth_rx_queue_setup(dev->port_id, i, NIC_PORT_RX_Q_SIZE,
   dev->socket_id,
-  &rx_conf, dev->dpdk_mp->mp);
+  NULL, dev->dpdk_mp->mp);
 if (diag) {
 VLOG_ERR("eth dev rx queue setup error %d",diag);
 return -diag;
-- 
1.7.4.1

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

[ovs-dev] [PATCH] ovn: Change type of 'chassis' column in 'Binding' table.

2015-05-19 Thread Alex Wang 
This commit changes the type of 'chassis' column in 'Binding' table
from string to weak reference of 'Chassis' table entry.  This will
make accessing the chassis from binding more efficient.

Signed-off-by: Alex Wang 
---
 ovn/controller/binding.c  |   35 ++-
 ovn/controller/physical.c |2 +-
 ovn/northd/ovn-northd.c   |4 ++--
 ovn/ovn-sb.ovsschema  |5 -
 ovn/ovn-sb.xml|3 +--
 5 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index ab6d9f9..b51d6a7 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -74,6 +74,7 @@ get_local_iface_ids(struct controller_ctx *ctx, struct sset 
*lports)
 void
 binding_run(struct controller_ctx *ctx)
 {
+const struct sbrec_chassis *chassis_rec;
 const struct sbrec_binding *binding_rec;
 struct ovsdb_idl_txn *txn;
 struct sset lports, all_lports;
@@ -85,6 +86,13 @@ binding_run(struct controller_ctx *ctx)
 get_local_iface_ids(ctx, &lports);
 sset_clone(&all_lports, &lports);
 
+SBREC_CHASSIS_FOR_EACH(chassis_rec, ctx->ovnsb_idl) {
+if (!strcmp(chassis_rec->name, ctx->chassis_id)) {
+break;
+}
+}
+ovs_assert(chassis_rec);
+
 txn = ovsdb_idl_txn_create(ctx->ovnsb_idl);
 ovsdb_idl_txn_add_comment(txn,
   "ovn-controller: updating bindings for '%s'",
@@ -94,17 +102,18 @@ binding_run(struct controller_ctx *ctx)
 if (sset_find_and_delete(&lports, binding_rec->logical_port) ||
 (binding_rec->parent_port && binding_rec->parent_port[0] &&
  sset_contains(&all_lports, binding_rec->parent_port))) {
-if (!strcmp(binding_rec->chassis, ctx->chassis_id)) {
+if (binding_rec->chassis == chassis_rec) {
 continue;
 }
-if (binding_rec->chassis[0]) {
+if (binding_rec->chassis) {
 VLOG_INFO("Changing chassis for lport %s from %s to %s",
-  binding_rec->logical_port, binding_rec->chassis,
-  ctx->chassis_id);
+  binding_rec->logical_port,
+  binding_rec->chassis->name,
+  chassis_rec->name);
 }
-sbrec_binding_set_chassis(binding_rec, ctx->chassis_id);
-} else if (!strcmp(binding_rec->chassis, ctx->chassis_id)) {
-sbrec_binding_set_chassis(binding_rec, "");
+sbrec_binding_set_chassis(binding_rec, chassis_rec);
+} else if (binding_rec->chassis == chassis_rec) {
+sbrec_binding_set_chassis(binding_rec, NULL);
 }
 }
 
@@ -126,10 +135,18 @@ binding_run(struct controller_ctx *ctx)
 void
 binding_destroy(struct controller_ctx *ctx)
 {
+const struct sbrec_chassis *chassis_rec;
 int retval = TXN_TRY_AGAIN;
 
 ovs_assert(ctx->ovnsb_idl);
 
+SBREC_CHASSIS_FOR_EACH(chassis_rec, ctx->ovnsb_idl) {
+if (!strcmp(chassis_rec->name, ctx->chassis_id)) {
+break;
+}
+}
+ovs_assert(chassis_rec);
+
 while (retval != TXN_SUCCESS && retval != TXN_UNCHANGED) {
 const struct sbrec_binding *binding_rec;
 struct ovsdb_idl_txn *txn;
@@ -140,8 +157,8 @@ binding_destroy(struct controller_ctx *ctx)
   ctx->chassis_id);
 
 SBREC_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
-if (!strcmp(binding_rec->chassis, ctx->chassis_id)) {
-sbrec_binding_set_chassis(binding_rec, "");
+if (binding_rec->chassis == chassis_rec) {
+sbrec_binding_set_chassis(binding_rec, NULL);
 }
 }
 
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 0fb43c0..dc2fcee 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -116,7 +116,7 @@ physical_run(struct controller_ctx *ctx)
 bool local = ofport != 0;
 if (!local) {
 ofport = u16_to_ofp(simap_get(&chassis_to_ofport,
-  binding->chassis));
+  binding->chassis->name));
 if (!ofport) {
 continue;
 }
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index cfad6be..f00e43e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -660,10 +660,10 @@ ovnsb_db_changed(struct northd_context *ctx)
 continue;
 }
 
-if (*binding->chassis && (!lport->up || !*lport->up)) {
+if (binding->chassis && (!lport->up || !*lport->up)) {
 bool up = true;
 nbrec_logical_port_set_up(lport, &up, 1);
-} else if (!*binding->chassis && (!lport->up || *lport->up)) {
+} else if (!binding->chassis && (!lport->up || *lport->up)) {
 bool up = false;
 nbrec_logical_port_set_up(

Re: [ovs-dev] [PATCH] ovn: Fix a northd bug.

2015-05-19 Thread Alex Wang 
Based on offline discussion, we want to change the type of 'chassis' column
in 'Binding' table from string to weak reference of 'Chassis' table entry.

Already sent new patch.

Thanks,
Alex Wang,

On Tue, May 19, 2015 at 9:09 AM, Alex Wang  wrote:

>
>
> On Tue, May 19, 2015 at 8:31 AM, Ben Pfaff  wrote:
>
>> On Mon, May 18, 2015 at 06:58:15PM -0700, Alex Wang wrote:
>> > The 'chassis' member in the 'struct sbrec_binding' must always be
>> > non-null.  However, this is not case when creating the binding
>> > in "set_bindings()".  And it causes segfault while starting northd
>> > with existing ovnnb configuration.
>> >
>> > This commit fixes the bug by always setting the 'chassis' to an
>> > empty string.
>> >
>> > Signed-off-by: Alex Wang 
>>
>> This may not be the best fix.  When the Binding table was introduced,
>> the 'chassis' column could always be populated immediately because the
>> chassis was what added it and thus it could always fill in its own name,
>> so there wasn't any point in allowing the chassis column to be empty.
>> But now that Binding rows are added before we have a chassis, the best
>> fix might be to change the schema to allow 'chassis' to be empty (and
>> then change any references to 'chassis' to no longer assume it must be
>> nonnull).
>>
>> What do you think?
>>
>>
> Thx for the explanation, and makes sense,
>
> I'll make the change,
>
> Thanks,
> Alex Wang,
>
>
>
>> Thanks,
>>
>> Ben.
>>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/2] sparse: Fix sparse when compiling DPDK.

2015-05-19 Thread Ethan Jackson 
I think you're running into the issue that sparse doesn't understand
the "gnu_inline" macro which is used extensively in the DPDK header
files.  If you run tip of master sparse
(git://git.kernel.org/pub/scm/devel/sparse/sparse.git) it should work
for you.

In the past we've typically required relatively bleeding edge sparse
builds due to similar reasons, so IMO I'm fine with making this
requirement.  Also, it's unclear to me how to auto generate these
stubs, it's actually somewhat difficult requiring a lot of manually
tuning.

Ethan

On Mon, May 18, 2015 at 1:20 PM, Daniele Di Proietto
 wrote:
> On my system (I'm using GCC 4.9 and sparse 0.4.5) I need
> lots of other headers like emmintrin.h to compile without
> warnings.
>
> The complete list is:
>
> adxintrin.h,ammintrin.h,avx2intrin.h,avx512fintrin.h,
> avxintrin.h,bmi2intrin.h,bmiintrin.h,f16cintrin.h,
> fma4intrin.h,fmaintrin.h,fxsrintrin.h,ia32intrin.h,
> immintrin.h,lwpintrin.h,lzcntintrin.h,mm3dnow.h,
> mmintrin.h,pmmintrin.h,popcntintrin.h,prfchwintrin.h,
> rdseedintrin.h,smmintrin.h,tbmintrin.h,tmmintrin.h,
> wmmintrin.h,xmmintrin.h,xopintrin.h,xsaveintrin.h,
> xsaveoptintrin.h
>
> Do we want to generate these at build time, maybe?
>
> On 18/05/2015 16:53, "Ethan Jackson"  wrote:
>
>>Sparse doesn't like several of the DPDK header files.  This patch
>>works around it so we can get analysis when compiling DPDK.
>>
>>Signed-off-by: Ethan Jackson 
>>---
>> include/sparse/automake.mk  |  4 
>> include/sparse/emmintrin.h  | 21 +
>> include/sparse/rte_atomic.h | 25 +
>> include/sparse/rte_lcore.h  | 23 +++
>> include/sparse/rte_vect.h   | 23 +++
>> 5 files changed, 96 insertions(+)
>> create mode 100644 include/sparse/emmintrin.h
>> create mode 100644 include/sparse/rte_atomic.h
>> create mode 100644 include/sparse/rte_lcore.h
>> create mode 100644 include/sparse/rte_vect.h
>>
>>diff --git a/include/sparse/automake.mk b/include/sparse/automake.mk
>>index 572c7c2..c80c4c2 100644
>>--- a/include/sparse/automake.mk
>>+++ b/include/sparse/automake.mk
>>@@ -1,10 +1,14 @@
>> noinst_HEADERS += \
>> include/sparse/arpa/inet.h \
>> include/sparse/assert.h \
>>+include/sparse/emmintrin.h \
>> include/sparse/math.h \
>> include/sparse/netinet/in.h \
>> include/sparse/netinet/ip6.h \
>> include/sparse/netpacket/packet.h \
>> include/sparse/pthread.h \
>>+include/sparse/rte_atomic.h \
>>+include/sparse/rte_lcore.h \
>>+include/sparse/rte_vect.h \
>> include/sparse/sys/socket.h \
>> include/sparse/sys/wait.h
>>diff --git a/include/sparse/emmintrin.h b/include/sparse/emmintrin.h
>>new file mode 100644
>>index 000..7c788dc
>>--- /dev/null
>>+++ b/include/sparse/emmintrin.h
>>@@ -0,0 +1,21 @@
>>+/* Copyright (c) 2015 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:
>>+ *
>>+ *
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__www.apache.org_license
>>s_LICENSE-2D2.0&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=S
>>mB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=B8EAybkydoKA6-zjSe0T_2cshiumC
>>E9wwf7RTwL7R6E&s=XbDR6JeYnydOdBUZ9vrXLn17h0EOW9wIF7lhpFjM_fo&e=
>>+ *
>>+ * 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.
>>+ */
>>+
>>+#ifndef __CHECKER__
>>+#error "Use this header only with sparse.  It is not a correct
>>implementation."
>>+#endif
>>+
>>+/* Sparse doesn't support SSE2 so the "real" header file quits with an
>>error.
>>+ * Instead, we simply do nothing thereby surpressing the message. */
>>diff --git a/include/sparse/rte_atomic.h b/include/sparse/rte_atomic.h
>>new file mode 100644
>>index 000..ae49fe5
>>--- /dev/null
>>+++ b/include/sparse/rte_atomic.h
>>@@ -0,0 +1,25 @@
>>+/* Copyright (c) 2015 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:
>>+ *
>>+ *
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__www.apache.org_license
>>s_LICENSE-2D2.0&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=S
>>mB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=B8EAybkydoKA6-zjSe0T_2cshiumC
>>E9wwf7RTwL7R6E&s=XbDR6JeYnydOdBUZ9vrXLn17h0EOW9wIF7lhpFjM_fo&e=
>>+ *
>>+ * 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 o

Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: Fix sparse warnings.

2015-05-19 Thread Ethan Jackson 
Thanks  I'll merge soon

On Mon, May 18, 2015 at 1:26 PM, Daniele Di Proietto
 wrote:
>
> Acked-by: Daniele Di Proietto 
>
> On 18/05/2015 16:53, "Ethan Jackson"  wrote:
>
>>These are all minor style issues.
>>
>>Signed-off-by: Ethan Jackson 
>>---
>> lib/netdev-dpdk.c | 22 ++
>> 1 file changed, 14 insertions(+), 8 deletions(-)
>>
>>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>index 505ab75..5f8c60f 100644
>>--- a/lib/netdev-dpdk.c
>>+++ b/lib/netdev-dpdk.c
>>@@ -102,7 +102,7 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
>>ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
>> #define MAX_PKT_BURST 32   /* Max burst size for RX/TX */
>>
>> /* Character device cuse_dev_name. */
>>-char *cuse_dev_name = NULL;
>>+static char *cuse_dev_name = NULL;
>>
>> /*
>>  * Maximum amount of time in micro seconds to try and enqueue to vhost.
>>@@ -173,7 +173,7 @@ static struct ovs_list dpdk_mp_list
>>OVS_GUARDED_BY(dpdk_mutex)
>> /* This mutex must be used by non pmd threads when allocating or freeing
>>  * mbufs through mempools. Since dpdk_queue_pkts() and
>>dpdk_queue_flush() may
>>  * use mempools, a non pmd thread should hold this mutex while calling
>>them */
>>-struct ovs_mutex nonpmd_mempool_mutex = OVS_MUTEX_INITIALIZER;
>>+static struct ovs_mutex nonpmd_mempool_mutex = OVS_MUTEX_INITIALIZER;
>>
>> struct dpdk_mp {
>> struct rte_mempool *mp;
>>@@ -589,7 +589,7 @@ dpdk_dev_parse_name(const char dev_name[], const char
>>prefix[],
>> }
>>
>> cport = dev_name + strlen(prefix);
>>-*port_no = strtol(cport, 0, 0); /* string must be null terminated */
>>+*port_no = strtol(cport, NULL, 0); /* string must be null terminated
>>*/
>> return 0;
>> }
>>
>>@@ -1004,8 +1004,14 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid,
>>struct dp_packet **pkts,
>> int cnt)
>> OVS_NO_THREAD_SAFETY_ANALYSIS
>> {
>>+#if !defined(__CHECKER__) && !defined(_WIN32)
>>+const size_t PKT_ARRAY_SIZE = cnt;
>>+#else
>>+/* Sparse or MSVC doesn't like variable length array. */
>>+enum { PKT_ARRAY_SIZE = NETDEV_MAX_RX_BATCH };
>>+#endif
>> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>>-struct rte_mbuf *mbufs[cnt];
>>+struct rte_mbuf *mbufs[PKT_ARRAY_SIZE];
>> int dropped = 0;
>> int newcnt = 0;
>> int i;
>>@@ -1650,7 +1656,7 @@ netdev_dpdk_get_virtio(const struct netdev_dpdk
>>*dev)
>>  * These callbacks allow virtio-net devices to be added to vhost ports
>>when
>>  * configuration has been fully complete.
>>  */
>>-const struct virtio_net_device_ops virtio_net_device_ops =
>>+static const struct virtio_net_device_ops virtio_net_device_ops =
>> {
>> .new_device =  new_device,
>> .destroy_device = destroy_device,
>>@@ -1957,7 +1963,7 @@ dpdk_init(int argc, char **argv)
>> return result + 1 + base;
>> }
>>
>>-const struct netdev_class dpdk_class =
>>+static const struct netdev_class dpdk_class =
>> NETDEV_DPDK_CLASS(
>> "dpdk",
>> NULL,
>>@@ -1971,7 +1977,7 @@ const struct netdev_class dpdk_class =
>> netdev_dpdk_get_status,
>> netdev_dpdk_rxq_recv);
>>
>>-const struct netdev_class dpdk_ring_class =
>>+static const struct netdev_class dpdk_ring_class =
>> NETDEV_DPDK_CLASS(
>> "dpdkr",
>> NULL,
>>@@ -1985,7 +1991,7 @@ const struct netdev_class dpdk_ring_class =
>> netdev_dpdk_get_status,
>> netdev_dpdk_rxq_recv);
>>
>>-const struct netdev_class dpdk_vhost_class =
>>+static const struct netdev_class dpdk_vhost_class =
>> NETDEV_DPDK_CLASS(
>> "dpdkvhost",
>> dpdk_vhost_class_init,
>>--
>>1.9.1
>>
>>___
>>dev mailing list
>>dev@openvswitch.org
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>>n_listinfo_dev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Sm
>>B5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=hoxyjcBNufALnZ_Yi7bFepTWy7MYtM
>>NGdugpmO-wZs0&s=QIfWBVI9ihc1Q_Cv2cqpCiUcV828KHly9TJ7wAcAftE&e=
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] dpdk: Ditch MAX_PKT_BURST macro.

2015-05-19 Thread Ethan Jackson 
Cool thanks for trying it out.  I'll merge soon.

Ethan

On Mon, May 18, 2015 at 1:22 PM, Traynor, Kevin  wrote:
>
>> -Original Message-
>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Ethan Jackson
>> Sent: Monday, May 18, 2015 5:08 PM
>> To: dev@openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH] dpdk: Ditch MAX_PKT_BURST macro.
>>
>> This version of the patch breaks sparse, I sent out another.
>>
>> Ethan
>
> The change makes sense - I tested this version on various dpdk interfaces and
> as expected there was no performance issues.
>
>>
>> On Sat, May 16, 2015 at 11:24 AM, Ethan Jackson  wrote:
>> > The MAX_PKT_BURST and NETDEV_MAX_RX_BATCH macros had a confusing
>> > relationship.  They basically purport to do the same thing, making it
>> > unclear which is the source of truth.
>> >
>> > Furthermore, while NETDEV_MAX_RX_BATCH was 256, MAX_PKT_BURST was 32,
>> > meaning we never process a batch larger than 32 packets further adding
>> > to the confusion.
>> >
>> > This patch resolves the issue by removing MAX_PKT_BURST completely,
>> > and shrinking the new NETDEV_MAX_BURST macro to only 32.  This should
>> > have no change in the execution path except shrinking a couple of
>> > structs and memory allocations (can't hurt).
>> >
>> > Signed-off-by: Ethan Jackson 
>> > ---
>> >  lib/dpif-netdev.c | 10 +-
>> >  lib/netdev-dpdk.c |  7 ++-
>> >  lib/netdev.h  |  2 +-
>> >  3 files changed, 8 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> > index f1d65f5..4216865 100644
>> > --- a/lib/dpif-netdev.c
>> > +++ b/lib/dpif-netdev.c
>> > @@ -2500,7 +2500,7 @@ dp_netdev_process_rxq_port(struct
>> dp_netdev_pmd_thread *pmd,
>> > struct dp_netdev_port *port,
>> > struct netdev_rxq *rxq)
>> >  {
>> > -struct dp_packet *packets[NETDEV_MAX_RX_BATCH];
>> > +struct dp_packet *packets[NETDEV_MAX_BURST];
>> >  int error, cnt;
>> >
>> >  cycles_count_start(pmd);
>> > @@ -3027,7 +3027,7 @@ struct packet_batch {
>> >
>> >  struct dp_netdev_flow *flow;
>> >
>> > -struct dp_packet *packets[NETDEV_MAX_RX_BATCH];
>> > +struct dp_packet *packets[NETDEV_MAX_BURST];
>> >  };
>> >
>> >  static inline void
>> > @@ -3397,7 +3397,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>> int cnt,
>> >
>> >  case OVS_ACTION_ATTR_TUNNEL_PUSH:
>> >  if (*depth < MAX_RECIRC_DEPTH) {
>> > -struct dp_packet *tnl_pkt[NETDEV_MAX_RX_BATCH];
>> > +struct dp_packet *tnl_pkt[NETDEV_MAX_BURST];
>> >  int err;
>> >
>> >  if (!may_steal) {
>> > @@ -3423,7 +3423,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>> int cnt,
>> >
>> >  p = dp_netdev_lookup_port(dp, portno);
>> >  if (p) {
>> > -struct dp_packet *tnl_pkt[NETDEV_MAX_RX_BATCH];
>> > +struct dp_packet *tnl_pkt[NETDEV_MAX_BURST];
>> >  int err;
>> >
>> >  if (!may_steal) {
>> > @@ -3485,7 +3485,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>> int cnt,
>> >
>> >  case OVS_ACTION_ATTR_RECIRC:
>> >  if (*depth < MAX_RECIRC_DEPTH) {
>> > -struct dp_packet *recirc_pkts[NETDEV_MAX_RX_BATCH];
>> > +struct dp_packet *recirc_pkts[NETDEV_MAX_BURST];
>> >
>> >  if (!may_steal) {
>> > dp_netdev_clone_pkt_batch(recirc_pkts, packets, cnt);
>> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> > index 505ab75..b06f92a 100644
>> > --- a/lib/netdev-dpdk.c
>> > +++ b/lib/netdev-dpdk.c
>> > @@ -99,8 +99,6 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
>> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
>> >  #define TX_HTHRESH 0  /* Default values of TX host threshold reg. */
>> >  #define TX_WTHRESH 0  /* Default values of TX write-back threshold reg. */
>> >
>> > -#define MAX_PKT_BURST 32   /* Max burst size for RX/TX */
>> > -
>> >  /* Character device cuse_dev_name. */
>> >  char *cuse_dev_name = NULL;
>> >
>> > @@ -862,7 +860,7 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq_,
>> >  nb_rx = rte_vhost_dequeue_burst(virtio_dev, qid,
>> >  vhost_dev->dpdk_mp->mp,
>> >  (struct rte_mbuf **)packets,
>> > -MAX_PKT_BURST);
>> > +NETDEV_MAX_BURST);
>> >  if (!nb_rx) {
>> >  return EAGAIN;
>> >  }
>> > @@ -889,8 +887,7 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, struct
>> dp_packet **packets,
>> >
>> >  nb_rx = rte_eth_rx_burst(rx->port_id, rxq_->queue_id,
>> >   (struct rte_mbuf **) packets,
>> > - MIN((int) NETDEV_MAX_RX_BATCH,
>> > - (int) MAX_PKT_BURST));
>> > + NETDEV_MAX_BURST);
>> >  if (!nb_rx) {
>> >  return EAGAIN

Re: [ovs-dev] [PATCH] dpdk: Ditch MAX_PKT_BURST macro.

2015-05-19 Thread Ethan Jackson 
Cool thanks for trying it out.  I'll merge soon.

Ethan

On Mon, May 18, 2015 at 1:22 PM, Traynor, Kevin  wrote:
>
>> -Original Message-
>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Ethan Jackson
>> Sent: Monday, May 18, 2015 5:08 PM
>> To: dev@openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH] dpdk: Ditch MAX_PKT_BURST macro.
>>
>> This version of the patch breaks sparse, I sent out another.
>>
>> Ethan
>
> The change makes sense - I tested this version on various dpdk interfaces and
> as expected there was no performance issues.
>
>>
>> On Sat, May 16, 2015 at 11:24 AM, Ethan Jackson  wrote:
>> > The MAX_PKT_BURST and NETDEV_MAX_RX_BATCH macros had a confusing
>> > relationship.  They basically purport to do the same thing, making it
>> > unclear which is the source of truth.
>> >
>> > Furthermore, while NETDEV_MAX_RX_BATCH was 256, MAX_PKT_BURST was 32,
>> > meaning we never process a batch larger than 32 packets further adding
>> > to the confusion.
>> >
>> > This patch resolves the issue by removing MAX_PKT_BURST completely,
>> > and shrinking the new NETDEV_MAX_BURST macro to only 32.  This should
>> > have no change in the execution path except shrinking a couple of
>> > structs and memory allocations (can't hurt).
>> >
>> > Signed-off-by: Ethan Jackson 
>> > ---
>> >  lib/dpif-netdev.c | 10 +-
>> >  lib/netdev-dpdk.c |  7 ++-
>> >  lib/netdev.h  |  2 +-
>> >  3 files changed, 8 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> > index f1d65f5..4216865 100644
>> > --- a/lib/dpif-netdev.c
>> > +++ b/lib/dpif-netdev.c
>> > @@ -2500,7 +2500,7 @@ dp_netdev_process_rxq_port(struct
>> dp_netdev_pmd_thread *pmd,
>> > struct dp_netdev_port *port,
>> > struct netdev_rxq *rxq)
>> >  {
>> > -struct dp_packet *packets[NETDEV_MAX_RX_BATCH];
>> > +struct dp_packet *packets[NETDEV_MAX_BURST];
>> >  int error, cnt;
>> >
>> >  cycles_count_start(pmd);
>> > @@ -3027,7 +3027,7 @@ struct packet_batch {
>> >
>> >  struct dp_netdev_flow *flow;
>> >
>> > -struct dp_packet *packets[NETDEV_MAX_RX_BATCH];
>> > +struct dp_packet *packets[NETDEV_MAX_BURST];
>> >  };
>> >
>> >  static inline void
>> > @@ -3397,7 +3397,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>> int cnt,
>> >
>> >  case OVS_ACTION_ATTR_TUNNEL_PUSH:
>> >  if (*depth < MAX_RECIRC_DEPTH) {
>> > -struct dp_packet *tnl_pkt[NETDEV_MAX_RX_BATCH];
>> > +struct dp_packet *tnl_pkt[NETDEV_MAX_BURST];
>> >  int err;
>> >
>> >  if (!may_steal) {
>> > @@ -3423,7 +3423,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>> int cnt,
>> >
>> >  p = dp_netdev_lookup_port(dp, portno);
>> >  if (p) {
>> > -struct dp_packet *tnl_pkt[NETDEV_MAX_RX_BATCH];
>> > +struct dp_packet *tnl_pkt[NETDEV_MAX_BURST];
>> >  int err;
>> >
>> >  if (!may_steal) {
>> > @@ -3485,7 +3485,7 @@ dp_execute_cb(void *aux_, struct dp_packet **packets,
>> int cnt,
>> >
>> >  case OVS_ACTION_ATTR_RECIRC:
>> >  if (*depth < MAX_RECIRC_DEPTH) {
>> > -struct dp_packet *recirc_pkts[NETDEV_MAX_RX_BATCH];
>> > +struct dp_packet *recirc_pkts[NETDEV_MAX_BURST];
>> >
>> >  if (!may_steal) {
>> > dp_netdev_clone_pkt_batch(recirc_pkts, packets, cnt);
>> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> > index 505ab75..b06f92a 100644
>> > --- a/lib/netdev-dpdk.c
>> > +++ b/lib/netdev-dpdk.c
>> > @@ -99,8 +99,6 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
>> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
>> >  #define TX_HTHRESH 0  /* Default values of TX host threshold reg. */
>> >  #define TX_WTHRESH 0  /* Default values of TX write-back threshold reg. */
>> >
>> > -#define MAX_PKT_BURST 32   /* Max burst size for RX/TX */
>> > -
>> >  /* Character device cuse_dev_name. */
>> >  char *cuse_dev_name = NULL;
>> >
>> > @@ -862,7 +860,7 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq_,
>> >  nb_rx = rte_vhost_dequeue_burst(virtio_dev, qid,
>> >  vhost_dev->dpdk_mp->mp,
>> >  (struct rte_mbuf **)packets,
>> > -MAX_PKT_BURST);
>> > +NETDEV_MAX_BURST);
>> >  if (!nb_rx) {
>> >  return EAGAIN;
>> >  }
>> > @@ -889,8 +887,7 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, struct
>> dp_packet **packets,
>> >
>> >  nb_rx = rte_eth_rx_burst(rx->port_id, rxq_->queue_id,
>> >   (struct rte_mbuf **) packets,
>> > - MIN((int) NETDEV_MAX_RX_BATCH,
>> > - (int) MAX_PKT_BURST));
>> > + NETDEV_MAX_BURST);
>> >  if (!nb_rx) {
>> >  return EAGAIN