Hello!
I am Goh Eng Eam Francis, attorney at law with many years of practice. I was an
attorney to a late client of mine who died in Malay-sia of a heart related
condition in 2008. My reason of sending you this email is to help secure the
amount left behind by my client before it is confiscate
On Thu, Sep 15, 2011 at 6:37 PM, Jesse Gross wrote:
> On Thu, Sep 15, 2011 at 6:17 PM, Pravin Shelar wrote:
>> diff --git a/include/openvswitch/datapath-protocol.h
>> b/include/openvswitch/datapath-protocol.h
>> index 5d765b1..3c79c0f 100644
>> --- a/include/openvswitch/datapath-protocol.h
>> ++
On Thu, Sep 15, 2011 at 6:17 PM, Pravin Shelar wrote:
> diff --git a/include/openvswitch/datapath-protocol.h
> b/include/openvswitch/datapath-protocol.h
> index 5d765b1..3c79c0f 100644
> --- a/include/openvswitch/datapath-protocol.h
> +++ b/include/openvswitch/datapath-protocol.h
> struct ovs_vp
This is incremental to previous patch. It is fixed according to
comments from Jesse. It removes dipf_port->stat as aggregate stats are only
available at netdev layer.
Signed-off-by: Pravin B Shelar
---
include/openvswitch/datapath-protocol.h | 16 +++---
lib/dpif-linux.c
On Thu, Sep 15, 2011 at 5:55 PM, Ethan Jackson wrote:
> Older kernels do not advertise the multicast groups of families
> when requested by userspace. As a workaround, this patch hardcodes
> the multicast group ID of the ovs_vport family on these kernels.
> Userspace will be able to fall back to
The nl_lookup_genl_mcgroup() function can fail on older kernels
which do not support the required netlink interface. Before this
patch, dpif-linux would refuse to create a datapath when this
happened. With this patch, it attempts to use a workaround. If
the workaround fails it simply disables th
It's possible to start receiving packets on a datapath as soon as
the internal device is created. It's therefore important that the
datapath be fully initialized before this, which it currently isn't.
In particularly, the fact that dp->stats_percpu is not yet set is
potentially fatal. In addition
Older kernels do not advertise the multicast groups of families
when requested by userspace. As a workaround, this patch hardcodes
the multicast group ID of the ovs_vport family on these kernels.
Userspace will be able to fall back to this hardcoded value if the
standard mechanism is unavailable.
> I'm going to consider this patch reviewed.
With one minor adjustment (copied into gmail):
diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
index b7c6723..24a6826 100644
--- a/include/openvswitch/automake.mk
+++ b/include/openvswitch/automake.mk
@@ -1,5 +1,6 @@
noi
On Thu, Sep 15, 2011 at 4:57 PM, Ben Pfaff wrote:
> On Thu, Sep 15, 2011 at 04:52:39PM -0700, Jesse Gross wrote:
>> It's possible to start receiving packets on a datapath as soon as
>> the internal device is created. It's therefore important that the
>> datapath be fully initialized before this,
33 it is!
I'm going to consider this patch reviewed.
Ethan
On Thu, Sep 15, 2011 at 17:11, Jesse Gross wrote:
> I don't really think this all that important. Neither moving the
> range that we allocate from nor making it discontinuous is that hard.
> Like I said before, I don't really care that
I don't really think this all that important. Neither moving the
range that we allocate from nor making it discontinuous is that hard.
Like I said before, I don't really care that much. 33 is fine.
On Thu, Sep 15, 2011 at 5:06 PM, Ben Pfaff wrote:
> I can't argue with that. We could add a way
I can't argue with that. We could add a way to query it, I guess, if
really necessary.
On Thu, Sep 15, 2011 at 05:04:16PM -0700, Jesse Gross wrote:
> I meant moving the group of fallback IDs would break things.
>
> On Thu, Sep 15, 2011 at 5:01 PM, Ben Pfaff wrote:
> > It wouldn't break the ABI
I meant moving the group of fallback IDs would break things.
On Thu, Sep 15, 2011 at 5:01 PM, Ben Pfaff wrote:
> It wouldn't break the ABI to move either pool around, because those
> aren't hardcoded in userspace, only in the kernel. A discontinuous
> range would also work but wouldn't be necess
It wouldn't break the ABI to move either pool around, because those
aren't hardcoded in userspace, only in the kernel. A discontinuous
range would also work but wouldn't be necessary.
On Thu, Sep 15, 2011 at 04:55:56PM -0700, Jesse Gross wrote:
> I guess the other thing is if we want to increase
On Thu, Sep 15, 2011 at 04:52:39PM -0700, Jesse Gross wrote:
> It's possible to start receiving packets on a datapath as soon as
> the internal device is created. It's therefore important that the
> datapath be fully initialized before this, which it currently isn't.
> In particularly, the fact th
I guess the other thing is if we want to increase our pool of
preallocated multicast groups, we have to either break the ABI or make
the current pool discontinuous.
On Thu, Sep 15, 2011 at 4:48 PM, Ben Pfaff wrote:
> Personally I'd suggest 33 for this one and increment for each
> succeeding famil
It's possible to start receiving packets on a datapath as soon as
the internal device is created. It's therefore important that the
datapath be fully initialized before this, which it currently isn't.
In particularly, the fact that dp->stats_percpu is not yet set is
potentially fatal. In addition
Personally I'd suggest 33 for this one and increment for each
succeeding family. No one's ever mentioned a problem with our use of
genetlink groups. Since RHEL5 is probably declining rather than
increasing in deployment, my guess is that no one ever will.
On Thu, Sep 15, 2011 at 04:44:53PM -0700
Not really, I don't have any particular opinion on the actual number.
The only thing that I was concerned about is what it would look like
if we want to do this with the multicast groups for other families.
On Thu, Sep 15, 2011 at 4:40 PM, Ethan Jackson wrote:
> Based on my offline discussions wi
Based on my offline discussions with Jesse I arrived, rather
arbitrarily, at the number 214. I don't know enough about the kernel
to judge what a good number choice would be. Jesse seemed to think
larger was better. I'll use whatever the two of you think is best.
Ethan
On Thu, Sep 15, 2011 at
On Thu, Sep 15, 2011 at 04:27:03PM -0700, Ethan Jackson wrote:
> > "Draconian"? ?Like torturing kittens or something?
>
> Haha, yes exactly like torturing kittens, assuming rhel 5 is a kitten,
> and a broken vswitch install is torture.
>
> > Do we at least get a VLOG_ERR if we can't register the
On Thu, Sep 15, 2011 at 04:10:55PM -0700, Ethan Jackson wrote:
> > Where does the number 214 come from?
>
> Experimentally I found that the number had to be fairly small. I
> wanted it to be large enough to be unlikely conflict to values the
> proper way. I also wanted a number which was arbitra
On Sep 15, 2011, at 4:24 PM, Jesse Gross wrote:
> On Thu, Sep 15, 2011 at 3:53 PM, Justin Pettit wrote:
>> Currently the kernel automatically sets the MTU of any internal
>> interfaces to the minimum of all attached interfaces because the Linux
>> bridge does this. Userspace can do this with mor
> In the future, it is possible that we will want a fallback for other
> multicast groups as well. Would it make sense to make the fallback
> group an argument to nl_lookup_genl_mcgroup(), with zero to disable
> fallback? (Of course nl_lookup_genl_mcgroup() would still log a
> message that the fa
> "Draconian"? Like torturing kittens or something?
Haha, yes exactly like torturing kittens, assuming rhel 5 is a kitten,
and a broken vswitch install is torture.
> Do we at least get a VLOG_ERR if we can't register the notifier? I
> don't see one in this patch but there might be one lower dow
On Thu, Sep 15, 2011 at 3:53 PM, Justin Pettit wrote:
> Currently the kernel automatically sets the MTU of any internal
> interfaces to the minimum of all attached interfaces because the Linux
> bridge does this. Userspace can do this with more knowledge and
> flexibility.
>
> Feature #7323
>
> S
On Thu, Sep 15, 2011 at 04:18:06PM -0700, Jesse Gross wrote:
> On Tue, Sep 13, 2011 at 6:11 PM, Pravin Shelar wrote:
> > Fixed according to comments from Jesse.
> >
> > --
> > Currently ovs is using device stats for Linux devices and count them
> > itself in other situations. This leads to ove
Older kernels do not advertise the multicast groups of families
when requested by userspace. As a workaround, this patch hardcodes
the multicast group ID of the ovs_vport family on these kernels.
Userspace will be able to fall back to this hardcoded value if the
standard mechanism is unavailable.
On Thu, Sep 15, 2011 at 04:17:27PM -0700, Ethan Jackson wrote:
> > EINVAL is pretty generic. ?Is there something lower down in the stack
> > that will log the actual error should one occur?
>
> Yes, nln_notifier_create() logs.
>
> Also, only two callers payed attention to the error, one of those
On Tue, Sep 13, 2011 at 6:11 PM, Pravin Shelar wrote:
> Fixed according to comments from Jesse.
>
> --
> Currently ovs is using device stats for Linux devices and count them
> itself in other situations. This leads to overlap with hardware stats,
> inconsistencies, etc. It's much better to jus
> EINVAL is pretty generic. Is there something lower down in the stack
> that will log the actual error should one occur?
Yes, nln_notifier_create() logs.
Also, only two callers payed attention to the error, one of those will
be ignoring it in a future patch. The EINVAL makes me a tad
uncomfort
On Thu, Sep 15, 2011 at 04:06:22PM -0700, Ethan Jackson wrote:
> This patch changes the interface of netlink-notifier and
> rtnetlink-link. Now nln_notifiers are allocated and destroyed by
> the module instead of passed in by callers. This allows the
> definition of nln_notifier to be hidden, and
> Where does the number 214 come from?
Experimentally I found that the number had to be fairly small. I
wanted it to be large enough to be unlikely conflict to values the
proper way. I also wanted a number which was arbitrary to avoid
conflicting with other people who may be improperly hardcodin
Looks good,
Ethan
On Fri, Aug 26, 2011 at 13:19, Ben Pfaff wrote:
> Reported-by: Philippe Jung
> ---
> ofproto/ofproto-dpif.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index f09c230..c05ca9e 100644
> --- a/o
On Thu, Sep 15, 2011 at 04:06:21PM -0700, Ethan Jackson wrote:
> It makes more sense to call nln_notifier_run() and
> nln_notifier_wait() simply nln_run() and nln_wait() since they
> don't operate on notifiers but the entire nln object. This patch
> changes the nln and the rtnetlink-link modules t
> A different approach, that seems to me more conventional, would be for
> nln_notifier_register() to allocate the struct nln_notifier and return
> it as a pointer (probably through a struct nln_notifier ** parameter).
> Then nln_notifier_unregister() would just ignore a null pointer. If
> we do i
This patch changes the interface of netlink-notifier and
rtnetlink-link. Now nln_notifiers are allocated and destroyed by
the module instead of passed in by callers. This allows the
definition of nln_notifier to be hidden, and generally cleans up
the code.
---
lib/dpif-linux.c | 16 +++
It makes more sense to call nln_notifier_run() and
nln_notifier_wait() simply nln_run() and nln_wait() since they
don't operate on notifiers but the entire nln object. This patch
changes the nln and the rtnetlink-link modules to the new
convention.
---
lib/dpif-linux.c |4 ++--
lib/ne
It would be good to get at least this trivial bug fix in soon, since
it affects master and branch-1.2.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Thanks a lot for testing this patch. I committed this to master and
branch-1.2, so the bug fix will appear in 1.2.2 and in 1.3.0 (when
they are released).
On Wed, Sep 14, 2011 at 07:31:11PM -0500, Tyler Coumbes wrote:
> Sorry for the delay. I totally missed your email. I have tested the patch on
Currently the kernel automatically sets the MTU of any internal
interfaces to the minimum of all attached interfaces because the Linux
bridge does this. Userspace can do this with more knowledge and
flexibility.
Feature #7323
Signed-off-by: Justin Pettit
---
datapath/datapath.c | 4
On Sep 15, 2011, at 10:28 AM, Ben Pfaff wrote:
> This makes adding N ports to a bridge take N**2 time by checking the
> MTU of every existing port whenever a new port is added. I see that
> the kernel datapath did the same thing, so maybe it's OK given
> caching.
As we discussed in person, this
On Sep 15, 2011, at 10:42 AM, Jesse Gross wrote:
> I think that we can actually kill off the entire vport_set_mtu()
> function and all of the vport implementations because there are no
> longer any callers.
Good point. I'll send out a revised version soon.
--Justin
__
Thanks, pushed.
On Thu, Sep 15, 2011 at 03:48:10PM -0700, Ethan Jackson wrote:
> Looks good.
>
> Ethan
>
> On Thu, Sep 15, 2011 at 15:45, Ben Pfaff wrote:
> > ---
> > ?lib/cfm.c | ? ?1 -
> > ?1 files changed, 0 insertions(+), 1 deletions(-)
> >
> > diff --git a/lib/cfm.c b/lib/cfm.c
> > index b
Looks good.
Ethan
On Thu, Sep 15, 2011 at 15:45, Ben Pfaff wrote:
> ---
> lib/cfm.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/lib/cfm.c b/lib/cfm.c
> index bc59b62..c3d96d8 100644
> --- a/lib/cfm.c
> +++ b/lib/cfm.c
> @@ -357,7 +357,6 @@ cfm_compose_ccm(struc
---
lib/cfm.c |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/lib/cfm.c b/lib/cfm.c
index bc59b62..c3d96d8 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -357,7 +357,6 @@ cfm_compose_ccm(struct cfm *cfm, struct ofpbuf *packet,
void
cfm_wait(struct cfm *cfm)
{
-
timer_wa
Thanks, I pushed it.
On Thu, Sep 15, 2011 at 03:13:24PM -0700, Ethan Jackson wrote:
> haha this is fine.
>
> Ethan
>
> On Thu, Sep 15, 2011 at 13:31, Ben Pfaff wrote:
> > On Thu, Sep 15, 2011 at 01:24:52PM -0700, Ethan Jackson wrote:
> >> > + ?del-controller BRIDGE ? ? ?delete the controllers f
On Thu, Sep 15, 2011 at 03:29:50PM -0700, Ethan Jackson wrote:
> The introduction of cfm or lacp objects to ofproto, requires the
> removal of all flows which originate from the newly "special"
> in_port.
Looks good to me, thanks.
I don't think it's necessary now, but if we start setting up kerne
The introduction of cfm or lacp objects to ofproto, requires the
removal of all flows which originate from the newly "special"
in_port.
---
ofproto/ofproto-dpif.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 88ec
haha this is fine.
Ethan
On Thu, Sep 15, 2011 at 13:31, Ben Pfaff wrote:
> On Thu, Sep 15, 2011 at 01:24:52PM -0700, Ethan Jackson wrote:
>> > + ?del-controller BRIDGE ? ? ?delete the controllers for BRIDGE\n\
>> > + ?set-controller BRIDGE TARGET... ?set the controller(s) for BRIDGE\n\
>>
>> Loo
On Thu, Sep 15, 2011 at 01:24:52PM -0700, Ethan Jackson wrote:
> > + ?del-controller BRIDGE ? ? ?delete the controllers for BRIDGE\n\
> > + ?set-controller BRIDGE TARGET... ?set the controller(s) for BRIDGE\n\
>
> Looks good,
>
> I think the del-controller needs parens around it's s. I may be wr
> + del-controller BRIDGE delete the controllers for BRIDGE\n\
> + set-controller BRIDGE TARGET... set the controller(s) for BRIDGE\n\
Looks good,
I think the del-controller needs parens around it's s. I may be wrong though.
Ethan
> get-fail-mode BRIDGE print the fail-mode for
On Thu, Sep 15, 2011 at 01:09:28PM -0700, Ethan Jackson wrote:
> > + ?get-controller BRIDGE ? ? ?print all controller(s) for BRIDGE\n\
> > + ?del-controller BRIDGE ? ? ?delete all controllers for BRIDGE\n\
> > + ?set-controller BRIDGE TARGET... ?set the controller(s) for BRIDGE\n\
>
> This seems f
> + get-controller BRIDGE print all controller(s) for BRIDGE\n\
> + del-controller BRIDGE delete all controllers for BRIDGE\n\
> + set-controller BRIDGE TARGET... set the controller(s) for BRIDGE\n\
This seems fine, the wording is not self consistent though. Perhaps
something like t
On Thu, Sep 15, 2011 at 09:32:47AM -0700, Ethan Jackson wrote:
> looks good
Thanks for all the reviews. I pushed these four patches.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Yes, "make distcheck" checks "make uninstall". I tested it that way.
Thanks,
Ben.
On Thu, Sep 15, 2011 at 09:32:30AM -0700, Ethan Jackson wrote:
> looks fine assuming you tested it
>
> On Thu, Aug 25, 2011 at 10:22, Ben Pfaff wrote:
> > This works toward making "make distcheck" succeed.
> > -
On Thu, Sep 15, 2011 at 09:25:37AM -0700, Ethan Jackson wrote:
> looks good
Thanks for the reviews. I pushed these.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Thu, Sep 15, 2011 at 10:45:29AM -0700, Jesse Gross wrote:
> On Thu, Sep 15, 2011 at 10:41 AM, Ben Pfaff wrote:
> > Most netdev provider functions are allowed to be null if the implementation
> > does not support this feature. ??This commit adds this feature for get_mtu
> > and set_mtu, and chan
Bug #7332.
Reported-by: Gordon Good
---
AUTHORS |1 +
utilities/ovs-vsctl.c |6 +++---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index 44311ff..eefffee 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -64,6 +64,7 @@ DK Moon dkm...@
On Thu, Sep 15, 2011 at 10:41 AM, Ben Pfaff wrote:
> Most netdev provider functions are allowed to be null if the implementation
> does not support this feature. This commit adds this feature for get_mtu
> and set_mtu, and changes netdev-vport to take advantage of it.
>
> Also, changes netdev_get
On Thu, Sep 15, 2011 at 12:22 AM, Justin Pettit wrote:
> diff --git a/datapath/vport.c b/datapath/vport.c
> index b1418a4..c0bb585 100644
> --- a/datapath/vport.c
> +++ b/datapath/vport.c
> @@ -301,16 +301,9 @@ int vport_set_mtu(struct vport *vport, int mtu)
> if (mtu < 68)
>
Most netdev provider functions are allowed to be null if the implementation
does not support this feature. This commit adds this feature for get_mtu
and set_mtu, and changes netdev-vport to take advantage of it.
Also, changes netdev_get_mtu() to report an MTU of 0 on error, instead of
leaving the
On Thu, Sep 15, 2011 at 12:22:35AM -0700, Justin Pettit wrote:
> Currently the kernel automatically sets the MTU of any internal
> interfaces to the minimum of all attached interfaces because the Linux
> bridge does this. Userspace can do this with more knowledge and
> flexibility.
>
> Feature #7
On Wed, Sep 14, 2011 at 5:55 PM, Ethan Jackson wrote:
> diff --git a/include/openvswitch/datapath-protocol.h
> b/include/openvswitch/datapath-protocol.h
> index 5687792..e2fa578 100644
> --- a/include/openvswitch/datapath-protocol.h
> +++ b/include/openvswitch/datapath-protocol.h
> @@ -202,6 +202
On Wed, Sep 14, 2011 at 05:55:34PM -0700, Ethan Jackson wrote:
> The nl_lookup_genl_mcgroup() function can fail on older kernels
> which do not support the required netlink interface. Before this
> patch, dpif-linux would refuse to create a datapath when this
> happened. With this patch, it attem
On Wed, Sep 14, 2011 at 05:55:33PM -0700, Ethan Jackson wrote:
> Before this patch, if dpif-linux failed to register a notifier it
> would give up opening the datapath entirely. This seems draconian
> as a dpif can still perform the majority of its intended
> functionality without vport notificati
On Wed, Sep 14, 2011 at 05:55:32PM -0700, Ethan Jackson wrote:
> Older kernels do not advertise the multicast groups of families
> when requested by userspace. As a workaround, this patch hardcodes
> the multicast group ID of the ovs_vport family on these kernels.
> Userspace will be able to fall
On Wed, Sep 14, 2011 at 05:55:31PM -0700, Ethan Jackson wrote:
> Quite a bit of code in OVS ignores the return code of
> nln_notifier_register() and will nln_notifier_unregister() on
> notifiers which failed to register in the first place. This can
> cause segmentation faults.
>
> Instead of forc
looks good
On Thu, Aug 25, 2011 at 10:22, Ben Pfaff wrote:
> ---
> utilities/bugtool/automake.mk | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/utilities/bugtool/automake.mk b/utilities/bugtool/automake.mk
> index 622e5aa..98b6db4 100644
> --- a/utilities/bugtool/a
looks fine assuming you tested it
On Thu, Aug 25, 2011 at 10:22, Ben Pfaff wrote:
> This works toward making "make distcheck" succeed.
> ---
> utilities/bugtool/automake.mk | 12
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/utilities/bugtool/automake.mk b/u
looks good
On Thu, Aug 25, 2011 at 10:22, Ben Pfaff wrote:
> ---
> Makefile.am | 4 +++-
> ovsdb/ovsdbmonitor/automake.mk | 2 +-
> python/ovs/automake.mk | 2 +-
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> i
seems fine
On Thu, Aug 25, 2011 at 10:22, Ben Pfaff wrote:
> We want to regenerate the RPM spec files whenever the version number
> changes, hence the dependency on config.status. But that means that we
> try to modify the spec files even when the version number doesn't change,
> which cause "ma
looks good
On Thu, Sep 8, 2011 at 11:40, Ben Pfaff wrote:
> Seems like a very small cleanup.
> ---
> ofproto/ofproto.c | 3 +--
> ofproto/pktbuf.c | 10 --
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 849a376..090
looks good
On Thu, Sep 8, 2011 at 11:40, Ben Pfaff wrote:
> Commit d1e9b9bf3 "nicira-ext: Renumber NXT_FLOW_MOD_TABLE_ID" eliminated
> the need for the NXT_SET_FLOW_FORMAT and NXT_FLOW_MOD_TABLE_ID commands to
> have different sizes, so asserting that they are different isn't useful
> anymore (al
looks good
On Thu, Sep 8, 2011 at 11:40, Ben Pfaff wrote:
> ---
> lib/odp-util.c | 4
> 1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index b42a03c..5b86b47 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -989,10 +989,6 @@ o
Currently the kernel automatically sets the MTU of any internal
interfaces to the minimum of all attached interfaces because the Linux
bridge does this. Userspace can do this with more knowledge and
flexibility.
Feature #7323
Signed-off-by: Justin Pettit
---
datapath/datapath.c | 4
77 matches
Mail list logo
