sure no problem.
Ethan
On Thu, Aug 11, 2011 at 21:44, Ben Pfaff wrote:
> On Thu, Aug 11, 2011 at 07:47:27PM -0700, Ethan Jackson wrote:
>> This patch also fixes a bug introduced in Commit 43edca57
>> "nx-match: New helpers.", which caused the "move" action to
>> improperly handle bit ranges.
>
>
On Thu, Aug 11, 2011 at 07:47:23PM -0700, Ethan Jackson wrote:
> > The return value semantics of nxm_read_field() are surprising in that
> > it just masks off the bits not being read. ?If I had not read the code
> > for the function, I would have guessed that it also shifted the bits
> > so that th
On Thu, Aug 11, 2011 at 07:47:27PM -0700, Ethan Jackson wrote:
> This patch also fixes a bug introduced in Commit 43edca57
> "nx-match: New helpers.", which caused the "move" action to
> improperly handle bit ranges.
I assume that a minimal fix would be much shorter? Would you mind
writing that u
> The return value semantics of nxm_read_field() are surprising in that
> it just masks off the bits not being read. If I had not read the code
> for the function, I would have guessed that it also shifted the bits
> so that the least-significant bit being read was the least-significant
> bit in t
nxm_read_field_bits() simplifies reading of NXM fields with an
ofs_nbits parameter. This patch updates nxm_execute_reg_move() to
use the new function. A user outside of the nx-match module will
be added in future patches.
This patch also fixes a bug introduced in Commit 43edca57
"nx-match: New h
---
tests/ofproto-dpif.at | 25 +
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 846eccb..ea66bb4 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -17,3 +17,28 @@ AT_CHECK([tail -1 stdou
The 64-bit packed field is just an intermediate step. I agree that it
is somewhat ugly. It will be replaced by a unique ID of one kind or
another (an incrementing counter? random number?) either when it gets
to be too small, or when we decide to fix up the stats race window I
described earlier.
OK. Makes sense.
It might be OK from an sFlow perspective if the user-space lookup occasionally
fails.Just as long as it never retrieves the wrong action-list. That would
be a problem.
If you can look up the actions in user-space, do you still need to generate
that 64-bit packed-field?
For the record, I decided to drop this patch.
EThan
On Thu, Aug 11, 2011 at 12:29, Ben Pfaff wrote:
> On Thu, Aug 11, 2011 at 11:53:42AM -0700, Ethan Jackson wrote:
>> It doesn't make sense for either of these parameters to be
>> negative. This seems more intuitive.
>
> OK, but can you also adju
On Thu, Aug 11, 2011 at 11:53:43AM -0700, Ethan Jackson wrote:
> This patch simplifies the API of nxm_dst_check() and adds a new
> function nxm_src_check() for checking source fields.
> ---
>
> Come to think of it, I don't really like the interface of either
> nxm_dst_check() or nxm_src_check().
On Thu, Aug 11, 2011 at 11:53:42AM -0700, Ethan Jackson wrote:
> It doesn't make sense for either of these parameters to be
> negative. This seems more intuitive.
OK, but can you also adjust the callers of nxm_decode_ofs() and
nxm_decode_n_bits() that currently assign the return values to `int's?
This patch simplifies the API of nxm_dst_check() and adds a new
function nxm_src_check() for checking source fields.
---
Come to think of it, I don't really like the interface of either
nxm_dst_check() or nxm_src_check(). What do you think of something like this?
---
lib/autopath.c | 11
It doesn't make sense for either of these parameters to be
negative. This seems more intuitive.
---
lib/nx-match.c |7 ---
lib/nx-match.h |9 +
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/lib/nx-match.c b/lib/nx-match.c
index ecc284e..cc29c6c 100644
--- a/lib
Neil, don't worry! This really is a step forward and not a step back.
The actions passed to the kernel and back to userspace are not really
as helpful as you think, because they discard a lot of semantic
information. These actions do say *what* happened but they do not say
*why*. One off-the-cuf
I think the current behavior where the whole actions-list is serialized and
passed up with the sample is actually really elegant and future-proof. New
actions are being added all the time, and having access to a sampled-stream of
"exactly-what-is-happening-in-there" is always going to be good
Thanks, I pushed this one.
On Thu, Aug 11, 2011 at 10:50:04AM -0700, Ethan Jackson wrote:
> Looks good,
>
> Ethan
>
> On Thu, Aug 11, 2011 at 09:10, Ben Pfaff wrote:
> > This comment must be very old. ?ovs-ofctl has supported all the OpenFlow
> > and Nicira extensions actions for a long time no
Looks good, thank you.
On Thu, Aug 11, 2011 at 10:48:40AM -0700, Ethan Jackson wrote:
> Thanks for the review, I added the following comment.
>
> /* This sequence number should be incremented whenever anything involving
> flows
> * or the wildcarding of flows changes. This will cause build ass
Looks good,
Ethan
On Thu, Aug 11, 2011 at 09:10, Ben Pfaff wrote:
> This comment must be very old. ovs-ofctl has supported all the OpenFlow
> and Nicira extensions actions for a long time now.
> ---
> utilities/ovs-ofctl.8.in | 4
> 1 files changed, 0 insertions(+), 4 deletions(-)
>
>
Thanks for the review, I added the following comment.
/* This sequence number should be incremented whenever anything involving flows
* or the wildcarding of flows changes. This will cause build assertion
* failures in places which likely need to be updated. */
#define FLOW_WC_SEQ 1
On Wed, Au
On Wed, Aug 10, 2011 at 05:21:43PM -0700, Ethan Jackson wrote:
> The NXAST_OUTPUT_REG outputs to the OpenFlow port contained in a
> supplied NXM field.
I think that the NEWS item should say that we've added an OpenFlow
extension to output to the port given in a field, instead of just
saying that "
On Wed, Aug 10, 2011 at 05:21:42PM -0700, Ethan Jackson wrote:
> nxm_read_field() simplifies reading of NXM fields with an ofs_nbits
> parameter. This patch updates nxm_execute_reg_move() to use the
> new function. A user outside of the nx-match module will be added
> in future patches.
The retu
On Wed, Aug 10, 2011 at 05:21:41PM -0700, Ethan Jackson wrote:
> This patch also updates nxm_check_reg_move() to use nxm_src_check()
> and nxm_dst_check(). A user outside of nxm-match will be added in
> a future patch.
I have a similar patch in the branch that I'm working on, too. The
only impor
This comment must be very old. ovs-ofctl has supported all the OpenFlow
and Nicira extensions actions for a long time now.
---
utilities/ovs-ofctl.8.in |4
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index aa37969..b0
23 matches
Mail list logo
