On Oct 1, 2013 12:25 AM, "Ben Pfaff" wrote:
>
> On Mon, Sep 30, 2013 at 08:50:50AM +0400, Vasiliy Tolstov wrote:
> > Hi, all. I'm try to build deb package for openvswitch 2.0.
> > Now build fails:
> >
https://launchpadlibrarian.net/151884888/buildlog_ubuntu-precise-amd64.openvswitch_2.0~2013093008
On Tue, Oct 01, 2013 at 03:15:22PM +0900, Simon Horman wrote:
> Use nl_attr_get_u32() instead of nl_attr_get_be32() to parse nla
> so that the decoded value which is passed to mhash_add()
> is host byte order as mhash_add() expects.
>
> This resolves a minor regression introduced by
> 10e576406c74
To use ovs-gso-compatibility we need to record inner skb offset.
In case of vxlan it is done before vlan header is pushed which
gives wrong inner packet to ovs-gso.
Following patch reset skb offsets after inner skb is completely built.
Signed-off-by: Pravin B Shelar
---
datapath/linux/compat/vxl
On Mon, Sep 30, 2013 at 07:27:07PM -0700, Duffie Cooley wrote:
> From: Duffie Cooley
> This is a fix for a request to make sure that the openvswitch status command
> in rhel based distros gives a useful exit status. That was fixed in
>
> commit 5e0c05bc058c78a11be6747f62e6ad88e5d06b70
> debian: F
On Mon, Sep 30, 2013 at 6:47 PM, Duffie Cooley wrote:
> From: Duffie Cooley
>
> This pulls some of the changes to the debian init script into the rhel init
> script. Specifically this is a fix for a request to make sure that the
> openvswitch status command in rhel based distros gives a useful e
On Sep 30, 2013, at 5:46 PM, Jesse Gross wrote:
> On Fri, Sep 20, 2013 at 1:04 AM, pritesh wrote:
>> This patch adds support for Network Service Headers (nsh) over VXLAN
>> as mentioned in [1]. Here changes are made to datapath to add nsh
>> headers whenever a vxlan port with destination port as
On Mon, Sep 30, 2013 at 7:27 PM, Duffie Cooley wrote:
> From: Duffie Cooley
> This is a fix for a request to make sure that the openvswitch status command
> in rhel based distros gives a useful exit status. That was fixed in
>
> commit 5e0c05bc058c78a11be6747f62e6ad88e5d06b70
> debian: Fix exit s
On Mon, Sep 30, 2013 at 3:16 PM, Ethan Jackson wrote:
> Why do we need the bfd->last_tx change? Should it be in a different
> patch with a commit message explaining it?
>
I'll send a separate patch for it.
> As discussed off list, the monitor_seq thing is a layering violation.
> Instead we
On Tue, Oct 1, 2013 at 10:38 AM, Alex Wang wrote:
>
> I don't really like the function name monitor_handler(). It's not
>> really handling anything in the same sense that the miss_handler is.
>> I think we should rename the function interface_monitor(), and
>> set_subprogram_name("interface_moni
On Oct 1, 2013, at 10:07 AM, Pritesh Kothari (pritkoth)
wrote:
>
> On Sep 30, 2013, at 5:46 PM, Jesse Gross wrote:
>
>> On Fri, Sep 20, 2013 at 1:04 AM, pritesh wrote:
>>> This patch adds support for Network Service Headers (nsh) over VXLAN
>>> as mentioned in [1]. Here changes are made to d
On Tue, Oct 1, 2013 at 8:24 AM, Pravin B Shelar wrote:
> To use ovs-gso-compatibility we need to record inner skb offset.
> In case of vxlan it is done before vlan header is pushed which
> gives wrong inner packet to ovs-gso.
> Following patch reset skb offsets after inner skb is completely built.
> I still feel it is not very clear to name it interface_monitor while the
> module is called ofproto_dpif_monitor. Can I use monitor_main()? and just
> set the name to "monitor". The set_subprogram_name cannot exceeds 16 char's
Fine with me.
Ethan
__
> Yes, I originally worried about the behavior of creating a thread while
> holding the xlate_rwlock & monitor_rwlock. After check with Ben, I'm good
> to modify it.
Why can't you simply release the lock before creating the thread?
Ethan
___
dev mailin
On Tue, Oct 1, 2013 at 11:12 AM, Jesse Gross wrote:
> On Tue, Oct 1, 2013 at 8:24 AM, Pravin B Shelar wrote:
>> To use ovs-gso-compatibility we need to record inner skb offset.
>> In case of vxlan it is done before vlan header is pushed which
>> gives wrong inner packet to ovs-gso.
>> Following p
As discussed off list, there can be deadlock when starting/terminating
monitor thread while holding monitor_rwlock.
So, I'll go back and check thread creation/termination in ofproto-dpif.c
On Tue, Oct 1, 2013 at 11:17 AM, Ethan Jackson wrote:
> > Yes, I originally worried about the behavior of
Just a comment: one of the architectural differences between IPFIX and sFlow
is the location of the flow-cache: with sFlow the cache is implemented in
external software on a central server, which can then easily export IPFIX if
required (while also providing real-time network-wide visibility
This patch works for me, but it is a little hacky.
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 1684ddc..b0cdaf2 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -51,7 +51,13 @@ ovs_ctl () {
;;
*)
echo "`date -u`:$@" >> "${logdir}/ovs
On Tue, Oct 1, 2013 at 10:07 AM, Pritesh Kothari (pritkoth)
wrote:
> On Sep 30, 2013, at 5:46 PM, Jesse Gross wrote:
>> One other thing - would you mind using your full name in the signed-off-by
>> line?
>
> i generally prefer just my first name, but if its needed i can use my full
> name here.
On Oct 1, 2013, at 12:08 PM, Jesse Gross wrote:
> On Tue, Oct 1, 2013 at 10:07 AM, Pritesh Kothari (pritkoth)
> wrote:
>> On Sep 30, 2013, at 5:46 PM, Jesse Gross wrote:
>>> One other thing - would you mind using your full name in the signed-off-by
>>> line?
>>
>> i generally prefer just my fi
On Fri, Sep 27, 2013 at 3:04 PM, Ben Pfaff wrote:
> On Wed, Sep 25, 2013 at 09:42:28AM -0700, Jarno Rajahalme wrote:
>> Widen TCP flags handling from 7 bits (uint8_t) to 12 bits (uint16_t).
>> The kernel interface remains at 8 bits, which makes no functional
>> difference now, as none of the highe
This commit adds an extern sequence number 'timewarp_seq' in timeval,
so that threads other than main thread can wait on this sequence number
and be waken up when time is warped.
Signed-off-by: Alex Wang
---
lib/timeval.c |8
lib/timeval.h |3 +++
2 files changed, 11 insertions(
This commit moves the main logic of send_packet() function into
the ofproto-dpif-xlate module. Also, modification is made to
guarantee the thread safety of ofproto-dpif-xlate module.
Signed-off-by: Alex Wang
---
ofproto/ofproto-dpif-xlate.c | 72 +++---
ofp
This commit makes the update of 'stats' member in ofproto_dpif
struct thread safe.
Signed-off-by: Alex Wang
---
ofproto/ofproto-dpif.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6420b60..15c4dd3 100644
--- a/ofproto/ofproto
This series of patches implements the multi-threading bfd/cfm in
ofproto-dpif-monitor module. This module is in charge of the
execution of periodic functions (like bfd_run, cfm_run,
bfd_send_packets).
Alex Wang (5):
ofproto-dpif: Move send_packet() to ofproto-dpif-xlate module.
ofproto-dpif:
This commit moves the ofproto-dpif-monitor module into a
dedicated thread. This helps eliminate the burden of main
thread having to wake up very frequently for periodic
interface monitoring (bfd, cfm). Also, this commit greatly
increases the number of bfd/cfm sessions that can be supported
by ovs
This commit adds a new module ofproto-dpif-monitor in ofproto
directory. This module is in charge of executing the periodic
functions of monitoring code (e.g. bfd and cfm).
Signed-off-by: Alex Wang
---
ofproto/automake.mk|2 +
ofproto/ofproto-dpif-monitor.c | 203 ++
On Oct 1, 2013, at 11:03 AM, Jarno Rajahalme wrote:
>
> On Oct 1, 2013, at 10:07 AM, Pritesh Kothari (pritkoth)
> wrote:
>
>>
>> On Sep 30, 2013, at 5:46 PM, Jesse Gross wrote:
>>
>>> On Fri, Sep 20, 2013 at 1:04 AM, pritesh wrote:
This patch adds support for Network Service Headers (
On Wed, Sep 25, 2013 at 9:42 AM, Jarno Rajahalme wrote:
> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> index 09c26b5..2642ca5 100644
> --- a/include/linux/openvswitch.h
> +++ b/include/linux/openvswitch.h
> @@ -291,6 +291,8 @@ enum ovs_key_attr {
> OVS_KEY_ATTR_
Hides mega-flow implementation in flow_table.c rather than
datapath.c. This also helps next patch.
Signed-off-by: Pravin B Shelar
---
v4:
Fixes dues to name changes.
v3:
No change.
v2:
No change.
---
datapath/datapath.c | 27 +++---
datapath/flow_table.c | 138 +
On Mon, Sep 30, 2013 at 2:27 PM, Jesse Gross wrote:
> Some very quick high level things while I look at this:
>
> On Mon, Sep 30, 2013 at 1:01 PM, Pravin B Shelar wrote:
>> Following patch breaks down ovs_mutex into multiple locks. This
>> patch specifically targets flow-install. By breaking do
ovs-flow rehash does not touch mega flow list. Following patch
moves it dp struct datapath. Avoid one extra indirection for
accessing mega-flow list head on every packet receive.
Signed-off-by: Pravin B Shelar
---
v4:
rename hash_table to table_instance.
other fixes according to comments from Je
On Sep 20, 2013, at 1:04 AM, pritesh wrote:
> This patch adds support for Network Service Headers (nsh) over VXLAN
> as mentioned in [1]. Here changes are made to datapath to add nsh
> headers whenever a vxlan port with destination port as 9030 is created.
> IANA port allocation for nsh over vxl
On Sep 20, 2013, at 1:04 AM, pritesh wrote:
> NSH service path (nsp) can be set/unset while creating the port
> as well nsp can be matched on incoming packets.
>
> Signed-off-by: pritesh
>
>
...
> +++ b/lib/meta-flow.c
> @@ -694,7 +694,24 @@ static const struct mf_field mf_fields[MFF_N_IDS]
On Sep 20, 2013, at 1:04 AM, pritesh wrote:
> Support for setting nsp using an action namely set_nsp. It works similar to
> set_tunnel in vxlan/gre tunnel and can be used to set the outgoing nsh
> service path id.
Should tell here that NXM_NX_NSP is also defined, which enables
matching NSPs.
A
On Sep 20, 2013, at 1:04 AM, pritesh wrote:
> Here, datapath support for setting nsh service index (nsi) is added.
> nsi can now be set on outgoing packet depending on port settings or
> set_nsi action.
>
> Signed-off-by: pritesh
>
...
> diff --git a/datapath/flow.c b/datapath/flow.c
> index
On Sep 20, 2013, at 1:04 AM, pritesh wrote:
> Support for nsh service index (nsi) is added, mainly incoming
> nsi in a flow can be matched and appropriate action can be
> taken on the flow based on it.
>
> Signed-off-by: pritesh
>
>
...
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> ind
On Sep 20, 2013, at 1:04 AM, pritesh wrote:
> Support for setting nsi using an action namely set_nsi. It works similar to
> set_tunnel in vxlan/gre tunnel and can be used to set the outgoing nsh
> service index (nsi).
>
Again, a new action is not necessary, "load", "move", and "set_field" can
I pushed patch to master and branch-2.0.
On Tue, Oct 1, 2013 at 11:12 AM, Jesse Gross wrote:
> On Tue, Oct 1, 2013 at 8:24 AM, Pravin B Shelar wrote:
>> To use ovs-gso-compatibility we need to record inner skb offset.
>> In case of vxlan it is done before vlan header is pushed which
>> gives wro
On Sep 20, 2013, at 1:04 AM, pritesh wrote:
> Support for setting the nsh network and service contexts is added here.
> The support is added only for adding context while the port is created.
I understand that this may be an intermediate step, but how are these
useful as part of the tunnel port
What about this?
referenced from here.
http://unix.stackexchange.com/questions/14270/get-exit-status-of-process-thats-piped-to-another/70675#70675
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 1684ddc..5286e12 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -41,6
That is pretty horrible. I was trying to construct something similar
manually, but you finished it off.
I suggest adding spaces between the initial s, to avoid this
pitfall described at
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18:
If a character sequen
Why the redirect from descriptor 4?
:: psi
On Oct 1, 2013, at 3:13 PM, Duffie Cooley wrote:
> What about this?
>
> referenced from here.
>
> http://unix.stackexchange.com/questions/14270/get-exit-status-of-process-thats-piped-to-another/70675#70675
>
> diff --git a/utilities/ovs-lib.in b/uti
On Tue, Oct 01, 2013 at 01:36:11PM -0700, Alex Wang wrote:
> This commit moves the main logic of send_packet() function into
> the ofproto-dpif-xlate module. Also, modification is made to
> guarantee the thread safety of ofproto-dpif-xlate module.
>
> Signed-off-by: Alex Wang
xlate_send_packet(
> xlate_send_packet() locks xlate_rwlock before it looks up the xport,
> but at the end it unlocks xlate_rwlock before it dereferences
> xport->xbridge->dpif. Is the latter dereference safe, that is, does
> anything guarantee that 'xport' and 'xbridge' aren't destroyed?
No that's definitely not s
On Tue, Oct 01, 2013 at 03:41:17PM -0700, Ethan Jackson wrote:
> > xlate_send_packet() locks xlate_rwlock before it looks up the xport,
> > but at the end it unlocks xlate_rwlock before it dereferences
> > xport->xbridge->dpif. Is the latter dereference safe, that is, does
> > anything guarantee t
Technically I don't think it is because the dpif could be destroyed as
soon as the xlate_rwlock is released. In practice this seems pretty
unlikely. If we wanted to do that we'd have to ref count the dpif,
which may not be a bad idea anyway.
Ethan
On Tue, Oct 1, 2013 at 3:45 PM, Ben Pfaff wrot
On Tue, Oct 01, 2013 at 01:36:12PM -0700, Alex Wang wrote:
> This commit makes the update of 'stats' member in ofproto_dpif
> struct thread safe.
>
> Signed-off-by: Alex Wang
Should this also annotate the 'stats' member as OVS_GUARDED?
Thanks,
Ben.
_
Yes, I'll add it, Thanks
On Tue, Oct 1, 2013 at 3:49 PM, Ben Pfaff wrote:
> On Tue, Oct 01, 2013 at 01:36:12PM -0700, Alex Wang wrote:
> > This commit makes the update of 'stats' member in ofproto_dpif
> > struct thread safe.
> >
> > Signed-off-by: Alex Wang
>
> Should this also annotate the '
I agree with Ethan, in the worst case, xport could be destroyed when
dereferenced.
I can add reference counter to dpif. Should I do that?
On Tue, Oct 1, 2013 at 3:47 PM, Ethan Jackson wrote:
> Technically I don't think it is because the dpif could be destroyed as
> soon as the xlate_rwlock is
On Tue, Oct 01, 2013 at 03:57:59PM -0700, Alex Wang wrote:
> I can add reference counter to dpif. Should I do that?
Can we just unlock later?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
The logic that I was using is that either send_packet() is being
called from some thread other than the main one, in which case the
dpif is not being destroyed (because we would kill those threads
before destroying the dpif) or from the main thread, in which case
the dpif is not being destroyed (be
> Can we just unlock later?
+1 let's just go for simple now. If it actually turns out to matter
we can change it, but since we're only holding the read lock I doubt
it will matter.
Ethan
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/m
>From the reference.
Lets examine that step by step.
|( ( ( ( someprog; #part6
echo $? >&3#part5
) | filter >&4 #part4
) 3>&1 #part3
) | stdintoexitstatus#part2
) 4>&1 #part1|
>From bottom up:
1. A subshell is c
On Mon, Sep 30, 2013 at 11:47 PM, Simon Horman wrote:
> diff --git a/datapath/actions.c b/datapath/actions.c
> index d961e5d..bfab9ec 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> +/* Push MPLS after the ethernet header. */
> +static int push_mpls(struct sk_buff *skb,
> +
To make sure I understand it right. You means: The main thread must delete
all bridges (removing cfm/bfd on all ports, => remove the monitor thread)
before removing the dpif.
Do i understand it right?
If so, I think "dpif = xport->xbridge->dpif inside the lock" is thread-safe.
On Tue, Oct 1, 20
On Tue, Oct 01, 2013 at 04:09:17PM -0700, Alex Wang wrote:
> To make sure I understand it right. You means: The main thread must delete
> all bridges (removing cfm/bfd on all ports, => remove the monitor thread)
> before removing the dpif.
>
> Do i understand it right?
Yes.
> If so, I think "dpi
OK, but I don't see the redirect to descriptor 4 in the real code (it's
obviously there in the example).
:: psi
On Oct 1, 2013, at 3:58 PM, Duffie Cooley wrote:
> From the reference.
>
> Lets examine that step by step.
>
> ( ( ( ( someprog; #part6
> echo $? >&3#part
On Tue, Oct 01, 2013 at 01:36:13PM -0700, Alex Wang wrote:
> This commit adds an extern sequence number 'timewarp_seq' in timeval,
> so that threads other than main thread can wait on this sequence number
> and be waken up when time is warped.
>
> Signed-off-by: Alex Wang
Could timeval export a
On Sep 24, 2013, at 9:09 PM, Ben Pfaff wrote:
> On Tue, Sep 24, 2013 at 04:41:43PM -0700, Justin Pettit wrote:
>> On Sep 24, 2013, at 2:11 PM, Ben Pfaff wrote:
>>
>>> On Mon, Sep 23, 2013 at 02:36:39PM -0700, Justin Pettit wrote:
Signed-off-by: Justin Pettit
>>>
>>> Should the tcpdump p
Thanks Ben for the review,
Could timeval export a function e.g. "timewarp_wait()" rather than a
> variable here?
>
It would be even better if threads didn't have to explicitly wait on
> this. Maybe timeval or the poll loop should automatically wake up all
> threads when time warping happens.
>
On Tue, Oct 01, 2013 at 01:36:14PM -0700, Alex Wang wrote:
> This commit adds a new module ofproto-dpif-monitor in ofproto
> directory. This module is in charge of executing the periodic
> functions of monitoring code (e.g. bfd and cfm).
>
> Signed-off-by: Alex Wang
The comment here might more
On Tue, Oct 01, 2013 at 04:02:17PM -0700, Jesse Gross wrote:
> On Mon, Sep 30, 2013 at 11:47 PM, Simon Horman wrote:
> > diff --git a/datapath/actions.c b/datapath/actions.c
> > index d961e5d..bfab9ec 100644
> > --- a/datapath/actions.c
> > +++ b/datapath/actions.c
> > +/* Push MPLS after the ethe
On Tue, Oct 1, 2013 at 5:40 PM, Simon Horman wrote:
> On Tue, Oct 01, 2013 at 04:02:17PM -0700, Jesse Gross wrote:
>> On Mon, Sep 30, 2013 at 11:47 PM, Simon Horman wrote:
>> > @@ -545,6 +662,14 @@ static int do_execute_actions(struct datapath *dp,
>> > struct sk_buff *skb,
>> >
On Tue, Oct 01, 2013 at 04:00:25PM -0700, Ethan Jackson wrote:
> > Can we just unlock later?
>
> +1 let's just go for simple now. If it actually turns out to matter
> we can change it, but since we're only holding the read lock I doubt
> it will matter.
Yeah, like I said I was trying to explore
Yeah I still vote for simple. Even if it works today, it's less
obviously correct and make break in future.
Ethan
On Tue, Oct 1, 2013 at 6:29 PM, Ben Pfaff wrote:
> On Tue, Oct 01, 2013 at 04:00:25PM -0700, Ethan Jackson wrote:
>> > Can we just unlock later?
>>
>> +1 let's just go for simple no
btw, first and foremost, thanks for the review, i greatly appreciate it :)
>> diff --git a/datapath/flow.c b/datapath/flow.c
>> index 29122af..4f47a48 100644
>> --- a/datapath/flow.c
>> +++ b/datapath/flow.c
>> @@ -1235,6 +1235,7 @@ int ovs_ipv4_tun_from_nlattr(const struct nlattr *attr,
>> i
>>
>> +{
>> +MFF_NSP, "nsp", NULL,
>> +sizeof(ovs_be32), 24,
>> +MFM_FULLY,
>> +MFS_HEXADECIMAL,
>> +MFP_NONE,
>> +false,
>> +0, NULL,
>> +0, NULL,
>> +OFPUTIL_P_OF10_NXM_ANY,
>> +OFPUTIL_P_OF10_NXM_ANY,
>
> These
> On Sep 20, 2013, at 1:04 AM, pritesh wrote:
>
>> Support for setting nsp using an action namely set_nsp. It works similar to
>> set_tunnel in vxlan/gre tunnel and can be used to set the outgoing nsh
>> service path id.
>
> Should tell here that NXM_NX_NSP is also defined, which enables
> match
>
>> diff --git a/datapath/flow.c b/datapath/flow.c
>> index 4f47a48..4335d67 100644
>> --- a/datapath/flow.c
>> +++ b/datapath/flow.c
>> @@ -46,6 +46,8 @@
>>
>> #include "vlan.h"
>>
>> +#define NSH_M_NSI 0x00FF
>> +
>
> Is this the same as NSH_M_NSI currently defined in nsh.h?
>
>> stat
On Oct 1, 2013, at 3:07 PM, Jarno Rajahalme wrote:
>
> On Sep 20, 2013, at 1:04 AM, pritesh wrote:
>
>> Support for nsh service index (nsi) is added, mainly incoming
>> nsi in a flow can be matched and appropriate action can be
>> taken on the flow based on it.
>>
>> Signed-off-by: pritesh
> On Oct 1, 2013, at 6:36 PM, "Pritesh Kothari (pritkoth)"
> wrote:
>
>> Also, the usual treatment of reserved bits is ignore on reception, rather
>> than
>> dropping.
>
> hmm. i didn't quite get what you are saying here, vxlan_vni and svc_path both
> use upper 24bits
> and their detection s
>> vxlan.c:118: if (vxh->vx_flags != htonl(VXLAN_FLAGS) || (vxh->vx_vni &
>> htonl(0xff))) {
>>
>> or if (unlikely(nsh->b.svc_path & htonl(NSH_M_NSI))) { // NSH_M_NSI ==>
>> 0x00ff which is same as 0xff above?
>> am i missing something here?
>>
>
> I did not check if the NSH spec tells to
On Oct 1, 2013, at 3:09 PM, Jarno Rajahalme wrote:
>
> On Sep 20, 2013, at 1:04 AM, pritesh wrote:
>
>> Support for setting nsi using an action namely set_nsi. It works similar to
>> set_tunnel in vxlan/gre tunnel and can be used to set the outgoing nsh
>> service index (nsi).
>>
>
> Again,
On Oct 1, 2013, at 3:13 PM, Jarno Rajahalme wrote:
>
> On Sep 20, 2013, at 1:04 AM, pritesh wrote:
>
>> Support for setting the nsh network and service contexts is added here.
>> The support is added only for adding context while the port is created.
>
> I understand that this may be an inter
GCC provides two useful builtin functions that can help
to improve array size checking during compilation.
This patch contains no functional changes, but it makes
it easier to detect mistakes.
Signed-off-by: Flavio Leitner
---
lib/util.h | 17 -
1 file changed, 16 insertions(+),
75 matches
Mail list logo
