Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-09-09 Thread Nithin Raju
leteNBL() only to decrease the reference counter by one, instead of > decreasing it in the very beginning and not worry about any missed exit point. > As for OvsTunnelPortTx, you mean I should use OvsInitForwardingCtx() instead > of simply dropping, at the beginning, the reference to the

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-09-09 Thread Samuel Ghinet
you mean I should use OvsInitForwardingCtx() instead of simply dropping, at the beginning, the reference to the splitNbl (last patch version)? Thanks, Samuel From: Nithin Raju [nit...@vmware.com] Sent: Tuesday, September 09, 2014 6:36 PM To: Samuel Ghin

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-09-09 Thread Nithin Raju
On Aug 29, 2014, at 1:20 PM, Saurabh Shah wrote: >> Generally, we have not used '_' prefix for function parameter names. We >> use them for macros etc, but not for functions. Do you prefer it this way? I >> am not against it, but I don't think it is necessary. For a macro, it is >> necessary >>

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-09-09 Thread Nithin Raju
hi Sam, Pls. find my answers inlined. Pls. feel free to repost the patches for the two different functionalities. On Sep 6, 2014, at 2:39 PM, Samuel Ghinet wrote: > Yes, you were right with the TSO. I'll send a new version of the patch, > though I am not sure how it would be easier for you to

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-09-08 Thread Saurabh Shah
> > This function has become a bit too long now, can you please refactor it? > > One obvious thing you could do is to extract the processing of an > > single NB into a separate function and the parent function can take > > care of splitting the NBL (if required) & creating the right forwarding ct

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-09-06 Thread Samuel Ghinet
confused me. Regarding OvsSlowPathDecapVxlan: > You can be ASSURED that you'll not get an NBL with multiple NBs here. > OvsInjectPacketThroughActions() has already checked for this case. Nevertheless, it makes the code in OvsSlowPathDecapVxlan make it clear that only one NB is expected. A

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-09-06 Thread Samuel Ghinet
Thanks, Samuel From: Saurabh Shah [ssaur...@vmware.com] Sent: Friday, August 29, 2014 9:51 PM To: Samuel Ghinet; Alin Serdean; dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] Create a NBL for each NB when required Hi Samuel, > > &g

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-08-29 Thread Nithin Raju
hi Sam/Alin, Thanks for working on this. In general, it would have been better if the two changes: 1. handle NBL with multiple NBs 2. Optimizing the parsing code to handle NBs rather than NBLs. could have been split into different review. I was not a big fan of split up changes into multiple rev

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-08-29 Thread Nithin Raju
hi Sam/Alin, Thanks for working on this. In general, it would have been better if the two changes: 1. handle NBL with multiple NBs 2. Optimizing the parsing code to handle NBs rather than NBLs. could have been split into different review. I was not a big fan of split up changes into multiple rev

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-08-29 Thread Saurabh Shah
Hi Samuel, > > >All NET_BUFFERs of a NET_BUFFER_LIST must go through the pipeline: > >extract, find flow, execute. Previously, only the first NET_BUFFER of a > >NET_BUFFER_LIST was going through this pipeline, which was erroneous. > > comments here. > > It might make the reading clearer, and st

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-08-27 Thread Ben Pfaff
On Wed, Aug 27, 2014 at 04:01:17PM +, Samuel Ghinet wrote: > For future reviews, could you please reduce or mark somehow the patch, so it > would be easier for me to find your comments? > Something like, > > >All NET_BUFFERs of a NET_BUFFER_LIST must go through > >the pipeline: extract, find

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-08-27 Thread Samuel Ghinet
15 AM To: Alin Serdean; dev@openvswitch.org; Nithin Raju; Samuel Ghinet Subject: Re: [ovs-dev] [PATCH] Create a NBL for each NB when required Hi Alin, Thanks for working on it! I am almost done reviewing the change. I have a few comments below. >ovs/ovs-issues#15 > >All NET_BUFFERs of a NET_BUFFE

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-08-22 Thread Saurabh Shah
Hi Alin, Thanks for working on it! I am almost done reviewing the change. I have a few comments below. >ovs/ovs-issues#15 > >All NET_BUFFERs of a NET_BUFFER_LIST must go through >the pipeline: extract, find flow, execute. Previously, >only the first NET_BUFFER of a NET_BUFFER_LIST was going >thro

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-08-22 Thread Samuel Ghinet
35 PM To: Alin Serdean Cc: dev@openvswitch.org; Samuel Ghinet Subject: Re: [ovs-dev] [PATCH] Create a NBL for each NB when required hi Alin/Sam, Like I said earlier, this is a critical fix, and thanks for working on this. I started looking at OvsPacketIO.c, and you are right in using the OvsP

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-08-22 Thread Nithin Raju
while a NET_BUFFER_LIST as param > constraints the use of an OvsGetTcp call to a specific scenario / usage only. > > Regarding braces, sorry for that. It will be fixed. > > Sam > > From: Nithin Raju [nit...@vmware.com] > Sent: Thursda

Re: [ovs-dev] [PATCH] Create a NBL for each NB when required

2014-08-21 Thread Nithin Raju
hi Alin/Sam, Like I said earlier, this is a critical fix, and thanks for working on this. I started looking at OvsPacketIO.c, and you are right in using the OvsPartialCopyToMultipleNBLs() API. However, is it required to update many of the functions to process an NB instead of an NBL? From my und

[ovs-dev] [PATCH] Create a NBL for each NB when required

2014-08-19 Thread Alin Serdean
ovs/ovs-issues#15 All NET_BUFFERs of a NET_BUFFER_LIST must go through the pipeline: extract, find flow, execute. Previously, only the first NET_BUFFER of a NET_BUFFER_LIST was going through this pipeline, which was erroneous. OvsPartialCopyToMultipleNBLs is used to make each NET_BUFFER have its