[ovs-dev] [PATCH v2.39 1/7] odp: Only pass vlan_tci to commit_vlan_action()

[Simon Horman]

From: Joe Stringer 

This allows for future patches to pass different tci values to
commit_vlan_action() without passing an entire flow structure.

Signed-off-by: Joe Stringer 
Signed-off-by: Simon Horman 

---

v2.36 - v2.39
* No change

v2.35
* First post
---
 lib/odp-util.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index f20bd8a..11aa32f 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3326,10 +3326,10 @@ commit_set_ether_addr_action(const struct flow *flow, 
struct flow *base,
 }
 
 static void
-commit_vlan_action(const struct flow *flow, struct flow *base,
+commit_vlan_action(ovs_be16 vlan_tci, struct flow *base,
struct ofpbuf *odp_actions, struct flow_wildcards *wc)
 {
-if (base->vlan_tci == flow->vlan_tci) {
+if (base->vlan_tci == vlan_tci) {
 return;
 }
 
@@ -3339,15 +3339,15 @@ commit_vlan_action(const struct flow *flow, struct flow 
*base,
 nl_msg_put_flag(odp_actions, OVS_ACTION_ATTR_POP_VLAN);
 }
 
-if (flow->vlan_tci & htons(VLAN_CFI)) {
+if (vlan_tci & htons(VLAN_CFI)) {
 struct ovs_action_push_vlan vlan;
 
 vlan.vlan_tpid = htons(ETH_TYPE_VLAN);
-vlan.vlan_tci = flow->vlan_tci;
+vlan.vlan_tci = vlan_tci;
 nl_msg_put_unspec(odp_actions, OVS_ACTION_ATTR_PUSH_VLAN,
   &vlan, sizeof vlan);
 }
-base->vlan_tci = flow->vlan_tci;
+base->vlan_tci = vlan_tci;
 }
 
 static void
@@ -3567,7 +3567,7 @@ commit_odp_actions(const struct flow *flow, struct flow 
*base,
struct ofpbuf *odp_actions, struct flow_wildcards *wc)
 {
 commit_set_ether_addr_action(flow, base, odp_actions, wc);
-commit_vlan_action(flow, base, odp_actions, wc);
+commit_vlan_action(flow->vlan_tci, base, odp_actions, wc);
 commit_set_nw_action(flow, base, odp_actions, wc);
 commit_set_port_action(flow, base, odp_actions, wc);
 /* Committing MPLS actions should occur after committing nw and port
-- 
1.8.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2.39 3/7] ofp-actions: Add OFPUTIL_OFPAT13_PUSH_MPLS

[Simon Horman]

From: Joe Stringer 

This patch adds a new compatibility enum for use with MPLS, so that the
differing behaviour between OpenFlow 1.2 and 1.3 can be implemented in
ofproto-dpif-xlate.

Signed-off-by: Joe Stringer 
Signed-off-by: Simon Horman 

---

v2.36 - v2.39
* No change

v2.35
* First post
---
 lib/ofp-actions.c | 5 -
 lib/ofp-parse.c   | 1 +
 lib/ofp-util.c| 3 +++
 lib/ofp-util.h| 1 +
 4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 77aa69c..964ecd7 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -322,6 +322,7 @@ ofpact_from_nxast(const union ofp_action *a, enum 
ofputil_action_code code,
 #define OFPAT10_ACTION(ENUM, STRUCT, NAME) case OFPUTIL_##ENUM:
 #define OFPAT11_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) case OFPUTIL_##ENUM:
 #include "ofp-util.def"
+case OFPUTIL_OFPAT13_PUSH_MPLS:
 NOT_REACHED();
 
 case OFPUTIL_NXAST_RESUBMIT:
@@ -480,6 +481,7 @@ ofpact_from_openflow10(const union ofp_action *a, struct 
ofpbuf *out)
 case OFPUTIL_ACTION_INVALID:
 #define OFPAT11_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) case OFPUTIL_##ENUM:
 #include "ofp-util.def"
+case OFPUTIL_OFPAT13_PUSH_MPLS:
 NOT_REACHED();
 
 case OFPUTIL_OFPAT10_OUTPUT:
@@ -842,7 +844,8 @@ ofpact_from_openflow11(const union ofp_action *a, struct 
ofpbuf *out)
 ofpact_put_DEC_MPLS_TTL(out);
 break;
 
-case OFPUTIL_OFPAT11_PUSH_MPLS: {
+case OFPUTIL_OFPAT11_PUSH_MPLS:
+case OFPUTIL_OFPAT13_PUSH_MPLS: {
 struct ofp11_action_push *oap = (struct ofp11_action_push *)a;
 if (!eth_type_mpls(oap->ethertype)) {
 return OFPERR_OFPBAC_BAD_ARGUMENT;
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index a61beb9..9aac20b 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -804,6 +804,7 @@ parse_named_action(enum ofputil_action_code code,
 break;
 
 case OFPUTIL_OFPAT11_PUSH_MPLS:
+case OFPUTIL_OFPAT13_PUSH_MPLS:
 case OFPUTIL_NXAST_PUSH_MPLS:
 error = str_to_u16(arg, "push_mpls", ðertype);
 if (!error) {
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 23c7136..87336e2 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -4768,6 +4768,9 @@ ofputil_put_action(enum ofputil_action_code code, struct 
ofpbuf *buf)
 case OFPUTIL_ACTION_INVALID:
 NOT_REACHED();
 
+case OFPUTIL_OFPAT13_PUSH_MPLS:
+return ofputil_put_OFPAT11_PUSH_MPLS(buf);
+
 #define OFPAT10_ACTION(ENUM, STRUCT, NAME)  \
 case OFPUTIL_##ENUM: return ofputil_put_##ENUM(buf);
 #define OFPAT11_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)  \
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 0ca483c..3ca189d 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -787,6 +787,7 @@ enum OVS_PACKED_ENUM ofputil_action_code {
 #define OFPAT11_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) OFPUTIL_##ENUM,
 #define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)   OFPUTIL_##ENUM,
 #include "ofp-util.def"
+OFPUTIL_OFPAT13_PUSH_MPLS
 };
 
 /* The number of values of "enum ofputil_action_code". */
-- 
1.8.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2.39 4/7] ofp-actions: Add separate OpenFlow 1.3 action parser

[Simon Horman]

From: Joe Stringer 

This patch adds new ofpact_from_openflow13() and
ofpacts_from_openflow13() functions parallel to the existing ofpact
handling code. In the OpenFlow 1.3 version, push_mpls is handled
differently, but all other actions are handled by the existing code.

For push_mpls, ofpact_push_mpls.ofpact.compat is set to
OFPUTIL_OFPAT13_PUSH_MPLS, which allows correct VLAN+MPLS datapath
behaviour to be determined at odp translation time.

Signed-off-by: Joe Stringer 
Signed-off-by: Simon Horman 

---

v2.36 - v2.39
* No change

v2.35
* First post
---
 lib/ofp-actions.c | 63 ---
 1 file changed, 60 insertions(+), 3 deletions(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 964ecd7..a890773 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -884,6 +884,40 @@ ofpacts_from_openflow11(const union ofp_action *in, size_t 
n_in,
 return ofpacts_from_openflow(in, n_in, out, ofpact_from_openflow11);
 }
 
+static enum ofperr
+ofpact_from_openflow13(const union ofp_action *a, struct ofpbuf *out)
+{
+enum ofputil_action_code code;
+enum ofperr error;
+
+error = decode_openflow11_action(a, &code);
+if (error) {
+return error;
+}
+
+if (code == OFPUTIL_OFPAT11_PUSH_MPLS) {
+struct ofpact_push_mpls *oam;
+struct ofp11_action_push *oap = (struct ofp11_action_push *)a;
+if (!eth_type_mpls(oap->ethertype)) {
+return OFPERR_OFPBAC_BAD_ARGUMENT;
+}
+oam = ofpact_put_PUSH_MPLS(out);
+oam->ethertype = oap->ethertype;
+oam->ofpact.compat = OFPUTIL_OFPAT13_PUSH_MPLS;
+} else {
+return ofpact_from_openflow11(a, out);
+}
+
+return error;
+}
+
+static enum ofperr
+ofpacts_from_openflow13(const union ofp_action *in, size_t n_in,
+struct ofpbuf *out)
+{
+return ofpacts_from_openflow(in, n_in, out, ofpact_from_openflow13);
+}
+
 /* OpenFlow 1.1 instructions. */
 
 #define DEFINE_INST(ENUM, STRUCT, EXTENSIBLE, NAME) \
@@ -1088,6 +1122,17 @@ get_actions_from_instruction(const struct 
ofp11_instruction *inst,
 *n_actions = (ntohs(inst->len) - sizeof *inst) / OFP11_INSTRUCTION_ALIGN;
 }
 
+static uint8_t
+get_version_from_ofpbuf(const struct ofpbuf *openflow)
+{
+if (openflow && openflow->l2) {
+struct ofp_header *oh = openflow->l2;
+return oh->version;
+}
+
+return OFP10_VERSION;
+}
+
 /* Attempts to convert 'actions_len' bytes of OpenFlow 1.1 actions from the
  * front of 'openflow' into ofpacts.  On success, replaces any existing content
  * in 'ofpacts' by the converted ofpacts; on failure, clears 'ofpacts'.
@@ -1107,8 +1152,15 @@ ofpacts_pull_openflow11_actions(struct ofpbuf *openflow,
 unsigned int actions_len,
 struct ofpbuf *ofpacts)
 {
-return ofpacts_pull_actions(openflow, actions_len, ofpacts,
-ofpacts_from_openflow11);
+uint8_t version = get_version_from_ofpbuf(openflow);
+
+if (version < OFP13_VERSION) {
+return ofpacts_pull_actions(openflow, actions_len, ofpacts,
+ofpacts_from_openflow11);
+} else {
+return ofpacts_pull_actions(openflow, actions_len, ofpacts,
+ofpacts_from_openflow13);
+}
 }
 
 enum ofperr
@@ -1160,10 +1212,15 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf 
*openflow,
 if (insts[OVSINST_OFPIT11_APPLY_ACTIONS]) {
 const union ofp_action *actions;
 size_t n_actions;
+uint8_t version = get_version_from_ofpbuf(openflow);
 
 get_actions_from_instruction(insts[OVSINST_OFPIT11_APPLY_ACTIONS],
  &actions, &n_actions);
-error = ofpacts_from_openflow11(actions, n_actions, ofpacts);
+if (version < OFP13_VERSION) {
+error = ofpacts_from_openflow11(actions, n_actions, ofpacts);
+} else {
+error = ofpacts_from_openflow13(actions, n_actions, ofpacts);
+}
 if (error) {
 goto exit;
 }
-- 
1.8.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2.39 2/7] odp: Allow VLAN actions after MPLS actions

[Simon Horman]

From: Joe Stringer 

OpenFlow 1.2 and 1.3 differ on their handling of MPLS actions in the
presence of VLAN tags. To allow correct behaviour to be committed in
each situation, this patch adds a second round of VLAN tag action
handling to commit_odp_actions(), which occurs after MPLS actions. This
is implemented with a new field in 'struct xlate_in' called 'vlan_tci'.

When an push_mpls action is composed, the flow's current VLAN state is
stored into xin->vlan_tci, and flow->vlan_tci is set to 0 (pop_vlan). If
a VLAN tag is present, it is stripped; if not, then there is no change.
Any later modifications to the VLAN state is written to xin->vlan_tci.
When committing the actions, flow->vlan_tci is used before MPLS actions,
and xin->vlan_tci is used afterwards. This retains the current datapath
behaviour, but allows VLAN actions to be applied in a more flexible
manner.

Signed-off-by: Joe Stringer 
Signed-off-by: Simon Horman 

---

v2.38 - v2.39
* No change

v2.37
* Rebase

v2.36
* No change

v2.5
* First post
---
 lib/odp-util.c   |  10 ++-
 lib/odp-util.h   |   4 +-
 ofproto/ofproto-dpif-xlate.c |  95 +++-
 ofproto/ofproto-dpif-xlate.h |   5 ++
 tests/ofproto-dpif.at| 209 +++
 5 files changed, 297 insertions(+), 26 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 11aa32f..6ed7aa3 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3561,10 +3561,15 @@ commit_set_pkt_mark_action(const struct flow *flow, 
struct flow *base,
  * key from 'base' into 'flow', and then changes 'base' the same way.  Does not
  * commit set_tunnel actions.  Users should call commit_odp_tunnel_action()
  * in addition to this function if needed.  Sets fields in 'wc' that are
- * used as part of the action. */
+ * used as part of the action.
+ *
+ * VLAN actions may be committed twice; If vlan_tci in 'flow' differs from the
+ * one in 'base', then it is committed before MPLS actions. If 'final_vlan_tci'
+ * differs from 'flow->vlan_tci', it is committed afterwards. */
 void
 commit_odp_actions(const struct flow *flow, struct flow *base,
-   struct ofpbuf *odp_actions, struct flow_wildcards *wc)
+   struct ofpbuf *odp_actions, struct flow_wildcards *wc,
+   ovs_be16 final_vlan_tci)
 {
 commit_set_ether_addr_action(flow, base, odp_actions, wc);
 commit_vlan_action(flow->vlan_tci, base, odp_actions, wc);
@@ -3575,6 +3580,7 @@ commit_odp_actions(const struct flow *flow, struct flow 
*base,
  * that it is no longer IP and thus nw and port actions are no longer 
valid.
  */
 commit_mpls_action(flow, base, odp_actions, wc);
+commit_vlan_action(final_vlan_tci, base, odp_actions, wc);
 commit_set_priority_action(flow, base, odp_actions, wc);
 commit_set_pkt_mark_action(flow, base, odp_actions, wc);
 }
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 192cfa0..be68d4e 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -130,8 +130,8 @@ const char *odp_key_fitness_to_string(enum odp_key_fitness);
 void commit_odp_tunnel_action(const struct flow *, struct flow *base,
   struct ofpbuf *odp_actions);
 void commit_odp_actions(const struct flow *, struct flow *base,
-struct ofpbuf *odp_actions,
-struct flow_wildcards *wc);
+struct ofpbuf *odp_actions, struct flow_wildcards *wc,
+ovs_be16 final_vlan_tci);
 
 /* ofproto-dpif interface.
  *
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e7cec14..2fa7785 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -974,10 +974,11 @@ static void
 output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
   uint16_t vlan)
 {
-ovs_be16 *flow_tci = &ctx->xin->flow.vlan_tci;
+ovs_be16 *flow_tci = &ctx->xin->vlan_tci;
 uint16_t vid;
 ovs_be16 tci, old_tci;
 struct xport *xport;
+bool flow_tci_equal_to_xin = (*flow_tci == ctx->xin->flow.vlan_tci);
 
 vid = output_vlan_to_vid(out_xbundle, vlan);
 if (list_is_empty(&out_xbundle->xports)) {
@@ -1008,9 +1009,15 @@ output_normal(struct xlate_ctx *ctx, const struct 
xbundle *out_xbundle,
 }
 }
 *flow_tci = tci;
+if (flow_tci_equal_to_xin) {
+ctx->xin->flow.vlan_tci = tci;
+}
 
 compose_output_action(ctx, xport->ofp_port);
 *flow_tci = old_tci;
+if (flow_tci_equal_to_xin) {
+ctx->xin->flow.vlan_tci = old_tci;
+}
 }
 
 /* A VM broadcasts a gratuitous ARP to indicate that it has resumed after
@@ -1243,7 +1250,7 @@ xlate_normal(struct xlate_ctx *ctx)
 
 /* Drop malformed frames. */
 if (flow->dl_type == htons(ETH_TYPE_VLAN) &&
-!(flow->vlan_tci & htons(VLAN_CFI))) {
+!(ctx->xin->vlan_tci & htons(VLAN_CFI))) {
 if (ctx->xin->packet != NULL) {
 static struct vlog

[ovs-dev] [PATCH v2.39 7/7] datapath: Add basic MPLS support to kernel

[Simon Horman]

Allow datapath to recognize and extract MPLS labels into flow keys
and execute actions which push, pop, and set labels on packets.

Based heavily on work by Leo Alterman, Ravi K, Isaku Yamahata and Joe Stringer.

Cc: Ravi K 
Cc: Leo Alterman 
Cc: Isaku Yamahata 
Cc: Joe Stringer 
Signed-off-by: Simon Horman 

---

v2.39
* Rebase for removal of vlan, checksum and skb->mark compat code

v2.38
* Rebase for SCTP support
* Refactor validate_tp_port() to iterate over eth_types rather
  than open-coding the loop. With the addition of SCTP this logic
  is now used three times.

v2.36 - v2.37
* Rebase

* Do not add set_ethertype() to datapath/actions.c.
  As this patch has evolved this function had devolved into
  to sets of functionality wrapped into a single function with
  only one line of common code. Refactor things to simply
  open-code setting the ether type in the two locations where
  set_ethertype() was previously used. The aim here is to improve
  readability.

* Update setting skb->ethertype after mpls push and pop.
  - In the case of push_mpls it should be set unconditionally
as in v2.35 the behaviour of this function to always push
an MPLS LSE before any VLAN tags.
  - In the case of mpls_pop eth_p_mpls(skb->protocol) is a better
test than skb->protocol != htons(ETH_P_8021Q) as it will give the
correct behaviour in the presence of other VLAN ethernet types,
for example 0x88a8 which is used by 802.1ad. Moreover, it seems
correct to update the ethernet type if it was previously set
according to the top-most MPLS LSE.

* Deaccelerate VLANs when pushing MPLS tags the
  - Since v2.35 MPLS push will insert an MPLS LSE before any VLAN tags.
This means that if an accelerated tag is present it should be
deaccelerated to ensure it ends up in the correct position.

* Update skb->mac_len in push_mpls() so that it will be correct
  when used by a subsequent call to pop_mpls().

  As things stand I do not believe this is strictly necessary as
  ovs-vswitchd will not send a pop MPLS action after a push MPLS action.
  However, I have added this in order to code more defensively as I believe
  that if such a sequence did occur it would be rather unobvious why
  it didn't work.

* Do not add skb_cow_head() call in push_mpls().
  It is unnecessary as there is a make_writable() call.
  This change was also made in v2.30 but some how the
  code regressed between then and v2.35.

v2.35
* Rebase
* Move MPLS constants to mpls.h
* Push MPLS tags after ethernet, before VLAN tags
  - This is consistent with the OpenFlow 1.3 specification
  - Compatibility with OpenFlow 1.2 and earlier versions
may be provided by ovs-vswitchd.
* Correct GSO behaviour in the presence of MPLS but absence of VLANs

v2.34
* Rebase for megaflow changes

v2.33
* Ensure that inner_protocol is always set to to the current
  skb->protocol value in ovs_execute_actions(). This ensures
  it is set to the correct value in the absence of a push_mpls action.
  Also remove setting of inner_protocol in push_mpls() as
  it duplicates the code now in ovs_execute_actions().
* Call __skb_gso_segment() instead of skb_gso_segment() from
  rpl___skb_gso_segment() in the case that HAVE___SKB_GSO_SEGMENT is set.
  This was a typo.

v2.32
* As suggested by Jesse Gross
  - Use int instead of size_t in validate_and_copy_actions__().
  - Fix crazy edit mess in pop_mpls() action comment
  - Move eth_p_mpls() into mpls.h
  - Refactor skb_gso_segment MPLS handling into rpl_skb_gso_segment
Address Jesse's comments regarding this code:
"Can we push this completely into the skb_gso_segment() compatibility
 code? It's both nicer and may make the interactions with the vlan code
 less confusing."
  - Move GSO compatibility code into linux/compat/gso.*
  - Set skb->protocol on mpls_push and mpls_pop in the presence
of an offloaded VLAN.

v2.31
* As suggested by Jesse Gross
  - There is no need to make mac_header_end inline as it is not in a header file
  - Remove dubious if (*skb_ethertype == ethertype) optimisation from
set_ethertype
  - Only set skb->protocol in push_mpls() or pop_mpls() for non-VLAN packets
  - Use MAX_ETH_TYPES instead of SAMPLE_ACTION_DEPTH for array size
of types in struct eth_types. This corrects a typo/thinko.
  - Correct eth type tracking logic such that start isn't advanced
when entering a sample action, ensuring that all possibly types
are checked when verifying nested actions.
* Define HAVE_INNER_PROTOCOL based on kernel version.
  inner_protocol has been merged into net-next and should appear in
  v3.11 so there is no longer a need for a acinclude.m4 test to check for it.
* Add MPLS GSO compatibility code.
  This is for use on kernels that do not have MPLS GSO support.
  Thanks to Joe Stringer for his work on this.

v2.30
* As suggested by Jesse Gross
  - Use skb_cow_head in push_mpls to ensure there is sufficient headroom for
skb_push
  - Call make_writable with skb->mac_len 

[ovs-dev] [PATCH v2.39 0/7] MPLS actions and matches

[Simon Horman]

Hi,

This series implements MPLS actions and matches based on work by
Ravi K, Leo Alterman, Yamahata-san and Joe Stringer.

This series provides two changes

* Provide user-space support for the VLAN/MPLS tag insertion order
  up to and including OpenFlow 1.2, and the different ordering
  specified from OpenFlow 1.3. In a nutshell the datapath always
  uses the OpenFlow 1.3 ordering, which is to always insert tags
  immediately after the L2 header, regardless of the presence of other
  tags. And ovs-vswtichd provides compatibility for the behaviour up
  to OpenFlow 1.2, which is that MPLS tags should follow VLAN tags
  if present.

* Adding basic MPLS action and match support to the kernel datapath


Differences between v2.39 and v2.38:

* Rebase for removal of vlan, checksum and skb->mark compat code
  - This includes adding adding a new patch,
"[PATCH v2.39 6/7] datapath: Break out deacceleration portion of
vlan_push" to allow re-use of some existing code.


Differences between v2.38 and v2.37:

* Rebase for SCTP support
* Refactor validate_tp_port() to iterate over eth_types rather
  than open-coding the loop. With the addition of SCTP this logic
  is now used three times.


Differences between v2.37 and v2.36:

* Rebase


Differences between v2.36 and v2.35:

* Rebase

* Do not add set_ethertype() to datapath/actions.c.
  As this patch has evolved this function had devolved into
  to sets of functionality wrapped into a single function with
  only one line of common code. Refactor things to simply
  open-code setting the ether type in the two locations where
  set_ethertype() was previously used. The aim here is to improve
  readability.

* Update setting skb->ethertype after mpls push and pop.
  - In the case of push_mpls it should be set unconditionally
as in v2.35 the behaviour of this function to always push
an MPLS LSE before any VLAN tags.
  - In the case of mpls_pop eth_p_mpls(skb->protocol) is a better
test than skb->protocol != htons(ETH_P_8021Q) as it will give the
correct behaviour in the presence of other VLAN ethernet types,
for example 0x88a8 which is used by 802.1ad. Moreover, it seems
correct to update the ethernet type if it was previously set
according to the top-most MPLS LSE.

* Deaccelerate VLANs when pushing MPLS tags the
  - Since v2.35 MPLS push will insert an MPLS LSE before any VLAN tags.
This means that if an accelerated tag is present it should be
deaccelerated to ensure it ends up in the correct position.

* Update skb->mac_len in push_mpls() so that it will be correct
  when used by a subsequent call to pop_mpls().

  As things stand I do not believe this is strictly necessary as
  ovs-vswitchd will not send a pop MPLS action after a push MPLS action.
  However, I have added this in order to code more defensively as I believe
  that if such a sequence did occur it would be rather unobvious why
  it didn't work.

* Do not add skb_cow_head() call in push_mpls().
  It is unnecessary as there is a make_writable() call.
  This change was also made in v2.30 but some how the
  code regressed between then and v2.35.


Differences between v2.35 and v2.34:

* Add support for the tag ordering specified up until OpenFlow 1.2 and
  the ordering specified from OpenFlow 1.3.

* Correct error in datapath patch's handling of GSO in the presence
  of MPLS and absence of VLANs.


Patch overview:

* The first 5 patches of this series are new,
  adding support for different tag ordering.
* The last patch is a revised version of the patch to add MPLS support
  to the datapath. It has been updated to use OpenFlow 1.3 tag ordering
  and resolve a GSO handling bug: both mentioned above. Its change log
  includes a history of changes.


To aid review this series is available in git at:

git://github.com/horms/openvswitch.git devel/mpls-v2.38


Patch list and overall diffstat:

Joe Stringer (5):
  odp: Only pass vlan_tci to commit_vlan_action()
  odp: Allow VLAN actions after MPLS actions
  ofp-actions: Add OFPUTIL_OFPAT13_PUSH_MPLS
  ofp-actions: Add separate OpenFlow 1.3 action parser
  lib: Push MPLS tags in the OpenFlow 1.3 ordering

Simon Horman (1):
  datapath: Add basic MPLS support to kernel

 datapath/Modules.mk |1 +
 datapath/actions.c  |  121 +-
 datapath/datapath.c |  259 +++--
 datapath/datapath.h |9 +
 datapath/flow.c |   58 ++-
 datapath/flow.h |   17 +-
 datapath/linux/compat/gso.c |   50 ++-
 datapath/linux/compat/gso.h |   39 ++
 datapath/linux/compat/include/linux/netdevice.h |   12 -
 datapath/linux/compat/netdevice.c   |   28 --
 datapath/mpls.h |   15 +
 datapath/vport-lisp.c   |1 +
 datapath/vport-netdev.c  

[ovs-dev] [PATCH v2.39 6/7] datapath: Break out deacceleration portion of vlan_push

[Simon Horman]

Break out deacceleration portion of vlan_push into vlan_put
so that it may be re-used by mpls_push.

For both vlan_push and mpls_push if there is an accelerated VLAN tag
present then it should be deaccelerated, adding it to the data of
the skb, before the new tag is added.

Signed-off-by: Simon Horman 

---
v2.39
* First post
---
 datapath/actions.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 30ea1d2..6741d81 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -105,22 +105,29 @@ static int pop_vlan(struct sk_buff *skb)
return 0;
 }
 
-static int push_vlan(struct sk_buff *skb, const struct ovs_action_push_vlan 
*vlan)
+/* push down current VLAN tag */
+static struct sk_buff *put_vlan(struct sk_buff *skb)
 {
-   if (unlikely(vlan_tx_tag_present(skb))) {
-   u16 current_tag;
+   u16 current_tag = vlan_tx_tag_get(skb);
 
-   /* push down current VLAN tag */
-   current_tag = vlan_tx_tag_get(skb);
+   if (!__vlan_put_tag(skb, skb->vlan_proto, current_tag))
+   return ERR_PTR(-ENOMEM);
 
-   if (!__vlan_put_tag(skb, skb->vlan_proto, current_tag))
-   return -ENOMEM;
+   if (skb->ip_summed == CHECKSUM_COMPLETE)
+   skb->csum = csum_add(skb->csum, csum_partial(skb->data
+   + (2 * ETH_ALEN), VLAN_HLEN, 0));
 
-   if (skb->ip_summed == CHECKSUM_COMPLETE)
-   skb->csum = csum_add(skb->csum, csum_partial(skb->data
-   + (2 * ETH_ALEN), VLAN_HLEN, 0));
+   return skb;
+}
 
+static int push_vlan(struct sk_buff *skb, const struct ovs_action_push_vlan 
*vlan)
+{
+   if (unlikely(vlan_tx_tag_present(skb))) {
+   skb = put_vlan(skb);
+   if (IS_ERR(skb))
+   return PTR_ERR(skb);
}
+
__vlan_hwaccel_put_tag(skb, vlan->vlan_tpid, ntohs(vlan->vlan_tci) & 
~VLAN_TAG_PRESENT);
return 0;
 }
-- 
1.8.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2.39 5/7] lib: Push MPLS tags in the OpenFlow 1.3 ordering

[Simon Horman]

From: Joe Stringer 

This patch modifies the push_mpls behaviour to follow the OpenFlow 1.3
specification in the presence of VLAN tagged packets. From the spec:

"Newly pushed tags should always be inserted as the outermost tag in the
outermost valid location for that tag. When a new VLAN tag is pushed, it
should be the outermost tag inserted, immediately after the Ethernet
header and before other tags. Likewise, when a new MPLS tag is pushed,
it should be the outermost tag inserted, immediately after the Ethernet
header and before other tags."

When the push_mpls action was inserted using OpenFlow 1.2, we implement
the previous behaviour by inserting VLAN actions around the MPLS action
in the odp translation; Pop VLAN tags before committing MPLS actions,
and push the expected VLAN tag afterwards. The trigger condition for
this is based on the ofpact->compat field.

Signed-off-by: Joe Stringer 
Signed-off-by: Simon Horman 

---

v2.36 - v2.39
* No change

v2.35
* First post
---
 lib/flow.c   |   2 +-
 lib/packets.c|  10 +-
 lib/packets.h|   2 +-
 ofproto/ofproto-dpif-xlate.c |  10 +-
 tests/ofproto-dpif.at| 237 +++
 5 files changed, 253 insertions(+), 8 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index 9ab1961..64a0e0c 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1063,7 +1063,7 @@ flow_compose(struct ofpbuf *b, const struct flow *flow)
 }
 
 if (eth_type_mpls(flow->dl_type)) {
-b->l2_5 = b->l3;
+b->l2_5 = (char*)b->l2 + ETH_HEADER_LEN;
 push_mpls(b, flow->dl_type, flow->mpls_lse);
 }
 }
diff --git a/lib/packets.c b/lib/packets.c
index d15c402..6637575 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -246,11 +246,11 @@ eth_mpls_depth(const struct ofpbuf *packet)
 
 /* Set ethertype of the packet. */
 void
-set_ethertype(struct ofpbuf *packet, ovs_be16 eth_type)
+set_ethertype(struct ofpbuf *packet, ovs_be16 eth_type, bool inner)
 {
 struct eth_header *eh = packet->data;
 
-if (eh->eth_type == htons(ETH_TYPE_VLAN)) {
+if (inner && eh->eth_type == htons(ETH_TYPE_VLAN)) {
 ovs_be16 *p;
 p = ALIGNED_CAST(ovs_be16 *,
 (char *)(packet->l2_5 ? packet->l2_5 : packet->l3) - 2);
@@ -358,8 +358,8 @@ push_mpls(struct ofpbuf *packet, ovs_be16 ethtype, ovs_be32 
lse)
 
 if (!is_mpls(packet)) {
 /* Set ethtype and MPLS label stack entry. */
-set_ethertype(packet, ethtype);
-packet->l2_5 = packet->l3;
+set_ethertype(packet, ethtype, false);
+packet->l2_5 = (char*)packet->l2 + ETH_HEADER_LEN;
 }
 
 /* Push new MPLS shim header onto packet. */
@@ -380,7 +380,7 @@ pop_mpls(struct ofpbuf *packet, ovs_be16 ethtype)
 size_t len;
 mh = packet->l2_5;
 len = (char*)packet->l2_5 - (char*)packet->l2;
-set_ethertype(packet, ethtype);
+set_ethertype(packet, ethtype, true);
 if (mh->mpls_lse & htonl(MPLS_BOS_MASK)) {
 packet->l2_5 = NULL;
 } else {
diff --git a/lib/packets.h b/lib/packets.h
index b776914..555a8f3 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -145,7 +145,7 @@ void eth_pop_vlan(struct ofpbuf *);
 
 uint16_t eth_mpls_depth(const struct ofpbuf *packet);
 
-void set_ethertype(struct ofpbuf *packet, ovs_be16 eth_type);
+void set_ethertype(struct ofpbuf *packet, ovs_be16 eth_type, bool inner);
 
 const char *eth_from_hex(const char *hex, struct ofpbuf **packetp);
 void eth_format_masked(const uint8_t eth[ETH_ADDR_LEN],
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2fa7785..c690db1 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2175,6 +2175,12 @@ may_receive(const struct xport *xport, struct xlate_ctx 
*ctx)
 return true;
 }
 
+static bool
+mpls_compat_behaviour(enum ofputil_action_code compat)
+{
+return (compat != OFPUTIL_OFPAT13_PUSH_MPLS);
+}
+
 static void
 vlan_tci_restore(struct xlate_in *xin, ovs_be16 *tci_ptr, ovs_be16 orig_tci)
 {
@@ -2360,7 +2366,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t 
ofpacts_len,
 
 /* Save and pop any existing VLAN tags if running in OF1.2 mode. */
 ctx->xin->vlan_tci = *vlan_tci;
-flow->vlan_tci = htons(0);
+if (mpls_compat_behaviour(a->compat)) {
+flow->vlan_tci = htons(0);
+}
 vlan_tci = &ctx->xin->vlan_tci;
 break;
 
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 605feeb..0a40e69 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -1056,6 +1056,243 @@ NXST_FLOW reply:
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - OF1.3+ VLAN+MPLS handling])
+OVS_VSWITCHD_START([dnl
+   add-port br0 p1 -- set Interface p1 type=dummy
+])
+ON_EXIT([kill `cat ovs-ofctl.pid`])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+AT_DATA([flows.txt], [dnl
+cookie=0xa dl_src=40:44:44:44:55:4

Re: [ovs-dev] [PATCH v2.39 0/7] MPLS actions and matches

[Simon Horman]

On Mon, Sep 09, 2013 at 04:20:00PM +0900, Simon Horman wrote:
> Hi,
> 
> This series implements MPLS actions and matches based on work by
> Ravi K, Leo Alterman, Yamahata-san and Joe Stringer.
> 
> This series provides two changes
> 
> * Provide user-space support for the VLAN/MPLS tag insertion order
>   up to and including OpenFlow 1.2, and the different ordering
>   specified from OpenFlow 1.3. In a nutshell the datapath always
>   uses the OpenFlow 1.3 ordering, which is to always insert tags
>   immediately after the L2 header, regardless of the presence of other
>   tags. And ovs-vswtichd provides compatibility for the behaviour up
>   to OpenFlow 1.2, which is that MPLS tags should follow VLAN tags
>   if present.
> 
> * Adding basic MPLS action and match support to the kernel datapath
> 
> 
> Differences between v2.39 and v2.38:
> 
> * Rebase for removal of vlan, checksum and skb->mark compat code
>   - This includes adding adding a new patch,
> "[PATCH v2.39 6/7] datapath: Break out deacceleration portion of
> vlan_push" to allow re-use of some existing code.
> 
> 
> Differences between v2.38 and v2.37:
> 
> * Rebase for SCTP support
> * Refactor validate_tp_port() to iterate over eth_types rather
>   than open-coding the loop. With the addition of SCTP this logic
>   is now used three times.
> 
> 
> Differences between v2.37 and v2.36:
> 
> * Rebase
> 
> 
> Differences between v2.36 and v2.35:
> 
> * Rebase
> 
> * Do not add set_ethertype() to datapath/actions.c.
>   As this patch has evolved this function had devolved into
>   to sets of functionality wrapped into a single function with
>   only one line of common code. Refactor things to simply
>   open-code setting the ether type in the two locations where
>   set_ethertype() was previously used. The aim here is to improve
>   readability.
> 
> * Update setting skb->ethertype after mpls push and pop.
>   - In the case of push_mpls it should be set unconditionally
> as in v2.35 the behaviour of this function to always push
> an MPLS LSE before any VLAN tags.
>   - In the case of mpls_pop eth_p_mpls(skb->protocol) is a better
> test than skb->protocol != htons(ETH_P_8021Q) as it will give the
> correct behaviour in the presence of other VLAN ethernet types,
> for example 0x88a8 which is used by 802.1ad. Moreover, it seems
> correct to update the ethernet type if it was previously set
> according to the top-most MPLS LSE.
> 
> * Deaccelerate VLANs when pushing MPLS tags the
>   - Since v2.35 MPLS push will insert an MPLS LSE before any VLAN tags.
> This means that if an accelerated tag is present it should be
> deaccelerated to ensure it ends up in the correct position.
> 
> * Update skb->mac_len in push_mpls() so that it will be correct
>   when used by a subsequent call to pop_mpls().
> 
>   As things stand I do not believe this is strictly necessary as
>   ovs-vswitchd will not send a pop MPLS action after a push MPLS action.
>   However, I have added this in order to code more defensively as I believe
>   that if such a sequence did occur it would be rather unobvious why
>   it didn't work.
> 
> * Do not add skb_cow_head() call in push_mpls().
>   It is unnecessary as there is a make_writable() call.
>   This change was also made in v2.30 but some how the
>   code regressed between then and v2.35.
> 
> 
> Differences between v2.35 and v2.34:
> 
> * Add support for the tag ordering specified up until OpenFlow 1.2 and
>   the ordering specified from OpenFlow 1.3.
> 
> * Correct error in datapath patch's handling of GSO in the presence
>   of MPLS and absence of VLANs.

I forgot to update the trailing portion of this cover letter.
It should read as follows:

Patch overview:

* The first 5 patches add support for different tag ordering
  to user-space.
* The 6th patch breaks out kernel datapath some code to allow
  it to be re-used by the last patch.
* The last patch is a revised version of the patch to add MPLS support
  to the kernel datapath.

To aid review this series is available in git at:

git://github.com/horms/openvswitch.git devel/mpls-v2.39


Patch list and overall diffstat:

Joe Stringer (5):
  odp: Only pass vlan_tci to commit_vlan_action()
  odp: Allow VLAN actions after MPLS actions
  ofp-actions: Add OFPUTIL_OFPAT13_PUSH_MPLS
  ofp-actions: Add separate OpenFlow 1.3 action parser
  lib: Push MPLS tags in the OpenFlow 1.3 ordering

Simon Horman (2):
  datapath: Break out deacceleration portion of vlan_push
  datapath: Add basic MPLS support to kernel

 datapath/Modules.mk |   1 +
 datapath/actions.c  | 154 +++-
 datapath/datapath.c | 259 --
 datapath/datapath.h |   9 +
 datapath/flow.c |  58 ++-
 datapath/flow.h |  17 +-
 datapath/linux/compat/gso.c |  50 ++-
 datapath/linux/c

Re: [ovs-dev] [RFC PATCH] ofproto: update flow_stats flags on flow_stats_request

[Daniel Baluta]

On Mon, Sep 9, 2013 at 1:58 AM, Ben Pfaff  wrote:
> On Sun, Sep 08, 2013 at 02:21:25AM +0300, Daniel Baluta wrote:
>> This is a first step in implementing 'on demand flow counters'.
>> We save flow_mod flags into newly created rule when a new flow
>> is added, and echo them back in the flow stats request.
>>
>> Signed-off-by: Daniel Baluta 
>
> It's a reasonable approach.  Two comments:
>
> * Some of the flags apply to the request, not to the flow.
>   Specifically, I don't think that it makes sense to save the
>   OFPUTIL_FF_CHECK_OVERLAP or OFPUTIL_FF_RESET_COUNTS flags with
>   the flow.

Good point. I will only set only OFPUTIL_FF_NO_PKT_COUNTS and
OFPUTIL_FF_NO_BYT_COUNTS flags.

>
> * If we save the flags this way, then we don't need the
>   send_flow_removed member anymore because it just reflects the
>   status of the OFPUTIL_FF_SEND_FLOW_REM flag.

I will send v2 asap. Thanks for reviewing this.

Daniel.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] lisp: Reset vlan tag on send.

[Pravin B Shelar]

Lisp needs to discards all l2 packet headers but if vlan tx
is hw-acceleraed vlan tag is stored in skb struct. Following
patch resets it.

Signed-off-by: Pravin B Shelar 
---
 datapath/vport-lisp.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/datapath/vport-lisp.c b/datapath/vport-lisp.c
index c7da276..e4e603f 100644
--- a/datapath/vport-lisp.c
+++ b/datapath/vport-lisp.c
@@ -437,8 +437,11 @@ static int lisp_send(struct vport *vport, struct sk_buff 
*skb)
goto err_free_rt;
}
 
+   /* Reset l2 headers. */
skb_pull(skb, network_offset);
skb_reset_mac_header(skb);
+   vlan_set_tci(skb, 0);
+
skb_reset_inner_headers(skb);
 
__skb_push(skb, LISP_HLEN);
-- 
1.7.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 2/2] datapath lisp: Use ovs offload compat functionality.

[Pravin Shelar]

On Fri, Sep 6, 2013 at 12:02 PM, Jesse Gross  wrote:
> On Fri, Sep 6, 2013 at 11:16 AM, Pravin B Shelar  wrote:
>> OVS already has compat functions to handle GSO packets.
>> Following patch get rid of GSO packet handling in lisp
>> and use ovs iptunnel_xmit() function for same.
>>
>> CC: Lori Jakab 
>> Signed-off-by: Pravin B Shelar 
>
> Acked-by: Jesse Gross 
>
I pushed both patches.

> I think I see a bug which not new but probably has a slightly
> different form now: If we have an accelerated vlan tag it won't get
> stripped off and will turn into an outer tag. Before, I think we would
> have inserted inside the LISP header which is not right either.
sent a fix for this issue on master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] datapath: Move flow table rehashing to flow install.

[Pravin Shelar]

On Fri, Sep 6, 2013 at 4:42 PM, Jesse Gross  wrote:
> On Fri, Sep 6, 2013 at 2:54 PM, Pravin B Shelar  wrote:
>> Rehashing in ovs-workqueue can cause ovs-mutex lock contentions
>> in case of heavy flow setups where both needs ovs-mutex.  So by
>> moving rehashing to flow-setup we can eliminate contention.
>> This also simplify ovs locking and reduces dependence on
>> workqueue.
>>
>> Signed-off-by: Pravin B Shelar 
>
> Acked-by: Jesse Gross 

Thanks.
I pushed patch to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] lib/timeval: don't forget to initialize a rwlock

[Ben Pfaff]

On Mon, Sep 09, 2013 at 03:56:23PM +0900, YAMAMOTO Takashi wrote:
> Commit 31ef9f5178 (timeval: Remove CACHE_TIME scheme.) removed
> initialization of a rwlock which is still used for some operations.
> This restores it.

Applied, thank you!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2] Don't set subprogram name to empty

[gyang]

From: Guolin Yang 

In monitor_daemon(), it set subprogram_name to "" which causes
system crash in some platform. This change set subprogram name
to program name.

Signed-off-by: Guolin Yang 
---
Based on Ben's comments, only set pthread's subprogram name to non empty
---
 lib/util.c |7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/util.c b/lib/util.c
index 3cada4a..b800455 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -405,13 +405,14 @@ get_subprogram_name(void)
 void
 set_subprogram_name(const char *name)
 {
+const char *pname = name[0] ? name : program_name;
 free(subprogram_name_set(xstrdup(name)));
 #if HAVE_GLIBC_PTHREAD_SETNAME_NP
-pthread_setname_np(pthread_self(), name);
+pthread_setname_np(pthread_self(), pname);
 #elif HAVE_NETBSD_PTHREAD_SETNAME_NP
-pthread_setname_np(pthread_self(), "%s", name);
+pthread_setname_np(pthread_self(), "%s", pname);
 #elif HAVE_PTHREAD_SET_NAME_NP
-pthread_set_name_np(pthread_self(), name);
+pthread_set_name_np(pthread_self(), pname);
 #endif
 }
 
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] Trigger bridge reconfigure from ofproto layer

[gyang]

From: Guolin Yang 

There are two models in OVS in configure datapath port:
1. Datapath port created before user space bridge is configured.
   In this model, datapath can be deleted or added independent of
   user-space.
2. Datapath port created when ovsdb is requested to add the relevant
   port.

Traditionally OVS supports the second model which is used in Linux
platform.

With more platform support, OVS requires to support first model. In
this case, datapath port change detected by ofproto layer need to trigger
brdige to reconfigure so that bridge and ofproto layers can be
synchronized.

This change introduced a API for ofproto to request reconfiguration and
for bridge to detect the request.

Signed-off-by: Guolin Yang 
---
 ofproto/ofproto-dpif.c |   14 ++
 vswitchd/bridge.c  |7 ++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b52d4ee..0036081 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -527,6 +527,9 @@ static bool clogged;
  * may be disabled with the "ovs-appctl dpif/disable-megaflows" command. */
 static bool enable_megaflows = true;
 
+/* ofproto require bridge reconfigure */
+static bool need_reconfigure = false;
+
 /* All existing ofproto_dpif instances, indexed by ->up.name. */
 static struct hmap all_ofproto_dpifs = HMAP_INITIALIZER(&all_ofproto_dpifs);
 
@@ -899,6 +902,17 @@ type_run(const char *type)
 return 0;
 }
 
+void
+ofproto_set_need_reconfigure(bool need)
+{
+need_reconfigure = need;
+}
+
+bool
+ofproto_need_reconfigure(void) {
+return need_reconfigure;
+}
+
 /* Check for and handle port changes in 'backer''s dpif. */
 static void
 process_dpif_port_changes(struct dpif_backer *backer)
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 5e54e0b..878ab3a 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2405,7 +2405,9 @@ bridge_run(void)
 }
 }
 
-if (ovsdb_idl_get_seqno(idl) != idl_seqno || vlan_splinters_changed) {
+if (ovsdb_idl_get_seqno(idl) != idl_seqno || vlan_splinters_changed ||
+ofproto_need_reconfigure()) {
+ofproto_set_need_reconfigure(false);
 idl_seqno = ovsdb_idl_get_seqno(idl);
 if (cfg) {
 reconf_txn = ovsdb_idl_txn_create(idl);
@@ -2514,6 +2516,9 @@ bridge_wait(void)
 poll_immediate_wake();
 }
 
+if (ofproto_need_reconfigure()) {
+poll_immediate_wake();
+}
 sset_init(&types);
 ofproto_enumerate_types(&types);
 SSET_FOR_EACH (type, &types) {
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] lisp: Reset vlan tag on send.

[Jesse Gross]

On Mon, Sep 9, 2013 at 9:21 AM, Pravin B Shelar  wrote:
> Lisp needs to discards all l2 packet headers but if vlan tx
> is hw-acceleraed vlan tag is stored in skb struct. Following
> patch resets it.
>
> Signed-off-by: Pravin B Shelar 

Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath: Remove compat workqueue.

[Jesse Gross]

On Mon, Sep 9, 2013 at 1:53 PM, Pravin B Shelar  wrote:
> OVS has its own workq implementation for coupe of reasons. first
> was to avoid system freeze due to ovs-flow rehash softlockup.
> We have moved out rehash from workq, So this problem does not exist.
> second was related bugs in kernel workq implementation in pre-2.6.32
> kernel. But we have dropped support for older kernel.
> So there is no reason to keep ovs-workq around. Following patch
> removes it.
>
> Signed-off-by: Pravin B Shelar 

Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 03/11] ofproto: Factor code out of collect_rules_{loose, strict} into new helper.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto.c |  125 +++--
 1 file changed, 55 insertions(+), 70 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index e7ee60e..81b6c43 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2935,6 +2935,26 @@ next_matching_table(const struct ofproto *ofproto,
  (TABLE) != NULL; \
  (TABLE) = next_matching_table(OFPROTO, TABLE, TABLE_ID))
 
+static enum ofperr
+collect_rule(struct rule *rule, uint8_t table_id,
+ ovs_be64 cookie, ovs_be64 cookie_mask,
+ ofp_port_t out_port, uint32_t out_group, struct list *rules)
+{
+if (ofproto_rule_is_hidden(rule)) {
+return 0;
+} else if (rule->pending) {
+return OFPROTO_POSTPONE;
+} else {
+if ((table_id == rule->table_id || table_id == 0xff)
+&& ofproto_rule_has_out_port(rule, out_port)
+&& ofproto_rule_has_out_group(rule, out_group)
+&& !((rule->flow_cookie ^ cookie) & cookie_mask)) {
+list_push_back(rules, &rule->ofproto_node);
+}
+return 0;
+}
+}
+
 /* Searches 'ofproto' for rules in table 'table_id' (or in all tables, if
  * 'table_id' is 0xff) that match 'match' in the "loose" way required for
  * OpenFlow OFPFC_MODIFY and OFPFC_DELETE requests and puts them on list
@@ -2970,50 +2990,32 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t 
table_id,
 
 HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node, hash_cookie(cookie),
&ofproto->cookies) {
-if (table_id != rule->table_id && table_id != 0xff) {
-continue;
-}
-if (ofproto_rule_is_hidden(rule)) {
-continue;
-}
 if (cls_rule_is_loose_match(&rule->cr, &cr.match)) {
-if (rule->pending) {
-error = OFPROTO_POSTPONE;
-goto exit;
-}
-if (rule->flow_cookie == cookie /* Hash collisions possible. */
-&& ofproto_rule_has_out_port(rule, out_port)
-&& ofproto_rule_has_out_group(rule, out_group)) {
-list_push_back(rules, &rule->ofproto_node);
+error = collect_rule(rule, table_id, cookie, cookie_mask,
+ out_port, out_group, rules);
+if (error) {
+break;
 }
 }
 }
-goto exit;
-}
-
-FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) {
-struct cls_cursor cursor;
-struct rule *rule;
+} else {
+FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) {
+struct cls_cursor cursor;
+struct rule *rule;
 
-ovs_rwlock_rdlock(&table->cls.rwlock);
-cls_cursor_init(&cursor, &table->cls, &cr);
-CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
-if (rule->pending) {
-ovs_rwlock_unlock(&table->cls.rwlock);
-error = OFPROTO_POSTPONE;
-goto exit;
-}
-if (!ofproto_rule_is_hidden(rule)
-&& ofproto_rule_has_out_port(rule, out_port)
-&& ofproto_rule_has_out_group(rule, out_group)
-&& !((rule->flow_cookie ^ cookie) & cookie_mask)) {
-list_push_back(rules, &rule->ofproto_node);
+ovs_rwlock_rdlock(&table->cls.rwlock);
+cls_cursor_init(&cursor, &table->cls, &cr);
+CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
+error = collect_rule(rule, table_id, cookie, cookie_mask,
+ out_port, out_group, rules);
+if (error) {
+break;
+}
 }
+ovs_rwlock_unlock(&table->cls.rwlock);
 }
-ovs_rwlock_unlock(&table->cls.rwlock);
 }
 
-exit:
 cls_rule_destroy(&cr);
 return error;
 }
@@ -3053,51 +3055,34 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t 
table_id,
 
 HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node, hash_cookie(cookie),
&ofproto->cookies) {
-if (table_id != rule->table_id && table_id != 0xff) {
-continue;
-}
-if (ofproto_rule_is_hidden(rule)) {
-continue;
-}
 if (cls_rule_equal(&rule->cr, &cr)) {
-if (rule->pending) {
-error = OFPROTO_POSTPONE;
-goto exit;
-}
-if (rule->flow_cookie == cookie /* Hash collisions possible. */
-&& ofproto_rule_has_out_port(rule, out_port)
-&& ofproto_rule_has_out_group(rule, out_group)) {
-list_push_back(rules, &rule->ofproto_node);
+error = coll

[ovs-dev] [PATCH 01/11] ofproto: Do not call ->rule_destruct() if ->rule_construct() failed.

[Ben Pfaff]

Found by inspection.

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto.c |   23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 4d33de7..03738ab 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -199,6 +199,7 @@ static int init_ports(struct ofproto *);
 static void reinit_ports(struct ofproto *);
 
 /* rule. */
+static void ofproto_rule_destroy(struct rule *);
 static void ofproto_rule_destroy__(struct rule *);
 static void ofproto_rule_send_removed(struct rule *, uint8_t reason);
 static bool rule_is_modifiable(const struct rule *);
@@ -2256,18 +2257,24 @@ update_mtu(struct ofproto *p, struct ofport *port)
 }
 
 static void
-ofproto_rule_destroy__(struct rule *rule)
+ofproto_rule_destroy(struct rule *rule)
 {
 if (rule) {
 rule->ofproto->ofproto_class->rule_destruct(rule);
-cls_rule_destroy(&rule->cr);
-free(rule->ofpacts);
-ovs_mutex_destroy(&rule->timeout_mutex);
-ovs_rwlock_destroy(&rule->rwlock);
-rule->ofproto->ofproto_class->rule_dealloc(rule);
+ofproto_rule_destroy__(rule);
 }
 }
 
+static void
+ofproto_rule_destroy__(struct rule *rule)
+{
+cls_rule_destroy(&rule->cr);
+free(rule->ofpacts);
+ovs_mutex_destroy(&rule->timeout_mutex);
+ovs_rwlock_destroy(&rule->rwlock);
+rule->ofproto->ofproto_class->rule_dealloc(rule);
+}
+
 /* This function allows an ofproto implementation to destroy any rules that
  * remain when its ->destruct() function is called..  This function implements
  * steps 4.4 and 4.5 in the section titled "Rule Life Cycle" in
@@ -5508,13 +5515,13 @@ ofopgroup_complete(struct ofopgroup *group)
 } else {
 ovs_rwlock_wrlock(&rule->rwlock);
 oftable_remove_rule(rule);
-ofproto_rule_destroy__(rule);
+ofproto_rule_destroy(rule);
 }
 break;
 
 case OFOPERATION_DELETE:
 ovs_assert(!op->error);
-ofproto_rule_destroy__(rule);
+ofproto_rule_destroy(rule);
 op->rule = NULL;
 break;
 
-- 
1.7.10.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 00/11] refactor locking rules for rules

[Ben Pfaff]

This is a set of precursor patches for moving the "learn" action
into individual threads rather than doing them all in the main
thread, which in turn is necessary to fix a regression in the
promptness of learning.

Ben Pfaff (11):
  ofproto: Do not call ->rule_destruct() if ->rule_construct() failed.
  ofproto: Support out_group feature when matching on cookie.
  ofproto: Factor code out of collect_rules_{loose,strict} into new
helper.
  ofproto: Correct comments.
  ofproto: Avoid gratuitous memory allocation and free.
  ofproto-dpif: Remove vestigial "clogged" feature.
  ofproto: Merge ofproto_rule_delete() and ofproto_delete_rule().
  ofproto: Move function find_meter() into ofpacts as
ofpacts_get_meter().
  ofproto: Remove soon-to-be-invalid optimizations.
  ofproto: Break actions out of rule into new rule_actions structure.
  ofproto: Add a ref_count to "struct rule" to protect it from being
freed.

 lib/ofp-actions.c |   24 +++
 lib/ofp-actions.h |1 +
 ofproto/connmgr.c |4 +-
 ofproto/ofproto-dpif-upcall.c |2 +-
 ofproto/ofproto-dpif-xlate.c  |   32 ++--
 ofproto/ofproto-dpif.c|  126 -
 ofproto/ofproto-dpif.h|   16 +-
 ofproto/ofproto-provider.h|   33 +++-
 ofproto/ofproto.c |  420 +++--
 9 files changed, 357 insertions(+), 301 deletions(-)

-- 
1.7.10.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 04/11] ofproto: Correct comments.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 81b6c43..388463a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2276,7 +2276,7 @@ ofproto_rule_destroy__(struct rule *rule)
 }
 
 /* This function allows an ofproto implementation to destroy any rules that
- * remain when its ->destruct() function is called..  This function implements
+ * remain when its ->destruct() function is called.  This function implements
  * steps 4.4 and 4.5 in the section titled "Rule Life Cycle" in
  * ofproto-provider.h.
  *
@@ -3435,8 +3435,7 @@ evict_rule_from_table(struct ofproto *ofproto, struct 
oftable *table)
  * error code on failure, or OFPROTO_POSTPONE if the operation cannot be
  * initiated now but may be retried later.
  *
- * Upon successful return, takes ownership of 'fm->ofpacts'.  On failure,
- * ownership remains with the caller.
+ * The caller retains ownership of 'fm->ofpacts'.
  *
  * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id,
  * if any. */
-- 
1.7.10.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 06/11] ofproto-dpif: Remove vestigial "clogged" feature.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto-dpif.c |   64 ++--
 1 file changed, 2 insertions(+), 62 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b52d4ee..97735e2 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -484,9 +484,6 @@ struct ofproto_dpif {
 struct classifier facets; /* Contains 'struct facet's. */
 long long int consistency_rl;
 
-/* Support for debugging async flow mods. */
-struct list completions;
-
 struct netdev_stats stats; /* To account packets generated and consumed in
 * userspace. */
 
@@ -519,10 +516,6 @@ struct ofproto_dpif {
 size_t n_pins OVS_GUARDED;
 };
 
-/* Defer flow mod completion until "ovs-appctl ofproto/unclog"?  (Useful only
- * for debugging the asynchronous flow_mod implementation.) */
-static bool clogged;
-
 /* By default, flows in the datapath are wildcarded (megaflows).  They
  * may be disabled with the "ovs-appctl dpif/disable-megaflows" command. */
 static bool enable_megaflows = true;
@@ -1293,8 +1286,6 @@ construct(struct ofproto *ofproto_)
 classifier_init(&ofproto->facets);
 ofproto->consistency_rl = LLONG_MIN;
 
-list_init(&ofproto->completions);
-
 ovs_mutex_init(&ofproto->flow_mod_mutex);
 ovs_mutex_lock(&ofproto->flow_mod_mutex);
 list_init(&ofproto->flow_mods);
@@ -1424,18 +1415,6 @@ add_internal_flows(struct ofproto_dpif *ofproto)
 }
 
 static void
-complete_operations(struct ofproto_dpif *ofproto)
-{
-struct dpif_completion *c, *next;
-
-LIST_FOR_EACH_SAFE (c, next, list_node, &ofproto->completions) {
-ofoperation_complete(c->op, 0);
-list_remove(&c->list_node);
-free(c);
-}
-}
-
-static void
 destruct(struct ofproto *ofproto_)
 {
 struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
@@ -1461,7 +1440,6 @@ destruct(struct ofproto *ofproto_)
 flow_miss_batch_ofproto_destroyed(ofproto->backer->udpif, ofproto);
 
 hmap_remove(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node);
-complete_operations(ofproto);
 
 OFPROTO_FOR_EACH_TABLE (table, &ofproto->up) {
 struct cls_cursor cursor;
@@ -1473,7 +1451,6 @@ destruct(struct ofproto *ofproto_)
 }
 ovs_rwlock_unlock(&table->cls.rwlock);
 }
-complete_operations(ofproto);
 
 ovs_mutex_lock(&ofproto->flow_mod_mutex);
 LIST_FOR_EACH_SAFE (fm, next_fm, list_node, &ofproto->flow_mods) {
@@ -1577,10 +1554,6 @@ run(struct ofproto *ofproto_)
 struct ofbundle *bundle;
 int error;
 
-if (!clogged) {
-complete_operations(ofproto);
-}
-
 if (mbridge_need_revalidate(ofproto->mbridge)) {
 ofproto->backer->need_revalidate = REV_RECONFIGURE;
 ovs_rwlock_wrlock(&ofproto->ml->rwlock);
@@ -1658,10 +1631,6 @@ wait(struct ofproto *ofproto_)
 struct ofport_dpif *ofport;
 struct ofbundle *bundle;
 
-if (!clogged && !list_is_empty(&ofproto->completions)) {
-poll_immediate_wake();
-}
-
 if (ofproto_get_flow_restore_wait()) {
 return;
 }
@@ -3978,10 +3947,7 @@ rule_expire(struct rule_dpif *rule)
 long long int now;
 uint8_t reason;
 
-if (rule->up.pending) {
-/* We'll have to expire it later. */
-return;
-}
+ovs_assert(!rule->up.pending);
 
 ovs_mutex_lock(&rule->up.timeout_mutex);
 hard_timeout = rule->up.hard_timeout;
@@ -4908,13 +4874,7 @@ complete_operation(struct rule_dpif *rule)
 struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
 
 ofproto->backer->need_revalidate = REV_FLOW_TABLE;
-if (clogged) {
-struct dpif_completion *c = xmalloc(sizeof *c);
-c->op = rule->up.pending;
-list_push_back(&ofproto->completions, &c->list_node);
-} else {
-ofoperation_complete(rule->up.pending, 0);
-}
+ofoperation_complete(rule->up.pending, 0);
 }
 
 static struct rule_dpif *rule_dpif_cast(const struct rule *rule)
@@ -5622,22 +5582,6 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct 
flow *flow,
 rule_dpif_release(rule);
 }
 
-static void
-ofproto_dpif_clog(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
-  const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
-{
-clogged = true;
-unixctl_command_reply(conn, NULL);
-}
-
-static void
-ofproto_dpif_unclog(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
-const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
-{
-clogged = false;
-unixctl_command_reply(conn, NULL);
-}
-
 /* Runs a self-check of flow translations in 'ofproto'.  Appends a message to
  * 'reply' describing the results. */
 static void
@@ -6054,10 +5998,6 @@ ofproto_dpif_unixctl_init(void)
  ofproto_unixctl_fdb_flush, NULL);
 unixctl_command_register("fdb/show", "bridge", 1, 1,
  ofproto_unixctl_fdb_show, NULL);
-

[ovs-dev] [PATCH 05/11] ofproto: Avoid gratuitous memory allocation and free.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 388463a..b027873 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1717,10 +1717,9 @@ ofproto_add_flow(struct ofproto *ofproto, const struct 
match *match,
 fm.match = *match;
 fm.priority = priority;
 fm.buffer_id = UINT32_MAX;
-fm.ofpacts = xmemdup(ofpacts, ofpacts_len);
+fm.ofpacts = ofpacts;
 fm.ofpacts_len = ofpacts_len;
 add_flow(ofproto, NULL, &fm, NULL);
-free(fm.ofpacts);
 }
 }
 
-- 
1.7.10.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 08/11] ofproto: Move function find_meter() into ofpacts as ofpacts_get_meter().

[Ben Pfaff]

ofproto is too big anyway so we might as well move out code that can
reasonably live elsewhere.

Signed-off-by: Ben Pfaff 
---
 lib/ofp-actions.c |   24 
 lib/ofp-actions.h |1 +
 ofproto/ofproto.c |   33 +
 3 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 77aa69c..54df17f 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -2117,6 +2117,30 @@ ofpacts_equal(const struct ofpact *a, size_t a_len,
 {
 return a_len == b_len && !memcmp(a, b, a_len);
 }
+
+/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
+ * 'ofpacts'.  If found, returns its meter ID; if not, returns 0.
+ *
+ * This function relies on the order of 'ofpacts' being correct (as checked by
+ * ofpacts_verify()). */
+uint32_t
+ofpacts_get_meter(const struct ofpact ofpacts[], size_t ofpacts_len)
+{
+const struct ofpact *a;
+
+OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+enum ovs_instruction_type inst;
+
+inst = ovs_instruction_type_from_ofpact_type(a->type);
+if (a->type == OFPACT_METER) {
+return ofpact_get_METER(a)->meter_id;
+} else if (inst > OVSINST_OFPIT13_METER) {
+break;
+}
+}
+
+return 0;
+}
 
 /* Formatting ofpacts. */
 
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index a3fb60f..0876ed7 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -530,6 +530,7 @@ bool ofpacts_output_to_group(const struct ofpact[], size_t 
ofpacts_len,
  uint32_t group_id);
 bool ofpacts_equal(const struct ofpact a[], size_t a_len,
const struct ofpact b[], size_t b_len);
+uint32_t ofpacts_get_meter(const struct ofpact[], size_t ofpacts_len);
 
 /* Formatting ofpacts.
  *
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index b365866..410c2e5 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1727,7 +1727,7 @@ ofproto_add_flow(struct ofproto *ofproto, const struct 
match *match,
 fm.match = *match;
 fm.priority = priority;
 fm.buffer_id = UINT32_MAX;
-fm.ofpacts = ofpacts;
+fm.ofpacts = CONST_CAST(struct ofpact *, ofpacts);
 fm.ofpacts_len = ofpacts_len;
 add_flow(ofproto, NULL, &fm, NULL);
 }
@@ -2489,30 +2489,6 @@ reject_slave_controller(struct ofconn *ofconn)
 }
 }
 
-/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
- * 'ofpacts'.  If found, returns its meter ID; if not, returns 0.
- *
- * This function relies on the order of 'ofpacts' being correct (as checked by
- * ofpacts_verify()). */
-static uint32_t
-find_meter(const struct ofpact ofpacts[], size_t ofpacts_len)
-{
-const struct ofpact *a;
-
-OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
-enum ovs_instruction_type inst;
-
-inst = ovs_instruction_type_from_ofpact_type(a->type);
-if (a->type == OFPACT_METER) {
-return ofpact_get_METER(a)->meter_id;
-} else if (inst > OVSINST_OFPIT13_METER) {
-break;
-}
-}
-
-return 0;
-}
-
 /* Checks that the 'ofpacts_len' bytes of actions in 'ofpacts' are appropriate
  * for a packet with the prerequisites satisfied by 'flow' in table 'table_id'.
  * 'flow' may be temporarily modified, but is restored at return.
@@ -2531,7 +2507,7 @@ ofproto_check_ofpacts(struct ofproto *ofproto,
 return error;
 }
 
-mid = find_meter(ofpacts, ofpacts_len);
+mid = ofpacts_get_meter(ofpacts, ofpacts_len);
 if (mid && ofproto_get_provider_meter_id(ofproto, mid) == UINT32_MAX) {
 return OFPERR_OFPMMFC_INVALID_METER;
 }
@@ -3557,7 +3533,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
 rule->send_flow_removed = (fm->flags & OFPUTIL_FF_SEND_FLOW_REM) != 0;
 rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
 rule->ofpacts_len = fm->ofpacts_len;
-rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len);
+rule->meter_id = ofpacts_get_meter(rule->ofpacts, rule->ofpacts_len);
 list_init(&rule->meter_list_node);
 rule->eviction_group = NULL;
 list_init(&rule->expirable);
@@ -3664,7 +3640,8 @@ modify_flows__(struct ofproto *ofproto, struct ofconn 
*ofconn,
 rule->ofpacts_len = fm->ofpacts_len;
 ovs_rwlock_unlock(&rule->rwlock);
 
-rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len);
+rule->meter_id = ofpacts_get_meter(rule->ofpacts,
+   rule->ofpacts_len);
 rule->ofproto->ofproto_class->rule_modify_actions(rule,
   reset_counters);
 } else {
-- 
1.7.10.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 09/11] ofproto: Remove soon-to-be-invalid optimizations.

[Ben Pfaff]

Until now, ofproto_add_flow() and ofproto_delete_flow() assumed that the
flow table could not change between its flow table check and its later
modification to the flow table.  This assumption will soon be untrue, so
remove it.

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto.c |   64 +++--
 1 file changed, 43 insertions(+), 21 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 410c2e5..458ff04 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1697,6 +1697,33 @@ ofproto_port_del(struct ofproto *ofproto, ofp_port_t 
ofp_port)
 return error;
 }
 
+static int
+simple_flow_mod(struct ofproto *ofproto,
+const struct match *match, unsigned int priority,
+const struct ofpact *ofpacts, size_t ofpacts_len,
+enum ofp_flow_mod_command command)
+{
+struct ofputil_flow_mod fm;
+
+memset(&fm, 0, sizeof fm);
+fm.match = *match;
+fm.priority = priority;
+fm.cookie = 0;
+fm.new_cookie = 0;
+fm.modify_cookie = false;
+fm.table_id = 0;
+fm.command = command;
+fm.idle_timeout = 0;
+fm.hard_timeout = 0;
+fm.buffer_id = UINT32_MAX;
+fm.out_port = OFPP_ANY;
+fm.out_group = OFPG_ANY;
+fm.flags = 0;
+fm.ofpacts = CONST_CAST(struct ofpact *, ofpacts);
+fm.ofpacts_len = ofpacts_len;
+return handle_flow_mod__(ofproto, NULL, &fm, NULL);
+}
+
 /* Adds a flow to OpenFlow flow table 0 in 'p' that matches 'cls_rule' and
  * performs the 'n_actions' actions in 'actions'.  The new flow will not
  * timeout.
@@ -1715,21 +1742,21 @@ ofproto_add_flow(struct ofproto *ofproto, const struct 
match *match,
 {
 const struct rule *rule;
 
+/* First do a cheap check whether the rule we're looking for already exists
+ * with the actions that we want.  If it does, then we're done. */
 ovs_rwlock_rdlock(&ofproto->tables[0].cls.rwlock);
 rule = rule_from_cls_rule(classifier_find_match_exactly(
   &ofproto->tables[0].cls, match, priority));
 ovs_rwlock_unlock(&ofproto->tables[0].cls.rwlock);
+
+/* If there's no such rule or the rule doesn't have the actions we want,
+ * fall back to a executing a full flow mod.  We can't optimize this at
+ * all because we didn't take enough locks above to ensure that the flow
+ * table didn't already change beneath us.  */
 if (!rule || !ofpacts_equal(rule->ofpacts, rule->ofpacts_len,
 ofpacts, ofpacts_len)) {
-struct ofputil_flow_mod fm;
-
-memset(&fm, 0, sizeof fm);
-fm.match = *match;
-fm.priority = priority;
-fm.buffer_id = UINT32_MAX;
-fm.ofpacts = CONST_CAST(struct ofpact *, ofpacts);
-fm.ofpacts_len = ofpacts_len;
-add_flow(ofproto, NULL, &fm, NULL);
+simple_flow_mod(ofproto, match, priority, ofpacts, ofpacts_len,
+OFPFC_MODIFY_STRICT);
 }
 }
 
@@ -1755,26 +1782,21 @@ ofproto_delete_flow(struct ofproto *ofproto,
 struct classifier *cls = &ofproto->tables[0].cls;
 struct rule *rule;
 
+/* First do a cheap check whether the rule we're looking for has already
+ * been deleted.  If so, then we're done. */
 ovs_rwlock_rdlock(&cls->rwlock);
 rule = rule_from_cls_rule(classifier_find_match_exactly(cls, target,
 priority));
 ovs_rwlock_unlock(&cls->rwlock);
 if (!rule) {
-/* No such rule -> success. */
-return true;
-} else if (rule->pending) {
-/* An operation on the rule is already pending -> failure.
- * Caller must retry later if it's important. */
-return false;
-} else {
-/* Initiate deletion -> success. */
-ovs_rwlock_wrlock(&cls->rwlock);
-ofproto_rule_delete(ofproto, cls, rule);
-ovs_rwlock_unlock(&cls->rwlock);
-
 return true;
 }
 
+/* Fall back to a executing a full flow mod.  We can't optimize this at all
+ * because we didn't take enough locks above to ensure that the flow table
+ * didn't already change beneath us.  */
+return simple_flow_mod(ofproto, target, priority, NULL, 0,
+   OFPFC_DELETE_STRICT) != OFPROTO_POSTPONE;
 }
 
 /* Starts the process of deleting all of the flows from all of ofproto's flow
-- 
1.7.10.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 10/11] ofproto: Break actions out of rule into new rule_actions structure.

[Ben Pfaff]

This permits code to ensure long-term access to a rule's actions
without holding a long-term lock on the rule's rwlock.

Signed-off-by: Ben Pfaff 
---
 ofproto/connmgr.c|4 +-
 ofproto/ofproto-dpif-xlate.c |   16 +++--
 ofproto/ofproto-dpif.c   |   26 +---
 ofproto/ofproto-dpif.h   |4 +-
 ofproto/ofproto-provider.h   |   28 -
 ofproto/ofproto.c|  140 --
 6 files changed, 151 insertions(+), 67 deletions(-)

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 2f315e6..1edbd59 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1903,8 +1903,8 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
 ovs_mutex_unlock(&rule->timeout_mutex);
 
 if (flags & NXFMF_ACTIONS) {
-fu.ofpacts = rule->ofpacts;
-fu.ofpacts_len = rule->ofpacts_len;
+fu.ofpacts = rule->actions->ofpacts;
+fu.ofpacts_len = rule->actions->ofpacts_len;
 } else {
 fu.ofpacts = NULL;
 fu.ofpacts_len = 0;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e7cec14..eb6a1f9 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -44,6 +44,7 @@
 #include "ofproto/ofproto-dpif-mirror.h"
 #include "ofproto/ofproto-dpif-sflow.h"
 #include "ofproto/ofproto-dpif.h"
+#include "ofproto/ofproto-provider.h"
 #include "tunnel.h"
 #include "vlog.h"
 
@@ -1671,8 +1672,7 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif 
*rule)
 OVS_RELEASES(rule)
 {
 struct rule_dpif *old_rule = ctx->rule;
-const struct ofpact *ofpacts;
-size_t ofpacts_len;
+struct rule_actions *actions;
 
 if (ctx->xin->resubmit_stats) {
 rule_dpif_credit_stats(rule, ctx->xin->resubmit_stats);
@@ -1680,8 +1680,9 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif 
*rule)
 
 ctx->recurse++;
 ctx->rule = rule;
-rule_dpif_get_actions(rule, &ofpacts, &ofpacts_len);
-do_xlate_actions(ofpacts, ofpacts_len, ctx);
+actions = rule_dpif_get_actions(rule);
+do_xlate_actions(actions->ofpacts, actions->ofpacts_len, ctx);
+rule_actions_unref(actions);
 ctx->rule = old_rule;
 ctx->recurse--;
 
@@ -2528,6 +2529,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out 
*xout)
 struct flow_wildcards *wc = &xout->wc;
 struct flow *flow = &xin->flow;
 
+struct rule_actions *actions = NULL;
 enum slow_path_reason special;
 const struct ofpact *ofpacts;
 struct xport *in_port;
@@ -2604,7 +2606,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out 
*xout)
 ofpacts = xin->ofpacts;
 ofpacts_len = xin->ofpacts_len;
 } else if (xin->rule) {
-rule_dpif_get_actions(xin->rule, &ofpacts, &ofpacts_len);
+actions = rule_dpif_get_actions(xin->rule);
+ofpacts = actions->ofpacts;
+ofpacts_len = actions->ofpacts_len;
 } else {
 NOT_REACHED();
 }
@@ -2690,4 +2694,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out 
*xout)
 
 out:
 ovs_rwlock_unlock(&xlate_rwlock);
+
+rule_actions_unref(actions);
 }
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 97735e2..cbaae5a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4162,12 +4162,13 @@ facet_is_controller_flow(struct facet *facet)
 bool is_controller;
 
 rule_dpif_lookup(ofproto, &facet->flow, NULL, &rule);
-ofpacts_len = rule->up.ofpacts_len;
-ofpacts = rule->up.ofpacts;
+ofpacts_len = rule->up.actions->ofpacts_len;
+ofpacts = rule->up.actions->ofpacts;
 is_controller = ofpacts_len > 0
 && ofpacts->type == OFPACT_CONTROLLER
 && ofpact_next(ofpacts) >= ofpact_end(ofpacts, ofpacts_len);
 rule_dpif_release(rule);
+
 return is_controller;
 }
 return false;
@@ -4519,12 +4520,20 @@ rule_dpif_reduce_timeouts(struct rule_dpif *rule, 
uint16_t idle_timeout,
 ofproto_rule_reduce_timeouts(&rule->up, idle_timeout, hard_timeout);
 }
 
-void
-rule_dpif_get_actions(const struct rule_dpif *rule,
-  const struct ofpact **ofpacts, size_t *ofpacts_len)
+/* Returns 'rule''s actions.  The caller owns a reference on the returned
+ * actions and must eventually release it (with rule_actions_unref()) to avoid
+ * a memory leak. */
+struct rule_actions *
+rule_dpif_get_actions(const struct rule_dpif *rule)
 {
-*ofpacts = rule->up.ofpacts;
-*ofpacts_len = rule->up.ofpacts_len;
+struct rule_actions *actions;
+
+ovs_rwlock_rdlock(&rule->up.rwlock);
+actions = rule->up.actions;
+rule_actions_ref(actions);
+ovs_rwlock_unlock(&rule->up.rwlock);
+
+return actions;
 }
 
 /* Subfacets. */
@@ -5308,7 +5317,8 @@ trace_format_rule(struct ds *result, int level, const 
struct rule_dpif *rule)
 
 ds_put_char_mu

[ovs-dev] [PATCH 07/11] ofproto: Merge ofproto_rule_delete() and ofproto_delete_rule().

[Ben Pfaff]

These functions were identical but had different names (one just called
the other).  Make them a single function.

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto.c |   35 +++
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index b027873..b365866 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1081,10 +1081,20 @@ ofproto_get_snoops(const struct ofproto *ofproto, 
struct sset *snoops)
 
 /* Deletes 'rule' from 'cls' within 'ofproto'.
  *
+ * Within an ofproto implementation, this function allows an ofproto
+ * implementation to destroy any rules that remain when its ->destruct()
+ * function is called.  This function is not suitable for use elsewhere in an
+ * ofproto implementation.
+ *
+ * This function is also used internally in ofproto.c.
+ *
+ * This function implements steps 4.4 and 4.5 in the section titled "Rule Life
+ * Cycle" in ofproto-provider.h.
+
  * The 'cls' argument is redundant (it is &ofproto->tables[rule->table_id].cls)
  * but it allows Clang to do better checking. */
-static void
-ofproto_delete_rule(struct ofproto *ofproto, struct classifier *cls,
+void
+ofproto_rule_delete(struct ofproto *ofproto, struct classifier *cls,
 struct rule *rule)
 OVS_REQ_WRLOCK(cls->rwlock)
 {
@@ -1122,7 +1132,7 @@ ofproto_flush__(struct ofproto *ofproto)
 cls_cursor_init(&cursor, &table->cls, NULL);
 CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, cr, &cursor) {
 if (!rule->pending) {
-ofproto_delete_rule(ofproto, &table->cls, rule);
+ofproto_rule_delete(ofproto, &table->cls, rule);
 }
 }
 ovs_rwlock_unlock(&table->cls.rwlock);
@@ -1759,7 +1769,7 @@ ofproto_delete_flow(struct ofproto *ofproto,
 } else {
 /* Initiate deletion -> success. */
 ovs_rwlock_wrlock(&cls->rwlock);
-ofproto_delete_rule(ofproto, cls, rule);
+ofproto_rule_delete(ofproto, cls, rule);
 ovs_rwlock_unlock(&cls->rwlock);
 
 return true;
@@ -2274,21 +2284,6 @@ ofproto_rule_destroy__(struct rule *rule)
 rule->ofproto->ofproto_class->rule_dealloc(rule);
 }
 
-/* This function allows an ofproto implementation to destroy any rules that
- * remain when its ->destruct() function is called.  This function implements
- * steps 4.4 and 4.5 in the section titled "Rule Life Cycle" in
- * ofproto-provider.h.
- *
- * This function should only be called from an ofproto implementation's
- * ->destruct() function.  It is not suitable elsewhere. */
-void
-ofproto_rule_delete(struct ofproto *ofproto, struct classifier *cls,
-struct rule *rule)
-OVS_REQ_WRLOCK(cls->rwlock)
-{
-ofproto_delete_rule(ofproto, cls, rule);
-}
-
 /* Returns true if 'rule' has an OpenFlow OFPAT_OUTPUT or OFPAT_ENQUEUE action
  * that outputs to 'port' (output to OFPP_FLOOD and OFPP_ALL doesn't count). */
 bool
@@ -3862,7 +3857,7 @@ ofproto_rule_expire(struct rule *rule, uint8_t reason)
 ofproto_rule_send_removed(rule, reason);
 
 ovs_rwlock_wrlock(&cls->rwlock);
-ofproto_delete_rule(ofproto, cls, rule);
+ofproto_rule_delete(ofproto, cls, rule);
 ovs_rwlock_unlock(&cls->rwlock);
 }
 
-- 
1.7.10.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 02/11] ofproto: Support out_group feature when matching on cookie.

[Ben Pfaff]

Found by inspection.

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 03738ab..e7ee60e 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2982,7 +2982,8 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t 
table_id,
 goto exit;
 }
 if (rule->flow_cookie == cookie /* Hash collisions possible. */
-&& ofproto_rule_has_out_port(rule, out_port)) {
+&& ofproto_rule_has_out_port(rule, out_port)
+&& ofproto_rule_has_out_group(rule, out_group)) {
 list_push_back(rules, &rule->ofproto_node);
 }
 }
@@ -3064,7 +3065,8 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t 
table_id,
 goto exit;
 }
 if (rule->flow_cookie == cookie /* Hash collisions possible. */
-&& ofproto_rule_has_out_port(rule, out_port)) {
+&& ofproto_rule_has_out_port(rule, out_port)
+&& ofproto_rule_has_out_group(rule, out_group)) {
 list_push_back(rules, &rule->ofproto_node);
 }
 }
-- 
1.7.10.4

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 11/11] ofproto: Add a ref_count to "struct rule" to protect it from being freed.

[Ben Pfaff]

Taking a read-lock on the 'rwlock' member of struct rule prevents members
of the rule from changing.  This is a short-term use of the 'rwlock': one
takes the lock, reads some members, and releases the lock.

Taking a read-lock on the 'rwlock' also prevents the rule from being freed.
This is often a relatively long-term need.  For example, until now flow
translation has held the rwlock in xlate_table_action() across the entire
recursive translation, which can call into a great deal of different code
across multiple files.

This commit switches to using a reference count for this second purpose
of struct rule's rwlock.  This means that all the code that previously
held a read-lock on the rwlock across deep stacks of functions can now
simply keep a reference.

Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto-dpif-upcall.c |2 +-
 ofproto/ofproto-dpif-xlate.c  |   16 +++-
 ofproto/ofproto-dpif.c|   36 +++-
 ofproto/ofproto-dpif.h|   12 +---
 ofproto/ofproto-provider.h|5 +
 ofproto/ofproto.c |   32 +---
 6 files changed, 62 insertions(+), 41 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 54f441b..ae856a4 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -725,7 +725,7 @@ execute_flow_miss(struct flow_miss *miss, struct dpif_op 
*ops, size_t *n_ops)
 xlate_actions_for_side_effects(&xin);
 }
 }
-rule_dpif_release(rule);
+rule_dpif_unref(rule);
 
 if (miss->xout.odp_actions.size) {
 LIST_FOR_EACH (packet, list_node, &miss->packets) {
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index eb6a1f9..611861c 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1669,7 +1669,6 @@ compose_output_action(struct xlate_ctx *ctx, ofp_port_t 
ofp_port)
 
 static void
 xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule)
-OVS_RELEASES(rule)
 {
 struct rule_dpif *old_rule = ctx->rule;
 struct rule_actions *actions;
@@ -1686,7 +1685,7 @@ xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif 
*rule)
 ctx->rule = old_rule;
 ctx->recurse--;
 
-rule_dpif_release(rule);
+rule_dpif_unref(rule);
 }
 
 static void
@@ -1697,7 +1696,6 @@ xlate_table_action(struct xlate_ctx *ctx,
 struct rule_dpif *rule;
 ofp_port_t old_in_port = ctx->xin->flow.in_port.ofp_port;
 uint8_t old_table_id = ctx->table_id;
-bool got_rule;
 
 ctx->table_id = table_id;
 
@@ -1705,18 +1703,16 @@ xlate_table_action(struct xlate_ctx *ctx,
  * original input port (otherwise OFPP_NORMAL and OFPP_IN_PORT will
  * have surprising behavior). */
 ctx->xin->flow.in_port.ofp_port = in_port;
-got_rule = rule_dpif_lookup_in_table(ctx->xbridge->ofproto,
- &ctx->xin->flow, &ctx->xout->wc,
- table_id, &rule);
+rule_dpif_lookup_in_table(ctx->xbridge->ofproto,
+  &ctx->xin->flow, &ctx->xout->wc,
+  table_id, &rule);
 ctx->xin->flow.in_port.ofp_port = old_in_port;
 
 if (ctx->xin->resubmit_hook) {
 ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse);
 }
 
-if (got_rule) {
-xlate_recursively(ctx, rule);
-} else if (may_packet_in) {
+if (!rule && may_packet_in) {
 struct xport *xport;
 
 /* XXX
@@ -1729,6 +1725,8 @@ xlate_table_action(struct xlate_ctx *ctx,
 choose_miss_rule(xport ? xport->config : 0,
  ctx->xbridge->miss_rule,
  ctx->xbridge->no_packet_in_rule, &rule);
+}
+if (rule) {
 xlate_recursively(ctx, rule);
 }
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index cbaae5a..2b2fe62 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1371,7 +1371,7 @@ add_internal_flow(struct ofproto_dpif *ofproto, int id,
 
 if (rule_dpif_lookup_in_table(ofproto, &fm.match.flow, NULL, TBL_INTERNAL,
   rulep)) {
-rule_dpif_release(*rulep);
+rule_dpif_unref(*rulep);
 } else {
 NOT_REACHED();
 }
@@ -4167,7 +4167,7 @@ facet_is_controller_flow(struct facet *facet)
 is_controller = ofpacts_len > 0
 && ofpacts->type == OFPACT_CONTROLLER
 && ofpact_next(ofpacts) >= ofpact_end(ofpacts, ofpacts_len);
-rule_dpif_release(rule);
+rule_dpif_unref(rule);
 
 return is_controller;
 }
@@ -4262,7 +4262,7 @@ facet_check_consistency(struct facet *facet)
 rule_dpif_lookup(facet->ofproto, &facet->flow, NULL, &rule);
 xlate_in_init(&xin, facet->ofproto, &facet->flow, rule, 0, NULL);
 xlate_a

Re: [ovs-dev] [PATCH] lisp: Reset vlan tag on send.

[Pravin Shelar]

Thanks.
I pushed patch to master.

On Mon, Sep 9, 2013 at 1:26 PM, Jesse Gross  wrote:
> On Mon, Sep 9, 2013 at 9:21 AM, Pravin B Shelar  wrote:
>> Lisp needs to discards all l2 packet headers but if vlan tx
>> is hw-acceleraed vlan tag is stored in skb struct. Following
>> patch resets it.
>>
>> Signed-off-by: Pravin B Shelar 
>
> Acked-by: Jesse Gross 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath: Remove compat workqueue.

[Pravin Shelar]

On Mon, Sep 9, 2013 at 2:07 PM, Jesse Gross  wrote:
> On Mon, Sep 9, 2013 at 1:53 PM, Pravin B Shelar  wrote:
>> OVS has its own workq implementation for coupe of reasons. first
>> was to avoid system freeze due to ovs-flow rehash softlockup.
>> We have moved out rehash from workq, So this problem does not exist.
>> second was related bugs in kernel workq implementation in pre-2.6.32
>> kernel. But we have dropped support for older kernel.
>> So there is no reason to keep ovs-workq around. Following patch
>> removes it.
>>
>> Signed-off-by: Pravin B Shelar 
>
> Acked-by: Jesse Gross 

Thanks.
I pushed patch to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] Don't set subprogram name to empty

[Ben Pfaff]

On Mon, Sep 09, 2013 at 09:38:01AM -0700, gy...@nicira.com wrote:
> From: Guolin Yang 
> 
> In monitor_daemon(), it set subprogram_name to "" which causes
> system crash in some platform. This change set subprogram name
> to program name.
> 
> Signed-off-by: Guolin Yang 
> ---
> Based on Ben's comments, only set pthread's subprogram name to non empty

Applied, thanks.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] datapath: Remove compat workqueue.

[Pravin B Shelar]

OVS has its own workq implementation for coupe of reasons. first
was to avoid system freeze due to ovs-flow rehash softlockup.
We have moved out rehash from workq, So this problem does not exist.
second was related bugs in kernel workq implementation in pre-2.6.32
kernel. But we have dropped support for older kernel.
So there is no reason to keep ovs-workq around. Following patch
removes it.

Signed-off-by: Pravin B Shelar 
---
 datapath/datapath.c |9 +-
 datapath/dp_notify.c|2 +-
 datapath/linux/Modules.mk   |1 -
 datapath/linux/compat/include/linux/workqueue.h |   70 +---
 datapath/linux/compat/vxlan.c   |2 +-
 datapath/linux/compat/workqueue.c   |  225 ---
 6 files changed, 6 insertions(+), 303 deletions(-)
 delete mode 100644 datapath/linux/compat/workqueue.c

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 0780589..4defcdb 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -2385,13 +2385,9 @@ static int __init dp_init(void)
pr_info("Open vSwitch switching datapath %s, built "__DATE__" 
"__TIME__"\n",
VERSION);
 
-   err = ovs_workqueues_init();
-   if (err)
-   goto error;
-
err = ovs_flow_init();
if (err)
-   goto error_wq;
+   goto error;
 
err = ovs_vport_init();
if (err)
@@ -2419,8 +2415,6 @@ error_vport_exit:
ovs_vport_exit();
 error_flow_exit:
ovs_flow_exit();
-error_wq:
-   ovs_workqueues_exit();
 error:
return err;
 }
@@ -2433,7 +2427,6 @@ static void dp_cleanup(void)
rcu_barrier();
ovs_vport_exit();
ovs_flow_exit();
-   ovs_workqueues_exit();
 }
 
 module_init(dp_init);
diff --git a/datapath/dp_notify.c b/datapath/dp_notify.c
index 847f611..b5178fc 100644
--- a/datapath/dp_notify.c
+++ b/datapath/dp_notify.c
@@ -90,7 +90,7 @@ static int dp_device_event(struct notifier_block *unused, 
unsigned long event,
 
if (event == NETDEV_UNREGISTER) {
ovs_net = net_generic(dev_net(dev), ovs_net_id);
-   queue_work(&ovs_net->dp_notify_work);
+   queue_work(system_wq, &ovs_net->dp_notify_work);
}
 
return NOTIFY_DONE;
diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index e3c42cd..057e1d5 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -12,7 +12,6 @@ openvswitch_sources += \
linux/compat/reciprocal_div.c \
linux/compat/skbuff-openvswitch.c \
linux/compat/vxlan.c\
-   linux/compat/workqueue.c \
linux/compat/utils.c
 openvswitch_headers += \
linux/compat/gso.h \
diff --git a/datapath/linux/compat/include/linux/workqueue.h 
b/datapath/linux/compat/include/linux/workqueue.h
index b2de545..461fefd 100644
--- a/datapath/linux/compat/include/linux/workqueue.h
+++ b/datapath/linux/compat/include/linux/workqueue.h
@@ -1,74 +1,10 @@
 #ifndef __LINUX_WORKQUEUE_WRAPPER_H
 #define __LINUX_WORKQUEUE_WRAPPER_H 1
 
-#include 
+#include_next 
 
-int __init ovs_workqueues_init(void);
-void ovs_workqueues_exit(void);
-
-/* Older kernels have an implementation of work queues with some very bad
- * characteristics when trying to cancel work (potential deadlocks, use after
- * free, etc.  Therefore we implement simple ovs specific work queue using
- * single worker thread. work-queue API are kept similar for compatibility.
- * It seems it is useful even on newer kernel. As it can avoid system wide
- * freeze in event of softlockup due to workq blocked on genl_lock.
- */
-
-struct work_struct;
-
-typedef void (*work_func_t)(struct work_struct *work);
-
-#define work_data_bits(work) ((unsigned long *)(&(work)->data))
-
-struct work_struct {
-#define WORK_STRUCT_PENDING 0   /* T if work item pending execution */
-   atomic_long_t data;
-   struct list_head entry;
-   work_func_t func;
-#ifdef CONFIG_LOCKDEP
-   struct lockdep_map lockdep_map;
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,36)
+#define queue_work(wq, dw) schedule_work(dw);
 #endif
-};
-
-#define WORK_DATA_INIT()ATOMIC_LONG_INIT(0)
-
-#define work_clear_pending(work)   \
-   clear_bit(WORK_STRUCT_PENDING, work_data_bits(work))
-
-struct delayed_work {
-   struct work_struct work;
-   struct timer_list timer;
-};
-
-#define __WORK_INITIALIZER(n, f) { \
-   .data = WORK_DATA_INIT(),   \
-   .entry  = { &(n).entry, &(n).entry },   \
-   .func = (f),\
-}
-
-#define __DELAYED_WORK_INITIALIZER(n, f) { \
-   .work = __WORK_INITIALIZER((n).work, (f)),  \
-   .timer = TIMER_INITIALIZER(NULL, 0, 0), \
-}
-
-#define DECLARE_DELAYED_WORK(n, f)   

Re: [ovs-dev] [PATCH 01/11] ofproto: Do not call ->rule_destruct() if ->rule_construct() failed.

[Ethan Jackson]

Acked-by: Ethan Jackson 


On Mon, Sep 9, 2013 at 2:22 PM, Ben Pfaff  wrote:
> Found by inspection.
>
> Signed-off-by: Ben Pfaff 
> ---
>  ofproto/ofproto.c |   23 +++
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 4d33de7..03738ab 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -199,6 +199,7 @@ static int init_ports(struct ofproto *);
>  static void reinit_ports(struct ofproto *);
>
>  /* rule. */
> +static void ofproto_rule_destroy(struct rule *);
>  static void ofproto_rule_destroy__(struct rule *);
>  static void ofproto_rule_send_removed(struct rule *, uint8_t reason);
>  static bool rule_is_modifiable(const struct rule *);
> @@ -2256,18 +2257,24 @@ update_mtu(struct ofproto *p, struct ofport *port)
>  }
>
>  static void
> -ofproto_rule_destroy__(struct rule *rule)
> +ofproto_rule_destroy(struct rule *rule)
>  {
>  if (rule) {
>  rule->ofproto->ofproto_class->rule_destruct(rule);
> -cls_rule_destroy(&rule->cr);
> -free(rule->ofpacts);
> -ovs_mutex_destroy(&rule->timeout_mutex);
> -ovs_rwlock_destroy(&rule->rwlock);
> -rule->ofproto->ofproto_class->rule_dealloc(rule);
> +ofproto_rule_destroy__(rule);
>  }
>  }
>
> +static void
> +ofproto_rule_destroy__(struct rule *rule)
> +{
> +cls_rule_destroy(&rule->cr);
> +free(rule->ofpacts);
> +ovs_mutex_destroy(&rule->timeout_mutex);
> +ovs_rwlock_destroy(&rule->rwlock);
> +rule->ofproto->ofproto_class->rule_dealloc(rule);
> +}
> +
>  /* This function allows an ofproto implementation to destroy any rules that
>   * remain when its ->destruct() function is called..  This function 
> implements
>   * steps 4.4 and 4.5 in the section titled "Rule Life Cycle" in
> @@ -5508,13 +5515,13 @@ ofopgroup_complete(struct ofopgroup *group)
>  } else {
>  ovs_rwlock_wrlock(&rule->rwlock);
>  oftable_remove_rule(rule);
> -ofproto_rule_destroy__(rule);
> +ofproto_rule_destroy(rule);
>  }
>  break;
>
>  case OFOPERATION_DELETE:
>  ovs_assert(!op->error);
> -ofproto_rule_destroy__(rule);
> +ofproto_rule_destroy(rule);
>  op->rule = NULL;
>  break;
>
> --
> 1.7.10.4
>
> ___
> 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 02/11] ofproto: Support out_group feature when matching on cookie.

[Ethan Jackson]

Acked-by: Ethan Jackson 


On Mon, Sep 9, 2013 at 2:22 PM, Ben Pfaff  wrote:
> Found by inspection.
>
> Signed-off-by: Ben Pfaff 
> ---
>  ofproto/ofproto.c |6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 03738ab..e7ee60e 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -2982,7 +2982,8 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t 
> table_id,
>  goto exit;
>  }
>  if (rule->flow_cookie == cookie /* Hash collisions possible. 
> */
> -&& ofproto_rule_has_out_port(rule, out_port)) {
> +&& ofproto_rule_has_out_port(rule, out_port)
> +&& ofproto_rule_has_out_group(rule, out_group)) {
>  list_push_back(rules, &rule->ofproto_node);
>  }
>  }
> @@ -3064,7 +3065,8 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t 
> table_id,
>  goto exit;
>  }
>  if (rule->flow_cookie == cookie /* Hash collisions possible. 
> */
> -&& ofproto_rule_has_out_port(rule, out_port)) {
> +&& ofproto_rule_has_out_port(rule, out_port)
> +&& ofproto_rule_has_out_group(rule, out_group)) {
>  list_push_back(rules, &rule->ofproto_node);
>  }
>  }
> --
> 1.7.10.4
>
> ___
> 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] Report timestamps in millisecond resolution in log messages.

[Paul Ingram]

To make debugging easier.
---
 lib/dynamic-string.c  |   21 +++--
 lib/dynamic-string.h  |7 ++---
 lib/table.c   |2 +-
 lib/timeval.c |   79 +
 lib/timeval.h |9 ++
 lib/vlog.c|   16 +-
 lib/vlog.h|2 +-
 python/ovs/vlog.py|3 +-
 tests/vlog.at |2 +-
 utilities/ovs-ofctl.c |3 +-
 10 files changed, 118 insertions(+), 26 deletions(-)

diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c
index 9b3e7ba..66ffc8f 100644
--- a/lib/dynamic-string.c
+++ b/lib/dynamic-string.c
@@ -183,21 +183,24 @@ ds_put_printable(struct ds *ds, const char *s, size_t n)
 }
 }
 
-/* Writes time 'when' to 'string' based on 'template', in local time or UTC
- * based on 'utc'. */
+/* Writes the current time with optional millisecond resolution to 'string'
+ * based on 'template'.
+ * The current time is either localtime or UTC based on 'utc'. */
 void
-ds_put_strftime(struct ds *ds, const char *template, time_t when, bool utc)
+ds_put_strftime_msec(struct ds *ds, const char *template, long long int when,
+bool utc)
 {
-struct tm tm;
+struct tm_msec tm;
 if (utc) {
-gmtime_r(&when, &tm);
+gmtime_msec(when, &tm);
 } else {
-localtime_r(&when, &tm);
+localtime_msec(when, &tm);
 }
 
 for (;;) {
 size_t avail = ds->string ? ds->allocated - ds->length + 1 : 0;
-size_t used = strftime(&ds->string[ds->length], avail, template, &tm);
+size_t used = strftime_msec(&ds->string[ds->length], avail, template,
+   &tm);
 if (used) {
 ds->length += used;
 return;
@@ -209,12 +212,12 @@ ds_put_strftime(struct ds *ds, const char *template, 
time_t when, bool utc)
 /* Returns a malloc()'d string for time 'when' based on 'template', in local
  * time or UTC based on 'utc'. */
 char *
-xastrftime(const char *template, time_t when, bool utc)
+xastrftime_msec(const char *template, long long int when, bool utc)
 {
 struct ds s;
 
 ds_init(&s);
-ds_put_strftime(&s, template, when, utc);
+ds_put_strftime_msec(&s, template, when, utc);
 return s.string;
 }
 
diff --git a/lib/dynamic-string.h b/lib/dynamic-string.h
index c069586..2272343 100644
--- a/lib/dynamic-string.h
+++ b/lib/dynamic-string.h
@@ -61,10 +61,9 @@ int ds_get_line(struct ds *, FILE *);
 int ds_get_preprocessed_line(struct ds *, FILE *, int *line_number);
 int ds_get_test_line(struct ds *, FILE *);
 
-void ds_put_strftime(struct ds *, const char *template, time_t when, bool utc)
-STRFTIME_FORMAT(2);
-char *xastrftime(const char *template, time_t when, bool utc)
-STRFTIME_FORMAT(1);
+void ds_put_strftime_msec(struct ds *, const char *template, long long int 
when,
+ bool utc);
+char *xastrftime_msec(const char *template, long long int when, bool utc);
 
 char *ds_cstr(struct ds *);
 const char *ds_cstr_ro(const struct ds *);
diff --git a/lib/table.c b/lib/table.c
index 21f4905..c49487c 100644
--- a/lib/table.c
+++ b/lib/table.c
@@ -221,7 +221,7 @@ table_print_table_line__(struct ds *line)
 static char *
 table_format_timestamp__(void)
 {
-return xastrftime("%Y-%m-%d %H:%M:%S", time_wall(), true);
+return xastrftime_msec("%Y-%m-%d %H:%M:%S.%.", time_wall_msec(), true);
 }
 
 static void
diff --git a/lib/timeval.c b/lib/timeval.c
index 37b4353..dd1948e 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -70,6 +70,9 @@ static struct rusage *get_recent_rusage(void);
 static void refresh_rusage(void);
 static void timespec_add(struct timespec *sum,
  const struct timespec *a, const struct timespec *b);
+static void format_msec(char *buf, size_t buf_len, const char *fmt, int msec);
+static char *buf_copy(char *dst, const char *dst_end,
+ const char *src, const char *src_end);
 
 static void
 init_clock(struct clock *c, clockid_t id)
@@ -493,3 +496,79 @@ timeval_dummy_register(void)
 unixctl_command_register("time/warp", "MSECS", 1, 1,
  timeval_warp_cb, NULL);
 }
+
+
+/* Millisecond resolution timestamps. */
+size_t
+strftime_msec(char *s, size_t max, const char *format,
+  const struct tm_msec *tm)
+{
+char buf[256];
+
+format_msec(buf, sizeof buf, format, tm->msec);
+
+return strftime(s, max, buf, &tm->tm);
+}
+
+struct tm_msec*
+localtime_msec(long long int now, struct tm_msec *result) {
+  time_t now_sec = now / 1000;
+  localtime_r(now_sec, &result->tm);
+  result->msec = now % 1000;
+  return result;
+}
+
+struct tm_msec*
+gmtime_msec(long long int now, struct tm_msec *result) {
+  time_t now_sec = now / 1000;
+  gmtime_r(&now_sec, &result->tm);
+  result->msec = now % 1000;
+  return result;
+}
+
+void
+format_msec(char *buf, size_t buf_len, const char *fmt, int msec)
+{
+char *buf_ptr = buf;
+const char *buf_end = 

[ovs-dev] [PATCH 2/2] ovsdb: timestamp database records to millisecond resolution.

[Paul Ingram]

The ovsdb-server compaction timing logic is written assuming milliscond
resolution timestamps but ovsdb-server wrote second resolution timestamps.

This commit changes ovsdb-server to write millisecond resolution timestamps
and ovsdb-tool to report millisecond timestamps.

This raises two compatibility issues:
1. When a new ovsdb-server or ovsdb-tool reads an old database, it will
multiply by 1000 any timestamp it reads which is less than 1<<31. Since
this date corresponds to Jan 16 1970 this is unlikely to cause a problem.
2. When an old ovsdb-tool reads a new database, it will interpret the
millisecond timestamps as seconds and report dates in the far future; the
time of this commit is reported as the year 45645 (each second since the
epoch is interpreted as 16 minutes). (When an old ovsdb-server reads a
new database there is no problem, it is already interpreting the timestamps
in milliseconds).
---
 ovsdb/file.c   |6 +-
 ovsdb/ovsdb-tool.c |4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/ovsdb/file.c b/ovsdb/file.c
index b02d5a3..5b79f8c 100644
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -408,6 +408,10 @@ ovsdb_file_txn_from_json(struct ovsdb *db, const struct 
json *json,
 if (!strcmp(table_name, "_date")
 && node_json->type == JSON_INTEGER) {
 *date = json_integer(node_json);
+if (*date < INT_MAX) {
+/* Older versions of ovsdb wrote timestamps in seconds. */
+*date *= 1000;
+}
 continue;
 } else if (!strcmp(table_name, "_comment") || converting) {
 continue;
@@ -787,7 +791,7 @@ ovsdb_file_txn_commit(struct json *json, const char 
*comment,
 if (comment) {
 json_object_put_string(json, "_comment", comment);
 }
-json_object_put(json, "_date", json_integer_create(time_wall()));
+json_object_put(json, "_date", json_integer_create(time_wall_msec()));
 
 error = ovsdb_log_write(log, json);
 json_destroy(json);
diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
index 7cd0485..9c7eeb2 100644
--- a/ovsdb/ovsdb-tool.c
+++ b/ovsdb/ovsdb-tool.c
@@ -518,8 +518,8 @@ do_show_log(int argc, char *argv[])
 
 date = shash_find_data(json_object(json), "_date");
 if (date && date->type == JSON_INTEGER) {
-time_t t = json_integer(date);
-char *s = xastrftime(" %Y-%m-%d %H:%M:%S", t, true);
+long long int t = json_integer(date);
+char *s = xastrftime_msec(" %Y-%m-%d %H:%M:%S.%.", t, true);
 fputs(s, stdout);
 free(s);
 }
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Openflow ofp 1.o wireshark plugin

[Vasiliy Tolstov]

I need to debug some issues with openvswitch openflow messages that sended
to my own controller, where I can find wireshark plugin to see what
actually happening?
I can find some dissectors but all needs patching and have errors.
I have wireshark 1.8 and support for ofp 1.0.
Thanks for any links and help.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Ben Pfaff]

On Tue, Sep 10, 2013 at 06:56:13AM +0400, Vasiliy Tolstov wrote:
> I need to debug some issues with openvswitch openflow messages that sended
> to my own controller, where I can find wireshark plugin to see what
> actually happening?
> I can find some dissectors but all needs patching and have errors.
> I have wireshark 1.8 and support for ofp 1.0.

I don't think that such a dissector has ever been part of Open vSwitch.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Vasiliy Tolstov]

On Sep 10, 2013 7:16 AM, "Ben Pfaff"  wrote:

> I don't think that such a dissector has ever been part of Open vSwitch.

Thank you.
Is that possible that openvswitch 1.12 send openflow message of dhcp
discover with all options bytes equal to 0?
I'm try in-band / out-band but this no helps
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Ben Pfaff]

On Tue, Sep 10, 2013 at 07:27:51AM +0400, Vasiliy Tolstov wrote:
> Is that possible that openvswitch 1.12 send openflow message of dhcp
> discover with all options bytes equal to 0?

No version of Open vSwitch ever generates or modifies DHCP messages.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Ben Pfaff]

On Tue, Sep 10, 2013 at 08:09:04AM +0400, Vasiliy Tolstov wrote:
> 2013/9/10 Ben Pfaff :
> > No version of Open vSwitch ever generates or modifies DHCP messages.
> 
> Hmm. I'm find some dissector that support 0openflow 1.0.
> This is openflow message that send to controoller.
> As You can see it does not have option bytes. Last valid bytes is
> 52 54 00 5c cd 3a that contains mac address of network card
> (http://en.wikipedia.org/wiki/Dynamic_Host_Configuration_Protocol
> CHADDR (Client Hardware Address))
> But magic cookie and option bytes absent.
> Why ?

No version of Open vSwitch ever generates or modifies DHCP messages.  If
Open vSwitch sends a DHCP message to the controller in a packet-in, and
the packet-in does not show a magic cookie or option bytes, then it is
because, when Open vSwitch received the DHCP message, it did not have a
magic cookie or option bytes.

I can't speak for other OpenFlow implementations.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Vasiliy Tolstov]

2013/9/10 Ben Pfaff :
> No version of Open vSwitch ever generates or modifies DHCP messages.  If
> Open vSwitch sends a DHCP message to the controller in a packet-in, and
> the packet-in does not show a magic cookie or option bytes, then it is
> because, when Open vSwitch received the DHCP message, it did not have a
> magic cookie or option bytes.
>
> I can't speak for other OpenFlow implementations.


Okay. But why wireshark in case of dumpink bootp on vm interface sees
dhcp magic that absent on openflow message?

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
jabber: v...@selfip.ru
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Ben Pfaff]

On Tue, Sep 10, 2013 at 08:14:37AM +0400, Vasiliy Tolstov wrote:
> 2013/9/10 Ben Pfaff :
> > No version of Open vSwitch ever generates or modifies DHCP messages.  If
> > Open vSwitch sends a DHCP message to the controller in a packet-in, and
> > the packet-in does not show a magic cookie or option bytes, then it is
> > because, when Open vSwitch received the DHCP message, it did not have a
> > magic cookie or option bytes.
> >
> > I can't speak for other OpenFlow implementations.
> 
> Okay. But why wireshark in case of dumpink bootp on vm interface sees
> dhcp magic that absent on openflow message?

I don't know.  Are these options beyond the number of bytes sent by
default within an OpenFlow packet-in?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Vasiliy Tolstov]

2013/9/10 Ben Pfaff :
> I don't know.  Are these options beyond the number of bytes sent by
> default within an OpenFlow packet-in?


No in both cases thernet frame message have 342 bytes.

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
jabber: v...@selfip.ru
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Ben Pfaff]

On Tue, Sep 10, 2013 at 08:19:03AM +0400, Vasiliy Tolstov wrote:
> 2013/9/10 Ben Pfaff :
> > I don't know.  Are these options beyond the number of bytes sent by
> > default within an OpenFlow packet-in?
> 
> 
> No in both cases thernet frame message have 342 bytes.

OpenFlow defaults to sending the first 128 bytes.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Ben Pfaff]

On Tue, Sep 10, 2013 at 08:23:42AM +0400, Vasiliy Tolstov wrote:
> 2013/9/10 Vasiliy Tolstov :
> >
> > No in both cases thernet frame message have 342 bytes.
> 
> This is openflow ethernet frame in packet in
>    ff ff ff ff ff ff 52 54 00 5c cd 3a 08 00 45
> 0010   10 01 48 00 00 00 00 80 11 39 96 00 00 00 00 ff ff
> 0020   ff ff 00 44 00 43 01 34 cb 48 01 01 06 00 e6 39
> 0030   0c 51 00 03 00 00 00 00 00 00 00 00 00 00 00 00
> 0040   00 00 00 00 00 00 52 54 00 5c cd 3a 00 00 00 00
> 0050   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0060   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0070   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 
> This is normal dhcp message
>    ff ff ff ff ff ff 52 54 00 5c cd 3a 08 00 45 10
> 0010   01 48 00 00 00 00 80 11 39 96 00 00 00 00 ff ff
> 0020   ff ff 00 44 00 43 01 34 cb 4b 01 01 06 00 e6 39
> 0030   0c 51 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0040   00 00 00 00 00 00 52 54 00 5c cd 3a 00 00 00 00
> 0050   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0060   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0070   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0080   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0090   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00a0   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00b0   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00c0   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00d0   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00e0   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00f0   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0100   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0110   00 00 00 00 00 00 63 82 53 63 35 01 01 37 07 01
> 0120   1c 02 03 0f 06 0c ff 00 00 00 00 00 00 00 00 00
> 0130   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0140   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0150   00 00 00 00 00 00

The differences between these messages are different dhcp xids
(0x0134cb48 versus 0x0134cb4b) and, if I'm counting correctly, different
dhcp yiaddrs.  I see no other differences other than the first frame
being cut off after 128 bytes.  That is undoubtedly because you did not
tell the switch to send more than 128 bytes, which is the default.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Vasiliy Tolstov]

2013/9/10 Ben Pfaff :
> The differences between these messages are different dhcp xids
> (0x0134cb48 versus 0x0134cb4b) and, if I'm counting correctly, different
> dhcp yiaddrs.  I see no other differences other than the first frame
> being cut off after 128 bytes.  That is undoubtedly because you did not
> tell the switch to send more than 128 bytes, which is the default.


Very big thanks. Is that possible to configure vswitch to send first 512 bytes?

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
jabber: v...@selfip.ru
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Ben Pfaff]

On Tue, Sep 10, 2013 at 08:41:40AM +0400, Vasiliy Tolstov wrote:
> 2013/9/10 Ben Pfaff :
> > The differences between these messages are different dhcp xids
> > (0x0134cb48 versus 0x0134cb4b) and, if I'm counting correctly, different
> > dhcp yiaddrs.  I see no other differences other than the first frame
> > being cut off after 128 bytes.  That is undoubtedly because you did not
> > tell the switch to send more than 128 bytes, which is the default.
> 
> 
> Very big thanks. Is that possible to configure vswitch to send first
> 512 bytes?

Yes, read the OpenFlow spec.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Openflow ofp 1.o wireshark plugin

[Vasiliy Tolstov]

2013/9/10 Ben Pfaff :
> Yes, read the OpenFlow spec.


Thanks for answers.

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
jabber: v...@selfip.ru
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev