[ovs-dev] [RFC PATCH 1/2] add igmp snooping support
WARNING: This patch *only* compiles! NOT tested yet!
This patch adds the initial IGMP snooping support.
I sent out this patch early to get some early reviews,
especially the design of the code.
Any comments are welcome!
Cc: Ben Pfaff
Cc: Jesse Gross
Not-Yet-Signed-off-by: Cong Wang
---
include/sparse/netinet/in.h |1 +
lib/automake.mk |2 +
lib/flow.c | 21 +++
lib/flow.h |3 +-
lib/igmp-snooping.c | 349 +++
lib/igmp-snooping.h | 102 +
lib/learning-switch.c | 11 ++
lib/packets.h | 15 ++
ofproto/ofproto-dpif.c | 195 +
ofproto/ofproto-provider.h |2 +
ofproto/ofproto.c | 10 ++
ofproto/ofproto.h |2 +
vswitchd/bridge.c |2 +
13 files changed, 688 insertions(+), 27 deletions(-)
create mode 100644 lib/igmp-snooping.c
create mode 100644 lib/igmp-snooping.h
diff --git a/include/sparse/netinet/in.h b/include/sparse/netinet/in.h
index b3924c3..16af225 100644
--- a/include/sparse/netinet/in.h
+++ b/include/sparse/netinet/in.h
@@ -51,6 +51,7 @@ extern const struct in6_addr in6addr_any;
#define IPPROTO_IP 0
#define IPPROTO_HOPOPTS 0
#define IPPROTO_ICMP 1
+#define IPPROTO_IGMP 2
#define IPPROTO_TCP 6
#define IPPROTO_UDP 17
#define IPPROTO_ROUTING 43
diff --git a/lib/automake.mk b/lib/automake.mk
index ce3edc3..33af27a 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -79,6 +79,8 @@ lib_libopenvswitch_a_SOURCES = \
lib/lockfile.h \
lib/mac-learning.c \
lib/mac-learning.h \
+ lib/igmp-snooping.c \
+ lib/igmp-snooping.h \
lib/match.c \
lib/match.h \
lib/memory.c \
diff --git a/lib/flow.c b/lib/flow.c
index 397bda1..9cca7d9 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -87,6 +87,12 @@ pull_icmp(struct ofpbuf *packet)
return ofpbuf_try_pull(packet, ICMP_HEADER_LEN);
}
+static struct igmp_header *
+pull_igmp(struct ofpbuf *packet)
+{
+return ofpbuf_try_pull(packet, IGMP_HEADER_LEN);
+}
+
static struct icmp6_hdr *
pull_icmpv6(struct ofpbuf *packet)
{
@@ -460,6 +466,13 @@ flow_extract_l3_onwards(struct ofpbuf *packet, struct flow
*flow,
flow->tp_dst = htons(icmp->icmp_code);
packet->l7 = b.data;
}
+} else if (flow->nw_proto == IPPROTO_IGMP) {
+const struct igmp_header *igmp = pull_igmp(&b);
+if (igmp) {
+flow->tp_src = htons(igmp->igmp_type);
+flow->tp_dst = htons(igmp->igmp_code);
+flow->igmp_group = htons(igmp->group);
+}
}
}
}
@@ -925,6 +938,14 @@ flow_compose(struct ofpbuf *b, const struct flow *flow)
icmp->icmp_type = ntohs(flow->tp_src);
icmp->icmp_code = ntohs(flow->tp_dst);
icmp->icmp_csum = csum(icmp, ICMP_HEADER_LEN);
+} else if (flow->nw_proto == IPPROTO_IGMP) {
+struct igmp_header *igmp;
+
+b->l4 = igmp = ofpbuf_put_zeros(b, sizeof *igmp);
+igmp->igmp_type = ntohs(flow->tp_src);
+igmp->igmp_code = ntohs(flow->tp_dst);
+igmp->group = ntohs(flow->igmp_group);
+igmp->igmp_csum = csum(igmp, IGMP_HEADER_LEN);
}
}
diff --git a/lib/flow.h b/lib/flow.h
index e6da480..4b540f9 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -98,6 +98,7 @@ struct flow {
ovs_be16 encap_dl_type; /* MPLS encapsulated Ethernet frame type */
ovs_be16 tp_src;/* TCP/UDP source port. */
ovs_be16 tp_dst;/* TCP/UDP destination port. */
+ovs_be32 igmp_group;/* IGMP multicast group. */
uint8_t dl_src[6]; /* Ethernet source address. */
uint8_t dl_dst[6]; /* Ethernet destination address. */
uint8_t nw_proto; /* IP protocol or low 8 bits of ARP opcode. */
@@ -113,7 +114,7 @@ BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0);
#define FLOW_U32S (sizeof(struct flow) / 4)
/* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
-BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_tnl) + 160 &&
+BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_tnl) + 168 &&
FLOW_WC_SEQ == 19);
/* Represents the metadata fields of struct flow. */
diff --git a/lib/igmp-snooping.c b/lib/igmp-snooping.c
new file mode 100644
index 000..3e96b38
--- /dev/null
+++ b/lib/igmp-snooping.c
@@ -0,0 +1,349 @@
+/*
+ * Copyright (c) 2013 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+
[ovs-dev] [RFC PATCH 2/2] add igmp6 snooping support
WARNING: This patch *only* compiles! NOT tested yet!
Based on patch 1/2, this patch adds IPv6 support.
Any comments are welcome!
Cc: Ben Pfaff
Cc: Jesse Gross
Not-Yet-Signed-off-by: Cong Wang
---
lib/flow.c | 18 +---
lib/flow.h | 12 +++-
lib/igmp-snooping.c| 70 ++-
lib/igmp-snooping.h| 11 +---
lib/packets.h | 11 +++
ofproto/ofproto-dpif.c | 64 ---
6 files changed, 144 insertions(+), 42 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index 9cca7d9..cc515aa 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -286,9 +286,11 @@ parse_icmpv6(struct ofpbuf *b, struct flow *flow)
flow->tp_src = htons(icmp->icmp6_type);
flow->tp_dst = htons(icmp->icmp6_code);
-if (icmp->icmp6_code == 0 &&
-(icmp->icmp6_type == ND_NEIGHBOR_SOLICIT ||
- icmp->icmp6_type == ND_NEIGHBOR_ADVERT)) {
+if (icmp->icmp6_code != 0)
+return true;
+
+if (icmp->icmp6_type == ND_NEIGHBOR_SOLICIT ||
+icmp->icmp6_type == ND_NEIGHBOR_ADVERT) {
const struct in6_addr *nd_target;
nd_target = ofpbuf_try_pull(b, sizeof *nd_target);
@@ -330,6 +332,12 @@ parse_icmpv6(struct ofpbuf *b, struct flow *flow)
goto invalid;
}
}
+} else if (icmp->icmp6_type == ICMPV6_MGM_REPORT ||
+ icmp->icmp6_type == ICMPV6_MGM_REDUCTION) {
+const struct mld_hdr *mld = b->data;
+if (b->size < sizeof(*mld))
+goto invalid;
+flow->igmp_group.u.ip6 = mld->mld_addr;
}
return true;
@@ -471,7 +479,7 @@ flow_extract_l3_onwards(struct ofpbuf *packet, struct flow
*flow,
if (igmp) {
flow->tp_src = htons(igmp->igmp_type);
flow->tp_dst = htons(igmp->igmp_code);
-flow->igmp_group = htons(igmp->group);
+flow->igmp_group.u.ip4 = htons(igmp->group);
}
}
}
@@ -944,7 +952,7 @@ flow_compose(struct ofpbuf *b, const struct flow *flow)
b->l4 = igmp = ofpbuf_put_zeros(b, sizeof *igmp);
igmp->igmp_type = ntohs(flow->tp_src);
igmp->igmp_code = ntohs(flow->tp_dst);
-igmp->group = ntohs(flow->igmp_group);
+igmp->group = ntohs(flow->igmp_group.u.ip4);
igmp->igmp_csum = csum(igmp, IGMP_HEADER_LEN);
}
}
diff --git a/lib/flow.h b/lib/flow.h
index 4b540f9..3276a3c 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -59,6 +59,14 @@ BUILD_ASSERT_DECL(FLOW_NW_FRAG_LATER == NX_IP_FRAG_LATER);
const char *flow_tun_flag_to_string(uint32_t flags);
+struct mdb_ip {
+union {
+ovs_be32 ip4;
+struct in6_addr ip6;
+} u;
+uint8_t proto;
+};
+
struct flow_tnl {
ovs_be64 tun_id;
ovs_be32 ip_src;
@@ -98,7 +106,7 @@ struct flow {
ovs_be16 encap_dl_type; /* MPLS encapsulated Ethernet frame type */
ovs_be16 tp_src;/* TCP/UDP source port. */
ovs_be16 tp_dst;/* TCP/UDP destination port. */
-ovs_be32 igmp_group;/* IGMP multicast group. */
+struct mdb_ip igmp_group; /* IGMP multicast group. */
uint8_t dl_src[6]; /* Ethernet source address. */
uint8_t dl_dst[6]; /* Ethernet destination address. */
uint8_t nw_proto; /* IP protocol or low 8 bits of ARP opcode. */
@@ -114,7 +122,7 @@ BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0);
#define FLOW_U32S (sizeof(struct flow) / 4)
/* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
-BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_tnl) + 168 &&
+BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_tnl) + 184 &&
FLOW_WC_SEQ == 19);
/* Represents the metadata fields of struct flow. */
diff --git a/lib/igmp-snooping.c b/lib/igmp-snooping.c
index 3e96b38..22185ec 100644
--- a/lib/igmp-snooping.c
+++ b/lib/igmp-snooping.c
@@ -46,10 +46,17 @@ mdb_entry_age(const struct igmp_mdb *mdb, const struct
mdb_entry *e)
}
static uint32_t
-igmp_mdb_hash(const struct igmp_mdb *mdb, ovs_be32 grp,
+igmp_mdb_hash(const struct igmp_mdb *mdb, struct mdb_ip *grp,
uint16_t vlan)
{
-return hash_3words(grp, vlan, mdb->secret);
+if (grp->proto == htons(ETH_TYPE_IP)) {
+return hash_3words(grp->u.ip4, vlan, mdb->secret);
+} else {
+uint32_t tmp[sizeof(*grp)/sizeof(uint32_t) + 1] = {0};
+memcpy(tmp, grp, sizeof(*grp));
+tmp[sizeof(tmp)/sizeof(uint32_t) - 1] = vlan;
+return hash_words(tmp, sizeof(tmp)/sizeof(uint32_t), mdb->secret);
+}
}
static struct mdb_entry *
@@ -63,22 +70,27 @@ mdb_entry_from_lru_node(struct list *list)
* flooded to be revalidated.) */
static tag_type
make_unknown_mdb_tag(const struct igmp_mdb *m
[ovs-dev] [PATCH branch-1.7] meta-flow: Fix mf_set_wild() on ip_frag field.
Fixes behavior of utilities/ovs-ofctl parse-flow 'ip,ip_frag=any,actions=drop' and other cases that invoke this function on the ip_frag field. Reported-by: love you Signed-off-by: Ben Pfaff --- AUTHORS |1 + lib/meta-flow.c |6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/AUTHORS b/AUTHORS index d718038..5a5d1e6 100644 --- a/AUTHORS +++ b/AUTHORS @@ -160,6 +160,7 @@ YAMAMOTO Takashiyamam...@valinux.co.jp Yongqiang Liu liuyq7...@gmail.com kk yap yap...@stanford.edu likunyunkunyu...@hotmail.com +love youthunder.lov...@gmail.com 冯全树(Crab)fqs...@126.com 胡靖飞 hujingfei...@msn.com diff --git a/lib/meta-flow.c b/lib/meta-flow.c index 7a63922..29f0b0d 100644 --- a/lib/meta-flow.c +++ b/lib/meta-flow.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2012 Nicira, Inc. + * Copyright (c) 2011, 2012, 2013 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -1604,8 +1604,8 @@ mf_set_wild(const struct mf_field *mf, struct cls_rule *rule) break; case MFF_IP_FRAG: -rule->wc.nw_frag_mask |= FLOW_NW_FRAG_MASK; -rule->flow.nw_frag &= ~FLOW_NW_FRAG_MASK; +rule->wc.nw_frag_mask = 0; +rule->flow.nw_frag = 0; break; case MFF_ARP_OP: -- 1.7.10.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] openvswitch: cannot register gre protocol handler
Hi everybody, I am having this error message in dmesg when I: insmod openvswitch_mod.ko openvswitch: cannot register gre protocol handler Then when ovs-vswitchd goes in segmentation fault at the first received packet. I am deploying openvswitch in a custom build linux kernel for DD-WRT( http://www.dd-wrt.com/site/index) Any idea about kernel modules missing? Thank you! -- - *University of Rome "La Sapienza"* *Ing Andi Palo* *PhD student* *Department of Computer, Control and Management Engineering (DIAG)* Via Ariosto, 25 - 00185 Rome - Italy Tel: +39 06 77274034 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] openvswitch: cannot register gre protocol handler
Hi everybody, I am having this error message in dmesg when I: insmod openvswitch_mod.ko openvswitch: cannot register gre protocol handler Then when ovs-vswitchd goes in segmentation fault at the first received packet. I am deploying openvswitch in a custom build linux kernel for DD-WRT( http://www.dd-wrt.com/site/index) Any idea about kernel modules missing? Thank you! -- - *University of Rome "La Sapienza"* *Ing Andi Palo* *PhD student* *Department of Computer, Control and Management Engineering (DIAG)* Via Ariosto, 25 - 00185 Rome - Italy Tel: +39 06 77274034 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] openvswitch: cannot register gre protocol handler
Hi everybody, I am having this error message in dmesg when I: insmod openvswitch_mod.ko openvswitch: cannot register gre protocol handler Then when ovs-vswitchd goes in segmentation fault at the first received packet. I am deploying openvswitch in a custom build linux kernel for DD-WRT(http://www.dd-wrt.com/site/index) Any idea about kernel modules missing? Thank you! Raffaele___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofpbuf: Remove OFPBUF_STACK_BUFFER.
This looks good.
On Wed, Feb 27, 2013 at 5:03 PM, Ben Pfaff wrote:
> Its alleged convenience just doesn't outweigh the syntactical ugliness, and
> so it didn't have any users.
>
> Signed-off-by: Ben Pfaff
> ---
> lib/ofpbuf.c |6 +++---
> lib/ofpbuf.h |6 +-
> 2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
> index fd10da3..c960a7e 100644
> --- a/lib/ofpbuf.c
> +++ b/lib/ofpbuf.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -52,7 +52,7 @@ ofpbuf_use(struct ofpbuf *b, void *base, size_t
> allocated)
> *
> * 'base' should be appropriately aligned. Using an array of uint32_t or
> * uint64_t for the buffer is a reasonable way to ensure appropriate
> alignment
> - * for 32- or 64-bit data. OFPBUF_STACK_BUFFER is a convenient way to do
> so.
> + * for 32- or 64-bit data.
> *
> * An ofpbuf operation that requires reallocating data will assert-fail
> if this
> * function was used to initialize it. Thus, one need not call
> ofpbuf_uninit()
> @@ -72,7 +72,7 @@ ofpbuf_use_stack(struct ofpbuf *b, void *base, size_t
> allocated)
> *
> * 'base' should be appropriately aligned. Using an array of uint32_t or
> * uint64_t for the buffer is a reasonable way to ensure appropriate
> alignment
> - * for 32- or 64-bit data. OFPBUF_STACK_BUFFER is a convenient way to do
> so.
> + * for 32- or 64-bit data.
> *
> * An ofpbuf operation that requires reallocating data will copy the
> provided
> * buffer into a malloc()'d buffer. Thus, it is wise to call
> ofpbuf_uninit()
> diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
> index bae3c0a..8b03c7e 100644
> --- a/lib/ofpbuf.h
> +++ b/lib/ofpbuf.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -52,10 +52,6 @@ struct ofpbuf {
> void *private_p;/* Private pointer for use by owner. */
> };
>
> -/* Declares NAME as a SIZE-byte array aligned properly for storing any
> kind of
> - * data. For use with ofpbuf_use_stack(). */
> -#define OFPBUF_STACK_BUFFER(NAME, SIZE) uint64_t NAME[DIV_ROUND_UP(SIZE,
> 8)]
> -
> void ofpbuf_use(struct ofpbuf *, void *, size_t);
> void ofpbuf_use_stack(struct ofpbuf *, void *, size_t);
> void ofpbuf_use_stub(struct ofpbuf *, void *, size_t);
> --
> 1.7.2.5
>
> ___
> 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] openvswitch: cannot register gre protocol handler
On Fri, Mar 1, 2013 at 10:13 AM, Raffaele wrote: > Hi everybody, > I am having this error message in dmesg when I: insmod openvswitch_mod.ko > openvswitch: cannot register gre protocol handler > You need to unload ip_gre and gre module, openvswitch gre does not work along with kernel gre module for now. > Then when ovs-vswitchd goes in segmentation fault at the first received > packet. > I could not reproduce this segfault, what is version of openvswitch you using? > I am deploying openvswitch in a custom build linux kernel for > DD-WRT(http://www.dd-wrt.com/site/index) > Any idea about kernel modules missing? > > Thank you! > Raffaele > > ___ > 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] ovs-ofctl: Better ovs-ofctl add-flows error reporting
On Thu, Feb 28, 2013 at 07:36:47PM -0800, Andy Zhou wrote:
> When error is encountered with add-flows command, this patch
> adds file name and line number in addition to the parser
> error message to the output.
>
> The file name and line number will not be added to
> the output of "ovs-ofctl add-flow", only parser error
> message will appear. Same as before.
>
> Signed-off-by: Andy Zhou
This is a big step in the right direction. Thank you.
The comment on ds_preprocess_line() is wrong (the function doesn't
clear anything initially in 'ds').
The return value convention for ds_preprocess_line() is unusual for
this code base. Our most common return value conventions are either a
"bool" indicating success or failure, or an "int" with 0 for success
or a positive errno for failure. I guess a "bool" might be best here?
I see a few "char*" etc. in this patch, that should be written "char
*".
This patch changes several function to return NULL for success,
otherwise a "char *" indicating an error. That makes sense, but I
have two suggestions. First, I think that these functions should have
a comment explaining the convention, because it is not a widespread
convention in the code base. Second, I suggest tagging these
functions with WARN_UNUSED_RESULT (see lib/compiler.h), because it is
easy to accidentally forget to check the return value in a caller and
this attribute helps to avoid that mistake.
I think that this change is somewhat incomplete. For example,
parse_named_action() calls parse_output(), which calls
mf_parse_subfield(), which aborts if there is a parse error.
str_to_ofpacts() has a typo:
}else {
should be
} else {
In multiple places, this should not be written on a single line (and
coding style requires {}):
if (err_str) return err_str;
In parse_ofp_str(), I see two potential memory leaks on error paths.
First, the 'string' allocated at the top of the function. Second, the
'ofpacts' buffer allocated near the bottom of the function.
+if (err_str) {
+fprintf(stderr, "OFCTL-ERR [%s:%d] ", file_name, lineno);
+parser_fatal_exit_with_reason(err_str);
+}
I think I'd rather just use "%s:%d: " as the lead-in. I don't think
OFCTL-ERR adds much, and editors that know how to parse error messages
already understand "%s:%d ".
We usually write comments as complete sentences that end with periods
(or with exclamation points if they are very exciting comments):
+/* Display the err_str to stderr, then exit
+ *
+ * This function will free the memory allocated for err_str
+ */
Please write the return type on a separate line:
+void parser_fatal_exit_with_reason(char *err_str)
+{
+fprintf(stderr, "%s\n", err_str);
+free(err_str);
I don't think we use -1 as an exit status right now, EXIT_FAILURE (1)
is more conventional:
+exit(-1);
+}
Thanks,
Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] CodingStyle: Mention our common return value conventions.
CC: Andy Zhou Signed-off-by: Ben Pfaff --- CodingStyle | 12 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/CodingStyle b/CodingStyle index ee7a0e6..22f0f45 100644 --- a/CodingStyle +++ b/CodingStyle @@ -249,6 +249,18 @@ details. (Some compilers also assume that the "if" branch is the more common case, so this can be a real form of optimization as well.) +RETURN VALUES + + For functions that return a success or failure indication, prefer +one of the following return value conventions: + +* An "int" where 0 indicates success and a positive errno value + indicates a reason for failure. + +* A "bool" where true indicates success and false indicates + failure. + + MACROS Don't define an object-like macro if an enum can be used instead. -- 1.7.2.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofpbuf: Remove OFPBUF_STACK_BUFFER.
Acked-by: Ethan Jackson
On Fri, Mar 1, 2013 at 10:26 AM, Andy Zhou wrote:
> This looks good.
>
>
> On Wed, Feb 27, 2013 at 5:03 PM, Ben Pfaff wrote:
>
>> Its alleged convenience just doesn't outweigh the syntactical ugliness,
>> and
>> so it didn't have any users.
>>
>> Signed-off-by: Ben Pfaff
>> ---
>> lib/ofpbuf.c |6 +++---
>> lib/ofpbuf.h |6 +-
>> 2 files changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
>> index fd10da3..c960a7e 100644
>> --- a/lib/ofpbuf.c
>> +++ b/lib/ofpbuf.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
>> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
>> *
>> * Licensed under the Apache License, Version 2.0 (the "License");
>> * you may not use this file except in compliance with the License.
>> @@ -52,7 +52,7 @@ ofpbuf_use(struct ofpbuf *b, void *base, size_t
>> allocated)
>> *
>> * 'base' should be appropriately aligned. Using an array of uint32_t or
>> * uint64_t for the buffer is a reasonable way to ensure appropriate
>> alignment
>> - * for 32- or 64-bit data. OFPBUF_STACK_BUFFER is a convenient way to
>> do so.
>> + * for 32- or 64-bit data.
>> *
>> * An ofpbuf operation that requires reallocating data will assert-fail
>> if this
>> * function was used to initialize it. Thus, one need not call
>> ofpbuf_uninit()
>> @@ -72,7 +72,7 @@ ofpbuf_use_stack(struct ofpbuf *b, void *base, size_t
>> allocated)
>> *
>> * 'base' should be appropriately aligned. Using an array of uint32_t or
>> * uint64_t for the buffer is a reasonable way to ensure appropriate
>> alignment
>> - * for 32- or 64-bit data. OFPBUF_STACK_BUFFER is a convenient way to
>> do so.
>> + * for 32- or 64-bit data.
>> *
>> * An ofpbuf operation that requires reallocating data will copy the
>> provided
>> * buffer into a malloc()'d buffer. Thus, it is wise to call
>> ofpbuf_uninit()
>> diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
>> index bae3c0a..8b03c7e 100644
>> --- a/lib/ofpbuf.h
>> +++ b/lib/ofpbuf.h
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
>> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
>> *
>> * Licensed under the Apache License, Version 2.0 (the "License");
>> * you may not use this file except in compliance with the License.
>> @@ -52,10 +52,6 @@ struct ofpbuf {
>> void *private_p;/* Private pointer for use by owner. */
>> };
>>
>> -/* Declares NAME as a SIZE-byte array aligned properly for storing any
>> kind of
>> - * data. For use with ofpbuf_use_stack(). */
>> -#define OFPBUF_STACK_BUFFER(NAME, SIZE) uint64_t NAME[DIV_ROUND_UP(SIZE,
>> 8)]
>> -
>> void ofpbuf_use(struct ofpbuf *, void *, size_t);
>> void ofpbuf_use_stack(struct ofpbuf *, void *, size_t);
>> void ofpbuf_use_stub(struct ofpbuf *, void *, size_t);
>> --
>> 1.7.2.5
>>
>> ___
>> 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] openvswitch: cannot register gre protocol handler
On Fri, Mar 01, 2013 at 11:00:10AM -0800, Pravin Shelar wrote: > On Fri, Mar 1, 2013 at 10:13 AM, Raffaele wrote: > > Hi everybody, > > I am having this error message in dmesg when I: insmod openvswitch_mod.ko > > openvswitch: cannot register gre protocol handler > > > You need to unload ip_gre and gre module, openvswitch gre does not > work along with kernel gre module for now. > > > Then when ovs-vswitchd goes in segmentation fault at the first received > > packet. > > > I could not reproduce this segfault, what is version of openvswitch you using? Maybe there's a kernel OOPS that manifests as a segfault. Raffaele, do you see any OOPSes in "dmesg"? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofpbuf: Remove OFPBUF_STACK_BUFFER.
Thanks, I applied it to master.
On Fri, Mar 01, 2013 at 10:26:56AM -0800, Andy Zhou wrote:
> This looks good.
>
>
> On Wed, Feb 27, 2013 at 5:03 PM, Ben Pfaff wrote:
>
> > Its alleged convenience just doesn't outweigh the syntactical ugliness, and
> > so it didn't have any users.
> >
> > Signed-off-by: Ben Pfaff
> > ---
> > lib/ofpbuf.c |6 +++---
> > lib/ofpbuf.h |6 +-
> > 2 files changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
> > index fd10da3..c960a7e 100644
> > --- a/lib/ofpbuf.c
> > +++ b/lib/ofpbuf.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -52,7 +52,7 @@ ofpbuf_use(struct ofpbuf *b, void *base, size_t
> > allocated)
> > *
> > * 'base' should be appropriately aligned. Using an array of uint32_t or
> > * uint64_t for the buffer is a reasonable way to ensure appropriate
> > alignment
> > - * for 32- or 64-bit data. OFPBUF_STACK_BUFFER is a convenient way to do
> > so.
> > + * for 32- or 64-bit data.
> > *
> > * An ofpbuf operation that requires reallocating data will assert-fail
> > if this
> > * function was used to initialize it. Thus, one need not call
> > ofpbuf_uninit()
> > @@ -72,7 +72,7 @@ ofpbuf_use_stack(struct ofpbuf *b, void *base, size_t
> > allocated)
> > *
> > * 'base' should be appropriately aligned. Using an array of uint32_t or
> > * uint64_t for the buffer is a reasonable way to ensure appropriate
> > alignment
> > - * for 32- or 64-bit data. OFPBUF_STACK_BUFFER is a convenient way to do
> > so.
> > + * for 32- or 64-bit data.
> > *
> > * An ofpbuf operation that requires reallocating data will copy the
> > provided
> > * buffer into a malloc()'d buffer. Thus, it is wise to call
> > ofpbuf_uninit()
> > diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
> > index bae3c0a..8b03c7e 100644
> > --- a/lib/ofpbuf.h
> > +++ b/lib/ofpbuf.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> > *
> > * Licensed under the Apache License, Version 2.0 (the "License");
> > * you may not use this file except in compliance with the License.
> > @@ -52,10 +52,6 @@ struct ofpbuf {
> > void *private_p;/* Private pointer for use by owner. */
> > };
> >
> > -/* Declares NAME as a SIZE-byte array aligned properly for storing any
> > kind of
> > - * data. For use with ofpbuf_use_stack(). */
> > -#define OFPBUF_STACK_BUFFER(NAME, SIZE) uint64_t NAME[DIV_ROUND_UP(SIZE,
> > 8)]
> > -
> > void ofpbuf_use(struct ofpbuf *, void *, size_t);
> > void ofpbuf_use_stack(struct ofpbuf *, void *, size_t);
> > void ofpbuf_use_stub(struct ofpbuf *, void *, size_t);
> > --
> > 1.7.2.5
> >
> > ___
> > 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] ovsdb: Allow recovery from transient write errors in log implementation.
Until now, the OVSDB data journaling implementation has made write errors
"sticky", so that a single write error persists as long as ovsdb-server is
alive. However, some kinds of write errors (such as ENOSPC) can be
transient in practice. I don't know of a good reason to make such errors
sticky, so this commit makes the journaling code retry writes even after
an error occurs, allowing ovsdb-server to recover from transient errors.
Reported-by: likunyun
Signed-off-by: Ben Pfaff
---
ovsdb/log.c | 14 ++
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/ovsdb/log.c b/ovsdb/log.c
index 440e8d0..ea3c3f3 100644
--- a/ovsdb/log.c
+++ b/ovsdb/log.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -48,7 +48,7 @@ struct ovsdb_log {
struct lockfile *lockfile;
FILE *stream;
struct ovsdb_error *read_error;
-struct ovsdb_error *write_error;
+bool write_error;
enum ovsdb_log_mode mode;
};
@@ -133,7 +133,7 @@ ovsdb_log_open(const char *name, enum ovsdb_log_open_mode
open_mode,
file->prev_offset = 0;
file->offset = 0;
file->read_error = NULL;
-file->write_error = NULL;
+file->write_error = false;
file->mode = OVSDB_LOG_READ;
*filep = file;
return NULL;
@@ -154,7 +154,6 @@ ovsdb_log_close(struct ovsdb_log *file)
fclose(file->stream);
lockfile_unlock(file->lockfile);
ovsdb_error_destroy(file->read_error);
-ovsdb_error_destroy(file->write_error);
free(file);
}
}
@@ -332,10 +331,9 @@ ovsdb_log_write(struct ovsdb_log *file, struct json *json)
json_string = NULL;
-if (file->write_error) {
-return ovsdb_error_clone(file->write_error);
-} else if (file->mode == OVSDB_LOG_READ) {
+if (file->mode == OVSDB_LOG_READ || file->write_error) {
file->mode = OVSDB_LOG_WRITE;
+file->write_error = false;
if (fseeko(file->stream, file->offset, SEEK_SET)) {
error = ovsdb_io_error(errno, "%s: cannot seek to offset %lld",
file->name, (long long int) file->offset);
@@ -383,7 +381,7 @@ ovsdb_log_write(struct ovsdb_log *file, struct json *json)
return NULL;
error:
-file->write_error = ovsdb_error_clone(error);
+file->write_error = true;
free(json_string);
return error;
}
--
1.7.2.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] openvswitch: cannot register gre protocol handler
Thank for the answers, @Pravin I'don't have that compiled as module so i have to make a new image with gre "stuff" compiled as module and try. The version is 1.6.1. This is a run [1]. @Ben This is the dmesg [0] bu i don't have OOPSes in dmesg. Raffaele [0] http://pastebin.com/uSFszmCs [1] http://pastebin.com/amhPFJ29 Il giorno 01/mar/2013, alle ore 20:04, Ben Pfaff ha scritto: > On Fri, Mar 01, 2013 at 11:00:10AM -0800, Pravin Shelar wrote: >> On Fri, Mar 1, 2013 at 10:13 AM, Raffaele wrote: >>> Hi everybody, >>> I am having this error message in dmesg when I: insmod openvswitch_mod.ko >>> openvswitch: cannot register gre protocol handler >>> >> You need to unload ip_gre and gre module, openvswitch gre does not >> work along with kernel gre module for now. >> >>> Then when ovs-vswitchd goes in segmentation fault at the first received >>> packet. >>> >> I could not reproduce this segfault, what is version of openvswitch you >> using? > > Maybe there's a kernel OOPS that manifests as a segfault. Raffaele, > do you see any OOPSes in "dmesg"? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] openvswitch: cannot register gre protocol handler
On Fri, Mar 01, 2013 at 08:19:44PM +0100, Raffaele wrote: > @Ben > This is the dmesg [0] bu i don't have OOPSes in dmesg. OK, in that case can you get a backtrace from the segmentation fault? Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] openvswitch: cannot register gre protocol handler
Il giorno 01/mar/2013, alle ore 20:23, Ben Pfaff ha scritto: > On Fri, Mar 01, 2013 at 08:19:44PM +0100, Raffaele wrote: >> @Ben >> This is the dmesg [0] bu i don't have OOPSes in dmesg. > > OK, in that case can you get a backtrace from the segmentation fault? > > Thanks, > > Ben. No backtrace at the moment, i'll try to fix removing the gre standard modules. Raffaele ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] CodingStyle: Mention our common return value conventions.
Looks good. --Justin On Mar 1, 2013, at 11:02 AM, Ben Pfaff wrote: > CC: Andy Zhou > Signed-off-by: Ben Pfaff > --- > CodingStyle | 12 > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/CodingStyle b/CodingStyle > index ee7a0e6..22f0f45 100644 > --- a/CodingStyle > +++ b/CodingStyle > @@ -249,6 +249,18 @@ details. (Some compilers also assume that the "if" > branch is the more > common case, so this can be a real form of optimization as well.) > > > +RETURN VALUES > + > + For functions that return a success or failure indication, prefer > +one of the following return value conventions: > + > +* An "int" where 0 indicates success and a positive errno value > + indicates a reason for failure. > + > +* A "bool" where true indicates success and false indicates > + failure. > + > + > MACROS > > Don't define an object-like macro if an enum can be used instead. > -- > 1.7.2.5 > > ___ > 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] Add Nicira vendor extension actions NXAST_STACK_PUSH/POP
Thanks for the comments. I will post the reworked patch based on the
feedbacks to this thread soon.
On Wed, Feb 27, 2013 at 8:44 PM, Ben Pfaff wrote:
> On Wed, Feb 27, 2013 at 05:12:16PM -0800, Ethan Jackson wrote:
> > +static union mf_value*
> > > +nx_stack_pop(struct ofpbuf* stack)
> > > +{
> > > +union mf_value* v = NULL;
> > > +if (stack->size) {
> > > +stack -> size -= sizeof *v;
> > > +v = (union mf_value*)((char*)stack->data + stack->size);
> > > +}
> > > +
> > > +return v;
> > > +}
> > > +
> > >
> >
> > Any reason we can't just use ofpbuf_try_pull() instead?
>
> I think that would make it a queue instead of a stack.
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] nicira-ext: Add Nicira actions NXAST_STACK_PUSH and NXAST_STACK_POP.
The Push action takes a single parameter. Any source allowed by NXAST_REG_MOVE
is allowed to be pushed onto the stack. When the source is a bit field,
its value will be right shifted to bit zero before being pushed onto the
stack. The remaining bits will be set to zero.
The Pop action also takes a single parameter. Any destination allowed by
NXAST_REG_MOVE can be used as the destination of the action. The value, in
case of a bit field, will be taken from top of the stack, starting from
bit zero.
The stack size is not limited. The initial 8KB is statically allocated to
efficiently handle most common use cases. When more stack space is
required, the stack can grow using malloc().
Signed-off-by: Andy Zhou
---
NEWS |2 +
include/openflow/nicira-ext.h | 21 ++
lib/nx-match.c| 160 +
lib/nx-match.h| 26 +++
lib/ofp-actions.c | 38 ++
lib/ofp-actions.h | 10 +++
lib/ofp-parse.c |4 ++
lib/ofp-util.def |2 +
ofproto/ofproto-dpif.c| 32 +
tests/ofproto-dpif.at | 20 ++
tests/ovs-ofctl.at|2 +
utilities/ovs-ofctl.8.in | 17 +
12 files changed, 334 insertions(+)
diff --git a/NEWS b/NEWS
index 35b9212..222f240 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,8 @@ post-v1.10.0
- New support for the data encapsulation format of the LISP tunnel
protocol (RFC 6830). An external control plane or manual flow
setup is required for EID-to-RLOC mapping.
+- New "stack" extension for use in actions, to push and pop
+ from NXM fields.
v1.10.0 - xx xxx
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index c4ff904..fac998d 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -306,6 +306,8 @@ enum nx_action_subtype {
NXAST_WRITE_METADATA, /* struct nx_action_write_metadata */
NXAST_PUSH_MPLS,/* struct nx_action_push_mpls */
NXAST_POP_MPLS, /* struct nx_action_pop_mpls */
+NXAST_STACK_PUSH, /* struct nx_action_stack */
+NXAST_STACK_POP,/* struct nx_action_stack */
};
/* Header for Nicira-defined actions. */
@@ -560,6 +562,25 @@ struct nx_action_reg_load {
};
OFP_ASSERT(sizeof(struct nx_action_reg_load) == 24);
+/* Action structure for NXAST_STACK_PUSH.
+ *
+ * Pushes (or pop) field[offset: offset + n_bits] to (or from)
+ * top of the stack.
+ */
+struct nx_action_stack {
+ovs_be16 type; /* OFPAT_VENDOR. */
+ovs_be16 len; /* Length is 16. */
+ovs_be32 vendor;/* NX_VENDOR_ID. */
+ovs_be16 subtype; /* NXAST_REG_PUSH or NXAST_REG_POP. */
+ovs_be16 offset;/* Bit offset into the field. */
+ovs_be16 n_bits;/* (n_bits + 1) bits of the field. */
+uint8_t zero1[2]; /* Padding to keep next field 8 byte
+ aligned */
+ovs_be32 field; /* The field used for push or pop. */
+uint8_t zero2[4]; /* Reserved for future use, must be zero.*/
+};
+OFP_ASSERT(sizeof(struct nx_action_stack) == 24);
+
/* Action structure for NXAST_NOTE.
*
* This action has no effect. It is variable length. The switch does not
diff --git a/lib/nx-match.c b/lib/nx-match.c
index e5545de..ab1b7c7 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1312,3 +1312,163 @@ nxm_reg_load(const struct mf_subfield *dst, uint64_t
src_data,
sizeof src_data_be * 8);
mf_write_subfield_flow(dst, &src_subvalue, flow);
}
+
+/* nxm_parse_stack_action, works for both push() and pop(). */
+void
+nxm_parse_stack_action(struct ofpact_stack *stack_action, const char *s)
+{
+s = mf_parse_subfield(&stack_action->subfield, s);
+if (*s != '\0') {
+ovs_fatal(0, "%s: trailing garbage following push or pop)", s);
+}
+}
+
+void
+nxm_format_stack_push(const struct ofpact_stack *push, struct ds *s)
+{
+ds_put_cstr(s, "push:");
+mf_format_subfield(&push->subfield, s);
+}
+
+void
+nxm_format_stack_pop(const struct ofpact_stack *pop, struct ds *s)
+{
+ds_put_cstr(s, "pop:");
+mf_format_subfield(&pop->subfield, s);
+}
+
+/* Common set for both push and pop actions. */
+static inline void
+stack_action_from_openflow_common__(const struct nx_action_stack *nasp,
+struct ofpact_stack * stack_action)
+{
+stack_action->subfield.field = mf_from_nxm_header(ntohl(nasp->field));
+stack_action->subfield.ofs = ntohs(nasp->offset);
+stack_action->subfield.n_bits = ntohs(nasp->n_bits);
+}
+
+static inline void
+nxm_stack_to_nxast_common__(const struct ofpact_stack *stack_action,
+struct nx_action_stack *nasp)
+{
+nasp->offset = htons(stack_action->subfield.ofs
