Re: [ovs-dev] [PATCH v3 07/16] ofproto-dpif: Bonding with in_port that isn't present in the datapath
Simon, I thought this test exposed a bug that is fixed with this patch series. However, when I apply this patch alone against current master, it passed fine. (I tried many times). So I must misunderstand the intention of the test, or why current master failed to address packet out. Would you please shed some light on this? On Tue, Apr 22, 2014 at 1:54 AM, Simon Horman wrote: > This tests exercises execution of actions in ovs-vswitchd > in the case where a packet is processed due to a packet out message > with an in_port that doesn't exist in the datapath and translation > results in recirc actions due to bonding. > > Signed-off-by: Simon Horman > --- > tests/ofproto-dpif.at | 80 > +++ > 1 file changed, 80 insertions(+) > > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > index a6bfa1a..72f1a24 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -197,6 +197,86 @@ AT_CHECK([test `grep in_port.6 br1_flows.txt |wc -l` -gt > 7]) > OVS_VSWITCHD_STOP() > AT_CLEANUP > > +AT_SETUP([ofproto-dpif, balance-tcp bonding, packet-out]) > +# Create br0 with interfaces bond0(p1, p2, p3) and bond1(p4,p5,p6), > +#br1 with interfaces bond10(p11, p12, p13) and bond11(p14,p15,p16) > and > +#br2 with interfaces bond20(p21, p22, p23) and bond21(p24,p25,p26) > +#bond0 <-> bond10 > +#bond1 <-> bond20 > +# Send some traffic, make sure the traffic are spread based on L4 headers. > +OVS_VSWITCHD_START( > + [add-bond br0 bond0 p1 p2 p3 bond_mode=balance-tcp lacp=active \ > +other-config:lacp-time=fast other-config:bond-rebalance-interval=0 > --\ > + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock > ofport_request=1 -- \ > + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock > ofport_request=2 -- \ > + set interface p3 type=dummy options:pstream=punix:$OVS_RUNDIR/p3.sock > ofport_request=3 -- \ > + add-bond br0 bond1 p4 p5 p6 bond_mode=balance-tcp lacp=active \ > +other-config:lacp-time=fast other-config:bond-rebalance-interval=0 > --\ > + set interface p4 type=dummy options:pstream=punix:$OVS_RUNDIR/p4.sock > ofport_request=4 -- \ > + set interface p5 type=dummy options:pstream=punix:$OVS_RUNDIR/p5.sock > ofport_request=5 -- \ > + set interface p6 type=dummy options:pstream=punix:$OVS_RUNDIR/p6.sock > ofport_request=6 -- \ > + add-br br1 -- \ > + set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \ > + set bridge br1 datapath-type=dummy other-config:datapath-id=1234 \ > + fail-mode=secure -- \ > + add-bond br1 bond10 p11 p12 p13 bond_mode=balance-tcp lacp=active \ > +other-config:lacp-time=fast other-config:bond-rebalance-interval=0 > --\ > + set interface p11 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock > ofport_request=1 -- \ > + set interface p12 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock > ofport_request=3 -- \ > + set interface p13 type=dummy options:stream=unix:$OVS_RUNDIR/p3.sock > ofport_request=2 -- \ > + add-bond br1 bond11 p14 p15 p16 bond_mode=balance-tcp lacp=active \ > +other-config:lacp-time=fast other-config:bond-rebalance-interval=0 > --\ > + set interface p14 type=dummy options:stream=unix:$OVS_RUNDIR/p7.sock > ofport_request=4 -- \ > + set interface p15 type=dummy options:stream=unix:$OVS_RUNDIR/p8.sock > ofport_request=5 -- \ > + set interface p16 type=dummy options:stream=unix:$OVS_RUNDIR/p9.sock > ofport_request=6 -- \ > + add-br br2 -- \ > + set bridge br2 other-config:hwaddr=aa:66:aa:66:00:02 -- \ > + set bridge br2 datapath-type=dummy other-config:datapath-id=1235 \ > + fail-mode=secure -- \ > + add-bond br2 bond20 p21 p22 p23 bond_mode=balance-tcp lacp=active \ > +other-config:lacp-time=fast other-config:bond-rebalance-interval=0 > --\ > + set interface p21 type=dummy options:stream=unix:$OVS_RUNDIR/p4.sock > ofport_request=1 -- \ > + set interface p22 type=dummy options:stream=unix:$OVS_RUNDIR/p5.sock > ofport_request=2 -- \ > + set interface p23 type=dummy options:stream=unix:$OVS_RUNDIR/p6.sock > ofport_request=3 -- ]) > +AT_CHECK([ovs-appctl netdev-dummy/set-admin-state up], 0, [OK > +]) > +AT_CHECK([ovs-ofctl add-flow br1 action=normal]) > +AT_CHECK([ovs-ofctl add-flow br2 action=normal]) > +AT_CHECK([ovs-appctl upcall/disable-megaflows], [0], [megaflows disabled > +], []) > +sleep 1; > +ovs-appctl time/stop > +ovs-appctl time/warp 100 > +ovs-appctl lacp/show > lacp.txt > +ovs-appctl bond/show > bond.txt > + > +dnl The input is a TCP/IP frame which tcpdump -vve shows as: > +dnl 60:66:66:66:00:01 > 50:54:00:00:00:07, ethertype IPv4 (0x0800), length > 58: (tos 0x0, ttl 255, id 0, offset 0, flags [none], proto TCP (6), length 44) > +dnl 192.168.0.1.80 > 192.168.0.2.$port: Flags [none], cksum 0x7744 > (correct), seq 42:46, win 1, length 4 > +( > +for i in `seq 0 255` ; > +do > +
Re: [ovs-dev] [PATCH v2.56] datapath: Add basic MPLS support to kernel
> On Thu, Apr 24, 2014 at 05:57:29PM +0900, YAMAMOTO Takashi wrote: >> hi, >> >> > + * Due to the sample action there may be multiple possible eth types. >> > + * In order to correctly validate actions all possible types are tracked >> > + * and verified. This is done using struct eth_types. >> >> is there any real-world use cases of these actions inside a sample? >> otherwise, how about just rejecting such combinations? >> it doesn't seem to worth the code complexity to me. >> (sorry if it has been already discussed. it's the first time for me >> to seriously read this long-lived patch.) > > Good point, the code is rather complex. > > My understanding is that it comes into effect in the case > of sflow or ipfix being configured on the bridge. I tend > to think these are real-world use-cases, though that thinking > is by no means fixed. > > My reading of the code is that in the case of sflow and ipfix a single > sample rule appears at the beginning of the flow. And that it may be > possible to replace the code that you are referring to with something > simpler to handle these cases. it seems that they put only a userland action inside a sample. it's what i expected from its name "sample". > > My understanding is that the code you are referring to also comes into > effect when a sample action (a Nicira extension) is used directly in a > rule. I am less sure that this is a real-world case but the complex logic > you are referring to should to handle this use-case. probably nicira folks can clarify? YAMAMOTO Takashi ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 3/3] datapath: add layer 3 flow/port support
On 4/25/14, 5:19 AM, Jesse Gross wrote: +noethernet: if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, output->eth.type)) goto nla_put_failure; Does it still make sense to send the EtherType? It's not present in the packet and I believe that the information is contained in the attributes that we do send (i.e. IPv4 or v6 attributes). We had a discussion about this in August last year: One such decision is how to handle the flow key. I set all fields in key->eth to 0, except the type, because we still need to know what kind of L3 packet do we have. Since a lot of code is accessing key->eth.type, this is easier than having this information in a different place, although it would be more elegant to set this field to 0 as well. Can we use skb->protocol for the cases where we currently access the EtherType? Are there cases that you are particularly concerned about? The only case I'm concerned about is in the action validation code, particularly the validate_set() and validate_tp_port() functions, since they don't have access to the skb, and need to know the layer 3 protocol of the flow. In validate_set() we could relax the check for eth.type for OVS_KEY_ATTR_IPV4 and OVS_KEY_ATTR_IPV6, but I'm not sure what to do about validate_tp_port(). In general, I think it would be a good idea for the flow key to have a field specifying the layer 3 protocol, when an ethernet header is not present. That makes sense to me. You mean that we keep eth.type as the L3 protocol field, or define a new one, separate from the eth union? I think it's fine to keep using eth.type as the protocol field. I think we can probably some holes to move the is_layer3 flag into the non-tunnel portion of the flow though. Should we revisit this discussion? I was just referring to the Netlink encoding here. Can we populate the flow key in the kernel when we translate the flow? Not sure I understand the question. Going through the code, I see that omitting OVS_KEY_ATTR_ETHERTYPE currently means an 802.2 packet, if the mask is set to 0x. Are you suggesting to omit OVS_KEY_ATTR_ETHERTYPE for layer 3 packets both in the flow key and the mask? All I was trying to say is that is that the Netlink and struct sw_flow_key representations don't necessarily have to be exactly the same as long as we can translate back and forth. I'm not sure that the previous discussion applies - this is more about Netlink and that seemed to apply to struct sw_flow_key. Ok, understand now. But validation would become even more complicated if we didn't send the OVS_KEY_ATTR_ETHERTYPE over Netlink for layer 3 packets. I think we would basically have to scan for L3 attributes and use that to fill in the EtherType, right? Is there anything else? It would be nice to fully break the link to Ethernet and it seems like a half measure if we keep EtherType in. There have been various requests for non-Ethernet protocols in the past so it seems like it could get messy if we assume some elements of Ethernet. I had a discussion about this with Thomas Morin, who proposed the IP-over-GRE patches and MPLS-over-GRE patches based on this work, and we think it would still be useful to keep the EtherType, even in the Netlink attributes, to be able to support generic packet types in the future. Ben was involved in EXT-112 at the ONF, which has the objective to "Support non-Ethernet packets throughout the pipeline". The current version of the proposal, recently moved from incubation to prototyping, defines a new "packet type" match field, which is used to determine the type of the first header. There are 5 namespaces that can be used to define the packet type, one of which is EtherType. With this match type, the canonical representation for an IPv4 packet would be (namespace=1,ns_type=0x800), namespace 1 being the EtherType. Because of the potential future implementation of EXT-112 in OVS I think keeping the EtherType around would be beneficial. I'm starting to think that the path we went down with the megaflow Netlink encoding is not particularly sustainable because it means that while the keys can do something fairly reasonable, the masks must always list all the protocols that it is not, such as Ethernet and EtherType in this case. I wonder if it actually makes more sense to switch over to something similar to what Jarno is planning for the actions - a key and mask paired together and then we can also fix some left over oddities from the exact match days. I wonder if we could refine this in later patches? My hope is that we can at least get some other opinions on this first. Sure, looking forward to that! -Lori ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 3/3] datapath: add layer 3 flow/port support
Hi Jesse, (inlined below) Jesse Gross : >> >> In general, I think it would be a good idea for the flow key to >> have >> a >> field >> specifying the layer 3 protocol, when an ethernet header is not >> present. > > That makes sense to me. You mean that we keep eth.type as the L3 protocol field, or define a new one, separate from the eth union? >>> I think it's fine to keep using eth.type as the protocol field. I >>> think we can probably some holes to move the is_layer3 flag into the >>> non-tunnel portion of the flow though. >> >> Should we revisit this discussion? > I was just referring to the Netlink encoding here. Can we populate the > flow key in the kernel when we translate the flow? Not sure I understand the question. Going through the code, I see that omitting OVS_KEY_ATTR_ETHERTYPE currently means an 802.2 packet, if the mask is set to 0x. Are you suggesting to omit OVS_KEY_ATTR_ETHERTYPE for layer 3 packets both in the flow key and the mask? >>> All I was trying to say is that is that the Netlink and struct >>> sw_flow_key representations don't necessarily have to be exactly the >>> same as long as we can translate back and forth. I'm not sure that the >>> previous discussion applies - this is more about Netlink and that >>> seemed to apply to struct sw_flow_key. >> >> Ok, understand now. But validation would become even more complicated if we >> didn't send the OVS_KEY_ATTR_ETHERTYPE over Netlink for layer 3 packets. > I think we would basically have to scan for L3 attributes and use that > to fill in the EtherType, right? Is there anything else? > > It would be nice to fully break the link to Ethernet and it seems like > a half measure if we keep EtherType in. There have been various > requests for non-Ethernet protocols in the past so it seems like it > could get messy if we assume some elements of Ethernet. Assuming that the target is to carry any non-Ethernet payloads in the future, it seems like it is important to have a way of tracking what is the type of the payload. A natural way to do this is to rely on the ethertype value. Note that the use of an ethertype value is not, at least not anymore, an element of Ethernet; it is used by many protocols such as GRE. So keeping track of the ethertype of the payload looks to me as a very good idea. In the general case, this is something needed (even though in the particular case of IP, you can find what the ethertype would be based on the (IP) payload header). To break the link with ethernet, which I agree is a good thing, this value could be carried in a field other than dl_type. A practical example is the transport of MPLS over GRE. The vport-gre code would extract the ethertype from the GRE header, and the MPLS payload will then be processed further. If we did not preserve the knowledge of the fact that the payload is MPLS, it becomes impossible to parse the payload properly. Another example is for any non-Ethernet payload received of GRE that you need to forward on an Ethernet port, to push a valid Ethernet header, you need to have preserved the knowledge of the ethertype. Best, -Thomas _ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] revalidator: Fix ukey stats cache updating.
found this in the morning, will keep investigating,
./learn.at:362: (ovs-ofctl dump-ports br0 2; ovs-ofctl dump-ports br0 3) |
sed 's/ (xid=0x[0-9a-fA-F]*)//'
--- - 2014-04-25 01:25:43.022048477 -0700
+++ /root/openvswitch/tests/testsuite.dir/at-groups/328/stdout 2014-04-25
01:25:43.017711899 -0700
@@ -1,7 +1,7 @@
OFPST_PORT reply: 1 ports
port 2: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
- tx pkts=2, bytes=120, drop=0, errs=0, coll=0
+ tx pkts=6, bytes=360, drop=0, errs=0, coll=0
OFPST_PORT reply: 1 ports
port 3: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
- tx pkts=18, bytes=1080, drop=0, errs=0, coll=0
+ tx pkts=14, bytes=840, drop=0, errs=0, coll=0
On Thu, Apr 24, 2014 at 10:34 PM, Alex Wang wrote:
> i'll kick an overnight run on "learning action - self-modifying flow with
> idle_timeout".~ (seq 1). if it survives, tmr morning,
> i'll ack it~~
>
>
> On Thu, Apr 24, 2014 at 10:26 PM, Joe Stringer wrote:
>
>> revalidate_ukey() had a bug where it would update the ukey->stats even
>> if it decided not to push stats (as an optimisation). ukey->stats should
>> only be updated when those stats are pushed.
>>
>> This bug would arise in the following situation:
>> * A flow has been dumped before.
>> * The flow needs to be revalidated.
>> * The flow is low-throughput.
>> * The flow has new statistics to push.
>>
>> Such cases rely on flow deletion to update the stats. However, that code
>> pushes the delta between the ukey->stats and the final flow dump. If the
>> ukey stats cache is updated without the stats being pushed, those stats
>> would be lost.
>>
>> This caused intermittent testsuite failures on "learning action -
>> self-modifying flow with idle_timeout". Introduced by 698ffe3623f1b630ae
>> "revalidator: Only revalidate high-throughput flows."
>>
>> Bug #1238927.
>>
>> Signed-off-by: Joe Stringer
>> ---
>> ofproto/ofproto-dpif-upcall.c |3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>> index 84afc56..5871772 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -1245,7 +1245,6 @@ revalidate_ukey(struct udpif *udpif, struct
>> udpif_key *ukey,
>> push.n_bytes = stats->n_bytes > ukey->stats.n_bytes
>> ? stats->n_bytes - ukey->stats.n_bytes
>> : 0;
>> -ukey->stats = *stats;
>>
>> if (!ukey->flow_exists) {
>> /* Don't bother revalidating if the flow was already deleted. */
>> @@ -1258,6 +1257,8 @@ revalidate_ukey(struct udpif *udpif, struct
>> udpif_key *ukey,
>> goto exit;
>> }
>>
>> +/* We will push the stats, so update the ukey stats cache. */
>> +ukey->stats = *stats;
>> if (!push.n_packets && !udpif->need_revalidate) {
>> ok = true;
>> goto exit;
>> --
>> 1.7.10.4
>>
>>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] lacp: Don't lock potentially uninitialized mutex in lacp_status().
On Thu, Apr 24, 2014 at 11:03:16PM -0700, Andy Zhou wrote:
> Acked-by: Andy Zhou
>
> With a question inline.
>
>
> On Thu, Apr 24, 2014 at 4:59 PM, Ben Pfaff wrote:
> > If the 'lacp' parameter is nonnull, then we know that the file scope mutex
> > has been initialized, since that's done as a side effect of creating a
> > lacp object, but otherwise there's no guarantee.
> >
> > Signed-off-by: Ben Pfaff
> > ---
> > lib/lacp.c | 18 +-
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/lacp.c b/lib/lacp.c
> > index 4aee64f..49ae5e5 100644
> > --- a/lib/lacp.c
> > +++ b/lib/lacp.c
> > @@ -345,18 +345,18 @@ out:
> > enum lacp_status
> > lacp_status(const struct lacp *lacp) OVS_EXCLUDED(mutex)
> > {
> > -enum lacp_status ret;
> > +if (lacp) {
> > +enum lacp_status ret;
> >
> > -ovs_mutex_lock(&mutex);
> > -if (!lacp) {
> > -ret = LACP_DISABLED;
> > -} else if (lacp->negotiated) {
> > -ret = LACP_NEGOTIATED;
> > +ovs_mutex_lock(&mutex);
> > +ret = lacp->negotiated ? LACP_NEGOTIATED : LACP_CONFIGURED;
> > +ovs_mutex_unlock(&mutex);
> > +return ret;
> > } else {
> > -ret = LACP_CONFIGURED;
> > +/* Don't take 'mutex'. It might not even be initialized, since we
> > + * don't know that any lacp object has been created. */
> > +return LACP_DISABLED;
> This would have been hard to understand without the comment above.
> Thanks for adding them.
> I am curious why not just initialize the mutex in lacp_init()?
Hmm. lacp_init() isn't necessarily run at the right time, since it's
only to set up unixctl commands.
But just initializing more carefully does seem like a better solution.
I'll respin.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] ovs-thread: Add checking for mutex and rwlock initialization.
On Thu, Apr 24, 2014 at 11:27:47PM -0700, Andy Zhou wrote: > The logic looks good. 233 unit tests failed now. Apparently this > patch is great doing a great job on catching uninitialized mutex and > rwlocks. > > Assume the tests will be fixed before checking in: I did my testing with one of Guru's bug fixes related to locking from yesterday in my tree. No failures then. I'm planning to wait for him to apply those before I push. > Acked-by: Andy Zhou Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] ovs-thread: Add checking for mutex and rwlock initialization.
On Fri, Apr 25, 2014 at 8:46 AM, Ben Pfaff wrote: > On Thu, Apr 24, 2014 at 11:27:47PM -0700, Andy Zhou wrote: >> The logic looks good. 233 unit tests failed now. Apparently this >> patch is great doing a great job on catching uninitialized mutex and >> rwlocks. >> >> Assume the tests will be fixed before checking in: > > I did my testing with one of Guru's bug fixes related to locking from > yesterday in my tree. No failures then. I'm planning to wait for him > to apply those before I push. I applied them today morning. I suppose the first patch in this series needs to be checked in before this one goes in, otherwise the lacp test will fail? > >> Acked-by: Andy Zhou > > Thanks, > > Ben. > ___ > 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] ofproto: Inline actions in struct rule_actions.
Allocate struct rule_actions and the space for the actions at once.
This reduces one memory indirection and helps reduce cache misses
visible in perf annotations.
Fix some old comments referring to ref count, since we now use RCU for
this.
Enforce constness of the actions throughout the code.
Signed-off-by: Jarno Rajahalme
---
lib/ofp-actions.c|8
lib/ofp-actions.h|6 +++---
lib/ofp-parse.c |2 +-
lib/ofp-util.c |2 +-
lib/ofp-util.h | 10 +-
ofproto/connmgr.c|2 +-
ofproto/ofproto-dpif-xlate.c |4 ++--
ofproto/ofproto-dpif.c |4 ++--
ofproto/ofproto-dpif.h |2 +-
ofproto/ofproto-provider.h | 27 ++-
ofproto/ofproto.c| 38 +++---
utilities/ovs-ofctl.c| 10 +-
12 files changed, 54 insertions(+), 61 deletions(-)
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index ce14004..9abb383 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1899,7 +1899,7 @@ inconsistent_match(enum ofputil_protocol
*usable_protocols)
* Modifies some actions, filling in fields that could not be properly set
* without context. */
static enum ofperr
-ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a,
+ofpact_check__(enum ofputil_protocol *usable_protocols, const struct ofpact *a,
struct flow *flow, ofp_port_t max_ports,
uint8_t table_id, uint8_t n_tables)
{
@@ -2150,12 +2150,12 @@ ofpact_check__(enum ofputil_protocol *usable_protocols,
struct ofpact *a,
*
* May temporarily modify 'flow', but restores the changes before returning. */
enum ofperr
-ofpacts_check(struct ofpact ofpacts[], size_t ofpacts_len,
+ofpacts_check(const struct ofpact ofpacts[], size_t ofpacts_len,
struct flow *flow, ofp_port_t max_ports,
uint8_t table_id, uint8_t n_tables,
enum ofputil_protocol *usable_protocols)
{
-struct ofpact *a;
+const struct ofpact *a;
ovs_be16 dl_type = flow->dl_type;
ovs_be16 vlan_tci = flow->vlan_tci;
uint8_t nw_proto = flow->nw_proto;
@@ -2178,7 +2178,7 @@ ofpacts_check(struct ofpact ofpacts[], size_t ofpacts_len,
/* Like ofpacts_check(), but reports inconsistencies as
* OFPERR_OFPBAC_MATCH_INCONSISTENT rather than clearing bits. */
enum ofperr
-ofpacts_check_consistency(struct ofpact ofpacts[], size_t ofpacts_len,
+ofpacts_check_consistency(const struct ofpact ofpacts[], size_t ofpacts_len,
struct flow *flow, ofp_port_t max_ports,
uint8_t table_id, uint8_t n_tables,
enum ofputil_protocol usable_protocols)
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 89bf867..a0fd5c7 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -234,8 +234,8 @@ struct ofpact_enqueue {
* Used for NXAST_OUTPUT_REG. */
struct ofpact_output_reg {
struct ofpact ofpact;
-struct mf_subfield src;
uint16_t max_len;
+struct mf_subfield src;
};
/* OFPACT_BUNDLE.
@@ -592,11 +592,11 @@ enum ofperr ofpacts_pull_openflow_instructions(struct
ofpbuf *openflow,
unsigned int instructions_len,
enum ofp_version version,
struct ofpbuf *ofpacts);
-enum ofperr ofpacts_check(struct ofpact[], size_t ofpacts_len,
+enum ofperr ofpacts_check(const struct ofpact[], size_t ofpacts_len,
struct flow *, ofp_port_t max_ports,
uint8_t table_id, uint8_t n_tables,
enum ofputil_protocol *usable_protocols);
-enum ofperr ofpacts_check_consistency(struct ofpact[], size_t ofpacts_len,
+enum ofperr ofpacts_check_consistency(const struct ofpact[], size_t
ofpacts_len,
struct flow *, ofp_port_t max_ports,
uint8_t table_id, uint8_t n_tables,
enum ofputil_protocol usable_protocols);
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index d250042..c759f03 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1924,7 +1924,7 @@ parse_ofp_flow_mod_file(const char *file_name, uint16_t
command,
size_t i;
for (i = 0; i < *n_fms; i++) {
-free((*fms)[i].ofpacts);
+free(CONST_CAST(struct ofpact *, (*fms)[i].ofpacts));
}
free(*fms);
*fms = NULL;
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 3484394..4473e3c 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -6208,7 +6208,7 @@ ofputil_bucket_list_destroy(struct list *buckets)
LIST_FOR_EACH_SAFE (bucket, next_bucket, list_node, buckets) {
list_remove(&bucket->list_node);
-free(bucket->ofpacts);
+free(C
Re: [ovs-dev] [PATCHv13] ofproto-dpif-upcall: Remove the flow_dumper thread.
With this patch, ovs-vswitchd now crashes with the following error
during upgrades.
ovs-vswitchd: pthread_barrier_init failed (Invalid argument)
ovs-vswitchd: fork child died before signaling startup (killed
(Aborted), core dumped)
Note that this happens when ovs-vswitchd is started with:
other_config:flow-restore-wait=true (which is true for all package
installations)
Can you guys please take a look.
On Wed, Apr 23, 2014 at 11:24 PM, Ethan Jackson wrote:
> W00t, nice to see this in.
>
> Ethan
>
> On Wed, Apr 23, 2014 at 9:32 PM, Joe Stringer wrote:
>> Thanks, I tidied up the comments, added threadsafety annotations for xcache
>> and pushed.
>>
>>
>> On 24 April 2014 06:38, Ben Pfaff wrote:
>>>
>>> On Tue, Apr 22, 2014 at 04:54:24PM +1200, Joe Stringer wrote:
>>> > From: Ethan Jackson
>>> >
>>> > Previously, we had a separate flow_dumper thread that fetched flows from
>>> > the datapath to distribute to revalidator threads. This patch takes the
>>> > logic for dumping and pushes it into the revalidator threads, resulting
>>> > in simpler code with similar performance to the current code.
>>> >
>>> > One thread, the "leader", is responsible for beginning and ending each
>>> > flow dump, maintaining the flow_limit, and checking whether the
>>> > revalidator threads need to exit. All revalidator threads dump,
>>> > revalidate, delete datapath flows and garbage collect ukeys.
>>> >
>>> > Co-authored-by: Joe Stringer
>>> > Signed-off-by: Joe Stringer
>>>
>>> Here in the definition of struct udpif, I would mention that there are
>>> n_revalidators member of 'ukeys'. Otherwise the casual reader might
>>> guess that there was only one:
>>> > +/* During the flow dump phase, revalidators insert into these with
>>> > a random
>>> > + * distribution. During the garbage collection phase, each
>>> > revalidator
>>> > + * takes care of garbage collecting one of these hmaps. */
>>> > +struct {
>>> > +struct ovs_mutex mutex;/* Guards the following. */
>>> > +struct hmap hmap OVS_GUARDED; /* Datapath flow keys. */
>>> > +} *ukeys;
>>>
>>> In the definition of struct udpif_key, the synchronization of most of
>>> the members is pretty clear, except for 'xcache'.
>>>
>>> Acked-by: Ben Pfaff
>>
>>
>>
>> ___
>> 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] [PATCHv13] ofproto-dpif-upcall: Remove the flow_dumper thread.
Alex wanted the backtrace:
at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1 0x76d13bab in __GI_abort () at abort.c:91
#2 0x004f08b0 in ovs_abort_valist (err_no=22,
format=0x55714f "%s failed", args=0x7fffdf58) at lib/util.c:306
#3 0x004f0883 in ovs_abort (err_no=22, format=0x55714f "%s failed")
at lib/util.c:298
#4 0x004cae36 in xpthread_barrier_init (arg1=0x9dbfd0, arg2=0x0,
arg3=0) at lib/ovs-thread.c:149
#5 0x00439929 in udpif_start_threads (udpif=0x9dbf70, n_handlers=0,
n_revalidators=0) at ofproto/ofproto-dpif-upcall.c:361
#6 0x00439d95 in udpif_flush (udpif=0x9dbf70)
at ofproto/ofproto-dpif-upcall.c:476
#7 0x0042a2ce in flush (ofproto_=0x9eaee0)
at ofproto/ofproto-dpif.c:1497
#8 0x00418fc0 in ofproto_flush__ (ofproto=0x9eaee0)
at ofproto/ofproto.c:1279
#9 0x00419c11 in ofproto_run (p=0x9eaee0) at ofproto/ofproto.c:1553
#10 0x0040aec0 in bridge_run__ () at vswitchd/bridge.c:2340
#11 0x00406c78 in bridge_reconfigure (ovs_cfg=0xa6ff30)
at vswitchd/bridge.c:623
#12 0x0040b14c in bridge_run () at vswitchd/bridge.c:2423
#13 0x00410365 in main (argc=9, argv=0x7fffe4a8)
at vswitchd/ovs-vswitchd.c:116
On Fri, Apr 25, 2014 at 9:39 AM, Gurucharan Shetty wrote:
> With this patch, ovs-vswitchd now crashes with the following error
> during upgrades.
>
> ovs-vswitchd: pthread_barrier_init failed (Invalid argument)
> ovs-vswitchd: fork child died before signaling startup (killed
> (Aborted), core dumped)
>
> Note that this happens when ovs-vswitchd is started with:
> other_config:flow-restore-wait=true (which is true for all package
> installations)
>
> Can you guys please take a look.
>
> On Wed, Apr 23, 2014 at 11:24 PM, Ethan Jackson wrote:
>> W00t, nice to see this in.
>>
>> Ethan
>>
>> On Wed, Apr 23, 2014 at 9:32 PM, Joe Stringer wrote:
>>> Thanks, I tidied up the comments, added threadsafety annotations for xcache
>>> and pushed.
>>>
>>>
>>> On 24 April 2014 06:38, Ben Pfaff wrote:
On Tue, Apr 22, 2014 at 04:54:24PM +1200, Joe Stringer wrote:
> From: Ethan Jackson
>
> Previously, we had a separate flow_dumper thread that fetched flows from
> the datapath to distribute to revalidator threads. This patch takes the
> logic for dumping and pushes it into the revalidator threads, resulting
> in simpler code with similar performance to the current code.
>
> One thread, the "leader", is responsible for beginning and ending each
> flow dump, maintaining the flow_limit, and checking whether the
> revalidator threads need to exit. All revalidator threads dump,
> revalidate, delete datapath flows and garbage collect ukeys.
>
> Co-authored-by: Joe Stringer
> Signed-off-by: Joe Stringer
Here in the definition of struct udpif, I would mention that there are
n_revalidators member of 'ukeys'. Otherwise the casual reader might
guess that there was only one:
> +/* During the flow dump phase, revalidators insert into these with
> a random
> + * distribution. During the garbage collection phase, each
> revalidator
> + * takes care of garbage collecting one of these hmaps. */
> +struct {
> +struct ovs_mutex mutex;/* Guards the following. */
> +struct hmap hmap OVS_GUARDED; /* Datapath flow keys. */
> +} *ukeys;
In the definition of struct udpif_key, the synchronization of most of
the members is pretty clear, except for 'xcache'.
Acked-by: Ben Pfaff
>>>
>>>
>>>
>>> ___
>>> 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
[ovs-dev] [PATCH 1/2] lacp: Ensure that mutex is initialized when used in lacp_status().
If 'lacp' is NULL, then lacp_create() might not have been called to
indirectly initialize the mutex via lacp_init(), so call lacp_init()
from lacp_status().
Signed-off-by: Ben Pfaff
Acked-by: Andy Zhou
---
lib/lacp.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/lib/lacp.c b/lib/lacp.c
index 4aee64f..dfb7159 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -199,21 +199,23 @@ parse_lacp_packet(const struct ofpbuf *b)
void
lacp_init(void)
{
-unixctl_command_register("lacp/show", "[port]", 0, 1,
- lacp_unixctl_show, NULL);
+static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+
+if (ovsthread_once_start(&once)) {
+ovs_mutex_init_recursive(&mutex);
+unixctl_command_register("lacp/show", "[port]", 0, 1,
+ lacp_unixctl_show, NULL);
+ovsthread_once_done(&once);
+}
}
/* Creates a LACP object. */
struct lacp *
lacp_create(void) OVS_EXCLUDED(mutex)
{
-static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
struct lacp *lacp;
-if (ovsthread_once_start(&once)) {
-ovs_mutex_init_recursive(&mutex);
-ovsthread_once_done(&once);
-}
+lacp_init();
lacp = xzalloc(sizeof *lacp);
hmap_init(&lacp->slaves);
@@ -347,6 +349,8 @@ lacp_status(const struct lacp *lacp) OVS_EXCLUDED(mutex)
{
enum lacp_status ret;
+lacp_init();
+
ovs_mutex_lock(&mutex);
if (!lacp) {
ret = LACP_DISABLED;
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] ovs-thread: Add checking for mutex and rwlock initialization.
With glibc, a mutex or rwlock filled with all-zero-bytes is properly
initialized for use, but this is not true for any other libc that OVS
supports. However, OVS gets a lot more testing with glibc than any other
libc. This means that developers keep introducing bugs that do not
manifest on the main development platform.
This commit should help avoid the problem, by reusing the existing 'where'
members to indicate whether a mutex or rwlock has been initialized.
Signed-off-by: Ben Pfaff
---
lib/ovs-thread.c | 36 +---
lib/ovs-thread.h | 13 +++--
2 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index 3ca686f..f347fa8 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -52,12 +52,17 @@ static bool multithreaded;
OVS_NO_THREAD_SAFETY_ANALYSIS \
{ \
struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \
-int error = pthread_##TYPE##_##FUN(&l->lock); \
+int error; \
+ \
+/* Verify that 'l' was initialized. */ \
+ovs_assert(l->where); \
+ \
+error = pthread_##TYPE##_##FUN(&l->lock); \
if (OVS_UNLIKELY(error)) { \
ovs_abort(error, "pthread_%s_%s failed", #TYPE, #FUN); \
} \
l->where = where; \
-}
+ }
LOCK_FUNCTION(mutex, lock);
LOCK_FUNCTION(rwlock, rdlock);
LOCK_FUNCTION(rwlock, wrlock);
@@ -69,7 +74,12 @@ LOCK_FUNCTION(rwlock, wrlock);
OVS_NO_THREAD_SAFETY_ANALYSIS \
{ \
struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \
-int error = pthread_##TYPE##_##FUN(&l->lock); \
+int error; \
+ \
+/* Verify that 'l' was initialized. */ \
+ovs_assert(l->where); \
+ \
+error = pthread_##TYPE##_##FUN(&l->lock); \
if (OVS_UNLIKELY(error) && error != EBUSY) { \
ovs_abort(error, "pthread_%s_%s failed", #TYPE, #FUN); \
} \
@@ -82,23 +92,27 @@ TRY_LOCK_FUNCTION(mutex, trylock);
TRY_LOCK_FUNCTION(rwlock, tryrdlock);
TRY_LOCK_FUNCTION(rwlock, trywrlock);
-#define UNLOCK_FUNCTION(TYPE, FUN) \
+#define UNLOCK_FUNCTION(TYPE, FUN, WHERE) \
void \
ovs_##TYPE##_##FUN(const struct ovs_##TYPE *l_) \
OVS_NO_THREAD_SAFETY_ANALYSIS \
{ \
struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \
int error; \
-l->where = NULL; \
+ \
+/* Verify that 'l' was initialized. */ \
+ovs_assert(l->where); \
+ \
+l->where = WHERE; \
error = pthread_##TYPE##_##FUN(&l->lock); \
if (OVS_UNLIKELY(error)) { \
ovs_abort(error, "pthread_%s_%sfailed", #TYPE, #FUN); \
} \
}
-UNLOCK_FUNCTION(mutex, unlock);
-UNLOCK_FUNCTION(mutex, destroy);
-UNLOCK_FUNCTION(rwlock, unlock);
-UNLOCK_FUNCTION(rwlock, destroy);
+UNLOCK_FUNCTION(mutex, unlock, "");
+UNLOCK_FUNCTION(mutex, destroy, NULL);
+UNLOCK_FUNCTION(rwlock, unlock, "");
+UNLOCK_FUNCTION(rwlock, destroy, NULL);
#define XPTHREAD_FUNC1(FUNCTION, PARAM1)\
void\
@@ -164,7 +178,7 @@ ovs_mutex_init__(const struct ovs_mutex *l_, int type)
pthread_mutexattr_t attr;
int error;
-l->where = NULL;
+l->where = "";
xpthread_mutexattr_init(&attr);
xpthread_mutexattr_settype(&attr, type);
error = pthread_mutex_init(&l->lock, &attr);
@@ -206,7 +220,7 @@ ovs_rwlock_init(const struct ovs_rwlock *l_)
pthread_rwlockattr_t attr;
int error;
-l->where = NULL;
+l->where = "";
xpthread_rwlockattr_init(&attr);
#ifdef PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP
diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
index 02a81f7..e5a99bd 100644
--- a/lib/ovs-thread.h
+++ b/lib/ovs-thread.h
@@ -27,14 +27,15 @@
/* Mutex. */
struct OVS_LOCKABLE ovs_mutex {
pthread_mutex_t lock;
-const char *where;
+const char *where; /* NULL if and only if uninitialized. */
};
/* "struct ovs_mutex" initializer. */
#ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
-#define OVS_MUTEX_INITIALIZER { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, NULL }
+#define OVS_MUTEX_INITIALIZER { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, \
+"" }
#else
-#define OVS_MUTEX_INITIALIZER { PTHREAD_MUTEX_INITIALIZER, NULL }
+#define OVS_MUTEX_INITIALIZER { PTHREAD_MUTEX_INITIALIZER, "" }
#endif
#ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP
@@ -91,15 +92,15 @@ void xpthread_mutexattr_gettype(pthread_mutexattr_t *, int
*typep);
* than exposing them only to porters. */
struct OVS_LOCKABLE ovs_rwlock {
pthread_rwlock_t lock;
-const char *where;
+const char *where; /* NULL if and only if uninitialized. */
};
/* Initializer. */
#ifdef PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP
#define OVS_RWLOCK_INITIALIZER \
-{ PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP, NULL }
+{ PTHREAD_RWLOCK_WRITER_NON
Re: [ovs-dev] [PATCH 0/2] Add checking for mutex and rwlock initialization.
These should have been marked v2. Sorry. On Fri, Apr 25, 2014 at 09:56:22AM -0700, Ben Pfaff wrote: > v1->v2: In patch 1, always initialize the lacp mutex, instead of avoiding >the need to initialize it. > > Ben Pfaff (2): > lacp: Ensure that mutex is initialized when used in lacp_status(). > ovs-thread: Add checking for mutex and rwlock initialization. > > lib/lacp.c | 18 +++--- > lib/ovs-thread.c | 36 +--- > lib/ovs-thread.h | 13 +++-- > 3 files changed, 43 insertions(+), 24 deletions(-) > > -- > 1.7.10.4 > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] ovs-thread: Add checking for mutex and rwlock initialization.
On Fri, Apr 25, 2014 at 08:50:10AM -0700, Gurucharan Shetty wrote: > On Fri, Apr 25, 2014 at 8:46 AM, Ben Pfaff wrote: > > On Thu, Apr 24, 2014 at 11:27:47PM -0700, Andy Zhou wrote: > >> The logic looks good. 233 unit tests failed now. Apparently this > >> patch is great doing a great job on catching uninitialized mutex and > >> rwlocks. > >> > >> Assume the tests will be fixed before checking in: > > > > I did my testing with one of Guru's bug fixes related to locking from > > yesterday in my tree. No failures then. I'm planning to wait for him > > to apply those before I push. > I applied them today morning. Even better, thanks! > I suppose the first patch in this series needs to be checked in before this > one goes in, otherwise the lacp test will fail? Yes. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 0/2] Add checking for mutex and rwlock initialization.
v1->v2: In patch 1, always initialize the lacp mutex, instead of avoiding the need to initialize it. Ben Pfaff (2): lacp: Ensure that mutex is initialized when used in lacp_status(). ovs-thread: Add checking for mutex and rwlock initialization. lib/lacp.c | 18 +++--- lib/ovs-thread.c | 36 +--- lib/ovs-thread.h | 13 +++-- 3 files changed, 43 insertions(+), 24 deletions(-) -- 1.7.10.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCHv13] ofproto-dpif-upcall: Remove the flow_dumper thread.
Found the bug, i introduced it, so sorry,
On Fri, Apr 25, 2014 at 9:45 AM, Gurucharan Shetty wrote:
> Alex wanted the backtrace:
> at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> #1 0x76d13bab in __GI_abort () at abort.c:91
> #2 0x004f08b0 in ovs_abort_valist (err_no=22,
> format=0x55714f "%s failed", args=0x7fffdf58) at lib/util.c:306
> #3 0x004f0883 in ovs_abort (err_no=22, format=0x55714f "%s
> failed")
> at lib/util.c:298
> #4 0x004cae36 in xpthread_barrier_init (arg1=0x9dbfd0, arg2=0x0,
> arg3=0) at lib/ovs-thread.c:149
> #5 0x00439929 in udpif_start_threads (udpif=0x9dbf70,
> n_handlers=0,
> n_revalidators=0) at ofproto/ofproto-dpif-upcall.c:361
> #6 0x00439d95 in udpif_flush (udpif=0x9dbf70)
> at ofproto/ofproto-dpif-upcall.c:476
> #7 0x0042a2ce in flush (ofproto_=0x9eaee0)
> at ofproto/ofproto-dpif.c:1497
> #8 0x00418fc0 in ofproto_flush__ (ofproto=0x9eaee0)
> at ofproto/ofproto.c:1279
> #9 0x00419c11 in ofproto_run (p=0x9eaee0) at
> ofproto/ofproto.c:1553
> #10 0x0040aec0 in bridge_run__ () at vswitchd/bridge.c:2340
> #11 0x00406c78 in bridge_reconfigure (ovs_cfg=0xa6ff30)
> at vswitchd/bridge.c:623
> #12 0x0040b14c in bridge_run () at vswitchd/bridge.c:2423
> #13 0x00410365 in main (argc=9, argv=0x7fffe4a8)
> at vswitchd/ovs-vswitchd.c:116
>
> On Fri, Apr 25, 2014 at 9:39 AM, Gurucharan Shetty
> wrote:
> > With this patch, ovs-vswitchd now crashes with the following error
> > during upgrades.
> >
> > ovs-vswitchd: pthread_barrier_init failed (Invalid argument)
> > ovs-vswitchd: fork child died before signaling startup (killed
> > (Aborted), core dumped)
> >
> > Note that this happens when ovs-vswitchd is started with:
> > other_config:flow-restore-wait=true (which is true for all package
> > installations)
> >
> > Can you guys please take a look.
> >
> > On Wed, Apr 23, 2014 at 11:24 PM, Ethan Jackson
> wrote:
> >> W00t, nice to see this in.
> >>
> >> Ethan
> >>
> >> On Wed, Apr 23, 2014 at 9:32 PM, Joe Stringer
> wrote:
> >>> Thanks, I tidied up the comments, added threadsafety annotations for
> xcache
> >>> and pushed.
> >>>
> >>>
> >>> On 24 April 2014 06:38, Ben Pfaff wrote:
>
> On Tue, Apr 22, 2014 at 04:54:24PM +1200, Joe Stringer wrote:
> > From: Ethan Jackson
> >
> > Previously, we had a separate flow_dumper thread that fetched flows
> from
> > the datapath to distribute to revalidator threads. This patch takes
> the
> > logic for dumping and pushes it into the revalidator threads,
> resulting
> > in simpler code with similar performance to the current code.
> >
> > One thread, the "leader", is responsible for beginning and ending
> each
> > flow dump, maintaining the flow_limit, and checking whether the
> > revalidator threads need to exit. All revalidator threads dump,
> > revalidate, delete datapath flows and garbage collect ukeys.
> >
> > Co-authored-by: Joe Stringer
> > Signed-off-by: Joe Stringer
>
> Here in the definition of struct udpif, I would mention that there are
> n_revalidators member of 'ukeys'. Otherwise the casual reader might
> guess that there was only one:
> > +/* During the flow dump phase, revalidators insert into these
> with
> > a random
> > + * distribution. During the garbage collection phase, each
> > revalidator
> > + * takes care of garbage collecting one of these hmaps. */
> > +struct {
> > +struct ovs_mutex mutex;/* Guards the following. */
> > +struct hmap hmap OVS_GUARDED; /* Datapath flow keys. */
> > +} *ukeys;
>
> In the definition of struct udpif_key, the synchronization of most of
> the members is pretty clear, except for 'xcache'.
>
> Acked-by: Ben Pfaff
> >>>
> >>>
> >>>
> >>> ___
> >>> 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
[ovs-dev] [PATCH] bridge: Add test that ports that disappear get added back to the datapath.
The test added in this commit would have caught the bug fixed by commit
96be8de595150 (bridge: When ports disappear from a datapath, add them
back.). With that commit reverted, the new test fails.
Signed-off-by: Ben Pfaff
---
lib/dpif-netdev.c | 61
tests/automake.mk |1 +
tests/bridge.at| 38
tests/testsuite.at |3 ++-
4 files changed, 88 insertions(+), 15 deletions(-)
create mode 100644 tests/bridge.at
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 07f181d..cff8df7 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -345,7 +345,7 @@ static void dp_netdev_flow_flush(struct dp_netdev *);
static int do_add_port(struct dp_netdev *dp, const char *devname,
const char *type, odp_port_t port_no)
OVS_REQ_WRLOCK(dp->port_rwlock);
-static int do_del_port(struct dp_netdev *dp, odp_port_t port_no)
+static void do_del_port(struct dp_netdev *dp, struct dp_netdev_port *)
OVS_REQ_WRLOCK(dp->port_rwlock);
static void dp_netdev_destroy_all_queues(struct dp_netdev *dp)
OVS_REQ_WRLOCK(dp->queue_rwlock);
@@ -567,7 +567,7 @@ dp_netdev_free(struct dp_netdev *dp)
dp_netdev_flow_flush(dp);
ovs_rwlock_wrlock(&dp->port_rwlock);
HMAP_FOR_EACH_SAFE (port, next, node, &dp->ports) {
-do_del_port(dp, port->port_no);
+do_del_port(dp, port);
}
ovs_rwlock_unlock(&dp->port_rwlock);
@@ -775,7 +775,16 @@ dpif_netdev_port_del(struct dpif *dpif, odp_port_t port_no)
int error;
ovs_rwlock_wrlock(&dp->port_rwlock);
-error = port_no == ODPP_LOCAL ? EINVAL : do_del_port(dp, port_no);
+if (port_no == ODPP_LOCAL) {
+error = EINVAL;
+} else {
+struct dp_netdev_port *port;
+
+error = get_port_by_number(dp, port_no, &port);
+if (!error) {
+do_del_port(dp, port);
+}
+}
ovs_rwlock_unlock(&dp->port_rwlock);
return error;
@@ -857,18 +866,10 @@ get_port_by_name(struct dp_netdev *dp,
return ENOENT;
}
-static int
-do_del_port(struct dp_netdev *dp, odp_port_t port_no)
+static void
+do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port)
OVS_REQ_WRLOCK(dp->port_rwlock)
{
-struct dp_netdev_port *port;
-int error;
-
-error = get_port_by_number(dp, port_no, &port);
-if (error) {
-return error;
-}
-
hmap_remove(&dp->ports, &port->node);
seq_change(dp->port_seq);
if (netdev_is_pmd(port->netdev)) {
@@ -876,7 +877,6 @@ do_del_port(struct dp_netdev *dp, odp_port_t port_no)
}
port_unref(port);
-return 0;
}
static void
@@ -2301,6 +2301,37 @@ exit:
}
static void
+dpif_dummy_delete_port(struct unixctl_conn *conn, int argc OVS_UNUSED,
+ const char *argv[], void *aux OVS_UNUSED)
+{
+struct dp_netdev_port *port;
+struct dp_netdev *dp;
+
+ovs_mutex_lock(&dp_netdev_mutex);
+dp = shash_find_data(&dp_netdevs, argv[1]);
+if (!dp || !dpif_netdev_class_is_dummy(dp->class)) {
+ovs_mutex_unlock(&dp_netdev_mutex);
+unixctl_command_reply_error(conn, "unknown datapath or not a dummy");
+return;
+}
+ovs_refcount_ref(&dp->ref_cnt);
+ovs_mutex_unlock(&dp_netdev_mutex);
+
+ovs_rwlock_wrlock(&dp->port_rwlock);
+if (get_port_by_name(dp, argv[2], &port)) {
+unixctl_command_reply_error(conn, "unknown port");
+} else if (port->port_no == ODPP_LOCAL) {
+unixctl_command_reply_error(conn, "can't delete local port");
+} else {
+do_del_port(dp, port);
+unixctl_command_reply(conn, NULL);
+}
+ovs_rwlock_unlock(&dp->port_rwlock);
+
+dp_netdev_unref(dp);
+}
+
+static void
dpif_dummy_register__(const char *type)
{
struct dpif_class *class;
@@ -2333,4 +2364,6 @@ dpif_dummy_register(bool override)
unixctl_command_register("dpif-dummy/change-port-number",
"DP PORT NEW-NUMBER",
3, 3, dpif_dummy_change_port_number, NULL);
+unixctl_command_register("dpif-dummy/delete-port", "DP PORT",
+ 2, 2, dpif_dummy_delete_port, NULL);
}
diff --git a/tests/automake.mk b/tests/automake.mk
index bf80702..b7b85d4 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -38,6 +38,7 @@ TESTSUITE_AT = \
tests/reconnect.at \
tests/ovs-vswitchd.at \
tests/ofproto-dpif.at \
+ tests/bridge.at \
tests/vlan-splinters.at \
tests/ofproto-macros.at \
tests/ofproto.at \
diff --git a/tests/bridge.at b/tests/bridge.at
new file mode 100644
index 000..817931f
--- /dev/null
+++ b/tests/bridge.at
@@ -0,0 +1,38 @@
+AT_BANNER([bridge])
+
+dnl When a port disappears from a datapath, e.g. because an admin used
+dnl "ovs-dpctl del-port", the bridge code should be resilient enough to
+dnl notice and add it back the next time we reconfigure. A prior version
+dnl of the code f
[ovs-dev] [PATCH] ofproto-dpif-upcall: Fix a bug.
Commit 7d170098 (ofproto-dpif-upcall: Remove the flow_dumper thread.)
initialized the memory barrier inside the udpif_start_threads() function.
However, the udpif_start_threads() function does not check the number of
revalidator threads specified in udpif. So, when the number is zero, it
causes the error in barrier initialization. This could happen when the
other_config:flow-restore-wait is set and the udpif_flush() is called.
This commit fixes the issue, by checking the specified number of threads
in udpif_start_threads().
Reported-by: Gurucharan Shetty
Signed-off-by: Alex Wang
---
ofproto/ofproto-dpif-upcall.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 84afc56..717563a 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -342,7 +342,7 @@ static void
udpif_start_threads(struct udpif *udpif, size_t n_handlers,
size_t n_revalidators)
{
-if (udpif && (!udpif->handlers && !udpif->revalidators)) {
+if (udpif && n_handlers && n_revalidators) {
size_t i;
udpif->n_handlers = n_handlers;
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Job Vacancies at Atlas Petroleum Ltd
Interested candidates should forward their resumes/CV (Specifying position of interest) with verifiable reference(s) as word attached document to E-Mail: jobcare...@shqiptar.eu Yours SincerelyShqiptar Recruitment Agency___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 00/16] Flow-Based Recirculation for MPLS
On Fri, Apr 25, 2014 at 01:40:13PM +0900, Simon Horman wrote: > I'd like to clarify the status of this series. > > I believe that as it stands it is ready for review. > And that the dependencies I list above only affect the reliability > of tests added in the last patch of the series: they address > issues that are already present in the master branch but don't manifest > with the tests currently in the test-suite. Great, I'll start taking a look at it. Recirculation holds the best promise for high-quality MPLS support in OVS. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix a bug.
On Fri, Apr 25, 2014 at 11:04 AM, Alex Wang wrote:
> Commit 7d170098 (ofproto-dpif-upcall: Remove the flow_dumper thread.)
> initialized the memory barrier inside the udpif_start_threads() function.
> However, the udpif_start_threads() function does not check the number of
> revalidator threads specified in udpif. So, when the number is zero, it
> causes the error in barrier initialization. This could happen when the
> other_config:flow-restore-wait is set and the udpif_flush() is called.
>
> This commit fixes the issue, by checking the specified number of threads
> in udpif_start_threads().
>
> Reported-by: Gurucharan Shetty
> Signed-off-by: Alex Wang
This patch fixes the problem for me. It also looks correct. But the
code here has changed so much over the past few months, that I do not
understand all the implications.
So, with that :
Acked-by: Gurucharan Shetty
> ---
> ofproto/ofproto-dpif-upcall.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 84afc56..717563a 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -342,7 +342,7 @@ static void
> udpif_start_threads(struct udpif *udpif, size_t n_handlers,
> size_t n_revalidators)
> {
> -if (udpif && (!udpif->handlers && !udpif->revalidators)) {
> +if (udpif && n_handlers && n_revalidators) {
> size_t i;
>
> udpif->n_handlers = n_handlers;
> --
> 1.7.9.5
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] lacp: Ensure that mutex is initialized when used in lacp_status().
Looks great. Thanks!
On Fri, Apr 25, 2014 at 9:56 AM, Ben Pfaff wrote:
> If 'lacp' is NULL, then lacp_create() might not have been called to
> indirectly initialize the mutex via lacp_init(), so call lacp_init()
> from lacp_status().
>
> Signed-off-by: Ben Pfaff
> Acked-by: Andy Zhou
> ---
> lib/lacp.c | 18 +++---
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/lib/lacp.c b/lib/lacp.c
> index 4aee64f..dfb7159 100644
> --- a/lib/lacp.c
> +++ b/lib/lacp.c
> @@ -199,21 +199,23 @@ parse_lacp_packet(const struct ofpbuf *b)
> void
> lacp_init(void)
> {
> -unixctl_command_register("lacp/show", "[port]", 0, 1,
> - lacp_unixctl_show, NULL);
> +static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> +
> +if (ovsthread_once_start(&once)) {
> +ovs_mutex_init_recursive(&mutex);
> +unixctl_command_register("lacp/show", "[port]", 0, 1,
> + lacp_unixctl_show, NULL);
> +ovsthread_once_done(&once);
> +}
> }
>
> /* Creates a LACP object. */
> struct lacp *
> lacp_create(void) OVS_EXCLUDED(mutex)
> {
> -static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> struct lacp *lacp;
>
> -if (ovsthread_once_start(&once)) {
> -ovs_mutex_init_recursive(&mutex);
> -ovsthread_once_done(&once);
> -}
> +lacp_init();
>
> lacp = xzalloc(sizeof *lacp);
> hmap_init(&lacp->slaves);
> @@ -347,6 +349,8 @@ lacp_status(const struct lacp *lacp) OVS_EXCLUDED(mutex)
> {
> enum lacp_status ret;
>
> +lacp_init();
> +
> ovs_mutex_lock(&mutex);
> if (!lacp) {
> ret = LACP_DISABLED;
> --
> 1.7.10.4
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Datapath kern log msg key attribute error
How about we skip logging each unexpected key attributes once? We can use a u64 bit fields to keep track the flow key attributes in the kernel module. On Thu, Apr 24, 2014 at 6:30 PM, Jesse Gross wrote: > On Thu, Apr 24, 2014 at 4:30 PM, Jarno Rajahalme > wrote: >> >> On Apr 24, 2014, at 3:32 PM, Ben Pfaff wrote: >> >>> On Thu, Apr 24, 2014 at 02:57:28PM -0700, Jesse Gross wrote: I suppose the other possibility is to pass some kind of flag attribute with messages indicating that this is a test probe and would silence logging. Existing kernels would ignore this so they would still log but the behavior would be otherwise unchanged. >>> >>> That makes me nervous--it poses the temptation of changing behavior in >>> some other way, not just log-wise. >> >> How about a specific netlink (DP) command to set the logging level and >> invoking it before and after probing? > > I guess something seems vaguely wrong about this to me - it seems to > assume too much about the kernel implementation. > ___ > 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 v4 1/2] datapath: Add support for kernel 3.14.
Hi Jesse, >> >> diff --git a/datapath/linux/compat/include/linux/etherdevice.h >> b/datapath/linux/compat/include/linux/etherdevice.h >> index 556729d..c6e8e92 100644 >> --- a/datapath/linux/compat/include/linux/etherdevice.h >> +++ b/datapath/linux/compat/include/linux/etherdevice.h >> @@ -34,6 +34,7 @@ static inline int eth_mac_addr(struct net_device *dev, >> void *p) >> } >> #endif >> >> +#ifndef HAVE_ETHER_ADDR_COPY >> static inline void ether_addr_copy(u8 *dst, const u8 *src) > > Now that we have a configure check, I think it is probably cleaner to > move this out of the version check. maybe i am mistaken, but i don’t see any version checks here ? Regards, Pritesh ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix a bug.
Applied, thx a lot,
Now, let's fix the build (unittest) issue~~~
On Fri, Apr 25, 2014 at 11:31 AM, Gurucharan Shetty wrote:
> On Fri, Apr 25, 2014 at 11:04 AM, Alex Wang wrote:
> > Commit 7d170098 (ofproto-dpif-upcall: Remove the flow_dumper thread.)
> > initialized the memory barrier inside the udpif_start_threads() function.
> > However, the udpif_start_threads() function does not check the number of
> > revalidator threads specified in udpif. So, when the number is zero, it
> > causes the error in barrier initialization. This could happen when the
> > other_config:flow-restore-wait is set and the udpif_flush() is called.
> >
> > This commit fixes the issue, by checking the specified number of threads
> > in udpif_start_threads().
> >
> > Reported-by: Gurucharan Shetty
> > Signed-off-by: Alex Wang
>
> This patch fixes the problem for me. It also looks correct. But the
> code here has changed so much over the past few months, that I do not
> understand all the implications.
> So, with that :
> Acked-by: Gurucharan Shetty
>
> > ---
> > ofproto/ofproto-dpif-upcall.c |2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ofproto/ofproto-dpif-upcall.c
> b/ofproto/ofproto-dpif-upcall.c
> > index 84afc56..717563a 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -342,7 +342,7 @@ static void
> > udpif_start_threads(struct udpif *udpif, size_t n_handlers,
> > size_t n_revalidators)
> > {
> > -if (udpif && (!udpif->handlers && !udpif->revalidators)) {
> > +if (udpif && n_handlers && n_revalidators) {
> > size_t i;
> >
> > udpif->n_handlers = n_handlers;
> > --
> > 1.7.9.5
> >
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v4 1/2] datapath: Add support for kernel 3.14.
On Fri, Apr 25, 2014 at 12:16 PM, Pritesh Kothari (pritkoth) wrote: > Hi Jesse, > >>> >>> diff --git a/datapath/linux/compat/include/linux/etherdevice.h >>> b/datapath/linux/compat/include/linux/etherdevice.h >>> index 556729d..c6e8e92 100644 >>> --- a/datapath/linux/compat/include/linux/etherdevice.h >>> +++ b/datapath/linux/compat/include/linux/etherdevice.h >>> @@ -34,6 +34,7 @@ static inline int eth_mac_addr(struct net_device *dev, >>> void *p) >>> } >>> #endif >>> >>> +#ifndef HAVE_ETHER_ADDR_COPY >>> static inline void ether_addr_copy(u8 *dst, const u8 *src) >> >> Now that we have a configure check, I think it is probably cleaner to >> move this out of the version check. > > maybe i am mistaken, but i don’t see any version checks here ? Sorry, I was mistaken. I thought that the version check from the previous function continued down to this one. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Datapath kern log msg key attribute error
Jarno and I just talked and agreed that it's probably finally time to introduce some capabilities attributes. This would allow us to avoid probing altogether and therefore avoid the logging issues. On Fri, Apr 25, 2014 at 11:59 AM, Andy Zhou wrote: > How about we skip logging each unexpected key attributes once? We can > use a u64 bit fields to keep track the flow key attributes in the > kernel module. > > On Thu, Apr 24, 2014 at 6:30 PM, Jesse Gross wrote: >> On Thu, Apr 24, 2014 at 4:30 PM, Jarno Rajahalme >> wrote: >>> >>> On Apr 24, 2014, at 3:32 PM, Ben Pfaff wrote: >>> On Thu, Apr 24, 2014 at 02:57:28PM -0700, Jesse Gross wrote: > I suppose the other possibility is to pass some kind of flag attribute > with messages indicating that this is a test probe and would silence > logging. Existing kernels would ignore this so they would still log > but the behavior would be otherwise unchanged. That makes me nervous--it poses the temptation of changing behavior in some other way, not just log-wise. >>> >>> How about a specific netlink (DP) command to set the logging level and >>> invoking it before and after probing? >> >> I guess something seems vaguely wrong about this to me - it seems to >> assume too much about the kernel implementation. >> ___ >> 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 v2.56] datapath: Add basic MPLS support to kernel
On Fri, Apr 25, 2014 at 1:06 AM, YAMAMOTO Takashi wrote: >> On Thu, Apr 24, 2014 at 05:57:29PM +0900, YAMAMOTO Takashi wrote: >>> hi, >>> >>> > + * Due to the sample action there may be multiple possible eth types. >>> > + * In order to correctly validate actions all possible types are tracked >>> > + * and verified. This is done using struct eth_types. >>> >>> is there any real-world use cases of these actions inside a sample? >>> otherwise, how about just rejecting such combinations? >>> it doesn't seem to worth the code complexity to me. >>> (sorry if it has been already discussed. it's the first time for me >>> to seriously read this long-lived patch.) >> >> Good point, the code is rather complex. >> >> My understanding is that it comes into effect in the case >> of sflow or ipfix being configured on the bridge. I tend >> to think these are real-world use-cases, though that thinking >> is by no means fixed. >> >> My reading of the code is that in the case of sflow and ipfix a single >> sample rule appears at the beginning of the flow. And that it may be >> possible to replace the code that you are referring to with something >> simpler to handle these cases. > > it seems that they put only a userland action inside a sample. > it's what i expected from its name "sample". Yes, that's the only current use case. In theory, this could be used more generally although nothing has come up yet. In retrospect, I regret the design of the sample action - not the part that allows it to handle different actions but the fact that the results can affect things outside of the sample action list. I think that if we had made it more like a subroutine then that would have retained all of the functionality but none of the complexity here. Perhaps if we can find a clean way to restructure it without breaking compatibility then that would simplify the validation here. >> >> My understanding is that the code you are referring to also comes into >> effect when a sample action (a Nicira extension) is used directly in a >> rule. I am less sure that this is a real-world case but the complex logic >> you are referring to should to handle this use-case. > > probably nicira folks can clarify? It's the same set of use cases, just extending it to OpenFlow to enable building sampling into different situations. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v4 1/2] datapath: Add support for kernel 3.14.
On Apr 25, 2014, at 12:36 PM, Jesse Gross wrote: > On Fri, Apr 25, 2014 at 12:16 PM, Pritesh Kothari (pritkoth) > wrote: >> Hi Jesse, >> diff --git a/datapath/linux/compat/include/linux/etherdevice.h b/datapath/linux/compat/include/linux/etherdevice.h index 556729d..c6e8e92 100644 --- a/datapath/linux/compat/include/linux/etherdevice.h +++ b/datapath/linux/compat/include/linux/etherdevice.h @@ -34,6 +34,7 @@ static inline int eth_mac_addr(struct net_device *dev, void *p) } #endif +#ifndef HAVE_ETHER_ADDR_COPY static inline void ether_addr_copy(u8 *dst, const u8 *src) >>> >>> Now that we have a configure check, I think it is probably cleaner to >>> move this out of the version check. >> >> maybe i am mistaken, but i don’t see any version checks here ? > > Sorry, I was mistaken. I thought that the version check from the > previous function continued down to this one. no worries, fixed other stuff will send in v5 soon. thanks for the review. Regards, Pritesh ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 3/3] datapath: add layer 3 flow/port support
On Thu, Apr 17, 2014 at 2:54 AM, wrote: > Hi Lori, all, > > While I've attempted to adapt the layer 3 port patch to GRE tunneling, > and obtained nice results for IP-over-GRE (as I reported last week), > I've had a harder time reusing this framework for -over-GRE, in > particular for MPLS-over-GRE. > > Given its name, I maybe shouldn't have expected -over-GRE to > work on a "layer 3 port", but I did, given that this patch seems to > really be a generic framework for popping/pushing the Ethernet header > before/after tunnelling encap/decap, which is essentially what we need > for any non-Ethernet tunnel. > > Unfortunately, as it is, the patch assumes in some places that the > tunnel payload will be IP if it is not Ethernet, and does not generally > support the tunnel payload being some other ethertype. > > Practically speaking, making your layer 3 port code work for > MOLS-over-GRE is not entirely trivial: > - on emission, compose_output_action__ pops the Ethernet header before > push_mpls is called (which is done during commit_odp_actions) ; this > does not work since push_mpls actually relies on the presence of the > Ethernet header to set the Ethernet ethertype to an MPLS ethertype. One > way to fix it would be to adapt MPLS code to support the case where the > frame is layer3. Another is to have compose_output_action__ call > odp_put_pop_eth_action after commit_odp_actions. I implemented the > latter and it works. This latter option has the benefit preserving > compatibility with other actions that can be done before outputting and > which rely on the Ethernet header being there (such as changing a > Ethernet dst) -- such operations do not make sense if the header is > popped afterward, but I think we should try to not break if the user is > to define such a flow. I think it's actually OK to break (or really ignore) actions that don't make sense. There are some similar examples already - if you try to set the TCP port on an L2 packet then the action will just get ignored. > - on reception, although I haven't fully tried to reach a resolution, > there is at least an issue in flow_extract: with the current patch flow > extract does not parse MPLS for a layer3 frame. I guess that could be > solved although I haven't clarified exactly yet how flow_extract would > guess what payload is in the ofppuf->packet (as I understand, this is > initially determined in the kernel datapath and stored in skb->protocol, > but it has to be propagated to userspace). I think this would probably have to come as metadata from the originator in some form, similar to the input port which also can't be extracted from the packet. In theory, this could either be from the kernel or OpenFlow directly. > Based on the above, I wonder if the code couldn't be made overall > simpler by *always* pushing an Ethernet header on reception of a > non-Ethernet tunnelled packet (not only when, after flow identification, > we conclude that the in_port is layer 3 while the out_port is not). > Similarly, on emission, the popping of the Ethernet header would not > have to be conditioned to the in/out ports being layer 3 or not. The > coexistence of the layer 3 port code with operations relying on the fact > that the packet being manipulated is Ethernet, would become a non-issue > ; the code related to these operations could remain completely untouched. > > In practice, that could be done by doing pop_eth based on flow actions > (as currently done by your patch), but by doing push_eth unconditionally > on reception of a packet from a layer3 tunnel (*not* as a flow action as > currently done in your patch). This is pretty much what is already done in the checked-in LISP code. I think at some point, we need to bite the bullet and figure out how to make OVS independent of Ethernet so this seems to be the time to do it. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 1/8] openvswitch.h: Note that 64 bit ints are 4-aligned.
On Fri, Apr 11, 2014 at 4:29 PM, Jarno Rajahalme wrote: > In general, all Netlink 64-bit data may be 4-byte aligned, due to > netlink header and attributes being 4-aligned. > > To avoid unaligned access the data should be copied out of the netlink > attribute before access. > > Signed-off-by: Jarno Rajahalme Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 7/8] openvswitch.h: Clarify use of key attributes.
On Fri, Apr 11, 2014 at 4:29 PM, Jarno Rajahalme wrote: > Key attributes relating to actual packet headers are ignored for > OVS_PACKET_CMD_EXECUTE as the header key attributes are retrieved > from the packet itself. > > Signed-off-by: Jarno Rajahalme Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 3/3] datapath: add layer 3 flow/port support
Hi Jesse, 2014-04-25, Jesse Gross: >> Practically speaking, making your layer 3 port code work for >> MPLS-over-GRE is not entirely trivial: >> - on emission, compose_output_action__ pops the Ethernet header before >> push_mpls is called (which is done during commit_odp_actions) ; this >> does not work since push_mpls actually relies on the presence of the >> Ethernet header to set the Ethernet ethertype to an MPLS ethertype. One >> way to fix it would be to adapt MPLS code to support the case where the >> frame is layer3. Another is to have compose_output_action__ call >> odp_put_pop_eth_action after commit_odp_actions. I implemented the >> latter and it works. This latter option has the benefit preserving >> compatibility with other actions that can be done before outputting and >> which rely on the Ethernet header being there (such as changing a >> Ethernet dst) -- such operations do not make sense if the header is >> popped afterward, but I think we should try to not break if the user is >> to define such a flow. > > I think it's actually OK to break (or really ignore) actions that > don't make sense. There are some similar examples already - if you try > to set the TCP port on an L2 packet then the action will just get > ignored. The alternative is making all MPLS code work for both Ethernet and non Ethernet cases. More on this below. >> - on reception, although I haven't fully tried to reach a resolution, >> there is at least an issue in flow_extract: with the current patch flow >> extract does not parse MPLS for a layer3 frame. I guess that could be >> solved although I haven't clarified exactly yet how flow_extract would >> guess what payload is in the ofppuf->packet (as I understand, this is >> initially determined in the kernel datapath and stored in skb->protocol, >> but it has to be propagated to userspace). > > I think this would probably have to come as metadata from the > originator in some form, similar to the input port which also can't be > extracted from the packet. Ok. > In theory, this could either be from the > kernel or OpenFlow directly. (I'm not sure how this can come from Openflow, but possibly that's a side question) >> Based on the above, I wonder if the code couldn't be made overall >> simpler by *always* pushing an Ethernet header on reception of a >> non-Ethernet tunnelled packet (not only when, after flow identification, >> we conclude that the in_port is layer 3 while the out_port is not). >> Similarly, on emission, the popping of the Ethernet header would not >> have to be conditioned to the in/out ports being layer 3 or not. The >> coexistence of the layer 3 port code with operations relying on the fact >> that the packet being manipulated is Ethernet, would become a non-issue >> ; the code related to these operations could remain completely untouched. >> >> In practice, that could be done by doing pop_eth based on flow actions >> (as currently done by your patch), but by doing push_eth unconditionally >> on reception of a packet from a layer3 tunnel (*not* as a flow action as >> currently done in your patch). > > This is pretty much what is already done in the checked-in LISP code. > I think at some point, we need to bite the bullet and figure out how > to make OVS independent of Ethernet so this seems to be the time to do > it. I can understand your point of view, although it seems to me that normalizing all packets to Ethernet in input would be beneficial in that it would avoid having to make a bunch of codepaths conditional to the type of payload (MPLS comes to mind, there might be others). It would be similarly beneficial in terms of testsuite coverage, and in terms of avoiding the introduction of regressions in the kernel datapath or in kernel/user space exchanges. -Thomas _ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] revalidator: Fix ukey stats cache updating.
It is even harder to reproduce the new failure, rarer than the flow-dumps
one. Just ran into this once in my life...
More importantly, the flow-dumps pkt counts are sync'ed in the failed case.
>From my understanding, this new issue, relates to reading stats from
netdev-dummy, which is different
area from this fix.
So, this change makes sense to me. I'll apply it first,
On Fri, Apr 25, 2014 at 8:10 AM, Alex Wang wrote:
> found this in the morning, will keep investigating,
>
>
> ./learn.at:362: (ovs-ofctl dump-ports br0 2; ovs-ofctl dump-ports br0 3)
> | sed 's/ (xid=0x[0-9a-fA-F]*)//'
>
> --- - 2014-04-25 01:25:43.022048477 -0700
>
> +++ /root/openvswitch/tests/testsuite.dir/at-groups/328/stdout 2014-04-25
> 01:25:43.017711899 -0700
>
> @@ -1,7 +1,7 @@
>
> OFPST_PORT reply: 1 ports
>
>port 2: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
>
> - tx pkts=2, bytes=120, drop=0, errs=0, coll=0
>
> + tx pkts=6, bytes=360, drop=0, errs=0, coll=0
>
> OFPST_PORT reply: 1 ports
>
>port 3: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
>
> - tx pkts=18, bytes=1080, drop=0, errs=0, coll=0
>
> + tx pkts=14, bytes=840, drop=0, errs=0, coll=0
>
>
> On Thu, Apr 24, 2014 at 10:34 PM, Alex Wang wrote:
>
>> i'll kick an overnight run on "learning action - self-modifying flow
>> with idle_timeout".~ (seq 1). if it survives, tmr morning,
>> i'll ack it~~
>>
>>
>> On Thu, Apr 24, 2014 at 10:26 PM, Joe Stringer wrote:
>>
>>> revalidate_ukey() had a bug where it would update the ukey->stats even
>>> if it decided not to push stats (as an optimisation). ukey->stats should
>>> only be updated when those stats are pushed.
>>>
>>> This bug would arise in the following situation:
>>> * A flow has been dumped before.
>>> * The flow needs to be revalidated.
>>> * The flow is low-throughput.
>>> * The flow has new statistics to push.
>>>
>>> Such cases rely on flow deletion to update the stats. However, that code
>>> pushes the delta between the ukey->stats and the final flow dump. If the
>>> ukey stats cache is updated without the stats being pushed, those stats
>>> would be lost.
>>>
>>> This caused intermittent testsuite failures on "learning action -
>>> self-modifying flow with idle_timeout". Introduced by 698ffe3623f1b630ae
>>> "revalidator: Only revalidate high-throughput flows."
>>>
>>> Bug #1238927.
>>>
>>> Signed-off-by: Joe Stringer
>>> ---
>>> ofproto/ofproto-dpif-upcall.c |3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/ofproto/ofproto-dpif-upcall.c
>>> b/ofproto/ofproto-dpif-upcall.c
>>> index 84afc56..5871772 100644
>>> --- a/ofproto/ofproto-dpif-upcall.c
>>> +++ b/ofproto/ofproto-dpif-upcall.c
>>> @@ -1245,7 +1245,6 @@ revalidate_ukey(struct udpif *udpif, struct
>>> udpif_key *ukey,
>>> push.n_bytes = stats->n_bytes > ukey->stats.n_bytes
>>> ? stats->n_bytes - ukey->stats.n_bytes
>>> : 0;
>>> -ukey->stats = *stats;
>>>
>>> if (!ukey->flow_exists) {
>>> /* Don't bother revalidating if the flow was already deleted. */
>>> @@ -1258,6 +1257,8 @@ revalidate_ukey(struct udpif *udpif, struct
>>> udpif_key *ukey,
>>> goto exit;
>>> }
>>>
>>> +/* We will push the stats, so update the ukey stats cache. */
>>> +ukey->stats = *stats;
>>> if (!push.n_packets && !udpif->need_revalidate) {
>>> ok = true;
>>> goto exit;
>>> --
>>> 1.7.10.4
>>>
>>>
>>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] revalidator: Fix ukey stats cache updating.
Acked-by: Alex Wang
And applied,
On Fri, Apr 25, 2014 at 3:12 PM, Alex Wang wrote:
> It is even harder to reproduce the new failure, rarer than the flow-dumps
> one. Just ran into this once in my life...
>
> More importantly, the flow-dumps pkt counts are sync'ed in the failed case.
>
> From my understanding, this new issue, relates to reading stats from
> netdev-dummy, which is different
> area from this fix.
>
> So, this change makes sense to me. I'll apply it first,
>
>
> On Fri, Apr 25, 2014 at 8:10 AM, Alex Wang wrote:
>
>> found this in the morning, will keep investigating,
>>
>>
>> ./learn.at:362: (ovs-ofctl dump-ports br0 2; ovs-ofctl dump-ports br0 3)
>> | sed 's/ (xid=0x[0-9a-fA-F]*)//'
>>
>> --- - 2014-04-25 01:25:43.022048477 -0700
>>
>> +++ /root/openvswitch/tests/testsuite.dir/at-groups/328/stdout
>> 2014-04-25 01:25:43.017711899 -0700
>>
>> @@ -1,7 +1,7 @@
>>
>> OFPST_PORT reply: 1 ports
>>
>>port 2: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
>>
>> - tx pkts=2, bytes=120, drop=0, errs=0, coll=0
>>
>> + tx pkts=6, bytes=360, drop=0, errs=0, coll=0
>>
>> OFPST_PORT reply: 1 ports
>>
>>port 3: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
>>
>> - tx pkts=18, bytes=1080, drop=0, errs=0, coll=0
>>
>> + tx pkts=14, bytes=840, drop=0, errs=0, coll=0
>>
>>
>> On Thu, Apr 24, 2014 at 10:34 PM, Alex Wang wrote:
>>
>>> i'll kick an overnight run on "learning action - self-modifying flow
>>> with idle_timeout".~ (seq 1). if it survives, tmr morning,
>>> i'll ack it~~
>>>
>>>
>>> On Thu, Apr 24, 2014 at 10:26 PM, Joe Stringer
>>> wrote:
>>>
revalidate_ukey() had a bug where it would update the ukey->stats even
if it decided not to push stats (as an optimisation). ukey->stats should
only be updated when those stats are pushed.
This bug would arise in the following situation:
* A flow has been dumped before.
* The flow needs to be revalidated.
* The flow is low-throughput.
* The flow has new statistics to push.
Such cases rely on flow deletion to update the stats. However, that code
pushes the delta between the ukey->stats and the final flow dump. If the
ukey stats cache is updated without the stats being pushed, those stats
would be lost.
This caused intermittent testsuite failures on "learning action -
self-modifying flow with idle_timeout". Introduced by 698ffe3623f1b630ae
"revalidator: Only revalidate high-throughput flows."
Bug #1238927.
Signed-off-by: Joe Stringer
---
ofproto/ofproto-dpif-upcall.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-upcall.c
b/ofproto/ofproto-dpif-upcall.c
index 84afc56..5871772 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1245,7 +1245,6 @@ revalidate_ukey(struct udpif *udpif, struct
udpif_key *ukey,
push.n_bytes = stats->n_bytes > ukey->stats.n_bytes
? stats->n_bytes - ukey->stats.n_bytes
: 0;
-ukey->stats = *stats;
if (!ukey->flow_exists) {
/* Don't bother revalidating if the flow was already deleted.
*/
@@ -1258,6 +1257,8 @@ revalidate_ukey(struct udpif *udpif, struct
udpif_key *ukey,
goto exit;
}
+/* We will push the stats, so update the ukey stats cache. */
+ukey->stats = *stats;
if (!push.n_packets && !udpif->need_revalidate) {
ok = true;
goto exit;
--
1.7.10.4
>>>
>>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v5] datapath: Add support for kernel 3.14.
Signed-off-by: Pritesh Kothari --- v5: move skb_clear_rxhash() from compat.h to skbuff.h, simplify skb_get_hash() integration, fix random.h order in acinclude.m4, move pcpu_sw_netstats to netdevice.h. v4: rebase for changes in datapath/actions.c and datapath/vport.c, split patch in two for seperating out skb_clear_hash related stuff. v3: rebase for changes in datapath/vport.c. v2: Use OVS_GREP_IFELSE instead of upstream kernel versions as Thomas suggested. --- FAQ | 2 +- NEWS | 2 +- acinclude.m4 | 10 +++-- datapath/actions.c| 14 ++--- datapath/compat.h | 7 --- datapath/linux/Modules.mk | 2 +- datapath/linux/compat/flow_dissector.c| 2 +- datapath/linux/compat/include/linux/etherdevice.h | 2 ++ datapath/linux/compat/include/linux/if_tunnel.h | 20 -- datapath/linux/compat/include/linux/netdevice.h | 13 datapath/linux/compat/include/linux/random.h | 10 + datapath/linux/compat/include/linux/skbuff.h | 25 ++- datapath/linux/compat/ip_tunnels_core.c | 4 ++-- datapath/linux/compat/vxlan.c | 2 +- datapath/vport-lisp.c | 2 +- datapath/vport.c | 16 +++ datapath/vport.h | 2 +- 17 files changed, 77 insertions(+), 58 deletions(-) delete mode 100644 datapath/linux/compat/include/linux/if_tunnel.h create mode 100644 datapath/linux/compat/include/linux/random.h diff --git a/FAQ b/FAQ index c43b0c8..22a1cf0 100644 --- a/FAQ +++ b/FAQ @@ -149,7 +149,7 @@ A: The following table lists the Linux kernel versions against which the 1.11.x 2.6.18 to 3.8 2.0.x 2.6.32 to 3.10 2.1.x 2.6.32 to 3.11 - 2.2.x 2.6.32 to 3.13 + 2.2.x 2.6.32 to 3.14 Open vSwitch userspace should also work with the Linux kernel module built into Linux 3.3 and later. diff --git a/NEWS b/NEWS index 7925598..292db8e 100644 --- a/NEWS +++ b/NEWS @@ -13,7 +13,7 @@ Post-v2.1.0 - Upon the receipt of a SIGHUP signal, ovs-vswitchd no longer reopens its log file (it will terminate instead). Please use 'ovs-appctl vlog/reopen' instead. - - Support for Linux kernels up to 3.13. From Kernel 3.12 onwards OVS uses + - Support for Linux kernels up to 3.14. From Kernel 3.12 onwards OVS uses tunnel API for GRE and VXLAN. - Added DPDK support. diff --git a/acinclude.m4 b/acinclude.m4 index e6bed37..83110ec 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -134,10 +134,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [ AC_MSG_RESULT([$kversion]) if test "$version" -ge 3; then - if test "$version" = 3 && test "$patchlevel" -le 13; then + if test "$version" = 3 && test "$patchlevel" -le 14; then : # Linux 3.x else - AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version newer than 3.13.x is not supported (please refer to the FAQ for advice)]) + AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version newer than 3.14.x is not supported (please refer to the FAQ for advice)]) fi else if test "$version" -le 1 || test "$patchlevel" -le 5 || test "$sublevel" -le 31; then @@ -244,6 +244,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ OVS_GREP_IFELSE([$KSRC/include/linux/err.h], [ERR_CAST]) OVS_GREP_IFELSE([$KSRC/include/linux/etherdevice.h], [eth_hw_addr_random]) + OVS_GREP_IFELSE([$KSRC/include/linux/etherdevice.h], [ether_addr_copy]) OVS_GREP_IFELSE([$KSRC/include/linux/if_vlan.h], [vlan_set_encap_proto]) @@ -255,6 +256,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [__skb_gso_segment]) OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [can_checksum_protocol]) OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [netdev_features_t]) + OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [pcpu_sw_netstats]) + + OVS_GREP_IFELSE([$KSRC/include/linux/random.h], [prandom_u32]) OVS_GREP_IFELSE([$KSRC/include/linux/rcupdate.h], [rcu_read_lock_held], [], [OVS_GREP_IFELSE([$KSRC/include/linux/rtnetlink.h], @@ -288,6 +292,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [ OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [__skb_fill_page_desc]) OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_reset_mac_len]) OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_unclone]) + OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_get_hash]) + OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_clear_hash]) OVS_GREP_IFELSE([$KSRC/include/linux/types.h], [bool], [OVS_DEFINE([HAVE_BOOL_TYPE])]) diff --git a/dat
Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix a bug.
Thanks for fixing this, Alex. I think there was a bit of mixup the last
time I rebased.
On 26 April 2014 07:33, Alex Wang wrote:
> Applied, thx a lot,
>
> Now, let's fix the build (unittest) issue~~~
>
>
> On Fri, Apr 25, 2014 at 11:31 AM, Gurucharan Shetty wrote:
>
>> On Fri, Apr 25, 2014 at 11:04 AM, Alex Wang wrote:
>> > Commit 7d170098 (ofproto-dpif-upcall: Remove the flow_dumper thread.)
>> > initialized the memory barrier inside the udpif_start_threads()
>> function.
>> > However, the udpif_start_threads() function does not check the number of
>> > revalidator threads specified in udpif. So, when the number is zero, it
>> > causes the error in barrier initialization. This could happen when the
>> > other_config:flow-restore-wait is set and the udpif_flush() is called.
>> >
>> > This commit fixes the issue, by checking the specified number of threads
>> > in udpif_start_threads().
>> >
>> > Reported-by: Gurucharan Shetty
>> > Signed-off-by: Alex Wang
>>
>> This patch fixes the problem for me. It also looks correct. But the
>> code here has changed so much over the past few months, that I do not
>> understand all the implications.
>> So, with that :
>> Acked-by: Gurucharan Shetty
>>
>> > ---
>> > ofproto/ofproto-dpif-upcall.c |2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/ofproto/ofproto-dpif-upcall.c
>> b/ofproto/ofproto-dpif-upcall.c
>> > index 84afc56..717563a 100644
>> > --- a/ofproto/ofproto-dpif-upcall.c
>> > +++ b/ofproto/ofproto-dpif-upcall.c
>> > @@ -342,7 +342,7 @@ static void
>> > udpif_start_threads(struct udpif *udpif, size_t n_handlers,
>> > size_t n_revalidators)
>> > {
>> > -if (udpif && (!udpif->handlers && !udpif->revalidators)) {
>> > +if (udpif && n_handlers && n_revalidators) {
>> > size_t i;
>> >
>> > udpif->n_handlers = n_handlers;
>> > --
>> > 1.7.9.5
>> >
>>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix a bug.
On Fri, Apr 25, 2014 at 5:47 PM, Joe Stringer wrote: > Thanks for fixing this, Alex. I think there was a bit of mixup the last > time I rebased. > Np, actually, I will have another patch to add assert (!udpif->handlers && !udpif->revalidators) ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
